Hi Magnus, Quoting Magnus Damm (2021-11-20 15:41:46) > From: Magnus Damm <damm+renesas@xxxxxxxxxxxxx> > > 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@xxxxxxxxxxxxx> > --- > > 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);