Hello Krzysztof,
On 02/15/2016 08:20 PM, Krzysztof Kozlowski wrote:
On 16.02.2016 00:21, Javier Martinez Canillas wrote:
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 <jav...@osg.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.
+Cc Marek Szyprowski, who may know a lot more about dependencies between
these.
I wouldn't care for drivers not taking references to regulators/clocks.
Most of necessary regulators and clocks are turned on by bootloader or
by default values in PMIC. This means that later probing of PMIC
shouldn't influence drivers which are not using it.
The remaining problem was unsupported deferred probing by some of the
drivers using regulators/clocks (drivers being consumers of regulators
or clocks). AFAIR one of example was USB OTG.
By "please check" in this case I mean - look if every regulator/clock
consumer using stuff exposed by PMIC, supports properly deferred probing.
Got it, I checked and all but one consumer driver that use resources
provided by the max77686 defer probe when this is not found AFAICT.
Clocks:
drivers/mmc/core/pwrseq_simple.c
drivers/rtc/rtc-s3c.c
Regulators:
drivers/cpufreq/cpufreq-dt.c
drivers/gpu/drm/exynos/exynos_drm_dsi.c
drivers/gpu/drm/exynos/exynos_hdmi.c
drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c
drivers/iio/adc/exynos_adc.c
drivers/input/misc/max77693-haptic.c
drivers/input/touchscreen/mms114.c
drivers/media/i2c/s5c73m3/s5c73m3-core.c
drivers/media/i2c/s5k6a3.c
drivers/media/platform/exynos4-is/mipi-csis.c
drivers/mfd/wm8994-core.c
drivers/mmc/host/dw_mmc.c
drivers/mmc/host/sdhci.c
drivers/usb/dwc2/platform.c
The only driver that does not defer probe when an input supply
isn't found is drivers/thermal/samsung/exynos_tmu.c (vtmu-supply).
So that should be handled before this series are merged.
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?
I suspect the consumers are not defined in DTS. However I wouldn't care
about such issue. If there is no consumer, then probe order shouldn't
matter...
Ok.
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.
So test it... You are posting a small improvement without any important
Yes, problem is that I don't have access to boards with a max77686,
and that's why I asked for help to test the series on those :)
benefit but in the same time it might broke existing platforms. Perfect
is the enemy of the good (or if it ain't broken, don't touch it), so
please be sure that max77686 still works. :)
Agreed, I never said that adding regressions was justified. I just wanted
to get rid of the initcall levels ordering hack but of course only after
the patches have been tested and found to not cause regressions.
But feel free to nack the series if you consider this to be too risky.
Best regards,
Krzysztof
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America