Re: [net-next: PATCH 09/12] Documentation: ACPI: DSD: introduce DSA description

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, Jun 21, 2022 at 01:18:38PM +0200, Andrew Lunn wrote:
> On Tue, Jun 21, 2022 at 02:09:14PM +0300, Andy Shevchenko wrote:
> > On Mon, Jun 20, 2022 at 09:47:31PM +0200, Andrew Lunn wrote:

...

> > > > +        Name (_CRS, ResourceTemplate ()
> > > > +        {
> > > > +            Memory32Fixed (ReadWrite,
> > > > +                0xf212a200,
> > > > +                0x00000010,
> > > 
> > > What do these magic numbers mean?
> > 
> > Address + Length, it's all described in the ACPI specification.
> 
> The address+plus length of what? This device is on an MDIO bus. As
> such, there is no memory! It probably makes sense to somebody who
> knows ACPI, but to me i have no idea what it means.

I see what you mean. Honestly I dunno what the device this description is for.
For the DSA that's behind MDIO bus? Then it's definitely makes no sense and
MDIOSerialBus() resources type is what would be good to have in ACPI
specification.

> > Or if you asked
> > why the values there are the particular numbers? I guess it's fined to have
> > anything sane in the example. OTOH a comment may be added.
> > 
> > > > +                )
> > > > +        })

...

> > > > +        Device (SWI0)
> > > > +        {
> > > > +            Name (_HID, "MRVL0120")
> > > > +            Name (_UID, 0x00)
> > > > +            Name (_ADR, 0x4)
> > > > +            <...>
> > > > +        }
> > > 
> > > I guess it is not normal for ACPI, but could you add some comments
> > > which explain this. In DT we have
> > > 
> > >     properties:
> > >       reg:
> > >         minimum: 0
> > >         maximum: 31
> > >         description:
> > >           The ID number for the device.
> > > 
> > > which i guess what this _ADR property is, but it would be nice if it
> > > actually described what it is supposed to mean. You have a lot of
> > > undocumented properties here.
> > 
> > Btw, you are right, _ADR mustn't go together with _HID/_CID.
> 
> Does ACPI have anything like .yaml to describe the binding and tools
> to validate it?

ACPI is a language, the "bindings" (in a way how you probably use this term)
are a small subset and usually refers to the format whoever provides them. I.e.
_DSD with a certain UUID is a part of the DT bindings, so DT validation is what
is expected to be performed. Otherwise for other UUIDs it may or may be no such
validators exist, I don't know.

-- 
With Best Regards,
Andy Shevchenko