On Wed, Aug 07, 2013 at 10:15:22AM +0200, Sascha Hauer wrote: > On Wed, Aug 07, 2013 at 01:34:37PM +0800, Peter Chen wrote: > > At former design, both ci_hdrc_imx and usbmisc_imx are individual > > module, ci_hdrc_imx is glue layer for imx usb driver. usbmisc_imx > > handles non-core registers which has different register layout > > for imx SoC serials, it usually supplies interface for wakeup > > setting, PHY setting, etc. > > > > usbmisc_imx uses symbols from ci_hdrc_imx, So, when we combile both > > of drivers as loadable module, usbmisc is needed to be unload first. > > However at ci_hdrc_imx, it needs to call usbmisc_imx's interface > > at its removal function once we enable wakeup/runtime pm function, so > > keeping two drivers can't work well at loadable module use case. > > > > To fix loadable module use case, we need to build usbmisc_imx into > > ci_hdrc_imx. The usbmisc_imx still handles misc setting for controllers, > > and will be included at ci_hdrc_imx. > > > > There is a short discussion for it: > > http://marc.info/?l=linux-usb&m=136861599423172&w=2 > > > > Besides, we update dts and binding doc for at this commit > > 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) Yes, I need to split doc from implementation. > - converts the misc stuff to regmap > > Please split this up so that it can be reviewed. The changes are all related to convert usbmisc_imx from a driver to something which can be called directly. - usbmisc has #index-cells to indicate controller number, now, it is not a driver, we use aliases at controller driver to instead of it. - 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. > > > > > #include "ci.h" > > #include "ci_hdrc_imx.h" > > +#include "usbmisc_imx.c" > > Don't include C files. > Yes, it is not a good practice, but I want to keep ci_hdrc_imx.c as clean as possible. The SoC related implementation will be more and more in the future after we add more USB functions. Do you have any good suggestions how to organize general imx stuff and SoC specifc? Put all the things at controller driver is another solution. > > - ret); > > - memset(usbdev, 0, sizeof(*usbdev)); > > + ret = of_alias_get_id(np, "usb"); > > + if (ret < 0) { > > + dev_err(dev, "failed to get alias id, errno %d\n", ret); > > return ret; > > } > > - usbdev->index = args.args[0]; > > - of_node_put(args.np); > > > > if (of_find_property(np, "disable-over-current", NULL)) > > - usbdev->disable_oc = 1; > > + data->disable_oc = 1; > > > > if (of_find_property(np, "external-vbus-divider", NULL)) > > - usbdev->evdo = 1; > > + data->evdo = 1; > > + > > + /* mx23 and mx28 doesn't have non core registers */ > > + if (data->misc_data && (!strcmp(data->misc_data->name, "imx23-usb") || > > + !strcmp(data->misc_data->name, "imx28-usb"))) > > + return 0; > > If a USB node does not have a usbmisc (or noncore) property then don't > initialize it. No need for string compares. > OK, will let the code be simple, I can't add device_id for mxs either. -- 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
