Re: [RFC] ACPI table HID/CID allocation

[Udit Kumar <udit.kumar@nxp.com>]

Thanks Ard, 

My intend of this email to know, what is right way to define HID and CID in ACPI firmware i.e 

Device(XYZ) { 
                Name(_HID, "NXP0001") 
                Name(_CID, "PRP0001")
	  Device(Slave1) {
		                Name(_CID, "PRP0001")
                 }
} 
is ok or 


Device(XYZ) { 
                Name(_HID, "NXP0001") 
                Name(_CID, " NXP0001")
	  Device(Slave1) {
		                Name(_CID, " NXP0002")
                 }
} 
Seems good

For sure, AML methods (as needed _ON/OFF/RST/STA etc) /_DSD will to be coded in table using either of.

Please see more in line 

Regards
Udit

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, November 21, 2017 5:59 PM
> To: Udit Kumar <udit.kumar@nxp.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>; edk2-devel@lists.01.org; Varun
> Sethi <V.Sethi@nxp.com>; Daniel Thompson <daniel.thompson@linaro.org>;
> Graeme Gregory <graeme.gregory@linaro.org>
> Subject: Re: [RFC] ACPI table HID/CID allocation
> 
> On 21 November 2017 at 11:32, Udit Kumar <udit.kumar@nxp.com> wrote:
> >
> >> On 21 November 2017 at 09:59, Udit Kumar <udit.kumar@nxp.com> wrote:
> >> > Thanks Ard.
> >> > Below table was for example. I am not converting whole DT to ACPI
> >> > tables :) My idea is to reduce Linux patches for probing as possible.
> >> > Also keeping firmware and OS separately then Let firmware expose
> >> > both way (HID and PRP00001) and Linux to decide binding
> >>
> >> No.
> >>
> >> You are still assuming ACPI and DT device drivers bind at the same
> >> level, and they don't.
> >
> > No, my above comments was just limited to binding.
> 
> Yes, but if you leave it to the OS to decide which binding it uses, you will have to
> support all of them into eternity. And the more detailed binding you need to
> support may limit you in the available choices when it comes to making
> hardware changes, something ACPI allows you to do.

Thanks,  
Is this ok to say , we can provide max same properties in driver as of DT
(with _DSD) , But prefer to use AML methods.
With note, clocking regulators are out of scope here. 

 
> > Right, here device driver should know that device is present in
> > system, I see probe function (device-driver binding) is way to know this.
> > Then driver can execute AML methods exposed by firmware.
> >
> 
> Yes, declaring the presence of the device is the main purpose of describing it
> both in ACPI and in DT.
> 
> >> An ACPI device has AML methods to manage power state and perform
> >> other device related low-level tasks. The device driver has no
> >> knowledge of the hardware beyond what it needs to invoke those abstract
> methods.
> >
> > You meant, If we need to update driver for AML methods then why not to
> update binding as well . ?
> >
> 
> No. What I am saying is that you should not expose two different bindings, and
> let the OS choose.

Ok, got it , then PRP00001 stuff should be used only at urgent or say 
no other choice left , Right ? 
 
> > On side track,
> >  With limited search,  I got many each driver is having (acpi_id and
> > of_id), looks, acpi support is added on the top of DT flavored drivers.
> > and therefore acpi tables are following the same.
> > Sorry to say, reference I am looking at (edk2-platform) JunoPkg and
> > VExpressPkg, Has following devices has description similar to Device tree
> >     Device (NET0) {
> >     Device (SREG) {
> >     Device (VIRT) {
> 
> These were taken from the ACPI tables for an emulator
> 
> >    Device(KMI0) {
> 
> I have no clue what kind of device this is
> 
> >    Device(ETH0) {
> 
> This one uses _DSD as intended, to set device properties in a DT compatible
> fashion, but note that this does *not* include the 'compatible' property, nor
> anything else that ACPI defines itself (status, dma-coherent, etc)

Let me put in simple way,
A simple driver can survive only with _DSD in acpi world. (clocking/regulators are used
as said in UEFI/ACPI)

Copying below from Juno, 
Are below kind of tables are acceptable ? 

    Device(ETH0) {
      Name(_HID, "ARMH9118")
      Name(_UID, Zero)
      Name(_CRS, ResourceTemplate() {
              Memory32Fixed(ReadWrite, 0x18000000, 0x1000)
              Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive) { 192 }
      })
      Name(_DSD, Package() {
                   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
                       Package() {
                               Package(2) {"phy-mode", "mii"},
                               Package(2) {"reg-io-width", 4 },
                               Package(2) {"smsc,irq-active-high",1},
                               Package(2) {"smsc,irq-push-pull",1}
                      }
      }) // _DSD()
    }

> > Where no AML method is exposed. Then I expect OS driver to manage this
> device.
> > While grepping over few other edk2-platforms.  Looks some of methods
> > are abstracted, not whole device.
> >
> 
> So what is your point? Why does this argue in favor of allowing
> PRP0001 + compatible?

I am seeking your help here to define HID and CID,  please see above 
Also for non-NXP devices, how to define HID (if PRP0001 + compatible not to be used) 
 
> >> A DT device describes everything in detail, and requires clock and
> >> regulator drivers and other bits and pieces that are tightly coupled
> >> to details of the hardware.
> >>
> >> So now, you have the worst of both worlds:
> >>
> >> - you need to implement all of this in firmware so ACPI can support
> >> it,
> >> - you have to expose the internals to the OS so DT can support it.
> >
> > Yes, for time being or may be longer, DT support needs to be there
> > along with ACPI introduction.
> >
> > Are you suggesting here to abstract whole device details from OS and
> > expose AML methods to be used by device driver.
> > And maintain two drivers instead of fitting DT style driver into ACPI world ?
> >
> 
> No. You should update the driver so it can support both ACPI and DT bindings.
> That way, the driver can use the abstractions offered by ACPI when it can, and
> can invoke the clock and regulator frameworks and other low level
> infrastructure only when it needs to.

Ok, I am align on this, to have one driver which supports both. 

> Let me try to illustrate this a bit better: imagine a NXP customer that runs a
> datacenter that has 10,000 NXP servers, and is using RHEL x.y. The business is
> going well, and at some point, he wants to order another 2,000 servers.
> Unfortunately, the vendor cannot supply the exact same revision of the
> hardware, and the latest revision uses some component that is not supported in
> RHEL x.y.
> 
> You now have made your customer very unhappy. He invested in RHEL and ACPI
> based servers precisely to avoid this situation. The cost of adding 2,000 servers
> now includes the cost of upgrading the other
> 10,000 servers to a new OS version or the cost of supporting two different OS
> versions at the same time, for a reason that is not justifiable.

Do you mean here with PRP0001 HID/CID, we cannot use AML methods.