hwmon: (pmbus/ibm-cffps) Fixes for the LED subsystem
cover hwmon: (pmbus/ibm-cffps) Fixes for the LED subsystem
This series fixes a "scheduling while atomic" bug in the CFFPS driver and some incorrect behavior with the LED blink functionality.
Eddie James (2):
hwmon: (pmbus/ibm-cffps) Switch LEDs to blocking brightness call
hwmon: (pmbus/ibm-cffps) Fix LED blink behavior
drivers/hwmon/pmbus/ibm-cffps.c | 37 ++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 12 deletions(-)
Signature · 1 line
2.23.0
PATCH 1/2 hwmon: (pmbus/ibm-cffps) Switch LEDs to blocking brightness call
Since i2c_smbus functions can sleep, the brightness setting function for this driver must be the blocking version to avoid scheduling while atomic.
--- drivers/hwmon/pmbus/ibm-cffps.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/hwmon/pmbus/ibm-cffps.c +6 -4
@@ -297,8 +297,8 @@ static int ibm_cffps_read_word_data(struct i2c_client *client, int page,return rc; } -static void ibm_cffps_led_brightness_set (struct led_classdev *led_cdev, -enum led_brightness brightness) +static int ibm_cffps_led_brightness_set (struct led_classdev *led_cdev, +enum led_brightness brightness) {int rc; struct ibm_cffps *psu = container_of(led_cdev, struct ibm_cffps, led);
@@ -316,9 +316,11 @@ static void ibm_cffps_led_brightness_set(struct led_classdev *led_cdev, rc =i2c_smbus_write_byte_data (psu->client,CFFPS_SYS_CONFIG_CMD , psu->led_state);if (rc <0 ) -return ; +return rc; led_cdev->brightness = brightness; + +return 0 ; } static int ibm_cffps_led_blink_set(struct led_classdev *led_cdev,
@@ -356,7 +358,7 @@ static void ibm_cffps_create_led_class(struct ibm_cffps *psu) client->addr); psu->led.name = psu->led_name; psu->led.max_brightness =LED_FULL ; - psu->led.brightness_set = ibm_cffps_led_brightness_set; + psu->led.brightness_set_blocking = ibm_cffps_led_brightness_set; psu->led.blink_set = ibm_cffps_led_blink_set; rc =devm_led_classdev_register (dev, &psu->led);
Signature · 1 line
2.23.0
Re: [PATCH 1/2] hwmon: (pmbus/ibm-cffps) Switch LEDs to blocking brightness call
Quoted 5 lines · Eddie James
Since
i2c_smbusfunctions can sleep, the brightness setting function for this driver must be the blocking version to avoid scheduling while atomic.Signed-off-by Eddie James <[email protected]>
Applied.
Thanks,
Guenter
Quoted 41 lines
--- drivers/hwmon/pmbus/ibm-cffps.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)drivers/hwmon/pmbus/ibm-cffps.c +6
-4@@ -297,8 +297,8 @@ static int ibm_cffps_read_word_data(struct i2c_client *client, int page,return rc; } -static void ibm_cffps_led_brightness_set (struct led_classdev *led_cdev, -enum led_brightness brightness) +static int ibm_cffps_led_brightness_set (struct led_classdev *led_cdev, +enum led_brightness brightness) {int rc; struct ibm_cffps *psu = container_of(led_cdev, struct ibm_cffps, led);@@ -316,9 +316,11 @@ static void ibm_cffps_led_brightness_set(struct led_classdev *led_cdev, rc =i2c_smbus_write_byte_data (psu->client,CFFPS_SYS_CONFIG_CMD , psu->led_state);if (rc <0 ) -return ; +return rc; led_cdev->brightness = brightness; + +return 0 ; } static int ibm_cffps_led_blink_set(struct led_classdev *led_cdev,@@ -356,7 +358,7 @@ static void ibm_cffps_create_led_class(struct ibm_cffps *psu) client->addr); psu->led.name = psu->led_name; psu->led.max_brightness =LED_FULL ; - psu->led.brightness_set = ibm_cffps_led_brightness_set; + psu->led.brightness_set_blocking = ibm_cffps_led_brightness_set; psu->led.blink_set = ibm_cffps_led_blink_set; rc =devm_led_classdev_register (dev, &psu->led);
PATCH 2/2 hwmon: (pmbus/ibm-cffps) Fix LED blink behavior
In reply to Eddie JamesThe LED blink_set function incorrectly did not tell the PSU LED to blink if brightness was LED_OFF. Fix this, and also correct the LED_OFF command data, which should give control of the LED back to the PSU firmware. Also prevent I2C failures from getting the driver LED state out of sync and add some dev_dbg statements.
--- drivers/hwmon/pmbus/ibm-cffps.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)
drivers/hwmon/pmbus/ibm-cffps.c +19 -8
@@ -44,9 +44,13 @@#define CFFPS_MFR_VAUX_FAULT BIT(6)#define CFFPS_MFR_CURRENT_SHARE_WARNING BIT(7) +/* +* LED off state actually relinquishes LED control to PSU firmware, so it can +* turn on the LED for faults. +*/ +#define CFFPS_LED_OFF 0#define CFFPS_LED_BLINK BIT(0)#define CFFPS_LED_ON BIT(1) -#define CFFPS_LED_OFF BIT(2)#define CFFPS_BLINK_RATE_MS 250enum {
@@ -301,23 +305,31 @@ static int ibm_cffps_led_brightness_set(struct led_classdev *led_cdev, enum led_brightness brightness) {int rc; +u8 next_led_state; struct ibm_cffps *psu = container_of(led_cdev, struct ibm_cffps, led);if (brightness ==LED_OFF ) { - psu->led_state =CFFPS_LED_OFF ; + next_led_state =CFFPS_LED_OFF ; }else { brightness =LED_FULL ; +if (psu->led_state !=CFFPS_LED_BLINK ) - psu->led_state =CFFPS_LED_ON ; + next_led_state =CFFPS_LED_ON ; +else + next_led_state =CFFPS_LED_BLINK ; } +dev_dbg (&psu->client->dev,"LED brightness set: %d. Command: %d.\n" , + brightness, next_led_state); +pmbus_set_page (psu->client,0 ); rc =i2c_smbus_write_byte_data (psu->client,CFFPS_SYS_CONFIG_CMD , - psu->led_state); + next_led_state);if (rc <0 )return rc; + psu->led_state = next_led_state; led_cdev->brightness = brightness;return 0 ;
@@ -330,10 +342,7 @@ static int ibm_cffps_led_blink_set(struct led_classdev *led_cdev,int rc; struct ibm_cffps *psu = container_of(led_cdev, struct ibm_cffps, led); - psu->led_state =CFFPS_LED_BLINK ; - -if (led_cdev->brightness ==LED_OFF ) -return 0 ; +dev_dbg (&psu->client->dev,"LED blink set.\n" );pmbus_set_page (psu->client,0 );
@@ -342,6 +351,8 @@ static int ibm_cffps_led_blink_set(struct led_classdev *led_cdev,if (rc <0 )return rc; + psu->led_state =CFFPS_LED_BLINK ; + led_cdev->brightness =LED_FULL ; *delay_on =CFFPS_BLINK_RATE_MS ; *delay_off =CFFPS_BLINK_RATE_MS ;
Signature · 1 line
2.23.0
Re: [PATCH 2/2] hwmon: (pmbus/ibm-cffps) Fix LED blink behavior
Quoted 7 lines · Eddie James
The LED
blink_setfunction incorrectly did not tell the PSU LED to blink if brightness wasLED_OFF. Fix this, and also correct theLED_OFFcommand data, which should give control of the LED back to the PSU firmware. Also prevent I2C failures from getting the driver LED state out of sync and add somedev_dbgstatements.Signed-off-by Eddie James <[email protected]>
Applied.
Thanks,
Guenter
Quoted 79 lines
--- drivers/hwmon/pmbus/ibm-cffps.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)drivers/hwmon/pmbus/ibm-cffps.c +19
-8@@ -44,9 +44,13 @@#define CFFPS_MFR_VAUX_FAULT BIT(6)#define CFFPS_MFR_CURRENT_SHARE_WARNING BIT(7) +/* +* LED off state actually relinquishes LED control to PSU firmware, so it can +* turn on the LED for faults. +*/ +#define CFFPS_LED_OFF 0#define CFFPS_LED_BLINK BIT(0)#define CFFPS_LED_ON BIT(1) -#define CFFPS_LED_OFF BIT(2)#define CFFPS_BLINK_RATE_MS 250enum {@@ -301,23 +305,31 @@ static int ibm_cffps_led_brightness_set(struct led_classdev *led_cdev, enum led_brightness brightness) {int rc; +u8 next_led_state; struct ibm_cffps *psu = container_of(led_cdev, struct ibm_cffps, led);if (brightness ==LED_OFF ) { - psu->led_state =CFFPS_LED_OFF ; + next_led_state =CFFPS_LED_OFF ; }else { brightness =LED_FULL ; +if (psu->led_state !=CFFPS_LED_BLINK ) - psu->led_state =CFFPS_LED_ON ; + next_led_state =CFFPS_LED_ON ; +else + next_led_state =CFFPS_LED_BLINK ; } +dev_dbg (&psu->client->dev,"LED brightness set: %d. Command: %d.\n" , + brightness, next_led_state); +pmbus_set_page (psu->client,0 ); rc =i2c_smbus_write_byte_data (psu->client,CFFPS_SYS_CONFIG_CMD , - psu->led_state); + next_led_state);if (rc <0 )return rc; + psu->led_state = next_led_state; led_cdev->brightness = brightness;return 0 ;@@ -330,10 +342,7 @@ static int ibm_cffps_led_blink_set(struct led_classdev *led_cdev,int rc; struct ibm_cffps *psu = container_of(led_cdev, struct ibm_cffps, led); - psu->led_state =CFFPS_LED_BLINK ; - -if (led_cdev->brightness ==LED_OFF ) -return 0 ; +dev_dbg (&psu->client->dev,"LED blink set.\n" );pmbus_set_page (psu->client,0 );@@ -342,6 +351,8 @@ static int ibm_cffps_led_blink_set(struct led_classdev *led_cdev,if (rc <0 )return rc; + psu->led_state =CFFPS_LED_BLINK ; + led_cdev->brightness =LED_FULL ; *delay_on =CFFPS_BLINK_RATE_MS ; *delay_off =CFFPS_BLINK_RATE_MS ;