2020-05-11 15:57:29

by Charles Keepax

[permalink] [raw]
Subject: [PATCH 1/2] mfd: mfd-core: Add mechanism for removal of a subset of children

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 <[email protected]>
---

Following on from our discussions here:

https://lore.kernel.org/lkml/[email protected]/#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


2020-05-11 22:01:10

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/2] mfd: mfd-core: Add mechanism for removal of a subset of children

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 <[email protected]>

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/[email protected]


Attachments:
(No filename) (2.72 kB)
.config.gz (26.67 kB)
Download all attachments

2020-05-13 10:23:02

by Charles Keepax

[permalink] [raw]
Subject: Re: [PATCH 1/2] mfd: mfd-core: Add mechanism for removal of a subset of children

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 <[email protected]>
> ---
>
> Following on from our discussions here:
>
> https://lore.kernel.org/lkml/[email protected]/#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