| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 29 Apr 2024 15:10:34 +0000
From: Witold Sadowski <wsadowski@...vell.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
"broonie@...nel.org"
<broonie@...nel.org>
CC: "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"jpawar@...ence.com" <jpawar@...ence.com>,
"pthombar@...ence.com"
<pthombar@...ence.com>,
"konrad@...ence.com" <konrad@...ence.com>,
Wojciech
Bartczak <wbartczak@...vell.com>,
Wojciech Zmuda <wzmuda@...vell.com>
Subject: RE: [EXT] Re: [PATCH 6/7] spi: cadence: Add Marvell IP modification
changes
> ----------------------------------------------------------------------
> On 19/12/2022 15:42, Witold Sadowski wrote:
> > Add support for Marvell IP modification - clock divider, and PHY
> > config, and IRQ clearing.
> > Clock divider block is build into Cadence XSPI controller and is
> > connected directly to 800MHz clock.
> > As PHY config is not set directly in IP block, driver can load custom
> > PHY configuration values.
> > To correctly clear interrupt in Marvell implementation MSI-X must be
> > cleared too.
> >
> > Signed-off-by: Witold Sadowski <wsadowski@...vell.com>
> > Reviewed-by: Chandrakala Chavva <cchavva@...vell.com>
> > Tested-by: Sunil Kovvuri Goutham <sgoutham@...vell.com>
> > ---
> > drivers/spi/Kconfig | 12 +++
> > drivers/spi/spi-cadence-xspi.c | 172
> > ++++++++++++++++++++++++++++++++-
> > 2 files changed, 183 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index
> > 3b1c0878bb85..42af5943d00a 100644
> > --- a/drivers/spi/Kconfig
> > +++ b/drivers/spi/Kconfig
> > @@ -251,6 +251,18 @@ config SPI_CADENCE_XSPI
> > device with a Cadence XSPI controller and want to access the
> > Flash as an MTD device.
> >
> > +config SPI_CADENCE_MRVL_XSPI
> > + tristate "Marvell mods for XSPI controller"
> > + depends on SPI_CADENCE_XSPI
> > +
> > + help
> > + Enable support for Marvell XSPI modifications
> > +
> > + During implementation of Cadence XSPI core Marvell
> > + has added some additional features like clock divider,
> > + PHY config support or non-memory SPI capabilities.
> > + Enable that option if you want to enable these features.
> > +
> > config SPI_CLPS711X
> > tristate "CLPS711X host SPI controller"
> > depends on ARCH_CLPS711X || COMPILE_TEST diff --git
> > a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
> > index 719c2f3b4771..c73faf6b0546 100644
> > --- a/drivers/spi/spi-cadence-xspi.c
> > +++ b/drivers/spi/spi-cadence-xspi.c
> > @@ -193,6 +193,46 @@
> > #define CDNS_XSPI_POLL_TIMEOUT_US 1000
> > #define CDNS_XSPI_POLL_DELAY_US 10
> >
> > +#if IS_ENABLED(CONFIG_SPI_CADENCE_MRVL_XSPI)
> > +/* clock config register */
> > +#define CDNS_XSPI_CLK_CTRL_AUX_REG 0x2020
> > +#define CDNS_XSPI_CLK_ENABLE BIT(0)
> > +#define CDNS_XSPI_CLK_DIV GENMASK(4, 1)
> > +
> > +/* Clock macros */
> > +#define CDNS_XSPI_CLOCK_IO_HZ 800000000 #define
> > +CDNS_XSPI_CLOCK_DIVIDED(div) ((CDNS_XSPI_CLOCK_IO_HZ) / (div))
> > +
> > +/*PHY default values*/
>
> Keep consistent style.
>
> > +#define REGS_DLL_PHY_CTRL 0x00000707
> > +#define CTB_RFILE_PHY_CTRL 0x00004000
> > +#define RFILE_PHY_TSEL 0x00000000
> > +#define RFILE_PHY_DQ_TIMING 0x00000101
> > +#define RFILE_PHY_DQS_TIMING 0x00700404
> > +#define RFILE_PHY_GATE_LPBK_CTRL 0x00200030
> > +#define RFILE_PHY_DLL_MASTER_CTRL 0x00800000
> > +#define RFILE_PHY_DLL_SLAVE_CTRL 0x0000ff01
> > +
> > +/*PHY config rtegisters*/
> > +#define CDNS_XSPI_RF_MINICTRL_REGS_DLL_PHY_CTRL 0x1034
> > +#define CDNS_XSPI_PHY_CTB_RFILE_PHY_CTRL 0x0080
> > +#define CDNS_XSPI_PHY_CTB_RFILE_PHY_TSEL 0x0084
> > +#define CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DQ_TIMING 0x0000
> > +#define CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DQS_TIMING 0x0004
> > +#define CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_GATE_LPBK_CTRL 0x0008
> > +#define CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DLL_MASTER_CTRL 0x000c
> > +#define CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DLL_SLAVE_CTRL 0x0010
> > +#define CDNS_XSPI_DATASLICE_RFILE_PHY_DLL_OBS_REG_0 0x001c
> > +
> > +#define CDNS_XSPI_DLL_RST_N BIT(24)
> > +#define CDNS_XSPI_DLL_LOCK BIT(0)
> > +
> > +/* MSI-X clear interrupt register */
> > +#define CDNS_XSPI_SPIX_INTR_AUX 0x2000
> > +#define CDNS_MSIX_CLEAR_IRQ 0x01
> > +
> > +#endif
> > +
> > enum cdns_xspi_stig_instr_type {
> > CDNS_XSPI_STIG_INSTR_TYPE_0,
> > CDNS_XSPI_STIG_INSTR_TYPE_1,
> > @@ -238,6 +278,106 @@ struct cdns_xspi_dev {
> > enum cdns_xspi_sdma_size read_size;
> > };
> >
> > +#if IS_ENABLED(CONFIG_SPI_CADENCE_MRVL_XSPI)
>
> Why this is under #if? Different devices have always built-in support and
> behavior is determined by binding method (e.g. match data).
That is already addressed(matching with compatible property)
>
> > +
> > +#define MRVL_DEFAULT_CLK 25000000
> > +
> > +const int cdns_xspi_clk_div_list[] = {
> > + 4, //0x0 = Divide by 4. SPI clock is 200 MHz.
> > + 6, //0x1 = Divide by 6. SPI clock is 133.33 MHz.
> > + 8, //0x2 = Divide by 8. SPI clock is 100 MHz.
> > + 10, //0x3 = Divide by 10. SPI clock is 80 MHz.
> > + 12, //0x4 = Divide by 12. SPI clock is 66.666 MHz.
> > + 16, //0x5 = Divide by 16. SPI clock is 50 MHz.
> > + 18, //0x6 = Divide by 18. SPI clock is 44.44 MHz.
> > + 20, //0x7 = Divide by 20. SPI clock is 40 MHz.
> > + 24, //0x8 = Divide by 24. SPI clock is 33.33 MHz.
> > + 32, //0x9 = Divide by 32. SPI clock is 25 MHz.
> > + 40, //0xA = Divide by 40. SPI clock is 20 MHz.
> > + 50, //0xB = Divide by 50. SPI clock is 16 MHz.
> > + 64, //0xC = Divide by 64. SPI clock is 12.5 MHz.
> > + 128, //0xD = Divide by 128. SPI clock is 6.25 MHz.
> > + -1 //End of list
>
> Why? This is a static list so size is known.
Ok.
>
> > +};
> > +
> > +static bool cdns_xspi_reset_dll(struct cdns_xspi_dev *cdns_xspi) {
> > + u32 dll_cntrl = readl(cdns_xspi->iobase +
> CDNS_XSPI_RF_MINICTRL_REGS_DLL_PHY_CTRL);
> > + u32 dll_lock;
> > +
> > + /*Reset DLL*/
> > + dll_cntrl |= CDNS_XSPI_DLL_RST_N;
> > + writel(dll_cntrl, cdns_xspi->iobase +
> > +CDNS_XSPI_RF_MINICTRL_REGS_DLL_PHY_CTRL);
> > +
> > + /*Wait for DLL lock*/
>
> All these comments miss spaces around.
>
> > + return readl_relaxed_poll_timeout(cdns_xspi->iobase +
> > + CDNS_XSPI_INTR_STATUS_REG,
> > + dll_lock, ((dll_lock & CDNS_XSPI_DLL_LOCK) == 1), 10, 10000);
> }
> > +
> > +//Static confiuration of PHY
>
> Keep consistent style.
>
> > +static bool cdns_xspi_configure_phy(struct cdns_xspi_dev *cdns_xspi)
> > +{
> > + writel(REGS_DLL_PHY_CTRL,
> > + cdns_xspi->iobase + CDNS_XSPI_RF_MINICTRL_REGS_DLL_PHY_CTRL);
>
> Don't you need a phy driver?
That is one time configuration, that won't change during run.
Is the phy driver needed for such a simple case?
>
> > + writel(CTB_RFILE_PHY_CTRL,
> > + cdns_xspi->auxbase + CDNS_XSPI_PHY_CTB_RFILE_PHY_CTRL);
> > + writel(RFILE_PHY_TSEL,
> > + cdns_xspi->auxbase + CDNS_XSPI_PHY_CTB_RFILE_PHY_TSEL);
> > + writel(RFILE_PHY_DQ_TIMING,
> > + cdns_xspi->auxbase +
> CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DQ_TIMING);
> > + writel(RFILE_PHY_DQS_TIMING,
> > + cdns_xspi->auxbase +
> CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DQS_TIMING);
> > + writel(RFILE_PHY_GATE_LPBK_CTRL,
> > + cdns_xspi->auxbase +
> CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_GATE_LPBK_CTRL);
> > + writel(RFILE_PHY_DLL_MASTER_CTRL,
> > + cdns_xspi->auxbase +
> CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DLL_MASTER_CTRL);
> > + writel(RFILE_PHY_DLL_SLAVE_CTRL,
> > + cdns_xspi->auxbase +
> > +CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DLL_SLAVE_CTRL);
> > +
> > + return cdns_xspi_reset_dll(cdns_xspi); }
> > +
> > +// Find max avalible clock
>
> Run spell-check.
>
>
> Best regards,
> Krzysztof
Regards
Witek
Powered by blists - more mailing lists