| Message ID | 20240629-camss_first_post_linux_next-v1-0-bc798edabc3a@quicinc.com |
|---|---|
| Headers | show |
| Series | media: qcom: camss: Add sc7280 support | expand |
On Freitag, 28. Juni 2024 20:32:39 MESZ Vikram Sharma wrote: > This change enables IMX577 sensor driver for qcm6490. > > Signed-off-by: Hariram Purushothaman <quic_hariramp@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- > drivers/i2c/busses/i2c-qcom-cci.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c > index 414882c57d7f..10e6df566ae3 100644 > --- a/drivers/i2c/busses/i2c-qcom-cci.c > +++ b/drivers/i2c/busses/i2c-qcom-cci.c > @@ -817,6 +817,7 @@ static const struct of_device_id cci_dt_match[] = { > * Do not add any new ones unless they introduce a new config > */ > { .compatible = "qcom,msm8916-cci", .data = &cci_v1_data}, > + { .compatible = "qcom,sc7280-cci", .data = &cci_v2_data}, Please read the comment above qcom,msm8916-cci. And sc7280.dtsi already uses compatible = "qcom,sc7280-cci", "qcom,msm8996-cci"; So qcom,msm8996-cci with the same match data (cci_v2_data) gets used, so just drop this patch. Regards Luca > { .compatible = "qcom,sdm845-cci", .data = &cci_v2_data}, > { .compatible = "qcom,sm8250-cci", .data = &cci_v2_data}, > { .compatible = "qcom,sm8450-cci", .data = &cci_v2_data}, > >
On 28/06/2024 19:32, Vikram Sharma wrote: > Add bindings for qcom,sc7280-camss in order to support the camera > subsystem for sc7280. > > Signed-off-by: Suresh Vankadara <quic_svankada@quicinc.com> > Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- > .../bindings/media/qcom,sc7280-camss.yaml | 477 +++++++++++++++++++++ > 1 file changed, 477 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/qcom,sc7280-camss.yaml b/Documentation/devicetree/bindings/media/qcom,sc7280-camss.yaml > new file mode 100644 > index 000000000000..588c6fb50e2f > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/qcom,sc7280-camss.yaml > @@ -0,0 +1,477 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > + > +--- > +$id: http://devicetree.org/schemas/media/qcom,sc7280-camss.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm Technologies, Inc. SC7280 CAMSS ISP > + > +maintainers: > + - Azam Sadiq Pasha Kapatrala Syed <akapatra@quicinc.com> > + - Hariram Purushothaman <hariramp@quicinc.com> > + > +description: | > + The CAMSS IP is a CSI decoder and ISP present on > + Qualcomm Technologies, Inc. platforms. > + > +properties: > + compatible: > + const: qcom,sc7280-camss > + > + clocks: > + minItems: 41 > + maxItems: 41 > + > + clock-names: > + items: > + > + - const: cam_hf_axi > + - const: slow_ahb_src > + - const: cpas_ahb > + - const: camnoc_axi_src You almost certainly don't need to include the "_src" clocks in the list of clocks since the CAMCC driver lists "someclock" as having parent "someclock_src" > + - const: camnoc_axi > + - const: csiphy0 > + - const: csiphy0_timer > + - const: csiphy0_timer_src > + - const: csiphy1 > + - const: csiphy1_timer > + - const: csiphy1_timer_src > + - const: csiphy2 > + - const: csiphy2_timer > + - const: csiphy2_timer_src > + - const: csiphy3 > + - const: csiphy3_timer > + - const: csiphy3_timer_src > + - const: csiphy4 > + - const: csiphy4_timer > + - const: csiphy4_timer_src > + - const: vfe0_csid > + - const: vfe0_cphy_rx > + - const: vfe0 > + - const: vfe0_axi > + - const: csiphy_rx_src > + - const: vfe1_csid > + - const: vfe1_cphy_rx > + - const: vfe1 > + - const: vfe1_axi > + - const: vfe2_csid > + - const: vfe2_cphy_rx > + - const: vfe2 > + - const: vfe2_axi > + - const: vfe0_lite_csid > + - const: vfe0_lite_cphy_rx > + - const: vfe0_lite > + - const: vfe1_lite_csid > + - const: vfe1_lite_cphy_rx > + - const: vfe1_lite > + - const: vfe_lite0 > + - const: vfe_lite1 > + > + interrupts: > + minItems: 15 > + maxItems: 15 > + > + interrupt-names: > + items: > + - const: csiphy0 > + - const: csiphy1 > + - const: csiphy2 > + - const: csiphy3 > + - const: csiphy4 > + - const: csid0 > + - const: csid1 > + - const: csid2 > + - const: csid_lite0 > + - const: csid_lite1 > + - const: vfe0 > + - const: vfe1 > + - const: vfe2 > + - const: vfe_lite0 > + - const: vfe_lite1 > + > + iommus: > + maxItems: 1 > + > + interconnects: > + minItems: 2 > + maxItems: 2 > + > + interconnect-names: > + items: > + - const: cam_ahb > + - const: cam_hf_0 > + > + power-domains: > + items: > + - description: IFE0 GDSC - Image Front End, Global Distributed Switch Controller. > + - description: IFE1 GDSC - Image Front End, Global Distributed Switch Controller. > + - description: IFE2 GDSC - Image Front End, Global Distributed Switch Controller. > + - description: Titan GDSC - Titan ISP Block, Global Distributed Switch Controller. Please name these power domains. https://lore.kernel.org/linux-arm-msm/fcdb072d-6099-4423-b4b5-21e9052b82cc@linaro.org/ --- bod
On 28/06/2024 19:32, Vikram Sharma wrote: > Add support for IMX577 camera sensor for SC7280 SoC. > > Signed-off-by: Hariram Purushothaman <quic_hariramp@quicinc.com> > Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc7280.dtsi | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > index 9ac251fec262..1c99ee09a11a 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -5167,6 +5167,39 @@ cci3_sleep: cci3-sleep-state { > bias-pull-down; > }; > > + cam2_default: cam2-default { > + rst { > + pins = "gpio78"; /*cam3*/ I don't think the /* cam3 */ adds much here TBH. > + function = "gpio"; > + drive-strength = <2>; > + bias-disable; > + }; > + > + mclk { > + pins = "gpio67"; /*cam3*/ > + function = "cam_mclk"; > + drive-strength = <2>; /*RB5 was 16 and i changed to 2 here*/ You can drop that comment too, actually more saliently, what are you changing from 16 to 2 since its being mentioned ? --- bod
On 28.06.2024 8:32 PM, Vikram Sharma wrote: > Add support for IMX577 camera sensor for SC7280 SoC. > > Signed-off-by: Hariram Purushothaman <quic_hariramp@quicinc.com> > Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc7280.dtsi | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > index 9ac251fec262..1c99ee09a11a 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -5167,6 +5167,39 @@ cci3_sleep: cci3-sleep-state { > bias-pull-down; > }; > > + cam2_default: cam2-default { > + rst { > + pins = "gpio78"; /*cam3*/ You can drop these comments.. the node name and label suggest this is cam*2* anyway > + function = "gpio"; > + drive-strength = <2>; > + bias-disable; > + }; > + > + mclk { > + pins = "gpio67"; /*cam3*/ > + function = "cam_mclk"; > + drive-strength = <2>; /*RB5 was 16 and i changed to 2 here*/ /* why? */ Konrad
On 28/06/2024 20:32, Vikram Sharma wrote: > Add support for IMX577 camera sensor for SC7280 SoC. > > Signed-off-by: Hariram Purushothaman <quic_hariramp@quicinc.com> > Signed-off-by: Trishansh Bhardwaj <quic_tbhardwa@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc7280.dtsi | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > index 9ac251fec262..1c99ee09a11a 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -5167,6 +5167,39 @@ cci3_sleep: cci3-sleep-state { > bias-pull-down; > }; > > + cam2_default: cam2-default { There are tools. Use them, instead of us. Read you internal guideline - it asks you to do that. It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). Best regards, Krzysztof
On 28/06/2024 20:32, Vikram Sharma wrote: > This change enables IMX577 sensor driver for qcm6490. > > Signed-off-by: Hariram Purushothaman <quic_hariramp@quicinc.com> > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- > drivers/i2c/busses/i2c-qcom-cci.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c > index 414882c57d7f..10e6df566ae3 100644 > --- a/drivers/i2c/busses/i2c-qcom-cci.c > +++ b/drivers/i2c/busses/i2c-qcom-cci.c > @@ -817,6 +817,7 @@ static const struct of_device_id cci_dt_match[] = { > * Do not add any new ones unless they introduce a new config > */ > { .compatible = "qcom,msm8916-cci", .data = &cci_v1_data}, > + { .compatible = "qcom,sc7280-cci", .data = &cci_v2_data}, NAK, ridiculous. Best regards, Krzysztof
On 29/06/2024 10:22, Luca Weiss wrote: > On Freitag, 28. Juni 2024 20:32:39 MESZ Vikram Sharma wrote: >> This change enables IMX577 sensor driver for qcm6490. >> >> Signed-off-by: Hariram Purushothaman <quic_hariramp@quicinc.com> >> Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> >> --- >> drivers/i2c/busses/i2c-qcom-cci.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c >> index 414882c57d7f..10e6df566ae3 100644 >> --- a/drivers/i2c/busses/i2c-qcom-cci.c >> +++ b/drivers/i2c/busses/i2c-qcom-cci.c >> @@ -817,6 +817,7 @@ static const struct of_device_id cci_dt_match[] = { >> * Do not add any new ones unless they introduce a new config >> */ >> { .compatible = "qcom,msm8916-cci", .data = &cci_v1_data}, >> + { .compatible = "qcom,sc7280-cci", .data = &cci_v2_data}, > > Please read the comment above qcom,msm8916-cci. > > And sc7280.dtsi already uses > > compatible = "qcom,sc7280-cci", "qcom,msm8996-cci"; > > So qcom,msm8996-cci with the same match data (cci_v2_data) gets used, so > just drop this patch. > I think we put quite obvious comment, yet it is ignored. Any ideas how to change the comment so people will read it? Best regards, Krzysztof
SC7280 is a Qualcomm SoC. This series adds support to bring up the CSIPHY, CSID, VFE/RDI interfaces in SC7280. SC7280 provides - 3 x VFE, 3 RDI per VFE - 2 x VFE Lite, 4 RDI per VFE - 3 x CSID - 2 x CSID Lite - 5 x CSI PHY This series is rebased based on： https://lore.kernel.org/linux-arm-msm/20240522154659.510-1-quic_grosikop@quicinc.com/ The changes are verified on SC7280 qcs6490-rb3gen2 board, the base dts for qcs6490-rb3gen2 is: https://lore.kernel.org/all/20231103184655.23555-1-quic_kbajaj@quicinc.com/ Suresh Vankadara (2): media: qcom: camss: support for camss driver on sc7280 arm64: dts: qcom: sc7280: Add support for camss Trishansh Bhardwaj (2): media: qcom: camss: support for camss driver on sc7280 arm64: dts: qcom: sc7280: Add support for camss Vikram Sharma (1): media: dt-bindings: media: camss: Add qcom,sc7280-camss binding Hariram Purshotam (3): i2c: Enable IMX577 camera sensor for qcm6490 arm64: dts: qcom: qcs6490-rb3gen2: Enable IMX577 camera sensor arm64: dts: qcom: sc7280: Add IMX577 camera sensor Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> --- Suresh Vankadara (1): media: qcom: camss: support for camss driver for sc7280 Vikram Sharma (5): media: dt-bindings: media: camss: Add qcom,sc7280-camss binding arm64: dts: qcom: sc7280: Add support for camss arm64: dts: qcom: sc7280: Add IMX577 camera sensor arm64: dts: qcom: qcs6490-rb3gen2: Enable IMX577 camera sensor i2c: Enable IMX577 camera sensor for qcm6490 .../bindings/media/qcom,sc7280-camss.yaml | 477 +++++++++++++++++++++ arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 67 +++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 215 ++++++++++ drivers/i2c/busses/i2c-qcom-cci.c | 1 + drivers/media/platform/qcom/camss/camss-csid.c | 16 +- .../platform/qcom/camss/camss-csiphy-3ph-1-0.c | 2 + drivers/media/platform/qcom/camss/camss-vfe.c | 2 + drivers/media/platform/qcom/camss/camss.c | 340 +++++++++++++++ drivers/media/platform/qcom/camss/camss.h | 2 + 9 files changed, 1119 insertions(+), 3 deletions(-) --- base-commit: 18eeb2d92baca167809cd5d48eb60e0a5c036d51 change-id: 20240627-camss_first_post_linux_next-f4163c90093c Best regards,