Re: [PATCH v3 3/4] arm64: dts: qcom: sm8250: Add JPEG encoder node

From: Dmitry Baryshkov

Date: Mon Jun 29 2026 - 09:08:41 EST


On Mon, Jun 29, 2026 at 03:17:49PM +0300, Atanas Filipov wrote:
> Add the JPEG encoder hardware node to the SM8250 device tree so the
> qcom-jpeg V4L2 encoder driver can bind and operate on this platform.
>
> Signed-off-by: Atanas Filipov <atanas.filipov@xxxxxxxxxxxxxxxx>
> ---
> arch/arm64/boot/dts/qcom/sm8250.dtsi | 78 ++++++++++++++++++++++++++--
> 1 file changed, 74 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 7076720413ab..d0cacb4ec35b 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -4472,6 +4472,9 @@ cci1_i2c1: i2c-bus@1 {
> camss: camss@ac6a000 {
> compatible = "qcom,sm8250-camss";
> status = "disabled";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;

Status should be the last property, add empty line before it.

>
> reg = <0 0x0ac6a000 0 0x2000>,
> <0 0x0ac6c000 0 0x2000>,
> @@ -4616,10 +4619,10 @@ camss: camss@ac6a000 {
> <&mmss_noc MASTER_CAMNOC_HF 0 &mc_virt SLAVE_EBI_CH0 0>,
> <&mmss_noc MASTER_CAMNOC_SF 0 &mc_virt SLAVE_EBI_CH0 0>,
> <&mmss_noc MASTER_CAMNOC_ICP 0 &mc_virt SLAVE_EBI_CH0 0>;
> - interconnect-names = "cam_ahb",
> - "cam_hf_0_mnoc",
> - "cam_sf_0_mnoc",
> - "cam_sf_icp_mnoc";
> + interconnect-names = "ahb",
> + "hf_mnoc",
> + "sf_mnoc",
> + "icp_mnoc";

No, that's a part of the ABI, you can't just change those.

>
> ports {
> #address-cells = <1>;
> @@ -4649,6 +4652,73 @@ port@5 {
> reg = <5>;
> };
> };
> +
> + qcom_jpeg_enc: jpeg-encoder@ac53000 {

Unused label. Drop it.

> + compatible = "qcom,sm8250-jenc";
> +
> + reg = <0 0xac53000 0 0x1000>;

0x0, pad address to 8 digits after 0x.

> +
> + interrupts = <GIC_SPI 474 IRQ_TYPE_EDGE_RISING>;
> + power-domains = <&camcc TITAN_TOP_GDSC>;

This should not be necessary, if I'm not mistaken. All devices inside
camss are part of the TITAN_TOP_GDSC per design. Handle it in the driver
if required.

> +
> + clocks = <&gcc GCC_CAMERA_HF_AXI_CLK>,
> + <&gcc GCC_CAMERA_SF_AXI_CLK>,
> + <&camcc CAM_CC_CORE_AHB_CLK>,
> + <&camcc CAM_CC_CPAS_AHB_CLK>,
> + <&camcc CAM_CC_CAMNOC_AXI_CLK>,
> + <&camcc CAM_CC_JPEG_CLK>;
> +
> + clock-names = "hf_axi",
> + "sf_axi",
> + "core_ahb",
> + "cpas_ahb",
> + "cnoc_axi",
> + "jpeg";
> +
> + iommus = <&apps_smmu 0x2040 0x400>,
> + <&apps_smmu 0x2440 0x400>;

These two entries result in exactly the same base & mask. Why do you
need to duplicate them?

> +
> + interconnects =
> + <&gem_noc MASTER_AMPSS_M0 0 &config_noc SLAVE_CAMERA_CFG 0>,

Move to the previous line, use proper tags instead of 0.

> + <&mmss_noc MASTER_CAMNOC_HF 0 &mc_virt SLAVE_EBI_CH0 0>,
> + <&mmss_noc MASTER_CAMNOC_SF 0 &mc_virt SLAVE_EBI_CH0 0>,
> + <&mmss_noc MASTER_CAMNOC_ICP 0 &mc_virt SLAVE_EBI_CH0 0>;
> +
> + interconnect-names = "cpu-cfg",
> + "hf-mnoc",
> + "sf-mnoc",
> + "icp-mnoc";
> +
> + operating-points-v2 = <&jpeg_opp_table>;
> +
> + jpeg_opp_table: opp-table {
> + compatible = "operating-points-v2";
> +
> + opp-300000000 {
> + opp-hz = /bits/ 64 <300000000>;
> + opp-level = <0>;
> + required-opps = <&rpmhpd_opp_svs>;
> + };
> +
> + opp-400000000 {
> + opp-hz = /bits/ 64 <400000000>;
> + opp-level = <1>;
> + required-opps = <&rpmhpd_opp_svs>;
> + };
> +
> + opp-480000000 {
> + opp-hz = /bits/ 64 <480000000>;
> + opp-level = <2>;
> + required-opps = <&rpmhpd_opp_svs_l1>;
> + };
> +
> + opp-600000000 {
> + opp-hz = /bits/ 64 <600000000>;
> + opp-level = <3>;
> + required-opps = <&rpmhpd_opp_nom>;
> + };
> + };
> + };
> };
>
> camcc: clock-controller@ad00000 {
> --
> 2.34.1
>

--
With best wishes
Dmitry