diff mbox series

[RFC,6/9] dt-bindings: vendor-prefixes: add a PCI prefix for Qualcomm Atheros

Message ID 20240104130123.37115-7-brgl@bgdev.pl (mailing list archive)
State New, archived
Headers show
Series PCI: introduce the concept of power sequencing of PCIe devices | expand

Commit Message

Bartosz Golaszewski Jan. 4, 2024, 1:01 p.m. UTC
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Document the PCI vendor prefix for Qualcomm Atheros so that we can
define the QCA PCI devices on device tree.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 1 +
 1 file changed, 1 insertion(+)

Comments

Sebastian Reichel Jan. 4, 2024, 2:49 p.m. UTC | #1
Hi,

On Thu, Jan 04, 2024 at 02:01:20PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Document the PCI vendor prefix for Qualcomm Atheros so that we can
> define the QCA PCI devices on device tree.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 2dc098b39234..297d6037cd12 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1128,6 +1128,7 @@ patternProperties:
>    "^purism,.*":
>      description: Purism, SPC
>    "^qca,.*":
> +  "^pci17cb,.*":

I don't think it's a good idea to list all the PCI vendor IDs
in vendor-prefixes.yaml. To please the tooling, I suggest to
have a generic entry instead. Something like this (untested):

"^pci[0-9a-f][0-9a-f][0-9a-f][0-9a-f],.*":
  description: PCI SIG Vendor ID

Note, that we we already have a bunch of them:

grep -ho 'pci[0-9a-f][0-9a-f][0-9a-f][0-9a-f],' **/*.dts* | sort | uniq -c
     70 pci0014,
      3 pci10b5,
      1 pci10ee,
      6 pci14e4,
      1 pci16c3,
      2 pci17a0,
      1 pci17cb,
      1 pci1b4b,
     63 pci8086,

Greetings,

-- Sebastian

>      description: Qualcomm Atheros, Inc.
>    "^qcom,.*":
>      description: Qualcomm Technologies, Inc
> -- 
> 2.40.1
> 
>
Bartosz Golaszewski Jan. 8, 2024, 7:22 p.m. UTC | #2
On Mon, Jan 8, 2024 at 8:10 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Jan 04, 2024 at 02:01:20PM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Document the PCI vendor prefix for Qualcomm Atheros so that we can
> > define the QCA PCI devices on device tree.
>
> Why? vendor-prefixes.yaml is only applied to property names. 'qca'
> should be the prefix for those.
>
> Rob

I didn't have any better idea. PCI devices on DT are defined by their
"pci<vendor ID>,<model ID>" compatible, not regular human-readable
strings and this makes checkpatch.pl complain.

I'm open to suggestions.

Bartosz
Rob Herring (Arm) Jan. 9, 2024, 2:56 a.m. UTC | #3
On Mon, Jan 8, 2024 at 12:22 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Mon, Jan 8, 2024 at 8:10 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Thu, Jan 04, 2024 at 02:01:20PM +0100, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > Document the PCI vendor prefix for Qualcomm Atheros so that we can
> > > define the QCA PCI devices on device tree.
> >
> > Why? vendor-prefixes.yaml is only applied to property names. 'qca'
> > should be the prefix for those.
> >
> > Rob
>
> I didn't have any better idea. PCI devices on DT are defined by their
> "pci<vendor ID>,<model ID>" compatible, not regular human-readable
> strings and this makes checkpatch.pl complain.
>
> I'm open to suggestions.

The checkpatch.pl check predates schemas and we could consider just
dropping it. The only thing it provides is checking a patch rather
than the tree (which the schema do). It's pretty hacky because it just
greps the tree for a compatible string which is not entirely accurate.
Also, we can extract an exact list of compatibles with
"dt-extract-compatibles" which would make a better check, but I'm not
sure making dtschema a dependency on checkpatch would be good.

The other option is just ignore the warning. PCI compatibles are fairly rare.

Rob
Krzysztof Kozlowski Jan. 9, 2024, 9:17 a.m. UTC | #4
On 09/01/2024 03:56, Rob Herring wrote:
> On Mon, Jan 8, 2024 at 12:22 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>
>> On Mon, Jan 8, 2024 at 8:10 PM Rob Herring <robh@kernel.org> wrote:
>>>
>>> On Thu, Jan 04, 2024 at 02:01:20PM +0100, Bartosz Golaszewski wrote:
>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>
>>>> Document the PCI vendor prefix for Qualcomm Atheros so that we can
>>>> define the QCA PCI devices on device tree.
>>>
>>> Why? vendor-prefixes.yaml is only applied to property names. 'qca'
>>> should be the prefix for those.
>>>
>>> Rob
>>
>> I didn't have any better idea. PCI devices on DT are defined by their
>> "pci<vendor ID>,<model ID>" compatible, not regular human-readable
>> strings and this makes checkpatch.pl complain.
>>
>> I'm open to suggestions.
> 
> The checkpatch.pl check predates schemas and we could consider just
> dropping it. The only thing it provides is checking a patch rather
> than the tree (which the schema do). It's pretty hacky because it just
> greps the tree for a compatible string which is not entirely accurate.
> Also, we can extract an exact list of compatibles with
> "dt-extract-compatibles" which would make a better check, but I'm not
> sure making dtschema a dependency on checkpatch would be good.
> 
> The other option is just ignore the warning. PCI compatibles are fairly rare.

Yep, the same warnings are for EEPROM and USB VID/PID compatibles, and
we live with these, so I don't think PCI should be treated differently.

Best regards,
Krzysztof
Bartosz Golaszewski Jan. 9, 2024, 9:30 a.m. UTC | #5
On Tue, Jan 9, 2024 at 10:17 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 09/01/2024 03:56, Rob Herring wrote:
> > On Mon, Jan 8, 2024 at 12:22 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >>
> >> On Mon, Jan 8, 2024 at 8:10 PM Rob Herring <robh@kernel.org> wrote:
> >>>
> >>> On Thu, Jan 04, 2024 at 02:01:20PM +0100, Bartosz Golaszewski wrote:
> >>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >>>>
> >>>> Document the PCI vendor prefix for Qualcomm Atheros so that we can
> >>>> define the QCA PCI devices on device tree.
> >>>
> >>> Why? vendor-prefixes.yaml is only applied to property names. 'qca'
> >>> should be the prefix for those.
> >>>
> >>> Rob
> >>
> >> I didn't have any better idea. PCI devices on DT are defined by their
> >> "pci<vendor ID>,<model ID>" compatible, not regular human-readable
> >> strings and this makes checkpatch.pl complain.
> >>
> >> I'm open to suggestions.
> >
> > The checkpatch.pl check predates schemas and we could consider just
> > dropping it. The only thing it provides is checking a patch rather
> > than the tree (which the schema do). It's pretty hacky because it just
> > greps the tree for a compatible string which is not entirely accurate.
> > Also, we can extract an exact list of compatibles with
> > "dt-extract-compatibles" which would make a better check, but I'm not
> > sure making dtschema a dependency on checkpatch would be good.
> >
> > The other option is just ignore the warning. PCI compatibles are fairly rare.
>
> Yep, the same warnings are for EEPROM and USB VID/PID compatibles, and
> we live with these, so I don't think PCI should be treated differently.
>

Got it, I will drop this patch.

Bart
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 2dc098b39234..297d6037cd12 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1128,6 +1128,7 @@  patternProperties:
   "^purism,.*":
     description: Purism, SPC
   "^qca,.*":
+  "^pci17cb,.*":
     description: Qualcomm Atheros, Inc.
   "^qcom,.*":
     description: Qualcomm Technologies, Inc