diff mbox series

serial: sh-sci: Use plain struct copy in early_console_setup()

Message ID e097e5c11afe5bd4c01135779c9a40e707ef6374.1733243287.git.geert+renesas@glider.be
State New
Headers show
Series serial: sh-sci: Use plain struct copy in early_console_setup() | expand

Commit Message

Geert Uytterhoeven Dec. 3, 2024, 4:30 p.m. UTC
Using memcpy() prevents the compiler from doing any checking on the
types of the passed pointer parameters.  Copy the structure using struct
assignment instead, to increase type-safety.

No change in generated code on all relevant architectures
(arm/arm64/riscv/sh).

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/tty/serial/sh-sci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Claudiu Beznea Dec. 3, 2024, 4:45 p.m. UTC | #1
Hi, Geert,

On 03.12.2024 18:30, Geert Uytterhoeven wrote:
> Using memcpy() prevents the compiler from doing any checking on the
> types of the passed pointer parameters.  Copy the structure using struct
> assignment instead, to increase type-safety.
> 
> No change in generated code on all relevant architectures
> (arm/arm64/riscv/sh).
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Not sure, do you think it should carry a fixes tag?

> ---
>  drivers/tty/serial/sh-sci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index df523c7444230836..1ed13ce2c2952547 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -3542,7 +3542,7 @@ static int __init early_console_setup(struct earlycon_device *device,
>  		return -ENODEV;
>  
>  	device->port.type = type;
> -	memcpy(&sci_ports[0].port, &device->port, sizeof(struct uart_port));
> +	sci_ports[0].port = device->port;

I'm currently preparing my fixes around this code. I think I should rebase
on top of your patch :)


Thank you,
Claudiu

>  	port_cfg.type = type;
>  	sci_ports[0].cfg = &port_cfg;
>  	sci_ports[0].params = sci_probe_regmap(&port_cfg);
Geert Uytterhoeven Dec. 3, 2024, 5:01 p.m. UTC | #2
Hi Claudiu,

On Tue, Dec 3, 2024 at 5:45 PM Claudiu Beznea <claudiu.beznea@tuxon.dev> wrote:
> On 03.12.2024 18:30, Geert Uytterhoeven wrote:
> > Using memcpy() prevents the compiler from doing any checking on the
> > types of the passed pointer parameters.  Copy the structure using struct
> > assignment instead, to increase type-safety.
> >
> > No change in generated code on all relevant architectures
> > (arm/arm64/riscv/sh).
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Not sure, do you think it should carry a fixes tag?

Not really, as it's not a bug fix.

Gr{oetje,eeting}s,

                        Geert
Claudiu Beznea Dec. 4, 2024, 4:12 p.m. UTC | #3
Hi, Geert,

On 03.12.2024 18:30, Geert Uytterhoeven wrote:
> Using memcpy() prevents the compiler from doing any checking on the
> types of the passed pointer parameters.  Copy the structure using struct
> assignment instead, to increase type-safety.
> 
> No change in generated code on all relevant architectures
> (arm/arm64/riscv/sh).
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

I've tested this on RZ/G3S on top of series at [1] and device tree + clock
patches from [2], with renesas_defconfig and with upstream config, in the
following scenarios:

1/ "earlycon keep_bootcon" in bootargs
2/ "earlycon" in bootargs
3/ none of the "earlycon keep_bootcon", "earlycon" in bootargs

All good!

Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Thank you,
Claudiu

[1]
https://lore.kernel.org/all/20241204155806.3781200-1-claudiu.beznea.uj@bp.renesas.com/
[2]
https://lore.kernel.org/all/20241115134401.3893008-1-claudiu.beznea.uj@bp.renesas.com/

> ---
>  drivers/tty/serial/sh-sci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index df523c7444230836..1ed13ce2c2952547 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -3542,7 +3542,7 @@ static int __init early_console_setup(struct earlycon_device *device,
>  		return -ENODEV;
>  
>  	device->port.type = type;
> -	memcpy(&sci_ports[0].port, &device->port, sizeof(struct uart_port));
> +	sci_ports[0].port = device->port;
>  	port_cfg.type = type;
>  	sci_ports[0].cfg = &port_cfg;
>  	sci_ports[0].params = sci_probe_regmap(&port_cfg);
diff mbox series

Patch

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index df523c7444230836..1ed13ce2c2952547 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3542,7 +3542,7 @@  static int __init early_console_setup(struct earlycon_device *device,
 		return -ENODEV;
 
 	device->port.type = type;
-	memcpy(&sci_ports[0].port, &device->port, sizeof(struct uart_port));
+	sci_ports[0].port = device->port;
 	port_cfg.type = type;
 	sci_ports[0].cfg = &port_cfg;
 	sci_ports[0].params = sci_probe_regmap(&port_cfg);