2019-04-16 04:49:03

by NeilBrown

[permalink] [raw]
Subject: [PATCH 0/5] mtk-sd enhancement to support MT7621

The MT7621 MIPS-based SOC contains an sdhci unit that is
much the same as the units supported by mtk-sd.c.

These patches enhance the driver so that I can use it on my MT7621
board (gnubee.org).

Some thoughts:
- I wonder if voltage-ranges should be a standard option, processed
by mmc_of_parse(), rather than requiring mmc_of_parse_voltage()
to be called as well?

- the "compatible" name "ralink,mt7620-sdhci" doesn't fit the pattern
of other names. It is a name I found in openwrt - I was previously
using a driver from there. I have no objection to changing it,
but I have no way to determine what the "correct" name it.

- I have tested the card-detect logic but not the write-protect, as
that doesn't seem to be wired on this board.

- My SOC doesn't have software-controlled clocks (that I can find).
To get the clocks that the driver requires, I define a "fixed-clock"
with the appropriate frequency and use that for both "source" and
"hclk". Many these clocks should be optional - at least hclk ??

- I don't need to reconfigure the pins for "uhs" so I just use the
same pinctrl setting for "default" and for "state_uhs". Maybe
"state_uhs" should be optional?


For reference, excerpts from my dts file are below.

Thanks,
NeilBrown


mmc_clock: mmc_clock@0 {
#clock-cells = <0>;
compatible = "fixed-clock";
clock-frequency = <48000000>;
};

and

sdhci: sdhci@1E130000 {
status = "disabled";

compatible = "ralink,mt7620-sdhci";
reg = <0x1E130000 0x4000>;

bus-width = <4>;
max-frequency = <48000000>;
cap-sd-highspeed;
cap-mmc-highspeed;
voltage-ranges = <2800 3300>;

pinctrl-names = "default", "state_uhs";
pinctrl-0 = <&sdhci_pins>;
pinctrl-1 = <&sdhci_pins>;
clocks = <&mmc_clock &mmc_clock>;
clock-names = "source", "hclk";

interrupt-parent = <&gic>;
interrupts = <GIC_SHARED 20 IRQ_TYPE_LEVEL_HIGH>;
};

---

NeilBrown (5):
mmc: mtk-sd: support "voltage-ranges" setting.
mmc: mtk-sd: don't hard-code interrupt trigger type
mmc: mtk-sd: add support for config found in mt7620 family SOCs.
mmc: mtk-sd: enable internal card-detect logic.
mmc: mtk-sd: enable internal write-protect logic.


Documentation/devicetree/bindings/mmc/mtk-sd.txt | 7 +
drivers/mmc/host/mtk-sd.c | 105 +++++++++++++++++++++-
2 files changed, 104 insertions(+), 8 deletions(-)

--
Signature


2019-04-16 04:49:07

by NeilBrown

[permalink] [raw]
Subject: [PATCH 1/5] mmc: mtk-sd: support "voltage-ranges" setting.

If the mtk-sd silicon is used in a context where there is no explicit
regulator, it is not currently possible to specify the voltage
ranges. This is true for the MT7621 MIPS Soc.

So add a called to mmc_of_parse_voltage() so that voltage-ranges can
be given.

Signed-off-by: NeilBrown <[email protected]>
---
Documentation/devicetree/bindings/mmc/mtk-sd.txt | 6 ++++--
drivers/mmc/host/mtk-sd.c | 3 +++
2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
index f5bcda3980cc..ed61cd5a5b8f 100644
--- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
+++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
@@ -27,10 +27,12 @@ Required properties:
- pinctrl-names: should be "default", "state_uhs"
- pinctrl-0: should contain default/high speed pin ctrl
- pinctrl-1: should contain uhs mode pin ctrl
-- vmmc-supply: power to the Core
-- vqmmc-supply: power to the IO

Optional properties:
+- vmmc-supply: power to the Core
+- vqmmc-supply: power to the IO
+- voltage-ranges: if vmmc-supply not present, this can specify pairs
+ of millivolt numbers to describe available ranged.
- assigned-clocks: PLL of the source clock
- assigned-clock-parents: parent of source clock, used for HS400 mode to get 400Mhz source clock
- hs400-ds-delay: HS400 DS delay setting
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 0798f0ba6d34..4492a4465c0e 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -2125,6 +2125,9 @@ static int msdc_drv_probe(struct platform_device *pdev)
ret = mmc_of_parse(mmc);
if (ret)
goto host_free;
+ ret = mmc_of_parse_voltage(pdev->dev.of_node, &mmc->ocr_avail);
+ if (ret < 0)
+ goto host_free;

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
host->base = devm_ioremap_resource(&pdev->dev, res);


2019-04-16 04:49:19

by NeilBrown

[permalink] [raw]
Subject: [PATCH 2/5] mmc: mtk-sd: don't hard-code interrupt trigger type

When using devicetree for configuration, interrupt trigger type
should be described in the dts file, not hard-coded in the C code.

The mtk-sd silicon in the mt7621 soc uses an active-high interrupt
and so cannot be used with the current code.

So remove the trigger and leave it to be set from devicetree.

Signed-off-by: NeilBrown <[email protected]>
---
drivers/mmc/host/mtk-sd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 4492a4465c0e..14e048239143 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -2243,7 +2243,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
msdc_init_hw(host);

ret = devm_request_irq(&pdev->dev, host->irq, msdc_irq,
- IRQF_TRIGGER_LOW | IRQF_ONESHOT, pdev->name, host);
+ 0, pdev->name, host);
if (ret)
goto release;



2019-04-16 04:50:06

by NeilBrown

[permalink] [raw]
Subject: [PATCH 4/5] mmc: mtk-sd: enable internal card-detect logic.

The mtk-sd silicon has integrated card-detect logic that is
enabled, at least, on the MT7621 as used in the GNUBEE NAS.

If the sdhci isn't marked non-removable and doesn't have a
cd-gpio configured, assume the internal cd logic should be used.

Signed-off-by: NeilBrown <[email protected]>
---
drivers/mmc/host/mtk-sd.c | 58 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index d379f2ece305..341cf5f03429 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -300,6 +300,8 @@
#define CMD_TIMEOUT (HZ/10 * 5) /* 100ms x5 */
#define DAT_TIMEOUT (HZ * 5) /* 1000ms x5 */

+#define DEFAULT_DEBOUNCE (8) /* 8 cycles CD debounce */
+
#define PAD_DELAY_MAX 32 /* PAD delay cells */
/*--------------------------------------------------------------------------*/
/* Descriptor Structure */
@@ -430,6 +432,7 @@ struct msdc_host {
bool hs400_cmd_resp_sel_rising;
/* cmd response sample selection for HS400 */
bool hs400_mode; /* current eMMC will run at hs400 mode */
+ bool internal_cd; /* Use internal card-detect logic */
struct msdc_save_para save_para; /* used when gate HCLK */
struct msdc_tune_para def_tune_para; /* default tune setting */
struct msdc_tune_para saved_tune_para; /* tune result of CMD21/CMD19 */
@@ -1430,6 +1433,11 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
sdio_signal_irq(host->mmc);
}

+ if ((events & event_mask) & MSDC_INT_CDSC) {
+ mmc_detect_change(host->mmc, msecs_to_jiffies(20));
+ events &= ~MSDC_INT_CDSC;
+ }
+
if (!(events & (event_mask & ~MSDC_INT_SDIOIRQ)))
break;

@@ -1463,14 +1471,24 @@ static void msdc_init_hw(struct msdc_host *host)
/* Reset */
msdc_reset_hw(host);

- /* Disable card detection */
- sdr_clr_bits(host->base + MSDC_PS, MSDC_PS_CDEN);
-
/* Disable and clear all interrupts */
writel(0, host->base + MSDC_INTEN);
val = readl(host->base + MSDC_INT);
writel(val, host->base + MSDC_INT);

+ /* Configure card detection */
+ if (host->internal_cd) {
+ sdr_set_field(host->base + MSDC_PS, MSDC_PS_CDDEBOUNCE,
+ DEFAULT_DEBOUNCE);
+ sdr_set_bits(host->base + MSDC_PS, MSDC_PS_CDEN);
+ sdr_set_bits(host->base + MSDC_INTEN, MSDC_INTEN_CDSC);
+ sdr_set_bits(host->base + SDC_CFG, SDC_CFG_INSWKUP);
+ } else {
+ sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_INSWKUP);
+ sdr_clr_bits(host->base + MSDC_PS, MSDC_PS_CDEN);
+ sdr_clr_bits(host->base + MSDC_INTEN, MSDC_INTEN_CDSC);
+ }
+
if (host->top_base) {
writel(0, host->top_base + EMMC_TOP_CONTROL);
writel(0, host->top_base + EMMC_TOP_CMD);
@@ -1580,6 +1598,11 @@ static void msdc_init_hw(struct msdc_host *host)
static void msdc_deinit_hw(struct msdc_host *host)
{
u32 val;
+
+ /* Disabled card-detect */
+ sdr_clr_bits(host->base + MSDC_PS, MSDC_PS_CDEN);
+ sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_INSWKUP);
+
/* Disable and clear all interrupts */
writel(0, host->base + MSDC_INTEN);

@@ -2078,13 +2101,31 @@ static void msdc_ack_sdio_irq(struct mmc_host *mmc)
__msdc_enable_sdio_irq(mmc, 1);
}

+static int msdc_get_cd(struct mmc_host *mmc)
+{
+ struct msdc_host *host = mmc_priv(mmc);
+ int val;
+
+ if (mmc->caps & MMC_CAP_NONREMOVABLE)
+ return 1;
+
+ if (!host->internal_cd)
+ return mmc_gpio_get_cd(mmc);
+
+ val = readl(host->base + MSDC_PS) & MSDC_PS_CDSTS;
+ if (mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH)
+ return !!val;
+ else
+ return !val;
+}
+
static const struct mmc_host_ops mt_msdc_ops = {
.post_req = msdc_post_req,
.pre_req = msdc_pre_req,
.request = msdc_ops_request,
.set_ios = msdc_ops_set_ios,
.get_ro = mmc_gpio_get_ro,
- .get_cd = mmc_gpio_get_cd,
+ .get_cd = msdc_get_cd,
.enable_sdio_irq = msdc_enable_sdio_irq,
.ack_sdio_irq = msdc_ack_sdio_irq,
.start_signal_voltage_switch = msdc_ops_switch_volt,
@@ -2206,6 +2247,15 @@ static int msdc_drv_probe(struct platform_device *pdev)
goto host_free;
}

+ if (!(mmc->caps & MMC_CAP_NONREMOVABLE) &&
+ !mmc_can_gpio_cd(mmc)) {
+ /*
+ * Is removable but no GPIO declared, so
+ * use internal functionality.
+ */
+ host->internal_cd = true;
+ }
+
msdc_of_property_parse(pdev, host);

host->dev = &pdev->dev;


2019-04-16 04:50:19

by NeilBrown

[permalink] [raw]
Subject: [PATCH 5/5] mmc: mtk-sd: enable internal write-protect logic.

The mtk-sd silicon has integrated write-protect detection logic.

If the sdhci isn't marked no-write-protect and doesn't have a
ro-gpio configured, assume the internal wp logic should be used.

Signed-off-by: NeilBrown <[email protected]>
---
drivers/mmc/host/mtk-sd.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 341cf5f03429..d63d6b62f49a 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -433,6 +433,7 @@ struct msdc_host {
/* cmd response sample selection for HS400 */
bool hs400_mode; /* current eMMC will run at hs400 mode */
bool internal_cd; /* Use internal card-detect logic */
+ bool internal_ro; /* Use internal write-protect logic */
struct msdc_save_para save_para; /* used when gate HCLK */
struct msdc_tune_para def_tune_para; /* default tune setting */
struct msdc_tune_para saved_tune_para; /* tune result of CMD21/CMD19 */
@@ -2119,12 +2120,30 @@ static int msdc_get_cd(struct mmc_host *mmc)
return !val;
}

+static int msdc_get_ro(struct mmc_host *mmc)
+{
+ struct msdc_host *host = mmc_priv(mmc);
+ int val;
+
+ if (mmc->caps2 & MMC_CAP2_NO_WRITE_PROTECT)
+ return 0;
+
+ if (!host->internal_ro)
+ return mmc_gpio_get_ro(mmc);
+
+ val = readl(host->base + MSDC_PS) & MSDC_PS_WP;
+ if (mmc->caps2 & MMC_CAP2_RO_ACTIVE_HIGH)
+ return !!val;
+ else
+ return !val;
+}
+
static const struct mmc_host_ops mt_msdc_ops = {
.post_req = msdc_post_req,
.pre_req = msdc_pre_req,
.request = msdc_ops_request,
.set_ios = msdc_ops_set_ios,
- .get_ro = mmc_gpio_get_ro,
+ .get_ro = msdc_get_ro,
.get_cd = msdc_get_cd,
.enable_sdio_irq = msdc_enable_sdio_irq,
.ack_sdio_irq = msdc_ack_sdio_irq,
@@ -2256,6 +2275,15 @@ static int msdc_drv_probe(struct platform_device *pdev)
host->internal_cd = true;
}

+ if (!(mmc->caps2 & MMC_CAP2_NO_WRITE_PROTECT) &&
+ !mmc_can_gpio_ro(mmc)) {
+ /*
+ * Has write-protect but no GPIO declared, so
+ * use internal functionality.
+ */
+ host->internal_ro = true;
+ }
+
msdc_of_property_parse(pdev, host);

host->dev = &pdev->dev;


2019-04-16 04:50:44

by NeilBrown

[permalink] [raw]
Subject: [PATCH 3/5] mmc: mtk-sd: add support for config found in mt7620 family SOCs.

mt7620 family MIPS SOCs contain the mtk-sd silicon.
Add support for this.

Signed-off-by: NeilBrown <[email protected]>

# Conflicts:
# drivers/mmc/host/mtk-sd.c
---
Documentation/devicetree/bindings/mmc/mtk-sd.txt | 1 +
drivers/mmc/host/mtk-sd.c | 12 ++++++++++++
2 files changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
index ed61cd5a5b8f..b6daa9773b25 100644
--- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
+++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
@@ -15,6 +15,7 @@ Required properties:
"mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
"mediatek,mt7622-mmc": for MT7622 SoC
"mediatek,mt7623-mmc", "mediatek,mt2701-mmc": for MT7623 SoC
+ "ralink,mt7620-sdhci", for MT7621 SoC (and others)

- reg: physical base address of the controller and length
- interrupts: Should contain MSDC interrupt number
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 14e048239143..d379f2ece305 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -517,6 +517,17 @@ static const struct mtk_mmc_compatible mt8516_compat = {
.stop_clk_fix = true,
};

+static const struct mtk_mmc_compatible mt7620_compat = {
+ .clk_div_bits = 8,
+ .hs400_tune = false,
+ .pad_tune_reg = MSDC_PAD_TUNE,
+ .async_fifo = false,
+ .data_tune = false,
+ .busy_check = false,
+ .stop_clk_fix = false,
+ .enhance_rx = false,
+};
+
static const struct of_device_id msdc_of_ids[] = {
{ .compatible = "mediatek,mt8135-mmc", .data = &mt8135_compat},
{ .compatible = "mediatek,mt8173-mmc", .data = &mt8173_compat},
@@ -525,6 +536,7 @@ static const struct of_device_id msdc_of_ids[] = {
{ .compatible = "mediatek,mt2712-mmc", .data = &mt2712_compat},
{ .compatible = "mediatek,mt7622-mmc", .data = &mt7622_compat},
{ .compatible = "mediatek,mt8516-mmc", .data = &mt8516_compat},
+ { .compatible = "ralink,mt7620-sdhci", .data = &mt7620_compat},
{}
};
MODULE_DEVICE_TABLE(of, msdc_of_ids);


2019-04-16 08:12:50

by Chaotian Jing

[permalink] [raw]
Subject: Re: [PATCH 2/5] mmc: mtk-sd: don't hard-code interrupt trigger type

On Tue, 2019-04-16 at 14:47 +1000, NeilBrown wrote:
> When using devicetree for configuration, interrupt trigger type
> should be described in the dts file, not hard-coded in the C code.
>
> The mtk-sd silicon in the mt7621 soc uses an active-high interrupt
> and so cannot be used with the current code.
>
> So remove the trigger and leave it to be set from devicetree.
>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> drivers/mmc/host/mtk-sd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 4492a4465c0e..14e048239143 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -2243,7 +2243,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
> msdc_init_hw(host);
>
> ret = devm_request_irq(&pdev->dev, host->irq, msdc_irq,
> - IRQF_TRIGGER_LOW | IRQF_ONESHOT, pdev->name, host);
> +
change it to IRQF_TRIGGER_NONE | IRQF_ONESHOT
> 0, pdev->name, host);
> if (ret)
> goto release;
>
>
>


2019-04-16 22:13:42

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH 2/5] mmc: mtk-sd: don't hard-code interrupt trigger type

On Tue, Apr 16 2019, Chaotian Jing wrote:

> On Tue, 2019-04-16 at 14:47 +1000, NeilBrown wrote:
>> When using devicetree for configuration, interrupt trigger type
>> should be described in the dts file, not hard-coded in the C code.
>>
>> The mtk-sd silicon in the mt7621 soc uses an active-high interrupt
>> and so cannot be used with the current code.
>>
>> So remove the trigger and leave it to be set from devicetree.
>>
>> Signed-off-by: NeilBrown <[email protected]>
>> ---
>> drivers/mmc/host/mtk-sd.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
>> index 4492a4465c0e..14e048239143 100644
>> --- a/drivers/mmc/host/mtk-sd.c
>> +++ b/drivers/mmc/host/mtk-sd.c
>> @@ -2243,7 +2243,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
>> msdc_init_hw(host);
>>
>> ret = devm_request_irq(&pdev->dev, host->irq, msdc_irq,
>> - IRQF_TRIGGER_LOW | IRQF_ONESHOT, pdev->name, host);
>> +
> change it to IRQF_TRIGGER_NONE | IRQF_ONESHOT

Why do we need IRQF_ONESHOT. That is for threaded interrupted
handlers...
msdc_irq() clears the interrupts, so ONESHOT isn't needed.

???

NeilBrown


>> 0, pdev->name, host);
>> if (ret)
>> goto release;
>>
>>
>>


Attachments:
signature.asc (847.00 B)

2019-04-18 06:39:25

by Chaotian Jing

[permalink] [raw]
Subject: Re: [PATCH 2/5] mmc: mtk-sd: don't hard-code interrupt trigger type

On Wed, 2019-04-17 at 08:12 +1000, NeilBrown wrote:
> On Tue, Apr 16 2019, Chaotian Jing wrote:
>
> > On Tue, 2019-04-16 at 14:47 +1000, NeilBrown wrote:
> >> When using devicetree for configuration, interrupt trigger type
> >> should be described in the dts file, not hard-coded in the C code.
> >>
> >> The mtk-sd silicon in the mt7621 soc uses an active-high interrupt
> >> and so cannot be used with the current code.
> >>
> >> So remove the trigger and leave it to be set from devicetree.
> >>
> >> Signed-off-by: NeilBrown <[email protected]>
> >> ---
> >> drivers/mmc/host/mtk-sd.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> >> index 4492a4465c0e..14e048239143 100644
> >> --- a/drivers/mmc/host/mtk-sd.c
> >> +++ b/drivers/mmc/host/mtk-sd.c
> >> @@ -2243,7 +2243,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
> >> msdc_init_hw(host);
> >>
> >> ret = devm_request_irq(&pdev->dev, host->irq, msdc_irq,
> >> - IRQF_TRIGGER_LOW | IRQF_ONESHOT, pdev->name, host);
> >> +
> > change it to IRQF_TRIGGER_NONE | IRQF_ONESHOT
>
> Why do we need IRQF_ONESHOT. That is for threaded interrupted
> handlers...
> msdc_irq() clears the interrupts, so ONESHOT isn't needed.
>
> ???
>
> NeilBrown
>
>
I just want to change it to IRQF_TRIGGER_NONE, Since IRQF_TRIGGER_NONE
is defined as 0, it's ok to use 0 instead of it.
> >> 0, pdev->name, host);
> >> if (ret)
> >> goto release;
> >>
> >>
> >>


2019-04-18 06:47:50

by Chaotian Jing

[permalink] [raw]
Subject: Re: [PATCH 4/5] mmc: mtk-sd: enable internal card-detect logic.

On Tue, 2019-04-16 at 14:47 +1000, NeilBrown wrote:
> The mtk-sd silicon has integrated card-detect logic that is
> enabled, at least, on the MT7621 as used in the GNUBEE NAS.
>
> If the sdhci isn't marked non-removable and doesn't have a
> cd-gpio configured, assume the internal cd logic should be used.
>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> drivers/mmc/host/mtk-sd.c | 58 ++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 54 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index d379f2ece305..341cf5f03429 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -300,6 +300,8 @@
> #define CMD_TIMEOUT (HZ/10 * 5) /* 100ms x5 */
> #define DAT_TIMEOUT (HZ * 5) /* 1000ms x5 */
>
> +#define DEFAULT_DEBOUNCE (8) /* 8 cycles CD debounce */
> +
> #define PAD_DELAY_MAX 32 /* PAD delay cells */
> /*--------------------------------------------------------------------------*/
> /* Descriptor Structure */
> @@ -430,6 +432,7 @@ struct msdc_host {
> bool hs400_cmd_resp_sel_rising;
> /* cmd response sample selection for HS400 */
> bool hs400_mode; /* current eMMC will run at hs400 mode */
> + bool internal_cd; /* Use internal card-detect logic */
> struct msdc_save_para save_para; /* used when gate HCLK */
> struct msdc_tune_para def_tune_para; /* default tune setting */
> struct msdc_tune_para saved_tune_para; /* tune result of CMD21/CMD19 */
> @@ -1430,6 +1433,11 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
> sdio_signal_irq(host->mmc);
> }
>
> + if ((events & event_mask) & MSDC_INT_CDSC) {
> + mmc_detect_change(host->mmc, msecs_to_jiffies(20));
> + events &= ~MSDC_INT_CDSC;
> + }
> +
> if (!(events & (event_mask & ~MSDC_INT_SDIOIRQ)))
> break;
>
> @@ -1463,14 +1471,24 @@ static void msdc_init_hw(struct msdc_host *host)
> /* Reset */
> msdc_reset_hw(host);
>
> - /* Disable card detection */
> - sdr_clr_bits(host->base + MSDC_PS, MSDC_PS_CDEN);
> -
> /* Disable and clear all interrupts */
> writel(0, host->base + MSDC_INTEN);
> val = readl(host->base + MSDC_INT);
> writel(val, host->base + MSDC_INT);
>
> + /* Configure card detection */
> + if (host->internal_cd) {
> + sdr_set_field(host->base + MSDC_PS, MSDC_PS_CDDEBOUNCE,
> + DEFAULT_DEBOUNCE);
> + sdr_set_bits(host->base + MSDC_PS, MSDC_PS_CDEN);
> + sdr_set_bits(host->base + MSDC_INTEN, MSDC_INTEN_CDSC);
> + sdr_set_bits(host->base + SDC_CFG, SDC_CFG_INSWKUP);
> + } else {
> + sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_INSWKUP);
> + sdr_clr_bits(host->base + MSDC_PS, MSDC_PS_CDEN);
> + sdr_clr_bits(host->base + MSDC_INTEN, MSDC_INTEN_CDSC);
> + }
> +
> if (host->top_base) {
> writel(0, host->top_base + EMMC_TOP_CONTROL);
> writel(0, host->top_base + EMMC_TOP_CMD);
> @@ -1580,6 +1598,11 @@ static void msdc_init_hw(struct msdc_host *host)
> static void msdc_deinit_hw(struct msdc_host *host)
> {
> u32 val;
> +
> + /* Disabled card-detect */
> + sdr_clr_bits(host->base + MSDC_PS, MSDC_PS_CDEN);
> + sdr_clr_bits(host->base + SDC_CFG, SDC_CFG_INSWKUP);
> +
> /* Disable and clear all interrupts */
> writel(0, host->base + MSDC_INTEN);
>
> @@ -2078,13 +2101,31 @@ static void msdc_ack_sdio_irq(struct mmc_host *mmc)
> __msdc_enable_sdio_irq(mmc, 1);
> }
>
> +static int msdc_get_cd(struct mmc_host *mmc)
> +{
> + struct msdc_host *host = mmc_priv(mmc);
> + int val;
> +
> + if (mmc->caps & MMC_CAP_NONREMOVABLE)
> + return 1;
> +
> + if (!host->internal_cd)
> + return mmc_gpio_get_cd(mmc);
> +
> + val = readl(host->base + MSDC_PS) & MSDC_PS_CDSTS;
> + if (mmc->caps2 & MMC_CAP2_CD_ACTIVE_HIGH)
> + return !!val;
> + else
> + return !val;
> +}
> +
> static const struct mmc_host_ops mt_msdc_ops = {
> .post_req = msdc_post_req,
> .pre_req = msdc_pre_req,
> .request = msdc_ops_request,
> .set_ios = msdc_ops_set_ios,
> .get_ro = mmc_gpio_get_ro,
> - .get_cd = mmc_gpio_get_cd,
> + .get_cd = msdc_get_cd,
> .enable_sdio_irq = msdc_enable_sdio_irq,
> .ack_sdio_irq = msdc_ack_sdio_irq,
> .start_signal_voltage_switch = msdc_ops_switch_volt,
> @@ -2206,6 +2247,15 @@ static int msdc_drv_probe(struct platform_device *pdev)
> goto host_free;
> }
>
> + if (!(mmc->caps & MMC_CAP_NONREMOVABLE) &&
> + !mmc_can_gpio_cd(mmc)) {

Should not do this assume!
better to add "mediatek,internal-cd" in your DTS, then no impact to
other Soc.
> + /*
> + * Is removable but no GPIO declared, so
> + * use internal functionality.
> + */
> + host->internal_cd = true;
> + }
> +
> msdc_of_property_parse(pdev, host);
>
> host->dev = &pdev->dev;
>
>


2019-05-04 08:23:07

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH 4/5] mmc: mtk-sd: enable internal card-detect logic.

On Thu, Apr 18 2019, Chaotian Jing wrote:

> On Tue, 2019-04-16 at 14:47 +1000, NeilBrown wrote:
>> The mtk-sd silicon has integrated card-detect logic that is
>> enabled, at least, on the MT7621 as used in the GNUBEE NAS.
>>
>> If the sdhci isn't marked non-removable and doesn't have a
>> cd-gpio configured, assume the internal cd logic should be used.
>>
>> Signed-off-by: NeilBrown <[email protected]>
...

>> @@ -2206,6 +2247,15 @@ static int msdc_drv_probe(struct platform_device *pdev)
>> goto host_free;
>> }
>>
>> + if (!(mmc->caps & MMC_CAP_NONREMOVABLE) &&
>> + !mmc_can_gpio_cd(mmc)) {
>
> Should not do this assume!
> better to add "mediatek,internal-cd" in your DTS, then no impact to
> other Soc.

(Sorry for the delay).

Documentation/devicetree/bindings/mmc/mmc.txt

says:
If no property below is supplied, host native card detect is used.

So this assumption is *exactly* what the documentation said we should
do.

How about I limit this assumption to mt7621 using a flag in the
mtk_mmc_compatible structure?

Thanks,
NeilBrown


Attachments:
signature.asc (847.00 B)