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.be...@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(®->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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
