diff mbox series

[v2,1/2] dt-bindings: remoteproc: mediatek: Add binding for mt8195 scp

Message ID 20210710122446.5439-1-tinghan.shen@mediatek.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] dt-bindings: remoteproc: mediatek: Add binding for mt8195 scp | expand

Commit Message

Tinghan Shen July 10, 2021, 12:24 p.m. UTC
Add mt8195 compatible to binding document. The description of required
properties are also modified to reflect the hardware change between
mt8183 and mt8195. The mt8195 doesn't have to control the scp clock on
kernel side.

Signed-off-by: Tinghan Shen <tinghan.shen@mediatek.com>
---
changes in v2:
- fix missing 'compatible' line in binding document

 Documentation/devicetree/bindings/remoteproc/mtk,scp.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Tzung-Bi Shih July 12, 2021, 6:05 a.m. UTC | #1
On Sat, Jul 10, 2021 at 8:25 PM Tinghan Shen <tinghan.shen@mediatek.com> wrote:
> @@ -5,13 +5,15 @@ This binding provides support for ARM Cortex M4 Co-processor found on some
>  Mediatek SoCs.
>
>  Required properties:
> -- compatible           Should be "mediatek,mt8183-scp"
> +- compatible           Should be one of:
> +                               "mediatek,mt8183-scp"
> +                               "mediatek,mt8195-scp"
Just realized we forgot to add DT bindings for mediatek,mt8192-scp[1].
Could you send another patch for adding the missing property?

[1]: https://elixir.bootlin.com/linux/v5.13.1/source/drivers/remoteproc/mtk_scp.c#L879

> -- clocks               Clock for co-processor (See: ../clock/clock-bindings.txt)
> -- clock-names          Contains the corresponding name for the clock. This
> +- clocks               Required by mt8183. Clock for co-processor (See: ../clock/clock-bindings.txt)
> +- clock-names          Required by mt8183. Contains the corresponding name for the clock. This
>                         should be named "main".
Let's move clocks and clock-names to "Optional properties".  See [2]
for your reference.  I guess it doesn't need to mention which chip
needs the properties.  For those chips that need the clock properties,
they won't work correctly without correct clock properties.

[2]: https://elixir.bootlin.com/linux/v5.13.1/source/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt#L87


Suggested to provide a cover letter for the series next time.

nit: other patches usually use "[PATCH v2 1/2]" in the email title
instead of the one used in the mail.
Chen-Yu Tsai July 12, 2021, 6:59 a.m. UTC | #2
On Mon, Jul 12, 2021 at 2:06 PM Tzung-Bi Shih <tzungbi@google.com> wrote:
>
> On Sat, Jul 10, 2021 at 8:25 PM Tinghan Shen <tinghan.shen@mediatek.com> wrote:
> > @@ -5,13 +5,15 @@ This binding provides support for ARM Cortex M4 Co-processor found on some
> >  Mediatek SoCs.
> >
> >  Required properties:
> > -- compatible           Should be "mediatek,mt8183-scp"
> > +- compatible           Should be one of:
> > +                               "mediatek,mt8183-scp"
> > +                               "mediatek,mt8195-scp"
> Just realized we forgot to add DT bindings for mediatek,mt8192-scp[1].
> Could you send another patch for adding the missing property?
>
> [1]: https://elixir.bootlin.com/linux/v5.13.1/source/drivers/remoteproc/mtk_scp.c#L879
>
> > -- clocks               Clock for co-processor (See: ../clock/clock-bindings.txt)
> > -- clock-names          Contains the corresponding name for the clock. This
> > +- clocks               Required by mt8183. Clock for co-processor (See: ../clock/clock-bindings.txt)
> > +- clock-names          Required by mt8183. Contains the corresponding name for the clock. This
> >                         should be named "main".
> Let's move clocks and clock-names to "Optional properties".  See [2]
> for your reference.  I guess it doesn't need to mention which chip
> needs the properties.  For those chips that need the clock properties,
> they won't work correctly without correct clock properties.

I would suggest still adding them. We will need to describe the requirements
anyway then the binding is converted to DT schema.

ChenYu

> [2]: https://elixir.bootlin.com/linux/v5.13.1/source/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt#L87
>
>
> Suggested to provide a cover letter for the series next time.
>
> nit: other patches usually use "[PATCH v2 1/2]" in the email title
> instead of the one used in the mail.
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Chen-Yu Tsai July 12, 2021, 7:01 a.m. UTC | #3
On Mon, Jul 12, 2021 at 2:59 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Mon, Jul 12, 2021 at 2:06 PM Tzung-Bi Shih <tzungbi@google.com> wrote:
> >
> > On Sat, Jul 10, 2021 at 8:25 PM Tinghan Shen <tinghan.shen@mediatek.com> wrote:
> > > @@ -5,13 +5,15 @@ This binding provides support for ARM Cortex M4 Co-processor found on some
> > >  Mediatek SoCs.
> > >
> > >  Required properties:
> > > -- compatible           Should be "mediatek,mt8183-scp"
> > > +- compatible           Should be one of:
> > > +                               "mediatek,mt8183-scp"
> > > +                               "mediatek,mt8195-scp"
> > Just realized we forgot to add DT bindings for mediatek,mt8192-scp[1].
> > Could you send another patch for adding the missing property?
> >
> > [1]: https://elixir.bootlin.com/linux/v5.13.1/source/drivers/remoteproc/mtk_scp.c#L879
> >
> > > -- clocks               Clock for co-processor (See: ../clock/clock-bindings.txt)
> > > -- clock-names          Contains the corresponding name for the clock. This
> > > +- clocks               Required by mt8183. Clock for co-processor (See: ../clock/clock-bindings.txt)
> > > +- clock-names          Required by mt8183. Contains the corresponding name for the clock. This
> > >                         should be named "main".
> > Let's move clocks and clock-names to "Optional properties".  See [2]
> > for your reference.  I guess it doesn't need to mention which chip
> > needs the properties.  For those chips that need the clock properties,
> > they won't work correctly without correct clock properties.
>
> I would suggest still adding them. We will need to describe the requirements
> anyway then the binding is converted to DT schema.

Also, a coprocessor without any clock feeding it makes little sense.
Any processor requires a running clock. Whether that clock is controllable
is beside the point.

> ChenYu
>
> > [2]: https://elixir.bootlin.com/linux/v5.13.1/source/Documentation/devicetree/bindings/remoteproc/ti,keystone-rproc.txt#L87
> >
> >
> > Suggested to provide a cover letter for the series next time.
> >
> > nit: other patches usually use "[PATCH v2 1/2]" in the email title
> > instead of the one used in the mail.
> >
> > _______________________________________________
> > Linux-mediatek mailing list
> > Linux-mediatek@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.txt b/Documentation/devicetree/bindings/remoteproc/mtk,scp.txt
index 3f5f78764b60..d64466eefbe3 100644
--- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.txt
+++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.txt
@@ -5,13 +5,15 @@  This binding provides support for ARM Cortex M4 Co-processor found on some
 Mediatek SoCs.
 
 Required properties:
-- compatible		Should be "mediatek,mt8183-scp"
+- compatible		Should be one of:
+				"mediatek,mt8183-scp"
+				"mediatek,mt8195-scp"
 - reg			Should contain the address ranges for memory regions:
 			SRAM, CFG, and L1TCM.
 - reg-names		Contains the corresponding names for the memory regions:
 			"sram", "cfg", and "l1tcm".
-- clocks		Clock for co-processor (See: ../clock/clock-bindings.txt)
-- clock-names		Contains the corresponding name for the clock. This
+- clocks		Required by mt8183. Clock for co-processor (See: ../clock/clock-bindings.txt)
+- clock-names		Required by mt8183. Contains the corresponding name for the clock. This
 			should be named "main".
 
 Subnodes