[PATCH V3 0/4] soc/tegra: Add support for IO pads power and voltage control

[PATCH V3 0/4] soc/tegra: Add support for IO pads power and voltage control

[Laxman Dewangan]

The IO pins of Tegra SoCs are grouped for common control of IO interface
like setting voltage signal levels and power state of the interface. The
group is generally referred as IO pads. The power state and voltage control
of IO pins can be done at IO pads level.

Tegra124 onwards IO pads support the power down state when system is
active. The voltage levels on IO pads are auto detected on Tegra124/
Tegra132 but it is not in Tegra210. For Tegra210, the SW need to
explicitly configure the voltage levels of IO pads

This series add the interface for the IO pad power state and voltage
configurations. Modifies the client to use new APIs.
Register the child devices to provide the client interface to configure
IO pads power state and voltage from Device tree.

---
Changes from V2:
- Drop the pinctrl driver from series till pmc interfce is finalise.
- Move client to use the new APIs.
- Remove older APIs to configure IO rail and related macros.

Laxman Dewangan (4):
  soc/tegra: pmc: Use BIT macro for register field definition
  soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()
  soc/tegra: pmc: Add support for IO pads power state and voltage
  soc/tegra: pmc: Register PMC child devices as platform device

 drivers/gpu/drm/tegra/sor.c |   8 +-
 drivers/soc/tegra/pmc.c     | 382 +++++++++++++++++++++++++++++++++++++-------
 include/soc/tegra/pmc.h     | 116 +++++++++-----
 3 files changed, 410 insertions(+), 96 deletions(-)

-- 
2.1.4

[PATCH V3 0/4] soc/tegra: Add support for IO pads power and voltage control

[Laxman Dewangan]

The IO pins of Tegra SoCs are grouped for common control of IO interface
like setting voltage signal levels and power state of the interface. The
group is generally referred as IO pads. The power state and voltage control
of IO pins can be done at IO pads level.

Tegra124 onwards IO pads support the power down state when system is
active. The voltage levels on IO pads are auto detected on Tegra124/
Tegra132 but it is not in Tegra210. For Tegra210, the SW need to
explicitly configure the voltage levels of IO pads

This series add the interface for the IO pad power state and voltage
configurations. Modifies the client to use new APIs.
Register the child devices to provide the client interface to configure
IO pads power state and voltage from Device tree.

---
Changes from V2:
- Drop the pinctrl driver from series till pmc interfce is finalise.
- Move client to use the new APIs.
- Remove older APIs to configure IO rail and related macros.

Laxman Dewangan (4):
  soc/tegra: pmc: Use BIT macro for register field definition
  soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()
  soc/tegra: pmc: Add support for IO pads power state and voltage
  soc/tegra: pmc: Register PMC child devices as platform device

 drivers/gpu/drm/tegra/sor.c |   8 +-
 drivers/soc/tegra/pmc.c     | 382 +++++++++++++++++++++++++++++++++++++-------
 include/soc/tegra/pmc.h     | 116 +++++++++-----
 3 files changed, 410 insertions(+), 96 deletions(-)

-- 
2.1.4

[PATCH V3 1/4] soc/tegra: pmc: Use BIT macro for register field definition

[Laxman Dewangan]

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

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

---
Changes from V1:
- Remove the indenting of line which is not for BIT macro usage.
Changes from V2:
- None
---
 drivers/soc/tegra/pmc.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index bb17345..2c3f1f9 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -45,28 +45,28 @@
 #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_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)
@@ -77,14 +77,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
@@ -96,10 +96,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

[PATCH V3 1/4] soc/tegra: pmc: Use BIT macro for register field definition

[Laxman Dewangan]

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

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

---
Changes from V1:
- Remove the indenting of line which is not for BIT macro usage.
Changes from V2:
- None
---
 drivers/soc/tegra/pmc.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index bb17345..2c3f1f9 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -45,28 +45,28 @@
 #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_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)
@@ -77,14 +77,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
@@ -96,10 +96,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

[PATCH V3 2/4] soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()

[Laxman Dewangan]

The function tegra_pmc_readl() returns the u32 type data and hence
change the data type of variable where this data is stored to u32
type.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

---
Changes from V1:
-This is new in series as per discussion on V1 series to use u32 for
tegra_pmc_readl.

Changes from V2:
- Make unsigned long to u32 for some missed variable from V1.
---
 drivers/soc/tegra/pmc.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 2c3f1f9..eff9425 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -844,7 +844,8 @@ static void tegra_powergate_init(struct tegra_pmc *pmc)
 static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
 				 unsigned long *status, unsigned int *bit)
 {
-	unsigned long rate, value;
+	unsigned long rate;
+	u32 value;
 
 	*bit = id % 32;
 
@@ -868,17 +869,18 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
 	tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
 
 	/* must be at least 200 ns, in APB (PCLK) clock cycles */
-	value = DIV_ROUND_UP(1000000000, rate);
-	value = DIV_ROUND_UP(200, value);
+	rate = DIV_ROUND_UP(1000000000, rate);
+	rate = DIV_ROUND_UP(200, rate);
+	value = (u32)rate;
 	tegra_pmc_writel(value, SEL_DPD_TIM);
 
 	return 0;
 }
 
-static int tegra_io_rail_poll(unsigned long offset, unsigned long mask,
-			      unsigned long val, unsigned long timeout)
+static int tegra_io_rail_poll(unsigned long offset, u32 mask,
+			      u32 val, unsigned long timeout)
 {
-	unsigned long value;
+	u32 value;
 
 	timeout = jiffies + msecs_to_jiffies(timeout);
 
@@ -900,8 +902,9 @@ static void tegra_io_rail_unprepare(void)
 
 int tegra_io_rail_power_on(unsigned int id)
 {
-	unsigned long request, status, value;
-	unsigned int bit, mask;
+	unsigned long request, status;
+	unsigned int bit;
+	u32 value, mask;
 	int err;
 
 	mutex_lock(&pmc->powergates_lock);
@@ -935,8 +938,9 @@ EXPORT_SYMBOL(tegra_io_rail_power_on);
 
 int tegra_io_rail_power_off(unsigned int id)
 {
-	unsigned long request, status, value;
-	unsigned int bit, mask;
+	unsigned long request, status;
+	unsigned int bit;
+	u32 value, mask;
 	int err;
 
 	mutex_lock(&pmc->powergates_lock);
-- 
2.1.4

[PATCH V3 2/4] soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()

[Laxman Dewangan]

The function tegra_pmc_readl() returns the u32 type data and hence
change the data type of variable where this data is stored to u32
type.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

---
Changes from V1:
-This is new in series as per discussion on V1 series to use u32 for
tegra_pmc_readl.

Changes from V2:
- Make unsigned long to u32 for some missed variable from V1.
---
 drivers/soc/tegra/pmc.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 2c3f1f9..eff9425 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -844,7 +844,8 @@ static void tegra_powergate_init(struct tegra_pmc *pmc)
 static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
 				 unsigned long *status, unsigned int *bit)
 {
-	unsigned long rate, value;
+	unsigned long rate;
+	u32 value;
 
 	*bit = id % 32;
 
@@ -868,17 +869,18 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
 	tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
 
 	/* must be at least 200 ns, in APB (PCLK) clock cycles */
-	value = DIV_ROUND_UP(1000000000, rate);
-	value = DIV_ROUND_UP(200, value);
+	rate = DIV_ROUND_UP(1000000000, rate);
+	rate = DIV_ROUND_UP(200, rate);
+	value = (u32)rate;
 	tegra_pmc_writel(value, SEL_DPD_TIM);
 
 	return 0;
 }
 
-static int tegra_io_rail_poll(unsigned long offset, unsigned long mask,
-			      unsigned long val, unsigned long timeout)
+static int tegra_io_rail_poll(unsigned long offset, u32 mask,
+			      u32 val, unsigned long timeout)
 {
-	unsigned long value;
+	u32 value;
 
 	timeout = jiffies + msecs_to_jiffies(timeout);
 
@@ -900,8 +902,9 @@ static void tegra_io_rail_unprepare(void)
 
 int tegra_io_rail_power_on(unsigned int id)
 {
-	unsigned long request, status, value;
-	unsigned int bit, mask;
+	unsigned long request, status;
+	unsigned int bit;
+	u32 value, mask;
 	int err;
 
 	mutex_lock(&pmc->powergates_lock);
@@ -935,8 +938,9 @@ EXPORT_SYMBOL(tegra_io_rail_power_on);
 
 int tegra_io_rail_power_off(unsigned int id)
 {
-	unsigned long request, status, value;
-	unsigned int bit, mask;
+	unsigned long request, status;
+	unsigned int bit;
+	u32 value, mask;
 	int err;
 
 	mutex_lock(&pmc->powergates_lock);
-- 
2.1.4

[PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Laxman Dewangan]

The IO pins of Tegra SoCs are grouped for common control of IO
interface like setting voltage signal levels and power state of
the interface. The group is generally referred as IO pads. The
power state and voltage control of IO pins can be done at IO pads
level.

Tegra generation SoC supports the power down of IO pads when it
is not used even in the active state of system. This saves power
from that IO interface. Also it supports multiple voltage level
in IO pins for interfacing on some of pads. The IO pad voltage is
automatically detected till T124, hence SW need not to configure
this. But from T210, the automatically detection logic has been
removed, hence SW need to explicitly set the IO pad voltage into
IO pad configuration registers.

Add support to set the power states and voltage level of the IO pads
from client driver. The implementation for the APIs are in generic
which is applicable for all generation os Tegra SoC.

IO pads ID and information of bit field for power state and voltage
level controls are added for Tegra124, Tegra132 and Tegra210. The sor
driver is modified to use the new APIs.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

---
Changes from V1:
This is reworked on earlier path to have separation between IO rails and
io pads and add power state and voltage control APIs in single call.

Changes from V2:
- Remove the tegra_io_rail_power_off/on() apis and change client (sor) driver
to use the new APIs for IO pad power.
- Remove the TEGRA_IO_RAIL_ macros.
---
 drivers/gpu/drm/tegra/sor.c |   8 +-
 drivers/soc/tegra/pmc.c     | 242 +++++++++++++++++++++++++++++++++++++++-----
 include/soc/tegra/pmc.h     | 116 ++++++++++++++-------
 3 files changed, 299 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 757c6e8..1d90171 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -1134,7 +1134,7 @@ static void tegra_sor_edp_disable(struct drm_encoder *encoder)
 			dev_err(sor->dev, "failed to disable DP: %d\n", err);
 	}
 
-	err = tegra_io_rail_power_off(TEGRA_IO_RAIL_LVDS);
+	err = tegra_io_pads_power_disable(TEGRA_IO_PAD_LVDS);
 	if (err < 0)
 		dev_err(sor->dev, "failed to power off I/O rail: %d\n", err);
 
@@ -1296,7 +1296,7 @@ static void tegra_sor_edp_enable(struct drm_encoder *encoder)
 	tegra_sor_writel(sor, value, SOR_DP_PADCTL0);
 
 	/* step 2 */
-	err = tegra_io_rail_power_on(TEGRA_IO_RAIL_LVDS);
+	err = tegra_io_pads_power_enable(TEGRA_IO_PAD_LVDS);
 	if (err < 0)
 		dev_err(sor->dev, "failed to power on I/O rail: %d\n", err);
 
@@ -1748,7 +1748,7 @@ static void tegra_sor_hdmi_disable(struct drm_encoder *encoder)
 	if (err < 0)
 		dev_err(sor->dev, "failed to power down SOR: %d\n", err);
 
-	err = tegra_io_rail_power_off(TEGRA_IO_RAIL_HDMI);
+	err = tegra_io_pads_power_disable(TEGRA_IO_PAD_HDMI);
 	if (err < 0)
 		dev_err(sor->dev, "failed to power off HDMI rail: %d\n", err);
 
@@ -1787,7 +1787,7 @@ static void tegra_sor_hdmi_enable(struct drm_encoder *encoder)
 
 	div = clk_get_rate(sor->clk) / 1000000 * 4;
 
-	err = tegra_io_rail_power_on(TEGRA_IO_RAIL_HDMI);
+	err = tegra_io_pads_power_enable(TEGRA_IO_PAD_HDMI);
 	if (err < 0)
 		dev_err(sor->dev, "failed to power on HDMI rail: %d\n", err);
 
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index eff9425..0611cea 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -76,6 +76,10 @@
 
 #define PMC_SCRATCH41			0x140
 
+/* Power detect for pad voltage */
+#define PMC_PWR_DET			0x48
+#define PMC_PWR_DET_VAL			0xe4
+
 #define PMC_SENSOR_CTRL			0x1b0
 #define PMC_SENSOR_CTRL_SCRATCH_WRITE	BIT(2)
 #define PMC_SENSOR_CTRL_ENABLE_RST	BIT(1)
@@ -115,12 +119,19 @@ struct tegra_powergate {
 	unsigned int num_resets;
 };
 
+struct tegra_io_pads_control {
+	int pad_id;
+	int dpd_bit_pos;
+	int pwr_bit_pos;
+};
+
 struct tegra_pmc_soc {
 	unsigned int num_powergates;
 	const char *const *powergates;
 	unsigned int num_cpu_powergates;
 	const u8 *cpu_powergates;
-
+	const struct tegra_io_pads_control *io_pads_control;
+	unsigned int num_io_pads;
 	bool has_tsense_reset;
 	bool has_gpu_clamps;
 };
@@ -196,6 +207,14 @@ static void tegra_pmc_writel(u32 value, unsigned long offset)
 	writel(value, pmc->base + offset);
 }
 
+static void tegra_pmc_read_modify_write(unsigned long offset, u32 mask, u32 val)
+{
+	u32 pmc_reg = tegra_pmc_readl(offset);
+
+	pmc_reg = (pmc_reg & ~mask) | (val & mask);
+	tegra_pmc_writel(pmc_reg, offset);
+}
+
 static inline bool tegra_powergate_state(int id)
 {
 	if (id == TEGRA_POWERGATE_3D && pmc->soc->has_gpu_clamps)
@@ -841,22 +860,51 @@ static void tegra_powergate_init(struct tegra_pmc *pmc)
 	of_node_put(np);
 }
 
-static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
-				 unsigned long *status, unsigned int *bit)
+static int tegra_io_pads_to_dpd_bit(const struct tegra_pmc_soc *soc, int pad_id)
+{
+	int i;
+
+	if (!soc || !soc->num_io_pads)
+		return -EINVAL;
+
+	for (i = 0; i < soc->num_io_pads; ++i) {
+		if (soc->io_pads_control[i].pad_id == pad_id)
+			return soc->io_pads_control[i].dpd_bit_pos;
+	}
+
+	return -EINVAL;
+}
+
+static int tegra_io_pads_to_power_bit(const struct tegra_pmc_soc *soc,
+				      int pad_id)
+{
+	int i;
+
+	if (!soc || !soc->num_io_pads)
+		return -EINVAL;
+
+	for (i = 0; i < soc->num_io_pads; ++i) {
+		if (soc->io_pads_control[i].pad_id == pad_id)
+			return soc->io_pads_control[i].pwr_bit_pos;
+	}
+
+	return -EINVAL;
+}
+
+static int tegra_io_pad_dpd_prepare(int pad_id, unsigned long *request,
+				    unsigned long *status, unsigned int *bit)
 {
 	unsigned long rate;
 	u32 value;
+	int dpd_bit;
 
-	*bit = id % 32;
+	dpd_bit = tegra_io_pads_to_dpd_bit(pmc->soc, pad_id);
+	if (dpd_bit < 0)
+		return dpd_bit;
 
-	/*
-	 * There are two sets of 30 bits to select IO rails, but bits 30 and
-	 * 31 are control bits rather than IO rail selection bits.
-	 */
-	if (id > 63 || *bit == 30 || *bit == 31)
-		return -EINVAL;
+	*bit = dpd_bit % 32;
 
-	if (id < 32) {
+	if (dpd_bit < 32) {
 		*status = IO_DPD_STATUS;
 		*request = IO_DPD_REQ;
 	} else {
@@ -877,8 +925,8 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
 	return 0;
 }
 
-static int tegra_io_rail_poll(unsigned long offset, u32 mask,
-			      u32 val, unsigned long timeout)
+static int tegra_io_pad_dpd_poll(unsigned long offset, u32 mask,
+				 u32 val, unsigned long timeout)
 {
 	u32 value;
 
@@ -895,12 +943,12 @@ static int tegra_io_rail_poll(unsigned long offset, u32 mask,
 	return -ETIMEDOUT;
 }
 
-static void tegra_io_rail_unprepare(void)
+static void tegra_io_pad_dpd_unprepare(void)
 {
 	tegra_pmc_writel(DPD_SAMPLE_DISABLE, DPD_SAMPLE);
 }
 
-int tegra_io_rail_power_on(unsigned int id)
+int tegra_io_pads_power_enable(int pad_id)
 {
 	unsigned long request, status;
 	unsigned int bit;
@@ -909,7 +957,7 @@ int tegra_io_rail_power_on(unsigned int id)
 
 	mutex_lock(&pmc->powergates_lock);
 
-	err = tegra_io_rail_prepare(id, &request, &status, &bit);
+	err = tegra_io_pad_dpd_prepare(pad_id, &request, &status, &bit);
 	if (err)
 		goto error;
 
@@ -921,22 +969,22 @@ int tegra_io_rail_power_on(unsigned int id)
 	value |= IO_DPD_REQ_CODE_OFF;
 	tegra_pmc_writel(value, request);
 
-	err = tegra_io_rail_poll(status, mask, 0, 250);
+	err = tegra_io_pad_dpd_poll(status, mask, 0, 250);
 	if (err) {
-		pr_info("tegra_io_rail_poll() failed: %d\n", err);
+		pr_info("tegra_io_pad_dpd_poll() failed: %d\n", err);
 		goto error;
 	}
 
-	tegra_io_rail_unprepare();
+	tegra_io_pad_dpd_unprepare();
 
 error:
 	mutex_unlock(&pmc->powergates_lock);
 
 	return err;
 }
-EXPORT_SYMBOL(tegra_io_rail_power_on);
+EXPORT_SYMBOL(tegra_io_pads_power_enable);
 
-int tegra_io_rail_power_off(unsigned int id)
+int tegra_io_pads_power_disable(int pad_id)
 {
 	unsigned long request, status;
 	unsigned int bit;
@@ -945,9 +993,9 @@ int tegra_io_rail_power_off(unsigned int id)
 
 	mutex_lock(&pmc->powergates_lock);
 
-	err = tegra_io_rail_prepare(id, &request, &status, &bit);
+	err = tegra_io_pad_dpd_prepare(pad_id, &request, &status, &bit);
 	if (err) {
-		pr_info("tegra_io_rail_prepare() failed: %d\n", err);
+		pr_info("tegra_io_pad_dpd_prepare() failed: %d\n", err);
 		goto error;
 	}
 
@@ -959,18 +1007,77 @@ int tegra_io_rail_power_off(unsigned int id)
 	value |= IO_DPD_REQ_CODE_ON;
 	tegra_pmc_writel(value, request);
 
-	err = tegra_io_rail_poll(status, mask, mask, 250);
+	err = tegra_io_pad_dpd_poll(status, mask, mask, 250);
 	if (err)
 		goto error;
 
-	tegra_io_rail_unprepare();
+	tegra_io_pad_dpd_unprepare();
 
 error:
 	mutex_unlock(&pmc->powergates_lock);
 
 	return err;
 }
-EXPORT_SYMBOL(tegra_io_rail_power_off);
+EXPORT_SYMBOL(tegra_io_pads_power_disable);
+
+int tegra_io_pads_power_is_enabled(int io_pad_id)
+{
+	unsigned long status_reg;
+	u32 status;
+	int dpd_bit;
+
+	dpd_bit = tegra_io_pads_to_dpd_bit(pmc->soc, io_pad_id);
+	if (dpd_bit < 0)
+		return dpd_bit;
+
+	status_reg = (dpd_bit < 32) ? IO_DPD_STATUS : IO_DPD2_STATUS;
+	status = tegra_pmc_readl(status_reg);
+
+	return !!(status & BIT(dpd_bit % 32));
+}
+EXPORT_SYMBOL(tegra_io_pads_power_is_enabled);
+
+int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
+{
+	int pwr_bit;
+	u32 pwr_mask;
+	u32 bval;
+
+	if ((io_volt_uv != 3300000) && (io_volt_uv != 1800000))
+		return -EINVAL;
+
+	pwr_bit = tegra_io_pads_to_power_bit(pmc->soc, io_pad_id);
+	if (pwr_bit < 0)
+		return pwr_bit;
+
+	pwr_mask = BIT(pwr_bit);
+	bval = (io_volt_uv == 3300000) ? pwr_mask : 0;
+
+	mutex_lock(&pmc->powergates_lock);
+	tegra_pmc_read_modify_write(PMC_PWR_DET, pwr_mask, pwr_mask);
+	tegra_pmc_read_modify_write(PMC_PWR_DET_VAL, pwr_mask, bval);
+	mutex_unlock(&pmc->powergates_lock);
+
+	usleep_range(100, 250);
+
+	return 0;
+}
+EXPORT_SYMBOL(tegra_io_pads_configure_voltage);
+
+int tegra_io_pads_get_configured_voltage(int io_pad_id)
+{
+	int pwr_bit;
+	u32 pwr_det_val;
+
+	pwr_bit = tegra_io_pads_to_power_bit(pmc->soc, io_pad_id);
+	if (pwr_bit < 0)
+		return pwr_bit;
+
+	pwr_det_val = tegra_pmc_readl(PMC_PWR_DET_VAL);
+
+	return (pwr_det_val & BIT(pwr_bit)) ? 3300000 : 1800000;
+}
+EXPORT_SYMBOL(tegra_io_pads_get_configured_voltage);
 
 #ifdef CONFIG_PM_SLEEP
 enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void)
@@ -1397,11 +1504,53 @@ static const u8 tegra124_cpu_powergates[] = {
 	TEGRA_POWERGATE_CPU3,
 };
 
+#define TEGRA_IO_PADS_CONTROL(_pad, _dpd, _pwr)		\
+{							\
+	.pad_id = (TEGRA_IO_PAD_##_pad),		\
+	.dpd_bit_pos = (_dpd),				\
+	.pwr_bit_pos = (_pwr),				\
+}
+
+struct tegra_io_pads_control tegra124_io_pads_control[] = {
+	TEGRA_IO_PADS_CONTROL(CSIA, 0, -1),
+	TEGRA_IO_PADS_CONTROL(CSIB, 1, -1),
+	TEGRA_IO_PADS_CONTROL(DSI, 2, -1),
+	TEGRA_IO_PADS_CONTROL(MIPI_BIAS, 3, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_BIAS, 4, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_CLK1, 5, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_CLK2, 6, -1),
+	TEGRA_IO_PADS_CONTROL(USB0, 9, -1),
+	TEGRA_IO_PADS_CONTROL(USB1, 10, -1),
+	TEGRA_IO_PADS_CONTROL(USB2, 11, -1),
+	TEGRA_IO_PADS_CONTROL(USB_BIAS, 12, -1),
+	TEGRA_IO_PADS_CONTROL(NAND, 13, -1),
+	TEGRA_IO_PADS_CONTROL(UART, 14, -1),
+	TEGRA_IO_PADS_CONTROL(BB, 15, -1),
+	TEGRA_IO_PADS_CONTROL(AUDIO, 17, -1),
+	TEGRA_IO_PADS_CONTROL(HSIC, 19, -1),
+	TEGRA_IO_PADS_CONTROL(COMP, 22, -1),
+	TEGRA_IO_PADS_CONTROL(HDMI, 28, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_CNTRL, 32, -1),
+	TEGRA_IO_PADS_CONTROL(SDMMC1, 33, -1),
+	TEGRA_IO_PADS_CONTROL(SDMMC3, 34, -1),
+	TEGRA_IO_PADS_CONTROL(SDMMC4, 35, -1),
+	TEGRA_IO_PADS_CONTROL(CAM, 36, -1),
+	TEGRA_IO_PADS_CONTROL(HV, 38, -1),
+	TEGRA_IO_PADS_CONTROL(DSIB, 39, -1),
+	TEGRA_IO_PADS_CONTROL(DSIC, 40, -1),
+	TEGRA_IO_PADS_CONTROL(DSID, 41, -1),
+	TEGRA_IO_PADS_CONTROL(CSIE, 44, -1),
+	TEGRA_IO_PADS_CONTROL(LVDS, 57, -1),
+	TEGRA_IO_PADS_CONTROL(SYS_DDC, 58, -1),
+};
+
 static const struct tegra_pmc_soc tegra124_pmc_soc = {
 	.num_powergates = ARRAY_SIZE(tegra124_powergates),
 	.powergates = tegra124_powergates,
 	.num_cpu_powergates = ARRAY_SIZE(tegra124_cpu_powergates),
 	.cpu_powergates = tegra124_cpu_powergates,
+	.io_pads_control = tegra124_io_pads_control,
+	.num_io_pads = ARRAY_SIZE(tegra124_io_pads_control),
 	.has_tsense_reset = true,
 	.has_gpu_clamps = true,
 };
@@ -1440,11 +1589,52 @@ static const u8 tegra210_cpu_powergates[] = {
 	TEGRA_POWERGATE_CPU3,
 };
 
+struct tegra_io_pads_control tegra210_io_pads_control[] = {
+	TEGRA_IO_PADS_CONTROL(CSIA, 0, -1),
+	TEGRA_IO_PADS_CONTROL(CSIB, 1, -1),
+	TEGRA_IO_PADS_CONTROL(DSI, 2, -1),
+	TEGRA_IO_PADS_CONTROL(MIPI_BIAS, 3, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_BIAS, 4, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_CLK1, 5, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_CLK2, 6, -1),
+	TEGRA_IO_PADS_CONTROL(USB0, 9, -1),
+	TEGRA_IO_PADS_CONTROL(USB1, 10, -1),
+	TEGRA_IO_PADS_CONTROL(USB2, 11, -1),
+	TEGRA_IO_PADS_CONTROL(USB_BIAS, 12, -1),
+	TEGRA_IO_PADS_CONTROL(UART, 14, -1),
+	TEGRA_IO_PADS_CONTROL(AUDIO, 17, -1),
+	TEGRA_IO_PADS_CONTROL(USB3, 18, -1),
+	TEGRA_IO_PADS_CONTROL(HSIC, 19, -1),
+	TEGRA_IO_PADS_CONTROL(DBG, 25, -1),
+	TEGRA_IO_PADS_CONTROL(DEBUG_NONAO, 26, -1),
+	TEGRA_IO_PADS_CONTROL(GPIO, 27, 21),
+	TEGRA_IO_PADS_CONTROL(HDMI, 28, -1),
+	TEGRA_IO_PADS_CONTROL(SDMMC1, 33, 12),
+	TEGRA_IO_PADS_CONTROL(SDMMC3, 34, 13),
+	TEGRA_IO_PADS_CONTROL(EMMC, 35, -1),
+	TEGRA_IO_PADS_CONTROL(CAM, 36, -1),
+	TEGRA_IO_PADS_CONTROL(EMMC2, 37, -1),
+	TEGRA_IO_PADS_CONTROL(DSIB, 39, -1),
+	TEGRA_IO_PADS_CONTROL(DSIC, 40, -1),
+	TEGRA_IO_PADS_CONTROL(DSID, 41, -1),
+	TEGRA_IO_PADS_CONTROL(CSIC, 42, -1),
+	TEGRA_IO_PADS_CONTROL(CSID, 43, -1),
+	TEGRA_IO_PADS_CONTROL(CSIE, 44, -1),
+	TEGRA_IO_PADS_CONTROL(CSIF, 45, -1),
+	TEGRA_IO_PADS_CONTROL(SPI, 46, -1),
+	TEGRA_IO_PADS_CONTROL(SPI_HV, 47, 23),
+	TEGRA_IO_PADS_CONTROL(DMIC, 50, -1),
+	TEGRA_IO_PADS_CONTROL(DP, 51, -1),
+	TEGRA_IO_PADS_CONTROL(LVDS, 57, -1),
+	TEGRA_IO_PADS_CONTROL(AUDIO_HV, 61, 18),
+};
 static const struct tegra_pmc_soc tegra210_pmc_soc = {
 	.num_powergates = ARRAY_SIZE(tegra210_powergates),
 	.powergates = tegra210_powergates,
 	.num_cpu_powergates = ARRAY_SIZE(tegra210_cpu_powergates),
 	.cpu_powergates = tegra210_cpu_powergates,
+	.io_pads_control = tegra210_io_pads_control,
+	.num_io_pads = ARRAY_SIZE(tegra210_io_pads_control),
 	.has_tsense_reset = true,
 	.has_gpu_clamps = true,
 };
diff --git a/include/soc/tegra/pmc.h b/include/soc/tegra/pmc.h
index e9e5347..763bd6d 100644
--- a/include/soc/tegra/pmc.h
+++ b/include/soc/tegra/pmc.h
@@ -76,37 +76,57 @@ int tegra_pmc_cpu_remove_clamping(unsigned int cpuid);
 
 #define TEGRA_POWERGATE_3D0	TEGRA_POWERGATE_3D
 
-#define TEGRA_IO_RAIL_CSIA	0
-#define TEGRA_IO_RAIL_CSIB	1
-#define TEGRA_IO_RAIL_DSI	2
-#define TEGRA_IO_RAIL_MIPI_BIAS	3
-#define TEGRA_IO_RAIL_PEX_BIAS	4
-#define TEGRA_IO_RAIL_PEX_CLK1	5
-#define TEGRA_IO_RAIL_PEX_CLK2	6
-#define TEGRA_IO_RAIL_USB0	9
-#define TEGRA_IO_RAIL_USB1	10
-#define TEGRA_IO_RAIL_USB2	11
-#define TEGRA_IO_RAIL_USB_BIAS	12
-#define TEGRA_IO_RAIL_NAND	13
-#define TEGRA_IO_RAIL_UART	14
-#define TEGRA_IO_RAIL_BB	15
-#define TEGRA_IO_RAIL_AUDIO	17
-#define TEGRA_IO_RAIL_HSIC	19
-#define TEGRA_IO_RAIL_COMP	22
-#define TEGRA_IO_RAIL_HDMI	28
-#define TEGRA_IO_RAIL_PEX_CNTRL	32
-#define TEGRA_IO_RAIL_SDMMC1	33
-#define TEGRA_IO_RAIL_SDMMC3	34
-#define TEGRA_IO_RAIL_SDMMC4	35
-#define TEGRA_IO_RAIL_CAM	36
-#define TEGRA_IO_RAIL_RES	37
-#define TEGRA_IO_RAIL_HV	38
-#define TEGRA_IO_RAIL_DSIB	39
-#define TEGRA_IO_RAIL_DSIC	40
-#define TEGRA_IO_RAIL_DSID	41
-#define TEGRA_IO_RAIL_CSIE	44
-#define TEGRA_IO_RAIL_LVDS	57
-#define TEGRA_IO_RAIL_SYS_DDC	58
+/* TEGRA_IO_PAD: The IO pins of Tegra SoCs are grouped for common control
+ * of IO interface like setting voltage signal levels, power state of the
+ * interface. The group is generally referred as io-pads. The power and
+ * voltage control of IO pins are available at io-pads level.
+ * The following macros make the super list all IO pads found on Tegra SoC
+ * generations.
+ */
+#define TEGRA_IO_PAD_AUDIO		0
+#define TEGRA_IO_PAD_AUDIO_HV		1
+#define TEGRA_IO_PAD_BB			2
+#define TEGRA_IO_PAD_CAM		3
+#define TEGRA_IO_PAD_COMP		4
+#define TEGRA_IO_PAD_CSIA		5
+#define TEGRA_IO_PAD_CSIB		6
+#define TEGRA_IO_PAD_CSIC		7
+#define TEGRA_IO_PAD_CSID		8
+#define TEGRA_IO_PAD_CSIE		9
+#define TEGRA_IO_PAD_CSIF		10
+#define TEGRA_IO_PAD_DBG		11
+#define TEGRA_IO_PAD_DEBUG_NONAO	12
+#define TEGRA_IO_PAD_DMIC		13
+#define TEGRA_IO_PAD_DP			14
+#define TEGRA_IO_PAD_DSI		15
+#define TEGRA_IO_PAD_DSIB		16
+#define TEGRA_IO_PAD_DSIC		17
+#define TEGRA_IO_PAD_DSID		18
+#define TEGRA_IO_PAD_EMMC		19
+#define TEGRA_IO_PAD_EMMC2		20
+#define TEGRA_IO_PAD_GPIO		21
+#define TEGRA_IO_PAD_HDMI		22
+#define TEGRA_IO_PAD_HSIC		23
+#define TEGRA_IO_PAD_HV			24
+#define TEGRA_IO_PAD_LVDS		25
+#define TEGRA_IO_PAD_MIPI_BIAS		26
+#define TEGRA_IO_PAD_NAND		27
+#define TEGRA_IO_PAD_PEX_BIAS		28
+#define TEGRA_IO_PAD_PEX_CLK1		29
+#define TEGRA_IO_PAD_PEX_CLK2		30
+#define TEGRA_IO_PAD_PEX_CNTRL		31
+#define TEGRA_IO_PAD_SDMMC1		32
+#define TEGRA_IO_PAD_SDMMC3		33
+#define TEGRA_IO_PAD_SDMMC4		34
+#define TEGRA_IO_PAD_SPI		35
+#define TEGRA_IO_PAD_SPI_HV		36
+#define TEGRA_IO_PAD_SYS_DDC		37
+#define TEGRA_IO_PAD_UART		38
+#define TEGRA_IO_PAD_USB0		39
+#define TEGRA_IO_PAD_USB1		40
+#define TEGRA_IO_PAD_USB2		41
+#define TEGRA_IO_PAD_USB3		42
+#define TEGRA_IO_PAD_USB_BIAS		43
 
 #ifdef CONFIG_ARCH_TEGRA
 int tegra_powergate_is_powered(unsigned int id);
@@ -118,8 +138,15 @@ int tegra_powergate_remove_clamping(unsigned int id);
 int tegra_powergate_sequence_power_up(unsigned int id, struct clk *clk,
 				      struct reset_control *rst);
 
-int tegra_io_rail_power_on(unsigned int id);
-int tegra_io_rail_power_off(unsigned int id);
+/* Power enable/disable of the IO pads */
+int tegra_io_pads_power_enable(int io_pad_id);
+int tegra_io_pads_power_disable(int io_pad_id);
+int tegra_io_pads_power_is_enabled(int io_pad_id);
+
+/* Set/Get of IO pad voltage */
+int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv);
+int tegra_io_pads_get_configured_voltage(int io_pad_id);
+
 #else
 static inline int tegra_powergate_is_powered(unsigned int id)
 {
@@ -148,14 +175,29 @@ static inline int tegra_powergate_sequence_power_up(unsigned int id,
 	return -ENOSYS;
 }
 
-static inline int tegra_io_rail_power_on(unsigned int id)
+static inline int tegra_io_pads_power_enable(int io_pad_id)
 {
-	return -ENOSYS;
+	return -ENOTSUPP;
 }
 
-static inline int tegra_io_rail_power_off(unsigned int id)
+static inline int tegra_io_pads_power_disable(int io_pad_id)
 {
-	return -ENOSYS;
+	return -ENOTSUPP;
+}
+
+static inline int tegra_io_pads_power_is_enabled(int io_pad_id)
+{
+	return -ENOTSUPP;
+}
+
+static inline int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
+{
+	return -ENOTSUPP;
+}
+
+static inline int tegra_io_pads_get_configured_voltage(int io_pad_id)
+{
+	return -ENOTSUPP;
 }
 #endif /* CONFIG_ARCH_TEGRA */
 
-- 
2.1.4

[PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Laxman Dewangan]

The IO pins of Tegra SoCs are grouped for common control of IO
interface like setting voltage signal levels and power state of
the interface. The group is generally referred as IO pads. The
power state and voltage control of IO pins can be done at IO pads
level.

Tegra generation SoC supports the power down of IO pads when it
is not used even in the active state of system. This saves power
from that IO interface. Also it supports multiple voltage level
in IO pins for interfacing on some of pads. The IO pad voltage is
automatically detected till T124, hence SW need not to configure
this. But from T210, the automatically detection logic has been
removed, hence SW need to explicitly set the IO pad voltage into
IO pad configuration registers.

Add support to set the power states and voltage level of the IO pads
from client driver. The implementation for the APIs are in generic
which is applicable for all generation os Tegra SoC.

IO pads ID and information of bit field for power state and voltage
level controls are added for Tegra124, Tegra132 and Tegra210. The sor
driver is modified to use the new APIs.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

---
Changes from V1:
This is reworked on earlier path to have separation between IO rails and
io pads and add power state and voltage control APIs in single call.

Changes from V2:
- Remove the tegra_io_rail_power_off/on() apis and change client (sor) driver
to use the new APIs for IO pad power.
- Remove the TEGRA_IO_RAIL_ macros.
---
 drivers/gpu/drm/tegra/sor.c |   8 +-
 drivers/soc/tegra/pmc.c     | 242 +++++++++++++++++++++++++++++++++++++++-----
 include/soc/tegra/pmc.h     | 116 ++++++++++++++-------
 3 files changed, 299 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 757c6e8..1d90171 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -1134,7 +1134,7 @@ static void tegra_sor_edp_disable(struct drm_encoder *encoder)
 			dev_err(sor->dev, "failed to disable DP: %d\n", err);
 	}
 
-	err = tegra_io_rail_power_off(TEGRA_IO_RAIL_LVDS);
+	err = tegra_io_pads_power_disable(TEGRA_IO_PAD_LVDS);
 	if (err < 0)
 		dev_err(sor->dev, "failed to power off I/O rail: %d\n", err);
 
@@ -1296,7 +1296,7 @@ static void tegra_sor_edp_enable(struct drm_encoder *encoder)
 	tegra_sor_writel(sor, value, SOR_DP_PADCTL0);
 
 	/* step 2 */
-	err = tegra_io_rail_power_on(TEGRA_IO_RAIL_LVDS);
+	err = tegra_io_pads_power_enable(TEGRA_IO_PAD_LVDS);
 	if (err < 0)
 		dev_err(sor->dev, "failed to power on I/O rail: %d\n", err);
 
@@ -1748,7 +1748,7 @@ static void tegra_sor_hdmi_disable(struct drm_encoder *encoder)
 	if (err < 0)
 		dev_err(sor->dev, "failed to power down SOR: %d\n", err);
 
-	err = tegra_io_rail_power_off(TEGRA_IO_RAIL_HDMI);
+	err = tegra_io_pads_power_disable(TEGRA_IO_PAD_HDMI);
 	if (err < 0)
 		dev_err(sor->dev, "failed to power off HDMI rail: %d\n", err);
 
@@ -1787,7 +1787,7 @@ static void tegra_sor_hdmi_enable(struct drm_encoder *encoder)
 
 	div = clk_get_rate(sor->clk) / 1000000 * 4;
 
-	err = tegra_io_rail_power_on(TEGRA_IO_RAIL_HDMI);
+	err = tegra_io_pads_power_enable(TEGRA_IO_PAD_HDMI);
 	if (err < 0)
 		dev_err(sor->dev, "failed to power on HDMI rail: %d\n", err);
 
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index eff9425..0611cea 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -76,6 +76,10 @@
 
 #define PMC_SCRATCH41			0x140
 
+/* Power detect for pad voltage */
+#define PMC_PWR_DET			0x48
+#define PMC_PWR_DET_VAL			0xe4
+
 #define PMC_SENSOR_CTRL			0x1b0
 #define PMC_SENSOR_CTRL_SCRATCH_WRITE	BIT(2)
 #define PMC_SENSOR_CTRL_ENABLE_RST	BIT(1)
@@ -115,12 +119,19 @@ struct tegra_powergate {
 	unsigned int num_resets;
 };
 
+struct tegra_io_pads_control {
+	int pad_id;
+	int dpd_bit_pos;
+	int pwr_bit_pos;
+};
+
 struct tegra_pmc_soc {
 	unsigned int num_powergates;
 	const char *const *powergates;
 	unsigned int num_cpu_powergates;
 	const u8 *cpu_powergates;
-
+	const struct tegra_io_pads_control *io_pads_control;
+	unsigned int num_io_pads;
 	bool has_tsense_reset;
 	bool has_gpu_clamps;
 };
@@ -196,6 +207,14 @@ static void tegra_pmc_writel(u32 value, unsigned long offset)
 	writel(value, pmc->base + offset);
 }
 
+static void tegra_pmc_read_modify_write(unsigned long offset, u32 mask, u32 val)
+{
+	u32 pmc_reg = tegra_pmc_readl(offset);
+
+	pmc_reg = (pmc_reg & ~mask) | (val & mask);
+	tegra_pmc_writel(pmc_reg, offset);
+}
+
 static inline bool tegra_powergate_state(int id)
 {
 	if (id == TEGRA_POWERGATE_3D && pmc->soc->has_gpu_clamps)
@@ -841,22 +860,51 @@ static void tegra_powergate_init(struct tegra_pmc *pmc)
 	of_node_put(np);
 }
 
-static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
-				 unsigned long *status, unsigned int *bit)
+static int tegra_io_pads_to_dpd_bit(const struct tegra_pmc_soc *soc, int pad_id)
+{
+	int i;
+
+	if (!soc || !soc->num_io_pads)
+		return -EINVAL;
+
+	for (i = 0; i < soc->num_io_pads; ++i) {
+		if (soc->io_pads_control[i].pad_id == pad_id)
+			return soc->io_pads_control[i].dpd_bit_pos;
+	}
+
+	return -EINVAL;
+}
+
+static int tegra_io_pads_to_power_bit(const struct tegra_pmc_soc *soc,
+				      int pad_id)
+{
+	int i;
+
+	if (!soc || !soc->num_io_pads)
+		return -EINVAL;
+
+	for (i = 0; i < soc->num_io_pads; ++i) {
+		if (soc->io_pads_control[i].pad_id == pad_id)
+			return soc->io_pads_control[i].pwr_bit_pos;
+	}
+
+	return -EINVAL;
+}
+
+static int tegra_io_pad_dpd_prepare(int pad_id, unsigned long *request,
+				    unsigned long *status, unsigned int *bit)
 {
 	unsigned long rate;
 	u32 value;
+	int dpd_bit;
 
-	*bit = id % 32;
+	dpd_bit = tegra_io_pads_to_dpd_bit(pmc->soc, pad_id);
+	if (dpd_bit < 0)
+		return dpd_bit;
 
-	/*
-	 * There are two sets of 30 bits to select IO rails, but bits 30 and
-	 * 31 are control bits rather than IO rail selection bits.
-	 */
-	if (id > 63 || *bit == 30 || *bit == 31)
-		return -EINVAL;
+	*bit = dpd_bit % 32;
 
-	if (id < 32) {
+	if (dpd_bit < 32) {
 		*status = IO_DPD_STATUS;
 		*request = IO_DPD_REQ;
 	} else {
@@ -877,8 +925,8 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
 	return 0;
 }
 
-static int tegra_io_rail_poll(unsigned long offset, u32 mask,
-			      u32 val, unsigned long timeout)
+static int tegra_io_pad_dpd_poll(unsigned long offset, u32 mask,
+				 u32 val, unsigned long timeout)
 {
 	u32 value;
 
@@ -895,12 +943,12 @@ static int tegra_io_rail_poll(unsigned long offset, u32 mask,
 	return -ETIMEDOUT;
 }
 
-static void tegra_io_rail_unprepare(void)
+static void tegra_io_pad_dpd_unprepare(void)
 {
 	tegra_pmc_writel(DPD_SAMPLE_DISABLE, DPD_SAMPLE);
 }
 
-int tegra_io_rail_power_on(unsigned int id)
+int tegra_io_pads_power_enable(int pad_id)
 {
 	unsigned long request, status;
 	unsigned int bit;
@@ -909,7 +957,7 @@ int tegra_io_rail_power_on(unsigned int id)
 
 	mutex_lock(&pmc->powergates_lock);
 
-	err = tegra_io_rail_prepare(id, &request, &status, &bit);
+	err = tegra_io_pad_dpd_prepare(pad_id, &request, &status, &bit);
 	if (err)
 		goto error;
 
@@ -921,22 +969,22 @@ int tegra_io_rail_power_on(unsigned int id)
 	value |= IO_DPD_REQ_CODE_OFF;
 	tegra_pmc_writel(value, request);
 
-	err = tegra_io_rail_poll(status, mask, 0, 250);
+	err = tegra_io_pad_dpd_poll(status, mask, 0, 250);
 	if (err) {
-		pr_info("tegra_io_rail_poll() failed: %d\n", err);
+		pr_info("tegra_io_pad_dpd_poll() failed: %d\n", err);
 		goto error;
 	}
 
-	tegra_io_rail_unprepare();
+	tegra_io_pad_dpd_unprepare();
 
 error:
 	mutex_unlock(&pmc->powergates_lock);
 
 	return err;
 }
-EXPORT_SYMBOL(tegra_io_rail_power_on);
+EXPORT_SYMBOL(tegra_io_pads_power_enable);
 
-int tegra_io_rail_power_off(unsigned int id)
+int tegra_io_pads_power_disable(int pad_id)
 {
 	unsigned long request, status;
 	unsigned int bit;
@@ -945,9 +993,9 @@ int tegra_io_rail_power_off(unsigned int id)
 
 	mutex_lock(&pmc->powergates_lock);
 
-	err = tegra_io_rail_prepare(id, &request, &status, &bit);
+	err = tegra_io_pad_dpd_prepare(pad_id, &request, &status, &bit);
 	if (err) {
-		pr_info("tegra_io_rail_prepare() failed: %d\n", err);
+		pr_info("tegra_io_pad_dpd_prepare() failed: %d\n", err);
 		goto error;
 	}
 
@@ -959,18 +1007,77 @@ int tegra_io_rail_power_off(unsigned int id)
 	value |= IO_DPD_REQ_CODE_ON;
 	tegra_pmc_writel(value, request);
 
-	err = tegra_io_rail_poll(status, mask, mask, 250);
+	err = tegra_io_pad_dpd_poll(status, mask, mask, 250);
 	if (err)
 		goto error;
 
-	tegra_io_rail_unprepare();
+	tegra_io_pad_dpd_unprepare();
 
 error:
 	mutex_unlock(&pmc->powergates_lock);
 
 	return err;
 }
-EXPORT_SYMBOL(tegra_io_rail_power_off);
+EXPORT_SYMBOL(tegra_io_pads_power_disable);
+
+int tegra_io_pads_power_is_enabled(int io_pad_id)
+{
+	unsigned long status_reg;
+	u32 status;
+	int dpd_bit;
+
+	dpd_bit = tegra_io_pads_to_dpd_bit(pmc->soc, io_pad_id);
+	if (dpd_bit < 0)
+		return dpd_bit;
+
+	status_reg = (dpd_bit < 32) ? IO_DPD_STATUS : IO_DPD2_STATUS;
+	status = tegra_pmc_readl(status_reg);
+
+	return !!(status & BIT(dpd_bit % 32));
+}
+EXPORT_SYMBOL(tegra_io_pads_power_is_enabled);
+
+int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
+{
+	int pwr_bit;
+	u32 pwr_mask;
+	u32 bval;
+
+	if ((io_volt_uv != 3300000) && (io_volt_uv != 1800000))
+		return -EINVAL;
+
+	pwr_bit = tegra_io_pads_to_power_bit(pmc->soc, io_pad_id);
+	if (pwr_bit < 0)
+		return pwr_bit;
+
+	pwr_mask = BIT(pwr_bit);
+	bval = (io_volt_uv == 3300000) ? pwr_mask : 0;
+
+	mutex_lock(&pmc->powergates_lock);
+	tegra_pmc_read_modify_write(PMC_PWR_DET, pwr_mask, pwr_mask);
+	tegra_pmc_read_modify_write(PMC_PWR_DET_VAL, pwr_mask, bval);
+	mutex_unlock(&pmc->powergates_lock);
+
+	usleep_range(100, 250);
+
+	return 0;
+}
+EXPORT_SYMBOL(tegra_io_pads_configure_voltage);
+
+int tegra_io_pads_get_configured_voltage(int io_pad_id)
+{
+	int pwr_bit;
+	u32 pwr_det_val;
+
+	pwr_bit = tegra_io_pads_to_power_bit(pmc->soc, io_pad_id);
+	if (pwr_bit < 0)
+		return pwr_bit;
+
+	pwr_det_val = tegra_pmc_readl(PMC_PWR_DET_VAL);
+
+	return (pwr_det_val & BIT(pwr_bit)) ? 3300000 : 1800000;
+}
+EXPORT_SYMBOL(tegra_io_pads_get_configured_voltage);
 
 #ifdef CONFIG_PM_SLEEP
 enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void)
@@ -1397,11 +1504,53 @@ static const u8 tegra124_cpu_powergates[] = {
 	TEGRA_POWERGATE_CPU3,
 };
 
+#define TEGRA_IO_PADS_CONTROL(_pad, _dpd, _pwr)		\
+{							\
+	.pad_id = (TEGRA_IO_PAD_##_pad),		\
+	.dpd_bit_pos = (_dpd),				\
+	.pwr_bit_pos = (_pwr),				\
+}
+
+struct tegra_io_pads_control tegra124_io_pads_control[] = {
+	TEGRA_IO_PADS_CONTROL(CSIA, 0, -1),
+	TEGRA_IO_PADS_CONTROL(CSIB, 1, -1),
+	TEGRA_IO_PADS_CONTROL(DSI, 2, -1),
+	TEGRA_IO_PADS_CONTROL(MIPI_BIAS, 3, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_BIAS, 4, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_CLK1, 5, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_CLK2, 6, -1),
+	TEGRA_IO_PADS_CONTROL(USB0, 9, -1),
+	TEGRA_IO_PADS_CONTROL(USB1, 10, -1),
+	TEGRA_IO_PADS_CONTROL(USB2, 11, -1),
+	TEGRA_IO_PADS_CONTROL(USB_BIAS, 12, -1),
+	TEGRA_IO_PADS_CONTROL(NAND, 13, -1),
+	TEGRA_IO_PADS_CONTROL(UART, 14, -1),
+	TEGRA_IO_PADS_CONTROL(BB, 15, -1),
+	TEGRA_IO_PADS_CONTROL(AUDIO, 17, -1),
+	TEGRA_IO_PADS_CONTROL(HSIC, 19, -1),
+	TEGRA_IO_PADS_CONTROL(COMP, 22, -1),
+	TEGRA_IO_PADS_CONTROL(HDMI, 28, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_CNTRL, 32, -1),
+	TEGRA_IO_PADS_CONTROL(SDMMC1, 33, -1),
+	TEGRA_IO_PADS_CONTROL(SDMMC3, 34, -1),
+	TEGRA_IO_PADS_CONTROL(SDMMC4, 35, -1),
+	TEGRA_IO_PADS_CONTROL(CAM, 36, -1),
+	TEGRA_IO_PADS_CONTROL(HV, 38, -1),
+	TEGRA_IO_PADS_CONTROL(DSIB, 39, -1),
+	TEGRA_IO_PADS_CONTROL(DSIC, 40, -1),
+	TEGRA_IO_PADS_CONTROL(DSID, 41, -1),
+	TEGRA_IO_PADS_CONTROL(CSIE, 44, -1),
+	TEGRA_IO_PADS_CONTROL(LVDS, 57, -1),
+	TEGRA_IO_PADS_CONTROL(SYS_DDC, 58, -1),
+};
+
 static const struct tegra_pmc_soc tegra124_pmc_soc = {
 	.num_powergates = ARRAY_SIZE(tegra124_powergates),
 	.powergates = tegra124_powergates,
 	.num_cpu_powergates = ARRAY_SIZE(tegra124_cpu_powergates),
 	.cpu_powergates = tegra124_cpu_powergates,
+	.io_pads_control = tegra124_io_pads_control,
+	.num_io_pads = ARRAY_SIZE(tegra124_io_pads_control),
 	.has_tsense_reset = true,
 	.has_gpu_clamps = true,
 };
@@ -1440,11 +1589,52 @@ static const u8 tegra210_cpu_powergates[] = {
 	TEGRA_POWERGATE_CPU3,
 };
 
+struct tegra_io_pads_control tegra210_io_pads_control[] = {
+	TEGRA_IO_PADS_CONTROL(CSIA, 0, -1),
+	TEGRA_IO_PADS_CONTROL(CSIB, 1, -1),
+	TEGRA_IO_PADS_CONTROL(DSI, 2, -1),
+	TEGRA_IO_PADS_CONTROL(MIPI_BIAS, 3, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_BIAS, 4, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_CLK1, 5, -1),
+	TEGRA_IO_PADS_CONTROL(PEX_CLK2, 6, -1),
+	TEGRA_IO_PADS_CONTROL(USB0, 9, -1),
+	TEGRA_IO_PADS_CONTROL(USB1, 10, -1),
+	TEGRA_IO_PADS_CONTROL(USB2, 11, -1),
+	TEGRA_IO_PADS_CONTROL(USB_BIAS, 12, -1),
+	TEGRA_IO_PADS_CONTROL(UART, 14, -1),
+	TEGRA_IO_PADS_CONTROL(AUDIO, 17, -1),
+	TEGRA_IO_PADS_CONTROL(USB3, 18, -1),
+	TEGRA_IO_PADS_CONTROL(HSIC, 19, -1),
+	TEGRA_IO_PADS_CONTROL(DBG, 25, -1),
+	TEGRA_IO_PADS_CONTROL(DEBUG_NONAO, 26, -1),
+	TEGRA_IO_PADS_CONTROL(GPIO, 27, 21),
+	TEGRA_IO_PADS_CONTROL(HDMI, 28, -1),
+	TEGRA_IO_PADS_CONTROL(SDMMC1, 33, 12),
+	TEGRA_IO_PADS_CONTROL(SDMMC3, 34, 13),
+	TEGRA_IO_PADS_CONTROL(EMMC, 35, -1),
+	TEGRA_IO_PADS_CONTROL(CAM, 36, -1),
+	TEGRA_IO_PADS_CONTROL(EMMC2, 37, -1),
+	TEGRA_IO_PADS_CONTROL(DSIB, 39, -1),
+	TEGRA_IO_PADS_CONTROL(DSIC, 40, -1),
+	TEGRA_IO_PADS_CONTROL(DSID, 41, -1),
+	TEGRA_IO_PADS_CONTROL(CSIC, 42, -1),
+	TEGRA_IO_PADS_CONTROL(CSID, 43, -1),
+	TEGRA_IO_PADS_CONTROL(CSIE, 44, -1),
+	TEGRA_IO_PADS_CONTROL(CSIF, 45, -1),
+	TEGRA_IO_PADS_CONTROL(SPI, 46, -1),
+	TEGRA_IO_PADS_CONTROL(SPI_HV, 47, 23),
+	TEGRA_IO_PADS_CONTROL(DMIC, 50, -1),
+	TEGRA_IO_PADS_CONTROL(DP, 51, -1),
+	TEGRA_IO_PADS_CONTROL(LVDS, 57, -1),
+	TEGRA_IO_PADS_CONTROL(AUDIO_HV, 61, 18),
+};
 static const struct tegra_pmc_soc tegra210_pmc_soc = {
 	.num_powergates = ARRAY_SIZE(tegra210_powergates),
 	.powergates = tegra210_powergates,
 	.num_cpu_powergates = ARRAY_SIZE(tegra210_cpu_powergates),
 	.cpu_powergates = tegra210_cpu_powergates,
+	.io_pads_control = tegra210_io_pads_control,
+	.num_io_pads = ARRAY_SIZE(tegra210_io_pads_control),
 	.has_tsense_reset = true,
 	.has_gpu_clamps = true,
 };
diff --git a/include/soc/tegra/pmc.h b/include/soc/tegra/pmc.h
index e9e5347..763bd6d 100644
--- a/include/soc/tegra/pmc.h
+++ b/include/soc/tegra/pmc.h
@@ -76,37 +76,57 @@ int tegra_pmc_cpu_remove_clamping(unsigned int cpuid);
 
 #define TEGRA_POWERGATE_3D0	TEGRA_POWERGATE_3D
 
-#define TEGRA_IO_RAIL_CSIA	0
-#define TEGRA_IO_RAIL_CSIB	1
-#define TEGRA_IO_RAIL_DSI	2
-#define TEGRA_IO_RAIL_MIPI_BIAS	3
-#define TEGRA_IO_RAIL_PEX_BIAS	4
-#define TEGRA_IO_RAIL_PEX_CLK1	5
-#define TEGRA_IO_RAIL_PEX_CLK2	6
-#define TEGRA_IO_RAIL_USB0	9
-#define TEGRA_IO_RAIL_USB1	10
-#define TEGRA_IO_RAIL_USB2	11
-#define TEGRA_IO_RAIL_USB_BIAS	12
-#define TEGRA_IO_RAIL_NAND	13
-#define TEGRA_IO_RAIL_UART	14
-#define TEGRA_IO_RAIL_BB	15
-#define TEGRA_IO_RAIL_AUDIO	17
-#define TEGRA_IO_RAIL_HSIC	19
-#define TEGRA_IO_RAIL_COMP	22
-#define TEGRA_IO_RAIL_HDMI	28
-#define TEGRA_IO_RAIL_PEX_CNTRL	32
-#define TEGRA_IO_RAIL_SDMMC1	33
-#define TEGRA_IO_RAIL_SDMMC3	34
-#define TEGRA_IO_RAIL_SDMMC4	35
-#define TEGRA_IO_RAIL_CAM	36
-#define TEGRA_IO_RAIL_RES	37
-#define TEGRA_IO_RAIL_HV	38
-#define TEGRA_IO_RAIL_DSIB	39
-#define TEGRA_IO_RAIL_DSIC	40
-#define TEGRA_IO_RAIL_DSID	41
-#define TEGRA_IO_RAIL_CSIE	44
-#define TEGRA_IO_RAIL_LVDS	57
-#define TEGRA_IO_RAIL_SYS_DDC	58
+/* TEGRA_IO_PAD: The IO pins of Tegra SoCs are grouped for common control
+ * of IO interface like setting voltage signal levels, power state of the
+ * interface. The group is generally referred as io-pads. The power and
+ * voltage control of IO pins are available at io-pads level.
+ * The following macros make the super list all IO pads found on Tegra SoC
+ * generations.
+ */
+#define TEGRA_IO_PAD_AUDIO		0
+#define TEGRA_IO_PAD_AUDIO_HV		1
+#define TEGRA_IO_PAD_BB			2
+#define TEGRA_IO_PAD_CAM		3
+#define TEGRA_IO_PAD_COMP		4
+#define TEGRA_IO_PAD_CSIA		5
+#define TEGRA_IO_PAD_CSIB		6
+#define TEGRA_IO_PAD_CSIC		7
+#define TEGRA_IO_PAD_CSID		8
+#define TEGRA_IO_PAD_CSIE		9
+#define TEGRA_IO_PAD_CSIF		10
+#define TEGRA_IO_PAD_DBG		11
+#define TEGRA_IO_PAD_DEBUG_NONAO	12
+#define TEGRA_IO_PAD_DMIC		13
+#define TEGRA_IO_PAD_DP			14
+#define TEGRA_IO_PAD_DSI		15
+#define TEGRA_IO_PAD_DSIB		16
+#define TEGRA_IO_PAD_DSIC		17
+#define TEGRA_IO_PAD_DSID		18
+#define TEGRA_IO_PAD_EMMC		19
+#define TEGRA_IO_PAD_EMMC2		20
+#define TEGRA_IO_PAD_GPIO		21
+#define TEGRA_IO_PAD_HDMI		22
+#define TEGRA_IO_PAD_HSIC		23
+#define TEGRA_IO_PAD_HV			24
+#define TEGRA_IO_PAD_LVDS		25
+#define TEGRA_IO_PAD_MIPI_BIAS		26
+#define TEGRA_IO_PAD_NAND		27
+#define TEGRA_IO_PAD_PEX_BIAS		28
+#define TEGRA_IO_PAD_PEX_CLK1		29
+#define TEGRA_IO_PAD_PEX_CLK2		30
+#define TEGRA_IO_PAD_PEX_CNTRL		31
+#define TEGRA_IO_PAD_SDMMC1		32
+#define TEGRA_IO_PAD_SDMMC3		33
+#define TEGRA_IO_PAD_SDMMC4		34
+#define TEGRA_IO_PAD_SPI		35
+#define TEGRA_IO_PAD_SPI_HV		36
+#define TEGRA_IO_PAD_SYS_DDC		37
+#define TEGRA_IO_PAD_UART		38
+#define TEGRA_IO_PAD_USB0		39
+#define TEGRA_IO_PAD_USB1		40
+#define TEGRA_IO_PAD_USB2		41
+#define TEGRA_IO_PAD_USB3		42
+#define TEGRA_IO_PAD_USB_BIAS		43
 
 #ifdef CONFIG_ARCH_TEGRA
 int tegra_powergate_is_powered(unsigned int id);
@@ -118,8 +138,15 @@ int tegra_powergate_remove_clamping(unsigned int id);
 int tegra_powergate_sequence_power_up(unsigned int id, struct clk *clk,
 				      struct reset_control *rst);
 
-int tegra_io_rail_power_on(unsigned int id);
-int tegra_io_rail_power_off(unsigned int id);
+/* Power enable/disable of the IO pads */
+int tegra_io_pads_power_enable(int io_pad_id);
+int tegra_io_pads_power_disable(int io_pad_id);
+int tegra_io_pads_power_is_enabled(int io_pad_id);
+
+/* Set/Get of IO pad voltage */
+int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv);
+int tegra_io_pads_get_configured_voltage(int io_pad_id);
+
 #else
 static inline int tegra_powergate_is_powered(unsigned int id)
 {
@@ -148,14 +175,29 @@ static inline int tegra_powergate_sequence_power_up(unsigned int id,
 	return -ENOSYS;
 }
 
-static inline int tegra_io_rail_power_on(unsigned int id)
+static inline int tegra_io_pads_power_enable(int io_pad_id)
 {
-	return -ENOSYS;
+	return -ENOTSUPP;
 }
 
-static inline int tegra_io_rail_power_off(unsigned int id)
+static inline int tegra_io_pads_power_disable(int io_pad_id)
 {
-	return -ENOSYS;
+	return -ENOTSUPP;
+}
+
+static inline int tegra_io_pads_power_is_enabled(int io_pad_id)
+{
+	return -ENOTSUPP;
+}
+
+static inline int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
+{
+	return -ENOTSUPP;
+}
+
+static inline int tegra_io_pads_get_configured_voltage(int io_pad_id)
+{
+	return -ENOTSUPP;
 }
 #endif /* CONFIG_ARCH_TEGRA */
 
-- 
2.1.4

[PATCH V3 4/4] soc/tegra: pmc: Register PMC child devices as platform device

[Laxman Dewangan]

Power Management Controller(PMC) of Tegra does the multiple chip
power related functionality for internal and IO interfacing.
Some of the functionalities are power gating of IP blocks, IO pads
voltage and power state configuration, system power state configurations,
wakeup controls etc.

Different functionalities of the PMC are provided through different
framework like IO pads control can be provided through pinctrl framework,
IO power control is via misc driver etc. All sub functionalities are
represented as PMC child devices.

Register the PMC child devices as platform device and fill the child
devices table for Tegra124 and Tegra210.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

---
Changes from V1:
Reworked on DT for having flat entry and register all child devices
as simple platform device instead of of_populate_device().

Changes from V2:
- Handling of the release of child devices on error.
- Register sub devices for T124 and T210 as T124 supports the DPD of IO
  pads.
---
 drivers/soc/tegra/pmc.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 80 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 0611cea..0eb652f 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -132,6 +132,8 @@ struct tegra_pmc_soc {
 	const u8 *cpu_powergates;
 	const struct tegra_io_pads_control *io_pads_control;
 	unsigned int num_io_pads;
+	const char **sub_devs_name;
+	unsigned int num_sub_devs;
 	bool has_tsense_reset;
 	bool has_gpu_clamps;
 };
@@ -158,6 +160,7 @@ struct tegra_pmc_soc {
  * @lp0_vec_size: size of the LP0 warm boot code
  * @powergates_available: Bitmap of available power gates
  * @powergates_lock: mutex for power gate register access
+ * @pdevs: Platform device for PMC child devices.
  */
 struct tegra_pmc {
 	struct device *dev;
@@ -184,6 +187,8 @@ struct tegra_pmc {
 	DECLARE_BITMAP(powergates_available, TEGRA_POWERGATE_MAX);
 
 	struct mutex powergates_lock;
+
+	struct platform_device **pdevs;
 };
 
 static struct tegra_pmc *pmc = &(struct tegra_pmc) {
@@ -1310,6 +1315,57 @@ out:
 	of_node_put(np);
 }
 
+static int  tegra_pmc_init_sub_devs(struct tegra_pmc *pmc)
+{
+	int ret, i;
+
+	if (!pmc->soc->num_sub_devs)
+		return 0;
+
+	pmc->pdevs = devm_kzalloc(pmc->dev, pmc->soc->num_sub_devs *
+				  sizeof(**pmc->pdevs), GFP_KERNEL);
+	if (!pmc->pdevs)
+		return -ENOMEM;
+
+	for (i = 0; i < pmc->soc->num_sub_devs; ++i) {
+		pmc->pdevs[i] = platform_device_register_data(pmc->dev,
+						pmc->soc->sub_devs_name[i],
+						0, NULL, 0);
+		if (IS_ERR(pmc->pdevs[i])) {
+			ret = PTR_ERR(pmc->pdevs[i]);
+			dev_err(pmc->dev,
+				"Failed to register platform device for %s: %d\n",
+				pmc->soc->sub_devs_name[i], ret);
+			goto pdev_cleanups;
+		}
+	}
+
+	return 0;
+
+pdev_cleanups:
+	while (--i >= 0) {
+		platform_device_unregister(pmc->pdevs[i]);
+		pmc->pdevs[i] = NULL;
+	}
+
+	return ret;
+}
+
+static void tegra_pmc_deinit_sub_devs(struct tegra_pmc *pmc)
+{
+	int i;
+
+	if (!pmc->soc->num_sub_devs || !pmc->pdevs)
+		return;
+
+	for (i = 0; i < pmc->soc->num_sub_devs; ++i) {
+		if (pmc->pdevs[i]) {
+			platform_device_unregister(pmc->pdevs[i]);
+			pmc->pdevs[i] = NULL;
+		}
+	}
+}
+
 static int tegra_pmc_probe(struct platform_device *pdev)
 {
 	void __iomem *base;
@@ -1339,10 +1395,14 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 
 	tegra_pmc_init_tsense_reset(pmc);
 
+	err = tegra_pmc_init_sub_devs(pmc);
+	if (err < 0)
+		dev_warn(pmc->dev, "Failed to register sub devices: %d\n", err);
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
 		err = tegra_powergate_debugfs_init();
 		if (err < 0)
-			return err;
+			goto cleanups;
 	}
 
 	err = register_restart_handler(&tegra_pmc_restart_handler);
@@ -1350,7 +1410,7 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 		debugfs_remove(pmc->debugfs);
 		dev_err(&pdev->dev, "unable to register restart handler, %d\n",
 			err);
-		return err;
+		goto cleanups;
 	}
 
 	tegra_powergate_init(pmc);
@@ -1361,6 +1421,11 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 	mutex_unlock(&pmc->powergates_lock);
 
 	return 0;
+
+cleanups:
+	tegra_pmc_deinit_sub_devs(pmc);
+
+	return err;
 }
 
 #if defined(CONFIG_PM_SLEEP) && defined(CONFIG_ARM)
@@ -1544,6 +1609,10 @@ struct tegra_io_pads_control tegra124_io_pads_control[] = {
 	TEGRA_IO_PADS_CONTROL(SYS_DDC, 58, -1),
 };
 
+static const char *tegra124_sub_devs_name[] = {
+	"pinctrl-tegra124-io-pad",
+};
+
 static const struct tegra_pmc_soc tegra124_pmc_soc = {
 	.num_powergates = ARRAY_SIZE(tegra124_powergates),
 	.powergates = tegra124_powergates,
@@ -1551,6 +1620,8 @@ static const struct tegra_pmc_soc tegra124_pmc_soc = {
 	.cpu_powergates = tegra124_cpu_powergates,
 	.io_pads_control = tegra124_io_pads_control,
 	.num_io_pads = ARRAY_SIZE(tegra124_io_pads_control),
+	.sub_devs_name = tegra124_sub_devs_name,
+	.num_sub_devs = ARRAY_SIZE(tegra124_sub_devs_name),
 	.has_tsense_reset = true,
 	.has_gpu_clamps = true,
 };
@@ -1628,6 +1699,11 @@ struct tegra_io_pads_control tegra210_io_pads_control[] = {
 	TEGRA_IO_PADS_CONTROL(LVDS, 57, -1),
 	TEGRA_IO_PADS_CONTROL(AUDIO_HV, 61, 18),
 };
+
+static const char *tegra210_sub_devs_name[] = {
+	"pinctrl-tegra210-io-pad",
+};
+
 static const struct tegra_pmc_soc tegra210_pmc_soc = {
 	.num_powergates = ARRAY_SIZE(tegra210_powergates),
 	.powergates = tegra210_powergates,
@@ -1635,6 +1711,8 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
 	.cpu_powergates = tegra210_cpu_powergates,
 	.io_pads_control = tegra210_io_pads_control,
 	.num_io_pads = ARRAY_SIZE(tegra210_io_pads_control),
+	.sub_devs_name = tegra210_sub_devs_name,
+	.num_sub_devs = ARRAY_SIZE(tegra210_sub_devs_name),
 	.has_tsense_reset = true,
 	.has_gpu_clamps = true,
 };
-- 
2.1.4

[PATCH V3 4/4] soc/tegra: pmc: Register PMC child devices as platform device

[Laxman Dewangan]

Power Management Controller(PMC) of Tegra does the multiple chip
power related functionality for internal and IO interfacing.
Some of the functionalities are power gating of IP blocks, IO pads
voltage and power state configuration, system power state configurations,
wakeup controls etc.

Different functionalities of the PMC are provided through different
framework like IO pads control can be provided through pinctrl framework,
IO power control is via misc driver etc. All sub functionalities are
represented as PMC child devices.

Register the PMC child devices as platform device and fill the child
devices table for Tegra124 and Tegra210.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

---
Changes from V1:
Reworked on DT for having flat entry and register all child devices
as simple platform device instead of of_populate_device().

Changes from V2:
- Handling of the release of child devices on error.
- Register sub devices for T124 and T210 as T124 supports the DPD of IO
  pads.
---
 drivers/soc/tegra/pmc.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 80 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 0611cea..0eb652f 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -132,6 +132,8 @@ struct tegra_pmc_soc {
 	const u8 *cpu_powergates;
 	const struct tegra_io_pads_control *io_pads_control;
 	unsigned int num_io_pads;
+	const char **sub_devs_name;
+	unsigned int num_sub_devs;
 	bool has_tsense_reset;
 	bool has_gpu_clamps;
 };
@@ -158,6 +160,7 @@ struct tegra_pmc_soc {
  * @lp0_vec_size: size of the LP0 warm boot code
  * @powergates_available: Bitmap of available power gates
  * @powergates_lock: mutex for power gate register access
+ * @pdevs: Platform device for PMC child devices.
  */
 struct tegra_pmc {
 	struct device *dev;
@@ -184,6 +187,8 @@ struct tegra_pmc {
 	DECLARE_BITMAP(powergates_available, TEGRA_POWERGATE_MAX);
 
 	struct mutex powergates_lock;
+
+	struct platform_device **pdevs;
 };
 
 static struct tegra_pmc *pmc = &(struct tegra_pmc) {
@@ -1310,6 +1315,57 @@ out:
 	of_node_put(np);
 }
 
+static int  tegra_pmc_init_sub_devs(struct tegra_pmc *pmc)
+{
+	int ret, i;
+
+	if (!pmc->soc->num_sub_devs)
+		return 0;
+
+	pmc->pdevs = devm_kzalloc(pmc->dev, pmc->soc->num_sub_devs *
+				  sizeof(**pmc->pdevs), GFP_KERNEL);
+	if (!pmc->pdevs)
+		return -ENOMEM;
+
+	for (i = 0; i < pmc->soc->num_sub_devs; ++i) {
+		pmc->pdevs[i] = platform_device_register_data(pmc->dev,
+						pmc->soc->sub_devs_name[i],
+						0, NULL, 0);
+		if (IS_ERR(pmc->pdevs[i])) {
+			ret = PTR_ERR(pmc->pdevs[i]);
+			dev_err(pmc->dev,
+				"Failed to register platform device for %s: %d\n",
+				pmc->soc->sub_devs_name[i], ret);
+			goto pdev_cleanups;
+		}
+	}
+
+	return 0;
+
+pdev_cleanups:
+	while (--i >= 0) {
+		platform_device_unregister(pmc->pdevs[i]);
+		pmc->pdevs[i] = NULL;
+	}
+
+	return ret;
+}
+
+static void tegra_pmc_deinit_sub_devs(struct tegra_pmc *pmc)
+{
+	int i;
+
+	if (!pmc->soc->num_sub_devs || !pmc->pdevs)
+		return;
+
+	for (i = 0; i < pmc->soc->num_sub_devs; ++i) {
+		if (pmc->pdevs[i]) {
+			platform_device_unregister(pmc->pdevs[i]);
+			pmc->pdevs[i] = NULL;
+		}
+	}
+}
+
 static int tegra_pmc_probe(struct platform_device *pdev)
 {
 	void __iomem *base;
@@ -1339,10 +1395,14 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 
 	tegra_pmc_init_tsense_reset(pmc);
 
+	err = tegra_pmc_init_sub_devs(pmc);
+	if (err < 0)
+		dev_warn(pmc->dev, "Failed to register sub devices: %d\n", err);
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
 		err = tegra_powergate_debugfs_init();
 		if (err < 0)
-			return err;
+			goto cleanups;
 	}
 
 	err = register_restart_handler(&tegra_pmc_restart_handler);
@@ -1350,7 +1410,7 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 		debugfs_remove(pmc->debugfs);
 		dev_err(&pdev->dev, "unable to register restart handler, %d\n",
 			err);
-		return err;
+		goto cleanups;
 	}
 
 	tegra_powergate_init(pmc);
@@ -1361,6 +1421,11 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 	mutex_unlock(&pmc->powergates_lock);
 
 	return 0;
+
+cleanups:
+	tegra_pmc_deinit_sub_devs(pmc);
+
+	return err;
 }
 
 #if defined(CONFIG_PM_SLEEP) && defined(CONFIG_ARM)
@@ -1544,6 +1609,10 @@ struct tegra_io_pads_control tegra124_io_pads_control[] = {
 	TEGRA_IO_PADS_CONTROL(SYS_DDC, 58, -1),
 };
 
+static const char *tegra124_sub_devs_name[] = {
+	"pinctrl-tegra124-io-pad",
+};
+
 static const struct tegra_pmc_soc tegra124_pmc_soc = {
 	.num_powergates = ARRAY_SIZE(tegra124_powergates),
 	.powergates = tegra124_powergates,
@@ -1551,6 +1620,8 @@ static const struct tegra_pmc_soc tegra124_pmc_soc = {
 	.cpu_powergates = tegra124_cpu_powergates,
 	.io_pads_control = tegra124_io_pads_control,
 	.num_io_pads = ARRAY_SIZE(tegra124_io_pads_control),
+	.sub_devs_name = tegra124_sub_devs_name,
+	.num_sub_devs = ARRAY_SIZE(tegra124_sub_devs_name),
 	.has_tsense_reset = true,
 	.has_gpu_clamps = true,
 };
@@ -1628,6 +1699,11 @@ struct tegra_io_pads_control tegra210_io_pads_control[] = {
 	TEGRA_IO_PADS_CONTROL(LVDS, 57, -1),
 	TEGRA_IO_PADS_CONTROL(AUDIO_HV, 61, 18),
 };
+
+static const char *tegra210_sub_devs_name[] = {
+	"pinctrl-tegra210-io-pad",
+};
+
 static const struct tegra_pmc_soc tegra210_pmc_soc = {
 	.num_powergates = ARRAY_SIZE(tegra210_powergates),
 	.powergates = tegra210_powergates,
@@ -1635,6 +1711,8 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
 	.cpu_powergates = tegra210_cpu_powergates,
 	.io_pads_control = tegra210_io_pads_control,
 	.num_io_pads = ARRAY_SIZE(tegra210_io_pads_control),
+	.sub_devs_name = tegra210_sub_devs_name,
+	.num_sub_devs = ARRAY_SIZE(tegra210_sub_devs_name),
 	.has_tsense_reset = true,
 	.has_gpu_clamps = true,
 };
-- 
2.1.4

Re: [PATCH V3 2/4] soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()

[Jon Hunter]

On 04/05/16 12:39, Laxman Dewangan wrote:
> The function tegra_pmc_readl() returns the u32 type data and hence
> change the data type of variable where this data is stored to u32
> type.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> 
> ---
> Changes from V1:
> -This is new in series as per discussion on V1 series to use u32 for
> tegra_pmc_readl.
> 
> Changes from V2:
> - Make unsigned long to u32 for some missed variable from V1.
> ---
>  drivers/soc/tegra/pmc.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 2c3f1f9..eff9425 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -844,7 +844,8 @@ static void tegra_powergate_init(struct tegra_pmc *pmc)
>  static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
>  				 unsigned long *status, unsigned int *bit)
>  {
> -	unsigned long rate, value;
> +	unsigned long rate;
> +	u32 value;
>  
>  	*bit = id % 32;
>  
> @@ -868,17 +869,18 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
>  	tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
>  
>  	/* must be at least 200 ns, in APB (PCLK) clock cycles */
> -	value = DIV_ROUND_UP(1000000000, rate);
> -	value = DIV_ROUND_UP(200, value);
> +	rate = DIV_ROUND_UP(1000000000, rate);
> +	rate = DIV_ROUND_UP(200, rate);
> +	value = (u32)rate;

Although it is unlikely, I think that we should check it is less
than U32_MAX, return an error if it is not.

Cheers
Jon
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V3 2/4] soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()

[Jon Hunter]

On 04/05/16 12:39, Laxman Dewangan wrote:
> The function tegra_pmc_readl() returns the u32 type data and hence
> change the data type of variable where this data is stored to u32
> type.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> 
> ---
> Changes from V1:
> -This is new in series as per discussion on V1 series to use u32 for
> tegra_pmc_readl.
> 
> Changes from V2:
> - Make unsigned long to u32 for some missed variable from V1.
> ---
>  drivers/soc/tegra/pmc.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 2c3f1f9..eff9425 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -844,7 +844,8 @@ static void tegra_powergate_init(struct tegra_pmc *pmc)
>  static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
>  				 unsigned long *status, unsigned int *bit)
>  {
> -	unsigned long rate, value;
> +	unsigned long rate;
> +	u32 value;
>  
>  	*bit = id % 32;
>  
> @@ -868,17 +869,18 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
>  	tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
>  
>  	/* must be at least 200 ns, in APB (PCLK) clock cycles */
> -	value = DIV_ROUND_UP(1000000000, rate);
> -	value = DIV_ROUND_UP(200, value);
> +	rate = DIV_ROUND_UP(1000000000, rate);
> +	rate = DIV_ROUND_UP(200, rate);
> +	value = (u32)rate;

Although it is unlikely, I think that we should check it is less
than U32_MAX, return an error if it is not.

Cheers
Jon

Re: [PATCH V3 2/4] soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()

[Laxman Dewangan]

On Thursday 05 May 2016 03:19 PM, Jon Hunter wrote:
> On 04/05/16 12:39, Laxman Dewangan wrote:
>> The function tegra_pmc_readl() returns the u32 type data and hence
>> change the data type of variable where this data is stored to u32
>> type.
>>
>> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>
>> ---
>> Changes from V1:
>> -This is new in series as per discussion on V1 series to use u32 for
>> tegra_pmc_readl.
>>
>> Changes from V2:
>> - Make unsigned long to u32 for some missed variable from V1.
>> ---
>>   drivers/soc/tegra/pmc.c | 24 ++++++++++++++----------
>>   1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index 2c3f1f9..eff9425 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -844,7 +844,8 @@ static void tegra_powergate_init(struct tegra_pmc *pmc)
>>   static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
>>   				 unsigned long *status, unsigned int *bit)
>>   {
>> -	unsigned long rate, value;
>> +	unsigned long rate;
>> +	u32 value;
>>   
>>   	*bit = id % 32;
>>   
>> @@ -868,17 +869,18 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
>>   	tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
>>   
>>   	/* must be at least 200 ns, in APB (PCLK) clock cycles */
>> -	value = DIV_ROUND_UP(1000000000, rate);
>> -	value = DIV_ROUND_UP(200, value);
>> +	rate = DIV_ROUND_UP(1000000000, rate);
>> +	rate = DIV_ROUND_UP(200, rate);
>> +	value = (u32)rate;
> Although it is unlikely, I think that we should check it is less
> than U32_MAX, return an error if it is not.

rate = DIV_ROUNC_UP(200, rate) means

rate = (200 + rate -1)/rate

and can not be more than 200 in any case (if rate =1).
So no need of the error check.

Re: [PATCH V3 2/4] soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()

[Laxman Dewangan]

On Thursday 05 May 2016 03:19 PM, Jon Hunter wrote:
> On 04/05/16 12:39, Laxman Dewangan wrote:
>> The function tegra_pmc_readl() returns the u32 type data and hence
>> change the data type of variable where this data is stored to u32
>> type.
>>
>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>>
>> ---
>> Changes from V1:
>> -This is new in series as per discussion on V1 series to use u32 for
>> tegra_pmc_readl.
>>
>> Changes from V2:
>> - Make unsigned long to u32 for some missed variable from V1.
>> ---
>>   drivers/soc/tegra/pmc.c | 24 ++++++++++++++----------
>>   1 file changed, 14 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index 2c3f1f9..eff9425 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -844,7 +844,8 @@ static void tegra_powergate_init(struct tegra_pmc *pmc)
>>   static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
>>   				 unsigned long *status, unsigned int *bit)
>>   {
>> -	unsigned long rate, value;
>> +	unsigned long rate;
>> +	u32 value;
>>   
>>   	*bit = id % 32;
>>   
>> @@ -868,17 +869,18 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
>>   	tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
>>   
>>   	/* must be at least 200 ns, in APB (PCLK) clock cycles */
>> -	value = DIV_ROUND_UP(1000000000, rate);
>> -	value = DIV_ROUND_UP(200, value);
>> +	rate = DIV_ROUND_UP(1000000000, rate);
>> +	rate = DIV_ROUND_UP(200, rate);
>> +	value = (u32)rate;
> Although it is unlikely, I think that we should check it is less
> than U32_MAX, return an error if it is not.

rate = DIV_ROUNC_UP(200, rate) means

rate = (200 + rate -1)/rate

and can not be more than 200 in any case (if rate =1).
So no need of the error check.

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Jon Hunter]

On 04/05/16 12:39, Laxman Dewangan wrote:
> The IO pins of Tegra SoCs are grouped for common control of IO
> interface like setting voltage signal levels and power state of
> the interface. The group is generally referred as IO pads. The
> power state and voltage control of IO pins can be done at IO pads
> level.
> 
> Tegra generation SoC supports the power down of IO pads when it
> is not used even in the active state of system. This saves power
> from that IO interface. Also it supports multiple voltage level
> in IO pins for interfacing on some of pads. The IO pad voltage is
> automatically detected till T124, hence SW need not to configure
> this. But from T210, the automatically detection logic has been
> removed, hence SW need to explicitly set the IO pad voltage into
> IO pad configuration registers.
> 
> Add support to set the power states and voltage level of the IO pads
> from client driver. The implementation for the APIs are in generic
> which is applicable for all generation os Tegra SoC.
> 
> IO pads ID and information of bit field for power state and voltage
> level controls are added for Tegra124, Tegra132 and Tegra210. The sor
> driver is modified to use the new APIs.
> 
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> ---
> Changes from V1:
> This is reworked on earlier path to have separation between IO rails and
> io pads and add power state and voltage control APIs in single call.
> 
> Changes from V2:
> - Remove the tegra_io_rail_power_off/on() apis and change client (sor) driver
> to use the new APIs for IO pad power.
> - Remove the TEGRA_IO_RAIL_ macros.
> ---
>  drivers/gpu/drm/tegra/sor.c |   8 +-
>  drivers/soc/tegra/pmc.c     | 242 +++++++++++++++++++++++++++++++++++++++-----
>  include/soc/tegra/pmc.h     | 116 ++++++++++++++-------
>  3 files changed, 299 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index 757c6e8..1d90171 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -1134,7 +1134,7 @@ static void tegra_sor_edp_disable(struct drm_encoder *encoder)
>  			dev_err(sor->dev, "failed to disable DP: %d\n", err);
>  	}
>  
> -	err = tegra_io_rail_power_off(TEGRA_IO_RAIL_LVDS);
> +	err = tegra_io_pads_power_disable(TEGRA_IO_PAD_LVDS);
>  	if (err < 0)
>  		dev_err(sor->dev, "failed to power off I/O rail: %d\n", err);
>  
> @@ -1296,7 +1296,7 @@ static void tegra_sor_edp_enable(struct drm_encoder *encoder)
>  	tegra_sor_writel(sor, value, SOR_DP_PADCTL0);
>  
>  	/* step 2 */
> -	err = tegra_io_rail_power_on(TEGRA_IO_RAIL_LVDS);
> +	err = tegra_io_pads_power_enable(TEGRA_IO_PAD_LVDS);
>  	if (err < 0)
>  		dev_err(sor->dev, "failed to power on I/O rail: %d\n", err);
>  
> @@ -1748,7 +1748,7 @@ static void tegra_sor_hdmi_disable(struct drm_encoder *encoder)
>  	if (err < 0)
>  		dev_err(sor->dev, "failed to power down SOR: %d\n", err);
>  
> -	err = tegra_io_rail_power_off(TEGRA_IO_RAIL_HDMI);
> +	err = tegra_io_pads_power_disable(TEGRA_IO_PAD_HDMI);
>  	if (err < 0)
>  		dev_err(sor->dev, "failed to power off HDMI rail: %d\n", err);
>  
> @@ -1787,7 +1787,7 @@ static void tegra_sor_hdmi_enable(struct drm_encoder *encoder)
>  
>  	div = clk_get_rate(sor->clk) / 1000000 * 4;
>  
> -	err = tegra_io_rail_power_on(TEGRA_IO_RAIL_HDMI);
> +	err = tegra_io_pads_power_enable(TEGRA_IO_PAD_HDMI);
>  	if (err < 0)
>  		dev_err(sor->dev, "failed to power on HDMI rail: %d\n", err);
>  
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index eff9425..0611cea 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -76,6 +76,10 @@
>  
>  #define PMC_SCRATCH41			0x140
>  
> +/* Power detect for pad voltage */
> +#define PMC_PWR_DET			0x48
> +#define PMC_PWR_DET_VAL			0xe4
> +
>  #define PMC_SENSOR_CTRL			0x1b0
>  #define PMC_SENSOR_CTRL_SCRATCH_WRITE	BIT(2)
>  #define PMC_SENSOR_CTRL_ENABLE_RST	BIT(1)
> @@ -115,12 +119,19 @@ struct tegra_powergate {
>  	unsigned int num_resets;
>  };
>  
> +struct tegra_io_pads_control {
> +	int pad_id;
> +	int dpd_bit_pos;
> +	int pwr_bit_pos;
> +};
> +
>  struct tegra_pmc_soc {
>  	unsigned int num_powergates;
>  	const char *const *powergates;
>  	unsigned int num_cpu_powergates;
>  	const u8 *cpu_powergates;
> -
> +	const struct tegra_io_pads_control *io_pads_control;
> +	unsigned int num_io_pads;
>  	bool has_tsense_reset;
>  	bool has_gpu_clamps;
>  };
> @@ -196,6 +207,14 @@ static void tegra_pmc_writel(u32 value, unsigned long offset)
>  	writel(value, pmc->base + offset);
>  }
>  
> +static void tegra_pmc_read_modify_write(unsigned long offset, u32 mask, u32 val)

Nit, I would prefer something shorter such as tegra_pmc_rmw() or
tegra_pmc_set_field().

> +{
> +	u32 pmc_reg = tegra_pmc_readl(offset);
> +
> +	pmc_reg = (pmc_reg & ~mask) | (val & mask);
> +	tegra_pmc_writel(pmc_reg, offset);
> +}
> +
>  static inline bool tegra_powergate_state(int id)
>  {
>  	if (id == TEGRA_POWERGATE_3D && pmc->soc->has_gpu_clamps)
> @@ -841,22 +860,51 @@ static void tegra_powergate_init(struct tegra_pmc *pmc)
>  	of_node_put(np);
>  }
>  
> -static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
> -				 unsigned long *status, unsigned int *bit)
> +static int tegra_io_pads_to_dpd_bit(const struct tegra_pmc_soc *soc, int pad_id)

Please just use "id". This is consistent with the pmc driver and I think
it is clear from the function that this is the pad ID.

> +{
> +	int i;
> +
> +	if (!soc || !soc->num_io_pads)
> +		return -EINVAL;
> +
> +	for (i = 0; i < soc->num_io_pads; ++i) {
> +		if (soc->io_pads_control[i].pad_id == pad_id)
> +			return soc->io_pads_control[i].dpd_bit_pos;
> +	}

Do we need a loop here? Can't we just make the table a look-up table now
that the ID is just an index?

> +
> +	return -EINVAL;
> +}
> +
> +static int tegra_io_pads_to_power_bit(const struct tegra_pmc_soc *soc,
> +				      int pad_id)
> +{
> +	int i;
> +
> +	if (!soc || !soc->num_io_pads)
> +		return -EINVAL;
> +
> +	for (i = 0; i < soc->num_io_pads; ++i) {
> +		if (soc->io_pads_control[i].pad_id == pad_id)
> +			return soc->io_pads_control[i].pwr_bit_pos;
> +	}

Same here a lookup table would be better.

> +
> +	return -EINVAL;
> +}
> +
> +static int tegra_io_pad_dpd_prepare(int pad_id, unsigned long *request,
> +				    unsigned long *status, unsigned int *bit)

Some functions are tegra_io_pads_xxx and some are tegra_io_pad_xxx.
Please be consistent.

>  {
>  	unsigned long rate;
>  	u32 value;
> +	int dpd_bit;
>  
> -	*bit = id % 32;
> +	dpd_bit = tegra_io_pads_to_dpd_bit(pmc->soc, pad_id);
> +	if (dpd_bit < 0)
> +		return dpd_bit;
>  
> -	/*
> -	 * There are two sets of 30 bits to select IO rails, but bits 30 and
> -	 * 31 are control bits rather than IO rail selection bits.
> -	 */
> -	if (id > 63 || *bit == 30 || *bit == 31)
> -		return -EINVAL;
> +	*bit = dpd_bit % 32;
>  
> -	if (id < 32) {
> +	if (dpd_bit < 32) {
>  		*status = IO_DPD_STATUS;
>  		*request = IO_DPD_REQ;
>  	} else {
> @@ -877,8 +925,8 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
>  	return 0;
>  }
>  
> -static int tegra_io_rail_poll(unsigned long offset, u32 mask,
> -			      u32 val, unsigned long timeout)
> +static int tegra_io_pad_dpd_poll(unsigned long offset, u32 mask,
> +				 u32 val, unsigned long timeout)

tegra_io_pads_xxx

>  {
>  	u32 value;
>  
> @@ -895,12 +943,12 @@ static int tegra_io_rail_poll(unsigned long offset, u32 mask,
>  	return -ETIMEDOUT;
>  }
>  
> -static void tegra_io_rail_unprepare(void)
> +static void tegra_io_pad_dpd_unprepare(void)

tegra_io_pads_xxx

>  {
>  	tegra_pmc_writel(DPD_SAMPLE_DISABLE, DPD_SAMPLE);
>  }
>  
> -int tegra_io_rail_power_on(unsigned int id)
> +int tegra_io_pads_power_enable(int pad_id)

s/pad_id/id/

>  {
>  	unsigned long request, status;
>  	unsigned int bit;
> @@ -909,7 +957,7 @@ int tegra_io_rail_power_on(unsigned int id)
>  
>  	mutex_lock(&pmc->powergates_lock);
>  
> -	err = tegra_io_rail_prepare(id, &request, &status, &bit);
> +	err = tegra_io_pad_dpd_prepare(pad_id, &request, &status, &bit);
>  	if (err)
>  		goto error;
>  
> @@ -921,22 +969,22 @@ int tegra_io_rail_power_on(unsigned int id)
>  	value |= IO_DPD_REQ_CODE_OFF;
>  	tegra_pmc_writel(value, request);
>  
> -	err = tegra_io_rail_poll(status, mask, 0, 250);
> +	err = tegra_io_pad_dpd_poll(status, mask, 0, 250);
>  	if (err) {
> -		pr_info("tegra_io_rail_poll() failed: %d\n", err);
> +		pr_info("tegra_io_pad_dpd_poll() failed: %d\n", err);
>  		goto error;
>  	}
>  
> -	tegra_io_rail_unprepare();
> +	tegra_io_pad_dpd_unprepare();
>  
>  error:
>  	mutex_unlock(&pmc->powergates_lock);
>  
>  	return err;
>  }
> -EXPORT_SYMBOL(tegra_io_rail_power_on);
> +EXPORT_SYMBOL(tegra_io_pads_power_enable);
>  
> -int tegra_io_rail_power_off(unsigned int id)
> +int tegra_io_pads_power_disable(int pad_id)

s/pad_id/id/

>  {
>  	unsigned long request, status;
>  	unsigned int bit;
> @@ -945,9 +993,9 @@ int tegra_io_rail_power_off(unsigned int id)
>  
>  	mutex_lock(&pmc->powergates_lock);
>  
> -	err = tegra_io_rail_prepare(id, &request, &status, &bit);
> +	err = tegra_io_pad_dpd_prepare(pad_id, &request, &status, &bit);
>  	if (err) {
> -		pr_info("tegra_io_rail_prepare() failed: %d\n", err);
> +		pr_info("tegra_io_pad_dpd_prepare() failed: %d\n", err);
>  		goto error;
>  	}
>  
> @@ -959,18 +1007,77 @@ int tegra_io_rail_power_off(unsigned int id)
>  	value |= IO_DPD_REQ_CODE_ON;
>  	tegra_pmc_writel(value, request);
>  
> -	err = tegra_io_rail_poll(status, mask, mask, 250);
> +	err = tegra_io_pad_dpd_poll(status, mask, mask, 250);
>  	if (err)
>  		goto error;
>  
> -	tegra_io_rail_unprepare();
> +	tegra_io_pad_dpd_unprepare();
>  
>  error:
>  	mutex_unlock(&pmc->powergates_lock);
>  
>  	return err;
>  }
> -EXPORT_SYMBOL(tegra_io_rail_power_off);
> +EXPORT_SYMBOL(tegra_io_pads_power_disable);
> +
> +int tegra_io_pads_power_is_enabled(int io_pad_id)

s/io_pad_id/id/


> +{
> +	unsigned long status_reg;
> +	u32 status;
> +	int dpd_bit;

s/dpd_bit/bit/

> +
> +	dpd_bit = tegra_io_pads_to_dpd_bit(pmc->soc, io_pad_id);
> +	if (dpd_bit < 0)
> +		return dpd_bit;
> +
> +	status_reg = (dpd_bit < 32) ? IO_DPD_STATUS : IO_DPD2_STATUS;
> +	status = tegra_pmc_readl(status_reg);
> +
> +	return !!(status & BIT(dpd_bit % 32));
> +}
> +EXPORT_SYMBOL(tegra_io_pads_power_is_enabled);
> +
> +int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)

s/io_pad_id/id/

I think I prefer tegra_io_pads_set/get_voltage_conf(). What is the point
in passing uV here if in device-tree you are using the enum for the
voltage level? I know that I had suggested this, but given we are not
going to use voltage in the DT then, not sure it has any value here.

> +{
> +	int pwr_bit;
> +	u32 pwr_mask;

I would drop the "pwr_" and just have bit and mask.

> +	u32 bval;
> +
> +	if ((io_volt_uv != 3300000) && (io_volt_uv != 1800000))
> +		return -EINVAL;
> +
> +	pwr_bit = tegra_io_pads_to_power_bit(pmc->soc, io_pad_id);
> +	if (pwr_bit < 0)
> +		return pwr_bit;
> +
> +	pwr_mask = BIT(pwr_bit);
> +	bval = (io_volt_uv == 3300000) ? pwr_mask : 0;
> +
> +	mutex_lock(&pmc->powergates_lock);
> +	tegra_pmc_read_modify_write(PMC_PWR_DET, pwr_mask, pwr_mask);
> +	tegra_pmc_read_modify_write(PMC_PWR_DET_VAL, pwr_mask, bval);
> +	mutex_unlock(&pmc->powergates_lock);
> +
> +	usleep_range(100, 250);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(tegra_io_pads_configure_voltage);
> +
> +int tegra_io_pads_get_configured_voltage(int io_pad_id)

s/io_pad_id/id/

> +{
> +	int pwr_bit;
> +	u32 pwr_det_val;

int bit;
u32 val;

> +
> +	pwr_bit = tegra_io_pads_to_power_bit(pmc->soc, io_pad_id);
> +	if (pwr_bit < 0)
> +		return pwr_bit;
> +
> +	pwr_det_val = tegra_pmc_readl(PMC_PWR_DET_VAL);
> +
> +	return (pwr_det_val & BIT(pwr_bit)) ? 3300000 : 1800000;
> +}
> +EXPORT_SYMBOL(tegra_io_pads_get_configured_voltage);
>  
>  #ifdef CONFIG_PM_SLEEP
>  enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void)
> @@ -1397,11 +1504,53 @@ static const u8 tegra124_cpu_powergates[] = {
>  	TEGRA_POWERGATE_CPU3,
>  };
>  
> +#define TEGRA_IO_PADS_CONTROL(_pad, _dpd, _pwr)		\
> +{							\
> +	.pad_id = (TEGRA_IO_PAD_##_pad),		\

Not sure this needs to be part of the structure as it is just an index.

> +	.dpd_bit_pos = (_dpd),				\
> +	.pwr_bit_pos = (_pwr),				\
> +}
> +
> +struct tegra_io_pads_control tegra124_io_pads_control[] = {
> +	TEGRA_IO_PADS_CONTROL(CSIA, 0, -1),
> +	TEGRA_IO_PADS_CONTROL(CSIB, 1, -1),
> +	TEGRA_IO_PADS_CONTROL(DSI, 2, -1),
> +	TEGRA_IO_PADS_CONTROL(MIPI_BIAS, 3, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_BIAS, 4, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_CLK1, 5, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_CLK2, 6, -1),
> +	TEGRA_IO_PADS_CONTROL(USB0, 9, -1),
> +	TEGRA_IO_PADS_CONTROL(USB1, 10, -1),
> +	TEGRA_IO_PADS_CONTROL(USB2, 11, -1),
> +	TEGRA_IO_PADS_CONTROL(USB_BIAS, 12, -1),
> +	TEGRA_IO_PADS_CONTROL(NAND, 13, -1),
> +	TEGRA_IO_PADS_CONTROL(UART, 14, -1),
> +	TEGRA_IO_PADS_CONTROL(BB, 15, -1),
> +	TEGRA_IO_PADS_CONTROL(AUDIO, 17, -1),
> +	TEGRA_IO_PADS_CONTROL(HSIC, 19, -1),
> +	TEGRA_IO_PADS_CONTROL(COMP, 22, -1),
> +	TEGRA_IO_PADS_CONTROL(HDMI, 28, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_CNTRL, 32, -1),
> +	TEGRA_IO_PADS_CONTROL(SDMMC1, 33, -1),
> +	TEGRA_IO_PADS_CONTROL(SDMMC3, 34, -1),
> +	TEGRA_IO_PADS_CONTROL(SDMMC4, 35, -1),
> +	TEGRA_IO_PADS_CONTROL(CAM, 36, -1),
> +	TEGRA_IO_PADS_CONTROL(HV, 38, -1),
> +	TEGRA_IO_PADS_CONTROL(DSIB, 39, -1),
> +	TEGRA_IO_PADS_CONTROL(DSIC, 40, -1),
> +	TEGRA_IO_PADS_CONTROL(DSID, 41, -1),
> +	TEGRA_IO_PADS_CONTROL(CSIE, 44, -1),
> +	TEGRA_IO_PADS_CONTROL(LVDS, 57, -1),
> +	TEGRA_IO_PADS_CONTROL(SYS_DDC, 58, -1),
> +};
> +
>  static const struct tegra_pmc_soc tegra124_pmc_soc = {
>  	.num_powergates = ARRAY_SIZE(tegra124_powergates),
>  	.powergates = tegra124_powergates,
>  	.num_cpu_powergates = ARRAY_SIZE(tegra124_cpu_powergates),
>  	.cpu_powergates = tegra124_cpu_powergates,
> +	.io_pads_control = tegra124_io_pads_control,
> +	.num_io_pads = ARRAY_SIZE(tegra124_io_pads_control),
>  	.has_tsense_reset = true,
>  	.has_gpu_clamps = true,
>  };
> @@ -1440,11 +1589,52 @@ static const u8 tegra210_cpu_powergates[] = {
>  	TEGRA_POWERGATE_CPU3,
>  };
>  
> +struct tegra_io_pads_control tegra210_io_pads_control[] = {
> +	TEGRA_IO_PADS_CONTROL(CSIA, 0, -1),
> +	TEGRA_IO_PADS_CONTROL(CSIB, 1, -1),
> +	TEGRA_IO_PADS_CONTROL(DSI, 2, -1),
> +	TEGRA_IO_PADS_CONTROL(MIPI_BIAS, 3, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_BIAS, 4, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_CLK1, 5, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_CLK2, 6, -1),
> +	TEGRA_IO_PADS_CONTROL(USB0, 9, -1),
> +	TEGRA_IO_PADS_CONTROL(USB1, 10, -1),
> +	TEGRA_IO_PADS_CONTROL(USB2, 11, -1),
> +	TEGRA_IO_PADS_CONTROL(USB_BIAS, 12, -1),
> +	TEGRA_IO_PADS_CONTROL(UART, 14, -1),
> +	TEGRA_IO_PADS_CONTROL(AUDIO, 17, -1),
> +	TEGRA_IO_PADS_CONTROL(USB3, 18, -1),
> +	TEGRA_IO_PADS_CONTROL(HSIC, 19, -1),
> +	TEGRA_IO_PADS_CONTROL(DBG, 25, -1),
> +	TEGRA_IO_PADS_CONTROL(DEBUG_NONAO, 26, -1),
> +	TEGRA_IO_PADS_CONTROL(GPIO, 27, 21),
> +	TEGRA_IO_PADS_CONTROL(HDMI, 28, -1),
> +	TEGRA_IO_PADS_CONTROL(SDMMC1, 33, 12),
> +	TEGRA_IO_PADS_CONTROL(SDMMC3, 34, 13),
> +	TEGRA_IO_PADS_CONTROL(EMMC, 35, -1),
> +	TEGRA_IO_PADS_CONTROL(CAM, 36, -1),
> +	TEGRA_IO_PADS_CONTROL(EMMC2, 37, -1),
> +	TEGRA_IO_PADS_CONTROL(DSIB, 39, -1),
> +	TEGRA_IO_PADS_CONTROL(DSIC, 40, -1),
> +	TEGRA_IO_PADS_CONTROL(DSID, 41, -1),
> +	TEGRA_IO_PADS_CONTROL(CSIC, 42, -1),
> +	TEGRA_IO_PADS_CONTROL(CSID, 43, -1),
> +	TEGRA_IO_PADS_CONTROL(CSIE, 44, -1),
> +	TEGRA_IO_PADS_CONTROL(CSIF, 45, -1),
> +	TEGRA_IO_PADS_CONTROL(SPI, 46, -1),
> +	TEGRA_IO_PADS_CONTROL(SPI_HV, 47, 23),
> +	TEGRA_IO_PADS_CONTROL(DMIC, 50, -1),
> +	TEGRA_IO_PADS_CONTROL(DP, 51, -1),
> +	TEGRA_IO_PADS_CONTROL(LVDS, 57, -1),
> +	TEGRA_IO_PADS_CONTROL(AUDIO_HV, 61, 18),
> +};
>  static const struct tegra_pmc_soc tegra210_pmc_soc = {
>  	.num_powergates = ARRAY_SIZE(tegra210_powergates),
>  	.powergates = tegra210_powergates,
>  	.num_cpu_powergates = ARRAY_SIZE(tegra210_cpu_powergates),
>  	.cpu_powergates = tegra210_cpu_powergates,
> +	.io_pads_control = tegra210_io_pads_control,
> +	.num_io_pads = ARRAY_SIZE(tegra210_io_pads_control),
>  	.has_tsense_reset = true,
>  	.has_gpu_clamps = true,
>  };
> diff --git a/include/soc/tegra/pmc.h b/include/soc/tegra/pmc.h
> index e9e5347..763bd6d 100644
> --- a/include/soc/tegra/pmc.h
> +++ b/include/soc/tegra/pmc.h
> @@ -76,37 +76,57 @@ int tegra_pmc_cpu_remove_clamping(unsigned int cpuid);
>  
>  #define TEGRA_POWERGATE_3D0	TEGRA_POWERGATE_3D
>  
> -#define TEGRA_IO_RAIL_CSIA	0
> -#define TEGRA_IO_RAIL_CSIB	1
> -#define TEGRA_IO_RAIL_DSI	2
> -#define TEGRA_IO_RAIL_MIPI_BIAS	3
> -#define TEGRA_IO_RAIL_PEX_BIAS	4
> -#define TEGRA_IO_RAIL_PEX_CLK1	5
> -#define TEGRA_IO_RAIL_PEX_CLK2	6
> -#define TEGRA_IO_RAIL_USB0	9
> -#define TEGRA_IO_RAIL_USB1	10
> -#define TEGRA_IO_RAIL_USB2	11
> -#define TEGRA_IO_RAIL_USB_BIAS	12
> -#define TEGRA_IO_RAIL_NAND	13
> -#define TEGRA_IO_RAIL_UART	14
> -#define TEGRA_IO_RAIL_BB	15
> -#define TEGRA_IO_RAIL_AUDIO	17
> -#define TEGRA_IO_RAIL_HSIC	19
> -#define TEGRA_IO_RAIL_COMP	22
> -#define TEGRA_IO_RAIL_HDMI	28
> -#define TEGRA_IO_RAIL_PEX_CNTRL	32
> -#define TEGRA_IO_RAIL_SDMMC1	33
> -#define TEGRA_IO_RAIL_SDMMC3	34
> -#define TEGRA_IO_RAIL_SDMMC4	35
> -#define TEGRA_IO_RAIL_CAM	36
> -#define TEGRA_IO_RAIL_RES	37
> -#define TEGRA_IO_RAIL_HV	38
> -#define TEGRA_IO_RAIL_DSIB	39
> -#define TEGRA_IO_RAIL_DSIC	40
> -#define TEGRA_IO_RAIL_DSID	41
> -#define TEGRA_IO_RAIL_CSIE	44
> -#define TEGRA_IO_RAIL_LVDS	57
> -#define TEGRA_IO_RAIL_SYS_DDC	58
> +/* TEGRA_IO_PAD: The IO pins of Tegra SoCs are grouped for common control
> + * of IO interface like setting voltage signal levels, power state of the
> + * interface. The group is generally referred as io-pads. The power and
> + * voltage control of IO pins are available at io-pads level.
> + * The following macros make the super list all IO pads found on Tegra SoC
> + * generations.
> + */
> +#define TEGRA_IO_PAD_AUDIO		0
> +#define TEGRA_IO_PAD_AUDIO_HV		1
> +#define TEGRA_IO_PAD_BB			2
> +#define TEGRA_IO_PAD_CAM		3
> +#define TEGRA_IO_PAD_COMP		4
> +#define TEGRA_IO_PAD_CSIA		5
> +#define TEGRA_IO_PAD_CSIB		6
> +#define TEGRA_IO_PAD_CSIC		7
> +#define TEGRA_IO_PAD_CSID		8
> +#define TEGRA_IO_PAD_CSIE		9
> +#define TEGRA_IO_PAD_CSIF		10
> +#define TEGRA_IO_PAD_DBG		11
> +#define TEGRA_IO_PAD_DEBUG_NONAO	12
> +#define TEGRA_IO_PAD_DMIC		13
> +#define TEGRA_IO_PAD_DP			14
> +#define TEGRA_IO_PAD_DSI		15
> +#define TEGRA_IO_PAD_DSIB		16
> +#define TEGRA_IO_PAD_DSIC		17
> +#define TEGRA_IO_PAD_DSID		18
> +#define TEGRA_IO_PAD_EMMC		19
> +#define TEGRA_IO_PAD_EMMC2		20
> +#define TEGRA_IO_PAD_GPIO		21
> +#define TEGRA_IO_PAD_HDMI		22
> +#define TEGRA_IO_PAD_HSIC		23
> +#define TEGRA_IO_PAD_HV			24
> +#define TEGRA_IO_PAD_LVDS		25
> +#define TEGRA_IO_PAD_MIPI_BIAS		26
> +#define TEGRA_IO_PAD_NAND		27
> +#define TEGRA_IO_PAD_PEX_BIAS		28
> +#define TEGRA_IO_PAD_PEX_CLK1		29
> +#define TEGRA_IO_PAD_PEX_CLK2		30
> +#define TEGRA_IO_PAD_PEX_CNTRL		31
> +#define TEGRA_IO_PAD_SDMMC1		32
> +#define TEGRA_IO_PAD_SDMMC3		33
> +#define TEGRA_IO_PAD_SDMMC4		34
> +#define TEGRA_IO_PAD_SPI		35
> +#define TEGRA_IO_PAD_SPI_HV		36
> +#define TEGRA_IO_PAD_SYS_DDC		37
> +#define TEGRA_IO_PAD_UART		38
> +#define TEGRA_IO_PAD_USB0		39
> +#define TEGRA_IO_PAD_USB1		40
> +#define TEGRA_IO_PAD_USB2		41
> +#define TEGRA_IO_PAD_USB3		42
> +#define TEGRA_IO_PAD_USB_BIAS		43

Enum?

>  
>  #ifdef CONFIG_ARCH_TEGRA
>  int tegra_powergate_is_powered(unsigned int id);
> @@ -118,8 +138,15 @@ int tegra_powergate_remove_clamping(unsigned int id);
>  int tegra_powergate_sequence_power_up(unsigned int id, struct clk *clk,
>  				      struct reset_control *rst);
>  
> -int tegra_io_rail_power_on(unsigned int id);
> -int tegra_io_rail_power_off(unsigned int id);
> +/* Power enable/disable of the IO pads */
> +int tegra_io_pads_power_enable(int io_pad_id);
> +int tegra_io_pads_power_disable(int io_pad_id);
> +int tegra_io_pads_power_is_enabled(int io_pad_id);
> +
> +/* Set/Get of IO pad voltage */
> +int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv);
> +int tegra_io_pads_get_configured_voltage(int io_pad_id);
> +
>  #else
>  static inline int tegra_powergate_is_powered(unsigned int id)
>  {
> @@ -148,14 +175,29 @@ static inline int tegra_powergate_sequence_power_up(unsigned int id,
>  	return -ENOSYS;
>  }
>  
> -static inline int tegra_io_rail_power_on(unsigned int id)
> +static inline int tegra_io_pads_power_enable(int io_pad_id)
>  {
> -	return -ENOSYS;
> +	return -ENOTSUPP;
>  }
>  
> -static inline int tegra_io_rail_power_off(unsigned int id)
> +static inline int tegra_io_pads_power_disable(int io_pad_id)
>  {
> -	return -ENOSYS;
> +	return -ENOTSUPP;
> +}
> +
> +static inline int tegra_io_pads_power_is_enabled(int io_pad_id)
> +{
> +	return -ENOTSUPP;
> +}
> +
> +static inline int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
> +{
> +	return -ENOTSUPP;
> +}
> +
> +static inline int tegra_io_pads_get_configured_voltage(int io_pad_id)
> +{
> +	return -ENOTSUPP;
>  }
>  #endif /* CONFIG_ARCH_TEGRA */

In genernal this feels like 3 patches ...

1. Patch to add the new APIs
2. Patch to convert SOR to use new APIs
3. Patch to remove old APIs

May be let's see what Thierry thinks.

Cheers
Jon

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Jon Hunter]

On 04/05/16 12:39, Laxman Dewangan wrote:
> The IO pins of Tegra SoCs are grouped for common control of IO
> interface like setting voltage signal levels and power state of
> the interface. The group is generally referred as IO pads. The
> power state and voltage control of IO pins can be done at IO pads
> level.
> 
> Tegra generation SoC supports the power down of IO pads when it
> is not used even in the active state of system. This saves power
> from that IO interface. Also it supports multiple voltage level
> in IO pins for interfacing on some of pads. The IO pad voltage is
> automatically detected till T124, hence SW need not to configure
> this. But from T210, the automatically detection logic has been
> removed, hence SW need to explicitly set the IO pad voltage into
> IO pad configuration registers.
> 
> Add support to set the power states and voltage level of the IO pads
> from client driver. The implementation for the APIs are in generic
> which is applicable for all generation os Tegra SoC.
> 
> IO pads ID and information of bit field for power state and voltage
> level controls are added for Tegra124, Tegra132 and Tegra210. The sor
> driver is modified to use the new APIs.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> 
> ---
> Changes from V1:
> This is reworked on earlier path to have separation between IO rails and
> io pads and add power state and voltage control APIs in single call.
> 
> Changes from V2:
> - Remove the tegra_io_rail_power_off/on() apis and change client (sor) driver
> to use the new APIs for IO pad power.
> - Remove the TEGRA_IO_RAIL_ macros.
> ---
>  drivers/gpu/drm/tegra/sor.c |   8 +-
>  drivers/soc/tegra/pmc.c     | 242 +++++++++++++++++++++++++++++++++++++++-----
>  include/soc/tegra/pmc.h     | 116 ++++++++++++++-------
>  3 files changed, 299 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index 757c6e8..1d90171 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -1134,7 +1134,7 @@ static void tegra_sor_edp_disable(struct drm_encoder *encoder)
>  			dev_err(sor->dev, "failed to disable DP: %d\n", err);
>  	}
>  
> -	err = tegra_io_rail_power_off(TEGRA_IO_RAIL_LVDS);
> +	err = tegra_io_pads_power_disable(TEGRA_IO_PAD_LVDS);
>  	if (err < 0)
>  		dev_err(sor->dev, "failed to power off I/O rail: %d\n", err);
>  
> @@ -1296,7 +1296,7 @@ static void tegra_sor_edp_enable(struct drm_encoder *encoder)
>  	tegra_sor_writel(sor, value, SOR_DP_PADCTL0);
>  
>  	/* step 2 */
> -	err = tegra_io_rail_power_on(TEGRA_IO_RAIL_LVDS);
> +	err = tegra_io_pads_power_enable(TEGRA_IO_PAD_LVDS);
>  	if (err < 0)
>  		dev_err(sor->dev, "failed to power on I/O rail: %d\n", err);
>  
> @@ -1748,7 +1748,7 @@ static void tegra_sor_hdmi_disable(struct drm_encoder *encoder)
>  	if (err < 0)
>  		dev_err(sor->dev, "failed to power down SOR: %d\n", err);
>  
> -	err = tegra_io_rail_power_off(TEGRA_IO_RAIL_HDMI);
> +	err = tegra_io_pads_power_disable(TEGRA_IO_PAD_HDMI);
>  	if (err < 0)
>  		dev_err(sor->dev, "failed to power off HDMI rail: %d\n", err);
>  
> @@ -1787,7 +1787,7 @@ static void tegra_sor_hdmi_enable(struct drm_encoder *encoder)
>  
>  	div = clk_get_rate(sor->clk) / 1000000 * 4;
>  
> -	err = tegra_io_rail_power_on(TEGRA_IO_RAIL_HDMI);
> +	err = tegra_io_pads_power_enable(TEGRA_IO_PAD_HDMI);
>  	if (err < 0)
>  		dev_err(sor->dev, "failed to power on HDMI rail: %d\n", err);
>  
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index eff9425..0611cea 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -76,6 +76,10 @@
>  
>  #define PMC_SCRATCH41			0x140
>  
> +/* Power detect for pad voltage */
> +#define PMC_PWR_DET			0x48
> +#define PMC_PWR_DET_VAL			0xe4
> +
>  #define PMC_SENSOR_CTRL			0x1b0
>  #define PMC_SENSOR_CTRL_SCRATCH_WRITE	BIT(2)
>  #define PMC_SENSOR_CTRL_ENABLE_RST	BIT(1)
> @@ -115,12 +119,19 @@ struct tegra_powergate {
>  	unsigned int num_resets;
>  };
>  
> +struct tegra_io_pads_control {
> +	int pad_id;
> +	int dpd_bit_pos;
> +	int pwr_bit_pos;
> +};
> +
>  struct tegra_pmc_soc {
>  	unsigned int num_powergates;
>  	const char *const *powergates;
>  	unsigned int num_cpu_powergates;
>  	const u8 *cpu_powergates;
> -
> +	const struct tegra_io_pads_control *io_pads_control;
> +	unsigned int num_io_pads;
>  	bool has_tsense_reset;
>  	bool has_gpu_clamps;
>  };
> @@ -196,6 +207,14 @@ static void tegra_pmc_writel(u32 value, unsigned long offset)
>  	writel(value, pmc->base + offset);
>  }
>  
> +static void tegra_pmc_read_modify_write(unsigned long offset, u32 mask, u32 val)

Nit, I would prefer something shorter such as tegra_pmc_rmw() or
tegra_pmc_set_field().

> +{
> +	u32 pmc_reg = tegra_pmc_readl(offset);
> +
> +	pmc_reg = (pmc_reg & ~mask) | (val & mask);
> +	tegra_pmc_writel(pmc_reg, offset);
> +}
> +
>  static inline bool tegra_powergate_state(int id)
>  {
>  	if (id == TEGRA_POWERGATE_3D && pmc->soc->has_gpu_clamps)
> @@ -841,22 +860,51 @@ static void tegra_powergate_init(struct tegra_pmc *pmc)
>  	of_node_put(np);
>  }
>  
> -static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
> -				 unsigned long *status, unsigned int *bit)
> +static int tegra_io_pads_to_dpd_bit(const struct tegra_pmc_soc *soc, int pad_id)

Please just use "id". This is consistent with the pmc driver and I think
it is clear from the function that this is the pad ID.

> +{
> +	int i;
> +
> +	if (!soc || !soc->num_io_pads)
> +		return -EINVAL;
> +
> +	for (i = 0; i < soc->num_io_pads; ++i) {
> +		if (soc->io_pads_control[i].pad_id == pad_id)
> +			return soc->io_pads_control[i].dpd_bit_pos;
> +	}

Do we need a loop here? Can't we just make the table a look-up table now
that the ID is just an index?

> +
> +	return -EINVAL;
> +}
> +
> +static int tegra_io_pads_to_power_bit(const struct tegra_pmc_soc *soc,
> +				      int pad_id)
> +{
> +	int i;
> +
> +	if (!soc || !soc->num_io_pads)
> +		return -EINVAL;
> +
> +	for (i = 0; i < soc->num_io_pads; ++i) {
> +		if (soc->io_pads_control[i].pad_id == pad_id)
> +			return soc->io_pads_control[i].pwr_bit_pos;
> +	}

Same here a lookup table would be better.

> +
> +	return -EINVAL;
> +}
> +
> +static int tegra_io_pad_dpd_prepare(int pad_id, unsigned long *request,
> +				    unsigned long *status, unsigned int *bit)

Some functions are tegra_io_pads_xxx and some are tegra_io_pad_xxx.
Please be consistent.

>  {
>  	unsigned long rate;
>  	u32 value;
> +	int dpd_bit;
>  
> -	*bit = id % 32;
> +	dpd_bit = tegra_io_pads_to_dpd_bit(pmc->soc, pad_id);
> +	if (dpd_bit < 0)
> +		return dpd_bit;
>  
> -	/*
> -	 * There are two sets of 30 bits to select IO rails, but bits 30 and
> -	 * 31 are control bits rather than IO rail selection bits.
> -	 */
> -	if (id > 63 || *bit == 30 || *bit == 31)
> -		return -EINVAL;
> +	*bit = dpd_bit % 32;
>  
> -	if (id < 32) {
> +	if (dpd_bit < 32) {
>  		*status = IO_DPD_STATUS;
>  		*request = IO_DPD_REQ;
>  	} else {
> @@ -877,8 +925,8 @@ static int tegra_io_rail_prepare(unsigned int id, unsigned long *request,
>  	return 0;
>  }
>  
> -static int tegra_io_rail_poll(unsigned long offset, u32 mask,
> -			      u32 val, unsigned long timeout)
> +static int tegra_io_pad_dpd_poll(unsigned long offset, u32 mask,
> +				 u32 val, unsigned long timeout)

tegra_io_pads_xxx

>  {
>  	u32 value;
>  
> @@ -895,12 +943,12 @@ static int tegra_io_rail_poll(unsigned long offset, u32 mask,
>  	return -ETIMEDOUT;
>  }
>  
> -static void tegra_io_rail_unprepare(void)
> +static void tegra_io_pad_dpd_unprepare(void)

tegra_io_pads_xxx

>  {
>  	tegra_pmc_writel(DPD_SAMPLE_DISABLE, DPD_SAMPLE);
>  }
>  
> -int tegra_io_rail_power_on(unsigned int id)
> +int tegra_io_pads_power_enable(int pad_id)

s/pad_id/id/

>  {
>  	unsigned long request, status;
>  	unsigned int bit;
> @@ -909,7 +957,7 @@ int tegra_io_rail_power_on(unsigned int id)
>  
>  	mutex_lock(&pmc->powergates_lock);
>  
> -	err = tegra_io_rail_prepare(id, &request, &status, &bit);
> +	err = tegra_io_pad_dpd_prepare(pad_id, &request, &status, &bit);
>  	if (err)
>  		goto error;
>  
> @@ -921,22 +969,22 @@ int tegra_io_rail_power_on(unsigned int id)
>  	value |= IO_DPD_REQ_CODE_OFF;
>  	tegra_pmc_writel(value, request);
>  
> -	err = tegra_io_rail_poll(status, mask, 0, 250);
> +	err = tegra_io_pad_dpd_poll(status, mask, 0, 250);
>  	if (err) {
> -		pr_info("tegra_io_rail_poll() failed: %d\n", err);
> +		pr_info("tegra_io_pad_dpd_poll() failed: %d\n", err);
>  		goto error;
>  	}
>  
> -	tegra_io_rail_unprepare();
> +	tegra_io_pad_dpd_unprepare();
>  
>  error:
>  	mutex_unlock(&pmc->powergates_lock);
>  
>  	return err;
>  }
> -EXPORT_SYMBOL(tegra_io_rail_power_on);
> +EXPORT_SYMBOL(tegra_io_pads_power_enable);
>  
> -int tegra_io_rail_power_off(unsigned int id)
> +int tegra_io_pads_power_disable(int pad_id)

s/pad_id/id/

>  {
>  	unsigned long request, status;
>  	unsigned int bit;
> @@ -945,9 +993,9 @@ int tegra_io_rail_power_off(unsigned int id)
>  
>  	mutex_lock(&pmc->powergates_lock);
>  
> -	err = tegra_io_rail_prepare(id, &request, &status, &bit);
> +	err = tegra_io_pad_dpd_prepare(pad_id, &request, &status, &bit);
>  	if (err) {
> -		pr_info("tegra_io_rail_prepare() failed: %d\n", err);
> +		pr_info("tegra_io_pad_dpd_prepare() failed: %d\n", err);
>  		goto error;
>  	}
>  
> @@ -959,18 +1007,77 @@ int tegra_io_rail_power_off(unsigned int id)
>  	value |= IO_DPD_REQ_CODE_ON;
>  	tegra_pmc_writel(value, request);
>  
> -	err = tegra_io_rail_poll(status, mask, mask, 250);
> +	err = tegra_io_pad_dpd_poll(status, mask, mask, 250);
>  	if (err)
>  		goto error;
>  
> -	tegra_io_rail_unprepare();
> +	tegra_io_pad_dpd_unprepare();
>  
>  error:
>  	mutex_unlock(&pmc->powergates_lock);
>  
>  	return err;
>  }
> -EXPORT_SYMBOL(tegra_io_rail_power_off);
> +EXPORT_SYMBOL(tegra_io_pads_power_disable);
> +
> +int tegra_io_pads_power_is_enabled(int io_pad_id)

s/io_pad_id/id/


> +{
> +	unsigned long status_reg;
> +	u32 status;
> +	int dpd_bit;

s/dpd_bit/bit/

> +
> +	dpd_bit = tegra_io_pads_to_dpd_bit(pmc->soc, io_pad_id);
> +	if (dpd_bit < 0)
> +		return dpd_bit;
> +
> +	status_reg = (dpd_bit < 32) ? IO_DPD_STATUS : IO_DPD2_STATUS;
> +	status = tegra_pmc_readl(status_reg);
> +
> +	return !!(status & BIT(dpd_bit % 32));
> +}
> +EXPORT_SYMBOL(tegra_io_pads_power_is_enabled);
> +
> +int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)

s/io_pad_id/id/

I think I prefer tegra_io_pads_set/get_voltage_conf(). What is the point
in passing uV here if in device-tree you are using the enum for the
voltage level? I know that I had suggested this, but given we are not
going to use voltage in the DT then, not sure it has any value here.

> +{
> +	int pwr_bit;
> +	u32 pwr_mask;

I would drop the "pwr_" and just have bit and mask.

> +	u32 bval;
> +
> +	if ((io_volt_uv != 3300000) && (io_volt_uv != 1800000))
> +		return -EINVAL;
> +
> +	pwr_bit = tegra_io_pads_to_power_bit(pmc->soc, io_pad_id);
> +	if (pwr_bit < 0)
> +		return pwr_bit;
> +
> +	pwr_mask = BIT(pwr_bit);
> +	bval = (io_volt_uv == 3300000) ? pwr_mask : 0;
> +
> +	mutex_lock(&pmc->powergates_lock);
> +	tegra_pmc_read_modify_write(PMC_PWR_DET, pwr_mask, pwr_mask);
> +	tegra_pmc_read_modify_write(PMC_PWR_DET_VAL, pwr_mask, bval);
> +	mutex_unlock(&pmc->powergates_lock);
> +
> +	usleep_range(100, 250);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(tegra_io_pads_configure_voltage);
> +
> +int tegra_io_pads_get_configured_voltage(int io_pad_id)

s/io_pad_id/id/

> +{
> +	int pwr_bit;
> +	u32 pwr_det_val;

int bit;
u32 val;

> +
> +	pwr_bit = tegra_io_pads_to_power_bit(pmc->soc, io_pad_id);
> +	if (pwr_bit < 0)
> +		return pwr_bit;
> +
> +	pwr_det_val = tegra_pmc_readl(PMC_PWR_DET_VAL);
> +
> +	return (pwr_det_val & BIT(pwr_bit)) ? 3300000 : 1800000;
> +}
> +EXPORT_SYMBOL(tegra_io_pads_get_configured_voltage);
>  
>  #ifdef CONFIG_PM_SLEEP
>  enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void)
> @@ -1397,11 +1504,53 @@ static const u8 tegra124_cpu_powergates[] = {
>  	TEGRA_POWERGATE_CPU3,
>  };
>  
> +#define TEGRA_IO_PADS_CONTROL(_pad, _dpd, _pwr)		\
> +{							\
> +	.pad_id = (TEGRA_IO_PAD_##_pad),		\

Not sure this needs to be part of the structure as it is just an index.

> +	.dpd_bit_pos = (_dpd),				\
> +	.pwr_bit_pos = (_pwr),				\
> +}
> +
> +struct tegra_io_pads_control tegra124_io_pads_control[] = {
> +	TEGRA_IO_PADS_CONTROL(CSIA, 0, -1),
> +	TEGRA_IO_PADS_CONTROL(CSIB, 1, -1),
> +	TEGRA_IO_PADS_CONTROL(DSI, 2, -1),
> +	TEGRA_IO_PADS_CONTROL(MIPI_BIAS, 3, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_BIAS, 4, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_CLK1, 5, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_CLK2, 6, -1),
> +	TEGRA_IO_PADS_CONTROL(USB0, 9, -1),
> +	TEGRA_IO_PADS_CONTROL(USB1, 10, -1),
> +	TEGRA_IO_PADS_CONTROL(USB2, 11, -1),
> +	TEGRA_IO_PADS_CONTROL(USB_BIAS, 12, -1),
> +	TEGRA_IO_PADS_CONTROL(NAND, 13, -1),
> +	TEGRA_IO_PADS_CONTROL(UART, 14, -1),
> +	TEGRA_IO_PADS_CONTROL(BB, 15, -1),
> +	TEGRA_IO_PADS_CONTROL(AUDIO, 17, -1),
> +	TEGRA_IO_PADS_CONTROL(HSIC, 19, -1),
> +	TEGRA_IO_PADS_CONTROL(COMP, 22, -1),
> +	TEGRA_IO_PADS_CONTROL(HDMI, 28, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_CNTRL, 32, -1),
> +	TEGRA_IO_PADS_CONTROL(SDMMC1, 33, -1),
> +	TEGRA_IO_PADS_CONTROL(SDMMC3, 34, -1),
> +	TEGRA_IO_PADS_CONTROL(SDMMC4, 35, -1),
> +	TEGRA_IO_PADS_CONTROL(CAM, 36, -1),
> +	TEGRA_IO_PADS_CONTROL(HV, 38, -1),
> +	TEGRA_IO_PADS_CONTROL(DSIB, 39, -1),
> +	TEGRA_IO_PADS_CONTROL(DSIC, 40, -1),
> +	TEGRA_IO_PADS_CONTROL(DSID, 41, -1),
> +	TEGRA_IO_PADS_CONTROL(CSIE, 44, -1),
> +	TEGRA_IO_PADS_CONTROL(LVDS, 57, -1),
> +	TEGRA_IO_PADS_CONTROL(SYS_DDC, 58, -1),
> +};
> +
>  static const struct tegra_pmc_soc tegra124_pmc_soc = {
>  	.num_powergates = ARRAY_SIZE(tegra124_powergates),
>  	.powergates = tegra124_powergates,
>  	.num_cpu_powergates = ARRAY_SIZE(tegra124_cpu_powergates),
>  	.cpu_powergates = tegra124_cpu_powergates,
> +	.io_pads_control = tegra124_io_pads_control,
> +	.num_io_pads = ARRAY_SIZE(tegra124_io_pads_control),
>  	.has_tsense_reset = true,
>  	.has_gpu_clamps = true,
>  };
> @@ -1440,11 +1589,52 @@ static const u8 tegra210_cpu_powergates[] = {
>  	TEGRA_POWERGATE_CPU3,
>  };
>  
> +struct tegra_io_pads_control tegra210_io_pads_control[] = {
> +	TEGRA_IO_PADS_CONTROL(CSIA, 0, -1),
> +	TEGRA_IO_PADS_CONTROL(CSIB, 1, -1),
> +	TEGRA_IO_PADS_CONTROL(DSI, 2, -1),
> +	TEGRA_IO_PADS_CONTROL(MIPI_BIAS, 3, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_BIAS, 4, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_CLK1, 5, -1),
> +	TEGRA_IO_PADS_CONTROL(PEX_CLK2, 6, -1),
> +	TEGRA_IO_PADS_CONTROL(USB0, 9, -1),
> +	TEGRA_IO_PADS_CONTROL(USB1, 10, -1),
> +	TEGRA_IO_PADS_CONTROL(USB2, 11, -1),
> +	TEGRA_IO_PADS_CONTROL(USB_BIAS, 12, -1),
> +	TEGRA_IO_PADS_CONTROL(UART, 14, -1),
> +	TEGRA_IO_PADS_CONTROL(AUDIO, 17, -1),
> +	TEGRA_IO_PADS_CONTROL(USB3, 18, -1),
> +	TEGRA_IO_PADS_CONTROL(HSIC, 19, -1),
> +	TEGRA_IO_PADS_CONTROL(DBG, 25, -1),
> +	TEGRA_IO_PADS_CONTROL(DEBUG_NONAO, 26, -1),
> +	TEGRA_IO_PADS_CONTROL(GPIO, 27, 21),
> +	TEGRA_IO_PADS_CONTROL(HDMI, 28, -1),
> +	TEGRA_IO_PADS_CONTROL(SDMMC1, 33, 12),
> +	TEGRA_IO_PADS_CONTROL(SDMMC3, 34, 13),
> +	TEGRA_IO_PADS_CONTROL(EMMC, 35, -1),
> +	TEGRA_IO_PADS_CONTROL(CAM, 36, -1),
> +	TEGRA_IO_PADS_CONTROL(EMMC2, 37, -1),
> +	TEGRA_IO_PADS_CONTROL(DSIB, 39, -1),
> +	TEGRA_IO_PADS_CONTROL(DSIC, 40, -1),
> +	TEGRA_IO_PADS_CONTROL(DSID, 41, -1),
> +	TEGRA_IO_PADS_CONTROL(CSIC, 42, -1),
> +	TEGRA_IO_PADS_CONTROL(CSID, 43, -1),
> +	TEGRA_IO_PADS_CONTROL(CSIE, 44, -1),
> +	TEGRA_IO_PADS_CONTROL(CSIF, 45, -1),
> +	TEGRA_IO_PADS_CONTROL(SPI, 46, -1),
> +	TEGRA_IO_PADS_CONTROL(SPI_HV, 47, 23),
> +	TEGRA_IO_PADS_CONTROL(DMIC, 50, -1),
> +	TEGRA_IO_PADS_CONTROL(DP, 51, -1),
> +	TEGRA_IO_PADS_CONTROL(LVDS, 57, -1),
> +	TEGRA_IO_PADS_CONTROL(AUDIO_HV, 61, 18),
> +};
>  static const struct tegra_pmc_soc tegra210_pmc_soc = {
>  	.num_powergates = ARRAY_SIZE(tegra210_powergates),
>  	.powergates = tegra210_powergates,
>  	.num_cpu_powergates = ARRAY_SIZE(tegra210_cpu_powergates),
>  	.cpu_powergates = tegra210_cpu_powergates,
> +	.io_pads_control = tegra210_io_pads_control,
> +	.num_io_pads = ARRAY_SIZE(tegra210_io_pads_control),
>  	.has_tsense_reset = true,
>  	.has_gpu_clamps = true,
>  };
> diff --git a/include/soc/tegra/pmc.h b/include/soc/tegra/pmc.h
> index e9e5347..763bd6d 100644
> --- a/include/soc/tegra/pmc.h
> +++ b/include/soc/tegra/pmc.h
> @@ -76,37 +76,57 @@ int tegra_pmc_cpu_remove_clamping(unsigned int cpuid);
>  
>  #define TEGRA_POWERGATE_3D0	TEGRA_POWERGATE_3D
>  
> -#define TEGRA_IO_RAIL_CSIA	0
> -#define TEGRA_IO_RAIL_CSIB	1
> -#define TEGRA_IO_RAIL_DSI	2
> -#define TEGRA_IO_RAIL_MIPI_BIAS	3
> -#define TEGRA_IO_RAIL_PEX_BIAS	4
> -#define TEGRA_IO_RAIL_PEX_CLK1	5
> -#define TEGRA_IO_RAIL_PEX_CLK2	6
> -#define TEGRA_IO_RAIL_USB0	9
> -#define TEGRA_IO_RAIL_USB1	10
> -#define TEGRA_IO_RAIL_USB2	11
> -#define TEGRA_IO_RAIL_USB_BIAS	12
> -#define TEGRA_IO_RAIL_NAND	13
> -#define TEGRA_IO_RAIL_UART	14
> -#define TEGRA_IO_RAIL_BB	15
> -#define TEGRA_IO_RAIL_AUDIO	17
> -#define TEGRA_IO_RAIL_HSIC	19
> -#define TEGRA_IO_RAIL_COMP	22
> -#define TEGRA_IO_RAIL_HDMI	28
> -#define TEGRA_IO_RAIL_PEX_CNTRL	32
> -#define TEGRA_IO_RAIL_SDMMC1	33
> -#define TEGRA_IO_RAIL_SDMMC3	34
> -#define TEGRA_IO_RAIL_SDMMC4	35
> -#define TEGRA_IO_RAIL_CAM	36
> -#define TEGRA_IO_RAIL_RES	37
> -#define TEGRA_IO_RAIL_HV	38
> -#define TEGRA_IO_RAIL_DSIB	39
> -#define TEGRA_IO_RAIL_DSIC	40
> -#define TEGRA_IO_RAIL_DSID	41
> -#define TEGRA_IO_RAIL_CSIE	44
> -#define TEGRA_IO_RAIL_LVDS	57
> -#define TEGRA_IO_RAIL_SYS_DDC	58
> +/* TEGRA_IO_PAD: The IO pins of Tegra SoCs are grouped for common control
> + * of IO interface like setting voltage signal levels, power state of the
> + * interface. The group is generally referred as io-pads. The power and
> + * voltage control of IO pins are available at io-pads level.
> + * The following macros make the super list all IO pads found on Tegra SoC
> + * generations.
> + */
> +#define TEGRA_IO_PAD_AUDIO		0
> +#define TEGRA_IO_PAD_AUDIO_HV		1
> +#define TEGRA_IO_PAD_BB			2
> +#define TEGRA_IO_PAD_CAM		3
> +#define TEGRA_IO_PAD_COMP		4
> +#define TEGRA_IO_PAD_CSIA		5
> +#define TEGRA_IO_PAD_CSIB		6
> +#define TEGRA_IO_PAD_CSIC		7
> +#define TEGRA_IO_PAD_CSID		8
> +#define TEGRA_IO_PAD_CSIE		9
> +#define TEGRA_IO_PAD_CSIF		10
> +#define TEGRA_IO_PAD_DBG		11
> +#define TEGRA_IO_PAD_DEBUG_NONAO	12
> +#define TEGRA_IO_PAD_DMIC		13
> +#define TEGRA_IO_PAD_DP			14
> +#define TEGRA_IO_PAD_DSI		15
> +#define TEGRA_IO_PAD_DSIB		16
> +#define TEGRA_IO_PAD_DSIC		17
> +#define TEGRA_IO_PAD_DSID		18
> +#define TEGRA_IO_PAD_EMMC		19
> +#define TEGRA_IO_PAD_EMMC2		20
> +#define TEGRA_IO_PAD_GPIO		21
> +#define TEGRA_IO_PAD_HDMI		22
> +#define TEGRA_IO_PAD_HSIC		23
> +#define TEGRA_IO_PAD_HV			24
> +#define TEGRA_IO_PAD_LVDS		25
> +#define TEGRA_IO_PAD_MIPI_BIAS		26
> +#define TEGRA_IO_PAD_NAND		27
> +#define TEGRA_IO_PAD_PEX_BIAS		28
> +#define TEGRA_IO_PAD_PEX_CLK1		29
> +#define TEGRA_IO_PAD_PEX_CLK2		30
> +#define TEGRA_IO_PAD_PEX_CNTRL		31
> +#define TEGRA_IO_PAD_SDMMC1		32
> +#define TEGRA_IO_PAD_SDMMC3		33
> +#define TEGRA_IO_PAD_SDMMC4		34
> +#define TEGRA_IO_PAD_SPI		35
> +#define TEGRA_IO_PAD_SPI_HV		36
> +#define TEGRA_IO_PAD_SYS_DDC		37
> +#define TEGRA_IO_PAD_UART		38
> +#define TEGRA_IO_PAD_USB0		39
> +#define TEGRA_IO_PAD_USB1		40
> +#define TEGRA_IO_PAD_USB2		41
> +#define TEGRA_IO_PAD_USB3		42
> +#define TEGRA_IO_PAD_USB_BIAS		43

Enum?

>  
>  #ifdef CONFIG_ARCH_TEGRA
>  int tegra_powergate_is_powered(unsigned int id);
> @@ -118,8 +138,15 @@ int tegra_powergate_remove_clamping(unsigned int id);
>  int tegra_powergate_sequence_power_up(unsigned int id, struct clk *clk,
>  				      struct reset_control *rst);
>  
> -int tegra_io_rail_power_on(unsigned int id);
> -int tegra_io_rail_power_off(unsigned int id);
> +/* Power enable/disable of the IO pads */
> +int tegra_io_pads_power_enable(int io_pad_id);
> +int tegra_io_pads_power_disable(int io_pad_id);
> +int tegra_io_pads_power_is_enabled(int io_pad_id);
> +
> +/* Set/Get of IO pad voltage */
> +int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv);
> +int tegra_io_pads_get_configured_voltage(int io_pad_id);
> +
>  #else
>  static inline int tegra_powergate_is_powered(unsigned int id)
>  {
> @@ -148,14 +175,29 @@ static inline int tegra_powergate_sequence_power_up(unsigned int id,
>  	return -ENOSYS;
>  }
>  
> -static inline int tegra_io_rail_power_on(unsigned int id)
> +static inline int tegra_io_pads_power_enable(int io_pad_id)
>  {
> -	return -ENOSYS;
> +	return -ENOTSUPP;
>  }
>  
> -static inline int tegra_io_rail_power_off(unsigned int id)
> +static inline int tegra_io_pads_power_disable(int io_pad_id)
>  {
> -	return -ENOSYS;
> +	return -ENOTSUPP;
> +}
> +
> +static inline int tegra_io_pads_power_is_enabled(int io_pad_id)
> +{
> +	return -ENOTSUPP;
> +}
> +
> +static inline int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
> +{
> +	return -ENOTSUPP;
> +}
> +
> +static inline int tegra_io_pads_get_configured_voltage(int io_pad_id)
> +{
> +	return -ENOTSUPP;
>  }
>  #endif /* CONFIG_ARCH_TEGRA */

In genernal this feels like 3 patches ...

1. Patch to add the new APIs
2. Patch to convert SOR to use new APIs
3. Patch to remove old APIs

May be let's see what Thierry thinks.

Cheers
Jon

Re: [PATCH V3 4/4] soc/tegra: pmc: Register PMC child devices as platform device

[Jon Hunter]

On 04/05/16 12:39, Laxman Dewangan wrote:
> Power Management Controller(PMC) of Tegra does the multiple chip
> power related functionality for internal and IO interfacing.
> Some of the functionalities are power gating of IP blocks, IO pads
> voltage and power state configuration, system power state configurations,
> wakeup controls etc.
> 
> Different functionalities of the PMC are provided through different
> framework like IO pads control can be provided through pinctrl framework,
> IO power control is via misc driver etc. All sub functionalities are
> represented as PMC child devices.
> 
> Register the PMC child devices as platform device and fill the child
> devices table for Tegra124 and Tegra210.
> 
> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> ---
> Changes from V1:
> Reworked on DT for having flat entry and register all child devices
> as simple platform device instead of of_populate_device().
> 
> Changes from V2:
> - Handling of the release of child devices on error.
> - Register sub devices for T124 and T210 as T124 supports the DPD of IO
>   pads.
> ---
>  drivers/soc/tegra/pmc.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 80 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 0611cea..0eb652f 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -132,6 +132,8 @@ struct tegra_pmc_soc {
>  	const u8 *cpu_powergates;
>  	const struct tegra_io_pads_control *io_pads_control;
>  	unsigned int num_io_pads;
> +	const char **sub_devs_name;
> +	unsigned int num_sub_devs;
>  	bool has_tsense_reset;
>  	bool has_gpu_clamps;
>  };
> @@ -158,6 +160,7 @@ struct tegra_pmc_soc {
>   * @lp0_vec_size: size of the LP0 warm boot code
>   * @powergates_available: Bitmap of available power gates
>   * @powergates_lock: mutex for power gate register access
> + * @pdevs: Platform device for PMC child devices.
>   */
>  struct tegra_pmc {
>  	struct device *dev;
> @@ -184,6 +187,8 @@ struct tegra_pmc {
>  	DECLARE_BITMAP(powergates_available, TEGRA_POWERGATE_MAX);
>  
>  	struct mutex powergates_lock;
> +
> +	struct platform_device **pdevs;
>  };
>  
>  static struct tegra_pmc *pmc = &(struct tegra_pmc) {
> @@ -1310,6 +1315,57 @@ out:
>  	of_node_put(np);
>  }
>  
> +static int  tegra_pmc_init_sub_devs(struct tegra_pmc *pmc)
> +{
> +	int ret, i;
> +
> +	if (!pmc->soc->num_sub_devs)
> +		return 0;
> +
> +	pmc->pdevs = devm_kzalloc(pmc->dev, pmc->soc->num_sub_devs *
> +				  sizeof(**pmc->pdevs), GFP_KERNEL);
> +	if (!pmc->pdevs)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < pmc->soc->num_sub_devs; ++i) {
> +		pmc->pdevs[i] = platform_device_register_data(pmc->dev,
> +						pmc->soc->sub_devs_name[i],
> +						0, NULL, 0);
> +		if (IS_ERR(pmc->pdevs[i])) {
> +			ret = PTR_ERR(pmc->pdevs[i]);
> +			dev_err(pmc->dev,
> +				"Failed to register platform device for %s: %d\n",
> +				pmc->soc->sub_devs_name[i], ret);
> +			goto pdev_cleanups;
> +		}
> +	}
> +
> +	return 0;
> +
> +pdev_cleanups:
> +	while (--i >= 0) {
> +		platform_device_unregister(pmc->pdevs[i]);
> +		pmc->pdevs[i] = NULL;
> +	}
> +
> +	return ret;
> +}
> +
> +static void tegra_pmc_deinit_sub_devs(struct tegra_pmc *pmc)
> +{
> +	int i;
> +
> +	if (!pmc->soc->num_sub_devs || !pmc->pdevs)
> +		return;
> +
> +	for (i = 0; i < pmc->soc->num_sub_devs; ++i) {
> +		if (pmc->pdevs[i]) {
> +			platform_device_unregister(pmc->pdevs[i]);
> +			pmc->pdevs[i] = NULL;
> +		}
> +	}
> +}
> +
>  static int tegra_pmc_probe(struct platform_device *pdev)
>  {
>  	void __iomem *base;
> @@ -1339,10 +1395,14 @@ static int tegra_pmc_probe(struct platform_device *pdev)
>  
>  	tegra_pmc_init_tsense_reset(pmc);
>  
> +	err = tegra_pmc_init_sub_devs(pmc);
> +	if (err < 0)
> +		dev_warn(pmc->dev, "Failed to register sub devices: %d\n", err);
> +
>  	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
>  		err = tegra_powergate_debugfs_init();
>  		if (err < 0)
> -			return err;
> +			goto cleanups;
>  	}
>  
>  	err = register_restart_handler(&tegra_pmc_restart_handler);
> @@ -1350,7 +1410,7 @@ static int tegra_pmc_probe(struct platform_device *pdev)
>  		debugfs_remove(pmc->debugfs);
>  		dev_err(&pdev->dev, "unable to register restart handler, %d\n",
>  			err);
> -		return err;
> +		goto cleanups;
>  	}
>  
>  	tegra_powergate_init(pmc);
> @@ -1361,6 +1421,11 @@ static int tegra_pmc_probe(struct platform_device *pdev)
>  	mutex_unlock(&pmc->powergates_lock);
>  
>  	return 0;
> +
> +cleanups:
> +	tegra_pmc_deinit_sub_devs(pmc);
> +
> +	return err;
>  }
>  
>  #if defined(CONFIG_PM_SLEEP) && defined(CONFIG_ARM)
> @@ -1544,6 +1609,10 @@ struct tegra_io_pads_control tegra124_io_pads_control[] = {
>  	TEGRA_IO_PADS_CONTROL(SYS_DDC, 58, -1),
>  };
>  
> +static const char *tegra124_sub_devs_name[] = {
> +	"pinctrl-tegra124-io-pad",
> +};
> +
>  static const struct tegra_pmc_soc tegra124_pmc_soc = {
>  	.num_powergates = ARRAY_SIZE(tegra124_powergates),
>  	.powergates = tegra124_powergates,
> @@ -1551,6 +1620,8 @@ static const struct tegra_pmc_soc tegra124_pmc_soc = {
>  	.cpu_powergates = tegra124_cpu_powergates,
>  	.io_pads_control = tegra124_io_pads_control,
>  	.num_io_pads = ARRAY_SIZE(tegra124_io_pads_control),
> +	.sub_devs_name = tegra124_sub_devs_name,
> +	.num_sub_devs = ARRAY_SIZE(tegra124_sub_devs_name),
>  	.has_tsense_reset = true,
>  	.has_gpu_clamps = true,
>  };
> @@ -1628,6 +1699,11 @@ struct tegra_io_pads_control tegra210_io_pads_control[] = {
>  	TEGRA_IO_PADS_CONTROL(LVDS, 57, -1),
>  	TEGRA_IO_PADS_CONTROL(AUDIO_HV, 61, 18),
>  };
> +
> +static const char *tegra210_sub_devs_name[] = {
> +	"pinctrl-tegra210-io-pad",
> +};
> +
>  static const struct tegra_pmc_soc tegra210_pmc_soc = {
>  	.num_powergates = ARRAY_SIZE(tegra210_powergates),
>  	.powergates = tegra210_powergates,
> @@ -1635,6 +1711,8 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
>  	.cpu_powergates = tegra210_cpu_powergates,
>  	.io_pads_control = tegra210_io_pads_control,
>  	.num_io_pads = ARRAY_SIZE(tegra210_io_pads_control),
> +	.sub_devs_name = tegra210_sub_devs_name,
> +	.num_sub_devs = ARRAY_SIZE(tegra210_sub_devs_name),
>  	.has_tsense_reset = true,
>  	.has_gpu_clamps = true,
>  };

I don't think that it makes sense to include this in this series. This
should be with the pinctrl driver.

Jon

Re: [PATCH V3 4/4] soc/tegra: pmc: Register PMC child devices as platform device

[Jon Hunter]

On 04/05/16 12:39, Laxman Dewangan wrote:
> Power Management Controller(PMC) of Tegra does the multiple chip
> power related functionality for internal and IO interfacing.
> Some of the functionalities are power gating of IP blocks, IO pads
> voltage and power state configuration, system power state configurations,
> wakeup controls etc.
> 
> Different functionalities of the PMC are provided through different
> framework like IO pads control can be provided through pinctrl framework,
> IO power control is via misc driver etc. All sub functionalities are
> represented as PMC child devices.
> 
> Register the PMC child devices as platform device and fill the child
> devices table for Tegra124 and Tegra210.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> 
> ---
> Changes from V1:
> Reworked on DT for having flat entry and register all child devices
> as simple platform device instead of of_populate_device().
> 
> Changes from V2:
> - Handling of the release of child devices on error.
> - Register sub devices for T124 and T210 as T124 supports the DPD of IO
>   pads.
> ---
>  drivers/soc/tegra/pmc.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 80 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 0611cea..0eb652f 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -132,6 +132,8 @@ struct tegra_pmc_soc {
>  	const u8 *cpu_powergates;
>  	const struct tegra_io_pads_control *io_pads_control;
>  	unsigned int num_io_pads;
> +	const char **sub_devs_name;
> +	unsigned int num_sub_devs;
>  	bool has_tsense_reset;
>  	bool has_gpu_clamps;
>  };
> @@ -158,6 +160,7 @@ struct tegra_pmc_soc {
>   * @lp0_vec_size: size of the LP0 warm boot code
>   * @powergates_available: Bitmap of available power gates
>   * @powergates_lock: mutex for power gate register access
> + * @pdevs: Platform device for PMC child devices.
>   */
>  struct tegra_pmc {
>  	struct device *dev;
> @@ -184,6 +187,8 @@ struct tegra_pmc {
>  	DECLARE_BITMAP(powergates_available, TEGRA_POWERGATE_MAX);
>  
>  	struct mutex powergates_lock;
> +
> +	struct platform_device **pdevs;
>  };
>  
>  static struct tegra_pmc *pmc = &(struct tegra_pmc) {
> @@ -1310,6 +1315,57 @@ out:
>  	of_node_put(np);
>  }
>  
> +static int  tegra_pmc_init_sub_devs(struct tegra_pmc *pmc)
> +{
> +	int ret, i;
> +
> +	if (!pmc->soc->num_sub_devs)
> +		return 0;
> +
> +	pmc->pdevs = devm_kzalloc(pmc->dev, pmc->soc->num_sub_devs *
> +				  sizeof(**pmc->pdevs), GFP_KERNEL);
> +	if (!pmc->pdevs)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < pmc->soc->num_sub_devs; ++i) {
> +		pmc->pdevs[i] = platform_device_register_data(pmc->dev,
> +						pmc->soc->sub_devs_name[i],
> +						0, NULL, 0);
> +		if (IS_ERR(pmc->pdevs[i])) {
> +			ret = PTR_ERR(pmc->pdevs[i]);
> +			dev_err(pmc->dev,
> +				"Failed to register platform device for %s: %d\n",
> +				pmc->soc->sub_devs_name[i], ret);
> +			goto pdev_cleanups;
> +		}
> +	}
> +
> +	return 0;
> +
> +pdev_cleanups:
> +	while (--i >= 0) {
> +		platform_device_unregister(pmc->pdevs[i]);
> +		pmc->pdevs[i] = NULL;
> +	}
> +
> +	return ret;
> +}
> +
> +static void tegra_pmc_deinit_sub_devs(struct tegra_pmc *pmc)
> +{
> +	int i;
> +
> +	if (!pmc->soc->num_sub_devs || !pmc->pdevs)
> +		return;
> +
> +	for (i = 0; i < pmc->soc->num_sub_devs; ++i) {
> +		if (pmc->pdevs[i]) {
> +			platform_device_unregister(pmc->pdevs[i]);
> +			pmc->pdevs[i] = NULL;
> +		}
> +	}
> +}
> +
>  static int tegra_pmc_probe(struct platform_device *pdev)
>  {
>  	void __iomem *base;
> @@ -1339,10 +1395,14 @@ static int tegra_pmc_probe(struct platform_device *pdev)
>  
>  	tegra_pmc_init_tsense_reset(pmc);
>  
> +	err = tegra_pmc_init_sub_devs(pmc);
> +	if (err < 0)
> +		dev_warn(pmc->dev, "Failed to register sub devices: %d\n", err);
> +
>  	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
>  		err = tegra_powergate_debugfs_init();
>  		if (err < 0)
> -			return err;
> +			goto cleanups;
>  	}
>  
>  	err = register_restart_handler(&tegra_pmc_restart_handler);
> @@ -1350,7 +1410,7 @@ static int tegra_pmc_probe(struct platform_device *pdev)
>  		debugfs_remove(pmc->debugfs);
>  		dev_err(&pdev->dev, "unable to register restart handler, %d\n",
>  			err);
> -		return err;
> +		goto cleanups;
>  	}
>  
>  	tegra_powergate_init(pmc);
> @@ -1361,6 +1421,11 @@ static int tegra_pmc_probe(struct platform_device *pdev)
>  	mutex_unlock(&pmc->powergates_lock);
>  
>  	return 0;
> +
> +cleanups:
> +	tegra_pmc_deinit_sub_devs(pmc);
> +
> +	return err;
>  }
>  
>  #if defined(CONFIG_PM_SLEEP) && defined(CONFIG_ARM)
> @@ -1544,6 +1609,10 @@ struct tegra_io_pads_control tegra124_io_pads_control[] = {
>  	TEGRA_IO_PADS_CONTROL(SYS_DDC, 58, -1),
>  };
>  
> +static const char *tegra124_sub_devs_name[] = {
> +	"pinctrl-tegra124-io-pad",
> +};
> +
>  static const struct tegra_pmc_soc tegra124_pmc_soc = {
>  	.num_powergates = ARRAY_SIZE(tegra124_powergates),
>  	.powergates = tegra124_powergates,
> @@ -1551,6 +1620,8 @@ static const struct tegra_pmc_soc tegra124_pmc_soc = {
>  	.cpu_powergates = tegra124_cpu_powergates,
>  	.io_pads_control = tegra124_io_pads_control,
>  	.num_io_pads = ARRAY_SIZE(tegra124_io_pads_control),
> +	.sub_devs_name = tegra124_sub_devs_name,
> +	.num_sub_devs = ARRAY_SIZE(tegra124_sub_devs_name),
>  	.has_tsense_reset = true,
>  	.has_gpu_clamps = true,
>  };
> @@ -1628,6 +1699,11 @@ struct tegra_io_pads_control tegra210_io_pads_control[] = {
>  	TEGRA_IO_PADS_CONTROL(LVDS, 57, -1),
>  	TEGRA_IO_PADS_CONTROL(AUDIO_HV, 61, 18),
>  };
> +
> +static const char *tegra210_sub_devs_name[] = {
> +	"pinctrl-tegra210-io-pad",
> +};
> +
>  static const struct tegra_pmc_soc tegra210_pmc_soc = {
>  	.num_powergates = ARRAY_SIZE(tegra210_powergates),
>  	.powergates = tegra210_powergates,
> @@ -1635,6 +1711,8 @@ static const struct tegra_pmc_soc tegra210_pmc_soc = {
>  	.cpu_powergates = tegra210_cpu_powergates,
>  	.io_pads_control = tegra210_io_pads_control,
>  	.num_io_pads = ARRAY_SIZE(tegra210_io_pads_control),
> +	.sub_devs_name = tegra210_sub_devs_name,
> +	.num_sub_devs = ARRAY_SIZE(tegra210_sub_devs_name),
>  	.has_tsense_reset = true,
>  	.has_gpu_clamps = true,
>  };

I don't think that it makes sense to include this in this series. This
should be with the pinctrl driver.

Jon

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Laxman Dewangan]

On Thursday 05 May 2016 03:43 PM, Jon Hunter wrote:
> On 04/05/16 12:39, Laxman Dewangan wrote:

> +		return -EINVAL;
> +
> +	for (i = 0; i < soc->num_io_pads; ++i) {
> +		if (soc->io_pads_control[i].pad_id == pad_id)
> +			return soc->io_pads_control[i].dpd_bit_pos;
> +	}
> Do we need a loop here? Can't we just make the table a look-up table now
> that the ID is just an index?

We do not support the table for all pads and so for those non supported 
pad index, it will be 0 (default) and 0 is the valid bit position here.

If you want table then we will need one more information for making that 
index as valid/invalid.
We can pack the valid/invalid with bit position to make u32.


> +	return !!(status & BIT(dpd_bit % 32));
> +}
> +EXPORT_SYMBOL(tegra_io_pads_power_is_enabled);
> +
> +int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
> s/io_pad_id/id/
>
> I think I prefer tegra_io_pads_set/get_voltage_conf(). What is the point
> in passing uV here if in device-tree you are using the enum for the
> voltage level? I know that I had suggested this, but given we are not
> going to use voltage in the DT then, not sure it has any value here.
This is generic interface and hence. So in future if we have more 
option, we will not need change in interface.

Otherwise, make enums for 1.8/3.3 and pass as enum here. So in future if 
we have any other voltage then again add enums.
I wanted to avoid this.



>   
> +#define TEGRA_IO_PADS_CONTROL(_pad, _dpd, _pwr)		\
> +{							\
> +	.pad_id = (TEGRA_IO_PAD_##_pad),		\
> Not sure this needs to be part of the structure as it is just an index.

it is there for matching.

>> +#define TEGRA_IO_PAD_USB2		41
>> +#define TEGRA_IO_PAD_USB3		42
>> +#define TEGRA_IO_PAD_USB_BIAS		43
> Enum?
>

Yaah, that will also be possible. Then then argument is

enum tegra_io_pad_id id

instead of unsigned int.

May be not much benifit here.

>>   
>>
>>   #endif /* CONFIG_ARCH_TEGRA */
> In genernal this feels like 3 patches ...
>
> 1. Patch to add the new APIs
> 2. Patch to convert SOR to use new APIs
> 3. Patch to remove old APIs
>
> May be let's see what Thierry thinks.
>

OK,
I saw when such changes happen, we can have in single patch. This is 
exactly we did in the RTC patches for rtc + MFD.

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Laxman Dewangan]

On Thursday 05 May 2016 03:43 PM, Jon Hunter wrote:
> On 04/05/16 12:39, Laxman Dewangan wrote:

> +		return -EINVAL;
> +
> +	for (i = 0; i < soc->num_io_pads; ++i) {
> +		if (soc->io_pads_control[i].pad_id == pad_id)
> +			return soc->io_pads_control[i].dpd_bit_pos;
> +	}
> Do we need a loop here? Can't we just make the table a look-up table now
> that the ID is just an index?

We do not support the table for all pads and so for those non supported 
pad index, it will be 0 (default) and 0 is the valid bit position here.

If you want table then we will need one more information for making that 
index as valid/invalid.
We can pack the valid/invalid with bit position to make u32.


> +	return !!(status & BIT(dpd_bit % 32));
> +}
> +EXPORT_SYMBOL(tegra_io_pads_power_is_enabled);
> +
> +int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
> s/io_pad_id/id/
>
> I think I prefer tegra_io_pads_set/get_voltage_conf(). What is the point
> in passing uV here if in device-tree you are using the enum for the
> voltage level? I know that I had suggested this, but given we are not
> going to use voltage in the DT then, not sure it has any value here.
This is generic interface and hence. So in future if we have more 
option, we will not need change in interface.

Otherwise, make enums for 1.8/3.3 and pass as enum here. So in future if 
we have any other voltage then again add enums.
I wanted to avoid this.



>   
> +#define TEGRA_IO_PADS_CONTROL(_pad, _dpd, _pwr)		\
> +{							\
> +	.pad_id = (TEGRA_IO_PAD_##_pad),		\
> Not sure this needs to be part of the structure as it is just an index.

it is there for matching.

>> +#define TEGRA_IO_PAD_USB2		41
>> +#define TEGRA_IO_PAD_USB3		42
>> +#define TEGRA_IO_PAD_USB_BIAS		43
> Enum?
>

Yaah, that will also be possible. Then then argument is

enum tegra_io_pad_id id

instead of unsigned int.

May be not much benifit here.

>>   
>>
>>   #endif /* CONFIG_ARCH_TEGRA */
> In genernal this feels like 3 patches ...
>
> 1. Patch to add the new APIs
> 2. Patch to convert SOR to use new APIs
> 3. Patch to remove old APIs
>
> May be let's see what Thierry thinks.
>

OK,
I saw when such changes happen, we can have in single patch. This is 
exactly we did in the RTC patches for rtc + MFD.

Re: [PATCH V3 2/4] soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()

[Laxman Dewangan]

On Thursday 05 May 2016 06:13 PM, Jon Hunter wrote:
> On 05/05/16 10:52, Laxman Dewangan wrote:
>> On Thursday 05 May 2016 03:19 PM, Jon Hunter wrote:
>>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>>> The function tegra_pmc_readl() returns the u32 type data and hence
>>>> change the data type of variable where this data is stored to u32
>>>> type.
>>>>
>>>> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>>
>>>> ---
>>>> Changes from V1:
>>>> -This is new in series as per discussion on V1 series to use u32 for
>>>> tegra_pmc_readl.
>>>>
>>>> Changes from V2:
>>>> - Make unsigned long to u32 for some missed variable from V1.
>>>> ---
>>>>    drivers/soc/tegra/pmc.c | 24 ++++++++++++++----------
>>>>    1 file changed, 14 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>>> index 2c3f1f9..eff9425 100644
>>>> --- a/drivers/soc/tegra/pmc.c
>>>> +++ b/drivers/soc/tegra/pmc.c
>>>> @@ -844,7 +844,8 @@ static void tegra_powergate_init(struct tegra_pmc
>>>> *pmc)
>>>>    static int tegra_io_rail_prepare(unsigned int id, unsigned long
>>>> *request,
>>>>                     unsigned long *status, unsigned int *bit)
>>>>    {
>>>> -    unsigned long rate, value;
>>>> +    unsigned long rate;
>>>> +    u32 value;
>>>>          *bit = id % 32;
>>>>    @@ -868,17 +869,18 @@ static int tegra_io_rail_prepare(unsigned int
>>>> id, unsigned long *request,
>>>>        tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
>>>>          /* must be at least 200 ns, in APB (PCLK) clock cycles */
>>>> -    value = DIV_ROUND_UP(1000000000, rate);
>>>> -    value = DIV_ROUND_UP(200, value);
>>>> +    rate = DIV_ROUND_UP(1000000000, rate);
>>>> +    rate = DIV_ROUND_UP(200, rate);
>>>> +    value = (u32)rate;
>>> Although it is unlikely, I think that we should check it is less
>>> than U32_MAX, return an error if it is not.
>> rate = DIV_ROUNC_UP(200, rate) means
>>
>> rate = (200 + rate -1)/rate
>>
>> and can not be more than 200 in any case (if rate =1).
>> So no need of the error check.
> OK, yes you are right. In that case there is no need to cast and so I
> would leave this code as-is and not change the type.
>

You mean keep value as unsigned long for value?

I think we can still say value as u32 and simply write
value = rate


Just remove the casting.

Re: [PATCH V3 2/4] soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()

[Laxman Dewangan]

On Thursday 05 May 2016 06:13 PM, Jon Hunter wrote:
> On 05/05/16 10:52, Laxman Dewangan wrote:
>> On Thursday 05 May 2016 03:19 PM, Jon Hunter wrote:
>>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>>> The function tegra_pmc_readl() returns the u32 type data and hence
>>>> change the data type of variable where this data is stored to u32
>>>> type.
>>>>
>>>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>>>>
>>>> ---
>>>> Changes from V1:
>>>> -This is new in series as per discussion on V1 series to use u32 for
>>>> tegra_pmc_readl.
>>>>
>>>> Changes from V2:
>>>> - Make unsigned long to u32 for some missed variable from V1.
>>>> ---
>>>>    drivers/soc/tegra/pmc.c | 24 ++++++++++++++----------
>>>>    1 file changed, 14 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>>> index 2c3f1f9..eff9425 100644
>>>> --- a/drivers/soc/tegra/pmc.c
>>>> +++ b/drivers/soc/tegra/pmc.c
>>>> @@ -844,7 +844,8 @@ static void tegra_powergate_init(struct tegra_pmc
>>>> *pmc)
>>>>    static int tegra_io_rail_prepare(unsigned int id, unsigned long
>>>> *request,
>>>>                     unsigned long *status, unsigned int *bit)
>>>>    {
>>>> -    unsigned long rate, value;
>>>> +    unsigned long rate;
>>>> +    u32 value;
>>>>          *bit = id % 32;
>>>>    @@ -868,17 +869,18 @@ static int tegra_io_rail_prepare(unsigned int
>>>> id, unsigned long *request,
>>>>        tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
>>>>          /* must be at least 200 ns, in APB (PCLK) clock cycles */
>>>> -    value = DIV_ROUND_UP(1000000000, rate);
>>>> -    value = DIV_ROUND_UP(200, value);
>>>> +    rate = DIV_ROUND_UP(1000000000, rate);
>>>> +    rate = DIV_ROUND_UP(200, rate);
>>>> +    value = (u32)rate;
>>> Although it is unlikely, I think that we should check it is less
>>> than U32_MAX, return an error if it is not.
>> rate = DIV_ROUNC_UP(200, rate) means
>>
>> rate = (200 + rate -1)/rate
>>
>> and can not be more than 200 in any case (if rate =1).
>> So no need of the error check.
> OK, yes you are right. In that case there is no need to cast and so I
> would leave this code as-is and not change the type.
>

You mean keep value as unsigned long for value?

I think we can still say value as u32 and simply write
value = rate


Just remove the casting.

Re: [PATCH V3 2/4] soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()

[Jon Hunter]

On 05/05/16 10:52, Laxman Dewangan wrote:
> 
> On Thursday 05 May 2016 03:19 PM, Jon Hunter wrote:
>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>> The function tegra_pmc_readl() returns the u32 type data and hence
>>> change the data type of variable where this data is stored to u32
>>> type.
>>>
>>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>>>
>>> ---
>>> Changes from V1:
>>> -This is new in series as per discussion on V1 series to use u32 for
>>> tegra_pmc_readl.
>>>
>>> Changes from V2:
>>> - Make unsigned long to u32 for some missed variable from V1.
>>> ---
>>>   drivers/soc/tegra/pmc.c | 24 ++++++++++++++----------
>>>   1 file changed, 14 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>> index 2c3f1f9..eff9425 100644
>>> --- a/drivers/soc/tegra/pmc.c
>>> +++ b/drivers/soc/tegra/pmc.c
>>> @@ -844,7 +844,8 @@ static void tegra_powergate_init(struct tegra_pmc
>>> *pmc)
>>>   static int tegra_io_rail_prepare(unsigned int id, unsigned long
>>> *request,
>>>                    unsigned long *status, unsigned int *bit)
>>>   {
>>> -    unsigned long rate, value;
>>> +    unsigned long rate;
>>> +    u32 value;
>>>         *bit = id % 32;
>>>   @@ -868,17 +869,18 @@ static int tegra_io_rail_prepare(unsigned int
>>> id, unsigned long *request,
>>>       tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
>>>         /* must be at least 200 ns, in APB (PCLK) clock cycles */
>>> -    value = DIV_ROUND_UP(1000000000, rate);
>>> -    value = DIV_ROUND_UP(200, value);
>>> +    rate = DIV_ROUND_UP(1000000000, rate);
>>> +    rate = DIV_ROUND_UP(200, rate);
>>> +    value = (u32)rate;
>> Although it is unlikely, I think that we should check it is less
>> than U32_MAX, return an error if it is not.
> 
> rate = DIV_ROUNC_UP(200, rate) means
> 
> rate = (200 + rate -1)/rate
> 
> and can not be more than 200 in any case (if rate =1).
> So no need of the error check.

OK, yes you are right. In that case there is no need to cast and so I
would leave this code as-is and not change the type.

Jon




_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V3 2/4] soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()

[Jon Hunter]

On 05/05/16 10:52, Laxman Dewangan wrote:
> 
> On Thursday 05 May 2016 03:19 PM, Jon Hunter wrote:
>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>> The function tegra_pmc_readl() returns the u32 type data and hence
>>> change the data type of variable where this data is stored to u32
>>> type.
>>>
>>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>>>
>>> ---
>>> Changes from V1:
>>> -This is new in series as per discussion on V1 series to use u32 for
>>> tegra_pmc_readl.
>>>
>>> Changes from V2:
>>> - Make unsigned long to u32 for some missed variable from V1.
>>> ---
>>>   drivers/soc/tegra/pmc.c | 24 ++++++++++++++----------
>>>   1 file changed, 14 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>> index 2c3f1f9..eff9425 100644
>>> --- a/drivers/soc/tegra/pmc.c
>>> +++ b/drivers/soc/tegra/pmc.c
>>> @@ -844,7 +844,8 @@ static void tegra_powergate_init(struct tegra_pmc
>>> *pmc)
>>>   static int tegra_io_rail_prepare(unsigned int id, unsigned long
>>> *request,
>>>                    unsigned long *status, unsigned int *bit)
>>>   {
>>> -    unsigned long rate, value;
>>> +    unsigned long rate;
>>> +    u32 value;
>>>         *bit = id % 32;
>>>   @@ -868,17 +869,18 @@ static int tegra_io_rail_prepare(unsigned int
>>> id, unsigned long *request,
>>>       tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
>>>         /* must be at least 200 ns, in APB (PCLK) clock cycles */
>>> -    value = DIV_ROUND_UP(1000000000, rate);
>>> -    value = DIV_ROUND_UP(200, value);
>>> +    rate = DIV_ROUND_UP(1000000000, rate);
>>> +    rate = DIV_ROUND_UP(200, rate);
>>> +    value = (u32)rate;
>> Although it is unlikely, I think that we should check it is less
>> than U32_MAX, return an error if it is not.
> 
> rate = DIV_ROUNC_UP(200, rate) means
> 
> rate = (200 + rate -1)/rate
> 
> and can not be more than 200 in any case (if rate =1).
> So no need of the error check.

OK, yes you are right. In that case there is no need to cast and so I
would leave this code as-is and not change the type.

Jon

Re: [PATCH V3 2/4] soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()

[Jon Hunter]

On 05/05/16 13:35, Laxman Dewangan wrote:
> 
> On Thursday 05 May 2016 06:13 PM, Jon Hunter wrote:
>> On 05/05/16 10:52, Laxman Dewangan wrote:
>>> On Thursday 05 May 2016 03:19 PM, Jon Hunter wrote:
>>>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>>>> The function tegra_pmc_readl() returns the u32 type data and hence
>>>>> change the data type of variable where this data is stored to u32
>>>>> type.
>>>>>
>>>>> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>>>
>>>>> ---
>>>>> Changes from V1:
>>>>> -This is new in series as per discussion on V1 series to use u32 for
>>>>> tegra_pmc_readl.
>>>>>
>>>>> Changes from V2:
>>>>> - Make unsigned long to u32 for some missed variable from V1.
>>>>> ---
>>>>>    drivers/soc/tegra/pmc.c | 24 ++++++++++++++----------
>>>>>    1 file changed, 14 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>>>> index 2c3f1f9..eff9425 100644
>>>>> --- a/drivers/soc/tegra/pmc.c
>>>>> +++ b/drivers/soc/tegra/pmc.c
>>>>> @@ -844,7 +844,8 @@ static void tegra_powergate_init(struct tegra_pmc
>>>>> *pmc)
>>>>>    static int tegra_io_rail_prepare(unsigned int id, unsigned long
>>>>> *request,
>>>>>                     unsigned long *status, unsigned int *bit)
>>>>>    {
>>>>> -    unsigned long rate, value;
>>>>> +    unsigned long rate;
>>>>> +    u32 value;
>>>>>          *bit = id % 32;
>>>>>    @@ -868,17 +869,18 @@ static int tegra_io_rail_prepare(unsigned int
>>>>> id, unsigned long *request,
>>>>>        tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
>>>>>          /* must be at least 200 ns, in APB (PCLK) clock cycles */
>>>>> -    value = DIV_ROUND_UP(1000000000, rate);
>>>>> -    value = DIV_ROUND_UP(200, value);
>>>>> +    rate = DIV_ROUND_UP(1000000000, rate);
>>>>> +    rate = DIV_ROUND_UP(200, rate);
>>>>> +    value = (u32)rate;
>>>> Although it is unlikely, I think that we should check it is less
>>>> than U32_MAX, return an error if it is not.
>>> rate = DIV_ROUNC_UP(200, rate) means
>>>
>>> rate = (200 + rate -1)/rate
>>>
>>> and can not be more than 200 in any case (if rate =1).
>>> So no need of the error check.
>> OK, yes you are right. In that case there is no need to cast and so I
>> would leave this code as-is and not change the type.
>>
> 
> You mean keep value as unsigned long for value?

Yes.

> I think we can still say value as u32 and simply write
> value = rate
> 
> Just remove the casting.

I would not change this at all. I don't see any benefit.

Jon

Re: [PATCH V3 2/4] soc/tegra: pmc: Correct type of variable for tegra_pmc_readl()

[Jon Hunter]

On 05/05/16 13:35, Laxman Dewangan wrote:
> 
> On Thursday 05 May 2016 06:13 PM, Jon Hunter wrote:
>> On 05/05/16 10:52, Laxman Dewangan wrote:
>>> On Thursday 05 May 2016 03:19 PM, Jon Hunter wrote:
>>>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>>>> The function tegra_pmc_readl() returns the u32 type data and hence
>>>>> change the data type of variable where this data is stored to u32
>>>>> type.
>>>>>
>>>>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>>>>>
>>>>> ---
>>>>> Changes from V1:
>>>>> -This is new in series as per discussion on V1 series to use u32 for
>>>>> tegra_pmc_readl.
>>>>>
>>>>> Changes from V2:
>>>>> - Make unsigned long to u32 for some missed variable from V1.
>>>>> ---
>>>>>    drivers/soc/tegra/pmc.c | 24 ++++++++++++++----------
>>>>>    1 file changed, 14 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>>>> index 2c3f1f9..eff9425 100644
>>>>> --- a/drivers/soc/tegra/pmc.c
>>>>> +++ b/drivers/soc/tegra/pmc.c
>>>>> @@ -844,7 +844,8 @@ static void tegra_powergate_init(struct tegra_pmc
>>>>> *pmc)
>>>>>    static int tegra_io_rail_prepare(unsigned int id, unsigned long
>>>>> *request,
>>>>>                     unsigned long *status, unsigned int *bit)
>>>>>    {
>>>>> -    unsigned long rate, value;
>>>>> +    unsigned long rate;
>>>>> +    u32 value;
>>>>>          *bit = id % 32;
>>>>>    @@ -868,17 +869,18 @@ static int tegra_io_rail_prepare(unsigned int
>>>>> id, unsigned long *request,
>>>>>        tegra_pmc_writel(DPD_SAMPLE_ENABLE, DPD_SAMPLE);
>>>>>          /* must be at least 200 ns, in APB (PCLK) clock cycles */
>>>>> -    value = DIV_ROUND_UP(1000000000, rate);
>>>>> -    value = DIV_ROUND_UP(200, value);
>>>>> +    rate = DIV_ROUND_UP(1000000000, rate);
>>>>> +    rate = DIV_ROUND_UP(200, rate);
>>>>> +    value = (u32)rate;
>>>> Although it is unlikely, I think that we should check it is less
>>>> than U32_MAX, return an error if it is not.
>>> rate = DIV_ROUNC_UP(200, rate) means
>>>
>>> rate = (200 + rate -1)/rate
>>>
>>> and can not be more than 200 in any case (if rate =1).
>>> So no need of the error check.
>> OK, yes you are right. In that case there is no need to cast and so I
>> would leave this code as-is and not change the type.
>>
> 
> You mean keep value as unsigned long for value?

Yes.

> I think we can still say value as u32 and simply write
> value = rate
> 
> Just remove the casting.

I would not change this at all. I don't see any benefit.

Jon

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Jon Hunter]

On 05/05/16 11:32, Laxman Dewangan wrote:
> 
> On Thursday 05 May 2016 03:43 PM, Jon Hunter wrote:
>> On 04/05/16 12:39, Laxman Dewangan wrote:
> 
>> +        return -EINVAL;
>> +
>> +    for (i = 0; i < soc->num_io_pads; ++i) {
>> +        if (soc->io_pads_control[i].pad_id == pad_id)
>> +            return soc->io_pads_control[i].dpd_bit_pos;
>> +    }
>> Do we need a loop here? Can't we just make the table a look-up table now
>> that the ID is just an index?
> 
> We do not support the table for all pads and so for those non supported
> pad index, it will be 0 (default) and 0 is the valid bit position here.

That does make it tricky.

> If you want table then we will need one more information for making that
> index as valid/invalid.
> We can pack the valid/invalid with bit position to make u32.

Another option would be, to have a single table for all devices and the
make the valid field a valid mask which has a bit for each SoC.

>> +    return !!(status & BIT(dpd_bit % 32));
>> +}
>> +EXPORT_SYMBOL(tegra_io_pads_power_is_enabled);
>> +
>> +int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
>> s/io_pad_id/id/
>>
>> I think I prefer tegra_io_pads_set/get_voltage_conf(). What is the point
>> in passing uV here if in device-tree you are using the enum for the
>> voltage level? I know that I had suggested this, but given we are not
>> going to use voltage in the DT then, not sure it has any value here.
> This is generic interface and hence. So in future if we have more
> option, we will not need change in interface.

Yes but apart from the SOR driver should only be used by the pinctrl
driver (I hope).

> Otherwise, make enums for 1.8/3.3 and pass as enum here. So in future if
> we have any other voltage then again add enums.
> I wanted to avoid this.

You already have added the enum for the pinctrl driver and you would
have to change that enum in the future anyway. So why not use it here?

>>   +#define TEGRA_IO_PADS_CONTROL(_pad, _dpd, _pwr)        \
>> +{                            \
>> +    .pad_id = (TEGRA_IO_PAD_##_pad),        \
>> Not sure this needs to be part of the structure as it is just an index.
> 
> it is there for matching.
> 
>>> +#define TEGRA_IO_PAD_USB2        41
>>> +#define TEGRA_IO_PAD_USB3        42
>>> +#define TEGRA_IO_PAD_USB_BIAS        43
>> Enum?
>>
> 
> Yaah, that will also be possible. Then then argument is
> 
> enum tegra_io_pad_id id
> 
> instead of unsigned int.
> 
> May be not much benifit here.

I think that this is exactly what enums are for, then you don't have to
explicitly define each number.

Cheers
Jon

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Jon Hunter]

On 05/05/16 11:32, Laxman Dewangan wrote:
> 
> On Thursday 05 May 2016 03:43 PM, Jon Hunter wrote:
>> On 04/05/16 12:39, Laxman Dewangan wrote:
> 
>> +        return -EINVAL;
>> +
>> +    for (i = 0; i < soc->num_io_pads; ++i) {
>> +        if (soc->io_pads_control[i].pad_id == pad_id)
>> +            return soc->io_pads_control[i].dpd_bit_pos;
>> +    }
>> Do we need a loop here? Can't we just make the table a look-up table now
>> that the ID is just an index?
> 
> We do not support the table for all pads and so for those non supported
> pad index, it will be 0 (default) and 0 is the valid bit position here.

That does make it tricky.

> If you want table then we will need one more information for making that
> index as valid/invalid.
> We can pack the valid/invalid with bit position to make u32.

Another option would be, to have a single table for all devices and the
make the valid field a valid mask which has a bit for each SoC.

>> +    return !!(status & BIT(dpd_bit % 32));
>> +}
>> +EXPORT_SYMBOL(tegra_io_pads_power_is_enabled);
>> +
>> +int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
>> s/io_pad_id/id/
>>
>> I think I prefer tegra_io_pads_set/get_voltage_conf(). What is the point
>> in passing uV here if in device-tree you are using the enum for the
>> voltage level? I know that I had suggested this, but given we are not
>> going to use voltage in the DT then, not sure it has any value here.
> This is generic interface and hence. So in future if we have more
> option, we will not need change in interface.

Yes but apart from the SOR driver should only be used by the pinctrl
driver (I hope).

> Otherwise, make enums for 1.8/3.3 and pass as enum here. So in future if
> we have any other voltage then again add enums.
> I wanted to avoid this.

You already have added the enum for the pinctrl driver and you would
have to change that enum in the future anyway. So why not use it here?

>>   +#define TEGRA_IO_PADS_CONTROL(_pad, _dpd, _pwr)        \
>> +{                            \
>> +    .pad_id = (TEGRA_IO_PAD_##_pad),        \
>> Not sure this needs to be part of the structure as it is just an index.
> 
> it is there for matching.
> 
>>> +#define TEGRA_IO_PAD_USB2        41
>>> +#define TEGRA_IO_PAD_USB3        42
>>> +#define TEGRA_IO_PAD_USB_BIAS        43
>> Enum?
>>
> 
> Yaah, that will also be possible. Then then argument is
> 
> enum tegra_io_pad_id id
> 
> instead of unsigned int.
> 
> May be not much benifit here.

I think that this is exactly what enums are for, then you don't have to
explicitly define each number.

Cheers
Jon

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Laxman Dewangan]

On Thursday 05 May 2016 06:38 PM, Jon Hunter wrote:
> On 05/05/16 11:32, Laxman Dewangan wrote:
>> On Thursday 05 May 2016 03:43 PM, Jon Hunter wrote:
>>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>> +        return -EINVAL;
>>> +
>>> +    for (i = 0; i < soc->num_io_pads; ++i) {
>>> +        if (soc->io_pads_control[i].pad_id == pad_id)
>>> +            return soc->io_pads_control[i].dpd_bit_pos;
>>> +    }
>>> Do we need a loop here? Can't we just make the table a look-up table now
>>> that the ID is just an index?
>> We do not support the table for all pads and so for those non supported
>> pad index, it will be 0 (default) and 0 is the valid bit position here.
> That does make it tricky.
>
>> If you want table then we will need one more information for making that
>> index as valid/invalid.
>> We can pack the valid/invalid with bit position to make u32.
> Another option would be, to have a single table for all devices and the
> make the valid field a valid mask which has a bit for each SoC.

We have 2 register for DPD and hence making the mask bit will need u64.

I think we can have like below to avoid loop.
struct tegra_io_pads_control {
         int dpd_supported;
         int voltage_change_supported;
         int dpd_config_bit;
         int voltage_config_bit;
};


And the *_supported will be true for those supported pads.

Logic will be
if (soc->io_pads_control[id].dpd_supported)
      return soc->io_pads_control[id].dpd_config_bit;
else
      return -ENOTSUPP;

There is no loop in this.
Infact we will not need additional function here and no need to 
initialize the non-supported pads also.

Same for voltage config bit also.



>>> +    return !!(status & BIT(dpd_bit % 32));
>>> +}
>>> +EXPORT_SYMBOL(tegra_io_pads_power_is_enabled);
>>> +
>>> +int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
>>> s/io_pad_id/id/
>>>
>>> I think I prefer tegra_io_pads_set/get_voltage_conf(). What is the point
>>> in passing uV here if in device-tree you are using the enum for the
>>> voltage level? I know that I had suggested this, but given we are not
>>> going to use voltage in the DT then, not sure it has any value here.
>> This is generic interface and hence. So in future if we have more
>> option, we will not need change in interface.
> Yes but apart from the SOR driver should only be used by the pinctrl
> driver (I hope).
>
>> Otherwise, make enums for 1.8/3.3 and pass as enum here. So in future if
>> we have any other voltage then again add enums.
>> I wanted to avoid this.
> You already have added the enum for the pinctrl driver and you would
> have to change that enum in the future anyway. So why not use it here?
>
>>>    +#define TEGRA_IO_PADS_CONTROL(_pad, _dpd, _pwr)        \
>>> +{                            \
>>> +    .pad_id = (TEGRA_IO_PAD_##_pad),        \
>>> Not sure this needs to be part of the structure as it is just an index.
>> it is there for matching.
>>
>>>> +#define TEGRA_IO_PAD_USB2        41
>>>> +#define TEGRA_IO_PAD_USB3        42
>>>> +#define TEGRA_IO_PAD_USB_BIAS        43
>>> Enum?
>>>
>> Yaah, that will also be possible. Then then argument is
>>
>> enum tegra_io_pad_id id
>>
>> instead of unsigned int.
>>
>> May be not much benifit here.
> I think that this is exactly what enums are for, then you don't have to
> explicitly define each number.
>
We have defines in the dt binding header.
OK, let me expose the enums from pmc header and use this.


BTW, are you fine to keep TEGRA_IO_PAD_* as defines instead of enums.
This is what POWERGATE are there.

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Laxman Dewangan]

On Thursday 05 May 2016 06:38 PM, Jon Hunter wrote:
> On 05/05/16 11:32, Laxman Dewangan wrote:
>> On Thursday 05 May 2016 03:43 PM, Jon Hunter wrote:
>>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>> +        return -EINVAL;
>>> +
>>> +    for (i = 0; i < soc->num_io_pads; ++i) {
>>> +        if (soc->io_pads_control[i].pad_id == pad_id)
>>> +            return soc->io_pads_control[i].dpd_bit_pos;
>>> +    }
>>> Do we need a loop here? Can't we just make the table a look-up table now
>>> that the ID is just an index?
>> We do not support the table for all pads and so for those non supported
>> pad index, it will be 0 (default) and 0 is the valid bit position here.
> That does make it tricky.
>
>> If you want table then we will need one more information for making that
>> index as valid/invalid.
>> We can pack the valid/invalid with bit position to make u32.
> Another option would be, to have a single table for all devices and the
> make the valid field a valid mask which has a bit for each SoC.

We have 2 register for DPD and hence making the mask bit will need u64.

I think we can have like below to avoid loop.
struct tegra_io_pads_control {
         int dpd_supported;
         int voltage_change_supported;
         int dpd_config_bit;
         int voltage_config_bit;
};


And the *_supported will be true for those supported pads.

Logic will be
if (soc->io_pads_control[id].dpd_supported)
      return soc->io_pads_control[id].dpd_config_bit;
else
      return -ENOTSUPP;

There is no loop in this.
Infact we will not need additional function here and no need to 
initialize the non-supported pads also.

Same for voltage config bit also.



>>> +    return !!(status & BIT(dpd_bit % 32));
>>> +}
>>> +EXPORT_SYMBOL(tegra_io_pads_power_is_enabled);
>>> +
>>> +int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
>>> s/io_pad_id/id/
>>>
>>> I think I prefer tegra_io_pads_set/get_voltage_conf(). What is the point
>>> in passing uV here if in device-tree you are using the enum for the
>>> voltage level? I know that I had suggested this, but given we are not
>>> going to use voltage in the DT then, not sure it has any value here.
>> This is generic interface and hence. So in future if we have more
>> option, we will not need change in interface.
> Yes but apart from the SOR driver should only be used by the pinctrl
> driver (I hope).
>
>> Otherwise, make enums for 1.8/3.3 and pass as enum here. So in future if
>> we have any other voltage then again add enums.
>> I wanted to avoid this.
> You already have added the enum for the pinctrl driver and you would
> have to change that enum in the future anyway. So why not use it here?
>
>>>    +#define TEGRA_IO_PADS_CONTROL(_pad, _dpd, _pwr)        \
>>> +{                            \
>>> +    .pad_id = (TEGRA_IO_PAD_##_pad),        \
>>> Not sure this needs to be part of the structure as it is just an index.
>> it is there for matching.
>>
>>>> +#define TEGRA_IO_PAD_USB2        41
>>>> +#define TEGRA_IO_PAD_USB3        42
>>>> +#define TEGRA_IO_PAD_USB_BIAS        43
>>> Enum?
>>>
>> Yaah, that will also be possible. Then then argument is
>>
>> enum tegra_io_pad_id id
>>
>> instead of unsigned int.
>>
>> May be not much benifit here.
> I think that this is exactly what enums are for, then you don't have to
> explicitly define each number.
>
We have defines in the dt binding header.
OK, let me expose the enums from pmc header and use this.


BTW, are you fine to keep TEGRA_IO_PAD_* as defines instead of enums.
This is what POWERGATE are there.

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Jon Hunter]

On 05/05/16 14:09, Laxman Dewangan wrote:
> 
> On Thursday 05 May 2016 06:38 PM, Jon Hunter wrote:
>> On 05/05/16 11:32, Laxman Dewangan wrote:
>>> On Thursday 05 May 2016 03:43 PM, Jon Hunter wrote:
>>>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>>> +        return -EINVAL;
>>>> +
>>>> +    for (i = 0; i < soc->num_io_pads; ++i) {
>>>> +        if (soc->io_pads_control[i].pad_id == pad_id)
>>>> +            return soc->io_pads_control[i].dpd_bit_pos;
>>>> +    }
>>>> Do we need a loop here? Can't we just make the table a look-up table
>>>> now
>>>> that the ID is just an index?
>>> We do not support the table for all pads and so for those non supported
>>> pad index, it will be 0 (default) and 0 is the valid bit position here.
>> That does make it tricky.
>>
>>> If you want table then we will need one more information for making that
>>> index as valid/invalid.
>>> We can pack the valid/invalid with bit position to make u32.
>> Another option would be, to have a single table for all devices and the
>> make the valid field a valid mask which has a bit for each SoC.
> 
> We have 2 register for DPD and hence making the mask bit will need u64.
> 
> I think we can have like below to avoid loop.
> struct tegra_io_pads_control {
>         int dpd_supported;
>         int voltage_change_supported;
>         int dpd_config_bit;
>         int voltage_config_bit;
> };

Why can't we have ...

struct tegra_io_pads_control {
        int dpd_config_bit;
        int voltage_config_bit;
	unsigned int soc_mask;
};

Then .valid should indicate if it the IO pads group is valid for the
device ...

	.soc_mask = TEGRA_IO_PADS_T124
or
	.soc_mask = TEGRA_IO_PADS_T210
or
	.soc_mask = TEGRA_IO_PADS_T124 | TEGRA_IO_PADS_T210

You can use -1 to indicate the for the dpd and voltage bit to indicate
if they are valid. In other words, you need to check the IO pad is valid
for the soc and then the bit is valid.

>>>> +    return !!(status & BIT(dpd_bit % 32));
>>>> +}
>>>> +EXPORT_SYMBOL(tegra_io_pads_power_is_enabled);
>>>> +
>>>> +int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
>>>> s/io_pad_id/id/
>>>>
>>>> I think I prefer tegra_io_pads_set/get_voltage_conf(). What is the
>>>> point
>>>> in passing uV here if in device-tree you are using the enum for the
>>>> voltage level? I know that I had suggested this, but given we are not
>>>> going to use voltage in the DT then, not sure it has any value here.
>>> This is generic interface and hence. So in future if we have more
>>> option, we will not need change in interface.
>> Yes but apart from the SOR driver should only be used by the pinctrl
>> driver (I hope).
>>
>>> Otherwise, make enums for 1.8/3.3 and pass as enum here. So in future if
>>> we have any other voltage then again add enums.
>>> I wanted to avoid this.
>> You already have added the enum for the pinctrl driver and you would
>> have to change that enum in the future anyway. So why not use it here?
>>
>>>>    +#define TEGRA_IO_PADS_CONTROL(_pad, _dpd, _pwr)        \
>>>> +{                            \
>>>> +    .pad_id = (TEGRA_IO_PAD_##_pad),        \
>>>> Not sure this needs to be part of the structure as it is just an index.
>>> it is there for matching.
>>>
>>>>> +#define TEGRA_IO_PAD_USB2        41
>>>>> +#define TEGRA_IO_PAD_USB3        42
>>>>> +#define TEGRA_IO_PAD_USB_BIAS        43
>>>> Enum?
>>>>
>>> Yaah, that will also be possible. Then then argument is
>>>
>>> enum tegra_io_pad_id id
>>>
>>> instead of unsigned int.
>>>
>>> May be not much benifit here.
>> I think that this is exactly what enums are for, then you don't have to
>> explicitly define each number.
>>
> We have defines in the dt binding header.

Nothing to stop us including the dt binding header in the pmc.c. We do
this for tegra clks.

> BTW, are you fine to keep TEGRA_IO_PAD_* as defines instead of enums.
> This is what POWERGATE are there.

Up to you, I prefer an enum. The POWERGATE IDs defines match the bit in
the register so it makes sense these are explicit.

Cheers
Jon

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Jon Hunter]

On 05/05/16 14:09, Laxman Dewangan wrote:
> 
> On Thursday 05 May 2016 06:38 PM, Jon Hunter wrote:
>> On 05/05/16 11:32, Laxman Dewangan wrote:
>>> On Thursday 05 May 2016 03:43 PM, Jon Hunter wrote:
>>>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>>> +        return -EINVAL;
>>>> +
>>>> +    for (i = 0; i < soc->num_io_pads; ++i) {
>>>> +        if (soc->io_pads_control[i].pad_id == pad_id)
>>>> +            return soc->io_pads_control[i].dpd_bit_pos;
>>>> +    }
>>>> Do we need a loop here? Can't we just make the table a look-up table
>>>> now
>>>> that the ID is just an index?
>>> We do not support the table for all pads and so for those non supported
>>> pad index, it will be 0 (default) and 0 is the valid bit position here.
>> That does make it tricky.
>>
>>> If you want table then we will need one more information for making that
>>> index as valid/invalid.
>>> We can pack the valid/invalid with bit position to make u32.
>> Another option would be, to have a single table for all devices and the
>> make the valid field a valid mask which has a bit for each SoC.
> 
> We have 2 register for DPD and hence making the mask bit will need u64.
> 
> I think we can have like below to avoid loop.
> struct tegra_io_pads_control {
>         int dpd_supported;
>         int voltage_change_supported;
>         int dpd_config_bit;
>         int voltage_config_bit;
> };

Why can't we have ...

struct tegra_io_pads_control {
        int dpd_config_bit;
        int voltage_config_bit;
	unsigned int soc_mask;
};

Then .valid should indicate if it the IO pads group is valid for the
device ...

	.soc_mask = TEGRA_IO_PADS_T124
or
	.soc_mask = TEGRA_IO_PADS_T210
or
	.soc_mask = TEGRA_IO_PADS_T124 | TEGRA_IO_PADS_T210

You can use -1 to indicate the for the dpd and voltage bit to indicate
if they are valid. In other words, you need to check the IO pad is valid
for the soc and then the bit is valid.

>>>> +    return !!(status & BIT(dpd_bit % 32));
>>>> +}
>>>> +EXPORT_SYMBOL(tegra_io_pads_power_is_enabled);
>>>> +
>>>> +int tegra_io_pads_configure_voltage(int io_pad_id, int io_volt_uv)
>>>> s/io_pad_id/id/
>>>>
>>>> I think I prefer tegra_io_pads_set/get_voltage_conf(). What is the
>>>> point
>>>> in passing uV here if in device-tree you are using the enum for the
>>>> voltage level? I know that I had suggested this, but given we are not
>>>> going to use voltage in the DT then, not sure it has any value here.
>>> This is generic interface and hence. So in future if we have more
>>> option, we will not need change in interface.
>> Yes but apart from the SOR driver should only be used by the pinctrl
>> driver (I hope).
>>
>>> Otherwise, make enums for 1.8/3.3 and pass as enum here. So in future if
>>> we have any other voltage then again add enums.
>>> I wanted to avoid this.
>> You already have added the enum for the pinctrl driver and you would
>> have to change that enum in the future anyway. So why not use it here?
>>
>>>>    +#define TEGRA_IO_PADS_CONTROL(_pad, _dpd, _pwr)        \
>>>> +{                            \
>>>> +    .pad_id = (TEGRA_IO_PAD_##_pad),        \
>>>> Not sure this needs to be part of the structure as it is just an index.
>>> it is there for matching.
>>>
>>>>> +#define TEGRA_IO_PAD_USB2        41
>>>>> +#define TEGRA_IO_PAD_USB3        42
>>>>> +#define TEGRA_IO_PAD_USB_BIAS        43
>>>> Enum?
>>>>
>>> Yaah, that will also be possible. Then then argument is
>>>
>>> enum tegra_io_pad_id id
>>>
>>> instead of unsigned int.
>>>
>>> May be not much benifit here.
>> I think that this is exactly what enums are for, then you don't have to
>> explicitly define each number.
>>
> We have defines in the dt binding header.

Nothing to stop us including the dt binding header in the pmc.c. We do
this for tegra clks.

> BTW, are you fine to keep TEGRA_IO_PAD_* as defines instead of enums.
> This is what POWERGATE are there.

Up to you, I prefer an enum. The POWERGATE IDs defines match the bit in
the register so it makes sense these are explicit.

Cheers
Jon

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Laxman Dewangan]

On Thursday 05 May 2016 07:03 PM, Jon Hunter wrote:
> On 05/05/16 14:09, Laxman Dewangan wrote:
>> On Thursday 05 May 2016 06:38 PM, Jon Hunter wrote:
>>> On 05/05/16 11:32, Laxman Dewangan wrote:
>>>> On Thursday 05 May 2016 03:43 PM, Jon Hunter wrote:
>>>>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    for (i = 0; i < soc->num_io_pads; ++i) {
>>>>> +        if (soc->io_pads_control[i].pad_id == pad_id)
>>>>> +            return soc->io_pads_control[i].dpd_bit_pos;
>>>>> +    }
>>>>> Do we need a loop here? Can't we just make the table a look-up table
>>>>> now
>>>>> that the ID is just an index?
>>>> We do not support the table for all pads and so for those non supported
>>>> pad index, it will be 0 (default) and 0 is the valid bit position here.
>>> That does make it tricky.
>>>
>>>> If you want table then we will need one more information for making that
>>>> index as valid/invalid.
>>>> We can pack the valid/invalid with bit position to make u32.
>>> Another option would be, to have a single table for all devices and the
>>> make the valid field a valid mask which has a bit for each SoC.
>> We have 2 register for DPD and hence making the mask bit will need u64.
>>
>> I think we can have like below to avoid loop.
>> struct tegra_io_pads_control {
>>          int dpd_supported;
>>          int voltage_change_supported;
>>          int dpd_config_bit;
>>          int voltage_config_bit;
>> };
> Why can't we have ...
>
> struct tegra_io_pads_control {
>          int dpd_config_bit;
>          int voltage_config_bit;
> 	unsigned int soc_mask;
> };
>
> Then .valid should indicate if it the IO pads group is valid for the
> device ...
>
> 	.soc_mask = TEGRA_IO_PADS_T124
> or
> 	.soc_mask = TEGRA_IO_PADS_T210
> or
> 	.soc_mask = TEGRA_IO_PADS_T124 | TEGRA_IO_PADS_T210
>
> You can use -1 to indicate the for the dpd and voltage bit to indicate
> if they are valid. In other words, you need to check the IO pad is valid
> for the soc and then the bit is valid.

This will also work.
Then this is not required part of the soc data.
Only soc_io_mask need to be part of soc data.
This table can be file static.


>>>
>>> I think that this is exactly what enums are for, then you don't have to
>>> explicitly define each number.
>>>
>> We have defines in the dt binding header.
> Nothing to stop us including the dt binding header in the pmc.c. We do
> this for tegra clks.
The dt binding header is not there and need to add.
Part of this patch or different patch?

Also we can not have enums in binding header. Only macros/defines.

We do not have the dt binding doc yet as it will be in future patch.


>> BTW, are you fine to keep TEGRA_IO_PAD_* as defines instead of enums.
>> This is what POWERGATE are there.
> Up to you, I prefer an enum. The POWERGATE IDs defines match the bit in
> the register so it makes sense these are explicit.
>
OK, let me make enums. Last member as MAX so that I can initialize the 
tegra_io_pad_info[TEGRA_IO_PAD_MAX] =

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Laxman Dewangan]

On Thursday 05 May 2016 07:03 PM, Jon Hunter wrote:
> On 05/05/16 14:09, Laxman Dewangan wrote:
>> On Thursday 05 May 2016 06:38 PM, Jon Hunter wrote:
>>> On 05/05/16 11:32, Laxman Dewangan wrote:
>>>> On Thursday 05 May 2016 03:43 PM, Jon Hunter wrote:
>>>>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>>>> +        return -EINVAL;
>>>>> +
>>>>> +    for (i = 0; i < soc->num_io_pads; ++i) {
>>>>> +        if (soc->io_pads_control[i].pad_id == pad_id)
>>>>> +            return soc->io_pads_control[i].dpd_bit_pos;
>>>>> +    }
>>>>> Do we need a loop here? Can't we just make the table a look-up table
>>>>> now
>>>>> that the ID is just an index?
>>>> We do not support the table for all pads and so for those non supported
>>>> pad index, it will be 0 (default) and 0 is the valid bit position here.
>>> That does make it tricky.
>>>
>>>> If you want table then we will need one more information for making that
>>>> index as valid/invalid.
>>>> We can pack the valid/invalid with bit position to make u32.
>>> Another option would be, to have a single table for all devices and the
>>> make the valid field a valid mask which has a bit for each SoC.
>> We have 2 register for DPD and hence making the mask bit will need u64.
>>
>> I think we can have like below to avoid loop.
>> struct tegra_io_pads_control {
>>          int dpd_supported;
>>          int voltage_change_supported;
>>          int dpd_config_bit;
>>          int voltage_config_bit;
>> };
> Why can't we have ...
>
> struct tegra_io_pads_control {
>          int dpd_config_bit;
>          int voltage_config_bit;
> 	unsigned int soc_mask;
> };
>
> Then .valid should indicate if it the IO pads group is valid for the
> device ...
>
> 	.soc_mask = TEGRA_IO_PADS_T124
> or
> 	.soc_mask = TEGRA_IO_PADS_T210
> or
> 	.soc_mask = TEGRA_IO_PADS_T124 | TEGRA_IO_PADS_T210
>
> You can use -1 to indicate the for the dpd and voltage bit to indicate
> if they are valid. In other words, you need to check the IO pad is valid
> for the soc and then the bit is valid.

This will also work.
Then this is not required part of the soc data.
Only soc_io_mask need to be part of soc data.
This table can be file static.


>>>
>>> I think that this is exactly what enums are for, then you don't have to
>>> explicitly define each number.
>>>
>> We have defines in the dt binding header.
> Nothing to stop us including the dt binding header in the pmc.c. We do
> this for tegra clks.
The dt binding header is not there and need to add.
Part of this patch or different patch?

Also we can not have enums in binding header. Only macros/defines.

We do not have the dt binding doc yet as it will be in future patch.


>> BTW, are you fine to keep TEGRA_IO_PAD_* as defines instead of enums.
>> This is what POWERGATE are there.
> Up to you, I prefer an enum. The POWERGATE IDs defines match the bit in
> the register so it makes sense these are explicit.
>
OK, let me make enums. Last member as MAX so that I can initialize the 
tegra_io_pad_info[TEGRA_IO_PAD_MAX] =

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Jon Hunter]

On 05/05/16 14:35, Laxman Dewangan wrote:
> 
> On Thursday 05 May 2016 07:03 PM, Jon Hunter wrote:
>> On 05/05/16 14:09, Laxman Dewangan wrote:
>>> On Thursday 05 May 2016 06:38 PM, Jon Hunter wrote:
>>>> On 05/05/16 11:32, Laxman Dewangan wrote:
>>>>> On Thursday 05 May 2016 03:43 PM, Jon Hunter wrote:
>>>>>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>>>>> +        return -EINVAL;
>>>>>> +
>>>>>> +    for (i = 0; i < soc->num_io_pads; ++i) {
>>>>>> +        if (soc->io_pads_control[i].pad_id == pad_id)
>>>>>> +            return soc->io_pads_control[i].dpd_bit_pos;
>>>>>> +    }
>>>>>> Do we need a loop here? Can't we just make the table a look-up table
>>>>>> now
>>>>>> that the ID is just an index?
>>>>> We do not support the table for all pads and so for those non
>>>>> supported
>>>>> pad index, it will be 0 (default) and 0 is the valid bit position
>>>>> here.
>>>> That does make it tricky.
>>>>
>>>>> If you want table then we will need one more information for making
>>>>> that
>>>>> index as valid/invalid.
>>>>> We can pack the valid/invalid with bit position to make u32.
>>>> Another option would be, to have a single table for all devices and the
>>>> make the valid field a valid mask which has a bit for each SoC.
>>> We have 2 register for DPD and hence making the mask bit will need u64.
>>>
>>> I think we can have like below to avoid loop.
>>> struct tegra_io_pads_control {
>>>          int dpd_supported;
>>>          int voltage_change_supported;
>>>          int dpd_config_bit;
>>>          int voltage_config_bit;
>>> };
>> Why can't we have ...
>>
>> struct tegra_io_pads_control {
>>          int dpd_config_bit;
>>          int voltage_config_bit;
>>     unsigned int soc_mask;
>> };
>>
>> Then .valid should indicate if it the IO pads group is valid for the
>> device ...
>>
>>     .soc_mask = TEGRA_IO_PADS_T124
>> or
>>     .soc_mask = TEGRA_IO_PADS_T210
>> or
>>     .soc_mask = TEGRA_IO_PADS_T124 | TEGRA_IO_PADS_T210
>>
>> You can use -1 to indicate the for the dpd and voltage bit to indicate
>> if they are valid. In other words, you need to check the IO pad is valid
>> for the soc and then the bit is valid.
> 
> This will also work.
> Then this is not required part of the soc data.
> Only soc_io_mask need to be part of soc data.
> This table can be file static.

Ok.


>>>>
>>>> I think that this is exactly what enums are for, then you don't have to
>>>> explicitly define each number.
>>>>
>>> We have defines in the dt binding header.
>> Nothing to stop us including the dt binding header in the pmc.c. We do
>> this for tegra clks.
> The dt binding header is not there and need to add.
> Part of this patch or different patch?
> 
> Also we can not have enums in binding header. Only macros/defines.
> 
> We do not have the dt binding doc yet as it will be in future patch.

OK. For now put it in the pmc.h and we can always move to dt-bindings later.

>>> BTW, are you fine to keep TEGRA_IO_PAD_* as defines instead of enums.
>>> This is what POWERGATE are there.
>> Up to you, I prefer an enum. The POWERGATE IDs defines match the bit in
>> the register so it makes sense these are explicit.
>>
> OK, let me make enums. Last member as MAX so that I can initialize the
> tegra_io_pad_info[TEGRA_IO_PAD_MAX] =

OK.

Cheers
Jon

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH V3 3/4] soc/tegra: pmc: Add support for IO pads power state and voltage

[Jon Hunter]

On 05/05/16 14:35, Laxman Dewangan wrote:
> 
> On Thursday 05 May 2016 07:03 PM, Jon Hunter wrote:
>> On 05/05/16 14:09, Laxman Dewangan wrote:
>>> On Thursday 05 May 2016 06:38 PM, Jon Hunter wrote:
>>>> On 05/05/16 11:32, Laxman Dewangan wrote:
>>>>> On Thursday 05 May 2016 03:43 PM, Jon Hunter wrote:
>>>>>> On 04/05/16 12:39, Laxman Dewangan wrote:
>>>>>> +        return -EINVAL;
>>>>>> +
>>>>>> +    for (i = 0; i < soc->num_io_pads; ++i) {
>>>>>> +        if (soc->io_pads_control[i].pad_id == pad_id)
>>>>>> +            return soc->io_pads_control[i].dpd_bit_pos;
>>>>>> +    }
>>>>>> Do we need a loop here? Can't we just make the table a look-up table
>>>>>> now
>>>>>> that the ID is just an index?
>>>>> We do not support the table for all pads and so for those non
>>>>> supported
>>>>> pad index, it will be 0 (default) and 0 is the valid bit position
>>>>> here.
>>>> That does make it tricky.
>>>>
>>>>> If you want table then we will need one more information for making
>>>>> that
>>>>> index as valid/invalid.
>>>>> We can pack the valid/invalid with bit position to make u32.
>>>> Another option would be, to have a single table for all devices and the
>>>> make the valid field a valid mask which has a bit for each SoC.
>>> We have 2 register for DPD and hence making the mask bit will need u64.
>>>
>>> I think we can have like below to avoid loop.
>>> struct tegra_io_pads_control {
>>>          int dpd_supported;
>>>          int voltage_change_supported;
>>>          int dpd_config_bit;
>>>          int voltage_config_bit;
>>> };
>> Why can't we have ...
>>
>> struct tegra_io_pads_control {
>>          int dpd_config_bit;
>>          int voltage_config_bit;
>>     unsigned int soc_mask;
>> };
>>
>> Then .valid should indicate if it the IO pads group is valid for the
>> device ...
>>
>>     .soc_mask = TEGRA_IO_PADS_T124
>> or
>>     .soc_mask = TEGRA_IO_PADS_T210
>> or
>>     .soc_mask = TEGRA_IO_PADS_T124 | TEGRA_IO_PADS_T210
>>
>> You can use -1 to indicate the for the dpd and voltage bit to indicate
>> if they are valid. In other words, you need to check the IO pad is valid
>> for the soc and then the bit is valid.
> 
> This will also work.
> Then this is not required part of the soc data.
> Only soc_io_mask need to be part of soc data.
> This table can be file static.

Ok.


>>>>
>>>> I think that this is exactly what enums are for, then you don't have to
>>>> explicitly define each number.
>>>>
>>> We have defines in the dt binding header.
>> Nothing to stop us including the dt binding header in the pmc.c. We do
>> this for tegra clks.
> The dt binding header is not there and need to add.
> Part of this patch or different patch?
> 
> Also we can not have enums in binding header. Only macros/defines.
> 
> We do not have the dt binding doc yet as it will be in future patch.

OK. For now put it in the pmc.h and we can always move to dt-bindings later.

>>> BTW, are you fine to keep TEGRA_IO_PAD_* as defines instead of enums.
>>> This is what POWERGATE are there.
>> Up to you, I prefer an enum. The POWERGATE IDs defines match the bit in
>> the register so it makes sense these are explicit.
>>
> OK, let me make enums. Last member as MAX so that I can initialize the
> tegra_io_pad_info[TEGRA_IO_PAD_MAX] =

OK.

Cheers
Jon