Received: by 2002:a05:6a10:5bc5:0:0:0:0 with SMTP id os5csp3192706pxb; Wed, 13 Oct 2021 00:29:17 -0700 (PDT) X-Google-Smtp-Source: ABdhPJy+5DnvlhY8rdS0dhwq4O31Av0/olb9FwS+WXtbrtdLfNXloDdtJvHZefQU4Uoe7J21GT5/ X-Received: by 2002:a63:b707:: with SMTP id t7mr26253201pgf.55.1634110157154; Wed, 13 Oct 2021 00:29:17 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1634110157; cv=none; d=google.com; s=arc-20160816; b=HnkiofS8CflgbmS+bZwtZemjzBaZIrr9ShKvLe4Gh72wSg5gE6aZVENjWw3zfRT4Sa ImXDdLZPTJcmnpnuwAn04PQNsn0E7kBOktbV+UhvpisrFMrQi/ltahQW8uED67SGBxNh fC6C/vkWmW1XxTABEHn1fcpbzQuiinXgGNeRukt7zWURHXEoNJR0hxl7popIGgEugQdX xhGQZ/OoUR9cegxJ9+i9Rkrz9UvMWX3k4eGI2kcMYQ8LoU8o5JPDmR6oobiVpFHzqLuL JvCOTHP6ELjM1z81Ku5y2h9WK4WJmwrMgJUpTrTnBv3RWTFofnmm+TDVK7qV8i1Elh5I j7JQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:organization :message-id:user-agent:references:in-reply-to:reply-to:subject:cc:to :from:date:mime-version; bh=u3mtdQ5ydcLGH/ipnGCU3VSs1yblXeqB5wXj/u3eK0U=; b=NJHyAfguqowWdhhZSmUHBU53tDS283PZBmf3nv8hPmv1PRSNfLF5CSR7GbQNHJNISb l7HZROFTsn8JPtJ/a1cyhletFXQrSR3HUBUB3zUKiLbtZln0uP5uI+3z8SsLGV+wDB9O IM2yripsCrLXUUo4vLjqia3Lo9JixB25V5ZtWJRqo16bK63GW41oZyeuh01ca1BO89pV uSko+ZOMJOYBDT6+DtTzBFFVW5WTYmGtv2Ssz9ZZ80nUQj+HLeNir9WIvT8KFPt9zKTx oE8iFdIqzmuuWDOTLRCNDbpuSwd2nWsj204EejHCZ43vmVgwcsdHgJwtK4JftpIocZ+L vedQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id c5si18817522plo.158.2021.10.13.00.29.04; Wed, 13 Oct 2021 00:29:17 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232357AbhJMH3y (ORCPT + 99 others); Wed, 13 Oct 2021 03:29:54 -0400 Received: from protonic.xs4all.nl ([83.163.252.89]:56022 "EHLO protonic.xs4all.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229664AbhJMH3x (ORCPT ); Wed, 13 Oct 2021 03:29:53 -0400 Received: from fiber.protonic.nl (edge2.prtnl [192.168.1.170]) by sparta.prtnl (Postfix) with ESMTP id 1A7EB44A024F; Wed, 13 Oct 2021 09:27:36 +0200 (CEST) MIME-Version: 1.0 Date: Wed, 13 Oct 2021 09:27:36 +0200 From: Robin van der Gracht To: Geert Uytterhoeven Cc: Miguel Ojeda , Rob Herring , Paul Burton , Greg Kroah-Hartman , Pavel Machek , Marek Behun , devicetree@vger.kernel.org, linux-leds@vger.kernel.org, linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org, =?UTF-8?Q?Marek_Beh=C3=BAn?= Subject: Re: [PATCH v7 20/21] auxdisplay: ht16k33: Add LED support Reply-To: robin@protonic.nl In-Reply-To: <20211012183327.649865-21-geert@linux-m68k.org> References: <20211012183327.649865-1-geert@linux-m68k.org> <20211012183327.649865-21-geert@linux-m68k.org> User-Agent: Roundcube Webmail/1.4.11 Message-ID: <799e3b17f2981371529b06631ccefbe8@protonic.nl> X-Sender: robin@protonic.nl Organization: Protonic Holland Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Reviewed-by: Robin van der Gracht On 2021-10-12 20:33, Geert Uytterhoeven wrote: > Instantiate a single LED based on the "led" subnode in DT. > This allows the user to control display brightness and blinking (backed > by hardware support) through the LED class API and triggers, and exposes > the display color. The LED will be named > "auxdisplay::". > > When running in dot-matrix mode and if no "led" subnode is found, the > driver falls back to the traditional backlight mode, to preserve > backwards compatibility. > > Signed-off-by: Geert Uytterhoeven > Reviewed-by: Marek BehĂșn > --- > v7: > - Add missing call to ht16k33_brightness_set() in > ht16k33_fbdev_probe(), to make sure brightness and blinking are set > to a sane state, > > v6: > - Add Reviewed-by, > - Reorder operations in ht16k33_led_probe() to simplify future > conversion to device properties, > > v5: > - Add missing select NEW_LEDS, > > v4: > - Add missing select LEDS_CLASS, > > v3: > - Remove unneeded C++ comment, > - Use "err" instead of "error" to be consistent with existing driver > naming style, > - Make the creation of the LED device dependent on the presence of the > "led" subnode in DT, so it can be used in dot-matrix mode too. > - Use led_init_data() and devm_led_classdev_register_ext() to retrieve > all LED properties from DT, instead of manual LED name construction > based on just the "color" property, > > v2: > - Use "auxdisplay" instead of DRIVER_NAME in LED name. > --- > drivers/auxdisplay/Kconfig | 2 + > drivers/auxdisplay/ht16k33.c | 126 ++++++++++++++++++++++++++++++----- > 2 files changed, 112 insertions(+), 16 deletions(-) > > diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig > index 42fc7b155de09dbc..e32ef7f9945d49b2 100644 > --- a/drivers/auxdisplay/Kconfig > +++ b/drivers/auxdisplay/Kconfig > @@ -176,6 +176,8 @@ config HT16K33 > select FB_SYS_IMAGEBLIT > select INPUT_MATRIXKMAP > select FB_BACKLIGHT > + select NEW_LEDS > + select LEDS_CLASS > select LINEDISP > help > Say yes here to add support for Holtek HT16K33, RAM mapping 16*8 > diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c > index 9ce9271d5e3d0b31..aef3dc87dc9f5ed2 100644 > --- a/drivers/auxdisplay/ht16k33.c > +++ b/drivers/auxdisplay/ht16k33.c > @@ -18,6 +18,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -34,6 +35,10 @@ > > #define REG_DISPLAY_SETUP 0x80 > #define REG_DISPLAY_SETUP_ON BIT(0) > +#define REG_DISPLAY_SETUP_BLINK_OFF (0 << 1) > +#define REG_DISPLAY_SETUP_BLINK_2HZ (1 << 1) > +#define REG_DISPLAY_SETUP_BLINK_1HZ (2 << 1) > +#define REG_DISPLAY_SETUP_BLINK_0HZ5 (3 << 1) > > #define REG_ROWINT_SET 0xA0 > #define REG_ROWINT_SET_INT_EN BIT(0) > @@ -94,12 +99,14 @@ struct ht16k33_seg { > struct ht16k33_priv { > struct i2c_client *client; > struct delayed_work work; > + struct led_classdev led; > struct ht16k33_keypad keypad; > union { > struct ht16k33_fbdev fbdev; > struct ht16k33_seg seg; > }; > enum display_type type; > + uint8_t blink; > }; > > static const struct fb_fix_screeninfo ht16k33_fb_fix = { > @@ -158,7 +165,7 @@ static DEVICE_ATTR(map_seg14, 0644, map_seg_show, > map_seg_store); > > static int ht16k33_display_on(struct ht16k33_priv *priv) > { > - uint8_t data = REG_DISPLAY_SETUP | REG_DISPLAY_SETUP_ON; > + uint8_t data = REG_DISPLAY_SETUP | REG_DISPLAY_SETUP_ON | priv->blink; > > return i2c_smbus_write_byte(priv->client, data); > } > @@ -173,8 +180,10 @@ static int ht16k33_brightness_set(struct ht16k33_priv > *priv, > { > int err; > > - if (brightness == 0) > + if (brightness == 0) { > + priv->blink = REG_DISPLAY_SETUP_BLINK_OFF; > return ht16k33_display_off(priv); > + } > > err = ht16k33_display_on(priv); > if (err) > @@ -184,6 +193,49 @@ static int ht16k33_brightness_set(struct ht16k33_priv > *priv, > REG_BRIGHTNESS | (brightness - 1)); > } > > +static int ht16k33_brightness_set_blocking(struct led_classdev *led_cdev, > + enum led_brightness brightness) > +{ > + struct ht16k33_priv *priv = container_of(led_cdev, struct ht16k33_priv, > + led); > + > + return ht16k33_brightness_set(priv, brightness); > +} > + > +static int ht16k33_blink_set(struct led_classdev *led_cdev, > + unsigned long *delay_on, unsigned long *delay_off) > +{ > + struct ht16k33_priv *priv = container_of(led_cdev, struct ht16k33_priv, > + led); > + unsigned int delay; > + uint8_t blink; > + int err; > + > + if (!*delay_on && !*delay_off) { > + blink = REG_DISPLAY_SETUP_BLINK_1HZ; > + delay = 1000; > + } else if (*delay_on <= 750) { > + blink = REG_DISPLAY_SETUP_BLINK_2HZ; > + delay = 500; > + } else if (*delay_on <= 1500) { > + blink = REG_DISPLAY_SETUP_BLINK_1HZ; > + delay = 1000; > + } else { > + blink = REG_DISPLAY_SETUP_BLINK_0HZ5; > + delay = 2000; > + } > + > + err = i2c_smbus_write_byte(priv->client, > + REG_DISPLAY_SETUP | REG_DISPLAY_SETUP_ON | > + blink); > + if (err) > + return err; > + > + priv->blink = blink; > + *delay_on = *delay_off = delay; > + return 0; > +} > + > static void ht16k33_fb_queue(struct ht16k33_priv *priv) > { > struct ht16k33_fbdev *fbdev = &priv->fbdev; > @@ -435,6 +487,35 @@ static void ht16k33_seg14_update(struct work_struct > *work) > i2c_smbus_write_i2c_block_data(priv->client, 0, ARRAY_SIZE(buf), buf); > } > > +static int ht16k33_led_probe(struct device *dev, struct led_classdev *led, > + unsigned int brightness) > +{ > + struct led_init_data init_data = {}; > + struct device_node *node; > + int err; > + > + /* The LED is optional */ > + node = of_get_child_by_name(dev->of_node, "led"); > + if (!node) > + return 0; > + > + init_data.fwnode = of_fwnode_handle(node); > + init_data.devicename = "auxdisplay"; > + init_data.devname_mandatory = true; > + > + led->brightness_set_blocking = ht16k33_brightness_set_blocking; > + led->blink_set = ht16k33_blink_set; > + led->flags = LED_CORE_SUSPENDRESUME; > + led->brightness = brightness; > + led->max_brightness = MAX_BRIGHTNESS; > + > + err = devm_led_classdev_register_ext(dev, led, &init_data); > + if (err) > + dev_err(dev, "Failed to register LED\n"); > + > + return err; > +} > + > static int ht16k33_keypad_probe(struct i2c_client *client, > struct ht16k33_keypad *keypad) > { > @@ -508,25 +589,33 @@ static int ht16k33_fbdev_probe(struct device *dev, > struct ht16k33_priv *priv, > uint32_t brightness) > { > struct ht16k33_fbdev *fbdev = &priv->fbdev; > - struct backlight_properties bl_props; > - struct backlight_device *bl; > + struct backlight_device *bl = NULL; > int err; > > - /* Backlight */ > - memset(&bl_props, 0, sizeof(struct backlight_properties)); > - bl_props.type = BACKLIGHT_RAW; > - bl_props.max_brightness = MAX_BRIGHTNESS; > + if (priv->led.dev) { > + err = ht16k33_brightness_set(priv, brightness); > + if (err) > + return err; > + } else { > + /* backwards compatibility with DT lacking an led subnode */ > + struct backlight_properties bl_props; > + > + memset(&bl_props, 0, sizeof(struct backlight_properties)); > + bl_props.type = BACKLIGHT_RAW; > + bl_props.max_brightness = MAX_BRIGHTNESS; > + > + bl = devm_backlight_device_register(dev, DRIVER_NAME"-bl", dev, > + priv, &ht16k33_bl_ops, > + &bl_props); > + if (IS_ERR(bl)) { > + dev_err(dev, "failed to register backlight\n"); > + return PTR_ERR(bl); > + } > > - bl = devm_backlight_device_register(dev, DRIVER_NAME"-bl", dev, priv, > - &ht16k33_bl_ops, &bl_props); > - if (IS_ERR(bl)) { > - dev_err(dev, "failed to register backlight\n"); > - return PTR_ERR(bl); > + bl->props.brightness = brightness; > + ht16k33_bl_update_status(bl); > } > > - bl->props.brightness = brightness; > - ht16k33_bl_update_status(bl); > - > /* Framebuffer (2 bytes per column) */ > BUILD_BUG_ON(PAGE_SIZE < HT16K33_FB_SIZE); > fbdev->buffer = (unsigned char *) get_zeroed_page(GFP_KERNEL); > @@ -663,6 +752,11 @@ static int ht16k33_probe(struct i2c_client *client) > dft_brightness = MAX_BRIGHTNESS; > } > > + /* LED */ > + err = ht16k33_led_probe(dev, &priv->led, dft_brightness); > + if (err) > + return err; > + > /* Keypad */ > if (client->irq > 0) { > err = ht16k33_keypad_probe(client, &priv->keypad); Met vriendelijke groet, Robin van der Gracht -- Protonic Holland Factorij 36 1689AL Zwaag +31 (0)229 212928 https://www.protonic.nl