[U-Boot] [PATCH v1 15/19] spi: mvebu_a3700_spi: Use Armada 37xx clk driver for SPI clock frequency

[Stefan Roese]

Please add Jagan (on Cc) to Cc for SPI driver related changes.

Please find some comments below...

On 07.03.2018 22:52, Marek Behún wrote:
> Since now we have driver for clocks on Armada 37xx, use it to determine
> SQF clock frequency for the SPI driver.
> 
> Also change the default config files for Armada 37xx devices so that
> the clock driver is enabled by default, otherwise the SPI driver cannot
> be enabled.
> 
> Signed-off-by: Marek Behun <marek.behun at nic.cz>
> ---
>   arch/arm/dts/armada-37xx.dtsi               |  4 +--
>   configs/mvebu_db-88f3720_defconfig          |  3 ++
>   configs/mvebu_espressobin-88f3720_defconfig |  3 ++
>   drivers/spi/Kconfig                         |  1 +
>   drivers/spi/mvebu_a3700_spi.c               | 52 ++++++++++++++++-------------
>   5 files changed, 37 insertions(+), 26 deletions(-)
> 
> diff --git a/arch/arm/dts/armada-37xx.dtsi b/arch/arm/dts/armada-37xx.dtsi
> index e848812fca..c254c0aded 100644
> --- a/arch/arm/dts/armada-37xx.dtsi
> +++ b/arch/arm/dts/armada-37xx.dtsi
> @@ -281,8 +281,8 @@
>   				#address-cells = <1>;
>   				#size-cells = <0>;
>   				#clock-cells = <0>;
> -				clock-frequency = <160000>;
> -				spi-max-frequency = <40000>;
> +				spi-max-frequency = <50000000>;
> +				clocks = <&nb_periph_clk 7>;
>   				status = "disabled";
>   			};
>   
> diff --git a/configs/mvebu_db-88f3720_defconfig b/configs/mvebu_db-88f3720_defconfig
> index 338d764d84..c8ca06e428 100644
> --- a/configs/mvebu_db-88f3720_defconfig
> +++ b/configs/mvebu_db-88f3720_defconfig
> @@ -33,6 +33,9 @@ CONFIG_DM_GPIO=y
>   # CONFIG_MVEBU_GPIO is not set
>   CONFIG_DM_I2C=y
>   CONFIG_MISC=y
> +CONFIG_CLK=y
> +CONFIG_CLK_MVEBU=y
> +CONFIG_CLK_ARMADA_3720=y
>   CONFIG_DM_MMC=y
>   CONFIG_MMC_SDHCI=y
>   CONFIG_MMC_SDHCI_SDMA=y
> diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
> index 28005e6131..5f449d34ea 100644
> --- a/configs/mvebu_espressobin-88f3720_defconfig
> +++ b/configs/mvebu_espressobin-88f3720_defconfig
> @@ -30,6 +30,9 @@ CONFIG_SCSI_AHCI=y
>   CONFIG_BLOCK_CACHE=y
>   CONFIG_DM_I2C=y
>   CONFIG_MISC=y
> +CONFIG_CLK=y
> +CONFIG_CLK_MVEBU=y
> +CONFIG_CLK_ARMADA_3720=y
>   CONFIG_DM_MMC=y
>   CONFIG_MMC_SDHCI=y
>   CONFIG_MMC_SDHCI_SDMA=y
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 235a8c7d73..4ea94a5f35 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -94,6 +94,7 @@ config ICH_SPI
>   
>   config MVEBU_A3700_SPI
>   	bool "Marvell Armada 3700 SPI driver"
> +	depends on CLK_ARMADA_3720

Wouldn't it be easier to "select" CLK_ARMADA_3720 here instead? No
changes to the config files necessary this way.

>   	help
>   	  Enable the Marvell Armada 3700 SPI driver. This driver can be
>   	  used to access the SPI NOR flash on platforms embedding this
> diff --git a/drivers/spi/mvebu_a3700_spi.c b/drivers/spi/mvebu_a3700_spi.c
> index d1708a8d56..19e854945b 100644
> --- a/drivers/spi/mvebu_a3700_spi.c
> +++ b/drivers/spi/mvebu_a3700_spi.c
> @@ -10,6 +10,7 @@
>   #include <dm.h>
>   #include <malloc.h>
>   #include <spi.h>
> +#include <clk.h>
>   #include <wait_bit.h>
>   #include <asm/io.h>
>   
> @@ -22,9 +23,8 @@ DECLARE_GLOBAL_DATA_PTR;
>   #define MVEBU_SPI_A3700_CLK_POL			BIT(7)
>   #define MVEBU_SPI_A3700_FIFO_EN			BIT(17)
>   #define MVEBU_SPI_A3700_SPI_EN_0		BIT(16)
> -#define MVEBU_SPI_A3700_CLK_PRESCALE_BIT	0
> -#define MVEBU_SPI_A3700_CLK_PRESCALE_MASK	\
> -	(0x1f << MVEBU_SPI_A3700_CLK_PRESCALE_BIT)
> +#define MVEBU_SPI_A3700_CLK_PRESCALE_MASK	0x1f
> +
>   
>   /* SPI registers */
>   struct spi_reg {
> @@ -36,8 +36,7 @@ struct spi_reg {
>   
>   struct mvebu_spi_platdata {
>   	struct spi_reg *spireg;
> -	unsigned int frequency;
> -	unsigned int clock;
> +	struct clk clk;
>   };
>   
>   static void spi_cs_activate(struct spi_reg *reg, int cs)
> @@ -178,17 +177,18 @@ static int mvebu_spi_set_speed(struct udevice *bus, uint hz)
>   {
>   	struct mvebu_spi_platdata *plat = dev_get_platdata(bus);
>   	struct spi_reg *reg = plat->spireg;
> -	u32 data;
> +	u32 data, prescale;
>   
>   	data = readl(&reg->cfg);
>   
> -	/* Set Prescaler */
> -	data &= ~MVEBU_SPI_A3700_CLK_PRESCALE_MASK;
> +	prescale = DIV_ROUND_UP(clk_get_rate(&plat->clk), hz);
> +	if (prescale > 31)
> +		prescale = 0x1f;
> +	else if (prescale > 15)
> +		prescale = 0x10 + (prescale + 1)/2;

checkpatch will most likely complain about missing space in the line
above.

>   
> -	/* Calculate Prescaler = (spi_input_freq / spi_max_freq) */
> -	if (hz > plat->frequency)
> -		hz = plat->frequency;
> -	data |= plat->clock / hz;
> +	data &= ~MVEBU_SPI_A3700_CLK_PRESCALE_MASK;
> +	data |= prescale & MVEBU_SPI_A3700_CLK_PRESCALE_MASK;

Perhaps its better to check on "prescale > MVEBU_SPI_A3700_CLK_PRESCALE_MASK"
instead of silently removing the potential additional bits.

Thanks,
Stefan