Mezzanine Low Speed connector bus

I have been a bit hesitant to reiterate the series, but this is anyways a v1 of the 96Boards Mezzanine Low Speed connector driver framework.

I know the boards have a high speed connector as well, just trying to work stepwise and do the simple things first.

What convinced me to continue was very real usecases that exist today, as this makes the secure96 Mezzanine work:

- Get something upstream that makes it possible to without any trouble or extensive ugly hacking probe and use the secure96 mezzanine. This is motivated by the IoT design space which needs a way to plug in proper security and the secure96 offers a TPM chip for root of trust and key handling.

- A clear indication that the same approach will work just as fine with ACPI without reinventing the universe. I do not have a definitive proof of this, but it is more plausible that this will be reusable for ACPI than any of the DT-centric ideas I've seen for populating daughterboards. There are 96boards using ACPI today. A secure96 TPM is desireable for things like the ARM developer box which has this LS connector:
https://www.96boards.org/product/developerbox/

- Some indication that binding the connector like this will not implicate or screw things up for the DT-overlays idea, i.e. a both/and and not either/or approach.

The idea is for this to go upstream through ARM SoC the day noone actively NACK it and someone actively ACK it. Expect some more iterations.

Linus Walleij (4):
eeprom: at24: Allow passing gpiodesc from pdata
spi: Make of_find_spi_device_by_node() available
bus: Add DT bindings for 96Boards low speed connector
bus: 96boards Low-Speed Connector

 .../bus/96boards,low-speed-connector.txt      |  50 +++
 drivers/bus/Kconfig                           |   2 +
 drivers/bus/Makefile                          |   4 +-
 drivers/bus/daughterboards/96boards-ls-bus.c  |  39 ++
 .../daughterboards/96boards-ls-connector.c    | 367 ++++++++++++++++++
 .../bus/daughterboards/96boards-mezzanines.h  |  77 ++++
 .../bus/daughterboards/96boards-secure96.c    | 265 +++++++++++++
 drivers/bus/daughterboards/Kconfig            |  50 +++
 drivers/bus/daughterboards/Makefile           |   6 +
 drivers/misc/eeprom/at24.c                    |   6 +-
 drivers/spi/spi.c                             |  33 +-
 include/linux/platform_data/at24.h            |   2 +
 include/linux/spi/spi.h                       |   4 +
 13 files changed, 888 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/bus/96boards,low-speed-connector.txt
 create mode 100644 drivers/bus/daughterboards/96boards-ls-bus.c
 create mode 100644 drivers/bus/daughterboards/96boards-ls-connector.c
 create mode 100644 drivers/bus/daughterboards/96boards-mezzanines.h
 create mode 100644 drivers/bus/daughterboards/96boards-secure96.c
 create mode 100644 drivers/bus/daughterboards/Kconfig
 create mode 100644 drivers/bus/daughterboards/Makefile

2.17.0

This makes it possible to pass an initialized GPIO descriptor to the driver through platform data.

This is useful when we are dealing with EEPROMs on expansion boards where the GPIO has to be looked up indirectly using a connector abstraction (several systems using the same connector) so the machine descriptor tables cannot be used to associate the descriptor with the device and we then want to pass this descriptor on to the EEPROM driver this way instead.

---
ChangeLog v1->v2:
- No changes
- Requesting an ACK from the EEPROM maintainer (Bartosz) so we can apply this to the ARM SoC tree with the series depending on it.

---
 drivers/misc/eeprom/at24.c         | 6 +++++-
 include/linux/platform_data/at24.h | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

index f5cc517d1131..d577cdbe221e 100644
@@ -662,7 +662,11 @@ static int at24_probe(struct i2c_client *client)
 	at24->client[0].client = client;
 	at24->client[0].regmap = regmap;
 
-	at24->wp_gpio = devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_HIGH);
+	if (pdata.wp_gpiod)
+		at24->wp_gpio = pdata.wp_gpiod;
+	else
+		at24->wp_gpio =
+			devm_gpiod_get_optional(dev, "wp", GPIOD_OUT_HIGH);
 	if (IS_ERR(at24->wp_gpio))
 		return PTR_ERR(at24->wp_gpio);
 

index 63507ff464ee..5606fb2ef76c 100644
@@ -11,6 +11,7 @@
 #include <linux/types.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/bitops.h>
+#include <linux/gpio/consumer.h>
 
 /**
  * struct at24_platform_data - data to set up at24 (generic eeprom) driver

@@ -55,6 +56,7 @@ struct at24_platform_data {
 
 	void		(*setup)(struct nvmem_device *nvmem, void *context);
 	void		*context;
+	struct gpio_desc *wp_gpiod;
 };
 
 #endif /* _LINUX_AT24_H */

2.17.0

2018-08-23 12:33 GMT+02:00 Linus Walleij <[email protected]>:

Ugh I don't like this at all. In fact I'm right now trying to *get rid* of platform_data from at24 in favor of device properties[1].

Part of the changes from my series actually removes the setup function from at24 and replaces it with a notifier fired by the nvmem framework.

I would really prefer that we introduce some generic way of handling write-protect mechanisms in nvmem. Maybe starting by moving the wp_gpio to nvmem and then passing it in nvmem_config?

Best regards,
Bartosz Golaszewski

[1] https://patchwork.kernel.org/cover/10562365/

This externalizes and exports the symbol of_find_spi_device_by_node() from the SPI core akin to how of_find_i2c_adapter_by_node() is already available. As we will need this also for non-dynamic OF setups, we move it under a CONFIG_OF check.

---
ChangeLog RFC->v1:
- No changes
- Requesting an ACK from the SPI mainatiner so that this patch can be applied with the rest of the series.

---
 drivers/spi/spi.c       | 33 ++++++++++++++++++---------------
 include/linux/spi/spi.h |  4 ++++
 2 files changed, 22 insertions(+), 15 deletions(-)

index ec395a6baf9c..6a1085077317 100644
@@ -3314,27 +3314,14 @@ EXPORT_SYMBOL_GPL(spi_write_then_read);
 
 /*-------------------------------------------------------------------------*/
 
-#if IS_ENABLED(CONFIG_OF_DYNAMIC)
-static int __spi_of_device_match(struct device *dev, void *data)
-{
-	return dev->of_node == data;
-}
-
-/* must call put_device() when done with returned spi_device device */
-static struct spi_device *of_find_spi_device_by_node(struct device_node *node)
-{
-	struct device *dev = bus_find_device(&spi_bus_type, NULL, node,
-						__spi_of_device_match);
-	return dev ? to_spi_device(dev) : NULL;
-}
-
+#if IS_ENABLED(CONFIG_OF)
 static int __spi_of_controller_match(struct device *dev, const void *data)
 {
 	return dev->of_node == data;
 }
 
 /* the spi controllers are not using spi_bus, so we find it with another way */
-static struct spi_controller *of_find_spi_controller_by_node(struct device_node *node)
+struct spi_controller *of_find_spi_controller_by_node(struct device_node *node)
 {
 	struct device *dev;
 

@@ -3349,6 +3336,22 @@ static struct spi_controller *of_find_spi_controller_by_node(struct device_node
 	/* reference got in class_find_device */
 	return container_of(dev, struct spi_controller, dev);
 }
+EXPORT_SYMBOL_GPL(of_find_spi_controller_by_node);
+#endif
+
+#if IS_ENABLED(CONFIG_OF_DYNAMIC)
+static int __spi_of_device_match(struct device *dev, void *data)
+{
+	return dev->of_node == data;
+}
+
+/* must call put_device() when done with returned spi_device device */
+static struct spi_device *of_find_spi_device_by_node(struct device_node *node)
+{
+	struct device *dev = bus_find_device(&spi_bus_type, NULL, node,
+						__spi_of_device_match);
+	return dev ? to_spi_device(dev) : NULL;
+}
 
 static int of_spi_notify(struct notifier_block *nb, unsigned long action,
 			 void *arg)

index a64235e05321..c2be93224bd1 100644
@@ -639,6 +639,10 @@ extern int devm_spi_register_controller(struct device *dev,
 					struct spi_controller *ctlr);
 extern void spi_unregister_controller(struct spi_controller *ctlr);
 
+#if IS_ENABLED(CONFIG_OF)
+struct spi_controller *of_find_spi_controller_by_node(struct device_node *node);
+#endif
+
 extern struct spi_controller *spi_busnum_to_master(u16 busnum);
 
 /*

2.17.0

We need to be able to grab resources on a 96Boards low speed connector to populate and use a daughterboard, so define some very rudimentary properties simply passing phandles for the resources used on the low speed connector.

---
 .../bus/96boards,low-speed-connector.txt      | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/96boards,low-speed-connector.txt

new file mode 100644
index 000000000000..6631b3edd01f
@@ -0,0 +1,50 @@
+96Boards Low Speed Connectors
+
+The 96Boards connectors represent a non-dicoverable Low Speed (LS)
+and High Speed (HS) external bus on the 96Board development
+boards. Each 96Board presents both of these connectors.
+
+The Low Speed (LS) connector is a 40 pin 2x20 female pin header:
+- Power
+- Two buttons: power and reset
+- 2 x UART one optional with just RX/TX, one required with modem
+  CTS/RTS
+- 2 x I2C
+- 1 x SPI
+- 1 x I2S (audio)
+- 12 x GPIO lines named GPIO-A thru GPIO-L
+
+Further details on the electronics and signals are available in
+"96Boards Consumer Edition, Low Cost Hardware Platform Specification"
+As of writing version 1.0, January 2015.
+
+Required properties:
+
+- compatible: shall be "96boards,low-speed-connector"
+- i2c0: phandle to the I2C0 bus
+- i2c1: phandle to the I2C1 bus
+- spi: phandle to the SPI bus
+- gpios: a list of phandles to the GPIOs connected to the
+  connector. The handles need to be uniformly specified with the
+  0 flag as the connector is not the end consumer.
+
+Example:
+
+lscon: connector {
+	compatible = "96boards,low-speed-connector";
+	i2c0 = <&i2csw_0>;
+	i2c1 = <&i2csw_1>;
+	spi = <&spi0>;
+	gpios = <&gpio 36 0>, /* GPIO-A */
+	        <&gpio 37 0>, /* GPIO-B */
+		<&gpio 39 0>, /* GPIO-C */
+		<&gpio 40 0>, /* GPIO-D */
+		<&gpio 44 0>, /* GPIO-E */
+		<&gpio 45 0>, /* GPIO-F */
+		<&gpio 78 0>, /* GPIO-G */
+		<&gpio 79 0>, /* GPIO-H */
+		<&gpio 80 0>, /* GPIO-I */
+		<&gpio 81 0>, /* GPIO-J */
+		<&gpio 82 0>, /* GPIO-K */
+		<&gpio 83 0>; /* GPIO-L */
+};

2.17.0

This only works for describing what's on the connector, but does nothing for describing downstream devices on the connector. We already have a way to do the former with label properties at least for uart, i2c and spi. And gpio has it's own method of labeling lines.

lscon: connector {
       compatible = "96boards,low-speed-connector";
};

This is on purpose as the downstream daughterboards are described by C code rather than DT. The idea is to use the same for ACPI rather than DSDT.

Sorry, I am just too ignorant about the semantic difference between a phandle and an alias. As long as I can follow it to get a handle on the device node I'm happy, aliases are fine I guess.

The specification says:

"The phandle property specifies a numerical identifier for a node that is unique within the devicetree. The phandle property value is used by other nodes that need to refer to the node associated with the property."

Numerical? Hm.

The aliases are very vague and seem to not be really specified. Not in the DT spec at least. And that is not in line with how the Linux kernel DTs uses aliases anyway, it seems.

Syntactically they look like so:

aliases {
        serial0 = &uart1;
};

phandles {
        serial0 = <&uart1>;
};

Not a big difference.

In your opinion, what constitues the semantic meaning of an alias vs a phandle?

We have code for it (by Stephen Boyd) which works and I tested it even, it works fine!

But it is not intended for what I do here. It shares the ambition to define all devices in the daughterboard using DT, so there must be a consuming node, on the daughterboard, using
my-gpios = <&connector 0>; and we are back
at overlays. There is no way for a connector driver to grab a GPIO from the GPIO map, the map is just a cross-reference, not resource provider.

Maybe I could let the connector say that it's using GPIOs from itself? I can take a stab at that. It will look weird but it's allright I guess.

I suspect "full solution" in the above paragraph means full DT overlays, and ACPI systems are none of your concern in this context?

A halfway solution is what I am trying to do, because the walking all the way down the road means we run into the imperialistic ambitions of both DT and ACPI DSDT to describe the whole system using just DT or just ACPI DSDT.

As mentioned, we have today the developer box that is using ACPI, and the other 96Boards using mostly DT.

Which currently means similar support code twice for these (simple) mezzanines, then author and support overlays in both HW description languages.

Like with the fwnode rework that is going on in parallel, in my view it is really necessary for both DT and ACPI to take a step back and stop being so imperialistic. It seems they are both living under the assumption that they do not really need to care about the other HW description language, and it is stifling development. Maybe this view is too harsh, I am sometimes also just thinking inside my little box :(

My current plan is to develop the code in the patch set using fwnode where possible so that I have a proof of concept of the same mezzanine population code being used for DT and ACPI alike. It works for GPIOs, but phandles or aliases are going to be a challenge.

Yours,
Linus Walleij

This creates a bus for using a small connector driver to plug in "mezzanine boards" on the 96boards low-speed connector either by a very simple compatible string for the entire board, or by simply inserting the board device from userspace.

These "mezzanine boards" are no different than "capes", "logic modules", etc. This thing, a non-discoverable connector where a user can plug in a few peripherals has been reinvented a few times.

As a proof-of-concept we add the Secure96, a quite minimal mezzanine board.

Users can register their boards in a simple way. Either just add their compatible-string in the device tree:

board {
        compatible = "96boards,secure96";
};

And if they can't even change three lines in their device tree, or if they at runtime decide to plug in some board and test it, they can use sysfs, as exemplified by plugging in the secure96 security board at runtime:

secure96

[   61.014629] lscon connector: called mezzanine_store on secure96
[   61.020530] lscon connector: populate secure96
[   61.027081] at24 1-0050: 2048 byte 24c128 EEPROM, writable, 128 bytes/write
[   61.053569] atmel-ecc 1-0060: configuration zone is unlocked
[   61.502535] tpm_tis_spi spi0.0: 2.0 TPM (device-id 0x1B, rev-id 16)

(...)

The plug-in board can be removed from sysfs and added back again multiple times like this with the devices being runtime added and removed by two writes to sysfs.

(...)

---
ChangeLog RFC->v1:
- Create a new bus type as recommended by Arnd, fixing a bunch of scalability issues and making the whole thing look like most other kernel device driver cores.
- Create LS connector devices that have .probe() and .remove() functions like any other device.
- Move dynamic device creation to bus attribute files.

---
 drivers/bus/Kconfig                           |   2 +
 drivers/bus/Makefile                          |   4 +-
 drivers/bus/daughterboards/96boards-ls-bus.c  |  39 ++
 .../daughterboards/96boards-ls-connector.c    | 367 ++++++++++++++++++
 .../bus/daughterboards/96boards-mezzanines.h  |  77 ++++
 .../bus/daughterboards/96boards-secure96.c    | 265 +++++++++++++
 drivers/bus/daughterboards/Kconfig            |  50 +++
 drivers/bus/daughterboards/Makefile           |   6 +
 8 files changed, 809 insertions(+), 1 deletion(-)
 create mode 100644 drivers/bus/daughterboards/96boards-ls-bus.c
 create mode 100644 drivers/bus/daughterboards/96boards-ls-connector.c
 create mode 100644 drivers/bus/daughterboards/96boards-mezzanines.h
 create mode 100644 drivers/bus/daughterboards/96boards-secure96.c
 create mode 100644 drivers/bus/daughterboards/Kconfig
 create mode 100644 drivers/bus/daughterboards/Makefile

index d1c0b60e9326..98393c6e2190 100644
@@ -173,4 +173,6 @@ config DA8XX_MSTPRI
 
 source "drivers/bus/fsl-mc/Kconfig"
 
+source "drivers/bus/daughterboards/Kconfig"
+
 endmenu

index b8f036cca7ff..7a792e3e2cc2 100644
@@ -29,5 +29,7 @@ obj-$(CONFIG_TI_SYSC)		+= ti-sysc.o
 obj-$(CONFIG_TS_NBUS)		+= ts-nbus.o
 obj-$(CONFIG_UNIPHIER_SYSTEM_BUS)	+= uniphier-system-bus.o
 obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o
-
 obj-$(CONFIG_DA8XX_MSTPRI)	+= da8xx-mstpri.o
+
+# Non-discoverable daughterboards
+obj-$(CONFIG_DAUGHTERBOARDS)	+= daughterboards/

new file mode 100644
index 000000000000..23de06b3fce3
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * 96boards Low-speed Connector bus initialization
+ * (C) 2018 Linus Walleij <[email protected]>
+ */
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/of_platform.h>
+#include "96boards-mezzanines.h"
+
+static int ls_match(struct device *dev, struct device_driver *drv)
+{
+	/* First match on OF node */
+	if (of_driver_match_device(dev, drv))
+		return 1;
+
+	/* Second match on name */
+	return !strcmp(dev_name(dev), drv->name);
+}
+
+struct bus_type ls_bus_type = {
+	.name = "96boards-ls-connector-bus",
+	.match = ls_match,
+};
+EXPORT_SYMBOL_GPL(ls_bus_type);
+
+static int __init ls_bus_init(void)
+{
+	int ret;
+
+	/* Register the LS connector bus so devices can start to probe */
+	ret = bus_register(&ls_bus_type);
+	if (ret) {
+		pr_err("could not register LS connector bus\n");
+		return ret;
+	}
+	return 0;
+}
+postcore_initcall(ls_bus_init);

new file mode 100644
index 000000000000..c9691b470fc5
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for non-discoverable daughterboards
+#
+obj-$(CONFIG_96BOARDS_LS_CONNECTOR)	+= 96boards-ls-bus.o 96boards-ls-connector.o
+obj-$(CONFIG_96BOARDS_SECURE96)		+= 96boards-secure96.o

2.17.0

For the moment, NACK since there is not a substantive change from before. This is without having read beyond this introduction.

I will read through the rest of the series and revisit my previous conclusions, but most likely not for a few days.

-Frank

(...)

So I guess you disagree with the previous statement, and I need some elaboration to understand exactly what in this approach works counter to the goals of DT overlays.

It's not like I'm overly infatuated with my own approach, but I want to get to a point where people can start to use these daughterboards no matter whether they use DT or ACPI.

I'm missing ACPI input though, the DT people are more responsive :/

OK no problem there is no immediate hurry.

Yours,
Linus Walleij

Hi Linus,

< snip >

< snip >

My apologies for continuing to not getting to this task. It has been in the top grouping of my todo list but has been pushed aside by a rather busy schedule of conferences and company internal work. It remains up at the top of my todo list, but most likely will still be shunted aside for three more weeks of other pressing work.

I will get to it, I definitely have not forgotten it.

-Frank

It's OK, I might get to rebasing the series even.

We discussed it in a session with Linaro in Vancouver recently (some month ago). What we took away from it is that there are many precedents of populating devices ad hoc, what really makes this one special is that it takes something device tree and ad hoc populates something on top of it, not defined in the device tree itself.

On a related note, Heikki Krogerus is making a software FWnode population series. I ran into this when converting board files to use more abstract descriptions. Check it out:
https://marc.info/?l=linux-kernel&m=154177331200561&w=2

The idea with the series is to expand FWnode so that it doesn't only populate Linux devices from device tree or ACPI but also by software, where needed, further expanding the fwnode abstraction. I will probably look into using that.

Yours,
Linus Walleij

It would be helpful for me to look at that. What was the title of the session, or a pointer to the resource page for it?

-Frank