diff mbox series

[2/4] dt-bindings: memory: mediatek: Add smi-sub-common property for reset

Message ID 20240821082845.11792-3-friday.yang@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Add SMI clamp and reset | expand

Commit Message

Friday Yang (杨阳) Aug. 21, 2024, 8:26 a.m. UTC
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@mediatek.com>
---
 .../mediatek,smi-common.yaml                  |  2 ++
 .../memory-controllers/mediatek,smi-larb.yaml | 22 +++++++++++++++++++
 2 files changed, 24 insertions(+)

Comments

Krzysztof Kozlowski Aug. 21, 2024, 8:55 a.m. UTC | #1
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@mediatek.com>
> ---
>  .../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?

> +
> +  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.

> +
>  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).

Best regards,
Krzysztof
Friday Yang (杨阳) Oct. 24, 2024, 1:28 a.m. UTC | #2
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@mediatek.com>
> > ---
> >  .../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).
> 

OK, I will add this to the example.

> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Oct. 24, 2024, 6:38 a.m. UTC | #3
On 24/10/2024 03:28, Friday Yang (杨阳) wrote:
> 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@mediatek.com>
>>> ---
>>>  .../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.

You respond two months after... That email conversation is not even in
my mailbox anymore.

Anyway, sub-common are subnodes, no?

> 
>>> +
>>> +  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>;

Yes


Best regards,
Krzysztof
AngeloGioacchino Del Regno Oct. 24, 2024, 11:59 a.m. UTC | #4
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@mediatek.com>
>>> ---
>>>   .../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
>>
Friday Yang (杨阳) Oct. 25, 2024, 9:32 a.m. UTC | #5
On Thu, 2024-10-24 at 08:38 +0200, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 24/10/2024 03:28, Friday Yang (杨阳) wrote:
> > 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@mediatek.com>
> >>> ---
> >>>  .../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.
> 
> You respond two months after... That email conversation is not even
> in
> my mailbox anymore.
> 
> Anyway, sub-common are subnodes, no?
> 


                         IOMMU
                           |
                     SMI Common(Smart Multimedia Interface Common)
                           |
          +----------------+------------------------+
          |                |                        |
          |                |                        |
          |                |                        |
          |                |                        |
          |                |                        |
       larb0       SMI-Sub-Common0          SMI-Sub-Common1
                   |       |     |          |             |
                  larb2   larb4  larb5   larb1         larb7

This is the new version diagram.
In MediaTek platform, some larbs are linked to SMI Common, while some
other larbs are linked to smi-sub-common. Different platforms have
different hardware structures.

In the device tree, smi sub common is not a child node, but is at the
same level as smi common. 'sub' is just a internal name rule.
And 'mediatek,smi' is used to describe this connection. 
Example as below...

> > 
> >>> +
> >>> +  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>;
> 
> Yes
> 

We will remove 'mediatek,smi-sub-comm', which should be redundant.
Merge this 'mediatek,smi-sub-comm-in-portid' into 'mediatek,smi', is
this OK for you?

		smi_common_vdo: smi@1c024000 {
			compatible = "mediatek,mt8188-smi-common-vdo";
			reg = <0 0x1c024000 0 0x1000>;
			...
		};
		
		smi_sub_common_img0_4x1: smi@15002000 {
			compatible = "mediatek,mt8188-smi-sub-common";
			reg = <0 0x15002000 0 0x1000>;
			mediatek,smi = <&smi_common_vpp 5>;
			....
		};

		larb10: larb@15120000 {
			compatible = "mediatek,mt8188-smi-larb";
			reg = <0 0x15120000 0 0x1000>;
			mediatek,smi = <&smi_sub_common_img0_4x1 1>;
			...
		};

> 
> Best regards,
> Krzysztof
>
Friday Yang (杨阳) Oct. 25, 2024, 9:33 a.m. UTC | #6
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@mediatek.com>
> > > > ---
> > > >   .../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?

>
diff mbox series

Patch

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.
+
+  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.
+
 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
+
 additionalProperties: false
 
 examples: