There are some situations where it is interesting to load/remove serdev
devices dynamically, like during board bring-up or when we are
developing a new driver or for devices that are neither described via
ACPI or device tree.
This implementation allows the creation of serdev devices via sysfs,
in a similar way as the i2c bus allows sysfs instantiation [1].
It also opens the door to create platform drivers for specific
platforms, such us notebooks or industrial computers, when their serial
devices are not described via DT or ACPI.
This patchset also supports automatic module load via udev/modprobe,
simplifying its use by the final user.
Currently, serdev does not manage ports that do not have a serdev device
defined at boot time. This implementation adds a new serdev module,
ttydev, that provides the same functionality of tty (it calls the
original one), but can be unloaded.
TL/DR:
When we want to create a new device, we just run:
root@qt5022:~# echo ttydev > /sys/bus/serial/devices/serial0/new_device
This will create a new device:
root@qt5022:~# ls /sys/bus/serial/devices/serial0-0/
driver modalias power subsystem tty uevent
And load the required driver to use it:
root@qt5022:~# lsmod | grep serdev_ttydev
serdev_ttydev 16384 0
The device can be removed:
root@qt5022:~#
echo serial0-0 > /sys/bus/serial/devices/serial0/delete_device
And now we can connect a new device:
root@qt5022:~# echo hci-ti > /sys/bus/serial/devices/serial0/new_device
[1] https://www.kernel.org/doc/Documentation/i2c/instantiating-devices
Ricardo Ribalda Delgado (19):
serdev: Add id_table to serdev_device_driver
Bluetooth: hci_bcm: Add serdev_id_table
Bluetooth: hci_ll: Add serdev_id_table
Bluetooth: hci_nokia: Add serdev_id_table
serdev: Introduce modalias field
serdev: Support bus matching with modalias field
serdev: Allows dynamic creation of devices via sysfs
serdev: Provide modalias attribute for modalias devices
serdev: Provide modalias uevent for modalias devices
file2alias: Support for serdev devices
Bluetooth: hci_bcm: MODULE_DEVICE_TABLE(serdev)
Bluetooth: hci_ll: MODULE_DEVICE_TABLE(serdev)
Bluetooth: hci_nokia: MODULE_DEVICE_TABLE(serdev)
mfd: rave-sp: MODULE_DEVICE_TABLE(serdev)
net: qualcomm: MODULE_DEVICE_TABLE(serdev)
serdev: ttyport: Move serport structure to its own header
serdev: Mark controllers compatible with ttyport
serdev: ttydev: Serdev driver that creates an standard TTY port
serdev: Instantiate a ttydev serdev if acpi and of fails
drivers/bluetooth/hci_bcm.c | 8 ++
drivers/bluetooth/hci_ll.c | 19 ++++
drivers/bluetooth/hci_nokia.c | 8 ++
drivers/mfd/rave-sp.c | 7 ++
drivers/net/ethernet/qualcomm/qca_uart.c | 7 ++
drivers/tty/serdev/Kconfig | 11 ++
drivers/tty/serdev/Makefile | 2 +
drivers/tty/serdev/core.c | 133 +++++++++++++++++++++--
drivers/tty/serdev/serdev-ttydev.c | 70 ++++++++++++
drivers/tty/serdev/serdev-ttyport.c | 10 +-
drivers/tty/serdev/serport.h | 16 +++
include/linux/mod_devicetable.h | 10 ++
include/linux/serdev.h | 4 +
scripts/mod/devicetable-offsets.c | 3 +
scripts/mod/file2alias.c | 11 ++
15 files changed, 304 insertions(+), 15 deletions(-)
create mode 100644 drivers/tty/serdev/serdev-ttydev.c
create mode 100644 drivers/tty/serdev/serport.h
--
2.17.0
Currently, serdev device driver can only be used with devices that are
nodes of a device tree, or are part of the ACPI table.
Id_table will be used for devices that are created at runtime or that
are not part of the device tree nor the ACPI table.
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
include/linux/mod_devicetable.h | 10 ++++++++++
include/linux/serdev.h | 2 ++
2 files changed, 12 insertions(+)
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 7d361be2e24f..1877a4e43f1b 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -448,6 +448,16 @@ struct pci_epf_device_id {
kernel_ulong_t driver_data;
};
+/* serdev */
+
+#define SERDEV_NAME_SIZE 32
+#define SERDEV_MODULE_PREFIX "serdev:"
+
+struct serdev_device_id {
+ char name[SERDEV_NAME_SIZE];
+ kernel_ulong_t driver_data; /* Data private to the driver */
+};
+
/* spi */
#define SPI_NAME_SIZE 32
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index f153b2c7f0cd..62f1b085a794 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -15,6 +15,7 @@
#include <linux/types.h>
#include <linux/device.h>
+#include <linux/mod_devicetable.h>
#include <linux/termios.h>
#include <linux/delay.h>
@@ -68,6 +69,7 @@ static inline struct serdev_device *to_serdev_device(struct device *d)
* @remove: unbinds this driver from the serdev device.
*/
struct serdev_device_driver {
+ const struct serdev_device_id *id_table;
struct device_driver driver;
int (*probe)(struct serdev_device *);
void (*remove)(struct serdev_device *);
--
2.17.0
If a serdev ttyport controller does not have an acpi nor an of child,
create a ttydev as a child of that controller.
Doing this allows the removal, addition and replacement of ttydev devices
at runtime.
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/tty/serdev/core.c | 36 ++++++++++++++++++++++++++++++++----
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 9414700e6442..34295dacfb84 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -619,6 +619,27 @@ static inline int acpi_serdev_register_devices(struct serdev_controller *ctrl)
}
#endif /* CONFIG_ACPI */
+
+#if IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV)
+static int serdev_controller_add_ttydev(struct serdev_controller *ctrl)
+{
+ struct serdev_device *serdev;
+ int err;
+
+ serdev = serdev_device_alloc(ctrl);
+ if (!serdev)
+ return -ENOMEM;
+
+ strcpy(serdev->modalias, "ttydev");
+
+ err = serdev_device_add(serdev);
+ if (err)
+ serdev_device_put(serdev);
+
+ return err;
+}
+#endif
+
/**
* serdev_controller_add() - Add an serdev controller
* @ctrl: controller to be registered.
@@ -628,7 +649,7 @@ static inline int acpi_serdev_register_devices(struct serdev_controller *ctrl)
*/
int serdev_controller_add(struct serdev_controller *ctrl)
{
- int ret_of, ret_acpi, ret;
+ int ret_of, ret_acpi, ret, ret_tty = -ENODEV;
/* Can't register until after driver model init */
if (WARN_ON(!is_registered))
@@ -640,9 +661,16 @@ int serdev_controller_add(struct serdev_controller *ctrl)
ret_of = of_serdev_register_devices(ctrl);
ret_acpi = acpi_serdev_register_devices(ctrl);
- if (ret_of && ret_acpi) {
- dev_dbg(&ctrl->dev, "no devices registered: of:%d acpi:%d\n",
- ret_of, ret_acpi);
+
+#if IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV)
+ if (ret_of && ret_acpi && ctrl->is_ttyport)
+ ret_tty = serdev_controller_add_ttydev(ctrl);
+#endif
+
+ if (ret_of && ret_acpi && ret_tty) {
+ dev_dbg(&ctrl->dev,
+ "no devices registered: of:%d acpi:%d tty:%d\n",
+ ret_of, ret_acpi, ret_tty);
ret = -ENODEV;
goto out_dev_del;
}
--
2.17.0
Standard TTY port that can be loaded/unloaded via serdev sysfs. This
serdev driver can only be used by serdev controllers that are compatible
with ttyport.
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/tty/serdev/Kconfig | 11 +++++
drivers/tty/serdev/Makefile | 2 +
drivers/tty/serdev/serdev-ttydev.c | 70 ++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+)
create mode 100644 drivers/tty/serdev/serdev-ttydev.c
diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
index 1dbc8352e027..d19bf689a424 100644
--- a/drivers/tty/serdev/Kconfig
+++ b/drivers/tty/serdev/Kconfig
@@ -21,4 +21,15 @@ config SERIAL_DEV_CTRL_TTYPORT
depends on SERIAL_DEV_BUS != m
default y
+config SERIAL_DEV_CTRL_TTYDEV
+ tristate "TTY port dynamically loaded by the Serial Device Bus"
+ help
+ Say Y here if you want to create a bridge driver between the Serial
+ device bus and the TTY chardevice. This driver can be dynamically
+ loaded/unloaded by the Serial Device Bus.
+
+ If unsure, say Y.
+ depends on SERIAL_DEV_CTRL_TTYPORT
+ default m
+
endif
diff --git a/drivers/tty/serdev/Makefile b/drivers/tty/serdev/Makefile
index 0cbdb9444d9d..5c807b77d12d 100644
--- a/drivers/tty/serdev/Makefile
+++ b/drivers/tty/serdev/Makefile
@@ -3,3 +3,5 @@ serdev-objs := core.o
obj-$(CONFIG_SERIAL_DEV_BUS) += serdev.o
obj-$(CONFIG_SERIAL_DEV_CTRL_TTYPORT) += serdev-ttyport.o
+
+obj-$(CONFIG_SERIAL_DEV_CTRL_TTYDEV) += serdev-ttydev.o
diff --git a/drivers/tty/serdev/serdev-ttydev.c b/drivers/tty/serdev/serdev-ttydev.c
new file mode 100644
index 000000000000..66479d6534dd
--- /dev/null
+++ b/drivers/tty/serdev/serdev-ttydev.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Ricardo Ribalda <[email protected]>
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/serdev.h>
+#include <linux/tty.h>
+#include "serport.h"
+
+static int ttydev_serdev_probe(struct serdev_device *serdev)
+{
+ struct serdev_controller *ctrl = serdev->ctrl;
+ struct serport *serport;
+ struct device *dev;
+
+ if (!ctrl->is_ttyport)
+ return -ENODEV;
+
+ serport = serdev_controller_get_drvdata(ctrl);
+
+ dev = tty_register_device_attr(serport->tty_drv, serport->tty_idx,
+ &serdev->dev, NULL, NULL);
+
+ return dev ? 0 : PTR_ERR(dev);
+}
+
+static void ttydev_serdev_remove(struct serdev_device *serdev)
+{
+ struct serdev_controller *ctrl = serdev->ctrl;
+ struct serport *serport;
+
+ serport = serdev_controller_get_drvdata(ctrl);
+ tty_unregister_device(serport->tty_drv, serport->tty_idx);
+}
+
+static const struct serdev_device_id ttydev_serdev_id[] = {
+ { "ttydev", },
+ { "ttydev_serdev", },
+ {}
+};
+MODULE_DEVICE_TABLE(serdev, ttydev_serdev_id);
+
+static struct serdev_device_driver ttydev_serdev_driver = {
+ .probe = ttydev_serdev_probe,
+ .remove = ttydev_serdev_remove,
+ .driver = {
+ .name = "ttydev_serdev",
+ },
+ .id_table = ttydev_serdev_id,
+};
+
+static int __init ttydev_serdev_init(void)
+{
+ return serdev_device_driver_register(&ttydev_serdev_driver);
+}
+module_init(ttydev_serdev_init);
+
+static void __exit ttydev_serdev_exit(void)
+{
+ return serdev_device_driver_unregister(&ttydev_serdev_driver);
+}
+module_exit(ttydev_serdev_exit);
+
+MODULE_AUTHOR("Ricardo Ribalda <[email protected]>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Serdev to ttydev module");
--
2.17.0
This patch allows file2alias to generate the proper module headers to
support serdev modalias drivers.
Eg:
root@qt5022:~# modinfo serdev:ttydev | grep alias
alias: serdev:ttydev_serdev
alias: serdev:ttydev
root@qt5022:~#
cat /lib/modules/4.16.0-qtec-standard/modules.alias | grep serdev
alias serdev:ttydev_serdev serdev_ttydev
alias serdev:ttydev serdev_ttydev
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Philippe Ombredanne <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
scripts/mod/devicetable-offsets.c | 3 +++
scripts/mod/file2alias.c | 11 +++++++++++
2 files changed, 14 insertions(+)
diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index 9fad6afe4c41..6082c41b7ad7 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -142,6 +142,9 @@ int main(void)
DEVID(i2c_device_id);
DEVID_FIELD(i2c_device_id, name);
+ DEVID(serdev_device_id);
+ DEVID_FIELD(serdev_device_id, name);
+
DEVID(spi_device_id);
DEVID_FIELD(spi_device_id, name);
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index b9beeaa4695b..dce6df3a159a 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -955,6 +955,17 @@ static int do_i2c_entry(const char *filename, void *symval,
}
ADD_TO_DEVTABLE("i2c", i2c_device_id, do_i2c_entry);
+/* Looks like: serdev:S */
+static int do_serdev_entry(const char *filename, void *symval,
+ char *alias)
+{
+ DEF_FIELD_ADDR(symval, serdev_device_id, name);
+ sprintf(alias, SERDEV_MODULE_PREFIX "%s", *name);
+
+ return 1;
+}
+ADD_TO_DEVTABLE("serdev", serdev_device_id, do_serdev_entry);
+
/* Looks like: spi:S */
static int do_spi_entry(const char *filename, void *symval,
char *alias)
--
2.17.0
This way we can reuse this structure in other modules.
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/tty/serdev/serdev-ttyport.c | 9 +--------
drivers/tty/serdev/serport.h | 16 ++++++++++++++++
2 files changed, 17 insertions(+), 8 deletions(-)
create mode 100644 drivers/tty/serdev/serport.h
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index fa1672993b4c..4acc5f41dc67 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -7,17 +7,10 @@
#include <linux/tty.h>
#include <linux/tty_driver.h>
#include <linux/poll.h>
+#include "serport.h"
#define SERPORT_ACTIVE 1
-struct serport {
- struct tty_port *port;
- struct tty_struct *tty;
- struct tty_driver *tty_drv;
- int tty_idx;
- unsigned long flags;
-};
-
/*
* Callback functions from the tty port.
*/
diff --git a/drivers/tty/serdev/serport.h b/drivers/tty/serdev/serport.h
new file mode 100644
index 000000000000..15bc1ff6326e
--- /dev/null
+++ b/drivers/tty/serdev/serport.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2016-2017 Linaro Ltd., Rob Herring <[email protected]>
+ */
+#ifndef _SERPORT_H
+#define _SERPORT_H
+
+struct serport {
+ struct tty_port *port;
+ struct tty_struct *tty;
+ struct tty_driver *tty_drv;
+ int tty_idx;
+ unsigned long flags;
+};
+
+#endif
--
2.17.0
Export serdev table to the module header, allowing module autoload via
udev/modprobe.
Cc: Marcel Holtmann <[email protected]>
Cc: Johan Hedberg <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: [email protected]
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/bluetooth/hci_bcm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index f4d7846c06b8..ff0fd3502a90 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -1327,8 +1327,10 @@ MODULE_DEVICE_TABLE(of, bcm_bluetooth_of_match);
static const struct serdev_device_id bcm_serdev_id[] = {
{ "bcm43438-bt", },
+ { "hci_uart_bcm", },
{}
};
+MODULE_DEVICE_TABLE(serdev, bcm_serdev_id);
static struct serdev_device_driver bcm_serdev_driver = {
.probe = bcm_serdev_probe,
--
2.17.0
This allows us to treat differently this controllers, by creating a tty
compatibility layer.
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/tty/serdev/serdev-ttyport.c | 1 +
include/linux/serdev.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index 4acc5f41dc67..ae961260e4a4 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -276,6 +276,7 @@ struct device *serdev_tty_port_register(struct tty_port *port,
serport->tty_drv = drv;
ctrl->ops = &ctrl_ops;
+ ctrl->is_ttyport = true;
old_ops = port->client_ops;
port->client_ops = &client_ops;
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index bb3b9599c652..07d63933bdb9 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -116,6 +116,7 @@ struct serdev_controller {
unsigned int nr;
struct serdev_device *serdev;
const struct serdev_controller_ops *ops;
+ bool is_ttyport;
};
static inline struct serdev_controller *to_serdev_controller(struct device *d)
--
2.17.0
Export serdev table to the module header, allowing module autoload via
udev/modprobe.
Cc: Lee Jones <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/mfd/rave-sp.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/mfd/rave-sp.c b/drivers/mfd/rave-sp.c
index 5c858e784a89..807c237e061b 100644
--- a/drivers/mfd/rave-sp.c
+++ b/drivers/mfd/rave-sp.c
@@ -694,12 +694,19 @@ static int rave_sp_probe(struct serdev_device *serdev)
MODULE_DEVICE_TABLE(of, rave_sp_dt_ids);
+static struct serdev_device_id rave_sp_serdev_id[] = {
+ { "rave-sp", },
+ {},
+};
+MODULE_DEVICE_TABLE(serdev, rave_sp_serdev_id);
+
static struct serdev_device_driver rave_sp_drv = {
.probe = rave_sp_probe,
.driver = {
.name = "rave-sp",
.of_match_table = rave_sp_dt_ids,
},
+ .id_table = rave_sp_serdev_id,
};
module_serdev_device_driver(rave_sp_drv);
--
2.17.0
Export serdev table to the module header, allowing module autoload via
udev/modprobe.
Cc: Marcel Holtmann <[email protected]>
Cc: Johan Hedberg <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: [email protected]
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/bluetooth/hci_nokia.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
index e32dfcd56b8d..c283e1ca6064 100644
--- a/drivers/bluetooth/hci_nokia.c
+++ b/drivers/bluetooth/hci_nokia.c
@@ -803,8 +803,10 @@ MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
{ "hp4-bluetooth", },
+ { "nokia-bluetooth", },
{},
};
+MODULE_DEVICE_TABLE(serdev, nokia_bluetooth_serdev_id);
static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
.probe = nokia_bluetooth_serdev_probe,
--
2.17.0
Export serdev table to the module header, allowing module autoload via
udev/modprobe.
Cc: Marcel Holtmann <[email protected]>
Cc: Johan Hedberg <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: [email protected]
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/bluetooth/hci_ll.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index c31942c9b466..5c36d8967a00 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -789,8 +789,10 @@ static struct serdev_device_id hci_ti_id[] = {
{ "wl1831-st", },
{ "wl1835-st", },
{ "wl1837-st", },
+ { "hci-ti", },
{},
};
+MODULE_DEVICE_TABLE(serdev, hci_ti_id);
static struct serdev_device_driver hci_ti_drv = {
.driver = {
--
2.17.0
Export serdev table to the module header, allowing module autoload via
udev/modprobe.
Cc: Lino Sanfilippo <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Stefan Wahren <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: [email protected]
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/net/ethernet/qualcomm/qca_uart.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/qualcomm/qca_uart.c b/drivers/net/ethernet/qualcomm/qca_uart.c
index db6068cd7a1f..bb7aed805083 100644
--- a/drivers/net/ethernet/qualcomm/qca_uart.c
+++ b/drivers/net/ethernet/qualcomm/qca_uart.c
@@ -405,6 +405,12 @@ static void qca_uart_remove(struct serdev_device *serdev)
free_netdev(qca->net_dev);
}
+static struct serdev_device_id qca_uart_serdev_id[] = {
+ { QCAUART_DRV_NAME, },
+ {},
+};
+MODULE_DEVICE_TABLE(serdev, qca_uart_serdev_id);
+
static struct serdev_device_driver qca_uart_driver = {
.probe = qca_uart_probe,
.remove = qca_uart_remove,
@@ -412,6 +418,7 @@ static struct serdev_device_driver qca_uart_driver = {
.name = QCAUART_DRV_NAME,
.of_match_table = of_match_ptr(qca_uart_of_match),
},
+ .id_table = qca_uart_serdev_id,
};
module_serdev_device_driver(qca_uart_driver);
--
2.17.0
Describe which hardware is supported by the current driver.
Cc: Marcel Holtmann <[email protected]>
Cc: Johan Hedberg <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: [email protected]
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/bluetooth/hci_ll.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index 27e414b4e3a2..c31942c9b466 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -776,6 +776,22 @@ static const struct of_device_id hci_ti_of_match[] = {
};
MODULE_DEVICE_TABLE(of, hci_ti_of_match);
+static struct serdev_device_id hci_ti_id[] = {
+ { "cc2560", },
+ { "wl1271-st", },
+ { "wl1273-st", },
+ { "wl1281-st", },
+ { "wl1283-st", },
+ { "wl1285-st", },
+ { "wl1801-st", },
+ { "wl1805-st", },
+ { "wl1807-st", },
+ { "wl1831-st", },
+ { "wl1835-st", },
+ { "wl1837-st", },
+ {},
+};
+
static struct serdev_device_driver hci_ti_drv = {
.driver = {
.name = "hci-ti",
@@ -783,6 +799,7 @@ static struct serdev_device_driver hci_ti_drv = {
},
.probe = hci_ti_probe,
.remove = hci_ti_remove,
+ .id_table = hci_ti_id,
};
#else
#define ll_setup NULL
--
2.17.0
Describe which hardware is supported by the current driver.
Cc: Marcel Holtmann <[email protected]>
Cc: Johan Hedberg <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: [email protected]
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/bluetooth/hci_bcm.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 441f5e1deb11..f4d7846c06b8 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -1325,6 +1325,11 @@ static const struct of_device_id bcm_bluetooth_of_match[] = {
MODULE_DEVICE_TABLE(of, bcm_bluetooth_of_match);
#endif
+static const struct serdev_device_id bcm_serdev_id[] = {
+ { "bcm43438-bt", },
+ {}
+};
+
static struct serdev_device_driver bcm_serdev_driver = {
.probe = bcm_serdev_probe,
.remove = bcm_serdev_remove,
@@ -1334,6 +1339,7 @@ static struct serdev_device_driver bcm_serdev_driver = {
.acpi_match_table = ACPI_PTR(bcm_acpi_match),
.pm = &bcm_pm_ops,
},
+ .id_table = bcm_serdev_id,
};
int __init bcm_init(void)
--
2.17.0
Allow creating and deleting devices via sysfs. Devices created will be
matched to serdev drivers via modalias (the string provided by the user)
and deleted via their name. Eg:
# Create device
root@qt5022:~# echo ttydev > /sys/bus/serial/devices/serial0/new_device
# Delete device
root@qt5022:~#
echo serial0-0 > /sys/bus/serial/devices/serial0/delete_device
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/tty/serdev/core.c | 60 +++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index 2c79f47fc0db..e695fa649a6d 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -75,7 +75,67 @@ static void serdev_ctrl_release(struct device *dev)
kfree(ctrl);
}
+static ssize_t
+new_device_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct serdev_controller *ctrl = to_serdev_controller(dev);
+ struct serdev_device *serdev;
+ int err;
+ char *nline;
+
+ serdev = serdev_device_alloc(ctrl);
+ if (!serdev)
+ return -ENOMEM;
+
+ nline = strchr(buf, '\n');
+ if (nline)
+ *nline = '\0';
+
+ strncpy(serdev->modalias, buf, SERDEV_NAME_SIZE);
+
+ err = serdev_device_add(serdev);
+ if (err) {
+ serdev_device_put(serdev);
+ return err;
+ }
+
+ return count;
+}
+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 serdev_controller *ctrl = to_serdev_controller(dev);
+ struct serdev_device *serdev = ctrl->serdev;
+ char *nline;
+
+ nline = strchr(buf, '\n');
+ if (nline)
+ *nline = '\0';
+
+ if (!ctrl->serdev ||
+ strncmp(dev_name(&serdev->dev), buf, SERDEV_NAME_SIZE))
+ return -ENODEV;
+
+ serdev_device_remove(serdev);
+
+ return count;
+}
+static DEVICE_ATTR_IGNORE_LOCKDEP(delete_device, 0200, NULL,
+ delete_device_store);
+
+static struct attribute *serdev_ctrl_attrs[] = {
+ &dev_attr_new_device.attr,
+ &dev_attr_delete_device.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(serdev_ctrl);
+
static const struct device_type serdev_ctrl_type = {
+ .groups = serdev_ctrl_groups,
.release = serdev_ctrl_release,
};
--
2.17.0
Match devices to drivers by their modalias when the ACPI and the OF
match fails.
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/tty/serdev/core.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index df93b727e984..2c79f47fc0db 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -79,8 +79,23 @@ static const struct device_type serdev_ctrl_type = {
.release = serdev_ctrl_release,
};
+static int serdev_match_id(const struct serdev_device_id *id,
+ const struct serdev_device *sdev)
+{
+ while (id->name[0]) {
+ if (!strcmp(sdev->modalias, id->name))
+ return 1;
+ id++;
+ }
+
+ return 0;
+}
+
static int serdev_device_match(struct device *dev, struct device_driver *drv)
{
+ const struct serdev_device *sdev = to_serdev_device(dev);
+ const struct serdev_device_driver *sdrv = to_serdev_device_driver(drv);
+
if (!is_serdev_device(dev))
return 0;
@@ -88,7 +103,13 @@ static int serdev_device_match(struct device *dev, struct device_driver *drv)
if (acpi_driver_match_device(dev, drv))
return 1;
- return of_driver_match_device(dev, drv);
+ if (of_driver_match_device(dev, drv))
+ return 1;
+
+ if (sdrv->id_table)
+ return serdev_match_id(sdrv->id_table, sdev);
+
+ return strcmp(sdev->modalias, drv->name) == 0;
}
/**
--
2.17.0
Name of the driver to use with this device, or an alias for that name,
or an alias for the part.
Required for hardware that is neither an of_node nor part of the ACPI
table.
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
include/linux/serdev.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 62f1b085a794..bb3b9599c652 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -54,6 +54,7 @@ struct serdev_device {
const struct serdev_device_ops *ops;
struct completion write_comp;
struct mutex write_lock;
+ char modalias[SERDEV_NAME_SIZE];
};
static inline struct serdev_device *to_serdev_device(struct device *d)
--
2.17.0
Create the sysfs uevent for modalias devices. This is required by newer
versions of udev for autoload modules.
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/tty/serdev/core.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index a9c935f68076..9414700e6442 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -46,6 +46,7 @@ ATTRIBUTE_GROUPS(serdev_device);
static int serdev_device_uevent(struct device *dev, struct kobj_uevent_env *env)
{
int rc;
+ struct serdev_device *serdev = to_serdev_device(dev);
/* TODO: platform modalias */
@@ -53,7 +54,11 @@ static int serdev_device_uevent(struct device *dev, struct kobj_uevent_env *env)
if (rc != -ENODEV)
return rc;
- return of_device_uevent_modalias(dev, env);
+ if (rc != of_device_uevent_modalias(dev, env))
+ return rc;
+
+ return add_uevent_var(env, "MODALIAS=%s%s", SERDEV_MODULE_PREFIX,
+ serdev->modalias);
}
static void serdev_device_release(struct device *dev)
--
2.17.0
Create modalias sysfs attribute for modalias devices.
This is required by modprobe/udev to autoload the serdev driver.
Eg:
root@qt5022:~# cat /sys/bus/serial/devices/serial1-0/modalias
serdev:ttydev
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Jiri Slaby <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/tty/serdev/core.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index e695fa649a6d..a9c935f68076 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -23,12 +23,17 @@ static ssize_t modalias_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
int len;
+ struct serdev_device *serdev = to_serdev_device(dev);
len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
if (len != -ENODEV)
return len;
- return of_device_modalias(dev, buf, PAGE_SIZE);
+ len = of_device_modalias(dev, buf, PAGE_SIZE);
+ if (len != -ENODEV)
+ return len;
+
+ return sprintf(buf, "%s%s\n", SERDEV_MODULE_PREFIX, serdev->modalias);
}
static DEVICE_ATTR_RO(modalias);
--
2.17.0
Describe which hardware is supported by the current driver.
Cc: Marcel Holtmann <[email protected]>
Cc: Johan Hedberg <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: [email protected]
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/bluetooth/hci_nokia.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
index 3539fd03f47e..e32dfcd56b8d 100644
--- a/drivers/bluetooth/hci_nokia.c
+++ b/drivers/bluetooth/hci_nokia.c
@@ -801,6 +801,11 @@ static const struct of_device_id nokia_bluetooth_of_match[] = {
MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
#endif
+static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
+ { "hp4-bluetooth", },
+ {},
+};
+
static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
.probe = nokia_bluetooth_serdev_probe,
.remove = nokia_bluetooth_serdev_remove,
@@ -809,6 +814,7 @@ static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
.pm = &nokia_bluetooth_pm_ops,
.of_match_table = of_match_ptr(nokia_bluetooth_of_match),
},
+ .id_table = nokia_bluetooth_serdev_id,
};
module_serdev_device_driver(nokia_bluetooth_serdev_driver);
--
2.17.0
On Tue, May 29, 2018 at 8:10 AM, Ricardo Ribalda Delgado
<[email protected]> wrote:
> Allow creating and deleting devices via sysfs. Devices created will be
> matched to serdev drivers via modalias (the string provided by the user)
> and deleted via their name. Eg:
>
> # Create device
> root@qt5022:~# echo ttydev > /sys/bus/serial/devices/serial0/new_device
>
> # Delete device
> root@qt5022:~#
> echo serial0-0 > /sys/bus/serial/devices/serial0/delete_device
I think the model here should be the kernel provides dummy slave
device for each serial port and then you can use bind and unbind to
bind to a particular driver.
Rob
Hi Rob
On Tue, May 29, 2018 at 5:38 PM Rob Herring <[email protected]> wrote:
> On Tue, May 29, 2018 at 8:10 AM, Ricardo Ribalda Delgado
> <[email protected]> wrote:
> > Allow creating and deleting devices via sysfs. Devices created will be
> > matched to serdev drivers via modalias (the string provided by the user)
> > and deleted via their name. Eg:
> >
> > # Create device
> > root@qt5022:~# echo ttydev > /sys/bus/serial/devices/serial0/new_device
> >
> > # Delete device
> > root@qt5022:~#
> > echo serial0-0 > /sys/bus/serial/devices/serial0/delete_device
> I think the model here should be the kernel provides dummy slave
> device for each serial port and then you can use bind and unbind to
> bind to a particular driver.
I have been researching a bit that approach, but I found a couple of issues:
- With the bind/unbind you need to modprobe manually the module. Something
like
modprobe myserdev
echo myserdev > bind
- You need one module per part_number, with modalias you can have a
different alias per module
- I guess that the final user will appreciate that the serdev has the same
API as other serial slow bus (i2c).
ccing Wolfram becase maybe he has some feedback to same from his experience
with the i2c bus.
Thanks!
> Rob
--
Ricardo Ribalda
Using the right address for Wolfram
On Tue, May 29, 2018 at 6:30 PM Ricardo Ribalda Delgado <
[email protected]> wrote:
> Hi Rob
> On Tue, May 29, 2018 at 5:38 PM Rob Herring <[email protected]> wrote:
> > On Tue, May 29, 2018 at 8:10 AM, Ricardo Ribalda Delgado
> > <[email protected]> wrote:
> > > Allow creating and deleting devices via sysfs. Devices created will be
> > > matched to serdev drivers via modalias (the string provided by the
user)
> > > and deleted via their name. Eg:
> > >
> > > # Create device
> > > root@qt5022:~# echo ttydev >
/sys/bus/serial/devices/serial0/new_device
> > >
> > > # Delete device
> > > root@qt5022:~#
> > > echo serial0-0 > /sys/bus/serial/devices/serial0/delete_device
> > I think the model here should be the kernel provides dummy slave
> > device for each serial port and then you can use bind and unbind to
> > bind to a particular driver.
> I have been researching a bit that approach, but I found a couple of
issues:
> - With the bind/unbind you need to modprobe manually the module. Something
> like
> modprobe myserdev
> echo myserdev > bind
> - You need one module per part_number, with modalias you can have a
> different alias per module
> - I guess that the final user will appreciate that the serdev has the same
> API as other serial slow bus (i2c).
> ccing Wolfram becase maybe he has some feedback to same from his
experience
> with the i2c bus.
> Thanks!
> > Rob
> --
> Ricardo Ribalda
--
Ricardo Ribalda
On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
<[email protected]> wrote:
> Allow creating and deleting devices via sysfs. Devices created will be
> matched to serdev drivers via modalias (the string provided by the user)
> and deleted via their name.
> + int err;
> + char *nline;
Better to read in reversed order.
> + nline = strchr(buf, '\n');
> + if (nline)
> + *nline = '\0';
strim() / strstrip() ?
> + nline = strchr(buf, '\n');
> + if (nline)
> + *nline = '\0';
Ditto.
> +static DEVICE_ATTR_IGNORE_LOCKDEP(delete_device, 0200, NULL,
> + delete_device_store);
Perhaps leave it on one line?
--
With Best Regards,
Andy Shevchenko
Hi Andy,
On Tue, May 29, 2018 at 10:35 PM Andy Shevchenko <[email protected]>
wrote:
> On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
> <[email protected]> wrote:
> > Allow creating and deleting devices via sysfs. Devices created will be
> > matched to serdev drivers via modalias (the string provided by the user)
> > and deleted via their name.
> > + int err;
> > + char *nline;
> Better to read in reversed order.
> > + nline = strchr(buf, '\n');
> > + if (nline)
> > + *nline = '\0';
> strim() / strstrip() ?
> > + nline = strchr(buf, '\n');
> > + if (nline)
> > + *nline = '\0';
> Ditto.
> > +static DEVICE_ATTR_IGNORE_LOCKDEP(delete_device, 0200, NULL,
> > + delete_device_store);
> Perhaps leave it on one line?
> --
> With Best Regards,
> Andy Shevchenko
Thanks for your review. I am working on v2 at
https://github.com/ribalda/linux/tree/serdev2 it fixes your comments except
the line cut for DEVICE_ATTR, I want also to make checkpatch happy ;)
Will send v2 to the list after I got more feedback, you can of course ping
me if there is something in my tree that it is not fixed properly.
Cheers
--
Ricardo Ribalda
Hi Ricardo,
Am 29.05.2018 um 15:10 schrieb Ricardo Ribalda Delgado:
> Export serdev table to the module header, allowing module autoload via
> udev/modprobe.
>
> Cc: Lino Sanfilippo <[email protected]>
> Cc: David S. Miller <[email protected]>
> Cc: Stefan Wahren <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Johan Hovold <[email protected]>
> Cc: [email protected]
> Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
> ---
> drivers/net/ethernet/qualcomm/qca_uart.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/ethernet/qualcomm/qca_uart.c b/drivers/net/ethernet/qualcomm/qca_uart.c
> index db6068cd7a1f..bb7aed805083 100644
> --- a/drivers/net/ethernet/qualcomm/qca_uart.c
> +++ b/drivers/net/ethernet/qualcomm/qca_uart.c
> @@ -405,6 +405,12 @@ static void qca_uart_remove(struct serdev_device *serdev)
> free_netdev(qca->net_dev);
> }
>
> +static struct serdev_device_id qca_uart_serdev_id[] = {
> + { QCAUART_DRV_NAME, },
i guess the id must be stable, so in case someone changes the driver
name this has unexpected side effects?
Apart from that i'm fine with this patch.
Thanks
Stefan
> + {},
> +};
> +MODULE_DEVICE_TABLE(serdev, qca_uart_serdev_id);
> +
> static struct serdev_device_driver qca_uart_driver = {
> .probe = qca_uart_probe,
> .remove = qca_uart_remove,
> @@ -412,6 +418,7 @@ static struct serdev_device_driver qca_uart_driver = {
> .name = QCAUART_DRV_NAME,
> .of_match_table = of_match_ptr(qca_uart_of_match),
> },
> + .id_table = qca_uart_serdev_id,
> };
>
> module_serdev_device_driver(qca_uart_driver);
On Tue, May 29, 2018 at 4:09 PM, Ricardo Ribalda Delgado
<[email protected]> wrote:
> Describe which hardware is supported by the current driver.
> +static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
> + { "hp4-bluetooth", },
> + {},
Terminator line better w/o comma.
> +};
--
With Best Regards,
Andy Shevchenko
On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
<[email protected]> wrote:
> Standard TTY port that can be loaded/unloaded via serdev sysfs. This
> serdev driver can only be used by serdev controllers that are compatible
> with ttyport.
> +config SERIAL_DEV_CTRL_TTYDEV
> + tristate "TTY port dynamically loaded by the Serial Device Bus"
> + help
> + Say Y here if you want to create a bridge driver between the Serial
> + device bus and the TTY chardevice. This driver can be dynamically
> + loaded/unloaded by the Serial Device Bus.
> +
> + If unsure, say Y.
> + depends on SERIAL_DEV_CTRL_TTYPORT
> + default m
Hmm... Can't we survive w/o this by default?
> +static int __init ttydev_serdev_init(void)
> +{
> + return serdev_device_driver_register(&ttydev_serdev_driver);
> +}
> +module_init(ttydev_serdev_init);
> +
> +static void __exit ttydev_serdev_exit(void)
> +{
> + return serdev_device_driver_unregister(&ttydev_serdev_driver);
> +}
> +module_exit(ttydev_serdev_exit);
Isn't above is just a macro in serdev.h?
I.e. module_serdev_device_driver().
--
With Best Regards,
Andy Shevchenko
On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
<[email protected]> wrote:
> If a serdev ttyport controller does not have an acpi nor an of child,
> create a ttydev as a child of that controller.
>
> Doing this allows the removal, addition and replacement of ttydev devices
> at runtime.
> @@ -619,6 +619,27 @@ static inline int acpi_serdev_register_devices(struct serdev_controller *ctrl)
> }
> #endif /* CONFIG_ACPI */
>
> +
Redundant blank line.
> +#if IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV)
> - if (ret_of && ret_acpi) {
> - dev_dbg(&ctrl->dev, "no devices registered: of:%d acpi:%d\n",
> - ret_of, ret_acpi);
> +
> +#if IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV)
> + if (ret_of && ret_acpi && ctrl->is_ttyport)
> + ret_tty = serdev_controller_add_ttydev(ctrl);
> +#endif
> +
> + if (ret_of && ret_acpi && ret_tty) {
> + dev_dbg(&ctrl->dev,
> + "no devices registered: of:%d acpi:%d tty:%d\n",
> + ret_of, ret_acpi, ret_tty);
> ret = -ENODEV;
> goto out_dev_del;
> }
Wouldn't be better to leave above if-condition and introduce your
stuff inside it?
--
With Best Regards,
Andy Shevchenko
Hi Ricardo,
> Describe which hardware is supported by the current driver.
>
> Cc: Marcel Holtmann <[email protected]>
> Cc: Johan Hedberg <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Johan Hovold <[email protected]>
> Cc: [email protected]
> Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
> ---
> drivers/bluetooth/hci_nokia.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
> index 3539fd03f47e..e32dfcd56b8d 100644
> --- a/drivers/bluetooth/hci_nokia.c
> +++ b/drivers/bluetooth/hci_nokia.c
> @@ -801,6 +801,11 @@ static const struct of_device_id nokia_bluetooth_of_match[] = {
> MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
> #endif
>
> +static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
> + { "hp4-bluetooth", },
> + {},
> +};
> +
> static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> .probe = nokia_bluetooth_serdev_probe,
> .remove = nokia_bluetooth_serdev_remove,
> @@ -809,6 +814,7 @@ static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> .pm = &nokia_bluetooth_pm_ops,
> .of_match_table = of_match_ptr(nokia_bluetooth_of_match),
> },
> + .id_table = nokia_bluetooth_serdev_id,
> };
I would actually skip this hardware. First of all it is such a dedicated custom Nokia transport and hardware, and secondly it is no longer produced anyway.
Regards
Marcel
Hi Andy
On Tue, Jun 5, 2018 at 3:42 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
> <[email protected]> wrote:
> > Standard TTY port that can be loaded/unloaded via serdev sysfs. This
> > serdev driver can only be used by serdev controllers that are compatible
> > with ttyport.
>
> > +config SERIAL_DEV_CTRL_TTYDEV
> > + tristate "TTY port dynamically loaded by the Serial Device Bus"
> > + help
> > + Say Y here if you want to create a bridge driver between the Serial
> > + device bus and the TTY chardevice. This driver can be dynamically
> > + loaded/unloaded by the Serial Device Bus.
> > +
> > + If unsure, say Y.
> > + depends on SERIAL_DEV_CTRL_TTYPORT
>
> > + default m
>
> Hmm... Can't we survive w/o this by default?
If this module is not available and serdev is enabled the user would
miss his /dev/ttyS* nodes, so I rather leave this on.
>
> > +static int __init ttydev_serdev_init(void)
> > +{
> > + return serdev_device_driver_register(&ttydev_serdev_driver);
> > +}
> > +module_init(ttydev_serdev_init);
> > +
> > +static void __exit ttydev_serdev_exit(void)
> > +{
> > + return serdev_device_driver_unregister(&ttydev_serdev_driver);
> > +}
> > +module_exit(ttydev_serdev_exit);
>
> Isn't above is just a macro in serdev.h?
> I.e. module_serdev_device_driver().
ACK, thanks!
>
> --
> With Best Regards,
> Andy Shevchenko
Best regards
--
Ricardo Ribalda
Hi Andy,
On Tue, Jun 5, 2018 at 3:44 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
> <[email protected]> wrote:
> > If a serdev ttyport controller does not have an acpi nor an of child,
> > create a ttydev as a child of that controller.
> >
> > Doing this allows the removal, addition and replacement of ttydev devices
> > at runtime.
>
> > @@ -619,6 +619,27 @@ static inline int acpi_serdev_register_devices(struct serdev_controller *ctrl)
> > }
> > #endif /* CONFIG_ACPI */
> >
> > +
>
> Redundant blank line.
ACK
>
> > +#if IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV)
>
> > - if (ret_of && ret_acpi) {
> > - dev_dbg(&ctrl->dev, "no devices registered: of:%d acpi:%d\n",
> > - ret_of, ret_acpi);
> > +
> > +#if IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV)
> > + if (ret_of && ret_acpi && ctrl->is_ttyport)
> > + ret_tty = serdev_controller_add_ttydev(ctrl);
> > +#endif
> > +
> > + if (ret_of && ret_acpi && ret_tty) {
> > + dev_dbg(&ctrl->dev,
> > + "no devices registered: of:%d acpi:%d tty:%d\n",
> > + ret_of, ret_acpi, ret_tty);
> > ret = -ENODEV;
> > goto out_dev_del;
> > }
>
> Wouldn't be better to leave above if-condition and introduce your
> stuff inside it?
I have redesign the logic in the function on the next version
Thanks
--
Ricardo Ribalda
Hi Andy,
On Wed, Jun 6, 2018 at 8:58 AM Ricardo Ribalda Delgado
<[email protected]> wrote:
>
> Hi Andy
> On Tue, Jun 5, 2018 at 3:42 PM Andy Shevchenko
> <[email protected]> wrote:
> >
> > On Tue, May 29, 2018 at 4:10 PM, Ricardo Ribalda Delgado
> > <[email protected]> wrote:
> > > Standard TTY port that can be loaded/unloaded via serdev sysfs. This
> > > serdev driver can only be used by serdev controllers that are compatible
> > > with ttyport.
> >
> > > +config SERIAL_DEV_CTRL_TTYDEV
> > > + tristate "TTY port dynamically loaded by the Serial Device Bus"
> > > + help
> > > + Say Y here if you want to create a bridge driver between the Serial
> > > + device bus and the TTY chardevice. This driver can be dynamically
> > > + loaded/unloaded by the Serial Device Bus.
> > > +
> > > + If unsure, say Y.
> > > + depends on SERIAL_DEV_CTRL_TTYPORT
> >
> > > + default m
> >
> > Hmm... Can't we survive w/o this by default?
>
> If this module is not available and serdev is enabled the user would
> miss his /dev/ttyS* nodes, so I rather leave this on.
Sorry, brain fart. This is exactly what #if
IS_ENABLED(CONFIG_SERIAL_DEV_CTRL_TTYDEV) prevents.
I have defaulted the module to n as you suggested. You can take a look
to the patches that I plan to submit tomorrow at:
https://github.com/ribalda/linux/tree/serdev2
>
> >
> > > +static int __init ttydev_serdev_init(void)
> > > +{
> > > + return serdev_device_driver_register(&ttydev_serdev_driver);
> > > +}
> > > +module_init(ttydev_serdev_init);
> > > +
> > > +static void __exit ttydev_serdev_exit(void)
> > > +{
> > > + return serdev_device_driver_unregister(&ttydev_serdev_driver);
> > > +}
> > > +module_exit(ttydev_serdev_exit);
> >
> > Isn't above is just a macro in serdev.h?
> > I.e. module_serdev_device_driver().
>
> ACK, thanks!
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
>
> Best regards
>
> --
> Ricardo Ribalda
--
Ricardo Ribalda
On Wed, Jun 6, 2018 at 10:47 AM, Ricardo Ribalda Delgado
<[email protected]> wrote:
> Hi Andy,
> On Wed, Jun 6, 2018 at 8:58 AM Ricardo Ribalda Delgado
> <[email protected]> wrote:
> I have defaulted the module to n as you suggested. You can take a look
> to the patches that I plan to submit tomorrow at:
>
> https://github.com/ribalda/linux/tree/serdev2
n _is_ default. So, just remove a line.
--
With Best Regards,
Andy Shevchenko
On Wed, Jun 6, 2018 at 11:55 AM Andy Shevchenko
<[email protected]> wrote:
> >
> > https://github.com/ribalda/linux/tree/serdev2
>
> n _is_ default. So, just remove a line.
>
Done. Thanks!
> --
> With Best Regards,
> Andy Shevchenko
--
Ricardo Ribalda
Hi!
> > Describe which hardware is supported by the current driver.
> >
> > Cc: Marcel Holtmann <[email protected]>
> > Cc: Johan Hedberg <[email protected]>
> > Cc: Rob Herring <[email protected]>
> > Cc: Johan Hovold <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
> > ---
> > drivers/bluetooth/hci_nokia.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
> > index 3539fd03f47e..e32dfcd56b8d 100644
> > --- a/drivers/bluetooth/hci_nokia.c
> > +++ b/drivers/bluetooth/hci_nokia.c
> > @@ -801,6 +801,11 @@ static const struct of_device_id nokia_bluetooth_of_match[] = {
> > MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
> > #endif
> >
> > +static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
> > + { "hp4-bluetooth", },
> > + {},
> > +};
> > +
> > static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> > .probe = nokia_bluetooth_serdev_probe,
> > .remove = nokia_bluetooth_serdev_remove,
> > @@ -809,6 +814,7 @@ static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> > .pm = &nokia_bluetooth_pm_ops,
> > .of_match_table = of_match_ptr(nokia_bluetooth_of_match),
> > },
> > + .id_table = nokia_bluetooth_serdev_id,
> > };
>
> I would actually skip this hardware. First of all it is such a dedicated custom Nokia transport and hardware, and secondly it is no longer produced anyway.
>
Would it make sense to cc: sre here? We want good support even for old
hardware, and this is n9/n950, it is still on "top ten supported
phones" list... Probably even top 5.
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi!
> >>> + { "hp4-bluetooth", },
> >>> + {},
> >>> +};
> >>> +
> >>> static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> >>> .probe = nokia_bluetooth_serdev_probe,
> >>> .remove = nokia_bluetooth_serdev_remove,
> >>> @@ -809,6 +814,7 @@ static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> >>> .pm = &nokia_bluetooth_pm_ops,
> >>> .of_match_table = of_match_ptr(nokia_bluetooth_of_match),
> >>> },
> >>> + .id_table = nokia_bluetooth_serdev_id,
> >>> };
> >>
> >> I would actually skip this hardware. First of all it is such a dedicated custom Nokia transport and hardware, and secondly it is no longer produced anyway.
> >>
> >
> > Would it make sense to cc: sre here? We want good support even for old
> > hardware, and this is n9/n950, it is still on "top ten supported
> > phones" list... Probably even top 5.
>
> but that is not what this patch series is about. We do not need new_id kinda support for the existing hardware. My point is there will be no newly designed hardware using this driver.
>
You are right, new hardware with that protocol sounds unlikely at this
point.
Sorry for the noise,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Pavel,
>>> Describe which hardware is supported by the current driver.
>>>
>>> Cc: Marcel Holtmann <[email protected]>
>>> Cc: Johan Hedberg <[email protected]>
>>> Cc: Rob Herring <[email protected]>
>>> Cc: Johan Hovold <[email protected]>
>>> Cc: [email protected]
>>> Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
>>> ---
>>> drivers/bluetooth/hci_nokia.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
>>> index 3539fd03f47e..e32dfcd56b8d 100644
>>> --- a/drivers/bluetooth/hci_nokia.c
>>> +++ b/drivers/bluetooth/hci_nokia.c
>>> @@ -801,6 +801,11 @@ static const struct of_device_id nokia_bluetooth_of_match[] = {
>>> MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
>>> #endif
>>>
>>> +static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
>>> + { "hp4-bluetooth", },
>>> + {},
>>> +};
>>> +
>>> static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
>>> .probe = nokia_bluetooth_serdev_probe,
>>> .remove = nokia_bluetooth_serdev_remove,
>>> @@ -809,6 +814,7 @@ static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
>>> .pm = &nokia_bluetooth_pm_ops,
>>> .of_match_table = of_match_ptr(nokia_bluetooth_of_match),
>>> },
>>> + .id_table = nokia_bluetooth_serdev_id,
>>> };
>>
>> I would actually skip this hardware. First of all it is such a dedicated custom Nokia transport and hardware, and secondly it is no longer produced anyway.
>>
>
> Would it make sense to cc: sre here? We want good support even for old
> hardware, and this is n9/n950, it is still on "top ten supported
> phones" list... Probably even top 5.
but that is not what this patch series is about. We do not need new_id kinda support for the existing hardware. My point is there will be no newly designed hardware using this driver.
Regards
Marcel