Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 16 Jun 2020 08:58:34 +0100
From:   Lee Jones <lee.jones@linaro.org>
To:     Charles Keepax <ckeepax@opensource.cirrus.com>
Cc:     linux-kernel@vger.kernel.org, patches@opensource.cirrus.com
Subject: Re: [PATCH v2 1/2] mfd: mfd-core: Add mechanism for removal of a
 subset of children
Message-ID: <20200616075834.GF2608702@dell>
References: <20200615150722.5249-1-ckeepax@opensource.cirrus.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20200615150722.5249-1-ckeepax@opensource.cirrus.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, 15 Jun 2020, 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.                                                                                                                                                                        │··················

What about this?

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index f5a73af60dd40..a06e0332e1e31 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -283,7 +283,7 @@ int mfd_add_devices(struct device *parent, int id,
 }
 EXPORT_SYMBOL(mfd_add_devices);
 
-static int mfd_remove_devices_fn(struct device *dev, void *data)
+static int mfd_remove_devices_fn(struct device *dev, void *level)
 {
        struct platform_device *pdev;
        const struct mfd_cell *cell;
@@ -294,6 +294,9 @@ static int mfd_remove_devices_fn(struct device *dev, void *data)
        pdev = to_platform_device(dev);
        cell = mfd_get_cell(pdev);
 
+       if (cell->level && (!level || cell->level != *level))
+               return 0;
+
        regulator_bulk_unregister_supply_alias(dev, cell->parent_supplies,
                                               cell->num_parent_supplies);
 
@@ -303,7 +306,11 @@ static int mfd_remove_devices_fn(struct device *dev, void *data)
 
 void mfd_remove_devices(struct device *parent)
 {
+       int level = MFD_DEP_LEVEL_HIGH;
+
        device_for_each_child_reverse(parent, NULL, mfd_remove_devices_fn);
+       device_for_each_child_reverse(parent, &level, mfd_remove_devices_fn);
 }
 EXPORT_SYMBOL(mfd_remove_devices);

No need for special calls from the parent driver in this case.

Just a requirement to set the cell's dependency level.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog