| Message ID | 20191122104505.8986-1-vidyas@nvidia.com |
|---|---|
| Headers | show |
| Series | Add support for PCIe endpoint mode in Tegra194 | expand |
On Fri, Nov 22, 2019 at 04:15:04PM +0530, Vidya Sagar wrote: > Enable GPIO controllers nodes for P2972-0000 platform which are required > by other controllers in the SoC for example when PCIe C5 controller > operates in endpoint mode. > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts | 8 ++++++++ > 1 file changed, 8 insertions(+) The GPIO controllers are enabled by default, so there's no need to explicitly enable them. Thierry > > diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > index 353a6a22196d..7eb64b816e08 100644 > --- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > @@ -37,6 +37,14 @@ > status = "okay"; > }; > > + gpio@2200000 { > + status = "okay"; > + }; > + > + gpio@c2f0000 { > + status = "okay"; > + }; > + > pwm@c340000 { > status = "okay"; > }; > -- > 2.17.1 >
On Fri, Nov 22, 2019 at 04:15:05PM +0530, Vidya Sagar wrote: > Add endpoint mode support for PCIe C5 controller in P2972-0000 platform > with information about supplies, PHY, PERST GPIO and GPIO that controls > PCIe reference clock coming from the host system. > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > .../boot/dts/nvidia/tegra194-p2972-0000.dts | 29 +++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > index 7eb64b816e08..58c3a9677bc8 100644 > --- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > @@ -43,6 +43,19 @@ > > gpio@c2f0000 { > status = "okay"; > + /* > + * Change the below node's status to 'okay' when > + * PCIe C5 controller is enabled to operate in endpoint > + * to allow REFCLK from the host system to flow into > + * the controller. > + */ > + pex-refclk-sel-high { > + gpio-hog; > + output-high; > + gpios = <TEGRA194_AON_GPIO(AA, 5) 0>; > + label = "pex_refclk_sel_high"; > + status = "disabled"; > + }; Why don't we put this into the PCIe controller's node as a reference to that GPIO? Seems like the controller would know exactly when this pin needs to go high or low, so why does it have to be a hog? Thierry > }; > > pwm@c340000 { > @@ -144,6 +157,22 @@ > "p2u-5", "p2u-6", "p2u-7"; > }; > > + pcie_ep@141a0000 { > + status = "disabled"; > + > + vddio-pex-ctl-supply = <&vdd_1v8ao>; > + > + nvidia,pex-rst-gpio = <&gpio TEGRA194_MAIN_GPIO(GG, 1) > + GPIO_ACTIVE_LOW>; > + > + phys = <&p2u_nvhs_0>, <&p2u_nvhs_1>, <&p2u_nvhs_2>, > + <&p2u_nvhs_3>, <&p2u_nvhs_4>, <&p2u_nvhs_5>, > + <&p2u_nvhs_6>, <&p2u_nvhs_7>; > + > + phy-names = "p2u-0", "p2u-1", "p2u-2", "p2u-3", "p2u-4", > + "p2u-5", "p2u-6", "p2u-7"; > + }; > + > fan: fan { > compatible = "pwm-fan"; > pwms = <&pwm4 0 45334>; > -- > 2.17.1 >
On 11/22/2019 6:50 PM, Thierry Reding wrote: > On Fri, Nov 22, 2019 at 04:15:04PM +0530, Vidya Sagar wrote: >> Enable GPIO controllers nodes for P2972-0000 platform which are required >> by other controllers in the SoC for example when PCIe C5 controller >> operates in endpoint mode. >> >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts | 8 ++++++++ >> 1 file changed, 8 insertions(+) > > The GPIO controllers are enabled by default, so there's no need to > explicitly enable them. Yup. I'll remove this patch. Since I didn't see 'status' entry in GPIO nodes in tegra194.dtsi file, I thought it means 'disabled'. But, contrary to my understanding, absence of 'status' entry seems to mean 'okay'. - Vidya Sagar > > Thierry > >> >> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts >> index 353a6a22196d..7eb64b816e08 100644 >> --- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts >> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts >> @@ -37,6 +37,14 @@ >> status = "okay"; >> }; >> >> + gpio@2200000 { >> + status = "okay"; >> + }; >> + >> + gpio@c2f0000 { >> + status = "okay"; >> + }; >> + >> pwm@c340000 { >> status = "okay"; >> }; >> -- >> 2.17.1 >>
On 11/22/2019 6:55 PM, Thierry Reding wrote: > On Fri, Nov 22, 2019 at 04:15:05PM +0530, Vidya Sagar wrote: >> Add endpoint mode support for PCIe C5 controller in P2972-0000 platform >> with information about supplies, PHY, PERST GPIO and GPIO that controls >> PCIe reference clock coming from the host system. >> >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> .../boot/dts/nvidia/tegra194-p2972-0000.dts | 29 +++++++++++++++++++ >> 1 file changed, 29 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts >> index 7eb64b816e08..58c3a9677bc8 100644 >> --- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts >> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts >> @@ -43,6 +43,19 @@ >> >> gpio@c2f0000 { >> status = "okay"; >> + /* >> + * Change the below node's status to 'okay' when >> + * PCIe C5 controller is enabled to operate in endpoint >> + * to allow REFCLK from the host system to flow into >> + * the controller. >> + */ >> + pex-refclk-sel-high { >> + gpio-hog; >> + output-high; >> + gpios = <TEGRA194_AON_GPIO(AA, 5) 0>; >> + label = "pex_refclk_sel_high"; >> + status = "disabled"; >> + }; > > Why don't we put this into the PCIe controller's node as a reference to > that GPIO? Seems like the controller would know exactly when this pin > needs to go high or low, so why does it have to be a hog? > > Thierry Are you saying something like 'nvidia,enable-refclk-in'? I was thinking, since this is like a board level configuration specific to Jetson-Xavier, it would suffice to just hog it according to the mode of operation of PCIe controller. But, I see one advantage of referencing it in the PCIe node (so that the driver can configure it as and when needed) is that one has to be careful just to enable either PCIe RP or EP node and not worry about other thing (like this). Let me know if I got this right. - Vidya Sagar > >> }; >> >> pwm@c340000 { >> @@ -144,6 +157,22 @@ >> "p2u-5", "p2u-6", "p2u-7"; >> }; >> >> + pcie_ep@141a0000 { >> + status = "disabled"; >> + >> + vddio-pex-ctl-supply = <&vdd_1v8ao>; >> + >> + nvidia,pex-rst-gpio = <&gpio TEGRA194_MAIN_GPIO(GG, 1) >> + GPIO_ACTIVE_LOW>; >> + >> + phys = <&p2u_nvhs_0>, <&p2u_nvhs_1>, <&p2u_nvhs_2>, >> + <&p2u_nvhs_3>, <&p2u_nvhs_4>, <&p2u_nvhs_5>, >> + <&p2u_nvhs_6>, <&p2u_nvhs_7>; >> + >> + phy-names = "p2u-0", "p2u-1", "p2u-2", "p2u-3", "p2u-4", >> + "p2u-5", "p2u-6", "p2u-7"; >> + }; >> + >> fan: fan { >> compatible = "pwm-fan"; >> pwms = <&pwm4 0 45334>; >> -- >> 2.17.1 >>
On Mon, Nov 25, 2019 at 12:30:53PM +0530, Vidya Sagar wrote: > On 11/22/2019 6:55 PM, Thierry Reding wrote: > > On Fri, Nov 22, 2019 at 04:15:05PM +0530, Vidya Sagar wrote: > > > Add endpoint mode support for PCIe C5 controller in P2972-0000 platform > > > with information about supplies, PHY, PERST GPIO and GPIO that controls > > > PCIe reference clock coming from the host system. > > > > > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > > > --- > > > .../boot/dts/nvidia/tegra194-p2972-0000.dts | 29 +++++++++++++++++++ > > > 1 file changed, 29 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > > > index 7eb64b816e08..58c3a9677bc8 100644 > > > --- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > > > +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > > > @@ -43,6 +43,19 @@ > > > gpio@c2f0000 { > > > status = "okay"; > > > + /* > > > + * Change the below node's status to 'okay' when > > > + * PCIe C5 controller is enabled to operate in endpoint > > > + * to allow REFCLK from the host system to flow into > > > + * the controller. > > > + */ > > > + pex-refclk-sel-high { > > > + gpio-hog; > > > + output-high; > > > + gpios = <TEGRA194_AON_GPIO(AA, 5) 0>; > > > + label = "pex_refclk_sel_high"; > > > + status = "disabled"; > > > + }; > > > > Why don't we put this into the PCIe controller's node as a reference to > > that GPIO? Seems like the controller would know exactly when this pin > > needs to go high or low, so why does it have to be a hog? > > > > Thierry > Are you saying something like 'nvidia,enable-refclk-in'? > I was thinking, since this is like a board level configuration specific to Jetson-Xavier, > it would suffice to just hog it according to the mode of operation of PCIe controller. > But, I see one advantage of referencing it in the PCIe node (so that the driver can configure > it as and when needed) is that one has to be careful just to enable either PCIe RP or EP > node and not worry about other thing (like this). > Let me know if I got this right. Yeah, that's exactly why I think referencing this from the controller and controlling it in the driver is preferable. If this is some sort of select signal I think it makes sense to name it "nvidia,refclk-select-gpios" or something. Does this appear in the schematic somewhere? Or does the IP have a name for this? Those are usually good places to look for inspiration on the name because it's what hardware designers will be familiar with and they are technically the ones who should write the DT, even if that's rarely the case. Thierry > > - Vidya Sagar > > > > > > }; > > > pwm@c340000 { > > > @@ -144,6 +157,22 @@ > > > "p2u-5", "p2u-6", "p2u-7"; > > > }; > > > + pcie_ep@141a0000 { > > > + status = "disabled"; > > > + > > > + vddio-pex-ctl-supply = <&vdd_1v8ao>; > > > + > > > + nvidia,pex-rst-gpio = <&gpio TEGRA194_MAIN_GPIO(GG, 1) > > > + GPIO_ACTIVE_LOW>; > > > + > > > + phys = <&p2u_nvhs_0>, <&p2u_nvhs_1>, <&p2u_nvhs_2>, > > > + <&p2u_nvhs_3>, <&p2u_nvhs_4>, <&p2u_nvhs_5>, > > > + <&p2u_nvhs_6>, <&p2u_nvhs_7>; > > > + > > > + phy-names = "p2u-0", "p2u-1", "p2u-2", "p2u-3", "p2u-4", > > > + "p2u-5", "p2u-6", "p2u-7"; > > > + }; > > > + > > > fan: fan { > > > compatible = "pwm-fan"; > > > pwms = <&pwm4 0 45334>; > > > -- > > > 2.17.1 > > > >
On 11/25/2019 12:55 PM, Thierry Reding wrote: > On Mon, Nov 25, 2019 at 12:30:53PM +0530, Vidya Sagar wrote: >> On 11/22/2019 6:55 PM, Thierry Reding wrote: >>> On Fri, Nov 22, 2019 at 04:15:05PM +0530, Vidya Sagar wrote: >>>> Add endpoint mode support for PCIe C5 controller in P2972-0000 platform >>>> with information about supplies, PHY, PERST GPIO and GPIO that controls >>>> PCIe reference clock coming from the host system. >>>> >>>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >>>> --- >>>> .../boot/dts/nvidia/tegra194-p2972-0000.dts | 29 +++++++++++++++++++ >>>> 1 file changed, 29 insertions(+) >>>> >>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts >>>> index 7eb64b816e08..58c3a9677bc8 100644 >>>> --- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts >>>> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts >>>> @@ -43,6 +43,19 @@ >>>> gpio@c2f0000 { >>>> status = "okay"; >>>> + /* >>>> + * Change the below node's status to 'okay' when >>>> + * PCIe C5 controller is enabled to operate in endpoint >>>> + * to allow REFCLK from the host system to flow into >>>> + * the controller. >>>> + */ >>>> + pex-refclk-sel-high { >>>> + gpio-hog; >>>> + output-high; >>>> + gpios = <TEGRA194_AON_GPIO(AA, 5) 0>; >>>> + label = "pex_refclk_sel_high"; >>>> + status = "disabled"; >>>> + }; >>> >>> Why don't we put this into the PCIe controller's node as a reference to >>> that GPIO? Seems like the controller would know exactly when this pin >>> needs to go high or low, so why does it have to be a hog? >>> >>> Thierry >> Are you saying something like 'nvidia,enable-refclk-in'? >> I was thinking, since this is like a board level configuration specific to Jetson-Xavier, >> it would suffice to just hog it according to the mode of operation of PCIe controller. >> But, I see one advantage of referencing it in the PCIe node (so that the driver can configure >> it as and when needed) is that one has to be careful just to enable either PCIe RP or EP >> node and not worry about other thing (like this). >> Let me know if I got this right. > > Yeah, that's exactly why I think referencing this from the controller > and controlling it in the driver is preferable. > > If this is some sort of select signal I think it makes sense to name it > "nvidia,refclk-select-gpios" or something. Does this appear in the > schematic somewhere? Or does the IP have a name for this? Those are > usually good places to look for inspiration on the name because it's > what hardware designers will be familiar with and they are technically > the ones who should write the DT, even if that's rarely the case. Schematic has "PEX_REFCLK_SEL" name. I would go with 'nvidia,refclk-select-gpios' and make the change. - Vidya Sagar > > Thierry > >> >> - Vidya Sagar >> >>> >>>> }; >>>> pwm@c340000 { >>>> @@ -144,6 +157,22 @@ >>>> "p2u-5", "p2u-6", "p2u-7"; >>>> }; >>>> + pcie_ep@141a0000 { >>>> + status = "disabled"; >>>> + >>>> + vddio-pex-ctl-supply = <&vdd_1v8ao>; >>>> + >>>> + nvidia,pex-rst-gpio = <&gpio TEGRA194_MAIN_GPIO(GG, 1) >>>> + GPIO_ACTIVE_LOW>; >>>> + >>>> + phys = <&p2u_nvhs_0>, <&p2u_nvhs_1>, <&p2u_nvhs_2>, >>>> + <&p2u_nvhs_3>, <&p2u_nvhs_4>, <&p2u_nvhs_5>, >>>> + <&p2u_nvhs_6>, <&p2u_nvhs_7>; >>>> + >>>> + phy-names = "p2u-0", "p2u-1", "p2u-2", "p2u-3", "p2u-4", >>>> + "p2u-5", "p2u-6", "p2u-7"; >>>> + }; >>>> + >>>> fan: fan { >>>> compatible = "pwm-fan"; >>>> pwms = <&pwm4 0 45334>; >>>> -- >>>> 2.17.1 >>>> >>
On Mon, Nov 25, 2019 at 01:03:27PM +0530, Vidya Sagar wrote: > On 11/25/2019 12:55 PM, Thierry Reding wrote: > > On Mon, Nov 25, 2019 at 12:30:53PM +0530, Vidya Sagar wrote: > > > On 11/22/2019 6:55 PM, Thierry Reding wrote: > > > > On Fri, Nov 22, 2019 at 04:15:05PM +0530, Vidya Sagar wrote: > > > > > Add endpoint mode support for PCIe C5 controller in P2972-0000 platform > > > > > with information about supplies, PHY, PERST GPIO and GPIO that controls > > > > > PCIe reference clock coming from the host system. > > > > > > > > > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > > > > > --- > > > > > .../boot/dts/nvidia/tegra194-p2972-0000.dts | 29 +++++++++++++++++++ > > > > > 1 file changed, 29 insertions(+) > > > > > > > > > > diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > > > > > index 7eb64b816e08..58c3a9677bc8 100644 > > > > > --- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > > > > > +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts > > > > > @@ -43,6 +43,19 @@ > > > > > gpio@c2f0000 { > > > > > status = "okay"; > > > > > + /* > > > > > + * Change the below node's status to 'okay' when > > > > > + * PCIe C5 controller is enabled to operate in endpoint > > > > > + * to allow REFCLK from the host system to flow into > > > > > + * the controller. > > > > > + */ > > > > > + pex-refclk-sel-high { > > > > > + gpio-hog; > > > > > + output-high; > > > > > + gpios = <TEGRA194_AON_GPIO(AA, 5) 0>; > > > > > + label = "pex_refclk_sel_high"; > > > > > + status = "disabled"; > > > > > + }; > > > > > > > > Why don't we put this into the PCIe controller's node as a reference to > > > > that GPIO? Seems like the controller would know exactly when this pin > > > > needs to go high or low, so why does it have to be a hog? > > > > > > > > Thierry > > > Are you saying something like 'nvidia,enable-refclk-in'? > > > I was thinking, since this is like a board level configuration specific to Jetson-Xavier, > > > it would suffice to just hog it according to the mode of operation of PCIe controller. > > > But, I see one advantage of referencing it in the PCIe node (so that the driver can configure > > > it as and when needed) is that one has to be careful just to enable either PCIe RP or EP > > > node and not worry about other thing (like this). > > > Let me know if I got this right. > > > > Yeah, that's exactly why I think referencing this from the controller > > and controlling it in the driver is preferable. > > > > If this is some sort of select signal I think it makes sense to name it > > "nvidia,refclk-select-gpios" or something. Does this appear in the > > schematic somewhere? Or does the IP have a name for this? Those are > > usually good places to look for inspiration on the name because it's > > what hardware designers will be familiar with and they are technically > > the ones who should write the DT, even if that's rarely the case. > Schematic has "PEX_REFCLK_SEL" name. > I would go with 'nvidia,refclk-select-gpios' and make the change. It might be worth checking the interface definition of the IP if you have access to that, since it may be using a different name from the one that we have in the schematics. Also, given that other instantiations don't have this, I'm beginning to wonder if this is perhaps somehow specific to how this is used in this particular board design. If it is, then I think the nvidia, prefix would be appropriate. But if this is something that is part of the IP interface then we can probably drop the prefix since it would be applicable to non-NVIDIA instantiations as well. Thierry > > - Vidya Sagar > > > > Thierry > > > > > > > > - Vidya Sagar > > > > > > > > > > > > }; > > > > > pwm@c340000 { > > > > > @@ -144,6 +157,22 @@ > > > > > "p2u-5", "p2u-6", "p2u-7"; > > > > > }; > > > > > + pcie_ep@141a0000 { > > > > > + status = "disabled"; > > > > > + > > > > > + vddio-pex-ctl-supply = <&vdd_1v8ao>; > > > > > + > > > > > + nvidia,pex-rst-gpio = <&gpio TEGRA194_MAIN_GPIO(GG, 1) > > > > > + GPIO_ACTIVE_LOW>; > > > > > + > > > > > + phys = <&p2u_nvhs_0>, <&p2u_nvhs_1>, <&p2u_nvhs_2>, > > > > > + <&p2u_nvhs_3>, <&p2u_nvhs_4>, <&p2u_nvhs_5>, > > > > > + <&p2u_nvhs_6>, <&p2u_nvhs_7>; > > > > > + > > > > > + phy-names = "p2u-0", "p2u-1", "p2u-2", "p2u-3", "p2u-4", > > > > > + "p2u-5", "p2u-6", "p2u-7"; > > > > > + }; > > > > > + > > > > > fan: fan { > > > > > compatible = "pwm-fan"; > > > > > pwms = <&pwm4 0 45334>; > > > > > -- > > > > > 2.17.1 > > > > > > > > >
Tegra194 has three (C0, C4 & C5) dual mode PCIe controllers that can operate either in root port mode or in end point mode but only in one mode at a time. Platform P2972-0000 supports enabling endpoint mode for C5 controller. This patch series adds support for PCIe endpoint mode in both the driver as well as in DT. This patch series depends on the changes made for Synopsys DesignWare endpoint mode subsystem that are currently under review @ https://patchwork.kernel.org/project/linux-pci/list/?series=202211 which in turn depends on the patch made by Kishon @ https://patchwork.kernel.org/patch/10975123/ which is also under review. Vidya Sagar (6): soc/tegra: bpmp: Update ABI header dt-bindings: PCI: tegra: Add DT support for PCIe EP nodes in Tegra194 PCI: tegra: Add support for PCIe endpoint mode in Tegra194 arm64: tegra: Add PCIe endpoint controllers nodes for Tegra194 arm64: tegra: Enable GPIO controllers nodes for P2972-0000 platform arm64: tegra: Add support for PCIe endpoint mode in P2972-0000 platform .../bindings/pci/nvidia,tegra194-pcie-ep.txt | 138 ++++ .../boot/dts/nvidia/tegra194-p2972-0000.dts | 37 + arch/arm64/boot/dts/nvidia/tegra194.dtsi | 99 +++ drivers/pci/controller/dwc/Kconfig | 30 +- drivers/pci/controller/dwc/pcie-tegra194.c | 751 +++++++++++++++++- include/soc/tegra/bpmp-abi.h | 10 +- 6 files changed, 1048 insertions(+), 17 deletions(-) create mode 100644 Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie-ep.txt