On Wed, Feb 12, 2025 at 04:46:29PM +0100, Alexis Lothoré wrote:
> +#include "linux/bitops.h"
> +#include "linux/byteorder/generic.h"
> +#include "linux/err.h"
> +#include "linux/gfp_types.h"
> +#include "net/bluetooth/bluetooth.h"
> +#include "net/bluetooth/hci.h"
Keep some order here. Why some are <> some "", why net is mixed with
linux...
> +#include <linux/module.h>
> +#include <linux/firmware.h>
> +#include <linux/of.h>
> +#include <linux/serdev.h>
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +#include <net/wilc.h>
> +
> +#include "hci_uart.h"
> +
...
> +static const struct hci_uart_proto wilc_bt_proto = {
> + .id = HCI_UART_WILC,
> + .name = "Microchip",
> + .manufacturer = WILC_BT_UART_MANUFACTURER,
> + .init_speed = WILC_UART_DEFAULT_BAUDRATE,
> + .open = wilc_open,
> + .close = wilc_close,
> + .flush = wilc_flush,
> + .recv = wilc_recv,
> + .enqueue = wilc_enqueue,
> + .dequeue = wilc_dequeue,
> + .setup = wilc_setup,
> + .set_baudrate = wilc_set_baudrate,
> +};
> +
> +static int wilc_bt_serdev_probe(struct serdev_device *serdev)
> +{
> + struct wilc_adapter *wilc_adapter;
> + struct device_node *wlan_node;
> + void *wlan = NULL;
> + int ret;
> +
> + wilc_adapter = kzalloc(sizeof(*wilc_adapter), GFP_KERNEL);
Why not devm?
> + if (!wilc_adapter)
> + return -ENOMEM;
> +
> + wlan_node = of_parse_phandle(serdev->dev.of_node, "wlan", 0);
> + if (!wlan_node) {
> + BT_ERR("Can not run wilc bluetooth without wlan node");
> + ret = -EINVAL;
> + goto exit_free_adapter;
> + }
> +
> +#if IS_ENABLED(CONFIG_WILC1000_SDIO)
> + wlan = wilc_sdio_get_byphandle(wlan_node);
> +#endif
> +#if IS_ENABLED(CONFIG_WILC1000_SPI)
> + if (!wlan || wlan == ERR_PTR(-EPROBE_DEFER))
> + wlan = wilc_spi_get_byphandle(wlan_node);
> +#endif
> + if (IS_ERR(wlan)) {
> + pr_warn("Can not initialize bluetooth: %pe\n", wlan);
dev_warn or even dev_err_probe to handle deferral.
> + ret = PTR_ERR(wlan);
> + goto exit_put_wlan_node;
> + }
> +
> + of_node_put(wlan_node);
> + wilc_adapter->wlan_priv = wlan;
> + ret = wilc_bt_init(wlan);
> + if (ret) {
> + pr_err("Failed to initialize bluetooth firmware (%d)\n", ret);
dev_err_probe
> + goto exit_put_wlan;
> + }
> +
> + wilc_adapter->dev = &serdev->dev;
> + wilc_adapter->hu.serdev = serdev;
> + wilc_adapter->flow_control = false;
> + serdev_device_set_drvdata(serdev, wilc_adapter);
> + ret = hci_uart_register_device(&wilc_adapter->hu, &wilc_bt_proto);
> + if (ret) {
> + dev_err(&serdev->dev, "Failed to register hci device");
> + goto exit_deinit_bt;
> + }
> +
> + dev_info(&serdev->dev, "WILC hci interface registered");
Drop simple probe statuses. sysfs already provides this.
> + return 0;
> +
> +exit_deinit_bt:
> + wilc_bt_shutdown(wlan);
> +exit_put_wlan:
> + wilc_put(wlan);
> +exit_put_wlan_node:
> + of_node_put(wlan_node);
> +exit_free_adapter:
> + kfree(wilc_adapter);
> + return ret;
> +}
> +
> +static void wilc_bt_serdev_remove(struct serdev_device *serdev)
> +{
> + struct wilc_adapter *wilc_adapter = serdev_device_get_drvdata(serdev);
> +
> + hci_uart_unregister_device(&wilc_adapter->hu);
> + wilc_bt_shutdown(wilc_adapter->wlan_priv);
> + wilc_put(wilc_adapter->wlan_priv);
> + kfree(wilc_adapter);
> +}
> +
> +static const struct of_device_id wilc_bt_of_match[] = {
> + { .compatible = "microchip,wilc3000-bt" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, wilc_bt_of_match);
> +
> +static struct serdev_device_driver wilc_bt_serdev_driver = {
> + .probe = wilc_bt_serdev_probe,
> + .remove = wilc_bt_serdev_remove,
> + .driver = {
> + .name = "hci_uart_wilc",
> + .of_match_table = of_match_ptr(wilc_bt_of_match),
Drop of_match_tr, you have warnings here.
Best regards,
Krzysztof
