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 power rail sources. When IO
interface are not used then IO pads can be configure in low power
state to reduce the power from that IO pads.
This series add the support of configuration of IO pad via pinctrl
framework. The io pad driver uses the tegra PMC interface.
---
This driver was sent earlier for review along with soc/tegra pmc
changes. During review, decided to first conclude in soc/tegra pmc
patches and then review this.
Thierry applied the pmc patches in the private tree
https://github.com/thierryreding/linux/tree/tegra186
and he wanted to have the patches for user of the new APIs so that
it can be pushed to mainline.
Sending the pinctrl driver. This needs Ack/reviewed from pinctrl subsystem
i.e. Linus Welleij to apply in the Thierry's T186 branch along with
PMC patches.
Laxman Dewangan (2):
pinctrl: tegra: Add DT binding for io pads control
pinctrl: tegra: Add driver to configure voltage and power of io pads
.../bindings/pinctrl/nvidia,tegra-io-pad.txt | 112 +++++++
drivers/pinctrl/tegra/Kconfig | 12 +
drivers/pinctrl/tegra/Makefile | 1 +
drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c | 369 +++++++++++++++++++++
include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h | 21 ++
5 files changed, 515 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
create mode 100644 drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
create mode 100644 include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h
--
2.1.4
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 power rail sources. When IO
interface are not used then IO pads can be configure in low power
state to reduce the power from that IO pads.
On Tegra124, the 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 auto-detection is removed from SoC and hence SW
must 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 <[email protected]>
---
On top of the branch from Thierry's T186 work
https://github.com/thierryreding/linux/tree/tegra186
.../bindings/pinctrl/nvidia,tegra-io-pad.txt | 112 +++++++++++++++++++++
include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h | 21 ++++
2 files changed, 133 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
create mode 100644 include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h
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..15cd21c
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-io-pad.txt
@@ -0,0 +1,112 @@
+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. The IO pads can work in
+the voltage of the 1.8V and 3.3V of IO power rail sources. When IO
+interface are not used then IO pads can be configure in low power
+state to reduce the power from that IO pads.
+
+On Tegra124, the 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 auto-detection is removed from SoC and hence SW
+must 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 other wise dynamically based on IO
+rail voltage on that IO pads and usage of IO pads
+
+The DT property of the io pads must be under the node of pmc i.e.
+pmc@7000e400 for Tegra124 onwards.
+
+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-tegra-io-pad.h>
+
+The voltage supported on the pads are 1.8V and 3.3V. The enums are defined as:
+ For 1.8V, use TEGRA_IO_PAD_POWER_SOURCE_1800000UV
+ For 3.3V, use TEGRA_IO_PAD_POWER_SOURCE_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,power-source-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-tegra-io-pad.h>
+ Use TEGRA_IO_PAD_POWER_SOURCE_1800000UV for 1.8V
+ Use TEGRA_IO_PAD_POWER_SOURCE_3300000UV for 3.3V
+
+ All IO pads do not support the 1.8V/3.3V
+ configurations. Valid values for "pins" are
+ audio-hv, dmic, gpio, sdmmc1, sdmmc3, spi-hv.
+
+Other than above, following properties are supported from generic pin
+configuration explained in <dt-bindings/pinctrl/pinctrl-binding.txt>.
+low-power-enable: enable low power mode.
+low-power-disable: disable low power mode.
+
+Valid values for pin for T124 are:
+ 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
+
+ All above pins support low power mode.
+
+Valid values for pin for T210 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 above pins support low power mode. However, all IO pads do not
+ support the 1.8V/3.3V configurations. Valid values for
+ nvidia,io-pad-voltage are:
+ audio, audio-hv, cam, dbg, dmic, gpio, pex-ctrl, sdmmc1, sdmmc3,
+ spi, spi-hv,uart
+
+Example:
+ #include <dt-bindings/pinctrl/pinctrl-tegra-io-pad.h>
+ pmc@7000e400 {
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&tegra_io_pad_volt_default>;
+ tegra_io_pad_volt_default: common {
+ audio-hv {
+ pins = "audio-hv";
+ nvidia,power-source-voltage = <TEGRA_IO_PAD_POWER_SOURCE_3300000UV>;
+ };
+
+ gpio {
+ pins = "gpio";
+ invidia,power-source-voltage = <TEGRA_IO_PAD_POWER_SOURCE_1800000UV>;
+ };
+
+ audio {
+ pins = "audio", "dmic", "sdmmc1", "sdmmc3";
+ low-power-enable;
+ };
+ };
+ };
diff --git a/include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h b/include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h
new file mode 100644
index 0000000..ae55768
--- /dev/null
+++ b/include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h
@@ -0,0 +1,21 @@
+/*
+ * pinctrl-tegra-io-pad.h: Provides constants for Tegra IO pads
+ * pinctrl bindings.
+ *
+ * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
+ *
+ * Author: Laxman Dewangan <[email protected]>
+ *
+ * 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_TEGRA_IO_PAD_H
+#define _DT_BINDINGS_PINCTRL_TEGRA_IO_PAD_H
+
+/* Power source voltage of IO pads. */
+#define TEGRA_IO_PAD_POWER_SOURCE_1800000UV 0
+#define TEGRA_IO_PAD_POWER_SOURCE_3300000UV 1
+
+#endif
--
2.1.4
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 power rail sources. When IO
interface are not used then IO pads can be configure in low power
state to reduce the power from that IO pads.
On Tegra124, the 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 auto-detection is removed from SoC and hence SW
must configure the PMC register explicitly to set proper voltage in
IO pads based on IO rail power source voltage.
This driver adds the IO pad driver to configure the power state and
IO pad voltage based on the usage and power tree via pincontrol
framework. The configuration can be static and dynamic.
Signed-off-by: Laxman Dewangan <[email protected]>
---
On top of the branch from Thierry's T186 work
https://github.com/thierryreding/linux/tree/tegra186
drivers/pinctrl/tegra/Kconfig | 12 +
drivers/pinctrl/tegra/Makefile | 1 +
drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c | 369 +++++++++++++++++++++++++++
3 files changed, 382 insertions(+)
create mode 100644 drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
diff --git a/drivers/pinctrl/tegra/Kconfig b/drivers/pinctrl/tegra/Kconfig
index 24e20cc..eef1d6b 100644
--- a/drivers/pinctrl/tegra/Kconfig
+++ b/drivers/pinctrl/tegra/Kconfig
@@ -23,6 +23,18 @@ config PINCTRL_TEGRA210
bool
select PINCTRL_TEGRA
+config PINCTRL_TEGRA_IO_PAD
+ bool "Tegra IO pad Control Driver"
+ depends on ARCH_TEGRA
+ select PINCONF
+ select PINMUX
+ help
+ NVIDIA Tegra124/210 SoC has IO pads which supports multi-voltage
+ level of interfacing and deep power down mode of IO pads. The
+ voltage of IO pads are SW configurable based on IO rail of that
+ pads on T210. This driver provides the interface to change IO pad
+ voltage and power state via pincontrol interface.
+
config PINCTRL_TEGRA_XUSB
def_bool y if ARCH_TEGRA
select GENERIC_PHY
diff --git a/drivers/pinctrl/tegra/Makefile b/drivers/pinctrl/tegra/Makefile
index d9ea2be..3ebaaa2 100644
--- a/drivers/pinctrl/tegra/Makefile
+++ b/drivers/pinctrl/tegra/Makefile
@@ -4,4 +4,5 @@ obj-$(CONFIG_PINCTRL_TEGRA30) += pinctrl-tegra30.o
obj-$(CONFIG_PINCTRL_TEGRA114) += pinctrl-tegra114.o
obj-$(CONFIG_PINCTRL_TEGRA124) += pinctrl-tegra124.o
obj-$(CONFIG_PINCTRL_TEGRA210) += pinctrl-tegra210.o
+obj-$(CONFIG_PINCTRL_TEGRA_IO_PAD) += pinctrl-tegra-io-pad.o
obj-$(CONFIG_PINCTRL_TEGRA_XUSB) += pinctrl-tegra-xusb.o
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c b/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
new file mode 100644
index 0000000..7af9552
--- /dev/null
+++ b/drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
@@ -0,0 +1,369 @@
+/*
+ * pinctrl-tegra-io-pad: IO PAD driver for configuration of IO rail and deep
+ * Power Down mode via pinctrl framework.
+ *
+ * Copyright (C) 2016 NVIDIA CORPORATION. All rights reserved.
+ *
+ * Author: Laxman Dewangan <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <soc/tegra/pmc.h>
+
+#include "../core.h"
+#include "../pinconf.h"
+#include "../pinctrl-utils.h"
+
+enum tegra_io_rail_pads_params {
+ TEGRA_IO_PAD_POWER_SOURCE_VOLTAGE = PIN_CONFIG_END + 1,
+};
+
+static const struct pinconf_generic_params tegra_io_pads_cfg_params[] = {
+ {
+ .property = "nvidia,power-source-voltage",
+ .param = TEGRA_IO_PAD_POWER_SOURCE_VOLTAGE,
+ },
+};
+
+struct tegra_io_pads_cfg_info {
+ const char *name;
+ const unsigned int pins[1];
+ enum tegra_io_pad pad_id;
+ bool voltage_can_change;
+ bool support_low_power_state;
+};
+
+struct tegra_io_pad_soc_data {
+ const struct tegra_io_pads_cfg_info *pads_cfg;
+ int num_pads_cfg;
+ const struct pinctrl_pin_desc *pins_desc;
+ int num_pins_desc;
+};
+
+struct tegra_io_pads_info {
+ struct device *dev;
+ struct pinctrl_dev *pctl;
+ const struct tegra_io_pad_soc_data *soc_data;
+};
+
+static int tegra_iop_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+
+ return tiopi->soc_data->num_pads_cfg;
+}
+
+static const char *tegra_iop_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
+ unsigned int group)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+
+ return tiopi->soc_data->pads_cfg[group].name;
+}
+
+static int tegra_iop_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
+ unsigned int group,
+ const unsigned int **pins,
+ unsigned int *num_pins)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+
+ *pins = tiopi->soc_data->pads_cfg[group].pins;
+ *num_pins = 1;
+
+ return 0;
+}
+
+static const struct pinctrl_ops tegra_iop_pinctrl_ops = {
+ .get_groups_count = tegra_iop_pinctrl_get_groups_count,
+ .get_group_name = tegra_iop_pinctrl_get_group_name,
+ .get_group_pins = tegra_iop_pinctrl_get_group_pins,
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
+ .dt_free_map = pinctrl_utils_free_map,
+};
+
+static int tegra_io_pad_pinconf_get(struct pinctrl_dev *pctldev,
+ unsigned int pin, unsigned long *config)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+ int param = pinconf_to_config_param(*config);
+ const struct tegra_io_pads_cfg_info *pad_cfg =
+ &tiopi->soc_data->pads_cfg[pin];
+ enum tegra_io_pad pad_id = pad_cfg->pad_id;
+ int arg = 0;
+ int ret;
+
+ switch (param) {
+ case TEGRA_IO_PAD_POWER_SOURCE_VOLTAGE:
+ ret = tegra_io_pad_get_voltage(pad_id);
+ if (ret < 0)
+ return ret;
+ arg = ret;
+ break;
+
+ case PIN_CONFIG_LOW_POWER_MODE:
+ ret = tegra_io_pad_power_get_status(pad_id);
+ if (ret < 0)
+ return ret;
+ arg = !ret;
+ break;
+
+ default:
+ dev_err(tiopi->dev, "The parameter %d not supported\n", param);
+ return -EINVAL;
+ }
+
+ *config = pinconf_to_config_packed(param, (u16)arg);
+ return 0;
+}
+
+static int tegra_io_pad_pinconf_set(struct pinctrl_dev *pctldev,
+ unsigned int pin, unsigned long *configs,
+ unsigned int num_configs)
+{
+ struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
+ const struct tegra_io_pads_cfg_info *pad_cfg =
+ &tiopi->soc_data->pads_cfg[pin];
+ int pad_id = pad_cfg->pad_id;
+ u16 param_val;
+ int param;
+ int ret;
+ int i;
+
+ for (i = 0; i < num_configs; i++) {
+ param = pinconf_to_config_param(configs[i]);
+ param_val = pinconf_to_config_argument(configs[i]);
+
+ switch (param) {
+ case TEGRA_IO_PAD_POWER_SOURCE_VOLTAGE:
+ ret = tegra_io_pad_set_voltage(pad_id, param_val);
+ if (ret < 0) {
+ dev_err(tiopi->dev,
+ "Failed to set voltage %d of pin %u: %d\n",
+ param_val, pin, ret);
+ return ret;
+ }
+ break;
+
+ case PIN_CONFIG_LOW_POWER_MODE:
+ if (param_val)
+ ret = tegra_io_pad_power_disable(pad_id);
+ else
+ ret = tegra_io_pad_power_enable(pad_id);
+ if (ret < 0) {
+ dev_err(tiopi->dev,
+ "Failed to set DPD %d of pin %u: %d\n",
+ param_val, pin, ret);
+ return ret;
+ }
+ break;
+
+ default:
+ dev_err(tiopi->dev, "The parameter %d not supported\n",
+ param);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+static const struct pinconf_ops tegra_io_pad_pinconf_ops = {
+ .pin_config_get = tegra_io_pad_pinconf_get,
+ .pin_config_set = tegra_io_pad_pinconf_set,
+};
+
+static struct pinctrl_desc tegra_iop_pinctrl_desc = {
+ .name = "pinctrl-tegra-io-pads",
+ .pctlops = &tegra_iop_pinctrl_ops,
+ .confops = &tegra_io_pad_pinconf_ops,
+ .custom_params = tegra_io_pads_cfg_params,
+ .num_custom_params = ARRAY_SIZE(tegra_io_pads_cfg_params),
+};
+
+static int tegra_iop_pinctrl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const struct platform_device_id *id = platform_get_device_id(pdev);
+ struct device_node *np_parent = pdev->dev.parent->of_node;
+ struct tegra_io_pads_info *tiopi;
+
+ if (!np_parent) {
+ dev_err(&pdev->dev, "PMC should be register from DT\n");
+ return -ENODEV;
+ }
+
+ tiopi = devm_kzalloc(&pdev->dev, sizeof(*tiopi), GFP_KERNEL);
+ if (!tiopi)
+ return -ENOMEM;
+
+ tiopi->dev = &pdev->dev;
+ pdev->dev.of_node = np_parent;
+ tiopi->soc_data = (const struct tegra_io_pad_soc_data *)id->driver_data;
+ tegra_iop_pinctrl_desc.pins = tiopi->soc_data->pins_desc;
+ tegra_iop_pinctrl_desc.npins = tiopi->soc_data->num_pins_desc;
+ platform_set_drvdata(pdev, tiopi);
+
+ tiopi->pctl = devm_pinctrl_register(dev, &tegra_iop_pinctrl_desc,
+ tiopi);
+ if (IS_ERR(tiopi->pctl)) {
+ int ret = PTR_ERR(tiopi->pctl);
+
+ dev_err(dev, "Failed to register io-pad pinctrl driver: %d\n",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+#define TEGRA124_PAD_INFO_TABLE(_entry_) \
+ _entry_(0, "audio", AUDIO, true, false), \
+ _entry_(1, "bb", BB, true, false), \
+ _entry_(2, "cam", CAM, true, false), \
+ _entry_(3, "comp", COMP, true, false), \
+ _entry_(4, "csia", CSIA, true, false), \
+ _entry_(5, "csib", CSIB, true, false), \
+ _entry_(6, "csie", CSIE, true, false), \
+ _entry_(7, "dsi", DSI, true, false), \
+ _entry_(8, "dsib", DSIB, true, false), \
+ _entry_(9, "dsic", DSIC, true, false), \
+ _entry_(10, "dsid", DSID, true, false), \
+ _entry_(11, "hdmi", HDMI, true, false), \
+ _entry_(12, "hsic", HSIC, true, false), \
+ _entry_(13, "hv", HV, true, false), \
+ _entry_(14, "lvds", LVDS, true, false), \
+ _entry_(15, "mipi-bias", MIPI_BIAS, true, false), \
+ _entry_(16, "nand", NAND, true, false), \
+ _entry_(17, "pex-bias", PEX_BIAS, true, false), \
+ _entry_(18, "pex-clk1", PEX_CLK1, true, false), \
+ _entry_(19, "pex-clk2", PEX_CLK2, true, false), \
+ _entry_(20, "pex-ctrl", PEX_CNTRL, true, false), \
+ _entry_(21, "sdmmc1", SDMMC1, true, false), \
+ _entry_(22, "sdmmc3", SDMMC3, true, false), \
+ _entry_(23, "sdmmc4", SDMMC4, true, false), \
+ _entry_(24, "sys-ddc", SYS_DDC, true, false), \
+ _entry_(25, "uart", UART, true, false), \
+ _entry_(26, "usb0", USB0, true, false), \
+ _entry_(27, "usb1", USB1, true, false), \
+ _entry_(28, "usb2", USB2, true, false), \
+ _entry_(29, "usb-bias", USB_BIAS, true, false)
+
+#define TEGRA210_PAD_INFO_TABLE(_entry_) \
+ _entry_(0, "audio", AUDIO, true, true), \
+ _entry_(1, "audio-hv", AUDIO_HV, true, true), \
+ _entry_(2, "cam", CAM, true, true), \
+ _entry_(3, "csia", CSIA, true, false), \
+ _entry_(4, "csib", CSIB, true, false), \
+ _entry_(5, "csic", CSIC, true, false), \
+ _entry_(6, "csid", CSID, true, false), \
+ _entry_(7, "csie", CSIE, true, false), \
+ _entry_(8, "csif", CSIF, true, false), \
+ _entry_(9, "dbg", DBG, true, true), \
+ _entry_(10, "debug-nonao", DEBUG_NONAO, true, false), \
+ _entry_(11, "dmic", DMIC, true, true), \
+ _entry_(12, "dp", DP, true, false), \
+ _entry_(13, "dsi", DSI, true, false), \
+ _entry_(14, "dsib", DSIB, true, false), \
+ _entry_(15, "dsic", DSIC, true, false), \
+ _entry_(16, "dsid", DSID, true, false), \
+ _entry_(17, "emmc", SDMMC4, true, false), \
+ _entry_(18, "emmc2", EMMC2, true, false), \
+ _entry_(19, "gpio", GPIO, true, true), \
+ _entry_(20, "hdmi", HDMI, true, false), \
+ _entry_(21, "hsic", HSIC, true, false), \
+ _entry_(22, "lvds", LVDS, true, false), \
+ _entry_(23, "mipi-bias", MIPI_BIAS, true, false), \
+ _entry_(24, "pex-bias", PEX_BIAS, true, false), \
+ _entry_(25, "pex-clk1", PEX_CLK1, true, false), \
+ _entry_(26, "pex-clk2", PEX_CLK2, true, false), \
+ _entry_(27, "pex-ctrl", PEX_CNTRL, false, true), \
+ _entry_(28, "sdmmc1", SDMMC1, true, true), \
+ _entry_(29, "sdmmc3", SDMMC3, true, true), \
+ _entry_(30, "spi", SPI, true, true), \
+ _entry_(31, "spi-hv", SPI_HV, true, true), \
+ _entry_(32, "uart", UART, true, true), \
+ _entry_(33, "usb0", USB0, true, false), \
+ _entry_(34, "usb1", USB1, true, false), \
+ _entry_(35, "usb2", USB2, true, false), \
+ _entry_(36, "usb3", USB3, true, false), \
+ _entry_(37, "usb-bias", USB_BIAS, true, false)
+
+#define TEGRA_IO_PAD_INFO(_id, _name, _pad_id, _lpstate, _vchange) \
+ { \
+ .name = _name, \
+ .pins = {(_id)}, \
+ .pad_id = TEGRA_IO_PAD_##_pad_id, \
+ .voltage_can_change = (_vchange), \
+ .support_low_power_state = (_lpstate), \
+ }
+
+static const struct tegra_io_pads_cfg_info tegra124_io_pads_cfg_info[] = {
+ TEGRA124_PAD_INFO_TABLE(TEGRA_IO_PAD_INFO),
+};
+
+static const struct tegra_io_pads_cfg_info tegra210_io_pads_cfg_info[] = {
+ TEGRA210_PAD_INFO_TABLE(TEGRA_IO_PAD_INFO),
+};
+
+#define TEGRA_IO_PAD_DESC(_id, _name, _pad_id, _vchange, _lpstate) \
+ PINCTRL_PIN(_id, _name)
+
+static const struct pinctrl_pin_desc tegra124_io_pads_pinctrl_desc[] = {
+ TEGRA124_PAD_INFO_TABLE(TEGRA_IO_PAD_DESC),
+};
+
+static const struct pinctrl_pin_desc tegra210_io_pads_pinctrl_desc[] = {
+ TEGRA210_PAD_INFO_TABLE(TEGRA_IO_PAD_DESC),
+};
+
+static const struct tegra_io_pad_soc_data tegra124_io_pad_soc_data = {
+ .pins_desc = tegra124_io_pads_pinctrl_desc,
+ .num_pins_desc = ARRAY_SIZE(tegra124_io_pads_pinctrl_desc),
+ .pads_cfg = tegra124_io_pads_cfg_info,
+ .num_pins_desc = ARRAY_SIZE(tegra124_io_pads_cfg_info),
+};
+
+static const struct tegra_io_pad_soc_data tegra210_io_pad_soc_data = {
+ .pins_desc = tegra210_io_pads_pinctrl_desc,
+ .num_pins_desc = ARRAY_SIZE(tegra210_io_pads_pinctrl_desc),
+ .pads_cfg = tegra210_io_pads_cfg_info,
+ .num_pins_desc = ARRAY_SIZE(tegra210_io_pads_cfg_info),
+};
+
+static const struct platform_device_id tegra_io_pads_dev_id[] = {
+ {
+ .name = "pinctrl-t124-io-pad",
+ .driver_data = (kernel_ulong_t)&tegra124_io_pad_soc_data,
+ }, {
+ .name = "pinctrl-t210-io-pad",
+ .driver_data = (kernel_ulong_t)&tegra210_io_pad_soc_data,
+ }, {
+ },
+};
+MODULE_DEVICE_TABLE(platform, tegra_io_pads_dev_id);
+
+static struct platform_driver tegra_iop_pinctrl_driver = {
+ .driver = {
+ .name = "pinctrl-tegra-io-pad",
+ },
+ .probe = tegra_iop_pinctrl_probe,
+ .id_table = tegra_io_pads_dev_id,
+};
+
+module_platform_driver(tegra_iop_pinctrl_driver);
+
+MODULE_DESCRIPTION("NVIDIA TEGRA IO pad Control Driver");
+MODULE_AUTHOR("Laxman Dewangan <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.1.4
Hi Laxman,
[auto build test ERROR on tegra/for-next]
[also build test ERROR on v4.9-rc3 next-20161028]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Laxman-Dewangan/pinctrl-tegra-Add-support-for-IO-pad-control/20161102-173122
base: https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git for-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All error/warnings (new ones prefixed by >>):
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:42:20: error: field 'pad_id' has incomplete type
enum tegra_io_pad pad_id;
^~~~~~
drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c: In function 'tegra_io_pad_pinconf_get':
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:103:7: error: variable 'pad_id' has initializer but incomplete type
enum tegra_io_pad pad_id = pad_cfg->pad_id;
^~~~~~~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:103:20: error: storage size of 'pad_id' isn't known
enum tegra_io_pad pad_id = pad_cfg->pad_id;
^~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:109:9: error: implicit declaration of function 'tegra_io_pad_get_voltage' [-Werror=implicit-function-declaration]
ret = tegra_io_pad_get_voltage(pad_id);
^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:116:9: error: implicit declaration of function 'tegra_io_pad_power_get_status' [-Werror=implicit-function-declaration]
ret = tegra_io_pad_power_get_status(pad_id);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:103:20: warning: unused variable 'pad_id' [-Wunused-variable]
enum tegra_io_pad pad_id = pad_cfg->pad_id;
^~~~~~
drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c: In function 'tegra_io_pad_pinconf_set':
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:150:10: error: implicit declaration of function 'tegra_io_pad_set_voltage' [-Werror=implicit-function-declaration]
ret = tegra_io_pad_set_voltage(pad_id, param_val);
^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:161:11: error: implicit declaration of function 'tegra_io_pad_power_disable' [-Werror=implicit-function-declaration]
ret = tegra_io_pad_power_disable(pad_id);
^~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:163:11: error: implicit declaration of function 'tegra_io_pad_power_enable' [-Werror=implicit-function-declaration]
ret = tegra_io_pad_power_enable(pad_id);
^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c: At top level:
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:307:13: error: 'TEGRA_IO_PAD_AUDIO' undeclared here (not in a function)
.pad_id = TEGRA_IO_PAD_##_pad_id, \
^
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:232:2: note: in expansion of macro 'TEGRA_IO_PAD_INFO'
_entry_(0, "audio", AUDIO, true, false), \
^~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:313:2: note: in expansion of macro 'TEGRA124_PAD_INFO_TABLE'
TEGRA124_PAD_INFO_TABLE(TEGRA_IO_PAD_INFO),
^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:307:13: error: 'TEGRA_IO_PAD_BB' undeclared here (not in a function)
.pad_id = TEGRA_IO_PAD_##_pad_id, \
^
drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:233:2: note: in expansion of macro 'TEGRA_IO_PAD_INFO'
_entry_(1, "bb", BB, true, false), \
^~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:313:2: note: in expansion of macro 'TEGRA124_PAD_INFO_TABLE'
TEGRA124_PAD_INFO_TABLE(TEGRA_IO_PAD_INFO),
^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:307:13: error: 'TEGRA_IO_PAD_CAM' undeclared here (not in a function)
.pad_id = TEGRA_IO_PAD_##_pad_id, \
^
drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:234:2: note: in expansion of macro 'TEGRA_IO_PAD_INFO'
_entry_(2, "cam", CAM, true, false), \
^~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:313:2: note: in expansion of macro 'TEGRA124_PAD_INFO_TABLE'
TEGRA124_PAD_INFO_TABLE(TEGRA_IO_PAD_INFO),
^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:307:13: error: 'TEGRA_IO_PAD_COMP' undeclared here (not in a function)
.pad_id = TEGRA_IO_PAD_##_pad_id, \
^
drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:235:2: note: in expansion of macro 'TEGRA_IO_PAD_INFO'
_entry_(3, "comp", COMP, true, false), \
^~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:313:2: note: in expansion of macro 'TEGRA124_PAD_INFO_TABLE'
TEGRA124_PAD_INFO_TABLE(TEGRA_IO_PAD_INFO),
^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:307:13: error: 'TEGRA_IO_PAD_CSIA' undeclared here (not in a function)
.pad_id = TEGRA_IO_PAD_##_pad_id, \
^
drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:236:2: note: in expansion of macro 'TEGRA_IO_PAD_INFO'
_entry_(4, "csia", CSIA, true, false), \
^~~~~~~
>> drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c:313:2: note: in expansion of macro 'TEGRA124_PAD_INFO_TABLE'
TEGRA124_PAD_INFO_TABLE(TEGRA_IO_PAD_INFO),
^~~~~~~~~~~~~~~~~~~~~~~
vim +/pad_id +42 drivers/pinctrl/tegra/pinctrl-tegra-io-pad.c
36 },
37 };
38
39 struct tegra_io_pads_cfg_info {
40 const char *name;
41 const unsigned int pins[1];
> 42 enum tegra_io_pad pad_id;
43 bool voltage_can_change;
44 bool support_low_power_state;
45 };
46
47 struct tegra_io_pad_soc_data {
48 const struct tegra_io_pads_cfg_info *pads_cfg;
49 int num_pads_cfg;
50 const struct pinctrl_pin_desc *pins_desc;
51 int num_pins_desc;
52 };
53
54 struct tegra_io_pads_info {
55 struct device *dev;
56 struct pinctrl_dev *pctl;
57 const struct tegra_io_pad_soc_data *soc_data;
58 };
59
60 static int tegra_iop_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
61 {
62 struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
63
64 return tiopi->soc_data->num_pads_cfg;
65 }
66
67 static const char *tegra_iop_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
68 unsigned int group)
69 {
70 struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
71
72 return tiopi->soc_data->pads_cfg[group].name;
73 }
74
75 static int tegra_iop_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
76 unsigned int group,
77 const unsigned int **pins,
78 unsigned int *num_pins)
79 {
80 struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
81
82 *pins = tiopi->soc_data->pads_cfg[group].pins;
83 *num_pins = 1;
84
85 return 0;
86 }
87
88 static const struct pinctrl_ops tegra_iop_pinctrl_ops = {
89 .get_groups_count = tegra_iop_pinctrl_get_groups_count,
90 .get_group_name = tegra_iop_pinctrl_get_group_name,
91 .get_group_pins = tegra_iop_pinctrl_get_group_pins,
92 .dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
93 .dt_free_map = pinctrl_utils_free_map,
94 };
95
96 static int tegra_io_pad_pinconf_get(struct pinctrl_dev *pctldev,
97 unsigned int pin, unsigned long *config)
98 {
99 struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
100 int param = pinconf_to_config_param(*config);
101 const struct tegra_io_pads_cfg_info *pad_cfg =
102 &tiopi->soc_data->pads_cfg[pin];
> 103 enum tegra_io_pad pad_id = pad_cfg->pad_id;
104 int arg = 0;
105 int ret;
106
107 switch (param) {
108 case TEGRA_IO_PAD_POWER_SOURCE_VOLTAGE:
> 109 ret = tegra_io_pad_get_voltage(pad_id);
110 if (ret < 0)
111 return ret;
112 arg = ret;
113 break;
114
115 case PIN_CONFIG_LOW_POWER_MODE:
> 116 ret = tegra_io_pad_power_get_status(pad_id);
117 if (ret < 0)
118 return ret;
119 arg = !ret;
120 break;
121
122 default:
123 dev_err(tiopi->dev, "The parameter %d not supported\n", param);
124 return -EINVAL;
125 }
126
127 *config = pinconf_to_config_packed(param, (u16)arg);
128 return 0;
129 }
130
131 static int tegra_io_pad_pinconf_set(struct pinctrl_dev *pctldev,
132 unsigned int pin, unsigned long *configs,
133 unsigned int num_configs)
134 {
135 struct tegra_io_pads_info *tiopi = pinctrl_dev_get_drvdata(pctldev);
136 const struct tegra_io_pads_cfg_info *pad_cfg =
137 &tiopi->soc_data->pads_cfg[pin];
138 int pad_id = pad_cfg->pad_id;
139 u16 param_val;
140 int param;
141 int ret;
142 int i;
143
144 for (i = 0; i < num_configs; i++) {
145 param = pinconf_to_config_param(configs[i]);
146 param_val = pinconf_to_config_argument(configs[i]);
147
148 switch (param) {
149 case TEGRA_IO_PAD_POWER_SOURCE_VOLTAGE:
> 150 ret = tegra_io_pad_set_voltage(pad_id, param_val);
151 if (ret < 0) {
152 dev_err(tiopi->dev,
153 "Failed to set voltage %d of pin %u: %d\n",
154 param_val, pin, ret);
155 return ret;
156 }
157 break;
158
159 case PIN_CONFIG_LOW_POWER_MODE:
160 if (param_val)
> 161 ret = tegra_io_pad_power_disable(pad_id);
162 else
> 163 ret = tegra_io_pad_power_enable(pad_id);
164 if (ret < 0) {
165 dev_err(tiopi->dev,
166 "Failed to set DPD %d of pin %u: %d\n",
167 param_val, pin, ret);
168 return ret;
169 }
170 break;
171
172 default:
173 dev_err(tiopi->dev, "The parameter %d not supported\n",
174 param);
175 return -EINVAL;
176 }
177 }
178
179 return 0;
180 }
181
182 static const struct pinconf_ops tegra_io_pad_pinconf_ops = {
183 .pin_config_get = tegra_io_pad_pinconf_get,
184 .pin_config_set = tegra_io_pad_pinconf_set,
185 };
186
187 static struct pinctrl_desc tegra_iop_pinctrl_desc = {
188 .name = "pinctrl-tegra-io-pads",
189 .pctlops = &tegra_iop_pinctrl_ops,
190 .confops = &tegra_io_pad_pinconf_ops,
191 .custom_params = tegra_io_pads_cfg_params,
192 .num_custom_params = ARRAY_SIZE(tegra_io_pads_cfg_params),
193 };
194
195 static int tegra_iop_pinctrl_probe(struct platform_device *pdev)
196 {
197 struct device *dev = &pdev->dev;
198 const struct platform_device_id *id = platform_get_device_id(pdev);
199 struct device_node *np_parent = pdev->dev.parent->of_node;
200 struct tegra_io_pads_info *tiopi;
201
202 if (!np_parent) {
203 dev_err(&pdev->dev, "PMC should be register from DT\n");
204 return -ENODEV;
205 }
206
207 tiopi = devm_kzalloc(&pdev->dev, sizeof(*tiopi), GFP_KERNEL);
208 if (!tiopi)
209 return -ENOMEM;
210
211 tiopi->dev = &pdev->dev;
212 pdev->dev.of_node = np_parent;
213 tiopi->soc_data = (const struct tegra_io_pad_soc_data *)id->driver_data;
214 tegra_iop_pinctrl_desc.pins = tiopi->soc_data->pins_desc;
215 tegra_iop_pinctrl_desc.npins = tiopi->soc_data->num_pins_desc;
216 platform_set_drvdata(pdev, tiopi);
217
218 tiopi->pctl = devm_pinctrl_register(dev, &tegra_iop_pinctrl_desc,
219 tiopi);
220 if (IS_ERR(tiopi->pctl)) {
221 int ret = PTR_ERR(tiopi->pctl);
222
223 dev_err(dev, "Failed to register io-pad pinctrl driver: %d\n",
224 ret);
225 return ret;
226 }
227
228 return 0;
229 }
230
231 #define TEGRA124_PAD_INFO_TABLE(_entry_) \
> 232 _entry_(0, "audio", AUDIO, true, false), \
233 _entry_(1, "bb", BB, true, false), \
234 _entry_(2, "cam", CAM, true, false), \
235 _entry_(3, "comp", COMP, true, false), \
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Wed, Nov 2, 2016 at 10:09 AM, Laxman Dewangan <[email protected]> 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 power rail sources. When IO
> interface are not used then IO pads can be configure in low power
> state to reduce the power from that IO pads.
>
> On Tegra124, the 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 auto-detection is removed from SoC and hence SW
> must configure the PMC register explicitly to set proper voltage in
> IO pads based on IO rail power source voltage.
>
> This driver adds the IO pad driver to configure the power state and
> IO pad voltage based on the usage and power tree via pincontrol
> framework. The configuration can be static and dynamic.
>
> Signed-off-by: Laxman Dewangan <[email protected]>
Looking for an ACK from Stephen &| Thierry.
> ---
> On top of the branch from Thierry's T186 work
> https://github.com/thierryreding/linux/tree/tegra186
But it's an orthogonal patch right?
The build robot seems to have problems with it so pls fix these.
> +static const struct pinconf_generic_params tegra_io_pads_cfg_params[] = {
> + {
> + .property = "nvidia,power-source-voltage",
> + .param = TEGRA_IO_PAD_POWER_SOURCE_VOLTAGE,
> + },
> +};
Why can you not use the standard power-source binding
from Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
instead of inventing this nvidia,* variant?
Yours,
Linus Walleij
On Wed, Nov 2, 2016 at 10:09 AM, Laxman Dewangan <[email protected]> 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 power rail sources. When IO
> interface are not used then IO pads can be configure in low power
> state to reduce the power from that IO pads.
>
> On Tegra124, the 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 auto-detection is removed from SoC and hence SW
> must 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 <[email protected]>
(...)
> +-nvidia,power-source-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-tegra-io-pad.h>
> + Use TEGRA_IO_PAD_POWER_SOURCE_1800000UV for 1.8V
> + Use TEGRA_IO_PAD_POWER_SOURCE_3300000UV for 3.3V
> +
> + All IO pads do not support the 1.8V/3.3V
> + configurations. Valid values for "pins" are
> + audio-hv, dmic, gpio, sdmmc1, sdmmc3, spi-hv.
As mentioned in another patch, what is wrong with the standard
power-source binding?
Yours,
Linus Walleij
On Saturday 05 November 2016 03:54 AM, Linus Walleij wrote:
> On Wed, Nov 2, 2016 at 10:09 AM, Laxman Dewangan <[email protected]> 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 power rail sources. When IO
>> interface are not used then IO pads can be configure in low power
>> state to reduce the power from that IO pads.
>>
>> On Tegra124, the 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 auto-detection is removed from SoC and hence SW
>> must configure the PMC register explicitly to set proper voltage in
>> IO pads based on IO rail power source voltage.
>>
>> This driver adds the IO pad driver to configure the power state and
>> IO pad voltage based on the usage and power tree via pincontrol
>> framework. The configuration can be static and dynamic.
>>
>> Signed-off-by: Laxman Dewangan <[email protected]>
> Looking for an ACK from Stephen &| Thierry.
This driver depends on some new APIs from tegra pmc driver. The new APIs
are integrated in Thierry's branch and he wanted to push the changes to
linux-next/main path if there is any client driver for this.
This series is the client driver for tegra PMC.
So if you are fine, then you can ACK and Thierry can take this in his
branch.
Or I will leave to you/Thierry to propose if some other idea for
discussion/acceptance.
>
>> ---
>> On top of the branch from Thierry's T186 work
>> https://github.com/thierryreding/linux/tree/tegra186
> But it's an orthogonal patch right?
>
> The build robot seems to have problems with it so pls fix these.
The driver built in pinctrl branch and so this is expected. The APIs are
in the above branch.
>
>> +static const struct pinconf_generic_params tegra_io_pads_cfg_params[] = {
>> + {
>> + .property = "nvidia,power-source-voltage",
>> + .param = TEGRA_IO_PAD_POWER_SOURCE_VOLTAGE,
>> + },
>> +};
> Why can you not use the standard power-source binding
> from Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> instead of inventing this nvidia,* variant?
Per binding doc,
power-source - select between different power supplies
So actually it selects the different source of power supply.
In my case, I will have same supply but voltage of that supply get changed.
So here property is for the power-supply-voltage.
On Mon, Nov 7, 2016 at 6:41 AM, Laxman Dewangan <[email protected]> wrote:
> On Saturday 05 November 2016 03:54 AM, Linus Walleij wrote:
>> On Wed, Nov 2, 2016 at 10:09 AM, Laxman Dewangan <[email protected]>
(....)
>>> On Tegra124, the 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 auto-detection is removed from SoC and hence SW
>>> must configure the PMC register explicitly to set proper voltage in
>>> IO pads based on IO rail power source voltage.
(...)
>>> +static const struct pinconf_generic_params tegra_io_pads_cfg_params[] =
>>> {
>>> + {
>>> + .property = "nvidia,power-source-voltage",
>>> + .param = TEGRA_IO_PAD_POWER_SOURCE_VOLTAGE,
>>> + },
>>> +};
>>
>> Why can you not use the standard power-source binding
>> from Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>> instead of inventing this nvidia,* variant?
>
>
> Per binding doc,
> power-source - select between different power supplies
>
> So actually it selects the different source of power supply.
> In my case, I will have same supply but voltage of that supply get changed.
> So here property is for the power-supply-voltage.
I doubt that seriously. Are you sure? Then the commit message is
misleading because it is talking about different power rails.
The usual design of such IP is that there is a switch that select
a voltage from several available rails and this is what the commit
message seems to be saying, and that is what the binding is for.
If you could actually change the voltage it would change for all
other pins using the same voltage source as well, would it not?
Unless there is one voltage regulator per pin, which seems like
a very expensive and chip surface consuming solution. (Albeit
theoretically possible.)
If you can *actually* change the volatage, it needs to be modeled
as a (fixed voltage?) regulator, not as a custom property for the pin
control attributes. I guess you definiately need the regulator framework
to accumulate and infer the different consumer requirements anyway
in that case.
Yours,
Linus Walleij
On Tuesday 08 November 2016 03:45 PM, Linus Walleij wrote:
> On Mon, Nov 7, 2016 at 6:41 AM, Laxman Dewangan <[email protected]> wrote:
>> On Saturday 05 November 2016 03:54 AM, Linus Walleij wrote:
>>> On Wed, Nov 2, 2016 at 10:09 AM, Laxman Dewangan <[email protected]>
> (....)
>>>> On Tegra124, the 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 auto-detection is removed from SoC and hence SW
>>>> must configure the PMC register explicitly to set proper voltage in
>>>> IO pads based on IO rail power source voltage.
> (...)
>>>> +static const struct pinconf_generic_params tegra_io_pads_cfg_params[] =
>>>> {
>>>> + {
>>>> + .property = "nvidia,power-source-voltage",
>>>> + .param = TEGRA_IO_PAD_POWER_SOURCE_VOLTAGE,
>>>> + },
>>>> +};
>>> Why can you not use the standard power-source binding
>>> from Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>>> instead of inventing this nvidia,* variant?
>>
>> Per binding doc,
>> power-source - select between different power supplies
>>
>> So actually it selects the different source of power supply.
>> In my case, I will have same supply but voltage of that supply get changed.
>> So here property is for the power-supply-voltage.
> I doubt that seriously. Are you sure? Then the commit message is
> misleading because it is talking about different power rails.
The set of pins belongs to the IO pad group and this pad group has power
supply from external PMIC. The IO pads support multi-level voltage and
the level need to be configured in the PMIC rail via regulator calls and
the IO pads configuration register for that level.
> The usual design of such IP is that there is a switch that select
> a voltage from several available rails and this is what the commit
> message seems to be saying, and that is what the binding is for.
There is no switch to select the power source inside IP. We have only
one source for supply these pins (IO pads) and the source voltage can be
change here.
>
> If you could actually change the voltage it would change for all
> other pins using the same voltage source as well, would it not?
There is grouping of pins based on interface and yes, voltage level gets
changed for those group of pins. Like form SDMMC interface all data nd
clock lines, for i2c SCL and SDA lines etc.
The HW IP design is like that from single IO voltage source, all pins
are affected.
>
> Unless there is one voltage regulator per pin, which seems like
> a very expensive and chip surface consuming solution. (Albeit
> theoretically possible.)
>
> If you can *actually* change the volatage, it needs to be modeled
> as a (fixed voltage?) regulator, not as a custom property for the pin
> control attributes. I guess you definiately need the regulator framework
> to accumulate and infer the different consumer requirements anyway
> in that case.
The PMIC voltage output is changed via regulator calls.
Here, we need to have two configruations for given voltage level of
interface:
* One at IO voltage from PMIC via regulator call to change votlage of IO
rail.
* Second, configure the IO pad register to tell the IO voltage level so
that it can configured internally for that level.
On Tue, Nov 8, 2016 at 11:20 AM, Laxman Dewangan <[email protected]> wrote:
> On Tuesday 08 November 2016 03:45 PM, Linus Walleij wrote:
>> If you can *actually* change the volatage, it needs to be modeled
>> as a (fixed voltage?) regulator, not as a custom property for the pin
>> control attributes. I guess you definiately need the regulator framework
>> to accumulate and infer the different consumer requirements anyway
>> in that case.
>
> The PMIC voltage output is changed via regulator calls.
> Here, we need to have two configruations for given voltage level of
> interface:
> * One at IO voltage from PMIC via regulator call to change votlage of IO
> rail.
> * Second, configure the IO pad register to tell the IO voltage level so that
> it can configured internally for that level.
I understand! (I think.)
But then the two things (A) changing the regulator voltage and (B) changing
the pin setting need to happen at the same time do they
not?
Now you're just hardcoding something into these device tree properties
and hoping that the regulators will somehow be set up in accordance to
what you set up for the pads in the device tree, correct?
To me it seems like the pins/pads should all have an <&phandle> to
the regulator controlling its voltage output, in the device tree.
In the Linux kernel, the driver has to regulator_[bulk_]get() this for
each pin, check the voltage with regulator_get_voltage() and set up
this according to the supplied voltage.
The driver then ideally should subscribe to regulator voltage notifier
events to change the setting if the voltage changes. I guess. But
atleast the first step seems inevitable: get the voltage from a regulator.
Else there is no dependency between the regulator and its consumer.
So what your pins need is a regulator phandle, not a magic value to
be poked into a register, hoping things will match up.
I understand that this is a simple quick-and-dirty solution but it is
not the right solution.
Yours,
Linus Walleij
On Tuesday 08 November 2016 06:59 PM, Linus Walleij wrote:
> On Tue, Nov 8, 2016 at 11:20 AM, Laxman Dewangan <[email protected]> wrote:
>> On Tuesday 08 November 2016 03:45 PM, Linus Walleij wrote:
>>> If you can *actually* change the volatage, it needs to be modeled
>>> as a (fixed voltage?) regulator, not as a custom property for the pin
>>> control attributes. I guess you definiately need the regulator framework
>>> to accumulate and infer the different consumer requirements anyway
>>> in that case.
>> The PMIC voltage output is changed via regulator calls.
>> Here, we need to have two configruations for given voltage level of
>> interface:
>> * One at IO voltage from PMIC via regulator call to change votlage of IO
>> rail.
>> * Second, configure the IO pad register to tell the IO voltage level so that
>> it can configured internally for that level.
> I understand! (I think.)
Thanks,
>
> But then the two things (A) changing the regulator voltage and (B) changing
> the pin setting need to happen at the same time do they
> not?
>
> Now you're just hardcoding something into these device tree properties
> and hoping that the regulators will somehow be set up in accordance to
> what you set up for the pads in the device tree, correct?
There is two types of configuration in given platform, the IO voltage
does not get change (fixed in given platform) and in some of cases, get
change dynamically like SDIO3.0 where the voltage switches to 3.3V and 1.8V.
Yes, it can be integrated with the regulator handle and then it can call
the required configurations through notifier and regulator_get_voltage().
But I think it is too much complex for the static configurations. This
mandate also to populate the regulator handle and all power tree.
The simple way for static configuration (case where voltage does not get
change), just take the power tree IO voltage from DT and configure the
IO pad control register.
For dynamic case, there is some sequence need to be followed based on
voltage direction change (towards lower or towards higher) for the
voltage change and the IO pad voltage configuration and it is simple to
do it from client driver.
>
> To me it seems like the pins/pads should all have an <&phandle> to
> the regulator controlling its voltage output, in the device tree.
>
> In the Linux kernel, the driver has to regulator_[bulk_]get() this for
> each pin, check the voltage with regulator_get_voltage() and set up
> this according to the supplied voltage.
>
> The driver then ideally should subscribe to regulator voltage notifier
> events to change the setting if the voltage changes. I guess. But
> atleast the first step seems inevitable: get the voltage from a regulator.
>
> Else there is no dependency between the regulator and its consumer.
>
> So what your pins need is a regulator phandle, not a magic value to
> be poked into a register, hoping things will match up.
>
> I understand that this is a simple quick-and-dirty solution but it is
> not the right solution.
Yaah, the static power tree configuration is much simple in this
approach without having regulator drivers and support.
Integrating with regulator driver can be done here also.
I like to have both approach, through pinmux DT and also from regulator.
So based on the platform, if regulator supported then populate required
properties in DT for regulator else go on standard pinmux DT way (for
non-regulator cases).
Need your opinion?
On Tue, Nov 08, 2016 at 07:05:26PM +0530, Laxman Dewangan wrote:
>
> On Tuesday 08 November 2016 06:59 PM, Linus Walleij wrote:
> > On Tue, Nov 8, 2016 at 11:20 AM, Laxman Dewangan <[email protected]> wrote:
> > > On Tuesday 08 November 2016 03:45 PM, Linus Walleij wrote:
> > > > If you can *actually* change the volatage, it needs to be modeled
> > > > as a (fixed voltage?) regulator, not as a custom property for the pin
> > > > control attributes. I guess you definiately need the regulator framework
> > > > to accumulate and infer the different consumer requirements anyway
> > > > in that case.
> > > The PMIC voltage output is changed via regulator calls.
> > > Here, we need to have two configruations for given voltage level of
> > > interface:
> > > * One at IO voltage from PMIC via regulator call to change votlage of IO
> > > rail.
> > > * Second, configure the IO pad register to tell the IO voltage level so that
> > > it can configured internally for that level.
> > I understand! (I think.)
>
> Thanks,
>
> >
> > But then the two things (A) changing the regulator voltage and (B) changing
> > the pin setting need to happen at the same time do they
> > not?
> >
> > Now you're just hardcoding something into these device tree properties
> > and hoping that the regulators will somehow be set up in accordance to
> > what you set up for the pads in the device tree, correct?
>
> There is two types of configuration in given platform, the IO voltage does
> not get change (fixed in given platform) and in some of cases, get change
> dynamically like SDIO3.0 where the voltage switches to 3.3V and 1.8V.
>
> Yes, it can be integrated with the regulator handle and then it can call the
> required configurations through notifier and regulator_get_voltage().
> But I think it is too much complex for the static configurations. This
> mandate also to populate the regulator handle and all power tree.
It looks as if regulator notifiers should be able to support whatever
use-cases we may have (I suspect that we really only need pre- and post-
voltage-change notifications.
> The simple way for static configuration (case where voltage does not get
> change), just take the power tree IO voltage from DT and configure the IO
> pad control register.
For static configurations where we have a regulator along with a pinmux
setting for the I/O voltage we could potentially run into issues where
both settings don't match. How would we handle that?
> For dynamic case, there is some sequence need to be followed based on
> voltage direction change (towards lower or towards higher) for the voltage
> change and the IO pad voltage configuration and it is simple to do it from
> client driver.
Are patches available for this? It'd be useful to know how this will end
up being used in order to come up with the best solution.
In general I think it's good to send a complete series of patches, even
if it's long and spans multiple subsystems. As it is it's difficult for
anyone else (myself included) to understand where this is headed, which
makes it more complicated than necessary to get at the final solution.
> > To me it seems like the pins/pads should all have an <&phandle> to
> > the regulator controlling its voltage output, in the device tree.
> >
> > In the Linux kernel, the driver has to regulator_[bulk_]get() this for
> > each pin, check the voltage with regulator_get_voltage() and set up
> > this according to the supplied voltage.
> >
> > The driver then ideally should subscribe to regulator voltage notifier
> > events to change the setting if the voltage changes. I guess. But
> > atleast the first step seems inevitable: get the voltage from a regulator.
> >
> > Else there is no dependency between the regulator and its consumer.
> >
> > So what your pins need is a regulator phandle, not a magic value to
> > be poked into a register, hoping things will match up.
> >
> > I understand that this is a simple quick-and-dirty solution but it is
> > not the right solution.
>
>
> Yaah, the static power tree configuration is much simple in this approach
> without having regulator drivers and support.
>
> Integrating with regulator driver can be done here also.
>
> I like to have both approach, through pinmux DT and also from regulator. So
> based on the platform, if regulator supported then populate required
> properties in DT for regulator else go on standard pinmux DT way (for
> non-regulator cases).
Again, it would be best to see this in actual use. Right now it's not
clear that we'll even have both cases. Implementing both approaches will
mean potentially unused code.
On another note: in my experience you seldom need both cases, since the
dynamic configuration is the hard one, and the static configuration will
usually be a special case of the dynamic configuration.
Thierry
On Tue, Nov 8, 2016 at 2:35 PM, Laxman Dewangan <[email protected]> wrote:
> There is two types of configuration in given platform, the IO voltage does
> not get change (fixed in given platform) and in some of cases, get change
> dynamically like SDIO3.0 where the voltage switches to 3.3V and 1.8V.
>
> Yes, it can be integrated with the regulator handle and then it can call the
> required configurations through notifier and regulator_get_voltage().
> But I think it is too much complex for the static configurations. This
> mandate also to populate the regulator handle and all power tree.
>
> The simple way for static configuration (case where voltage does not get
> change), just take the power tree IO voltage from DT and configure the IO
> pad control register.
>
> For dynamic case, there is some sequence need to be followed based on
> voltage direction change (towards lower or towards higher) for the voltage
> change and the IO pad voltage configuration and it is simple to do it from
> client driver.
The devicetree should describe the platform.
Adding this custom attribute does not describe the platform very
well since the dependency to the corresponding regulator is hidden.
The point of device tree is not as much to make things simple as
to describe the world properly.
So to me it is simple: use regulators and phandles.
It might require a bit of upfront coding but the result will look
much nicer.
Yours,
Linus Walleij
On Tuesday 08 November 2016 08:12 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Tue, Nov 08, 2016 at 07:05:26PM +0530, Laxman Dewangan wrote:
>>
>> Yes, it can be integrated with the regulator handle and then it can call the
>> required configurations through notifier and regulator_get_voltage().
>> But I think it is too much complex for the static configurations. This
>> mandate also to populate the regulator handle and all power tree.
> It looks as if regulator notifiers should be able to support whatever
> use-cases we may have (I suspect that we really only need pre- and post-
> voltage-change notifications.
>
Yes, we have pre/post and abort notification from regulator.
REGULATOR_EVENT_PRE_VOLTAGE_CHANGE,
REGULATOR_EVENT_ABORT_VOLTAGE_CHANGE,
REGULATOR_EVENT_VOLTAGE_CHANGE,
So it can be done dynamically.
As the notification is atomic context, we need to support atomic context
of pmc register configurations.
>> The simple way for static configuration (case where voltage does not get
>> change), just take the power tree IO voltage from DT and configure the IO
>> pad control register.
> For static configurations where we have a regulator along with a pinmux
> setting for the I/O voltage we could potentially run into issues where
> both settings don't match. How would we handle that?
As a process, we are generating the IO pad control DT configuration from
the customer pinmux spreadsheet. So there is data population by hardware
(system eng) team and SW just use the auto-generated dtsi file in SW.
This avoided the error/mistake created by the SW engineer.
>
>> For dynamic case, there is some sequence need to be followed based on
>> voltage direction change (towards lower or towards higher) for the voltage
>> change and the IO pad voltage configuration and it is simple to do it from
>> client driver.
> Are patches available for this? It'd be useful to know how this will end
> up being used in order to come up with the best solution.
>
> In general I think it's good to send a complete series of patches, even
> if it's long and spans multiple subsystems. As it is it's difficult for
> anyone else (myself included) to understand where this is headed, which
> makes it more complicated than necessary to get at the final solution.
The dynamic setting is used by mmc driver and it is handled by mmc team.
They are waiting for framework/infrastructure to be available for their
patches.
However, in downstream, we have all these implemented. In downstram we
have pad control driver but instead of new framework, we decided to do
it through pinctrl framework.
So if you look for mmc driver in downstream, you can get the actual
code about usage.
However, for clarity, here is sequence:
1. Voltage from 3.3V to 1.8V
ret = regulator_set_voltage(regulator, 1800000, 1800000);
if (!ret)
ret = padctrl_set_voltage(padctrl, 1800000);
2. Voltage from 1.8V to 3.3V
ret = padctrl_set_voltage(padctrl, 3300000);
if (!ret)
ret = regulator_set_voltage(regulator, 3300000, 3300000);
With pinctrl framework, the APIs will be
pinctrl_lookup_state(pinctrl, "io-pad-3v3");
or
pinctrl_lookup_state(pinctrl, "io-pad-1v8");
The static setting is doen during initialization of the driver.
>>
>> I like to have both approach, through pinmux DT and also from regulator. So
>> based on the platform, if regulator supported then populate required
>> properties in DT for regulator else go on standard pinmux DT way (for
>> non-regulator cases).
> Again, it would be best to see this in actual use. Right now it's not
> clear that we'll even have both cases. Implementing both approaches will
> mean potentially unused code.
We can have the only one say regulator approach then it will be
mandatory to have regulator nodes for all vdd-io so that io-pads are
configured properly.
In the probe, it will get regulator and if found the get_voltage and
configure pmc.
I am thinking about supporting IO pad configuration(static) in case we
do not have regulator enabled/up in platform and want to set IO pads
manually from DT.
> On another note: in my experience you seldom need both cases, since the
> dynamic configuration is the hard one, and the static configuration will
> usually be a special case of the dynamic configuration.
>
>
Cases will be with regulator support available or not.
So if we donot want to support the cases, where actual regualator
support is not available then we will not need to set IO pads voltage
via DT framework.
However, we will still needs low-power support from pinctrl framework.
On Tuesday 08 November 2016 09:16 PM, Linus Walleij wrote:
> On Tue, Nov 8, 2016 at 2:35 PM, Laxman Dewangan <[email protected]> wrote:
>
>> There is two types of configuration in given platform, the IO voltage does
>> not get change (fixed in given platform) and in some of cases, get change
>> dynamically like SDIO3.0 where the voltage switches to 3.3V and 1.8V.
>>
>> Yes, it can be integrated with the regulator handle and then it can call the
>> required configurations through notifier and regulator_get_voltage().
>> But I think it is too much complex for the static configurations. This
>> mandate also to populate the regulator handle and all power tree.
>>
>> The simple way for static configuration (case where voltage does not get
>> change), just take the power tree IO voltage from DT and configure the IO
>> pad control register.
>>
>> For dynamic case, there is some sequence need to be followed based on
>> voltage direction change (towards lower or towards higher) for the voltage
>> change and the IO pad voltage configuration and it is simple to do it from
>> client driver.
> The devicetree should describe the platform.
>
> Adding this custom attribute does not describe the platform very
> well since the dependency to the corresponding regulator is hidden.
>
> The point of device tree is not as much to make things simple as
> to describe the world properly.
>
> So to me it is simple: use regulators and phandles.
>
> It might require a bit of upfront coding but the result will look
> much nicer.
Oops, I asked same clarification when replying the Thierry's comment.
Got answer now.. only via regulator support.
I am going to support the IO pad voltage control with regulator only.
No custom attribute for this.
However, for support for low-power will be same as this patch.