From: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: Simon Horman <horms@verge.net.au>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"linux-renesas-soc@vger.kernel.org"
<linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH v2] usb: renesas_usbhs: Use specific struct instead of USBHS_TYPE_* enums
Date: Tue, 21 May 2019 10:03:54 +0200 [thread overview]
Message-ID: <20190521080354.GA25718@kroah.com> (raw)
In-Reply-To: <OSBPR01MB3174C65FDF208431F988068DD8090@OSBPR01MB3174.jpnprd01.prod.outlook.com>
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@renesas.com>
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > Hi Shimoda-san,
> >
> > the minor suggestion below not withstanding this looks good to me.
> >
> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
>
> 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!
prev parent reply other threads:[~2019-05-21 8:03 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-13 2:40 [PATCH v2] usb: renesas_usbhs: Use specific struct instead of USBHS_TYPE_* enums Yoshihiro Shimoda
2019-05-13 12:01 ` Simon Horman
2019-05-15 6:57 ` Yoshihiro Shimoda
2019-05-15 8:10 ` Simon Horman
2019-05-21 8:03 ` gregkh [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190521080354.GA25718@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=horms@verge.net.au \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=yoshihiro.shimoda.uh@renesas.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).