Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1513772 > unrolled thread

[PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads Laxman Dewangan <ldewangan@nvidia.com> - 2016-11-02 10:30 +0100
    Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and  power of io pads Linus Walleij <linus.walleij@linaro.org> - 2016-11-04 23:30 +0100
      Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and  power of io pads Laxman Dewangan <ldewangan@nvidia.com> - 2016-11-07 07:00 +0100
        Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and  power of io pads Linus Walleij <linus.walleij@linaro.org> - 2016-11-08 11:20 +0100
          Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and  power of io pads Laxman Dewangan <ldewangan@nvidia.com> - 2016-11-08 11:50 +0100
            Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and  power of io pads Linus Walleij <linus.walleij@linaro.org> - 2016-11-08 14:30 +0100
              Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and  power of io pads Laxman Dewangan <ldewangan@nvidia.com> - 2016-11-08 15:00 +0100
                Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and  power of io pads Thierry Reding <thierry.reding@gmail.com> - 2016-11-08 15:50 +0100
                  Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and  power of io pads Laxman Dewangan <ldewangan@nvidia.com> - 2016-11-08 17:10 +0100
                Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and  power of io pads Linus Walleij <linus.walleij@linaro.org> - 2016-11-08 16:50 +0100
                  Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and  power of io pads Laxman Dewangan <ldewangan@nvidia.com> - 2016-11-08 17:10 +0100

#1513772 — [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads

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 <ldewangan@nvidia.com>

---
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 <ldewangan@nvidia.com>
+ *
+ * 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 <ldewangan@nvidia.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

[toc] | [next] | [standalone]

#1515458 — Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads

On Wed, Nov 2, 2016 at 10:09 AM, Laxman Dewangan <ldewangan@nvidia.com> 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 <ldewangan@nvidia.com>

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

[toc] | [prev] | [next] | [standalone]

#1515854 — Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads

On Saturday 05 November 2016 03:54 AM, Linus Walleij wrote:
> On Wed, Nov 2, 2016 at 10:09 AM, Laxman Dewangan <ldewangan@nvidia.com> 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 <ldewangan@nvidia.com>
> 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.

[toc] | [prev] | [next] | [standalone]

#1517035 — Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads

On Mon, Nov 7, 2016 at 6:41 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> On Saturday 05 November 2016 03:54 AM, Linus Walleij wrote:
>> On Wed, Nov 2, 2016 at 10:09 AM, Laxman Dewangan <ldewangan@nvidia.com>
(....)
>>> 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

[toc] | [prev] | [next] | [standalone]

#1517068 — Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads

On Tuesday 08 November 2016 03:45 PM, Linus Walleij wrote:
> On Mon, Nov 7, 2016 at 6:41 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>> On Saturday 05 November 2016 03:54 AM, Linus Walleij wrote:
>>> On Wed, Nov 2, 2016 at 10:09 AM, Laxman Dewangan <ldewangan@nvidia.com>
> (....)
>>>> 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.

[toc] | [prev] | [next] | [standalone]

#1517163 — Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads

On Tue, Nov 8, 2016 at 11:20 AM, Laxman Dewangan <ldewangan@nvidia.com> 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

[toc] | [prev] | [next] | [standalone]

#1517209 — Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads

On Tuesday 08 November 2016 06:59 PM, Linus Walleij wrote:
> On Tue, Nov 8, 2016 at 11:20 AM, Laxman Dewangan <ldewangan@nvidia.com> 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?

[toc] | [prev] | [next] | [standalone]

#1517249 — Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads

[Multipart message — attachments visible in raw view] — view raw

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 <ldewangan@nvidia.com> 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

[toc] | [prev] | [next] | [standalone]

#1517321 — Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads

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.

[toc] | [prev] | [next] | [standalone]

#1517309 — Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads

On Tue, Nov 8, 2016 at 2:35 PM, Laxman Dewangan <ldewangan@nvidia.com> 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

[toc] | [prev] | [next] | [standalone]

#1517326 — Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads

On Tuesday 08 November 2016 09:16 PM, Linus Walleij wrote:
> On Tue, Nov 8, 2016 at 2:35 PM, Laxman Dewangan <ldewangan@nvidia.com> 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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web