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

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;