mfd: da9052: Avoid setting read_flag_mask for da9052-i2c driver

PATCH In review Axel Lin 3 participants 5 messages Aug 16, 2014 – Sep 4, 2014

Subsystems (2)

Multifunction devices (MFD) Dialog semiconductor drivers

lore

Share & tools

lore

Axel Lin

author

2014-08-16 13:23 UTC #0 lore raw

PATCH 1/1 mfd: da9052: Avoid setting read_flag_mask for da9052-i2c driver

Current code init regmap with &da9052_regmap_config for both da9052-spi and da9052-i2c drivers. da9052-spi sets the read_flag_mask.
The same setting may be applied for da9052-i2c if da9052-spi driver is loaded first because they actually use the same regmap_config setting.
Fix this issue by using a local variable for regmap_config in da9052-spi driver, so the settings in spi driver won't impact the settings in i2c driver. Also makes da9052_regmap_config const to avoid similar issue.

Signed-off-by Axel Lin <[email protected]>

---
Hi Adam and Steve,
Any chance to test this patch?

Thanks,
Axel

 drivers/mfd/da9052-core.c         | 2 +-
 drivers/mfd/da9052-spi.c          | 7 ++++---
 include/linux/mfd/da9052/da9052.h | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

drivers/mfd/da9052-core.c +1 -1

index e8af816..52a0c2f 100644
@@ -522,7 +522,7 @@ static const struct mfd_cell da9052_subdev_info[] = {
 	},
 };
 
-struct regmap_config da9052_regmap_config = {
+const struct regmap_config da9052_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,

drivers/mfd/da9052-spi.c +4 -3

index 17666b4..45ae0b7 100644
@@ -23,6 +23,7 @@
 
 static int da9052_spi_probe(struct spi_device *spi)
 {
+	struct regmap_config config;
 	int ret;
 	const struct spi_device_id *id = spi_get_device_id(spi);
 	struct da9052 *da9052;

@@ -40,10 +41,10 @@ static int da9052_spi_probe(struct spi_device *spi)
 
 	spi_set_drvdata(spi, da9052);
 
-	da9052_regmap_config.read_flag_mask = 1;
-	da9052_regmap_config.write_flag_mask = 0;
+	config = da9052_regmap_config;
+	config.read_flag_mask = 1;
 
-	da9052->regmap = devm_regmap_init_spi(spi, &da9052_regmap_config);
+	da9052->regmap = devm_regmap_init_spi(spi, &config);
 	if (IS_ERR(da9052->regmap)) {
 		ret = PTR_ERR(da9052->regmap);
 		dev_err(&spi->dev, "Failed to allocate register map: %d\n",

include/linux/mfd/da9052/da9052.h +1 -1

index bba65f5..c18a4c1 100644
@@ -211,7 +211,7 @@ static inline int da9052_reg_update(struct da9052 *da9052, unsigned char reg,
 int da9052_device_init(struct da9052 *da9052, u8 chip_id);
 void da9052_device_exit(struct da9052 *da9052);
 
-extern struct regmap_config da9052_regmap_config;
+extern const struct regmap_config da9052_regmap_config;
 
 int da9052_irq_init(struct da9052 *da9052);
 int da9052_irq_exit(struct da9052 *da9052);

Signature · 1 line

1.9.1

Lee Jones

2014-08-28 11:21 UTC #1 lore raw

Steve, Adam,

Quoted 8 lines

Current code init regmap with &da9052_regmap_config for both da9052-spi and da9052-i2c drivers. da9052-spi sets the read_flag_mask.
The same setting may be applied for da9052-i2c if da9052-spi driver is loaded first because they actually use the same regmap_config setting.
Fix this issue by using a local variable for regmap_config in da9052-spi driver, so the settings in spi driver won't impact the settings in i2c driver. Also makes da9052_regmap_config const to avoid similar issue.

Quoted 4 lines

Signed-off-by Axel Lin <[email protected]>
---
Hi Adam and Steve,
Any chance to test this patch?

** Look here **

Quoted 59 lines

Thanks,
Axel

 drivers/mfd/da9052-core.c         | 2 +-
 drivers/mfd/da9052-spi.c          | 7 ++++---
 include/linux/mfd/da9052/da9052.h | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

drivers/mfd/da9052-core.c +1 -1

index e8af816..52a0c2f 100644
@@ -522,7 +522,7 @@ static const struct mfd_cell da9052_subdev_info[] = {
 	},
 };
 
-struct regmap_config da9052_regmap_config = {
+const struct regmap_config da9052_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,

drivers/mfd/da9052-spi.c +4 -3

index 17666b4..45ae0b7 100644
@@ -23,6 +23,7 @@
 
 static int da9052_spi_probe(struct spi_device *spi)
 {
+	struct regmap_config config;
 	int ret;
 	const struct spi_device_id *id = spi_get_device_id(spi);
 	struct da9052 *da9052;

@@ -40,10 +41,10 @@ static int da9052_spi_probe(struct spi_device *spi)
 
 	spi_set_drvdata(spi, da9052);
 
-	da9052_regmap_config.read_flag_mask = 1;
-	da9052_regmap_config.write_flag_mask = 0;
+	config = da9052_regmap_config;
+	config.read_flag_mask = 1;
 
-	da9052->regmap = devm_regmap_init_spi(spi, &da9052_regmap_config);
+	da9052->regmap = devm_regmap_init_spi(spi, &config);
 	if (IS_ERR(da9052->regmap)) {
 		ret = PTR_ERR(da9052->regmap);
 		dev_err(&spi->dev, "Failed to allocate register map: %d\n",

include/linux/mfd/da9052/da9052.h +1 -1

index bba65f5..c18a4c1 100644
@@ -211,7 +211,7 @@ static inline int da9052_reg_update(struct da9052 *da9052, unsigned char reg,
 int da9052_device_init(struct da9052 *da9052, u8 chip_id);
 void da9052_device_exit(struct da9052 *da9052);
 
-extern struct regmap_config da9052_regmap_config;
+extern const struct regmap_config da9052_regmap_config;
 
 int da9052_irq_init(struct da9052 *da9052);
 int da9052_irq_exit(struct da9052 *da9052);

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Opensource [Adam Thomson]

2014-08-28 13:38 UTC #2 lore raw

Quoted 10 lines · Lee Jones

Steve, Adam,
Quoted 8 lines
Current code init regmap with &da9052_regmap_config for both da9052-spi and da9052-i2c drivers. da9052-spi sets the read_flag_mask.
The same setting may be applied for da9052-i2c if da9052-spi driver is loaded first because they actually use the same regmap_config setting.
Fix this issue by using a local variable for regmap_config in da9052-spi driver, so the settings in spi driver won't impact the settings in i2c driver. Also makes da9052_regmap_config const to avoid similar issue.

Quoted 6 lines

Quoted 4 lines
Signed-off-by Axel Lin <[email protected]>
---
Hi Adam and Steve,
Any chance to test this patch?
** Look here **

Lee, Axel,

This was on my to-do list but hadn't yet responded. Will try and find some time
in the next week to see if we can verify this. ��{.n�+��+%��ݶ��w��{.n�+��{��G��{ay�ʇڙ�,j��f��h��z_�(�階�ݢj"��m��G��?��&��~��iO��z��v�^�m��?�I�

Opensource [Adam Thomson]

Acked-by

2014-09-03 10:50 UTC #3 lore raw

Quoted 17 lines · Adam Thomson

Quoted 15 lines · Lee Jones
Steve, Adam,
Quoted 12 lines
Current code init regmap with &da9052_regmap_config for both da9052-spi and da9052-i2c drivers. da9052-spi sets the read_flag_mask.
The same setting may be applied for da9052-i2c if da9052-spi driver is loaded first because they actually use the same regmap_config setting.
Fix this issue by using a local variable for regmap_config in da9052-spi driver, so the settings in spi driver won't impact the settings in i2c driver. Also makes da9052_regmap_config const to avoid similar issue.
Signed-off-by Axel Lin <[email protected]>
---
Hi Adam and Steve,
Any chance to test this patch?

Quoted 6 lines

Quoted 1 line
** Look here **
Lee, Axel,
This was on my to-do list but hadn't yet responded. Will try and find some time in the next week to see if we can verify this.

Hi Axel, Lee,

I don't think I'm going to have time to test this soon. In terms of the change it looks fine to me, although I don't think you can run both SPI and I2C at the same time for this driver (I tried a quick test to instantiate both and saw issues with regulator initialisation for the second instance). Anyway, here's my ack for the change.

Acked-by Adam Thomson <[email protected]>

��{.n�+��+%��ݶ��w��{.n�+��{��G��{ay�ʇڙ�,j��f��h��z_�(�階�ݢj"��m��G��?��&��~��iO��z��v�^�m��?�I�

Lee Jones

2014-09-04 08:41 UTC #4 lore raw

In reply to Axel Lin

Quoted 11 lines · Axel Lin

Current code init regmap with &da9052_regmap_config for both da9052-spi and da9052-i2c drivers. da9052-spi sets the read_flag_mask.
The same setting may be applied for da9052-i2c if da9052-spi driver is loaded first because they actually use the same regmap_config setting.
Fix this issue by using a local variable for regmap_config in da9052-spi driver, so the settings in spi driver won't impact the settings in i2c driver. Also makes da9052_regmap_config const to avoid similar issue.
Signed-off-by Axel Lin <[email protected]>
---
Hi Adam and Steve,

Applied with Adam's Ack.

Quoted 61 lines

Any chance to test this patch?

Thanks,
Axel

 drivers/mfd/da9052-core.c         | 2 +-
 drivers/mfd/da9052-spi.c          | 7 ++++---
 include/linux/mfd/da9052/da9052.h | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

drivers/mfd/da9052-core.c +1 -1

index e8af816..52a0c2f 100644
@@ -522,7 +522,7 @@ static const struct mfd_cell da9052_subdev_info[] = {
 	},
 };
 
-struct regmap_config da9052_regmap_config = {
+const struct regmap_config da9052_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,

drivers/mfd/da9052-spi.c +4 -3

index 17666b4..45ae0b7 100644
@@ -23,6 +23,7 @@
 
 static int da9052_spi_probe(struct spi_device *spi)
 {
+	struct regmap_config config;
 	int ret;
 	const struct spi_device_id *id = spi_get_device_id(spi);
 	struct da9052 *da9052;

@@ -40,10 +41,10 @@ static int da9052_spi_probe(struct spi_device *spi)
 
 	spi_set_drvdata(spi, da9052);
 
-	da9052_regmap_config.read_flag_mask = 1;
-	da9052_regmap_config.write_flag_mask = 0;
+	config = da9052_regmap_config;
+	config.read_flag_mask = 1;
 
-	da9052->regmap = devm_regmap_init_spi(spi, &da9052_regmap_config);
+	da9052->regmap = devm_regmap_init_spi(spi, &config);
 	if (IS_ERR(da9052->regmap)) {
 		ret = PTR_ERR(da9052->regmap);
 		dev_err(&spi->dev, "Failed to allocate register map: %d\n",

include/linux/mfd/da9052/da9052.h +1 -1

index bba65f5..c18a4c1 100644
@@ -211,7 +211,7 @@ static inline int da9052_reg_update(struct da9052 *da9052, unsigned char reg,
 int da9052_device_init(struct da9052 *da9052, u8 chip_id);
 void da9052_device_exit(struct da9052 *da9052);
 
-extern struct regmap_config da9052_regmap_config;
+extern const struct regmap_config da9052_regmap_config;
 
 int da9052_irq_init(struct da9052 *da9052);
 int da9052_irq_exit(struct da9052 *da9052);

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog