Hi Shimoda-san,
On Fri, May 10, 2019 at 12:53 PM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> This patch adds a specific struct "usbhs_of_data" to add a new SoC
> data easily instead of code basis in the future.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
Thanks for your patch!
Looks correct to me, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
with a few suggestions/questions below...
> --- a/drivers/usb/renesas_usbhs/common.c
> +++ b/drivers/usb/renesas_usbhs/common.c
> @@ -598,8 +638,15 @@ static struct renesas_usbhs_platform_info *usbhs_parse_dt(struct device *dev)
> if (!info)
> return NULL;
>
> + data = of_device_get_match_data(dev);
> + if (!data)
> + return NULL;
> +
> dparam = &info->driver_param;
> - dparam->type = (uintptr_t)of_device_get_match_data(dev);
> + memcpy(dparam, &data->param, sizeof(struct renesas_usbhs_driver_param));
sizeof(data->param), for increased safety?
> + memcpy(&info->platform_callback, data->platform_callback,
> + sizeof(struct renesas_usbhs_platform_callback));
sizeof(data->platform_callback)?
I'm not really familiar with this driver and with the USB subsystem, but
why is this driver copying so many structs around, instead of just
keeping pointers to the originals?
Is all of that done just so .notify_hotplug() can be overridden, or am I
missing something?
Thanks again!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
