PATCH (No really this time) - Fix hci_serdev drivers _remove() path

PATCH (No really this time) - Fix hci_serdev drivers _remove() path

[Ian Molton]

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

[PATCH 1/3] bluetooth: Introduce hci_uart_unregister_device()

[Ian Molton]

Several drivers have the same (and incorrect) code in their
_remove() handler.

Coalesce this into a shared function.

Signed-off-by: Ian Molton <ian@mnementh.co.uk>
---
 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

Re: [PATCH 1/3] bluetooth: Introduce hci_uart_unregister_device()

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1764 bytes --]

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 <ian@mnementh.co.uk>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/3] bluetooth: Introduce hci_uart_unregister_device()

[Marcel Holtmann]

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 <ian@mnementh.co.uk>
> ---
> 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

Re: [PATCH 1/3] bluetooth: Introduce hci_uart_unregister_device()

[Ian Molton]

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 <ian@mnementh.co.uk>
>> ---
>> 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

[PATCH 2/3] bluetooth: Nokia: Use new hci_uart_unregister_device() function.

[Ian Molton]

Simplify _remove() path for hci_nokia.c

Signed-off-by: Ian Molton <ian@mnementh.co.uk>
---
 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

Re: [PATCH 2/3] bluetooth: Nokia: Use new hci_uart_unregister_device() function.

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

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 <ian@mnementh.co.uk>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>

-- 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 3/3] bluetooth: hci_ll: Use new hci_uart_unregister_device() function.

[Ian Molton]

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 <ian@mnementh.co.uk>
---
 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

Re: [PATCH 3/3] bluetooth: hci_ll: Use new hci_uart_unregister_device() function.

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]

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 <ian@mnementh.co.uk>

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabor.co.uk>

-- 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Further _remove() cleanup for hci_uart drivers

[Ian Molton]

Hi Marcel,

Here are three untested patches. They introduce a common function
hci_uart_unregister_device().

This cleans up hci_nokia.c further, and fixes a cut-n-paste error that
introduced the same race condition to hci_ll.c (or perhaps the other
way around).

HTH,

-Ian

[PATCH 1/3] bluetooth: Introduce hci_uart_unregister_device()

[Ian Molton]

Several drivers have the same (and incorrect) code in their
_remove() handler.

Coalesce this into a shared function.


Signed-off-by: Ian Molton <ian@mnementh.co.uk>
---
 drivers/bluetooth/hci_serdev.c | 10 ++++++++++
 drivers/bluetooth/hci_uart.h   |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index aea930101dd2..20e356168156 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -354,3 +354,13 @@ 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);
+}
+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