From: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
To: Vikram Sharma <quic_vikramsa@quicinc.com>,
mchehab@kernel.org, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, andersson@kernel.org,
konradybcio@kernel.org, hverkuil-cisco@xs4all.nl,
cros-qcom-dts-watchers@chromium.org, catalin.marinas@arm.com,
will@kernel.org
Cc: linux-arm-kernel@lists.infradead.org, quic_svankada@quicinc.com,
linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Nihal Kumar Gupta <quic_nihalkum@quicinc.com>,
bryan.odonoghue@linaro.org
Subject: Re: [PATCH v4 1/2] dt-bindings: media: qcom,qcs8300-camss: Add missing power supplies
Date: Wed, 15 Oct 2025 21:41:32 +0300 [thread overview]
Message-ID: <bfa0d4f2-45c6-463e-8afc-d90edcf8cf2f@linaro.org> (raw)
In-Reply-To: <20251015130130.2790829-2-quic_vikramsa@quicinc.com>
On 10/15/25 16:01, Vikram Sharma wrote:
> Add support for vdda-phy-supply and vdda-pll-supply in the QCS8300
> CAMSS binding to reflect camera sensor hardware requirements.
What are "camera sensor hardware requirements"? You do add properties
to the ISP.
>
> Co-developed-by: Nihal Kumar Gupta <quic_nihalkum@quicinc.com>
> Signed-off-by: Nihal Kumar Gupta <quic_nihalkum@quicinc.com>
> Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com>
> ---
> .../bindings/media/qcom,qcs8300-camss.yaml | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/qcom,qcs8300-camss.yaml b/Documentation/devicetree/bindings/media/qcom,qcs8300-camss.yaml
> index 80a4540a22dc..dce0a1fcb10c 100644
> --- a/Documentation/devicetree/bindings/media/qcom,qcs8300-camss.yaml
> +++ b/Documentation/devicetree/bindings/media/qcom,qcs8300-camss.yaml
> @@ -120,6 +120,14 @@ properties:
> items:
> - const: top
>
> + vdda-phy-supply:
> + description:
> + Phandle to a regulator supply to PHY core block.
What is "PHY core block" here?
> +
> + vdda-pll-supply:
> + description:
> + Phandle to 1.8V regulator supply to PHY refclk pll block.
This is a copy-paste example of a known to be wrong pattern.
If you open a QCS8300 documentation or a schematics of some QCS8300 powered
board, can you get a link between the actual pin names and the introduced
property? Likely there should be nothing, which resembles "PHY refclk pll
block" over there, and thus the description of the property is misleading.
Please check other similar and recently added properties, like it's been
done if you add a voltage level reference (and that's the only known for
sure information), there will be no need to guess the right voltage level.
And it's either incorrect in the description above or in the example below.
> +
> ports:
> $ref: /schemas/graph.yaml#/properties/ports
>
> @@ -160,6 +168,8 @@ required:
> - power-domains
> - power-domain-names
> - ports
> + - vdda-phy-supply
> + - vdda-pll-supply
>
> additionalProperties: false
>
> @@ -328,6 +338,9 @@ examples:
> power-domains = <&camcc CAM_CC_TITAN_TOP_GDSC>;
> power-domain-names = "top";
>
> + vdda-phy-supply = <&vreg_l4a_0p88>;
> + vdda-pll-supply = <&vreg_l1c_1p2>;
> +
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
--
Best wishes,
Vladimir
next prev parent reply other threads:[~2025-10-15 18:41 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-15 13:01 [PATCH v4 0/2] qcom: qcs8300: Add qcs8300 camss support Vikram Sharma
2025-10-15 13:01 ` [PATCH v4 1/2] dt-bindings: media: qcom,qcs8300-camss: Add missing power supplies Vikram Sharma
2025-10-15 13:30 ` Bryan O'Donoghue
2025-10-15 18:41 ` Vladimir Zapolskiy [this message]
2025-10-15 19:12 ` Bryan O'Donoghue
2025-10-15 19:20 ` Vladimir Zapolskiy
2025-10-15 13:01 ` [PATCH v4 2/2] arm64: dts: qcom: qcs8300: Add support for camss Vikram Sharma
2025-10-15 18:49 ` Vladimir Zapolskiy
2025-10-15 19:58 ` Bryan O'Donoghue
2025-10-15 23:21 ` Vladimir Zapolskiy
2025-10-16 5:59 ` Krzysztof Kozlowski
2025-10-16 9:52 ` Vladimir Zapolskiy
2025-10-16 10:39 ` Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bfa0d4f2-45c6-463e-8afc-d90edcf8cf2f@linaro.org \
--to=vladimir.zapolskiy@linaro.org \
--cc=andersson@kernel.org \
--cc=bryan.odonoghue@linaro.org \
--cc=catalin.marinas@arm.com \
--cc=conor+dt@kernel.org \
--cc=cros-qcom-dts-watchers@chromium.org \
--cc=devicetree@vger.kernel.org \
--cc=hverkuil-cisco@xs4all.nl \
--cc=konradybcio@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=quic_nihalkum@quicinc.com \
--cc=quic_svankada@quicinc.com \
--cc=quic_vikramsa@quicinc.com \
--cc=robh@kernel.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).