2021-12-13 07:18:38

by Rong Chen

[permalink] [raw]
Subject: [PATCH] mmc: meson: initial ocr available by default value

The patch will add a value of ocr supported by the controller,
to specify some of voltage values are supported.

Ocr_avail should place an initial value to avoid uncertain
value for the platform that unsupport regulator such as S4.

Signed-off-by: Rong Chen <[email protected]>
---
drivers/mmc/host/meson-gx-mmc.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 8f36536cb1b6..c339e12d5516 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -4,6 +4,9 @@
*
* Copyright (c) 2016 BayLibre, SAS.
* Author: Kevin Hilman <[email protected]>
+ *
+ * Copyright (c) 2021 Amlogic, inc.
+ * Author: Rong Chen <[email protected]>
*/
#include <linux/kernel.h>
#include <linux/module.h>
@@ -130,6 +133,7 @@
#define SD_EMMC_DESC_CHAIN_MODE BIT(1)

#define MUX_CLK_NUM_PARENTS 2
+#define SD_EMMC_OCR_AVAIL 0x200080

struct meson_mmc_data {
unsigned int tx_delay_mask;
@@ -1150,6 +1154,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
host->dram_access_quirk = device_property_read_bool(&pdev->dev,
"amlogic,dram-access-quirk");

+ mmc->ocr_avail = SD_EMMC_OCR_AVAIL;
/* Get regulators and the supported OCR mask */
host->vqmmc_enabled = false;
ret = mmc_regulator_get_supply(mmc);

base-commit: 141edd9e99eb91393e8a4d97742bd98328bff724
--
2.25.1



2021-12-13 08:00:31

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH] mmc: meson: initial ocr available by default value

Hello,

On Mon, Dec 13, 2021 at 8:03 AM Rong Chen <[email protected]> wrote:
>
> The patch will add a value of ocr supported by the controller,
> to specify some of voltage values are supported.
Can you please share some details on this topic?

We already have many boards upstreamed which work fine without
manually setting ocr_avail in the driver.
In general there's two types of boards:
1) boards where the regulator can switch between different voltages.
One example is the Odroid-C2, see meson-gxbb-odroidc2.dts [0]
Here we specify:
vmmc-supply = <&tflash_vdd>;
vqmmc-supply = <&tf_io>;
&tflash_vdd is either turned off or at 3.3V when turned on.
&tf_io can be either turned off, 1.8V or 3.3V.

2) boards where the voltages are fixed.
One example is Radxa Zero, see meson-g12a-radxa-zero.dts [1]
For the SD card the board uses:
vmmc-supply = <&vddao_3v3>;
vqmmc-supply = <&vddao_3v3>;
(meaning fixed 3.3V always-on power source for both VMMC and VQMMC)
and for eMMC the board uses:
vmmc-supply = <&vcc_3v3>;
vqmmc-supply = <&vcc_1v8>;
(meaning: 3.3V always-on power source for vmmc and 1.8V always-on
power source for VQMMC)

With the information from the regulators the MMC core will
automatically detect ocr_avail in Linux.
Or in other words: your .dts may be missing the link between regulator
and MMC controller vmmc/vqmmc.
Can you please double-check this?


Best regards,
Martin


[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts?id=2585cf9dfaaddf00b069673f27bb3f8530e2039c#n354
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts?id=2585cf9dfaaddf00b069673f27bb3f8530e2039c#n333

2021-12-13 13:25:51

by Jerome Brunet

[permalink] [raw]
Subject: Re: [PATCH] mmc: meson: initial ocr available by default value


On Mon 13 Dec 2021 at 19:50, "[email protected]" <[email protected]> wrote:

> Hello,
> If 3.3V always-on power source for vmmc is supplied by hardware boards fixed, don't need regulator in software, so .dts unnecessary the link between regulator
> and MMC controller vmmc/vqmmc.

Controllable or not, your vmmc/vqmmc regulators should be described in
DT.

There are several examples of this in arch/arm64/boot/dts/amlogic/*.dts


>
> ------------------------------------------------------------------------------------------------------------------------------------------------------
> [email protected]
>
>
> From: Martin Blumenstingl
> Date: 2021-12-13 16:00
> To: Rong Chen
> CC: Ulf Hansson; Neil Armstrong; Kevin Hilman; Jerome Brunet; --to=1131046452; 45581586; linux-mmc; linux-arm-kernel; linux-amlogic; linux-kernel
> Subject: Re: [PATCH] mmc: meson: initial ocr available by default value
> [ EXTERNAL EMAIL ]
>
> Hello,
>
> On Mon, Dec 13, 2021 at 8:03 AM Rong Chen <[email protected]> wrote:
> >
> > The patch will add a value of ocr supported by the controller,
> > to specify some of voltage values are supported.
> Can you please share some details on this topic?
>
> We already have many boards upstreamed which work fine without
> manually setting ocr_avail in the driver.
> In general there's two types of boards:
> 1) boards where the regulator can switch between different voltages.
> One example is the Odroid-C2, see meson-gxbb-odroidc2.dts [0]
> Here we specify:
> vmmc-supply = <&tflash_vdd>;
> vqmmc-supply = <&tf_io>;
> &tflash_vdd is either turned off or at 3.3V when turned on.
> &tf_io can be either turned off, 1.8V or 3.3V.
>
> 2) boards where the voltages are fixed.
> One example is Radxa Zero, see meson-g12a-radxa-zero.dts [1]
> For the SD card the board uses:
> vmmc-supply = <&vddao_3v3>;
> vqmmc-supply = <&vddao_3v3>;
> (meaning fixed 3.3V always-on power source for both VMMC and VQMMC)
> and for eMMC the board uses:
> vmmc-supply = <&vcc_3v3>;
> vqmmc-supply = <&vcc_1v8>;
> (meaning: 3.3V always-on power source for vmmc and 1.8V always-on
> power source for VQMMC)
>
> With the information from the regulators the MMC core will
> automatically detect ocr_avail in Linux.
> Or in other words: your .dts may be missing the link between regulator
> and MMC controller vmmc/vqmmc.
> Can you please double-check this?
>
>
> Best regards,
> Martin
>
>
> [0]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts?id=2585cf9dfaaddf00b069673f27bb3f8530e2039c#n354
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-g12a-radxa-zero.dts?id=2585cf9dfaaddf00b069673f27bb3f8530e2039c#n333


2021-12-13 19:03:05

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH] mmc: meson: initial ocr available by default value

On Mon, Dec 13, 2021 at 2:25 PM Jerome Brunet <[email protected]> wrote:
>
>
> On Mon 13 Dec 2021 at 19:50, "[email protected]" <[email protected]> wrote:
>
> > Hello,
> > If 3.3V always-on power source for vmmc is supplied by hardware boards fixed, don't need regulator in software, so .dts unnecessary the link between regulator
> > and MMC controller vmmc/vqmmc.
>
> Controllable or not, your vmmc/vqmmc regulators should be described in
> DT.
I agree with Jerome. Here are the two reasons why I think that it's
best to describe these regulators in device-tree:
- device-tree is there to describe the hardware. It's especially
useful for non-discoverable information (USB and PCI IDs are
discoverable examples), for example: voltage supply of an MMC
controller, UART IRQ line, ...
- reviewing board.dts would be a lot harder if we rely on defaults
(defaults which cannot be described in a device-tree schema). For
meson-g12a-radxa-zero.dts this means we could omit the vmmc/vqmmc
regulators for the SD card slot but we need them for the eMMC (because
vqmmc is fixed at 1.8V).


Best regards,
Martin