| Message ID | 20200305145601.3467-1-m.felsch@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Documentation: ACPI: fix port numbering example | expand |
Hi Marco, Thanks for the patch. On Thu, Mar 05, 2020 at 03:56:01PM +0100, Marco Felsch wrote: > If I understood it right the ports should be numbered using the "port" > property and not the "reg" property. I stumbled over it during > extending the v4l2_fwnode_parse_link() helper which also use the "port" > property. > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > --- > Hi, > > I don't know if this is right since I'm not a ACPI guy *sorry* > Anyway reading the doc description and the v4l2_fwnode_parse_link() code > give me a 2/3 chance. Looking at the documentation, this indeed seems to be a bug in the documentation. The code is right, as is the example. As the property was previously called "port", there is no actual harm even if someone just read the documentation, and not the examples or the code parsing this. The buggy patch is a4138e7c12287268348cc2dcad414a62c515d77a . Could you use this instead? diff --git a/Documentation/firmware-guide/acpi/dsd/graph.rst b/Documentation/firmware-guide/acpi/dsd/graph.rst index 1a6ce7afba5ea..2f19a0487b18c 100644 --- a/Documentation/firmware-guide/acpi/dsd/graph.rst +++ b/Documentation/firmware-guide/acpi/dsd/graph.rst @@ -56,7 +56,7 @@ package would be:: Package() { "endpoint@0", "EP40" } -Each port node contains a property extension key "port", the value of which is +Each port node contains a property extension key "reg", the value of which is the number of the port. Each endpoint is similarly numbered with a property extension key "reg", the value of which is the number of the endpoint. Port numbers must be unique within a device and endpoint numbers must be unique
Hi Sakari, On 20-03-09 15:37, Sakari Ailus wrote: > Hi Marco, > > Thanks for the patch. > > On Thu, Mar 05, 2020 at 03:56:01PM +0100, Marco Felsch wrote: > > If I understood it right the ports should be numbered using the "port" > > property and not the "reg" property. I stumbled over it during > > extending the v4l2_fwnode_parse_link() helper which also use the "port" > > property. > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > --- > > Hi, > > > > I don't know if this is right since I'm not a ACPI guy *sorry* > > Anyway reading the doc description and the v4l2_fwnode_parse_link() code > > give me a 2/3 chance. > > Looking at the documentation, this indeed seems to be a bug in the > documentation. The code is right, as is the example. As the property was > previously called "port", there is no actual harm even if someone just read > the documentation, and not the examples or the code parsing this. > > The buggy patch is a4138e7c12287268348cc2dcad414a62c515d77a . > > Could you use this instead? Of course, thanks for the clarification. It seems that we need to update the v4l2_fwnode_parse_link() too? Regards, Marco > diff --git a/Documentation/firmware-guide/acpi/dsd/graph.rst b/Documentation/firmware-guide/acpi/dsd/graph.rst > index 1a6ce7afba5ea..2f19a0487b18c 100644 > --- a/Documentation/firmware-guide/acpi/dsd/graph.rst > +++ b/Documentation/firmware-guide/acpi/dsd/graph.rst > @@ -56,7 +56,7 @@ package would be:: > > Package() { "endpoint@0", "EP40" } > > -Each port node contains a property extension key "port", the value of which is > +Each port node contains a property extension key "reg", the value of which is > the number of the port. Each endpoint is similarly numbered with a property > extension key "reg", the value of which is the number of the endpoint. Port > numbers must be unique within a device and endpoint numbers must be unique > > -- > Sakari Ailus >
Marco, On Tue, Mar 10, 2020 at 07:14:58AM +0100, Marco Felsch wrote: > Hi Sakari, > > On 20-03-09 15:37, Sakari Ailus wrote: > > Hi Marco, > > > > Thanks for the patch. > > > > On Thu, Mar 05, 2020 at 03:56:01PM +0100, Marco Felsch wrote: > > > If I understood it right the ports should be numbered using the "port" > > > property and not the "reg" property. I stumbled over it during > > > extending the v4l2_fwnode_parse_link() helper which also use the "port" > > > property. > > > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > --- > > > Hi, > > > > > > I don't know if this is right since I'm not a ACPI guy *sorry* > > > Anyway reading the doc description and the v4l2_fwnode_parse_link() code > > > give me a 2/3 chance. > > > > Looking at the documentation, this indeed seems to be a bug in the > > documentation. The code is right, as is the example. As the property was > > previously called "port", there is no actual harm even if someone just read > > the documentation, and not the examples or the code parsing this. > > > > The buggy patch is a4138e7c12287268348cc2dcad414a62c515d77a . > > > > Could you use this instead? > > Of course, thanks for the clarification. It seems that we need to update the > v4l2_fwnode_parse_link() too? Well, yes. This has escaped me because there have been no ACPI users of that function. In fact, there are only two users in total. That suggests it may not be that useful after all as other drivers do the same job without. Feel free to write a patch. :-)
On 20-03-10 10:23, Sakari Ailus wrote: > Marco, > > On Tue, Mar 10, 2020 at 07:14:58AM +0100, Marco Felsch wrote: > > Hi Sakari, > > > > On 20-03-09 15:37, Sakari Ailus wrote: > > > Hi Marco, > > > > > > Thanks for the patch. > > > > > > On Thu, Mar 05, 2020 at 03:56:01PM +0100, Marco Felsch wrote: > > > > If I understood it right the ports should be numbered using the "port" > > > > property and not the "reg" property. I stumbled over it during > > > > extending the v4l2_fwnode_parse_link() helper which also use the "port" > > > > property. > > > > > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > > --- > > > > Hi, > > > > > > > > I don't know if this is right since I'm not a ACPI guy *sorry* > > > > Anyway reading the doc description and the v4l2_fwnode_parse_link() code > > > > give me a 2/3 chance. > > > > > > Looking at the documentation, this indeed seems to be a bug in the > > > documentation. The code is right, as is the example. As the property was > > > previously called "port", there is no actual harm even if someone just read > > > the documentation, and not the examples or the code parsing this. > > > > > > The buggy patch is a4138e7c12287268348cc2dcad414a62c515d77a . > > > > > > Could you use this instead? > > > > Of course, thanks for the clarification. It seems that we need to update the > > v4l2_fwnode_parse_link() too? > > Well, yes. This has escaped me because there have been no ACPI users of > that function. In fact, there are only two users in total. That suggests it > may not be that useful after all as other drivers do the same job without. 3 with the _new_ v4l2-fwnode-connectors ;-) > Feel free to write a patch. :-) K, I will do so. Regards, Marco
diff --git a/Documentation/firmware-guide/acpi/dsd/graph.rst b/Documentation/firmware-guide/acpi/dsd/graph.rst index 1a6ce7afba5e..dcf0102aeb29 100644 --- a/Documentation/firmware-guide/acpi/dsd/graph.rst +++ b/Documentation/firmware-guide/acpi/dsd/graph.rst @@ -97,7 +97,7 @@ A simple example of this is show below:: Name (PRT0, Package() { ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Package () { - Package () { "reg", 0 }, + Package () { "port", 0 }, }, ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"), Package () {
If I understood it right the ports should be numbered using the "port" property and not the "reg" property. I stumbled over it during extending the v4l2_fwnode_parse_link() helper which also use the "port" property. Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> --- Hi, I don't know if this is right since I'm not a ACPI guy *sorry* Anyway reading the doc description and the v4l2_fwnode_parse_link() code give me a 2/3 chance. Regards, Marco Documentation/firmware-guide/acpi/dsd/graph.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)