[U-Boot] [PATCH 1/2] gpio: tca642x: Add the tca642x gpio expander driver

Nishanth Menon
2013-07-09T16:21:04Z

On 07/09/2013 11:17 AM, Dan Murphy wrote:

> On 07/08/2013 04:27 PM, Nishanth Menon wrote:
>> On 07/08/2013 03:53 PM, Dan Murphy wrote:
[...]
>>> +/* tca642x register address definitions */
>>> +struct tca642x_bank_info tca642x_banks[] = {
>>> + {0x00, 0x04, 0x08, 0x0c},
>>> + {0x01, 0x05, 0x09, 0x0d},
>>> + {0x02, 0x06, 0x0a, 0x0e},
>> could we use explicit map of the struct params? helps any future
>> expansion.
>>
> Not sure what you are looking for here. These are the register
> addresses within the chip.

something like
{.input_reg = 0x00, .polarity_reg = 0x04, ....

It helps readability and future modification to bank_info structure.

[..]
>>> + int org_bus_num;
>>> + int ret;
>>> +
>>> + org_bus_num = i2c_get_bus_num();
>>> + i2c_set_bus_num(CONFIG_SYS_I2C_TCA642X_BUS_NUM);
>>> +
>>> + if (i2c_read(chip, addr, 1, (u8 *)&valw, 1)) {
>>> + printf("Could not read before writing\n");
>>> + ret = -1;
>>> + goto error;
>>> + }
>>> + valw &= ~mask;
>> data &= mask ?
> Well in this case the mask is not being passed in. The bit to flip is
> being passed and then the actual mask
> is created here.

reg_setbits? instead then? following the standard api convention - maybe
make this as a i2c generic api?
--
Regards,
Nishanth Menon