* [RFC PATCH 0/3] soc/tegra: Turn on XUSB partitions @ 2016-06-28 11:20 Jon Hunter 2016-06-28 11:20 ` [RFC PATCH 1/3] soc/tegra: pmc: Initialise power partitions early Jon Hunter ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Jon Hunter @ 2016-06-28 11:20 UTC (permalink / raw) To: Stephen Warren, Thierry Reding, Alexandre Courbot Cc: linux-tegra, linux-kernel, Jon Hunter The Tegra XHCI driver currently assumes the XUSB power partitions have been enabled by the bootloader. This is not for the Tegra210 Smaug board and so cause the kernel to hang when enabling XHCI support. Although the XHCI driver itself needs to manage these partitions, for now enable the partitions if the XHCI driver is enabled. In order to do this I have made a fundamental change to the PMC driver to initialise the power partitions during early init. A benefit of this is that if CONFIG_PM_GENERIC_DOMAINS is not enabled, then we can simply turn on the partitions early before any devices are probed. This is based upon the PMC fixes series [0]. [0] http://marc.info/?l=linux-tegra&m=146711078013182&w=2 Jon Hunter (3): soc/tegra: pmc: Initialise power partitions early soc/tegra: pmc: Enable XUSB partitions on boot arm64: tegra210: Add XUSB powergates arch/arm64/boot/dts/nvidia/tegra210.dtsi | 24 +++++++++++++++++ drivers/soc/tegra/pmc.c | 45 ++++++++++++++++++++++++-------- 2 files changed, 58 insertions(+), 11 deletions(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH 1/3] soc/tegra: pmc: Initialise power partitions early 2016-06-28 11:20 [RFC PATCH 0/3] soc/tegra: Turn on XUSB partitions Jon Hunter @ 2016-06-28 11:20 ` Jon Hunter 2016-06-30 10:17 ` Thierry Reding 2016-06-28 11:20 ` [RFC PATCH 2/3] soc/tegra: pmc: Enable XUSB partitions on boot Jon Hunter 2016-06-28 11:20 ` [RFC PATCH 3/3] arm64: tegra210: Add XUSB powergates Jon Hunter 2 siblings, 1 reply; 10+ messages in thread From: Jon Hunter @ 2016-06-28 11:20 UTC (permalink / raw) To: Stephen Warren, Thierry Reding, Alexandre Courbot Cc: linux-tegra, linux-kernel, Jon Hunter If CONFIG_PM_GENERIC_DOMAINS is not enabled, then power partitions associated with a device will not be enabled automatically by the PM core when the device is in use. To avoid situations where a device in a power partition is to be used but the partition is not enabled, initialise the power partitions for Tegra early in the boot process and if CONFIG_PM_GENERIC_DOMAINS is not enabled, then power on all partitions defined in the device-tree blob. Note that if CONFIG_PM_GENERIC_DOMAINS is not enabled, after the partitions are turned on, the clocks and resets used as part of the sequence for turning on the partition are released again as they are no longer needed by the PMC driver. Another benefit of this is that this avoids any issues of sharing resets between the PMC driver and other device drivers that may wish to independently control a particular reset. Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- drivers/soc/tegra/pmc.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c index 1f702538f8ec..64678ff2173e 100644 --- a/drivers/soc/tegra/pmc.c +++ b/drivers/soc/tegra/pmc.c @@ -788,7 +788,7 @@ error: static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np) { struct tegra_powergate *pg; - bool off; + bool off, err = true; int id; pg = kzalloc(sizeof(*pg), GFP_KERNEL); @@ -819,6 +819,9 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np) if (tegra_powergate_of_get_resets(pg, np, off)) goto remove_clks; + if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) + goto power_on_cleanup; + pm_genpd_init(&pg->genpd, NULL, off); if (of_genpd_add_provider_simple(np, &pg->genpd)) @@ -828,6 +831,11 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np) return; +power_on_cleanup: + if (off) + WARN_ON(tegra_powergate_power_up(pg, true)); + err = false; + remove_resets: while (pg->num_resets--) reset_control_put(pg->resets[pg->num_resets]); @@ -845,14 +853,23 @@ free_mem: kfree(pg); error: - dev_err(pmc->dev, "failed to create power domain for %s\n", np->name); + if (err) + dev_err(pmc->dev, "failed to configure partition %s\n", + np->name); } -static void tegra_powergate_init(struct tegra_pmc *pmc) +static void tegra_powergate_init(struct tegra_pmc *pmc, + struct device_node *parent) { struct device_node *np, *child; + unsigned int i; - np = of_get_child_by_name(pmc->dev->of_node, "powergates"); + /* Create a bit-map of the available and valid partitions */ + for (i = 0; i < pmc->soc->num_powergates; i++) + if (pmc->soc->powergates[i]) + set_bit(i, pmc->powergates_available); + + np = of_get_child_by_name(parent, "powergates"); if (!np) return; @@ -1273,8 +1290,6 @@ static int tegra_pmc_probe(struct platform_device *pdev) return err; } - tegra_powergate_init(pmc); - mutex_lock(&pmc->powergates_lock); iounmap(pmc->base); pmc->base = base; @@ -1508,7 +1523,6 @@ static int __init tegra_pmc_early_init(void) const struct of_device_id *match; struct device_node *np; struct resource regs; - unsigned int i; bool invert; u32 value; @@ -1563,10 +1577,7 @@ static int __init tegra_pmc_early_init(void) if (np) { pmc->soc = match->data; - /* Create a bit-map of the available and valid partitions */ - for (i = 0; i < pmc->soc->num_powergates; i++) - if (pmc->soc->powergates[i]) - set_bit(i, pmc->powergates_available); + tegra_powergate_init(pmc, np); /* * Invert the interrupt polarity if a PMC device tree node -- 2.1.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 1/3] soc/tegra: pmc: Initialise power partitions early 2016-06-28 11:20 ` [RFC PATCH 1/3] soc/tegra: pmc: Initialise power partitions early Jon Hunter @ 2016-06-30 10:17 ` Thierry Reding 2016-06-30 10:20 ` Jon Hunter 0 siblings, 1 reply; 10+ messages in thread From: Thierry Reding @ 2016-06-30 10:17 UTC (permalink / raw) To: Jon Hunter; +Cc: Stephen Warren, Alexandre Courbot, linux-tegra, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2719 bytes --] On Tue, Jun 28, 2016 at 12:20:42PM +0100, Jon Hunter wrote: > If CONFIG_PM_GENERIC_DOMAINS is not enabled, then power partitions > associated with a device will not be enabled automatically by the PM > core when the device is in use. To avoid situations where a device in > a power partition is to be used but the partition is not enabled, > initialise the power partitions for Tegra early in the boot process and > if CONFIG_PM_GENERIC_DOMAINS is not enabled, then power on all > partitions defined in the device-tree blob. > > Note that if CONFIG_PM_GENERIC_DOMAINS is not enabled, after the > partitions are turned on, the clocks and resets used as part of the > sequence for turning on the partition are released again as they are no > longer needed by the PMC driver. Another benefit of this is that this > avoids any issues of sharing resets between the PMC driver and other > device drivers that may wish to independently control a particular > reset. > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > drivers/soc/tegra/pmc.c | 33 ++++++++++++++++++++++----------- > 1 file changed, 22 insertions(+), 11 deletions(-) > > diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c > index 1f702538f8ec..64678ff2173e 100644 > --- a/drivers/soc/tegra/pmc.c > +++ b/drivers/soc/tegra/pmc.c > @@ -788,7 +788,7 @@ error: > static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np) > { > struct tegra_powergate *pg; > - bool off; > + bool off, err = true; > int id; > > pg = kzalloc(sizeof(*pg), GFP_KERNEL); > @@ -819,6 +819,9 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np) > if (tegra_powergate_of_get_resets(pg, np, off)) > goto remove_clks; > > + if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) > + goto power_on_cleanup; > + > pm_genpd_init(&pg->genpd, NULL, off); > > if (of_genpd_add_provider_simple(np, &pg->genpd)) > @@ -828,6 +831,11 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np) > > return; > > +power_on_cleanup: > + if (off) > + WARN_ON(tegra_powergate_power_up(pg, true)); > + err = false; > + > remove_resets: > while (pg->num_resets--) > reset_control_put(pg->resets[pg->num_resets]); > @@ -845,14 +853,23 @@ free_mem: > kfree(pg); > > error: > - dev_err(pmc->dev, "failed to create power domain for %s\n", np->name); > + if (err) > + dev_err(pmc->dev, "failed to configure partition %s\n", > + np->name); This is beginning to look very spaghetti-like. Do we really need the error message here? Could we instead add more explicit error messages before the gotos above? Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 1/3] soc/tegra: pmc: Initialise power partitions early 2016-06-30 10:17 ` Thierry Reding @ 2016-06-30 10:20 ` Jon Hunter 0 siblings, 0 replies; 10+ messages in thread From: Jon Hunter @ 2016-06-30 10:20 UTC (permalink / raw) To: Thierry Reding Cc: Stephen Warren, Alexandre Courbot, linux-tegra, linux-kernel On 30/06/16 11:17, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Tue, Jun 28, 2016 at 12:20:42PM +0100, Jon Hunter wrote: >> If CONFIG_PM_GENERIC_DOMAINS is not enabled, then power partitions >> associated with a device will not be enabled automatically by the PM >> core when the device is in use. To avoid situations where a device in >> a power partition is to be used but the partition is not enabled, >> initialise the power partitions for Tegra early in the boot process and >> if CONFIG_PM_GENERIC_DOMAINS is not enabled, then power on all >> partitions defined in the device-tree blob. >> >> Note that if CONFIG_PM_GENERIC_DOMAINS is not enabled, after the >> partitions are turned on, the clocks and resets used as part of the >> sequence for turning on the partition are released again as they are no >> longer needed by the PMC driver. Another benefit of this is that this >> avoids any issues of sharing resets between the PMC driver and other >> device drivers that may wish to independently control a particular >> reset. >> >> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >> --- >> drivers/soc/tegra/pmc.c | 33 ++++++++++++++++++++++----------- >> 1 file changed, 22 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c >> index 1f702538f8ec..64678ff2173e 100644 >> --- a/drivers/soc/tegra/pmc.c >> +++ b/drivers/soc/tegra/pmc.c >> @@ -788,7 +788,7 @@ error: >> static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np) >> { >> struct tegra_powergate *pg; >> - bool off; >> + bool off, err = true; >> int id; >> >> pg = kzalloc(sizeof(*pg), GFP_KERNEL); >> @@ -819,6 +819,9 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np) >> if (tegra_powergate_of_get_resets(pg, np, off)) >> goto remove_clks; >> >> + if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) >> + goto power_on_cleanup; >> + >> pm_genpd_init(&pg->genpd, NULL, off); >> >> if (of_genpd_add_provider_simple(np, &pg->genpd)) >> @@ -828,6 +831,11 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np) >> >> return; >> >> +power_on_cleanup: >> + if (off) >> + WARN_ON(tegra_powergate_power_up(pg, true)); >> + err = false; >> + >> remove_resets: >> while (pg->num_resets--) >> reset_control_put(pg->resets[pg->num_resets]); >> @@ -845,14 +853,23 @@ free_mem: >> kfree(pg); >> >> error: >> - dev_err(pmc->dev, "failed to create power domain for %s\n", np->name); >> + if (err) >> + dev_err(pmc->dev, "failed to configure partition %s\n", >> + np->name); > > This is beginning to look very spaghetti-like. Do we really need the > error message here? Could we instead add more explicit error messages > before the gotos above? Yes I can't say I loved it either. Ok, yes will add more specific error messages above gotos instead. Cheers Jon -- nvpublic ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH 2/3] soc/tegra: pmc: Enable XUSB partitions on boot 2016-06-28 11:20 [RFC PATCH 0/3] soc/tegra: Turn on XUSB partitions Jon Hunter 2016-06-28 11:20 ` [RFC PATCH 1/3] soc/tegra: pmc: Initialise power partitions early Jon Hunter @ 2016-06-28 11:20 ` Jon Hunter 2016-06-28 11:20 ` [RFC PATCH 3/3] arm64: tegra210: Add XUSB powergates Jon Hunter 2 siblings, 0 replies; 10+ messages in thread From: Jon Hunter @ 2016-06-28 11:20 UTC (permalink / raw) To: Stephen Warren, Thierry Reding, Alexandre Courbot Cc: linux-tegra, linux-kernel, Jon Hunter The Tegra XHCI driver does not currently manage the Tegra XUSB power partitions and so it these partitions have not been enabled by the bootloader then the system will crash when probing the XHCI device. While proper support for managing the power partitions is being developed to the XHCI driver for Tegra, for now power on all the XUSB partitions for USB host and super-speed on boot if the XHCI driver is enabled. Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- drivers/soc/tegra/pmc.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c index 64678ff2173e..afaa5891c8f5 100644 --- a/drivers/soc/tegra/pmc.c +++ b/drivers/soc/tegra/pmc.c @@ -822,6 +822,18 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np) if (!IS_ENABLED(CONFIG_PM_GENERIC_DOMAINS)) goto power_on_cleanup; + /* + * FIXME: If XHCI is enabled for Tegra, then power-up the XUSB + * host and super-speed partitions. Once the XHCI driver + * manages the partitions itself this code can be removed. Note + * that we don't register these partitions with the genpd core + * to avoid it from powering down the partitions as they appear + * to be unused. + */ + if (IS_ENABLED(CONFIG_USB_XHCI_TEGRA) && + (id == TEGRA_POWERGATE_XUSBA || id == TEGRA_POWERGATE_XUSBC)) + goto power_on_cleanup; + pm_genpd_init(&pg->genpd, NULL, off); if (of_genpd_add_provider_simple(np, &pg->genpd)) -- 2.1.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH 3/3] arm64: tegra210: Add XUSB powergates 2016-06-28 11:20 [RFC PATCH 0/3] soc/tegra: Turn on XUSB partitions Jon Hunter 2016-06-28 11:20 ` [RFC PATCH 1/3] soc/tegra: pmc: Initialise power partitions early Jon Hunter 2016-06-28 11:20 ` [RFC PATCH 2/3] soc/tegra: pmc: Enable XUSB partitions on boot Jon Hunter @ 2016-06-28 11:20 ` Jon Hunter 2016-06-29 15:30 ` Jon Hunter 2 siblings, 1 reply; 10+ messages in thread From: Jon Hunter @ 2016-06-28 11:20 UTC (permalink / raw) To: Stephen Warren, Thierry Reding, Alexandre Courbot Cc: linux-tegra, linux-kernel, Jon Hunter The Tegra210 XUSB subsystem has 3 power partitions which are XUSBA (super-speed logic), XUSBB (USB device logic) and XUSBC (USB host logic). Populate the device-tree nodes for these XUSB partitions. Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- arch/arm64/boot/dts/nvidia/tegra210.dtsi | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi index 65b829b762bb..efb0fd98b789 100644 --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi @@ -670,6 +670,30 @@ <&tegra_car TEGRA210_CLK_MIPI_CAL>; #power-domain-cells = <0>; }; + + pd_xusbss: xusba { + clocks = <&tegra_car TEGRA210_CLK_XUSB_SS>; + clock-names = "xusb_ss"; + resets = <&tegra_car TEGRA210_CLK_XUSB_SS>; + reset-names = "xusb_ss"; + #power-domain-cells = <0>; + }; + + pd_xusbdev: xusbb { + clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>; + clock-names = "xusb_dev"; + resets = <&tegra_car 95>; + reset-names = "xusb_dev"; + #power-domain-cells = <0>; + }; + + pd_xusbhost: xusbc { + clocks = <&tegra_car TEGRA210_CLK_XUSB_HOST>; + clock-names = "xusb_host"; + resets = <&tegra_car TEGRA210_CLK_XUSB_HOST>; + reset-names = "xusb_host"; + #power-domain-cells = <0>; + }; }; }; -- 2.1.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 3/3] arm64: tegra210: Add XUSB powergates 2016-06-28 11:20 ` [RFC PATCH 3/3] arm64: tegra210: Add XUSB powergates Jon Hunter @ 2016-06-29 15:30 ` Jon Hunter 2016-06-29 15:56 ` Thierry Reding 0 siblings, 1 reply; 10+ messages in thread From: Jon Hunter @ 2016-06-29 15:30 UTC (permalink / raw) To: Stephen Warren, Thierry Reding, Alexandre Courbot Cc: linux-tegra, linux-kernel On 28/06/16 12:20, Jon Hunter wrote: > The Tegra210 XUSB subsystem has 3 power partitions which are XUSBA > (super-speed logic), XUSBB (USB device logic) and XUSBC (USB host > logic). Populate the device-tree nodes for these XUSB partitions. > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > arch/arm64/boot/dts/nvidia/tegra210.dtsi | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi > index 65b829b762bb..efb0fd98b789 100644 > --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi > +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi > @@ -670,6 +670,30 @@ > <&tegra_car TEGRA210_CLK_MIPI_CAL>; > #power-domain-cells = <0>; > }; > + > + pd_xusbss: xusba { > + clocks = <&tegra_car TEGRA210_CLK_XUSB_SS>; > + clock-names = "xusb_ss"; > + resets = <&tegra_car TEGRA210_CLK_XUSB_SS>; > + reset-names = "xusb_ss"; > + #power-domain-cells = <0>; > + }; > + > + pd_xusbdev: xusbb { > + clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>; > + clock-names = "xusb_dev"; > + resets = <&tegra_car 95>; > + reset-names = "xusb_dev"; > + #power-domain-cells = <0>; > + }; > + > + pd_xusbhost: xusbc { > + clocks = <&tegra_car TEGRA210_CLK_XUSB_HOST>; > + clock-names = "xusb_host"; > + resets = <&tegra_car TEGRA210_CLK_XUSB_HOST>; > + reset-names = "xusb_host"; > + #power-domain-cells = <0>; > + }; > }; > }; The 'clock-names' and 'reset-names' nodes are not used/required and so I will remove these. Jon -- nvpublic ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 3/3] arm64: tegra210: Add XUSB powergates 2016-06-29 15:30 ` Jon Hunter @ 2016-06-29 15:56 ` Thierry Reding 2016-06-29 16:07 ` Jon Hunter 0 siblings, 1 reply; 10+ messages in thread From: Thierry Reding @ 2016-06-29 15:56 UTC (permalink / raw) To: Jon Hunter; +Cc: Stephen Warren, Alexandre Courbot, linux-tegra, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2196 bytes --] On Wed, Jun 29, 2016 at 04:30:08PM +0100, Jon Hunter wrote: > > On 28/06/16 12:20, Jon Hunter wrote: > > The Tegra210 XUSB subsystem has 3 power partitions which are XUSBA > > (super-speed logic), XUSBB (USB device logic) and XUSBC (USB host > > logic). Populate the device-tree nodes for these XUSB partitions. > > > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > > --- > > arch/arm64/boot/dts/nvidia/tegra210.dtsi | 24 ++++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi > > index 65b829b762bb..efb0fd98b789 100644 > > --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi > > +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi > > @@ -670,6 +670,30 @@ > > <&tegra_car TEGRA210_CLK_MIPI_CAL>; > > #power-domain-cells = <0>; > > }; > > + > > + pd_xusbss: xusba { > > + clocks = <&tegra_car TEGRA210_CLK_XUSB_SS>; > > + clock-names = "xusb_ss"; > > + resets = <&tegra_car TEGRA210_CLK_XUSB_SS>; > > + reset-names = "xusb_ss"; > > + #power-domain-cells = <0>; > > + }; > > + > > + pd_xusbdev: xusbb { > > + clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>; > > + clock-names = "xusb_dev"; > > + resets = <&tegra_car 95>; > > + reset-names = "xusb_dev"; > > + #power-domain-cells = <0>; > > + }; > > + > > + pd_xusbhost: xusbc { > > + clocks = <&tegra_car TEGRA210_CLK_XUSB_HOST>; > > + clock-names = "xusb_host"; > > + resets = <&tegra_car TEGRA210_CLK_XUSB_HOST>; > > + reset-names = "xusb_host"; > > + #power-domain-cells = <0>; > > + }; > > }; > > }; > > The 'clock-names' and 'reset-names' nodes are not used/required and so I > will remove these. Please keep them and make use of the names. We used to not do this in the past, and then things became tricky to describe in the DT bindings in order to keep backwards-compatibility. Though perhaps you're not using them because they are found by index? In that case I think it might still be useful to have them for consistency. If you keep them, you might want to turn the _ into -. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 3/3] arm64: tegra210: Add XUSB powergates 2016-06-29 15:56 ` Thierry Reding @ 2016-06-29 16:07 ` Jon Hunter 2016-06-30 10:20 ` Thierry Reding 0 siblings, 1 reply; 10+ messages in thread From: Jon Hunter @ 2016-06-29 16:07 UTC (permalink / raw) To: Thierry Reding Cc: Stephen Warren, Alexandre Courbot, linux-tegra, linux-kernel On 29/06/16 16:56, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Wed, Jun 29, 2016 at 04:30:08PM +0100, Jon Hunter wrote: >> >> On 28/06/16 12:20, Jon Hunter wrote: >>> The Tegra210 XUSB subsystem has 3 power partitions which are XUSBA >>> (super-speed logic), XUSBB (USB device logic) and XUSBC (USB host >>> logic). Populate the device-tree nodes for these XUSB partitions. >>> >>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >>> --- >>> arch/arm64/boot/dts/nvidia/tegra210.dtsi | 24 ++++++++++++++++++++++++ >>> 1 file changed, 24 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi >>> index 65b829b762bb..efb0fd98b789 100644 >>> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi >>> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi >>> @@ -670,6 +670,30 @@ >>> <&tegra_car TEGRA210_CLK_MIPI_CAL>; >>> #power-domain-cells = <0>; >>> }; >>> + >>> + pd_xusbss: xusba { >>> + clocks = <&tegra_car TEGRA210_CLK_XUSB_SS>; >>> + clock-names = "xusb_ss"; >>> + resets = <&tegra_car TEGRA210_CLK_XUSB_SS>; >>> + reset-names = "xusb_ss"; >>> + #power-domain-cells = <0>; >>> + }; >>> + >>> + pd_xusbdev: xusbb { >>> + clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>; >>> + clock-names = "xusb_dev"; >>> + resets = <&tegra_car 95>; >>> + reset-names = "xusb_dev"; >>> + #power-domain-cells = <0>; >>> + }; >>> + >>> + pd_xusbhost: xusbc { >>> + clocks = <&tegra_car TEGRA210_CLK_XUSB_HOST>; >>> + clock-names = "xusb_host"; >>> + resets = <&tegra_car TEGRA210_CLK_XUSB_HOST>; >>> + reset-names = "xusb_host"; >>> + #power-domain-cells = <0>; >>> + }; >>> }; >>> }; >> >> The 'clock-names' and 'reset-names' nodes are not used/required and so I >> will remove these. > > Please keep them and make use of the names. We used to not do this in > the past, and then things became tricky to describe in the DT bindings > in order to keep backwards-compatibility. Unfortunately, in order to use them, we would need to keep a list of all the clock and reset names in the PMC driver which would be huge. > Though perhaps you're not using them because they are found by index? In > that case I think it might still be useful to have them for consistency. Right, we just iterate over the number of the clocks and resets found when we initialise the powergate. > If you keep them, you might want to turn the _ into -. I can keep them, however, in other patches I have sent out, for example the SOR powergate (part of the DPAUX series) and Audio powergate, they do not have them. So I thought I would remove them here to be consistent. However, we could add them for these other powergates as well. Cheers Jon -- nvpublic ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 3/3] arm64: tegra210: Add XUSB powergates 2016-06-29 16:07 ` Jon Hunter @ 2016-06-30 10:20 ` Thierry Reding 0 siblings, 0 replies; 10+ messages in thread From: Thierry Reding @ 2016-06-30 10:20 UTC (permalink / raw) To: Jon Hunter; +Cc: Stephen Warren, Alexandre Courbot, linux-tegra, linux-kernel [-- Attachment #1: Type: text/plain, Size: 3262 bytes --] On Wed, Jun 29, 2016 at 05:07:15PM +0100, Jon Hunter wrote: > > On 29/06/16 16:56, Thierry Reding wrote: > > * PGP Signed by an unknown key > > > > On Wed, Jun 29, 2016 at 04:30:08PM +0100, Jon Hunter wrote: > >> > >> On 28/06/16 12:20, Jon Hunter wrote: > >>> The Tegra210 XUSB subsystem has 3 power partitions which are XUSBA > >>> (super-speed logic), XUSBB (USB device logic) and XUSBC (USB host > >>> logic). Populate the device-tree nodes for these XUSB partitions. > >>> > >>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > >>> --- > >>> arch/arm64/boot/dts/nvidia/tegra210.dtsi | 24 ++++++++++++++++++++++++ > >>> 1 file changed, 24 insertions(+) > >>> > >>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi > >>> index 65b829b762bb..efb0fd98b789 100644 > >>> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi > >>> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi > >>> @@ -670,6 +670,30 @@ > >>> <&tegra_car TEGRA210_CLK_MIPI_CAL>; > >>> #power-domain-cells = <0>; > >>> }; > >>> + > >>> + pd_xusbss: xusba { > >>> + clocks = <&tegra_car TEGRA210_CLK_XUSB_SS>; > >>> + clock-names = "xusb_ss"; > >>> + resets = <&tegra_car TEGRA210_CLK_XUSB_SS>; > >>> + reset-names = "xusb_ss"; > >>> + #power-domain-cells = <0>; > >>> + }; > >>> + > >>> + pd_xusbdev: xusbb { > >>> + clocks = <&tegra_car TEGRA210_CLK_XUSB_DEV>; > >>> + clock-names = "xusb_dev"; > >>> + resets = <&tegra_car 95>; > >>> + reset-names = "xusb_dev"; > >>> + #power-domain-cells = <0>; > >>> + }; > >>> + > >>> + pd_xusbhost: xusbc { > >>> + clocks = <&tegra_car TEGRA210_CLK_XUSB_HOST>; > >>> + clock-names = "xusb_host"; > >>> + resets = <&tegra_car TEGRA210_CLK_XUSB_HOST>; > >>> + reset-names = "xusb_host"; > >>> + #power-domain-cells = <0>; > >>> + }; > >>> }; > >>> }; > >> > >> The 'clock-names' and 'reset-names' nodes are not used/required and so I > >> will remove these. > > > > Please keep them and make use of the names. We used to not do this in > > the past, and then things became tricky to describe in the DT bindings > > in order to keep backwards-compatibility. > > Unfortunately, in order to use them, we would need to keep a list of all > the clock and reset names in the PMC driver which would be huge. That's a Linux driver implementation detail, so it shouldn't influence the binding. > > Though perhaps you're not using them because they are found by index? In > > that case I think it might still be useful to have them for consistency. > > Right, we just iterate over the number of the clocks and resets found > when we initialise the powergate. Again, that's a driver implementation detail. > > If you keep them, you might want to turn the _ into -. > > I can keep them, however, in other patches I have sent out, for example > the SOR powergate (part of the DPAUX series) and Audio powergate, they > do not have them. So I thought I would remove them here to be > consistent. However, we could add them for these other powergates as well. Yes, I'd prefer them to be listed in all nodes for documentation, if for nothing else. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-06-30 10:21 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-28 11:20 [RFC PATCH 0/3] soc/tegra: Turn on XUSB partitions Jon Hunter 2016-06-28 11:20 ` [RFC PATCH 1/3] soc/tegra: pmc: Initialise power partitions early Jon Hunter 2016-06-30 10:17 ` Thierry Reding 2016-06-30 10:20 ` Jon Hunter 2016-06-28 11:20 ` [RFC PATCH 2/3] soc/tegra: pmc: Enable XUSB partitions on boot Jon Hunter 2016-06-28 11:20 ` [RFC PATCH 3/3] arm64: tegra210: Add XUSB powergates Jon Hunter 2016-06-29 15:30 ` Jon Hunter 2016-06-29 15:56 ` Thierry Reding 2016-06-29 16:07 ` Jon Hunter 2016-06-30 10:20 ` Thierry Reding
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).