| Message ID | 1353506793-8354-7-git-send-email-m.grzeschik@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 21, 2012 at 11:28:36PM +0300, Sergei Shtylyov wrote: > Hello. > > On 11/21/2012 05:06 PM, Michael Grzeschik wrote: > > > This adds mx53 as the next user of the usbmisc driver and makes it > > possible to disable the overcurrent-detection of the internal phy. > > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > --- > > Changes since v2: > > * added defines for register offsets and bitmasks > > * fixed disable_oc option for all ports > > > drivers/usb/chipidea/usbmisc_imx.c | 51 ++++++++++++++++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > > diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c > > index e090e0f..b1811df 100644 > > --- a/drivers/usb/chipidea/usbmisc_imx.c > > +++ b/drivers/usb/chipidea/usbmisc_imx.c > > @@ -19,6 +19,13 @@ > > > > #define USB_DEV_MAX 4 > > > > +#define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08; > > +#define MX53_USB_UH2_CTRL_OFFSET 0x14; > > +#define MX53_USB_UH3_CTRL_OFFSET 0x18; > > Semicolons here? This can't be right. You're right. Its compiling without errors, because its only used at the end of an assignement. So this will result in two Semicolons. I will fix that. Thanks, Michael
Hello. On 11/21/2012 05:06 PM, Michael Grzeschik wrote: > This adds mx53 as the next user of the usbmisc driver and makes it > possible to disable the overcurrent-detection of the internal phy. > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > --- > Changes since v2: > * added defines for register offsets and bitmasks > * fixed disable_oc option for all ports > drivers/usb/chipidea/usbmisc_imx.c | 51 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c > index e090e0f..b1811df 100644 > --- a/drivers/usb/chipidea/usbmisc_imx.c > +++ b/drivers/usb/chipidea/usbmisc_imx.c > @@ -19,6 +19,13 @@ > > #define USB_DEV_MAX 4 > > +#define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08; > +#define MX53_USB_UH2_CTRL_OFFSET 0x14; > +#define MX53_USB_UH3_CTRL_OFFSET 0x18; Semicolons here? This can't be right. WBR, Sergei
On Wed, Nov 21, 2012 at 03:06:32PM +0100, Michael Grzeschik wrote: > This adds mx53 as the next user of the usbmisc driver and makes it > possible to disable the overcurrent-detection of the internal phy. > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > --- > Changes since v2: > * added defines for register offsets and bitmasks > * fixed disable_oc option for all ports > > drivers/usb/chipidea/usbmisc_imx.c | 51 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c > index e090e0f..b1811df 100644 > --- a/drivers/usb/chipidea/usbmisc_imx.c > +++ b/drivers/usb/chipidea/usbmisc_imx.c > @@ -19,6 +19,13 @@ > > #define USB_DEV_MAX 4 > > +#define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08; > +#define MX53_USB_UH2_CTRL_OFFSET 0x14; > +#define MX53_USB_UH3_CTRL_OFFSET 0x18; > +#define MX53_BM_OVER_CUR_DIS_H1 BIT(5) > +#define MX53_BM_OVER_CUR_DIS_OTG BIT(8) > +#define MX53_BM_OVER_CUR_DIS_UHx BIT(30) > + > #define MX6_BM_OVER_CUR_DIS BIT(7) As every SoC platform's non-core register mapping is different, it is better to use .h to put register bit definition > > struct imx_usbmisc { > @@ -54,6 +61,45 @@ static struct usbmisc_usb_device *get_usbdev(struct device *dev) > return &usbmisc->usbdev[i]; > } > > +static int usbmisc_imx53_init(struct device *dev) > +{ > + struct usbmisc_usb_device *usbdev; > + void __iomem *reg; > + unsigned long flags; > + u32 val; > + > + usbdev = get_usbdev(dev); > + if (IS_ERR(usbdev)) > + return PTR_ERR(usbdev); > + > + if (usbdev->disable_oc) { > + spin_lock_irqsave(&usbmisc->lock, flags); > + switch (usbdev->index) { > + case 0: > + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; > + val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; > + break; > + case 1: > + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; > + val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; > + break; > + case 2: > + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; > + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; > + break; > + case 3: > + reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; > + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; > + break; > + } > + if (reg && val) > + writel(val, reg); > + spin_unlock_irqrestore(&usbmisc->lock, flags); > + } > + > + return 0; > +} > + > static int usbmisc_imx6q_init(struct device *dev) > { > > @@ -76,11 +122,16 @@ static int usbmisc_imx6q_init(struct device *dev) > return 0; > } > > +static const struct usbmisc_ops imx53_usbmisc_ops = { > + .init = usbmisc_imx53_init, > +}; > + > static const struct usbmisc_ops imx6q_usbmisc_ops = { > .init = usbmisc_imx6q_init, > }; > > static const struct of_device_id usbmisc_imx_dt_ids[] = { > + { .compatible = "fsl,imx53-usbmisc", .data = (void *)&imx53_usbmisc_ops }, > { .compatible = "fsl,imx6q-usbmisc", .data = (void *)&imx6q_usbmisc_ops }, > { /* sentinel */ } > }; > -- > 1.7.10.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Fri, Nov 23, 2012 at 02:53:41PM +0800, Peter Chen wrote: > On Wed, Nov 21, 2012 at 03:06:32PM +0100, Michael Grzeschik wrote: > > This adds mx53 as the next user of the usbmisc driver and makes it > > possible to disable the overcurrent-detection of the internal phy. > > > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > --- > > Changes since v2: > > * added defines for register offsets and bitmasks > > * fixed disable_oc option for all ports > > > > drivers/usb/chipidea/usbmisc_imx.c | 51 ++++++++++++++++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > > > diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c > > index e090e0f..b1811df 100644 > > --- a/drivers/usb/chipidea/usbmisc_imx.c > > +++ b/drivers/usb/chipidea/usbmisc_imx.c > > @@ -19,6 +19,13 @@ > > > > #define USB_DEV_MAX 4 > > > > +#define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08; > > +#define MX53_USB_UH2_CTRL_OFFSET 0x14; > > +#define MX53_USB_UH3_CTRL_OFFSET 0x18; > > +#define MX53_BM_OVER_CUR_DIS_H1 BIT(5) > > +#define MX53_BM_OVER_CUR_DIS_OTG BIT(8) > > +#define MX53_BM_OVER_CUR_DIS_UHx BIT(30) > > + > > #define MX6_BM_OVER_CUR_DIS BIT(7) > > As every SoC platform's non-core register mapping is different, > it is better to use .h to put register bit definition What do the different register mapping have to do with putting the definitions in .h files or not? Sascha
On 11/23/2012 07:53 AM, Peter Chen wrote: > On Wed, Nov 21, 2012 at 03:06:32PM +0100, Michael Grzeschik wrote: >> This adds mx53 as the next user of the usbmisc driver and makes it >> possible to disable the overcurrent-detection of the internal phy. >> >> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> >> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> >> --- >> Changes since v2: >> * added defines for register offsets and bitmasks >> * fixed disable_oc option for all ports >> >> drivers/usb/chipidea/usbmisc_imx.c | 51 ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 51 insertions(+) >> >> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c >> index e090e0f..b1811df 100644 >> --- a/drivers/usb/chipidea/usbmisc_imx.c >> +++ b/drivers/usb/chipidea/usbmisc_imx.c >> @@ -19,6 +19,13 @@ >> >> #define USB_DEV_MAX 4 >> >> +#define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08; >> +#define MX53_USB_UH2_CTRL_OFFSET 0x14; >> +#define MX53_USB_UH3_CTRL_OFFSET 0x18; >> +#define MX53_BM_OVER_CUR_DIS_H1 BIT(5) >> +#define MX53_BM_OVER_CUR_DIS_OTG BIT(8) >> +#define MX53_BM_OVER_CUR_DIS_UHx BIT(30) >> + >> #define MX6_BM_OVER_CUR_DIS BIT(7) > > As every SoC platform's non-core register mapping is different, > it is better to use .h to put register bit definition AFAIK in the kernel we don't make extra .h files any more, if there is only a single user of the contents. Marc
On Fri, Nov 23, 2012 at 08:39:39AM +0100, Sascha Hauer wrote: > On Fri, Nov 23, 2012 at 02:53:41PM +0800, Peter Chen wrote: > > On Wed, Nov 21, 2012 at 03:06:32PM +0100, Michael Grzeschik wrote: > > > This adds mx53 as the next user of the usbmisc driver and makes it > > > possible to disable the overcurrent-detection of the internal phy. > > > > > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > > --- > > > Changes since v2: > > > * added defines for register offsets and bitmasks > > > * fixed disable_oc option for all ports > > > > > > drivers/usb/chipidea/usbmisc_imx.c | 51 ++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 51 insertions(+) > > > > > > diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c > > > index e090e0f..b1811df 100644 > > > --- a/drivers/usb/chipidea/usbmisc_imx.c > > > +++ b/drivers/usb/chipidea/usbmisc_imx.c > > > @@ -19,6 +19,13 @@ > > > > > > #define USB_DEV_MAX 4 > > > > > > +#define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08; > > > +#define MX53_USB_UH2_CTRL_OFFSET 0x14; > > > +#define MX53_USB_UH3_CTRL_OFFSET 0x18; > > > +#define MX53_BM_OVER_CUR_DIS_H1 BIT(5) > > > +#define MX53_BM_OVER_CUR_DIS_OTG BIT(8) > > > +#define MX53_BM_OVER_CUR_DIS_UHx BIT(30) > > > + > > > #define MX6_BM_OVER_CUR_DIS BIT(7) > > > > As every SoC platform's non-core register mapping is different, > > it is better to use .h to put register bit definition > > What do the different register mapping have to do with putting the > definitions in .h files or not? No difference, just keep the .c clean and short, as every soc has its non-core register mapping, so it may occupy several ten lines if you write down every offset and bit for single SoC, the bit definition will grow longer and longer when more SoCs and more function are added. > > Sascha > > -- > 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 | >
On Fri, Nov 23, 2012 at 11:02:13AM +0100, Marc Kleine-Budde wrote: > On 11/23/2012 07:53 AM, Peter Chen wrote: > > On Wed, Nov 21, 2012 at 03:06:32PM +0100, Michael Grzeschik wrote: > >> This adds mx53 as the next user of the usbmisc driver and makes it > >> possible to disable the overcurrent-detection of the internal phy. > >> > >> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > >> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > >> --- > >> Changes since v2: > >> * added defines for register offsets and bitmasks > >> * fixed disable_oc option for all ports > >> > >> drivers/usb/chipidea/usbmisc_imx.c | 51 ++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 51 insertions(+) > >> > >> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c > >> index e090e0f..b1811df 100644 > >> --- a/drivers/usb/chipidea/usbmisc_imx.c > >> +++ b/drivers/usb/chipidea/usbmisc_imx.c > >> @@ -19,6 +19,13 @@ > >> > >> #define USB_DEV_MAX 4 > >> > >> +#define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08; > >> +#define MX53_USB_UH2_CTRL_OFFSET 0x14; > >> +#define MX53_USB_UH3_CTRL_OFFSET 0x18; > >> +#define MX53_BM_OVER_CUR_DIS_H1 BIT(5) > >> +#define MX53_BM_OVER_CUR_DIS_OTG BIT(8) > >> +#define MX53_BM_OVER_CUR_DIS_UHx BIT(30) > >> + > >> #define MX6_BM_OVER_CUR_DIS BIT(7) > > > > As every SoC platform's non-core register mapping is different, > > it is better to use .h to put register bit definition > > AFAIK in the kernel we don't make extra .h files any more, if there is > only a single user of the contents. OK, if it is the current rule for kernel, then just keep it. My original mind was there will be more and more bit definition in future, and move it to .h to make the code clean and short. > > Marc > -- > Pengutronix e.K. | Marc Kleine-Budde | > Industrial Linux Solutions | Phone: +49-231-2826-924 | > Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | > Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | >
On Sun, 25 Nov 2012 16:09:31 +0800, Peter Chen <peter.chen@freescale.com> wrote: > On Fri, Nov 23, 2012 at 11:02:13AM +0100, Marc Kleine-Budde wrote: > > On 11/23/2012 07:53 AM, Peter Chen wrote: > > > On Wed, Nov 21, 2012 at 03:06:32PM +0100, Michael Grzeschik wrote: > > >> This adds mx53 as the next user of the usbmisc driver and makes it > > >> possible to disable the overcurrent-detection of the internal phy. > > >> > > >> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > > >> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > >> --- > > >> Changes since v2: > > >> * added defines for register offsets and bitmasks > > >> * fixed disable_oc option for all ports > > >> > > >> drivers/usb/chipidea/usbmisc_imx.c | 51 ++++++++++++++++++++++++++++++++++++ > > >> 1 file changed, 51 insertions(+) > > >> > > >> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c > > >> index e090e0f..b1811df 100644 > > >> --- a/drivers/usb/chipidea/usbmisc_imx.c > > >> +++ b/drivers/usb/chipidea/usbmisc_imx.c > > >> @@ -19,6 +19,13 @@ > > >> > > >> #define USB_DEV_MAX 4 > > >> > > >> +#define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08; > > >> +#define MX53_USB_UH2_CTRL_OFFSET 0x14; > > >> +#define MX53_USB_UH3_CTRL_OFFSET 0x18; > > >> +#define MX53_BM_OVER_CUR_DIS_H1 BIT(5) > > >> +#define MX53_BM_OVER_CUR_DIS_OTG BIT(8) > > >> +#define MX53_BM_OVER_CUR_DIS_UHx BIT(30) > > >> + > > >> #define MX6_BM_OVER_CUR_DIS BIT(7) > > > > > > As every SoC platform's non-core register mapping is different, > > > it is better to use .h to put register bit definition > > > > AFAIK in the kernel we don't make extra .h files any more, if there is > > only a single user of the contents. > OK, if it is the current rule for kernel, then just keep it. > My original mind was there will be more and more bit definition > in future, and move it to .h to make the code clean and short. Yeah. If there is only one user then keep it in the .c file. The kernel is huge, so keeping things out of the global scope is a GoodThing(tm). g.
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index e090e0f..b1811df 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -19,6 +19,13 @@ #define USB_DEV_MAX 4 +#define MX53_USB_OTG_PHY_CTRL_0_OFFSET 0x08; +#define MX53_USB_UH2_CTRL_OFFSET 0x14; +#define MX53_USB_UH3_CTRL_OFFSET 0x18; +#define MX53_BM_OVER_CUR_DIS_H1 BIT(5) +#define MX53_BM_OVER_CUR_DIS_OTG BIT(8) +#define MX53_BM_OVER_CUR_DIS_UHx BIT(30) + #define MX6_BM_OVER_CUR_DIS BIT(7) struct imx_usbmisc { @@ -54,6 +61,45 @@ static struct usbmisc_usb_device *get_usbdev(struct device *dev) return &usbmisc->usbdev[i]; } +static int usbmisc_imx53_init(struct device *dev) +{ + struct usbmisc_usb_device *usbdev; + void __iomem *reg; + unsigned long flags; + u32 val; + + usbdev = get_usbdev(dev); + if (IS_ERR(usbdev)) + return PTR_ERR(usbdev); + + if (usbdev->disable_oc) { + spin_lock_irqsave(&usbmisc->lock, flags); + switch (usbdev->index) { + case 0: + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + break; + case 1: + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + break; + case 2: + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + break; + case 3: + reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + break; + } + if (reg && val) + writel(val, reg); + spin_unlock_irqrestore(&usbmisc->lock, flags); + } + + return 0; +} + static int usbmisc_imx6q_init(struct device *dev) { @@ -76,11 +122,16 @@ static int usbmisc_imx6q_init(struct device *dev) return 0; } +static const struct usbmisc_ops imx53_usbmisc_ops = { + .init = usbmisc_imx53_init, +}; + static const struct usbmisc_ops imx6q_usbmisc_ops = { .init = usbmisc_imx6q_init, }; static const struct of_device_id usbmisc_imx_dt_ids[] = { + { .compatible = "fsl,imx53-usbmisc", .data = (void *)&imx53_usbmisc_ops }, { .compatible = "fsl,imx6q-usbmisc", .data = (void *)&imx6q_usbmisc_ops }, { /* sentinel */ } };