Re: [PATCH] i2c: omap: Don't do pm_runtime_resume in .remove()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Andreas,

On Sun, Apr 02, 2023 at 10:50:01PM +0200, Andreas Kemnade wrote:
> On Sun,  2 Apr 2023 12:55:18 +0200
> Uwe Kleine-König         <u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
> 
> > One of the first things that the driver's pm_runtime_resume callback
> > does is to write zero to the OMAP_I2C_CON_REG register.
> > So there is no need to have the device resumed just to write to the
> > OMAP_I2C_CON_REG register and the call to pm_runtime_resume_and_get()
> > can be dropped.
> > 
> > The intended side effect of this commit is to remove an error path of
> > the function resulting in the remove callback returning a mostly ignored
> > error code. This prepares changing the prototype of struct
> > platform_driver's remove callback to return void.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> 
> As far as I have understand the code that runtime resume is needed
> for enabling clocks to access the device

The only HW access the .remove() callback does is

	omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, 0);

. The driver's runtime resume callback looks as follows:

        pinctrl_pm_select_default_state(dev);

        if (!omap->regs)
                return 0;

        __omap_i2c_init(omap);

and the first thing in in __omap_i2c_init is (also) writing a zero to
OMAP_I2C_CON_REG.

I wouldn't expect that the pinctrl call is a precondition for the
register access?! The check for omap->regs is somehow strange, because
other code locations that do register access just assume that ->regs is
non-NULL.

So if there is some clk handling necessary before the register access,
I'm not aware where it's hidden. Is there some bus or omap specific code
that ensures clk handling?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux