diff mbox

[02/24] Documentation: DT bindings: add more chip compatible strings for Tegra PCIe

Message ID 20150128234937.20644.9400.stgit@dusk.lan
State Not Applicable
Headers show

Commit Message

Paul Walmsley Jan. 28, 2015, 11:49 p.m. UTC
Add compatible strings for the PCIe IP blocks present on several Tegra
chips.  The primary objective here is to avoid checkpatch warnings,
per:

http://marc.info/?l=linux-tegra&m=142201349727836&w=2

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Cc: linux-tegra@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 .../bindings/pci/nvidia,tegra20-pcie.txt           |    2 ++
 1 file changed, 2 insertions(+)



--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Rob Herring Jan. 29, 2015, 2:45 p.m. UTC | #1
On Wed, Jan 28, 2015 at 5:49 PM, Paul Walmsley <paul@pwsan.com> wrote:
>
> Add compatible strings for the PCIe IP blocks present on several Tegra
> chips.  The primary objective here is to avoid checkpatch warnings,
> per:
>
> http://marc.info/?l=linux-tegra&m=142201349727836&w=2
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: linux-tegra@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  .../bindings/pci/nvidia,tegra20-pcie.txt           |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> index d763e047c6ae..e772884f1c33 100644
> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> @@ -5,6 +5,8 @@ Required properties:
>    - "nvidia,tegra20-pcie"
>    - "nvidia,tegra30-pcie"
>    - "nvidia,tegra124-pcie"
> +  - "nvidia,tegra132-pcie" (not yet matched in the driver)
> +  - "nvidia,tegra210-pcie" (not yet matched in the driver)

Whether the driver matches or not is irrelevant to the binding and may
change over time. Does this mean the driver matches on something else
or Tegra132 is not yet supported in the driver? If the former, what is
important is what are the valid combinations of compatible properties
and that is not captured here. In other words, what is the fallback
compatible string for each chip?

The same comments apply to the rest of the series.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Walmsley Jan. 29, 2015, 3:45 p.m. UTC | #2
Hi Rob

On Thu, 29 Jan 2015, Rob Herring wrote:

> On Wed, Jan 28, 2015 at 5:49 PM, Paul Walmsley <paul@pwsan.com> wrote:
> >
> > Add compatible strings for the PCIe IP blocks present on several Tegra
> > chips.  The primary objective here is to avoid checkpatch warnings,
> > per:
> >
> > http://marc.info/?l=linux-tegra&m=142201349727836&w=2
> >
> > Signed-off-by: Paul Walmsley <paul@pwsan.com>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Pawel Moll <pawel.moll@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> > Cc: Kumar Gala <galak@codeaurora.org>
> > Cc: Stephen Warren <swarren@wwwdotorg.org>
> > Cc: Alexandre Courbot <gnurou@gmail.com>
> > Cc: linux-tegra@vger.kernel.org
> > Cc: linux-pci@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> >  .../bindings/pci/nvidia,tegra20-pcie.txt           |    2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> > index d763e047c6ae..e772884f1c33 100644
> > --- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
> > @@ -5,6 +5,8 @@ Required properties:
> >    - "nvidia,tegra20-pcie"
> >    - "nvidia,tegra30-pcie"
> >    - "nvidia,tegra124-pcie"
> > +  - "nvidia,tegra132-pcie" (not yet matched in the driver)
> > +  - "nvidia,tegra210-pcie" (not yet matched in the driver)
> 
> Whether the driver matches or not is irrelevant to the binding and may
> change over time. Does this mean the driver matches on something else
> or Tegra132 is not yet supported in the driver? 

It means that the driver currently matches on one of the first three 
strings that don't carry that annotation.

> If the former, what is important is what are the valid combinations of 
> compatible properties and that is not captured here. In other words, 
> what is the fallback compatible string for each chip?

The intention was to try to be helpful: to document that anyone adding a 
"nvidia,tegra132-pcie" compatible string would also need to add one of the 
other strings as a fallback.  Would you like that to be documented in a 
different way, or removed?


- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring Jan. 29, 2015, 4:44 p.m. UTC | #3
On Thu, Jan 29, 2015 at 9:45 AM, Paul Walmsley <paul@pwsan.com> wrote:
> Hi Rob
>
> On Thu, 29 Jan 2015, Rob Herring wrote:
>
>> On Wed, Jan 28, 2015 at 5:49 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> >
>> > Add compatible strings for the PCIe IP blocks present on several Tegra
>> > chips.  The primary objective here is to avoid checkpatch warnings,
>> > per:
>> >

[...]

>> > +  - "nvidia,tegra132-pcie" (not yet matched in the driver)
>> > +  - "nvidia,tegra210-pcie" (not yet matched in the driver)
>>
>> Whether the driver matches or not is irrelevant to the binding and may
>> change over time. Does this mean the driver matches on something else
>> or Tegra132 is not yet supported in the driver?
>
> It means that the driver currently matches on one of the first three
> strings that don't carry that annotation.
>
>> If the former, what is important is what are the valid combinations of
>> compatible properties and that is not captured here. In other words,
>> what is the fallback compatible string for each chip?
>
> The intention was to try to be helpful: to document that anyone adding a
> "nvidia,tegra132-pcie" compatible string would also need to add one of the
> other strings as a fallback.  Would you like that to be documented in a
> different way, or removed?

Then you should say something like 'must contain "nvidia,tegra20-pcie"
and one of: ...'

You can also use nvidia,<chip>-pcie if you want. checkpatch will check
for that pattern too. Then your documentation can be something like:

Must contain '"nvidia,<chip>-pcie", "nvidia,tegra20-pcie"' where
<chip> is tegra30, tegra132, ...

We don't enforce that the <chip> part is documented ATM and not likely
until we have a schema if ever.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Walmsley Jan. 29, 2015, 5:08 p.m. UTC | #4
Hi Rob

On Thu, 29 Jan 2015, Rob Herring wrote:

> On Thu, Jan 29, 2015 at 9:45 AM, Paul Walmsley <paul@pwsan.com> wrote:
> > On Thu, 29 Jan 2015, Rob Herring wrote:
> >
> >> On Wed, Jan 28, 2015 at 5:49 PM, Paul Walmsley <paul@pwsan.com> wrote:
> >> >
> >> > Add compatible strings for the PCIe IP blocks present on several Tegra
> >> > chips.  The primary objective here is to avoid checkpatch warnings,
> >> > per:
> >> >
> 
> [...]
> 
> >> > +  - "nvidia,tegra132-pcie" (not yet matched in the driver)
> >> > +  - "nvidia,tegra210-pcie" (not yet matched in the driver)
> >>
> >> Whether the driver matches or not is irrelevant to the binding and may
> >> change over time. Does this mean the driver matches on something else
> >> or Tegra132 is not yet supported in the driver?
> >
> > It means that the driver currently matches on one of the first three
> > strings that don't carry that annotation.
> >
> >> If the former, what is important is what are the valid combinations of
> >> compatible properties and that is not captured here. In other words,
> >> what is the fallback compatible string for each chip?
> >
> > The intention was to try to be helpful: to document that anyone adding a
> > "nvidia,tegra132-pcie" compatible string would also need to add one of the
> > other strings as a fallback.  Would you like that to be documented in a
> > different way, or removed?
> 
> Then you should say something like 'must contain "nvidia,tegra20-pcie"
> and one of: ...'
> 
> You can also use nvidia,<chip>-pcie if you want. checkpatch will check
> for that pattern too. Then your documentation can be something like:
> 
> Must contain '"nvidia,<chip>-pcie", "nvidia,tegra20-pcie"' where
> <chip> is tegra30, tegra132, ...
> 
> We don't enforce that the <chip> part is documented ATM and not likely
> until we have a schema if ever.

OK, thanks for the explanation.

So would it be acceptable to you to skip the attempt to document which 
strings are actually supported by the current driver, and to simply use 
the <chip> wildcard?


- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Walmsley Jan. 29, 2015, 5:29 p.m. UTC | #5
On Thu, 29 Jan 2015, Paul Walmsley wrote:

> Hi Rob
> 
> On Thu, 29 Jan 2015, Rob Herring wrote:
> 
> > On Thu, Jan 29, 2015 at 9:45 AM, Paul Walmsley <paul@pwsan.com> wrote:
> > > On Thu, 29 Jan 2015, Rob Herring wrote:
> > >
> > >> On Wed, Jan 28, 2015 at 5:49 PM, Paul Walmsley <paul@pwsan.com> wrote:
> > >> >
> > >> > Add compatible strings for the PCIe IP blocks present on several Tegra
> > >> > chips.  The primary objective here is to avoid checkpatch warnings,
> > >> > per:
> > >> >
> > 
> > [...]
> > 
> > >> > +  - "nvidia,tegra132-pcie" (not yet matched in the driver)
> > >> > +  - "nvidia,tegra210-pcie" (not yet matched in the driver)
> > >>
> > >> Whether the driver matches or not is irrelevant to the binding and may
> > >> change over time. Does this mean the driver matches on something else
> > >> or Tegra132 is not yet supported in the driver?
> > >
> > > It means that the driver currently matches on one of the first three
> > > strings that don't carry that annotation.
> > >
> > >> If the former, what is important is what are the valid combinations of
> > >> compatible properties and that is not captured here. In other words,
> > >> what is the fallback compatible string for each chip?
> > >
> > > The intention was to try to be helpful: to document that anyone adding a
> > > "nvidia,tegra132-pcie" compatible string would also need to add one of the
> > > other strings as a fallback.  Would you like that to be documented in a
> > > different way, or removed?
> > 
> > Then you should say something like 'must contain "nvidia,tegra20-pcie"
> > and one of: ...'
> > 
> > You can also use nvidia,<chip>-pcie if you want. checkpatch will check
> > for that pattern too. Then your documentation can be something like:
> > 
> > Must contain '"nvidia,<chip>-pcie", "nvidia,tegra20-pcie"' where
> > <chip> is tegra30, tegra132, ...
> > 
> > We don't enforce that the <chip> part is documented ATM and not likely
> > until we have a schema if ever.
> 
> OK, thanks for the explanation.
> 
> So would it be acceptable to you to skip the attempt to document which 
> strings are actually supported by the current driver, and to simply use 
> the <chip> wildcard?

(Just in case it wasn't clear, I mean for the purposes of the current 
patch series, not necessarily in general)


- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring Jan. 29, 2015, 6:34 p.m. UTC | #6
On Thu, Jan 29, 2015 at 11:08 AM, Paul Walmsley <paul@pwsan.com> wrote:
> Hi Rob
>
> On Thu, 29 Jan 2015, Rob Herring wrote:
>
>> On Thu, Jan 29, 2015 at 9:45 AM, Paul Walmsley <paul@pwsan.com> wrote:
>> > On Thu, 29 Jan 2015, Rob Herring wrote:
>> >
>> >> On Wed, Jan 28, 2015 at 5:49 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> >> >
>> >> > Add compatible strings for the PCIe IP blocks present on several Tegra
>> >> > chips.  The primary objective here is to avoid checkpatch warnings,
>> >> > per:
>> >> >
>>
>> [...]
>>
>> >> > +  - "nvidia,tegra132-pcie" (not yet matched in the driver)
>> >> > +  - "nvidia,tegra210-pcie" (not yet matched in the driver)
>> >>
>> >> Whether the driver matches or not is irrelevant to the binding and may
>> >> change over time. Does this mean the driver matches on something else
>> >> or Tegra132 is not yet supported in the driver?
>> >
>> > It means that the driver currently matches on one of the first three
>> > strings that don't carry that annotation.
>> >
>> >> If the former, what is important is what are the valid combinations of
>> >> compatible properties and that is not captured here. In other words,
>> >> what is the fallback compatible string for each chip?
>> >
>> > The intention was to try to be helpful: to document that anyone adding a
>> > "nvidia,tegra132-pcie" compatible string would also need to add one of the
>> > other strings as a fallback.  Would you like that to be documented in a
>> > different way, or removed?
>>
>> Then you should say something like 'must contain "nvidia,tegra20-pcie"
>> and one of: ...'
>>
>> You can also use nvidia,<chip>-pcie if you want. checkpatch will check
>> for that pattern too. Then your documentation can be something like:
>>
>> Must contain '"nvidia,<chip>-pcie", "nvidia,tegra20-pcie"' where
>> <chip> is tegra30, tegra132, ...
>>
>> We don't enforce that the <chip> part is documented ATM and not likely
>> until we have a schema if ever.
>
> OK, thanks for the explanation.
>
> So would it be acceptable to you to skip the attempt to document which
> strings are actually supported by the current driver, and to simply use
> the <chip> wildcard?

I don't think the binding document should say anything about what the
driver uses or not. It should describe what combinations of compatible
strings in a dts are valid. I didn't look at every patch, but the ones
like this one are. I'd be fine with most of this all in one patch BTW.

You should attempt to document known values of <chip> if you use it
(you could refer to another doc for the list). I was only highlighting
what you can get away with if no one is paying attention, not that you
don't need to add tegra132, etc. :)

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Walmsley Jan. 29, 2015, 6:49 p.m. UTC | #7
On Thu, 29 Jan 2015, Rob Herring wrote:

> On Thu, Jan 29, 2015 at 11:08 AM, Paul Walmsley <paul@pwsan.com> wrote:
> > On Thu, 29 Jan 2015, Rob Herring wrote:
> >
> >> On Thu, Jan 29, 2015 at 9:45 AM, Paul Walmsley <paul@pwsan.com> wrote:
> >> > On Thu, 29 Jan 2015, Rob Herring wrote:
> >> >
> >> >> On Wed, Jan 28, 2015 at 5:49 PM, Paul Walmsley <paul@pwsan.com> wrote:
> >> >> >
> >> >> > Add compatible strings for the PCIe IP blocks present on several Tegra
> >> >> > chips.  The primary objective here is to avoid checkpatch warnings,
> >> >> > per:
> >> >> >
> >>
> >> [...]
> >>
> >> >> > +  - "nvidia,tegra132-pcie" (not yet matched in the driver)
> >> >> > +  - "nvidia,tegra210-pcie" (not yet matched in the driver)
> >> >>
> >> >> Whether the driver matches or not is irrelevant to the binding and may
> >> >> change over time. Does this mean the driver matches on something else
> >> >> or Tegra132 is not yet supported in the driver?
> >> >
> >> > It means that the driver currently matches on one of the first three
> >> > strings that don't carry that annotation.
> >> >
> >> >> If the former, what is important is what are the valid combinations of
> >> >> compatible properties and that is not captured here. In other words,
> >> >> what is the fallback compatible string for each chip?
> >> >
> >> > The intention was to try to be helpful: to document that anyone adding a
> >> > "nvidia,tegra132-pcie" compatible string would also need to add one of the
> >> > other strings as a fallback.  Would you like that to be documented in a
> >> > different way, or removed?
> >>
> >> Then you should say something like 'must contain "nvidia,tegra20-pcie"
> >> and one of: ...'
> >>
> >> You can also use nvidia,<chip>-pcie if you want. checkpatch will check
> >> for that pattern too. Then your documentation can be something like:
> >>
> >> Must contain '"nvidia,<chip>-pcie", "nvidia,tegra20-pcie"' where
> >> <chip> is tegra30, tegra132, ...
> >>
> >> We don't enforce that the <chip> part is documented ATM and not likely
> >> until we have a schema if ever.
> >
> > OK, thanks for the explanation.
> >
> > So would it be acceptable to you to skip the attempt to document which
> > strings are actually supported by the current driver, and to simply use
> > the <chip> wildcard?
> 
> I don't think the binding document should say anything about what the
> driver uses or not. 

OK, will remove those.

> It should describe what combinations of compatible strings in a dts are 
> valid. I didn't look at every patch, but the ones like this one are.

OK, will switch to the format you described earlier:

"Must contain '"nvidia,<chip>-pcie", "nvidia,tegra20-pcie"' where
 <chip> is tegra30, tegra132, ..."

> I'd be fine with most of this all in one patch BTW.

OK, will squash.

> You should attempt to document known values of <chip> if you use it
> (you could refer to another doc for the list).

Is there an example of this in the tree that you have in mind, where 
another document is used for the list?  Would it just contain a list of 
the Tegra chips, e.g., tegra30, tegra114, tegra124, etc?

> I was only highlighting what you can get away with if no one is paying 
> attention, not that you don't need to add tegra132, etc. :)

OK.


- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
index d763e047c6ae..e772884f1c33 100644
--- a/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
@@ -5,6 +5,8 @@  Required properties:
   - "nvidia,tegra20-pcie"
   - "nvidia,tegra30-pcie"
   - "nvidia,tegra124-pcie"
+  - "nvidia,tegra132-pcie" (not yet matched in the driver)
+  - "nvidia,tegra210-pcie" (not yet matched in the driver)
 - device_type: Must be "pci"
 - reg: A list of physical base address and length for each set of controller
   registers. Must contain an entry for each entry in the reg-names property.