Re: [PATCH v2 1/9] power: pmic-uclass: implement poweroff ops

2023-07-20 Thread Svyatoslav Ryhel
чт, 20 лип. 2023 р. о 22:43 Simon Glass  пише:
>
> Hi Svyatoslav,
>
> On Thu, 20 Jul 2023 at 02:48, Svyatoslav Ryhel  wrote:
> >
> > PMICs are responsible for device poweroff sequence so lets implement
> > this function.
> >
> > Signed-off-by: Svyatoslav Ryhel 
> > ---
> >  drivers/power/pmic/pmic-uclass.c | 12 
> >  include/power/pmic.h | 13 +
> >  2 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/power/pmic/pmic-uclass.c 
> > b/drivers/power/pmic/pmic-uclass.c
> > index 0e2f5e1f41..23803bc96a 100644
> > --- a/drivers/power/pmic/pmic-uclass.c
> > +++ b/drivers/power/pmic/pmic-uclass.c
> > @@ -99,6 +99,18 @@ int pmic_get(const char *name, struct udevice **devp)
> > return uclass_get_device_by_name(UCLASS_PMIC, name, devp);
> >  }
> >
> > +int pmic_poweroff(struct udevice *dev)
> > +{
> > +   const struct dm_pmic_ops *ops = dev_get_driver_ops(dev);
> > +   struct uc_pmic_priv *priv = dev_get_uclass_priv(dev);
> > +
> > +   if (!ops || !ops->poweroff ||
> > +   !priv->sys_pow_ctrl)
> > +   return -ENOSYS;
> > +
> > +   return ops->poweroff(dev);
> > +}
> > +
> >  int pmic_reg_count(struct udevice *dev)
> >  {
> > const struct dm_pmic_ops *ops = dev_get_driver_ops(dev);
> > diff --git a/include/power/pmic.h b/include/power/pmic.h
> > index 636221692d..d8dd1ceaa3 100644
> > --- a/include/power/pmic.h
> > +++ b/include/power/pmic.h
> > @@ -157,11 +157,13 @@ struct pmic {
> >   * Should be implemented by UCLASS_PMIC device drivers. The standard
> >   * device operations provides the I/O interface for it's childs.
> >   *
> > + * @poweroff:  perform poweroff sequence
> >   * @reg_count: device's register count
> >   * @read:  read 'len' bytes at "reg" and store it into the 'buffer'
> >   * @write: write 'len' bytes from the 'buffer' to the register at 
> > 'reg' address
> >   */
> >  struct dm_pmic_ops {
> > +   int (*poweroff)(struct udevice *dev);
>
> Please do remember to fully comment each method here too - see p2sb
> set_hide() for an example.
>

It is commented in description before ops
+ * @poweroff:  perform poweroff sequence

p2sb set_hide() description is nice but it does not fit this case
since its description is set before ops.

> > int (*reg_count)(struct udevice *dev);
> > int (*read)(struct udevice *dev, uint reg, uint8_t *buffer, int 
> > len);
> > int (*write)(struct udevice *dev, uint reg, const uint8_t *buffer,
> > @@ -242,6 +244,16 @@ int pmic_bind_children(struct udevice *pmic, ofnode 
> > parent,
> >   */
> >  int pmic_get(const char *name, struct udevice **devp);
> >
> > +/**
> > + * pmic_poweroff: call the pmic poweroff sequence
> > + *
> > + * The required pmic device can be obtained by 'pmic_get()'
> > + *
> > + * @dev - pointer to the UCLASS_PMIC device
> > + * Return: device turns off or negative value of errno.
> > + */
> > +int pmic_poweroff(struct udevice *dev);
> > +
> >  /**
> >   * pmic_reg_count: get the pmic register count
> >   *
> > @@ -306,6 +318,7 @@ int pmic_clrsetbits(struct udevice *dev, uint reg, uint 
> > clr, uint set);
> >   */
> >  struct uc_pmic_priv {
> > uint trans_len;
> > +   bool sys_pow_ctrl;
>
> comment
>
> >  };
> >
> >  #endif /* DM_PMIC */
> > --
> > 2.39.2
> >
>
> This needs a test addition to test/dm/pmic.c (perhaps you have it in
> another patch).

I knew you would ask for this and I have thought about implementing a
dm test but I have faced the next issue: poweroff call (in this
context) performs device poweroff on success and -err on failure. Iirc
sandbox pmic is not dedicated for poweroff and it can not be
"emulated" correctly on sandbox (emulation will be just a passing
value like for example pmic_reg_count test).

If this is mandatory then tell pls which behavior is expected in the
test and I will try to replicate it.

> Regards,
> Simon


Re: [PATCH v2 3/7] power: regulator-uclass: perform regulator setup inside uclass

2023-07-20 Thread Svyatoslav Ryhel
чт, 20 лип. 2023 р. о 22:43 Simon Glass  пише:
>
> Hi Svyatoslav,
>
> On Thu, 20 Jul 2023 at 06:38, Svyatoslav Ryhel  wrote:
> >
> > Regulators initial setup was previously dependent on board call.
> > To move from this behaviour were introduced two steps. First is
> > that all individual regulators will be probed just after binding
>
> We must not probe devices automatically when bound. The i2c interface
> may not be available, etc. Do a probe afterwards.
>
> Perhaps I am misunderstanding this, iwc please reword this commit message.

After bound. If the regulator is a self-sufficient i2c device then it
will be bound
after i2c is available by code design so i2c interface should be
available at that
moment. At least led and gpio uclasses perform this for initial setup
of devices.

Platform regulators (aka fixed/gpio regulators) work perfectly fine. I
have no i2c
regulators to test deeply.

As for now only one uclass is not compatible with this system - PMIC which has
strong dependency between regulator and main mfd. This is why probing after
bind for pmic regulators is disabled and pmic regulators are probed on pmic core
post_probe.

> > which ensures that regulator pdata is filled and second is setting
> > up regulator in post probe which enseres that correct regulator
> > state according to pdata is reached.
> >
> > Signed-off-by: Svyatoslav Ryhel 
> > ---
> >  drivers/power/regulator/regulator-uclass.c | 212 ++---
> >  include/power/regulator.h  | 119 
> >  2 files changed, 62 insertions(+), 269 deletions(-)
>
> Regards,
> SImon


Re: [PATCH v2 1/7] power: regulator: expand basic reference counter onto all uclass

2023-07-20 Thread Svyatoslav Ryhel
чт, 20 лип. 2023 р. о 22:43 Simon Glass  пише:
>
> Hi Svyatoslav,
>
> On Thu, 20 Jul 2023 at 06:38, Svyatoslav Ryhel  wrote:
> >
> > Commit is based on 4fcba5d ("regulator: implement basic reference
> > counter") but expands the idea to all regulators instead of just
> > fixed/gpio regulators.
> >
> > Signed-off-by: Svyatoslav Ryhel 
> > ---
> >  drivers/power/regulator/regulator-uclass.c | 41 ++
> >  drivers/power/regulator/regulator_common.c | 22 
> >  drivers/power/regulator/regulator_common.h | 21 ---
> >  include/power/regulator.h  |  2 ++
> >  4 files changed, 43 insertions(+), 43 deletions(-)
>
> Reviewed-by: Simon Glass 
>
> nit below
>
> >
> > diff --git a/drivers/power/regulator/regulator-uclass.c 
> > b/drivers/power/regulator/regulator-uclass.c
> > index 3a6ba69f6d..fc7a4631b4 100644
> > --- a/drivers/power/regulator/regulator-uclass.c
> > +++ b/drivers/power/regulator/regulator-uclass.c
> > @@ -159,6 +159,25 @@ int regulator_get_enable(struct udevice *dev)
> > return ops->get_enable(dev);
> >  }
> >
> > +/*
> > + * Enable or Disable a regulator
> > + *
> > + * This is a reentrant function and subsequent calls that enable will
> > + * increase an internal counter, and disable calls will decrease the 
> > counter.
> > + * The actual resource will be enabled when the counter gets to 1 coming 
> > from 0,
> > + * and disabled when it reaches 0 coming from 1.
> > + *
> > + * @dev: regulator device
> > + * @enable: bool indicating whether to enable or disable the regulator
> > + * @return:
> > + * 0 on Success
> > + * -EBUSY if the regulator cannot be disabled because it's requested by
> > + *another device
> > + * -EALREADY if the regulator has already been enabled or has already been
> > + *disabled
> > + * -EACCES if there is no possibility to enable/disable the regulator
> > + * -ve on different error situation
> > + */
> >  int regulator_set_enable(struct udevice *dev, bool enable)
> >  {
> > const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
> > @@ -172,6 +191,23 @@ int regulator_set_enable(struct udevice *dev, bool 
> > enable)
> > if (!enable && uc_pdata->always_on)
> > return -EACCES;
> >
> > +   /* If previously enabled, increase count */
> > +   if (enable && uc_pdata->enable_count > 0) {
> > +   uc_pdata->enable_count++;
> > +   return -EALREADY;
> > +   }
> > +
> > +   if (!enable) {
> > +   if (uc_pdata->enable_count > 1) {
> > +   /* If enabled multiple times, decrease count */
> > +   uc_pdata->enable_count--;
> > +   return -EBUSY;
> > +   } else if (!uc_pdata->enable_count) {
> > +   /* If already disabled, do nothing */
> > +   return -EALREADY;
> > +   }
> > +   }
> > +
> > if (uc_pdata->ramp_delay)
> > old_enable = regulator_get_enable(dev);
> >
> > @@ -187,6 +223,11 @@ int regulator_set_enable(struct udevice *dev, bool 
> > enable)
> > }
> > }
> >
> > +   if (enable)
> > +   uc_pdata->enable_count++;
> > +   else
> > +   uc_pdata->enable_count--;
> > +
> > return ret;
> >  }
> >
> > diff --git a/drivers/power/regulator/regulator_common.c 
> > b/drivers/power/regulator/regulator_common.c
> > index e26f5ebec3..d88bc6f6de 100644
> > --- a/drivers/power/regulator/regulator_common.c
> > +++ b/drivers/power/regulator/regulator_common.c
> > @@ -72,23 +72,6 @@ int regulator_common_set_enable(const struct udevice 
> > *dev,
> > return 0;
> > }
> >
> > -   /* If previously enabled, increase count */
> > -   if (enable && plat->enable_count > 0) {
> > -   plat->enable_count++;
> > -   return -EALREADY;
> > -   }
> > -
> > -   if (!enable) {
> > -   if (plat->enable_count > 1) {
> > -   /* If enabled multiple times, decrease count */
> > -   plat->enable_count--;
> > -   return -EBUSY;
> > -   } else if (!plat->enable_count) {
> > -   /* If already disabled, do nothing */
> > -   return -EALREADY;
> > -   }
> > -   }
> > -
> > ret = dm_gpio_set_value(>gpio, enable);
> > if (ret) {
> > pr_err("Can't set regulator : %s gpio to: %d\n", dev->name,
> > @@ -103,10 +86,5 @@ int regulator_common_set_enable(const struct udevice 
> > *dev,
> > if (!enable && plat->off_on_delay_us)
> > udelay(plat->off_on_delay_us);
> >
> > -   if (enable)
> > -   plat->enable_count++;
> > -   else
> > -   plat->enable_count--;
> > -
> > return 0;
> >  }
> > diff --git a/drivers/power/regulator/regulator_common.h 
> > 

Re: [PATCH v2 8/9] power: pmic: tps65910: add TPS65911 PMIC support

2023-07-20 Thread Svyatoslav Ryhel
чт, 20 лип. 2023 р. о 22:43 Simon Glass  пише:
>
> On Thu, 20 Jul 2023 at 02:48, Svyatoslav Ryhel  wrote:
> >
> > Add support to bind the regulators/child nodes with the pmic.
> > Also adds the pmic i2c based read/write functions to access pmic
> > registers.
> >
> > Signed-off-by: Svyatoslav Ryhel 
> > ---
> >  doc/device-tree-bindings/pmic/tps65911.txt | 78 ++
> >  drivers/power/pmic/pmic_tps65910_dm.c  | 49 +-
> >  include/power/tps65910_pmic.h  | 52 +++
> >  3 files changed, 176 insertions(+), 3 deletions(-)
> >  create mode 100644 doc/device-tree-bindings/pmic/tps65911.txt
> >
>
> Reviewed-by: Simon Glass 
>
> nits below
>
> [..]
>
> > diff --git a/drivers/power/pmic/pmic_tps65910_dm.c 
> > b/drivers/power/pmic/pmic_tps65910_dm.c
> > index e03ddc98d7..770010d53d 100644
> > --- a/drivers/power/pmic/pmic_tps65910_dm.c
> > +++ b/drivers/power/pmic/pmic_tps65910_dm.c
> > @@ -11,13 +11,19 @@
> >  #include 
> >  #include 
> >
> > -static const struct pmic_child_info pmic_children_info[] = {
> > +static const struct pmic_child_info tps65910_children_info[] = {
> > { .prefix = "ldo_", .driver = TPS65910_LDO_DRIVER },
> > { .prefix = "buck_", .driver = TPS65910_BUCK_DRIVER },
> > { .prefix = "boost_", .driver = TPS65910_BOOST_DRIVER },
> > { },
> >  };
> >
> > +static const struct pmic_child_info tps65911_children_info[] = {
> > +   { .prefix = "ldo", .driver = TPS65911_LDO_DRIVER },
> > +   { .prefix = "vdd", .driver = TPS65911_VDD_DRIVER },
> > +   { },
> > +};
> > +
> >  static int pmic_tps65910_reg_count(struct udevice *dev)
> >  {
> > return TPS65910_NUM_REGS;
> > @@ -47,10 +53,29 @@ static int pmic_tps65910_read(struct udevice *dev, uint 
> > reg, u8 *buffer,
> > return ret;
> >  }
> >
> > +static int pmic_tps65910_poweroff(struct udevice *dev)
> > +{
> > +   int ret;
> > +
> > +   ret = pmic_reg_read(dev, TPS65910_REG_DEVICE_CTRL);
> > +   if (ret < 0)
> > +   return ret;
> > +
> > +   ret |= DEVCTRL_PWR_OFF_MASK;
>
> Please use a separate var for this. We use ret for return values, as
> you have above.
>
> > +
> > +   pmic_reg_write(dev, TPS65910_REG_DEVICE_CTRL, ret);
> > +
> > +   ret |= DEVCTRL_DEV_OFF_MASK;
> > +   ret &= ~DEVCTRL_DEV_ON_MASK;
> > +
> > +   return pmic_reg_write(dev, TPS65910_REG_DEVICE_CTRL, ret);
> > +}
> > +
> >  static int pmic_tps65910_bind(struct udevice *dev)
> >  {
> > ofnode regulators_node;
> > int children;
> > +   int type = dev_get_driver_data(dev);
> >
> > regulators_node = dev_read_subnode(dev, "regulators");
> > if (!ofnode_valid(regulators_node)) {
> > @@ -58,7 +83,19 @@ static int pmic_tps65910_bind(struct udevice *dev)
> > return -EINVAL;
> > }
> >
> > -   children = pmic_bind_children(dev, regulators_node, 
> > pmic_children_info);
> > +   switch (type) {
>
> Why not always binding them?
>

They may not be enabled, each pmic has its own regulator driver and they
can be enabled separately. Not sure if binding without enabled driver is
permitted so I decided to preemptively separate them to avoid any issues.

> > +   case TPS65910:
> > +   children = pmic_bind_children(dev, regulators_node,
> > + tps65910_children_info);
> > +   break;
> > +   case TPS65911:
> > +   children = pmic_bind_children(dev, regulators_node,
> > + tps65911_children_info);
> > +   break;
> > +   default:
> > +   log_err("unknown PMIC type\n");
> > +   }
> > +
> > if (!children)
> > debug("%s has no children (regulators)\n", dev->name);
> >
> > @@ -67,6 +104,10 @@ static int pmic_tps65910_bind(struct udevice *dev)
> >
> >  static int pmic_tps65910_probe(struct udevice *dev)
> >  {
> > +   struct uc_pmic_priv *priv = dev_get_uclass_priv(dev);
> > +
> > +   priv->sys_pow_ctrl = dev_read_bool(dev, 
> > "ti,system-power-controller");
> > +
> > /* use I2C control interface instead of I2C smartreflex interface to
> >  * access smartrefelex registers VDD1_OP_REG, VDD1_SR_REG, 
> > VDD2_OP_REG
> >  * and VDD2_SR_REG
> > @@ -76,13 +117,15 @@ static int pmic_tps65910_probe(struct udevice *dev)
> >  }
> >
> >  static struct dm_pmic_ops pmic_tps65910_ops = {
> > +   .poweroff = pmic_tps65910_poweroff,
> > .reg_count = pmic_tps65910_reg_count,
> > .read = pmic_tps65910_read,
> > .write = pmic_tps65910_write,
> >  };
> >
> >  static const struct udevice_id pmic_tps65910_match[] = {
> > -   { .compatible = "ti,tps65910" },
> > +   { .compatible = "ti,tps65910", .data = TPS65910 },
> > +   { .compatible = "ti,tps65911", .data = TPS65911 },
> > { /* sentinel */ }
> >  };
> >
> > diff --git 

Re: [PATCH v5 02/46] x86: Return mtrr_add_request() to its old purpose

2023-07-20 Thread Bin Meng
On Sun, Jul 16, 2023 at 11:39 AM Simon Glass  wrote:
>
> This function used to be for adding a list of requests to be actioned on
> relocation. Revert it back to this purpose, to avoid problems with boards
> which need control of their MTRRs (i.e. those which don't use FSP).
>
> The mtrr_set_next_var() function is available when the next free
> variable-MTRR must be set, so this can be used instead.
>
> Signed-off-by: Simon Glass 
> Fixes: 3bcd6cf89ef ("x86: mtrr: Skip MSRs that were already programmed..")
> Fixes: 596bd0589ad ("x86: mtrr: Do not clear the unused ones..")
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Fix invalid commit IDs obtained from another branch
>
>  arch/x86/cpu/mtrr.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng 

I will include this patch in my mtrr series.

Regards,
Bin


Re: [PATCH v1] HSD #18028953892: usb: xhci-dwc3: Fix USB3.1 controller register access in reset state

2023-07-20 Thread Bin Meng
On Wed, Jun 21, 2023 at 10:11 PM Jit Loon Lim  wrote:
>
> From: Teik Heng Chong 
>
> The controller registers should not be accessed while the controller's
> vcc_reset_n is asserted.
>
> Signed-off-by: Teik Heng Chong 
> ---
>  drivers/usb/host/xhci-dwc3.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c
> index 1dbd65dfaa..3172956bd6 100644
> --- a/drivers/usb/host/xhci-dwc3.c
> +++ b/drivers/usb/host/xhci-dwc3.c
> @@ -227,6 +227,11 @@ static int xhci_dwc3_probe(struct udevice *dev)
>  static int xhci_dwc3_remove(struct udevice *dev)
>  {
> struct xhci_dwc3_plat *plat = dev_get_plat(dev);
> +   int ret;
> +
> +   ret = xhci_deregister(dev);
> +   if (ret)
> +   return ret;
>
> dwc3_shutdown_phy(dev, >phys);
>
> @@ -234,7 +239,7 @@ static int xhci_dwc3_remove(struct udevice *dev)
>
> reset_release_bulk(>resets);
>
> -   return xhci_deregister(dev);
> +   return 0;
>  }
>
>  static const struct udevice_id xhci_dwc3_ids[] = {

Please remove the HSD tag in the commit summary.

Regards,
Bin


Re: [PATCH] efi_loader: Allow also empty capsule to be process

2023-07-20 Thread AKASHI Takahiro
On Thu, Jul 20, 2023 at 10:42:10PM +0200, Heinrich Schuchardt wrote:
> On 7/20/23 11:48, Sughosh Ganu wrote:
> > On Thu, 20 Jul 2023 at 14:56, Michal Simek  wrote:
> > > 
> > > 
> > > 
> > > On 7/20/23 10:45, Sughosh Ganu wrote:
> > > > On Thu, 20 Jul 2023 at 13:26, Michal Simek  wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > On 7/20/23 08:36, Sughosh Ganu wrote:
> > > > > > On Thu, 20 Jul 2023 at 11:37, Michal Simek  
> > > > > > wrote:
> > > > > > > 
> > > > > > > Hi,
> > > > > > > 
> > > > > > > On 7/20/23 07:49, AKASHI Takahiro wrote:
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > On Wed, Jul 19, 2023 at 08:28:41AM +0200, Michal Simek wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On 7/18/23 17:41, Heinrich Schuchardt wrote:
> > > > > > > > > > On 13.07.23 16:35, Michal Simek wrote:
> > > > > > > > > > > Empty capsule are also allowed to be process. Without it 
> > > > > > > > > > > updated images
> > > > > > > > > > > can't change their Image Acceptance state from no to yes.
> > > > > > > > > > 
> > > > > > > > > > Is there any documentation describing the usage of empty 
> > > > > > > > > > capsule to set
> > > > > > > > > > the image acceptance state?
> > > > > > > > > 
> > > > > > > > > I actually don't know about documentation. I was talking to 
> > > > > > > > > Ilias to make
> > > > > > > > > sure that documentation is up2date because there are missing 
> > > > > > > > > couple of
> > > > > > > > > things there.
> > > > > > > > 
> > > > > > > > Sughosh should have more to say here about A/B update.
> > > > > > > > 
> > > > > > > > > I am testing A/B update and if you setup oemflags to 0x8000 
> > > > > > > > > then capsules
> > > > > > > > > are not automatically accepted and waiting for acceptance 
> > > > > > > > > capsule to be
> > > > > > > > > passed.
> > > > > > > > > When I tested it I found out that they are not process that's 
> > > > > > > > > why I created
> > > > > > > > > this patch.
> > > > > > > > 
> > > > > > > > The path you tried to modify is only executed by "efidebug 
> > > > > > > > capsule update"
> > > > > > > > or more specifically via the runtime service, UPDATE_CAPSULE.
> > > > > > > > 
> > > > > > > > But this API is NOT officially supported in the current capsule 
> > > > > > > > implementation
> > > > > > > > (at least, in my initial intention).
> > > > > > > > The only way to invoke capsule updates is to reboot the system.
> > > > > > > > If you want to test A/B update, please do the reboot.
> > > > > > > 
> > > > > > > I realized that to get full flow you need to use capsule update 
> > > > > > > on disk to get
> > > > > > > all functionalities. But it is very impractical. Actually I would 
> > > > > > > expect via
> > > > > > > efidebug you should be able to perform all steps as capsule 
> > > > > > > update performs when
> > > > > > > you do reboot.
> > > > > > > I would also understand that via efidebug you are not able to 
> > > > > > > apply any capsule
> > > > > > > but I don't think it is right that you can apply just update 
> > > > > > > capsules but not
> > > > > > > empty capsules. I would understand none or all but not something 
> > > > > > > in the middle.
> > > > > > 
> > > > > > The A/B update functionality requires using the capsule-on-disk
> > > > > > functionality for performing the updates. This is also mentioned in
> > > > > > the fwu_updates.rst document. You should be able to apply empty
> > > > > > capsules even with the 'efidebug disk-update' command.
> > > > > 
> > > > > Yes this is working fine.
> > > > > 
> > > > > ZynqMP> efidebug capsule disk-update
> > > > > #
> > > > > Applying capsule capsule1.bin succeeded.
> > > > > #
> > > > > Applying capsule capsule2.bin succeeded.
> > > > > Reboot after firmware update.
> > > > > 
> > > > > I tested it also with empty capsules which are also process properly.
> > > > > 
> > > > > > I have never
> > > > > > used the 'efidebug capsule update' command, so I'm not sure if that 
> > > > > > is
> > > > > > supported. Like Takahiro mentioned, if you place the 
> > > > > > capsules(genuine
> > > > > > or empty) under the /EFI/UpdateCapsule/ directory, the update should
> > > > > > happen automatically, since the fwu update feature also enables the
> > > > > > EFI_CAPSULE_ON_DISK_EARLY config.
> > > > > 
> > > > > Yes that's work fine on production systems.
> > > > > But from my point of view there shouldn't be really a problem to also 
> > > > > apply
> > > > > empty capsule via efidebug capsule update to be able to see that 
> > > > > steps and
> > > > > changes in mdata structure without performing reset.
> > > > 
> > > > The 'efidebug capsule update' command calls the efi_update_capsule
> > > > function, which implements the UpdateCapsule runtime service call. The
> > > > initial versions of my fwu patches were indeed adding support for this
> > > > path, but one of the review comments was to restrict support only for
> > > > the capsule-on-disk 

[PATCH 16/16] board: ten64: strip extra u-boot compatibles from FDT

2023-07-20 Thread Mathew McBride
The u-boot version of the LS1088A device tree has
an extra compatible (simple-mfd) added to _mc
to facilitate usage with U-Boot's device model.

Unfortunately FreeBSD will only match the single
"fsl,qoriq-mc" exactly when the node is a "bus"
object, so we need to strip out the extra compatible
before presenting it to the operating system.

Signed-off-by: Mathew McBride 
---
 board/traverse/ten64/ten64.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/board/traverse/ten64/ten64.c b/board/traverse/ten64/ten64.c
index 3cb8bad855..52daae6e24 100644
--- a/board/traverse/ten64/ten64.c
+++ b/board/traverse/ten64/ten64.c
@@ -174,6 +174,12 @@ void fdt_fixup_board_enet(void *fdt)
return;
}
 
+   /* In the U-Boot FDT, a 'simple-mfd' compatible is added.
+* Remove this as FreeBSD will only match "fsl,qoriq-mc"
+* exactly on the DPAA2 bus/MC node.
+*/
+   fdt_setprop(fdt, offset, "compatible", "fsl,qoriq-mc", 12);
+
if (get_mc_boot_status() == 0 &&
(is_lazy_dpl_addr_valid() || get_dpl_apply_status() == 0))
fdt_status_okay(fdt, offset);
-- 
2.30.1



[PATCH 15/16] board: ten64: opt out of fsl_setenv_bootcmd

2023-07-20 Thread Mathew McBride
Our bootcmd is the same regardless of where the SoC
loaded it's code from, so we don't want
fsl_setenv_bootcmd to do anything.

Signed-off-by: Mathew McBride 
---
 board/traverse/ten64/ten64.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/board/traverse/ten64/ten64.c b/board/traverse/ten64/ten64.c
index 17057966c8..3cb8bad855 100644
--- a/board/traverse/ten64/ten64.c
+++ b/board/traverse/ten64/ten64.c
@@ -465,3 +465,12 @@ static void ten64_board_retimer_ds110df410_init(void)
puts("OK\n");
 }
 
+/* Opt out of the fsl_setenv_bootcmd
+ * in arch/arm/cpu/armv8/fsl-layerscape/soc.c
+ * which is invoked by board_late_init.
+ */
+int fsl_setenv_bootcmd(void)
+{
+   return 0;
+}
+
-- 
2.30.1



[PATCH 14/16] arch: arm: fsl-layerscape: allow "opt-out" of fsl_setenv_bootcmd

2023-07-20 Thread Mathew McBride
Allow individual Layerscape boards to opt-out of fsl_setenv_bootcmd
by declaring the original function as weak.

fsl_setenv_bootcmd is used to change the bootcmd based on the
TF-A boot source (e.g QSPI vs SD/MMC) for reasons including
secure boot / integrity measurements and DPAA2 configuration loading.
See previous discussion at [1].

On the Ten64 board, our bootcmd is the same across
all TF-A boot sources so we don't want this behaviour.

Signed-off-by: Mathew McBride 

[1] 
https://patchwork.ozlabs.org/project/uboot/patch/2020044639.7070-3-m...@traverse.com.au/#2790037
---
 arch/arm/cpu/armv8/fsl-layerscape/soc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c 
b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
index 359cbc0430..577a0b6098 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
@@ -810,7 +810,7 @@ int qspi_ahb_init(void)
 #ifdef CONFIG_TFABOOT
 #define MAX_BOOTCMD_SIZE   512
 
-int fsl_setenv_bootcmd(void)
+__weak int fsl_setenv_bootcmd(void)
 {
int ret;
enum boot_src src = get_boot_src();
-- 
2.30.1



[PATCH 13/16] board: traverse: ten64: adopt standard boot defaults

2023-07-20 Thread Mathew McBride
With the previous updates to the device tree, Ten64
can use Standard Boot 'out of the box'.

Signed-off-by: Mathew McBride 
---
 configs/ten64_tfa_defconfig | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/configs/ten64_tfa_defconfig b/configs/ten64_tfa_defconfig
index 796a826b72..78e05cb4b6 100644
--- a/configs/ten64_tfa_defconfig
+++ b/configs/ten64_tfa_defconfig
@@ -22,7 +22,9 @@ CONFIG_OF_STDOUT_VIA_ALIAS=y
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyS0,115200 root=/dev/ram0 
earlycon=uart8250,mmio,0x21c0500 ramdisk_size=0x300 default_hugepagesz=2m 
hugepagesz=2m hugepages=256"
-# CONFIG_USE_BOOTCOMMAND is not set
+CONFIG_BOOTSTD_FULL=y
+CONFIG_BOOTSTD_DEFAULTS=y
+CONFIG_BOOTSTD_BOOTCOMMAND=y
 CONFIG_LOGLEVEL=7
 # CONFIG_DISPLAY_BOARDINFO is not set
 CONFIG_DISPLAY_BOARDINFO_LATE=y
-- 
2.30.1



[PATCH 12/16] board: ten64: disable watchdog autostart

2023-07-20 Thread Mathew McBride
The watchdog driver was previously enabled but not used
until U-Boot's fsl-ls1088a.dtsi was updated to describe them.

Some Linux distributions (e.g Debian 11) do not engage the
SP805 watchdogs, causing unexpected resets after boot.

To conserve the user experience, turn off the autostart,
and we will provide a mechanism to turn them on at boot
via env vars.

Signed-off-by: Mathew McBride 
---
 configs/ten64_tfa_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/ten64_tfa_defconfig b/configs/ten64_tfa_defconfig
index 7d63ee40e9..796a826b72 100644
--- a/configs/ten64_tfa_defconfig
+++ b/configs/ten64_tfa_defconfig
@@ -96,6 +96,7 @@ CONFIG_USB_XHCI_HCD=y
 CONFIG_USB_XHCI_DWC3=y
 CONFIG_USB_DWC3=y
 CONFIG_USB_GADGET=y
+# CONFIG_WATCHDOG_AUTOSTART is not set
 CONFIG_WDT=y
 CONFIG_WDT_SP805=y
 CONFIG_TPM=y
-- 
2.30.1



[PATCH 11/16] board: traverse: ten64: set serial# to be 'label' MAC

2023-07-20 Thread Mathew McBride
The GE0 (first Gigabit Ethernet interface) is used as the
'serial number' for the board and appliance.

To ensure the 'true' board S/N is available regardless of how
the DPAA2 subsystem is configured, use serial# so it is passed in
the device tree.

Signed-off-by: Mathew McBride 
---
 board/traverse/ten64/ten64.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/board/traverse/ten64/ten64.c b/board/traverse/ten64/ten64.c
index 0febc0baf0..17057966c8 100644
--- a/board/traverse/ten64/ten64.c
+++ b/board/traverse/ten64/ten64.c
@@ -296,6 +296,7 @@ static void ten64_set_macaddrs_from_board_info(struct 
t64uc_board_info *boardinf
 {
char ethaddr[18];
char enetvar[10];
+   char serial[18];
u8 intfidx, this_dpmac_num;
u64 macaddr = 0;
/* We will copy the MAC address returned from the
@@ -316,6 +317,19 @@ static void ten64_set_macaddrs_from_board_info(struct 
t64uc_board_info *boardinf
 */
macaddr = __be64_to_cpu(macaddr);
 
+   /* Set serial# to GE0/DPMAC7 MAC address
+* (Matches the labels on the board and appliance)
+*/
+   snprintf(serial, 18, "%02X%02X%02X%02X%02X%02X",
+MACADDRBITS(macaddr, 40),
+MACADDRBITS(macaddr, 32),
+MACADDRBITS(macaddr, 24),
+MACADDRBITS(macaddr, 16),
+MACADDRBITS(macaddr, 8),
+MACADDRBITS(macaddr, 0));
+   if (!env_get("serial#"))
+   env_set("serial#", serial);
+
for (intfidx = 0; intfidx < 10; intfidx++) {
snprintf(ethaddr, 18, "%02X:%02X:%02X:%02X:%02X:%02X",
 MACADDRBITS(macaddr, 40),
-- 
2.30.1



[PATCH 10/16] board: traverse: ten64: fix allocation order of MAC addresses

2023-07-20 Thread Mathew McBride
On Ten64 boards, the "serial number" is the MAC address of the
first Gigabit Ethernet interface (labelled GE0 on the appliance),
and counted up from there.

The previous logic did not take into account U-Boot's ordering
of the network interfaces. By setting aliases/ethernetX in the device
tree we can ensure the U-Boot 'ethX' is the same as the labelled
port order on the unit, as well as the one adopted by Linux.

Signed-off-by: Mathew McBride 
---
 arch/arm/dts/fsl-ls1088a-ten64-u-boot.dtsi | 10 ++
 board/traverse/ten64/ten64.c   |  4 ++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/arm/dts/fsl-ls1088a-ten64-u-boot.dtsi 
b/arch/arm/dts/fsl-ls1088a-ten64-u-boot.dtsi
index 89566bf849..4e6700d586 100644
--- a/arch/arm/dts/fsl-ls1088a-ten64-u-boot.dtsi
+++ b/arch/arm/dts/fsl-ls1088a-ten64-u-boot.dtsi
@@ -6,6 +6,16 @@
 /{
aliases {
spi0 = 
+   ethernet0 = 
+   ethernet1 = 
+   ethernet2 = 
+   ethernet3 = 
+   ethernet4 = 
+   ethernet5 = 
+   ethernet6 = 
+   ethernet7 = 
+   ethernet8 = 
+   ethernet9 = 
};
 };
 
diff --git a/board/traverse/ten64/ten64.c b/board/traverse/ten64/ten64.c
index 39f0d107cd..0febc0baf0 100644
--- a/board/traverse/ten64/ten64.c
+++ b/board/traverse/ten64/ten64.c
@@ -328,8 +328,8 @@ static void ten64_set_macaddrs_from_board_info(struct 
t64uc_board_info *boardinf
this_dpmac_num = allocation_order[intfidx];
printf("DPMAC%d: %s\n", this_dpmac_num, ethaddr);
snprintf(enetvar, 10,
-(this_dpmac_num != 1) ? "eth%daddr" : "ethaddr",
-this_dpmac_num - 1);
+(intfidx != 0) ? "eth%daddr" : "ethaddr",
+intfidx);
macaddr++;
 
if (!env_get(enetvar))
-- 
2.30.1



[PATCH 09/16] board: traverse: ten64: init nvme devices in late boot to ensure bootflow availability

2023-07-20 Thread Mathew McBride
Ensure nvme devices are scanned before reaching the shell,
otherwise extra user intervention ("nvme scan") is required
before they are visible to bootdev/bootflow.

Signed-off-by: Mathew McBride 
---
 board/traverse/ten64/ten64.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/board/traverse/ten64/ten64.c b/board/traverse/ten64/ten64.c
index df44baf24f..39f0d107cd 100644
--- a/board/traverse/ten64/ten64.c
+++ b/board/traverse/ten64/ten64.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -184,6 +185,11 @@ void fdt_fixup_board_enet(void *fdt)
 int fsl_board_late_init(void)
 {
ten64_board_retimer_ds110df410_init();
+
+   /* Ensure nvme storage devices are available to bootflow */
+   if (IS_ENABLED(CONFIG_NVME))
+   nvme_scan_namespace();
+
return 0;
 }
 
@@ -444,3 +450,4 @@ static void ten64_board_retimer_ds110df410_init(void)
 
puts("OK\n");
 }
+
-- 
2.30.1



[PATCH 08/16] configs: ten64: enable NVME_PCI

2023-07-20 Thread Mathew McBride
This restores NVMe functionality after PCI(e) NVMe support
was split out from the NVMe driver.

Signed-off-by: Mathew McBride 
---
 configs/ten64_tfa_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/ten64_tfa_defconfig b/configs/ten64_tfa_defconfig
index 9797a343e5..7d63ee40e9 100644
--- a/configs/ten64_tfa_defconfig
+++ b/configs/ten64_tfa_defconfig
@@ -76,6 +76,8 @@ CONFIG_SYS_MEMAC_LITTLE_ENDIAN=y
 CONFIG_E1000=y
 CONFIG_MII=y
 CONFIG_NVME=y
+CONFIG_NVME_PCI=y
+CONFIG_PCI=y
 CONFIG_DM_PCI_COMPAT=y
 CONFIG_PCIE_LAYERSCAPE_RC=y
 CONFIG_DM_RTC=y
-- 
2.30.1



[PATCH 07/16] board: ten64: add a bootmenu entries for NAND-based entries

2023-07-20 Thread Mathew McBride
The recovery-firmware and OpenWrt-NAND do not yet have bootflow
/bootstd entrypoints, so add bootmenu entries to make them
accessible.

Signed-off-by: Mathew McBride 
---
 include/configs/ten64.h | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/configs/ten64.h b/include/configs/ten64.h
index f9e800d4b6..d2bef9b6e5 100644
--- a/include/configs/ten64.h
+++ b/include/configs/ten64.h
@@ -46,6 +46,15 @@
"bootm $load_addr#ten64\0"
 #undef CFG_EXTRA_ENV_SETTINGS
 
+#if CONFIG_IS_ENABLED(CMD_BOOTMENU)
+#define DEFAULT_MENU_ENTRIES \
+   "bootmenu_0=Continue standard boot=run bootcmd\0" \
+   "bootmenu_1=Boot into recovery=run bootcmd_recovery\0" \
+   "bootmenu_2=Boot OpenWrt from NAND=run bootcmd_openwrt_nand\0"
+#else
+#define DEFAULT_MENU_ENTRIES ""
+#endif /* CONFIG_IS_ENABLED(CMD_BOOTMENU) */
+
 #define CFG_EXTRA_ENV_SETTINGS \
"BOARD=ten64\0" \
"fdt_addr_r=0x9000\0"   \
@@ -57,7 +66,8 @@
"openwrt_active_sys=a\0" \
"load_efi_dtb=mtd read devicetree $fdt_addr_r && fdt addr $fdt_addr_r 
&& " \
"fdt resize && fdt boardsetup\0" \
-   "bootcmd_recovery=mtd read recovery 0xa000 && " \
-   "setenv bootargs \"earlycon root=/dev/ram0 ramdisk_size=0x300\" && 
bootm 0xa000#ten64\0"
+   "bootcmd_recovery=mtd read recovery 0xa000;  " \
+   "setenv bootargs \"earlycon root=/dev/ram0 ramdisk_size=0x300\" && 
bootm 0xa000#ten64\0" \
+   DEFAULT_MENU_ENTRIES
 
 #endif /* __TEN64_H */
-- 
2.30.1



[PATCH 06/16] board: traverse: ten64: add NAND based OpenWrt bootcmd

2023-07-20 Thread Mathew McBride
The default Ten64 MTD configuration reserves two ubifs partitions
for OpenWrt residing on NAND flash. Add the bootcmd for this system
into the default environment.

Signed-off-by: Mathew McBride 
---
 include/configs/ten64.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/include/configs/ten64.h b/include/configs/ten64.h
index 1b8b27c230..f9e800d4b6 100644
--- a/include/configs/ten64.h
+++ b/include/configs/ten64.h
@@ -39,6 +39,11 @@
func(PXE, pxe, 0)
 #include 
 
+#define OPENWRT_NAND_BOOTCMD   \
+   "bootcmd_openwrt_nand=ubi part ubi${openwrt_active_sys} && "\
+   "ubi read $load_addr kernel && " \
+   "setenv bootargs \"root=/dev/ubiblock0_1 earlycon 
ubi.mtd=ubi${openwrt_active_sys}\" &&"\
+   "bootm $load_addr#ten64\0"
 #undef CFG_EXTRA_ENV_SETTINGS
 
 #define CFG_EXTRA_ENV_SETTINGS \
@@ -48,6 +53,8 @@
"kernel_addr_r=0x8100\0"\
"load_addr=0xa000\0"\
BOOTENV \
+   OPENWRT_NAND_BOOTCMD \
+   "openwrt_active_sys=a\0" \
"load_efi_dtb=mtd read devicetree $fdt_addr_r && fdt addr $fdt_addr_r 
&& " \
"fdt resize && fdt boardsetup\0" \
"bootcmd_recovery=mtd read recovery 0xa000 && " \
-- 
2.30.1



[PATCH 05/16] board: traverse: ten64: specify bootargs for recovery environment

2023-07-20 Thread Mathew McBride
The recovery environment[1] on the Ten64 is a OpenWrt-
based ramdisk stored on the NAND intended to help with
system setup tasks.

Before the bootargs were not being set for the recovery
command, relying instead on the existing bootargs variable.

Ensure the bootargs are set correctly prior to booting recovery.

Signed-off-by: Mathew McBride 

[1] https://ten64doc.traverse.com.au/software/recovery/
---
 include/configs/ten64.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/configs/ten64.h b/include/configs/ten64.h
index 63fbafd132..1b8b27c230 100644
--- a/include/configs/ten64.h
+++ b/include/configs/ten64.h
@@ -50,6 +50,7 @@
BOOTENV \
"load_efi_dtb=mtd read devicetree $fdt_addr_r && fdt addr $fdt_addr_r 
&& " \
"fdt resize && fdt boardsetup\0" \
-   "bootcmd_recovery=mtd read recovery 0xa000; && bootm 
0xa000#ten64\0"
+   "bootcmd_recovery=mtd read recovery 0xa000 && " \
+   "setenv bootargs \"earlycon root=/dev/ram0 ramdisk_size=0x300\" && 
bootm 0xa000#ten64\0"
 
 #endif /* __TEN64_H */
-- 
2.30.1



[PATCH 04/16] board: traverse: ten64: update DPAA2 (network) binary path on sdcards

2023-07-20 Thread Mathew McBride
Change the firmware on microSD path to "firmware/traverse/ten64"
as per EBBR section 4.2[1].

The Traverse firmware tools now locate the DPAA2 firmware
and configuration files under that path on the rescue
SD card image.
If a user then installs a standard Linux
distribution over the top of that sdcard, (in theory)
it will be left alone by distribution boot tooling.

Signed-off-by: Mathew McBride 

[1] https://arm-software.github.io/ebbr/index.html#firmware-partition-filesystem
---
 include/configs/ten64.h | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/configs/ten64.h b/include/configs/ten64.h
index 1601fb733e..63fbafd132 100644
--- a/include/configs/ten64.h
+++ b/include/configs/ten64.h
@@ -15,6 +15,8 @@
 #define QSPI_NOR_BOOTCOMMAND   "run distro_bootcmd"
 #define SD_BOOTCOMMAND "run distro_bootcmd"
 
+#define SD_FIRMWARE_PATH "firmware/traverse/ten64/"
+
 #define QSPI_MC_INIT_CMD   \
"sf probe 0:0 && sf read 0x8000 0x30 0x20 &&"   \
"sf read 0x8020 0x5C 0x4 &&"
\
@@ -22,10 +24,10 @@
"sf read 0x8E00 0x58 0x4 && fsl_mc lazyapply DPL 0x8E00 
&& "\
"echo 'default DPL loaded'\0"
 #define SD_MC_INIT_CMD \
-   "mmcinfo; fatload mmc 0 0x8000 mcfirmware/mc_ls1088a.itb; "\
-   "fatload mmc 0 0x8020 dpaa2config/dpc.0x1D-0x0D.dtb; "\
+   "mmcinfo; fatload mmc 0 0x8000 " SD_FIRMWARE_PATH "mc_ls1088a.itb; 
"\
+   "fatload mmc 0 0x8020 " SD_FIRMWARE_PATH "dpc.0x1D-0x0D.dtb; "\
"fsl_mc start mc 0x8000 0x8020 && " \
-   "fatload mmc 0 0x8E00 dpaa2config/eth-dpl-all.dtb && " \
+   "fatload mmc 0 0x8E00 " SD_FIRMWARE_PATH "eth-dpl-all.dtb && " \
"fsl_mc lazyapply DPL 0x8E00 && echo 'default DPL loaded'\0"
 
 #define BOOT_TARGET_DEVICES(func) \
-- 
2.30.1



[PATCH 03/16] board: traverse: ten64: fix DPAA2 (network) DPL corruption issue

2023-07-20 Thread Mathew McBride
The DPAA2 DPL (data plane layout) file was previously
being loaded into 0x8030, and set to be applied
just before hand off to the kernel.

When a FIT image with a load_address of 0x8000 was
booted with bootm, the DPL in memory was overwritten.

Move the DPL load to 0x8E00 (196MiB away from 0x8000,
and below the other typical load addr of 0x9000).

Ideally in the future, the DPL lazyapply command
("fsl_mc lazyapply DPL $dpl_addr") should be set to
load the DPL contents into a memory area owned by U-Boot.

Signed-off-by: Mathew McBride 
---
 include/configs/ten64.h | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/configs/ten64.h b/include/configs/ten64.h
index e86c163132..1601fb733e 100644
--- a/include/configs/ten64.h
+++ b/include/configs/ten64.h
@@ -19,11 +19,14 @@
"sf probe 0:0 && sf read 0x8000 0x30 0x20 &&"   \
"sf read 0x8020 0x5C 0x4 &&"
\
"fsl_mc start mc 0x8000 0x8020 && " \
-   "sf read 0x8030 0x58 0x4 && fsl_mc lazyapply DPL 
0x8030\0"
+   "sf read 0x8E00 0x58 0x4 && fsl_mc lazyapply DPL 0x8E00 
&& "\
+   "echo 'default DPL loaded'\0"
 #define SD_MC_INIT_CMD \
"mmcinfo; fatload mmc 0 0x8000 mcfirmware/mc_ls1088a.itb; "\
"fatload mmc 0 0x8020 dpaa2config/dpc.0x1D-0x0D.dtb; "\
-   "fsl_mc start mc 0x8000 0x8020\0"
+   "fsl_mc start mc 0x8000 0x8020 && " \
+   "fatload mmc 0 0x8E00 dpaa2config/eth-dpl-all.dtb && " \
+   "fsl_mc lazyapply DPL 0x8E00 && echo 'default DPL loaded'\0"
 
 #define BOOT_TARGET_DEVICES(func) \
func(NVME, nvme, 0) \
@@ -45,7 +48,6 @@
BOOTENV \
"load_efi_dtb=mtd read devicetree $fdt_addr_r && fdt addr $fdt_addr_r 
&& " \
"fdt resize && fdt boardsetup\0" \
-   "bootcmd_recovery=mtd read recovery 0xa000; mtd read dpl 0x8010 
&& " \
-   "fsl_mc apply DPL 0x8010 && bootm 0xa000#ten64\0"
+   "bootcmd_recovery=mtd read recovery 0xa000; && bootm 
0xa000#ten64\0"
 
 #endif /* __TEN64_H */
-- 
2.30.1



[PATCH 02/16] board: traverse: ten64: ensure retimer reset is done on new board revisions

2023-07-20 Thread Mathew McBride
Board revision C (production) and later require the SFP+
retimer to be turned on (or reset) on boot, by way of issuing
a command to the board's microcontroller (via I2C).

The comparison statement here was incorrect, as the board
ID decrements every revision (from 0xFF downwards),
so this was matching board RevA,B,C instead of Rev >= C.

Another oops that transpired when working on this issue,
is that if the board controller is not called (such as
CONFIG_TEN64_CONTROLLER=n or earlier board rev), then
the retimer udevice was not obtained. So the board
version check has to be moved inside board_cycle_retimer
(which probes/fetches the retimer device) as well.

Signed-off-by: Mathew McBride 
---
 board/traverse/ten64/ten64.c | 37 ++--
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/board/traverse/ten64/ten64.c b/board/traverse/ten64/ten64.c
index 88f22e85d7..df44baf24f 100644
--- a/board/traverse/ten64/ten64.c
+++ b/board/traverse/ten64/ten64.c
@@ -341,20 +341,27 @@ static int board_cycle_retimer(struct udevice **retim_dev)
u8 loop;
struct udevice *uc_dev;
struct udevice *i2cbus;
+   u32 board_rev = ten64_get_board_rev();
 
ret = ten64_get_micro_udevice(_dev, );
if (ret)
return ret;
 
-   ret = dm_i2c_probe(i2cbus, I2C_RETIMER_ADDR, 0, retim_dev);
-   if (ret == 0) {
-   puts("(retimer on, resetting...) ");
+   /* Retimer power cycle not implemented on early board
+* revisions/controller firmwares
+*/
+   if (IS_ENABLED(CONFIG_TEN64_CONTROLLER) &&
+   board_rev <= TEN64_BOARD_REV_C) {
+   ret = dm_i2c_probe(i2cbus, I2C_RETIMER_ADDR, 0, retim_dev);
+   if (ret == 0) {
+   puts("(retimer on, resetting...) ");
 
-   ret = misc_call(uc_dev, TEN64_CNTRL_10G_OFF, NULL, 0, NULL, 0);
-   mdelay(1000);
-   }
+   ret = misc_call(uc_dev, TEN64_CNTRL_10G_OFF, NULL, 0, 
NULL, 0);
+   mdelay(1000);
+   }
 
-   ret = misc_call(uc_dev, TEN64_CNTRL_10G_ON, NULL, 0, NULL, 0);
+   ret = misc_call(uc_dev, TEN64_CNTRL_10G_ON, NULL, 0, NULL, 0);
+   }
 
// Wait for retimer to come back
for (loop = 0; loop < 5; loop++) {
@@ -375,19 +382,13 @@ static void ten64_board_retimer_ds110df410_init(void)
u8 reg;
int ret;
struct udevice *retim_dev;
-   u32 board_rev = ten64_get_board_rev();
 
puts("Retimer: ");
-   /* Retimer power cycle not implemented on early board
-* revisions/controller firmwares
-*/
-   if (IS_ENABLED(CONFIG_TEN64_CONTROLLER) &&
-   board_rev >= TEN64_BOARD_REV_C) {
-   ret = board_cycle_retimer(_dev);
-   if (ret) {
-   puts("Retimer power on failed\n");
-   return;
-   }
+
+   ret = board_cycle_retimer(_dev);
+   if (ret) {
+   puts("Retimer power on failed\n");
+   return;
}
 
/* Access to Control/Shared register */
-- 
2.30.1



[PATCH 00/16] Ten64 updates 2023-07

2023-07-20 Thread Mathew McBride
This is a series of updates for the Ten64 board,
that are part of our firmware releases but not yet upstreamed
into U-Boot.

Changes of note include:

- Turning on standard boot support

  Standard boot improves the user experience
  over distroboot on Ten64, as we had
  various hacks in our firmware to solve
  some corner-case issues (e.g DTB handling)
  in distroboot, which are not needed with
  the bootflow system.

- Recognition of the new 'RevD' board variant
  distributed to OEM customers

- Fixing various boot issues related to FIT images and
  operating systems running out of the NAND (OpenWrt,
  recovery environment).

- A better 'opt-out' solution for fsl_setenv_bootcmd
  for Layerscape platforms booting from TF-A.

  This was discussed when the Ten64 was upstreamed into
  U-Boot. I think declaring fsl_setenv_bootcmd
  as __weak and allowing individual boards to override
  is the best way to do this without significant rework.
  (We actually depend on a similar feature for the DPAA2/MC
  firmware loading)

Compared to our firmware branch, there is still a few
features missing (e.g USB Hub, fan controller and fixes
for the VSC8514). Some of these depend on other things
(like sorting out device tree schemas) so may not appear
in mainline U-Boot for a while yet.

Mathew McBride (16):
  board: traverse: ten64: recognize board revision D
  board: traverse: ten64: ensure retimer reset is done on new board
revisions
  board: traverse: ten64: fix DPAA2 (network) DPL corruption issue
  board: traverse: ten64: update DPAA2 (network) binary path on sdcards
  board: traverse: ten64: specify bootargs for recovery environment
  board: traverse: ten64: add NAND based OpenWrt bootcmd
  board: ten64: add a bootmenu entries for NAND-based entries
  configs: ten64: enable NVME_PCI
  board: traverse: ten64: init nvme devices in late boot to ensure
bootflow availability
  board: traverse: ten64: fix allocation order of MAC addresses
  board: traverse: ten64: set serial# to be 'label' MAC
  board: ten64: disable watchdog autostart
  board: traverse: ten64: adopt standard boot defaults
  arch: arm: fsl-layerscape: allow "opt-out" of fsl_setenv_bootcmd
  board: ten64: opt out of fsl_setenv_bootcmd
  board: ten64: strip extra u-boot compatibles from FDT

 arch/arm/cpu/armv8/fsl-layerscape/soc.c|  2 +-
 arch/arm/dts/fsl-ls1088a-ten64-u-boot.dtsi | 10 +++
 board/traverse/ten64/ten64.c   | 95 --
 configs/ten64_tfa_defconfig|  7 +-
 include/configs/ten64.h| 34 ++--
 5 files changed, 114 insertions(+), 34 deletions(-)

-- 
2.30.1



[PATCH 01/16] board: traverse: ten64: recognize board revision D

2023-07-20 Thread Mathew McBride
Ten64 board revision D is a variant that removes the USB hub
and PCIe expander/switch, but is otherwise compatible with the
main production "C" version.

At the same time, revise the printf specifiers (PCB version
"1064-0201%s") to reduce the number of string characters related
to the boot printout.

Signed-off-by: Mathew McBride 
---
 board/traverse/ten64/ten64.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/board/traverse/ten64/ten64.c b/board/traverse/ten64/ten64.c
index 5dfb7165c0..88f22e85d7 100644
--- a/board/traverse/ten64/ten64.c
+++ b/board/traverse/ten64/ten64.c
@@ -47,7 +47,9 @@ static void ten64_board_retimer_ds110df410_init(void);
 enum {
TEN64_BOARD_REV_A = 0xFF,
TEN64_BOARD_REV_B = 0xFE,
-   TEN64_BOARD_REV_C = 0xFD
+   TEN64_BOARD_REV_C = 0xFD,
+   TEN64_BOARD_REV_D = 0xFC,
+   TEN64_BOARD_MAX
 };
 
 #define RESV_MEM_IN_BANK(b)(gd->arch.resv_ram >= base[b] && \
@@ -75,20 +77,24 @@ int checkboard(void)
 
switch (board_rev) {
case TEN64_BOARD_REV_A:
-   snprintf(boardmodel, 32, "1064-0201A (Alpha)");
+   snprintf(boardmodel, 32, "A (Alpha)");
break;
case TEN64_BOARD_REV_B:
-   snprintf(boardmodel, 32, "1064-0201B (Beta)");
+   snprintf(boardmodel, 32, "B (Beta)");
break;
case TEN64_BOARD_REV_C:
-   snprintf(boardmodel, 32, "1064-0201C");
+   snprintf(boardmodel, 32, "C");
+   break;
+   case TEN64_BOARD_REV_D:
+   snprintf(boardmodel, 32, "D");
break;
default:
-   snprintf(boardmodel, 32, "1064 Revision %X", (0xFF - 
board_rev));
+   snprintf(boardmodel, 32, " Revision %X", (0xFF - board_rev));
break;
}
 
-   printf("Board: %s, boot from ", boardmodel);
+   printf("Board: 1064-0201%s, boot from ", boardmodel);
+
if (src == BOOT_SOURCE_SD_MMC)
puts("SD card\n");
else if (src == BOOT_SOURCE_QSPI_NOR)
-- 
2.30.1



RE: [PATCH v1] HSD #18028953892: usb: xhci-dwc3: Fix USB3.1 controller register access in reset state

2023-07-20 Thread Lim, Jit Loon
Hi

> -Original Message-
> From: Marek Vasut 
> Sent: Friday, 7 July, 2023 5:50 AM
> To: Lim, Jit Loon ; u-boot@lists.denx.de
> Cc: Jagan Teki ; Simon
> ; Chee, Tien Fong
> ; Hea, Kok Kiang ;
> Lokanathan, Raaj ; Maniyam, Dinesh
> ; Ng, Boon Khai ;
> Yuslaimi, Alif Zakuan ; Chong, Teik Heng
> ; Zamri, Muhammad Hazim Izzat
> ; Tang, Sieu Mun
> ; Bin Meng ; Michal
> Simek ; Tom Rini ; Eugen Hristev
> 
> Subject: Re: [PATCH v1] HSD #18028953892: usb: xhci-dwc3: Fix USB3.1
> controller register access in reset state
> 
> On 6/22/23 16:08, Lim, Jit Loon wrote:
> >> -Original Message-
> >> From: Marek Vasut 
> >> Sent: Thursday, 22 June, 2023 5:35 PM
> >> To: Lim, Jit Loon ; u-boot@lists.denx.de
> >> Cc: Jagan Teki ; Simon
> >> ; Chee, Tien Fong
> >> ; Hea, Kok Kiang ;
> >> Lokanathan, Raaj ; Maniyam, Dinesh
> >> ; Ng, Boon Khai
> ;
> >> Yuslaimi, Alif Zakuan ; Chong, Teik
> >> Heng ; Zamri, Muhammad Hazim Izzat
> >> ; Tang, Sieu Mun
> >> ; Bin Meng ; Michal
> >> Simek ; Tom Rini ; Eugen
> >> Hristev 
> >> Subject: Re: [PATCH v1] HSD #18028953892: usb: xhci-dwc3: Fix USB3.1
> >> controller register access in reset state
> >>
> >> On 6/22/23 04:48, Lim, Jit Loon wrote:
>  -Original Message-
>  From: Marek Vasut 
>  Sent: Wednesday, 21 June, 2023 10:19 PM
>  To: Lim, Jit Loon ; u-boot@lists.denx.de
>  Cc: Jagan Teki ; Simon
>  ; Chee, Tien Fong
>  ; Hea, Kok Kiang
>  ; Lokanathan, Raaj
>  ; Maniyam, Dinesh
>  ; Ng, Boon Khai
> ;
>  Yuslaimi, Alif Zakuan ; Chong, Teik
>  Heng ; Zamri, Muhammad Hazim Izzat
>  ; Tang, Sieu Mun
>  ; Bin Meng 
>  Subject: Re: [PATCH v1] HSD #18028953892: usb: xhci-dwc3: Fix
>  USB3.1 controller register access in reset state
> 
>  On 6/21/23 16:11, Jit Loon Lim wrote:
> > From: Teik Heng Chong 
> >
> > The controller registers should not be accessed while the
> > controller's vcc_reset_n is asserted.
> >
> > Signed-off-by: Teik Heng Chong 
> 
>  Is this patch ported from Linux or is this custom development ?
> 
>  Is there a matching patch/fix in Linux already ?
> >>>
> >>> In xhci_dwc3_probe(), the program sequence is vcc reset -> clk init
> >>> -> phy setup -> xhci_register therefore, when xhci_dwc3_remove is
> >>> called, the proper usb stop sequence should be xhci_register _> phy
> >>> setup -> clk init -> vcc reset
> >>>
> >>> if we look at linux driver
> >>> https://elixir.bootlin.com/linux/latest/source/drivers/usb/dwc3/dwc3
> >>> -o
> >>> f-simple.c#L33
> >>>
> >>> dwc3_of_simple_probe: The sequence is reset -> clock
> >>> __dwc3_of_simple_teardown: Then, clock -> reset
> >>>
> >>> So based on the above, we have made changes and the uboot fix is now
> >> aligned with linux driver.
> >>
> >> Instead of adding random patches to the U-Boot dwc3 driver, please
> >> just synchronize the driver with Linux. You should be able to add the
> >> missing patches to the DWC3 driver from Linux since the last
> >> synchronization point, the process should be largely mechanical. Make
> >> sure to include commit ID of each Linux commit in the new matching U-
> Boot patch.
> >>
> >> It shouldn't be difficult, one approach I can think of is roughly this:
> >> - figure out the original merge base from which the DWC3 driver was
> >> imported to U-Boot
> >> - in U-Boot, revert all dwc3 patches on top of that import patch
> >> - pick all Linux kernel dwc3 patches from that merge base and apply
> >> on top of this U-Boot with reverts
> >> - Run rebase and drop the reverts, let git drop duplicate patches
> >
> > We believed the previous reply from us is a bit confusing.
> > There is no exact same function/file for U-Boot to use in Linux.
> > What we are doing is, we are referring to Linux sequence.
> >
> > Linux:  (dwc3_of_simple_probe)
> > https://elixir.bootlin.com/linux/latest/source/drivers/usb/dwc3/dwc3-o
> > f-simple.c#L33
> > U-Boot: (xhci_dwc3_probe)
> > https://elixir.bootlin.com/u-boot/latest/source/drivers/usb/host/xhci-
> > dwc3.c#L159
> >
> > Linux:  (__dwc3_of_simple_teardown)
> > https://elixir.bootlin.com/linux/latest/source/drivers/usb/dwc3/dwc3-o
> > f-simple.c#L98
> > U-Boot: (xhci_dwc3_remove)
> > https://elixir.bootlin.com/u-boot/latest/source/drivers/usb/host/xhci-
> > dwc3.c#L227
> >
> > So we believed that we can't directly pickup all Linux kernel dwc3 patches
> and merge to U-Boot.
> 
> If you were to sync the driver from Linux to U-Boot, then the same sequence
> as Linux uses would be automatically used too, right ?
> 
> Sorry for the abysmal delay in my reply.

Are we saying that we shall port/use Linux driver in U-Boot and abandon the 
existing USB host driver in U-Boot?




Re: [PATCH v7 1/4] i2c: designware: Add CONFIG_ACPIGEN limitation to designware_i2c_pci.c

2023-07-20 Thread Minda Chen



On 2023/7/21 3:42, Simon Glass wrote:
> Hi,
> 
> On Thu, 20 Jul 2023 at 05:24, Minda Chen  wrote:
>>
>> As the designware_i2c_pci.c uses ACPI APIs, If some SoCs (StarFive
>> JH7110) contain designware i2c and PCI but do not use ACPI,
>> This file will be can't be compiled. So add ACPIGEN to
>> designware_i2c_pci.c
>>
>> Signed-off-by: Minda Chen 
>> ---
>>  drivers/i2c/Makefile | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
>> index 99545df2e5..92b1ec6bc3 100644
>> --- a/drivers/i2c/Makefile
>> +++ b/drivers/i2c/Makefile
>> @@ -18,9 +18,11 @@ obj-$(CONFIG_SYS_I2C_CADENCE) += i2c-cdns.o
>>  obj-$(CONFIG_SYS_I2C_CA) += i2c-cortina.o
>>  obj-$(CONFIG_SYS_I2C_DAVINCI) += davinci_i2c.o
>>  obj-$(CONFIG_SYS_I2C_DW) += designware_i2c.o
>> +ifdef CONFIG_ACPIGEN
> 
> You should use Kconfig to see SYS_I2C_DW depends on ACPIGEN. You might
> need a separate I2C_DW_PCI Kconfig.
> 
I think so.

I can add Kconfig like this.
config SYS_I2C_DW_PCI
bool "Designware PCI I2C Controller"
depends on SYS_I2C_DW && PCI && APIGEN
default y

> But that sounds bad to me. Why does it have to generate ACPI tables?
> It should work fine without that.
> 
designware_i2c_pci.c contain ACPI and lpss related codes.
>>  ifdef CONFIG_PCI
>>  obj-$(CONFIG_SYS_I2C_DW) += designware_i2c_pci.o
>>  endif
>> +endif
>>  obj-$(CONFIG_SYS_I2C_FSL) += fsl_i2c.o
>>  obj-$(CONFIG_SYS_I2C_IHS) += ihs_i2c.o
>>  obj-$(CONFIG_SYS_I2C_INTEL) += intel_i2c.o
>> --
>> 2.17.1
>>
> 
> Regards,
> Simon


Re: [PATCH] Azure: Rework our Rockchip jobs slightly

2023-07-20 Thread Tom Rini
On Wed, Jul 19, 2023 at 03:09:12PM -0400, Tom Rini wrote:

> Currently the 64bit "rk" job is close to and sometimes goes over the job
> time limit.  Let us rework this in to one job for "rk" and "rv" (which
> are the SoC prefixes) jobs which include or exclude "rockchip" the board
> vendor.  This gives us two jobs of similar numbers of platforms to build
> now instead.
> 
> Signed-off-by: Tom Rini 
> Reviewed-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 5/5] CI: Update to the latest "Jammy" tag

2023-07-20 Thread Tom Rini
On Thu, Jul 13, 2023 at 08:37:36PM -0400, Tom Rini wrote:

> Move to the latest "Jammy" tag from Ubuntu.
> 
> Signed-off-by: Tom Rini 
> Reviewed-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 4/5] CI: Update to gcc-13.1.0

2023-07-20 Thread Tom Rini
On Thu, Jul 13, 2023 at 08:37:35PM -0400, Tom Rini wrote:

> As this is the current version of the public cross toolchains we use,
> upgrade to this now.
> 
> Suggested-by: Alexey Brodkin 
> Signed-off-by: Tom Rini 
> Acked-by: Alexey Brodkin 
> Reviewed-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 3/5] spl: Correct spl_board_boot_device function prototype

2023-07-20 Thread Tom Rini
On Thu, Jul 13, 2023 at 08:37:34PM -0400, Tom Rini wrote:

> With gcc-13.1 we get a warning about enum vs int here, so correct the
> declaration to match the implementation.
> 
> Signed-off-by: Tom Rini 
> Reviewed-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 2/5] arm: mx5: Correct mxc_set_clock function prototype

2023-07-20 Thread Tom Rini
On Thu, Jul 13, 2023 at 08:37:33PM -0400, Tom Rini wrote:

> With gcc-13.1 we get a warning about enum vs int here, so correct the
> declaration to match the implementation.
> 
> Signed-off-by: Tom Rini 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 1/5] mips: octeon: Correct types in cvmx-pko3-queue

2023-07-20 Thread Tom Rini
On Thu, Jul 13, 2023 at 08:37:32PM -0400, Tom Rini wrote:

> When building with gcc-13.1 we see that the prototype for
> cvmx_pko3_sq_config_children does not match the declaration. Make these
> match and correct a typo in the function's version of the docs that the
> prototype did not have, as part of keeping those in-sync.
> 
> Signed-off-by: Tom Rini 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCHv2 1/2] CI: Update to QEMU 8.0.3

2023-07-20 Thread Tom Rini
On Thu, Jul 13, 2023 at 10:16:28AM -0400, Tom Rini wrote:

> Move up to the latest tagged release of QEMU
> 
> Signed-off-by: Tom Rini 
> Reviewed-by: Bin Meng 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] Azure: Add excludes to the imx8_imx9 job

2023-07-20 Thread Tom Rini
On Wed, Jul 12, 2023 at 04:22:22PM -0400, Tom Rini wrote:

> The job to build all imx8 and imx9 platforms is currently close to, or
> sometimes exceeding the allowed build time. Exclude some platforms that
> are already being built under their vendor-specific job as well to
> reduce the time.
> 
> Signed-off-by: Tom Rini 
> Reviewed-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] CI: Add automatic retry for test.py jobs

2023-07-20 Thread Tom Rini
On Tue, Jul 11, 2023 at 10:33:03PM -0400, Tom Rini wrote:

> It is not uncommon for some of the QEMU-based jobs to fail not because
> of a code issue but rather because of a timing issue or similar problem
> that is out of our control. Make use of the keywords that Azure and
> GitLab provide so that we will automatically re-run these when they fail
> 2 times. If they fail that often it is likely we have found a real issue
> to investigate.
> 
> Signed-off-by: Tom Rini 
> Reviewed-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] tools/docker: Dockerfile: Don't specify dtc submodule

2023-07-20 Thread Tom Rini
On Fri, Jul 07, 2023 at 06:04:30PM -0400, Tom Rini wrote:

> When building qemu, all required submodules (of which we need more than
> just dtc) are handled automatically. Currently trying to init the
> submodule the way we do results in a git failure.
> 
> Reported-by: Alexey Brodkin 
> Signed-off-by: Tom Rini 
> Reviewed-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] buildman: Switch ARC toolchain to the upstream version

2023-07-20 Thread Tom Rini
On Fri, Jul 07, 2023 at 10:04:53PM +0100, Alexey Brodkin wrote:

> Back in the day we relied a lot on Synopsys own build of the GNU tools
> for ARC processors, but since then we worked hard on getting all our changes
> upstream and for a couple of years now we have ARCompact (AKA ARCv1)
> and ARCv2 processors supported very well in upstream GCC, Binutils, GDB etc.
> 
> And so there's no need to use Synopsys forks any longer, thus we remove
> all the references to that form and use upstream components as majority
> of other architectures in U-Boot.
> 
> Thanks to Tom for pointing to that left-over!
> 
> Signed-off-by: Alexey Brodkin 
> Cc: Simon Glass 
> Cc: Tom Rini 
> Reviewed-by: Tom Rini 
> Reviewed-by: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] usb: cdns3: gadget: Configure speed in udc_start

2023-07-20 Thread Marek Vasut

On 7/19/23 13:06, Ravi Gunasekaran wrote:



On 7/19/23 4:03 PM, Marek Vasut wrote:

On 7/19/23 10:59, Ravi Gunasekaran wrote:

When one of the functions does not support super speed, the composite
driver forces the gadget to high speed. But the speed is never
configured in the cdns3 gadget driver. So configure the speed
in cdns3_gadget_udc_start just like in the kernel.

Signed-off-by: Ravi Gunasekaran 


Is this a patch picked from the kernel ?
Is there a matching kernel commit ID ?


The commit 4df50f89f5 ("usb: composite: force gadget to be USB2 for HS only 
function")
in u-boot, forces the gadget's max speed to high speed and had a mention in the 
commit
description that the gadget's udc_start would configure itself in USB 2.0.

I checked the cdns3 gadget driver in kernel, and the cdns3_gadget_udc_start()
configures speed. So this is not a patch picked directly from the kernel. I used
kernel code for reference.


Thanks for clarification.

Reviewed-by: Marek Vasut 


[PATCH 1/1] efi_loader: device paths for special block devices

2023-07-20 Thread Heinrich Schuchardt
The UEFI specification does not provide node types matching UCLASS_BLKMAP,
UCLASS_HOST, UCLASS_VIRTIO block devices.

The current implementation uses VenHw() nodes with uclass specific GUIDs
and a single byte for the device number appended. This leads to unaligned
integers is succeeding device path nodes.

The current implementation fails to create unique device paths for block
devices based on other uclasses like UCLASS_PVBLOCK.

Let's use a VenHw() node with the U-Boot GUID with a length dividable by
four and encoding blkdesc->uclass_id as well as  blkdesc->devnum.

Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_loader/efi_device_path.c | 114 ++-
 1 file changed, 21 insertions(+), 93 deletions(-)

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index 19e8861ef4..5b050fa17c 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -22,16 +22,6 @@
 #include 
 #include  /* U16_MAX */
 
-#ifdef CONFIG_BLKMAP
-const efi_guid_t efi_guid_blkmap_dev = U_BOOT_BLKMAP_DEV_GUID;
-#endif
-#ifdef CONFIG_SANDBOX
-const efi_guid_t efi_guid_host_dev = U_BOOT_HOST_DEV_GUID;
-#endif
-#ifdef CONFIG_VIRTIO_BLK
-const efi_guid_t efi_guid_virtio_dev = U_BOOT_VIRTIO_DEV_GUID;
-#endif
-
 /* template END node: */
 const struct efi_device_path END = {
.type = DEVICE_PATH_TYPE_END,
@@ -524,43 +514,15 @@ __maybe_unused static unsigned int dp_size(struct udevice 
*dev)
return dp_size(dev->parent) +
sizeof(struct efi_device_path_nvme);
 #endif
-#ifdef CONFIG_SANDBOX
-   case UCLASS_HOST:
-/*
- * Sandbox's host device will be represented
- * as vendor device with extra one byte for
- * device number
- */
-   return dp_size(dev->parent)
-   + sizeof(struct efi_device_path_vendor) + 1;
-#endif
 #ifdef CONFIG_USB
case UCLASS_MASS_STORAGE:
return dp_size(dev->parent)
+ sizeof(struct efi_device_path_controller);
-#endif
-#ifdef CONFIG_VIRTIO_BLK
-   case UCLASS_VIRTIO:
-/*
- * Virtio devices will be represented as a vendor
- * device node with an extra byte for the device
- * number.
- */
-   return dp_size(dev->parent)
-   + sizeof(struct efi_device_path_vendor) + 1;
-#endif
-#ifdef CONFIG_BLKMAP
-   case UCLASS_BLKMAP:
-/*
- * blkmap devices will be represented as a vendor
- * device node with an extra byte for the device
- * number.
- */
-   return dp_size(dev->parent)
-   + sizeof(struct efi_device_path_vendor) + 1;
 #endif
default:
-   return dp_size(dev->parent);
+   /* UCLASS_BLKMAP, UCLASS_HOST, UCLASS_VIRTIO */
+   return dp_size(dev->parent) +
+   sizeof(struct efi_device_path_udevice);
}
 #if defined(CONFIG_MMC)
case UCLASS_MMC:
@@ -608,53 +570,7 @@ __maybe_unused static void *dp_fill(void *buf, struct 
udevice *dev)
}
 #endif
case UCLASS_BLK:
-   switch (dev->parent->uclass->uc_drv->id) {
-#ifdef CONFIG_BLKMAP
-   case UCLASS_BLKMAP: {
-   struct efi_device_path_vendor *dp;
-   struct blk_desc *desc = dev_get_uclass_plat(dev);
-
-   dp = dp_fill(buf, dev->parent);
-   dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
-   dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
-   dp->dp.length = sizeof(*dp) + 1;
-   memcpy(>guid, _guid_blkmap_dev,
-  sizeof(efi_guid_t));
-   dp->vendor_data[0] = desc->devnum;
-   return >vendor_data[1];
-   }
-#endif
-#ifdef CONFIG_SANDBOX
-   case UCLASS_HOST: {
-   /* stop traversing parents at this point: */
-   struct efi_device_path_vendor *dp;
-   struct blk_desc *desc = dev_get_uclass_plat(dev);
-
-   dp = dp_fill(buf, dev->parent);
-   dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
-   dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
-   dp->dp.length = sizeof(*dp) + 1;
-   memcpy(>guid, _guid_host_dev,
-  sizeof(efi_guid_t));
-   

[RFC PATCH 2/2] arm: dts: k3-am625-sk-u-boot.dtsi: drop mac_efuse

2023-07-20 Thread Roger Quadros
This was a custom property and we don't need it anymore.
We are now using the standard property "ti,syscon-efuse".

Signed-off-by: Roger Quadros 
---

RFC because Nishanth can squash this into his DT sync series.

The /delete-property/ ranges also needs to be deleted along with the
duplicate cpsw-phy-sel@04044. But first will need to teach
the driver to deal with the syscon node properly. phys = <_gmii_sel n>;

Will send a follow up patch for that soon.

 arch/arm/dts/k3-am625-sk-u-boot.dtsi | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi 
b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
index 249155733a..54fdabdb8e 100644
--- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
+++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
@@ -128,9 +128,6 @@
 };
 
  {
-   reg = <0x0 0x800 0x0 0x20>,
- <0x0 0x43000200 0x0 0x8>;
-   reg-names = "cpsw_nuss", "mac_efuse";
/delete-property/ ranges;
bootph-pre-ram;
 
-- 
2.34.1



[PATCH 1/2] net: ti: am65-cpsw-nuss: Use approved property to get efuse address

2023-07-20 Thread Roger Quadros
The approved DT property for MAC efuse (ROM) address is
"ti,syscon-efuse".

Use that and drop custom property "mac_efuse".

Signed-off-by: Roger Quadros 
---
 configs/am62x_evm_a53_defconfig |  1 +
 drivers/net/ti/am65-cpsw-nuss.c | 52 +++--
 2 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/configs/am62x_evm_a53_defconfig b/configs/am62x_evm_a53_defconfig
index 7c3bc184cf..6757fe662d 100644
--- a/configs/am62x_evm_a53_defconfig
+++ b/configs/am62x_evm_a53_defconfig
@@ -102,3 +102,4 @@ CONFIG_SYSRESET=y
 CONFIG_SPL_SYSRESET=y
 CONFIG_SYSRESET_TI_SCI=y
 CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
+CONFIG_SYSCON=y
diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c
index 523a4c9f91..ee46676ec8 100644
--- a/drivers/net/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ti/am65-cpsw-nuss.c
@@ -21,7 +21,9 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 
@@ -101,7 +103,6 @@ struct am65_cpsw_common {
fdt_addr_t  mdio_base;
fdt_addr_t  ale_base;
fdt_addr_t  gmii_sel;
-   fdt_addr_t  mac_efuse;
 
struct clk  fclk;
struct power_domain pwrdmn;
@@ -516,24 +517,45 @@ static void am65_cpsw_stop(struct udevice *dev)
common->started = false;
 }
 
+static int am65_cpsw_am654_get_efuse_macid(struct udevice *dev,
+  int slave, u8 *mac_addr)
+{
+   u32 mac_lo, mac_hi, offset;
+   struct regmap *syscon;
+   int ret;
+
+   syscon = syscon_regmap_lookup_by_phandle(dev, "ti,syscon-efuse");
+   if (IS_ERR(syscon)) {
+   if (PTR_ERR(syscon) == -ENODEV)
+   return 0;
+   return PTR_ERR(syscon);
+   }
+
+   ret = dev_read_u32_index(dev, "ti,syscon-efuse", 1, );
+   if (ret)
+   return ret;
+
+   regmap_read(syscon, offset, _lo);
+   regmap_read(syscon, offset + 4, _hi);
+
+   mac_addr[0] = (mac_hi >> 8) & 0xff;
+   mac_addr[1] = mac_hi & 0xff;
+   mac_addr[2] = (mac_lo >> 24) & 0xff;
+   mac_addr[3] = (mac_lo >> 16) & 0xff;
+   mac_addr[4] = (mac_lo >> 8) & 0xff;
+   mac_addr[5] = mac_lo & 0xff;
+
+   return 0;
+}
+
 static int am65_cpsw_read_rom_hwaddr(struct udevice *dev)
 {
struct am65_cpsw_priv *priv = dev_get_priv(dev);
-   struct am65_cpsw_common *common = priv->cpsw_common;
struct eth_pdata *pdata = dev_get_plat(dev);
-   u32 mac_hi, mac_lo;
-
-   if (common->mac_efuse == FDT_ADDR_T_NONE)
-   return -1;
 
-   mac_lo = readl(common->mac_efuse);
-   mac_hi = readl(common->mac_efuse + 4);
-   pdata->enetaddr[0] = (mac_hi >> 8) & 0xff;
-   pdata->enetaddr[1] = mac_hi & 0xff;
-   pdata->enetaddr[2] = (mac_lo >> 24) & 0xff;
-   pdata->enetaddr[3] = (mac_lo >> 16) & 0xff;
-   pdata->enetaddr[4] = (mac_lo >> 8) & 0xff;
-   pdata->enetaddr[5] = mac_lo & 0xff;
+   am65_cpsw_am654_get_efuse_macid(dev,
+   priv->port_id,
+   pdata->enetaddr);
 
return 0;
 }
@@ -710,8 +732,6 @@ static int am65_cpsw_probe_nuss(struct udevice *dev)
cpsw_common->ss_base = dev_read_addr(dev);
if (cpsw_common->ss_base == FDT_ADDR_T_NONE)
return -EINVAL;
-   cpsw_common->mac_efuse = devfdt_get_addr_name(dev, "mac_efuse");
-   /* no err check - optional */
 
ret = power_domain_get_by_index(dev, _common->pwrdmn, 0);
if (ret) {
-- 
2.34.1



[PATCH 0/2] net: ti: am65-cpsw-nuss: Drop custom property "mac_efuse"

2023-07-20 Thread Roger Quadros
Hi,

We need to track the Device tree in Linux.
The approved property for MAC address EFUSE is "ti,syscon-efuse".

Use that and drop custom property "mac_efuse".

cheers,
-roger

Roger Quadros (2):
  net: ti: am65-cpsw-nuss: Use approved property to get efuse address
  arm: dts: k3-am625-sk-u-boot.dtsi: drop mac_efuse

 arch/arm/dts/k3-am625-sk-u-boot.dtsi |  3 --
 configs/am62x_evm_a53_defconfig  |  1 +
 drivers/net/ti/am65-cpsw-nuss.c  | 52 +++-
 3 files changed, 37 insertions(+), 19 deletions(-)

-- 
2.34.1



Re: [PATCH] arm: mvebu: x240: Use i2c-gpio instead of built in controller

2023-07-20 Thread Chris Packham
Hi Me,

On Thu, Jul 20, 2023 at 3:03 PM Chris Packham  wrote:
>
> There is an Errata with the built-in I2C controller where various I2C
> hardware errors cause a complete lockup of the CPU (which eventually
> results in an watchdog reset).
>
> Put the I2C MPP pins into GPIO mode and use the i2c-gpio driver instead.
> This uses a bit-banged implementation of an I2C controller and avoids
> triggering the Errata.
>
> Signed-off-by: Chris Packham 
> ---
> This is dependent on a bug-fix for the i2c-gpio driver I just sent
> out[1] (sorry I should have sent them as a series but I thought this
> would take me longer to test than it did).
>
> [1] - 
> https://lore.kernel.org/u-boot/20230720023107.1184147-1-judge.pack...@gmail.com/
>
>  arch/arm/dts/ac5-98dx35xx-atl-x240.dts | 30 ++
>  configs/x240_defconfig |  1 +
>  2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/dts/ac5-98dx35xx-atl-x240.dts 
> b/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
> index 820ec18b4355..d47220520b9e 100644
> --- a/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
> +++ b/arch/arm/dts/ac5-98dx35xx-atl-x240.dts
> @@ -71,8 +71,25 @@
>  };
>
>   {
> -   status = "okay";
> +   status = "disabled";
> +};

I'll remove this section completely. I did want to make it clear that
we're using the same pins just as GPIOs instead of the dedicated I2C
mode which has issues but that can be explained better in the commit
message.

>
> +&{/} {

I should probably move this part up to the root node rather than
addressing it here.

> +   i2cgpio: i2c-gpio-0 {
> +   compatible = "i2c-gpio";
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +
> +   pinctrl-names = "default";
> +   pinctrl-0 =  <_gpio>;
> +   scl-gpios = < 26 (GPIO_ACTIVE_HIGH | 
> GPIO_OPEN_DRAIN)>;
> +   sda-gpios = < 27 (GPIO_ACTIVE_HIGH | 
> GPIO_OPEN_DRAIN)>;
> +   i2c-gpio,delay-us = <2>;
> +   status = "okay";
> +};
> +};
> +
> + {
> mux@71 {
> #address-cells = <1>;
> #size-cells = <0>;
> @@ -177,8 +194,8 @@
>  * LED_OE_N  [23]
>  * USB_PWR_FLT_N [24]
>  * SFP_INT_N [25]
> -* I2C0_SCL  [26]
> -* I2C0_SDA  [27]
> +* I2C0_SCL  [26] (GPIO)
> +* I2C0_SDA  [27] (GPIO)
>  * USB_EN[28]
>  * MONITOR_INT_N [29]
>  * XM1_MDC   [30]
> @@ -201,7 +218,7 @@
> /*   0123456789 */
> pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
>  0xff 0xff 11110xff 0xff 00
> -0000001100
> +0000000xff 0xff 00
>  1111000000
>  000111>;
>
> @@ -209,4 +226,9 @@
> marvell,pins = <0 1 2 3 4 5 6 7 8 9 10 11 16 17>;
> marvell,function = <2>;
> };
> +
> +   i2c0_gpio: i2c0-gpio-pins {
> +   marvell,pins = <26 27>;
> +   marvell,function = <0>;
> +   };
>  };
> diff --git a/configs/x240_defconfig b/configs/x240_defconfig
> index 6d25c5ae3fcf..e8589d636081 100644
> --- a/configs/x240_defconfig
> +++ b/configs/x240_defconfig
> @@ -43,6 +43,7 @@ CONFIG_CLK_MVEBU=y
>  CONFIG_GPIO_HOG=y
>  CONFIG_DM_PCA953X=y
>  CONFIG_DM_I2C=y
> +CONFIG_DM_I2C_GPIO=y
>  CONFIG_SYS_I2C_MVTWSI=y
>  CONFIG_I2C_MUX=y
>  CONFIG_I2C_MUX_PCA954x=y
> --
> 2.41.0
>


Re: [PATCH] efi_loader: Allow also empty capsule to be process

2023-07-20 Thread Heinrich Schuchardt

On 7/20/23 11:48, Sughosh Ganu wrote:

On Thu, 20 Jul 2023 at 14:56, Michal Simek  wrote:




On 7/20/23 10:45, Sughosh Ganu wrote:

On Thu, 20 Jul 2023 at 13:26, Michal Simek  wrote:




On 7/20/23 08:36, Sughosh Ganu wrote:

On Thu, 20 Jul 2023 at 11:37, Michal Simek  wrote:


Hi,

On 7/20/23 07:49, AKASHI Takahiro wrote:

Hi,

On Wed, Jul 19, 2023 at 08:28:41AM +0200, Michal Simek wrote:



On 7/18/23 17:41, Heinrich Schuchardt wrote:

On 13.07.23 16:35, Michal Simek wrote:

Empty capsule are also allowed to be process. Without it updated images
can't change their Image Acceptance state from no to yes.


Is there any documentation describing the usage of empty capsule to set
the image acceptance state?


I actually don't know about documentation. I was talking to Ilias to make
sure that documentation is up2date because there are missing couple of
things there.


Sughosh should have more to say here about A/B update.


I am testing A/B update and if you setup oemflags to 0x8000 then capsules
are not automatically accepted and waiting for acceptance capsule to be
passed.
When I tested it I found out that they are not process that's why I created
this patch.


The path you tried to modify is only executed by "efidebug capsule update"
or more specifically via the runtime service, UPDATE_CAPSULE.

But this API is NOT officially supported in the current capsule implementation
(at least, in my initial intention).
The only way to invoke capsule updates is to reboot the system.
If you want to test A/B update, please do the reboot.


I realized that to get full flow you need to use capsule update on disk to get
all functionalities. But it is very impractical. Actually I would expect via
efidebug you should be able to perform all steps as capsule update performs when
you do reboot.
I would also understand that via efidebug you are not able to apply any capsule
but I don't think it is right that you can apply just update capsules but not
empty capsules. I would understand none or all but not something in the middle.


The A/B update functionality requires using the capsule-on-disk
functionality for performing the updates. This is also mentioned in
the fwu_updates.rst document. You should be able to apply empty
capsules even with the 'efidebug disk-update' command.


Yes this is working fine.

ZynqMP> efidebug capsule disk-update
#
Applying capsule capsule1.bin succeeded.
#
Applying capsule capsule2.bin succeeded.
Reboot after firmware update.

I tested it also with empty capsules which are also process properly.


I have never
used the 'efidebug capsule update' command, so I'm not sure if that is
supported. Like Takahiro mentioned, if you place the capsules(genuine
or empty) under the /EFI/UpdateCapsule/ directory, the update should
happen automatically, since the fwu update feature also enables the
EFI_CAPSULE_ON_DISK_EARLY config.


Yes that's work fine on production systems.
But from my point of view there shouldn't be really a problem to also apply
empty capsule via efidebug capsule update to be able to see that steps and
changes in mdata structure without performing reset.


The 'efidebug capsule update' command calls the efi_update_capsule
function, which implements the UpdateCapsule runtime service call. The
initial versions of my fwu patches were indeed adding support for this
path, but one of the review comments was to restrict support only for
the capsule-on-disk path when performing the update in u-boot, since
we are not using the runtime call in u-boot.


I don't think this is a valid argument. As I said I would understand if there is
no interface for any capsule. It means having support for both or none is IMHO
the way we should support.
Can you please point me to that discussion?


There is mention of the point in this discussion [1]. Even this thread
has Takahiro mention the point he is making above, that maybe there
shouldn't be the efi_update_capsule function.

-sughosh


Hello Sugosh,

fwu_empty_capsule() detects an empty capsule as one with a GUID
fwu_guid_os_request_fw_revert or fwu_guid_os_request_fw_accept.

I am not aware of a requirement in the UEFI specification to treat
capsules read from file in a different way than capsules passed via
UpdateCapsule(). Is there any reason why UpdateCapsule() should not
process an empty capsule when called from a boot-time EFI application?

Best regards

Heinrich



[1] - https://lists.denx.de/pipermail/u-boot/2022-February/473891.html




Please pull u-boot-dm

2023-07-20 Thread Simon Glass
Hi Tom,

I am bringing in the binman changes now. There are going to be some
minor tweaks to templating as well as various patches from others, but
most are based on these patches.


https://source.denx.de/u-boot/custodians/u-boot-dm/-/pipelines/16964


The following changes since commit 0274eb61e1f2a8e053fb028b6c668c67c0b75b9c:

  Merge tag 'efi-2023-10-rc1-2' of
https://source.denx.de/u-boot/custodians/u-boot-efi (2023-07-20
10:19:04 -0400)

are available in the Git repository at:

  git://git.denx.de/u-boot-dm.git tags/dm-pull-20jul23

for you to fetch changes up to 24142ead21ed5e4d2d6f39dd410d91d815ea1ae2:

  binman: Reduce state.SetInt and bintool cmd to debug level
(2023-07-20 14:10:58 -0600)


binman mkimage and template enhancements
misc fixes


Eugen Hristev (1):
  dm: core: of_access: fix return value in of_property_match_string

Heinrich Schuchardt (1):
  cmd: fix loads, saves on sandbox

John Clark (1):
  bootstd: USB devtype detection for script boot

John Keeping (1):
  core: read: fix dev_read_addr_size()

Marek Vasut (1):
  binman: Convert mkimage to Entry_section

Maxim Cournoyer (2):
  tools: Fix README file in pyproject.toml of u_boot_pylib.
  tools: Fix package discovery in pyproject.toml of u_boot_pylib.

Sergei Antonov (1):
  sandbox: fix a compilation error

Simon Glass (18):
  binman: Init align_default in entry_Section
  binman: Use GetEntries() to obtain section contents
  binman: Read _multiple_data_files in the correct place
  binman: Allow disabling symbol writing
  stm32mp15: Avoid writing symbols in SPL
  binman: Update elf to return number of written symbols
  binman: Add more detail on how ObtainContents() works
  binman: Provide a way to specify the fdt-list directly
  binman: Drop __bss_size variable in bss_data.c
  binman: Correct handling of zero bss size
  dtoc: Support copying the contents of a node into another
  dtoc: Allow inserting a list of nodes into another
  binman: Support simple templates
  binman: Support templating with multiple images
  binman: Add a test for templating in a FIT
  binman: Support templates at any level
  binman: Support writing symbols inside a mkimage image
  binman: Reduce state.SetInt and bintool cmd to debug level

 arch/arm/dts/stm32mp15-u-boot.dtsi |   1 +
 arch/sandbox/include/asm/sdl.h |  23 +++
 arch/sandbox/include/asm/test.h|  25 ---
 boot/bootmeth_script.c |   5 +-
 cmd/load.c |  16 +++--
 drivers/core/of_access.c   |   5 +-
 drivers/core/read.c|   5 +-
 drivers/reset/reset-rockchip.c |   2 +-
 include/dm/read.h  |  12 +---
 test/dm/video.c|   1 +
 tools/binman/binman.rst|  94 +++
 tools/binman/bintool.py|   2 +-
 tools/binman/control.py|  32 -
 tools/binman/elf.py|  13 +++-
 tools/binman/elf_test.py   |  13 +++-
 tools/binman/entries.rst   |   6 ++
 tools/binman/entry.py  |  11 ++--
 tools/binman/etype/blob_phase.py   |   5 ++
 tools/binman/etype/fit.py  |   9 +++
 tools/binman/etype/mkimage.py  | 136
+++
 tools/binman/etype/section.py  |  54 
 tools/binman/etype/u_boot_spl_bss_pad.py   |   2 +-
 tools/binman/etype/u_boot_tpl_bss_pad.py   |   2 +-
 tools/binman/etype/u_boot_vpl_bss_pad.py   |   2 +-
 tools/binman/ftest.py  | 218
--
 tools/binman/state.py  |   4 +-
 tools/binman/test/282_symbols_disable.dts  |  25 +++
 tools/binman/test/283_mkimage_special.dts  |  24 +++
 tools/binman/test/284_fit_fdt_list.dts |  58 +
 tools/binman/test/285_spl_expand.dts   |  13 
 tools/binman/test/286_template.dts |  42 
 tools/binman/test/287_template_multi.dts   |  27 
 tools/binman/test/288_template_fit.dts |  37 +++
 tools/binman/test/289_template_section.dts |  52 +++
 tools/binman/test/290_mkimage_sym.dts  |  27 
 tools/binman/test/Makefile |   5 +-
 tools/binman/test/bss_data.c   |   3 +-
 tools/binman/test/bss_data_zero.c  |  16 +
 tools/binman/test/bss_data_zero.lds|  15 +
 tools/binman/test/embed_data.lds   |   1 +
 tools/dtoc/fdt.py  | 141
+++-
 tools/dtoc/test/dtoc_test_copy.dts |  88 +
 

[PATCH 1/1] doc: update coc/sphinx/requirements.txt

2023-07-20 Thread Heinrich Schuchardt
Update the following requirements to their latest version:

* Pygments - syntax highlighting
* pytz - world timezone definitions
* certifi  - Mozilla's CA bundle

Signed-off-by: Heinrich Schuchardt 
---
 doc/sphinx/requirements.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/sphinx/requirements.txt b/doc/sphinx/requirements.txt
index aed4492117..b74661ad3f 100644
--- a/doc/sphinx/requirements.txt
+++ b/doc/sphinx/requirements.txt
@@ -1,6 +1,6 @@
 alabaster==0.7.12
 Babel==2.9.1
-certifi==2022.12.7
+certifi==2023.5.7
 charset-normalizer==2.0.12
 docutils==0.16
 idna==3.3
@@ -8,9 +8,9 @@ imagesize==1.3.0
 Jinja2==3.0.3
 MarkupSafe==2.1.1
 packaging==21.3
-Pygments==2.11.2
+Pygments==2.15.1
 pyparsing==3.0.7
-pytz==2022.1
+pytz==2023.3
 requests==2.31.0
 six==1.16.0
 snowballstemmer==2.2.0
-- 
2.40.1



Re: [PATCH] travis-ci: Add m68k M5208EVBE machine

2023-07-20 Thread Tom Rini
On Thu, Jul 20, 2023 at 01:42:50PM -0600, Simon Glass wrote:
> Hi,
> 
> On Tue, 4 Apr 2023 at 15:43, Angelo Dureghello  
> wrote:
> >
> > Hi Tom,
> >
> > On 02/04/23 4:36 PM, Tom Rini wrote:
> > > On Sun, Apr 02, 2023 at 07:37:29AM +0200, Angelo Dureghello wrote:
> > >> Hi Marek,
> > >>
> > >> On 26/03/23 4:33 PM, Tom Rini wrote:
> > >>>
> > >>> On Mon, 20 Mar 2023 20:46:47 +0100, Marek Vasut wrote:
> >  Add m68k M5208EVBE machine configured to test U-Boot m68k support.
> > 
> > 
> > >>>
> > >>> Applied, thanks!
> > >>>
> > >>> [1/1] travis-ci: Add m68k M5208EVBE machine
> > >>> commit: 3f604a1b68a07e6c20f617c38fc849eb796f9af0
> > >>>
> > >>> Best regards,
> > >>
> > >> i rebased and tested the build, i see still a python error,
> > >>
> > >> https://source.denx.de/u-boot/custodians/u-boot-coldfire/-/pipelines/15856
> > >>
> > >> I applied
> > >>
> > >> 93acc7282341a294c28f50086ca1fb3d4cd66a90
> > >> travis-ci: Add m68k M5208EVBE machine
> > >>
> > >> 77e22d25af74c3be461b27d35805072ead63c178
> > >> CI: Add m68k target
> > >>
> > >> 31368868227702ad7176e4729a837e4a2183b739
> > >> arch: m68k: Add QEMU specific RAMBAR workaround
> > >>
> > >> f087d8873614e5bfa1093d5e3a1b6d4cf85623d1
> > >> arch: m68k: Introduce trivial PIT based timer
> > >>
> > >> d7ef34a0e1c500d4db2331fefaea688b1946a351
> > >> arch: m68k: Use existing CONFIG_MCFTMR instead of CFG_MCFTMR
> > >
> > > You should rebase this all on -next at this point, where the container
> > > has qemu-system-m68k, which is why it's failing there.
> > >
> >
> > Ok. So i keep applied this patch serie to u-boot-coldfire master.
> 
> This file has turned up in a new 'bin' directory in U-Boot. Shouldn't
> it be in the hoooks repo?

Yes, it's also in the hooks repository where it needs to be in order to
work.  This was committed to the main tree by accident and not noticed
until now.

-- 
Tom


signature.asc
Description: PGP signature


[PATCH v2 2/2] schemas: Add a schema for binman

2023-07-20 Thread Simon Glass
With this version I have done with a generic name, in this case 'data',
as suggested by Alper Nebi Yasak. This may be controversial, but we may
as well have the dicussion now. I assume that there are no other
ongoing attempts to define the layout of firmware in devicetree.

Signed-off-by: Simon Glass 
---

Changes in v2:
- Reworked significantly based on Alper's comments

 dtschema/schemas/firmware/binman/entry.yaml | 80 +
 dtschema/schemas/firmware/image.yaml| 77 
 2 files changed, 157 insertions(+)
 create mode 100644 dtschema/schemas/firmware/binman/entry.yaml
 create mode 100644 dtschema/schemas/firmware/image.yaml

diff --git a/dtschema/schemas/firmware/binman/entry.yaml 
b/dtschema/schemas/firmware/binman/entry.yaml
new file mode 100644
index 000..d50f96d
--- /dev/null
+++ b/dtschema/schemas/firmware/binman/entry.yaml
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2023 Google LLC
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/image/entry.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Image entry
+
+maintainers:
+  - Simon Glass 
+
+description: |
+  The entry node specifies a single entry in the firmware image.
+
+  Entries have a specific type, such as "u-boot" or "atf-bl31". This is 
provided
+  using compatible = "data,".
+
+  Note: This definition is intended to be hierarchical, so that entries can
+  appear in other entries. Schema for that is TBD.
+
+properties:
+  $nodename:
+pattern: "^[-a-z]+(-[0-9]+)?$"
+
+  compatible:
+$ref: /schemas/types.yaml#/definitions/string
+
+  offset:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: |
+  Provides the offset of this entry from the start of its parent section.
+
+  This may be omitted in the description provided by Binman, in which case
+  the value is calculated as part of image packing.
+
+  size:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: |
+  Provides the size of this entry in bytes.
+
+  This may be omitted in the description provided by Binman, in which case
+  the value is calculated as part of image packing.
+
+  reg:
+description: |
+  Defines the offset and size of this entry, with reference to its parent
+  image / section.
+
+  Note This is typically omitted in the description provided to Binman,
+  since the value is calculated as part of image packing. Separate
+  properties are provided for the size and offset of an entry, so that it 
is
+  easy to specify none, one or both. The `reg` property is the only one 
that
+  needs to be looked at once the image has been built.
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+firmware {
+  image {
+compatible = "data,image";
+#address-cells = <1>;
+$size-cells = <1>;
+
+u-boot@0 {
+  compatible = "data,u-boot";
+  reg = <0 0xa>;
+};
+
+atf-bl31@0x10 {
+  compatible = "data,atf-bl31";
+  reg = <0x10 0x2>;
+};
+  };
+};
diff --git a/dtschema/schemas/firmware/image.yaml 
b/dtschema/schemas/firmware/image.yaml
new file mode 100644
index 000..949b067
--- /dev/null
+++ b/dtschema/schemas/firmware/image.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2023 Google LLC
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/image.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Binman firmware layout
+
+maintainers:
+  - Simon Glass 
+
+description: |
+  The image node provides a layout for firmware, used when packaging firmware
+  from multiple projects. For now it just supports a very simple set of
+  features, as a starting point for discussion.
+
+  The Binman tool processes this node to produce a final image which can be
+  loaded into suitable storage device. Documentation is at:
+
+  https://u-boot.readthedocs.io/en/latest/develop/package/binman.html
+
+  The current image-description format is here:
+
+  
https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#image-description-format
+
+  It is desirable to reference the image from the storage-device node, perhaps
+  using an image-desc property:
+
+spiflash@0 {
+  compatible = "spidev", "jedec,spi-nor";
+  image-desc = <>;
+};
+
+  Note that the intention is to change Binman to use whatever schema is agreed
+  here.
+
+properties:
+  $nodename:
+const: binman
+
+  compatible:
+const: data,image
+
+  "#address-cells":
+const: 1
+
+  "#size-cells":
+const: 1
+
+required:
+  - compatible
+  - "#address-cell"
+  - "#size-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+firmware {
+  image {
+compatible = "data,image";
+#address-cells = <1>;
+$size-cells = <1>;
+
+u-boot@0 {
+  

[PATCH v2 1/2] schemas: Add firmware node schema

2023-07-20 Thread Simon Glass
Add a motivation and purpose for this new proposed node.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 dtschema/schemas/firmware.yaml | 83 ++
 1 file changed, 83 insertions(+)
 create mode 100644 dtschema/schemas/firmware.yaml

diff --git a/dtschema/schemas/firmware.yaml b/dtschema/schemas/firmware.yaml
new file mode 100644
index 000..4439a70
--- /dev/null
+++ b/dtschema/schemas/firmware.yaml
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-clause
+# Copyright 2023 Google LLC
+#
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: /firmware Node
+
+maintainers:
+  - Simon Glass 
+
+description: |
+  The '/firmware' node does not represent a real device, but serves as a place
+  for recording information about the main firmware used on the device, such as
+  a map of its contents. This is used by the Operating System (OS), user space
+  programs and possibly other firmware components. Data in the '/firmware' node
+  does not itself represent the hardware.
+
+  Properties in this node should be common to (and used by) at least two
+  firmware projects, such as U-Boot and TF-A. Project-specific subnodes can be
+  used for properties which are specific to a single project.
+
+  Purpose of '/firmware' node
+  ---
+
+  Firmware has traditionally been fairly opaque to the OS, with the OS taking
+  no interest in its contents, version, layout or how it might be updated. This
+  is less than ideal, since firmware is an important part of the system and
+  visibility into its operation is every bit as important as visbility into the
+  OS and user-space programs within the system.
+
+  The traditional approach has been to let firmware deal with firmware, and the
+  OS deal with everything else. Updating firmware has been handled by firmware.
+  For example, the UEFI spec defines a way for the OS to post a 'capsule' which
+  is discovered next time the system boots, permitting firmware updates. But
+  firmware updates in firmware are highly problematic. They require a reboot
+  and a sometimes-lengthy wait with a strange-looking interface unfamiliar
+  to most users. It seems better to make the update as transparent as possible
+  to the user. As an example of that, ChromeOS has full knowledge of the
+  firmware version and layout, updates it in the background from user space and
+  instantly selects the new firmware when the user reboots or logs out.
+
+  A common objection to considering the system holistically is that some parts
+  of the system are inaccessible to the OS, such as a secure enclave. However
+  this does not preclude providing visibility into what is present in that
+  enclave. Firmware-version information is still useful. Firmware updates are
+  still needed and can still be initiated from user space.
+
+  Another objection is that firmware should provide an interface to the OS,
+  while keeping its structure private. This thinking is largely driven by
+  extrapolating from how firmware has been handled in the 'BIOS' days.
+  It should be considered a degenerate case rather than the norm. As complexity
+  increases, it creates an artificial boundary between two pieces of the whole.
+  Mechanisms then need to be invented to cross this unnecessary chasm. An
+  example of this is Intel's Dynamic Platform and Thermal Framework (DPTF),
+  which consists of user-space, OS and firmware components all working towards
+  a shared goal. We need a standard description of these cross-system pieces.
+
+  In order to 'teach the OS about firmware', we need a place to put this
+  information. That is the purpose of this node.
+
+  In an Open Source world the entire model of firmware needs to adjust to be
+  more open, more visible and managed just like any other part of the system.
+  The major goal is to standardise how firmware is presented to the OS and user
+  space, so that common utilities can be used to manage the entire system,
+  including the firmware. For example, fwupd can look in this node for
+  information on how to update the firmware, similar to how VBE works. [1]
+  It is likely that other purposes will come to light over time.
+
+  [1] https://github.com/fwupd/fwupd/tree/main/plugins/vbe
+
+properties:
+  $nodename:
+const: firmware
+
+  "#address-cells": true
+  "#size-cells": true
+
+additionalProperties:
+  type: object
-- 
2.41.0.487.g6d72f3e995-goog



Re: [PATCH 2/2] schemas: Add a schema for binman

2023-07-20 Thread Simon Glass
Hi Alper,

On Thu, 20 Jul 2023 at 09:23, Alper Nebi Yasak  wrote:
>
> Hi Simon,
>
> I know I haven't been able to look at binman stuff for a long time, but
> I've been occasionally thinking about it through the course of a year
> and think binman severely needs a design-wise review before things get
> entrenched, even in the most fundamental parts. I do see the
> cross-project collaboration intention here, but still...

Yes, I agree. I was expecting a few more comments but I suppose not
that many people worry too much about open firmware. Many of the
points you've mentioned here have been in my mind as we've moved from
a prototype 6 years ago to what Binman is today, and I know we've
discussed some before on the mailing list. I really appreciated you
collecting your thoughts on this. The whole thing needs a new look and
I hope that the discussion here can help us settle on an industry
standard for this stuff.

So...

>
> There's also the issue of binman having multiple different device-trees:
> its input, the ones in fdtmaps per image, the ones injected to U-Boot
> dtb files per image. I'd say each has different needs, and those
> differences have to be figured out before specified upstream. I can only
> guess this is about the one that'll be in a u-boot.dtb.

Well, there is really only one. The fdtmap is actually the same
schema, except that it mentions only the image that it is embedded in,
i.e. if the fdtmap is for the SPI image, then the fdtmap in SPI flash
only contains the definition for the SPI image, not the MMC image
which is in a different device. The input is the same schema, albeit
that things like 'offset' may be filled in by Binman automatically
when it packs things.

>
> I want to go through binman more thoroughly, but a lot of changes
> happened since I last looked at it and I'm a bit slow at writing things,
> so won't exactly be soon. That being said, here's some ideas off the top
> of my head, for inspiration on both this schema and binman itself.

Do you mean the code? There are definitely some abstractions that are
stretching a bit, but it is mostly holding together for now.

>
> On 2023-07-12 00:18 +03:00, Simon Glass wrote:
> > I am unsure whether to add this with a generic name, such as 'layout',
> > but for now am using /firmware/binman to avoid conflicts with any other
> > firmware-layout schema that others might be working on.
> >
> > Signed-off-by: Simon Glass 
> > ---
>
> I've been thinking of compatible = "data," for entries, so
> proposing 'data' here. A big ask, but it might be the one schema to
> unify them all if we look at things as "description of data" instead of
> "firmware layout".

As I mentioned in the commit message I was a bit unsure about just
coming out with a generic name, but let's do it and see what people
think.

>
> Also consider data layouts in-memory. For example it could be an
> alternative to /chosen linux,initrd-start to specify initramfs location.
> Or things like keys or logs received from previous stages / other parts?
> Weak examples, but maybe might connect better into firmware handoff
> things. (Sorry, I don't know much about those.)

Yes I suspect at the high level those are better handled by firmware
handoff, which is some other work going on. But I would like this to
be a standard for firmware images, so BInman could perhaps plug into
the handoff size of things too,

>
> I'm also thinking about things like JPEG/BMP files for ACPI BGRT-like
> boot splash, unique firmware/configuration/calibration data for drivers,
> but they don't exactly need to be in-memory I guess.

Sure,

>
> >
> >  dtschema/schemas/firmware/binman.yaml   | 51 ++
> >  dtschema/schemas/firmware/binman/entry.yaml | 57 +
> >  2 files changed, 108 insertions(+)
> >  create mode 100644 dtschema/schemas/firmware/binman.yaml
> >  create mode 100644 dtschema/schemas/firmware/binman/entry.yaml
> >
> > diff --git a/dtschema/schemas/firmware/binman.yaml 
> > b/dtschema/schemas/firmware/binman.yaml
> > new file mode 100644
> > index 000..4b1ecf6
> > --- /dev/null
> > +++ b/dtschema/schemas/firmware/binman.yaml
> > @@ -0,0 +1,51 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright 2023 Google LLC
> > +
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/firmware/binman.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Binman firmware layout
> > +
> > +maintainers:
> > +  - Simon Glass 
> > +
> > +description: |
> > +  The binman node provides a layout for firmware, used when packaging 
> > firmware
> > +  from multiple projects. For now it just supports a very simple set of
> > +  features, as a starting point for discussion.
> > +
> > +  Documentation for Binman is available at:
> > +
> > +  https://u-boot.readthedocs.io/en/latest/develop/package/binman.html
> > +
> > +  with the current image-description format at:
> > +
> > +  
> > 

Re: [PATCH v2 5/7] power: pmic-uclass: probe every child on pmic_post_probe

2023-07-20 Thread Simon Glass
Hi Svyatoslav,

On Thu, 20 Jul 2023 at 06:38, Svyatoslav Ryhel  wrote:
>
> Main goal is to probe all regulator childrens for their
> proper setup but if pmic has non regulator children they
> should not suffer from this either.
>
> Signed-off-by: Svyatoslav Ryhel 
> ---
>  drivers/power/pmic/pmic-uclass.c | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/power/pmic/pmic-uclass.c 
> b/drivers/power/pmic/pmic-uclass.c
> index 0e2f5e1f41..8ca717bd5e 100644
> --- a/drivers/power/pmic/pmic-uclass.c
> +++ b/drivers/power/pmic/pmic-uclass.c
> @@ -16,6 +16,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 

I'm not sure about this.

The idea is that power is handling automatically, e.g. a device is
probed and so its power is enabled. If you do everything at the start,
doesn't that violate the 'lazy' init side of U-Boot?

>
>  #if CONFIG_IS_ENABLED(PMIC_CHILDREN)
> @@ -198,9 +199,18 @@ static int pmic_pre_probe(struct udevice *dev)
> return 0;
>  }
>
> +static int pmic_post_probe(struct udevice *dev)
> +{
> +   struct udevice *child;
> +
> +   device_foreach_child_probe(child, dev);
> +   return 0;
> +}
> +
>  UCLASS_DRIVER(pmic) = {
> .id = UCLASS_PMIC,
> .name   = "pmic",
> .pre_probe  = pmic_pre_probe,
> +   .post_probe = pmic_post_probe,
> .per_device_auto= sizeof(struct uc_pmic_priv),
>  };
> --
> 2.39.2
>

Regards,
Simon


Re: [PATCH v7 1/4] i2c: designware: Add CONFIG_ACPIGEN limitation to designware_i2c_pci.c

2023-07-20 Thread Simon Glass
Hi,

On Thu, 20 Jul 2023 at 05:24, Minda Chen  wrote:
>
> As the designware_i2c_pci.c uses ACPI APIs, If some SoCs (StarFive
> JH7110) contain designware i2c and PCI but do not use ACPI,
> This file will be can't be compiled. So add ACPIGEN to
> designware_i2c_pci.c
>
> Signed-off-by: Minda Chen 
> ---
>  drivers/i2c/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> index 99545df2e5..92b1ec6bc3 100644
> --- a/drivers/i2c/Makefile
> +++ b/drivers/i2c/Makefile
> @@ -18,9 +18,11 @@ obj-$(CONFIG_SYS_I2C_CADENCE) += i2c-cdns.o
>  obj-$(CONFIG_SYS_I2C_CA) += i2c-cortina.o
>  obj-$(CONFIG_SYS_I2C_DAVINCI) += davinci_i2c.o
>  obj-$(CONFIG_SYS_I2C_DW) += designware_i2c.o
> +ifdef CONFIG_ACPIGEN

You should use Kconfig to see SYS_I2C_DW depends on ACPIGEN. You might
need a separate I2C_DW_PCI Kconfig.

But that sounds bad to me. Why does it have to generate ACPI tables?
It should work fine without that.

>  ifdef CONFIG_PCI
>  obj-$(CONFIG_SYS_I2C_DW) += designware_i2c_pci.o
>  endif
> +endif
>  obj-$(CONFIG_SYS_I2C_FSL) += fsl_i2c.o
>  obj-$(CONFIG_SYS_I2C_IHS) += ihs_i2c.o
>  obj-$(CONFIG_SYS_I2C_INTEL) += intel_i2c.o
> --
> 2.17.1
>

Regards,
Simon


Re: [PATCH v2 1/9] power: pmic-uclass: implement poweroff ops

2023-07-20 Thread Simon Glass
Hi Svyatoslav,

On Thu, 20 Jul 2023 at 02:48, Svyatoslav Ryhel  wrote:
>
> PMICs are responsible for device poweroff sequence so lets implement
> this function.
>
> Signed-off-by: Svyatoslav Ryhel 
> ---
>  drivers/power/pmic/pmic-uclass.c | 12 
>  include/power/pmic.h | 13 +
>  2 files changed, 25 insertions(+)
>
> diff --git a/drivers/power/pmic/pmic-uclass.c 
> b/drivers/power/pmic/pmic-uclass.c
> index 0e2f5e1f41..23803bc96a 100644
> --- a/drivers/power/pmic/pmic-uclass.c
> +++ b/drivers/power/pmic/pmic-uclass.c
> @@ -99,6 +99,18 @@ int pmic_get(const char *name, struct udevice **devp)
> return uclass_get_device_by_name(UCLASS_PMIC, name, devp);
>  }
>
> +int pmic_poweroff(struct udevice *dev)
> +{
> +   const struct dm_pmic_ops *ops = dev_get_driver_ops(dev);
> +   struct uc_pmic_priv *priv = dev_get_uclass_priv(dev);
> +
> +   if (!ops || !ops->poweroff ||
> +   !priv->sys_pow_ctrl)
> +   return -ENOSYS;
> +
> +   return ops->poweroff(dev);
> +}
> +
>  int pmic_reg_count(struct udevice *dev)
>  {
> const struct dm_pmic_ops *ops = dev_get_driver_ops(dev);
> diff --git a/include/power/pmic.h b/include/power/pmic.h
> index 636221692d..d8dd1ceaa3 100644
> --- a/include/power/pmic.h
> +++ b/include/power/pmic.h
> @@ -157,11 +157,13 @@ struct pmic {
>   * Should be implemented by UCLASS_PMIC device drivers. The standard
>   * device operations provides the I/O interface for it's childs.
>   *
> + * @poweroff:  perform poweroff sequence
>   * @reg_count: device's register count
>   * @read:  read 'len' bytes at "reg" and store it into the 'buffer'
>   * @write: write 'len' bytes from the 'buffer' to the register at 'reg' 
> address
>   */
>  struct dm_pmic_ops {
> +   int (*poweroff)(struct udevice *dev);

Please do remember to fully comment each method here too - see p2sb
set_hide() for an example.

> int (*reg_count)(struct udevice *dev);
> int (*read)(struct udevice *dev, uint reg, uint8_t *buffer, int len);
> int (*write)(struct udevice *dev, uint reg, const uint8_t *buffer,
> @@ -242,6 +244,16 @@ int pmic_bind_children(struct udevice *pmic, ofnode 
> parent,
>   */
>  int pmic_get(const char *name, struct udevice **devp);
>
> +/**
> + * pmic_poweroff: call the pmic poweroff sequence
> + *
> + * The required pmic device can be obtained by 'pmic_get()'
> + *
> + * @dev - pointer to the UCLASS_PMIC device
> + * Return: device turns off or negative value of errno.
> + */
> +int pmic_poweroff(struct udevice *dev);
> +
>  /**
>   * pmic_reg_count: get the pmic register count
>   *
> @@ -306,6 +318,7 @@ int pmic_clrsetbits(struct udevice *dev, uint reg, uint 
> clr, uint set);
>   */
>  struct uc_pmic_priv {
> uint trans_len;
> +   bool sys_pow_ctrl;

comment

>  };
>
>  #endif /* DM_PMIC */
> --
> 2.39.2
>

This needs a test addition to test/dm/pmic.c (perhaps you have it in
another patch).

Regards,
Simon


Re: [PATCH v2 1/7] power: regulator: expand basic reference counter onto all uclass

2023-07-20 Thread Simon Glass
Hi Svyatoslav,

On Thu, 20 Jul 2023 at 06:38, Svyatoslav Ryhel  wrote:
>
> Commit is based on 4fcba5d ("regulator: implement basic reference
> counter") but expands the idea to all regulators instead of just
> fixed/gpio regulators.
>
> Signed-off-by: Svyatoslav Ryhel 
> ---
>  drivers/power/regulator/regulator-uclass.c | 41 ++
>  drivers/power/regulator/regulator_common.c | 22 
>  drivers/power/regulator/regulator_common.h | 21 ---
>  include/power/regulator.h  |  2 ++
>  4 files changed, 43 insertions(+), 43 deletions(-)

Reviewed-by: Simon Glass 

nit below

>
> diff --git a/drivers/power/regulator/regulator-uclass.c 
> b/drivers/power/regulator/regulator-uclass.c
> index 3a6ba69f6d..fc7a4631b4 100644
> --- a/drivers/power/regulator/regulator-uclass.c
> +++ b/drivers/power/regulator/regulator-uclass.c
> @@ -159,6 +159,25 @@ int regulator_get_enable(struct udevice *dev)
> return ops->get_enable(dev);
>  }
>
> +/*
> + * Enable or Disable a regulator
> + *
> + * This is a reentrant function and subsequent calls that enable will
> + * increase an internal counter, and disable calls will decrease the counter.
> + * The actual resource will be enabled when the counter gets to 1 coming 
> from 0,
> + * and disabled when it reaches 0 coming from 1.
> + *
> + * @dev: regulator device
> + * @enable: bool indicating whether to enable or disable the regulator
> + * @return:
> + * 0 on Success
> + * -EBUSY if the regulator cannot be disabled because it's requested by
> + *another device
> + * -EALREADY if the regulator has already been enabled or has already been
> + *disabled
> + * -EACCES if there is no possibility to enable/disable the regulator
> + * -ve on different error situation
> + */
>  int regulator_set_enable(struct udevice *dev, bool enable)
>  {
> const struct dm_regulator_ops *ops = dev_get_driver_ops(dev);
> @@ -172,6 +191,23 @@ int regulator_set_enable(struct udevice *dev, bool 
> enable)
> if (!enable && uc_pdata->always_on)
> return -EACCES;
>
> +   /* If previously enabled, increase count */
> +   if (enable && uc_pdata->enable_count > 0) {
> +   uc_pdata->enable_count++;
> +   return -EALREADY;
> +   }
> +
> +   if (!enable) {
> +   if (uc_pdata->enable_count > 1) {
> +   /* If enabled multiple times, decrease count */
> +   uc_pdata->enable_count--;
> +   return -EBUSY;
> +   } else if (!uc_pdata->enable_count) {
> +   /* If already disabled, do nothing */
> +   return -EALREADY;
> +   }
> +   }
> +
> if (uc_pdata->ramp_delay)
> old_enable = regulator_get_enable(dev);
>
> @@ -187,6 +223,11 @@ int regulator_set_enable(struct udevice *dev, bool 
> enable)
> }
> }
>
> +   if (enable)
> +   uc_pdata->enable_count++;
> +   else
> +   uc_pdata->enable_count--;
> +
> return ret;
>  }
>
> diff --git a/drivers/power/regulator/regulator_common.c 
> b/drivers/power/regulator/regulator_common.c
> index e26f5ebec3..d88bc6f6de 100644
> --- a/drivers/power/regulator/regulator_common.c
> +++ b/drivers/power/regulator/regulator_common.c
> @@ -72,23 +72,6 @@ int regulator_common_set_enable(const struct udevice *dev,
> return 0;
> }
>
> -   /* If previously enabled, increase count */
> -   if (enable && plat->enable_count > 0) {
> -   plat->enable_count++;
> -   return -EALREADY;
> -   }
> -
> -   if (!enable) {
> -   if (plat->enable_count > 1) {
> -   /* If enabled multiple times, decrease count */
> -   plat->enable_count--;
> -   return -EBUSY;
> -   } else if (!plat->enable_count) {
> -   /* If already disabled, do nothing */
> -   return -EALREADY;
> -   }
> -   }
> -
> ret = dm_gpio_set_value(>gpio, enable);
> if (ret) {
> pr_err("Can't set regulator : %s gpio to: %d\n", dev->name,
> @@ -103,10 +86,5 @@ int regulator_common_set_enable(const struct udevice *dev,
> if (!enable && plat->off_on_delay_us)
> udelay(plat->off_on_delay_us);
>
> -   if (enable)
> -   plat->enable_count++;
> -   else
> -   plat->enable_count--;
> -
> return 0;
>  }
> diff --git a/drivers/power/regulator/regulator_common.h 
> b/drivers/power/regulator/regulator_common.h
> index d4962899d8..15f1fa4c93 100644
> --- a/drivers/power/regulator/regulator_common.h
> +++ b/drivers/power/regulator/regulator_common.h
> @@ -13,7 +13,6 @@ struct regulator_common_plat {
> struct gpio_desc gpio; /* GPIO for regulator enable control */
> unsigned int 

Re: [PATCH v2 3/7] power: regulator-uclass: perform regulator setup inside uclass

2023-07-20 Thread Simon Glass
Hi Svyatoslav,

On Thu, 20 Jul 2023 at 06:38, Svyatoslav Ryhel  wrote:
>
> Regulators initial setup was previously dependent on board call.
> To move from this behaviour were introduced two steps. First is
> that all individual regulators will be probed just after binding

We must not probe devices automatically when bound. The i2c interface
may not be available, etc. Do a probe afterwards.

Perhaps I am misunderstanding this, iwc please reword this commit message.

> which ensures that regulator pdata is filled and second is setting
> up regulator in post probe which enseres that correct regulator
> state according to pdata is reached.
>
> Signed-off-by: Svyatoslav Ryhel 
> ---
>  drivers/power/regulator/regulator-uclass.c | 212 ++---
>  include/power/regulator.h  | 119 
>  2 files changed, 62 insertions(+), 269 deletions(-)

Regards,
SImon


Re: [PATCH v2 8/9] power: pmic: tps65910: add TPS65911 PMIC support

2023-07-20 Thread Simon Glass
On Thu, 20 Jul 2023 at 02:48, Svyatoslav Ryhel  wrote:
>
> Add support to bind the regulators/child nodes with the pmic.
> Also adds the pmic i2c based read/write functions to access pmic
> registers.
>
> Signed-off-by: Svyatoslav Ryhel 
> ---
>  doc/device-tree-bindings/pmic/tps65911.txt | 78 ++
>  drivers/power/pmic/pmic_tps65910_dm.c  | 49 +-
>  include/power/tps65910_pmic.h  | 52 +++
>  3 files changed, 176 insertions(+), 3 deletions(-)
>  create mode 100644 doc/device-tree-bindings/pmic/tps65911.txt
>

Reviewed-by: Simon Glass 

nits below

[..]

> diff --git a/drivers/power/pmic/pmic_tps65910_dm.c 
> b/drivers/power/pmic/pmic_tps65910_dm.c
> index e03ddc98d7..770010d53d 100644
> --- a/drivers/power/pmic/pmic_tps65910_dm.c
> +++ b/drivers/power/pmic/pmic_tps65910_dm.c
> @@ -11,13 +11,19 @@
>  #include 
>  #include 
>
> -static const struct pmic_child_info pmic_children_info[] = {
> +static const struct pmic_child_info tps65910_children_info[] = {
> { .prefix = "ldo_", .driver = TPS65910_LDO_DRIVER },
> { .prefix = "buck_", .driver = TPS65910_BUCK_DRIVER },
> { .prefix = "boost_", .driver = TPS65910_BOOST_DRIVER },
> { },
>  };
>
> +static const struct pmic_child_info tps65911_children_info[] = {
> +   { .prefix = "ldo", .driver = TPS65911_LDO_DRIVER },
> +   { .prefix = "vdd", .driver = TPS65911_VDD_DRIVER },
> +   { },
> +};
> +
>  static int pmic_tps65910_reg_count(struct udevice *dev)
>  {
> return TPS65910_NUM_REGS;
> @@ -47,10 +53,29 @@ static int pmic_tps65910_read(struct udevice *dev, uint 
> reg, u8 *buffer,
> return ret;
>  }
>
> +static int pmic_tps65910_poweroff(struct udevice *dev)
> +{
> +   int ret;
> +
> +   ret = pmic_reg_read(dev, TPS65910_REG_DEVICE_CTRL);
> +   if (ret < 0)
> +   return ret;
> +
> +   ret |= DEVCTRL_PWR_OFF_MASK;

Please use a separate var for this. We use ret for return values, as
you have above.

> +
> +   pmic_reg_write(dev, TPS65910_REG_DEVICE_CTRL, ret);
> +
> +   ret |= DEVCTRL_DEV_OFF_MASK;
> +   ret &= ~DEVCTRL_DEV_ON_MASK;
> +
> +   return pmic_reg_write(dev, TPS65910_REG_DEVICE_CTRL, ret);
> +}
> +
>  static int pmic_tps65910_bind(struct udevice *dev)
>  {
> ofnode regulators_node;
> int children;
> +   int type = dev_get_driver_data(dev);
>
> regulators_node = dev_read_subnode(dev, "regulators");
> if (!ofnode_valid(regulators_node)) {
> @@ -58,7 +83,19 @@ static int pmic_tps65910_bind(struct udevice *dev)
> return -EINVAL;
> }
>
> -   children = pmic_bind_children(dev, regulators_node, 
> pmic_children_info);
> +   switch (type) {

Why not always binding them?

> +   case TPS65910:
> +   children = pmic_bind_children(dev, regulators_node,
> + tps65910_children_info);
> +   break;
> +   case TPS65911:
> +   children = pmic_bind_children(dev, regulators_node,
> + tps65911_children_info);
> +   break;
> +   default:
> +   log_err("unknown PMIC type\n");
> +   }
> +
> if (!children)
> debug("%s has no children (regulators)\n", dev->name);
>
> @@ -67,6 +104,10 @@ static int pmic_tps65910_bind(struct udevice *dev)
>
>  static int pmic_tps65910_probe(struct udevice *dev)
>  {
> +   struct uc_pmic_priv *priv = dev_get_uclass_priv(dev);
> +
> +   priv->sys_pow_ctrl = dev_read_bool(dev, "ti,system-power-controller");
> +
> /* use I2C control interface instead of I2C smartreflex interface to
>  * access smartrefelex registers VDD1_OP_REG, VDD1_SR_REG, VDD2_OP_REG
>  * and VDD2_SR_REG
> @@ -76,13 +117,15 @@ static int pmic_tps65910_probe(struct udevice *dev)
>  }
>
>  static struct dm_pmic_ops pmic_tps65910_ops = {
> +   .poweroff = pmic_tps65910_poweroff,
> .reg_count = pmic_tps65910_reg_count,
> .read = pmic_tps65910_read,
> .write = pmic_tps65910_write,
>  };
>
>  static const struct udevice_id pmic_tps65910_match[] = {
> -   { .compatible = "ti,tps65910" },
> +   { .compatible = "ti,tps65910", .data = TPS65910 },
> +   { .compatible = "ti,tps65911", .data = TPS65911 },
> { /* sentinel */ }
>  };
>
> diff --git a/include/power/tps65910_pmic.h b/include/power/tps65910_pmic.h
> index 66214786d3..282863805a 100644
> --- a/include/power/tps65910_pmic.h
> +++ b/include/power/tps65910_pmic.h
> @@ -6,6 +6,9 @@
>  #ifndef __TPS65910_PMIC_H_
>  #define __TPS65910_PMIC_H_
>
> +#define TPS65910   1
> +#define TPS65911   2
> +
>  #define TPS65910_I2C_SEL_MASK  (0x1 << 4)
>  #define TPS65910_VDD_SR_MASK   (0x1 << 7)
>  #define TPS65910_GAIN_SEL_MASK (0x3 << 6)
> @@ -17,6 +20,11 @@
>  #define TPS65910_SUPPLY_STATE_OFF  

Re: fdt_high default value causes unbootable kernel due to misalignment

2023-07-20 Thread Simon Glass
Hi Tim,

On Thu, 20 Jul 2023 at 08:40, Tim van der Staaij | Zign
 wrote:
>
> >I wonder if we should adjust mkimage to use -B 8 as the default?
>
> Ah, overlooked this option. I ran a test with -B 8 and by itself it doesn't 
> fix the issue. However, as the manpage notes, -B is only effective when -E is 
> also used. And indeed, using -E -B 8 does avoid misalignment when the device 
> tree is used in-place.
>
> Sounds like a good fix to me, but I don't know if there would be broader 
> consequences of defaulting to -E? It sounds like this changes the structure 
> of the fitimage rather dramatically.

Yes it does, see [1]. But there are some advantages to having all the
nodes/properties at the start of the file.

Regards,
Simon

[1] 
https://u-boot.readthedocs.io/en/latest/usage/fit/source_file_format.html#external-data


Re: [PATCH] travis-ci: Add m68k M5208EVBE machine

2023-07-20 Thread Simon Glass
Hi,

On Tue, 4 Apr 2023 at 15:43, Angelo Dureghello  wrote:
>
> Hi Tom,
>
> On 02/04/23 4:36 PM, Tom Rini wrote:
> > On Sun, Apr 02, 2023 at 07:37:29AM +0200, Angelo Dureghello wrote:
> >> Hi Marek,
> >>
> >> On 26/03/23 4:33 PM, Tom Rini wrote:
> >>>
> >>> On Mon, 20 Mar 2023 20:46:47 +0100, Marek Vasut wrote:
>  Add m68k M5208EVBE machine configured to test U-Boot m68k support.
> 
> 
> >>>
> >>> Applied, thanks!
> >>>
> >>> [1/1] travis-ci: Add m68k M5208EVBE machine
> >>> commit: 3f604a1b68a07e6c20f617c38fc849eb796f9af0
> >>>
> >>> Best regards,
> >>
> >> i rebased and tested the build, i see still a python error,
> >>
> >> https://source.denx.de/u-boot/custodians/u-boot-coldfire/-/pipelines/15856
> >>
> >> I applied
> >>
> >> 93acc7282341a294c28f50086ca1fb3d4cd66a90
> >> travis-ci: Add m68k M5208EVBE machine
> >>
> >> 77e22d25af74c3be461b27d35805072ead63c178
> >> CI: Add m68k target
> >>
> >> 31368868227702ad7176e4729a837e4a2183b739
> >> arch: m68k: Add QEMU specific RAMBAR workaround
> >>
> >> f087d8873614e5bfa1093d5e3a1b6d4cf85623d1
> >> arch: m68k: Introduce trivial PIT based timer
> >>
> >> d7ef34a0e1c500d4db2331fefaea688b1946a351
> >> arch: m68k: Use existing CONFIG_MCFTMR instead of CFG_MCFTMR
> >
> > You should rebase this all on -next at this point, where the container
> > has qemu-system-m68k, which is why it's failing there.
> >
>
> Ok. So i keep applied this patch serie to u-boot-coldfire master.

This file has turned up in a new 'bin' directory in U-Boot. Shouldn't
it be in the hoooks repo?

Regards,
Simon


Re: data abort when run 'dhcp'

2023-07-20 Thread Tom Rini
On Thu, Jul 20, 2023 at 06:39:17PM +0200, Miquel Raynal wrote:
> Hello,
> 
> qianfangui...@163.com wrote on Fri, 25 Mar 2022 18:04:46 +0800:
> 
> > It's very strange. And I can't detect it's a bug of usb or dlmalloc.
> > 
> > 1. Starting u-boot and dhcp via am335x's ethernet(cpsw driver), it's ok.
> > 
> > 2. Starting u-boot and dhcp via am335x's usb net, data abort.
> > 
> > 3. start fastboot, and CTRL C right now, dhcp via am335x's usb net, it's ok.
> 
> I am sorry to re-open a thread that is one year old but this is
> still an open bug. The BBB is affected. In particular the BBBW
> because there is no Ethernet connector, which makes the Eth-over-USB
> emulation even more important. All U-Boots since 2021 are affected:
> spurious data aborts, usually at the end of network interactions (tftp,
> ping). I could not bisect it because the boot was deeply broken as
> well on a significant range of commits :-/.
> 
> On my side I narrowed it down to an env update which fails in malloc as
> well. If I comment the env update, it fails a bit later. It really
> looks like a stack corruption which is either related to the Ethernet
> USB gadget or the USB controller driver itself. Network transfers on
> the BBBW using regular Ethernet does not trigger any error.
> 
> I also observe the very strange "fix" mentioned above: starting and
> killing fastboot makes all tftp pass... If anyone has more details to
> share, or perhaps a subsequent thread giving more details, I would
> really like to see this fixed upstream, I suppose I am not the only one
> :-)

What happens if you increase the malloc pool from say 32MB (current
value, 0x200) to 64MB (so 0x400) ?

-- 
Tom


signature.asc
Description: PGP signature


Re: data abort when run 'dhcp'

2023-07-20 Thread Heinrich Schuchardt



Am 20. Juli 2023 18:39:17 MESZ schrieb Miquel Raynal 
:
>Hello,
>
>qianfangui...@163.com wrote on Fri, 25 Mar 2022 18:04:46 +0800:
>
>> It's very strange. And I can't detect it's a bug of usb or dlmalloc.
>> 
>> 1. Starting u-boot and dhcp via am335x's ethernet(cpsw driver), it's ok.
>> 
>> 2. Starting u-boot and dhcp via am335x's usb net, data abort.
>> 
>> 3. start fastboot, and CTRL C right now, dhcp via am335x's usb net, it's ok.
>
>I am sorry to re-open a thread that is one year old but this is
>still an open bug. The BBB is affected. In particular the BBBW
>because there is no Ethernet connector, which makes the Eth-over-USB
>emulation even more important. All U-Boots since 2021 are affected:
>spurious data aborts, usually at the end of network interactions (tftp,
>ping). I could not bisect it because the boot was deeply broken as
>well on a significant range of commits :-/.
>
>On my side I narrowed it down to an env update which fails in malloc as
>well. If I comment the env update, it fails a bit later. It really
>looks like a stack corruption which is either related to the Ethernet
>USB gadget or the USB controller driver itself. Network transfers on
>the BBBW using regular Ethernet does not trigger any error.
>
>I also observe the very strange "fix" mentioned above: starting and
>killing fastboot makes all tftp pass... If anyone has more details to
>share, or perhaps a subsequent thread giving more details, I would
>really like to see this fixed upstream, I suppose I am not the only one
>:-)
>
>Thanks,
>Miquèl


Can this problem be reproduced on QEMU?

Best regards

Heinrich 

>
>> 
>> 在 2022/3/24 17:33, qianfan 写道:
>> >
>> > 在 2022/3/23 18:12, Heinrich Schuchardt 写道:  
>> >> On 3/23/22 11:07, qianfan wrote:  
>> >>>
>> >>> 在 2022/3/23 17:51, Heinrich Schuchardt 写道:  
>>  On 3/23/22 10:13, qianfan wrote:  
>> >
>> > 在 2022/3/23 16:02, qianfan 写道:  
>> >>
>> >>
>> >> 在 2022/3/23 15:45, qianfan 写道:  
>> >>>
>> >>>
>> >>> 在 2022/3/23 10:28, qianfan 写道:  
>> 
>>  Hi:
>> 
>>  I had a custom AM335X board connected my computer by usbnet. It
>>  always report data abort when 'dhcp':
>> 
>>  Next it the log:
>> 
>>  U-Boot 2022.01-rc1-00183-gfa5b4e2d19-dirty (Feb 25 2022 - 15:45:02
>>  +0800)
>> 
>>  CPU  : AM335X-GP rev 2.1
>>  Model: WISDOM AM335X CCT
>>  DRAM:  512 MiB
>>  NAND:  256 MiB
>>  MMC:   OMAP SD/MMC: 0
>>  Loading Environment from NAND... *** Warning - bad CRC, using
>>  default environment
>> 
>>  Net:   Could not get PHY for ethernet@4a10: addr 0
>>  eth2: ethernet@4a10, eth3: usb_ether
>>  Hit any key to stop autoboot:  0  
>>  => setenv autoload no
>>  => dhcp  
>>  using musb-hdrc, OUT ep1out IN ep1in STATUS ep2in
>>  MAC de:ad:be:ef:00:01
>>  HOST MAC de:ad:be:ef:00:00
>>  RNDIS ready
>>  musb-hdrc: peripheral reset irq lost!
>>  high speed config #2: 2 mA, Ethernet Gadget, using RNDIS
>>  USB RNDIS network up!
>>  BOOTP broadcast 1
>>  BOOTP broadcast 2
>>  BOOTP broadcast 3
>>  DHCP client bound to address 192.168.200.4 (757 ms)
>>  data abort
>>  pc : [<9fe9b0a2>]  lr : [<9febbc3f>]
>>  reloc pc : [<808130a2>]    lr : [<80833c3f>]
>>  sp : 9de53410  ip : 9de53578 fp : 0001
>>  r10: 9de5345c  r9 : 9de67e80 r8 : 9febbae5
>>  r7 : 9de72c30  r6 : 9feec710 r5 : 000d  r4 : 0018
>>  r3 : 3fdd8e04  r2 : 0002 r1 : 9feec728  r0 : 9feec700
>>  Flags: Nzcv  IRQs off  FIQs on  Mode SVC_32 (T)
>>  Code: f023 0303 60ca 4403 (6091) 685a
>>  Resetting CPU ...
>> 
>>  resetting ...
>> 
>> 
>>  It's there has any doc about how to debug data abort? Or is the bug
>>  is already fixed?
>> 
>>  Thanks
>>   
>> >>> This bug doesn't fixed on master code. I found v2021.01 is good and
>> >>> v2021.04-rc2 is bad.
>> >>>
>> >>> Also I had tested this on beaglebone black with am335x_evm_defconfig,
>> >>> has the simliar problem.
>> >>>
>> >>> find the first bug commit via 'git bisect': it told me that commit
>> >>> e97eb638de0dc8f6e989e20eaeb0342f103cb917 broke it. But it is very
>> >>> strange due to this commit doesn't touch any dhcp or network code.
>> >>>
>> >>> ➜  u-boot-main git:(e97eb638de) ✗ git bisect bug
>> >>> e97eb638de0dc8f6e989e20eaeb0342f103cb917 is the first bug commit
>> >>> commit e97eb638de0dc8f6e989e20eaeb0342f103cb917
>> >>> Author: Heinrich Schuchardt 
>> >>> Date:   Wed Jan 20 22:21:53 2021 +0100
>> >>>
>> >>>     fs: fat: consistent error handling for flush_dir()
>> >>>
>> >>>     Provide function 

Re: [PATCH v5] event: Add fpga load event

2023-07-20 Thread Tom Rini
On Thu, Jul 20, 2023 at 09:27:24AM +0200, christian.taedcke-...@weidmueller.com 
wrote:

> From: Christian Taedcke 
> 
> This enables implementing custom logic after a bitstream was loaded
> into the fpga.
> 
> Signed-off-by: Christian Taedcke 
> Reviewed-by: Simon Glass 

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature


Re: Pull request efi-2023-10-rc1-2

2023-07-20 Thread Tom Rini
On Thu, Jul 20, 2023 at 01:04:21PM +0200, Heinrich Schuchardt wrote:

> Dear Tom,
> 
> The following changes since commit 5dcfc99b2b17fa1497adea47a50bf7c7a6ba5709:
> 
>   Merge tag 'fsl-qoriq-2023-7-13' of
> https://source.denx.de/u-boot/custodians/u-boot-fsl-qoriq (2023-07-19
> 07:59:34 -0400)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-efi.git
> tags/efi-2023-10-rc1-2
> 
> for you to fetch changes up to e07368ea57d224557570a6715dcffdbc883a8079:
> 
>   efi_loader: support all uclasses in device path (2023-07-20 09:12:50
> +0200)
> 
> Gitlab showed no issues:
> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/16959
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: patch to submit

2023-07-20 Thread Fabio Estevam
Hi Mark,

On Thu, Jul 20, 2023 at 1:55 PM mark roths  wrote:
>
> Avalanche Technology has developed a new MRAM device (MTD).  We would like
> to submit a patch to u-boot including the Avalanche jedec memory identifier
> and some code to allow boot from this device.
>
> Please point me to the instructions for submitting such a patch for
> inclusion in u-boot.

Here it goes:
https://u-boot.readthedocs.io/en/latest/develop/sending_patches.html


patch to submit

2023-07-20 Thread mark roths
Avalanche Technology has developed a new MRAM device (MTD).  We would like
to submit a patch to u-boot including the Avalanche jedec memory identifier
and some code to allow boot from this device.

Please point me to the instructions for submitting such a patch for
inclusion in u-boot.

Mark Roths


Re: fdt_high default value causes unbootable kernel due to misalignment

2023-07-20 Thread Tim van der Staaij | Zign
>I wonder if we should adjust mkimage to use -B 8 as the default?

Ah, overlooked this option. I ran a test with -B 8 and by itself it doesn't fix 
the issue. However, as the manpage notes, -B is only effective when -E is also 
used. And indeed, using -E -B 8 does avoid misalignment when the device tree is 
used in-place.

Sounds like a good fix to me, but I don't know if there would be broader 
consequences of defaulting to -E? It sounds like this changes the structure of 
the fitimage rather dramatically.

Kind regards,
Tim


Re: data abort when run 'dhcp'

2023-07-20 Thread Miquel Raynal
Hello,

qianfangui...@163.com wrote on Fri, 25 Mar 2022 18:04:46 +0800:

> It's very strange. And I can't detect it's a bug of usb or dlmalloc.
> 
> 1. Starting u-boot and dhcp via am335x's ethernet(cpsw driver), it's ok.
> 
> 2. Starting u-boot and dhcp via am335x's usb net, data abort.
> 
> 3. start fastboot, and CTRL C right now, dhcp via am335x's usb net, it's ok.

I am sorry to re-open a thread that is one year old but this is
still an open bug. The BBB is affected. In particular the BBBW
because there is no Ethernet connector, which makes the Eth-over-USB
emulation even more important. All U-Boots since 2021 are affected:
spurious data aborts, usually at the end of network interactions (tftp,
ping). I could not bisect it because the boot was deeply broken as
well on a significant range of commits :-/.

On my side I narrowed it down to an env update which fails in malloc as
well. If I comment the env update, it fails a bit later. It really
looks like a stack corruption which is either related to the Ethernet
USB gadget or the USB controller driver itself. Network transfers on
the BBBW using regular Ethernet does not trigger any error.

I also observe the very strange "fix" mentioned above: starting and
killing fastboot makes all tftp pass... If anyone has more details to
share, or perhaps a subsequent thread giving more details, I would
really like to see this fixed upstream, I suppose I am not the only one
:-)

Thanks,
Miquèl

> 
> 在 2022/3/24 17:33, qianfan 写道:
> >
> > 在 2022/3/23 18:12, Heinrich Schuchardt 写道:  
> >> On 3/23/22 11:07, qianfan wrote:  
> >>>
> >>> 在 2022/3/23 17:51, Heinrich Schuchardt 写道:  
>  On 3/23/22 10:13, qianfan wrote:  
> >
> > 在 2022/3/23 16:02, qianfan 写道:  
> >>
> >>
> >> 在 2022/3/23 15:45, qianfan 写道:  
> >>>
> >>>
> >>> 在 2022/3/23 10:28, qianfan 写道:  
> 
>  Hi:
> 
>  I had a custom AM335X board connected my computer by usbnet. It
>  always report data abort when 'dhcp':
> 
>  Next it the log:
> 
>  U-Boot 2022.01-rc1-00183-gfa5b4e2d19-dirty (Feb 25 2022 - 15:45:02
>  +0800)
> 
>  CPU  : AM335X-GP rev 2.1
>  Model: WISDOM AM335X CCT
>  DRAM:  512 MiB
>  NAND:  256 MiB
>  MMC:   OMAP SD/MMC: 0
>  Loading Environment from NAND... *** Warning - bad CRC, using
>  default environment
> 
>  Net:   Could not get PHY for ethernet@4a10: addr 0
>  eth2: ethernet@4a10, eth3: usb_ether
>  Hit any key to stop autoboot:  0  
>  => setenv autoload no
>  => dhcp  
>  using musb-hdrc, OUT ep1out IN ep1in STATUS ep2in
>  MAC de:ad:be:ef:00:01
>  HOST MAC de:ad:be:ef:00:00
>  RNDIS ready
>  musb-hdrc: peripheral reset irq lost!
>  high speed config #2: 2 mA, Ethernet Gadget, using RNDIS
>  USB RNDIS network up!
>  BOOTP broadcast 1
>  BOOTP broadcast 2
>  BOOTP broadcast 3
>  DHCP client bound to address 192.168.200.4 (757 ms)
>  data abort
>  pc : [<9fe9b0a2>]  lr : [<9febbc3f>]
>  reloc pc : [<808130a2>]    lr : [<80833c3f>]
>  sp : 9de53410  ip : 9de53578 fp : 0001
>  r10: 9de5345c  r9 : 9de67e80 r8 : 9febbae5
>  r7 : 9de72c30  r6 : 9feec710 r5 : 000d  r4 : 0018
>  r3 : 3fdd8e04  r2 : 0002 r1 : 9feec728  r0 : 9feec700
>  Flags: Nzcv  IRQs off  FIQs on  Mode SVC_32 (T)
>  Code: f023 0303 60ca 4403 (6091) 685a
>  Resetting CPU ...
> 
>  resetting ...
> 
> 
>  It's there has any doc about how to debug data abort? Or is the bug
>  is already fixed?
> 
>  Thanks
>   
> >>> This bug doesn't fixed on master code. I found v2021.01 is good and
> >>> v2021.04-rc2 is bad.
> >>>
> >>> Also I had tested this on beaglebone black with am335x_evm_defconfig,
> >>> has the simliar problem.
> >>>
> >>> find the first bug commit via 'git bisect': it told me that commit
> >>> e97eb638de0dc8f6e989e20eaeb0342f103cb917 broke it. But it is very
> >>> strange due to this commit doesn't touch any dhcp or network code.
> >>>
> >>> ➜  u-boot-main git:(e97eb638de) ✗ git bisect bug
> >>> e97eb638de0dc8f6e989e20eaeb0342f103cb917 is the first bug commit
> >>> commit e97eb638de0dc8f6e989e20eaeb0342f103cb917
> >>> Author: Heinrich Schuchardt 
> >>> Date:   Wed Jan 20 22:21:53 2021 +0100
> >>>
> >>>     fs: fat: consistent error handling for flush_dir()
> >>>
> >>>     Provide function description for flush_dir().
> >>>     Move all error messages for flush_dir() from the callers to the
> >>> function.
> >>>     Move mapping of errors to -EIO to the function.
> >>>     Always check return value of flush_dir() (Coverity CID 

Re: [PATCH] Makefile: Use sort shortopts

2023-07-20 Thread Milan P . Stanić
On Thu, 2023-07-20 at 14:50, Marek Vasut wrote:
> POSIX does not defined longopts for sort, use shortops
> for even more compatibility.
> 
> Fixes: cc5a490cf465 ("Makefile: Sort u-boot-initial-env output")
> Reported-by: Milan P. Stanić 
> Signed-off-by: Marek Vasut 

Tested-by: Milan P. Stanić 

small note: single quotes are not needed aroud = (equal sign)

> ---
> Cc: Christoph Niedermaier 
> Cc: Marek Behún 
> Cc: Simon Glass 
> Cc: Stefano Babic 
> Cc: u-b...@dh-electronics.com
> Cc: u-boot@lists.denx.de
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 87f9fc786e8..5fc16b3b1f1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2444,7 +2444,7 @@ quiet_cmd_genenv = GENENV  $@
>  cmd_genenv = \
>   $(objtree)/tools/printinitialenv | \
>   sed -e '/^\s*$$/d' | \
> - sort --field-separator='=' -k1,1 --stable -o $@
> + sort -t '=' -k 1,1 -s -o $@
>  
>  u-boot-initial-env: $(env_h) FORCE
>   $(Q)$(MAKE) $(build)=tools $(objtree)/tools/printinitialenv
> -- 
> 2.40.1
> 


Re: [PATCH 1/4] pinctrl: Create a select_state variant with the ofnode

2023-07-20 Thread Roger Quadros



On 20/07/2023 12:55, Maxime Ripard wrote:
> Some drivers might not follow entirely the driver/device association,
> and thus might support what should be multiple devices in a single
> driver.
> 
> Such a driver is am65-cpsw-nuss, where the MAC and MDIO controllers are
> different device tree nodes, with their own resources (including pinctrl
> pins) but supported by a single driver tied to the MAC device in U-Boot.
> 
> In order to get the proper pinctrl state, we would need to get the
> state from the MDIO device tree node, but tie it to the MAC device since
> it's the only thing we have access to.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/pinctrl/pinctrl-uclass.c | 15 +--
>  include/dm/pinctrl.h | 26 --
>  2 files changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-uclass.c 
> b/drivers/pinctrl/pinctrl-uclass.c
> index 73dd7b1038bb..9e28f1858cbd 100644
> --- a/drivers/pinctrl/pinctrl-uclass.c
> +++ b/drivers/pinctrl/pinctrl-uclass.c
> @@ -53,7 +53,8 @@ static int pinctrl_config_one(struct udevice *config)
>   * @statename: state name, like "default"
>   * @return: 0 on success, or negative error code on failure
>   */
> -static int pinctrl_select_state_full(struct udevice *dev, const char 
> *statename)
> +static int pinctrl_select_state_full(struct udevice *dev, ofnode node,
> +  const char *statename)
>  {
>   char propname[32]; /* long enough */
>   const fdt32_t *list;
> @@ -61,7 +62,7 @@ static int pinctrl_select_state_full(struct udevice *dev, 
> const char *statename)
>   struct udevice *config;
>   int state, size, i, ret;
>  
> - state = dev_read_stringlist_search(dev, "pinctrl-names", statename);
> + state = ofnode_stringlist_search(node, "pinctrl-names", statename);
>   if (state < 0) {
>   char *end;
>   /*
> @@ -74,7 +75,7 @@ static int pinctrl_select_state_full(struct udevice *dev, 
> const char *statename)
>   }
>  
>   snprintf(propname, sizeof(propname), "pinctrl-%d", state);
> - list = dev_read_prop(dev, propname, );
> + list = ofnode_get_property(node, propname, );
>   if (!list)
>   return -ENOSYS;
>  
> @@ -293,20 +294,22 @@ static int pinctrl_select_state_simple(struct udevice 
> *dev)
>   return ops->set_state_simple(pctldev, dev);
>  }
>  
> -int pinctrl_select_state(struct udevice *dev, const char *statename)
> +int pinctrl_select_state_by_ofnode(struct udevice *dev, ofnode node,
> +const char *statename)
>  {
>   /*
>* Some device which is logical like mmc.blk, do not have
>* a valid ofnode.
>*/
> - if (!dev_has_ofnode(dev))
> + if (!ofnode_valid(node))
>   return 0;
> +
>   /*
>* Try full-implemented pinctrl first.
>* If it fails or is not implemented, try simple one.
>*/
>   if (CONFIG_IS_ENABLED(PINCTRL_FULL))
> - return pinctrl_select_state_full(dev, statename);
> + return pinctrl_select_state_full(dev, node, statename);
>  
>   return pinctrl_select_state_simple(dev);
>  }
> diff --git a/include/dm/pinctrl.h b/include/dm/pinctrl.h
> index e3e50afeaff0..be4679b7f20d 100644
> --- a/include/dm/pinctrl.h
> +++ b/include/dm/pinctrl.h
> @@ -502,6 +502,24 @@ static inline int pinctrl_generic_set_state(struct 
> udevice *pctldev,
>  #endif
>  
>  #if CONFIG_IS_ENABLED(PINCTRL)
> +/**
> + * pinctrl_select_state_by_ofnode() - Set a device to a given state using 
> the given ofnode
> + * @dev: Peripheral device
> + * @ofnode:  Device Tree node to get the state from
> + * @statename:   State name, like "default"
> + *
> + * Return: 0 on success, or negative error code on failure
> + */
> +int pinctrl_select_state_by_ofnode(struct udevice *dev, ofnode node, const 
> char *statename);
> +#else
> +static inline int pinctrl_select_state_by_ofnode(struct udevice *dev,
> +  ofnode node,
> +  const char *statename)
> +{
> + return -ENOSYS;
> +}

This allows & encourages one device driver to change another devices pinctrl 
state
which doesn't look like a good idea to me.

Please see if an alternative solution pointed out in patch2 works
so we don't have to need this patch.

> +#endif
> +
>  /**
>   * pinctrl_select_state() - Set a device to a given state
>   * @dev: Peripheral device
> @@ -509,14 +527,10 @@ static inline int pinctrl_generic_set_state(struct 
> udevice *pctldev,
>   *
>   * Return: 0 on success, or negative error code on failure
>   */
> -int pinctrl_select_state(struct udevice *dev, const char *statename);
> -#else
> -static inline int pinctrl_select_state(struct udevice *dev,
> -const char *statename)
> +static inline int pinctrl_select_state(struct udevice *dev, const char 
> *statename)

Re: [PATCH 2/4] net: ti: am65-cpsw-nuss: Enforce pinctrl state on the MDIO child node

2023-07-20 Thread Roger Quadros



On 20/07/2023 12:55, Maxime Ripard wrote:
> The binding represents the MDIO controller as a child device tree
> node of the MAC device tree node.
> 
> The U-Boot driver mostly ignores that child device tree node and just
> hardcodes the resources it uses to support both the MAC and MDIO in a
> single driver.
> 
> However, some resources like pinctrl muxing states are thus ignored.
> This has been a problem with device trees since Linux 6.5-rc1 which will
> put some pinctrl states on the MDIO device tree node.
> 
> Let's rework the driver a bit to fetch the pinctrl state from the MDIO
> node and apply it.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/net/ti/am65-cpsw-nuss.c | 49 
> +
>  1 file changed, 49 insertions(+)
> 
> diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c
> index f674b0baa359..2d579bf4af2c 100644
> --- a/drivers/net/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ti/am65-cpsw-nuss.c
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -696,6 +697,50 @@ out:
>   return ret;
>  }
>  
> +static ofnode am65_cpsw_find_mdio(ofnode parent)
> +{
> + ofnode node;
> +
> + ofnode_for_each_subnode(node, parent)
> + if (ofnode_device_is_compatible(node, "ti,cpsw-mdio"))
> + return node;
> +
> + return ofnode_null();
> +}
> +
> +static int am65_cpsw_setup_mdio(struct udevice *dev)
> +{
> + ofnode mdio;
> + int ret;
> +
> + mdio = am65_cpsw_find_mdio(dev_ofnode(dev));
> + if (!ofnode_valid(mdio))
> + return -ENODEV;
> +
> + /*
> +  * The MDIO controller is represented in the DT binding by a
> +  * subnode of the MAC controller.
> +  *
> +  * Our driver largely ignores that and supports MDIO by
> +  * hardcoding the register offsets.
> +  *
> +  * However, some resources (clocks, pinctrl) might be tied to
> +  * the MDIO node, and thus ignored.
> +  *
> +  * Clocks are not a concern at the moment since it's shared
> +  * between the MAC and MDIO, so the driver handles both at the
> +  * same time.
> +  *
> +  * However, we do need to make sure the pins states tied to the
> +  * MDIO node are configured properly.
> +  */
Please mention this as a HACK: that needs to go away when davinci_mdio
and this driver properly supports MDIO infrastructure.

> + ret = pinctrl_select_state_by_ofnode(dev, mdio, "default");

Instead of modifying the API, can we have a dummy driver for the 
"ti,davinci_mdio"
device that registers as class UCLASS_MDIO but does nothing else?

Then you can drop patch 1 and instead do

ret = uclass_get_device_by_ofnode(UCLASS_MDIO, node, _dev);
if (!ret)
pinctrl_select_state(mdio_dev, "default");


> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
>  static int am65_cpsw_probe_nuss(struct udevice *dev)
>  {
>   struct am65_cpsw_common *cpsw_common = dev_get_priv(dev);
> @@ -728,6 +773,10 @@ static int am65_cpsw_probe_nuss(struct udevice *dev)
>   AM65_CPSW_CPSW_NU_ALE_BASE;
>   cpsw_common->mdio_base = cpsw_common->ss_base + AM65_CPSW_MDIO_BASE;
>  
> + ret = am65_cpsw_setup_mdio(dev);
> + if (ret)
> + goto out;
> +
>   ports_np = dev_read_subnode(dev, "ethernet-ports");
>   if (!ofnode_valid(ports_np)) {
>   ret = -ENOENT;
> 

-- 
cheers,
-roger


Re: [PATCH 4/4] fixup! arm: dts: k3-am62: Bump dtsi from linux v6.5-rc1

2023-07-20 Thread Roger Quadros



On 20/07/2023 12:55, Maxime Ripard wrote:
> Dropping ranges entirely doesn't work since the register offset on the
> MDIO device node will now be completely off, so we need to adjust it to
> the right value without the translation.
> 
> We also need to have an empty ranges property for the reg address to be
> properly evaluated.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  arch/arm/dts/k3-am625-sk-u-boot.dtsi | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi 
> b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> index fe1c7408a89d..2ec3fff99f32 100644
> --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> @@ -122,8 +122,8 @@
>   reg = <0x0 0x800 0x0 0x20>,
> <0x0 0x43000200 0x0 0x8>;
>   reg-names = "cpsw_nuss", "mac_efuse";
> - /delete-property/ ranges;
>   bootph-pre-ram;
> + ranges;
>  
>   cpsw-phy-sel@04044 {
>   compatible = "ti,am64-phy-gmii-sel";
> @@ -132,6 +132,10 @@
>   };
>  };
>  
> +_mdio {
> + reg = <0x0 0x8000f00 0x0 0x100>;
> +};
> +

Why this change?

Linux DT has
cpsw3g_mdio: mdio@f00 {
compatible = "ti,cpsw-mdio","ti,davinci_mdio";
reg = <0x00 0xf00 0x00 0x100>;

And it is a child of cpsw3g node.

>  _port1 {
>   bootph-pre-ram;
>  };
> 

-- 
cheers,
-roger


Re: [RFC PATCH 4/4] arm: dts: k3-am62: Bump dtsi from linux v6.5-rc1

2023-07-20 Thread Roger Quadros



On 13/07/2023 10:20, Nishanth Menon wrote:
> Update the am62 and am625 device-trees from linux v6.3-rc5 This needed the 
> followin
> tweaks to the u-boot specific dtsi as well:
> - Switch tick-timer to the main_timer as it's now defined in the main dtsi
> - Secure proxies are defined in Soc dtsis
> - Drop duplicate nodes - u-boot.dtsi is includes in r5-sk, no need for
>   either the definitions from main.dtsi OR duplication from u-boot.dtsi
> - Add mdio pins to the cpsw3g pinctrl in u-boot dtsi. It moved to a subnode 
> in the
>   linux dtsi that u-boot doesn't use/support
> 
> Cc: Francesco Dolcini 
> Cc: Sjoerd Simons 
> Cc: Wadim Egorov 
> Signed-off-by: Nishanth Menon 
> ---
> 
> This is a respin of Sjoerd's series.. quiet a bit of sync there. but
> should make it easier to add on newer boards by backporting from
> upstream kernel.
> 
>  arch/arm/dts/k3-am62-main.dtsi   | 389 +++--
>  arch/arm/dts/k3-am62-mcu.dtsi|  66 +
>  arch/arm/dts/k3-am62-thermal.dtsi|  33 +++
>  arch/arm/dts/k3-am62-wakeup.dtsi |  33 ++-
>  arch/arm/dts/k3-am62.dtsi|  11 +-
>  arch/arm/dts/k3-am625-r5-sk.dts  |  92 +-
>  arch/arm/dts/k3-am625-sk-u-boot.dtsi |  24 +-
>  arch/arm/dts/k3-am625-sk.dts | 286 ++-
>  arch/arm/dts/k3-am625.dtsi   |  54 +++-
>  arch/arm/dts/k3-am62x-sk-common.dtsi | 412 +++
>  arch/arm/dts/k3-pinctrl.h|  53 
>  11 files changed, 1065 insertions(+), 388 deletions(-)
>  create mode 100644 arch/arm/dts/k3-am62-thermal.dtsi
>  create mode 100644 arch/arm/dts/k3-am62x-sk-common.dtsi
>  create mode 100644 arch/arm/dts/k3-pinctrl.h
> 



>  
> diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi 
> b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> index a60c37f1dbf0..76589c7025a0 100644
> --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> @@ -9,7 +9,7 @@
>  / {
>   chosen {
>   stdout-path = "serial2:115200n8";
> - tick-timer = 
> + tick-timer = _timer0;
>   };
>  
>   aliases {
> @@ -21,16 +21,13 @@
>   };
>  };
>  
> -_main{
> +_main {
>   bootph-pre-ram;
> +};
>  
> - timer1: timer@240 {
> - compatible = "ti,omap5430-timer";
> - reg = <0x00 0x240 0x00 0x80>;
> - ti,timer-alwon;
> - clock-frequency = <2500>;
> - bootph-pre-ram;
> - };
> +_timer0 {
> + clock-frequency = <2500>;
> + bootph-pre-ram;
>  };
>  
>   {
> @@ -77,10 +74,6 @@
>   bootph-pre-ram;
>  };
>  
> -_uart1 {
> - bootph-pre-ram;
> -};
> -
>  _mcu {
>   bootph-pre-ram;
>  };
> @@ -93,10 +86,6 @@
>   bootph-pre-ram;
>  };
>  
> -_uart0 {
> - bootph-pre-ram;
> -};
> -
>   {
>   bootph-pre-ram;
>  };
> @@ -135,6 +124,7 @@
>   reg-names = "cpsw_nuss", "mac_efuse";

mac_efuse is another odd duck.

The efuse offset needs to be obtained from
"ti,syscon-efuse = <_conf 0x200>;"
which is already present in am62-main.dtsi

>   /delete-property/ ranges;

Any idea why we delete ranges?

>   bootph-pre-ram;
> + pinctrl-0 = <_mdio1_pins_default _rgmii1_pins_default>;
>  
>   cpsw-phy-sel@04044 {
>   compatible = "ti,am64-phy-gmii-sel";

This can also go away.
Only bootph-pre-ram required.

-- 
cheers,
-roger


Re: [PATCH 2/2] schemas: Add a schema for binman

2023-07-20 Thread Alper Nebi Yasak
Hi Simon,

I know I haven't been able to look at binman stuff for a long time, but
I've been occasionally thinking about it through the course of a year
and think binman severely needs a design-wise review before things get
entrenched, even in the most fundamental parts. I do see the
cross-project collaboration intention here, but still...

There's also the issue of binman having multiple different device-trees:
its input, the ones in fdtmaps per image, the ones injected to U-Boot
dtb files per image. I'd say each has different needs, and those
differences have to be figured out before specified upstream. I can only
guess this is about the one that'll be in a u-boot.dtb.

I want to go through binman more thoroughly, but a lot of changes
happened since I last looked at it and I'm a bit slow at writing things,
so won't exactly be soon. That being said, here's some ideas off the top
of my head, for inspiration on both this schema and binman itself.

On 2023-07-12 00:18 +03:00, Simon Glass wrote:
> I am unsure whether to add this with a generic name, such as 'layout',
> but for now am using /firmware/binman to avoid conflicts with any other
> firmware-layout schema that others might be working on.
> 
> Signed-off-by: Simon Glass 
> ---

I've been thinking of compatible = "data," for entries, so
proposing 'data' here. A big ask, but it might be the one schema to
unify them all if we look at things as "description of data" instead of
"firmware layout".

Also consider data layouts in-memory. For example it could be an
alternative to /chosen linux,initrd-start to specify initramfs location.
Or things like keys or logs received from previous stages / other parts?
Weak examples, but maybe might connect better into firmware handoff
things. (Sorry, I don't know much about those.)

I'm also thinking about things like JPEG/BMP files for ACPI BGRT-like
boot splash, unique firmware/configuration/calibration data for drivers,
but they don't exactly need to be in-memory I guess.

> 
>  dtschema/schemas/firmware/binman.yaml   | 51 ++
>  dtschema/schemas/firmware/binman/entry.yaml | 57 +
>  2 files changed, 108 insertions(+)
>  create mode 100644 dtschema/schemas/firmware/binman.yaml
>  create mode 100644 dtschema/schemas/firmware/binman/entry.yaml
> 
> diff --git a/dtschema/schemas/firmware/binman.yaml 
> b/dtschema/schemas/firmware/binman.yaml
> new file mode 100644
> index 000..4b1ecf6
> --- /dev/null
> +++ b/dtschema/schemas/firmware/binman.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Google LLC
> +
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/binman.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Binman firmware layout
> +
> +maintainers:
> +  - Simon Glass 
> +
> +description: |
> +  The binman node provides a layout for firmware, used when packaging 
> firmware
> +  from multiple projects. For now it just supports a very simple set of
> +  features, as a starting point for discussion.
> +
> +  Documentation for Binman is available at:
> +
> +  https://u-boot.readthedocs.io/en/latest/develop/package/binman.html
> +
> +  with the current image-description format at:
> +
> +  
> https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#image-description-format
> +
> +properties:
> +  $nodename:
> +const: binman

I think having a single /firmware/binman node for a single image *and*
having a "multiple-images" mode under it is quite bad because the
single-image mode is multiple-images with one image. Multiple images
should be the only mode of operation

Furthermore, each image should be able to function independently of
others, so I think to help enforce that maybe there shouldn't be any
formal binman node. Merely instances of images that may happen to be
under one parent node that may happen to be called /firmware/binman. The
images can be compatible = "data,image" or something and we could search
by that.

Or, heh, a single image node that happens to be named /firmware/binman
because we are embedding it into a u-boot.dtb in that image. But perhaps
we shouldn't restrain things like that. A bit contrived, but why should
we be unable to start from an image in SPI and make it load something
from microSD?

Going even further, let me suggest putting the image nodes as children
of the devices whose layout the image describes. Then I imagine we can
have something like a "data driver" (both for U-Boot and Linux) that
reads the entry data from the parent data/block device and makes it
available for other drivers or in sysfs.

> +
> +required:
> +  - compatible
> +
> +additionalProperties: false

Including the binman version or a version number for the spec might be
useful later on.

> +
> +examples:
> +  - |
> +  - |
> +firmware {
> +  binman {
> +compatible = "u-boot,binman";

Consider including "image" or "namespace" in the compatible string to

Re: [PATCH 0/4] net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl

2023-07-20 Thread Nishanth Menon
On 17:41-20230720, Roger Quadros wrote:
[...]
> >> from what I understand, is around 2024 timeframe.. which is probably not
> >> something that helps us in the community at this point.
> 
> OK, then we need to resort to some temporary solution like this then.

Thanks. I will squash up the fixup patches, lets try and get the rest
of the patches reviewed? I can put out a new RFC based on this series
(or updates).

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D


[PATCH v4 6/6] sysinfo: rcar3: Implement BOARD_ID and BOARD_REV_*

2023-07-20 Thread Detlev Casanova
Expose that information to the sysinfo command to let scripts make
decisions based on the board id and revision.

Signed-off-by: Detlev Casanova 
---
 drivers/sysinfo/rcar3.c | 103 +++-
 1 file changed, 70 insertions(+), 33 deletions(-)

diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c
index 450a4c26773..1a99642bf75 100644
--- a/drivers/sysinfo/rcar3.c
+++ b/drivers/sysinfo/rcar3.c
@@ -32,6 +32,10 @@
  */
 struct sysinfo_rcar_priv {
charboardmodel[64];
+   u8  id;
+   u8  rev_major;
+   u8  rev_minor;
+   boolhas_rev;
u8  val;
 };
 
@@ -56,9 +60,33 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, 
size_t size, char *
};
 }
 
+static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val)
+{
+   struct sysinfo_rcar_priv *priv = dev_get_priv(dev);
+
+   switch (id) {
+   case SYSINFO_ID_BOARD_ID:
+   *val = priv->id;
+   return 0;
+   case SYSINFO_ID_BOARD_REVISION_MAJOR:
+   if (!priv->has_rev)
+   return -EINVAL;
+   *val = priv->rev_major;
+   return 0;
+   case SYSINFO_ID_BOARD_REVISION_MINOR:
+   if (!priv->has_rev)
+   return -EINVAL;
+   *val = priv->rev_minor;
+   return 0;
+   default:
+   return -EINVAL;
+   };
+}
+
 static const struct sysinfo_ops sysinfo_rcar_ops = {
.detect = sysinfo_rcar_detect,
.get_str = sysinfo_rcar_get_str,
+   .get_int = sysinfo_rcar_get_int,
 };
 
 static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv)
@@ -70,8 +98,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv)
bool condor_i = false;
char model[64];
char rev[4] = "?.?";
-   u8 rev_major = 0;
-   u8 rev_minor = 0;
+
+   priv->id = board_id;
+   priv->has_rev = false;
 
switch (board_id) {
case BOARD_SALVATOR_XS:
@@ -81,9 +110,10 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv 
*priv)
snprintf(model, sizeof(model),
 "Renesas Salvator-X%s board", salvator_xs ? "S" : "");
if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */
-   rev_major = 1;
-   rev_minor = board_rev & BIT(0);
-   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
+   priv->rev_major = 1;
+   priv->rev_minor = board_rev & BIT(0);
+   priv->has_rev = true;
+   snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, 
priv->rev_minor);
}
 
snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
@@ -91,34 +121,37 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv 
*priv)
 
return;
case BOARD_STARTER_KIT:
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+   snprintf(model, sizeof(model),
 "Renesas Starter Kit board");
if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */
-   rev_major = (board_rev & BIT(0)) ? 3 : 1;
-   rev_minor = 0;
-   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
+   priv->rev_major = (board_rev & BIT(0)) ? 3 : 1;
+   priv->rev_minor = 0;
+   snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, 
priv->rev_minor);
+   priv->has_rev = true;
}
snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
 model, rev);
return;
case BOARD_STARTER_KIT_PRE:
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+   snprintf(model, sizeof(model),
 "Renesas Starter Kit Premier board");
if (!(board_rev & ~3)) { /* Only rev 0..3 is valid */
-   rev_major = (board_rev & BIT(1)) ? 2 : 1;
-   rev_minor = (board_rev == 3) ? 1 : 0;
-   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
+   priv->rev_major = (board_rev & BIT(1)) ? 2 : 1;
+   priv->rev_minor = (board_rev == 3) ? 1 : 0;
+   snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, 
priv->rev_minor);
+   priv->has_rev = true;
}
snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
 model, rev);
return;
case BOARD_EAGLE:
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+   snprintf(model, sizeof(model),
 "Renesas Eagle board");

[PATCH v4 5/6] sysinfo: rcar3: Use int instead of char for revision

2023-07-20 Thread Detlev Casanova
To be used with the sysinfo command, revision values must be considered
as integers, not chars as some boards will implement BOARD_REVISION_*
and might use numbers greater than 9.

Signed-off-by: Detlev Casanova 
---
 drivers/sysinfo/rcar3.c | 104 
 1 file changed, 62 insertions(+), 42 deletions(-)

diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c
index 7b127986da7..450a4c26773 100644
--- a/drivers/sysinfo/rcar3.c
+++ b/drivers/sysinfo/rcar3.c
@@ -68,90 +68,110 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv 
*priv)
bool salvator_xs = false;
bool ebisu_4d = false;
bool condor_i = false;
-   char rev_major = '?';
-   char rev_minor = '?';
+   char model[64];
+   char rev[4] = "?.?";
+   u8 rev_major = 0;
+   u8 rev_minor = 0;
 
switch (board_id) {
case BOARD_SALVATOR_XS:
salvator_xs = true;
fallthrough;
case BOARD_SALVATOR_X:
+   snprintf(model, sizeof(model),
+"Renesas Salvator-X%s board", salvator_xs ? "S" : "");
if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */
-   rev_major = '1';
-   rev_minor = '0' + (board_rev & BIT(0));
+   rev_major = 1;
+   rev_minor = board_rev & BIT(0);
+   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
}
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
-"Renesas Salvator-X%s board rev %c.%c",
-salvator_xs ? "S" : "", rev_major, rev_minor);
+
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
+model, rev);
+
return;
case BOARD_STARTER_KIT:
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+"Renesas Starter Kit board");
if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */
-   rev_major = (board_rev & BIT(0)) ? '3' : '1';
-   rev_minor = '0';
+   rev_major = (board_rev & BIT(0)) ? 3 : 1;
+   rev_minor = 0;
+   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
}
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
-"Renesas Starter Kit board rev %c.%c",
-rev_major, rev_minor);
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
+model, rev);
return;
case BOARD_STARTER_KIT_PRE:
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+"Renesas Starter Kit Premier board");
if (!(board_rev & ~3)) { /* Only rev 0..3 is valid */
-   rev_major = (board_rev & BIT(1)) ? '2' : '1';
-   rev_minor = (board_rev == 3) ? '1' : '0';
+   rev_major = (board_rev & BIT(1)) ? 2 : 1;
+   rev_minor = (board_rev == 3) ? 1 : 0;
+   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
}
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
-"Renesas Starter Kit Premier board rev %c.%c",
-rev_major, rev_minor);
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
+model, rev);
return;
case BOARD_EAGLE:
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+"Renesas Eagle board");
if (!board_rev) { /* Only rev 0 is valid */
-   rev_major = '1';
-   rev_minor = '0';
+   rev_major = 1;
+   rev_minor = 0;
+   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
}
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
-"Renesas Eagle board rev %c.%c",
-rev_major, rev_minor);
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
+model, rev);
return;
case BOARD_EBISU_4D:
ebisu_4d = true;
fallthrough;
case BOARD_EBISU:
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+"Renesas Ebisu%s board", ebisu_4d ? "-4D" : "");
if (!board_rev) { /* Only rev 0 is valid */
-   rev_major = '1';
-   rev_minor = '0';
+   rev_major = 1;
+   rev_minor = 0;
+   snprintf(rev, sizeof(rev), 

[PATCH v4 4/6] sysinfo: Add documentation

2023-07-20 Thread Detlev Casanova
Add documentation for the sysinfo command with examples.

Reviewed-by: Marek Vasut 
Signed-off-by: Detlev Casanova 
---
 doc/usage/cmd/sysinfo.rst | 56 +++
 1 file changed, 56 insertions(+)
 create mode 100644 doc/usage/cmd/sysinfo.rst

diff --git a/doc/usage/cmd/sysinfo.rst b/doc/usage/cmd/sysinfo.rst
new file mode 100644
index 000..1660b2aa1a6
--- /dev/null
+++ b/doc/usage/cmd/sysinfo.rst
@@ -0,0 +1,56 @@
+.. SPDX-License-Identifier: GPL-2.0+:
+
+sysinfo command
+===
+
+Synopis
+---
+
+::
+
+sysinfo id 
+sysinfo model 
+sysinfo revision 
+
+Description
+---
+
+The `sysinfo` command is used to show information about the running system
+
+The `sysinfo id` command prints or sets an environment variable to the board id
+as an hex value.
+
+varname
+an optional environment variable to store the board id into.
+
+The `sysinfo model` command prints or sets an environment variable to the board
+model name as a string value.
+
+varname
+an optional environment variable to store the board model name into.
+
+The `sysinfo revision` command prints or sets an environment variable to the
+board revision in the . format, where MINOR and MINOR are int
+values.
+
+varname
+an optional environment variable to store the board revision into.
+
+Examples
+
+
+::
+
+=> sysinfo id
+0x0b
+=> sysinfo model
+Renesas Starter Kit Premier board rev 2.1
+=> sysinfo revision varname
+=> env print varname
+2.1
+
+Return value
+
+
+The return value $? is set to 0 (true) if the command succeeded. If an
+error occurs, the return value $? is set to 1 (false).
-- 
2.41.0



[PATCH v4 3/6] sysinfo: Add a test

2023-07-20 Thread Detlev Casanova
The test runs one of each subcommand and checks that the output matches
the values in the sandbox sysinfo driver.

Reviewed-by: Marek Vasut 
Signed-off-by: Detlev Casanova 
---
 configs/sandbox_defconfig |  1 +
 drivers/sysinfo/sandbox.c | 17 +
 test/cmd/Makefile |  1 +
 test/cmd/test_sysinfo.c   | 51 +++
 4 files changed, 70 insertions(+)
 create mode 100644 test/cmd/test_sysinfo.c

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 19cc6701e62..a51762aa3ae 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -130,6 +130,7 @@ CONFIG_CMD_EROFS=y
 CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_SQUASHFS=y
 CONFIG_CMD_MTDPARTS=y
+CONFIG_CMD_SYSINFO=y
 CONFIG_CMD_STACKPROTECTOR_TEST=y
 CONFIG_MAC_PARTITION=y
 CONFIG_AMIGA_PARTITION=y
diff --git a/drivers/sysinfo/sandbox.c b/drivers/sysinfo/sandbox.c
index d270a26aa43..cc7783907a9 100644
--- a/drivers/sysinfo/sandbox.c
+++ b/drivers/sysinfo/sandbox.c
@@ -7,9 +7,14 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "sandbox.h"
 
+#define SANDBOX_BOARD_ID   0x42
+#define SANDBOX_BOARD_REV_MAJORU_BOOT_VERSION_NUM
+#define SANDBOX_BOARD_REV_MINORU_BOOT_VERSION_NUM_PATCH
+
 struct sysinfo_sandbox_priv {
bool called_detect;
int test_i1;
@@ -48,6 +53,15 @@ int sysinfo_sandbox_get_int(struct udevice *dev, int id, int 
*val)
struct sysinfo_sandbox_priv *priv = dev_get_priv(dev);
 
switch (id) {
+   case SYSINFO_ID_BOARD_ID:
+   *val = SANDBOX_BOARD_ID;
+   return 0;
+   case SYSINFO_ID_BOARD_REV_MAJOR:
+   *val = SANDBOX_BOARD_REV_MAJOR;
+   return 0;
+   case SYSINFO_ID_BOARD_REV_MINOR:
+   *val = SANDBOX_BOARD_REV_MINOR;
+   return 0;
case INT_TEST1:
*val = priv->test_i1;
/* Increments with every call */
@@ -71,6 +85,9 @@ int sysinfo_sandbox_get_str(struct udevice *dev, int id, 
size_t size, char *val)
int index = (i1 * i2) % ARRAY_SIZE(vacation_spots);
 
switch (id) {
+   case SYSINFO_ID_BOARD_MODEL:
+   snprintf(val, size, "sandbox");
+   return 0;
case STR_VACATIONSPOT:
/* Picks a vacation spot depending on i1 and i2 */
snprintf(val, size, vacation_spots[index]);
diff --git a/test/cmd/Makefile b/test/cmd/Makefile
index a3cf983739e..d3ac5bf2d5e 100644
--- a/test/cmd/Makefile
+++ b/test/cmd/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_CMD_SEAMA) += seama.o
 ifdef CONFIG_SANDBOX
 obj-$(CONFIG_CMD_READ) += rw.o
 obj-$(CONFIG_CMD_SETEXPR) += setexpr.o
+obj-$(CONFIG_CMD_SYSINFO) += test_sysinfo.o
 endif
 obj-$(CONFIG_CMD_TEMPERATURE) += temperature.o
 obj-$(CONFIG_CMD_WGET) += wget.o
diff --git a/test/cmd/test_sysinfo.c b/test/cmd/test_sysinfo.c
new file mode 100644
index 000..7ba6dd0df89
--- /dev/null
+++ b/test/cmd/test_sysinfo.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Tests for sysinfo command
+ *
+ * Copyright 2023, Detlev Casanova 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define REV_(x, y) #x "." #y
+#define REV(x, y) REV_(x, y)
+
+struct test_data {
+   char *cmd;
+   char *expected;
+};
+
+static struct test_data sysinfo_data[] = {
+   {"sysinfo model", "sandbox"},
+   {"sysinfo id", "0x42"},
+   {"sysinfo revision", REV(U_BOOT_VERSION_NUM, U_BOOT_VERSION_NUM_PATCH)},
+};
+
+static int lib_test_sysinfo(struct unit_test_state *uts)
+{
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(sysinfo_data); ++i) {
+   ut_silence_console(uts);
+   console_record_reset_enable();
+   ut_assertok(run_command(sysinfo_data[i].cmd, 0));
+   ut_unsilence_console(uts);
+   console_record_readline(uts->actual_str,
+   sizeof(uts->actual_str));
+   ut_asserteq_str(sysinfo_data[i].expected, uts->actual_str);
+   ut_assertok(ut_check_console_end(uts));
+   }
+
+   return 0;
+}
+
+LIB_TEST(lib_test_sysinfo, 0);
-- 
2.41.0



[PATCH v4 2/6] cmd: Add a sysinfo command

2023-07-20 Thread Detlev Casanova
The command is able to show different information for the running
system:
* Model name
* Board ID
* Revision

This command can be used by boot shell scripts to select configurations
depending on the specific running system.

Reviewed-by: Marek Vasut 
Signed-off-by: Detlev Casanova 
---
 cmd/Kconfig   |   6 +++
 cmd/Makefile  |   1 +
 cmd/sysinfo.c | 133 ++
 3 files changed, 140 insertions(+)
 create mode 100644 cmd/sysinfo.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index ecfd5752377..796eedba194 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -218,6 +218,12 @@ config CMD_SBI
help
  Display information about the SBI implementation.
 
+config CMD_SYSINFO
+   bool "sysinfo"
+   depends on SYSINFO
+   help
+ Display information about the system.
+
 endmenu
 
 menu "Boot commands"
diff --git a/cmd/Makefile b/cmd/Makefile
index 9f8c0b058be..87a196b4a03 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -166,6 +166,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
 obj-$(CONFIG_CMD_STRINGS) += strings.o
 obj-$(CONFIG_CMD_SMC) += smccc.o
 obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o
+obj-$(CONFIG_CMD_SYSINFO) += sysinfo.o
 obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
 obj-$(CONFIG_CMD_TEMPERATURE) += temperature.o
 obj-$(CONFIG_CMD_TERMINAL) += terminal.o
diff --git a/cmd/sysinfo.c b/cmd/sysinfo.c
new file mode 100644
index 000..46369ff9ac7
--- /dev/null
+++ b/cmd/sysinfo.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2023
+ * Detlev Casanova 
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+static int get_sysinfo(struct udevice **devp)
+{
+   int ret = sysinfo_get(devp);
+
+   if (ret) {
+   printf("Cannot get sysinfo: %d\n", ret);
+   return ret;
+   }
+
+   ret = sysinfo_detect(*devp);
+   if (ret) {
+   printf("Cannot detect sysinfo: %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
+static int do_sysinfo_model(struct cmd_tbl *cmdtp, int flag, int argc,
+   char *const argv[])
+{
+   struct udevice *dev;
+   char model[64];
+   int ret = get_sysinfo();
+
+   if (ret)
+   return CMD_RET_FAILURE;
+
+   ret = sysinfo_get_str(dev,
+ SYSINFO_ID_BOARD_MODEL,
+ sizeof(model),
+ model);
+   if (ret) {
+   printf("Cannot get sysinfo str: %d\n", ret);
+   return CMD_RET_FAILURE;
+   }
+
+   if (argc == 2)
+   ret = env_set(argv[1], model);
+   else
+   printf("%s\n", model);
+
+   if (ret)
+   return CMD_RET_FAILURE;
+   else
+   return CMD_RET_SUCCESS;
+}
+
+static int do_sysinfo_id(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])
+{
+   struct udevice *dev;
+   u32 board_id;
+   int ret = get_sysinfo();
+
+   if (ret)
+   return CMD_RET_FAILURE;
+
+   ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_ID, _id);
+   if (ret) {
+   printf("Cannot get sysinfo int: %d\n", ret);
+   return CMD_RET_FAILURE;
+   }
+
+   if (argc == 2)
+   ret = env_set_hex(argv[1], board_id);
+   else
+   printf("0x%02x\n", board_id);
+
+   if (ret)
+   return CMD_RET_FAILURE;
+   else
+   return CMD_RET_SUCCESS;
+}
+
+static int do_sysinfo_revision(struct cmd_tbl *cmdtp, int flag, int argc,
+  char *const argv[])
+{
+   struct udevice *dev;
+   int rev_major;
+   int rev_minor;
+   char rev[64];
+   int ret = get_sysinfo();
+
+   if (ret)
+   return CMD_RET_FAILURE;
+
+   ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_REV_MAJOR, _major);
+   if (ret) {
+   printf("Cannot get sysinfo int: %d\n", ret);
+   return CMD_RET_FAILURE;
+   }
+
+   ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_REV_MINOR, _minor);
+   if (ret) {
+   printf("Cannot get sysinfo int: %d\n", ret);
+   return CMD_RET_FAILURE;
+   }
+
+   snprintf(rev, sizeof(rev), "%d.%d", rev_major, rev_minor);
+
+   if (argc == 2)
+   ret = env_set(argv[1], rev);
+   else
+   printf("%s\n", rev);
+
+   if (ret)
+   return CMD_RET_FAILURE;
+   else
+   return CMD_RET_SUCCESS;
+}
+
+static char sysinfo_help_text[] =
+   "model  - Show or set the board model in varname\n"
+   "sysinfo id - Show or set the board id in varname (in 
format 0xHH)\n"
+   "sysinfo revision   - Show or set the board revision in 
varname";
+
+U_BOOT_CMD_WITH_SUBCMDS(sysinfo, "System information", sysinfo_help_text,
+   U_BOOT_SUBCMD_MKENT(model, 2, 1, do_sysinfo_model),
+   

[PATCH v4 1/6] sysinfo: Add IDs for board id and revision

2023-07-20 Thread Detlev Casanova
These IDs will be used by the sysinfo command. The new IDs are:
 * SYSINFO_ID_BOARD_ID: The board ID as an integer
 * SYSINFO_ID_BOARD_REV_MAJOR: The board major revision as int
 * SYSINFO_ID_BOARD_REV_MINOR: The board minor revision as int

Reviewed-by: Marek Vasut 
Signed-off-by: Detlev Casanova 
---
 include/sysinfo.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/sysinfo.h b/include/sysinfo.h
index b140d742e93..13815600ae6 100644
--- a/include/sysinfo.h
+++ b/include/sysinfo.h
@@ -47,6 +47,11 @@ enum sysinfo_id {
/* For show_board_info() */
SYSINFO_ID_BOARD_MODEL,
 
+   /* For sysinfo command (all int) */
+   SYSINFO_ID_BOARD_ID,
+   SYSINFO_ID_BOARD_REV_MAJOR,
+   SYSINFO_ID_BOARD_REV_MINOR,
+
/* First value available for downstream/board used */
SYSINFO_ID_USER = 0x1000,
 };
-- 
2.41.0



[PATCH v4 0/6] Introduce the sysinfo command

2023-07-20 Thread Detlev Casanova
The command can be used to show various information that can be used to
identify the running system.

Currently supported subcommands are:
* model: A string representing the model
* id: The id of the board
* revision: The revision of this board.

Changes since v3:
 - Fix documentation typo.
Changes since v2:
 - Fix code style.
 - Use printf() instead of debug().
 - Clarify sysinfo new ids types (int).
 - Add a test for sysinfo command.
 - Add documentation for sysinfo command.
Changes since v1:
 - Removed shell function to select linux device tree. This will be
   distributions job.
 - Break revision in rev_major and rev_minor in the sysinfo driver.

Detlev Casanova (6):
  sysinfo: Add IDs for board id and revision
  cmd: Add a sysinfo command
  sysinfo: Add a test
  sysinfo: Add documentation
  sysinfo: rcar3: Use int instead of char for revision
  sysinfo: rcar3: Implement BOARD_ID and BOARD_REV_*

 cmd/Kconfig   |   6 ++
 cmd/Makefile  |   1 +
 cmd/sysinfo.c | 133 +++
 configs/sandbox_defconfig |   1 +
 doc/usage/cmd/sysinfo.rst |  56 +++
 drivers/sysinfo/rcar3.c   | 141 ++
 drivers/sysinfo/sandbox.c |  17 +
 include/sysinfo.h |   5 ++
 test/cmd/Makefile |   1 +
 test/cmd/test_sysinfo.c   |  51 ++
 10 files changed, 370 insertions(+), 42 deletions(-)
 create mode 100644 cmd/sysinfo.c
 create mode 100644 doc/usage/cmd/sysinfo.rst
 create mode 100644 test/cmd/test_sysinfo.c

-- 
2.41.0



Re: [PATCH] clk: Dont return error when assigned-clocks is empty or missing

2023-07-20 Thread Michal Simek

Hi Sean,

On 7/11/23 16:55, Michal Simek wrote:



On 7/11/23 16:28, Sean Anderson wrote:

On 7/11/23 10:20, Michal Simek wrote:

Hi Sean,

On 7/11/23 15:40, Sean Anderson wrote:

On 7/11/23 05:51, Ashok Reddy Soma wrote:

There is a chance that assigned-clock-rates is given and assigned-clocks
could be empty. Dont return error in that case, because the probe of the
corresponding driver will not be called at all if this fails.
Better to continue to look for it and return 0.


No, this is an error in the device tree. assigned-clock-rates depends on
assigned-clocks, so you must provide the latter if the former is present.


We were also checking it and in the Linux kernel it is handle like this.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-conf.c#n95

It means you can have rate assigned but not assigned-clocks property.

And yes in working case both should be present to work properly.


What is the use-case for this? It will not pass schema checking [1] anyway.

--Sean

[1] 
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/clock/clock.yaml


If you check your DT against schema. No doubt how correct behavior should be.

It is just aligning behavior with Linux kernel if user messes up DT.


I see that you are listed as clock maintainer. Are you fine with this alignment 
patch or do you want to change something or reject it?


Thanks,
Michal


Re: [PATCH 0/4] net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl

2023-07-20 Thread Roger Quadros



On 20/07/2023 17:12, Maxime Ripard wrote:
> Hi
> 
> Sorry, I didn't receive Roger mail so I'll reply here
> 
> On Thu, Jul 20, 2023 at 09:00:19AM -0500, Nishanth Menon wrote:
>> On 16:56-20230720, Roger Quadros wrote:
>>> Hi,
>>>
>>> On 20/07/2023 16:33, Ravi Gunasekaran wrote:
>>>>
>>>>
>>>> On 7/20/2023 3:25 PM, Maxime Ripard wrote:
>>>>> Hi,
>>>>>
>>>>> This series is based on:
>>>>> https://lore.kernel.org/all/20230713072019.3153871-1...@ti.com/
>>>>>
>>>>> It fixes the issue of Linux booting from the DT embedded by U-boot. The
>>>>> main issue there is that U-Boot doesn't handle the MDIO child node that
>>>>> might have resources attached to it.
>>>>>
>>>>> Thus, any pinctrl configuration that could be attached to the MDIO
>>>>> child node is effectively ignored. Unfortunately, starting with 6.5-rc1,
>>>>> Linux does just that.
>>>
>>> I didn't get this part. Linux does not ignore pinctrl configuration attached
>>> to MDIO child node. What changed in 6.5-rc1?
> 
> Linux doesn't ignore it, but Linux started putting pinctrl configuration
> on the MDIO node, which is ignored by U-Boot.

Linux always had MDIO pinctrl configuration in the MDIO node, way before 
6.5-rc1.
Let's not mention 6.5-rc1 here as it gives an indication that something in 
6.5-rc1
caused this issue. ;)

Earlier, u-boot didn't enable the MDIO node. With Nishanth's sync it does which
is fine, but duplicating the MDIO pinctrl node in the CPSW node causes problems
on Linux.

I see you reverted that change in patch 3, but probably Nishanth can avoid it 
if we
got this route.

> 
>>>>> This was solved by duplicating the pinctrl configuration onto the MAC
>>>
>>> If I remember right, there is no driver model driver for MDIO in u-boot and
>>> adding the mdio pinctrl into CPSW DT node was a hack in u-boot.
> 
> Yeah, but we now have in the U-Boot DT two nodes referencing the same
> pinctrl configuration: the CPSW and the MDIO node. Linux then tries to
> assign that configuration to both devices and it fails.

Understood.
> 
>>>>> device node. Unfortunately, this doesn't work for Linux since now it has
>>>>> two devices competing for the same pins.
>>>
>>> What has really changed here is that you are passing u-boot's device
>>> tree to Linux.
> 
> I didn't change anything. This is the default boot process if you don't
> provide a DT in the ESP partition.
> 

OK.
>>> This has nothing to do with 6.5-rc1 right?
> 
> The issue started to occur with Nishanth patch to sync with Linux
> 6.5-rc1 device trees, so there's definitely a connection.
> 
>>> I suppose your solution is still a hack but of lesser evil than
>>> duplicating MDIO pinctrl in CPSW node.
>>>
>>> The proper solution would be to implement driver model for the
>>> davinci MDIO driver. Siddharth has been working on this. If that is
>>> close to ready we should just use that instead.
>>
>> But this allows for a cleaner device tree while the driver can be fixed
>> up independently, correct? Unfortunately, Siddharth's driver model work,

Yes. MDIO pinctrl no longer needs to be duplicated in CPSW node at u-boot.

>> from what I understand, is around 2024 timeframe.. which is probably not
>> something that helps us in the community at this point.

OK, then we need to resort to some temporary solution like this then.
> 
> Yeah, at the moment I have to choose between getting the MMC to work in
> Linux or the Ethernet ports. The former is working thanks to your patch,
> the latter is broken because of it. Ideally I'd like both :)
> 

-- 
cheers,
-roger


Re: [PATCH 0/4] net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl

2023-07-20 Thread Maxime Ripard
Hi

Sorry, I didn't receive Roger mail so I'll reply here

On Thu, Jul 20, 2023 at 09:00:19AM -0500, Nishanth Menon wrote:
> On 16:56-20230720, Roger Quadros wrote:
> > Hi,
> > 
> > On 20/07/2023 16:33, Ravi Gunasekaran wrote:
> > > 
> > > 
> > > On 7/20/2023 3:25 PM, Maxime Ripard wrote:
> > >> Hi,
> > >>
> > >> This series is based on:
> > >> https://lore.kernel.org/all/20230713072019.3153871-1...@ti.com/
> > >>
> > >> It fixes the issue of Linux booting from the DT embedded by U-boot. The
> > >> main issue there is that U-Boot doesn't handle the MDIO child node that
> > >> might have resources attached to it.
> > >>
> > >> Thus, any pinctrl configuration that could be attached to the MDIO
> > >> child node is effectively ignored. Unfortunately, starting with 6.5-rc1,
> > >> Linux does just that.
> > 
> > I didn't get this part. Linux does not ignore pinctrl configuration attached
> > to MDIO child node. What changed in 6.5-rc1?

Linux doesn't ignore it, but Linux started putting pinctrl configuration
on the MDIO node, which is ignored by U-Boot.

> > >> This was solved by duplicating the pinctrl configuration onto the MAC
> > 
> > If I remember right, there is no driver model driver for MDIO in u-boot and
> > adding the mdio pinctrl into CPSW DT node was a hack in u-boot.

Yeah, but we now have in the U-Boot DT two nodes referencing the same
pinctrl configuration: the CPSW and the MDIO node. Linux then tries to
assign that configuration to both devices and it fails.

> > >> device node. Unfortunately, this doesn't work for Linux since now it has
> > >> two devices competing for the same pins.
> > 
> > What has really changed here is that you are passing u-boot's device
> > tree to Linux.

I didn't change anything. This is the default boot process if you don't
provide a DT in the ESP partition.

> > This has nothing to do with 6.5-rc1 right?

The issue started to occur with Nishanth patch to sync with Linux
6.5-rc1 device trees, so there's definitely a connection.

> > I suppose your solution is still a hack but of lesser evil than
> > duplicating MDIO pinctrl in CPSW node.
> > 
> > The proper solution would be to implement driver model for the
> > davinci MDIO driver. Siddharth has been working on this. If that is
> > close to ready we should just use that instead.
> 
> But this allows for a cleaner device tree while the driver can be fixed
> up independently, correct? Unfortunately, Siddharth's driver model work,
> from what I understand, is around 2024 timeframe.. which is probably not
> something that helps us in the community at this point.

Yeah, at the moment I have to choose between getting the MMC to work in
Linux or the Ethernet ports. The former is working thanks to your patch,
the latter is broken because of it. Ideally I'd like both :)

Maxime


signature.asc
Description: PGP signature


[PATCH 4/4] fixup! arm: dts: k3-am62: Bump dtsi from linux v6.5-rc1

2023-07-20 Thread Maxime Ripard
Dropping ranges entirely doesn't work since the register offset on the
MDIO device node will now be completely off, so we need to adjust it to
the right value without the translation.

We also need to have an empty ranges property for the reg address to be
properly evaluated.

Signed-off-by: Maxime Ripard 
---
 arch/arm/dts/k3-am625-sk-u-boot.dtsi | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi 
b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
index fe1c7408a89d..2ec3fff99f32 100644
--- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
+++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
@@ -122,8 +122,8 @@
reg = <0x0 0x800 0x0 0x20>,
  <0x0 0x43000200 0x0 0x8>;
reg-names = "cpsw_nuss", "mac_efuse";
-   /delete-property/ ranges;
bootph-pre-ram;
+   ranges;
 
cpsw-phy-sel@04044 {
compatible = "ti,am64-phy-gmii-sel";
@@ -132,6 +132,10 @@
};
 };
 
+_mdio {
+   reg = <0x0 0x8000f00 0x0 0x100>;
+};
+
 _port1 {
bootph-pre-ram;
 };

-- 
2.41.0



[PATCH 2/4] net: ti: am65-cpsw-nuss: Enforce pinctrl state on the MDIO child node

2023-07-20 Thread Maxime Ripard
The binding represents the MDIO controller as a child device tree
node of the MAC device tree node.

The U-Boot driver mostly ignores that child device tree node and just
hardcodes the resources it uses to support both the MAC and MDIO in a
single driver.

However, some resources like pinctrl muxing states are thus ignored.
This has been a problem with device trees since Linux 6.5-rc1 which will
put some pinctrl states on the MDIO device tree node.

Let's rework the driver a bit to fetch the pinctrl state from the MDIO
node and apply it.

Signed-off-by: Maxime Ripard 
---
 drivers/net/ti/am65-cpsw-nuss.c | 49 +
 1 file changed, 49 insertions(+)

diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c
index f674b0baa359..2d579bf4af2c 100644
--- a/drivers/net/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ti/am65-cpsw-nuss.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -696,6 +697,50 @@ out:
return ret;
 }
 
+static ofnode am65_cpsw_find_mdio(ofnode parent)
+{
+   ofnode node;
+
+   ofnode_for_each_subnode(node, parent)
+   if (ofnode_device_is_compatible(node, "ti,cpsw-mdio"))
+   return node;
+
+   return ofnode_null();
+}
+
+static int am65_cpsw_setup_mdio(struct udevice *dev)
+{
+   ofnode mdio;
+   int ret;
+
+   mdio = am65_cpsw_find_mdio(dev_ofnode(dev));
+   if (!ofnode_valid(mdio))
+   return -ENODEV;
+
+   /*
+* The MDIO controller is represented in the DT binding by a
+* subnode of the MAC controller.
+*
+* Our driver largely ignores that and supports MDIO by
+* hardcoding the register offsets.
+*
+* However, some resources (clocks, pinctrl) might be tied to
+* the MDIO node, and thus ignored.
+*
+* Clocks are not a concern at the moment since it's shared
+* between the MAC and MDIO, so the driver handles both at the
+* same time.
+*
+* However, we do need to make sure the pins states tied to the
+* MDIO node are configured properly.
+*/
+   ret = pinctrl_select_state_by_ofnode(dev, mdio, "default");
+   if (ret)
+   return ret;
+
+   return 0;
+}
+
 static int am65_cpsw_probe_nuss(struct udevice *dev)
 {
struct am65_cpsw_common *cpsw_common = dev_get_priv(dev);
@@ -728,6 +773,10 @@ static int am65_cpsw_probe_nuss(struct udevice *dev)
AM65_CPSW_CPSW_NU_ALE_BASE;
cpsw_common->mdio_base = cpsw_common->ss_base + AM65_CPSW_MDIO_BASE;
 
+   ret = am65_cpsw_setup_mdio(dev);
+   if (ret)
+   goto out;
+
ports_np = dev_read_subnode(dev, "ethernet-ports");
if (!ofnode_valid(ports_np)) {
ret = -ENOENT;

-- 
2.41.0



[PATCH 3/4] fixup! arm: dts: k3-am62: Bump dtsi from linux v6.5-rc1

2023-07-20 Thread Maxime Ripard
The MDIO pinctrl nodes can't be duplicated between the child and device,
because if we ever boot Linux with our DT it will try to attach that
pinctrl configuration to both the MAC and MDIO devices, which will
result in failure to probe.

Signed-off-by: Maxime Ripard 
---
 arch/arm/dts/k3-am625-sk-u-boot.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi 
b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
index 76589c7025a0..fe1c7408a89d 100644
--- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
+++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
@@ -124,7 +124,6 @@
reg-names = "cpsw_nuss", "mac_efuse";
/delete-property/ ranges;
bootph-pre-ram;
-   pinctrl-0 = <_mdio1_pins_default _rgmii1_pins_default>;
 
cpsw-phy-sel@04044 {
compatible = "ti,am64-phy-gmii-sel";

-- 
2.41.0



[PATCH 1/4] pinctrl: Create a select_state variant with the ofnode

2023-07-20 Thread Maxime Ripard
Some drivers might not follow entirely the driver/device association,
and thus might support what should be multiple devices in a single
driver.

Such a driver is am65-cpsw-nuss, where the MAC and MDIO controllers are
different device tree nodes, with their own resources (including pinctrl
pins) but supported by a single driver tied to the MAC device in U-Boot.

In order to get the proper pinctrl state, we would need to get the
state from the MDIO device tree node, but tie it to the MAC device since
it's the only thing we have access to.

Signed-off-by: Maxime Ripard 
---
 drivers/pinctrl/pinctrl-uclass.c | 15 +--
 include/dm/pinctrl.h | 26 --
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c
index 73dd7b1038bb..9e28f1858cbd 100644
--- a/drivers/pinctrl/pinctrl-uclass.c
+++ b/drivers/pinctrl/pinctrl-uclass.c
@@ -53,7 +53,8 @@ static int pinctrl_config_one(struct udevice *config)
  * @statename: state name, like "default"
  * @return: 0 on success, or negative error code on failure
  */
-static int pinctrl_select_state_full(struct udevice *dev, const char 
*statename)
+static int pinctrl_select_state_full(struct udevice *dev, ofnode node,
+const char *statename)
 {
char propname[32]; /* long enough */
const fdt32_t *list;
@@ -61,7 +62,7 @@ static int pinctrl_select_state_full(struct udevice *dev, 
const char *statename)
struct udevice *config;
int state, size, i, ret;
 
-   state = dev_read_stringlist_search(dev, "pinctrl-names", statename);
+   state = ofnode_stringlist_search(node, "pinctrl-names", statename);
if (state < 0) {
char *end;
/*
@@ -74,7 +75,7 @@ static int pinctrl_select_state_full(struct udevice *dev, 
const char *statename)
}
 
snprintf(propname, sizeof(propname), "pinctrl-%d", state);
-   list = dev_read_prop(dev, propname, );
+   list = ofnode_get_property(node, propname, );
if (!list)
return -ENOSYS;
 
@@ -293,20 +294,22 @@ static int pinctrl_select_state_simple(struct udevice 
*dev)
return ops->set_state_simple(pctldev, dev);
 }
 
-int pinctrl_select_state(struct udevice *dev, const char *statename)
+int pinctrl_select_state_by_ofnode(struct udevice *dev, ofnode node,
+  const char *statename)
 {
/*
 * Some device which is logical like mmc.blk, do not have
 * a valid ofnode.
 */
-   if (!dev_has_ofnode(dev))
+   if (!ofnode_valid(node))
return 0;
+
/*
 * Try full-implemented pinctrl first.
 * If it fails or is not implemented, try simple one.
 */
if (CONFIG_IS_ENABLED(PINCTRL_FULL))
-   return pinctrl_select_state_full(dev, statename);
+   return pinctrl_select_state_full(dev, node, statename);
 
return pinctrl_select_state_simple(dev);
 }
diff --git a/include/dm/pinctrl.h b/include/dm/pinctrl.h
index e3e50afeaff0..be4679b7f20d 100644
--- a/include/dm/pinctrl.h
+++ b/include/dm/pinctrl.h
@@ -502,6 +502,24 @@ static inline int pinctrl_generic_set_state(struct udevice 
*pctldev,
 #endif
 
 #if CONFIG_IS_ENABLED(PINCTRL)
+/**
+ * pinctrl_select_state_by_ofnode() - Set a device to a given state using the 
given ofnode
+ * @dev:   Peripheral device
+ * @ofnode:Device Tree node to get the state from
+ * @statename: State name, like "default"
+ *
+ * Return: 0 on success, or negative error code on failure
+ */
+int pinctrl_select_state_by_ofnode(struct udevice *dev, ofnode node, const 
char *statename);
+#else
+static inline int pinctrl_select_state_by_ofnode(struct udevice *dev,
+ofnode node,
+const char *statename)
+{
+   return -ENOSYS;
+}
+#endif
+
 /**
  * pinctrl_select_state() - Set a device to a given state
  * @dev:   Peripheral device
@@ -509,14 +527,10 @@ static inline int pinctrl_generic_set_state(struct 
udevice *pctldev,
  *
  * Return: 0 on success, or negative error code on failure
  */
-int pinctrl_select_state(struct udevice *dev, const char *statename);
-#else
-static inline int pinctrl_select_state(struct udevice *dev,
-  const char *statename)
+static inline int pinctrl_select_state(struct udevice *dev, const char 
*statename)
 {
-   return -ENOSYS;
+   return pinctrl_select_state_by_ofnode(dev, dev_ofnode(dev), statename);
 }
-#endif
 
 /**
  * pinctrl_request() - Request a particular pinctrl function

-- 
2.41.0



[PATCH 0/4] net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl

2023-07-20 Thread Maxime Ripard
Hi,

This series is based on:
https://lore.kernel.org/all/20230713072019.3153871-1...@ti.com/

It fixes the issue of Linux booting from the DT embedded by U-boot. The
main issue there is that U-Boot doesn't handle the MDIO child node that
might have resources attached to it.

Thus, any pinctrl configuration that could be attached to the MDIO
child node is effectively ignored. Unfortunately, starting with 6.5-rc1,
Linux does just that.

This was solved by duplicating the pinctrl configuration onto the MAC
device node. Unfortunately, this doesn't work for Linux since now it has
two devices competing for the same pins.

Let me know what you think,
Maxime

Signed-off-by: Maxime Ripard 
---
Maxime Ripard (4):
  pinctrl: Create a select_state variant with the ofnode
  net: ti: am65-cpsw-nuss: Enforce pinctrl state on the MDIO child node
  fixup! arm: dts: k3-am62: Bump dtsi from linux v6.5-rc1
  fixup! arm: dts: k3-am62: Bump dtsi from linux v6.5-rc1

 arch/arm/dts/k3-am625-sk-u-boot.dtsi |  7 --
 drivers/net/ti/am65-cpsw-nuss.c  | 49 
 drivers/pinctrl/pinctrl-uclass.c | 15 ++-
 include/dm/pinctrl.h | 26 ++-
 4 files changed, 83 insertions(+), 14 deletions(-)
---
base-commit: acff6e7c553d5a839e885730a4018465a34ba5a7
change-id: 20230720-ti-mdio-pinmux-a12525dba973

Best regards,
-- 
Maxime Ripard 



Re: [PATCH 0/4] net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl

2023-07-20 Thread Nishanth Menon
On 16:56-20230720, Roger Quadros wrote:
> Hi,
> 
> On 20/07/2023 16:33, Ravi Gunasekaran wrote:
> > 
> > 
> > On 7/20/2023 3:25 PM, Maxime Ripard wrote:
> >> Hi,
> >>
> >> This series is based on:
> >> https://lore.kernel.org/all/20230713072019.3153871-1...@ti.com/
> >>
> >> It fixes the issue of Linux booting from the DT embedded by U-boot. The
> >> main issue there is that U-Boot doesn't handle the MDIO child node that
> >> might have resources attached to it.
> >>
> >> Thus, any pinctrl configuration that could be attached to the MDIO
> >> child node is effectively ignored. Unfortunately, starting with 6.5-rc1,
> >> Linux does just that.
> 
> I didn't get this part. Linux does not ignore pinctrl configuration attached
> to MDIO child node. What changed in 6.5-rc1?
> 
> >>
> >> This was solved by duplicating the pinctrl configuration onto the MAC
> 
> If I remember right, there is no driver model driver for MDIO in u-boot and
> adding the mdio pinctrl into CPSW DT node was a hack in u-boot.
> 
> >> device node. Unfortunately, this doesn't work for Linux since now it has
> >> two devices competing for the same pins.
> 
> What has really changed here is that you are passing u-boot's device tree to 
> Linux.
> This has nothing to do with 6.5-rc1 right?
> 
> I suppose your solution is still a hack but of lesser evil than
> duplicating MDIO pinctrl in CPSW node.
> 
> The proper solution would be to implement driver model for the davinci MDIO 
> driver.
> Siddharth has been working on this. If that is close to ready we should just 
> use
> that instead.

But this allows for a cleaner device tree while the driver can be fixed
up independently, correct? Unfortunately, Siddharth's driver model work,
from what I understand, is around 2024 timeframe.. which is probably not
something that helps us in the community at this point.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D


Re: [PATCH 0/4] net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl

2023-07-20 Thread Roger Quadros
Hi,

On 20/07/2023 16:33, Ravi Gunasekaran wrote:
> 
> 
> On 7/20/2023 3:25 PM, Maxime Ripard wrote:
>> Hi,
>>
>> This series is based on:
>> https://lore.kernel.org/all/20230713072019.3153871-1...@ti.com/
>>
>> It fixes the issue of Linux booting from the DT embedded by U-boot. The
>> main issue there is that U-Boot doesn't handle the MDIO child node that
>> might have resources attached to it.
>>
>> Thus, any pinctrl configuration that could be attached to the MDIO
>> child node is effectively ignored. Unfortunately, starting with 6.5-rc1,
>> Linux does just that.

I didn't get this part. Linux does not ignore pinctrl configuration attached
to MDIO child node. What changed in 6.5-rc1?

>>
>> This was solved by duplicating the pinctrl configuration onto the MAC

If I remember right, there is no driver model driver for MDIO in u-boot and
adding the mdio pinctrl into CPSW DT node was a hack in u-boot.

>> device node. Unfortunately, this doesn't work for Linux since now it has
>> two devices competing for the same pins.

What has really changed here is that you are passing u-boot's device tree to 
Linux.
This has nothing to do with 6.5-rc1 right?

I suppose your solution is still a hack but of lesser evil than
duplicating MDIO pinctrl in CPSW node.

The proper solution would be to implement driver model for the davinci MDIO 
driver.
Siddharth has been working on this. If that is close to ready we should just use
that instead.

>>
>> Let me know what you think,
>> Maxime
>>
>> Signed-off-by: Maxime Ripard 
>> ---
>> Maxime Ripard (4):
>>   pinctrl: Create a select_state variant with the ofnode
>>   net: ti: am65-cpsw-nuss: Enforce pinctrl state on the MDIO child node
>>   fixup! arm: dts: k3-am62: Bump dtsi from linux v6.5-rc1
>>   fixup! arm: dts: k3-am62: Bump dtsi from linux v6.5-rc1
>>
>>  arch/arm/dts/k3-am625-sk-u-boot.dtsi |  7 --
>>  drivers/net/ti/am65-cpsw-nuss.c  | 49 
>> 
>>  drivers/pinctrl/pinctrl-uclass.c | 15 ++-
>>  include/dm/pinctrl.h | 26 ++-
>>  4 files changed, 83 insertions(+), 14 deletions(-)
>> ---
>> base-commit: acff6e7c553d5a839e885730a4018465a34ba5a7
>> change-id: 20230720-ti-mdio-pinmux-a12525dba973
>>
>> Best regards,
> 
> Adding Roger

-- 
cheers,
-roger


Re: [PATCH] Makefile: Use sort shortopts

2023-07-20 Thread Mark Kettenis
> From: Marek Vasut 
> Date: Thu, 20 Jul 2023 14:50:42 +0200
> 
> POSIX does not defined longopts for sort, use shortops
> for even more compatibility.

Even though OpenBSD's sort does implement the long options this is
still good to have!

Reviewed-by: Mark Kettenis 

> Fixes: cc5a490cf465 ("Makefile: Sort u-boot-initial-env output")
> Reported-by: Milan P. Stanić 
> Signed-off-by: Marek Vasut 
> ---
> Cc: Christoph Niedermaier 
> Cc: Marek Behún 
> Cc: Simon Glass 
> Cc: Stefano Babic 
> Cc: u-b...@dh-electronics.com
> Cc: u-boot@lists.denx.de
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 87f9fc786e8..5fc16b3b1f1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2444,7 +2444,7 @@ quiet_cmd_genenv = GENENV  $@
>  cmd_genenv = \
>   $(objtree)/tools/printinitialenv | \
>   sed -e '/^\s*$$/d' | \
> - sort --field-separator='=' -k1,1 --stable -o $@
> + sort -t '=' -k 1,1 -s -o $@
>  
>  u-boot-initial-env: $(env_h) FORCE
>   $(Q)$(MAKE) $(build)=tools $(objtree)/tools/printinitialenv
> -- 
> 2.40.1
> 
> 


Re: [PATCH 0/4] net: ti: am65-cpsw-nuss: Fix DT binding handling of pinctrl

2023-07-20 Thread Ravi Gunasekaran



On 7/20/2023 3:25 PM, Maxime Ripard wrote:
> Hi,
>
> This series is based on:
> https://lore.kernel.org/all/20230713072019.3153871-1...@ti.com/
>
> It fixes the issue of Linux booting from the DT embedded by U-boot. The
> main issue there is that U-Boot doesn't handle the MDIO child node that
> might have resources attached to it.
>
> Thus, any pinctrl configuration that could be attached to the MDIO
> child node is effectively ignored. Unfortunately, starting with 6.5-rc1,
> Linux does just that.
>
> This was solved by duplicating the pinctrl configuration onto the MAC
> device node. Unfortunately, this doesn't work for Linux since now it has
> two devices competing for the same pins.
>
> Let me know what you think,
> Maxime
>
> Signed-off-by: Maxime Ripard 
> ---
> Maxime Ripard (4):
>   pinctrl: Create a select_state variant with the ofnode
>   net: ti: am65-cpsw-nuss: Enforce pinctrl state on the MDIO child node
>   fixup! arm: dts: k3-am62: Bump dtsi from linux v6.5-rc1
>   fixup! arm: dts: k3-am62: Bump dtsi from linux v6.5-rc1
>
>  arch/arm/dts/k3-am625-sk-u-boot.dtsi |  7 --
>  drivers/net/ti/am65-cpsw-nuss.c  | 49 
> 
>  drivers/pinctrl/pinctrl-uclass.c | 15 ++-
>  include/dm/pinctrl.h | 26 ++-
>  4 files changed, 83 insertions(+), 14 deletions(-)
> ---
> base-commit: acff6e7c553d5a839e885730a4018465a34ba5a7
> change-id: 20230720-ti-mdio-pinmux-a12525dba973
>
> Best regards,

Adding Roger


[PATCH] net: wget: Avoid packet queue overflow

2023-07-20 Thread Richard Weinberger
Make sure to stay within bounds, as a misbehaving HTTP server
can trigger a buffer overflow if not properly handled.

Cc: Joe Hershberger 
Cc: Ramon Fried 
Signed-off-by: Richard Weinberger 
---
 net/wget.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/wget.c b/net/wget.c
index 2dbfeb1a1d5b..8bb4d72db1ae 100644
--- a/net/wget.c
+++ b/net/wget.c
@@ -35,7 +35,8 @@ struct pkt_qd {
  * The actual packet bufers are in the kernel space, and are
  * expected to be overwritten by the downloaded image.
  */
-static struct pkt_qd pkt_q[PKTBUFSRX / 4];
+#define PKTQ_SZ (PKTBUFSRX / 4)
+static struct pkt_qd pkt_q[PKTQ_SZ];
 static int pkt_q_idx;
 static unsigned long content_length;
 static unsigned int packets;
@@ -202,6 +203,13 @@ static void wget_connected(uchar *pkt, unsigned int 
tcp_seq_num,
pkt_q[pkt_q_idx].tcp_seq_num = tcp_seq_num;
pkt_q[pkt_q_idx].len = len;
pkt_q_idx++;
+
+   if (pkt_q_idx >= PKTQ_SZ) {
+   printf("wget: Fatal error, queue overrun!\n");
+   net_set_state(NETLOOP_FAIL);
+
+   return;
+   }
} else {
debug_cond(DEBUG_WGET, "wget: Connected HTTP Header %p\n", pkt);
/* sizeof(http_eom) - 1 is the string length of (http_eom) */
-- 
2.26.2



[PATCH] Makefile: Use sort shortopts

2023-07-20 Thread Marek Vasut
POSIX does not defined longopts for sort, use shortops
for even more compatibility.

Fixes: cc5a490cf465 ("Makefile: Sort u-boot-initial-env output")
Reported-by: Milan P. Stanić 
Signed-off-by: Marek Vasut 
---
Cc: Christoph Niedermaier 
Cc: Marek Behún 
Cc: Simon Glass 
Cc: Stefano Babic 
Cc: u-b...@dh-electronics.com
Cc: u-boot@lists.denx.de
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 87f9fc786e8..5fc16b3b1f1 100644
--- a/Makefile
+++ b/Makefile
@@ -2444,7 +2444,7 @@ quiet_cmd_genenv = GENENV  $@
 cmd_genenv = \
$(objtree)/tools/printinitialenv | \
sed -e '/^\s*$$/d' | \
-   sort --field-separator='=' -k1,1 --stable -o $@
+   sort -t '=' -k 1,1 -s -o $@
 
 u-boot-initial-env: $(env_h) FORCE
$(Q)$(MAKE) $(build)=tools $(objtree)/tools/printinitialenv
-- 
2.40.1



[PATCH v2 7/7] power: regulator-uclass: remove all deprecated API use

2023-07-20 Thread Svyatoslav Ryhel
THIS REQUIRES PRECISE TESTING!

Signed-off-by: Svyatoslav Ryhel 
---
 arch/arm/mach-rockchip/board.c  |  8 
 arch/arm/mach-rockchip/rk3399/rk3399.c  | 10 --
 board/Marvell/octeontx2_cn913x/board.c  |  5 -
 board/amlogic/odroid-go-ultra/odroid-go-ultra.c |  2 --
 board/dhelectronics/dh_stm32mp1/board.c |  2 --
 board/engicam/stm32mp1/stm32mp1.c   |  3 ---
 board/google/veyron/veyron.c|  6 --
 board/samsung/common/exynos5-dt.c   |  4 
 board/samsung/odroid/odroid.c   | 10 --
 board/st/stm32mp1/stm32mp1.c|  9 -
 drivers/video/bridge/ps862x.c   | 12 
 drivers/video/rockchip/rk_vop.c |  6 ++
 12 files changed, 10 insertions(+), 67 deletions(-)

diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
index 8d7b39ba15..3eaea4b04a 100644
--- a/arch/arm/mach-rockchip/board.c
+++ b/arch/arm/mach-rockchip/board.c
@@ -189,14 +189,6 @@ int board_late_init(void)
 
 int board_init(void)
 {
-   int ret;
-
-#ifdef CONFIG_DM_REGULATOR
-   ret = regulators_enable_boot_on(false);
-   if (ret)
-   debug("%s: Cannot enable boot on regulator\n", __func__);
-#endif
-
return 0;
 }
 
diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c 
b/arch/arm/mach-rockchip/rk3399/rk3399.c
index a7cc91a952..cbd2ea047d 100644
--- a/arch/arm/mach-rockchip/rk3399/rk3399.c
+++ b/arch/arm/mach-rockchip/rk3399/rk3399.c
@@ -280,15 +280,5 @@ void spl_board_init(void)
if (cru->glb_rst_st != 0)
rk3399_force_power_on_reset();
}
-
-   if (IS_ENABLED(CONFIG_SPL_DM_REGULATOR)) {
-   /*
-* Turning the eMMC and SPI back on (if disabled via the Qseven
-* BIOS_ENABLE) signal is done through a always-on regulator).
-*/
-   if (regulators_enable_boot_on(false))
-   debug("%s: Cannot enable boot on regulator\n",
- __func__);
-   }
 }
 #endif
diff --git a/board/Marvell/octeontx2_cn913x/board.c 
b/board/Marvell/octeontx2_cn913x/board.c
index 3d20cfb2fa..3ffe15d42b 100644
--- a/board/Marvell/octeontx2_cn913x/board.c
+++ b/board/Marvell/octeontx2_cn913x/board.c
@@ -23,11 +23,6 @@ int board_early_init_f(void)
 
 int board_early_init_r(void)
 {
-   if (CONFIG_IS_ENABLED(DM_REGULATOR)) {
-   /* Check if any existing regulator should be turned down */
-   regulators_enable_boot_off(false);
-   }
-
return 0;
 }
 
diff --git a/board/amlogic/odroid-go-ultra/odroid-go-ultra.c 
b/board/amlogic/odroid-go-ultra/odroid-go-ultra.c
index bbd23e20fc..fa6105a071 100644
--- a/board/amlogic/odroid-go-ultra/odroid-go-ultra.c
+++ b/board/amlogic/odroid-go-ultra/odroid-go-ultra.c
@@ -16,7 +16,5 @@ int mmc_get_env_dev(void)
 
 int board_init(void)
 {
-   regulators_enable_boot_on(_DEBUG);
-
return 0;
 }
diff --git a/board/dhelectronics/dh_stm32mp1/board.c 
b/board/dhelectronics/dh_stm32mp1/board.c
index f9cfabe242..a8ad5a1620 100644
--- a/board/dhelectronics/dh_stm32mp1/board.c
+++ b/board/dhelectronics/dh_stm32mp1/board.c
@@ -615,8 +615,6 @@ static void board_init_regulator_av96(void)
 static void board_init_regulator(void)
 {
board_init_regulator_av96();
-
-   regulators_enable_boot_on(_DEBUG);
 }
 #else
 static inline int board_get_regulator_buck3_nvm_uv_av96(int *uv)
diff --git a/board/engicam/stm32mp1/stm32mp1.c 
b/board/engicam/stm32mp1/stm32mp1.c
index 5223e9bae8..c98bbdc71b 100644
--- a/board/engicam/stm32mp1/stm32mp1.c
+++ b/board/engicam/stm32mp1/stm32mp1.c
@@ -38,9 +38,6 @@ int checkboard(void)
 /* board dependent setup after realloc */
 int board_init(void)
 {
-   if (IS_ENABLED(CONFIG_DM_REGULATOR))
-   regulators_enable_boot_on(_DEBUG);
-
return 0;
 }
 
diff --git a/board/google/veyron/veyron.c b/board/google/veyron/veyron.c
index 32dbcdc4d1..527e9d4b0e 100644
--- a/board/google/veyron/veyron.c
+++ b/board/google/veyron/veyron.c
@@ -62,12 +62,6 @@ static int veyron_init(void)
if (ret)
return ret;
 
-   ret = regulators_enable_boot_on(false);
-   if (ret) {
-   debug("%s: Cannot enable boot on regulators\n", __func__);
-   return ret;
-   }
-
return 0;
 }
 #endif
diff --git a/board/samsung/common/exynos5-dt.c 
b/board/samsung/common/exynos5-dt.c
index cde77d79a0..45d34d838d 100644
--- a/board/samsung/common/exynos5-dt.c
+++ b/board/samsung/common/exynos5-dt.c
@@ -92,10 +92,6 @@ int exynos_power_init(void)
if (ret == -ENODEV)
return 0;
 
-   ret = regulators_enable_boot_on(false);
-   if (ret)
-   return ret;
-
ret = exynos_set_regulator("vdd_mif", 110);
if (ret)
return ret;
diff --git 

[PATCH v2 6/7] test: dm: pmic: provide test of child autosetup

2023-07-20 Thread Svyatoslav Ryhel
Test if regulator uclass children of PMIC are auto probed and set
after PMIC probe.

Signed-off-by: Svyatoslav Ryhel 
---
 test/dm/pmic.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/test/dm/pmic.c b/test/dm/pmic.c
index ce671202fb..d0a3c24a54 100644
--- a/test/dm/pmic.c
+++ b/test/dm/pmic.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -129,3 +130,36 @@ static int dm_test_power_pmic_mc34708_rw_val(struct 
unit_test_state *uts)
 }
 
 DM_TEST(dm_test_power_pmic_mc34708_rw_val, UT_TESTF_SCAN_FDT);
+
+static int dm_test_power_pmic_child_probe(struct unit_test_state *uts)
+{
+   const char *name = "sandbox_pmic";
+   const char *devname = "ldo1";
+   struct udevice *dev;
+
+   ut_assertok(pmic_get(name, ));
+
+   /*
+* LDO1 with fdt properties:
+* - min-microvolt = max-microvolt = 180
+* - min-microamp = max-microamp = 10
+* - always-on = not set
+* - boot-on = set
+* Expected output state: uV=180; uA=10; output enabled
+*/
+
+   /* Check, that the returned device is proper */
+   ut_assertok(regulator_get_by_devname(devname, ));
+
+   /* Check the setup after autoset */
+   ut_asserteq(regulator_get_value(dev),
+   SANDBOX_LDO1_AUTOSET_EXPECTED_UV);
+   ut_asserteq(regulator_get_current(dev),
+   SANDBOX_LDO1_AUTOSET_EXPECTED_UA);
+   ut_asserteq(regulator_get_enable(dev),
+   SANDBOX_LDO1_AUTOSET_EXPECTED_ENABLE);
+
+   return 0;
+}
+
+DM_TEST(dm_test_power_pmic_child_probe, UT_TESTF_SCAN_FDT);
-- 
2.39.2



[PATCH v2 5/7] power: pmic-uclass: probe every child on pmic_post_probe

2023-07-20 Thread Svyatoslav Ryhel
Main goal is to probe all regulator childrens for their
proper setup but if pmic has non regulator children they
should not suffer from this either.

Signed-off-by: Svyatoslav Ryhel 
---
 drivers/power/pmic/pmic-uclass.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c
index 0e2f5e1f41..8ca717bd5e 100644
--- a/drivers/power/pmic/pmic-uclass.c
+++ b/drivers/power/pmic/pmic-uclass.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #if CONFIG_IS_ENABLED(PMIC_CHILDREN)
@@ -198,9 +199,18 @@ static int pmic_pre_probe(struct udevice *dev)
return 0;
 }
 
+static int pmic_post_probe(struct udevice *dev)
+{
+   struct udevice *child;
+
+   device_foreach_child_probe(child, dev);
+   return 0;
+}
+
 UCLASS_DRIVER(pmic) = {
.id = UCLASS_PMIC,
.name   = "pmic",
.pre_probe  = pmic_pre_probe,
+   .post_probe = pmic_post_probe,
.per_device_auto= sizeof(struct uc_pmic_priv),
 };
-- 
2.39.2



[PATCH v2 4/7] test: dm: regulator: provide test of auto setup

2023-07-20 Thread Svyatoslav Ryhel
Replace deprecated API test with test of uclass wide autosetup.

LDO2 voltage according to dts should be 3.3v not 3.0v

Signed-off-by: Svyatoslav Ryhel 
---
 include/power/sandbox_pmic.h |   2 +-
 test/dm/regulator.c  | 131 +++
 2 files changed, 27 insertions(+), 106 deletions(-)

diff --git a/include/power/sandbox_pmic.h b/include/power/sandbox_pmic.h
index 1dbd15b523..50fc5881f8 100644
--- a/include/power/sandbox_pmic.h
+++ b/include/power/sandbox_pmic.h
@@ -137,7 +137,7 @@ enum {
 #define SANDBOX_LDO1_AUTOSET_EXPECTED_UA   10
 #define SANDBOX_LDO1_AUTOSET_EXPECTED_ENABLE   true
 
-#define SANDBOX_LDO2_AUTOSET_EXPECTED_UV   300
+#define SANDBOX_LDO2_AUTOSET_EXPECTED_UV   330
 #define SANDBOX_LDO2_AUTOSET_EXPECTED_UA   -ENOSYS
 #define SANDBOX_LDO2_AUTOSET_EXPECTED_ENABLE   false
 
diff --git a/test/dm/regulator.c b/test/dm/regulator.c
index 9b503e4c59..eb96ddcdac 100644
--- a/test/dm/regulator.c
+++ b/test/dm/regulator.c
@@ -182,12 +182,11 @@ static
 int dm_test_power_regulator_set_enable_if_allowed(struct unit_test_state *uts)
 {
const char *platname;
-   struct udevice *dev, *dev_autoset;
+   struct udevice *dev;
bool val_set = false;
 
/* Get BUCK1 - always on regulator */
platname = regulator_names[BUCK1][PLATNAME];
-   ut_assertok(regulator_autoset_by_name(platname, _autoset));
ut_assertok(regulator_get_by_platname(platname, ));
 
/* Try disabling always-on regulator */
@@ -196,7 +195,6 @@ int dm_test_power_regulator_set_enable_if_allowed(struct 
unit_test_state *uts)
 
/* Set the Enable of LDO1 - default is disabled */
platname = regulator_names[LDO1][PLATNAME];
-   ut_assertok(regulator_autoset_by_name(platname, _autoset));
ut_assertok(regulator_get_by_platname(platname, ));
 
/* Get the Enable state of LDO1 and compare it with the requested one */
@@ -293,131 +291,54 @@ static int 
dm_test_power_regulator_set_get_suspend_enable(struct unit_test_state
 }
 DM_TEST(dm_test_power_regulator_set_get_suspend_enable, UT_TESTF_SCAN_FDT);
 
-/* Test regulator autoset method */
-static int dm_test_power_regulator_autoset(struct unit_test_state *uts)
+/* Test regulator setup inside uclass driver */
+static int dm_test_power_regulator_set(struct unit_test_state *uts)
 {
const char *platname;
-   struct udevice *dev, *dev_autoset;
+   struct udevice *dev;
 
/*
-* Test the BUCK1 with fdt properties
-* - min-microvolt = max-microvolt = 120
-* - min-microamp = max-microamp = 20
-* - always-on = set
-* - boot-on = not set
-* Expected output state: uV=120; uA=20; output enabled
+* LDO1 with fdt properties:
+* - min-microvolt = max-microvolt = 180
+* - min-microamp = max-microamp = 10
+* - always-on = not set
+* - boot-on = set
+* Expected output state: uV=180; uA=10; output enabled
 */
-   platname = regulator_names[BUCK1][PLATNAME];
-   ut_assertok(regulator_autoset_by_name(platname, _autoset));
 
/* Check, that the returned device is proper */
+   platname = regulator_names[LDO1][PLATNAME];
ut_assertok(regulator_get_by_platname(platname, ));
-   ut_asserteq_ptr(dev, dev_autoset);
 
/* Check the setup after autoset */
ut_asserteq(regulator_get_value(dev),
-   SANDBOX_BUCK1_AUTOSET_EXPECTED_UV);
+   SANDBOX_LDO1_AUTOSET_EXPECTED_UV);
ut_asserteq(regulator_get_current(dev),
-   SANDBOX_BUCK1_AUTOSET_EXPECTED_UA);
+   SANDBOX_LDO1_AUTOSET_EXPECTED_UA);
ut_asserteq(regulator_get_enable(dev),
-   SANDBOX_BUCK1_AUTOSET_EXPECTED_ENABLE);
-
-   return 0;
-}
-DM_TEST(dm_test_power_regulator_autoset, UT_TESTF_SCAN_FDT);
-
-/*
- * Struct setting: to keep the expected output settings.
- * @voltage: Voltage value [uV]
- * @current: Current value [uA]
- * @enable: output enable state: true/false
- */
-struct setting {
-   int voltage;
-   int current;
-   bool enable;
-};
-
-/*
- * platname_list: an array of regulator platform names.
- * For testing regulator_list_autoset() for outputs:
- * - LDO1
- * - LDO2
- */
-static const char *platname_list[] = {
-   SANDBOX_LDO1_PLATNAME,
-   SANDBOX_LDO2_PLATNAME,
-   NULL,
-};
-
-/*
- * expected_setting_list: an array of regulator output setting, expected after
- * call of the regulator_list_autoset() for the "platname_list" array.
- * For testing results of regulator_list_autoset() for outputs:
- * - LDO1
- * - LDO2
- * The settings are defined in: include/power/sandbox_pmic.h
- */
-static const struct setting expected_setting_list[] = {
-   [0] = { /* LDO1 */
-   .voltage = SANDBOX_LDO1_AUTOSET_EXPECTED_UV,
-   .current = SANDBOX_LDO1_AUTOSET_EXPECTED_UA,
-   .enable  = 

[PATCH v2 3/7] power: regulator-uclass: perform regulator setup inside uclass

2023-07-20 Thread Svyatoslav Ryhel
Regulators initial setup was previously dependent on board call.
To move from this behaviour were introduced two steps. First is
that all individual regulators will be probed just after binding
which ensures that regulator pdata is filled and second is setting
up regulator in post probe which enseres that correct regulator
state according to pdata is reached.

Signed-off-by: Svyatoslav Ryhel 
---
 drivers/power/regulator/regulator-uclass.c | 212 ++---
 include/power/regulator.h  | 119 
 2 files changed, 62 insertions(+), 269 deletions(-)

diff --git a/drivers/power/regulator/regulator-uclass.c 
b/drivers/power/regulator/regulator-uclass.c
index fc7a4631b4..32f4e64ca4 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -327,112 +327,6 @@ int device_get_supply_regulator(struct udevice *dev, 
const char *supply_name,
supply_name, devp);
 }
 
-int regulator_autoset(struct udevice *dev)
-{
-   struct dm_regulator_uclass_plat *uc_pdata;
-   int ret = 0;
-
-   uc_pdata = dev_get_uclass_plat(dev);
-
-   ret = regulator_set_suspend_enable(dev, uc_pdata->suspend_on);
-   if (ret == -ENOSYS)
-   ret = 0;
-
-   if (!ret && uc_pdata->suspend_on) {
-   ret = regulator_set_suspend_value(dev, uc_pdata->suspend_uV);
-   if (ret == -ENOSYS)
-   ret = 0;
-
-   if (ret)
-   return ret;
-   }
-
-   if (!uc_pdata->always_on && !uc_pdata->boot_on)
-   return -EMEDIUMTYPE;
-
-   if (uc_pdata->type == REGULATOR_TYPE_FIXED)
-   return regulator_set_enable(dev, true);
-
-   if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV)
-   ret = regulator_set_value(dev, uc_pdata->min_uV);
-   if (uc_pdata->init_uV > 0)
-   ret = regulator_set_value(dev, uc_pdata->init_uV);
-   if (!ret && (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA))
-   ret = regulator_set_current(dev, uc_pdata->min_uA);
-
-   if (!ret)
-   ret = regulator_set_enable(dev, true);
-
-   return ret;
-}
-
-int regulator_unset(struct udevice *dev)
-{
-   struct dm_regulator_uclass_plat *uc_pdata;
-
-   uc_pdata = dev_get_uclass_plat(dev);
-   if (uc_pdata && uc_pdata->force_off)
-   return regulator_set_enable(dev, false);
-
-   return -EMEDIUMTYPE;
-}
-
-static void regulator_show(struct udevice *dev, int ret)
-{
-   struct dm_regulator_uclass_plat *uc_pdata;
-
-   uc_pdata = dev_get_uclass_plat(dev);
-
-   printf("%s@%s: ", dev->name, uc_pdata->name);
-   if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UV)
-   printf("set %d uV", uc_pdata->min_uV);
-   if (uc_pdata->flags & REGULATOR_FLAG_AUTOSET_UA)
-   printf("; set %d uA", uc_pdata->min_uA);
-   printf("; enabling");
-   if (ret)
-   printf(" (ret: %d)", ret);
-   printf("\n");
-}
-
-int regulator_autoset_by_name(const char *platname, struct udevice **devp)
-{
-   struct udevice *dev;
-   int ret;
-
-   ret = regulator_get_by_platname(platname, );
-   if (devp)
-   *devp = dev;
-   if (ret) {
-   debug("Can get the regulator: %s (err=%d)\n", platname, ret);
-   return ret;
-   }
-
-   return regulator_autoset(dev);
-}
-
-int regulator_list_autoset(const char *list_platname[],
-  struct udevice *list_devp[],
-  bool verbose)
-{
-   struct udevice *dev;
-   int error = 0, i = 0, ret;
-
-   while (list_platname[i]) {
-   ret = regulator_autoset_by_name(list_platname[i], );
-   if (ret != -EMEDIUMTYPE && verbose)
-   regulator_show(dev, ret);
-   if (ret & !error)
-   error = ret;
-
-   if (list_devp)
-   list_devp[i] = dev;
-
-   i++;
-   }
-
-   return error;
-}
-
 static bool regulator_name_is_unique(struct udevice *check_dev,
 const char *check_name)
 {
@@ -463,6 +357,7 @@ static int regulator_post_bind(struct udevice *dev)
 {
struct dm_regulator_uclass_plat *uc_pdata;
const char *property = "regulator-name";
+   int ret;
 
uc_pdata = dev_get_uclass_plat(dev);
 
@@ -476,13 +371,28 @@ static int regulator_post_bind(struct udevice *dev)
return -EINVAL;
}
 
-   if (regulator_name_is_unique(dev, uc_pdata->name))
-   return 0;
+   ret = regulator_name_is_unique(dev, uc_pdata->name);
+   if (!ret) {
+   debug("'%s' of dev: '%s', has nonunique value: '%s\n",
+ property, dev->name, uc_pdata->name);
+   return -EINVAL;
+   }
 
-   debug("'%s' of dev: '%s', has nonunique 

[PATCH v2 2/7] test: dm: regulator: test counter in set_enable_if_allowed test

2023-07-20 Thread Svyatoslav Ryhel
Emulate use of the regulator by a few consumers with balanced on/off
calls.

Signed-off-by: Svyatoslav Ryhel 
---
 test/dm/regulator.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/test/dm/regulator.c b/test/dm/regulator.c
index 86f4862d9d..9b503e4c59 100644
--- a/test/dm/regulator.c
+++ b/test/dm/regulator.c
@@ -194,6 +194,25 @@ int dm_test_power_regulator_set_enable_if_allowed(struct 
unit_test_state *uts)
ut_assertok(regulator_set_enable_if_allowed(dev, val_set));
ut_asserteq(regulator_get_enable(dev), !val_set);
 
+   /* Set the Enable of LDO1 - default is disabled */
+   platname = regulator_names[LDO1][PLATNAME];
+   ut_assertok(regulator_autoset_by_name(platname, _autoset));
+   ut_assertok(regulator_get_by_platname(platname, ));
+
+   /* Get the Enable state of LDO1 and compare it with the requested one */
+   ut_assertok(regulator_set_enable_if_allowed(dev, true));
+   ut_asserteq(regulator_get_enable(dev), true);
+
+   /* Emulate second consumer */
+   ut_assertok(regulator_set_enable_if_allowed(dev, true));
+   ut_asserteq(regulator_get_enable(dev), true);
+
+   /* Emulate one of counsumers disable call */
+   ut_assertok(regulator_set_enable_if_allowed(dev, false));
+
+   /* Regulator should still be on since counter indicates one consumer 
active */
+   ut_asserteq(regulator_get_enable(dev), true);
+
return 0;
 }
 DM_TEST(dm_test_power_regulator_set_enable_if_allowed, UT_TESTF_SCAN_FDT);
-- 
2.39.2



  1   2   >