Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1183933 > unrolled thread

[PATCH] mmc: omap_hsmmc: Update driver to support without regulators

Started byFranklin S Cooper Jr <fcooper@ti.com>
First post2015-07-14 21:40 +0200
Last post2015-07-22 14:20 +0200
Articles 4 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] mmc: omap_hsmmc: Update driver to support without regulators Franklin S Cooper Jr <fcooper@ti.com> - 2015-07-14 21:40 +0200
    Re: [PATCH] mmc: omap_hsmmc: Update driver to support without regulators "Franklin S Cooper Jr." <fcooper@ti.com> - 2015-07-22 13:50 +0200
      Re: [PATCH] mmc: omap_hsmmc: Update driver to support without regulators Ulf Hansson <ulf.hansson@linaro.org> - 2015-07-22 14:10 +0200
        Re: [PATCH] mmc: omap_hsmmc: Update driver to support without regulators Kishon Vijay Abraham I <kishon@ti.com> - 2015-07-22 14:20 +0200

#1183933 — [PATCH] mmc: omap_hsmmc: Update driver to support without regulators

FromFranklin S Cooper Jr <fcooper@ti.com>
Date2015-07-14 21:40 +0200
Subject[PATCH] mmc: omap_hsmmc: Update driver to support without regulators
Message-ID<pMedz-1pd-7@gated-at.bofh.it>
From: Roger Quadros <rogerq@ti.com>

Update driver to support without regulators.

Without this patch boards that do not enable regulator config options will
fail to boot with a kernel panic.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
---
 Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt |  2 ++
 drivers/mmc/host/omap_hsmmc.c                           | 14 ++++++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
index 76bf087..2408e87 100644
--- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
+++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
@@ -22,6 +22,8 @@ ti,dual-volt: boolean, supports dual voltage cards
 ti,non-removable: non-removable slot (like eMMC)
 ti,needs-special-reset: Requires a special softreset sequence
 ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed
+voltage-ranges: Specify the voltage range supported if regulator framework
+isn't enabled.
 dmas: List of DMA specifiers with the controller specific format
 as described in the generic DMA client binding. A tx and rx
 specifier is required.
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index b2b411d..16c870f 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -1551,10 +1551,13 @@ static void omap_hsmmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	if (ios->power_mode != host->power_mode) {
 		switch (ios->power_mode) {
 		case MMC_POWER_OFF:
-			mmc_pdata(host)->set_power(host->dev, 0, 0);
+			if (host->use_reg)
+				mmc_pdata(host)->set_power(host->dev, 0, 0);
 			break;
 		case MMC_POWER_UP:
-			mmc_pdata(host)->set_power(host->dev, 1, ios->vdd);
+			if (host->use_reg)
+				mmc_pdata(host)->set_power(host->dev, 1,
+							   ios->vdd);
 			break;
 		case MMC_POWER_ON:
 			do_send_init_stream = 1;
@@ -2082,10 +2085,13 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
 		if (ret)
 			goto err_irq;
 		host->use_reg = 1;
+		mmc->ocr_avail = mmc_pdata(host)->ocr_mask;
+	} else {
+		ret = mmc_of_parse_voltage(pdev->dev.of_node, &mmc->ocr_avail);
+		if (ret)
+			goto err_irq;
 	}
 
-	mmc->ocr_avail = mmc_pdata(host)->ocr_mask;
-
 	omap_hsmmc_disable_irq(host);
 
 	/*
-- 
1.9.1

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

[toc] | [next] | [standalone]


#1189841

From"Franklin S Cooper Jr." <fcooper@ti.com>
Date2015-07-22 13:50 +0200
Message-ID<pP0H7-5KL-3@gated-at.bofh.it>
In reply to#1183933

On 07/21/2015 02:39 PM, Franklin S Cooper Jr. wrote:
>
> On 07/21/2015 06:40 AM, Ulf Hansson wrote:
>> On 14 July 2015 at 21:29, Franklin S Cooper Jr <fcooper@ti.com> wrote:
>>> From: Roger Quadros <rogerq@ti.com>
>>>
>>> Update driver to support without regulators.
>>>
>>> Without this patch boards that do not enable regulator config options will
>>> fail to boot with a kernel panic.
>> I guess that's because the rootfs is mounted on the card that doesn't
>> get detected, right?
> No at ramfs was used to validate this and was loaded via ethernet in U-boot.
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>> ---
>>>  Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt |  2 ++
>>>  drivers/mmc/host/omap_hsmmc.c                           | 14 ++++++++++----
>>>  2 files changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
>>> index 76bf087..2408e87 100644
>>> --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
>>> +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
>>> @@ -22,6 +22,8 @@ ti,dual-volt: boolean, supports dual voltage cards
>>>  ti,non-removable: non-removable slot (like eMMC)
>>>  ti,needs-special-reset: Requires a special softreset sequence
>>>  ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed
>>> +voltage-ranges: Specify the voltage range supported if regulator framework
>>> +isn't enabled.
>>>  dmas: List of DMA specifiers with the controller specific format
>>>  as described in the generic DMA client binding. A tx and rx
>>>  specifier is required.
>>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>>> index b2b411d..16c870f 100644
>>> --- a/drivers/mmc/host/omap_hsmmc.c
>>> +++ b/drivers/mmc/host/omap_hsmmc.c
>>> @@ -1551,10 +1551,13 @@ static void omap_hsmmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>>         if (ios->power_mode != host->power_mode) {
>>>                 switch (ios->power_mode) {
>>>                 case MMC_POWER_OFF:
>>> -                       mmc_pdata(host)->set_power(host->dev, 0, 0);
>>> +                       if (host->use_reg)
>>> +                               mmc_pdata(host)->set_power(host->dev, 0, 0);
>>>                         break;
>>>                 case MMC_POWER_UP:
>>> -                       mmc_pdata(host)->set_power(host->dev, 1, ios->vdd);
>>> +                       if (host->use_reg)
>>> +                               mmc_pdata(host)->set_power(host->dev, 1,
>>> +                                                          ios->vdd);
>>>                         break;
>>>                 case MMC_POWER_ON:
>>>                         do_send_init_stream = 1;
>>> @@ -2082,10 +2085,13 @@ static int omap_hsmmc_probe(struct platform_device *pdev)
>>>                 if (ret)
>>>                         goto err_irq;
>>>                 host->use_reg = 1;
>>> +               mmc->ocr_avail = mmc_pdata(host)->ocr_mask;
>>> +       } else {
>>> +               ret = mmc_of_parse_voltage(pdev->dev.of_node, &mmc->ocr_avail);
>>> +               if (ret)
>>> +                       goto err_irq;
>>>         }
>>>
>>> -       mmc->ocr_avail = mmc_pdata(host)->ocr_mask;
>>> -
>>>         omap_hsmmc_disable_irq(host);
>>>
>>>         /*
>>> --
>>> 1.9.1
>>>
>>> --
>>> 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
>> No, I don't like this patch. If your board have regulators which can
>> be used to find out the voltage levels, we shall use them. We shall
>> not use DT to workaround this.
>>
>> So, perhaps the Kconfig section for omap_hsmmc should select
>> CONFIG_REGULATOR (and friends)?
> Looking at the code the "ifdefCONFIG_REGULATOR" and the usage of use_reg
> (use regulator) shows that this driver is suppose to support boards that
> don't have the regulator framework enabled.
>
> This patches insures that this driver can work without the regulator framework
> which is currently broken.
Hi Ulf,

Just to clarify do you have an issue with this patch or in general
you rather nothave !CONFIG_REGULATOR supported? If its the latter
then wouldit be better to remove the !CONFIG_REGULATOR portion from
this driver all together.
>> Kind regards
>> Uffe
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[toc] | [prev] | [next] | [standalone]


#1189851

FromUlf Hansson <ulf.hansson@linaro.org>
Date2015-07-22 14:10 +0200
Message-ID<pP10u-6mP-25@gated-at.bofh.it>
In reply to#1189841
[...]

> Hi Ulf,
>
> Just to clarify do you have an issue with this patch or in general
> you rather nothave !CONFIG_REGULATOR supported? If its the latter
> then wouldit be better to remove the !CONFIG_REGULATOR portion from
> this driver all together.

I don't like this patch as is, as it seems like you are replacing the
VMMC regulator that actually exist in HW by parsing a DT property.

I think mmc_of_parse_voltage() shall *only* be used when the OCR mask
(->ocr_avail) can't be fetched by using a regulator. Typically that's
when the VMMC voltage levels are managed within the mmc controller
itself.

I don't have a strong opinion on how you solve that, but I guess you
have two options.

1.
Make sure the driver can be build and used both for CONFIG_REGULATOR
and !CONFIG_REGULATOR. What you need to take care of then, is to
assign the ->ocr_avail mask to a default value in the case when it
can't be fetched from a regulator. Most mmc drivers has some kind of
fall-back mechanism like this.

For those SoC where you need/can control the VMMC regulator, make sure
those enables CONFIG_REGULATOR to get the proper behaviour by
omap_hsmmc.

2.
Force omap_hsmmc to be build with CONFIG_REGULATOR.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1189855

FromKishon Vijay Abraham I <kishon@ti.com>
Date2015-07-22 14:20 +0200
Message-ID<pP1a9-6y6-3@gated-at.bofh.it>
In reply to#1189851
Hi,

On Wednesday 22 July 2015 05:37 PM, Ulf Hansson wrote:
> [...]
> 
>> Hi Ulf,
>>
>> Just to clarify do you have an issue with this patch or in general
>> you rather nothave !CONFIG_REGULATOR supported? If its the latter
>> then wouldit be better to remove the !CONFIG_REGULATOR portion from
>> this driver all together.
> 
> I don't like this patch as is, as it seems like you are replacing the
> VMMC regulator that actually exist in HW by parsing a DT property.
> 
> I think mmc_of_parse_voltage() shall *only* be used when the OCR mask
> (->ocr_avail) can't be fetched by using a regulator. Typically that's
> when the VMMC voltage levels are managed within the mmc controller
> itself.
> 
> I don't have a strong opinion on how you solve that, but I guess you
> have two options.
> 
> 1.
> Make sure the driver can be build and used both for CONFIG_REGULATOR
> and !CONFIG_REGULATOR. What you need to take care of then, is to
> assign the ->ocr_avail mask to a default value in the case when it
> can't be fetched from a regulator. Most mmc drivers has some kind of
> fall-back mechanism like this.

I'm working on cleaning up omap_hsmmc driver where I have taken care to avoid
dependency with CONFIG_REGULATOR. I'll post the series asap.

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web