Hi Bin,

On Sun, 28 Jun 2020 at 19:27, Bin Meng <bmeng...@gmail.com> wrote:
>
> Hi Simon,
>
> On Sun, Jun 14, 2020 at 10:55 AM Simon Glass <s...@chromium.org> wrote:
> >
> > Add a function to write a SPI descriptor to the generated ACPI code.
> >
> > Signed-off-by: Simon Glass <s...@chromium.org>
> > ---
> >
> > Changes in v3:
> > - Make acpi_device_write_spi() static
> > - Add an extra comment about scope to acpi_device_set_spi()
> > - Use BIT() in a few places
> > - Resist the temptation to go to >80 characters
> >
> > Changes in v2:
> > - Fix memset of SPI descriptor
> >
> >  drivers/spi/sandbox_spi.c  |  11 ++++
> >  include/acpi/acpi_device.h |  36 +++++++++++
> >  include/spi.h              |   4 +-
> >  lib/acpi/acpi_device.c     | 124 +++++++++++++++++++++++++++++++++++++
> >  test/dm/acpigen.c          |  36 +++++++++++
> >  5 files changed, 209 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c
> > index b0a46c8868..4264acc953 100644
> > --- a/drivers/spi/sandbox_spi.c
> > +++ b/drivers/spi/sandbox_spi.c
> > @@ -21,6 +21,7 @@
> >  #include <linux/errno.h>
> >  #include <asm/spi.h>
> >  #include <asm/state.h>
> > +#include <dm/acpi.h>
> >  #include <dm/device-internal.h>
> >
> >  #ifndef CONFIG_SPI_IDLE_VAL
> > @@ -133,6 +134,15 @@ static int sandbox_spi_get_mmap(struct udevice *dev, 
> > ulong *map_basep,
> >         return 0;
> >  }
> >
> > +static int sandbox_spi_get_name(const struct udevice *dev, char *out_name)
> > +{
> > +       return acpi_copy_name(out_name, "SSPI");
> > +}
> > +
> > +struct acpi_ops sandbox_spi_acpi_ops = {
> > +       .get_name       = sandbox_spi_get_name,
> > +};
> > +
> >  static const struct dm_spi_ops sandbox_spi_ops = {
> >         .xfer           = sandbox_spi_xfer,
> >         .set_speed      = sandbox_spi_set_speed,
> > @@ -151,4 +161,5 @@ U_BOOT_DRIVER(spi_sandbox) = {
> >         .id     = UCLASS_SPI,
> >         .of_match = sandbox_spi_ids,
> >         .ops    = &sandbox_spi_ops,
> > +       ACPI_OPS_PTR(&sandbox_spi_acpi_ops)
> >  };
> > diff --git a/include/acpi/acpi_device.h b/include/acpi/acpi_device.h
> > index f738b03d58..c0c96183e4 100644
> > --- a/include/acpi/acpi_device.h
> > +++ b/include/acpi/acpi_device.h
> > @@ -10,6 +10,7 @@
> >  #define __ACPI_DEVICE_H
> >
> >  #include <i2c.h>
> > +#include <spi.h>
> >  #include <linux/bitops.h>
> >
> >  struct acpi_ctx;
> > @@ -207,6 +208,29 @@ struct acpi_i2c {
> >         const char *resource;
> >  };
> >
> > +/**
> > + * struct acpi_spi - representation of an ACPI SPI device
> > + *
> > + * @device_select: Chip select used by this device (typically 0)
> > + * @device_select_polarity: Polarity for the device
> > + * @wire_mode: Number of wires used for SPI
> > + * @speed: Bus speed in Hz
> > + * @data_bit_length: Word length for SPI (typically 8)
> > + * @clock_phase: Clock phase to capture data
> > + * @clock_polarity: Bus polarity
> > + * @resource: Resource name for the SPI controller
> > + */
> > +struct acpi_spi {
> > +       u16 device_select;
> > +       enum spi_polarity device_select_polarity;
> > +       enum spi_wire_mode wire_mode;
> > +       unsigned int speed;
> > +       u8 data_bit_length;
> > +       enum spi_clock_phase clock_phase;
> > +       enum spi_polarity clock_polarity;
> > +       const char *resource;
> > +};
> > +
> >  /**
> >   * acpi_device_path() - Get the full path to an ACPI device
> >   *
> > @@ -306,4 +330,16 @@ int acpi_device_write_interrupt_or_gpio(struct 
> > acpi_ctx *ctx,
> >   */
> >  int acpi_device_write_i2c_dev(struct acpi_ctx *ctx, const struct udevice 
> > *dev);
> >
> > +/**
> > + * acpi_device_write_spi_dev() - Write a SPI device to ACPI
> > + *
> > + * This writes a serial bus descriptor for the SPI device so that ACPI can 
> > use
> > + * it
> > + *
> > + * @ctx: ACPI context pointer
> > + * @dev: SPI device to write
> > + * @return 0 if OK, -ve on error
> > + */
> > +int acpi_device_write_spi_dev(struct acpi_ctx *ctx, const struct udevice 
> > *dev);
> > +
> >  #endif
> > diff --git a/include/spi.h b/include/spi.h
> > index 5cc6d6e008..f34533f54e 100644
> > --- a/include/spi.h
> > +++ b/include/spi.h
> > @@ -13,8 +13,8 @@
> >  #include <linux/bitops.h>
> >
> >  /* SPI mode flags */
> > -#define SPI_CPHA       BIT(0)                  /* clock phase */
> > -#define SPI_CPOL       BIT(1)                  /* clock polarity */
> > +#define SPI_CPHA       BIT(0)  /* clock phase (1 = SPI_CLOCK_PHASE_SECOND) 
> > */
> > +#define SPI_CPOL       BIT(1)  /* clock polarity (1 = SPI_POLARITY_HIGH) */
> >  #define SPI_MODE_0     (0|0)                   /* (original MicroWire) */
> >  #define SPI_MODE_1     (0|SPI_CPHA)
> >  #define SPI_MODE_2     (SPI_CPOL|0)
> > diff --git a/lib/acpi/acpi_device.c b/lib/acpi/acpi_device.c
> > index 40d1a82d27..38101c8b96 100644
> > --- a/lib/acpi/acpi_device.c
> > +++ b/lib/acpi/acpi_device.c
> > @@ -488,3 +488,127 @@ int acpi_device_write_i2c_dev(struct acpi_ctx *ctx, 
> > const struct udevice *dev)
> >
> >         return 0;
> >  }
> > +
> > +#ifdef CONFIG_SPI
>
> Why #ifdef? Other APIs don't have this

This is due to the #ifdef in the spi.h header file. I sent you a patch
to fix that for a different reason (APL not booting). So if that goes
in we can drop this.

Regards,
SImon

Reply via email to