miquel.raynal@xxxxxxxxxxx wrote on Thu, 16 Dec 2021 10:01:47 +0100: > 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. Actually as the spi-mem.h header is not included in spi.h, it makes defining a static mem_caps entry harder. I'll go for another approach. Maybe putting the capabilities within the spi_controller_mem_ops structure, as these are highly related. > I'll send a proposal soon. > > Thanks, > Miquèl Thanks, Miquèl