| Message ID | 20200623145528.1658337-2-thierry.reding@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [v2,1/2] dt-bindings: pci: tegra: Remove PLL power supplies | expand |
On Tue, Jun 23, 2020 at 04:55:28PM +0200, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > The Tegra PCI controller driver doesn't need to control the PLL power > supplies directly, but rather uses the pads provided by the XUSB pad > controller, which in turn is responsible for supplying power to the > PLLs. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/pci/controller/pci-tegra.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) Hi Lorenzo, do you have any comments on this? Can we get this into v5.9? Thanks, Thierry > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > index 235b456698fc..f87a09d21eb0 100644 > --- a/drivers/pci/controller/pci-tegra.c > +++ b/drivers/pci/controller/pci-tegra.c > @@ -2025,7 +2025,7 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask) > pcie->supplies[i++].supply = "hvdd-pex"; > pcie->supplies[i++].supply = "vddio-pexctl-aud"; > } else if (of_device_is_compatible(np, "nvidia,tegra210-pcie")) { > - pcie->num_supplies = 6; > + pcie->num_supplies = 3; > > pcie->supplies = devm_kcalloc(pcie->dev, pcie->num_supplies, > sizeof(*pcie->supplies), > @@ -2033,14 +2033,11 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask) > if (!pcie->supplies) > return -ENOMEM; > > - pcie->supplies[i++].supply = "avdd-pll-uerefe"; > pcie->supplies[i++].supply = "hvddio-pex"; > pcie->supplies[i++].supply = "dvddio-pex"; > - pcie->supplies[i++].supply = "dvdd-pex-pll"; > - pcie->supplies[i++].supply = "hvdd-pex-pll-e"; > pcie->supplies[i++].supply = "vddio-pex-ctl"; > } else if (of_device_is_compatible(np, "nvidia,tegra124-pcie")) { > - pcie->num_supplies = 7; > + pcie->num_supplies = 4; > > pcie->supplies = devm_kcalloc(dev, pcie->num_supplies, > sizeof(*pcie->supplies), > @@ -2050,11 +2047,8 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask) > > pcie->supplies[i++].supply = "avddio-pex"; > pcie->supplies[i++].supply = "dvddio-pex"; > - pcie->supplies[i++].supply = "avdd-pex-pll"; > pcie->supplies[i++].supply = "hvdd-pex"; > - pcie->supplies[i++].supply = "hvdd-pex-pll-e"; > pcie->supplies[i++].supply = "vddio-pex-ctl"; > - pcie->supplies[i++].supply = "avdd-pll-erefe"; > } else if (of_device_is_compatible(np, "nvidia,tegra30-pcie")) { > bool need_pexa = false, need_pexb = false; > > -- > 2.27.0 >
On Thu, Jul 16, 2020 at 03:00:34PM +0200, Thierry Reding wrote: > On Tue, Jun 23, 2020 at 04:55:28PM +0200, Thierry Reding wrote: > > From: Thierry Reding <treding@nvidia.com> > > > > The Tegra PCI controller driver doesn't need to control the PLL power > > supplies directly, but rather uses the pads provided by the XUSB pad > > controller, which in turn is responsible for supplying power to the > > PLLs. > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > drivers/pci/controller/pci-tegra.c | 10 ++-------- > > 1 file changed, 2 insertions(+), 8 deletions(-) > > Hi Lorenzo, > > do you have any comments on this? Can we get this into v5.9? Yes we can if Rob is happy with patch (1). Thanks, Lorenzo > Thanks, > Thierry > > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > > index 235b456698fc..f87a09d21eb0 100644 > > --- a/drivers/pci/controller/pci-tegra.c > > +++ b/drivers/pci/controller/pci-tegra.c > > @@ -2025,7 +2025,7 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask) > > pcie->supplies[i++].supply = "hvdd-pex"; > > pcie->supplies[i++].supply = "vddio-pexctl-aud"; > > } else if (of_device_is_compatible(np, "nvidia,tegra210-pcie")) { > > - pcie->num_supplies = 6; > > + pcie->num_supplies = 3; > > > > pcie->supplies = devm_kcalloc(pcie->dev, pcie->num_supplies, > > sizeof(*pcie->supplies), > > @@ -2033,14 +2033,11 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask) > > if (!pcie->supplies) > > return -ENOMEM; > > > > - pcie->supplies[i++].supply = "avdd-pll-uerefe"; > > pcie->supplies[i++].supply = "hvddio-pex"; > > pcie->supplies[i++].supply = "dvddio-pex"; > > - pcie->supplies[i++].supply = "dvdd-pex-pll"; > > - pcie->supplies[i++].supply = "hvdd-pex-pll-e"; > > pcie->supplies[i++].supply = "vddio-pex-ctl"; > > } else if (of_device_is_compatible(np, "nvidia,tegra124-pcie")) { > > - pcie->num_supplies = 7; > > + pcie->num_supplies = 4; > > > > pcie->supplies = devm_kcalloc(dev, pcie->num_supplies, > > sizeof(*pcie->supplies), > > @@ -2050,11 +2047,8 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask) > > > > pcie->supplies[i++].supply = "avddio-pex"; > > pcie->supplies[i++].supply = "dvddio-pex"; > > - pcie->supplies[i++].supply = "avdd-pex-pll"; > > pcie->supplies[i++].supply = "hvdd-pex"; > > - pcie->supplies[i++].supply = "hvdd-pex-pll-e"; > > pcie->supplies[i++].supply = "vddio-pex-ctl"; > > - pcie->supplies[i++].supply = "avdd-pll-erefe"; > > } else if (of_device_is_compatible(np, "nvidia,tegra30-pcie")) { > > bool need_pexa = false, need_pexb = false; > > > > -- > > 2.27.0 > >
On Thu, Jul 16, 2020 at 03:07:13PM +0100, Lorenzo Pieralisi wrote: > On Thu, Jul 16, 2020 at 03:00:34PM +0200, Thierry Reding wrote: > > On Tue, Jun 23, 2020 at 04:55:28PM +0200, Thierry Reding wrote: > > > From: Thierry Reding <treding@nvidia.com> > > > > > > The Tegra PCI controller driver doesn't need to control the PLL power > > > supplies directly, but rather uses the pads provided by the XUSB pad > > > controller, which in turn is responsible for supplying power to the > > > PLLs. > > > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > > --- > > > drivers/pci/controller/pci-tegra.c | 10 ++-------- > > > 1 file changed, 2 insertions(+), 8 deletions(-) > > > > Hi Lorenzo, > > > > do you have any comments on this? Can we get this into v5.9? > > Yes we can if Rob is happy with patch (1). I think this is mostly orthogonal to the bindings. We're already controlling these supplies from the UPHY driver and controlling them from the PCI driver is completely broken. So I think technically these two patches could be applied separately, but it's fine if you want to wait for Rob's feedback. Thierry
On Tue, Jun 23, 2020 at 8:55 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > From: Thierry Reding <treding@nvidia.com> > > The Tegra PCI controller driver doesn't need to control the PLL power > supplies directly, but rather uses the pads provided by the XUSB pad > controller, which in turn is responsible for supplying power to the > PLLs. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/pci/controller/pci-tegra.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) What's going to happen here with a new dtb and an old kernel? Is it going to error out due to missing supplies? Rob
On Mon, Jul 27, 2020 at 10:21:42AM -0600, Rob Herring wrote: > On Tue, Jun 23, 2020 at 8:55 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > From: Thierry Reding <treding@nvidia.com> > > > > The Tegra PCI controller driver doesn't need to control the PLL power > > supplies directly, but rather uses the pads provided by the XUSB pad > > controller, which in turn is responsible for supplying power to the > > PLLs. > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > drivers/pci/controller/pci-tegra.c | 10 ++-------- > > 1 file changed, 2 insertions(+), 8 deletions(-) > > What's going to happen here with a new dtb and an old kernel? Is it > going to error out due to missing supplies? It's not going to error out but fallback to the "dummy" regulator, so this should be fine from a forwards-compatibility point of view. Though I didn't think we technically cared about that direction very much. Thierry
On Mon, Jul 27, 2020 at 11:21 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > On Mon, Jul 27, 2020 at 10:21:42AM -0600, Rob Herring wrote: > > On Tue, Jun 23, 2020 at 8:55 AM Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > > From: Thierry Reding <treding@nvidia.com> > > > > > > The Tegra PCI controller driver doesn't need to control the PLL power > > > supplies directly, but rather uses the pads provided by the XUSB pad > > > controller, which in turn is responsible for supplying power to the > > > PLLs. > > > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > > --- > > > drivers/pci/controller/pci-tegra.c | 10 ++-------- > > > 1 file changed, 2 insertions(+), 8 deletions(-) > > > > What's going to happen here with a new dtb and an old kernel? Is it > > going to error out due to missing supplies? > > It's not going to error out but fallback to the "dummy" regulator, so > this should be fine from a forwards-compatibility point of view. Though > I didn't think we technically cared about that direction very much. AIUI, SUSE ships newer DTs with stable kernels. Of course, the dtb's shouldn't really come from the OS vendors, but you wouldn't want a newer firmware (w/ dtb) to break your OS either. In any case, Reviewed-by: Rob Herring <robh@kernel.org> Rob
diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c index 235b456698fc..f87a09d21eb0 100644 --- a/drivers/pci/controller/pci-tegra.c +++ b/drivers/pci/controller/pci-tegra.c @@ -2025,7 +2025,7 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask) pcie->supplies[i++].supply = "hvdd-pex"; pcie->supplies[i++].supply = "vddio-pexctl-aud"; } else if (of_device_is_compatible(np, "nvidia,tegra210-pcie")) { - pcie->num_supplies = 6; + pcie->num_supplies = 3; pcie->supplies = devm_kcalloc(pcie->dev, pcie->num_supplies, sizeof(*pcie->supplies), @@ -2033,14 +2033,11 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask) if (!pcie->supplies) return -ENOMEM; - pcie->supplies[i++].supply = "avdd-pll-uerefe"; pcie->supplies[i++].supply = "hvddio-pex"; pcie->supplies[i++].supply = "dvddio-pex"; - pcie->supplies[i++].supply = "dvdd-pex-pll"; - pcie->supplies[i++].supply = "hvdd-pex-pll-e"; pcie->supplies[i++].supply = "vddio-pex-ctl"; } else if (of_device_is_compatible(np, "nvidia,tegra124-pcie")) { - pcie->num_supplies = 7; + pcie->num_supplies = 4; pcie->supplies = devm_kcalloc(dev, pcie->num_supplies, sizeof(*pcie->supplies), @@ -2050,11 +2047,8 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask) pcie->supplies[i++].supply = "avddio-pex"; pcie->supplies[i++].supply = "dvddio-pex"; - pcie->supplies[i++].supply = "avdd-pex-pll"; pcie->supplies[i++].supply = "hvdd-pex"; - pcie->supplies[i++].supply = "hvdd-pex-pll-e"; pcie->supplies[i++].supply = "vddio-pex-ctl"; - pcie->supplies[i++].supply = "avdd-pll-erefe"; } else if (of_device_is_compatible(np, "nvidia,tegra30-pcie")) { bool need_pexa = false, need_pexb = false;