[PATCH 1/2] mfd: anatop: make register accessor more flexible and rename meaningfully

[PATCH 1/2] mfd: anatop: make register accessor more flexible and rename meaningfully

[Richard Zhao]

 - rename to anatop_read_reg and anatop_write_reg
 - anatop_read_reg directly return reg value
 - anatop_write_reg write reg with mask

Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 drivers/mfd/anatop-mfd.c             |   35 +++++++++++-----------------------
 drivers/regulator/anatop-regulator.c |   18 ++++++++---------
 include/linux/mfd/anatop.h           |    4 ++--
 3 files changed, 21 insertions(+), 36 deletions(-)

diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
index 2af4248..3bb7c49 100644
--- a/drivers/mfd/anatop-mfd.c
+++ b/drivers/mfd/anatop-mfd.c
@@ -41,39 +41,26 @@
 #include <linux/of_address.h>
 #include <linux/mfd/anatop.h>
 
-u32 anatop_get_bits(struct anatop *adata, u32 addr, int bit_shift,
-		    int bit_width)
+u32 anatop_read_reg(struct anatop *adata, u32 addr)
 {
-	u32 val, mask;
-
-	if (bit_width == 32)
-		mask = ~0;
-	else
-		mask = (1 << bit_width) - 1;
-
-	val = readl(adata->ioreg + addr);
-	val = (val >> bit_shift) & mask;
-
-	return val;
+	return readl(adata->ioreg + addr);
 }
-EXPORT_SYMBOL_GPL(anatop_get_bits);
+EXPORT_SYMBOL_GPL(anatop_read_reg);
 
-void anatop_set_bits(struct anatop *adata, u32 addr, int bit_shift,
-		     int bit_width, u32 data)
+void anatop_write_reg(struct anatop *adata, u32 addr, u32 data, u32 mask)
 {
-	u32 val, mask;
+	u32 val;
 
-	if (bit_width == 32)
-		mask = ~0;
-	else
-		mask = (1 << bit_width) - 1;
+	data &= mask;
 
 	spin_lock(&adata->reglock);
-	val = readl(adata->ioreg + addr) & ~(mask << bit_shift);
-	writel((data << bit_shift) | val, adata->ioreg + addr);
+	val = readl(adata->ioreg + addr);
+	val |= data;
+	val &= ~data;
+	writel(val, adata->ioreg + addr);
 	spin_unlock(&adata->reglock);
 }
-EXPORT_SYMBOL_GPL(anatop_set_bits);
+EXPORT_SYMBOL_GPL(anatop_write_reg);
 
 static const struct of_device_id of_anatop_match[] = {
 	{ .compatible = "fsl,imx6q-anatop", },
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 81fd606..0a34085 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -47,7 +47,7 @@ static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
 				  int max_uV, unsigned *selector)
 {
 	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
-	u32 val, sel;
+	u32 val, sel, mask;
 	int uv;
 
 	uv = min_uV;
@@ -71,11 +71,10 @@ static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
 	val = anatop_reg->min_bit_val + sel;
 	*selector = sel;
 	dev_dbg(&reg->dev, "%s: calculated val %d\n", __func__, val);
-	anatop_set_bits(anatop_reg->mfd,
-			anatop_reg->control_reg,
-			anatop_reg->vol_bit_shift,
-			anatop_reg->vol_bit_width,
-			val);
+	mask = ((1 << anatop_reg->vol_bit_width) - 1) <<
+		anatop_reg->vol_bit_shift;
+	val <<= anatop_reg->vol_bit_shift;
+	anatop_write_reg(anatop_reg->mfd, anatop_reg->control_reg, val, mask);
 
 	return 0;
 }
@@ -88,10 +87,9 @@ static int anatop_get_voltage_sel(struct regulator_dev *reg)
 	if (!anatop_reg->control_reg)
 		return -ENOTSUPP;
 
-	val = anatop_get_bits(anatop_reg->mfd,
-			      anatop_reg->control_reg,
-			      anatop_reg->vol_bit_shift,
-			      anatop_reg->vol_bit_width);
+	val = anatop_read_reg(anatop_reg->mfd, anatop_reg->control_reg);
+	val = (val & ((1 << anatop_reg->vol_bit_width) - 1)) >>
+		anatop_reg->vol_bit_shift;
 
 	return val - anatop_reg->min_bit_val;
 }
diff --git a/include/linux/mfd/anatop.h b/include/linux/mfd/anatop.h
index 22c1007..7f92acf 100644
--- a/include/linux/mfd/anatop.h
+++ b/include/linux/mfd/anatop.h
@@ -34,7 +34,7 @@ struct anatop {
 	spinlock_t reglock;
 };
 
-extern u32 anatop_get_bits(struct anatop *, u32, int, int);
-extern void anatop_set_bits(struct anatop *, u32, int, int, u32);
+extern u32 anatop_read_reg(struct anatop *, u32);
+extern void anatop_write_reg(struct anatop *, u32, u32, u32);
 
 #endif /*  __LINUX_MFD_ANATOP_H */
-- 
1.7.9.5

[PATCH 1/2] mfd: anatop: make register accessor more flexible and rename meaningfully

[Richard Zhao]

 - rename to anatop_read_reg and anatop_write_reg
 - anatop_read_reg directly return reg value
 - anatop_write_reg write reg with mask

Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 drivers/mfd/anatop-mfd.c             |   35 +++++++++++-----------------------
 drivers/regulator/anatop-regulator.c |   18 ++++++++---------
 include/linux/mfd/anatop.h           |    4 ++--
 3 files changed, 21 insertions(+), 36 deletions(-)

diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
index 2af4248..3bb7c49 100644
--- a/drivers/mfd/anatop-mfd.c
+++ b/drivers/mfd/anatop-mfd.c
@@ -41,39 +41,26 @@
 #include <linux/of_address.h>
 #include <linux/mfd/anatop.h>
 
-u32 anatop_get_bits(struct anatop *adata, u32 addr, int bit_shift,
-		    int bit_width)
+u32 anatop_read_reg(struct anatop *adata, u32 addr)
 {
-	u32 val, mask;
-
-	if (bit_width == 32)
-		mask = ~0;
-	else
-		mask = (1 << bit_width) - 1;
-
-	val = readl(adata->ioreg + addr);
-	val = (val >> bit_shift) & mask;
-
-	return val;
+	return readl(adata->ioreg + addr);
 }
-EXPORT_SYMBOL_GPL(anatop_get_bits);
+EXPORT_SYMBOL_GPL(anatop_read_reg);
 
-void anatop_set_bits(struct anatop *adata, u32 addr, int bit_shift,
-		     int bit_width, u32 data)
+void anatop_write_reg(struct anatop *adata, u32 addr, u32 data, u32 mask)
 {
-	u32 val, mask;
+	u32 val;
 
-	if (bit_width == 32)
-		mask = ~0;
-	else
-		mask = (1 << bit_width) - 1;
+	data &= mask;
 
 	spin_lock(&adata->reglock);
-	val = readl(adata->ioreg + addr) & ~(mask << bit_shift);
-	writel((data << bit_shift) | val, adata->ioreg + addr);
+	val = readl(adata->ioreg + addr);
+	val |= data;
+	val &= ~data;
+	writel(val, adata->ioreg + addr);
 	spin_unlock(&adata->reglock);
 }
-EXPORT_SYMBOL_GPL(anatop_set_bits);
+EXPORT_SYMBOL_GPL(anatop_write_reg);
 
 static const struct of_device_id of_anatop_match[] = {
 	{ .compatible = "fsl,imx6q-anatop", },
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 81fd606..0a34085 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -47,7 +47,7 @@ static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
 				  int max_uV, unsigned *selector)
 {
 	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
-	u32 val, sel;
+	u32 val, sel, mask;
 	int uv;
 
 	uv = min_uV;
@@ -71,11 +71,10 @@ static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
 	val = anatop_reg->min_bit_val + sel;
 	*selector = sel;
 	dev_dbg(&reg->dev, "%s: calculated val %d\n", __func__, val);
-	anatop_set_bits(anatop_reg->mfd,
-			anatop_reg->control_reg,
-			anatop_reg->vol_bit_shift,
-			anatop_reg->vol_bit_width,
-			val);
+	mask = ((1 << anatop_reg->vol_bit_width) - 1) <<
+		anatop_reg->vol_bit_shift;
+	val <<= anatop_reg->vol_bit_shift;
+	anatop_write_reg(anatop_reg->mfd, anatop_reg->control_reg, val, mask);
 
 	return 0;
 }
@@ -88,10 +87,9 @@ static int anatop_get_voltage_sel(struct regulator_dev *reg)
 	if (!anatop_reg->control_reg)
 		return -ENOTSUPP;
 
-	val = anatop_get_bits(anatop_reg->mfd,
-			      anatop_reg->control_reg,
-			      anatop_reg->vol_bit_shift,
-			      anatop_reg->vol_bit_width);
+	val = anatop_read_reg(anatop_reg->mfd, anatop_reg->control_reg);
+	val = (val & ((1 << anatop_reg->vol_bit_width) - 1)) >>
+		anatop_reg->vol_bit_shift;
 
 	return val - anatop_reg->min_bit_val;
 }
diff --git a/include/linux/mfd/anatop.h b/include/linux/mfd/anatop.h
index 22c1007..7f92acf 100644
--- a/include/linux/mfd/anatop.h
+++ b/include/linux/mfd/anatop.h
@@ -34,7 +34,7 @@ struct anatop {
 	spinlock_t reglock;
 };
 
-extern u32 anatop_get_bits(struct anatop *, u32, int, int);
-extern void anatop_set_bits(struct anatop *, u32, int, int, u32);
+extern u32 anatop_read_reg(struct anatop *, u32);
+extern void anatop_write_reg(struct anatop *, u32, u32, u32);
 
 #endif /*  __LINUX_MFD_ANATOP_H */
-- 
1.7.9.5

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Richard Zhao]

It makes anatop register access easier. Anatop has many misc registers,
which may not be a specific driver.

There's only one anatop device for a running system, so we use a global
variable to store struct anatop.

Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 drivers/mfd/anatop-mfd.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
index 3bb7c49..b0313210 100644
--- a/drivers/mfd/anatop-mfd.c
+++ b/drivers/mfd/anatop-mfd.c
@@ -41,8 +41,15 @@
 #include <linux/of_address.h>
 #include <linux/mfd/anatop.h>
 
+/* For any running system, there's only one anatop device. */
+static struct anatop *anatop_data;
+
 u32 anatop_read_reg(struct anatop *adata, u32 addr)
 {
+	BUG_ON(!anatop_data);
+	if (!adata)
+		adata = anatop_data;
+
 	return readl(adata->ioreg + addr);
 }
 EXPORT_SYMBOL_GPL(anatop_read_reg);
@@ -51,6 +58,10 @@ void anatop_write_reg(struct anatop *adata, u32 addr, u32 data, u32 mask)
 {
 	u32 val;
 
+	BUG_ON(!anatop_data);
+	if (!adata)
+		adata = anatop_data;
+
 	data &= mask;
 
 	spin_lock(&adata->reglock);
@@ -83,6 +94,7 @@ static int __devinit of_anatop_probe(struct platform_device *pdev)
 	drvdata->ioreg = ioreg;
 	spin_lock_init(&drvdata->reglock);
 	platform_set_drvdata(pdev, drvdata);
+	anatop_data = drvdata;
 	of_platform_populate(np, of_anatop_match, NULL, dev);
 
 	return 0;
-- 
1.7.9.5

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Richard Zhao]

It makes anatop register access easier. Anatop has many misc registers,
which may not be a specific driver.

There's only one anatop device for a running system, so we use a global
variable to store struct anatop.

Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
 drivers/mfd/anatop-mfd.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
index 3bb7c49..b0313210 100644
--- a/drivers/mfd/anatop-mfd.c
+++ b/drivers/mfd/anatop-mfd.c
@@ -41,8 +41,15 @@
 #include <linux/of_address.h>
 #include <linux/mfd/anatop.h>
 
+/* For any running system, there's only one anatop device. */
+static struct anatop *anatop_data;
+
 u32 anatop_read_reg(struct anatop *adata, u32 addr)
 {
+	BUG_ON(!anatop_data);
+	if (!adata)
+		adata = anatop_data;
+
 	return readl(adata->ioreg + addr);
 }
 EXPORT_SYMBOL_GPL(anatop_read_reg);
@@ -51,6 +58,10 @@ void anatop_write_reg(struct anatop *adata, u32 addr, u32 data, u32 mask)
 {
 	u32 val;
 
+	BUG_ON(!anatop_data);
+	if (!adata)
+		adata = anatop_data;
+
 	data &= mask;
 
 	spin_lock(&adata->reglock);
@@ -83,6 +94,7 @@ static int __devinit of_anatop_probe(struct platform_device *pdev)
 	drvdata->ioreg = ioreg;
 	spin_lock_init(&drvdata->reglock);
 	platform_set_drvdata(pdev, drvdata);
+	anatop_data = drvdata;
 	of_platform_populate(np, of_anatop_match, NULL, dev);
 
 	return 0;
-- 
1.7.9.5

[PATCH V2] mfd: anatop: make register accessor more flexible and rename meaningfully

[Richard Zhao]

 - rename to anatop_read_reg and anatop_write_reg
 - anatop_read_reg directly return reg value
 - anatop_write_reg write reg with mask

Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
Changes since V1:
correct bit operation in anatop_write_reg

 drivers/mfd/anatop-mfd.c             |   35 +++++++++++-----------------------
 drivers/regulator/anatop-regulator.c |   18 ++++++++---------
 include/linux/mfd/anatop.h           |    4 ++--
 3 files changed, 21 insertions(+), 36 deletions(-)

diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
index 2af4248..6da0634 100644
--- a/drivers/mfd/anatop-mfd.c
+++ b/drivers/mfd/anatop-mfd.c
@@ -41,39 +41,26 @@
 #include <linux/of_address.h>
 #include <linux/mfd/anatop.h>
 
-u32 anatop_get_bits(struct anatop *adata, u32 addr, int bit_shift,
-		    int bit_width)
+u32 anatop_read_reg(struct anatop *adata, u32 addr)
 {
-	u32 val, mask;
-
-	if (bit_width == 32)
-		mask = ~0;
-	else
-		mask = (1 << bit_width) - 1;
-
-	val = readl(adata->ioreg + addr);
-	val = (val >> bit_shift) & mask;
-
-	return val;
+	return readl(adata->ioreg + addr);
 }
-EXPORT_SYMBOL_GPL(anatop_get_bits);
+EXPORT_SYMBOL_GPL(anatop_read_reg);
 
-void anatop_set_bits(struct anatop *adata, u32 addr, int bit_shift,
-		     int bit_width, u32 data)
+void anatop_write_reg(struct anatop *adata, u32 addr, u32 data, u32 mask)
 {
-	u32 val, mask;
+	u32 val;
 
-	if (bit_width == 32)
-		mask = ~0;
-	else
-		mask = (1 << bit_width) - 1;
+	data &= mask;
 
 	spin_lock(&adata->reglock);
-	val = readl(adata->ioreg + addr) & ~(mask << bit_shift);
-	writel((data << bit_shift) | val, adata->ioreg + addr);
+	val = readl(adata->ioreg + addr);
+	val &= ~mask;
+	val |= data;
+	writel(val, adata->ioreg + addr);
 	spin_unlock(&adata->reglock);
 }
-EXPORT_SYMBOL_GPL(anatop_set_bits);
+EXPORT_SYMBOL_GPL(anatop_write_reg);
 
 static const struct of_device_id of_anatop_match[] = {
 	{ .compatible = "fsl,imx6q-anatop", },
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 81fd606..0a34085 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -47,7 +47,7 @@ static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
 				  int max_uV, unsigned *selector)
 {
 	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
-	u32 val, sel;
+	u32 val, sel, mask;
 	int uv;
 
 	uv = min_uV;
@@ -71,11 +71,10 @@ static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
 	val = anatop_reg->min_bit_val + sel;
 	*selector = sel;
 	dev_dbg(&reg->dev, "%s: calculated val %d\n", __func__, val);
-	anatop_set_bits(anatop_reg->mfd,
-			anatop_reg->control_reg,
-			anatop_reg->vol_bit_shift,
-			anatop_reg->vol_bit_width,
-			val);
+	mask = ((1 << anatop_reg->vol_bit_width) - 1) <<
+		anatop_reg->vol_bit_shift;
+	val <<= anatop_reg->vol_bit_shift;
+	anatop_write_reg(anatop_reg->mfd, anatop_reg->control_reg, val, mask);
 
 	return 0;
 }
@@ -88,10 +87,9 @@ static int anatop_get_voltage_sel(struct regulator_dev *reg)
 	if (!anatop_reg->control_reg)
 		return -ENOTSUPP;
 
-	val = anatop_get_bits(anatop_reg->mfd,
-			      anatop_reg->control_reg,
-			      anatop_reg->vol_bit_shift,
-			      anatop_reg->vol_bit_width);
+	val = anatop_read_reg(anatop_reg->mfd, anatop_reg->control_reg);
+	val = (val & ((1 << anatop_reg->vol_bit_width) - 1)) >>
+		anatop_reg->vol_bit_shift;
 
 	return val - anatop_reg->min_bit_val;
 }
diff --git a/include/linux/mfd/anatop.h b/include/linux/mfd/anatop.h
index 22c1007..7f92acf 100644
--- a/include/linux/mfd/anatop.h
+++ b/include/linux/mfd/anatop.h
@@ -34,7 +34,7 @@ struct anatop {
 	spinlock_t reglock;
 };
 
-extern u32 anatop_get_bits(struct anatop *, u32, int, int);
-extern void anatop_set_bits(struct anatop *, u32, int, int, u32);
+extern u32 anatop_read_reg(struct anatop *, u32);
+extern void anatop_write_reg(struct anatop *, u32, u32, u32);
 
 #endif /*  __LINUX_MFD_ANATOP_H */
-- 
1.7.9.5

[PATCH V2] mfd: anatop: make register accessor more flexible and rename meaningfully

[Richard Zhao]

 - rename to anatop_read_reg and anatop_write_reg
 - anatop_read_reg directly return reg value
 - anatop_write_reg write reg with mask

Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
---
Changes since V1:
correct bit operation in anatop_write_reg

 drivers/mfd/anatop-mfd.c             |   35 +++++++++++-----------------------
 drivers/regulator/anatop-regulator.c |   18 ++++++++---------
 include/linux/mfd/anatop.h           |    4 ++--
 3 files changed, 21 insertions(+), 36 deletions(-)

diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
index 2af4248..6da0634 100644
--- a/drivers/mfd/anatop-mfd.c
+++ b/drivers/mfd/anatop-mfd.c
@@ -41,39 +41,26 @@
 #include <linux/of_address.h>
 #include <linux/mfd/anatop.h>
 
-u32 anatop_get_bits(struct anatop *adata, u32 addr, int bit_shift,
-		    int bit_width)
+u32 anatop_read_reg(struct anatop *adata, u32 addr)
 {
-	u32 val, mask;
-
-	if (bit_width == 32)
-		mask = ~0;
-	else
-		mask = (1 << bit_width) - 1;
-
-	val = readl(adata->ioreg + addr);
-	val = (val >> bit_shift) & mask;
-
-	return val;
+	return readl(adata->ioreg + addr);
 }
-EXPORT_SYMBOL_GPL(anatop_get_bits);
+EXPORT_SYMBOL_GPL(anatop_read_reg);
 
-void anatop_set_bits(struct anatop *adata, u32 addr, int bit_shift,
-		     int bit_width, u32 data)
+void anatop_write_reg(struct anatop *adata, u32 addr, u32 data, u32 mask)
 {
-	u32 val, mask;
+	u32 val;
 
-	if (bit_width == 32)
-		mask = ~0;
-	else
-		mask = (1 << bit_width) - 1;
+	data &= mask;
 
 	spin_lock(&adata->reglock);
-	val = readl(adata->ioreg + addr) & ~(mask << bit_shift);
-	writel((data << bit_shift) | val, adata->ioreg + addr);
+	val = readl(adata->ioreg + addr);
+	val &= ~mask;
+	val |= data;
+	writel(val, adata->ioreg + addr);
 	spin_unlock(&adata->reglock);
 }
-EXPORT_SYMBOL_GPL(anatop_set_bits);
+EXPORT_SYMBOL_GPL(anatop_write_reg);
 
 static const struct of_device_id of_anatop_match[] = {
 	{ .compatible = "fsl,imx6q-anatop", },
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 81fd606..0a34085 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -47,7 +47,7 @@ static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
 				  int max_uV, unsigned *selector)
 {
 	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
-	u32 val, sel;
+	u32 val, sel, mask;
 	int uv;
 
 	uv = min_uV;
@@ -71,11 +71,10 @@ static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
 	val = anatop_reg->min_bit_val + sel;
 	*selector = sel;
 	dev_dbg(&reg->dev, "%s: calculated val %d\n", __func__, val);
-	anatop_set_bits(anatop_reg->mfd,
-			anatop_reg->control_reg,
-			anatop_reg->vol_bit_shift,
-			anatop_reg->vol_bit_width,
-			val);
+	mask = ((1 << anatop_reg->vol_bit_width) - 1) <<
+		anatop_reg->vol_bit_shift;
+	val <<= anatop_reg->vol_bit_shift;
+	anatop_write_reg(anatop_reg->mfd, anatop_reg->control_reg, val, mask);
 
 	return 0;
 }
@@ -88,10 +87,9 @@ static int anatop_get_voltage_sel(struct regulator_dev *reg)
 	if (!anatop_reg->control_reg)
 		return -ENOTSUPP;
 
-	val = anatop_get_bits(anatop_reg->mfd,
-			      anatop_reg->control_reg,
-			      anatop_reg->vol_bit_shift,
-			      anatop_reg->vol_bit_width);
+	val = anatop_read_reg(anatop_reg->mfd, anatop_reg->control_reg);
+	val = (val & ((1 << anatop_reg->vol_bit_width) - 1)) >>
+		anatop_reg->vol_bit_shift;
 
 	return val - anatop_reg->min_bit_val;
 }
diff --git a/include/linux/mfd/anatop.h b/include/linux/mfd/anatop.h
index 22c1007..7f92acf 100644
--- a/include/linux/mfd/anatop.h
+++ b/include/linux/mfd/anatop.h
@@ -34,7 +34,7 @@ struct anatop {
 	spinlock_t reglock;
 };
 
-extern u32 anatop_get_bits(struct anatop *, u32, int, int);
-extern void anatop_set_bits(struct anatop *, u32, int, int, u32);
+extern u32 anatop_read_reg(struct anatop *, u32);
+extern void anatop_write_reg(struct anatop *, u32, u32, u32);
 
 #endif /*  __LINUX_MFD_ANATOP_H */
-- 
1.7.9.5

Re: [PATCH V2] mfd: anatop: make register accessor more flexible and rename meaningfully

[Ying-Chun Liu (PaulLiu)]

(2012年05月13日 09:18), Richard Zhao wrote:
>  - rename to anatop_read_reg and anatop_write_reg
>  - anatop_read_reg directly return reg value
>  - anatop_write_reg write reg with mask
> 
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> ---
> Changes since V1:
> correct bit operation in anatop_write_reg
> 
>  drivers/mfd/anatop-mfd.c             |   35 +++++++++++-----------------------
>  drivers/regulator/anatop-regulator.c |   18 ++++++++---------
>  include/linux/mfd/anatop.h           |    4 ++--
>  3 files changed, 21 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
> index 2af4248..6da0634 100644
> --- a/drivers/mfd/anatop-mfd.c
> +++ b/drivers/mfd/anatop-mfd.c
> @@ -41,39 +41,26 @@
>  #include <linux/of_address.h>
>  #include <linux/mfd/anatop.h>
>  
> -u32 anatop_get_bits(struct anatop *adata, u32 addr, int bit_shift,
> -		    int bit_width)
> +u32 anatop_read_reg(struct anatop *adata, u32 addr)
>  {
> -	u32 val, mask;
> -
> -	if (bit_width == 32)
> -		mask = ~0;
> -	else
> -		mask = (1 << bit_width) - 1;
> -
> -	val = readl(adata->ioreg + addr);
> -	val = (val >> bit_shift) & mask;
> -
> -	return val;
> +	return readl(adata->ioreg + addr);
>  }
> -EXPORT_SYMBOL_GPL(anatop_get_bits);
> +EXPORT_SYMBOL_GPL(anatop_read_reg);
>  
> -void anatop_set_bits(struct anatop *adata, u32 addr, int bit_shift,
> -		     int bit_width, u32 data)
> +void anatop_write_reg(struct anatop *adata, u32 addr, u32 data, u32 mask)
>  {
> -	u32 val, mask;
> +	u32 val;
>  
> -	if (bit_width == 32)
> -		mask = ~0;
> -	else
> -		mask = (1 << bit_width) - 1;
> +	data &= mask;
>  
>  	spin_lock(&adata->reglock);
> -	val = readl(adata->ioreg + addr) & ~(mask << bit_shift);
> -	writel((data << bit_shift) | val, adata->ioreg + addr);
> +	val = readl(adata->ioreg + addr);
> +	val &= ~mask;
> +	val |= data;
> +	writel(val, adata->ioreg + addr);
>  	spin_unlock(&adata->reglock);
>  }
> -EXPORT_SYMBOL_GPL(anatop_set_bits);
> +EXPORT_SYMBOL_GPL(anatop_write_reg);
>  
>  static const struct of_device_id of_anatop_match[] = {
>  	{ .compatible = "fsl,imx6q-anatop", },
> diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
> index 81fd606..0a34085 100644
> --- a/drivers/regulator/anatop-regulator.c
> +++ b/drivers/regulator/anatop-regulator.c
> @@ -47,7 +47,7 @@ static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
>  				  int max_uV, unsigned *selector)
>  {
>  	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
> -	u32 val, sel;
> +	u32 val, sel, mask;
>  	int uv;
>  
>  	uv = min_uV;
> @@ -71,11 +71,10 @@ static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
>  	val = anatop_reg->min_bit_val + sel;
>  	*selector = sel;
>  	dev_dbg(&reg->dev, "%s: calculated val %d\n", __func__, val);
> -	anatop_set_bits(anatop_reg->mfd,
> -			anatop_reg->control_reg,
> -			anatop_reg->vol_bit_shift,
> -			anatop_reg->vol_bit_width,
> -			val);
> +	mask = ((1 << anatop_reg->vol_bit_width) - 1) <<
> +		anatop_reg->vol_bit_shift;
> +	val <<= anatop_reg->vol_bit_shift;
> +	anatop_write_reg(anatop_reg->mfd, anatop_reg->control_reg, val, mask);
>  
>  	return 0;
>  }
> @@ -88,10 +87,9 @@ static int anatop_get_voltage_sel(struct regulator_dev *reg)
>  	if (!anatop_reg->control_reg)
>  		return -ENOTSUPP;
>  
> -	val = anatop_get_bits(anatop_reg->mfd,
> -			      anatop_reg->control_reg,
> -			      anatop_reg->vol_bit_shift,
> -			      anatop_reg->vol_bit_width);
> +	val = anatop_read_reg(anatop_reg->mfd, anatop_reg->control_reg);
> +	val = (val & ((1 << anatop_reg->vol_bit_width) - 1)) >>
> +		anatop_reg->vol_bit_shift;
>  
>  	return val - anatop_reg->min_bit_val;
>  }
> diff --git a/include/linux/mfd/anatop.h b/include/linux/mfd/anatop.h
> index 22c1007..7f92acf 100644
> --- a/include/linux/mfd/anatop.h
> +++ b/include/linux/mfd/anatop.h
> @@ -34,7 +34,7 @@ struct anatop {
>  	spinlock_t reglock;
>  };
>  
> -extern u32 anatop_get_bits(struct anatop *, u32, int, int);
> -extern void anatop_set_bits(struct anatop *, u32, int, int, u32);
> +extern u32 anatop_read_reg(struct anatop *, u32);
> +extern void anatop_write_reg(struct anatop *, u32, u32, u32);
>  
>  #endif /*  __LINUX_MFD_ANATOP_H */

Reviewed-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>

Thanks...

[PATCH V2] mfd: anatop: make register accessor more flexible and rename meaningfully

[Ying-Chun Liu (PaulLiu)]

(2012?05?13? 09:18), Richard Zhao wrote:
>  - rename to anatop_read_reg and anatop_write_reg
>  - anatop_read_reg directly return reg value
>  - anatop_write_reg write reg with mask
> 
> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> ---
> Changes since V1:
> correct bit operation in anatop_write_reg
> 
>  drivers/mfd/anatop-mfd.c             |   35 +++++++++++-----------------------
>  drivers/regulator/anatop-regulator.c |   18 ++++++++---------
>  include/linux/mfd/anatop.h           |    4 ++--
>  3 files changed, 21 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
> index 2af4248..6da0634 100644
> --- a/drivers/mfd/anatop-mfd.c
> +++ b/drivers/mfd/anatop-mfd.c
> @@ -41,39 +41,26 @@
>  #include <linux/of_address.h>
>  #include <linux/mfd/anatop.h>
>  
> -u32 anatop_get_bits(struct anatop *adata, u32 addr, int bit_shift,
> -		    int bit_width)
> +u32 anatop_read_reg(struct anatop *adata, u32 addr)
>  {
> -	u32 val, mask;
> -
> -	if (bit_width == 32)
> -		mask = ~0;
> -	else
> -		mask = (1 << bit_width) - 1;
> -
> -	val = readl(adata->ioreg + addr);
> -	val = (val >> bit_shift) & mask;
> -
> -	return val;
> +	return readl(adata->ioreg + addr);
>  }
> -EXPORT_SYMBOL_GPL(anatop_get_bits);
> +EXPORT_SYMBOL_GPL(anatop_read_reg);
>  
> -void anatop_set_bits(struct anatop *adata, u32 addr, int bit_shift,
> -		     int bit_width, u32 data)
> +void anatop_write_reg(struct anatop *adata, u32 addr, u32 data, u32 mask)
>  {
> -	u32 val, mask;
> +	u32 val;
>  
> -	if (bit_width == 32)
> -		mask = ~0;
> -	else
> -		mask = (1 << bit_width) - 1;
> +	data &= mask;
>  
>  	spin_lock(&adata->reglock);
> -	val = readl(adata->ioreg + addr) & ~(mask << bit_shift);
> -	writel((data << bit_shift) | val, adata->ioreg + addr);
> +	val = readl(adata->ioreg + addr);
> +	val &= ~mask;
> +	val |= data;
> +	writel(val, adata->ioreg + addr);
>  	spin_unlock(&adata->reglock);
>  }
> -EXPORT_SYMBOL_GPL(anatop_set_bits);
> +EXPORT_SYMBOL_GPL(anatop_write_reg);
>  
>  static const struct of_device_id of_anatop_match[] = {
>  	{ .compatible = "fsl,imx6q-anatop", },
> diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
> index 81fd606..0a34085 100644
> --- a/drivers/regulator/anatop-regulator.c
> +++ b/drivers/regulator/anatop-regulator.c
> @@ -47,7 +47,7 @@ static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
>  				  int max_uV, unsigned *selector)
>  {
>  	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
> -	u32 val, sel;
> +	u32 val, sel, mask;
>  	int uv;
>  
>  	uv = min_uV;
> @@ -71,11 +71,10 @@ static int anatop_set_voltage(struct regulator_dev *reg, int min_uV,
>  	val = anatop_reg->min_bit_val + sel;
>  	*selector = sel;
>  	dev_dbg(&reg->dev, "%s: calculated val %d\n", __func__, val);
> -	anatop_set_bits(anatop_reg->mfd,
> -			anatop_reg->control_reg,
> -			anatop_reg->vol_bit_shift,
> -			anatop_reg->vol_bit_width,
> -			val);
> +	mask = ((1 << anatop_reg->vol_bit_width) - 1) <<
> +		anatop_reg->vol_bit_shift;
> +	val <<= anatop_reg->vol_bit_shift;
> +	anatop_write_reg(anatop_reg->mfd, anatop_reg->control_reg, val, mask);
>  
>  	return 0;
>  }
> @@ -88,10 +87,9 @@ static int anatop_get_voltage_sel(struct regulator_dev *reg)
>  	if (!anatop_reg->control_reg)
>  		return -ENOTSUPP;
>  
> -	val = anatop_get_bits(anatop_reg->mfd,
> -			      anatop_reg->control_reg,
> -			      anatop_reg->vol_bit_shift,
> -			      anatop_reg->vol_bit_width);
> +	val = anatop_read_reg(anatop_reg->mfd, anatop_reg->control_reg);
> +	val = (val & ((1 << anatop_reg->vol_bit_width) - 1)) >>
> +		anatop_reg->vol_bit_shift;
>  
>  	return val - anatop_reg->min_bit_val;
>  }
> diff --git a/include/linux/mfd/anatop.h b/include/linux/mfd/anatop.h
> index 22c1007..7f92acf 100644
> --- a/include/linux/mfd/anatop.h
> +++ b/include/linux/mfd/anatop.h
> @@ -34,7 +34,7 @@ struct anatop {
>  	spinlock_t reglock;
>  };
>  
> -extern u32 anatop_get_bits(struct anatop *, u32, int, int);
> -extern void anatop_set_bits(struct anatop *, u32, int, int, u32);
> +extern u32 anatop_read_reg(struct anatop *, u32);
> +extern void anatop_write_reg(struct anatop *, u32, u32, u32);
>  
>  #endif /*  __LINUX_MFD_ANATOP_H */

Reviewed-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>

Thanks...

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Shawn Guo]

On Sun, May 13, 2012 at 08:59:54AM +0800, Richard Zhao wrote:
> It makes anatop register access easier. Anatop has many misc registers,
> which may not be a specific driver.
> 
> There's only one anatop device for a running system, so we use a global
> variable to store struct anatop.
> 
>From what I see, it's reasonable.  Then the immediate question I have
is, should we remove "struct anatop *adata" from anatop_read_reg and
anatop_write_reg completely?

Regards,
Shawn

> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> ---
>  drivers/mfd/anatop-mfd.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
> index 3bb7c49..b0313210 100644
> --- a/drivers/mfd/anatop-mfd.c
> +++ b/drivers/mfd/anatop-mfd.c
> @@ -41,8 +41,15 @@
>  #include <linux/of_address.h>
>  #include <linux/mfd/anatop.h>
>  
> +/* For any running system, there's only one anatop device. */
> +static struct anatop *anatop_data;
> +
>  u32 anatop_read_reg(struct anatop *adata, u32 addr)
>  {
> +	BUG_ON(!anatop_data);
> +	if (!adata)
> +		adata = anatop_data;
> +
>  	return readl(adata->ioreg + addr);
>  }
>  EXPORT_SYMBOL_GPL(anatop_read_reg);
> @@ -51,6 +58,10 @@ void anatop_write_reg(struct anatop *adata, u32 addr, u32 data, u32 mask)
>  {
>  	u32 val;
>  
> +	BUG_ON(!anatop_data);
> +	if (!adata)
> +		adata = anatop_data;
> +
>  	data &= mask;
>  
>  	spin_lock(&adata->reglock);
> @@ -83,6 +94,7 @@ static int __devinit of_anatop_probe(struct platform_device *pdev)
>  	drvdata->ioreg = ioreg;
>  	spin_lock_init(&drvdata->reglock);
>  	platform_set_drvdata(pdev, drvdata);
> +	anatop_data = drvdata;
>  	of_platform_populate(np, of_anatop_match, NULL, dev);
>  
>  	return 0;
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Shawn Guo]

On Sun, May 13, 2012 at 08:59:54AM +0800, Richard Zhao wrote:
> It makes anatop register access easier. Anatop has many misc registers,
> which may not be a specific driver.
> 
> There's only one anatop device for a running system, so we use a global
> variable to store struct anatop.
> 
>From what I see, it's reasonable.  Then the immediate question I have
is, should we remove "struct anatop *adata" from anatop_read_reg and
anatop_write_reg completely?

Regards,
Shawn

> Signed-off-by: Richard Zhao <richard.zhao@freescale.com>
> ---
>  drivers/mfd/anatop-mfd.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/mfd/anatop-mfd.c b/drivers/mfd/anatop-mfd.c
> index 3bb7c49..b0313210 100644
> --- a/drivers/mfd/anatop-mfd.c
> +++ b/drivers/mfd/anatop-mfd.c
> @@ -41,8 +41,15 @@
>  #include <linux/of_address.h>
>  #include <linux/mfd/anatop.h>
>  
> +/* For any running system, there's only one anatop device. */
> +static struct anatop *anatop_data;
> +
>  u32 anatop_read_reg(struct anatop *adata, u32 addr)
>  {
> +	BUG_ON(!anatop_data);
> +	if (!adata)
> +		adata = anatop_data;
> +
>  	return readl(adata->ioreg + addr);
>  }
>  EXPORT_SYMBOL_GPL(anatop_read_reg);
> @@ -51,6 +58,10 @@ void anatop_write_reg(struct anatop *adata, u32 addr, u32 data, u32 mask)
>  {
>  	u32 val;
>  
> +	BUG_ON(!anatop_data);
> +	if (!adata)
> +		adata = anatop_data;
> +
>  	data &= mask;
>  
>  	spin_lock(&adata->reglock);
> @@ -83,6 +94,7 @@ static int __devinit of_anatop_probe(struct platform_device *pdev)
>  	drvdata->ioreg = ioreg;
>  	spin_lock_init(&drvdata->reglock);
>  	platform_set_drvdata(pdev, drvdata);
> +	anatop_data = drvdata;
>  	of_platform_populate(np, of_anatop_match, NULL, dev);
>  
>  	return 0;
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 380 bytes --]

On Mon, May 14, 2012 at 11:51:38AM +0800, Shawn Guo wrote:

> From what I see, it's reasonable.  Then the immediate question I have
> is, should we remove "struct anatop *adata" from anatop_read_reg and
> anatop_write_reg completely?

Given the way these things tend to go it's probably guaranteeing that
your next round of SoCs will have two register compatible anatop blocks :)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Mark Brown]

On Mon, May 14, 2012 at 11:51:38AM +0800, Shawn Guo wrote:

> From what I see, it's reasonable.  Then the immediate question I have
> is, should we remove "struct anatop *adata" from anatop_read_reg and
> anatop_write_reg completely?

Given the way these things tend to go it's probably guaranteeing that
your next round of SoCs will have two register compatible anatop blocks :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120514/f89fb098/attachment.sig>

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Shawn Guo]

On Mon, May 14, 2012 at 09:08:36AM +0100, Mark Brown wrote:
> On Mon, May 14, 2012 at 11:51:38AM +0800, Shawn Guo wrote:
> 
> > From what I see, it's reasonable.  Then the immediate question I have
> > is, should we remove "struct anatop *adata" from anatop_read_reg and
> > anatop_write_reg completely?
> 
> Given the way these things tend to go it's probably guaranteeing that
> your next round of SoCs will have two register compatible anatop blocks :)

Considering anatop block tends to be a container of misc hardware
control bits, I haven't really seen any possibility that the future
SoCs will have multiple anatop blocks.

-- 
Regards,
Shawn

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Shawn Guo]

On Mon, May 14, 2012 at 09:08:36AM +0100, Mark Brown wrote:
> On Mon, May 14, 2012 at 11:51:38AM +0800, Shawn Guo wrote:
> 
> > From what I see, it's reasonable.  Then the immediate question I have
> > is, should we remove "struct anatop *adata" from anatop_read_reg and
> > anatop_write_reg completely?
> 
> Given the way these things tend to go it's probably guaranteeing that
> your next round of SoCs will have two register compatible anatop blocks :)

Considering anatop block tends to be a container of misc hardware
control bits, I haven't really seen any possibility that the future
SoCs will have multiple anatop blocks.

-- 
Regards,
Shawn

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Ying-Chun Liu (PaulLiu)]

(2012年05月14日 16:48), Shawn Guo wrote:
> On Mon, May 14, 2012 at 09:08:36AM +0100, Mark Brown wrote:
>> On Mon, May 14, 2012 at 11:51:38AM +0800, Shawn Guo wrote:
>>
>>> From what I see, it's reasonable.  Then the immediate question I have
>>> is, should we remove "struct anatop *adata" from anatop_read_reg and
>>> anatop_write_reg completely?
>>
>> Given the way these things tend to go it's probably guaranteeing that
>> your next round of SoCs will have two register compatible anatop blocks :)
> 
> Considering anatop block tends to be a container of misc hardware
> control bits, I haven't really seen any possibility that the future
> SoCs will have multiple anatop blocks.
> 

Hi Shawn,

I think what the concern is we probably don't want several
non-continuous memory blocks of misc hardwares.
If we look into the current registers in anatop, it is really sparse.
Several regulators are using non-continuous address and the thermals are
also using different addresses. If the addresses are continuous then we
don't need the mfd driver.

We've already told Lily Zhang from Freescale and she promises to report
this problem to some inner team.

Yours Sincerely,
Paul

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Ying-Chun Liu (PaulLiu)]

(2012?05?14? 16:48), Shawn Guo wrote:
> On Mon, May 14, 2012 at 09:08:36AM +0100, Mark Brown wrote:
>> On Mon, May 14, 2012 at 11:51:38AM +0800, Shawn Guo wrote:
>>
>>> From what I see, it's reasonable.  Then the immediate question I have
>>> is, should we remove "struct anatop *adata" from anatop_read_reg and
>>> anatop_write_reg completely?
>>
>> Given the way these things tend to go it's probably guaranteeing that
>> your next round of SoCs will have two register compatible anatop blocks :)
> 
> Considering anatop block tends to be a container of misc hardware
> control bits, I haven't really seen any possibility that the future
> SoCs will have multiple anatop blocks.
> 

Hi Shawn,

I think what the concern is we probably don't want several
non-continuous memory blocks of misc hardwares.
If we look into the current registers in anatop, it is really sparse.
Several regulators are using non-continuous address and the thermals are
also using different addresses. If the addresses are continuous then we
don't need the mfd driver.

We've already told Lily Zhang from Freescale and she promises to report
this problem to some inner team.

Yours Sincerely,
Paul

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Shawn Guo]

On Mon, May 14, 2012 at 05:01:08PM +0800, Ying-Chun Liu (PaulLiu) wrote:
> I think what the concern is we probably don't want several
> non-continuous memory blocks of misc hardwares.
> If we look into the current registers in anatop, it is really sparse.
> Several regulators are using non-continuous address and the thermals are
> also using different addresses. If the addresses are continuous then we
> don't need the mfd driver.
> 
I do not quite follow that.  The reason we need mfd driver isn't because
we do not want to both regulator and thermal drivers to map and access
the same address on their own which may have synchronization issue?

-- 
Regards,
Shawn

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Shawn Guo]

On Mon, May 14, 2012 at 05:01:08PM +0800, Ying-Chun Liu (PaulLiu) wrote:
> I think what the concern is we probably don't want several
> non-continuous memory blocks of misc hardwares.
> If we look into the current registers in anatop, it is really sparse.
> Several regulators are using non-continuous address and the thermals are
> also using different addresses. If the addresses are continuous then we
> don't need the mfd driver.
> 
I do not quite follow that.  The reason we need mfd driver isn't because
we do not want to both regulator and thermal drivers to map and access
the same address on their own which may have synchronization issue?

-- 
Regards,
Shawn

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Ying-Chun Liu (PaulLiu)]

(2012年05月14日 17:43), Shawn Guo wrote:
> On Mon, May 14, 2012 at 05:01:08PM +0800, Ying-Chun Liu (PaulLiu) wrote:
>> I think what the concern is we probably don't want several
>> non-continuous memory blocks of misc hardwares.
>> If we look into the current registers in anatop, it is really sparse.
>> Several regulators are using non-continuous address and the thermals are
>> also using different addresses. If the addresses are continuous then we
>> don't need the mfd driver.
>>
> I do not quite follow that.  The reason we need mfd driver isn't because
> we do not want to both regulator and thermal drivers to map and access
> the same address on their own which may have synchronization issue?
> 

Not sure about the synchronization issue. But currently thermal driver
in Linaro kernel do map and access the same address on its own now. It
is not a device driver yet and just access the address directly and
work. It seems to me that each different type of misc devices in Anatop
just work alone.

So let's go back to the patch. Why do we need this modification? Anatop
thermal driver can be written as a device driver and don't need this
patch. And we might get benefits when thermal driver written in this
way. Especially some boards do not have a correct fuse data. Any real
use cases of this patch?

Yours Sincerely,
Paul

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Ying-Chun Liu (PaulLiu)]

(2012?05?14? 17:43), Shawn Guo wrote:
> On Mon, May 14, 2012 at 05:01:08PM +0800, Ying-Chun Liu (PaulLiu) wrote:
>> I think what the concern is we probably don't want several
>> non-continuous memory blocks of misc hardwares.
>> If we look into the current registers in anatop, it is really sparse.
>> Several regulators are using non-continuous address and the thermals are
>> also using different addresses. If the addresses are continuous then we
>> don't need the mfd driver.
>>
> I do not quite follow that.  The reason we need mfd driver isn't because
> we do not want to both regulator and thermal drivers to map and access
> the same address on their own which may have synchronization issue?
> 

Not sure about the synchronization issue. But currently thermal driver
in Linaro kernel do map and access the same address on its own now. It
is not a device driver yet and just access the address directly and
work. It seems to me that each different type of misc devices in Anatop
just work alone.

So let's go back to the patch. Why do we need this modification? Anatop
thermal driver can be written as a device driver and don't need this
patch. And we might get benefits when thermal driver written in this
way. Especially some boards do not have a correct fuse data. Any real
use cases of this patch?

Yours Sincerely,
Paul

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Richard Zhao]

On Mon, May 14, 2012 at 09:26:57PM +0800, Ying-Chun Liu (PaulLiu) wrote:
> (2012年05月14日 17:43), Shawn Guo wrote:
> > On Mon, May 14, 2012 at 05:01:08PM +0800, Ying-Chun Liu (PaulLiu) wrote:
> >> I think what the concern is we probably don't want several
> >> non-continuous memory blocks of misc hardwares.
> >> If we look into the current registers in anatop, it is really sparse.
> >> Several regulators are using non-continuous address and the thermals are
> >> also using different addresses. If the addresses are continuous then we
> >> don't need the mfd driver.
> >>
> > I do not quite follow that.  The reason we need mfd driver isn't because
> > we do not want to both regulator and thermal drivers to map and access
> > the same address on their own which may have synchronization issue?
> > 
> 
> Not sure about the synchronization issue. But currently thermal driver
> in Linaro kernel do map and access the same address on its own now. It
> is not a device driver yet and just access the address directly and
> work. It seems to me that each different type of misc devices in Anatop
> just work alone.
> 
> So let's go back to the patch. Why do we need this modification? Anatop
> thermal driver can be written as a device driver and don't need this
> patch. And we might get benefits when thermal driver written in this
> way. Especially some boards do not have a correct fuse data. Any real
> use cases of this patch?
Some bits in anatop is not owned by any driver.
It's for below code:
        /* Some phy and power's special controls for host1           
         * 1. The external charger detector needs to be disabled
         * or the signal at DP will be poor                          
         * 2. The PLL's power and output to usb for host 1
         * is totally controlled by IC, so the Software only needs   
         * to enable them at initializtion.
         */ 
        
        anatop_write_reg(NULL, HW_ANADIG_USB2_CHRG_DETECT,           
                        BM_ANADIG_USB2_CHRG_DETECT_EN_B |
                        BM_ANADIG_USB2_CHRG_DETECT_CHK_CHRG_B,       
                        ~0);
        anatop_write_reg(NULL, HW_ANADIG_USB2_PLL_480_CTRL, 0,       
                        BM_ANADIG_USB2_PLL_480_CTRL_BYPASS);

        val = BM_ANADIG_USB2_PLL_480_CTRL_ENABLE |                   
              BM_ANADIG_USB2_PLL_480_CTRL_POWER |
              BM_ANADIG_USB2_PLL_480_CTRL_EN_USB_CLKS;               
        anatop_write_reg(NULL, HW_ANADIG_USB2_PLL_480_CTRL, val, val);

Thanks
Richard
> 
> Yours Sincerely,
> Paul
> 

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Richard Zhao]

On Mon, May 14, 2012 at 09:26:57PM +0800, Ying-Chun Liu (PaulLiu) wrote:
> (2012?05?14? 17:43), Shawn Guo wrote:
> > On Mon, May 14, 2012 at 05:01:08PM +0800, Ying-Chun Liu (PaulLiu) wrote:
> >> I think what the concern is we probably don't want several
> >> non-continuous memory blocks of misc hardwares.
> >> If we look into the current registers in anatop, it is really sparse.
> >> Several regulators are using non-continuous address and the thermals are
> >> also using different addresses. If the addresses are continuous then we
> >> don't need the mfd driver.
> >>
> > I do not quite follow that.  The reason we need mfd driver isn't because
> > we do not want to both regulator and thermal drivers to map and access
> > the same address on their own which may have synchronization issue?
> > 
> 
> Not sure about the synchronization issue. But currently thermal driver
> in Linaro kernel do map and access the same address on its own now. It
> is not a device driver yet and just access the address directly and
> work. It seems to me that each different type of misc devices in Anatop
> just work alone.
> 
> So let's go back to the patch. Why do we need this modification? Anatop
> thermal driver can be written as a device driver and don't need this
> patch. And we might get benefits when thermal driver written in this
> way. Especially some boards do not have a correct fuse data. Any real
> use cases of this patch?
Some bits in anatop is not owned by any driver.
It's for below code:
        /* Some phy and power's special controls for host1           
         * 1. The external charger detector needs to be disabled
         * or the signal at DP will be poor                          
         * 2. The PLL's power and output to usb for host 1
         * is totally controlled by IC, so the Software only needs   
         * to enable them at initializtion.
         */ 
        
        anatop_write_reg(NULL, HW_ANADIG_USB2_CHRG_DETECT,           
                        BM_ANADIG_USB2_CHRG_DETECT_EN_B |
                        BM_ANADIG_USB2_CHRG_DETECT_CHK_CHRG_B,       
                        ~0);
        anatop_write_reg(NULL, HW_ANADIG_USB2_PLL_480_CTRL, 0,       
                        BM_ANADIG_USB2_PLL_480_CTRL_BYPASS);

        val = BM_ANADIG_USB2_PLL_480_CTRL_ENABLE |                   
              BM_ANADIG_USB2_PLL_480_CTRL_POWER |
              BM_ANADIG_USB2_PLL_480_CTRL_EN_USB_CLKS;               
        anatop_write_reg(NULL, HW_ANADIG_USB2_PLL_480_CTRL, val, val);

Thanks
Richard
> 
> Yours Sincerely,
> Paul
> 

Re: [PATCH V2] mfd: anatop: make register accessor more flexible and rename meaningfully

[Samuel Ortiz]

Hi Richard,

On Sun, May 13, 2012 at 09:18:02AM +0800, Richard Zhao wrote:
>  - rename to anatop_read_reg and anatop_write_reg
>  - anatop_read_reg directly return reg value
>  - anatop_write_reg write reg with mask
Patch applied, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[PATCH V2] mfd: anatop: make register accessor more flexible and rename meaningfully

[Samuel Ortiz]

Hi Richard,

On Sun, May 13, 2012 at 09:18:02AM +0800, Richard Zhao wrote:
>  - rename to anatop_read_reg and anatop_write_reg
>  - anatop_read_reg directly return reg value
>  - anatop_write_reg write reg with mask
Patch applied, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Richard Zhao]

On Mon, May 14, 2012 at 09:50:36PM +0800, Richard Zhao wrote:
> On Mon, May 14, 2012 at 09:26:57PM +0800, Ying-Chun Liu (PaulLiu) wrote:
> > (2012年05月14日 17:43), Shawn Guo wrote:
> > > On Mon, May 14, 2012 at 05:01:08PM +0800, Ying-Chun Liu (PaulLiu) wrote:
> > >> I think what the concern is we probably don't want several
> > >> non-continuous memory blocks of misc hardwares.
> > >> If we look into the current registers in anatop, it is really sparse.
> > >> Several regulators are using non-continuous address and the thermals are
> > >> also using different addresses. If the addresses are continuous then we
> > >> don't need the mfd driver.
> > >>
> > > I do not quite follow that.  The reason we need mfd driver isn't because
> > > we do not want to both regulator and thermal drivers to map and access
> > > the same address on their own which may have synchronization issue?
> > > 
> > 
> > Not sure about the synchronization issue. But currently thermal driver
> > in Linaro kernel do map and access the same address on its own now. It
> > is not a device driver yet and just access the address directly and
> > work. It seems to me that each different type of misc devices in Anatop
> > just work alone.
Guys, we need to draw conclusion.
I think current patch keep some how compatible with multi-anatop.
It's ok.

Thanks
Richard
> > 
> > So let's go back to the patch. Why do we need this modification? Anatop
> > thermal driver can be written as a device driver and don't need this
> > patch. And we might get benefits when thermal driver written in this
> > way. Especially some boards do not have a correct fuse data. Any real
> > use cases of this patch?
> Some bits in anatop is not owned by any driver.
> It's for below code:
>         /* Some phy and power's special controls for host1           
>          * 1. The external charger detector needs to be disabled
>          * or the signal at DP will be poor                          
>          * 2. The PLL's power and output to usb for host 1
>          * is totally controlled by IC, so the Software only needs   
>          * to enable them at initializtion.
>          */ 
>         
>         anatop_write_reg(NULL, HW_ANADIG_USB2_CHRG_DETECT,           
>                         BM_ANADIG_USB2_CHRG_DETECT_EN_B |
>                         BM_ANADIG_USB2_CHRG_DETECT_CHK_CHRG_B,       
>                         ~0);
>         anatop_write_reg(NULL, HW_ANADIG_USB2_PLL_480_CTRL, 0,       
>                         BM_ANADIG_USB2_PLL_480_CTRL_BYPASS);
> 
>         val = BM_ANADIG_USB2_PLL_480_CTRL_ENABLE |                   
>               BM_ANADIG_USB2_PLL_480_CTRL_POWER |
>               BM_ANADIG_USB2_PLL_480_CTRL_EN_USB_CLKS;               
>         anatop_write_reg(NULL, HW_ANADIG_USB2_PLL_480_CTRL, val, val);
> 
> Thanks
> Richard
> > 
> > Yours Sincerely,
> > Paul
> > 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Richard Zhao]

On Mon, May 14, 2012 at 09:50:36PM +0800, Richard Zhao wrote:
> On Mon, May 14, 2012 at 09:26:57PM +0800, Ying-Chun Liu (PaulLiu) wrote:
> > (2012?05?14? 17:43), Shawn Guo wrote:
> > > On Mon, May 14, 2012 at 05:01:08PM +0800, Ying-Chun Liu (PaulLiu) wrote:
> > >> I think what the concern is we probably don't want several
> > >> non-continuous memory blocks of misc hardwares.
> > >> If we look into the current registers in anatop, it is really sparse.
> > >> Several regulators are using non-continuous address and the thermals are
> > >> also using different addresses. If the addresses are continuous then we
> > >> don't need the mfd driver.
> > >>
> > > I do not quite follow that.  The reason we need mfd driver isn't because
> > > we do not want to both regulator and thermal drivers to map and access
> > > the same address on their own which may have synchronization issue?
> > > 
> > 
> > Not sure about the synchronization issue. But currently thermal driver
> > in Linaro kernel do map and access the same address on its own now. It
> > is not a device driver yet and just access the address directly and
> > work. It seems to me that each different type of misc devices in Anatop
> > just work alone.
Guys, we need to draw conclusion.
I think current patch keep some how compatible with multi-anatop.
It's ok.

Thanks
Richard
> > 
> > So let's go back to the patch. Why do we need this modification? Anatop
> > thermal driver can be written as a device driver and don't need this
> > patch. And we might get benefits when thermal driver written in this
> > way. Especially some boards do not have a correct fuse data. Any real
> > use cases of this patch?
> Some bits in anatop is not owned by any driver.
> It's for below code:
>         /* Some phy and power's special controls for host1           
>          * 1. The external charger detector needs to be disabled
>          * or the signal at DP will be poor                          
>          * 2. The PLL's power and output to usb for host 1
>          * is totally controlled by IC, so the Software only needs   
>          * to enable them at initializtion.
>          */ 
>         
>         anatop_write_reg(NULL, HW_ANADIG_USB2_CHRG_DETECT,           
>                         BM_ANADIG_USB2_CHRG_DETECT_EN_B |
>                         BM_ANADIG_USB2_CHRG_DETECT_CHK_CHRG_B,       
>                         ~0);
>         anatop_write_reg(NULL, HW_ANADIG_USB2_PLL_480_CTRL, 0,       
>                         BM_ANADIG_USB2_PLL_480_CTRL_BYPASS);
> 
>         val = BM_ANADIG_USB2_PLL_480_CTRL_ENABLE |                   
>               BM_ANADIG_USB2_PLL_480_CTRL_POWER |
>               BM_ANADIG_USB2_PLL_480_CTRL_EN_USB_CLKS;               
>         anatop_write_reg(NULL, HW_ANADIG_USB2_PLL_480_CTRL, val, val);
> 
> Thanks
> Richard
> > 
> > Yours Sincerely,
> > Paul
> > 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Shawn Guo]

On Fri, May 18, 2012 at 05:59:32PM +0800, Richard Zhao wrote:
> Guys, we need to draw conclusion.
> I think current patch keep some how compatible with multi-anatop.
> It's ok.
> 
I'm fine with that.

-- 
Regards,
Shawn

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Shawn Guo]

On Fri, May 18, 2012 at 05:59:32PM +0800, Richard Zhao wrote:
> Guys, we need to draw conclusion.
> I think current patch keep some how compatible with multi-anatop.
> It's ok.
> 
I'm fine with that.

-- 
Regards,
Shawn

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Ying-Chun Liu (PaulLiu)]

(2012年05月21日 12:13), Shawn Guo wrote:
> On Fri, May 18, 2012 at 05:59:32PM +0800, Richard Zhao wrote:
>> Guys, we need to draw conclusion.
>> I think current patch keep some how compatible with multi-anatop.
>> It's ok.
>>
> I'm fine with that.
> 
Yes. It is good for me.

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Ying-Chun Liu (PaulLiu)]

(2012?05?21? 12:13), Shawn Guo wrote:
> On Fri, May 18, 2012 at 05:59:32PM +0800, Richard Zhao wrote:
>> Guys, we need to draw conclusion.
>> I think current patch keep some how compatible with multi-anatop.
>> It's ok.
>>
> I'm fine with that.
> 
Yes. It is good for me.

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Richard Zhao]

On Mon, May 21, 2012 at 05:27:54PM +0800, Ying-Chun Liu (PaulLiu) wrote:
> (2012年05月21日 12:13), Shawn Guo wrote:
> > On Fri, May 18, 2012 at 05:59:32PM +0800, Richard Zhao wrote:
> >> Guys, we need to draw conclusion.
> >> I think current patch keep some how compatible with multi-anatop.
> >> It's ok.
> >>
> > I'm fine with that.
> > 
> Yes. It is good for me.
> 
So, Sameo, could you pick the patch.

Thanks
Richard

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Richard Zhao]

On Mon, May 21, 2012 at 05:27:54PM +0800, Ying-Chun Liu (PaulLiu) wrote:
> (2012?05?21? 12:13), Shawn Guo wrote:
> > On Fri, May 18, 2012 at 05:59:32PM +0800, Richard Zhao wrote:
> >> Guys, we need to draw conclusion.
> >> I think current patch keep some how compatible with multi-anatop.
> >> It's ok.
> >>
> > I'm fine with that.
> > 
> Yes. It is good for me.
> 
So, Sameo, could you pick the patch.

Thanks
Richard

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Rob Lee]

Hi,

What is the status of this patch?

Thanks,
Rob

On Mon, May 21, 2012 at 4:39 AM, Richard Zhao
<richard.zhao@freescale.com> wrote:
> On Mon, May 21, 2012 at 05:27:54PM +0800, Ying-Chun Liu (PaulLiu) wrote:
>> (2012年05月21日 12:13), Shawn Guo wrote:
>> > On Fri, May 18, 2012 at 05:59:32PM +0800, Richard Zhao wrote:
>> >> Guys, we need to draw conclusion.
>> >> I think current patch keep some how compatible with multi-anatop.
>> >> It's ok.
>> >>
>> > I'm fine with that.
>> >
>> Yes. It is good for me.
>>
> So, Sameo, could you pick the patch.
>
> Thanks
> Richard
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Rob Lee]

Hi,

What is the status of this patch?

Thanks,
Rob

On Mon, May 21, 2012 at 4:39 AM, Richard Zhao
<richard.zhao@freescale.com> wrote:
> On Mon, May 21, 2012 at 05:27:54PM +0800, Ying-Chun Liu (PaulLiu) wrote:
>> (2012?05?21? 12:13), Shawn Guo wrote:
>> > On Fri, May 18, 2012 at 05:59:32PM +0800, Richard Zhao wrote:
>> >> Guys, we need to draw conclusion.
>> >> I think current patch keep some how compatible with multi-anatop.
>> >> It's ok.
>> >>
>> > I'm fine with that.
>> >
>> Yes. It is good for me.
>>
> So, Sameo, could you pick the patch.
>
> Thanks
> Richard
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Samuel Ortiz]

Hi Rob,

On Mon, Jun 18, 2012 at 03:23:45PM -0500, Rob Lee wrote:
> Hi,
> 
> What is the status of this patch?
I'm swamped with other things to look at right now, but I expect to clean my
MFD patches backlog at the beginning of next week.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Samuel Ortiz]

Hi Rob,

On Mon, Jun 18, 2012 at 03:23:45PM -0500, Rob Lee wrote:
> Hi,
> 
> What is the status of this patch?
I'm swamped with other things to look at right now, but I expect to clean my
MFD patches backlog at the beginning of next week.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Rob Lee]

Hey Samuel, thanks for your response.

On Mon, Jun 18, 2012 at 4:58 PM, Samuel Ortiz <sameo@linux.intel.com> wrote:
> Hi Rob,
>
> On Mon, Jun 18, 2012 at 03:23:45PM -0500, Rob Lee wrote:
>> Hi,
>>
>> What is the status of this patch?
> I'm swamped with other things to look at right now, but I expect to clean my
> MFD patches backlog at the beginning of next week.

Fyi, I also saw this patch was resent (and acked) here:
https://lkml.org/lkml/2012/6/3/159

Thanks,
Rob

>
> Cheers,
> Samuel.
>
> --
> Intel Open Source Technology Centre
> http://oss.intel.com/

[PATCH 2/2] mfd: anatop: permit adata be NULL when access register

[Rob Lee]

Hey Samuel, thanks for your response.

On Mon, Jun 18, 2012 at 4:58 PM, Samuel Ortiz <sameo@linux.intel.com> wrote:
> Hi Rob,
>
> On Mon, Jun 18, 2012 at 03:23:45PM -0500, Rob Lee wrote:
>> Hi,
>>
>> What is the status of this patch?
> I'm swamped with other things to look at right now, but I expect to clean my
> MFD patches backlog at the beginning of next week.

Fyi, I also saw this patch was resent (and acked) here:
https://lkml.org/lkml/2012/6/3/159

Thanks,
Rob

>
> Cheers,
> Samuel.
>
> --
> Intel Open Source Technology Centre
> http://oss.intel.com/