* [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use.
@ 2016-09-07 20:05 jim_baxter
2016-09-07 20:05 ` [PATCH v1 1/2] i2c-mux: add i2c_mux_add_reparented_adapter api jim_baxter
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: jim_baxter @ 2016-09-07 20:05 UTC (permalink / raw)
To: linux-i2c; +Cc: Peter Korsgaard, Wolfram Sang, Peter Rosin, linux-kernel
From: Jim Baxter <jim_baxter@mentor.com>
This patchset adds a new i2c_mux_add_reparented_adapter API to the i2c
that allows owning modules to use module_get/module_put and stop the
i2c bus module being removed whilst in use.
This was tested on an ARM i.MX6 Sabre board with the pca953x gpio module.
Joshua Frkuska (2):
i2c-mux: add i2c_mux_add_reparented_adapter api
i2c-mux-gpio: call i2c_add_reparented_mux_adapter
drivers/i2c/i2c-mux.c | 14 ++++++++++++--
drivers/i2c/muxes/i2c-mux-gpio.c | 6 +++++-
include/linux/i2c-mux.h | 15 +++++++++++++++
3 files changed, 32 insertions(+), 3 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v1 1/2] i2c-mux: add i2c_mux_add_reparented_adapter api 2016-09-07 20:05 [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use jim_baxter @ 2016-09-07 20:05 ` jim_baxter 2016-09-07 20:05 ` [PATCH v1 2/2] i2c-mux-gpio: call i2c_add_reparented_mux_adapter jim_baxter 2016-09-09 20:40 ` [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use Peter Rosin 2 siblings, 0 replies; 6+ messages in thread From: jim_baxter @ 2016-09-07 20:05 UTC (permalink / raw) To: linux-i2c; +Cc: Peter Korsgaard, Wolfram Sang, Peter Rosin, linux-kernel From: Joshua Frkuska <joshua_frkuska@mentor.com> This new api allows the i2c adapter created by i2c-mux to be owned by the module that calls this api. This allows module removal chaining to be done at a higher level. Signed-off-by: Joshua Frkuska <joshua_frkuska@mentor.com> Signed-off-by: Jim Baxter <jim_baxter@mentor.com> --- drivers/i2c/i2c-mux.c | 14 ++++++++++++-- include/linux/i2c-mux.h | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c index 8eee986..191816b 100644 --- a/drivers/i2c/i2c-mux.c +++ b/drivers/i2c/i2c-mux.c @@ -263,7 +263,8 @@ struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent, } EXPORT_SYMBOL_GPL(i2c_mux_alloc); -int i2c_mux_add_adapter(struct i2c_mux_core *muxc, +int i2c_mux_add_reparented_adapter(struct module *owner, + struct i2c_mux_core *muxc, u32 force_nr, u32 chan_id, unsigned int class) { @@ -305,7 +306,7 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc, /* Now fill out new adapter structure */ snprintf(priv->adap.name, sizeof(priv->adap.name), "i2c-%d-mux (chan_id %d)", i2c_adapter_id(parent), chan_id); - priv->adap.owner = THIS_MODULE; + priv->adap.owner = owner; priv->adap.algo = &priv->algo; priv->adap.algo_data = priv; priv->adap.dev.parent = &parent->dev; @@ -385,6 +386,15 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc, muxc->adapter[muxc->num_adapters++] = &priv->adap; return 0; } +EXPORT_SYMBOL_GPL(i2c_mux_add_reparented_adapter); + +int i2c_mux_add_adapter(struct i2c_mux_core *muxc, + u32 force_nr, u32 chan_id, + unsigned int class) +{ + return i2c_mux_add_reparented_adapter(THIS_MODULE, muxc, force_nr, + chan_id, class); +} EXPORT_SYMBOL_GPL(i2c_mux_add_adapter); void i2c_mux_del_adapters(struct i2c_mux_core *muxc) diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h index d4c1d12..d4f8610 100644 --- a/include/linux/i2c-mux.h +++ b/include/linux/i2c-mux.h @@ -64,6 +64,21 @@ struct i2c_adapter *i2c_root_adapter(struct device *dev); * Called to create an i2c bus on a multiplexed bus segment. * The chan_id parameter is passed to the select and deselect * callback functions to perform hardware-specific mux control. + * + * Unlike the simple i2c_add_mux_adapter, this is passed in the caller's + * module reference in order to reparent the adapter from the perspective of + * the i2c hierarchy. This allows the caller to be reference locked while the + * i2c adapter is in use. + */ +int i2c_mux_add_reparented_adapter(struct module *owner, + struct i2c_mux_core *muxc, + u32 force_nr, u32 chan_id, + unsigned int class); + +/* + * Called to create an i2c bus on a multiplexed bus segment. + * The chan_id parameter is passed to the select and deselect + * callback functions to perform hardware-specific mux control. */ int i2c_mux_add_adapter(struct i2c_mux_core *muxc, u32 force_nr, u32 chan_id, -- 1.9.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v1 2/2] i2c-mux-gpio: call i2c_add_reparented_mux_adapter 2016-09-07 20:05 [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use jim_baxter 2016-09-07 20:05 ` [PATCH v1 1/2] i2c-mux: add i2c_mux_add_reparented_adapter api jim_baxter @ 2016-09-07 20:05 ` jim_baxter 2016-09-09 20:40 ` [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use Peter Rosin 2 siblings, 0 replies; 6+ messages in thread From: jim_baxter @ 2016-09-07 20:05 UTC (permalink / raw) To: linux-i2c; +Cc: Peter Korsgaard, Wolfram Sang, Peter Rosin, linux-kernel From: Joshua Frkuska <joshua_frkuska@mentor.com> This reparents the adapter created in i2c-mux to this module for module unloading and chaining purposes. Signed-off-by: Joshua Frkuska <joshua_frkuska@mentor.com> Signed-off-by: Jim Baxter <jim_baxter@mentor.com> --- drivers/i2c/muxes/i2c-mux-gpio.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c index e5cf26e..9f92535 100644 --- a/drivers/i2c/muxes/i2c-mux-gpio.c +++ b/drivers/i2c/muxes/i2c-mux-gpio.c @@ -234,7 +234,11 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev) u32 nr = mux->data.base_nr ? (mux->data.base_nr + i) : 0; unsigned int class = mux->data.classes ? mux->data.classes[i] : 0; - ret = i2c_mux_add_adapter(muxc, nr, mux->data.values[i], class); + ret = i2c_mux_add_reparented_adapter(THIS_MODULE, + muxc, + nr, + mux->data.values[i], + class); if (ret) { dev_err(&pdev->dev, "Failed to add adapter %d\n", i); goto add_adapter_failed; -- 1.9.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use. 2016-09-07 20:05 [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use jim_baxter 2016-09-07 20:05 ` [PATCH v1 1/2] i2c-mux: add i2c_mux_add_reparented_adapter api jim_baxter 2016-09-07 20:05 ` [PATCH v1 2/2] i2c-mux-gpio: call i2c_add_reparented_mux_adapter jim_baxter @ 2016-09-09 20:40 ` Peter Rosin 2016-09-13 16:55 ` Baxter, Jim 2 siblings, 1 reply; 6+ messages in thread From: Peter Rosin @ 2016-09-09 20:40 UTC (permalink / raw) To: jim_baxter, linux-i2c; +Cc: Peter Korsgaard, Wolfram Sang, linux-kernel On 2016-09-07 22:05, jim_baxter@mentor.com wrote: > From: Jim Baxter <jim_baxter@mentor.com> > > This patchset adds a new i2c_mux_add_reparented_adapter API to the i2c > that allows owning modules to use module_get/module_put and stop the > i2c bus module being removed whilst in use. > > This was tested on an ARM i.MX6 Sabre board with the pca953x gpio module. > > Joshua Frkuska (2): > i2c-mux: add i2c_mux_add_reparented_adapter api > i2c-mux-gpio: call i2c_add_reparented_mux_adapter nitpick: Patch subjects for the second patch is wrong. "reparented" is a bit dual when dealing with i2c adapter trees. i2c_mux_add_owned_adapter is perhaps clearer? Aside from that, I'm not using modules much and need some enlightenment as to why the i2c_del_mux_adapter() call in i2c_mux_gpio_remove() is not sufficient and what exactly the problem is? Why would someone/something unload the i2c-mux module prematurely? Would it be an alternative to make i2c-mux a proper kernel object of some kind? I mean, why do not all other mux users also need to modify the owner? Why is i2c-mux-gpio special? CHeers, Peter ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use. 2016-09-09 20:40 ` [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use Peter Rosin @ 2016-09-13 16:55 ` Baxter, Jim 2016-09-14 12:57 ` Peter Rosin 0 siblings, 1 reply; 6+ messages in thread From: Baxter, Jim @ 2016-09-13 16:55 UTC (permalink / raw) To: Peter Rosin, linux-i2c Cc: Peter Korsgaard, Wolfram Sang, linux-kernel, Frkuska, Joshua, jiwang Hi Peter, > nitpick: Patch subjects for the second patch is wrong. > > "reparented" is a bit dual when dealing with i2c adapter trees. > i2c_mux_add_owned_adapter is perhaps clearer? Agreed, I will update that. > > > Aside from that, I'm not using modules much and need some enlightenment > as to why the i2c_del_mux_adapter() call in i2c_mux_gpio_remove() is not > sufficient and what exactly the problem is? Why would someone/something > unload the i2c-mux module prematurely? It is not a normal operation to remove the i2c gpio mux, however systemd could unload modules out of order if users are restarting services incorrectly and cause unintended side-effects. This change would stop an i2c-mux that maybe controlling a voltage regulator from being unloaded and disabling power to parts of the system unexpectedly. > > > Would it be an alternative to make i2c-mux a proper kernel object of > some kind? I mean, why do not all other mux users also need to modify > the owner? Why is i2c-mux-gpio special? > i2c-mux-gpio is not special, the code inserted by [PATCH v1 1/2] i2c-mux: add i2c_mux_add_reparented_adapter api could be used by other muxes if required. Best regards Jim ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use. 2016-09-13 16:55 ` Baxter, Jim @ 2016-09-14 12:57 ` Peter Rosin 0 siblings, 0 replies; 6+ messages in thread From: Peter Rosin @ 2016-09-14 12:57 UTC (permalink / raw) To: Baxter, Jim, linux-i2c Cc: Peter Korsgaard, Wolfram Sang, linux-kernel, Frkuska, Joshua, jiwang Hi Jim! On 2016-09-13 18:55, Baxter, Jim wrote: > Hi Peter, > >> nitpick: Patch subjects for the second patch is wrong. >> >> "reparented" is a bit dual when dealing with i2c adapter trees. >> i2c_mux_add_owned_adapter is perhaps clearer? > > Agreed, I will update that. > >> >> >> Aside from that, I'm not using modules much and need some enlightenment >> as to why the i2c_del_mux_adapter() call in i2c_mux_gpio_remove() is not >> sufficient and what exactly the problem is? Why would someone/something >> unload the i2c-mux module prematurely? > > It is not a normal operation to remove the i2c gpio mux, however systemd > could unload modules out of order if users are restarting services > incorrectly and cause unintended side-effects. This change would stop an > i2c-mux that maybe controlling a voltage regulator from being unloaded > and disabling power to parts of the system unexpectedly. I don't see how that can happen. The voltage regulator that sits behind the mux should be a user of one of the child adapters on the mux, so that child adapter can't be removed as long as the voltage regulator is there. And the i2c-mux can't be unloaded as long as the child adapter is there, since it is the owner. So what's going on? Simply changing the owner of the child adapter will break this chain leaving the i2c-mux as owner of nothing, thus *making* it a target for premature unloading. I must be fundamentally misunderstanding something. BTW, was this solving a real issue, or is it all theory? Cheers, Peter >> Would it be an alternative to make i2c-mux a proper kernel object of >> some kind? I mean, why do not all other mux users also need to modify >> the owner? Why is i2c-mux-gpio special? >> > > i2c-mux-gpio is not special, the code inserted by > [PATCH v1 1/2] i2c-mux: add i2c_mux_add_reparented_adapter api could be > used by other muxes if required. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-14 13:13 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-09-07 20:05 [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use jim_baxter 2016-09-07 20:05 ` [PATCH v1 1/2] i2c-mux: add i2c_mux_add_reparented_adapter api jim_baxter 2016-09-07 20:05 ` [PATCH v1 2/2] i2c-mux-gpio: call i2c_add_reparented_mux_adapter jim_baxter 2016-09-09 20:40 ` [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use Peter Rosin 2016-09-13 16:55 ` Baxter, Jim 2016-09-14 12:57 ` Peter Rosin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).