[PATCH v3 06/15] board: ns3: default reset type to L3

[Rayagonda Kokatanur]

Hi Simon,

On Fri, Jul 3, 2020 at 6:16 AM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Rayagonda,
>
> On Mon, 29 Jun 2020 at 22:43, Rayagonda Kokatanur
> <rayagonda.kokatanur at broadcom.com> wrote:
> >
> > Hi Simon,
> >
> > On Fri, Jun 26, 2020 at 6:42 AM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Rayagonda,
> > >
> > > On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur
> > > <rayagonda.kokatanur at broadcom.com> wrote:
> > > >
> > > > Default "reset" from u-boot to L3 reset.
> > >
> > > U-Boot
> >
> > Thank you, will fix this.
> > >
> > > > "reset" command with argument will trigger L1 reset.
> > > >
> > > > Signed-off-by: Rajesh Ravi <rajesh.ravi at broadcom.com>
> > > > Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty at broadcom.com>
> > > > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur at broadcom.com>
> > > > ---
> > > >  board/broadcom/bcmns3/ns3.c | 20 ++++++++++++++++++--
> > > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c
> > > > index 5e644bd466..1221f26ddc 100644
> > > > --- a/board/broadcom/bcmns3/ns3.c
> > > > +++ b/board/broadcom/bcmns3/ns3.c
> > > > @@ -68,7 +68,23 @@ int dram_init_banksize(void)
> > > >         return 0;
> > > >  }
> > > >
> > > > -void reset_cpu(ulong addr)
> > > > +void reset_cpu(ulong level)
> > > >  {
> > >
> > > This should be in a sysreset driver. It supports different reset types.
> >
> > I checked the sysreset driver and found a generic/common psci driver -
> > drivers/sysreset/sysreset_psci.c.
> > We could use this common driver in our platform.
> >
> > Right now this common driver uses the same command or function_id for
> > both WARM and COLD reset.
> > But in our case we should use different commands for WARM and COLD reset.
> >
> > I am planning to add one kconfig option (USE_FN64_CMD) through which
> > we can select different commands or the same command for COLD reset.
> >
> > Something like this,
> >
> > static int psci_sysreset_request(struct udevice *dev, enum sysreset_t type)
> > {
> >     unsigned long function_id;
> >
> >     switch (type) {
> >     case SYSRESET_WARM:
> >          function_id = PSCI_0_2_FN_SYSTEM_RESET;
> >          break;
> >
> >      case SYSRESET_COLD:
> >          if (CONFIG_IS_ENABLED(USE_FN64_CMD))
> >               function_id = PSCI_0_2_FN64_SYSTEM_RESET;
> >          else
> >               function_id = PSCI_0_2_FN_SYSTEM_RESET;
> >          break;
> >
> >      case SYSRESET_POWER_OFF:
> >          function_id = PSCI_0_2_FN_SYSTEM_OFF;
> >          break;
> >
> >      default:
> >          return -ENOSYS;
> >     }
> >
> >     invoke_psci_fn(function_id, 0, 0, 0);
> >
> >     return -EINPROGRESS;
> > }
> >
> > This way any platform can define/select USE_FN64_CMD if they need
> > different commands for WARM reset.
> > Please let me know about this approach.
>
> It seems OK, but better would be to use the device tree to determine
> the command (e.g. compatible string or a property).

I was checking linux psci drivers (drivers/firmware/psci/psci.c), they
are handling SYSTEM_RESET and SYSTEM_RESET2 without dt ie they read
psci capability, if it supports SYSTEM_RESET2 then they use it else
they use SYSTEM_RESET only.
I think, in U-Boot also we should handle in the same way.
This is a little bit more work, hence I am planning to handle this as
a separate patch set only.
So request you to allow this patch as it is.

Please let me know.

Best regards,
Rayagonda
>
> Regards,
> Simon