* [PATCH 0/3] RZ/G2L SCI support and sh-sci driver update @ 2021-11-03 17:31 Lad Prabhakar 2021-11-03 17:31 ` [PATCH 1/3] dt-bindings: serial: renesas,scif: Make resets as a required property Lad Prabhakar ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Lad Prabhakar @ 2021-11-03 17:31 UTC (permalink / raw) To: Geert Uytterhoeven, Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel Cc: linux-serial, devicetree, linux-kernel, linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar Hi All, This patch series updates binding doc to support RZ/G2L and adds support to perform deassert/assert in sh-sci driver. Cheers, Prabhakar Lad Prabhakar (3): dt-bindings: serial: renesas,scif: Make resets as a required property dt-bindings: serial: renesas,sci: Document RZ/G2L SoC serial: sh-sci: Add reset support for RZ/G2L SoC .../bindings/serial/renesas,sci.yaml | 43 +++++++++++++--- .../bindings/serial/renesas,scif.yaml | 1 + drivers/tty/serial/sh-sci.c | 50 ++++++++++++++++--- 3 files changed, 81 insertions(+), 13 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] dt-bindings: serial: renesas,scif: Make resets as a required property 2021-11-03 17:31 [PATCH 0/3] RZ/G2L SCI support and sh-sci driver update Lad Prabhakar @ 2021-11-03 17:31 ` Lad Prabhakar 2021-11-08 16:29 ` Geert Uytterhoeven 2021-11-03 17:31 ` [PATCH 2/3] dt-bindings: serial: renesas,sci: Document RZ/G2L SoC Lad Prabhakar 2021-11-03 17:31 ` [PATCH 3/3] serial: sh-sci: Add reset support for " Lad Prabhakar 2 siblings, 1 reply; 14+ messages in thread From: Lad Prabhakar @ 2021-11-03 17:31 UTC (permalink / raw) To: Geert Uytterhoeven, Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel Cc: linux-serial, devicetree, linux-kernel, linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar Make "resets" as required property for RZ/G2L. On RZ/G2L the devices should be explicitly pulled out of reset for this reason make "resets" as required property. Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> --- Documentation/devicetree/bindings/serial/renesas,scif.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/serial/renesas,scif.yaml b/Documentation/devicetree/bindings/serial/renesas,scif.yaml index 6b8731f7f2fb..df052dec7e63 100644 --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml @@ -151,6 +151,7 @@ if: compatible: contains: enum: + - renesas,scif-r9a07g044 - renesas,rcar-gen2-scif - renesas,rcar-gen3-scif then: -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: serial: renesas,scif: Make resets as a required property 2021-11-03 17:31 ` [PATCH 1/3] dt-bindings: serial: renesas,scif: Make resets as a required property Lad Prabhakar @ 2021-11-08 16:29 ` Geert Uytterhoeven 2021-11-09 0:36 ` Lad, Prabhakar 0 siblings, 1 reply; 14+ messages in thread From: Geert Uytterhoeven @ 2021-11-08 16:29 UTC (permalink / raw) To: Lad Prabhakar Cc: Geert Uytterhoeven, Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel, open list:SERIAL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List, Linux-Renesas, Prabhakar, Biju Das On Wed, Nov 3, 2021 at 6:31 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Make "resets" as required property for RZ/G2L. On RZ/G2L the devices > should be explicitly pulled out of reset for this reason make "resets" > as required property. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml > +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml > @@ -151,6 +151,7 @@ if: > compatible: > contains: > enum: > + - renesas,scif-r9a07g044 > - renesas,rcar-gen2-scif > - renesas,rcar-gen3-scif People might prefer alphabetical sort order... > then: Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] dt-bindings: serial: renesas,scif: Make resets as a required property 2021-11-08 16:29 ` Geert Uytterhoeven @ 2021-11-09 0:36 ` Lad, Prabhakar 0 siblings, 0 replies; 14+ messages in thread From: Lad, Prabhakar @ 2021-11-09 0:36 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Lad Prabhakar, Geert Uytterhoeven, Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel, open list:SERIAL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List, Linux-Renesas, Biju Das Hi Geert, Thank you for the review. On Mon, Nov 8, 2021 at 4:29 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Wed, Nov 3, 2021 at 6:31 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Make "resets" as required property for RZ/G2L. On RZ/G2L the devices > > should be explicitly pulled out of reset for this reason make "resets" > > as required property. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > --- a/Documentation/devicetree/bindings/serial/renesas,scif.yaml > > +++ b/Documentation/devicetree/bindings/serial/renesas,scif.yaml > > @@ -151,6 +151,7 @@ if: > > compatible: > > contains: > > enum: > > + - renesas,scif-r9a07g044 > > - renesas,rcar-gen2-scif > > - renesas,rcar-gen3-scif > > People might prefer alphabetical sort order... > My intention was to keep the generic ones at the end as we do for compatible property. Let me know if you still want me to change it. Cheers, Prabhakar > > then: > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] dt-bindings: serial: renesas,sci: Document RZ/G2L SoC 2021-11-03 17:31 [PATCH 0/3] RZ/G2L SCI support and sh-sci driver update Lad Prabhakar 2021-11-03 17:31 ` [PATCH 1/3] dt-bindings: serial: renesas,scif: Make resets as a required property Lad Prabhakar @ 2021-11-03 17:31 ` Lad Prabhakar 2021-11-08 16:19 ` Geert Uytterhoeven 2021-11-08 16:21 ` Geert Uytterhoeven 2021-11-03 17:31 ` [PATCH 3/3] serial: sh-sci: Add reset support for " Lad Prabhakar 2 siblings, 2 replies; 14+ messages in thread From: Lad Prabhakar @ 2021-11-03 17:31 UTC (permalink / raw) To: Geert Uytterhoeven, Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel Cc: linux-serial, devicetree, linux-kernel, linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar Add SCI binding documentation for Renesas RZ/G2L SoC. Also update the example node with real world DT node. Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> --- .../bindings/serial/renesas,sci.yaml | 43 ++++++++++++++++--- 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/Documentation/devicetree/bindings/serial/renesas,sci.yaml b/Documentation/devicetree/bindings/serial/renesas,sci.yaml index 22ed2f0b1dc3..6fc573406240 100644 --- a/Documentation/devicetree/bindings/serial/renesas,sci.yaml +++ b/Documentation/devicetree/bindings/serial/renesas,sci.yaml @@ -14,7 +14,11 @@ allOf: properties: compatible: - const: renesas,sci + oneOf: + - items: + - enum: + - renesas,r9a07g044-sci # RZ/G2{L,LC} + - const: renesas,sci # generic SCI compatible UART reg: maxItems: 1 @@ -54,18 +58,45 @@ required: - clocks - clock-names +if: + properties: + compatible: + contains: + enum: + - renesas,r9a07g044-sci +then: + properties: + resets: + maxItems: 1 + + power-domains: + maxItems: 1 + + required: + - resets + - power-domains + unevaluatedProperties: false examples: - | + #include <dt-bindings/clock/r9a07g044-cpg.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + aliases { serial0 = &sci0; }; - sci0: serial@ffff78 { - compatible = "renesas,sci"; - reg = <0xffff78 8>; - interrupts = <88 0>, <89 0>, <90 0>, <91 0>; - clocks = <&fclk>; + sci0: serial@1004d000 { + compatible = "renesas,r9a07g044-sci", "renesas,sci"; + reg = <0x1004d000 0x400>; + interrupts = <GIC_SPI 405 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 407 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 408 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "eri", "rxi", "txi", "tei"; + clocks = <&cpg CPG_MOD R9A07G044_SCI0_CLKP>; clock-names = "fck"; + power-domains = <&cpg>; + resets = <&cpg R9A07G044_SCI0_RST>; }; -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] dt-bindings: serial: renesas,sci: Document RZ/G2L SoC 2021-11-03 17:31 ` [PATCH 2/3] dt-bindings: serial: renesas,sci: Document RZ/G2L SoC Lad Prabhakar @ 2021-11-08 16:19 ` Geert Uytterhoeven 2021-11-09 0:17 ` Lad, Prabhakar 2021-11-08 16:21 ` Geert Uytterhoeven 1 sibling, 1 reply; 14+ messages in thread From: Geert Uytterhoeven @ 2021-11-08 16:19 UTC (permalink / raw) To: Lad Prabhakar Cc: Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel, open list:SERIAL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List, Linux-Renesas, Prabhakar Hi Prabhakar, On Wed, Nov 3, 2021 at 6:31 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Add SCI binding documentation for Renesas RZ/G2L SoC. > > Also update the example node with real world DT node. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > --- a/Documentation/devicetree/bindings/serial/renesas,sci.yaml > +++ b/Documentation/devicetree/bindings/serial/renesas,sci.yaml > @@ -14,7 +14,11 @@ allOf: > > properties: > compatible: > - const: renesas,sci > + oneOf: > + - items: > + - enum: > + - renesas,r9a07g044-sci # RZ/G2{L,LC} > + - const: renesas,sci # generic SCI compatible UART You added a oneOf, but forgot to keep the old single compatible value as used on H8/300? > > reg: > maxItems: 1 With the above fixed: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] dt-bindings: serial: renesas,sci: Document RZ/G2L SoC 2021-11-08 16:19 ` Geert Uytterhoeven @ 2021-11-09 0:17 ` Lad, Prabhakar 0 siblings, 0 replies; 14+ messages in thread From: Lad, Prabhakar @ 2021-11-09 0:17 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Lad Prabhakar, Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel, open list:SERIAL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List, Linux-Renesas Hi Geert, Thank you for the review. On Mon, Nov 8, 2021 at 4:19 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Wed, Nov 3, 2021 at 6:31 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Add SCI binding documentation for Renesas RZ/G2L SoC. > > > > Also update the example node with real world DT node. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- a/Documentation/devicetree/bindings/serial/renesas,sci.yaml > > +++ b/Documentation/devicetree/bindings/serial/renesas,sci.yaml > > @@ -14,7 +14,11 @@ allOf: > > > > properties: > > compatible: > > - const: renesas,sci > > + oneOf: > > + - items: > > + - enum: > > + - renesas,r9a07g044-sci # RZ/G2{L,LC} > > + - const: renesas,sci # generic SCI compatible UART > > You added a oneOf, but forgot to keep the old single compatible > value as used on H8/300? > Ouch I grepped renesas,sci only in arm{64}, will fix that. Cheers, Prabhakar > > > > reg: > > maxItems: 1 > > With the above fixed: > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] dt-bindings: serial: renesas,sci: Document RZ/G2L SoC 2021-11-03 17:31 ` [PATCH 2/3] dt-bindings: serial: renesas,sci: Document RZ/G2L SoC Lad Prabhakar 2021-11-08 16:19 ` Geert Uytterhoeven @ 2021-11-08 16:21 ` Geert Uytterhoeven 2021-11-09 0:19 ` Lad, Prabhakar 1 sibling, 1 reply; 14+ messages in thread From: Geert Uytterhoeven @ 2021-11-08 16:21 UTC (permalink / raw) To: Lad Prabhakar Cc: Geert Uytterhoeven, Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel, open list:SERIAL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List, Linux-Renesas, Prabhakar, Biju Das On Wed, Nov 3, 2021 at 6:31 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > Add SCI binding documentation for Renesas RZ/G2L SoC. > > Also update the example node with real world DT node. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > --- a/Documentation/devicetree/bindings/serial/renesas,sci.yaml > +++ b/Documentation/devicetree/bindings/serial/renesas,sci.yaml > @@ -54,18 +58,45 @@ required: > - clocks > - clock-names > > +if: > + properties: > + compatible: > + contains: > + enum: > + - renesas,r9a07g044-sci > +then: > + properties: > + resets: > + maxItems: 1 > + > + power-domains: > + maxItems: 1 > + > + required: > + - resets > + - power-domains We really should make interrupt-names required everywhere, after fixing the H8/300 DTS files. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] dt-bindings: serial: renesas,sci: Document RZ/G2L SoC 2021-11-08 16:21 ` Geert Uytterhoeven @ 2021-11-09 0:19 ` Lad, Prabhakar 0 siblings, 0 replies; 14+ messages in thread From: Lad, Prabhakar @ 2021-11-09 0:19 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Lad Prabhakar, Geert Uytterhoeven, Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel, open list:SERIAL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List, Linux-Renesas, Biju Das Hi Geert, On Mon, Nov 8, 2021 at 4:21 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Wed, Nov 3, 2021 at 6:31 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Add SCI binding documentation for Renesas RZ/G2L SoC. > > > > Also update the example node with real world DT node. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- a/Documentation/devicetree/bindings/serial/renesas,sci.yaml > > +++ b/Documentation/devicetree/bindings/serial/renesas,sci.yaml > > > @@ -54,18 +58,45 @@ required: > > - clocks > > - clock-names > > > > +if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - renesas,r9a07g044-sci > > +then: > > + properties: > > + resets: > > + maxItems: 1 > > + > > + power-domains: > > + maxItems: 1 > > + > > + required: > > + - resets > > + - power-domains > > We really should make interrupt-names required everywhere, after > fixing the H8/300 DTS files. > Agreed, also in other scif*.yaml files this needs fixing. Cheers, Prabhakar > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] serial: sh-sci: Add reset support for RZ/G2L SoC 2021-11-03 17:31 [PATCH 0/3] RZ/G2L SCI support and sh-sci driver update Lad Prabhakar 2021-11-03 17:31 ` [PATCH 1/3] dt-bindings: serial: renesas,scif: Make resets as a required property Lad Prabhakar 2021-11-03 17:31 ` [PATCH 2/3] dt-bindings: serial: renesas,sci: Document RZ/G2L SoC Lad Prabhakar @ 2021-11-03 17:31 ` Lad Prabhakar 2021-11-08 16:39 ` Geert Uytterhoeven 2 siblings, 1 reply; 14+ messages in thread From: Lad Prabhakar @ 2021-11-03 17:31 UTC (permalink / raw) To: Geert Uytterhoeven, Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel Cc: linux-serial, devicetree, linux-kernel, linux-renesas-soc, Prabhakar, Biju Das, Lad Prabhakar On RZ/G2L devices should be explicitly pulled out of reset for it to work. This patch adds support to read the "resets" property and performs deassert/assert when required. Also, propagate the error to the caller of sci_parse_dt() instead of returning NULL in case of failure. Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> --- Hi Geert, For handling the resets I was in dual mind whether to perform reset based on compatible strings or soc-id, let me know your thoughts. Currently no SoC's use "renesas,sci" so using the same for performing the reset operation for SCI. --- drivers/tty/serial/sh-sci.c | 50 +++++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c index 89ee43061d3a..a0f68b887e29 100644 --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -37,6 +37,7 @@ #include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> +#include <linux/reset.h> #include <linux/scatterlist.h> #include <linux/serial.h> #include <linux/serial_sci.h> @@ -3203,23 +3204,58 @@ static const struct of_device_id of_sci_match[] = { }; MODULE_DEVICE_TABLE(of, of_sci_match); +static void sci_reset_control_assert(void *data) +{ + reset_control_assert(data); +} + static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev, unsigned int *dev_id) { struct device_node *np = pdev->dev.of_node; + const struct of_device_id *of_id; struct plat_sci_port *p; struct sci_port *sp; const void *data; int id; if (!IS_ENABLED(CONFIG_OF) || !np) - return NULL; + return ERR_PTR(-EINVAL); + + of_id = of_match_device(of_sci_match, &pdev->dev); + if (!of_id) + return ERR_PTR(-EINVAL); - data = of_device_get_match_data(&pdev->dev); + if (!strcmp(of_id->compatible, "renesas,scif-r9a07g044") || + !strcmp(of_id->compatible, "renesas,sci")) { + struct reset_control *rstc; + int ret; + + rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); + if (IS_ERR(rstc)) { + dev_err(&pdev->dev, "Error: missing reset ctrl\n"); + return ERR_PTR(PTR_ERR(rstc)); + } + + ret = reset_control_deassert(rstc); + if (ret) { + dev_err(&pdev->dev, "failed to deassert %d\n", ret); + return ERR_PTR(ret); + } + + ret = devm_add_action_or_reset(&pdev->dev, sci_reset_control_assert, rstc); + if (ret) { + dev_err(&pdev->dev, "failed to register assert devm action, %d\n", + ret); + return ERR_PTR(ret); + } + } + + data = of_id->data; p = devm_kzalloc(&pdev->dev, sizeof(struct plat_sci_port), GFP_KERNEL); if (!p) - return NULL; + return ERR_PTR(-ENOMEM); /* Get the line number from the aliases node. */ id = of_alias_get_id(np, "serial"); @@ -3227,11 +3263,11 @@ static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev, id = ffz(sci_ports_in_use); if (id < 0) { dev_err(&pdev->dev, "failed to get alias id (%d)\n", id); - return NULL; + return ERR_PTR(-EINVAL); } if (id >= ARRAY_SIZE(sci_ports)) { dev_err(&pdev->dev, "serial%d out of range\n", id); - return NULL; + return ERR_PTR(-EINVAL); } sp = &sci_ports[id]; @@ -3318,8 +3354,8 @@ static int sci_probe(struct platform_device *dev) if (dev->dev.of_node) { p = sci_parse_dt(dev, &dev_id); - if (p == NULL) - return -EINVAL; + if (IS_ERR(p)) + return PTR_ERR(p); } else { p = dev->dev.platform_data; if (p == NULL) { -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] serial: sh-sci: Add reset support for RZ/G2L SoC 2021-11-03 17:31 ` [PATCH 3/3] serial: sh-sci: Add reset support for " Lad Prabhakar @ 2021-11-08 16:39 ` Geert Uytterhoeven 2021-11-09 0:33 ` Lad, Prabhakar 0 siblings, 1 reply; 14+ messages in thread From: Geert Uytterhoeven @ 2021-11-08 16:39 UTC (permalink / raw) To: Lad Prabhakar Cc: Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel, open list:SERIAL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List, Linux-Renesas, Prabhakar, Biju Das Hi Prabhakar, On Wed, Nov 3, 2021 at 6:31 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > On RZ/G2L devices should be explicitly pulled out of reset for it > to work. This patch adds support to read the "resets" property and > performs deassert/assert when required. > > Also, propagate the error to the caller of sci_parse_dt() instead of > returning NULL in case of failure. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> Thanks for your patch! > --- > Hi Geert, > For handling the resets I was in dual mind whether to perform > reset based on compatible strings or soc-id, let me know your > thoughts. Currently no SoC's use "renesas,sci" so using the same > for performing the reset operation for SCI. We do, on H8/300. > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -3203,23 +3204,58 @@ static const struct of_device_id of_sci_match[] = { > }; > MODULE_DEVICE_TABLE(of, of_sci_match); > > +static void sci_reset_control_assert(void *data) > +{ > + reset_control_assert(data); > +} > + > static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev, > unsigned int *dev_id) > { > struct device_node *np = pdev->dev.of_node; > + const struct of_device_id *of_id; > struct plat_sci_port *p; > struct sci_port *sp; > const void *data; > int id; > > if (!IS_ENABLED(CONFIG_OF) || !np) > - return NULL; > + return ERR_PTR(-EINVAL); > + > + of_id = of_match_device(of_sci_match, &pdev->dev); > + if (!of_id) > + return ERR_PTR(-EINVAL); > > - data = of_device_get_match_data(&pdev->dev); > + if (!strcmp(of_id->compatible, "renesas,scif-r9a07g044") || > + !strcmp(of_id->compatible, "renesas,sci")) { This will match on H8/300, too, which doesn't have resets. Please match against "renesas,sci-r9a07g044" instead. Please don't use explicit strcmp() calls here, but add a flag to of_sci_match[].data. > + struct reset_control *rstc; > + int ret; > + > + rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); > + if (IS_ERR(rstc)) { > + dev_err(&pdev->dev, "Error: missing reset ctrl\n"); > + return ERR_PTR(PTR_ERR(rstc)); > + } > + > + ret = reset_control_deassert(rstc); > + if (ret) { > + dev_err(&pdev->dev, "failed to deassert %d\n", ret); > + return ERR_PTR(ret); > + } > + > + ret = devm_add_action_or_reset(&pdev->dev, sci_reset_control_assert, rstc); > + if (ret) { > + dev_err(&pdev->dev, "failed to register assert devm action, %d\n", > + ret); > + return ERR_PTR(ret); > + } > + } > + > + data = of_id->data; > Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] serial: sh-sci: Add reset support for RZ/G2L SoC 2021-11-08 16:39 ` Geert Uytterhoeven @ 2021-11-09 0:33 ` Lad, Prabhakar 2021-11-09 7:49 ` Geert Uytterhoeven 0 siblings, 1 reply; 14+ messages in thread From: Lad, Prabhakar @ 2021-11-09 0:33 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Lad Prabhakar, Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel, open list:SERIAL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List, Linux-Renesas, Biju Das Hi Geert, Thank you for the review. On Mon, Nov 8, 2021 at 4:40 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Wed, Nov 3, 2021 at 6:31 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > On RZ/G2L devices should be explicitly pulled out of reset for it > > to work. This patch adds support to read the "resets" property and > > performs deassert/assert when required. > > > > Also, propagate the error to the caller of sci_parse_dt() instead of > > returning NULL in case of failure. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > Thanks for your patch! > > > --- > > Hi Geert, > > For handling the resets I was in dual mind whether to perform > > reset based on compatible strings or soc-id, let me know your > > thoughts. Currently no SoC's use "renesas,sci" so using the same > > for performing the reset operation for SCI. > > We do, on H8/300. > Missed that. > > --- a/drivers/tty/serial/sh-sci.c > > +++ b/drivers/tty/serial/sh-sci.c > > @@ -3203,23 +3204,58 @@ static const struct of_device_id of_sci_match[] = { > > }; > > MODULE_DEVICE_TABLE(of, of_sci_match); > > > > +static void sci_reset_control_assert(void *data) > > +{ > > + reset_control_assert(data); > > +} > > + > > static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev, > > unsigned int *dev_id) > > { > > struct device_node *np = pdev->dev.of_node; > > + const struct of_device_id *of_id; > > struct plat_sci_port *p; > > struct sci_port *sp; > > const void *data; > > int id; > > > > if (!IS_ENABLED(CONFIG_OF) || !np) > > - return NULL; > > + return ERR_PTR(-EINVAL); > > + > > + of_id = of_match_device(of_sci_match, &pdev->dev); > > + if (!of_id) > > + return ERR_PTR(-EINVAL); > > > > - data = of_device_get_match_data(&pdev->dev); > > + if (!strcmp(of_id->compatible, "renesas,scif-r9a07g044") || > > + !strcmp(of_id->compatible, "renesas,sci")) { > > This will match on H8/300, too, which doesn't have resets. > Please match against "renesas,sci-r9a07g044" instead. > > Please don't use explicit strcmp() calls here, but add a flag to > of_sci_match[].data. > Agreed, does the below hunk look good for handling the reset? -#define SCI_OF_DATA(type, regtype) (void *)((type) << 16 | (regtype)) +#define SCIx_RESET BIT(31) +#define SCI_OF_DATA(type, regtype, reset) (void *)(reset | (type) << 16 | (regtype)) #define SCI_OF_TYPE(data) ((unsigned long)(data) >> 16) #define SCI_OF_REGTYPE(data) ((unsigned long)(data) & 0xffff) @@ -3169,7 +3170,11 @@ static const struct of_device_id of_sci_match[] = { }, { .compatible = "renesas,scif-r9a07g044", - .data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE), + .data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE, SCIx_RESET), + }, + { + .compatible = "renesas,sci-r9a07g044", + .data = SCI_OF_DATA(PORT_SCI, SCIx_SCI_REGTYPE, SCIx_RESET), }, Cheers, Prabhakar > > + struct reset_control *rstc; > > + int ret; > > + > > + rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); > > + if (IS_ERR(rstc)) { > > + dev_err(&pdev->dev, "Error: missing reset ctrl\n"); > > + return ERR_PTR(PTR_ERR(rstc)); > > + } > > + > > + ret = reset_control_deassert(rstc); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to deassert %d\n", ret); > > + return ERR_PTR(ret); > > + } > > + > > + ret = devm_add_action_or_reset(&pdev->dev, sci_reset_control_assert, rstc); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to register assert devm action, %d\n", > > + ret); > > + return ERR_PTR(ret); > > + } > > + } > > + > > + data = of_id->data; > > > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] serial: sh-sci: Add reset support for RZ/G2L SoC 2021-11-09 0:33 ` Lad, Prabhakar @ 2021-11-09 7:49 ` Geert Uytterhoeven 2021-11-09 9:02 ` Lad, Prabhakar 0 siblings, 1 reply; 14+ messages in thread From: Geert Uytterhoeven @ 2021-11-09 7:49 UTC (permalink / raw) To: Lad, Prabhakar Cc: Lad Prabhakar, Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel, open list:SERIAL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List, Linux-Renesas, Biju Das Hi Prabhakar, On Tue, Nov 9, 2021 at 1:34 AM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > On Mon, Nov 8, 2021 at 4:40 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Wed, Nov 3, 2021 at 6:31 PM Lad Prabhakar > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > On RZ/G2L devices should be explicitly pulled out of reset for it > > > to work. This patch adds support to read the "resets" property and > > > performs deassert/assert when required. > > > > > > Also, propagate the error to the caller of sci_parse_dt() instead of > > > returning NULL in case of failure. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > Thanks for your patch! > > > > > --- > > > Hi Geert, > > > For handling the resets I was in dual mind whether to perform > > > reset based on compatible strings or soc-id, let me know your > > > thoughts. Currently no SoC's use "renesas,sci" so using the same > > > for performing the reset operation for SCI. > > > > We do, on H8/300. > > > Missed that. > > > > --- a/drivers/tty/serial/sh-sci.c > > > +++ b/drivers/tty/serial/sh-sci.c > > > @@ -3203,23 +3204,58 @@ static const struct of_device_id of_sci_match[] = { > > > }; > > > MODULE_DEVICE_TABLE(of, of_sci_match); > > > > > > +static void sci_reset_control_assert(void *data) > > > +{ > > > + reset_control_assert(data); > > > +} > > > + > > > static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev, > > > unsigned int *dev_id) > > > { > > > struct device_node *np = pdev->dev.of_node; > > > + const struct of_device_id *of_id; > > > struct plat_sci_port *p; > > > struct sci_port *sp; > > > const void *data; > > > int id; > > > > > > if (!IS_ENABLED(CONFIG_OF) || !np) > > > - return NULL; > > > + return ERR_PTR(-EINVAL); > > > + > > > + of_id = of_match_device(of_sci_match, &pdev->dev); > > > + if (!of_id) > > > + return ERR_PTR(-EINVAL); > > > > > > - data = of_device_get_match_data(&pdev->dev); > > > + if (!strcmp(of_id->compatible, "renesas,scif-r9a07g044") || > > > + !strcmp(of_id->compatible, "renesas,sci")) { > > > > This will match on H8/300, too, which doesn't have resets. > > Please match against "renesas,sci-r9a07g044" instead. > > > > Please don't use explicit strcmp() calls here, but add a flag to > > of_sci_match[].data. > > > Agreed, does the below hunk look good for handling the reset? > > -#define SCI_OF_DATA(type, regtype) (void *)((type) << 16 | (regtype)) > +#define SCIx_RESET BIT(31) > +#define SCI_OF_DATA(type, regtype, reset) (void *)(reset | > (type) << 16 | (regtype)) > #define SCI_OF_TYPE(data) ((unsigned long)(data) >> 16) > #define SCI_OF_REGTYPE(data) ((unsigned long)(data) & 0xffff) > > @@ -3169,7 +3170,11 @@ static const struct of_device_id of_sci_match[] = { > }, > { > .compatible = "renesas,scif-r9a07g044", > - .data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE), > + .data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE, > SCIx_RESET), > + }, > + { > + .compatible = "renesas,sci-r9a07g044", > + .data = SCI_OF_DATA(PORT_SCI, SCIx_SCI_REGTYPE, SCIx_RESET), > }, That's what I had in mind. But upon second thought, it might be better to just drop the check, and obtain an optional reset instead? After all the reset requirement is not a feature of this specific SCI(F) variant, but an SoC integration feature. And deasserting the reset on other SoCs that have a reset should be fine. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] serial: sh-sci: Add reset support for RZ/G2L SoC 2021-11-09 7:49 ` Geert Uytterhoeven @ 2021-11-09 9:02 ` Lad, Prabhakar 0 siblings, 0 replies; 14+ messages in thread From: Lad, Prabhakar @ 2021-11-09 9:02 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Lad Prabhakar, Greg Kroah-Hartman, Rob Herring, Jiri Slaby, Philipp Zabel, open list:SERIAL DRIVERS, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux Kernel Mailing List, Linux-Renesas, Biju Das Hi Geert, On Tue, Nov 9, 2021 at 7:50 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Prabhakar, > > On Tue, Nov 9, 2021 at 1:34 AM Lad, Prabhakar > <prabhakar.csengg@gmail.com> wrote: > > On Mon, Nov 8, 2021 at 4:40 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Wed, Nov 3, 2021 at 6:31 PM Lad Prabhakar > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > On RZ/G2L devices should be explicitly pulled out of reset for it > > > > to work. This patch adds support to read the "resets" property and > > > > performs deassert/assert when required. > > > > > > > > Also, propagate the error to the caller of sci_parse_dt() instead of > > > > returning NULL in case of failure. > > > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > Thanks for your patch! > > > > > > > --- > > > > Hi Geert, > > > > For handling the resets I was in dual mind whether to perform > > > > reset based on compatible strings or soc-id, let me know your > > > > thoughts. Currently no SoC's use "renesas,sci" so using the same > > > > for performing the reset operation for SCI. > > > > > > We do, on H8/300. > > > > > Missed that. > > > > > > --- a/drivers/tty/serial/sh-sci.c > > > > +++ b/drivers/tty/serial/sh-sci.c > > > > @@ -3203,23 +3204,58 @@ static const struct of_device_id of_sci_match[] = { > > > > }; > > > > MODULE_DEVICE_TABLE(of, of_sci_match); > > > > > > > > +static void sci_reset_control_assert(void *data) > > > > +{ > > > > + reset_control_assert(data); > > > > +} > > > > + > > > > static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev, > > > > unsigned int *dev_id) > > > > { > > > > struct device_node *np = pdev->dev.of_node; > > > > + const struct of_device_id *of_id; > > > > struct plat_sci_port *p; > > > > struct sci_port *sp; > > > > const void *data; > > > > int id; > > > > > > > > if (!IS_ENABLED(CONFIG_OF) || !np) > > > > - return NULL; > > > > + return ERR_PTR(-EINVAL); > > > > + > > > > + of_id = of_match_device(of_sci_match, &pdev->dev); > > > > + if (!of_id) > > > > + return ERR_PTR(-EINVAL); > > > > > > > > - data = of_device_get_match_data(&pdev->dev); > > > > + if (!strcmp(of_id->compatible, "renesas,scif-r9a07g044") || > > > > + !strcmp(of_id->compatible, "renesas,sci")) { > > > > > > This will match on H8/300, too, which doesn't have resets. > > > Please match against "renesas,sci-r9a07g044" instead. > > > > > > Please don't use explicit strcmp() calls here, but add a flag to > > > of_sci_match[].data. > > > > > Agreed, does the below hunk look good for handling the reset? > > > > -#define SCI_OF_DATA(type, regtype) (void *)((type) << 16 | (regtype)) > > +#define SCIx_RESET BIT(31) > > +#define SCI_OF_DATA(type, regtype, reset) (void *)(reset | > > (type) << 16 | (regtype)) > > #define SCI_OF_TYPE(data) ((unsigned long)(data) >> 16) > > #define SCI_OF_REGTYPE(data) ((unsigned long)(data) & 0xffff) > > > > @@ -3169,7 +3170,11 @@ static const struct of_device_id of_sci_match[] = { > > }, > > { > > .compatible = "renesas,scif-r9a07g044", > > - .data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE), > > + .data = SCI_OF_DATA(PORT_SCIF, SCIx_RZ_SCIFA_REGTYPE, > > SCIx_RESET), > > + }, > > + { > > + .compatible = "renesas,sci-r9a07g044", > > + .data = SCI_OF_DATA(PORT_SCI, SCIx_SCI_REGTYPE, SCIx_RESET), > > }, > > That's what I had in mind. > > But upon second thought, it might be better to just drop the check, > and obtain an optional reset instead? Agreed, will do that. > After all the reset requirement is not a feature of this specific > SCI(F) variant, but an SoC integration feature. And deasserting > the reset on other SoCs that have a reset should be fine. > Indeed. Cheers, Prabhakar > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-11-09 9:03 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-03 17:31 [PATCH 0/3] RZ/G2L SCI support and sh-sci driver update Lad Prabhakar 2021-11-03 17:31 ` [PATCH 1/3] dt-bindings: serial: renesas,scif: Make resets as a required property Lad Prabhakar 2021-11-08 16:29 ` Geert Uytterhoeven 2021-11-09 0:36 ` Lad, Prabhakar 2021-11-03 17:31 ` [PATCH 2/3] dt-bindings: serial: renesas,sci: Document RZ/G2L SoC Lad Prabhakar 2021-11-08 16:19 ` Geert Uytterhoeven 2021-11-09 0:17 ` Lad, Prabhakar 2021-11-08 16:21 ` Geert Uytterhoeven 2021-11-09 0:19 ` Lad, Prabhakar 2021-11-03 17:31 ` [PATCH 3/3] serial: sh-sci: Add reset support for " Lad Prabhakar 2021-11-08 16:39 ` Geert Uytterhoeven 2021-11-09 0:33 ` Lad, Prabhakar 2021-11-09 7:49 ` Geert Uytterhoeven 2021-11-09 9:02 ` Lad, Prabhakar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).