* [PATCH linux dev-4.10 v2] leds: pca955x: check for I2C errors
@ 2017-08-28 6:37 Cédric Le Goater
2017-08-29 5:45 ` Andrew Jeffery
0 siblings, 1 reply; 3+ messages in thread
From: Cédric Le Goater @ 2017-08-28 6:37 UTC (permalink / raw)
To: openbmc; +Cc: Andrew Jeffery, Joel Stanley, Cédric Le Goater
This should also allow probing to fail when a pca955x chip is not
found on a I2C bus.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
Changes since v1:
- introduced a pca955x_set_value() routine returning an errno
- modified pca955x_gpio_direction_{in,out}put to return an errno
drivers/leds/leds-pca955x.c | 114 ++++++++++++++++++++++++++++++++------------
1 file changed, 83 insertions(+), 31 deletions(-)
diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index 09303fd1fdc6..905729191d3e 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -165,13 +165,18 @@ static inline u8 pca955x_ledsel(u8 oldval, int led_num, int state)
* Write to frequency prescaler register, used to program the
* period of the PWM output. period = (PSCx + 1) / 38
*/
-static void pca955x_write_psc(struct i2c_client *client, int n, u8 val)
+static int pca955x_write_psc(struct i2c_client *client, int n, u8 val)
{
struct pca955x *pca955x = i2c_get_clientdata(client);
+ int ret;
- i2c_smbus_write_byte_data(client,
+ ret = i2c_smbus_write_byte_data(client,
pca95xx_num_input_regs(pca955x->chipdef->bits) + 2*n,
val);
+ if (ret < 0)
+ dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n",
+ __func__, n, val, ret);
+ return ret;
}
/*
@@ -181,38 +186,56 @@ static void pca955x_write_psc(struct i2c_client *client, int n, u8 val)
*
* Duty cycle is (256 - PWMx) / 256
*/
-static void pca955x_write_pwm(struct i2c_client *client, int n, u8 val)
+static int pca955x_write_pwm(struct i2c_client *client, int n, u8 val)
{
struct pca955x *pca955x = i2c_get_clientdata(client);
+ int ret;
- i2c_smbus_write_byte_data(client,
+ ret = i2c_smbus_write_byte_data(client,
pca95xx_num_input_regs(pca955x->chipdef->bits) + 1 + 2*n,
val);
+ if (ret < 0)
+ dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n",
+ __func__, n, val, ret);
+ return ret;
}
/*
* Write to LED selector register, which determines the source that
* drives the LED output.
*/
-static void pca955x_write_ls(struct i2c_client *client, int n, u8 val)
+static int pca955x_write_ls(struct i2c_client *client, int n, u8 val)
{
struct pca955x *pca955x = i2c_get_clientdata(client);
+ int ret;
- i2c_smbus_write_byte_data(client,
+ ret = i2c_smbus_write_byte_data(client,
pca95xx_num_input_regs(pca955x->chipdef->bits) + 4 + n,
val);
+ if (ret < 0)
+ dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n",
+ __func__, n, val, ret);
+ return ret;
}
/*
* Read the LED selector register, which determines the source that
* drives the LED output.
*/
-static u8 pca955x_read_ls(struct i2c_client *client, int n)
+static int pca955x_read_ls(struct i2c_client *client, int n, u8 *val)
{
struct pca955x *pca955x = i2c_get_clientdata(client);
+ int ret;
- return (u8) i2c_smbus_read_byte_data(client,
+ ret = i2c_smbus_read_byte_data(client,
pca95xx_num_input_regs(pca955x->chipdef->bits) + 4 + n);
+ if (ret < 0) {
+ dev_err(&client->dev, "%s: reg 0x%x, err %d\n",
+ __func__, n, ret);
+ return ret;
+ }
+ *val = (u8)ret;
+ return 0;
}
static int pca955x_led_set(struct led_classdev *led_cdev,
@@ -223,6 +246,7 @@ static int pca955x_led_set(struct led_classdev *led_cdev,
u8 ls;
int chip_ls; /* which LSx to use (0-3 potentially) */
int ls_led; /* which set of bits within LSx to use (0-3) */
+ int ret;
pca955x_led = container_of(led_cdev, struct pca955x_led, led_cdev);
pca955x = pca955x_led->pca955x;
@@ -232,7 +256,9 @@ static int pca955x_led_set(struct led_classdev *led_cdev,
mutex_lock(&pca955x->lock);
- ls = pca955x_read_ls(pca955x->client, chip_ls);
+ ret = pca955x_read_ls(pca955x->client, chip_ls, &ls);
+ if (ret)
+ goto out;
switch (value) {
case LED_FULL:
@@ -252,26 +278,37 @@ static int pca955x_led_set(struct led_classdev *led_cdev,
* OFF, HALF, or FULL. But, this is probably better than
* just turning off for all other values.
*/
- pca955x_write_pwm(pca955x->client, 1,
- 255 - value);
+ ret = pca955x_write_pwm(pca955x->client, 1, 255 - value);
+ if (ret)
+ goto out;
ls = pca955x_ledsel(ls, ls_led, PCA955X_LS_BLINK1);
break;
}
- pca955x_write_ls(pca955x->client, chip_ls, ls);
+ ret = pca955x_write_ls(pca955x->client, chip_ls, ls);
+out:
mutex_unlock(&pca955x->lock);
- return 0;
+ return ret;
}
#ifdef CONFIG_LEDS_PCA955X_GPIO
/*
* Read the INPUT register, which contains the state of LEDs.
*/
-static u8 pca955x_read_input(struct i2c_client *client, int n)
+static int pca955x_read_input(struct i2c_client *client, int n, u8 *val)
{
- return (u8)i2c_smbus_read_byte_data(client, n);
+ int ret = i2c_smbus_read_byte_data(client, n);
+
+ if (ret < 0) {
+ dev_err(&client->dev, "%s: reg 0x%x, err %d\n",
+ __func__, n, ret);
+ return ret;
+ }
+ *val = (u8)ret;
+ return 0;
+
}
static int pca955x_gpio_request_pin(struct gpio_chip *gc, unsigned int offset)
@@ -285,23 +322,32 @@ static int pca955x_gpio_request_pin(struct gpio_chip *gc, unsigned int offset)
return -EBUSY;
}
-static void pca955x_gpio_set_value(struct gpio_chip *gc, unsigned int offset,
- int val)
+static int pca955x_set_value(struct gpio_chip *gc, unsigned int offset,
+ int val)
{
struct pca955x *pca955x = gpiochip_get_data(gc);
struct pca955x_led *led = &pca955x->leds[offset];
if (val)
- pca955x_led_set(&led->led_cdev, LED_FULL);
+ return pca955x_led_set(&led->led_cdev, LED_FULL);
else
- pca955x_led_set(&led->led_cdev, LED_OFF);
+ return pca955x_led_set(&led->led_cdev, LED_OFF);
+}
+
+static void pca955x_gpio_set_value(struct gpio_chip *gc, unsigned int offset,
+ int val)
+{
+ pca955x_set_value(gc, offset, val);
}
static int pca955x_gpio_get_value(struct gpio_chip *gc, unsigned int offset)
{
struct pca955x *pca955x = gpiochip_get_data(gc);
struct pca955x_led *led = &pca955x->leds[offset];
- u8 reg = pca955x_read_input(pca955x->client, led->led_num / 8);
+ u8 reg = 0;
+
+ /* There is nothing we can do about errors */
+ pca955x_read_input(pca955x->client, led->led_num / 8, ®);
return !!(reg & (1 << (led->led_num % 8)));
}
@@ -310,17 +356,13 @@ static int pca955x_gpio_direction_input(struct gpio_chip *gc,
unsigned int offset)
{
/* To use as input ensure pin is not driven */
- pca955x_gpio_set_value(gc, offset, 0);
-
- return 0;
+ return pca955x_set_value(gc, offset, 0);
}
static int pca955x_gpio_direction_output(struct gpio_chip *gc,
unsigned int offset, int val)
{
- pca955x_gpio_set_value(gc, offset, val);
-
- return 0;
+ return pca955x_set_value(gc, offset, val);
}
#endif /* CONFIG_LEDS_PCA955X_GPIO */
@@ -495,19 +537,29 @@ static int pca955x_probe(struct i2c_client *client,
return err;
/* Turn off LED */
- pca955x_led_set(&pca955x_led->led_cdev, LED_OFF);
+ err = pca955x_led_set(&pca955x_led->led_cdev, LED_OFF);
+ if (err)
+ return err;
}
}
/* PWM0 is used for half brightness or 50% duty cycle */
- pca955x_write_pwm(client, 0, 255-LED_HALF);
+ err = pca955x_write_pwm(client, 0, 255 - LED_HALF);
+ if (err)
+ return err;
/* PWM1 is used for variable brightness, default to OFF */
- pca955x_write_pwm(client, 1, 0);
+ err = pca955x_write_pwm(client, 1, 0);
+ if (err)
+ return err;
/* Set to fast frequency so we do not see flashing */
- pca955x_write_psc(client, 0, 0);
- pca955x_write_psc(client, 1, 0);
+ err = pca955x_write_psc(client, 0, 0);
+ if (err)
+ return err;
+ err = pca955x_write_psc(client, 1, 0);
+ if (err)
+ return err;
#ifdef CONFIG_LEDS_PCA955X_GPIO
if (ngpios) {
--
2.13.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH linux dev-4.10 v2] leds: pca955x: check for I2C errors
2017-08-28 6:37 [PATCH linux dev-4.10 v2] leds: pca955x: check for I2C errors Cédric Le Goater
@ 2017-08-29 5:45 ` Andrew Jeffery
2017-08-29 5:58 ` Cédric Le Goater
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Jeffery @ 2017-08-29 5:45 UTC (permalink / raw)
To: Cédric Le Goater, openbmc
[-- Attachment #1: Type: text/plain, Size: 9031 bytes --]
On Mon, 2017-08-28 at 08:37 +0200, Cédric Le Goater wrote:
> This should also allow probing to fail when a pca955x chip is not
> found on a I2C bus.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Thanks Cédric. I've applied this to dev-4.10, fixing up the conflicts
with my series. I felt this was reasonable given we've both been
discussing the changes.
Andrew
> ---
> Changes since v1:
>
> - introduced a pca955x_set_value() routine returning an errno
> - modified pca955x_gpio_direction_{in,out}put to return an errno
>
> drivers/leds/leds-pca955x.c | 114 ++++++++++++++++++++++++++++++++------------
> 1 file changed, 83 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
> index 09303fd1fdc6..905729191d3e 100644
> --- a/drivers/leds/leds-pca955x.c
> +++ b/drivers/leds/leds-pca955x.c
> @@ -165,13 +165,18 @@ static inline u8 pca955x_ledsel(u8 oldval, int led_num, int state)
> * Write to frequency prescaler register, used to program the
> * period of the PWM output. period = (PSCx + 1) / 38
> */
> -static void pca955x_write_psc(struct i2c_client *client, int n, u8 val)
> +static int pca955x_write_psc(struct i2c_client *client, int n, u8 val)
> {
> > struct pca955x *pca955x = i2c_get_clientdata(client);
> > + int ret;
>
> > - i2c_smbus_write_byte_data(client,
> > + ret = i2c_smbus_write_byte_data(client,
> > pca95xx_num_input_regs(pca955x->chipdef->bits) + 2*n,
> > val);
> > + if (ret < 0)
> > + dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n",
> > + __func__, n, val, ret);
> > + return ret;
> }
>
> /*
> @@ -181,38 +186,56 @@ static void pca955x_write_psc(struct i2c_client *client, int n, u8 val)
> *
> * Duty cycle is (256 - PWMx) / 256
> */
> -static void pca955x_write_pwm(struct i2c_client *client, int n, u8 val)
> +static int pca955x_write_pwm(struct i2c_client *client, int n, u8 val)
> {
> > struct pca955x *pca955x = i2c_get_clientdata(client);
> > + int ret;
>
> > - i2c_smbus_write_byte_data(client,
> > + ret = i2c_smbus_write_byte_data(client,
> > pca95xx_num_input_regs(pca955x->chipdef->bits) + 1 + 2*n,
> > val);
> > + if (ret < 0)
> > + dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n",
> > + __func__, n, val, ret);
> > + return ret;
> }
>
> /*
> * Write to LED selector register, which determines the source that
> * drives the LED output.
> */
> -static void pca955x_write_ls(struct i2c_client *client, int n, u8 val)
> +static int pca955x_write_ls(struct i2c_client *client, int n, u8 val)
> {
> > struct pca955x *pca955x = i2c_get_clientdata(client);
> > + int ret;
>
> > - i2c_smbus_write_byte_data(client,
> > + ret = i2c_smbus_write_byte_data(client,
> > pca95xx_num_input_regs(pca955x->chipdef->bits) + 4 + n,
> > val);
> > + if (ret < 0)
> > + dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n",
> > + __func__, n, val, ret);
> > + return ret;
> }
>
> /*
> * Read the LED selector register, which determines the source that
> * drives the LED output.
> */
> -static u8 pca955x_read_ls(struct i2c_client *client, int n)
> +static int pca955x_read_ls(struct i2c_client *client, int n, u8 *val)
> {
> > struct pca955x *pca955x = i2c_get_clientdata(client);
> > + int ret;
>
> > - return (u8) i2c_smbus_read_byte_data(client,
> > + ret = i2c_smbus_read_byte_data(client,
> > pca95xx_num_input_regs(pca955x->chipdef->bits) + 4 + n);
> > + if (ret < 0) {
> > + dev_err(&client->dev, "%s: reg 0x%x, err %d\n",
> > + __func__, n, ret);
> > + return ret;
> > + }
> > + *val = (u8)ret;
> > + return 0;
> }
>
> static int pca955x_led_set(struct led_classdev *led_cdev,
> @@ -223,6 +246,7 @@ static int pca955x_led_set(struct led_classdev *led_cdev,
> > u8 ls;
> > > int chip_ls; /* which LSx to use (0-3 potentially) */
> > > int ls_led; /* which set of bits within LSx to use (0-3) */
> > + int ret;
>
> > pca955x_led = container_of(led_cdev, struct pca955x_led, led_cdev);
> > pca955x = pca955x_led->pca955x;
> @@ -232,7 +256,9 @@ static int pca955x_led_set(struct led_classdev *led_cdev,
>
> > mutex_lock(&pca955x->lock);
>
> > - ls = pca955x_read_ls(pca955x->client, chip_ls);
> > + ret = pca955x_read_ls(pca955x->client, chip_ls, &ls);
> > + if (ret)
> > + goto out;
>
> > switch (value) {
> > case LED_FULL:
> @@ -252,26 +278,37 @@ static int pca955x_led_set(struct led_classdev *led_cdev,
> > * OFF, HALF, or FULL. But, this is probably better than
> > * just turning off for all other values.
> > */
> > - pca955x_write_pwm(pca955x->client, 1,
> > - 255 - value);
> > + ret = pca955x_write_pwm(pca955x->client, 1, 255 - value);
> > + if (ret)
> > + goto out;
> > ls = pca955x_ledsel(ls, ls_led, PCA955X_LS_BLINK1);
> > break;
> > }
>
> > - pca955x_write_ls(pca955x->client, chip_ls, ls);
> > + ret = pca955x_write_ls(pca955x->client, chip_ls, ls);
>
> +out:
> > mutex_unlock(&pca955x->lock);
>
> > - return 0;
> > + return ret;
> }
>
> #ifdef CONFIG_LEDS_PCA955X_GPIO
> /*
> * Read the INPUT register, which contains the state of LEDs.
> */
> -static u8 pca955x_read_input(struct i2c_client *client, int n)
> +static int pca955x_read_input(struct i2c_client *client, int n, u8 *val)
> {
> > - return (u8)i2c_smbus_read_byte_data(client, n);
> > + int ret = i2c_smbus_read_byte_data(client, n);
> +
> > + if (ret < 0) {
> > + dev_err(&client->dev, "%s: reg 0x%x, err %d\n",
> > + __func__, n, ret);
> > + return ret;
> > + }
> > + *val = (u8)ret;
> > + return 0;
> +
> }
>
> static int pca955x_gpio_request_pin(struct gpio_chip *gc, unsigned int offset)
> @@ -285,23 +322,32 @@ static int pca955x_gpio_request_pin(struct gpio_chip *gc, unsigned int offset)
> > return -EBUSY;
> }
>
> -static void pca955x_gpio_set_value(struct gpio_chip *gc, unsigned int offset,
> > - int val)
> +static int pca955x_set_value(struct gpio_chip *gc, unsigned int offset,
> > + int val)
> {
> > struct pca955x *pca955x = gpiochip_get_data(gc);
> > struct pca955x_led *led = &pca955x->leds[offset];
>
> > if (val)
> > - pca955x_led_set(&led->led_cdev, LED_FULL);
> > + return pca955x_led_set(&led->led_cdev, LED_FULL);
> > else
> > - pca955x_led_set(&led->led_cdev, LED_OFF);
> > + return pca955x_led_set(&led->led_cdev, LED_OFF);
> +}
> +
> +static void pca955x_gpio_set_value(struct gpio_chip *gc, unsigned int offset,
> > + int val)
> +{
> > + pca955x_set_value(gc, offset, val);
> }
>
> static int pca955x_gpio_get_value(struct gpio_chip *gc, unsigned int offset)
> {
> > struct pca955x *pca955x = gpiochip_get_data(gc);
> > struct pca955x_led *led = &pca955x->leds[offset];
> > - u8 reg = pca955x_read_input(pca955x->client, led->led_num / 8);
> > + u8 reg = 0;
> +
> > + /* There is nothing we can do about errors */
> > + pca955x_read_input(pca955x->client, led->led_num / 8, ®);
>
> > return !!(reg & (1 << (led->led_num % 8)));
> }
> @@ -310,17 +356,13 @@ static int pca955x_gpio_direction_input(struct gpio_chip *gc,
> > unsigned int offset)
> {
> > /* To use as input ensure pin is not driven */
> > - pca955x_gpio_set_value(gc, offset, 0);
> -
> > - return 0;
> > + return pca955x_set_value(gc, offset, 0);
> }
>
> static int pca955x_gpio_direction_output(struct gpio_chip *gc,
> > unsigned int offset, int val)
> {
> > - pca955x_gpio_set_value(gc, offset, val);
> -
> > - return 0;
> > + return pca955x_set_value(gc, offset, val);
> }
> #endif /* CONFIG_LEDS_PCA955X_GPIO */
>
> @@ -495,19 +537,29 @@ static int pca955x_probe(struct i2c_client *client,
> > return err;
>
> > /* Turn off LED */
> > - pca955x_led_set(&pca955x_led->led_cdev, LED_OFF);
> > + err = pca955x_led_set(&pca955x_led->led_cdev, LED_OFF);
> > + if (err)
> > + return err;
> > }
> > }
>
> > /* PWM0 is used for half brightness or 50% duty cycle */
> > - pca955x_write_pwm(client, 0, 255-LED_HALF);
> > + err = pca955x_write_pwm(client, 0, 255 - LED_HALF);
> > + if (err)
> > + return err;
>
> > /* PWM1 is used for variable brightness, default to OFF */
> > - pca955x_write_pwm(client, 1, 0);
> > + err = pca955x_write_pwm(client, 1, 0);
> > + if (err)
> > + return err;
>
> > /* Set to fast frequency so we do not see flashing */
> > - pca955x_write_psc(client, 0, 0);
> > - pca955x_write_psc(client, 1, 0);
> > + err = pca955x_write_psc(client, 0, 0);
> > + if (err)
> > + return err;
> > + err = pca955x_write_psc(client, 1, 0);
> > + if (err)
> > + return err;
>
> #ifdef CONFIG_LEDS_PCA955X_GPIO
> > if (ngpios) {
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH linux dev-4.10 v2] leds: pca955x: check for I2C errors
2017-08-29 5:45 ` Andrew Jeffery
@ 2017-08-29 5:58 ` Cédric Le Goater
0 siblings, 0 replies; 3+ messages in thread
From: Cédric Le Goater @ 2017-08-29 5:58 UTC (permalink / raw)
To: Andrew Jeffery, openbmc
On 08/29/2017 07:45 AM, Andrew Jeffery wrote:
> On Mon, 2017-08-28 at 08:37 +0200, Cédric Le Goater wrote:
>> This should also allow probing to fail when a pca955x chip is not
>> found on a I2C bus.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>
> Thanks Cédric. I've applied this to dev-4.10, fixing up the conflicts
> with my series. I felt this was reasonable given we've both been
> discussing the changes.
yes. Thanks,
C.
> Andrew
>
>> ---
>> Changes since v1:
>>
>> - introduced a pca955x_set_value() routine returning an errno
>> - modified pca955x_gpio_direction_{in,out}put to return an errno
>>
>> drivers/leds/leds-pca955x.c | 114 ++++++++++++++++++++++++++++++++------------
>> 1 file changed, 83 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
>> index 09303fd1fdc6..905729191d3e 100644
>> --- a/drivers/leds/leds-pca955x.c
>> +++ b/drivers/leds/leds-pca955x.c
>> @@ -165,13 +165,18 @@ static inline u8 pca955x_ledsel(u8 oldval, int led_num, int state)
>> * Write to frequency prescaler register, used to program the
>> * period of the PWM output. period = (PSCx + 1) / 38
>> */
>> -static void pca955x_write_psc(struct i2c_client *client, int n, u8 val)
>> +static int pca955x_write_psc(struct i2c_client *client, int n, u8 val)
>> {
>>> struct pca955x *pca955x = i2c_get_clientdata(client);
>>> + int ret;
>>
>>> - i2c_smbus_write_byte_data(client,
>>> + ret = i2c_smbus_write_byte_data(client,
>>> pca95xx_num_input_regs(pca955x->chipdef->bits) + 2*n,
>>> val);
>>> + if (ret < 0)
>>> + dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n",
>>> + __func__, n, val, ret);
>>> + return ret;
>> }
>>
>> /*
>> @@ -181,38 +186,56 @@ static void pca955x_write_psc(struct i2c_client *client, int n, u8 val)
>> *
>> * Duty cycle is (256 - PWMx) / 256
>> */
>> -static void pca955x_write_pwm(struct i2c_client *client, int n, u8 val)
>> +static int pca955x_write_pwm(struct i2c_client *client, int n, u8 val)
>> {
>>> struct pca955x *pca955x = i2c_get_clientdata(client);
>>> + int ret;
>>
>>> - i2c_smbus_write_byte_data(client,
>>> + ret = i2c_smbus_write_byte_data(client,
>>> pca95xx_num_input_regs(pca955x->chipdef->bits) + 1 + 2*n,
>>> val);
>>> + if (ret < 0)
>>> + dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n",
>>> + __func__, n, val, ret);
>>> + return ret;
>> }
>>
>> /*
>> * Write to LED selector register, which determines the source that
>> * drives the LED output.
>> */
>> -static void pca955x_write_ls(struct i2c_client *client, int n, u8 val)
>> +static int pca955x_write_ls(struct i2c_client *client, int n, u8 val)
>> {
>>> struct pca955x *pca955x = i2c_get_clientdata(client);
>>> + int ret;
>>
>>> - i2c_smbus_write_byte_data(client,
>>> + ret = i2c_smbus_write_byte_data(client,
>>> pca95xx_num_input_regs(pca955x->chipdef->bits) + 4 + n,
>>> val);
>>> + if (ret < 0)
>>> + dev_err(&client->dev, "%s: reg 0x%x, val 0x%x, err %d\n",
>>> + __func__, n, val, ret);
>>> + return ret;
>> }
>>
>> /*
>> * Read the LED selector register, which determines the source that
>> * drives the LED output.
>> */
>> -static u8 pca955x_read_ls(struct i2c_client *client, int n)
>> +static int pca955x_read_ls(struct i2c_client *client, int n, u8 *val)
>> {
>>> struct pca955x *pca955x = i2c_get_clientdata(client);
>>> + int ret;
>>
>>> - return (u8) i2c_smbus_read_byte_data(client,
>>> + ret = i2c_smbus_read_byte_data(client,
>>> pca95xx_num_input_regs(pca955x->chipdef->bits) + 4 + n);
>>> + if (ret < 0) {
>>> + dev_err(&client->dev, "%s: reg 0x%x, err %d\n",
>>> + __func__, n, ret);
>>> + return ret;
>>> + }
>>> + *val = (u8)ret;
>>> + return 0;
>> }
>>
>> static int pca955x_led_set(struct led_classdev *led_cdev,
>> @@ -223,6 +246,7 @@ static int pca955x_led_set(struct led_classdev *led_cdev,
>>> u8 ls;
>>>> int chip_ls; /* which LSx to use (0-3 potentially) */
>>>> int ls_led; /* which set of bits within LSx to use (0-3) */
>>> + int ret;
>>
>>> pca955x_led = container_of(led_cdev, struct pca955x_led, led_cdev);
>>> pca955x = pca955x_led->pca955x;
>> @@ -232,7 +256,9 @@ static int pca955x_led_set(struct led_classdev *led_cdev,
>>
>>> mutex_lock(&pca955x->lock);
>>
>>> - ls = pca955x_read_ls(pca955x->client, chip_ls);
>>> + ret = pca955x_read_ls(pca955x->client, chip_ls, &ls);
>>> + if (ret)
>>> + goto out;
>>
>>> switch (value) {
>>> case LED_FULL:
>> @@ -252,26 +278,37 @@ static int pca955x_led_set(struct led_classdev *led_cdev,
>>> * OFF, HALF, or FULL. But, this is probably better than
>>> * just turning off for all other values.
>>> */
>>> - pca955x_write_pwm(pca955x->client, 1,
>>> - 255 - value);
>>> + ret = pca955x_write_pwm(pca955x->client, 1, 255 - value);
>>> + if (ret)
>>> + goto out;
>>> ls = pca955x_ledsel(ls, ls_led, PCA955X_LS_BLINK1);
>>> break;
>>> }
>>
>>> - pca955x_write_ls(pca955x->client, chip_ls, ls);
>>> + ret = pca955x_write_ls(pca955x->client, chip_ls, ls);
>>
>> +out:
>>> mutex_unlock(&pca955x->lock);
>>
>>> - return 0;
>>> + return ret;
>> }
>>
>> #ifdef CONFIG_LEDS_PCA955X_GPIO
>> /*
>> * Read the INPUT register, which contains the state of LEDs.
>> */
>> -static u8 pca955x_read_input(struct i2c_client *client, int n)
>> +static int pca955x_read_input(struct i2c_client *client, int n, u8 *val)
>> {
>>> - return (u8)i2c_smbus_read_byte_data(client, n);
>>> + int ret = i2c_smbus_read_byte_data(client, n);
>> +
>>> + if (ret < 0) {
>>> + dev_err(&client->dev, "%s: reg 0x%x, err %d\n",
>>> + __func__, n, ret);
>>> + return ret;
>>> + }
>>> + *val = (u8)ret;
>>> + return 0;
>> +
>> }
>>
>> static int pca955x_gpio_request_pin(struct gpio_chip *gc, unsigned int offset)
>> @@ -285,23 +322,32 @@ static int pca955x_gpio_request_pin(struct gpio_chip *gc, unsigned int offset)
>>> return -EBUSY;
>> }
>>
>> -static void pca955x_gpio_set_value(struct gpio_chip *gc, unsigned int offset,
>>> - int val)
>> +static int pca955x_set_value(struct gpio_chip *gc, unsigned int offset,
>>> + int val)
>> {
>>> struct pca955x *pca955x = gpiochip_get_data(gc);
>>> struct pca955x_led *led = &pca955x->leds[offset];
>>
>>> if (val)
>>> - pca955x_led_set(&led->led_cdev, LED_FULL);
>>> + return pca955x_led_set(&led->led_cdev, LED_FULL);
>>> else
>>> - pca955x_led_set(&led->led_cdev, LED_OFF);
>>> + return pca955x_led_set(&led->led_cdev, LED_OFF);
>> +}
>> +
>> +static void pca955x_gpio_set_value(struct gpio_chip *gc, unsigned int offset,
>>> + int val)
>> +{
>>> + pca955x_set_value(gc, offset, val);
>> }
>>
>> static int pca955x_gpio_get_value(struct gpio_chip *gc, unsigned int offset)
>> {
>>> struct pca955x *pca955x = gpiochip_get_data(gc);
>>> struct pca955x_led *led = &pca955x->leds[offset];
>>> - u8 reg = pca955x_read_input(pca955x->client, led->led_num / 8);
>>> + u8 reg = 0;
>> +
>>> + /* There is nothing we can do about errors */
>>> + pca955x_read_input(pca955x->client, led->led_num / 8, ®);
>>
>>> return !!(reg & (1 << (led->led_num % 8)));
>> }
>> @@ -310,17 +356,13 @@ static int pca955x_gpio_direction_input(struct gpio_chip *gc,
>>> unsigned int offset)
>> {
>>> /* To use as input ensure pin is not driven */
>>> - pca955x_gpio_set_value(gc, offset, 0);
>> -
>>> - return 0;
>>> + return pca955x_set_value(gc, offset, 0);
>> }
>>
>> static int pca955x_gpio_direction_output(struct gpio_chip *gc,
>>> unsigned int offset, int val)
>> {
>>> - pca955x_gpio_set_value(gc, offset, val);
>> -
>>> - return 0;
>>> + return pca955x_set_value(gc, offset, val);
>> }
>> #endif /* CONFIG_LEDS_PCA955X_GPIO */
>>
>> @@ -495,19 +537,29 @@ static int pca955x_probe(struct i2c_client *client,
>>> return err;
>>
>>> /* Turn off LED */
>>> - pca955x_led_set(&pca955x_led->led_cdev, LED_OFF);
>>> + err = pca955x_led_set(&pca955x_led->led_cdev, LED_OFF);
>>> + if (err)
>>> + return err;
>>> }
>>> }
>>
>>> /* PWM0 is used for half brightness or 50% duty cycle */
>>> - pca955x_write_pwm(client, 0, 255-LED_HALF);
>>> + err = pca955x_write_pwm(client, 0, 255 - LED_HALF);
>>> + if (err)
>>> + return err;
>>
>>> /* PWM1 is used for variable brightness, default to OFF */
>>> - pca955x_write_pwm(client, 1, 0);
>>> + err = pca955x_write_pwm(client, 1, 0);
>>> + if (err)
>>> + return err;
>>
>>> /* Set to fast frequency so we do not see flashing */
>>> - pca955x_write_psc(client, 0, 0);
>>> - pca955x_write_psc(client, 1, 0);
>>> + err = pca955x_write_psc(client, 0, 0);
>>> + if (err)
>>> + return err;
>>> + err = pca955x_write_psc(client, 1, 0);
>>> + if (err)
>>> + return err;
>>
>> #ifdef CONFIG_LEDS_PCA955X_GPIO
>>> if (ngpios) {
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-08-29 5:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-28 6:37 [PATCH linux dev-4.10 v2] leds: pca955x: check for I2C errors Cédric Le Goater
2017-08-29 5:45 ` Andrew Jeffery
2017-08-29 5:58 ` Cédric Le Goater
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.