Re: [RFC/PATCH v2 09/16] soc: bcm: bcm2835-power: Add support for BCM2711's Argon ASB

[Phil Elwell]

Nicolas,


On Tue, 9 Feb 2021 at 14:00, Nicolas Saenz Julienne
<nsaenzjulienne@xxxxxxx> wrote:
>
> On Tue, 2021-02-09 at 13:19 +0000, Phil Elwell wrote:
> > Hi Nicolas,
> >
> > On Tue, 9 Feb 2021 at 13:00, Nicolas Saenz Julienne
> > <nsaenzjulienne@xxxxxxx> wrote:
> > >
> > > In BCM2711 the new ARGON ASB took over V3D. The old ASB is still present
> > > with the ISP and H264 bits, and V3D is in the same place in the new ASB
> > > as the old one.
> > >
> > > Use the fact that 'pm->argon_asb' is populated as a hint that we're on
> > > BCM2711. On top of that introduce the macro ASB_BASE() which will select
> > > the correct ASB register base, based on whether we're trying to access
> > > V3D and which platform we're on.
> >
> > Please don't refer to this block as ARGON - it is the IP of Raspberry
> > Pi Trading and it's name is RPiVid.
>
> OK, sorry for that. I, again, mixed both ASB names. I'll rename the Argon ASB
> to RPiVid. How should I call the one present in older RPis?

It can keep its unqualified name of ASB_*.

>
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx>
> > >
> > > ---
> > >
> > > Changes since v1:
> > >  - Correct names
> > >
> > >  drivers/soc/bcm/bcm2835-power.c | 68 ++++++++++++++++++++-------------
> > >  1 file changed, 42 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/drivers/soc/bcm/bcm2835-power.c b/drivers/soc/bcm/bcm2835-power.c
> > > index 59b8abfc5617..42e105758b47 100644
> > > --- a/drivers/soc/bcm/bcm2835-power.c
>
> [...]
>
> > >         case BCM2835_POWER_DOMAIN_USB:
> > >                 PM_WRITE(PM_USB, 0);
> > > @@ -626,13 +633,22 @@ static int bcm2835_power_probe(struct platform_device *pdev)
> > >         power->dev = dev;
> > >         power->base = pm->base;
> > >         power->rpivid_asb = pm->rpivid_asb;
> > > +       power->argon_asb = pm->argon_asb;
> > >
> > > -       id = ASB_READ(ASB_AXI_BRDG_ID);
> > > +       id = ASB_READ(ASB_AXI_BRDG_ID, false);
> > >         if (id != 0x62726467 /* "BRDG" */) {
> > > -               dev_err(dev, "ASB register ID returned 0x%08x\n", id);
> > > +               dev_err(dev, "RPiVid ASB register ID returned 0x%08x\n", id);
> > >                 return -ENODEV;
> > >         }
> > >
> > > +       if (pm->argon_asb) {
> > > +               id = ASB_READ(ASB_AXI_BRDG_ID, true);
> > > +               if (id != 0x62726467 /* "BRDG" */) {
> > > +                       dev_err(dev, "Argon ASB register ID returned 0x%08x\n", id);
> > > +                       return -ENODEV;
> > > +               }
> > > +       }
> > > +
> >
> > Surely these are the same register. Is this the result of a bad merge?
>
> AFAIU There are two ASBs the old one at 0x7e00a000 and the new RPiVid one at
> 0x7ec11000. They both can be checked for valid IDs. Note the new argument in
> ASB_READ().

You're right - I'd missed the parameter. Apologies.

Phil