Re: [PATCH v2 41/44] x86: Drop setup_pcat_compatibility()

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:34 AM Simon Glass  wrote:
>
> This function does not exist anymore. Drop it from the header file.
>
> Signed-off-by: Simon Glass 
> ---
>
> Changes in v2:
> - Remove the function from zimage.c also
>
>  arch/x86/include/asm/u-boot-x86.h |  2 --
>  arch/x86/lib/zimage.c | 10 --
>  2 files changed, 12 deletions(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table

2020-07-12 Thread Bin Meng
Hi Simon,

On Mon, Jul 13, 2020 at 3:37 AM Simon Glass  wrote:
>
> Hi Bin,
>
> On Tue, 7 Jul 2020 at 21:25, Simon Glass  wrote:
> >
> > Hi Bin,
> >
> > On Mon, 6 Jul 2020 at 18:22, Bin Meng  wrote:
> > >
> > > Hi Simon,
> > >
> > > On Tue, Jul 7, 2020 at 3:22 AM Simon Glass  wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Thu, 2 Jul 2020 at 22:33, Bin Meng  wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Fri, Jul 3, 2020 at 11:50 AM Simon Glass  wrote:
> > > > > >
> > > > > > Hi Bin,
> > > > > >
> > > > > > On Thu, 2 Jul 2020 at 18:54, Bin Meng  wrote:
> > > > > > >
> > > > > > > Hi Simon,
> > > > > > >
> > > > > > > On Fri, Jul 3, 2020 at 8:46 AM Simon Glass  
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Bin,
> > > > > > > >
> > > > > > > > On Mon, 29 Jun 2020 at 20:49, Bin Meng  
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Simon,
> > > > > > > > >
> > > > > > > > > On Mon, Jun 15, 2020 at 11:57 AM Simon Glass 
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > The Intel Non-High-Definition-Audio Link Table (NHLT) table 
> > > > > > > > > > describes the
> > > > > > > > > > audio codecs and connections in a system. Various devices 
> > > > > > > > > > can contribute
> > > > > > > > > > information to produce the table.
> > > > > > > > > >
> > > > > > > > > > Add core support for this, based on a structure which is 
> > > > > > > > > > built up through
> > > > > > > > > > calls to the driver.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Simon Glass 
> > > > > > > > > > ---
> > > > > > > > > >
> > > > > > > > > >  drivers/core/acpi.c | 15 +++
> > > > > > > > > >  include/dm/acpi.h   | 26 ++
> > > > > > > > > >  2 files changed, 41 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > > > > > > > > > index ea304a3067..a5053fec6f 100644
> > > > > > > > > > --- a/drivers/core/acpi.c
> > > > > > > > > > +++ b/drivers/core/acpi.c
> > > > > > > > > > @@ -31,6 +31,7 @@ enum method_t {
> > > > > > > > > > METHOD_WRITE_TABLES,
> > > > > > > > > > METHOD_FILL_SSDT,
> > > > > > > > > > METHOD_INJECT_DSDT,
> > > > > > > > > > +   METHOD_SETUP_NHLT,
> > > > > > > > >
> > > > > > > > > Do we really need to promote this to be an ACPI core method? 
> > > > > > > > > Can we
> > > > > > > > > reuse the SSDT/DSDT one?
> > > > > > > >
> > > > > > > > I don't think so. Those two are for a particular purpose. In 
> > > > > > > > fact NHLT
> > > > > > > > is generated while doing SSDT I think. The idea is that drivers 
> > > > > > > > that
> > > > > > > > want to contribute to NHLT can do so. But we cannot use the SSDT
> > > > > > > > mechanism since each driver contributes only a part of the 
> > > > > > > > info, and
> > > > > > > > we need something else to bring it all together.
> > > > > > >
> > > > > > > Will there be only one device that sets up the NHLT info?
> > > > > >
> > > > > > WIth coral it is two devices. I'm not sure of the maximum, but I
> > > > > > suppose it depends on the audio codecs present.
> > > > >
> > > > > Could we make this method to be provided by the codec device, instead
> > > > > of a generic ACPI core method?
> > > >
> > > > The codec device does implement this. See the drivers where they
> > > > actually implement the NHLT method.
> > > >
> > > > This is definitely an ACPI-specific thing, so I think we need core
> > > > support for iterating through drivers that want to provide this info.
> > >
> > > My concern is that this is not generic enough to promote this to ACPI 
> > > core.
> > >
> > > I wanted to have something like this:
> > >
> > > Create a codec uclass driver, and in the code uclass driver, create an
> > > op that is used to set up the NHLT infor if ACPI_GEN is on.
> >
> > We already have UCLASS_SOUND so could add it to sound_ops. But that
> > seems weird to me since this is not an operation to play a sound. We
> > do this with GIPOs and IRQs but in that case the operation returns
> > some data. Here we are asking the driver to add some data to a table.
> > I'm just not sure it makes sense.
> >
> > What do you think?
>
> Let me know what you think about this as I'd like to finish this series.

I haven't looked into the details of NHLT but I trust your
consideration so let's leave this as it is.

Regards,
Bin


Re: [PATCH v2 28/44] i2c: designware_i2c: Support ACPI table generation

2020-07-12 Thread Bin Meng
Hi Simon,

On Mon, Jul 13, 2020 at 1:54 PM Bin Meng  wrote:
>
> On Wed, Jul 8, 2020 at 7:11 PM Wolfgang Wallner
>  wrote:
> >
> > Hi Simon,
> >
> > -"Simon Glass"  schrieb: -
> > > Betreff: [PATCH v2 28/44] i2c: designware_i2c: Support ACPI table 
> > > generation
> > >
> > > Update the PCI driver to generate ACPI information so that Linux has the
> > > full information about each I2C bus.
> > >
> > > Signed-off-by: Simon Glass 
> > >
> > > Reviewed-by: Heiko Schocher 
> > > ---
> > >
> > > Changes in v2:
> > > - Add a few blank lines
> > > - Drop dead code behind if (0)
> > >
> > > Changes in v1:
> > > - Capitalise ACPI_OPS_PTR
> > >
> > >  drivers/i2c/designware_i2c.c | 26 +
> > >  drivers/i2c/designware_i2c.h | 15 +
> > >  drivers/i2c/designware_i2c_pci.c | 96 +++-
> > >  3 files changed, 136 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c
> > > index 44a1f33398..cf892c69d9 100644
> > > --- a/drivers/i2c/designware_i2c.c
> > > +++ b/drivers/i2c/designware_i2c.c
> > > @@ -333,6 +333,32 @@ static int _dw_i2c_set_bus_speed(struct dw_i2c 
> > > *priv, struct i2c_regs *i2c_base,
> > >   /* Restore back i2c now speed set */
> > >   if (ena == IC_ENABLE_0B)
> > >   dw_i2c_enable(i2c_base, true);
> > > + if (priv)
> > > + priv->config = config;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +int dw_i2c_gen_speed_config(const struct udevice *dev, int speed_hz,
> > > + struct dw_i2c_speed_config *config)
> > > +{
> > > + struct dw_i2c *priv = dev_get_priv(dev);
> > > + ulong rate;
> > > + int ret;
> > > +
> > > +#if CONFIG_IS_ENABLED(CLK)
> > > + rate = clk_get_rate(&priv->clk);
> > > + if (IS_ERR_VALUE(rate))
> > > + return log_msg_ret("clk", -EINVAL);
> > > +#else
> > > + rate = IC_CLK;
> > > +#endif
> > > +
> > > + ret = calc_bus_speed(priv, priv->regs, speed_hz, rate, config);
> > > + if (ret)
> > > + printf("%s: ret=%d\n", __func__, ret);
> > > + if (ret)
> > > + return log_msg_ret("calc_bus_speed", ret);
> > >
> > >   return 0;
> > >  }
> > > diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h
> > > index dc9a6ccb63..d87a3bff93 100644
> > > --- a/drivers/i2c/designware_i2c.h
> > > +++ b/drivers/i2c/designware_i2c.h
> > > @@ -205,6 +205,7 @@ struct dw_i2c {
> > >  #if CONFIG_IS_ENABLED(CLK)
> > >   struct clk clk;
> > >  #endif
> > > + struct dw_i2c_speed_config config;
> > >  };
> > >
> > >  extern const struct dm_i2c_ops designware_i2c_ops;
> > > @@ -213,4 +214,18 @@ int designware_i2c_probe(struct udevice *bus);
> > >  int designware_i2c_remove(struct udevice *dev);
> > >  int designware_i2c_ofdata_to_platdata(struct udevice *bus);
> > >
> > > +/**
> > > + * dw_i2c_gen_speed_config() - Calculate config info from requested 
> > > speed1
> >
> > Nit: is "speed1" a typo?
>
> I think so. Will fix it when applying.

Since this series requires rebasing on top of u-boot-x86/master,
please fix this in your next version.

>
> >
> > > + *
> > > + * Calculate the speed config from the given @speed_hz and return it so 
> > > that
> > > + * it can be incorporated in ACPI tables
> > > + *
> > > + * @dev: I2C bus to check
> > > + * @speed_hz: Requested speed in Hz
> > > + * @config: Returns config to use for that speed
> > > + * @return 0 if OK, -ve on error
> > > + */
> > > +int dw_i2c_gen_speed_config(const struct udevice *dev, int speed_hz,
> > > + struct dw_i2c_speed_config *config);
> > > +
> > >  #endif /* __DW_I2C_H_ */

Regards,
Bin


Re: [PATCH v2 44/44] acpi: Enable ACPI table generation by default on x86

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:34 AM Simon Glass  wrote:
>
> This should ideally be used by all x86 boards in U-Boot. Enable it by
> default. If some boards don't use it, the cost is small.
>
> Signed-off-by: Simon Glass 
> ---
>
> Changes in v2:
> - Don't enable this for qemu
>
>  arch/Kconfig | 1 +
>  drivers/core/Kconfig | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 39/44] x86: mp: Allow use of mp_run_on_cpus() without MP

2020-07-12 Thread Bin Meng
Hi Simon,

On Wed, Jul 8, 2020 at 11:34 AM Simon Glass  wrote:
>
> At present if MP is not enabled (e.g. booting from coreboot) the 'mtrr'
> command does not work correctly. It is not easy to make it work for all
> CPUs, since coreboot has halted them and we would need to start them up
> again, but it is easy enough to make them work on the boot CPU.
>
> Update the code to avoid assuming that the MP init routine has completed,
> so that this can work.
>
> Signed-off-by: Simon Glass 
> ---
>
> (no changes since v1)
>
>  arch/x86/cpu/mp_init.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
> index 0f99a405bb..21fbe5bda5 100644
> --- a/arch/x86/cpu/mp_init.c
> +++ b/arch/x86/cpu/mp_init.c
> @@ -558,7 +558,7 @@ static int get_bsp(struct udevice **devp, int *cpu_countp)
> if (cpu_countp)
> *cpu_countp = ret;
>
> -   return dev->req_seq;
> +   return dev->req_seq >= 0 ? dev->req_seq : 0;

I believe this change was already squashed into the part B series,
hence this does not apply on top of u-boot-x86/master.

>  }
>
>  /**
> @@ -718,9 +718,6 @@ int mp_run_on_cpus(int cpu_select, mp_run_func func, void 
> *arg)
> int num_cpus;
> int ret;
>
> -   if (!(gd->flags & GD_FLG_SMP_READY))
> -   return -ENXIO;
> -
> ret = get_bsp(&dev, &num_cpus);
> if (ret < 0)
> return log_msg_ret("bsp", ret);
> @@ -730,6 +727,13 @@ int mp_run_on_cpus(int cpu_select, mp_run_func func, 
> void *arg)
> func(arg);
> }
>
> +   if (!(gd->flags & GD_FLG_SMP_READY)) {
> +   /* Allow use of this function on the BSP only */
> +   if (cpu_select == MP_SELECT_BSP || !cpu_select)
> +   return 0;
> +   return -ENXIO;
> +   }
> +
> /* Allow up to 1 second for all APs to finish */
> ret = run_ap_work(&lcb, dev, num_cpus, 1000 /* ms */);
> if (ret)

Reviewed-by: Bin Meng 

Regards,
Bin


Re: [PATCH v2 37/44] x86: apl: Adjust FSP-M code to avoid hard-coded address

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:34 AM Simon Glass  wrote:
>
> Update this code to calculate the address to use, rather than hard-coding
> it. Obtain the requested stack size from the FSP.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v2:
> - Split out the boot_mode change into a separate patch
>
>  arch/x86/cpu/apollolake/fsp_m.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 36/44] x86: apl: Set the correct boot mode in the FSP-M code

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:34 AM Simon Glass  wrote:
>
> If there is MRC information we should run FSP-M with a different
> boot_mode flag since it is supposed to do a 'fast path' through the
> memory init. Fix this.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v2:
> - Add a new commit to handle the boot_mode fix
>
>  arch/x86/cpu/apollolake/fsp_m.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 32/44] pmc: Move common registers to the header file

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:34 AM Simon Glass  wrote:
>
> These registers need to be accesses from ACPI code, so move them to the
> header file.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> (no changes since v1)
>
>  drivers/power/acpi_pmc/acpi-pmc-uclass.c |  9 -
>  include/power/acpi_pmc.h | 14 ++
>  2 files changed, 14 insertions(+), 9 deletions(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 34/44] x86: apl: Fix save/restore of ITSS priorities

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:34 AM Simon Glass  wrote:
>
> The FSP-S changes the ITSS priorities. The code that tries to save it
> before running FSP-S and restore it afterwards does not work as U-Boot
> relocates in between the save and restore. This means that the driver
> data saved before relocation is lost and the new driver just sees zeroes.
>
> Fix this by allocating space in the relocated memory for the ITSS data.
> Save it there and access it from the driver after relocation.
>
> This fixes interrupt handling on coral.
>
> Also drop the log_msg_ret() in irq_first_device_type() since this function
> can be called speculatively in places where we are not sure if there is
> an interrupt controller of that type. The resulting log errors are
> confusing when there is no error.
>
> Signed-off-by: Simon Glass 
> ---
>
> Changes in v2:
> - Add mention of why log_msg_ret() is dropped
>
>  arch/x86/cpu/apollolake/fsp_s.c| 11 +--
>  arch/x86/cpu/cpu.c | 13 +
>  arch/x86/cpu/intel_common/itss.c   | 25 +++--
>  arch/x86/include/asm/global_data.h |  1 +
>  arch/x86/include/asm/itss.h|  2 +-
>  drivers/misc/irq-uclass.c  |  2 +-
>  6 files changed, 44 insertions(+), 10 deletions(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 28/44] i2c: designware_i2c: Support ACPI table generation

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 7:11 PM Wolfgang Wallner
 wrote:
>
> Hi Simon,
>
> -"Simon Glass"  schrieb: -
> > Betreff: [PATCH v2 28/44] i2c: designware_i2c: Support ACPI table generation
> >
> > Update the PCI driver to generate ACPI information so that Linux has the
> > full information about each I2C bus.
> >
> > Signed-off-by: Simon Glass 
> >
> > Reviewed-by: Heiko Schocher 
> > ---
> >
> > Changes in v2:
> > - Add a few blank lines
> > - Drop dead code behind if (0)
> >
> > Changes in v1:
> > - Capitalise ACPI_OPS_PTR
> >
> >  drivers/i2c/designware_i2c.c | 26 +
> >  drivers/i2c/designware_i2c.h | 15 +
> >  drivers/i2c/designware_i2c_pci.c | 96 +++-
> >  3 files changed, 136 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c
> > index 44a1f33398..cf892c69d9 100644
> > --- a/drivers/i2c/designware_i2c.c
> > +++ b/drivers/i2c/designware_i2c.c
> > @@ -333,6 +333,32 @@ static int _dw_i2c_set_bus_speed(struct dw_i2c *priv, 
> > struct i2c_regs *i2c_base,
> >   /* Restore back i2c now speed set */
> >   if (ena == IC_ENABLE_0B)
> >   dw_i2c_enable(i2c_base, true);
> > + if (priv)
> > + priv->config = config;
> > +
> > + return 0;
> > +}
> > +
> > +int dw_i2c_gen_speed_config(const struct udevice *dev, int speed_hz,
> > + struct dw_i2c_speed_config *config)
> > +{
> > + struct dw_i2c *priv = dev_get_priv(dev);
> > + ulong rate;
> > + int ret;
> > +
> > +#if CONFIG_IS_ENABLED(CLK)
> > + rate = clk_get_rate(&priv->clk);
> > + if (IS_ERR_VALUE(rate))
> > + return log_msg_ret("clk", -EINVAL);
> > +#else
> > + rate = IC_CLK;
> > +#endif
> > +
> > + ret = calc_bus_speed(priv, priv->regs, speed_hz, rate, config);
> > + if (ret)
> > + printf("%s: ret=%d\n", __func__, ret);
> > + if (ret)
> > + return log_msg_ret("calc_bus_speed", ret);
> >
> >   return 0;
> >  }
> > diff --git a/drivers/i2c/designware_i2c.h b/drivers/i2c/designware_i2c.h
> > index dc9a6ccb63..d87a3bff93 100644
> > --- a/drivers/i2c/designware_i2c.h
> > +++ b/drivers/i2c/designware_i2c.h
> > @@ -205,6 +205,7 @@ struct dw_i2c {
> >  #if CONFIG_IS_ENABLED(CLK)
> >   struct clk clk;
> >  #endif
> > + struct dw_i2c_speed_config config;
> >  };
> >
> >  extern const struct dm_i2c_ops designware_i2c_ops;
> > @@ -213,4 +214,18 @@ int designware_i2c_probe(struct udevice *bus);
> >  int designware_i2c_remove(struct udevice *dev);
> >  int designware_i2c_ofdata_to_platdata(struct udevice *bus);
> >
> > +/**
> > + * dw_i2c_gen_speed_config() - Calculate config info from requested speed1
>
> Nit: is "speed1" a typo?

I think so. Will fix it when applying.

>
> > + *
> > + * Calculate the speed config from the given @speed_hz and return it so 
> > that
> > + * it can be incorporated in ACPI tables
> > + *
> > + * @dev: I2C bus to check
> > + * @speed_hz: Requested speed in Hz
> > + * @config: Returns config to use for that speed
> > + * @return 0 if OK, -ve on error
> > + */
> > +int dw_i2c_gen_speed_config(const struct udevice *dev, int speed_hz,
> > + struct dw_i2c_speed_config *config);
> > +
> >  #endif /* __DW_I2C_H_ */
> > diff --git a/drivers/i2c/designware_i2c_pci.c 
> > b/drivers/i2c/designware_i2c_pci.c
> > index bd34ec0b47..d0d869c81a 100644
> > --- a/drivers/i2c/designware_i2c_pci.c
> > +++ b/drivers/i2c/designware_i2c_pci.c
> > @@ -9,7 +9,12 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> >  #include 
> > +#include 
> > +#include 
> > +#include 
> >  #include "designware_i2c.h"

Regards,
Bin


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

2020-07-12 Thread Bin Meng
Hi Simon,

On Mon, Jul 13, 2020 at 12:22 PM Bin Meng  wrote:
>
> On Wed, Jul 8, 2020 at 3:12 AM Simon Glass  wrote:
> >
> > Add a function to write a SPI descriptor to the generated ACPI code.
> >
> > Signed-off-by: Simon Glass 
> > Reviewed-by: Wolfgang Wallner 
> > Reviewed-by: Bin Meng 
> > ---
> >
> > 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 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >
> >  #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 
> > +#include 
> >  #include 
> >
> >  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_ID1 /* TODO: upgrade 
> > to 2 */
> >  #define ACPI_I2C_TYPE_SPECIFIC_REVISION_ID 1
> > +#define ACPI_SPI_SERIAL_BUS_REVISION_ID1
> > +#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 
> >
> >  /* SPI mode flags */
> > -#define SPI_CPHA   BIT(0)  /* clock phase */
> > -#define SPI_CPOL   BIT(1)  /* clock polarity */
> > +#define SPI_

Re: [PATCH v2 21/44] x86: pinctrl: Set up itss in the probe() method

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:33 AM Simon Glass  wrote:
>
> At present the itss is probed in the ofdata_to_platdata() method. This is
> incorrect since itss is a child of p2sb which itself needs to probe the
> pinctrl device. This means that p2sb is effectively not probed when the
> itss is probed, so we get the wrong register address from p2sb.
>
> Fix this by moving the itss probe to the correct place.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Bin Meng 
> ---
>
> (no changes since v1)
>
>  drivers/pinctrl/intel/pinctrl.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 22/44] x86: pinctrl: Drop the acpi_path member

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:34 AM Simon Glass  wrote:
>
> This is in the device tree now, so drop the unnecessary field here.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v2:
> - Fix the commit subject to mention dropping acpi_path, not acpi_name
>
>  arch/x86/include/asm/intel_pinctrl.h | 2 --
>  drivers/pinctrl/intel/pinctrl_apl.c  | 4 
>  2 files changed, 6 deletions(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 25/44] x86: gpio: Add support for obtaining ACPI info for a GPIO

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:34 AM Simon Glass  wrote:
>
> Implement the method that converts a GPIO into the form used by ACPI, so
> that GPIOs can be added to ACPI tables.
>
> Signed-off-by: Simon Glass 
> ---
>
> (no changes since v1)
>
> Changes in v1:
> - Use acpi_get_path() to get device path
>
>  drivers/gpio/intel_gpio.c | 34 ++
>  1 file changed, 34 insertions(+)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 19/44] x86: pinctrl: Update comment for intel_pinctrl_get_pad()

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:33 AM Simon Glass  wrote:
>
> Add information about what is returned on error.
>
> Signed-off-by: Simon Glass 
> ---
>
> (no changes since v1)
>
>  arch/x86/include/asm/intel_pinctrl.h | 1 +
>  1 file changed, 1 insertion(+)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 11/44] acpi: mmc: Generate ACPI info for the PCI SD Card

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:33 AM Simon Glass  wrote:
>
> Write required information into the SSDT to describe the SD card
> card-detect pin. Since the required GPIO properties are not present in
> the device-tree binding, set them manually for now.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> (no changes since v1)
>
> Changes in v1:
> - Capitalise ACPI_OPS_PTR
>
>  configs/sandbox_defconfig |  2 +
>  drivers/mmc/pci_mmc.c | 78 ++-
>  2 files changed, 79 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 12/44] x86: Add bindings for NHLT

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:33 AM Simon Glass  wrote:
>
> Add devicetree bindings for the Intel Non-High-Definition-Audio Link Table
> (NHLT).
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v2:
> - Add a comment pointing to the PCI spec
>
>  include/dt-bindings/sound/nhlt.h | 24 
>  1 file changed, 24 insertions(+)
>  create mode 100644 include/dt-bindings/sound/nhlt.h
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 08/44] acpi: Export functions to write sized values

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:33 AM Simon Glass  wrote:
>
> At present only acpigen_write_integer() is exported for use by other code.
> But in some cases it is useful to call the specific function depending on
> the size of the value.
>
> Export these functions and add a test.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> (no changes since v1)
>
>  include/acpi/acpigen.h | 46 ++
>  test/dm/acpigen.c  | 45 -
>  2 files changed, 90 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 09/44] acpi: Support generation of a scope

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:33 AM Simon Glass  wrote:
>
> Add a function to write a scope to the generated ACPI code.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v2:
> - Rename parameter from 'name' to 'scope'
>
>  include/acpi/acpigen.h |  9 +
>  lib/acpi/acpigen.c |  7 +++
>  test/dm/acpi.c |  3 +--
>  test/dm/acpigen.c  | 30 ++
>  4 files changed, 47 insertions(+), 2 deletions(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 03/44] binman: Add way to locate an entry in memory

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:33 AM Simon Glass  wrote:
>
> Add support for accessing an entry's contents in memory-mapped SPI flash.
>
> Signed-off-by: Simon Glass 
> ---
>
> (no changes since v1)
>
>  include/binman.h | 22 ++
>  lib/binman.c | 23 +++
>  2 files changed, 45 insertions(+)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 02/44] binman: Refactor binman_entry_find() to allow other nodes

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:33 AM Simon Glass  wrote:
>
> At present we can only read from a top-level binman node entry. Refactor
> this function to produce a second local function which supports reading
> from any node.
>
> Signed-off-by: Simon Glass 
> ---
>
> Changes in v2:
> - Rename binman_entry_find_()
>
>  lib/binman.c | 19 +--
>  1 file changed, 13 insertions(+), 6 deletions(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v2 01/44] binman: Allow setting the ROM offset

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 11:33 AM Simon Glass  wrote:
>
> On x86 the SPI ROM can be memory-mapped, at least most of it. Add a way
> to tell binman the offset from a ROM address to a RAM address.
>
> Signed-off-by: Simon Glass 
> ---
>
> (no changes since v1)
>
> Changes in v1:
> - Add a way to set the binman ROM offset
>
>  include/binman.h |  8 
>  lib/binman.c | 17 +
>  2 files changed, 25 insertions(+)
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 29/35] acpi: Support ordering SSDT data by device

2020-07-12 Thread Bin Meng
Hi Simon,

On Wed, Jul 8, 2020 at 3:13 AM Simon Glass  wrote:
>
> Add a /chosen property to control the order in which the data appears
> in the SSDT. This allows matching up U-Boot's output from a dump of the
> known-good data obtained from within Linux.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Explain in sort_acpi_item_type() why ctx->current is not updated
>
> Changes in v3:
> - Make find_item() static and rename to find_acpi_item()
> - Rename build_type() and add a comment
>
> Changes in v1:
> - Generalise the ACPI function recursion with acpi_recurse_method()
>
>  arch/sandbox/dts/test.dts   |  5 +-
>  doc/device-tree-bindings/chosen.txt |  9 
>  drivers/core/acpi.c | 84 +
>  test/dm/acpi.c  | 15 +++---
>  4 files changed, 105 insertions(+), 8 deletions(-)
>
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index ea686f0cb3..6687efe2ae 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -254,7 +254,7 @@
> compatible = "denx,u-boot-devres-test";
> };
>
> -   acpi-test {
> +   acpi_test1: acpi-test {
> compatible = "denx,u-boot-acpi-test";
> acpi-ssdt-test-data = "ab";
> child {
> @@ -262,7 +262,7 @@
> };
> };
>
> -   acpi-test2 {
> +   acpi_test2: acpi-test2 {
> compatible = "denx,u-boot-acpi-test";
> acpi-ssdt-test-data = "cd";
> };
> @@ -904,6 +904,7 @@
> setting = "sunrise ohoka";
> other-node = "/some-bus/c-test@5";
> int-values = <0x1937 72993>;
> +   u-boot,acpi-ssdt-order = <&acpi_test2 &acpi_test1>;
> chosen-test {
> compatible = "denx,u-boot-fdt-test";
> reg = <9 1>;
> diff --git a/doc/device-tree-bindings/chosen.txt 
> b/doc/device-tree-bindings/chosen.txt
> index 395c9501e3..d4dfc05847 100644
> --- a/doc/device-tree-bindings/chosen.txt
> +++ b/doc/device-tree-bindings/chosen.txt
> @@ -134,3 +134,12 @@ Example
> phandlepart = <&mmc 1>;
> };
>  };
> +
> +u-boot,acpi-ssdt-order
> +--
> +
> +This provides the ordering to use when writing device data to the ACPI SSDT
> +(Secondary System Descriptor Table). Each cell is a phandle pointer to a 
> device
> +node to add. The ACPI information is written in this order.
> +
> +If the ordering does not include all nodes, an error is generated.
> diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> index df3d7ba417..a9b7fc1d9a 100644
> --- a/drivers/core/acpi.c
> +++ b/drivers/core/acpi.c
> @@ -108,6 +108,85 @@ static int acpi_add_item(struct acpi_ctx *ctx, struct 
> udevice *dev,
> return 0;
>  }
>
> +static struct acpi_item *find_acpi_item(const char *devname)
> +{
> +   int i;
> +
> +   for (i = 0; i < item_count; i++) {
> +   struct acpi_item *item = &acpi_item[i];
> +
> +   if (!strcmp(devname, item->dev->name))
> +   return item;
> +   }
> +
> +   return NULL;
> +}
> +
> +/**
> + * sort_acpi_item_type - Sort the ACPI items into the desired order
> + *
> + * This looks up the ordering in the device tree and then adds each item one 
> by
> + * one into the supplied buffer
> + *
> + * @ctx: ACPI context
> + * @start: Start position to put the sorted items. The items will follow each
> + * other in sorted order
> + * @type: Type of items to sort
> + * @return 0 if OK, -ve on error
> + */
> +static int sort_acpi_item_type(struct acpi_ctx *ctx, void *start,
> +  enum gen_type_t type)
> +{
> +   const u32 *order;
> +   int size;
> +   int count;
> +   void *ptr;
> +   void *end = ctx->current;
> +
> +   ptr = start;
> +   order = ofnode_read_chosen_prop("u-boot,acpi-ssdt-order", &size);
> +   if (!order) {
> +   log_warning("Failed to find ordering, leaving as is\n");

This warning is showing 4 times when booting to U-Boot shell on
Minnowmax. I think we should suppress it by default.

If you agree, please send a version of this patch only, so I can apply
it in u-boot-x86.

> +   return 0;
> +   }
> +
> +   /*
> +* This algorithm rewrites the context buffer without changing its
> +* length. So there is no need to update ctx-current
> +*/


Regards,
Bin


Re: [PATCH v4 25/25] x86: mtrr: Enhance 'mtrr' command to list MTRRs on any CPU

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 9:37 AM Simon Glass  wrote:
>
> Update this command so it can list the MTRRs on a selected CPU. If
> '-c all' is used, then all CPUs are listed.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> (no changes since v1)
>
>  cmd/x86/mtrr.c | 22 +-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng 
Tested-by: Bin Meng 


Re: [PATCH v4 19/25] x86: mtrr: Update MTRRs on all CPUs

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 9:37 AM Simon Glass  wrote:
>
> When the boot CPU MTRRs are updated, perform the same update on all other
> CPUs so they are kept in sync.
>
> This avoids kernel warnings about mismatched MTRRs.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Rename function to mtrr_write_all()
>
>  arch/x86/cpu/mtrr.c | 57 +
>  1 file changed, 57 insertions(+)
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 15/25] x86: mp: Add iterators for CPUs

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 9:37 AM Simon Glass  wrote:
>
> It is convenient to iterate through the CPUs performing work on each one
> and processing the result. Add a few iterator functions which handle this.
> These can be used by any client code. It can call mp_run_on_cpus() on
> each CPU that is returned, handling them one at a time.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Update mp_next_cpu() to stop if CONFIG_SMP_AP_WORK is not enabled
>
> Changes in v3:
> - Add more comments on how the iterators work
>
>  arch/x86/cpu/mp_init.c| 63 +++
>  arch/x86/include/asm/mp.h | 42 ++
>  2 files changed, 105 insertions(+)
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 13/25] x86: mp: Allow running functions on multiple CPUs

2020-07-12 Thread Bin Meng
Hi Simon,

On Wed, Jul 8, 2020 at 9:37 AM Simon Glass  wrote:
>
> Add a way to run a function on a selection of CPUs. This supports either
> a single CPU, all CPUs, just the main CPU or just the 'APs', in Intel
> terminology.
>
> It works by writing into a mailbox and then waiting for the CPUs to notice
> it, take action and indicate they are done.
>
> When SMP is not yet enabled, this just calls the function on the main CPU.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Only enable this feature of CONFIG_SMP_AP_WORK is enabled
> - Allow running on the BSP if SMP is not enabled
>
> Changes in v3:
> - Add a comment to run_ap_work()
> - Rename flag to GD_FLG_SMP_READY
> - Update the comment for run_ap_work() to explain logical_cpu_number
> - Clarify meaning of @cpu_select in mp_run_on_cpus() comment
>
>  arch/x86/cpu/mp_init.c| 107 +++---
>  arch/x86/include/asm/mp.h |  33 
>  2 files changed, 134 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/cpu/mp_init.c b/arch/x86/cpu/mp_init.c
> index 69a23829b9..dd6d6bfab7 100644
> --- a/arch/x86/cpu/mp_init.c
> +++ b/arch/x86/cpu/mp_init.c
> @@ -54,12 +54,7 @@ struct mp_flight_plan {
>   * callback
>   */
>  struct mp_callback {
> -   /**
> -* func() - Function to call on the AP
> -*
> -* @arg: Argument to pass
> -*/
> -   void (*func)(void *arg);
> +   mp_run_func func;
> void *arg;
> int logical_cpu_number;
>  };
> @@ -514,6 +509,70 @@ static void store_callback(struct mp_callback **slot, 
> struct mp_callback *val)
> dmb();
>  }
>
> +/**
> + * run_ap_work() - Run a callback on selected APs
> + *
> + * This writes @callback to all APs and waits for them all to acknowledge it,
> + * Note that whether each AP actually calls the callback depends on the value
> + * of logical_cpu_number (see struct mp_callback). The logical CPU number is
> + * the CPU device's req->seq value.
> + *
> + * @callback: Callback information to pass to all APs
> + * @bsp: CPU device for the BSP
> + * @num_cpus: The number of CPUs in the system (= number of APs + 1)
> + * @expire_ms: Timeout to wait for all APs to finish, in milliseconds, or 0 
> for
> + * no timeout
> + * @return 0 if OK, -ETIMEDOUT if one or more APs failed to respond in time
> + */
> +static int run_ap_work(struct mp_callback *callback, struct udevice *bsp,
> +  int num_cpus, uint expire_ms)
> +{
> +   int cur_cpu = bsp->req_seq;
> +   int num_aps = num_cpus - 1; /* number of non-BSPs to get this message 
> */
> +   int cpus_accepted;
> +   ulong start;
> +   int i;
> +
> +   if (!IS_ENABLED(CONFIG_SMP_AP_WORK)) {
> +   printf("APs already parked. CONFIG_SMP_AP_WORK not 
> enabled\n");
> +   return -ENOTSUPP;
> +   }
> +
> +   /* Signal to all the APs to run the func. */
> +   for (i = 0; i < num_cpus; i++) {
> +   if (cur_cpu != i)
> +   store_callback(&ap_callbacks[i], callback);
> +   }
> +   mfence();
> +
> +   /* Wait for all the APs to signal back that call has been accepted. */
> +   start = get_timer(0);
> +
> +   do {
> +   mdelay(1);
> +   cpus_accepted = 0;
> +
> +   for (i = 0; i < num_cpus; i++) {
> +   if (cur_cpu == i)
> +   continue;
> +   if (!read_callback(&ap_callbacks[i]))
> +   cpus_accepted++;

It looks my previous comments were not addressed. I believe there is a
bug here that cpus_accepted will double count for the 2nd time in the
do {} while () loop.

> +   }
> +
> +   if (expire_ms && get_timer(start) >= expire_ms) {
> +   log(UCLASS_CPU, LOGL_CRIT,
> +   "AP call expired; %d/%d CPUs accepted\n",
> +   cpus_accepted, num_aps);
> +   return -ETIMEDOUT;
> +   }
> +   } while (cpus_accepted != num_aps);
> +
> +   /* Make sure we can see any data written by the APs */
> +   mfence();
> +
> +   return 0;
> +}
> +
>  /**
>   * ap_wait_for_instruction() - Wait for and process requests from the main 
> CPU
>   *
> @@ -573,6 +632,42 @@ static struct mp_flight_record mp_steps[] = {
> MP_FR_BLOCK_APS(ap_wait_for_instruction, NULL, NULL, NULL),
>  };
>
> +int mp_run_on_cpus(int cpu_select, mp_run_func func, void *arg)
> +{
> +   struct mp_callback lcb = {
> +   .func = func,
> +   .arg = arg,
> +   .logical_cpu_number = cpu_select,
> +   };
> +   struct udevice *dev;
> +   int num_cpus;
> +   int ret;
> +
> +   ret = get_bsp(&dev, &num_cpus);
> +   if (ret < 0)
> +   return log_msg_ret("bsp", ret);
> +   if (cpu_select == MP_SELECT_ALL || cpu_select 

Re: [PATCH v4 11/25] global_data: Add a generic global_data flag for SMP state

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 9:37 AM Simon Glass  wrote:
>
> Allow keeping track of whether all CPUs have been enabled yet. This allows
> us to know whether other CPUs need to be considered when updating
> CPU-specific settings such as MTRRs on x86.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Rename flag to GD_FLG_SMP_READY
>
>  include/asm-generic/global_data.h | 1 +
>  1 file changed, 1 insertion(+)
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 10/25] x86: mp: Support APs waiting for instructions

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 9:37 AM Simon Glass  wrote:
>
> At present the APs (non-boot CPUs) are inited once and then parked ready
> for the OS to use them. However in some cases we want to send new requests
> through, such as to change MTRRs and keep them consistent across CPUs.
>
> Change the last state of the flight plan to go into a wait loop, accepting
> instructions from the main CPU.
>
> Drop cpu_map since it is not used.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Add a Kconfig to control this feature, enabled only on APL
>
> Changes in v3:
> - s/slow/slot/
> - Use C code instead of assembler to read/write callback pointers
> - Update commit message to mention dropping of cpu_map
>
> Changes in v2:
> - Add more comments
>
>  arch/x86/Kconfig|   7 ++
>  arch/x86/cpu/apollolake/Kconfig |   1 +
>  arch/x86/cpu/mp_init.c  | 123 +---
>  arch/x86/include/asm/mp.h   |  11 +++
>  4 files changed, 134 insertions(+), 8 deletions(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 00/35] dm: Add programmatic generation of ACPI tables (part B)

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 3:12 AM Simon Glass  wrote:
>
> This is split from the original series in an attempt to get things applied
> in chunks.
>
> This part includes:
> - writing basic ACPI code for integers, strings, names, packages
> - writing descriptors for GPIO, I2C, interrupts, SPI
> - writing code to enable/disable ACPI peripherals via GPIOs
> - writing SSDT and DSDT tables
> - additional ways to determine ACPI device names
>
> Much of this code is taken from coreboot and I have tried to avoid
> changing the original code for no reason. Changes include:
> - splitting the acpi_dp functions into their own file
> - adding tests
> - adding (lots of) comments
> - using a context pointer instead of global variables
> - tidying up some code where couldn't resist (e.g. acpigen_emit_namestring())
>
> Changes in v4:
> - Add a comment about sandbox to dm_test_irq_get_acpi()
> - Setup ctx->base as well as ctx->current in acpi_setup_base_tables()
> - Add a comment about little-endian in acpigen_emit_dword()
> - Add a #define for the context size in alloc_context()
> - Rename the length-writing functions to indicate they are for large resources
> - Use memset() to zero struct acpi_gpio in gpio_get_acpi()
> - Move common code about the if...else in sb_gpio_get_acpi()
> - Don't set zero fields in sb_gpio_get_acpi(), and add a comment about it
> - Update acpi_device_write_interrupt_irq() to return IRQ pin number
> - Use NULL instead of NUL and drop the 'NUL character' comment
> - Drop comment about the type always being ACPI_GPIO_TYPE_IO
> - Rename the length-writing functions to indicate they are for large resources
> - Update functions to return a GPIO pin number
> - Update functions to return GPIO/IRQ pin number
> - Move SPI macros to next patch
> - Rename the length-writing functions to indicate they are for large
> - Update functions to return GPIO/IRQ pin number
> - Move SPI macros to next patch
> - Rename the length-writing functions to indicate they are for large resources
> - Add a note about the hard-coded 3-byte length used
> - Fix 'gas' typo
> - Use 'null' instead of 'nul'
> - Use 'writing' instead of emitting in test comment
> - Use 'writing' instead of emitting in test comment
> - Change 'nul' to 'null' in commit message and acpigen_write_string()
> - Move emit->write comment changes to previous patches
> - s/nul/null/ in the comment
> - Drop embedded // comments in file comment
> - Correct return value comment in acpi_dp_add_child()
> - Rename acpi_dp_write_() to acpi_dp_write_internal() and make static
> - Use a #define for the test context size
> - Add a header file for test to allow sharing the context init function
> - Rename and get_length() into a shared test file
> - Add missing arg to comment for acpi_dp_write()
> - Correct reference to device tree in the commit message
> - Squash in comment change to an earlier patch
> - Rename acpigen_write_method_()
> - Use acpi_test_get_length() instead of get_length()
> - Use acpi_test_get_length() instead of get_length()
> - Update functions to return GPIO/IRQ pin number
> - Use a separate variables for reading and writing DW0
> - Correct bug in acpigen_set_gpio_val() by putting tx_state_val in LOCAL0
> - Fix up the comment for acpigen_set_enable_tx_gpio()
> - Explain access to DW0 register a bit better and provide a sample
> - Expand the comments for acpigen_get_dw0_in_local5()
> - Fix 'diabl' typo
> - Use new shared acpi_test_alloc_context_size() in test
> - Update functions to return GPIO/IRQ pin number
> - Use a separate variables for reading and writing DW0
> - Explain in sort_acpi_item_type() why ctx->current is not updated
> - Explain why 'fill' is used for one method and 'inject' for another
> - Mention that acpi_device_infer_name() only supports x86
> - Add a comment about only supporting USB3.0
>

series applied to u-boot-x86, thanks!


Re: [PATCH v4 09/35] acpi: Support generation of GPIO descriptor

2020-07-12 Thread Bin Meng
On Mon, Jul 13, 2020 at 10:47 AM Bin Meng  wrote:
>
> Hi Simon,
>
> On Wed, Jul 8, 2020 at 3:12 AM Simon Glass  wrote:
> >
> > Add a function to write a GPIO descriptor to the generated ACPI code.
> >
> > Signed-off-by: Simon Glass 
> > Reviewed-by: Wolfgang Wallner 
> > ---
> >
> > Changes in v4:
> > - Drop comment about the type always being ACPI_GPIO_TYPE_IO
> > - Rename the length-writing functions to indicate they are for large 
> > resources
> > - Update functions to return a GPIO pin number
> >
> >  include/acpi/acpi_device.h |  22 ++
> >  lib/acpi/acpi_device.c | 151 +
> >  test/dm/acpigen.c  |  91 ++
> >  3 files changed, 264 insertions(+)
> >
> > diff --git a/include/acpi/acpi_device.h b/include/acpi/acpi_device.h
> > index 69b90968a8..18063e977f 100644
> > --- a/include/acpi/acpi_device.h
> > +++ b/include/acpi/acpi_device.h
> > @@ -12,6 +12,7 @@
> >  #include 
> >
> >  struct acpi_ctx;
> > +struct gpio_desc;
> >  struct irq;
> >  struct udevice;
> >
> > @@ -233,4 +234,25 @@ enum acpi_dev_status acpi_device_status(const struct 
> > udevice *dev);
> >  int acpi_device_write_interrupt_irq(struct acpi_ctx *ctx,
> > const struct irq *req_irq);
> >
> > +/**
> > + * acpi_device_write_gpio() - Write GpioIo() or GpioInt() descriptor
> > + *
> > + * @gpio: GPIO information to write
> > + * @return GPIO pin number of first GPIO if OK, -ve on error
> > + */
> > +int acpi_device_write_gpio(struct acpi_ctx *ctx, const struct acpi_gpio 
> > *gpio);
> > +
> > +/**
> > + * acpi_device_write_gpio_desc() - Write a GPIO to ACPI
> > + *
> > + * This creates a GPIO descriptor for a GPIO, including information ACPI 
> > needs
> > + * to use it. The type is always ACPI_GPIO_TYPE_IO.
>
> This comment is not dropped ?
>

Dropped this comment when applying

> > + *
> > + * @ctx: ACPI context pointer
> > + * @desc: GPIO to write
> > + * @return 0 if OK, -ve on error
> > + */
> > +int acpi_device_write_gpio_desc(struct acpi_ctx *ctx,
> > +   const struct gpio_desc *desc);
> > +
> >  #endif
> > diff --git a/lib/acpi/acpi_device.c b/lib/acpi/acpi_device.c
> > index d854a45cbc..bbe1cfc57a 100644
> > --- a/lib/acpi/acpi_device.c
> > +++ b/lib/acpi/acpi_device.c
> > @@ -12,6 +12,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >
> >  /**
> > @@ -203,5 +204,155 @@ int acpi_device_write_interrupt_irq(struct acpi_ctx 
> > *ctx,
> > if (ret)
> > return log_msg_ret("write", ret);
> >
> > +   return irq.pin;
> > +}
> > +
> > +/* ACPI 6.3 section 6.4.3.8.1 - GPIO Interrupt or I/O */
> > +int acpi_device_write_gpio(struct acpi_ctx *ctx, const struct acpi_gpio 
> > *gpio)
> > +{
> > +   void *start, *desc_length;
> > +   void *pin_table_offset, *vendor_data_offset, *resource_offset;
> > +   u16 flags = 0;
> > +   int pin;
> > +
> > +   if (gpio->type > ACPI_GPIO_TYPE_IO)
> > +   return -EINVAL;
> > +
> > +   start = acpigen_get_current(ctx);
> > +
> > +   /* Byte 0: Descriptor Type */
> > +   acpigen_emit_byte(ctx, ACPI_DESCRIPTOR_GPIO);
> > +
> > +   /* Byte 1-2: Length (fill in later) */
> > +   desc_length = largeres_write_len_f(ctx);
> > +
> > +   /* Byte 3: Revision ID */
> > +   acpigen_emit_byte(ctx, ACPI_GPIO_REVISION_ID);
> > +
> > +   /* Byte 4: GpioIo or GpioInt */
> > +   acpigen_emit_byte(ctx, gpio->type);
> > +
> > +   /*
> > +* Byte 5-6: General Flags
> > +*   [15:1]: 0 => Reserved
> > +*  [0]: 1 => ResourceConsumer
> > +*/
> > +   acpigen_emit_word(ctx, 1 << 0);
> > +
> > +   switch (gpio->type) {
> > +   case ACPI_GPIO_TYPE_INTERRUPT:
> > +   /*
> > +* Byte 7-8: GPIO Interrupt Flags
> > +*   [15:5]: 0 => Reserved
> > +*  [4]: Wake (0=NO_WAKE   1=WAKE)
> > +*  [3]: Sharing  (0=EXCLUSIVE 1=SHARED)
> > +*[2:1]: Polarity (0=HIGH  1=LOW 2=BOTH)
> > +*  [0]: Mode (0=LEVEL 1=EDGE)
> > +*/
> > +   if (gpio->irq.mode == ACPI_IRQ_EDGE_TRIGGERED)
> > +   flags |= 1 << 0;
> > +   if (gpio->irq.shared == ACPI_IRQ_SHARED)
> > +   flags |= 1 << 3;
> > +   if (gpio->irq.wake == ACPI_IRQ_WAKE)
> > +   flags |= 1 << 4;
> > +
> > +   switch (gpio->irq.polarity) {
> > +   case ACPI_IRQ_ACTIVE_HIGH:
> > +   flags |= 0 << 1;
> > +   break;
> > +   case ACPI_IRQ_ACTIVE_LOW:
> > +   flags |= 1 << 1;
> > +   break;
> > +   case ACPI_IRQ_ACTIVE_BOTH:
> > +   flags |= 2 << 1;
> > +   break;
> > +   }
> > +  

Re: [PATCH v4 31/35] acpi: Add support for DSDT generation

2020-07-12 Thread Bin Meng
On Mon, Jul 13, 2020 at 11:17 AM Bin Meng  wrote:
>
> On Wed, Jul 8, 2020 at 3:13 AM Simon Glass  wrote:
> >
> > Some devices need to inject extra code into the Differentiated System
> > Descriptor Table (DSDT). Add a method to handle this.
> >
> > Signed-off-by: Simon Glass 
> > Reviewed-by: Wolfgang Wallner 
> > ---
> >
> > Changes in v4:
> > - Explain why 'fill' is used for one method and 'inject' for another
> >
> > Changes in v3:
> > - Fix 'THe' typo
> > - Rename build_type() to sort_acpi_item_type()
> >
> > Changes in v1:
> > - Generalise the ACPI function recursion with acpi_recurse_method()
> >
> >  arch/sandbox/dts/test.dts |  2 ++
> >  drivers/core/acpi.c   | 25 +-
> >  include/dm/acpi.h | 30 ++
> >  test/dm/acpi.c| 44 +++
> >  4 files changed, 100 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> > index 6687efe2ae..fc6a6b1774 100644
> > --- a/arch/sandbox/dts/test.dts
> > +++ b/arch/sandbox/dts/test.dts
> > @@ -257,6 +257,7 @@
> > acpi_test1: acpi-test {
> > compatible = "denx,u-boot-acpi-test";
> > acpi-ssdt-test-data = "ab";
> > +   acpi-dsdt-test-data = "hi";
> > child {
> > compatible = "denx,u-boot-acpi-test";
> > };
> > @@ -265,6 +266,7 @@
> > acpi_test2: acpi-test2 {
> > compatible = "denx,u-boot-acpi-test";
> > acpi-ssdt-test-data = "cd";
> > +   acpi-dsdt-test-data = "jk";
> > };
> >
> > clocks {
> > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > index a9b7fc1d9a..7b32694ad4 100644
> > --- a/drivers/core/acpi.c
> > +++ b/drivers/core/acpi.c
> > @@ -22,12 +22,14 @@
> >  enum gen_type_t {
> > TYPE_NONE,
> > TYPE_SSDT,
> > +   TYPE_DSDT,
> >  };
> >
> >  /* Type of method to call */
> >  enum method_t {
> > METHOD_WRITE_TABLES,
> > METHOD_FILL_SSDT,
> > +   METHOD_INJECT_DSDT,
> >  };
> >
> >  /* Prototype for all methods */
> > @@ -144,7 +146,9 @@ static int sort_acpi_item_type(struct acpi_ctx *ctx, 
> > void *start,
> > void *end = ctx->current;
> >
> > ptr = start;
> > -   order = ofnode_read_chosen_prop("u-boot,acpi-ssdt-order", &size);
> > +   order = ofnode_read_chosen_prop(type == TYPE_DSDT ?
> > +   "u-boot,acpi-dsdt-order" :
> > +   "u-boot,acpi-ssdt-order", &size);
> > if (!order) {
> > log_warning("Failed to find ordering, leaving as is\n");
> > return 0;
> > @@ -198,6 +202,8 @@ acpi_method acpi_get_method(struct udevice *dev, enum 
> > method_t method)
> > return aops->write_tables;
> > case METHOD_FILL_SSDT:
> > return aops->fill_ssdt;
> > +   case METHOD_INJECT_DSDT:
> > +   return aops->inject_dsdt;
> > }
> > }
> >
> > @@ -256,6 +262,23 @@ int acpi_fill_ssdt(struct acpi_ctx *ctx)
> > return ret;
> >  }
> >
> > +int acpi_inject_dsdt(struct acpi_ctx *ctx)
> > +{
> > +   void *start = ctx->current;
> > +   int ret;
> > +
> > +   log_debug("Writing DSDT tables\n");
> > +   item_count = 0;
> > +   ret = acpi_recurse_method(ctx, dm_root(), METHOD_INJECT_DSDT,
> > + TYPE_DSDT);
> > +   log_debug("Writing DSDT finished, err=%d\n", ret);
> > +   ret = sort_acpi_item_type(ctx, start, TYPE_DSDT);
> > +   if (ret)
> > +   return log_msg_ret("build", ret);
> > +
> > +   return ret;
> > +}
> > +
> >  int acpi_write_dev_tables(struct acpi_ctx *ctx)
> >  {
> > int ret;
> > diff --git a/include/dm/acpi.h b/include/dm/acpi.h
> > index e956e49680..72fa71d0a2 100644
> > --- a/include/dm/acpi.h
> > +++ b/include/dm/acpi.h
> > @@ -82,11 +82,31 @@ struct acpi_ops {
> >  * whatever ACPI code is needed by this device. It will end up in 
> > the
> >  * SSDT table.
> >  *
> > +* Note that this is called 'fill' because the entire contents of 
> > the
> > +* SSDT is build by calling this method on all devices.
> > +*
> >  * @dev: Device to write
> >  * @ctx: ACPI context to use
> >  * @return 0 if OK, -ve on error
> >  */
> > int (*fill_ssdt)(const struct udevice *dev, struct acpi_ctx *ctx);
> > +
> > +   /**
> > +* inject_dsdt() - Generate DSDT code for a device
> > +*
> > +* This is called to create the DSDT code. The method should write 
> > out
> > +* whatever ACPI code is needed by this device. It will end up in 
> > the
> > +* DSDT table.
> > +*
> > +* Note that this is called 'inject' because the output of calling 
> > this

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

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 3:12 AM Simon Glass  wrote:
>
> Add a function to write a SPI descriptor to the generated ACPI code.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> Reviewed-by: Bin Meng 
> ---
>
> 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 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  #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 
> +#include 
>  #include 
>
>  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_ID1 /* TODO: upgrade to 
> 2 */
>  #define ACPI_I2C_TYPE_SPECIFIC_REVISION_ID 1
> +#define ACPI_SPI_SERIAL_BUS_REVISION_ID1
> +#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 
>
>  /* 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

Re: [PATCH v4 34/35] dm: acpi: Enhance acpi_get_name()

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 3:13 AM Simon Glass  wrote:
>
> For many device types it is possible to figure out the name just by
> looking at its uclass or parent. Add a function to handle this, since it
> allows us to cover the vast majority of cases automatically.
>
> However it is sometimes impossible to figure out an ACPI name for a device
> just by looking at its uclass. For example a touch device may have a
> vendor-specific name. Add a new "acpi,name" property to allow a custom
> name to be created.
>
> With this new feature we can drop the get_name() methods in the sandbox
> I2C and SPI drivers. They were only added for testing purposes. Update the
> tests to use the new values.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Mention that acpi_device_infer_name() only supports x86
> - Add a comment about only supporting USB3.0
>
> Changes in v3:
> - Fix 'of' typo
>
> Changes in v1:
> - Use acpi,ddn instead of acpi,desc
> - Rename to acpi_device_infer_name()
> - Update newly created sandbox tests
>
>  arch/sandbox/dts/test.dts   |   1 +
>  doc/device-tree-bindings/device.txt |  13 
>  drivers/core/acpi.c |  13 +++-
>  drivers/i2c/sandbox_i2c.c   |  10 ---
>  drivers/spi/sandbox_spi.c   |  10 ---
>  include/acpi/acpi_device.h  |  18 +
>  lib/acpi/acpi_device.c  | 106 
>  test/dm/acpi.c  |  42 ++-
>  8 files changed, 190 insertions(+), 23 deletions(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 31/35] acpi: Add support for DSDT generation

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 3:13 AM Simon Glass  wrote:
>
> Some devices need to inject extra code into the Differentiated System
> Descriptor Table (DSDT). Add a method to handle this.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Explain why 'fill' is used for one method and 'inject' for another
>
> Changes in v3:
> - Fix 'THe' typo
> - Rename build_type() to sort_acpi_item_type()
>
> Changes in v1:
> - Generalise the ACPI function recursion with acpi_recurse_method()
>
>  arch/sandbox/dts/test.dts |  2 ++
>  drivers/core/acpi.c   | 25 +-
>  include/dm/acpi.h | 30 ++
>  test/dm/acpi.c| 44 +++
>  4 files changed, 100 insertions(+), 1 deletion(-)
>
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index 6687efe2ae..fc6a6b1774 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -257,6 +257,7 @@
> acpi_test1: acpi-test {
> compatible = "denx,u-boot-acpi-test";
> acpi-ssdt-test-data = "ab";
> +   acpi-dsdt-test-data = "hi";
> child {
> compatible = "denx,u-boot-acpi-test";
> };
> @@ -265,6 +266,7 @@
> acpi_test2: acpi-test2 {
> compatible = "denx,u-boot-acpi-test";
> acpi-ssdt-test-data = "cd";
> +   acpi-dsdt-test-data = "jk";
> };
>
> clocks {
> diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> index a9b7fc1d9a..7b32694ad4 100644
> --- a/drivers/core/acpi.c
> +++ b/drivers/core/acpi.c
> @@ -22,12 +22,14 @@
>  enum gen_type_t {
> TYPE_NONE,
> TYPE_SSDT,
> +   TYPE_DSDT,
>  };
>
>  /* Type of method to call */
>  enum method_t {
> METHOD_WRITE_TABLES,
> METHOD_FILL_SSDT,
> +   METHOD_INJECT_DSDT,
>  };
>
>  /* Prototype for all methods */
> @@ -144,7 +146,9 @@ static int sort_acpi_item_type(struct acpi_ctx *ctx, void 
> *start,
> void *end = ctx->current;
>
> ptr = start;
> -   order = ofnode_read_chosen_prop("u-boot,acpi-ssdt-order", &size);
> +   order = ofnode_read_chosen_prop(type == TYPE_DSDT ?
> +   "u-boot,acpi-dsdt-order" :
> +   "u-boot,acpi-ssdt-order", &size);
> if (!order) {
> log_warning("Failed to find ordering, leaving as is\n");
> return 0;
> @@ -198,6 +202,8 @@ acpi_method acpi_get_method(struct udevice *dev, enum 
> method_t method)
> return aops->write_tables;
> case METHOD_FILL_SSDT:
> return aops->fill_ssdt;
> +   case METHOD_INJECT_DSDT:
> +   return aops->inject_dsdt;
> }
> }
>
> @@ -256,6 +262,23 @@ int acpi_fill_ssdt(struct acpi_ctx *ctx)
> return ret;
>  }
>
> +int acpi_inject_dsdt(struct acpi_ctx *ctx)
> +{
> +   void *start = ctx->current;
> +   int ret;
> +
> +   log_debug("Writing DSDT tables\n");
> +   item_count = 0;
> +   ret = acpi_recurse_method(ctx, dm_root(), METHOD_INJECT_DSDT,
> + TYPE_DSDT);
> +   log_debug("Writing DSDT finished, err=%d\n", ret);
> +   ret = sort_acpi_item_type(ctx, start, TYPE_DSDT);
> +   if (ret)
> +   return log_msg_ret("build", ret);
> +
> +   return ret;
> +}
> +
>  int acpi_write_dev_tables(struct acpi_ctx *ctx)
>  {
> int ret;
> diff --git a/include/dm/acpi.h b/include/dm/acpi.h
> index e956e49680..72fa71d0a2 100644
> --- a/include/dm/acpi.h
> +++ b/include/dm/acpi.h
> @@ -82,11 +82,31 @@ struct acpi_ops {
>  * whatever ACPI code is needed by this device. It will end up in the
>  * SSDT table.
>  *
> +* Note that this is called 'fill' because the entire contents of the
> +* SSDT is build by calling this method on all devices.
> +*
>  * @dev: Device to write
>  * @ctx: ACPI context to use
>  * @return 0 if OK, -ve on error
>  */
> int (*fill_ssdt)(const struct udevice *dev, struct acpi_ctx *ctx);
> +
> +   /**
> +* inject_dsdt() - Generate DSDT code for a device
> +*
> +* This is called to create the DSDT code. The method should write out
> +* whatever ACPI code is needed by this device. It will end up in the
> +* DSDT table.
> +*
> +* Note that this is called 'inject' because the output of calling 
> this
> +* method on all devices is injected into the DSDT, the bulk of which
> +* is writte in .asl files for the board.

is wirtten

> +*
> +* @dev: Device to write
> +* @ctx: ACPI context to use
> +* @return 0 if OK, -ve on error
> +*/
> +   int (*inject_dsdt)(const struct udevice *dev,

Re: [PATCH v4 29/35] acpi: Support ordering SSDT data by device

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 3:13 AM Simon Glass  wrote:
>
> Add a /chosen property to control the order in which the data appears
> in the SSDT. This allows matching up U-Boot's output from a dump of the
> known-good data obtained from within Linux.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Explain in sort_acpi_item_type() why ctx->current is not updated
>
> Changes in v3:
> - Make find_item() static and rename to find_acpi_item()
> - Rename build_type() and add a comment
>
> Changes in v1:
> - Generalise the ACPI function recursion with acpi_recurse_method()
>
>  arch/sandbox/dts/test.dts   |  5 +-
>  doc/device-tree-bindings/chosen.txt |  9 
>  drivers/core/acpi.c | 84 +
>  test/dm/acpi.c  | 15 +++---
>  4 files changed, 105 insertions(+), 8 deletions(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 25/35] acpi: Add support for a generic power sequence

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 3:13 AM Simon Glass  wrote:
>
> Add a way for devices to enable and disable themselves using ACPI code
> that updates GPIOs. This takes several timing parameters and supports
> enable, reset and stop.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Use new shared acpi_test_alloc_context_size() in test
> - Update functions to return GPIO/IRQ pin number
> - Use a separate variables for reading and writing DW0
>
>  include/acpi/acpi_device.h | 42 
>  lib/acpi/acpi_device.c | 99 ++
>  test/dm/acpigen.c  | 68 ++
>  3 files changed, 209 insertions(+)
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 24/35] acpi: Add support for writing a GPIO power sequence

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 3:13 AM Simon Glass  wrote:
>
> Power to some devices is controlled by GPIOs. Add a way to generate ACPI
> code to enable and disable a GPIO so that this can be handled within an
> ACPI method.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Update functions to return GPIO/IRQ pin number
> - Use a separate variables for reading and writing DW0
> - Correct bug in acpigen_set_gpio_val() by putting tx_state_val in LOCAL0
> - Fix up the comment for acpigen_set_enable_tx_gpio()
> - Explain access to DW0 register a bit better and provide a sample
> - Expand the comments for acpigen_get_dw0_in_local5()
> - Fix 'diabl' typo
>
>  include/acpi/acpigen.h | 56 
>  lib/acpi/acpigen.c | 85 ++
>  test/dm/acpigen.c  | 64 +++
>  3 files changed, 205 insertions(+)
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 19/35] acpi: Support writing Device Properties objects via _DSD

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 3:12 AM Simon Glass  wrote:
>
> More complex device properties can be provided to drivers via a
> device-specific data (_DSD) object.
>
> To create this we need to build it up in a separate data structure and
> then generate the ACPI code, due to its recursive nature.
>
> Add an implementation of this.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Drop embedded // comments in file comment
> - Correct return value comment in acpi_dp_add_child()
> - Rename acpi_dp_write_() to acpi_dp_write_internal() and make static
> - Use a #define for the test context size
> - Add a header file for test to allow sharing the context init function
> - Rename and get_length() into a shared test file
> - Add missing arg to comment for acpi_dp_write()
>
> Changes in v3:
> - Allow the name parameter to be NULL
> - Add error checking to acpi_dp_add_integer_array()
> - Fix 'acpi_device.v' typo
> - Drop unused ACPI_CPU_STRING
>
>  include/acpi/acpi_dp.h | 217 +++
>  include/acpi/acpigen.h |   1 +
>  lib/acpi/Makefile  |   1 +
>  lib/acpi/acpi_dp.c | 323 ++
>  test/dm/Makefile   |   1 +
>  test/dm/acpi.h |  29 
>  test/dm/acpi_dp.c  | 385 +
>  test/dm/acpigen.c  |  39 ++---
>  8 files changed, 974 insertions(+), 22 deletions(-)
>  create mode 100644 include/acpi/acpi_dp.h
>  create mode 100644 lib/acpi/acpi_dp.c
>  create mode 100644 test/dm/acpi.h
>  create mode 100644 test/dm/acpi_dp.c
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 13/35] acpigen: Support writing a length

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 3:12 AM Simon Glass  wrote:
>
> It is convenient to write a length value for preceding a block of data.
> Of course the length is not known or is hard to calculate a priori. So add
> a way to mark the start on a stack, so the length can be updated when
> known.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Add a note about the hard-coded 3-byte length used
> - Fix 'gas' typo
> - Use 'null' instead of 'nul'
>
> Changes in v3:
> - Add a reference to the ACPI spec
> - Add a define for the 0x80 constant
> - Move the function comments into this patch
>
>  include/acpi/acpigen.h | 41 +++
>  include/dm/acpi.h  |  9 +-
>  lib/acpi/acpigen.c | 33 ++
>  test/dm/acpigen.c  | 64 --
>  4 files changed, 144 insertions(+), 3 deletions(-)
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 09/35] acpi: Support generation of GPIO descriptor

2020-07-12 Thread Bin Meng
Hi Simon,

On Wed, Jul 8, 2020 at 3:12 AM Simon Glass  wrote:
>
> Add a function to write a GPIO descriptor to the generated ACPI code.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Drop comment about the type always being ACPI_GPIO_TYPE_IO
> - Rename the length-writing functions to indicate they are for large resources
> - Update functions to return a GPIO pin number
>
>  include/acpi/acpi_device.h |  22 ++
>  lib/acpi/acpi_device.c | 151 +
>  test/dm/acpigen.c  |  91 ++
>  3 files changed, 264 insertions(+)
>
> diff --git a/include/acpi/acpi_device.h b/include/acpi/acpi_device.h
> index 69b90968a8..18063e977f 100644
> --- a/include/acpi/acpi_device.h
> +++ b/include/acpi/acpi_device.h
> @@ -12,6 +12,7 @@
>  #include 
>
>  struct acpi_ctx;
> +struct gpio_desc;
>  struct irq;
>  struct udevice;
>
> @@ -233,4 +234,25 @@ enum acpi_dev_status acpi_device_status(const struct 
> udevice *dev);
>  int acpi_device_write_interrupt_irq(struct acpi_ctx *ctx,
> const struct irq *req_irq);
>
> +/**
> + * acpi_device_write_gpio() - Write GpioIo() or GpioInt() descriptor
> + *
> + * @gpio: GPIO information to write
> + * @return GPIO pin number of first GPIO if OK, -ve on error
> + */
> +int acpi_device_write_gpio(struct acpi_ctx *ctx, const struct acpi_gpio 
> *gpio);
> +
> +/**
> + * acpi_device_write_gpio_desc() - Write a GPIO to ACPI
> + *
> + * This creates a GPIO descriptor for a GPIO, including information ACPI 
> needs
> + * to use it. The type is always ACPI_GPIO_TYPE_IO.

This comment is not dropped ?

> + *
> + * @ctx: ACPI context pointer
> + * @desc: GPIO to write
> + * @return 0 if OK, -ve on error
> + */
> +int acpi_device_write_gpio_desc(struct acpi_ctx *ctx,
> +   const struct gpio_desc *desc);
> +
>  #endif
> diff --git a/lib/acpi/acpi_device.c b/lib/acpi/acpi_device.c
> index d854a45cbc..bbe1cfc57a 100644
> --- a/lib/acpi/acpi_device.c
> +++ b/lib/acpi/acpi_device.c
> @@ -12,6 +12,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  /**
> @@ -203,5 +204,155 @@ int acpi_device_write_interrupt_irq(struct acpi_ctx 
> *ctx,
> if (ret)
> return log_msg_ret("write", ret);
>
> +   return irq.pin;
> +}
> +
> +/* ACPI 6.3 section 6.4.3.8.1 - GPIO Interrupt or I/O */
> +int acpi_device_write_gpio(struct acpi_ctx *ctx, const struct acpi_gpio 
> *gpio)
> +{
> +   void *start, *desc_length;
> +   void *pin_table_offset, *vendor_data_offset, *resource_offset;
> +   u16 flags = 0;
> +   int pin;
> +
> +   if (gpio->type > ACPI_GPIO_TYPE_IO)
> +   return -EINVAL;
> +
> +   start = acpigen_get_current(ctx);
> +
> +   /* Byte 0: Descriptor Type */
> +   acpigen_emit_byte(ctx, ACPI_DESCRIPTOR_GPIO);
> +
> +   /* Byte 1-2: Length (fill in later) */
> +   desc_length = largeres_write_len_f(ctx);
> +
> +   /* Byte 3: Revision ID */
> +   acpigen_emit_byte(ctx, ACPI_GPIO_REVISION_ID);
> +
> +   /* Byte 4: GpioIo or GpioInt */
> +   acpigen_emit_byte(ctx, gpio->type);
> +
> +   /*
> +* Byte 5-6: General Flags
> +*   [15:1]: 0 => Reserved
> +*  [0]: 1 => ResourceConsumer
> +*/
> +   acpigen_emit_word(ctx, 1 << 0);
> +
> +   switch (gpio->type) {
> +   case ACPI_GPIO_TYPE_INTERRUPT:
> +   /*
> +* Byte 7-8: GPIO Interrupt Flags
> +*   [15:5]: 0 => Reserved
> +*  [4]: Wake (0=NO_WAKE   1=WAKE)
> +*  [3]: Sharing  (0=EXCLUSIVE 1=SHARED)
> +*[2:1]: Polarity (0=HIGH  1=LOW 2=BOTH)
> +*  [0]: Mode (0=LEVEL 1=EDGE)
> +*/
> +   if (gpio->irq.mode == ACPI_IRQ_EDGE_TRIGGERED)
> +   flags |= 1 << 0;
> +   if (gpio->irq.shared == ACPI_IRQ_SHARED)
> +   flags |= 1 << 3;
> +   if (gpio->irq.wake == ACPI_IRQ_WAKE)
> +   flags |= 1 << 4;
> +
> +   switch (gpio->irq.polarity) {
> +   case ACPI_IRQ_ACTIVE_HIGH:
> +   flags |= 0 << 1;
> +   break;
> +   case ACPI_IRQ_ACTIVE_LOW:
> +   flags |= 1 << 1;
> +   break;
> +   case ACPI_IRQ_ACTIVE_BOTH:
> +   flags |= 2 << 1;
> +   break;
> +   }
> +   break;
> +
> +   case ACPI_GPIO_TYPE_IO:
> +   /*
> +* Byte 7-8: GPIO IO Flags
> +*   [15:4]: 0 => Reserved
> +*  [3]: Sharing  (0=EXCLUSIVE 1=SHARED)
> +*  [2]: 0 => Reserved
> +*[1:0]: IO Restriction
> +*   0 => IoRestrictionNone
> +   

Re: [PATCH v4 07/35] gpio: Add a method to convert a GPIO to ACPI

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 3:12 AM Simon Glass  wrote:
>
> When generating ACPI tables we need to convert GPIOs in U-Boot to the ACPI
> structures required by ACPI. This is a SoC-specific conversion and cannot
> be handled by generic code, so add a new GPIO method to do the conversion.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Use memset() to zero struct acpi_gpio in gpio_get_acpi()
> - Move common code about the if...else in sb_gpio_get_acpi()
> - Don't set zero fields in sb_gpio_get_acpi(), and add a comment about it
> - Update acpi_device_write_interrupt_irq() to return IRQ pin number
>
> Changes in v1:
> - Update sandbox driver slightly for testing
>
>  drivers/gpio/gpio-uclass.c | 22 ++
>  drivers/gpio/sandbox.c | 77 
>  include/acpi/acpi_device.h | 90 ++
>  include/asm-generic/gpio.h | 27 
>  test/dm/gpio.c | 62 ++
>  5 files changed, 278 insertions(+)
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 06/35] acpi: Support generation of interrupt descriptor

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 3:12 AM Simon Glass  wrote:
>
> Add a function to write an interrupt descriptor to the generated ACPI
> code.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> ---
>
> Changes in v4:
> - Rename the length-writing functions to indicate they are for large resources
>
>  include/acpi/acpi_device.h |  15 +
>  lib/acpi/acpi_device.c | 119 +
>  test/dm/acpigen.c  |  32 ++
>  3 files changed, 166 insertions(+)
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 05/35] acpi: Support generation of ACPI code

2020-07-12 Thread Bin Meng
On Wed, Jul 8, 2020 at 3:12 AM Simon Glass  wrote:
>
> Add a new file to handle generating ACPI code programatically. This is
> used when information must be dynamically added to the tables, e.g. the
> SSDT.
>
> Initial support is just for writing simple values. Also add a 'base' value
> so that the table can be freed. This likely doesn't happen in normal code,
> but is nice to do in tests.
>
> Reviewed-by: Wolfgang Wallner 
> Signed-off-by: Simon Glass 
> ---
>
> Changes in v4:
> - Setup ctx->base as well as ctx->current in acpi_setup_base_tables()
> - Add a comment about little-endian in acpigen_emit_dword()
> - Add a #define for the context size in alloc_context()
>
> Changes in v2:
> - Update to add a new 'base' field to struct acpi_ctx
> - Free the memory allocated to the table and context
>
>  include/acpi/acpigen.h | 49 
>  include/dm/acpi.h  |  2 ++
>  lib/acpi/Makefile  |  1 +
>  lib/acpi/acpi_table.c  |  1 +
>  lib/acpi/acpigen.c | 39 +++
>  test/dm/Makefile   |  1 +
>  test/dm/acpigen.c  | 72 ++
>  7 files changed, 165 insertions(+)
>  create mode 100644 include/acpi/acpigen.h
>  create mode 100644 lib/acpi/acpigen.c
>  create mode 100644 test/dm/acpigen.c
>

Reviewed-by: Bin Meng 


Re: [PATCH v4 2/6] sifive: fu540: Add Booting from SPI

2020-07-12 Thread Rick Chen
Hi Jagan

> From: Jagan Teki [mailto:ja...@amarulasolutions.com]
> Sent: Thursday, July 02, 2020 4:03 PM
> To: Rick Jian-Zhi Chen(陳建志); Atish Patra; Palmer Dabbelt; Bin Meng; Paul 
> Walmsley; Anup Patel; Sagar Kadam
> Cc: u-boot@lists.denx.de; linux-amar...@amarulasolutions.com; Jagan Teki; Bin 
> Meng
> Subject: [PATCH v4 2/6] sifive: fu540: Add Booting from SPI
>
> Add booting from SPI for SiFive Unleashed board.
>
> Signed-off-by: Jagan Teki 
> Reviewed-by: Bin Meng 
> Tested-by: Bin Meng 
> ---
> Changes for v4:
> - drop BOARD configs
>

Can you rebase and send the whole patches of v4 again ?
When I tried to pick up others from v3, there have some conflicts.

Thanks,
Rick

>  .../dts/hifive-unleashed-a00-u-boot.dtsi  | 12 ++
>  configs/sifive_fu540_defconfig|  4 ++
>  doc/board/sifive/fu540.rst| 41 +++
>  3 files changed, 57 insertions(+)
>
> diff --git a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi 
> b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
> index 303806454b..4b2b242deb 100644
> --- a/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
> +++ b/arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi
> @@ -12,6 +12,10 @@
> spi2 = &qspi2;
> };
>
> +   config {
> +   u-boot,spl-payload-offset = <0x105000>; /* loader2 @1044KB */
> +   };
> +
> hfclk {
> u-boot,dm-spl;
> };
> @@ -22,6 +26,14 @@
>
>  };
>
> +&qspi0 {
> +   u-boot,dm-spl;
> +
> +   flash@0 {
> +   u-boot,dm-spl;
> +   };
> +};
> +
>  &qspi2 {
> mmc@0 {
> u-boot,dm-spl;
> diff --git a/configs/sifive_fu540_defconfig b/configs/sifive_fu540_defconfig 
> index 8d412f8d6a..551d4b04a5 100644
> --- a/configs/sifive_fu540_defconfig
> +++ b/configs/sifive_fu540_defconfig
> @@ -2,9 +2,11 @@ CONFIG_RISCV=y
>  CONFIG_SPL_GPIO_SUPPORT=y
>  CONFIG_SYS_MALLOC_F_LEN=0x3000
>  CONFIG_ENV_SIZE=0x2
> +CONFIG_SPL_DM_SPI=y
>  CONFIG_SPL_MMC_SUPPORT=y
>  CONFIG_NR_DRAM_BANKS=1
>  CONFIG_SPL=y
> +CONFIG_SPL_SPI_FLASH_SUPPORT=y
>  CONFIG_SPL_SPI_SUPPORT=y
>  CONFIG_TARGET_SIFIVE_FU540=y
>  CONFIG_ARCH_RV64I=y
> @@ -15,9 +17,11 @@ CONFIG_MISC_INIT_R=y
>  CONFIG_DISPLAY_CPUINFO=y
>  CONFIG_DISPLAY_BOARDINFO=y
>  CONFIG_SPL_SEPARATE_BSS=y
> +CONFIG_SPL_SPI_LOAD=y
>  CONFIG_SPL_YMODEM_SUPPORT=y
>  CONFIG_OF_BOARD_FIXUP=y
>  CONFIG_DEFAULT_DEVICE_TREE="hifive-unleashed-a00"
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> +CONFIG_SPL_DM_SEQ_ALIAS=y
>  CONFIG_SPL_CLK=y
>  CONFIG_DM_MTD=y
> diff --git a/doc/board/sifive/fu540.rst b/doc/board/sifive/fu540.rst index 
> 739eefa669..1ce9ab14f5 100644
> --- a/doc/board/sifive/fu540.rst
> +++ b/doc/board/sifive/fu540.rst
> @@ -533,3 +533,44 @@ Sample boot log from HiFive Unleashed board
> type:   0fc63daf-8483-4772-8e79-3d69d8477de4
> type:   linux
> guid:   9faa81b6-39b1-4418-af5e-89c48f29c20d
> +
> +Booting from SPI
> +
> +
> +Use Building steps from "Booting from MMC using U-Boot SPL" section.
> +
> +Partition the SPI in Linux via mtdblock. (Require to boot the board in
> +SD boot mode by enabling MTD block in Linux)
> +
> +Use prebuilt image from here [1], which support to partition the SPI flash.
> +
> +.. code-block:: none
> +
> +  # sgdisk --clear \
> +  > --set-alignment=2 \
> +  > --new=1:40:2087 --change-name=1:loader1
> + --typecode=1:5B193300-FC78-40CD-8002-E86C45580B47 \  >
> + --new=2:2088:10279 --change-name=2:loader2
> + --typecode=2:2E54B353-1271-4842-806F-E436D6AF6985 \  >
> + --new=3:10536:65494 --change-name=3:rootfs
> + --typecode=3:0FC63DAF-8483-4772-8E79-3D69D8477DE4 \  > /dev/mtdblock0
> +
> +Program the SPI (Require to boot the board in SD boot mode)
> +
> +Execute below steps on U-Boot proper,
> +
> +.. code-block:: none
> +
> +  tftpboot $kernel_addr_r u-boot-spl.bin  sf erase 0x5000 $filesize  sf
> + write $kernel_addr_r 0x5000 $filesize
> +
> +  tftpboot $kernel_addr_r u-boot.itb
> +  sf erase 0x105000 $filesize
> +  sf write $kernel_addr_r 0x105000 $filesize
> +
> +Power off the board
> +
> +Change DIP switches MSEL[3:0] are set to 0110
> +
> +Power up the board.
> +
> +[1] https://github.com/amarula/bsp-sifive
> --
> 2.25.1
>


Re: [PATCH] x86: remove unused setup_pcat_compatibility() stub

2020-07-12 Thread Bin Meng
On Tue, Jul 7, 2020 at 10:08 AM Masahiro Yamada  wrote:
>
> Simon,
>
> On Tue, Jul 7, 2020 at 3:44 AM Simon Glass  wrote:
> >
> > Hi Masahiro,
> >
> > On Sat, 4 Jul 2020 at 11:43, Masahiro Yamada  wrote:
> > >
> > > 'git grep' did not find any user of this stub.
> > >
> > > Signed-off-by: Masahiro Yamada 
> > > ---
> > >
> > >  arch/x86/include/asm/u-boot-x86.h |  2 --
> > >  arch/x86/lib/zimage.c | 10 --
> > >  2 files changed, 12 deletions(-)
> > >
> >
> > See also: 
> > http://patchwork.ozlabs.org/project/uboot/patch/20200615035738.248710-21-...@chromium.org/
>
>
> That's fine if you are removing this function.
>
> Thanks.

Thanks Masahiro. I will use Simon's version then as it is included in a series.

Regards,
Bin


Re: [PATCH v3 1/4] timer: Allow delays with a 32-bit microsecond timer

2020-07-12 Thread Bin Meng
Hi Simon,

On Mon, Jul 13, 2020 at 10:25 AM Simon Glass  wrote:
>
> Hi Bin,
>
> On Sun, 12 Jul 2020 at 20:13, Bin Meng  wrote:
> >
> > On Fri, Jul 10, 2020 at 8:49 AM Simon Glass  wrote:
> > >
> > > The current get_timer_us() uses 64-bit arithmetic on 32-bit machines.
> > > When implementing microsecond-level timeouts, 32-bits is plenty. Add a
> > > new function that uses an unsigned long. On 64-bit machines this is
> > > still 64-bit, but this doesn't introduce a penalty. On 32-bit machines
> > > it is more efficient.
> > >
> > > Signed-off-by: Simon Glass 
> > > ---
> > >
> > > Changes in v3:
> > > - Expand the commit message and function comment
> > >
> > >  include/time.h | 11 +++
> > >  lib/time.c |  5 +
> > >  2 files changed, 16 insertions(+)
> > >
> > > diff --git a/include/time.h b/include/time.h
> > > index e99f9c8012..3f00e68713 100644
> > > --- a/include/time.h
> > > +++ b/include/time.h
> > > @@ -17,6 +17,17 @@ unsigned long get_timer(unsigned long base);
> > >  unsigned long timer_get_us(void);
> > >  uint64_t get_timer_us(uint64_t base);
> > >
> > > +/**
> > > + * get_timer_us_long() - Get the number of elapsed microseconds
> > > + *
> > > + * This uses 32-bit arithmetic on 32-bit machines, which is enough to 
> > > handle
> > > + * delays of over an hour. For 64-bit machines it uses a 64-bit value.
> > > + *
> > > + *@base: Base time to consider
> > > + *@return elapsed time since @base
> > > + */
> > > +unsigned long get_timer_us_long(unsigned long base);
> > > +
> > >  /*
> > >   * timer_test_add_offset()
> > >   *
> > > diff --git a/lib/time.c b/lib/time.c
> > > index 65db0f6cda..47f8c84327 100644
> > > --- a/lib/time.c
> > > +++ b/lib/time.c
> > > @@ -152,6 +152,11 @@ uint64_t __weak get_timer_us(uint64_t base)
> > > return tick_to_time_us(get_ticks()) - base;
> > >  }
> > >
> > > +unsigned long __weak get_timer_us_long(unsigned long base)
> > > +{
> > > +   return timer_get_us() - base;
> > > +}
> > > +
> > >  unsigned long __weak notrace timer_get_us(void)
> > >  {
> > > return tick_to_time(get_ticks() * 1000);
> > > --
> >
> > Reviewed-by: Bin Meng 
> >
> > Some day we should clean up the timer APIs. We have get_timer_us(),
> > and now get_timer_us_long(), but we also have timer_get_us() that
> > returns long value!
>
> Yes, but that one returns the monotonic time, so isn't so easy for
> timeouts. Perhaps we should remove it since get_timer_us_long(0) does
> the same thing?

Yes, I think so.

Regards,
Bin


Re: [PATCH v3 1/4] timer: Allow delays with a 32-bit microsecond timer

2020-07-12 Thread Simon Glass
Hi Bin,

On Sun, 12 Jul 2020 at 20:13, Bin Meng  wrote:
>
> On Fri, Jul 10, 2020 at 8:49 AM Simon Glass  wrote:
> >
> > The current get_timer_us() uses 64-bit arithmetic on 32-bit machines.
> > When implementing microsecond-level timeouts, 32-bits is plenty. Add a
> > new function that uses an unsigned long. On 64-bit machines this is
> > still 64-bit, but this doesn't introduce a penalty. On 32-bit machines
> > it is more efficient.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> > Changes in v3:
> > - Expand the commit message and function comment
> >
> >  include/time.h | 11 +++
> >  lib/time.c |  5 +
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/include/time.h b/include/time.h
> > index e99f9c8012..3f00e68713 100644
> > --- a/include/time.h
> > +++ b/include/time.h
> > @@ -17,6 +17,17 @@ unsigned long get_timer(unsigned long base);
> >  unsigned long timer_get_us(void);
> >  uint64_t get_timer_us(uint64_t base);
> >
> > +/**
> > + * get_timer_us_long() - Get the number of elapsed microseconds
> > + *
> > + * This uses 32-bit arithmetic on 32-bit machines, which is enough to 
> > handle
> > + * delays of over an hour. For 64-bit machines it uses a 64-bit value.
> > + *
> > + *@base: Base time to consider
> > + *@return elapsed time since @base
> > + */
> > +unsigned long get_timer_us_long(unsigned long base);
> > +
> >  /*
> >   * timer_test_add_offset()
> >   *
> > diff --git a/lib/time.c b/lib/time.c
> > index 65db0f6cda..47f8c84327 100644
> > --- a/lib/time.c
> > +++ b/lib/time.c
> > @@ -152,6 +152,11 @@ uint64_t __weak get_timer_us(uint64_t base)
> > return tick_to_time_us(get_ticks()) - base;
> >  }
> >
> > +unsigned long __weak get_timer_us_long(unsigned long base)
> > +{
> > +   return timer_get_us() - base;
> > +}
> > +
> >  unsigned long __weak notrace timer_get_us(void)
> >  {
> > return tick_to_time(get_ticks() * 1000);
> > --
>
> Reviewed-by: Bin Meng 
>
> Some day we should clean up the timer APIs. We have get_timer_us(),
> and now get_timer_us_long(), but we also have timer_get_us() that
> returns long value!

Yes, but that one returns the monotonic time, so isn't so easy for
timeouts. Perhaps we should remove it since get_timer_us_long(0) does
the same thing?

Regards,
Simon


Re: [PATCH v3 4/4] x86: fsp: Support a warning message when DRAM init is slow

2020-07-12 Thread Bin Meng
On Fri, Jul 10, 2020 at 8:43 AM Simon Glass  wrote:
>
> With DDR4, Intel SOCs take quite a long time to init their memory. During
> this time, if the user is watching, it looks like SPL has hung. Add a
> message in this case.
>
> This works by adding a return code to fspm_update_config() that indicates
> whether MRC data was found and a new property to the device tree.
>
> Also add one more debug message while starting.
>
> Signed-off-by: Simon Glass 
> Reviewed-by: Wolfgang Wallner 
> Tested-by: Wolfgang Wallner 
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Use the three-argument CONFIG_IS_ENABLED() instead of IF_ENABLED_INT()
> - Update binding to mention timing for coral and a 1GB APL board
> - Drop patch 'kconfig: Add support for conditional values'
>
>  arch/x86/cpu/apollolake/fsp_m.c   | 12 --
>  arch/x86/dts/chromebook_coral.dts |  1 +
>  arch/x86/include/asm/fsp2/fsp_internal.h  |  3 ++-
>  arch/x86/lib/fsp2/fsp_meminit.c   | 24 +++
>  .../fsp/fsp2/apollolake/fsp-m.txt |  4 
>  5 files changed, 36 insertions(+), 8 deletions(-)
>

applied to u-boot-x86, thanks!


Re: [PATCH v3 2/4] coral: Enable the copy framebuffer

2020-07-12 Thread Bin Meng
On Fri, Jul 10, 2020 at 8:49 AM Simon Glass  wrote:
>
> Enable this feature on chromebook_coral to speed up the display.
>
> With this change, the time taken to print the environment to the display
> without CONFIG_CONSOLE_SCROLL_LINES is reduced from 1830ms to 62ms.
>
> Signed-off-by: Simon Glass 
> ---
>
> (no changes since v1)
>
>  configs/chromebook_coral_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>

applied to u-boot-x86, thanks!


Re: [PATCH v3 3/4] x86: Avoid #ifdef with CONFIG_HAVE_ACPI_RESUME

2020-07-12 Thread Bin Meng
On Fri, Jul 10, 2020 at 8:43 AM Simon Glass  wrote:
>
> At present this enables a few arch-specific members of the global_data
> struct which are otherwise not part of the struct. As a result we have to
> use #ifdef in various places.
>
> The cost of always having these in the struct is small. Adjust things so
> that we can use compile-time code instead of #ifdefs.
>
> Signed-off-by: Simon Glass 
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Use new three-argument CONFIG_IS_ENABLED() instead
>
>  arch/x86/cpu/apollolake/cpu_spl.c| 13 +-
>  arch/x86/cpu/apollolake/fsp_s.c  | 10 
>  arch/x86/cpu/baytrail/acpi.c |  2 --
>  arch/x86/cpu/broadwell/power_state.c |  5 ++--
>  arch/x86/cpu/cpu.c   | 38 ++--
>  arch/x86/include/asm/global_data.h   |  2 --
>  arch/x86/lib/coreboot_table.c|  6 ++---
>  arch/x86/lib/fsp/fsp_common.c|  2 --
>  arch/x86/lib/fsp/fsp_dram.c  | 26 +++
>  arch/x86/lib/fsp1/fsp_common.c   | 16 +++-
>  arch/x86/lib/fsp2/fsp_dram.c |  7 +++--
>  11 files changed, 63 insertions(+), 64 deletions(-)
>

applied to u-boot-x86, thanks!


Re: [PATCH v3 1/4] timer: Allow delays with a 32-bit microsecond timer

2020-07-12 Thread Bin Meng
On Mon, Jul 13, 2020 at 10:13 AM Bin Meng  wrote:
>
> On Fri, Jul 10, 2020 at 8:49 AM Simon Glass  wrote:
> >
> > The current get_timer_us() uses 64-bit arithmetic on 32-bit machines.
> > When implementing microsecond-level timeouts, 32-bits is plenty. Add a
> > new function that uses an unsigned long. On 64-bit machines this is
> > still 64-bit, but this doesn't introduce a penalty. On 32-bit machines
> > it is more efficient.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> > Changes in v3:
> > - Expand the commit message and function comment
> >
> >  include/time.h | 11 +++
> >  lib/time.c |  5 +
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/include/time.h b/include/time.h
> > index e99f9c8012..3f00e68713 100644
> > --- a/include/time.h
> > +++ b/include/time.h
> > @@ -17,6 +17,17 @@ unsigned long get_timer(unsigned long base);
> >  unsigned long timer_get_us(void);
> >  uint64_t get_timer_us(uint64_t base);
> >
> > +/**
> > + * get_timer_us_long() - Get the number of elapsed microseconds
> > + *
> > + * This uses 32-bit arithmetic on 32-bit machines, which is enough to 
> > handle
> > + * delays of over an hour. For 64-bit machines it uses a 64-bit value.
> > + *
> > + *@base: Base time to consider
> > + *@return elapsed time since @base
> > + */
> > +unsigned long get_timer_us_long(unsigned long base);
> > +
> >  /*
> >   * timer_test_add_offset()
> >   *
> > diff --git a/lib/time.c b/lib/time.c
> > index 65db0f6cda..47f8c84327 100644
> > --- a/lib/time.c
> > +++ b/lib/time.c
> > @@ -152,6 +152,11 @@ uint64_t __weak get_timer_us(uint64_t base)
> > return tick_to_time_us(get_ticks()) - base;
> >  }
> >
> > +unsigned long __weak get_timer_us_long(unsigned long base)
> > +{
> > +   return timer_get_us() - base;
> > +}
> > +
> >  unsigned long __weak notrace timer_get_us(void)
> >  {
> > return tick_to_time(get_ticks() * 1000);
> > --
>
> Reviewed-by: Bin Meng 
>
> Some day we should clean up the timer APIs. We have get_timer_us(),
> and now get_timer_us_long(), but we also have timer_get_us() that
> returns long value!

applied to u-boot-x86, thanks!


Re: [PATCH v3 1/4] timer: Allow delays with a 32-bit microsecond timer

2020-07-12 Thread Bin Meng
On Fri, Jul 10, 2020 at 8:49 AM Simon Glass  wrote:
>
> The current get_timer_us() uses 64-bit arithmetic on 32-bit machines.
> When implementing microsecond-level timeouts, 32-bits is plenty. Add a
> new function that uses an unsigned long. On 64-bit machines this is
> still 64-bit, but this doesn't introduce a penalty. On 32-bit machines
> it is more efficient.
>
> Signed-off-by: Simon Glass 
> ---
>
> Changes in v3:
> - Expand the commit message and function comment
>
>  include/time.h | 11 +++
>  lib/time.c |  5 +
>  2 files changed, 16 insertions(+)
>
> diff --git a/include/time.h b/include/time.h
> index e99f9c8012..3f00e68713 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -17,6 +17,17 @@ unsigned long get_timer(unsigned long base);
>  unsigned long timer_get_us(void);
>  uint64_t get_timer_us(uint64_t base);
>
> +/**
> + * get_timer_us_long() - Get the number of elapsed microseconds
> + *
> + * This uses 32-bit arithmetic on 32-bit machines, which is enough to handle
> + * delays of over an hour. For 64-bit machines it uses a 64-bit value.
> + *
> + *@base: Base time to consider
> + *@return elapsed time since @base
> + */
> +unsigned long get_timer_us_long(unsigned long base);
> +
>  /*
>   * timer_test_add_offset()
>   *
> diff --git a/lib/time.c b/lib/time.c
> index 65db0f6cda..47f8c84327 100644
> --- a/lib/time.c
> +++ b/lib/time.c
> @@ -152,6 +152,11 @@ uint64_t __weak get_timer_us(uint64_t base)
> return tick_to_time_us(get_ticks()) - base;
>  }
>
> +unsigned long __weak get_timer_us_long(unsigned long base)
> +{
> +   return timer_get_us() - base;
> +}
> +
>  unsigned long __weak notrace timer_get_us(void)
>  {
> return tick_to_time(get_ticks() * 1000);
> --

Reviewed-by: Bin Meng 

Some day we should clean up the timer APIs. We have get_timer_us(),
and now get_timer_us_long(), but we also have timer_get_us() that
returns long value!

Regards,
Bin


Re: [PATCH v3 1/2] drivers: p2sb: replace Primary-to-Sideband Bus with Primary to Sideband Bridge

2020-07-12 Thread Bin Meng
On Wed, Jul 1, 2020 at 7:37 PM Wolfgang Wallner
 wrote:
>
> In Intel's documentation the term P2SB stands for "Primary to Sideband
> Bridge".
>
> Signed-off-by: Wolfgang Wallner 
> ---
>
> Changes in v3:
> - Replaced the term in two more places
>
>  drivers/misc/Kconfig | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>

applied to u-boot-x86, thanks!


Re: [PATCH v3 2/2] x86: p2sb: make P2SB driver depend on P2SB uclass

2020-07-12 Thread Bin Meng
On Wed, Jul 1, 2020 at 7:37 PM Wolfgang Wallner
 wrote:
>
> Currently it is possible to select the P2SB driver without selecting the
> P2SB uclass, which can't work. Fix this by adding a "depends on" in
> Kconfig.
>
> Signed-off-by: Wolfgang Wallner 
>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Fixed cover letter
>
>  arch/x86/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>

applied to u-boot-x86, thanks!


Re: [PATCH v3 13/13] test/py: efi_secboot: add a test for verifying with digest of signed image

2020-07-12 Thread AKASHI Takahiro
Heinrich,

On Sat, Jul 11, 2020 at 08:47:15AM +0200, Heinrich Schuchardt wrote:
> On 7/8/20 7:02 AM, AKASHI Takahiro wrote:
> > Signature database (db or dbx) may have not only certificates that contain
> > a public key for RSA decryption, but also digests of signed images.
> >
> > In this test case, if database has an image's digest (EFI_CERT_SHA256_GUID)
> > and if the value matches to a hash value calculated from image's binary,
> > authentication should pass in case of db, and fail in case of dbx.
> >
> > Signed-off-by: AKASHI Takahiro 
> > ---
> >  test/py/tests/test_efi_secboot/conftest.py| 10 
> >  test/py/tests/test_efi_secboot/test_signed.py | 49 +++
> >  2 files changed, 59 insertions(+)
> >
> > diff --git a/test/py/tests/test_efi_secboot/conftest.py 
> > b/test/py/tests/test_efi_secboot/conftest.py
> > index c7da1a6e29a3..553550ee02b1 100644
> > --- a/test/py/tests/test_efi_secboot/conftest.py
> > +++ b/test/py/tests/test_efi_secboot/conftest.py
> > @@ -120,6 +120,10 @@ def efi_boot_env(request, u_boot_config):
> >  check_call('cd %s; %scert-to-efi-hash-list -g %s -t 0 -s 256 
> > db1.crt dbx_hash1.crl; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx 
> > dbx_hash1.crl dbx_hash1.auth'
> > % (mnt_point, EFITOOLS_PATH, GUID, EFITOOLS_PATH),
> > shell=True)
> > +## dbx_db (with TEST_db certificate)
> > +check_call('cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx 
> > db.esl dbx_db.auth'
> > +   % (mnt_point, EFITOOLS_PATH),
> > +   shell=True)
> >
> >  # Copy image
> >  check_call('cp %s %s' % (HELLO_PATH, mnt_point), shell=True)
> > @@ -134,6 +138,12 @@ def efi_boot_env(request, u_boot_config):
> >  check_call('cd %s; %shash-to-efi-sig-list helloworld.efi 
> > db_hello.hash; %ssign-efi-sig-list -t "2020-04-07" -c KEK.crt -k KEK.key db 
> > db_hello.hash db_hello.auth'
> > % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH),
> > shell=True)
> > +check_call('cd %s; %shash-to-efi-sig-list helloworld.efi.signed 
> > db_hello_signed.hash; %ssign-efi-sig-list -c KEK.crt -k KEK.key db 
> > db_hello_signed.hash db_hello_signed.auth'
> > +   % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH),
> > +   shell=True)
> > +check_call('cd %s; %ssign-efi-sig-list -c KEK.crt -k KEK.key dbx 
> > db_hello_signed.hash dbx_hello_signed.auth'
> > +   % (mnt_point, EFITOOLS_PATH),
> > +   shell=True)
> >
> >  check_call('sudo umount %s' % loop_dev, shell=True)
> >  check_call('sudo losetup -d %s' % loop_dev, shell=True)
> > diff --git a/test/py/tests/test_efi_secboot/test_signed.py 
> > b/test/py/tests/test_efi_secboot/test_signed.py
> > index 1a31a57e12c2..7531bbac6a5f 100644
> > --- a/test/py/tests/test_efi_secboot/test_signed.py
> > +++ b/test/py/tests/test_efi_secboot/test_signed.py
> > @@ -198,3 +198,52 @@ class TestEfiSignedImage(object):
> >  'efidebug test bootmgr'])
> >  assert '\'HELLO\' failed' in ''.join(output)
> >  assert 'efi_start_image() returned: 26' in ''.join(output)
> > +
> > +def test_efi_signed_image_auth6(self, u_boot_console, efi_boot_env):
> > +"""
> > +Test Case 6 - using digest of signed image in database
> > +"""
> > +u_boot_console.restart_uboot()
> > +disk_img = efi_boot_env
> > +with u_boot_console.log.section('Test Case 6a'):
> > +# Test Case 6a, verified by image's digest in db
> > +output = u_boot_console.run_command_list([
> > +'host bind 0 %s' % disk_img,
> > +'fatload host 0:1 400 db_hello_signed.auth',
> > +'setenv -e -nv -bs -rt -at -i 400,$filesize db',
> > +'fatload host 0:1 400 KEK.auth',
> > +'setenv -e -nv -bs -rt -at -i 400,$filesize KEK',
> > +'fatload host 0:1 400 PK.auth',
> > +'setenv -e -nv -bs -rt -at -i 400,$filesize PK'])
> > +assert 'Failed to set EFI variable' not in ''.join(output)
> > +output = u_boot_console.run_command_list([
> > +'efidebug boot add 1 HELLO host 0:1 /helloworld.efi.signed 
> > ""',
> > +'efidebug boot next 1',
> > +'bootefi bootmgr'])
> > +assert 'Hello, world!' in ''.join(output)
> > +
> > +with u_boot_console.log.section('Test Case 6b'):
> > +# Test Case 6b, rejected by TEST_db certificate in dbx
> > +output = u_boot_console.run_command_list([
> > +'fatload host 0:1 400 dbx_db.auth',
> > +'setenv -e -nv -bs -rt -at -i 400,$filesize dbx'])
> > +assert 'Failed to set EFI variable' not in ''.join(output)
> > +output = u_boot_console.run_command_list([
> > +   

Re: [PATCH] cmd: mvebu/bubt: Drop unused SPI_FLASH_PROTECTION

2020-07-12 Thread Tom Rini
On Sun, Jul 12, 2020 at 11:29:01PM +0530, Jagan Teki wrote:

> SPI_FLASH_PROTECTION config item is never used in anywhere
> in the U-Boot tree.
> 
> Drop it.
> 
> Signed-off-by: Jagan Teki 
> ---
>  cmd/mvebu/bubt.c | 7 ---
>  1 file changed, 7 deletions(-)
> 
> diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
> index a27b0df8ae..b3b5844267 100644
> --- a/cmd/mvebu/bubt.c
> +++ b/cmd/mvebu/bubt.c
> @@ -299,9 +299,6 @@ static int spi_burn_image(size_t image_size)
>   return -ENOMEDIUM;
>   }
>  
> -#ifdef CONFIG_SPI_FLASH_PROTECTION
> - spi_flash_protect(flash, 0);
> -#endif
>   erase_bytes = image_size +
>   (flash->erase_size - image_size % flash->erase_size);
>   printf("Erasing %d bytes (%d blocks) at offset 0 ...",
> @@ -320,10 +317,6 @@ static int spi_burn_image(size_t image_size)
>   else
>   printf("Done!\n");
>  
> -#ifdef CONFIG_SPI_FLASH_PROTECTION
> - spi_flash_protect(flash, 1);
> -#endif
> -
>   return ret;
>  }

Add maintainers.

-- 
Tom


signature.asc
Description: PGP signature


Re: Pull request for UEFI sub-system for efi-2020-10-rc1 (2)

2020-07-12 Thread AKASHI Takahiro
Heinrich,

On Sun, Jul 12, 2020 at 12:05:32AM +0200, Heinrich Schuchardt wrote:
> On 7/11/20 2:16 PM, Tom Rini wrote:
> > On Sat, Jul 11, 2020 at 09:00:16AM +0200, Heinrich Schuchardt wrote:
> >> On 7/10/20 8:09 PM, Tom Rini wrote:
> >>> On Thu, Jul 09, 2020 at 06:12:02PM +0200, Heinrich Schuchardt wrote:
> >>>
>  The following changes since commit 
>  61608f395e7dcb2be6060407a72a1149b046430a:
> 
>    Merge branch '2020-07-08-misc-features-and-fixes' (2020-07-08 20:20:24
>  -0400)
> 
>  are available in the Git repository at:
> 
>    https://gitlab.denx.de/u-boot/custodians/u-boot-efi.git 
>  efi-2020-10-rc1-2
> 
>  for you to fetch changes up to f4cef8e7585c268f05a8c39e368ca115c25e40d5:
> 
>    efi_selftest: adjust runtime test for variables (2020-07-09 12:08:41
>  +0200)
> 
> >>>
> >>> NAK.  This is reliably failing here:
> >>> https://gitlab.denx.de/u-boot/u-boot/-/jobs/122018
> >>>
> >>> I see it passed Azure, and hasn't run through Travis yet.  Maybe it
> >>> needs to be run repeatedly to fail and we just got "lucky" ?
> >>>
> >>
> >> Hello Tom,
> >>
> >> you saw unreproducible results with multiple runs failing and one run
> >> succeeding. The reason is that when signing with sign-efi-sig-list in
> >> out Python tests without passing a timestamp two signatures may be in
> >> the same second or not.
> >>
> >> When using the signed files to set UEFI variables a variable can only be
> >> overwritten by a file with a newer timestamp. But without setting
> >> timestamps explicitly using parameter -t passed to sign-efi-sig-list we
> >> have no control.
> >>
> >> I already fixed this for some elder tests but missed to fix this for the
> >> merged patches from Takahiro.
> >
> > Ah, thanks for the explanation.
> >
> 
> Hello Tom,
> 
> what I still do not understand why tests are sometimes skipped and
> sometimes not for the same source code:
> 
> https://gitlab.denx.de/u-boot/u-boot/-/jobs/122018
> Commit 7068e523
> 
> 140 test/py/tests/test_efi_secboot/test_authvar.py .
> 141 test/py/tests/test_efi_secboot/test_signed.py .F
> 142 test/py/tests/test_efi_secboot/test_unsigned.py ...
> 
> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/122155
> Commit 7068e523
> 
> 148 test/py/tests/test_efi_secboot/test_authvar.py s
> 149 test/py/tests/test_efi_secboot/test_signed.py ss
> 150 test/py/tests/test_efi_secboot/test_unsigned.py sss
> 
> Both runs used the same Docker image
> trini/u-boot-gitlab-ci-runner:bionic-20200526-18Jun2020
> 
> What influence have different versions of the Gitlab runner?
> 
> gitlab-runner 13.1.1
> gitlab-runner 12.2.0
> 
> Some of our tests create and delete files in /tmp. How are parallel jobs
> separated in Gitlab?

No longer true.
You fixed it with the commit b32ac16f9a32 ("test/py: fix
test_efi_secboot/conftest.py").

-Takahiro Akashi



> Best regards
> 
> Heinrich


[PATCH 2/3] board: mediatek: Use CONFIG_DEFAULT_FDT_FILE for default environment

2020-07-12 Thread David Woodhouse
Rather than hard-coding it to the Banana Pi R2.

Signed-off-by: David Woodhouse 
---
 configs/mt7623n_bpir2_defconfig | 2 +-
 include/configs/mt7623.h| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configs/mt7623n_bpir2_defconfig b/configs/mt7623n_bpir2_defconfig
index 6b9fbd7e22..918c52d608 100644
--- a/configs/mt7623n_bpir2_defconfig
+++ b/configs/mt7623n_bpir2_defconfig
@@ -12,7 +12,7 @@ CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_BOOTDELAY=3
 CONFIG_SYS_CONSOLE_IS_IN_ENV=y
-CONFIG_DEFAULT_FDT_FILE="mt7623n-bananapi-bpi-r2"
+CONFIG_DEFAULT_FDT_FILE="mt7623n-bananapi-bpi-r2.dtb"
 # CONFIG_DISPLAY_BOARDINFO is not set
 CONFIG_SYS_PROMPT="U-Boot> "
 CONFIG_CMD_BOOTMENU=y
diff --git a/include/configs/mt7623.h b/include/configs/mt7623.h
index fe436cca38..33e07ed646 100644
--- a/include/configs/mt7623.h
+++ b/include/configs/mt7623.h
@@ -51,7 +51,7 @@
"fdt_high=" FDT_HIGH "\0"   \
"kernel_addr_r=0x8400\0"\
"fdt_addr_r=" FDT_HIGH "\0" \
-   "fdtfile=mt7623n-bananapi-bpi-r2.dtb" "\0"
+   "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0"
 
 /* Ethernet */
 #define CONFIG_IPADDR  192.168.1.1
-- 
2.26.2



[PATCH 3/3] board: mediatek: Add support for UniElec U7623 board

2020-07-12 Thread David Woodhouse
This is an MT7623A-based board, very similar to the Banana Pi R2.

http://www.unielecinc.com/q/news/cn/p/product/detail.html?qd_guid=OjXwKCaRlN

Signed-off-by: David Woodhouse 
---
 arch/arm/dts/Makefile |   1 +
 .../arm/dts/mt7623a-unielec-u7623-02-emmc.dts | 211 ++
 configs/mt7623a_unielec_u7623_02_defconfig|  54 +
 3 files changed, 266 insertions(+)
 create mode 100644 arch/arm/dts/mt7623a-unielec-u7623-02-emmc.dts
 create mode 100644 configs/mt7623a_unielec_u7623_02_defconfig

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 89fa448818..a140b1c8d4 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -938,6 +938,7 @@ dtb-$(CONFIG_SOC_K3_J721E) += 
k3-j721e-common-proc-board.dtb \
 
 dtb-$(CONFIG_ARCH_MEDIATEK) += \
mt7622-rfb.dtb \
+   mt7623a-unielec-u7623-02-emmc.dtb \
mt7623n-bananapi-bpi-r2.dtb \
mt7629-rfb.dtb \
mt8512-bm1-emmc.dtb \
diff --git a/arch/arm/dts/mt7623a-unielec-u7623-02-emmc.dts 
b/arch/arm/dts/mt7623a-unielec-u7623-02-emmc.dts
new file mode 100644
index 00..fdeec75b05
--- /dev/null
+++ b/arch/arm/dts/mt7623a-unielec-u7623-02-emmc.dts
@@ -0,0 +1,211 @@
+/*
+ * Copyright (C) 2018 MediaTek Inc.
+ * Author: Ryder Lee 
+ *
+ * SPDX-License-Identifier: (GPL-2.0 OR MIT)
+ */
+
+/dts-v1/;
+#include "mt7623.dtsi"
+#include "mt7623-u-boot.dtsi"
+
+/ {
+   model = "UniElec U7623-02 eMMC";
+   compatible = "unielec,u7623-02-emmc", "mediatek,mt7623";
+
+   memory@8000 {
+   device_type = "memory";
+   reg = <0 0x8000 0 0x2000>;
+   };
+
+   chosen {
+   stdout-path = &uart2;
+   tick-timer = &timer0;
+   };
+
+   reg_1p8v: regulator-1p8v {
+   compatible = "regulator-fixed";
+   regulator-name = "fixed-1.8V";
+   regulator-min-microvolt = <180>;
+   regulator-max-microvolt = <180>;
+   regulator-boot-on;
+   regulator-always-on;
+   };
+
+   reg_3p3v: regulator-3p3v {
+   compatible = "regulator-fixed";
+   regulator-name = "fixed-3.3V";
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   regulator-boot-on;
+   regulator-always-on;
+   };
+
+   reg_5v: regulator-5v {
+   compatible = "regulator-fixed";
+   regulator-name = "fixed-5V";
+   regulator-min-microvolt = <500>;
+   regulator-max-microvolt = <500>;
+   regulator-boot-on;
+   regulator-always-on;
+   };
+
+   leds {
+   compatible = "gpio-leds";
+
+   led3 {
+   label = "u7623-01:green:led3";
+   gpios = <&gpio 14 GPIO_ACTIVE_LOW>;
+   default-state = "off";
+   };
+
+   led4 {
+   label = "u7623-01:green:led4";
+   gpios = <&gpio 15 GPIO_ACTIVE_LOW>;
+   default-state = "off";
+   };
+   };
+};
+
+ð {
+   status = "okay";
+   mediatek,gmac-id = <0>;
+   phy-mode = "rgmii";
+   mediatek,switch = "mt7530";
+   mediatek,mcm;
+
+   fixed-link {
+   speed = <1000>;
+   full-duplex;
+   };
+};
+
+&mmc0 {
+   pinctrl-names = "default";
+   pinctrl-0 = <&mmc0_pins_default>;
+   status = "okay";
+   bus-width = <8>;
+   max-frequency = <5000>;
+   cap-mmc-highspeed;
+   vmmc-supply = <®_3p3v>;
+   vqmmc-supply = <®_1p8v>;
+   non-removable;
+};
+
+&pinctrl {
+   ephy_default: ephy_default {
+   mux {
+   function = "eth";
+   groups = "mdc_mdio", "ephy";
+   };
+
+   conf {
+   pins = "G2_TXEN", "G2_TXD0", "G2_TXD1", "G2_TXD2",
+  "G2_TXD3", "G2_TXC", "G2_RXC", "G2_RXD0",
+  "G2_RXD1", "G2_RXD2", "G2_RXD3", "G2_RXDV",
+  "MDC", "MDIO";
+   drive-strength = <12>;
+   mediatek,tdsel = <5>;
+   };
+   };
+
+   mmc0_pins_default: mmc0default {
+   mux {
+   function = "msdc";
+   groups =  "msdc0";
+   };
+
+   conf-cmd-data {
+   pins = "MSDC0_CMD", "MSDC0_DAT0", "MSDC0_DAT1",
+  "MSDC0_DAT2", "MSDC0_DAT3", "MSDC0_DAT4",
+  "MSDC0_DAT5", "MSDC0_DAT6", "MSDC0_DAT7";
+   input-enable;
+   bias-pull-up;
+   };
+
+   conf-clk {
+   pins = "MSDC0_CLK";
+   bias-pull-down;
+   };
+
+  

[PATCH 1/3] board: mediatek: fix mmc_get_boot_dev() for platforms without external SD

2020-07-12 Thread David Woodhouse
On the UniElec U7623 board there is no external SD slot and the preloader
doesn't fill in the magic field at 0x81d0 to indicate that it was
booted from eMMC.

Signed-off-by: David Woodhouse 
---
 board/mediatek/mt7623/mt7623_rfb.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/board/mediatek/mt7623/mt7623_rfb.c 
b/board/mediatek/mt7623/mt7623_rfb.c
index 4ec2764976..984e75ccaf 100644
--- a/board/mediatek/mt7623/mt7623_rfb.c
+++ b/board/mediatek/mt7623/mt7623_rfb.c
@@ -4,6 +4,7 @@
  */
 
 #include 
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -15,10 +16,15 @@ int board_init(void)
return 0;
 }
 
+#ifdef CONFIG_MMC
 int mmc_get_boot_dev(void)
 {
int g_mmc_devid = -1;
char *uflag = (char *)0x81D0;
+
+   if (!find_mmc_device(1))
+   return 0;
+
if (strncmp(uflag,"eMMC",4)==0) {
g_mmc_devid = 0;
printf("Boot From Emmc(id:%d)\n\n", g_mmc_devid);
@@ -33,3 +39,4 @@ int mmc_get_env_dev(void)
 {
return mmc_get_boot_dev();
 }
+#endif
-- 
2.26.2



Re: [PATCH v1 07/43] dm: acpi: Add support for the NHLT table

2020-07-12 Thread Simon Glass
Hi Bin,

On Tue, 7 Jul 2020 at 21:25, Simon Glass  wrote:
>
> Hi Bin,
>
> On Mon, 6 Jul 2020 at 18:22, Bin Meng  wrote:
> >
> > Hi Simon,
> >
> > On Tue, Jul 7, 2020 at 3:22 AM Simon Glass  wrote:
> > >
> > > Hi Bin,
> > >
> > > On Thu, 2 Jul 2020 at 22:33, Bin Meng  wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Fri, Jul 3, 2020 at 11:50 AM Simon Glass  wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Thu, 2 Jul 2020 at 18:54, Bin Meng  wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Fri, Jul 3, 2020 at 8:46 AM Simon Glass  
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Bin,
> > > > > > >
> > > > > > > On Mon, 29 Jun 2020 at 20:49, Bin Meng  wrote:
> > > > > > > >
> > > > > > > > Hi Simon,
> > > > > > > >
> > > > > > > > On Mon, Jun 15, 2020 at 11:57 AM Simon Glass 
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > The Intel Non-High-Definition-Audio Link Table (NHLT) table 
> > > > > > > > > describes the
> > > > > > > > > audio codecs and connections in a system. Various devices can 
> > > > > > > > > contribute
> > > > > > > > > information to produce the table.
> > > > > > > > >
> > > > > > > > > Add core support for this, based on a structure which is 
> > > > > > > > > built up through
> > > > > > > > > calls to the driver.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Simon Glass 
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > >  drivers/core/acpi.c | 15 +++
> > > > > > > > >  include/dm/acpi.h   | 26 ++
> > > > > > > > >  2 files changed, 41 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c
> > > > > > > > > index ea304a3067..a5053fec6f 100644
> > > > > > > > > --- a/drivers/core/acpi.c
> > > > > > > > > +++ b/drivers/core/acpi.c
> > > > > > > > > @@ -31,6 +31,7 @@ enum method_t {
> > > > > > > > > METHOD_WRITE_TABLES,
> > > > > > > > > METHOD_FILL_SSDT,
> > > > > > > > > METHOD_INJECT_DSDT,
> > > > > > > > > +   METHOD_SETUP_NHLT,
> > > > > > > >
> > > > > > > > Do we really need to promote this to be an ACPI core method? 
> > > > > > > > Can we
> > > > > > > > reuse the SSDT/DSDT one?
> > > > > > >
> > > > > > > I don't think so. Those two are for a particular purpose. In fact 
> > > > > > > NHLT
> > > > > > > is generated while doing SSDT I think. The idea is that drivers 
> > > > > > > that
> > > > > > > want to contribute to NHLT can do so. But we cannot use the SSDT
> > > > > > > mechanism since each driver contributes only a part of the info, 
> > > > > > > and
> > > > > > > we need something else to bring it all together.
> > > > > >
> > > > > > Will there be only one device that sets up the NHLT info?
> > > > >
> > > > > WIth coral it is two devices. I'm not sure of the maximum, but I
> > > > > suppose it depends on the audio codecs present.
> > > >
> > > > Could we make this method to be provided by the codec device, instead
> > > > of a generic ACPI core method?
> > >
> > > The codec device does implement this. See the drivers where they
> > > actually implement the NHLT method.
> > >
> > > This is definitely an ACPI-specific thing, so I think we need core
> > > support for iterating through drivers that want to provide this info.
> >
> > My concern is that this is not generic enough to promote this to ACPI core.
> >
> > I wanted to have something like this:
> >
> > Create a codec uclass driver, and in the code uclass driver, create an
> > op that is used to set up the NHLT infor if ACPI_GEN is on.
>
> We already have UCLASS_SOUND so could add it to sound_ops. But that
> seems weird to me since this is not an operation to play a sound. We
> do this with GIPOs and IRQs but in that case the operation returns
> some data. Here we are asking the driver to add some data to a table.
> I'm just not sure it makes sense.
>
> What do you think?

Let me know what you think about this as I'd like to finish this series.

Regards,
SImon


[PATCH] cmd: mvebu/bubt: Drop unused SPI_FLASH_PROTECTION

2020-07-12 Thread Jagan Teki
SPI_FLASH_PROTECTION config item is never used in anywhere
in the U-Boot tree.

Drop it.

Signed-off-by: Jagan Teki 
---
 cmd/mvebu/bubt.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
index a27b0df8ae..b3b5844267 100644
--- a/cmd/mvebu/bubt.c
+++ b/cmd/mvebu/bubt.c
@@ -299,9 +299,6 @@ static int spi_burn_image(size_t image_size)
return -ENOMEDIUM;
}
 
-#ifdef CONFIG_SPI_FLASH_PROTECTION
-   spi_flash_protect(flash, 0);
-#endif
erase_bytes = image_size +
(flash->erase_size - image_size % flash->erase_size);
printf("Erasing %d bytes (%d blocks) at offset 0 ...",
@@ -320,10 +317,6 @@ static int spi_burn_image(size_t image_size)
else
printf("Done!\n");
 
-#ifdef CONFIG_SPI_FLASH_PROTECTION
-   spi_flash_protect(flash, 1);
-#endif
-
return ret;
 }
 
-- 
2.25.1



[PATCH] config: Enable USB Keyboard suuport on RPi4 32 bit

2020-07-12 Thread matthias . bgg
From: Matthias Brugger 

Supporting USB keyboards out of the box is both handy for development
and production. Notably if u-boot is used to boot into GRUB. This patch
adds USB keyboard support for 32 bit RPi4 config.

Signed-off-by: Matthias Brugger 

---

 configs/rpi_4_32b_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/rpi_4_32b_defconfig b/configs/rpi_4_32b_defconfig
index db7b781976..4a88448e9d 100644
--- a/configs/rpi_4_32b_defconfig
+++ b/configs/rpi_4_32b_defconfig
@@ -42,6 +42,7 @@ CONFIG_DM_USB=y
 CONFIG_DM_USB_GADGET=y
 CONFIG_USB_XHCI_HCD=y
 CONFIG_USB_XHCI_PCI=y
+CONFIG_USB_KEYBOARD=y
 CONFIG_USB_GADGET=y
 CONFIG_USB_GADGET_MANUFACTURER="FSL"
 CONFIG_USB_GADGET_VENDOR_NUM=0x0525
-- 
2.27.0



Re: Seemless Boot Splash on iMX-based boards

2020-07-12 Thread Igor Opaniuk
Hi Stefano,

On Mon, Jul 6, 2020 at 12:10 PM Stefano Babic  wrote:
>
> Hi Igor,
>
> my two cents from previous experience:
>
> On 06.07.20 10:34, Igor Opaniuk wrote:
> > Hi,
> >
> > Does anyone have experience in setting up seamless
> > boot splash on iMX-based platforms?
> >
> > I'm currently trying to do that on iMX7-based board with
> > 4.9 2.3.x IMX downstream kernel running.
> >
>
> Well, i.MX7 have a chance because it has no GPU. I did this for TI based
> boards, too.
>
> But let me say that it is tricky and it creates big dependencies between
> U-Boot and kernel. And to get a flicker-free images, the clock setup
> should be exactly the same between U-Boot and kernel, and this is not
> often (or never) the same because we need a lot of more features
> (layers, overlays, ..) in kernel, while we just to need to show
> something like a logo in U-Boot.
>
> With the introduction of DRM, things are more difficult. Take also into
> account that a small change in kernel can be enough to have a weird
> transition on the framebuffer, and it is more nasty as to start with a
> black screen.
>
> > I've backported console deferred takeover patch-series for fbcon [1],
> > which permits the contents of the framebuffer (initialized before in U-Boot)
> > to stay in place as is till error message is shown
> > or boot is finished.
> >
> > The initial splash is shown in U-Boot(mainline) (mxsfb driver is used
> > for controller/fb initialization).
> >
> > Nevertheless, MXSFB controller every time keeps resetting just after
> > U-Boot->Linux takeover and fb is cleared (I see a black screen) till
> > login request shows up.
>
> Right.
>
> >
> > Questions:
> > 1. Did anyone have a chance to work on such setups based on deferred
> > console "feature"?
> > 2. Does it make sense at all to continue moving towards with this approach
> > (we initialize graphics core and show boot splash by firmware/bootloader,
> > then hand over it to Linux).
>
> My personal point of view: it makes much more sense to invest the time
> to speed up the boot process, and let the kernel to boot in a shorten
> time. And if this time is short enough, kernel has initialized the
> framebuffer and can show something. This leads also to the main focus
> for U-Boot, that should be to start the kernel in the shortest time
> possible.
>
> > Taking into account on-going transition to DRM/KMS in the mainline kernel
>
> Fully agree.
>
> > and that fact that there is no any mainline compatible way to take over
> > an initialized graphics core, I assume the only generic solution could be
> > avoiding showing boot logo
>
> This is my position, too, and I do not show logo in U-Boot since some
> years (ok, since starting from DRM /KMS). And well, with i.MX6 / i.MX8,
> I do not see a chance without a very big effort, and IMHO it is not
> worth of it.
>
> > at all and do that only from the Linux (
> > Plymouth-based etc.)
>
> Agree.
>
> >
> > Any comments are welcome. Thanks!
> >
> > [1] https://patchwork.kernel.org/patch/10432641/
> >
>
> Best regards,
> Stefano
>
> --
> =
> DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
> =

Thanks a lot for your valuable input!

-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opan...@gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk