Re: [PATCH v2 01/12] drivers: base: Unified device connection lookup

[Randy Dunlap]

On 02/25/2018 07:24 AM, Hans de Goede wrote:
> From: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> 
> Several frameworks - clk, gpio, phy, pmw, etc. - maintain
> lookup tables for describing connections and provide custom
> API for handling them. This introduces a single generic
> lookup table and API for the connections.
> 
> The motivation for this commit is centralizing the
> connection lookup, but the goal is to ultimately extract the
> connection descriptions also from firmware by using the
> fwnode_graph_* functions and other mechanisms that are
> available.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> Changes in v2:
> -Add a (struct devcon) cast to the DEVCON() macro
> ---
>  Documentation/driver-api/device_connection.rst |  43 ++++++++
>  drivers/base/Makefile                          |   3 +-
>  drivers/base/devcon.c                          | 139 +++++++++++++++++++++++++
>  include/linux/connection.h                     |  33 ++++++
>  4 files changed, 217 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/driver-api/device_connection.rst
>  create mode 100644 drivers/base/devcon.c
>  create mode 100644 include/linux/connection.h
> 
> diff --git a/Documentation/driver-api/device_connection.rst b/Documentation/driver-api/device_connection.rst
> new file mode 100644
> index 000000000000..d52604448356
> --- /dev/null
> +++ b/Documentation/driver-api/device_connection.rst
> @@ -0,0 +1,43 @@
> +==================
> +Device connections
> +==================
> +
> +Introduction
> +------------
> +
> +Devices have often connections to other devices that are out side of the direct

   Devices often have connections                       are outside of

> +child/parent relationship. A serial or network communication controller, which
> +could be a PCI device, may need to be able to get a reference to its PHY
> +component, which could be attached to for example the I2C bus. Some device

                    could be attached for example to the I2C bus.

> +drivers need to be able to control the clocks or the GPIOs for their devices,
> +and so on.
> +
> +Device connections are generic descriptions of any type of connection between
> +two separate devices.
> +
> +Device connections alone do not create a dependency between the two devices.
> +They are only descriptions which are not tied to either of devices directly.

                                                           of the devices directly.

> +A dependency between the two devices exists only if one of the two endpoint
> +devices requests a reference to the other. The descriptions themselves can be
> +defined in firmware (not yet supported) or they can be build-in.

                                                          built-in.

> +
> +Usage
> +-----
> +
> +Device connections should exist before device ``->probe`` callback is called for
> +either endpoint devices in the description. If the connections are defined in

                   device

> +firmware, this is not a problem. It should be considered if the connection
> +descriptions are "build-in", and need to be added separately.

                    "built-in",

> +
> +The connection description consist of the names of the two devices with the

                              consists

> +connection, i.e. the endpoints, and unique identifier for the connection which
> +is needed if there are multiple connections between the two devices.
> +
> +After a descriptions exist, the devices in it can request reference to the other

           description exists,

> +endpoint device, or they can request the description itself.
> +
> +API
> +---
> +
> +.. kernel-doc:: drivers/base/devcon.c
> +   : functions: __device_find_connection device_find_connection add_device_connection remove_device_connection

> diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
> new file mode 100644
> index 000000000000..6f9e4f7280a5
> --- /dev/null
> +++ b/drivers/base/devcon.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/**
> + * Device connections
> + *
> + * Copyright (C) 2018 Intel Corporation
> + * Author: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> + */
> +
> +#include <linux/connection.h>
> +#include <linux/device.h>
> +
> +static DEFINE_MUTEX(devcon_lock);
> +static LIST_HEAD(devcon_list);
> +
> +/**
> + * __device_find_connection - Find physical connection to a device
> + * @dev: Device with the connection
> + * @con_id: Identifier for the connection
> + * @data: Data for the match function
> + * @match: Function to check and convert the connection description
> + *
> + * Find a connection with unique identifier @con_id between @dev and an other

                                                                        another

> + * device. @match will be used to convert the connection description to data the
> + * caller is expecting to be returned.
> + */
> +void *__device_find_connection(struct device *dev, const char *con_id,
> +			       void *data,
> +			       void *(*match)(struct devcon *con, int ep,
> +					      void *data))
> +{
> +	const char *devname = dev_name(dev);
> +	struct devcon *con;
> +	void *ret = NULL;
> +	int ep;
> +
> +	if (!match)
> +		return NULL;
> +
> +	rcu_read_lock();
> +
> +	list_for_each_entry_rcu(con, &devcon_list, list) {
> +		ep = match_string(con->endpoint, 2, devname);
> +		if (ep < 0)
> +			continue;
> +
> +		if (con_id && strcmp(con->id, con_id))
> +			continue;
> +
> +		ret = match(con, !ep, data);
> +		if (ret)
> +			break;
> +	}
> +
> +	rcu_read_unlock();
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(__device_find_connection);
> +
> +#include <linux/platform_device.h>
> +#include <linux/spi/spi.h>
> +#include <linux/i2c.h>
> +#include <linux/pci.h>
> +
> +static struct bus_type *generic_match_buses[] = {
> +	&platform_bus_type,
> +#ifdef CONFIG_PCI
> +	&pci_bus_type,
> +#endif
> +#ifdef CONFIG_I2C
> +	&i2c_bus_type,
> +#endif
> +#ifdef CONFIG_SPI_MASTER
> +	&spi_bus_type,
> +#endif
> +	NULL,
> +};
> +
> +/* This tries to find the device from the most common bus types by name. */
> +static void *generic_match(struct devcon *con, int ep, void *data)
> +{
> +	struct bus_type *bus;
> +	struct device *dev;
> +
> +	for (bus = generic_match_buses[0]; bus; bus++) {
> +		dev = bus_find_device_by_name(bus, NULL, con->endpoint[ep]);
> +		if (dev)
> +			return dev;
> +	}
> +
> +	/*
> +	 * We only get called if a connection was found, tell the caller to

	                                          found; tell

> +	 * wait for the other device to show-up.

	                                show up.

> +	 */
> +	return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +/**
> + * device_find_connection - Find two devices connected together
> + * @dev: Device with the connection
> + * @con_id: Identifier for the connection
> + *
> + * Find a connection with unique identifier @con_id between @dev and an

                                                                    and

> + * other device. On success returns handle to the device that is connected

      another

> + * to @dev, with the reference count for the found device incremented. Returns
> + * NULL if no matching connection was found, or ERR_PTR(-EPROBE_DEFER) when a
> + * connection was found but the other device has not been enumerated yet.
> + */
> +struct device *device_find_connection(struct device *dev, const char *con_id)
> +{
> +	return __device_find_connection(dev, con_id, generic_match, NULL);
> +}
> +EXPORT_SYMBOL_GPL(device_find_connection);



-- 
~Randy