platform/surface: aggregator: Move device registry helper functions to core module
authorMaximilian Luz <luzmaximilian@gmail.com>
Fri, 24 Jun 2022 20:57:58 +0000 (22:57 +0200)
committerHans de Goede <hdegoede@redhat.com>
Sat, 2 Jul 2022 09:23:14 +0000 (11:23 +0200)
Move helper functions for client device registration to the core module.
This simplifies addition of future DT/OF support and also allows us to
split out the device hub drivers into their own module.

At the same time, also improve device node validation a bit by not
silently skipping devices with invalid device UID specifiers. Further,
ensure proper lifetime management for the firmware/software nodes
associated with the added devices.

Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20220624205800.1355621-2-luzmaximilian@gmail.com
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
drivers/platform/surface/aggregator/bus.c
drivers/platform/surface/surface_aggregator_registry.c
include/linux/surface_aggregator/device.h

index abbbb5b..e0b0381 100644 (file)
@@ -6,6 +6,7 @@
  */
 
 #include <linux/device.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 
 #include <linux/surface_aggregator/controller.h>
@@ -14,6 +15,9 @@
 #include "bus.h"
 #include "controller.h"
 
+
+/* -- Device and bus functions. --------------------------------------------- */
+
 static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
                             char *buf)
 {
@@ -46,6 +50,7 @@ static void ssam_device_release(struct device *dev)
        struct ssam_device *sdev = to_ssam_device(dev);
 
        ssam_controller_put(sdev->ctrl);
+       fwnode_handle_put(sdev->dev.fwnode);
        kfree(sdev);
 }
 
@@ -363,6 +368,134 @@ void ssam_device_driver_unregister(struct ssam_device_driver *sdrv)
 }
 EXPORT_SYMBOL_GPL(ssam_device_driver_unregister);
 
+
+/* -- Bus registration. ----------------------------------------------------- */
+
+/**
+ * ssam_bus_register() - Register and set-up the SSAM client device bus.
+ */
+int ssam_bus_register(void)
+{
+       return bus_register(&ssam_bus_type);
+}
+
+/**
+ * ssam_bus_unregister() - Unregister the SSAM client device bus.
+ */
+void ssam_bus_unregister(void)
+{
+       return bus_unregister(&ssam_bus_type);
+}
+
+
+/* -- Helpers for controller and hub devices. ------------------------------- */
+
+static int ssam_device_uid_from_string(const char *str, struct ssam_device_uid *uid)
+{
+       u8 d, tc, tid, iid, fn;
+       int n;
+
+       n = sscanf(str, "%hhx:%hhx:%hhx:%hhx:%hhx", &d, &tc, &tid, &iid, &fn);
+       if (n != 5)
+               return -EINVAL;
+
+       uid->domain = d;
+       uid->category = tc;
+       uid->target = tid;
+       uid->instance = iid;
+       uid->function = fn;
+
+       return 0;
+}
+
+static int ssam_get_uid_for_node(struct fwnode_handle *node, struct ssam_device_uid *uid)
+{
+       const char *str = fwnode_get_name(node);
+
+       /*
+        * To simplify definitions of firmware nodes, we set the device name
+        * based on the UID of the device, prefixed with "ssam:".
+        */
+       if (strncmp(str, "ssam:", strlen("ssam:")) != 0)
+               return -ENODEV;
+
+       str += strlen("ssam:");
+       return ssam_device_uid_from_string(str, uid);
+}
+
+static int ssam_add_client_device(struct device *parent, struct ssam_controller *ctrl,
+                                 struct fwnode_handle *node)
+{
+       struct ssam_device_uid uid;
+       struct ssam_device *sdev;
+       int status;
+
+       status = ssam_get_uid_for_node(node, &uid);
+       if (status)
+               return status;
+
+       sdev = ssam_device_alloc(ctrl, uid);
+       if (!sdev)
+               return -ENOMEM;
+
+       sdev->dev.parent = parent;
+       sdev->dev.fwnode = fwnode_handle_get(node);
+
+       status = ssam_device_add(sdev);
+       if (status)
+               ssam_device_put(sdev);
+
+       return status;
+}
+
+/**
+ * __ssam_register_clients() - Register client devices defined under the
+ * given firmware node as children of the given device.
+ * @parent: The parent device under which clients should be registered.
+ * @ctrl: The controller with which client should be registered.
+ * @node: The firmware node holding definitions of the devices to be added.
+ *
+ * Register all clients that have been defined as children of the given root
+ * firmware node as children of the given parent device. The respective child
+ * firmware nodes will be associated with the correspondingly created child
+ * devices.
+ *
+ * The given controller will be used to instantiate the new devices. See
+ * ssam_device_add() for details.
+ *
+ * Note that, generally, the use of either ssam_device_register_clients() or
+ * ssam_register_clients() should be preferred as they directly use the
+ * firmware node and/or controller associated with the given device. This
+ * function is only intended for use when different device specifications (e.g.
+ * ACPI and firmware nodes) need to be combined (as is done in the platform hub
+ * of the device registry).
+ *
+ * Return: Returns zero on success, nonzero on failure.
+ */
+int __ssam_register_clients(struct device *parent, struct ssam_controller *ctrl,
+                           struct fwnode_handle *node)
+{
+       struct fwnode_handle *child;
+       int status;
+
+       fwnode_for_each_child_node(node, child) {
+               /*
+                * Try to add the device specified in the firmware node. If
+                * this fails with -ENODEV, the node does not specify any SSAM
+                * device, so ignore it and continue with the next one.
+                */
+               status = ssam_add_client_device(parent, ctrl, child);
+               if (status && status != -ENODEV)
+                       goto err;
+       }
+
+       return 0;
+err:
+       ssam_remove_clients(parent);
+       return status;
+}
+EXPORT_SYMBOL_GPL(__ssam_register_clients);
+
 static int ssam_remove_device(struct device *dev, void *_data)
 {
        struct ssam_device *sdev = to_ssam_device(dev);
@@ -387,19 +520,3 @@ void ssam_remove_clients(struct device *dev)
        device_for_each_child_reverse(dev, NULL, ssam_remove_device);
 }
 EXPORT_SYMBOL_GPL(ssam_remove_clients);
-
-/**
- * ssam_bus_register() - Register and set-up the SSAM client device bus.
- */
-int ssam_bus_register(void)
-{
-       return bus_register(&ssam_bus_type);
-}
-
-/**
- * ssam_bus_unregister() - Unregister the SSAM client device bus.
- */
-void ssam_bus_unregister(void)
-{
-       return bus_unregister(&ssam_bus_type);
-}
index f1c5905..c680792 100644 (file)
@@ -286,76 +286,6 @@ static const struct software_node *ssam_node_group_sp8[] = {
 };
 
 
-/* -- Device registry helper functions. ------------------------------------- */
-
-static int ssam_uid_from_string(const char *str, struct ssam_device_uid *uid)
-{
-       u8 d, tc, tid, iid, fn;
-       int n;
-
-       n = sscanf(str, "ssam:%hhx:%hhx:%hhx:%hhx:%hhx", &d, &tc, &tid, &iid, &fn);
-       if (n != 5)
-               return -EINVAL;
-
-       uid->domain = d;
-       uid->category = tc;
-       uid->target = tid;
-       uid->instance = iid;
-       uid->function = fn;
-
-       return 0;
-}
-
-static int ssam_hub_add_device(struct device *parent, struct ssam_controller *ctrl,
-                              struct fwnode_handle *node)
-{
-       struct ssam_device_uid uid;
-       struct ssam_device *sdev;
-       int status;
-
-       status = ssam_uid_from_string(fwnode_get_name(node), &uid);
-       if (status)
-               return status;
-
-       sdev = ssam_device_alloc(ctrl, uid);
-       if (!sdev)
-               return -ENOMEM;
-
-       sdev->dev.parent = parent;
-       sdev->dev.fwnode = node;
-
-       status = ssam_device_add(sdev);
-       if (status)
-               ssam_device_put(sdev);
-
-       return status;
-}
-
-static int ssam_hub_register_clients(struct device *parent, struct ssam_controller *ctrl,
-                                    struct fwnode_handle *node)
-{
-       struct fwnode_handle *child;
-       int status;
-
-       fwnode_for_each_child_node(node, child) {
-               /*
-                * Try to add the device specified in the firmware node. If
-                * this fails with -EINVAL, the node does not specify any SSAM
-                * device, so ignore it and continue with the next one.
-                */
-
-               status = ssam_hub_add_device(parent, ctrl, child);
-               if (status && status != -EINVAL)
-                       goto err;
-       }
-
-       return 0;
-err:
-       ssam_remove_clients(parent);
-       return status;
-}
-
-
 /* -- SSAM generic subsystem hub driver framework. -------------------------- */
 
 enum ssam_hub_state {
@@ -385,7 +315,6 @@ struct ssam_hub {
 static void ssam_hub_update_workfn(struct work_struct *work)
 {
        struct ssam_hub *hub = container_of(work, struct ssam_hub, update_work.work);
-       struct fwnode_handle *node = dev_fwnode(&hub->sdev->dev);
        enum ssam_hub_state state;
        int status = 0;
 
@@ -425,7 +354,7 @@ static void ssam_hub_update_workfn(struct work_struct *work)
        hub->state = state;
 
        if (hub->state == SSAM_HUB_CONNECTED)
-               status = ssam_hub_register_clients(&hub->sdev->dev, hub->sdev->ctrl, node);
+               status = ssam_device_register_clients(hub->sdev);
        else
                ssam_remove_clients(&hub->sdev->dev);
 
@@ -769,7 +698,7 @@ static int ssam_platform_hub_probe(struct platform_device *pdev)
 
        set_secondary_fwnode(&pdev->dev, root);
 
-       status = ssam_hub_register_clients(&pdev->dev, ctrl, root);
+       status = __ssam_register_clients(&pdev->dev, ctrl, root);
        if (status) {
                set_secondary_fwnode(&pdev->dev, NULL);
                software_node_unregister_node_group(nodes);
index 6cf7e80..46c45d1 100644 (file)
@@ -15,6 +15,7 @@
 
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
+#include <linux/property.h>
 #include <linux/types.h>
 
 #include <linux/surface_aggregator/controller.h>
@@ -375,11 +376,62 @@ void ssam_device_driver_unregister(struct ssam_device_driver *d);
 /* -- Helpers for controller and hub devices. ------------------------------- */
 
 #ifdef CONFIG_SURFACE_AGGREGATOR_BUS
+
+int __ssam_register_clients(struct device *parent, struct ssam_controller *ctrl,
+                           struct fwnode_handle *node);
 void ssam_remove_clients(struct device *dev);
+
 #else /* CONFIG_SURFACE_AGGREGATOR_BUS */
+
+static inline int __ssam_register_clients(struct device *parent, struct ssam_controller *ctrl,
+                                         struct fwnode_handle *node)
+{
+       return 0;
+}
+
 static inline void ssam_remove_clients(struct device *dev) {}
+
 #endif /* CONFIG_SURFACE_AGGREGATOR_BUS */
 
+/**
+ * ssam_register_clients() - Register all client devices defined under the
+ * given parent device.
+ * @dev: The parent device under which clients should be registered.
+ * @ctrl: The controller with which client should be registered.
+ *
+ * Register all clients that have via firmware nodes been defined as children
+ * of the given (parent) device. The respective child firmware nodes will be
+ * associated with the correspondingly created child devices.
+ *
+ * The given controller will be used to instantiate the new devices. See
+ * ssam_device_add() for details.
+ *
+ * Return: Returns zero on success, nonzero on failure.
+ */
+static inline int ssam_register_clients(struct device *dev, struct ssam_controller *ctrl)
+{
+       return __ssam_register_clients(dev, ctrl, dev_fwnode(dev));
+}
+
+/**
+ * ssam_device_register_clients() - Register all client devices defined under
+ * the given SSAM parent device.
+ * @sdev: The parent device under which clients should be registered.
+ *
+ * Register all clients that have via firmware nodes been defined as children
+ * of the given (parent) device. The respective child firmware nodes will be
+ * associated with the correspondingly created child devices.
+ *
+ * The controller used by the parent device will be used to instantiate the new
+ * devices. See ssam_device_add() for details.
+ *
+ * Return: Returns zero on success, nonzero on failure.
+ */
+static inline int ssam_device_register_clients(struct ssam_device *sdev)
+{
+       return ssam_register_clients(&sdev->dev, sdev->ctrl);
+}
+
 
 /* -- Helpers for client-device requests. ----------------------------------- */