Re: mc13xxx-core: kernel hangs after 'regmap_read'

[Marc Reilly <marc@cpdesign.com.au>]

Hi,

On Thursday, May 24, 2012 05:33:16 PM Shawn Guo wrote:
> On Thu, May 24, 2012 at 08:46:35AM +0200, Uwe Kleine-König wrote:
> > Hello,
> > 
> > On Thu, May 24, 2012 at 02:39:00PM +0800, Shawn Guo wrote:
> > > On Thu, May 24, 2012 at 01:07:43AM -0300, Fabio Estevam wrote:
> > > > I am trying to understand why mx31pdk still fails.
> > > 
> > > So different from imx51-babbage which uses MC13892, mx31pdk uses
> > > MC13783?  But both chips should have the same regmap, right?
> > 
> > They are similar. One difference is the protocol used. MC13783 only
> > speaks spi, MC13892 can do both, spi and i2c. Does someone has a working
> > MC13892 that uses spi?
> 
> I do.  With the following patch applied on top of linux-next, it works
> on my imx51-babbage board.
> 
> Regards,
> Shawn
> 
> diff --git a/drivers/mfd/mc13xxx-spi.c b/drivers/mfd/mc13xxx-spi.c
> index 3fcdab3..5d1969f 100644
> --- a/drivers/mfd/mc13xxx-spi.c
> +++ b/drivers/mfd/mc13xxx-spi.c
> @@ -49,6 +49,7 @@ static struct regmap_config mc13xxx_regmap_spi_config = {
>         .reg_bits = 7,
>         .pad_bits = 1,
>         .val_bits = 24,
> +       .write_flag_mask = 0x80,

Should probably have .read_flag_mask = 0x00, here too. If either are non zero, 
both are set.

I guess this is the problem, regmap's default read_flag_mask for the spi bus 
is 0x80, and the write mask defaults to 0. The mc13xxx works the opposite way 
though! aarg. My bad for not noticing. So setting the write_flag_mask to 0x80 
also sets the read mask to 0 correctly also.

 
>         .max_register = MC13XXX_NUMREGS,
> 
> @@ -73,7 +74,6 @@ static int mc13xxx_spi_probe(struct spi_device *spi)
> 
>         dev_set_drvdata(&spi->dev, mc13xxx);
>         spi->mode = SPI_MODE_0 | SPI_CS_HIGH;
> -       spi->bits_per_word = 32;

My bad here too. Because I had no hardware to test with I tried to preserve as 
much of the old code as I could.

Sorry about the stuff ups. I owe each of you a beer (unfortunately I'll have 
to exclude the list in general from that offer :) ).

I'm wondering about regmap_init where the buf_size is set up. I think it will 
only end up being 3 bytes. I think line 249 should be something like:

	map->format.buf_size = (config->reg_bits 
                          + config->val_bits 
					         + (config->pad_bits % 8)) / 8;

or perhaps better:
	map->reg_shift = config->pad_bits % 8;
	map->format.buf_size = (config->reg_bits 
                          + config->val_bits 
					         + map->reg_shift) / 8;

Am I right here or just confused?

Cheers,
Marc