diff mbox

All OMAP platforms: MMC is broken

Message ID 20151006153710.GS23801@atomide.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tony Lindgren Oct. 6, 2015, 3:37 p.m. UTC
* Russell King - ARM Linux <linux@arm.linux.org.uk> [151006 08:04]:
> On Tue, Oct 06, 2015 at 02:44:25AM -0700, Tony Lindgren wrote:
> > 
> > Hmm DT-based boot finds the MMC card for LDP, dmesg below from DT boot [1].
> > Looks like you're on on -rc4 and not -rc3. My guess is that MMC is not
> > working for you with DT-based booting because you don't seem to have
> > CONFIG_REGULATOR_PBIAS in your seed config for. Care to try enabling that
> > for both your omap3 and omap4 seed config files?
> 
> This is precisely the kind of crap I'm objecting to.  New kernel versions
> come along, and things break because people add extra Kconfig symbols that
> previous versions did not rely upon - and there's no communication of
> what's required for new kernel versions.
> 
> This stuff needs documenting, so that people are aware what changes they
> need to make - please put something in Documentation/arm/OMAP which
> tracks what new additions are required to the Kconfig to keep things
> working.

Good idea, how about something like the following? AFAIK that's the
only .config option needed as MFD_SYSCON is selected by Kconfig
already.

Regrds,

Tony

8< ----------
From: Tony Lindgren <tony@atomide.com>
Date: Tue, 6 Oct 2015 08:33:16 -0700
Subject: [PATCH] Documentation: ARM: List new omap MMC requirements

Earlier the PBIAS regulator was optional, not so with recent
omap_hsmmc changes. To make things easier for people with
custom .config files, let's add minimal documentation for it
as suggested by Russell King <rmk+kernel@arm.linux.org.uk>.

Signed-off-by: Tony Lindgren <tony@atomide.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Russell King - ARM Linux Oct. 7, 2015, 12:45 p.m. UTC | #1
On Tue, Oct 06, 2015 at 08:37:11AM -0700, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [151006 08:04]:
> > On Tue, Oct 06, 2015 at 02:44:25AM -0700, Tony Lindgren wrote:
> > > 
> > > Hmm DT-based boot finds the MMC card for LDP, dmesg below from DT boot [1].
> > > Looks like you're on on -rc4 and not -rc3. My guess is that MMC is not
> > > working for you with DT-based booting because you don't seem to have
> > > CONFIG_REGULATOR_PBIAS in your seed config for. Care to try enabling that
> > > for both your omap3 and omap4 seed config files?
> > 
> > This is precisely the kind of crap I'm objecting to.  New kernel versions
> > come along, and things break because people add extra Kconfig symbols that
> > previous versions did not rely upon - and there's no communication of
> > what's required for new kernel versions.
> > 
> > This stuff needs documenting, so that people are aware what changes they
> > need to make - please put something in Documentation/arm/OMAP which
> > tracks what new additions are required to the Kconfig to keep things
> > working.
> 
> Good idea, how about something like the following? AFAIK that's the
> only .config option needed as MFD_SYSCON is selected by Kconfig
> already.

Right, that resolves the issue.  Now, what's the story on the revert
and the other patch - have they already been taken for -rc kernels?
Tony Lindgren Oct. 7, 2015, 1:26 p.m. UTC | #2
* Russell King - ARM Linux <linux@arm.linux.org.uk> [151007 05:50]:
> On Tue, Oct 06, 2015 at 08:37:11AM -0700, Tony Lindgren wrote:
> > * Russell King - ARM Linux <linux@arm.linux.org.uk> [151006 08:04]:
> > > On Tue, Oct 06, 2015 at 02:44:25AM -0700, Tony Lindgren wrote:
> > > > 
> > > > Hmm DT-based boot finds the MMC card for LDP, dmesg below from DT boot [1].
> > > > Looks like you're on on -rc4 and not -rc3. My guess is that MMC is not
> > > > working for you with DT-based booting because you don't seem to have
> > > > CONFIG_REGULATOR_PBIAS in your seed config for. Care to try enabling that
> > > > for both your omap3 and omap4 seed config files?
> > > 
> > > This is precisely the kind of crap I'm objecting to.  New kernel versions
> > > come along, and things break because people add extra Kconfig symbols that
> > > previous versions did not rely upon - and there's no communication of
> > > what's required for new kernel versions.
> > > 
> > > This stuff needs documenting, so that people are aware what changes they
> > > need to make - please put something in Documentation/arm/OMAP which
> > > tracks what new additions are required to the Kconfig to keep things
> > > working.
> > 
> > Good idea, how about something like the following? AFAIK that's the
> > only .config option needed as MFD_SYSCON is selected by Kconfig
> > already.
> 
> Right, that resolves the issue.  Now, what's the story on the revert
> and the other patch - have they already been taken for -rc kernels?

Not yet, I was waiting for the confirmation from you. I've now posted the
revert with proper description and the fix so Ulf can pick them up and
merged as regression fixes for the -rc series:

http://marc.info/?l=linux-omap&m=144422416621373&w=2
http://marc.info/?l=linux-omap&m=144422416921375&w=2

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson Oct. 7, 2015, 1:41 p.m. UTC | #3
On 7 October 2015 at 15:26, Tony Lindgren <tony@atomide.com> wrote:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [151007 05:50]:
>> On Tue, Oct 06, 2015 at 08:37:11AM -0700, Tony Lindgren wrote:
>> > * Russell King - ARM Linux <linux@arm.linux.org.uk> [151006 08:04]:
>> > > On Tue, Oct 06, 2015 at 02:44:25AM -0700, Tony Lindgren wrote:
>> > > >
>> > > > Hmm DT-based boot finds the MMC card for LDP, dmesg below from DT boot [1].
>> > > > Looks like you're on on -rc4 and not -rc3. My guess is that MMC is not
>> > > > working for you with DT-based booting because you don't seem to have
>> > > > CONFIG_REGULATOR_PBIAS in your seed config for. Care to try enabling that
>> > > > for both your omap3 and omap4 seed config files?
>> > >
>> > > This is precisely the kind of crap I'm objecting to.  New kernel versions
>> > > come along, and things break because people add extra Kconfig symbols that
>> > > previous versions did not rely upon - and there's no communication of
>> > > what's required for new kernel versions.
>> > >
>> > > This stuff needs documenting, so that people are aware what changes they
>> > > need to make - please put something in Documentation/arm/OMAP which
>> > > tracks what new additions are required to the Kconfig to keep things
>> > > working.
>> >
>> > Good idea, how about something like the following? AFAIK that's the
>> > only .config option needed as MFD_SYSCON is selected by Kconfig
>> > already.

Similar to MFD_SYSCON, why don't we have REGULATOR_PBIAS to be
selected when omap_hsmmc is being used?

It seems like that should also be a patch for the rc, right!?

>>
>> Right, that resolves the issue.  Now, what's the story on the revert
>> and the other patch - have they already been taken for -rc kernels?

Great!

>
> Not yet, I was waiting for the confirmation from you. I've now posted the
> revert with proper description and the fix so Ulf can pick them up and
> merged as regression fixes for the -rc series:

I am on it!

>
> http://marc.info/?l=linux-omap&m=144422416621373&w=2
> http://marc.info/?l=linux-omap&m=144422416921375&w=2

Russell, may I add your tested by tag for these?

Again, thanks!

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Oct. 7, 2015, 3:52 p.m. UTC | #4
* Ulf Hansson <ulf.hansson@linaro.org> [151007 06:46]:
> On 7 October 2015 at 15:26, Tony Lindgren <tony@atomide.com> wrote:
> >> > Good idea, how about something like the following? AFAIK that's the
> >> > only .config option needed as MFD_SYSCON is selected by Kconfig
> >> > already.
> 
> Similar to MFD_SYSCON, why don't we have REGULATOR_PBIAS to be
> selected when omap_hsmmc is being used?
> 
> It seems like that should also be a patch for the rc, right!?

Well selecting CONFIG_REGULATOR is still optional and force selecting
drivers usually leads into randconfig build issues sooner or later.
And we'd like to make everything into loadable modules eventually.

We could print a warning during MMC1 probe if REGULATOR_PBIAS
is not selected and attempt to continue probing?

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux Oct. 7, 2015, 5:53 p.m. UTC | #5
On Wed, Oct 07, 2015 at 03:41:46PM +0200, Ulf Hansson wrote:
> > http://marc.info/?l=linux-omap&m=144422416621373&w=2
> > http://marc.info/?l=linux-omap&m=144422416921375&w=2
> 
> Russell, may I add your tested by tag for these?

You may -

Tested-by: Russell King <rmk+kernel@arm.linux.org.uk>

Thanks.
Ulf Hansson Oct. 7, 2015, 7:40 p.m. UTC | #6
On 7 October 2015 at 17:52, Tony Lindgren <tony@atomide.com> wrote:
> * Ulf Hansson <ulf.hansson@linaro.org> [151007 06:46]:
>> On 7 October 2015 at 15:26, Tony Lindgren <tony@atomide.com> wrote:
>> >> > Good idea, how about something like the following? AFAIK that's the
>> >> > only .config option needed as MFD_SYSCON is selected by Kconfig
>> >> > already.
>>
>> Similar to MFD_SYSCON, why don't we have REGULATOR_PBIAS to be
>> selected when omap_hsmmc is being used?
>>
>> It seems like that should also be a patch for the rc, right!?
>
> Well selecting CONFIG_REGULATOR is still optional and force selecting
> drivers usually leads into randconfig build issues sooner or later.
> And we'd like to make everything into loadable modules eventually.

I am not sure I get your point. Perhaps I was too vague in what I suggested.

Unless we express the dependencies via Kconfig files (or perhaps via
updated defconfigs), how do you expect build/boot automated tools to
handle this?

*People* can of course manually poll a README to learn about new
dependencies for each new kernel version, but me personally would
prefer if don't have to.

>
> We could print a warning during MMC1 probe if REGULATOR_PBIAS
> is not selected and attempt to continue probing?

No thanks, it would sprinkle drivers with ugly code :-). I would also
expect that we would get warnings even when we shouldn't, especially
as a cross SoC/board driver may have different dependencies.

To me REGULATOR_PBIAS is a dependency required by a certain SoC/board
when MMC_OMAP_HS is selected. Isn't such dependency easiest dealt with
from SoC/board Kconfig files? Llike for example in the OMAP2 case,
arch/arm/mach-omap2/Kconfig.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kishon Vijay Abraham I Oct. 7, 2015, 11:13 p.m. UTC | #7
Hi,

On Thursday 08 October 2015 01:10 AM, Ulf Hansson wrote:
> On 7 October 2015 at 17:52, Tony Lindgren <tony@atomide.com> wrote:
>> * Ulf Hansson <ulf.hansson@linaro.org> [151007 06:46]:
>>> On 7 October 2015 at 15:26, Tony Lindgren <tony@atomide.com> wrote:
>>>>>> Good idea, how about something like the following? AFAIK that's the
>>>>>> only .config option needed as MFD_SYSCON is selected by Kconfig
>>>>>> already.
>>>
>>> Similar to MFD_SYSCON, why don't we have REGULATOR_PBIAS to be
>>> selected when omap_hsmmc is being used?
>>>
>>> It seems like that should also be a patch for the rc, right!?
>>
>> Well selecting CONFIG_REGULATOR is still optional and force selecting
>> drivers usually leads into randconfig build issues sooner or later.
>> And we'd like to make everything into loadable modules eventually.
> 
> I am not sure I get your point. Perhaps I was too vague in what I suggested.
> 
> Unless we express the dependencies via Kconfig files (or perhaps via
> updated defconfigs), how do you expect build/boot automated tools to
> handle this?

Both omap2plus_defconfig and multi_v7_defconfig has
CONFIG_PBIAS_REGULATOR enabled [1].

I think by using *depends on* in Kconfig, we'll end up in the same issue
faced by Russell (since even with that CONFIG_PBIAS_REGULATOR won't be
enabled) and using *select* can lead to randconfig errors.

[1] -> https://lkml.org/lkml/2015/9/3/119

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Oct. 8, 2015, 8:40 a.m. UTC | #8
* Kishon Vijay Abraham I <kishon@ti.com> [151007 16:18]:
> Hi,
> 
> On Thursday 08 October 2015 01:10 AM, Ulf Hansson wrote:
> > On 7 October 2015 at 17:52, Tony Lindgren <tony@atomide.com> wrote:
> >> * Ulf Hansson <ulf.hansson@linaro.org> [151007 06:46]:
> >>> On 7 October 2015 at 15:26, Tony Lindgren <tony@atomide.com> wrote:
> >>>>>> Good idea, how about something like the following? AFAIK that's the
> >>>>>> only .config option needed as MFD_SYSCON is selected by Kconfig
> >>>>>> already.
> >>>
> >>> Similar to MFD_SYSCON, why don't we have REGULATOR_PBIAS to be
> >>> selected when omap_hsmmc is being used?
> >>>
> >>> It seems like that should also be a patch for the rc, right!?
> >>
> >> Well selecting CONFIG_REGULATOR is still optional and force selecting
> >> drivers usually leads into randconfig build issues sooner or later.
> >> And we'd like to make everything into loadable modules eventually.
> > 
> > I am not sure I get your point. Perhaps I was too vague in what I suggested.
> > 
> > Unless we express the dependencies via Kconfig files (or perhaps via
> > updated defconfigs), how do you expect build/boot automated tools to
> > handle this?
> 
> Both omap2plus_defconfig and multi_v7_defconfig has
> CONFIG_PBIAS_REGULATOR enabled [1].
> 
> I think by using *depends on* in Kconfig, we'll end up in the same issue
> faced by Russell (since even with that CONFIG_PBIAS_REGULATOR won't be
> enabled) and using *select* can lead to randconfig errors.

Well the way distros deal with issues like this is have everything
possible as loadable modules. We should get the regulator_pbiaa
loaded automatically in that case as long as it's in the dts.. And
as long as we have the MODULE_DEVICE_TABLE entries right.

Maybe we could also use the composite device to indicate when MMC1
is using PBIAS regulator?

Regrads,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux Oct. 8, 2015, 9:35 a.m. UTC | #9
On Thu, Oct 08, 2015 at 01:40:21AM -0700, Tony Lindgren wrote:
> Well the way distros deal with issues like this is have everything
> possible as loadable modules. We should get the regulator_pbiaa
> loaded automatically in that case as long as it's in the dts.. And
> as long as we have the MODULE_DEVICE_TABLE entries right.

Assuming you have a rootfs which doesn't depend on one of those modules,
or an initramfs, and a way to get the built modules into that initial
filesystem.  Automated test boot systems do not have that luxury: the
generation of initramfs is distro specific, and would be very large if
it were to include all possible modules.

Some distros need their initramfs statically configured between "mount
a local rootfs" and "mount a nfs rootfs" and can't be changed once
generated.
Tony Lindgren Oct. 8, 2015, 9:56 a.m. UTC | #10
* Russell King - ARM Linux <linux@arm.linux.org.uk> [151008 02:40]:
> On Thu, Oct 08, 2015 at 01:40:21AM -0700, Tony Lindgren wrote:
> > Well the way distros deal with issues like this is have everything
> > possible as loadable modules. We should get the regulator_pbiaa
> > loaded automatically in that case as long as it's in the dts.. And
> > as long as we have the MODULE_DEVICE_TABLE entries right.
> 
> Assuming you have a rootfs which doesn't depend on one of those modules,
> or an initramfs, and a way to get the built modules into that initial
> filesystem.  Automated test boot systems do not have that luxury: the
> generation of initramfs is distro specific, and would be very large if
> it were to include all possible modules.

Right, we need to keep the kernel usable in all these configurations
somehow.

For the kernel generated minimal initramfs we could grep for the
needed modules in the dts file.

> Some distros need their initramfs statically configured between "mount
> a local rootfs" and "mount a nfs rootfs" and can't be changed once
> generated.

And some devices need also EHCI for NFSroot :)

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux Oct. 8, 2015, 10 a.m. UTC | #11
On Thu, Oct 08, 2015 at 02:56:36AM -0700, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [151008 02:40]:
> > On Thu, Oct 08, 2015 at 01:40:21AM -0700, Tony Lindgren wrote:
> > > Well the way distros deal with issues like this is have everything
> > > possible as loadable modules. We should get the regulator_pbiaa
> > > loaded automatically in that case as long as it's in the dts.. And
> > > as long as we have the MODULE_DEVICE_TABLE entries right.
> > 
> > Assuming you have a rootfs which doesn't depend on one of those modules,
> > or an initramfs, and a way to get the built modules into that initial
> > filesystem.  Automated test boot systems do not have that luxury: the
> > generation of initramfs is distro specific, and would be very large if
> > it were to include all possible modules.
> 
> Right, we need to keep the kernel usable in all these configurations
> somehow.
> 
> For the kernel generated minimal initramfs we could grep for the
> needed modules in the dts file.

Yes, you might be able to do that, but the only thing that can generate
the initramfs is the target system itself, or a similar target system.
For example, you can't sanely generate an initramfs for an ARM Ubuntu
system on an x86 Fedora host.

Distros have the luxury of always being able to do the initramfs
generation on the target system at installation or distro kernel update
time.
diff mbox

Patch

--- /dev/null
+++ b/Documentation/arm/OMAP/README
@@ -0,0 +1,7 @@ 
+This file contains documentation for running mainline
+kernel on omaps.
+
+KERNEL		NEW DEPENDENCIES
+v4.3+		Update is needed for custom .config files to make sure
+		CONFIG_REGULATOR_PBIAS is enabled for MMC1 to work
+		properly.