Re: [PATCH 4/7] arm64: dts: qcom: sm8450: Add CAMSS device tree node

[Vladimir Zapolskiy <vz@kernel.org>]

On 10/27/26 11:07, Loic Poulain wrote:
> On Mon, Oct 26, 2026 at 11:39 PM Vladimir Zapolskiy <vz@kernel.org> wrote:
>>
>> Hi Loic.
>>
>> On 10/26/26 18:32, Loic Poulain wrote:
>>> On Fri, Oct 23, 2026 at 6:31 PM Vladimir Zapolskiy <vz@kernel.org> wrote:
>>>>
>>>> Add Qualcomm Waipio CAMSS device tree node to the platform dtsi file,
>>>> the CAMSS input ports are connected to output ports of CSIPHY devices.
>>>>
>>>> Signed-off-by: Vladimir Zapolskiy <vz@kernel.org>
>>>> ---
>>>>    arch/arm64/boot/dts/qcom/sm8450.dtsi | 167 +++++++++++++++++++++++++++
>>>>    1 file changed, 167 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>> index babf1a4196a1..7f5b0501a23e 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>> @@ -3310,6 +3310,167 @@ cci1_i2c1: i2c-bus@1 {
>>>>                           };
>>>>                   };
>>>>
>>>> +               camss: isp@acb6000 {
>>>> +                       compatible = "qcom,waipio-camss";
>>>> +                       reg = <0 0xacb6000 0 0x1000>,
>>>> +                             <0 0xacb7000 0 0x1000>,
>>>> +                             <0 0xacb9000 0 0x1000>,
>>>> +                             <0 0xacbb000 0 0x1000>,
>>>> +                             <0 0xacc6000 0 0x1000>,
>>>> +                             <0 0xacca000 0 0x1000>,
>>>> +                             <0 0xacf6000 0 0x1000>,
>>>> +                             <0 0xacf7000 0 0x1000>,
>>>> +                             <0 0xacf8000 0 0x1000>,
>>>> +                             <0 0xac62000 0 0x3000>,
>>>> +                             <0 0xac71000 0 0x3000>,
>>>> +                             <0 0xac80000 0 0x3000>,
>>>> +                             <0 0xacc7000 0 0x2000>,
>>>> +                             <0 0xaccb000 0 0x2000>;
>>>> +                       reg-names = "csid_wrapper",
>>>> +                                   "csid0",
>>>> +                                   "csid1",
>>>> +                                   "csid2",
>>>> +                                   "csid_lite0",
>>>> +                                   "csid_lite1",
>>>> +                                   "tpg0",
>>>> +                                   "tpg1",
>>>> +                                   "tpg2",
>>>> +                                   "vfe0",
>>>> +                                   "vfe1",
>>>> +                                   "vfe2",
>>>> +                                   "vfe_lite0",
>>>> +                                   "vfe_lite1";
>>>
>>> Is there a camss "top" register to describe here? e.g. to retrieve the
>>> CAMSS hardware version.
>>
>> I'm not aware of anything like that. What is "CAMSS hardware version"?
>>
>> CAMSS embraces various blocks, there are CSID and VFE, full and lite
>> IPs, and all these components do export their versions individually.
> 
> I mean, CAMSS usually has a top-level register block (often top or
> cpas_top) that contains the Titan/Spectra version information, along
> with various global configuration and mux registers.

On SM8450 there is "cpas_top" register <0xac13000 0x1000> (not in the
series), I believe you reference to it.

Is CPAS a natural part of CAMSS? Can it be modelled differently, e.g.
as a separate device/driver, like it was done with JPEG or CSIPHY?

I see that there are only SM6150 and SM6350 platforms mention this
register, to capture and process frames on SM8450 there is no need
to program CPAS.

Note, that even SM6150 and SM6350 platforms do not use this register,
it makes it redundant.

> In particular, exposing this block could be useful for reporting the
> Spectra version to userspace, for example through libcamera.

It could be so, but adding dead code is an anti-pattern in software design.

> Moreover, if the goal is to describe the CAMSS hardware exhaustively
> in DT, this block should probably be represented as well. Whether it

Here I oppose such a goal as impractical, the set goal is to get working
camera on the platform with the minimum of irreversible changes. It was
proven multiple times in the past, that "an exhaustive description"
equals to "the totally broken description", and brings nothing valuable
but irrecoverable ethernal problems to overcome.

> belongs in this specific binding is open for discussion, but my
> inclination is that it does.

This register is not needed to get CAMSS up to the functionally
working stage, which is represented by the series.

I believe someone may create a new fancy CPAS device driver or so, and
the new device driver will utilize the "cpas_top" register and other
CPAS resources in a better way, the register shall not be nailed to
the CAMSS preliminarly.

-- 
Best wishes,
Vladimir