* [PATCH 0/2] pinctrl: tegra: Add support for IO pad control
@ 2016-11-02 9:09 Laxman Dewangan
2016-11-02 9:09 ` [PATCH 1/2] pinctrl: tegra: Add DT binding for io pads control Laxman Dewangan
2016-11-02 9:09 ` [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads Laxman Dewangan
0 siblings, 2 replies; 15+ messages in thread
From: Laxman Dewangan @ 2016-11-02 9:09 UTC (permalink / raw)
To: thierry.reding, swarren, linus.walleij, robh+dt, mark.rutland
Cc: jonathanh, yamada.masahiro, linux-gpio, devicetree, linux-tegra,
linux-kernel, Laxman Dewangan
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
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/2] pinctrl: tegra: Add DT binding for io pads control 2016-11-02 9:09 [PATCH 0/2] pinctrl: tegra: Add support for IO pad control Laxman Dewangan @ 2016-11-02 9:09 ` Laxman Dewangan 2016-11-04 22:29 ` Linus Walleij 2016-11-02 9:09 ` [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads Laxman Dewangan 1 sibling, 1 reply; 15+ messages in thread From: Laxman Dewangan @ 2016-11-02 9:09 UTC (permalink / raw) To: thierry.reding, swarren, linus.walleij, robh+dt, mark.rutland Cc: jonathanh, yamada.masahiro, linux-gpio, devicetree, linux-tegra, linux-kernel, Laxman Dewangan 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 <ldewangan@nvidia.com> --- 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 <ldewangan@nvidia.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + */ + +#ifndef _DT_BINDINGS_PINCTRL_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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] pinctrl: tegra: Add DT binding for io pads control 2016-11-02 9:09 ` [PATCH 1/2] pinctrl: tegra: Add DT binding for io pads control Laxman Dewangan @ 2016-11-04 22:29 ` Linus Walleij 0 siblings, 0 replies; 15+ messages in thread From: Linus Walleij @ 2016-11-04 22:29 UTC (permalink / raw) To: Laxman Dewangan Cc: thierry.reding, Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter, Masahiro Yamada, linux-gpio, devicetree, linux-tegra, linux-kernel 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. > > Add DT binding document for detailing the DT properties for > configuring IO pads voltage levels and its power state. > > Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> (...) > +-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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads 2016-11-02 9:09 [PATCH 0/2] pinctrl: tegra: Add support for IO pad control Laxman Dewangan 2016-11-02 9:09 ` [PATCH 1/2] pinctrl: tegra: Add DT binding for io pads control Laxman Dewangan @ 2016-11-02 9:09 ` Laxman Dewangan 2016-11-02 10:49 ` kbuild test robot 2016-11-04 22:24 ` Linus Walleij 1 sibling, 2 replies; 15+ messages in thread From: Laxman Dewangan @ 2016-11-02 9:09 UTC (permalink / raw) To: thierry.reding, swarren, linus.walleij, robh+dt, mark.rutland Cc: jonathanh, yamada.masahiro, linux-gpio, devicetree, linux-tegra, linux-kernel, Laxman Dewangan 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads 2016-11-02 9:09 ` [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads Laxman Dewangan @ 2016-11-02 10:49 ` kbuild test robot 2016-11-04 22:24 ` Linus Walleij 1 sibling, 0 replies; 15+ messages in thread From: kbuild test robot @ 2016-11-02 10:49 UTC (permalink / raw) To: Laxman Dewangan Cc: kbuild-all, thierry.reding, swarren, linus.walleij, robh+dt, mark.rutland, jonathanh, yamada.masahiro, linux-gpio, devicetree, linux-tegra, linux-kernel, Laxman Dewangan [-- Attachment #1: Type: text/plain, Size: 12563 bytes --] 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 [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 58490 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads 2016-11-02 9:09 ` [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads Laxman Dewangan 2016-11-02 10:49 ` kbuild test robot @ 2016-11-04 22:24 ` Linus Walleij 2016-11-07 5:41 ` Laxman Dewangan 1 sibling, 1 reply; 15+ messages in thread From: Linus Walleij @ 2016-11-04 22:24 UTC (permalink / raw) To: Laxman Dewangan Cc: thierry.reding, Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter, Masahiro Yamada, linux-gpio, devicetree, linux-tegra, linux-kernel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads 2016-11-04 22:24 ` Linus Walleij @ 2016-11-07 5:41 ` Laxman Dewangan 2016-11-08 10:15 ` Linus Walleij 0 siblings, 1 reply; 15+ messages in thread From: Laxman Dewangan @ 2016-11-07 5:41 UTC (permalink / raw) To: Linus Walleij Cc: thierry.reding, Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter, Masahiro Yamada, linux-gpio, devicetree, linux-tegra, linux-kernel 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads 2016-11-07 5:41 ` Laxman Dewangan @ 2016-11-08 10:15 ` Linus Walleij 2016-11-08 10:20 ` Laxman Dewangan 0 siblings, 1 reply; 15+ messages in thread From: Linus Walleij @ 2016-11-08 10:15 UTC (permalink / raw) To: Laxman Dewangan Cc: thierry.reding, Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter, Masahiro Yamada, linux-gpio, devicetree, linux-tegra, linux-kernel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads 2016-11-08 10:15 ` Linus Walleij @ 2016-11-08 10:20 ` Laxman Dewangan 2016-11-08 13:29 ` Linus Walleij 0 siblings, 1 reply; 15+ messages in thread From: Laxman Dewangan @ 2016-11-08 10:20 UTC (permalink / raw) To: Linus Walleij Cc: thierry.reding, Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter, Masahiro Yamada, linux-gpio, devicetree, linux-tegra, linux-kernel 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads 2016-11-08 10:20 ` Laxman Dewangan @ 2016-11-08 13:29 ` Linus Walleij 2016-11-08 13:35 ` Laxman Dewangan 0 siblings, 1 reply; 15+ messages in thread From: Linus Walleij @ 2016-11-08 13:29 UTC (permalink / raw) To: Laxman Dewangan Cc: thierry.reding, Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter, Masahiro Yamada, linux-gpio, devicetree, linux-tegra, linux-kernel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads 2016-11-08 13:29 ` Linus Walleij @ 2016-11-08 13:35 ` Laxman Dewangan 2016-11-08 14:42 ` Thierry Reding 2016-11-08 15:46 ` Linus Walleij 0 siblings, 2 replies; 15+ messages in thread From: Laxman Dewangan @ 2016-11-08 13:35 UTC (permalink / raw) To: Linus Walleij Cc: thierry.reding, Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter, Masahiro Yamada, linux-gpio, devicetree, linux-tegra, linux-kernel 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? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads 2016-11-08 13:35 ` Laxman Dewangan @ 2016-11-08 14:42 ` Thierry Reding 2016-11-08 15:45 ` Laxman Dewangan 2016-11-08 15:46 ` Linus Walleij 1 sibling, 1 reply; 15+ messages in thread From: Thierry Reding @ 2016-11-08 14:42 UTC (permalink / raw) To: Laxman Dewangan Cc: Linus Walleij, Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter, Masahiro Yamada, linux-gpio, devicetree, linux-tegra, linux-kernel [-- Attachment #1: Type: text/plain, Size: 4950 bytes --] 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads 2016-11-08 14:42 ` Thierry Reding @ 2016-11-08 15:45 ` Laxman Dewangan 0 siblings, 0 replies; 15+ messages in thread From: Laxman Dewangan @ 2016-11-08 15:45 UTC (permalink / raw) To: Thierry Reding Cc: Linus Walleij, Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter, Masahiro Yamada, linux-gpio, devicetree, linux-tegra, linux-kernel 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads 2016-11-08 13:35 ` Laxman Dewangan 2016-11-08 14:42 ` Thierry Reding @ 2016-11-08 15:46 ` Linus Walleij 2016-11-08 15:48 ` Laxman Dewangan 1 sibling, 1 reply; 15+ messages in thread From: Linus Walleij @ 2016-11-08 15:46 UTC (permalink / raw) To: Laxman Dewangan Cc: thierry.reding, Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter, Masahiro Yamada, linux-gpio, devicetree, linux-tegra, linux-kernel 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads 2016-11-08 15:46 ` Linus Walleij @ 2016-11-08 15:48 ` Laxman Dewangan 0 siblings, 0 replies; 15+ messages in thread From: Laxman Dewangan @ 2016-11-08 15:48 UTC (permalink / raw) To: Linus Walleij Cc: thierry.reding, Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter, Masahiro Yamada, linux-gpio, devicetree, linux-tegra, linux-kernel 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-11-08 16:05 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-02 9:09 [PATCH 0/2] pinctrl: tegra: Add support for IO pad control Laxman Dewangan 2016-11-02 9:09 ` [PATCH 1/2] pinctrl: tegra: Add DT binding for io pads control Laxman Dewangan 2016-11-04 22:29 ` Linus Walleij 2016-11-02 9:09 ` [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads Laxman Dewangan 2016-11-02 10:49 ` kbuild test robot 2016-11-04 22:24 ` Linus Walleij 2016-11-07 5:41 ` Laxman Dewangan 2016-11-08 10:15 ` Linus Walleij 2016-11-08 10:20 ` Laxman Dewangan 2016-11-08 13:29 ` Linus Walleij 2016-11-08 13:35 ` Laxman Dewangan 2016-11-08 14:42 ` Thierry Reding 2016-11-08 15:45 ` Laxman Dewangan 2016-11-08 15:46 ` Linus Walleij 2016-11-08 15:48 ` Laxman Dewangan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).