Re: [PATCH RFC 1/7] spi: Add 'rx_sampling_delay_ns' parameter for clock to RX delay

Mark Brown
2026-03-05T22:14:28Z

[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]

On Tue, Mar 03, 2026 at 04:01:14PM -0500, Frank Li wrote:
> From: Frank Li (AI-BOT) <frank.li@nxp.com>

Please add a human step before sending these reviews.

> > + * @rx_sampling_delay_ns: spi clk to spi rx data delay

> Comment is too terse. Expand to clarify the unit and purpose:
> "clock-to-RX-data delay in nanoseconds; see tCLQV in device datasheets"

I'm not sure restating the unit in the comment as well as the name is
super useful...

> > + /* Transfer characteristics */
> > + u32 rx_sampling_delay_ns; /* clk to rx data delay */

> The inline comment duplicates the field name. Remove it or expand the
> block comment above to explain when/how drivers should use this field.
> Also: is u32 the right type? Consider if negative values or larger
> ranges are possible, or if this should be `struct spi_delay` for
> consistency with cs_setup, cs_hold, cs_inactive above.

...especially given the contradictory feedback here, and the comment
about a negative delay is obviously not appropriate. I'm not convinced
anyone needs an explanation of what to do with the field either.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]