Question about windfarm drivers

Question about windfarm drivers

[Jean Delvare]

Hi Ben, hi Paul,

As I am converting the windfarm drivers to the new i2c device binding
model, I need to understand how these drivers work currently. There's
one thing I do not understand so I'd appreciate if you could explain it
to me.

I am looking at function wf_lm75_detach() in windfarm_lm75_sensor.c.
This function is called by i2c-core if either i2c-powermac or
windfarm_lm75_sensor is unloaded. It sets lm->i2c.adapter to NULL and
then calls wf_unregister_sensor(), which in turn calls wf_put_sensor(),
which calls wf_sensor_release() is the reference count drops to 0,
which in turns call the driver's .release() method, that is,
wf_lm75_release() in our case.

In wf_lm75_release(), i2c_detach_client() is called if and only if
lm->i2c.adapter is set, which is not the case, and then the data
structure, including the i2c client, is freed from memory. This means
that the freed i2c client is still registered with i2c-core, this looks
wrong.

Am I missing something? Or is this clean-up path broken and nobody ever
noticed?

I am also curious why wf_unregister_sensor() calls wf_put_sensor()
while wf_register_sensor() doesn't call wf_get_sensor().

Thanks,
-- 
Jean Delvare

Re: Question about windfarm drivers

[Benjamin Herrenschmidt]

On Thu, 2009-04-16 at 10:52 +0200, Jean Delvare wrote:

> In wf_lm75_release(), i2c_detach_client() is called if and only if
> lm->i2c.adapter is set, which is not the case, and then the data
> structure, including the i2c client, is freed from memory. This means
> that the freed i2c client is still registered with i2c-core, this looks
> wrong.
> 
> Am I missing something? Or is this clean-up path broken and nobody ever
> noticed?

Probably the later :-)

> I am also curious why wf_unregister_sensor() calls wf_put_sensor()
> while wf_register_sensor() doesn't call wf_get_sensor().

It's quite possible that those code path are a bit buggy and not well
exercised. I don't think people ever unload those modules and we tend to
have that stuff built-in ourselves.

Cheers,
Ben.