From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.remote-ip=100.103.45.18 ARC-Seal: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=none; t=1793105858; b=32ig/YH3NctxQ2nO7n1xmR2LBlSgbstzGkJL7VX6mC9lM3fc9aJ2+FWWAr8SmU1nMg9J J6uB62UK8r4eWilQ3ePEn81NtJC73bmWlHeIMpRZBOHDjUROnW3dwdrhFgvfL99kc3Mcz lhrbXy6Uq42qP6CdA418sWZm4GY2gWogCVOp3EVQxXO8sAt39uWvlt/ezveP+cDrFwXoD Nvijd45dA80c1sPOjTlnfxCn0b7Ifp3NtBgFMg8f4ygQWy//PcX7rz/Imz16hlKPAWFxe y1g8SXTTIChZKQQe04u9+c/9LB2wWhpd0ZYR8QkaH5xtMwODxtUFmVrZCmAf7v9Zxqw== ARC-Message-Signature: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1793105858; h=Received:DKIM-Signature:Message-ID:Date:MIME-Version:User-Agent: Subject:To:Cc:References:From:In-Reply-To:Content-Type: Content-Transfer-Encoding; bh=ladm7VNw3w94qCYNsul6pq86i2SGy96Tryu3X7+mFyY=; b=23aMJRzZ+fBTZ7JoBtpzTCbu62WU7FBFxcUFPQD/gG18OuHcijUQ3jQQ3rQDBUKfjiPt DmCMhnjXU662pDKpX0w7Ve5hL8eihUEpWqmCAFY1KDlwSZ4HRvEJemWDl7T0xdhUQZ8AU e/Zjrgetj8KozjQZv6VsS3a91uNDE92fANrF41srHsLeqvf84Igpe1cbA7YUC4jCRssDg F1q/TbusdEga5ov/ams3VWNkLuao1qbv3CNRJ+p2bLw6myvhwySNkdCdUl+MDr5+N5N8Q gACrKuXVKX3QVpgZ5Th1rMOCKgKAvHgPA427Pah0JrLhG79gtQZF24eBlqoZ4mkimWw== ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=mRc9dkJP; arc=none smtp.remote-ip=100.103.45.18 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0F853606A31B; Tue, 27 Oct 2026 12:57:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key, unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=mRc9dkJP Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1BB821F01825; Tue, 27 Oct 2026 12:57:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1793105857; bh=ladm7VNw3w94qCYNsul6pq86i2SGy96Tryu3X7+mFyY=; h=Date:Subject:To:Cc:References:From:In-Reply-To; b=mRc9dkJP49MiwKf3rSOK32sJbhXthHhtbOG/JfyMsNlpq7pddc8gs2aJtUy8AY7Mh e6fQH/rpFVkUt8zIxrDEPKetT2/IVD5SDxlMNlpd4B1KjR5E/NPJRgUKtMPNeeyJUp wRqyNNMGOH8MgTooKcF+gd8ErmtonbxJKyUs5/hmKrrBE5bjsaaRYSdHsZwtWl0DVx i3IFSra+WQuJmNFXuA28aJnUOLl4TvRN2SffGsty6xKJq99kBbfX1DA/1BPL2AyAXs DEUqcdLKfPs7C4l3dOWepcqNoiFYD9dOggir6Vsr3/0u3ZWs8O58uYpDxNriGMY/SF dncx6iS/hqWuw== Message-ID: <6358dcdc-cdf4-4986-ba6b-a5cca4a093fa@kernel.org> Date: Tue, 27 Oct 2026 14:57:31 +0200 X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 4/7] arm64: dts: qcom: sm8450: Add CAMSS device tree node To: Loic Poulain Cc: Bjorn Andersson , Konrad Dybcio , Abel Vesa , Bryan O'Donoghue , Vinod Koul , Rob Herring , Krzysztof Kozlowski , linux-arm-msm@vger.kernel.org, Mauro Carvalho Chehab , linux-media@vger.kernel.org, Neil Armstrong , Manivannan Sadhasivam , linux-phy@lists.infradead.org, Conor Dooley , devicetree@vger.kernel.org References: <20261023163057.775917-1-vz@kernel.org> <20261023163057.775917-5-vz@kernel.org> <378094fb-5f7f-42c6-a91f-158e2083a6cb@kernel.org> <0d25a732-ccd5-4a7b-9809-93e67899b4e7@kernel.org> From: Vladimir Zapolskiy In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi Loic. On 10/27/26 12:31, Loic Poulain wrote: > Hi Vlad, > > > On Tue, Oct 27, 2026 at 10:54 AM Vladimir Zapolskiy wrote: >> >> On 10/27/26 11:07, Loic Poulain wrote: >>> On Mon, Oct 26, 2026 at 11:39 PM Vladimir Zapolskiy wrote: >>>> >>>> Hi Loic. >>>> >>>> On 10/26/26 18:32, Loic Poulain wrote: >>>>> On Fri, Oct 23, 2026 at 6:31 PM Vladimir Zapolskiy 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 >>>>>> --- >>>>>> 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. > > CAMSS already supports reading the hardware version from such > top-level register blocks and exposing it through the media framework. > While this is not strictly required, it will likely become useful to > libcamera in the future. Likely so, and I didn't know about the fact that "cpas_top" stores some hardware version information, which may be utilized by userspace, thank you for sharing this. Anyway at this stage, if there is no strong objections, I'd prefer to follow the conservative model of not adding "cpas_top" to CAMSS. Like it's been said, it might be preferred to create a new stand-alone CPAS device abstraction, with no conflict for I/O regions with CAMSS, especially assuming the latter one just does not need this I/O region at all to be functional. Also, I'm a bit concerned when the abstraction inversion is used as an argument, here the kernel should not depend on libcamera or other userspace component, but the opposite is to be satisfied, i.e. libcamera should work properly with 90% of CAMSS variants, which do not export "cpas_top" info, like this or many other recently added CAMSS variants. >> 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. > > Well, yes, we could certainly model it as a syscon node, especially > now that CAMSS is no longer the monolithic block it used to be. > Sure, that's the view I share to exploit the modularisation at maximum. As always, thank you very much for review! -- Best wishes, Vladimir