On 06/08/2017 05:30 PM, Javier Martinez Canillas wrote:
> On Thu, Jun 8, 2017 at 11:35 PM, Enric Balletbo Serra
> <eballetbo@xxxxxxxxx> wrote:
>> 2017-06-08 19:11 GMT+02:00 Grygorii Strashko <grygorii.strashko@xxxxxx>:
>
> [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@xxxxxx>
>>> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
