[PATCH v5 0/9] Add RZ/G2L POEG support

[PATCH v5 0/9] Add RZ/G2L POEG support

[Biju Das]

The output pins of the general PWM timer (GPT) can be disabled by using the port
output enabling function for the GPT (POEG). Specifically, either of the
following ways can be used.

  * Input level detection of the GTETRGA to GTETRGD pins.
  * Output-disable request from the GPT.
  * Register setting(ie, by setting POEGGn.SSF to 1)

This patch series add support for controlling output disable function using sysfs.

For output disable operation, POEG group needs to be linked with GPT.
Plan to send a follow up patch with renesas,poeg-group as numeric
property in pwm bindings for linking both GPT and POEG devices.

Patch #3 to patch #9 are new and just for testing only(GPT Output-disable
request to POEG)

When dead time error occurs or the GTIOCA pin output value is
the same as the GTIOCB pin output value, output protection is
required. GPT detects this condition and generates output disable
requests to POEG based on the settings in the output disable request
permission bits, such as GTINTAD.GRPDTE, GTINTAD.GRPABH,
GTINTAD.GRPABL. After the POEG receives output disable requests from
each channel and calculates external input using an OR operation, the
POEG generates output disable requests to GPT.

POEG handles output disable request and send an event to userspace.
Userspace clears the fault condition and request poeg to cancel
the output disable request.

Logs:
root@smarc-rzg2l:~# /poeg_app &
[1] 182
root@smarc-rzg2l:~# [POEG]open

root@smarc-rzg2l:~# /poeg.sh
16
pwmchip0
Test case 1 user POEG control
 72:          0          0     GICv3 354 Level     10048800.poeg
 73:          0          0     GICv3 355 Level     10048c00.poeg
 74:          0          0     GICv3 356 Level     10049000.poeg
 76:          0          0     GICv3 357 Level     10049400.poeg
Read at address  0x10048434 (0xffff8d92b434): 0x0200031B
Read at address  0x10048438 (0xffff9884e438): 0x03000000
Read at address  0x10049400 (0xffffbd7b9400): 0x00000030
Test case 2 user GPT control both high
Read at address  0x10048434 (0xffff96dcf434): 0x021B031B
Read at address  0x10048438 (0xffff9ddfc438): 0x23000000
Read at address  0x10049400 (0xffff9a206400): 0x00000030
gpt ch:3, irq=2
gpt ch:3, irq=2
Read at address  0x10048434 (0xffff97fb2434): 0x021B031B
Read at address  0x10048438 (0xffff817a1438): 0x03000000
Read at address  0x10049400 (0xffffb8f5e400): 0x00000030
gpt ch:3, irq=2
 72:          0          0     GICv3 354 Level     10048800.poeg
 73:          0          0     GICv3 355 Level     10048c00.poeg
 74:          0          0     GICv3 356 Level     10049000.poeg
 76:          6          0     GICv3 357 Level     10049400.poeg
Test case 3 user GPT control both low
Read at address  0x10048434 (0xffff897d8434): 0x031B031B
Read at address  0x10048438 (0xffffa981a438): 0x43000000
Read at address  0x10049400 (0xffff90eba400): 0x00000030
gpt ch:3, irq=4
gpt ch:3, irq=4
Read at address  0x10048434 (0xffff959d7434): 0x021B031B
Read at address  0x10048438 (0xffff92b6d438): 0x03000000
Read at address  0x10049400 (0xffffb60b3400): 0x00000030
gpt ch:3, irq=4
 72:          0          0     GICv3 354 Level     10048800.poeg
 73:          0          0     GICv3 355 Level     10048c00.poeg
 74:          0          0     GICv3 356 Level     10049000.poeg
 76:         12          0     GICv3 357 Level     10049400.poeg
root@smarc-rzg2l:~#

v4->v5:
 * Added Rb tag from Rob.
 * Updated kernel version in sysfs doc.
v3->v4:
 * Replaced companion->renesas,gpt for the phandle to gpt instance
 * Replaced renesas,id->renesas,poeg-id
 * Removed default from renesas,poeg-id as default for a required
   property doesn't make much sense.
 * Updated the example and required properties with above changes
v2->v3:
 * Removed Rb tag from Rob as there are some changes introduced.
 * Added companion property, so that poeg can link with gpt device
 * Documented renesas,id, as identifier for POEGG{A,B,C,D}.
 * Updated the binding example.
 * Added sysfs documentation for output_disable
 * PWM_RZG2L_GPT implies ARCH_RZG2L. So removed ARCH_RZG2L dependency
 * Used dev_get_drvdata to get device data
 * Replaced sprintf->sysfs_emit in show().
v1->v2:
 * Updated binding description.
 * Renamed the file poeg-rzg2l->rzg2l-poeg
 * Removed the macro POEGG as there is only single register and
   updated rzg2l_poeg_write() and rzg2l_poeg_read()
 * Updated error handling in probe()
REF->v1:
 * Modelled as pincontrol as most of its configuration is intended to be
   static and moved driver files from soc to pincontrol directory.
 * Updated reg size in dt binding example.
 * Updated Kconfig

REF:
https://lore.kernel.org/linux-renesas-soc/20220510151112.16249-1-biju.das.jz@bp.renesas.com/


Biju Das (9):
  dt-bindings: pinctrl: renesas: Add RZ/G2L POEG binding
  drivers: pinctrl: renesas: Add RZ/G2L POEG driver support
  pwm: rzg2l-gpt: Add support for output disable request from gpt
  pinctrl: renesas: rzg2l-poeg: Add support for GPT Output-Disable
    Request
  pwm: rzg2l-gpt: Add support for output disable when both output low
  pinctrl: renesas: rzg2l-poeg: output-disable request from GPT when
    both outputs are low.
  pwm: rzg2l-gpt: Add support for output disable on dead time error
  pinctrl: renesas: rzg2l-poeg: output-disable request from GPT on dead
    time error
  tools/poeg: Add test app for poeg

 .../ABI/testing/sysfs-platform-rzg2l-poeg     |  87 ++++
 .../bindings/pinctrl/renesas,rzg2l-poeg.yaml  |  86 ++++
 drivers/pinctrl/renesas/Kconfig               |   2 +
 drivers/pinctrl/renesas/Makefile              |   2 +
 drivers/pinctrl/renesas/poeg/Kconfig          |  11 +
 drivers/pinctrl/renesas/poeg/Makefile         |   2 +
 drivers/pinctrl/renesas/poeg/rzg2l-poeg.c     | 476 ++++++++++++++++++
 drivers/pwm/pwm-rzg2l-gpt.c                   | 129 +++++
 include/linux/soc/renesas/rzg2l-gpt.h         |  44 ++
 include/linux/soc/renesas/rzg2l-poeg.h        |  16 +
 tools/poeg/Build                              |   1 +
 tools/poeg/Makefile                           |  53 ++
 tools/poeg/poeg_app.c                         |  60 +++
 13 files changed, 969 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-rzg2l-poeg
 create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-poeg.yaml
 create mode 100644 drivers/pinctrl/renesas/poeg/Kconfig
 create mode 100644 drivers/pinctrl/renesas/poeg/Makefile
 create mode 100644 drivers/pinctrl/renesas/poeg/rzg2l-poeg.c
 create mode 100644 include/linux/soc/renesas/rzg2l-gpt.h
 create mode 100644 include/linux/soc/renesas/rzg2l-poeg.h
 create mode 100644 tools/poeg/Build
 create mode 100644 tools/poeg/Makefile
 create mode 100644 tools/poeg/poeg_app.c

-- 
2.25.1

[PATCH v5 1/9] dt-bindings: pinctrl: renesas: Add RZ/G2L POEG binding

[Biju Das]

Add device tree bindings for the RZ/G2L Port Output Enable for GPT (POEG).

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v4->v5:
 * Added Rb tag from Rob.
v3->v4:
 * Replaced companion->renesas,gpt for the phandle to gpt instance
 * Replaced renesas,id->renesas,poeg-id
 * Removed default from renesas,poeg-id as default for a required
   property doesn't make much sense.
 * Updated the example and required properties with above changes
v2->v3:
 * Removed Rb tag from Rob as there are some changes introduced.
 * Added companion property, so that poeg can link with gpt device
 * Documented renesas,id, as identifier for POEGG{A,B,C,D}.
 * Updated the example.
v1->v2:
 * Updated the description.
REF->v1:
 * Modelled as pincontrol as most of its configuration is intended to be
   static.
 * Updated reg size in example.
---
 .../bindings/pinctrl/renesas,rzg2l-poeg.yaml  | 86 +++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-poeg.yaml

diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-poeg.yaml b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-poeg.yaml
new file mode 100644
index 000000000000..ab2d456c93e4
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-poeg.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/renesas,rzg2l-poeg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G2L Port Output Enable for GPT (POEG)
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+
+description: |
+  The output pins(GTIOCxA and GTIOCxB) of the general PWM timer (GPT) can be
+  disabled by using the port output enabling function for the GPT (POEG).
+  Specifically, either of the following ways can be used.
+  * Input level detection of the GTETRGA to GTETRGD pins.
+  * Output-disable request from the GPT.
+  * SSF bit setting(ie, by setting POEGGn.SSF to 1)
+
+  The state of the GTIOCxA and the GTIOCxB pins when the output is disabled,
+  are controlled by the GPT module.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,r9a07g044-poeg  # RZ/G2{L,LC}
+          - renesas,r9a07g054-poeg  # RZ/V2L
+      - const: renesas,rzg2l-poeg
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  renesas,gpt:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle to gpt instance that serves the pwm operation.
+
+  renesas,poeg-id:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 0, 1, 2, 3 ]
+    description: |
+      POEG group index. Valid values are:
+        <0> : POEG group A
+        <1> : POEG group B
+        <2> : POEG group C
+        <3> : POEG group D
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - power-domains
+  - resets
+  - renesas,poeg-id
+  - renesas,gpt
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r9a07g044-cpg.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    poeggd: poeg@10049400 {
+        compatible = "renesas,r9a07g044-poeg", "renesas,rzg2l-poeg";
+        reg = <0x10049400 0x400>;
+        interrupts = <GIC_SPI 325 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&cpg CPG_MOD R9A07G044_POEG_D_CLKP>;
+        power-domains = <&cpg>;
+        resets = <&cpg R9A07G044_POEG_D_RST>;
+        renesas,poeg-id = <3>;
+        renesas,gpt = <&gpt>;
+    };
-- 
2.25.1

Re: [PATCH v5 0/9] Add RZ/G2L POEG support

[Linus Walleij]

On Thu, Dec 15, 2022 at 10:32 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:

> This patch series add support for controlling output disable function using sysfs.

What's wrong with using the debugfs approach Drew implemented in
commit 6199f6becc869d30ca9394ca0f7a484bf9d598eb
"pinctrl: pinmux: Add pinmux-select debugfs file"
?

Something driver specific seems like a bit of a hack, does it not?

If this should go into sysfs we should probably create something
generic, such as a list of stuff to be exported as sysfs switches.

It generally also looks really dangerous, which is another reason
for keeping it in debugfs. It's the big hammer to hurt yourself with,
more or less.

Yours,
Linus Walleij

Re: [PATCH v5 1/9] dt-bindings: pinctrl: renesas: Add RZ/G2L POEG binding

[Linus Walleij]

On Thu, Dec 15, 2022 at 10:32 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:

> Add device tree bindings for the RZ/G2L Port Output Enable for GPT (POEG).
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Patch applied!

Yours,
Linus Walleij

Re: [PATCH v5 0/9] Add RZ/G2L POEG support

[Geert Uytterhoeven]

Hi Linus,

On Thu, Dec 29, 2022 at 2:17 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Dec 15, 2022 at 10:32 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > This patch series add support for controlling output disable function using sysfs.
>
> What's wrong with using the debugfs approach Drew implemented in
> commit 6199f6becc869d30ca9394ca0f7a484bf9d598eb
> "pinctrl: pinmux: Add pinmux-select debugfs file"
> ?

I think the main difference is that debugfs is meant for debugging
and development features, while this feature is to be configured on
production systems.  There's just no existing API for it.

> Something driver specific seems like a bit of a hack, does it not?
>
> If this should go into sysfs we should probably create something
> generic, such as a list of stuff to be exported as sysfs switches.
>
> It generally also looks really dangerous, which is another reason
> for keeping it in debugfs. It's the big hammer to hurt yourself with,
> more or less.

Yes, generic would be nice.  Anyone familiar with other hardware
that could make use of this?

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

Re: [PATCH v5 0/9] Add RZ/G2L POEG support

[Geert Uytterhoeven]

Hi Linus,

On Tue, Jan 3, 2023 at 10:01 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Thu, Dec 29, 2022 at 2:17 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Thu, Dec 15, 2022 at 10:32 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > This patch series add support for controlling output disable function using sysfs.
> >
> > What's wrong with using the debugfs approach Drew implemented in
> > commit 6199f6becc869d30ca9394ca0f7a484bf9d598eb
> > "pinctrl: pinmux: Add pinmux-select debugfs file"
> > ?
>
> I think the main difference is that debugfs is meant for debugging
> and development features, while this feature is to be configured on
> production systems.  There's just no existing API for it.
>
> > Something driver specific seems like a bit of a hack, does it not?
> >
> > If this should go into sysfs we should probably create something
> > generic, such as a list of stuff to be exported as sysfs switches.
> >
> > It generally also looks really dangerous, which is another reason
> > for keeping it in debugfs. It's the big hammer to hurt yourself with,
> > more or less.
>
> Yes, generic would be nice.  Anyone familiar with other hardware
> that could make use of this?

That's also the reason why I have been rather hesitant in accepting
this driver and bindings (I just saw you applied the bindings): I wanted
to hear your input first ;-)

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

Re: [PATCH v5 0/9] Add RZ/G2L POEG support

[Linus Walleij]

On Tue, Jan 3, 2023 at 10:01 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> > If this should go into sysfs we should probably create something
> > generic, such as a list of stuff to be exported as sysfs switches.
> >
> > It generally also looks really dangerous, which is another reason
> > for keeping it in debugfs. It's the big hammer to hurt yourself with,
> > more or less.
>
> Yes, generic would be nice.  Anyone familiar with other hardware
> that could make use of this?

Drew was using this for Beagle Bone IIRC, Drew?

Yours,
Linus Walleij

Re: [PATCH v5 0/9] Add RZ/G2L POEG support

[Geert Uytterhoeven]

Hi Linus,

On Mon, Jan 9, 2023 at 2:16 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Jan 3, 2023 at 10:01 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > If this should go into sysfs we should probably create something
> > > generic, such as a list of stuff to be exported as sysfs switches.
> > >
> > > It generally also looks really dangerous, which is another reason
> > > for keeping it in debugfs. It's the big hammer to hurt yourself with,
> > > more or less.
> >
> > Yes, generic would be nice.  Anyone familiar with other hardware
> > that could make use of this?
>
> Drew was using this for Beagle Bone IIRC, Drew?

Yes, that's what I remember, too.  And I tested it on Koelsch.

But again, that's for debugging purposes.  For non-debugging
operation, we need something different.

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

Re: [PATCH v5 0/9] Add RZ/G2L POEG support

[Andy Shevchenko]

On Mon, Jan 09, 2023 at 02:41:24PM +0100, Geert Uytterhoeven wrote:
> On Mon, Jan 9, 2023 at 2:16 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Tue, Jan 3, 2023 at 10:01 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > If this should go into sysfs we should probably create something
> > > > generic, such as a list of stuff to be exported as sysfs switches.
> > > >
> > > > It generally also looks really dangerous, which is another reason
> > > > for keeping it in debugfs. It's the big hammer to hurt yourself with,
> > > > more or less.
> > >
> > > Yes, generic would be nice.  Anyone familiar with other hardware
> > > that could make use of this?
> >
> > Drew was using this for Beagle Bone IIRC, Drew?
> 
> Yes, that's what I remember, too.  And I tested it on Koelsch.
> 
> But again, that's for debugging purposes.  For non-debugging
> operation, we need something different.

I really would like to know the use case when you need to mux pins at run time.
And the guarantee it won't break users' devices into smoke or killing somebody
playing with robots.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 0/9] Add RZ/G2L POEG support

[Andy Shevchenko]

On Mon, Jan 09, 2023 at 04:00:35PM +0200, Andy Shevchenko wrote:
> On Mon, Jan 09, 2023 at 02:41:24PM +0100, Geert Uytterhoeven wrote:
> > On Mon, Jan 9, 2023 at 2:16 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > > On Tue, Jan 3, 2023 at 10:01 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > If this should go into sysfs we should probably create something
> > > > > generic, such as a list of stuff to be exported as sysfs switches.
> > > > >
> > > > > It generally also looks really dangerous, which is another reason
> > > > > for keeping it in debugfs. It's the big hammer to hurt yourself with,
> > > > > more or less.
> > > >
> > > > Yes, generic would be nice.  Anyone familiar with other hardware
> > > > that could make use of this?
> > >
> > > Drew was using this for Beagle Bone IIRC, Drew?
> > 
> > Yes, that's what I remember, too.  And I tested it on Koelsch.
> > 
> > But again, that's for debugging purposes.  For non-debugging
> > operation, we need something different.
> 
> I really would like to know the use case when you need to mux pins at run time.
> And the guarantee it won't break users' devices into smoke or killing somebody
> playing with robots.

Btw, I might agree on having something like this in production, but enabling
it should print a BIG warning that the functionality is DANGEROUS. Something
like trace_printk() has.

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v5 0/9] Add RZ/G2L POEG support

[Biju Das]

Hi Linus Walleij,

Thanks for the feedback.

> Subject: Re: [PATCH v5 0/9] Add RZ/G2L POEG support
> 
> On Thu, Dec 15, 2022 at 10:32 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> 
> > This patch series add support for controlling output disable function
> using sysfs.
> 
> What's wrong with using the debugfs approach Drew implemented in commit
> 6199f6becc869d30ca9394ca0f7a484bf9d598eb
> "pinctrl: pinmux: Add pinmux-select debugfs file"
> ?

I am not sure, we supposed to use debugfs for production environment??
 
Currently output pins of the general PWM timer (GPT) can be disabled by using the below methods.

  1) Register setting(ie, by setting POEGGn.SSF to 1) --> by Software control
  2) Input level detection of the GTETRGA to GTETRGD pins->  sending an active level signal to disable the output pins of PWM.
  3) Output-disable request from the GPT--> Here GPT detects short circuits and request POEG to disable the output pins.

In case, if any one doesn't want to use 2) and 3), we should be able to disable output pins of the general PWM timer (GPT) by register control.

> 
> Something driver specific seems like a bit of a hack, does it not?
> 
> If this should go into sysfs we should probably create something generic,

Yes, generic sysfs entry will be good

> such as a list of stuff to be exported as sysfs switches.

Can you please elaborate? Or Point me to an example for this?

Cheers,
Biju

Re: [PATCH v5 0/9] Add RZ/G2L POEG support

[Linus Walleij]

On Mon, Jan 9, 2023 at 2:41 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Mon, Jan 9, 2023 at 2:16 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Tue, Jan 3, 2023 at 10:01 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > If this should go into sysfs we should probably create something
> > > > generic, such as a list of stuff to be exported as sysfs switches.
> > > >
> > > > It generally also looks really dangerous, which is another reason
> > > > for keeping it in debugfs. It's the big hammer to hurt yourself with,
> > > > more or less.
> > >
> > > Yes, generic would be nice.  Anyone familiar with other hardware
> > > that could make use of this?
> >
> > Drew was using this for Beagle Bone IIRC, Drew?
>
> Yes, that's what I remember, too.  And I tested it on Koelsch.
>
> But again, that's for debugging purposes.  For non-debugging
> operation, we need something different.

Actually Drew's usecase wasn't for debugging. It was kind-of production,
but it was for "one-offs" such as factory lines and other very specific-purpose
embedded.

The placement in debugfs was mostly because it is fragile and dangerous.

Yours,
Linus Walleij

Re: [PATCH v5 0/9] Add RZ/G2L POEG support

[Linus Walleij]

On Mon, Jan 9, 2023 at 4:10 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:

> > What's wrong with using the debugfs approach Drew implemented in commit
> > 6199f6becc869d30ca9394ca0f7a484bf9d598eb
> > "pinctrl: pinmux: Add pinmux-select debugfs file"
> > ?
>
> I am not sure, we supposed to use debugfs for production environment??

It depends what is meant by "production environment".

If you mean a controlled environment "one-off" such as a factory line,
a specific installation for a specific purpose such as a water purifier,
that is very custom and hacked together for that one usecase. It will
have other hacks too, so then Beagle is using debugfs in "production"
if that is what you mean by "production", i.e. used to produce something.

This is the same "production" use cases as used by i.e. the GPIO
character device.

If you mean that you are producing 6 million laptops where userspace is
going to hammer this constantly, then no. In that case a real sysfs
knob and ABI contract is needed.

Usually vendors know which usecase their hardware is intended for,
there is in my experience no unknown target audience, so which one is it in
your case?

> > such as a list of stuff to be exported as sysfs switches.
>
> Can you please elaborate? Or Point me to an example for this?

Not sure what to say about that, you will have to invent something I'm
afraid, good examples are in Documentation/ABI.

Yours,
Linus Walleij

RE: [PATCH v5 0/9] Add RZ/G2L POEG support

[Biju Das]

Hi Linus Walleij,

Thanks for the feedback.

> Subject: Re: [PATCH v5 0/9] Add RZ/G2L POEG support
> 
> On Mon, Jan 9, 2023 at 4:10 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 
> > > What's wrong with using the debugfs approach Drew implemented in
> > > commit 6199f6becc869d30ca9394ca0f7a484bf9d598eb
> > > "pinctrl: pinmux: Add pinmux-select debugfs file"
> > > ?
> >
> > I am not sure, we supposed to use debugfs for production environment??
> 
> It depends what is meant by "production environment".

Sorry for the confusion. I meant the final product used by the customers.

I was under the impression that debugfs is for hacks and debugging.
(For eg: if the HW doesn't have a USB3 function port, but using debugfs,
we can emulate the condition and test)

> 
> If you mean a controlled environment "one-off" such as a factory line, a
> specific installation for a specific purpose such as a water purifier, that
> is very custom and hacked together for that one usecase. It will have other
> hacks too, so then Beagle is using debugfs in "production"
> if that is what you mean by "production", i.e. used to produce something.
> 
> This is the same "production" use cases as used by i.e. the GPIO character
> device.
> 
> If you mean that you are producing 6 million laptops where userspace is
> going to hammer this constantly, then no. In that case a real sysfs knob and
> ABI contract is needed.
> 
> Usually vendors know which usecase their hardware is intended for, there is
> in my experience no unknown target audience, so which one is it in your
> case?

POEG use case is related to protection from system failure(disable output pins in case of short circuits)

Either

1) we detect externally and use software control to disable output pins --> Here I am using sysfs variable
(/sys/devices/platform/soc/10049400.poeg/output_disable) to control it.

Or

2) we detect externally and send an active level signal to disable output pins

Or

3) we detect internally using GPT(PWM) and disable output pins --> Here 3 options or combination is possible
                              for configuring the short circuit detection like
		1) Dead Time Error Output Disable Request Enable
		2) Same Time Output Level High Disable Request Enable
		3) Same Time Output Level Low Disable Request Enable

I have exported 3 sysfs variables for configuring these 3 options.
  1) /sys/devices/platform/soc/10049400.poeg/gpt_req_both_high
  2) /sys/devices/platform/soc/10049400.poeg/gpt_req_both_low
  3) /sys/devices/platform/soc/10049400.poeg/gpt_req_deadtime_err

> 
> > > such as a list of stuff to be exported as sysfs switches.
> >
> > Can you please elaborate? Or Point me to an example for this?
> 
> Not sure what to say about that, you will have to invent something I'm
> afraid, good examples are in Documentation/ABI.

If it is preferable to use debugfs compared to sysfs for the use cases I mentioned above,
I could change it to debugfs like Drew's patch. Please let me know.

Cheers,
Biju

Re: [PATCH v5 0/9] Add RZ/G2L POEG support

[Drew Fustini]

On Tue, Jan 10, 2023 at 09:09:21AM +0100, Linus Walleij wrote:
> On Mon, Jan 9, 2023 at 2:41 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Mon, Jan 9, 2023 at 2:16 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > > On Tue, Jan 3, 2023 at 10:01 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > If this should go into sysfs we should probably create something
> > > > > generic, such as a list of stuff to be exported as sysfs switches.
> > > > >
> > > > > It generally also looks really dangerous, which is another reason
> > > > > for keeping it in debugfs. It's the big hammer to hurt yourself with,
> > > > > more or less.
> > > >
> > > > Yes, generic would be nice.  Anyone familiar with other hardware
> > > > that could make use of this?
> > >
> > > Drew was using this for Beagle Bone IIRC, Drew?
> >
> > Yes, that's what I remember, too.  And I tested it on Koelsch.
> >
> > But again, that's for debugging purposes.  For non-debugging
> > operation, we need something different.
> 
> Actually Drew's usecase wasn't for debugging. It was kind-of production,
> but it was for "one-offs" such as factory lines and other very specific-purpose
> embedded.
> 
> The placement in debugfs was mostly because it is fragile and dangerous.
> 
> Yours,
> Linus Walleij

For the BeagleBone, the use case for selecting pin fuctions from
userspace with pinmux-select in debugfs is to allow for rapid
prototyping situations such as breadboarding. Our Debian install on the
boards has an utility named config-pin that allows the user to select
between defined pinctrl states for each pin on the expansion header.

Some users like this as it means they do not need to constantly be
editing device tree files and rebooting while protoyping. I agree that
this is not a fool-proof scheme but Beaglebones have been shipping with
this functionality for many years without any significant problems that
I'm aware of.

I do admit that it is possible for someone to potentially damage
circuits with this flexibility, so having a warning in the kernel log
like Andy suggested elsewhere in this thread might be a good idea.

Thanks,
Drew

RE: [PATCH v5 0/9] Add RZ/G2L POEG support

[Biju Das]

Hi Linus,

Thanks for feedback.

> Subject: Re: [PATCH v5 0/9] Add RZ/G2L POEG support
> 
> On Mon, Jan 9, 2023 at 4:10 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 
> > > What's wrong with using the debugfs approach Drew implemented in
> > > commit 6199f6becc869d30ca9394ca0f7a484bf9d598eb
> > > "pinctrl: pinmux: Add pinmux-select debugfs file"
> > > ?
> >
> > I am not sure, we supposed to use debugfs for production environment??
> 
> It depends what is meant by "production environment".
> 
> If you mean a controlled environment "one-off" such as a factory line, a
> specific installation for a specific purpose such as a water purifier, that
> is very custom and hacked together for that one usecase. It will have other
> hacks too, so then Beagle is using debugfs in "production"
> if that is what you mean by "production", i.e. used to produce something.
> 
> This is the same "production" use cases as used by i.e. the GPIO character
> device.
> 
> If you mean that you are producing 6 million laptops where userspace is
> going to hammer this constantly, then no. In that case a real sysfs knob and
> ABI contract is needed.
> 
> Usually vendors know which usecase their hardware is intended for, there is
> in my experience no unknown target audience, so which one is it in your
> case?
> 
> > > such as a list of stuff to be exported as sysfs switches.
> >
> > Can you please elaborate? Or Point me to an example for this?
> 
> Not sure what to say about that, you will have to invent something I'm
> afraid, good examples are in Documentation/ABI.


For generic approach, here is my plan

From userspace

echo "fname gname config configvalue" > output_disable

At core level, we identify the device associated with the above "fname gname" --> this will be a new api.

Core calls respective pincontrol driver with output_disable_cb(dev, fname, gname, config configvalue)--> there will be a new cb in pinctrl.

The pincontrol driver has a list of devices with device_output_disable callbacks
which will be registered by different drivers (eg:poeg_driver, poe3-driver)

The poeg_driver gets callback and it will match against "fname gname" and configures the value.

If anything fails, it will report error and propagates to user space.

I will prototype based on the above. Please let me know if you have different opinion.

Cheers,
Biju