MFD: mc13xxx workaround SPI hardware bug on i.Mx

The MC13xxx PMIC is mainly used on i.Mx SoC. On thoses SoC the SPI hardware will deassert CS line as soon as the SPI FIFO is empty. The MC13xxx hardware is very sensitive to CS line change as it corrupts the transfert if CS is deasserted in the middle of a register read or write.
It is not possible to use the CS line as a GPIO on some SoC, so we need to workaround this by implementing a single SPI transfer to access the PMIC.

Quoted 8 lines

The MC13xxx PMIC is mainly used on i.Mx SoC. On thoses SoC the SPI hardware will deassert CS line as soon as the SPI FIFO is empty. The MC13xxx hardware is very sensitive to CS line change as it corrupts the transfert if CS is deasserted in the middle of a register read or write.
It is not possible to use the CS line as a GPIO on some SoC, so we need to workaround this by implementing a single SPI transfer to access the PMIC.

Reviwed-by Mark Brown <[email protected]>

though it's really sad this can't be done in the SPI controller where the bug is. You should also set use_single_rw in the regmap_config, though this is less critical as currently the core won't automatically generate any bulk I/O.

Le mercredi 30 mai 2012 18:08:38 Mark Brown a ?crit :

Quoted 10 lines · Philippe R?tornaz

Quoted 8 lines

The MC13xxx PMIC is mainly used on i.Mx SoC. On thoses SoC the SPI hardware will deassert CS line as soon as the SPI FIFO is empty. The MC13xxx hardware is very sensitive to CS line change as it corrupts the transfert if CS is deasserted in the middle of a register read or write.
It is not possible to use the CS line as a GPIO on some SoC, so we need to workaround this by implementing a single SPI transfer to access the PMIC.

Quoted 6 lines

Reviwed-by Mark Brown <[email protected]>

though it's really sad this can't be done in the SPI controller where the bug is. You should also set use_single_rw in the regmap_config, though this is less critical as currently the core won't automatically generate any bulk I/O.

I already put it in struct regmap_config, should I put it elsewhere ?

(quoted diff) +2 -0

@@ -54,6 +54,67 @@ static struct regmap_config mc13xxx_regmap_spi_config = {
        .max_register = MC13XXX_NUMREGS,
 
        .cache_type = REGCACHE_NONE,
+       .use_single_rw = 1,
+};
 

BTW, who will merge this patchset ?

Quoted 1 line

BTW, who will merge this patchset ?

I will.

Hi Samuel,

Quoted 3 lines · Samuel Ortiz

Quoted 1 line

BTW, who will merge this patchset ?

I will.

Have you had a chance to apply this?

The MC13xxx PMIC is mainly used on i.Mx SoC. On thoses SoC the SPI

hardware will deassert CS line as soon as the SPI FIFO is empty. The MC13xxx hardware is very sensitive to CS line change as it corrupts the transfert if CS is deasserted in the middle of a register

read or write.
It is not possible to use the CS line as a GPIO on some SoC, so we need to workaround this by implementing a single SPI transfer to access the PMIC.

Signed-off-by Philippe R?tornaz <[email protected]>

---
 drivers/mfd/mc13xxx-spi.c |   65

++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 64 insertions(+), 1 deletions(-)

drivers/mfd/mc13xxx-spi.c +44 -0

index 5d1969f..03df422 100644
@@ -54,6 +54,67 @@ static struct regmap_config mc13xxx_regmap_spi_config =
 { .max_register = MC13XXX_NUMREGS,
 
 	.cache_type = REGCACHE_NONE,
+	.use_single_rw = 1,
+};
+
+static int mc13xxx_spi_read(void *context, const void *reg, size_t
 reg_size, +				void *val, size_t val_size)
+{
+	unsigned char w[4] = { *((unsigned char *) reg), 0, 0, 0};
+	unsigned char r[4];
+	unsigned char *p = val;
+	struct device *dev = context;
+	struct spi_device *spi = to_spi_device(dev);
+	struct spi_transfer t = {
+		.tx_buf = w,
+		.rx_buf = r,
+		.len = 4,
+	};
+
+	struct spi_message m;
+	int ret;
+
+	if (val_size != 3 || reg_size != 1)
+		return -ENOTSUPP;
+
+	spi_message_init(&m);
+	spi_message_add_tail(&t, &m);
+	ret = spi_sync(spi, &m);
+
+	memcpy(p, &r[1], 3);
+
+	return ret;
+}
+
+static int mc13xxx_spi_write(void *context, const void *data, size_t
 count) +{
+	struct device *dev = context;
+	struct spi_device *spi = to_spi_device(dev);
+
+	if (count != 4)
+		return -ENOTSUPP;
+
+	return spi_write(spi, data, count);
+}
+
+/*
+ * We cannot use regmap-spi generic bus implementation here.
+ * The MC13783 chip will get corrupted if CS signal is deasserted

drivers/mfd/mc13xxx-spi.c +8 -1

+ */
+
+static struct regmap_bus regmap_mc13xxx_bus = {
+	.write = mc13xxx_spi_write,
+	.read = mc13xxx_spi_read,
 };
 
 static int mc13xxx_spi_probe(struct spi_device *spi)

@@ -78,7 +139,9 @@ static int mc13xxx_spi_probe(struct spi_device *spi)
 	mc13xxx->dev = &spi->dev;
 	mutex_init(&mc13xxx->lock);
 
-	mc13xxx->regmap = regmap_init_spi(spi, &mc13xxx_regmap_spi_config);
+	mc13xxx->regmap = regmap_init(&spi->dev, &regmap_mc13xxx_bus, &spi->dev,
+					&mc13xxx_regmap_spi_config);
+
 	if (IS_ERR(mc13xxx->regmap)) {
 		ret = PTR_ERR(mc13xxx->regmap);
 		dev_err(mc13xxx->dev, "Failed to initialize register map: %d\n",

This fix the SPI regmap configuration, the wrong write flag was used. Also, bits_per_word should not be set as the regmap spi implementation uses a 8bits transfert granularity.

Signed-off-by Philippe R?tornaz <[email protected]>

Hello

This patchset fixes the mc13xxx regression introduced by:
"mfd: Use regmap for the mc13xxx-core register access".

The first patch correct the write_flag_mask to use the correct value and remove the invalid bits_per_word in the probe.

The second patch workaround an silicon errata on some imx SoC.

Hi Philippe,

Quoted 5 lines · Philippe R?tornaz

Hello

This patchset fixes the mc13xxx regression introduced by:
"mfd: Use regmap for the mc13xxx-core register access".

Quoted 4 lines

The first patch correct the write_flag_mask to use the correct value and remove the invalid bits_per_word in the probe.

The second patch workaround an silicon errata on some imx SoC.

Both patches applied to my for-linus branch. They'll be part of the MFD fixes pull request for 3.5.