| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0j_Uv=qVo6eFdz-Q7Ne7W7aTf9ThN128Ust+YEQQ9Gy+Q@mail.gmail.com>
Date: Tue, 31 Oct 2023 14:46:51 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>,
Linux ACPI <linux-acpi@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 5/6] ACPI: scan: Extract MIPI DiSco for Imaging data
into swnodes
Hi Sakari,
On Tue, Oct 31, 2023 at 11:33 AM Sakari Ailus
<sakari.ailus@...ux.intel.com> wrote:
>
> Hi Rafael,
>
> On subject:
>
> s/DiSco/DisCo/
Huh
Thanks for catching this!
> On Fri, Oct 20, 2023 at 04:39:27PM +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >
> > Add information extracted from the MIPI DiSco for Imaging device
>
> Ditto.
>
> > properties to software nodes created during the CSI-2 connection graph
> > discovery.
> >
> > Link: https://www.mipi.org/specifications/mipi-di
>
> This URL is broken. The correct URL is:
>
> https://www.mipi.org/specifications/mipi-disco-imaging
OK
> > Co-developed-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
> > Signed-off-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > ---
> > drivers/acpi/internal.h | 1
> > drivers/acpi/mipi-di.c | 240 +++++++++++++++++++++++++++++++++++++++++++++++-
>
> How about mipi-disco.c? I wouldn't mind having mipi-disco-imaging.c either.
Well, mipi-disco-imaging is a bit too long and the meaning of
mipi-disco would not be clear IMO.
What about mipi-disco-img?
[cut]
> > + ret = fwnode_property_count_u8(port_fwnode, "mipi-img-lane-polarities");
> > + if (ret > 0) {
> > + unsigned long mask;
> > + unsigned int i;
> > +
> > + /*
> > + * Total number of lanes here is clock lane + data lanes.
> > + * Require that number to be low enough so they all can be
> > + * covered by the bits in one byte.
> > + */
> > + BUILD_BUG_ON(BITS_PER_TYPE(u8) <= ARRAY_SIZE(port->data_lanes));
> > +
> > + fwnode_property_read_u8_array(port_fwnode,
> > + "mipi-img-lane-polarities",
> > + val, 1);
> > +
> > + for (mask = val[0], i = 0; i < num_lanes + 1; i++)
> > + port->lane_polarities[i] = test_bit(i, &mask);
>
> This works only up to seven lanes.
True, but then ACPI_DEVICE_CSI2_DATA_LANES is defined as 4.
> I guess this can be addressed later in
> case we get devices with 8 lanes. But adding a comment here (or where the
> maximum number of lanes is defined) that this needs to be fixed would be
> prudent to avoid introducing a bug later on.
The comment above talks about this, but I can add one more comment
next to the ACPI_DEVICE_CSI2_DATA_LANES definition.
> > +
> > + port->ep_props[NEXT_PROPERTY(ep_prop_index, EP_LANE_POLARITIES)] =
> > + PROPERTY_ENTRY_U32_ARRAY_LEN("lane-polarities",
> > + port->lane_polarities,
> > + 1 + num_lanes);
> > + } else {
> > + acpi_handle_info(handle, "No lane polarity bytes\n");
> > + }
> > +
> > + swnodes->nodes[ACPI_DEVICE_SWNODE_EP(port_index)] =
> > + SOFTWARE_NODE("endpoint@0", swnodes->ports[port_index].ep_props,
> > + &swnodes->nodes[ACPI_DEVICE_SWNODE_PORT(port_index)]);
> > +
> > + if (port->crs_csi2_local)
> > + init_csi2_port_local(adev, port, port_fwnode, ep_prop_index);
> > +}
> > +
> > +#define MIPI_IMG_PORT_PREFIX "mipi-img-port-"
> > +
> > +static struct fwnode_handle *get_mipi_port_handle(struct fwnode_handle *adev_fwnode,
> > + unsigned int port_nr)
> > +{
> > + char port_name[sizeof(MIPI_IMG_PORT_PREFIX) + 2];
> > +
> > + if (snprintf(port_name, sizeof(port_name), "%s%u",
> > + MIPI_IMG_PORT_PREFIX, port_nr) >= sizeof(port_name))
> > + return NULL;
> > +
> > + return fwnode_get_named_child_node(adev_fwnode, port_name);
> > +}
> > +
> > +static void init_crs_csi2_swnodes(struct crs_csi2 *csi2)
> > +{
> > + struct acpi_buffer buffer = { .length = ACPI_ALLOCATE_BUFFER };
> > + struct acpi_device_software_nodes *swnodes = csi2->swnodes;
> > + acpi_handle handle = csi2->handle;
> > + struct fwnode_handle *adev_fwnode;
> > + struct acpi_device *adev;
> > + acpi_status status;
> > + unsigned int i;
> > + int ret;
> > +
> > + /*
> > + * Bail out if the swnodes are not available (either they have not been
> > + * allocated or they have been assigned to the device already).
> > + */
> > + if (!swnodes)
> > + return;
> > +
> > + adev = acpi_fetch_acpi_dev(handle);
> > + if (!adev)
> > + return;
> > +
> > + adev_fwnode = acpi_fwnode_handle(adev);
> > +
> > + status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
> > + if (ACPI_FAILURE(status)) {
> > + acpi_handle_info(handle, "Unable to get the path name\n");
> > + return;
> > + }
> > +
> > + swnodes->nodes[ACPI_DEVICE_SWNODE_ROOT] =
> > + SOFTWARE_NODE(buffer.pointer, swnodes->dev_props, NULL);
> > +
> > + for (i = 0; i < swnodes->num_ports; i++) {
> > + struct acpi_device_software_node_port *port = &swnodes->ports[i];
> > + struct fwnode_handle *port_fwnode;
> > +
> > + /*
> > + * The MIPI DisCo for Imaging specification defines _DSD device
> > + * properties for providing CSI-2 port parameters that can be
> > + * accessed through the generic device properties framework. To
> > + * access them, it is first necessary to find the data node
> > + * representing the port under the given ACPI device object.
> > + */
> > + port_fwnode = get_mipi_port_handle(adev_fwnode, port->port_nr);
> > + if (!port_fwnode) {
> > + acpi_handle_info(handle,
> > + "MIPI port name too long for port %u\n",
> > + port->port_nr);
> > + continue;
> > + }
> > +
> > + init_csi2_port(adev, swnodes, port, port_fwnode, i);
> > +
> > + fwnode_handle_put(port_fwnode);
> > + }
> > +
> > + ret = software_node_register_node_group(swnodes->nodeptrs);
> > + if (ret < 0) {
> > + acpi_handle_info(handle,
> > + "Unable to register software nodes (%d)\n", ret);
> > + return;
> > + }
> > +
> > + adev->swnodes = swnodes;
> > + adev_fwnode->secondary = software_node_fwnode(swnodes->nodes);
> > +
> > + /*
> > + * Prevents the swnodes from this csi2 entry from being assigned again
> > + * or freed prematurely.
> > + */
> > + csi2->swnodes = NULL;
> > +}
> > +
> > +/**
> > + * acpi_mipi_init_crs_csi2_swnodes - Initialize _CRS CSI-2 software nodes
> > + *
> > + * Use MIPI DiSco for Imaging device properties to finalize the initialization
> > + * of CSI-2 software nodes for all ACPI device objects that have been already
> > + * enumerated.
> > + */
> > +void acpi_mipi_init_crs_csi2_swnodes(void)
> > +{
> > + struct crs_csi2 *csi2, *csi2_tmp;
> > +
> > + list_for_each_entry_safe(csi2, csi2_tmp, &acpi_mipi_crs_csi2_list, entry)
> > + init_crs_csi2_swnodes(csi2);
> > +}
> > +
> > /**
> > * acpi_mipi_crs_csi2_cleanup - Free _CRS CSI-2 temporary data
> > */
> > Index: linux-pm/drivers/acpi/internal.h
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/internal.h
> > +++ linux-pm/drivers/acpi/internal.h
> > @@ -287,6 +287,7 @@ static inline void acpi_init_lpit(void)
> >
> > void acpi_mipi_check_crs_csi2(acpi_handle handle);
> > void acpi_mipi_scan_crs_csi2(void);
> > +void acpi_mipi_init_crs_csi2_swnodes(void);
> > void acpi_mipi_crs_csi2_cleanup(void);
> >
> > #endif /* _ACPI_INTERNAL_H_ */
> > Index: linux-pm/drivers/acpi/scan.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/scan.c
> > +++ linux-pm/drivers/acpi/scan.c
> > @@ -2447,6 +2447,13 @@ static void acpi_scan_postponed_branch(a
> >
> > acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
> > acpi_bus_check_add_2, NULL, NULL, (void **)&adev);
> > +
> > + /*
> > + * Populate the ACPI _CRS CSI-2 software nodes for the ACPI devices that
> > + * have been added above.
> > + */
> > + acpi_mipi_init_crs_csi2_swnodes();
> > +
> > acpi_bus_attach(adev, NULL);
> > }
> >
> > @@ -2516,11 +2523,12 @@ int acpi_bus_scan(acpi_handle handle)
> > return -ENODEV;
> >
> > /*
> > - * Allocate ACPI _CRS CSI-2 software nodes using information extracted
> > + * Set up ACPI _CRS CSI-2 software nodes using information extracted
> > * from the _CRS CSI-2 resource descriptors during the ACPI namespace
> > - * walk above.
> > + * walk above and MIPI DiSco for Imaging device properties.
> > */
> > acpi_mipi_scan_crs_csi2();
> > + acpi_mipi_init_crs_csi2_swnodes();
> >
> > acpi_bus_attach(device, (void *)true);
> >
> > Index: linux-pm/include/acpi/acpi_bus.h
> > ===================================================================
> > --- linux-pm.orig/include/acpi/acpi_bus.h
> > +++ linux-pm/include/acpi/acpi_bus.h
> > @@ -366,10 +366,17 @@ struct acpi_device_data {
> >
> > struct acpi_gpio_mapping;
> >
> > +#define ACPI_DEVICE_SWNODE_ROOT 0
> > +
> > #define ACPI_DEVICE_CSI2_DATA_LANES 4
> >
> > #define ACPI_DEVICE_SWNODE_PORT_NAME_LENGTH 8
> >
> > +enum acpi_device_swnode_dev_props {
> > + ACPI_DEVICE_SWNODE_DEV_NUM_OF,
> > + ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES
> > +};
> > +
> > enum acpi_device_swnode_port_props {
> > ACPI_DEVICE_SWNODE_PORT_REG,
> > ACPI_DEVICE_SWNODE_PORT_NUM_OF,
> > @@ -425,12 +432,14 @@ struct acpi_device_software_node_port {
> >
> > /**
> > * struct acpi_device_software_nodes - Software nodes for an ACPI device
> > + * @dev_props: Device properties.
> > * @nodes: Software nodes for root as well as ports and endpoints.
> > * @nodeprts: Array of software node pointers, for (un)registering them.
> > * @ports: Information related to each port and endpoint within a port.
> > * @num_ports: The number of ports.
> > */
> > struct acpi_device_software_nodes {
> > + struct property_entry dev_props[ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES];
> > struct software_node *nodes;
> > const struct software_node **nodeptrs;
> > struct acpi_device_software_node_port *ports;
> > @@ -455,6 +464,7 @@ struct acpi_device {
> > struct acpi_device_data data;
> > struct acpi_scan_handler *handler;
> > struct acpi_hotplug_context *hp;
> > + struct acpi_device_software_nodes *swnodes;
> > const struct acpi_gpio_mapping *driver_gpios;
> > void *driver_data;
> > struct device dev;
> >
> >
> >
>
> --
> Regards,
>
> Sakari Ailus
>
Powered by blists - more mailing lists