| Message ID | 20200327104727.4708-2-kishon@ti.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | PCI: cadence: Deprecate inbound/outbound specific bindings | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Fri, Mar 27, 2020 at 04:17:25PM +0530, Kishon Vijay Abraham I wrote: > Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for > host mode as both these could be derived from "ranges" and "dma-ranges" > property. "cdns,max-outbound-regions" property would still be required > for EP mode. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > .../bindings/pci/cdns,cdns-pcie-ep.yaml | 2 +- > .../bindings/pci/cdns,cdns-pcie-host.yaml | 3 +-- > .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++ > .../bindings/pci/cdns-pcie-host.yaml | 10 ++++++++ > .../devicetree/bindings/pci/cdns-pcie.yaml | 8 ------ > 5 files changed, 37 insertions(+), 11 deletions(-) > create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml > > diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml > index 2996f8d4777c..50ce5d79d2c7 100644 > --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml > +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml > @@ -10,7 +10,7 @@ maintainers: > - Tom Joseph <tjoseph@cadence.com> > > allOf: > - - $ref: "cdns-pcie.yaml#" > + - $ref: "cdns-pcie-ep.yaml#" > - $ref: "pci-ep.yaml#" > > properties: > diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml > index cabbe46ff578..84a8f095d031 100644 > --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml > +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml > @@ -45,8 +45,6 @@ examples: > #size-cells = <2>; > bus-range = <0x0 0xff>; > linux,pci-domain = <0>; > - cdns,max-outbound-regions = <16>; > - cdns,no-bar-match-nbits = <32>; > vendor-id = <0x17cd>; > device-id = <0x0200>; > > @@ -57,6 +55,7 @@ examples: > > ranges = <0x02000000 0x0 0x42000000 0x0 0x42000000 0x0 0x1000000>, > <0x01000000 0x0 0x43000000 0x0 0x43000000 0x0 0x0010000>; > + dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x1 0x00000000>; > > #interrupt-cells = <0x1>; > > diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml > new file mode 100644 > index 000000000000..6150a7a7bdbf > --- /dev/null > +++ b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml > @@ -0,0 +1,25 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/pci/cdns-pcie-ep.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Cadence PCIe Device > + > +maintainers: > + - Tom Joseph <tjoseph@cadence.com> > + > +allOf: > + - $ref: "cdns-pcie.yaml#" > + > +properties: > + cdns,max-outbound-regions: > + description: maximum number of outbound regions > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 1 > + maximum: 32 > + default: 32 I have a feeling that as the PCI endpoint binding evolves this won't be necessary. I can see a common need to define the number of BARs for an endpoint and then this will again just be error checking. What's the result if you write to a non-existent region in register CDNS_PCIE_AT_OB_REGION_PCI_ADDR0/1? If the register is non-existent and doesn't abort, you could detect this instead. Rob
Hi Rob, On 3/30/2020 9:31 PM, Rob Herring wrote: > On Fri, Mar 27, 2020 at 04:17:25PM +0530, Kishon Vijay Abraham I wrote: >> Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for >> host mode as both these could be derived from "ranges" and "dma-ranges" >> property. "cdns,max-outbound-regions" property would still be required >> for EP mode. >> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> .../bindings/pci/cdns,cdns-pcie-ep.yaml | 2 +- >> .../bindings/pci/cdns,cdns-pcie-host.yaml | 3 +-- >> .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++ >> .../bindings/pci/cdns-pcie-host.yaml | 10 ++++++++ >> .../devicetree/bindings/pci/cdns-pcie.yaml | 8 ------ >> 5 files changed, 37 insertions(+), 11 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml >> >> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >> index 2996f8d4777c..50ce5d79d2c7 100644 >> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >> @@ -10,7 +10,7 @@ maintainers: >> - Tom Joseph <tjoseph@cadence.com> >> >> allOf: >> - - $ref: "cdns-pcie.yaml#" >> + - $ref: "cdns-pcie-ep.yaml#" >> - $ref: "pci-ep.yaml#" >> >> properties: >> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >> index cabbe46ff578..84a8f095d031 100644 >> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >> @@ -45,8 +45,6 @@ examples: >> #size-cells = <2>; >> bus-range = <0x0 0xff>; >> linux,pci-domain = <0>; >> - cdns,max-outbound-regions = <16>; >> - cdns,no-bar-match-nbits = <32>; >> vendor-id = <0x17cd>; >> device-id = <0x0200>; >> >> @@ -57,6 +55,7 @@ examples: >> >> ranges = <0x02000000 0x0 0x42000000 0x0 0x42000000 0x0 0x1000000>, >> <0x01000000 0x0 0x43000000 0x0 0x43000000 0x0 0x0010000>; >> + dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x1 0x00000000>; >> >> #interrupt-cells = <0x1>; >> >> diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml >> new file mode 100644 >> index 000000000000..6150a7a7bdbf >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml >> @@ -0,0 +1,25 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: "http://devicetree.org/schemas/pci/cdns-pcie-ep.yaml#" >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >> + >> +title: Cadence PCIe Device >> + >> +maintainers: >> + - Tom Joseph <tjoseph@cadence.com> >> + >> +allOf: >> + - $ref: "cdns-pcie.yaml#" >> + >> +properties: >> + cdns,max-outbound-regions: >> + description: maximum number of outbound regions >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/uint32 >> + minimum: 1 >> + maximum: 32 >> + default: 32 > > I have a feeling that as the PCI endpoint binding evolves this won't be > necessary. I can see a common need to define the number of BARs for an > endpoint and then this will again just be error checking. For every buffer given by the host, we have to create a new outbound translation. If there are no outbound regions, we have to report the error to the endpoint function driver. At-least for reporting the error, we'd need to have this binding no? > > What's the result if you write to a non-existent region in register > CDNS_PCIE_AT_OB_REGION_PCI_ADDR0/1? If the register is non-existent and > doesn't abort, you could detect this instead. I'm not sure if we should ever try to write to a non-existent register though the behavior could be different in different platforms. IMHO maximum number of outbound regions is a HW property and is best described in device tree. Thanks Kishon
On Tue, Mar 31, 2020 at 09:08:12AM +0530, Kishon Vijay Abraham I wrote: > Hi Rob, > > On 3/30/2020 9:31 PM, Rob Herring wrote: > > On Fri, Mar 27, 2020 at 04:17:25PM +0530, Kishon Vijay Abraham I wrote: > >> Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for > >> host mode as both these could be derived from "ranges" and "dma-ranges" > >> property. "cdns,max-outbound-regions" property would still be required > >> for EP mode. > >> > >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > >> --- > >> .../bindings/pci/cdns,cdns-pcie-ep.yaml | 2 +- > >> .../bindings/pci/cdns,cdns-pcie-host.yaml | 3 +-- > >> .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++ > >> .../bindings/pci/cdns-pcie-host.yaml | 10 ++++++++ > >> .../devicetree/bindings/pci/cdns-pcie.yaml | 8 ------ > >> 5 files changed, 37 insertions(+), 11 deletions(-) > >> create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml > >> > >> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml > >> index 2996f8d4777c..50ce5d79d2c7 100644 > >> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml > >> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml > >> @@ -10,7 +10,7 @@ maintainers: > >> - Tom Joseph <tjoseph@cadence.com> > >> > >> allOf: > >> - - $ref: "cdns-pcie.yaml#" > >> + - $ref: "cdns-pcie-ep.yaml#" > >> - $ref: "pci-ep.yaml#" > >> > >> properties: > >> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml > >> index cabbe46ff578..84a8f095d031 100644 > >> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml > >> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml > >> @@ -45,8 +45,6 @@ examples: > >> #size-cells = <2>; > >> bus-range = <0x0 0xff>; > >> linux,pci-domain = <0>; > >> - cdns,max-outbound-regions = <16>; > >> - cdns,no-bar-match-nbits = <32>; > >> vendor-id = <0x17cd>; > >> device-id = <0x0200>; > >> > >> @@ -57,6 +55,7 @@ examples: > >> > >> ranges = <0x02000000 0x0 0x42000000 0x0 0x42000000 0x0 0x1000000>, > >> <0x01000000 0x0 0x43000000 0x0 0x43000000 0x0 0x0010000>; > >> + dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x1 0x00000000>; > >> > >> #interrupt-cells = <0x1>; > >> > >> diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml > >> new file mode 100644 > >> index 000000000000..6150a7a7bdbf > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml > >> @@ -0,0 +1,25 @@ > >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >> +%YAML 1.2 > >> +--- > >> +$id: "http://devicetree.org/schemas/pci/cdns-pcie-ep.yaml#" > >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > >> + > >> +title: Cadence PCIe Device > >> + > >> +maintainers: > >> + - Tom Joseph <tjoseph@cadence.com> > >> + > >> +allOf: > >> + - $ref: "cdns-pcie.yaml#" > >> + > >> +properties: > >> + cdns,max-outbound-regions: > >> + description: maximum number of outbound regions > >> + allOf: > >> + - $ref: /schemas/types.yaml#/definitions/uint32 > >> + minimum: 1 > >> + maximum: 32 > >> + default: 32 > > > > I have a feeling that as the PCI endpoint binding evolves this won't be > > necessary. I can see a common need to define the number of BARs for an > > endpoint and then this will again just be error checking. > > For every buffer given by the host, we have to create a new outbound > translation. If there are no outbound regions, we have to report the error to > the endpoint function driver. At-least for reporting the error, we'd need to > have this binding no? But isn't the endpoint defined to have some number of BARs? The PCI host doesn't decide that. > > > > What's the result if you write to a non-existent region in register > > CDNS_PCIE_AT_OB_REGION_PCI_ADDR0/1? If the register is non-existent and > > doesn't abort, you could detect this instead. > > I'm not sure if we should ever try to write to a non-existent register though > the behavior could be different in different platforms. IMHO maximum number of > outbound regions is a HW property and is best described in device tree. AIUI, PCI defines non-existent (config space) registers to return all 1s. Not sure if this register is in PCI config space or the host SoC bus (e.g. AXI). It seems PCI bridges get done both ways from what I've seen. Rob
Hi Rob, On 3/31/2020 10:15 PM, Rob Herring wrote: > On Tue, Mar 31, 2020 at 09:08:12AM +0530, Kishon Vijay Abraham I wrote: >> Hi Rob, >> >> On 3/30/2020 9:31 PM, Rob Herring wrote: >>> On Fri, Mar 27, 2020 at 04:17:25PM +0530, Kishon Vijay Abraham I wrote: >>>> Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for >>>> host mode as both these could be derived from "ranges" and "dma-ranges" >>>> property. "cdns,max-outbound-regions" property would still be required >>>> for EP mode. >>>> >>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >>>> --- >>>> .../bindings/pci/cdns,cdns-pcie-ep.yaml | 2 +- >>>> .../bindings/pci/cdns,cdns-pcie-host.yaml | 3 +-- >>>> .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++ >>>> .../bindings/pci/cdns-pcie-host.yaml | 10 ++++++++ >>>> .../devicetree/bindings/pci/cdns-pcie.yaml | 8 ------ >>>> 5 files changed, 37 insertions(+), 11 deletions(-) >>>> create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >>>> index 2996f8d4777c..50ce5d79d2c7 100644 >>>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >>>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >>>> @@ -10,7 +10,7 @@ maintainers: >>>> - Tom Joseph <tjoseph@cadence.com> >>>> >>>> allOf: >>>> - - $ref: "cdns-pcie.yaml#" >>>> + - $ref: "cdns-pcie-ep.yaml#" >>>> - $ref: "pci-ep.yaml#" >>>> >>>> properties: >>>> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >>>> index cabbe46ff578..84a8f095d031 100644 >>>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >>>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >>>> @@ -45,8 +45,6 @@ examples: >>>> #size-cells = <2>; >>>> bus-range = <0x0 0xff>; >>>> linux,pci-domain = <0>; >>>> - cdns,max-outbound-regions = <16>; >>>> - cdns,no-bar-match-nbits = <32>; >>>> vendor-id = <0x17cd>; >>>> device-id = <0x0200>; >>>> >>>> @@ -57,6 +55,7 @@ examples: >>>> >>>> ranges = <0x02000000 0x0 0x42000000 0x0 0x42000000 0x0 0x1000000>, >>>> <0x01000000 0x0 0x43000000 0x0 0x43000000 0x0 0x0010000>; >>>> + dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x1 0x00000000>; >>>> >>>> #interrupt-cells = <0x1>; >>>> >>>> diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml >>>> new file mode 100644 >>>> index 000000000000..6150a7a7bdbf >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml >>>> @@ -0,0 +1,25 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: "http://devicetree.org/schemas/pci/cdns-pcie-ep.yaml#" >>>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >>>> + >>>> +title: Cadence PCIe Device >>>> + >>>> +maintainers: >>>> + - Tom Joseph <tjoseph@cadence.com> >>>> + >>>> +allOf: >>>> + - $ref: "cdns-pcie.yaml#" >>>> + >>>> +properties: >>>> + cdns,max-outbound-regions: >>>> + description: maximum number of outbound regions >>>> + allOf: >>>> + - $ref: /schemas/types.yaml#/definitions/uint32 >>>> + minimum: 1 >>>> + maximum: 32 >>>> + default: 32 >>> >>> I have a feeling that as the PCI endpoint binding evolves this won't be >>> necessary. I can see a common need to define the number of BARs for an >>> endpoint and then this will again just be error checking. >> >> For every buffer given by the host, we have to create a new outbound >> translation. If there are no outbound regions, we have to report the error to >> the endpoint function driver. At-least for reporting the error, we'd need to >> have this binding no? > > But isn't the endpoint defined to have some number of BARs? The PCI host > doesn't decide that. cdns,max-outbound-regions defined here doesn't configure the BARs. BARs provide an interface for the host to access the endpoints memory. IOW for BARs we configure the inbound address translation unit. cdns,max-outbound-regions is used while configuring the outbound address translation unit. Outbound regions are used while the endpoint access host memory and in that path endpoint BARs doesn't come. > >>> >>> What's the result if you write to a non-existent region in register >>> CDNS_PCIE_AT_OB_REGION_PCI_ADDR0/1? If the register is non-existent and >>> doesn't abort, you could detect this instead. >> >> I'm not sure if we should ever try to write to a non-existent register though >> the behavior could be different in different platforms. IMHO maximum number of >> outbound regions is a HW property and is best described in device tree. > > AIUI, PCI defines non-existent (config space) registers to return all > 1s. Not sure if this register is in PCI config space or the host SoC bus > (e.g. AXI). It seems PCI bridges get done both ways from what I've seen. All of that is correct for the Host or RC. However here cdns,max-outbound-regions is an endpoint specific property (defined only in cdns-pcie-ep.yaml) and is useful while configuring OB address translation unit for the endpoint to access host memory. Thanks Kishon
Hi Rob, On 4/1/2020 8:38 AM, Kishon Vijay Abraham I wrote: > Hi Rob, > > On 3/31/2020 10:15 PM, Rob Herring wrote: >> On Tue, Mar 31, 2020 at 09:08:12AM +0530, Kishon Vijay Abraham I wrote: >>> Hi Rob, >>> >>> On 3/30/2020 9:31 PM, Rob Herring wrote: >>>> On Fri, Mar 27, 2020 at 04:17:25PM +0530, Kishon Vijay Abraham I wrote: >>>>> Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for >>>>> host mode as both these could be derived from "ranges" and "dma-ranges" >>>>> property. "cdns,max-outbound-regions" property would still be required >>>>> for EP mode. >>>>> >>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >>>>> --- >>>>> .../bindings/pci/cdns,cdns-pcie-ep.yaml | 2 +- >>>>> .../bindings/pci/cdns,cdns-pcie-host.yaml | 3 +-- >>>>> .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++ >>>>> .../bindings/pci/cdns-pcie-host.yaml | 10 ++++++++ >>>>> .../devicetree/bindings/pci/cdns-pcie.yaml | 8 ------ >>>>> 5 files changed, 37 insertions(+), 11 deletions(-) >>>>> create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >>>>> index 2996f8d4777c..50ce5d79d2c7 100644 >>>>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >>>>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml >>>>> @@ -10,7 +10,7 @@ maintainers: >>>>> - Tom Joseph <tjoseph@cadence.com> >>>>> >>>>> allOf: >>>>> - - $ref: "cdns-pcie.yaml#" >>>>> + - $ref: "cdns-pcie-ep.yaml#" >>>>> - $ref: "pci-ep.yaml#" >>>>> >>>>> properties: >>>>> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >>>>> index cabbe46ff578..84a8f095d031 100644 >>>>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >>>>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml >>>>> @@ -45,8 +45,6 @@ examples: >>>>> #size-cells = <2>; >>>>> bus-range = <0x0 0xff>; >>>>> linux,pci-domain = <0>; >>>>> - cdns,max-outbound-regions = <16>; >>>>> - cdns,no-bar-match-nbits = <32>; >>>>> vendor-id = <0x17cd>; >>>>> device-id = <0x0200>; >>>>> >>>>> @@ -57,6 +55,7 @@ examples: >>>>> >>>>> ranges = <0x02000000 0x0 0x42000000 0x0 0x42000000 0x0 0x1000000>, >>>>> <0x01000000 0x0 0x43000000 0x0 0x43000000 0x0 0x0010000>; >>>>> + dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x1 0x00000000>; >>>>> >>>>> #interrupt-cells = <0x1>; >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml >>>>> new file mode 100644 >>>>> index 000000000000..6150a7a7bdbf >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml >>>>> @@ -0,0 +1,25 @@ >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>>> +%YAML 1.2 >>>>> +--- >>>>> +$id: "http://devicetree.org/schemas/pci/cdns-pcie-ep.yaml#" >>>>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >>>>> + >>>>> +title: Cadence PCIe Device >>>>> + >>>>> +maintainers: >>>>> + - Tom Joseph <tjoseph@cadence.com> >>>>> + >>>>> +allOf: >>>>> + - $ref: "cdns-pcie.yaml#" >>>>> + >>>>> +properties: >>>>> + cdns,max-outbound-regions: >>>>> + description: maximum number of outbound regions >>>>> + allOf: >>>>> + - $ref: /schemas/types.yaml#/definitions/uint32 >>>>> + minimum: 1 >>>>> + maximum: 32 >>>>> + default: 32 >>>> >>>> I have a feeling that as the PCI endpoint binding evolves this won't be >>>> necessary. I can see a common need to define the number of BARs for an >>>> endpoint and then this will again just be error checking. >>> >>> For every buffer given by the host, we have to create a new outbound >>> translation. If there are no outbound regions, we have to report the error to >>> the endpoint function driver. At-least for reporting the error, we'd need to >>> have this binding no? >> >> But isn't the endpoint defined to have some number of BARs? The PCI host >> doesn't decide that. > > cdns,max-outbound-regions defined here doesn't configure the BARs. BARs provide > an interface for the host to access the endpoints memory. IOW for BARs we > configure the inbound address translation unit. > > cdns,max-outbound-regions is used while configuring the outbound address > translation unit. Outbound regions are used while the endpoint access host > memory and in that path endpoint BARs doesn't come. >> >>>> >>>> What's the result if you write to a non-existent region in register >>>> CDNS_PCIE_AT_OB_REGION_PCI_ADDR0/1? If the register is non-existent and >>>> doesn't abort, you could detect this instead. >>> >>> I'm not sure if we should ever try to write to a non-existent register though >>> the behavior could be different in different platforms. IMHO maximum number of >>> outbound regions is a HW property and is best described in device tree. >> >> AIUI, PCI defines non-existent (config space) registers to return all >> 1s. Not sure if this register is in PCI config space or the host SoC bus >> (e.g. AXI). It seems PCI bridges get done both ways from what I've seen. > > All of that is correct for the Host or RC. However here > cdns,max-outbound-regions is an endpoint specific property (defined only in > cdns-pcie-ep.yaml) and is useful while configuring OB address translation unit > for the endpoint to access host memory. Do you still have concerns regarding this? If you don't have any further comments on this, can you give your Acked-by please? Thanks Kishon
On Fri, 27 Mar 2020 16:17:25 +0530, Kishon Vijay Abraham I wrote: > Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for > host mode as both these could be derived from "ranges" and "dma-ranges" > property. "cdns,max-outbound-regions" property would still be required > for EP mode. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > .../bindings/pci/cdns,cdns-pcie-ep.yaml | 2 +- > .../bindings/pci/cdns,cdns-pcie-host.yaml | 3 +-- > .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++ > .../bindings/pci/cdns-pcie-host.yaml | 10 ++++++++ > .../devicetree/bindings/pci/cdns-pcie.yaml | 8 ------ > 5 files changed, 37 insertions(+), 11 deletions(-) > create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml > Reviewed-by: Rob Herring <robh@kernel.org>
On 4/10/2020 10:08 PM, Rob Herring wrote: > On Fri, 27 Mar 2020 16:17:25 +0530, Kishon Vijay Abraham I wrote: >> Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for >> host mode as both these could be derived from "ranges" and "dma-ranges" >> property. "cdns,max-outbound-regions" property would still be required >> for EP mode. >> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> .../bindings/pci/cdns,cdns-pcie-ep.yaml | 2 +- >> .../bindings/pci/cdns,cdns-pcie-host.yaml | 3 +-- >> .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++ >> .../bindings/pci/cdns-pcie-host.yaml | 10 ++++++++ >> .../devicetree/bindings/pci/cdns-pcie.yaml | 8 ------ >> 5 files changed, 37 insertions(+), 11 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml >> > > Reviewed-by: Rob Herring <robh@kernel.org> Thank you Rob! Regards Kishon
diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml index 2996f8d4777c..50ce5d79d2c7 100644 --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml @@ -10,7 +10,7 @@ maintainers: - Tom Joseph <tjoseph@cadence.com> allOf: - - $ref: "cdns-pcie.yaml#" + - $ref: "cdns-pcie-ep.yaml#" - $ref: "pci-ep.yaml#" properties: diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml index cabbe46ff578..84a8f095d031 100644 --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml @@ -45,8 +45,6 @@ examples: #size-cells = <2>; bus-range = <0x0 0xff>; linux,pci-domain = <0>; - cdns,max-outbound-regions = <16>; - cdns,no-bar-match-nbits = <32>; vendor-id = <0x17cd>; device-id = <0x0200>; @@ -57,6 +55,7 @@ examples: ranges = <0x02000000 0x0 0x42000000 0x0 0x42000000 0x0 0x1000000>, <0x01000000 0x0 0x43000000 0x0 0x43000000 0x0 0x0010000>; + dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x1 0x00000000>; #interrupt-cells = <0x1>; diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml new file mode 100644 index 000000000000..6150a7a7bdbf --- /dev/null +++ b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml @@ -0,0 +1,25 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/pci/cdns-pcie-ep.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Cadence PCIe Device + +maintainers: + - Tom Joseph <tjoseph@cadence.com> + +allOf: + - $ref: "cdns-pcie.yaml#" + +properties: + cdns,max-outbound-regions: + description: maximum number of outbound regions + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 32 + default: 32 + +required: + - cdns,max-outbound-regions diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml index ab6e43b636ec..3d64f85aeb39 100644 --- a/Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml +++ b/Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml @@ -14,6 +14,15 @@ allOf: - $ref: "cdns-pcie.yaml#" properties: + cdns,max-outbound-regions: + description: maximum number of outbound regions + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 32 + default: 32 + deprecated: true + cdns,no-bar-match-nbits: description: Set into the no BAR match register to configure the number of least @@ -23,5 +32,6 @@ properties: minimum: 0 maximum: 64 default: 32 + deprecated: true msi-parent: true diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie.yaml index 6887ccc339cc..02553d5e6c51 100644 --- a/Documentation/devicetree/bindings/pci/cdns-pcie.yaml +++ b/Documentation/devicetree/bindings/pci/cdns-pcie.yaml @@ -10,14 +10,6 @@ maintainers: - Tom Joseph <tjoseph@cadence.com> properties: - cdns,max-outbound-regions: - description: maximum number of outbound regions - allOf: - - $ref: /schemas/types.yaml#/definitions/uint32 - minimum: 1 - maximum: 32 - default: 32 - phys: description: One per lane if more than one in the list. If only one PHY listed it must
Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for host mode as both these could be derived from "ranges" and "dma-ranges" property. "cdns,max-outbound-regions" property would still be required for EP mode. Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> --- .../bindings/pci/cdns,cdns-pcie-ep.yaml | 2 +- .../bindings/pci/cdns,cdns-pcie-host.yaml | 3 +-- .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++ .../bindings/pci/cdns-pcie-host.yaml | 10 ++++++++ .../devicetree/bindings/pci/cdns-pcie.yaml | 8 ------ 5 files changed, 37 insertions(+), 11 deletions(-) create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml