On Wed, May 15, 2019 at 06:57:17AM +0000, Yoshihiro Shimoda wrote: > Hi Simon-san, > > > From: Simon Horman, Sent: Monday, May 13, 2019 9:01 PM > > > > On Mon, May 13, 2019 at 11:40:29AM +0900, Yoshihiro Shimoda 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> > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > > > > Hi Shimoda-san, > > > > the minor suggestion below not withstanding this looks good to me. > > > > Reviewed-by: Simon Horman <horms+renesas@xxxxxxxxxxxx> > > Thank you for your review! > > > > --- > > > Changes from v1 [1]: > > > - Use sizeof(variable) instead of sizeof(type). > > > - Add Geert-san's reviewed-by (thanks!). > > > > > > [1] > > > https://patchwork.kernel.org/patch/10938575/ > > > > > > drivers/usb/renesas_usbhs/common.c | 112 +++++++++++++++++++++---------------- > > > drivers/usb/renesas_usbhs/common.h | 5 ++ > > > 2 files changed, 70 insertions(+), 47 deletions(-) > > > > > > diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c > > > index 249fbee..0ca89de 100644 > > > --- a/drivers/usb/renesas_usbhs/common.c > > > +++ b/drivers/usb/renesas_usbhs/common.c > <snip> > > > @@ -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(data->param)); > > > + memcpy(&info->platform_callback, data->platform_callback, > > > + sizeof(*data->platform_callback)); > > > > Can we avoid the error-proneness of calls to sizeof() as follows? > > > > *dparam = data->param; > > info->platform_callback = *data->platform_callback; > > Since Chris-san has submitted a patch series [1] that is based on this patch today, > I'd like to submit an incremental patch to avoid the error-proneness in the renesas_usbhs > (common.c and mod_host.c) later, if possible. > > Greg-san, is it acceptable? Fine with me!