[v3,3/5] spi: dw: Add support for master mode selection for DWC SSI controller

Commit Message

Srikandan, Nandhini Nov. 11, 2021, 6:51 a.m. UTC

From: Nandhini Srikandan <[e-mail]>

Add support to select the controller mode as master mode by setting
Bit 31 of CTRLR0 register. This feature is supported for controller
versions above v1.02.

Signed-off-by: Nandhini Srikandan <[e-mail]>
---
 drivers/spi/spi-dw-core.c | 4 ++--
 drivers/spi/spi-dw.h      | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Mark Brown Nov. 11, 2021, 2:16 p.m. UTC | #1

On Thu, Nov 11, 2021 at 02:51:59PM +0800, [e-mail] wrote:

> Add support to select the controller mode as master mode by setting
> Bit 31 of CTRLR0 register. This feature is supported for controller
> versions above v1.02.

Clearly older versions of the controller can also run in this mode...

> -		if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> -			cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
> +		/* CTRLR0[31] MST */
> +		cr0 |= DWC_SSI_CTRLR0_MST;

This makes the configuration unconditional, it's not gated by controller
version checks or any kind of quirk any more meaning that if anything
interprets that bit differently things might break.  If this is really
required to put the controller in master mode it seems that either the
1.02 version is not widespread or this is generally the hardware
default.

Serge Semin Nov. 11, 2021, 2:52 p.m. UTC | #2

Hello Mark,

On Thu, Nov 11, 2021 at 02:16:05PM +0000, Mark Brown wrote:
> On Thu, Nov 11, 2021 at 02:51:59PM +0800, [e-mail] wrote:
> 
> > Add support to select the controller mode as master mode by setting
> > Bit 31 of CTRLR0 register. This feature is supported for controller
> > versions above v1.02.
> 

> Clearly older versions of the controller can also run in this mode...

Yes, but the driver doesn't support the slave mode at the moment.
So always enabling the master mode seems natural. (see my next comment
also concerning this matter)

> 
> > -		if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> > -			cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
> > +		/* CTRLR0[31] MST */
> > +		cr0 |= DWC_SSI_CTRLR0_MST;
> 

> This makes the configuration unconditional, it's not gated by controller
> version checks or any kind of quirk any more meaning that if anything
> interprets that bit differently things might break.  If this is really
> required to put the controller in master mode it seems that either the
> 1.02 version is not widespread or this is generally the hardware
> default.

We have already discussed this feature in v2:
https://patchwork.kernel.org/project/spi-devel-general/patch/[e-mail]/
Since that bit has been reserved before 1.02a but is no available for
any DWC SSI controller and the driver doesn't support the SPI-slave mode
at the moment I suggested to just always set that flag for the DWC SSI
code. Please see my reply to Nandhini here:
https://patchwork.kernel.org/project/spi-devel-general/patch/[e-mail]/#24433679

-Sergey

Mark Brown Nov. 11, 2021, 3:14 p.m. UTC | #3

On Thu, Nov 11, 2021 at 05:52:46PM +0300, Serge Semin wrote:
> On Thu, Nov 11, 2021 at 02:16:05PM +0000, Mark Brown wrote:
> > On Thu, Nov 11, 2021 at 02:51:59PM +0800, [e-mail] wrote:

> > > Add support to select the controller mode as master mode by setting
> > > Bit 31 of CTRLR0 register. This feature is supported for controller
> > > versions above v1.02.

> > Clearly older versions of the controller can also run in this mode...

> Yes, but the driver doesn't support the slave mode at the moment.
> So always enabling the master mode seems natural. (see my next comment
> also concerning this matter)

The commit message makes it sound like master mode is only supported for
the newer versions.

> > This makes the configuration unconditional, it's not gated by controller
> > version checks or any kind of quirk any more meaning that if anything

> We have already discussed this feature in v2:
> https://patchwork.kernel.org/project/spi-devel-general/patch/[e-mail]/
> Since that bit has been reserved before 1.02a but is no available for
> any DWC SSI controller and the driver doesn't support the SPI-slave mode
> at the moment I suggested to just always set that flag for the DWC SSI
> code. Please see my reply to Nandhini here:

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.

Serge Semin Nov. 11, 2021, 4:06 p.m. UTC | #4

On Thu, Nov 11, 2021 at 03:14:26PM +0000, Mark Brown wrote:
> On Thu, Nov 11, 2021 at 05:52:46PM +0300, Serge Semin wrote:
> > On Thu, Nov 11, 2021 at 02:16:05PM +0000, Mark Brown wrote:
> > > On Thu, Nov 11, 2021 at 02:51:59PM +0800, [e-mail] wrote:
> 
> > > > Add support to select the controller mode as master mode by setting
> > > > Bit 31 of CTRLR0 register. This feature is supported for controller
> > > > versions above v1.02.
> 
> > > Clearly older versions of the controller can also run in this mode...
> 
> > Yes, but the driver doesn't support the slave mode at the moment.
> > So always enabling the master mode seems natural. (see my next comment
> > also concerning this matter)
> 

> The commit message makes it sound like master mode is only supported for
> the newer versions.

I meant it doesn't really matter if the bit has been reserved before
and the driver doesn't support the Slave-mode of the controller
anyway.
Regarding the Master-mode feature availability. Originally Wan added
that flag setting for v1.01a here:
https://patchwork.kernel.org/project/spi-devel-general/patch/[e-mail]/
Nandhini said in v2 that both Keem Bay and Thunder Bay uses DWC SSI
v1.02a and the BIT[31] functionality is not Intel-specific, but
generic for DWC SSIs.  So version-wise it's either Wan or Nandhini
ware mistaken at some point.

> 
> > > This makes the configuration unconditional, it's not gated by controller
> > > version checks or any kind of quirk any more meaning that if anything
> 
> > We have already discussed this feature in v2:
> > https://patchwork.kernel.org/project/spi-devel-general/patch/[e-mail]/
> > Since that bit has been reserved before 1.02a but is no available for
> > any DWC SSI controller and the driver doesn't support the SPI-slave mode
> > at the moment I suggested to just always set that flag for the DWC SSI
> > code. Please see my reply to Nandhini here:
> 

> 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.

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.

-Sergey

Mark Brown Nov. 11, 2021, 4:25 p.m. UTC | #5

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.

> 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.

Serge Semin Nov. 16, 2021, 7:15 p.m. UTC | #6

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/[e-mail]/

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

Srikandan, Nandhini Nov. 17, 2021, 11:59 a.m. UTC | #7

> -----Original Message-----
> From: Serge Semin <[e-mail]>
> Sent: Wednesday, November 17, 2021 12:46 AM
> To: Mark Brown <[e-mail]>; Srikandan, Nandhini
> <[e-mail]>
> Cc: Serge Semin <[e-mail]>; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; Pan, Kris
> <[e-mail]>; Demakkanavar, Kenchappa
> <[e-mail]>; Zhou, Furong
> <[e-mail]>; Sangannavar, Mallikarjunappa
> <[e-mail]>; Vaidya, Mahesh R
> <[e-mail]>; A, Rashmi <[e-mail]>
> Subject: Re: [PATCH v3 3/5] spi: dw: Add support for master mode selection
> for DWC SSI controller
> 
> 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-
> [e-mail]/
> 
> 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
Sure, I will test this patch set on our hardware and then use the IP-core version infrastructure support 
and make the changes as mentioned for MST bit.

- Nandhini

Srikandan, Nandhini Dec. 8, 2021, 11:03 a.m. UTC | #8

> -----Original Message-----
> From: Srikandan, Nandhini
> Sent: Wednesday, November 17, 2021 5:29 PM
> To: Serge Semin <[e-mail]>; Mark Brown
> <[e-mail]>
> Cc: Serge Semin <[e-mail]>; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; Pan, Kris
> <[e-mail]>; Demakkanavar, Kenchappa
> <[e-mail]>; Zhou, Furong
> <[e-mail]>; Sangannavar, Mallikarjunappa
> <[e-mail]>; Vaidya, Mahesh R
> <[e-mail]>; A, Rashmi <[e-mail]>
> Subject: RE: [PATCH v3 3/5] spi: dw: Add support for master mode selection
> for DWC SSI controller
> 
> 
> 
> > -----Original Message-----
> > From: Serge Semin <[e-mail]>
> > Sent: Wednesday, November 17, 2021 12:46 AM
> > To: Mark Brown <[e-mail]>; Srikandan, Nandhini
> > <[e-mail]>
> > Cc: Serge Semin <[e-mail]>;
> > [e-mail]; [e-mail];
> > [e-mail]; [e-mail];
> > [e-mail]; Pan, Kris <[e-mail]>; Demakkanavar,
> > Kenchappa <[e-mail]>; Zhou, Furong
> > <[e-mail]>; Sangannavar, Mallikarjunappa
> > <[e-mail]>; Vaidya, Mahesh R
> > <[e-mail]>; A, Rashmi <[e-mail]>
> > Subject: Re: [PATCH v3 3/5] spi: dw: Add support for master mode
> > selection for DWC SSI controller
> >
> > 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-
> > [e-mail]/
> >
> > 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
> Sure, I will test this patch set on our hardware and then use the IP-core
> version infrastructure support and make the changes as mentioned for MST
> bit.
> 
> - Nandhini
I have tested the patches on our hardware and it is working fine. When would these patches be available in mainline? So that I can start applying the changes on top of these patches and share for review.
- Nandhini

Serge Semin Dec. 9, 2021, 8:40 a.m. UTC | #9

On Wed, Dec 08, 2021 at 11:03:03AM +0000, Srikandan, Nandhini wrote:
> 
> 
> > -----Original Message-----
> > From: Srikandan, Nandhini
> > Sent: Wednesday, November 17, 2021 5:29 PM
> > To: Serge Semin <[e-mail]>; Mark Brown
> > <[e-mail]>
> > Cc: Serge Semin <[e-mail]>; [e-mail];
> > [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; Pan, Kris
> > <[e-mail]>; Demakkanavar, Kenchappa
> > <[e-mail]>; Zhou, Furong
> > <[e-mail]>; Sangannavar, Mallikarjunappa
> > <[e-mail]>; Vaidya, Mahesh R
> > <[e-mail]>; A, Rashmi <[e-mail]>
> > Subject: RE: [PATCH v3 3/5] spi: dw: Add support for master mode selection
> > for DWC SSI controller
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Serge Semin <[e-mail]>
> > > Sent: Wednesday, November 17, 2021 12:46 AM
> > > To: Mark Brown <[e-mail]>; Srikandan, Nandhini
> > > <[e-mail]>
> > > Cc: Serge Semin <[e-mail]>;
> > > [e-mail]; [e-mail];
> > > [e-mail]; [e-mail];
> > > [e-mail]; Pan, Kris <[e-mail]>; Demakkanavar,
> > > Kenchappa <[e-mail]>; Zhou, Furong
> > > <[e-mail]>; Sangannavar, Mallikarjunappa
> > > <[e-mail]>; Vaidya, Mahesh R
> > > <[e-mail]>; A, Rashmi <[e-mail]>
> > > Subject: Re: [PATCH v3 3/5] spi: dw: Add support for master mode
> > > selection for DWC SSI controller
> > >
> > > 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-
> > > [e-mail]/
> > >
> > > 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
> > Sure, I will test this patch set on our hardware and then use the IP-core
> > version infrastructure support and make the changes as mentioned for MST
> > bit.
> > 
> > - Nandhini

> I have tested the patches on our hardware and it is working fine. When would these patches be available in mainline? So that I can start applying the changes on top of these patches and share for review.
> - Nandhini 

These changes have been merged into the Mark' repo in the spi-next
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/log/?h=for-next
The series together with the rest of the SPI-related changed will get
into the mainline during the merge window.
For the time being you can rebase your patches on top of the Mark' repo
and if it works well for you just post it out for review. If it looks
good you'll even be able to have your series merged into the kernel in
the next merge-window.

-Sergey

>

Patch

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index bfa075a4f779..26d0ef87661d 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -308,8 +308,8 @@  static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
 		/* CTRLR0[14] Slave Select Toggle Enable */
 		cr0 |= chip->sste << DWC_SSI_CTRLR0_SSTE_OFFSET;
 
-		if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
-			cr0 |= DWC_SSI_CTRLR0_KEEMBAY_MST;
+		/* CTRLR0[31] MST */
+		cr0 |= DWC_SSI_CTRLR0_MST;
 	}
 
 	return cr0;
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 2ee3f839de39..d67f62ff79c9 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -78,11 +78,11 @@ 
 #define DWC_SSI_CTRLR0_DFS_OFFSET	0
 
 /*
- * For Keem Bay, CTRLR0[31] is used to select controller mode.
+ * CTRLR0[31] is used to select controller mode.
  * 0: SSI is slave
  * 1: SSI is master
  */
-#define DWC_SSI_CTRLR0_KEEMBAY_MST	BIT(31)
+#define DWC_SSI_CTRLR0_MST		BIT(31)
 
 /* Bit fields in CTRLR1 */
 #define SPI_NDF_MASK			GENMASK(15, 0)