diff mbox series

[RESEND] arm64: dts: qcom: sm8550: Add support for camss

Message ID 20250516072707.388332-1-quic_wenmliu@quicinc.com
State Superseded
Headers show
Series [RESEND] arm64: dts: qcom: sm8550: Add support for camss | expand

Commit Message

Wenmeng Liu May 16, 2025, 7:27 a.m. UTC
Add support for the camera subsystem on the SM8550 Qualcomm SoC. This
includes bringing up the CSIPHY, CSID, VFE/RDI interfaces.

SM8550 provides
- 3 x VFE, 3 RDI per VFE
- 2 x VFE Lite, 4 RDI per VFE
- 3 x CSID
- 2 x CSID Lite
- 8 x CSI PHY

Co-developed-by: Depeng Shao <quic_depengs@quicinc.com>
Signed-off-by: Depeng Shao <quic_depengs@quicinc.com>
Signed-off-by: Wenmeng Liu <quic_wenmliu@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8550.dtsi | 210 +++++++++++++++++++++++++++
 1 file changed, 210 insertions(+)

Comments

Bryan O'Donoghue May 17, 2025, 12:07 p.m. UTC | #1
On 16/05/2025 08:27, Wenmeng Liu wrote:
> Add support for the camera subsystem on the SM8550 Qualcomm SoC. This
> includes bringing up the CSIPHY, CSID, VFE/RDI interfaces.
> 
> SM8550 provides
> - 3 x VFE, 3 RDI per VFE
> - 2 x VFE Lite, 4 RDI per VFE
> - 3 x CSID
> - 2 x CSID Lite
> - 8 x CSI PHY
> 
> Co-developed-by: Depeng Shao <quic_depengs@quicinc.com>
> Signed-off-by: Depeng Shao <quic_depengs@quicinc.com>
> Signed-off-by: Wenmeng Liu <quic_wenmliu@quicinc.com>
> ---
>   arch/arm64/boot/dts/qcom/sm8550.dtsi | 210 +++++++++++++++++++++++++++
>   1 file changed, 210 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> index e9bb077aa9f0..722521496a2d 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> @@ -3326,6 +3326,216 @@ cci2_i2c1: i2c-bus@1 {
>   			};
>   		};
>   
> +		isp: isp@acb7000 {
> +			compatible = "qcom,sm8550-camss";
> +
> +			reg = <0x0 0x0acb7000 0x0 0x0d00>,
> +			      <0x0 0x0acb9000 0x0 0x0d00>,
> +			      <0x0 0x0acbb000 0x0 0x0d00>,
> +			      <0x0 0x0acca000 0x0 0x0a00>,
> +			      <0x0 0x0acce000 0x0 0x0a00>,
> +			      <0x0 0x0acb6000 0x0 0x1000>,
> +			      <0x0 0x0ace4000 0x0 0x2000>,
> +			      <0x0 0x0ace6000 0x0 0x2000>,
> +			      <0x0 0x0ace8000 0x0 0x2000>,
> +			      <0x0 0x0acea000 0x0 0x2000>,
> +			      <0x0 0x0acec000 0x0 0x2000>,
> +			      <0x0 0x0acee000 0x0 0x2000>,
> +			      <0x0 0x0acf0000 0x0 0x2000>,
> +			      <0x0 0x0acf2000 0x0 0x2000>,
> +			      <0x0 0x0ac62000 0x0 0xf000>,
> +			      <0x0 0x0ac71000 0x0 0xf000>,
> +			      <0x0 0x0ac80000 0x0 0xf000>,
> +			      <0x0 0x0accb000 0x0 0x1800>,
> +			      <0x0 0x0accf000 0x0 0x1800>;
> +			reg-names = "csid0",
> +				    "csid1",
> +				    "csid2",
> +				    "csid_lite0",
> +				    "csid_lite1",
> +				    "csid_wrapper",
> +				    "csiphy0",
> +				    "csiphy1",
> +				    "csiphy2",
> +				    "csiphy3",
> +				    "csiphy4",
> +				    "csiphy5",
> +				    "csiphy6",
> +				    "csiphy7",
> +				    "vfe0",
> +				    "vfe1",
> +				    "vfe2",
> +				    "vfe_lite0",
> +				    "vfe_lite1";
> +
> +			clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
> +				 <&camcc CAM_CC_CPAS_AHB_CLK>,
> +				 <&camcc CAM_CC_CPAS_FAST_AHB_CLK>,
> +				 <&camcc CAM_CC_CPAS_IFE_LITE_CLK>,
> +				 <&camcc CAM_CC_CPAS_IFE_0_CLK>,
> +				 <&camcc CAM_CC_CPAS_IFE_1_CLK>,
> +				 <&camcc CAM_CC_CPAS_IFE_2_CLK>,
> +				 <&camcc CAM_CC_CSID_CLK>,
> +				 <&camcc CAM_CC_CSIPHY0_CLK>,
> +				 <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CSIPHY1_CLK>,
> +				 <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CSIPHY2_CLK>,
> +				 <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CSIPHY3_CLK>,
> +				 <&camcc CAM_CC_CSI3PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CSIPHY4_CLK>,
> +				 <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CSIPHY5_CLK>,
> +				 <&camcc CAM_CC_CSI5PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CSIPHY6_CLK>,
> +				 <&camcc CAM_CC_CSI6PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CSIPHY7_CLK>,
> +				 <&camcc CAM_CC_CSI7PHYTIMER_CLK>,
> +				 <&camcc CAM_CC_CSID_CSIPHY_RX_CLK>,
> +				 <&gcc GCC_CAMERA_HF_AXI_CLK>,
> +				 <&camcc CAM_CC_IFE_0_CLK>,
> +				 <&camcc CAM_CC_IFE_0_FAST_AHB_CLK>,
> +				 <&camcc CAM_CC_IFE_1_CLK>,
> +				 <&camcc CAM_CC_IFE_1_FAST_AHB_CLK>,
> +				 <&camcc CAM_CC_IFE_2_CLK>,
> +				 <&camcc CAM_CC_IFE_2_FAST_AHB_CLK>,
> +				 <&camcc CAM_CC_IFE_LITE_CLK>,
> +				 <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
> +				 <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
> +				 <&camcc CAM_CC_IFE_LITE_CSID_CLK>;
> +			clock-names = "camnoc_axi",
> +				      "cpas_ahb",
> +				      "cpas_fast_ahb_clk",
> +				      "cpas_ife_lite",
> +				      "cpas_vfe0",
> +				      "cpas_vfe1",
> +				      "cpas_vfe2",
> +				      "csid",
> +				      "csiphy0",
> +				      "csiphy0_timer",
> +				      "csiphy1",
> +				      "csiphy1_timer",
> +				      "csiphy2",
> +				      "csiphy2_timer",
> +				      "csiphy3",
> +				      "csiphy3_timer",
> +				      "csiphy4",
> +				      "csiphy4_timer",
> +				      "csiphy5",
> +				      "csiphy5_timer",
> +				      "csiphy6",
> +				      "csiphy6_timer",
> +				      "csiphy7",
> +				      "csiphy7_timer",
> +				      "csiphy_rx",
> +				      "gcc_axi_hf",
> +				      "vfe0",
> +				      "vfe0_fast_ahb",
> +				      "vfe1",
> +				      "vfe1_fast_ahb",
> +				      "vfe2",
> +				      "vfe2_fast_ahb",
> +				      "vfe_lite",
> +				      "vfe_lite_ahb",
> +				      "vfe_lite_cphy_rx",
> +				      "vfe_lite_csid";
> +
> +			interrupts = <GIC_SPI 601 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 603 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 431 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 605 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 376 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 448 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 122 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 89 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 278 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 277 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 602 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 604 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 688 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 606 IRQ_TYPE_EDGE_RISING>,
> +				     <GIC_SPI 377 IRQ_TYPE_EDGE_RISING>;
> +			interrupt-names = "csid0",
> +					  "csid1",
> +					  "csid2",
> +					  "csid_lite0",
> +					  "csid_lite1",
> +					  "csiphy0",
> +					  "csiphy1",
> +					  "csiphy2",
> +					  "csiphy3",
> +					  "csiphy4",
> +					  "csiphy5",
> +					  "csiphy6",
> +					  "csiphy7",
> +					  "vfe0",
> +					  "vfe1",
> +					  "vfe2",
> +					  "vfe_lite0",
> +					  "vfe_lite1";
> +
> +			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
> +					 &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
> +					<&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS
> +					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
> +			interconnect-names = "ahb",
> +					     "hf_0_mnoc";
> +
> +			iommus = <&apps_smmu 0x800 0x20>;
> +
> +			power-domains = <&camcc CAM_CC_IFE_0_GDSC>,
> +					<&camcc CAM_CC_IFE_1_GDSC>,
> +					<&camcc CAM_CC_IFE_2_GDSC>,
> +					<&camcc CAM_CC_TITAN_TOP_GDSC>;
> +			power-domain-names = "ife0",
> +					     "ife1",
> +					     "ife2",
> +					     "top";
> +
> +			status = "disabled";
> +
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;
> +				};
> +
> +				port@1 {
> +					reg = <1>;
> +				};
> +
> +				port@2 {
> +					reg = <2>;
> +				};
> +
> +				port@3 {
> +					reg = <3>;
> +				};
> +
> +				port@4 {
> +					reg = <4>;
> +				};
> +
> +				port@5 {
> +					reg = <5>;
> +				};
> +
> +				port@6 {
> +					reg = <6>;
> +				};
> +
> +				port@7 {
> +					reg = <7>;
> +				};
> +			};
> +		};
> +
>   		camcc: clock-controller@ade0000 {
>   			compatible = "qcom,sm8550-camcc";
>   			reg = <0 0x0ade0000 0 0x20000>;

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Konrad Dybcio May 21, 2025, 7:02 p.m. UTC | #2
On 5/16/25 9:27 AM, Wenmeng Liu wrote:
> Add support for the camera subsystem on the SM8550 Qualcomm SoC. This
> includes bringing up the CSIPHY, CSID, VFE/RDI interfaces.
> 
> SM8550 provides
> - 3 x VFE, 3 RDI per VFE
> - 2 x VFE Lite, 4 RDI per VFE
> - 3 x CSID
> - 2 x CSID Lite
> - 8 x CSI PHY
> 
> Co-developed-by: Depeng Shao <quic_depengs@quicinc.com>
> Signed-off-by: Depeng Shao <quic_depengs@quicinc.com>
> Signed-off-by: Wenmeng Liu <quic_wenmliu@quicinc.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad
Vladimir Zapolskiy June 10, 2025, 9:48 a.m. UTC | #3
Hello Wenmeng.

On 5/16/25 10:27, Wenmeng Liu wrote:
> Add support for the camera subsystem on the SM8550 Qualcomm SoC. This
> includes bringing up the CSIPHY, CSID, VFE/RDI interfaces.
> 
> SM8550 provides
> - 3 x VFE, 3 RDI per VFE
> - 2 x VFE Lite, 4 RDI per VFE
> - 3 x CSID
> - 2 x CSID Lite
> - 8 x CSI PHY
> 
> Co-developed-by: Depeng Shao <quic_depengs@quicinc.com>
> Signed-off-by: Depeng Shao <quic_depengs@quicinc.com>
> Signed-off-by: Wenmeng Liu <quic_wenmliu@quicinc.com>
> ---
>   arch/arm64/boot/dts/qcom/sm8550.dtsi | 210 +++++++++++++++++++++++++++
>   1 file changed, 210 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> index e9bb077aa9f0..722521496a2d 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
> @@ -3326,6 +3326,216 @@ cci2_i2c1: i2c-bus@1 {
>   			};
>   		};
>   
> +		isp: isp@acb7000 {
> +			compatible = "qcom,sm8550-camss";
> +

This is the first time, when 'isp' label is used instead of 'camss', it might
be I missed the context, is there any particular reason to do such a change?

If the label name is changed to the regular 'camss', then

Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>

--
Best wishes,
Vladimir
Bryan O'Donoghue June 10, 2025, 9:50 a.m. UTC | #4
On 10/06/2025 10:48, Vladimir Zapolskiy wrote:
> Hello Wenmeng.
> 
> On 5/16/25 10:27, Wenmeng Liu wrote:
>> Add support for the camera subsystem on the SM8550 Qualcomm SoC. This
>> includes bringing up the CSIPHY, CSID, VFE/RDI interfaces.
>>
>> SM8550 provides
>> - 3 x VFE, 3 RDI per VFE
>> - 2 x VFE Lite, 4 RDI per VFE
>> - 3 x CSID
>> - 2 x CSID Lite
>> - 8 x CSI PHY
>>
>> Co-developed-by: Depeng Shao <quic_depengs@quicinc.com>
>> Signed-off-by: Depeng Shao <quic_depengs@quicinc.com>
>> Signed-off-by: Wenmeng Liu <quic_wenmliu@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/sm8550.dtsi | 210 +++++++++++++++++++++++++++
>>   1 file changed, 210 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/ 
>> dts/qcom/sm8550.dtsi
>> index e9bb077aa9f0..722521496a2d 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
>> @@ -3326,6 +3326,216 @@ cci2_i2c1: i2c-bus@1 {
>>               };
>>           };
>> +        isp: isp@acb7000 {
>> +            compatible = "qcom,sm8550-camss";
>> +
> 
> This is the first time, when 'isp' label is used instead of 'camss', it 
> might
> be I missed the context, is there any particular reason to do such a 
> change?
> 
> If the label name is changed to the regular 'camss', then
> 
> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> 
> -- 
> Best wishes,
> Vladimir

List feedback from DT people is isp@ is the correct prefix.

---
bod
Vladimir Zapolskiy June 10, 2025, 12:49 p.m. UTC | #5
On 6/10/25 12:50, Bryan O'Donoghue wrote:
> On 10/06/2025 10:48, Vladimir Zapolskiy wrote:
>> Hello Wenmeng.
>>
>> On 5/16/25 10:27, Wenmeng Liu wrote:
>>> Add support for the camera subsystem on the SM8550 Qualcomm SoC. This
>>> includes bringing up the CSIPHY, CSID, VFE/RDI interfaces.
>>>
>>> SM8550 provides
>>> - 3 x VFE, 3 RDI per VFE
>>> - 2 x VFE Lite, 4 RDI per VFE
>>> - 3 x CSID
>>> - 2 x CSID Lite
>>> - 8 x CSI PHY
>>>
>>> Co-developed-by: Depeng Shao <quic_depengs@quicinc.com>
>>> Signed-off-by: Depeng Shao <quic_depengs@quicinc.com>
>>> Signed-off-by: Wenmeng Liu <quic_wenmliu@quicinc.com>
>>> ---
>>>    arch/arm64/boot/dts/qcom/sm8550.dtsi | 210 +++++++++++++++++++++++++++
>>>    1 file changed, 210 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/
>>> dts/qcom/sm8550.dtsi
>>> index e9bb077aa9f0..722521496a2d 100644
>>> --- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
>>> @@ -3326,6 +3326,216 @@ cci2_i2c1: i2c-bus@1 {
>>>                };
>>>            };
>>> +        isp: isp@acb7000 {
>>> +            compatible = "qcom,sm8550-camss";
>>> +
>>
>> This is the first time, when 'isp' label is used instead of 'camss', it
>> might
>> be I missed the context, is there any particular reason to do such a
>> change?
>>
>> If the label name is changed to the regular 'camss', then
>>
>> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>
>> -- 
>> Best wishes,
>> Vladimir
> 
> List feedback from DT people is isp@ is the correct prefix.
> 

My bad, but I don't understand this comment, it seems irrelevant...

The expressed concern is about the novel label name.

--
Best wishes,
Vladimir
Bryan O'Donoghue June 10, 2025, 7:02 p.m. UTC | #6
On 10/06/2025 13:49, Vladimir Zapolskiy wrote:
>>
>> List feedback from DT people is isp@ is the correct prefix.
>>
> 
> My bad, but I don't understand this comment, it seems irrelevant...
> 
> The expressed concern is about the novel label name.

I mean to say the feedback from Krzysztof was that we should use isp@ 
not camss@ and I agree.

---
bod
Vladimir Zapolskiy June 10, 2025, 9:02 p.m. UTC | #7
On 6/10/25 22:02, Bryan O'Donoghue wrote:
> On 10/06/2025 13:49, Vladimir Zapolskiy wrote:
>>>
>>> List feedback from DT people is isp@ is the correct prefix.
>>>
>>
>> My bad, but I don't understand this comment, it seems irrelevant...
>>
>> The expressed concern is about the novel label name.
> 
> I mean to say the feedback from Krzysztof was that we should use isp@
> not camss@ and I agree.
> 

Let me repeat it thrice, it's okay...

I don't object against the properly selected device tree node name "isp",
here I object against a never used and very questionable label name "isp".

Please feel free to ask more questions, if you still find it confusing.

Again, I may missed a discussion about the need to get and use a novel
label name, then please share a link to it, it'll be very much appreciated.

--
Best wishes,
Vladimir
Konrad Dybcio June 10, 2025, 9:04 p.m. UTC | #8
On 6/10/25 11:02 PM, Vladimir Zapolskiy wrote:
> On 6/10/25 22:02, Bryan O'Donoghue wrote:
>> On 10/06/2025 13:49, Vladimir Zapolskiy wrote:
>>>>
>>>> List feedback from DT people is isp@ is the correct prefix.
>>>>
>>>
>>> My bad, but I don't understand this comment, it seems irrelevant...
>>>
>>> The expressed concern is about the novel label name.
>>
>> I mean to say the feedback from Krzysztof was that we should use isp@
>> not camss@ and I agree.
>>
> 
> Let me repeat it thrice, it's okay...
> 
> I don't object against the properly selected device tree node name "isp",
> here I object against a never used and very questionable label name "isp".
> 
> Please feel free to ask more questions, if you still find it confusing.
> 
> Again, I may missed a discussion about the need to get and use a novel
> label name, then please share a link to it, it'll be very much appreciated.

To hopefully help out:

label: node-name@unit-address {
	property = value;
};

Konrad
Vladimir Zapolskiy June 10, 2025, 9:13 p.m. UTC | #9
Hi Konrad.

On 6/11/25 00:04, Konrad Dybcio wrote:
> On 6/10/25 11:02 PM, Vladimir Zapolskiy wrote:
>> On 6/10/25 22:02, Bryan O'Donoghue wrote:
>>> On 10/06/2025 13:49, Vladimir Zapolskiy wrote:
>>>>>
>>>>> List feedback from DT people is isp@ is the correct prefix.
>>>>>
>>>>
>>>> My bad, but I don't understand this comment, it seems irrelevant...
>>>>
>>>> The expressed concern is about the novel label name.
>>>
>>> I mean to say the feedback from Krzysztof was that we should use isp@
>>> not camss@ and I agree.
>>>
>>
>> Let me repeat it thrice, it's okay...
>>
>> I don't object against the properly selected device tree node name "isp",
>> here I object against a never used and very questionable label name "isp".
>>
>> Please feel free to ask more questions, if you still find it confusing.
>>
>> Again, I may missed a discussion about the need to get and use a novel
>> label name, then please share a link to it, it'll be very much appreciated.
> 
> To hopefully help out:
> 
> label: node-name@unit-address {
> 	property = value;
> };
> 

Thank you, here is a link to the wanted section of the dt specification
for Bryan's comprehension:

* https://github.com/devicetree-org/devicetree-specification/blob/main/source/chapter6-source-language.rst.

If for whatever reason a proposed "isp" label is preferred, then
since a label rename is not an ABI change, it would make sense to
do a massive change of renaming all camss labels. Otherwise there will
be an outstanding incorrespondence/confusion of the label names in
board .dts files, and that's bad.

--
Best wishes,
Vladimir
Bryan O'Donoghue June 10, 2025, 10:17 p.m. UTC | #10
On 10/06/2025 22:13, Vladimir Zapolskiy wrote:
> Hi Konrad.
> 
> On 6/11/25 00:04, Konrad Dybcio wrote:
>> On 6/10/25 11:02 PM, Vladimir Zapolskiy wrote:
>>> On 6/10/25 22:02, Bryan O'Donoghue wrote:
>>>> On 10/06/2025 13:49, Vladimir Zapolskiy wrote:
>>>>>>
>>>>>> List feedback from DT people is isp@ is the correct prefix.
>>>>>>
>>>>>
>>>>> My bad, but I don't understand this comment, it seems irrelevant...
>>>>>
>>>>> The expressed concern is about the novel label name.
>>>>
>>>> I mean to say the feedback from Krzysztof was that we should use isp@
>>>> not camss@ and I agree.
>>>>
>>>
>>> Let me repeat it thrice, it's okay...
>>>
>>> I don't object against the properly selected device tree node name 
>>> "isp",
>>> here I object against a never used and very questionable label name 
>>> "isp".
>>>
>>> Please feel free to ask more questions, if you still find it confusing.
>>>
>>> Again, I may missed a discussion about the need to get and use a novel
>>> label name, then please share a link to it, it'll be very much 
>>> appreciated.
>>
>> To hopefully help out:
>>
>> label: node-name@unit-address {
>>     property = value;
>> };
>>
> 
> Thank you, here is a link to the wanted section of the dt specification
> for Bryan's comprehension:
> 
> * https://github.com/devicetree-org/devicetree-specification/blob/main/ 
> source/chapter6-source-language.rst.
> 
> If for whatever reason a proposed "isp" label is preferred, then
> since a label rename is not an ABI change, it would make sense to
> do a massive change of renaming all camss labels. Otherwise there will
> be an outstanding incorrespondence/confusion of the label names in
> board .dts files, and that's bad.
> 
> -- 
> Best wishes,
> Vladimir

Ah the label, I thought you meant node.

camss: isp@value

Yes
---
bod
Vladimir Zapolskiy June 10, 2025, 10:52 p.m. UTC | #11
On 6/11/25 01:17, Bryan O'Donoghue wrote:
> On 10/06/2025 22:13, Vladimir Zapolskiy wrote:
>> Hi Konrad.
>>
>> On 6/11/25 00:04, Konrad Dybcio wrote:
>>> On 6/10/25 11:02 PM, Vladimir Zapolskiy wrote:
>>>> On 6/10/25 22:02, Bryan O'Donoghue wrote:
>>>>> On 10/06/2025 13:49, Vladimir Zapolskiy wrote:
>>>>>>>
>>>>>>> List feedback from DT people is isp@ is the correct prefix.
>>>>>>>
>>>>>>
>>>>>> My bad, but I don't understand this comment, it seems irrelevant...
>>>>>>
>>>>>> The expressed concern is about the novel label name.
>>>>>
>>>>> I mean to say the feedback from Krzysztof was that we should use isp@
>>>>> not camss@ and I agree.
>>>>>
>>>>
>>>> Let me repeat it thrice, it's okay...
>>>>
>>>> I don't object against the properly selected device tree node name
>>>> "isp",
>>>> here I object against a never used and very questionable label name
>>>> "isp".
>>>>
>>>> Please feel free to ask more questions, if you still find it confusing.
>>>>
>>>> Again, I may missed a discussion about the need to get and use a novel
>>>> label name, then please share a link to it, it'll be very much
>>>> appreciated.
>>>
>>> To hopefully help out:
>>>
>>> label: node-name@unit-address {
>>>      property = value;
>>> };
>>>
>>
>> Thank you, here is a link to the wanted section of the dt specification
>> for Bryan's comprehension:
>>
>> * https://github.com/devicetree-org/devicetree-specification/blob/main/
>> source/chapter6-source-language.rst.
>>
>> If for whatever reason a proposed "isp" label is preferred, then
>> since a label rename is not an ABI change, it would make sense to
>> do a massive change of renaming all camss labels. Otherwise there will
>> be an outstanding incorrespondence/confusion of the label names in
>> board .dts files, and that's bad.
>>
>> -- 
>> Best wishes,
>> Vladimir
> 
> Ah the label, I thought you meant node.

I'm trying to do my best in expressing myself by means of the second
signaling system. As an example when I write "a label" repeatedly, I mean
to transmit "a label" symbol, hence I hope it should not be overly
complicated to understand me.

It's great that the understanding is reached now, it would be better,
if we can save some time in future.

There is no bug introduced in this particular change, however it shall be
fixed and be resubmitted.

--
Best wishes,
Vladimir
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index e9bb077aa9f0..722521496a2d 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -3326,6 +3326,216 @@  cci2_i2c1: i2c-bus@1 {
 			};
 		};
 
+		isp: isp@acb7000 {
+			compatible = "qcom,sm8550-camss";
+
+			reg = <0x0 0x0acb7000 0x0 0x0d00>,
+			      <0x0 0x0acb9000 0x0 0x0d00>,
+			      <0x0 0x0acbb000 0x0 0x0d00>,
+			      <0x0 0x0acca000 0x0 0x0a00>,
+			      <0x0 0x0acce000 0x0 0x0a00>,
+			      <0x0 0x0acb6000 0x0 0x1000>,
+			      <0x0 0x0ace4000 0x0 0x2000>,
+			      <0x0 0x0ace6000 0x0 0x2000>,
+			      <0x0 0x0ace8000 0x0 0x2000>,
+			      <0x0 0x0acea000 0x0 0x2000>,
+			      <0x0 0x0acec000 0x0 0x2000>,
+			      <0x0 0x0acee000 0x0 0x2000>,
+			      <0x0 0x0acf0000 0x0 0x2000>,
+			      <0x0 0x0acf2000 0x0 0x2000>,
+			      <0x0 0x0ac62000 0x0 0xf000>,
+			      <0x0 0x0ac71000 0x0 0xf000>,
+			      <0x0 0x0ac80000 0x0 0xf000>,
+			      <0x0 0x0accb000 0x0 0x1800>,
+			      <0x0 0x0accf000 0x0 0x1800>;
+			reg-names = "csid0",
+				    "csid1",
+				    "csid2",
+				    "csid_lite0",
+				    "csid_lite1",
+				    "csid_wrapper",
+				    "csiphy0",
+				    "csiphy1",
+				    "csiphy2",
+				    "csiphy3",
+				    "csiphy4",
+				    "csiphy5",
+				    "csiphy6",
+				    "csiphy7",
+				    "vfe0",
+				    "vfe1",
+				    "vfe2",
+				    "vfe_lite0",
+				    "vfe_lite1";
+
+			clocks = <&camcc CAM_CC_CAMNOC_AXI_CLK>,
+				 <&camcc CAM_CC_CPAS_AHB_CLK>,
+				 <&camcc CAM_CC_CPAS_FAST_AHB_CLK>,
+				 <&camcc CAM_CC_CPAS_IFE_LITE_CLK>,
+				 <&camcc CAM_CC_CPAS_IFE_0_CLK>,
+				 <&camcc CAM_CC_CPAS_IFE_1_CLK>,
+				 <&camcc CAM_CC_CPAS_IFE_2_CLK>,
+				 <&camcc CAM_CC_CSID_CLK>,
+				 <&camcc CAM_CC_CSIPHY0_CLK>,
+				 <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
+				 <&camcc CAM_CC_CSIPHY1_CLK>,
+				 <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
+				 <&camcc CAM_CC_CSIPHY2_CLK>,
+				 <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
+				 <&camcc CAM_CC_CSIPHY3_CLK>,
+				 <&camcc CAM_CC_CSI3PHYTIMER_CLK>,
+				 <&camcc CAM_CC_CSIPHY4_CLK>,
+				 <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
+				 <&camcc CAM_CC_CSIPHY5_CLK>,
+				 <&camcc CAM_CC_CSI5PHYTIMER_CLK>,
+				 <&camcc CAM_CC_CSIPHY6_CLK>,
+				 <&camcc CAM_CC_CSI6PHYTIMER_CLK>,
+				 <&camcc CAM_CC_CSIPHY7_CLK>,
+				 <&camcc CAM_CC_CSI7PHYTIMER_CLK>,
+				 <&camcc CAM_CC_CSID_CSIPHY_RX_CLK>,
+				 <&gcc GCC_CAMERA_HF_AXI_CLK>,
+				 <&camcc CAM_CC_IFE_0_CLK>,
+				 <&camcc CAM_CC_IFE_0_FAST_AHB_CLK>,
+				 <&camcc CAM_CC_IFE_1_CLK>,
+				 <&camcc CAM_CC_IFE_1_FAST_AHB_CLK>,
+				 <&camcc CAM_CC_IFE_2_CLK>,
+				 <&camcc CAM_CC_IFE_2_FAST_AHB_CLK>,
+				 <&camcc CAM_CC_IFE_LITE_CLK>,
+				 <&camcc CAM_CC_IFE_LITE_AHB_CLK>,
+				 <&camcc CAM_CC_IFE_LITE_CPHY_RX_CLK>,
+				 <&camcc CAM_CC_IFE_LITE_CSID_CLK>;
+			clock-names = "camnoc_axi",
+				      "cpas_ahb",
+				      "cpas_fast_ahb_clk",
+				      "cpas_ife_lite",
+				      "cpas_vfe0",
+				      "cpas_vfe1",
+				      "cpas_vfe2",
+				      "csid",
+				      "csiphy0",
+				      "csiphy0_timer",
+				      "csiphy1",
+				      "csiphy1_timer",
+				      "csiphy2",
+				      "csiphy2_timer",
+				      "csiphy3",
+				      "csiphy3_timer",
+				      "csiphy4",
+				      "csiphy4_timer",
+				      "csiphy5",
+				      "csiphy5_timer",
+				      "csiphy6",
+				      "csiphy6_timer",
+				      "csiphy7",
+				      "csiphy7_timer",
+				      "csiphy_rx",
+				      "gcc_axi_hf",
+				      "vfe0",
+				      "vfe0_fast_ahb",
+				      "vfe1",
+				      "vfe1_fast_ahb",
+				      "vfe2",
+				      "vfe2_fast_ahb",
+				      "vfe_lite",
+				      "vfe_lite_ahb",
+				      "vfe_lite_cphy_rx",
+				      "vfe_lite_csid";
+
+			interrupts = <GIC_SPI 601 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 603 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 431 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 605 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 376 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 448 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 122 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 89 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 278 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 277 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 602 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 604 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 688 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 606 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 377 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "csid0",
+					  "csid1",
+					  "csid2",
+					  "csid_lite0",
+					  "csid_lite1",
+					  "csiphy0",
+					  "csiphy1",
+					  "csiphy2",
+					  "csiphy3",
+					  "csiphy4",
+					  "csiphy5",
+					  "csiphy6",
+					  "csiphy7",
+					  "vfe0",
+					  "vfe1",
+					  "vfe2",
+					  "vfe_lite0",
+					  "vfe_lite1";
+
+			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
+					 &config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
+					<&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS
+					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
+			interconnect-names = "ahb",
+					     "hf_0_mnoc";
+
+			iommus = <&apps_smmu 0x800 0x20>;
+
+			power-domains = <&camcc CAM_CC_IFE_0_GDSC>,
+					<&camcc CAM_CC_IFE_1_GDSC>,
+					<&camcc CAM_CC_IFE_2_GDSC>,
+					<&camcc CAM_CC_TITAN_TOP_GDSC>;
+			power-domain-names = "ife0",
+					     "ife1",
+					     "ife2",
+					     "top";
+
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+				};
+
+				port@1 {
+					reg = <1>;
+				};
+
+				port@2 {
+					reg = <2>;
+				};
+
+				port@3 {
+					reg = <3>;
+				};
+
+				port@4 {
+					reg = <4>;
+				};
+
+				port@5 {
+					reg = <5>;
+				};
+
+				port@6 {
+					reg = <6>;
+				};
+
+				port@7 {
+					reg = <7>;
+				};
+			};
+		};
+
 		camcc: clock-controller@ade0000 {
 			compatible = "qcom,sm8550-camcc";
 			reg = <0 0x0ade0000 0 0x20000>;