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

[Bryan O'Donoghue]

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