[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230306092332.rrwg45b4bhda4ssw@pengutronix.de>
Date: Mon, 6 Mar 2023 10:23:32 +0100
From: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To: Lee Jones <lee@...nel.org>
Cc: Jiri Slaby <jirislaby@...nel.org>, Florian Eckert <fe@....tdt.de>,
gregkh@...uxfoundation.org, pavel@....cz,
linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org,
Eckert.Florian@...glemail.com
Subject: Re: [PATCH v7 2/2] trigger: ledtrig-tty: add additional modes
On Mon, Mar 06, 2023 at 09:04:56AM +0000, Lee Jones wrote:
> On Mon, 06 Mar 2023, Jiri Slaby wrote:
>
> > On 03. 03. 23, 15:11, Lee Jones wrote:
> > > On Wed, 22 Feb 2023, Florian Eckert wrote:
> > > > @@ -113,21 +207,38 @@ static void ledtrig_tty_work(struct work_struct *work)
> > > > trigger_data->tty = tty;
> > > > }
> > > > - ret = tty_get_icount(trigger_data->tty, &icount);
> > > > - if (ret) {
> > > > - dev_info(trigger_data->tty->dev, "Failed to get icount, stopped polling\n");
> > > > - mutex_unlock(&trigger_data->mutex);
> > > > - return;
> > > > - }
> > > > -
> > > > - if (icount.rx != trigger_data->rx ||
> > > > - icount.tx != trigger_data->tx) {
> > > > - led_set_brightness_sync(trigger_data->led_cdev, LED_ON);
> > > > -
> > > > - trigger_data->rx = icount.rx;
> > > > - trigger_data->tx = icount.tx;
> > > > - } else {
> > > > - led_set_brightness_sync(trigger_data->led_cdev, LED_OFF);
> > > > + switch (trigger_data->mode) {
> > > > + case TTY_LED_CTS:
> > > > + ledtrig_tty_flags(trigger_data, TIOCM_CTS);
> > > > + break;
> > > > + case TTY_LED_DSR:
> > > > + ledtrig_tty_flags(trigger_data, TIOCM_DSR);
> > > > + break;
> > > > + case TTY_LED_CAR:
> > > > + ledtrig_tty_flags(trigger_data, TIOCM_CAR);
> > > > + break;
> > > > + case TTY_LED_RNG:
> > > > + ledtrig_tty_flags(trigger_data, TIOCM_RNG);
> > > > + break;
> > > > + case TTY_LED_CNT:
> > >
> > > I believe this requires a 'fall-through' statement.
> >
> > I don't think this is the case. Isn't fallthrough required only in cases
> > when there is at least one statement, i.e. a block?
>
> There's no mention of this caveat in the document.
>
> To my untrained eyes, the rule looks fairly explicit, starting with "All".
>
> "
> All switch/case blocks must end in one of:
>
> * break;
> * fallthrough;
> * continue;
> * goto <label>;
> * return [expression];
> "
>
> If you're aware of something I'm not, please consider updating the doc.
Just to add my 0.02€: I think this case (i.e.
case TTY_LED_CNT:
default:
...
) doesn't need a fall-through for two reasons:
a) The compilers don't warn about this construct even with the
fall-through warning enabled; and
b) I wouldn't call the TTY_LED_CNT line a "block", so the quoted
document[1] doesn't apply. (Though I agree that there is some
potential for improvement by mentioning this case. (haha))
Best regards
Uwe
[1] Documentation/process/deprecated.rst
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists