[V4,1/2] pinctrl: tegra: Add DT binding for io pads control

NVIDIA Tegra124 and later SoCs support the multi-voltage level and
low power state of some of its IO pads. The IO pads can work in
the voltage of the 1.8V and 3.3V of IO voltage from IO power rail
sources. When IO interfaces are not used then IO pads can be
configure in low power state to reduce the power consumption from
that IO pads.

On Tegra124, the voltage level of IO power rail source is auto
detected by hardware(SoC) and hence it is only require to configure
in low power mode if IO pads are not used.

On T210 onwards, the auto-detection of voltage level from IO power
rail is removed from SoC and hence SW need to configure the PMC
register explicitly to set proper voltage in IO pads based on
IO rail power source voltage.

Add DT binding document for detailing the DT properties for
configuring IO pads voltage levels and its power state.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

---
Changes from V1:
 New in series based on pinctrl driver requirement.

Changes from V2:
 Updated the statement to say 1.8V and 3.3V as nominal voltage.
 Corrected DT example by adding -supply and taken care of V1 review
 from Rob.

 .../bindings/pinctrl/nvidia,tegra-io-pad.txt       | 126 +++++++++++++++++++++
 1 file changed, 126 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt

On Thu, Nov 24, 2016 at 02:08:53PM +0530, Laxman Dewangan wrote:
> NVIDIA Tegra124 and later SoCs support the multi-voltage level and
> low power state of some of its IO pads. The IO pads can work in
> the voltage of the 1.8V and 3.3V of IO voltage from IO power rail
> sources. When IO interfaces are not used then IO pads can be
> configure in low power state to reduce the power consumption from
> that IO pads.
> 
> On Tegra124, the voltage level of IO power rail source is auto
> detected by hardware(SoC) and hence it is only require to configure
> in low power mode if IO pads are not used.
> 
> On T210 onwards, the auto-detection of voltage level from IO power

You say Tegra124 above but T210 here. Please use consistent spelling to
make it more obvious that we're talking about chip generations here.
Tegra<gen> is preferred over T<gen>.

> rail is removed from SoC and hence SW need to configure the PMC
> register explicitly to set proper voltage in IO pads based on
> IO rail power source voltage.
> 
> Add DT binding document for detailing the DT properties for
> configuring IO pads voltage levels and its power state.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> 
> ---
> Changes from V1:
>  New in series based on pinctrl driver requirement.
> 
> Changes from V2:
>  Updated the statement to say 1.8V and 3.3V as nominal voltage.
>  Corrected DT example by adding -supply and taken care of V1 review
>  from Rob.
> 
>  .../bindings/pinctrl/nvidia,tegra-io-pad.txt       | 126 +++++++++++++++++++++
>  1 file changed, 126 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
> new file mode 100644
> index 0000000..a88c484
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
> @@ -0,0 +1,126 @@
> +NVIDIA Tegra PMC IO pad controller
> +
> +NVIDIA Tegra124 and later SoCs support the multi-voltage level and low power
> +state of some of its IO pads. When IO interface are not used then IO pads can
> +be configure in low power state to reduce the power from that IO pads. The IO
> +pads can work in the nominal IO voltage of 1.8V and 3.3V from power rail
> +sources.
> +
> +On Tegra124, the voltage of IO power rail source is auto detected by SoC and
> +hence it is only require to configure in low power mode if IO pads are not
> +used.
> +
> +On T210 onwards, the HW based auto-detection for IO voltage is removed and
> +hence SW need to configure the PMC register explicitly, to set proper voltage
> +in IO pads, based on IO rail power source voltage.
> +
> +The voltage configurations and low power state of IO pads should be done in
> +boot if it is not going to change otherwise dynamically based on IO rail
> +voltage on that IO pads and usage of IO pads

Missing fullstop at the end.

> +
> +The DT property of the IO pads must be under the node of pmc i.e.
> +pmc@7000e400 for Tegra124 onwards.

The PMC is at a different address on Tegra186, so I think we should just
drop this to avoid having to update it whenever a new chip relocates it
to a different address.

Come to think of it, if these I/O pads are represented as subnodes in
the PMC device tree node, perhaps this should be merged into the PMC
documentation?

> +Please refer to <pinctrl-bindings.txt> in this directory for details of the
> +common pinctrl bindings used by client devices, including the meaning of the
> +phrase "pin configuration node".
> +
> +Tegra's pin configuration nodes act as a container for an arbitrary number of
> +subnodes. Each of these subnodes represents some desired configuration for an
> +IO pads, or a list of IO pads. This configuration can include the voltage and
> +power enable/disable control

Missing fullstop.

> +
> +The name of each subnode is not important; all subnodes should be enumerated
> +and processed purely based on their content. Each subnode only affects those
> +parameters that are explicitly listed. Unspecified is represented as an absent
> +property,

Should be fullstop instead of comma at the end of the sentence. Also I'm
not sure the last sentence is even necessary. The previous one already
says that only explicitly listed parameters take effect.

> +
> +See the TRM to determine which properties and values apply to each IO pads.

Perhaps give a reference to where in the TRM this can be found?

> +
> +Required subnode-properties:
> +==========================
> +- pins : An array of strings. Each string contains the name of an IO pads. Valid
> +	 values for these names are listed below.
> +
> +Optional subnode-properties:
> +==========================
> +Following properties are supported from generic pin configuration explained
> +in <dt-bindings/pinctrl/pinctrl-binding.txt>.

I don't think such a directory exists. Maybe make these relative
references, though that might become slightly more difficult if this
whole document is merged with the PMC documentation.

> +low-power-enable:		enable low power mode.
> +low-power-disable:		disable low power mode.

Why the extra padding with tabs? I find that difficult to read. Also, no
need for a fullstop since it's not a proper sentence.

> +Valid values for pin for T124 are:

Tegra124

> +	audio, bb, cam, comp, csia, csib, csie, dsi, dsib, dsic, dsid, hdmi,
> +	hsic, hv, lvds, mipi-bias, nand, pex-bias, pex-clk1, pex-clk2,
> +	pex-ctrl, sdmmc1, sdmmc3, sdmmc4, sys-ddc, uart, usb0, usb1, usb2,
> +	usb-bias
> +
> +Valid values for pin for T210 are:

Tegra210

> +	audio, audio-hv, cam, csia, csib, csic, csid, csie, csif,
> +	dbg, debug-nonao, dmic, dp, dsi, dsib, dsic, dsid, emmc, emmc2,
> +	gpio, hdmi, hsic, lvds, mipi-bias, pex-bias, pex-clk1, pex-clk2,
> +	pex-ctrl, sdmmc1, sdmmc3, spi, spi-hv, uart, usb-bias, usb0,
> +	usb1, usb2, usb3.
> +
> +To find out the IO rail voltage for setting the voltage of IO pad by SW,
> +the regulator supply handle must provided from the DT and it is explained
> +in the regulator DT binding document
> +	<devicetree/bindings/regulator/regulator.txt>.

Again, maybe make this relative because as it is it isn't obvious as to
what the base of the above path is.

> +For example, for GPIO rail the supply name is vddio-gpio and regulator
> +handle is supplied from DT as
> +	vddio-gpio-supply = <&regulator_xyz>;
> +
> +For T210, following IO pads support the 1.8V/3.3V and the corresponding

Tegra210

> +IO voltage pin names are as follows:
> +	audio -> vddio-audio
> +	audio-hv -> vddio-audio-hv
> +	cam ->vddio-cam
> +	dbg -> vddio-dbg
> +	dmic -> vddio-dmic
> +	gpio -> vddio-gpio
> +	pex-ctrl -> vddio-pex-ctrl
> +	sdmmc1 -> vddio-sdmmc1
> +	sdmmc3 -> vddio-sdmmc3
> +	spi -> vddio-spi
> +	spi-hv -> vddio-spi-hv
> +	uart -> vddio-uart

It's slightly confusing to only have this list for Tegra210. I assume
that is because on Tegra124 there is no way to control the voltage of
the pins, but I think that could be made clearer. Also, it might be
worth explicitly mentioning that this is a subset of the list of pins
given above and that the other pins (those not in this list) don't
support 1.8/3.3 V control, but only the low power state.

> +
> +Example:
> +	i2c@7000d000 {
> +		pmic@3c {
> +			regulators {
> +				vddio_sdmmc1: ldo2 {
> +					/* Regulator entries for LDO2 */
> +				};
> +
> +				vdd_cam: ldo3 {
> +					/* Regulator entries for LDO3 */

These comments are somewhat stating the obvious. Perhaps simply use a
... as a placeholder?

> +				};
> +			};
> +		};
> +	};
> +
> +	pmc@7000e400 {
> +		vddio-cam-supply = <&vdd_cam>;
> +		vddio-sdmmc1-supply = <&vddio_sdmmc1>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&tegra_io_pad_volt_default>;
> +		tegra_io_pad_volt_default: common {
> +			audio-hv {
> +				pins = "audio-hv";
> +				low-power-disable;
> +			};

I wonder if this is at all useful. Shouldn't we rather put all pads into
a low-power state by default and only take them out of the low-power
state when the driver decides to do so?

Thierry

On Friday 25 November 2016 02:43 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Thu, Nov 24, 2016 at 02:08:53PM +0530, Laxman Dewangan wrote:
>> +
>> +The DT property of the IO pads must be under the node of pmc i.e.
>> +pmc@7000e400 for Tegra124 onwards.
> The PMC is at a different address on Tegra186, so I think we should just
> drop this to avoid having to update it whenever a new chip relocates it
> to a different address.

I wanted to provide the example. So let me say that "i.e. pmc@7000e400 
for Tegra124". In this way we will not need to update for every chips 
and also give idea bout the node.

>
> Come to think of it, if these I/O pads are represented as subnodes in
> the PMC device tree node, perhaps this should be merged into the PMC
> documentation?

Based on my MFD and sub devices experience, maintainers prefer to have  
different DT binding document for subsystem child devices.
So if we say the io-pad is sub device of pmc then it will be in 
respective subsystem dt binding.
>
>> +
>> +See the TRM to determine which properties and values apply to each IO pads.
> Perhaps give a reference to where in the TRM this can be found?
Do we have fixed link for the TRM? and do we really need to provide the 
link here? If link get changed then we will need to change all places.


>
>> +low-power-enable:		enable low power mode.
>> +low-power-disable:		disable low power mode.
> Why the extra padding with tabs? I find that difficult to read. Also, no
> need for a fullstop since it's not a proper sentence.

Just to make proper alignment. Regular typing is not preferred in general.

>> +IO voltage pin names are as follows:
>> +	audio -> vddio-audio
>> +	audio-hv -> vddio-audio-hv
>> +	cam ->vddio-cam
>> +	dbg -> vddio-dbg
>> +	dmic -> vddio-dmic
>> +	gpio -> vddio-gpio
>> +	pex-ctrl -> vddio-pex-ctrl
>> +	sdmmc1 -> vddio-sdmmc1
>> +	sdmmc3 -> vddio-sdmmc3
>> +	spi -> vddio-spi
>> +	spi-hv -> vddio-spi-hv
>> +	uart -> vddio-uart
> It's slightly confusing to only have this list for Tegra210. I assume
> that is because on Tegra124 there is no way to control the voltage of
> the pins, but I think that could be made clearer.

I think I made it in first few paragraph of this document but will add 
here also.

>   Also, it might be
> worth explicitly mentioning that this is a subset of the list of pins
> given above and that the other pins (those not in this list) don't
> support 1.8/3.3 V control, but only the low power state.
ok.

>
> +			audio-hv {
> +				pins = "audio-hv";
> +				low-power-disable;
> +			};
> I wonder if this is at all useful. Shouldn't we rather put all pads into
> a low-power state by default and only take them out of the low-power
> state when the driver decides to do so?
>

We can not do this all disable by default and enable by driver. it may 
be possible that some of io-pads need to be enable during boot.

We need to only depends on platform specific data provided from DT here 
for configurations.

However, for dynamic control, driver can use pinctrl framework for 
optimizing the power.

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 24/11/16 08:38, Laxman Dewangan wrote:
> NVIDIA Tegra124 and later SoCs support the multi-voltage level and
> low power state of some of its IO pads. The IO pads can work in
> the voltage of the 1.8V and 3.3V of IO voltage from IO power rail
> sources.

Can you fix the above sentence?

> When IO interfaces are not used then IO pads can be
> configure in low power state to reduce the power consumption from
> that IO pads.
> 
> On Tegra124, the voltage level of IO power rail source is auto
> detected by hardware(SoC) and hence it is only require to configure
> in low power mode if IO pads are not used.
> 
> On T210 onwards, the auto-detection of voltage level from IO power
> rail is removed from SoC and hence SW need to configure the PMC
> register explicitly to set proper voltage in IO pads based on
> IO rail power source voltage.
> 
> Add DT binding document for detailing the DT properties for
> configuring IO pads voltage levels and its power state.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> 
> ---
> Changes from V1:
>  New in series based on pinctrl driver requirement.
> 
> Changes from V2:
>  Updated the statement to say 1.8V and 3.3V as nominal voltage.
>  Corrected DT example by adding -supply and taken care of V1 review
>  from Rob.
> 
>  .../bindings/pinctrl/nvidia,tegra-io-pad.txt       | 126 +++++++++++++++++++++
>  1 file changed, 126 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
> new file mode 100644
> index 0000000..a88c484
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
> @@ -0,0 +1,126 @@
> +NVIDIA Tegra PMC IO pad controller
> +
> +NVIDIA Tegra124 and later SoCs support the multi-voltage level and low power
> +state of some of its IO pads. When IO interface are not used then IO pads can
> +be configure in low power state to reduce the power from that IO pads. The IO
> +pads can work in the nominal IO voltage of 1.8V and 3.3V from power rail
> +sources.

I would replace the above sentence with ...

"The IO pads can operate at the nominal IO voltage of either 1.8V or 3.3V".

Cheers
Jon