On Thu, Nov 11, 2021 at 04:25:02PM +0000, Mark Brown wrote: > On Thu, Nov 11, 2021 at 07:06:27PM +0300, Serge Semin wrote: > > On Thu, Nov 11, 2021 at 03:14:26PM +0000, Mark Brown wrote: > > > > Given that people seem to frequently customise these IPs when > > > integrating them I wouldn't trust people not to have added some other > > > control into that reserved bit doing some magic stuff that's useful in > > > their system. > > > In that case the corresponding platform code would have needed to have > > that peculiarity properly handled and not to use a generic compatibles > > like "snps,dwc-ssi-1.01a" or "snps,dw-apb-ssi", which are supposed to > > be utilized for the default IP-core configs only. For the sake of the > > code simplification I'd stick to setting that flag for each generic > > DWC SSI-compatible device. That will be also helpful for DWC SSIs > > which for some reason have the slave-mode enabled by default. > > That's easier right up until the point where it explodes - I'd prefer to > be more conservative here. Fixing things up after the fact gets painful > when people end up only finding the bug in released kernels, especially > if it's distro end users or similar rather than developers. Since IP-core and components versions is now supported that will easy to implement. Thanks for merging the corresponding series in BTW. > > > Alternatively the driver could read the IP-core version from the > > DW_SPI_VERSION register, parse it (since it's in ASCII) and then use > > it in the conditional Master mode activation here. But that could have > > been a better solution in case if the older IP-cores would have used > > that bit for something special, while Nandhini claims it was reserved. > > So in this case I would stick with a simpler approach until we get to > > face any problem in this matter, especially seeing we already pocking > > the reserved bits of the CTRL0 register in this driver in the > > spi_hw_init() method when it comes to the DFS field width detection. > > If the device has a version register checking that seems ideal - the > infrastructure will most likely be useful in future anyway. A bit of a > shame that it's an ASCII string though. That's what the patchset has been implemented for in the first place https://lore.kernel.org/linux-spi/20211115181917.7521-1-Sergey.Semin@xxxxxxxxxxxxxxxxxxxx/ Nandhini, Mark has just merged in the series that adds the IP-core versions infrastructure support to the DW SSI driver. So now you can easily convert this patch to be using that new interface like this: - if (dws->caps & DW_SPI_CAP_KEEMBAY_MST) - cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST; + /* CTRLR0[31] MST */ + if (dw_spi_ver_is_ge(dws, HSSI, 102A)) + cr0 |= DWC_HSSI_CTRLR0_MST; Please don't forget to convert the DWC_SSI_CTRLR0_KEEMBAY_MST name to something like DWC_HSSI_CTRLR0_MST and place it at the top of the DWC HSSI CTRLR0 register macros list. -Sergey
