linux-kernel - Re: [PATCH 4/4] mfd: tps65217: Instantiate sub-devices from device tree

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 8 Jun 2017 18:47:48 -0500
From:   Grygorii Strashko <grygorii.strashko@...com>
To:     Javier Martinez Canillas <javier@...hile0.org>,
        Enric Balletbo Serra <eballetbo@...il.com>
CC:     Mark Brown <broonie@...nel.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Lee Jones <lee.jones@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Tony Lindgren <tony@...mide.com>,
        Enric Balletbo i Serra <enric.balletbo@...labora.com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        <linux-leds@...r.kernel.org>, <linux-input@...r.kernel.org>,
        Daniel Thompson <daniel.thompson@...aro.org>,
        Jingoo Han <jingoohan1@...il.com>,
        Richard Purdie <rpurdie@...ys.net>,
        Jacek Anaszewski <jacek.anaszewski@...il.com>,
        Pavel Machek <pavel@....cz>,
        Mark Rutland <mark.rutland@....com>,
        Russell King <linux@...linux.org.uk>
Subject: Re: [PATCH 4/4] mfd: tps65217: Instantiate sub-devices from device
 tree



On 06/08/2017 05:30 PM, Javier Martinez Canillas wrote:
> On Thu, Jun 8, 2017 at 11:35 PM, Enric Balletbo Serra
> <eballetbo@...il.com> wrote:
>> 2017-06-08 19:11 GMT+02:00 Grygorii Strashko <grygorii.strashko@...com>:
> 
> [snip]
> 
>>>>
>>>> &tps {
>>>>           charger {
>>>>                   interrupts = <0>, <1>;
>>>>                   interrupt-names = "USB", "AC";
>>>>                   status = "okay";
>>>>           };
>>>>
>>>>           pwrbutton {
>>>>                   interrupts = <2>;
>>>>                   status = "okay";
>>>>           };
>>>> };
>>>
>>> Sry, but this make no sense - those IRQ configuration is static,
>>> so it should be defined in arch/arm/boot/dts/tps65217.dtsi at least.
> 
> Agreed.
> 
>>
>> I was describing the state-of-art not what should be. If you mean that
>> what doesn't make sense have these interrupts portions in the DT and
>> the resources in the driver I'm completely agree. So we have two
>> options:
>>
>>   1) Get rid of the irq resources from tps65217 MFD driver and
>> configure all with the DT (these patches)
>>   2) Get rid of the DT portions as doesn't make sense have them in two places.
>>
>> If we select 2) at least we have the problem that currently all
>> sub-devices are instantiated and there is no way to disable a
>> sub-device (the problem I'm trying to solve) hence I proposed 1)
>>
> 
> There's also an option (3) AFAICT. To do (1) but instead of hardcoding
> in the DTS[i], to do it in the charger driver since it seems the
> sub-devices are only used for this particular MFD device, and so the
> IRQ numbers are always going to be the same (it's already hardcoded in
> the MFD driver anyways).
> 
> So you can just move TPS65217_IRQ_{AC,USB} to the driver. Not sure
> what's general opinion of having drivers calling irq_create_mapping()
> to get the virtual IRQ's though...
> 
> [snip]
> 
>>>
>>> I can't find links on corresponding discussions, but mfd_add_devices() is
>>> preferred for MDF devices. Below is one commit i've found. Also you can compare number of
>>> drivers using mfd_add_devices() and of_platform_populate().
> 
> I'm not sure if we should use that as an argument, it may well be
> that's just the drivers being half converted to DT (which is pretty
> common TBH).
> 
>>>
>>
>> I don't think this a valid argument, I can also provide you a commit
>> that does exactly the contrary, replaces mfd_add_devices for
>> of_platform_populate (commit bb03ffb96c72)
>>
>>>
> 
> [snip]
> 
>>
>>> ---
>>> commit 4531156db726d27e593d35800d43c74be4e393b9
>>> Author: Keerthy <j-keerthy@...com>
>>> Date:   Mon Sep 19 13:09:05 2016 +0530
>>>
>>>      mfd: tps65218: Use mfd_add_devices instead of of_platform_populate
>>>
>>>      mfd_add_devices enables parsing device tree nodes without compatibles
>>>      for regulators and gpio modules. Replace of_platform_populate with
>>>      mfd_add_devices. mfd_cell currently is populated with regulators,
>>>      gpio and powerbutton.
>>>
> 
> For tps65218 couldn't instead of using mfd_add_devices() for all the
> sub-devs, had used of_platform_populate() for the ones that have
> device nodes and mfd_add_devices() only for the "tps65218-regulator"?
> 
> The commit talks about nodes without compatibles but's actually about
> sub-devices without an associated device node. For me it makes sense
> to use of_platform_populate() when the MFD has device nodes for their
> sub-devices and mfd_add_devices() when DT knows nothing about the
> sub-devices.

FYI. Below is link discussion I'm referring to between Mark Brown and Andrew F. Davis 
https://lkml.org/lkml/2015/10/22/823
the same - https://groups.google.com/forum/#!topic/linux.kernel/wQsdSpPMroQ




-- 
regards,
-grygorii