[PATCH v1 0/4] serdev: Stop using device_driver callbacks

[PATCH v1 0/4] serdev: Stop using device_driver callbacks

[Uwe Kleine-König]

Hello,

the serdev subsystem currently doesn't provide a shutdown callback, thus
drivers that want being notified on shutdown have to implement the
respective callback in struct device_driver. This (and more)
functionality can be provided by a bus method as it already done for
.probe() and .remove().

The eventual goal is to remove .shutdown() (and .probe() and .remove())
from struct device_driver.

Note that the first patch introduces a warning when the three affected
drivers are registered (in driver_register() because `drv->bus->shutdown
&& drv->shutdown`). Patches #2 - #4 fix these warnings. So from a user
perspective it would be good to get the whole series in during a single
merge window---either by creating an immutable branch containing patch
#1 that is merged into the respective subsystems before applying the
following patches, or merging the complete series via a single tree.

At a later point in time the added check in
__serdev_device_driver_register() and the function
serdev_legacy_shutdown() can be dropped. I intend to cope for that in
the merge window that removes the callbacks from struct device_driver
because drivers that I might have missed to convert or that are rebased
over that change break silently as long as struct
device_driver::shutdown exists.

Best regards
Uwe

Uwe Kleine-König (4):
  serdev: Provide a bustype shutdown function
  Bluetooth: hci_aml: Migrate to serdev specific shutdown function
  Bluetooth: hci_qca: Migrate to serdev specific shutdown function
  platform/surface: Migrate to serdev specific shutdown function

 drivers/bluetooth/hci_aml.c                | 16 ++++++++--------
 drivers/bluetooth/hci_qca.c                |  5 ++---
 drivers/platform/surface/aggregator/core.c |  6 +++---
 drivers/tty/serdev/core.c                  | 21 +++++++++++++++++++++
 include/linux/serdev.h                     |  1 +
 5 files changed, 35 insertions(+), 14 deletions(-)


base-commit: 7d0a66e4bb9081d75c82ec4957c50034cb0ea449
-- 
2.47.3

[PATCH v1 1/4] serdev: Provide a bustype shutdown function

[Uwe Kleine-König]

To prepare serdev driver to migrate away from struct device_driver::shutdown
(and then eventually remove that callback) create a serdev driver shutdown
callback and migration code to keep the existing behaviour. Note this
introduces a warning for each driver at register time that isn't converted
yet to that callback.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/tty/serdev/core.c | 21 +++++++++++++++++++++
 include/linux/serdev.h    |  1 +
 2 files changed, 22 insertions(+)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index b33e708cb245..40eedc15277c 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -414,11 +414,21 @@ static void serdev_drv_remove(struct device *dev)
 		sdrv->remove(to_serdev_device(dev));
 }
 
+static void serdev_drv_shutdown(struct device *dev)
+{
+	const struct serdev_device_driver *sdrv =
+		to_serdev_device_driver(dev->driver);
+
+	if (dev->driver && sdrv->shutdown)
+		sdrv->shutdown(to_serdev_device(dev));
+}
+
 static const struct bus_type serdev_bus_type = {
 	.name		= "serial",
 	.match		= serdev_device_match,
 	.probe		= serdev_drv_probe,
 	.remove		= serdev_drv_remove,
+	.shutdown	= serdev_drv_shutdown,
 };
 
 /**
@@ -814,6 +824,14 @@ void serdev_controller_remove(struct serdev_controller *ctrl)
 }
 EXPORT_SYMBOL_GPL(serdev_controller_remove);
 
+static void serdev_legacy_shutdown(struct serdev_device *serdev)
+{
+	struct device *dev = &serdev->dev;
+	struct device_driver *driver = dev->driver;
+
+	driver->shutdown(dev);
+}
+
 /**
  * __serdev_device_driver_register() - Register client driver with serdev core
  * @sdrv:	client driver to be associated with client-device.
@@ -830,6 +848,9 @@ int __serdev_device_driver_register(struct serdev_device_driver *sdrv, struct mo
 	/* force drivers to async probe so I/O is possible in probe */
         sdrv->driver.probe_type = PROBE_PREFER_ASYNCHRONOUS;
 
+	if (!sdrv->shutdown && sdrv->driver.shutdown)
+		sdrv->shutdown = serdev_legacy_shutdown;
+
 	return driver_register(&sdrv->driver);
 }
 EXPORT_SYMBOL_GPL(__serdev_device_driver_register);
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 34562eb99931..5654c58eb73c 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -65,6 +65,7 @@ struct serdev_device_driver {
 	struct device_driver driver;
 	int	(*probe)(struct serdev_device *);
 	void	(*remove)(struct serdev_device *);
+	void	(*shutdown)(struct serdev_device *);
 };
 
 static inline struct serdev_device_driver *to_serdev_device_driver(struct device_driver *d)
-- 
2.47.3

RE: serdev: Stop using device_driver callbacks

[bluez.test.bot]

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

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1032563

---Test result---

Test Summary:
CheckPatch                    PENDING   0.40 seconds
GitLint                       PENDING   0.39 seconds
SubjectPrefix                 FAIL      0.51 seconds
BuildKernel                   PASS      25.57 seconds
CheckAllWarning               PASS      28.10 seconds
CheckSparse                   PASS      31.71 seconds
BuildKernel32                 PASS      25.01 seconds
TestRunnerSetup               PASS      555.29 seconds
TestRunner_l2cap-tester       PASS      25.16 seconds
TestRunner_iso-tester         PASS      67.42 seconds
TestRunner_bnep-tester        PASS      6.21 seconds
TestRunner_mgmt-tester        FAIL      116.37 seconds
TestRunner_rfcomm-tester      PASS      9.40 seconds
TestRunner_sco-tester         FAIL      14.80 seconds
TestRunner_ioctl-tester       PASS      10.13 seconds
TestRunner_mesh-tester        FAIL      11.61 seconds
TestRunner_smp-tester         PASS      8.57 seconds
TestRunner_userchan-tester    PASS      7.38 seconds
IncrementalBuild              PENDING   1.01 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 494, Passed: 489 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.105 seconds
##############################
Test: TestRunner_sco-tester - FAIL
Desc: Run sco-tester with test-runner
Output:
WARNING: possible circular locking dependency detected
BUG: sleeping function called from invalid context at net/core/sock.c:3782
Total: 30, Passed: 30 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    1.994 seconds
Mesh - Send cancel - 2                               Timed out    1.996 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

[PATCH v1 2/4] Bluetooth: hci_aml: Migrate to serdev specific shutdown function

[Uwe Kleine-König]

This saves a cast in the driver. The motivation is stop using the callback
.shutdown in qca_serdev_driver.driver to make it possible to drop that.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/bluetooth/hci_aml.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/bluetooth/hci_aml.c b/drivers/bluetooth/hci_aml.c
index b1f32c5a8a3f..4981c82d634d 100644
--- a/drivers/bluetooth/hci_aml.c
+++ b/drivers/bluetooth/hci_aml.c
@@ -677,13 +677,6 @@ static const struct hci_uart_proto aml_hci_proto = {
 	.dequeue	= aml_dequeue,
 };
 
-static void aml_device_driver_shutdown(struct device *dev)
-{
-	struct aml_serdev *amldev = dev_get_drvdata(dev);
-
-	aml_power_off(amldev);
-}
-
 static int aml_serdev_probe(struct serdev_device *serdev)
 {
 	struct aml_serdev *amldev;
@@ -714,6 +707,13 @@ static void aml_serdev_remove(struct serdev_device *serdev)
 	hci_uart_unregister_device(&amldev->serdev_hu);
 }
 
+static void aml_serdev_shutdown(struct serdev_device *serdev)
+{
+	struct aml_serdev *amldev = serdev_device_get_drvdata(serdev);
+
+	aml_power_off(amldev);
+}
+
 static const struct aml_device_data data_w155s2 = {
 	.iccm_offset = 256 * 1024,
 };
@@ -732,10 +732,10 @@ MODULE_DEVICE_TABLE(of, aml_bluetooth_of_match);
 static struct serdev_device_driver aml_serdev_driver = {
 	.probe = aml_serdev_probe,
 	.remove = aml_serdev_remove,
+	.shutdown = aml_serdev_shutdown,
 	.driver = {
 		.name = "hci_uart_aml",
 		.of_match_table = aml_bluetooth_of_match,
-		.shutdown = aml_device_driver_shutdown,
 	},
 };
 
-- 
2.47.3

[PATCH v1 3/4] Bluetooth: hci_qca: Migrate to serdev specific shutdown function

[Uwe Kleine-König]

This saves a cast in the driver. The motivation is stop using the callback
.shutdown in qca_serdev_driver.driver to make it possible to drop that.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
 drivers/bluetooth/hci_qca.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 888176b0faa9..6d54f747fde4 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -2530,11 +2530,10 @@ static void qca_serdev_remove(struct serdev_device *serdev)
 	hci_uart_unregister_device(&qcadev->serdev_hu);
 }
 
-static void qca_serdev_shutdown(struct device *dev)
+static void qca_serdev_shutdown(struct serdev_device *serdev)
 {
 	int ret;
 	int timeout = msecs_to_jiffies(CMD_TRANS_TIMEOUT_MS);
-	struct serdev_device *serdev = to_serdev_device(dev);
 	struct qca_serdev *qcadev = serdev_device_get_drvdata(serdev);
 	struct hci_uart *hu = &qcadev->serdev_hu;
 	struct hci_dev *hdev = hu->hdev;
@@ -2756,11 +2755,11 @@ static void hciqca_coredump(struct device *dev)
 static struct serdev_device_driver qca_serdev_driver = {
 	.probe = qca_serdev_probe,
 	.remove = qca_serdev_remove,
+	.shutdown = qca_serdev_shutdown,
 	.driver = {
 		.name = "hci_uart_qca",
 		.of_match_table = of_match_ptr(qca_bluetooth_of_match),
 		.acpi_match_table = ACPI_PTR(qca_bluetooth_acpi_match),
-		.shutdown = qca_serdev_shutdown,
 		.pm = &qca_pm_ops,
 #ifdef CONFIG_DEV_COREDUMP
 		.coredump = hciqca_coredump,
-- 
2.47.3