Hi Jean, On Fri, Nov 28, 2014 at 02:48:13PM +0100, Jean Delvare wrote: > On Tue, 25 Nov 2014 22:14:32 +0100, Uwe Kleine-König wrote: > > On Wed, Jan 08, 2014 at 02:28:49PM +0100, Jean Delvare wrote: > > > Having looked at the code in deeper detail, I think I understand what > > > is going on. The problem is with: > > > > > > i2c_set_adapdata(&priv->adapter, priv); > > > > > > at the beginning of i801_probe(). It triggers the allocation of dev->p > > > by the driver core. If we bail out at any point before i2c_add_adapter > > > (and subsequently device_register) is called, then that memory is never > > > freed. > > > > > > Unfortunately it is not possible to move the i2c_set_adapdata() call > > > after i2c_add_adapter(), because the data pointer is needed by code > > > which runs as part of i2c_add_adapter(). > > > > > > We could move it right before the call to i2c_add_adapter(), to make > > > the problem window smaller, but this wouldn't solve the problem > > > completely, as i2c_add_adapter() itself can fail before > > > device_register() is called. > > > > > > The only solution I can think of at this point is to stop using > > > i2c_set_adapdata() altogether, and use i2c_adapter.algo_data instead: > > > > > > From: Jean Delvare <khali@xxxxxxxxxxxx> > > > Subject: i2c-i801: Use i2c_adapter.algo_data > > > > > > Use i2c_adapter.algo_data instead of i2c_set/get_adapdata(). The > > > latter makes use of the driver core's private data mechanism, which > > > allocates memory. That memory is never released if an error happens > > > between the call to i2c_set_adapdata() and the actual i2c_adapter > > > registration. > > > > Since commit 1bb6c08abfb6 (which makes the driver core use a pointer in > > struct device again for dev_set_drvdata; went into v3.16-rc1) this patch > > is obsolete, right? > > Correct. It was never applied upstream anyway, which is good as I never > liked it. I came back to it as it was still in the linux-i2c patchwork queue. It is set to be superseeded now. > > (Still there might be the opportunity for a few patches converting all > > driver to i2c_set_adapdata and then drop adapter.algo_data.) > > That's at least 35 bus drivers that would have to be converted then. > But you should first check if it is possible to get rid of > i2c_adapter.algo_data without breaking i2c-algo-bit and i2c-mux. If not > then converting the bus drivers would really only be a minor cleanup > with no real benefit (not saying it's not worth it though.) Yeah. I'm not sure I will address this cleanup, but will keep your hint on my radar. Best regards and thanks for your reply, Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
