From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 EE8A53126BF; Wed, 8 Jul 2026 06:15:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783491357; cv=none; b=CPC7+O59FWg1uotNnb+GlpxY4jdcDrMA+8nInz5lhvE0fFqYdbpU4PTrxvSo64lkqX3Pw35UwQaYNJiLaxptHgQumomfB4lVCF9TKB3EcqOcZN2qVcFEXU5/ZwNeL2xp0kazdeU6BKrTQMLciM5E7CFkSOLShsahSW7l+5OZemw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783491357; c=relaxed/simple; bh=pIUQ4l/zyckh6vUaw6vYAa6JnTL6+y1795NFYKUQuuk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=VDOsKS11/xkUbRHktRWddEkslxU9Ex2VobVGKFjG6tSDZMxjA0B2TgGEGfctsv0X9VZ4z7M9JHDKYiXmJDl6QgILOf5bYVPo6K7ikju4P+CgQETFNYC7Rnx7OgAxuUE6A72Pdgnqp6mxai7CwMD7GJxNTlNPzCw+nsPWcr3pBbo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=IOQjCEKz; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="IOQjCEKz" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5C6841F000E9; Wed, 8 Jul 2026 06:15:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1783491355; bh=0k5II8l8SmqeI+eI4lrAlQGKex7vSPZrEvokGtu5P1U=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=IOQjCEKzAGl2EPxaOBq8AwDpbcOHv9g1XYjFHTNdnnFBZhqjghnm0EW0gXGu9e/3n VK0Muko6N+6PO+73JFdb1q/q6M+yGcuVWssdEgUTKxDHzKmn4QxLeSDfDdah2vhynw tO4+dpTqUjKc7cR0MMxl3AnUKoNDW8IOpNqvrEKD0eLw9/6JElhdY+V+BFGpaRMgxV HxDsEK6HJWx3tppvrVBJEnfoMw2nJGJ+O7GhOZGXITPMwJQscXf0ivcWjCs9q+CR4S WeZp+2f4JPDUu/PULFQDR81LC3jbrV/YZ9Dw4p2VKg63zQ0Y4wxvEIxhLktyvpIPQT BUY23mOdMfWVg== Date: Wed, 8 Jul 2026 08:15:51 +0200 From: Krzysztof Kozlowski To: Bryan O'Donoghue Cc: Vinod Koul , Kishon Vijay Abraham I , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Neil Armstrong , Bryan O'Donoghue , Vladimir Zapolskiy , linux-arm-msm@vger.kernel.org, linux-phy@lists.infradead.org, linux-media@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v9 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema Message-ID: <20260708-gentle-classy-tiger-af3ab0@quoll> References: <20260708-x1e-csi2-phy-v9-0-0210b90c04cf@linaro.org> <20260708-x1e-csi2-phy-v9-1-0210b90c04cf@linaro.org> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20260708-x1e-csi2-phy-v9-1-0210b90c04cf@linaro.org> On Wed, Jul 08, 2026 at 12:39:02AM +0100, Bryan O'Donoghue wrote: > Add a base schema initially compatible with x1e80100 to describe MIPI CSI2 > PHY devices. > > The hardware can support both CPHY, DPHY and a special split-mode DPHY. > > The schema here defines two ports with three endpoints: > > port@0: Sensor input. > endpoint@0: primary sensor > endpoint@1: optional second sensor, implies DPHY split-mode > > port@1: Controller output. Do not paste contents of the diff into the commit message, it brings no benefits. > > The CSIPHY devices have their own pinouts on the SoC as well as their own > individual voltage rails. > > The need to model voltage rails on a per-PHY basis leads us to define > CSIPHY devices as individual nodes. This entire commit msg is difficult to read - every sentence per paragraph. This makes no sense. We do not write like that. Please organize the flow in some logical chunks and combine paragraphs. So it will be easier to read. > > Two nice outcomes in terms of schema and DT arise from this change. > > 1. The ability to define on a per-PHY basis voltage rails. > 2. The ability to require those voltage. > > We have had a complete bodge upstream for this where a single set of > voltage rail for all CSIPHYs has been buried inside of CAMSS. > > Much like the I2C bus which is dedicated to Camera sensors - the CCI bus in > CAMSS parlance, the CSIPHY devices should be individually modelled. > > Signed-off-by: Bryan O'Donoghue > --- > .../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 202 +++++++++++++++++++++ > 1 file changed, 202 insertions(+) > > diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml > new file mode 100644 > index 0000000000000..a7fbf6804cd9e > --- /dev/null > +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml > @@ -0,0 +1,202 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm SoC CSI2 PHY Qualcomm X1E80100 SoC.... > + > +maintainers: > + - Bryan O'Donoghue > + > +description: > + Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2 sensors > + to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and D-PHY > + modes. > + > +properties: > + compatible: > + const: qcom,x1e80100-csi2-phy > + > + reg: > + maxItems: 1 > + > + "#phy-cells": > + const: 1 > + description: > + The single cell specifies the PHY operating mode. > + > + clocks: > + maxItems: 3 > + > + clock-names: > + items: > + - const: core > + - const: timer > + - const: ahb > + > + interrupts: > + maxItems: 1 > + > + operating-points-v2: true opp-table: type: object > + > + power-domains: > + items: > + - description: Titan Top GDSC - Titan ISP Block, Global Distributed Switch Controller. > + - description: MMCX voltage rail > + - description: MXC or MXA voltage rail > + > + power-domain-names: > + items: > + - const: top > + - const: mmcx > + - const: mx > + > + vdda-0p8-supply: > + description: Phandle to a 0.8V regulator supply to a PHY. > + > + vdda-1p2-supply: > + description: Phandle to 1.2V regulator supply to a PHY. > + > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/$defs/port-base > + description: > + Sensor input. Always present. A single sensor is described by a > + single endpoint with one to four data lanes. DPHY split mode, > + where two independent sensors share the same PHY, is described > + by two endpoints; endpoint@0 with exactly two-data lanes and > + endpoint@1 with exactly one data-lane. > + unevaluatedProperties: false > + > + patternProperties: > + "^endpoint(@[0-9a-f]+)?$": > + $ref: /schemas/media/video-interfaces.yaml# > + unevaluatedProperties: false > + properties: > + data-lanes: > + minItems: 1 > + maxItems: 4 > + remote-endpoint: true Drop this one Blank line > + required: > + - data-lanes > + - remote-endpoint > + > + allOf: > + - if: > + required: > + - endpoint@1 > + then: > + properties: > + endpoint@0: > + properties: > + data-lanes: > + minItems: 2 > + maxItems: 2 > + endpoint@1: > + properties: > + data-lanes: > + maxItems: 1 > + required: > + - endpoint@0 > + > + port@1: > + $ref: /schemas/graph.yaml#/$defs/port-base This is odd, don't use endpoint-base below. Why do you need it? That's just /schemas/graph.yaml#/properties/port schema, no? > + description: Output to the CAMSS CSID controller. > + unevaluatedProperties: false > + > + patternProperties: > + "^endpoint(@[0-9a-f]+)?$": > + $ref: /schemas/graph.yaml#/$defs/endpoint-base > + unevaluatedProperties: false > + properties: > + remote-endpoint: true > + required: > + - remote-endpoint > + > + required: > + - port@0 > + - port@1 > + > +required: > + - compatible > + - reg > + - "#phy-cells" > + - clocks > + - clock-names > + - interrupts > + - operating-points-v2 > + - power-domains > + - power-domain-names > + - vdda-0p8-supply > + - vdda-1p2-supply > + - ports > + > +additionalProperties: false > + > +examples: > + - | > + #include > + #include > + #include > + #include > + > + phy@ace4000 { > + compatible = "qcom,x1e80100-csi2-phy"; > + reg = <0x0ace4000 0x2000>; > + #phy-cells = <1>; > + > + clocks = <&camcc CAM_CC_CSIPHY0_CLK>, > + <&camcc CAM_CC_CSI0PHYTIMER_CLK>, > + <&camcc CAM_CC_CORE_AHB_CLK>; > + clock-names = "core", > + "timer", > + "ahb"; > + > + interrupts = ; > + > + operating-points-v2 = <&csiphy_opp_table>; > + > + power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>, > + <&rpmhpd RPMHPD_MMCX>, > + <&rpmhpd RPMHPD_MX>; > + power-domain-names = "top", > + "mmcx", > + "mx"; > + > + vdda-0p8-supply = <&vreg_l2c_0p8>; > + vdda-1p2-supply = <&vreg_l1c_1p2>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + csiphy0_in: endpoint { > + data-lanes = <0 1 2 3>; > + remote-endpoint = <&sensor_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + csiphy0_out: endpoint { > + remote-endpoint = <&csid_in>; > + }; > + }; > + }; > + }; > + > + csiphy_opp_table: opp-table { Drop node, if outside then not really relevant to the example. Best regards, Krzysztof