From patchwork Tue Sep 17 08:53:05 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luca Ceresoli X-Patchwork-Id: 829303 Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EA2AA1531DB; Tue, 17 Sep 2024 08:53:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.70.183.193 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726563203; cv=none; b=MHziCfq7wkPdRZG8jQtO+2unfchK9GLeQhUzke768sJsUNUFf/I9jKvtlH2nygIYuDeuQptDq0SNe9ziB277GE+En2mJiK1tj7yZGE4mjtcvXEErm1Zy9/ygZkvfkz4MDgmK73eXQcp1XzhVku7HbNbJ9k5AU6L0JZogsXxZXLA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726563203; c=relaxed/simple; bh=XH+lCK7llcoA290mRx3gP5ZezkRnxv/iDZpXGCT3vsY=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=HYVAf2rn8oUbk6Gdyef0jiTGvlpptorU1HffESyUTFJi3zECv6PsZkyhRhfdh1gqKkDCTBFIpTNxdlvOJ0UGfIN4jRXGxXo0cIpxh91h2k7xGM6fRQbQzXfTBCuR/SK1pUGQDr0PT5GACjnCfEp+CsA2+dI7BIoxbjIhVV8c7uI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=fwRLNqf5; arc=none smtp.client-ip=217.70.183.193 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="fwRLNqf5" Received: by mail.gandi.net (Postfix) with ESMTPSA id 2E09824000A; Tue, 17 Sep 2024 08:53:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1726563199; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=TjrKPmUGSuwPpCD0ZiBfH1qgP1RNlV3NW8vcSdl6R+w=; b=fwRLNqf51vYQMe1JZjA43nqoAMVK7IQ/1UkGOW7h3Mdwdwl9ktQEFnc8yVQnGdKzrTurb1 uOrw3k1+ZLGGIogC8/233uPTItzaNUzLiyrsdoYTnJky4ojYoLzlNvPMXc7fjWjlnijdqt 3lsp2YGjKhpG4MEOEN5GH/rou3vss9CdVKZeYFn0oXBAxJr0NyfF/yZ1XvxILjlYHH/wwP JuxKub3MQ+7qk3ntRU3kSQDPiO2fJ37hPbsA/zxNdVzEdX+DLuwtJ62UxSlDYvuZMiQ3H6 LLCb4NGppydlE+VhsCSsPOX4XyRui3IkxGj5HRwNyeukvdqMv5ziQzQcBY8coA== From: Luca Ceresoli Date: Tue, 17 Sep 2024 10:53:05 +0200 Subject: [PATCH v4 1/8] dt-bindings: connector: add GE SUNH hotplug addon connector Precedence: bulk X-Mailing-List: linux-i2c@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20240917-hotplug-drm-bridge-v4-1-bc4dfee61be6@bootlin.com> References: <20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com> In-Reply-To: <20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com> To: Rob Herring , Krzysztof Kozlowski , Conor Dooley , Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter , Derek Kiernan , Dragan Cvetic , Arnd Bergmann , Greg Kroah-Hartman , Saravana Kannan , Wolfram Sang , "Rafael J. Wysocki" , Lee Jones , Daniel Thompson , Jingoo Han , Helge Deller Cc: Paul Kocialkowski , =?utf-8?q?Herv=C3=A9_Codina?= , Thomas Petazzoni , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-i2c@vger.kernel.org, linux-fbdev@vger.kernel.org, Paul Kocialkowski , Luca Ceresoli X-Mailer: b4 0.14.1 X-GND-Sasl: luca.ceresoli@bootlin.com Add bindings for the GE SUNH add-on connector. This is a physical, hot-pluggable connector that allows to attach and detach at runtime an add-on adding peripherals on non-discoverable busses. Signed-off-by: Luca Ceresoli --- Changed in v4: - rename 'nobus-devices' to 'devices' - use 'additionalProperties: true' for the 'devices' node (nodes are added by overlays) - document GPIO polarity - add '|' for descriptions to preserve line breaks - remove powergood-gpios (removed in hardware design) - Omit "/" node, not needed and cause of warnings - remove reference to v2 examples from example comment - remove unneeded "addon_connector" label from example Changed in v3: - change the layout to only add subnodes, not properties - add the 'nobus-devices' node description to hold devices not on any bus - add 'i2c-*' nodes for the I2C busses, using a i2c-parent phandle - and the 'dsi' node for the DSI bus - move the entire port@1 node to the overlay (not only the remote-endpoint property) - remove the overlay examples (Overlays in examples are not supported) - add more clarifying descriptions and comments for examples - some rewording This patch was added in v2. --- .../connector/ge,sunh-addon-connector.yaml | 177 +++++++++++++++++++++ MAINTAINERS | 5 + 2 files changed, 182 insertions(+) diff --git a/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml b/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml new file mode 100644 index 000000000000..68d7d7d7cf7e --- /dev/null +++ b/Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml @@ -0,0 +1,177 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/connector/ge,sunh-addon-connector.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: GE SUNH hotplug add-on connector + +maintainers: + - Luca Ceresoli + +description: | + Represent the physical connector present on GE SUNH devices that allows + to attach and detach at runtime an add-on adding peripherals on + non-discoverable busses. Peripherals on the add-on include I2C sensors + and a video bridge controlled via I2C. + + The connector has status GPIOs to notify the connection status to the CPU + and a reset GPIO to allow the CPU to reset all the peripherals on the + add-on. It also has I2C busses and a 4-lane MIPI DSI bus. + + Different add-on models can be connected, each having different + peripherals. For this reason each add-on has a model ID stored in a + non-volatile memory, which is accessed in the same way on all add-ons. + + Add-on removal can happen at any moment under user control and without + prior notice to the CPU, making all of its components not usable + anymore. Later on, the same or a different add-on model can be connected. + +properties: + compatible: + const: ge,sunh-addon-connector + + reset-gpios: + description: An output GPIO to reset the peripherals on the + add-on. Active low. + maxItems: 1 + + plugged-gpios: + description: An input GPIO that is asserted if and only if an add-on is + physically connected. Active low. + maxItems: 1 + + devices: + description: | + A container for devices not accessible via any data bus. Common use + cases include fixed and GPIO regulators, simple video panels and LED + or GPIO backlight devices. When not hot-pluggable, nodes such devices + are children of the root node. + + This node should not be present in the connector description in the + base device tree. It should be added by overlays along with a subnode + per device. + + type: object + additionalProperties: true + + dsi: + type: object + additionalProperties: false + + properties: + ports: + $ref: /schemas/graph.yaml#/properties/ports + + description: | + OF graph bindings modeling the MIPI DSI bus across the connector. The + connector splits the video pipeline in a fixed part and a removable + part. + + The fixed part of the video pipeline includes all components up to + the display controller and 0 or more bridges. The removable part + includes any bridges and any other components up to the panel. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: + The last point of the non-removable part of the MIPI DSI bus + line. The remote-endpoint sub-node must point to the last + non-removable video component of the video pipeline. + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + The first point of the removable part of the MIPI DSI bus + line. The remote-endpoint sub-node must point to the first + video pipeline component on the add-on. As it describes the + hot-pluggable hardware, the endpoint node cannot be filled + until an add-on is detected, so this node needs to be added + by a device tree overlay at runtime. + + required: + - port@0 + # port@1 is added by the overlay for any add-on using the DSI lines + + required: + - ports + +patternProperties: + '^i2c-(dbat|gp|btp)$': + description: + An I2C bus that goes through the connector. The adapter (and possibly + some clients) are on the fixed side. Add-ons that have any clients on + this bus have to be added by the add-on overlay, inside this node. + + $ref: /schemas/i2c/i2c-controller.yaml# + unevaluatedProperties: false + type: object + + properties: + i2c-parent: + $ref: /schemas/types.yaml#/definitions/phandle + description: + Phandle pointing to the I2C bus controller on the fixed side that + drives this bus + +required: + - compatible + - i2c-dbat + - i2c-gp + - i2c-btp + - dsi + +unevaluatedProperties: false + +examples: + # This is the description of the connector as it should appear in the + # main DTS describing the "main" board up to the connector. This is + # supposed to be used together with the overlays that describe the add-on + # components. + - | + #include + addon-connector { + compatible = "ge,sunh-addon-connector"; + reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>; + plugged-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>; + powergood-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>; + + i2c-dbat { + i2c-parent = <&i2c5_ch2>; + #address-cells = <1>; + #size-cells = <0>; + // device subnodes to be added by overlays + }; + + i2c-gp { + i2c-parent = <&i2c4>; + #address-cells = <1>; + #size-cells = <0>; + // device subnodes to be added by overlays + }; + + i2c-btp { + i2c-parent = <&i2c3>; + #address-cells = <1>; + #size-cells = <0>; + // device subnodes to be added by overlays + }; + + dsi { + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + endpoint { + remote-endpoint = <&previous_bridge_out>; + }; + }; + + // port@1 to be added by overlay + }; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index cc40a9d9b8cd..b939284d1350 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10270,6 +10270,11 @@ S: Maintained F: Documentation/devicetree/bindings/iio/pressure/honeywell,mprls0025pa.yaml F: drivers/iio/pressure/mprls0025pa* +HOTPLUG CONNECTOR FOR GE SUNH ADDONS +M: Luca Ceresoli +S: Maintained +F: Documentation/devicetree/bindings/connector/ge,sunh-addon-connector.yaml + HP BIOSCFG DRIVER M: Jorge Lopez L: platform-driver-x86@vger.kernel.org From patchwork Tue Sep 17 08:53:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luca Ceresoli X-Patchwork-Id: 829302 Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A9836169AC5; Tue, 17 Sep 2024 08:53:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.70.183.193 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726563208; cv=none; b=WF8tcSRZbdF9piTd0FA6aZMX/lIJx9XCCHO0Er5YKw6V2EOkr0uOWUj/HcSz5hCxQR7Y5g2zwnqufSUzOfsCjinHMJC4TwQUiv0igUmYM9FSSKLidgnc3ywOJShkcJS22ChXIk06LB2M0oTo2bTNnn2FtNWOO2FS0lFu5D8srTA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726563208; c=relaxed/simple; bh=tbi2GjX0sVPxeekko6WyWU769v76iroU9gpWVQtjZyo=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=tuH+oUHdoKcPtOs4EnV3Daw1DDKW1ofRr+pObSV8kZPOJM/J8aKjfbR704FJDIbpP02feKh16wE70ja8bZ4jxmw4wY93oRX2/wsApaKzHsCJr7p+0X/ai6glskxwg24PiaLg9c98HUiB4KhGM/qOlctnz9d6Q2jjS7/zWnwZIe0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=frYdVkIC; arc=none smtp.client-ip=217.70.183.193 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="frYdVkIC" Received: by mail.gandi.net (Postfix) with ESMTPSA id D24F9240011; Tue, 17 Sep 2024 08:53:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1726563204; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zXfD1uIHQ08PVRbHD3ejH9ya7NUdvN1EDBI59inSFEs=; b=frYdVkICECqGxrTtkHsi9cIFBdFPDUgixWgCirbDgtHtZNVWVR3KlzIw6NP8dhkvn9N4Ot DejauOc53hS6e2Nv0ypLQOLFODrbDFuyQLcQFzgsVMYPkDkQC2hpy0grf97yJBAj2cYqEe yt7no8x0tp9WUryXn3qOJ3FfmFdGIp3tbhVgeAChgtU2KC7bkkOh6eGWDuLJCYpixQolJn EouB1v7dSJiIjro1Fy84d0mYIppHIg1yq1LZNTxd3FI8hjTr5yNUjHUlnBKTfsVloTw3id MgYBCC6Or/wizYW/ocWSQrb83opp4eWp8q2thK6hIRemexFkXPeGDqhncU4abQ== From: Luca Ceresoli Date: Tue, 17 Sep 2024 10:53:07 +0200 Subject: [PATCH v4 3/8] drm/encoder: add drm_encoder_cleanup_from() Precedence: bulk X-Mailing-List: linux-i2c@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20240917-hotplug-drm-bridge-v4-3-bc4dfee61be6@bootlin.com> References: <20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com> In-Reply-To: <20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com> To: Rob Herring , Krzysztof Kozlowski , Conor Dooley , Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter , Derek Kiernan , Dragan Cvetic , Arnd Bergmann , Greg Kroah-Hartman , Saravana Kannan , Wolfram Sang , "Rafael J. Wysocki" , Lee Jones , Daniel Thompson , Jingoo Han , Helge Deller Cc: Paul Kocialkowski , =?utf-8?q?Herv=C3=A9_Codina?= , Thomas Petazzoni , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-i2c@vger.kernel.org, linux-fbdev@vger.kernel.org, Paul Kocialkowski , Luca Ceresoli X-Mailer: b4 0.14.1 X-GND-Sasl: luca.ceresoli@bootlin.com Supporting hardware whose final part of the DRM pipeline can be physically removed requires the ability to detach all bridges from a given point to the end of the pipeline. Introduce a variant of drm_encoder_cleanup() for this. Signed-off-by: Luca Ceresoli --- Changes in v3: none Changed in v2: - fix a typo in a comment --- drivers/gpu/drm/drm_encoder.c | 21 +++++++++++++++++++++ include/drm/drm_encoder.h | 1 + 2 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c index 8f2bc6a28482..472dfbefe296 100644 --- a/drivers/gpu/drm/drm_encoder.c +++ b/drivers/gpu/drm/drm_encoder.c @@ -207,6 +207,27 @@ void drm_encoder_cleanup(struct drm_encoder *encoder) } EXPORT_SYMBOL(drm_encoder_cleanup); +/** + * drm_encoder_cleanup_from - remove a given bridge and all the following + * @encoder: encoder whole list of bridges shall be pruned + * @bridge: first bridge to remove + * + * Removes from an encoder all the bridges starting with a given bridge + * and until the end of the chain. + * + * This should not be used in "normal" DRM pipelines. It is only useful for + * devices whose final part of the DRM chain can be physically removed and + * later reconnected (possibly with different hardware). + */ +void drm_encoder_cleanup_from(struct drm_encoder *encoder, struct drm_bridge *bridge) +{ + struct drm_bridge *next; + + list_for_each_entry_safe_from(bridge, next, &encoder->bridge_chain, chain_node) + drm_bridge_detach(bridge); +} +EXPORT_SYMBOL(drm_encoder_cleanup_from); + static void drmm_encoder_alloc_release(struct drm_device *dev, void *ptr) { struct drm_encoder *encoder = ptr; diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h index 977a9381c8ba..bafcabb24267 100644 --- a/include/drm/drm_encoder.h +++ b/include/drm/drm_encoder.h @@ -320,6 +320,7 @@ static inline struct drm_encoder *drm_encoder_find(struct drm_device *dev, } void drm_encoder_cleanup(struct drm_encoder *encoder); +void drm_encoder_cleanup_from(struct drm_encoder *encoder, struct drm_bridge *bridge); /** * drm_for_each_encoder_mask - iterate over encoders specified by bitmask From patchwork Tue Sep 17 08:53:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luca Ceresoli X-Patchwork-Id: 829301 Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DFE3F1607B2; Tue, 17 Sep 2024 08:53:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.70.183.193 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726563213; cv=none; b=MsbNpH1+uYCb99Tdw1WvroPewcT8Vx1oP+omC3yUIkxoeaLECSkOUgNau17mGTQQd6Z6AWGinzquMsc6PjoWyABO5lvMQtIcWt48Z/fZstzT1EAm34u2wDPF9ZVmesMyZmNXWKAiy9NVi6r08vB0l2/Orvoo2C33ilCEkJzqS8A= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726563213; c=relaxed/simple; bh=cJqQamecAfh1kKbIdUhrIjWf7sADs+aSb770KbZzL5E=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=TFAMbT+1zh4e3+s66DDdlXAwM2PlfD8C5WdXMcGnHq54TuvAyU4lDErI9B9VSlFTJpl+K9W4EvMOrnhTXBEnChtYDpgUAeL7CvDsAaAi+DJltBKPp1YPyoniQDjHZKKfb3FR/gTk0F6Mi2aDWNKLWviklPF/UvehpCE/KsEW7OM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=NfCadUBK; arc=none smtp.client-ip=217.70.183.193 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="NfCadUBK" Received: by mail.gandi.net (Postfix) with ESMTPSA id A76D4240013; Tue, 17 Sep 2024 08:53:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1726563208; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YGkP3RSqsbxDFwItYNCazpWcQ0c7RuUBa78KAXrpiwU=; b=NfCadUBKxqPYu6lRcHzEnzlZheitBAe4Ttqi8ZGF4JZQ5Z6p2QFavrsHR4D8Xn/jAN0CB2 PsD/TalDeXXnmXMk00imlYrQOP+pCFcejlTO3c9QAHdLy9UeOmK+JRUiLHU0XeW6QeTJGo dUcGlEb+EUcD+GW9DRhZet3tfDo2uytn1WZb7mm2k80c7WYAlfeB4jEgOLIlFp0I1Kg+W/ hAMQlnygkUAvQeQSjrezr1n0KJVgHlrI5WDNsu5t9GUjvZplsBuPZApHah9ni2r4zWTL+p AOPZBuW+4P8rYGB+R1miXNW2FRxDir4IGdF+P6hROcXKEgfV1ga6JWJjQ4HUSg== From: Luca Ceresoli Date: Tue, 17 Sep 2024 10:53:09 +0200 Subject: [PATCH v4 5/8] i2c: i2c-core-of: follow i2c-parent phandle to probe devices from added nodes Precedence: bulk X-Mailing-List: linux-i2c@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20240917-hotplug-drm-bridge-v4-5-bc4dfee61be6@bootlin.com> References: <20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com> In-Reply-To: <20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com> To: Rob Herring , Krzysztof Kozlowski , Conor Dooley , Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter , Derek Kiernan , Dragan Cvetic , Arnd Bergmann , Greg Kroah-Hartman , Saravana Kannan , Wolfram Sang , "Rafael J. Wysocki" , Lee Jones , Daniel Thompson , Jingoo Han , Helge Deller Cc: Paul Kocialkowski , =?utf-8?q?Herv=C3=A9_Codina?= , Thomas Petazzoni , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-i2c@vger.kernel.org, linux-fbdev@vger.kernel.org, Paul Kocialkowski , Luca Ceresoli X-Mailer: b4 0.14.1 X-GND-Sasl: luca.ceresoli@bootlin.com When device tree nodes are added, the I2C core tries to probe client devices based on the classic DT structure: i2c@abcd0000 { some-client@42 { compatible = "xyz,blah"; ... }; }; However for hotplug connectors described via device tree overlays there is additional level of indirection, which is needed to decouple the overlay and the base tree: --- base device tree --- i2c1: i2c@abcd0000 { compatible = "xyz,i2c-ctrl"; ... }; i2c5: i2c@cafe0000 { compatible = "xyz,i2c-ctrl"; ... }; connector { i2c-ctrl { i2c-parent = <&i2c1>; #address-cells = <1>; #size-cells = <0>; }; i2c-sensors { i2c-parent = <&i2c5>; #address-cells = <1>; #size-cells = <0>; }; }; --- device tree overlay --- ... // This node will overlay on the i2c-ctrl node of the base tree i2c-ctrl { eeprom@50 { compatible = "atmel,24c64"; ... }; }; ... --- resulting device tree --- i2c1: i2c@abcd0000 { compatible = "xyz,i2c-ctrl"; ... }; i2c5: i2c@cafe0000 { compatible = "xyz,i2c-ctrl"; ... }; connector { i2c-ctrl { i2c-parent = <&i2c1>; #address-cells = <1>; #size-cells = <0>; eeprom@50 { compatible = "atmel,24c64"; ... }; }; i2c-sensors { i2c-parent = <&i2c5>; #address-cells = <1>; #size-cells = <0>; }; }; Here i2c-ctrl (same goes for i2c-sensors) represent the part of I2C bus that is on the hot-pluggable add-on. On hot-plugging it will physically connect to the I2C adapter on the base board. Let's call the 'i2c-ctrl' node an "extension node". In order to decouple the overlay from the base tree, the I2C adapter (i2c@abcd0000) and the extension node (i2c-ctrl) are separate nodes. Rightfully, only the former will probe into an I2C adapter, and it will do that perhaps during boot, long before overlay insertion. The extension node won't probe into an I2C adapter or any other device or bus, so its subnodes ('eeprom@50') won't be interpreted as I2C clients by current I2C core code. However it has an 'i2c-parent' phandle to point to the corresponding I2C adapter node. This tells those nodes are I2C clients of the adapter in that other node. Extend the i2c-core-of code to look for the adapter via the 'i2c-parent' phandle when the regular adapter lookup does not find one. This allows all clients to be probed: both those on the base board (described in the base device tree) and those on the add-on and described by an overlay. Signed-off-by: Luca Ceresoli --- Note: while this patch works for normal hotplug and unplug, it has some weaknesses too, due to the implementation being in a OF change notifier. Two cases come to mind: 1. In the base device tree there must be _no_ nodes under the "extension node" (i2c-ctrl), or they won't be picked up as they are not dynamically added. 2. In case the I2C adapter is unbound and rebound, or it probes after overlay insertion, it will miss the OF notifier events and so it won't find the devices in the extension node. The first case is not a limiting factor: fixed I2C devices should just stay under the good old I2C adapter node. The second case is a limiting factor, even though not happening in "normal" use cases. I cannot see any solution without making the adapter aware of the "bus extensions" it has, so on its probe it can always go look for any devices there. Taking into account the case of multiple connectors each having an extension of the same bus, this may look as follows in device tree: --- base device tree --- i2c1: i2c@abcd0000 { compatible = "xyz,i2c-ctrl"; ... i2c-bus-extensions = <&i2c_ctrl_conn0, &i2c_ctrl_conn1>; }; connector@0 { i2c_ctrl_conn0: i2c-ctrl { i2c-parent = <&i2c1>; #address-cells = <1>; #size-cells = <0>; }; }; connector@1 { i2c_ctrl_conn1: i2c-ctrl { i2c-parent = <&i2c1>; #address-cells = <1>; #size-cells = <0>; }; }; I'd love to have some feedback and opinions about the basic idea before digging into the details of this additional step. --- Changes in v4: - fix a typo in commit message This patch first appeared in v3. --- drivers/i2c/i2c-core-of.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c index a6c407d36800..71c559539a13 100644 --- a/drivers/i2c/i2c-core-of.c +++ b/drivers/i2c/i2c-core-of.c @@ -170,6 +170,15 @@ static int of_i2c_notify(struct notifier_block *nb, unsigned long action, switch (of_reconfig_get_state_change(action, rd)) { case OF_RECONFIG_CHANGE_ADD: adap = of_find_i2c_adapter_by_node(rd->dn->parent); + if (adap == NULL) { + struct device_node *i2c_bus; + + i2c_bus = of_parse_phandle(rd->dn->parent, "i2c-parent", 0); + if (i2c_bus) { + adap = of_find_i2c_adapter_by_node(i2c_bus); + of_node_put(i2c_bus); + } + } if (adap == NULL) return NOTIFY_OK; /* not for us */ From patchwork Tue Sep 17 08:53:11 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luca Ceresoli X-Patchwork-Id: 829300 Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 655EC175D3E; Tue, 17 Sep 2024 08:53:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.70.183.193 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726563218; cv=none; b=oJYEynqeOYtJRwrCAqbNBal7pvR/TRM7yiXulXA7tD4QUerPS63j78y08Sb719MkQop1g32bDnLQ9xAQikSP5XQotcPBdVZzXIcdF1eJHiqF9Noio0La/oR2yHz2Fuxrx2Ep1eCfnYdWLv/38Enz/zdGu3kuqsnEBgwiCVu02AY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1726563218; c=relaxed/simple; bh=aZMHIYSvQYFo8rG+6/e+z3dNpbF6VfNgXGZj4ldnjV8=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=OPtGmaoEXy+k9lF/R+izrgfY0TztLOqNVKo1OsnV82mDRw8yYqPmyhOgcBLLcE5BALGux3p1ERhJBA7t144Z79IeYFKr9zSGWE1kCdznrZyF7XjFvcbz7n6sFOEccAuauJCL2svPTmzsmYlbjAwXikeZP0LGlhINQjqbA0QBsdI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=L+t+Zk4o; arc=none smtp.client-ip=217.70.183.193 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="L+t+Zk4o" Received: by mail.gandi.net (Postfix) with ESMTPSA id 7F3C124000C; Tue, 17 Sep 2024 08:53:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1726563213; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=wo3lz1l56RxsDdOlxpx+pnQbRgk2TjIRJ6ukqR8uV0U=; b=L+t+Zk4oxxGLH3wTjb+wPIp7JTamHH5F9CaQkTF3mXH083piSwjvXLYVz6nULR431YwP/G aUoY/kXDk2eTG8egGzU7ePADwIQsilApcECFx7A6VxTDJWo00zvsyuwChBsZDtDCY0LvQt B7uFwk7M9YUdNY4yVPI+VN+ViKrOdY5S+T83Mv4jRTs7hkxI2SI4/DfnzOTZ3DoyYj4Zpi dMQ+IuL5LmKGBMd4Bv8jE5GzkQolvcMNww5R5tyXQ+nOKxdicBLy7lX6QWMAX83ifvydBL yyRbx7UvFbUjbJT98tMVdhhYDtP3cayndL7VbvJcM0ii0Lkh40eapRLSVuGBfQ== From: Luca Ceresoli Date: Tue, 17 Sep 2024 10:53:11 +0200 Subject: [PATCH RFC v4 7/8] driver core: devlink: do not unblock consumers without any drivers found Precedence: bulk X-Mailing-List: linux-i2c@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20240917-hotplug-drm-bridge-v4-7-bc4dfee61be6@bootlin.com> References: <20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com> In-Reply-To: <20240917-hotplug-drm-bridge-v4-0-bc4dfee61be6@bootlin.com> To: Rob Herring , Krzysztof Kozlowski , Conor Dooley , Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter , Derek Kiernan , Dragan Cvetic , Arnd Bergmann , Greg Kroah-Hartman , Saravana Kannan , Wolfram Sang , "Rafael J. Wysocki" , Lee Jones , Daniel Thompson , Jingoo Han , Helge Deller Cc: Paul Kocialkowski , =?utf-8?q?Herv=C3=A9_Codina?= , Thomas Petazzoni , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-i2c@vger.kernel.org, linux-fbdev@vger.kernel.org, Paul Kocialkowski , Luca Ceresoli X-Mailer: b4 0.14.1 X-GND-Sasl: luca.ceresoli@bootlin.com Quick summary: I have investigated a problem for a long time, I have a pretty good understanding, I tried various fixes but none except this is working. The goal of this patch is to discuss the problem to converge to the best solution. --------------------------------- Symptoms --------------------------------- The problem appeared while testing the v3 addon connector driver that is part of this series, and which is based on device tree overlays. Note the symptom happens often, but not always. Changes to logging is a typical way to make it appear/disappear, so it appears as time sensitive. The relevant DT overlay snippet is: / { fragment@0 { target-path = ""; __overlay__ { devices { // nodes in here are populated as platform devices reg_addon_3v3_lcd: regulator-addon-3v3-lcd { compatible = "regulator-fixed"; regulator-name = "3V3_LCD_ADDON"; gpios = <...>; }; addon_panel_dsi_lvds: panel-dsi-lvds { compatible = "..."; power-supply = <®_addon_3v3_lcd>; }; }; }; }; }; So the regulator is a supplier to the panel. Nothing special here, except we are in an overlay. The overlay gets applied and all devices work correctly. Troubles start appearing in the form of two messages on overlay removal, in this order: * WARNING: CPU: 1 PID: 189 at drivers/regulator/core.c:5856 regulator_unregister+0x1ec/0x208 This is issued during removal of the 3V3_LCD_ADDON regulator because rdev->open_count is 1, while it should be 0. This is because the panel still hasn't closed the regulator. * Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 Call trace: regulator_lock_recursive+0x5c/0x200 regulator_lock_dependent+0xc0/0x140 regulator_enable+0x44/0x98 panel_simple_resume+0x38/0x108 [panel_simple] pm_generic_runtime_resume+0x34/0x58 __rpm_callback+0x50/0x1f0 rpm_callback+0x70/0x88 rpm_resume+0x49c/0x678 __pm_runtime_resume+0x54/0xa0 device_release_driver_internal+0xd4/0x240 device_release_driver+0x20/0x38 bus_remove_device+0xd4/0x120 device_del+0x154/0x388 This happens while the panel driver is being removed and the devm infra tries to close the regulator which is already gone. Both errors have the same origin: the regulator driver is removed before the panel driver. --------------------------------- Problem analysis --------------------------------- My analysis showed that the problem originates from devlink manipulation during overlay insertion, but shows its effects on removal. This is the sequence of events: * During overlay insertion: 1. the devlink code creates a devlink (A) with: supplier = regulator-addon-3v3-lcd consumer = panel-dsi-lvds flags = DL_FLAG_INFERRED (+ possibly others) because it has been inferred from firmware data 2. soon after, devlink A is relaxed and then dropped - does not happen always, based on timing - see below for details 3. the regulator-addon-3v3-lcd regulator gets probed as a platform device - the probe function for regulator-addon-3v3-lcd is reg_fixed_voltage_probe(), which calls devm_regulator_register() to register a single new regulator class device for the voltage output - regulator_register() does, among others: - instantiate a new regulator class device (3V3_LCD_ADDON), with parent = regulator-addon-3v3-lcd - adds a devlink (B) with: supplier = 3V3_LCD_ADDON consumer = panel-dsi-lvds At this point we have these devices and devlinks: .---------------------------. | regulator-addon-3v3-lcd | | regulator platform device | supplier consumer | (struct device) |<--------- devlink A -------------. '---------------------------' (inferred) | ^ V | .-----------------. | parent | panel-dsi-lvds | | | (struct device) | | '-----------------' .---------------------------. ^ | 3V3_LCD_ADDON | supplier consumer | | regulator class device |<--------- devlink B -------------' | (struct device) | (created by '---------------------------' regulator core) Depending on whether step 2 happens or not, devlink A will be still present or not during overlay removal. When step 2 happens and devlink A gets dropped (which happens to me almost always), the removal code calls: -> device_release_driver(dev = regulator-addon-3v3-lcd) -> device_release_driver_internal() -> __device_release_driver() -> if (device_links_busy()) // see below { device_links_unbind_consumers(dev = regulator-addon-3v3-lcd) -> for each consumer for which 'dev' is a supplier: { device_release_driver_internal(dev = panel-dsi-lvds) } } The logic is pretty clear: before removing a device that is a supplier to other devices (regulator-addon-3v3-lcd), use devlink to find all consumers (panel-dsi-lvds) and remove them first, recursively. However in case devlink A had been initially dropped, there is no devlink between the two devices. The regulator removal will just proceed, and the regulator device gets removed before its consumer. Note devlink B is not at all examined within this removal phase. device_links_busy() looks at the platform device (regulator-addon-3v3-lcd), and it has no way to know about the class device (3V3_LCD_ADDON). Assuming the whole device_links_busy() / device_links_unbind_consumers() logic is correct, let's move to why devlink A gets dropped. --------------------------------- Why the devlink is dropped --------------------------------- It all starts in the device_add() for regulator-addon-3v3-lcd: /* * If all driver registration is done and a newly added device doesn't * match with any driver, don't block its consumers from probing in * case the consumer device is able to operate without this supplier. */ if (dev->fwnode && fw_devlink_drv_reg_done && !dev->can_match) fw_devlink_unblock_consumers(dev); The three conditions in the if() mean: 1. this device comes from firmware -> always true in my case (device tree) 2. this global flag is set via the deferred_probe_timeout_work as soon as for 10 consecutive seconds there is no new driver being probed; it is never cleared 3. no driver has been matched with this device so far (IOW the probe function of the driver for this device has never been called, regardless of the return value) If all condtions apply, fw_devlink_unblock_consumers() will (after some checks) call fw_devlink_relax_link() on every link to consumers and "relax" it. Relaxing means setting link flags to DL_FLAG_MANAGED | FW_DEVLINK_FLAGS_PERMISSIVE. Soon later, device_links_driver_bound() will take devlinks with these flags and drop them. I was unable to understand in full detail the flag manipulation logic happening in the devlink code. However I think the high-level logic here can be expressed as: if a devlink was inferred from firmware and its supplier device did not probe after a while (*) because no potential driver was found, then maybe that devlink was wrong or it is enforcing a supplier that is optional for the consumer: let's drop the link and see whether the (formerly devlink consumer) device can now probe. (*) "after a while" is implemented by the fw_devlink_drv_reg_done flag, which typically gets set way less than a minute after boot Basically the fw_devlink_drv_reg_done flag splits the probing in two phases. In phase 1 we try to probe all inferred suppliers before probing consumers. Then we set fw_devlink_drv_reg_done and relax+drop the "dangling" inferred devlinks. Then in phase 2 we try to probe without inferred devlinks. This is to see if we can probe more devices due to incorrectly inferred devlinks or missing drivers for optional suppliers. Overlays however can be loaded at any time, even a long time after booting. This is totally normal when used for a hotplug connector, where the devices get physically connected by the user. This implies the fw_devlink_drv_reg_done flag is found already set when probing overlay devices. And so, conditions 1 and 2 above are always set in the overlay case. So we are left with condition 3 only. Again I haven't done a full analysis here, but it is perfectly fine that a driver is not immediately present when adding a new device. It can just have not yet been matched, possibly because a driver module is in process of being loaded from storage. I think there is a race here: on one side the driver becoming available and matched to the device to be probed, on the other side the fw_devlink_unblock_consumers() logic to relax and drop inferred devlinks. If the device probes first, the link won't be dropped. --------------------------------- Same problem without DT overlays? --------------------------------- Based on the above, I suspect the exact same problem exists even without any overlay usage. Indeed, the conditions might exist in other corner cases. One example is a device whose driver is a module and is not loaded during boot: e.g. it is not on the root filesystem, it is being developed and the programmer sends the driver via SSH to a tmpfs to load and test it. As said, this is a matter of corner cases, but still possible. Note that no problem should happen to natively removable devices such as USB, because condition 1 defuses the whole if() above for devices not described in firmware. --------------------------------- Fixes I have tried (not working) --------------------------------- I tried a couple approaches based on devlink to fix this issue. One was augmenting the regulator core code to add a new devlink between the regulator platform device (regulator-addon-3v3-lcd) and the regulator class device (3V3_LCD_ADDON), to create a chain for device_links_busy() to follow. The devlink is created and persists until removal time. However it does not enforce the correct ordering: device_links_busy() ignores it because the link status is always "dormant". The reason appears to be that the "regulator output device" is a struct device but never has a driver. Recently Saravana pointed out that: > device links don't work correctly for "class" type devices (https://lore.kernel.org/all/CAGETcx-ioF=jTbyQMeD2fsYKz8q5vw_TWYWS9m8H5=pCo5KFYA@mail.gmail.com/) which is possibly related. I tried a variant: change the devlink already created by _regulator_get() to use the regulator platform device (regulator-addon-3v3-lcd) instead of the regulator class device (3V3_LCD_ADDON) as the supplier. That would make devlink B have the same endpoints as devlink A. However this did not work due to the link state staying "not tracked" and thus again being ignored by device_links_busy(). I haven't managed to find out the flag manipulations that would make it work. --------------------------------- Conclusions --------------------------------- The current logic is clearly OK for "typical" current use cases (not counting corner cases), but unsuitable for hotplugged devices described by firmware. The question is: do we have an underlying assumption that was valid so far but is wrong when overlays are added? One possible answer is: dropping inferred devlinks is wrong. Generally speaking, inferred devlinks are sometimes useless but don't hurt, so there is no need to drop them. This is what this patch changes. However I realize there is a use case for dropping inferred devlink: optional suppliers that prevent consumer probing until they are dropped. Indeed, inferring devlinks from firmware data can create links that prevent some device to probe. For this reason my first attempts have been to add or change the devlinks that subsystem code creates. So a more sophisticated idea is that after phase 1 we try to probe all not-probed-yet consumers ignoring the relaxed devlinks, instead of removing them. This would allow the same amount of devices to be probed using the same amount of optional suppliers, but leaving the inferred devlinks in place because they might be useful later on. And then of course there are the above solutions I failed to get working, which might be the right way but need some directions for me to have them working. I am very open to more answers and suggestions. Best regards, Luca Signed-off-by: Luca Ceresoli --- Changes in v4: - fix typos and update device tree names in commit message This patch first appeared in v3. --- drivers/base/core.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 82cc4069a24a..32413dd330b1 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1876,19 +1876,6 @@ void __init wait_for_init_devices_probe(void) fw_devlink_best_effort = false; } -static void fw_devlink_unblock_consumers(struct device *dev) -{ - struct device_link *link; - - if (!fw_devlink_flags || fw_devlink_is_permissive()) - return; - - device_links_write_lock(); - list_for_each_entry(link, &dev->links.consumers, s_node) - fw_devlink_relax_link(link); - device_links_write_unlock(); -} - #define get_dev_from_fwnode(fwnode) get_device((fwnode)->dev) static bool fwnode_init_without_drv(struct fwnode_handle *fwnode) @@ -3682,14 +3669,6 @@ int device_add(struct device *dev) bus_probe_device(dev); - /* - * If all driver registration is done and a newly added device doesn't - * match with any driver, don't block its consumers from probing in - * case the consumer device is able to operate without this supplier. - */ - if (dev->fwnode && fw_devlink_drv_reg_done && !dev->can_match) - fw_devlink_unblock_consumers(dev); - if (parent) klist_add_tail(&dev->p->knode_parent, &parent->p->klist_children);