include: spi.h: Add native HW timing parameters to config API struct

[decsny]

Expand the struct spi_cs_control to be able to specify hardware CS
timing parameters in addition to GPIO CS, without adding memory usage
and without breaking existing API usage.

Also add a property for delay between data frames to struct spi_config.

The purpose of these changes is to be able to better support hardware
which can configure specific fine tuned delays in the nanosecond range,
which is the range that most spi device datasheets are specified in.
Microseconds chip select control is not that useful except for the slow
control caused by using a GPIO.

The expectation is that this will be used by specifying peripheral
timing parameters for the devices on the spi bus in DT, and then those
drivers can call into the spi bus driver API by specifying these parameters.

Support these parameters in NXP LPSPI driver

If the PR approach is okay, then TODO on this PR:

• deprecate LPSPI timing DT props
• add timing parameters to spi-device.yaml generic binding
• edit SPI_DT_SPEC_GET macros to use new timing parameters for hw CS if no cs-gpios on the node.
• example of new API struct parameters and new DT spi-device props with MCXW71 platform DT and edit spi nor flash driver

[tbursztyka]

This needs further thoughts.

For instance, you set the size of the ns delay to uint8_t, will that be sufficient in all cases?

Another issue as well: how device drivers are going to know which from the delay to the anonymous struct to use? there is no flag anywhere for mitigation. Unless we want to move to a new delay system, which case we will need to make delay attribute obsolete etc...

a few additional comments inline

[tbursztyka]

Perhaps it's time to rename this to spi_cs_and_delay_control

[decsny]

renaming would be an API change instead of an extension though, are you sure it's worth doing that, as opposed to just having a couple extra bytes in the overall spi_config struct (whether the way I have in this PR or introduce a new spi_timing struct)

[tbursztyka]

If we need an API change (whatever that could be), so be it. Better having a sane API rather than a half-hacked one.

[tbursztyka]

and this could go into the anonymous struct you added, in the union.

That way: to change in structure size.

That said, this is very much spi controller specific. Some controller do not offer anything to configure this.
And 0 as 50% of the frequency, why? why not just "hardware default"?

[decsny]

I thought about this too but since the struct was called spi_cs_control and this was not related I thought it would be confusing

the reason for 50% of period is because of the same reason that is motivating this PR, which is that the hardware default for the NXP spi is the shortest possible, which causes problems and looks strange, and people often report it as a bug because the users actually expect that the behavior will be 50% of period by default, and don't even think about configuring it.

And this expectation is I think , not specific to people using the NXP part. A 50% delay of clk period between words means that the clock signal does not have any blips between words. Also this makes perhaps applications more portable because the hardware default can be totally different between parts but like I said maybe they just always expect 50% of period like has been reported to me

[tbursztyka]

this was not only meant for gpio cs actually. thus why the structure was called spi_cs_control, and not spi_gpio_cs_control (for instance)

[decsny]

yeah, that was convenient that it was already named generically, I noticed

[decsny]

This needs further thoughts.

For instance, you set the size of the ns delay to uint8_t, will that be sufficient in all cases?

I think almost all data sheets I have seen have CS delay parameters of < 10 ns, but if someone knows about some relevant hardware that is going to need delays of over 1/4 of a microsecond then I am open to putting them at uint16_t. I thought about doing this but the reason I put at uint8_t is because there are also some other CS related timing parameters that people might want to add in the future. But maybe not since I think many of them such as like pcs-pcs delay are not relevant to zephyr API since we expect CS asserted through whole spi_buf_set.

Another issue as well: how device drivers are going to know which from the delay to the anonymous struct to use? there is no flag anywhere for mitigation. Unless we want to move to a new delay system, which case we will need to make delay attribute obsolete etc...

Actually, this is not an issue, and I did think it out, but does require a change on this PR, which is on my TODO list in the PR message, maybe I didn't explain it well or should have finished the work before submitting the PR, but I'm just low on bandwidth so wanted the idea reviewed before I did it.

Basically this information about the chip select pretty much always comes from DT already right now, for GPIO CS. And there is no source of information for native CS. GPIO cs information comes from the cs-gpios property on the DT node and is initialized by the SPI_DT_SPEC_GET macro. That macro can just be edited to use initialize the struct using the new spi timing properties on the peripheral nodes if they are there and there is no cs-gpios, to initialize the struct to be gpio port as null and use the hw timing members of the union

[decsny]

I forgot to put DNM since this PR is really only a WIP draft but I opened it as non-draft to get feedback on the idea from relevant reviewers

[tbursztyka]

Basically this information about the chip select pretty much always comes from DT already right now, for GPIO CS. And there is no source of information for native CS.

The source of native CS is reg in spi-device.yaml. Not very much intuitive I agree as is also selects the right slot of cs-gpios when this one is present.

[decsny]

Basically this information about the chip select pretty much always comes from DT already right now, for GPIO CS. And there is no source of information for native CS.

The source of native CS is reg in spi-device.yaml. Not very much intuitive I agree as is also selects the right slot of cs-gpios when this one is present.

I'm not seeing anything there right now, which property are you talking about? To be clear I am saying I will add it on this PR if we agree about this. Right now I just see spi-cpol, spi-cpha, spi-hold-cs,frame-format,spi-max-frequency, and duplex on the spi-device.yaml. Maybe you are thinking about the spi-controller.yaml ?

To be clear, the main point of what I am saying here is that the spi clock timing parameters should be defined on the device/slave/peripheral DT node, and not the controller/master node. Right now we have at least spi-max-frequency on the SPI device nodes, but we also should put these other timing parameters which would come from the datasheets of spi devices to describe the digital AC characteristics of the signals required to interface properly to the device.

Now as for whether to use a gpio or not for CS, that should remain on the controller node