Message-ID: <be7c90cf-4c65-1cf0-3001-8706415c3d34@kernel.org>
Date:   Mon, 6 Mar 2023 07:57:28 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v7 2/2] trigger: ledtrig-tty: add additional modes
Content-Language: en-US
To:     Lee Jones <lee@kernel.org>, Florian Eckert <fe@dev.tdt.de>
Cc:     u.kleine-koenig@pengutronix.de, gregkh@linuxfoundation.org,
        pavel@ucw.cz, linux-kernel@vger.kernel.org,
        linux-leds@vger.kernel.org, Eckert.Florian@googlemail.com
References: <20230222083335.847655-1-fe@dev.tdt.de>
 <20230222083335.847655-3-fe@dev.tdt.de> <20230303141139.GP2420672@google.com>
From:   Jiri Slaby <jirislaby@kernel.org>
In-Reply-To: <20230303141139.GP2420672@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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?

> Documentation/process/deprecated.rst
> 
>> +	default:
>> +		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;
>> +		}
>> +


-- 
js