linux-kernel - RE: [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable independent irqs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 9 Apr 2019 09:47:49 +0000
From:   Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
To:     Simon Horman <horms@...ge.net.au>
CC:     "kishon@...com" <kishon@...com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-renesas-soc@...r.kernel.org" 
        <linux-renesas-soc@...r.kernel.org>
Subject: RE: [PATCH 3/3] phy: renesas: rcar-gen3-usb2: enable/disable
 independent irqs

Hi Simon-san,

> From: Simon Horman, Sent: Wednesday, April 3, 2019 6:20 PM
> 
> Hi Shimoda-san,
> 
> On Mon, Apr 01, 2019 at 09:01:23PM +0900, Yoshihiro Shimoda wrote:
> > Since the previous code enabled/disabled the irqs both OHCI and EHCI,
> > it is possible to cause unexpected interruptions. To avoid this,
> > this patch creates multiple phy instances from phandle and
> > enables/disables independent irqs by the instances.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>
> 
> I noted a few nits below but overall this looks good to me.
> 
> Reviewed-by: Simon Horman <horms+renesas@...ge.net.au>

Thank you for your review!

<snip>
> > +#define NUM_OF_PHYS			4
> > +#define PHY_INDEX_BOTH_HC		0
> > +#define PHY_INDEX_OHCI			1
> > +#define PHY_INDEX_EHCI			2
> > +#define PHY_INDEX_HSUSB			3
> 
> nit: I think the above #defines would be better expressed as an enum.

I'll modify it.

<snip>
> > +static enum usb_dr_mode rcar_gen3_get_dr_mode(struct device_node *np)
> > +{
> > +	enum usb_dr_mode candidate = USB_DR_MODE_UNKNOWN, tmp;
> 
> nit: I think that there could be a better name for tmp, f.e. mode
> nit: The scope of tmp could be limited to inside the for loop.

I'll modify it.

Best regards,
Yoshihiro Shimoda