On Thu, 2024-10-24 at 13:59 +0200, AngeloGioacchino Del Regno wrote: > Il 24/10/24 03:28, Friday Yang (杨阳) ha scritto: > > On Wed, 2024-08-21 at 10:55 +0200, Krzysztof Kozlowski wrote: > > > > > > External email : Please do not click links or open attachments > > > until > > > you have verified the sender or the content. > > > On 21/08/2024 10:26, friday.yang wrote: > > > > On the MediaTek platform, some SMI LARBs are directly linked to > > > > SMI > > > > common. While some SMI LARBs are linked to SMI sub common, then > > > > SMI > > > > sub common is linked to SMI common. Add 'mediatek,smi-sub-comm' > > > > and > > > > 'mediatek,smi-sub-comm-in-portid' properties here. The SMI > > > > reset > > > > driver could query which port of the SMI sub common the current > > > > > > LARB > > > > is linked to through the two properties. The hardware block > > > > diagram > > > > could be described as below. > > > > > > > > SMI Common(Smart Multimedia Interface Common) > > > > | > > > > +----------------+------- > > > > | | > > > > | | > > > > | | > > > > | | > > > > | | > > > > larb0 SMI Sub Common > > > > | | | > > > > larb1 larb2 larb3 > > > > > > > > Signed-off-by: friday.yang <friday.yang@xxxxxxxxxxxx> > > > > --- > > > > .../mediatek,smi-common.yaml | 2 ++ > > > > .../memory-controllers/mediatek,smi-larb.yaml | 22 > > > > > > +++++++++++++++++++ > > > > 2 files changed, 24 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/memory- > > > > > > controllers/mediatek,smi-common.yaml > > > b/Documentation/devicetree/bindings/memory- > > > controllers/mediatek,smi- > > > common.yaml > > > > index 2f36ac23604c..4392d349878c 100644 > > > > --- a/Documentation/devicetree/bindings/memory- > > > > > > controllers/mediatek,smi-common.yaml > > > > +++ b/Documentation/devicetree/bindings/memory- > > > > > > controllers/mediatek,smi-common.yaml > > > > @@ -39,6 +39,7 @@ properties: > > > > - mediatek,mt8186-smi-common > > > > - mediatek,mt8188-smi-common-vdo > > > > - mediatek,mt8188-smi-common-vpp > > > > + - mediatek,mt8188-smi-sub-common > > > > - mediatek,mt8192-smi-common > > > > - mediatek,mt8195-smi-common-vdo > > > > - mediatek,mt8195-smi-common-vpp > > > > @@ -107,6 +108,7 @@ allOf: > > > > compatible: > > > > contains: > > > > enum: > > > > + - mediatek,mt8188-smi-sub-common > > > > - mediatek,mt8195-smi-sub-common > > > > then: > > > > required: > > > > diff --git a/Documentation/devicetree/bindings/memory- > > > > > > controllers/mediatek,smi-larb.yaml > > > b/Documentation/devicetree/bindings/memory- > > > controllers/mediatek,smi- > > > larb.yaml > > > > index 2381660b324c..5f162bb360db 100644 > > > > --- a/Documentation/devicetree/bindings/memory- > > > > > > controllers/mediatek,smi-larb.yaml > > > > +++ b/Documentation/devicetree/bindings/memory- > > > > > > controllers/mediatek,smi-larb.yaml > > > > @@ -69,6 +69,16 @@ properties: > > > > description: the hardware id of this larb. It's only > > > > required > > > > > > when this > > > > hardware id is not consecutive from its M4U point of > > > > view. > > > > > > > > + mediatek,smi-sub-comm: > > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > > + description: a phandle of smi_sub_common that the larb is > > > > > > linked to. > > > > > > Why do you have to smi phandle properties per each node? > > > > > > > As shown in the picture from the commit message, we have multipule > > smi- > > sub-common, each SMI larb may link to one of the smi-sub-common. So > > we > > need the 'mediatek,smi-sub-comm' to describe which smi-sub-common > > the > > larb is linked to. > > In next version, I will add two smi-sub-common to the diagram in > > the > > commit message. > > > > > > + > > > > + mediatek,smi-sub-comm-in-portid: > > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > > > + minimum: 0 > > > > + maximum: 7 > > > > + description: which port of smi_sub_common that the larb is > > > > > > linked to. > > > > > > Merge it into phandle. > > > > > > > Just confirm, > > Do you mean merge these two into one property, like: > > mediatek,smi-sub-comm = <&phandle port-id>; > > > > > > + > > > > required: > > > > - compatible > > > > - reg > > > > @@ -125,6 +135,18 @@ allOf: > > > > required: > > > > - mediatek,larb-id > > > > > > > > + - if: > > > > + properties: > > > > + compatible: > > > > + contains: > > > > + enum: > > > > + - mediatek,mt8188-smi-larb > > > > + > > > > + then: > > > > + required: > > > > + - mediatek,smi-sub-comm > > > > + - mediatek,smi-sub-comm-in-portid > > > > + > > > > > > and add it to the example (since you claim it is valid for every > > > device). > > > > > It's valid only for the Local Arbiters that have a sub-common port, > which anyway > are only the ones that are used by CAMSYS if I'm not wrong.... > > Regardless of that, not all of the mt8188-smi-larb *require* smi-sub- > comm. > > Besides, if the larb is anyway already linked to a sub-common, can't > we just grab > that from walking back? > Or is this property's purpose to actually add a link to a sub-common? > > Regards, > Angelo > > > > > OK, I will add this to the example. > > > > > Best regards, > > > Krzysztof > > > > Not only camerasys has smi-sub-common, other subsys also has smi-sub- common. And this property's purpose is to add a link to a sub-common, this is right. We will remove 'mediatek,smi-sub-comm', just use 'mediatek,smi'. Merge 'mediatek,smi-sub-comm-in-portid' into 'mediatek,smi', SMI driver should be changed to adapt this. Is this ok for you? >