| Message ID | 20210914143835.511051-20-geert@linux-m68k.org |
|---|---|
| State | New |
| Headers | show |
| Series | auxdisplay: ht16k33: Add character display support | expand |
Hi Geert, On 2021-09-14 16:38, 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:<color>:<function>". > > 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 <geert@linux-m68k.org> > Reviewed-by: Marek Behún <kabel@kernel.org> > --- > v6: > - Add Reviewed-by, > - Reorder operations in ht16k33_led_probe() to ease 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 | 124 ++++++++++++++++++++++++++++++----- > 2 files changed, 109 insertions(+), 17 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 3b555e119e326cec..89ee5b4b3dfccb68 100644 > --- a/drivers/auxdisplay/ht16k33.c > +++ b/drivers/auxdisplay/ht16k33.c > @@ -18,6 +18,7 @@ > #include <linux/backlight.h> > #include <linux/input.h> > #include <linux/input/matrix_keypad.h> > +#include <linux/leds.h> > #include <linux/workqueue.h> > #include <linux/mm.h> > > @@ -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; > @@ -425,6 +477,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; What do you think about adding a default trigger and making it 'backlight'? led->default_trigger = "blacklight"; Or as an alternative, suggesting linux,default-trigger = "backlight" in the docs? Since the led class won't respond to blank events by just making it's function LED_FUNCTION_BACKLIGHT. led { function = LED_FUNCTION_BACKLIGHT; color = <LED_COLOR_ID_GREEN>; linux,default-trigger = "backlight"; }; I noticed blanking is broken. The backlight device (or LED device with backlight trigger) doens't get notified when the framebuffer is blanked since the driver doesn't implement fb_blank. Right now: echo 1 > /sys/class/graphics/fb0/blank | sh: write error: Invalid argument Due to: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/video/fbdev/core/fbmem.c?h=v5.15-rc3#n1078 Something like this fixes it. diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c index 89ee5b4b3dfc..0883d5252c81 100644 --- a/drivers/auxdisplay/ht16k33.c +++ b/drivers/auxdisplay/ht16k33.c @@ -346,6 +346,15 @@ static int ht16k33_mmap(struct fb_info *info, struct vm_area_struct *vma) return vm_map_pages_zero(vma, &pages, 1); } +/* + * Blank events will be passed to the backlight device (or the LED device if + * it's trigger is 'backlight') when we return 0 here. + */ +static int ht16k33_blank(int blank, struct fb_info *info) +{ + return 0; +} + static const struct fb_ops ht16k33_fb_ops = { .owner = THIS_MODULE, .fb_read = fb_sys_read, @@ -354,6 +363,7 @@ static const struct fb_ops ht16k33_fb_ops = { .fb_copyarea = sys_copyarea, .fb_imageblit = sys_imageblit, .fb_mmap = ht16k33_mmap, + .fb_blank = ht16k33_blank, }; /* Feel free to include (something like) this in the patch stack. > + > + err = devm_led_classdev_register_ext(dev, led, &init_data); > + if (err) > + dev_err(dev, "Failed to register LED\n"); You might want to call ht16k33_brightness_set(priv, brightness) here to get a know value into the display setup register (0x80). Right now if I enable hardware blinking and (soft)reboot my board it keeps on blinking even after a re-probe. > + > + return err; > +} > + > static int ht16k33_keypad_probe(struct i2c_client *client, > struct ht16k33_keypad *keypad) > { > @@ -498,24 +579,28 @@ 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) { > + /* backwards compatibility with DT lacking an led subnode */ > + struct backlight_properties bl_props; > > - 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); > - } > + 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->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); > @@ -575,7 +660,7 @@ static int ht16k33_seg_probe(struct device *dev, struct > ht16k33_priv *priv, > struct ht16k33_seg *seg = &priv->seg; > int err; > > - err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS); > + err = ht16k33_brightness_set(priv, brightness); This looks like a bugfix for patch 17, maybe move this change there? > if (err) > return err; > > @@ -653,6 +738,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); Gr{oetje,eeting}s, Robin
Hoi Robin, On Thu, Sep 30, 2021 at 12:57 PM Robin van der Gracht <robin@protonic.nl> wrote: > On 2021-09-14 16:38, 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:<color>:<function>". > > > > 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 <geert@linux-m68k.org> > > Reviewed-by: Marek Behún <kabel@kernel.org> > > --- > > v6: > > - Add Reviewed-by, > > - Reorder operations in ht16k33_led_probe() to ease future conversion > > to device properties, > > --- a/drivers/auxdisplay/ht16k33.c > > +++ b/drivers/auxdisplay/ht16k33.c > > @@ -425,6 +477,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; > > What do you think about adding a default trigger and making it 'backlight'? > > led->default_trigger = "blacklight"; > > Or as an alternative, suggesting linux,default-trigger = "backlight" in the > docs? Since the led class won't respond to blank events by just making it's > function LED_FUNCTION_BACKLIGHT. > > led { > function = LED_FUNCTION_BACKLIGHT; > color = <LED_COLOR_ID_GREEN>; > linux,default-trigger = "backlight"; > }; The latter makes perfect sense to me. Will do. > I noticed blanking is broken. The backlight device (or LED device with > backlight trigger) doens't get notified when the framebuffer is blanked since > the driver doesn't implement fb_blank. > > Right now: > > echo 1 > /sys/class/graphics/fb0/blank > | > sh: write error: Invalid argument > > Due to: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/video/fbdev/core/fbmem.c?h=v5.15-rc3#n1078 That's a pre-existing problem, righ? ;-) > Something like this fixes it. > > diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c > index 89ee5b4b3dfc..0883d5252c81 100644 > --- a/drivers/auxdisplay/ht16k33.c > +++ b/drivers/auxdisplay/ht16k33.c > @@ -346,6 +346,15 @@ static int ht16k33_mmap(struct fb_info *info, struct > vm_area_struct *vma) > return vm_map_pages_zero(vma, &pages, 1); > } > > +/* > + * Blank events will be passed to the backlight device (or the LED device if > + * it's trigger is 'backlight') when we return 0 here. > + */ > +static int ht16k33_blank(int blank, struct fb_info *info) > +{ > + return 0; > +} > + > static const struct fb_ops ht16k33_fb_ops = { > .owner = THIS_MODULE, > .fb_read = fb_sys_read, > @@ -354,6 +363,7 @@ static const struct fb_ops ht16k33_fb_ops = { > .fb_copyarea = sys_copyarea, > .fb_imageblit = sys_imageblit, > .fb_mmap = ht16k33_mmap, > + .fb_blank = ht16k33_blank, > }; > > /* > > Feel free to include (something like) this in the patch stack. Thanks, will do. > > + > > + err = devm_led_classdev_register_ext(dev, led, &init_data); > > + if (err) > > + dev_err(dev, "Failed to register LED\n"); > > You might want to call ht16k33_brightness_set(priv, brightness) here to get a > know value into the display setup register (0x80). > > Right now if I enable hardware blinking and (soft)reboot my board it keeps on > blinking even after a re-probe. I don't have that issue. Aha, ht16k33_seg_probe() calls ht16k33_brightness_set(), but ht16k33_fbdev_probe() doesn't. The latter should do that, too, when not using backwards compatibility mode. > > @@ -575,7 +660,7 @@ static int ht16k33_seg_probe(struct device *dev, struct > > ht16k33_priv *priv, > > struct ht16k33_seg *seg = &priv->seg; > > int err; > > > > - err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS); > > + err = ht16k33_brightness_set(priv, brightness); > > This looks like a bugfix for patch 17, maybe move this change there? Indeed. Bad rebase. Will move. Thanks a lot for your comments! 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 Geert, On 2021-10-01 17:51, Geert Uytterhoeven wrote: > Hoi Robin, > > On Thu, Sep 30, 2021 at 12:57 PM Robin van der Gracht <robin@protonic.nl> > wrote: >> On 2021-09-14 16:38, 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:<color>:<function>". >> > >> > 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 <geert@linux-m68k.org> >> > Reviewed-by: Marek Behún <kabel@kernel.org> >> > --- >> > v6: >> > - Add Reviewed-by, >> > - Reorder operations in ht16k33_led_probe() to ease future conversion >> > to device properties, > >> > --- a/drivers/auxdisplay/ht16k33.c >> > +++ b/drivers/auxdisplay/ht16k33.c >> > @@ -425,6 +477,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; >> >> What do you think about adding a default trigger and making it 'backlight'? >> >> led->default_trigger = "blacklight"; >> >> Or as an alternative, suggesting linux,default-trigger = "backlight" in the >> docs? Since the led class won't respond to blank events by just making it's >> function LED_FUNCTION_BACKLIGHT. >> >> led { >> function = LED_FUNCTION_BACKLIGHT; >> color = <LED_COLOR_ID_GREEN>; >> linux,default-trigger = "backlight"; >> }; > > The latter makes perfect sense to me. Will do. Ack. > >> I noticed blanking is broken. The backlight device (or LED device with >> backlight trigger) doens't get notified when the framebuffer is blanked >> since >> the driver doesn't implement fb_blank. >> >> Right now: >> >> echo 1 > /sys/class/graphics/fb0/blank >> | >> sh: write error: Invalid argument >> >> Due to: >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/video/fbdev/core/fbmem.c?h=v5.15-rc3#n1078 > > That's a pre-existing problem, righ? ;-) Yes it is. The fix is easy and since we're making major driver changes now... I needed the fix to properly test the changes we're making, so I thought we might as well include it. > >> Something like this fixes it. >> >> diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c >> index 89ee5b4b3dfc..0883d5252c81 100644 >> --- a/drivers/auxdisplay/ht16k33.c >> +++ b/drivers/auxdisplay/ht16k33.c >> @@ -346,6 +346,15 @@ static int ht16k33_mmap(struct fb_info *info, struct >> vm_area_struct *vma) >> return vm_map_pages_zero(vma, &pages, 1); >> } >> >> +/* >> + * Blank events will be passed to the backlight device (or the LED device >> if >> + * it's trigger is 'backlight') when we return 0 here. >> + */ >> +static int ht16k33_blank(int blank, struct fb_info *info) >> +{ >> + return 0; >> +} >> + >> static const struct fb_ops ht16k33_fb_ops = { >> .owner = THIS_MODULE, >> .fb_read = fb_sys_read, >> @@ -354,6 +363,7 @@ static const struct fb_ops ht16k33_fb_ops = { >> .fb_copyarea = sys_copyarea, >> .fb_imageblit = sys_imageblit, >> .fb_mmap = ht16k33_mmap, >> + .fb_blank = ht16k33_blank, >> }; >> >> /* >> >> Feel free to include (something like) this in the patch stack. > > Thanks, will do. Ack. > >> > + >> > + err = devm_led_classdev_register_ext(dev, led, &init_data); >> > + if (err) >> > + dev_err(dev, "Failed to register LED\n"); >> >> You might want to call ht16k33_brightness_set(priv, brightness) here to get >> a >> know value into the display setup register (0x80). >> >> Right now if I enable hardware blinking and (soft)reboot my board it keeps >> on >> blinking even after a re-probe. > > I don't have that issue. > Aha, ht16k33_seg_probe() calls ht16k33_brightness_set(), but > ht16k33_fbdev_probe() doesn't. The latter should do that, too, > when not using backwards compatibility mode. Ack. I have hardware which uses the ht16k33 in dot matrix mode and I tested both the backlight and led setup. I ran into this with the fbdev + led setup. I noticed ht16k33_bl_update_status() is called in ht16k33_fbdev_probe() before the fbdev device is registered. Which is fine right now, but in theory the fbdev blank state can influence the backlight setting (nitpick since the fbdev device is unblanked by default). The point: Maybe ht16k33_brightness_set() (or ht16k33_bl_update_status() for backlight device) should be called in one central place (i.e at the end of the main probe function). > >> > @@ -575,7 +660,7 @@ static int ht16k33_seg_probe(struct device *dev, struct >> > ht16k33_priv *priv, >> > struct ht16k33_seg *seg = &priv->seg; >> > int err; >> > >> > - err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS); >> > + err = ht16k33_brightness_set(priv, brightness); >> >> This looks like a bugfix for patch 17, maybe move this change there? > > Indeed. Bad rebase. Will move. > > Thanks a lot for your comments! :) - Robin
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 3b555e119e326cec..89ee5b4b3dfccb68 100644 --- a/drivers/auxdisplay/ht16k33.c +++ b/drivers/auxdisplay/ht16k33.c @@ -18,6 +18,7 @@ #include <linux/backlight.h> #include <linux/input.h> #include <linux/input/matrix_keypad.h> +#include <linux/leds.h> #include <linux/workqueue.h> #include <linux/mm.h> @@ -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; @@ -425,6 +477,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) { @@ -498,24 +579,28 @@ 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) { + /* backwards compatibility with DT lacking an led subnode */ + struct backlight_properties bl_props; - 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); - } + 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->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); @@ -575,7 +660,7 @@ static int ht16k33_seg_probe(struct device *dev, struct ht16k33_priv *priv, struct ht16k33_seg *seg = &priv->seg; int err; - err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS); + err = ht16k33_brightness_set(priv, brightness); if (err) return err; @@ -653,6 +738,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);