diff mbox

[v2,3/5] spi-nor: Make spi_nor_scan() take a chip type name, not an spi_device_id

Message ID 1412043317.9388.84.camel@decadent.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Hutchings Sept. 30, 2014, 2:15 a.m. UTC
Drivers currently call spi_nor_match_id() and then spi_nor_scan().
This adds a dependency on struct spi_device_id which we want to
avoid.  Make spi_nor_scan() do it for them.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 drivers/mtd/devices/m25p80.c      | 13 +++++--------
 drivers/mtd/spi-nor/fsl-quadspi.c |  7 +------
 drivers/mtd/spi-nor/spi-nor.c     | 13 +++++++++----
 include/linux/mtd/spi-nor.h       | 20 ++------------------
 4 files changed, 17 insertions(+), 36 deletions(-)

Comments

Rafał Miłecki Sept. 30, 2014, 5:14 a.m. UTC | #1
On 30 September 2014 04:15, Ben Hutchings <ben@decadent.org.uk> wrote:
> @@ -236,16 +236,13 @@ static int m25p_probe(struct spi_device *spi)
>          * If that's the case, respect "type" and ignore a "name".
>          */
>         if (data && data->type)
> -               id = spi_nor_match_id(data->type);
> +               name = data->type;
>
>         /* If we didn't get name from platform, simply use "modalias". */
> -       if (!id) {
> -               id = spi_nor_match_id(spi_get_device_id(spi)->name);
> -               if (WARN_ON(!id))
> -                       return -ENODEV;
> -       }
> +       if (!name)
> +               name = spi_get_device_id(spi)->name;

Huh? Iterating the whole id_table, checking the entries (looking for
one with name equal to the spi->modalias) and then... getting that
name?

Did it hurt to use the patch I've sent
mtd: m25p80: get rid of spi_get_device_id
https://patchwork.ozlabs.org/patch/394328/
Ben Hutchings Sept. 30, 2014, 11:38 a.m. UTC | #2
On Tue, 2014-09-30 at 07:14 +0200, Rafa? Mi?ecki wrote:
> On 30 September 2014 04:15, Ben Hutchings <ben@decadent.org.uk> wrote:
> > @@ -236,16 +236,13 @@ static int m25p_probe(struct spi_device *spi)
> >          * If that's the case, respect "type" and ignore a "name".
> >          */
> >         if (data && data->type)
> > -               id = spi_nor_match_id(data->type);
> > +               name = data->type;
> >
> >         /* If we didn't get name from platform, simply use "modalias". */
> > -       if (!id) {
> > -               id = spi_nor_match_id(spi_get_device_id(spi)->name);
> > -               if (WARN_ON(!id))
> > -                       return -ENODEV;
> > -       }
> > +       if (!name)
> > +               name = spi_get_device_id(spi)->name;
> 
> Huh? Iterating the whole id_table, checking the entries (looking for
> one with name equal to the spi->modalias) and then... getting that
> name?

Ah, I didn't realise what spi_get_device_id() was doing.

> Did it hurt to use the patch I've sent
> mtd: m25p80: get rid of spi_get_device_id
> https://patchwork.ozlabs.org/patch/394328/

It would make sense to insert that before this.

Ben.
diff mbox

Patch

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 204bec1..d9578b9 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -193,7 +193,7 @@  static int m25p_probe(struct spi_device *spi)
 {
 	struct mtd_part_parser_data	ppdata;
 	struct flash_platform_data	*data;
-	const struct spi_device_id *id = NULL;
+	const char *name = NULL;
 	struct m25p *flash;
 	struct spi_nor *nor;
 	enum read_mode mode = SPI_NOR_NORMAL;
@@ -236,16 +236,13 @@  static int m25p_probe(struct spi_device *spi)
 	 * If that's the case, respect "type" and ignore a "name".
 	 */
 	if (data && data->type)
-		id = spi_nor_match_id(data->type);
+		name = data->type;
 
 	/* If we didn't get name from platform, simply use "modalias". */
-	if (!id) {
-		id = spi_nor_match_id(spi_get_device_id(spi)->name);
-		if (WARN_ON(!id))
-			return -ENODEV;
-	}
+	if (!name)
+		name = spi_get_device_id(spi)->name;
 
-	ret = spi_nor_scan(nor, id, mode);
+	ret = spi_nor_scan(nor, name, mode);
 	if (ret)
 		return ret;
 
diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 8d659a2..d5269a2 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -881,7 +881,6 @@  static int fsl_qspi_probe(struct platform_device *pdev)
 
 	/* iterate the subnodes. */
 	for_each_available_child_of_node(dev->of_node, np) {
-		const struct spi_device_id *id;
 		char modalias[40];
 
 		/* skip the holes */
@@ -909,10 +908,6 @@  static int fsl_qspi_probe(struct platform_device *pdev)
 		if (of_modalias_node(np, modalias, sizeof(modalias)) < 0)
 			goto map_failed;
 
-		id = spi_nor_match_id(modalias);
-		if (!id)
-			goto map_failed;
-
 		ret = of_property_read_u32(np, "spi-max-frequency",
 				&q->clk_rate);
 		if (ret < 0)
@@ -921,7 +916,7 @@  static int fsl_qspi_probe(struct platform_device *pdev)
 		/* set the chip address for READID */
 		fsl_qspi_set_base_addr(q, nor);
 
-		ret = spi_nor_scan(nor, id, SPI_NOR_QUAD);
+		ret = spi_nor_scan(nor, modalias, SPI_NOR_QUAD);
 		if (ret)
 			goto map_failed;
 
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index c2f0573..171c665 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -28,6 +28,8 @@ 
 
 #define JEDEC_MFR(_jedec_id)	((_jedec_id) >> 16)
 
+static const struct spi_device_id *spi_nor_match_id(const char *name);
+
 /*
  * Read the status register, returning its value in the location
  * Return the status register value.
@@ -908,9 +910,9 @@  static int spi_nor_check(struct spi_nor *nor)
 	return 0;
 }
 
-int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
-			enum read_mode mode)
+int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 {
+	const struct spi_device_id	*id = NULL;
 	struct flash_info		*info;
 	struct device *dev = nor->dev;
 	struct mtd_info *mtd = nor->mtd;
@@ -922,6 +924,10 @@  int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 	if (ret)
 		return ret;
 
+	id = spi_nor_match_id(name);
+	if (!id)
+		return -ENODEV;
+
 	info = (void *)id->driver_data;
 
 	if (info->jedec_id) {
@@ -1110,7 +1116,7 @@  int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 }
 EXPORT_SYMBOL_GPL(spi_nor_scan);
 
-const struct spi_device_id *spi_nor_match_id(const char *name)
+static const struct spi_device_id *spi_nor_match_id(const char *name)
 {
 	const struct spi_device_id *id = spi_nor_ids;
 
@@ -1121,7 +1127,6 @@  const struct spi_device_id *spi_nor_match_id(const char *name)
 	}
 	return NULL;
 }
-EXPORT_SYMBOL_GPL(spi_nor_match_id);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Huang Shijie <shijie8@gmail.com>");
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 66af67a..c48ad49 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -184,31 +184,15 @@  struct spi_nor {
 /**
  * spi_nor_scan() - scan the SPI NOR
  * @nor:	the spi_nor structure
- * @id:		the spi_device_id provided by the driver
+ * @name:	the chip type name
  * @mode:	the read mode supported by the driver
  *
  * The drivers can use this fuction to scan the SPI NOR.
  * In the scanning, it will try to get all the necessary information to
  * fill the mtd_info{} and the spi_nor{}.
  *
- * The board may assigns a spi_device_id with @id which be used to compared with
- * the spi_device_id detected by the scanning.
- *
  * Return: 0 for success, others for failure.
  */
-int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
-			enum read_mode mode);
-
-/**
- * spi_nor_match_id() - find the spi_device_id by the name
- * @name:	the name of the spi_device_id
- *
- * The drivers use this function to find the spi_device_id
- * specified by the @name.
- *
- * Return: returns the right spi_device_id pointer on success,
- *         and returns NULL on failure.
- */
-const struct spi_device_id *spi_nor_match_id(const char *name);
+int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode);
 
 #endif