Further _remove() cleanup for hci_uart drivers

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

[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 | 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);
 }
-- 
2.11.0

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

[Sebastian Reichel]

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

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

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

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

[Ian Molton]

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

[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 | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index c982943f0747..20e90fc1f7be 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -745,10 +745,8 @@ static void hci_ti_remove(struct serdev_device *serdev)
 	struct hci_uart *hu = &lldev->hu;
 	struct hci_dev *hdev = hu->hdev;
 
-	cancel_work_sync(&hu->write_work);
+	hci_uart_unregister_device(hdev);
 
-	hci_unregister_dev(hdev);
-	hci_free_dev(hdev);
 	hu->proto->close(hu);
 }
 
-- 
2.11.0

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