Hi Claudiu,
> diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
> index 83e4d5e14ab6..002b11b020fa 100644
> --- a/drivers/i2c/busses/i2c-riic.c
> +++ b/drivers/i2c/busses/i2c-riic.c
> @@ -113,6 +113,8 @@ struct riic_irq_desc {
> char *name;
> };
>
> +static const char * const riic_rpm_err_msg = "Failed to runtime resume";
Please, don't do this. Much clearer to write the message
explicitly.
> +
> static inline void riic_writeb(struct riic_dev *riic, u8 val, u8 offset)
> {
> writeb(val, riic->base + riic->info->regs[offset]);
> @@ -133,10 +135,14 @@ static int riic_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> struct riic_dev *riic = i2c_get_adapdata(adap);
> struct device *dev = adap->dev.parent;
> unsigned long time_left;
> - int i;
> + int i, ret;
> u8 start_bit;
>
> - pm_runtime_get_sync(dev);
> + ret = pm_runtime_resume_and_get(dev);
In principle I like the error message to be always checked and I
will always approve it. Whenever there is a return value, even
when we are sure it's always '0', it needs to be checked.
I had lots of discussions in the past about this topic but I
haven't always found support. I'd love to have the ack from a
renesas maintainer here.
> + if (ret) {
> + dev_err(dev, riic_rpm_err_msg);
> + return ret;
> + }
>
> if (riic_readb(riic, RIIC_ICCR2) & ICCR2_BBSY) {
> riic->err = -EBUSY;
...
> @@ -498,11 +509,18 @@ static void riic_i2c_remove(struct platform_device *pdev)
> {
> struct riic_dev *riic = platform_get_drvdata(pdev);
> struct device *dev = &pdev->dev;
> + int ret;
>
> - pm_runtime_get_sync(dev);
> - riic_writeb(riic, 0, RIIC_ICIER);
> - pm_runtime_put(dev);
> i2c_del_adapter(&riic->adapter);
You swapped the position of this. Please put the
i2c_del_adapter() at the end.
Thanks,
Andi
> +
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret) {
> + dev_err(dev, riic_rpm_err_msg);
> + } else {
> + riic_writeb(riic, 0, RIIC_ICIER);
> + pm_runtime_put(dev);
> + }
> +
> pm_runtime_disable(dev);
> }
>
> --
> 2.39.2
>
