| Message ID | 163742290656.715.15960553560678858057.sendpatchset@octo (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | r8a77995 Draak SCIF0 LED and KEY Serdev prototype | expand |
Hi Magnus, On Sat, Nov 20, 2021 at 5:32 PM Magnus Damm <damm@opensource.se> wrote: > From: Magnus Damm <damm+renesas@opensource.se> > > Here's a work-in-progress patch for shared pin LED and KEY functionality: > - UART TX Serdev LED driver prototype (functional) > - UART RX Serdev KEY driver prototype (partial) > - r8a77995 Draak DTS modifications to use above drivers with SCIF0 > > With this code my hope is to use hardware to drive an LED and allow > detection of a key press without software performing any kind of polling. > > In theory on SoCs that support UART RX and TX on the same pin (and also > open drain output) with the above software it is possible to handle boards > with single pin shared LED and KEY functionality. > > This prototype on r8a77995 Draak makes use of 3 pins and an external circuit: > - LED13/SW59/GP4_07 <-> EXIO_A:10 (CN46) > - SCIF0_RX/GP4_20 <- EXIO_A:38 (CN46) > - SCIF0_TX/GP4_21 -> EXIO_A:36 (CN46) > Ether-AVB PHY connector (CN23) has 3.3V on pin 54 and 56 and GND on 14 > In the future SCIF1 and SCIF3 may also be used for other LEDs and switches. > > Currently two inverters on SN74HC05 together with pull-ups are used to extend > the D3 SoC and the Draak board with open drain functionality and also tie > together the TX and RX pins with LED13/SW59. > > The prototype LED driver allows user space to turn on/off the LED using: > # echo 1 > /sys/class/leds/serial0-0/brightness > # echo 0 > /sys/class/leds/serial0-0/brightness > Must be easy to extend the driver with some degree of brightness control. > > Apart from some general brush up the following issues have surfaced: > - "controller busy" error happens when more than one serdev is used > - it is unclear how to take RX errors from serdev and generate key events > - there seem to be no way to silence "sh-sci e6e60000.serial: frame error" > - the DTS "current-speed" property looks like sw config and not hw description > > Obviously not for upstream merge as-is. Might however be useful as SCIF error > test bench and/or as potential (corner) use case for serdev. > > Not-Yet-Signed-off-by: Magnus Damm <damm+renesas@opensource.se> Thanks for your patch, which is definitely an interesting approach! > --- 0001/arch/arm64/boot/dts/renesas/r8a77995-draak.dts > +++ work/arch/arm64/boot/dts/renesas/r8a77995-draak.dts 2021-11-20 23:47:14.965609878 +0900 > @@ -479,13 +495,29 @@ > status = "okay"; > }; > > +&scif0 { > + pinctrl-0 = <&scif0_pins>; > + pinctrl-names = "default"; > + > + status = "okay"; > +#if 1 > + led { > + compatible = "serdev,led"; > + current-speed = <9600>; > + }; > +#else > + key { > + compatible = "serdev,key"; > + current-speed = <9600>; > + }; > +#endif So LED and key are still mutually-exclusive, despite using 3 signals into the SoC? > +}; 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
Hi Magnus, Quoting Magnus Damm (2021-11-20 15:41:46) > From: Magnus Damm <damm+renesas@opensource.se> > > Here's a work-in-progress patch for shared pin LED and KEY functionality: > - UART TX Serdev LED driver prototype (functional) > - UART RX Serdev KEY driver prototype (partial) > - r8a77995 Draak DTS modifications to use above drivers with SCIF0 > > With this code my hope is to use hardware to drive an LED and allow > detection of a key press without software performing any kind of polling. > > In theory on SoCs that support UART RX and TX on the same pin (and also > open drain output) with the above software it is possible to handle boards > with single pin shared LED and KEY functionality. > > This prototype on r8a77995 Draak makes use of 3 pins and an external circuit: > - LED13/SW59/GP4_07 <-> EXIO_A:10 (CN46) > - SCIF0_RX/GP4_20 <- EXIO_A:38 (CN46) > - SCIF0_TX/GP4_21 -> EXIO_A:36 (CN46) > Ether-AVB PHY connector (CN23) has 3.3V on pin 54 and 56 and GND on 14 > In the future SCIF1 and SCIF3 may also be used for other LEDs and switches. > > Currently two inverters on SN74HC05 together with pull-ups are used to extend > the D3 SoC and the Draak board with open drain functionality and also tie > together the TX and RX pins with LED13/SW59. > > The prototype LED driver allows user space to turn on/off the LED using: > # echo 1 > /sys/class/leds/serial0-0/brightness > # echo 0 > /sys/class/leds/serial0-0/brightness > Must be easy to extend the driver with some degree of brightness control. > > Apart from some general brush up the following issues have surfaced: > - "controller busy" error happens when more than one serdev is used > - it is unclear how to take RX errors from serdev and generate key events > - there seem to be no way to silence "sh-sci e6e60000.serial: frame error" > - the DTS "current-speed" property looks like sw config and not hw description > > Obviously not for upstream merge as-is. Might however be useful as SCIF error > test bench and/or as potential (corner) use case for serdev. Very interesting use of the DMA capabilities for the SCIF to generate output on the lines. What's the maximum speed of the SCIF? I could see this being further used to provide a software defined controller for RGB LEDs [0], which have often previously used SPI in a similar fashion to your proposal [1]. https://github.com/msperl/rgbled-fb/blob/master/ws2812b-spi-fb.c https://www.arrow.com/en/research-and-events/articles/protocol-for-the-ws2812b-programmable-led > Not-Yet-Signed-off-by: Magnus Damm <damm+renesas@opensource.se> > --- > > arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 34 ++++++ > drivers/tty/serdev/Makefile | 2 > drivers/tty/serdev/barfoo.c | 99 +++++++++++++++++++ > drivers/tty/serdev/foobar.c | 121 ++++++++++++++++++++++++ > 4 files changed, 254 insertions(+), 2 deletions(-) > > --- 0001/arch/arm64/boot/dts/renesas/r8a77995-draak.dts > +++ work/arch/arm64/boot/dts/renesas/r8a77995-draak.dts 2021-11-20 23:47:14.965609878 +0900 > @@ -16,6 +16,8 @@ > > aliases { > serial0 = &scif2; > + serial1 = &scif0; > + > ethernet0 = &avb; > }; > > @@ -226,6 +228,15 @@ > clock-frequency = <48000000>; > }; > > +&gpio4 { > + gp4_07_led13_sw59 { > + gpio-hog; > + gpios = <7 GPIO_ACTIVE_HIGH>; > + input; > + line-name = "gp4_07"; > + }; > +}; > + > &hsusb { > dr_mode = "host"; > status = "okay"; > @@ -432,6 +443,11 @@ > function = "pwm1"; > }; > > + scif0_pins: scif0 { > + groups = "scif0_data_a"; > + function = "scif0"; > + }; > + > scif2_pins: scif2 { > groups = "scif2_data"; > function = "scif2"; > @@ -479,13 +495,29 @@ > status = "okay"; > }; > > +&scif0 { > + pinctrl-0 = <&scif0_pins>; > + pinctrl-names = "default"; > + > + status = "okay"; > +#if 1 > + led { > + compatible = "serdev,led"; > + current-speed = <9600>; > + }; > +#else > + key { > + compatible = "serdev,key"; > + current-speed = <9600>; > + }; > +#endif > +}; > &scif2 { > pinctrl-0 = <&scif2_pins>; > pinctrl-names = "default"; > > status = "okay"; > }; > - > &sdhi2 { > /* used for on-board eMMC */ > pinctrl-0 = <&sdhi2_pins>; > --- 0001/drivers/tty/serdev/Makefile > +++ work/drivers/tty/serdev/Makefile 2021-11-20 23:08:15.925462579 +0900 > @@ -1,5 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0 > -serdev-objs := core.o > +serdev-objs := core.o foobar.o barfoo.o > > obj-$(CONFIG_SERIAL_DEV_BUS) += serdev.o > > --- /dev/null > +++ work/drivers/tty/serdev/barfoo.c 2021-11-20 23:42:21.628591406 +0900 > @@ -0,0 +1,99 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Serdev Push Switch Device Driver Prototype > + * Copyright (C) 2021 Magnus Damm > + * > + * Detect a key press using the RX pin function of an UART > + * > + * This assumes the RX pin is kept in high state one way or the other and > + * the push switch will pull down the pin once asserted. > + * > + * The idea is that any kind of RX errors will be treated as a key press: > + * Frame errors, Parity errors and/or Break > + * > + * Currently with the RX line being idle asserting the switch results in: > + * # [ 18.627197] barfoo_receive_buf 1: > + * [ 18.627283] 0x00 > + * [ 18.636261] sh-sci e6e60000.serial: frame error > + * [ 18.653335] > + * [ 18.653335] barfoo_receive_buf - done > + */ > + > +#include <linux/errno.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/leds.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/serdev.h> > + > +struct key_priv { > + struct serdev_device *serdev; > + u32 bps; > +}; > + > +static int key_receive_buf(struct serdev_device *serdev, > + const unsigned char *buf, size_t size) > +{ > + int k; > + > + printk("barfoo_receive_buf %d: ", (int)size); > + > + for (k = 0; k < size; k++) { > + printk("0x%02x ", buf[k]); > + } > + > + printk("\nbarfoo_receive_buf - done\n"); > + > + return size; > +} > + > +static const struct serdev_device_ops key_serdev_device_ops = { > + .receive_buf = key_receive_buf, > + .write_wakeup = serdev_device_write_wakeup, > +}; > + > +static int key_probe(struct serdev_device *serdev) > +{ > + struct key_priv *p; > + int ret; > + > + p = devm_kzalloc(&serdev->dev, sizeof(*p), GFP_KERNEL); > + if (!p) > + return -ENOMEM; > + > + p->serdev = serdev; > + dev_set_drvdata(&serdev->dev, p); > + > + if (of_property_read_u32(serdev->dev.of_node, "current-speed", &p->bps)) > + return -EINVAL; > + > + serdev_device_set_client_ops(serdev, &key_serdev_device_ops); > + ret = devm_serdev_device_open(&serdev->dev, serdev); > + if (ret) > + return ret; > + > + serdev_device_set_baudrate(serdev, p->bps); > + serdev_device_set_flow_control(serdev, false); > + return serdev_device_set_parity(serdev, SERDEV_PARITY_NONE); > +} > + > +static void key_remove(struct serdev_device *serdev) > +{ > +}; > + > +static const struct of_device_id key_of_match[] = { > + { .compatible = "serdev,key" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, key_of_match); > + > +static struct serdev_device_driver key_driver = { > + .driver = { > + .name = "serdev-key", > + .of_match_table = of_match_ptr(key_of_match), > + }, > + .probe = key_probe, > + .remove = key_remove, > +}; > +module_serdev_device_driver(key_driver); > --- /dev/null > +++ work/drivers/tty/serdev/foobar.c 2021-11-20 23:42:35.800592298 +0900 > @@ -0,0 +1,121 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Serdev LED Device Driver Prototype > + * Copyright (C) 2021 Magnus Damm > + * > + * Control brightness of an LED using the TX pin of an UART > + * > + * At this time two levels of brightness are supported: > + * Brightness 1: Make LED lit by setting UART TX pin to idle state > + * Brightness 0: Send dim data pattern 0x01 which keeps pin mostly low > + * > + * The above UART data patterns may optionally be used with an external open > + * drain circuit driving both the LED and a push switch using a single pin. > + */ > + > +#include <linux/errno.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/leds.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/serdev.h> > +#include <linux/workqueue.h> > + > +struct led_priv { > + struct serdev_device *serdev; > + u32 bps; > + struct led_classdev lcd; > + unsigned int led_brightness; > + struct delayed_work work; > + unsigned char buf[128]; /* dim pattern data */ > +}; > + > +static void led_work(struct work_struct *work) > +{ > + struct led_priv *p = container_of(work, struct led_priv, work.work); > + unsigned int bits_queued; > + int ret; > + > + /* wait in case all dim pattern data is not sent out yet */ > + serdev_device_wait_until_sent(p->serdev, 0); > + > + if (p->led_brightness) { > + /* uart line idle state is high so do nothing */ > + return; > + } > + > + /* output continuous dim pattern */ > + ret = serdev_device_write_buf(p->serdev, p->buf, sizeof(p->buf)); > + bits_queued = (ret > 0 ? ret : 1) * 10; > + schedule_delayed_work(&p->work, (HZ * bits_queued) / p->bps); > +} > + > +static void led_brightness_set(struct led_classdev *lcdp, > + enum led_brightness brightness) > +{ > + struct led_priv *p = container_of(lcdp, struct led_priv, lcd); > + > + p->led_brightness = (int)brightness; > + schedule_delayed_work(&p->work, 0); > +} > + > +static const struct serdev_device_ops led_serdev_device_ops = { > + .write_wakeup = serdev_device_write_wakeup, > +}; > + > +static int led_probe(struct serdev_device *serdev) > +{ > + struct led_priv *p; > + int ret; > + > + p = devm_kzalloc(&serdev->dev, sizeof(*p), GFP_KERNEL); > + if (!p) > + return -ENOMEM; > + > + p->serdev = serdev; > + dev_set_drvdata(&serdev->dev, p); > + > + if (of_property_read_u32(serdev->dev.of_node, "current-speed", &p->bps)) > + return -EINVAL; > + > + memset(p->buf, sizeof(p->buf), 1); /* almost zero brightness */ > + INIT_DELAYED_WORK(&p->work, led_work); > + p->lcd.name = dev_name(&serdev->dev); > + p->lcd.max_brightness = 1; > + p->lcd.brightness_set = led_brightness_set; > + > + ret = devm_led_classdev_register_ext(&serdev->dev, &p->lcd, NULL); > + > + serdev_device_set_client_ops(serdev, &led_serdev_device_ops); > + ret = devm_serdev_device_open(&serdev->dev, serdev); > + if (ret) > + return ret; > + > + serdev_device_set_baudrate(serdev, p->bps); > + serdev_device_set_flow_control(serdev, false); > + return serdev_device_set_parity(serdev, SERDEV_PARITY_NONE); > +} > + > +static void led_remove(struct serdev_device *serdev) > +{ > + struct led_priv *p = dev_get_drvdata(&serdev->dev); > + > + cancel_delayed_work_sync(&p->work); > +}; > + > +static const struct of_device_id led_of_match[] = { > + { .compatible = "serdev,led" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, led_of_match); > + > +static struct serdev_device_driver led_driver = { > + .driver = { > + .name = "serdev-led", > + .of_match_table = of_match_ptr(led_of_match), > + }, > + .probe = led_probe, > + .remove = led_remove, > +}; > +module_serdev_device_driver(led_driver);
Hi Geert, On Mon, Nov 22, 2021 at 5:02 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Magnus, > > On Sat, Nov 20, 2021 at 5:32 PM Magnus Damm <damm@opensource.se> wrote: > > From: Magnus Damm <damm+renesas@opensource.se> > > > > Here's a work-in-progress patch for shared pin LED and KEY functionality: > > - UART TX Serdev LED driver prototype (functional) > > - UART RX Serdev KEY driver prototype (partial) > > - r8a77995 Draak DTS modifications to use above drivers with SCIF0 > > > > With this code my hope is to use hardware to drive an LED and allow > > detection of a key press without software performing any kind of polling. > > > > In theory on SoCs that support UART RX and TX on the same pin (and also > > open drain output) with the above software it is possible to handle boards > > with single pin shared LED and KEY functionality. > > > > This prototype on r8a77995 Draak makes use of 3 pins and an external circuit: > > - LED13/SW59/GP4_07 <-> EXIO_A:10 (CN46) > > - SCIF0_RX/GP4_20 <- EXIO_A:38 (CN46) > > - SCIF0_TX/GP4_21 -> EXIO_A:36 (CN46) > > Ether-AVB PHY connector (CN23) has 3.3V on pin 54 and 56 and GND on 14 > > In the future SCIF1 and SCIF3 may also be used for other LEDs and switches. > > > > Currently two inverters on SN74HC05 together with pull-ups are used to extend > > the D3 SoC and the Draak board with open drain functionality and also tie > > together the TX and RX pins with LED13/SW59. > > > > The prototype LED driver allows user space to turn on/off the LED using: > > # echo 1 > /sys/class/leds/serial0-0/brightness > > # echo 0 > /sys/class/leds/serial0-0/brightness > > Must be easy to extend the driver with some degree of brightness control. > > > > Apart from some general brush up the following issues have surfaced: > > - "controller busy" error happens when more than one serdev is used > > - it is unclear how to take RX errors from serdev and generate key events > > - there seem to be no way to silence "sh-sci e6e60000.serial: frame error" > > - the DTS "current-speed" property looks like sw config and not hw description > > > > Obviously not for upstream merge as-is. Might however be useful as SCIF error > > test bench and/or as potential (corner) use case for serdev. > > > > Not-Yet-Signed-off-by: Magnus Damm <damm+renesas@opensource.se> > > Thanks for your patch, which is definitely an interesting approach! Thanks! I've been meaning to look into serdev for a while now and scratching an itch like this is one way to get acquainted with the code base. > > --- 0001/arch/arm64/boot/dts/renesas/r8a77995-draak.dts > > +++ work/arch/arm64/boot/dts/renesas/r8a77995-draak.dts 2021-11-20 23:47:14.965609878 +0900 > > @@ -479,13 +495,29 @@ > > status = "okay"; > > }; > > > > +&scif0 { > > + pinctrl-0 = <&scif0_pins>; > > + pinctrl-names = "default"; > > + > > + status = "okay"; > > +#if 1 > > + led { > > + compatible = "serdev,led"; > > + current-speed = <9600>; > > + }; > > +#else > > + key { > > + compatible = "serdev,key"; > > + current-speed = <9600>; > > + }; > > +#endif > > So LED and key are still mutually-exclusive, despite using 3 signals > into the SoC? In this patch indeed they are, yes. It is quite possible to enable both LED and key in the DTS today, but during runtime I could not get both drivers to load. As you say, the three signals into the SoC (instead of 1) is currently working around a hardware limitation. The mutual-exclusiveness with one serdev driver enabled out of two is however working around a software limitation. It seems that there is room for improvement both on the hardware and the software side. The main reason for the software limitation seems to be that currently the serdev core does not support more than one driver at a time, see "controller busy" error statement in: https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/tree/drivers/tty/serdev/core.c?h=renesas-drivers-2021-11-16-v5.16-rc1 To be honest, my approach of two serdev drivers in parallel with one serial port device might not exactly be how multi-device support with serdev was envisioned. At the same time I can't really imagine what more of a sane multi-device serdev use case would be. Any ideas? Cheers, / magnus
Hi Kieran, On Mon, Nov 22, 2021 at 9:25 PM Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Hi Magnus, > > Quoting Magnus Damm (2021-11-20 15:41:46) > > From: Magnus Damm <damm+renesas@opensource.se> > > > > Here's a work-in-progress patch for shared pin LED and KEY functionality: > > - UART TX Serdev LED driver prototype (functional) > > - UART RX Serdev KEY driver prototype (partial) > > - r8a77995 Draak DTS modifications to use above drivers with SCIF0 > > > > With this code my hope is to use hardware to drive an LED and allow > > detection of a key press without software performing any kind of polling. > > > > In theory on SoCs that support UART RX and TX on the same pin (and also > > open drain output) with the above software it is possible to handle boards > > with single pin shared LED and KEY functionality. > > > > This prototype on r8a77995 Draak makes use of 3 pins and an external circuit: > > - LED13/SW59/GP4_07 <-> EXIO_A:10 (CN46) > > - SCIF0_RX/GP4_20 <- EXIO_A:38 (CN46) > > - SCIF0_TX/GP4_21 -> EXIO_A:36 (CN46) > > Ether-AVB PHY connector (CN23) has 3.3V on pin 54 and 56 and GND on 14 > > In the future SCIF1 and SCIF3 may also be used for other LEDs and switches. > > > > Currently two inverters on SN74HC05 together with pull-ups are used to extend > > the D3 SoC and the Draak board with open drain functionality and also tie > > together the TX and RX pins with LED13/SW59. > > > > The prototype LED driver allows user space to turn on/off the LED using: > > # echo 1 > /sys/class/leds/serial0-0/brightness > > # echo 0 > /sys/class/leds/serial0-0/brightness > > Must be easy to extend the driver with some degree of brightness control. > > > > Apart from some general brush up the following issues have surfaced: > > - "controller busy" error happens when more than one serdev is used > > - it is unclear how to take RX errors from serdev and generate key events > > - there seem to be no way to silence "sh-sci e6e60000.serial: frame error" > > - the DTS "current-speed" property looks like sw config and not hw description > > > > Obviously not for upstream merge as-is. Might however be useful as SCIF error > > test bench and/or as potential (corner) use case for serdev. > > Very interesting use of the DMA capabilities for the SCIF to generate > output on the lines. Indeed there are quite a few hardware components working together to emulate a low signal by sending a mostly-zero pattern on the UART TX line. So far I've adjusted the size of the data buffers to avoid too many wake ups per second for transmitting. In my opinion it would be even better to use some sort of circular DMA mode and have zero wake ups. I'm quite sure the hardware would support such configuration, but how to make it happen with software is a different story. > What's the maximum speed of the SCIF? I could see this being further > used to provide a software defined controller for RGB LEDs [0], which > have often previously used SPI in a similar fashion to your proposal [1]. > > https://github.com/msperl/rgbled-fb/blob/master/ws2812b-spi-fb.c > https://www.arrow.com/en/research-and-events/articles/protocol-for-the-ws2812b-programmable-led Thanks. Indeed a software defined controller for RGB LEDs would be very interesting to see. The maximum speed of the SCIF is most likely in the Mbit-range but it really depends on the SoC type and perhaps also board specific external crystal configuration. And of course the device that the UART is connected to together with the quality of the PCB. Initially I had planned to rely on UART hardware break condition detection for the key press event, but with the 9600 bps on my SN75HC05 breadboard prototype the signals are looking pretty noisy. Now I have adjusted my expectation to "any UART error condition" for the key detection to make something work with what I've got. =) But to get those UART errors related to the key event into the serdev driver some framework extensions would most likely be needed. Cheers, / magnus
Hi Kieran, On Mon, Nov 22, 2021 at 1:25 PM Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > What's the maximum speed of the SCIF? I could see this being further > used to provide a software defined controller for RGB LEDs [0], which > have often previously used SPI in a similar fashion to your proposal [1]. The problem with using a SCIF variant to drive WS2812B RGB LEDs won't be maximum speed, but speed accuracy and precise control of bits. I think you'd be better off trying this with MSIOF instead. > https://github.com/msperl/rgbled-fb/blob/master/ws2812b-spi-fb.c > https://www.arrow.com/en/research-and-events/articles/protocol-for-the-ws2812b-programmable-led 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
--- 0001/arch/arm64/boot/dts/renesas/r8a77995-draak.dts +++ work/arch/arm64/boot/dts/renesas/r8a77995-draak.dts 2021-11-20 23:47:14.965609878 +0900 @@ -16,6 +16,8 @@ aliases { serial0 = &scif2; + serial1 = &scif0; + ethernet0 = &avb; }; @@ -226,6 +228,15 @@ clock-frequency = <48000000>; }; +&gpio4 { + gp4_07_led13_sw59 { + gpio-hog; + gpios = <7 GPIO_ACTIVE_HIGH>; + input; + line-name = "gp4_07"; + }; +}; + &hsusb { dr_mode = "host"; status = "okay"; @@ -432,6 +443,11 @@ function = "pwm1"; }; + scif0_pins: scif0 { + groups = "scif0_data_a"; + function = "scif0"; + }; + scif2_pins: scif2 { groups = "scif2_data"; function = "scif2"; @@ -479,13 +495,29 @@ status = "okay"; }; +&scif0 { + pinctrl-0 = <&scif0_pins>; + pinctrl-names = "default"; + + status = "okay"; +#if 1 + led { + compatible = "serdev,led"; + current-speed = <9600>; + }; +#else + key { + compatible = "serdev,key"; + current-speed = <9600>; + }; +#endif +}; &scif2 { pinctrl-0 = <&scif2_pins>; pinctrl-names = "default"; status = "okay"; }; - &sdhi2 { /* used for on-board eMMC */ pinctrl-0 = <&sdhi2_pins>; --- 0001/drivers/tty/serdev/Makefile +++ work/drivers/tty/serdev/Makefile 2021-11-20 23:08:15.925462579 +0900 @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -serdev-objs := core.o +serdev-objs := core.o foobar.o barfoo.o obj-$(CONFIG_SERIAL_DEV_BUS) += serdev.o --- /dev/null +++ work/drivers/tty/serdev/barfoo.c 2021-11-20 23:42:21.628591406 +0900 @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Serdev Push Switch Device Driver Prototype + * Copyright (C) 2021 Magnus Damm + * + * Detect a key press using the RX pin function of an UART + * + * This assumes the RX pin is kept in high state one way or the other and + * the push switch will pull down the pin once asserted. + * + * The idea is that any kind of RX errors will be treated as a key press: + * Frame errors, Parity errors and/or Break + * + * Currently with the RX line being idle asserting the switch results in: + * # [ 18.627197] barfoo_receive_buf 1: + * [ 18.627283] 0x00 + * [ 18.636261] sh-sci e6e60000.serial: frame error + * [ 18.653335] + * [ 18.653335] barfoo_receive_buf - done + */ + +#include <linux/errno.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/leds.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/serdev.h> + +struct key_priv { + struct serdev_device *serdev; + u32 bps; +}; + +static int key_receive_buf(struct serdev_device *serdev, + const unsigned char *buf, size_t size) +{ + int k; + + printk("barfoo_receive_buf %d: ", (int)size); + + for (k = 0; k < size; k++) { + printk("0x%02x ", buf[k]); + } + + printk("\nbarfoo_receive_buf - done\n"); + + return size; +} + +static const struct serdev_device_ops key_serdev_device_ops = { + .receive_buf = key_receive_buf, + .write_wakeup = serdev_device_write_wakeup, +}; + +static int key_probe(struct serdev_device *serdev) +{ + struct key_priv *p; + int ret; + + p = devm_kzalloc(&serdev->dev, sizeof(*p), GFP_KERNEL); + if (!p) + return -ENOMEM; + + p->serdev = serdev; + dev_set_drvdata(&serdev->dev, p); + + if (of_property_read_u32(serdev->dev.of_node, "current-speed", &p->bps)) + return -EINVAL; + + serdev_device_set_client_ops(serdev, &key_serdev_device_ops); + ret = devm_serdev_device_open(&serdev->dev, serdev); + if (ret) + return ret; + + serdev_device_set_baudrate(serdev, p->bps); + serdev_device_set_flow_control(serdev, false); + return serdev_device_set_parity(serdev, SERDEV_PARITY_NONE); +} + +static void key_remove(struct serdev_device *serdev) +{ +}; + +static const struct of_device_id key_of_match[] = { + { .compatible = "serdev,key" }, + {}, +}; +MODULE_DEVICE_TABLE(of, key_of_match); + +static struct serdev_device_driver key_driver = { + .driver = { + .name = "serdev-key", + .of_match_table = of_match_ptr(key_of_match), + }, + .probe = key_probe, + .remove = key_remove, +}; +module_serdev_device_driver(key_driver); --- /dev/null +++ work/drivers/tty/serdev/foobar.c 2021-11-20 23:42:35.800592298 +0900 @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Serdev LED Device Driver Prototype + * Copyright (C) 2021 Magnus Damm + * + * Control brightness of an LED using the TX pin of an UART + * + * At this time two levels of brightness are supported: + * Brightness 1: Make LED lit by setting UART TX pin to idle state + * Brightness 0: Send dim data pattern 0x01 which keeps pin mostly low + * + * The above UART data patterns may optionally be used with an external open + * drain circuit driving both the LED and a push switch using a single pin. + */ + +#include <linux/errno.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/leds.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/serdev.h> +#include <linux/workqueue.h> + +struct led_priv { + struct serdev_device *serdev; + u32 bps; + struct led_classdev lcd; + unsigned int led_brightness; + struct delayed_work work; + unsigned char buf[128]; /* dim pattern data */ +}; + +static void led_work(struct work_struct *work) +{ + struct led_priv *p = container_of(work, struct led_priv, work.work); + unsigned int bits_queued; + int ret; + + /* wait in case all dim pattern data is not sent out yet */ + serdev_device_wait_until_sent(p->serdev, 0); + + if (p->led_brightness) { + /* uart line idle state is high so do nothing */ + return; + } + + /* output continuous dim pattern */ + ret = serdev_device_write_buf(p->serdev, p->buf, sizeof(p->buf)); + bits_queued = (ret > 0 ? ret : 1) * 10; + schedule_delayed_work(&p->work, (HZ * bits_queued) / p->bps); +} + +static void led_brightness_set(struct led_classdev *lcdp, + enum led_brightness brightness) +{ + struct led_priv *p = container_of(lcdp, struct led_priv, lcd); + + p->led_brightness = (int)brightness; + schedule_delayed_work(&p->work, 0); +} + +static const struct serdev_device_ops led_serdev_device_ops = { + .write_wakeup = serdev_device_write_wakeup, +}; + +static int led_probe(struct serdev_device *serdev) +{ + struct led_priv *p; + int ret; + + p = devm_kzalloc(&serdev->dev, sizeof(*p), GFP_KERNEL); + if (!p) + return -ENOMEM; + + p->serdev = serdev; + dev_set_drvdata(&serdev->dev, p); + + if (of_property_read_u32(serdev->dev.of_node, "current-speed", &p->bps)) + return -EINVAL; + + memset(p->buf, sizeof(p->buf), 1); /* almost zero brightness */ + INIT_DELAYED_WORK(&p->work, led_work); + p->lcd.name = dev_name(&serdev->dev); + p->lcd.max_brightness = 1; + p->lcd.brightness_set = led_brightness_set; + + ret = devm_led_classdev_register_ext(&serdev->dev, &p->lcd, NULL); + + serdev_device_set_client_ops(serdev, &led_serdev_device_ops); + ret = devm_serdev_device_open(&serdev->dev, serdev); + if (ret) + return ret; + + serdev_device_set_baudrate(serdev, p->bps); + serdev_device_set_flow_control(serdev, false); + return serdev_device_set_parity(serdev, SERDEV_PARITY_NONE); +} + +static void led_remove(struct serdev_device *serdev) +{ + struct led_priv *p = dev_get_drvdata(&serdev->dev); + + cancel_delayed_work_sync(&p->work); +}; + +static const struct of_device_id led_of_match[] = { + { .compatible = "serdev,led" }, + {}, +}; +MODULE_DEVICE_TABLE(of, led_of_match); + +static struct serdev_device_driver led_driver = { + .driver = { + .name = "serdev-led", + .of_match_table = of_match_ptr(led_of_match), + }, + .probe = led_probe, + .remove = led_remove, +}; +module_serdev_device_driver(led_driver);
From: Magnus Damm <damm+renesas@opensource.se> Here's a work-in-progress patch for shared pin LED and KEY functionality: - UART TX Serdev LED driver prototype (functional) - UART RX Serdev KEY driver prototype (partial) - r8a77995 Draak DTS modifications to use above drivers with SCIF0 With this code my hope is to use hardware to drive an LED and allow detection of a key press without software performing any kind of polling. In theory on SoCs that support UART RX and TX on the same pin (and also open drain output) with the above software it is possible to handle boards with single pin shared LED and KEY functionality. This prototype on r8a77995 Draak makes use of 3 pins and an external circuit: - LED13/SW59/GP4_07 <-> EXIO_A:10 (CN46) - SCIF0_RX/GP4_20 <- EXIO_A:38 (CN46) - SCIF0_TX/GP4_21 -> EXIO_A:36 (CN46) Ether-AVB PHY connector (CN23) has 3.3V on pin 54 and 56 and GND on 14 In the future SCIF1 and SCIF3 may also be used for other LEDs and switches. Currently two inverters on SN74HC05 together with pull-ups are used to extend the D3 SoC and the Draak board with open drain functionality and also tie together the TX and RX pins with LED13/SW59. The prototype LED driver allows user space to turn on/off the LED using: # echo 1 > /sys/class/leds/serial0-0/brightness # echo 0 > /sys/class/leds/serial0-0/brightness Must be easy to extend the driver with some degree of brightness control. Apart from some general brush up the following issues have surfaced: - "controller busy" error happens when more than one serdev is used - it is unclear how to take RX errors from serdev and generate key events - there seem to be no way to silence "sh-sci e6e60000.serial: frame error" - the DTS "current-speed" property looks like sw config and not hw description Obviously not for upstream merge as-is. Might however be useful as SCIF error test bench and/or as potential (corner) use case for serdev. Not-Yet-Signed-off-by: Magnus Damm <damm+renesas@opensource.se> --- arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 34 ++++++ drivers/tty/serdev/Makefile | 2 drivers/tty/serdev/barfoo.c | 99 +++++++++++++++++++ drivers/tty/serdev/foobar.c | 121 ++++++++++++++++++++++++ 4 files changed, 254 insertions(+), 2 deletions(-)