| Message ID | 20220310220932.140973-1-jsd@semihalf.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,-next] i2c: designware: Remove code duplication | expand |
On 3/11/22 00:09, Jan Dabros wrote: > Simplify code by moving common part to one function. > > Signed-off-by: Jan Dabros <jsd@semihalf.com> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > v1->v2: > * Add kudos for Andy who suggested this change > * Get rid of extra function and move common code to psp_send_i2c_req > * Update commit message and commit title > (was "i2c:designware: Add helper to remove redundancy") > drivers/i2c/busses/i2c-designware-amdpsp.c | 35 ++++++++++------------ > 1 file changed, 15 insertions(+), 20 deletions(-) > Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
On Thu, Mar 10, 2022 at 11:09:32PM +0100, Jan Dabros wrote: > Simplify code by moving common part to one function. LGTM Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Jan Dabros <jsd@semihalf.com> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > v1->v2: > * Add kudos for Andy who suggested this change > * Get rid of extra function and move common code to psp_send_i2c_req > * Update commit message and commit title > (was "i2c:designware: Add helper to remove redundancy") > drivers/i2c/busses/i2c-designware-amdpsp.c | 35 ++++++++++------------ > 1 file changed, 15 insertions(+), 20 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-designware-amdpsp.c b/drivers/i2c/busses/i2c-designware-amdpsp.c > index c64e459afb5c..5c32255c3239 100644 > --- a/drivers/i2c/busses/i2c-designware-amdpsp.c > +++ b/drivers/i2c/busses/i2c-designware-amdpsp.c > @@ -214,17 +214,28 @@ static int psp_send_i2c_req(enum psp_i2c_req_type i2c_req_type) > PSP_I2C_REQ_RETRY_DELAY_US, > PSP_I2C_REQ_RETRY_CNT * PSP_I2C_REQ_RETRY_DELAY_US, > 0, req); > - if (ret) > + if (ret) { > + dev_err(psp_i2c_dev, "Timed out waiting for PSP to %s I2C bus\n", > + (i2c_req_type == PSP_I2C_REQ_ACQUIRE) ? > + "release" : "acquire"); > goto cleanup; > + } > > ret = status; > - if (ret) > + if (ret) { > + dev_err(psp_i2c_dev, "PSP communication error\n"); > goto cleanup; > + } > > dev_dbg(psp_i2c_dev, "Request accepted by PSP after %ums\n", > jiffies_to_msecs(jiffies - start)); > > cleanup: > + if (ret) { > + dev_err(psp_i2c_dev, "Assume i2c bus is for exclusive host usage\n"); > + psp_i2c_mbox_fail = true; > + } > + > kfree(req); > return ret; > } > @@ -249,16 +260,8 @@ static int psp_acquire_i2c_bus(void) > }; > > status = psp_send_i2c_req(PSP_I2C_REQ_ACQUIRE); > - if (status) { > - if (status == -ETIMEDOUT) > - dev_err(psp_i2c_dev, "Timed out waiting for PSP to release I2C bus\n"); > - else > - dev_err(psp_i2c_dev, "PSP communication error\n"); > - > - dev_err(psp_i2c_dev, "Assume i2c bus is for exclusive host usage\n"); > - psp_i2c_mbox_fail = true; > + if (status) > goto cleanup; > - } > > psp_i2c_sem_acquired = jiffies; > psp_i2c_access_count++; > @@ -294,16 +297,8 @@ static void psp_release_i2c_bus(void) > > /* Send a release command to PSP */ > status = psp_send_i2c_req(PSP_I2C_REQ_RELEASE); > - if (status) { > - if (status == -ETIMEDOUT) > - dev_err(psp_i2c_dev, "Timed out waiting for PSP to acquire I2C bus\n"); > - else > - dev_err(psp_i2c_dev, "PSP communication error\n"); > - > - dev_err(psp_i2c_dev, "Assume i2c bus is for exclusive host usage\n"); > - psp_i2c_mbox_fail = true; > + if (status) > goto cleanup; > - } > > dev_dbg(psp_i2c_dev, "PSP semaphore held for %ums\n", > jiffies_to_msecs(jiffies - psp_i2c_sem_acquired)); > -- > 2.31.0 >
On Thu, Mar 10, 2022 at 11:09:32PM +0100, Jan Dabros wrote: > Simplify code by moving common part to one function. > > Signed-off-by: Jan Dabros <jsd@semihalf.com> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Is it really based on for-next? I can't apply the patch. Am I missing something?
Hi Wolfram, pt., 11 mar 2022 o 21:51 Wolfram Sang <wsa@kernel.org> napisał(a): > > On Thu, Mar 10, 2022 at 11:09:32PM +0100, Jan Dabros wrote: > > Simplify code by moving common part to one function. > > > > Signed-off-by: Jan Dabros <jsd@semihalf.com> > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Is it really based on for-next? I can't apply the patch. Am I missing > something? I checked this once again and I mistakenly used the old linux-next baseline. Actually the cherry-pick of the above patch was successful, but I confirmed "git am" is not happy because of the missing semicolon in one of the lines from context. Baseline for v2 doesn't include 1e4fe5430b: "i2c: designware: remove unneeded semicolon" thus the problem. Sorry for the confusion, I will send v3 rebased on top of fresh -next. Best Regards, Jan
On 3/11/22 22:51, Wolfram Sang wrote: > On Thu, Mar 10, 2022 at 11:09:32PM +0100, Jan Dabros wrote: >> Simplify code by moving common part to one function. >> >> Signed-off-by: Jan Dabros <jsd@semihalf.com> >> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Is it really based on for-next? I can't apply the patch. Am I missing > something? > Looks like Jan's patch is done without commit 1e4fe5430bd7 ("i2c: designware: remove unneeded semicolon") and that causes the git am not able to apply it while plain patch is not so strict "Hunk #2 succeeded at 260 with fuzz 1.". Jarkko
diff --git a/drivers/i2c/busses/i2c-designware-amdpsp.c b/drivers/i2c/busses/i2c-designware-amdpsp.c index c64e459afb5c..5c32255c3239 100644 --- a/drivers/i2c/busses/i2c-designware-amdpsp.c +++ b/drivers/i2c/busses/i2c-designware-amdpsp.c @@ -214,17 +214,28 @@ static int psp_send_i2c_req(enum psp_i2c_req_type i2c_req_type) PSP_I2C_REQ_RETRY_DELAY_US, PSP_I2C_REQ_RETRY_CNT * PSP_I2C_REQ_RETRY_DELAY_US, 0, req); - if (ret) + if (ret) { + dev_err(psp_i2c_dev, "Timed out waiting for PSP to %s I2C bus\n", + (i2c_req_type == PSP_I2C_REQ_ACQUIRE) ? + "release" : "acquire"); goto cleanup; + } ret = status; - if (ret) + if (ret) { + dev_err(psp_i2c_dev, "PSP communication error\n"); goto cleanup; + } dev_dbg(psp_i2c_dev, "Request accepted by PSP after %ums\n", jiffies_to_msecs(jiffies - start)); cleanup: + if (ret) { + dev_err(psp_i2c_dev, "Assume i2c bus is for exclusive host usage\n"); + psp_i2c_mbox_fail = true; + } + kfree(req); return ret; } @@ -249,16 +260,8 @@ static int psp_acquire_i2c_bus(void) }; status = psp_send_i2c_req(PSP_I2C_REQ_ACQUIRE); - if (status) { - if (status == -ETIMEDOUT) - dev_err(psp_i2c_dev, "Timed out waiting for PSP to release I2C bus\n"); - else - dev_err(psp_i2c_dev, "PSP communication error\n"); - - dev_err(psp_i2c_dev, "Assume i2c bus is for exclusive host usage\n"); - psp_i2c_mbox_fail = true; + if (status) goto cleanup; - } psp_i2c_sem_acquired = jiffies; psp_i2c_access_count++; @@ -294,16 +297,8 @@ static void psp_release_i2c_bus(void) /* Send a release command to PSP */ status = psp_send_i2c_req(PSP_I2C_REQ_RELEASE); - if (status) { - if (status == -ETIMEDOUT) - dev_err(psp_i2c_dev, "Timed out waiting for PSP to acquire I2C bus\n"); - else - dev_err(psp_i2c_dev, "PSP communication error\n"); - - dev_err(psp_i2c_dev, "Assume i2c bus is for exclusive host usage\n"); - psp_i2c_mbox_fail = true; + if (status) goto cleanup; - } dev_dbg(psp_i2c_dev, "PSP semaphore held for %ums\n", jiffies_to_msecs(jiffies - psp_i2c_sem_acquired));
Simplify code by moving common part to one function. Signed-off-by: Jan Dabros <jsd@semihalf.com> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- v1->v2: * Add kudos for Andy who suggested this change * Get rid of extra function and move common code to psp_send_i2c_req * Update commit message and commit title (was "i2c:designware: Add helper to remove redundancy") drivers/i2c/busses/i2c-designware-amdpsp.c | 35 ++++++++++------------ 1 file changed, 15 insertions(+), 20 deletions(-)