Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759630AbcDMJFB (ORCPT ); Wed, 13 Apr 2016 05:05:01 -0400 Received: from hqemgate15.nvidia.com ([216.228.121.64]:13298 "EHLO hqemgate15.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759581AbcDMJEy (ORCPT ); Wed, 13 Apr 2016 05:04:54 -0400 X-PGP-Universal: processed; by hqnvupgp07.nvidia.com on Wed, 13 Apr 2016 02:02:21 -0700 Subject: Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control To: Laxman Dewangan , , , , , , References: <1460473007-11535-1-git-send-email-ldewangan@nvidia.com> <1460473007-11535-7-git-send-email-ldewangan@nvidia.com> CC: , , , From: Jon Hunter Message-ID: <570E0BAE.8090404@nvidia.com> Date: Wed, 13 Apr 2016 10:04:46 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1460473007-11535-7-git-send-email-ldewangan@nvidia.com> X-Originating-IP: [10.21.132.108] X-ClientProxiedBy: UKMAIL101.nvidia.com (10.26.138.13) To UKMAIL101.nvidia.com (10.26.138.13) Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6313 Lines: 156 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 > --- > .../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 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 > + > + > +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 > + > + 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 > + > + 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 > + 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 = ; > + }; > + audio-hv { > + pins = "audio-hv"; > + nvidia,io-rail-voltage = ; > + }; > + gpio { > + pins = "gpio"; > + nvidia,io-rail-voltage = ; > + }; > + rest { > + pins = "dmic", "sdmmc1", "sdmmc3"; > + nvidia,io-rail-voltage = ; > + }; 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