Skip to contentSkip to conversation

sdhci-s3c: add platform_8bit_width()

PATCH In review Jaehoon Chung 3 participants 6 messages Jan 12, 2011

Subsystems (3)
lore
Share & tools
lore

Jaehoon Chung

author maintainer
#0 lore raw

PATCH 1/1 sdhci-s3c: add platform_8bit_width()

This patch is added platform_8bit_width() function in sdhci-s3c.

We has 8-bit width support but is not a v3 controller. So we need platform_8bit_width() to support 8-bit buswidth.

Also we need MMC_CAP_8_BIT_DATA, so we added them in platdata.

---
 drivers/mmc/host/sdhci-s3c.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)
drivers/mmc/host/sdhci-s3c.c +32 -0
index 70cc3d7..4dc7d6d 100644
@@ -276,10 +276,39 @@ static void sdhci_cmu_set_clock(struct sdhci_host *host, unsigned int clock)
 	host->clock = clock;
 }
 
+/*
+ * sdhci_s3c_platform_8bit_width - support 8bit buswidth
+ *
+ * We has 8-bit width support but is not a v3 controller.
+ * So we add  platform_8bit_width() and support 8bit width.
+ **/
+static int sdhci_s3c_platform_8bit_width(struct sdhci_host *host,
+		int bus_width)
+{
+	u8 ctrl;
+
+	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+
+	if (bus_width == MMC_BUS_WIDTH_8)
+		ctrl |= SDHCI_CTRL_8BITBUS;
+	else
+		ctrl &= ~SDHCI_CTRL_8BITBUS;
+
+	if (bus_width == MMC_BUS_WIDTH_4)
+		ctrl |= SDHCI_CTRL_4BITBUS;
+	else
+		ctrl &= ~SDHCI_CTRL_4BITBUS;
+
+	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+
+	return 0;
+}
+
 static struct sdhci_ops sdhci_s3c_ops = {
 	.get_max_clock		= sdhci_s3c_get_max_clk,
 	.set_clock		= sdhci_s3c_set_clock,
 	.get_min_clock		= sdhci_s3c_get_min_clock,
+	.platform_8bit_width	= sdhci_s3c_platform_8bit_width,
 };
 
 static void sdhci_s3c_notify_change(struct platform_device *dev, int state)
@@ -472,6 +501,9 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	if (pdata->cd_type == S3C_SDHCI_CD_PERMANENT)
 		host->mmc->caps = MMC_CAP_NONREMOVABLE;
 
+	if (pdata->host_caps)
+		host->mmc->caps |= pdata->host_caps;
+
 	host->quirks |= (SDHCI_QUIRK_32BIT_DMA_ADDR |
 			 SDHCI_QUIRK_32BIT_DMA_SIZE);
 
Signature · 1 line
1.6.0.4
#1 lore raw

Hi Jaehoon,

Looks fine, just style corrections:

Quoted 1 line · Jaehoon Chung

This patch is added platform_8bit_width() function in sdhci-s3c.

^ adds

Quoted 2 lines

We has 8-bit width support but is not a v3 controller.

^ have

Quoted 3 lines

So we need platform_8bit_width() to support 8-bit buswidth.

Also we need MMC_CAP_8_BIT_DATA, so we added them in platdata.

^ add it

Quoted 12 lines
---
 drivers/mmc/host/sdhci-s3c.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)
drivers/mmc/host/sdhci-s3c.c +0 -0
index 70cc3d7..4dc7d6d 100644

drivers/mmc/host/sdhci-s3c.c static void sdhci_cmu_set_clock(struct sdhci_host *host, unsigned int clock) · line 282

Quoted 7 lines
drivers/mmc/host/sdhci-s3c.c +4 -0
@@ -276,10 +276,39 @@ static void sdhci_cmu_set_clock(struct sdhci_host *host, unsigned int clock)
 	host->clock = clock;
 }
 
+/*
+ * sdhci_s3c_platform_8bit_width - support 8bit buswidth
+ *
+ * We has 8-bit width support but is not a v3 controller.

^ have

drivers/mmc/host/sdhci-s3c.c static void sdhci_cmu_set_clock(struct sdhci_host *host, unsigned int clock) · line 283

Quoted 1 line
drivers/mmc/host/sdhci-s3c.c +1 -0
+ * So we add  platform_8bit_width() and support 8bit width.

^ extra space here

drivers/mmc/host/sdhci-s3c.c static void sdhci_cmu_set_clock(struct sdhci_host *host, unsigned int clock) · line 286

Quoted 4 lines
drivers/mmc/host/sdhci-s3c.c +3 -0
+ **/
 
+static int sdhci_s3c_platform_8bit_width(struct sdhci_host *host,
+		int bus_width)

This will (just) fit on one line.

Quoted 4 lines
drivers/mmc/host/sdhci-s3c.c +4 -0
+{
+	u8 ctrl;
+
+	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);

drivers/mmc/host/sdhci-s3c.c static void sdhci_cmu_set_clock(struct sdhci_host *host, unsigned int clock) · line 300

Quoted 10 lines
drivers/mmc/host/sdhci-s3c.c +10 -0
+
+	if (bus_width == MMC_BUS_WIDTH_8)
+		ctrl |= SDHCI_CTRL_8BITBUS;
+	else
+		ctrl &= ~SDHCI_CTRL_8BITBUS;
+
+	if (bus_width == MMC_BUS_WIDTH_4)
+		ctrl |= SDHCI_CTRL_4BITBUS;
+	else
+		ctrl &= ~SDHCI_CTRL_4BITBUS;

You could use a switch statement here.

Quoted 23 lines
drivers/mmc/host/sdhci-s3c.c +7 -0
+
+	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+
+	return 0;
+}
+
 static struct sdhci_ops sdhci_s3c_ops = {
 	.get_max_clock		= sdhci_s3c_get_max_clk,
 	.set_clock		= sdhci_s3c_set_clock,
 	.get_min_clock		= sdhci_s3c_get_min_clock,
+	.platform_8bit_width	= sdhci_s3c_platform_8bit_width,
 };
 

static void sdhci_s3c_notify_change(struct platform_device *dev, int state)

drivers/mmc/host/sdhci-s3c.c +3 -0
@@ -472,6 +501,9 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	if (pdata->cd_type == S3C_SDHCI_CD_PERMANENT)
 		host->mmc->caps = MMC_CAP_NONREMOVABLE;
 
+	if (pdata->host_caps)
+		host->mmc->caps |= pdata->host_caps;
+
 	host->quirks |= (SDHCI_QUIRK_32BIT_DMA_ADDR |
 			 SDHCI_QUIRK_32BIT_DMA_SIZE);
Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
author maintainer
#2 lore raw

Hi Chris.

Thanks for your comments.
So i fixed them, and resend the patch.

We have 8-bit width support bu is not a v3 controller. So we need platform_8bit_width() to support 8-bit buswidth. Also we need MMC_CAP_8_BIT_DATA, so we add it in platdata.

---
 drivers/mmc/host/sdhci-s3c.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)
drivers/mmc/host/sdhci-s3c.c +34 -0
index 70cc3d7..8d27bbd 100644
@@ -276,10 +276,41 @@ static void sdhci_cmu_set_clock(struct sdhci_host *host, unsigned int clock)
 	host->clock = clock;
 }
 
+/*
+ * sdhci_s3c_platform_8bit_width - support 8bit buswidth
+ *
+ * We have 8-bit width support but is not a v3 controller.
+ * So we add platform_8bit_width() and support 8bit width.
+ **/
+static int sdhci_s3c_platform_8bit_width(struct sdhci_host *host, int width)
+{
+	u8 ctrl;
+
+	ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
+
+	switch (width) {
+	case MMC_BUS_WIDTH_8:
+		ctrl |= SDHCI_CTRL_8BITBUS;
+		ctrl &= ~SDHCI_CTRL_4BITBUS;
+		break;
+	case MMC_BUS_WIDTH_4:
+		ctrl |= SDHCI_CTRL_4BITBUS;
+		ctrl &= ~SDHCI_CTRL_8BITBUS;
+		break;
+	default:
+		break;
+	}
+
+	sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
+
+	return 0;
+}
+
 static struct sdhci_ops sdhci_s3c_ops = {
 	.get_max_clock		= sdhci_s3c_get_max_clk,
 	.set_clock		= sdhci_s3c_set_clock,
 	.get_min_clock		= sdhci_s3c_get_min_clock,
+	.platform_8bit_width	= sdhci_s3c_platform_8bit_width,
 };
 
 static void sdhci_s3c_notify_change(struct platform_device *dev, int state)
@@ -472,6 +503,9 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev)
 	if (pdata->cd_type == S3C_SDHCI_CD_PERMANENT)
 		host->mmc->caps = MMC_CAP_NONREMOVABLE;
 
+	if (pdata->host_caps)
+		host->mmc->caps |= pdata->host_caps;
+
 	host->quirks |= (SDHCI_QUIRK_32BIT_DMA_ADDR |
 			 SDHCI_QUIRK_32BIT_DMA_SIZE);
 
Signature · 1 line
1.6.0.4
#3 lore raw

Hi Jaehoon,

Quoted 4 lines · Jaehoon Chung

Hi Chris.

Thanks for your comments.
So i fixed them, and resend the patch.

Thanks! I've pushed it to mmc-next.

Quoted 5 lines

We have 8-bit width support bu is not a v3 controller. So we need platform_8bit_width() to support 8-bit buswidth. Also we need MMC_CAP_8_BIT_DATA, so we add it in platdata.

I made one more change -- the rest of sdhci-s3c.c uses kerneldoc format function comments, but your patch doesn't have the correct kerneldoc syntax:

drivers/mmc/host/sdhci-s3c.c +4 -2
index 89f019f..5309ab9 100644
@@ -277,12 +277,14 @@ static void sdhci_cmu_set_clock(struct sdhci_host *host, unsigned int clock)
 	host->clock = clock;
 }
 
-/*
+/**
  * sdhci_s3c_platform_8bit_width - support 8bit buswidth
+ * @host: The SDHCI host being queried
+ * @width: MMC_BUS_WIDTH_ macro for the bus width being requested
  *
  * We have 8-bit width support but is not a v3 controller.
  * So we add platform_8bit_width() and support 8bit width.
- **/
+ */
 static int sdhci_s3c_platform_8bit_width(struct sdhci_host *host, int width)
 {
 	u8 ctrl;

Ben, feel free to yell if you object to the final version of the patch, else I'll merge it for .39.

Thanks,

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
#4 lore raw
Quoted 42 lines · Chris Ball

Hi Jaehoon,

Quoted 4 lines · Jaehoon Chung

Hi Chris.

Thanks for your comments.
So i fixed them, and resend the patch.

Thanks!  I've pushed it to mmc-next.

Quoted 5 lines

We have 8-bit width support bu is not a v3 controller. So we need platform_8bit_width() to support 8-bit buswidth. Also we need MMC_CAP_8_BIT_DATA, so we add it in platdata.

I made one more change -- the rest of sdhci-s3c.c uses kerneldoc format function comments, but your patch doesn't have the correct kerneldoc syntax:

drivers/mmc/host/sdhci-s3c.c +4 -2
index 89f019f..5309ab9 100644
@@ -277,12 +277,14 @@ static void sdhci_cmu_set_clock(struct sdhci_host *host, unsigned int clock)
        host->clock = clock;
  }
 
-/*
+/**
  * sdhci_s3c_platform_8bit_width - support 8bit buswidth
+ * @host: The SDHCI host being queried
+ * @width: MMC_BUS_WIDTH_ macro for the bus width being requested
  *
  * We have 8-bit width support but is not a v3 controller.
  * So we add platform_8bit_width() and support 8bit width.
- **/
+ */
  static int sdhci_s3c_platform_8bit_width(struct sdhci_host *host, int width)
  {
        u8 ctrl;
Quoted 2 lines

Ben, feel free to yell if you object to the final version of the patch, else I'll merge it for .39.

Hi Chris,

Umm I think it's kind of bug fix since we used 8-bit buswidth previous code but recent change don't make it working
So re-support the 8-bit buswidth. Can you merge it at 38.rc period?

How do you think?

Thank you,
Kyungmin Park

#5 lore raw
Quoted 3 lines · Kyungmin Park

Umm I think it's kind of bug fix since we used 8-bit buswidth previous code but recent change don't make it working
So re-support the 8-bit buswidth. Can you merge it at 38.rc period?

Okay, that should be fine. As always, though, it would be nice to see an ACK from Ben before pushing it through quickly.

Thanks,

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child