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
On Fri Jun 28, 2024 at 8:32 PM CEST, Vikram Sharma wrote: > 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/ Hi Vikram, Thanks for sending this patch! I just tried this on QCM6490 Fairphone 5 smartphone but unfortunately during probe something is not quite right. [ 99.531855] qcom-camss acaf000.camss: Adding to iommu group 11 [ 99.533180] ------------[ cut here ]------------ [ 99.533187] qcom-camss acaf000.camss: Error: CSID depends on VFE/IFE device ops! [ 99.533219] WARNING: CPU: 2 PID: 6902 at drivers/media/platform/qcom/camss/camss-csid.c:1024 msm_csid_subdev_init+0x41c/0x460 [qcom_camss] [ 99.533248] Modules linked in: qcom_camss(+) videobuf2_dma_sg videobuf2_memops videobuf2_v4l2 videobuf2_common [ 99.533266] CPU: 2 PID: 6902 Comm: modprobe Not tainted 6.10.0-rc5-00087-g1dd25cd60c69 #138 [ 99.533272] Hardware name: Fairphone 5 (DT) [ 99.533276] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 99.533281] pc : msm_csid_subdev_init+0x41c/0x460 [qcom_camss] [ 99.533301] lr : msm_csid_subdev_init+0x41c/0x460 [qcom_camss] [ 99.533321] sp : ffff80008575b760 [ 99.533324] x29: ffff80008575b760 x28: ffffae85fd7685d8 x27: ffff80008575bca8 [ 99.533334] x26: 0000000000002c28 x25: 0000000000000000 x24: ffff307e4c48a080 [ 99.533343] x23: ffff307e46876080 x22: 0000000000000000 x21: ffff307e40bdac10 [ 99.533352] x20: ffff307e46876080 x19: 0000000000000000 x18: fffffffffffed520 [ 99.533361] x17: 2065636976656420 x16: 4546492f45465620 x15: 6e6f2073646e6570 [ 99.533369] x14: ffffae865b46dad0 x13: 2173706f20656369 x12: 766564204546492f [ 99.533377] x11: ffffae865b46dad0 x10: 00000000000002d3 x9 : ffffae865b4c5ad0 [ 99.533386] x8 : 0000000000017fe8 x7 : 00000000fffff000 x6 : ffffae865b4c5ad0 [ 99.533394] x5 : ffff307fb6f83848 x4 : 0000000000000000 x3 : ffff81f95bd4d000 [ 99.533402] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff307e4dfe9000 [ 99.533412] Call trace: [ 99.533415] msm_csid_subdev_init+0x41c/0x460 [qcom_camss] [ 99.533435] camss_probe+0x310/0x998 [qcom_camss] [ 99.533454] platform_probe+0x68/0xe0 [ 99.533465] really_probe+0xbc/0x2c0 [ 99.533471] __driver_probe_device+0x78/0x120 [ 99.533479] driver_probe_device+0x3c/0x160 [ 99.533485] __driver_attach+0x90/0x1a0 [ 99.533490] bus_for_each_dev+0x7c/0xf0 [ 99.533497] driver_attach+0x24/0x30 [ 99.533503] bus_add_driver+0xe4/0x208 [ 99.533509] driver_register+0x68/0x124 [ 99.533514] __platform_driver_register+0x28/0x40 [ 99.533521] qcom_camss_driver_init+0x20/0x1000 [qcom_camss] [ 99.533540] do_one_initcall+0x60/0x1d4 [ 99.533547] do_init_module+0x5c/0x21c [ 99.533555] load_module+0x18b0/0x1e40 [ 99.533562] init_module_from_file+0x88/0xcc [ 99.533568] __arm64_sys_finit_module+0x174/0x340 [ 99.533575] invoke_syscall+0x48/0x10c [ 99.533582] el0_svc_common.constprop.0+0x40/0xe0 [ 99.533588] do_el0_svc+0x1c/0x34 [ 99.533594] el0_svc+0x34/0xe0 [ 99.533602] el0t_64_sync_handler+0x120/0x12c [ 99.533608] el0t_64_sync+0x190/0x194 [ 99.533613] ---[ end trace 0000000000000000 ]--- [ 99.533619] qcom-camss acaf000.camss: Failed to init csid0 sub-device: -22 [ 99.533828] qcom-camss acaf000.camss: probe with driver qcom-camss failed with error -22 My tree is based on 6.10-rc5 plus v4 of: https://lore.kernel.org/linux-arm-msm/20240522154659.510-1-quic_grosikop@quicinc.com/ plus your v1 series. And then some extra patches for my device but nothing touching camss driver. Am I missing something? Regards Luca > > 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,
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,