This series enables support for 7 additional regulators on the BCM59056
PMU. These regulators are accessed via a secondary I2C slave address.
The MFD driver exposes an additional regmap descriptor for the additional
address space and the regulator implements support for GPLDO1-6 and VBUS
regulators.
Matt Porter (4):
mfd: bcm590xx: update binding with additional BCM59056 regulators
mfd: bcm590xx: add support for second i2c slave address space
regulator: bcm590xx: add support for regulators on secondary i2c slave
ARM: dts: bcm590xx: add support for GPLDO and VBUS regulators
Documentation/devicetree/bindings/mfd/bcm590xx.txt | 4 +-
arch/arm/boot/dts/bcm59056.dtsi | 21 +++++
drivers/mfd/bcm590xx.c | 60 ++++++++++----
drivers/regulator/bcm590xx-regulator.c | 92 +++++++++++++++++++---
include/linux/mfd/bcm590xx.h | 9 ++-
5 files changed, 158 insertions(+), 28 deletions(-)
--
1.8.4
The bcm590xx mfd driver now exposes a second regmap descriptor making
the registers for regulators on the secondary i2c slave address
available. Add support for GPLDO1-6 and VBUS regulators found within
this register range.
Signed-off-by: Matt Porter <[email protected]>
---
drivers/regulator/bcm590xx-regulator.c | 92 ++++++++++++++++++++++++++++++----
1 file changed, 82 insertions(+), 10 deletions(-)
diff --git a/drivers/regulator/bcm590xx-regulator.c b/drivers/regulator/bcm590xx-regulator.c
index c3750c5..4f0b5d9 100644
--- a/drivers/regulator/bcm590xx-regulator.c
+++ b/drivers/regulator/bcm590xx-regulator.c
@@ -22,7 +22,7 @@
#include <linux/regulator/of_regulator.h>
#include <linux/slab.h>
-/* Register defs */
+/* I2C slave 0 registers */
#define BCM590XX_RFLDOPMCTRL1 0x60
#define BCM590XX_IOSR1PMCTRL1 0x7a
#define BCM590XX_IOSR2PMCTRL1 0x7c
@@ -31,13 +31,34 @@
#define BCM590XX_SDSR2PMCTRL1 0x86
#define BCM590XX_MSRPMCTRL1 0x8a
#define BCM590XX_VSRPMCTRL1 0x8e
-#define BCM590XX_REG_ENABLE BIT(7)
-
#define BCM590XX_RFLDOCTRL 0x96
#define BCM590XX_CSRVOUT1 0xc0
+
+/* I2C slave 1 registers */
+#define BCM590XX_GPLDO5PMCTRL1 0x16
+#define BCM590XX_GPLDO6PMCTRL1 0x18
+#define BCM590XX_GPLDO1CTRL 0x1a
+#define BCM590XX_GPLDO2CTRL 0x1b
+#define BCM590XX_GPLDO3CTRL 0x1c
+#define BCM590XX_GPLDO4CTRL 0x1d
+#define BCM590XX_GPLDO5CTRL 0x1e
+#define BCM590XX_GPLDO6CTRL 0x1f
+#define BCM590XX_OTG_CTRL 0x40
+#define BCM590XX_GPLDO1PMCTRL1 0x57
+#define BCM590XX_GPLDO2PMCTRL1 0x59
+#define BCM590XX_GPLDO3PMCTRL1 0x5b
+#define BCM590XX_GPLDO4PMCTRL1 0x5d
+
+#define BCM590XX_REG_ENABLE BIT(7)
+#define BCM590XX_VBUS_ENABLE BIT(2)
#define BCM590XX_LDO_VSEL_MASK GENMASK(5, 3)
#define BCM590XX_SR_VSEL_MASK GENMASK(5, 0)
+/*
+ * RFLDO to VSR regulators are
+ * accessed via I2C slave 0
+ */
+
/* LDO regulator IDs */
#define BCM590XX_REG_RFLDO 0
#define BCM590XX_REG_CAMLDO1 1
@@ -62,9 +83,25 @@
#define BCM590XX_REG_SDSR2 18
#define BCM590XX_REG_VSR 19
-#define BCM590XX_NUM_REGS 20
+/*
+ * GPLDO1 to VBUS regulators are
+ * accessed via I2C slave 1
+ */
+
+#define BCM590XX_REG_GPLDO1 20
+#define BCM590XX_REG_GPLDO2 21
+#define BCM590XX_REG_GPLDO3 22
+#define BCM590XX_REG_GPLDO4 23
+#define BCM590XX_REG_GPLDO5 24
+#define BCM590XX_REG_GPLDO6 25
+#define BCM590XX_REG_VBUS 26
+
+#define BCM590XX_NUM_REGS 27
#define BCM590XX_REG_IS_LDO(n) (n < BCM590XX_REG_CSR)
+#define BCM590XX_REG_IS_GPLDO(n) \
+ ((n > BCM590XX_REG_VSR) && (n < BCM590XX_REG_VBUS))
+#define BCM590XX_REG_IS_VBUS(n) (n == BCM590XX_REG_VBUS)
struct bcm590xx_board {
struct regulator_init_data *bcm590xx_pmu_init_data[BCM590XX_NUM_REGS];
@@ -149,6 +186,12 @@ static struct bcm590xx_info bcm590xx_regs[] = {
BCM590XX_REG_RANGES(sdsr1, dcdc_sdsr1_ranges),
BCM590XX_REG_RANGES(sdsr2, dcdc_iosr1_ranges),
BCM590XX_REG_RANGES(vsr, dcdc_iosr1_ranges),
+ BCM590XX_REG_TABLE(gpldo1, ldo_a_table),
+ BCM590XX_REG_TABLE(gpldo2, ldo_a_table),
+ BCM590XX_REG_TABLE(gpldo3, ldo_a_table),
+ BCM590XX_REG_TABLE(gpldo4, ldo_a_table),
+ BCM590XX_REG_TABLE(gpldo5, ldo_a_table),
+ BCM590XX_REG_TABLE(gpldo6, ldo_a_table),
};
struct bcm590xx_reg {
@@ -161,6 +204,8 @@ static int bcm590xx_get_vsel_register(int id)
{
if (BCM590XX_REG_IS_LDO(id))
return BCM590XX_RFLDOCTRL + id;
+ else if (BCM590XX_REG_IS_GPLDO(id))
+ return BCM590XX_GPLDO1CTRL + id;
else
return BCM590XX_CSRVOUT1 + (id - BCM590XX_REG_CSR) * 3;
}
@@ -171,6 +216,8 @@ static int bcm590xx_get_enable_register(int id)
if (BCM590XX_REG_IS_LDO(id))
reg = BCM590XX_RFLDOPMCTRL1 + id * 2;
+ else if (BCM590XX_REG_IS_GPLDO(id))
+ reg = BCM590XX_GPLDO1PMCTRL1 + id * 2;
else
switch (id) {
case BCM590XX_REG_CSR:
@@ -191,8 +238,11 @@ static int bcm590xx_get_enable_register(int id)
case BCM590XX_REG_SDSR2:
reg = BCM590XX_SDSR2PMCTRL1;
break;
+ case BCM590XX_REG_VBUS:
+ reg = BCM590XX_OTG_CTRL;
};
+
return reg;
}
@@ -216,6 +266,12 @@ static struct regulator_ops bcm590xx_ops_dcdc = {
.map_voltage = regulator_map_voltage_linear_range,
};
+static struct regulator_ops bcm590xx_ops_vbus = {
+ .is_enabled = regulator_is_enabled_regmap,
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+};
+
#define BCM590XX_MATCH(_name, _id) \
{ \
.name = #_name, \
@@ -243,6 +299,13 @@ static struct of_regulator_match bcm590xx_matches[] = {
BCM590XX_MATCH(sdsr1, SDSR1),
BCM590XX_MATCH(sdsr2, SDSR2),
BCM590XX_MATCH(vsr, VSR),
+ BCM590XX_MATCH(gpldo1, GPLDO1),
+ BCM590XX_MATCH(gpldo2, GPLDO2),
+ BCM590XX_MATCH(gpldo3, GPLDO3),
+ BCM590XX_MATCH(gpldo4, GPLDO4),
+ BCM590XX_MATCH(gpldo5, GPLDO5),
+ BCM590XX_MATCH(gpldo6, GPLDO6),
+ BCM590XX_MATCH(vbus, VBUS),
};
static struct bcm590xx_board *bcm590xx_parse_dt_reg_data(
@@ -353,17 +416,23 @@ static int bcm590xx_probe(struct platform_device *pdev)
pmu->desc[i].linear_ranges = info->linear_ranges;
pmu->desc[i].n_linear_ranges = info->n_linear_ranges;
- if (BCM590XX_REG_IS_LDO(i)) {
+ if ((BCM590XX_REG_IS_LDO(i)) || (BCM590XX_REG_IS_GPLDO(i))) {
pmu->desc[i].ops = &bcm590xx_ops_ldo;
pmu->desc[i].vsel_mask = BCM590XX_LDO_VSEL_MASK;
- } else {
+ } else if (BCM590XX_REG_IS_VBUS(i))
+ pmu->desc[i].ops = &bcm590xx_ops_vbus;
+ else {
pmu->desc[i].ops = &bcm590xx_ops_dcdc;
pmu->desc[i].vsel_mask = BCM590XX_SR_VSEL_MASK;
}
- pmu->desc[i].vsel_reg = bcm590xx_get_vsel_register(i);
- pmu->desc[i].enable_is_inverted = true;
- pmu->desc[i].enable_mask = BCM590XX_REG_ENABLE;
+ if (BCM590XX_REG_IS_VBUS(i))
+ pmu->desc[i].enable_mask = BCM590XX_VBUS_ENABLE;
+ else {
+ pmu->desc[i].vsel_reg = bcm590xx_get_vsel_register(i);
+ pmu->desc[i].enable_is_inverted = true;
+ pmu->desc[i].enable_mask = BCM590XX_REG_ENABLE;
+ }
pmu->desc[i].enable_reg = bcm590xx_get_enable_register(i);
pmu->desc[i].type = REGULATOR_VOLTAGE;
pmu->desc[i].owner = THIS_MODULE;
@@ -371,7 +440,10 @@ static int bcm590xx_probe(struct platform_device *pdev)
config.dev = bcm590xx->dev;
config.init_data = reg_data;
config.driver_data = pmu;
- config.regmap = bcm590xx->regmap;
+ if (BCM590XX_REG_IS_GPLDO(i) || BCM590XX_REG_IS_VBUS(i))
+ config.regmap = bcm590xx->regmap1;
+ else
+ config.regmap = bcm590xx->regmap0;
if (bcm590xx_reg_matches)
config.of_node = bcm590xx_reg_matches[i].of_node;
--
1.8.4
The BCM59056 supports GPLDO1-6 and VBUS regulators in a secondary
I2C slave address space. Add these regulators to the list of valid
regulator node names for BCM59056.
Signed-off-by: Matt Porter <[email protected]>
---
Documentation/devicetree/bindings/mfd/bcm590xx.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mfd/bcm590xx.txt b/Documentation/devicetree/bindings/mfd/bcm590xx.txt
index 1fe30e2..be51a15 100644
--- a/Documentation/devicetree/bindings/mfd/bcm590xx.txt
+++ b/Documentation/devicetree/bindings/mfd/bcm590xx.txt
@@ -19,7 +19,9 @@ Optional child nodes:
The valid regulator node names for BCM59056 are:
rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
- csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr
+ csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr,
+ gpldo1, gpldo2, gpldo3, gpldo4, gpldo5, gpldo6,
+ vbus
Example:
pmu: bcm59056@8 {
--
1.8.4
Adds additional nodes to support GPLDO1-6 and VBUS regulators which
are not supported in the bcm590xx regulator driver.
Signed-off-by: Matt Porter <[email protected]>
---
arch/arm/boot/dts/bcm59056.dtsi | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/arch/arm/boot/dts/bcm59056.dtsi b/arch/arm/boot/dts/bcm59056.dtsi
index dfadaaa..066adfb 100644
--- a/arch/arm/boot/dts/bcm59056.dtsi
+++ b/arch/arm/boot/dts/bcm59056.dtsi
@@ -70,5 +70,26 @@
vsr_reg: vsr {
};
+
+ gpldo1_reg: gpldo1 {
+ };
+
+ gpldo2_reg: gpldo2 {
+ };
+
+ gpldo3_reg: gpldo3 {
+ };
+
+ gpldo4_reg: gpldo4 {
+ };
+
+ gpldo5_reg: gpldo5 {
+ };
+
+ gpldo6_reg: gpldo6 {
+ };
+
+ vbus_reg: vbus {
+ };
};
};
--
1.8.4
BCM590xx utilizes a second i2c slave address to access additional
register space. Add support for the second address space by
instantiated a dummy i2c device with the appropriate secondary
i2c slave address. Expose a second regmap register space so that
mfd drivers can access this secondary i2c slave address space.
Signed-off-by: Matt Porter <[email protected]>
---
drivers/mfd/bcm590xx.c | 60 +++++++++++++++++++++++++++++++++-----------
include/linux/mfd/bcm590xx.h | 9 ++++---
2 files changed, 52 insertions(+), 17 deletions(-)
diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c
index e9a33c7..b710ffa 100644
--- a/drivers/mfd/bcm590xx.c
+++ b/drivers/mfd/bcm590xx.c
@@ -28,39 +28,71 @@ static const struct mfd_cell bcm590xx_devs[] = {
},
};
-static const struct regmap_config bcm590xx_regmap_config = {
+static const struct regmap_config bcm590xx_regmap_config_0 = {
.reg_bits = 8,
.val_bits = 8,
- .max_register = BCM590XX_MAX_REGISTER,
+ .max_register = BCM590XX_MAX_REGISTER_0,
.cache_type = REGCACHE_RBTREE,
};
-static int bcm590xx_i2c_probe(struct i2c_client *i2c,
+static const struct regmap_config bcm590xx_regmap_config_1 = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = BCM590XX_MAX_REGISTER_1,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static int bcm590xx_i2c_probe(struct i2c_client *addmap0,
const struct i2c_device_id *id)
{
struct bcm590xx *bcm590xx;
int ret;
- bcm590xx = devm_kzalloc(&i2c->dev, sizeof(*bcm590xx), GFP_KERNEL);
+ bcm590xx = devm_kzalloc(&addmap0->dev, sizeof(*bcm590xx), GFP_KERNEL);
if (!bcm590xx)
return -ENOMEM;
- i2c_set_clientdata(i2c, bcm590xx);
- bcm590xx->dev = &i2c->dev;
- bcm590xx->i2c_client = i2c;
+ i2c_set_clientdata(addmap0, bcm590xx);
+ bcm590xx->dev = &addmap0->dev;
+ bcm590xx->addmap0 = addmap0;
- bcm590xx->regmap = devm_regmap_init_i2c(i2c, &bcm590xx_regmap_config);
- if (IS_ERR(bcm590xx->regmap)) {
- ret = PTR_ERR(bcm590xx->regmap);
- dev_err(&i2c->dev, "regmap initialization failed: %d\n", ret);
+ bcm590xx->regmap0 = devm_regmap_init_i2c(addmap0,
+ &bcm590xx_regmap_config_0);
+ if (IS_ERR(bcm590xx->regmap0)) {
+ ret = PTR_ERR(bcm590xx->regmap0);
+ dev_err(&addmap0->dev, "regmap 0 init failed: %d\n", ret);
return ret;
}
- ret = mfd_add_devices(&i2c->dev, -1, bcm590xx_devs,
+ /* Second I2C slave address is the base address with A(2) asserted */
+ bcm590xx->addmap1 = i2c_new_dummy(addmap0->adapter,
+ addmap0->addr | BIT(2));
+ if (IS_ERR_OR_NULL(bcm590xx->addmap1)) {
+ dev_err(&addmap0->dev, "failed to add address map 1 device\n");
+ return -ENODEV;
+ }
+ i2c_set_clientdata(bcm590xx->addmap1, bcm590xx);
+
+ bcm590xx->regmap1 = devm_regmap_init_i2c(bcm590xx->addmap1,
+ &bcm590xx_regmap_config_1);
+ if (IS_ERR(bcm590xx->regmap1)) {
+ ret = PTR_ERR(bcm590xx->regmap1);
+ dev_err(&bcm590xx->addmap1->dev,
+ "regmap 1 init failed: %d\n", ret);
+ goto err;
+ }
+
+ ret = mfd_add_devices(&addmap0->dev, -1, bcm590xx_devs,
ARRAY_SIZE(bcm590xx_devs), NULL, 0, NULL);
- if (ret < 0)
- dev_err(&i2c->dev, "failed to add sub-devices: %d\n", ret);
+ if (ret < 0) {
+ dev_err(&addmap0->dev, "failed to add sub-devices: %d\n", ret);
+ goto err;
+ }
+
+ return 0;
+err:
+ i2c_unregister_device(bcm590xx->addmap1);
return ret;
}
diff --git a/include/linux/mfd/bcm590xx.h b/include/linux/mfd/bcm590xx.h
index 434df2d..a2723f2 100644
--- a/include/linux/mfd/bcm590xx.h
+++ b/include/linux/mfd/bcm590xx.h
@@ -19,12 +19,15 @@
#include <linux/regmap.h>
/* max register address */
-#define BCM590XX_MAX_REGISTER 0xe7
+#define BCM590XX_MAX_REGISTER_0 0xe7
+#define BCM590XX_MAX_REGISTER_1 0xf0
struct bcm590xx {
struct device *dev;
- struct i2c_client *i2c_client;
- struct regmap *regmap;
+ struct i2c_client *addmap0;
+ struct i2c_client *addmap1;
+ struct regmap *regmap0;
+ struct regmap *regmap1;
unsigned int id;
};
--
1.8.4
On Mon, Apr 14, 2014 at 02:50:27PM -0400, Matt Porter wrote:
> The bcm590xx mfd driver now exposes a second regmap descriptor making
> the registers for regulators on the secondary i2c slave address
> available. Add support for GPLDO1-6 and VBUS regulators found within
> this register range.
>
> Signed-off-by: Matt Porter <[email protected]>
Acked-by: Mark Brown <[email protected]>
though if possible I'd prefer to merge this into the regulator tree
which would need a cross tree merge with MFD.
On Mon, 14 Apr 2014, Matt Porter wrote:
> BCM590xx utilizes a second i2c slave address to access additional
s/i2c/I2C
> register space. Add support for the second address space by
> instantiated a dummy i2c device with the appropriate secondary
s/instantiated/instantiating
> i2c slave address. Expose a second regmap register space so that
s/i2c/I2C
Exposing?
s/regmap/Regmap
> mfd drivers can access this secondary i2c slave address space.
s/mfd/MFD
s/i2c/I2C
> Signed-off-by: Matt Porter <[email protected]>
> ---
> drivers/mfd/bcm590xx.c | 60 +++++++++++++++++++++++++++++++++-----------
> include/linux/mfd/bcm590xx.h | 9 ++++---
> 2 files changed, 52 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c
> index e9a33c7..b710ffa 100644
> --- a/drivers/mfd/bcm590xx.c
> +++ b/drivers/mfd/bcm590xx.c
> @@ -28,39 +28,71 @@ static const struct mfd_cell bcm590xx_devs[] = {
> },
> };
>
> -static const struct regmap_config bcm590xx_regmap_config = {
> +static const struct regmap_config bcm590xx_regmap_config_0 = {
Not loving _0 and _1 appendages.
Is one of them {primary|master} and the other {secondary|slave}?
> .reg_bits = 8,
> .val_bits = 8,
> - .max_register = BCM590XX_MAX_REGISTER,
> + .max_register = BCM590XX_MAX_REGISTER_0,
> .cache_type = REGCACHE_RBTREE,
> };
>
> -static int bcm590xx_i2c_probe(struct i2c_client *i2c,
> +static const struct regmap_config bcm590xx_regmap_config_1 = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = BCM590XX_MAX_REGISTER_1,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int bcm590xx_i2c_probe(struct i2c_client *addmap0,
Would this be best left as i2c, then naming the other one
i2c_secondary for instance?
addmap{0,1} doesn't quite sit right with me.
REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
as I first thought, but still, is there a better naming convention you
could use?
> const struct i2c_device_id *id)
> {
> struct bcm590xx *bcm590xx;
> int ret;
>
> - bcm590xx = devm_kzalloc(&i2c->dev, sizeof(*bcm590xx), GFP_KERNEL);
> + bcm590xx = devm_kzalloc(&addmap0->dev, sizeof(*bcm590xx), GFP_KERNEL);
> if (!bcm590xx)
> return -ENOMEM;
>
> - i2c_set_clientdata(i2c, bcm590xx);
> - bcm590xx->dev = &i2c->dev;
> - bcm590xx->i2c_client = i2c;
> + i2c_set_clientdata(addmap0, bcm590xx);
> + bcm590xx->dev = &addmap0->dev;
> + bcm590xx->addmap0 = addmap0;
>
> - bcm590xx->regmap = devm_regmap_init_i2c(i2c, &bcm590xx_regmap_config);
> - if (IS_ERR(bcm590xx->regmap)) {
> - ret = PTR_ERR(bcm590xx->regmap);
> - dev_err(&i2c->dev, "regmap initialization failed: %d\n", ret);
> + bcm590xx->regmap0 = devm_regmap_init_i2c(addmap0,
> + &bcm590xx_regmap_config_0);
> + if (IS_ERR(bcm590xx->regmap0)) {
> + ret = PTR_ERR(bcm590xx->regmap0);
> + dev_err(&addmap0->dev, "regmap 0 init failed: %d\n", ret);
> return ret;
> }
>
> - ret = mfd_add_devices(&i2c->dev, -1, bcm590xx_devs,
> + /* Second I2C slave address is the base address with A(2) asserted */
> + bcm590xx->addmap1 = i2c_new_dummy(addmap0->adapter,
> + addmap0->addr | BIT(2));
> + if (IS_ERR_OR_NULL(bcm590xx->addmap1)) {
> + dev_err(&addmap0->dev, "failed to add address map 1 device\n");
> + return -ENODEV;
> + }
> + i2c_set_clientdata(bcm590xx->addmap1, bcm590xx);
> +
> + bcm590xx->regmap1 = devm_regmap_init_i2c(bcm590xx->addmap1,
> + &bcm590xx_regmap_config_1);
> + if (IS_ERR(bcm590xx->regmap1)) {
> + ret = PTR_ERR(bcm590xx->regmap1);
> + dev_err(&bcm590xx->addmap1->dev,
> + "regmap 1 init failed: %d\n", ret);
> + goto err;
> + }
> +
> + ret = mfd_add_devices(&addmap0->dev, -1, bcm590xx_devs,
> ARRAY_SIZE(bcm590xx_devs), NULL, 0, NULL);
> - if (ret < 0)
> - dev_err(&i2c->dev, "failed to add sub-devices: %d\n", ret);
> + if (ret < 0) {
> + dev_err(&addmap0->dev, "failed to add sub-devices: %d\n", ret);
> + goto err;
> + }
> +
> + return 0;
>
> +err:
> + i2c_unregister_device(bcm590xx->addmap1);
> return ret;
> }
>
> diff --git a/include/linux/mfd/bcm590xx.h b/include/linux/mfd/bcm590xx.h
> index 434df2d..a2723f2 100644
> --- a/include/linux/mfd/bcm590xx.h
> +++ b/include/linux/mfd/bcm590xx.h
> @@ -19,12 +19,15 @@
> #include <linux/regmap.h>
>
> /* max register address */
> -#define BCM590XX_MAX_REGISTER 0xe7
> +#define BCM590XX_MAX_REGISTER_0 0xe7
> +#define BCM590XX_MAX_REGISTER_1 0xf0
>
> struct bcm590xx {
> struct device *dev;
> - struct i2c_client *i2c_client;
> - struct regmap *regmap;
> + struct i2c_client *addmap0;
> + struct i2c_client *addmap1;
> + struct regmap *regmap0;
> + struct regmap *regmap1;
> unsigned int id;
> };
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Wed, Apr 16, 2014 at 12:06:03PM +0100, Lee Jones wrote:
> s/regmap/Regmap
It's consistently written regmap in all the documentation and so on :)
> addmap{0,1} doesn't quite sit right with me.
> REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> as I first thought, but still, is there a better naming convention you
> could use?
addrmap or something?
> > s/regmap/Regmap
>
> It's consistently written regmap in all the documentation and so on :)
Furry muff; but the comments still stand for the acronyms.
> > addmap{0,1} doesn't quite sit right with me.
>
> > REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> > as I first thought, but still, is there a better naming convention you
> > could use?
>
> addrmap or something?
Right, that was what I was thinking. However, I prefer something along
the lines of 'i2c' and 'i2c_sec' or 'client' and 'client_slv' etc.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Thu, Apr 17, 2014 at 07:57:53AM +0100, Lee Jones wrote:
> > > s/regmap/Regmap
> >
> > It's consistently written regmap in all the documentation and so on :)
>
> Furry muff; but the comments still stand for the acronyms.
>
> > > addmap{0,1} doesn't quite sit right with me.
> >
> > > REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> > > as I first thought, but still, is there a better naming convention you
> > > could use?
> >
> > addrmap or something?
>
> Right, that was what I was thinking. However, I prefer something along
> the lines of 'i2c' and 'i2c_sec' or 'client' and 'client_slv' etc.
FWIW, the reason it's addmap{0,1} is that the datasheet has documents
ADDMAP=0 and the first bank of registers and ADDMAP=1 as the second bank
of registers. I adopted that to match the docs for the part.
I guess we could do i2c and i2c_sec, I'll just have to put a comment
correlating it to the h/w. Calling it 'slv' implies something else
so we should avoid that here. The notion of a "secondary" i2c device
is completely a Linux I2C subsystem fabrication which wouldn't exist
if it allowed multiple slave addresses per device. From a h/w
perspective there is really no primary and secondary relationship.
I'm fine with i2c/i2c_sec or addrmap0/1 and I will just comment to
correlate with the datasheet..pick one.
-Matt
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
On Wed, Apr 16, 2014 at 12:06:03PM +0100, Lee Jones wrote:
> On Mon, 14 Apr 2014, Matt Porter wrote:
>
> > BCM590xx utilizes a second i2c slave address to access additional
>
> s/i2c/I2C
>
> > register space. Add support for the second address space by
> > instantiated a dummy i2c device with the appropriate secondary
>
> s/instantiated/instantiating
>
> > i2c slave address. Expose a second regmap register space so that
>
> s/i2c/I2C
>
> Exposing?
>
> s/regmap/Regmap
>
> > mfd drivers can access this secondary i2c slave address space.
>
> s/mfd/MFD
>
> s/i2c/I2C
Ok, I'll fix the capitalization and wording..except for regmap as
noted by Mark.
>
> > Signed-off-by: Matt Porter <[email protected]>
> > ---
> > drivers/mfd/bcm590xx.c | 60 +++++++++++++++++++++++++++++++++-----------
> > include/linux/mfd/bcm590xx.h | 9 ++++---
> > 2 files changed, 52 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c
> > index e9a33c7..b710ffa 100644
> > --- a/drivers/mfd/bcm590xx.c
> > +++ b/drivers/mfd/bcm590xx.c
> > @@ -28,39 +28,71 @@ static const struct mfd_cell bcm590xx_devs[] = {
> > },
> > };
> >
> > -static const struct regmap_config bcm590xx_regmap_config = {
> > +static const struct regmap_config bcm590xx_regmap_config_0 = {
>
> Not loving _0 and _1 appendages.
>
> Is one of them {primary|master} and the other {secondary|slave}?
I guess from a Linux I2C subsystem, we can view _1 as the
"secondary"...it does correspond the the i2c_new_dummy() device
that we create in the mfd probe. That device corresponds to the
ADDMAP=1 address on the PMU. This is why I used those appendages.
-Matt
> > > > s/regmap/Regmap
> > >
> > > It's consistently written regmap in all the documentation and so on :)
> >
> > Furry muff; but the comments still stand for the acronyms.
> >
> > > > addmap{0,1} doesn't quite sit right with me.
> > >
> > > > REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> > > > as I first thought, but still, is there a better naming convention you
> > > > could use?
> > >
> > > addrmap or something?
> >
> > Right, that was what I was thinking. However, I prefer something along
> > the lines of 'i2c' and 'i2c_sec' or 'client' and 'client_slv' etc.
>
> FWIW, the reason it's addmap{0,1} is that the datasheet has documents
> ADDMAP=0 and the first bank of registers and ADDMAP=1 as the second bank
> of registers. I adopted that to match the docs for the part.
>
> I guess we could do i2c and i2c_sec, I'll just have to put a comment
> correlating it to the h/w. Calling it 'slv' implies something else
> so we should avoid that here. The notion of a "secondary" i2c device
> is completely a Linux I2C subsystem fabrication which wouldn't exist
> if it allowed multiple slave addresses per device. From a h/w
> perspective there is really no primary and secondary relationship.
>
> I'm fine with i2c/i2c_sec or addrmap0/1 and I will just comment to
> correlate with the datasheet..pick one.
Let's stick method fabricated by the I2C subsystem. It may seem strange
from a h/w perspective, but it is the way we (you) have coded it, as
the first parameter of i2c_new_dummy() is the 'managing' (primary,
parent, master, whatever) device, so '_sec' would suit as an
identifying appendage for the resultant device.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Tue, Apr 22, 2014 at 09:21:39AM +0100, Lee Jones wrote:
> > > > > s/regmap/Regmap
> > > >
> > > > It's consistently written regmap in all the documentation and so on :)
> > >
> > > Furry muff; but the comments still stand for the acronyms.
> > >
> > > > > addmap{0,1} doesn't quite sit right with me.
> > > >
> > > > > REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> > > > > as I first thought, but still, is there a better naming convention you
> > > > > could use?
> > > >
> > > > addrmap or something?
> > >
> > > Right, that was what I was thinking. However, I prefer something along
> > > the lines of 'i2c' and 'i2c_sec' or 'client' and 'client_slv' etc.
> >
> > FWIW, the reason it's addmap{0,1} is that the datasheet has documents
> > ADDMAP=0 and the first bank of registers and ADDMAP=1 as the second bank
> > of registers. I adopted that to match the docs for the part.
> >
> > I guess we could do i2c and i2c_sec, I'll just have to put a comment
> > correlating it to the h/w. Calling it 'slv' implies something else
> > so we should avoid that here. The notion of a "secondary" i2c device
> > is completely a Linux I2C subsystem fabrication which wouldn't exist
> > if it allowed multiple slave addresses per device. From a h/w
> > perspective there is really no primary and secondary relationship.
> >
> > I'm fine with i2c/i2c_sec or addrmap0/1 and I will just comment to
> > correlate with the datasheet..pick one.
>
> Let's stick method fabricated by the I2C subsystem. It may seem strange
> from a h/w perspective, but it is the way we (you) have coded it, as
> the first parameter of i2c_new_dummy() is the 'managing' (primary,
> parent, master, whatever) device, so '_sec' would suit as an
> identifying appendage for the resultant device.
That works, I'll also switch to addrmap_[pri|sec] which touches the
regulator driver as well. That will keep the relationship between device
and regmap clear.
-Matt
On Wed, Apr 23, 2014 at 06:01:26PM -0400, Matt Porter wrote:
> On Tue, Apr 22, 2014 at 09:21:39AM +0100, Lee Jones wrote:
> > > > > > s/regmap/Regmap
> > > > >
> > > > > It's consistently written regmap in all the documentation and so on :)
> > > >
> > > > Furry muff; but the comments still stand for the acronyms.
> > > >
> > > > > > addmap{0,1} doesn't quite sit right with me.
> > > > >
> > > > > > REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> > > > > > as I first thought, but still, is there a better naming convention you
> > > > > > could use?
> > > > >
> > > > > addrmap or something?
> > > >
> > > > Right, that was what I was thinking. However, I prefer something along
> > > > the lines of 'i2c' and 'i2c_sec' or 'client' and 'client_slv' etc.
> > >
> > > FWIW, the reason it's addmap{0,1} is that the datasheet has documents
> > > ADDMAP=0 and the first bank of registers and ADDMAP=1 as the second bank
> > > of registers. I adopted that to match the docs for the part.
> > >
> > > I guess we could do i2c and i2c_sec, I'll just have to put a comment
> > > correlating it to the h/w. Calling it 'slv' implies something else
> > > so we should avoid that here. The notion of a "secondary" i2c device
> > > is completely a Linux I2C subsystem fabrication which wouldn't exist
> > > if it allowed multiple slave addresses per device. From a h/w
> > > perspective there is really no primary and secondary relationship.
> > >
> > > I'm fine with i2c/i2c_sec or addrmap0/1 and I will just comment to
> > > correlate with the datasheet..pick one.
> >
> > Let's stick method fabricated by the I2C subsystem. It may seem strange
> > from a h/w perspective, but it is the way we (you) have coded it, as
> > the first parameter of i2c_new_dummy() is the 'managing' (primary,
> > parent, master, whatever) device, so '_sec' would suit as an
> > identifying appendage for the resultant device.
>
> That works, I'll also switch to addrmap_[pri|sec] which touches the
> regulator driver as well. That will keep the relationship between device
> and regmap clear.
Misspoke...I'm switching regmap[0|1] to regmap_[pri|sec] to keep that
synced with i2c_[pri|sec]
-Matt
> > > > > > > s/regmap/Regmap
> > > > > >
> > > > > > It's consistently written regmap in all the documentation and so on :)
> > > > >
> > > > > Furry muff; but the comments still stand for the acronyms.
> > > > >
> > > > > > > addmap{0,1} doesn't quite sit right with me.
> > > > > >
> > > > > > > REVISIT: Ah, it's address-map, rather than add map. Okay, not as bad
> > > > > > > as I first thought, but still, is there a better naming convention you
> > > > > > > could use?
> > > > > >
> > > > > > addrmap or something?
> > > > >
> > > > > Right, that was what I was thinking. However, I prefer something along
> > > > > the lines of 'i2c' and 'i2c_sec' or 'client' and 'client_slv' etc.
> > > >
> > > > FWIW, the reason it's addmap{0,1} is that the datasheet has documents
> > > > ADDMAP=0 and the first bank of registers and ADDMAP=1 as the second bank
> > > > of registers. I adopted that to match the docs for the part.
> > > >
> > > > I guess we could do i2c and i2c_sec, I'll just have to put a comment
> > > > correlating it to the h/w. Calling it 'slv' implies something else
> > > > so we should avoid that here. The notion of a "secondary" i2c device
> > > > is completely a Linux I2C subsystem fabrication which wouldn't exist
> > > > if it allowed multiple slave addresses per device. From a h/w
> > > > perspective there is really no primary and secondary relationship.
> > > >
> > > > I'm fine with i2c/i2c_sec or addrmap0/1 and I will just comment to
> > > > correlate with the datasheet..pick one.
> > >
> > > Let's stick method fabricated by the I2C subsystem. It may seem strange
> > > from a h/w perspective, but it is the way we (you) have coded it, as
> > > the first parameter of i2c_new_dummy() is the 'managing' (primary,
> > > parent, master, whatever) device, so '_sec' would suit as an
> > > identifying appendage for the resultant device.
> >
> > That works, I'll also switch to addrmap_[pri|sec] which touches the
> > regulator driver as well. That will keep the relationship between device
> > and regmap clear.
>
> Misspoke...I'm switching regmap[0|1] to regmap_[pri|sec] to keep that
> synced with i2c_[pri|sec]
Sounds good, thanks Matt.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog