lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 11 Oct 2022 14:41:29 +0200 From: Andrew Lunn <andrew@...n.ch> To: Soha Jin <soha@...u.info> Cc: 'Giuseppe Cavallaro' <peppe.cavallaro@...com>, 'Alexandre Torgue' <alexandre.torgue@...s.st.com>, 'Jose Abreu' <joabreu@...opsys.com>, "'David S. Miller'" <davem@...emloft.net>, 'Eric Dumazet' <edumazet@...gle.com>, 'Jakub Kicinski' <kuba@...nel.org>, 'Paolo Abeni' <pabeni@...hat.com>, 'Yangyu Chen' <cyy@...self.name>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH 1/3] net: stmmac: use fwnode instead of of to configure driver On Tue, Oct 11, 2022 at 11:12:45AM +0800, Soha Jin wrote: > Hi Andrew, > > > From: Andrew Lunn <andrew@...n.ch> > > Sent: Tuesday, October 11, 2022 4:38 AM > > > > None of these are documented as being valid in ACPI. Do you need to ensure > > they only come from DT, or you document them for ACPI, and get the ACPI > > maintainers to ACK that they are O.K. > > There is _DSD object in ACPI which is used to define Device Specific Data, > and provide additional properties and information to the driver. With > specific UUID listed in _DSD, a package can be used like Device Tree (a > string key associated with a value), and this is also the object > fwnode_property_* will parse with. > > I have tested some of properties with a device describing stmmac device in > ACPI, and it works. These properties should be the configuration to the > driver, and is not related to the way configuring it. Moreover, these are > described in _DSD and not a part of ACPI standard, there seems no need to > ask ACPI maintainers. > > Also, as described in Documentation/firmware-guide/acpi/enumeration.rst, > there is a Device Tree Namespace Link HID (PRP0001) in kernel. PRP0001 can > be used to describe a Device Tree in ACPI's _DSD object, and it just put DT > properties in a _DSD package with the specific UUID I said above. But to > utilize this feature, the driver seems need to use fwnode APIs. The problem i see with ACPI is that everybody does it differently. Each driver defines its own set of properties, which are different to every other driver. You end up with snowflakes, each driver is unique, making it harder to understand, you cannot transfer knowledge from one driver to another. This is fine in the closed sources world of binary blob drivers, you don't get to see other drivers that much. But for Linux, everything is open, and we want you to look at other drivers, copy the good ideas from other driver, make drivers look similar, so they are easy to maintain. So ACPI snowflakes are bad. DT on the other hand, through DT maintainers and general good practices, splits its properties into driver unique and generic properties. If there is a generic property for what you want to express, you us it. Otherwise, you define a vendor property. If you see the same vendor property a few times, you add a generic property, since it is a concept which is shared by a number of drivers. DT documents all its properties. The other problem is that people just seem to think they can stuff DT properties into ACPI and call it done. But ACPI is not DT. DT has a lot of history, things done wrong initially, and then corrected. We don't want to just copy this history into ACPI. We want a clean design, throwing away the history of things done wrongly. So i don't expect ACPI to have any backwards compatibility hacks. As you say, the ACPI standard says nothing about networking, or MDIO busses, or PHYs. Which means we the Linux community can defines how ACPI is used within networking, and we can say snowflakes are not wanted. We can follow the good practices of DT, and document everything. So, please read Documentation/firmware-guide/acpi/dsd/phy.rst. Anything generic to do with PHYs, MDIO, etc should be documented in there. And your driver should not handle any generic properties which are not listed in there. Note that document says nothing about backwards compatibility. Please add an Documentation/firmware-guide/acpi/dsd/ethernet-controller.rst for all the generic properties to do with the MAC driver, similar to the DT document ethernet-controller.rst. I would also suggest you add a document specific to this MAC driver, documenting properties specific to it. Any DT properties which are listed as deprecated, i don't expect to be seen implemented in ACPI. The reset GPIO is a good example of this. Look at its horrible history, how it is totally messed up in DT. Don't copy that mess into ACPI. > > > Backward compatibility only applies to DT. Anybody using ACPI should not > > expect any backwards compatibility, they should be documented mandatory > > properties right from the beginning. > > Just do not want to mix the use of OF and fwnode APIs, I simply changed all > OF property APIs with fwnode APIs. If you really think the backward > compatibility should not exist in ACPI, I will change these compatible > codes back to OF APIs. Because ACPI is not DT, sometimes you do need to handle them differently. If you have the exact same property in DT and ACPI, you are just stuffing DT into ACPI, yes, you can use the fwnode APIs. But when ACPI and DT differ, you need to use the lower level APIs to deal with these differences. Andrew
Powered by blists - more mailing lists