| Message ID | 20241217140656.965235-1-quic_vikramsa@quicinc.com |
|---|---|
| Headers | show |
| Series | media: qcom: camss: Add sc7280 support | expand |
On 17/12/2024 15:06, Vikram Sharma wrote: > This patch change clock names to make it consistent with > existing platforms as gcc_cam_hf_axi -> gcc_axi_hf. > This also adds gcc_axi_sf and remove gcc_camera_ahb. Don't combine ABI changes with some less important things. You miss here explanation why doing the ABI change in the first place. Without that explanation I find it rather churn. But anyway, reason for ABI break and impact should be documented in commit msg. > > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > --- Best regards, Krzysztof
On 17/12/2024 14:10, Krzysztof Kozlowski wrote: > On 17/12/2024 15:06, Vikram Sharma wrote: >> This patch change clock names to make it consistent with >> existing platforms as gcc_cam_hf_axi -> gcc_axi_hf. >> This also adds gcc_axi_sf and remove gcc_camera_ahb. > > Don't combine ABI changes with some less important things. > > You miss here explanation why doing the ABI change in the first place. > Without that explanation I find it rather churn. But anyway, reason for > ABI break and impact should be documented in commit msg. > >> >> Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> >> --- > > Best regards, > Krzysztof This change should be fine since we haven't committed and upstream dtsi, so there's no ABI to break yet. Agree the cover letter should have been explicit in explaining. --- bod
On 17/12/2024 16:23, Krzysztof Kozlowski wrote: > On 17/12/2024 17:12, Bryan O'Donoghue wrote: >> On 17/12/2024 14:10, Krzysztof Kozlowski wrote: >>> On 17/12/2024 15:06, Vikram Sharma wrote: >>>> This patch change clock names to make it consistent with >>>> existing platforms as gcc_cam_hf_axi -> gcc_axi_hf. >>>> This also adds gcc_axi_sf and remove gcc_camera_ahb. >>> >>> Don't combine ABI changes with some less important things. >>> >>> You miss here explanation why doing the ABI change in the first place. >>> Without that explanation I find it rather churn. But anyway, reason for >>> ABI break and impact should be documented in commit msg. >>> >>>> >>>> Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> >>>> --- >>> >>> Best regards, >>> Krzysztof >> >> This change should be fine since we haven't committed and upstream dtsi, >> so there's no ABI to break yet. >> >> Agree the cover letter should have been explicit in explaining. > > So these are recently added bindings (sc7280 is not particularly new)? > If so mention in the commit msg that no users are affected because of this. > > Best regards, > Krzysztof Agreed. The commit log should make clear that the ABI hasn't been cemented yet. 20241217140656.965235-4-quic_vikramsa@quicinc.com <- is still pending --- bod
On Tue, Dec 17, 2024 at 04:30:37PM +0000, Bryan O'Donoghue wrote: > On 17/12/2024 16:23, Krzysztof Kozlowski wrote: > > On 17/12/2024 17:12, Bryan O'Donoghue wrote: > > > On 17/12/2024 14:10, Krzysztof Kozlowski wrote: > > > > On 17/12/2024 15:06, Vikram Sharma wrote: > > > > > This patch change clock names to make it consistent with > > > > > existing platforms as gcc_cam_hf_axi -> gcc_axi_hf. > > > > > This also adds gcc_axi_sf and remove gcc_camera_ahb. > > > > > > > > Don't combine ABI changes with some less important things. > > > > > > > > You miss here explanation why doing the ABI change in the first place. > > > > Without that explanation I find it rather churn. But anyway, reason for > > > > ABI break and impact should be documented in commit msg. > > > > > > > > > > > > > > Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> > > > > > --- > > > > > > > > Best regards, > > > > Krzysztof > > > > > > This change should be fine since we haven't committed and upstream dtsi, > > > so there's no ABI to break yet. > > > > > > Agree the cover letter should have been explicit in explaining. > > > > So these are recently added bindings (sc7280 is not particularly new)? > > If so mention in the commit msg that no users are affected because of this. > > > > Best regards, > > Krzysztof > > Agreed. > > The commit log should make clear that the ABI hasn't been cemented yet. > > 20241217140656.965235-4-quic_vikramsa@quicinc.com <- is still pending If it hasn't been comitted yet, isn't it better to post a fixed version rather than committing a version which has known issues? > > --- > bod
On 18/12/2024 11:36, Dmitry Baryshkov wrote: >> Agreed. >> >> The commit log should make clear that the ABI hasn't been cemented yet. >> >> 20241217140656.965235-4-quic_vikramsa@quicinc.com <- is still pending > If it hasn't been comitted yet, isn't it better to post a fixed version > rather than committing a version which has known issues? Yes. - yaml - driver are merged but - dtsi is not So we can still change yaml and driver prior to fixing dtsi. --- bod
On 18/12/2024 12:17, Bryan O'Donoghue wrote: > On 18/12/2024 11:36, Dmitry Baryshkov wrote: >>> Agreed. >>> >>> The commit log should make clear that the ABI hasn't been cemented yet. >>> >>> 20241217140656.965235-4-quic_vikramsa@quicinc.com <- is still pending >> If it hasn't been comitted yet, isn't it better to post a fixed version >> rather than committing a version which has known issues? > > Yes. > > - yaml > - driver > > are merged but > > - dtsi is not > > So we can still change yaml and driver prior to fixing dtsi. > > --- > bod Excuse me; prior to _committing_ the dtsi
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 We have tested this on qcs6490-rb3gen2-vision-mezzanine board having IMX577 sensor. Verified both TPG and IMX577 sensor. Used following tools for the sanity check of these changes. - make CHECK_DTBS=y W=1 DT_SCHEMA_FILES=media/qcom,sc7280-camss.yaml qcom/qcs6490-rb3gen2-vision-mezzanine.dtb - make DT_CHECKER_FLAGS=-m W=1 DT_SCHEMA_FILES=media/qcom,sc7280-camss.yaml dt_binding_check - Smatch: make CHECK="smatch --full-path" M=drivers/media/platform/qcom/camss/ - Sparse: make C=2 M=drivers/media/platform/qcom/camss/ - coccicheck : make coccicheck M=drivers/media/platform/qcom/camss/ - make -j32 W=1 - ./scripts/checkpatch.pl Changes in V10: - Updated cover letter to add link for v8 under changes in v9. - No change in the patches w.r.t V9 - Link to v9: https://lore.kernel.org/linux-arm-msm/20241217133955.946426-1-quic_vikramsa@quicinc.com/ Changes in V9: - Removed GCC_CAMERA_AHB_CLK as its always enabled. - Added GCC_CAMERA_SF_AXI_CLK. - Renamed gcc_cam_hf_axi to gcc_axi_hf. - V8 had 5 patches and V9 have 4 patches. - First 3 patches of V8 are already promoted to linux-next i.e media: dt-bindings: Add qcom,sc7280-camss media: qcom: camss: Sort camss version enums and compatible strings media: qcom: camss: Add support for camss driver on sc7280 - 2 new patches are added to handle new comments from Konrad on "Patch v8 4/5 arm64: dts: qcom: sc7280: Add support for camss" 1 of the 2 new patches make changes in yaml and other one is making change in camss driver to handle new comments in dtsi. - for "Patch v8 4/5 arm64: dts: qcom: sc7280: Add support for camss" I got comments from Konrad to make changes for clock names so I had to make respective changes in "bindings/media/qcom,sc7280-camss.yaml". As dtsi changes are not merged yet, so there is no issues with backward compatibility and I am assuming this should be acceptable. - Link to v8: https://lore.kernel.org/linux-arm-msm/20241206191900.2545069-1-quic_vikramsa@quicinc.com/ Changes in V8: - Changed node name from camss to isp. - Added QCOM_ICC_TAG_ACTIVE_ONLY and QCOM_ICC_TAG_ALWAYS tags for interconnects. - Added blank lines when required. - Modified power-domain-names from horizontal to vertical list. - Sorted pinctrl nodes based on gpio index. - Link to v7: https://lore.kernel.org/linux-arm-msm/20241204100003.300123-1-quic_vikramsa@quicinc.com/ Changes in V7: - Changed unit address for camss in documention and dts. - Added avdd-supply and dvdd-supply for sensor. - Changed reg/clocks/interrupts name for vfe_lite and csid_lite. - Link to v6: https://lore.kernel.org/linux-arm-msm/20241127100421.3447601-1-quic_vikramsa@quicinc.com/ Changes in V6: - Changed order of properties in Documentation [PATCH 1/5]. - Updated description for ports in Documentaion [PATCH 1/5]. - Moved regulators from csid to csiphy [PATCH 3/5]. - Link to v5: https://lore.kernel.org/linux-arm-msm/20241112173032.2740119-1-quic_vikramsa@quicinc.com/ Changes in V5: - Updated Commit text for [PATCH v5 1/6]. - Moved reg after compatible string. - Renamed csi'x' clocks to vfe'x'_csid - Removed [PATCH v4 4/6] and raised a seprate series for this one. - Moved gpio states to mezzanine dtso. - Added more clock levels to address TPG related issues. - Renamed power-domains-names -> power-domain-names. - Link to v4: https://lore.kernel.org/linux-arm-msm/20241030105347.2117034-1-quic_vikramsa@quicinc.com/ Changes in V4: - V3 had 8 patches and V4 is reduced to 6. - Removed [Patch v3 2/8] as binding change is not required for dtso. - Removed [Patch v3 3/8] as the fix is already taken care in latest kernel tip. - Updated alignment for dtsi and dt-bindings. - Adding qcs6490-rb3gen2-vision-mezzanine as overlay. - Link to v3: https://lore.kernel.org/linux-arm-msm/20241011140932.1744124-1-quic_vikramsa@quicinc.com/ Changes in V3: - Added missed subject line for cover letter of V2. - Updated Alignment, indentation and properties order. - edit commit text for [PATCH 02/10] and [PATCH 03/10]. - Refactor camss_link_entities. - Removed camcc enablement changes as it already done. - Link to v2: https://lore.kernel.org/linux-arm-msm/20240904-camss_on_sc7280_rb3gen2_vision_v2_patches-v1-0-b18ddcd7d9df@quicinc.com/ Changes in V2: - Improved indentation/formatting. - Removed _src clocks and misleading code comments. - Added name fields for power domains and csid register offset in DTSI. - Dropped minItems field from YAML file. - Listed changes in alphabetical order. - Updated description and commit text to reflect changes - Changed the compatible string from imx412 to imx577. - Added board-specific enablement changes in the newly created vision board DTSI file. - Fixed bug encountered during testing. - Moved logically independent changes to a new/seprate patch. - Removed cci0 as no sensor is on this port and MCLK2, which was a copy-paste error from the RB5 board reference. - Added power rails, referencing the RB5 board. - Discarded Patch 5/6 completely (not required). - Removed unused enums. - Link to v1: https://lore.kernel.org/linux-arm-msm/20240629-camss_first_post_linux_next-v1-0-bc798edabc3a@quicinc.com/ Signed-off-by: Vikram Sharma <quic_vikramsa@quicinc.com> Vikram Sharma (4): media: dt-bindings: update clocks for sc7280-camss media: qcom: camss: update clock names for sc7280 arm64: dts: qcom: sc7280: Add support for camss arm64: dts: qcom: qcs6490-rb3gen2-vision-mezzanine: Add vision mezzanine .../bindings/media/qcom,sc7280-camss.yaml | 10 +- arch/arm64/boot/dts/qcom/Makefile | 4 + .../qcs6490-rb3gen2-vision-mezzanine.dtso | 109 +++++++++++ arch/arm64/boot/dts/qcom/sc7280.dtsi | 178 ++++++++++++++++++ drivers/media/platform/qcom/camss/camss.c | 15 +- 5 files changed, 306 insertions(+), 10 deletions(-) create mode 100644 arch/arm64/boot/dts/qcom/qcs6490-rb3gen2-vision-mezzanine.dtso