From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c06::232; helo=mail-io0-x232.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io0-x232.google.com (mail-io0-x232.google.com [IPv6:2607:f8b0:4001:c06::232]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4244921B00DC1 for ; Tue, 21 Nov 2017 05:59:41 -0800 (PST) Received: by mail-io0-x232.google.com with SMTP id q101so19513283ioi.1 for ; Tue, 21 Nov 2017 06:03:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=ezMhQ+fTK4WJNOSCcdo7+WLpAIbNEuVplQZ+D9Yinlc=; b=Olg7DX3Ho/hZfNCmO6x0uDJ/TtFqCcwlk1fn6xb2RdkGwQ9xJ8z1yFlkiuTGlUGuE1 w9tPIFyJUi1gG0jRn7xXuUbWBKsay4pvOhKFFJWw2YNvoAyGKdF1bAXlTdm8s10sR0GR L8GplwVI6fzv099Otyeg0kqQXJP8x/1Y6yr50= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=ezMhQ+fTK4WJNOSCcdo7+WLpAIbNEuVplQZ+D9Yinlc=; b=sA+G7cx5cImBoFhukJofFjHE0nUqYQ3cwrlXTSTiyRJb1X7qexncZA4ZBR3iWG7+Ll db6XMDyuGZTCyk5D4v02YhJzCnIc3mr0oVBcsBhLlaPxT8O7yWA+DSE7NO0gcA7T8+S7 96LP5JTxm5hHtK21pp4gdIaUqKQQiCUg+160102UjK1+5BF5BWoCVgeKpz4b8/G+xR+N mcAsFsnEkFE4GfHVeZU80w09poYFsE7Kek2gZQguvrk1Q1BEMYhJr/tGMWuMixUYayED jVU5C8kn/KGNBKfLQNN838cwavJEmxyftC2obbC/Sq3pjSrNnkph6oyLI44cQVRQH25P ZkjQ== X-Gm-Message-State: AJaThX69GvbAvXLeTxIbYlm45oJLOHpC4p21//HdkeBHlhv7CsThvk4T vVm5mPF07aXnKjqLxxl9qx7w1//gBItB5rBK1U6SZg== X-Google-Smtp-Source: AGs4zMYWbV99wvfix+riGF2VvOENMPEt7KyYmhzk+W2HezhpT7dbASN82nSeLoyVKClvQ3iSVHUtzsKyKF2SY3k9TWA= X-Received: by 10.107.2.137 with SMTP id 131mr19110790ioc.186.1511273036207; Tue, 21 Nov 2017 06:03:56 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.16 with HTTP; Tue, 21 Nov 2017 06:03:55 -0800 (PST) In-Reply-To: References: From: Ard Biesheuvel Date: Tue, 21 Nov 2017 14:03:55 +0000 Message-ID: To: Udit Kumar Cc: Leif Lindholm , "edk2-devel@lists.01.org" , Varun Sethi , Daniel Thompson , Graeme Gregory Subject: Re: [RFC] ACPI table HID/CID allocation X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Nov 2017 13:59:42 -0000 Content-Type: text/plain; charset="UTF-8" On 21 November 2017 at 13:24, Udit Kumar 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. > 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 >> Cc: Leif Lindholm ; edk2-devel@lists.01.org; Varun >> Sethi ; Daniel Thompson ; >> Graeme Gregory >> Subject: Re: [RFC] ACPI table HID/CID allocation >> >> On 21 November 2017 at 11:32, Udit Kumar wrote: >> > >> >> On 21 November 2017 at 09:59, Udit Kumar 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. > >> > 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. > 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 https://www.kernel.org/doc/Documentation/acpi/DSD-properties-rules.txt >> > 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 >> >> 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.