[PATCH 1/6] mci: print current clock in info

[PATCH 1/6] mci: print current clock in info

[Sascha Hauer]

When printing the cards information the actual clock is much
more interesting than what the card could support.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 416168f..283df2f 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1395,7 +1395,7 @@ static void mci_info(struct device_d *dev)
 		mci->cid[2], mci->cid[3]);
 	printf("   CSD: %08X-%08X-%08X-%08X\n", mci->csd[0], mci->csd[1],
 		mci->csd[2], mci->csd[3]);
-	printf("  Max. transfer speed: %u Hz\n", mci->tran_speed);
+	printf("  clock: %u Hz\n", mci->host->clock);
 	printf("  Manufacturer ID: %02X\n", extract_mid(mci));
 	printf("  OEM/Application ID: %04X\n", extract_oid(mci));
 	printf("  Product name: '%c%c%c%c%c'\n", mci->cid[0] & 0xff,
-- 
1.8.2.rc2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/6] mci: do not limit clock to card capabilities

[Sascha Hauer]

No need to limit the clock to the cards capabilities since the values
passed to mci_set_clock are based on the cards capabilities. This
enables MMC high speed cards to operate at higher speeds since on
this cards the csd field only holds the non highspeed maximum clock.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci-core.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 283df2f..895108f 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -664,10 +664,6 @@ static void mci_set_clock(struct mci *mci, unsigned clock)
 	if (clock < host->f_min)
 		clock = host->f_min;
 
-	/* check against the limit at the card's side */
-	if (mci->tran_speed != 0 && clock > mci->tran_speed)
-		clock = mci->tran_speed;
-
 	host->clock = clock;	/* the new target frequency */
 	mci_set_ios(mci);
 }
-- 
1.8.2.rc2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/6] mci: rename trans_speed field to csd_max_dtr

[Sascha Hauer]

tran_speed is confusing since the name doesn't tell us if this
is the maximum speed, the current speed or whatever else. Rename
it to csd_max_dtr since this variable holds the maximum data transfer
rate we extracted from csd.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci-core.c | 4 ++--
 include/mci.h          | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 895108f..d7b8fc9 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -779,8 +779,8 @@ static void mci_extract_max_tran_speed_from_csd(struct mci *mci)
 		return;
 	}
 
-	mci->tran_speed = time * unit;
-	dev_dbg(&mci->dev, "Transfer speed: %u\n", mci->tran_speed);
+	mci->csd_max_dtr = time * unit;
+	dev_dbg(&mci->dev, "Transfer speed: %u\n", mci->csd_max_dtr);
 }
 
 /**
diff --git a/include/mci.h b/include/mci.h
index 1eb967d..2b87691 100644
--- a/include/mci.h
+++ b/include/mci.h
@@ -340,7 +340,7 @@ struct mci {
 	unsigned csd[4];	/**< card's "card specific data register" */
 	unsigned cid[4];	/**< card's "card identification register" */
 	unsigned short rca;	/* FIXME */
-	unsigned tran_speed;	/**< not yet used */
+	unsigned csd_max_dtr;	/**< max data transfer rate as decoded from csd */
 	/** currently used data block length for read accesses */
 	unsigned read_bl_len;
 	/** currently used data block length for write accesses */
-- 
1.8.2.rc2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/6] mci: Use csd_max_dtr when appropriate

[Sascha Hauer]

The csd_max_dtr tells us at which speed a card is able to operate,
so use it to adjust the cards clock where appropriate.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci-core.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index d7b8fc9..54660dc 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -919,7 +919,7 @@ static int mci_startup_sd(struct mci *mci)
 	if (mci->card_caps & MMC_MODE_HS)
 		mci_set_clock(mci, 50000000);
 	else
-		mci_set_clock(mci, 25000000);
+		mci_set_clock(mci, mci->csd_max_dtr);
 
 	return 0;
 }
@@ -929,6 +929,7 @@ static int mci_startup_mmc(struct mci *mci)
 	struct mci_host *host = mci->host;
 	int err;
 	int idx = 0;
+	int max_dtr;
 	static unsigned ext_csd_bits[] = {
 		EXT_CSD_BUS_WIDTH_4,
 		EXT_CSD_BUS_WIDTH_8,
@@ -941,13 +942,15 @@ static int mci_startup_mmc(struct mci *mci)
 	/* if possible, speed up the transfer */
 	if (mci->card_caps & MMC_MODE_HS) {
 		if (mci->card_caps & MMC_MODE_HS_52MHz)
-			mci_set_clock(mci, 52000000);
+			max_dtr = 52000000;
 		else
-			mci_set_clock(mci, 26000000);
+			max_dtr = 26000000;
 	} else {
-		mci_set_clock(mci, 20000000);
+		max_dtr = mci->csd_max_dtr;
 	}
 
+	mci_set_clock(mci, max_dtr);
+
 	/*
 	 * Unlike SD, MMC cards dont have a configuration register to notify
 	 * supported bus width. So bus test command should be run to identify
-- 
1.8.2.rc2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/6] mci: Add devicetree helper function

[Sascha Hauer]

This adds helper code to parse the bus-width and max-frequency
property from devicetree.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci-core.c | 38 ++++++++++++++++++++++++++++++++++++++
 include/mci.h          |  1 +
 2 files changed, 39 insertions(+)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 54660dc..5209d1c 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1649,3 +1649,41 @@ err_free:
 
 	return ret;
 }
+
+void mci_of_parse(struct mci_host *host)
+{
+	struct device_node *np;
+	u32 bus_width;
+
+	if (!IS_ENABLED(CONFIG_OFDEVICE))
+		return;
+
+	if (!host->hw_dev || !host->hw_dev->device_node)
+		return;
+
+	np = host->hw_dev->device_node;
+
+	/* "bus-width" is translated to MMC_CAP_*_BIT_DATA flags */
+	if (of_property_read_u32(np, "bus-width", &bus_width) < 0) {
+		dev_dbg(host->hw_dev,
+			"\"bus-width\" property is missing, assuming 1 bit.\n");
+		bus_width = 1;
+	}
+
+	switch (bus_width) {
+	case 8:
+		host->host_caps |= MMC_MODE_8BIT;
+		/* Hosts capable of 8-bit transfers can also do 4 bits */
+	case 4:
+		host->host_caps |= MMC_MODE_4BIT;
+		break;
+	case 1:
+		break;
+	default:
+		dev_err(host->hw_dev,
+			"Invalid \"bus-width\" value %ud!\n", bus_width);
+	}
+
+	/* f_max is obtained from the optional "max-frequency" property */
+	of_property_read_u32(np, "max-frequency", &host->f_max);
+}
diff --git a/include/mci.h b/include/mci.h
index 2b87691..70df641 100644
--- a/include/mci.h
+++ b/include/mci.h
@@ -362,5 +362,6 @@ struct mci {
 };
 
 int mci_register(struct mci_host*);
+void mci_of_parse(struct mci_host *host);
 
 #endif /* _MCI_H_ */
-- 
1.8.2.rc2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 6/6] mci: i.MX esdhc: get additional parameters from devicetree

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/imx-esdhc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mci/imx-esdhc.c b/drivers/mci/imx-esdhc.c
index 1e3b307..b1686d3 100644
--- a/drivers/mci/imx-esdhc.c
+++ b/drivers/mci/imx-esdhc.c
@@ -558,6 +558,8 @@ static int fsl_esdhc_probe(struct device_d *dev)
 		host->mci.f_min = 200000;
 	host->mci.f_max = rate;
 
+	mci_of_parse(&host->mci);
+
 	mci_register(&host->mci);
 
 	return 0;
-- 
1.8.2.rc2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/6] mci: do not limit clock to card capabilities

[Juergen Beisert]

Hi Sascha,

Sascha Hauer wrote:
> No need to limit the clock to the cards capabilities since the values
> passed to mci_set_clock are based on the cards capabilities. This
> enables MMC high speed cards to operate at higher speeds since on
> this cards the csd field only holds the non highspeed maximum clock.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mci/mci-core.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index 283df2f..895108f 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -664,10 +664,6 @@ static void mci_set_clock(struct mci *mci, unsigned clock)
>       if (clock < host->f_min) 
>  		clock = host->f_min;
>
> -	/* check against the limit at the card's side */
> -	if (mci->tran_speed != 0 && clock > mci->tran_speed)
> -		clock = mci->tran_speed;
> -
>  	host->clock = clock;	/* the new target frequency */
>  	mci_set_ios(mci);
>  }

I'm not sure. In mci_startup_sd() there is a very primitive decision for the
clock frequency.

	/* if possible, speed up the transfer */
	if (mci->card_caps & MMC_MODE_HS)
		mci_set_clock(mci, 50000000);
	else
		mci_set_clock(mci, 25000000);

And yes, the spec tells us, there are two cards at 3.3 V available: 25 MHz and
50 MHz. But theoretically a vendor can limit the max transfer speed to a
different value. I'm confused about the spec here, as the card reports a max
transfer speed *and* a standard/high speed bit. Which one is reliable?

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/6] mci: print current clock in info

[Juergen Beisert]

Sascha Hauer wrote:
> When printing the cards information the actual clock is much
> more interesting than what the card could support.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mci/mci-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index 416168f..283df2f 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -1395,7 +1395,7 @@ static void mci_info(struct device_d *dev)
>  		mci->cid[2], mci->cid[3]);
>  	printf("   CSD: %08X-%08X-%08X-%08X\n", mci->csd[0], mci->csd[1],
>  		mci->csd[2], mci->csd[3]);
> -	printf("  Max. transfer speed: %u Hz\n", mci->tran_speed);
> +	printf("  clock: %u Hz\n", mci->host->clock);
>  	printf("  Manufacturer ID: %02X\n", extract_mid(mci));
>  	printf("  OEM/Application ID: %04X\n", extract_oid(mci));
>  	printf("  Product name: '%c%c%c%c%c'\n", mci->cid[0] & 0xff,

But here you ask for the card itself. If you want to know the currently *used* 
transfer speed you must ask the interface.

Please keep this information available, as it helps others to get some 
information about the used card and might help to debug.

jbe
-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/6] mci: rename trans_speed field to csd_max_dtr

[Juergen Beisert]

Sascha Hauer wrote:
> tran_speed is confusing since the name doesn't tell us if this
> is the maximum speed, the current speed or whatever else. Rename
> it to csd_max_dtr since this variable holds the maximum data transfer
> rate we extracted from csd.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mci/mci-core.c | 4 ++--
>  include/mci.h          | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index 895108f..d7b8fc9 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -779,8 +779,8 @@ static void mci_extract_max_tran_speed_from_csd(struct mci *mci)
> 		return; 
>  	}
>
> -	mci->tran_speed = time * unit;
> -	dev_dbg(&mci->dev, "Transfer speed: %u\n", mci->tran_speed);
> +	mci->csd_max_dtr = time * unit;
> +	dev_dbg(&mci->dev, "Transfer speed: %u\n", mci->csd_max_dtr);
>  }
>
>  /**
> diff --git a/include/mci.h b/include/mci.h
> index 1eb967d..2b87691 100644
> --- a/include/mci.h
> +++ b/include/mci.h
> @@ -340,7 +340,7 @@ struct mci {
>  	unsigned csd[4];	/**< card's "card specific data register" */
>  	unsigned cid[4];	/**< card's "card identification register" */
>  	unsigned short rca;	/* FIXME */
> -	unsigned tran_speed;	/**< not yet used */
> +	unsigned csd_max_dtr;	/**< max data transfer rate as decoded from csd */
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

\o/ \o/ \o/ ;)

>  	/** currently used data block length for read accesses */
>  	unsigned read_bl_len;
>  	/** currently used data block length for write accesses */

Acked-by: Juergen Beisert <jbe@pengutronix.de>

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 4/6] mci: Use csd_max_dtr when appropriate

[Juergen Beisert]

Sascha Hauer wrote:
> The csd_max_dtr tells us at which speed a card is able to operate,
> so use it to adjust the cards clock where appropriate.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mci/mci-core.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index d7b8fc9..54660dc 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -919,7 +919,7 @@ static int mci_startup_sd(struct mci *mci)
>  	if (mci->card_caps & MMC_MODE_HS)
>  		mci_set_clock(mci, 50000000);
>  	else
> -		mci_set_clock(mci, 25000000);
> +		mci_set_clock(mci, mci->csd_max_dtr);
>
>  	return 0;
>  }

This one needs some discussion. See my other answer to your "PATCH 2/6".

Some SD card experts here?

jbe


-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox