linux-kernel - Re: [PATCH 2/4] mfd: max77686: Use module_i2c

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:	Mon, 15 Feb 2016 12:21:43 -0300
From:	Javier Martinez Canillas <javier@....samsung.com>
To:	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	linux-kernel@...r.kernel.org
Cc:	Andi Shyti <andi.shyti@...sung.com>,
	linux-samsung-soc@...r.kernel.org,
	Lee Jones <lee.jones@...aro.org>,
	Laxman Dewangan <ldewangan@...dia.com>
Subject: Re: [PATCH 2/4] mfd: max77686: Use module_i2c_driver() instead of
 subsys initcall

Hello Krzysztof,

On 02/15/2016 03:54 AM, Krzysztof Kozlowski wrote:
> On 12.02.2016 13:30, Javier Martinez Canillas wrote:
>> The driver's init and exit function don't do anything besides adding and
>> deleting the I2C driver so the module_i2c_driver() macro could be used.
>>
>> Currently is not being used because the driver is initialized at subsys
>> initcall level, claiming that this is done to allow consumers devices to
>> use the resources provided by this driver. But dependencies should be in
>> the DT and consumers drivers should not rely in the registration order.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@....samsung.com>
>> ---
>>
>>   drivers/mfd/max77686.c | 13 +------------
>>   1 file changed, 1 insertion(+), 12 deletions(-)
>>
>
> In the past not all dependencies supported deferred probing so such
> ordering was required.
>
> I don't like the "dependencies should be in DT" reason for the change...
> because it is kind of wishful thinking. Yeah, the dependencies should be
> in DT, but are they?
>
> Instead *please check it* and write:
> "Dependencies are in DT so manual ordering of init calls is not
> necessary any more".
>

For the max77802 I know that's the case since the only two DTS in mainline
that use it are the Peach Pit and Pi and I'm very familiar with those two.

But I wonder how can I check that this is the case for the max77686. Most
DTS in mainline have nodes that use some clocks and regulators provided by
the PMIC, only arch/arm/boot/dts/exynos5250-smdk5250.dts doesn't have one
of the regulators as input supply or clock consumer defined.

For the clock, I guess the RTC is just broken since it's using the s3c6410
controller that requires a source clock and this is not defined.

Now the question is if it doesn't really need the regulators or is that
the DTS isn't correctly defined and some drivers were relying on the MFD
and regulator drivers to be registered at subsys initcall level?

> My fast tests of this patch shown that it works good... but some more
> thorough tests should be done.
>

What do you suggest? The drivers now support deferred probing but as said,
I don't know how I can be sure that drivers aren't missing input supplies
and relying in regulators being registered early and marked as always-on.

> Best regards,
> Krzysztof
>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America