* [PATCH 1/2] mfd: mfd-core: Add mechanism for removal of a subset of children @ 2020-05-11 15:53 Charles Keepax 2020-05-11 15:53 ` [PATCH v2 2/2] mfd: madera: Improve handling of regulator unbinding Charles Keepax ` (2 more replies) 0 siblings, 3 replies; 4+ messages in thread From: Charles Keepax @ 2020-05-11 15:53 UTC (permalink / raw) To: lee.jones; +Cc: broonie, patches, linux-kernel Currently, the only way to remove MFD children is with a call to mfd_remove_devices, which will remove all the children. Under some circumstances it is useful to remove only a subset of the child devices. For example if some additional clean up is required between removal of certain child devices. To accomplish this a tag field is added to mfd_cell, and a corresponding mfd_remove_devices_by_tag function is added to remove children with a specific tag. This allows a good amount of flexibility in which child devices a driver wishes to remove, as a driver could target specific drivers or a large group. Allowing other use-cases such as removing drivers for functionality that is no longer required. Some investigation was done of using the mfd_cell name and id fields, but it is hard to achieve a good level of flexibility there, at least without significant complexity. Since the id gets modified by the core and can even by automatically generated. Using the name alone would work for my usecase but it is not hard to imagine a situation where it wouldn't. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> --- Following on from our discussions here: https://lore.kernel.org/lkml/20200122110842.10702-2-ckeepax@opensource.cirrus.com/#t Happy to discuss other approaches as well, but this one was quite appealing as it was very simple but affords quite a lot of flexibility. Thanks, Charles drivers/mfd/mfd-core.c | 11 +++++++++++ include/linux/mfd/core.h | 2 ++ 2 files changed, 13 insertions(+) diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c index f5a73af60dd40..83a57186169de 100644 --- a/drivers/mfd/mfd-core.c +++ b/drivers/mfd/mfd-core.c @@ -287,6 +287,7 @@ static int mfd_remove_devices_fn(struct device *dev, void *data) { struct platform_device *pdev; const struct mfd_cell *cell; + int tag = (int)data; if (dev->type != &mfd_dev_type) return 0; @@ -294,6 +295,9 @@ static int mfd_remove_devices_fn(struct device *dev, void *data) pdev = to_platform_device(dev); cell = mfd_get_cell(pdev); + if (tag && cell->tag != tag) + return 0; + regulator_bulk_unregister_supply_alias(dev, cell->parent_supplies, cell->num_parent_supplies); @@ -301,6 +305,13 @@ static int mfd_remove_devices_fn(struct device *dev, void *data) return 0; } +void mfd_remove_devices_by_tag(struct device *parent, int tag) +{ + device_for_each_child_reverse(parent, (void *)tag, + mfd_remove_devices_fn); +} +EXPORT_SYMBOL(mfd_remove_devices_by_tag); + void mfd_remove_devices(struct device *parent) { device_for_each_child_reverse(parent, NULL, mfd_remove_devices_fn); diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h index d01d1299e49dc..f68d668b2cb7c 100644 --- a/include/linux/mfd/core.h +++ b/include/linux/mfd/core.h @@ -58,6 +58,7 @@ struct mfd_cell_acpi_match { struct mfd_cell { const char *name; int id; + int tag; int (*enable)(struct platform_device *dev); int (*disable)(struct platform_device *dev); @@ -135,6 +136,7 @@ static inline int mfd_add_hotplug_devices(struct device *parent, } extern void mfd_remove_devices(struct device *parent); +extern void mfd_remove_devices_by_tag(struct device *parent, int tag); extern int devm_mfd_add_devices(struct device *dev, int id, const struct mfd_cell *cells, int n_devs, -- 2.11.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] mfd: madera: Improve handling of regulator unbinding 2020-05-11 15:53 [PATCH 1/2] mfd: mfd-core: Add mechanism for removal of a subset of children Charles Keepax @ 2020-05-11 15:53 ` Charles Keepax 2020-05-11 21:57 ` [PATCH 1/2] mfd: mfd-core: Add mechanism for removal of a subset of children kbuild test robot 2020-05-13 10:18 ` Charles Keepax 2 siblings, 0 replies; 4+ messages in thread From: Charles Keepax @ 2020-05-11 15:53 UTC (permalink / raw) To: lee.jones; +Cc: broonie, patches, linux-kernel The current unbinding process for Madera has some issues. The trouble is runtime PM is disabled as the first step of the process, but some of the drivers release IRQs causing regmap IRQ to issue a runtime get which fails. To allow runtime PM to remain enabled during mfd_remove_devices, the DCVDD regulator must remain available. In the case of external DCVDD's this is simple, the regulator can simply be disabled/put after the call to mfd_remove_devices. However, in the case of an internally supplied DCVDD the regulator needs to be released after the other MFD children depending on it. Use the new MFD mfd_remove_devices_by_tag functionality to split the child drivers into two groups. An "optional" group of drivers and a core group that provide functionality to the others. Remove the optional group first, clean up the regulators etc. then finally remove the remaining children. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> --- Changes since v1: - Added MFD by tag thing and removed the regulator parts Thanks, Charles drivers/mfd/madera-core.c | 73 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 58 insertions(+), 15 deletions(-) diff --git a/drivers/mfd/madera-core.c b/drivers/mfd/madera-core.c index 7e0835cb062b1..f89b8825ebc1f 100644 --- a/drivers/mfd/madera-core.c +++ b/drivers/mfd/madera-core.c @@ -38,6 +38,8 @@ #define MADERA_RESET_MIN_US 2000 #define MADERA_RESET_MAX_US 3000 +#define MADERA_OPTIONAL_DRIVER 1 + static const char * const madera_core_supplies[] = { "AVDD", "DBVDD1", @@ -56,14 +58,19 @@ static const char * const cs47l15_supplies[] = { static const struct mfd_cell cs47l15_devs[] = { { .name = "madera-pinctrl", }, { .name = "madera-irq" }, - { .name = "madera-gpio" }, + { + .name = "madera-gpio", + .tag = MADERA_OPTIONAL_DRIVER, + }, { .name = "madera-extcon", + .tag = MADERA_OPTIONAL_DRIVER, .parent_supplies = cs47l15_supplies, .num_parent_supplies = 1, /* We only need MICVDD */ }, { .name = "cs47l15-codec", + .tag = MADERA_OPTIONAL_DRIVER, .parent_supplies = cs47l15_supplies, .num_parent_supplies = ARRAY_SIZE(cs47l15_supplies), }, @@ -80,15 +87,23 @@ static const char * const cs47l35_supplies[] = { static const struct mfd_cell cs47l35_devs[] = { { .name = "madera-pinctrl", }, { .name = "madera-irq", }, - { .name = "madera-micsupp", }, - { .name = "madera-gpio", }, + { + .name = "madera-micsupp", + .tag = MADERA_OPTIONAL_DRIVER, + }, + { + .name = "madera-gpio", + .tag = MADERA_OPTIONAL_DRIVER, + }, { .name = "madera-extcon", + .tag = MADERA_OPTIONAL_DRIVER, .parent_supplies = cs47l35_supplies, .num_parent_supplies = 1, /* We only need MICVDD */ }, { .name = "cs47l35-codec", + .tag = MADERA_OPTIONAL_DRIVER, .parent_supplies = cs47l35_supplies, .num_parent_supplies = ARRAY_SIZE(cs47l35_supplies), }, @@ -108,15 +123,23 @@ static const char * const cs47l85_supplies[] = { static const struct mfd_cell cs47l85_devs[] = { { .name = "madera-pinctrl", }, { .name = "madera-irq", }, - { .name = "madera-micsupp" }, - { .name = "madera-gpio", }, + { + .name = "madera-micsupp", + .tag = MADERA_OPTIONAL_DRIVER, + }, + { + .name = "madera-gpio", + .tag = MADERA_OPTIONAL_DRIVER, + }, { .name = "madera-extcon", + .tag = MADERA_OPTIONAL_DRIVER, .parent_supplies = cs47l85_supplies, .num_parent_supplies = 1, /* We only need MICVDD */ }, { .name = "cs47l85-codec", + .tag = MADERA_OPTIONAL_DRIVER, .parent_supplies = cs47l85_supplies, .num_parent_supplies = ARRAY_SIZE(cs47l85_supplies), }, @@ -134,15 +157,23 @@ static const char * const cs47l90_supplies[] = { static const struct mfd_cell cs47l90_devs[] = { { .name = "madera-pinctrl", }, { .name = "madera-irq", }, - { .name = "madera-micsupp", }, - { .name = "madera-gpio", }, + { + .name = "madera-micsupp", + .tag = MADERA_OPTIONAL_DRIVER, + }, + { + .name = "madera-gpio", + .tag = MADERA_OPTIONAL_DRIVER, + }, { .name = "madera-extcon", + .tag = MADERA_OPTIONAL_DRIVER, .parent_supplies = cs47l90_supplies, .num_parent_supplies = 1, /* We only need MICVDD */ }, { .name = "cs47l90-codec", + .tag = MADERA_OPTIONAL_DRIVER, .parent_supplies = cs47l90_supplies, .num_parent_supplies = ARRAY_SIZE(cs47l90_supplies), }, @@ -157,15 +188,23 @@ static const char * const cs47l92_supplies[] = { static const struct mfd_cell cs47l92_devs[] = { { .name = "madera-pinctrl" }, { .name = "madera-irq", }, - { .name = "madera-micsupp", }, - { .name = "madera-gpio" }, + { + .name = "madera-micsupp", + .tag = MADERA_OPTIONAL_DRIVER, + }, + { + .name = "madera-gpio", + .tag = MADERA_OPTIONAL_DRIVER, + }, { .name = "madera-extcon", + .tag = MADERA_OPTIONAL_DRIVER, .parent_supplies = cs47l92_supplies, .num_parent_supplies = 1, /* We only need MICVDD */ }, { .name = "cs47l92-codec", + .tag = MADERA_OPTIONAL_DRIVER, .parent_supplies = cs47l92_supplies, .num_parent_supplies = ARRAY_SIZE(cs47l92_supplies), }, @@ -743,18 +782,22 @@ int madera_dev_exit(struct madera *madera) /* Prevent any IRQs being serviced while we clean up */ disable_irq(madera->irq); - /* - * DCVDD could be supplied by a child node, we must disable it before - * removing the children, and prevent PM runtime from turning it back on - */ - pm_runtime_disable(madera->dev); + pm_runtime_get_sync(madera->dev); - clk_disable_unprepare(madera->mclk[MADERA_MCLK2].clk); + mfd_remove_devices_by_tag(madera->dev, MADERA_OPTIONAL_DRIVER); + + pm_runtime_disable(madera->dev); regulator_disable(madera->dcvdd); regulator_put(madera->dcvdd); mfd_remove_devices(madera->dev); + + pm_runtime_set_suspended(madera->dev); + pm_runtime_put_noidle(madera->dev); + + clk_disable_unprepare(madera->mclk[MADERA_MCLK2].clk); + madera_enable_hard_reset(madera); regulator_bulk_disable(madera->num_core_supplies, -- 2.11.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] mfd: mfd-core: Add mechanism for removal of a subset of children 2020-05-11 15:53 [PATCH 1/2] mfd: mfd-core: Add mechanism for removal of a subset of children Charles Keepax 2020-05-11 15:53 ` [PATCH v2 2/2] mfd: madera: Improve handling of regulator unbinding Charles Keepax @ 2020-05-11 21:57 ` kbuild test robot 2020-05-13 10:18 ` Charles Keepax 2 siblings, 0 replies; 4+ messages in thread From: kbuild test robot @ 2020-05-11 21:57 UTC (permalink / raw) To: Charles Keepax, lee.jones; +Cc: kbuild-all, broonie, patches, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2715 bytes --] Hi Charles, I love your patch! Perhaps something to improve: [auto build test WARNING on ljones-mfd/for-mfd-next] [also build test WARNING on v5.7-rc5 next-20200511] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Charles-Keepax/mfd-mfd-core-Add-mechanism-for-removal-of-a-subset-of-children/20200512-032030 base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next config: s390-randconfig-r034-20200511 (attached as .config) compiler: s390-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=s390 If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot <lkp@intel.com> All warnings (new ones prefixed by >>): drivers/mfd/mfd-core.c: In function 'mfd_remove_devices_fn': >> drivers/mfd/mfd-core.c:290:12: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 290 | int tag = (int)data; | ^ drivers/mfd/mfd-core.c: In function 'mfd_remove_devices_by_tag': >> drivers/mfd/mfd-core.c:310:40: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] 310 | device_for_each_child_reverse(parent, (void *)tag, | ^ vim +290 drivers/mfd/mfd-core.c 285 286 static int mfd_remove_devices_fn(struct device *dev, void *data) 287 { 288 struct platform_device *pdev; 289 const struct mfd_cell *cell; > 290 int tag = (int)data; 291 292 if (dev->type != &mfd_dev_type) 293 return 0; 294 295 pdev = to_platform_device(dev); 296 cell = mfd_get_cell(pdev); 297 298 if (tag && cell->tag != tag) 299 return 0; 300 301 regulator_bulk_unregister_supply_alias(dev, cell->parent_supplies, 302 cell->num_parent_supplies); 303 304 platform_device_unregister(pdev); 305 return 0; 306 } 307 308 void mfd_remove_devices_by_tag(struct device *parent, int tag) 309 { > 310 device_for_each_child_reverse(parent, (void *)tag, 311 mfd_remove_devices_fn); 312 } 313 EXPORT_SYMBOL(mfd_remove_devices_by_tag); 314 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 27311 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] mfd: mfd-core: Add mechanism for removal of a subset of children 2020-05-11 15:53 [PATCH 1/2] mfd: mfd-core: Add mechanism for removal of a subset of children Charles Keepax 2020-05-11 15:53 ` [PATCH v2 2/2] mfd: madera: Improve handling of regulator unbinding Charles Keepax 2020-05-11 21:57 ` [PATCH 1/2] mfd: mfd-core: Add mechanism for removal of a subset of children kbuild test robot @ 2020-05-13 10:18 ` Charles Keepax 2 siblings, 0 replies; 4+ messages in thread From: Charles Keepax @ 2020-05-13 10:18 UTC (permalink / raw) To: lee.jones; +Cc: broonie, patches, linux-kernel On Mon, May 11, 2020 at 04:53:32PM +0100, Charles Keepax wrote: > Currently, the only way to remove MFD children is with a call to > mfd_remove_devices, which will remove all the children. Under > some circumstances it is useful to remove only a subset of the > child devices. For example if some additional clean up is required > between removal of certain child devices. > > To accomplish this a tag field is added to mfd_cell, and a > corresponding mfd_remove_devices_by_tag function is added to > remove children with a specific tag. This allows a good amount of > flexibility in which child devices a driver wishes to remove, as a > driver could target specific drivers or a large group. Allowing other > use-cases such as removing drivers for functionality that is no longer > required. > > Some investigation was done of using the mfd_cell name and id fields, > but it is hard to achieve a good level of flexibility there, at least > without significant complexity. Since the id gets modified by the core > and can even by automatically generated. Using the name alone would > work for my usecase but it is not hard to imagine a situation where it > wouldn't. > > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> > --- > > Following on from our discussions here: > > https://lore.kernel.org/lkml/20200122110842.10702-2-ckeepax@opensource.cirrus.com/#t > > Happy to discuss other approaches as well, but this one was quite > appealing as it was very simple but affords quite a lot of flexibility. > > Thanks, > Charles > > > drivers/mfd/mfd-core.c | 11 +++++++++++ > include/linux/mfd/core.h | 2 ++ > 2 files changed, 13 insertions(+) > > diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c > index f5a73af60dd40..83a57186169de 100644 > --- a/drivers/mfd/mfd-core.c > +++ b/drivers/mfd/mfd-core.c > @@ -287,6 +287,7 @@ static int mfd_remove_devices_fn(struct device *dev, void *data) > { > struct platform_device *pdev; > const struct mfd_cell *cell; > + int tag = (int)data; Yeah as the build bot points out should have actually used a pointer here. Will update for that if we don't have any major objections to the approach in general. Thanks, Charles ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-13 10:18 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-11 15:53 [PATCH 1/2] mfd: mfd-core: Add mechanism for removal of a subset of children Charles Keepax 2020-05-11 15:53 ` [PATCH v2 2/2] mfd: madera: Improve handling of regulator unbinding Charles Keepax 2020-05-11 21:57 ` [PATCH 1/2] mfd: mfd-core: Add mechanism for removal of a subset of children kbuild test robot 2020-05-13 10:18 ` Charles Keepax
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).