diff mbox series

[v3,1/2] dt-bindings: clock: mediatek: Add SMI LARBs reset for MT8188

Message ID 20250121065045.13514-2-friday.yang@mediatek.com (mailing list archive)
State New
Headers show
Series Add SMI LARBs reset for MediaTek MT8188 SoC | expand

Commit Message

Friday Yang (杨阳) Jan. 21, 2025, 6:50 a.m. UTC
SMI LARBs require reset functions when applying clamp operations.
Add '#reset-cells' for the clock controller located in image, camera
and IPE subsystems.

Signed-off-by: Friday Yang <friday.yang@mediatek.com>
---
 .../bindings/clock/mediatek,mt8188-clock.yaml | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

--
2.46.0

Comments

Conor Dooley Jan. 21, 2025, 5:30 p.m. UTC | #1
On Tue, Jan 21, 2025 at 02:50:40PM +0800, Friday Yang wrote:
> SMI LARBs require reset functions when applying clamp operations.
> Add '#reset-cells' for the clock controller located in image, camera
> and IPE subsystems.

A new required property is an abi break, please explain why this is
required. What are "SMI LARBs"? Why did things previously work without
acting as a reset controller?

> 
> Signed-off-by: Friday Yang <friday.yang@mediatek.com>
> ---
>  .../bindings/clock/mediatek,mt8188-clock.yaml | 21 +++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
> index 860570320545..2985c8c717d7 100644
> --- a/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
> @@ -57,6 +57,27 @@ required:
>    - reg
>    - '#clock-cells'
> 
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - mediatek,mt8188-camsys-rawa
> +              - mediatek,mt8188-camsys-rawb
> +              - mediatek,mt8188-camsys-yuva
> +              - mediatek,mt8188-camsys-yuvb
> +              - mediatek,mt8188-imgsys-wpe1
> +              - mediatek,mt8188-imgsys-wpe2
> +              - mediatek,mt8188-imgsys-wpe3
> +              - mediatek,mt8188-imgsys1-dip-nr
> +              - mediatek,mt8188-imgsys1-dip-top
> +              - mediatek,mt8188-ipesys
> +
> +    then:
> +      required:
> +        - '#reset-cells'
> +
>  additionalProperties: false
> 
>  examples:
> --
> 2.46.0
>
Friday Yang (杨阳) Jan. 22, 2025, 7:40 a.m. UTC | #2
On Tue, 2025-01-21 at 17:30 +0000, Conor Dooley wrote:
> On Tue, Jan 21, 2025 at 02:50:40PM +0800, Friday Yang wrote:
> > SMI LARBs require reset functions when applying clamp operations.
> > Add '#reset-cells' for the clock controller located in image,
> > camera
> > and IPE subsystems.
> 
> A new required property is an abi break, please explain why this is
> required. What are "SMI LARBs"? Why did things previously work
> without
> acting as a reset controller?
> 

The background can refer to the discussion in the following link:

https://lore.kernel.org/all/CAFGrd9qZhObQXvm2_abqaX83xMLqxjQETB2=wXpobDWU1CnvkA@mail.gmail.com/

https://lore.kernel.org/all/CAPDyKFpokXV2gJDgowbixTvOH_5VL3B5H8eyhP+KJ5Fasm2rFg@mail.gmail.com/
SMI clamp and reset operations should be implemented in SMI driver
instead of PM driver.

I previously added the SMI reset control driver. However, the
reviewer's comments are correct, these functions have already
been implemented in the clock control driver. There is no need
to submit duplicate code. 

https://lore.kernel.org/lkml/20241120063305.8135-2-friday.yang@mediatek.com/

https://lore.kernel.org/lkml/20241120063305.8135-3-friday.yang@mediatek.com/


On the MediaTek platform, the SMI block diagram like this:

                DRAM
                 |
            EMI(External Memory Interface)
                 |  |
          MediaTek IOMMU(Input Output Memory Management Unit)
                 |  |
             SMI-Common(Smart Multimedia Interface Common)
                 |
         +----------------+------------------+
         |                |                  |
         |                |                  |
         |                |                  |
         |                |                  |
         |                |                  |
       larb0       SMI-Sub-Common0     SMI-Sub-Common1
                   |      |     |      |             |
                  larb1  larb2 larb3  larb7       larb9

The SMI-Common connects with SMI LARBs and IOMMU. The maximum LARBs
number that connects with a SMI-Common is 8. If the engines number is
over 8, sometimes we use a SMI-Sub-Common which is nearly same with
SMI-Common. It supports up to 8 input and 1 output(SMI-Common has 2
output).

> > 
> > Signed-off-by: Friday Yang <friday.yang@mediatek.com>
> > ---
> >  .../bindings/clock/mediatek,mt8188-clock.yaml | 21
> > +++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/clock/mediatek,mt8188-
> > clock.yaml
> > b/Documentation/devicetree/bindings/clock/mediatek,mt8188-
> > clock.yaml
> > index 860570320545..2985c8c717d7 100644
> > --- a/Documentation/devicetree/bindings/clock/mediatek,mt8188-
> > clock.yaml
> > +++ b/Documentation/devicetree/bindings/clock/mediatek,mt8188-
> > clock.yaml
> > @@ -57,6 +57,27 @@ required:
> >    - reg
> >    - '#clock-cells'
> > 
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - mediatek,mt8188-camsys-rawa
> > +              - mediatek,mt8188-camsys-rawb
> > +              - mediatek,mt8188-camsys-yuva
> > +              - mediatek,mt8188-camsys-yuvb
> > +              - mediatek,mt8188-imgsys-wpe1
> > +              - mediatek,mt8188-imgsys-wpe2
> > +              - mediatek,mt8188-imgsys-wpe3
> > +              - mediatek,mt8188-imgsys1-dip-nr
> > +              - mediatek,mt8188-imgsys1-dip-top
> > +              - mediatek,mt8188-ipesys
> > +
> > +    then:
> > +      required:
> > +        - '#reset-cells'
> > +
> >  additionalProperties: false
> > 
> >  examples:
> > --
> > 2.46.0
> >
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
index 860570320545..2985c8c717d7 100644
--- a/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
+++ b/Documentation/devicetree/bindings/clock/mediatek,mt8188-clock.yaml
@@ -57,6 +57,27 @@  required:
   - reg
   - '#clock-cells'

+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - mediatek,mt8188-camsys-rawa
+              - mediatek,mt8188-camsys-rawb
+              - mediatek,mt8188-camsys-yuva
+              - mediatek,mt8188-camsys-yuvb
+              - mediatek,mt8188-imgsys-wpe1
+              - mediatek,mt8188-imgsys-wpe2
+              - mediatek,mt8188-imgsys-wpe3
+              - mediatek,mt8188-imgsys1-dip-nr
+              - mediatek,mt8188-imgsys1-dip-top
+              - mediatek,mt8188-ipesys
+
+    then:
+      required:
+        - '#reset-cells'
+
 additionalProperties: false

 examples: