[pm_wip/voltdm_nm][PATCH v2 0/4] OMAP4: PM: Voltage cleanups

[pm_wip/voltdm_nm][PATCH v2 0/4] OMAP4: PM: Voltage cleanups

[Nishanth Menon]

The following patches are on top of:
git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git
branch: pm-wip/voltdm_nm

Tested on: SDP4430

This is v2 of the series (of which most of the patches were incorporated)
Dropped patches:
	http://marc.info/?l=linux-omap&m=130570864519114&w=2
		- Agree that we should wait for regulators
Pending:
	http://marc.info/?l=linux-omap&m=130569587107997&w=2
		- I have a bunch of scope pictures and xls
		 from which this data was created, so no equation yet available.
Updated patches:
	http://marc.info/?l=linux-omap&m=130569587007994&w=2 - review comments

v1: http://marc.info/?l=linux-omap&m=130569586007964&w=2

Nishanth Menon (3):
  OMAP4: PM: VC: fix channel bit offset for MPU
  OMAP4: PM: VC: introduce concept of default channel
  OMAP4: PM: VC: make omap_vc_i2c_init static

 arch/arm/mach-omap2/vc.c          |   82 ++++++++++++++++++++++---------------
 arch/arm/mach-omap2/vc.h          |   37 +++++++++++++++-
 arch/arm/mach-omap2/vc3xxx_data.c |   11 +++++
 arch/arm/mach-omap2/vc44xx_data.c |   22 ++++++++++
 arch/arm/mach-omap2/voltage.h     |    9 ++++
 5 files changed, 125 insertions(+), 36 deletions(-)

Regards,
	Nishanth Menon

[pm_wip/voltdm_nm][PATCH v2 1/3] OMAP4: PM: VC: fix channel bit offset for MPU

[Nishanth Menon]

Patch "OMAP3+: VC: abstract out channel configuration" abstracts out
VC channel configuration. However, TRM has it's little surprises such
as the following for channel_cfg:
CFG_CHANNEL_SA    BIT(0)
CFG_CHANNEL_RAV   BIT(1)
CFG_CHANNEL_RAC   BIT(2)
CFG_CHANNEL_RACEN BIT(3)
CFG_CHANNEL_CMD   BIT(4)
is valid for core and iva, *but* for mpu, the channel offsets are as follows:
CFG_CHANNEL_SA    BIT(0)
CFG_CHANNEL_CMD   BIT(1)
CFG_CHANNEL_RAV   BIT(2)
CFG_CHANNEL_RAC   BIT(3)
CFG_CHANNEL_RACEN BIT(4)

To handle this on the fly, add a structure to describe this and use the
structure for vc definition.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/vc.c          |   30 ++++++++++++++----------------
 arch/arm/mach-omap2/vc.h          |   29 +++++++++++++++++++++++++++++
 arch/arm/mach-omap2/vc3xxx_data.c |   11 +++++++++++
 arch/arm/mach-omap2/vc44xx_data.c |   21 +++++++++++++++++++++
 4 files changed, 75 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-omap2/vc.c b/arch/arm/mach-omap2/vc.c
index fba352d..6437460 100644
--- a/arch/arm/mach-omap2/vc.c
+++ b/arch/arm/mach-omap2/vc.c
@@ -10,18 +10,6 @@
 #include "prm-regbits-44xx.h"
 #include "prm44xx.h"
 
-/*
- * Channel configuration bits, common for OMAP3 & 4
- * OMAP3 register: PRM_VC_CH_CONF
- * OMAP4 register: PRM_VC_CFG_CHANNEL
- */
-#define CFG_CHANNEL_SA    BIT(0)
-#define CFG_CHANNEL_RAV   BIT(1)
-#define CFG_CHANNEL_RAC   BIT(2)
-#define CFG_CHANNEL_RACEN BIT(3)
-#define CFG_CHANNEL_CMD   BIT(4)
-#define CFG_CHANNEL_MASK 0x3f
-
 /**
  * omap_vc_config_channel - configure VC channel to PMIC mappings
  * @voltdm: pointer to voltagdomain defining the desired VC channel
@@ -249,6 +237,7 @@ void __init omap_vc_init_channel(struct voltagedomain *voltdm)
 {
 	struct omap_vc_channel *vc = voltdm->vc;
 	u8 on_vsel, onlp_vsel, ret_vsel, off_vsel;
+	struct omap_vc_channel_cfg *cfg_channel_data;
 	u32 val;
 
 	if (!voltdm->pmic || !voltdm->pmic->uv_to_vsel) {
@@ -264,6 +253,14 @@ void __init omap_vc_init_channel(struct voltagedomain *voltdm)
 		return;
 	}
 
+	/* Ensure we have VC channel data */
+	if (!vc->cfg_ch_bits) {
+		pr_err("%s: No CFG Channel data for vdd_%s regs\n",
+			__func__, voltdm->name);
+		return;
+	}
+	cfg_channel_data = vc->cfg_ch_bits;
+
 	vc->cfg_channel = 0;
 
 	/* get PMIC/board specific settings */
@@ -276,7 +273,7 @@ void __init omap_vc_init_channel(struct voltagedomain *voltdm)
 	voltdm->rmw(vc->smps_sa_mask,
 		    vc->i2c_slave_addr << __ffs(vc->smps_sa_mask),
 		    vc->common->smps_sa_reg);
-	vc->cfg_channel |= CFG_CHANNEL_SA;
+	vc->cfg_channel |= cfg_channel_data->sa;
 
 	/*
 	 * Configure the PMIC register addresses.
@@ -284,13 +281,14 @@ void __init omap_vc_init_channel(struct voltagedomain *voltdm)
 	voltdm->rmw(vc->smps_volra_mask,
 		    vc->volt_reg_addr << __ffs(vc->smps_volra_mask),
 		    vc->common->smps_volra_reg);
-	vc->cfg_channel |= CFG_CHANNEL_RAV;
+	vc->cfg_channel |= cfg_channel_data->rav;
 
 	if (vc->cmd_reg_addr) {
 		voltdm->rmw(vc->smps_cmdra_mask,
 			    vc->cmd_reg_addr << __ffs(vc->smps_cmdra_mask),
 			    vc->common->smps_cmdra_reg);
-		vc->cfg_channel |= CFG_CHANNEL_RAC | CFG_CHANNEL_RACEN;
+		vc->cfg_channel |= cfg_channel_data->rac |
+					cfg_channel_data->racen;
 	}
 
 	/* Set up the on, inactive, retention and off voltage */
@@ -303,7 +301,7 @@ void __init omap_vc_init_channel(struct voltagedomain *voltdm)
 	       (ret_vsel << vc->common->cmd_ret_shift) |
 	       (off_vsel << vc->common->cmd_off_shift));
 	voltdm->write(val, vc->cmdval_reg);
-	vc->cfg_channel |= CFG_CHANNEL_CMD;
+	vc->cfg_channel |= cfg_channel_data->cmd;
 
 	/* Channel configuration */
 	omap_vc_config_channel(voltdm);
diff --git a/arch/arm/mach-omap2/vc.h b/arch/arm/mach-omap2/vc.h
index f0fb61f..2538089 100644
--- a/arch/arm/mach-omap2/vc.h
+++ b/arch/arm/mach-omap2/vc.h
@@ -62,11 +62,39 @@ struct omap_vc_common {
 	u8 i2c_mcode_mask;
 };
 
+/*
+ * Channel configuration bits, common for OMAP3 & 4
+ * OMAP3 register: PRM_VC_CH_CONF
+ * OMAP4 register: PRM_VC_CFG_CHANNEL
+ */
+#define CFG_CHANNEL_SA    BIT(0)
+#define CFG_CHANNEL_RAV   BIT(1)
+#define CFG_CHANNEL_RAC   BIT(2)
+#define CFG_CHANNEL_RACEN BIT(3)
+#define CFG_CHANNEL_CMD   BIT(4)
+#define CFG_CHANNEL_MASK 0x3f
+/**
+ * struct omap_vc_channel_cfg - describe the cfg_channel bitfield
+ * @sa:	SA_VDD_xxx_L
+ * @rav:	RAV_VDD_xxx_L
+ * @rac:	RAC_VDD_xxx_L
+ * @racen:	RACEN_VDD_xxx_L
+ * @cmd:	CMD_VDD_xxx_L
+ */
+struct omap_vc_channel_cfg {
+	u8 sa;
+	u8 rav;
+	u8 rac;
+	u8 racen;
+	u8 cmd;
+};
+
 /**
  * struct omap_vc_channel - VC per-instance data
  * @common: pointer to VC common data for this platform
  * @smps_sa_mask: i2c slave address bitmask in the PRM_VC_SMPS_SA register
  * @smps_volra_mask: VOLRA* bitmask in the PRM_VC_VOL_RA register
+ * @cfg_ch_bits: exception handling for unordered register bits in cfg_channel
  */
 struct omap_vc_channel {
 	/* channel state */
@@ -74,6 +102,7 @@ struct omap_vc_channel {
 	u8 volt_reg_addr;
 	u8 cmd_reg_addr;
 	u8 cfg_channel;
+	struct omap_vc_channel_cfg *cfg_ch_bits;
 	u16 setup_time;
 	bool i2c_high_speed;
 
diff --git a/arch/arm/mach-omap2/vc3xxx_data.c b/arch/arm/mach-omap2/vc3xxx_data.c
index 95d7701..0a22b1d 100644
--- a/arch/arm/mach-omap2/vc3xxx_data.c
+++ b/arch/arm/mach-omap2/vc3xxx_data.c
@@ -49,6 +49,15 @@ static struct omap_vc_common omap3_vc_common = {
 	.i2c_mcode_mask	 = OMAP3430_MCODE_MASK,
 };
 
+/* for all channels */
+struct omap_vc_channel_cfg vc3xxx_common_cfg_channel = {
+	.sa = CFG_CHANNEL_SA,
+	.rav = CFG_CHANNEL_RAV,
+	.rac = CFG_CHANNEL_RAC,
+	.racen = CFG_CHANNEL_RACEN,
+	.cmd = CFG_CHANNEL_CMD,
+};
+
 struct omap_vc_channel omap3_vc_mpu = {
 	.common = &omap3_vc_common,
 	.cmdval_reg = OMAP3_PRM_VC_CMD_VAL_0_OFFSET,
@@ -56,6 +65,7 @@ struct omap_vc_channel omap3_vc_mpu = {
 	.smps_volra_mask = OMAP3430_VOLRA0_MASK,
 	.smps_cmdra_mask = OMAP3430_CMDRA0_MASK,
 	.cfg_channel_sa_shift = OMAP3430_PRM_VC_SMPS_SA_SA0_SHIFT,
+	.cfg_ch_bits = &vc3xxx_common_cfg_channel,
 };
 
 struct omap_vc_channel omap3_vc_core = {
@@ -65,4 +75,5 @@ struct omap_vc_channel omap3_vc_core = {
 	.smps_volra_mask = OMAP3430_VOLRA1_MASK,
 	.smps_cmdra_mask = OMAP3430_CMDRA1_MASK,
 	.cfg_channel_sa_shift = OMAP3430_PRM_VC_SMPS_SA_SA1_SHIFT,
+	.cfg_ch_bits = &vc3xxx_common_cfg_channel,
 };
diff --git a/arch/arm/mach-omap2/vc44xx_data.c b/arch/arm/mach-omap2/vc44xx_data.c
index fe4f4e5..3d32ab0 100644
--- a/arch/arm/mach-omap2/vc44xx_data.c
+++ b/arch/arm/mach-omap2/vc44xx_data.c
@@ -50,6 +50,24 @@ static const struct omap_vc_common omap4_vc_common = {
 	.i2c_mcode_mask	 = OMAP4430_HSMCODE_MASK,
 };
 
+/* Handle exception case for vc44xx MPU */
+static struct omap_vc_channel_cfg vc44xx_mpu_cfg_channel = {
+	.sa = CFG_CHANNEL_SA,
+	.cmd = BIT(1),
+	.rav = BIT(2),
+	.rac = BIT(3),
+	.racen = BIT(4),
+};
+
+/* for all other */
+struct omap_vc_channel_cfg vc44xx_common_cfg_channel = {
+	.sa = CFG_CHANNEL_SA,
+	.rav = CFG_CHANNEL_RAV,
+	.rac = CFG_CHANNEL_RAC,
+	.racen = CFG_CHANNEL_RACEN,
+	.cmd = CFG_CHANNEL_CMD,
+};
+
 /* VC instance data for each controllable voltage line */
 struct omap_vc_channel omap4_vc_mpu = {
 	.common = &omap4_vc_common,
@@ -58,6 +76,7 @@ struct omap_vc_channel omap4_vc_mpu = {
 	.smps_volra_mask = OMAP4430_VOLRA_VDD_MPU_L_MASK,
 	.smps_cmdra_mask = OMAP4430_CMDRA_VDD_MPU_L_MASK,
 	.cfg_channel_sa_shift = OMAP4430_SA_VDD_MPU_L_SHIFT,
+	.cfg_ch_bits = &vc44xx_mpu_cfg_channel,
 };
 
 struct omap_vc_channel omap4_vc_iva = {
@@ -67,6 +86,7 @@ struct omap_vc_channel omap4_vc_iva = {
 	.smps_volra_mask = OMAP4430_VOLRA_VDD_IVA_L_MASK,
 	.smps_cmdra_mask = OMAP4430_CMDRA_VDD_IVA_L_MASK,
 	.cfg_channel_sa_shift = OMAP4430_SA_VDD_IVA_L_SHIFT,
+	.cfg_ch_bits = &vc44xx_common_cfg_channel,
 };
 
 struct omap_vc_channel omap4_vc_core = {
@@ -76,5 +96,6 @@ struct omap_vc_channel omap4_vc_core = {
 	.smps_volra_mask = OMAP4430_VOLRA_VDD_CORE_L_MASK,
 	.smps_cmdra_mask = OMAP4430_CMDRA_VDD_CORE_L_MASK,
 	.cfg_channel_sa_shift = OMAP4430_SA_VDD_CORE_L_SHIFT,
+	.cfg_ch_bits = &vc44xx_common_cfg_channel,
 };
 
-- 
1.7.1

Re: [pm_wip/voltdm_nm][PATCH v2 1/3] OMAP4: PM: VC: fix channel bit offset for MPU

[Kevin Hilman]

Nishanth Menon <nm@ti.com> writes:

> Patch "OMAP3+: VC: abstract out channel configuration" abstracts out
> VC channel configuration. However, TRM has it's little surprises such
> as the following for channel_cfg:
> CFG_CHANNEL_SA    BIT(0)
> CFG_CHANNEL_RAV   BIT(1)
> CFG_CHANNEL_RAC   BIT(2)
> CFG_CHANNEL_RACEN BIT(3)
> CFG_CHANNEL_CMD   BIT(4)
> is valid for core and iva, *but* for mpu, the channel offsets are as follows:
> CFG_CHANNEL_SA    BIT(0)
> CFG_CHANNEL_CMD   BIT(1)
> CFG_CHANNEL_RAV   BIT(2)
> CFG_CHANNEL_RAC   BIT(3)
> CFG_CHANNEL_RACEN BIT(4)
>
> To handle this on the fly, add a structure to describe this and use the
> structure for vc definition.
>
> Signed-off-by: Nishanth Menon <nm@ti.com>

You're going to be irritated with me (again), but I changed my mind on
this one.

Initially I thought that this was intentional and that OMAP5+ was going
to have the bit fields in yet a different way, so I wanted to have this
very flexible at a per-channel level.  It turns out that this one
channel really is a mutant.  Somebody fat-fingered this in Bitwise and
resulted in a single anomaly. 

Now that I know this is a single mutant case, I prefer handling it as an
exception (kinda like you did in your original version.)

The RFC/patch below (currently at the tip of voltdm_nm, but only compile
tested) is my version of solving the problem to handle it as an
exception case.   What do you think?

Kevin


>From 6df1f45d4b2a732083992e47596c67e6bdd0311f Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@ti.com>
Date: Thu, 2 Jun 2011 17:28:13 -0700
Subject: [RFC/PATCH] OMAP3+: PM: VC: handle mutant channel config for OMAP4 MPU channel

On OMAP3+, all VC channels have the the same bitfield ordering for all
VC channels, except the OMAP4 MPU channel.  This appears to be a freak
accident as all other VC channel (including OMAP5) have the standard
configuration.  Handle the mutant case by adding a per-channel flag
to signal the deformity and handle it during VC init.

Special thanks to Nishanth Menon <nm@ti.com> for finding this problem
and for proposing the initial solution.

Cc: Nishanth Menon <nm@ti.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 arch/arm/mach-omap2/vc.c          |   62 +++++++++++++++++++++++++++++--------
 arch/arm/mach-omap2/vc.h          |    1 +
 arch/arm/mach-omap2/vc44xx_data.c |    2 +-
 3 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/vc.c b/arch/arm/mach-omap2/vc.c
index d027864..c4edc62 100644
--- a/arch/arm/mach-omap2/vc.c
+++ b/arch/arm/mach-omap2/vc.c
@@ -10,17 +10,51 @@
 #include "prm-regbits-44xx.h"
 #include "prm44xx.h"
 
-/*
- * Channel configuration bits, common for OMAP3 & 4
+/**
+ * struct omap_vc_channel_cfg - describe the cfg_channel bitfield
+ * @sa: bit for slave address 
+ * @rav: bit for voltage configuration register
+ * @rac: bit for command configuration register	
+ * @racen: enable bit for RAC
+ * @cmd: bit for command value set selection
+ *
+ * Channel configuration bits, common for OMAP3+
  * OMAP3 register: PRM_VC_CH_CONF
  * OMAP4 register: PRM_VC_CFG_CHANNEL
+ * OMAP5 register: PRM_VC_SMPS_<voltdm>_CONFIG
  */
-#define CFG_CHANNEL_SA    BIT(0)
-#define CFG_CHANNEL_RAV   BIT(1)
-#define CFG_CHANNEL_RAC   BIT(2)
-#define CFG_CHANNEL_RACEN BIT(3)
-#define CFG_CHANNEL_CMD   BIT(4)
-#define CFG_CHANNEL_MASK 0x3f
+struct omap_vc_channel_cfg {
+	u8 sa;
+	u8 rav;
+	u8 rac;
+	u8 racen;
+	u8 cmd;
+};
+
+static struct omap_vc_channel_cfg vc_default_channel_cfg = {
+	.sa    = BIT(0),
+	.rav   = BIT(1),
+	.rac   = BIT(2),
+	.racen = BIT(3),
+	.cmd   = BIT(4),
+};
+
+/*
+ * On OMAP3+, all VC channels have the above default bitfield
+ * configuration, except the OMAP4 MPU channel.  This appears
+ * to be a freak accident as every other VC channel has the
+ * default configuration, thus creating a mutant channel config.
+ */
+static struct omap_vc_channel_cfg vc_mutant_channel_cfg = {
+	.sa    = BIT(0),
+	.rav   = BIT(2),
+	.rac   = BIT(3),
+	.racen = BIT(4),
+	.cmd   = BIT(1),
+};
+
+static struct omap_vc_channel_cfg *vc_cfg_bits = &vc_default_channel_cfg;
+#define CFG_CHANNEL_MASK 0x1f
 
 /**
  * omap_vc_config_channel - configure VC channel to PMIC mappings
@@ -47,7 +81,7 @@ static int omap_vc_config_channel(struct voltagedomain *voltdm)
 	 * All others must stay at zero (see function comment above.)
 	 */
 	if (vc->flags & OMAP_VC_CHANNEL_DEFAULT)
-		vc->cfg_channel &= CFG_CHANNEL_RACEN;
+		vc->cfg_channel &= vc_cfg_bits->racen;
 
 	voltdm->rmw(CFG_CHANNEL_MASK << vc->cfg_channel_sa_shift,
 		    vc->cfg_channel << vc->cfg_channel_sa_shift,
@@ -264,6 +298,8 @@ void __init omap_vc_init_channel(struct voltagedomain *voltdm)
 	}
 
 	vc->cfg_channel = 0;
+	if (vc->flags & OMAP_VC_CHANNEL_CFG_MUTANT)
+		vc_cfg_bits = vc_mutant_channel_cfg;
 
 	/* get PMIC/board specific settings */
 	vc->i2c_slave_addr = voltdm->pmic->i2c_slave_addr;
@@ -275,7 +311,7 @@ void __init omap_vc_init_channel(struct voltagedomain *voltdm)
 	voltdm->rmw(vc->smps_sa_mask,
 		    vc->i2c_slave_addr << __ffs(vc->smps_sa_mask),
 		    vc->common->smps_sa_reg);
-	vc->cfg_channel |= CFG_CHANNEL_SA;
+	vc->cfg_channel |= vc_cfg_bits->sa;
 
 	/*
 	 * Configure the PMIC register addresses.
@@ -283,13 +319,13 @@ void __init omap_vc_init_channel(struct voltagedomain *voltdm)
 	voltdm->rmw(vc->smps_volra_mask,
 		    vc->volt_reg_addr << __ffs(vc->smps_volra_mask),
 		    vc->common->smps_volra_reg);
-	vc->cfg_channel |= CFG_CHANNEL_RAV;
+	vc->cfg_channel |= vc_cfg_bits->rav;
 
 	if (vc->cmd_reg_addr) {
 		voltdm->rmw(vc->smps_cmdra_mask,
 			    vc->cmd_reg_addr << __ffs(vc->smps_cmdra_mask),
 			    vc->common->smps_cmdra_reg);
-		vc->cfg_channel |= CFG_CHANNEL_RAC | CFG_CHANNEL_RACEN;
+		vc->cfg_channel |= vc_cfg_bits->rac | vc_cfg_bits->racen;
 	}
 
 	/* Set up the on, inactive, retention and off voltage */
@@ -302,7 +338,7 @@ void __init omap_vc_init_channel(struct voltagedomain *voltdm)
 	       (ret_vsel << vc->common->cmd_ret_shift) |
 	       (off_vsel << vc->common->cmd_off_shift));
 	voltdm->write(val, vc->cmdval_reg);
-	vc->cfg_channel |= CFG_CHANNEL_CMD;
+	vc->cfg_channel |= vc_cfg_bits->cmd;
 
 	/* Channel configuration */
 	omap_vc_config_channel(voltdm);
diff --git a/arch/arm/mach-omap2/vc.h b/arch/arm/mach-omap2/vc.h
index e7d410c..e16dacf 100644
--- a/arch/arm/mach-omap2/vc.h
+++ b/arch/arm/mach-omap2/vc.h
@@ -64,6 +64,7 @@ struct omap_vc_common {
 
 /* omap_vc_channel.flags values */
 #define OMAP_VC_CHANNEL_DEFAULT BIT(0)
+#define OMAP_VC_CHANNEL_CFG_MUTANT BIT(1)
 
 /**
  * struct omap_vc_channel - VC per-instance data
diff --git a/arch/arm/mach-omap2/vc44xx_data.c b/arch/arm/mach-omap2/vc44xx_data.c
index 148be18..0a4fc37 100644
--- a/arch/arm/mach-omap2/vc44xx_data.c
+++ b/arch/arm/mach-omap2/vc44xx_data.c
@@ -52,7 +52,7 @@ static const struct omap_vc_common omap4_vc_common = {
 
 /* VC instance data for each controllable voltage line */
 struct omap_vc_channel omap4_vc_mpu = {
-	.flags = OMAP_VC_CHANNEL_DEFAULT,
+	.flags = OMAP_VC_CHANNEL_DEFAULT | OMAP_VC_CHANNEL_CFG_MUTANT,
 	.common = &omap4_vc_common,
 	.cmdval_reg = OMAP4_PRM_VC_VAL_CMD_VDD_MPU_L_OFFSET,
 	.smps_sa_mask = OMAP4430_SA_VDD_MPU_L_PRM_VC_SMPS_SA_MASK,
-- 
1.7.4

[pm_wip/voltdm_nm][PATCH v2 2/3] OMAP4: PM: VC: introduce concept of default channel

[Nishanth Menon]

Due to a TRM bug, the current code assumes that channel0(core) is the default
channel. With the additional explanation provided by the hardware team, it is
clear that PRM_VC_CFG_CHANNEL register allows for flexibility of configuring
for various PMIC configurations. For example, the I2C slave address on TWL6030
when using 4430 configuration could potentially use the same slave address for
all domains, while in 4460 configuration, we drive MPU using TPS; Core and IVA
using TWL6030. To allow this flexibility, we state in existing parameters using
a flag to indicate usage of default channel.

In addition, the TRM update clears up the confusion on the fact that MPU is
infact the default channel on OMAP4.

We update the same here.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/vc.c          |   58 ++++++++++++++++++++++++-------------
 arch/arm/mach-omap2/vc.h          |    8 +++--
 arch/arm/mach-omap2/vc44xx_data.c |    1 +
 arch/arm/mach-omap2/voltage.h     |    9 ++++++
 4 files changed, 53 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-omap2/vc.c b/arch/arm/mach-omap2/vc.c
index 6437460..42c77a8 100644
--- a/arch/arm/mach-omap2/vc.c
+++ b/arch/arm/mach-omap2/vc.c
@@ -22,20 +22,20 @@
  * - command values for ON, ONLP, RET and OFF (CMD)
  *
  * This function currently only allows flexible configuration of
- * the non-default channel (e.g. non-zero channels.)  Starting with
- * OMAP4, only the non-zero channels can be configured.  Channel zero
- * always uses the channel zero register values.  Therefore, the
- * same limitation is imposed on OMAP3 for consistency.
+ * the non-default channels.  Introduced in OMAP4, is a concept of a default
+ * channel - in OMAP4, this channel is MPU, all other domains could optionally
+ * link their i2c regs to use MPU channel's configuration if required.
  */
 static int omap_vc_config_channel(struct voltagedomain *voltdm)
 {
 	struct omap_vc_channel *vc = voltdm->vc;
 
 	/*
-	 * For channel zero, the only configurable bit is RACEN.
-	 * All others must stay at zero (see function comment above.)
+	 * For default channel, the only configurable bit is RACEN.
+	 * All others default channel bits stays at zero
+	 * (see function comment above.)
 	 */
-	if (!vc->cfg_channel_sa_shift)
+	if (vc->is_default_channel)
 		vc->cfg_channel &= CFG_CHANNEL_RACEN;
 
 	voltdm->rmw(CFG_CHANNEL_MASK << vc->cfg_channel_sa_shift,
@@ -261,6 +261,18 @@ void __init omap_vc_init_channel(struct voltagedomain *voltdm)
 	}
 	cfg_channel_data = vc->cfg_ch_bits;
 
+	/* Dont proceed if channel configurations are messed up */
+	if (vc->is_default_channel &&
+		(vc->i2c_slave_addr == USE_DEFAULT_CHANNEL_I2C_PARAM ||
+		 vc->cmd_reg_addr == USE_DEFAULT_CHANNEL_I2C_PARAM ||
+		 vc->volt_reg_addr == USE_DEFAULT_CHANNEL_I2C_PARAM)) {
+		pr_err("%s: Voltage Domain %s is default.INVALID CONFIGURATION:"
+			"slave_add = %04x cmd=%04x vol=%04x!\n",
+			__func__, voltdm->name, vc->i2c_slave_addr,
+			vc->cmd_reg_addr, vc->volt_reg_addr);
+		return;
+	}
+
 	vc->cfg_channel = 0;
 
 	/* get PMIC/board specific settings */
@@ -269,26 +281,32 @@ void __init omap_vc_init_channel(struct voltagedomain *voltdm)
 	vc->cmd_reg_addr = voltdm->pmic->cmd_reg_addr;
 	vc->setup_time = voltdm->pmic->volt_setup_time;
 
-	/* Configure the i2c slave address for this VC */
-	voltdm->rmw(vc->smps_sa_mask,
-		    vc->i2c_slave_addr << __ffs(vc->smps_sa_mask),
-		    vc->common->smps_sa_reg);
-	vc->cfg_channel |= cfg_channel_data->sa;
+	/* Configure the I2C slave address for this VC */
+	if (vc->i2c_slave_addr != USE_DEFAULT_CHANNEL_I2C_PARAM) {
+		voltdm->rmw(vc->smps_sa_mask,
+			    vc->i2c_slave_addr << __ffs(vc->smps_sa_mask),
+			    vc->common->smps_sa_reg);
+		vc->cfg_channel |= cfg_channel_data->sa;
+	}
 
 	/*
 	 * Configure the PMIC register addresses.
 	 */
-	voltdm->rmw(vc->smps_volra_mask,
-		    vc->volt_reg_addr << __ffs(vc->smps_volra_mask),
-		    vc->common->smps_volra_reg);
-	vc->cfg_channel |= cfg_channel_data->rav;
-
-	if (vc->cmd_reg_addr) {
+	if (vc->cmd_reg_addr != USE_DEFAULT_CHANNEL_I2C_PARAM) {
 		voltdm->rmw(vc->smps_cmdra_mask,
 			    vc->cmd_reg_addr << __ffs(vc->smps_cmdra_mask),
 			    vc->common->smps_cmdra_reg);
-		vc->cfg_channel |= cfg_channel_data->rac |
-					cfg_channel_data->racen;
+		vc->cfg_channel |= cfg_channel_data->rac;
+	}
+
+	/* if voltage and cmd regs are same, we can use cmdra register */
+	if (vc->volt_reg_addr == vc->cmd_reg_addr) {
+		vc->cfg_channel |= cfg_channel_data->racen;
+	} else if (vc->volt_reg_addr != USE_DEFAULT_CHANNEL_I2C_PARAM) {
+		voltdm->rmw(vc->smps_volra_mask,
+			    vc->volt_reg_addr << __ffs(vc->smps_volra_mask),
+			    vc->common->smps_volra_reg);
+		vc->cfg_channel |= cfg_channel_data->rav;
 	}
 
 	/* Set up the on, inactive, retention and off voltage */
diff --git a/arch/arm/mach-omap2/vc.h b/arch/arm/mach-omap2/vc.h
index 2538089..96739a2 100644
--- a/arch/arm/mach-omap2/vc.h
+++ b/arch/arm/mach-omap2/vc.h
@@ -92,15 +92,16 @@ struct omap_vc_channel_cfg {
 /**
  * struct omap_vc_channel - VC per-instance data
  * @common: pointer to VC common data for this platform
+ * @is_master_channel: if the channel is the master channel
  * @smps_sa_mask: i2c slave address bitmask in the PRM_VC_SMPS_SA register
  * @smps_volra_mask: VOLRA* bitmask in the PRM_VC_VOL_RA register
  * @cfg_ch_bits: exception handling for unordered register bits in cfg_channel
  */
 struct omap_vc_channel {
 	/* channel state */
-	u8 i2c_slave_addr;
-	u8 volt_reg_addr;
-	u8 cmd_reg_addr;
+	u16 i2c_slave_addr;
+	u16 volt_reg_addr;
+	u16 cmd_reg_addr;
 	u8 cfg_channel;
 	struct omap_vc_channel_cfg *cfg_ch_bits;
 	u16 setup_time;
@@ -108,6 +109,7 @@ struct omap_vc_channel {
 
 	/* register access data */
 	const struct omap_vc_common *common;
+	bool is_default_channel;
 	u32 smps_sa_mask;
 	u32 smps_volra_mask;
 	u32 smps_cmdra_mask;
diff --git a/arch/arm/mach-omap2/vc44xx_data.c b/arch/arm/mach-omap2/vc44xx_data.c
index 3d32ab0..817e87d 100644
--- a/arch/arm/mach-omap2/vc44xx_data.c
+++ b/arch/arm/mach-omap2/vc44xx_data.c
@@ -71,6 +71,7 @@ struct omap_vc_channel_cfg vc44xx_common_cfg_channel = {
 /* VC instance data for each controllable voltage line */
 struct omap_vc_channel omap4_vc_mpu = {
 	.common = &omap4_vc_common,
+	.is_default_channel = true,
 	.cmdval_reg = OMAP4_PRM_VC_VAL_CMD_VDD_MPU_L_OFFSET,
 	.smps_sa_mask = OMAP4430_SA_VDD_MPU_L_PRM_VC_SMPS_SA_MASK,
 	.smps_volra_mask = OMAP4430_VOLRA_VDD_MPU_L_MASK,
diff --git a/arch/arm/mach-omap2/voltage.h b/arch/arm/mach-omap2/voltage.h
index f079167..02f8969 100644
--- a/arch/arm/mach-omap2/voltage.h
+++ b/arch/arm/mach-omap2/voltage.h
@@ -109,6 +109,15 @@ struct omap_volt_data {
 	u8	vp_errgain;
 };
 
+/*
+ * Introduced in OMAP4, is a concept of a default channe - in OMAP4, this
+ * channel is MPU, all other domains such as IVA/CORE, could optionally
+ * link their i2c reg configuration to use MPU channel's configuration if
+ * required. To do this, mark in the PMIC structure's
+ * i2c_slave_addr/volt_reg_addr/cmd_reg_addr with this macro.
+ */
+#define USE_DEFAULT_CHANNEL_I2C_PARAM	0x10000
+
 /**
  * struct omap_voltdm_pmic - PMIC specific data required by voltage driver.
  * @slew_rate:	PMIC slew rate (in uv/us)
-- 
1.7.1

Re: [pm_wip/voltdm_nm][PATCH v2 2/3] OMAP4: PM: VC: introduce concept of default channel

[Kevin Hilman]

Nishanth Menon <nm@ti.com> writes:

> Due to a TRM bug, the current code assumes that channel0(core) is the default
> channel. With the additional explanation provided by the hardware team, it is
> clear that PRM_VC_CFG_CHANNEL register allows for flexibility of configuring
> for various PMIC configurations. For example, the I2C slave address on TWL6030
> when using 4430 configuration could potentially use the same slave address for
> all domains, while in 4460 configuration, we drive MPU using TPS; Core and IVA
> using TWL6030. To allow this flexibility, we state in existing parameters using
> a flag to indicate usage of default channel.
>
> In addition, the TRM update clears up the confusion on the fact that MPU is
> infact the default channel on OMAP4.
>
> We update the same here.
>
> Signed-off-by: Nishanth Menon <nm@ti.com>

There's too much going on in this patch not described in the changelog
(extra error checking, volt & cmd reg checking etc.)

Also, I don't like the USE_DEFAULT_CHANNEL_I2C_PARAM, mainly because it
adds clutter without any obvious value.  To me, zero is a perfectly good
value to signify "use default channel value", especially since that's
the value used by the hardware.

Incidentally, adding this doesn't actually change current behavior.
Currently, I use zero (an unconfigured value in the VC settings) to
signify it should use default settings.  In your patch, you defined
USE_DEFAULT_CHANNEL_I2C_PARAM as 0x10000 (17 bits) but assign it to 16
bit fields, which means they are just set to zero. :)

So rather than take this patch, I'm just going to fold the following
diff into "OMAP3+: VC: abstract out channel configuration" in voltdm_b.
I'll also update the changelog noting that the TRM is wrong here in that
it describes CORE as the default channel when it's in fact MPU.

Kevin

diff --git a/arch/arm/mach-omap2/vc.c b/arch/arm/mach-omap2/vc.c
index fba352d..fa48944 100644
--- a/arch/arm/mach-omap2/vc.c
+++ b/arch/arm/mach-omap2/vc.c
@@ -33,21 +33,20 @@
  * - command configuration address (RAC) and enable bit (RACEN)
  * - command values for ON, ONLP, RET and OFF (CMD)
  *
- * This function currently only allows flexible configuration of
- * the non-default channel (e.g. non-zero channels.)  Starting with
- * OMAP4, only the non-zero channels can be configured.  Channel zero
- * always uses the channel zero register values.  Therefore, the
- * same limitation is imposed on OMAP3 for consistency.
+ * This function currently only allows flexible configuration of the
+ * non-default channel.  Starting with OMAP4, there are more than 2
+ * channels, with one defined as the default (on OMAP4, it's MPU.)
+ * Only the non-default channel can be configured.
  */
 static int omap_vc_config_channel(struct voltagedomain *voltdm)
 {
 	struct omap_vc_channel *vc = voltdm->vc;
 
 	/*
-	 * For channel zero, the only configurable bit is RACEN.
+	 * For default channel, the only configurable bit is RACEN.
 	 * All others must stay at zero (see function comment above.)
 	 */
-	if (!vc->cfg_channel_sa_shift)
+	if (vc->is_default_channel)
 		vc->cfg_channel &= CFG_CHANNEL_RACEN;
 
 	voltdm->rmw(CFG_CHANNEL_MASK << vc->cfg_channel_sa_shift,
diff --git a/arch/arm/mach-omap2/vc.h b/arch/arm/mach-omap2/vc.h
index f0fb61f..c216b57 100644
--- a/arch/arm/mach-omap2/vc.h
+++ b/arch/arm/mach-omap2/vc.h
@@ -84,6 +84,8 @@ struct omap_vc_channel {
 	u32 smps_cmdra_mask;
 	u8 cmdval_reg;
 	u8 cfg_channel_sa_shift;
+
+	bool is_default_channel;
 };
 
 extern struct omap_vc_channel omap3_vc_mpu;
diff --git a/arch/arm/mach-omap2/vc44xx_data.c b/arch/arm/mach-omap2/vc44xx_data.c
index fe4f4e5..5844b20 100644
--- a/arch/arm/mach-omap2/vc44xx_data.c
+++ b/arch/arm/mach-omap2/vc44xx_data.c
@@ -58,6 +58,7 @@ struct omap_vc_channel omap4_vc_mpu = {
 	.smps_volra_mask = OMAP4430_VOLRA_VDD_MPU_L_MASK,
 	.smps_cmdra_mask = OMAP4430_CMDRA_VDD_MPU_L_MASK,
 	.cfg_channel_sa_shift = OMAP4430_SA_VDD_MPU_L_SHIFT,
+	.is_default_channel = true,
 };
 
 struct omap_vc_channel omap4_vc_iva = {

Re: [pm_wip/voltdm_nm][PATCH v2 2/3] OMAP4: PM: VC: introduce concept of default channel

[Menon, Nishanth]

On Thu, Jun 2, 2011 at 18:45, Kevin Hilman <khilman@ti.com> wrote:
> Nishanth Menon <nm@ti.com> writes:
>
>> Due to a TRM bug, the current code assumes that channel0(core) is the default
>> channel. With the additional explanation provided by the hardware team, it is
>> clear that PRM_VC_CFG_CHANNEL register allows for flexibility of configuring
>> for various PMIC configurations. For example, the I2C slave address on TWL6030
>> when using 4430 configuration could potentially use the same slave address for
>> all domains, while in 4460 configuration, we drive MPU using TPS; Core and IVA
>> using TWL6030. To allow this flexibility, we state in existing parameters using
>> a flag to indicate usage of default channel.
>>
>> In addition, the TRM update clears up the confusion on the fact that MPU is
>> infact the default channel on OMAP4.
>>
>> We update the same here.
>>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>
> There's too much going on in this patch not described in the changelog
> (extra error checking, volt & cmd reg checking etc.)
>
> Also, I don't like the USE_DEFAULT_CHANNEL_I2C_PARAM, mainly because it
> adds clutter without any obvious value.  To me, zero is a perfectly good
> value to signify "use default channel value", especially since that's
> the value used by the hardware.
The reason is that 0 is a valid i2c register address. 8 bits is used
in VC and I wanted one bit which was further away, hence the 16 bit.
The usage could be that in .volt_reg_addr or in .cmd_reg_addr I could
set this up with the macro. and bingo, we use the default domain's
configuration.

This is esp useful if we had a single pmic reg for all domains.. I
mean the VC design allows for it, even though we dont use it
currently.
>
> Incidentally, adding this doesn't actually change current behavior.
> Currently, I use zero (an unconfigured value in the VC settings) to
> signify it should use default settings.  In your patch, you defined
> USE_DEFAULT_CHANNEL_I2C_PARAM as 0x10000 (17 bits) but assign it to 16
> bit fields, which means they are just set to zero. :)


> So rather than take this patch, I'm just going to fold the following
> diff into "OMAP3+: VC: abstract out channel configuration" in voltdm_b.
> I'll also update the changelog noting that the TRM is wrong here in that
> it describes CORE as the default channel when it's in fact MPU.

I intend to post a new series including my PMIC changes as well early
next week(mondayish hopefully). Can we hold off review of any further
of my voltage fixes patches till then?

> Kevin
>
> diff --git a/arch/arm/mach-omap2/vc.c b/arch/arm/mach-omap2/vc.c
> index fba352d..fa48944 100644
> --- a/arch/arm/mach-omap2/vc.c
> +++ b/arch/arm/mach-omap2/vc.c
> @@ -33,21 +33,20 @@
>  * - command configuration address (RAC) and enable bit (RACEN)
>  * - command values for ON, ONLP, RET and OFF (CMD)
>  *
> - * This function currently only allows flexible configuration of
> - * the non-default channel (e.g. non-zero channels.)  Starting with
> - * OMAP4, only the non-zero channels can be configured.  Channel zero
> - * always uses the channel zero register values.  Therefore, the
> - * same limitation is imposed on OMAP3 for consistency.
> + * This function currently only allows flexible configuration of the
> + * non-default channel.  Starting with OMAP4, there are more than 2
> + * channels, with one defined as the default (on OMAP4, it's MPU.)
> + * Only the non-default channel can be configured.
>  */
>  static int omap_vc_config_channel(struct voltagedomain *voltdm)
>  {
>        struct omap_vc_channel *vc = voltdm->vc;
>
>        /*
> -        * For channel zero, the only configurable bit is RACEN.
> +        * For default channel, the only configurable bit is RACEN.
>         * All others must stay at zero (see function comment above.)
>         */
> -       if (!vc->cfg_channel_sa_shift)
> +       if (vc->is_default_channel)
>                vc->cfg_channel &= CFG_CHANNEL_RACEN;
>
>        voltdm->rmw(CFG_CHANNEL_MASK << vc->cfg_channel_sa_shift,
> diff --git a/arch/arm/mach-omap2/vc.h b/arch/arm/mach-omap2/vc.h
> index f0fb61f..c216b57 100644
> --- a/arch/arm/mach-omap2/vc.h
> +++ b/arch/arm/mach-omap2/vc.h
> @@ -84,6 +84,8 @@ struct omap_vc_channel {
>        u32 smps_cmdra_mask;
>        u8 cmdval_reg;
>        u8 cfg_channel_sa_shift;
> +
> +       bool is_default_channel;
>  };
>
>  extern struct omap_vc_channel omap3_vc_mpu;
> diff --git a/arch/arm/mach-omap2/vc44xx_data.c b/arch/arm/mach-omap2/vc44xx_data.c
> index fe4f4e5..5844b20 100644
> --- a/arch/arm/mach-omap2/vc44xx_data.c
> +++ b/arch/arm/mach-omap2/vc44xx_data.c
> @@ -58,6 +58,7 @@ struct omap_vc_channel omap4_vc_mpu = {
>        .smps_volra_mask = OMAP4430_VOLRA_VDD_MPU_L_MASK,
>        .smps_cmdra_mask = OMAP4430_CMDRA_VDD_MPU_L_MASK,
>        .cfg_channel_sa_shift = OMAP4430_SA_VDD_MPU_L_SHIFT,
> +       .is_default_channel = true,
>  };
>
>  struct omap_vc_channel omap4_vc_iva = {
>


Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [pm_wip/voltdm_nm][PATCH v2 2/3] OMAP4: PM: VC: introduce concept of default channel

[Kevin Hilman]

"Menon, Nishanth" <nm@ti.com> writes:

> On Thu, Jun 2, 2011 at 18:45, Kevin Hilman <khilman@ti.com> wrote:
>> Nishanth Menon <nm@ti.com> writes:
>>
>>> Due to a TRM bug, the current code assumes that channel0(core) is the default
>>> channel. With the additional explanation provided by the hardware team, it is
>>> clear that PRM_VC_CFG_CHANNEL register allows for flexibility of configuring
>>> for various PMIC configurations. For example, the I2C slave address on TWL6030
>>> when using 4430 configuration could potentially use the same slave address for
>>> all domains, while in 4460 configuration, we drive MPU using TPS; Core and IVA
>>> using TWL6030. To allow this flexibility, we state in existing parameters using
>>> a flag to indicate usage of default channel.
>>>
>>> In addition, the TRM update clears up the confusion on the fact that MPU is
>>> infact the default channel on OMAP4.
>>>
>>> We update the same here.
>>>
>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>
>> There's too much going on in this patch not described in the changelog
>> (extra error checking, volt & cmd reg checking etc.)
>>
>> Also, I don't like the USE_DEFAULT_CHANNEL_I2C_PARAM, mainly because it
>> adds clutter without any obvious value.  To me, zero is a perfectly good
>> value to signify "use default channel value", especially since that's
>> the value used by the hardware.
> The reason is that 0 is a valid i2c register address. 8 bits is used
> in VC and I wanted one bit which was further away, hence the 16 bit.
> The usage could be that in .volt_reg_addr or in .cmd_reg_addr I could
> set this up with the macro. and bingo, we use the default domain's
> configuration.
>
> This is esp useful if we had a single pmic reg for all domains.. I
> mean the VC design allows for it, even though we dont use it
> currently.

OK.

See, it's easy to convince me that something is needed/useful.  Of
course, I prefer to be conviced by the changelog. :)  

Please make this feature into a dedicated patch with an appropriately
descriptive changelog.  Thanks.

>>
>> Incidentally, adding this doesn't actually change current behavior.
>> Currently, I use zero (an unconfigured value in the VC settings) to
>> signify it should use default settings.  In your patch, you defined
>> USE_DEFAULT_CHANNEL_I2C_PARAM as 0x10000 (17 bits) but assign it to 16
>> bit fields, which means they are just set to zero. :)
>
>
>> So rather than take this patch, I'm just going to fold the following
>> diff into "OMAP3+: VC: abstract out channel configuration" in voltdm_b.
>> I'll also update the changelog noting that the TRM is wrong here in that
>> it describes CORE as the default channel when it's in fact MPU.
>
> I intend to post a new series including my PMIC changes as well early
> next week(mondayish hopefully). Can we hold off review of any further
> of my voltage fixes patches till then?
>

Sure.  It's the first time anyone as asked me not to review patches. :)
I'll gladly comply.

I've already folded the minimal change I proposed into the original
patch locally, and will push updated voltdm_* branches by the end of
today.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [pm_wip/voltdm_nm][PATCH v2 2/3] OMAP4: PM: VC: introduce concept of default channel

[Menon, Nishanth]

On Thu, Jun 2, 2011 at 19:10, Kevin Hilman <khilman@ti.com> wrote:
> "Menon, Nishanth" <nm@ti.com> writes:
>
>> On Thu, Jun 2, 2011 at 18:45, Kevin Hilman <khilman@ti.com> wrote:
>>> Nishanth Menon <nm@ti.com> writes:
>>>
>>>> Due to a TRM bug, the current code assumes that channel0(core) is the default
>>>> channel. With the additional explanation provided by the hardware team, it is
>>>> clear that PRM_VC_CFG_CHANNEL register allows for flexibility of configuring
>>>> for various PMIC configurations. For example, the I2C slave address on TWL6030
>>>> when using 4430 configuration could potentially use the same slave address for
>>>> all domains, while in 4460 configuration, we drive MPU using TPS; Core and IVA
>>>> using TWL6030. To allow this flexibility, we state in existing parameters using
>>>> a flag to indicate usage of default channel.
>>>>
>>>> In addition, the TRM update clears up the confusion on the fact that MPU is
>>>> infact the default channel on OMAP4.
>>>>
>>>> We update the same here.
>>>>
>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>
>>> There's too much going on in this patch not described in the changelog
>>> (extra error checking, volt & cmd reg checking etc.)
>>>
>>> Also, I don't like the USE_DEFAULT_CHANNEL_I2C_PARAM, mainly because it
>>> adds clutter without any obvious value.  To me, zero is a perfectly good
>>> value to signify "use default channel value", especially since that's
>>> the value used by the hardware.
>> The reason is that 0 is a valid i2c register address. 8 bits is used
>> in VC and I wanted one bit which was further away, hence the 16 bit.
>> The usage could be that in .volt_reg_addr or in .cmd_reg_addr I could
>> set this up with the macro. and bingo, we use the default domain's
>> configuration.
>>
>> This is esp useful if we had a single pmic reg for all domains.. I
>> mean the VC design allows for it, even though we dont use it
>> currently.
>
> OK.
>
> See, it's easy to convince me that something is needed/useful.  Of
> course, I prefer to be conviced by the changelog. :)
>
> Please make this feature into a dedicated patch with an appropriately
> descriptive changelog.  Thanks.

Yes, I agree. it probably is a better idea to break this thing up - I
guess I was a bit lazyish considering these were targetted to be
squashed.. not in the next series :)
>
>>>
>>> Incidentally, adding this doesn't actually change current behavior.
>>> Currently, I use zero (an unconfigured value in the VC settings) to
>>> signify it should use default settings.  In your patch, you defined
>>> USE_DEFAULT_CHANNEL_I2C_PARAM as 0x10000 (17 bits) but assign it to 16
>>> bit fields, which means they are just set to zero. :)
>>
>>
>>> So rather than take this patch, I'm just going to fold the following
>>> diff into "OMAP3+: VC: abstract out channel configuration" in voltdm_b.
>>> I'll also update the changelog noting that the TRM is wrong here in that
>>> it describes CORE as the default channel when it's in fact MPU.
>>
>> I intend to post a new series including my PMIC changes as well early
>> next week(mondayish hopefully). Can we hold off review of any further
>> of my voltage fixes patches till then?
>>
>
> Sure.  It's the first time anyone as asked me not to review patches. :)
> I'll gladly comply.
>
> I've already folded the minimal change I proposed into the original
> patch locally, and will push updated voltdm_* branches by the end of
> today.

:) Thanks. I will rework the patches and line up a series from cpufreq
and voltage domain perspective in the upcoming days..

Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [pm_wip/voltdm_nm][PATCH v2 2/3] OMAP4: PM: VC: introduce concept of default channel

[Kevin Hilman]

Hi Nishanth,

Kevin Hilman <khilman@ti.com> writes:

[...]

>
> So rather than take this patch, I'm just going to fold the following
> diff into "OMAP3+: VC: abstract out channel configuration" in voltdm_b.
> I'll also update the changelog noting that the TRM is wrong here in that
> it describes CORE as the default channel when it's in fact MPU.
>

FYI...

I modified this slightly in the current voltdm_* branch in that I'm now
using a 'u8 flags' in the VC struct intead of a bool for this since  I expect
to have some other reasons to use various flags.

Kevin

>
> diff --git a/arch/arm/mach-omap2/vc.c b/arch/arm/mach-omap2/vc.c
> index fba352d..fa48944 100644
> --- a/arch/arm/mach-omap2/vc.c
> +++ b/arch/arm/mach-omap2/vc.c
> @@ -33,21 +33,20 @@
>   * - command configuration address (RAC) and enable bit (RACEN)
>   * - command values for ON, ONLP, RET and OFF (CMD)
>   *
> - * This function currently only allows flexible configuration of
> - * the non-default channel (e.g. non-zero channels.)  Starting with
> - * OMAP4, only the non-zero channels can be configured.  Channel zero
> - * always uses the channel zero register values.  Therefore, the
> - * same limitation is imposed on OMAP3 for consistency.
> + * This function currently only allows flexible configuration of the
> + * non-default channel.  Starting with OMAP4, there are more than 2
> + * channels, with one defined as the default (on OMAP4, it's MPU.)
> + * Only the non-default channel can be configured.
>   */
>  static int omap_vc_config_channel(struct voltagedomain *voltdm)
>  {
>  	struct omap_vc_channel *vc = voltdm->vc;
>  
>  	/*
> -	 * For channel zero, the only configurable bit is RACEN.
> +	 * For default channel, the only configurable bit is RACEN.
>  	 * All others must stay at zero (see function comment above.)
>  	 */
> -	if (!vc->cfg_channel_sa_shift)
> +	if (vc->is_default_channel)
>  		vc->cfg_channel &= CFG_CHANNEL_RACEN;
>  
>  	voltdm->rmw(CFG_CHANNEL_MASK << vc->cfg_channel_sa_shift,
> diff --git a/arch/arm/mach-omap2/vc.h b/arch/arm/mach-omap2/vc.h
> index f0fb61f..c216b57 100644
> --- a/arch/arm/mach-omap2/vc.h
> +++ b/arch/arm/mach-omap2/vc.h
> @@ -84,6 +84,8 @@ struct omap_vc_channel {
>  	u32 smps_cmdra_mask;
>  	u8 cmdval_reg;
>  	u8 cfg_channel_sa_shift;
> +
> +	bool is_default_channel;
>  };
>  
>  extern struct omap_vc_channel omap3_vc_mpu;
> diff --git a/arch/arm/mach-omap2/vc44xx_data.c b/arch/arm/mach-omap2/vc44xx_data.c
> index fe4f4e5..5844b20 100644
> --- a/arch/arm/mach-omap2/vc44xx_data.c
> +++ b/arch/arm/mach-omap2/vc44xx_data.c
> @@ -58,6 +58,7 @@ struct omap_vc_channel omap4_vc_mpu = {
>  	.smps_volra_mask = OMAP4430_VOLRA_VDD_MPU_L_MASK,
>  	.smps_cmdra_mask = OMAP4430_CMDRA_VDD_MPU_L_MASK,
>  	.cfg_channel_sa_shift = OMAP4430_SA_VDD_MPU_L_SHIFT,
> +	.is_default_channel = true,
>  };
>  
>  struct omap_vc_channel omap4_vc_iva = {

[pm_wip/voltdm_nm][PATCH v2 3/3] OMAP4: PM: VC: make omap_vc_i2c_init static

[Nishanth Menon]

The only user of omap_vc_i2c_init is vc.c itself, makes
no reason for us to expose it out.

This also fixes the sparse warning:
arch/arm/mach-omap2/vc.c:207:13: warning: symbol 'omap_vc_i2c_init' was not declared. Should it be static?

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/vc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/vc.c b/arch/arm/mach-omap2/vc.c
index 42c77a8..d0b52cc 100644
--- a/arch/arm/mach-omap2/vc.c
+++ b/arch/arm/mach-omap2/vc.c
@@ -204,7 +204,7 @@ static void __init omap4_vc_init_channel(struct voltagedomain *voltdm)
  * channel registers.  All other VC channels will use the
  * same configuration.
  */
-void __init omap_vc_i2c_init(struct voltagedomain *voltdm)
+static void __init omap_vc_i2c_init(struct voltagedomain *voltdm)
 {
 	struct omap_vc_channel *vc = voltdm->vc;
 	static bool initialized;
-- 
1.7.1

Re: [pm_wip/voltdm_nm][PATCH v2 3/3] OMAP4: PM: VC: make omap_vc_i2c_init static

[Kevin Hilman]

Nishanth Menon <nm@ti.com> writes:

> The only user of omap_vc_i2c_init is vc.c itself, makes
> no reason for us to expose it out.
>
> This also fixes the sparse warning:
> arch/arm/mach-omap2/vc.c:207:13: warning: symbol 'omap_vc_i2c_init' was not declared. Should it be static?
>
> Signed-off-by: Nishanth Menon <nm@ti.com>

Thanks, will fold into 'OMAP3+: VC: make I2C config programmable with
PMIC-specific settings'

Kevin

> ---
>  arch/arm/mach-omap2/vc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/vc.c b/arch/arm/mach-omap2/vc.c
> index 42c77a8..d0b52cc 100644
> --- a/arch/arm/mach-omap2/vc.c
> +++ b/arch/arm/mach-omap2/vc.c
> @@ -204,7 +204,7 @@ static void __init omap4_vc_init_channel(struct voltagedomain *voltdm)
>   * channel registers.  All other VC channels will use the
>   * same configuration.
>   */
> -void __init omap_vc_i2c_init(struct voltagedomain *voltdm)
> +static void __init omap_vc_i2c_init(struct voltagedomain *voltdm)
>  {
>  	struct omap_vc_channel *vc = voltdm->vc;
>  	static bool initialized;