Re: [PATCHv5 2/7] usb: dwc2: Move gadget probe function into platform code

From: Felipe Balbi
Date: Thu Oct 23 2014 - 15:11:32 EST


On Wed, Oct 22, 2014 at 08:54:00PM +0000, Paul Zimmerman wrote:
> > From: Bartlomiej Zolnierkiewicz [mailto:b.zolnierkie@xxxxxxxxxxx]
> > Sent: Wednesday, October 22, 2014 4:16 AM
> >
> > On Monday, October 20, 2014 01:52:01 PM dinguyen@xxxxxxxxxxxxxxxxxxxxx wrote:
> > > From: Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx>
> > >
> > > This patch will aggregate the probing of gadget/hcd driver into platform.c.
> > > The gadget probe funtion is converted into gadget_init that is now only
> > > responsible for gadget only initialization. All the gadget resources is now
> > > handled by platform.c
> > >
> > > Since the host workqueue will not get initialized if the driver is configured
> > > for peripheral mode only. Thus we need to check for wq_otg before calling
> > > queue_work().
> > >
> > > Also, we move spin_lock_init to common location for both host and gadget that
> > > is either in platform.c or pci.c.
> > >
> > > We also ove suspend/resume code to common platform code, and update it to use
> > > the new PM API (struct dev_pm_ops).
> > >
> > > Lastly, move the "samsung,s3c6400-hsotg" binding into dwc2_of_match_table.
> >
> > This patch seems to break bisectability. It moves all the gadget probing
> > to platform.c but Kconfig/Makefile are not updated (platform.c will be
> > compiled only for CONFIG_USB_DWC2_PLATFORM=y which in turn depends on
> > CONFIG_USB_DWC2_HOST). IMO patch #7 should be merged into this one (#2).
>
> It doesn't break the compile, I already tested it. It does break the
> operation of the driver until patch #7 is applied, but I think that's
> OK in the middle of a patch series. I think it's a bit much to expect
> the driver to keep working at each step of a patch series like this.

It's your driver and, at the end of the day, your headache; but the
very day you need to run a git bisect and you end up in the middle of
one of these commits, you'll regret this statement :-)

There's usually a way to make sure things continue to work even if it
means duplicating some code until the conversion is completed, or adding
temporary flags which get added and removed within the same series, etc.

my 2 cents

--
balbi

Attachment: signature.asc
Description: Digital signature