diff mbox series

[RFC,2/2] bluetooth/hci_h4: add serdev support

Message ID 20221108055531.2176793-3-dominique.martinet@atmark-techno.com
State New
Headers
Series Add serdev support for hci h4 |

Commit Message

Dominique Martinet Nov. 8, 2022, 5:55 a.m. UTC 
  adding serdev support to hci_h4 allows users to define h4 bluetooth
controllers in dts files

This allows users of bluetooth modules with an uart h4 interface to use
dt bindings instead of manually running btattach

Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
---
 drivers/bluetooth/Kconfig  |  1 +
 drivers/bluetooth/hci_h4.c | 65 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

Comments

Andrew Lunn Nov. 8, 2022, 8:38 p.m. UTC | #1 
> +static int h4_probe(struct serdev_device *serdev)
> +{
> +	struct h4_device *h4dev;
> +	struct hci_uart *hu;
> +	int ret;
> +	u32 speed;
> +
> +	h4dev = devm_kzalloc(&serdev->dev, sizeof(*h4dev), GFP_KERNEL);
> +	if (!h4dev)
> +		return -ENOMEM;
> +
> +	hu = &h4dev->hu;
> +
> +	hu->serdev = serdev;
> +	ret = device_property_read_u32(&serdev->dev, "max-speed", &speed);
> +	if (!ret) {
> +		/* h4 does not have any baudrate handling:
> +		 * user oper speed from the start
> +		 */
> +		hu->init_speed = speed;
> +		hu->oper_speed = speed;
> +	}
> +
> +	serdev_device_set_drvdata(serdev, h4dev);
> +	dev_info(&serdev->dev, "h4 device registered.\n");

It is considered bad practice to spam the logs like this. dev_dbg().

> +
> +	return hci_uart_register_device(hu, &h4p);
> +}
> +
> +static void h4_remove(struct serdev_device *serdev)
> +{
> +	struct h4_device *h4dev = serdev_device_get_drvdata(serdev);
> +
> +	dev_info(&serdev->dev, "h4 device unregistered.\n");

dev_dbg().

	Andrew
Krzysztof Kozlowski Nov. 8, 2022, 8:58 p.m. UTC | #2 
On 08/11/2022 21:38, Andrew Lunn wrote:
>> +static int h4_probe(struct serdev_device *serdev)
>> +{
>> +	struct h4_device *h4dev;
>> +	struct hci_uart *hu;
>> +	int ret;
>> +	u32 speed;
>> +
>> +	h4dev = devm_kzalloc(&serdev->dev, sizeof(*h4dev), GFP_KERNEL);
>> +	if (!h4dev)
>> +		return -ENOMEM;
>> +
>> +	hu = &h4dev->hu;
>> +
>> +	hu->serdev = serdev;
>> +	ret = device_property_read_u32(&serdev->dev, "max-speed", &speed);
>> +	if (!ret) {
>> +		/* h4 does not have any baudrate handling:
>> +		 * user oper speed from the start
>> +		 */
>> +		hu->init_speed = speed;
>> +		hu->oper_speed = speed;
>> +	}
>> +
>> +	serdev_device_set_drvdata(serdev, h4dev);
>> +	dev_info(&serdev->dev, "h4 device registered.\n");
> 
> It is considered bad practice to spam the logs like this. dev_dbg().
> 
>> +
>> +	return hci_uart_register_device(hu, &h4p);
>> +}
>> +
>> +static void h4_remove(struct serdev_device *serdev)
>> +{
>> +	struct h4_device *h4dev = serdev_device_get_drvdata(serdev);
>> +
>> +	dev_info(&serdev->dev, "h4 device unregistered.\n");
> 
> dev_dbg().

I would say none of them (the same in probe). Any prints in probe/remove
paths are considered redundant, as core already gives that information.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index e30707405455..69edc76a8611 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -113,6 +113,7 @@  config BT_HCIUART_SERDEV
 config BT_HCIUART_H4
 	bool "UART (H4) protocol support"
 	depends on BT_HCIUART
+	depends on BT_HCIUART_SERDEV
 	help
 	  UART (H4) is serial protocol for communication between Bluetooth
 	  device and host. This protocol is required for most Bluetooth devices
diff --git a/drivers/bluetooth/hci_h4.c b/drivers/bluetooth/hci_h4.c
index 1d0cdf023243..b214c8a4d450 100644
--- a/drivers/bluetooth/hci_h4.c
+++ b/drivers/bluetooth/hci_h4.c
@@ -18,6 +18,8 @@ 
 #include <linux/ptrace.h>
 #include <linux/poll.h>
 
+#include <linux/of.h>
+#include <linux/serdev.h>
 #include <linux/slab.h>
 #include <linux/tty.h>
 #include <linux/errno.h>
@@ -32,6 +34,10 @@ 
 
 #include "hci_uart.h"
 
+struct h4_device {
+	struct hci_uart hu;
+};
+
 struct h4_struct {
 	struct sk_buff *rx_skb;
 	struct sk_buff_head txq;
@@ -130,6 +136,63 @@  static struct sk_buff *h4_dequeue(struct hci_uart *hu)
 	return skb_dequeue(&h4->txq);
 }
 
+static const struct hci_uart_proto h4p;
+
+static int h4_probe(struct serdev_device *serdev)
+{
+	struct h4_device *h4dev;
+	struct hci_uart *hu;
+	int ret;
+	u32 speed;
+
+	h4dev = devm_kzalloc(&serdev->dev, sizeof(*h4dev), GFP_KERNEL);
+	if (!h4dev)
+		return -ENOMEM;
+
+	hu = &h4dev->hu;
+
+	hu->serdev = serdev;
+	ret = device_property_read_u32(&serdev->dev, "max-speed", &speed);
+	if (!ret) {
+		/* h4 does not have any baudrate handling:
+		 * user oper speed from the start
+		 */
+		hu->init_speed = speed;
+		hu->oper_speed = speed;
+	}
+
+	serdev_device_set_drvdata(serdev, h4dev);
+	dev_info(&serdev->dev, "h4 device registered.\n");
+
+	return hci_uart_register_device(hu, &h4p);
+}
+
+static void h4_remove(struct serdev_device *serdev)
+{
+	struct h4_device *h4dev = serdev_device_get_drvdata(serdev);
+
+	dev_info(&serdev->dev, "h4 device unregistered.\n");
+
+	hci_uart_unregister_device(&h4dev->hu);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id h4_bluetooth_of_match[] = {
+	{ .compatible = "nxp,aw-xm458-bt" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, h4_bluetooth_of_match);
+#endif
+
+static struct serdev_device_driver h4_serdev_driver = {
+	.probe = h4_probe,
+	.remove = h4_remove,
+	.driver = {
+		.name = "hci_uart_h4",
+		.of_match_table = of_match_ptr(h4_bluetooth_of_match),
+	},
+};
+
 static const struct hci_uart_proto h4p = {
 	.id		= HCI_UART_H4,
 	.name		= "H4",
@@ -143,11 +206,13 @@  static const struct hci_uart_proto h4p = {
 
 int __init h4_init(void)
 {
+	serdev_device_driver_register(&h4_serdev_driver);
 	return hci_uart_register_proto(&h4p);
 }
 
 int __exit h4_deinit(void)
 {
+	serdev_device_driver_unregister(&h4_serdev_driver);
 	return hci_uart_unregister_proto(&h4p);
 }