[U-Boot] [PATCH v5 073/101] spi: ich: Support hardware sequencing

[Bin Meng]

Hi Simon,

On Thu, Nov 28, 2019 at 11:19 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Bin,
>
> On Wed, 27 Nov 2019 at 20:08, Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Thu, Nov 28, 2019 at 10:23 AM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Wed, 27 Nov 2019 at 00:16, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Mon, Nov 25, 2019 at 12:12 PM Simon Glass <sjg at chromium.org> wrote:
> > > > >
> > > > > Apollo Lake (APL) only supports hardware sequencing. Add support for this
> > > > > into the SPI driver, as an option.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > >
> > > > > ---
> > > > >
> > > > > Changes in v5: None
> > > > > Changes in v4:
> > > > > - Fix comment for exec_sync_hwseq_xfer()
> > > > > - apollolake -> Apollo Lake
> > > > >
> > > > > Changes in v3: None
> > > > > Changes in v2: None
> > > > >
> > > > >  drivers/spi/ich.c | 205 +++++++++++++++++++++++++++++++++++++++++++++-
> > > > >  drivers/spi/ich.h |  39 +++++++++
> > > > >  2 files changed, 241 insertions(+), 3 deletions(-)
> > > > >
> > > >
> > > > [snip]
> > > >
> > > > > +static int ich_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
> > > > > +{
> > > > > +       struct udevice *bus = dev_get_parent(slave->dev);
> > > > > +       struct ich_spi_platdata *plat = dev_get_platdata(bus);
> > > > > +       int ret;
> > > > > +
> > > > > +       bootstage_start(BOOTSTAGE_ID_ACCUM_SPI, "fast_spi");
> > > > > +       if (plat->hwseq)
> > > > > +               ret = ich_spi_exec_op_hwseq(slave, op);
> > > > > +       else
> > > > > +               ret = ich_spi_exec_op_swseq(slave, op);
> > > > > +       bootstage_accum(BOOTSTAGE_ID_ACCUM_SPI);
> > > > > +
> > > > > +       return ret;
> > > > > +}
> > > > > +
> > > > >  static int ich_spi_adjust_size(struct spi_slave *slave, struct spi_mem_op *op)
> > > > >  {
> > > > >         unsigned int page_offset;
> > > > > @@ -583,9 +778,11 @@ static int ich_spi_child_pre_probe(struct udevice *dev)
> > > > >
> > > > >         /*
> > > > >          * Yes this controller can only write a small number of bytes at
> > > > > -        * once! The limit is typically 64 bytes.
> > > > > +        * once! The limit is typically 64 bytes. For hardware sequencing a
> > > > > +        * a loop is used to get around this.
> > > > >          */
> > > > > -       slave->max_write_size = priv->databytes;
> > > > > +       if (!plat->hwseq)
> > > > > +               slave->max_write_size = priv->databytes;
> > > > >         /*
> > > > >          * ICH 7 SPI controller only supports array read command
> > > > >          * and byte program command for SST flash
> > > > > @@ -611,10 +808,12 @@ static int ich_spi_ofdata_to_platdata(struct udevice *dev)
> > > > >         plat->ich_version = dev_get_driver_data(dev);
> > > > >         plat->lockdown = dev_read_bool(dev, "intel,spi-lock-down");
> > > > >         pch_get_spi_base(priv->pch, &plat->mmio_base);
> > > > > +       plat->hwseq = dev_read_u32_default(dev, "intel,hardware-seq", 0);
> > > >
> > > > I believe dev_read_bool() is enough for now, unless we have different
> > > > types of hardware sequencer to support?
> > >
> > > Yes I remember this...unfortunately dtoc does not generate a struct
> > > member for a bool if it is not present (since it has no way of knowing
> > > it *might* be there.
> > >
> >
> > OK, could you please put a comment here?
>
> Yes, will do.
>
> I'll hold off sending any updates for now. I'll redo the cr50/tpm
> series after that. I am playing around with ACPI too, but best to send
> that one we have this one sorted out.
>

OK. I plan to continue reviewing the remaining patches. But if you can
hold on sending more ACPI patches, that will be helpful. I will see if
I can selectively pick up patches that look good to save both our
time.

> >
> > > So to keep of-platdata happy I made this a 0/1 value.
> > >
> > > We probably need a better solution but I don't have one right now.
> > >

Regards,
Bin