linux-kernel - Re: [PATCH 2/2] mfd: mfd-core: Honour Device Tree's request to disable a child-device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8d458795-1081-313e-f5f6-7ca8572e7457@arm.com>
Date:   Tue, 22 Oct 2019 19:15:55 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     broonie@...nel.org, linus.walleij@...aro.org,
        daniel.thompson@...aro.org, arnd@...db.de, baohua@...nel.org,
        stephan@...hold.net, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/2] mfd: mfd-core: Honour Device Tree's request to
 disable a child-device

On 19/10/2019 08:28, Lee Jones wrote:
> Good morning Robin,
> 
> It's been a while.  I hope that you are well.
> 
> Thanks for taking an interest.
> 
> On Fri, 18 Oct 2019, Robin Murphy wrote:
>> On 18/10/2019 13:26, Lee Jones wrote:
>>> Until now, MFD has assumed all child devices passed to it (via
>>> mfd_cells) are to be registered.  It does not take into account
>>> requests from Device Tree and the like to disable child devices
>>> on a per-platform basis.
>>>
>>> Well now it does.
>>>
>>> Reported-by: Barry Song <Baohua.Song@....com>
>>> Reported-by: Stephan Gerhold <stephan@...hold.net>
>>> Signed-off-by: Lee Jones <lee.jones@...aro.org>
>>> ---
>>>    drivers/mfd/mfd-core.c | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
>>> index eafdadd58e8b..24c139633524 100644
>>> --- a/drivers/mfd/mfd-core.c
>>> +++ b/drivers/mfd/mfd-core.c
>>> @@ -182,6 +182,11 @@ static int mfd_add_device(struct device *parent, int id,
>>>    	if (parent->of_node && cell->of_compatible) {
>>>    		for_each_child_of_node(parent->of_node, np) {
>>>    			if (of_device_is_compatible(np, cell->of_compatible)) {
>>> +				if (!of_device_is_available(np)) {
>>> +					/* Ignore disabled devices error free */
>>> +					ret = 0;
>>> +					goto fail_alias;
>>> +				}
>>
>> Is it possible for a device to have multiple children of the same type? If
>> so, it seems like this might not work as desired if, say, the first child
>> was disabled but subsequent ones weren't.
>>
>> It might make sense to use for_each_available_child_of_node() for the outer
>> loop, then check afterwards if anything was found.
> 
> The subsystem in its current guise doesn't reliably support the
> situation you describe. We have no way to track which child nodes have
> been through this process previously, thus mfd-core will always choose
> the first child node with a matching compatible string.

Ah, OK, if that situation has never been expected to work properly then 
the simple patch is probably fine.

> If you have any suggests in terms of adding support for multiple
> children with matching compatible strings, I'd be very receptive.

I know very little about the MFD layer and its users, so I wasn't sure 
whether this 'multiple child instances' thing would ever actually be a 
real concern (other than for "simple-mfd"s which apparently don't use 
this mechanism anyway) - I was just considering the code from a pure DT 
perspective.

Cheers,
Robin.

>>>    				pdev->dev.of_node = np;
>>>    				pdev->dev.fwnode = &np->fwnode;
>>>    				break;
>>>
>