linux-kernel - Re: [PATCH 1/3] gpio: pca953x: make the register access by GPIO bank

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 07 Jan 2013 15:26:01 +0100
From:	Maxime Ripard <maxime.ripard@...e-electrons.com>
To:	Gregory CLEMENT <gregory.clement@...e-electrons.com>
CC:	Linus Walleij <linus.walleij@...aro.org>,
	Grant Likely <grant.likely@...retlab.ca>,
	Andreas Schallenberg <Andreas.Schallenberg@...itytechnica.com>,
	Roland Stigge <stigge@...com.de>,
	Jason Cooper <jason@...edaemon.net>,
	Andrew Lunn <andrew@...n.ch>,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] gpio: pca953x: make the register access by GPIO bank

Hi Gregory,

Le 06/01/2013 18:34, Gregory CLEMENT a écrit :
> +static int pca953x_write_regs(struct pca953x_chip *chip, int reg, u8 *val)
>  {
>  	int ret = 0;
>  
>  	if (chip->gpio_chip.ngpio <= 8)
> -		ret = i2c_smbus_write_byte_data(chip->client, reg, val);
> -	else if (chip->gpio_chip.ngpio == 24) {
> -		cpu_to_le32s(&val);
> +		ret = i2c_smbus_write_byte_data(chip->client, reg, *val);
> +	else if (chip->gpio_chip.ngpio >= 24) {
> +		int bank_shift = fls(chip->gpio_chip.ngpio) - 3;
>  		ret = i2c_smbus_write_i2c_block_data(chip->client,
> -						(reg << 2) | REG_ADDR_AI,
> -						3,
> -						(u8 *) &val);
> +					(reg << bank_shift) | REG_ADDR_AI,
> +					NBANK(chip),
> +					val);
>  	}
>  	else {
>  		switch (chip->chip_type) {
>  		case PCA953X_TYPE:
>  			ret = i2c_smbus_write_word_data(chip->client,
> -							reg << 1, val);
> +							reg << 1, (u16) *val);
>  			break;
>  		case PCA957X_TYPE:
>  			ret = i2c_smbus_write_byte_data(chip->client, reg << 1,
> -							val & 0xff);
> +							val[0]);
>  			if (ret < 0)
>  				break;
>  			ret = i2c_smbus_write_byte_data(chip->client,
>  							(reg << 1) + 1,
> -							(val & 0xff00) >> 8);
> +							val[1]);
>  			break;
>  		}
>  	}

I just tested your patch on a PCA9555 on the Crystalfontz CFA-10049, and
it doesn't work anywore. It returns ENXIO now when you use this
function. As discussed on IRC, it seems that the fix would be to replace
all the calls to fls(chip->gpio_chip.ngpio) by fls(chip->gpio_chip.ngpio
- 1).

Also, all the irq handling is not compiling anymore for trivial errors
(variables not defined, incompatible types, etc.). So obviously, I
couldn't test the changes you made on the IRQ related functions.

Maxime

-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/