xhci_enumerate() reports failure by marking the hub port disconnected,
which is what makes xhci_wait() return and the attempt repeat. The root
port is still connected, so the two disagree again immediately and the
attempt repeats for as long as the device stays plugged in. A device that
fails every time is retried forever: 1055 attempts in 90 seconds on an
EIC7700X board, enough console traffic to make the board unusable.
Count consecutive failures per root port and stop at
CONFIG_USBHOST_XHCI_ENUM_RETRIES, leaving the port as it is so xhci_wait()
blocks until something physically changes. A new connection clears the
count, as does a successful enumeration, so a device needing a second
attempt still gets one. The default of three rides out a slow device or a
marginal reset.
The same board now makes three attempts, reports that it has given up and
falls silent, while a keyboard on the other port enumerates throughout.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
A device slot is a finite controller resource: HCSPARAMS1 reports how many
exist and Enable Slot fails with No Slots Available once they are gone.
Two paths took one and returned without giving it back.
xhci_device_init() enables a slot before initialising the transfer ring,
the slot context and the device address, and each of those returned
directly on failure. It also treated a slot number larger than the
controller supports as success, since Enable Slot itself had succeeded.
xhci_enumerate() is the larger leak: the device is addressed by the time
usbhost_enumerate() runs, so a device whose descriptor cannot be read, or
that no class driver claims, leaves the slot held. That path clears
hport->connected so the port is retried, taking another slot each time.
Release the slot on both paths with xhci_device_deinit(), which issues
Disable Slot, clears the DCBAA entry and resets the context. The endpoint
ring is left allocated; xhci_ring_init() reuses an existing one.
Tested on an EIC7700X board with a device no class driver claims, so the
port retries indefinitely: previously the eighth attempt failed with
completion code 9 and the controller enumerated nothing further on either
port; now 1104 consecutive attempts produced no slot failure.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
xhci_ctrl_xfer() and xhci_transfer() release the controller lock before
xhci_transfer_wait(), so the lock does not cover the interval in which a
transfer is outstanding. Two threads issuing requests on the same
endpoint both reach xhci_ioc_setup(), and the second trips the
DEBUGASSERT(!epinfo->iocwait) that guards it, or overwrites the first
thread's completion state where assertions are compiled out.
A default control endpoint reaches this readily: every interface driver on
a composite device speaks through endpoint 0, so a two interface HID
keyboard runs two poll threads both issuing GET_REPORT.
Other host controller drivers hold the controller lock across the wait,
which here would serialise the whole controller and give up the per
endpoint rings xHCI provides. Add a mutex to struct xhci_epinfo_s and
hold that instead. It is taken before the controller lock on both paths,
so the order is endpoint then controller.
xhci_epfree() also freed the endpoint container without destroying iocsem.
Destroy both.
Reachable on any xHCI controller, independently of the preceding commits.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
Submitting an asynchronous transfer refused any buffer needing a cache
line stand-in, and that test also refuses every buffer whose length is not
a whole number of cache lines, which an interrupt transfer's rarely is: a
HID keyboard reads eight bytes. Every submission returned -EFAULT before
a descriptor was written, and a class driver resubmitting from its
completion callback never sees a second chance.
The refusal existed because the copy out of a stand-in is done by the
blocked caller, and an asynchronous transfer has none. The work queue
thread handling the completion will do: a buffer given to DRVR_ASYNCH
comes from DRVR_ALLOC, so it is kernel memory reachable from any thread.
Use the same stand-in machinery as every other transfer and finish the DMA
in the completion, just before the callback. A cancelled transfer returns
its stand-in on cancellation.
The callback also moves outside the spinlock. It is class driver code
that queues work and takes its own locks, and it may now free a stand-in.
Whether a completion is synchronous is still decided under the lock, since
a posted waiter may be carrying a new transfer immediately.
The asynchronous setup now records the requested length, as the
synchronous setup does. The byte count handed to the callback is worked
out from it and the residue, and was previously whatever the endpoint held
from an earlier transfer.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
A root hub port whose enumeration failed is enumerated again, and the slot
the failed attempt used has been given back by then, so the port has no
device context behind it. xhci_epalloc() took that pointer and wrote the
new endpoint through it without looking, so the retry stored through NULL
and took the system down in answer to a device that had merely failed to
come up.
Check for the device, and free the endpoint that has no home rather than
leaking it.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
The Interval field of an endpoint context is an exponent: the controller
services the endpoint every 2^Interval microframes. An endpoint
descriptor states its period differently depending on device speed, so the
number cannot be copied across, which is what this did. A low speed
keyboard asking to be polled every 10ms was programmed as 2^10
microframes, which the controller would not accept: Configure Endpoint
went unanswered and allocation failed with -EIO.
Low and full speed interrupt endpoints state a period in frames, so the
exponent is the highest bit of that period in microframes, clamped to the
range the specification allows. Other periodic endpoints already state an
exponent, one greater than the one wanted here. Control and bulk
endpoints are not periodic and the field means nothing to them.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
The copy out of a stand-in was done in the completion handler, which runs
on a work queue, while the buffer it copies into may belong to a user
process whose addresses mean nothing there. Reading a block device
directly from a user program faulted. The caller is blocked until the
transfer finishes, so the copy belongs there.
An asynchronous transfer has no blocked caller to come back to, so a
buffer that would need a stand-in is refused for that path. Its callers
are class drivers using kernel memory, which do not need one. The
refusal is lifted once the completion path can do the copy itself.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
Report each device as it comes up, and report it going away.
The announcement is made at the end of the port enable rather than at
connect, because the PORTSC speed field means nothing until the port has
been reset: a USB2 port reports its reset default, full speed, until then,
so every device would be announced at 12Mbps regardless of what it
negotiates a moment later.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
A transfer described by more than one TRB can reach the end of the ring
part way through, so the link that sends the controller back to the
beginning falls inside the transfer rather than between two of them.
Written without the chain bit, that link ends the transfer where it
stands: the controller follows it, considers the work finished, and
reports nothing, because the TRB that asked for the completion interrupt
is on the far side of the join. Nothing waiting is woken, and transfers
have no timeout, so the symptom is a read that never returns.
Carry the chain bit onto the link when the TRB it follows has it.
Reading 1MiB from a USB drive, where the last two sizes did not complete
at all before:
512 byte blocks 166 KB/s
4 KiB blocks 1333 KB/s
32 KiB blocks 10666 KB/s
64 KiB blocks 15515 KB/s
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
What a controller is told about a device before it will accept it. A DWC3
core validates these where QEMU's controller does not.
- HCCPARAMS1 says whether context structures are 32 or 64 bytes, and the
wider form was refused outright with -EIO; the EIC7700X reports
0x0220fe45 on both of its controllers, so this driver could not have
driven either. A wide context is the same fields with reserved space
after them, so only the stride changes. Read it at start up and use it
wherever a context array is walked.
- Contexts must be 64 byte aligned, since every device context base
address array entry points at one, and the output context came from
kmm_zalloc().
- The slot context never carried the device speed, which has no valid
zero, so a validating controller answers Address Device with a parameter
error. The speed was already implied by the endpoint context's maximum
packet size. The numbering is xHCI's own, hence the mapping.
- The output device context was cleared and never flushed. That context
is the controller's to write, so what stays behind is a dirty line of
zeros written back over the slot state, and the next command against the
slot is refused with a context state error. Enumeration reached
SET_ADDRESS and stopped.
- A buffer copied through an aligned stand-in was copied back using buflen,
which control transfers deliberately leave zero, so a descriptor read
copied nothing back and the caller was handed whatever its buffer held
before. Keep the requested length separately, and maintain the cache
over the whole stand-in rather than the part in use.
- A buffer the controller cannot reach is now copied through a stand-in
rather than refused. -EFAULT works for a caller with somewhere better
to put the data, and fails outright for one without: reading a block
device directly from a user program returned an error where the transfer
could have gone through a stand-in.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
The number of event ring segments a controller allows is a power of two
reported as its exponent, and the exponent can reach 15. Computing
1 << exponent into the uint8_t that holds it wraps to zero on any
controller offering more than 128 segments, and a controller told its
event ring table holds no entries has nowhere to report anything: every
command times out.
Work it out at full width and narrow afterwards.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
A failed command logged only its completion code. The difference between
a refused Address Device and a refused Evaluate Context is most of the
diagnosis, and the completion code does not give it.
Keep the command type before the result overwrites the TRB, and name it in
the message.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
The register dump read HCIVERSION with a 32-bit access at offset two. It
is a 16-bit register sharing a word with CAPLENGTH, so that is an
unaligned read of a device register: harmless where the bus permits it and
a fault where it does not.
Read the word once and take both fields from it.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
The controller moves every byte itself, so on a machine whose caches are
not coherent with it the driver must flush before the controller reads and
invalidate before the processor does. Data buffers got no maintenance at
all: nothing pushed before an OUT, nothing dropped after an IN.
Cache operations act a whole line at a time, which is unsafe for a buffer
that does not own its lines: invalidating drops whatever else shares the
line, and a writeback lands on top of what the controller has just put
there. Mass storage passes a 31 byte command block and a 13 byte status
out of its instance structure. Such a buffer is copied through an aligned
stand-in; anything large comes from a filesystem or from xhci_ioalloc(),
which now rounds its length up as well as aligning its start, so what it
returns owns its last line.
Whether the controller can reach a buffer at all is asked of the platform
through a new dmacapable operation, since it is a property of the system
the controller was fitted into rather than of the controller. A platform
that does not supply it is taken to accept every address, which is what
existing users have. A refused buffer gives -EFAULT, which the FAT
filesystem answers by retrying through its own DMA-safe sector buffer.
The device output context is also invalidated before the assigned address
is read out of it; the controller wrote that address, and reading without
invalidating returns whatever the processor had cached.
Compiles to nothing where there is no cache to maintain, and dmacapable is
NULL on PCI, so the existing user is unaffected.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
A Normal TRB describes one run of memory that may not cross a 64K
boundary, and the block layer hands down whole multi-sector reads whose
length is bounded by nothing here. One TRB was programmed regardless, so
a long enough transfer, or merely one starting near the wrong side of a
boundary, produced a descriptor the controller is entitled to reject or to
satisfy in part.
Program as many as the run needs, chained, asking for the completion
interrupt only on the last so one event still arrives for the transfer.
A transfer needing more TRBs than the ring holds is refused.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
xhci_ctrl_start() published the event ring segment table, the device
context base address array and the scratchpad pointers with
up_flush_dcache_all(), which an architecture whose cache can only be
maintained by address implements as a barrier and nothing more, so none of
them reached memory. The controller then reads whatever those addresses
held before, which presents as every command timing out with no events
arriving. Flush each structure by address.
xhci_ring_init() has the same fault from the other direction: it clears a
whole ring and flushes only the link entry it writes afterwards, leaving
the rest of the clearing in the cache. The controller writes into that
memory itself, so a line written back later lands on top of an event
somebody is waiting for. Flush the whole ring.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
The interval was left at its reset value of 4000, a millisecond, which is
how long the controller waits after an event before reporting it. Every
completion paid that, and mass storage spends three transfers on a
request.
Set it to 160, which is 40us, as Linux does. Zero puts no bound on how
often a controller may interrupt: a keyboard on an interrupt endpoint then
takes them continuously and occupies a processor.
Measured on a DWC3 with a USB 2.0 drive, doorbell to interrupt 986-1021us
before and 13-56us after:
reading 1MiB before after
512 byte blocks 166 KB/s 775 KB/s
32 KiB blocks 10666 KB/s 18618 KB/s
mounting a FAT32 volume: 92.7s before, 21.1s after
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
The handler read the status, queued the work that would answer it, and
returned with the source still asserted. On a level triggered line the
interrupt controller sees the condition still true and raises it again at
once, so the work that would have cleared it never runs.
Mask the interrupter in the handler and let the worker unmask when it is
done. The unmask clears the pending flag in the same write, because a
message is sent on that flag's clear to set transition and events that
arrived while the interrupter was masked have already set it.
Clearing opens its own window, so the worker drains the ring again after
unmasking and repeats while a drain finds anything; xhci_events_poll()
returns how many events it handled for that purpose. A drain that finds
nothing is the only state in which no event can have been lost.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
The event ring was acknowledged after being walked. An event arriving
during the walk sets the pending bit again, and clearing the bit
afterwards discards it. Transfers have no timeout, so the transfer that
event belonged to waits forever.
Acknowledge first. A spurious second pass over an empty ring costs
nothing.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
xhci_probe_ports() wrote PORTSC back to clear the change bits, including
PED, which is write-one-to-clear. A port that came up enabled, which is
what a device attached at power up produces, was switched off by the act
of reading it.
Mask PED out of the value written back. The port status worker already
does this.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
The handler defers to a worker that walks the event ring, and the ring is
not allocated until the controller is started, several steps later. A
controller left running by a boot loader has an interrupt pending as soon
as the line is enabled, so attaching earlier is a race with nothing able
to answer it.
Attach after the start, and clear USBSTS and the interrupter pending flag
once the handler is in place: a message signalled interrupt is sent on the
flag's clear to set transition, so a flag raised before the handler
existed would never produce another.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
Every other timer driver block in this Make.defs sets TMRDEPPATH and
TMRVPATH so DEPPATH/VPATH include this directory. CONFIG_PTP_CLOCK
and CONFIG_PTP_CLOCK_DUMMY were the only two missing it, leaving
ptp_clock.c/ptp_clock_dummy.c unreachable via VPATH and without a
generated dependency file when no other timer driver in this file is
also selected.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Daniel P. Carvalho <danieloak@gmail.com>
(cherry picked from commit 6d3812229d1c527971bf3a2b9d7d2675796fc688)
The tflm tool registered DEPTHWISE_CONV_2D in nuttx-apps#3773, but the
docs still listed eight operators. Document the unused -C compile path,
that the sim helper uses heap I/O, and the pinned TFLM/CMSIS/NNABLA
versions. Add missing gemmlowp, KissFFT, Ruy, and FlatBuffers pages,
document the AI-engine character driver, and wire it into CMake.
Signed-off-by: Abhishek Mishra <mishra.abhishek2808@gmail.com>
uart_writev() queues output one byte at a time via uart_putxmitchar().
Add uart_putxmitbuf() that memcpy()s a whole run into the TX ring buffer
and use it when no per-byte processing is needed (OPOST and ECHO clear,
not a console). On a full buffer fall back to uart_putxmitchar(), which
keeps the blocking and error handling unchanged.
8 MiB write() to /dev/ttyACM0 on nRF52840: 455 -> 573 KB/s.
Guarded by CONFIG_SERIAL_TXBULK, default !DEFAULT_SMALL.
Assisted-by: Claude Code
Signed-off-by: raiden00pl <raiden00@railab.me>
Add support for the Microchip TC74 digital temperature sensor using the
Sensor Driver Framework (uORB). The TC74 is an 8-bit I2C temperature
sensor with a measurement range from -40C to +125C and a resolution
of 1C.
The driver registers as a uORB topic (/dev/uorb/sensor_temp<n>) and
polls on the low-priority work queue. It supports dynamic interval
configuration and automatically enters low-power standby mode when
the topic is deactivated.
Validated against a real TC74A5-3.3 on a custom STM32H743BI board.
Assisted-by: Gemini:gemini-3.8-pro
Signed-off-by: Daniel P. Carvalho <danieloak@gmail.com>
mtdconfig_unregister_by_path() opened the device with file_open(),
which runs mtdconfig_open() and therefore holds dev->lock for the
whole lifetime of the temporary file reference. It then destroyed
the mutex and freed the private device structure while that
reference was still open, so the subsequent file_close() reached
mtdconfig_close(), which performs nxmutex_unlock() on freed memory.
Destroying a held mutex and unlocking it after free corrupt the heap;
on sim this crashes deterministically in the next allocation
(EXC_BAD_ACCESS in mm_malloc). Both file_close() and
unregister_driver() return values were also discarded and the
function unconditionally returned OK, masking legitimate errors.
Reorder the teardown to close -> unregister -> destroy/free and
propagate errors, so that:
- file_close() (driver close callback and inode release) runs while
the private device structure is still valid, releasing the
exclusive access taken by mtdconfig_open(),
- the private structure is destroyed and freed only after
unregister_driver() succeeds. On failure the inode (and with it
i_private) may still be referenced, so freeing would be wrong.
Returning the error also honors the documented API contract
(zero on success, negated errno on failure).
This matches the established close -> unregister -> teardown ordering
used by e.g. bchdev_unregister().
Verified with sim:configdata plus a register/unregister lifetime
exercise in examples/configdata: 934706/934706 checks pass with the
fix; with the fix stashed the same run dies with SIGSEGV right after
mtdconfig_unregister_by_path() returns.
Fixes: https://github.com/apache/nuttx/issues/20166
Signed-off-by: Arnav Sharma <2006arnavsharma@gmail.com>
sensor_poll() arms a per-subscriber watchdog for fetch()-only sensors
with a requested interval. The watchdog handler sensor_fetch_expired()
dereferences the subscriber and re-arms itself unless user->fds is NULL.
sensor_poll() teardown clears user->fds and cancels the watchdog, but
sensor_close() removed the subscriber from the user list and freed it
without doing either. A close() racing an armed timer therefore lets
the handler run after the subscriber is freed, causing a timer-context
use-after-free and re-arm of a freed watchdog.
Mirror the poll teardown in sensor_close(): clear user->fds and cancel
user->wdog under upper->lock before notifying other users and freeing
the subscriber.
Fixes#20145.
Signed-off-by: arnavsharma990 <2006arnavsharma@gmail.com>
Running `./tools/checkpatch.sh -g` on this file reported seven pre-existing
style errors. Fix them so that the file passes nxstyle cleanly:
- Add the missing blank line after the declarations in sensor_lock(),
sensor_unlock(), sensor_update_interval() and sensor_generate_timing().
- Indent the SNIOC_GET_EVENTS and SNIOC_FLUSH case labels with six spaces
like the other twelve case labels of the same switch.
- Drop the two extra spaces in front of the poll_notify() call in
sensor_poll().
Whitespace only, no functional change: the file is byte identical once all
whitespace is stripped.
Signed-off-by: likun17 <likun17@xiaomi.com>
Cover the remaining electrical quantities so that they do not have to fork
into driver private namespaces later. Add SENSOR_TYPE_RESISTANCE (Ohm),
SENSOR_TYPE_CONDUCTIVITY (S/m), SENSOR_TYPE_ENERGY (J) and
SENSOR_TYPE_CHARGE (C), the last two matching the native unit of the
accumulator registers in power and energy monitors.
Signed-off-by: likun17 <likun17@xiaomi.com>
uORB has no type for electrical quantities, so power monitors can only use
the legacy character drivers, which are deprecated and report their values
in three incompatible unit systems. Add SENSOR_TYPE_VOLTAGE,
SENSOR_TYPE_CURRENT and SENSOR_TYPE_POWER with their message structs in SI
units (V, A, W).
Signed-off-by: likun17 <likun17@xiaomi.com>
Same issue as the previous commit, on the receive side: uart_read()
advances recv.tail from thread context without holding the critical
section, but the recvbuf batch path of uart_recvchars() reads recv.tail
several times (the full check, the watermark count and the free-space
computation). If uart_read() moves and wraps the index in between, the
computed free space goes negative and is passed to recvbuf() as a huge
size_t, which lets the driver store past the end of the ring buffer.
Read recv.tail once per loop iteration and derive everything from that
snapshot. The consumer only ever moves the index forward, so a stale
snapshot merely stores less now.
Assisted-by: Claude Code
Signed-off-by: raiden00pl <raiden00@railab.me>
uart_putxmitchar() advances xmit.head from thread context without
holding the critical section, so on SMP the head index can move, and
wrap around, while uart_xmitchars() runs in the TX interrupt on another
CPU. Since commit b319c27f03 ("serial: Added APIs for receiving and
sending multiple chars") the sendbuf path of uart_xmitchars() reads
xmit.head twice: once to decide whether the pending data is contiguous
and again to compute its length. If the producer wraps the index in
between, the computed length goes negative, is passed to sendbuf() as a
huge size_t and the driver transmits memory far beyond the ring buffer.
The per-byte path reads the index only once and is not affected, which
is why this went unnoticed: the batch path is only used by drivers that
implement sendbuf, and the 16550 driver gained it in commit 45c38d8592
("drivers/serial/16550: add polling mode support for serial drivers").
qemu-intel64 with SMP is the first configuration combining a sendbuf
driver with a producer running on another CPU.
On qemu-intel64 SMP this shows up as an endless stream of NUL bytes on
the console (captured with gdb: head = 1, tail = 8, size = 16, and
u16550_sendbuf() called with size = (size_t)-7), which makes the ntfc
test harness fail every test that runs while the flood lasts.
Read the head index once per loop iteration and use that snapshot for
both the contiguity test and the length. The producer only ever moves
the index forward, so a stale snapshot merely sends less now.
Assisted-by: Claude Code
Signed-off-by: raiden00pl <raiden00@railab.me>
Commit 2a7cf05 added support for QSPI control but removed functions
gd25_purdid (leave power down state) and gd25_pd (enter power down).
It's likely ok to avoid putting the device in power down state after
every operation, but we need to wake it up from the power down state
before first accessing it.
Without the fix the flashes used with NuttX prior to 2a7cf05 commit
don't work anymore as they are in power down state. The fix ensures
we wake from this state during the initialization.
Also fixes various coding style errors.
Signed-off-by: Michal Lenc <michallenc@seznam.cz>
Fix checkpatch "Missing blank line after declarations" errors in
drivers/timers/arch_timer.c and sched/sched/sched_processtickless.c.
These are pre-existing issues, not introduced by the recent tickless
RR series.
Assisted-by: Zhipu GLM-5.3
Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
The mask computation introduced by "fix infinite loop in
up_timer_getmask when maxticks == CLOCK_MAX" has two problems:
1. If maxticks == 0, flsx(0) expands to __builtin_clz(0), which is
undefined behavior, and the shift count becomes 8 * sizeof(clock_t)
= 64 for a 64-bit clock_t, which is undefined behavior as well.
The loop-based code that was replaced kept *mask = 0 in this case.
2. CLOCK_MAX is INT64_MAX, i.e. 63 one bits, not a full-width bit
pattern. The resulting mask is always one bit narrower than the
one produced by the original loop; e.g. a 32-bit timer got
0x7fffffff instead of 0xffffffff, so counter deltas >= 2^31 were
truncated in the clock timekeeping code.
Fix this by keeping *mask = 0 when maxticks == 0 and by deriving the
mask from the full-width unsigned constant (uint64_t)-1, which
restores the all-ones semantics of the original loop and still covers
the maxticks == CLOCK_MAX case.
Also initialize maxticks in arch_timer.c: if the lower half does not
implement the maxtimeout ops, the value is left untouched and would
otherwise be read uninitialized.
Assisted-by: Zhipu GLM-5.3
Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
When maxticks equals CLOCK_MAX (all bits set), the loop that builds
the mask by (*mask << 1) | 1 never terminates because the shifted
value wraps around to the same mask value, making next > maxticks
always false.
Replace the loop with a single flsx-based expression that computes
the mask directly, which naturally covers the CLOCK_MAX case.
Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
Pre-existing violations in this file, reported by checkpatch because the
preceding commit touches it, and requested by a reviewer.
Three "Missing blank line after declarations", in the BIOC_PARTINFO,
MTDIOC_ERASESTATE and register-time-erase blocks.
The rest were one problem: the whole mtd_loop_ioctl() switch body sits two
columns short of NuttX style. With `switch` at 2 and its brace at 4, case
labels belong at 6 — as they already are in filemtd_ioctl() earlier in this
same file — but here the comments and cases sit at 4 and everything under
them follows suit, which checkpatch reports as 21 separate comment,
alignment and brace errors. Reindented the block to match, including the
two stray closing lines that had drifted to seven and five columns.
Whitespace only: `git diff -w` is empty apart from the three added blank
lines, and the brace count is unchanged. checkpatch is clean against
master.
Signed-off-by: AlmAck <gluca86@gmail.com>
filemtd_initialize() opens its backing file with
mode = O_RDONLY | O_WRONLY | O_CLOEXEC;
Commit 6161c73639 introduced this when it replaced the non-standard
O_RDOK | O_WROK pair, describing the change as a pure text substitution.
That held while the access mode was a genuine bitmask: O_RDONLY was
(1 << 0), O_WRONLY was (1 << 1), and O_RDWR was both bits, so the OR
produced O_RDWR. O_ACCMODE was defined as an alias for O_RDWR.
Commit 9e141acab3 then aligned the flags with Linux. The low two bits
became an enumeration -- O_RDONLY 0, O_WRONLY 1, O_RDWR 2 -- and
O_ACCMODE stopped being an alias for O_RDWR and became an independent
mask of 3. OR-ing two members of that enumeration is no longer
meaningful: O_RDONLY | O_WRONLY evaluates to 1, and masking it with
O_ACCMODE yields O_WRONLY.
The file is therefore opened write-only, and fs_read.c rejects every
read on it with -EACCES. The failure does not name filemtd: on the
simulator it surfaces as a LittleFS mount of a filemtd-backed partition
returning -ENOSPC, after which nothing on that volume works.
This appears to be an isolated miss rather than a pattern. Grepping the
tree for the same construct -- two access-mode constants OR-ed together
-- finds only this one call site. The one remaining place that
bit-tests the mode, fs/xipfs/xipfs_vfs.c:606, happens to be correct
under the new values (and would have been wrong under the old ones).
The hostfs NUTTX_O_* mirror and host_oflags_convert() were updated in
lockstep and switch on the masked value.
Signed-off-by: AlmAck <gluca86@gmail.com>
Adds a section covering CONFIG_SENSORS_LSM6DS3TRC_FIFO: what it changes
(one interrupt/I2C read per watermark instead of per sample) and its
three limitations while it's on -- both sub-sensors forced to the same
ODR, temperature no longer per-sample (one read per drain applied to
the whole batch), and both sub-sensors must stay physically enabled
regardless of subscription, since the chip's FIFO write trigger needs
both running -- plus the watermark/ORB-buffer-size relationship callers
need to respect.
Also documents a related chip quirk found during bench testing:
diff_words reads 0 at the exact moment a real FIFO overrun occurs, even
though the FIFO is still completely full of valid, retained data (a
forced read past that point recovers real samples, not garbage). This
isn't a bug in this driver: ST's own engineers confirm the same
behavior for this chip family on their community forum (thread
"LSM6DS3 FIFO status clarification", td-p/184022), and the mainline
Linux st_lsm6dsx driver doesn't attempt to recover from it either -- it
only special-cases an empty FIFO, not an overrun one. Documenting this
in a comment rather than adding recovery logic: with the small
watermark this driver uses, reaching a real overrun at all means the
drain has already fallen many seconds behind, and ST's own guidance for
this condition is to avoid it via watermark sizing rather than recover
from it.
Signed-off-by: Felipe Moura <moura.fmo@gmail.com>
Hardware testing (single-topic FIFO subscriptions, register dumps via
manual i2c commands) turned up two independent bugs that together
made the FIFO silently never accumulate data for a single-topic (e.g.
accel-only) subscription, while showing FIFO_STATUS2 stuck reporting
OVER_RUN and FIFO_FULL_SMART with a simultaneous zero DIFF_FIFO count:
- The LSM6DS3TR-C's FIFO write trigger (data-ready-based, the only mode
this driver uses) only fires while BOTH the accelerometer and the
gyroscope are physically running, regardless of which one(s) are
actually decimated into the FIFO pattern -- confirmed in the
datasheet's FIFO section ("the ODR must be lower than or equal to
both the accelerometer and gyroscope ODRs") and reproduced by
register-level testing with the unsubscribed sensor powered down vs.
powered up. lsm6ds3trc_fifo_configure() now forces whichever
sub-sensor isn't subscribed to run at the shared rate anyway (still
excluded from the pattern, so this costs no extra I2C bandwidth on
drain, just that sensor's own unavoidable power draw), and brings it
back down once neither sub-sensor is subscribed.
- Separately, lsm6ds3trc_fifo_configure() reconfigured decimation,
watermark and ODR while the FIFO was still running in Continuous
mode from a previous configuration. Reproduced manually: reconfiguring
live leaves FIFO_STATUS1/2 stuck reporting a stale diff count even
once the trigger fix above is in place; resetting through Bypass mode
first (which also empties the FIFO) before writing the new settings,
the same procedure the datasheet documents for changing FIFO
settings, and only re-entering Continuous mode last, is what actually
gets the diff counter to track correctly.
Also fixes two bit-definition bugs found while cross-referencing the
real ST datasheet instead of the in-tree lsm6dsl.h header used as a
starting point: MASK_FIFO_DIFF_HI was 4 bits (0x0f) instead of the
documented 3 (0x07), and MASK_DEC_FIFO_XL/SHIFT_DEC_FIFO_GY assumed
2-bit decimation fields instead of the documented 3-bit ones.
Verified on hardware: accel-only, gyro-only, and both-topics
subscriptions all now drain cleanly at the configured watermark with
no overrun, sustained over tens of seconds of continuous streaming.
Signed-off-by: Felipe Moura <moura.fmo@gmail.com>
Previously, activate() always fell back to a hardcoded ODR_52HZ (or the
other sub-sensor's rate, in FIFO mode) whenever a sub-sensor went from
disabled to enabled, discarding whatever rate the application had
explicitly requested via set_interval() before disabling it. An
application that only ever wants, say, 25Hz would see the sensor
restart at 52Hz on every re-activation, and in FIFO mode this fills the
FIFO faster than intended, defeating the point of choosing a lower ODR
for power savings.
Add last_odr, which -- unlike odr -- survives being disabled. activate()
now restores it on the next enable, only falling back to ODR_52HZ on a
sub-sensor's genuine first-ever activation.
Signed-off-by: Felipe Moura <moura.fmo@gmail.com>
Interrupt-driven mode so far pushes one uORB event per physical sample:
one I2C burst read and one interrupt per sample, at whatever ODR the
topic is running. That's the dominant power cost for a
battery-constrained use case sampling continuously -- draining the
chip's FIFO in batches cuts both by roughly the watermark size.
The LSM6DS3TR-C's FIFO is the older ST "pattern" style (no per-sample
tag byte, unlike LSM6DSO/ISM330): FIFO_CTRL3's per-sub-sensor decimation
bits choose which of gyro/accel feed the FIFO (0 = excluded, 1 = no
decimation -- only 0/1 are used here), FIFO_CTRL5 sets one shared
FIFO-only ODR, and FIFO_STATUS1/2's DIFF_FIFO reports how many 16-bit
words are waiting. Reused the existing INT1 wiring, but switched from
DRDY (per-sample) to FTH (threshold reached) when
CONFIG_SENSORS_LSM6DS3TRC_FIFO is on -- new bool that's a whole-driver
mode switch, not a per-instance choice, so a board doesn't change; only
its Kconfig does.
lsm6ds3trc_fifo_configure() re-derives and writes the decimation bits,
FIFO ODR, and watermark threshold (in words = watermark-in-samples *
words-per-pattern, 3 with one sub-sensor active or 6 with both) from
current dev->gyro/accel enabled+odr state; called from activate() and
set_interval(). Both sub-sensors are forced to the same ODR while FIFO
is on -- decimation factors > 1 for independent per-topic rates is real
complexity (matching ODR ratios to decimation values) left for later.
lsm6ds3trc_fifo_worker() replaces lsm6ds3trc_worker() under the Kconfig
guard: reads DIFF_FIFO, bursts that many words (rounded down to a whole
pattern chunk, capped at 2x the configured watermark so a late drain
doesn't overflow the read buffer -- whatever's left over just waits in
the chip's own FIFO for the next drain), then walks the buffer decoding
each chunk into a push_event() same as before. FIFO entries don't carry
their own timestamp, so each one is interpolated backwards from the
ISR's timestamp by the configured ODR interval. Temperature isn't part
of the FIFO pattern (FIFO_TEMP_EN stays off to keep the pattern width
simple); one direct OUT_TEMP_L read per drain is applied to the whole
batch instead.
Validated on the bench, both pattern widths: with both topics
subscribed (6-word pattern) and with only the accelerometer (3-word),
samples arrive in watermark-sized bursts with interpolated timestamps
spaced by the exact configured ODR interval (52Hz -> 19230us between
every consecutive sample, matched exactly), sane accel/gyro values, no
I2C errors, no overruns, sustained for 15+ seconds continuous.
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Felipe Moura <moura.fmo@gmail.com>
Two defects keep the driver from reporting touches on a board that
cannot use the interrupt line.
The register write built two messages joined by I2C_M_NOSTART. That
puts the same bytes on the wire as a single three byte message, but
resuming a transfer without a start condition is optional, and a
controller that does not implement it fails the transfer. On the
ESP32-P4 every write returned -ETIMEDOUT, so the buffer status clear
at 0x814E never reached the controller and gt9xx_read_touch_data()
returned an error for every read. Send the register address and the
value as a single message.
read() returned a full struct touch_sample_s even when the controller
reported no contact, with npoints set to zero. A caller that judges
the read by its return value takes that for valid data: the LVGL
touchscreen driver reads a second sample to decide whether to keep
reading, always gets one, so it sets continue_reading on every pass
and lv_indev_read() never returns. The display then stops refreshing
after the first frame while the touch reads spin. Return -EAGAIN when
there is no contact and the file was opened with O_NONBLOCK, which is
what the touchscreen upper half does in the same situation. A
blocking reader keeps the previous behaviour.
While here, add the blank line after the declaration in gt9xx_poll() that
nxstyle asks for. It predates this change, but the CI runs checkpatch over
every file a commit touches, so it has to go.
Signed-off-by: Jorge Guzman <jorge.gzm@gmail.com>
No driver exists for this exact chip. lsm6dsl.c is the closest
register-compatible match but is the deprecated legacy char-device
style; lsm6dso32_uorb.c is the closest uORB-style match but is for a
different chip variant. The new driver borrows lsm6dso32_uorb.c's
structure (dual sensor_lowerhalf_s, raw I2C_TRANSFER helpers) and
lsm6dsl.h's register map -- fixing a bug in the header it was ported
from along the way: LSM6DSL_FIFO_CTRL2_SHIFT is defined as 255 instead
of 0.
Delivery mode is chosen the same way mpu6050 does: kthread polling by
default, or interrupt-driven if the board supplies attach(). Unlike
the earlier lsm6dso32-style design this went through first -- one INT
pin and one activate()/interrupt path per sub-sensor -- the shipped
version uses a single shared INT pin for both, mirroring mpu6050's own
one-handler-one-worker design (#19601) instead. The two-independent-
paths version worked for accel alone but was intermittently broken for
gyro: activate() sometimes never actually turned CTRL2_G on even
though the interrupt-enable bit was written correctly, and other times
the whole console hung -- a real race, never conclusively root-caused
on a serial console with no JTAG available. The LSM6DS3TR-C supports
OR'ing both DRDY_XL and DRDY_G onto one pin via independent enable
bits in that pin's INTn_CTRL register, so there was no need for two
paths in the first place: one ISR times the burst, one HPWORK worker
reads OUT_TEMP_L..OUTZ_H_A (14 contiguous bytes covering temp, gyro
and accel in one I2C transaction) and pushes whichever topic(s) are
currently subscribed. activate() now just flips each sub-sensor's own
bit in the shared register instead of running its own attach.
On the XIAO ESP32-S3 with Seeed's IMU Breakout Board, INT1/INT2 route
to GPIO3/GPIO4 (confirmed from the breakout board's schematic, not
guessed). Only INT1/GPIO3 is wired up, since one pin is now enough;
GPIO4/INT2 is documented as available but unused.
Also: CTRL1_XL's FS_XL bits were never actually written to match the
driver's own software default (4g) -- registration set the in-memory
value but the chip stayed at its 2g reset default until a caller
issued an explicit SNIOC_SETFULLSCALE. register() now writes it.
Validated on the bench, both modes, reproduced across multiple fresh
reboots: WHO_AM_I reads 0x6a, sensor_accel0/sensor_gyro0 stream
continuously. Interrupt mode delivers ~300 samples of each per 6s
window with shared timestamps down to the microsecond between the two
topics per event, confirming both come from the same burst read.
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Felipe Moura <moura.fmo@gmail.com>
PCI endpoints cannot perform memory transactions when a bridge in
their hierarchy has memory forwarding or bus mastering disabled.
Firmware may leave these bits clear when handing a device over
after PXE boot.
Enable both bits on every parent bridge before enabling the
endpoint.
Signed-off-by: raiden00pl <raiden00@railab.me>
Assisted-by: Claude Code
The CH422G offers eight bi-directional pins, IO0-IO7, and four open-drain
outputs, OC0-OC3. It appears on boards that have run out of usable GPIOs
once a parallel RGB panel has taken its share, the Waveshare
ESP32-S3-Touch-LCD-7 among them, where it holds the panel and touch
controller in and out of reset and switches the backlight.
Two things about the device do not fit the shape a register-per-address
I2C driver usually takes, and both are handled here rather than pushed on
to board logic:
- A register is selected by the I2C address the transfer is addressed
to, not by a register address written ahead of the data. Each access
carries a single byte to one of four addresses.
- None of the write-only registers can be read back, so the driver
keeps a shadow copy of each and updates it in step with the device.
IO0-IO7 have no individual direction control; one bit of the system
parameter register drives the whole group. The driver records the
direction asked of each pin and puts the group in output mode once at
least one of them is an output, which is what a board that drives some of
the pins would expect. Reading a pin of a group held in output mode
reports the value last written, because the hardware cannot report the
level, and that is documented rather than hidden.
The four open-drain outputs are presented as pins 8-11 of the same
ioexpander_dev_s so that one instance covers the chip, which means
CONFIG_IOEXPANDER_NPINS must be at least 12.
Builds clean with no new warnings on esp32s3-touch-lcd7:usbnsh and passes
nxstyle.
Signed-off-by: Alan Carvalho de Assis <acassis@gmail.com>
Assisted-by: Claude Code
Rename atomic_fetch_add/sub/or/and/xor to atomic_add/sub/or/and/xor
to avoid conflicts with the C/C++ standard library naming. The
atomic_fetch_xxx naming is reserved by the standard; keeping it causes
function name conflicts when source files indirectly include both
<nuttx/atomic.h> and <atomic>/<stdatomic.h>.
Signed-off-by: zhangyu117 <zhangyu117@xiaomi.com>
