[U-Boot,06/10] spi: sun4i: Add CLK support

Add CLK support to enable AHB and MOD SPI clocks on sun4i_spi driver.

Note, that the code will enable and disable clock in claim and release
calls to make proper clock and reset handling between claiming and
releasing SPI bus.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/spi/sun4i_spi.c | 56 +++++++++++++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 8 deletions(-)

On 09/02/2019 13:14, Jagan Teki wrote:
> Add CLK support to enable AHB and MOD SPI clocks on sun4i_spi driver.
> 
> Note, that the code will enable and disable clock in claim and release
> calls to make proper clock and reset handling between claiming and
> releasing SPI bus.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>

That looks alright to me, but I wonder if you should introduce the reset
support here as well. Since it's optional, it shouldn't hurt to have it
in already, even though the SUN4I generation doesn't need it.

Cheers,
Andre.

> ---
>  drivers/spi/sun4i_spi.c | 56 +++++++++++++++++++++++++++++++++++------
>  1 file changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/sun4i_spi.c b/drivers/spi/sun4i_spi.c
> index d3cf25db6f..aeed68764c 100644
> --- a/drivers/spi/sun4i_spi.c
> +++ b/drivers/spi/sun4i_spi.c
> @@ -19,6 +19,7 @@
>   */
>  
>  #include <common.h>
> +#include <clk.h>
>  #include <dm.h>
>  #include <spi.h>
>  #include <errno.h>
> @@ -29,8 +30,6 @@
>  #include <asm/gpio.h>
>  #include <asm/io.h>
>  
> -#include <asm/arch/clock.h>
> -
>  #include <linux/iopoll.h>
>  
>  #define SUN4I_RXDATA_REG	0x00
> @@ -133,6 +132,7 @@ struct sun4i_spi_platdata {
>  
>  struct sun4i_spi_priv {
>  	struct sun4i_spi_variant *variant;
> +	struct clk clk_ahb, clk_mod;
>  	u32 base_addr;
>  	u32 freq;
>  	u32 mode;
> @@ -263,13 +263,34 @@ static int sun4i_spi_parse_pins(struct udevice *dev)
>  	return 0;
>  }
>  
> -static inline void sun4i_spi_enable_clock(void)
> +static inline int sun4i_spi_set_clock(struct udevice *dev, bool enable)
>  {
> -	struct sunxi_ccm_reg *const ccm =
> -		(struct sunxi_ccm_reg *const)SUNXI_CCM_BASE;
> +	struct sun4i_spi_priv *priv = dev_get_priv(dev);
> +	int ret;
> +
> +	if (!enable) {
> +		clk_disable(&priv->clk_ahb);
> +		clk_disable(&priv->clk_mod);
> +		return 0;
> +	}
>  
> -	setbits_le32(&ccm->ahb_gate0, (1 << AHB_GATE_OFFSET_SPI0));
> -	writel((1 << 31), &ccm->spi0_clk_cfg);
> +	ret = clk_enable(&priv->clk_ahb);
> +	if (ret) {
> +		dev_err(dev, "failed to enable ahb clock (ret=%d)\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_enable(&priv->clk_mod);
> +	if (ret) {
> +		dev_err(dev, "failed to enable mod clock (ret=%d)\n", ret);
> +		goto err_ahb;
> +	}
> +
> +	return 0;
> +
> +err_ahb:
> +	clk_disable(&priv->clk_ahb);
> +	return ret;
>  }
>  
>  static int sun4i_spi_ofdata_to_platdata(struct udevice *bus)
> @@ -293,8 +314,20 @@ static int sun4i_spi_probe(struct udevice *bus)
>  {
>  	struct sun4i_spi_platdata *plat = dev_get_platdata(bus);
>  	struct sun4i_spi_priv *priv = dev_get_priv(bus);
> +	int ret;
> +
> +	ret = clk_get_by_name(bus, "ahb", &priv->clk_ahb);
> +	if (ret) {
> +		dev_err(dev, "failed to get ahb clock\n");
> +		return ret;
> +	}
> +
> +	ret = clk_get_by_name(bus, "mod", &priv->clk_mod);
> +	if (ret) {
> +		dev_err(dev, "failed to get mod clock\n");
> +		return ret;
> +	}
>  
> -	sun4i_spi_enable_clock();
>  	sun4i_spi_parse_pins(bus);
>  
>  	priv->variant = plat->variant;
> @@ -308,6 +341,11 @@ static int sun4i_spi_claim_bus(struct udevice *dev)
>  {
>  	struct sun4i_spi_priv *priv = dev_get_priv(dev->parent);
>  	struct sun4i_spi_variant *variant = priv->variant;
> +	int ret;
> +
> +	ret = sun4i_spi_set_clock(dev->parent, true);
> +	if (ret)
> +		return ret;
>  
>  	setbits_le32(priv->base_addr + variant->regs[SPI_GCR],
>  		     SUN4I_CTL_ENABLE | SUN4I_CTL_MASTER |
> @@ -328,6 +366,8 @@ static int sun4i_spi_release_bus(struct udevice *dev)
>  	clrbits_le32(priv->base_addr + variant->regs[SPI_GCR],
>  		     SUN4I_CTL_ENABLE);
>  
> +	sun4i_spi_set_clock(dev->parent, false);
> +
>  	return 0;
>  }
>  
>

On Wed, Feb 13, 2019 at 6:51 AM André Przywara <andre.przywara@arm.com> wrote:
>
> On 09/02/2019 13:14, Jagan Teki wrote:
> > Add CLK support to enable AHB and MOD SPI clocks on sun4i_spi driver.
> >
> > Note, that the code will enable and disable clock in claim and release
> > calls to make proper clock and reset handling between claiming and
> > releasing SPI bus.
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>
> That looks alright to me, but I wonder if you should introduce the reset
> support here as well. Since it's optional, it shouldn't hurt to have it
> in already, even though the SUN4I generation doesn't need it.

OK, but I see the point of adding reset during A31 support patch since
it has reset control on it and it's becoming optional since we are
trying to use same driver even for A10.