Re: [PATCH v3 3/5] spi: dw: Add support for master mode selection for DWC SSI controller
From: Serge Semin
Date: Tue Nov 16 2021 - 14:16:00 EST
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
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 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.