Commit 4a4ab610 authored by Maximilian Luz's avatar Maximilian Luz Committed by Hans de Goede
Browse files

platform/surface: aggregator: Move device registry helper functions to core module 



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: default avatarMaximilian Luz <luzmaximilian@gmail.com>
Link: https://lore.kernel.org/r/20220624205800.1355621-2-luzmaximilian@gmail.com


Reviewed-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
parent 70e85eb0

drivers/platform/surface/aggregator/bus.c

+133 −16
Original line number Diff line number Diff line
@@ -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);

}

drivers/platform/surface/surface_aggregator_registry.c

+2 −73
Original line number Diff line number Diff line
@@ -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);

include/linux/surface_aggregator/device.h

+52 −0
Original line number Diff line number Diff line
@@ -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. ----------------------------------- */