All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control
@ 2016-10-22  0:45 Kun Yi
  2016-10-22  0:45 ` [RFC 1/2] spi: aspeed: ASPEED 24XX/25XX SPI1 disable/passthrough/master mode control Kun Yi
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Kun Yi @ 2016-10-22  0:45 UTC (permalink / raw)
  To: openbmc; +Cc: kunyi, brendanhiggins, clg

Hello,

I'm looking for feedback on a simple Aspeed SPI1 mode control driver that does two things mainly:
1. Expose a sysfs interface for userland control of SPI1 interface.
2. Optionally override the SPI1 mode at probe if "start_mode" is configured in 
   device tree.

Aspeed 24XX/25XX support three interface modes for their SPI1 interface,
which can be configured via HW strap register. In the typical case of connecting
a host flash chip SPI1 interface:
- SPI master mode makes BMC the master to the host SPI flash
- SPI pass-through mode make whichever external SPI interface connected to SYSSPI 
  be the master
- There are also "disable" and "debug" modes. Aspeed 25XX specified the debug
  mode is reserved though.

In particular, SPI pass-through mode would enable an external SPI master to
directly program host SPI chip and possibly serve as an alternative way to LPC
for host CPU accessing SPI flash. I think by providing a sysfs handle, userspace
applications can have a flexible way to mux host SPI flash ownership between 
BMC/external on the fly.

Overriding the SPI mode at probe time can be useful to make sure 2500-smc flash
controller always identifies the flash chip and create /dev/mtd files correctly.

I wrote the driver as a standalone misc driver for the ease of initial implementation.
There are other options such as merging the functionality into the SMC driver
itself, possibly enabled by optional device tree arguments/nodes, but I just want to
send the patches out first to gather comments on perceived usefulness of
the feature and how you think the driver should be constructed.

Feedback greatly appreciated!

Regards,
Kun

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

* [RFC 1/2] spi: aspeed: ASPEED 24XX/25XX SPI1 disable/passthrough/master mode control
  2016-10-22  0:45 [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control Kun Yi
@ 2016-10-22  0:45 ` Kun Yi
  2016-10-22  0:45 ` [RFC 2/2] spi: aspeed: documentation and device tree bindings for ASPEED SPI1 mode controller Kun Yi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Kun Yi @ 2016-10-22  0:45 UTC (permalink / raw)
  To: openbmc; +Cc: kunyi, brendanhiggins, clg

Simple driver that exposes userspace sysfs to control SPI1 mode. It also
optionally override initial SPI mode if given "start_mode" parameter in
the device tree.

Signed-off-by: Kun Yi <kunyi@google.com>
---
 drivers/misc/Kconfig           |   6 ++
 drivers/misc/Makefile          |   1 +
 drivers/misc/aspeed-spi-mode.c | 191 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 198 insertions(+)
 create mode 100644 drivers/misc/aspeed-spi-mode.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 4617ddc..ea9b713 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -809,6 +809,12 @@ config ASPEED_BT_IPMI_HOST
 	help
 	  Support for the Aspeed BT ipmi host.
 
+config SPI_MODECTRL_ASPEED
+        tristate "Support for SPI1 host flash mode change on Aspeed 24xx/25xx"
+        ---help---
+          Support for the Aspeed 24xx/25xx sysfs control toggle between
+	  disabled/master/pass-through modes.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 724861b..0cce3d6 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -58,3 +58,4 @@ obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
 obj-$(CONFIG_PANEL)             += panel.o
 obj-$(CONFIG_ASPEED_BT_IPMI_HOST)	+= bt-host.o
+obj-$(CONFIG_SPI_MODECTRL_ASPEED)	+= aspeed-spi-mode.o
diff --git a/drivers/misc/aspeed-spi-mode.c b/drivers/misc/aspeed-spi-mode.c
new file mode 100644
index 0000000..20f7754
--- /dev/null
+++ b/drivers/misc/aspeed-spi-mode.c
@@ -0,0 +1,191 @@
+/*
+ * ASPEED host flash SPI1 mode controller driver.
+ *
+ * Copyright (c) 2016 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_platform.h>
+
+struct aspeed_spi_mode_controller {
+	struct mutex mutex;			/* controller access mutex */
+	void __iomem *hwstrap_regs;		/* controller registers */
+	void __iomem *hwstrap_reset;		/* scu reset registers */
+};
+
+/* SPI mode definitions. MODE_DEBUG is reserved on AST2500. */
+#define SPI_MODE_DISABLED	0
+#define SPI_MODE_MASTER		1
+#define SPI_MODE_DEBUG		2
+#define SPI_MODE_PASSTHROUGH	3
+/* SPI mode control bits in HW strap register */
+#define SPI_MODE_OFFSET		(12)
+#define SPI_MODE_MASK		(BIT(12) | BIT(13))
+/* Extract SPI mode from register value */
+#define SPI_MODE(reg)		((reg & SPI_MODE_MASK) >> SPI_MODE_OFFSET)
+
+static int aspeed_get_spi_mode(struct aspeed_spi_mode_controller *controller)
+{
+	u32 reg;
+
+	mutex_lock(&controller->mutex);
+	reg = readl(controller->hwstrap_regs);
+	mutex_unlock(&controller->mutex);
+
+	return SPI_MODE(reg);
+}
+
+/*
+ * Set SPI1 interface to given mode.
+ * Note SCU70 reg bits cannot be set to 0. Need to write to SCU7C in order to
+ * clear bits in SCU70.
+ */
+static int aspeed_set_spi_mode(struct aspeed_spi_mode_controller *controller,
+			       unsigned int mode)
+{
+
+	if (mode != SPI_MODE_DISABLED &&
+	    mode != SPI_MODE_MASTER &&
+	    mode != SPI_MODE_PASSTHROUGH) {
+		return -EPERM;
+	}
+
+	void __iomem *regs;
+	void __iomem *regs_reset;
+	u32 val;
+
+	regs = controller->hwstrap_regs;
+	regs_reset = controller->hwstrap_reset;
+	mutex_lock(&controller->mutex);
+	val = readl(regs);
+
+	if (SPI_MODE(val) == mode)
+		goto finish;
+
+	/* Reset bit 13:12 and set to new value */
+	writel(SPI_MODE_MASK, regs_reset);
+	val &= ~SPI_MODE_MASK;
+	val |= (mode << SPI_MODE_OFFSET);
+	writel(val, regs);
+
+	pr_debug("HW strap register set to 0x%x\n", readl(regs));
+
+finish:
+	mutex_unlock(&controller->mutex);
+	return 0;
+}
+
+static ssize_t aspeed_spi_mode_show(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	struct aspeed_spi_mode_controller *controller;
+
+	controller = dev_get_drvdata(dev);
+	return scnprintf(buf, PAGE_SIZE, "%d\n",
+			 aspeed_get_spi_mode(controller));
+}
+
+static ssize_t aspeed_spi_mode_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct aspeed_spi_mode_controller *controller = dev_get_drvdata(dev);
+	unsigned long mode;
+	int err;
+
+	err = kstrtoul(buf, 10, &mode);
+	if (err)
+		return err;
+
+	err = aspeed_set_spi_mode(controller, mode);
+
+	if (err) {
+		dev_err(dev, "Support disabled(0) master(1) pass-through(3)");
+		return err;
+	}
+
+	return count;
+}
+
+static DEVICE_ATTR(spi_mode, S_IRUSR | S_IWUSR, aspeed_spi_mode_show,
+		   aspeed_spi_mode_store);
+
+
+static const struct of_device_id aspeed_spi_mode_match[] = {
+	{ .compatible = "aspeed,aspeed_spi_mode" },
+	{},
+}
+MODULE_DEVICE_TABLE(of, aspeed_spi_mode_match);
+
+static int aspeed_spi_mode_probe(struct platform_device *pdev)
+{
+	struct aspeed_spi_mode_controller *controller;
+	struct resource *r;
+	void __iomem *regs, *regs_reset;
+	u32 mode;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	regs = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	regs_reset = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(regs_reset))
+		return PTR_ERR(regs_reset);
+
+	controller = devm_kzalloc(&pdev->dev, sizeof(*controller), GFP_KERNEL);
+	if (!controller)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, controller);
+	controller->hwstrap_regs = regs;
+	controller->hwstrap_reset = regs_reset;
+	mutex_init(&controller->mutex);
+
+	if (!of_property_read_u32(pdev->dev.of_node, "start_mode", &mode)) {
+		int err;
+
+		err = aspeed_set_spi_mode(controller, mode);
+		if (!err)
+			dev_info(&pdev->dev, "Set SPI mode to %d\n", mode);
+	}
+
+	if (device_create_file(&pdev->dev, &dev_attr_spi_mode) != 0)
+		dev_err(&pdev->dev, "Sysfs attribute creation failed");
+
+	dev_info(&pdev->dev, "ASPEED spi mode controller initialized");
+
+	return 0;
+}
+
+static int aspeed_spi_mode_remove(struct platform_device *pdev)
+{
+	device_remove_file(&pdev->dev, &dev_attr_spi_mode);
+	return 0;
+}
+
+static struct platform_driver aspeed_spi_mode_driver = {
+	.probe = aspeed_spi_mode_probe,
+	.remove = aspeed_spi_mode_remove,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = aspeed_spi_mode_match,
+	}
+};
+
+module_platform_driver(aspeed_spi_mode_driver);
+
+MODULE_AUTHOR("Kun Yi <kunyi@google.com>");
+MODULE_DESCRIPTION("ASPEED SPI mode controller");
+MODULE_LICENSE("GPL v2");
-- 
2.8.0.rc3.226.g39d4020

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

* [RFC 2/2] spi: aspeed: documentation and device tree bindings for ASPEED SPI1 mode controller
  2016-10-22  0:45 [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control Kun Yi
  2016-10-22  0:45 ` [RFC 1/2] spi: aspeed: ASPEED 24XX/25XX SPI1 disable/passthrough/master mode control Kun Yi
@ 2016-10-22  0:45 ` Kun Yi
  2016-10-24  1:59 ` [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control Joel Stanley
  2016-10-24  3:05 ` Andrew Jeffery
  3 siblings, 0 replies; 9+ messages in thread
From: Kun Yi @ 2016-10-22  0:45 UTC (permalink / raw)
  To: openbmc; +Cc: kunyi, brendanhiggins, clg

There is no accurate HW representation for the driver at the moment,
thus it is represented as a "virtual" platform device node. The driver
is placed in misc for the same reason.

Signed-off-by: Kun Yi <kunyi@google.com>
---
 .../devicetree/bindings/misc/aspeed-spi-mode.txt   | 29 ++++++++++++++++++++++
 arch/arm/boot/dts/aspeed-ast2500-evb.dts           | 10 ++++++++
 2 files changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/aspeed-spi-mode.txt

diff --git a/Documentation/devicetree/bindings/misc/aspeed-spi-mode.txt b/Documentation/devicetree/bindings/misc/aspeed-spi-mode.txt
new file mode 100644
index 0000000..540b101
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/aspeed-spi-mode.txt
@@ -0,0 +1,29 @@
+Device tree entry for SPI1 mode controller on AST24XX/AST25XX.
+
+This binding enables the SPI1 mode controller virtual driver that exposes
+the SPI1 interface toggle capability to userspace through sysfs interface. If
+optional argument "start_mode" is defined, it also configures the SPI mode.
+
+Required Properties:
+- #address-cells	: should be 1
+- #size-cells		: should be 1
+- compatible		: should be "aspeed, aspeed_spi_mode"
+- reg			: addresses and sizes of HW strap register and HW strap
+			  reset register. Should be < 0x1e6e2070 0x4 > and
+			  < 0x1e6e207c 0x4 > respectively.
+
+Optional Properties:
+- start_mode		: spi mode to override HW strap setting during probe.
+			  Can be 0(disabled) 1(master) or 3(pass-through).
+			  If not defined, HW strap or pinctrl setting is kept.
+
+Example:
+	spi_mode_ctrl: spi_mode {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "aspeed,aspeed_spi_mode";
+		start_mode = <1>;
+		status = "okay";
+		reg = < 0x1e6e2070 0x4
+			0x1e6e207c 0x4 >;
+	};
diff --git a/arch/arm/boot/dts/aspeed-ast2500-evb.dts b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
index 33b3907..f50812a 100644
--- a/arch/arm/boot/dts/aspeed-ast2500-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2500-evb.dts
@@ -18,6 +18,16 @@
 	memory {
 		reg = <0x80000000 0x20000000>;
 	};
+
+	spi_mode_ctrl: spi_mode {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "aspeed,aspeed_spi_mode";
+		start_mode = <1>;
+		status = "okay";
+		reg = < 0x1e6e2070 0x4
+			0x1e6e207c 0x4 >;
+	};
 };
 
 &fmc {
-- 
2.8.0.rc3.226.g39d4020

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

* Re: [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control
  2016-10-22  0:45 [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control Kun Yi
  2016-10-22  0:45 ` [RFC 1/2] spi: aspeed: ASPEED 24XX/25XX SPI1 disable/passthrough/master mode control Kun Yi
  2016-10-22  0:45 ` [RFC 2/2] spi: aspeed: documentation and device tree bindings for ASPEED SPI1 mode controller Kun Yi
@ 2016-10-24  1:59 ` Joel Stanley
  2016-10-24 18:35   ` Kun Yi
  2016-10-24  3:05 ` Andrew Jeffery
  3 siblings, 1 reply; 9+ messages in thread
From: Joel Stanley @ 2016-10-24  1:59 UTC (permalink / raw)
  To: Kun Yi; +Cc: OpenBMC Maillist, Brendan Higgins

Hell Kun,

On Sat, Oct 22, 2016 at 11:15 AM, Kun Yi <kunyi@google.com> wrote:
> Hello,
>
> I'm looking for feedback on a simple Aspeed SPI1 mode control driver that does two things mainly:
> 1. Expose a sysfs interface for userland control of SPI1 interface.
> 2. Optionally override the SPI1 mode at probe if "start_mode" is configured in
>    device tree.

Why do you plan to override the mode at boot time? Is this for firmware updates?

We plan to move away from having the NOR flash accessible from both
the host and the BMC for all platforms. There's no good way to
indicate ownership, and the Linux mtd subsystem does not have a good
way of relinquishing nor reestablishing control. For instance, there's
no way to "unmount" the MTD device, and there's no way to force a
re-scan of partitions if you modify the layout.

I think there are some issues with your approach that break
pinctrl/pinmux that Andrew will highlight.

Cheers,

Joel

>
> Aspeed 24XX/25XX support three interface modes for their SPI1 interface,
> which can be configured via HW strap register. In the typical case of connecting
> a host flash chip SPI1 interface:
> - SPI master mode makes BMC the master to the host SPI flash
> - SPI pass-through mode make whichever external SPI interface connected to SYSSPI
>   be the master
> - There are also "disable" and "debug" modes. Aspeed 25XX specified the debug
>   mode is reserved though.
>
> In particular, SPI pass-through mode would enable an external SPI master to
> directly program host SPI chip and possibly serve as an alternative way to LPC
> for host CPU accessing SPI flash. I think by providing a sysfs handle, userspace
> applications can have a flexible way to mux host SPI flash ownership between
> BMC/external on the fly.
>
> Overriding the SPI mode at probe time can be useful to make sure 2500-smc flash
> controller always identifies the flash chip and create /dev/mtd files correctly.
>
> I wrote the driver as a standalone misc driver for the ease of initial implementation.
> There are other options such as merging the functionality into the SMC driver
> itself, possibly enabled by optional device tree arguments/nodes, but I just want to
> send the patches out first to gather comments on perceived usefulness of
> the feature and how you think the driver should be constructed.
>
> Feedback greatly appreciated!
>
> Regards,
> Kun
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc

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

* Re: [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control
  2016-10-22  0:45 [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control Kun Yi
                   ` (2 preceding siblings ...)
  2016-10-24  1:59 ` [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control Joel Stanley
@ 2016-10-24  3:05 ` Andrew Jeffery
  2016-10-24 21:22   ` Kun Yi
  3 siblings, 1 reply; 9+ messages in thread
From: Andrew Jeffery @ 2016-10-24  3:05 UTC (permalink / raw)
  To: Kun Yi, openbmc; +Cc: brendanhiggins, Joel Stanley

[-- Attachment #1: Type: text/plain, Size: 2363 bytes --]

Hi Kun,

On Fri, 2016-10-21 at 17:45 -0700, Kun Yi wrote:
> Hello,
> 
> I'm looking for feedback on a simple Aspeed SPI1 mode control driver that does two things mainly:
> 1. Expose a sysfs interface for userland control of SPI1 interface.
> 2. Optionally override the SPI1 mode at probe if "start_mode" is configured in 
>    device tree.
> 
> Aspeed 24XX/25XX support three interface modes for their SPI1 interface,
> which can be configured via HW strap register. In the typical case of connecting
> a host flash chip SPI1 interface:
> - SPI master mode makes BMC the master to the host SPI flash
> - SPI pass-through mode make whichever external SPI interface connected to SYSSPI 
>   be the master
> - There are also "disable" and "debug" modes. Aspeed 25XX specified the debug
>   mode is reserved though.
> 
> In particular, SPI pass-through mode would enable an external SPI master to
> directly program host SPI chip and possibly serve as an alternative way to LPC
> for host CPU accessing SPI flash. I think by providing a sysfs handle, userspace
> applications can have a flexible way to mux host SPI flash ownership between 
> BMC/external on the fly.
> 
> Overriding the SPI mode at probe time can be useful to make sure 2500-smc flash
> controller always identifies the flash chip and create /dev/mtd files correctly.
> 
> I wrote the driver as a standalone misc driver for the ease of initial implementation.
> There are other options such as merging the functionality into the SMC driver
> itself, possibly enabled by optional device tree arguments/nodes, but I just want to
> send the patches out first to gather comments on perceived usefulness of
> the feature and how you think the driver should be constructed.

At least one issue I have with the implementation is that the strap
register is currently "owned" by the pinctrl driver, but the patch as
it stands has no interaction with this subsystem. Arguably the pinctrl
driver is where this should be implemented, as it encapsulates pinmux
whose aim is to manage the functionality made available on the SoC's
pins.

Currently the pinctrl driver treats the strapping register as read-
only. That should probably change in light of what you're trying to do
here, though only to the extent of allowing the SPI mode switch.

Cheers,

Andrew

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control
  2016-10-24  1:59 ` [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control Joel Stanley
@ 2016-10-24 18:35   ` Kun Yi
  0 siblings, 0 replies; 9+ messages in thread
From: Kun Yi @ 2016-10-24 18:35 UTC (permalink / raw)
  To: Joel Stanley; +Cc: OpenBMC Maillist, Brendan Higgins

[-- Attachment #1: Type: text/plain, Size: 3968 bytes --]

Hi Joel, thanks for commenting!


On Sun, Oct 23, 2016 at 6:59 PM, Joel Stanley <joel@jms.id.au> wrote:

> Hell Kun,
>
> On Sat, Oct 22, 2016 at 11:15 AM, Kun Yi <kunyi@google.com> wrote:
> > Hello,
> >
> > I'm looking for feedback on a simple Aspeed SPI1 mode control driver
> that does two things mainly:
> > 1. Expose a sysfs interface for userland control of SPI1 interface.
> > 2. Optionally override the SPI1 mode at probe if "start_mode" is
> configured in
> >    device tree.
>
> Why do you plan to override the mode at boot time? Is this for firmware
> updates?
>

More important here is the ability to change the SPI mode at *run time*.
One use case is for platforms without eSPI/LPC support. When host CPU is
booting and BIOS needs to read SPI chip, the most straightforward way for
BMC to allow host access to the SPI chip is to toggle to SPI pass-through
mode. Thus the core purpose of the driver is for enabling SPI pass-through
mode on demand.

Overriding the SPI mode at boot time is only to make sure the host SPI chip
can be recognized correctly by the SMC driver. Because currently pinctrl
treats HW strap as read-only, mis-configured HW strap pins will cause SMC
driver to fail reading SPI flash ID and not register it in MTD subsystem,
and there is no easy way to probe again later. Though enabling pinctrl to
set HW strap register might be a better approach as Andrew mentioned.


> We plan to move away from having the NOR flash accessible from both
> the host and the BMC for all platforms. There's no good way to
> indicate ownership, and the Linux mtd subsystem does not have a good
> way of relinquishing nor reestablishing control. For instance, there's
> no way to "unmount" the MTD device, and there's no way to force a
> re-scan of partitions if you modify the layout.
>

I saw some discussions related on IRC but wasn't clear about the details.
Could you elaborate a little? Do you plan to only allow BMC daemons to
access both host/BMC NOR flashes?

I got the impression that mtd will stick around for now. Do we have a time
frame for deprecating that?


> I think there are some issues with your approach that break
> pinctrl/pinmux that Andrew will highlight.
>

Sure I will reply to those in Andrew's email.


>
> Cheers,
>
> Joel
>
> >
> > Aspeed 24XX/25XX support three interface modes for their SPI1 interface,
> > which can be configured via HW strap register. In the typical case of
> connecting
> > a host flash chip SPI1 interface:
> > - SPI master mode makes BMC the master to the host SPI flash
> > - SPI pass-through mode make whichever external SPI interface connected
> to SYSSPI
> >   be the master
> > - There are also "disable" and "debug" modes. Aspeed 25XX specified the
> debug
> >   mode is reserved though.
> >
> > In particular, SPI pass-through mode would enable an external SPI master
> to
> > directly program host SPI chip and possibly serve as an alternative way
> to LPC
> > for host CPU accessing SPI flash. I think by providing a sysfs handle,
> userspace
> > applications can have a flexible way to mux host SPI flash ownership
> between
> > BMC/external on the fly.
> >
> > Overriding the SPI mode at probe time can be useful to make sure
> 2500-smc flash
> > controller always identifies the flash chip and create /dev/mtd files
> correctly.
> >
> > I wrote the driver as a standalone misc driver for the ease of initial
> implementation.
> > There are other options such as merging the functionality into the SMC
> driver
> > itself, possibly enabled by optional device tree arguments/nodes, but I
> just want to
> > send the patches out first to gather comments on perceived usefulness of
> > the feature and how you think the driver should be constructed.
> >
> > Feedback greatly appreciated!
> >
> > Regards,
> > Kun
> >
> > _______________________________________________
> > openbmc mailing list
> > openbmc@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/openbmc
>



-- 
Regards,
Kun

[-- Attachment #2: Type: text/html, Size: 5703 bytes --]

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

* Re: [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control
  2016-10-24  3:05 ` Andrew Jeffery
@ 2016-10-24 21:22   ` Kun Yi
  2016-10-24 23:26     ` Andrew Jeffery
  0 siblings, 1 reply; 9+ messages in thread
From: Kun Yi @ 2016-10-24 21:22 UTC (permalink / raw)
  To: Andrew Jeffery; +Cc: OpenBMC Maillist, Brendan Higgins, Joel Stanley

[-- Attachment #1: Type: text/plain, Size: 2823 bytes --]

Hi Andrew,

One question I have though: I think pinmux/pinctrl driver is supposed to be
a boot time static pin configure module and parses DT and configures
accordingly. Is it possible to use pinmux to dynamically modify the pin
states? This is essentially the problem I'm trying to deal with.

Thanks,
Kun

On Sun, Oct 23, 2016 at 8:05 PM, Andrew Jeffery <andrew@aj.id.au> wrote:

> Hi Kun,
>
> On Fri, 2016-10-21 at 17:45 -0700, Kun Yi wrote:
> > Hello,
> >
> > I'm looking for feedback on a simple Aspeed SPI1 mode control driver
> that does two things mainly:
> > 1. Expose a sysfs interface for userland control of SPI1 interface.
> > 2. Optionally override the SPI1 mode at probe if "start_mode" is
> configured in
> >    device tree.
> >
> > Aspeed 24XX/25XX support three interface modes for their SPI1 interface,
> > which can be configured via HW strap register. In the typical case of
> connecting
> > a host flash chip SPI1 interface:
> > - SPI master mode makes BMC the master to the host SPI flash
> > - SPI pass-through mode make whichever external SPI interface connected
> to SYSSPI
> >   be the master
> > - There are also "disable" and "debug" modes. Aspeed 25XX specified the
> debug
> >   mode is reserved though.
> >
> > In particular, SPI pass-through mode would enable an external SPI master
> to
> > directly program host SPI chip and possibly serve as an alternative way
> to LPC
> > for host CPU accessing SPI flash. I think by providing a sysfs handle,
> userspace
> > applications can have a flexible way to mux host SPI flash ownership
> between
> > BMC/external on the fly.
> >
> > Overriding the SPI mode at probe time can be useful to make sure
> 2500-smc flash
> > controller always identifies the flash chip and create /dev/mtd files
> correctly.
> >
> > I wrote the driver as a standalone misc driver for the ease of initial
> implementation.
> > There are other options such as merging the functionality into the SMC
> driver
> > itself, possibly enabled by optional device tree arguments/nodes, but I
> just want to
> > send the patches out first to gather comments on perceived usefulness of
> > the feature and how you think the driver should be constructed.
>
> At least one issue I have with the implementation is that the strap
> register is currently "owned" by the pinctrl driver, but the patch as
> it stands has no interaction with this subsystem. Arguably the pinctrl
> driver is where this should be implemented, as it encapsulates pinmux
> whose aim is to manage the functionality made available on the SoC's
> pins.
>
> Currently the pinctrl driver treats the strapping register as read-
> only. That should probably change in light of what you're trying to do
> here, though only to the extent of allowing the SPI mode switch.
>
> Cheers,
>
> Andrew




-- 
Regards,
Kun

[-- Attachment #2: Type: text/html, Size: 3608 bytes --]

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

* Re: [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control
  2016-10-24 21:22   ` Kun Yi
@ 2016-10-24 23:26     ` Andrew Jeffery
  2016-10-25 17:51       ` Kun Yi
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Jeffery @ 2016-10-24 23:26 UTC (permalink / raw)
  To: Kun Yi; +Cc: OpenBMC Maillist, Brendan Higgins, Joel Stanley

[-- Attachment #1: Type: text/plain, Size: 1053 bytes --]

On Mon, 2016-10-24 at 14:22 -0700, Kun Yi wrote:
> Hi Andrew,
> 
> One question I have though: I think pinmux/pinctrl driver is supposed
> to be a boot time static pin configure module and parses DT and
> configures accordingly. Is it possible to use pinmux to dynamically
> modify the pin states? 

Yes, see "Runtime pinmuxing" here (at the bottom of the document):

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/pinctrl.txt

So you'd need to patch the FMC driver that Milton and Cedric have been
working on to (de)activate pin states, likely at probe() and remove()
(this implies you'll need to somehow orchestrate a remove/switch,
switch/probe dance). Alternatively, provide a way to suspend operations
on the MTD and switch the pins between master and passthru in the FMC
driver. Some complications are that this functionality is only
available for SPI1, and you might need to do a bit of work to guarantee
sensible outcomes for operations that might span the mode switch.

Cheers,

Andrew

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control
  2016-10-24 23:26     ` Andrew Jeffery
@ 2016-10-25 17:51       ` Kun Yi
  0 siblings, 0 replies; 9+ messages in thread
From: Kun Yi @ 2016-10-25 17:51 UTC (permalink / raw)
  To: Andrew Jeffery; +Cc: OpenBMC Maillist, Brendan Higgins, Joel Stanley

[-- Attachment #1: Type: text/plain, Size: 1262 bytes --]

Thanks Andrew that was helpful! I will look into pinctrl and FMC driver
then.

Thanks,
Kun

On Mon, Oct 24, 2016 at 4:26 PM, Andrew Jeffery <andrew@aj.id.au> wrote:

> On Mon, 2016-10-24 at 14:22 -0700, Kun Yi wrote:
> > Hi Andrew,
> >
> > One question I have though: I think pinmux/pinctrl driver is supposed
> > to be a boot time static pin configure module and parses DT and
> > configures accordingly. Is it possible to use pinmux to dynamically
> > modify the pin states?
>
> Yes, see "Runtime pinmuxing" here (at the bottom of the document):
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/
> linux.git/tree/Documentation/pinctrl.txt
>
> So you'd need to patch the FMC driver that Milton and Cedric have been
> working on to (de)activate pin states, likely at probe() and remove()
> (this implies you'll need to somehow orchestrate a remove/switch,
> switch/probe dance). Alternatively, provide a way to suspend operations
> on the MTD and switch the pins between master and passthru in the FMC
> driver. Some complications are that this functionality is only
> available for SPI1, and you might need to do a bit of work to guarantee
> sensible outcomes for operations that might span the mode switch.
>
> Cheers,
>
> Andrew




-- 
Regards,
Kun

[-- Attachment #2: Type: text/html, Size: 1999 bytes --]

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

end of thread, other threads:[~2016-10-25 17:51 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-22  0:45 [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control Kun Yi
2016-10-22  0:45 ` [RFC 1/2] spi: aspeed: ASPEED 24XX/25XX SPI1 disable/passthrough/master mode control Kun Yi
2016-10-22  0:45 ` [RFC 2/2] spi: aspeed: documentation and device tree bindings for ASPEED SPI1 mode controller Kun Yi
2016-10-24  1:59 ` [RFC 0/2] spi: aspeed: SPI1 mode control driver enabling sysfs control Joel Stanley
2016-10-24 18:35   ` Kun Yi
2016-10-24  3:05 ` Andrew Jeffery
2016-10-24 21:22   ` Kun Yi
2016-10-24 23:26     ` Andrew Jeffery
2016-10-25 17:51       ` Kun Yi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.