Re: [PATCH v3 01/19] media: dt-bindings: media: camss: Add qcom,sm8250-camss binding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/01/2022 13:42, Vladimir Zapolskiy wrote:
Hi Bryan, Robert,

On 12/22/21 2:37 AM, Bryan O'Donoghue wrote:
From: Jonathan Marek <jonathan@xxxxxxxx>

Add bindings for qcom,sm8250-camss in order to support the camera
subsystem for SM8250.

Cc: devicetree@xxxxxxxxxxxxxxx
Signed-off-by: Jonathan Marek <jonathan@xxxxxxxx>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
---
  .../bindings/media/qcom,sm8250-camss.yaml     | 450 ++++++++++++++++++
  1 file changed, 450 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml

diff --git a/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sm8250-camss.yaml

<snip>

+required:
+  - clock-names
+  - clocks
+  - compatible
+  - interconnects
+  - interconnect-names
+  - interrupts
+  - interrupt-names
+  - iommus
+  - power-domains
+  - reg
+  - reg-names
+
+additionalProperties: false

I've discovered that there is a noticeable difference between this bindings
and all the previous ones, for instance see qcom,sdm845-camss.yaml

There is no required 'vdda-supply' property on the list, and fwiw I believe
there should be two supply properties for 0p9 and 1p2 supplies in fact.
Similarly, two separate supplies should be present in sdm845 camss bindings.

The 1p2 supply is defined in the camera sensor section as dvdd-supply

https://git.linaro.org/people/bryan.odonoghue/kernel.git/tree/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts?h=v5.16-rc6-sm8250-camss-imx577-only-v3

1p2 connects to dvdd-supply as per Miura(865-RB5)-Camera_NAV_Mezz page 10 - thundercomm registration required to view

arch/arm64/boot/dts/qcom/qrb5165-rb5.dts::&cci_i2c2::camera@1a
{
    dovdd-supply  = <&vreg_s4a_1p8>;
    avdd-supply = <&vreg_l7f_1p8>;
    dvdd-supply = <&vreg_l9a_1p2>;
}

similarly to

arch/arm64/boot/dts/qcom/sdm845-db845c.dts::&cci_i2c0::camera@10 {
    dovdd-supply = <&vreg_lvs1a_1p8>;
    avdd-supply = <&cam0_avdd_2v8>;
    dvdd-supply = <&cam0_dvdd_1v2>;
}

and

arch/arm64/boot/dts/qcom/apq8016-sbc.dts::&cci_i2c0::camera_rear@3b
{
    vdddo-supply = <&camera_vdddo_1v8>;
    vdda-supply = <&camera_vdda_2v8>;
    vddd-supply = <&camera_vddd_1v5>; /* bod: here */
}

The IMX sensor needs to have the regulator_bulk_enable() stuff added, I'll post this patch it works standalone

https://git.linaro.org/people/bryan.odonoghue/kernel.git/commit/?h=v5.16-rc6-sm8250-camss-imx577-only-v3&id=e82fc1b29d9227cad3ad7dcab362c39dd4a63bdb

Simiar to 0c2c7a1e0d69 ("media: ov8856: Add devicetree support")

Downstream points the CPA to "camss-vdd-supply = <&titan_top_gdsc>;" which is covered by our TITAN_TOP_GDSC power-domain and "mipi-csi-vdd-supply = <&pm8150_l9>;"

regulator-pm8150-l9 == rpmh-regulator-ldoa9 == ldoa9 == pmic5-ldo

aka l9a upstream

"dvdd-supply = <&vreg_l9a_1p2>;"

vreg_l9a_1p2: ldo9 {
    regulator-name = "vreg_l9a_1p2";
    regulator-min-microvolt = <1200000>;
    regulator-max-microvolt = <1200000>;
    regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};

Not sure I see on the schematic or in the downstream dts how 0p9 is connected to the camera - seems to be a pcie and or mdss-dsi regulator.

If vdda-supply is a required property of the camera and not the sensor then what regulator do you think it should point to ?

At the moment the driver operates with 'vdda' supply only, commit 9e5d1581
introduced undocumented 'vdd_sec' for sdm660, but, if I'm not mistaken,
it's unused.

I agree with you there - vdd_sec is either unspecified in the Documentation or not required.

---
bod



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux