MikroBUS is an open standard developed by MikroElektronika for connecting
add-on boards to microcontrollers or microprocessors. It essentially
allows you to easily expand the functionality of your main boards using
these add-on boards.
This patchset adds mikroBUS as a Linux bus type and provides a driver to
parse, and flash mikroBUS manifest and register the mikroBUS board.
The v1 and v2 of this patchset was submitted by Vaishnav M A back in
2020. This patchset also includes changes made over the years as part of
BeagleBoards kernel.
Link: https://www.mikroe.com/mikrobus
Link: https://docs.beagleboard.org/latest/boards/beagleplay/
Link: https://lore.kernel.org/lkml/[email protected]/ Patch v2
Changes in v3:
- Use phandle instead of busname for spi
- Use spi board info for registering new device
- Convert dt bindings to yaml
- Add support for clickID
- Code cleanup and style changes
- Additions required to spi, serdev, w1 and regulator subsystems
Changes in v2:
- support for adding mikroBUS ports from DT overlays,
- remove debug sysFS interface for adding mikrobus ports,
- consider extended pin usage/deviations from mikrobus standard
specifications
- use greybus CPort protocol enum instead of new protocol enums
- Fix cases of wrong indentation, ignoring return values, freeing allocated
resources in case of errors and other style suggestions in v1 review.
Ayush Singh (7):
dt-bindings: misc: Add mikrobus-connector
w1: Add w1_find_master_device
spi: Make of_find_spi_controller_by_node() available
regulator: fixed-helper: export regulator_register_always_on
greybus: Add mikroBUS manifest types
mikrobus: Add mikrobus driver
dts: ti: k3-am625-beagleplay: Add mikroBUS
Vaishnav M A (1):
serdev: add of_ helper to get serdev controller
.../bindings/misc/mikrobus-connector.yaml | 110 ++
MAINTAINERS | 7 +
.../arm64/boot/dts/ti/k3-am625-beagleplay.dts | 76 +-
drivers/misc/Kconfig | 1 +
drivers/misc/Makefile | 1 +
drivers/misc/mikrobus/Kconfig | 19 +
drivers/misc/mikrobus/Makefile | 6 +
drivers/misc/mikrobus/mikrobus_core.c | 942 ++++++++++++++++++
drivers/misc/mikrobus/mikrobus_core.h | 201 ++++
drivers/misc/mikrobus/mikrobus_id.c | 229 +++++
drivers/misc/mikrobus/mikrobus_manifest.c | 502 ++++++++++
drivers/misc/mikrobus/mikrobus_manifest.h | 20 +
drivers/regulator/fixed-helper.c | 1 +
drivers/spi/spi.c | 206 ++--
drivers/tty/serdev/core.c | 19 +
drivers/w1/w1.c | 6 +-
drivers/w1/w1_int.c | 27 +
include/linux/greybus/greybus_manifest.h | 49 +
include/linux/serdev.h | 4 +
include/linux/spi/spi.h | 4 +
include/linux/w1.h | 1 +
21 files changed, 2318 insertions(+), 113 deletions(-)
create mode 100644 Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
create mode 100644 drivers/misc/mikrobus/Kconfig
create mode 100644 drivers/misc/mikrobus/Makefile
create mode 100644 drivers/misc/mikrobus/mikrobus_core.c
create mode 100644 drivers/misc/mikrobus/mikrobus_core.h
create mode 100644 drivers/misc/mikrobus/mikrobus_id.c
create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.c
create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.h
base-commit: 61996c073c9b070922ad3a36c981ca6ddbea19a5
--
2.44.0
Add DT bindings for mikroBUS interface. MikroBUS is an open standard
developed by MikroElektronika for connecting add-on boards to
microcontrollers or microprocessors.
Signed-off-by: Ayush Singh <[email protected]>
---
.../bindings/misc/mikrobus-connector.yaml | 110 ++++++++++++++++++
MAINTAINERS | 6 +
2 files changed, 116 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
diff --git a/Documentation/devicetree/bindings/misc/mikrobus-connector.yaml b/Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
new file mode 100644
index 000000000000..6eace2c0dddc
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
@@ -0,0 +1,110 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/mikrobus-connector.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: mikroBUS add-on board socket
+
+maintainers:
+ - Ayush Singh <[email protected]>
+
+properties:
+ compatible:
+ const: mikrobus-connector
+
+ pinctrl-0: true
+ pinctrl-1: true
+ pinctrl-2: true
+ pinctrl-3: true
+ pinctrl-4: true
+ pinctrl-5: true
+ pinctrl-6: true
+ pinctrl-7: true
+ pinctrl-8: true
+
+ pinctrl-names:
+ items:
+ - const: default
+ - const: pwm_default
+ - const: pwm_gpio
+ - const: uart_default
+ - const: uart_gpio
+ - const: i2c_default
+ - const: i2c_gpio
+ - const: spi_default
+ - const: spi_gpio
+
+ mikrobus-gpios:
+ minItems: 11
+ maxItems: 12
+
+ i2c-adapter:
+ description: i2c adapter attached to the mikrobus socket.
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ spi-controller:
+ description: spi bus number of the spi-master attached to the mikrobus socket.
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ uart:
+ description: uart port attached to the mikrobus socket
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+ pwms:
+ description: the pwm-controller corresponding to the mikroBUS PWM pin.
+ maxItems: 1
+
+ spi-cs:
+ description: spi chip-select numbers corresponding to the chip-selects on the mikrobus socket.
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ items:
+ - description: chip select corresponding to CS pin
+ - description: chip select corresponding to RST pin
+
+required:
+ - compatible
+ - pinctrl-0
+ - pinctrl-1
+ - pinctrl-2
+ - pinctrl-3
+ - pinctrl-4
+ - pinctrl-5
+ - pinctrl-6
+ - pinctrl-7
+ - pinctrl-8
+ - i2c-adapter
+ - spi-controller
+ - spi-cs
+ - uart
+ - pwms
+ - mikrobus-gpios
+
+additionalProperties: false
+
+examples:
+ - |
+ mikrobus-0 {
+ compatible = "mikrobus-connector";
+ status = "okay";
+ pinctrl-names = "default", "pwm_default", "pwm_gpio","uart_default", "uart_gpio", "i2c_default",
+ "i2c_gpio", "spi_default", "spi_gpio";
+ pinctrl-0 = <&P2_03_gpio_input_pin &P1_04_gpio_pin &P1_02_gpio_pin>;
+ pinctrl-1 = <&P2_01_pwm_pin>;
+ pinctrl-2 = <&P2_01_gpio_pin>;
+ pinctrl-3 = <&P2_05_uart_pin &P2_07_uart_pin>;
+ pinctrl-4 = <&P2_05_gpio_pin &P2_07_gpio_pin>;
+ pinctrl-5 = <&P2_09_i2c_pin &P2_11_i2c_pin>;
+ pinctrl-6 = <&P2_09_gpio_pin &P2_11_gpio_pin>;
+ pinctrl-7 = <&P1_12_spi_pin &P1_10_spi_pin &P1_08_spi_sclk_pin &P1_06_spi_cs_pin>;
+ pinctrl-8 = <&P1_12_gpio_pin &P1_10_gpio_pin &P1_08_gpio_pin &P1_06_gpio_pin>;
+ i2c-adapter = <&i2c1>;
+ spi-controller = <&spi1>;
+ spi-cs = <0 1>;
+ uart = <&uart1>;
+ pwms = <&ehrpwm1 0 500000 0>;
+ mikrobus-gpios = <&gpio1 18 0> , <&gpio0 23 0>, <&gpio0 30 0> , <&gpio0 31 0>, <&gpio0 15 0>,
+ <&gpio0 14 0>, <&gpio0 4 0> , <&gpio0 3 0>, <&gpio0 2 0>, <&gpio0 5 0>,
+ <&gpio2 25 0>, <&gpio2 3 0>;
+ };
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 375d34363777..69418a058c6b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14767,6 +14767,12 @@ M: Oliver Neukum <[email protected]>
S: Maintained
F: drivers/usb/image/microtek.*
+MIKROBUS
+M: Ayush Singh <[email protected]>
+M: Vaishnav M A <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
+
MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT
M: Luka Kovacic <[email protected]>
M: Luka Perkov <[email protected]>
--
2.44.0
Add helper to find w1_master from w1_bus_master, which is present in
drvdata of platform device.
Signed-off-by: Vaishnav M A <[email protected]>
Signed-off-by: Ayush Singh <[email protected]>
---
drivers/w1/w1.c | 6 +++---
drivers/w1/w1_int.c | 27 +++++++++++++++++++++++++++
include/linux/w1.h | 1 +
3 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index afb1cc4606c5..ce8a3f93f2ef 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -673,9 +673,9 @@ static int __w1_attach_slave_device(struct w1_slave *sl)
sl->dev.of_node = of_find_matching_node(sl->master->dev.of_node,
sl->family->of_match_table);
- dev_set_name(&sl->dev, "%02x-%012llx",
- (unsigned int) sl->reg_num.family,
- (unsigned long long) sl->reg_num.id);
+ dev_set_name(&sl->dev, "%s-%02x-%012llx", sl->master->name,
+ (unsigned int)sl->reg_num.family,
+ (unsigned long long)sl->reg_num.id);
snprintf(&sl->name[0], sizeof(sl->name),
"%02x-%012llx",
(unsigned int) sl->reg_num.family,
diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
index 3a71c5eb2f83..2bfef8e67687 100644
--- a/drivers/w1/w1_int.c
+++ b/drivers/w1/w1_int.c
@@ -242,3 +242,30 @@ void w1_remove_master_device(struct w1_bus_master *bm)
__w1_remove_master_device(found);
}
EXPORT_SYMBOL(w1_remove_master_device);
+
+/**
+ * w1_find_master_device() - find a master device
+ * @bm: master bus device to search
+ */
+struct w1_master *w1_find_master_device(struct w1_bus_master *bm)
+{
+ struct w1_master *dev, *found = NULL;
+
+ list_for_each_entry(dev, &w1_masters, w1_master_entry) {
+ if (!dev->initialized)
+ continue;
+
+ if (dev->bus_master->data == bm->data) {
+ found = dev;
+ break;
+ }
+ }
+
+ if (!found) {
+ pr_err("device doesn't exist.\n");
+ return ERR_PTR(-ENODEV);
+ }
+
+ return found;
+}
+EXPORT_SYMBOL(w1_find_master_device);
diff --git a/include/linux/w1.h b/include/linux/w1.h
index 9a2a0ef39018..24269d0dd5d1 100644
--- a/include/linux/w1.h
+++ b/include/linux/w1.h
@@ -242,6 +242,7 @@ struct w1_master {
int w1_add_master_device(struct w1_bus_master *master);
void w1_remove_master_device(struct w1_bus_master *master);
+struct w1_master *w1_find_master_device(struct w1_bus_master *master);
/**
* struct w1_family_ops - operations for a family type
--
2.44.0
This externalizes and exports the symbol
of_find_spi_controller_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.
Signed-off-by: Ayush Singh <[email protected]>
---
drivers/spi/spi.c | 206 ++++++++++++++++++++--------------------
include/linux/spi/spi.h | 4 +
2 files changed, 108 insertions(+), 102 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index ba4d3fde2054..9ec507d92f80 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2320,6 +2320,93 @@ void spi_flush_queue(struct spi_controller *ctlr)
/*-------------------------------------------------------------------------*/
+static void spi_controller_release(struct device *dev)
+{
+ struct spi_controller *ctlr;
+
+ ctlr = container_of(dev, struct spi_controller, dev);
+ kfree(ctlr);
+}
+
+static struct class spi_master_class = {
+ .name = "spi_master",
+ .dev_release = spi_controller_release,
+ .dev_groups = spi_master_groups,
+};
+
+static ssize_t slave_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct spi_controller *ctlr = container_of(dev, struct spi_controller,
+ dev);
+ struct device *child;
+
+ child = device_find_any_child(&ctlr->dev);
+ return sysfs_emit(buf, "%s\n", child ? to_spi_device(child)->modalias : NULL);
+}
+
+static ssize_t slave_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct spi_controller *ctlr = container_of(dev, struct spi_controller,
+ dev);
+ struct spi_device *spi;
+ struct device *child;
+ char name[32];
+ int rc;
+
+ rc = sscanf(buf, "%31s", name);
+ if (rc != 1 || !name[0])
+ return -EINVAL;
+
+ child = device_find_any_child(&ctlr->dev);
+ if (child) {
+ /* Remove registered slave */
+ device_unregister(child);
+ put_device(child);
+ }
+
+ if (strcmp(name, "(null)")) {
+ /* Register new slave */
+ spi = spi_alloc_device(ctlr);
+ if (!spi)
+ return -ENOMEM;
+
+ strscpy(spi->modalias, name, sizeof(spi->modalias));
+
+ rc = spi_add_device(spi);
+ if (rc) {
+ spi_dev_put(spi);
+ return rc;
+ }
+ }
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(slave);
+
+static struct attribute *spi_slave_attrs[] = {
+ &dev_attr_slave.attr,
+ NULL,
+};
+
+static const struct attribute_group spi_slave_group = {
+ .attrs = spi_slave_attrs,
+};
+
+static const struct attribute_group *spi_slave_groups[] = {
+ &spi_controller_statistics_group,
+ &spi_slave_group,
+ NULL,
+};
+
+static struct class spi_slave_class = {
+ .name = "spi_slave",
+ .dev_release = spi_controller_release,
+ .dev_groups = spi_slave_groups,
+};
+
#if defined(CONFIG_OF)
static void of_spi_parse_dt_cs_delay(struct device_node *nc,
struct spi_delay *delay, const char *prop)
@@ -2543,6 +2630,23 @@ static void of_register_spi_devices(struct spi_controller *ctlr)
}
}
}
+
+/* The spi controllers are not using spi_bus, so we find it with another way */
+struct spi_controller *of_find_spi_controller_by_node(struct device_node *node)
+{
+ struct device *dev;
+
+ dev = class_find_device_by_of_node(&spi_master_class, node);
+ if (!dev && IS_ENABLED(CONFIG_SPI_SLAVE))
+ dev = class_find_device_by_of_node(&spi_slave_class, node);
+ if (!dev)
+ return NULL;
+
+ /* Reference got in class_find_device */
+ return container_of(dev, struct spi_controller, dev);
+}
+EXPORT_SYMBOL_GPL(of_find_spi_controller_by_node);
+
#else
static void of_register_spi_devices(struct spi_controller *ctlr) { }
#endif
@@ -2942,20 +3046,6 @@ static void acpi_register_spi_devices(struct spi_controller *ctlr)
static inline void acpi_register_spi_devices(struct spi_controller *ctlr) {}
#endif /* CONFIG_ACPI */
-static void spi_controller_release(struct device *dev)
-{
- struct spi_controller *ctlr;
-
- ctlr = container_of(dev, struct spi_controller, dev);
- kfree(ctlr);
-}
-
-static struct class spi_master_class = {
- .name = "spi_master",
- .dev_release = spi_controller_release,
- .dev_groups = spi_master_groups,
-};
-
#ifdef CONFIG_SPI_SLAVE
/**
* spi_slave_abort - abort the ongoing transfer request on an SPI slave
@@ -2983,79 +3073,6 @@ int spi_target_abort(struct spi_device *spi)
return -ENOTSUPP;
}
EXPORT_SYMBOL_GPL(spi_target_abort);
-
-static ssize_t slave_show(struct device *dev, struct device_attribute *attr,
- char *buf)
-{
- struct spi_controller *ctlr = container_of(dev, struct spi_controller,
- dev);
- struct device *child;
-
- child = device_find_any_child(&ctlr->dev);
- return sysfs_emit(buf, "%s\n", child ? to_spi_device(child)->modalias : NULL);
-}
-
-static ssize_t slave_store(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct spi_controller *ctlr = container_of(dev, struct spi_controller,
- dev);
- struct spi_device *spi;
- struct device *child;
- char name[32];
- int rc;
-
- rc = sscanf(buf, "%31s", name);
- if (rc != 1 || !name[0])
- return -EINVAL;
-
- child = device_find_any_child(&ctlr->dev);
- if (child) {
- /* Remove registered slave */
- device_unregister(child);
- put_device(child);
- }
-
- if (strcmp(name, "(null)")) {
- /* Register new slave */
- spi = spi_alloc_device(ctlr);
- if (!spi)
- return -ENOMEM;
-
- strscpy(spi->modalias, name, sizeof(spi->modalias));
-
- rc = spi_add_device(spi);
- if (rc) {
- spi_dev_put(spi);
- return rc;
- }
- }
-
- return count;
-}
-
-static DEVICE_ATTR_RW(slave);
-
-static struct attribute *spi_slave_attrs[] = {
- &dev_attr_slave.attr,
- NULL,
-};
-
-static const struct attribute_group spi_slave_group = {
- .attrs = spi_slave_attrs,
-};
-
-static const struct attribute_group *spi_slave_groups[] = {
- &spi_controller_statistics_group,
- &spi_slave_group,
- NULL,
-};
-
-static struct class spi_slave_class = {
- .name = "spi_slave",
- .dev_release = spi_controller_release,
- .dev_groups = spi_slave_groups,
-};
#else
extern struct class spi_slave_class; /* dummy */
#endif
@@ -4749,21 +4766,6 @@ static struct spi_device *of_find_spi_device_by_node(struct device_node *node)
return dev ? to_spi_device(dev) : NULL;
}
-/* 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 device *dev;
-
- dev = class_find_device_by_of_node(&spi_master_class, node);
- if (!dev && IS_ENABLED(CONFIG_SPI_SLAVE))
- dev = class_find_device_by_of_node(&spi_slave_class, node);
- if (!dev)
- return NULL;
-
- /* Reference got in class_find_device */
- return container_of(dev, struct spi_controller, dev);
-}
-
static int of_spi_notify(struct notifier_block *nb, unsigned long action,
void *arg)
{
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index e400d454b3f0..f6fb7bad9a90 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -1684,4 +1684,8 @@ spi_transfer_is_last(struct spi_controller *ctlr, struct spi_transfer *xfer)
return list_is_last(&xfer->transfer_list, &ctlr->cur_msg->transfers);
}
+#if IS_ENABLED(CONFIG_OF)
+struct spi_controller *of_find_spi_controller_by_node(struct device_node *node);
+#endif
+
#endif /* __LINUX_SPI_H */
--
2.44.0
From: Vaishnav M A <[email protected]>
add of_find_serdev_controller_by_node to obtain a
serdev_controller from the device_node, which
can help if the serdev_device is not described
over device tree and instantiation of the device
happens from a different driver, for the same purpose
an option to not delete an empty serdev controller
is added.
Signed-off-by: Vaishnav M A <[email protected]>
Signed-off-by: Ayush Singh <[email protected]>
---
drivers/tty/serdev/core.c | 19 +++++++++++++++++++
include/linux/serdev.h | 4 ++++
2 files changed, 23 insertions(+)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 613cb356b918..5b5b3f2b2e42 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -555,6 +555,19 @@ static int of_serdev_register_devices(struct serdev_controller *ctrl)
return 0;
}
+#if defined(CONFIG_OF)
+struct serdev_controller *of_find_serdev_controller_by_node(struct device_node *node)
+{
+ struct device *dev = bus_find_device_by_of_node(&serdev_bus_type, node);
+
+ if (!dev)
+ return NULL;
+
+ return (dev->type == &serdev_ctrl_type) ? to_serdev_controller(dev) : NULL;
+}
+EXPORT_SYMBOL_GPL(of_find_serdev_controller_by_node);
+#endif
+
#ifdef CONFIG_ACPI
#define SERDEV_ACPI_MAX_SCAN_DEPTH 32
@@ -785,6 +798,12 @@ int serdev_controller_add(struct serdev_controller *ctrl)
pm_runtime_enable(&ctrl->dev);
+ /* provide option to not delete a serdev controller without devices
+ * if property is present
+ */
+ if (device_property_present(&ctrl->dev, "force-empty-serdev-controller"))
+ return 0;
+
ret_of = of_serdev_register_devices(ctrl);
ret_acpi = acpi_serdev_register_devices(ctrl);
if (ret_of && ret_acpi) {
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index ff78efc1f60d..287d7b9bc10a 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -117,6 +117,10 @@ static inline struct serdev_controller *to_serdev_controller(struct device *d)
return container_of(d, struct serdev_controller, dev);
}
+#if defined(CONFIG_OF)
+struct serdev_controller *of_find_serdev_controller_by_node(struct device_node *node);
+#endif
+
static inline void *serdev_device_get_drvdata(const struct serdev_device *serdev)
{
return dev_get_drvdata(&serdev->dev);
--
2.44.0
Export regulator_register_always_on() to allow use in kernel modules
Signed-off-by: Ayush Singh <[email protected]>
---
drivers/regulator/fixed-helper.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/regulator/fixed-helper.c b/drivers/regulator/fixed-helper.c
index 2d5a42b2b3d8..0e9784009c07 100644
--- a/drivers/regulator/fixed-helper.c
+++ b/drivers/regulator/fixed-helper.c
@@ -59,3 +59,4 @@ struct platform_device *regulator_register_always_on(int id, const char *name,
return &data->pdev;
}
+EXPORT_SYMBOL_GPL(regulator_register_always_on);
--
2.44.0
Add data structures for parsing mikroBUS manifests, which are based on
greybus manifest.
Signed-off-by: Ayush Singh <[email protected]>
---
include/linux/greybus/greybus_manifest.h | 49 ++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/include/linux/greybus/greybus_manifest.h b/include/linux/greybus/greybus_manifest.h
index bef9eb2093e9..83241e19d9b3 100644
--- a/include/linux/greybus/greybus_manifest.h
+++ b/include/linux/greybus/greybus_manifest.h
@@ -23,6 +23,9 @@ enum greybus_descriptor_type {
GREYBUS_TYPE_STRING = 0x02,
GREYBUS_TYPE_BUNDLE = 0x03,
GREYBUS_TYPE_CPORT = 0x04,
+ GREYBUS_TYPE_MIKROBUS = 0x05,
+ GREYBUS_TYPE_PROPERTY = 0x06,
+ GREYBUS_TYPE_DEVICE = 0x07,
};
enum greybus_protocol {
@@ -151,6 +154,49 @@ struct greybus_descriptor_cport {
__u8 protocol_id; /* enum greybus_protocol */
} __packed;
+/*
+ * A mikrobus descriptor is used to describe the details
+ * about the bus ocnfiguration for the add-on board
+ * connected to the mikrobus port.
+ */
+struct greybus_descriptor_mikrobus {
+ __u8 pin_state[12];
+} __packed;
+
+/*
+ * A property descriptor is used to pass named properties
+ * to device drivers through the unified device properties
+ * interface under linux/property.h
+ */
+struct greybus_descriptor_property {
+ __u8 length;
+ __u8 id;
+ __u8 propname_stringid;
+ __u8 type;
+ __u8 value[];
+} __packed;
+
+/*
+ * A device descriptor is used to describe the
+ * details required by a add-on board device
+ * driver.
+ */
+struct greybus_descriptor_device {
+ __u8 id;
+ __u8 driver_stringid;
+ __u8 protocol;
+ __u8 reg;
+ __le32 max_speed_hz;
+ __u8 irq;
+ __u8 irq_type;
+ __u8 mode;
+ __u8 prop_link;
+ __u8 gpio_link;
+ __u8 reg_link;
+ __u8 clock_link;
+ __u8 pad[1];
+} __packed;
+
struct greybus_descriptor_header {
__le16 size;
__u8 type; /* enum greybus_descriptor_type */
@@ -164,6 +210,9 @@ struct greybus_descriptor {
struct greybus_descriptor_interface interface;
struct greybus_descriptor_bundle bundle;
struct greybus_descriptor_cport cport;
+ struct greybus_descriptor_mikrobus mikrobus;
+ struct greybus_descriptor_property property;
+ struct greybus_descriptor_device device;
};
} __packed;
--
2.44.0
Add mikroBUS connector support for Beagleplay.
Signed-off-by: Ayush Singh <[email protected]>
---
.../arm64/boot/dts/ti/k3-am625-beagleplay.dts | 76 +++++++++++++++++--
1 file changed, 68 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
index a34e0df2ab86..886308f99d1a 100644
--- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
+++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
@@ -29,6 +29,7 @@ aliases {
i2c3 = &main_i2c3;
i2c4 = &wkup_i2c0;
i2c5 = &mcu_i2c0;
+ mikrobus0 = &mikrobus0;
mmc0 = &sdhci0;
mmc1 = &sdhci1;
mmc2 = &sdhci2;
@@ -230,6 +231,33 @@ simple-audio-card,codec {
};
};
+ mikrobus0: linux-mikrobus {
+ compatible = "mikrobus-connector";
+ pinctrl-names = "default", "pwm_default", "pwm_gpio",
+ "uart_default", "uart_gpio", "i2c_default",
+ "i2c_gpio", "spi_default", "spi_gpio";
+ pinctrl-0 = <&mikrobus_gpio_pins_default>;
+ pinctrl-1 = <&mikrobus_pwm_pins_default>;
+ pinctrl-2 = <&mikrobus_pwm_pins_gpio>;
+ pinctrl-3 = <&mikrobus_uart_pins_default>;
+ pinctrl-4 = <&mikrobus_uart_pins_gpio>;
+ pinctrl-5 = <&mikrobus_i2c_pins_default>;
+ pinctrl-6 = <&mikrobus_i2c_pins_gpio>;
+ pinctrl-7 = <&mikrobus_spi_pins_default>;
+ pinctrl-8 = <&mikrobus_spi_pins_gpio>;
+ i2c-adapter = <&main_i2c3>;
+ spi-controller = <&main_spi2>;
+ spi-cs = <0 1>;
+ uart = <&main_uart5>;
+ pwms = <&ecap2 0 500000 0>;
+ mikrobus-gpios =
+ <&main_gpio1 11 GPIO_ACTIVE_HIGH>, <&main_gpio1 9 GPIO_ACTIVE_HIGH>,
+ <&main_gpio1 24 GPIO_ACTIVE_HIGH>, <&main_gpio1 25 GPIO_ACTIVE_HIGH>,
+ <&main_gpio1 22 GPIO_ACTIVE_HIGH>, <&main_gpio1 23 GPIO_ACTIVE_HIGH>,
+ <&main_gpio1 7 GPIO_ACTIVE_HIGH>, <&main_gpio1 8 GPIO_ACTIVE_HIGH>,
+ <&main_gpio1 14 GPIO_ACTIVE_HIGH>, <&main_gpio1 13 GPIO_ACTIVE_HIGH>,
+ <&main_gpio1 12 GPIO_ACTIVE_HIGH>, <&main_gpio1 10 GPIO_ACTIVE_HIGH>;
+ };
};
&main_pmx0 {
@@ -389,6 +417,18 @@ AM62X_IOPAD(0x01f0, PIN_OUTPUT, 5) /* (A18) EXT_REFCLK1.CLKOUT0 */
>;
};
+ mikrobus_pwm_pins_default: mikrobus-pwm-default-pins {
+ pinctrl-single,pins = <
+ AM62X_IOPAD(0x01a4, PIN_INPUT, 2) /* (B20) MCASP0_ACLKX.ECAP2_IN_APWM_OUT */
+ >;
+ };
+
+ mikrobus_pwm_pins_gpio: mikrobus-pwm-gpio-pins {
+ pinctrl-single,pins = <
+ AM62X_IOPAD(0x01a4, PIN_INPUT, 7) /* (B20) MCASP0_ACLKX.GPIO1_11 */
+ >;
+ };
+
mikrobus_i2c_pins_default: mikrobus-i2c-default-pins {
pinctrl-single,pins = <
AM62X_IOPAD(0x01d0, PIN_INPUT_PULLUP, 2) /* (A15) UART0_CTSn.I2C3_SCL */
@@ -396,6 +436,13 @@ AM62X_IOPAD(0x01d4, PIN_INPUT_PULLUP, 2) /* (B15) UART0_RTSn.I2C3_SDA */
>;
};
+ mikrobus_i2c_pins_gpio: mikrobus-i2c-gpio-pins {
+ pinctrl-single,pins = <
+ AM62X_IOPAD(0x01d0, PIN_INPUT, 7) /* (A15) UART0_CTSn.GPIO1_22 */
+ AM62X_IOPAD(0x01d4, PIN_INPUT, 7) /* (B15) UART0_RTSn.GPIO1_23 */
+ >;
+ };
+
mikrobus_uart_pins_default: mikrobus-uart-default-pins {
pinctrl-single,pins = <
AM62X_IOPAD(0x01d8, PIN_INPUT, 1) /* (C15) MCAN0_TX.UART5_RXD */
@@ -403,6 +450,13 @@ AM62X_IOPAD(0x01dc, PIN_OUTPUT, 1) /* (E15) MCAN0_RX.UART5_TXD */
>;
};
+ mikrobus_uart_pins_gpio: mikrobus-uart-gpio-pins {
+ pinctrl-single,pins = <
+ AM62X_IOPAD(0x01d8, PIN_INPUT, 7) /* (C15) MCAN0_TX.GPIO1_24 */
+ AM62X_IOPAD(0x01dc, PIN_INPUT, 7) /* (E15) MCAN0_RX.GPIO1_25 */
+ >;
+ };
+
mikrobus_spi_pins_default: mikrobus-spi-default-pins {
pinctrl-single,pins = <
AM62X_IOPAD(0x01b0, PIN_INPUT, 1) /* (A20) MCASP0_ACLKR.SPI2_CLK */
@@ -412,6 +466,15 @@ AM62X_IOPAD(0x0198, PIN_INPUT, 1) /* (A19) MCASP0_AXR2.SPI2_D1 */
>;
};
+ mikrobus_spi_pins_gpio: mikrobus-spi-gpio-pins {
+ pinctrl-single,pins = <
+ AM62X_IOPAD(0x0194, PIN_INPUT, 7) /* (B19) MCASP0_AXR3.GPIO1_7 */
+ AM62X_IOPAD(0x0198, PIN_INPUT, 7) /* (A19) MCASP0_AXR2.GPIO1_8 */
+ AM62X_IOPAD(0x01ac, PIN_INPUT, 7) /* (E19) MCASP0_AFSR.GPIO1_13 */
+ AM62X_IOPAD(0x01b0, PIN_INPUT, 7) /* (A20) MCASP0_ACLKR.GPIO1_14 */
+ >;
+ };
+
mikrobus_gpio_pins_default: mikrobus-gpio-default-pins {
bootph-all;
pinctrl-single,pins = <
@@ -629,8 +692,6 @@ &main_gpio0 {
&main_gpio1 {
bootph-all;
- pinctrl-names = "default";
- pinctrl-0 = <&mikrobus_gpio_pins_default>;
gpio-line-names = "", "", "", "", "", /* 0-4 */
"SPE_RSTN", "SPE_INTN", "MIKROBUS_GPIO1_7", /* 5-7 */
"MIKROBUS_GPIO1_8", "MIKROBUS_GPIO1_9", /* 8-9 */
@@ -803,15 +864,11 @@ it66121_out: endpoint {
};
&main_i2c3 {
- pinctrl-names = "default";
- pinctrl-0 = <&mikrobus_i2c_pins_default>;
clock-frequency = <400000>;
status = "okay";
};
&main_spi2 {
- pinctrl-names = "default";
- pinctrl-0 = <&mikrobus_spi_pins_default>;
status = "okay";
};
@@ -875,9 +932,8 @@ &main_uart1 {
};
&main_uart5 {
- pinctrl-names = "default";
- pinctrl-0 = <&mikrobus_uart_pins_default>;
status = "okay";
+ force-empty-serdev-controller;
};
&main_uart6 {
@@ -926,3 +982,7 @@ &mcasp1 {
tx-num-evt = <32>;
rx-num-evt = <32>;
};
+
+&ecap2 {
+ status = "okay";
+};
--
2.44.0
- Setup I2C, SPI, serdev controllers associated with mikrobus connector
- Check if a board with valid mikroBUS manifest is connected
- Parse the manifest and register the device to kernel
Signed-off-by: Vaishnav M A <[email protected]>
Signed-off-by: Ayush Singh <[email protected]>
---
MAINTAINERS | 1 +
drivers/misc/Kconfig | 1 +
drivers/misc/Makefile | 1 +
drivers/misc/mikrobus/Kconfig | 19 +
drivers/misc/mikrobus/Makefile | 6 +
drivers/misc/mikrobus/mikrobus_core.c | 942 ++++++++++++++++++++++
drivers/misc/mikrobus/mikrobus_core.h | 201 +++++
drivers/misc/mikrobus/mikrobus_id.c | 229 ++++++
drivers/misc/mikrobus/mikrobus_manifest.c | 502 ++++++++++++
drivers/misc/mikrobus/mikrobus_manifest.h | 20 +
10 files changed, 1922 insertions(+)
create mode 100644 drivers/misc/mikrobus/Kconfig
create mode 100644 drivers/misc/mikrobus/Makefile
create mode 100644 drivers/misc/mikrobus/mikrobus_core.c
create mode 100644 drivers/misc/mikrobus/mikrobus_core.h
create mode 100644 drivers/misc/mikrobus/mikrobus_id.c
create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.c
create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 69418a058c6b..83bc5e48bef9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14772,6 +14772,7 @@ M: Ayush Singh <[email protected]>
M: Vaishnav M A <[email protected]>
S: Maintained
F: Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
+F: drivers/misc/mikrobus/*
MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT
M: Luka Kovacic <[email protected]>
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 4fb291f0bf7c..3d5c36205c6c 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -591,4 +591,5 @@ source "drivers/misc/cardreader/Kconfig"
source "drivers/misc/uacce/Kconfig"
source "drivers/misc/pvpanic/Kconfig"
source "drivers/misc/mchp_pci1xxxx/Kconfig"
+source "drivers/misc/mikrobus/Kconfig"
endmenu
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index ea6ea5bbbc9c..b9ac88055b87 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -68,3 +68,4 @@ obj-$(CONFIG_TMR_INJECT) += xilinx_tmr_inject.o
obj-$(CONFIG_TPS6594_ESM) += tps6594-esm.o
obj-$(CONFIG_TPS6594_PFSM) += tps6594-pfsm.o
obj-$(CONFIG_NSM) += nsm.o
+obj-y += mikrobus/
diff --git a/drivers/misc/mikrobus/Kconfig b/drivers/misc/mikrobus/Kconfig
new file mode 100644
index 000000000000..f0770006b4fe
--- /dev/null
+++ b/drivers/misc/mikrobus/Kconfig
@@ -0,0 +1,19 @@
+menuconfig MIKROBUS
+ tristate "Module for instantiating devices on mikroBUS ports"
+ depends on GPIOLIB
+ depends on W1
+ depends on W1_MASTER_GPIO
+ help
+ This option enables the mikroBUS driver. mikroBUS is an add-on
+ board socket standard that offers maximum expandability with
+ the smallest number of pins. The mikroBUS driver instantiates
+ devices on a mikroBUS port described by identifying data present
+ in an add-on board resident EEPROM, more details on the mikroBUS
+ driver support and discussion can be found in this eLinux wiki :
+ elinux.org/Mikrobus
+
+
+ Say Y here to enable support for this driver.
+
+ To compile this code as a module, chose M here: the module
+ will be called mikrobus.ko
diff --git a/drivers/misc/mikrobus/Makefile b/drivers/misc/mikrobus/Makefile
new file mode 100644
index 000000000000..c89ff2abb80e
--- /dev/null
+++ b/drivers/misc/mikrobus/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+# mikroBUS Core
+
+obj-$(CONFIG_MIKROBUS) += mikrobus.o
+mikrobus-y := mikrobus_core.o mikrobus_manifest.o
+obj-$(CONFIG_MIKROBUS) += mikrobus_id.o
diff --git a/drivers/misc/mikrobus/mikrobus_core.c b/drivers/misc/mikrobus/mikrobus_core.c
new file mode 100644
index 000000000000..17718ed315b9
--- /dev/null
+++ b/drivers/misc/mikrobus/mikrobus_core.c
@@ -0,0 +1,942 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mikroBUS driver for instantiating add-on
+ * board devices with an identifier EEPROM
+ *
+ * Copyright 2020 Vaishnav M A, BeagleBoard.org Foundation.
+ * Copyright 2024 Ayush Singh <[email protected]>
+ */
+
+#define pr_fmt(fmt) "mikrobus:%s: " fmt, __func__
+
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/idr.h>
+#include <linux/init.h>
+#include <linux/jump_label.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/gpio/consumer.h>
+#include <linux/mutex.h>
+#include <linux/w1.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/gpio.h>
+#include <linux/gpio/machine.h>
+#include <linux/gpio/driver.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/nvmem-provider.h>
+#include <linux/interrupt.h>
+#include <linux/spi/spi.h>
+#include <linux/serdev.h>
+#include <linux/property.h>
+#include <linux/platform_device.h>
+#include <linux/debugfs.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/regulator/fixed.h>
+#include <linux/regulator/machine.h>
+#include <linux/clk-provider.h>
+#include <linux/greybus/greybus_manifest.h>
+#include <linux/of_platform.h>
+#include <linux/pwm.h>
+
+#include "mikrobus_core.h"
+#include "mikrobus_manifest.h"
+
+#define MIKROBUS_ID_EEPROM_MANIFEST_ADDR 0x20
+
+static DEFINE_MUTEX(core_lock);
+static DEFINE_IDR(mikrobus_port_idr);
+static struct class_compat *mikrobus_port_compat_class;
+int __mikrobus_first_dynamic_bus_num;
+static bool is_registered;
+static int mikrobus_port_id_eeprom_probe(struct mikrobus_port *port);
+
+const char *MIKROBUS_PINCTRL_STR[] = { "pwm", "uart", "i2c", "spi" };
+
+const struct bus_type mikrobus_bus_type = {
+ .name = "mikrobus",
+};
+EXPORT_SYMBOL_GPL(mikrobus_bus_type);
+
+int mikrobus_port_scan_eeprom(struct mikrobus_port *port)
+{
+ const u16 manifest_start_addr = MIKROBUS_ID_EEPROM_MANIFEST_ADDR;
+ struct addon_board_info *board;
+ int manifest_size, retval;
+ char header[12], *buf;
+
+ if (port->skip_scan)
+ return -EINVAL;
+
+ retval = nvmem_device_read(port->eeprom, manifest_start_addr, 12, header);
+ if (retval != 12) {
+ return dev_err_probe(&port->dev, -EINVAL, "failed to fetch manifest header %d\n",
+ retval);
+ }
+
+ manifest_size = mikrobus_manifest_header_validate(header, 12);
+ if (manifest_size < 0) {
+ return dev_err_probe(&port->dev, -EINVAL, "invalid manifest size %d\n",
+ manifest_size);
+ }
+
+ buf = kzalloc(manifest_size, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ retval = nvmem_device_read(port->eeprom, manifest_start_addr, manifest_size, buf);
+ if (retval != manifest_size) {
+ retval =
+ dev_err_probe(&port->dev, -EINVAL, "failed to fetch manifest %d\n", retval);
+ goto err_free_buf;
+ }
+
+ board = kzalloc(sizeof(*board), GFP_KERNEL);
+ if (!board) {
+ retval = -ENOMEM;
+ goto err_free_buf;
+ }
+
+ w1_reset_bus(port->w1_master);
+ /* set RST HIGH */
+ gpiod_direction_output(port->gpios->desc[MIKROBUS_PIN_RST], 1);
+ set_bit(W1_ABORT_SEARCH, &port->w1_master->flags);
+
+ INIT_LIST_HEAD(&board->manifest_descs);
+ INIT_LIST_HEAD(&board->devices);
+ retval = mikrobus_manifest_parse(board, buf, manifest_size);
+ if (!retval) {
+ retval = dev_err_probe(&port->dev, -EINVAL, "failed to parse manifest, size %d\n",
+ manifest_size);
+ goto err_free_board;
+ }
+
+ retval = mikrobus_board_register(port, board);
+ if (retval) {
+ dev_err(&port->dev, "failed to register board %s\n", board->name);
+ goto err_free_board;
+ }
+
+ kfree(buf);
+ return 0;
+
+err_free_board:
+ kfree(board);
+err_free_buf:
+ kfree(buf);
+ return retval;
+}
+EXPORT_SYMBOL_GPL(mikrobus_port_scan_eeprom);
+
+static ssize_t name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%s\n", to_mikrobus_port(dev)->name);
+}
+static DEVICE_ATTR_RO(name);
+
+static ssize_t new_device_store(struct device *dev, struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ struct mikrobus_port *port = to_mikrobus_port(dev);
+ struct addon_board_info *board;
+ int retval;
+
+ if (port->board)
+ return dev_err_probe(dev, -EBUSY, "already has board registered\n");
+
+ board = kzalloc(sizeof(*board), GFP_KERNEL);
+ if (!board)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&board->manifest_descs);
+ INIT_LIST_HEAD(&board->devices);
+ retval = mikrobus_manifest_parse(board, (void *)buf, count);
+ if (!retval) {
+ retval = dev_err_probe(dev, -EINVAL, "failed to parse manifest\n");
+ goto err_free_board;
+ }
+
+ retval = mikrobus_board_register(port, board);
+ if (retval) {
+ retval = dev_err_probe(dev, -EINVAL, "failed to register board %s\n", board->name);
+ goto err_free_board;
+ }
+
+ return count;
+
+err_free_board:
+ kfree(board);
+ return retval;
+}
+static DEVICE_ATTR_WO(new_device);
+
+static ssize_t delete_device_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct mikrobus_port *port = to_mikrobus_port(dev);
+ unsigned long id;
+
+ if (kstrtoul(buf, 0, &id))
+ return dev_err_probe(dev, -EINVAL, "cannot parse board id");
+
+ if (!port->board)
+ return dev_err_probe(dev, -ENODEV, "does not have registered boards");
+
+ mikrobus_board_unregister(port, port->board);
+ return count;
+}
+static DEVICE_ATTR_IGNORE_LOCKDEP(delete_device, 0200, NULL, delete_device_store);
+
+static struct attribute *mikrobus_port_attrs[] = { &dev_attr_new_device.attr,
+ &dev_attr_delete_device.attr,
+ &dev_attr_name.attr, NULL };
+ATTRIBUTE_GROUPS(mikrobus_port);
+
+static void mikrobus_port_release(struct device *dev)
+{
+ struct mikrobus_port *port = to_mikrobus_port(dev);
+
+ mutex_lock(&core_lock);
+ idr_remove(&mikrobus_port_idr, port->id);
+ mutex_unlock(&core_lock);
+
+ kfree(port);
+}
+
+const struct device_type mikrobus_port_type = {
+ .groups = mikrobus_port_groups,
+ .release = mikrobus_port_release,
+};
+EXPORT_SYMBOL_GPL(mikrobus_port_type);
+
+static int mikrobus_w1_master_match(struct device *dev, const void *data)
+{
+ struct mikrobus_port *port;
+
+ if (dev->type != &mikrobus_port_type)
+ return 0;
+
+ port = to_mikrobus_port(dev);
+
+ return port->w1_master == data;
+}
+
+struct mikrobus_port *mikrobus_find_port_by_w1_master(struct w1_master *master)
+{
+ struct device *dev;
+
+ dev = bus_find_device(&mikrobus_bus_type, NULL, master, mikrobus_w1_master_match);
+ if (!dev)
+ return NULL;
+
+ return (dev->type == &mikrobus_port_type) ? to_mikrobus_port(dev) : NULL;
+}
+EXPORT_SYMBOL(mikrobus_find_port_by_w1_master);
+
+int mikrobus_port_pinctrl_select(struct mikrobus_port *port)
+{
+ struct pinctrl_state *state;
+ int retval, i;
+
+ for (i = 0; i < MIKROBUS_NUM_PINCTRL_STATE; i++) {
+ state = pinctrl_lookup_state(port->pinctrl, port->pinctrl_selected[i]);
+ if (!IS_ERR(state)) {
+ retval = pinctrl_select_state(port->pinctrl, state);
+ if (retval) {
+ return dev_err_probe(&port->dev, retval,
+ "failed to select state %s\n",
+ port->pinctrl_selected[i]);
+ }
+ dev_dbg(&port->dev, "setting pinctrl %s\n", port->pinctrl_selected[i]);
+ } else {
+ return dev_err_probe(&port->dev, PTR_ERR(state),
+ "failed to find state %s\n",
+ port->pinctrl_selected[i]);
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mikrobus_port_pinctrl_select);
+
+static int mikrobus_port_pinctrl_setup(struct mikrobus_port *port, struct addon_board_info *board)
+{
+ int retval;
+
+ if (board->pin_state[MIKROBUS_PIN_PWM] == MIKROBUS_STATE_PWM)
+ sprintf(port->pinctrl_selected[MIKROBUS_PINCTRL_PWM], "%s_%s",
+ MIKROBUS_PINCTRL_STR[MIKROBUS_PINCTRL_PWM], PINCTRL_STATE_DEFAULT);
+ else
+ sprintf(port->pinctrl_selected[MIKROBUS_PINCTRL_PWM], "%s_%s",
+ MIKROBUS_PINCTRL_STR[MIKROBUS_PINCTRL_PWM], MIKROBUS_PINCTRL_STATE_GPIO);
+
+ if (board->pin_state[MIKROBUS_PIN_RX] == MIKROBUS_STATE_UART)
+ sprintf(port->pinctrl_selected[MIKROBUS_PINCTRL_UART], "%s_%s",
+ MIKROBUS_PINCTRL_STR[MIKROBUS_PINCTRL_UART], PINCTRL_STATE_DEFAULT);
+ else
+ sprintf(port->pinctrl_selected[MIKROBUS_PINCTRL_UART], "%s_%s",
+ MIKROBUS_PINCTRL_STR[MIKROBUS_PINCTRL_UART], MIKROBUS_PINCTRL_STATE_GPIO);
+
+ if (board->pin_state[MIKROBUS_PIN_SCL] == MIKROBUS_STATE_I2C)
+ sprintf(port->pinctrl_selected[MIKROBUS_PINCTRL_I2C], "%s_%s",
+ MIKROBUS_PINCTRL_STR[MIKROBUS_PINCTRL_I2C], PINCTRL_STATE_DEFAULT);
+ else
+ sprintf(port->pinctrl_selected[MIKROBUS_PINCTRL_I2C], "%s_%s",
+ MIKROBUS_PINCTRL_STR[MIKROBUS_PINCTRL_I2C], MIKROBUS_PINCTRL_STATE_GPIO);
+
+ if (board->pin_state[MIKROBUS_PIN_MOSI] == MIKROBUS_STATE_SPI)
+ sprintf(port->pinctrl_selected[MIKROBUS_PINCTRL_SPI], "%s_%s",
+ MIKROBUS_PINCTRL_STR[MIKROBUS_PINCTRL_SPI], PINCTRL_STATE_DEFAULT);
+ else
+ sprintf(port->pinctrl_selected[MIKROBUS_PINCTRL_SPI], "%s_%s",
+ MIKROBUS_PINCTRL_STR[MIKROBUS_PINCTRL_SPI], MIKROBUS_PINCTRL_STATE_GPIO);
+
+ retval = mikrobus_port_pinctrl_select(port);
+ if (retval)
+ dev_err(&port->dev, "failed to select pinctrl states [%d]", retval);
+
+ return retval;
+}
+
+static int mikrobus_irq_get(struct mikrobus_port *port, int irqno, int irq_type)
+{
+ int irq;
+
+ if (irqno > port->gpios->ndescs - 1)
+ return dev_err_probe(&port->dev, -ENODEV, "GPIO %d does not exist", irqno);
+
+ irq = gpiod_to_irq(port->gpios->desc[irqno]);
+ if (irq < 0)
+ return dev_err_probe(&port->dev, -EINVAL, "could not get irq %d", irqno);
+
+ irq_set_irq_type(irq, irq_type);
+
+ return irq;
+}
+
+static int mikrobus_gpio_setup(struct gpio_desc *gpio, int gpio_state)
+{
+ switch (gpio_state) {
+ case MIKROBUS_STATE_INPUT:
+ return gpiod_direction_input(gpio);
+ case MIKROBUS_STATE_OUTPUT_HIGH:
+ return gpiod_direction_output(gpio, 1);
+ case MIKROBUS_STATE_OUTPUT_LOW:
+ return gpiod_direction_output(gpio, 0);
+ case MIKROBUS_STATE_PWM:
+ case MIKROBUS_STATE_SPI:
+ case MIKROBUS_STATE_I2C:
+ default:
+ return 0;
+ }
+}
+
+static char *mikrobus_gpio_chip_name_get(struct mikrobus_port *port, int gpio)
+{
+ struct gpio_chip *gpiochip;
+
+ if (gpio > port->gpios->ndescs - 1)
+ return NULL;
+
+ gpiochip = gpiod_to_chip(port->gpios->desc[gpio]);
+ return kmemdup(gpiochip->label, MIKROBUS_NAME_SIZE, GFP_KERNEL);
+}
+
+static int mikrobus_gpio_hwnum_get(struct mikrobus_port *port, int gpio)
+{
+ struct gpio_chip *gpiochip;
+
+ if (gpio > port->gpios->ndescs - 1)
+ return -ENODEV;
+
+ gpiochip = gpiod_to_chip(port->gpios->desc[gpio]);
+ return desc_to_gpio(port->gpios->desc[gpio]) - gpiochip->base;
+}
+
+static void mikrobus_board_device_release_all(struct addon_board_info *info)
+{
+ struct board_device_info *dev, *next;
+
+ list_for_each_entry_safe(dev, next, &info->devices, links) {
+ list_del(&dev->links);
+ kfree(dev);
+ }
+}
+
+static int mikrobus_device_register(struct mikrobus_port *port, struct board_device_info *dev,
+ char *board_name)
+{
+ struct regulator_consumer_supply regulator;
+ struct gpiod_lookup_table *lookup;
+ char devname[MIKROBUS_NAME_SIZE];
+ struct spi_board_info *spi_info;
+ struct i2c_board_info *i2c_info;
+ struct serdev_device *serdev;
+ struct platform_device *pdev;
+ struct fwnode_handle *fwnode;
+ struct spi_device *spi;
+ int i, retval;
+ u64 *val;
+
+ dev_info(&port->dev, "registering device : %s", dev->drv_name);
+
+ if (dev->gpio_lookup) {
+ lookup = dev->gpio_lookup;
+
+ switch (dev->protocol) {
+ case GREYBUS_PROTOCOL_SPI:
+ snprintf(devname, sizeof(devname), "%s.%u", dev_name(&port->spi_ctrl->dev),
+ port->chip_select[dev->reg]);
+ lookup->dev_id = kmemdup(devname, MIKROBUS_NAME_SIZE, GFP_KERNEL);
+ break;
+ case GREYBUS_PROTOCOL_RAW:
+ snprintf(devname, sizeof(devname), "%s.%u", dev->drv_name, dev->reg);
+ lookup->dev_id = kmemdup(devname, MIKROBUS_NAME_SIZE, GFP_KERNEL);
+ break;
+ default:
+ lookup->dev_id = dev->drv_name;
+ }
+
+ dev_info(&port->dev, "adding lookup table : %s", lookup->dev_id);
+
+ for (i = 0; i < dev->num_gpio_resources; i++) {
+ lookup->table[i].key =
+ mikrobus_gpio_chip_name_get(port, lookup->table[i].chip_hwnum);
+ lookup->table[i].chip_hwnum =
+ mikrobus_gpio_hwnum_get(port, lookup->table[i].chip_hwnum);
+ }
+
+ gpiod_add_lookup_table(lookup);
+ }
+
+ if (dev->regulators) {
+ if (dev->protocol == GREYBUS_PROTOCOL_SPI) {
+ snprintf(devname, sizeof(devname), "%s.%u", dev_name(&port->spi_ctrl->dev),
+ port->chip_select[dev->reg]);
+ regulator.dev_name = kmemdup(devname, MIKROBUS_NAME_SIZE, GFP_KERNEL);
+ } else if (dev->protocol == GREYBUS_PROTOCOL_RAW) {
+ snprintf(devname, sizeof(devname), "%s.%u", dev->drv_name, dev->reg);
+ regulator.dev_name = kmemdup(devname, MIKROBUS_NAME_SIZE, GFP_KERNEL);
+ } else {
+ regulator.dev_name = dev->drv_name;
+ }
+
+ for (i = 0; i < dev->num_regulators; i++) {
+ val = dev->regulators[i].value.u64_data;
+ regulator.supply =
+ kmemdup(dev->regulators[i].name, MIKROBUS_NAME_SIZE, GFP_KERNEL);
+ dev_info(&port->dev, "adding fixed regulator %llu uv, %s for %s", *val,
+ regulator.supply, regulator.dev_name);
+ regulator_register_always_on(0, dev->regulators[i].name, ®ulator, 1,
+ *val);
+ }
+ }
+
+ switch (dev->protocol) {
+ case GREYBUS_PROTOCOL_SPI:
+ spi_info = kzalloc(sizeof(*spi_info), GFP_KERNEL);
+ strscpy_pad(spi_info->modalias, dev->drv_name, sizeof(spi_info->modalias) - 1);
+ if (dev->irq)
+ spi_info->irq = mikrobus_irq_get(port, dev->irq, dev->irq_type);
+ if (dev->properties) {
+ fwnode = fwnode_create_software_node(dev->properties, NULL);
+ spi_info->swnode = to_software_node(fwnode);
+ }
+ spi_info->chip_select = port->chip_select[dev->reg];
+ spi_info->max_speed_hz = dev->max_speed_hz;
+ spi_info->mode = dev->mode;
+
+ spi = spi_new_device(port->spi_ctrl, spi_info);
+ if (!spi)
+ return dev_err_probe(&port->dev, -ENOMEM, "failed to create spi device");
+
+ if (dev->clocks) {
+ for (i = 0; i < dev->num_clocks; i++) {
+ val = dev->clocks[i].value.u64_data;
+ dev_info(&port->dev, "adding fixed clock %s, %llu hz",
+ dev->clocks[i].name, *val);
+ clk_register_fixed_rate(&spi->dev, dev->clocks[i].name, devname, 0,
+ *val);
+ }
+ }
+
+ dev->dev_client = (void *)spi;
+ break;
+ case GREYBUS_PROTOCOL_I2C:
+ i2c_info = kzalloc(sizeof(*i2c_info), GFP_KERNEL);
+ if (!i2c_info)
+ return -ENOMEM;
+
+ strscpy_pad(i2c_info->type, dev->drv_name, sizeof(i2c_info->type) - 1);
+ if (dev->irq)
+ i2c_info->irq = mikrobus_irq_get(port, dev->irq, dev->irq_type);
+ if (dev->properties) {
+ fwnode = fwnode_create_software_node(dev->properties, NULL);
+ i2c_info->swnode = to_software_node(fwnode);
+ }
+ i2c_info->addr = dev->reg;
+ dev->dev_client = (void *)i2c_new_client_device(port->i2c_adap, i2c_info);
+ break;
+ case GREYBUS_PROTOCOL_RAW:
+ pdev = platform_device_alloc(dev->drv_name, 0);
+ if (!pdev)
+ return -ENOMEM;
+
+ if (dev->properties) {
+ retval = device_create_managed_software_node(&pdev->dev, dev->properties,
+ NULL);
+ if (retval)
+ return dev_err_probe(&port->dev, retval,
+ "failed to create software node");
+ }
+ dev->dev_client = pdev;
+ platform_device_add(dev->dev_client);
+ break;
+ case GREYBUS_PROTOCOL_UART:
+ serdev = serdev_device_alloc(port->ser_ctrl);
+ if (!serdev)
+ return -ENOMEM;
+
+ if (dev->properties)
+ device_create_managed_software_node(&serdev->dev, dev->properties, NULL);
+
+ dev->dev_client = serdev;
+ serdev_device_add(serdev);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void mikrobus_device_unregister(struct mikrobus_port *port, struct board_device_info *dev,
+ char *board_name)
+{
+ dev_info(&port->dev, "removing device %s\n", dev->drv_name);
+ if (dev->gpio_lookup) {
+ gpiod_remove_lookup_table(dev->gpio_lookup);
+ kfree(dev->gpio_lookup);
+ }
+
+ kfree(dev->properties);
+
+ switch (dev->protocol) {
+ case GREYBUS_PROTOCOL_SPI:
+ spi_unregister_device((struct spi_device *)dev->dev_client);
+ break;
+ case GREYBUS_PROTOCOL_I2C:
+ i2c_unregister_device((struct i2c_client *)dev->dev_client);
+ break;
+ case GREYBUS_PROTOCOL_RAW:
+ platform_device_unregister((struct platform_device *)dev->dev_client);
+ break;
+ case GREYBUS_PROTOCOL_UART:
+ serdev_device_remove((struct serdev_device *)dev->dev_client);
+ break;
+ }
+}
+
+int mikrobus_board_register(struct mikrobus_port *port, struct addon_board_info *board)
+{
+ struct board_device_info *devinfo, *next;
+ int retval, i;
+
+ if (WARN_ON(list_empty(&board->devices)))
+ return false;
+
+ if (port->pinctrl) {
+ retval = mikrobus_port_pinctrl_setup(port, board);
+ if (retval)
+ dev_err(&port->dev, "failed to setup pinctrl state [%d]", retval);
+ }
+
+ if (port->gpios) {
+ for (i = 0; i < port->gpios->ndescs; i++) {
+ retval = mikrobus_gpio_setup(port->gpios->desc[i], board->pin_state[i]);
+ if (retval)
+ dev_err(&port->dev, "failed to setup gpio %d, state %d", i,
+ board->pin_state[i]);
+
+ gpiochip_free_own_desc(port->gpios->desc[i]);
+ }
+ }
+
+ list_for_each_entry_safe(devinfo, next, &board->devices, links)
+ mikrobus_device_register(port, devinfo, board->name);
+
+ port->board = board;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mikrobus_board_register);
+
+void mikrobus_board_unregister(struct mikrobus_port *port, struct addon_board_info *board)
+{
+ struct board_device_info *devinfo, *next;
+
+ if (WARN_ON(list_empty(&board->devices)))
+ return;
+
+ list_for_each_entry_safe(devinfo, next, &board->devices, links)
+ mikrobus_device_unregister(port, devinfo, board->name);
+
+ mikrobus_board_device_release_all(board);
+ kfree(board);
+ port->board = NULL;
+}
+EXPORT_SYMBOL_GPL(mikrobus_board_unregister);
+
+static int mikrobus_port_id_eeprom_probe(struct mikrobus_port *port)
+{
+ static const char drvname[MIKROBUS_NAME_SIZE] = "w1-gpio\0";
+ struct platform_device *mikrobus_id_eeprom_w1_device;
+ struct gpiod_lookup_table *lookup;
+ char devname[MIKROBUS_NAME_SIZE];
+ struct w1_bus_master *bm;
+ int retval;
+
+ sprintf(port->pinctrl_selected[MIKROBUS_PINCTRL_SPI], "%s_%s",
+ MIKROBUS_PINCTRL_STR[MIKROBUS_PINCTRL_SPI], MIKROBUS_PINCTRL_STATE_GPIO);
+
+ retval = mikrobus_port_pinctrl_select(port);
+
+ /* set RST LOW */
+ gpiod_direction_output(port->gpios->desc[MIKROBUS_PIN_RST], 0);
+
+ lookup = kzalloc(struct_size(lookup, table, 1), GFP_KERNEL);
+ if (!lookup)
+ return -ENOMEM;
+
+ retval = snprintf(devname, sizeof(devname), "%s.%u", drvname, port->id);
+ lookup->dev_id = kmemdup(devname, retval + 1, GFP_KERNEL);
+ lookup->table[0].key = mikrobus_gpio_chip_name_get(port, MIKROBUS_PIN_CS);
+ lookup->table[0].flags = GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN;
+ lookup->table[0].chip_hwnum = mikrobus_gpio_hwnum_get(port, MIKROBUS_PIN_CS);
+
+ gpiochip_free_own_desc(port->gpios->desc[MIKROBUS_PIN_CS]);
+ gpiod_add_lookup_table(lookup);
+
+ mikrobus_id_eeprom_w1_device = platform_device_register_simple(drvname, port->id, NULL, 0);
+ if (IS_ERR(mikrobus_id_eeprom_w1_device)) {
+ retval = PTR_ERR(mikrobus_id_eeprom_w1_device);
+ dev_err(&port->dev, "failed to register w1-gpio device %d", retval);
+ goto early_exit;
+ }
+
+ port->w1_gpio = mikrobus_id_eeprom_w1_device;
+
+ bm = (struct w1_bus_master *)platform_get_drvdata(mikrobus_id_eeprom_w1_device);
+ if (!bm) {
+ dev_err(&port->dev, "failed to get w1_bus_master");
+ return 0;
+ }
+
+ port->w1_master = w1_find_master_device(bm);
+ if (!port->w1_master) {
+ dev_err(&port->dev, "failed to find W1 GPIO master, port [%s]", port->name);
+ goto early_exit;
+ }
+
+ port->w1_master->search_count = 4;
+
+ return 0;
+
+early_exit:
+ gpiod_remove_lookup_table(lookup);
+ kfree(lookup);
+ return -ENODEV;
+}
+
+int mikrobus_port_register(struct mikrobus_port *port)
+{
+ struct device *dev = &port->dev;
+ int retval, id;
+
+ if (WARN_ON(!is_registered))
+ return -EAGAIN;
+
+ if (dev->of_node) {
+ id = of_alias_get_id(dev->of_node, "mikrobus");
+ if (id >= 0) {
+ port->id = id;
+ mutex_lock(&core_lock);
+ id = idr_alloc(&mikrobus_port_idr, port, port->id, port->id + 1,
+ GFP_KERNEL);
+ mutex_unlock(&core_lock);
+ if (WARN(id < 0, "couldn't get idr"))
+ return id == -ENOSPC ? -EBUSY : id;
+ }
+ } else {
+ mutex_lock(&core_lock);
+ id = idr_alloc(&mikrobus_port_idr, port, __mikrobus_first_dynamic_bus_num, 0,
+ GFP_KERNEL);
+ mutex_unlock(&core_lock);
+ if (id < 0)
+ return id;
+
+ port->id = id;
+ }
+
+ port->dev.bus = &mikrobus_bus_type;
+ port->dev.type = &mikrobus_port_type;
+ strscpy_pad(port->name, "mikrobus-port", sizeof(port->name) - 1);
+ dev_set_name(&port->dev, "mikrobus-%d", port->id);
+ dev_info(&port->dev, "registering port mikrobus-%d ", port->id);
+
+ retval = device_register(&port->dev);
+ if (retval) {
+ dev_err(&port->dev, "port '%d': can't register device (%d)", port->id, retval);
+ put_device(&port->dev);
+ return retval;
+ }
+
+ retval = class_compat_create_link(mikrobus_port_compat_class, &port->dev, port->dev.parent);
+ if (retval)
+ dev_warn(&port->dev, "failed to create compatibility class link");
+
+ if (!port->w1_master) {
+ dev_info(&port->dev, "mikrobus port %d eeprom empty probing default eeprom",
+ port->id);
+ mutex_lock(&core_lock);
+ retval = mikrobus_port_id_eeprom_probe(port);
+ mutex_unlock(&core_lock);
+ }
+
+ return retval;
+}
+EXPORT_SYMBOL_GPL(mikrobus_port_register);
+
+void mikrobus_port_delete(struct mikrobus_port *port)
+{
+ struct mikrobus_port *found;
+
+ mutex_lock(&core_lock);
+ found = idr_find(&mikrobus_port_idr, port->id);
+ mutex_unlock(&core_lock);
+ if (found != port) {
+ pr_err("port [%s] not registered", port->name);
+ return;
+ }
+
+ if (port->board) {
+ dev_err(&port->dev, "attempting to delete port with registered boards, port [%s]\n",
+ port->name);
+ return;
+ }
+
+ if (port->eeprom) {
+ nvmem_device_put(port->eeprom);
+ platform_device_unregister(port->w1_gpio);
+ }
+
+ class_compat_remove_link(mikrobus_port_compat_class, &port->dev, port->dev.parent);
+ device_unregister(&port->dev);
+ mutex_lock(&core_lock);
+ idr_remove(&mikrobus_port_idr, port->id);
+ mutex_unlock(&core_lock);
+ memset(&port->dev, 0, sizeof(port->dev));
+}
+EXPORT_SYMBOL_GPL(mikrobus_port_delete);
+
+static int mikrobus_port_probe_pinctrl_setup(struct mikrobus_port *port)
+{
+ struct device *dev = port->dev.parent;
+ struct pinctrl_state *state;
+ int retval, i;
+
+ state = pinctrl_lookup_state(port->pinctrl, PINCTRL_STATE_DEFAULT);
+ if (!IS_ERR(state)) {
+ retval = pinctrl_select_state(port->pinctrl, state);
+ if (retval) {
+ return dev_err_probe(dev, retval, "Failed to select state %s\n",
+ PINCTRL_STATE_DEFAULT);
+ }
+ } else {
+ return dev_err_probe(dev, PTR_ERR(state), "failed to find state %s\n",
+ PINCTRL_STATE_DEFAULT);
+ }
+
+ for (i = 0; i < MIKROBUS_NUM_PINCTRL_STATE; i++) {
+ port->pinctrl_selected[i] = kmalloc(MIKROBUS_PINCTRL_NAME_SIZE, GFP_KERNEL);
+ sprintf(port->pinctrl_selected[i], "%s_%s", MIKROBUS_PINCTRL_STR[i],
+ PINCTRL_STATE_DEFAULT);
+ }
+
+ retval = mikrobus_port_pinctrl_select(port);
+ if (retval)
+ dev_err(dev, "failed to select pinctrl states [%d]", retval);
+
+ return retval;
+}
+
+static int mikrobus_port_probe(struct platform_device *pdev)
+{
+ struct device_node *i2c_adap_np, *uart_np, *spi_np;
+ struct device *dev = &pdev->dev;
+ struct mikrobus_port *port;
+ int retval;
+
+ port = kzalloc(sizeof(*port), GFP_KERNEL);
+ if (!port)
+ return -ENOMEM;
+
+ /* I2C setup */
+ i2c_adap_np = of_parse_phandle(dev->of_node, "i2c-adapter", 0);
+ if (!i2c_adap_np) {
+ retval = dev_err_probe(dev, -ENODEV, "cannot parse i2c-adapter");
+ goto err_port;
+ }
+ port->i2c_adap = of_find_i2c_adapter_by_node(i2c_adap_np);
+ of_node_put(i2c_adap_np);
+
+ /* GPIO setup */
+ port->gpios = gpiod_get_array(dev, "mikrobus", GPIOD_OUT_LOW);
+ if (IS_ERR(port->gpios)) {
+ retval = PTR_ERR(port->gpios);
+ dev_err(dev, "failed to get gpio array [%d]", retval);
+ goto err_port;
+ }
+
+ /* SPI setup */
+ spi_np = of_parse_phandle(dev->of_node, "spi-controller", 0);
+ if (!spi_np) {
+ retval = dev_err_probe(dev, -ENODEV, "cannot parse spi-controller");
+ goto err_port;
+ }
+ port->spi_ctrl = of_find_spi_controller_by_node(spi_np);
+ of_node_put(spi_np);
+ if (!port->spi_ctrl) {
+ retval = dev_err_probe(dev, -ENODEV, "cannot find spi controller");
+ goto err_port;
+ }
+ retval = device_property_read_u32_array(dev, "spi-cs", port->chip_select, MIKROBUS_NUM_CS);
+ if (retval) {
+ dev_err(dev, "failed to get spi-cs [%d]", retval);
+ goto err_port;
+ }
+
+ /* UART setup */
+ uart_np = of_parse_phandle(dev->of_node, "uart", 0);
+ if (!uart_np) {
+ retval = dev_err_probe(dev, -ENODEV, "cannot parse uart");
+ goto err_port;
+ }
+ port->ser_ctrl = of_find_serdev_controller_by_node(uart_np);
+ of_node_put(uart_np);
+ if (!port->ser_ctrl) {
+ retval = dev_err_probe(dev, -ENODEV, "cannot find uart controller");
+ goto err_port;
+ }
+
+ /* PWM setup */
+ port->pwm = devm_pwm_get(dev, NULL);
+ if (!port->pwm) {
+ retval = dev_err_probe(dev, -ENODEV, "cannot find pwm controller");
+ goto err_port;
+ }
+
+ /* pinctrl setup */
+ port->pinctrl = devm_pinctrl_get(dev);
+ if (IS_ERR(port->pinctrl)) {
+ retval = PTR_ERR(port->pinctrl);
+ dev_err(dev, "failed to get pinctrl [%d]", retval);
+ goto err_port;
+ }
+ port->dev.parent = dev;
+ port->dev.of_node = pdev->dev.of_node;
+ retval = mikrobus_port_probe_pinctrl_setup(port);
+ if (retval) {
+ dev_err(dev, "failed to setup pinctrl [%d]", retval);
+ goto err_port;
+ }
+
+ retval = mikrobus_port_register(port);
+ if (retval) {
+ dev_err(dev, "port : can't register port [%d]", retval);
+ goto err_port;
+ }
+
+ platform_set_drvdata(pdev, port);
+
+ return 0;
+
+err_port:
+ kfree(port);
+ return retval;
+}
+
+static int mikrobus_port_remove(struct platform_device *pdev)
+{
+ struct mikrobus_port *port = platform_get_drvdata(pdev);
+
+ mikrobus_port_delete(port);
+ return 0;
+}
+
+static const struct of_device_id mikrobus_port_of_match[] = {
+ { .compatible = "mikrobus-connector" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mikrobus_port_of_match);
+
+static struct platform_driver mikrobus_port_driver = {
+ .probe = mikrobus_port_probe,
+ .remove = mikrobus_port_remove,
+ .driver = {
+ .name = "mikrobus",
+ .of_match_table = of_match_ptr(mikrobus_port_of_match),
+ },
+};
+
+static int mikrobus_init(void)
+{
+ int retval;
+
+ retval = bus_register(&mikrobus_bus_type);
+ if (retval) {
+ pr_err("bus_register failed (%d)\n", retval);
+ return retval;
+ }
+
+ mikrobus_port_compat_class = class_compat_register("mikrobus-port");
+ if (!mikrobus_port_compat_class) {
+ pr_err("class_compat register failed (%d)\n", retval);
+ retval = -ENOMEM;
+ goto class_err;
+ }
+
+ retval = of_alias_get_highest_id("mikrobus");
+ if (retval >= __mikrobus_first_dynamic_bus_num)
+ __mikrobus_first_dynamic_bus_num = retval + 1;
+
+ is_registered = true;
+ retval = platform_driver_register(&mikrobus_port_driver);
+ if (retval)
+ pr_err("driver register failed [%d]\n", retval);
+
+ return retval;
+
+class_err:
+ bus_unregister(&mikrobus_bus_type);
+ idr_destroy(&mikrobus_port_idr);
+ is_registered = false;
+ return retval;
+}
+subsys_initcall(mikrobus_init);
+
+static void mikrobus_exit(void)
+{
+ platform_driver_unregister(&mikrobus_port_driver);
+ bus_unregister(&mikrobus_bus_type);
+ class_compat_unregister(mikrobus_port_compat_class);
+ idr_destroy(&mikrobus_port_idr);
+}
+module_exit(mikrobus_exit);
+
+MODULE_AUTHOR("Vaishnav M A <[email protected]>");
+MODULE_AUTHOR("Ayush Singh <[email protected]>");
+MODULE_DESCRIPTION("mikroBUS main module");
+MODULE_LICENSE("GPL");
diff --git a/drivers/misc/mikrobus/mikrobus_core.h b/drivers/misc/mikrobus/mikrobus_core.h
new file mode 100644
index 000000000000..8bd101828964
--- /dev/null
+++ b/drivers/misc/mikrobus/mikrobus_core.h
@@ -0,0 +1,201 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * mikroBUS Driver for instantiating add-on
+ * board devices with an identifier EEPROM
+ *
+ * Copyright 2020 Vaishnav M A, BeagleBoard.org Foundation.
+ */
+
+#ifndef __MIKROBUS_H
+#define __MIKROBUS_H
+
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
+#include <linux/spi/spi.h>
+#include <linux/serdev.h>
+#include <linux/property.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/nvmem-provider.h>
+
+#define MIKROBUS_VERSION_MAJOR 0x00
+#define MIKROBUS_VERSION_MINOR 0x03
+
+#define MIKROBUS_NAME_SIZE 40
+#define MIKROBUS_PINCTRL_NAME_SIZE 20
+
+#define MIKROBUS_NUM_PINCTRL_STATE 4
+#define MIKROBUS_NUM_CS 2
+
+#define MIKROBUS_PINCTRL_PWM 0
+#define MIKROBUS_PINCTRL_UART 1
+#define MIKROBUS_PINCTRL_I2C 2
+#define MIKROBUS_PINCTRL_SPI 3
+
+#define MIKROBUS_PINCTRL_STATE_GPIO "gpio"
+
+#define MIKROBUS_EEPROM_EXIT_ID_CMD 0xD2
+
+extern const struct bus_type mikrobus_bus_type;
+extern const struct device_type mikrobus_port_type;
+extern const char *MIKROBUS_PINCTRL_STR[];
+
+enum mikrobus_property_type {
+ MIKROBUS_PROPERTY_TYPE_MIKROBUS = 0x00,
+ MIKROBUS_PROPERTY_TYPE_PROPERTY = 0x01,
+ MIKROBUS_PROPERTY_TYPE_GPIO = 0x02,
+ MIKROBUS_PROPERTY_TYPE_U8 = 0x03,
+ MIKROBUS_PROPERTY_TYPE_U16 = 0x04,
+ MIKROBUS_PROPERTY_TYPE_U32 = 0x05,
+ MIKROBUS_PROPERTY_TYPE_U64 = 0x06,
+ MIKROBUS_PROPERTY_TYPE_REGULATOR = 0x07,
+ MIKROBUS_PROPERTY_TYPE_CLOCK = 0x08,
+};
+
+enum mikrobus_pin {
+ MIKROBUS_PIN_PWM = 0x00,
+ MIKROBUS_PIN_INT = 0x01,
+ MIKROBUS_PIN_RX = 0x02,
+ MIKROBUS_PIN_TX = 0x03,
+ MIKROBUS_PIN_SCL = 0x04,
+ MIKROBUS_PIN_SDA = 0x05,
+ MIKROBUS_PIN_MOSI = 0x06,
+ MIKROBUS_PIN_MISO = 0x07,
+ MIKROBUS_PIN_SCK = 0x08,
+ MIKROBUS_PIN_CS = 0x09,
+ MIKROBUS_PIN_RST = 0x0A,
+ MIKROBUS_PIN_AN = 0x0B,
+ MIKROBUS_PORT_PIN_COUNT = 0x0C,
+};
+
+enum mikrobus_pin_state {
+ MIKROBUS_STATE_INPUT = 0x01,
+ MIKROBUS_STATE_OUTPUT_HIGH = 0x02,
+ MIKROBUS_STATE_OUTPUT_LOW = 0x03,
+ MIKROBUS_STATE_PWM = 0x04,
+ MIKROBUS_STATE_SPI = 0x05,
+ MIKROBUS_STATE_I2C = 0x06,
+ MIKROBUS_STATE_UART = 0x07,
+};
+
+/*
+ * board_device_info describes a single device on a mikrobus add-on
+ * board, an add-on board can present one or more device to the host
+ *
+ * @gpio_lookup: used to provide the GPIO lookup table for
+ * passing the named GPIOs to device drivers.
+ * @properties: used to provide the property_entry to pass named
+ * properties to device drivers, applicable only when driver uses
+ * device_property_read_* calls to fetch the properties.
+ * @num_gpio_resources: number of named gpio resources for the device,
+ * used mainly for gpiod_lookup_table memory allocation.
+ * @num_properties: number of custom properties for the device,
+ * used mainly for property_entry memory allocation.
+ * @protocol: used to know the type of the device and it should
+ * contain one of the values defined under 'enum greybus_class_type'
+ * under linux/greybus/greybus_manifest.h
+ * @reg: I2C address for the device, for devices on the SPI bus
+ * this field is the chip select address relative to the mikrobus
+ * port:0->device chip select connected to CS pin on mikroBUS port
+ * 1->device chip select connected to RST Pin on mikroBUS port
+ * @mode: SPI mode
+ * @max_speed_hz: SPI max speed(Hz)
+ * @drv_name: device_id to match with the driver
+ * @irq_type: type of IRQ trigger , match with defines in linux/interrupt.h
+ * @irq: irq number relative to the mikrobus port should contain one of the
+ * values defined under 'enum mikrobus_pin'
+ * @id: device id starting from 1
+ */
+struct board_device_info {
+ struct gpiod_lookup_table *gpio_lookup;
+ struct property_entry *properties;
+ struct property_entry *regulators;
+ struct property_entry *clocks;
+ struct list_head links;
+ unsigned short num_gpio_resources;
+ unsigned short num_properties;
+ unsigned short num_regulators;
+ unsigned short num_clocks;
+ unsigned short protocol;
+ unsigned short reg;
+ unsigned int mode;
+ void *dev_client;
+ u32 max_speed_hz;
+ char *drv_name;
+ int irq_type;
+ int irq;
+ int id;
+};
+
+/*
+ * addon_board_info describes a mikrobus add-on device the add-on
+ * board, an add-on board can present one or more device to the host
+ *
+ * @manifest_descs: list of manifest descriptors
+ * @devices: list of devices on the board
+ * @pin_state: the state of each pin on the mikrobus port required
+ * for the add-on board should contain one of the values defined under
+ * 'enum mikrobus_pin_state' restrictions are as per mikrobus standard
+ * specifications.
+ * @name: add-on board name
+ */
+struct addon_board_info {
+ struct list_head manifest_descs;
+ struct list_head devices;
+ u8 pin_state[MIKROBUS_PORT_PIN_COUNT];
+ char *name;
+};
+
+/*
+ * mikrobus_port describes the peripherals mapped to a
+ * mikrobus port.
+ *
+ * @eeprom_client: i2c_client corresponding to the eeprom
+ * on the add-on board.
+ * @board: pointer to the attached add-on board.
+ * @i2c_adap: I2C adapter attached to the mikrobus port.
+ * @spi_mstr: SPI master attached to the mikrobus port.
+ * @eeprom: nvmem_device for the eeprom on the add-on board.
+ * @pwm: pwm_device attached to the mikrobus port PWM pin.
+ * @pinctrl_selected: current pinctrl_selected state.
+ * @chip_select: chip select number mapped to the SPI
+ * CS pin on the mikrobus port and the RST pin on the mikrobus
+ * port
+ * @id: port id starting from 1
+ */
+struct mikrobus_port {
+ struct addon_board_info *board;
+ struct nvmem_device *eeprom;
+ struct i2c_adapter *i2c_adap;
+ struct spi_controller *spi_ctrl;
+ struct w1_master *w1_master;
+ struct platform_device *w1_gpio;
+ struct serdev_controller *ser_ctrl;
+ struct gpio_descs *gpios;
+ struct pwm_device *pwm;
+ struct pinctrl *pinctrl;
+ struct module *owner;
+ struct device dev;
+ char name[MIKROBUS_NAME_SIZE];
+ char *pinctrl_selected[MIKROBUS_NUM_PINCTRL_STATE];
+ unsigned int chip_select[MIKROBUS_NUM_CS];
+ int skip_scan;
+ int id;
+};
+
+#define to_mikrobus_port(d) container_of(d, struct mikrobus_port, dev)
+
+void mikrobus_board_unregister(struct mikrobus_port *port, struct addon_board_info *board);
+int mikrobus_board_register(struct mikrobus_port *port, struct addon_board_info *board);
+int mikrobus_port_register(struct mikrobus_port *port);
+int mikrobus_port_pinctrl_select(struct mikrobus_port *port);
+void mikrobus_port_delete(struct mikrobus_port *port);
+int mikrobus_port_scan_eeprom(struct mikrobus_port *port);
+struct mikrobus_port *mikrobus_find_port_by_w1_master(struct w1_master *master);
+#endif /* __MIKROBUS_H */
diff --git a/drivers/misc/mikrobus/mikrobus_id.c b/drivers/misc/mikrobus/mikrobus_id.c
new file mode 100644
index 000000000000..42a0a558785d
--- /dev/null
+++ b/drivers/misc/mikrobus/mikrobus_id.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * mikrobus_id.c - w1 mikroBUS ID family EEPROM driver
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/types.h>
+#include <linux/delay.h>
+
+#include <linux/crc16.h>
+#include <linux/w1.h>
+#include <linux/nvmem-provider.h>
+
+#include "mikrobus_core.h"
+
+#define W1_EEPROM_MIKROBUS_ID 0xCC
+#define W1_MIKROBUS_ID_EEPROM_SIZE 0x0200
+#define W1_MIKROBUS_ID_EEPROM_PAGE_SIZE 32
+#define W1_MIKROBUS_ID_READ_EEPROM 0x69
+#define W1_MIKROBUS_ID_WRITE_EEPROM 0x96
+#define W1_MIKROBUS_ID_RELEASE_EEPROM 0xAA
+#define W1_MIKROBUS_ID_EEPROM_READ_RETRIES 10
+
+#define W1_MIKROBUS_EEPROM_MANIFEST_START_PAGE 1
+
+static ssize_t mikrobus_manifest_store(struct device *device, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ u8 write_request[] = { W1_MIKROBUS_ID_WRITE_EEPROM,
+ W1_MIKROBUS_EEPROM_MANIFEST_START_PAGE };
+ u8 release_command = W1_MIKROBUS_ID_RELEASE_EEPROM;
+ struct w1_slave *sl = dev_to_w1_slave(device);
+ u16 crc, crc_read, pos = 0;
+ u8 status = 0;
+ int cnt;
+
+ if (count > W1_MIKROBUS_ID_EEPROM_SIZE)
+ return -ENOMEM;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ pr_info("mikrobus_id: writing manifest size = %lu bytes", count);
+ while (pos < count) {
+ if (w1_reset_select_slave(sl))
+ break;
+
+ w1_write_block(sl->master, write_request, sizeof(write_request));
+ crc = crc16(0, write_request, sizeof(write_request)) ^ 0xFFFF;
+ w1_read_block(sl->master, (u8 *)&crc_read, sizeof(crc_read));
+
+ if (crc != crc_read)
+ break;
+
+ for (cnt = 0; cnt < W1_MIKROBUS_ID_EEPROM_PAGE_SIZE; cnt++)
+ w1_write_8(sl->master, (u8)buf[cnt]);
+
+ crc = crc16(0, buf, W1_MIKROBUS_ID_EEPROM_PAGE_SIZE) ^ 0xFFFF;
+ usleep_range(1 * USEC_PER_MSEC, 2 * USEC_PER_MSEC);
+ w1_read_block(sl->master, (u8 *)&crc_read, sizeof(crc_read));
+
+ if (crc != crc_read)
+ break;
+
+ w1_write_8(sl->master, release_command);
+ usleep_range(10 * USEC_PER_MSEC, 15 * USEC_PER_MSEC);
+ status = w1_read_8(sl->master);
+ w1_read_block(sl->master, (u8 *)&crc_read, sizeof(crc_read));
+ crc = crc16(0, (u8 *)&release_command, sizeof(release_command)) ^ 0xFFFF;
+
+ if (status != W1_MIKROBUS_ID_RELEASE_EEPROM)
+ break;
+
+ if (crc != crc_read)
+ break;
+
+ buf += W1_MIKROBUS_ID_EEPROM_PAGE_SIZE;
+ pos += W1_MIKROBUS_ID_EEPROM_PAGE_SIZE;
+ write_request[1]++;
+ }
+
+ pr_info("mikrobus_id: manifest written bytes: %d", pos);
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return count > pos ? count : pos;
+}
+static DEVICE_ATTR_WO(mikrobus_manifest);
+
+static struct attribute *w1_mikrobus_attrs[] = { &dev_attr_mikrobus_manifest.attr, NULL };
+
+ATTRIBUTE_GROUPS(w1_mikrobus);
+
+static int w1_mikrobus_id_readpage(struct w1_slave *sl, int pageaddr, char *buf)
+{
+ u8 crc_rdbuf[2];
+
+ if (w1_reset_select_slave(sl))
+ return -1;
+
+ w1_write_8(sl->master, W1_MIKROBUS_ID_READ_EEPROM);
+ w1_write_8(sl->master, pageaddr);
+ w1_read_block(sl->master, crc_rdbuf, 2);
+ w1_write_8(sl->master, W1_MIKROBUS_ID_RELEASE_EEPROM);
+ usleep_range(10 * USEC_PER_MSEC, 15 * USEC_PER_MSEC);
+ w1_read_block(sl->master, crc_rdbuf, 1);
+ w1_read_block(sl->master, buf, W1_MIKROBUS_ID_EEPROM_PAGE_SIZE);
+ w1_read_block(sl->master, crc_rdbuf, 2);
+
+ return 0;
+}
+
+static int w1_mikrobus_id_readbuf(struct w1_slave *sl, int count, int off, char *buf)
+{
+ int len = count - (count % W1_MIKROBUS_ID_EEPROM_PAGE_SIZE);
+ u8 pageaddr = off / W1_MIKROBUS_ID_EEPROM_PAGE_SIZE;
+ u8 temp_rdbuf[W1_MIKROBUS_ID_EEPROM_PAGE_SIZE];
+ int iter, index, ret;
+
+ while (len > 0) {
+ ret = w1_mikrobus_id_readpage(
+ sl, pageaddr, buf + (W1_MIKROBUS_ID_EEPROM_PAGE_SIZE * pageaddr - off));
+ pageaddr += 1;
+ len -= W1_MIKROBUS_ID_EEPROM_PAGE_SIZE;
+ }
+
+ if (count % W1_MIKROBUS_ID_EEPROM_PAGE_SIZE) {
+ ret = w1_mikrobus_id_readpage(sl, pageaddr, temp_rdbuf);
+ for (iter = W1_MIKROBUS_ID_EEPROM_PAGE_SIZE * pageaddr - off, index = 0;
+ iter < count; iter++, index++)
+ buf[iter] = temp_rdbuf[index];
+ }
+
+ return ret;
+}
+
+static int w1_mikrobus_id_readblock(struct w1_slave *sl, int off, int count, char *buf)
+{
+ int tries = W1_MIKROBUS_ID_EEPROM_READ_RETRIES;
+ int ret = -EINVAL;
+ u8 *cmp;
+
+ cmp = kzalloc(count, GFP_KERNEL);
+ if (!cmp)
+ return -ENOMEM;
+
+ do {
+ w1_mikrobus_id_readbuf(sl, count, off, buf);
+
+ w1_mikrobus_id_readbuf(sl, count, off, cmp);
+ if (!memcmp(cmp, buf, count)) {
+ ret = 0;
+ break;
+ }
+ } while (--tries);
+
+ kfree(cmp);
+ return ret;
+}
+
+static int w1_mikrobus_id_nvmem_read(void *priv, unsigned int off, void *buf, size_t count)
+{
+ struct w1_slave *sl = priv;
+ int ret;
+
+ mutex_lock(&sl->master->bus_mutex);
+ ret = w1_mikrobus_id_readblock(sl, off, count, buf);
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
+static int w1_mikrobus_id_add_slave(struct w1_slave *sl)
+{
+ struct nvmem_device *nvmem;
+ struct mikrobus_port *port;
+ struct nvmem_config nvmem_cfg = {
+ .dev = &sl->dev,
+ .reg_read = w1_mikrobus_id_nvmem_read,
+ .type = NVMEM_TYPE_EEPROM,
+ .read_only = true,
+ .word_size = 1,
+ .stride = 1,
+ .size = W1_MIKROBUS_ID_EEPROM_SIZE,
+ .priv = sl,
+ };
+
+ port = mikrobus_find_port_by_w1_master(sl->master);
+ if (!port)
+ return -ENODEV;
+
+ nvmem_cfg.name = port->name;
+ nvmem = devm_nvmem_register(&sl->dev, &nvmem_cfg);
+ if (IS_ERR(nvmem))
+ return PTR_ERR(nvmem);
+
+ port->eeprom = nvmem;
+ mikrobus_port_scan_eeprom(port);
+
+ return 0;
+}
+
+static const struct w1_family_ops w1_family_mikrobus_id_fops = { .add_slave =
+ w1_mikrobus_id_add_slave,
+ .groups = w1_mikrobus_groups };
+
+static struct w1_family w1_family_mikrobus_id = {
+ .fid = W1_EEPROM_MIKROBUS_ID,
+ .fops = &w1_family_mikrobus_id_fops,
+};
+
+static int w1_mikrobusid_init(void)
+{
+ return w1_register_family(&w1_family_mikrobus_id);
+}
+
+static void w1_mikrobusid_exit(void)
+{
+ w1_unregister_family(&w1_family_mikrobus_id);
+}
+
+module_init(w1_mikrobusid_init);
+module_exit(w1_mikrobusid_exit);
+
+MODULE_AUTHOR("Vaishnav M A <[email protected]>");
+MODULE_DESCRIPTION("w1 family CC driver for mikroBUS ID EEPROM");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("w1-family-" __stringify(W1_EEPROM_MIKROBUS_ID));
diff --git a/drivers/misc/mikrobus/mikrobus_manifest.c b/drivers/misc/mikrobus/mikrobus_manifest.c
new file mode 100644
index 000000000000..3121a1d01b8b
--- /dev/null
+++ b/drivers/misc/mikrobus/mikrobus_manifest.c
@@ -0,0 +1,502 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mikroBUS manifest parsing, an
+ * extension to Greybus Manifest Parsing
+ * under drivers/greybus/manifest.c
+ *
+ * Copyright 2014-2015 Google Inc.
+ * Copyright 2014-2015 Linaro Ltd.
+ */
+
+#define pr_fmt(fmt) "mikrobus_manifest:%s: " fmt, __func__
+
+#include <linux/bits.h>
+#include <linux/types.h>
+#include <linux/property.h>
+#include <linux/greybus/greybus_manifest.h>
+
+#include "mikrobus_manifest.h"
+
+struct manifest_desc {
+ struct list_head links;
+ size_t size;
+ void *data;
+ enum greybus_descriptor_type type;
+};
+
+static void manifest_descriptor_release_all(struct addon_board_info *board)
+{
+ struct manifest_desc *descriptor, *next;
+
+ list_for_each_entry_safe(descriptor, next, &board->manifest_descs, links) {
+ list_del(&descriptor->links);
+ kfree(descriptor);
+ }
+}
+
+static int board_descriptor_add(struct addon_board_info *board, struct greybus_descriptor *desc,
+ size_t size)
+{
+ struct greybus_descriptor_header *desc_header = &desc->header;
+ struct manifest_desc *descriptor;
+ size_t desc_size, expected_size;
+
+ if (size < sizeof(*desc_header)) {
+ pr_err("short descriptor (%zu < %zu)", size, sizeof(*desc_header));
+ return -EINVAL;
+ }
+
+ desc_size = le16_to_cpu(desc_header->size);
+ if (desc_size > size) {
+ pr_err("incorrect descriptor size (%zu != %zu)", size, desc_size);
+ return -EINVAL;
+ }
+
+ expected_size = sizeof(*desc_header);
+ switch (desc_header->type) {
+ case GREYBUS_TYPE_STRING:
+ expected_size += sizeof(struct greybus_descriptor_string);
+ expected_size += desc->string.length;
+ expected_size = ALIGN(expected_size, 4);
+ break;
+ case GREYBUS_TYPE_PROPERTY:
+ expected_size += sizeof(struct greybus_descriptor_property);
+ expected_size += desc->property.length;
+ expected_size = ALIGN(expected_size, 4);
+ break;
+ case GREYBUS_TYPE_DEVICE:
+ expected_size += sizeof(struct greybus_descriptor_device);
+ break;
+ case GREYBUS_TYPE_MIKROBUS:
+ expected_size += sizeof(struct greybus_descriptor_mikrobus);
+ break;
+ case GREYBUS_TYPE_INTERFACE:
+ expected_size += sizeof(struct greybus_descriptor_interface);
+ break;
+ case GREYBUS_TYPE_CPORT:
+ expected_size += sizeof(struct greybus_descriptor_cport);
+ break;
+ case GREYBUS_TYPE_BUNDLE:
+ expected_size += sizeof(struct greybus_descriptor_bundle);
+ break;
+ case GREYBUS_TYPE_INVALID:
+ default:
+ pr_err("invalid descriptor type %d", desc_header->type);
+ return -EINVAL;
+ }
+
+ descriptor = kzalloc(sizeof(*descriptor), GFP_KERNEL);
+ if (!descriptor)
+ return -ENOMEM;
+
+ descriptor->size = desc_size;
+ descriptor->data = (char *)desc + sizeof(*desc_header);
+ descriptor->type = desc_header->type;
+ list_add_tail(&descriptor->links, &board->manifest_descs);
+
+ return desc_size;
+}
+
+static char *mikrobus_string_get(struct addon_board_info *board, u8 string_id)
+{
+ struct greybus_descriptor_string *desc_string;
+ struct manifest_desc *descriptor;
+ bool found = false;
+ char *string;
+
+ if (!string_id)
+ return NULL;
+
+ list_for_each_entry(descriptor, &board->manifest_descs, links) {
+ if (descriptor->type != GREYBUS_TYPE_STRING)
+ continue;
+
+ desc_string = descriptor->data;
+ if (desc_string->id == string_id) {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found)
+ return ERR_PTR(-ENOENT);
+
+ string = kmemdup(&desc_string->string, desc_string->length + 1, GFP_KERNEL);
+ if (!string)
+ return ERR_PTR(-ENOMEM);
+
+ string[desc_string->length] = '\0';
+
+ return string;
+}
+
+static void mikrobus_state_get(struct addon_board_info *board)
+{
+ struct greybus_descriptor_mikrobus *mikrobus;
+ struct greybus_descriptor_interface *interface;
+ struct manifest_desc *descriptor;
+ bool found = false;
+ size_t i;
+
+ list_for_each_entry(descriptor, &board->manifest_descs, links) {
+ if (descriptor->type == GREYBUS_TYPE_MIKROBUS) {
+ mikrobus = descriptor->data;
+ found = true;
+ break;
+ }
+ }
+
+ if (!found) {
+ pr_err("mikrobus descriptor not found");
+ return;
+ }
+
+ for (i = 0; i < MIKROBUS_PORT_PIN_COUNT; i++)
+ board->pin_state[i] = mikrobus->pin_state[i];
+
+ found = false;
+ list_for_each_entry(descriptor, &board->manifest_descs, links) {
+ if (descriptor->type == GREYBUS_TYPE_INTERFACE) {
+ interface = descriptor->data;
+ found = true;
+ break;
+ }
+ }
+
+ if (!found) {
+ pr_err("interface descriptor not found");
+ return;
+ }
+
+ board->name = mikrobus_string_get(board, interface->product_stringid);
+}
+
+static struct property_entry *mikrobus_property_entry_get(struct addon_board_info *board,
+ u8 *prop_link, int num_properties)
+{
+ struct greybus_descriptor_property *desc_property;
+ struct manifest_desc *descriptor;
+ struct property_entry *properties;
+ bool found = false;
+ char *prop_name;
+ int i, retval;
+ u64 *val_u64;
+ u32 *val_u32;
+ u16 *val_u16;
+ u8 *val_u8;
+
+ properties = kcalloc(num_properties, sizeof(*properties), GFP_KERNEL);
+ if (!properties)
+ return ERR_PTR(-ENOMEM);
+
+ for (i = 0; i < num_properties; i++) {
+ list_for_each_entry(descriptor, &board->manifest_descs, links) {
+ if (descriptor->type != GREYBUS_TYPE_PROPERTY)
+ continue;
+
+ desc_property = descriptor->data;
+ if (desc_property->id == prop_link[i]) {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found) {
+ retval = -ENOENT;
+ goto early_exit;
+ }
+
+ prop_name = mikrobus_string_get(board, desc_property->propname_stringid);
+ if (!prop_name) {
+ retval = -ENOENT;
+ goto early_exit;
+ }
+
+ switch (desc_property->type) {
+ case MIKROBUS_PROPERTY_TYPE_U8:
+ val_u8 = kmemdup(&desc_property->value,
+ (desc_property->length) * sizeof(u8), GFP_KERNEL);
+ if (desc_property->length == 1)
+ properties[i] = PROPERTY_ENTRY_U8(prop_name, *val_u8);
+ else
+ properties[i] = PROPERTY_ENTRY_U8_ARRAY_LEN(
+ prop_name, (void *)desc_property->value,
+ desc_property->length);
+ break;
+ case MIKROBUS_PROPERTY_TYPE_U16:
+ val_u16 = kmemdup(&desc_property->value,
+ (desc_property->length) * sizeof(u16), GFP_KERNEL);
+ if (desc_property->length == 1)
+ properties[i] = PROPERTY_ENTRY_U16(prop_name, *val_u16);
+ else
+ properties[i] = PROPERTY_ENTRY_U16_ARRAY_LEN(
+ prop_name, (void *)desc_property->value,
+ desc_property->length);
+ break;
+ case MIKROBUS_PROPERTY_TYPE_U32:
+ val_u32 = kmemdup(&desc_property->value,
+ (desc_property->length) * sizeof(u32), GFP_KERNEL);
+ if (desc_property->length == 1)
+ properties[i] = PROPERTY_ENTRY_U32(prop_name, *val_u32);
+ else
+ properties[i] = PROPERTY_ENTRY_U32_ARRAY_LEN(
+ prop_name, (void *)desc_property->value,
+ desc_property->length);
+ break;
+ case MIKROBUS_PROPERTY_TYPE_U64:
+ val_u64 = kmemdup(&desc_property->value,
+ (desc_property->length) * sizeof(u64), GFP_KERNEL);
+ if (desc_property->length == 1)
+ properties[i] = PROPERTY_ENTRY_U64(prop_name, *val_u64);
+ else
+ properties[i] = PROPERTY_ENTRY_U64_ARRAY_LEN(
+ prop_name, (void *)desc_property->value,
+ desc_property->length);
+ break;
+ default:
+ retval = -EINVAL;
+ goto early_exit;
+ }
+ }
+ return properties;
+
+early_exit:
+ kfree(properties);
+ return ERR_PTR(retval);
+}
+
+static u8 *mikrobus_property_link_get(struct addon_board_info *board, u8 prop_id,
+ struct board_device_info *board_dev, u8 prop_type)
+{
+ struct greybus_descriptor_property *desc_property;
+ struct manifest_desc *descriptor;
+ bool found = false;
+ u8 *val_u8;
+
+ if (!prop_id)
+ return NULL;
+
+ list_for_each_entry(descriptor, &board->manifest_descs, links) {
+ if (descriptor->type != GREYBUS_TYPE_PROPERTY)
+ continue;
+
+ desc_property = descriptor->data;
+ if (desc_property->id == prop_id && desc_property->type == prop_type) {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found)
+ return ERR_PTR(-ENOENT);
+
+ val_u8 = kmemdup(&desc_property->value, desc_property->length, GFP_KERNEL);
+ if (prop_type == MIKROBUS_PROPERTY_TYPE_GPIO)
+ board_dev->num_gpio_resources = desc_property->length;
+ else if (prop_type == MIKROBUS_PROPERTY_TYPE_PROPERTY)
+ board_dev->num_properties = desc_property->length;
+ else if (prop_type == MIKROBUS_PROPERTY_TYPE_REGULATOR)
+ board_dev->num_regulators = desc_property->length;
+ else if (prop_type == MIKROBUS_PROPERTY_TYPE_CLOCK)
+ board_dev->num_clocks = desc_property->length;
+
+ return val_u8;
+}
+
+static int mikrobus_manifest_attach_device(struct addon_board_info *board,
+ struct greybus_descriptor_device *dev_desc)
+{
+ u8 *gpio_desc_link, *prop_link, *gpioval, *reg_link, *clock_link;
+ struct greybus_descriptor_property *desc_property;
+ struct board_device_info *board_dev;
+ struct gpiod_lookup_table *lookup;
+ struct manifest_desc *descriptor;
+ int retval, i;
+
+ board_dev = kzalloc(sizeof(*board_dev), GFP_KERNEL);
+ if (!board_dev)
+ return -ENOMEM;
+
+ board_dev->id = dev_desc->id;
+ board_dev->drv_name = mikrobus_string_get(board, dev_desc->driver_stringid);
+ if (!board_dev->drv_name) {
+ retval = -ENOENT;
+ goto err_free_board_dev;
+ }
+
+ board_dev->protocol = dev_desc->protocol;
+ board_dev->reg = dev_desc->reg;
+ board_dev->irq = dev_desc->irq;
+ board_dev->irq_type = dev_desc->irq_type;
+ board_dev->max_speed_hz = le32_to_cpu(dev_desc->max_speed_hz);
+ board_dev->mode = dev_desc->mode;
+ pr_info("parsed device %d, driver=%s", board_dev->id, board_dev->drv_name);
+
+ if (dev_desc->prop_link > 0) {
+ prop_link = mikrobus_property_link_get(board, dev_desc->prop_link, board_dev,
+ MIKROBUS_PROPERTY_TYPE_PROPERTY);
+ if (!prop_link) {
+ retval = -ENOENT;
+ goto err_free_board_dev;
+ }
+
+ pr_info("device %d, number of properties=%d", board_dev->id,
+ board_dev->num_properties);
+ board_dev->properties =
+ mikrobus_property_entry_get(board, prop_link, board_dev->num_properties);
+ }
+
+ if (dev_desc->gpio_link > 0) {
+ gpio_desc_link = mikrobus_property_link_get(board, dev_desc->gpio_link, board_dev,
+ MIKROBUS_PROPERTY_TYPE_GPIO);
+ if (!gpio_desc_link) {
+ retval = -ENOENT;
+ goto err_free_board_dev;
+ }
+
+ pr_info("device %d, number of gpio resource=%d", board_dev->id,
+ board_dev->num_gpio_resources);
+ lookup = kzalloc(struct_size(lookup, table, board_dev->num_gpio_resources),
+ GFP_KERNEL);
+ if (!lookup) {
+ retval = -ENOMEM;
+ goto err_free_board_dev;
+ }
+
+ for (i = 0; i < board_dev->num_gpio_resources; i++) {
+ list_for_each_entry(descriptor, &board->manifest_descs, links) {
+ if (descriptor->type != GREYBUS_TYPE_PROPERTY)
+ continue;
+
+ desc_property = descriptor->data;
+ if (desc_property->id == gpio_desc_link[i]) {
+ gpioval = desc_property->value;
+ lookup->table[i].chip_hwnum = gpioval[0];
+ lookup->table[i].flags = gpioval[1];
+ lookup->table[i].con_id = mikrobus_string_get(
+ board, desc_property->propname_stringid);
+ break;
+ }
+ }
+ }
+ board_dev->gpio_lookup = lookup;
+ }
+
+ if (dev_desc->reg_link > 0) {
+ reg_link = mikrobus_property_link_get(board, dev_desc->reg_link, board_dev,
+ MIKROBUS_PROPERTY_TYPE_REGULATOR);
+ if (!reg_link) {
+ retval = -ENOENT;
+ goto err_free_board_dev;
+ }
+ pr_info("device %d, number of regulators=%d", board_dev->id,
+ board_dev->num_regulators);
+ board_dev->regulators =
+ mikrobus_property_entry_get(board, reg_link, board_dev->num_regulators);
+ }
+
+ if (dev_desc->clock_link > 0) {
+ clock_link = mikrobus_property_link_get(board, dev_desc->clock_link, board_dev,
+ MIKROBUS_PROPERTY_TYPE_CLOCK);
+ if (!clock_link) {
+ retval = -ENOENT;
+ goto err_free_board_dev;
+ }
+ pr_info("device %d, number of clocks=%d", board_dev->id, board_dev->num_clocks);
+ board_dev->clocks =
+ mikrobus_property_entry_get(board, clock_link, board_dev->num_clocks);
+ }
+
+ list_add_tail(&board_dev->links, &board->devices);
+ return 0;
+
+err_free_board_dev:
+ kfree(board_dev);
+ return retval;
+}
+
+static int mikrobus_manifest_parse_devices(struct addon_board_info *board)
+{
+ struct greybus_descriptor_device *desc_device;
+ struct manifest_desc *desc, *next;
+ int retval, devcount = 0;
+
+ list_for_each_entry_safe(desc, next, &board->manifest_descs, links) {
+ if (desc->type != GREYBUS_TYPE_DEVICE)
+ continue;
+
+ desc_device = desc->data;
+ retval = mikrobus_manifest_attach_device(board, desc_device);
+ devcount++;
+ }
+
+ return devcount;
+}
+
+int mikrobus_manifest_parse(struct addon_board_info *board, void *data, size_t size)
+{
+ struct greybus_manifest_header *header;
+ struct greybus_manifest *manifest;
+ struct greybus_descriptor *desc;
+ int dev_count, desc_size;
+ u16 manifest_size;
+
+ if (size < sizeof(*header)) {
+ pr_err("short manifest (%zu < %zu)", size, sizeof(*header));
+ return -EINVAL;
+ }
+
+ manifest = data;
+ header = &manifest->header;
+ manifest_size = le16_to_cpu(header->size);
+
+ if (manifest_size != size) {
+ pr_err("invalid manifest size(%zu < %u)", size, manifest_size);
+ return -EINVAL;
+ }
+
+ if (header->version_major > MIKROBUS_VERSION_MAJOR) {
+ pr_err("manifest version too new (%u.%u > %u.%u)", header->version_major,
+ header->version_minor, MIKROBUS_VERSION_MAJOR, MIKROBUS_VERSION_MINOR);
+ return -EINVAL;
+ }
+
+ desc = manifest->descriptors;
+ size -= sizeof(*header);
+ while (size) {
+ desc_size = board_descriptor_add(board, desc, size);
+ if (desc_size < 0) {
+ pr_err("invalid manifest descriptor, size: %u", desc_size);
+ return -EINVAL;
+ }
+
+ desc = (void *)desc + desc_size;
+ size -= desc_size;
+ }
+
+ mikrobus_state_get(board);
+ dev_count = mikrobus_manifest_parse_devices(board);
+ pr_info(" %s manifest parsed with %d devices", board->name, dev_count);
+ manifest_descriptor_release_all(board);
+
+ return true;
+}
+
+size_t mikrobus_manifest_header_validate(void *data, size_t size)
+{
+ struct greybus_manifest_header *header = data;
+ u16 manifest_size = le16_to_cpu(header->size);
+
+ if (manifest_size < sizeof(*header)) {
+ pr_err("short manifest (%zu < %zu)", size, sizeof(*header));
+ return -EINVAL;
+ }
+
+ if (header->version_major > MIKROBUS_VERSION_MAJOR) {
+ pr_err("manifest version too new (%u.%u > %u.%u)", header->version_major,
+ header->version_minor, MIKROBUS_VERSION_MAJOR, MIKROBUS_VERSION_MINOR);
+ return -EINVAL;
+ }
+
+ return manifest_size;
+}
diff --git a/drivers/misc/mikrobus/mikrobus_manifest.h b/drivers/misc/mikrobus/mikrobus_manifest.h
new file mode 100644
index 000000000000..36b64b2093f5
--- /dev/null
+++ b/drivers/misc/mikrobus/mikrobus_manifest.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * mikroBUS manifest definition
+ * extension to Greybus Manifest Definition
+ *
+ * Copyright 2014-2015 Google Inc.
+ * Copyright 2014-2015 Linaro Ltd.
+ *
+ * Released under the GPLv2 and BSD licenses.
+ */
+
+#ifndef __MIKROBUS_MANIFEST_H
+#define __MIKROBUS_MANIFEST_H
+
+#include "mikrobus_core.h"
+
+int mikrobus_manifest_parse(struct addon_board_info *info, void *data, size_t size);
+size_t mikrobus_manifest_header_validate(void *data, size_t size);
+
+#endif /* __MIKROBUS_MANIFEST_H */
--
2.44.0
On Sat, Mar 16, 2024 at 12:19:05AM +0530, Ayush Singh wrote:
> + if (dev->regulators) {
> + if (dev->protocol == GREYBUS_PROTOCOL_SPI) {
> + snprintf(devname, sizeof(devname), "%s.%u", dev_name(&port->spi_ctrl->dev),
> + port->chip_select[dev->reg]);
> + regulator.dev_name = kmemdup(devname, MIKROBUS_NAME_SIZE, GFP_KERNEL);
> + } else if (dev->protocol == GREYBUS_PROTOCOL_RAW) {
It looks like you're trying to write a switch statement here...
> + for (i = 0; i < dev->num_regulators; i++) {
> + val = dev->regulators[i].value.u64_data;
> + regulator.supply =
> + kmemdup(dev->regulators[i].name, MIKROBUS_NAME_SIZE, GFP_KERNEL);
> + dev_info(&port->dev, "adding fixed regulator %llu uv, %s for %s", *val,
> + regulator.supply, regulator.dev_name);
> + regulator_register_always_on(0, dev->regulators[i].name, ®ulator, 1,
> + *val);
> + }
The registered regualtor is ignored here which means you leak the
regualtors every time a device is unregistered...
> +static void mikrobus_device_unregister(struct mikrobus_port *port, struct board_device_info *dev,
> + char *board_name)
..an operation which does seem to be supported?
On Sat, Mar 16, 2024 at 12:19:05AM +0530, Ayush Singh wrote:
> diff --git a/drivers/misc/mikrobus/Kconfig b/drivers/misc/mikrobus/Kconfig
> new file mode 100644
> index 000000000000..f0770006b4fe
> --- /dev/null
> +++ b/drivers/misc/mikrobus/Kconfig
> @@ -0,0 +1,19 @@
> +menuconfig MIKROBUS
> + tristate "Module for instantiating devices on mikroBUS ports"
> + depends on GPIOLIB
> + depends on W1
> + depends on W1_MASTER_GPIO
> + help
> + This option enables the mikroBUS driver. mikroBUS is an add-on
> + board socket standard that offers maximum expandability with
> + the smallest number of pins. The mikroBUS driver instantiates
> + devices on a mikroBUS port described by identifying data present
> + in an add-on board resident EEPROM, more details on the mikroBUS
> + driver support and discussion can be found in this eLinux wiki :
> + elinux.org/Mikrobus
I think this is a fallacy. I have boards that support Mikrobus - some of
the SolidRun products do. I have several Mikrobus "click" boards.
This help text seems to imply that Mikrobus click boards include an
EEPROM that identify them, hence you make the support for mikroBUS
depend on it. No, this is not the case - the click boards do not
contain a 1-wire EEPROM.
Please fetch a copy of the official Mikrobus specification which is
available here:
https://download.mikroe.com/documents/standards/mikrobus/mikrobus-standard-specification-v200.pdf
and rather than creating something that is implementation specific but
appears to be generic, create something that is generic with
implementation specific extensions.
You'll find if you search that document, no mention is made of an
"eprom". "identification" is also not found. "one wire", "onewire",
"1-wire", "1wire" are also not found.
What I'm concerned about is if we create this "Mikrobus" subsystem
which appears to be dependent on one-wire EEPROMs somewhere in the
system, and then end up with a load of drivers for various mikroBUS
boards, what about cases where there is no one-wire EEPROM?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On 15/03/2024 19:48, Ayush Singh wrote:
> Add DT bindings for mikroBUS interface. MikroBUS is an open standard
> developed by MikroElektronika for connecting add-on boards to
> microcontrollers or microprocessors.
>
> Signed-off-by: Ayush Singh <[email protected]>
> ---
> .../bindings/misc/mikrobus-connector.yaml | 110 ++++++++++++++++++
> MAINTAINERS | 6 +
> 2 files changed, 116 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
>
> diff --git a/Documentation/devicetree/bindings/misc/mikrobus-connector.yaml b/Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
> new file mode 100644
> index 000000000000..6eace2c0dddc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
Please put it in connector directory.
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/misc/mikrobus-connector.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: mikroBUS add-on board socket
> +
> +maintainers:
> + - Ayush Singh <[email protected]>
> +
> +properties:
> + compatible:
> + const: mikrobus-connector
Hm, why do you create binding for the connector, not for some sort of
controller? Please provide some rationale for this in commit msg.
> +
> + pinctrl-0: true
> + pinctrl-1: true
> + pinctrl-2: true
> + pinctrl-3: true
> + pinctrl-4: true
> + pinctrl-5: true
> + pinctrl-6: true
> + pinctrl-7: true
> + pinctrl-8: true
> +
> + pinctrl-names:
> + items:
> + - const: default
> + - const: pwm_default
> + - const: pwm_gpio
> + - const: uart_default
> + - const: uart_gpio
> + - const: i2c_default
> + - const: i2c_gpio
> + - const: spi_default
> + - const: spi_gpio
I fail to see why such choice is related to the connector itself.
Connector could have just SPI attached, so why all other entries needs
to be provided? Or is it fully plugable? But then really please explain
the hardware in the binding description.
> +
> + mikrobus-gpios:
> + minItems: 11
> + maxItems: 12
> +
> + i2c-adapter:
> + description: i2c adapter attached to the mikrobus socket.
> + $ref: /schemas/types.yaml#/definitions/phandle
> +
> + spi-controller:
> + description: spi bus number of the spi-master attached to the mikrobus socket.
> + $ref: /schemas/types.yaml#/definitions/phandle
> +
> + uart:
> + description: uart port attached to the mikrobus socket
> + $ref: /schemas/types.yaml#/definitions/phandle
> +
> + pwms:
> + description: the pwm-controller corresponding to the mikroBUS PWM pin.
> + maxItems: 1
> +
> + spi-cs:
> + description: spi chip-select numbers corresponding to the chip-selects on the mikrobus socket.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + items:
> + - description: chip select corresponding to CS pin
> + - description: chip select corresponding to RST pin
I don't understand why do you need all these properties. First, if this
is connector then I would rather see some sort of graph, not phandles.
Why would connector need to do anything with SPI controller?
All this looks like made for software. For the driver.
> +
> +required:
> + - compatible
> + - pinctrl-0
> + - pinctrl-1
> + - pinctrl-2
> + - pinctrl-3
> + - pinctrl-4
> + - pinctrl-5
> + - pinctrl-6
> + - pinctrl-7
> + - pinctrl-8
> + - i2c-adapter
> + - spi-controller
> + - spi-cs
> + - uart
> + - pwms
> + - mikrobus-gpios
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + mikrobus-0 {
mikrobus {
and fix the indentation. Use 4 spaces for example indentation.
> + compatible = "mikrobus-connector";
> + status = "okay";
Drop.
> + pinctrl-names = "default", "pwm_default", "pwm_gpio","uart_default", "uart_gpio", "i2c_default",
> + "i2c_gpio", "spi_default", "spi_gpio";
> + pinctrl-0 = <&P2_03_gpio_input_pin &P1_04_gpio_pin &P1_02_gpio_pin>;
> + pinctrl-1 = <&P2_01_pwm_pin>;
> + pinctrl-2 = <&P2_01_gpio_pin>;
> + pinctrl-3 = <&P2_05_uart_pin &P2_07_uart_pin>;
> + pinctrl-4 = <&P2_05_gpio_pin &P2_07_gpio_pin>;
> + pinctrl-5 = <&P2_09_i2c_pin &P2_11_i2c_pin>;
> + pinctrl-6 = <&P2_09_gpio_pin &P2_11_gpio_pin>;
> + pinctrl-7 = <&P1_12_spi_pin &P1_10_spi_pin &P1_08_spi_sclk_pin &P1_06_spi_cs_pin>;
> + pinctrl-8 = <&P1_12_gpio_pin &P1_10_gpio_pin &P1_08_gpio_pin &P1_06_gpio_pin>;
> + i2c-adapter = <&i2c1>;
> + spi-controller = <&spi1>;
> + spi-cs = <0 1>;
> + uart = <&uart1>;
> + pwms = <&ehrpwm1 0 500000 0>;
> + mikrobus-gpios = <&gpio1 18 0> , <&gpio0 23 0>, <&gpio0 30 0> , <&gpio0 31 0>, <&gpio0 15 0>,
> + <&gpio0 14 0>, <&gpio0 4 0> , <&gpio0 3 0>, <&gpio0 2 0>, <&gpio0 5 0>,
> + <&gpio2 25 0>, <&gpio2 3 0>;
Use proper defines for GPIO flags.
Best regards,
Krzysztof
On 15/03/2024 19:49, Ayush Singh wrote:
> Add helper to find w1_master from w1_bus_master, which is present in
> drvdata of platform device.
Who needs this?
>
> Signed-off-by: Vaishnav M A <[email protected]>
> Signed-off-by: Ayush Singh <[email protected]>
> ---
> drivers/w1/w1.c | 6 +++---
> drivers/w1/w1_int.c | 27 +++++++++++++++++++++++++++
> include/linux/w1.h | 1 +
> 3 files changed, 31 insertions(+), 3 deletions(-)
Why is this in the patchset? What are the dependencies? Please clearly
express dependencies between patches or merging needs in cover letter.
Otherwise please do not combine unrelated patches from different
subsystems together. It's make review and merging only difficult.
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index afb1cc4606c5..ce8a3f93f2ef 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -673,9 +673,9 @@ static int __w1_attach_slave_device(struct w1_slave *sl)
> sl->dev.of_node = of_find_matching_node(sl->master->dev.of_node,
> sl->family->of_match_table);
>
> - dev_set_name(&sl->dev, "%02x-%012llx",
> - (unsigned int) sl->reg_num.family,
> - (unsigned long long) sl->reg_num.id);
> + dev_set_name(&sl->dev, "%s-%02x-%012llx", sl->master->name,
> + (unsigned int)sl->reg_num.family,
> + (unsigned long long)sl->reg_num.id);
> snprintf(&sl->name[0], sizeof(sl->name),
Why? How is this related to the goal "add a helper"? Where is the helper
used? I don't see. Don't combine unrelated topics in one patch.
> "%02x-%012llx",
> (unsigned int) sl->reg_num.family,
> diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
> index 3a71c5eb2f83..2bfef8e67687 100644
> --- a/drivers/w1/w1_int.c
> +++ b/drivers/w1/w1_int.c
> @@ -242,3 +242,30 @@ void w1_remove_master_device(struct w1_bus_master *bm)
> __w1_remove_master_device(found);
> }
> EXPORT_SYMBOL(w1_remove_master_device);
> +
> +/**
> + * w1_find_master_device() - find a master device
> + * @bm: master bus device to search
> + */
> +struct w1_master *w1_find_master_device(struct w1_bus_master *bm)
Why are you duplicating w1_search_master_id()? Without locking? Sorry,
this looks like you did not look at all at existing code.
Best regards,
Krzysztof
On 15/03/2024 19:49, Ayush Singh wrote:
> From: Vaishnav M A <[email protected]>
>
> add of_find_serdev_controller_by_node to obtain a
> serdev_controller from the device_node, which
> can help if the serdev_device is not described
> over device tree and instantiation of the device
> happens from a different driver, for the same purpose
> an option to not delete an empty serdev controller
> is added.
Don't make it difficult for us to read your commit msgs.
Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>
> Signed-off-by: Vaishnav M A <[email protected]>
> Signed-off-by: Ayush Singh <[email protected]>
> ---
> drivers/tty/serdev/core.c | 19 +++++++++++++++++++
> include/linux/serdev.h | 4 ++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index 613cb356b918..5b5b3f2b2e42 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -555,6 +555,19 @@ static int of_serdev_register_devices(struct serdev_controller *ctrl)
> return 0;
> }
>
> +#if defined(CONFIG_OF)
> +struct serdev_controller *of_find_serdev_controller_by_node(struct device_node *node)
> +{
> + struct device *dev = bus_find_device_by_of_node(&serdev_bus_type, node);
> +
> + if (!dev)
> + return NULL;
> +
> + return (dev->type == &serdev_ctrl_type) ? to_serdev_controller(dev) : NULL;
> +}
> +EXPORT_SYMBOL_GPL(of_find_serdev_controller_by_node);
> +#endif
> +
> #ifdef CONFIG_ACPI
>
> #define SERDEV_ACPI_MAX_SCAN_DEPTH 32
> @@ -785,6 +798,12 @@ int serdev_controller_add(struct serdev_controller *ctrl)
>
> pm_runtime_enable(&ctrl->dev);
>
> + /* provide option to not delete a serdev controller without devices
> + * if property is present
> + */
> + if (device_property_present(&ctrl->dev, "force-empty-serdev-controller"))
How is this related to topic of adding helper? Why are you adding some
undocumented properties?
No, it's the same in other patches - you combine unrelated goals into
one patch. Please read carefully submitting patches document how to
organize your patchset.
Best regards,
Krzysztof
On Fri, Mar 15, 2024 at 09:09:11PM +0100, Krzysztof Kozlowski wrote:
> > +properties:
> > + compatible:
> > + const: mikrobus-connector
>
> Hm, why do you create binding for the connector, not for some sort of
> controller? Please provide some rationale for this in commit msg.
I think you have a distorted view. I refer you to the Mikroe mikroBUS
specification - it's _just_ a connector which provides a fairly
standardised purpose for each pin and the electrical specifications.
For example of the pins: power, UART, SPIs, I2C, PWM, and analogue
pins.
> > + pinctrl-names:
> > + items:
> > + - const: default
> > + - const: pwm_default
> > + - const: pwm_gpio
> > + - const: uart_default
> > + - const: uart_gpio
> > + - const: i2c_default
> > + - const: i2c_gpio
> > + - const: spi_default
> > + - const: spi_gpio
>
> I fail to see why such choice is related to the connector itself.
This isn't a choice at all. Here's the list of pins:
Analog - AN
Reset - RST
SPI Chip Select - CS
SPI Clock - SCK
SPI Master Input Slave Output - MISO
SPI Master Output Slave Input - MOSI
VCC-3.3V power - +3.3V
Reference Ground - GND
PWM - PWM output
INT - Hardware Interrupt
RX - UART Receive
TX - UART Transmit
SCL - I2C Clock
SDA - I2C Data
+5V - VCC-5V power
GND - Reference Ground
Any data pin can be a GPIO if e.g. a relay board is plugged in, even
if some of the other pins are used for e.g. UART purposes. For example,
a GPS board that provides the GPS data over the UART pins, and the
PPS signal through a different pin.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On 15/03/2024 19:49, Ayush Singh wrote:
> Add mikroBUS connector support for Beagleplay.
>
> Signed-off-by: Ayush Singh <[email protected]>
> ---
> .../arm64/boot/dts/ti/k3-am625-beagleplay.dts | 76 +++++++++++++++++--
> 1 file changed, 68 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
> index a34e0df2ab86..886308f99d1a 100644
> --- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
> @@ -29,6 +29,7 @@ aliases {
> i2c3 = &main_i2c3;
> i2c4 = &wkup_i2c0;
> i2c5 = &mcu_i2c0;
> + mikrobus0 = &mikrobus0;
> mmc0 = &sdhci0;
> mmc1 = &sdhci1;
> mmc2 = &sdhci2;
> @@ -230,6 +231,33 @@ simple-audio-card,codec {
> };
> };
>
> + mikrobus0: linux-mikrobus {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
mikrobus {
> + compatible = "mikrobus-connector";
> + pinctrl-names = "default", "pwm_default", "pwm_gpio",
> + "uart_default", "uart_gpio", "i2c_default",
> + "i2c_gpio", "spi_default", "spi_gpio";
> + pinctrl-0 = <&mikrobus_gpio_pins_default>;
> + pinctrl-1 = <&mikrobus_pwm_pins_default>;
> + pinctrl-2 = <&mikrobus_pwm_pins_gpio>;
> + pinctrl-3 = <&mikrobus_uart_pins_default>;
> + pinctrl-4 = <&mikrobus_uart_pins_gpio>;
> + pinctrl-5 = <&mikrobus_i2c_pins_default>;
> + pinctrl-6 = <&mikrobus_i2c_pins_gpio>;
> + pinctrl-7 = <&mikrobus_spi_pins_default>;
> + pinctrl-8 = <&mikrobus_spi_pins_gpio>;
> + i2c-adapter = <&main_i2c3>;
> + spi-controller = <&main_spi2>;
> + spi-cs = <0 1>;
> + uart = <&main_uart5>;
> + pwms = <&ecap2 0 500000 0>;
> + mikrobus-gpios =
> + <&main_gpio1 11 GPIO_ACTIVE_HIGH>, <&main_gpio1 9 GPIO_ACTIVE_HIGH>,
Join with previous line.
> + <&main_gpio1 24 GPIO_ACTIVE_HIGH>, <&main_gpio1 25 GPIO_ACTIVE_HIGH>,
Fix indentation. See DTS coding style.
> + <&main_gpio1 22 GPIO_ACTIVE_HIGH>, <&main_gpio1 23 GPIO_ACTIVE_HIGH>,
> + <&main_gpio1 7 GPIO_ACTIVE_HIGH>, <&main_gpio1 8 GPIO_ACTIVE_HIGH>,
> + <&main_gpio1 14 GPIO_ACTIVE_HIGH>, <&main_gpio1 13 GPIO_ACTIVE_HIGH>,
> + <&main_gpio1 12 GPIO_ACTIVE_HIGH>, <&main_gpio1 10 GPIO_ACTIVE_HIGH>;
> + };
> };
>
>
> &main_spi2 {
> - pinctrl-names = "default";
> - pinctrl-0 = <&mikrobus_spi_pins_default>;
> status = "okay";
> };
>
> @@ -875,9 +932,8 @@ &main_uart1 {
> };
>
> &main_uart5 {
> - pinctrl-names = "default";
> - pinctrl-0 = <&mikrobus_uart_pins_default>;
> status = "okay";
> + force-empty-serdev-controller;
NAK. Don't add undocumented properties. That's a clear no go.
Best regards,
Krzysztof
On 15/03/2024 19:49, Ayush Singh wrote:
> - Setup I2C, SPI, serdev controllers associated with mikrobus connector
> - Check if a board with valid mikroBUS manifest is connected
> - Parse the manifest and register the device to kernel
>
> Signed-off-by: Vaishnav M A <[email protected]>
> Signed-off-by: Ayush Singh <[email protected]>
> ---
> MAINTAINERS | 1 +
> drivers/misc/Kconfig | 1 +
> drivers/misc/Makefile | 1 +
> drivers/misc/mikrobus/Kconfig | 19 +
> drivers/misc/mikrobus/Makefile | 6 +
> drivers/misc/mikrobus/mikrobus_core.c | 942 ++++++++++++++++++++++
> drivers/misc/mikrobus/mikrobus_core.h | 201 +++++
> drivers/misc/mikrobus/mikrobus_id.c | 229 ++++++
> drivers/misc/mikrobus/mikrobus_manifest.c | 502 ++++++++++++
> drivers/misc/mikrobus/mikrobus_manifest.h | 20 +
> 10 files changed, 1922 insertions(+)
> create mode 100644 drivers/misc/mikrobus/Kconfig
> create mode 100644 drivers/misc/mikrobus/Makefile
> create mode 100644 drivers/misc/mikrobus/mikrobus_core.c
> create mode 100644 drivers/misc/mikrobus/mikrobus_core.h
> create mode 100644 drivers/misc/mikrobus/mikrobus_id.c
> create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.c
> create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 69418a058c6b..83bc5e48bef9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14772,6 +14772,7 @@ M: Ayush Singh <[email protected]>
> M: Vaishnav M A <[email protected]>
> S: Maintained
> F: Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
> +F: drivers/misc/mikrobus/*
>
> MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT
> M: Luka Kovacic <[email protected]>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 4fb291f0bf7c..3d5c36205c6c 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -591,4 +591,5 @@ source "drivers/misc/cardreader/Kconfig"
> source "drivers/misc/uacce/Kconfig"
> source "drivers/misc/pvpanic/Kconfig"
> source "drivers/misc/mchp_pci1xxxx/Kconfig"
> +source "drivers/misc/mikrobus/Kconfig"
> endmenu
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index ea6ea5bbbc9c..b9ac88055b87 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -68,3 +68,4 @@ obj-$(CONFIG_TMR_INJECT) += xilinx_tmr_inject.o
> obj-$(CONFIG_TPS6594_ESM) += tps6594-esm.o
> obj-$(CONFIG_TPS6594_PFSM) += tps6594-pfsm.o
> obj-$(CONFIG_NSM) += nsm.o
> +obj-y += mikrobus/
> diff --git a/drivers/misc/mikrobus/Kconfig b/drivers/misc/mikrobus/Kconfig
> new file mode 100644
> index 000000000000..f0770006b4fe
> --- /dev/null
> +++ b/drivers/misc/mikrobus/Kconfig
> @@ -0,0 +1,19 @@
> +menuconfig MIKROBUS
> + tristate "Module for instantiating devices on mikroBUS ports"
> + depends on GPIOLIB
> + depends on W1
> + depends on W1_MASTER_GPIO
> + help
> + This option enables the mikroBUS driver. mikroBUS is an add-on
> + board socket standard that offers maximum expandability with
> + the smallest number of pins. The mikroBUS driver instantiates
> + devices on a mikroBUS port described by identifying data present
> + in an add-on board resident EEPROM, more details on the mikroBUS
> + driver support and discussion can be found in this eLinux wiki :
> + elinux.org/Mikrobus
> +
> +
> + Say Y here to enable support for this driver.
> +
> + To compile this code as a module, chose M here: the module
> + will be called mikrobus.ko
> diff --git a/drivers/misc/mikrobus/Makefile b/drivers/misc/mikrobus/Makefile
> new file mode 100644
> index 000000000000..c89ff2abb80e
> --- /dev/null
> +++ b/drivers/misc/mikrobus/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# mikroBUS Core
> +
> +obj-$(CONFIG_MIKROBUS) += mikrobus.o
> +mikrobus-y := mikrobus_core.o mikrobus_manifest.o
> +obj-$(CONFIG_MIKROBUS) += mikrobus_id.o
> diff --git a/drivers/misc/mikrobus/mikrobus_core.c b/drivers/misc/mikrobus/mikrobus_core.c
> new file mode 100644
> index 000000000000..17718ed315b9
> --- /dev/null
> +++ b/drivers/misc/mikrobus/mikrobus_core.c
> @@ -0,0 +1,942 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * mikroBUS driver for instantiating add-on
> + * board devices with an identifier EEPROM
> + *
> + * Copyright 2020 Vaishnav M A, BeagleBoard.org Foundation.
> + * Copyright 2024 Ayush Singh <[email protected]>
> + */
> +
> +#define pr_fmt(fmt) "mikrobus:%s: " fmt, __func__
> +
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/idr.h>
> +#include <linux/init.h>
> +#include <linux/jump_label.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/mutex.h>
> +#include <linux/w1.h>
> +#include <linux/device.h>
> +#include <linux/i2c.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/machine.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/interrupt.h>
> +#include <linux/spi/spi.h>
> +#include <linux/serdev.h>
> +#include <linux/property.h>
> +#include <linux/platform_device.h>
> +#include <linux/debugfs.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/regulator/fixed.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/clk-provider.h>
> +#include <linux/greybus/greybus_manifest.h>
> +#include <linux/of_platform.h>
> +#include <linux/pwm.h>
Order all includes by name and double check if you really need that
bunch. I have doubts. Why do you implement nvmem provider here? Why do
you include GPIO machine driver here? I could go on...
> +
> +#include "mikrobus_core.h"
> +#include "mikrobus_manifest.h"
> +
> +#define MIKROBUS_ID_EEPROM_MANIFEST_ADDR 0x20
> +
> +static DEFINE_MUTEX(core_lock);
> +static DEFINE_IDR(mikrobus_port_idr);
> +static struct class_compat *mikrobus_port_compat_class;
> +int __mikrobus_first_dynamic_bus_num;
First, don't mix local and global scope variables.
Second, no don't define global scope variables.
> +static bool is_registered;
> +static int mikrobus_port_id_eeprom_probe(struct mikrobus_port *port);
> +
> +const char *MIKROBUS_PINCTRL_STR[] = { "pwm", "uart", "i2c", "spi" };
No global scope variables.
> +
> +const struct bus_type mikrobus_bus_type = {
> + .name = "mikrobus",
> +};
> +EXPORT_SYMBOL_GPL(mikrobus_bus_type);
Why it has to be global and has to be exported?
> +
> +int mikrobus_port_scan_eeprom(struct mikrobus_port *port)
> +{
> + const u16 manifest_start_addr = MIKROBUS_ID_EEPROM_MANIFEST_ADDR;
> + struct addon_board_info *board;
> + int manifest_size, retval;
> + char header[12], *buf;
> +
> + if (port->skip_scan)
> + return -EINVAL;
> +
> + retval = nvmem_device_read(port->eeprom, manifest_start_addr, 12, header);
> + if (retval != 12) {
> + return dev_err_probe(&port->dev, -EINVAL, "failed to fetch manifest header %d\n",
> + retval);
Is it probe context? Same comment everywhere else.
> + }
> +
> + manifest_size = mikrobus_manifest_header_validate(header, 12);
> + if (manifest_size < 0) {
> + return dev_err_probe(&port->dev, -EINVAL, "invalid manifest size %d\n",
Are you sure this fits in Linux coding style limit (not checkpatch
limit, but the limit expressed by Linux coding style)?
> + manifest_size);
> + }
> +
> + buf = kzalloc(manifest_size, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + retval = nvmem_device_read(port->eeprom, manifest_start_addr, manifest_size, buf);
> + if (retval != manifest_size) {
> + retval =
> + dev_err_probe(&port->dev, -EINVAL, "failed to fetch manifest %d\n", retval);
No, this for sure does not fit.
> + goto err_free_buf;
> + }
> +
> + board = kzalloc(sizeof(*board), GFP_KERNEL);
> + if (!board) {
> + retval = -ENOMEM;
> + goto err_free_buf;
> + }
> +
> + w1_reset_bus(port->w1_master);
> + /* set RST HIGH */
> + gpiod_direction_output(port->gpios->desc[MIKROBUS_PIN_RST], 1);
> + set_bit(W1_ABORT_SEARCH, &port->w1_master->flags);
> +
> + INIT_LIST_HEAD(&board->manifest_descs);
> + INIT_LIST_HEAD(&board->devices);
> + retval = mikrobus_manifest_parse(board, buf, manifest_size);
> + if (!retval) {
> + retval = dev_err_probe(&port->dev, -EINVAL, "failed to parse manifest, size %d\n",
> + manifest_size);
> + goto err_free_board;
> + }
> +
> + retval = mikrobus_board_register(port, board);
> + if (retval) {
> + dev_err(&port->dev, "failed to register board %s\n", board->name);
> + goto err_free_board;
> + }
> +
> + kfree(buf);
> + return 0;
> +
> +err_free_board:
> + kfree(board);
> +err_free_buf:
> + kfree(buf);
> + return retval;
> +}
> +EXPORT_SYMBOL_GPL(mikrobus_port_scan_eeprom);
> +
> +static ssize_t name_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%s\n", to_mikrobus_port(dev)->name);
> +}
> +static DEVICE_ATTR_RO(name);
> +
> +static ssize_t new_device_store(struct device *dev, struct device_attribute *attr, const char *buf,
> + size_t count)
Again, does not fit to coding style wrap limit.
But what's more important: where is ABI for all your interfaces? It
seems you created user-space interface for registering devices. This
needs to be discussed first.
> +{
> + struct mikrobus_port *port = to_mikrobus_port(dev);
> + struct addon_board_info *board;
> + int retval;
> +
> + if (port->board)
> + return dev_err_probe(dev, -EBUSY, "already has board registered\n");
> +
> + board = kzalloc(sizeof(*board), GFP_KERNEL);
> + if (!board)
> + return -ENOMEM;
..
> +
> +static int mikrobus_port_probe_pinctrl_setup(struct mikrobus_port *port)
> +{
> + struct device *dev = port->dev.parent;
> + struct pinctrl_state *state;
> + int retval, i;
> +
> + state = pinctrl_lookup_state(port->pinctrl, PINCTRL_STATE_DEFAULT);
> + if (!IS_ERR(state)) {
> + retval = pinctrl_select_state(port->pinctrl, state);
> + if (retval) {
> + return dev_err_probe(dev, retval, "Failed to select state %s\n",
> + PINCTRL_STATE_DEFAULT);
> + }
> + } else {
> + return dev_err_probe(dev, PTR_ERR(state), "failed to find state %s\n",
> + PINCTRL_STATE_DEFAULT);
> + }
> +
> + for (i = 0; i < MIKROBUS_NUM_PINCTRL_STATE; i++) {
> + port->pinctrl_selected[i] = kmalloc(MIKROBUS_PINCTRL_NAME_SIZE, GFP_KERNEL);
> + sprintf(port->pinctrl_selected[i], "%s_%s", MIKROBUS_PINCTRL_STR[i],
> + PINCTRL_STATE_DEFAULT);
> + }
> +
> + retval = mikrobus_port_pinctrl_select(port);
> + if (retval)
> + dev_err(dev, "failed to select pinctrl states [%d]", retval);
> +
> + return retval;
> +}
> +
> +static int mikrobus_port_probe(struct platform_device *pdev)
> +{
> + struct device_node *i2c_adap_np, *uart_np, *spi_np;
> + struct device *dev = &pdev->dev;
> + struct mikrobus_port *port;
> + int retval;
int ret
> +
> + port = kzalloc(sizeof(*port), GFP_KERNEL);
Why not devm?
> + if (!port)
> + return -ENOMEM;
> +
> + /* I2C setup */
> + i2c_adap_np = of_parse_phandle(dev->of_node, "i2c-adapter", 0);
> + if (!i2c_adap_np) {
> + retval = dev_err_probe(dev, -ENODEV, "cannot parse i2c-adapter");
> + goto err_port;
> + }
> + port->i2c_adap = of_find_i2c_adapter_by_node(i2c_adap_np);
> + of_node_put(i2c_adap_np);
> +
> + /* GPIO setup */
> + port->gpios = gpiod_get_array(dev, "mikrobus", GPIOD_OUT_LOW);
> + if (IS_ERR(port->gpios)) {
> + retval = PTR_ERR(port->gpios);
> + dev_err(dev, "failed to get gpio array [%d]", retval);
ret = dev_err_probe
Where is put_device? You leak I2C adapter. All code leaks probably much
more.
> + goto err_port;
> + }
> +
> + /* SPI setup */
> + spi_np = of_parse_phandle(dev->of_node, "spi-controller", 0);
> + if (!spi_np) {
> + retval = dev_err_probe(dev, -ENODEV, "cannot parse spi-controller");
> + goto err_port;
> + }
> + port->spi_ctrl = of_find_spi_controller_by_node(spi_np);
> + of_node_put(spi_np);
> + if (!port->spi_ctrl) {
> + retval = dev_err_probe(dev, -ENODEV, "cannot find spi controller");
> + goto err_port;
> + }
> + retval = device_property_read_u32_array(dev, "spi-cs", port->chip_select, MIKROBUS_NUM_CS);
> + if (retval) {
> + dev_err(dev, "failed to get spi-cs [%d]", retval);
> + goto err_port;
> + }
> +
> + /* UART setup */
> + uart_np = of_parse_phandle(dev->of_node, "uart", 0);
> + if (!uart_np) {
> + retval = dev_err_probe(dev, -ENODEV, "cannot parse uart");
> + goto err_port;
> + }
> + port->ser_ctrl = of_find_serdev_controller_by_node(uart_np);
> + of_node_put(uart_np);
> + if (!port->ser_ctrl) {
> + retval = dev_err_probe(dev, -ENODEV, "cannot find uart controller");
> + goto err_port;
> + }
> +
> + /* PWM setup */
> + port->pwm = devm_pwm_get(dev, NULL);
> + if (!port->pwm) {
> + retval = dev_err_probe(dev, -ENODEV, "cannot find pwm controller");
> + goto err_port;
> + }
> +
> + /* pinctrl setup */
> + port->pinctrl = devm_pinctrl_get(dev);
> + if (IS_ERR(port->pinctrl)) {
> + retval = PTR_ERR(port->pinctrl);
> + dev_err(dev, "failed to get pinctrl [%d]", retval);
ret = dev_err_probe()
> + goto err_port;
> + }
> + port->dev.parent = dev;
> + port->dev.of_node = pdev->dev.of_node;
> + retval = mikrobus_port_probe_pinctrl_setup(port);
> + if (retval) {
> + dev_err(dev, "failed to setup pinctrl [%d]", retval);
> + goto err_port;
> + }
> +
> + retval = mikrobus_port_register(port);
> + if (retval) {
> + dev_err(dev, "port : can't register port [%d]", retval);
> + goto err_port;
> + }
> +
> + platform_set_drvdata(pdev, port);
> +
> + return 0;
> +
> +err_port:
> + kfree(port);
> + return retval;
> +}
> +
> +static int mikrobus_port_remove(struct platform_device *pdev)
> +{
> + struct mikrobus_port *port = platform_get_drvdata(pdev);
> +
> + mikrobus_port_delete(port);
> + return 0;
> +}
> +
> +static const struct of_device_id mikrobus_port_of_match[] = {
> + { .compatible = "mikrobus-connector" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, mikrobus_port_of_match);
> +
> +static struct platform_driver mikrobus_port_driver = {
> + .probe = mikrobus_port_probe,
> + .remove = mikrobus_port_remove,
> + .driver = {
> + .name = "mikrobus",
> + .of_match_table = of_match_ptr(mikrobus_port_of_match),
Drop of_match_ptr
> + },
> +};
> +
> +static int mikrobus_init(void)
> +{
> + int retval;
> +
> + retval = bus_register(&mikrobus_bus_type);
> + if (retval) {
> + pr_err("bus_register failed (%d)\n", retval);
> + return retval;
> + }
> +
> + mikrobus_port_compat_class = class_compat_register("mikrobus-port");
> + if (!mikrobus_port_compat_class) {
> + pr_err("class_compat register failed (%d)\n", retval);
> + retval = -ENOMEM;
> + goto class_err;
> + }
> +
> + retval = of_alias_get_highest_id("mikrobus");
> + if (retval >= __mikrobus_first_dynamic_bus_num)
> + __mikrobus_first_dynamic_bus_num = retval + 1;
> +
> + is_registered = true;
> + retval = platform_driver_register(&mikrobus_port_driver);
> + if (retval)
> + pr_err("driver register failed [%d]\n", retval);
> +
> + return retval;
> +
> +class_err:
> + bus_unregister(&mikrobus_bus_type);
> + idr_destroy(&mikrobus_port_idr);
> + is_registered = false;
> + return retval;
> +}
> +subsys_initcall(mikrobus_init);
> +
> +static void mikrobus_exit(void)
> +{
> + platform_driver_unregister(&mikrobus_port_driver);
> + bus_unregister(&mikrobus_bus_type);
> + class_compat_unregister(mikrobus_port_compat_class);
> + idr_destroy(&mikrobus_port_idr);
> +}
> +module_exit(mikrobus_exit);
> +
> +MODULE_AUTHOR("Vaishnav M A <[email protected]>");
> +MODULE_AUTHOR("Ayush Singh <[email protected]>");
> +MODULE_DESCRIPTION("mikroBUS main module");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/misc/mikrobus/mikrobus_core.h b/drivers/misc/mikrobus/mikrobus_core.h
> new file mode 100644
> index 000000000000..8bd101828964
> --- /dev/null
> +++ b/drivers/misc/mikrobus/mikrobus_core.h
> @@ -0,0 +1,201 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * mikroBUS Driver for instantiating add-on
> + * board devices with an identifier EEPROM
> + *
> + * Copyright 2020 Vaishnav M A, BeagleBoard.org Foundation.
> + */
> +
> +#ifndef __MIKROBUS_H
> +#define __MIKROBUS_H
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/i2c.h>
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/machine.h>
> +#include <linux/spi/spi.h>
> +#include <linux/serdev.h>
> +#include <linux/property.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/nvmem-provider.h>
Why did you stuff here so many includes? Are you sure you need all three
pinctrls? All three gpio? The header must include *only* stuff it
directly uses.
> +
> +#define MIKROBUS_VERSION_MAJOR 0x00
> +#define MIKROBUS_VERSION_MINOR 0x03
> +
> +#define MIKROBUS_NAME_SIZE 40
> +#define MIKROBUS_PINCTRL_NAME_SIZE 20
> +
> +#define MIKROBUS_NUM_PINCTRL_STATE 4
> +#define MIKROBUS_NUM_CS 2
> +
> +#define MIKROBUS_PINCTRL_PWM 0
> +#define MIKROBUS_PINCTRL_UART 1
> +#define MIKROBUS_PINCTRL_I2C 2
> +#define MIKROBUS_PINCTRL_SPI 3
> +
> +#define MIKROBUS_PINCTRL_STATE_GPIO "gpio"
> +
> +#define MIKROBUS_EEPROM_EXIT_ID_CMD 0xD2
> +
> +extern const struct bus_type mikrobus_bus_type;
> +extern const struct device_type mikrobus_port_type;
> +extern const char *MIKROBUS_PINCTRL_STR[];
All three look suspicious. Why do you create global variables? Looks
like some pattern from very old vendor code...
> +
> +enum mikrobus_property_type {
> + MIKROBUS_PROPERTY_TYPE_MIKROBUS = 0x00,
> + MIKROBUS_PROPERTY_TYPE_PROPERTY = 0x01,
> + MIKROBUS_PROPERTY_TYPE_GPIO = 0x02,
> + MIKROBUS_PROPERTY_TYPE_U8 = 0x03,
> + MIKROBUS_PROPERTY_TYPE_U16 = 0x04,
> + MIKROBUS_PROPERTY_TYPE_U32 = 0x05,
> + MIKROBUS_PROPERTY_TYPE_U64 = 0x06,
> + MIKROBUS_PROPERTY_TYPE_REGULATOR = 0x07,
> + MIKROBUS_PROPERTY_TYPE_CLOCK = 0x08,
> +};
> +
> +enum mikrobus_pin {
> + MIKROBUS_PIN_PWM = 0x00,
> + MIKROBUS_PIN_INT = 0x01,
> + MIKROBUS_PIN_RX = 0x02,
> + MIKROBUS_PIN_TX = 0x03,
> + MIKROBUS_PIN_SCL = 0x04,
> + MIKROBUS_PIN_SDA = 0x05,
> + MIKROBUS_PIN_MOSI = 0x06,
> + MIKROBUS_PIN_MISO = 0x07,
> + MIKROBUS_PIN_SCK = 0x08,
> + MIKROBUS_PIN_CS = 0x09,
> + MIKROBUS_PIN_RST = 0x0A,
> + MIKROBUS_PIN_AN = 0x0B,
> + MIKROBUS_PORT_PIN_COUNT = 0x0C,
Why do you need to assign values to the enumerated entries?
> +};
> +
> +enum mikrobus_pin_state {
> + MIKROBUS_STATE_INPUT = 0x01,
> + MIKROBUS_STATE_OUTPUT_HIGH = 0x02,
> + MIKROBUS_STATE_OUTPUT_LOW = 0x03,
> + MIKROBUS_STATE_PWM = 0x04,
> + MIKROBUS_STATE_SPI = 0x05,
> + MIKROBUS_STATE_I2C = 0x06,
> + MIKROBUS_STATE_UART = 0x07,
> +};
> +
> +/*
> + * board_device_info describes a single device on a mikrobus add-on
> + * board, an add-on board can present one or more device to the host
> + *
> + * @gpio_lookup: used to provide the GPIO lookup table for
> + * passing the named GPIOs to device drivers.
> + * @properties: used to provide the property_entry to pass named
> + * properties to device drivers, applicable only when driver uses
> + * device_property_read_* calls to fetch the properties.
> + * @num_gpio_resources: number of named gpio resources for the device,
> + * used mainly for gpiod_lookup_table memory allocation.
> + * @num_properties: number of custom properties for the device,
> + * used mainly for property_entry memory allocation.
> + * @protocol: used to know the type of the device and it should
> + * contain one of the values defined under 'enum greybus_class_type'
> + * under linux/greybus/greybus_manifest.h
> + * @reg: I2C address for the device, for devices on the SPI bus
> + * this field is the chip select address relative to the mikrobus
> + * port:0->device chip select connected to CS pin on mikroBUS port
> + * 1->device chip select connected to RST Pin on mikroBUS port
> + * @mode: SPI mode
> + * @max_speed_hz: SPI max speed(Hz)
> + * @drv_name: device_id to match with the driver
> + * @irq_type: type of IRQ trigger , match with defines in linux/interrupt.h
> + * @irq: irq number relative to the mikrobus port should contain one of the
> + * values defined under 'enum mikrobus_pin'
> + * @id: device id starting from 1
> + */
> +struct board_device_info {
> + struct gpiod_lookup_table *gpio_lookup;
> + struct property_entry *properties;
> + struct property_entry *regulators;
> + struct property_entry *clocks;
> + struct list_head links;
> + unsigned short num_gpio_resources;
> + unsigned short num_properties;
> + unsigned short num_regulators;
> + unsigned short num_clocks;
> + unsigned short protocol;
> + unsigned short reg;
> + unsigned int mode;
> + void *dev_client;
> + u32 max_speed_hz;
> + char *drv_name;
> + int irq_type;
> + int irq;
> + int id;
> +};
> +
> +/*
> + * addon_board_info describes a mikrobus add-on device the add-on
> + * board, an add-on board can present one or more device to the host
> + *
> + * @manifest_descs: list of manifest descriptors
> + * @devices: list of devices on the board
> + * @pin_state: the state of each pin on the mikrobus port required
> + * for the add-on board should contain one of the values defined under
> + * 'enum mikrobus_pin_state' restrictions are as per mikrobus standard
> + * specifications.
> + * @name: add-on board name
> + */
> +struct addon_board_info {
> + struct list_head manifest_descs;
> + struct list_head devices;
> + u8 pin_state[MIKROBUS_PORT_PIN_COUNT];
> + char *name;
> +};
> +
> +/*
> + * mikrobus_port describes the peripherals mapped to a
> + * mikrobus port.
> + *
> + * @eeprom_client: i2c_client corresponding to the eeprom
> + * on the add-on board.
> + * @board: pointer to the attached add-on board.
> + * @i2c_adap: I2C adapter attached to the mikrobus port.
> + * @spi_mstr: SPI master attached to the mikrobus port.
> + * @eeprom: nvmem_device for the eeprom on the add-on board.
> + * @pwm: pwm_device attached to the mikrobus port PWM pin.
> + * @pinctrl_selected: current pinctrl_selected state.
> + * @chip_select: chip select number mapped to the SPI
> + * CS pin on the mikrobus port and the RST pin on the mikrobus
> + * port
> + * @id: port id starting from 1
> + */
> +struct mikrobus_port {
> + struct addon_board_info *board;
> + struct nvmem_device *eeprom;
> + struct i2c_adapter *i2c_adap;
> + struct spi_controller *spi_ctrl;
> + struct w1_master *w1_master;
> + struct platform_device *w1_gpio;
> + struct serdev_controller *ser_ctrl;
> + struct gpio_descs *gpios;
> + struct pwm_device *pwm;
> + struct pinctrl *pinctrl;
> + struct module *owner;
> + struct device dev;
> + char name[MIKROBUS_NAME_SIZE];
Why do you have fixed size names?
> + char *pinctrl_selected[MIKROBUS_NUM_PINCTRL_STATE];
> + unsigned int chip_select[MIKROBUS_NUM_CS];
> + int skip_scan;
> + int id;
> +};
> +
> +#define to_mikrobus_port(d) container_of(d, struct mikrobus_port, dev)
> +
> +void mikrobus_board_unregister(struct mikrobus_port *port, struct addon_board_info *board);
> +int mikrobus_board_register(struct mikrobus_port *port, struct addon_board_info *board);
> +int mikrobus_port_register(struct mikrobus_port *port);
> +int mikrobus_port_pinctrl_select(struct mikrobus_port *port);
> +void mikrobus_port_delete(struct mikrobus_port *port);
> +int mikrobus_port_scan_eeprom(struct mikrobus_port *port);
> +struct mikrobus_port *mikrobus_find_port_by_w1_master(struct w1_master *master);
> +#endif /* __MIKROBUS_H */
> diff --git a/drivers/misc/mikrobus/mikrobus_id.c b/drivers/misc/mikrobus/mikrobus_id.c
> new file mode 100644
> index 000000000000..42a0a558785d
> --- /dev/null
> +++ b/drivers/misc/mikrobus/mikrobus_id.c
> @@ -0,0 +1,229 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * mikrobus_id.c - w1 mikroBUS ID family EEPROM driver
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/types.h>
> +#include <linux/delay.h>
> +
> +#include <linux/crc16.h>
> +#include <linux/w1.h>
> +#include <linux/nvmem-provider.h>
> +
> +#include "mikrobus_core.h"
> +
> +#define W1_EEPROM_MIKROBUS_ID 0xCC
> +#define W1_MIKROBUS_ID_EEPROM_SIZE 0x0200
> +#define W1_MIKROBUS_ID_EEPROM_PAGE_SIZE 32
> +#define W1_MIKROBUS_ID_READ_EEPROM 0x69
> +#define W1_MIKROBUS_ID_WRITE_EEPROM 0x96
> +#define W1_MIKROBUS_ID_RELEASE_EEPROM 0xAA
> +#define W1_MIKROBUS_ID_EEPROM_READ_RETRIES 10
> +
> +#define W1_MIKROBUS_EEPROM_MANIFEST_START_PAGE 1
> +
> +static ssize_t mikrobus_manifest_store(struct device *device, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + u8 write_request[] = { W1_MIKROBUS_ID_WRITE_EEPROM,
> + W1_MIKROBUS_EEPROM_MANIFEST_START_PAGE };
> + u8 release_command = W1_MIKROBUS_ID_RELEASE_EEPROM;
> + struct w1_slave *sl = dev_to_w1_slave(device);
This looks like w1 driver. Why is it not suitable for w1 directory?
Best regards,
Krzysztof
On 15/03/2024 21:20, Russell King (Oracle) wrote:
> On Fri, Mar 15, 2024 at 09:09:11PM +0100, Krzysztof Kozlowski wrote:
>>> +properties:
>>> + compatible:
>>> + const: mikrobus-connector
>>
>> Hm, why do you create binding for the connector, not for some sort of
>> controller? Please provide some rationale for this in commit msg.
>
> I think you have a distorted view. I refer you to the Mikroe mikroBUS
> specification - it's _just_ a connector which provides a fairly
> standardised purpose for each pin and the electrical specifications.
> For example of the pins: power, UART, SPIs, I2C, PWM, and analogue
> pins.
I refer to the commit msg or description in the binding and there is
nothing explained like this. Yeah, true, I could google every possible
bus specification, but I also expect some sort of help here by the patch
submitter.
The binding looks like binding for a connector, not for some sort of
controller, then are you saying the control part it is purely in
software? That's how DTS looks like, but then my question is are there
some sort of controller which would also perform this?
>
>>> + pinctrl-names:
>>> + items:
>>> + - const: default
>>> + - const: pwm_default
>>> + - const: pwm_gpio
>>> + - const: uart_default
>>> + - const: uart_gpio
>>> + - const: i2c_default
>>> + - const: i2c_gpio
>>> + - const: spi_default
>>> + - const: spi_gpio
>>
>> I fail to see why such choice is related to the connector itself.
>
> This isn't a choice at all. Here's the list of pins:
>
> Analog - AN
> Reset - RST
> SPI Chip Select - CS
> SPI Clock - SCK
> SPI Master Input Slave Output - MISO
> SPI Master Output Slave Input - MOSI
> VCC-3.3V power - +3.3V
> Reference Ground - GND
> PWM - PWM output
> INT - Hardware Interrupt
> RX - UART Receive
> TX - UART Transmit
> SCL - I2C Clock
> SDA - I2C Data
> +5V - VCC-5V power
> GND - Reference Ground
>
> Any data pin can be a GPIO if e.g. a relay board is plugged in, even
> if some of the other pins are used for e.g. UART purposes. For example,
> a GPS board that provides the GPS data over the UART pins, and the
> PPS signal through a different pin.
And could you not have some certain features supported? Could have some
pins just pull down / not connected?
Best regards,
Krzysztof
On Fri, Mar 15, 2024 at 09:40:13PM +0100, Krzysztof Kozlowski wrote:
> On 15/03/2024 21:20, Russell King (Oracle) wrote:
> > On Fri, Mar 15, 2024 at 09:09:11PM +0100, Krzysztof Kozlowski wrote:
> >>> +properties:
> >>> + compatible:
> >>> + const: mikrobus-connector
> >>
> >> Hm, why do you create binding for the connector, not for some sort of
> >> controller? Please provide some rationale for this in commit msg.
> >
> > I think you have a distorted view. I refer you to the Mikroe mikroBUS
> > specification - it's _just_ a connector which provides a fairly
> > standardised purpose for each pin and the electrical specifications.
> > For example of the pins: power, UART, SPIs, I2C, PWM, and analogue
> > pins.
>
> I refer to the commit msg or description in the binding and there is
> nothing explained like this. Yeah, true, I could google every possible
> bus specification, but I also expect some sort of help here by the patch
> submitter.
>
> The binding looks like binding for a connector, not for some sort of
> controller, then are you saying the control part it is purely in
> software? That's how DTS looks like, but then my question is are there
> some sort of controller which would also perform this?
There is, as far as I'm aware, no "controller" for a mikroBUS. As I
tried to explain, it's a bunch of pins with defined standard functions.
The idea seems to be they're connected to a SoC with a pinmux that can
reconfigure the function of the pin.
At least that's how the hardware implementations I've seen do it.
> > This isn't a choice at all. Here's the list of pins:
> >
> > Analog - AN
> > Reset - RST
> > SPI Chip Select - CS
> > SPI Clock - SCK
> > SPI Master Input Slave Output - MISO
> > SPI Master Output Slave Input - MOSI
> > VCC-3.3V power - +3.3V
> > Reference Ground - GND
> > PWM - PWM output
> > INT - Hardware Interrupt
> > RX - UART Receive
> > TX - UART Transmit
> > SCL - I2C Clock
> > SDA - I2C Data
> > +5V - VCC-5V power
> > GND - Reference Ground
> >
> > Any data pin can be a GPIO if e.g. a relay board is plugged in, even
> > if some of the other pins are used for e.g. UART purposes. For example,
> > a GPS board that provides the GPS data over the UART pins, and the
> > PPS signal through a different pin.
>
> And could you not have some certain features supported? Could have some
> pins just pull down / not connected?
A board plugged in doesn't have to use all the functions. I gave two
examples. Apart from the power pins,
The GPS board as I stated uses the two UART pins, and some GPS boards
route the PPS signal to another pin on the connector, but that's
entirely optional. Another pin might be used as a GPIO as an enable.
In the case of a relay board I've had, the SPI CS pin is used for one
relay, and the PWM pin for the other relay.
I also have a BME280 humidity/pressure sensor board, which just uses
the two I2C pins.
What is supported by a board is down to the board. Which pins it
connects to is down to the board. Which board is plugged in is up
to the user.
That is essentially the long and short of mikroBUS - I hope I've
given a good overview of it.
I am slightly confused by this series, because it seems the Linux
"mikroBUS" layer requires a one-wire EEPROM on the boards, but the
official mikroBUS specification doesn't state this. The author
really needs to clarify what they are implementing here. Are they
truly implementing mikroBUS as defined by Mikroe, or are they
implementing someone's custom extension to it that adds an
identification EEPROM - in which case I would argue that this
code as it stands is not suitable for mainline as long as it
purports to be implementing support for Mikroe's mikroBUS.
Hence, I think we should back off on reviewing this until we have
that clarification.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On 3/16/24 02:50, Vaishnav M A wrote:
> Hi Ayush,
>
> On Sat, Mar 16, 2024 at 12:19 AM Ayush Singh <[email protected]> wrote:
>> MikroBUS is an open standard developed by MikroElektronika for connecting
>> add-on boards to microcontrollers or microprocessors. It essentially
>> allows you to easily expand the functionality of your main boards using
>> these add-on boards.
>>
>> This patchset adds mikroBUS as a Linux bus type and provides a driver to
>> parse, and flash mikroBUS manifest and register the mikroBUS board.
>>
> As Russel had provided the feedback, this patchset does not add support
> for mikrobus, but a subset of mikrobus add-on boards which have a
> 1-wire click ID EEPROM with an identifier blob that is similar to the greybus
> manifest. This series lacks the necessary context and details to the
> specifications that is implemented.
>
> https://www.mikroe.com/clickid - you should atleast point to this specs.
>
>> The v1 and v2 of this patchset was submitted by Vaishnav M A back in
>> 2020. This patchset also includes changes made over the years as part of
>> BeagleBoards kernel.
>>
>> Link: https://www.mikroe.com/mikrobus
>> Link: https://docs.beagleboard.org/latest/boards/beagleplay/
>> Link: https://lore.kernel.org/lkml/[email protected]/ Patch v2
>>
> Thank you for making the effort to upstream this, arriving at the
> latest revision of the public available click ID hardware took almost 2-3 years
> and I could not personally keep up with upstreaming, my sincere apologies to
> the maintainers that provided feedback on the earlier revisions. I hope an
> updated version of this series lands upstream with your work as the efforts
> made at BeagleBoard.org Foundation makes development simpler by adding
> plug and play support to these add-on boards. Also this series mentions only
> the case where mikroBUS port is present physically on the board, but there
> can be mikroBUS ports appearing over greybus and that is the reason why
> greybus manifest was chose as descriptor format - the series needs to
> describe these details so that a reviewer has the necessary information
> to review your patches. A link to beagleconnect is also helpful to reviewers.
>
> https://docs.beagleboard.org/latest/projects/beagleconnect/index.html
Yes, I left out the mikroBUS over greybus patches for now since this
patch series is already too big.
>> Changes in v3:
>> - Use phandle instead of busname for spi
>> - Use spi board info for registering new device
>> - Convert dt bindings to yaml
>> - Add support for clickID
>> - Code cleanup and style changes
>> - Additions required to spi, serdev, w1 and regulator subsystems
>>
>> Changes in v2:
>> - support for adding mikroBUS ports from DT overlays,
>> - remove debug sysFS interface for adding mikrobus ports,
>> - consider extended pin usage/deviations from mikrobus standard
>> specifications
>> - use greybus CPort protocol enum instead of new protocol enums
>> - Fix cases of wrong indentation, ignoring return values, freeing allocated
>> resources in case of errors and other style suggestions in v1 review.
>>
>> Ayush Singh (7):
> It looks like the version you have sent is very similar to the
> version I had previously updated for Beagleboard git with
> only rebases and cleanup, but I don't see major functional
> changes. I request you give credit to the original author
> atleast as a Co-author with Co-developed by tag, As there
> there was a significant amount of work done by myself to
> come up with this specs and get everything working on close
> to 150 mikrobus add-on boards on physical mikroBUS ports
> and over greybus:
> https://github.com/vaishnavachath/manifesto/tree/mikrobusv3/manifests
Yes, I will add Co-author and Co-developed tags. I think I should use
your ti email? I would have preferred to keep you as the author in the
git commit but I could not get the patches applied cleanly back when I
tried it.
> The driver today is poorly written and was one of the first
> Linux kernel development work I did while I was still in college
> so I might have made a lot of blunders and just rebasing and
> reposting will not get this to an acceptable state, here is what
> I would recommend:
>
> * Drop all the unnecessary changes in the mikroBUS driver to
> support fixed-regulators, fixed-clocks, serdev device .etc and
> implement minimal changes to support the mikroBUS clickid
> devices.
>
> * Provide necessary justification to why the particular descriptor
> format (greybus manifest) is chosen, with pull request to manifesto
> and greybus-specification.
> https://github.com/projectara/greybus-spec
> and similar to : https://github.com/projectara/manifesto/pull/2
>
> * Move the mikrobus W1 driver to w1/ subsytem, it is a standard
> W1 EEPROM driver (also a standard part with updated family code)
> * Keep a RFC series of changes where mikroBUS ports can appear over
> greybus to justify why the identifier format needs to be extended greybus
> manifest.
>
>> dt-bindings: misc: Add mikrobus-connector
>> w1: Add w1_find_master_device
> Dependent patches that goes to different subsytems should
> be sent first separately to avoid confusion and then your series
> should mention the necessary dependencies. (same for
> spi).
>
>> spi: Make of_find_spi_controller_by_node() available
>> regulator: fixed-helper: export regulator_register_always_on
>> greybus: Add mikroBUS manifest types
>> mikrobus: Add mikrobus driver
>> dts: ti: k3-am625-beagleplay: Add mikroBUS
> Send this patch as DONOTMERGE till your bindings are
> accepted.
Thanks, should I just add it in the message body? I cannot see anything
in docs about that.
>> Vaishnav M A (1):
>> serdev: add of_ helper to get serdev controller
>>
> Drop this from initial series,
> I will provide further feedback from my TI e-mail,
> Vaishnav Achath <[email protected]>
>
> Thank again for taking this up,
>
> Thanks and Regards,
> Vaishnav
>
>> .../bindings/misc/mikrobus-connector.yaml | 110 ++
>> MAINTAINERS | 7 +
>> .../arm64/boot/dts/ti/k3-am625-beagleplay.dts | 76 +-
>> drivers/misc/Kconfig | 1 +
>> drivers/misc/Makefile | 1 +
>> drivers/misc/mikrobus/Kconfig | 19 +
>> drivers/misc/mikrobus/Makefile | 6 +
>> drivers/misc/mikrobus/mikrobus_core.c | 942 ++++++++++++++++++
>> drivers/misc/mikrobus/mikrobus_core.h | 201 ++++
>> drivers/misc/mikrobus/mikrobus_id.c | 229 +++++
>> drivers/misc/mikrobus/mikrobus_manifest.c | 502 ++++++++++
>> drivers/misc/mikrobus/mikrobus_manifest.h | 20 +
>> drivers/regulator/fixed-helper.c | 1 +
>> drivers/spi/spi.c | 206 ++--
>> drivers/tty/serdev/core.c | 19 +
>> drivers/w1/w1.c | 6 +-
>> drivers/w1/w1_int.c | 27 +
>> include/linux/greybus/greybus_manifest.h | 49 +
>> include/linux/serdev.h | 4 +
>> include/linux/spi/spi.h | 4 +
>> include/linux/w1.h | 1 +
>> 21 files changed, 2318 insertions(+), 113 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
>> create mode 100644 drivers/misc/mikrobus/Kconfig
>> create mode 100644 drivers/misc/mikrobus/Makefile
>> create mode 100644 drivers/misc/mikrobus/mikrobus_core.c
>> create mode 100644 drivers/misc/mikrobus/mikrobus_core.h
>> create mode 100644 drivers/misc/mikrobus/mikrobus_id.c
>> create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.c
>> create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.h
>>
>>
>> base-commit: 61996c073c9b070922ad3a36c981ca6ddbea19a5
>> --
>> 2.44.0
>>
I guess I will start with only i2c and spi support and go from there.
Ayush Singh
Hi Russell,
I had originally worked on this and will try to provide
some more context which is missing in this series. I am
replying from my TI email as I am active here.
On 16/03/24 02:49, Russell King (Oracle) wrote:
> On Sat, Mar 16, 2024 at 02:17:24AM +0530, Ayush Singh wrote:
>> On 3/16/24 01:02, Russell King (Oracle) wrote:
>>
>>> On Sat, Mar 16, 2024 at 12:19:05AM +0530, Ayush Singh wrote:
>>>> diff --git a/drivers/misc/mikrobus/Kconfig b/drivers/misc/mikrobus/Kconfig
>>>> new file mode 100644
>>>> index 000000000000..f0770006b4fe
>>>> --- /dev/null
>>>> +++ b/drivers/misc/mikrobus/Kconfig
>>>> @@ -0,0 +1,19 @@
>>>> +menuconfig MIKROBUS
>>>> + tristate "Module for instantiating devices on mikroBUS ports"
>>>> + depends on GPIOLIB
>>>> + depends on W1
>>>> + depends on W1_MASTER_GPIO
>>>> + help
>>>> + This option enables the mikroBUS driver. mikroBUS is an add-on
>>>> + board socket standard that offers maximum expandability with
>>>> + the smallest number of pins. The mikroBUS driver instantiates
>>>> + devices on a mikroBUS port described by identifying data present
>>>> + in an add-on board resident EEPROM, more details on the mikroBUS
>>>> + driver support and discussion can be found in this eLinux wiki :
>>>> + elinux.org/Mikrobus
>>> I think this is a fallacy. I have boards that support Mikrobus - some of
>>> the SolidRun products do. I have several Mikrobus "click" boards.
>>>
>>> This help text seems to imply that Mikrobus click boards include an
>>> EEPROM that identify them, hence you make the support for mikroBUS
>>> depend on it. No, this is not the case - the click boards do not
>>> contain a 1-wire EEPROM.
>>>
>>> Please fetch a copy of the official Mikrobus specification which is
>>> available here:
>>>
>>> https://download.mikroe.com/documents/standards/mikrobus/mikrobus-standard-specification-v200.pdf
>>>
>>> and rather than creating something that is implementation specific but
>>> appears to be generic, create something that is generic with
>>> implementation specific extensions.
>>
>> I think you mean mikroBUS addon boards? mikroBUS is an open socket and click
>> boards™ are MikroElektronika’s brand of mikroBUS™ add-on boards.
>
> MikroElektronika _owns_ the standard for mikroBUS, they're the ones
> who publish it and it has their logo plastered all over it.
>
Yes, MikroE owns the standard but they do not prevent anyone from
creating new add-on boards or adding the sockets in commercially
available boards, with the only condition that custom mikroBUS add-on
boards cannot be marketed with the name "click" board.
>> So I think
>> all click boards™ do have clickID support, but yes, mikroBUS spec is not the
>> same as clickID and thus are not mutually dependent.
>
> None of the MikroElektronika "click" boards that I have (and thus
> officially produced boards) have any ID EEPROM on them, so your
> statement is false. For example, if you look at the "relay click"
> board schematic:
>
> https://download.mikroe.com/documents/add-on-boards/click/relay/relay-click-schematic-v100-a.pdf
>
> you will find no EEPROM.
>
> The "relay 3" click board also doesn't:
>
> https://download.mikroe.com/documents/add-on-boards/click/relay-3/relay-3-schematic-v100.pdf
>
> However, the "relay 4" click board does:
>
> https://download.mikroe.com/documents/add-on-boards/click/relay_4_click/Relay_4_Click_v100_Schematic.PDF
>
> Now, ClickID is relatively new. Note that the mikroBUS standard dates
> from 2011, with v2 coming out in 2015. A blog post introducing ClickID
> was posted in November 2023, just some 5 months ago, so that leaves an
> awful lot of click boards out there at the moment which have no EEPROM
> on them.
>
> If what you have written assumes that all click boards have this EEPROM
> then you are - in my opinion - intolerably constraining the usefulness
> of your idea for those of us who have click boards bought over the past
> few years, and this will confuse users who have these older boards.
> "I've enabled mikroBUS support in the kernel, but my board isn't
> recognised" will probably end up being a regular cry from people with
> this.
>
> So, I think you need to consider how to support the already vast number
> of click boards that do not support ClickID.
>
The same series actually can support mikroBUS add-on boards without
EEPROM as well, it exposes a sysfs interface similar to i2c new_device,
so all you need to do is to plug-in the add-on board, the pass the
manifest using this interface.
Example :
cat /lib/firmware/mikrobus/AMBIENT-2-CLICK.mnfb >
/sys/bus/mikrobus/devices/mikrobus-0/new_device
Reference:
https://docs.beagleboard.org/latest/boards/beagleplay/demos-and-tutorials/using-mikrobus.html#what-if-my-add-on-doesn-t-have-clickid
I am not sure if passing binary manifest blob using the sysfs interface
is an ideal solution, but the driver can support boards without EEPROM.
Actually the 150+ boards we have validated in the past did not have
EEPROM on all of them :
https://github.com/MikroElektronika/click_id/tree/main/manifests
> At the moment, my own personal solution is currently to hack the
> platform's DT file for the board I wish to use, creating a new variant
> of the platform which configures the SoC so the mikroBUS connector pins
> are appropriately configured. It would be good to get away from the need
> to do that.
Yes, the pain point with creating device tree overlays for mikroBUS
add-on boards is that you need an almost new overlays for each
permutation of the hardware (150+ add-on boards with driver support in
Linux connected on just BeagleBoard platforms like BeaglePlay[1 port],
PocketBeagle [2 port], mikroBUS cape on BB black[4 ports]) is more than
1000 overlays to maintain, this driver aims to split the platform
aspects of mikroBUS (pinmux, SPI/I2C/GPIO controller .etc) from the
add-on board information, thus requiring one device tree overlay per
port and just a single manifest describing the add-on board.
The reason for choosing greybus manifest for the identifier is that so
far we discussed only about physical mikroBUS ports on the board, but we
can also have mikroBUS ports on a remote microcontroller appearing on
host over greybus and there a device tree overlay solution does not work
as the standard identifier mechanism is greybus manifest, some details
on the remote greybus mikrobus port is available here:
https://docs.beagleboard.org/boards/beagleconnect/technology/index.html
Here is an old video of the concept working:
https://www.youtube.com/watch?v=JKrCRRuCw3c
To summarise, the driver with proper fixes and feedback incorporated can
support mikroBUS Click ID boards(plug and play), existing mikroBUS
add-on boards (with a clean way to pass the manifest to driver, slight
manual intervention compared to click ID boards) and also mikroBUS
add-on board connected to a remote microcontroller appearing over
greybus (not part of the series, but mentioning it so that it is clear
why greybus manifest is chosen as the descriptor format).
Thanks and Regards,
Vaishnav
>
Hi Ayush,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 61996c073c9b070922ad3a36c981ca6ddbea19a5]
url: https://github.com/intel-lab-lkp/linux/commits/Ayush-Singh/dt-bindings-misc-Add-mikrobus-connector/20240316-025407
base: 61996c073c9b070922ad3a36c981ca6ddbea19a5
patch link: https://lore.kernel.org/r/20240315184908.500352-8-ayushdevel1325%40gmail.com
patch subject: [PATCH v3 7/8] mikrobus: Add mikrobus driver
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240316/[email protected]/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
drivers/misc/mikrobus/mikrobus_manifest.c: In function 'mikrobus_manifest_parse_devices':
>> drivers/misc/mikrobus/mikrobus_manifest.c:422:13: warning: variable 'retval' set but not used [-Wunused-but-set-variable]
422 | int retval, devcount = 0;
| ^~~~~~
vim +/retval +422 drivers/misc/mikrobus/mikrobus_manifest.c
417
418 static int mikrobus_manifest_parse_devices(struct addon_board_info *board)
419 {
420 struct greybus_descriptor_device *desc_device;
421 struct manifest_desc *desc, *next;
> 422 int retval, devcount = 0;
423
424 list_for_each_entry_safe(desc, next, &board->manifest_descs, links) {
425 if (desc->type != GREYBUS_TYPE_DEVICE)
426 continue;
427
428 desc_device = desc->data;
429 retval = mikrobus_manifest_attach_device(board, desc_device);
430 devcount++;
431 }
432
433 return devcount;
434 }
435
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Ayush,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 61996c073c9b070922ad3a36c981ca6ddbea19a5]
url: https://github.com/intel-lab-lkp/linux/commits/Ayush-Singh/dt-bindings-misc-Add-mikrobus-connector/20240316-025407
base: 61996c073c9b070922ad3a36c981ca6ddbea19a5
patch link: https://lore.kernel.org/r/20240315184908.500352-8-ayushdevel1325%40gmail.com
patch subject: [PATCH v3 7/8] mikrobus: Add mikrobus driver
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240316/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 8f68022f8e6e54d1aeae4ed301f5a015963089b7)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from drivers/misc/mikrobus/mikrobus_id.c:17:
In file included from drivers/misc/mikrobus/mikrobus_core.h:14:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/misc/mikrobus/mikrobus_id.c:17:
In file included from drivers/misc/mikrobus/mikrobus_core.h:14:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/misc/mikrobus/mikrobus_id.c:17:
In file included from drivers/misc/mikrobus/mikrobus_core.h:14:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:328:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
In file included from drivers/misc/mikrobus/mikrobus_id.c:17:
In file included from drivers/misc/mikrobus/mikrobus_core.h:14:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:21:
In file included from include/linux/mm.h:2208:
include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
522 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/misc/mikrobus/mikrobus_id.c:45:60: warning: format specifies type 'unsigned long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
45 | pr_info("mikrobus_id: writing manifest size = %lu bytes", count);
| ~~~ ^~~~~
| %zu
include/linux/printk.h:530:34: note: expanded from macro 'pr_info'
530 | printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
| ~~~ ^~~~~~~~~~~
include/linux/printk.h:457:60: note: expanded from macro 'printk'
457 | #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
| ~~~ ^~~~~~~~~~~
include/linux/printk.h:429:19: note: expanded from macro 'printk_index_wrap'
429 | _p_func(_fmt, ##__VA_ARGS__); \
| ~~~~ ^~~~~~~~~~~
8 warnings generated.
vim +45 drivers/misc/mikrobus/mikrobus_id.c
28
29 static ssize_t mikrobus_manifest_store(struct device *device, struct device_attribute *attr,
30 const char *buf, size_t count)
31 {
32 u8 write_request[] = { W1_MIKROBUS_ID_WRITE_EEPROM,
33 W1_MIKROBUS_EEPROM_MANIFEST_START_PAGE };
34 u8 release_command = W1_MIKROBUS_ID_RELEASE_EEPROM;
35 struct w1_slave *sl = dev_to_w1_slave(device);
36 u16 crc, crc_read, pos = 0;
37 u8 status = 0;
38 int cnt;
39
40 if (count > W1_MIKROBUS_ID_EEPROM_SIZE)
41 return -ENOMEM;
42
43 mutex_lock(&sl->master->bus_mutex);
44
> 45 pr_info("mikrobus_id: writing manifest size = %lu bytes", count);
46 while (pos < count) {
47 if (w1_reset_select_slave(sl))
48 break;
49
50 w1_write_block(sl->master, write_request, sizeof(write_request));
51 crc = crc16(0, write_request, sizeof(write_request)) ^ 0xFFFF;
52 w1_read_block(sl->master, (u8 *)&crc_read, sizeof(crc_read));
53
54 if (crc != crc_read)
55 break;
56
57 for (cnt = 0; cnt < W1_MIKROBUS_ID_EEPROM_PAGE_SIZE; cnt++)
58 w1_write_8(sl->master, (u8)buf[cnt]);
59
60 crc = crc16(0, buf, W1_MIKROBUS_ID_EEPROM_PAGE_SIZE) ^ 0xFFFF;
61 usleep_range(1 * USEC_PER_MSEC, 2 * USEC_PER_MSEC);
62 w1_read_block(sl->master, (u8 *)&crc_read, sizeof(crc_read));
63
64 if (crc != crc_read)
65 break;
66
67 w1_write_8(sl->master, release_command);
68 usleep_range(10 * USEC_PER_MSEC, 15 * USEC_PER_MSEC);
69 status = w1_read_8(sl->master);
70 w1_read_block(sl->master, (u8 *)&crc_read, sizeof(crc_read));
71 crc = crc16(0, (u8 *)&release_command, sizeof(release_command)) ^ 0xFFFF;
72
73 if (status != W1_MIKROBUS_ID_RELEASE_EEPROM)
74 break;
75
76 if (crc != crc_read)
77 break;
78
79 buf += W1_MIKROBUS_ID_EEPROM_PAGE_SIZE;
80 pos += W1_MIKROBUS_ID_EEPROM_PAGE_SIZE;
81 write_request[1]++;
82 }
83
84 pr_info("mikrobus_id: manifest written bytes: %d", pos);
85 mutex_unlock(&sl->master->bus_mutex);
86
87 return count > pos ? count : pos;
88 }
89 static DEVICE_ATTR_WO(mikrobus_manifest);
90
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
> Are you sure this fits in Linux coding style limit (not checkpatch
limit, but the limit expressed by Linux coding style)?
Well, I am just using clang-format with column width of 100 instead of
80. The docs now say 80 is prefered rather than mandatory, so well I was
using 100 since I prefer that. If 80 is necessary or would make review
easier than I can just switch to it.
I will remove serdev, pwm, clickID and send a new patch with the minimal
driver and better commit messages as suggested with Vaishnav. It is
important to have good support for mikroBUS boards without clickID as well.
Thanks for all your feedback and time.
Ayush Singh
> Yes, I will add Co-author and Co-developed tags. I think I should use your
> ti email? I would have preferred to keep you as the author in the git commit
> but I could not get the patches applied cleanly back when I tried it.
Probably not required now, but
git commit --amend --author A U Thor <[email protected]>
allows you to change the author of a patch in git.
Andrew
On Sat, Mar 16, 2024 at 12:18:59AM +0530, Ayush Singh wrote:
> Add DT bindings for mikroBUS interface. MikroBUS is an open standard
> developed by MikroElektronika for connecting add-on boards to
> microcontrollers or microprocessors.
>
> Signed-off-by: Ayush Singh <[email protected]>
> ---
> .../bindings/misc/mikrobus-connector.yaml | 110 ++++++++++++++++++
> MAINTAINERS | 6 +
> 2 files changed, 116 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
>
> diff --git a/Documentation/devicetree/bindings/misc/mikrobus-connector.yaml b/Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
> new file mode 100644
> index 000000000000..6eace2c0dddc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/misc/mikrobus-connector.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: mikroBUS add-on board socket
> +
> +maintainers:
> + - Ayush Singh <[email protected]>
> +
> +properties:
> + compatible:
> + const: mikrobus-connector
> +
> + pinctrl-0: true
> + pinctrl-1: true
> + pinctrl-2: true
> + pinctrl-3: true
> + pinctrl-4: true
> + pinctrl-5: true
> + pinctrl-6: true
> + pinctrl-7: true
> + pinctrl-8: true
> +
> + pinctrl-names:
> + items:
> + - const: default
> + - const: pwm_default
> + - const: pwm_gpio
> + - const: uart_default
> + - const: uart_gpio
> + - const: i2c_default
> + - const: i2c_gpio
> + - const: spi_default
> + - const: spi_gpio
> +
> + mikrobus-gpios:
> + minItems: 11
> + maxItems: 12
What is each GPIO entry?
> +
> + i2c-adapter:
We already have i2c-bus and i2c-parent properties. Neither of those work
for you?
> + description: i2c adapter attached to the mikrobus socket.
> + $ref: /schemas/types.yaml#/definitions/phandle
> +
> + spi-controller:
> + description: spi bus number of the spi-master attached to the mikrobus socket.
> + $ref: /schemas/types.yaml#/definitions/phandle
> +
> + uart:
Nice and consistent. In 3 properties, we have 'adapter', 'controller'
and <null>...
Also, DT generally uses 'serial' rather than 'uart'.
> + description: uart port attached to the mikrobus socket
> + $ref: /schemas/types.yaml#/definitions/phandle
> +
> + pwms:
> + description: the pwm-controller corresponding to the mikroBUS PWM pin.
> + maxItems: 1
> +
> + spi-cs:
> + description: spi chip-select numbers corresponding to the chip-selects on the mikrobus socket.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + items:
> + - description: chip select corresponding to CS pin
> + - description: chip select corresponding to RST pin
How would someone handle any of the properties defined in
spi-peripheral-props.yaml?
Rob
A new version of the patch is up and can be found here:
https://lore.kernel.org/lkml/[email protected]/
On 3/18/24 02:29, Rob Herring wrote:
> On Sat, Mar 16, 2024 at 12:18:59AM +0530, Ayush Singh wrote:
>> Add DT bindings for mikroBUS interface. MikroBUS is an open standard
>> developed by MikroElektronika for connecting add-on boards to
>> microcontrollers or microprocessors.
>>
>> Signed-off-by: Ayush Singh <[email protected]>
>> ---
>> .../bindings/misc/mikrobus-connector.yaml | 110 ++++++++++++++++++
>> MAINTAINERS | 6 +
>> 2 files changed, 116 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/misc/mikrobus-connector.yaml b/Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
>> new file mode 100644
>> index 000000000000..6eace2c0dddc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
>> @@ -0,0 +1,110 @@
>> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/misc/mikrobus-connector.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: mikroBUS add-on board socket
>> +
>> +maintainers:
>> + - Ayush Singh <[email protected]>
>> +
>> +properties:
>> + compatible:
>> + const: mikrobus-connector
>> +
>> + pinctrl-0: true
>> + pinctrl-1: true
>> + pinctrl-2: true
>> + pinctrl-3: true
>> + pinctrl-4: true
>> + pinctrl-5: true
>> + pinctrl-6: true
>> + pinctrl-7: true
>> + pinctrl-8: true
>> +
>> + pinctrl-names:
>> + items:
>> + - const: default
>> + - const: pwm_default
>> + - const: pwm_gpio
>> + - const: uart_default
>> + - const: uart_gpio
>> + - const: i2c_default
>> + - const: i2c_gpio
>> + - const: spi_default
>> + - const: spi_gpio
>> +
>> + mikrobus-gpios:
>> + minItems: 11
>> + maxItems: 12
> What is each GPIO entry?
>
>> +
>> + i2c-adapter:
> We already have i2c-bus and i2c-parent properties. Neither of those work
> for you?
I think i2c-bus should work. Although I could only find information
about what it is supposed to be in some old kernel i2c.txt so is there a
general place for such properties to be discovered?
>> + description: i2c adapter attached to the mikrobus socket.
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> +
>> + spi-controller:
>> + description: spi bus number of the spi-master attached to the mikrobus socket.
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> +
>> + uart:
> Nice and consistent. In 3 properties, we have 'adapter', 'controller'
> and <null>...
Right. So the names I am currently using are from v2 of the patch and
are based on Linux kernel names for this. But yes, they probably need to
be changed since dt-bindings are not supposed to be tied to Linux. Not
sure if `spi-bus` and `serial-bus` are appropriate though, so maybe
`{spi, serial}-controller` is fine?
To explain why these are here in the first place, mikroBUS addon boards
are free to only use a few of these buses or multiple of these
simultaneously. Also, some of the properties of spi, i2c etc device
needs to be changed depending on the mikroBUS board (mostly described by
mikroBUS manifest). This means, the driver needs access to i2c adapter,
spi controller, serdev-controller, pwm associated with the mikroBUS
connector to configure them (or not use them in case of Not Connected)
and register the board.
> Also, DT generally uses 'serial' rather than 'uart'.
Noted
>> + description: uart port attached to the mikrobus socket
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> +
>> + pwms:
>> + description: the pwm-controller corresponding to the mikroBUS PWM pin.
>> + maxItems: 1
>> +
>> + spi-cs:
>> + description: spi chip-select numbers corresponding to the chip-selects on the mikrobus socket.
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + items:
>> + - description: chip select corresponding to CS pin
>> + - description: chip select corresponding to RST pin
> How would someone handle any of the properties defined in
> spi-peripheral-props.yaml?
>
>
> Rob
After taking a look at `spi-peripheral-props.yaml`, the properties
described here will actually be specified by mikroBUS manifest and thus
will be set by the driver after parsing the manifest.
If you are referring to keeping `spi-cs` in sync with `reg`, well I'm
not quite sure how to do it better than the current implementation.
Ayush Singh
On 16/03/2024 14:06, Ayush Singh wrote:
> > Are you sure this fits in Linux coding style limit (not checkpatch
> limit, but the limit expressed by Linux coding style)?
>
>
> Well, I am just using clang-format with column width of 100 instead of
> 80. The docs now say 80 is prefered rather than mandatory, so well I was
So you introduce your own style? Then consider it mandatory...
> using 100 since I prefer that. If 80 is necessary or would make review
> easier than I can just switch to it.
You do not choose your own coding style.
>
>
> I will remove serdev, pwm, clickID and send a new patch with the minimal
> driver and better commit messages as suggested with Vaishnav. It is
> important to have good support for mikroBUS boards without clickID as well.
Best regards,
Krzysztof
On 3/19/24 11:02, Krzysztof Kozlowski wrote:
> On 16/03/2024 14:06, Ayush Singh wrote:
>> > Are you sure this fits in Linux coding style limit (not checkpatch
>> limit, but the limit expressed by Linux coding style)?
>>
>>
>> Well, I am just using clang-format with column width of 100 instead of
>> 80. The docs now say 80 is prefered rather than mandatory, so well I was
> So you introduce your own style? Then consider it mandatory...
>
>> using 100 since I prefer that. If 80 is necessary or would make review
>> easier than I can just switch to it.
> You do not choose your own coding style.
>
>>
>> I will remove serdev, pwm, clickID and send a new patch with the minimal
>> driver and better commit messages as suggested with Vaishnav. It is
>> important to have good support for mikroBUS boards without clickID as well.
> Best regards,
> Krzysztof
>
I mean after the whole discussion about 80 vs 100 column line limit a
few years ago, and change in checkpatch behavior, I thought 100 was an
acceptable column length in the kernel, but I guess was mistaken, and 80
character is still mandatory? Not sure why there was a change in
checkpatch and docs though.
Regardless, I have switched 80 in the next patch since it is mandatory,
and I do not care as long as I can format using a formatter.
Ayush Singh
On 19/03/2024 07:59, Ayush Singh wrote:
> On 3/19/24 11:02, Krzysztof Kozlowski wrote:
>
>> On 16/03/2024 14:06, Ayush Singh wrote:
>>> > Are you sure this fits in Linux coding style limit (not checkpatch
>>> limit, but the limit expressed by Linux coding style)?
>>>
>>>
>>> Well, I am just using clang-format with column width of 100 instead of
>>> 80. The docs now say 80 is prefered rather than mandatory, so well I was
>> So you introduce your own style? Then consider it mandatory...
>>
>>> using 100 since I prefer that. If 80 is necessary or would make review
>>> easier than I can just switch to it.
>> You do not choose your own coding style.
>>
>>>
>>> I will remove serdev, pwm, clickID and send a new patch with the minimal
>>> driver and better commit messages as suggested with Vaishnav. It is
>>> important to have good support for mikroBUS boards without clickID as well.
>> Best regards,
>> Krzysztof
>>
>
> I mean after the whole discussion about 80 vs 100 column line limit a
Yeah, and the discussion was saying: use 80, unless code readability is
improved by using 100-limit.
> few years ago, and change in checkpatch behavior, I thought 100 was an
> acceptable column length in the kernel, but I guess was mistaken, and 80
> character is still mandatory? Not sure why there was a change in
> checkpatch and docs though.
You mistake checkpatch with coding style. What checkpatch tells you, is
a suggestion. It's not the coding style. The problem with checkpatch is
that people do not understand "why" it proposes something and they
implement its warnings literally, thus sometimes decreasing code
readability.
>
> Regardless, I have switched 80 in the next patch since it is mandatory,
> and I do not care as long as I can format using a formatter.
Please use wrapping as explained in coding style and deviate to 100
character limit only if it increases the readability.
Best regards,
Krzysztof
Hi Ayush,
On 16/03/24 03:11, Ayush Singh wrote:
> On 3/16/24 02:50, Vaishnav M A wrote:
>
>> Hi Ayush,
>>
>> On Sat, Mar 16, 2024 at 12:19 AM Ayush Singh
>> <[email protected]> wrote:
>>> MikroBUS is an open standard developed by MikroElektronika for
>>> connecting
>>> add-on boards to microcontrollers or microprocessors. It essentially
>>> allows you to easily expand the functionality of your main boards using
>>> these add-on boards.
>>>
>>> This patchset adds mikroBUS as a Linux bus type and provides a driver to
>>> parse, and flash mikroBUS manifest and register the mikroBUS board.
>>>
>> As Russel had provided the feedback, this patchset does not add support
>> for mikrobus, but a subset of mikrobus add-on boards which have a
>> 1-wire click ID EEPROM with an identifier blob that is similar to the
>> greybus
>> manifest. This series lacks the necessary context and details to the
>> specifications that is implemented.
>>
>> https://www.mikroe.com/clickid - you should atleast point to this specs.
>>
>>> The v1 and v2 of this patchset was submitted by Vaishnav M A back in
>>> 2020. This patchset also includes changes made over the years as part of
>>> BeagleBoards kernel.
>>>
>>> Link: https://www.mikroe.com/mikrobus
>>> Link: https://docs.beagleboard.org/latest/boards/beagleplay/
>>> Link:
>>> https://lore.kernel.org/lkml/[email protected]/ Patch v2
>>>
>> Thank you for making the effort to upstream this, arriving at the
>> latest revision of the public available click ID hardware took almost
>> 2-3 years
>> and I could not personally keep up with upstreaming, my sincere
>> apologies to
>> the maintainers that provided feedback on the earlier revisions. I
>> hope an
>> updated version of this series lands upstream with your work as the
>> efforts
>> made at BeagleBoard.org Foundation makes development simpler by adding
>> plug and play support to these add-on boards. Also this series
>> mentions only
>> the case where mikroBUS port is present physically on the board, but
>> there
>> can be mikroBUS ports appearing over greybus and that is the reason why
>> greybus manifest was chose as descriptor format - the series needs to
>> describe these details so that a reviewer has the necessary information
>> to review your patches. A link to beagleconnect is also helpful to
>> reviewers.
>>
>> https://docs.beagleboard.org/latest/projects/beagleconnect/index.html
>
>
> Yes, I left out the mikroBUS over greybus patches for now since this
> patch series is already too big.
>
Agreed, I would recommend splitting this series into logically separate
changes, for example the W1 EEPROM driver could be separate, some extra
features on the mikroBUS driver could be separate patches or be part of
a separate series later.
>>> Changes in v3:
>>> - Use phandle instead of busname for spi
>>> - Use spi board info for registering new device
>>> - Convert dt bindings to yaml
>>> - Add support for clickID
>>> - Code cleanup and style changes
>>> - Additions required to spi, serdev, w1 and regulator subsystems
>>>
>>> Changes in v2:
>>> - support for adding mikroBUS ports from DT overlays,
>>> - remove debug sysFS interface for adding mikrobus ports,
>>> - consider extended pin usage/deviations from mikrobus standard
>>> specifications
>>> - use greybus CPort protocol enum instead of new protocol enums
>>> - Fix cases of wrong indentation, ignoring return values, freeing
>>> allocated
>>> resources in case of errors and other style suggestions in v1 review.
>>>
>>> Ayush Singh (7):
>> It looks like the version you have sent is very similar to the
>> version I had previously updated for Beagleboard git with
>> only rebases and cleanup, but I don't see major functional
>> changes. I request you give credit to the original author
>> atleast as a Co-author with Co-developed by tag, As there
>> there was a significant amount of work done by myself to
>> come up with this specs and get everything working on close
>> to 150 mikrobus add-on boards on physical mikroBUS ports
>> and over greybus:
>> https://github.com/vaishnavachath/manifesto/tree/mikrobusv3/manifests
>
> Yes, I will add Co-author and Co-developed tags. I think I should use
> your ti email? I would have preferred to keep you as the author in the
> git commit but I could not get the patches applied cleanly back when I
> tried it.
>
Thank you, please keep yourself as the primary author as you are putting
in the effort to get the driver upstream and you will need to work on
multiple revisions to address maintainer feedback and I feel you should
get credit for that, please put my BeagleBoard.org e-mail with
Co-developed-by tag as most of the work was done before I moved to the
Linux team at TI.
>> The driver today is poorly written and was one of the first
>> Linux kernel development work I did while I was still in college
>> so I might have made a lot of blunders and just rebasing and
>> reposting will not get this to an acceptable state, here is what
>> I would recommend:
>>
>> * Drop all the unnecessary changes in the mikroBUS driver to
>> support fixed-regulators, fixed-clocks, serdev device .etc and
>> implement minimal changes to support the mikroBUS clickid
>> devices.
>>
>> * Provide necessary justification to why the particular descriptor
>> format (greybus manifest) is chosen, with pull request to manifesto
>> and greybus-specification.
>> https://github.com/projectara/greybus-spec
>> and similar to : https://github.com/projectara/manifesto/pull/2
>>
>> * Move the mikrobus W1 driver to w1/ subsytem, it is a standard
>> W1 EEPROM driver (also a standard part with updated family code)
>> * Keep a RFC series of changes where mikroBUS ports can appear over
>> greybus to justify why the identifier format needs to be extended greybus
>> manifest.
>>
>>> dt-bindings: misc: Add mikrobus-connector
>>> w1: Add w1_find_master_device
>> Dependent patches that goes to different subsytems should
>> be sent first separately to avoid confusion and then your series
>> should mention the necessary dependencies. (same for
>> spi).
>>
>>> spi: Make of_find_spi_controller_by_node() available
>>> regulator: fixed-helper: export regulator_register_always_on
>>> greybus: Add mikroBUS manifest types
>>> mikrobus: Add mikrobus driver
>>> dts: ti: k3-am625-beagleplay: Add mikroBUS
>> Send this patch as DONOTMERGE till your bindings are
>> accepted.
>
> Thanks, should I just add it in the message body? I cannot see anything
> in docs about that.
>
The reasoning behind this is that these patches go in to separate
maintainer trees and without the bindings merged first the device tree
changes cannot be validated, thus it is a usual practice to get the
bindings and driver merged first and the device tree changes to go in
the next cycle. Another alternative is you can point to your fork with
all the changes together.
>>> Vaishnav M A (1):
>>> serdev: add of_ helper to get serdev controller
>>>
>> Drop this from initial series,
>> I will provide further feedback from my TI e-mail,
>> Vaishnav Achath <[email protected]>
>>
>> Thank again for taking this up,
>>
>> Thanks and Regards,
>> Vaishnav
>>
>>> .../bindings/misc/mikrobus-connector.yaml | 110 ++
>>> MAINTAINERS | 7 +
>>> .../arm64/boot/dts/ti/k3-am625-beagleplay.dts | 76 +-
>>> drivers/misc/Kconfig | 1 +
>>> drivers/misc/Makefile | 1 +
>>> drivers/misc/mikrobus/Kconfig | 19 +
>>> drivers/misc/mikrobus/Makefile | 6 +
>>> drivers/misc/mikrobus/mikrobus_core.c | 942 ++++++++++++++++++
>>> drivers/misc/mikrobus/mikrobus_core.h | 201 ++++
>>> drivers/misc/mikrobus/mikrobus_id.c | 229 +++++
>>> drivers/misc/mikrobus/mikrobus_manifest.c | 502 ++++++++++
>>> drivers/misc/mikrobus/mikrobus_manifest.h | 20 +
>>> drivers/regulator/fixed-helper.c | 1 +
>>> drivers/spi/spi.c | 206 ++--
>>> drivers/tty/serdev/core.c | 19 +
>>> drivers/w1/w1.c | 6 +-
>>> drivers/w1/w1_int.c | 27 +
>>> include/linux/greybus/greybus_manifest.h | 49 +
>>> include/linux/serdev.h | 4 +
>>> include/linux/spi/spi.h | 4 +
>>> include/linux/w1.h | 1 +
>>> 21 files changed, 2318 insertions(+), 113 deletions(-)
>>> create mode 100644
>>> Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
>>> create mode 100644 drivers/misc/mikrobus/Kconfig
>>> create mode 100644 drivers/misc/mikrobus/Makefile
>>> create mode 100644 drivers/misc/mikrobus/mikrobus_core.c
>>> create mode 100644 drivers/misc/mikrobus/mikrobus_core.h
>>> create mode 100644 drivers/misc/mikrobus/mikrobus_id.c
>>> create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.c
>>> create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.h
>>>
>>>
>>> base-commit: 61996c073c9b070922ad3a36c981ca6ddbea19a5
>>> --
>>> 2.44.0
>>>
>
> I guess I will start with only i2c and spi support and go from there.
>
Agreed, even with those you can get close to 100 add-on boards working.
But do keep the extension to the greybus manifest .etc for all
buses/devices so that approvals for extending the greybus manifest is
common.
Thanks and Regards,
Vaishnav
Thanks and Regards,
Vaishnav
>
> Ayush Singh
>
>
Hi Ayush,
On Sat, Mar 16, 2024 at 12:19 AM Ayush Singh <[email protected]> wrote:
>
> MikroBUS is an open standard developed by MikroElektronika for connecting
> add-on boards to microcontrollers or microprocessors. It essentially
> allows you to easily expand the functionality of your main boards using
> these add-on boards.
>
> This patchset adds mikroBUS as a Linux bus type and provides a driver to
> parse, and flash mikroBUS manifest and register the mikroBUS board.
>
As Russel had provided the feedback, this patchset does not add support
for mikrobus, but a subset of mikrobus add-on boards which have a
1-wire click ID EEPROM with an identifier blob that is similar to the greybus
manifest. This series lacks the necessary context and details to the
specifications that is implemented.
https://www.mikroe.com/clickid - you should atleast point to this specs.
> The v1 and v2 of this patchset was submitted by Vaishnav M A back in
> 2020. This patchset also includes changes made over the years as part of
> BeagleBoards kernel.
>
> Link: https://www.mikroe.com/mikrobus
> Link: https://docs.beagleboard.org/latest/boards/beagleplay/
> Link: https://lore.kernel.org/lkml/[email protected]/ Patch v2
>
Thank you for making the effort to upstream this, arriving at the
latest revision of the public available click ID hardware took almost 2-3 years
and I could not personally keep up with upstreaming, my sincere apologies to
the maintainers that provided feedback on the earlier revisions. I hope an
updated version of this series lands upstream with your work as the efforts
made at BeagleBoard.org Foundation makes development simpler by adding
plug and play support to these add-on boards. Also this series mentions only
the case where mikroBUS port is present physically on the board, but there
can be mikroBUS ports appearing over greybus and that is the reason why
greybus manifest was chose as descriptor format - the series needs to
describe these details so that a reviewer has the necessary information
to review your patches. A link to beagleconnect is also helpful to reviewers.
https://docs.beagleboard.org/latest/projects/beagleconnect/index.html
> Changes in v3:
> - Use phandle instead of busname for spi
> - Use spi board info for registering new device
> - Convert dt bindings to yaml
> - Add support for clickID
> - Code cleanup and style changes
> - Additions required to spi, serdev, w1 and regulator subsystems
>
> Changes in v2:
> - support for adding mikroBUS ports from DT overlays,
> - remove debug sysFS interface for adding mikrobus ports,
> - consider extended pin usage/deviations from mikrobus standard
> specifications
> - use greybus CPort protocol enum instead of new protocol enums
> - Fix cases of wrong indentation, ignoring return values, freeing allocated
> resources in case of errors and other style suggestions in v1 review.
>
> Ayush Singh (7):
It looks like the version you have sent is very similar to the
version I had previously updated for Beagleboard git with
only rebases and cleanup, but I don't see major functional
changes. I request you give credit to the original author
atleast as a Co-author with Co-developed by tag, As there
there was a significant amount of work done by myself to
come up with this specs and get everything working on close
to 150 mikrobus add-on boards on physical mikroBUS ports
and over greybus:
https://github.com/vaishnavachath/manifesto/tree/mikrobusv3/manifests
The driver today is poorly written and was one of the first
Linux kernel development work I did while I was still in college
so I might have made a lot of blunders and just rebasing and
reposting will not get this to an acceptable state, here is what
I would recommend:
* Drop all the unnecessary changes in the mikroBUS driver to
support fixed-regulators, fixed-clocks, serdev device .etc and
implement minimal changes to support the mikroBUS clickid
devices.
* Provide necessary justification to why the particular descriptor
format (greybus manifest) is chosen, with pull request to manifesto
and greybus-specification.
https://github.com/projectara/greybus-spec
and similar to : https://github.com/projectara/manifesto/pull/2
* Move the mikrobus W1 driver to w1/ subsytem, it is a standard
W1 EEPROM driver (also a standard part with updated family code)
* Keep a RFC series of changes where mikroBUS ports can appear over
greybus to justify why the identifier format needs to be extended greybus
manifest.
> dt-bindings: misc: Add mikrobus-connector
> w1: Add w1_find_master_device
Dependent patches that goes to different subsytems should
be sent first separately to avoid confusion and then your series
should mention the necessary dependencies. (same for
spi).
> spi: Make of_find_spi_controller_by_node() available
> regulator: fixed-helper: export regulator_register_always_on
> greybus: Add mikroBUS manifest types
> mikrobus: Add mikrobus driver
> dts: ti: k3-am625-beagleplay: Add mikroBUS
Send this patch as DONOTMERGE till your bindings are
accepted.
>
> Vaishnav M A (1):
> serdev: add of_ helper to get serdev controller
>
Drop this from initial series,
I will provide further feedback from my TI e-mail,
Vaishnav Achath <[email protected]>
Thank again for taking this up,
Thanks and Regards,
Vaishnav
> .../bindings/misc/mikrobus-connector.yaml | 110 ++
> MAINTAINERS | 7 +
> .../arm64/boot/dts/ti/k3-am625-beagleplay.dts | 76 +-
> drivers/misc/Kconfig | 1 +
> drivers/misc/Makefile | 1 +
> drivers/misc/mikrobus/Kconfig | 19 +
> drivers/misc/mikrobus/Makefile | 6 +
> drivers/misc/mikrobus/mikrobus_core.c | 942 ++++++++++++++++++
> drivers/misc/mikrobus/mikrobus_core.h | 201 ++++
> drivers/misc/mikrobus/mikrobus_id.c | 229 +++++
> drivers/misc/mikrobus/mikrobus_manifest.c | 502 ++++++++++
> drivers/misc/mikrobus/mikrobus_manifest.h | 20 +
> drivers/regulator/fixed-helper.c | 1 +
> drivers/spi/spi.c | 206 ++--
> drivers/tty/serdev/core.c | 19 +
> drivers/w1/w1.c | 6 +-
> drivers/w1/w1_int.c | 27 +
> include/linux/greybus/greybus_manifest.h | 49 +
> include/linux/serdev.h | 4 +
> include/linux/spi/spi.h | 4 +
> include/linux/w1.h | 1 +
> 21 files changed, 2318 insertions(+), 113 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/misc/mikrobus-connector.yaml
> create mode 100644 drivers/misc/mikrobus/Kconfig
> create mode 100644 drivers/misc/mikrobus/Makefile
> create mode 100644 drivers/misc/mikrobus/mikrobus_core.c
> create mode 100644 drivers/misc/mikrobus/mikrobus_core.h
> create mode 100644 drivers/misc/mikrobus/mikrobus_id.c
> create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.c
> create mode 100644 drivers/misc/mikrobus/mikrobus_manifest.h
>
>
> base-commit: 61996c073c9b070922ad3a36c981ca6ddbea19a5
> --
> 2.44.0
>
On Sat, Mar 16, 2024 at 12:19:00AM +0530, Ayush Singh wrote:
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index afb1cc4606c5..ce8a3f93f2ef 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -673,9 +673,9 @@ static int __w1_attach_slave_device(struct w1_slave *sl)
> sl->dev.of_node = of_find_matching_node(sl->master->dev.of_node,
> sl->family->of_match_table);
>
> - dev_set_name(&sl->dev, "%02x-%012llx",
> - (unsigned int) sl->reg_num.family,
> - (unsigned long long) sl->reg_num.id);
> + dev_set_name(&sl->dev, "%s-%02x-%012llx", sl->master->name,
> + (unsigned int)sl->reg_num.family,
> + (unsigned long long)sl->reg_num.id);
This is a user visible change likely to cause breakage. I know that I've
written programmes for reading the DS18B20 temperature probes that
depend on the sysfs device name remaining stable. The same is likely
true of other program authors.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Sat, Mar 16, 2024 at 02:17:24AM +0530, Ayush Singh wrote:
> On 3/16/24 01:02, Russell King (Oracle) wrote:
>
> > On Sat, Mar 16, 2024 at 12:19:05AM +0530, Ayush Singh wrote:
> > > diff --git a/drivers/misc/mikrobus/Kconfig b/drivers/misc/mikrobus/Kconfig
> > > new file mode 100644
> > > index 000000000000..f0770006b4fe
> > > --- /dev/null
> > > +++ b/drivers/misc/mikrobus/Kconfig
> > > @@ -0,0 +1,19 @@
> > > +menuconfig MIKROBUS
> > > + tristate "Module for instantiating devices on mikroBUS ports"
> > > + depends on GPIOLIB
> > > + depends on W1
> > > + depends on W1_MASTER_GPIO
> > > + help
> > > + This option enables the mikroBUS driver. mikroBUS is an add-on
> > > + board socket standard that offers maximum expandability with
> > > + the smallest number of pins. The mikroBUS driver instantiates
> > > + devices on a mikroBUS port described by identifying data present
> > > + in an add-on board resident EEPROM, more details on the mikroBUS
> > > + driver support and discussion can be found in this eLinux wiki :
> > > + elinux.org/Mikrobus
> > I think this is a fallacy. I have boards that support Mikrobus - some of
> > the SolidRun products do. I have several Mikrobus "click" boards.
> >
> > This help text seems to imply that Mikrobus click boards include an
> > EEPROM that identify them, hence you make the support for mikroBUS
> > depend on it. No, this is not the case - the click boards do not
> > contain a 1-wire EEPROM.
> >
> > Please fetch a copy of the official Mikrobus specification which is
> > available here:
> >
> > https://download.mikroe.com/documents/standards/mikrobus/mikrobus-standard-specification-v200.pdf
> >
> > and rather than creating something that is implementation specific but
> > appears to be generic, create something that is generic with
> > implementation specific extensions.
>
> I think you mean mikroBUS addon boards? mikroBUS is an open socket and click
> boards™ are MikroElektronika’s brand of mikroBUS™ add-on boards.
MikroElektronika _owns_ the standard for mikroBUS, they're the ones
who publish it and it has their logo plastered all over it.
> So I think
> all click boards™ do have clickID support, but yes, mikroBUS spec is not the
> same as clickID and thus are not mutually dependent.
None of the MikroElektronika "click" boards that I have (and thus
officially produced boards) have any ID EEPROM on them, so your
statement is false. For example, if you look at the "relay click"
board schematic:
https://download.mikroe.com/documents/add-on-boards/click/relay/relay-click-schematic-v100-a.pdf
you will find no EEPROM.
The "relay 3" click board also doesn't:
https://download.mikroe.com/documents/add-on-boards/click/relay-3/relay-3-schematic-v100.pdf
However, the "relay 4" click board does:
https://download.mikroe.com/documents/add-on-boards/click/relay_4_click/Relay_4_Click_v100_Schematic.PDF
Now, ClickID is relatively new. Note that the mikroBUS standard dates
from 2011, with v2 coming out in 2015. A blog post introducing ClickID
was posted in November 2023, just some 5 months ago, so that leaves an
awful lot of click boards out there at the moment which have no EEPROM
on them.
If what you have written assumes that all click boards have this EEPROM
then you are - in my opinion - intolerably constraining the usefulness
of your idea for those of us who have click boards bought over the past
few years, and this will confuse users who have these older boards.
"I've enabled mikroBUS support in the kernel, but my board isn't
recognised" will probably end up being a regular cry from people with
this.
So, I think you need to consider how to support the already vast number
of click boards that do not support ClickID.
At the moment, my own personal solution is currently to hack the
platform's DT file for the board I wish to use, creating a new variant
of the platform which configures the SoC so the mikroBUS connector pins
are appropriately configured. It would be good to get away from the need
to do that.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Sat, Mar 16, 2024 at 12:19:04AM +0530, Ayush Singh wrote:
> Add data structures for parsing mikroBUS manifests, which are based on
> greybus manifest.
>
> Signed-off-by: Ayush Singh <[email protected]>
> ---
> include/linux/greybus/greybus_manifest.h | 49 ++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/include/linux/greybus/greybus_manifest.h b/include/linux/greybus/greybus_manifest.h
> index bef9eb2093e9..83241e19d9b3 100644
> --- a/include/linux/greybus/greybus_manifest.h
> +++ b/include/linux/greybus/greybus_manifest.h
> @@ -23,6 +23,9 @@ enum greybus_descriptor_type {
> GREYBUS_TYPE_STRING = 0x02,
> GREYBUS_TYPE_BUNDLE = 0x03,
> GREYBUS_TYPE_CPORT = 0x04,
> + GREYBUS_TYPE_MIKROBUS = 0x05,
> + GREYBUS_TYPE_PROPERTY = 0x06,
> + GREYBUS_TYPE_DEVICE = 0x07,
These need approval in the spec before we can add them here.
And you are adding 3 different things here, not just one. Shouldn't
this be 3 patches?
> };
>
> enum greybus_protocol {
> @@ -151,6 +154,49 @@ struct greybus_descriptor_cport {
> __u8 protocol_id; /* enum greybus_protocol */
> } __packed;
>
> +/*
> + * A mikrobus descriptor is used to describe the details
> + * about the bus ocnfiguration for the add-on board
> + * connected to the mikrobus port.
> + */
> +struct greybus_descriptor_mikrobus {
> + __u8 pin_state[12];
> +} __packed;
> +
> +/*
> + * A property descriptor is used to pass named properties
> + * to device drivers through the unified device properties
> + * interface under linux/property.h
> + */
> +struct greybus_descriptor_property {
> + __u8 length;
> + __u8 id;
> + __u8 propname_stringid;
> + __u8 type;
> + __u8 value[];
Don't we have a "counted-by" marking that we can use to show how big
value[] here is?
> +} __packed;
> +
> +/*
> + * A device descriptor is used to describe the
> + * details required by a add-on board device
> + * driver.
> + */
> +struct greybus_descriptor_device {
> + __u8 id;
> + __u8 driver_stringid;
> + __u8 protocol;
> + __u8 reg;
> + __le32 max_speed_hz;
> + __u8 irq;
> + __u8 irq_type;
> + __u8 mode;
> + __u8 prop_link;
> + __u8 gpio_link;
> + __u8 reg_link;
> + __u8 clock_link;
> + __u8 pad[1];
Why the padding?
And this looks like a greybus thing, not a mikrobus thing, right? Some
description of exactly what this is and what it does would be good.
thanks,
greg k-h