Hello all
v4:
https://lore.kernel.org/linux-media/[email protected]/T/#m11b7cb977e1b73fba1e625c3d6a189e2943a7783
v3:
https://lore.kernel.org/linux-media/[email protected]/T/#m37b831bb2b406917d6db5da9acf9ed35df65d72d
v2:
https://lore.kernel.org/linux-media/[email protected]/T/#md93fd090009b42a6a98aed892aff0d38cf07e0cd
v1:
https://lore.kernel.org/linux-media/[email protected]/T/#m91934e12e3d033da2e768e952ea3b4a125ee3e67
This series is to start adding support for webcams on laptops with ACPI tables
designed for use with CIO2 on Windows. This series extends the ipu3-cio2
driver to allow for patching the firmware via software_nodes if endpoints
aren't defined by ACPI.
I'm hopeful that most or all of this series could get picked up for 5.12.
We touch a few different areas (listed below), but I think the easiest
approach would be to merge everything through media tree. Rafael, Greg,
Mauro and Sergey; are you ok with that plan, or would you prefer a
different approach? Mauro; if that plan is ok (and of course assuming that
the rest of the patches are acked by their respective maintainers) could
we get a dedicated feature branch just in case the following series ends
up being ready in time too?
lib
lib/test_printf.c: Use helper function to unwind array of
software_nodes
base
software_node: Fix refcounts in software_node_get_next_child()
property: Return true in fwnode_device_is_available for NULL ops
property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary
software_node: Enforce parent before child ordering of nodes arrays
software_node: unregister software_nodes in reverse order
include: fwnode.h: Define format macros for ports and endpoints
acpi
acpi: Add acpi_dev_get_next_match_dev() and helper macro
media
media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
match_fwnode()
ipu3-cio2: Add T: entry to MAINTAINERS
ipu3-cio2: Rename ipu3-cio2.c
ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver
include: media: v4l2-fwnode: Include v4l2_fwnode_bus_type
Series-level changelog:
- Rebased onto 5.11-rc1
Thanks
Dan
Andy Shevchenko (1):
media: ipu3-cio2: Add headers that ipu3-cio2.h is direct user of
Daniel Scally (13):
software_node: Fix refcounts in software_node_get_next_child()
device property: Return true in fwnode_device_is_available for NULL
ops
device property: Call fwnode_graph_get_endpoint_by_id() for
fwnode->secondary
software_node: Enforce parent before child ordering of nodes arrays
software_node: unregister software_nodes in reverse order
device property: Define format macros for ports and endpoints
lib/test_printf.c: Use helper function to unwind array of
software_nodes
ipu3-cio2: Add T: entry to MAINTAINERS
ipu3-cio2: Rename ipu3-cio2.c
media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
match_fwnode()
ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro
media: v4l2-fwnode: Include v4l2_fwnode_bus_type
ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver
Heikki Krogerus (1):
software_node: Add support for fwnode_graph*() family of functions
MAINTAINERS | 2 +
drivers/acpi/utils.c | 30 +-
drivers/base/property.c | 15 +-
drivers/base/swnode.c | 180 ++++++++--
drivers/media/pci/intel/ipu3/Kconfig | 18 +
drivers/media/pci/intel/ipu3/Makefile | 3 +
drivers/media/pci/intel/ipu3/cio2-bridge.c | 311 ++++++++++++++++++
drivers/media/pci/intel/ipu3/cio2-bridge.h | 125 +++++++
.../ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} | 34 ++
drivers/media/pci/intel/ipu3/ipu3-cio2.h | 24 ++
drivers/media/v4l2-core/v4l2-async.c | 8 +
drivers/media/v4l2-core/v4l2-fwnode.c | 11 -
include/acpi/acpi_bus.h | 7 +
include/linux/fwnode.h | 7 +
include/media/v4l2-fwnode.h | 22 ++
lib/test_printf.c | 4 +-
16 files changed, 763 insertions(+), 38 deletions(-)
create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.c
create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.h
rename drivers/media/pci/intel/ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} (98%)
--
2.25.1
Currently on platforms designed for Windows, connections between CIO2 and
sensors are not properly defined in DSDT. This patch extends the ipu3-cio2
driver to compensate by building software_node connections, parsing the
connection properties from the sensor's SSDB buffer.
Suggested-by: Jordan Hand <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Reviewed-by: Kieran Bingham <[email protected]>
Signed-off-by: Daniel Scally <[email protected]>
---
MAINTAINERS | 1 +
drivers/media/pci/intel/ipu3/Kconfig | 18 +
drivers/media/pci/intel/ipu3/Makefile | 1 +
drivers/media/pci/intel/ipu3/cio2-bridge.c | 311 ++++++++++++++++++
drivers/media/pci/intel/ipu3/cio2-bridge.h | 125 +++++++
drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 34 ++
drivers/media/pci/intel/ipu3/ipu3-cio2.h | 6 +
7 files changed, 496 insertions(+)
create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.c
create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 92228e8dd868..a091b496fdd8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9014,6 +9014,7 @@ INTEL IPU3 CSI-2 CIO2 DRIVER
M: Yong Zhi <[email protected]>
M: Sakari Ailus <[email protected]>
M: Bingbu Cao <[email protected]>
+M: Dan Scally <[email protected]>
R: Tianshu Qiu <[email protected]>
L: [email protected]
S: Maintained
diff --git a/drivers/media/pci/intel/ipu3/Kconfig b/drivers/media/pci/intel/ipu3/Kconfig
index 82d7f17e6a02..96a2231b16ad 100644
--- a/drivers/media/pci/intel/ipu3/Kconfig
+++ b/drivers/media/pci/intel/ipu3/Kconfig
@@ -16,3 +16,21 @@ config VIDEO_IPU3_CIO2
Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
connected camera.
The module will be called ipu3-cio2.
+
+config CIO2_BRIDGE
+ bool "IPU3 CIO2 Sensors Bridge"
+ depends on VIDEO_IPU3_CIO2
+ help
+ This extension provides an API for the ipu3-cio2 driver to create
+ connections to cameras that are hidden in the SSDB buffer in ACPI.
+ It can be used to enable support for cameras in detachable / hybrid
+ devices that ship with Windows.
+
+ Say Y here if your device is a detachable / hybrid laptop that comes
+ with Windows installed by the OEM, for example:
+
+ - Microsoft Surface models (except Surface Pro 3)
+ - The Lenovo Miix line (for example the 510, 520, 710 and 720)
+ - Dell 7285
+
+ If in doubt, say N here.
diff --git a/drivers/media/pci/intel/ipu3/Makefile b/drivers/media/pci/intel/ipu3/Makefile
index 429d516452e4..933777e6ea8a 100644
--- a/drivers/media/pci/intel/ipu3/Makefile
+++ b/drivers/media/pci/intel/ipu3/Makefile
@@ -2,3 +2,4 @@
obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
ipu3-cio2-y += ipu3-cio2-main.o
+ipu3-cio2-$(CONFIG_CIO2_BRIDGE) += cio2-bridge.o
diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.c b/drivers/media/pci/intel/ipu3/cio2-bridge.c
new file mode 100644
index 000000000000..143f3c0f445e
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/cio2-bridge.c
@@ -0,0 +1,311 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Author: Dan Scally <[email protected]> */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/pci.h>
+#include <linux/property.h>
+#include <media/v4l2-fwnode.h>
+
+#include "cio2-bridge.h"
+
+/*
+ * Extend this array with ACPI Hardware IDs of devices known to be working
+ * plus the number of link-frequencies expected by their drivers, along with
+ * the frequency values in hertz. This is somewhat opportunistic way of adding
+ * support for this for now in the hopes of a better source for the information
+ * (possibly some encoded value in the SSDB buffer that we're unaware of)
+ * becoming apparent in the future.
+ *
+ * Do not add an entry for a sensor that is not actually supported.
+ */
+static const struct cio2_sensor_config cio2_supported_sensors[] = {
+ /* Omnivision OV5693 */
+ CIO2_SENSOR_CONFIG("INT33BE", 0),
+ /* Omnivision OV2680 */
+ CIO2_SENSOR_CONFIG("OVTI2680", 0),
+};
+
+static const struct cio2_property_names prop_names = {
+ .clock_frequency = "clock-frequency",
+ .rotation = "rotation",
+ .bus_type = "bus-type",
+ .data_lanes = "data-lanes",
+ .remote_endpoint = "remote-endpoint",
+ .link_frequencies = "link-frequencies",
+};
+
+static int cio2_bridge_read_acpi_buffer(struct acpi_device *adev, char *id,
+ void *data, u32 size)
+{
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+ union acpi_object *obj;
+ acpi_status status;
+ int ret = 0;
+
+ status = acpi_evaluate_object(adev->handle, id, NULL, &buffer);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ obj = buffer.pointer;
+ if (!obj) {
+ dev_err(&adev->dev, "Couldn't locate ACPI buffer\n");
+ return -ENODEV;
+ }
+
+ if (obj->type != ACPI_TYPE_BUFFER) {
+ dev_err(&adev->dev, "Not an ACPI buffer\n");
+ ret = -ENODEV;
+ goto out_free_buff;
+ }
+
+ if (obj->buffer.length > size) {
+ dev_err(&adev->dev, "Given buffer is too small\n");
+ ret = -EINVAL;
+ goto out_free_buff;
+ }
+
+ memcpy(data, obj->buffer.pointer, obj->buffer.length);
+
+out_free_buff:
+ kfree(buffer.pointer);
+ return ret;
+}
+
+static void cio2_bridge_create_fwnode_properties(
+ struct cio2_sensor *sensor,
+ struct cio2_bridge *bridge,
+ const struct cio2_sensor_config *cfg)
+{
+ sensor->prop_names = prop_names;
+
+ sensor->local_ref[0].node = &sensor->swnodes[SWNODE_CIO2_ENDPOINT];
+ sensor->remote_ref[0].node = &sensor->swnodes[SWNODE_SENSOR_ENDPOINT];
+
+ sensor->dev_properties[0] = PROPERTY_ENTRY_U32(
+ sensor->prop_names.clock_frequency,
+ sensor->ssdb.mclkspeed);
+ sensor->dev_properties[1] = PROPERTY_ENTRY_U8(
+ sensor->prop_names.rotation,
+ sensor->ssdb.degree);
+
+ sensor->ep_properties[0] = PROPERTY_ENTRY_U32(
+ sensor->prop_names.bus_type,
+ V4L2_FWNODE_BUS_TYPE_CSI2_DPHY);
+ sensor->ep_properties[1] = PROPERTY_ENTRY_U32_ARRAY_LEN(
+ sensor->prop_names.data_lanes,
+ bridge->data_lanes,
+ sensor->ssdb.lanes);
+ sensor->ep_properties[2] = PROPERTY_ENTRY_REF_ARRAY(
+ sensor->prop_names.remote_endpoint,
+ sensor->local_ref);
+
+ if (cfg->nr_link_freqs > 0)
+ sensor->ep_properties[3] = PROPERTY_ENTRY_U64_ARRAY_LEN(
+ sensor->prop_names.link_frequencies,
+ cfg->link_freqs,
+ cfg->nr_link_freqs);
+
+ sensor->cio2_properties[0] = PROPERTY_ENTRY_U32_ARRAY_LEN(
+ sensor->prop_names.data_lanes,
+ bridge->data_lanes,
+ sensor->ssdb.lanes);
+ sensor->cio2_properties[1] = PROPERTY_ENTRY_REF_ARRAY(
+ sensor->prop_names.remote_endpoint,
+ sensor->remote_ref);
+}
+
+static void cio2_bridge_init_swnode_names(struct cio2_sensor *sensor)
+{
+ snprintf(sensor->node_names.remote_port,
+ sizeof(sensor->node_names.remote_port),
+ SWNODE_GRAPH_PORT_NAME_FMT, sensor->ssdb.link);
+ snprintf(sensor->node_names.port,
+ sizeof(sensor->node_names.port),
+ SWNODE_GRAPH_PORT_NAME_FMT, 0); /* Always port 0 */
+ snprintf(sensor->node_names.endpoint,
+ sizeof(sensor->node_names.endpoint),
+ SWNODE_GRAPH_ENDPOINT_NAME_FMT, 0); /* And endpoint 0 */
+}
+
+static void cio2_bridge_create_connection_swnodes(struct cio2_bridge *bridge,
+ struct cio2_sensor *sensor)
+{
+ struct software_node *nodes = sensor->swnodes;
+
+ cio2_bridge_init_swnode_names(sensor);
+
+ nodes[SWNODE_SENSOR_HID] = NODE_SENSOR(sensor->name,
+ sensor->dev_properties);
+ nodes[SWNODE_SENSOR_PORT] = NODE_PORT(sensor->node_names.port,
+ &nodes[SWNODE_SENSOR_HID]);
+ nodes[SWNODE_SENSOR_ENDPOINT] = NODE_ENDPOINT(
+ sensor->node_names.endpoint,
+ &nodes[SWNODE_SENSOR_PORT],
+ sensor->ep_properties);
+ nodes[SWNODE_CIO2_PORT] = NODE_PORT(sensor->node_names.remote_port,
+ &bridge->cio2_hid_node);
+ nodes[SWNODE_CIO2_ENDPOINT] = NODE_ENDPOINT(
+ sensor->node_names.endpoint,
+ &nodes[SWNODE_CIO2_PORT],
+ sensor->cio2_properties);
+}
+
+static void cio2_bridge_unregister_sensors(struct cio2_bridge *bridge)
+{
+ struct cio2_sensor *sensor;
+ unsigned int i;
+
+ for (i = 0; i < bridge->n_sensors; i++) {
+ sensor = &bridge->sensors[i];
+ software_node_unregister_nodes(sensor->swnodes);
+ acpi_dev_put(sensor->adev);
+ }
+}
+
+static int cio2_bridge_connect_sensor(const struct cio2_sensor_config *cfg,
+ struct cio2_bridge *bridge,
+ struct pci_dev *cio2)
+{
+ struct fwnode_handle *fwnode;
+ struct cio2_sensor *sensor;
+ struct acpi_device *adev;
+ int ret;
+
+ for_each_acpi_dev_match(adev, cfg->hid, NULL, -1) {
+ if (!adev->status.enabled)
+ continue;
+
+ if (bridge->n_sensors >= CIO2_NUM_PORTS) {
+ dev_err(&cio2->dev, "Exceeded available CIO2 ports\n");
+ cio2_bridge_unregister_sensors(bridge);
+ ret = -EINVAL;
+ goto err_out;
+ }
+
+ sensor = &bridge->sensors[bridge->n_sensors];
+ sensor->adev = adev;
+ strscpy(sensor->name, cfg->hid, sizeof(sensor->name));
+
+ ret = cio2_bridge_read_acpi_buffer(adev, "SSDB",
+ &sensor->ssdb,
+ sizeof(sensor->ssdb));
+ if (ret)
+ goto err_put_adev;
+
+ if (sensor->ssdb.lanes > CIO2_MAX_LANES) {
+ dev_err(&adev->dev,
+ "Number of lanes in SSDB is invalid\n");
+ ret = -EINVAL;
+ goto err_put_adev;
+ }
+
+ cio2_bridge_create_fwnode_properties(sensor, bridge, cfg);
+ cio2_bridge_create_connection_swnodes(bridge, sensor);
+
+ ret = software_node_register_nodes(sensor->swnodes);
+ if (ret)
+ goto err_put_adev;
+
+ fwnode = software_node_fwnode(&sensor->swnodes[SWNODE_SENSOR_HID]);
+ if (!fwnode) {
+ ret = -ENODEV;
+ goto err_free_swnodes;
+ }
+
+ adev->fwnode.secondary = fwnode;
+
+ dev_info(&cio2->dev, "Found supported sensor %s\n",
+ acpi_dev_name(adev));
+
+ bridge->n_sensors++;
+ }
+
+ return 0;
+
+err_free_swnodes:
+ software_node_unregister_nodes(sensor->swnodes);
+err_put_adev:
+ acpi_dev_put(sensor->adev);
+err_out:
+ return ret;
+}
+
+static int cio2_bridge_connect_sensors(struct cio2_bridge *bridge,
+ struct pci_dev *cio2)
+{
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < ARRAY_SIZE(cio2_supported_sensors); i++) {
+ const struct cio2_sensor_config *cfg = &cio2_supported_sensors[i];
+
+ ret = cio2_bridge_connect_sensor(cfg, bridge, cio2);
+ if (ret)
+ goto err_unregister_sensors;
+ }
+
+ return 0;
+
+err_unregister_sensors:
+ cio2_bridge_unregister_sensors(bridge);
+ return ret;
+}
+
+int cio2_bridge_init(struct pci_dev *cio2)
+{
+ struct device *dev = &cio2->dev;
+ struct fwnode_handle *fwnode;
+ struct cio2_bridge *bridge;
+ unsigned int i;
+ int ret;
+
+ bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
+ if (!bridge)
+ return -ENOMEM;
+
+ strscpy(bridge->cio2_node_name, CIO2_HID, sizeof(bridge->cio2_node_name));
+ bridge->cio2_hid_node.name = bridge->cio2_node_name;
+
+ ret = software_node_register(&bridge->cio2_hid_node);
+ if (ret < 0) {
+ dev_err(dev, "Failed to register the CIO2 HID node\n");
+ goto err_free_bridge;
+ }
+
+ /*
+ * Map the lane arrangement, which is fixed for the IPU3 (meaning we
+ * only need one, rather than one per sensor). We include it as a
+ * member of the struct cio2_bridge rather than a global variable so
+ * that it survives if the module is unloaded along with the rest of
+ * the struct.
+ */
+ for (i = 0; i < CIO2_MAX_LANES; i++)
+ bridge->data_lanes[i] = i + 1;
+
+ ret = cio2_bridge_connect_sensors(bridge, cio2);
+ if (ret || bridge->n_sensors == 0)
+ goto err_unregister_cio2;
+
+ dev_info(dev, "Connected %d cameras\n", bridge->n_sensors);
+
+ fwnode = software_node_fwnode(&bridge->cio2_hid_node);
+ if (!fwnode) {
+ dev_err(dev, "Error getting fwnode from cio2 software_node\n");
+ ret = -ENODEV;
+ goto err_unregister_sensors;
+ }
+
+ set_secondary_fwnode(dev, fwnode);
+
+ return 0;
+
+err_unregister_sensors:
+ cio2_bridge_unregister_sensors(bridge);
+err_unregister_cio2:
+ software_node_unregister(&bridge->cio2_hid_node);
+err_free_bridge:
+ kfree(bridge);
+
+ return ret;
+}
diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.h b/drivers/media/pci/intel/ipu3/cio2-bridge.h
new file mode 100644
index 000000000000..dd0ffcafa489
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/cio2-bridge.h
@@ -0,0 +1,125 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Author: Dan Scally <[email protected]> */
+#ifndef __CIO2_BRIDGE_H
+#define __CIO2_BRIDGE_H
+
+#include <linux/property.h>
+#include <linux/types.h>
+
+#include "ipu3-cio2.h"
+
+#define CIO2_HID "INT343E"
+#define CIO2_MAX_LANES 4
+#define MAX_NUM_LINK_FREQS 3
+
+#define CIO2_SENSOR_CONFIG(_HID, _NR, ...) \
+ (const struct cio2_sensor_config) { \
+ .hid = _HID, \
+ .nr_link_freqs = _NR, \
+ .link_freqs = { __VA_ARGS__ } \
+ }
+
+#define NODE_SENSOR(_HID, _PROPS) \
+ (const struct software_node) { \
+ .name = _HID, \
+ .properties = _PROPS, \
+ }
+
+#define NODE_PORT(_PORT, _SENSOR_NODE) \
+ (const struct software_node) { \
+ .name = _PORT, \
+ .parent = _SENSOR_NODE, \
+ }
+
+#define NODE_ENDPOINT(_EP, _PORT, _PROPS) \
+ (const struct software_node) { \
+ .name = _EP, \
+ .parent = _PORT, \
+ .properties = _PROPS, \
+ }
+
+enum cio2_sensor_swnodes {
+ SWNODE_SENSOR_HID,
+ SWNODE_SENSOR_PORT,
+ SWNODE_SENSOR_ENDPOINT,
+ SWNODE_CIO2_PORT,
+ SWNODE_CIO2_ENDPOINT,
+ SWNODE_COUNT
+};
+
+/* Data representation as it is in ACPI SSDB buffer */
+struct cio2_sensor_ssdb {
+ u8 version;
+ u8 sku;
+ u8 guid_csi2[16];
+ u8 devfunction;
+ u8 bus;
+ u32 dphylinkenfuses;
+ u32 clockdiv;
+ u8 link;
+ u8 lanes;
+ u32 csiparams[10];
+ u32 maxlanespeed;
+ u8 sensorcalibfileidx;
+ u8 sensorcalibfileidxInMBZ[3];
+ u8 romtype;
+ u8 vcmtype;
+ u8 platforminfo;
+ u8 platformsubinfo;
+ u8 flash;
+ u8 privacyled;
+ u8 degree;
+ u8 mipilinkdefined;
+ u32 mclkspeed;
+ u8 controllogicid;
+ u8 reserved1[3];
+ u8 mclkport;
+ u8 reserved2[13];
+} __packed;
+
+struct cio2_property_names {
+ char clock_frequency[16];
+ char rotation[9];
+ char bus_type[9];
+ char data_lanes[11];
+ char remote_endpoint[16];
+ char link_frequencies[17];
+};
+
+struct cio2_node_names {
+ char port[7];
+ char endpoint[11];
+ char remote_port[7];
+};
+
+struct cio2_sensor_config {
+ const char *hid;
+ const u8 nr_link_freqs;
+ const u64 link_freqs[MAX_NUM_LINK_FREQS];
+};
+
+struct cio2_sensor {
+ char name[ACPI_ID_LEN];
+ struct acpi_device *adev;
+
+ struct software_node swnodes[6];
+ struct cio2_node_names node_names;
+
+ struct cio2_sensor_ssdb ssdb;
+ struct cio2_property_names prop_names;
+ struct property_entry ep_properties[5];
+ struct property_entry dev_properties[3];
+ struct property_entry cio2_properties[3];
+ struct software_node_ref_args local_ref[1];
+ struct software_node_ref_args remote_ref[1];
+};
+
+struct cio2_bridge {
+ char cio2_node_name[ACPI_ID_LEN];
+ struct software_node cio2_hid_node;
+ u32 data_lanes[4];
+ unsigned int n_sensors;
+ struct cio2_sensor sensors[CIO2_NUM_PORTS];
+};
+
+#endif
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
index 36e354ecf71e..db59b19a6236 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
@@ -1702,11 +1702,28 @@ static void cio2_queues_exit(struct cio2_device *cio2)
cio2_queue_exit(cio2, &cio2->queue[i]);
}
+static int cio2_check_fwnode_graph(struct fwnode_handle *fwnode)
+{
+ struct fwnode_handle *endpoint;
+
+ if (IS_ERR_OR_NULL(fwnode))
+ return -EINVAL;
+
+ endpoint = fwnode_graph_get_next_endpoint(fwnode, NULL);
+ if (endpoint) {
+ fwnode_handle_put(endpoint);
+ return 0;
+ }
+
+ return cio2_check_fwnode_graph(fwnode->secondary);
+}
+
/**************** PCI interface ****************/
static int cio2_pci_probe(struct pci_dev *pci_dev,
const struct pci_device_id *id)
{
+ struct fwnode_handle *fwnode = dev_fwnode(&pci_dev->dev);
struct cio2_device *cio2;
int r;
@@ -1715,6 +1732,23 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
return -ENOMEM;
cio2->pci_dev = pci_dev;
+ /*
+ * On some platforms no connections to sensors are defined in firmware,
+ * if the device has no endpoints then we can try to build those as
+ * software_nodes parsed from SSDB.
+ */
+ r = cio2_check_fwnode_graph(fwnode);
+ if (r) {
+ if (fwnode && !IS_ERR_OR_NULL(fwnode->secondary)) {
+ dev_err(&pci_dev->dev, "fwnode graph has no endpoints connected\n");
+ return -EINVAL;
+ }
+
+ r = cio2_bridge_init(pci_dev);
+ if (r)
+ return r;
+ }
+
r = pcim_enable_device(pci_dev);
if (r) {
dev_err(&pci_dev->dev, "failed to enable device (%d)\n", r);
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.h b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
index 62187ab5ae43..dc3e343a37fb 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.h
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
@@ -455,4 +455,10 @@ static inline struct cio2_queue *vb2q_to_cio2_queue(struct vb2_queue *vq)
return container_of(vq, struct cio2_queue, vbq);
}
+#if IS_ENABLED(CONFIG_CIO2_BRIDGE)
+int cio2_bridge_init(struct pci_dev *cio2);
+#else
+int cio2_bridge_init(struct pci_dev *cio2) { return 0; }
+#endif
+
#endif
--
2.25.1
ipu3-cio2 driver needs extending with multiple files; rename the main
source file and specify the renamed file in Makefile to accommodate that.
Suggested-by: Andy Shevchenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Daniel Scally <[email protected]>
---
Changes in v5:
- None
drivers/media/pci/intel/ipu3/Makefile | 2 ++
drivers/media/pci/intel/ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} | 0
2 files changed, 2 insertions(+)
rename drivers/media/pci/intel/ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} (100%)
diff --git a/drivers/media/pci/intel/ipu3/Makefile b/drivers/media/pci/intel/ipu3/Makefile
index 98ddd5beafe0..429d516452e4 100644
--- a/drivers/media/pci/intel/ipu3/Makefile
+++ b/drivers/media/pci/intel/ipu3/Makefile
@@ -1,2 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
+
+ipu3-cio2-y += ipu3-cio2-main.o
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
similarity index 100%
rename from drivers/media/pci/intel/ipu3/ipu3-cio2.c
rename to drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
--
2.25.1
Where the fwnode graph is comprised of software_nodes, these will be
assigned as the secondary to dev->fwnode. Check the v4l2_subdev's fwnode
for a secondary and attempt to match against it during match_fwnode() to
accommodate that possibility.
Reviewed-by: Andy Shevchenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Signed-off-by: Daniel Scally <[email protected]>
---
Changes in v5:
- None
drivers/media/v4l2-core/v4l2-async.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index e3ab003a6c85..9dd896d085ec 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -87,6 +87,14 @@ static bool match_fwnode(struct v4l2_async_notifier *notifier,
if (sd->fwnode == asd->match.fwnode)
return true;
+ /*
+ * Check the same situation for any possible secondary assigned to the
+ * subdev's fwnode
+ */
+ if (!IS_ERR_OR_NULL(sd->fwnode->secondary) &&
+ sd->fwnode->secondary == asd->match.fwnode)
+ return true;
+
/*
* Otherwise, check if the sd fwnode and the asd fwnode refer to an
* endpoint or a device. If they're of the same type, there's no match.
--
2.25.1
Registering software_nodes with the .parent member set to point to a
currently unregistered software_node has the potential for problems,
so enforce parent -> child ordering in arrays passed in to
software_node_register_nodes().
Software nodes that are children of another software node should be
unregistered before their parent. To allow easy unregistering of an array
of software_nodes ordered parent to child, reverse the order in which
software_node_unregister_nodes() unregisters software_nodes.
Suggested-by: Andy Shevchenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Signed-off-by: Daniel Scally <[email protected]>
---
Changes in v5:
- None
drivers/base/swnode.c | 42 ++++++++++++++++++++++++++++++------------
1 file changed, 30 insertions(+), 12 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 4fcc1a6fb724..166c5cc73f39 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -692,7 +692,11 @@ swnode_register(const struct software_node *node, struct swnode *parent,
* software_node_register_nodes - Register an array of software nodes
* @nodes: Zero terminated array of software nodes to be registered
*
- * Register multiple software nodes at once.
+ * Register multiple software nodes at once. If any node in the array
+ * has its .parent pointer set (which can only be to another software_node),
+ * then its parent **must** have been registered before it is; either outside
+ * of this function or by ordering the array such that parent comes before
+ * child.
*/
int software_node_register_nodes(const struct software_node *nodes)
{
@@ -700,14 +704,23 @@ int software_node_register_nodes(const struct software_node *nodes)
int i;
for (i = 0; nodes[i].name; i++) {
- ret = software_node_register(&nodes[i]);
- if (ret) {
- software_node_unregister_nodes(nodes);
- return ret;
+ const struct software_node *parent = nodes[i].parent;
+
+ if (parent && !software_node_to_swnode(parent)) {
+ ret = -EINVAL;
+ goto err_unregister_nodes;
}
+
+ ret = software_node_register(&nodes[i]);
+ if (ret)
+ goto err_unregister_nodes;
}
return 0;
+
+err_unregister_nodes:
+ software_node_unregister_nodes(nodes);
+ return ret;
}
EXPORT_SYMBOL_GPL(software_node_register_nodes);
@@ -715,18 +728,23 @@ EXPORT_SYMBOL_GPL(software_node_register_nodes);
* software_node_unregister_nodes - Unregister an array of software nodes
* @nodes: Zero terminated array of software nodes to be unregistered
*
- * Unregister multiple software nodes at once.
+ * Unregister multiple software nodes at once. If parent pointers are set up
+ * in any of the software nodes then the array **must** be ordered such that
+ * parents come before their children.
*
- * NOTE: Be careful using this call if the nodes had parent pointers set up in
- * them before registering. If so, it is wiser to remove the nodes
- * individually, in the correct order (child before parent) instead of relying
- * on the sequential order of the list of nodes in the array.
+ * NOTE: If you are uncertain whether the array is ordered such that
+ * parents will be unregistered before their children, it is wiser to
+ * remove the nodes individually, in the correct order (child before
+ * parent).
*/
void software_node_unregister_nodes(const struct software_node *nodes)
{
- int i;
+ unsigned int i = 0;
+
+ while (nodes[i].name)
+ i++;
- for (i = 0; nodes[i].name; i++)
+ while (i--)
software_node_unregister(&nodes[i]);
}
EXPORT_SYMBOL_GPL(software_node_unregister_nodes);
--
2.25.1
Development for the ipu3-cio2 driver is taking place in media_tree, but
there's no T: entry in MAINTAINERS to denote that - rectify that oversight
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Daniel Scally <[email protected]>
---
Changes in v5:
- None
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 471561d9d55f..92228e8dd868 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9017,6 +9017,7 @@ M: Bingbu Cao <[email protected]>
R: Tianshu Qiu <[email protected]>
L: [email protected]
S: Maintained
+T: git git://linuxtv.org/media_tree.git
F: Documentation/userspace-api/media/v4l/pixfmt-srggb10-ipu3.rst
F: drivers/media/pci/intel/ipu3/
--
2.25.1
To ensure we handle situations in which multiple sensors of the same
model (and therefore _HID) are present in a system, we need to be able
to iterate over devices matching a known _HID but unknown _UID and _HRV
- add acpi_dev_get_next_match_dev() to accommodate that possibility and
change acpi_dev_get_first_match_dev() to simply call the new function
with a NULL starting point. Add an iterator macro for convenience.
Reviewed-by: Andy Shevchenko <[email protected]>
Reviewed-by: Sakari Ailus <[email protected]>
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Daniel Scally <[email protected]>
---
Changes in v5:
- Changed commit subject
drivers/acpi/utils.c | 30 ++++++++++++++++++++++++++----
include/acpi/acpi_bus.h | 7 +++++++
2 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index d5411a166685..ddca1550cce6 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -843,12 +843,13 @@ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
EXPORT_SYMBOL(acpi_dev_present);
/**
- * acpi_dev_get_first_match_dev - Return the first match of ACPI device
+ * acpi_dev_get_next_match_dev - Return the next match of ACPI device
+ * @adev: Pointer to the previous acpi_device matching this @hid, @uid and @hrv
* @hid: Hardware ID of the device.
* @uid: Unique ID of the device, pass NULL to not check _UID
* @hrv: Hardware Revision of the device, pass -1 to not check _HRV
*
- * Return the first match of ACPI device if a matching device was present
+ * Return the next match of ACPI device if another matching device was present
* at the moment of invocation, or NULL otherwise.
*
* The caller is responsible to call put_device() on the returned device.
@@ -856,8 +857,9 @@ EXPORT_SYMBOL(acpi_dev_present);
* See additional information in acpi_dev_present() as well.
*/
struct acpi_device *
-acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
+acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv)
{
+ struct device *start = adev ? &adev->dev : NULL;
struct acpi_dev_match_info match = {};
struct device *dev;
@@ -865,9 +867,29 @@ acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
match.uid = uid;
match.hrv = hrv;
- dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
+ dev = bus_find_device(&acpi_bus_type, start, &match, acpi_dev_match_cb);
return dev ? to_acpi_device(dev) : NULL;
}
+EXPORT_SYMBOL(acpi_dev_get_next_match_dev);
+
+/**
+ * acpi_dev_get_first_match_dev - Return the first match of ACPI device
+ * @hid: Hardware ID of the device.
+ * @uid: Unique ID of the device, pass NULL to not check _UID
+ * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
+ *
+ * Return the first match of ACPI device if a matching device was present
+ * at the moment of invocation, or NULL otherwise.
+ *
+ * The caller is responsible to call put_device() on the returned device.
+ *
+ * See additional information in acpi_dev_present() as well.
+ */
+struct acpi_device *
+acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
+{
+ return acpi_dev_get_next_match_dev(NULL, hid, uid, hrv);
+}
EXPORT_SYMBOL(acpi_dev_get_first_match_dev);
/*
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 6d1879bf9440..02a716a0af5d 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -683,9 +683,16 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2);
+struct acpi_device *
+acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);
struct acpi_device *
acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv);
+#define for_each_acpi_dev_match(adev, hid, uid, hrv) \
+ for (adev = acpi_dev_get_first_match_dev(hid, uid, hrv); \
+ adev; \
+ adev = acpi_dev_get_next_match_dev(adev, hid, uid, hrv))
+
static inline void acpi_dev_put(struct acpi_device *adev)
{
put_device(&adev->dev);
--
2.25.1
To maintain consistency with software_node_unregister_nodes(), reverse
the order in which the software_node_unregister_node_group() function
unregisters nodes.
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Sakari Ailus <[email protected]>
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Daniel Scally <[email protected]>
---
Changes in v5:
- None
drivers/base/swnode.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 166c5cc73f39..6f7443c6d3b5 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -779,16 +779,23 @@ EXPORT_SYMBOL_GPL(software_node_register_node_group);
* software_node_unregister_node_group - Unregister a group of software nodes
* @node_group: NULL terminated array of software node pointers to be unregistered
*
- * Unregister multiple software nodes at once.
+ * Unregister multiple software nodes at once. The array will be unwound in
+ * reverse order (i.e. last entry first) and thus if any members of the array are
+ * children of another member then the children must appear later in the list such
+ * that they are unregistered first.
*/
-void software_node_unregister_node_group(const struct software_node **node_group)
+void software_node_unregister_node_group(
+ const struct software_node **node_group)
{
- unsigned int i;
+ unsigned int i = 0;
if (!node_group)
return;
- for (i = 0; node_group[i]; i++)
+ while (node_group[i])
+ i++;
+
+ while (i--)
software_node_unregister(node_group[i]);
}
EXPORT_SYMBOL_GPL(software_node_unregister_node_group);
--
2.25.1
V4L2 fwnode bus types are enumerated in v4l2-fwnode.c, meaning they aren't
available to the rest of the kernel. Move the enum to the corresponding
header so that I can use the label to refer to those values.
Suggested-by: Andy Shevchenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Daniel Scally <[email protected]>
---
Changes in v5:
- Changed commit subject
drivers/media/v4l2-core/v4l2-fwnode.c | 11 -----------
include/media/v4l2-fwnode.h | 22 ++++++++++++++++++++++
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 5353e37eb950..c1c2b3060532 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -28,17 +28,6 @@
#include <media/v4l2-fwnode.h>
#include <media/v4l2-subdev.h>
-enum v4l2_fwnode_bus_type {
- V4L2_FWNODE_BUS_TYPE_GUESS = 0,
- V4L2_FWNODE_BUS_TYPE_CSI2_CPHY,
- V4L2_FWNODE_BUS_TYPE_CSI1,
- V4L2_FWNODE_BUS_TYPE_CCP2,
- V4L2_FWNODE_BUS_TYPE_CSI2_DPHY,
- V4L2_FWNODE_BUS_TYPE_PARALLEL,
- V4L2_FWNODE_BUS_TYPE_BT656,
- NR_OF_V4L2_FWNODE_BUS_TYPE,
-};
-
static const struct v4l2_fwnode_bus_conv {
enum v4l2_fwnode_bus_type fwnode_bus_type;
enum v4l2_mbus_type mbus_type;
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index 6d026dadbf98..e7ad95365a40 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -213,6 +213,28 @@ struct v4l2_fwnode_connector {
} connector;
};
+/**
+ * enum v4l2_fwnode_bus_type - Video bus types defined by firmware properties
+ * @V4L2_FWNODE_BUS_TYPE_GUESS: Default value if no bus-type fwnode property
+ * @V4L2_FWNODE_BUS_TYPE_CSI2_CPHY: MIPI CSI-2 bus, C-PHY physical layer
+ * @V4L2_FWNODE_BUS_TYPE_CSI1: MIPI CSI-1 bus
+ * @V4L2_FWNODE_BUS_TYPE_CCP2: SMIA Compact Camera Port 2 bus
+ * @V4L2_FWNODE_BUS_TYPE_CSI2_DPHY: MIPI CSI-2 bus, D-PHY physical layer
+ * @V4L2_FWNODE_BUS_TYPE_PARALLEL: Camera Parallel Interface bus
+ * @V4L2_FWNODE_BUS_TYPE_BT656: BT.656 video format bus-type
+ * @NR_OF_V4L2_FWNODE_BUS_TYPE: Number of bus-types
+ */
+enum v4l2_fwnode_bus_type {
+ V4L2_FWNODE_BUS_TYPE_GUESS = 0,
+ V4L2_FWNODE_BUS_TYPE_CSI2_CPHY,
+ V4L2_FWNODE_BUS_TYPE_CSI1,
+ V4L2_FWNODE_BUS_TYPE_CCP2,
+ V4L2_FWNODE_BUS_TYPE_CSI2_DPHY,
+ V4L2_FWNODE_BUS_TYPE_PARALLEL,
+ V4L2_FWNODE_BUS_TYPE_BT656,
+ NR_OF_V4L2_FWNODE_BUS_TYPE
+};
+
/**
* v4l2_fwnode_endpoint_parse() - parse all fwnode node properties
* @fwnode: pointer to the endpoint's fwnode handle
--
2.25.1
OF, ACPI and software_nodes all implement graphs including nodes for ports
and endpoints. These are all intended to be named with a common schema,
as "port@n" and "endpoint@n" where n is an unsigned int representing the
index of the node. To ensure commonality across the subsystems, provide a
set of macros to define the format.
Suggested-by: Andy Shevchenko <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Signed-off-by: Daniel Scally <[email protected]>
---
Changes in v5:
- Changed commit subject
include/linux/fwnode.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index fde4ad97564c..77414e431e89 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -50,6 +50,13 @@ struct fwnode_endpoint {
const struct fwnode_handle *local_fwnode;
};
+/*
+ * ports and endpoints defined as software_nodes should all follow a common
+ * naming scheme; use these macros to ensure commonality.
+ */
+#define SWNODE_GRAPH_PORT_NAME_FMT "port@%u"
+#define SWNODE_GRAPH_ENDPOINT_NAME_FMT "endpoint@%u"
+
#define NR_FWNODE_REFERENCE_ARGS 8
/**
--
2.25.1
Use the software_node_unregister_nodes() helper function to unwind this
array in a cleaner way.
Acked-by: Petr Mladek <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Sergey Senozhatsky <[email protected]>
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Daniel Scally <[email protected]>
---
Changes in v5:
- None
lib/test_printf.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/lib/test_printf.c b/lib/test_printf.c
index 7ac87f18a10f..7d60f24240a4 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -644,9 +644,7 @@ static void __init fwnode_pointer(void)
test(second_name, "%pfwP", software_node_fwnode(&softnodes[1]));
test(third_name, "%pfwP", software_node_fwnode(&softnodes[2]));
- software_node_unregister(&softnodes[2]);
- software_node_unregister(&softnodes[1]);
- software_node_unregister(&softnodes[0]);
+ software_node_unregister_nodes(softnodes);
}
static void __init
--
2.25.1
From: Heikki Krogerus <[email protected]>
This implements the remaining .graph_*() callbacks in the fwnode
operations structure for the software nodes. That makes the
fwnode_graph_*() functions available in the drivers also when software
nodes are used.
The implementation tries to mimic the "OF graph" as much as possible, but
there is no support for the "reg" device property. The ports will need to
have the index in their name which starts with "port@" (for example
"port@0", "port@1", ...) and endpoints will use the index of the software
node that is given to them during creation. The port nodes can also be
grouped under a specially named "ports" subnode, just like in DT, if
necessary.
The remote-endpoints are reference properties under the endpoint nodes
that are named "remote-endpoint".
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Heikki Krogerus <[email protected]>
Co-developed-by: Daniel Scally <[email protected]>
Signed-off-by: Daniel Scally <[email protected]>
---
Changes in v5:
- Cosmetic changes only
drivers/base/swnode.c | 115 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 114 insertions(+), 1 deletion(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 6f7443c6d3b5..9104a0abd531 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -540,6 +540,115 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
return 0;
}
+static struct fwnode_handle *
+swnode_graph_find_next_port(const struct fwnode_handle *parent,
+ struct fwnode_handle *port)
+{
+ struct fwnode_handle *old = port;
+
+ while ((port = software_node_get_next_child(parent, old))) {
+ /*
+ * fwnode ports have naming style "port@", so we search for any
+ * children that follow that convention.
+ */
+ if (!strncmp(to_swnode(port)->node->name, "port@",
+ strlen("port@")))
+ return port;
+ old = port;
+ }
+
+ return NULL;
+}
+
+static struct fwnode_handle *
+software_node_graph_get_next_endpoint(const struct fwnode_handle *fwnode,
+ struct fwnode_handle *endpoint)
+{
+ struct swnode *swnode = to_swnode(fwnode);
+ struct fwnode_handle *parent;
+ struct fwnode_handle *port;
+
+ if (!swnode)
+ return NULL;
+
+ if (endpoint) {
+ port = software_node_get_parent(endpoint);
+ parent = software_node_get_parent(port);
+ } else {
+ parent = software_node_get_named_child_node(fwnode, "ports");
+ if (!parent)
+ parent = software_node_get(&swnode->fwnode);
+
+ port = swnode_graph_find_next_port(parent, NULL);
+ }
+
+ for (; port; port = swnode_graph_find_next_port(parent, port)) {
+ endpoint = software_node_get_next_child(port, endpoint);
+ if (endpoint) {
+ fwnode_handle_put(port);
+ break;
+ }
+ }
+
+ fwnode_handle_put(parent);
+
+ return endpoint;
+}
+
+static struct fwnode_handle *
+software_node_graph_get_remote_endpoint(const struct fwnode_handle *fwnode)
+{
+ struct swnode *swnode = to_swnode(fwnode);
+ const struct software_node_ref_args *ref;
+ const struct property_entry *prop;
+
+ if (!swnode)
+ return NULL;
+
+ prop = property_entry_get(swnode->node->properties, "remote-endpoint");
+ if (!prop || prop->type != DEV_PROP_REF || prop->is_inline)
+ return NULL;
+
+ ref = prop->pointer;
+
+ return software_node_get(software_node_fwnode(ref[0].node));
+}
+
+static struct fwnode_handle *
+software_node_graph_get_port_parent(struct fwnode_handle *fwnode)
+{
+ struct swnode *swnode = to_swnode(fwnode);
+
+ swnode = swnode->parent;
+ if (swnode && !strcmp(swnode->node->name, "ports"))
+ swnode = swnode->parent;
+
+ return swnode ? software_node_get(&swnode->fwnode) : NULL;
+}
+
+static int
+software_node_graph_parse_endpoint(const struct fwnode_handle *fwnode,
+ struct fwnode_endpoint *endpoint)
+{
+ struct swnode *swnode = to_swnode(fwnode);
+ const char *parent_name = swnode->parent->node->name;
+ int ret;
+
+ if (strlen("port@") >= strlen(parent_name) ||
+ strncmp(parent_name, "port@", strlen("port@")))
+ return -EINVAL;
+
+ /* Ports have naming style "port@n", we need to select the n */
+ ret = kstrtou32(parent_name + strlen("port@"), 10, &endpoint->port);
+ if (ret)
+ return ret;
+
+ endpoint->id = swnode->id;
+ endpoint->local_fwnode = fwnode;
+
+ return 0;
+}
+
static const struct fwnode_operations software_node_ops = {
.get = software_node_get,
.put = software_node_put,
@@ -551,7 +660,11 @@ static const struct fwnode_operations software_node_ops = {
.get_parent = software_node_get_parent,
.get_next_child_node = software_node_get_next_child,
.get_named_child_node = software_node_get_named_child_node,
- .get_reference_args = software_node_get_reference_args
+ .get_reference_args = software_node_get_reference_args,
+ .graph_get_next_endpoint = software_node_graph_get_next_endpoint,
+ .graph_get_remote_endpoint = software_node_graph_get_remote_endpoint,
+ .graph_get_port_parent = software_node_graph_get_port_parent,
+ .graph_parse_endpoint = software_node_graph_parse_endpoint,
};
/* -------------------------------------------------------------------------- */
--
2.25.1
On Thu, Jan 07, 2021 at 01:28:28PM +0000, Daniel Scally wrote:
> Registering software_nodes with the .parent member set to point to a
> currently unregistered software_node has the potential for problems,
> so enforce parent -> child ordering in arrays passed in to
> software_node_register_nodes().
>
> Software nodes that are children of another software node should be
> unregistered before their parent. To allow easy unregistering of an array
> of software_nodes ordered parent to child, reverse the order in which
> software_node_unregister_nodes() unregisters software_nodes.
>
> Suggested-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> Changes in v5:
>
> - None
>
> drivers/base/swnode.c | 42 ++++++++++++++++++++++++++++++------------
> 1 file changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 4fcc1a6fb724..166c5cc73f39 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -692,7 +692,11 @@ swnode_register(const struct software_node *node, struct swnode *parent,
> * software_node_register_nodes - Register an array of software nodes
> * @nodes: Zero terminated array of software nodes to be registered
> *
> - * Register multiple software nodes at once.
> + * Register multiple software nodes at once. If any node in the array
> + * has its .parent pointer set (which can only be to another software_node),
> + * then its parent **must** have been registered before it is; either outside
> + * of this function or by ordering the array such that parent comes before
> + * child.
> */
> int software_node_register_nodes(const struct software_node *nodes)
> {
> @@ -700,14 +704,23 @@ int software_node_register_nodes(const struct software_node *nodes)
> int i;
>
> for (i = 0; nodes[i].name; i++) {
> - ret = software_node_register(&nodes[i]);
> - if (ret) {
> - software_node_unregister_nodes(nodes);
> - return ret;
> + const struct software_node *parent = nodes[i].parent;
> +
> + if (parent && !software_node_to_swnode(parent)) {
> + ret = -EINVAL;
> + goto err_unregister_nodes;
> }
> +
> + ret = software_node_register(&nodes[i]);
> + if (ret)
> + goto err_unregister_nodes;
> }
>
> return 0;
> +
> +err_unregister_nodes:
> + software_node_unregister_nodes(nodes);
> + return ret;
> }
> EXPORT_SYMBOL_GPL(software_node_register_nodes);
>
> @@ -715,18 +728,23 @@ EXPORT_SYMBOL_GPL(software_node_register_nodes);
> * software_node_unregister_nodes - Unregister an array of software nodes
> * @nodes: Zero terminated array of software nodes to be unregistered
> *
> - * Unregister multiple software nodes at once.
> + * Unregister multiple software nodes at once. If parent pointers are set up
> + * in any of the software nodes then the array **must** be ordered such that
> + * parents come before their children.
> *
> - * NOTE: Be careful using this call if the nodes had parent pointers set up in
> - * them before registering. If so, it is wiser to remove the nodes
> - * individually, in the correct order (child before parent) instead of relying
> - * on the sequential order of the list of nodes in the array.
> + * NOTE: If you are uncertain whether the array is ordered such that
> + * parents will be unregistered before their children, it is wiser to
> + * remove the nodes individually, in the correct order (child before
> + * parent).
> */
> void software_node_unregister_nodes(const struct software_node *nodes)
> {
> - int i;
> + unsigned int i = 0;
> +
> + while (nodes[i].name)
> + i++;
>
> - for (i = 0; nodes[i].name; i++)
> + while (i--)
> software_node_unregister(&nodes[i]);
> }
> EXPORT_SYMBOL_GPL(software_node_unregister_nodes);
> --
> 2.25.1
--
heikki
On Thu, Jan 07, 2021 at 04:05:05PM +0200, Andy Shevchenko wrote:
> On Thu, Jan 07, 2021 at 01:28:38PM +0000, Daniel Scally wrote:
> Seems like missed changelog (no need to resend, just send a follow up).
I meant the follow up with only changelog text, thanks!
--
With Best Regards,
Andy Shevchenko
On Thu, Jan 07, 2021 at 01:28:38PM +0000, Daniel Scally wrote:
> Currently on platforms designed for Windows, connections between CIO2 and
> sensors are not properly defined in DSDT. This patch extends the ipu3-cio2
> driver to compensate by building software_node connections, parsing the
> connection properties from the sensor's SSDB buffer.
>
> Suggested-by: Jordan Hand <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Kieran Bingham <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
> ---
Seems like missed changelog (no need to resend, just send a follow up).
--
With Best Regards,
Andy Shevchenko
On Thu, Jan 07, 2021 at 01:28:29PM +0000, Daniel Scally wrote:
> To maintain consistency with software_node_unregister_nodes(), reverse
> the order in which the software_node_unregister_node_group() function
> unregisters nodes.
>
> Reported-by: kernel test robot <[email protected]>
> Reported-by: Dan Carpenter <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Reviewed-by: Sakari Ailus <[email protected]>
> Suggested-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> Changes in v5:
>
> - None
>
> drivers/base/swnode.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 166c5cc73f39..6f7443c6d3b5 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -779,16 +779,23 @@ EXPORT_SYMBOL_GPL(software_node_register_node_group);
> * software_node_unregister_node_group - Unregister a group of software nodes
> * @node_group: NULL terminated array of software node pointers to be unregistered
> *
> - * Unregister multiple software nodes at once.
> + * Unregister multiple software nodes at once. The array will be unwound in
> + * reverse order (i.e. last entry first) and thus if any members of the array are
> + * children of another member then the children must appear later in the list such
> + * that they are unregistered first.
> */
> -void software_node_unregister_node_group(const struct software_node **node_group)
> +void software_node_unregister_node_group(
> + const struct software_node **node_group)
> {
> - unsigned int i;
> + unsigned int i = 0;
>
> if (!node_group)
> return;
>
> - for (i = 0; node_group[i]; i++)
> + while (node_group[i])
> + i++;
> +
> + while (i--)
> software_node_unregister(node_group[i]);
> }
> EXPORT_SYMBOL_GPL(software_node_unregister_node_group);
> --
> 2.25.1
--
heikki
On Thu, Jan 07, 2021 at 01:28:30PM +0000, Daniel Scally wrote:
> OF, ACPI and software_nodes all implement graphs including nodes for ports
> and endpoints. These are all intended to be named with a common schema,
> as "port@n" and "endpoint@n" where n is an unsigned int representing the
> index of the node. To ensure commonality across the subsystems, provide a
> set of macros to define the format.
>
> Suggested-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
> Changes in v5:
>
> - Changed commit subject
>
> include/linux/fwnode.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index fde4ad97564c..77414e431e89 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -50,6 +50,13 @@ struct fwnode_endpoint {
> const struct fwnode_handle *local_fwnode;
> };
>
> +/*
> + * ports and endpoints defined as software_nodes should all follow a common
> + * naming scheme; use these macros to ensure commonality.
> + */
> +#define SWNODE_GRAPH_PORT_NAME_FMT "port@%u"
> +#define SWNODE_GRAPH_ENDPOINT_NAME_FMT "endpoint@%u"
> +
> #define NR_FWNODE_REFERENCE_ARGS 8
>
> /**
> --
> 2.25.1
--
heikki
On 07/01/2021 13:28, Daniel Scally wrote:
> Currently on platforms designed for Windows, connections between CIO2 and
> sensors are not properly defined in DSDT. This patch extends the ipu3-cio2
> driver to compensate by building software_node connections, parsing the
> connection properties from the sensor's SSDB buffer.
>
> Suggested-by: Jordan Hand <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Kieran Bingham <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
> ---
Sorry all, missed the changelog on this one:
---
Changes in v5:
- Changed some macros to declare compound literals
- Switched cio2_check_fwnode_graph() to return int instead of bool
- Added some comments to clarify parts of the code that might look
weird initially
- Some cosmetic changes
> MAINTAINERS | 1 +
> drivers/media/pci/intel/ipu3/Kconfig | 18 +
> drivers/media/pci/intel/ipu3/Makefile | 1 +
> drivers/media/pci/intel/ipu3/cio2-bridge.c | 311 ++++++++++++++++++
> drivers/media/pci/intel/ipu3/cio2-bridge.h | 125 +++++++
> drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 34 ++
> drivers/media/pci/intel/ipu3/ipu3-cio2.h | 6 +
> 7 files changed, 496 insertions(+)
> create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.c
> create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 92228e8dd868..a091b496fdd8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9014,6 +9014,7 @@ INTEL IPU3 CSI-2 CIO2 DRIVER
> M: Yong Zhi <[email protected]>
> M: Sakari Ailus <[email protected]>
> M: Bingbu Cao <[email protected]>
> +M: Dan Scally <[email protected]>
> R: Tianshu Qiu <[email protected]>
> L: [email protected]
> S: Maintained
> diff --git a/drivers/media/pci/intel/ipu3/Kconfig b/drivers/media/pci/intel/ipu3/Kconfig
> index 82d7f17e6a02..96a2231b16ad 100644
> --- a/drivers/media/pci/intel/ipu3/Kconfig
> +++ b/drivers/media/pci/intel/ipu3/Kconfig
> @@ -16,3 +16,21 @@ config VIDEO_IPU3_CIO2
> Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
> connected camera.
> The module will be called ipu3-cio2.
> +
> +config CIO2_BRIDGE
> + bool "IPU3 CIO2 Sensors Bridge"
> + depends on VIDEO_IPU3_CIO2
> + help
> + This extension provides an API for the ipu3-cio2 driver to create
> + connections to cameras that are hidden in the SSDB buffer in ACPI.
> + It can be used to enable support for cameras in detachable / hybrid
> + devices that ship with Windows.
> +
> + Say Y here if your device is a detachable / hybrid laptop that comes
> + with Windows installed by the OEM, for example:
> +
> + - Microsoft Surface models (except Surface Pro 3)
> + - The Lenovo Miix line (for example the 510, 520, 710 and 720)
> + - Dell 7285
> +
> + If in doubt, say N here.
> diff --git a/drivers/media/pci/intel/ipu3/Makefile b/drivers/media/pci/intel/ipu3/Makefile
> index 429d516452e4..933777e6ea8a 100644
> --- a/drivers/media/pci/intel/ipu3/Makefile
> +++ b/drivers/media/pci/intel/ipu3/Makefile
> @@ -2,3 +2,4 @@
> obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
>
> ipu3-cio2-y += ipu3-cio2-main.o
> +ipu3-cio2-$(CONFIG_CIO2_BRIDGE) += cio2-bridge.o
> diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.c b/drivers/media/pci/intel/ipu3/cio2-bridge.c
> new file mode 100644
> index 000000000000..143f3c0f445e
> --- /dev/null
> +++ b/drivers/media/pci/intel/ipu3/cio2-bridge.c
> @@ -0,0 +1,311 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Author: Dan Scally <[email protected]> */
> +
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/pci.h>
> +#include <linux/property.h>
> +#include <media/v4l2-fwnode.h>
> +
> +#include "cio2-bridge.h"
> +
> +/*
> + * Extend this array with ACPI Hardware IDs of devices known to be working
> + * plus the number of link-frequencies expected by their drivers, along with
> + * the frequency values in hertz. This is somewhat opportunistic way of adding
> + * support for this for now in the hopes of a better source for the information
> + * (possibly some encoded value in the SSDB buffer that we're unaware of)
> + * becoming apparent in the future.
> + *
> + * Do not add an entry for a sensor that is not actually supported.
> + */
> +static const struct cio2_sensor_config cio2_supported_sensors[] = {
> + /* Omnivision OV5693 */
> + CIO2_SENSOR_CONFIG("INT33BE", 0),
> + /* Omnivision OV2680 */
> + CIO2_SENSOR_CONFIG("OVTI2680", 0),
> +};
> +
> +static const struct cio2_property_names prop_names = {
> + .clock_frequency = "clock-frequency",
> + .rotation = "rotation",
> + .bus_type = "bus-type",
> + .data_lanes = "data-lanes",
> + .remote_endpoint = "remote-endpoint",
> + .link_frequencies = "link-frequencies",
> +};
> +
> +static int cio2_bridge_read_acpi_buffer(struct acpi_device *adev, char *id,
> + void *data, u32 size)
> +{
> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> + union acpi_object *obj;
> + acpi_status status;
> + int ret = 0;
> +
> + status = acpi_evaluate_object(adev->handle, id, NULL, &buffer);
> + if (ACPI_FAILURE(status))
> + return -ENODEV;
> +
> + obj = buffer.pointer;
> + if (!obj) {
> + dev_err(&adev->dev, "Couldn't locate ACPI buffer\n");
> + return -ENODEV;
> + }
> +
> + if (obj->type != ACPI_TYPE_BUFFER) {
> + dev_err(&adev->dev, "Not an ACPI buffer\n");
> + ret = -ENODEV;
> + goto out_free_buff;
> + }
> +
> + if (obj->buffer.length > size) {
> + dev_err(&adev->dev, "Given buffer is too small\n");
> + ret = -EINVAL;
> + goto out_free_buff;
> + }
> +
> + memcpy(data, obj->buffer.pointer, obj->buffer.length);
> +
> +out_free_buff:
> + kfree(buffer.pointer);
> + return ret;
> +}
> +
> +static void cio2_bridge_create_fwnode_properties(
> + struct cio2_sensor *sensor,
> + struct cio2_bridge *bridge,
> + const struct cio2_sensor_config *cfg)
> +{
> + sensor->prop_names = prop_names;
> +
> + sensor->local_ref[0].node = &sensor->swnodes[SWNODE_CIO2_ENDPOINT];
> + sensor->remote_ref[0].node = &sensor->swnodes[SWNODE_SENSOR_ENDPOINT];
> +
> + sensor->dev_properties[0] = PROPERTY_ENTRY_U32(
> + sensor->prop_names.clock_frequency,
> + sensor->ssdb.mclkspeed);
> + sensor->dev_properties[1] = PROPERTY_ENTRY_U8(
> + sensor->prop_names.rotation,
> + sensor->ssdb.degree);
> +
> + sensor->ep_properties[0] = PROPERTY_ENTRY_U32(
> + sensor->prop_names.bus_type,
> + V4L2_FWNODE_BUS_TYPE_CSI2_DPHY);
> + sensor->ep_properties[1] = PROPERTY_ENTRY_U32_ARRAY_LEN(
> + sensor->prop_names.data_lanes,
> + bridge->data_lanes,
> + sensor->ssdb.lanes);
> + sensor->ep_properties[2] = PROPERTY_ENTRY_REF_ARRAY(
> + sensor->prop_names.remote_endpoint,
> + sensor->local_ref);
> +
> + if (cfg->nr_link_freqs > 0)
> + sensor->ep_properties[3] = PROPERTY_ENTRY_U64_ARRAY_LEN(
> + sensor->prop_names.link_frequencies,
> + cfg->link_freqs,
> + cfg->nr_link_freqs);
> +
> + sensor->cio2_properties[0] = PROPERTY_ENTRY_U32_ARRAY_LEN(
> + sensor->prop_names.data_lanes,
> + bridge->data_lanes,
> + sensor->ssdb.lanes);
> + sensor->cio2_properties[1] = PROPERTY_ENTRY_REF_ARRAY(
> + sensor->prop_names.remote_endpoint,
> + sensor->remote_ref);
> +}
> +
> +static void cio2_bridge_init_swnode_names(struct cio2_sensor *sensor)
> +{
> + snprintf(sensor->node_names.remote_port,
> + sizeof(sensor->node_names.remote_port),
> + SWNODE_GRAPH_PORT_NAME_FMT, sensor->ssdb.link);
> + snprintf(sensor->node_names.port,
> + sizeof(sensor->node_names.port),
> + SWNODE_GRAPH_PORT_NAME_FMT, 0); /* Always port 0 */
> + snprintf(sensor->node_names.endpoint,
> + sizeof(sensor->node_names.endpoint),
> + SWNODE_GRAPH_ENDPOINT_NAME_FMT, 0); /* And endpoint 0 */
> +}
> +
> +static void cio2_bridge_create_connection_swnodes(struct cio2_bridge *bridge,
> + struct cio2_sensor *sensor)
> +{
> + struct software_node *nodes = sensor->swnodes;
> +
> + cio2_bridge_init_swnode_names(sensor);
> +
> + nodes[SWNODE_SENSOR_HID] = NODE_SENSOR(sensor->name,
> + sensor->dev_properties);
> + nodes[SWNODE_SENSOR_PORT] = NODE_PORT(sensor->node_names.port,
> + &nodes[SWNODE_SENSOR_HID]);
> + nodes[SWNODE_SENSOR_ENDPOINT] = NODE_ENDPOINT(
> + sensor->node_names.endpoint,
> + &nodes[SWNODE_SENSOR_PORT],
> + sensor->ep_properties);
> + nodes[SWNODE_CIO2_PORT] = NODE_PORT(sensor->node_names.remote_port,
> + &bridge->cio2_hid_node);
> + nodes[SWNODE_CIO2_ENDPOINT] = NODE_ENDPOINT(
> + sensor->node_names.endpoint,
> + &nodes[SWNODE_CIO2_PORT],
> + sensor->cio2_properties);
> +}
> +
> +static void cio2_bridge_unregister_sensors(struct cio2_bridge *bridge)
> +{
> + struct cio2_sensor *sensor;
> + unsigned int i;
> +
> + for (i = 0; i < bridge->n_sensors; i++) {
> + sensor = &bridge->sensors[i];
> + software_node_unregister_nodes(sensor->swnodes);
> + acpi_dev_put(sensor->adev);
> + }
> +}
> +
> +static int cio2_bridge_connect_sensor(const struct cio2_sensor_config *cfg,
> + struct cio2_bridge *bridge,
> + struct pci_dev *cio2)
> +{
> + struct fwnode_handle *fwnode;
> + struct cio2_sensor *sensor;
> + struct acpi_device *adev;
> + int ret;
> +
> + for_each_acpi_dev_match(adev, cfg->hid, NULL, -1) {
> + if (!adev->status.enabled)
> + continue;
> +
> + if (bridge->n_sensors >= CIO2_NUM_PORTS) {
> + dev_err(&cio2->dev, "Exceeded available CIO2 ports\n");
> + cio2_bridge_unregister_sensors(bridge);
> + ret = -EINVAL;
> + goto err_out;
> + }
> +
> + sensor = &bridge->sensors[bridge->n_sensors];
> + sensor->adev = adev;
> + strscpy(sensor->name, cfg->hid, sizeof(sensor->name));
> +
> + ret = cio2_bridge_read_acpi_buffer(adev, "SSDB",
> + &sensor->ssdb,
> + sizeof(sensor->ssdb));
> + if (ret)
> + goto err_put_adev;
> +
> + if (sensor->ssdb.lanes > CIO2_MAX_LANES) {
> + dev_err(&adev->dev,
> + "Number of lanes in SSDB is invalid\n");
> + ret = -EINVAL;
> + goto err_put_adev;
> + }
> +
> + cio2_bridge_create_fwnode_properties(sensor, bridge, cfg);
> + cio2_bridge_create_connection_swnodes(bridge, sensor);
> +
> + ret = software_node_register_nodes(sensor->swnodes);
> + if (ret)
> + goto err_put_adev;
> +
> + fwnode = software_node_fwnode(&sensor->swnodes[SWNODE_SENSOR_HID]);
> + if (!fwnode) {
> + ret = -ENODEV;
> + goto err_free_swnodes;
> + }
> +
> + adev->fwnode.secondary = fwnode;
> +
> + dev_info(&cio2->dev, "Found supported sensor %s\n",
> + acpi_dev_name(adev));
> +
> + bridge->n_sensors++;
> + }
> +
> + return 0;
> +
> +err_free_swnodes:
> + software_node_unregister_nodes(sensor->swnodes);
> +err_put_adev:
> + acpi_dev_put(sensor->adev);
> +err_out:
> + return ret;
> +}
> +
> +static int cio2_bridge_connect_sensors(struct cio2_bridge *bridge,
> + struct pci_dev *cio2)
> +{
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < ARRAY_SIZE(cio2_supported_sensors); i++) {
> + const struct cio2_sensor_config *cfg = &cio2_supported_sensors[i];
> +
> + ret = cio2_bridge_connect_sensor(cfg, bridge, cio2);
> + if (ret)
> + goto err_unregister_sensors;
> + }
> +
> + return 0;
> +
> +err_unregister_sensors:
> + cio2_bridge_unregister_sensors(bridge);
> + return ret;
> +}
> +
> +int cio2_bridge_init(struct pci_dev *cio2)
> +{
> + struct device *dev = &cio2->dev;
> + struct fwnode_handle *fwnode;
> + struct cio2_bridge *bridge;
> + unsigned int i;
> + int ret;
> +
> + bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
> + if (!bridge)
> + return -ENOMEM;
> +
> + strscpy(bridge->cio2_node_name, CIO2_HID, sizeof(bridge->cio2_node_name));
> + bridge->cio2_hid_node.name = bridge->cio2_node_name;
> +
> + ret = software_node_register(&bridge->cio2_hid_node);
> + if (ret < 0) {
> + dev_err(dev, "Failed to register the CIO2 HID node\n");
> + goto err_free_bridge;
> + }
> +
> + /*
> + * Map the lane arrangement, which is fixed for the IPU3 (meaning we
> + * only need one, rather than one per sensor). We include it as a
> + * member of the struct cio2_bridge rather than a global variable so
> + * that it survives if the module is unloaded along with the rest of
> + * the struct.
> + */
> + for (i = 0; i < CIO2_MAX_LANES; i++)
> + bridge->data_lanes[i] = i + 1;
> +
> + ret = cio2_bridge_connect_sensors(bridge, cio2);
> + if (ret || bridge->n_sensors == 0)
> + goto err_unregister_cio2;
> +
> + dev_info(dev, "Connected %d cameras\n", bridge->n_sensors);
> +
> + fwnode = software_node_fwnode(&bridge->cio2_hid_node);
> + if (!fwnode) {
> + dev_err(dev, "Error getting fwnode from cio2 software_node\n");
> + ret = -ENODEV;
> + goto err_unregister_sensors;
> + }
> +
> + set_secondary_fwnode(dev, fwnode);
> +
> + return 0;
> +
> +err_unregister_sensors:
> + cio2_bridge_unregister_sensors(bridge);
> +err_unregister_cio2:
> + software_node_unregister(&bridge->cio2_hid_node);
> +err_free_bridge:
> + kfree(bridge);
> +
> + return ret;
> +}
> diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.h b/drivers/media/pci/intel/ipu3/cio2-bridge.h
> new file mode 100644
> index 000000000000..dd0ffcafa489
> --- /dev/null
> +++ b/drivers/media/pci/intel/ipu3/cio2-bridge.h
> @@ -0,0 +1,125 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Author: Dan Scally <[email protected]> */
> +#ifndef __CIO2_BRIDGE_H
> +#define __CIO2_BRIDGE_H
> +
> +#include <linux/property.h>
> +#include <linux/types.h>
> +
> +#include "ipu3-cio2.h"
> +
> +#define CIO2_HID "INT343E"
> +#define CIO2_MAX_LANES 4
> +#define MAX_NUM_LINK_FREQS 3
> +
> +#define CIO2_SENSOR_CONFIG(_HID, _NR, ...) \
> + (const struct cio2_sensor_config) { \
> + .hid = _HID, \
> + .nr_link_freqs = _NR, \
> + .link_freqs = { __VA_ARGS__ } \
> + }
> +
> +#define NODE_SENSOR(_HID, _PROPS) \
> + (const struct software_node) { \
> + .name = _HID, \
> + .properties = _PROPS, \
> + }
> +
> +#define NODE_PORT(_PORT, _SENSOR_NODE) \
> + (const struct software_node) { \
> + .name = _PORT, \
> + .parent = _SENSOR_NODE, \
> + }
> +
> +#define NODE_ENDPOINT(_EP, _PORT, _PROPS) \
> + (const struct software_node) { \
> + .name = _EP, \
> + .parent = _PORT, \
> + .properties = _PROPS, \
> + }
> +
> +enum cio2_sensor_swnodes {
> + SWNODE_SENSOR_HID,
> + SWNODE_SENSOR_PORT,
> + SWNODE_SENSOR_ENDPOINT,
> + SWNODE_CIO2_PORT,
> + SWNODE_CIO2_ENDPOINT,
> + SWNODE_COUNT
> +};
> +
> +/* Data representation as it is in ACPI SSDB buffer */
> +struct cio2_sensor_ssdb {
> + u8 version;
> + u8 sku;
> + u8 guid_csi2[16];
> + u8 devfunction;
> + u8 bus;
> + u32 dphylinkenfuses;
> + u32 clockdiv;
> + u8 link;
> + u8 lanes;
> + u32 csiparams[10];
> + u32 maxlanespeed;
> + u8 sensorcalibfileidx;
> + u8 sensorcalibfileidxInMBZ[3];
> + u8 romtype;
> + u8 vcmtype;
> + u8 platforminfo;
> + u8 platformsubinfo;
> + u8 flash;
> + u8 privacyled;
> + u8 degree;
> + u8 mipilinkdefined;
> + u32 mclkspeed;
> + u8 controllogicid;
> + u8 reserved1[3];
> + u8 mclkport;
> + u8 reserved2[13];
> +} __packed;
> +
> +struct cio2_property_names {
> + char clock_frequency[16];
> + char rotation[9];
> + char bus_type[9];
> + char data_lanes[11];
> + char remote_endpoint[16];
> + char link_frequencies[17];
> +};
> +
> +struct cio2_node_names {
> + char port[7];
> + char endpoint[11];
> + char remote_port[7];
> +};
> +
> +struct cio2_sensor_config {
> + const char *hid;
> + const u8 nr_link_freqs;
> + const u64 link_freqs[MAX_NUM_LINK_FREQS];
> +};
> +
> +struct cio2_sensor {
> + char name[ACPI_ID_LEN];
> + struct acpi_device *adev;
> +
> + struct software_node swnodes[6];
> + struct cio2_node_names node_names;
> +
> + struct cio2_sensor_ssdb ssdb;
> + struct cio2_property_names prop_names;
> + struct property_entry ep_properties[5];
> + struct property_entry dev_properties[3];
> + struct property_entry cio2_properties[3];
> + struct software_node_ref_args local_ref[1];
> + struct software_node_ref_args remote_ref[1];
> +};
> +
> +struct cio2_bridge {
> + char cio2_node_name[ACPI_ID_LEN];
> + struct software_node cio2_hid_node;
> + u32 data_lanes[4];
> + unsigned int n_sensors;
> + struct cio2_sensor sensors[CIO2_NUM_PORTS];
> +};
> +
> +#endif
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> index 36e354ecf71e..db59b19a6236 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> @@ -1702,11 +1702,28 @@ static void cio2_queues_exit(struct cio2_device *cio2)
> cio2_queue_exit(cio2, &cio2->queue[i]);
> }
>
> +static int cio2_check_fwnode_graph(struct fwnode_handle *fwnode)
> +{
> + struct fwnode_handle *endpoint;
> +
> + if (IS_ERR_OR_NULL(fwnode))
> + return -EINVAL;
> +
> + endpoint = fwnode_graph_get_next_endpoint(fwnode, NULL);
> + if (endpoint) {
> + fwnode_handle_put(endpoint);
> + return 0;
> + }
> +
> + return cio2_check_fwnode_graph(fwnode->secondary);
> +}
> +
> /**************** PCI interface ****************/
>
> static int cio2_pci_probe(struct pci_dev *pci_dev,
> const struct pci_device_id *id)
> {
> + struct fwnode_handle *fwnode = dev_fwnode(&pci_dev->dev);
> struct cio2_device *cio2;
> int r;
>
> @@ -1715,6 +1732,23 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
> return -ENOMEM;
> cio2->pci_dev = pci_dev;
>
> + /*
> + * On some platforms no connections to sensors are defined in firmware,
> + * if the device has no endpoints then we can try to build those as
> + * software_nodes parsed from SSDB.
> + */
> + r = cio2_check_fwnode_graph(fwnode);
> + if (r) {
> + if (fwnode && !IS_ERR_OR_NULL(fwnode->secondary)) {
> + dev_err(&pci_dev->dev, "fwnode graph has no endpoints connected\n");
> + return -EINVAL;
> + }
> +
> + r = cio2_bridge_init(pci_dev);
> + if (r)
> + return r;
> + }
> +
> r = pcim_enable_device(pci_dev);
> if (r) {
> dev_err(&pci_dev->dev, "failed to enable device (%d)\n", r);
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.h b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
> index 62187ab5ae43..dc3e343a37fb 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.h
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
> @@ -455,4 +455,10 @@ static inline struct cio2_queue *vb2q_to_cio2_queue(struct vb2_queue *vq)
> return container_of(vq, struct cio2_queue, vbq);
> }
>
> +#if IS_ENABLED(CONFIG_CIO2_BRIDGE)
> +int cio2_bridge_init(struct pci_dev *cio2);
> +#else
> +int cio2_bridge_init(struct pci_dev *cio2) { return 0; }
> +#endif
> +
> #endif
>
Hi Rafael,
Could you please review this patch, and let us know (see question in the
cover letter) if it can be merged through the linux-media tree for v5.12
?
On Thu, Jan 07, 2021 at 01:28:36PM +0000, Daniel Scally wrote:
> To ensure we handle situations in which multiple sensors of the same
> model (and therefore _HID) are present in a system, we need to be able
> to iterate over devices matching a known _HID but unknown _UID and _HRV
> - add acpi_dev_get_next_match_dev() to accommodate that possibility and
> change acpi_dev_get_first_match_dev() to simply call the new function
> with a NULL starting point. Add an iterator macro for convenience.
>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Sakari Ailus <[email protected]>
> Suggested-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
> ---
> Changes in v5:
>
> - Changed commit subject
>
> drivers/acpi/utils.c | 30 ++++++++++++++++++++++++++----
> include/acpi/acpi_bus.h | 7 +++++++
> 2 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
> index d5411a166685..ddca1550cce6 100644
> --- a/drivers/acpi/utils.c
> +++ b/drivers/acpi/utils.c
> @@ -843,12 +843,13 @@ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
> EXPORT_SYMBOL(acpi_dev_present);
>
> /**
> - * acpi_dev_get_first_match_dev - Return the first match of ACPI device
> + * acpi_dev_get_next_match_dev - Return the next match of ACPI device
> + * @adev: Pointer to the previous acpi_device matching this @hid, @uid and @hrv
> * @hid: Hardware ID of the device.
> * @uid: Unique ID of the device, pass NULL to not check _UID
> * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
> *
> - * Return the first match of ACPI device if a matching device was present
> + * Return the next match of ACPI device if another matching device was present
> * at the moment of invocation, or NULL otherwise.
> *
> * The caller is responsible to call put_device() on the returned device.
> @@ -856,8 +857,9 @@ EXPORT_SYMBOL(acpi_dev_present);
> * See additional information in acpi_dev_present() as well.
> */
> struct acpi_device *
> -acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
> +acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv)
> {
> + struct device *start = adev ? &adev->dev : NULL;
> struct acpi_dev_match_info match = {};
> struct device *dev;
>
> @@ -865,9 +867,29 @@ acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
> match.uid = uid;
> match.hrv = hrv;
>
> - dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
> + dev = bus_find_device(&acpi_bus_type, start, &match, acpi_dev_match_cb);
> return dev ? to_acpi_device(dev) : NULL;
> }
> +EXPORT_SYMBOL(acpi_dev_get_next_match_dev);
> +
> +/**
> + * acpi_dev_get_first_match_dev - Return the first match of ACPI device
> + * @hid: Hardware ID of the device.
> + * @uid: Unique ID of the device, pass NULL to not check _UID
> + * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
> + *
> + * Return the first match of ACPI device if a matching device was present
> + * at the moment of invocation, or NULL otherwise.
> + *
> + * The caller is responsible to call put_device() on the returned device.
> + *
> + * See additional information in acpi_dev_present() as well.
> + */
> +struct acpi_device *
> +acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
> +{
> + return acpi_dev_get_next_match_dev(NULL, hid, uid, hrv);
> +}
> EXPORT_SYMBOL(acpi_dev_get_first_match_dev);
>
> /*
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 6d1879bf9440..02a716a0af5d 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -683,9 +683,16 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
>
> bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2);
>
> +struct acpi_device *
> +acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);
> struct acpi_device *
> acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv);
>
> +#define for_each_acpi_dev_match(adev, hid, uid, hrv) \
> + for (adev = acpi_dev_get_first_match_dev(hid, uid, hrv); \
> + adev; \
> + adev = acpi_dev_get_next_match_dev(adev, hid, uid, hrv))
> +
> static inline void acpi_dev_put(struct acpi_device *adev)
> {
> put_device(&adev->dev);
--
Regards,
Laurent Pinchart
Hi Peter, Steven and Sergey,
Could you please let us know if you're fine with this patch getting
merged in v5.12 through the linux-media tree ? The cover letter contains
additional details (in a nutshell, this is a cross-tree series and we
would like to avoid topic branches if possible).
On Thu, Jan 07, 2021 at 01:28:32PM +0000, Daniel Scally wrote:
> Use the software_node_unregister_nodes() helper function to unwind this
> array in a cleaner way.
>
> Acked-by: Petr Mladek <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Reviewed-by: Sergey Senozhatsky <[email protected]>
> Suggested-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
> ---
> Changes in v5:
>
> - None
>
> lib/test_printf.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 7ac87f18a10f..7d60f24240a4 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -644,9 +644,7 @@ static void __init fwnode_pointer(void)
> test(second_name, "%pfwP", software_node_fwnode(&softnodes[1]));
> test(third_name, "%pfwP", software_node_fwnode(&softnodes[2]));
>
> - software_node_unregister(&softnodes[2]);
> - software_node_unregister(&softnodes[1]);
> - software_node_unregister(&softnodes[0]);
> + software_node_unregister_nodes(softnodes);
> }
>
> static void __init
--
Regards,
Laurent Pinchart
Hi Rafael,
Could you please let us know with an Acked-by if this patch can be
merged through the linux-media tree for v5.12 ? The cover letter
contains additional details (in a nutshell, this is a cross-tree series
and we would like to avoid topic branches).
On Thu, Jan 07, 2021 at 01:28:30PM +0000, Daniel Scally wrote:
> OF, ACPI and software_nodes all implement graphs including nodes for ports
> and endpoints. These are all intended to be named with a common schema,
> as "port@n" and "endpoint@n" where n is an unsigned int representing the
> index of the node. To ensure commonality across the subsystems, provide a
> set of macros to define the format.
>
> Suggested-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
> ---
> Changes in v5:
>
> - Changed commit subject
>
> include/linux/fwnode.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index fde4ad97564c..77414e431e89 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -50,6 +50,13 @@ struct fwnode_endpoint {
> const struct fwnode_handle *local_fwnode;
> };
>
> +/*
> + * ports and endpoints defined as software_nodes should all follow a common
> + * naming scheme; use these macros to ensure commonality.
> + */
> +#define SWNODE_GRAPH_PORT_NAME_FMT "port@%u"
> +#define SWNODE_GRAPH_ENDPOINT_NAME_FMT "endpoint@%u"
> +
> #define NR_FWNODE_REFERENCE_ARGS 8
>
> /**
--
Regards,
Laurent Pinchart
Hi Greg,
On Thu, Jan 07, 2021 at 01:28:23PM +0000, Daniel Scally wrote:
>
> Hello all
>
> v4:
> https://lore.kernel.org/linux-media/[email protected]/T/#m11b7cb977e1b73fba1e625c3d6a189e2943a7783
> v3:
> https://lore.kernel.org/linux-media/[email protected]/T/#m37b831bb2b406917d6db5da9acf9ed35df65d72d
> v2:
> https://lore.kernel.org/linux-media/[email protected]/T/#md93fd090009b42a6a98aed892aff0d38cf07e0cd
> v1:
> https://lore.kernel.org/linux-media/[email protected]/T/#m91934e12e3d033da2e768e952ea3b4a125ee3e67
>
> This series is to start adding support for webcams on laptops with ACPI tables
> designed for use with CIO2 on Windows. This series extends the ipu3-cio2
> driver to allow for patching the firmware via software_nodes if endpoints
> aren't defined by ACPI.
>
> I'm hopeful that most or all of this series could get picked up for 5.12.
> We touch a few different areas (listed below), but I think the easiest
> approach would be to merge everything through media tree. Rafael, Greg,
> Mauro and Sergey; are you ok with that plan, or would you prefer a
> different approach? Mauro; if that plan is ok (and of course assuming that
> the rest of the patches are acked by their respective maintainers) could
> we get a dedicated feature branch just in case the following series ends
> up being ready in time too?
>
> lib
> lib/test_printf.c: Use helper function to unwind array of
> software_nodes
>
> base
> software_node: Fix refcounts in software_node_get_next_child()
> property: Return true in fwnode_device_is_available for NULL ops
> property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary
> software_node: Enforce parent before child ordering of nodes arrays
> software_node: unregister software_nodes in reverse order
Could you please let us know with an Acked-by if these patches can be
merged through the linux-media tree for v5.12 ? This is a cross-tree
series and we would like to avoid topic branches if possible.
> include: fwnode.h: Define format macros for ports and endpoints
>
> acpi
> acpi: Add acpi_dev_get_next_match_dev() and helper macro
>
> media
> media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
> match_fwnode()
> ipu3-cio2: Add T: entry to MAINTAINERS
> ipu3-cio2: Rename ipu3-cio2.c
> ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver
> include: media: v4l2-fwnode: Include v4l2_fwnode_bus_type
>
> Series-level changelog:
> - Rebased onto 5.11-rc1
>
> Thanks
> Dan
>
> Andy Shevchenko (1):
> media: ipu3-cio2: Add headers that ipu3-cio2.h is direct user of
>
> Daniel Scally (13):
> software_node: Fix refcounts in software_node_get_next_child()
> device property: Return true in fwnode_device_is_available for NULL
> ops
> device property: Call fwnode_graph_get_endpoint_by_id() for
> fwnode->secondary
> software_node: Enforce parent before child ordering of nodes arrays
> software_node: unregister software_nodes in reverse order
> device property: Define format macros for ports and endpoints
> lib/test_printf.c: Use helper function to unwind array of
> software_nodes
> ipu3-cio2: Add T: entry to MAINTAINERS
> ipu3-cio2: Rename ipu3-cio2.c
> media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
> match_fwnode()
> ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro
> media: v4l2-fwnode: Include v4l2_fwnode_bus_type
> ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver
>
> Heikki Krogerus (1):
> software_node: Add support for fwnode_graph*() family of functions
>
> MAINTAINERS | 2 +
> drivers/acpi/utils.c | 30 +-
> drivers/base/property.c | 15 +-
> drivers/base/swnode.c | 180 ++++++++--
> drivers/media/pci/intel/ipu3/Kconfig | 18 +
> drivers/media/pci/intel/ipu3/Makefile | 3 +
> drivers/media/pci/intel/ipu3/cio2-bridge.c | 311 ++++++++++++++++++
> drivers/media/pci/intel/ipu3/cio2-bridge.h | 125 +++++++
> .../ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} | 34 ++
> drivers/media/pci/intel/ipu3/ipu3-cio2.h | 24 ++
> drivers/media/v4l2-core/v4l2-async.c | 8 +
> drivers/media/v4l2-core/v4l2-fwnode.c | 11 -
> include/acpi/acpi_bus.h | 7 +
> include/linux/fwnode.h | 7 +
> include/media/v4l2-fwnode.h | 22 ++
> lib/test_printf.c | 4 +-
> 16 files changed, 763 insertions(+), 38 deletions(-)
> create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.c
> create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.h
> rename drivers/media/pci/intel/ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} (98%)
--
Regards,
Laurent Pinchart
Hi Andy,
On Sat, Jan 09, 2021 at 11:07:33AM +0200, Andy Shevchenko wrote:
> On Saturday, January 9, 2021, Laurent Pinchart wrote:
>
> > Hi Peter, Steven and Sergey,
> >
> > Could you please let us know if you're fine with this patch getting
> > merged in v5.12 through the linux-media tree ? The cover letter contains
> > additional details (in a nutshell, this is a cross-tree series and we
> > would like to avoid topic branches if possible).
>
> There is already a tag by Petr.
I saw that, but looking at the corresponding e-mail, there was no clear
acknowledgement that we could merge this patch through a different tree.
> > On Thu, Jan 07, 2021 at 01:28:32PM +0000, Daniel Scally wrote:
> > > Use the software_node_unregister_nodes() helper function to unwind this
> > > array in a cleaner way.
> > >
> > > Acked-by: Petr Mladek <[email protected]>
> > > Reviewed-by: Andy Shevchenko <[email protected]>
> > > Reviewed-by: Laurent Pinchart <[email protected]>
> > > Reviewed-by: Sergey Senozhatsky <[email protected]>
> > > Suggested-by: Andy Shevchenko <[email protected]>
> > > Signed-off-by: Daniel Scally <[email protected]>
> > > ---
> > > Changes in v5:
> > >
> > > - None
> > >
> > > lib/test_printf.c | 4 +---
> > > 1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/lib/test_printf.c b/lib/test_printf.c
> > > index 7ac87f18a10f..7d60f24240a4 100644
> > > --- a/lib/test_printf.c
> > > +++ b/lib/test_printf.c
> > > @@ -644,9 +644,7 @@ static void __init fwnode_pointer(void)
> > > test(second_name, "%pfwP", software_node_fwnode(&softnodes[1]));
> > > test(third_name, "%pfwP", software_node_fwnode(&softnodes[2]));
> > >
> > > - software_node_unregister(&softnodes[2]);
> > > - software_node_unregister(&softnodes[1]);
> > > - software_node_unregister(&softnodes[0]);
> > > + software_node_unregister_nodes(softnodes);
> > > }
> > >
> > > static void __init
--
Regards,
Laurent Pinchart
On Sat, Jan 09, 2021 at 04:08:04AM +0200, Laurent Pinchart wrote:
> Hi Greg,
>
> On Thu, Jan 07, 2021 at 01:28:23PM +0000, Daniel Scally wrote:
> >
> > Hello all
> >
> > v4:
> > https://lore.kernel.org/linux-media/[email protected]/T/#m11b7cb977e1b73fba1e625c3d6a189e2943a7783
> > v3:
> > https://lore.kernel.org/linux-media/[email protected]/T/#m37b831bb2b406917d6db5da9acf9ed35df65d72d
> > v2:
> > https://lore.kernel.org/linux-media/[email protected]/T/#md93fd090009b42a6a98aed892aff0d38cf07e0cd
> > v1:
> > https://lore.kernel.org/linux-media/[email protected]/T/#m91934e12e3d033da2e768e952ea3b4a125ee3e67
> >
> > This series is to start adding support for webcams on laptops with ACPI tables
> > designed for use with CIO2 on Windows. This series extends the ipu3-cio2
> > driver to allow for patching the firmware via software_nodes if endpoints
> > aren't defined by ACPI.
> >
> > I'm hopeful that most or all of this series could get picked up for 5.12.
> > We touch a few different areas (listed below), but I think the easiest
> > approach would be to merge everything through media tree. Rafael, Greg,
> > Mauro and Sergey; are you ok with that plan, or would you prefer a
> > different approach? Mauro; if that plan is ok (and of course assuming that
> > the rest of the patches are acked by their respective maintainers) could
> > we get a dedicated feature branch just in case the following series ends
> > up being ready in time too?
> >
> > lib
> > lib/test_printf.c: Use helper function to unwind array of
> > software_nodes
> >
> > base
> > software_node: Fix refcounts in software_node_get_next_child()
> > property: Return true in fwnode_device_is_available for NULL ops
> > property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary
> > software_node: Enforce parent before child ordering of nodes arrays
> > software_node: unregister software_nodes in reverse order
>
> Could you please let us know with an Acked-by if these patches can be
> merged through the linux-media tree for v5.12 ? This is a cross-tree
> series and we would like to avoid topic branches if possible.
Yes, they are all fine with me, will go provide my ack, sorry for the
delay.
greg k-h
On Thu, Jan 07, 2021 at 01:28:29PM +0000, Daniel Scally wrote:
> To maintain consistency with software_node_unregister_nodes(), reverse
> the order in which the software_node_unregister_node_group() function
> unregisters nodes.
>
> Reported-by: kernel test robot <[email protected]>
> Reported-by: Dan Carpenter <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Reviewed-by: Sakari Ailus <[email protected]>
> Suggested-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
On Thu, Jan 07, 2021 at 01:28:31PM +0000, Daniel Scally wrote:
> From: Heikki Krogerus <[email protected]>
>
> This implements the remaining .graph_*() callbacks in the fwnode
> operations structure for the software nodes. That makes the
> fwnode_graph_*() functions available in the drivers also when software
> nodes are used.
>
> The implementation tries to mimic the "OF graph" as much as possible, but
> there is no support for the "reg" device property. The ports will need to
> have the index in their name which starts with "port@" (for example
> "port@0", "port@1", ...) and endpoints will use the index of the software
> node that is given to them during creation. The port nodes can also be
> grouped under a specially named "ports" subnode, just like in DT, if
> necessary.
>
> The remote-endpoints are reference properties under the endpoint nodes
> that are named "remote-endpoint".
>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Heikki Krogerus <[email protected]>
> Co-developed-by: Daniel Scally <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
> ---
> Changes in v5:
>
> - Cosmetic changes only
Acked-by: Greg Kroah-Hartman <[email protected]>
On Thu, Jan 07, 2021 at 01:28:28PM +0000, Daniel Scally wrote:
> Registering software_nodes with the .parent member set to point to a
> currently unregistered software_node has the potential for problems,
> so enforce parent -> child ordering in arrays passed in to
> software_node_register_nodes().
>
> Software nodes that are children of another software node should be
> unregistered before their parent. To allow easy unregistering of an array
> of software_nodes ordered parent to child, reverse the order in which
> software_node_unregister_nodes() unregisters software_nodes.
>
> Suggested-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
> ---
> Changes in v5:
>
> - None
Acked-by: Greg Kroah-Hartman <[email protected]>
On Sun, Jan 10, 2021 at 1:16 PM Laurent Pinchart
<[email protected]> wrote:
> On Sat, Jan 09, 2021 at 11:07:33AM +0200, Andy Shevchenko wrote:
> > On Saturday, January 9, 2021, Laurent Pinchart wrote:
> > > Could you please let us know if you're fine with this patch getting
> > > merged in v5.12 through the linux-media tree ? The cover letter contains
> > > additional details (in a nutshell, this is a cross-tree series and we
> > > would like to avoid topic branches if possible).
> >
> > There is already a tag by Petr.
>
> I saw that, but looking at the corresponding e-mail, there was no clear
> acknowledgement that we could merge this patch through a different tree.
Fair point.
One question though, what so wrong with topic branches.
--
With Best Regards,
Andy Shevchenko
On Sun, Jan 10, 2021 at 05:38:03PM +0200, Andy Shevchenko wrote:
> On Sun, Jan 10, 2021 at 1:16 PM Laurent Pinchart wrote:
> > On Sat, Jan 09, 2021 at 11:07:33AM +0200, Andy Shevchenko wrote:
> > > On Saturday, January 9, 2021, Laurent Pinchart wrote:
> > > > Could you please let us know if you're fine with this patch getting
> > > > merged in v5.12 through the linux-media tree ? The cover letter contains
> > > > additional details (in a nutshell, this is a cross-tree series and we
> > > > would like to avoid topic branches if possible).
> > >
> > > There is already a tag by Petr.
> >
> > I saw that, but looking at the corresponding e-mail, there was no clear
> > acknowledgement that we could merge this patch through a different tree.
>
> Fair point.
>
> One question though, what so wrong with topic branches.
They're not wrong, they just add more complexity as all maintainers
involved would need to create a topic branch based on v5.11-rc1 and
merge it in their respective tree. It's certainly doable, but when
there's no risk of conflict, merging the whole series through a single
tree is just easier.
--
Regards,
Laurent Pinchart
On Thu, Jan 7, 2021 at 2:31 PM Daniel Scally <[email protected]> wrote:
>
> To ensure we handle situations in which multiple sensors of the same
> model (and therefore _HID) are present in a system, we need to be able
> to iterate over devices matching a known _HID but unknown _UID and _HRV
> - add acpi_dev_get_next_match_dev() to accommodate that possibility and
> change acpi_dev_get_first_match_dev() to simply call the new function
> with a NULL starting point. Add an iterator macro for convenience.
>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Sakari Ailus <[email protected]>
> Suggested-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
> ---
> Changes in v5:
>
> - Changed commit subject
>
> drivers/acpi/utils.c | 30 ++++++++++++++++++++++++++----
> include/acpi/acpi_bus.h | 7 +++++++
> 2 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
> index d5411a166685..ddca1550cce6 100644
> --- a/drivers/acpi/utils.c
> +++ b/drivers/acpi/utils.c
> @@ -843,12 +843,13 @@ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
> EXPORT_SYMBOL(acpi_dev_present);
>
> /**
> - * acpi_dev_get_first_match_dev - Return the first match of ACPI device
> + * acpi_dev_get_next_match_dev - Return the next match of ACPI device
> + * @adev: Pointer to the previous acpi_device matching this @hid, @uid and @hrv
> * @hid: Hardware ID of the device.
> * @uid: Unique ID of the device, pass NULL to not check _UID
> * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
> *
> - * Return the first match of ACPI device if a matching device was present
> + * Return the next match of ACPI device if another matching device was present
> * at the moment of invocation, or NULL otherwise.
> *
> * The caller is responsible to call put_device() on the returned device.
> @@ -856,8 +857,9 @@ EXPORT_SYMBOL(acpi_dev_present);
> * See additional information in acpi_dev_present() as well.
> */
> struct acpi_device *
> -acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
> +acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv)
> {
> + struct device *start = adev ? &adev->dev : NULL;
> struct acpi_dev_match_info match = {};
> struct device *dev;
>
> @@ -865,9 +867,29 @@ acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
> match.uid = uid;
> match.hrv = hrv;
>
> - dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
> + dev = bus_find_device(&acpi_bus_type, start, &match, acpi_dev_match_cb);
> return dev ? to_acpi_device(dev) : NULL;
> }
> +EXPORT_SYMBOL(acpi_dev_get_next_match_dev);
> +
> +/**
> + * acpi_dev_get_first_match_dev - Return the first match of ACPI device
> + * @hid: Hardware ID of the device.
> + * @uid: Unique ID of the device, pass NULL to not check _UID
> + * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
> + *
> + * Return the first match of ACPI device if a matching device was present
> + * at the moment of invocation, or NULL otherwise.
> + *
> + * The caller is responsible to call put_device() on the returned device.
> + *
> + * See additional information in acpi_dev_present() as well.
> + */
> +struct acpi_device *
> +acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
> +{
> + return acpi_dev_get_next_match_dev(NULL, hid, uid, hrv);
> +}
> EXPORT_SYMBOL(acpi_dev_get_first_match_dev);
>
> /*
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 6d1879bf9440..02a716a0af5d 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -683,9 +683,16 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
>
> bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2);
>
> +struct acpi_device *
> +acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);
> struct acpi_device *
> acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv);
>
> +#define for_each_acpi_dev_match(adev, hid, uid, hrv) \
> + for (adev = acpi_dev_get_first_match_dev(hid, uid, hrv); \
> + adev; \
> + adev = acpi_dev_get_next_match_dev(adev, hid, uid, hrv))
> +
> static inline void acpi_dev_put(struct acpi_device *adev)
> {
> put_device(&adev->dev);
> --
> 2.25.1
>
Hi,
On Sat, Jan 9, 2021 at 2:55 AM Laurent Pinchart
<[email protected]> wrote:
>
> Hi Rafael,
>
> Could you please review this patch, and let us know (see question in the
> cover letter)
Done, sorry for the delay.
> if it can be merged through the linux-media tree for v5.12
> ?
Yes, it can.
Thanks!
On Thu, Jan 7, 2021 at 2:31 PM Daniel Scally <[email protected]> wrote:
>
> OF, ACPI and software_nodes all implement graphs including nodes for ports
> and endpoints. These are all intended to be named with a common schema,
> as "port@n" and "endpoint@n" where n is an unsigned int representing the
> index of the node. To ensure commonality across the subsystems, provide a
> set of macros to define the format.
>
> Suggested-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Signed-off-by: Daniel Scally <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
> ---
> Changes in v5:
>
> - Changed commit subject
>
> include/linux/fwnode.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index fde4ad97564c..77414e431e89 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -50,6 +50,13 @@ struct fwnode_endpoint {
> const struct fwnode_handle *local_fwnode;
> };
>
> +/*
> + * ports and endpoints defined as software_nodes should all follow a common
> + * naming scheme; use these macros to ensure commonality.
> + */
> +#define SWNODE_GRAPH_PORT_NAME_FMT "port@%u"
> +#define SWNODE_GRAPH_ENDPOINT_NAME_FMT "endpoint@%u"
> +
> #define NR_FWNODE_REFERENCE_ARGS 8
>
> /**
> --
> 2.25.1
>
On Thu, Jan 7, 2021 at 2:30 PM Daniel Scally <[email protected]> wrote:
>
>
> Hello all
>
> v4:
> https://lore.kernel.org/linux-media/[email protected]/T/#m11b7cb977e1b73fba1e625c3d6a189e2943a7783
> v3:
> https://lore.kernel.org/linux-media/[email protected]/T/#m37b831bb2b406917d6db5da9acf9ed35df65d72d
> v2:
> https://lore.kernel.org/linux-media/[email protected]/T/#md93fd090009b42a6a98aed892aff0d38cf07e0cd
> v1:
> https://lore.kernel.org/linux-media/[email protected]/T/#m91934e12e3d033da2e768e952ea3b4a125ee3e67
>
> This series is to start adding support for webcams on laptops with ACPI tables
> designed for use with CIO2 on Windows. This series extends the ipu3-cio2
> driver to allow for patching the firmware via software_nodes if endpoints
> aren't defined by ACPI.
>
> I'm hopeful that most or all of this series could get picked up for 5.12.
> We touch a few different areas (listed below), but I think the easiest
> approach would be to merge everything through media tree. Rafael, Greg,
> Mauro and Sergey; are you ok with that plan, or would you prefer a
> different approach? Mauro; if that plan is ok (and of course assuming that
> the rest of the patches are acked by their respective maintainers) could
> we get a dedicated feature branch just in case the following series ends
> up being ready in time too?
>
> lib
> lib/test_printf.c: Use helper function to unwind array of
> software_nodes
>
> base
> software_node: Fix refcounts in software_node_get_next_child()
> property: Return true in fwnode_device_is_available for NULL ops
> property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary
> software_node: Enforce parent before child ordering of nodes arrays
> software_node: unregister software_nodes in reverse order
> include: fwnode.h: Define format macros for ports and endpoints
>
> acpi
> acpi: Add acpi_dev_get_next_match_dev() and helper macro
>
> media
> media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
> match_fwnode()
> ipu3-cio2: Add T: entry to MAINTAINERS
> ipu3-cio2: Rename ipu3-cio2.c
> ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver
> include: media: v4l2-fwnode: Include v4l2_fwnode_bus_type
>
> Series-level changelog:
> - Rebased onto 5.11-rc1
>
> Thanks
> Dan
>
> Andy Shevchenko (1):
> media: ipu3-cio2: Add headers that ipu3-cio2.h is direct user of
>
> Daniel Scally (13):
> software_node: Fix refcounts in software_node_get_next_child()
> device property: Return true in fwnode_device_is_available for NULL
> ops
> device property: Call fwnode_graph_get_endpoint_by_id() for
> fwnode->secondary
> software_node: Enforce parent before child ordering of nodes arrays
> software_node: unregister software_nodes in reverse order
> device property: Define format macros for ports and endpoints
> lib/test_printf.c: Use helper function to unwind array of
> software_nodes
> ipu3-cio2: Add T: entry to MAINTAINERS
> ipu3-cio2: Rename ipu3-cio2.c
> media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
> match_fwnode()
> ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro
> media: v4l2-fwnode: Include v4l2_fwnode_bus_type
> ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver
>
> Heikki Krogerus (1):
> software_node: Add support for fwnode_graph*() family of functions
Please feel free to add
Acked-by: Rafael J. Wysocki <[email protected]>
to all of the device properties patches in this series if that helps.
Thanks!
On Sat, Jan 9, 2021 at 3:06 AM Laurent Pinchart
<[email protected]> wrote:
>
> Hi Rafael,
>
> Could you please let us know with an Acked-by
Done, sorry for the delay.
> if this patch can be merged through the linux-media tree for v5.12 ?
Yes, it can.
Thanks!
Hi Rafael, Sakari
On 12/01/2021 19:34, Rafael J. Wysocki wrote:
> <snip>
>> I'm hopeful that most or all of this series could get picked up for 5.12.
>> We touch a few different areas (listed below), but I think the easiest
>> approach would be to merge everything through media tree. Rafael, Greg,
>> Mauro and Sergey; are you ok with that plan, or would you prefer a
>> different approach? Mauro; if that plan is ok (and of course assuming that
>> the rest of the patches are acked by their respective maintainers) could
>> we get a dedicated feature branch just in case the following series ends
>> up being ready in time too?
>>
>> <snip>
> Please feel free to add
>
> Acked-by: Rafael J. Wysocki <[email protected]>
>
> to all of the device properties patches in this series if that helps.
>
> Thanks!
Thanks very much (and Greg too).
Sakari; unless I'm misunderstanding something, I think this series could
be picked up now, right? Would it be ok to do that through your tree? I
think the idea of a dedicated feature branch can be dropped, I won't
have the second series ready in time for this round anyway.
First time doing this, so if I've missed something please let me know!
Hi Daniel,
On Tue, Jan 12, 2021 at 11:35:37PM +0000, Daniel Scally wrote:
> Hi Rafael, Sakari
>
> On 12/01/2021 19:34, Rafael J. Wysocki wrote:
> > <snip>
> >> I'm hopeful that most or all of this series could get picked up for 5.12.
> >> We touch a few different areas (listed below), but I think the easiest
> >> approach would be to merge everything through media tree. Rafael, Greg,
> >> Mauro and Sergey; are you ok with that plan, or would you prefer a
> >> different approach? Mauro; if that plan is ok (and of course assuming that
> >> the rest of the patches are acked by their respective maintainers) could
> >> we get a dedicated feature branch just in case the following series ends
> >> up being ready in time too?
> >>
> >> <snip>
> > Please feel free to add
> >
> > Acked-by: Rafael J. Wysocki <[email protected]>
> >
> > to all of the device properties patches in this series if that helps.
> >
> > Thanks!
>
> Thanks very much (and Greg too).
>
>
> Sakari; unless I'm misunderstanding something, I think this series could
> be picked up now, right? Would it be ok to do that through your tree? I
> think the idea of a dedicated feature branch can be dropped, I won't
> have the second series ready in time for this round anyway.
>
>
> First time doing this, so if I've missed something please let me know!
I think it's ready, indeed. I'll let you know if there are any issues.
--
Kind regards,
Sakari Ailus
Hello all,
On Sat, Jan 09, 2021 at 04:01:05AM +0200, Laurent Pinchart wrote:
> Hi Peter, Steven and Sergey,
>
> Could you please let us know if you're fine with this patch getting
> merged in v5.12 through the linux-media tree ? The cover letter contains
> additional details (in a nutshell, this is a cross-tree series and we
> would like to avoid topic branches if possible).
I'll proceed to merge this patch through the media tree.
Thanks.
--
Kind regards,
Sakari Ailus
Hi Sakari, Mauro,
On Wed, Jan 13, 2021 at 2:26 PM Sakari Ailus
<[email protected]> wrote:
> On Sat, Jan 09, 2021 at 04:01:05AM +0200, Laurent Pinchart wrote:
> > Could you please let us know if you're fine with this patch getting
> > merged in v5.12 through the linux-media tree ? The cover letter contains
> > additional details (in a nutshell, this is a cross-tree series and we
> > would like to avoid topic branches if possible).
>
> I'll proceed to merge this patch through the media tree.
Prefixing this patch with "media:"[1] when committing is confusing.
[1] commit f0328be57568c795 ("media: lib/test_printf.c: Use helper
function to unwind array of software_nodes").
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds