[U-Boot] [PATCH] misc: led: Add PCA9551 LED driver

[Fabio Estevam <festevam@gmail.com>]

Hi Stefan,

On Wed, Mar 11, 2015 at 5:51 AM, Stefan Roese <sr@denx.de> wrote:

> +
> +config PCA9551_LED
> +       bool "Enable PCA9551 LED driver"
> +       depends on TARGET_TQMA6

Is this 'depends' correct? We should be able to use this driver with
other boards.

> +static int pca9551_led_get_state(int led, int *state)
> +{
> +       unsigned int reg;
> +       u8 shift, buf;
> +
> +       if (led < 0 || led > 7) {
> +               return -1;
> +       } else if (led < 4) {
> +               reg = PCA9551_REG_LS0;
> +               shift = led << 1;
> +       } else {
> +               reg = PCA9551_REG_LS1;
> +               shift = (led - 4) << 1;
> +       }
> +
> +       if (i2c_read(CONFIG_PCA9551_I2C_ADDR, reg, 1, &buf, 1))
> +               return -1;

what about:

ret = i2c_read(CONFIG_PCA9551_I2C_ADDR, reg, 1, &buf, 1);
if (ret)
    return ret;

> +
> +       *state = (buf >> shift) & 0x03;
> +       return 0;
> +}
> +
> +static int pca9551_led_set_state(int led, int state)
> +{
> +       unsigned int reg;
> +       u8 shift, buf, mask;
> +
> +       if (led < 0 || led > 7) {
> +               return -1;

return -EINVAL;

> +       } else if (led < 4) {
> +               reg = PCA9551_REG_LS0;
> +               shift = led << 1;
> +       } else {
> +               reg = PCA9551_REG_LS1;
> +               shift = (led - 4) << 1;
> +       }
> +       mask = 0x03 << shift;
> +
> +       if (i2c_read(CONFIG_PCA9551_I2C_ADDR, reg, 1, &buf, 1))
> +               return -1;

Same here.

> +
> +       buf = (buf & ~mask) | ((state & 0x03) << shift);
> +
> +       if (i2c_write(CONFIG_PCA9551_I2C_ADDR, reg, 1, &buf, 1))
> +               return -1;

Same here.

> +
> +       return 0;
> +}
> +
> +static int pca9551_led_set_blink_rate(int idx, struct pca9551_blink_rate rate)
> +{
> +       unsigned int reg;
> +
> +       switch (idx) {
> +       case 0:
> +               reg = PCA9551_REG_PSC0;
> +               break;
> +       case 1:
> +               reg = PCA9551_REG_PSC1;
> +               break;
> +       default:
> +               return -1;

return -EINVAL;