mci patches

mci patches

[Sascha Hauer]

The following patches reduce the string footprint of the mci core.
Also, There is no need for guessing disk sizes on !X86, so remove
the code for non X86 architectures.

Sascha Hauer (3):
      mci: shrink string footprint
      mci: turn several pr_* statements into debug
      ata: guessing disk sizes is only needed on X86

 drivers/ata/disk_drive.c |    5 ++++-
 drivers/mci/mci-core.c   |   40 +++++++++++++++++++++-------------------
 2 files changed, 25 insertions(+), 20 deletions(-)


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

[PATCH 1/3] mci: shrink string footprint

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mci/mci-core.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index a3ad809..a51c44b 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -599,6 +599,7 @@ static void mci_detect_version_from_csd(struct device_d *mci_dev)
 {
 	struct mci *mci = GET_MCI_DATA(mci_dev);
 	int version;
+	char *vstr;
 
 	if (mci->version == MMC_VERSION_UNKNOWN) {
 		/* the version is coded in the bits 127:126 (left aligned) */
@@ -606,30 +607,31 @@ static void mci_detect_version_from_csd(struct device_d *mci_dev)
 
 		switch (version) {
 		case 0:
-			printf("Detecting a 1.2 revision card\n");
+			vstr = "1.2";
 			mci->version = MMC_VERSION_1_2;
 			break;
 		case 1:
-			printf("Detecting a 1.4 revision card\n");
+			vstr = "1.4";
 			mci->version = MMC_VERSION_1_4;
 			break;
 		case 2:
-			printf("Detecting a 2.2 revision card\n");
+			vstr = "2.2";
 			mci->version = MMC_VERSION_2_2;
 			break;
 		case 3:
-			printf("Detecting a 3.0 revision card\n");
+			vstr = "3.0";
 			mci->version = MMC_VERSION_3;
 			break;
 		case 4:
-			printf("Detecting a 4.0 revision card\n");
+			vstr = "4.0";
 			mci->version = MMC_VERSION_4;
 			break;
 		default:
-			printf("Unknow revision. Falling back to a 1.2 revision card\n");
+			vstr = "unknown, fallback to 1.2";
 			mci->version = MMC_VERSION_1_2;
 			break;
 		}
+		printf("detected card version %s\n", vstr);
 	}
 }
 
-- 
1.7.2.3


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

[PATCH 2/3] mci: turn several pr_* statements into debug

[Sascha Hauer]

These are mostly useful for developers, so turn them off by default.

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

diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index a51c44b..c4c5c2f 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -684,7 +684,7 @@ static void mci_extract_max_tran_speed_from_csd(struct device_d *mci_dev)
 	unit = tran_speed_unit[(mci->csd[0] & 0x7)];
 	time = tran_speed_time[((mci->csd[0] >> 3) & 0xf)];
 	if ((unit == 0) || (time == 0)) {
-		pr_warning("Unsupported 'TRAN_SPEED' unit/time value."
+		pr_debug("Unsupported 'TRAN_SPEED' unit/time value."
 				" Can't calculate card's max. transfer speed\n");
 		return;
 	}
@@ -802,13 +802,13 @@ static int mci_startup(struct device_d *mci_dev)
 	/* sanitiy? */
 	if (mci->read_bl_len > 512) {
 		mci->read_bl_len = 512;
-		pr_warning("Limiting max. read block size down to %u\n",
+		pr_debug("Limiting max. read block size down to %u\n",
 				mci->read_bl_len);
 	}
 
 	if (mci->write_bl_len > 512) {
 		mci->write_bl_len = 512;
-		pr_warning("Limiting max. write block size down to %u\n",
+		pr_debug("Limiting max. write block size down to %u\n",
 				mci->read_bl_len);
 	}
 	pr_debug("Read block length: %u, Write block length: %u\n",
@@ -961,7 +961,7 @@ static int mci_sd_write(struct device_d *disk_dev, uint64_t sector_start,
 		__func__, sector_count, (unsigned)sector_start);
 
 	if (mci->write_bl_len != 512) {
-		pr_warning("MMC/SD block size is not 512 bytes (its %u bytes instead)\n",
+		pr_debug("MMC/SD block size is not 512 bytes (its %u bytes instead)\n",
 				mci->read_bl_len);
 		return -EINVAL;
 	}
@@ -969,13 +969,13 @@ static int mci_sd_write(struct device_d *disk_dev, uint64_t sector_start,
 	while (sector_count) {
 		/* size of the block number field in the MMC/SD command is 32 bit only */
 		if (sector_start > MAX_BUFFER_NUMBER) {
-			pr_err("Cannot handle block number %llu. Too large!\n",
+			pr_debug("Cannot handle block number %llu. Too large!\n",
 				sector_start);
 			return -EINVAL;
 		}
 		rc = mci_block_write(mci_dev, buffer, sector_start);
 		if (rc != 0) {
-			pr_err("Writing block %u failed with %d\n", (unsigned)sector_start, rc);
+			pr_debug("Writing block %u failed with %d\n", (unsigned)sector_start, rc);
 			return rc;
 		}
 		sector_count--;
@@ -1009,7 +1009,7 @@ static int mci_sd_read(struct device_d *disk_dev, uint64_t sector_start,
 		__func__, sector_count, (unsigned)sector_start);
 
 	if (mci->read_bl_len != 512) {
-		pr_warning("MMC/SD block size is not 512 bytes (its %u bytes instead)\n",
+		pr_debug("MMC/SD block size is not 512 bytes (its %u bytes instead)\n",
 				mci->read_bl_len);
 		return -EINVAL;
 	}
@@ -1023,7 +1023,7 @@ static int mci_sd_read(struct device_d *disk_dev, uint64_t sector_start,
 		}
 		rc = mci_read_block(mci_dev, buffer, (unsigned)sector_start, now);
 		if (rc != 0) {
-			pr_err("Reading block %u failed with %d\n", (unsigned)sector_start, rc);
+			pr_debug("Reading block %u failed with %d\n", (unsigned)sector_start, rc);
 			return rc;
 		}
 		sector_count -= now;
@@ -1182,7 +1182,7 @@ static int mci_card_probe(struct device_d *mci_dev)
 	/* start with a host interface reset */
 	rc = (host->init)(host, mci_dev);
 	if (rc) {
-		pr_err("Cannot reset the SD/MMC interface\n");
+		pr_debug("Cannot reset the SD/MMC interface\n");
 		return rc;
 	}
 
@@ -1192,7 +1192,7 @@ static int mci_card_probe(struct device_d *mci_dev)
 	/* reset the card */
 	rc = mci_go_idle(mci_dev);
 	if (rc) {
-		pr_warning("Cannot reset the SD/MMC card\n");
+		pr_debug("Cannot reset the SD/MMC card\n");
 		goto on_error;
 	}
 
@@ -1210,7 +1210,7 @@ static int mci_card_probe(struct device_d *mci_dev)
 
 	rc = mci_startup(mci_dev);
 	if (rc) {
-		printf("Card's startup fails with %d\n", rc);
+		pr_debug("Card's startup fails with %d\n", rc);
 		goto on_error;
 	}
 
@@ -1321,7 +1321,7 @@ static int mci_probe(struct device_d *mci_dev)
 		 */
 		rc = add_mci_parameter(mci_dev);
 		if (rc != 0) {
-			pr_err("Failed to add 'probe' parameter to the MCI device\n");
+			pr_debug("Failed to add 'probe' parameter to the MCI device\n");
 			goto on_error;
 		}
 	}
@@ -1331,7 +1331,7 @@ static int mci_probe(struct device_d *mci_dev)
 	/* add params on demand */
 	rc = add_mci_parameter(mci_dev);
 	if (rc != 0) {
-		pr_err("Failed to add 'probe' parameter to the MCI device\n");
+		pr_debug("Failed to add 'probe' parameter to the MCI device\n");
 		goto on_error;
 	}
 #endif
-- 
1.7.2.3


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

[PATCH 3/3] ata: guessing disk sizes is only needed on X86

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/ata/disk_drive.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/ata/disk_drive.c b/drivers/ata/disk_drive.c
index f526b1e..d17fbcb 100644
--- a/drivers/ata/disk_drive.c
+++ b/drivers/ata/disk_drive.c
@@ -60,6 +60,7 @@ struct partition_entry {
  * @param table partition table
  * @return size in sectors
  */
+#ifdef CONFIG_X86
 static unsigned long disk_guess_size(struct device_d *dev, struct partition_entry *table)
 {
 	int part_order[4] = {0, 1, 2, 3};
@@ -83,6 +84,7 @@ static unsigned long disk_guess_size(struct device_d *dev, struct partition_entr
 #endif
 	return size;
 }
+#endif
 
 /**
  * Register partitions found on the drive
@@ -186,6 +188,7 @@ static int disk_probe(struct device_d *dev)
 #endif
 		atablk->blk.cdev.name = asprintf("disk%d", dev->id);
 
+#ifdef CONFIG_X86
 	/* On x86, BIOS based disks are coming without a valid .size field */
 	if (dev->size == 0) {
 		/* guess the size of this drive if not otherwise given */
@@ -193,7 +196,7 @@ static int disk_probe(struct device_d *dev)
 			(struct partition_entry*)&sector[446]) * SECTOR_SIZE;
 		dev_info(dev, "Drive size guessed to %u kiB\n", dev->size / 1024);
 	}
-
+#endif
 	atablk->blk.num_blocks = dev->size / SECTOR_SIZE;
 	atablk->blk.ops = &ataops;
 	atablk->blk.blockbits = 9;
-- 
1.7.2.3


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

Re: [PATCH 2/3] mci: turn several pr_* statements into debug

[Juergen Beisert]

Mostly ACK, but ...

Sascha Hauer wrote:
> These are mostly useful for developers, so turn them off by default.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mci/mci-core.c |   26 +++++++++++++-------------
>  1 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index a51c44b..c4c5c2f 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
>
> [...]
>
> @@ -1182,7 +1182,7 @@ static int mci_card_probe(struct device_d *mci_dev)
>  	/* start with a host interface reset */
>  	rc = (host->init)(host, mci_dev);
>  	if (rc) {
> -		pr_err("Cannot reset the SD/MMC interface\n");
> +		pr_debug("Cannot reset the SD/MMC interface\n");
>  		return rc;
>  	}

This is not only useful for the developer. Also for the regular user when
something is broken at runtime.

> @@ -1192,7 +1192,7 @@ static int mci_card_probe(struct device_d *mci_dev)
>  	/* reset the card */
>  	rc = mci_go_idle(mci_dev);
>  	if (rc) {
> -		pr_warning("Cannot reset the SD/MMC card\n");
> +		pr_debug("Cannot reset the SD/MMC card\n");
>  		goto on_error;
>  	}

Also useful for the regular user, that tries his shiny new and fresh SD card
and something went wrong at runtime.

>
> [...]
>

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-8766-939 228     |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

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

Re: [PATCH 3/3] ata: guessing disk sizes is only needed on X86

[Juergen Beisert]

Sascha Hauer wrote:
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/ata/disk_drive.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/ata/disk_drive.c b/drivers/ata/disk_drive.c
> index f526b1e..d17fbcb 100644
> --- a/drivers/ata/disk_drive.c
> +++ b/drivers/ata/disk_drive.c
> @@ -60,6 +60,7 @@ struct partition_entry {
>   * @param table partition table
>   * @return size in sectors
>   */
> +#ifdef CONFIG_X86
>  static unsigned long disk_guess_size(struct device_d *dev, struct partition_entry *table) {
>  	int part_order[4] = {0, 1, 2, 3};
> @@ -83,6 +84,7 @@ static unsigned long disk_guess_size(struct device_d *dev, struct partition_entr
> #endif 
>  	return size;
>  }
> +#endif
>
>  /**
>   * Register partitions found on the drive
> @@ -186,6 +188,7 @@ static int disk_probe(struct device_d *dev)
>  #endif
>  		atablk->blk.cdev.name = asprintf("disk%d", dev->id);
>
> +#ifdef CONFIG_X86
>  	/* On x86, BIOS based disks are coming without a valid .size field */
>  	if (dev->size == 0) {
>  		/* guess the size of this drive if not otherwise given */
> @@ -193,7 +196,7 @@ static int disk_probe(struct device_d *dev)
>  			(struct partition_entry*)&sector[446]) * SECTOR_SIZE;
>  		dev_info(dev, "Drive size guessed to %u kiB\n", dev->size / 1024);
>  	}
> -
> +#endif
>  	atablk->blk.num_blocks = dev->size / SECTOR_SIZE;
>  	atablk->blk.ops = &ataops;
>  	atablk->blk.blockbits = 9;

Hmm, as there are other architectures with ATA interfaces out than x86, I
think we need a different solution, when to guessing the size and when not to
do so. In the current x86 case it depends on the ATA interface type: The BIOS
driver cannot read the disk size. But a native ATA driver (or SD card driver)
can. So, we should exclude this code on the symbol "CONFIG_ATA_BIOS" instead
of CONFIG_X86.

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-8766-939 228     |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

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

Re: [PATCH 2/3] mci: turn several pr_* statements into debug

[Sascha Hauer]

On Tue, Apr 12, 2011 at 10:57:11AM +0200, Juergen Beisert wrote:
> Mostly ACK, but ...
> 
> Sascha Hauer wrote:
> > These are mostly useful for developers, so turn them off by default.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/mci/mci-core.c |   26 +++++++++++++-------------
> >  1 files changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> > index a51c44b..c4c5c2f 100644
> > --- a/drivers/mci/mci-core.c
> > +++ b/drivers/mci/mci-core.c
> >
> > [...]
> >
> > @@ -1182,7 +1182,7 @@ static int mci_card_probe(struct device_d *mci_dev)
> >  	/* start with a host interface reset */
> >  	rc = (host->init)(host, mci_dev);
> >  	if (rc) {
> > -		pr_err("Cannot reset the SD/MMC interface\n");
> > +		pr_debug("Cannot reset the SD/MMC interface\n");
> >  		return rc;
> >  	}

Really this shouldn't fail for users. Most of the driver can't fail
or print something themselves.

> 
> This is not only useful for the developer. Also for the regular user when
> something is broken at runtime.
> 
> > @@ -1192,7 +1192,7 @@ static int mci_card_probe(struct device_d *mci_dev)
> >  	/* reset the card */
> >  	rc = mci_go_idle(mci_dev);
> >  	if (rc) {
> > -		pr_warning("Cannot reset the SD/MMC card\n");
> > +		pr_debug("Cannot reset the SD/MMC card\n");
> >  		goto on_error;
> >  	}
> 
> Also useful for the regular user, that tries his shiny new and fresh SD card
> and something went wrong at runtime.

Ok for this one as it depends on the card inserted.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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