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

[Nishanth Menon]

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