Re: [RFC PATCH 3/3] arm64: tegra210: Add XUSB powergates

[Thierry Reding <thierry.reding@gmail.com>]

[-- 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 --]