Hi Bin, On Wed, 20 Nov 2019 at 05:48, Bin Meng <[email protected]> wrote: > > Hi Simon, > > On Wed, Nov 20, 2019 at 1:53 AM Simon Glass <[email protected]> wrote: > > > > Hi Bin, > > > > On Tue, 19 Nov 2019 at 06:36, Bin Meng <[email protected]> wrote: > > > > > > Hi Simon, > > > > > > On Mon, Oct 21, 2019 at 11:40 AM Simon Glass <[email protected]> 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 <[email protected]> > > > > --- > > > > > > > > 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 _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
