On Thu, 30 Jan 2025, Eddie James wrote:
> Previously, the probe function might do up to 32 reads and writes
> to the same 4 registers to program the led selection. Reduce this to
> a maximum of 5 operations by accumulating the changes to the led
> selection and comparing with the previous value to write the
> selection if different.
>
> Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> ---
> drivers/leds/leds-pca955x.c | 38 +++++++++++++++++++++++++++----------
> 1 file changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
> index 4990f8aff6d16..8bdebc14ea2e6 100644
> --- a/drivers/leds/leds-pca955x.c
> +++ b/drivers/leds/leds-pca955x.c
> @@ -446,7 +446,9 @@ static int pca955x_probe(struct i2c_client *client)
> struct led_classdev *led;
> struct led_init_data init_data;
> struct i2c_adapter *adapter;
> - int i, err;
> + int i, bit, err, nls, reg;
> + u8 ls1[4];
> + u8 ls2[4];
> struct pca955x_platform_data *pdata;
> bool set_default_label = false;
> bool keep_pwm = false;
> @@ -504,6 +506,17 @@ static int pca955x_probe(struct i2c_client *client)
> init_data.devname_mandatory = false;
> init_data.devicename = "pca955x";
>
> + nls = pca955x_num_led_regs(chip->bits);
> + /* use auto-increment feature to read all the led selectors at once */
Sentences / comment should start with an upper-case char.
And acronyms should be capitalised too, so it's "LED".
> + err = i2c_smbus_read_i2c_block_data(client,
> + 0x10 | (pca955x_num_input_regs(chip->bits) + 4), nls,
> + ls1);
> + if (err < 0)
> + return err;
> +
> + for (i = 0; i < nls; ++i)
Why the need to pre-increment? Can you rejig this to be more consistent
with the way we usually format for loops please?
> + ls2[i] = ls1[i];
> +
> for (i = 0; i < chip->bits; i++) {
> pca955x_led = &pca955x->leds[i];
> pca955x_led->led_num = i;
> @@ -515,19 +528,16 @@ static int pca955x_probe(struct i2c_client *client)
> case PCA955X_TYPE_GPIO:
> break;
> case PCA955X_TYPE_LED:
> + bit = i % 4;
> + reg = i / 4;
> led = &pca955x_led->led_cdev;
> led->brightness_set_blocking = pca955x_led_set;
> led->brightness_get = pca955x_led_get;
>
> - if (pdata->leds[i].default_state == LEDS_DEFSTATE_OFF) {
> - err = pca955x_led_set(led, LED_OFF);
> - if (err)
> - return err;
> - } else if (pdata->leds[i].default_state == LEDS_DEFSTATE_ON) {
> - err = pca955x_led_set(led, LED_FULL);
> - if (err)
> - return err;
> - }
> + if (pdata->leds[i].default_state == LEDS_DEFSTATE_OFF)
> + ls2[reg] = pca955x_ledsel(ls2[reg], bit, PCA955X_LS_LED_OFF);
> + else if (pdata->leds[i].default_state == LEDS_DEFSTATE_ON)
> + ls2[reg] = pca955x_ledsel(ls2[reg], bit, PCA955X_LS_LED_ON);
>
> init_data.fwnode = pdata->leds[i].fwnode;
>
> @@ -571,6 +581,14 @@ static int pca955x_probe(struct i2c_client *client)
> }
> }
>
> + for (i = 0; i < nls; ++i) {
As above.
> + if (ls1[i] != ls2[i]) {
> + err = pca955x_write_ls(pca955x, i, ls2[i]);
> + if (err)
> + return err;
> + }
> + }
> +
> /* PWM0 is used for half brightness or 50% duty cycle */
> err = pca955x_write_pwm(pca955x, 0, 255 - LED_HALF);
> if (err)
> --
> 2.43.5
>
--
Lee Jones [李琼斯]
