Skip to contentSkip to conversation

Does Rockchip RK808 driver unload work as intended?

DISCUSSION Matti Vaittinen 1 participant 2 messages Sep 7, 2018 – Sep 10, 2018 Common clk

lore
Share & tools
lore
#0 lore raw

Hi dee Ho peeps,

I was browsing through the clk drivers as I tried to do some cleaning. Few days ago I submitted a patch which would add devm variants for clkdev lookup registration. I also added devm of_provider registration for cases where it is actually the parent device which contains clock definitions in DT. This seems to be quite typical for MFDs.

While doing this I hit to Rockchip RK808 driver which seems to utilize oarent device (MFD dev) for pretty much all devm releasing. I wonder if this is safe? What happens if one tries to remove the RK808 clk module?

I guess the clk deregistration and cleanups are not ran as parent device stays there, right? But is the clk module and clk module code still unload? So won't clk operation pointers registered to clk core become invalid?

I guess I don't have any HW to test this mnyself. And as the driver has been there since 2014 - well, chances are the driver does work and I just don't get it =)

So can someone please shed some light on this? Is this a bug or am I just plain wrong?

Br,
Matti Vaittinen

#1 lore raw

Hello,

Quoted 8 lines · Matti Vaittinen

While doing this I hit to Rockchip RK808 driver which seems to utilize oarent device (MFD dev) for pretty much all devm releasing. I wonder if this is safe? What happens if one tries to remove the RK808 clk module?

I guess the clk deregistration and cleanups are not ran as parent device stays there, right? But is the clk module and clk module code still unload? So won't clk operation pointers registered to clk core become invalid?

Maybe it is a good idea to paste some code and add my thoughts as comments here.

static int rk808_clkout_probe(struct platform_device *pdev)

{
	/* Get the i2c device "driven by" the parent MFD driver */
        struct rk808 *rk808 = dev_get_drvdata(pdev->dev.parent);
        struct i2c_client *client = rk808->i2c;

	[snip]

	/* Allocate driver provate data using devm - but bind it to MFD
	   device */
	rk808_clkout = devm_kzalloc(&client->dev,
                                    sizeof(*rk808_clkout), GFP_KERNEL);

	[snip]

	/* init clk ops to point operations brought in kernel in this
	   module */
	init.ops = &rk808_clkout1_ops;
        rk808_clkout->clkout1_hw.init = &init;

	[snip]

	/* register clock using devm - bound to parent device again */
        ret = devm_clk_hw_register(&client->dev, &rk808_clkout->clkout1_hw);

	[snip]

/* Omit remove callback => no clk deregistration at module removal */

static struct platform_driver rk808_clkout_driver = {
        .probe = rk808_clkout_probe,
        .driver         = {
                .name   = "rk808-clkout",
        },
};

module_platform_driver(rk808_clkout_driver);

Quoted 3 lines

So can someone please shed some light on this? Is this a bug or am I just plain wrong?