Re: [PATCH v4 1/2] dt-bindings: memory: mediatek: Add SMI reset and clamp for MT8188

["Friday Yang (杨阳)" <Friday.Yang@mediatek.com>]

On Mon, 2025-02-24 at 09:41 +0100, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 21/02/25 08:48, Friday Yang ha scritto:
> > From: "Friday Yang" <friday.yang@mediatek.com>
> > 
> > On the MediaTek platform, some SMI LARBs are directly connected to
> > the SMI Common, while others are connected to the SMI Sub-Common,
> > which in turn is connected to the SMI Common. The hardware block
> > diagram can be described as follows.
> > 
> >               SMI-Common(Smart Multimedia Interface Common)
> >                   |
> >           +----------------+------------------+
> >           |                |                  |
> >           |                |                  |
> >           |                |                  |
> >           |                |                  |
> >           |                |                  |
> >         larb0       SMI-Sub-Common0     SMI-Sub-Common1
> >                     |      |     |      |             |
> >                    larb1  larb2 larb3  larb7       larb9
> > 
> > For previous discussion on the direction of the code modifications,
> > please refer to:
> > https://lore.kernel.org/all/CAFGrd9qZhObQXvm2_abqaX83xMLqxjQETB2=
> > wXpobDWU1CnvkA@mail.gmail.com/
> > https://lore.kernel.org/all/CAPDyKFpokXV2gJDgowbixTvOH_5VL3B5H8ey
> > hP+KJ5Fasm2rFg@mail.gmail.com/
> > 
> > On the MediaTek MT8188 SoC platform, we encountered power-off
> > failures
> > and SMI bus hang issues during camera stress tests. The issue
> > arises
> > because bus glitches are sometimes produced when MTCMOS powers on
> > or
> > off. While this is fairly normal, the software must handle these
> > glitches to avoid mistaking them for transaction signals. What's
> > more, this issue emerged only after the initial upstreaming of this
> > binding. Without these patches, the SMI becomes unstable during
> > camera
> > stress tests.
> > 
> > The software solutions can be summarized as follows:
> > 
> > 1. Use CLAMP to disable the SMI sub-common port after turning off
> > the
> >     LARB CG and before turning off the LARB MTCMOS.
> > 2. Use CLAMP to disable/enable the SMI sub-common port.
> > 3. Implement an AXI reset for SMI LARBs.
> > 
> > This patch primarily add two changes:
> > 1. Add compatible for SMI sub-common on MT8188 SoC.
> > 2. Add 'resets' and 'reset-names' properties for SMI LARBs to
> >     support SMI reset operations.
> > 
> > Signed-off-by: Friday Yang <friday.yang@mediatek.com>
> > ---
> >   .../mediatek,smi-common.yaml                  |  2 ++
> >   .../memory-controllers/mediatek,smi-larb.yaml | 20
> > +++++++++++++++++++
> >   2 files changed, 22 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..2e86bb3455f9 100644
> > --- a/Documentation/devicetree/bindings/memory-
> > controllers/mediatek,smi-larb.yaml
> > +++ b/Documentation/devicetree/bindings/memory-
> > controllers/mediatek,smi-larb.yaml
> > @@ -69,6 +69,12 @@ 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.
> > 
> > +  resets:
> > +    maxItems: 1
> > +
> > +  reset-names:
> > +    const: larb
> > +
> >   required:
> >     - compatible
> >     - reg
> > @@ -125,6 +131,20 @@ allOf:
> >         required:
> >           - mediatek,larb-id
> > 
> > +  - if:  # only for image, camera and ipe subsys
> > +      properties:
> > +        compatible:
> > +          const: mediatek,mt8188-smi-larb
> > +        mediatek,larb-id:
> > +          oneOf:
> 
> Are you really sure that you need 'oneOf' here? :-)
> 
> Regards,
> Angelo

Yes, I have tested it. If I try to modify the 'examples'
like this. That is:
  change the compatible to "mediatek,mt8188-smi-larb",
  add 'mediatek,larb-id = <10>;'

examples:
  - |+
    #include <dt-bindings/clock/mt8173-clk.h>
    #includ
e <dt-bindings/power/mt8173-power.h>

    larb1: larb@16010000 {
      compatible = "mediatek,mt8188-smi-larb";
      reg = <0x16010000 0x1000>;
      mediatek,smi = <&smi_common>;
      mediatek,larb-id = <10>;
      power-domains = <&scpsys MT8188_POWER_DOMAIN_VDEC>;
      clocks = <&vdecsys CLK_VDEC_CKEN>,
               <&vdecsys CLK_VDEC_LARB_CKEN>;
      clock-names = "apb", "smi";
    };

The 'dt_binding_check' could give the following
errors:

Documentation/devicetree/bindings/memory-controllers/mediatek,smi-
larb.example.dtb: larb@16010000: 'resets' is a required property
from schema $id: 
http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml#
Documentation/devicetree/bindings/memory-controllers/mediatek,smi-
larb.example.dtb: larb@16010000: 'reset-names' is a required property
from schema $id:  
http://devicetree.org/schemas/memory-controllers/mediatek,smi-larb.yaml#

And this is what I want to achieve. On the MediaTek MT8188 SoC
platform, 'resets' and 'reset-names' are only required for SMI LARBs
located in image, camera and ipe subsys. Others can be ignored. And the
'larb-id' of these SMI LARBs are shown in this array: [ 9, 10, 11, 12,
13, 16, 17, 18, 19, 20 ].

Please feel free to let me know if you have any doubts.

> 
> > +            - enum:
> > +                [ 9, 10, 11, 12, 13, 16, 17, 18, 19, 20 ]
> > +
> > +    then:
> > +      required:
> > +        - resets
> > +        - reset-names
> > +
> >   additionalProperties: false
> > 
> >   examples:
> > --
> > 2.46.0
> > 
> 
> 
>