Re: [PATCH 14/24] serial: 8250: ioc3: store registered line number in driver data

[Jiri Slaby <jirislaby@kernel.org>]

On 27. 10. 26, 12:11, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] serial: 8250: ioc3: undeclared identifiers in serial8250_ioc3_probe()
> --
> 
> commit 42c7c91e0766573099398067e430ef38776baba0
> Author: Joshua Rogers <linux@joshua.hu>
> 
> serial: 8250: ioc3: store registered line number in driver data
> 
> This commit attempts to save the registered line number into data->line in
> serial8250_ioc3_probe() so the remove path unregisters the proper port.
> 
>> diff --git a/drivers/tty/serial/8250/8250_ioc3.c b/drivers/tty/serial/8250/8250_ioc3.c
>> index 088242de2eff9..3ab50e04be451 100644
>> --- a/drivers/tty/serial/8250/8250_ioc3.c
>> +++ b/drivers/tty/serial/8250/8250_ioc3.c
>> @@ -63,6 +63,7 @@ static int serial8250_ioc3_probe(struct platform_device *pdev)
>>   	if (IS_ERR(uport))
>>   		return PTR_ERR(uport);
>>   
>> +	data->line = line;
> 
> [Severity: Low]
> Does this break compilation when CONFIG_SERIAL_8250_IOC3 is enabled because
> neither data nor line is declared in serial8250_ioc3_probe()?
> 
> Prior commit d6a0791098bede ("serial: 8250: make port operations work on
> struct uart_8250_port") refactored drivers/tty/serial/8250/8250_ioc3.c to
> remove struct ioc3_8250_data, data, and line:
> 
> drivers/tty/serial/8250/8250_ioc3.c:serial8250_ioc3_probe() {
> 	struct uart_8250_port *uport, up;
> 	struct resource *r;
> 	void __iomem *membase;
> 	int irq;
> 	...
> 	uport = serial8250_register_8250_port(&up);
> 	if (IS_ERR(uport))
> 		return PTR_ERR(uport);
> 
> 	platform_set_drvdata(pdev, uport);
> 	return 0;
> }
> 
> and updated serial8250_ioc3_remove() to pass uport directly to
> serial8250_unregister_port():
> 
> drivers/tty/serial/8250/8250_ioc3.c:serial8250_ioc3_remove() {
> 	struct uart_8250_port *uport = platform_get_drvdata(pdev);
> 
> 	serial8250_unregister_port(uport);
> }
> 
> Since the driver now stores and unregisters uport directly rather than
> tracking a line number in data->line, is this change obsolete on top of
> commit d6a0791098bede?

It is. And I did not notice this bug prior to d6a0791098bede, so there 
is no "preparatory" fix before that commit. Therefore, this might be a 
fix for older (stable) kernels, actually.

thanks,
-- 
js
suse labs