linux-kernel - Re: [PATCH v2 1/3] serial: 8250: RT288x/Au1xxx code away from core

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 6 Jan 2023 08:36:39 +0100
From:   Jiri Slaby <jirislaby@...nel.org>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        linux-serial@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Philippe Mathieu-Daudé <philmd@...aro.org>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] serial: 8250: RT288x/Au1xxx code away from core

On 05. 01. 23, 13:47, Ilpo Järvinen wrote:
> A non-trivial amount of RT288x/Au1xxx code is encapsulated into
> ifdeffery in 8250_port / 8250_early and some if UPIO_AU blocks.
> 
> Create a separate file from them. Add mapsize, bugs, and divisor latch
> read/write functions into plat_serial8250_port to carry the setup
> necessary for these devices over to uart port.
> 
> Also handle errors properly in the cases where RT288x/Au1xxx code is
> not configured.
> 
> It seems that 0x1000 mapsize is likely overkill but I've kept it the
> same as previously (the value was shrunk to that value in b2b13cdfd05e
> ("SERIAL 8250: Fixes for Alchemy UARTs.")). Seemingly, the driver only
> needs to access register at 0x28 for the divisor latch.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
...
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
...
> @@ -28,8 +32,11 @@ struct plat_serial8250_port {
>   	unsigned char	has_sysrq;	/* supports magic SysRq */
>   	upf_t		flags;		/* UPF_* flags */
>   	unsigned int	type;		/* If UPF_FIXED_TYPE */
> +	unsigned short	bugs;		/* port bugs */
>   	unsigned int	(*serial_in)(struct uart_port *, int);
>   	void		(*serial_out)(struct uart_port *, int, int);
> +	int		(*dl_read)(struct uart_8250_port *);
> +	void		(*dl_write)(struct uart_8250_port *, int);

int sounds very weird in the write-to-HW context. Even though both the 
others and the original are ints. They all should be simply u32.

>   	void		(*set_termios)(struct uart_port *,
>   			               struct ktermios *new,
>   			               const struct ktermios *old);

Could you document the new ones in the kernel-doc manner above the 
structure (but so far without the kernel-doc tag /**). So that:
1) they are documented
2) once someone decides to document them all, they can simply stick the 
kernel-doc tag and document only the rest.

thanks,
-- 
js
suse labs