[U-Boot] [PATCH v3 083/108] spi: ich: Support of-platdata for fast-spi

[Simon Glass]

Hi Bin,

On Wed, 20 Nov 2019 at 05:48, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Wed, Nov 20, 2019 at 1:53 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Tue, 19 Nov 2019 at 06:36, Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Mon, Oct 21, 2019 at 11:40 AM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > The Intel Fast SPI interface is similar to ICH. Add of-platdata support
> > > > for this using the "intel,fast-spi" compatible string.
> > > >
> > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > ---
> > > >
> > > > Changes in v3: None
> > > > Changes in v2: None
> > > >
> > > >  drivers/spi/ich.c | 17 ++++++++++++++---
> > > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
> > > > index b28f3423584..ae3bff36bba 100644
> > > > --- a/drivers/spi/ich.c
> > > > +++ b/drivers/spi/ich.c
> > > > @@ -10,6 +10,7 @@
> > > >  #include <common.h>
> > > >  #include <div64.h>
> > > >  #include <dm.h>
> > > > +#include <dt-structs.h>
> > > >  #include <errno.h>
> > > >  #include <malloc.h>
> > > >  #include <pch.h>
> > > > @@ -28,9 +29,13 @@
> > > >  #endif
> > > >
> > > >  struct ich_spi_platdata {
> > > > +#if CONFIG_IS_ENABLED(OF_PLATDATA)
> > > > +       struct dtd_intel_fast_spi dtplat;
> > > > +#endif
> > > >         enum ich_version ich_version;   /* Controller version, 7 or 9 */
> > > >         bool lockdown;                  /* lock down controller settings? */
> > > >         ulong mmio_base;                /* Base of MMIO registers */
> > > > +       pci_dev_t bdf;                  /* PCI address used by of-platdata */
> > > >  };
> > > >
> > > >  static u8 ich_readb(struct ich_spi_priv *priv, int reg)
> > > > @@ -597,10 +602,16 @@ static int ich_spi_ofdata_to_platdata(struct udevice *dev)
> > > >  {
> > > >         struct ich_spi_platdata *plat = dev_get_platdata(dev);
> > > >
> > > > +#if !CONFIG_IS_ENABLED(OF_PLATDATA)
> > > >         plat->ich_version = dev_get_driver_data(dev);
> > > >         plat->lockdown = dev_read_bool(dev, "intel,spi-lock-down");
> > > > -
> > > >         pch_get_spi_base(dev->parent, &plat->mmio_base);
> > > > +#else
> > > > +       plat->ich_version = ICHV_APL;
> > >
> > > I don't think the ICH_VERSION is something that is controlled via OF_PLATDATA.
> >
> > Well, the fact that we are using dtd_intel_fast_spi means that the
> > compatible string is intel,fast-spi, so that's why we set this version
> > variable. The older SPI-controller versions don't actually support
> > of-platdata, so this is safe.
> >
>
> What if we have another platform that uses OF_PLATDATA but is not
> ICHV_APL? How do we distinguish that?
>
> > >
> > > > +       plat->mmio_base = plat->dtplat.early_regs[0];
> > > > +       plat->bdf = pci_x86_ofplat_get_devfn(plat->dtplat.reg[0]);
> > > > +#endif
> > > > +       debug("%s: mmio_base=%lx\n", __func__, plat->mmio_base);
> > > >
> > > >         return 0;
> > > >  }
> > > > @@ -628,8 +639,8 @@ static const struct udevice_id ich_spi_ids[] = {
> > > >         { }
> > > >  };
> > > >
> > > > -U_BOOT_DRIVER(ich_spi) = {
> > > > -       .name   = "ich_spi",
> > > > +U_BOOT_DRIVER(intel_fast_spi) = {
> > >
> > > Why changing the driver name?
> >
> > So that of-platdata will create a U_BOOT_DEVICE() for it and it will
> > be bound. Otherwise the name will not match and the device is ignored.
> >
>
> Could we avoid renaming the driver, ie: reusing the same "ich_spi" name?

Yes, but that would mean we'd have to use a compatible string of
"ich-spi' which breaks the rule of using 'vendor,device'.


>
> > I will update the commit message.
> >
> > >
> > > > +       .name   = "intel_fast_spi",
> > > >         .id     = UCLASS_SPI,
> > > >         .of_match = ich_spi_ids,
> > > >         .ops    = &ich_spi_ops,
> > > > --

Regards,
Simon