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

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

[Jagan Teki]

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

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);
@@ -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>
-
 struct pl022_spi_pdata {
 	fdt_addr_t addr;
 	fdt_size_t size;
-#if !CONFIG_IS_ENABLED(OF_PLATDATA)
-	struct clk clk;
-#else
 	unsigned int freq;
-#endif
 };
 
 #endif
-- 
2.18.0.321.gffc6fa0e3

[U-Boot] [PATCH 2/2] spi: pl022: Driver cleanup

[Jagan Teki]

- Drop unnecessary include files.
- Rename platform_data include file as spi_pl022.h from
  pl022_spi.h, this is generic notation used for spi platdat
  include files.

Cc: Quentin Schulz <quentin.schulz@bootlin.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/spi/pl022_spi.c                               | 7 +------
 include/dm/platform_data/{pl022_spi.h => spi_pl022.h} | 6 +++---
 2 files changed, 4 insertions(+), 9 deletions(-)
 rename include/dm/platform_data/{pl022_spi.h => spi_pl022.h} (81%)

diff --git a/drivers/spi/pl022_spi.c b/drivers/spi/pl022_spi.c
index dd27444a43..0ea51f98de 100644
--- a/drivers/spi/pl022_spi.c
+++ b/drivers/spi/pl022_spi.c
@@ -9,16 +9,11 @@
  * Driver for ARM PL022 SPI Controller.
  */
 
-#include <asm/io.h>
 #include <clk.h>
 #include <common.h>
 #include <dm.h>
-#include <dm/platform_data/pl022_spi.h>
-#include <fdtdec.h>
-#include <linux/bitops.h>
-#include <linux/bug.h>
+#include <dm/platform_data/spi_pl022.h>
 #include <linux/io.h>
-#include <linux/kernel.h>
 #include <spi.h>
 
 #define SSP_CR0		0x000
diff --git a/include/dm/platform_data/pl022_spi.h b/include/dm/platform_data/spi_pl022.h
similarity index 81%
rename from include/dm/platform_data/pl022_spi.h
rename to include/dm/platform_data/spi_pl022.h
index 57d12ac912..36e645c836 100644
--- a/include/dm/platform_data/pl022_spi.h
+++ b/include/dm/platform_data/spi_pl022.h
@@ -7,8 +7,8 @@
  * in ofdata_to_platdata.
  */
 
-#ifndef __PL022_SPI_H__
-#define __PL022_SPI_H__
+#ifndef __spi_pl022_h
+#define __spi_pl022_h
 
 struct pl022_spi_pdata {
 	fdt_addr_t addr;
@@ -16,4 +16,4 @@ struct pl022_spi_pdata {
 	unsigned int freq;
 };
 
-#endif
+#endif /* __spi_pl022_h */
-- 
2.18.0.321.gffc6fa0e3

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

[Quentin Schulz]

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>

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

[Jagan Teki]

On Mon, Nov 5, 2018 at 3:12 PM Quentin Schulz
<quentin.schulz@bootlin.com> wrote:
>
> 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.

Dropped ifdef which differentiate platdata vs ofcontrol, ie what I
mentioned in the commit.

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

rate is using many places, it's okay to have temp.

>
> > @@ -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.

I don't think we need this include here. one thing need to fix with is
clkdev variable, I did that and it's compiled.

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

[Quentin Schulz]

Hi Jagan,

On Wed, Nov 14, 2018 at 03:11:26PM +0530, Jagan Teki wrote:
> On Mon, Nov 5, 2018 at 3:12 PM Quentin Schulz
> <quentin.schulz@bootlin.com> wrote:
> >
> > 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.
> 
> Dropped ifdef which differentiate platdata vs ofcontrol, ie what I
> mentioned in the commit.
> 

IMHO, your commit log implies you dropped all the ifdefs to
differentiate between platdata and of_control. That is not true, there
are still some left.

You indeed remove a few but ultimately, it's a consequence of
simplifying the driver to store the frequency of the clock gotten from
DM instead of storing the clock itself. This is just fine for this
commit log.

> > 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.
> 
> rate is using many places, it's okay to have temp.
> 

Three, but fine.

> >
> > > @@ -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.
> 
> I don't think we need this include here. one thing need to fix with is
> clkdev variable, I did that and it's compiled.

The issue I'm having is with fdt_addr_t and fdt_size_t. Any board file
or driver that would include this header would in turn need to include
fdtdec.h. Couldn't we just leave the fdtdec.h header inclusion in this
file so that we don't have to add it in all the files that include the
include/dm/platform_data/pl022_spi.h header please?

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/20181114/7689b87a/attachment.sig>