[PATCH V2 1/3] gpio: pca953x: make the register access by GPIO bank

[gregory.clement@free-electrons.com (Gregory CLEMENT)]

On 01/08/2013 09:32 AM, Maxime Ripard wrote:
> Hi Gregory,

Hi Maxime,

thanks for testing
> 
> On 07/01/2013 23:51, Gregory CLEMENT wrote:
>> -static int pca953x_write_reg(struct pca953x_chip *chip, int reg, u32 val)
>> +static int pca953x_read_single(struct pca953x_chip *chip, int reg, u32 *val,
>> +				int off)
>> +{
>> +	int ret;
>> +	int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
>> +	int offset = off / BANK_SZ;
>> +
>> +	ret = i2c_smbus_read_byte_data(chip->client,
>> +				(reg << bank_shift) + offset);
>> +	*val = ret;
>> +
>> +	if (ret < 0) {
>> +		dev_err(&chip->client->dev, "failed reading register\n", reg);
> 
> This triggers a warning, since you have an argument but nothing corresponding to it in your format string.

OK this one is easy, I didn't finished to clean up the message.

> 
>>  static irqreturn_t pca953x_irq_handler(int irq, void *devid)
>>  {
>>  	struct pca953x_chip *chip = devid;
>> -	u32 pending;
>> -	u32 level;
>> +	u8 pending[MAX_BANK];
>> +	u8 level;
>> +	int i;
>>  
>> -	pending = pca953x_irq_pending(chip);
>> -
>> -	if (!pending)
>> +	if (!pca953x_irq_pending(chip, pending))
>>  		return IRQ_HANDLED;
>>  
>> -	do {
>> -		level = __ffs(pending);
>> -		handle_nested_irq(irq_find_mapping(chip->domain, level));
>> -
>> -		pending &= ~(1 << level);
>> -	} while (pending);
>> +	for (i = 0; i < NBANK(chip); i++) {
>> +		do {
>> +			level = __ffs(pending[i]);
>> +			handle_nested_irq(irq_find_mapping(chip->domain,
>> +							level + (BANK_SZ * i)));
>> +			pending[i] &= ~(1 << level);
>> +		} while (pending[i]);
>> +	}
>>  
>>  	return IRQ_HANDLED;
>>  }
> 
> This triggers the following warning when an interrupt is raised:
> 
> [   30.773500] ------------[ cut here ]------------
> [   30.778843] WARNING: at /home/tmp/linux/kernel/irq/irqdomain.c:137 irq_domain_legacy_revmap+0x2c/0x48()
> [   30.788375] Modules linked in:
> [   30.791531] [<c0014100>] (unwind_backtrace+0x0/0xf0) from [<c001c878>] (warn_slowpath_common+0x4c/0x64)
> [   30.801125] [<c001c878>] (warn_slowpath_common+0x4c/0x64) from [<c001c8ac>] (warn_slowpath_null+0x1c/0x24)
> [   30.810968] [<c001c8ac>] (warn_slowpath_null+0x1c/0x24) from [<c006b5e0>] (irq_domain_legacy_revmap+0x2c/0x48)
> [   30.821187] [<c006b5e0>] (irq_domain_legacy_revmap+0x2c/0x48) from [<c01c3734>] (pca953x_irq_handler+0x16c/0x1ac)
> [   30.831656] [<c01c3734>] (pca953x_irq_handler+0x16c/0x1ac) from [<c0068688>] (irq_thread+0xd0/0x124)
> [   30.841000] [<c0068688>] (irq_thread+0xd0/0x124) from [<c003c238>] (kthread+0xa4/0xb0)
> [   30.849125] [<c003c238>] (kthread+0xa4/0xb0) from [<c000ed48>] (ret_from_fork+0x14/0x2c)
> [   30.857375] ---[ end trace 09584b7a73100a49 ]---
> 

Humm it seems that this warning is caused by attempting to use an out of range hwirq.
I need to investigate it more in details.

Expect the IRQ part, is this version is working for your gpio expander?

> Maxime
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com