diff mbox

mmc: dw_mmc: add a quirk for the defferent bit of sdio interrupt

Message ID 1414635679-12565-1-git-send-email-addy.ke@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

addy ke Oct. 30, 2014, 2:21 a.m. UTC 
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 <addy.ke@rock-chips.com>
---
 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(-)

Comments

Jaehoon Chung Oct. 30, 2014, 4:35 a.m. UTC | #1 
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 <addy.ke@rock-chips.com>
> ---
>  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 */
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Anderson Oct. 30, 2014, 4:41 a.m. UTC | #2 
Addy,

On Wed, Oct 29, 2014 at 7:21 PM, Addy Ke <addy.ke@rock-chips.com> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Anderson Oct. 30, 2014, 4:49 a.m. UTC | #3 
Addy,

On Wed, Oct 29, 2014 at 9:41 PM, Doug Anderson <dianders@chromium.org> wrote:
> You can avoid a lot of "if" tests if you just add a new "sdio->id"

Whoops, I mean "slot->sdio_id"
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
addy ke Oct. 30, 2014, 6:54 a.m. UTC | #4 
Hi, Doug,

On 2014/10/30 12:49, Doug Anderson wrote:
> Addy,
> 
> On Wed, Oct 29, 2014 at 9:41 PM, Doug Anderson <dianders@chromium.org> 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!

> 
> 

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

Patch

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)
+		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 */