diff mbox series

[6/7] spi: cadence: Add Marvell IP modification changes

Message ID 20221219144254.20883-7-wsadowski@marvell.com
State New
Headers show
Series Support for Marvell modifications for Cadence XSPI | expand

Commit Message

Witold Sadowski Dec. 19, 2022, 2:42 p.m. UTC
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@marvell.com>
Reviewed-by: Chandrakala Chavva <cchavva@marvell.com>
Tested-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
---
 drivers/spi/Kconfig            |  12 +++
 drivers/spi/spi-cadence-xspi.c | 172 ++++++++++++++++++++++++++++++++-
 2 files changed, 183 insertions(+), 1 deletion(-)

Comments

Mark Brown Dec. 19, 2022, 6:27 p.m. UTC | #1
On Mon, Dec 19, 2022 at 06:42:53AM -0800, 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.

What is a PHY in the context of a SPI controller?

> +config SPI_CADENCE_MRVL_XSPI
> +	tristate "Marvell mods for XSPI controller"
> +	depends on SPI_CADENCE_XSPI
> +
> +	help

Extra blank line (does this work?).  It's not clear to me that there's
enough code here to justify a Kconfig.

> +	/*Reset DLL*/

Please follow the kernel coding style.

> @@ -328,6 +468,9 @@ static int cdns_xspi_controller_init(struct cdns_xspi_dev *cdns_xspi)
>  		return -EIO;
>  	}
>  
> +	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_STIG),
> +	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
> +

This is done unconditionally, will other instances in the IP be OK with
it?  Should it be a separate commit since it's affecting everything?

> +#if IS_ENABLED(CONFIG_SPI_CADENCE_MRVL_XSPI)
> +	writel(CDNS_MSIX_CLEAR_IRQ, cdns_xspi->auxbase + CDNS_XSPI_SPIX_INTR_AUX);
> +#endif

This is not how we do support for variants of an IP, we need to support
a single kernel image for many different systems so variant handling
needs to be done with runtime selection not build time selection.
Please handle this in a similar way to how other drivers handle support
for multiple devices.

> +#if IS_ENABLED(CONFIG_SPI_CADENCE_MRVL_XSPI)
> +static int cdns_xspi_setup(struct spi_device *spi_dev)
> +{
> +	struct cdns_xspi_dev *cdns_xspi = spi_master_get_devdata(spi_dev->master);
> +
> +	cdns_xspi_setup_clock(cdns_xspi, spi_dev->max_speed_hz);
> +
> +	return 0;
> +}
> +#endif

Note that setup() might be called while other transfers are in progress
and should not affect them.
Krzysztof Kozlowski Dec. 20, 2022, 2:12 p.m. UTC | #2
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@marvell.com>
> Reviewed-by: Chandrakala Chavva <cchavva@marvell.com>
> Tested-by: Sunil Kovvuri Goutham <sgoutham@marvell.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).

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

> +};
> +
> +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?

> +	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
Witold Sadowski April 29, 2024, 3:10 p.m. UTC | #3
> ----------------------------------------------------------------------
> 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@marvell.com>
> > Reviewed-by: Chandrakala Chavva <cchavva@marvell.com>
> > Tested-by: Sunil Kovvuri Goutham <sgoutham@marvell.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
Witold Sadowski April 29, 2024, 3:27 p.m. UTC | #4
> ----------------------------------------------------------------------
> On Mon, Dec 19, 2022 at 06:42:53AM -0800, 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.
> 
> What is a PHY in the context of a SPI controller?

In that particular driver, we have to set control registers to predefined
Values, that depends on feeding clock/internal architecture etc.

> 
> > +config SPI_CADENCE_MRVL_XSPI
> > +	tristate "Marvell mods for XSPI controller"
> > +	depends on SPI_CADENCE_XSPI
> > +
> > +	help
> 
> Extra blank line (does this work?).  It's not clear to me that there's
> enough code here to justify a Kconfig.

Kconfig is removed now.

> 
> > +	/*Reset DLL*/
> 
> Please follow the kernel coding style.
> 
> > @@ -328,6 +468,9 @@ static int cdns_xspi_controller_init(struct
> cdns_xspi_dev *cdns_xspi)
> >  		return -EIO;
> >  	}
> >
> > +	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE,
> CDNS_XSPI_WORK_MODE_STIG),
> > +	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
> > +
> 
> This is done unconditionally, will other instances in the IP be OK with
> it?  Should it be a separate commit since it's affecting everything?

You are referring to mode switch? That will affect only one IP, different
Will not be affected.

> 
> > +#if IS_ENABLED(CONFIG_SPI_CADENCE_MRVL_XSPI)
> > +	writel(CDNS_MSIX_CLEAR_IRQ, cdns_xspi->auxbase +
> > +CDNS_XSPI_SPIX_INTR_AUX); #endif
> 
> This is not how we do support for variants of an IP, we need to support a
> single kernel image for many different systems so variant handling needs
> to be done with runtime selection not build time selection.
> Please handle this in a similar way to how other drivers handle support
> for multiple devices.

Ok, that was reworked to base on device-tree compatible property.
Also, that part was changed in v2 overlay.

> 
> > +#if IS_ENABLED(CONFIG_SPI_CADENCE_MRVL_XSPI)
> > +static int cdns_xspi_setup(struct spi_device *spi_dev) {
> > +	struct cdns_xspi_dev *cdns_xspi =
> > +spi_master_get_devdata(spi_dev->master);
> > +
> > +	cdns_xspi_setup_clock(cdns_xspi, spi_dev->max_speed_hz);
> > +
> > +	return 0;
> > +}
> > +#endif
> 
> Note that setup() might be called while other transfers are in progress
> and should not affect them.

Clock set function will act only when actual change is needed. And it seems
Changing the clock is not affecting xSPI block

Regards
Witek
diff mbox series

Patch

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*/
+#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)
+
+#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
+};
+
+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*/
+	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
+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);
+	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
+static bool cdns_xspi_setup_clock(struct cdns_xspi_dev *cdns_xspi, int requested_clk)
+{
+	int i = 0;
+	int clk_val;
+	u32 clk_reg;
+	bool update_clk = false;
+
+	while (cdns_xspi_clk_div_list[i] > 0) {
+		clk_val = CDNS_XSPI_CLOCK_DIVIDED(cdns_xspi_clk_div_list[i]);
+		if (clk_val <= requested_clk)
+			break;
+		i++;
+	}
+
+	if (cdns_xspi_clk_div_list[i] == -1) {
+		dev_info(cdns_xspi->dev, "Unable to find clock divider for CLK: %d - setting 6.25MHz\n",
+		       requested_clk);
+		i = 0x0D;
+	} else {
+		dev_dbg(cdns_xspi->dev, "Found clk div: %d, clk val: %d\n",
+			cdns_xspi_clk_div_list[i],
+			CDNS_XSPI_CLOCK_DIVIDED(cdns_xspi_clk_div_list[i]));
+	}
+
+	clk_reg = readl(cdns_xspi->auxbase + CDNS_XSPI_CLK_CTRL_AUX_REG);
+
+	if (FIELD_GET(CDNS_XSPI_CLK_DIV, clk_reg) != i) {
+		clk_reg = FIELD_PREP(CDNS_XSPI_CLK_DIV, i);
+		clk_reg |= CDNS_XSPI_CLK_ENABLE;
+		update_clk = true;
+	}
+
+	if (update_clk)
+		writel(clk_reg, cdns_xspi->auxbase + CDNS_XSPI_CLK_CTRL_AUX_REG);
+
+	return update_clk;
+}
+#endif
+
 static int cdns_xspi_wait_for_controller_idle(struct cdns_xspi_dev *cdns_xspi)
 {
 	u32 ctrl_stat;
@@ -328,6 +468,9 @@  static int cdns_xspi_controller_init(struct cdns_xspi_dev *cdns_xspi)
 		return -EIO;
 	}
 
+	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_STIG),
+	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
+
 	ctrl_features = readl(cdns_xspi->iobase + CDNS_XSPI_CTRL_FEATURES_REG);
 	cdns_xspi->hw_num_banks = FIELD_GET(CDNS_XSPI_NUM_BANKS, ctrl_features);
 	cdns_xspi_set_interrupts(cdns_xspi, false);
@@ -534,6 +677,10 @@  static int cdns_xspi_mem_op(struct cdns_xspi_dev *cdns_xspi,
 	if (cdns_xspi->cur_cs != mem->spi->chip_select)
 		cdns_xspi->cur_cs = mem->spi->chip_select;
 
+#if IS_ENABLED(CONFIG_SPI_CADENCE_MRVL_XSPI)
+	cdns_xspi_setup_clock(cdns_xspi, mem->spi->max_speed_hz);
+#endif
+
 	return cdns_xspi_send_stig_command(cdns_xspi, op,
 					   (dir != SPI_MEM_NO_DATA));
 }
@@ -574,6 +721,10 @@  static irqreturn_t cdns_xspi_irq_handler(int this_irq, void *dev)
 	irq_status = readl(cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
 	writel(irq_status, cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
 
+#if IS_ENABLED(CONFIG_SPI_CADENCE_MRVL_XSPI)
+	writel(CDNS_MSIX_CLEAR_IRQ, cdns_xspi->auxbase + CDNS_XSPI_SPIX_INTR_AUX);
+#endif
+
 	if (irq_status &
 	    (CDNS_XSPI_SDMA_ERROR | CDNS_XSPI_SDMA_TRIGGER |
 	     CDNS_XSPI_STIG_DONE)) {
@@ -654,6 +805,18 @@  static void cdns_xspi_print_phy_config(struct cdns_xspi_dev *cdns_xspi)
 		 readl(cdns_xspi->auxbase + CDNS_XSPI_CCP_PHY_DLL_SLAVE_CTRL));
 }
 
+#if IS_ENABLED(CONFIG_SPI_CADENCE_MRVL_XSPI)
+static int cdns_xspi_setup(struct spi_device *spi_dev)
+{
+	struct cdns_xspi_dev *cdns_xspi = spi_master_get_devdata(spi_dev->master);
+
+	cdns_xspi_setup_clock(cdns_xspi, spi_dev->max_speed_hz);
+
+	return 0;
+}
+#endif
+
+
 static int cdns_xspi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -673,6 +836,9 @@  static int cdns_xspi_probe(struct platform_device *pdev)
 	master->mem_ops = &cadence_xspi_mem_ops;
 	master->dev.of_node = pdev->dev.of_node;
 	master->bus_num = -1;
+#if IS_ENABLED(CONFIG_SPI_CADENCE_MRVL_XSPI)
+	master->setup = cdns_xspi_setup;
+#endif
 
 	platform_set_drvdata(pdev, master);
 
@@ -720,8 +886,12 @@  static int cdns_xspi_probe(struct platform_device *pdev)
 		}
 	}
 
-	cdns_xspi_print_phy_config(cdns_xspi);
+#if IS_ENABLED(CONFIG_SPI_CADENCE_MRVL_XSPI)
+	cdns_xspi_setup_clock(cdns_xspi, MRVL_DEFAULT_CLK);
+	cdns_xspi_configure_phy(cdns_xspi);
+#endif
 
+	cdns_xspi_print_phy_config(cdns_xspi);
 	ret = cdns_xspi_controller_init(cdns_xspi);
 	if (ret) {
 		dev_err(dev, "Failed to initialize controller\n");