Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-20 Thread Kevin Liu 
2013/2/14 Ulf Hansson :
> On 14 February 2013 09:05, Marek Szyprowski  wrote:
>>
>> On 2/13/2013 12:35 PM, Mark Brown wrote:
>>>
>>> On Wed, Feb 13, 2013 at 08:45:56AM +0100, Guennadi Liakhovetski wrote:
>>> > On Wed, 13 Feb 2013, Marek Szyprowski wrote:
>>>
>>> > > BTW, mmc_regulator_get_ocrmask() won't work with continuous range
>>> > > regulators.
>>>
>>> > This seems like a problem, that has to be fixed...
>>>
>>> Indeed, what's the issue?
>>
>>
>> There are probably 2 issues:
>>
>> 1. mmc_regulator_get_ocrmask() works only with regulators which support
>> regulator_count_voltages() and regulator_list_voltage(). Recently support
>> for
>> continuous regulators have been merged. Such regulators doesn't provide
>> regulator_list_voltage() method, but are able to change/set voltage to the
>> given value. I agree that they are not very common, so right now we can
>> probably ignore them until the first board, which uses them arrives.
>>
>> 2. The second issue might be related to the testing of precise voltage
>> values
>> in the ocr mask, not the whole allowed ranges. Such issues in sdhci.c driver
>> has been recently fixed by commit cec2e216f72c6b5ccdadb60aadbe99821d744503
>> ("mmc: sdhci: Use regulator min/max voltage range according to spec"), but I
>> don't know MMC core code to judge if ocr mask is used for exact voltage
>> checking or only for checking the voltage ranges. However someone with good
>> mmc subsystem knowledge should check it.
>>
>
> Not 100% sure what your problem relates too here, but I am aware of an
> issue for how the mmc protocol layer are handling ocr_masks. Let me
> try to describe it here:
>
> 1. During "card init" mmc_power_up will be called for telling the host
> driver to provide power to the card. The level of voltage will be set
> to "ocr_avail" which means the highest supported voltage by the host.
> 2. At the protocol layer the card init sequence tries to negotiate to
> lowest possible ocr value from what the card and the host together
> supports. Once done, the ocr mask value will be cached into the host
> struct.
> 3. The host will informed about the new ocr mask from the protocol
> layer with mmc_select_voltage and it somewhere here the problems
> starts. No host are actually changing the voltage level at this state
> (MMC_POWER_ON) which is correct since it would likely mean violation
> of the spec. At the same time the protocol layer still believes the
> host has switched to operate at the new voltage level.

According to the spec, only during the initialization procedure, the
host is not allowed to change the operating voltage range.
In fact, mmc_select_voltage is called before init start in
mmc_attach_sd/mmc_attach_mmc/mmc_attach_sdio.
So voltage level should can be changed during MMC_POWER_ON.
sdhci.c did this way and worked well. But I noticed some other hosts
didn't change voltage for MMC_POWER_ON.
I think it should be changed.

> 4. So the host and the protocol layer are out of sync with regards to
> the ocr mask, which is why the cached ocr_mask in the host struct is
> reset when doing mmc_power_off. Otherwise the suspend/resume sequence
> would have been broken.
>

But there indeed exist issues that the host->ocr doesn't restore after
resume back and operation voltage use the highest voltage.
Just submitted below patch to fix this. Please help to review.
"[PATCH] mmc: core: restore ocr and operation voltage in resume"

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

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-20 Thread Kevin Liu 
2013/2/14 Ulf Hansson ulf.hans...@linaro.org:
 On 14 February 2013 09:05, Marek Szyprowski m.szyprow...@samsung.com wrote:

 On 2/13/2013 12:35 PM, Mark Brown wrote:

 On Wed, Feb 13, 2013 at 08:45:56AM +0100, Guennadi Liakhovetski wrote:
  On Wed, 13 Feb 2013, Marek Szyprowski wrote:

   BTW, mmc_regulator_get_ocrmask() won't work with continuous range
   regulators.

  This seems like a problem, that has to be fixed...

 Indeed, what's the issue?


 There are probably 2 issues:

 1. mmc_regulator_get_ocrmask() works only with regulators which support
 regulator_count_voltages() and regulator_list_voltage(). Recently support
 for
 continuous regulators have been merged. Such regulators doesn't provide
 regulator_list_voltage() method, but are able to change/set voltage to the
 given value. I agree that they are not very common, so right now we can
 probably ignore them until the first board, which uses them arrives.

 2. The second issue might be related to the testing of precise voltage
 values
 in the ocr mask, not the whole allowed ranges. Such issues in sdhci.c driver
 has been recently fixed by commit cec2e216f72c6b5ccdadb60aadbe99821d744503
 (mmc: sdhci: Use regulator min/max voltage range according to spec), but I
 don't know MMC core code to judge if ocr mask is used for exact voltage
 checking or only for checking the voltage ranges. However someone with good
 mmc subsystem knowledge should check it.


 Not 100% sure what your problem relates too here, but I am aware of an
 issue for how the mmc protocol layer are handling ocr_masks. Let me
 try to describe it here:

 1. During card init mmc_power_up will be called for telling the host
 driver to provide power to the card. The level of voltage will be set
 to ocr_avail which means the highest supported voltage by the host.
 2. At the protocol layer the card init sequence tries to negotiate to
 lowest possible ocr value from what the card and the host together
 supports. Once done, the ocr mask value will be cached into the host
 struct.
 3. The host will informed about the new ocr mask from the protocol
 layer with mmc_select_voltage and it somewhere here the problems
 starts. No host are actually changing the voltage level at this state
 (MMC_POWER_ON) which is correct since it would likely mean violation
 of the spec. At the same time the protocol layer still believes the
 host has switched to operate at the new voltage level.

According to the spec, only during the initialization procedure, the
host is not allowed to change the operating voltage range.
In fact, mmc_select_voltage is called before init start in
mmc_attach_sd/mmc_attach_mmc/mmc_attach_sdio.
So voltage level should can be changed during MMC_POWER_ON.
sdhci.c did this way and worked well. But I noticed some other hosts
didn't change voltage for MMC_POWER_ON.
I think it should be changed.

 4. So the host and the protocol layer are out of sync with regards to
 the ocr mask, which is why the cached ocr_mask in the host struct is
 reset when doing mmc_power_off. Otherwise the suspend/resume sequence
 would have been broken.


But there indeed exist issues that the host-ocr doesn't restore after
resume back and operation voltage use the highest voltage.
Just submitted below patch to fix this. Please help to review.
[PATCH] mmc: core: restore ocr and operation voltage in resume

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

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-14 Thread Ulf Hansson 
On 14 February 2013 09:05, Marek Szyprowski  wrote:
>
> On 2/13/2013 12:35 PM, Mark Brown wrote:
>>
>> On Wed, Feb 13, 2013 at 08:45:56AM +0100, Guennadi Liakhovetski wrote:
>> > On Wed, 13 Feb 2013, Marek Szyprowski wrote:
>>
>> > > BTW, mmc_regulator_get_ocrmask() won't work with continuous range
>> > > regulators.
>>
>> > This seems like a problem, that has to be fixed...
>>
>> Indeed, what's the issue?
>
>
> There are probably 2 issues:
>
> 1. mmc_regulator_get_ocrmask() works only with regulators which support
> regulator_count_voltages() and regulator_list_voltage(). Recently support
> for
> continuous regulators have been merged. Such regulators doesn't provide
> regulator_list_voltage() method, but are able to change/set voltage to the
> given value. I agree that they are not very common, so right now we can
> probably ignore them until the first board, which uses them arrives.
>
> 2. The second issue might be related to the testing of precise voltage
> values
> in the ocr mask, not the whole allowed ranges. Such issues in sdhci.c driver
> has been recently fixed by commit cec2e216f72c6b5ccdadb60aadbe99821d744503
> ("mmc: sdhci: Use regulator min/max voltage range according to spec"), but I
> don't know MMC core code to judge if ocr mask is used for exact voltage
> checking or only for checking the voltage ranges. However someone with good
> mmc subsystem knowledge should check it.
>

Not 100% sure what your problem relates too here, but I am aware of an
issue for how the mmc protocol layer are handling ocr_masks. Let me
try to describe it here:

1. During "card init" mmc_power_up will be called for telling the host
driver to provide power to the card. The level of voltage will be set
to "ocr_avail" which means the highest supported voltage by the host.
2. At the protocol layer the card init sequence tries to negotiate to
lowest possible ocr value from what the card and the host together
supports. Once done, the ocr mask value will be cached into the host
struct.
3. The host will informed about the new ocr mask from the protocol
layer with mmc_select_voltage and it somewhere here the problems
starts. No host are actually changing the voltage level at this state
(MMC_POWER_ON) which is correct since it would likely mean violation
of the spec. At the same time the protocol layer still believes the
host has switched to operate at the new voltage level.
4. So the host and the protocol layer are out of sync with regards to
the ocr mask, which is why the cached ocr_mask in the host struct is
reset when doing mmc_power_off. Otherwise the suspend/resume sequence
would have been broken.

I have been looking into a solution for the above problem, but has not
yet been able to finalize the task.

Hope this did not become more fussy now. :-)

>
> Best regards
> --
> Marek Szyprowski
> Samsung Poland R Center
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-14 Thread Mark Brown 
On Thu, Feb 14, 2013 at 09:05:43AM +0100, Marek Szyprowski wrote:

> 1. mmc_regulator_get_ocrmask() works only with regulators which support
> regulator_count_voltages() and regulator_list_voltage(). Recently
> support for
> continuous regulators have been merged. Such regulators doesn't provide
> regulator_list_voltage() method, but are able to change/set voltage to the
> given value. I agree that they are not very common, so right now we can
> probably ignore them until the first board, which uses them arrives.

OK, I think this should be changed to use regulator_is_supported_voltage()
to pick a range if list_voltage() isn't there, we don't want to list an
extremely large number of voltages so using list_voltage() for continous
regulators wouldn't make sense.

> 2. The second issue might be related to the testing of precise
> voltage values
> in the ocr mask, not the whole allowed ranges. Such issues in
> sdhci.c driver
> has been recently fixed by commit cec2e216f72c6b5ccdadb60aadbe99821d744503
> ("mmc: sdhci: Use regulator min/max voltage range according to spec"), but I
> don't know MMC core code to judge if ocr mask is used for exact voltage
> checking or only for checking the voltage ranges. However someone with good
> mmc subsystem knowledge should check it.

Looking at the code I'd expect it to work with continuous regulators, if
it doesn't we should fix that.


signature.asc
Description: Digital signature

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-14 Thread Marek Szyprowski 


On 2/13/2013 12:35 PM, Mark Brown wrote:

On Wed, Feb 13, 2013 at 08:45:56AM +0100, Guennadi Liakhovetski wrote:
> On Wed, 13 Feb 2013, Marek Szyprowski wrote:

> > BTW, mmc_regulator_get_ocrmask() won't work with continuous range 
regulators.

> This seems like a problem, that has to be fixed...

Indeed, what's the issue?


There are probably 2 issues:

1. mmc_regulator_get_ocrmask() works only with regulators which support
regulator_count_voltages() and regulator_list_voltage(). Recently 
support for

continuous regulators have been merged. Such regulators doesn't provide
regulator_list_voltage() method, but are able to change/set voltage to the
given value. I agree that they are not very common, so right now we can
probably ignore them until the first board, which uses them arrives.

2. The second issue might be related to the testing of precise voltage 
values
in the ocr mask, not the whole allowed ranges. Such issues in sdhci.c 
driver

has been recently fixed by commit cec2e216f72c6b5ccdadb60aadbe99821d744503
("mmc: sdhci: Use regulator min/max voltage range according to spec"), but I
don't know MMC core code to judge if ocr mask is used for exact voltage
checking or only for checking the voltage ranges. However someone with good
mmc subsystem knowledge should check it.

Best regards
--
Marek Szyprowski
Samsung Poland R Center


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

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-14 Thread Marek Szyprowski 


On 2/13/2013 12:35 PM, Mark Brown wrote:

On Wed, Feb 13, 2013 at 08:45:56AM +0100, Guennadi Liakhovetski wrote:
 On Wed, 13 Feb 2013, Marek Szyprowski wrote:

  BTW, mmc_regulator_get_ocrmask() won't work with continuous range 
regulators.

 This seems like a problem, that has to be fixed...

Indeed, what's the issue?


There are probably 2 issues:

1. mmc_regulator_get_ocrmask() works only with regulators which support
regulator_count_voltages() and regulator_list_voltage(). Recently 
support for

continuous regulators have been merged. Such regulators doesn't provide
regulator_list_voltage() method, but are able to change/set voltage to the
given value. I agree that they are not very common, so right now we can
probably ignore them until the first board, which uses them arrives.

2. The second issue might be related to the testing of precise voltage 
values
in the ocr mask, not the whole allowed ranges. Such issues in sdhci.c 
driver

has been recently fixed by commit cec2e216f72c6b5ccdadb60aadbe99821d744503
(mmc: sdhci: Use regulator min/max voltage range according to spec), but I
don't know MMC core code to judge if ocr mask is used for exact voltage
checking or only for checking the voltage ranges. However someone with good
mmc subsystem knowledge should check it.

Best regards
--
Marek Szyprowski
Samsung Poland RD Center


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

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-14 Thread Mark Brown 
On Thu, Feb 14, 2013 at 09:05:43AM +0100, Marek Szyprowski wrote:

 1. mmc_regulator_get_ocrmask() works only with regulators which support
 regulator_count_voltages() and regulator_list_voltage(). Recently
 support for
 continuous regulators have been merged. Such regulators doesn't provide
 regulator_list_voltage() method, but are able to change/set voltage to the
 given value. I agree that they are not very common, so right now we can
 probably ignore them until the first board, which uses them arrives.

OK, I think this should be changed to use regulator_is_supported_voltage()
to pick a range if list_voltage() isn't there, we don't want to list an
extremely large number of voltages so using list_voltage() for continous
regulators wouldn't make sense.

 2. The second issue might be related to the testing of precise
 voltage values
 in the ocr mask, not the whole allowed ranges. Such issues in
 sdhci.c driver
 has been recently fixed by commit cec2e216f72c6b5ccdadb60aadbe99821d744503
 (mmc: sdhci: Use regulator min/max voltage range according to spec), but I
 don't know MMC core code to judge if ocr mask is used for exact voltage
 checking or only for checking the voltage ranges. However someone with good
 mmc subsystem knowledge should check it.

Looking at the code I'd expect it to work with continuous regulators, if
it doesn't we should fix that.


signature.asc
Description: Digital signature

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-14 Thread Ulf Hansson 
On 14 February 2013 09:05, Marek Szyprowski m.szyprow...@samsung.com wrote:

 On 2/13/2013 12:35 PM, Mark Brown wrote:

 On Wed, Feb 13, 2013 at 08:45:56AM +0100, Guennadi Liakhovetski wrote:
  On Wed, 13 Feb 2013, Marek Szyprowski wrote:

   BTW, mmc_regulator_get_ocrmask() won't work with continuous range
   regulators.

  This seems like a problem, that has to be fixed...

 Indeed, what's the issue?


 There are probably 2 issues:

 1. mmc_regulator_get_ocrmask() works only with regulators which support
 regulator_count_voltages() and regulator_list_voltage(). Recently support
 for
 continuous regulators have been merged. Such regulators doesn't provide
 regulator_list_voltage() method, but are able to change/set voltage to the
 given value. I agree that they are not very common, so right now we can
 probably ignore them until the first board, which uses them arrives.

 2. The second issue might be related to the testing of precise voltage
 values
 in the ocr mask, not the whole allowed ranges. Such issues in sdhci.c driver
 has been recently fixed by commit cec2e216f72c6b5ccdadb60aadbe99821d744503
 (mmc: sdhci: Use regulator min/max voltage range according to spec), but I
 don't know MMC core code to judge if ocr mask is used for exact voltage
 checking or only for checking the voltage ranges. However someone with good
 mmc subsystem knowledge should check it.


Not 100% sure what your problem relates too here, but I am aware of an
issue for how the mmc protocol layer are handling ocr_masks. Let me
try to describe it here:

1. During card init mmc_power_up will be called for telling the host
driver to provide power to the card. The level of voltage will be set
to ocr_avail which means the highest supported voltage by the host.
2. At the protocol layer the card init sequence tries to negotiate to
lowest possible ocr value from what the card and the host together
supports. Once done, the ocr mask value will be cached into the host
struct.
3. The host will informed about the new ocr mask from the protocol
layer with mmc_select_voltage and it somewhere here the problems
starts. No host are actually changing the voltage level at this state
(MMC_POWER_ON) which is correct since it would likely mean violation
of the spec. At the same time the protocol layer still believes the
host has switched to operate at the new voltage level.
4. So the host and the protocol layer are out of sync with regards to
the ocr mask, which is why the cached ocr_mask in the host struct is
reset when doing mmc_power_off. Otherwise the suspend/resume sequence
would have been broken.

I have been looking into a solution for the above problem, but has not
yet been able to finalize the task.

Hope this did not become more fussy now. :-)


 Best regards
 --
 Marek Szyprowski
 Samsung Poland RD Center


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

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

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-13 Thread Mark Brown 
On Wed, Feb 13, 2013 at 08:45:56AM +0100, Guennadi Liakhovetski wrote:
> On Wed, 13 Feb 2013, Marek Szyprowski wrote:

> > BTW, mmc_regulator_get_ocrmask() won't work with continuous range 
> > regulators.

> This seems like a problem, that has to be fixed...

Indeed, what's the issue?


signature.asc
Description: Digital signature

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-13 Thread Mark Brown 
On Wed, Feb 13, 2013 at 08:45:56AM +0100, Guennadi Liakhovetski wrote:
 On Wed, 13 Feb 2013, Marek Szyprowski wrote:

  BTW, mmc_regulator_get_ocrmask() won't work with continuous range 
  regulators.

 This seems like a problem, that has to be fixed...

Indeed, what's the issue?


signature.asc
Description: Digital signature

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-12 Thread Guennadi Liakhovetski 
On Wed, 13 Feb 2013, Marek Szyprowski wrote:

> Hello,
> 
> On 2/12/2013 11:10 PM, Guennadi Liakhovetski wrote:
> > Hi Marek
> > 
> > On Tue, 12 Feb 2013, Marek Szyprowski wrote:
> > 
> > > Some regulators don't report any voltage values, so checking supported
> > > voltage range results in disabling all SDHCI_CAN_VDD_* flags and
> > > registration failure. This patch finally provides a correct fix for the
> > > registration of SDHCI driver with all possible voltage regulators:
> > > dummy, fixed and regulated without using regulator_count_voltages()
> > > hacks.
> > >
> > > Signed-off-by: Marek Szyprowski 
> > > ---
> > >  drivers/mmc/host/sdhci.c |6 +-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > > index ba586ae..735526b 100644
> > > --- a/drivers/mmc/host/sdhci.c
> > > +++ b/drivers/mmc/host/sdhci.c
> > > @@ -2976,7 +2976,11 @@ int sdhci_add_host(struct sdhci_host *host)
> > >   }
> > >
> > >  #ifdef CONFIG_REGULATOR
> > > - if (host->vmmc) {
> > > + /*
> > > +  * Voltage range check makes sense only if regulator reports
> > > +  * any voltage value.
> > > +  */
> > > + if (host->vmmc && regulator_get_voltage(host->vmmc) > 0) {
> > >   ret = regulator_is_supported_voltage(host->vmmc, 270,
> > >   360);
> > 
> > Wouldn't using mmc_regulator_get_ocrmask() be a better option?
> 
> The idea behind this patch it to avoid messing ocr mask and voltage
> regulators when voltage regulator is a simple on/off switch, which doesn't
> report any value.

Wouldn't mmc_regulator_get_ocrmask() also report an error back in this 
case?

> This solves the serious problems with sdhci driver when
> dummy regulator is enabled in kconfig, otherwise the sdhci driver
> concludes that no supported voltage is available and fails to initialize.
> Using mmc_regulator_get_ocrmask() won't solve this problem.
> 
> BTW, mmc_regulator_get_ocrmask() won't work with continuous range regulators.

This seems like a problem, that has to be fixed...

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-12 Thread Marek Szyprowski 

Hello,

On 2/12/2013 11:10 PM, Guennadi Liakhovetski wrote:

Hi Marek

On Tue, 12 Feb 2013, Marek Szyprowski wrote:

> Some regulators don't report any voltage values, so checking supported
> voltage range results in disabling all SDHCI_CAN_VDD_* flags and
> registration failure. This patch finally provides a correct fix for the
> registration of SDHCI driver with all possible voltage regulators:
> dummy, fixed and regulated without using regulator_count_voltages()
> hacks.
>
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/mmc/host/sdhci.c |6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index ba586ae..735526b 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2976,7 +2976,11 @@ int sdhci_add_host(struct sdhci_host *host)
>}
>
>  #ifdef CONFIG_REGULATOR
> -  if (host->vmmc) {
> +  /*
> +   * Voltage range check makes sense only if regulator reports
> +   * any voltage value.
> +   */
> +  if (host->vmmc && regulator_get_voltage(host->vmmc) > 0) {
>ret = regulator_is_supported_voltage(host->vmmc, 270,
>360);

Wouldn't using mmc_regulator_get_ocrmask() be a better option?


The idea behind this patch it to avoid messing ocr mask and voltage
regulators when voltage regulator is a simple on/off switch, which doesn't
report any value. This solves the serious problems with sdhci driver when
dummy regulator is enabled in kconfig, otherwise the sdhci driver
concludes that no supported voltage is available and fails to initialize.
Using mmc_regulator_get_ocrmask() won't solve this problem.

BTW, mmc_regulator_get_ocrmask() won't work with continuous range 
regulators.


Best regards
--
Marek Szyprowski
Samsung Poland R Center
 



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

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-12 Thread Guennadi Liakhovetski 
Hi Marek

On Tue, 12 Feb 2013, Marek Szyprowski wrote:

> Some regulators don't report any voltage values, so checking supported
> voltage range results in disabling all SDHCI_CAN_VDD_* flags and
> registration failure. This patch finally provides a correct fix for the
> registration of SDHCI driver with all possible voltage regulators:
> dummy, fixed and regulated without using regulator_count_voltages()
> hacks.
> 
> Signed-off-by: Marek Szyprowski 
> ---
>  drivers/mmc/host/sdhci.c |6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index ba586ae..735526b 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2976,7 +2976,11 @@ int sdhci_add_host(struct sdhci_host *host)
>   }
>  
>  #ifdef CONFIG_REGULATOR
> - if (host->vmmc) {
> + /*
> +  * Voltage range check makes sense only if regulator reports
> +  * any voltage value.
> +  */
> + if (host->vmmc && regulator_get_voltage(host->vmmc) > 0) {
>   ret = regulator_is_supported_voltage(host->vmmc, 270,
>   360);

Wouldn't using mmc_regulator_get_ocrmask() be a better option?

Thanks
Guennadi

>   if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-12 Thread Marek Szyprowski 
Some regulators don't report any voltage values, so checking supported
voltage range results in disabling all SDHCI_CAN_VDD_* flags and
registration failure. This patch finally provides a correct fix for the
registration of SDHCI driver with all possible voltage regulators:
dummy, fixed and regulated without using regulator_count_voltages()
hacks.

Signed-off-by: Marek Szyprowski 
---
 drivers/mmc/host/sdhci.c |6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ba586ae..735526b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2976,7 +2976,11 @@ int sdhci_add_host(struct sdhci_host *host)
}
 
 #ifdef CONFIG_REGULATOR
-   if (host->vmmc) {
+   /*
+* Voltage range check makes sense only if regulator reports
+* any voltage value.
+*/
+   if (host->vmmc && regulator_get_voltage(host->vmmc) > 0) {
ret = regulator_is_supported_voltage(host->vmmc, 270,
360);
if ((ret <= 0) || (!(caps[0] & SDHCI_CAN_VDD_330)))
-- 
1.7.9.5

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

[PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-12 Thread Marek Szyprowski 
Some regulators don't report any voltage values, so checking supported
voltage range results in disabling all SDHCI_CAN_VDD_* flags and
registration failure. This patch finally provides a correct fix for the
registration of SDHCI driver with all possible voltage regulators:
dummy, fixed and regulated without using regulator_count_voltages()
hacks.

Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
 drivers/mmc/host/sdhci.c |6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index ba586ae..735526b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2976,7 +2976,11 @@ int sdhci_add_host(struct sdhci_host *host)
}
 
 #ifdef CONFIG_REGULATOR
-   if (host-vmmc) {
+   /*
+* Voltage range check makes sense only if regulator reports
+* any voltage value.
+*/
+   if (host-vmmc  regulator_get_voltage(host-vmmc)  0) {
ret = regulator_is_supported_voltage(host-vmmc, 270,
360);
if ((ret = 0) || (!(caps[0]  SDHCI_CAN_VDD_330)))
-- 
1.7.9.5

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

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-12 Thread Guennadi Liakhovetski 
Hi Marek

On Tue, 12 Feb 2013, Marek Szyprowski wrote:

 Some regulators don't report any voltage values, so checking supported
 voltage range results in disabling all SDHCI_CAN_VDD_* flags and
 registration failure. This patch finally provides a correct fix for the
 registration of SDHCI driver with all possible voltage regulators:
 dummy, fixed and regulated without using regulator_count_voltages()
 hacks.
 
 Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
 ---
  drivers/mmc/host/sdhci.c |6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
 index ba586ae..735526b 100644
 --- a/drivers/mmc/host/sdhci.c
 +++ b/drivers/mmc/host/sdhci.c
 @@ -2976,7 +2976,11 @@ int sdhci_add_host(struct sdhci_host *host)
   }
  
  #ifdef CONFIG_REGULATOR
 - if (host-vmmc) {
 + /*
 +  * Voltage range check makes sense only if regulator reports
 +  * any voltage value.
 +  */
 + if (host-vmmc  regulator_get_voltage(host-vmmc)  0) {
   ret = regulator_is_supported_voltage(host-vmmc, 270,
   360);

Wouldn't using mmc_regulator_get_ocrmask() be a better option?

Thanks
Guennadi

   if ((ret = 0) || (!(caps[0]  SDHCI_CAN_VDD_330)))
 -- 
 1.7.9.5
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-mmc in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-12 Thread Marek Szyprowski 

Hello,

On 2/12/2013 11:10 PM, Guennadi Liakhovetski wrote:

Hi Marek

On Tue, 12 Feb 2013, Marek Szyprowski wrote:

 Some regulators don't report any voltage values, so checking supported
 voltage range results in disabling all SDHCI_CAN_VDD_* flags and
 registration failure. This patch finally provides a correct fix for the
 registration of SDHCI driver with all possible voltage regulators:
 dummy, fixed and regulated without using regulator_count_voltages()
 hacks.

 Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
 ---
  drivers/mmc/host/sdhci.c |6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

 diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
 index ba586ae..735526b 100644
 --- a/drivers/mmc/host/sdhci.c
 +++ b/drivers/mmc/host/sdhci.c
 @@ -2976,7 +2976,11 @@ int sdhci_add_host(struct sdhci_host *host)
}

  #ifdef CONFIG_REGULATOR
 -  if (host-vmmc) {
 +  /*
 +   * Voltage range check makes sense only if regulator reports
 +   * any voltage value.
 +   */
 +  if (host-vmmc  regulator_get_voltage(host-vmmc)  0) {
ret = regulator_is_supported_voltage(host-vmmc, 270,
360);

Wouldn't using mmc_regulator_get_ocrmask() be a better option?


The idea behind this patch it to avoid messing ocr mask and voltage
regulators when voltage regulator is a simple on/off switch, which doesn't
report any value. This solves the serious problems with sdhci driver when
dummy regulator is enabled in kconfig, otherwise the sdhci driver
concludes that no supported voltage is available and fails to initialize.
Using mmc_regulator_get_ocrmask() won't solve this problem.

BTW, mmc_regulator_get_ocrmask() won't work with continuous range 
regulators.


Best regards
--
Marek Szyprowski
Samsung Poland RD Center
 



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

Re: [PATCH 3/3 RESEND] mmc: sdhci: check voltage range only on regulators aware of voltage value

2013-02-12 Thread Guennadi Liakhovetski 
On Wed, 13 Feb 2013, Marek Szyprowski wrote:

 Hello,
 
 On 2/12/2013 11:10 PM, Guennadi Liakhovetski wrote:
  Hi Marek
  
  On Tue, 12 Feb 2013, Marek Szyprowski wrote:
  
   Some regulators don't report any voltage values, so checking supported
   voltage range results in disabling all SDHCI_CAN_VDD_* flags and
   registration failure. This patch finally provides a correct fix for the
   registration of SDHCI driver with all possible voltage regulators:
   dummy, fixed and regulated without using regulator_count_voltages()
   hacks.
  
   Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
   ---
drivers/mmc/host/sdhci.c |6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
  
   diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
   index ba586ae..735526b 100644
   --- a/drivers/mmc/host/sdhci.c
   +++ b/drivers/mmc/host/sdhci.c
   @@ -2976,7 +2976,11 @@ int sdhci_add_host(struct sdhci_host *host)
 }
  
#ifdef CONFIG_REGULATOR
   - if (host-vmmc) {
   + /*
   +  * Voltage range check makes sense only if regulator reports
   +  * any voltage value.
   +  */
   + if (host-vmmc  regulator_get_voltage(host-vmmc)  0) {
 ret = regulator_is_supported_voltage(host-vmmc, 270,
 360);
  
  Wouldn't using mmc_regulator_get_ocrmask() be a better option?
 
 The idea behind this patch it to avoid messing ocr mask and voltage
 regulators when voltage regulator is a simple on/off switch, which doesn't
 report any value.

Wouldn't mmc_regulator_get_ocrmask() also report an error back in this 
case?

 This solves the serious problems with sdhci driver when
 dummy regulator is enabled in kconfig, otherwise the sdhci driver
 concludes that no supported voltage is available and fails to initialize.
 Using mmc_regulator_get_ocrmask() won't solve this problem.
 
 BTW, mmc_regulator_get_ocrmask() won't work with continuous range regulators.

This seems like a problem, that has to be fixed...

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/