When wait_for_completion_timeout() failed, error is assigned
'-ETIMEDOUT'. But this error code is never used. Return '-ETIMEDOUT'
directly to fix this problem.
Signed-off-by: Su Hui <[email protected]>
---
I'm not sure that return directly is true or not, maybe need some
process before return directly.
drivers/phy/motorola/phy-mapphone-mdm6600.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
index 1d567604b650..e84e3390bff0 100644
--- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
+++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
@@ -421,8 +421,8 @@ static int phy_mdm6600_device_power_on(struct phy_mdm6600 *ddata)
dev_info(ddata->dev, "Powered up OK\n");
} else {
ddata->enabled = false;
- error = -ETIMEDOUT;
dev_err(ddata->dev, "Timed out powering up\n");
+ return -ETIMEDOUT;
}
/* Reconfigure mode1 GPIO as input for OOB wake */
--
2.30.2
inv_mpu6050_sensor_show() can return -EINVAL or IIO_VAL_INT. Return the
true value rather than only return IIO_VAL_INT.
Signed-off-by: Su Hui <[email protected]>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 29f906c884bd..a9a5fb266ef1 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -749,13 +749,13 @@ inv_mpu6050_read_raw(struct iio_dev *indio_dev,
ret = inv_mpu6050_sensor_show(st, st->reg->gyro_offset,
chan->channel2, val);
mutex_unlock(&st->lock);
- return IIO_VAL_INT;
+ return ret;
case IIO_ACCEL:
mutex_lock(&st->lock);
ret = inv_mpu6050_sensor_show(st, st->reg->accl_offset,
chan->channel2, val);
mutex_unlock(&st->lock);
- return IIO_VAL_INT;
+ return ret;
default:
return -EINVAL;
--
2.30.2
On 2023/10/20 23:55, Jonathan Cameron wrote:
> What does this have to do with the phy: mapphone-mdm6600?
Oh, really sorry for this. My careless mistake :( .
> I'm not sure why inv_mpu6050_sensor_show() doesn't return
> the actual error code from the regmap_bulk_read() and instead replaces it
> with -EINVAL. Given you are tidying up this related issues perhaps change
> that as well?
>
> static int inv_mpu6050_sensor_show(struct inv_mpu6050_state *st, int reg,
> int axis, int *val)
> {
> int ind, result;
> __be16 d;
>
> ind = (axis - IIO_MOD_X) * 2;
> result = regmap_bulk_read(st->map, reg + ind, &d, sizeof(d));
> if (result)
> return -EINVAL;
> //Make this return result;
Sure, I will tidy up this, Thanks for your suggestion!
Su Hui
On 2023/10/23 09:33, Su Hui wrote:
> On 2023/10/20 23:55, Jonathan Cameron wrote:
>> I'm not sure why inv_mpu6050_sensor_show() doesn't return
>> the actual error code from the regmap_bulk_read() and instead
>> replaces it
>> with -EINVAL. Given you are tidying up this related issues perhaps
>> change
>> that as well?
>>
>> static int inv_mpu6050_sensor_show(struct inv_mpu6050_state *st, int
>> reg,
>> int axis, int *val)
>> {
>> int ind, result;
>> __be16 d;
>>
>> ind = (axis - IIO_MOD_X) * 2;
>> result = regmap_bulk_read(st->map, reg + ind, &d, sizeof(d));
>> if (result)
>> return -EINVAL;
>> //Make this return result;
>
> Sure, I will tidy up this, Thanks for your suggestion!
I'm not sure whether the caller could handler this when return
'result' rather than '-EINVAL'.
This is not a big problem, maybe we shouldn't modify this code.
Su Hui
On Mon, 23 Oct 2023 11:29:30 +0800
Su Hui <[email protected]> wrote:
> On 2023/10/23 09:33, Su Hui wrote:
> > On 2023/10/20 23:55, Jonathan Cameron wrote:
> >> I'm not sure why inv_mpu6050_sensor_show() doesn't return
> >> the actual error code from the regmap_bulk_read() and instead
> >> replaces it
> >> with -EINVAL. Given you are tidying up this related issues perhaps
> >> change
> >> that as well?
> >>
> >> static int inv_mpu6050_sensor_show(struct inv_mpu6050_state *st, int
> >> reg,
> >> int axis, int *val)
> >> {
> >> int ind, result;
> >> __be16 d;
> >>
> >> ind = (axis - IIO_MOD_X) * 2;
> >> result = regmap_bulk_read(st->map, reg + ind, &d, sizeof(d));
> >> if (result)
> >> return -EINVAL;
> >> //Make this return result;
> >
> > Sure, I will tidy up this, Thanks for your suggestion!
>
> I'm not sure whether the caller could handler this when return
> 'result' rather than '-EINVAL'.
>
> This is not a big problem, maybe we shouldn't modify this code.
If the ultimate caller (userspace in this case) doesn't handle other
error codes it won't work with lots of other devices and is broken.
So I doubt any code is that fragile. If we get a report of a regression
we can look into whether it's sensible to resolve it.
So fine to change this, but could be a separate patch.
J
>
> Su Hui
>
On 20-10-23, 17:14, Su Hui wrote:
> When wait_for_completion_timeout() failed, error is assigned
> '-ETIMEDOUT'. But this error code is never used. Return '-ETIMEDOUT'
> directly to fix this problem.
Where is patch 2/2?
>
> Signed-off-by: Su Hui <[email protected]>
> ---
>
> I'm not sure that return directly is true or not, maybe need some
> process before return directly.
>
> drivers/phy/motorola/phy-mapphone-mdm6600.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
> index 1d567604b650..e84e3390bff0 100644
> --- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
> +++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
> @@ -421,8 +421,8 @@ static int phy_mdm6600_device_power_on(struct phy_mdm6600 *ddata)
> dev_info(ddata->dev, "Powered up OK\n");
> } else {
> ddata->enabled = false;
> - error = -ETIMEDOUT;
> dev_err(ddata->dev, "Timed out powering up\n");
> + return -ETIMEDOUT;
> }
>
> /* Reconfigure mode1 GPIO as input for OOB wake */
> --
> 2.30.2
--
~Vinod
On 2023/11/16 19:26, Vinod Koul wrote:
> On 20-10-23, 17:14, Su Hui wrote:
>> When wait_for_completion_timeout() failed, error is assigned
>> '-ETIMEDOUT'. But this error code is never used. Return '-ETIMEDOUT'
>> directly to fix this problem.
> Where is patch 2/2?
Hi,
Sorry, I send a error patch 2/2 because of my carelessness.
This is the address of patch 2/2 which is irrelevant about this patch.
https://lore.kernel.org/all/[email protected]/
Su Hui
>> Signed-off-by: Su Hui <[email protected]>
>> ---
>>
>> I'm not sure that return directly is true or not, maybe need some
>> process before return directly.
>>
>> drivers/phy/motorola/phy-mapphone-mdm6600.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
>> index 1d567604b650..e84e3390bff0 100644
>> --- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
>> +++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
>> @@ -421,8 +421,8 @@ static int phy_mdm6600_device_power_on(struct phy_mdm6600 *ddata)
>> dev_info(ddata->dev, "Powered up OK\n");
>> } else {
>> ddata->enabled = false;
>> - error = -ETIMEDOUT;
>> dev_err(ddata->dev, "Timed out powering up\n");
>> + return -ETIMEDOUT;
>> }
>>
>> /* Reconfigure mode1 GPIO as input for OOB wake */
>> --
>> 2.30.2