Thanks Ard, For internal SOC devices, this is perfectly ok to drop PRP0001 from CID.
> This could be a valid reason to use PRP0001 + compatible, for things like I2C > slaves that are external to the SoC For external devices (for which HID is not available), you suggest to go with PRP0001 + compatible or that device driver needs add ACPI HID support. As you pointed out, are external devices to SOC such exception to use PRP0001 + compatible be it i2c slave or SPI slave ? Regards Udit > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: Tuesday, November 21, 2017 7:34 PM > To: Udit Kumar <udit.ku...@nxp.com> > Cc: Leif Lindholm <leif.lindh...@linaro.org>; edk2-devel@lists.01.org; Varun > Sethi <v.se...@nxp.com>; Daniel Thompson <daniel.thomp...@linaro.org>; > Graeme Gregory <graeme.greg...@linaro.org> > Subject: Re: [RFC] ACPI table HID/CID allocation > > On 21 November 2017 at 13:24, Udit Kumar <udit.ku...@nxp.com> wrote: > > 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 > > > > I don't think it makes a lot of sense to use the same value for _HID and > _CID, so > you can just drop the latter. Sure, > > 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.biesheu...@linaro.org] > >> Sent: Tuesday, November 21, 2017 5:59 PM > >> To: Udit Kumar <udit.ku...@nxp.com> > >> Cc: Leif Lindholm <leif.lindh...@linaro.org>; > >> edk2-devel@lists.01.org; Varun Sethi <v.se...@nxp.com>; Daniel > >> Thompson <daniel.thomp...@linaro.org>; Graeme Gregory > >> <graeme.greg...@linaro.org> > >> Subject: Re: [RFC] ACPI table HID/CID allocation > >> > >> On 21 November 2017 at 11:32, Udit Kumar <udit.ku...@nxp.com> wrote: > >> > > >> >> On 21 November 2017 at 09:59, Udit Kumar <udit.ku...@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. > > > > Yes. _DSD may be used to describe device specific data that goes beyond what > ACPI can express natively. Using _DSD to describe clocks and regulators is an > absolute no-go. Putting things like "status" or "dma-coherent" in _DSD > properties is absolutely unacceptable as well. > But even things like initialization data that the driver programs into the > device a > single time really do not belong in _DSD. Instead, it should be the firmware > that > initializes the device, and presents it to the OS in its initialized state. > Agreed, I never meant something to add in DSD which was prohibited in ACPI spces. > > > >> > 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 ? > > > > Yes. > >> > 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) > > > > Why can a simple driver only survive with _DSD? That statement does not make > any sense to me. Why so, please see below one for example > > 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() > > } > > > > Yes. But please be aware that you should not simply invent your own properties > here. The _DSD namespace was intended to be managed, and not free for all Agreed, I didn’t meant to add something new, which is not available at present, > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.k > ernel.org%2Fdoc%2FDocumentation%2Facpi%2FDSD-properties- > rules.txt&data=02%7C01%7Cudit.kumar%40nxp.com%7C164c1ff7350a4f6373e > e08d530e8b591%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63646 > 8698397705869&sdata=O78k8r6tcK9fwpuTuQ82ZXGiWkBtLduf4bqrM6D6L1U% > 3D&reserved=0 > > >> > 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) > > > > This could be a valid reason to use PRP0001 + compatible, for things like I2C > slaves that are external to the SoC Well, for internal SOC devices, I am in agreement to use NXP specific HIDs But for external devices (for which HID is not available), you suggest to go With PRP0001 + compatible > >> >> 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. > > You cannot use the abstractions ACPI provides when using PRP0001 + > compatible. Oh, thx _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
