Re: [PATCH v13 03/10] usb: dwc3: core: Refactor PHY logic to support Multiport Controller

[Johan Hovold <johan@kernel.org>]

On Fri, Oct 20, 2023 at 05:11:46PM +0530, Krishna Kurapati PSSNV wrote:
> On 10/20/2023 3:27 PM, Johan Hovold wrote:
> > On Sat, Oct 07, 2023 at 09:17:59PM +0530, Krishna Kurapati wrote:
> >> From: Harsh Agarwal <quic_harshq@quicinc.com>

> >> Co-developed-by: Harsh Agarwal <quic_harshq@quicinc.com>
> >> Signed-off-by: Harsh Agarwal <quic_harshq@quicinc.com>
> >> Co-developed-by:Krishna Kurapati <quic_kriskura@quicinc.com>
> >> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> > 
> > Thinh pointed out the problems with the above which were also reported
> > by checkpatch.pl.
> 
> I see that removing Co-Developed by tag for Harsh is helping with one 
> checkpatch issue. From what I know, although I made Harsh the Primary 
> author for the patch, should we still mention his Co-Developed-by along 
> with this Signed-Of by ?

This is all documented in the process document I pointed you at earlier.

You don't need a Co-Developed-by tag for the primary author.

> >> @@ -748,23 +781,32 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
> >>   static int dwc3_phy_init(struct dwc3 *dwc)
> >>   {
> >>   	int ret;
> >> +	int i;
> >> +	int j;
> > 
> > These could be declared on one line (same throughout).
> > 
> 
> I did so in v7, but was asked to put them in separate lines:
> https://lore.kernel.org/all/20230502221100.ecska23anlzv3iwq@synopsys.com/

Ok, either is fine if Thinh prefers it this way.

The problem is usually the other way round where people try to squeeze
in too much (e.g. unrelated variables and declarations) on the same
line.

> >> @@ -1892,9 +1975,12 @@ static int dwc3_read_port_info(struct dwc3 *dwc)
> >>   
> >>   	dev_dbg(dwc->dev, "hs-ports: %u ss-ports: %u\n",
> >>   			dwc->num_usb2_ports, dwc->num_usb3_ports);
> >> -
> > 
> > Drop this random change.
> 
> The removal of extra line here done you mean ?

Yes.

> > 
> >>   	iounmap(base);

Johan