From: "Srikandan, Nandhini" <nandhini.srikandan@intel.com>
To: Serge Semin <fancer.lancer@gmail.com>
Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>,
"broonie@kernel.org" <broonie@kernel.org>,
"robh+dt@kernel.org" <robh+dt@kernel.org>,
"linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"mgross@linux.intel.com" <mgross@linux.intel.com>,
"Pan, Kris" <kris.pan@intel.com>,
"Demakkanavar, Kenchappa" <kenchappa.demakkanavar@intel.com>,
"Zhou, Furong" <furong.zhou@intel.com>,
"Sangannavar,
Mallikarjunappa" <mallikarjunappa.sangannavar@intel.com>,
"Vaidya, Mahesh R" <mahesh.r.vaidya@intel.com>,
"A, Rashmi" <rashmi.a@intel.com>
Subject: RE: [PATCH v4 3/3] spi: dw: Add support for master mode selection for DWC SSI controller
Date: Wed, 27 Apr 2022 09:51:47 +0000 [thread overview]
Message-ID: <SJ0PR11MB58160274EABEA39BFA625F5485FA9@SJ0PR11MB5816.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20220413130249.ueisqj5xs3komhmr@mobilestation>
> -----Original Message-----
> From: Serge Semin <fancer.lancer@gmail.com>
> Sent: Wednesday, April 13, 2022 6:33 PM
> To: Srikandan, Nandhini <nandhini.srikandan@intel.com>
> Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>; broonie@kernel.org;
> robh+dt@kernel.org; linux-spi@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org;
> mgross@linux.intel.com; Pan, Kris <kris.pan@intel.com>; Demakkanavar,
> Kenchappa <kenchappa.demakkanavar@intel.com>; Zhou, Furong
> <furong.zhou@intel.com>; Sangannavar, Mallikarjunappa
> <mallikarjunappa.sangannavar@intel.com>; Vaidya, Mahesh R
> <mahesh.r.vaidya@intel.com>; A, Rashmi <rashmi.a@intel.com>
> Subject: Re: [PATCH v4 3/3] spi: dw: Add support for master mode selection
> for DWC SSI controller
>
> Hello Nandhini
>
> AFAICS this patch should go before
> [PATCH v4 2/3] spi: dw: Add support for Intel Thunder Bay SPI controller
> Thus you'd perform the DWC AHB SSI Master mode conversion first, then
> introduce the new controller support. Otherwise without this patch applied
> the DW SPI driver is most likely left broken for the Intel SPI controllers since
> you drop the DW_SPI_CAP_KEEMBAY_MST macro usage in [PATCH 2/3] while
> the new DW AHB SSI Master functionality is introduced in the next patch
> [PATCH 3/3]. So please convert the series to the harmless configuration on
> each git image state.
>
Sure, I will reorder patch 2/3 and 3/3 so that the master mode conversion happens first followed by new controller support.
> On Tue, Mar 08, 2022 at 06:33:31PM +0800, nandhini.srikandan@intel.com
> wrote:
> > From: Nandhini Srikandan <nandhini.srikandan@intel.com>
> >
> > 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 <nandhini.srikandan@intel.com>
> > ---
> > drivers/spi/spi-dw-core.c | 4 ++--
> > drivers/spi/spi-dw.h | 7 +++----
> > 2 files changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> > index ecea471ff42c..68bfdf2c4dc7 100644
> > --- a/drivers/spi/spi-dw-core.c
> > +++ b/drivers/spi/spi-dw-core.c
> > @@ -307,8 +307,8 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws,
> struct spi_device *spi)
> > if (spi->mode & SPI_LOOP)
> > cr0 |= DW_HSSI_CTRLR0_SRL;
> >
>
> > - if (dws->caps & DW_SPI_CAP_KEEMBAY_MST)
> > - cr0 |= DW_HSSI_CTRLR0_KEEMBAY_MST;
> > + /* CTRLR0[31] MST */
> > + cr0 |= DW_HSSI_CTRLR0_MST;
>
> Could you please conditionally set that flag here? That's what we agreed to
> do in v3:
> https://lore.kernel.org/linux-
> spi/20211116191542.vc42cxvflzn66ien@mobilestation/
> like this:
> + /* CTRLR0[31] MST */
> + if (dw_spi_ver_is_ge(dws, HSSI, 102A))
> + cr0 |= DWC_HSSI_CTRLR0_MST;
>
In case of Keem Bay, though the version of SPI controller is shown as 1.01a from the HW register, it still needs the MST BIT31 to be set in order for controller to work in master mode.
Also since the older versions of the controller which do not need the BIT31 to be set, the bit was reserved. Hence there is no impact by setting this BIT31 for older versions.
So, the condition check was removed.
> > }
> >
> > return cr0;
> > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h index
> > d5ee5130601e..2583b7314c41 100644
> > --- a/drivers/spi/spi-dw.h
> > +++ b/drivers/spi/spi-dw.h
> > @@ -23,7 +23,7 @@
> > ((_dws)->ip == DW_ ## _ip ## _ID)
> >
>
> > #define __dw_spi_ver_cmp(_dws, _ip, _ver, _op) \
> > - (dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ver)
> > + (dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ ##
> _ver)
>
> Nice catch. My mistake. Could you please move this change into a dedicated
> patch with the next fixes tag?
> Fixes: 2cc8d9227bbb ("spi: dw: Introduce Synopsys IP-core versions
> interface")
>
Sure, I will convert this to a dedicated patch. Just for confirmation, the patch should be a separate patch with this title "Fixes: 2cc8d9227bbb ("spi: dw: Introduce Synopsys IP-core versions interface")"
and not part of the current patch set series.
> >
> > #define dw_spi_ver_is(_dws, _ip, _ver) __dw_spi_ver_cmp(_dws, _ip,
> > _ver, ==)
> >
> > @@ -31,8 +31,7 @@
> >
> > /* DW SPI controller capabilities */
> > #define DW_SPI_CAP_CS_OVERRIDE BIT(0)
> > -#define DW_SPI_CAP_KEEMBAY_MST BIT(1)
> > -#define DW_SPI_CAP_DFS32 BIT(2)
> > +#define DW_SPI_CAP_DFS32 BIT(1)
> >
> > /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */
> > #define DW_SPI_CTRLR0 0x00
> > @@ -100,7 +99,7 @@
>
> > * 0: SSI is slave
> > * 1: SSI is master
> > */
> > -#define DW_HSSI_CTRLR0_KEEMBAY_MST BIT(31)
> > +#define DW_HSSI_CTRLR0_MST BIT(31)
>
> Could you please drop the redundant comment above and join the macro
> with the DW_HSSI_* macros group?
>
Sure, I will remove the comment and group the macros.
> -Sergey
>
> >
> > /* Bit fields in CTRLR1 */
> > #define DW_SPI_NDF_MASK GENMASK(15, 0)
> > --
> > 2.17.1
> >
next prev parent reply other threads:[~2022-04-27 11:05 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-08 10:33 [PATCH v4 0/3] Add support for Intel Thunder Bay SPI controller nandhini.srikandan
2022-03-08 10:33 ` [PATCH v4 1/3] dt-bindings: spi: Add bindings for Intel Thunder Bay SoC nandhini.srikandan
2022-03-09 14:01 ` Rob Herring
2022-03-08 10:33 ` [PATCH v4 2/3] spi: dw: Add support for Intel Thunder Bay SPI controller nandhini.srikandan
2022-04-13 13:05 ` Serge Semin
2022-03-08 10:33 ` [PATCH v4 3/3] spi: dw: Add support for master mode selection for DWC SSI controller nandhini.srikandan
2022-04-13 13:02 ` Serge Semin
2022-04-27 9:51 ` Srikandan, Nandhini [this message]
2022-04-28 14:35 ` Serge Semin
2022-05-02 8:51 ` Srikandan, Nandhini
2022-05-04 10:33 ` Serge Semin
2022-04-04 11:51 ` [PATCH v4 0/3] Add support for Intel Thunder Bay SPI controller Srikandan, Nandhini
2022-04-04 12:00 ` Serge Semin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=SJ0PR11MB58160274EABEA39BFA625F5485FA9@SJ0PR11MB5816.namprd11.prod.outlook.com \
--to=nandhini.srikandan@intel.com \
--cc=Sergey.Semin@baikalelectronics.ru \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=fancer.lancer@gmail.com \
--cc=furong.zhou@intel.com \
--cc=kenchappa.demakkanavar@intel.com \
--cc=kris.pan@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-spi@vger.kernel.org \
--cc=mahesh.r.vaidya@intel.com \
--cc=mallikarjunappa.sangannavar@intel.com \
--cc=mgross@linux.intel.com \
--cc=rashmi.a@intel.com \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).