Hi,
Sorry for all that noise folks - I guess having a stray > character create a
file called --compose is a bit of a bummer :)
This patch series should cover what has been discussed thus far, please apply!
-Ian
On 08/07/17 20:59, Marcel Holtmann wrote:
> Hi Ian,
>
>> Several drivers have the same (and incorrect) code in their
>> _remove() handler.
>>
>> Coalesce this into a shared function.
>>
>> Signed-off-by: Ian Molton <[email protected]>
>> ---
>> drivers/bluetooth/hci_serdev.c | 12 ++++++++++++
>> drivers/bluetooth/hci_uart.h | 1 +
>> 2 files changed, 13 insertions(+)
>
> all 3 patches have been applied to bluetooth-next tree.
Thanks Marcel and Sebastian, for the review / apply.
-Ian
Hi Ian,
> Several drivers have the same (and incorrect) code in their
> _remove() handler.
>
> Coalesce this into a shared function.
>
> Signed-off-by: Ian Molton <[email protected]>
> ---
> drivers/bluetooth/hci_serdev.c | 12 ++++++++++++
> drivers/bluetooth/hci_uart.h | 1 +
> 2 files changed, 13 insertions(+)
all 3 patches have been applied to bluetooth-next tree.
Regards
Marcel
Hi,
On Sat, Jul 08, 2017 at 05:37:43PM +0100, Ian Molton wrote:
> Convert hci_ll to use hci_uart_unregister_device().
>
> This simplifies the _remove() handler as well as fixes a
> potential race condition on unload.
>
> Signed-off-by: Ian Molton <[email protected]>
Reviewed-by: Sebastian Reichel <[email protected]>
-- Sebastian
> ---
> drivers/bluetooth/hci_ll.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
> index c982943f0747..1b898445a0b8 100644
> --- a/drivers/bluetooth/hci_ll.c
> +++ b/drivers/bluetooth/hci_ll.c
> @@ -742,14 +742,8 @@ static int hci_ti_probe(struct serdev_device *serdev)
> static void hci_ti_remove(struct serdev_device *serdev)
> {
> struct ll_device *lldev = serdev_device_get_drvdata(serdev);
> - struct hci_uart *hu = &lldev->hu;
> - struct hci_dev *hdev = hu->hdev;
>
> - cancel_work_sync(&hu->write_work);
> -
> - hci_unregister_dev(hdev);
> - hci_free_dev(hdev);
> - hu->proto->close(hu);
> + hci_uart_unregister_device(&lldev->hu);
> }
>
> static const struct of_device_id hci_ti_of_match[] = {
> --
> 2.11.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
On Sat, Jul 08, 2017 at 05:37:42PM +0100, Ian Molton wrote:
> Simplify _remove() path for hci_nokia.c
>
> Signed-off-by: Ian Molton <[email protected]>
> ---
Reviewed-by: Sebastian Reichel <[email protected]>
-- Sebastian
> drivers/bluetooth/hci_nokia.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
> index 6dbb1f6ff6bd..3539fd03f47e 100644
> --- a/drivers/bluetooth/hci_nokia.c
> +++ b/drivers/bluetooth/hci_nokia.c
> @@ -767,16 +767,8 @@ static int nokia_bluetooth_serdev_probe(struct serdev_device *serdev)
> static void nokia_bluetooth_serdev_remove(struct serdev_device *serdev)
> {
> struct nokia_bt_dev *btdev = serdev_device_get_drvdata(serdev);
> - struct hci_uart *hu = &btdev->hu;
> - struct hci_dev *hdev = hu->hdev;
>
> -
> - hci_unregister_dev(hdev);
> - hci_free_dev(hdev);
> -
> - cancel_work_sync(&hu->write_work);
> -
> - hu->proto->close(hu);
> + hci_uart_unregister_device(&btdev->hu);
> }
>
> static int nokia_bluetooth_runtime_suspend(struct device *dev)
> --
> 2.11.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
On Sat, Jul 08, 2017 at 05:37:41PM +0100, Ian Molton wrote:
> Several drivers have the same (and incorrect) code in their
> _remove() handler.
>
> Coalesce this into a shared function.
>
> Signed-off-by: Ian Molton <[email protected]>
> ---
Reviewed-by: Sebastian Reichel <[email protected]>
-- Sebastian
> drivers/bluetooth/hci_serdev.c | 12 ++++++++++++
> drivers/bluetooth/hci_uart.h | 1 +
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
> index aea930101dd2..abc70b5e5647 100644
> --- a/drivers/bluetooth/hci_serdev.c
> +++ b/drivers/bluetooth/hci_serdev.c
> @@ -354,3 +354,15 @@ int hci_uart_register_device(struct hci_uart *hu,
> return err;
> }
> EXPORT_SYMBOL_GPL(hci_uart_register_device);
> +
> +void hci_uart_unregister_device(struct hci_uart *hu) {
> + struct hci_dev *hdev = hu->hdev;
> +
> + hci_unregister_dev(hdev);
> + hci_free_dev(hdev);
> +
> + cancel_work_sync(&hu->write_work);
> +
> + hu->proto->close(hu);
> +}
> +EXPORT_SYMBOL_GPL(hci_uart_unregister_device);
> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
> index c6e9e1cf63f8..d9cd95d81149 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -112,6 +112,7 @@ struct hci_uart {
> int hci_uart_register_proto(const struct hci_uart_proto *p);
> int hci_uart_unregister_proto(const struct hci_uart_proto *p);
> int hci_uart_register_device(struct hci_uart *hu, const struct hci_uart_proto *p);
> +void hci_uart_unregister_device(struct hci_uart *hu);
>
> int hci_uart_tx_wakeup(struct hci_uart *hu);
> int hci_uart_init_ready(struct hci_uart *hu);
> --
> 2.11.0
Convert hci_ll to use hci_uart_unregister_device().
This simplifies the _remove() handler as well as fixes a
potential race condition on unload.
Signed-off-by: Ian Molton <[email protected]>
---
drivers/bluetooth/hci_ll.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index c982943f0747..1b898445a0b8 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -742,14 +742,8 @@ static int hci_ti_probe(struct serdev_device *serdev)
static void hci_ti_remove(struct serdev_device *serdev)
{
struct ll_device *lldev = serdev_device_get_drvdata(serdev);
- struct hci_uart *hu = &lldev->hu;
- struct hci_dev *hdev = hu->hdev;
- cancel_work_sync(&hu->write_work);
-
- hci_unregister_dev(hdev);
- hci_free_dev(hdev);
- hu->proto->close(hu);
+ hci_uart_unregister_device(&lldev->hu);
}
static const struct of_device_id hci_ti_of_match[] = {
--
2.11.0
Simplify _remove() path for hci_nokia.c
Signed-off-by: Ian Molton <[email protected]>
---
drivers/bluetooth/hci_nokia.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
index 6dbb1f6ff6bd..3539fd03f47e 100644
--- a/drivers/bluetooth/hci_nokia.c
+++ b/drivers/bluetooth/hci_nokia.c
@@ -767,16 +767,8 @@ static int nokia_bluetooth_serdev_probe(struct serdev_device *serdev)
static void nokia_bluetooth_serdev_remove(struct serdev_device *serdev)
{
struct nokia_bt_dev *btdev = serdev_device_get_drvdata(serdev);
- struct hci_uart *hu = &btdev->hu;
- struct hci_dev *hdev = hu->hdev;
-
- hci_unregister_dev(hdev);
- hci_free_dev(hdev);
-
- cancel_work_sync(&hu->write_work);
-
- hu->proto->close(hu);
+ hci_uart_unregister_device(&btdev->hu);
}
static int nokia_bluetooth_runtime_suspend(struct device *dev)
--
2.11.0
Several drivers have the same (and incorrect) code in their
_remove() handler.
Coalesce this into a shared function.
Signed-off-by: Ian Molton <[email protected]>
---
drivers/bluetooth/hci_serdev.c | 12 ++++++++++++
drivers/bluetooth/hci_uart.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index aea930101dd2..abc70b5e5647 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -354,3 +354,15 @@ int hci_uart_register_device(struct hci_uart *hu,
return err;
}
EXPORT_SYMBOL_GPL(hci_uart_register_device);
+
+void hci_uart_unregister_device(struct hci_uart *hu) {
+ struct hci_dev *hdev = hu->hdev;
+
+ hci_unregister_dev(hdev);
+ hci_free_dev(hdev);
+
+ cancel_work_sync(&hu->write_work);
+
+ hu->proto->close(hu);
+}
+EXPORT_SYMBOL_GPL(hci_uart_unregister_device);
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index c6e9e1cf63f8..d9cd95d81149 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -112,6 +112,7 @@ struct hci_uart {
int hci_uart_register_proto(const struct hci_uart_proto *p);
int hci_uart_unregister_proto(const struct hci_uart_proto *p);
int hci_uart_register_device(struct hci_uart *hu, const struct hci_uart_proto *p);
+void hci_uart_unregister_device(struct hci_uart *hu);
int hci_uart_tx_wakeup(struct hci_uart *hu);
int hci_uart_init_ready(struct hci_uart *hu);
--
2.11.0
On 08/07/17 15:52, Sebastian Reichel wrote:
> I think "hu->proto->close(hu)" should also be moved into the
> helper function. Also I suggest to use hci_uart instead of hci_dev
> as parameter for hci_uart_unregister_device, which matches the
> related hci_uart_register_device.
Ha, it is. Thats what I get for posting an untested patch for review :)
Will fix, and move the ->close() call into the helper.
Thanks for the review, patch to follow.
-Ian
Hi,
On Sat, Jul 08, 2017 at 03:29:48PM +0100, Ian Molton wrote:
> Simplify _remove() path for hci_nokia.c
>
> Signed-off-by: Ian Molton <[email protected]>
> ---
> drivers/bluetooth/hci_nokia.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
> index 6dbb1f6ff6bd..1a465dd2970e 100644
> --- a/drivers/bluetooth/hci_nokia.c
> +++ b/drivers/bluetooth/hci_nokia.c
> @@ -770,11 +770,7 @@ static void nokia_bluetooth_serdev_remove(struct serdev_device *serdev)
> struct hci_uart *hu = &btdev->hu;
> struct hci_dev *hdev = hu->hdev;
>
> -
> - hci_unregister_dev(hdev);
> - hci_free_dev(hdev);
> -
> - cancel_work_sync(&hu->write_work);
> + hci_uart_unregister_device(hdev);
>
> hu->proto->close(hu);
> }
I think "hu->proto->close(hu)" should also be moved into the
helper function. Also I suggest to use hci_uart instead of hci_dev
as parameter for hci_uart_unregister_device, which matches the
related hci_uart_register_device.
-- Sebastian