All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] mmc: sdhci: Map more voltage level to SDHCI_POWER_330
@ 2021-09-26 13:28 Shawn Guo
  2021-09-30 11:00 ` Ulf Hansson
  0 siblings, 1 reply; 6+ messages in thread
From: Shawn Guo @ 2021-09-26 13:28 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Ulf Hansson, Bjorn Andersson, linux-mmc, linux-arm-msm,
	linux-kernel, Shawn Guo

On Thundercomm TurboX CM2290, the eMMC OCR reports vdd = 23 (3.5 ~ 3.6 V),
which is being treated as an invalid value by sdhci_set_power_noreg().
And thus eMMC is totally broken on the platform.

[    1.436599] ------------[ cut here ]------------
[    1.436606] mmc0: Invalid vdd 0x17
[    1.436640] WARNING: CPU: 2 PID: 69 at drivers/mmc/host/sdhci.c:2048 sdhci_set_power_noreg+0x168/0x2b4
[    1.436655] Modules linked in:
[    1.436662] CPU: 2 PID: 69 Comm: kworker/u8:1 Tainted: G        W         5.15.0-rc1+ #137
[    1.436669] Hardware name: Thundercomm TurboX CM2290 (DT)
[    1.436674] Workqueue: events_unbound async_run_entry_fn
[    1.436685] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    1.436692] pc : sdhci_set_power_noreg+0x168/0x2b4
[    1.436698] lr : sdhci_set_power_noreg+0x168/0x2b4
[    1.436703] sp : ffff800010803a60
[    1.436705] x29: ffff800010803a60 x28: ffff6a9102465f00 x27: ffff6a9101720a70
[    1.436715] x26: ffff6a91014de1c0 x25: ffff6a91014de010 x24: ffff6a91016af280
[    1.436724] x23: ffffaf7b1b276640 x22: 0000000000000000 x21: ffff6a9101720000
[    1.436733] x20: ffff6a9101720370 x19: ffff6a9101720580 x18: 0000000000000020
[    1.436743] x17: 0000000000000000 x16: 0000000000000004 x15: ffffffffffffffff
[    1.436751] x14: 0000000000000000 x13: 00000000fffffffd x12: ffffaf7b1b84b0bc
[    1.436760] x11: ffffaf7b1b720d10 x10: 000000000000000a x9 : ffff800010803a60
[    1.436769] x8 : 000000000000000a x7 : 000000000000000f x6 : 00000000fffff159
[    1.436778] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
[    1.436787] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff6a9101718d80
[    1.436797] Call trace:
[    1.436800]  sdhci_set_power_noreg+0x168/0x2b4
[    1.436805]  sdhci_set_ios+0xa0/0x7fc
[    1.436811]  mmc_power_up.part.0+0xc4/0x164
[    1.436818]  mmc_start_host+0xa0/0xb0
[    1.436824]  mmc_add_host+0x60/0x90
[    1.436830]  __sdhci_add_host+0x174/0x330
[    1.436836]  sdhci_msm_probe+0x7c0/0x920
[    1.436842]  platform_probe+0x68/0xe0
[    1.436850]  really_probe.part.0+0x9c/0x31c
[    1.436857]  __driver_probe_device+0x98/0x144
[    1.436863]  driver_probe_device+0xc8/0x15c
[    1.436869]  __device_attach_driver+0xb4/0x120
[    1.436875]  bus_for_each_drv+0x78/0xd0
[    1.436881]  __device_attach_async_helper+0xac/0xd0
[    1.436888]  async_run_entry_fn+0x34/0x110
[    1.436895]  process_one_work+0x1d0/0x354
[    1.436903]  worker_thread+0x13c/0x470
[    1.436910]  kthread+0x150/0x160
[    1.436915]  ret_from_fork+0x10/0x20
[    1.436923] ---[ end trace fcfac44cb045c3a8 ]---

Fix the issue by mapping MMC_VDD_35_36 (and MMC_VDD_34_35) to
SDHCI_POWER_330 as well.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
I'm not sure if this is the right solution, as I do not have SDHCI
specification.  Hence it's a RFC.

 drivers/mmc/host/sdhci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 8eefa7d5fe85..2427481535a3 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2042,6 +2042,8 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
 			break;
 		case MMC_VDD_32_33:
 		case MMC_VDD_33_34:
+		case MMC_VDD_34_35:
+		case MMC_VDD_35_36:
 			pwr = SDHCI_POWER_330;
 			break;
 		default:
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] mmc: sdhci: Map more voltage level to SDHCI_POWER_330
  2021-09-26 13:28 [RFC PATCH] mmc: sdhci: Map more voltage level to SDHCI_POWER_330 Shawn Guo
@ 2021-09-30 11:00 ` Ulf Hansson
  2021-10-03 13:58   ` Shawn Guo
  0 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2021-09-30 11:00 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Adrian Hunter, Bjorn Andersson, linux-mmc, linux-arm-msm,
	Linux Kernel Mailing List

On Sun, 26 Sept 2021 at 15:28, Shawn Guo <shawn.guo@linaro.org> wrote:
>
> On Thundercomm TurboX CM2290, the eMMC OCR reports vdd = 23 (3.5 ~ 3.6 V),
> which is being treated as an invalid value by sdhci_set_power_noreg().
> And thus eMMC is totally broken on the platform.
>
> [    1.436599] ------------[ cut here ]------------
> [    1.436606] mmc0: Invalid vdd 0x17
> [    1.436640] WARNING: CPU: 2 PID: 69 at drivers/mmc/host/sdhci.c:2048 sdhci_set_power_noreg+0x168/0x2b4
> [    1.436655] Modules linked in:
> [    1.436662] CPU: 2 PID: 69 Comm: kworker/u8:1 Tainted: G        W         5.15.0-rc1+ #137
> [    1.436669] Hardware name: Thundercomm TurboX CM2290 (DT)
> [    1.436674] Workqueue: events_unbound async_run_entry_fn
> [    1.436685] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    1.436692] pc : sdhci_set_power_noreg+0x168/0x2b4
> [    1.436698] lr : sdhci_set_power_noreg+0x168/0x2b4
> [    1.436703] sp : ffff800010803a60
> [    1.436705] x29: ffff800010803a60 x28: ffff6a9102465f00 x27: ffff6a9101720a70
> [    1.436715] x26: ffff6a91014de1c0 x25: ffff6a91014de010 x24: ffff6a91016af280
> [    1.436724] x23: ffffaf7b1b276640 x22: 0000000000000000 x21: ffff6a9101720000
> [    1.436733] x20: ffff6a9101720370 x19: ffff6a9101720580 x18: 0000000000000020
> [    1.436743] x17: 0000000000000000 x16: 0000000000000004 x15: ffffffffffffffff
> [    1.436751] x14: 0000000000000000 x13: 00000000fffffffd x12: ffffaf7b1b84b0bc
> [    1.436760] x11: ffffaf7b1b720d10 x10: 000000000000000a x9 : ffff800010803a60
> [    1.436769] x8 : 000000000000000a x7 : 000000000000000f x6 : 00000000fffff159
> [    1.436778] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
> [    1.436787] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff6a9101718d80
> [    1.436797] Call trace:
> [    1.436800]  sdhci_set_power_noreg+0x168/0x2b4
> [    1.436805]  sdhci_set_ios+0xa0/0x7fc
> [    1.436811]  mmc_power_up.part.0+0xc4/0x164
> [    1.436818]  mmc_start_host+0xa0/0xb0
> [    1.436824]  mmc_add_host+0x60/0x90
> [    1.436830]  __sdhci_add_host+0x174/0x330
> [    1.436836]  sdhci_msm_probe+0x7c0/0x920
> [    1.436842]  platform_probe+0x68/0xe0
> [    1.436850]  really_probe.part.0+0x9c/0x31c
> [    1.436857]  __driver_probe_device+0x98/0x144
> [    1.436863]  driver_probe_device+0xc8/0x15c
> [    1.436869]  __device_attach_driver+0xb4/0x120
> [    1.436875]  bus_for_each_drv+0x78/0xd0
> [    1.436881]  __device_attach_async_helper+0xac/0xd0
> [    1.436888]  async_run_entry_fn+0x34/0x110
> [    1.436895]  process_one_work+0x1d0/0x354
> [    1.436903]  worker_thread+0x13c/0x470
> [    1.436910]  kthread+0x150/0x160
> [    1.436915]  ret_from_fork+0x10/0x20
> [    1.436923] ---[ end trace fcfac44cb045c3a8 ]---
>
> Fix the issue by mapping MMC_VDD_35_36 (and MMC_VDD_34_35) to
> SDHCI_POWER_330 as well.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> I'm not sure if this is the right solution, as I do not have SDHCI
> specification.  Hence it's a RFC.
>
>  drivers/mmc/host/sdhci.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 8eefa7d5fe85..2427481535a3 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2042,6 +2042,8 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
>                         break;
>                 case MMC_VDD_32_33:
>                 case MMC_VDD_33_34:
> +               case MMC_VDD_34_35:
> +               case MMC_VDD_35_36:
>                         pwr = SDHCI_POWER_330;

The SDHCI specification doesn't state exactly what level
SDHCI_POWER_330 corresponds to. It's 3.3V typically.

I don't have any strong opinion about this change, although I am a
little bit puzzled over why this solves the problem for you.

Unless the host (sdhci) announces that it supports MMC_VDD_34_35 or
MMC_VDD_35_36 through its mmc->ocr_avail mask, the mmc core shouldn't
try to use it. Can you perhaps check what value the mmc->ocr_avail
gets assigned to in sdhci_setup_host() for your mmc host?

>                         break;
>                 default:
> --
> 2.17.1
>

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] mmc: sdhci: Map more voltage level to SDHCI_POWER_330
  2021-09-30 11:00 ` Ulf Hansson
@ 2021-10-03 13:58   ` Shawn Guo
  2021-10-03 15:47     ` Adrian Hunter
  2021-10-06 15:17     ` Ulf Hansson
  0 siblings, 2 replies; 6+ messages in thread
From: Shawn Guo @ 2021-10-03 13:58 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Adrian Hunter, Bjorn Andersson, linux-mmc, linux-arm-msm,
	Linux Kernel Mailing List

On Thu, Sep 30, 2021 at 01:00:03PM +0200, Ulf Hansson wrote:
> On Sun, 26 Sept 2021 at 15:28, Shawn Guo <shawn.guo@linaro.org> wrote:
> >
> > On Thundercomm TurboX CM2290, the eMMC OCR reports vdd = 23 (3.5 ~ 3.6 V),
> > which is being treated as an invalid value by sdhci_set_power_noreg().
> > And thus eMMC is totally broken on the platform.
> >
> > [    1.436599] ------------[ cut here ]------------
> > [    1.436606] mmc0: Invalid vdd 0x17
> > [    1.436640] WARNING: CPU: 2 PID: 69 at drivers/mmc/host/sdhci.c:2048 sdhci_set_power_noreg+0x168/0x2b4
> > [    1.436655] Modules linked in:
> > [    1.436662] CPU: 2 PID: 69 Comm: kworker/u8:1 Tainted: G        W         5.15.0-rc1+ #137
> > [    1.436669] Hardware name: Thundercomm TurboX CM2290 (DT)
> > [    1.436674] Workqueue: events_unbound async_run_entry_fn
> > [    1.436685] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > [    1.436692] pc : sdhci_set_power_noreg+0x168/0x2b4
> > [    1.436698] lr : sdhci_set_power_noreg+0x168/0x2b4
> > [    1.436703] sp : ffff800010803a60
> > [    1.436705] x29: ffff800010803a60 x28: ffff6a9102465f00 x27: ffff6a9101720a70
> > [    1.436715] x26: ffff6a91014de1c0 x25: ffff6a91014de010 x24: ffff6a91016af280
> > [    1.436724] x23: ffffaf7b1b276640 x22: 0000000000000000 x21: ffff6a9101720000
> > [    1.436733] x20: ffff6a9101720370 x19: ffff6a9101720580 x18: 0000000000000020
> > [    1.436743] x17: 0000000000000000 x16: 0000000000000004 x15: ffffffffffffffff
> > [    1.436751] x14: 0000000000000000 x13: 00000000fffffffd x12: ffffaf7b1b84b0bc
> > [    1.436760] x11: ffffaf7b1b720d10 x10: 000000000000000a x9 : ffff800010803a60
> > [    1.436769] x8 : 000000000000000a x7 : 000000000000000f x6 : 00000000fffff159
> > [    1.436778] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
> > [    1.436787] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff6a9101718d80
> > [    1.436797] Call trace:
> > [    1.436800]  sdhci_set_power_noreg+0x168/0x2b4
> > [    1.436805]  sdhci_set_ios+0xa0/0x7fc
> > [    1.436811]  mmc_power_up.part.0+0xc4/0x164
> > [    1.436818]  mmc_start_host+0xa0/0xb0
> > [    1.436824]  mmc_add_host+0x60/0x90
> > [    1.436830]  __sdhci_add_host+0x174/0x330
> > [    1.436836]  sdhci_msm_probe+0x7c0/0x920
> > [    1.436842]  platform_probe+0x68/0xe0
> > [    1.436850]  really_probe.part.0+0x9c/0x31c
> > [    1.436857]  __driver_probe_device+0x98/0x144
> > [    1.436863]  driver_probe_device+0xc8/0x15c
> > [    1.436869]  __device_attach_driver+0xb4/0x120
> > [    1.436875]  bus_for_each_drv+0x78/0xd0
> > [    1.436881]  __device_attach_async_helper+0xac/0xd0
> > [    1.436888]  async_run_entry_fn+0x34/0x110
> > [    1.436895]  process_one_work+0x1d0/0x354
> > [    1.436903]  worker_thread+0x13c/0x470
> > [    1.436910]  kthread+0x150/0x160
> > [    1.436915]  ret_from_fork+0x10/0x20
> > [    1.436923] ---[ end trace fcfac44cb045c3a8 ]---
> >
> > Fix the issue by mapping MMC_VDD_35_36 (and MMC_VDD_34_35) to
> > SDHCI_POWER_330 as well.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> > I'm not sure if this is the right solution, as I do not have SDHCI
> > specification.  Hence it's a RFC.
> >
> >  drivers/mmc/host/sdhci.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 8eefa7d5fe85..2427481535a3 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -2042,6 +2042,8 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
> >                         break;
> >                 case MMC_VDD_32_33:
> >                 case MMC_VDD_33_34:
> > +               case MMC_VDD_34_35:
> > +               case MMC_VDD_35_36:
> >                         pwr = SDHCI_POWER_330;
> 
> The SDHCI specification doesn't state exactly what level
> SDHCI_POWER_330 corresponds to. It's 3.3V typically.
> 
> I don't have any strong opinion about this change, although I am a
> little bit puzzled over why this solves the problem for you.
> 
> Unless the host (sdhci) announces that it supports MMC_VDD_34_35 or
> MMC_VDD_35_36 through its mmc->ocr_avail mask, the mmc core shouldn't
> try to use it. Can you perhaps check what value the mmc->ocr_avail
> gets assigned to in sdhci_setup_host() for your mmc host?

Hi Ulf,

Thanks for the comment!

ocr_avail is 0xfff800, which is a result of mmc_regulator_get_ocrmask()
call.  On this platform, the vmmc has a 3.6V max voltage.  I can enforce
`regulator-max-microvolt` to be 3.3V to fix the problem, but I'm not
sure it's more correct than this RFC change.

Shawn 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] mmc: sdhci: Map more voltage level to SDHCI_POWER_330
  2021-10-03 13:58   ` Shawn Guo
@ 2021-10-03 15:47     ` Adrian Hunter
  2021-10-04  2:33       ` Shawn Guo
  2021-10-06 15:17     ` Ulf Hansson
  1 sibling, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2021-10-03 15:47 UTC (permalink / raw)
  To: Shawn Guo, Ulf Hansson
  Cc: Bjorn Andersson, linux-mmc, linux-arm-msm, Linux Kernel Mailing List

On 03/10/2021 16:58, Shawn Guo wrote:
> On Thu, Sep 30, 2021 at 01:00:03PM +0200, Ulf Hansson wrote:
>> On Sun, 26 Sept 2021 at 15:28, Shawn Guo <shawn.guo@linaro.org> wrote:
>>>
>>> On Thundercomm TurboX CM2290, the eMMC OCR reports vdd = 23 (3.5 ~ 3.6 V),
>>> which is being treated as an invalid value by sdhci_set_power_noreg().
>>> And thus eMMC is totally broken on the platform.
>>>
>>> [    1.436599] ------------[ cut here ]------------
>>> [    1.436606] mmc0: Invalid vdd 0x17
>>> [    1.436640] WARNING: CPU: 2 PID: 69 at drivers/mmc/host/sdhci.c:2048 sdhci_set_power_noreg+0x168/0x2b4
>>> [    1.436655] Modules linked in:
>>> [    1.436662] CPU: 2 PID: 69 Comm: kworker/u8:1 Tainted: G        W         5.15.0-rc1+ #137
>>> [    1.436669] Hardware name: Thundercomm TurboX CM2290 (DT)
>>> [    1.436674] Workqueue: events_unbound async_run_entry_fn
>>> [    1.436685] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>>> [    1.436692] pc : sdhci_set_power_noreg+0x168/0x2b4
>>> [    1.436698] lr : sdhci_set_power_noreg+0x168/0x2b4
>>> [    1.436703] sp : ffff800010803a60
>>> [    1.436705] x29: ffff800010803a60 x28: ffff6a9102465f00 x27: ffff6a9101720a70
>>> [    1.436715] x26: ffff6a91014de1c0 x25: ffff6a91014de010 x24: ffff6a91016af280
>>> [    1.436724] x23: ffffaf7b1b276640 x22: 0000000000000000 x21: ffff6a9101720000
>>> [    1.436733] x20: ffff6a9101720370 x19: ffff6a9101720580 x18: 0000000000000020
>>> [    1.436743] x17: 0000000000000000 x16: 0000000000000004 x15: ffffffffffffffff
>>> [    1.436751] x14: 0000000000000000 x13: 00000000fffffffd x12: ffffaf7b1b84b0bc
>>> [    1.436760] x11: ffffaf7b1b720d10 x10: 000000000000000a x9 : ffff800010803a60
>>> [    1.436769] x8 : 000000000000000a x7 : 000000000000000f x6 : 00000000fffff159
>>> [    1.436778] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
>>> [    1.436787] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff6a9101718d80
>>> [    1.436797] Call trace:
>>> [    1.436800]  sdhci_set_power_noreg+0x168/0x2b4
>>> [    1.436805]  sdhci_set_ios+0xa0/0x7fc
>>> [    1.436811]  mmc_power_up.part.0+0xc4/0x164
>>> [    1.436818]  mmc_start_host+0xa0/0xb0
>>> [    1.436824]  mmc_add_host+0x60/0x90
>>> [    1.436830]  __sdhci_add_host+0x174/0x330
>>> [    1.436836]  sdhci_msm_probe+0x7c0/0x920
>>> [    1.436842]  platform_probe+0x68/0xe0
>>> [    1.436850]  really_probe.part.0+0x9c/0x31c
>>> [    1.436857]  __driver_probe_device+0x98/0x144
>>> [    1.436863]  driver_probe_device+0xc8/0x15c
>>> [    1.436869]  __device_attach_driver+0xb4/0x120
>>> [    1.436875]  bus_for_each_drv+0x78/0xd0
>>> [    1.436881]  __device_attach_async_helper+0xac/0xd0
>>> [    1.436888]  async_run_entry_fn+0x34/0x110
>>> [    1.436895]  process_one_work+0x1d0/0x354
>>> [    1.436903]  worker_thread+0x13c/0x470
>>> [    1.436910]  kthread+0x150/0x160
>>> [    1.436915]  ret_from_fork+0x10/0x20
>>> [    1.436923] ---[ end trace fcfac44cb045c3a8 ]---
>>>
>>> Fix the issue by mapping MMC_VDD_35_36 (and MMC_VDD_34_35) to
>>> SDHCI_POWER_330 as well.
>>>
>>> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>>> ---
>>> I'm not sure if this is the right solution, as I do not have SDHCI
>>> specification.  Hence it's a RFC.
>>>
>>>  drivers/mmc/host/sdhci.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 8eefa7d5fe85..2427481535a3 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -2042,6 +2042,8 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
>>>                         break;
>>>                 case MMC_VDD_32_33:
>>>                 case MMC_VDD_33_34:
>>> +               case MMC_VDD_34_35:
>>> +               case MMC_VDD_35_36:
>>>                         pwr = SDHCI_POWER_330;
>>
>> The SDHCI specification doesn't state exactly what level
>> SDHCI_POWER_330 corresponds to. It's 3.3V typically.
>>
>> I don't have any strong opinion about this change, although I am a
>> little bit puzzled over why this solves the problem for you.
>>
>> Unless the host (sdhci) announces that it supports MMC_VDD_34_35 or
>> MMC_VDD_35_36 through its mmc->ocr_avail mask, the mmc core shouldn't
>> try to use it. Can you perhaps check what value the mmc->ocr_avail
>> gets assigned to in sdhci_setup_host() for your mmc host?
> 
> Hi Ulf,
> 
> Thanks for the comment!
> 
> ocr_avail is 0xfff800, which is a result of mmc_regulator_get_ocrmask()
> call.  On this platform, the vmmc has a 3.6V max voltage.  I can enforce
> `regulator-max-microvolt` to be 3.3V to fix the problem, but I'm not
> sure it's more correct than this RFC change.

The host controller lines are not necessarily connected directly to the
card, and the 3.3V selection is not necessarily actually 3.3V either.
So I have no problem with the change, but the question of whether it is
right for you really depends on your hardware.  For the patch, I would
suggest adding a comment in the code, that the driver that allows
3.4V-3.6V is assumed to know that the hardware supports it.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] mmc: sdhci: Map more voltage level to SDHCI_POWER_330
  2021-10-03 15:47     ` Adrian Hunter
@ 2021-10-04  2:33       ` Shawn Guo
  0 siblings, 0 replies; 6+ messages in thread
From: Shawn Guo @ 2021-10-04  2:33 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Ulf Hansson, Bjorn Andersson, linux-mmc, linux-arm-msm,
	Linux Kernel Mailing List

On Sun, Oct 03, 2021 at 06:47:55PM +0300, Adrian Hunter wrote:
> On 03/10/2021 16:58, Shawn Guo wrote:
> > On Thu, Sep 30, 2021 at 01:00:03PM +0200, Ulf Hansson wrote:
> >> On Sun, 26 Sept 2021 at 15:28, Shawn Guo <shawn.guo@linaro.org> wrote:
> >>>
> >>> On Thundercomm TurboX CM2290, the eMMC OCR reports vdd = 23 (3.5 ~ 3.6 V),
> >>> which is being treated as an invalid value by sdhci_set_power_noreg().
> >>> And thus eMMC is totally broken on the platform.
> >>>
> >>> [    1.436599] ------------[ cut here ]------------
> >>> [    1.436606] mmc0: Invalid vdd 0x17
> >>> [    1.436640] WARNING: CPU: 2 PID: 69 at drivers/mmc/host/sdhci.c:2048 sdhci_set_power_noreg+0x168/0x2b4
> >>> [    1.436655] Modules linked in:
> >>> [    1.436662] CPU: 2 PID: 69 Comm: kworker/u8:1 Tainted: G        W         5.15.0-rc1+ #137
> >>> [    1.436669] Hardware name: Thundercomm TurboX CM2290 (DT)
> >>> [    1.436674] Workqueue: events_unbound async_run_entry_fn
> >>> [    1.436685] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> >>> [    1.436692] pc : sdhci_set_power_noreg+0x168/0x2b4
> >>> [    1.436698] lr : sdhci_set_power_noreg+0x168/0x2b4
> >>> [    1.436703] sp : ffff800010803a60
> >>> [    1.436705] x29: ffff800010803a60 x28: ffff6a9102465f00 x27: ffff6a9101720a70
> >>> [    1.436715] x26: ffff6a91014de1c0 x25: ffff6a91014de010 x24: ffff6a91016af280
> >>> [    1.436724] x23: ffffaf7b1b276640 x22: 0000000000000000 x21: ffff6a9101720000
> >>> [    1.436733] x20: ffff6a9101720370 x19: ffff6a9101720580 x18: 0000000000000020
> >>> [    1.436743] x17: 0000000000000000 x16: 0000000000000004 x15: ffffffffffffffff
> >>> [    1.436751] x14: 0000000000000000 x13: 00000000fffffffd x12: ffffaf7b1b84b0bc
> >>> [    1.436760] x11: ffffaf7b1b720d10 x10: 000000000000000a x9 : ffff800010803a60
> >>> [    1.436769] x8 : 000000000000000a x7 : 000000000000000f x6 : 00000000fffff159
> >>> [    1.436778] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
> >>> [    1.436787] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff6a9101718d80
> >>> [    1.436797] Call trace:
> >>> [    1.436800]  sdhci_set_power_noreg+0x168/0x2b4
> >>> [    1.436805]  sdhci_set_ios+0xa0/0x7fc
> >>> [    1.436811]  mmc_power_up.part.0+0xc4/0x164
> >>> [    1.436818]  mmc_start_host+0xa0/0xb0
> >>> [    1.436824]  mmc_add_host+0x60/0x90
> >>> [    1.436830]  __sdhci_add_host+0x174/0x330
> >>> [    1.436836]  sdhci_msm_probe+0x7c0/0x920
> >>> [    1.436842]  platform_probe+0x68/0xe0
> >>> [    1.436850]  really_probe.part.0+0x9c/0x31c
> >>> [    1.436857]  __driver_probe_device+0x98/0x144
> >>> [    1.436863]  driver_probe_device+0xc8/0x15c
> >>> [    1.436869]  __device_attach_driver+0xb4/0x120
> >>> [    1.436875]  bus_for_each_drv+0x78/0xd0
> >>> [    1.436881]  __device_attach_async_helper+0xac/0xd0
> >>> [    1.436888]  async_run_entry_fn+0x34/0x110
> >>> [    1.436895]  process_one_work+0x1d0/0x354
> >>> [    1.436903]  worker_thread+0x13c/0x470
> >>> [    1.436910]  kthread+0x150/0x160
> >>> [    1.436915]  ret_from_fork+0x10/0x20
> >>> [    1.436923] ---[ end trace fcfac44cb045c3a8 ]---
> >>>
> >>> Fix the issue by mapping MMC_VDD_35_36 (and MMC_VDD_34_35) to
> >>> SDHCI_POWER_330 as well.
> >>>
> >>> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> >>> ---
> >>> I'm not sure if this is the right solution, as I do not have SDHCI
> >>> specification.  Hence it's a RFC.
> >>>
> >>>  drivers/mmc/host/sdhci.c | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> >>> index 8eefa7d5fe85..2427481535a3 100644
> >>> --- a/drivers/mmc/host/sdhci.c
> >>> +++ b/drivers/mmc/host/sdhci.c
> >>> @@ -2042,6 +2042,8 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
> >>>                         break;
> >>>                 case MMC_VDD_32_33:
> >>>                 case MMC_VDD_33_34:
> >>> +               case MMC_VDD_34_35:
> >>> +               case MMC_VDD_35_36:
> >>>                         pwr = SDHCI_POWER_330;
> >>
> >> The SDHCI specification doesn't state exactly what level
> >> SDHCI_POWER_330 corresponds to. It's 3.3V typically.
> >>
> >> I don't have any strong opinion about this change, although I am a
> >> little bit puzzled over why this solves the problem for you.
> >>
> >> Unless the host (sdhci) announces that it supports MMC_VDD_34_35 or
> >> MMC_VDD_35_36 through its mmc->ocr_avail mask, the mmc core shouldn't
> >> try to use it. Can you perhaps check what value the mmc->ocr_avail
> >> gets assigned to in sdhci_setup_host() for your mmc host?
> > 
> > Hi Ulf,
> > 
> > Thanks for the comment!
> > 
> > ocr_avail is 0xfff800, which is a result of mmc_regulator_get_ocrmask()
> > call.  On this platform, the vmmc has a 3.6V max voltage.  I can enforce
> > `regulator-max-microvolt` to be 3.3V to fix the problem, but I'm not
> > sure it's more correct than this RFC change.
> 
> The host controller lines are not necessarily connected directly to the
> card, and the 3.3V selection is not necessarily actually 3.3V either.
> So I have no problem with the change, but the question of whether it is
> right for you really depends on your hardware.  For the patch, I would
> suggest adding a comment in the code, that the driver that allows
> 3.4V-3.6V is assumed to know that the hardware supports it.

Thanks for the suggestion, Adrian!  Will do.

Shawn

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH] mmc: sdhci: Map more voltage level to SDHCI_POWER_330
  2021-10-03 13:58   ` Shawn Guo
  2021-10-03 15:47     ` Adrian Hunter
@ 2021-10-06 15:17     ` Ulf Hansson
  1 sibling, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2021-10-06 15:17 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Adrian Hunter, Bjorn Andersson, linux-mmc, linux-arm-msm,
	Linux Kernel Mailing List

On Sun, 3 Oct 2021 at 15:58, Shawn Guo <shawn.guo@linaro.org> wrote:
>
> On Thu, Sep 30, 2021 at 01:00:03PM +0200, Ulf Hansson wrote:
> > On Sun, 26 Sept 2021 at 15:28, Shawn Guo <shawn.guo@linaro.org> wrote:
> > >
> > > On Thundercomm TurboX CM2290, the eMMC OCR reports vdd = 23 (3.5 ~ 3.6 V),
> > > which is being treated as an invalid value by sdhci_set_power_noreg().
> > > And thus eMMC is totally broken on the platform.
> > >
> > > [    1.436599] ------------[ cut here ]------------
> > > [    1.436606] mmc0: Invalid vdd 0x17
> > > [    1.436640] WARNING: CPU: 2 PID: 69 at drivers/mmc/host/sdhci.c:2048 sdhci_set_power_noreg+0x168/0x2b4
> > > [    1.436655] Modules linked in:
> > > [    1.436662] CPU: 2 PID: 69 Comm: kworker/u8:1 Tainted: G        W         5.15.0-rc1+ #137
> > > [    1.436669] Hardware name: Thundercomm TurboX CM2290 (DT)
> > > [    1.436674] Workqueue: events_unbound async_run_entry_fn
> > > [    1.436685] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > [    1.436692] pc : sdhci_set_power_noreg+0x168/0x2b4
> > > [    1.436698] lr : sdhci_set_power_noreg+0x168/0x2b4
> > > [    1.436703] sp : ffff800010803a60
> > > [    1.436705] x29: ffff800010803a60 x28: ffff6a9102465f00 x27: ffff6a9101720a70
> > > [    1.436715] x26: ffff6a91014de1c0 x25: ffff6a91014de010 x24: ffff6a91016af280
> > > [    1.436724] x23: ffffaf7b1b276640 x22: 0000000000000000 x21: ffff6a9101720000
> > > [    1.436733] x20: ffff6a9101720370 x19: ffff6a9101720580 x18: 0000000000000020
> > > [    1.436743] x17: 0000000000000000 x16: 0000000000000004 x15: ffffffffffffffff
> > > [    1.436751] x14: 0000000000000000 x13: 00000000fffffffd x12: ffffaf7b1b84b0bc
> > > [    1.436760] x11: ffffaf7b1b720d10 x10: 000000000000000a x9 : ffff800010803a60
> > > [    1.436769] x8 : 000000000000000a x7 : 000000000000000f x6 : 00000000fffff159
> > > [    1.436778] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 00000000ffffffff
> > > [    1.436787] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff6a9101718d80
> > > [    1.436797] Call trace:
> > > [    1.436800]  sdhci_set_power_noreg+0x168/0x2b4
> > > [    1.436805]  sdhci_set_ios+0xa0/0x7fc
> > > [    1.436811]  mmc_power_up.part.0+0xc4/0x164
> > > [    1.436818]  mmc_start_host+0xa0/0xb0
> > > [    1.436824]  mmc_add_host+0x60/0x90
> > > [    1.436830]  __sdhci_add_host+0x174/0x330
> > > [    1.436836]  sdhci_msm_probe+0x7c0/0x920
> > > [    1.436842]  platform_probe+0x68/0xe0
> > > [    1.436850]  really_probe.part.0+0x9c/0x31c
> > > [    1.436857]  __driver_probe_device+0x98/0x144
> > > [    1.436863]  driver_probe_device+0xc8/0x15c
> > > [    1.436869]  __device_attach_driver+0xb4/0x120
> > > [    1.436875]  bus_for_each_drv+0x78/0xd0
> > > [    1.436881]  __device_attach_async_helper+0xac/0xd0
> > > [    1.436888]  async_run_entry_fn+0x34/0x110
> > > [    1.436895]  process_one_work+0x1d0/0x354
> > > [    1.436903]  worker_thread+0x13c/0x470
> > > [    1.436910]  kthread+0x150/0x160
> > > [    1.436915]  ret_from_fork+0x10/0x20
> > > [    1.436923] ---[ end trace fcfac44cb045c3a8 ]---
> > >
> > > Fix the issue by mapping MMC_VDD_35_36 (and MMC_VDD_34_35) to
> > > SDHCI_POWER_330 as well.
> > >
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > ---
> > > I'm not sure if this is the right solution, as I do not have SDHCI
> > > specification.  Hence it's a RFC.
> > >
> > >  drivers/mmc/host/sdhci.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > > index 8eefa7d5fe85..2427481535a3 100644
> > > --- a/drivers/mmc/host/sdhci.c
> > > +++ b/drivers/mmc/host/sdhci.c
> > > @@ -2042,6 +2042,8 @@ void sdhci_set_power_noreg(struct sdhci_host *host, unsigned char mode,
> > >                         break;
> > >                 case MMC_VDD_32_33:
> > >                 case MMC_VDD_33_34:
> > > +               case MMC_VDD_34_35:
> > > +               case MMC_VDD_35_36:
> > >                         pwr = SDHCI_POWER_330;
> >
> > The SDHCI specification doesn't state exactly what level
> > SDHCI_POWER_330 corresponds to. It's 3.3V typically.
> >
> > I don't have any strong opinion about this change, although I am a
> > little bit puzzled over why this solves the problem for you.
> >
> > Unless the host (sdhci) announces that it supports MMC_VDD_34_35 or
> > MMC_VDD_35_36 through its mmc->ocr_avail mask, the mmc core shouldn't
> > try to use it. Can you perhaps check what value the mmc->ocr_avail
> > gets assigned to in sdhci_setup_host() for your mmc host?
>
> Hi Ulf,
>
> Thanks for the comment!
>
> ocr_avail is 0xfff800, which is a result of mmc_regulator_get_ocrmask()
> call.  On this platform, the vmmc has a 3.6V max voltage.  I can enforce
> `regulator-max-microvolt` to be 3.3V to fix the problem, but I'm not
> sure it's more correct than this RFC change.

I see, thanks for clarifying.

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-10-06 15:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-26 13:28 [RFC PATCH] mmc: sdhci: Map more voltage level to SDHCI_POWER_330 Shawn Guo
2021-09-30 11:00 ` Ulf Hansson
2021-10-03 13:58   ` Shawn Guo
2021-10-03 15:47     ` Adrian Hunter
2021-10-04  2:33       ` Shawn Guo
2021-10-06 15:17     ` Ulf Hansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.