[U-Boot] [PATCH 1/2] spi: pl022: Get rid of platdata ifdef's
Jagan Teki
jagan at amarulasolutions.com
Wed Nov 14 09:41:26 UTC 2018
On Mon, Nov 5, 2018 at 3:12 PM Quentin Schulz
<quentin.schulz at 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 at bootlin.com>
> > Signed-off-by: Jagan Teki <jagan at 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.
More information about the U-Boot
mailing list