Re: [PATCH 1/1] usb: chipidea: imx: include usbmisc_imx.c in ci_hdrc_imx.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 07, 2013 at 02:28:20PM +0200, Michael Grzeschik wrote:
> Hi Peter,
> 
> On Wed, Aug 07, 2013 at 05:24:45PM +0800, Peter Chen wrote:
> > On Wed, Aug 07, 2013 at 12:05:07PM +0200, Sascha Hauer wrote:
> > > > 
> > > > We need to know controller number, like pdev->id in the past.
> > > > The registers at non core register is messy, the specific bit
> > > > is for specific controller.
> > > 
> > > The controller number is present in the old binding:
> > > 
> > > fsl,usbmisc = <&usbmisc 1>;
> > >                        ^^^
> > > 
> > > Merging the usbmisc stuff into the imx driver shouldn't be a reason to
> > > change this binding.
> > > 
> > > Please get used to the fact that dt bindings cannot be changed easily
> > > just because the newer binding looks nicer. They define an ABI and
> > > breaking this ABI causes pain.
> > > 
> > 
> > But current situation is the device node of "usbmisc" will not be needed.
> > The usbmisc will not be a driver. We only need usbmisc to supply kinds
> > of SoC specific implementations, as well as filling the content of the data
> > at struct of_device_id (imx-usb's).
> > 
> > > > 
> > > > > > - When usbmisc_imx is a device, the register maps only one time.
> > > > > > But ci_hdrc_imx will be several devices, the non-core register
> > > > > > will be mapped several times, we have to use syscon to visit one register
> > > > > > region among several devices.
> > > > > 
> > > > > Converting the imx-usb misc driver to regmap is fine, but I see no
> > > > > reason to not make this a separate patch. This would make this much
> > > > > easier to read.
> > > > 
> > > > You mean delete "reg" entry at usbmisc dts, and using noncore phandle
> > > > at usbmisc_imx.c as the first patch, then, convert driver as separate
> > > > file.
> > > 
> > > I mean: create a patch which converts the usbmisc driver to regmap and
> > > base your other stuff on this patch.
> > > 
> > 
> > This is what I mean. We can't request one region twice, so we need to
> > delete devm_ioremap_resource at probe first.
> 
> Please try to reply with your answers in direct context to the questions.
> Otherwise this thread will not end and probably produce more confusion than
> answers. What we need is a clearly seperated series of independent changes;
> Just like Sascha already mentioned:
> 
> On Wed, Aug 07, 2013 at 10:15:22AM +0200, Sascha Hauer wrote:
> > This patch does far too many things at once.
> > 
> > - Convert imx-usb-misc from a driver into something which is called
> >   directly
> > - add aliases
> > - change devicetree bindings (which causes pain and it's not explained why
> >   this is necessary at all)
> > - converts the misc stuff to regmap
> > 
> > Please split this up so that it can be reviewed.
> 
> Your work needs the clean seperation before we can discuss the changes more in
> detail. Please try that first!
> 

Thanks, Michael. I will do that.
But discussion is needed, it can let us understand the problem deeply.
For you, what the real problem, for me, how to do it properly.

-- 

Best Regards,
Peter Chen

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux