[U-Boot] [PATCH 1/2] spi: pl022: Get rid of platdata ifdef's

[Quentin Schulz <quentin.schulz@bootlin.com>]

Hi Jagan,

On Mon, Nov 05, 2018 at 02:49:25PM +0530, Jagan Teki wrote:
> pl022 spi driver support both OF_CONTROL and PLATDATA and
> it's using #ifdef check for differentiating platdata vs of_control.
> So this patch simplify the code and drop the ifdefs
> 

You drop a few ifdef, not all. I also find this commit log a bit cryptic.
You basically store the frequency of the clock gotten from DM instead of
storing the clock itself, which is indeed a welcome simplification of
the driver (and you re-order function pointers in the driver structure).

> Cc: Quentin Schulz <quentin.schulz@bootlin.com>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/spi/pl022_spi.c              | 48 ++++++++++++----------------
>  include/dm/platform_data/pl022_spi.h |  9 ------
>  2 files changed, 20 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/spi/pl022_spi.c b/drivers/spi/pl022_spi.c
> index 86b71d2e21..dd27444a43 100644
> --- a/drivers/spi/pl022_spi.c
> +++ b/drivers/spi/pl022_spi.c
> @@ -72,11 +72,7 @@
>  
>  struct pl022_spi_slave {
>  	void *base;
> -#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> -	struct clk clk;
> -#else
>  	unsigned int freq;
> -#endif
>  };
>  
>  /*
> @@ -96,30 +92,13 @@ static int pl022_is_supported(struct pl022_spi_slave *ps)
>  	return 0;
>  }
>  
> -#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> -static int pl022_spi_ofdata_to_platdata(struct udevice *bus)
> -{
> -	struct pl022_spi_pdata *plat = bus->platdata;
> -	const void *fdt = gd->fdt_blob;
> -	int node = dev_of_offset(bus);
> -
> -	plat->addr = fdtdec_get_addr_size(fdt, node, "reg", &plat->size);
> -
> -	return clk_get_by_index(bus, 0, &plat->clk);
> -}
> -#endif
> -
>  static int pl022_spi_probe(struct udevice *bus)
>  {
>  	struct pl022_spi_pdata *plat = dev_get_platdata(bus);
>  	struct pl022_spi_slave *ps = dev_get_priv(bus);
>  
>  	ps->base = ioremap(plat->addr, plat->size);
> -#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> -	ps->clk = plat->clk;
> -#else
>  	ps->freq = plat->freq;
> -#endif
>  
>  	/* Check the PL022 version */
>  	if (!pl022_is_supported(ps))
> @@ -240,11 +219,7 @@ static int pl022_spi_set_speed(struct udevice *bus, uint speed)
>  	u16 scr = SSP_SCR_MIN, cr0 = 0, cpsr = SSP_CPSR_MIN, best_scr = scr,
>  	    best_cpsr = cpsr;
>  	u32 min, max, best_freq = 0, tmp;
> -#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> -	u32 rate = clk_get_rate(&ps->clk);
> -#else
>  	u32 rate = ps->freq;
> -#endif
>  	bool found = false;
>  
>  	max = spi_rate(rate, SSP_CPSR_MIN, SSP_SCR_MIN);

We don't need the u32 rate temp variable anymore so better replace it in
this function with ps->freq directly.

> @@ -316,6 +291,25 @@ static const struct dm_spi_ops pl022_spi_ops = {
>  };
>  
>  #if !CONFIG_IS_ENABLED(OF_PLATDATA)
> +static int pl022_spi_ofdata_to_platdata(struct udevice *bus)
> +{
> +	struct pl022_spi_pdata *plat = bus->platdata;
> +	struct udevice *clkdev;
> +	const void *fdt = gd->fdt_blob;
> +	int node = dev_of_offset(bus);
> +	int ret;
> +
> +	plat->addr = fdtdec_get_addr_size(fdt, node, "reg", &plat->size);
> +
> +	ret = clk_get_by_index(bus, 0, &clkdev);
> +	if (ret)
> +		return ret;
> +
> +	plat->freq = clk_get_rate(clkdev);
> +
> +	return 0;
> +}
> +
>  static const struct udevice_id pl022_spi_ids[] = {
>  	{ .compatible = "arm,pl022-spi" },
>  	{ }
> @@ -327,11 +321,9 @@ U_BOOT_DRIVER(pl022_spi) = {
>  	.id     = UCLASS_SPI,
>  #if !CONFIG_IS_ENABLED(OF_PLATDATA)
>  	.of_match = pl022_spi_ids,
> -#endif
> -	.ops    = &pl022_spi_ops,
> -#if !CONFIG_IS_ENABLED(OF_PLATDATA)
>  	.ofdata_to_platdata = pl022_spi_ofdata_to_platdata,
>  #endif
> +	.ops    = &pl022_spi_ops,
>  	.platdata_auto_alloc_size = sizeof(struct pl022_spi_pdata),
>  	.priv_auto_alloc_size = sizeof(struct pl022_spi_slave),
>  	.probe  = pl022_spi_probe,
> diff --git a/include/dm/platform_data/pl022_spi.h b/include/dm/platform_data/pl022_spi.h
> index 77fe6da3cb..57d12ac912 100644
> --- a/include/dm/platform_data/pl022_spi.h
> +++ b/include/dm/platform_data/pl022_spi.h
> @@ -10,19 +10,10 @@
>  #ifndef __PL022_SPI_H__
>  #define __PL022_SPI_H__
>  
> -#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> -#include <clk.h>
> -#endif
> -#include <fdtdec.h>

Nope, I need this one else it does not compile.

Thanks,
Quentin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181105/057d8337/attachment.sig>