[1/5] mtd: spi: Drop redundent SPI flash driver
diff mbox series

Message ID 20200709111709.68904-2-jagan@amarulasolutions.com
State New
Headers show
Series
  • mtd: Implement MTD UCLASS (use SPINOR)
Related show

Commit Message

Jagan Teki July 9, 2020, 11:17 a.m. UTC 
UCLASS_SPI_FLASH driver at driver/mtd/spi is a generic
spi flash driver to probe jedec,spi-nor flash chips.

Technically a probe call in U_BOOT_DRIVER is local to that
driver and not applicable to use it another driver or in
another code.

The apollolake SPL code using the generic probe by adding
extra SPI flash driver, which make more confusion in terms
of code readability and driver model structure.

The fact that apollolake SPL requires a separate SPI flash
driver to handle of-platdata via bind call, so move the
bind call in the generic flash driver and drop the driver
from apollolake code.

I hope this wouldn't break generic code usage flash chips
otherwise, we can handle this via driver data or a separate
spi driver in drivers/mtd/spi.

Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Vignesh R <vigneshr@ti.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/x86/cpu/apollolake/spl.c | 60 -----------------------------------
 drivers/mtd/spi/sf-uclass.c   | 29 ++++++++++++++++-
 include/spi_flash.h           | 12 -------
 3 files changed, 28 insertions(+), 73 deletions(-)

Comments

'Krzysztof Kozlowski' via Amarula Linux July 13, 2020, 8:32 a.m. UTC | #1 
Hi Jagan,

$subject: s/redundent/redundant

On 09/07/20 4:47 pm, Jagan Teki wrote:

> ---
[...]
> index b09046fec3..44cdb3151d 100644
> --- a/drivers/mtd/spi/sf-uclass.c
> +++ b/drivers/mtd/spi/sf-uclass.c
> @@ -122,7 +122,7 @@ static int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len)
>  	return mtd->_erase(mtd, &instr);
>  }
>  
> -int spi_flash_std_probe(struct udevice *dev)
> +static int spi_flash_std_probe(struct udevice *dev)
>  {
>  	struct spi_slave *slave = dev_get_parent_priv(dev);
>  	struct spi_flash *flash;
> @@ -164,6 +164,32 @@ static int spi_flash_std_remove(struct udevice *dev)
>  	return 0;
>  }
>  
> +/*
> + * Manually set the parent of the SPI flash to SPI, since dtoc doesn't. We also
> + * need to allocate the parent_platdata since by the time this function is
> + * called device_bind() has already gone past that step.
> + */
> +static int spi_flash_std_bind(struct udevice *dev)
> +{
> +	if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
> +		struct dm_spi_slave_platdata *plat;
> +		struct udevice *spi;
> +		int ret;
> +
> +		ret = uclass_first_device_err(UCLASS_SPI, &spi);
> +		if (ret)
> +			return ret;
> +		dev->parent = spi;

Hmm... How does this association work in case of multiple flash devices
in the system on multiple SPI buses? Seems like fixup works only for
first SPI bus?

Regards
Vignesh

> +
> +		plat = calloc(sizeof(*plat), 1);
> +		if (!plat)
> +			return -ENOMEM;
> +		dev->parent_platdata = plat;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct dm_spi_flash_ops spi_flash_std_ops = {
>  	.read = spi_flash_std_read,
>  	.write = spi_flash_std_write,
> @@ -180,6 +206,7 @@ U_BOOT_DRIVER(spi_flash_std) = {
>  	.id		= UCLASS_SPI_FLASH,
>  	.of_match	= spi_flash_std_ids,
>  	.probe		= spi_flash_std_probe,
> +	.bind		= spi_flash_std_bind,
>  	.remove		= spi_flash_std_remove,
>  	.priv_auto_alloc_size = sizeof(struct spi_flash),
>  	.ops		= &spi_flash_std_ops,
> diff --git a/include/spi_flash.h b/include/spi_flash.h
> index 41cf4b0670..24759944eb 100644
> --- a/include/spi_flash.h
> +++ b/include/spi_flash.h
> @@ -75,18 +75,6 @@ int spi_flash_write_dm(struct udevice *dev, u32 offset, size_t len,
>   */
>  int spi_flash_erase_dm(struct udevice *dev, u32 offset, size_t len);
>  
> -/**
> - * spi_flash_std_probe() - Probe a SPI flash device
> - *
> - * This is the standard internal method for probing a SPI flash device to
> - * determine its type. It can be used in chip-specific drivers which need to
> - * do this, typically with of-platdata
> - *
> - * @dev: SPI-flash device to probe
> - * @return 0 if OK, -ve on error
> - */
> -int spi_flash_std_probe(struct udevice *dev);
> -
>  static inline int spi_flash_read(struct spi_flash *flash, u32 offset,
>  				 size_t len, void *buf)
>  {
>

Patch
diff mbox series 

diff --git a/arch/x86/cpu/apollolake/spl.c b/arch/x86/cpu/apollolake/spl.c
index 5a53831dc6..e1ee1e0624 100644
--- a/arch/x86/cpu/apollolake/spl.c
+++ b/arch/x86/cpu/apollolake/spl.c
@@ -68,66 +68,6 @@  SPL_LOAD_IMAGE_METHOD("Mapped SPI", 2, BOOT_DEVICE_SPI_MMAP, rom_load_image);
 
 #if CONFIG_IS_ENABLED(SPI_FLASH_SUPPORT)
 
-static int apl_flash_std_read(struct udevice *dev, u32 offset, size_t len,
-			      void *buf)
-{
-	struct spi_flash *flash = dev_get_uclass_priv(dev);
-	struct mtd_info *mtd = &flash->mtd;
-	size_t retlen;
-
-	return log_ret(mtd->_read(mtd, offset, len, &retlen, buf));
-}
-
-static int apl_flash_probe(struct udevice *dev)
-{
-	return spi_flash_std_probe(dev);
-}
-
-/*
- * Manually set the parent of the SPI flash to SPI, since dtoc doesn't. We also
- * need to allocate the parent_platdata since by the time this function is
- * called device_bind() has already gone past that step.
- */
-static int apl_flash_bind(struct udevice *dev)
-{
-	if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
-		struct dm_spi_slave_platdata *plat;
-		struct udevice *spi;
-		int ret;
-
-		ret = uclass_first_device_err(UCLASS_SPI, &spi);
-		if (ret)
-			return ret;
-		dev->parent = spi;
-
-		plat = calloc(sizeof(*plat), 1);
-		if (!plat)
-			return -ENOMEM;
-		dev->parent_platdata = plat;
-	}
-
-	return 0;
-}
-
-static const struct dm_spi_flash_ops apl_flash_ops = {
-	.read		= apl_flash_std_read,
-};
-
-static const struct udevice_id apl_flash_ids[] = {
-	{ .compatible = "jedec,spi-nor" },
-	{ }
-};
-
-U_BOOT_DRIVER(winbond_w25q128fw) = {
-	.name		= "winbond_w25q128fw",
-	.id		= UCLASS_SPI_FLASH,
-	.of_match	= apl_flash_ids,
-	.bind		= apl_flash_bind,
-	.probe		= apl_flash_probe,
-	.priv_auto_alloc_size = sizeof(struct spi_flash),
-	.ops		= &apl_flash_ops,
-};
-
 /* This uses a SPI flash device to read the next phase */
 static int spl_fast_spi_load_image(struct spl_image_info *spl_image,
 				   struct spl_boot_device *bootdev)
diff --git a/drivers/mtd/spi/sf-uclass.c b/drivers/mtd/spi/sf-uclass.c
index b09046fec3..44cdb3151d 100644
--- a/drivers/mtd/spi/sf-uclass.c
+++ b/drivers/mtd/spi/sf-uclass.c
@@ -122,7 +122,7 @@  static int spi_flash_std_erase(struct udevice *dev, u32 offset, size_t len)
 	return mtd->_erase(mtd, &instr);
 }
 
-int spi_flash_std_probe(struct udevice *dev)
+static int spi_flash_std_probe(struct udevice *dev)
 {
 	struct spi_slave *slave = dev_get_parent_priv(dev);
 	struct spi_flash *flash;
@@ -164,6 +164,32 @@  static int spi_flash_std_remove(struct udevice *dev)
 	return 0;
 }
 
+/*
+ * Manually set the parent of the SPI flash to SPI, since dtoc doesn't. We also
+ * need to allocate the parent_platdata since by the time this function is
+ * called device_bind() has already gone past that step.
+ */
+static int spi_flash_std_bind(struct udevice *dev)
+{
+	if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
+		struct dm_spi_slave_platdata *plat;
+		struct udevice *spi;
+		int ret;
+
+		ret = uclass_first_device_err(UCLASS_SPI, &spi);
+		if (ret)
+			return ret;
+		dev->parent = spi;
+
+		plat = calloc(sizeof(*plat), 1);
+		if (!plat)
+			return -ENOMEM;
+		dev->parent_platdata = plat;
+	}
+
+	return 0;
+}
+
 static const struct dm_spi_flash_ops spi_flash_std_ops = {
 	.read = spi_flash_std_read,
 	.write = spi_flash_std_write,
@@ -180,6 +206,7 @@  U_BOOT_DRIVER(spi_flash_std) = {
 	.id		= UCLASS_SPI_FLASH,
 	.of_match	= spi_flash_std_ids,
 	.probe		= spi_flash_std_probe,
+	.bind		= spi_flash_std_bind,
 	.remove		= spi_flash_std_remove,
 	.priv_auto_alloc_size = sizeof(struct spi_flash),
 	.ops		= &spi_flash_std_ops,
diff --git a/include/spi_flash.h b/include/spi_flash.h
index 41cf4b0670..24759944eb 100644
--- a/include/spi_flash.h
+++ b/include/spi_flash.h
@@ -75,18 +75,6 @@  int spi_flash_write_dm(struct udevice *dev, u32 offset, size_t len,
  */
 int spi_flash_erase_dm(struct udevice *dev, u32 offset, size_t len);
 
-/**
- * spi_flash_std_probe() - Probe a SPI flash device
- *
- * This is the standard internal method for probing a SPI flash device to
- * determine its type. It can be used in chip-specific drivers which need to
- * do this, typically with of-platdata
- *
- * @dev: SPI-flash device to probe
- * @return 0 if OK, -ve on error
- */
-int spi_flash_std_probe(struct udevice *dev);
-
 static inline int spi_flash_read(struct spi_flash *flash, u32 offset,
 				 size_t len, void *buf)
 {