From: Eddie James <eajames@linux.ibm.com> To: Pavel Machek <pavel@ucw.cz> Cc: linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, patrick@stwcx.xyz, andy.shevchenko@gmail.com, openbmc@lists.ozlabs.org, joel@jms.id.au Subject: Re: [PATCH v3 3/4] leds: pca955x: Optimize probe led selection Date: Wed, 11 May 2022 14:43:57 -0500 [thread overview] Message-ID: <e3562109-789d-1c69-246b-670ded1a9d38@linux.ibm.com> (raw) In-Reply-To: <20220504172429.GF1623@bug> On 5/4/22 12:24, Pavel Machek wrote: > Hi! > >> 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 8 operations by accumulating the changes to the led >> selection and comparing with the previous value to write the >> selection if different. > We have regmap APIs. You are free to use them if you really care about > those few reads. Reimplementing them by hand is not acceptable. How big is > the seedup here? Hi, To be honest this was not about speed, it's about a problem I've observed where many operations in quick succession caused the PCA chip to NAK the i2c transfer. I'm not sure of the root cause yet. So reducing the number of operations helped. We are also probably going to carry a throttling patch, unless I can prove the PCA chip itself is at fault and needs slower transfers. I can try the regmap API and see if it helps though. Thanks, Eddie > > Best regards, > Pavel > >> @@ -554,6 +556,15 @@ static int pca955x_probe(struct i2c_client *client) >> init_data.devname_mandatory = false; >> init_data.devicename = "pca955x"; >> >> + nls = pca955x_num_led_regs(chip->bits); >> + for (i = 0; i < nls; ++i) { >> + err = pca955x_read_ls(pca955x, i, &ls1[i]); >> + if (err) >> + return err; >> + >> + ls2[i] = ls1[i]; >> + } >> + >> for (i = 0; i < chip->bits; i++) { >> pca955x_led = &pca955x->leds[i]; >> pca955x_led->led_num = i; >> @@ -624,6 +634,14 @@ static int pca955x_probe(struct i2c_client *client) >> } >> } >> >> + for (i = 0; i < nls; ++i) { >> + 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)
WARNING: multiple messages have this Message-ID (diff)
From: Eddie James <eajames@linux.ibm.com> To: Pavel Machek <pavel@ucw.cz> Cc: openbmc@lists.ozlabs.org, linux-kernel@vger.kernel.org, andy.shevchenko@gmail.com, joel@jms.id.au, linux-leds@vger.kernel.org Subject: Re: [PATCH v3 3/4] leds: pca955x: Optimize probe led selection Date: Wed, 11 May 2022 14:43:57 -0500 [thread overview] Message-ID: <e3562109-789d-1c69-246b-670ded1a9d38@linux.ibm.com> (raw) In-Reply-To: <20220504172429.GF1623@bug> On 5/4/22 12:24, Pavel Machek wrote: > Hi! > >> 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 8 operations by accumulating the changes to the led >> selection and comparing with the previous value to write the >> selection if different. > We have regmap APIs. You are free to use them if you really care about > those few reads. Reimplementing them by hand is not acceptable. How big is > the seedup here? Hi, To be honest this was not about speed, it's about a problem I've observed where many operations in quick succession caused the PCA chip to NAK the i2c transfer. I'm not sure of the root cause yet. So reducing the number of operations helped. We are also probably going to carry a throttling patch, unless I can prove the PCA chip itself is at fault and needs slower transfers. I can try the regmap API and see if it helps though. Thanks, Eddie > > Best regards, > Pavel > >> @@ -554,6 +556,15 @@ static int pca955x_probe(struct i2c_client *client) >> init_data.devname_mandatory = false; >> init_data.devicename = "pca955x"; >> >> + nls = pca955x_num_led_regs(chip->bits); >> + for (i = 0; i < nls; ++i) { >> + err = pca955x_read_ls(pca955x, i, &ls1[i]); >> + if (err) >> + return err; >> + >> + ls2[i] = ls1[i]; >> + } >> + >> for (i = 0; i < chip->bits; i++) { >> pca955x_led = &pca955x->leds[i]; >> pca955x_led->led_num = i; >> @@ -624,6 +634,14 @@ static int pca955x_probe(struct i2c_client *client) >> } >> } >> >> + for (i = 0; i < nls; ++i) { >> + 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)
next prev parent reply other threads:[~2022-05-11 19:44 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-11 16:20 [PATCH v3 0/4] leds: pca955x: Add HW blink support Eddie James 2022-04-11 16:20 ` Eddie James 2022-04-11 16:20 ` [PATCH v3 1/4] leds: pca955x: Refactor with helper functions and renaming Eddie James 2022-04-11 16:20 ` Eddie James 2022-04-11 16:20 ` [PATCH v3 2/4] leds: pca955x: Use pointers to driver data rather than I2C client Eddie James 2022-04-11 16:20 ` Eddie James 2022-04-11 16:20 ` [PATCH v3 3/4] leds: pca955x: Optimize probe led selection Eddie James 2022-04-11 16:20 ` Eddie James 2022-05-04 17:24 ` Pavel Machek 2022-05-04 17:24 ` Pavel Machek 2022-05-11 19:43 ` Eddie James [this message] 2022-05-11 19:43 ` Eddie James 2022-04-11 16:20 ` [PATCH v3 4/4] leds: pca955x: Add HW blink support Eddie James 2022-04-11 16:20 ` Eddie James 2022-05-04 17:24 ` Pavel Machek 2022-05-04 17:24 ` Pavel Machek 2022-05-11 19:54 ` Eddie James 2022-05-11 19:54 ` Eddie James 2022-04-11 17:06 ` [PATCH v3 0/4] " Andy Shevchenko 2022-04-11 17:06 ` Andy Shevchenko 2022-04-12 11:22 [PATCH v3 4/4] " kernel test robot 2022-04-12 11:33 ` Dan Carpenter 2022-04-12 11:33 ` Dan Carpenter 2022-04-12 11:33 ` Dan Carpenter
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=e3562109-789d-1c69-246b-670ded1a9d38@linux.ibm.com \ --to=eajames@linux.ibm.com \ --cc=andy.shevchenko@gmail.com \ --cc=joel@jms.id.au \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-leds@vger.kernel.org \ --cc=openbmc@lists.ozlabs.org \ --cc=patrick@stwcx.xyz \ --cc=pavel@ucw.cz \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.