[PATCH] i2c: exynos5: Initialise Samsung High Speed I2C controller early

[PATCH] i2c: exynos5: Initialise Samsung High Speed I2C controller early

[Naveen Krishna Chatradhi]

This patch moves initialization code to subsys_initcall() to ensure
that the i2c bus is available early so the regulators can be quickly
probed and available for other devices on their probe() call.

Such solution has been proposed by Mark Brown to fix the problem of
the regulators not beeing available on the peripheral device probe():
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-March/011971.html

Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
---
 drivers/i2c/busses/i2c-exynos5.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
index 00af0a0..20e3077 100644
--- a/drivers/i2c/busses/i2c-exynos5.c
+++ b/drivers/i2c/busses/i2c-exynos5.c
@@ -762,8 +762,18 @@ static struct platform_driver exynos5_i2c_driver = {
 	},
 };
 
-module_platform_driver(exynos5_i2c_driver);
+static int __init i2c_adap_exynos5_init(void)
+{
+	return platform_driver_register(&exynos5_i2c_driver);
+}
+subsys_initcall(i2c_adap_exynos5_init);
+
+static void __exit i2c_adap_exynos5_exit(void)
+{
+	platform_driver_unregister(&exynos5_i2c_driver);
+}
 
+module_exit(i2c_adap_exynos5_exit);
 MODULE_DESCRIPTION("Exynos5 HS-I2C Bus driver");
 MODULE_AUTHOR("Naveen Krishna Chatradhi, <ch.naveen@samsung.com>");
 MODULE_AUTHOR("Taekgyun Ko, <taeggyun.ko@samsung.com>");
-- 
1.7.9.5

Re: [PATCH] i2c: exynos5: Initialise Samsung High Speed I2C controller early

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 652 bytes --]

On Thu, Apr 24, 2014 at 08:18:36PM +0530, Naveen Krishna Chatradhi wrote:
> This patch moves initialization code to subsys_initcall() to ensure
> that the i2c bus is available early so the regulators can be quickly
> probed and available for other devices on their probe() call.

> Such solution has been proposed by Mark Brown to fix the problem of
> the regulators not beeing available on the peripheral device probe():
> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-March/011971.html

What specifically is this needed for?  We *should* be able to use
deferred probe for most things, but I know that not all subsystems are
able to yet.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] i2c: exynos5: Initialise Samsung High Speed I2C controller early

[Tushar Behera]

On 04/24/2014 09:55 PM, Mark Brown wrote:
> On Thu, Apr 24, 2014 at 08:18:36PM +0530, Naveen Krishna Chatradhi wrote:
>> This patch moves initialization code to subsys_initcall() to ensure
>> that the i2c bus is available early so the regulators can be quickly
>> probed and available for other devices on their probe() call.
> 
>> Such solution has been proposed by Mark Brown to fix the problem of
>> the regulators not beeing available on the peripheral device probe():
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-March/011971.html
> 
> What specifically is this needed for?  We *should* be able to use
> deferred probe for most things, but I know that not all subsystems are
> able to yet.
> 

DRM-Exynos is one such sub-system right now that doesn't handle deferred
probe well. That is one of the reasons for this patch.

-- 
Tushar Behera

Re: [PATCH] i2c: exynos5: Initialise Samsung High Speed I2C controller early

[Naveen Krishna Ch]

Hello Mark,

On 24 April 2014 21:55, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Apr 24, 2014 at 08:18:36PM +0530, Naveen Krishna Chatradhi wrote:
>> This patch moves initialization code to subsys_initcall() to ensure
>> that the i2c bus is available early so the regulators can be quickly
>> probed and available for other devices on their probe() call.
>
>> Such solution has been proposed by Mark Brown to fix the problem of
>> the regulators not beeing available on the peripheral device probe():
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-March/011971.html
>
> What specifically is this needed for?  We *should* be able to use
> deferred probe for most things, but I know that not all subsystems are
> able to yet.
DRM related drivers like DP, FIMD, HDMI, Mixer wants to be probed ASAP
during the boot.
The real problem comes when, one of these drivers do a regulator_get().

If the physical supply  is not enabled/hookedup the regulator_get() call
assumes that physical supply is present and returns a
"dummy_regulator" (But, not an error).

Because of which, Display and several other devices fails to work.

I2C, I2C_TUNNEL, SPI and DMA drivers are required as subsys_initcall()
for similar reason.

-- 
Shine bright,
(: Nav :)

Re: [PATCH] i2c: exynos5: Initialise Samsung High Speed I2C controller early

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]

On Fri, May 09, 2014 at 05:50:00PM +0530, Naveen Krishna Ch wrote:
> On 24 April 2014 21:55, Mark Brown <broonie@kernel.org> wrote:

> >> Such solution has been proposed by Mark Brown to fix the problem of
> >> the regulators not beeing available on the peripheral device probe():
> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-March/011971.html

> > What specifically is this needed for?  We *should* be able to use
> > deferred probe for most things, but I know that not all subsystems are
> > able to yet.

> DRM related drivers like DP, FIMD, HDMI, Mixer wants to be probed ASAP
> during the boot.
> The real problem comes when, one of these drivers do a regulator_get().

> If the physical supply  is not enabled/hookedup the regulator_get() call
> assumes that physical supply is present and returns a
> "dummy_regulator" (But, not an error).

> Because of which, Display and several other devices fails to work.

These drivers are buggy, if they geniunely expect and handle a missing
supply then they should be using regulator_get_optional() to request the
regulator and even if they don't the use of subsys_initcall() is not
going to fix anything here - if a dummy regulator is going to be
returned the time things are probed won't make a difference.

> I2C, I2C_TUNNEL, SPI and DMA drivers are required as subsys_initcall()
> for similar reason.

What makes you say this?  Typically those drivers don't use regulators
at all.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] i2c: exynos5: Initialise Samsung High Speed I2C controller early

[Naveen Krishna Ch]

Hello Mark,

On 9 May 2014 19:21, Mark Brown <broonie@kernel.org> wrote:
> On Fri, May 09, 2014 at 05:50:00PM +0530, Naveen Krishna Ch wrote:
>> On 24 April 2014 21:55, Mark Brown <broonie@kernel.org> wrote:
>
>> >> Such solution has been proposed by Mark Brown to fix the problem of
>> >> the regulators not beeing available on the peripheral device probe():
>> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-March/011971.html
>
>> > What specifically is this needed for?  We *should* be able to use
>> > deferred probe for most things, but I know that not all subsystems are
>> > able to yet.
>
>> DRM related drivers like DP, FIMD, HDMI, Mixer wants to be probed ASAP
>> during the boot.
>> The real problem comes when, one of these drivers do a regulator_get().
>
>> If the physical supply  is not enabled/hookedup the regulator_get() call
>> assumes that physical supply is present and returns a
>> "dummy_regulator" (But, not an error).
>
>> Because of which, Display and several other devices fails to work.
>
> These drivers are buggy, if they geniunely expect and handle a missing
> supply then they should be using regulator_get_optional() to request the
> regulator and even if they don't the use of subsys_initcall() is not
> going to fix anything here - if a dummy regulator is going to be
> returned the time things are probed won't make a difference.
We have a situation where.a PMIC is sitting under I2C_TUNNEL
with I2C, SPI and SPI uses DMA.

PMIC --- I2C_TUNNEL ---- I2C/SPI ---- DMA

This PMIC is setting some FETS required for Backlight, LCD and G3D modules.

If all the I2C, SPI, DMA, I2C_TUNNEL, DRM based LCD are all mod_probes()
DRM drivers are probing a head of the PMIC probe. Which is causing the
display drivers to get "dummy_regulators" instead of the real supplies.

We couldn't implement -PROBE_DEFER because, the probes are not failing.

Should we should check if the return value of regulator_get() is a
"dummy_regulator"
and then defer the probe /.

>
>> I2C, I2C_TUNNEL, SPI and DMA drivers are required as subsys_initcall()
>> for similar reason.
>
> What makes you say this?  Typically those drivers don't use regulators
> at all.
I mean I2C, I2C_TUNNEL, SPI and DMA drivers have be to subsys_initcall()
to get the PMIC up intime. so, that the system would use the real supplies.

This looks like a board specific solution. I guess, this is quite
frequent with the
buses like I2C, SPI.

Kindly, suggest a nicer way. Thanks



-- 
Shine bright,
(: Nav :)

Re: [PATCH] i2c: exynos5: Initialise Samsung High Speed I2C controller early

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1538 bytes --]

On Fri, May 09, 2014 at 08:12:47PM +0530, Naveen Krishna Ch wrote:
> On 9 May 2014 19:21, Mark Brown <broonie@kernel.org> wrote:
> > On Fri, May 09, 2014 at 05:50:00PM +0530, Naveen Krishna Ch wrote:

> >> DRM related drivers like DP, FIMD, HDMI, Mixer wants to be probed ASAP
> >> during the boot.
> >> The real problem comes when, one of these drivers do a regulator_get().

> >> If the physical supply  is not enabled/hookedup the regulator_get() call
> >> assumes that physical supply is present and returns a
> >> "dummy_regulator" (But, not an error).

> >> Because of which, Display and several other devices fails to work.

> > These drivers are buggy, if they geniunely expect and handle a missing
> > supply then they should be using regulator_get_optional() to request the
> > regulator and even if they don't the use of subsys_initcall() is not
> > going to fix anything here - if a dummy regulator is going to be
> > returned the time things are probed won't make a difference.

...

> If all the I2C, SPI, DMA, I2C_TUNNEL, DRM based LCD are all mod_probes()
> DRM drivers are probing a head of the PMIC probe. Which is causing the
> display drivers to get "dummy_regulators" instead of the real supplies.

No, it really won't - I have no idea what you are doing but it's not
mainline.  If you are getting dummy regulators then you don't have a
supply present at all and probe ordering isn't going to make a blind
bit of difference.

Please provide a specific technical description of the problem you are
seeing in mainline.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] i2c: exynos5: Initialise Samsung High Speed I2C controller early

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]

On Fri, May 09, 2014 at 03:54:25PM +0100, Mark Brown wrote:
> On Fri, May 09, 2014 at 08:12:47PM +0530, Naveen Krishna Ch wrote:
> > On 9 May 2014 19:21, Mark Brown <broonie@kernel.org> wrote:
> > > On Fri, May 09, 2014 at 05:50:00PM +0530, Naveen Krishna Ch wrote:
> 
> > >> DRM related drivers like DP, FIMD, HDMI, Mixer wants to be probed ASAP
> > >> during the boot.
> > >> The real problem comes when, one of these drivers do a regulator_get().
> 
> > >> If the physical supply  is not enabled/hookedup the regulator_get() call
> > >> assumes that physical supply is present and returns a
> > >> "dummy_regulator" (But, not an error).
> 
> > >> Because of which, Display and several other devices fails to work.
> 
> > > These drivers are buggy, if they geniunely expect and handle a missing
> > > supply then they should be using regulator_get_optional() to request the
> > > regulator and even if they don't the use of subsys_initcall() is not
> > > going to fix anything here - if a dummy regulator is going to be
> > > returned the time things are probed won't make a difference.
> 
> ...
> 
> > If all the I2C, SPI, DMA, I2C_TUNNEL, DRM based LCD are all mod_probes()
> > DRM drivers are probing a head of the PMIC probe. Which is causing the
> > display drivers to get "dummy_regulators" instead of the real supplies.
> 
> No, it really won't - I have no idea what you are doing but it's not
> mainline.  If you are getting dummy regulators then you don't have a
> supply present at all and probe ordering isn't going to make a blind
> bit of difference.
> 
> Please provide a specific technical description of the problem you are
> seeing in mainline.

+1 Unless we have a clear understanding that this is the only acceptable
solution in mainline, this is really an out-of-tree patch.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] i2c: exynos5: Initialise Samsung High Speed I2C controller early

[Naveen Krishna Ch]

Hello Wolfram,

On 21 May 2014 15:55, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Fri, May 09, 2014 at 03:54:25PM +0100, Mark Brown wrote:
>> On Fri, May 09, 2014 at 08:12:47PM +0530, Naveen Krishna Ch wrote:
>> > On 9 May 2014 19:21, Mark Brown <broonie@kernel.org> wrote:
>> > > On Fri, May 09, 2014 at 05:50:00PM +0530, Naveen Krishna Ch wrote:
>>
>> > >> DRM related drivers like DP, FIMD, HDMI, Mixer wants to be probed ASAP
>> > >> during the boot.
>> > >> The real problem comes when, one of these drivers do a regulator_get().
>>
>> > >> If the physical supply  is not enabled/hookedup the regulator_get() call
>> > >> assumes that physical supply is present and returns a
>> > >> "dummy_regulator" (But, not an error).
>>
>> > >> Because of which, Display and several other devices fails to work.
>>
>> > > These drivers are buggy, if they geniunely expect and handle a missing
>> > > supply then they should be using regulator_get_optional() to request the
>> > > regulator and even if they don't the use of subsys_initcall() is not
>> > > going to fix anything here - if a dummy regulator is going to be
>> > > returned the time things are probed won't make a difference.
>>
>> ...
>>
>> > If all the I2C, SPI, DMA, I2C_TUNNEL, DRM based LCD are all mod_probes()
>> > DRM drivers are probing a head of the PMIC probe. Which is causing the
>> > display drivers to get "dummy_regulators" instead of the real supplies.
>>
>> No, it really won't - I have no idea what you are doing but it's not
>> mainline.  If you are getting dummy regulators then you don't have a
>> supply present at all and probe ordering isn't going to make a blind
>> bit of difference.
>>
>> Please provide a specific technical description of the problem you are
>> seeing in mainline.
>
> +1 Unless we have a clear understanding that this is the only acceptable
> solution in mainline, this is really an out-of-tree patch.

Few of the DRM based devices need regulators during their probe and
DRM subsystem
wants to bring up LCD as fast as it could to give a better user experience.

Exynos DRM (few others for that matter) does platform_driver_register()
of devices in a sequence and finally binds them all together into one
blob or /dev/dri-0 device.
Which makes it hard to implement -EPROBE_DEFER. Even if -EPROBE_DEFER is
implemented in DRM, it adds lot of overhead during the boot up.

I've proposed to make exynos_drm_init() as late_initcall() instead of making
I2C, SPI, DMA and I2C-arbitrator and I2C-TUNNEL drivers as subsys_initcall()

http://www.spinics.net/lists/linux-samsung-soc/msg30858.html

Kindly, suggest a workable approach for all the subsystems.
>

-- 
Shine bright,
(: Nav :)

Re: [PATCH] i2c: exynos5: Initialise Samsung High Speed I2C controller early

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 333 bytes --]


> Kindly, suggest a workable approach for all the subsystems.

Keep this patch out-of-tree? I know that probe ordering causes problems,
and that it needs major efforts. Yet, I understood that adding more and
more subsys_initcall to mainline is not going to help the process unless
essential. Using subsys_initcall has issues, too.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]