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]

 



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

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
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