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

Groups > linux.kernel > #1376899 > unrolled thread

[PATCH 0/7] pinctrl: soc/tegra: Add support to configure IO rail voltage and pad power states

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] pinctrl: soc/tegra: Add support to configure IO rail voltage and pad power states Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-12 17:20 +0200
    [PATCH 2/7] soc/tegra: pmc: Add new Tegra210 IO rails Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-12 17:20 +0200
      Re: [PATCH 2/7] soc/tegra: pmc: Add new Tegra210 IO rails Thierry Reding <thierry.reding@gmail.com> - 2016-04-12 17:30 +0200
        Re: [PATCH 2/7] soc/tegra: pmc: Add new Tegra210 IO rails Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-12 19:20 +0200
          Re: [PATCH 2/7] soc/tegra: pmc: Add new Tegra210 IO rails Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-12 20:10 +0200
          Re: [PATCH 2/7] soc/tegra: pmc: Add new Tegra210 IO rails Jon Hunter <jonathanh@nvidia.com> - 2016-04-12 20:10 +0200
    [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-12 17:20 +0200
      Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Jon Hunter <jonathanh@nvidia.com> - 2016-04-13 11:10 +0200
        Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-13 11:20 +0200
          Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Jon Hunter <jonathanh@nvidia.com> - 2016-04-13 11:40 +0200
      Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Jon Hunter <jonathanh@nvidia.com> - 2016-04-15 16:20 +0200
        Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-15 16:30 +0200
          Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Jon Hunter <jonathanh@nvidia.com> - 2016-04-15 17:20 +0200
            Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-15 17:30 +0200
              Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Jon Hunter <jonathanh@nvidia.com> - 2016-04-15 17:50 +0200
                Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-15 19:00 +0200
                  Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Jon Hunter <jonathanh@nvidia.com> - 2016-04-15 19:50 +0200
                    Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-15 20:10 +0200
                      Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Jon Hunter <jonathanh@nvidia.com> - 2016-04-15 20:40 +0200
                        Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-15 21:00 +0200
      Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Stephen Warren <swarren@wwwdotorg.org> - 2016-04-15 18:40 +0200
        Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-15 18:50 +0200
    [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and power state of io pads Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-12 17:20 +0200
      Re: [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and  power state of io pads Linus Walleij <linus.walleij@linaro.org> - 2016-04-15 10:10 +0200
        Re: [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and  power state of io pads Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-15 11:00 +0200
          Re: [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and  power state of io pads Linus Walleij <linus.walleij@linaro.org> - 2016-04-15 11:30 +0200
            Re: [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and  power state of io pads Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-15 12:10 +0200
              Re: [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and  power state of io pads Linus Walleij <linus.walleij@linaro.org> - 2016-04-15 13:20 +0200
                Re: [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and  power state of io pads Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-15 14:00 +0200
                  Re: [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and  power state of io pads Linus Walleij <linus.walleij@linaro.org> - 2016-04-15 16:10 +0200
                    Re: [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and  power state of io pads Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-15 16:20 +0200
                      Re: [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and  power state of io pads Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-19 12:10 +0200
                  Re: [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and  power state of io pads Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-26 15:50 +0200
                    Re: [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and  power state of io pads Stephen Warren <swarren@wwwdotorg.org> - 2016-04-26 17:40 +0200
          Re: [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and  power state of io pads Stephen Warren <swarren@wwwdotorg.org> - 2016-04-15 18:40 +0200
    [PATCH 1/7] soc/tegra: pmc: Use BIT macro for register field definition Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-12 17:20 +0200
      Re: [PATCH 1/7] soc/tegra: pmc: Use BIT macro for register field  definition Thierry Reding <thierry.reding@gmail.com> - 2016-04-12 17:30 +0200
        Re: [PATCH 1/7] soc/tegra: pmc: Use BIT macro for register field  definition Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-12 19:10 +0200
        Re: [PATCH 1/7] soc/tegra: pmc: Use BIT macro for register field definition Linus Walleij <linus.walleij@linaro.org> - 2016-04-15 09:50 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1379996 — Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control

On 04/12/2016 08:56 AM, Laxman Dewangan wrote:
> NVIDIA Tegra210 supports the IO pads which can operate at 1.8V
> or 3.3V I/O voltage levels. Also IO pads can be configured for
> power down state if it is not in used. SW needs to configure the
> voltage level of IO pads based on IO rail voltage and its power
> state based on platform usage.
>
> Add DT binding document for detailing the DT properties for
> configuring IO pads voltage levels and its power state.

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

All static settings, and good boot defaults, should be set up by system 
FW. Perhaps not all FW does this on Tegra210 platforms:-( I hope that on 
future chips, the same FW that sets up the static pinmux sets up the 
static IO voltage configuration, in exactly the same way.

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

#1380004 — Re: [PATCH 6/7] pinctrl: tegra: Add DT binding for io pads control

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

Exactly yes.

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

#1376906 — [PATCH 7/7] pinctrl: tegra: Add driver to configure voltage and power state of io pads

NVIDIA Tegra210 supports the IO pads which can operate at 1.8V
or 3.3V I/O voltage levels. Also the IO pads can be configured
for power down state if it is not used. SW needs to configure the
voltage level of IO pads based on IO rail voltage and its power
state based on platform usage.

The voltage and power state configurations of pads are provided
through pin control frameworks. Add pin control driver for Tegra's
IO pads' voltage and power state configurations.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/pinctrl/tegra/Kconfig                   |  11 +
 drivers/pinctrl/tegra/Makefile                  |   1 +
 drivers/pinctrl/tegra/pinctrl-tegra210-io-pad.c | 302 ++++++++++++++++++++++++
 3 files changed, 314 insertions(+)
 create mode 100644 drivers/pinctrl/tegra/pinctrl-tegra210-io-pad.c

diff --git a/drivers/pinctrl/tegra/Kconfig b/drivers/pinctrl/tegra/Kconfig
index 24e20cc..37b85d7 100644
--- a/drivers/pinctrl/tegra/Kconfig
+++ b/drivers/pinctrl/tegra/Kconfig
@@ -23,6 +23,17 @@ config PINCTRL_TEGRA210
 	bool
 	select PINCTRL_TEGRA
 
+config PINCTRL_TEGRA210_IO_PAD
+	bool "Tegra210 IO pad Control Driver"
+	depends on ARCH_TEGRA
+	select PINCONF
+	select PINMUX
+	help
+	  NVIDIA Tegra210 SoC has IO pads which supports mult-voltage level
+	  of interfacing. The voltage of IO pads are SW configurable based
+	  on IO rail of that pads. 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 a927379..90f4000 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_TEGRA210_IO_PAD)	+= pinctrl-tegra210-io-pad.o
 obj-$(CONFIG_PINCTRL_TEGRA_XUSB)	+= pinctrl-tegra-xusb.o
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra210-io-pad.c b/drivers/pinctrl/tegra/pinctrl-tegra210-io-pad.c
new file mode 100644
index 0000000..4959c19
--- /dev/null
+++ b/drivers/pinctrl/tegra/pinctrl-tegra210-io-pad.c
@@ -0,0 +1,302 @@
+/*
+ * Generic ADC thermal driver
+ *
+ * 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_RAIL_VOLTAGE = PIN_CONFIG_END + 1,
+	TEGRA_IO_PAD_DEEP_POWER_DOWN,
+};
+
+static const struct pinconf_generic_params tegra_io_pads_cfg_params[] = {
+	{
+		.property = "nvidia,io-rail-voltage",
+		.param = TEGRA_IO_RAIL_VOLTAGE,
+	}, {
+		.property = "nvidia,io-pad-deep-power-down",
+		.param = TEGRA_IO_PAD_DEEP_POWER_DOWN,
+	},
+};
+
+struct tegra_io_pads_cfg_info {
+	const char *name;
+	const unsigned int pins[1];
+	int io_rail_id;
+};
+
+#define TEGRA210_PAD_INFO_TABLE(_entry_)		\
+	_entry_(0, "audio", AUDIO),			\
+	_entry_(1, "audio-hv", AUDIO_HV),	\
+	_entry_(2, "cam", CAM),			\
+	_entry_(3, "csia", CSIA),			\
+	_entry_(4, "csib", CSIB),			\
+	_entry_(5, "csic", CSIC),			\
+	_entry_(6, "csid", CSID),			\
+	_entry_(7, "csie", CSIE),			\
+	_entry_(8, "csif", CSIF),			\
+	_entry_(9, "dbg", DBG),			\
+	_entry_(10, "debug-nonao", DBG_NONAO), \
+	_entry_(11, "dmic", DMIC),			\
+	_entry_(12, "dp", DP),				\
+	_entry_(13, "dsi", DSI),			\
+	_entry_(14, "dsib", DSIB),			\
+	_entry_(15, "dsic", DSIC),			\
+	_entry_(16, "dsid", DSID),			\
+	_entry_(17, "emmc", SDMMC4),			\
+	_entry_(18, "emmc2", EMMC2),			\
+	_entry_(19, "gpio", GPIO),			\
+	_entry_(20, "hdmi", HDMI),			\
+	_entry_(21, "hsic", HSIC),			\
+	_entry_(22, "lvds", LVDS),			\
+	_entry_(23, "mipi-bias", MIPI_BIAS),	\
+	_entry_(24, "pex-bias", PEX_BIAS),	\
+	_entry_(25, "pex-clk1", PEX_CLK1),	\
+	_entry_(26, "pex-clk2", PEX_CLK2),	\
+	_entry_(27, "pex-ctrl", PEX_CNTRL),	\
+	_entry_(28, "sdmmc1", SDMMC1),		\
+	_entry_(29, "sdmmc3", SDMMC3),		\
+	_entry_(30, "spi", SPI),			\
+	_entry_(31, "spi-hv", SPI_HV),		\
+	_entry_(32, "uart", UART),			\
+	_entry_(33, "usb-bias", USB_BIAS),	\
+	_entry_(34, "usb0", USB0),			\
+	_entry_(35, "usb1", USB1),			\
+	_entry_(36, "usb2", USB2),			\
+	_entry_(37, "usb3", USB3)
+
+#define TEGRA_IO_PAD_INFO(_id, _name, _io_rail_id)			\
+	{								\
+		.name = _name,						\
+		.pins = {(_id)},					\
+		.io_rail_id = TEGRA_IO_RAIL_##_io_rail_id,		\
+	}
+
+static 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, _io_rail_id)			\
+	PINCTRL_PIN(_id, _name)
+
+static const struct pinctrl_pin_desc tegra210_io_pads_pinctrl_desc[] = {
+	TEGRA210_PAD_INFO_TABLE(TEGRA_IO_PAD_DESC),
+};
+
+struct tegra_io_pads_info {
+	struct device *dev;
+	struct pinctrl_dev *pctl;
+	struct tegra_io_pads_cfg_info *pads_cfg;
+	unsigned int num_pads_cfg;
+};
+
+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->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->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->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_dt_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);
+	struct tegra_io_pads_cfg_info *pad_cfg = &tiopi->pads_cfg[pin];
+	int io_rail_id = pad_cfg->io_rail_id;
+	int arg = 0;
+	int ret;
+
+	switch (param) {
+	case TEGRA_IO_RAIL_VOLTAGE:
+		ret = tegra_io_rail_voltage_get(io_rail_id);
+		if (ret < 0)
+			return ret;
+		arg = ret;
+		break;
+
+	case TEGRA_IO_PAD_DEEP_POWER_DOWN:
+		ret = tegra_io_rail_power_get_status(io_rail_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);
+	struct tegra_io_pads_cfg_info *pad_cfg = &tiopi->pads_cfg[pin];
+	int io_rail_id = pad_cfg->io_rail_id;
+	int param;
+	u16 param_val;
+	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_RAIL_VOLTAGE:
+			ret = tegra_io_rail_voltage_set(io_rail_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 TEGRA_IO_PAD_DEEP_POWER_DOWN:
+			if (param_val)
+				ret = tegra_io_rail_power_off(io_rail_id);
+			else
+				ret = tegra_io_rail_power_on(io_rail_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,
+	.pins = tegra210_io_pads_pinctrl_desc,
+	.npins = ARRAY_SIZE(tegra210_io_pads_pinctrl_desc),
+	.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 tegra_io_pads_info *tiopi;
+	struct device *dev = &pdev->dev;
+
+	tiopi = devm_kzalloc(&pdev->dev, sizeof(*tiopi), GFP_KERNEL);
+	if (!tiopi)
+		return -ENOMEM;
+
+	tiopi->dev = &pdev->dev;
+	tiopi->dev->of_node = pdev->dev.of_node;
+	tiopi->pads_cfg = tegra210_io_pads_cfg_info;
+	tiopi->num_pads_cfg = ARRAY_SIZE(tegra210_io_pads_cfg_info);
+
+	platform_set_drvdata(pdev, tiopi);
+
+	tiopi->pctl = pinctrl_register(&tegra_iop_pinctrl_desc, dev, tiopi);
+	if (IS_ERR(tiopi->pctl)) {
+		dev_err(&pdev->dev, "Couldn't register pinctrl driver\n");
+		return PTR_ERR(tiopi->pctl);
+	}
+
+	return 0;
+}
+
+static int tegra_iop_pinctrl_remove(struct platform_device *pdev)
+{
+	struct tegra_io_pads_info *tiopi = platform_get_drvdata(pdev);
+
+	pinctrl_unregister(tiopi->pctl);
+
+	return 0;
+}
+
+static const struct of_device_id tegra_io_pads_of_match[] = {
+	{ .compatible = "nvidia,tegra210-io-pad", },
+	{},
+};
+MODULE_DEVICE_TABLE(platform, tegra_iop_pinctrl_devtype);
+
+static struct platform_driver tegra_iop_pinctrl_driver = {
+	.driver = {
+		.name = "pinctrl-tegra-io-pad",
+		.of_match_table = tegra_io_pads_of_match,
+	},
+	.probe = tegra_iop_pinctrl_probe,
+	.remove = tegra_iop_pinctrl_remove,
+};
+
+module_platform_driver(tegra_iop_pinctrl_driver);
+
+MODULE_DESCRIPTION("NVIDIA TEGRA IO pad Control Driver");
+MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
+MODULE_ALIAS("platform:pinctrl-tegra-io-pads");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

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

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

On Tue, Apr 12, 2016 at 4:56 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:

> NVIDIA Tegra210 supports the IO pads which can operate at 1.8V
> or 3.3V I/O voltage levels. Also the IO pads can be configured
> for power down state if it is not used. SW needs to configure the
> voltage level of IO pads based on IO rail voltage and its power
> state based on platform usage.
>
> The voltage and power state configurations of pads are provided
> through pin control frameworks. Add pin control driver for Tegra's
> IO pads' voltage and power state configurations.
>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

(...)
> +config PINCTRL_TEGRA210_IO_PAD

Why does this need its own Kconfig option?
Can't you just unconditionally compile it in if
PINCTRL_TEGRA210 is selected, you seem to say
it is there on all these platforms anyway.

> +static const struct pinconf_generic_params tegra_io_pads_cfg_params[] = {
> +       {
> +               .property = "nvidia,io-rail-voltage",
> +               .param = TEGRA_IO_RAIL_VOLTAGE,
> +       }, {

What's so nvidia-specific about this?
We have power-source in
Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
which takes a custom argument. This is obviously what you
are doing (selecting one of two rails), so use that binding.

> +               .property = "nvidia,io-pad-deep-power-down",
> +               .param = TEGRA_IO_PAD_DEEP_POWER_DOWN,
> +       },

Likewise the generic bindings have low-power-enable and
low-power-disable, this seems like a copy of low-power-enable;
Even if it needs a new binding, it doesn't seem very nVidia-specific
so then propose something to the generic bindings.

Even if Tegra is not using the generic code for handling the
standard bindings (GENERIC_PINCONF) it doesn't stop
you from using the generic bindings and contributing to them.

Historically you have a few custom bindings like these:

nvidia,pins
nvidia,function
nvidia,pull
nvidia,tristate

etc etc, but that is just unfortunate and due to preceding the
generic bindings. I would appreciate if you started to support
the generic bindings in parallel, but I'm not gonna push that issue.
However for *new* stuff, I don't want the custom bindings
to proliferate. Use the generic stuff, I'm trying to keep the weirdness
in one place, and the generic stuff is needed for standardization
across platforms going forward.

Yours,
Linus Walleij

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

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

On Friday 15 April 2016 01:38 PM, Linus Walleij wrote:
> On Tue, Apr 12, 2016 at 4:56 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>
>> NVIDIA Tegra210 supports the IO pads which can operate at 1.8V
>> or 3.3V I/O voltage levels. Also the IO pads can be configured
>> for power down state if it is not used. SW needs to configure the
>> voltage level of IO pads based on IO rail voltage and its power
>> state based on platform usage.
>>
>> The voltage and power state configurations of pads are provided
>> through pin control frameworks. Add pin control driver for Tegra's
>> IO pads' voltage and power state configurations.
>>
>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> (...)
>> +config PINCTRL_TEGRA210_IO_PAD
> Why does this need its own Kconfig option?
> Can't you just unconditionally compile it in if
> PINCTRL_TEGRA210 is selected, you seem to say
> it is there on all these platforms anyway.

Yes, it can be done. The reason I kept is that this driver needed T210 
onwards and not for older generation of SoC.

May be we can select from T210 pincontrol.


>
>> +static const struct pinconf_generic_params tegra_io_pads_cfg_params[] = {
>> +       {
>> +               .property = "nvidia,io-rail-voltage",
>> +               .param = TEGRA_IO_RAIL_VOLTAGE,
>> +       }, {
> What's so nvidia-specific about this?
> We have power-source in
> Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> which takes a custom argument. This is obviously what you
> are doing (selecting one of two rails), so use that binding.

Yes, I looked for the common property but did not found anything near to 
this.
My understating for power-source is that selecting the source of supply, 
not the voltages.
I am looking something  power-source-voltage-level.
Should we add this?




>
>> +               .property = "nvidia,io-pad-deep-power-down",
>> +               .param = TEGRA_IO_PAD_DEEP_POWER_DOWN,
>> +       },
> Likewise the generic bindings have low-power-enable and
> low-power-disable, this seems like a copy of low-power-enable;
When writing, I considered this property but was not able to fully 
convinced myself to use this but I think now I am fine to use this as 
you suggested.



>
> Even if Tegra is not using the generic code for handling the
> standard bindings (GENERIC_PINCONF) it doesn't stop
> you from using the generic bindings and contributing to them.
>
> Historically you have a few custom bindings like these:
>
> nvidia,pins
> nvidia,function
> nvidia,pull
> nvidia,tristate
>
> etc etc, but that is just unfortunate and due to preceding the
> generic bindings. I would appreciate if you started to support
> the generic bindings in parallel, but I'm not gonna push that issue.

Yaah, these are in my plate to cleanup. Let me work with Stephen, what 
he think here.

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

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

On Fri, Apr 15, 2016 at 10:39 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> On Friday 15 April 2016 01:38 PM, Linus Walleij wrote:
>> On Tue, Apr 12, 2016 at 4:56 PM, Laxman Dewangan <ldewangan@nvidia.com>
>> wrote:

>>> +static const struct pinconf_generic_params tegra_io_pads_cfg_params[] =
>>> {
>>> +       {
>>> +               .property = "nvidia,io-rail-voltage",
>>> +               .param = TEGRA_IO_RAIL_VOLTAGE,
>>> +       }, {
>>
>> What's so nvidia-specific about this?
>> We have power-source in
>> Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>> which takes a custom argument. This is obviously what you
>> are doing (selecting one of two rails), so use that binding.
>
> Yes, I looked for the common property but did not found anything near to
> this.
> My understating for power-source is that selecting the source of supply, not
> the voltages.

Well in a comment to the previous patch you just said that the
hardware actually does not regulate voltages. Isn't the actual case
that there are two rails with two different voltages, and you select one
of the rails for the pin?

That is not really selecting a voltage, that is selecting a power
rail.

> I am looking something  power-source-voltage-level.
> Should we add this?

If the pin could actually set a voltage level it would have a regulator.
I don't believe that. I think it is selecting one of two rails which
could theoretically hold two totally different voltages.

And that is what power-source is about.

>>> +               .property = "nvidia,io-pad-deep-power-down",
>>> +               .param = TEGRA_IO_PAD_DEEP_POWER_DOWN,
>>> +       },
>>
>> Likewise the generic bindings have low-power-enable and
>> low-power-disable, this seems like a copy of low-power-enable;
>
> When writing, I considered this property but was not able to fully convinced
> myself to use this but I think now I am fine to use this as you suggested.

Thanks.

>> Even if Tegra is not using the generic code for handling the
>> standard bindings (GENERIC_PINCONF) it doesn't stop
>> you from using the generic bindings and contributing to them.
>>
>> Historically you have a few custom bindings like these:
>>
>> nvidia,pins
>> nvidia,function
>> nvidia,pull
>> nvidia,tristate
>>
>> etc etc, but that is just unfortunate and due to preceding the
>> generic bindings. I would appreciate if you started to support
>> the generic bindings in parallel, but I'm not gonna push that issue.
>
> Yaah, these are in my plate to cleanup. Let me work with Stephen, what he
> think here.

Much appreciated, thanks!

Yours,
Linus Walleij

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

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

On Friday 15 April 2016 02:55 PM, Linus Walleij wrote:
> On Fri, Apr 15, 2016 at 10:39 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>> On Friday 15 April 2016 01:38 PM, Linus Walleij wrote:
>>> On Tue, Apr 12, 2016 at 4:56 PM, Laxman Dewangan <ldewangan@nvidia.com>
>>> wrote:
>>>> +static const struct pinconf_generic_params tegra_io_pads_cfg_params[] =
>>>> {
>>>> +       {
>>>> +               .property = "nvidia,io-rail-voltage",
>>>> +               .param = TEGRA_IO_RAIL_VOLTAGE,
>>>> +       }, {
>>> What's so nvidia-specific about this?
>>> We have power-source in
>>> Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>>> which takes a custom argument. This is obviously what you
>>> are doing (selecting one of two rails), so use that binding.
>> Yes, I looked for the common property but did not found anything near to
>> this.
>> My understating for power-source is that selecting the source of supply, not
>> the voltages.
> Well in a comment to the previous patch you just said that the
> hardware actually does not regulate voltages. Isn't the actual case
> that there are two rails with two different voltages, and you select one
> of the rails for the pin?
>
> That is not really selecting a voltage, that is selecting a power
> rail.
>
>> I am looking something  power-source-voltage-level.
>> Should we add this?
> If the pin could actually set a voltage level it would have a regulator.
> I don't believe that. I think it is selecting one of two rails which
> could theoretically hold two totally different voltages.
>
> And that is what power-source is about.
>
>


The IO rails connected to PMIC rail and connection does not get change.
We change the voltage of PMIC rails via regulator calls. And then 
configure pads for the new voltage.
We really do not witch the supply connection here. power-source is about 
the power supply connection, not about the voltage change on same supply.

Like in MMC driver for SDIO3.0:

regulator_set_voltage(sdmmc_rail, 3300000, 3300000);
tegra_io_pad_voltage_configure(SDMMC, 3300000)


For 1.8V

regulator_set_voltage(sdmmc_rail, 1800000, 1800000);
tegra_io_pad_voltage_configure(SDMMC, 1800000)

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

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

On Fri, Apr 15, 2016 at 11:55 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> On Friday 15 April 2016 02:55 PM, Linus Walleij wrote:

>> If the pin could actually set a voltage level it would have a regulator.
>> I don't believe that. I think it is selecting one of two rails which
>> could theoretically hold two totally different voltages.
>>
>> And that is what power-source is about.
>
> The IO rails connected to PMIC rail and connection does not get change.
> We change the voltage of PMIC rails via regulator calls. And then configure
> pads for the new voltage.

Aha I get it! So you adjust something in the I/O-cell so that it is adapted
for the new voltage.

OK that seems to be something new. I suspect
power-voltage-select = <n>; where N i in uV would solve this?
(We should use uV since regulators use this.)

But to be sure we would like to know what is actually happening,
electronically speaking, when you set this up. Do you have any
idea?

Yours,
Linus Walleij

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

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

On Friday 15 April 2016 04:45 PM, Linus Walleij wrote:
> On Fri, Apr 15, 2016 at 11:55 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>> On Friday 15 April 2016 02:55 PM, Linus Walleij wrote:
>>> If the pin could actually set a voltage level it would have a regulator.
>>> I don't believe that. I think it is selecting one of two rails which
>>> could theoretically hold two totally different voltages.
>>>
>>> And that is what power-source is about.
>> The IO rails connected to PMIC rail and connection does not get change.
>> We change the voltage of PMIC rails via regulator calls. And then configure
>> pads for the new voltage.
> Aha I get it! So you adjust something in the I/O-cell so that it is adapted
> for the new voltage.
>
> OK that seems to be something new. I suspect
> power-voltage-select = <n>; where N i in uV would solve this?
> (We should use uV since regulators use this.)

Thanks for new property. I will make the unit and type same as the 
regulator framework.



>
> But to be sure we would like to know what is actually happening,
> electronically speaking, when you set this up. Do you have any
> idea?
>


 From electronic point of view, the value of VIL, VIH, VOL, VOH 
(Input/output voltage level for low and high state) are different when 
talking for 0 t 1.8V and 0 to 3.3V.

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

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

On Fri, Apr 15, 2016 at 1:47 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> On Friday 15 April 2016 04:45 PM, Linus Walleij wrote:
>> On Fri, Apr 15, 2016 at 11:55 AM, Laxman Dewangan <ldewangan@nvidia.com>
>> wrote:

>> But to be sure we would like to know what is actually happening,
>> electronically speaking, when you set this up. Do you have any
>> idea?
>
> From electronic point of view, the value of VIL, VIH, VOL, VOH (Input/output
> voltage level for low and high state) are different when talking for 0 t
> 1.8V and 0 to 3.3V.

Yeah that I get. But since it is switched on a per-pin basis, and
this is not about what voltage is actually supplied to the I/O cell,
because that comes from the outside, it is a mystery why it is
even needed.

I understand that there is a bit selecting driving voltage level in
the register range, what I don't understand is what that is
doing in the I/O cell.

The bit in the register must be routed to somehing in the I/O cell
and I would like to know what. I take it that an ordinary CMOS
totem-pole push-pull output is going to work the same with 1.8
and 3.3V alike so it's obviously not enabling any extra transistors
or anything.

Yours,
Linus Walleij

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

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

On Friday 15 April 2016 07:33 PM, Linus Walleij wrote:
> On Fri, Apr 15, 2016 at 1:47 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
>> On Friday 15 April 2016 04:45 PM, Linus Walleij wrote:
>>> On Fri, Apr 15, 2016 at 11:55 AM, Laxman Dewangan <ldewangan@nvidia.com>
>>> wrote:
>>> But to be sure we would like to know what is actually happening,
>>> electronically speaking, when you set this up. Do you have any
>>> idea?
>>  From electronic point of view, the value of VIL, VIH, VOL, VOH (Input/output
>> voltage level for low and high state) are different when talking for 0 t
>> 1.8V and 0 to 3.3V.
> Yeah that I get. But since it is switched on a per-pin basis, and
> this is not about what voltage is actually supplied to the I/O cell,
> because that comes from the outside, it is a mystery why it is
> even needed.
>
> I understand that there is a bit selecting driving voltage level in
> the register range, what I don't understand is what that is
> doing in the I/O cell.
>
> The bit in the register must be routed to somehing in the I/O cell
> and I would like to know what. I take it that an ordinary CMOS
> totem-pole push-pull output is going to work the same with 1.8
> and 3.3V alike so it's obviously not enabling any extra transistors
> or anything.
>
>
I dont have answer for this now and I need to discuss with HW team to 
get this info.

I will be back here after discussion with HW team.

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

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

Hi Linus,

On Friday 15 April 2016 07:29 PM, Laxman Dewangan wrote:
>
> On Friday 15 April 2016 07:33 PM, Linus Walleij wrote:
>> On Fri, Apr 15, 2016 at 1:47 PM, Laxman Dewangan 
>> <ldewangan@nvidia.com> wrote:
>>> On Friday 15 April 2016 04:45 PM, Linus Walleij wrote:
>>>> On Fri, Apr 15, 2016 at 11:55 AM, Laxman Dewangan 
>>>> <ldewangan@nvidia.com>
>>>> wrote:
>>>> But to be sure we would like to know what is actually happening,
>>>> electronically speaking, when you set this up. Do you have any
>>>> idea?
>>>  From electronic point of view, the value of VIL, VIH, VOL, VOH 
>>> (Input/output
>>> voltage level for low and high state) are different when talking for 
>>> 0 t
>>> 1.8V and 0 to 3.3V.
>> Yeah that I get. But since it is switched on a per-pin basis, and
>> this is not about what voltage is actually supplied to the I/O cell,
>> because that comes from the outside, it is a mystery why it is
>> even needed.
>>
>> I understand that there is a bit selecting driving voltage level in
>> the register range, what I don't understand is what that is
>> doing in the I/O cell.
>>
>> The bit in the register must be routed to somehing in the I/O cell
>> and I would like to know what. I take it that an ordinary CMOS
>> totem-pole push-pull output is going to work the same with 1.8
>> and 3.3V alike so it's obviously not enabling any extra transistors
>> or anything.
>>
>>
> I dont have answer for this now and I need to discuss with HW team to 
> get this info.
>
> I will be back here after discussion with HW team.
>
I had discussion with HW team to get this info from analog point of view.

The IO circuitry has to be configured correctly to engage the right 
level shifting circuits between the IO rail and the core voltage rail in 
each direction; and that is the main purpose of this configuration. You 
are correct, the output drivers will naturally drive towards the rails, 
whatever their voltage may be; and the input receiver will likewise 
reference itself naturally to the rail, although the switching threshold 
of the receiver transistors may sometimes need configuration too.

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

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

On Friday 15 April 2016 05:17 PM, Laxman Dewangan wrote:
>
> On Friday 15 April 2016 04:45 PM, Linus Walleij wrote:
>> On Fri, Apr 15, 2016 at 11:55 AM, Laxman Dewangan 
>> <ldewangan@nvidia.com> wrote:
>>> On Friday 15 April 2016 02:55 PM, Linus Walleij wrote:
>>>> If the pin could actually set a voltage level it would have a 
>>>> regulator.
>>>> I don't believe that. I think it is selecting one of two rails which
>>>> could theoretically hold two totally different voltages.
>>>>
>>>> And that is what power-source is about.
>>> The IO rails connected to PMIC rail and connection does not get change.
>>> We change the voltage of PMIC rails via regulator calls. And then 
>>> configure
>>> pads for the new voltage.
>> Aha I get it! So you adjust something in the I/O-cell so that it is 
>> adapted
>> for the new voltage.
>>
>> OK that seems to be something new. I suspect
>> power-voltage-select = <n>; where N i in uV would solve this?
>> (We should use uV since regulators use this.)
>
> Thanks for new property. I will make the unit and type same as the 
> regulator framework.

We have the ops for configuring the pin config as

         int (*pin_config_group_set) (struct pinctrl_dev *pctldev,
                                      unsigned selector,
                                      unsigned long *configs,
                                      unsigned num_configs);


The config is 32 bit, upper 16 for config argument and  and lower 16 for 
the config param.

So we can not accommodate 3300000uV until we change it to mV i.e. 3300mV.

So on interface, we can read uV from DT but when making config, we can 
translate it to mV before passing to pin_config_group_set.

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

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

On 04/26/2016 07:32 AM, Laxman Dewangan wrote:
>
> On Friday 15 April 2016 05:17 PM, Laxman Dewangan wrote:
>>
>> On Friday 15 April 2016 04:45 PM, Linus Walleij wrote:
>>> On Fri, Apr 15, 2016 at 11:55 AM, Laxman Dewangan
>>> <ldewangan@nvidia.com> wrote:
>>>> On Friday 15 April 2016 02:55 PM, Linus Walleij wrote:
>>>>> If the pin could actually set a voltage level it would have a
>>>>> regulator.
>>>>> I don't believe that. I think it is selecting one of two rails which
>>>>> could theoretically hold two totally different voltages.
>>>>>
>>>>> And that is what power-source is about.
>>>> The IO rails connected to PMIC rail and connection does not get change.
>>>> We change the voltage of PMIC rails via regulator calls. And then
>>>> configure
>>>> pads for the new voltage.
>>> Aha I get it! So you adjust something in the I/O-cell so that it is
>>> adapted
>>> for the new voltage.
>>>
>>> OK that seems to be something new. I suspect
>>> power-voltage-select = <n>; where N i in uV would solve this?
>>> (We should use uV since regulators use this.)
>>
>> Thanks for new property. I will make the unit and type same as the
>> regulator framework.
>
> We have the ops for configuring the pin config as
>
>          int (*pin_config_group_set) (struct pinctrl_dev *pctldev,
>                                       unsigned selector,
>                                       unsigned long *configs,
>                                       unsigned num_configs);
>
>
> The config is 32 bit, upper 16 for config argument and  and lower 16 for
> the config param.
>
> So we can not accommodate 3300000uV until we change it to mV i.e. 3300mV.
>
> So on interface, we can read uV from DT but when making config, we can
> translate it to mV before passing to pin_config_group_set.

A SoC-specific enum would make more sense. It would avoid any 
translation. There's no need for a generic value since the available set 
of options is SoC-specific, the data source is SoC-specific, and there's 
no need for any code besides the SoC-specific driver to interpret the 
value in any way at all.

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

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

On 04/15/2016 02:39 AM, Laxman Dewangan wrote:
>
> On Friday 15 April 2016 01:38 PM, Linus Walleij wrote:
>> On Tue, Apr 12, 2016 at 4:56 PM, Laxman Dewangan
>> <ldewangan@nvidia.com> wrote:
>>
>>> NVIDIA Tegra210 supports the IO pads which can operate at 1.8V
>>> or 3.3V I/O voltage levels. Also the IO pads can be configured
>>> for power down state if it is not used. SW needs to configure the
>>> voltage level of IO pads based on IO rail voltage and its power
>>> state based on platform usage.
>>>
>>> The voltage and power state configurations of pads are provided
>>> through pin control frameworks. Add pin control driver for Tegra's
>>> IO pads' voltage and power state configurations.

>> Even if Tegra is not using the generic code for handling the
>> standard bindings (GENERIC_PINCONF) it doesn't stop
>> you from using the generic bindings and contributing to them.
>>
>> Historically you have a few custom bindings like these:
>>
>> nvidia,pins
>> nvidia,function
>> nvidia,pull
>> nvidia,tristate
>>
>> etc etc, but that is just unfortunate and due to preceding the
>> generic bindings. I would appreciate if you started to support
>> the generic bindings in parallel, but I'm not gonna push that issue.
>
> Yaah, these are in my plate to cleanup. Let me work with Stephen, what
> he think here.

For existing chips, we must always support the existing bindings. 
There's no point moving to the new bindings since it'll just add more 
code that just does the same thing.

For new chips perhaps it makes sense to move to the new standardized 
properties. That said, I don't expect we'll need pinmux on those chips 
since it's guaranteed that FW will set up the static pinmux, and HW 
explicitly doesn't support dynamic pinmuxing?

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

#1376907 — [PATCH 1/7] soc/tegra: pmc: Use BIT macro for register field definition

Use BIT macro for register field definition and make constant as
unsigned when using in shift operator like instead of (3 << 30),
make it to (3U << 30).

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/soc/tegra/pmc.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 08966c2..762f4fa 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -42,29 +42,29 @@
 #include <soc/tegra/pmc.h>
 
 #define PMC_CNTRL			0x0
-#define  PMC_CNTRL_SYSCLK_POLARITY	(1 << 10)  /* sys clk polarity */
-#define  PMC_CNTRL_SYSCLK_OE		(1 << 11)  /* system clock enable */
-#define  PMC_CNTRL_SIDE_EFFECT_LP0	(1 << 14)  /* LP0 when CPU pwr gated */
-#define  PMC_CNTRL_CPU_PWRREQ_POLARITY	(1 << 15)  /* CPU pwr req polarity */
-#define  PMC_CNTRL_CPU_PWRREQ_OE	(1 << 16)  /* CPU pwr req enable */
-#define  PMC_CNTRL_INTR_POLARITY	(1 << 17)  /* inverts INTR polarity */
+#define PMC_CNTRL_SYSCLK_POLARITY	BIT(10) /* sys clk polarity */
+#define PMC_CNTRL_SYSCLK_OE		BIT(11) /* system clock enable */
+#define PMC_CNTRL_SIDE_EFFECT_LP0	BIT(14) /* LP0 when CPU pwr gated */
+#define PMC_CNTRL_CPU_PWRREQ_POLARITY	BIT(15) /* CPU pwr req polarity */
+#define PMC_CNTRL_CPU_PWRREQ_OE		BIT(16) /* CPU pwr req enable */
+#define PMC_CNTRL_INTR_POLARITY		BIT(17)/* inverts INTR polarity */
 
 #define DPD_SAMPLE			0x020
-#define  DPD_SAMPLE_ENABLE		(1 << 0)
-#define  DPD_SAMPLE_DISABLE		(0 << 0)
+#define DPD_SAMPLE_ENABLE		BIT(0)
+#define DPD_SAMPLE_DISABLE		(0 << 0)
 
 #define PWRGATE_TOGGLE			0x30
-#define  PWRGATE_TOGGLE_START		(1 << 8)
+#define PWRGATE_TOGGLE_START		BIT(8)
 
 #define REMOVE_CLAMPING			0x34
 
 #define PWRGATE_STATUS			0x38
 
 #define PMC_SCRATCH0			0x50
-#define  PMC_SCRATCH0_MODE_RECOVERY	(1 << 31)
-#define  PMC_SCRATCH0_MODE_BOOTLOADER	(1 << 30)
-#define  PMC_SCRATCH0_MODE_RCM		(1 << 1)
-#define  PMC_SCRATCH0_MODE_MASK		(PMC_SCRATCH0_MODE_RECOVERY | \
+#define PMC_SCRATCH0_MODE_RECOVERY	BIT(31)
+#define PMC_SCRATCH0_MODE_BOOTLOADER	BIT(30)
+#define PMC_SCRATCH0_MODE_RCM		BIT(1)
+#define PMC_SCRATCH0_MODE_MASK		(PMC_SCRATCH0_MODE_RECOVERY | \
 					 PMC_SCRATCH0_MODE_BOOTLOADER | \
 					 PMC_SCRATCH0_MODE_RCM)
 
@@ -74,14 +74,14 @@
 #define PMC_SCRATCH41			0x140
 
 #define PMC_SENSOR_CTRL			0x1b0
-#define PMC_SENSOR_CTRL_SCRATCH_WRITE	(1 << 2)
-#define PMC_SENSOR_CTRL_ENABLE_RST	(1 << 1)
+#define PMC_SENSOR_CTRL_SCRATCH_WRITE	BIT(2)
+#define PMC_SENSOR_CTRL_ENABLE_RST	BIT(1)
 
 #define IO_DPD_REQ			0x1b8
-#define  IO_DPD_REQ_CODE_IDLE		(0 << 30)
-#define  IO_DPD_REQ_CODE_OFF		(1 << 30)
-#define  IO_DPD_REQ_CODE_ON		(2 << 30)
-#define  IO_DPD_REQ_CODE_MASK		(3 << 30)
+#define IO_DPD_REQ_CODE_IDLE		(0 << 30)
+#define IO_DPD_REQ_CODE_OFF		(1U << 30)
+#define IO_DPD_REQ_CODE_ON		(2U << 30)
+#define IO_DPD_REQ_CODE_MASK		(3U << 30)
 
 #define IO_DPD_STATUS			0x1bc
 #define IO_DPD2_REQ			0x1c0
@@ -93,10 +93,10 @@
 #define PMC_SCRATCH54_ADDR_SHIFT	0
 
 #define PMC_SCRATCH55			0x25c
-#define PMC_SCRATCH55_RESET_TEGRA	(1 << 31)
+#define PMC_SCRATCH55_RESET_TEGRA	BIT(31)
 #define PMC_SCRATCH55_CNTRL_ID_SHIFT	27
 #define PMC_SCRATCH55_PINMUX_SHIFT	24
-#define PMC_SCRATCH55_16BITOP		(1 << 15)
+#define PMC_SCRATCH55_16BITOP		BIT(15)
 #define PMC_SCRATCH55_CHECKSUM_SHIFT	16
 #define PMC_SCRATCH55_I2CSLV1_SHIFT	0
 
-- 
2.1.4

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

#1376925 — Re: [PATCH 1/7] soc/tegra: pmc: Use BIT macro for register field definition

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

On Tue, Apr 12, 2016 at 08:26:41PM +0530, Laxman Dewangan wrote:
> Use BIT macro for register field definition and make constant as
> unsigned when using in shift operator like instead of (3 << 30),
> make it to (3U << 30).
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
>  drivers/soc/tegra/pmc.c | 42 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)

Does this matter at all? We use the explicit notation in quite a number
of places and it works great. I'd like to avoid needless churn unless
there is a very good reason to switch.

Also this contains whitespace changes that remove the extra level of
indentation that is used to separate register field definitions from the
register definitions.

Thierry

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

#1377031 — Re: [PATCH 1/7] soc/tegra: pmc: Use BIT macro for register field definition

On Tuesday 12 April 2016 08:56 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Tue, Apr 12, 2016 at 08:26:41PM +0530, Laxman Dewangan wrote:
>> Use BIT macro for register field definition and make constant as
>> unsigned when using in shift operator like instead of (3 << 30),
>> make it to (3U << 30).
>>
>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>> ---
>>   drivers/soc/tegra/pmc.c | 42 +++++++++++++++++++++---------------------
>>   1 file changed, 21 insertions(+), 21 deletions(-)
> Does this matter at all? We use the explicit notation in quite a number
> of places and it works great. I'd like to avoid needless churn unless
> there is a very good reason to switch.

When I run the checkpatch, I got the error and thought that better to 
fix and cleanup some warning/error from checkpatch.
This is just part of cleanups and properly defining constant like 3U 
instead of 3 to avoid any issue.

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

#1379515 — Re: [PATCH 1/7] soc/tegra: pmc: Use BIT macro for register field definition

On Tue, Apr 12, 2016 at 5:26 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:

> Does this matter at all? We use the explicit notation in quite a number
> of places and it works great. I'd like to avoid needless churn unless
> there is a very good reason to switch.

I am softly enforcing <linux/bitops.h> in the GPIO subsystem
because I think it's a great perceptual aid, I can skim code
faster if this is used, easier for the brain to handle and thus
simplifies maintenance.

Others may disagree on its virtues.

It is not a subjective opinion, but proving the point needs
perceptual psychology research.

Yours,
Linus Walleij

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web