> >
> > Signed-off-by: Alexander Shiyan <shc_work@xxxxxxx>
> > ---
> > drivers/usb/chipidea/usbmisc_imx.c | 40 +++++++++++++++++++++---------
> --------
> > 1 file changed, 22 insertions(+), 18 deletions(-)
> >
> >
> > -static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
> > +static int usbmisc_imx5_init(struct imx_usbmisc_data *data)
>
> Can we keep usbmisc_imx53_init named this?
>
> There is more to do here on i.MX5 for cases where the bootloader did
> not already try to set up USB (or did it badly) such as the
> transceiver reference clock rate and also setting the USB sysclk clock
> source (internal DPLL or from external transceiver in the external
> ULPI case) and so on, which is related and needs doing on both, but is
> different on i.MX51 than i.MX53. This is information sort of best
> passed in the PHY node that goes along with this, but it's set within
> the usbmisc block of the chips so the usbmisc driver will have a
> responsibility to go see if it's an external PHY that is feeding it's
> clock back into the USB block in this way.
>
> I am not sure we (Peter etc.) discussed how best to do this, the code
> to pull the correct information out always seems kind of misplaced no
> matter where it goes, but the responsibility for tweaking those
> registers is most certainly this driver.
>
> Essentially the layout of usbmisc->base + 0x10 register (USB_CTRL_1)
> is different when doing the above, and dependent on a board-specific
> option for the input clock to the transceiver. We could reduce a
> little churn, later, when usbmisc_imx could be given related usbphy
> information and actually do the right thing. I have a patch kinda
> sitting in the wings to do this.. and two *real* pieces of consumer
> hardware that need it, and some other kicking, to make USB work in the
> never-touched-before-Linux case.
>
> > -static const struct usbmisc_ops imx53_usbmisc_ops = {
> > - .init = usbmisc_imx53_init,
> > +static const struct usbmisc_ops imx5_usbmisc_ops = {
> > + .init = usbmisc_imx5_init,
> > };
>
> And keep imx53_usbmisc_ops named this?
>
> > static const struct usbmisc_ops imx6q_usbmisc_ops = {
> > @@ -204,8 +204,12 @@ static const struct of_device_id
> usbmisc_imx_dt_ids[] = {
> > .data = &imx27_usbmisc_ops,
> > },
> > {
> > + .compatible = "fsl,imx51-usbmisc",
> > + .data = &imx5_usbmisc_ops,
>
> And then just use &imx53_usbmisc_ops?
>
> This gives us some breathing room later to actually do the right thing
> without additionally performing renames all over the place to make
> imx5 -> imx53 (again)/imx51 (new).
Hi Alexander,
You may take matt's suggestion, it can reduce the code change now and in future.
We can only add device_id for imx51 in this patch, split imx53 and imx51's ops
when their differences are added in future.
Peter
--
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
