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

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

[Geert Uytterhoeven]

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(-)

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);
-- 
2.34.1

Re: [PATCH] serial: sh-sci: Use plain struct copy in early_console_setup()

[Claudiu Beznea]

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);

Re: [PATCH] serial: sh-sci: Use plain struct copy in early_console_setup()

[Claudiu Beznea]

On 03.12.2024 18:45, Claudiu Beznea wrote:
> 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

s/I think/I'm not sure if/g

> 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);

Re: [PATCH] serial: sh-sci: Use plain struct copy in early_console_setup()

[Geert Uytterhoeven]

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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] serial: sh-sci: Use plain struct copy in early_console_setup()

[Lad, Prabhakar]

On Tue, Dec 3, 2024 at 4:30 PM Geert Uytterhoeven
<geert+renesas@glider.be> 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>
> ---
>  drivers/tty/serial/sh-sci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

> 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);
> --
> 2.34.1
>
>

Re: [PATCH] serial: sh-sci: Use plain struct copy in early_console_setup()

[Claudiu Beznea]

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);

Re: [PATCH] serial: sh-sci: Use plain struct copy in early_console_setup()

[John Paul Adrian Glaubitz]

On Tue, 2024-12-03 at 17:30 +0100, 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>
> ---
>  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);

Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Thanks,
Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913