Re: [PATCH 16/17] auxdisplay: ht16k33: Add support for segment displays

On 2021-03-29 09:15, Geert Uytterhoeven wrote:

Hoi Robin,

On Mon, Mar 29, 2021 at 9:09 AM Robin van der Gracht 
[off-list ref] wrote:

quoted

On 2021-03-22 15:48, Geert Uytterhoeven wrote:

quoted

The Holtek HT16K33 LED controller is not only used for driving
dot-matrix displays, but also for driving segment displays.

Add support for 4-digit 7-segment and quad 14-segment alphanumeric
displays, like the Adafruit 7-segment and 14-segment display backpack
and FeatherWing expansion boards.  Use the character line display core
support to display a message, which will be scrolled if it doesn't fit.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
The 7-segment support is based on schematics, and untested on actual
hardware.
---
 drivers/auxdisplay/ht16k33.c | 198 +++++++++++++++++++++++++++++++++--
 1 file changed, 191 insertions(+), 7 deletions(-)

...

quoted

+static int ht16k33_seg_probe(struct i2c_client *client,
+                          struct ht16k33_priv *priv, uint32_t brightness)
+{
+     struct ht16k33_seg *seg = &priv->seg;
+     struct device *dev = &client->dev;
+     int err;
+
+     err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS);
+     if (err)
+             return err;
+
+     switch (priv->type) {
+     case DISP_MATRIX:
+             /* not handled here */
+             break;

This 'case' shouldn't happen. Having said that, the break here will
still
cause the linedisp_register() function to be called for the 
DISP_MATRIX
type.
If you'd like to handle this case, a return (or setting 'err') should
prevent this.

This function is never called if priv->type == DISP_MATRIX, so this
cannot happen.  However, gcc complains if not all enum values are
handled in a switch() statement, hence the dummy case.

I see. But if it's there it should work right? :P

Is there a better way to handle this?

How about adding the default case:

default:
     /* not handled */
     err = -EINVAL;

Groetjes/Kind regards,
Robin van der Gracht

-- 
Protonic Holland
Factorij 36
1689AL Zwaag
+31 (0)229 212928
https://www.protonic.nl