diff mbox series

[1/4] dt-bindings: memory: mediatek: Add mt8188 SMI reset control binding

Message ID 20240821082845.11792-2-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 
To support SMI clamp and reset operation in genpd callback, add
SMI LARB reset register offset and mask related information in
the bindings. Add index in mt8188-resets.h to query the register
offset and mask in the SMI reset control driver.

Signed-off-by: friday.yang <friday.yang@mediatek.com>
---
 .../bindings/reset/mediatek,smi-reset.yaml    | 46 +++++++++++++++++++
 include/dt-bindings/reset/mt8188-resets.h     | 11 +++++
 2 files changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/mediatek,smi-reset.yaml

Comments

Krzysztof Kozlowski Aug. 21, 2024, 8:54 a.m. UTC | #1 
On 21/08/2024 10:26, friday.yang wrote:
> To support SMI clamp and reset operation in genpd callback, add
> SMI LARB reset register offset and mask related information in
> the bindings. Add index in mt8188-resets.h to query the register
> offset and mask in the SMI reset control driver.
> 
> Signed-off-by: friday.yang <friday.yang@mediatek.com>

User proper full name instead of login.


Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

> ---
>  .../bindings/reset/mediatek,smi-reset.yaml    | 46 +++++++++++++++++++
>  include/dt-bindings/reset/mt8188-resets.h     | 11 +++++
>  2 files changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/mediatek,smi-reset.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reset/mediatek,smi-reset.yaml b/Documentation/devicetree/bindings/reset/mediatek,smi-reset.yaml
> new file mode 100644
> index 000000000000..66ac121d2396
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/mediatek,smi-reset.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (c) 2024 MediaTek Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reset/mediatek,smi-reset.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek SMI Reset Controller
> +
> +maintainers:
> +  - Friday Yang <friday.yang@mediatek.com>
> +
> +description: |
> +  This reset controller node is used to perform reset management
> +  of SMI larbs on MediaTek platform. It is used to implement various
> +  reset functions required when SMI larbs apply clamp operation.
> +
> +  For list of all valid reset indices see
> +    <dt-bindings/reset/mt8188-resets.h> for MT8188.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mediatek,smi-reset-mt8188

Wrong placement of soc. It's mediatek,mt8189-whatever

> +
> +  "#reset-cells":
> +    const: 1
> +
> +  mediatek,larb-rst-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle of the SMI larb's reset controller syscon.

Explain what is it used for.

> +
> +required:
> +  - compatible
> +  - "#reset-cells"
> +  - mediatek,larb-rst-syscon
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    imgsys1_dip_top_rst: reset-controller {

Drop label

> +          compatible = "mediatek,smi-reset-mt8188";

Use 4 spaces for example indentation.

> +          #reset-cells = <1>;
> +          mediatek,larb-rst-syscon = <&imgsys1_dip_top>;
> +    };


Best regards,
Krzysztof
Krzysztof Kozlowski Aug. 21, 2024, 9:28 a.m. UTC | #2 
On 21/08/2024 10:26, friday.yang wrote:
> To support SMI clamp and reset operation in genpd callback, add
> SMI LARB reset register offset and mask related information in
> the bindings. Add index in mt8188-resets.h to query the register
> offset and mask in the SMI reset control driver.
> 
> Signed-off-by: friday.yang <friday.yang@mediatek.com>
> ---
>  .../bindings/reset/mediatek,smi-reset.yaml    | 46 +++++++++++++++++++
>  include/dt-bindings/reset/mt8188-resets.h     | 11 +++++

Also, your patches did not reach DT patchwork, so something is odd.
Maybe they got flagged as spam? Please investigate with your IT
department. In case it keeps missing patchwork, they won't be tested by
automation and I will generally ignore them (not apply). :(

Best regards,
Krzysztof
Krzysztof Kozlowski Aug. 22, 2024, 8:05 a.m. UTC | #3 
On Wed, Aug 21, 2024 at 11:28:17AM +0200, Krzysztof Kozlowski wrote:
> On 21/08/2024 10:26, friday.yang wrote:
> > To support SMI clamp and reset operation in genpd callback, add
> > SMI LARB reset register offset and mask related information in
> > the bindings. Add index in mt8188-resets.h to query the register
> > offset and mask in the SMI reset control driver.
> > 
> > Signed-off-by: friday.yang <friday.yang@mediatek.com>
> > ---
> >  .../bindings/reset/mediatek,smi-reset.yaml    | 46 +++++++++++++++++++
> >  include/dt-bindings/reset/mt8188-resets.h     | 11 +++++
> 
> Also, your patches did not reach DT patchwork, so something is odd.
> Maybe they got flagged as spam? Please investigate with your IT
> department. In case it keeps missing patchwork, they won't be tested by
> automation and I will generally ignore them (not apply). :(

Update: they reached now, so maybe this was some Patchwork issue. You
still can double check that you send patches correctly.

Best regards,
Krzysztof
Friday Yang Oct. 24, 2024, 1:28 a.m. UTC | #4 
On Wed, 2024-08-21 at 10:54 +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:
> > To support SMI clamp and reset operation in genpd callback, add
> > SMI LARB reset register offset and mask related information in
> > the bindings. Add index in mt8188-resets.h to query the register
> > offset and mask in the SMI reset control driver.
> > 
> > Signed-off-by: friday.yang <friday.yang@mediatek.com>
> 
> User proper full name instead of login.
> 

Thanks for your comments and sorry for replying so late.
I will fix it to Friday Yang <friday.yang@mediatek.com>.

> 
> Please use subject prefixes matching the subsystem. You can get them
> for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the
> directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> 
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 

I have already used this command `git log --oneline Documentation/devicetree/bindings/reset/`If you think it is inappropriate, how about this one?
'dt-bindings: reset: mediatek,mt8188-smi: Add SMI reset control binding
for MT8188'

> > ---
> >  .../bindings/reset/mediatek,smi-reset.yaml    | 46
> +++++++++++++++++++
> >  include/dt-bindings/reset/mt8188-resets.h     | 11 +++++
> >  2 files changed, 57 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/reset/mediatek,smi-reset.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/reset/mediatek,smi-
> reset.yaml b/Documentation/devicetree/bindings/reset/mediatek,smi-
> reset.yaml
> > new file mode 100644
> > index 000000000000..66ac121d2396
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reset/mediatek,smi-
> reset.yaml
> > @@ -0,0 +1,46 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (c) 2024 MediaTek Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/reset/mediatek,smi-reset.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek SMI Reset Controller
> > +
> > +maintainers:
> > +  - Friday Yang <friday.yang@mediatek.com>
> > +
> > +description: |
> > +  This reset controller node is used to perform reset management
> > +  of SMI larbs on MediaTek platform. It is used to implement
> various
> > +  reset functions required when SMI larbs apply clamp operation.
> > +
> > +  For list of all valid reset indices see
> > +    <dt-bindings/reset/mt8188-resets.h> for MT8188.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - mediatek,smi-reset-mt8188
> 
> Wrong placement of soc. It's mediatek,mt8189-whatever
> 

Thanks, I will fix it to 'mediatek,mt8188-smi-reset'.

> > +
> > +  "#reset-cells":
> > +    const: 1
> > +
> > +  mediatek,larb-rst-syscon:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: phandle of the SMI larb's reset controller
> syscon.
> 
> Explain what is it used for.
> 

I will add the descriton like this, is this clear for you?
When SMI larb power on/off, we need larb clamp and reset to avoid bus
glitch, this is to improve bus protect. 
The reset controller node is used to update regmap to implement larb
reset function.

> > +
> > +required:
> > +  - compatible
> > +  - "#reset-cells"
> > +  - mediatek,larb-rst-syscon
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    imgsys1_dip_top_rst: reset-controller {
> 
> Drop label

I see marvell,berlin2-reset.yaml, which also have a label.
Do you mean we should remove imgsys1_dip_top_rst, just use
'reset-controller' here.

> 
> > +          compatible = "mediatek,smi-reset-mt8188";
> 
> Use 4 spaces for example indentation.
> 

OK, I will use spaces instead of tab.

> > +          #reset-cells = <1>;
> > +          mediatek,larb-rst-syscon = <&imgsys1_dip_top>;
> > +    };
> 
> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/reset/mediatek,smi-reset.yaml b/Documentation/devicetree/bindings/reset/mediatek,smi-reset.yaml
new file mode 100644
index 000000000000..66ac121d2396
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/mediatek,smi-reset.yaml
@@ -0,0 +1,46 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2024 MediaTek Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reset/mediatek,smi-reset.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek SMI Reset Controller
+
+maintainers:
+  - Friday Yang <friday.yang@mediatek.com>
+
+description: |
+  This reset controller node is used to perform reset management
+  of SMI larbs on MediaTek platform. It is used to implement various
+  reset functions required when SMI larbs apply clamp operation.
+
+  For list of all valid reset indices see
+    <dt-bindings/reset/mt8188-resets.h> for MT8188.
+
+properties:
+  compatible:
+    enum:
+      - mediatek,smi-reset-mt8188
+
+  "#reset-cells":
+    const: 1
+
+  mediatek,larb-rst-syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle of the SMI larb's reset controller syscon.
+
+required:
+  - compatible
+  - "#reset-cells"
+  - mediatek,larb-rst-syscon
+
+additionalProperties: false
+
+examples:
+  - |
+    imgsys1_dip_top_rst: reset-controller {
+          compatible = "mediatek,smi-reset-mt8188";
+          #reset-cells = <1>;
+          mediatek,larb-rst-syscon = <&imgsys1_dip_top>;
+    };
diff --git a/include/dt-bindings/reset/mt8188-resets.h b/include/dt-bindings/reset/mt8188-resets.h
index 5a58c54e7d20..387a4beac688 100644
--- a/include/dt-bindings/reset/mt8188-resets.h
+++ b/include/dt-bindings/reset/mt8188-resets.h
@@ -113,4 +113,15 @@ 
 #define MT8188_VDO1_RST_HDR_GFX_FE1_DL_ASYNC	52
 #define MT8188_VDO1_RST_HDR_VDO_BE_DL_ASYNC	53
 
+#define MT8188_SMI_RST_LARB10			0
+#define MT8188_SMI_RST_LARB11A			1
+#define MT8188_SMI_RST_LARB11C			2
+#define MT8188_SMI_RST_LARB12			3
+#define MT8188_SMI_RST_LARB11B			4
+#define MT8188_SMI_RST_LARB15			5
+#define MT8188_SMI_RST_LARB16B			6
+#define MT8188_SMI_RST_LARB17B			7
+#define MT8188_SMI_RST_LARB16A			8
+#define MT8188_SMI_RST_LARB17A			9
+
 #endif  /* _DT_BINDINGS_RESET_CONTROLLER_MT8188 */