diff mbox

[6/7] pinctrl: tegra: Add DT binding for io pads control

Message ID 1460473007-11535-7-git-send-email-ldewangan@nvidia.com
State New
Headers show

Commit Message

Laxman Dewangan April 12, 2016, 2:56 p.m. UTC
NVIDIA Tegra210 supports the IO pads which can operate at 1.8V
or 3.3V I/O voltage levels. Also IO pads can be configured for
power down state if it is not in used. SW needs to configure the
voltage level of IO pads based on IO rail voltage and its power
state based on platform usage.

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>
---
 .../bindings/pinctrl/nvidia,tegra210-io-pad.txt    | 102 +++++++++++++++++++++
 .../dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h  |  24 +++++
 2 files changed, 126 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-io-pad.txt
 create mode 100644 include/dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h

Comments

Jon Hunter April 13, 2016, 9:04 a.m. UTC | #1
On 12/04/16 15:56, Laxman Dewangan wrote:
> NVIDIA Tegra210 supports the IO pads which can operate at 1.8V
> or 3.3V I/O voltage levels. Also IO pads can be configured for
> power down state if it is not in used. SW needs to configure the
> voltage level of IO pads based on IO rail voltage and its power
> state based on platform usage.
> 
> 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>
> ---
>  .../bindings/pinctrl/nvidia,tegra210-io-pad.txt    | 102 +++++++++++++++++++++
>  .../dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h  |  24 +++++
>  2 files changed, 126 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-io-pad.txt
>  create mode 100644 include/dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-io-pad.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-io-pad.txt
> new file mode 100644
> index 0000000..97cdd4f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-io-pad.txt
> @@ -0,0 +1,102 @@
> +NVIDIA Tegra210 PMC IO pad controller
> +
> +NVIDIA Tegra210 supports IO pads which can operate at 1.8V or 3.3V I/O
> +power rail voltages. SW needs to configure the voltage level of IO pads
> +based on platform specific power tree.
> +
> +The voltage configurations of IO pads should be done in boot if it is not
> +going to change other wise dynamically based on IO rail voltage on that
> +IO pads.
> +
> +The node for the Tegra210 io-pad driver must be sub node of pmc@0,7000e400.

This should be 'pmc@7000e400'. We were incorrectly adding the '0,'
previously.

> +
> +Required properties:
> +- compatible: "nvidia,tegra210-io-pad"

I think you have have "Must be ..." here. I am also wondering if the
pinctrl device should be registered by the pmc driver and so not a
separate driver to the PMC driver. In other words, the PMC driver calls
pinctrl_register() directly.

> +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
> +
> +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,
> +
> +See the TRM to determine which properties and values apply to each IO pads.
> +Macro values for property values are defined in
> +<dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
> +
> +The voltage supported on the pads are 1.8V and 3.3V. The enums are defined as:
> +	For 1.8V, use TEGRA210_IO_RAIL_1800000UV
> +	For 3.3V, use TEGRA210_IO_RAIL_3300000UV

You may consider just using integer values here like we do for regulators.

> +
> +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.

Why are they not listed here? Array of strings sounds odd. Array/list of
pin names seems more appropriate.

> +Optional subnode-properties:
> +==========================
> +-nvidia,io-rail-voltage:	Integer. The voltage level of IO pads. The
> +				valid values are 1.8V and 3.3V. Macros are
> +				defined for these voltage levels in
> +				<dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
> +					Use TEGRA210_IO_RAIL_1800000UV for 1.8V
> +					Use TEGRA210_IO_RAIL_3300000UV for 3.3V
> +
> +-nvidia,io-pad-deep-power-down: Integer, representing the deep power down state
> +				of the IO pads. If this is enable then IO pads
> +				will be in power down state and interface is not
> +				enabled for any transaction. This is power
> +				saving mode of the IO pads. The macros are
> +				defined for enable/disable in
> +				<dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
> +				  TEGRA210_IO_PAD_DEEP_POWER_DOWN_DISABLE for
> +					disable.
> +				  TEGRA210_IO_PAD_DEEP_POWER_DOWN_ENABLE for
> +					enable.

Sounds like a boolean parameter. So may consider that if the property
'nvidia,io-pad-deep-power-down' is present then it means enable
deep-power-down and if not present then don't. Then you do not need to
assign a value to it.

> +Valid values for pin are:
> +	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.
> +
> +All IO pads do not support the 1.8V/3.3V configurations. Valid values for
> +nvidia,io-rail-voltage are:
> +	audio-hv, dmic, gpio, sdmmc1, sdmmc3, spi-hv.

May be this should be moved under the nvidia,io-rail-voltage description?

> +All above IO pads supports the deep power down state.

May be this should be moved under the nvidia,io-pad-deep-power-down
description?

> +Example:
> +	#include <dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
> +	pmc@0,7000e400 {
> +		pmc-pad-control {
> +			compatible = "nvidia,tegra210-io-pad";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&tegra_io_pad_volt_default>;
> +			tegra_io_pad_volt_default: common {
> +				audio {
> +					pins = "audio";
> +					nvidia,io-pad-deep-power-down = <TEGRA210_IO_PAD_DEEP_POWER_DOWN_DISABLE>;
> +				};
> +				audio-hv {
> +					pins = "audio-hv";
> +					nvidia,io-rail-voltage = <TEGRA210_IO_RAIL_1800000UV>;
> +				};
> +				gpio {
> +					pins = "gpio";
> +					nvidia,io-rail-voltage = <TEGRA210_IO_RAIL_1800000UV>;
> +				};
> +				rest {
> +					pins = "dmic", "sdmmc1", "sdmmc3";
> +					nvidia,io-rail-voltage = <TEGRA210_IO_RAIL_1800000UV>;
> +				};

I know this is an example, but it does not make sense to me why audio-hv
and gpio and separated from 'rest' when they have the same configuration.

Cheers
Jon
--
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
Laxman Dewangan April 13, 2016, 9:08 a.m. UTC | #2
On Wednesday 13 April 2016 02:34 PM, Jon Hunter wrote:
> On 12/04/16 15:56, Laxman Dewangan wrote:
>> NVIDIA Tegra210 supports the IO pads which can operate at 1.8V
>> or 3.3V I/O voltage levels. Also IO pads can be configured for
>> power down state if it is not in used. SW needs to configure the
>> voltage level of IO pads based on IO rail voltage and its power
>> state based on platform usage.
>>
>> 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>
>> ---
>>   .../bindings/pinctrl/nvidia,tegra210-io-pad.txt    | 102 +++++++++++++++++++++
>>   .../dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h  |  24 +++++
>>   2 files changed, 126 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-io-pad.txt
>>   create mode 100644 include/dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-io-pad.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-io-pad.txt
>> new file mode 100644
>> index 0000000..97cdd4f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-io-pad.txt
>> @@ -0,0 +1,102 @@
>> +NVIDIA Tegra210 PMC IO pad controller
>> +
>> +NVIDIA Tegra210 supports IO pads which can operate at 1.8V or 3.3V I/O
>> +power rail voltages. SW needs to configure the voltage level of IO pads
>> +based on platform specific power tree.
>> +
>> +The voltage configurations of IO pads should be done in boot if it is not
>> +going to change other wise dynamically based on IO rail voltage on that
>> +IO pads.
>> +
>> +The node for the Tegra210 io-pad driver must be sub node of pmc@0,7000e400.
> This should be 'pmc@7000e400'. We were incorrectly adding the '0,'
> previously.

For T210, 64 bit, it is
  tegra210.dtsi:    pmc: pmc@0,7000e400 {

For T124, it is

pmc@7000e400



>
>> +
>> +Required properties:
>> +- compatible: "nvidia,tegra210-io-pad"
> I think you have have "Must be ..." here. I am also wondering if the
> pinctrl device should be registered by the pmc driver and so not a
> separate driver to the PMC driver. In other words, the PMC driver calls
> pinctrl_register() directly.

I like to keep the pmc driver as main interface and other sub 
functionalities like pad control for voltage and power states, no 
iopower control etc as sub drivers. This will help in modular approach 
of the driver.


>
>> +The voltage supported on the pads are 1.8V and 3.3V. The enums are defined as:
>> +	For 1.8V, use TEGRA210_IO_RAIL_1800000UV
>> +	For 3.3V, use TEGRA210_IO_RAIL_3300000UV
> You may consider just using integer values here like we do for regulators.

We just support two values 1.8V and 3.3V only. I am fine with either way 
also.

>
>> +				  TEGRA210_IO_PAD_DEEP_POWER_DOWN_DISABLE for
>> +					disable.
>> +				  TEGRA210_IO_PAD_DEEP_POWER_DOWN_ENABLE for
>> +					enable.
> Sounds like a boolean parameter. So may consider that if the property
> 'nvidia,io-pad-deep-power-down' is present then it means enable
> deep-power-down and if not present then don't. Then you do not need to
> assign a value to it.

Three states, enable, disable and left default. So absent will be left 
default.

--
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
Jon Hunter April 13, 2016, 9:31 a.m. UTC | #3
On 13/04/16 10:08, Laxman Dewangan wrote:

[snip]

> For T210, 64 bit, it is
>  tegra210.dtsi:    pmc: pmc@0,7000e400 {

Yes and this is wrong [0].

>>> +Required properties:
>>> +- compatible: "nvidia,tegra210-io-pad"
>> I think you have have "Must be ..." here. I am also wondering if the
>> pinctrl device should be registered by the pmc driver and so not a
>> separate driver to the PMC driver. In other words, the PMC driver calls
>> pinctrl_register() directly.
> 
> I like to keep the pmc driver as main interface and other sub
> functionalities like pad control for voltage and power states, no
> iopower control etc as sub drivers. This will help in modular approach
> of the driver.

OK, let's see what Thierry thinks about this.

>>> +The voltage supported on the pads are 1.8V and 3.3V. The enums are
>>> defined as:
>>> +    For 1.8V, use TEGRA210_IO_RAIL_1800000UV
>>> +    For 3.3V, use TEGRA210_IO_RAIL_3300000UV
>> You may consider just using integer values here like we do for
>> regulators.
> 
> We just support two values 1.8V and 3.3V only. I am fine with either way
> also.
> 
>>
>>> +                  TEGRA210_IO_PAD_DEEP_POWER_DOWN_DISABLE for
>>> +                    disable.
>>> +                  TEGRA210_IO_PAD_DEEP_POWER_DOWN_ENABLE for
>>> +                    enable.
>> Sounds like a boolean parameter. So may consider that if the property
>> 'nvidia,io-pad-deep-power-down' is present then it means enable
>> deep-power-down and if not present then don't. Then you do not need to
>> assign a value to it.
> 
> Three states, enable, disable and left default. So absent will be left
> default.

Fine with me.

Cheers
Jon

[0] http://marc.info/?l=linux-tegra&m=146038329109871&w=2
--
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
Laxman Dewangan April 15, 2016, 2:12 p.m. UTC | #4
On Friday 15 April 2016 07:46 PM, Jon Hunter wrote:
> On 12/04/16 15:56, Laxman Dewangan wrote:
>> NVIDIA Tegra210 supports the IO pads which can operate at 1.8V
>> or 3.3V I/O voltage levels. Also IO pads can be configured for
>> power down state if it is not in used. SW needs to configure the
>> voltage level of IO pads based on IO rail voltage and its power
>> state based on platform usage.
>>
>> 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>
> [snip]
>
>> +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:
>> +==========================
>> +-nvidia,io-rail-voltage:	Integer. The voltage level of IO pads. The
>> +				valid values are 1.8V and 3.3V. Macros are
>> +				defined for these voltage levels in
>> +				<dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
>> +					Use TEGRA210_IO_RAIL_1800000UV for 1.8V
>> +					Use TEGRA210_IO_RAIL_3300000UV for 3.3V
>> +
>> +-nvidia,io-pad-deep-power-down: Integer, representing the deep power down state
>> +				of the IO pads. If this is enable then IO pads
>> +				will be in power down state and interface is not
>> +				enabled for any transaction. This is power
>> +				saving mode of the IO pads. The macros are
>> +				defined for enable/disable in
>> +				<dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
>> +				  TEGRA210_IO_PAD_DEEP_POWER_DOWN_DISABLE for
>> +					disable.
>> +				  TEGRA210_IO_PAD_DEEP_POWER_DOWN_ENABLE for
>> +					enable.
>> +Valid values for pin are:
>> +	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.
> Thinking about this some more, the above are not IO pads but supply
> pads, AFAICT. And these supply pads, are supplying the voltage to
> various IO pads. I am not sure if these should be named vddio_xxx. The
> 'pins' properties says these are IO pads, but this does not seem correct.

These are IO pads. One IO rail  have multiple sub pads to power down 
some of interface when not used. Like if CSIA is active, we can power 
down CSIB, CSIC etc.
All CSI pads are lined to single IO rail.

--
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
Jon Hunter April 15, 2016, 2:16 p.m. UTC | #5
On 12/04/16 15:56, Laxman Dewangan wrote:
> NVIDIA Tegra210 supports the IO pads which can operate at 1.8V
> or 3.3V I/O voltage levels. Also IO pads can be configured for
> power down state if it is not in used. SW needs to configure the
> voltage level of IO pads based on IO rail voltage and its power
> state based on platform usage.
> 
> 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>

[snip]

> +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:
> +==========================
> +-nvidia,io-rail-voltage:	Integer. The voltage level of IO pads. The
> +				valid values are 1.8V and 3.3V. Macros are
> +				defined for these voltage levels in
> +				<dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
> +					Use TEGRA210_IO_RAIL_1800000UV for 1.8V
> +					Use TEGRA210_IO_RAIL_3300000UV for 3.3V
> +
> +-nvidia,io-pad-deep-power-down: Integer, representing the deep power down state
> +				of the IO pads. If this is enable then IO pads
> +				will be in power down state and interface is not
> +				enabled for any transaction. This is power
> +				saving mode of the IO pads. The macros are
> +				defined for enable/disable in
> +				<dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
> +				  TEGRA210_IO_PAD_DEEP_POWER_DOWN_DISABLE for
> +					disable.
> +				  TEGRA210_IO_PAD_DEEP_POWER_DOWN_ENABLE for
> +					enable.
> +Valid values for pin are:
> +	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.

Thinking about this some more, the above are not IO pads but supply
pads, AFAICT. And these supply pads, are supplying the voltage to
various IO pads. I am not sure if these should be named vddio_xxx. The
'pins' properties says these are IO pads, but this does not seem correct.

We also need to think about how these supply pads are linked to the
actual IO pads. Or at least it seems they should be some how ...

Cheers
Jon
--
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
Laxman Dewangan April 15, 2016, 3:14 p.m. UTC | #6
On Friday 15 April 2016 08:44 PM, Jon Hunter wrote:
> On 15/04/16 15:12, Laxman Dewangan wrote:
>>
>>
>> All CSI pads are lined to single IO rail.
> I agree with this and from the data-sheet I see the rail that powers the
> CSI (and DSI) interfaces is called AVDD_DSI_CSI. But again, in the DT
> document you are referring to csia, csib, csic, csid, csie, csif as
> pins, but these don't appear to be physical pins, and this appears to be
> more of a software means to control power to the various csi_x pins.
>
> It seems to me that each of the existing CSI_A_xxx pins/pads should be
> mapped to or register with the appropriate power-down control and when
> all pads are set to inactive this then triggers the power-down of all
> the CSI_A_xxx pads.

I used pins as this is the property from pincon generic so that I can 
use the generic implementation.

Here, I will not go to the pin level control as HW does not support pin 
level control.

I will say the unit should be interface level. Should we say 
IO_GROUP_CSIA, IO_GROUP_CSIB etc?


>

--
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
Jon Hunter April 15, 2016, 3:14 p.m. UTC | #7
On 15/04/16 15:12, Laxman Dewangan wrote:
> 
> On Friday 15 April 2016 07:46 PM, Jon Hunter wrote:
>> On 12/04/16 15:56, Laxman Dewangan wrote:
>>> NVIDIA Tegra210 supports the IO pads which can operate at 1.8V
>>> or 3.3V I/O voltage levels. Also IO pads can be configured for
>>> power down state if it is not in used. SW needs to configure the
>>> voltage level of IO pads based on IO rail voltage and its power
>>> state based on platform usage.
>>>
>>> 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>
>> [snip]
>>
>>> +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:
>>> +==========================
>>> +-nvidia,io-rail-voltage:    Integer. The voltage level of IO pads. The
>>> +                valid values are 1.8V and 3.3V. Macros are
>>> +                defined for these voltage levels in
>>> +                <dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
>>> +                    Use TEGRA210_IO_RAIL_1800000UV for 1.8V
>>> +                    Use TEGRA210_IO_RAIL_3300000UV for 3.3V
>>> +
>>> +-nvidia,io-pad-deep-power-down: Integer, representing the deep power
>>> down state
>>> +                of the IO pads. If this is enable then IO pads
>>> +                will be in power down state and interface is not
>>> +                enabled for any transaction. This is power
>>> +                saving mode of the IO pads. The macros are
>>> +                defined for enable/disable in
>>> +                <dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
>>> +                  TEGRA210_IO_PAD_DEEP_POWER_DOWN_DISABLE for
>>> +                    disable.
>>> +                  TEGRA210_IO_PAD_DEEP_POWER_DOWN_ENABLE for
>>> +                    enable.
>>> +Valid values for pin are:
>>> +    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.
>> Thinking about this some more, the above are not IO pads but supply
>> pads, AFAICT. And these supply pads, are supplying the voltage to
>> various IO pads. I am not sure if these should be named vddio_xxx. The
>> 'pins' properties says these are IO pads, but this does not seem correct.
> 
> These are IO pads. One IO rail  have multiple sub pads to power down
> some of interface when not used. Like if CSIA is active, we can power
> down CSIB, CSIC etc.

To me, 'IO rail' implies a supply rail, but this is not the same as an
IO pad (or pin/ball). And hence, I think the terminology here is confusing.

For example, audio_hv powers the following IO pads ...

DAP1_DIN
DAP1_DOUT
DAP1_FS
DAP1_SCLK
SPI2_MOSI
SPI2_MISO
SPI2_SCK
SPI2_CS0
SPI2_CS1

And sdmmc1 powers the following IO pads ...

SDMMC1_CLK
SDMMC1_CMD
SDMMC1_DAT0
SDMMC1_DAT1
SDMMC1_DAT2
SDMMC1_DAT3
SDMMC1_COMP

As for CSIA, I don't think this is a pin/pad at all, but a software
means to control the power down for the CSI_A_xxx pads. If CSIA is an IO
pad then what is the ball number for Tegra210? In the datasheet I only
see ...

CSI_A_CLK_N Y6
CSI_A_CLK_P Y7
CSI_A_D0_N Y4
CSI_A_D0_P Y5
CSI_A_D1_N Y1
CSI_A_D1_P AA1

> All CSI pads are lined to single IO rail.

I agree with this and from the data-sheet I see the rail that powers the
CSI (and DSI) interfaces is called AVDD_DSI_CSI. But again, in the DT
document you are referring to csia, csib, csic, csid, csie, csif as
pins, but these don't appear to be physical pins, and this appears to be
more of a software means to control power to the various csi_x pins.

It seems to me that each of the existing CSI_A_xxx pins/pads should be
mapped to or register with the appropriate power-down control and when
all pads are set to inactive this then triggers the power-down of all
the CSI_A_xxx pads.

Cheers
Jon

--
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
Jon Hunter April 15, 2016, 3:45 p.m. UTC | #8
On 15/04/16 16:14, Laxman Dewangan wrote:
> On Friday 15 April 2016 08:44 PM, Jon Hunter wrote:
>> On 15/04/16 15:12, Laxman Dewangan wrote:
>>>
>>>
>>> All CSI pads are lined to single IO rail.
>> I agree with this and from the data-sheet I see the rail that powers the
>> CSI (and DSI) interfaces is called AVDD_DSI_CSI. But again, in the DT
>> document you are referring to csia, csib, csic, csid, csie, csif as
>> pins, but these don't appear to be physical pins, and this appears to be
>> more of a software means to control power to the various csi_x pins.
>>
>> It seems to me that each of the existing CSI_A_xxx pins/pads should be
>> mapped to or register with the appropriate power-down control and when
>> all pads are set to inactive this then triggers the power-down of all
>> the CSI_A_xxx pads.
>
> I used pins as this is the property from pincon generic so that I can
> use the generic implementation.
> 
> Here, I will not go to the pin level control as HW does not support pin
> level control.
> 
> I will say the unit should be interface level. Should we say
> IO_GROUP_CSIA, IO_GROUP_CSIB etc?

So we need to reflect the hardware in device-tree and although yes the
power-down for the CSI_x_xxx pads are all controlled together as a
single group, it does not feel right that we add a pseudo pin called
csix to represent these.

The CSI_x_xxx pads are already in device-tree and so why not add a
property to each of these pads which has the IO rail information for
power-down and voltage-select?

Cheers
Jon
--
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
Laxman Dewangan April 15, 2016, 4:31 p.m. UTC | #9
On Friday 15 April 2016 10:05 PM, Stephen Warren wrote:
> On 04/12/2016 08:56 AM, Laxman Dewangan wrote:
>> NVIDIA Tegra210 supports the IO pads which can operate at 1.8V
>> or 3.3V I/O voltage levels. Also IO pads can be configured for
>> power down state if it is not in used. SW needs to configure the
>> voltage level of IO pads based on IO rail voltage and its power
>> state based on platform usage.
>>
>> Add DT binding document for detailing the DT properties for
>> configuring IO pads voltage levels and its power state.
>
> I hope that we only intend to use this in the case where Linux must 
> make dynamic changes to the IO voltage (e.g. SD cards switching 
> between speeds).
>
> All static settings, and good boot defaults, should be set up by 
> system FW. Perhaps not all FW does this on Tegra210 platforms:-( I 
> hope that on future chips, the same FW that sets up the static pinmux 
> sets up the static IO voltage configuration, in exactly the same way.

Exactly yes.
--
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
Stephen Warren April 15, 2016, 4:35 p.m. UTC | #10
On 04/12/2016 08:56 AM, Laxman Dewangan wrote:
> NVIDIA Tegra210 supports the IO pads which can operate at 1.8V
> or 3.3V I/O voltage levels. Also IO pads can be configured for
> power down state if it is not in used. SW needs to configure the
> voltage level of IO pads based on IO rail voltage and its power
> state based on platform usage.
>
> Add DT binding document for detailing the DT properties for
> configuring IO pads voltage levels and its power state.

I hope that we only intend to use this in the case where Linux must make 
dynamic changes to the IO voltage (e.g. SD cards switching between speeds).

All static settings, and good boot defaults, should be set up by system 
FW. Perhaps not all FW does this on Tegra210 platforms:-( I hope that on 
future chips, the same FW that sets up the static pinmux sets up the 
static IO voltage configuration, in exactly the same way.
--
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
Laxman Dewangan April 15, 2016, 4:41 p.m. UTC | #11
On Friday 15 April 2016 09:15 PM, Jon Hunter wrote:
> On 15/04/16 16:14, Laxman Dewangan wrote:
>>
>> I used pins as this is the property from pincon generic so that I can
>> use the generic implementation.
>>
>> Here, I will not go to the pin level control as HW does not support pin
>> level control.
>>
>> I will say the unit should be interface level. Should we say
>> IO_GROUP_CSIA, IO_GROUP_CSIB etc?
> So we need to reflect the hardware in device-tree and although yes the
> power-down for the CSI_x_xxx pads are all controlled together as a
> single group, it does not feel right that we add a pseudo pin called
> csix to represent these.
>
> The CSI_x_xxx pads are already in device-tree and so why not add a
> property to each of these pads which has the IO rail information for
> power-down and voltage-select?

Which dt binding docs have these?
I looked for nvidia,tegra210-pinmux.txt and not able to find csi_xxx.

Here I dont want to refer the individual pins as control should be as group.

--
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
Jon Hunter April 15, 2016, 5:44 p.m. UTC | #12
On 15/04/16 17:41, Laxman Dewangan wrote:
> 
> On Friday 15 April 2016 09:15 PM, Jon Hunter wrote:
>> On 15/04/16 16:14, Laxman Dewangan wrote:
>>>
>>> I used pins as this is the property from pincon generic so that I can
>>> use the generic implementation.
>>>
>>> Here, I will not go to the pin level control as HW does not support pin
>>> level control.
>>>
>>> I will say the unit should be interface level. Should we say
>>> IO_GROUP_CSIA, IO_GROUP_CSIB etc?
>> So we need to reflect the hardware in device-tree and although yes the
>> power-down for the CSI_x_xxx pads are all controlled together as a
>> single group, it does not feel right that we add a pseudo pin called
>> csix to represent these.
>>
>> The CSI_x_xxx pads are already in device-tree and so why not add a
>> property to each of these pads which has the IO rail information for
>> power-down and voltage-select?
> 
> Which dt binding docs have these?
> I looked for nvidia,tegra210-pinmux.txt and not able to find csi_xxx.

For CSI you are right they are not included by the current DT binding
docs, however, the sdmmc1/3 pads are. So that makes things a bit more
messy as some are and some are not.

> Here I dont want to refer the individual pins as control should be as
> group.

I understand, however, at least for power-down control I don't see why
we cannot refer to the individual pins and once all are inactive then
the rail can be powered down.

For switching the voltage it is a bit more complex, but may be we could
still look-up the IO rail based upon the pads the device uses.

Cheers
Jon


--
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
Laxman Dewangan April 15, 2016, 5:49 p.m. UTC | #13
On Friday 15 April 2016 11:14 PM, Jon Hunter wrote:
> On 15/04/16 17:41, Laxman Dewangan wrote:
>> On Friday 15 April 2016 09:15 PM, Jon Hunter wrote:
>>> On 15/04/16 16:14, Laxman Dewangan wrote:
>>>> I used pins as this is the property from pincon generic so that I can
>>>> use the generic implementation.
>>>>
>>>> Here, I will not go to the pin level control as HW does not support pin
>>>> level control.
>>>>
>>>> I will say the unit should be interface level. Should we say
>>>> IO_GROUP_CSIA, IO_GROUP_CSIB etc?
>>> So we need to reflect the hardware in device-tree and although yes the
>>> power-down for the CSI_x_xxx pads are all controlled together as a
>>> single group, it does not feel right that we add a pseudo pin called
>>> csix to represent these.
>>>
>>> The CSI_x_xxx pads are already in device-tree and so why not add a
>>> property to each of these pads which has the IO rail information for
>>> power-down and voltage-select?
>> Which dt binding docs have these?
>> I looked for nvidia,tegra210-pinmux.txt and not able to find csi_xxx.
> For CSI you are right they are not included by the current DT binding
> docs, however, the sdmmc1/3 pads are. So that makes things a bit more
> messy as some are and some are not.

Yaah and so lets have the names in new dt files. Names may be same but 
define all possible names f groups in dt binding and need not to refer 
from other file which does not have all.

>> Here I dont want to refer the individual pins as control should be as
>> group.
> I understand, however, at least for power-down control I don't see why
> we cannot refer to the individual pins and once all are inactive then
> the rail can be powered down.
>
> For switching the voltage it is a bit more complex, but may be we could
> still look-up the IO rail based upon the pads the device uses.
>

Yes, it can be done with ref count also for power down.
But interfaces are complex. As a client, it is easy to say power down 
SDMMC1 IO interface rather than saying power down 10 pins (names) of 
that group.
We need to provide all these from DT for dynamic and static 
configuration and listing all pins of groups are complicate then the pad 
names.



--
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
Jon Hunter April 15, 2016, 6:30 p.m. UTC | #14
On 15/04/16 18:49, Laxman Dewangan wrote:
> 
> On Friday 15 April 2016 11:14 PM, Jon Hunter wrote:
>> On 15/04/16 17:41, Laxman Dewangan wrote:
>>> On Friday 15 April 2016 09:15 PM, Jon Hunter wrote:
>>>> On 15/04/16 16:14, Laxman Dewangan wrote:
>>>>> I used pins as this is the property from pincon generic so that I can
>>>>> use the generic implementation.
>>>>>
>>>>> Here, I will not go to the pin level control as HW does not support
>>>>> pin
>>>>> level control.
>>>>>
>>>>> I will say the unit should be interface level. Should we say
>>>>> IO_GROUP_CSIA, IO_GROUP_CSIB etc?
>>>> So we need to reflect the hardware in device-tree and although yes the
>>>> power-down for the CSI_x_xxx pads are all controlled together as a
>>>> single group, it does not feel right that we add a pseudo pin called
>>>> csix to represent these.
>>>>
>>>> The CSI_x_xxx pads are already in device-tree and so why not add a
>>>> property to each of these pads which has the IO rail information for
>>>> power-down and voltage-select?
>>> Which dt binding docs have these?
>>> I looked for nvidia,tegra210-pinmux.txt and not able to find csi_xxx.
>> For CSI you are right they are not included by the current DT binding
>> docs, however, the sdmmc1/3 pads are. So that makes things a bit more
>> messy as some are and some are not.
> 
> Yaah and so lets have the names in new dt files. Names may be same but
> define all possible names f groups in dt binding and need not to refer
> from other file which does not have all.

I still do not like that. In the case of sdmmc we now have two pinctrl
drivers to deal with for a single set of pins. That does not seem
correct IMO.

>>> Here I dont want to refer the individual pins as control should be as
>>> group.
>> I understand, however, at least for power-down control I don't see why
>> we cannot refer to the individual pins and once all are inactive then
>> the rail can be powered down.
>>
>> For switching the voltage it is a bit more complex, but may be we could
>> still look-up the IO rail based upon the pads the device uses.
>>
> 
> Yes, it can be done with ref count also for power down.

Exactly.

> But interfaces are complex. As a client, it is easy to say power down
> SDMMC1 IO interface rather than saying power down 10 pins (names) of
> that group.

Right and like I said, we could always look up the IO rail from the pins
associated once at probe time and then control it from there.

Cheers
Jon
--
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
Laxman Dewangan April 15, 2016, 6:43 p.m. UTC | #15
On Saturday 16 April 2016 12:00 AM, Jon Hunter wrote:
> On 15/04/16 18:49, Laxman Dewangan wrote:
>> On Friday 15 April 2016 11:14 PM, Jon Hunter wrote:
>>> On 15/04/16 17:41, Laxman Dewangan wrote:
>>>> On Friday 15 April 2016 09:15 PM, Jon Hunter wrote:
>>>>> On 15/04/16 16:14, Laxman Dewangan wrote:
>>>>>> I used pins as this is the property from pincon generic so that I can
>>>>>> use the generic implementation.
>>>>>>
>>>>>> Here, I will not go to the pin level control as HW does not support
>>>>>> pin
>>>>>> level control.
>>>>>>
>>>>>> I will say the unit should be interface level. Should we say
>>>>>> IO_GROUP_CSIA, IO_GROUP_CSIB etc?
>>>>> So we need to reflect the hardware in device-tree and although yes the
>>>>> power-down for the CSI_x_xxx pads are all controlled together as a
>>>>> single group, it does not feel right that we add a pseudo pin called
>>>>> csix to represent these.
>>>>>
>>>>> The CSI_x_xxx pads are already in device-tree and so why not add a
>>>>> property to each of these pads which has the IO rail information for
>>>>> power-down and voltage-select?
>>>> Which dt binding docs have these?
>>>> I looked for nvidia,tegra210-pinmux.txt and not able to find csi_xxx.
>>> For CSI you are right they are not included by the current DT binding
>>> docs, however, the sdmmc1/3 pads are. So that makes things a bit more
>>> messy as some are and some are not.
>> Yaah and so lets have the names in new dt files. Names may be same but
>> define all possible names f groups in dt binding and need not to refer
>> from other file which does not have all.
> I still do not like that. In the case of sdmmc we now have two pinctrl
> drivers to deal with for a single set of pins. That does not seem
> correct IMO.

We are ending two drivers because of the HW blocks. Pins interface and 
pad control are seen two different blocks.

Do you want to add the IO group names also in existing pin control 
driver and the new property, power-enable/disable and 
power-source-voltage belongs to these new io group names.

In this way we will have single driver. We need to see how we can 
support group/pins together.


>
>
>> But interfaces are complex. As a client, it is easy to say power down
>> SDMMC1 IO interface rather than saying power down 10 pins (names) of
>> that group.
> Right and like I said, we could always look up the IO rail from the pins
> associated once at probe time and then control it from there.

I did not get it fully. Can you please help on this using some psuedo 
code and dt property.

For  init, we can pass the regulator handle of the supply to this driver 
and during probe, it can get the voltage from regulator call and then 
set 1.8V or 3.3V. So we need to provide regulator handle from DT instead 
of voltage for probe configuration. Is this what you mean?





--
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
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-io-pad.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-io-pad.txt
new file mode 100644
index 0000000..97cdd4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-io-pad.txt
@@ -0,0 +1,102 @@ 
+NVIDIA Tegra210 PMC IO pad controller
+
+NVIDIA Tegra210 supports IO pads which can operate at 1.8V or 3.3V I/O
+power rail voltages. SW needs to configure the voltage level of IO pads
+based on platform specific power tree.
+
+The voltage configurations of IO pads should be done in boot if it is not
+going to change other wise dynamically based on IO rail voltage on that
+IO pads.
+
+The node for the Tegra210 io-pad driver must be sub node of pmc@0,7000e400.
+
+Required properties:
+- compatible: "nvidia,tegra210-io-pad"
+
+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
+
+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,
+
+See the TRM to determine which properties and values apply to each IO pads.
+Macro values for property values are defined in
+<dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
+
+The voltage supported on the pads are 1.8V and 3.3V. The enums are defined as:
+	For 1.8V, use TEGRA210_IO_RAIL_1800000UV
+	For 3.3V, use TEGRA210_IO_RAIL_3300000UV
+
+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:
+==========================
+-nvidia,io-rail-voltage:	Integer. The voltage level of IO pads. The
+				valid values are 1.8V and 3.3V. Macros are
+				defined for these voltage levels in
+				<dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
+					Use TEGRA210_IO_RAIL_1800000UV for 1.8V
+					Use TEGRA210_IO_RAIL_3300000UV for 3.3V
+
+-nvidia,io-pad-deep-power-down: Integer, representing the deep power down state
+				of the IO pads. If this is enable then IO pads
+				will be in power down state and interface is not
+				enabled for any transaction. This is power
+				saving mode of the IO pads. The macros are
+				defined for enable/disable in
+				<dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
+				  TEGRA210_IO_PAD_DEEP_POWER_DOWN_DISABLE for
+					disable.
+				  TEGRA210_IO_PAD_DEEP_POWER_DOWN_ENABLE for
+					enable.
+Valid values for pin are:
+	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.
+
+All IO pads do not support the 1.8V/3.3V configurations. Valid values for
+nvidia,io-rail-voltage are:
+	audio-hv, dmic, gpio, sdmmc1, sdmmc3, spi-hv.
+
+All above IO pads supports the deep power down state.
+
+Example:
+	#include <dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h>
+	pmc@0,7000e400 {
+		pmc-pad-control {
+			compatible = "nvidia,tegra210-io-pad";
+			pinctrl-names = "default";
+			pinctrl-0 = <&tegra_io_pad_volt_default>;
+			tegra_io_pad_volt_default: common {
+				audio {
+					pins = "audio";
+					nvidia,io-pad-deep-power-down = <TEGRA210_IO_PAD_DEEP_POWER_DOWN_DISABLE>;
+				};
+				audio-hv {
+					pins = "audio-hv";
+					nvidia,io-rail-voltage = <TEGRA210_IO_RAIL_1800000UV>;
+				};
+				gpio {
+					pins = "gpio";
+					nvidia,io-rail-voltage = <TEGRA210_IO_RAIL_1800000UV>;
+				};
+				rest {
+					pins = "dmic", "sdmmc1", "sdmmc3";
+					nvidia,io-rail-voltage = <TEGRA210_IO_RAIL_1800000UV>;
+				};
+			};
+		};
+	};
diff --git a/include/dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h b/include/dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h
new file mode 100644
index 0000000..e32166b
--- /dev/null
+++ b/include/dt-bindings/pinctrl/pinctrl-tegra210-io-pad.h
@@ -0,0 +1,24 @@ 
+/*
+ * This header provides constants for Tegra210 IO pads pinctrl bindings.
+ *
+ * Copyright (c) 2016, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * Author: Laxman Dewangan <ldewangan@nvidia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+
+#ifndef _DT_BINDINGS_PINCTRL_TEGRA210_IO_PAD_H
+#define _DT_BINDINGS_PINCTRL_TEGRA210_IO_PAD_H
+
+/* Voltage levels of Tegra210 IO rails. */
+#define TEGRA210_IO_RAIL_1800000UV		0
+#define TEGRA210_IO_RAIL_3300000UV		1
+
+/* Deep power down state enable/disable for Tegra210 IO pads */
+#define TEGRA210_IO_PAD_DEEP_POWER_DOWN_DISABLE	0
+#define TEGRA210_IO_PAD_DEEP_POWER_DOWN_ENABLE	1
+
+#endif