| Message ID | 20220511094024.175994-2-shreeya.patel@collabora.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add LTRF216A Driver | expand |
On Wed, May 11, 2022 at 03:10:22PM +0530, Shreeya Patel wrote: > 'liteon' is the correct vendor prefix for devices released by > LITE-ON Technology Corp. But one of the released device which uses > ltr216a light sensor exposes the vendor prefix name as 'ltr' through > ACPI. ACPI? NAK. There are no cases of 'ltr' for DT, so fix ACPI. > > Hence, add 'ltr' as a deprecated vendor prefix which would suppress the > following warning in case the compatible string used in ltrf216a driver > is "ltr,ltrf216a" > > WARNING: DT compatible string vendor "ltr" appears un-documented -- > check ./Documentation/devicetree/bindings/vendor-prefixes.yaml > 364: FILE: drivers/iio/light/ltrf216a.c:313: > + { .compatible = "ltr,ltrf216a" }, > > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> > --- > > Changes in v2 > - Add vendor prefix name as per the alphabetical order. > > Documentation/devicetree/bindings/vendor-prefixes.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml > index 01430973ecec..02f94fba03b6 100644 > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml > @@ -716,6 +716,9 @@ patternProperties: > description: Loongson Technology Corporation Limited > "^lsi,.*": > description: LSI Corp. (LSI Logic) > + "^ltr,.*": > + description: LITE-ON Technology Corp. > + deprecated: true > "^lwn,.*": > description: Liebherr-Werk Nenzing GmbH > "^lxa,.*": > -- > 2.30.2 > >
On 16/05/22 22:30, Rob Herring wrote: > On Wed, May 11, 2022 at 03:10:22PM +0530, Shreeya Patel wrote: >> 'liteon' is the correct vendor prefix for devices released by >> LITE-ON Technology Corp. But one of the released device which uses >> ltr216a light sensor exposes the vendor prefix name as 'ltr' through >> ACPI. > ACPI? NAK. > > There are no cases of 'ltr' for DT, so fix ACPI. Hi Rob, Yes, we understand there are no cases of 'ltr', but we have released devices which uses this string for probing the ltrf216a light sensor driver ( x86 with DT ) If we don't document this in vendor-prefixes.yaml, then the following warning is generated. WARNING: DT compatible string vendor "ltr" appears un-documented -- check ./Documentation/devicetree/bindings/vendor-prefixes.yaml 364: FILE: drivers/iio/light/ltrf216a.c:313: + { .compatible = "ltr,ltrf216a" }, Can you suggest us what would be the right way to fix this warning if not documenting in vendor-prefixes.yaml? Thanks, Shreeya Patel > >> Hence, add 'ltr' as a deprecated vendor prefix which would suppress the >> following warning in case the compatible string used in ltrf216a driver >> is "ltr,ltrf216a" >> >> WARNING: DT compatible string vendor "ltr" appears un-documented -- >> check ./Documentation/devicetree/bindings/vendor-prefixes.yaml >> 364: FILE: drivers/iio/light/ltrf216a.c:313: >> + { .compatible = "ltr,ltrf216a" }, >> >> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> >> Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> >> --- >> >> Changes in v2 >> - Add vendor prefix name as per the alphabetical order. >> >> Documentation/devicetree/bindings/vendor-prefixes.yaml | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml >> index 01430973ecec..02f94fba03b6 100644 >> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml >> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml >> @@ -716,6 +716,9 @@ patternProperties: >> description: Loongson Technology Corporation Limited >> "^lsi,.*": >> description: LSI Corp. (LSI Logic) >> + "^ltr,.*": >> + description: LITE-ON Technology Corp. >> + deprecated: true >> "^lwn,.*": >> description: Liebherr-Werk Nenzing GmbH >> "^lxa,.*": >> -- >> 2.30.2 >> >>
On Tue, May 17, 2022 at 04:07:33PM +0530, Shreeya Patel wrote: > > On 16/05/22 22:30, Rob Herring wrote: > > On Wed, May 11, 2022 at 03:10:22PM +0530, Shreeya Patel wrote: > > > 'liteon' is the correct vendor prefix for devices released by > > > LITE-ON Technology Corp. But one of the released device which uses > > > ltr216a light sensor exposes the vendor prefix name as 'ltr' through > > > ACPI. > > ACPI? NAK. > > > > There are no cases of 'ltr' for DT, so fix ACPI. > > Hi Rob, > > Yes, we understand there are no cases of 'ltr', but we have released devices > which uses this string for probing the ltrf216a light sensor driver ( x86 > with DT ) That's not what your commit message says. Even if this is DT based, given an undocumented vendor string is used, it seems doubtful the rest of the binding would match upstream. What about the rest of the DTB? Got a pointer to it or want to publish it? > If we don't document this in vendor-prefixes.yaml, then the following > warning > is generated. > > WARNING: DT compatible string vendor "ltr" appears un-documented -- check > ./Documentation/devicetree/bindings/vendor-prefixes.yaml 364: FILE: > drivers/iio/light/ltrf216a.c:313: + { .compatible = "ltr,ltrf216a" }, > > > Can you suggest us what would be the right way to fix this warning if not > documenting > in vendor-prefixes.yaml? Fix the DT. We don't accept bindings simply because they are already used in the field. If this was the only issue, it would be fine, but I suspect it's the tip of the iceberg. Rob
On 18/05/22 22:02, Rob Herring wrote: > On Tue, May 17, 2022 at 04:07:33PM +0530, Shreeya Patel wrote: >> On 16/05/22 22:30, Rob Herring wrote: >>> On Wed, May 11, 2022 at 03:10:22PM +0530, Shreeya Patel wrote: >>>> 'liteon' is the correct vendor prefix for devices released by >>>> LITE-ON Technology Corp. But one of the released device which uses >>>> ltr216a light sensor exposes the vendor prefix name as 'ltr' through >>>> ACPI. >>> ACPI? NAK. >>> >>> There are no cases of 'ltr' for DT, so fix ACPI. >> Hi Rob, >> >> Yes, we understand there are no cases of 'ltr', but we have released devices >> which uses this string for probing the ltrf216a light sensor driver ( x86 >> with DT ) > That's not what your commit message says. > > Even if this is DT based, given an undocumented vendor string is used, > it seems doubtful the rest of the binding would match upstream. What > about the rest of the DTB? Got a pointer to it or want to publish it? > >> If we don't document this in vendor-prefixes.yaml, then the following >> warning >> is generated. >> >> WARNING: DT compatible string vendor "ltr" appears un-documented -- check >> ./Documentation/devicetree/bindings/vendor-prefixes.yaml 364: FILE: >> drivers/iio/light/ltrf216a.c:313: + { .compatible = "ltr,ltrf216a" }, >> >> >> Can you suggest us what would be the right way to fix this warning if not >> documenting >> in vendor-prefixes.yaml? > Fix the DT. We don't accept bindings simply because they are already > used in the field. If this was the only issue, it would be fine, but I > suspect it's the tip of the iceberg. Hi Rob, To make things more clear, following is the modalias info of the device. (B+)(root@linux iio:device0)# cat /sys/bus/i2c/devices/i2c-PRP0001\:01/modalias of:NltrfTCltr,ltrf216a It's a dt namespace on an ACPI based device. We used an of_device_id table to be able to probe the driver using the vendor prefix and compatible string. But when we compile the driver, we get the following warning and hence we documented it in vendor-prefixes.yaml and also added a complete device tree file [Patch 3/3] just to get rid of the warning. In real life we are not using the device tree file at all. WARNING: DT compatible string vendor "ltr" appears un-documented -- check ./Documentation/devicetree/bindings/vendor-prefixes.yaml 364: FILE: drivers/iio/light/ltrf216a.c:313: + { .compatible = "ltr,ltrf216a" }, There are many existing devices used by people which has the vendor prefix name as 'ltr' and it won't be possible to change that hence we are trying to upstream it. Thanks, Shreeya Patel > > Rob >
On Mon, May 23, 2022 at 08:27:56PM +0530, Shreeya Patel wrote: > > On 18/05/22 22:02, Rob Herring wrote: > > On Tue, May 17, 2022 at 04:07:33PM +0530, Shreeya Patel wrote: > > > On 16/05/22 22:30, Rob Herring wrote: > > > > On Wed, May 11, 2022 at 03:10:22PM +0530, Shreeya Patel wrote: > > > > > 'liteon' is the correct vendor prefix for devices released by > > > > > LITE-ON Technology Corp. But one of the released device which uses > > > > > ltr216a light sensor exposes the vendor prefix name as 'ltr' through > > > > > ACPI. > > > > ACPI? NAK. > > > > > > > > There are no cases of 'ltr' for DT, so fix ACPI. > > > Hi Rob, > > > > > > Yes, we understand there are no cases of 'ltr', but we have released devices > > > which uses this string for probing the ltrf216a light sensor driver ( x86 > > > with DT ) > > That's not what your commit message says. > > > > Even if this is DT based, given an undocumented vendor string is used, > > it seems doubtful the rest of the binding would match upstream. What > > about the rest of the DTB? Got a pointer to it or want to publish it? > > > > > If we don't document this in vendor-prefixes.yaml, then the following > > > warning > > > is generated. > > > > > > WARNING: DT compatible string vendor "ltr" appears un-documented -- check > > > ./Documentation/devicetree/bindings/vendor-prefixes.yaml 364: FILE: > > > drivers/iio/light/ltrf216a.c:313: + { .compatible = "ltr,ltrf216a" }, > > > > > > > > > Can you suggest us what would be the right way to fix this warning if not > > > documenting > > > in vendor-prefixes.yaml? > > Fix the DT. We don't accept bindings simply because they are already > > used in the field. If this was the only issue, it would be fine, but I > > suspect it's the tip of the iceberg. > > > Hi Rob, > > To make things more clear, following is the modalias info of the device. > > (B+)(root@linux iio:device0)# cat > /sys/bus/i2c/devices/i2c-PRP0001\:01/modalias > of:NltrfTCltr,ltrf216a > > It's a dt namespace on an ACPI based device. We used an of_device_id table > to be able to probe the driver > using the vendor prefix and compatible string. Again, it's ACPI so I don't care. If someone cares about using DT bindings in ACPI they can step up and help maintain them. It's not a DT vs. ACPI thing, but just that I can only maintain so much and have to draw the line somewhere. > But when we compile the driver, we get the following warning and hence we > documented it in vendor-prefixes.yaml > and also added a complete device tree file [Patch 3/3] just to get rid of > the warning. In real life we are not using > the device tree file at all. > > WARNING: DT compatible string vendor "ltr" appears un-documented -- check > ./Documentation/devicetree/bindings/vendor-prefixes.yaml 364: FILE: > drivers/iio/light/ltrf216a.c:313: + { .compatible = "ltr,ltrf216a" }, So, is someone telling you to fix this? > There are many existing devices used by people which has the vendor prefix > name as 'ltr' > and it won't be possible to change that hence we are trying to upstream it. There are millions if not billions of DT based devices using undocumented bindings. If those used "ltr,ltrf216a", I wouldn't accept it either. Rob
On 24/05/22 21:17, Rob Herring wrote: > On Mon, May 23, 2022 at 08:27:56PM +0530, Shreeya Patel wrote: >> On 18/05/22 22:02, Rob Herring wrote: >>> On Tue, May 17, 2022 at 04:07:33PM +0530, Shreeya Patel wrote: >>>> On 16/05/22 22:30, Rob Herring wrote: >>>>> On Wed, May 11, 2022 at 03:10:22PM +0530, Shreeya Patel wrote: >>>>>> 'liteon' is the correct vendor prefix for devices released by >>>>>> LITE-ON Technology Corp. But one of the released device which uses >>>>>> ltr216a light sensor exposes the vendor prefix name as 'ltr' through >>>>>> ACPI. >>>>> ACPI? NAK. >>>>> >>>>> There are no cases of 'ltr' for DT, so fix ACPI. >>>> Hi Rob, >>>> >>>> Yes, we understand there are no cases of 'ltr', but we have released devices >>>> which uses this string for probing the ltrf216a light sensor driver ( x86 >>>> with DT ) >>> That's not what your commit message says. >>> >>> Even if this is DT based, given an undocumented vendor string is used, >>> it seems doubtful the rest of the binding would match upstream. What >>> about the rest of the DTB? Got a pointer to it or want to publish it? >>> >>>> If we don't document this in vendor-prefixes.yaml, then the following >>>> warning >>>> is generated. >>>> >>>> WARNING: DT compatible string vendor "ltr" appears un-documented -- check >>>> ./Documentation/devicetree/bindings/vendor-prefixes.yaml 364: FILE: >>>> drivers/iio/light/ltrf216a.c:313: + { .compatible = "ltr,ltrf216a" }, >>>> >>>> >>>> Can you suggest us what would be the right way to fix this warning if not >>>> documenting >>>> in vendor-prefixes.yaml? >>> Fix the DT. We don't accept bindings simply because they are already >>> used in the field. If this was the only issue, it would be fine, but I >>> suspect it's the tip of the iceberg. >> >> Hi Rob, >> >> To make things more clear, following is the modalias info of the device. >> >> (B+)(root@linux iio:device0)# cat >> /sys/bus/i2c/devices/i2c-PRP0001\:01/modalias >> of:NltrfTCltr,ltrf216a >> >> It's a dt namespace on an ACPI based device. We used an of_device_id table >> to be able to probe the driver >> using the vendor prefix and compatible string. > Again, it's ACPI so I don't care. If someone cares about using DT > bindings in ACPI they can step up and help maintain them. It's not a DT > vs. ACPI thing, but just that I can only maintain so much and have to > draw the line somewhere. > >> But when we compile the driver, we get the following warning and hence we >> documented it in vendor-prefixes.yaml >> and also added a complete device tree file [Patch 3/3] just to get rid of >> the warning. In real life we are not using >> the device tree file at all. >> >> WARNING: DT compatible string vendor "ltr" appears un-documented -- check >> ./Documentation/devicetree/bindings/vendor-prefixes.yaml 364: FILE: >> drivers/iio/light/ltrf216a.c:313: + { .compatible = "ltr,ltrf216a" }, > So, is someone telling you to fix this? So will it be right to just keep the warning and remove this patch? Is there a way you know to silent this warning? Thanks, Shreeya Patel > > >> There are many existing devices used by people which has the vendor prefix >> name as 'ltr' >> and it won't be possible to change that hence we are trying to upstream it. > There are millions if not billions of DT based devices using > undocumented bindings. If those used "ltr,ltrf216a", I wouldn't accept > it either. > > Rob >
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 01430973ecec..02f94fba03b6 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -716,6 +716,9 @@ patternProperties: description: Loongson Technology Corporation Limited "^lsi,.*": description: LSI Corp. (LSI Logic) + "^ltr,.*": + description: LITE-ON Technology Corp. + deprecated: true "^lwn,.*": description: Liebherr-Werk Nenzing GmbH "^lxa,.*":
'liteon' is the correct vendor prefix for devices released by LITE-ON Technology Corp. But one of the released device which uses ltr216a light sensor exposes the vendor prefix name as 'ltr' through ACPI. Hence, add 'ltr' as a deprecated vendor prefix which would suppress the following warning in case the compatible string used in ltrf216a driver is "ltr,ltrf216a" WARNING: DT compatible string vendor "ltr" appears un-documented -- check ./Documentation/devicetree/bindings/vendor-prefixes.yaml 364: FILE: drivers/iio/light/ltrf216a.c:313: + { .compatible = "ltr,ltrf216a" }, Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> --- Changes in v2 - Add vendor prefix name as per the alphabetical order. Documentation/devicetree/bindings/vendor-prefixes.yaml | 3 +++ 1 file changed, 3 insertions(+)