LinuxLists.cc - [PATCH] mmc: dw_mmc: add a quirk for the defferent bit of sdio interrupt

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: add a quirk for the defferent bit of sdio interrupt

Hi, Addy.

On 10/30/2014 11:21 AM, Addy Ke wrote:
> This patch add a quirk: DW_MCI_QUIRK_SDIO_INT_24BIT.
>
> The bit of sdio interrupt is 16 in designware implementation, but
> is 24 in RK3288. To support RK3288 mmc controller, we need add
> a quirk for it.
>
> Signed-off-by: Addy Ke <[email protected]>
> ---
> drivers/mmc/host/dw_mmc.c | 32 +++++++++++++++++++++++++++-----
> drivers/mmc/host/dw_mmc.h | 1 +
> include/linux/mmc/dw_mmc.h | 2 ++
> 3 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 69f0cc6..db29621 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -778,6 +778,12 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
> u32 div;
> u32 clk_en_a;
> u32 sdmmc_cmd_bits = SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT;
> + u32 sdio_int_bit;
> +
> + if (host->quirks & DW_MCI_QUIRK_SDIO_INT_24BIT)

I want to change the quirk naming.
If rockchip may use the other bit for sdio_int in future,
then you need to add the DW_MCI_QUIRK_SDIO_INT_xxBIT.?

How about DW_MCI_BROKEN_SDIO_INT_BIT?
And Could you consider to control with more general method than now?

Best Regards,
Jaehoon Chung

> + sdio_int_bit = SDMMC_INT_SDIO_24BIT(slot->id);
> + else
> + sdio_int_bit = SDMMC_INT_SDIO(slot->id);
>
> /* We must continue to set bit 28 in CMD until the change is complete */
> if (host->state == STATE_WAITING_CMD11_DONE)
> @@ -819,7 +825,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>
> /* enable clock; only low power if no SDIO */
> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
> + if (!(mci_readl(host, INTMASK) & sdio_int_bit))
> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
> mci_writel(host, CLKENA, clk_en_a);
>
> @@ -1167,6 +1173,12 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
> struct dw_mci_slot *slot = mmc_priv(mmc);
> struct dw_mci *host = slot->host;
> u32 int_mask;
> + u32 sdio_int_bit;
> +
> + if (host->quirks & DW_MCI_QUIRK_SDIO_INT_24BIT)
> + sdio_int_bit = SDMMC_INT_SDIO_24BIT(slot->id);
> + else
> + sdio_int_bit = SDMMC_INT_SDIO(slot->id);
>
> /* Enable/disable Slot Specific SDIO interrupt */
> int_mask = mci_readl(host, INTMASK);
> @@ -1180,10 +1192,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
> dw_mci_disable_low_power(slot);
>
> mci_writel(host, INTMASK,
> - (int_mask | SDMMC_INT_SDIO(slot->id)));
> + (int_mask | sdio_int_bit));
> } else {
> mci_writel(host, INTMASK,
> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
> + (int_mask & ~sdio_int_bit));
> }
> }
>
> @@ -2035,8 +2047,15 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> /* Handle SDIO Interrupts */
> for (i = 0; i < host->num_slots; i++) {
> struct dw_mci_slot *slot = host->slot[i];
> - if (pending & SDMMC_INT_SDIO(i)) {
> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
> + u32 sdio_int_bit;
> +
> + if (host->quirks & DW_MCI_QUIRK_SDIO_INT_24BIT)
> + sdio_int_bit = SDMMC_INT_SDIO_24BIT(i);
> + else
> + sdio_int_bit = SDMMC_INT_SDIO(i);
> +
> + if (pending & sdio_int_bit) {
> + mci_writel(host, RINTSTS, sdio_int_bit);
> mmc_signal_sdio_irq(slot->mmc);
> }
> }
> @@ -2452,6 +2471,9 @@ static struct dw_mci_of_quirks {
> }, {
> .quirk = "disable-wp",
> .id = DW_MCI_QUIRK_NO_WRITE_PROTECT,
> + }, {
> + .quirk = "sdio-int-24bit",
> + .id = DW_MCI_QUIRK_SDIO_INT_24BIT,
> },
> };
>
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 01b99e8..6a48015 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -92,6 +92,7 @@
> #define SDMMC_CTYPE_4BIT BIT(0)
> #define SDMMC_CTYPE_1BIT 0
> /* Interrupt status & mask register defines */
> +#define SDMMC_INT_SDIO_24BIT(n) BIT(24 + (n))
> #define SDMMC_INT_SDIO(n) BIT(16 + (n))
> #define SDMMC_INT_EBE BIT(15)
> #define SDMMC_INT_ACD BIT(14)
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 0013669..6d4669e 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -217,6 +217,8 @@ struct dw_mci_dma_ops {
> #define DW_MCI_QUIRK_BROKEN_CARD_DETECTION BIT(3)
> /* No write protect */
> #define DW_MCI_QUIRK_NO_WRITE_PROTECT BIT(4)
> +/* In RK3288, the bit of sdio interrupt is 24 */
> +#define DW_MCI_QUIRK_SDIO_INT_24BIT BIT(5)
>
> /* Slot level quirks */
> /* This slot has no write protect */
>

2014-10-30 04:41:10

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: add a quirk for the defferent bit of sdio interrupt

Addy,

On Wed, Oct 29, 2014 at 7:21 PM, Addy Ke <[email protected]> wrote:
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -778,6 +778,12 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
> u32 div;
> u32 clk_en_a;
> u32 sdmmc_cmd_bits = SDMMC_CMD_UPD_CLK | SDMMC_CMD_PRV_DAT_WAIT;
> + u32 sdio_int_bit;
> +
> + if (host->quirks & DW_MCI_QUIRK_SDIO_INT_24BIT)
> + sdio_int_bit = SDMMC_INT_SDIO_24BIT(slot->id);
> + else
> + sdio_int_bit = SDMMC_INT_SDIO(slot->id);

You can avoid a lot of "if" tests if you just add a new "sdio->id"
field to the slot and init it at probe time. It would be "8 +
slot->id" for rk3288 systems.

> @@ -2452,6 +2471,9 @@ static struct dw_mci_of_quirks {
> }, {
> .quirk = "disable-wp",
> .id = DW_MCI_QUIRK_NO_WRITE_PROTECT,
> + }, {
> + .quirk = "sdio-int-24bit",
> + .id = DW_MCI_QUIRK_SDIO_INT_24BIT,

This is adding a device tree binding. You need to document it.

...but you should probably avoid that anyway. All rk3288 chips need
this. You should just add do what you need to do automatically if
you're a rk3288. You've already got a specific compatible string for
rk3288.

-Doug

2014-10-30 04:49:51

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: add a quirk for the defferent bit of sdio interrupt

Addy,

On Wed, Oct 29, 2014 at 9:41 PM, Doug Anderson <[email protected]> wrote:
> You can avoid a lot of "if" tests if you just add a new "sdio->id"

Whoops, I mean "slot->sdio_id"

2014-10-30 06:54:26

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: add a quirk for the defferent bit of sdio interrupt

Hi, Doug,

On 2014/10/30 12:49, Doug Anderson wrote:
> Addy,
>
> On Wed, Oct 29, 2014 at 9:41 PM, Doug Anderson <[email protected]> wrote:
>> You can avoid a lot of "if" tests if you just add a new "sdio->id"
>
> Whoops, I mean "slot->sdio_id"
>

To use "slot->sdio_id", I think the subject must be changed.
So I will drop this patch ,and send new patch to use "slot->sdio_id" for this difference.

thank you!

>
>

2014-10-30 10:51:46

[permalink] [raw]

Subject: [PATCH] mmc: dw_mmc: add support for the other bit of sdio interrupt

The bit of sdio interrupt is 16 in designware implementation,
but it is 24 in RK3288. This patch add sdio_id0 for the number
of slot0 in the SDIO interrupt registers, which can be set in
platform DT table, such as:
- rockchip,sdio-interrupt-slot0 = <8>;

Signed-off-by: Addy Ke <[email protected]>
---
drivers/mmc/host/dw_mmc-rockchip.c | 13 +++++++++++++
drivers/mmc/host/dw_mmc.c | 12 +++++++-----
drivers/mmc/host/dw_mmc.h | 2 ++
include/linux/mmc/dw_mmc.h | 3 +++
4 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
index f0c2cb1..54655e7 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -65,6 +65,18 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
}
}

+static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
+{
+ struct device_node *np = host->dev->of_node;
+ int sdio_id0;
+
+ if (!of_property_read_u32(np, "rockchip,sdio-interrupt-slot0",
+ &sdio_id0))
+ host->sdio_id0 = sdio_id0;
+
+ return 0;
+}
+
static const struct dw_mci_drv_data rk2928_drv_data = {
.prepare_command = dw_mci_rockchip_prepare_command,
};
@@ -73,6 +85,7 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
.prepare_command = dw_mci_rockchip_prepare_command,
.set_ios = dw_mci_rk3288_set_ios,
.setup_clock = dw_mci_rk3288_setup_clock,
+ .parse_dt = dw_mci_rk3288_parse_dt,
};

static const struct of_device_id dw_mci_rockchip_match[] = {
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 69f0cc6..2ea7467 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -819,7 +819,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)

/* enable clock; only low power if no SDIO */
clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
- if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
+ if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
mci_writel(host, CLKENA, clk_en_a);

@@ -1180,10 +1180,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
dw_mci_disable_low_power(slot);

mci_writel(host, INTMASK,
- (int_mask | SDMMC_INT_SDIO(slot->id)));
+ (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
} else {
mci_writel(host, INTMASK,
- (int_mask & ~SDMMC_INT_SDIO(slot->id)));
+ (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
}
}

@@ -2035,8 +2035,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
/* Handle SDIO Interrupts */
for (i = 0; i < host->num_slots; i++) {
struct dw_mci_slot *slot = host->slot[i];
- if (pending & SDMMC_INT_SDIO(i)) {
- mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
+ if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
+ mci_writel(host, RINTSTS,
+ SDMMC_INT_SDIO(slot->sdio_id));
mmc_signal_sdio_irq(slot->mmc);
}
}
@@ -2206,6 +2207,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)

slot = mmc_priv(mmc);
slot->id = id;
+ slot->sdio_id = host->sdio_id0 + id;
slot->mmc = mmc;
slot->host = host;
host->slot[id] = slot;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 01b99e8..3e966a9 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
* with CONFIG_MMC_CLKGATE.
* @flags: Random state bits associated with the slot.
* @id: Number of this slot.
+ * @sdio_id: Number of this slot in the SDIO interrupt registers.
* @last_detect_state: Most recently observed card detect state.
*/
struct dw_mci_slot {
@@ -234,6 +235,7 @@ struct dw_mci_slot {
#define DW_MMC_CARD_PRESENT 0
#define DW_MMC_CARD_NEED_INIT 1
int id;
+ int sdio_id;
int last_detect_state;
};

diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 0013669..4c0d3f2 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -96,6 +96,7 @@ struct mmc_data;
* @quirks: Set of quirks that apply to specific versions of the IP.
* @irq_flags: The flags to be passed to request_irq.
* @irq: The irq value to be passed to request_irq.
+ * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
*
* Locking
* =======
@@ -193,6 +194,8 @@ struct dw_mci {
bool vqmmc_enabled;
unsigned long irq_flags; /* IRQ flags */
int irq;
+
+ int sdio_id0;
};

/* DMA ops for Internal/External DMAC interface */
--
1.8.3.2

2014-10-30 11:02:08

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: add support for the other bit of sdio interrupt

Hi, Addy.

This patch is conflicted..Could you rebase on latest Ulf's tree?

Best Regards,
Jaehoon Chung

On 10/30/2014 07:50 PM, Addy Ke wrote:
> The bit of sdio interrupt is 16 in designware implementation,
> but it is 24 in RK3288. This patch add sdio_id0 for the number
> of slot0 in the SDIO interrupt registers, which can be set in
> platform DT table, such as:
> - rockchip,sdio-interrupt-slot0 = <8>;
>
> Signed-off-by: Addy Ke <[email protected]>
> ---
> drivers/mmc/host/dw_mmc-rockchip.c | 13 +++++++++++++
> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
> drivers/mmc/host/dw_mmc.h | 2 ++
> include/linux/mmc/dw_mmc.h | 3 +++
> 4 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> index f0c2cb1..54655e7 100644
> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> @@ -65,6 +65,18 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> }
> }
>
> +static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
> +{
> + struct device_node *np = host->dev->of_node;
> + int sdio_id0;
> +
> + if (!of_property_read_u32(np, "rockchip,sdio-interrupt-slot0",
> + &sdio_id0))
> + host->sdio_id0 = sdio_id0;
> +
> + return 0;
> +}
> +
> static const struct dw_mci_drv_data rk2928_drv_data = {
> .prepare_command = dw_mci_rockchip_prepare_command,
> };
> @@ -73,6 +85,7 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
> .prepare_command = dw_mci_rockchip_prepare_command,
> .set_ios = dw_mci_rk3288_set_ios,
> .setup_clock = dw_mci_rk3288_setup_clock,
> + .parse_dt = dw_mci_rk3288_parse_dt,
> };
>
> static const struct of_device_id dw_mci_rockchip_match[] = {
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 69f0cc6..2ea7467 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -819,7 +819,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>
> /* enable clock; only low power if no SDIO */
> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
> + if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
> mci_writel(host, CLKENA, clk_en_a);
>
> @@ -1180,10 +1180,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
> dw_mci_disable_low_power(slot);
>
> mci_writel(host, INTMASK,
> - (int_mask | SDMMC_INT_SDIO(slot->id)));
> + (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
> } else {
> mci_writel(host, INTMASK,
> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
> + (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
> }
> }
>
> @@ -2035,8 +2035,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> /* Handle SDIO Interrupts */
> for (i = 0; i < host->num_slots; i++) {
> struct dw_mci_slot *slot = host->slot[i];
> - if (pending & SDMMC_INT_SDIO(i)) {
> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
> + if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
> + mci_writel(host, RINTSTS,
> + SDMMC_INT_SDIO(slot->sdio_id));
> mmc_signal_sdio_irq(slot->mmc);
> }
> }
> @@ -2206,6 +2207,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>
> slot = mmc_priv(mmc);
> slot->id = id;
> + slot->sdio_id = host->sdio_id0 + id;
> slot->mmc = mmc;
> slot->host = host;
> host->slot[id] = slot;
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 01b99e8..3e966a9 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
> * with CONFIG_MMC_CLKGATE.
> * @flags: Random state bits associated with the slot.
> * @id: Number of this slot.
> + * @sdio_id: Number of this slot in the SDIO interrupt registers.
> * @last_detect_state: Most recently observed card detect state.
> */
> struct dw_mci_slot {
> @@ -234,6 +235,7 @@ struct dw_mci_slot {
> #define DW_MMC_CARD_PRESENT 0
> #define DW_MMC_CARD_NEED_INIT 1
> int id;
> + int sdio_id;
> int last_detect_state;
> };
>
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 0013669..4c0d3f2 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -96,6 +96,7 @@ struct mmc_data;
> * @quirks: Set of quirks that apply to specific versions of the IP.
> * @irq_flags: The flags to be passed to request_irq.
> * @irq: The irq value to be passed to request_irq.
> + * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
> *
> * Locking
> * =======
> @@ -193,6 +194,8 @@ struct dw_mci {
> bool vqmmc_enabled;
> unsigned long irq_flags; /* IRQ flags */
> int irq;
> +
> + int sdio_id0;
> };
>
> /* DMA ops for Internal/External DMAC interface */
>

2014-10-30 11:11:38

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: add support for the other bit of sdio interrupt

On 30 October 2014 11:50, Addy Ke <[email protected]> wrote:
> The bit of sdio interrupt is 16 in designware implementation,
> but it is 24 in RK3288. This patch add sdio_id0 for the number
> of slot0 in the SDIO interrupt registers, which can be set in
> platform DT table, such as:
> - rockchip,sdio-interrupt-slot0 = <8>;

No, this shouldn't be information in DT.

Instead this can be kept in the driver, depending on what version of
the mmc controller that is being used. Right!?

Kind regards
Uffe

>
> Signed-off-by: Addy Ke <[email protected]>
> ---
> drivers/mmc/host/dw_mmc-rockchip.c | 13 +++++++++++++
> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
> drivers/mmc/host/dw_mmc.h | 2 ++
> include/linux/mmc/dw_mmc.h | 3 +++
> 4 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> index f0c2cb1..54655e7 100644
> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> @@ -65,6 +65,18 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> }
> }
>
> +static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
> +{
> + struct device_node *np = host->dev->of_node;
> + int sdio_id0;
> +
> + if (!of_property_read_u32(np, "rockchip,sdio-interrupt-slot0",
> + &sdio_id0))
> + host->sdio_id0 = sdio_id0;
> +
> + return 0;
> +}
> +
> static const struct dw_mci_drv_data rk2928_drv_data = {
> .prepare_command = dw_mci_rockchip_prepare_command,
> };
> @@ -73,6 +85,7 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
> .prepare_command = dw_mci_rockchip_prepare_command,
> .set_ios = dw_mci_rk3288_set_ios,
> .setup_clock = dw_mci_rk3288_setup_clock,
> + .parse_dt = dw_mci_rk3288_parse_dt,
> };
>
> static const struct of_device_id dw_mci_rockchip_match[] = {
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 69f0cc6..2ea7467 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -819,7 +819,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>
> /* enable clock; only low power if no SDIO */
> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
> + if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
> mci_writel(host, CLKENA, clk_en_a);
>
> @@ -1180,10 +1180,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
> dw_mci_disable_low_power(slot);
>
> mci_writel(host, INTMASK,
> - (int_mask | SDMMC_INT_SDIO(slot->id)));
> + (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
> } else {
> mci_writel(host, INTMASK,
> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
> + (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
> }
> }
>
> @@ -2035,8 +2035,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> /* Handle SDIO Interrupts */
> for (i = 0; i < host->num_slots; i++) {
> struct dw_mci_slot *slot = host->slot[i];
> - if (pending & SDMMC_INT_SDIO(i)) {
> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
> + if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
> + mci_writel(host, RINTSTS,
> + SDMMC_INT_SDIO(slot->sdio_id));
> mmc_signal_sdio_irq(slot->mmc);
> }
> }
> @@ -2206,6 +2207,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>
> slot = mmc_priv(mmc);
> slot->id = id;
> + slot->sdio_id = host->sdio_id0 + id;
> slot->mmc = mmc;
> slot->host = host;
> host->slot[id] = slot;
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 01b99e8..3e966a9 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
> * with CONFIG_MMC_CLKGATE.
> * @flags: Random state bits associated with the slot.
> * @id: Number of this slot.
> + * @sdio_id: Number of this slot in the SDIO interrupt registers.
> * @last_detect_state: Most recently observed card detect state.
> */
> struct dw_mci_slot {
> @@ -234,6 +235,7 @@ struct dw_mci_slot {
> #define DW_MMC_CARD_PRESENT 0
> #define DW_MMC_CARD_NEED_INIT 1
> int id;
> + int sdio_id;
> int last_detect_state;
> };
>
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 0013669..4c0d3f2 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -96,6 +96,7 @@ struct mmc_data;
> * @quirks: Set of quirks that apply to specific versions of the IP.
> * @irq_flags: The flags to be passed to request_irq.
> * @irq: The irq value to be passed to request_irq.
> + * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
> *
> * Locking
> * =======
> @@ -193,6 +194,8 @@ struct dw_mci {
> bool vqmmc_enabled;
> unsigned long irq_flags; /* IRQ flags */
> int irq;
> +
> + int sdio_id0;
> };
>
> /* DMA ops for Internal/External DMAC interface */
> --
> 1.8.3.2
>
>

2014-10-30 11:18:10

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: add support for the other bit of sdio interrupt

On 10/30/2014 08:11 PM, Ulf Hansson wrote:
> On 30 October 2014 11:50, Addy Ke <[email protected]> wrote:
>> The bit of sdio interrupt is 16 in designware implementation,
>> but it is 24 in RK3288. This patch add sdio_id0 for the number
>> of slot0 in the SDIO interrupt registers, which can be set in
>> platform DT table, such as:
>> - rockchip,sdio-interrupt-slot0 = <8>;
>
> No, this shouldn't be information in DT.
>
> Instead this can be kept in the driver, depending on what version of
> the mmc controller that is being used. Right!?

sdio-interrupt slot doesn't depend on IP version.
maybe it depends on rock-chip board.

Best Regards,
Jaehoon Chung

>
> Kind regards
> Uffe
>
>>
>> Signed-off-by: Addy Ke <[email protected]>
>> ---
>> drivers/mmc/host/dw_mmc-rockchip.c | 13 +++++++++++++
>> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
>> drivers/mmc/host/dw_mmc.h | 2 ++
>> include/linux/mmc/dw_mmc.h | 3 +++
>> 4 files changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
>> index f0c2cb1..54655e7 100644
>> --- a/drivers/mmc/host/dw_mmc-rockchip.c
>> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
>> @@ -65,6 +65,18 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>> }
>> }
>>
>> +static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
>> +{
>> + struct device_node *np = host->dev->of_node;
>> + int sdio_id0;
>> +
>> + if (!of_property_read_u32(np, "rockchip,sdio-interrupt-slot0",
>> + &sdio_id0))
>> + host->sdio_id0 = sdio_id0;
>> +
>> + return 0;
>> +}
>> +
>> static const struct dw_mci_drv_data rk2928_drv_data = {
>> .prepare_command = dw_mci_rockchip_prepare_command,
>> };
>> @@ -73,6 +85,7 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
>> .prepare_command = dw_mci_rockchip_prepare_command,
>> .set_ios = dw_mci_rk3288_set_ios,
>> .setup_clock = dw_mci_rk3288_setup_clock,
>> + .parse_dt = dw_mci_rk3288_parse_dt,
>> };
>>
>> static const struct of_device_id dw_mci_rockchip_match[] = {
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 69f0cc6..2ea7467 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -819,7 +819,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>
>> /* enable clock; only low power if no SDIO */
>> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
>> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
>> + if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
>> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
>> mci_writel(host, CLKENA, clk_en_a);
>>
>> @@ -1180,10 +1180,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>> dw_mci_disable_low_power(slot);
>>
>> mci_writel(host, INTMASK,
>> - (int_mask | SDMMC_INT_SDIO(slot->id)));
>> + (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
>> } else {
>> mci_writel(host, INTMASK,
>> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
>> + (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
>> }
>> }
>>
>> @@ -2035,8 +2035,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>> /* Handle SDIO Interrupts */
>> for (i = 0; i < host->num_slots; i++) {
>> struct dw_mci_slot *slot = host->slot[i];
>> - if (pending & SDMMC_INT_SDIO(i)) {
>> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
>> + if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
>> + mci_writel(host, RINTSTS,
>> + SDMMC_INT_SDIO(slot->sdio_id));
>> mmc_signal_sdio_irq(slot->mmc);
>> }
>> }
>> @@ -2206,6 +2207,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>>
>> slot = mmc_priv(mmc);
>> slot->id = id;
>> + slot->sdio_id = host->sdio_id0 + id;
>> slot->mmc = mmc;
>> slot->host = host;
>> host->slot[id] = slot;
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index 01b99e8..3e966a9 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
>> * with CONFIG_MMC_CLKGATE.
>> * @flags: Random state bits associated with the slot.
>> * @id: Number of this slot.
>> + * @sdio_id: Number of this slot in the SDIO interrupt registers.
>> * @last_detect_state: Most recently observed card detect state.
>> */
>> struct dw_mci_slot {
>> @@ -234,6 +235,7 @@ struct dw_mci_slot {
>> #define DW_MMC_CARD_PRESENT 0
>> #define DW_MMC_CARD_NEED_INIT 1
>> int id;
>> + int sdio_id;
>> int last_detect_state;
>> };
>>
>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>> index 0013669..4c0d3f2 100644
>> --- a/include/linux/mmc/dw_mmc.h
>> +++ b/include/linux/mmc/dw_mmc.h
>> @@ -96,6 +96,7 @@ struct mmc_data;
>> * @quirks: Set of quirks that apply to specific versions of the IP.
>> * @irq_flags: The flags to be passed to request_irq.
>> * @irq: The irq value to be passed to request_irq.
>> + * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
>> *
>> * Locking
>> * =======
>> @@ -193,6 +194,8 @@ struct dw_mci {
>> bool vqmmc_enabled;
>> unsigned long irq_flags; /* IRQ flags */
>> int irq;
>> +
>> + int sdio_id0;
>> };
>>
>> /* DMA ops for Internal/External DMAC interface */
>> --
>> 1.8.3.2
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2014-10-31 00:46:31

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: add support for the other bit of sdio interrupt

Hi, Jaehoon

On 2014/10/30 19:02, Jaehoon Chung wrote:
> Hi, Addy.
>
> This patch is conflicted..Could you rebase on latest Ulf's tree?
I have not found ulf's tree in git.kernel.org.
I can't 'git clone git://git.kernel.org/pub/scm/linux/kernel/git/ulf/xxx.git'.
So my patch is based on kernel-3.18.
I will send patch v2 for this.
Would you please give me a git url for it?
Thank you.

>
> Best Regards,
> Jaehoon Chung
>
> On 10/30/2014 07:50 PM, Addy Ke wrote:
>> The bit of sdio interrupt is 16 in designware implementation,
>> but it is 24 in RK3288. This patch add sdio_id0 for the number
>> of slot0 in the SDIO interrupt registers, which can be set in
>> platform DT table, such as:
>> - rockchip,sdio-interrupt-slot0 = <8>;
>>
>> Signed-off-by: Addy Ke <[email protected]>
>> ---
>> drivers/mmc/host/dw_mmc-rockchip.c | 13 +++++++++++++
>> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
>> drivers/mmc/host/dw_mmc.h | 2 ++
>> include/linux/mmc/dw_mmc.h | 3 +++
>> 4 files changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
>> index f0c2cb1..54655e7 100644
>> --- a/drivers/mmc/host/dw_mmc-rockchip.c
>> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
>> @@ -65,6 +65,18 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>> }
>> }
>>
>> +static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
>> +{
>> + struct device_node *np = host->dev->of_node;
>> + int sdio_id0;
>> +
>> + if (!of_property_read_u32(np, "rockchip,sdio-interrupt-slot0",
>> + &sdio_id0))
>> + host->sdio_id0 = sdio_id0;
>> +
>> + return 0;
>> +}
>> +
>> static const struct dw_mci_drv_data rk2928_drv_data = {
>> .prepare_command = dw_mci_rockchip_prepare_command,
>> };
>> @@ -73,6 +85,7 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
>> .prepare_command = dw_mci_rockchip_prepare_command,
>> .set_ios = dw_mci_rk3288_set_ios,
>> .setup_clock = dw_mci_rk3288_setup_clock,
>> + .parse_dt = dw_mci_rk3288_parse_dt,
>> };
>>
>> static const struct of_device_id dw_mci_rockchip_match[] = {
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 69f0cc6..2ea7467 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -819,7 +819,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>
>> /* enable clock; only low power if no SDIO */
>> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
>> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
>> + if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
>> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
>> mci_writel(host, CLKENA, clk_en_a);
>>
>> @@ -1180,10 +1180,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>> dw_mci_disable_low_power(slot);
>>
>> mci_writel(host, INTMASK,
>> - (int_mask | SDMMC_INT_SDIO(slot->id)));
>> + (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
>> } else {
>> mci_writel(host, INTMASK,
>> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
>> + (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
>> }
>> }
>>
>> @@ -2035,8 +2035,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>> /* Handle SDIO Interrupts */
>> for (i = 0; i < host->num_slots; i++) {
>> struct dw_mci_slot *slot = host->slot[i];
>> - if (pending & SDMMC_INT_SDIO(i)) {
>> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
>> + if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
>> + mci_writel(host, RINTSTS,
>> + SDMMC_INT_SDIO(slot->sdio_id));
>> mmc_signal_sdio_irq(slot->mmc);
>> }
>> }
>> @@ -2206,6 +2207,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>>
>> slot = mmc_priv(mmc);
>> slot->id = id;
>> + slot->sdio_id = host->sdio_id0 + id;
>> slot->mmc = mmc;
>> slot->host = host;
>> host->slot[id] = slot;
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index 01b99e8..3e966a9 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
>> * with CONFIG_MMC_CLKGATE.
>> * @flags: Random state bits associated with the slot.
>> * @id: Number of this slot.
>> + * @sdio_id: Number of this slot in the SDIO interrupt registers.
>> * @last_detect_state: Most recently observed card detect state.
>> */
>> struct dw_mci_slot {
>> @@ -234,6 +235,7 @@ struct dw_mci_slot {
>> #define DW_MMC_CARD_PRESENT 0
>> #define DW_MMC_CARD_NEED_INIT 1
>> int id;
>> + int sdio_id;
>> int last_detect_state;
>> };
>>
>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>> index 0013669..4c0d3f2 100644
>> --- a/include/linux/mmc/dw_mmc.h
>> +++ b/include/linux/mmc/dw_mmc.h
>> @@ -96,6 +96,7 @@ struct mmc_data;
>> * @quirks: Set of quirks that apply to specific versions of the IP.
>> * @irq_flags: The flags to be passed to request_irq.
>> * @irq: The irq value to be passed to request_irq.
>> + * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
>> *
>> * Locking
>> * =======
>> @@ -193,6 +194,8 @@ struct dw_mci {
>> bool vqmmc_enabled;
>> unsigned long irq_flags; /* IRQ flags */
>> int irq;
>> +
>> + int sdio_id0;
>> };
>>
>> /* DMA ops for Internal/External DMAC interface */
>>
>
>
>
>

2014-10-31 00:54:56

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: add support for the other bit of sdio interrupt

On 2014/10/30 19:17, Jaehoon Chung wrote:
> On 10/30/2014 08:11 PM, Ulf Hansson wrote:
>> On 30 October 2014 11:50, Addy Ke <[email protected]> wrote:
>>> The bit of sdio interrupt is 16 in designware implementation,
>>> but it is 24 in RK3288. This patch add sdio_id0 for the number
>>> of slot0 in the SDIO interrupt registers, which can be set in
>>> platform DT table, such as:
>>> - rockchip,sdio-interrupt-slot0 = <8>;
>>
>> No, this shouldn't be information in DT.
>>
>> Instead this can be kept in the driver, depending on what version of
>> the mmc controller that is being used. Right!?
>
> sdio-interrupt slot doesn't depend on IP version.
> maybe it depends on rock-chip board.

sure, it is denpends on rockchip soc, not IP version.
As far as I know, only our socs(such as RK3288) have this difference.
>
> Best Regards,
> Jaehoon Chung
>
>>
>> Kind regards
>> Uffe
>>
>>>
>>> Signed-off-by: Addy Ke <[email protected]>
>>> ---
>>> drivers/mmc/host/dw_mmc-rockchip.c | 13 +++++++++++++
>>> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
>>> drivers/mmc/host/dw_mmc.h | 2 ++
>>> include/linux/mmc/dw_mmc.h | 3 +++
>>> 4 files changed, 25 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
>>> index f0c2cb1..54655e7 100644
>>> --- a/drivers/mmc/host/dw_mmc-rockchip.c
>>> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
>>> @@ -65,6 +65,18 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>>> }
>>> }
>>>
>>> +static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
>>> +{
>>> + struct device_node *np = host->dev->of_node;
>>> + int sdio_id0;
>>> +
>>> + if (!of_property_read_u32(np, "rockchip,sdio-interrupt-slot0",
>>> + &sdio_id0))
>>> + host->sdio_id0 = sdio_id0;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static const struct dw_mci_drv_data rk2928_drv_data = {
>>> .prepare_command = dw_mci_rockchip_prepare_command,
>>> };
>>> @@ -73,6 +85,7 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
>>> .prepare_command = dw_mci_rockchip_prepare_command,
>>> .set_ios = dw_mci_rk3288_set_ios,
>>> .setup_clock = dw_mci_rk3288_setup_clock,
>>> + .parse_dt = dw_mci_rk3288_parse_dt,
>>> };
>>>
>>> static const struct of_device_id dw_mci_rockchip_match[] = {
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index 69f0cc6..2ea7467 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -819,7 +819,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>>
>>> /* enable clock; only low power if no SDIO */
>>> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
>>> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
>>> + if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
>>> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
>>> mci_writel(host, CLKENA, clk_en_a);
>>>
>>> @@ -1180,10 +1180,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>>> dw_mci_disable_low_power(slot);
>>>
>>> mci_writel(host, INTMASK,
>>> - (int_mask | SDMMC_INT_SDIO(slot->id)));
>>> + (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
>>> } else {
>>> mci_writel(host, INTMASK,
>>> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
>>> + (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
>>> }
>>> }
>>>
>>> @@ -2035,8 +2035,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>>> /* Handle SDIO Interrupts */
>>> for (i = 0; i < host->num_slots; i++) {
>>> struct dw_mci_slot *slot = host->slot[i];
>>> - if (pending & SDMMC_INT_SDIO(i)) {
>>> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
>>> + if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
>>> + mci_writel(host, RINTSTS,
>>> + SDMMC_INT_SDIO(slot->sdio_id));
>>> mmc_signal_sdio_irq(slot->mmc);
>>> }
>>> }
>>> @@ -2206,6 +2207,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>>>
>>> slot = mmc_priv(mmc);
>>> slot->id = id;
>>> + slot->sdio_id = host->sdio_id0 + id;
>>> slot->mmc = mmc;
>>> slot->host = host;
>>> host->slot[id] = slot;
>>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>>> index 01b99e8..3e966a9 100644
>>> --- a/drivers/mmc/host/dw_mmc.h
>>> +++ b/drivers/mmc/host/dw_mmc.h
>>> @@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
>>> * with CONFIG_MMC_CLKGATE.
>>> * @flags: Random state bits associated with the slot.
>>> * @id: Number of this slot.
>>> + * @sdio_id: Number of this slot in the SDIO interrupt registers.
>>> * @last_detect_state: Most recently observed card detect state.
>>> */
>>> struct dw_mci_slot {
>>> @@ -234,6 +235,7 @@ struct dw_mci_slot {
>>> #define DW_MMC_CARD_PRESENT 0
>>> #define DW_MMC_CARD_NEED_INIT 1
>>> int id;
>>> + int sdio_id;
>>> int last_detect_state;
>>> };
>>>
>>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>>> index 0013669..4c0d3f2 100644
>>> --- a/include/linux/mmc/dw_mmc.h
>>> +++ b/include/linux/mmc/dw_mmc.h
>>> @@ -96,6 +96,7 @@ struct mmc_data;
>>> * @quirks: Set of quirks that apply to specific versions of the IP.
>>> * @irq_flags: The flags to be passed to request_irq.
>>> * @irq: The irq value to be passed to request_irq.
>>> + * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
>>> *
>>> * Locking
>>> * =======
>>> @@ -193,6 +194,8 @@ struct dw_mci {
>>> bool vqmmc_enabled;
>>> unsigned long irq_flags; /* IRQ flags */
>>> int irq;
>>> +
>>> + int sdio_id0;
>>> };
>>>
>>> /* DMA ops for Internal/External DMAC interface */
>>> --
>>> 1.8.3.2
>>>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>
>
>
>

2014-10-31 01:14:44

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: add support for the other bit of sdio interrupt

Hi,

On 10/31/2014 09:46 AM, addy ke wrote:
> Hi, Jaehoon
>
> On 2014/10/30 19:02, Jaehoon Chung wrote:
>> Hi, Addy.
>>
>> This patch is conflicted..Could you rebase on latest Ulf's tree?
> I have not found ulf's tree in git.kernel.org.
> I can't 'git clone git://git.kernel.org/pub/scm/linux/kernel/git/ulf/xxx.git'.
> So my patch is based on kernel-3.18.
> I will send patch v2 for this.
> Would you please give me a git url for it?

http://git.linaro.org/git/people/ulf.hansson/mmc.git
I'm checking with next branch.

Best Regards,
Jaehoon Chung

> Thank you.
>
>>
>> Best Regards,
>> Jaehoon Chung
>>
>> On 10/30/2014 07:50 PM, Addy Ke wrote:
>>> The bit of sdio interrupt is 16 in designware implementation,
>>> but it is 24 in RK3288. This patch add sdio_id0 for the number
>>> of slot0 in the SDIO interrupt registers, which can be set in
>>> platform DT table, such as:
>>> - rockchip,sdio-interrupt-slot0 = <8>;
>>>
>>> Signed-off-by: Addy Ke <[email protected]>
>>> ---
>>> drivers/mmc/host/dw_mmc-rockchip.c | 13 +++++++++++++
>>> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
>>> drivers/mmc/host/dw_mmc.h | 2 ++
>>> include/linux/mmc/dw_mmc.h | 3 +++
>>> 4 files changed, 25 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
>>> index f0c2cb1..54655e7 100644
>>> --- a/drivers/mmc/host/dw_mmc-rockchip.c
>>> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
>>> @@ -65,6 +65,18 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>>> }
>>> }
>>>
>>> +static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
>>> +{
>>> + struct device_node *np = host->dev->of_node;
>>> + int sdio_id0;
>>> +
>>> + if (!of_property_read_u32(np, "rockchip,sdio-interrupt-slot0",
>>> + &sdio_id0))
>>> + host->sdio_id0 = sdio_id0;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static const struct dw_mci_drv_data rk2928_drv_data = {
>>> .prepare_command = dw_mci_rockchip_prepare_command,
>>> };
>>> @@ -73,6 +85,7 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
>>> .prepare_command = dw_mci_rockchip_prepare_command,
>>> .set_ios = dw_mci_rk3288_set_ios,
>>> .setup_clock = dw_mci_rk3288_setup_clock,
>>> + .parse_dt = dw_mci_rk3288_parse_dt,
>>> };
>>>
>>> static const struct of_device_id dw_mci_rockchip_match[] = {
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index 69f0cc6..2ea7467 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -819,7 +819,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>>
>>> /* enable clock; only low power if no SDIO */
>>> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
>>> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
>>> + if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
>>> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
>>> mci_writel(host, CLKENA, clk_en_a);
>>>
>>> @@ -1180,10 +1180,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>>> dw_mci_disable_low_power(slot);
>>>
>>> mci_writel(host, INTMASK,
>>> - (int_mask | SDMMC_INT_SDIO(slot->id)));
>>> + (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
>>> } else {
>>> mci_writel(host, INTMASK,
>>> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
>>> + (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
>>> }
>>> }
>>>
>>> @@ -2035,8 +2035,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>>> /* Handle SDIO Interrupts */
>>> for (i = 0; i < host->num_slots; i++) {
>>> struct dw_mci_slot *slot = host->slot[i];
>>> - if (pending & SDMMC_INT_SDIO(i)) {
>>> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
>>> + if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
>>> + mci_writel(host, RINTSTS,
>>> + SDMMC_INT_SDIO(slot->sdio_id));
>>> mmc_signal_sdio_irq(slot->mmc);
>>> }
>>> }
>>> @@ -2206,6 +2207,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>>>
>>> slot = mmc_priv(mmc);
>>> slot->id = id;
>>> + slot->sdio_id = host->sdio_id0 + id;
>>> slot->mmc = mmc;
>>> slot->host = host;
>>> host->slot[id] = slot;
>>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>>> index 01b99e8..3e966a9 100644
>>> --- a/drivers/mmc/host/dw_mmc.h
>>> +++ b/drivers/mmc/host/dw_mmc.h
>>> @@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
>>> * with CONFIG_MMC_CLKGATE.
>>> * @flags: Random state bits associated with the slot.
>>> * @id: Number of this slot.
>>> + * @sdio_id: Number of this slot in the SDIO interrupt registers.
>>> * @last_detect_state: Most recently observed card detect state.
>>> */
>>> struct dw_mci_slot {
>>> @@ -234,6 +235,7 @@ struct dw_mci_slot {
>>> #define DW_MMC_CARD_PRESENT 0
>>> #define DW_MMC_CARD_NEED_INIT 1
>>> int id;
>>> + int sdio_id;
>>> int last_detect_state;
>>> };
>>>
>>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>>> index 0013669..4c0d3f2 100644
>>> --- a/include/linux/mmc/dw_mmc.h
>>> +++ b/include/linux/mmc/dw_mmc.h
>>> @@ -96,6 +96,7 @@ struct mmc_data;
>>> * @quirks: Set of quirks that apply to specific versions of the IP.
>>> * @irq_flags: The flags to be passed to request_irq.
>>> * @irq: The irq value to be passed to request_irq.
>>> + * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
>>> *
>>> * Locking
>>> * =======
>>> @@ -193,6 +194,8 @@ struct dw_mci {
>>> bool vqmmc_enabled;
>>> unsigned long irq_flags; /* IRQ flags */
>>> int irq;
>>> +
>>> + int sdio_id0;
>>> };
>>>
>>> /* DMA ops for Internal/External DMAC interface */
>>>
>>
>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2014-10-31 03:50:40

[permalink] [raw]

Subject: [PATCH v2] mmc: dw_mmc: add support for the other bit of sdio interrupt

The bit of sdio interrupt is 16 in designware implementation,
but it is 24 in RK3288. This patch add sdio_id0 for the number
of slot0 in the SDIO interrupt registers, which can be set in
platform DT table, such as:
- rockchip,sdio-interrupt-slot0 = <8>;

Signed-off-by: Addy Ke <[email protected]>
---
Changes in v2:
- rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next branch

drivers/mmc/host/dw_mmc-rockchip.c | 13 +++++++++++++
drivers/mmc/host/dw_mmc.c | 12 +++++++-----
drivers/mmc/host/dw_mmc.h | 2 ++
include/linux/mmc/dw_mmc.h | 3 +++
4 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
index bbb4ec3..1cb3bc6 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -68,6 +68,18 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
}
}

+static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
+{
+ struct device_node *np = host->dev->of_node;
+ int sdio_id0;
+
+ if (!of_property_read_u32(np, "rockchip,sdio-interrupt-slot0",
+ &sdio_id0))
+ host->sdio_id0 = sdio_id0;
+
+ return 0;
+}
+
static const struct dw_mci_drv_data rk2928_drv_data = {
.prepare_command = dw_mci_rockchip_prepare_command,
};
@@ -76,6 +88,7 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
.prepare_command = dw_mci_rockchip_prepare_command,
.set_ios = dw_mci_rk3288_set_ios,
.setup_clock = dw_mci_rk3288_setup_clock,
+ .parse_dt = dw_mci_rk3288_parse_dt,
};

static const struct of_device_id dw_mci_rockchip_match[] = {
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index bb46b1b..a633b58 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -823,7 +823,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)

/* enable clock; only low power if no SDIO */
clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
- if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
+ if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
mci_writel(host, CLKENA, clk_en_a);

@@ -1184,10 +1184,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
dw_mci_disable_low_power(slot);

mci_writel(host, INTMASK,
- (int_mask | SDMMC_INT_SDIO(slot->id)));
+ (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
} else {
mci_writel(host, INTMASK,
- (int_mask & ~SDMMC_INT_SDIO(slot->id)));
+ (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
}
}

@@ -2056,8 +2056,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
/* Handle SDIO Interrupts */
for (i = 0; i < host->num_slots; i++) {
struct dw_mci_slot *slot = host->slot[i];
- if (pending & SDMMC_INT_SDIO(i)) {
- mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
+ if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
+ mci_writel(host, RINTSTS,
+ SDMMC_INT_SDIO(slot->sdio_id));
mmc_signal_sdio_irq(slot->mmc);
}
}
@@ -2145,6 +2146,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)

slot = mmc_priv(mmc);
slot->id = id;
+ slot->sdio_id = host->sdio_id0 + id;
slot->mmc = mmc;
slot->host = host;
host->slot[id] = slot;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 71d4995..0562f10 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
* with CONFIG_MMC_CLKGATE.
* @flags: Random state bits associated with the slot.
* @id: Number of this slot.
+ * @sdio_id: Number of this slot in the SDIO interrupt registers.
*/
struct dw_mci_slot {
struct mmc_host *mmc;
@@ -233,6 +234,7 @@ struct dw_mci_slot {
#define DW_MMC_CARD_PRESENT 0
#define DW_MMC_CARD_NEED_INIT 1
int id;
+ int sdio_id;
};

struct dw_mci_tuning_data {
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 69d0814..72c319f 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -96,6 +96,7 @@ struct mmc_data;
* @quirks: Set of quirks that apply to specific versions of the IP.
* @irq_flags: The flags to be passed to request_irq.
* @irq: The irq value to be passed to request_irq.
+ * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
*
* Locking
* =======
@@ -191,6 +192,8 @@ struct dw_mci {
bool vqmmc_enabled;
unsigned long irq_flags; /* IRQ flags */
int irq;
+
+ int sdio_id0;
};

/* DMA ops for Internal/External DMAC interface */
--
1.8.3.2

2014-10-31 05:14:19

[permalink] [raw]

Subject: Re: [PATCH v2] mmc: dw_mmc: add support for the other bit of sdio interrupt

Addy,

On Thu, Oct 30, 2014 at 8:50 PM, Addy Ke <[email protected]> wrote:
> +static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
> +{
> + struct device_node *np = host->dev->of_node;
> + int sdio_id0;
> +
> + if (!of_property_read_u32(np, "rockchip,sdio-interrupt-slot0",
> + &sdio_id0))
> + host->sdio_id0 = sdio_id0;

This function is only run on rk3288 and on all rk3288 SoCs this value
is exactly 8. Just replace this with:

/* SDIO IRQ shows up as if it were slot 8 on rk3288 SoCs */
host->sdio_id0 = 8;

2014-10-31 08:45:22

[permalink] [raw]

Subject: Re: [PATCH v2] mmc: dw_mmc: add support for the other bit of sdio interrupt

Hi, Addy.

On 10/31/2014 12:50 PM, Addy Ke wrote:
> The bit of sdio interrupt is 16 in designware implementation,
> but it is 24 in RK3288. This patch add sdio_id0 for the number
> of slot0 in the SDIO interrupt registers, which can be set in
> platform DT table, such as:
> - rockchip,sdio-interrupt-slot0 = <8>;

I have a question. (It's not important question)
You mentioned the sdio-irq bit used from 24 to 31, right?
Then what interrupts are used from 16 to 23 at RK3288? Just reserved?

Best Regards,
Jaehoon Chung

>
> Signed-off-by: Addy Ke <[email protected]>
> ---
> Changes in v2:
> - rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next branch
>
> drivers/mmc/host/dw_mmc-rockchip.c | 13 +++++++++++++
> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
> drivers/mmc/host/dw_mmc.h | 2 ++
> include/linux/mmc/dw_mmc.h | 3 +++
> 4 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> index bbb4ec3..1cb3bc6 100644
> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> @@ -68,6 +68,18 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> }
> }
>
> +static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
> +{
> + struct device_node *np = host->dev->of_node;
> + int sdio_id0;
> +
> + if (!of_property_read_u32(np, "rockchip,sdio-interrupt-slot0",
> + &sdio_id0))
> + host->sdio_id0 = sdio_id0;
> +
> + return 0;
> +}
> +
> static const struct dw_mci_drv_data rk2928_drv_data = {
> .prepare_command = dw_mci_rockchip_prepare_command,
> };
> @@ -76,6 +88,7 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
> .prepare_command = dw_mci_rockchip_prepare_command,
> .set_ios = dw_mci_rk3288_set_ios,
> .setup_clock = dw_mci_rk3288_setup_clock,
> + .parse_dt = dw_mci_rk3288_parse_dt,
> };
>
> static const struct of_device_id dw_mci_rockchip_match[] = {
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index bb46b1b..a633b58 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -823,7 +823,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>
> /* enable clock; only low power if no SDIO */
> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
> + if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
> mci_writel(host, CLKENA, clk_en_a);
>
> @@ -1184,10 +1184,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
> dw_mci_disable_low_power(slot);
>
> mci_writel(host, INTMASK,
> - (int_mask | SDMMC_INT_SDIO(slot->id)));
> + (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
> } else {
> mci_writel(host, INTMASK,
> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
> + (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
> }
> }
>
> @@ -2056,8 +2056,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> /* Handle SDIO Interrupts */
> for (i = 0; i < host->num_slots; i++) {
> struct dw_mci_slot *slot = host->slot[i];
> - if (pending & SDMMC_INT_SDIO(i)) {
> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
> + if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
> + mci_writel(host, RINTSTS,
> + SDMMC_INT_SDIO(slot->sdio_id));
> mmc_signal_sdio_irq(slot->mmc);
> }
> }
> @@ -2145,6 +2146,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>
> slot = mmc_priv(mmc);
> slot->id = id;
> + slot->sdio_id = host->sdio_id0 + id;
> slot->mmc = mmc;
> slot->host = host;
> host->slot[id] = slot;
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 71d4995..0562f10 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
> * with CONFIG_MMC_CLKGATE.
> * @flags: Random state bits associated with the slot.
> * @id: Number of this slot.
> + * @sdio_id: Number of this slot in the SDIO interrupt registers.
> */
> struct dw_mci_slot {
> struct mmc_host *mmc;
> @@ -233,6 +234,7 @@ struct dw_mci_slot {
> #define DW_MMC_CARD_PRESENT 0
> #define DW_MMC_CARD_NEED_INIT 1
> int id;
> + int sdio_id;
> };
>
> struct dw_mci_tuning_data {
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 69d0814..72c319f 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -96,6 +96,7 @@ struct mmc_data;
> * @quirks: Set of quirks that apply to specific versions of the IP.
> * @irq_flags: The flags to be passed to request_irq.
> * @irq: The irq value to be passed to request_irq.
> + * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
> *
> * Locking
> * =======
> @@ -191,6 +192,8 @@ struct dw_mci {
> bool vqmmc_enabled;
> unsigned long irq_flags; /* IRQ flags */
> int irq;
> +
> + int sdio_id0;
> };
>
> /* DMA ops for Internal/External DMAC interface */
>

2014-10-31 10:40:58

by Heiko Stuebner

[permalink] [raw]

Subject: Re: [PATCH v2] mmc: dw_mmc: add support for the other bit of sdio interrupt

Am Freitag, 31. Oktober 2014, 11:50:09 schrieb Addy Ke:
> The bit of sdio interrupt is 16 in designware implementation,
> but it is 24 in RK3288. This patch add sdio_id0 for the number
> of slot0 in the SDIO interrupt registers, which can be set in
> platform DT table, such as:
> - rockchip,sdio-interrupt-slot0 = <8>;

I just checked the manuals of rk3066 and rk3188 - and it seems the sdio
interrupt is in bit 24 on all of them.

Addy, could you check this and maybe enable this for all two variants we
currently support?

Thanks
Heiko

>
> Signed-off-by: Addy Ke <[email protected]>
> ---
> Changes in v2:
> - rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next
> branch
>
> drivers/mmc/host/dw_mmc-rockchip.c | 13 +++++++++++++
> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
> drivers/mmc/host/dw_mmc.h | 2 ++
> include/linux/mmc/dw_mmc.h | 3 +++
> 4 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c
> b/drivers/mmc/host/dw_mmc-rockchip.c index bbb4ec3..1cb3bc6 100644
> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> @@ -68,6 +68,18 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host,
> struct mmc_ios *ios) }
> }
>
> +static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
> +{
> + struct device_node *np = host->dev->of_node;
> + int sdio_id0;
> +
> + if (!of_property_read_u32(np, "rockchip,sdio-interrupt-slot0",
> + &sdio_id0))
> + host->sdio_id0 = sdio_id0;
> +
> + return 0;
> +}
> +
> static const struct dw_mci_drv_data rk2928_drv_data = {
> .prepare_command = dw_mci_rockchip_prepare_command,
> };
> @@ -76,6 +88,7 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
> .prepare_command = dw_mci_rockchip_prepare_command,
> .set_ios = dw_mci_rk3288_set_ios,
> .setup_clock = dw_mci_rk3288_setup_clock,
> + .parse_dt = dw_mci_rk3288_parse_dt,
> };
>
> static const struct of_device_id dw_mci_rockchip_match[] = {
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index bb46b1b..a633b58 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -823,7 +823,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot,
> bool force_clkinit)
>
> /* enable clock; only low power if no SDIO */
> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
> + if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
> mci_writel(host, CLKENA, clk_en_a);
>
> @@ -1184,10 +1184,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host
> *mmc, int enb) dw_mci_disable_low_power(slot);
>
> mci_writel(host, INTMASK,
> - (int_mask | SDMMC_INT_SDIO(slot->id)));
> + (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
> } else {
> mci_writel(host, INTMASK,
> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
> + (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
> }
> }
>
> @@ -2056,8 +2056,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void
> *dev_id) /* Handle SDIO Interrupts */
> for (i = 0; i < host->num_slots; i++) {
> struct dw_mci_slot *slot = host->slot[i];
> - if (pending & SDMMC_INT_SDIO(i)) {
> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
> + if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
> + mci_writel(host, RINTSTS,
> + SDMMC_INT_SDIO(slot->sdio_id));
> mmc_signal_sdio_irq(slot->mmc);
> }
> }
> @@ -2145,6 +2146,7 @@ static int dw_mci_init_slot(struct dw_mci *host,
> unsigned int id)
>
> slot = mmc_priv(mmc);
> slot->id = id;
> + slot->sdio_id = host->sdio_id0 + id;
> slot->mmc = mmc;
> slot->host = host;
> host->slot[id] = slot;
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 71d4995..0562f10 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
> * with CONFIG_MMC_CLKGATE.
> * @flags: Random state bits associated with the slot.
> * @id: Number of this slot.
> + * @sdio_id: Number of this slot in the SDIO interrupt registers.
> */
> struct dw_mci_slot {
> struct mmc_host *mmc;
> @@ -233,6 +234,7 @@ struct dw_mci_slot {
> #define DW_MMC_CARD_PRESENT 0
> #define DW_MMC_CARD_NEED_INIT 1
> int id;
> + int sdio_id;
> };
>
> struct dw_mci_tuning_data {
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 69d0814..72c319f 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -96,6 +96,7 @@ struct mmc_data;
> * @quirks: Set of quirks that apply to specific versions of the IP.
> * @irq_flags: The flags to be passed to request_irq.
> * @irq: The irq value to be passed to request_irq.
> + * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
> *
> * Locking
> * =======
> @@ -191,6 +192,8 @@ struct dw_mci {
> bool vqmmc_enabled;
> unsigned long irq_flags; /* IRQ flags */
> int irq;
> +
> + int sdio_id0;
> };
>
> /* DMA ops for Internal/External DMAC interface */

2014-10-31 15:55:17

[permalink] [raw]

Subject: Re: [PATCH v2] mmc: dw_mmc: add support for the other bit of sdio interrupt

Jaehoon,

On Fri, Oct 31, 2014 at 1:45 AM, Jaehoon Chung <[email protected]> wrote:
> Hi, Addy.
>
> On 10/31/2014 12:50 PM, Addy Ke wrote:
>> The bit of sdio interrupt is 16 in designware implementation,
>> but it is 24 in RK3288. This patch add sdio_id0 for the number
>> of slot0 in the SDIO interrupt registers, which can be set in
>> platform DT table, such as:
>> - rockchip,sdio-interrupt-slot0 = <8>;
>
> I have a question. (It's not important question)
> You mentioned the sdio-irq bit used from 24 to 31, right?
> Then what interrupts are used from 16 to 23 at RK3288? Just reserved?

In my TRM 16 shows as "data_nobusy". A quick search doesn't show any
documentation for what this means. 17-23 and 25-31 are reserved.

Note that (of course) there's no actual support for more than one slot
on rk3288, so claiming that "24-31" is SDIO INT is a little silly, but
I agree that it's probably is the best way to fit into the existing
code until someone gets around to removing the whole multi-slot cruft.

2014-11-03 00:55:11

[permalink] [raw]

Subject: Re: [PATCH v2] mmc: dw_mmc: add support for the other bit of sdio interrupt

On 2014/10/31 18:43, Heiko St?bner wrote:
> Am Freitag, 31. Oktober 2014, 11:50:09 schrieb Addy Ke:
>> The bit of sdio interrupt is 16 in designware implementation,
>> but it is 24 in RK3288. This patch add sdio_id0 for the number
>> of slot0 in the SDIO interrupt registers, which can be set in
>> platform DT table, such as:
>> - rockchip,sdio-interrupt-slot0 = <8>;
>
> I just checked the manuals of rk3066 and rk3188 - and it seems the sdio
> interrupt is in bit 24 on all of them.
>
> Addy, could you check this and maybe enable this for all two variants we
> currently support?
OK, I will do so in my next patch. thank you!
>
>
> Thanks
> Heiko
>
>
>>
>> Signed-off-by: Addy Ke <[email protected]>
>> ---
>> Changes in v2:
>> - rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next
>> branch
>>
>> drivers/mmc/host/dw_mmc-rockchip.c | 13 +++++++++++++
>> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
>> drivers/mmc/host/dw_mmc.h | 2 ++
>> include/linux/mmc/dw_mmc.h | 3 +++
>> 4 files changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c
>> b/drivers/mmc/host/dw_mmc-rockchip.c index bbb4ec3..1cb3bc6 100644
>> --- a/drivers/mmc/host/dw_mmc-rockchip.c
>> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
>> @@ -68,6 +68,18 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host,
>> struct mmc_ios *ios) }
>> }
>>
>> +static int dw_mci_rk3288_parse_dt(struct dw_mci *host)
>> +{
>> + struct device_node *np = host->dev->of_node;
>> + int sdio_id0;
>> +
>> + if (!of_property_read_u32(np, "rockchip,sdio-interrupt-slot0",
>> + &sdio_id0))
>> + host->sdio_id0 = sdio_id0;
>> +
>> + return 0;
>> +}
>> +
>> static const struct dw_mci_drv_data rk2928_drv_data = {
>> .prepare_command = dw_mci_rockchip_prepare_command,
>> };
>> @@ -76,6 +88,7 @@ static const struct dw_mci_drv_data rk3288_drv_data = {
>> .prepare_command = dw_mci_rockchip_prepare_command,
>> .set_ios = dw_mci_rk3288_set_ios,
>> .setup_clock = dw_mci_rk3288_setup_clock,
>> + .parse_dt = dw_mci_rk3288_parse_dt,
>> };
>>
>> static const struct of_device_id dw_mci_rockchip_match[] = {
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index bb46b1b..a633b58 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -823,7 +823,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot,
>> bool force_clkinit)
>>
>> /* enable clock; only low power if no SDIO */
>> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
>> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
>> + if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
>> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
>> mci_writel(host, CLKENA, clk_en_a);
>>
>> @@ -1184,10 +1184,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host
>> *mmc, int enb) dw_mci_disable_low_power(slot);
>>
>> mci_writel(host, INTMASK,
>> - (int_mask | SDMMC_INT_SDIO(slot->id)));
>> + (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
>> } else {
>> mci_writel(host, INTMASK,
>> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
>> + (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
>> }
>> }
>>
>> @@ -2056,8 +2056,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void
>> *dev_id) /* Handle SDIO Interrupts */
>> for (i = 0; i < host->num_slots; i++) {
>> struct dw_mci_slot *slot = host->slot[i];
>> - if (pending & SDMMC_INT_SDIO(i)) {
>> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
>> + if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
>> + mci_writel(host, RINTSTS,
>> + SDMMC_INT_SDIO(slot->sdio_id));
>> mmc_signal_sdio_irq(slot->mmc);
>> }
>> }
>> @@ -2145,6 +2146,7 @@ static int dw_mci_init_slot(struct dw_mci *host,
>> unsigned int id)
>>
>> slot = mmc_priv(mmc);
>> slot->id = id;
>> + slot->sdio_id = host->sdio_id0 + id;
>> slot->mmc = mmc;
>> slot->host = host;
>> host->slot[id] = slot;
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index 71d4995..0562f10 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
>> * with CONFIG_MMC_CLKGATE.
>> * @flags: Random state bits associated with the slot.
>> * @id: Number of this slot.
>> + * @sdio_id: Number of this slot in the SDIO interrupt registers.
>> */
>> struct dw_mci_slot {
>> struct mmc_host *mmc;
>> @@ -233,6 +234,7 @@ struct dw_mci_slot {
>> #define DW_MMC_CARD_PRESENT 0
>> #define DW_MMC_CARD_NEED_INIT 1
>> int id;
>> + int sdio_id;
>> };
>>
>> struct dw_mci_tuning_data {
>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>> index 69d0814..72c319f 100644
>> --- a/include/linux/mmc/dw_mmc.h
>> +++ b/include/linux/mmc/dw_mmc.h
>> @@ -96,6 +96,7 @@ struct mmc_data;
>> * @quirks: Set of quirks that apply to specific versions of the IP.
>> * @irq_flags: The flags to be passed to request_irq.
>> * @irq: The irq value to be passed to request_irq.
>> + * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
>> *
>> * Locking
>> * =======
>> @@ -191,6 +192,8 @@ struct dw_mci {
>> bool vqmmc_enabled;
>> unsigned long irq_flags; /* IRQ flags */
>> int irq;
>> +
>> + int sdio_id0;
>> };
>>
>> /* DMA ops for Internal/External DMAC interface */
>
>
>
>

2014-11-03 01:21:27

[permalink] [raw]

Subject: [PATCH v3] mmc: dw_mmc: add support for the other bit of sdio interrupt

The bit of sdio interrupt is 16 in designware implementation,
but it is 24 on Rockchip SoCs.This patch add sdio_id0 for the
number of slot0 in the SDIO interrupt registers.

Signed-off-by: Addy Ke <[email protected]>
---
Changes in v2:
- rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next branch
Changes in v3:
- Remove dts for sdio_id0, just replace this with 8, suggested by Doug
- Change to support all Rockchip Socs, suggested by Heiko

drivers/mmc/host/dw_mmc-rockchip.c | 10 ++++++++++
drivers/mmc/host/dw_mmc.c | 12 +++++++-----
drivers/mmc/host/dw_mmc.h | 2 ++
include/linux/mmc/dw_mmc.h | 3 +++
4 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
index bbb4ec3..b997c8f 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -68,14 +68,24 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
}
}

+static int dw_mci_rockchip_parse_dt(struct dw_mci *host)
+{
+ /* It is slot 8 on Rockchip SoCs */
+ host->sdio_id0 = 8;
+
+ return 0;
+}
+
static const struct dw_mci_drv_data rk2928_drv_data = {
.prepare_command = dw_mci_rockchip_prepare_command,
+ .parse_dt = dw_mci_rockchip_parse_dt,
};

static const struct dw_mci_drv_data rk3288_drv_data = {
.prepare_command = dw_mci_rockchip_prepare_command,
.set_ios = dw_mci_rk3288_set_ios,
.setup_clock = dw_mci_rk3288_setup_clock,
+ .parse_dt = dw_mci_rockchip_parse_dt,
};

static const struct of_device_id dw_mci_rockchip_match[] = {
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index bb46b1b..a633b58 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -823,7 +823,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)

/* enable clock; only low power if no SDIO */
clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
- if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
+ if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
mci_writel(host, CLKENA, clk_en_a);

@@ -1184,10 +1184,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
dw_mci_disable_low_power(slot);

mci_writel(host, INTMASK,
- (int_mask | SDMMC_INT_SDIO(slot->id)));
+ (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
} else {
mci_writel(host, INTMASK,
- (int_mask & ~SDMMC_INT_SDIO(slot->id)));
+ (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
}
}

@@ -2056,8 +2056,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
/* Handle SDIO Interrupts */
for (i = 0; i < host->num_slots; i++) {
struct dw_mci_slot *slot = host->slot[i];
- if (pending & SDMMC_INT_SDIO(i)) {
- mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
+ if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
+ mci_writel(host, RINTSTS,
+ SDMMC_INT_SDIO(slot->sdio_id));
mmc_signal_sdio_irq(slot->mmc);
}
}
@@ -2145,6 +2146,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)

slot = mmc_priv(mmc);
slot->id = id;
+ slot->sdio_id = host->sdio_id0 + id;
slot->mmc = mmc;
slot->host = host;
host->slot[id] = slot;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 71d4995..0562f10 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
* with CONFIG_MMC_CLKGATE.
* @flags: Random state bits associated with the slot.
* @id: Number of this slot.
+ * @sdio_id: Number of this slot in the SDIO interrupt registers.
*/
struct dw_mci_slot {
struct mmc_host *mmc;
@@ -233,6 +234,7 @@ struct dw_mci_slot {
#define DW_MMC_CARD_PRESENT 0
#define DW_MMC_CARD_NEED_INIT 1
int id;
+ int sdio_id;
};

struct dw_mci_tuning_data {
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 69d0814..72c319f 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -96,6 +96,7 @@ struct mmc_data;
* @quirks: Set of quirks that apply to specific versions of the IP.
* @irq_flags: The flags to be passed to request_irq.
* @irq: The irq value to be passed to request_irq.
+ * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
*
* Locking
* =======
@@ -191,6 +192,8 @@ struct dw_mci {
bool vqmmc_enabled;
unsigned long irq_flags; /* IRQ flags */
int irq;
+
+ int sdio_id0;
};

/* DMA ops for Internal/External DMAC interface */
--
1.8.3.2

2014-11-03 09:00:24

[permalink] [raw]

Subject: Re: [PATCH v3] mmc: dw_mmc: add support for the other bit of sdio interrupt

Hi, Addy.

On 11/03/2014 10:20 AM, Addy Ke wrote:
> The bit of sdio interrupt is 16 in designware implementation,
> but it is 24 on Rockchip SoCs.This patch add sdio_id0 for the
> number of slot0 in the SDIO interrupt registers.
>
> Signed-off-by: Addy Ke <[email protected]>
> ---
> Changes in v2:
> - rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next branch
> Changes in v3:
> - Remove dts for sdio_id0, just replace this with 8, suggested by Doug
> - Change to support all Rockchip Socs, suggested by Heiko
>
> drivers/mmc/host/dw_mmc-rockchip.c | 10 ++++++++++
> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
> drivers/mmc/host/dw_mmc.h | 2 ++
> include/linux/mmc/dw_mmc.h | 3 +++
> 4 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> index bbb4ec3..b997c8f 100644
> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> @@ -68,14 +68,24 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> }
> }
>
> +static int dw_mci_rockchip_parse_dt(struct dw_mci *host)
> +{
> + /* It is slot 8 on Rockchip SoCs */
> + host->sdio_id0 = 8;
> +
> + return 0;
> +}

Well, function is "__parse_dt__", but this function don't parse anything.
If All rockchip soc is supported, i think that it can be located to other place.

Best Regards,
Jaehoon Chung

> +
> static const struct dw_mci_drv_data rk2928_drv_data = {
> .prepare_command = dw_mci_rockchip_prepare_command,
> + .parse_dt = dw_mci_rockchip_parse_dt,
> };
>
> static const struct dw_mci_drv_data rk3288_drv_data = {
> .prepare_command = dw_mci_rockchip_prepare_command,
> .set_ios = dw_mci_rk3288_set_ios,
> .setup_clock = dw_mci_rk3288_setup_clock,
> + .parse_dt = dw_mci_rockchip_parse_dt,
> };
>
> static const struct of_device_id dw_mci_rockchip_match[] = {
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index bb46b1b..a633b58 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -823,7 +823,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>
> /* enable clock; only low power if no SDIO */
> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
> + if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
> mci_writel(host, CLKENA, clk_en_a);
>
> @@ -1184,10 +1184,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
> dw_mci_disable_low_power(slot);
>
> mci_writel(host, INTMASK,
> - (int_mask | SDMMC_INT_SDIO(slot->id)));
> + (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
> } else {
> mci_writel(host, INTMASK,
> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
> + (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
> }
> }
>
> @@ -2056,8 +2056,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> /* Handle SDIO Interrupts */
> for (i = 0; i < host->num_slots; i++) {
> struct dw_mci_slot *slot = host->slot[i];
> - if (pending & SDMMC_INT_SDIO(i)) {
> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
> + if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
> + mci_writel(host, RINTSTS,
> + SDMMC_INT_SDIO(slot->sdio_id));
> mmc_signal_sdio_irq(slot->mmc);
> }
> }
> @@ -2145,6 +2146,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>
> slot = mmc_priv(mmc);
> slot->id = id;
> + slot->sdio_id = host->sdio_id0 + id;
> slot->mmc = mmc;
> slot->host = host;
> host->slot[id] = slot;
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 71d4995..0562f10 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
> * with CONFIG_MMC_CLKGATE.
> * @flags: Random state bits associated with the slot.
> * @id: Number of this slot.
> + * @sdio_id: Number of this slot in the SDIO interrupt registers.
> */
> struct dw_mci_slot {
> struct mmc_host *mmc;
> @@ -233,6 +234,7 @@ struct dw_mci_slot {
> #define DW_MMC_CARD_PRESENT 0
> #define DW_MMC_CARD_NEED_INIT 1
> int id;
> + int sdio_id;
> };
>
> struct dw_mci_tuning_data {
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 69d0814..72c319f 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -96,6 +96,7 @@ struct mmc_data;
> * @quirks: Set of quirks that apply to specific versions of the IP.
> * @irq_flags: The flags to be passed to request_irq.
> * @irq: The irq value to be passed to request_irq.
> + * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
> *
> * Locking
> * =======
> @@ -191,6 +192,8 @@ struct dw_mci {
> bool vqmmc_enabled;
> unsigned long irq_flags; /* IRQ flags */
> int irq;
> +
> + int sdio_id0;
> };
>
> /* DMA ops for Internal/External DMAC interface */
>

2014-11-03 10:21:00

by Heiko Stuebner

[permalink] [raw]

Subject: Re: [PATCH v3] mmc: dw_mmc: add support for the other bit of sdio interrupt

Hi Jaehoon,

Am Montag, 3. November 2014, 17:59:58 schrieb Jaehoon Chung:
> Hi, Addy.
>
> On 11/03/2014 10:20 AM, Addy Ke wrote:
> > The bit of sdio interrupt is 16 in designware implementation,
> > but it is 24 on Rockchip SoCs.This patch add sdio_id0 for the
> > number of slot0 in the SDIO interrupt registers.
> >
> > Signed-off-by: Addy Ke <[email protected]>
> > ---
> > Changes in v2:
> > - rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next
> > branch Changes in v3:
> > - Remove dts for sdio_id0, just replace this with 8, suggested by Doug
> > - Change to support all Rockchip Socs, suggested by Heiko
> >
> > drivers/mmc/host/dw_mmc-rockchip.c | 10 ++++++++++
> > drivers/mmc/host/dw_mmc.c | 12 +++++++-----
> > drivers/mmc/host/dw_mmc.h | 2 ++
> > include/linux/mmc/dw_mmc.h | 3 +++
> > 4 files changed, 22 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/mmc/host/dw_mmc-rockchip.c
> > b/drivers/mmc/host/dw_mmc-rockchip.c index bbb4ec3..b997c8f 100644
> > --- a/drivers/mmc/host/dw_mmc-rockchip.c
> > +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> > @@ -68,14 +68,24 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host,
> > struct mmc_ios *ios)>
> > }
> >
> > }
> >
> > +static int dw_mci_rockchip_parse_dt(struct dw_mci *host)
> > +{
> > + /* It is slot 8 on Rockchip SoCs */
> > + host->sdio_id0 = 8;
> > +
> > + return 0;
> > +}
>
> Well, function is "__parse_dt__", but this function don't parse anything.
> If All rockchip soc is supported, i think that it can be located to other
> place.

do you have a suggestion for a location?

The only alternative I can see right now would be using the init-hook in
dw_mci_drv_data or adding a new field to it holding the slot-offset.
[with using the init-hook being my personal preference of the two]

Heiko

2014-11-03 10:24:17

[permalink] [raw]

Subject: Re: [PATCH v3] mmc: dw_mmc: add support for the other bit of sdio interrupt

Hi, Jaehoo

On 2014/11/3 16:59, Jaehoon Chung wrote:
> Hi, Addy.
>
> On 11/03/2014 10:20 AM, Addy Ke wrote:
>> The bit of sdio interrupt is 16 in designware implementation,
>> but it is 24 on Rockchip SoCs.This patch add sdio_id0 for the
>> number of slot0 in the SDIO interrupt registers.
>>
>> Signed-off-by: Addy Ke <[email protected]>
>> ---
>> Changes in v2:
>> - rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next branch
>> Changes in v3:
>> - Remove dts for sdio_id0, just replace this with 8, suggested by Doug
>> - Change to support all Rockchip Socs, suggested by Heiko
>>
>> drivers/mmc/host/dw_mmc-rockchip.c | 10 ++++++++++
>> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
>> drivers/mmc/host/dw_mmc.h | 2 ++
>> include/linux/mmc/dw_mmc.h | 3 +++
>> 4 files changed, 22 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
>> index bbb4ec3..b997c8f 100644
>> --- a/drivers/mmc/host/dw_mmc-rockchip.c
>> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
>> @@ -68,14 +68,24 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>> }
>> }
>>
>> +static int dw_mci_rockchip_parse_dt(struct dw_mci *host)
>> +{
>> + /* It is slot 8 on Rockchip SoCs */
>> + host->sdio_id0 = 8;
>> +
>> + return 0;
>> +}
>
> Well, function is "__parse_dt__", but this function don't parse anything.
> If All rockchip soc is supported, i think that it can be located to other place.
>
Can add it in "init" function? like this:
int dw_mci_rockchip_init(struct dw_mci *host)
{
/* It is slot 8 on Rockchip SoCs */
host->sdio_id0 = 8;

return 0;
}
static const struct dw_mci_drv_data xxxx {
....
.init = dw_mci_rockchip_init,
};

> Best Regards,
> Jaehoon Chung
>
>> +
>> static const struct dw_mci_drv_data rk2928_drv_data = {
>> .prepare_command = dw_mci_rockchip_prepare_command,
>> + .parse_dt = dw_mci_rockchip_parse_dt,
>> };
>>
>> static const struct dw_mci_drv_data rk3288_drv_data = {
>> .prepare_command = dw_mci_rockchip_prepare_command,
>> .set_ios = dw_mci_rk3288_set_ios,
>> .setup_clock = dw_mci_rk3288_setup_clock,
>> + .parse_dt = dw_mci_rockchip_parse_dt,
>> };
>>
>> static const struct of_device_id dw_mci_rockchip_match[] = {
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index bb46b1b..a633b58 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -823,7 +823,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>
>> /* enable clock; only low power if no SDIO */
>> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
>> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
>> + if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
>> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
>> mci_writel(host, CLKENA, clk_en_a);
>>
>> @@ -1184,10 +1184,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>> dw_mci_disable_low_power(slot);
>>
>> mci_writel(host, INTMASK,
>> - (int_mask | SDMMC_INT_SDIO(slot->id)));
>> + (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
>> } else {
>> mci_writel(host, INTMASK,
>> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
>> + (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
>> }
>> }
>>
>> @@ -2056,8 +2056,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>> /* Handle SDIO Interrupts */
>> for (i = 0; i < host->num_slots; i++) {
>> struct dw_mci_slot *slot = host->slot[i];
>> - if (pending & SDMMC_INT_SDIO(i)) {
>> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
>> + if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
>> + mci_writel(host, RINTSTS,
>> + SDMMC_INT_SDIO(slot->sdio_id));
>> mmc_signal_sdio_irq(slot->mmc);
>> }
>> }
>> @@ -2145,6 +2146,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>>
>> slot = mmc_priv(mmc);
>> slot->id = id;
>> + slot->sdio_id = host->sdio_id0 + id;
>> slot->mmc = mmc;
>> slot->host = host;
>> host->slot[id] = slot;
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index 71d4995..0562f10 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
>> * with CONFIG_MMC_CLKGATE.
>> * @flags: Random state bits associated with the slot.
>> * @id: Number of this slot.
>> + * @sdio_id: Number of this slot in the SDIO interrupt registers.
>> */
>> struct dw_mci_slot {
>> struct mmc_host *mmc;
>> @@ -233,6 +234,7 @@ struct dw_mci_slot {
>> #define DW_MMC_CARD_PRESENT 0
>> #define DW_MMC_CARD_NEED_INIT 1
>> int id;
>> + int sdio_id;
>> };
>>
>> struct dw_mci_tuning_data {
>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>> index 69d0814..72c319f 100644
>> --- a/include/linux/mmc/dw_mmc.h
>> +++ b/include/linux/mmc/dw_mmc.h
>> @@ -96,6 +96,7 @@ struct mmc_data;
>> * @quirks: Set of quirks that apply to specific versions of the IP.
>> * @irq_flags: The flags to be passed to request_irq.
>> * @irq: The irq value to be passed to request_irq.
>> + * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
>> *
>> * Locking
>> * =======
>> @@ -191,6 +192,8 @@ struct dw_mci {
>> bool vqmmc_enabled;
>> unsigned long irq_flags; /* IRQ flags */
>> int irq;
>> +
>> + int sdio_id0;
>> };
>>
>> /* DMA ops for Internal/External DMAC interface */
>>
>
>
>
>

2014-11-04 02:14:23

[permalink] [raw]

Subject: Re: [PATCH v3] mmc: dw_mmc: add support for the other bit of sdio interrupt

Dear, Addy.

On 11/03/2014 07:23 PM, addy ke wrote:
> Hi, Jaehoo
>
> On 2014/11/3 16:59, Jaehoon Chung wrote:
>> Hi, Addy.
>>
>> On 11/03/2014 10:20 AM, Addy Ke wrote:
>>> The bit of sdio interrupt is 16 in designware implementation,
>>> but it is 24 on Rockchip SoCs.This patch add sdio_id0 for the
>>> number of slot0 in the SDIO interrupt registers.
>>>
>>> Signed-off-by: Addy Ke <[email protected]>
>>> ---
>>> Changes in v2:
>>> - rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next branch
>>> Changes in v3:
>>> - Remove dts for sdio_id0, just replace this with 8, suggested by Doug
>>> - Change to support all Rockchip Socs, suggested by Heiko
>>>
>>> drivers/mmc/host/dw_mmc-rockchip.c | 10 ++++++++++
>>> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
>>> drivers/mmc/host/dw_mmc.h | 2 ++
>>> include/linux/mmc/dw_mmc.h | 3 +++
>>> 4 files changed, 22 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
>>> index bbb4ec3..b997c8f 100644
>>> --- a/drivers/mmc/host/dw_mmc-rockchip.c
>>> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
>>> @@ -68,14 +68,24 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
>>> }
>>> }
>>>
>>> +static int dw_mci_rockchip_parse_dt(struct dw_mci *host)
>>> +{
>>> + /* It is slot 8 on Rockchip SoCs */
>>> + host->sdio_id0 = 8;
>>> +
>>> + return 0;
>>> +}
>>
>> Well, function is "__parse_dt__", but this function don't parse anything.
>> If All rockchip soc is supported, i think that it can be located to other place.
>>
> Can add it in "init" function? like this:
> int dw_mci_rockchip_init(struct dw_mci *host)
> {
> /* It is slot 8 on Rockchip SoCs */
> host->sdio_id0 = 8;
>
> return 0;
> }
> static const struct dw_mci_drv_data xxxx {
> ....
> .init = dw_mci_rockchip_init,
> };

I think good this solution is used. "init-hook" can be also used in future.
When you resend the patch, i will reply with my-ack.

Best Regards,
Jaehoon Chung

>
>
>> Best Regards,
>> Jaehoon Chung
>>
>>> +
>>> static const struct dw_mci_drv_data rk2928_drv_data = {
>>> .prepare_command = dw_mci_rockchip_prepare_command,
>>> + .parse_dt = dw_mci_rockchip_parse_dt,
>>> };
>>>
>>> static const struct dw_mci_drv_data rk3288_drv_data = {
>>> .prepare_command = dw_mci_rockchip_prepare_command,
>>> .set_ios = dw_mci_rk3288_set_ios,
>>> .setup_clock = dw_mci_rk3288_setup_clock,
>>> + .parse_dt = dw_mci_rockchip_parse_dt,
>>> };
>>>
>>> static const struct of_device_id dw_mci_rockchip_match[] = {
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index bb46b1b..a633b58 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -823,7 +823,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>>
>>> /* enable clock; only low power if no SDIO */
>>> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
>>> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
>>> + if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
>>> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
>>> mci_writel(host, CLKENA, clk_en_a);
>>>
>>> @@ -1184,10 +1184,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
>>> dw_mci_disable_low_power(slot);
>>>
>>> mci_writel(host, INTMASK,
>>> - (int_mask | SDMMC_INT_SDIO(slot->id)));
>>> + (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
>>> } else {
>>> mci_writel(host, INTMASK,
>>> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
>>> + (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
>>> }
>>> }
>>>
>>> @@ -2056,8 +2056,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>>> /* Handle SDIO Interrupts */
>>> for (i = 0; i < host->num_slots; i++) {
>>> struct dw_mci_slot *slot = host->slot[i];
>>> - if (pending & SDMMC_INT_SDIO(i)) {
>>> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
>>> + if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
>>> + mci_writel(host, RINTSTS,
>>> + SDMMC_INT_SDIO(slot->sdio_id));
>>> mmc_signal_sdio_irq(slot->mmc);
>>> }
>>> }
>>> @@ -2145,6 +2146,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>>>
>>> slot = mmc_priv(mmc);
>>> slot->id = id;
>>> + slot->sdio_id = host->sdio_id0 + id;
>>> slot->mmc = mmc;
>>> slot->host = host;
>>> host->slot[id] = slot;
>>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>>> index 71d4995..0562f10 100644
>>> --- a/drivers/mmc/host/dw_mmc.h
>>> +++ b/drivers/mmc/host/dw_mmc.h
>>> @@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
>>> * with CONFIG_MMC_CLKGATE.
>>> * @flags: Random state bits associated with the slot.
>>> * @id: Number of this slot.
>>> + * @sdio_id: Number of this slot in the SDIO interrupt registers.
>>> */
>>> struct dw_mci_slot {
>>> struct mmc_host *mmc;
>>> @@ -233,6 +234,7 @@ struct dw_mci_slot {
>>> #define DW_MMC_CARD_PRESENT 0
>>> #define DW_MMC_CARD_NEED_INIT 1
>>> int id;
>>> + int sdio_id;
>>> };
>>>
>>> struct dw_mci_tuning_data {
>>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>>> index 69d0814..72c319f 100644
>>> --- a/include/linux/mmc/dw_mmc.h
>>> +++ b/include/linux/mmc/dw_mmc.h
>>> @@ -96,6 +96,7 @@ struct mmc_data;
>>> * @quirks: Set of quirks that apply to specific versions of the IP.
>>> * @irq_flags: The flags to be passed to request_irq.
>>> * @irq: The irq value to be passed to request_irq.
>>> + * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
>>> *
>>> * Locking
>>> * =======
>>> @@ -191,6 +192,8 @@ struct dw_mci {
>>> bool vqmmc_enabled;
>>> unsigned long irq_flags; /* IRQ flags */
>>> int irq;
>>> +
>>> + int sdio_id0;
>>> };
>>>
>>> /* DMA ops for Internal/External DMAC interface */
>>>
>>
>>
>>
>>
>
>

2014-11-04 02:16:03

[permalink] [raw]

Subject: Re: [PATCH v3] mmc: dw_mmc: add support for the other bit of sdio interrupt

Dear Heiko.

On 11/03/2014 07:23 PM, Heiko St?bner wrote:
> Hi Jaehoon,
>
> Am Montag, 3. November 2014, 17:59:58 schrieb Jaehoon Chung:
>> Hi, Addy.
>>
>> On 11/03/2014 10:20 AM, Addy Ke wrote:
>>> The bit of sdio interrupt is 16 in designware implementation,
>>> but it is 24 on Rockchip SoCs.This patch add sdio_id0 for the
>>> number of slot0 in the SDIO interrupt registers.
>>>
>>> Signed-off-by: Addy Ke <[email protected]>
>>> ---
>>> Changes in v2:
>>> - rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next
>>> branch Changes in v3:
>>> - Remove dts for sdio_id0, just replace this with 8, suggested by Doug
>>> - Change to support all Rockchip Socs, suggested by Heiko
>>>
>>> drivers/mmc/host/dw_mmc-rockchip.c | 10 ++++++++++
>>> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
>>> drivers/mmc/host/dw_mmc.h | 2 ++
>>> include/linux/mmc/dw_mmc.h | 3 +++
>>> 4 files changed, 22 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c
>>> b/drivers/mmc/host/dw_mmc-rockchip.c index bbb4ec3..b997c8f 100644
>>> --- a/drivers/mmc/host/dw_mmc-rockchip.c
>>> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
>>> @@ -68,14 +68,24 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host,
>>> struct mmc_ios *ios)>
>>> }
>>>
>>> }
>>>
>>> +static int dw_mci_rockchip_parse_dt(struct dw_mci *host)
>>> +{
>>> + /* It is slot 8 on Rockchip SoCs */
>>> + host->sdio_id0 = 8;
>>> +
>>> + return 0;
>>> +}
>>
>> Well, function is "__parse_dt__", but this function don't parse anything.
>> If All rockchip soc is supported, i think that it can be located to other
>> place.
>
> do you have a suggestion for a location?
>
> The only alternative I can see right now would be using the init-hook in
> dw_mci_drv_data or adding a new field to it holding the slot-offset.
> [with using the init-hook being my personal preference of the two]

init-hook can be used, then, in future, it can also included other specific code for rock-chip.

Best Regards,
Jaehoon Chung
>
>
> Heiko
>

2014-11-04 14:03:38

[permalink] [raw]

Subject: [PATCH v4] mmc: dw_mmc: add support for the other bit of sdio interrupt

The bit of sdio interrupt is 16 in designware implementation,
but it is 24 on Rockchip SoCs.This patch add sdio_id0 for the
number of slot0 in the SDIO interrupt registers.

Signed-off-by: Addy Ke <[email protected]>
---
Changes in v2:
- rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next branch
Changes in v3:
- Remove dts for sdio_id0, just replace this with 8, suggested by Doug
- Change to support all Rockchip Socs, suggested by Heiko
Changes in v4:
- use init-hook to set sdio_id0, suggested by Jaehoon

drivers/mmc/host/dw_mmc-rockchip.c | 10 ++++++++++
drivers/mmc/host/dw_mmc.c | 12 +++++++-----
drivers/mmc/host/dw_mmc.h | 2 ++
include/linux/mmc/dw_mmc.h | 3 +++
4 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
index bbb4ec3..5650ac4 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -68,14 +68,24 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
}
}

+static int dw_mci_rockchip_init(struct dw_mci *host)
+{
+ /* It is slot 8 on Rockchip SoCs */
+ host->sdio_id0 = 8;
+
+ return 0;
+}
+
static const struct dw_mci_drv_data rk2928_drv_data = {
.prepare_command = dw_mci_rockchip_prepare_command,
+ .init = dw_mci_rockchip_init,
};

static const struct dw_mci_drv_data rk3288_drv_data = {
.prepare_command = dw_mci_rockchip_prepare_command,
.set_ios = dw_mci_rk3288_set_ios,
.setup_clock = dw_mci_rk3288_setup_clock,
+ .init = dw_mci_rockchip_init,
};

static const struct of_device_id dw_mci_rockchip_match[] = {
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index bb46b1b..a633b58 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -823,7 +823,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)

/* enable clock; only low power if no SDIO */
clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
- if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
+ if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
mci_writel(host, CLKENA, clk_en_a);

@@ -1184,10 +1184,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
dw_mci_disable_low_power(slot);

mci_writel(host, INTMASK,
- (int_mask | SDMMC_INT_SDIO(slot->id)));
+ (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
} else {
mci_writel(host, INTMASK,
- (int_mask & ~SDMMC_INT_SDIO(slot->id)));
+ (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
}
}

@@ -2056,8 +2056,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
/* Handle SDIO Interrupts */
for (i = 0; i < host->num_slots; i++) {
struct dw_mci_slot *slot = host->slot[i];
- if (pending & SDMMC_INT_SDIO(i)) {
- mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
+ if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
+ mci_writel(host, RINTSTS,
+ SDMMC_INT_SDIO(slot->sdio_id));
mmc_signal_sdio_irq(slot->mmc);
}
}
@@ -2145,6 +2146,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)

slot = mmc_priv(mmc);
slot->id = id;
+ slot->sdio_id = host->sdio_id0 + id;
slot->mmc = mmc;
slot->host = host;
host->slot[id] = slot;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 71d4995..0562f10 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
* with CONFIG_MMC_CLKGATE.
* @flags: Random state bits associated with the slot.
* @id: Number of this slot.
+ * @sdio_id: Number of this slot in the SDIO interrupt registers.
*/
struct dw_mci_slot {
struct mmc_host *mmc;
@@ -233,6 +234,7 @@ struct dw_mci_slot {
#define DW_MMC_CARD_PRESENT 0
#define DW_MMC_CARD_NEED_INIT 1
int id;
+ int sdio_id;
};

struct dw_mci_tuning_data {
diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
index 69d0814..72c319f 100644
--- a/include/linux/mmc/dw_mmc.h
+++ b/include/linux/mmc/dw_mmc.h
@@ -96,6 +96,7 @@ struct mmc_data;
* @quirks: Set of quirks that apply to specific versions of the IP.
* @irq_flags: The flags to be passed to request_irq.
* @irq: The irq value to be passed to request_irq.
+ * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
*
* Locking
* =======
@@ -191,6 +192,8 @@ struct dw_mci {
bool vqmmc_enabled;
unsigned long irq_flags; /* IRQ flags */
int irq;
+
+ int sdio_id0;
};

/* DMA ops for Internal/External DMAC interface */
--
1.8.3.2

2014-11-11 04:03:58

[permalink] [raw]

Subject: [PATCH] mmc: dw_mmc: try pick the exact same voltage as vmmc for vqmmc

SD2.0 cards need vqmmc and vmmc to be the same.
But vqmmc call regulator_set_voltage to set min_uv(2.7v) as far as possible.
So if we set vmmc 3.3V in dt table, we will get error information as follows:

[ 17.785398] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req
50000000Hz, actual 50000000HZ div = 0)
[ 17.795175] mmc1: new high speed SDHC card at address e624
[ 17.801283] mmcblk1: mmc1:e624 SU08G 7.40 GiB
[ 17.816033] mmcblk1: p1
[ 17.839318] mmcblk1: error -110 sending status command, retrying
[ 17.845363] mmcblk1: error -115 sending stop command, original cmd
response 0x900, card status 0x800b00
[ 17.854758] mmcblk1: error -84 transferring data, sector 32, nr 24,
cmd response 0x900, card status 0xb00
[ 17.864328] mmcblk1: retrying using single block read
[ 17.873647] mmcblk1: error -110 sending status command, retrying
[ 17.879660] mmcblk1: error -84 transferring data, sector 44, nr 12,
cmd response 0x900, card status 0x0
[ 17.889051] end_request: I/O error, dev mmcblk1, sector 44
[ 17.895594] Buffer I/O error on device mmcblk1, logical block 5
[ 17.902484] mmcblk1: error -110 sending status command, retrying
[ 17.908498] mmcblk1: error -84 transferring data, sector 50, nr 6,
cmd response 0x900, card status 0x0
[ 17.917802] end_request: I/O error, dev mmcblk1, sector 50
[ 17.924984] Buffer I/O error on device mmcblk1, logical block 6
[ 18.431258] mmc_host mmc1: Timeout sending command (cmd 0x200000 arg
0x0 status 0x80200000)

Signed-off-by: Addy Ke <[email protected]>
---
drivers/mmc/host/dw_mmc.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index b4c3044..a8b70b5 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1163,8 +1163,14 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
*/
uhs = mci_readl(host, UHS_REG);
if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
- min_uv = 2700000;
- max_uv = 3600000;
+ /* try pick the exact same voltage as vmmc for vqmmc */
+ if (!IS_ERR(mmc->supply.vmmc)) {
+ min_uv = regulator_get_voltage(mmc->supply.vmmc);
+ max_uv = min_uv;
+ } else {
+ min_uv = 2700000;
+ max_uv = 3600000;
+ }
uhs &= ~v18;
} else {
min_uv = 1700000;
--
1.8.3.2

2014-11-11 08:52:51

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: try pick the exact same voltage as vmmc for vqmmc

On 11 November 2014 05:02, Addy Ke <[email protected]> wrote:
> SD2.0 cards need vqmmc and vmmc to be the same.

No, that's not correct.

If I remember the spec correctly, the bus signal threshold is 0.75 * VDD.

> But vqmmc call regulator_set_voltage to set min_uv(2.7v) as far as possible.

I guess you want to do that to save as much power as possible.

> So if we set vmmc 3.3V in dt table, we will get error information as follows:
>
> [ 17.785398] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req
> 50000000Hz, actual 50000000HZ div = 0)
> [ 17.795175] mmc1: new high speed SDHC card at address e624
> [ 17.801283] mmcblk1: mmc1:e624 SU08G 7.40 GiB
> [ 17.816033] mmcblk1: p1
> [ 17.839318] mmcblk1: error -110 sending status command, retrying
> [ 17.845363] mmcblk1: error -115 sending stop command, original cmd
> response 0x900, card status 0x800b00
> [ 17.854758] mmcblk1: error -84 transferring data, sector 32, nr 24,
> cmd response 0x900, card status 0xb00
> [ 17.864328] mmcblk1: retrying using single block read
> [ 17.873647] mmcblk1: error -110 sending status command, retrying
> [ 17.879660] mmcblk1: error -84 transferring data, sector 44, nr 12,
> cmd response 0x900, card status 0x0
> [ 17.889051] end_request: I/O error, dev mmcblk1, sector 44
> [ 17.895594] Buffer I/O error on device mmcblk1, logical block 5
> [ 17.902484] mmcblk1: error -110 sending status command, retrying
> [ 17.908498] mmcblk1: error -84 transferring data, sector 50, nr 6,
> cmd response 0x900, card status 0x0
> [ 17.917802] end_request: I/O error, dev mmcblk1, sector 50
> [ 17.924984] Buffer I/O error on device mmcblk1, logical block 6
> [ 18.431258] mmc_host mmc1: Timeout sending command (cmd 0x200000 arg
> 0x0 status 0x80200000)
>
> Signed-off-by: Addy Ke <[email protected]>
> ---
> drivers/mmc/host/dw_mmc.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index b4c3044..a8b70b5 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1163,8 +1163,14 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
> */
> uhs = mci_readl(host, UHS_REG);
> if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
> - min_uv = 2700000;
> - max_uv = 3600000;
> + /* try pick the exact same voltage as vmmc for vqmmc */

This seems like a generic SD protocol issue.

Should we maybe provide some helper function from the mmc core, which
in principle take the negotiated card->ocr into account while
calculating the signal voltage level. Typically min_uv should be 0.75
x (card->ocr), for these cases.

Comments?

> + if (!IS_ERR(mmc->supply.vmmc)) {
> + min_uv = regulator_get_voltage(mmc->supply.vmmc);
> + max_uv = min_uv;
> + } else {
> + min_uv = 2700000;
> + max_uv = 3600000;
> + }
> uhs &= ~v18;
> } else {
> min_uv = 1700000;
> --
> 1.8.3.2
>
>

Kind regards
Uffe

2014-11-12 18:04:36

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: try pick the exact same voltage as vmmc for vqmmc

Ulf,

On Tue, Nov 11, 2014 at 12:52 AM, Ulf Hansson <[email protected]> wrote:
> On 11 November 2014 05:02, Addy Ke <[email protected]> wrote:
>> SD2.0 cards need vqmmc and vmmc to be the same.
>
> No, that's not correct.
>
> If I remember the spec correctly, the bus signal threshold is 0.75 * VDD.

As usual, I will first state my utter lack of knowledge of all things mmc.

Now that's out of the way, on two separate board with two separate
SoCs I've heard stories of cards that don't work when there's a big
gap between vmmc and vqmmc.

If my memory serves, previously I heard of problems with vmmc=3.3V and
vqmmc=2.8V. That means there were problems with .85 * VDD. Certainly
Addy seems to have a card that has problems with vmmc=3.3V and
vqmmc=2.7V (but worked with vmmc=3.3V and vqmmc=2.8V). That is .82 *
VDD.

I have no idea if these old cards are "to spec", but they exist and it
would be nice to support them.

It seems like the absolute safest thing would be to try to keep vmmc
and vqmmc matching if possible, especially during card probe. Once
voltage negotiation happened then the vqmmc could go down.

>> But vqmmc call regulator_set_voltage to set min_uv(2.7v) as far as possible.
>
> I guess you want to do that to save as much power as possible.

I don't think it's Addy wanting it, I think it's the regulator framework.

If a regulator is current 1.8V and you request 2.7 - 3.3V, the
framework needs to pick one of those voltages. I believe it will pick
2.7V.

...so I think we get into trouble only when the 2.0 card is plugged in
after a UHS card has negotiated down the voltage, but I could be
wrong. Maybe Addy can clarify.

>> @@ -1163,8 +1163,14 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>> */
>> uhs = mci_readl(host, UHS_REG);
>> if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
>> - min_uv = 2700000;
>> - max_uv = 3600000;
>> + /* try pick the exact same voltage as vmmc for vqmmc */
>
> This seems like a generic SD protocol issue.
>
> Should we maybe provide some helper function from the mmc core, which
> in principle take the negotiated card->ocr into account while
> calculating the signal voltage level. Typically min_uv should be 0.75
> x (card->ocr), for these cases.

Yes, if there are ways to make the solution more generic I would
certainly support that.

-Doug

2014-11-13 02:19:46

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: try pick the exact same voltage as vmmc for vqmmc

On 2014/11/13 02:04, Doug Anderson wrote:
> Ulf,
>
> On Tue, Nov 11, 2014 at 12:52 AM, Ulf Hansson <[email protected]> wrote:
>> On 11 November 2014 05:02, Addy Ke <[email protected]> wrote:
>>> SD2.0 cards need vqmmc and vmmc to be the same.
>>
>> No, that's not correct.
>>
>> If I remember the spec correctly, the bus signal threshold is 0.75 * VDD.
>
> As usual, I will first state my utter lack of knowledge of all things mmc.
>
> Now that's out of the way, on two separate board with two separate
> SoCs I've heard stories of cards that don't work when there's a big
> gap between vmmc and vqmmc.
>
> If my memory serves, previously I heard of problems with vmmc=3.3V and
> vqmmc=2.8V. That means there were problems with .85 * VDD. Certainly
> Addy seems to have a card that has problems with vmmc=3.3V and
> vqmmc=2.7V (but worked with vmmc=3.3V and vqmmc=2.8V). That is .82 *
> VDD.
>
> I have no idea if these old cards are "to spec", but they exist and it
> would be nice to support them.
>
> It seems like the absolute safest thing would be to try to keep vmmc
> and vqmmc matching if possible, especially during card probe. Once
> voltage negotiation happened then the vqmmc could go down.
>
>
>>> But vqmmc call regulator_set_voltage to set min_uv(2.7v) as far as possible.
>>
>> I guess you want to do that to save as much power as possible.
>
> I don't think it's Addy wanting it, I think it's the regulator framework.
>
> If a regulator is current 1.8V and you request 2.7 - 3.3V, the
> framework needs to pick one of those voltages. I believe it will pick
> 2.7V.
>
> ...so I think we get into trouble only when the 2.0 card is plugged in
> after a UHS card has negotiated down the voltage, but I could be
> wrong. Maybe Addy can clarify.
>
Sure
If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,
and card can be identified. But if UHS card is pulgged in first, the vqmmc will be down to 1.8v.

when sd2.0 card is pulgged in, mmc core will call dw_mci_switch_voltage to change vqmmc to 3.3v
(MMC_SINGLE_VOTAGE_330). So vqmmc will be set 2.7v, if we request 2.7-3.6v.

But vmmc is always 3.3v,becuase it be set min_volt = max_volt = 3.3v in dt tables.

So the result:
vmmc = 3.3v and vqmmc = 2.7v, and sd2.0 card is failed to identify in my test.

>
>>> @@ -1163,8 +1163,14 @@ static int dw_mci_switch_voltage(struct mmc_host *mmc, struct mmc_ios *ios)
>>> */
>>> uhs = mci_readl(host, UHS_REG);
>>> if (ios->signal_voltage == MMC_SIGNAL_VOLTAGE_330) {
>>> - min_uv = 2700000;
>>> - max_uv = 3600000;
>>> + /* try pick the exact same voltage as vmmc for vqmmc */
>>
>> This seems like a generic SD protocol issue.
>>
>> Should we maybe provide some helper function from the mmc core, which
>> in principle take the negotiated card->ocr into account while
>> calculating the signal voltage level. Typically min_uv should be 0.75
>> x (card->ocr), for these cases.
>
> Yes, if there are ways to make the solution more generic I would
> certainly support that.
>
> -Doug
>
>
>

2014-11-13 18:58:52

[permalink] [raw]

Subject: Re: [PATCH v4] mmc: dw_mmc: add support for the other bit of sdio interrupt

Addy,

On Tue, Nov 4, 2014 at 6:03 AM, Addy Ke <[email protected]> wrote:
> The bit of sdio interrupt is 16 in designware implementation,
> but it is 24 on Rockchip SoCs.This patch add sdio_id0 for the
> number of slot0 in the SDIO interrupt registers.
>
> Signed-off-by: Addy Ke <[email protected]>
> ---
> Changes in v2:
> - rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next branch
> Changes in v3:
> - Remove dts for sdio_id0, just replace this with 8, suggested by Doug
> - Change to support all Rockchip Socs, suggested by Heiko
> Changes in v4:
> - use init-hook to set sdio_id0, suggested by Jaehoon
>
> drivers/mmc/host/dw_mmc-rockchip.c | 10 ++++++++++
> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
> drivers/mmc/host/dw_mmc.h | 2 ++
> include/linux/mmc/dw_mmc.h | 3 +++
> 4 files changed, 22 insertions(+), 5 deletions(-)

Reviewed-by: Doug Anderson <[email protected]>

2014-11-14 13:26:00

[permalink] [raw]

Subject: Re: [PATCH v4] mmc: dw_mmc: add support for the other bit of sdio interrupt

On 11/14/2014 03:58 AM, Doug Anderson wrote:
> Addy,
>
> On Tue, Nov 4, 2014 at 6:03 AM, Addy Ke <[email protected]> wrote:
>> The bit of sdio interrupt is 16 in designware implementation,
>> but it is 24 on Rockchip SoCs.This patch add sdio_id0 for the
>> number of slot0 in the SDIO interrupt registers.
>>
>> Signed-off-by: Addy Ke <[email protected]>
>> ---
>> Changes in v2:
>> - rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next branch
>> Changes in v3:
>> - Remove dts for sdio_id0, just replace this with 8, suggested by Doug
>> - Change to support all Rockchip Socs, suggested by Heiko
>> Changes in v4:
>> - use init-hook to set sdio_id0, suggested by Jaehoon
>>
>> drivers/mmc/host/dw_mmc-rockchip.c | 10 ++++++++++
>> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
>> drivers/mmc/host/dw_mmc.h | 2 ++
>> include/linux/mmc/dw_mmc.h | 3 +++
>> 4 files changed, 22 insertions(+), 5 deletions(-)
>
> Reviewed-by: Doug Anderson <[email protected]>
>

Acked-by: Jaehoon Chung <[email protected]>

Best Regards,
Jaehoon Chung

2014-11-19 10:32:10

[permalink] [raw]

Subject: Re: [PATCH v4] mmc: dw_mmc: add support for the other bit of sdio interrupt

On 4 November 2014 15:03, Addy Ke <[email protected]> wrote:
> The bit of sdio interrupt is 16 in designware implementation,
> but it is 24 on Rockchip SoCs.This patch add sdio_id0 for the
> number of slot0 in the SDIO interrupt registers.
>
> Signed-off-by: Addy Ke <[email protected]>

Thanks! Applied for next.

Kind regards
Uffe

> ---
> Changes in v2:
> - rebase on http://git.linaro.org/git/people/ulf.hansson/mmc.git, next branch
> Changes in v3:
> - Remove dts for sdio_id0, just replace this with 8, suggested by Doug
> - Change to support all Rockchip Socs, suggested by Heiko
> Changes in v4:
> - use init-hook to set sdio_id0, suggested by Jaehoon
>
> drivers/mmc/host/dw_mmc-rockchip.c | 10 ++++++++++
> drivers/mmc/host/dw_mmc.c | 12 +++++++-----
> drivers/mmc/host/dw_mmc.h | 2 ++
> include/linux/mmc/dw_mmc.h | 3 +++
> 4 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
> index bbb4ec3..5650ac4 100644
> --- a/drivers/mmc/host/dw_mmc-rockchip.c
> +++ b/drivers/mmc/host/dw_mmc-rockchip.c
> @@ -68,14 +68,24 @@ static void dw_mci_rk3288_set_ios(struct dw_mci *host, struct mmc_ios *ios)
> }
> }
>
> +static int dw_mci_rockchip_init(struct dw_mci *host)
> +{
> + /* It is slot 8 on Rockchip SoCs */
> + host->sdio_id0 = 8;
> +
> + return 0;
> +}
> +
> static const struct dw_mci_drv_data rk2928_drv_data = {
> .prepare_command = dw_mci_rockchip_prepare_command,
> + .init = dw_mci_rockchip_init,
> };
>
> static const struct dw_mci_drv_data rk3288_drv_data = {
> .prepare_command = dw_mci_rockchip_prepare_command,
> .set_ios = dw_mci_rk3288_set_ios,
> .setup_clock = dw_mci_rk3288_setup_clock,
> + .init = dw_mci_rockchip_init,
> };
>
> static const struct of_device_id dw_mci_rockchip_match[] = {
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index bb46b1b..a633b58 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -823,7 +823,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>
> /* enable clock; only low power if no SDIO */
> clk_en_a = SDMMC_CLKEN_ENABLE << slot->id;
> - if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->id)))
> + if (!(mci_readl(host, INTMASK) & SDMMC_INT_SDIO(slot->sdio_id)))
> clk_en_a |= SDMMC_CLKEN_LOW_PWR << slot->id;
> mci_writel(host, CLKENA, clk_en_a);
>
> @@ -1184,10 +1184,10 @@ static void dw_mci_enable_sdio_irq(struct mmc_host *mmc, int enb)
> dw_mci_disable_low_power(slot);
>
> mci_writel(host, INTMASK,
> - (int_mask | SDMMC_INT_SDIO(slot->id)));
> + (int_mask | SDMMC_INT_SDIO(slot->sdio_id)));
> } else {
> mci_writel(host, INTMASK,
> - (int_mask & ~SDMMC_INT_SDIO(slot->id)));
> + (int_mask & ~SDMMC_INT_SDIO(slot->sdio_id)));
> }
> }
>
> @@ -2056,8 +2056,9 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
> /* Handle SDIO Interrupts */
> for (i = 0; i < host->num_slots; i++) {
> struct dw_mci_slot *slot = host->slot[i];
> - if (pending & SDMMC_INT_SDIO(i)) {
> - mci_writel(host, RINTSTS, SDMMC_INT_SDIO(i));
> + if (pending & SDMMC_INT_SDIO(slot->sdio_id)) {
> + mci_writel(host, RINTSTS,
> + SDMMC_INT_SDIO(slot->sdio_id));
> mmc_signal_sdio_irq(slot->mmc);
> }
> }
> @@ -2145,6 +2146,7 @@ static int dw_mci_init_slot(struct dw_mci *host, unsigned int id)
>
> slot = mmc_priv(mmc);
> slot->id = id;
> + slot->sdio_id = host->sdio_id0 + id;
> slot->mmc = mmc;
> slot->host = host;
> host->slot[id] = slot;
> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> index 71d4995..0562f10 100644
> --- a/drivers/mmc/host/dw_mmc.h
> +++ b/drivers/mmc/host/dw_mmc.h
> @@ -214,6 +214,7 @@ extern int dw_mci_resume(struct dw_mci *host);
> * with CONFIG_MMC_CLKGATE.
> * @flags: Random state bits associated with the slot.
> * @id: Number of this slot.
> + * @sdio_id: Number of this slot in the SDIO interrupt registers.
> */
> struct dw_mci_slot {
> struct mmc_host *mmc;
> @@ -233,6 +234,7 @@ struct dw_mci_slot {
> #define DW_MMC_CARD_PRESENT 0
> #define DW_MMC_CARD_NEED_INIT 1
> int id;
> + int sdio_id;
> };
>
> struct dw_mci_tuning_data {
> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
> index 69d0814..72c319f 100644
> --- a/include/linux/mmc/dw_mmc.h
> +++ b/include/linux/mmc/dw_mmc.h
> @@ -96,6 +96,7 @@ struct mmc_data;
> * @quirks: Set of quirks that apply to specific versions of the IP.
> * @irq_flags: The flags to be passed to request_irq.
> * @irq: The irq value to be passed to request_irq.
> + * @sdio_id0: Number of slot0 in the SDIO interrupt registers.
> *
> * Locking
> * =======
> @@ -191,6 +192,8 @@ struct dw_mci {
> bool vqmmc_enabled;
> unsigned long irq_flags; /* IRQ flags */
> int irq;
> +
> + int sdio_id0;
> };
>
> /* DMA ops for Internal/External DMAC interface */
> --
> 1.8.3.2
>
>

2014-11-21 12:06:21

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: try pick the exact same voltage as vmmc for vqmmc

[...]

> Sure
> If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,

That can't be right. mmc_power_up() should trigger
dw_mci_switch_voltage() to be invoked.

> and card can be identified. But if UHS card is pulgged in first, the vqmmc will be down to 1.8v.
>
> when sd2.0 card is pulgged in, mmc core will call dw_mci_switch_voltage to change vqmmc to 3.3v
> (MMC_SINGLE_VOTAGE_330). So vqmmc will be set 2.7v, if we request 2.7-3.6v.
>
> But vmmc is always 3.3v,becuase it be set min_volt = max_volt = 3.3v in dt tables.
>
> So the result:
> vmmc = 3.3v and vqmmc = 2.7v, and sd2.0 card is failed to identify in my test.

Hmm. I wonder why it works the first time? Could it be that your
regulator have voltage ramp up time that isn't properly considered?

Kind regards
Uffe

2014-11-21 12:30:07

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: try pick the exact same voltage as vmmc for vqmmc

On 11/21/2014 09:06 PM, Ulf Hansson wrote:
> [...]
>
>> Sure
>> If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,
>
> That can't be right. mmc_power_up() should trigger
> dw_mci_switch_voltage() to be invoked.

Since dw_mci_switch_voltage() is invoked, voltage is changed from 1.8v to 2.7v (minimum value 2.7-3.6v), isn't?

>
>> and card can be identified. But if UHS card is pulgged in first, the vqmmc will be down to 1.8v.
>>
>> when sd2.0 card is pulgged in, mmc core will call dw_mci_switch_voltage to change vqmmc to 3.3v
>> (MMC_SINGLE_VOTAGE_330). So vqmmc will be set 2.7v, if we request 2.7-3.6v.
>>
>> But vmmc is always 3.3v,becuase it be set min_volt = max_volt = 3.3v in dt tables.

vmmc is fixed voltage?

>>
>> So the result:
>> vmmc = 3.3v and vqmmc = 2.7v, and sd2.0 card is failed to identify in my test.
>
> Hmm. I wonder why it works the first time? Could it be that your
> regulator have voltage ramp up time that isn't properly considered?

if oscilloscope can use, can we know more exactly?

Best Regards,
Jaehoon Chung
>
> Kind regards
> Uffe
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2014-11-21 17:42:13

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: try pick the exact same voltage as vmmc for vqmmc

Ulf,

On Fri, Nov 21, 2014 at 4:06 AM, Ulf Hansson <[email protected]> wrote:
> [...]
>
>> Sure
>> If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,
>
> That can't be right. mmc_power_up() should trigger
> dw_mci_switch_voltage() to be invoked.

Hmmm, I think you're right. Addy: can you double check if it's only
the 2nd card for you? I was thinking that if a regulator is currently
3.3V and you request 2.7 - 3.3V the regulator framework will treat
that as a noop. ...but that definitely doesn't appear to be the case.
When I boot up the first time even with no SD card plugged in, I see
this at bootup:

[ 3.042234] vccio_sd: 1800 <--> 3300 mV at 3300 mV

...showing that it started at 3.3V. Then I see:

$ grep "" /sys/class/regulator/regulator.16/{name,microvolts}
/sys/class/regulator/regulator.16/name:vccio_sd
/sys/class/regulator/regulator.16/microvolts:2700000

...so it is certainly getting changed even with no card plugged in.

BTW: I don't actually have one of these failing cards--all of mine
work. Addy, do you know the make and model of the card you have that
fails?

-Doug

2014-11-21 21:04:09

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: try pick the exact same voltage as vmmc for vqmmc

Hi,

On Fri, Nov 21, 2014 at 9:42 AM, Doug Anderson <[email protected]> wrote:
> Ulf,
>
> On Fri, Nov 21, 2014 at 4:06 AM, Ulf Hansson <[email protected]> wrote:
>> [...]
>>
>>> Sure
>>> If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,
>>
>> That can't be right. mmc_power_up() should trigger
>> dw_mci_switch_voltage() to be invoked.
>
> Hmmm, I think you're right. Addy: can you double check if it's only
> the 2nd card for you? I was thinking that if a regulator is currently
> 3.3V and you request 2.7 - 3.3V the regulator framework will treat
> that as a noop. ...but that definitely doesn't appear to be the case.
> When I boot up the first time even with no SD card plugged in, I see
> this at bootup:
>
> [ 3.042234] vccio_sd: 1800 <--> 3300 mV at 3300 mV
>
> ...showing that it started at 3.3V. Then I see:
>
> $ grep "" /sys/class/regulator/regulator.16/{name,microvolts}
> /sys/class/regulator/regulator.16/name:vccio_sd
> /sys/class/regulator/regulator.16/microvolts:2700000
>
> ...so it is certainly getting changed even with no card plugged in.
>
>
> BTW: I don't actually have one of these failing cards--all of mine
> work. Addy, do you know the make and model of the card you have that
> fails?

Just as a bit of a followup, I did some more digging...

1. It looks as if we now have a bit of "opposite" logic for vmmc vs.
vqmmc. In mmc_power_up() I see that it sets the initial voltage as:

host->ios.vdd = fls(ocr) - 1;

That actually means that we're going to pick the maximum voltage for
vmmc (of the supported voltages). For vqmmc dw_mmc is using the
regulator framework which (as described in my previous message) will
pick the minimum.

2. Several people I've talked to have expressed concerns that our
minimum value is 2.7V. Apparently that's really on the edge and makes
EEs a little nervous. The quick sample of cards sitting on my desk
shows that they seem to claim 0x00ff8000, which doesn't include 2.7V.

Both of the above make me feel like dw_mmc should try its best to pick
a value for vqmmc that is closest to the value of vmmc (and >= 2.7V).
That also happens to make us work exactly like hosts where vmmc and
vqmmc are supplied by the same supply.

-Doug

2014-11-24 13:29:34

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: try pick the exact same voltage as vmmc for vqmmc

On 21 November 2014 at 22:04, Doug Anderson <[email protected]> wrote:
> Hi,
>
> On Fri, Nov 21, 2014 at 9:42 AM, Doug Anderson <[email protected]> wrote:
>> Ulf,
>>
>> On Fri, Nov 21, 2014 at 4:06 AM, Ulf Hansson <[email protected]> wrote:
>>> [...]
>>>
>>>> Sure
>>>> If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,
>>>
>>> That can't be right. mmc_power_up() should trigger
>>> dw_mci_switch_voltage() to be invoked.
>>
>> Hmmm, I think you're right. Addy: can you double check if it's only
>> the 2nd card for you? I was thinking that if a regulator is currently
>> 3.3V and you request 2.7 - 3.3V the regulator framework will treat
>> that as a noop. ...but that definitely doesn't appear to be the case.
>> When I boot up the first time even with no SD card plugged in, I see
>> this at bootup:
>>
>> [ 3.042234] vccio_sd: 1800 <--> 3300 mV at 3300 mV
>>
>> ...showing that it started at 3.3V. Then I see:
>>
>> $ grep "" /sys/class/regulator/regulator.16/{name,microvolts}
>> /sys/class/regulator/regulator.16/name:vccio_sd
>> /sys/class/regulator/regulator.16/microvolts:2700000
>>
>> ...so it is certainly getting changed even with no card plugged in.
>>
>>
>> BTW: I don't actually have one of these failing cards--all of mine
>> work. Addy, do you know the make and model of the card you have that
>> fails?
>
> Just as a bit of a followup, I did some more digging...
>
> 1. It looks as if we now have a bit of "opposite" logic for vmmc vs.
> vqmmc. In mmc_power_up() I see that it sets the initial voltage as:
>
> host->ios.vdd = fls(ocr) - 1;

That's because we would like to supports as many cards as possible.
The policy is based upon that some cards may not support lower
voltages, but most will support higher.

>
> That actually means that we're going to pick the maximum voltage for
> vmmc (of the supported voltages). For vqmmc dw_mmc is using the
> regulator framework which (as described in my previous message) will
> pick the minimum.

Correct. I have thought this has been inside spec and choosing the
lower value would be preferred to lower power consumption. Maybe we
needs to re-visit this one more time.

Here are some of the interesting sections in the eMMC spec:
10.3.3 Power supply Voltages
"The VCCQ must be defined at equal to or less than VCC".

10.5 Bus signal levels
Push-pull mode:
Voh = 0.75 * VCCQ. (Do note, its VCCQ not VCC).

Summary eMMC: VCCQ must be less and VCC, we should be inside spec.

>From SD spec:
6.6.1 Threshold Level for High Voltage Range
Voh = 0.75 * VDD.

In worst case scenario, VDD = 3.6V and VIO = 2.7V. That gives as the
factor of 0.75, thus we are inside spec but without margins.

>
> 2. Several people I've talked to have expressed concerns that our
> minimum value is 2.7V. Apparently that's really on the edge and makes
> EEs a little nervous. The quick sample of cards sitting on my desk
> shows that they seem to claim 0x00ff8000, which doesn't include 2.7V.

0x00ff8000 states what values of VDD levels the device supports. Not VIO.

>
>
> Both of the above make me feel like dw_mmc should try its best to pick
> a value for vqmmc that is closest to the value of vmmc (and >= 2.7V).
> That also happens to make us work exactly like hosts where vmmc and
> vqmmc are supplied by the same supply.

I do see your point. And I agree that it would be nice to achieve
something like this.

The question is how to do this. For sure, we need to involve the mmc
core to handle this correctly.

Kind regards
Uffe

2014-11-25 02:38:43

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: try pick the exact same voltage as vmmc for vqmmc

On Fri, Nov 24, 2014 at 9:29 PM, Ulf Hansson <[email protected]>
wrote:

> On 21 November 2014 at 22:04, Doug Anderson <[email protected]> wrote:
>> Hi,
>>
>> On Fri, Nov 21, 2014 at 9:42 AM, Doug Anderson <[email protected]> wrote:
>>> Ulf,
>>>
>>> On Fri, Nov 21, 2014 at 4:06 AM, Ulf Hansson <[email protected]> wrote:
>>>> [...]
>>>>
>>>>> Sure
>>>>> If the first card is sd2.0 since startup, dw_mci_switch_voltage will not be called,
>>>> That can't be right. mmc_power_up() should trigger
>>>> dw_mci_switch_voltage() to be invoked.
>>> Hmmm, I think you're right. Addy: can you double check if it's only
>>> the 2nd card for you? I was thinking that if a regulator is currently
>>> 3.3V and you request 2.7 - 3.3V the regulator framework will treat
>>> that as a noop. ...but that definitely doesn't appear to be the case.
>>> When I boot up the first time even with no SD card plugged in, I see
>>> this at bootup:
>>>
>>> [ 3.042234] vccio_sd: 1800 <--> 3300 mV at 3300 mV
>>>
>>> ...showing that it started at 3.3V. Then I see:
>>>
>>> $ grep "" /sys/class/regulator/regulator.16/{name,microvolts}
>>> /sys/class/regulator/regulator.16/name:vccio_sd
>>> /sys/class/regulator/regulator.16/microvolts:2700000
>>>
>>> ...so it is certainly getting changed even with no card plugged in.
>>>
>>>
>>> BTW: I don't actually have one of these failing cards--all of mine
>>> work. Addy, do you know the make and model of the card you have that
>>> fails?
>> Just as a bit of a followup, I did some more digging...
>>
>> 1. It looks as if we now have a bit of "opposite" logic for vmmc vs.
>> vqmmc. In mmc_power_up() I see that it sets the initial voltage as:
>>
>> host->ios.vdd = fls(ocr) - 1;
> That's because we would like to supports as many cards as possible.
> The policy is based upon that some cards may not support lower
> voltages, but most will support higher.
>
>> That actually means that we're going to pick the maximum voltage for
>> vmmc (of the supported voltages). For vqmmc dw_mmc is using the
>> regulator framework which (as described in my previous message) will
>> pick the minimum.
> Correct. I have thought this has been inside spec and choosing the
> lower value would be preferred to lower power consumption. Maybe we
> needs to re-visit this one more time.
>
> Here are some of the interesting sections in the eMMC spec:
> 10.3.3 Power supply Voltages
> "The VCCQ must be defined at equal to or less than VCC".
>
> 10.5 Bus signal levels
> Push-pull mode:
> Voh = 0.75 * VCCQ. (Do note, its VCCQ not VCC).
>
> Summary eMMC: VCCQ must be less and VCC, we should be inside spec.
>
> >From SD spec:
> 6.6.1 Threshold Level for High Voltage Range
> Voh = 0.75 * VDD.
>
> In worst case scenario, VDD = 3.6V and VIO = 2.7V. That gives as the
> factor of 0.75, thus we are inside spec but without margins.
* From eMMC4.5 spec:
1. (VDDF)vcc: Supply voltage for flash memory, which is 2.7v -- 3.3v
2. (VDD)vccq: Supply voltage for memory controller, which is 1.7v --
1.95v and 2,7v -- 3.6v

* And from RK3288 datasheet:
Digtial GPIO Power(SDMMC0_VDD --> vccq) is 3.0v -- 3.6v and 1.62v - 1.98v

So I think:
3.3v: (2.7v < vccq < 3.6v) && (3.0v < vccq < 3.6v) ==> (3.0v < vccq
< 3.6v)
1.8v: (1.7v < vccq < 1.95v) && (1.62v < vccq < 1.98v) ==> (1.7v < vccq
< 1.95v)

and (2.7v < vcc < 3.3v)

* And according to our hardware engineer:
All of supply voltage must have +/- 10% cushion.

* And we have found in some worse card that there is 200mv voltage
collapse when these card is insert.

So I think the best resolution is that vcc and vccq is configurable int
dt table.

>> 2. Several people I've talked to have expressed concerns that our
>> minimum value is 2.7V. Apparently that's really on the edge and makes
>> EEs a little nervous. The quick sample of cards sitting on my desk
>> shows that they seem to claim 0x00ff8000, which doesn't include 2.7V.
> 0x00ff8000 states what values of VDD levels the device supports. Not VIO.
>
>>
>> Both of the above make me feel like dw_mmc should try its best to pick
>> a value for vqmmc that is closest to the value of vmmc (and >= 2.7V).
>> That also happens to make us work exactly like hosts where vmmc and
>> vqmmc are supplied by the same supply.
> I do see your point. And I agree that it would be nice to achieve
> something like this.
>
> The question is how to do this. For sure, we need to involve the mmc
> core to handle this correctly.
>
> Kind regards
> Uffe
>
>
>

2014-11-25 05:29:33

[permalink] [raw]

Subject: Re: [PATCH] mmc: dw_mmc: try pick the exact same voltage as vmmc for vqmmc

Ulf,

On Mon, Nov 24, 2014 at 5:29 AM, Ulf Hansson <[email protected]> wrote:
>> 2. Several people I've talked to have expressed concerns that our
>> minimum value is 2.7V. Apparently that's really on the edge and makes
>> EEs a little nervous. The quick sample of cards sitting on my desk
>> shows that they seem to claim 0x00ff8000, which doesn't include 2.7V.
>
> 0x00ff8000 states what values of VDD levels the device supports. Not VIO.

Yup, I was just pointing out that possibly others were trying to get a
little bit of margin (not going all the way down to 2.7V) too.

>> Both of the above make me feel like dw_mmc should try its best to pick
>> a value for vqmmc that is closest to the value of vmmc (and >= 2.7V).
>> That also happens to make us work exactly like hosts where vmmc and
>> vqmmc are supplied by the same supply.
>
> I do see your point. And I agree that it would be nice to achieve
> something like this.
>
> The question is how to do this. For sure, we need to involve the mmc
> core to handle this correctly.

You could add a function to the core that we could call from
dw_mci_switch_voltage() and it would do all the work except trying to
set the UHS register. That would certainly make it easy. That could
try to set the highest voltage that is <= vmmc when we're at
MMC_SIGNAL_VOLTAGE_330.

-Doug

2014-11-25 05:36:27