| Message ID | 1502167455-10516-3-git-send-email-oza.oza@broadcom.com |
|---|---|
| State | Superseded |
| Headers | show |
Please send bindings to DT list. On Mon, Aug 7, 2017 at 11:44 PM, Oza Pawandeep <oza.oza@broadcom.com> wrote: > Add description for optional device tree property > 'brcm,pci-hotplug' for PCI hotplug feature. > > Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com> > Reviewed-by: Ray Jui <ray.jui@broadcom.com> > > diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt > index b8e48b4..a3bad24 100644 > --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt > +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt > @@ -72,6 +72,29 @@ Optional properties: > - brcm,pcie-msi-inten: Needs to be present for some older iProc platforms that > require the interrupt enable registers to be set explicitly to enable MSI > > +Optional properties: > +- brcm,pci-hotplug: PCI hotplug feature is supported. I think we should make this a common property. We already have "ibm,slot-pluggable", so I'd propose "slot-pluggable". There's also "hotpluggable" for memory nodes defined, so we could reuse that here. > + > +If the brcm,pcie-hotplug property is present, the following properties become > +effective: > + > +- brcm,prsnt-gpio: Array of gpios, needs to be present if Hotplug is supported. prsnt-gpios > + > +PCI hotplug implementation is SOC/Board specific, and also it depends on > +how add-in card is designed (e.g. how many present pins are implemented). > + > +If x8 card is connected, then it might be possible that all the > +3 present pins could go low, or at least one pin goes low. > + > +If x4 card is connected, then it might be possible that 2 present > +pins go low, or at least one pin goes low. > + > +Example: > +brcm,prsnt-gpio: <&pca9505 32 1>, <&pca9505 33 1>; > +This is x4 connector: monitoring max 2 present lines. > +brcm,prsnt-gpio: <&pca9505 32 1>, <&pca9505 33 1>, <&pca9505 34 1>; > +This is x8 connector: monitoring max 3 present lines. > + > Example: > pcie0: pcie@18012000 { > compatible = "brcm,iproc-pcie"; > -- > 1.9.1 >
On Tue, Aug 8, 2017 at 7:50 PM, Rob Herring <robh+dt@kernel.org> wrote: > Please send bindings to DT list. Sure, will do that. > > On Mon, Aug 7, 2017 at 11:44 PM, Oza Pawandeep <oza.oza@broadcom.com> wrote: >> Add description for optional device tree property >> 'brcm,pci-hotplug' for PCI hotplug feature. >> >> Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com> >> Reviewed-by: Ray Jui <ray.jui@broadcom.com> >> >> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >> index b8e48b4..a3bad24 100644 >> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >> @@ -72,6 +72,29 @@ Optional properties: >> - brcm,pcie-msi-inten: Needs to be present for some older iProc platforms that >> require the interrupt enable registers to be set explicitly to enable MSI >> >> +Optional properties: >> +- brcm,pci-hotplug: PCI hotplug feature is supported. > > I think we should make this a common property. We already have > "ibm,slot-pluggable", so I'd propose "slot-pluggable". > > There's also "hotpluggable" for memory nodes defined, so we could > reuse that here. > ok I will rename this to brcm,slot-pluggable >> + >> +If the brcm,pcie-hotplug property is present, the following properties become >> +effective: >> + >> +- brcm,prsnt-gpio: Array of gpios, needs to be present if Hotplug is supported. > > prsnt-gpios will take care. > >> + >> +PCI hotplug implementation is SOC/Board specific, and also it depends on >> +how add-in card is designed (e.g. how many present pins are implemented). >> + >> +If x8 card is connected, then it might be possible that all the >> +3 present pins could go low, or at least one pin goes low. >> + >> +If x4 card is connected, then it might be possible that 2 present >> +pins go low, or at least one pin goes low. >> + >> +Example: >> +brcm,prsnt-gpio: <&pca9505 32 1>, <&pca9505 33 1>; >> +This is x4 connector: monitoring max 2 present lines. >> +brcm,prsnt-gpio: <&pca9505 32 1>, <&pca9505 33 1>, <&pca9505 34 1>; >> +This is x8 connector: monitoring max 3 present lines. >> + >> Example: >> pcie0: pcie@18012000 { >> compatible = "brcm,iproc-pcie"; >> -- >> 1.9.1 >>
On 8/8/2017 10:22 PM, Oza Oza wrote: > On Tue, Aug 8, 2017 at 7:50 PM, Rob Herring <robh+dt@kernel.org> wrote: >> Please send bindings to DT list. > Sure, will do that. > >> On Mon, Aug 7, 2017 at 11:44 PM, Oza Pawandeep <oza.oza@broadcom.com> wrote: >>> Add description for optional device tree property >>> 'brcm,pci-hotplug' for PCI hotplug feature. >>> >>> Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com> >>> Reviewed-by: Ray Jui <ray.jui@broadcom.com> >>> >>> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>> index b8e48b4..a3bad24 100644 >>> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>> @@ -72,6 +72,29 @@ Optional properties: >>> - brcm,pcie-msi-inten: Needs to be present for some older iProc platforms that >>> require the interrupt enable registers to be set explicitly to enable MSI >>> >>> +Optional properties: >>> +- brcm,pci-hotplug: PCI hotplug feature is supported. >> I think we should make this a common property. We already have >> "ibm,slot-pluggable", so I'd propose "slot-pluggable". >> >> There's also "hotpluggable" for memory nodes defined, so we could >> reuse that here. >> > ok I will rename this to > brcm,slot-pluggable How's brcm,slot-pluggable a common property? It's still brcm specific. Didn't Rob propose either "slot-pluggable" or "hotpluggable"? And note it goes to the generic PCI binding instead iProc PCIe specific binding.
On Wed, Aug 9, 2017 at 10:57 AM, Ray Jui <ray.jui@broadcom.com> wrote: > > > On 8/8/2017 10:22 PM, Oza Oza wrote: >> >> On Tue, Aug 8, 2017 at 7:50 PM, Rob Herring <robh+dt@kernel.org> wrote: >>> >>> Please send bindings to DT list. >> >> Sure, will do that. >> >>> On Mon, Aug 7, 2017 at 11:44 PM, Oza Pawandeep <oza.oza@broadcom.com> >>> wrote: >>>> >>>> Add description for optional device tree property >>>> 'brcm,pci-hotplug' for PCI hotplug feature. >>>> >>>> Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com> >>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com> >>>> >>>> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>>> b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>>> index b8e48b4..a3bad24 100644 >>>> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>>> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>>> @@ -72,6 +72,29 @@ Optional properties: >>>> - brcm,pcie-msi-inten: Needs to be present for some older iProc >>>> platforms that >>>> require the interrupt enable registers to be set explicitly to enable >>>> MSI >>>> >>>> +Optional properties: >>>> +- brcm,pci-hotplug: PCI hotplug feature is supported. >>> >>> I think we should make this a common property. We already have >>> "ibm,slot-pluggable", so I'd propose "slot-pluggable". >>> >>> There's also "hotpluggable" for memory nodes defined, so we could >>> reuse that here. >>> >> ok I will rename this to >> brcm,slot-pluggable > > > How's brcm,slot-pluggable a common property? It's still brcm specific. > Didn't Rob propose either "slot-pluggable" or "hotpluggable"? > > And note it goes to the generic PCI binding instead iProc PCIe specific > binding. > Initially I thought, Rob suggested either "slot-pluggable". followed by, "hotpluggable" since memory node already has such property. but not sure in which generic pci binding I should add ? should it be part of Documentation/devicetree/bindings/pci/host-generic-pci.txt Can you please clarify Rob ? Regards, Oza.
On Wed, Aug 9, 2017 at 11:13 AM, Oza Oza <oza.oza@broadcom.com> wrote: > On Wed, Aug 9, 2017 at 10:57 AM, Ray Jui <ray.jui@broadcom.com> wrote: >> >> >> On 8/8/2017 10:22 PM, Oza Oza wrote: >>> >>> On Tue, Aug 8, 2017 at 7:50 PM, Rob Herring <robh+dt@kernel.org> wrote: >>>> >>>> Please send bindings to DT list. >>> >>> Sure, will do that. >>> >>>> On Mon, Aug 7, 2017 at 11:44 PM, Oza Pawandeep <oza.oza@broadcom.com> >>>> wrote: >>>>> >>>>> Add description for optional device tree property >>>>> 'brcm,pci-hotplug' for PCI hotplug feature. >>>>> >>>>> Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com> >>>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com> >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>>>> b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>>>> index b8e48b4..a3bad24 100644 >>>>> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>>>> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>>>> @@ -72,6 +72,29 @@ Optional properties: >>>>> - brcm,pcie-msi-inten: Needs to be present for some older iProc >>>>> platforms that >>>>> require the interrupt enable registers to be set explicitly to enable >>>>> MSI >>>>> >>>>> +Optional properties: >>>>> +- brcm,pci-hotplug: PCI hotplug feature is supported. >>>> >>>> I think we should make this a common property. We already have >>>> "ibm,slot-pluggable", so I'd propose "slot-pluggable". >>>> >>>> There's also "hotpluggable" for memory nodes defined, so we could >>>> reuse that here. >>>> >>> ok I will rename this to >>> brcm,slot-pluggable >> >> >> How's brcm,slot-pluggable a common property? It's still brcm specific. >> Didn't Rob propose either "slot-pluggable" or "hotpluggable"? >> >> And note it goes to the generic PCI binding instead iProc PCIe specific >> binding. >> > > Initially I thought, Rob suggested either "slot-pluggable". > followed by, "hotpluggable" since memory node already has such property. > > but not sure in which generic pci binding I should add ? > should it be part of > Documentation/devicetree/bindings/pci/host-generic-pci.txt > > Can you please clarify Rob ? > > Regards, > Oza. To add, every SOC might have different way of implementing hotplug. so I suppose both the binding documents have to be updated. Documentation/devicetree/bindings/pci/host-generic-pci.txt which can have common boolean property named "hotpluggable" and SOC specific implementation can stay here for e.g. Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt brcm,prsnt-gpios Rob and Ray: please let me know how this sounds. Regards, Oza.
On Wed, Aug 9, 2017 at 12:52 AM, Oza Oza <oza.oza@broadcom.com> wrote: > On Wed, Aug 9, 2017 at 11:13 AM, Oza Oza <oza.oza@broadcom.com> wrote: >> On Wed, Aug 9, 2017 at 10:57 AM, Ray Jui <ray.jui@broadcom.com> wrote: >>> >>> >>> On 8/8/2017 10:22 PM, Oza Oza wrote: >>>> >>>> On Tue, Aug 8, 2017 at 7:50 PM, Rob Herring <robh+dt@kernel.org> wrote: >>>>> >>>>> Please send bindings to DT list. >>>> >>>> Sure, will do that. >>>> >>>>> On Mon, Aug 7, 2017 at 11:44 PM, Oza Pawandeep <oza.oza@broadcom.com> >>>>> wrote: >>>>>> >>>>>> Add description for optional device tree property >>>>>> 'brcm,pci-hotplug' for PCI hotplug feature. >>>>>> >>>>>> Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com> >>>>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com> >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>>>>> b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>>>>> index b8e48b4..a3bad24 100644 >>>>>> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>>>>> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt >>>>>> @@ -72,6 +72,29 @@ Optional properties: >>>>>> - brcm,pcie-msi-inten: Needs to be present for some older iProc >>>>>> platforms that >>>>>> require the interrupt enable registers to be set explicitly to enable >>>>>> MSI >>>>>> >>>>>> +Optional properties: >>>>>> +- brcm,pci-hotplug: PCI hotplug feature is supported. >>>>> >>>>> I think we should make this a common property. We already have >>>>> "ibm,slot-pluggable", so I'd propose "slot-pluggable". >>>>> >>>>> There's also "hotpluggable" for memory nodes defined, so we could >>>>> reuse that here. >>>>> >>>> ok I will rename this to >>>> brcm,slot-pluggable >>> >>> >>> How's brcm,slot-pluggable a common property? It's still brcm specific. >>> Didn't Rob propose either "slot-pluggable" or "hotpluggable"? >>> >>> And note it goes to the generic PCI binding instead iProc PCIe specific >>> binding. >>> >> >> Initially I thought, Rob suggested either "slot-pluggable". >> followed by, "hotpluggable" since memory node already has such property. I didn't say which because I'm fine with either one. Pick the color of the bikeshed. >> >> but not sure in which generic pci binding I should add ? >> should it be part of >> Documentation/devicetree/bindings/pci/host-generic-pci.txt That's for generic CAM/ECAM hosts. pci.txt would be the place. >> >> Can you please clarify Rob ? >> >> Regards, >> Oza. > > To add, every SOC might have different way of implementing hotplug. > so I suppose both the binding documents have to be updated. > > Documentation/devicetree/bindings/pci/host-generic-pci.txt > which can have common boolean property > named "hotpluggable" > > and SOC specific implementation can stay here > for e.g. > Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt > brcm,prsnt-gpios PRSNT# is a standard signal though, right? The h/w specific part would be whether it is connected to GPIOs or the host controller has some built-in controls. So yes, it should be documented if iproc-pcie uses "prsnt-gpios", but it can still be a common property. Rob
diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt index b8e48b4..a3bad24 100644 --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt @@ -72,6 +72,29 @@ Optional properties: - brcm,pcie-msi-inten: Needs to be present for some older iProc platforms that require the interrupt enable registers to be set explicitly to enable MSI +Optional properties: +- brcm,pci-hotplug: PCI hotplug feature is supported. + +If the brcm,pcie-hotplug property is present, the following properties become +effective: + +- brcm,prsnt-gpio: Array of gpios, needs to be present if Hotplug is supported. + +PCI hotplug implementation is SOC/Board specific, and also it depends on +how add-in card is designed (e.g. how many present pins are implemented). + +If x8 card is connected, then it might be possible that all the +3 present pins could go low, or at least one pin goes low. + +If x4 card is connected, then it might be possible that 2 present +pins go low, or at least one pin goes low. + +Example: +brcm,prsnt-gpio: <&pca9505 32 1>, <&pca9505 33 1>; +This is x4 connector: monitoring max 2 present lines. +brcm,prsnt-gpio: <&pca9505 32 1>, <&pca9505 33 1>, <&pca9505 34 1>; +This is x8 connector: monitoring max 3 present lines. + Example: pcie0: pcie@18012000 { compatible = "brcm,iproc-pcie";