Re: [PATCH v9 1/5] media: dt-bindings: Add CAMSS device for Kaanapali

[Vijay Kumar Tumati <vijay.tumati@oss.qualcomm.com>]

On 12/10/2025 2:05 PM, Bryan O'Donoghue wrote:
> On 10/12/2025 21:45, Bryan O'Donoghue wrote:
>> On 10/12/2025 19:36, Vijay Kumar Tumati wrote:
>>>
>>> On 12/10/2025 11:25 AM, Dmitry Baryshkov wrote:
>>>> On Wed, Dec 10, 2025 at 09:50:51AM -0800, Vijay Kumar Tumati wrote:
>>>>> On 12/8/2025 3:21 PM, Vijay Kumar Tumati wrote:
>>>>>> On 12/8/2025 2:48 PM, Dmitry Baryshkov wrote:
>>>>>>> On Mon, Dec 08, 2025 at 01:03:06PM -0800, Vijay Kumar Tumati wrote:
>>>>>>>> On 12/8/2025 11:53 AM, Dmitry Baryshkov wrote:
>>>>>>>>>> +  interconnects:
>>>>>>>>>> +    maxItems: 4
>>>>>>>>>> +
>>>>>>>>>> +  interconnect-names:
>>>>>>>>>> +    items:
>>>>>>>>>> +      - const: ahb
>>>>>>>>>> +      - const: hf_mnoc
>>>>>>>>>> +      - const: sf_icp_mnoc
>>>>>>>>>> +      - const: sf_mnoc
>>>>>>>>> You know... Failure to look around is a sin. What are the 
>>>>>>>>> names of
>>>>>>>>> interconnects used by other devices? What do they actually 
>>>>>>>>> describe?
>>>>>>>>>
>>>>>>>>> This is an absolute NAK.
>>>>>>>> Please feel free to correct me here but, a couple things.
>>>>>>>>
>>>>>>>> 1. This is consistent with
>>>>>>>> Documentation/devicetree/bindings/media/qcom,qcm2290-camss.yaml. 
>>>>>>>> no?
>>>>>>> I see that nobody noticed an issue with Agatti, Lemans and Monaco
>>>>>>> bindings (Krzysztof?)
>>>>>>>
>>>>>>> Usually interconnect names describe the blocks that are connected.
>>>>>>> Here
>>>>>>> are the top results of a quick git grep of interconnect names 
>>>>>>> through
>>>>>>> arch/arm64/dts/qcom:
>>>>>>>
>>>>>>>        729 "qup-core",
>>>>>>>        717 "qup-config",
>>>>>>>        457 "qup-memory",
>>>>>>>         41 "usb-ddr",
>>>>>>>         41 "apps-usb",
>>>>>>>         39 "pcie-mem",
>>>>>>>         39 "cpu-pcie",
>>>>>>>         28 "sdhc-ddr",
>>>>>>>         28 "cpu-sdhc",
>>>>>>>         28 "cpu-cfg",
>>>>>>>         24 "mdp0-mem",
>>>>>>>         17 "memory",
>>>>>>>         14 "ufs-ddr",
>>>>>>>         14 "mdp1-mem",
>>>>>>>         14 "cpu-ufs",
>>>>>>>         13 "video-mem",
>>>>>>>         13 "gfx-mem",
>>>>>>>
>>>>>>> I hope this gives you a pointer on how to name the interconnects.
>>>>>>>
>>>>>>>> 2. If you are referring to some other targets that use, "cam_"
>>>>>>>> prefix, we
>>>>>>>> may not need that , isn't it? If we look at these interconnects
>>>>>>>> from camera
>>>>>>>> side, as you advised for other things like this?
>>>>>>> See above.
>>>>>> I see, so the names cam-cfg, cam-hf-mem, cam-sf-mem, cam-sf-icp-mem
>>>>>> should be ok?
>>>>>>
>>>>>> Or the other option, go exactly like
>>>>>> Documentation/devicetree/bindings/media/qcom,sc8280xp-camss.yaml.
>>>>>>
>>>>>> What would you advise?
>>>>>>
>>>>> To keep it consistent with the previous generations and still
>>>>> represent the
>>>>> block name, we will go ahead with the style in qcom,sc8280xp-
>>>>> camss.yaml. If
>>>>> anyone has any concerns, please do let us know.
>>>> Krzysztof, Bryan, your opinion? My preference would be to start using
>>>> sensible names, but I wouldn't enforce that.
>>>>
>>>>>>>>>> +
>>>>>>>>>> +  iommus:
>>>>>>>>>> +    items:
>>>>>>>>>> +      - description: VFE non-protected stream
>>>>>>>>>> +      - description: ICP0 shared stream
>>>>>>>>>> +      - description: ICP1 shared stream
>>>>>>>>>> +      - description: IPE CDM non-protected stream
>>>>>>>>>> +      - description: IPE non-protected stream
>>>>>>>>>> +      - description: JPEG non-protected stream
>>>>>>>>>> +      - description: OFE CDM non-protected stream
>>>>>>>>>> +      - description: OFE non-protected stream
>>>>>>>>>> +      - description: VFE / VFE Lite CDM non-protected stream
>>>>>>>>> This will map all IOMMUs to the same domain. Are you sure that
>>>>>>>>> this is
>>>>>>>>> what we want? Or do we wait for iommu-maps to be fixed?
>>>>> Yes, when it is available, we can start using iommu-maps to create
>>>>> separate
>>>>> context banks.
>>>> It would be necessary to justify removing items from the list. 
>>>> Wouldn't
>>>> it be better to map only necessary SIDs now and add other later 
>>>> once we
>>>> have iommu-maps?
>>> I will let Bryan take the call on this. He was the one who wanted all
>>> the SIDs in the bindings. Hi @Bryan, if you can kindly share your
>>> thoughts on this and the interconnect naming, we will go ahead and push
>>> rev 10 for this. I believe we have taken care of other things. Thank 
>>> you.
>>>>
>>
>> Since when are we delaying patches for future patches that may land 
>> never ?
>>
>> I'm fine with whatever clock name changes you can agree with Krzysztof
>> but it seems a bit ironic to me to be given feedback to "align with
>> previous dts" to then have the result be further change.
>>
>> I'd like a bit of stability and consistency TBH.
>>
>> ---
>> bod
>>
>
> My feedback is
>
> - Include the full list of SIDs
> - Stick to previous clock and interconnect names
>
> Your other alternative is to suspend Kaanapali CAMSS unless/until 
> iommu-map is landed.
>
> As I say though "change your patch until my other patch is landed" is 
> the opposite of how things are supposed to be done.
>
> I recommend you focus on your own series. If iommu-map gets merged 
> first, adapt.
>
> If not, don't delay your work to accommodate stuff that is up in the 
> air which for all you know may never land or may take six more months.
>
> ---
> bod
Makes sense. Thanks Bryan, Dmitry. We will update this to,

Interconnect names: cam_ahb, cam_hf_mnoc, cam_sf_mnoc, cam_sf_icp_mnoc, 
consistent with a set of previous generations.

CX domain AXI clock names: gcc_axi_hf, gcc_axi_sf, consistent with other 
bindings.

We will post rev10 with these. Thanks.