Hi Mark, broonie@xxxxxxxxxx wrote on Wed, 15 Dec 2021 19:19:11 +0000: > On Wed, Dec 15, 2021 at 08:05:48PM +0100, Boris Brezillon wrote: > > > There's also a second option that doesn't involve patching existing > > users: add a spi_mem_controller_caps to the spi_controller struct, and > > check this instance in your spi_mem_default_supports_op() > > implementation. Note that the buswidth check done in the generic > > helper is already based on caps exposed by the controller > > through spi_controller.mode_bits ({RX/TX}_{DUAL,QUAD,OCTAL} bits). > > This approach is quite nice for things like this - having things as data > rather than code. The only issue is if any of the caps end up varying > by operation and we need different capabilities but that doesn't look > too likely here I think? Indeed that was the main point of the original review from Boris, the capabilities should be fixed on the controller's lifetime. So I believe we are safe. I think I am going to propose the following: const struct spi_controller_mem_ops *mem_ops; + struct spi_controller_mem_caps mem_caps; As the structure is not supposed to enlarge dramatically in the near future, I guess it's fine to have it defined statically. Please tell me if you prefer a *mem_caps pointer. I'll send a proposal soon. Thanks, Miquèl