On 04/01/2022 15:28, Bryan O'Donoghue wrote:
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
Keep guessing there BOD.
Having a deep dive on the schematics of the RB3 and the RB5 there are a
number of broken assumptions on regulators that "just happen to work"
I've corrected these for RB5 and will presently validate on RB3, I've
added in regulator_bulk_enable and regulator_bulk_disable which should
capture what we need to do for sm8250 and 630 - once we fixup the DTS
for 630
Note: I don't have schematics for 630 or to my knowledge DTS either - I
haven't looked all that hard, perhaps its easy to find.
The CSI vdda enable is still required in the sensor but @
dovdd-supply = <&vreg_l7f_1p8>; not dovdd-supply = <&vreg_s4a_1p8>;
The sensor stuff hasn't been submitted upstream but still my v3 tree is
wrong.
On RB5 if we switched off USB, UFS and PCIe there's no way the camera
could come up.
The patch in the series I picked up from @Jonathan to drop regulator
enable for rb3 is also wrong.
I've got the RB5 part working a bit better - need to fixup for RB3 and
will resend
git.linaro.org/people/bryan.odonoghue/kernel.git /
v5.16-rc6-sm8250-camss-imx577-only-v4
Thanks for the ping @Vladimir.
Please don't apply V3 for now
---
bod
