Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1309108 > unrolled thread
| Started by | Kishon Vijay Abraham I <kishon@ti.com> |
|---|---|
| First post | 2016-01-14 10:20 +0100 |
| Last post | 2016-01-27 15:20 +0100 |
| Articles | 9 — 5 participants |
Back to article view | Back to linux.kernel
[PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS Kishon Vijay Abraham I <kishon@ti.com> - 2016-01-14 10:20 +0100
Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS Sebastian Reichel <sre@kernel.org> - 2016-01-14 13:10 +0100
Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS Kishon Vijay Abraham I <kishon@ti.com> - 2016-01-14 13:50 +0100
Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS Tony Lindgren <tony@atomide.com> - 2016-01-14 18:00 +0100
Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-01-14 18:30 +0100
Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS Sebastian Reichel <sre@kernel.org> - 2016-01-14 23:50 +0100
Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-01-15 10:20 +0100
Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS Tony Lindgren <tony@atomide.com> - 2016-01-15 17:10 +0100
Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS Ulf Hansson <ulf.hansson@linaro.org> - 2016-01-27 15:20 +0100
| From | Kishon Vijay Abraham I <kishon@ti.com> |
|---|---|
| Date | 2016-01-14 10:20 +0100 |
| Subject | [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS |
| Message-ID | <qQMev-4Y2-29@gated-at.bofh.it> |
Since v4.3+, CONFIG_REGULATOR_PBIAS should be enabled (for platforms that
have PBIAS regulator) in order for MMC1 to work.
Add a more verbose print to help enable CONFIG_REGULATOR_PBIAS for users
using a olddefconfig or a custom .config.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
drivers/mmc/host/omap_hsmmc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 7fb0753..7086bd6 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -503,8 +503,11 @@ static int omap_hsmmc_reg_get(struct omap_hsmmc_host *host)
host->pbias = devm_regulator_get_optional(host->dev, "pbias");
if (IS_ERR(host->pbias)) {
ret = PTR_ERR(host->pbias);
- if ((ret != -ENODEV) && host->dev->of_node)
+ if ((ret != -ENODEV) && host->dev->of_node) {
+ dev_err(host->dev,
+ "SD card detect fail? enable CONFIG_REGULATOR_PBIAS\n");
return ret;
+ }
dev_dbg(host->dev, "unable to get pbias regulator %ld\n",
PTR_ERR(host->pbias));
host->pbias = NULL;
--
1.7.9.5
[toc] | [next] | [standalone]
| From | Sebastian Reichel <sre@kernel.org> |
|---|---|
| Date | 2016-01-14 13:10 +0100 |
| Subject | Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS |
| Message-ID | <qQOSZ-6Lb-1@gated-at.bofh.it> |
| In reply to | #1309108 |
[Multipart message — attachments visible in raw view] — view raw
Hi,
On Thu, Jan 14, 2016 at 02:45:20PM +0530, Kishon Vijay Abraham I wrote:
> Since v4.3+, CONFIG_REGULATOR_PBIAS should be enabled (for platforms that
> have PBIAS regulator) in order for MMC1 to work.
>
> Add a more verbose print to help enable CONFIG_REGULATOR_PBIAS for users
> using a olddefconfig or a custom .config.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/mmc/host/omap_hsmmc.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 7fb0753..7086bd6 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -503,8 +503,11 @@ static int omap_hsmmc_reg_get(struct omap_hsmmc_host *host)
> host->pbias = devm_regulator_get_optional(host->dev, "pbias");
> if (IS_ERR(host->pbias)) {
> ret = PTR_ERR(host->pbias);
> - if ((ret != -ENODEV) && host->dev->of_node)
> + if ((ret != -ENODEV) && host->dev->of_node) {
> + dev_err(host->dev,
> + "SD card detect fail? enable CONFIG_REGULATOR_PBIAS\n");
> return ret;
> + }
> dev_dbg(host->dev, "unable to get pbias regulator %ld\n",
> PTR_ERR(host->pbias));
> host->pbias = NULL;
Is there a good reason against adding "depends on REGULATOR_PBIAS"
to MMC_OMAP_HS? I guess it's required for most systems and the regulator
driver is just 250 loc. I am aware, that this will disable the
hsmmc driver for 'make oldconfig', but I guess that will be noticed
in a similar way your warning is noticed.
The advantage of having the depends line is, that people configuring
their kernel have a chance to notice the regulator requirement
during configuration.
Anyways, this message improves the situation, so:
Acked-By: Sebastian Reichel <sre@kernel.org>
-- Sebastian
[toc] | [prev] | [next] | [standalone]
| From | Kishon Vijay Abraham I <kishon@ti.com> |
|---|---|
| Date | 2016-01-14 13:50 +0100 |
| Subject | Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS |
| Message-ID | <qQPvI-75K-13@gated-at.bofh.it> |
| In reply to | #1309208 |
Hi,
On Thursday 14 January 2016 05:37 PM, Sebastian Reichel wrote:
> Hi,
>
> On Thu, Jan 14, 2016 at 02:45:20PM +0530, Kishon Vijay Abraham I wrote:
>> Since v4.3+, CONFIG_REGULATOR_PBIAS should be enabled (for platforms that
>> have PBIAS regulator) in order for MMC1 to work.
>>
>> Add a more verbose print to help enable CONFIG_REGULATOR_PBIAS for users
>> using a olddefconfig or a custom .config.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>> drivers/mmc/host/omap_hsmmc.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>> index 7fb0753..7086bd6 100644
>> --- a/drivers/mmc/host/omap_hsmmc.c
>> +++ b/drivers/mmc/host/omap_hsmmc.c
>> @@ -503,8 +503,11 @@ static int omap_hsmmc_reg_get(struct omap_hsmmc_host *host)
>> host->pbias = devm_regulator_get_optional(host->dev, "pbias");
>> if (IS_ERR(host->pbias)) {
>> ret = PTR_ERR(host->pbias);
>> - if ((ret != -ENODEV) && host->dev->of_node)
>> + if ((ret != -ENODEV) && host->dev->of_node) {
>> + dev_err(host->dev,
>> + "SD card detect fail? enable CONFIG_REGULATOR_PBIAS\n");
>> return ret;
>> + }
>> dev_dbg(host->dev, "unable to get pbias regulator %ld\n",
>> PTR_ERR(host->pbias));
>> host->pbias = NULL;
>
>
> Is there a good reason against adding "depends on REGULATOR_PBIAS"
> to MMC_OMAP_HS? I guess it's required for most systems and the regulator
> driver is just 250 loc. I am aware, that this will disable the
> hsmmc driver for 'make oldconfig', but I guess that will be noticed
> in a similar way your warning is noticed.
hmm.. only MMC1 requires PBIAS so I'm not sure if it's okay to add depends on
REGULATOR_PBIAS for the HSMMC driver as such. And as you said it'll also
disable the hsmmc driver if oldconfig is used. But do we get a warning?
Thanks
Kishon
[toc] | [prev] | [next] | [standalone]
| From | Tony Lindgren <tony@atomide.com> |
|---|---|
| Date | 2016-01-14 18:00 +0100 |
| Subject | Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS |
| Message-ID | <qQTpF-1kn-29@gated-at.bofh.it> |
| In reply to | #1309248 |
* Kishon Vijay Abraham I <kishon@ti.com> [160114 04:39]: > On Thursday 14 January 2016 05:37 PM, Sebastian Reichel wrote: > > > > Is there a good reason against adding "depends on REGULATOR_PBIAS" > > to MMC_OMAP_HS? I guess it's required for most systems and the regulator > > driver is just 250 loc. I am aware, that this will disable the > > hsmmc driver for 'make oldconfig', but I guess that will be noticed > > in a similar way your warning is noticed. > > hmm.. only MMC1 requires PBIAS so I'm not sure if it's okay to add depends on > REGULATOR_PBIAS for the HSMMC driver as such. And as you said it'll also > disable the hsmmc driver if oldconfig is used. But do we get a warning? Adding depends there is a bit wrong, many designs are not using MMC1 but use eMMC on the other controllers instead. I think the runtime warning is best we can do here to keep things modular. Regards, Tony
[toc] | [prev] | [next] | [standalone]
| From | Russell King - ARM Linux <linux@arm.linux.org.uk> |
|---|---|
| Date | 2016-01-14 18:30 +0100 |
| Subject | Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS |
| Message-ID | <qQTSG-1KH-19@gated-at.bofh.it> |
| In reply to | #1309208 |
On Thu, Jan 14, 2016 at 01:07:06PM +0100, Sebastian Reichel wrote:
> Hi,
>
> On Thu, Jan 14, 2016 at 02:45:20PM +0530, Kishon Vijay Abraham I wrote:
> > Since v4.3+, CONFIG_REGULATOR_PBIAS should be enabled (for platforms that
> > have PBIAS regulator) in order for MMC1 to work.
> >
> > Add a more verbose print to help enable CONFIG_REGULATOR_PBIAS for users
> > using a olddefconfig or a custom .config.
> >
> > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> > ---
> > drivers/mmc/host/omap_hsmmc.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> > index 7fb0753..7086bd6 100644
> > --- a/drivers/mmc/host/omap_hsmmc.c
> > +++ b/drivers/mmc/host/omap_hsmmc.c
> > @@ -503,8 +503,11 @@ static int omap_hsmmc_reg_get(struct omap_hsmmc_host *host)
> > host->pbias = devm_regulator_get_optional(host->dev, "pbias");
> > if (IS_ERR(host->pbias)) {
> > ret = PTR_ERR(host->pbias);
> > - if ((ret != -ENODEV) && host->dev->of_node)
> > + if ((ret != -ENODEV) && host->dev->of_node) {
> > + dev_err(host->dev,
> > + "SD card detect fail? enable CONFIG_REGULATOR_PBIAS\n");
> > return ret;
> > + }
> > dev_dbg(host->dev, "unable to get pbias regulator %ld\n",
> > PTR_ERR(host->pbias));
> > host->pbias = NULL;
>
>
> Is there a good reason against adding "depends on REGULATOR_PBIAS"
> to MMC_OMAP_HS?
There are very good reasons not to do this: that will result in
configurations where MMC_OMAP_HS was set but without REGULATOR_PBIAS
ending up with MMC_OMAP_HS being disabled. That doesn't help the
root problem, which is "why has the kernel boot regressed for my
previous working configuration?"
The solution proposed here adds a message to the boot which points
out fair and square what needs to be done to rectify the boot
failure. Adding a dependency just brings up the question "where
has my MMC driver gone?"
--
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
[toc] | [prev] | [next] | [standalone]
| From | Sebastian Reichel <sre@kernel.org> |
|---|---|
| Date | 2016-01-14 23:50 +0100 |
| Subject | Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS |
| Message-ID | <qQYSl-5iF-5@gated-at.bofh.it> |
| In reply to | #1309503 |
[Multipart message — attachments visible in raw view] — view raw
Hi,
On Thu, Jan 14, 2016 at 05:25:49PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 14, 2016 at 01:07:06PM +0100, Sebastian Reichel wrote:
> > On Thu, Jan 14, 2016 at 02:45:20PM +0530, Kishon Vijay Abraham I wrote:
> > > Since v4.3+, CONFIG_REGULATOR_PBIAS should be enabled (for platforms that
> > > have PBIAS regulator) in order for MMC1 to work.
> > >
> > > Add a more verbose print to help enable CONFIG_REGULATOR_PBIAS for users
> > > using a olddefconfig or a custom .config.
> > >
> > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> > > ---
> > > drivers/mmc/host/omap_hsmmc.c | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> > > index 7fb0753..7086bd6 100644
> > > --- a/drivers/mmc/host/omap_hsmmc.c
> > > +++ b/drivers/mmc/host/omap_hsmmc.c
> > > @@ -503,8 +503,11 @@ static int omap_hsmmc_reg_get(struct omap_hsmmc_host *host)
> > > host->pbias = devm_regulator_get_optional(host->dev, "pbias");
> > > if (IS_ERR(host->pbias)) {
> > > ret = PTR_ERR(host->pbias);
> > > - if ((ret != -ENODEV) && host->dev->of_node)
> > > + if ((ret != -ENODEV) && host->dev->of_node) {
> > > + dev_err(host->dev,
> > > + "SD card detect fail? enable CONFIG_REGULATOR_PBIAS\n");
> > > return ret;
> > > + }
> > > dev_dbg(host->dev, "unable to get pbias regulator %ld\n",
> > > PTR_ERR(host->pbias));
> > > host->pbias = NULL;
> >
> >
> > Is there a good reason against adding "depends on REGULATOR_PBIAS"
> > to MMC_OMAP_HS?
>
> There are very good reasons not to do this: that will result in
> configurations where MMC_OMAP_HS was set but without REGULATOR_PBIAS
> ending up with MMC_OMAP_HS being disabled. That doesn't help the
> root problem, which is "why has the kernel boot regressed for my
> previous working configuration?"
>
> The solution proposed here adds a message to the boot which points
> out fair and square what needs to be done to rectify the boot
> failure. Adding a dependency just brings up the question "where
> has my MMC driver gone?"
The best thing would be to have no regression. Just printing a
message means I have to build another kernel. But more importantly
the message may not be visible by the user - e.g. if the display has
not yet been initialized.
-- Sebastian
[toc] | [prev] | [next] | [standalone]
| From | Russell King - ARM Linux <linux@arm.linux.org.uk> |
|---|---|
| Date | 2016-01-15 10:20 +0100 |
| Subject | Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS |
| Message-ID | <qR8I2-42i-11@gated-at.bofh.it> |
| In reply to | #1309733 |
On Thu, Jan 14, 2016 at 11:40:25PM +0100, Sebastian Reichel wrote: > On Thu, Jan 14, 2016 at 05:25:49PM +0000, Russell King - ARM Linux wrote: > > There are very good reasons not to do this: that will result in > > configurations where MMC_OMAP_HS was set but without REGULATOR_PBIAS > > ending up with MMC_OMAP_HS being disabled. That doesn't help the > > root problem, which is "why has the kernel boot regressed for my > > previous working configuration?" > > > > The solution proposed here adds a message to the boot which points > > out fair and square what needs to be done to rectify the boot > > failure. Adding a dependency just brings up the question "where > > has my MMC driver gone?" > > The best thing would be to have no regression. Just printing a > message means I have to build another kernel. But more importantly > the message may not be visible by the user - e.g. if the display has > not yet been initialized. I agree in principle, but that's not possible here (see Tony's mails on why the PBIAS stuff needs to be optional.) I'd agree with changing the Kconfig if PBIAS were a hard and fast requirement, but it isn't. -- RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[toc] | [prev] | [next] | [standalone]
| From | Tony Lindgren <tony@atomide.com> |
|---|---|
| Date | 2016-01-15 17:10 +0100 |
| Subject | Re: [PATCH] mmc: host: omap_hsmmc: add a verbose print to enable CONFIG_REGULATOR_PBIAS |
| Message-ID | <qRf6N-5r-5@gated-at.bofh.it> |
| In reply to | #1309980 |
* Russell King - ARM Linux <linux@arm.linux.org.uk> [160115 01:15]: > On Thu, Jan 14, 2016 at 11:40:25PM +0100, Sebastian Reichel wrote: > > On Thu, Jan 14, 2016 at 05:25:49PM +0000, Russell King - ARM Linux wrote: > > > There are very good reasons not to do this: that will result in > > > configurations where MMC_OMAP_HS was set but without REGULATOR_PBIAS > > > ending up with MMC_OMAP_HS being disabled. That doesn't help the > > > root problem, which is "why has the kernel boot regressed for my > > > previous working configuration?" > > > > > > The solution proposed here adds a message to the boot which points > > > out fair and square what needs to be done to rectify the boot > > > failure. Adding a dependency just brings up the question "where > > > has my MMC driver gone?" > > > > The best thing would be to have no regression. Just printing a > > message means I have to build another kernel. But more importantly > > the message may not be visible by the user - e.g. if the display has > > not yet been initialized. > > I agree in principle, but that's not possible here (see Tony's mails > on why the PBIAS stuff needs to be optional.) I'd agree with changing > the Kconfig if PBIAS were a hard and fast requirement, but it isn't. Yeah. Probably the best solution for cases like this is to boot with a small initrd. That way you can get usb console and networking going before mounting the real root and you will see kernel messages with only minimal things built in. You can build a small initramfs with CONFIG_INITRAMFS_SOURCE that is appended to the kernel so you can also boot the legacy user space too with that if needed. Regards, Tony
[toc] | [prev] | [next] | [standalone]
| From | Ulf Hansson <ulf.hansson@linaro.org> |
|---|---|
| Date | 2016-01-27 15:20 +0100 |
| Message-ID | <qVz6X-4J4-35@gated-at.bofh.it> |
| In reply to | #1309108 |
On 14 January 2016 at 10:15, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Since v4.3+, CONFIG_REGULATOR_PBIAS should be enabled (for platforms that
> have PBIAS regulator) in order for MMC1 to work.
>
> Add a more verbose print to help enable CONFIG_REGULATOR_PBIAS for users
> using a olddefconfig or a custom .config.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Thanks,applied for next!
Kind regards
Uffe
> ---
> drivers/mmc/host/omap_hsmmc.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 7fb0753..7086bd6 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -503,8 +503,11 @@ static int omap_hsmmc_reg_get(struct omap_hsmmc_host *host)
> host->pbias = devm_regulator_get_optional(host->dev, "pbias");
> if (IS_ERR(host->pbias)) {
> ret = PTR_ERR(host->pbias);
> - if ((ret != -ENODEV) && host->dev->of_node)
> + if ((ret != -ENODEV) && host->dev->of_node) {
> + dev_err(host->dev,
> + "SD card detect fail? enable CONFIG_REGULATOR_PBIAS\n");
> return ret;
> + }
> dev_dbg(host->dev, "unable to get pbias regulator %ld\n",
> PTR_ERR(host->pbias));
> host->pbias = NULL;
> --
> 1.7.9.5
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web