openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status
@ 2021-09-29 11:54 Zev Weiss
  2021-09-29 11:54 ` [PATCH 1/6] of: base: Add function to check for status = "reserved" Zev Weiss
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Zev Weiss @ 2021-09-29 11:54 UTC (permalink / raw)
  To: openbmc
  Cc: devicetree, Vignesh Raghavendra, Zev Weiss, Tudor Ambarus,
	Andrew Jeffery, Greg Kroah-Hartman, linux-kernel,
	Richard Weinberger, Michael Walle, Rob Herring, linux-mtd,
	Cédric Le Goater, Miquel Raynal, Jeremy Kerr, linux-aspeed,
	Frank Rowand, Pratyush Yadav, linux-arm-kernel

Hello,

This patch series aims to improve a scenario that arises in OpenBMC
and which isn't handled very well at the moment.  Certain devices, the
example at hand being the flash chip used to store the host's firmware
(e.g. the BIOS), may be shared between the BMC and the host system but
only available to one or the other at any given time.  The device may
thus be effectively off-limits to the BMC when it boots, and only
usable after userspace performs the necessary steps to coordinate
appropriately with the host (tracking its power state, twiddling
GPIOs, sending IPMI commands, etc.).

Neither the "okay" nor the "disabled" device-tree status values works
nicely for the flash device this case -- an "okay" device gets probed
automatically as soon as the device and a driver for it are available,
and a "disabled" one gets forgotten about entirely, whereas we want
the BMC's kernel to be aware of the existence of the device, but not
try to actually do anything with it (i.e. probe it) until explicitly
requested to do so by userspace.

However, while there's no support for it currently in the kernel tree,
the device-tree spec [0] also lists "reserved" as a possible status
value, and its description seems like a fairly reasonable fit for this
situation:

  Indicates that the device is operational, but should not be used.
  Typically this is used for devices that are controlled by another
  software component, such as platform firmware.

These patches start making use of this status value in the aspeed-smc
driver.  The first patch adds a companion routine to
of_device_is_available() that checks for a "reserved" status instead
of "okay".  The second patch is a small MTD adjustment to allow an
unregistered device to be cleanly re-registered.  Patches 3 through 5
modify the aspeed-smc driver to allow individual chips to be attached
and detached at runtime, and to avoid automatically attaching any
marked as reserved.  Finally, patch 6 employs the newly-supported
status in adding support for the BIOS flash device to the ASRock Rack
e3c246d4i BMC.


Thanks,
Zev

[0] https://github.com/devicetree-org/devicetree-specification/releases/download/v0.3/devicetree-specification-v0.3.pdf

Zev Weiss (6):
  of: base: Add function to check for status = "reserved"
  mtd: core: clear out unregistered devices a bit more
  mtd: spi-nor: aspeed: Refactor registration/unregistration
  mtd: spi-nor: aspeed: Allow attaching & detaching chips at runtime
  mtd: spi-nor: aspeed: Don't automatically attach reserved chips
  ARM: dts: aspeed: Add e3c246d4i BIOS flash device

 .../ABI/stable/sysfs-driver-aspeed-smc        |  17 ++
 .../boot/dts/aspeed-bmc-asrock-e3c246d4i.dts  |  16 ++
 drivers/mtd/mtdcore.c                         |   7 +-
 drivers/mtd/spi-nor/controllers/aspeed-smc.c  | 177 +++++++++++++++---
 drivers/of/base.c                             |  53 +++++-
 include/linux/of.h                            |   6 +
 6 files changed, 238 insertions(+), 38 deletions(-)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-smc

-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/6] of: base: Add function to check for status = "reserved"
  2021-09-29 11:54 [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status Zev Weiss
@ 2021-09-29 11:54 ` Zev Weiss
  2021-09-29 15:49   ` Rob Herring
  2021-09-29 11:54 ` [PATCH 2/6] mtd: core: clear out unregistered devices a bit more Zev Weiss
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Zev Weiss @ 2021-09-29 11:54 UTC (permalink / raw)
  To: openbmc
  Cc: devicetree, Zev Weiss, Greg Kroah-Hartman, linux-kernel,
	Rob Herring, Cédric Le Goater, Jeremy Kerr, Frank Rowand

Per v0.3 of the Devicetree Specification [0]:

  Indicates that the device is operational, but should not be used.
  Typically this is used for devices that are controlled by another
  software component, such as platform firmware.

One use-case for this is in OpenBMC, where certain devices (such as a
BIOS flash chip) may be shared by the host and the BMC, but cannot be
accessed by the BMC during its usual boot-time device probing, because
they require additional (potentially elaborate) coordination with the
host to arbitrate which processor is controlling the device.

Devices marked with this status should thus be instantiated, but not
have a driver bound to them or be otherwise touched.

[0] https://github.com/devicetree-org/devicetree-specification/releases/download/v0.3/devicetree-specification-v0.3.pdf

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 drivers/of/base.c  | 53 +++++++++++++++++++++++++++++++++++++++-------
 include/linux/of.h |  6 ++++++
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index f720c0d246f2..c5cc178fc6bd 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -579,14 +579,18 @@ int of_machine_is_compatible(const char *compat)
 EXPORT_SYMBOL(of_machine_is_compatible);
 
 /**
- *  __of_device_is_available - check if a device is available for use
+ * __of_device_check_status - check if a device's status matches a particular string
  *
- *  @device: Node to check for availability, with locks already held
+ * @device: Node to check status of, with locks already held
+ * @val: Status string to check for
+ * @alt: Optional alternate status string to check for (NULL to check only @val)
+ * @dflt: default to return if status property absent
  *
- *  Return: True if the status property is absent or set to "okay" or "ok",
- *  false otherwise
+ * Return: True if status property exists and matches either @val or @alt.
+ * @dflt if status property is absent.  False otherwise.
  */
-static bool __of_device_is_available(const struct device_node *device)
+static bool __of_device_check_status(const struct device_node *device, const char *val,
+                                     const char *alt, bool dflt)
 {
 	const char *status;
 	int statlen;
@@ -595,17 +599,30 @@ static bool __of_device_is_available(const struct device_node *device)
 		return false;
 
 	status = __of_get_property(device, "status", &statlen);
-	if (status == NULL)
-		return true;
+	if (!status)
+		return dflt;
 
 	if (statlen > 0) {
-		if (!strcmp(status, "okay") || !strcmp(status, "ok"))
+		if (!strcmp(status, val) || (alt && !strcmp(status, alt)))
 			return true;
 	}
 
 	return false;
 }
 
+/**
+ * __of_device_is_available - check if a device is available for use
+ *
+ * @device: Node to check for availability, with locks already held
+ *
+ * Return: True if the status property is absent or set to "okay" or "ok",
+ * false otherwise
+ */
+static bool __of_device_is_available(const struct device_node *device)
+{
+	return __of_device_check_status(device, "okay", "ok", true);
+}
+
 /**
  *  of_device_is_available - check if a device is available for use
  *
@@ -627,6 +644,26 @@ bool of_device_is_available(const struct device_node *device)
 }
 EXPORT_SYMBOL(of_device_is_available);
 
+/**
+ * of_device_is_reserved - check if a device is marked as reserved
+ *
+ * @device: Node to check for reservation
+ *
+ * Return: True if the status property is set to "reserved", false otherwise
+ */
+bool of_device_is_reserved(const struct device_node *device)
+{
+	unsigned long flags;
+	bool res;
+
+	raw_spin_lock_irqsave(&devtree_lock, flags);
+	res = __of_device_check_status(device, "reserved", NULL, false);
+	raw_spin_unlock_irqrestore(&devtree_lock, flags);
+
+	return res;
+}
+EXPORT_SYMBOL(of_device_is_reserved);
+
 /**
  *  of_device_is_big_endian - check if a device has BE registers
  *
diff --git a/include/linux/of.h b/include/linux/of.h
index 6f1c41f109bb..aa9762da5e7c 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -345,6 +345,7 @@ extern int of_device_is_compatible(const struct device_node *device,
 extern int of_device_compatible_match(struct device_node *device,
 				      const char *const *compat);
 extern bool of_device_is_available(const struct device_node *device);
+extern bool of_device_is_reserved(const struct device_node *device);
 extern bool of_device_is_big_endian(const struct device_node *device);
 extern const void *of_get_property(const struct device_node *node,
 				const char *name,
@@ -707,6 +708,11 @@ static inline bool of_device_is_available(const struct device_node *device)
 	return false;
 }
 
+static inline bool of_device_is_reserved(const struct device_node *device)
+{
+	return false;
+}
+
 static inline bool of_device_is_big_endian(const struct device_node *device)
 {
 	return false;
-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/6] mtd: core: clear out unregistered devices a bit more
  2021-09-29 11:54 [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status Zev Weiss
  2021-09-29 11:54 ` [PATCH 1/6] of: base: Add function to check for status = "reserved" Zev Weiss
@ 2021-09-29 11:54 ` Zev Weiss
  2021-09-29 11:54 ` [PATCH 3/6] mtd: spi-nor: aspeed: Refactor registration/unregistration Zev Weiss
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Zev Weiss @ 2021-09-29 11:54 UTC (permalink / raw)
  To: openbmc
  Cc: Vignesh Raghavendra, Zev Weiss, Greg Kroah-Hartman, linux-kernel,
	Richard Weinberger, linux-mtd, Cédric Le Goater,
	Miquel Raynal, Jeremy Kerr

This allows an MTD device that has been unregistered to be easily
re-registered later without triggering "already registered" warnings in
mtd_device_parse_register() and add_mtd_device().

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 drivers/mtd/mtdcore.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index c8fd7f758938..e22266f63ae9 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -747,6 +747,9 @@ int del_mtd_device(struct mtd_info *mtd)
 
 		device_unregister(&mtd->dev);
 
+		/* Clear dev so mtd can be safely re-registered later if desired */
+		memset(&mtd->dev, 0, sizeof(mtd->dev));
+
 		idr_remove(&mtd_idr, mtd->index);
 		of_node_put(mtd_get_of_node(mtd));
 
@@ -1018,8 +1021,10 @@ int mtd_device_unregister(struct mtd_info *master)
 {
 	int err;
 
-	if (master->_reboot)
+	if (master->_reboot) {
 		unregister_reboot_notifier(&master->reboot_notifier);
+		memset(&master->reboot_notifier, 0, sizeof(master->reboot_notifier));
+	}
 
 	if (master->otp_user_nvmem)
 		nvmem_unregister(master->otp_user_nvmem);
-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/6] mtd: spi-nor: aspeed: Refactor registration/unregistration
  2021-09-29 11:54 [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status Zev Weiss
  2021-09-29 11:54 ` [PATCH 1/6] of: base: Add function to check for status = "reserved" Zev Weiss
  2021-09-29 11:54 ` [PATCH 2/6] mtd: core: clear out unregistered devices a bit more Zev Weiss
@ 2021-09-29 11:54 ` Zev Weiss
  2021-10-06  0:03   ` Dhananjay Phadke
  2021-09-29 11:54 ` [PATCH 4/6] mtd: spi-nor: aspeed: Allow attaching & detaching chips at runtime Zev Weiss
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Zev Weiss @ 2021-09-29 11:54 UTC (permalink / raw)
  To: openbmc
  Cc: Vignesh Raghavendra, Zev Weiss, Tudor Ambarus, Andrew Jeffery,
	Greg Kroah-Hartman, linux-kernel, Richard Weinberger,
	Michael Walle, linux-mtd, Cédric Le Goater, Miquel Raynal,
	Jeremy Kerr, linux-aspeed, Pratyush Yadav, linux-arm-kernel

We now have separate functions for registering and unregistering
individual flash chips, instead of the entire controller.  This is a
preparatory step for allowing userspace to request that a specific
chip be attached or detached at runtime.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 drivers/mtd/spi-nor/controllers/aspeed-smc.c | 73 ++++++++++++--------
 1 file changed, 45 insertions(+), 28 deletions(-)

diff --git a/drivers/mtd/spi-nor/controllers/aspeed-smc.c b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
index 7225870e8b18..3c153104a905 100644
--- a/drivers/mtd/spi-nor/controllers/aspeed-smc.c
+++ b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
@@ -107,9 +107,10 @@ struct aspeed_smc_controller {
 	const struct aspeed_smc_info *info;	/* type info of controller */
 	void __iomem *regs;			/* controller registers */
 	void __iomem *ahb_base;			/* per-chip windows resource */
+	struct resource *ahb_res;		/* resource for AHB address space */
 	u32 ahb_window_size;			/* full mapping window size */
 
-	struct aspeed_smc_chip *chips[];	/* pointers to attached chips */
+	struct aspeed_smc_chip *chips[];	/* pointers to connected chips */
 };
 
 /*
@@ -399,15 +400,24 @@ static ssize_t aspeed_smc_write_user(struct spi_nor *nor, loff_t to,
 	return len;
 }
 
+static int aspeed_smc_unregister_chip(struct aspeed_smc_chip *chip)
+{
+	return mtd_device_unregister(&chip->nor.mtd);
+}
+
 static int aspeed_smc_unregister(struct aspeed_smc_controller *controller)
 {
 	struct aspeed_smc_chip *chip;
-	int n;
+	int n, ret;
 
 	for (n = 0; n < controller->info->nce; n++) {
 		chip = controller->chips[n];
-		if (chip)
-			mtd_device_unregister(&chip->nor.mtd);
+		if (chip) {
+			ret = aspeed_smc_unregister_chip(chip);
+			if (ret)
+				dev_warn(controller->dev, "failed to unregister CS%d: %d\n",
+				         n, ret);
+		}
 	}
 
 	return 0;
@@ -756,14 +766,39 @@ static const struct spi_nor_controller_ops aspeed_smc_controller_ops = {
 	.write = aspeed_smc_write_user,
 };
 
-static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
-				  struct device_node *np, struct resource *r)
+static int aspeed_smc_register_chip(struct aspeed_smc_chip *chip)
 {
-	const struct spi_nor_hwcaps hwcaps = {
+	static const struct spi_nor_hwcaps hwcaps = {
 		.mask = SNOR_HWCAPS_READ |
 			SNOR_HWCAPS_READ_FAST |
 			SNOR_HWCAPS_PP,
 	};
+	int ret;
+
+	ret = aspeed_smc_chip_setup_init(chip, chip->controller->ahb_res);
+	if (ret)
+		goto out;
+
+	/*
+	 * TODO: Add support for Dual and Quad SPI protocols attach when board
+	 * support is present as determined by of property.
+	 */
+	ret = spi_nor_scan(&chip->nor, NULL, &hwcaps);
+	if (ret)
+		goto out;
+
+	ret = aspeed_smc_chip_setup_finish(chip);
+	if (ret)
+		goto out;
+
+	ret = mtd_device_register(&chip->nor.mtd, NULL, 0);
+out:
+	return ret;
+}
+
+static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
+				  struct device_node *np, struct resource *r)
+{
 	const struct aspeed_smc_info *info = controller->info;
 	struct device *dev = controller->dev;
 	struct device_node *child;
@@ -773,7 +808,6 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 	for_each_available_child_of_node(np, child) {
 		struct aspeed_smc_chip *chip;
 		struct spi_nor *nor;
-		struct mtd_info *mtd;
 
 		/* This driver does not support NAND or NOR flash devices. */
 		if (!of_device_is_compatible(child, "jedec,spi-nor"))
@@ -810,35 +844,17 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 		chip->cs = cs;
 
 		nor = &chip->nor;
-		mtd = &nor->mtd;
 
 		nor->dev = dev;
 		nor->priv = chip;
 		spi_nor_set_flash_node(nor, child);
 		nor->controller_ops = &aspeed_smc_controller_ops;
 
-		ret = aspeed_smc_chip_setup_init(chip, r);
-		if (ret)
-			break;
-
-		/*
-		 * TODO: Add support for Dual and Quad SPI protocols
-		 * attach when board support is present as determined
-		 * by of property.
-		 */
-		ret = spi_nor_scan(nor, NULL, &hwcaps);
-		if (ret)
-			break;
-
-		ret = aspeed_smc_chip_setup_finish(chip);
-		if (ret)
-			break;
+		controller->chips[cs] = chip;
 
-		ret = mtd_device_register(mtd, NULL, 0);
+		ret = aspeed_smc_register_chip(chip);
 		if (ret)
 			break;
-
-		controller->chips[cs] = chip;
 	}
 
 	if (ret) {
@@ -881,6 +897,7 @@ static int aspeed_smc_probe(struct platform_device *pdev)
 		return PTR_ERR(controller->regs);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	controller->ahb_res = res;
 	controller->ahb_base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(controller->ahb_base))
 		return PTR_ERR(controller->ahb_base);
-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/6] mtd: spi-nor: aspeed: Allow attaching & detaching chips at runtime
  2021-09-29 11:54 [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status Zev Weiss
                   ` (2 preceding siblings ...)
  2021-09-29 11:54 ` [PATCH 3/6] mtd: spi-nor: aspeed: Refactor registration/unregistration Zev Weiss
@ 2021-09-29 11:54 ` Zev Weiss
  2021-09-29 11:54 ` [PATCH 5/6] mtd: spi-nor: aspeed: Don't automatically attach reserved chips Zev Weiss
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Zev Weiss @ 2021-09-29 11:54 UTC (permalink / raw)
  To: openbmc
  Cc: Vignesh Raghavendra, Zev Weiss, Tudor Ambarus, Andrew Jeffery,
	Greg Kroah-Hartman, linux-kernel, Richard Weinberger,
	Michael Walle, linux-aspeed, Cédric Le Goater, linux-mtd,
	Miquel Raynal, Jeremy Kerr, Pratyush Yadav, linux-arm-kernel

There are now two new sysfs attributes, attach_chip and detach_chip,
into which a chip select number can be written to attach or detach the
corresponding chip.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 .../ABI/stable/sysfs-driver-aspeed-smc        |  17 +++
 drivers/mtd/spi-nor/controllers/aspeed-smc.c  | 101 +++++++++++++++++-
 2 files changed, 115 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-aspeed-smc

diff --git a/Documentation/ABI/stable/sysfs-driver-aspeed-smc b/Documentation/ABI/stable/sysfs-driver-aspeed-smc
new file mode 100644
index 000000000000..871b4d65ccb2
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-aspeed-smc
@@ -0,0 +1,17 @@
+What:		/sys/bus/platform/drivers/aspeed-smc/*/attach_chip
+Date:		September 2021
+Contact:	Zev Weiss <zev@bewilderbeest.net>
+Description:	A chip select number may be written into this file to
+		request that the corresponding chip be attached by the
+		driver.
+Users:		OpenBMC.  Proposed changes should be mailed to
+		openbmc@lists.ozlabs.org
+
+What:		/sys/bus/platform/drivers/aspeed-smc/*/detach_chip
+Date:		September 2021
+Contact:	Zev Weiss <zev@bewilderbeest.net>
+Description:	A chip select number may be written into this file to
+		request that the corresponding chip be detached by the
+		driver.
+Users:		OpenBMC.  Proposed changes should be mailed to
+		openbmc@lists.ozlabs.org
diff --git a/drivers/mtd/spi-nor/controllers/aspeed-smc.c b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
index 3c153104a905..da49192a8220 100644
--- a/drivers/mtd/spi-nor/controllers/aspeed-smc.c
+++ b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
@@ -91,6 +91,7 @@ struct aspeed_smc_controller;
 
 struct aspeed_smc_chip {
 	int cs;
+	bool attached;
 	struct aspeed_smc_controller *controller;
 	void __iomem *ctl;			/* control register */
 	void __iomem *ahb_base;			/* base of chip window */
@@ -402,7 +403,15 @@ static ssize_t aspeed_smc_write_user(struct spi_nor *nor, loff_t to,
 
 static int aspeed_smc_unregister_chip(struct aspeed_smc_chip *chip)
 {
-	return mtd_device_unregister(&chip->nor.mtd);
+	int ret = -ENOENT;
+	mutex_lock(&chip->controller->mutex);
+	if (chip->attached) {
+		ret = mtd_device_unregister(&chip->nor.mtd);
+		if (!ret)
+			chip->attached = false;
+	}
+	mutex_unlock(&chip->controller->mutex);
+	return ret;
 }
 
 static int aspeed_smc_unregister(struct aspeed_smc_controller *controller)
@@ -412,7 +421,7 @@ static int aspeed_smc_unregister(struct aspeed_smc_controller *controller)
 
 	for (n = 0; n < controller->info->nce; n++) {
 		chip = controller->chips[n];
-		if (chip) {
+		if (chip && chip->attached) {
 			ret = aspeed_smc_unregister_chip(chip);
 			if (ret)
 				dev_warn(controller->dev, "failed to unregister CS%d: %d\n",
@@ -775,6 +784,13 @@ static int aspeed_smc_register_chip(struct aspeed_smc_chip *chip)
 	};
 	int ret;
 
+	mutex_lock(&chip->controller->mutex);
+
+	if (chip->attached) {
+		ret = -EEXIST;
+		goto out;
+	}
+
 	ret = aspeed_smc_chip_setup_init(chip, chip->controller->ahb_res);
 	if (ret)
 		goto out;
@@ -792,7 +808,12 @@ static int aspeed_smc_register_chip(struct aspeed_smc_chip *chip)
 		goto out;
 
 	ret = mtd_device_register(&chip->nor.mtd, NULL, 0);
+	if (ret)
+		goto out;
+
+	chip->attached = true;
 out:
+	mutex_unlock(&chip->controller->mutex);
 	return ret;
 }
 
@@ -865,6 +886,72 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 	return ret;
 }
 
+static inline struct aspeed_smc_controller *to_aspeed_smc_controller(struct device *dev)
+{
+	struct platform_device *pdev = container_of(dev, struct platform_device, dev);
+	return platform_get_drvdata(pdev);
+}
+
+static ssize_t attach_chip_store(struct device *dev, struct device_attribute *attr,
+                                 const char *buf, size_t count)
+{
+	unsigned long cs;
+	struct aspeed_smc_controller *controller;
+	struct aspeed_smc_chip *chip;
+	ssize_t ret = kstrtoul(buf, 0, &cs);
+	if (ret)
+		return ret;
+
+	controller = to_aspeed_smc_controller(dev);
+	if (cs >= controller->info->nce)
+		return -EINVAL;
+
+	chip = controller->chips[cs];
+
+	if (!chip)
+		return -ENODEV;
+
+	ret = aspeed_smc_register_chip(chip);
+
+	return ret ? ret : count;
+}
+static DEVICE_ATTR_WO(attach_chip);
+
+static ssize_t detach_chip_store(struct device *dev, struct device_attribute *attr,
+                                 const char *buf, size_t count)
+{
+	unsigned long cs;
+	struct aspeed_smc_controller *controller;
+	struct aspeed_smc_chip *chip;
+	ssize_t ret = kstrtoul(buf, 0, &cs);
+	if (ret)
+		return ret;
+
+	controller = to_aspeed_smc_controller(dev);
+	if (cs >= controller->info->nce)
+		return -EINVAL;
+
+	chip = controller->chips[cs];
+
+	if (!chip)
+		return -ENODEV;
+
+	ret = aspeed_smc_unregister_chip(chip);
+
+	return ret ? ret : count;
+}
+static DEVICE_ATTR_WO(detach_chip);
+
+static struct attribute *aspeed_smc_sysfs_attrs[] = {
+	&dev_attr_attach_chip.attr,
+	&dev_attr_detach_chip.attr,
+	NULL,
+};
+
+static const struct attribute_group aspeed_smc_sysfs_attr_group = {
+	.attrs = aspeed_smc_sysfs_attrs,
+};
+
 static int aspeed_smc_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -905,8 +992,16 @@ static int aspeed_smc_probe(struct platform_device *pdev)
 	controller->ahb_window_size = resource_size(res);
 
 	ret = aspeed_smc_setup_flash(controller, np, res);
-	if (ret)
+	if (ret) {
 		dev_err(dev, "Aspeed SMC probe failed %d\n", ret);
+		return ret;
+	}
+
+	ret = devm_device_add_group(dev, &aspeed_smc_sysfs_attr_group);
+	if (ret) {
+		dev_err(dev, "Failed to create sysfs files\n");
+		aspeed_smc_unregister(controller);
+	}
 
 	return ret;
 }
-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 5/6] mtd: spi-nor: aspeed: Don't automatically attach reserved chips
  2021-09-29 11:54 [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status Zev Weiss
                   ` (3 preceding siblings ...)
  2021-09-29 11:54 ` [PATCH 4/6] mtd: spi-nor: aspeed: Allow attaching & detaching chips at runtime Zev Weiss
@ 2021-09-29 11:54 ` Zev Weiss
  2021-09-29 11:54 ` [PATCH 6/6] ARM: dts: aspeed: Add e3c246d4i BIOS flash device Zev Weiss
  2021-09-29 16:08 ` [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status Rob Herring
  6 siblings, 0 replies; 11+ messages in thread
From: Zev Weiss @ 2021-09-29 11:54 UTC (permalink / raw)
  To: openbmc
  Cc: Vignesh Raghavendra, Zev Weiss, Tudor Ambarus, Andrew Jeffery,
	Greg Kroah-Hartman, linux-kernel, Richard Weinberger,
	Michael Walle, linux-mtd, Cédric Le Goater, Miquel Raynal,
	Jeremy Kerr, linux-aspeed, Pratyush Yadav, linux-arm-kernel

With this change, any flash chips under the controller that are marked
with a DT status of "reserved" will be created, but not automatically
attached.  Userspace can later request that they be attached using the
attach_chip sysfs file.

This is to accommodate situations where a chip may be (for example)
shared with another controller external to the SoC and require
userspace to arbitrate access to it prior to actually attaching it.
(such as a firmware SPI flash shared between a BMC and the host
system).

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 drivers/mtd/spi-nor/controllers/aspeed-smc.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/controllers/aspeed-smc.c b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
index da49192a8220..328b008fafb2 100644
--- a/drivers/mtd/spi-nor/controllers/aspeed-smc.c
+++ b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
@@ -826,10 +826,14 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 	unsigned int cs;
 	int ret = -ENODEV;
 
-	for_each_available_child_of_node(np, child) {
+	for_each_child_of_node(np, child) {
 		struct aspeed_smc_chip *chip;
 		struct spi_nor *nor;
 
+		/* Skip disabled nodes, but include reserved ones for later attachment */
+		if (!of_device_is_available(child) && !of_device_is_reserved(child))
+			continue;
+
 		/* This driver does not support NAND or NOR flash devices. */
 		if (!of_device_is_compatible(child, "jedec,spi-nor"))
 			continue;
@@ -873,6 +877,9 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 
 		controller->chips[cs] = chip;
 
+		if (of_device_is_reserved(child))
+			continue;
+
 		ret = aspeed_smc_register_chip(chip);
 		if (ret)
 			break;
-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 6/6] ARM: dts: aspeed: Add e3c246d4i BIOS flash device
  2021-09-29 11:54 [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status Zev Weiss
                   ` (4 preceding siblings ...)
  2021-09-29 11:54 ` [PATCH 5/6] mtd: spi-nor: aspeed: Don't automatically attach reserved chips Zev Weiss
@ 2021-09-29 11:54 ` Zev Weiss
  2021-09-29 16:08 ` [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status Rob Herring
  6 siblings, 0 replies; 11+ messages in thread
From: Zev Weiss @ 2021-09-29 11:54 UTC (permalink / raw)
  To: openbmc
  Cc: devicetree, linux-aspeed, Zev Weiss, Andrew Jeffery,
	Greg Kroah-Hartman, linux-kernel, Rob Herring,
	Cédric Le Goater, Jeremy Kerr, linux-arm-kernel

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 .../arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
index 9b4cf5ebe6d5..456f4de53869 100644
--- a/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
@@ -68,6 +68,22 @@ flash@0 {
 	};
 };
 
+&spi1 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_spi1_default>;
+	flash@0 {
+		/*
+		 * The BIOS SPI flash is shared with the host via an
+		 * external mux, and is not accessible by the BMC by
+		 * default (hence "reserved" rather than "okay").
+		 */
+		status = "reserved";
+		label = "bios";
+		m25p,fast-read;
+	};
+};
+
 &uart5 {
 	status = "okay";
 };
-- 
2.33.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/6] of: base: Add function to check for status = "reserved"
  2021-09-29 11:54 ` [PATCH 1/6] of: base: Add function to check for status = "reserved" Zev Weiss
@ 2021-09-29 15:49   ` Rob Herring
  0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2021-09-29 15:49 UTC (permalink / raw)
  To: Zev Weiss
  Cc: devicetree, Greg Kroah-Hartman, OpenBMC Maillist, linux-kernel,
	Jeremy Kerr, Frank Rowand, Cédric Le Goater

On Wed, Sep 29, 2021 at 6:54 AM Zev Weiss <zev@bewilderbeest.net> wrote:
>
> Per v0.3 of the Devicetree Specification [0]:
>
>   Indicates that the device is operational, but should not be used.
>   Typically this is used for devices that are controlled by another
>   software component, such as platform firmware.
>
> One use-case for this is in OpenBMC, where certain devices (such as a
> BIOS flash chip) may be shared by the host and the BMC, but cannot be
> accessed by the BMC during its usual boot-time device probing, because
> they require additional (potentially elaborate) coordination with the
> host to arbitrate which processor is controlling the device.
>
> Devices marked with this status should thus be instantiated, but not
> have a driver bound to them or be otherwise touched.
>
> [0] https://github.com/devicetree-org/devicetree-specification/releases/download/v0.3/devicetree-specification-v0.3.pdf
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
>  drivers/of/base.c  | 53 +++++++++++++++++++++++++++++++++++++++-------
>  include/linux/of.h |  6 ++++++
>  2 files changed, 51 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index f720c0d246f2..c5cc178fc6bd 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -579,14 +579,18 @@ int of_machine_is_compatible(const char *compat)
>  EXPORT_SYMBOL(of_machine_is_compatible);
>
>  /**
> - *  __of_device_is_available - check if a device is available for use
> + * __of_device_check_status - check if a device's status matches a particular string
>   *
> - *  @device: Node to check for availability, with locks already held
> + * @device: Node to check status of, with locks already held
> + * @val: Status string to check for
> + * @alt: Optional alternate status string to check for (NULL to check only @val)
> + * @dflt: default to return if status property absent
>   *
> - *  Return: True if the status property is absent or set to "okay" or "ok",
> - *  false otherwise
> + * Return: True if status property exists and matches either @val or @alt.
> + * @dflt if status property is absent.  False otherwise.
>   */
> -static bool __of_device_is_available(const struct device_node *device)
> +static bool __of_device_check_status(const struct device_node *device, const char *val,
> +                                     const char *alt, bool dflt)

How about val==NULL means available/okay and then you can get rid of
alt and dflt.

Otherwise, I'd simply not try to share the implementation here and
just add of_device_is_reserved().

Rob

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status
  2021-09-29 11:54 [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status Zev Weiss
                   ` (5 preceding siblings ...)
  2021-09-29 11:54 ` [PATCH 6/6] ARM: dts: aspeed: Add e3c246d4i BIOS flash device Zev Weiss
@ 2021-09-29 16:08 ` Rob Herring
  2021-09-29 22:00   ` Zev Weiss
  6 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2021-09-29 16:08 UTC (permalink / raw)
  To: Zev Weiss
  Cc: devicetree, Vignesh Raghavendra, linux-aspeed, Tudor Ambarus,
	Andrew Jeffery, Greg Kroah-Hartman, OpenBMC Maillist,
	linux-kernel, Richard Weinberger, Michael Walle, MTD Maling List,
	Miquel Raynal, Jeremy Kerr, Frank Rowand, Pratyush Yadav,
	linux-arm-kernel, Cédric Le Goater

On Wed, Sep 29, 2021 at 6:54 AM Zev Weiss <zev@bewilderbeest.net> wrote:
>
> Hello,
>
> This patch series aims to improve a scenario that arises in OpenBMC
> and which isn't handled very well at the moment.  Certain devices, the
> example at hand being the flash chip used to store the host's firmware
> (e.g. the BIOS), may be shared between the BMC and the host system but
> only available to one or the other at any given time.  The device may
> thus be effectively off-limits to the BMC when it boots, and only
> usable after userspace performs the necessary steps to coordinate
> appropriately with the host (tracking its power state, twiddling
> GPIOs, sending IPMI commands, etc.).
>
> Neither the "okay" nor the "disabled" device-tree status values works
> nicely for the flash device this case -- an "okay" device gets probed
> automatically as soon as the device and a driver for it are available,
> and a "disabled" one gets forgotten about entirely, whereas we want
> the BMC's kernel to be aware of the existence of the device, but not
> try to actually do anything with it (i.e. probe it) until explicitly
> requested to do so by userspace.

While Linux treats 'disabled' as gone forever, that's not exactly what
the spec says. Either disabled or reserved could change in theory. But
I do agree 'reserved' is the right choice for your use.

> However, while there's no support for it currently in the kernel tree,
> the device-tree spec [0] also lists "reserved" as a possible status
> value, and its description seems like a fairly reasonable fit for this
> situation:
>
>   Indicates that the device is operational, but should not be used.
>   Typically this is used for devices that are controlled by another
>   software component, such as platform firmware.
>
> These patches start making use of this status value in the aspeed-smc
> driver.  The first patch adds a companion routine to
> of_device_is_available() that checks for a "reserved" status instead
> of "okay".  The second patch is a small MTD adjustment to allow an
> unregistered device to be cleanly re-registered.  Patches 3 through 5
> modify the aspeed-smc driver to allow individual chips to be attached
> and detached at runtime, and to avoid automatically attaching any
> marked as reserved.  Finally, patch 6 employs the newly-supported
> status in adding support for the BIOS flash device to the ASRock Rack
> e3c246d4i BMC.

I'm not sure this should be MTD specific. There's other cases where we
may want devices to become available. So the question is whether there
should be a more generic mechanism rather than each subsystem coming
up with their own thing.

There's out of tree support for applying overlays which could be used
here. The issue with it is we don't want it to be unconstrained where
an overlay can make any change anywhere in a DT.

Another possibility is making 'status' writeable from userspace. It is
just a sysfs file. That too may need to be opt-in.

Rob

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status
  2021-09-29 16:08 ` [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status Rob Herring
@ 2021-09-29 22:00   ` Zev Weiss
  0 siblings, 0 replies; 11+ messages in thread
From: Zev Weiss @ 2021-09-29 22:00 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Zev Weiss, Vignesh Raghavendra, linux-aspeed,
	Tudor Ambarus, Andrew Jeffery, Greg Kroah-Hartman,
	OpenBMC Maillist, Miquel Raynal, linux-kernel, Michael Walle,
	MTD Maling List, Cédric Le Goater, Richard Weinberger,
	Jeremy Kerr, Frank Rowand, Pratyush Yadav, linux-arm-kernel

On Wed, Sep 29, 2021 at 09:08:03AM PDT, Rob Herring wrote:
>On Wed, Sep 29, 2021 at 6:54 AM Zev Weiss <zev@bewilderbeest.net> wrote:
>>
>> Hello,
>>
>> This patch series aims to improve a scenario that arises in OpenBMC
>> and which isn't handled very well at the moment.  Certain devices, the
>> example at hand being the flash chip used to store the host's firmware
>> (e.g. the BIOS), may be shared between the BMC and the host system but
>> only available to one or the other at any given time.  The device may
>> thus be effectively off-limits to the BMC when it boots, and only
>> usable after userspace performs the necessary steps to coordinate
>> appropriately with the host (tracking its power state, twiddling
>> GPIOs, sending IPMI commands, etc.).
>>
>> Neither the "okay" nor the "disabled" device-tree status values works
>> nicely for the flash device this case -- an "okay" device gets probed
>> automatically as soon as the device and a driver for it are available,
>> and a "disabled" one gets forgotten about entirely, whereas we want
>> the BMC's kernel to be aware of the existence of the device, but not
>> try to actually do anything with it (i.e. probe it) until explicitly
>> requested to do so by userspace.
>
>While Linux treats 'disabled' as gone forever, that's not exactly what
>the spec says. Either disabled or reserved could change in theory. But
>I do agree 'reserved' is the right choice for your use.

True -- the spec's description of "disabled" also sounds like it could
be an appropriate fit for this case, but the existing (somewhat
different) interpretation in the kernel is well-established enough that
I figured that ship had sailed.

>
>> However, while there's no support for it currently in the kernel tree,
>> the device-tree spec [0] also lists "reserved" as a possible status
>> value, and its description seems like a fairly reasonable fit for this
>> situation:
>>
>>   Indicates that the device is operational, but should not be used.
>>   Typically this is used for devices that are controlled by another
>>   software component, such as platform firmware.
>>
>> These patches start making use of this status value in the aspeed-smc
>> driver.  The first patch adds a companion routine to
>> of_device_is_available() that checks for a "reserved" status instead
>> of "okay".  The second patch is a small MTD adjustment to allow an
>> unregistered device to be cleanly re-registered.  Patches 3 through 5
>> modify the aspeed-smc driver to allow individual chips to be attached
>> and detached at runtime, and to avoid automatically attaching any
>> marked as reserved.  Finally, patch 6 employs the newly-supported
>> status in adding support for the BIOS flash device to the ASRock Rack
>> e3c246d4i BMC.
>
>I'm not sure this should be MTD specific. There's other cases where we
>may want devices to become available. So the question is whether there
>should be a more generic mechanism rather than each subsystem coming
>up with their own thing.
>

Agreed, and in fact in an earlier version of these patches I had
approached this via a more generic tweak to the driver-core code to
inhibit attaching drivers to devices marked as reserved.  The problem I
had with that is that it ended up being kind of limited in how far down
the device tree it would actually take effect.  For example in this
particular case, I could mark the entire aspeed-smc controller as
reserved and prevent the driver core from binding the driver to it, but
nothing more fine-grained than that.  If I marked an individual flash
chip behind that controller as reserved, the aspeed-smc driver would get
attached (as expected), but that driver (not the driver core) is
responsible for inspecting its child devices and attaching them, and its
existing probe routine only checks of_device_is_available() and hence
can't distinguish between reserved and disabled.

So while I should probably reincorporate the corresponding driver-core
change, I don't see it as a complete solution, and I don't see an
obvious way to achieve one centrally without requiring modifications in
individual drivers, unfortunately.

>There's out of tree support for applying overlays which could be used
>here. The issue with it is we don't want it to be unconstrained where
>an overlay can make any change anywhere in a DT.
>

Yeah, I'm vaguely aware of the dt-overlay patches, but had been under
the impression that their prospects for mainlining were fairly dim and
hence was looking at alternate approaches (of somewhat more limited scope).

>Another possibility is making 'status' writeable from userspace. It is
>just a sysfs file. That too may need to be opt-in.
>

The sysfs file you're referring to being those under
/sys/firmware/devicetree I assume?  That's an interesting idea...in
addition to making it opt-in, we'd presumably need to restrict what
state transitions are allowed (maybe only okay<->reserved?).  Keeping
/sys/firmware/fdt in sync with that seems like it might be a bit of a
headache though...perhaps that would just remain a static reflection of
whatever the state was at boot?


Thanks,
Zev

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/6] mtd: spi-nor: aspeed: Refactor registration/unregistration
  2021-09-29 11:54 ` [PATCH 3/6] mtd: spi-nor: aspeed: Refactor registration/unregistration Zev Weiss
@ 2021-10-06  0:03   ` Dhananjay Phadke
  0 siblings, 0 replies; 11+ messages in thread
From: Dhananjay Phadke @ 2021-10-06  0:03 UTC (permalink / raw)
  To: zev
  Cc: vigneshr, linux-aspeed, tudor.ambarus, andrew, gregkh, openbmc,
	linux-kernel, richard, michael, linux-mtd, Dhananjay Phadke,
	miquel.raynal, jk, p.yadav, linux-arm-kernel, clg



On Wed, 29 Sep 2021, Zev Weiss wrote:

> We now have separate functions for registering and unregistering
> individual flash chips, instead of the entire controller.  This is a
> preparatory step for allowing userspace to request that a specific
> chip be attached or detached at runtime.
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
> drivers/mtd/spi-nor/controllers/aspeed-smc.c | 73 ++++++++++++--------
> 1 file changed, 45 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/controllers/aspeed-smc.c b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
> index 7225870e8b18..3c153104a905 100644
> --- a/drivers/mtd/spi-nor/controllers/aspeed-smc.c
> +++ b/drivers/mtd/spi-nor/controllers/aspeed-smc.c
> @@ -107,9 +107,10 @@ struct aspeed_smc_controller {
> 	const struct aspeed_smc_info *info;	/* type info of controller */
> 	void __iomem *regs;			/* controller registers */
> 	void __iomem *ahb_base;			/* per-chip windows resource */
> +	struct resource *ahb_res;		/* resource for AHB address space */
> 	u32 ahb_window_size;			/* full mapping window size */
>
> -	struct aspeed_smc_chip *chips[];	/* pointers to attached chips */
> +	struct aspeed_smc_chip *chips[];	/* pointers to connected chips */
> };
>
> /*
> @@ -399,15 +400,24 @@ static ssize_t aspeed_smc_write_user(struct spi_nor *nor, loff_t to,
> 	return len;
> }
>
> +static int aspeed_smc_unregister_chip(struct aspeed_smc_chip *chip)
> +{
> +	return mtd_device_unregister(&chip->nor.mtd);
> +}
> +
> static int aspeed_smc_unregister(struct aspeed_smc_controller *controller)
> {
> 	struct aspeed_smc_chip *chip;
> -	int n;
> +	int n, ret;
>
> 	for (n = 0; n < controller->info->nce; n++) {
> 		chip = controller->chips[n];
> -		if (chip)
> -			mtd_device_unregister(&chip->nor.mtd);
> +		if (chip) {
> +			ret = aspeed_smc_unregister_chip(chip);
> +			if (ret)
> +				dev_warn(controller->dev, "failed to unregister CS%d: %d\n",
> +				         n, ret);
> +		}
> 	}
>
> 	return 0;
> @@ -756,14 +766,39 @@ static const struct spi_nor_controller_ops aspeed_smc_controller_ops = {
> 	.write = aspeed_smc_write_user,
> };
>
> -static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
> -				  struct device_node *np, struct resource *r)
> +static int aspeed_smc_register_chip(struct aspeed_smc_chip *chip)
> {
> -	const struct spi_nor_hwcaps hwcaps = {
> +	static const struct spi_nor_hwcaps hwcaps = {
> 		.mask = SNOR_HWCAPS_READ |
> 			SNOR_HWCAPS_READ_FAST |
> 			SNOR_HWCAPS_PP,
> 	};
> +	int ret;
> +
> +	ret = aspeed_smc_chip_setup_init(chip, chip->controller->ahb_res);
> +	if (ret)
> +		goto out;
> +
> +	/*
> +	 * TODO: Add support for Dual and Quad SPI protocols attach when board
> +	 * support is present as determined by of property.
> +	 */
> +	ret = spi_nor_scan(&chip->nor, NULL, &hwcaps);
> +	if (ret)
> +		goto out;
> +
> +	ret = aspeed_smc_chip_setup_finish(chip);
> +	if (ret)
> +		goto out;
> +
> +	ret = mtd_device_register(&chip->nor.mtd, NULL, 0);
> +out:
> +	return ret;
> +}

I was looking into this driver probe walking sub-nodes.

It looks like all controller drivers are doing / have to do similar work -

(1) Parse common dt bindings documented in Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
(2) Run spi_nor_scan() with tweaked/reduced capabilities.
(3) mtd_register_device().

It would be good to absorb this workflow in mtd/spi-nor core and add 'reserved' as common
dt property to avoid (2) and (3) from probe.


Regards,
Dhananjay


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-10-06  0:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29 11:54 [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status Zev Weiss
2021-09-29 11:54 ` [PATCH 1/6] of: base: Add function to check for status = "reserved" Zev Weiss
2021-09-29 15:49   ` Rob Herring
2021-09-29 11:54 ` [PATCH 2/6] mtd: core: clear out unregistered devices a bit more Zev Weiss
2021-09-29 11:54 ` [PATCH 3/6] mtd: spi-nor: aspeed: Refactor registration/unregistration Zev Weiss
2021-10-06  0:03   ` Dhananjay Phadke
2021-09-29 11:54 ` [PATCH 4/6] mtd: spi-nor: aspeed: Allow attaching & detaching chips at runtime Zev Weiss
2021-09-29 11:54 ` [PATCH 5/6] mtd: spi-nor: aspeed: Don't automatically attach reserved chips Zev Weiss
2021-09-29 11:54 ` [PATCH 6/6] ARM: dts: aspeed: Add e3c246d4i BIOS flash device Zev Weiss
2021-09-29 16:08 ` [PATCH 0/6] Dynamic aspeed-smc flash chips via "reserved" DT status Rob Herring
2021-09-29 22:00   ` Zev Weiss

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).