[v4,12/35] acpi: Support generation of SPI descriptor

Message ID 20200707131135.v4.12.I533d36d6b9131850d53b716ea555a4e494fa14df@changeid
State Superseded
Headers show
Series
  • dm: Add programmatic generation of ACPI tables (part B)
Related show

Commit Message

Simon Glass July 7, 2020, 7:11 p.m.
Add a function to write a SPI descriptor to the generated ACPI code.

Signed-off-by: Simon Glass <sjg at chromium.org>
Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
---

Changes in v4:
- Move SPI macros to next patch
- Rename the length-writing functions to indicate they are for large resources

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 |  39 ++++++++++++
 include/spi.h              |   4 +-
 lib/acpi/acpi_device.c     | 124 +++++++++++++++++++++++++++++++++++++
 test/dm/acpigen.c          |  36 +++++++++++
 5 files changed, 212 insertions(+), 2 deletions(-)

Comments

Bin Meng July 13, 2020, 4:22 a.m. | #1
On Wed, Jul 8, 2020 at 3:12 AM Simon Glass <sjg at chromium.org> wrote:
>
> Add a function to write a SPI descriptor to the generated ACPI code.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
> Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> ---
>
> Changes in v4:
> - Move SPI macros to next patch
> - Rename the length-writing functions to indicate they are for large resources
>
> 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 |  39 ++++++++++++
>  include/spi.h              |   4 +-
>  lib/acpi/acpi_device.c     | 124 +++++++++++++++++++++++++++++++++++++
>  test/dm/acpigen.c          |  36 +++++++++++
>  5 files changed, 212 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c
> index 570ae285f2..77797bf096 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(sandbox_spi) = {
>         .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 9ba395771e..848ccb9434 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;
> @@ -186,8 +187,11 @@ struct acpi_gpio {
>
>  /* ACPI Descriptors for Serial Bus interfaces */
>  #define ACPI_SERIAL_BUS_TYPE_I2C               1
> +#define ACPI_SERIAL_BUS_TYPE_SPI               2
>  #define ACPI_I2C_SERIAL_BUS_REVISION_ID                1 /* TODO: upgrade to 2 */
>  #define ACPI_I2C_TYPE_SPECIFIC_REVISION_ID     1
> +#define ACPI_SPI_SERIAL_BUS_REVISION_ID                1
> +#define ACPI_SPI_TYPE_SPECIFIC_REVISION_ID     1
>
>  /**
>   * struct acpi_i2c - representation of an ACPI I2C device
> @@ -204,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
>   *
> @@ -303,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 7fca646759..fd931d221a 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 c0d5a9acae..b628fa1337 100644
> --- a/lib/acpi/acpi_device.c
> +++ b/lib/acpi/acpi_device.c
> @@ -492,3 +492,127 @@ int acpi_device_write_i2c_dev(struct acpi_ctx *ctx, const struct udevice *dev)
>
>         return ret;
>  }
> +
> +#ifdef CONFIG_SPI

Dropped this #ifdef when applying

> +/* ACPI 6.1 section 6.4.3.8.2.2 - SpiSerialBus() */
> +static void acpi_device_write_spi(struct acpi_ctx *ctx, const struct acpi_spi *spi)
> +{
> +       void *desc_length, *type_length;
> +       u16 flags = 0;
> +
> +       /* Byte 0: Descriptor Type */
> +       acpigen_emit_byte(ctx, ACPI_DESCRIPTOR_SERIAL_BUS);
> +
> +       /* Byte 1+2: Length (filled in later) */
> +       desc_length = largeres_write_len_f(ctx);
> +
> +       /* Byte 3: Revision ID */
> +       acpigen_emit_byte(ctx, ACPI_SPI_SERIAL_BUS_REVISION_ID);
> +
> +       /* Byte 4: Resource Source Index is Reserved */
> +       acpigen_emit_byte(ctx, 0);
> +
> +       /* Byte 5: Serial Bus Type is SPI */
> +       acpigen_emit_byte(ctx, ACPI_SERIAL_BUS_TYPE_SPI);
> +
> +       /*
> +        * Byte 6: Flags
> +        *  [7:2]: 0 => Reserved
> +        *    [1]: 1 => ResourceConsumer
> +        *    [0]: 0 => ControllerInitiated
> +        */
> +       acpigen_emit_byte(ctx, BIT(1));
> +
> +       /*
> +        * Byte 7-8: Type Specific Flags
> +        *   [15:2]: 0 => Reserveda
> +        *      [1]: 0 => ActiveLow, 1 => ActiveHigh
> +        *      [0]: 0 => FourWire, 1 => ThreeWire
> +        */
> +       if (spi->wire_mode == SPI_3_WIRE_MODE)
> +               flags |= BIT(0);
> +       if (spi->device_select_polarity == SPI_POLARITY_HIGH)
> +               flags |= BIT(1);
> +       acpigen_emit_word(ctx, flags);
> +
> +       /* Byte 9: Type Specific Revision ID */
> +       acpigen_emit_byte(ctx, ACPI_SPI_TYPE_SPECIFIC_REVISION_ID);
> +
> +       /* Byte 10-11: SPI Type Data Length */
> +       type_length = largeres_write_len_f(ctx);
> +
> +       /* Byte 12-15: Connection Speed */
> +       acpigen_emit_dword(ctx, spi->speed);
> +
> +       /* Byte 16: Data Bit Length */
> +       acpigen_emit_byte(ctx, spi->data_bit_length);
> +
> +       /* Byte 17: Clock Phase */
> +       acpigen_emit_byte(ctx, spi->clock_phase);
> +
> +       /* Byte 18: Clock Polarity */
> +       acpigen_emit_byte(ctx, spi->clock_polarity);
> +
> +       /* Byte 19-20: Device Selection */
> +       acpigen_emit_word(ctx, spi->device_select);
> +
> +       /* Fill in Type Data Length */
> +       largeres_fill_len(ctx, type_length);
> +
> +       /* Byte 21+: ResourceSource String */
> +       acpigen_emit_string(ctx, spi->resource);
> +
> +       /* Fill in SPI Descriptor Length */
> +       largeres_fill_len(ctx, desc_length);
> +}
> +
> +/**
> + * acpi_device_set_spi() - Set up an ACPI SPI struct from a device
> + *
> + * The value of @scope is not copied, but only referenced. This implies the
> + * caller has to ensure it stays valid for the lifetime of @spi.
> + *
> + * @dev: SPI device to convert
> + * @spi: Place to put the new structure
> + * @scope: Scope of the SPI device (this is the controller path)
> + * @return 0 (always)
> + */
> +static int acpi_device_set_spi(const struct udevice *dev, struct acpi_spi *spi,
> +                              const char *scope)
> +{
> +       struct dm_spi_slave_platdata *plat;
> +       struct spi_slave *slave = dev_get_parent_priv(dev);
> +
> +       plat = dev_get_parent_platdata(slave->dev);
> +       memset(spi, '\0', sizeof(*spi));
> +       spi->device_select = plat->cs;
> +       spi->device_select_polarity = SPI_POLARITY_LOW;
> +       spi->wire_mode = SPI_4_WIRE_MODE;
> +       spi->speed = plat->max_hz;
> +       spi->data_bit_length = slave->wordlen;
> +       spi->clock_phase = plat->mode & SPI_CPHA ?
> +                SPI_CLOCK_PHASE_SECOND : SPI_CLOCK_PHASE_FIRST;
> +       spi->clock_polarity = plat->mode & SPI_CPOL ?
> +                SPI_POLARITY_HIGH : SPI_POLARITY_LOW;
> +       spi->resource = scope;
> +
> +       return 0;
> +}
> +
> +int acpi_device_write_spi_dev(struct acpi_ctx *ctx, const struct udevice *dev)
> +{
> +       char scope[ACPI_PATH_MAX];
> +       struct acpi_spi spi;
> +       int ret;
> +
> +       ret = acpi_device_scope(dev, scope, sizeof(scope));
> +       if (ret)
> +               return log_msg_ret("scope", ret);
> +       ret = acpi_device_set_spi(dev, &spi, scope);
> +       if (ret)
> +               return log_msg_ret("set", ret);
> +       acpi_device_write_spi(ctx, &spi);
> +
> +       return 0;
> +}
> +#endif /* CONFIG_SPI */
> diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c
> index c210ceb404..f3d9915500 100644
> --- a/test/dm/acpigen.c
> +++ b/test/dm/acpigen.c
> @@ -308,3 +308,39 @@ static int dm_test_acpi_i2c(struct unit_test_state *uts)
>         return 0;
>  }
>  DM_TEST(dm_test_acpi_i2c, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> +
> +/* Test emitting a SPI descriptor */
> +static int dm_test_acpi_spi(struct unit_test_state *uts)
> +{
> +       struct acpi_ctx *ctx;
> +       struct udevice *dev;
> +       u8 *ptr;
> +
> +       ut_assertok(alloc_context(&ctx));
> +
> +       ptr = acpigen_get_current(ctx);
> +
> +       ut_assertok(uclass_first_device_err(UCLASS_SPI_FLASH, &dev));
> +       ut_assertok(acpi_device_write_spi_dev(ctx, dev));
> +       ut_asserteq(31, acpigen_get_current(ctx) - ptr);
> +       ut_asserteq(ACPI_DESCRIPTOR_SERIAL_BUS, ptr[0]);
> +       ut_asserteq(28, get_unaligned((u16 *)(ptr + 1)));
> +       ut_asserteq(ACPI_SPI_SERIAL_BUS_REVISION_ID, ptr[3]);
> +       ut_asserteq(0, ptr[4]);
> +       ut_asserteq(ACPI_SERIAL_BUS_TYPE_SPI, ptr[5]);
> +       ut_asserteq(2, ptr[6]);
> +       ut_asserteq(0, get_unaligned((u16 *)(ptr + 7)));
> +       ut_asserteq(ACPI_SPI_TYPE_SPECIFIC_REVISION_ID, ptr[9]);
> +       ut_asserteq(9, get_unaligned((u16 *)(ptr + 10)));
> +       ut_asserteq(40000000, get_unaligned((u32 *)(ptr + 12)));
> +       ut_asserteq(8, ptr[16]);
> +       ut_asserteq(0, ptr[17]);
> +       ut_asserteq(0, ptr[18]);
> +       ut_asserteq(0, get_unaligned((u16 *)(ptr + 19)));
> +       ut_asserteq_str("\\_SB.SPI0", (char *)ptr + 21);
> +
> +       free_context(&ctx);
> +
> +       return 0;
> +}
> +DM_TEST(dm_test_acpi_spi, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> --

applied to u-boot-x86, thanks!

Regards,
Bin
Bin Meng July 13, 2020, 5:48 a.m. | #2
Hi Simon,

On Mon, Jul 13, 2020 at 12:22 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> On Wed, Jul 8, 2020 at 3:12 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > Add a function to write a SPI descriptor to the generated ACPI code.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
> > Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> > ---
> >
> > Changes in v4:
> > - Move SPI macros to next patch
> > - Rename the length-writing functions to indicate they are for large resources
> >
> > 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 |  39 ++++++++++++
> >  include/spi.h              |   4 +-
> >  lib/acpi/acpi_device.c     | 124 +++++++++++++++++++++++++++++++++++++
> >  test/dm/acpigen.c          |  36 +++++++++++
> >  5 files changed, 212 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c
> > index 570ae285f2..77797bf096 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(sandbox_spi) = {
> >         .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 9ba395771e..848ccb9434 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;
> > @@ -186,8 +187,11 @@ struct acpi_gpio {
> >
> >  /* ACPI Descriptors for Serial Bus interfaces */
> >  #define ACPI_SERIAL_BUS_TYPE_I2C               1
> > +#define ACPI_SERIAL_BUS_TYPE_SPI               2
> >  #define ACPI_I2C_SERIAL_BUS_REVISION_ID                1 /* TODO: upgrade to 2 */
> >  #define ACPI_I2C_TYPE_SPECIFIC_REVISION_ID     1
> > +#define ACPI_SPI_SERIAL_BUS_REVISION_ID                1
> > +#define ACPI_SPI_TYPE_SPECIFIC_REVISION_ID     1
> >
> >  /**
> >   * struct acpi_i2c - representation of an ACPI I2C device
> > @@ -204,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
> >   *
> > @@ -303,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 7fca646759..fd931d221a 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 c0d5a9acae..b628fa1337 100644
> > --- a/lib/acpi/acpi_device.c
> > +++ b/lib/acpi/acpi_device.c
> > @@ -492,3 +492,127 @@ int acpi_device_write_i2c_dev(struct acpi_ctx *ctx, const struct udevice *dev)
> >
> >         return ret;
> >  }
> > +
> > +#ifdef CONFIG_SPI
>
> Dropped this #ifdef when applying

It seems that CONFIG_SPI cannot be dropped?

Please check the build failure @
https://dev.azure.com/bmeng/GitHub/_build/results?buildId=256&view=logs&j=8a1d3be7-a4c9-55b6-774d-e7f1a8f80847&t=b2f224a7-1103-5b52-edbc-3784ae727e03

Please suggest what should be the best to move on.

Regards,
Bin
Simon Glass Aug. 29, 2020, 9:20 p.m. | #3
Hi Bin,

On Sun, 12 Jul 2020 at 23:49, Bin Meng <bmeng.cn@gmail.com> wrote:
>

> Hi Simon,

>

> On Mon, Jul 13, 2020 at 12:22 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> >

> > On Wed, Jul 8, 2020 at 3:12 AM Simon Glass <sjg@chromium.org> wrote:

> > >

> > > Add a function to write a SPI descriptor to the generated ACPI code.

> > >

> > > Signed-off-by: Simon Glass <sjg@chromium.org>

> > > Reviewed-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>

> > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

> > > ---

> > >

> > > Changes in v4:

> > > - Move SPI macros to next patch

> > > - Rename the length-writing functions to indicate they are for large resources

> > >

> > > 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 |  39 ++++++++++++

> > >  include/spi.h              |   4 +-

> > >  lib/acpi/acpi_device.c     | 124 +++++++++++++++++++++++++++++++++++++

> > >  test/dm/acpigen.c          |  36 +++++++++++

> > >  5 files changed, 212 insertions(+), 2 deletions(-)

> > >

> > > diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c

> > > index 570ae285f2..77797bf096 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(sandbox_spi) = {

> > >         .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 9ba395771e..848ccb9434 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;

> > > @@ -186,8 +187,11 @@ struct acpi_gpio {

> > >

> > >  /* ACPI Descriptors for Serial Bus interfaces */

> > >  #define ACPI_SERIAL_BUS_TYPE_I2C               1

> > > +#define ACPI_SERIAL_BUS_TYPE_SPI               2

> > >  #define ACPI_I2C_SERIAL_BUS_REVISION_ID                1 /* TODO: upgrade to 2 */

> > >  #define ACPI_I2C_TYPE_SPECIFIC_REVISION_ID     1

> > > +#define ACPI_SPI_SERIAL_BUS_REVISION_ID                1

> > > +#define ACPI_SPI_TYPE_SPECIFIC_REVISION_ID     1

> > >

> > >  /**

> > >   * struct acpi_i2c - representation of an ACPI I2C device

> > > @@ -204,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

> > >   *

> > > @@ -303,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 7fca646759..fd931d221a 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 c0d5a9acae..b628fa1337 100644

> > > --- a/lib/acpi/acpi_device.c

> > > +++ b/lib/acpi/acpi_device.c

> > > @@ -492,3 +492,127 @@ int acpi_device_write_i2c_dev(struct acpi_ctx *ctx, const struct udevice *dev)

> > >

> > >         return ret;

> > >  }

> > > +

> > > +#ifdef CONFIG_SPI

> >

> > Dropped this #ifdef when applying

>

> It seems that CONFIG_SPI cannot be dropped?

>

> Please check the build failure @

> https://dev.azure.com/bmeng/GitHub/_build/results?buildId=256&view=logs&j=8a1d3be7-a4c9-55b6-774d-e7f1a8f80847&t=b2f224a7-1103-5b52-edbc-3784ae727e03

>

> Please suggest what should be the best to move on.


I think it is OK as it is. It will be easier when SPI migration is finished.

Regards,
SImon

Patch

diff --git a/drivers/spi/sandbox_spi.c b/drivers/spi/sandbox_spi.c
index 570ae285f2..77797bf096 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(sandbox_spi) = {
 	.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 9ba395771e..848ccb9434 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;
@@ -186,8 +187,11 @@  struct acpi_gpio {
 
 /* ACPI Descriptors for Serial Bus interfaces */
 #define ACPI_SERIAL_BUS_TYPE_I2C		1
+#define ACPI_SERIAL_BUS_TYPE_SPI		2
 #define ACPI_I2C_SERIAL_BUS_REVISION_ID		1 /* TODO: upgrade to 2 */
 #define ACPI_I2C_TYPE_SPECIFIC_REVISION_ID	1
+#define ACPI_SPI_SERIAL_BUS_REVISION_ID		1
+#define ACPI_SPI_TYPE_SPECIFIC_REVISION_ID	1
 
 /**
  * struct acpi_i2c - representation of an ACPI I2C device
@@ -204,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
  *
@@ -303,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 7fca646759..fd931d221a 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 c0d5a9acae..b628fa1337 100644
--- a/lib/acpi/acpi_device.c
+++ b/lib/acpi/acpi_device.c
@@ -492,3 +492,127 @@  int acpi_device_write_i2c_dev(struct acpi_ctx *ctx, const struct udevice *dev)
 
 	return ret;
 }
+
+#ifdef CONFIG_SPI
+/* ACPI 6.1 section 6.4.3.8.2.2 - SpiSerialBus() */
+static void acpi_device_write_spi(struct acpi_ctx *ctx, const struct acpi_spi *spi)
+{
+	void *desc_length, *type_length;
+	u16 flags = 0;
+
+	/* Byte 0: Descriptor Type */
+	acpigen_emit_byte(ctx, ACPI_DESCRIPTOR_SERIAL_BUS);
+
+	/* Byte 1+2: Length (filled in later) */
+	desc_length = largeres_write_len_f(ctx);
+
+	/* Byte 3: Revision ID */
+	acpigen_emit_byte(ctx, ACPI_SPI_SERIAL_BUS_REVISION_ID);
+
+	/* Byte 4: Resource Source Index is Reserved */
+	acpigen_emit_byte(ctx, 0);
+
+	/* Byte 5: Serial Bus Type is SPI */
+	acpigen_emit_byte(ctx, ACPI_SERIAL_BUS_TYPE_SPI);
+
+	/*
+	 * Byte 6: Flags
+	 *  [7:2]: 0 => Reserved
+	 *    [1]: 1 => ResourceConsumer
+	 *    [0]: 0 => ControllerInitiated
+	 */
+	acpigen_emit_byte(ctx, BIT(1));
+
+	/*
+	 * Byte 7-8: Type Specific Flags
+	 *   [15:2]: 0 => Reserveda
+	 *      [1]: 0 => ActiveLow, 1 => ActiveHigh
+	 *      [0]: 0 => FourWire, 1 => ThreeWire
+	 */
+	if (spi->wire_mode == SPI_3_WIRE_MODE)
+		flags |= BIT(0);
+	if (spi->device_select_polarity == SPI_POLARITY_HIGH)
+		flags |= BIT(1);
+	acpigen_emit_word(ctx, flags);
+
+	/* Byte 9: Type Specific Revision ID */
+	acpigen_emit_byte(ctx, ACPI_SPI_TYPE_SPECIFIC_REVISION_ID);
+
+	/* Byte 10-11: SPI Type Data Length */
+	type_length = largeres_write_len_f(ctx);
+
+	/* Byte 12-15: Connection Speed */
+	acpigen_emit_dword(ctx, spi->speed);
+
+	/* Byte 16: Data Bit Length */
+	acpigen_emit_byte(ctx, spi->data_bit_length);
+
+	/* Byte 17: Clock Phase */
+	acpigen_emit_byte(ctx, spi->clock_phase);
+
+	/* Byte 18: Clock Polarity */
+	acpigen_emit_byte(ctx, spi->clock_polarity);
+
+	/* Byte 19-20: Device Selection */
+	acpigen_emit_word(ctx, spi->device_select);
+
+	/* Fill in Type Data Length */
+	largeres_fill_len(ctx, type_length);
+
+	/* Byte 21+: ResourceSource String */
+	acpigen_emit_string(ctx, spi->resource);
+
+	/* Fill in SPI Descriptor Length */
+	largeres_fill_len(ctx, desc_length);
+}
+
+/**
+ * acpi_device_set_spi() - Set up an ACPI SPI struct from a device
+ *
+ * The value of @scope is not copied, but only referenced. This implies the
+ * caller has to ensure it stays valid for the lifetime of @spi.
+ *
+ * @dev: SPI device to convert
+ * @spi: Place to put the new structure
+ * @scope: Scope of the SPI device (this is the controller path)
+ * @return 0 (always)
+ */
+static int acpi_device_set_spi(const struct udevice *dev, struct acpi_spi *spi,
+			       const char *scope)
+{
+	struct dm_spi_slave_platdata *plat;
+	struct spi_slave *slave = dev_get_parent_priv(dev);
+
+	plat = dev_get_parent_platdata(slave->dev);
+	memset(spi, '\0', sizeof(*spi));
+	spi->device_select = plat->cs;
+	spi->device_select_polarity = SPI_POLARITY_LOW;
+	spi->wire_mode = SPI_4_WIRE_MODE;
+	spi->speed = plat->max_hz;
+	spi->data_bit_length = slave->wordlen;
+	spi->clock_phase = plat->mode & SPI_CPHA ?
+		 SPI_CLOCK_PHASE_SECOND : SPI_CLOCK_PHASE_FIRST;
+	spi->clock_polarity = plat->mode & SPI_CPOL ?
+		 SPI_POLARITY_HIGH : SPI_POLARITY_LOW;
+	spi->resource = scope;
+
+	return 0;
+}
+
+int acpi_device_write_spi_dev(struct acpi_ctx *ctx, const struct udevice *dev)
+{
+	char scope[ACPI_PATH_MAX];
+	struct acpi_spi spi;
+	int ret;
+
+	ret = acpi_device_scope(dev, scope, sizeof(scope));
+	if (ret)
+		return log_msg_ret("scope", ret);
+	ret = acpi_device_set_spi(dev, &spi, scope);
+	if (ret)
+		return log_msg_ret("set", ret);
+	acpi_device_write_spi(ctx, &spi);
+
+	return 0;
+}
+#endif /* CONFIG_SPI */
diff --git a/test/dm/acpigen.c b/test/dm/acpigen.c
index c210ceb404..f3d9915500 100644
--- a/test/dm/acpigen.c
+++ b/test/dm/acpigen.c
@@ -308,3 +308,39 @@  static int dm_test_acpi_i2c(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_acpi_i2c, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+/* Test emitting a SPI descriptor */
+static int dm_test_acpi_spi(struct unit_test_state *uts)
+{
+	struct acpi_ctx *ctx;
+	struct udevice *dev;
+	u8 *ptr;
+
+	ut_assertok(alloc_context(&ctx));
+
+	ptr = acpigen_get_current(ctx);
+
+	ut_assertok(uclass_first_device_err(UCLASS_SPI_FLASH, &dev));
+	ut_assertok(acpi_device_write_spi_dev(ctx, dev));
+	ut_asserteq(31, acpigen_get_current(ctx) - ptr);
+	ut_asserteq(ACPI_DESCRIPTOR_SERIAL_BUS, ptr[0]);
+	ut_asserteq(28, get_unaligned((u16 *)(ptr + 1)));
+	ut_asserteq(ACPI_SPI_SERIAL_BUS_REVISION_ID, ptr[3]);
+	ut_asserteq(0, ptr[4]);
+	ut_asserteq(ACPI_SERIAL_BUS_TYPE_SPI, ptr[5]);
+	ut_asserteq(2, ptr[6]);
+	ut_asserteq(0, get_unaligned((u16 *)(ptr + 7)));
+	ut_asserteq(ACPI_SPI_TYPE_SPECIFIC_REVISION_ID, ptr[9]);
+	ut_asserteq(9, get_unaligned((u16 *)(ptr + 10)));
+	ut_asserteq(40000000, get_unaligned((u32 *)(ptr + 12)));
+	ut_asserteq(8, ptr[16]);
+	ut_asserteq(0, ptr[17]);
+	ut_asserteq(0, ptr[18]);
+	ut_asserteq(0, get_unaligned((u16 *)(ptr + 19)));
+	ut_asserteq_str("\\_SB.SPI0", (char *)ptr + 21);
+
+	free_context(&ctx);
+
+	return 0;
+}
+DM_TEST(dm_test_acpi_spi, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);