Simplify code by moving common part to one function.
Signed-off-by: Jan Dabros <[email protected]>
Suggested-by: Andy Shevchenko <[email protected]>
---
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.
LGTM
Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Jan Dabros <[email protected]>
> Suggested-by: Andy Shevchenko <[email protected]>
> ---
> 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
>
--
With Best Regards,
Andy Shevchenko
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 <[email protected]>
> Suggested-by: Andy Shevchenko <[email protected]>
Is it really based on for-next? I can't apply the patch. Am I missing
something?
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 <[email protected]>
>> Suggested-by: Andy Shevchenko <[email protected]>
>
> 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
Hi Wolfram,
pt., 11 mar 2022 o 21:51 Wolfram Sang <[email protected]> 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 <[email protected]>
> > Suggested-by: Andy Shevchenko <[email protected]>
>
> 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