2018-06-11 11:52:27

by Ricardo Ribalda Delgado

Subject: [PATCH v2 11/24] Bluetooth: hci_bcm: MODULE_DEVICE_TABLE(serdev)

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.1

2018-06-11 13:31:08

by Andy Shevchenko

Subject: Re: [PATCH v2 11/24] Bluetooth: hci_bcm: MODULE_DEVICE_TABLE(serdev)

On Mon, Jun 11, 2018 at 3:59 PM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Ricardo,
>
>> Export serdev table to the module header, allowing module autoload via
>> udev/modprobe.

>> static const struct serdev_device_id bcm_serdev_id[] =3D {
>> { "bcm43438-bt", },
>> + { "hci_uart_bcm", },
>> {}
>> };
>> +MODULE_DEVICE_TABLE(serdev, bcm_serdev_id);
>
> so this one I can see real use of and is a good fix to finally clean up h=
ci_bcm.c and remove platform support for the Edison hardware. However, I wo=
uld really then first rename hci_uart_bcm into some Edison specific string =
since this is really just one outlier here.

Or other way around, hack
arch/x86/platform/intel-mid/device_libs/platform_bt.c to be compatible
with these changes (Dunno if it's possible).

--=20
With Best Regards,
Andy Shevchenko

2018-06-11 12:59:19

by Marcel Holtmann

Subject: Re: [PATCH v2 11/24] Bluetooth: hci_bcm: MODULE_DEVICE_TABLE(serdev)

Hi Ricardo,

> 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);

so this one I can see real use of and is a good fix to finally clean up hci_bcm.c and remove platform support for the Edison hardware. However, I would really then first rename hci_uart_bcm into some Edison specific string since this is really just one outlier here. Everything else will have ACPI or DT support.

With that said, I do not understand why we need to duplicate the DT compatible strings in serdev_device_id. They will be enumerated fine via ACPI or DT in the first place. So if we want some new_id support, then wouldn’t an empty serdev_device_id table just be fine?

Regards

Marcel