| Message ID | 1468840547-17899-1-git-send-email-jun.li@nxp.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Mon, Jul 18, 2016 at 07:15:47PM +0800, Li Jun wrote: > With over-current-polarity property added, imx usb over current > polarity can be configed to be low or high active, since the default > setting value(0) is for active high, so keep this setting for those > legacy platforms without this property specified. > > Signed-off-by: Li Jun <jun.li@nxp.com> > --- > drivers/usb/chipidea/ci_hdrc_imx.c | 9 +++++++++ > drivers/usb/chipidea/ci_hdrc_imx.h | 1 + > drivers/usb/chipidea/usbmisc_imx.c | 30 +++++++++++++++++++++++++----- > 3 files changed, 35 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c > index dedc33e..61e712b 100644 > --- a/drivers/usb/chipidea/ci_hdrc_imx.c > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c > @@ -140,6 +140,15 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev) > if (of_find_property(np, "disable-over-current", NULL)) > data->disable_oc = 1; > > + if (!of_property_read_u32(np, "over-current-polarity", &ret)) > + data->oc_polarity = ret ? 1 : 0; > + else > + /* > + * Keep the oc polarity setting of legacy > + * platforms unchanged. > + */ > + data->oc_polarity = 1; We may can't ensure that, since there are lots of i.mx platforms, eg from imx27 to imx7. If the user does not assign oc polarity at DT, we need to read it from register. Peter > + > if (of_find_property(np, "external-vbus-divider", NULL)) > data->evdo = 1; > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h > index 635717e..409aa5ca8 100644 > --- a/drivers/usb/chipidea/ci_hdrc_imx.h > +++ b/drivers/usb/chipidea/ci_hdrc_imx.h > @@ -17,6 +17,7 @@ struct imx_usbmisc_data { > int index; > > unsigned int disable_oc:1; /* over current detect disabled */ > + unsigned int oc_polarity:1; /* over current polarity if oc enabled */ > unsigned int evdo:1; /* set external vbus divider option */ > }; > > diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c > index ab8b027..193dbe4 100644 > --- a/drivers/usb/chipidea/usbmisc_imx.c > +++ b/drivers/usb/chipidea/usbmisc_imx.c > @@ -56,6 +56,7 @@ > > #define MX6_BM_NON_BURST_SETTING BIT(1) > #define MX6_BM_OVER_CUR_DIS BIT(7) > +#define MX6_BM_OVER_CUR_POLARITY BIT(8) > #define MX6_BM_WAKEUP_ENABLE BIT(10) > #define MX6_BM_ID_WAKEUP BIT(16) > #define MX6_BM_VBUS_WAKEUP BIT(17) > @@ -266,11 +267,18 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data *data) > > spin_lock_irqsave(&usbmisc->lock, flags); > > + reg = readl(usbmisc->base + data->index * 4); > if (data->disable_oc) { > - reg = readl(usbmisc->base + data->index * 4); > - writel(reg | MX6_BM_OVER_CUR_DIS, > - usbmisc->base + data->index * 4); > + reg |= MX6_BM_OVER_CUR_DIS; > + } else if (data->oc_polarity == 1) { > + /* High active */ > + reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY); > + } else { > + /* Low active */ > + reg &= ~MX6_BM_OVER_CUR_DIS; > + reg |= MX6_BM_OVER_CUR_POLARITY; > } > + writel(reg, usbmisc->base + data->index * 4); > > /* SoC non-burst setting */ > reg = readl(usbmisc->base + data->index * 4); > @@ -365,10 +373,18 @@ static int usbmisc_imx7d_init(struct imx_usbmisc_data *data) > return -EINVAL; > > spin_lock_irqsave(&usbmisc->lock, flags); > + reg = readl(usbmisc->base); > if (data->disable_oc) { > - reg = readl(usbmisc->base); > - writel(reg | MX6_BM_OVER_CUR_DIS, usbmisc->base); > + reg |= MX6_BM_OVER_CUR_DIS; > + } else if (data->oc_polarity == 1) { > + /* High active */ > + reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY); > + } else { > + /* Low active */ > + reg &= ~MX6_BM_OVER_CUR_DIS; > + reg |= MX6_BM_OVER_CUR_POLARITY; > } > + writel(reg, usbmisc->base); > > reg = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2); > reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK; > @@ -492,6 +508,10 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = { > .compatible = "fsl,imx6ul-usbmisc", > .data = &imx6sx_usbmisc_ops, > }, > + { > + .compatible = "fsl,imx7d-usbmisc", > + .data = &imx7d_usbmisc_ops, > + }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, usbmisc_imx_dt_ids); > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi > -----Original Message----- > From: Peter Chen [mailto:hzpeterchen@gmail.com] > Sent: Tuesday, July 19, 2016 9:57 AM > To: Jun Li <jun.li@nxp.com> > Cc: Peter Chen <peter.chen@nxp.com>; robh+dt@kernel.org; > shawnguo@kernel.org; devicetree@vger.kernel.org; linux-usb@vger.kernel.org; > linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH 4/4] usb: chipidea: imx: set over current polarity per > dts setting > > On Mon, Jul 18, 2016 at 07:15:47PM +0800, Li Jun wrote: > > With over-current-polarity property added, imx usb over current > > polarity can be configed to be low or high active, since the default > > setting value(0) is for active high, so keep this setting for those > > legacy platforms without this property specified. > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > --- > > drivers/usb/chipidea/ci_hdrc_imx.c | 9 +++++++++ > > drivers/usb/chipidea/ci_hdrc_imx.h | 1 + > > drivers/usb/chipidea/usbmisc_imx.c | 30 +++++++++++++++++++++++++----- > > 3 files changed, 35 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c > > b/drivers/usb/chipidea/ci_hdrc_imx.c > > index dedc33e..61e712b 100644 > > --- a/drivers/usb/chipidea/ci_hdrc_imx.c > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c > > @@ -140,6 +140,15 @@ static struct imx_usbmisc_data > *usbmisc_get_init_data(struct device *dev) > > if (of_find_property(np, "disable-over-current", NULL)) > > data->disable_oc = 1; > > > > + if (!of_property_read_u32(np, "over-current-polarity", &ret)) > > + data->oc_polarity = ret ? 1 : 0; > > + else > > + /* > > + * Keep the oc polarity setting of legacy > > + * platforms unchanged. > > + */ > > + data->oc_polarity = 1; > > We may can't ensure that, since there are lots of i.mx platforms, eg from > imx27 to imx7. > > If the user does not assign oc polarity at DT, we need to read it from > register. I suppose old platforms (other than i.mx6&7) should set either "over-current-polarity" or "disable-over-current" if want to use "data->oc_polarity". Do you mean read the register value before set "data->oc_polarity" here? Even with that, as I still can't ensure all i.mx platforms have the same mapping: 0 <--> active high. 1 <--> active low. How should I set "data->oc_polarity" accordingly? Li Jun > > Peter > > + > > if (of_find_property(np, "external-vbus-divider", NULL)) > > data->evdo = 1; > > > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h > > b/drivers/usb/chipidea/ci_hdrc_imx.h > > index 635717e..409aa5ca8 100644 > > --- a/drivers/usb/chipidea/ci_hdrc_imx.h > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.h > > @@ -17,6 +17,7 @@ struct imx_usbmisc_data { > > int index; > > > > unsigned int disable_oc:1; /* over current detect disabled */ > > + unsigned int oc_polarity:1; /* over current polarity if oc enabled > > +*/ > > unsigned int evdo:1; /* set external vbus divider option */ }; > > > > diff --git a/drivers/usb/chipidea/usbmisc_imx.c > > b/drivers/usb/chipidea/usbmisc_imx.c > > index ab8b027..193dbe4 100644 > > --- a/drivers/usb/chipidea/usbmisc_imx.c > > +++ b/drivers/usb/chipidea/usbmisc_imx.c > > @@ -56,6 +56,7 @@ > > > > #define MX6_BM_NON_BURST_SETTING BIT(1) > > #define MX6_BM_OVER_CUR_DIS BIT(7) > > +#define MX6_BM_OVER_CUR_POLARITY BIT(8) > > #define MX6_BM_WAKEUP_ENABLE BIT(10) > > #define MX6_BM_ID_WAKEUP BIT(16) > > #define MX6_BM_VBUS_WAKEUP BIT(17) > > @@ -266,11 +267,18 @@ static int usbmisc_imx6q_init(struct > > imx_usbmisc_data *data) > > > > spin_lock_irqsave(&usbmisc->lock, flags); > > > > + reg = readl(usbmisc->base + data->index * 4); > > if (data->disable_oc) { > > - reg = readl(usbmisc->base + data->index * 4); > > - writel(reg | MX6_BM_OVER_CUR_DIS, > > - usbmisc->base + data->index * 4); > > + reg |= MX6_BM_OVER_CUR_DIS; > > + } else if (data->oc_polarity == 1) { > > + /* High active */ > > + reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY); > > + } else { > > + /* Low active */ > > + reg &= ~MX6_BM_OVER_CUR_DIS; > > + reg |= MX6_BM_OVER_CUR_POLARITY; > > } > > + writel(reg, usbmisc->base + data->index * 4); > > > > /* SoC non-burst setting */ > > reg = readl(usbmisc->base + data->index * 4); @@ -365,10 +373,18 @@ > > static int usbmisc_imx7d_init(struct imx_usbmisc_data *data) > > return -EINVAL; > > > > spin_lock_irqsave(&usbmisc->lock, flags); > > + reg = readl(usbmisc->base); > > if (data->disable_oc) { > > - reg = readl(usbmisc->base); > > - writel(reg | MX6_BM_OVER_CUR_DIS, usbmisc->base); > > + reg |= MX6_BM_OVER_CUR_DIS; > > + } else if (data->oc_polarity == 1) { > > + /* High active */ > > + reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY); > > + } else { > > + /* Low active */ > > + reg &= ~MX6_BM_OVER_CUR_DIS; > > + reg |= MX6_BM_OVER_CUR_POLARITY; > > } > > + writel(reg, usbmisc->base); > > > > reg = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2); > > reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK; > > @@ -492,6 +508,10 @@ static const struct of_device_id > usbmisc_imx_dt_ids[] = { > > .compatible = "fsl,imx6ul-usbmisc", > > .data = &imx6sx_usbmisc_ops, > > }, > > + { > > + .compatible = "fsl,imx7d-usbmisc", > > + .data = &imx7d_usbmisc_ops, > > + }, > > { /* sentinel */ } > > }; > > MODULE_DEVICE_TABLE(of, usbmisc_imx_dt_ids); > > -- > > 1.9.1 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > -- > > Best Regards, > Peter Chen -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 19, 2016 at 02:31:41AM +0000, Jun Li wrote: > Hi > > > -----Original Message----- > > From: Peter Chen [mailto:hzpeterchen@gmail.com] > > Sent: Tuesday, July 19, 2016 9:57 AM > > To: Jun Li <jun.li@nxp.com> > > Cc: Peter Chen <peter.chen@nxp.com>; robh+dt@kernel.org; > > shawnguo@kernel.org; devicetree@vger.kernel.org; linux-usb@vger.kernel.org; > > linux-arm-kernel@lists.infradead.org > > Subject: Re: [PATCH 4/4] usb: chipidea: imx: set over current polarity per > > dts setting > > > > On Mon, Jul 18, 2016 at 07:15:47PM +0800, Li Jun wrote: > > > With over-current-polarity property added, imx usb over current > > > polarity can be configed to be low or high active, since the default > > > setting value(0) is for active high, so keep this setting for those > > > legacy platforms without this property specified. > > > > > > Signed-off-by: Li Jun <jun.li@nxp.com> > > > --- > > > drivers/usb/chipidea/ci_hdrc_imx.c | 9 +++++++++ > > > drivers/usb/chipidea/ci_hdrc_imx.h | 1 + > > > drivers/usb/chipidea/usbmisc_imx.c | 30 +++++++++++++++++++++++++----- > > > 3 files changed, 35 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c > > > b/drivers/usb/chipidea/ci_hdrc_imx.c > > > index dedc33e..61e712b 100644 > > > --- a/drivers/usb/chipidea/ci_hdrc_imx.c > > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c > > > @@ -140,6 +140,15 @@ static struct imx_usbmisc_data > > *usbmisc_get_init_data(struct device *dev) > > > if (of_find_property(np, "disable-over-current", NULL)) > > > data->disable_oc = 1; > > > > > > + if (!of_property_read_u32(np, "over-current-polarity", &ret)) > > > + data->oc_polarity = ret ? 1 : 0; > > > + else > > > + /* > > > + * Keep the oc polarity setting of legacy > > > + * platforms unchanged. > > > + */ > > > + data->oc_polarity = 1; > > > > We may can't ensure that, since there are lots of i.mx platforms, eg from > > imx27 to imx7. > > > > If the user does not assign oc polarity at DT, we need to read it from > > register. > > I suppose old platforms (other than i.mx6&7) should set either > "over-current-polarity" or "disable-over-current" if want to use > "data->oc_polarity". > The user may use old dtb and the latest kernel, so the introduced DT property should not affect old dtb platforms. > Do you mean read the register value before set "data->oc_polarity" here? yes, if there is a dts property, read it from DT; else, read it from the register. if (!of_property_read_u32(np, "over-current-polarity", &ret)) data->oc_polarity = ret ? 1 : 0; else /* * Keep the oc polarity setting of legacy * platforms unchanged. */ data->oc_polarity = readl(oc_polarity); Then, when you set register for oc polarity, you can depend on the flag. reg = readl(usbmisc->base + data->index * 4); value = data->oc_polarity ? MX6_BM_OVER_CUR_POLARITY : 0; writel(reg | value, usbmisc->base + data->index * 4); > Even with that, as I still can't ensure all i.mx platforms have the same > mapping: > 0 <--> active high. > 1 <--> active low. > How should I set "data->oc_polarity" accordingly? Just keep current register value unchanging if there is no dts property. Peter > Li Jun > > > > Peter > > > + > > > if (of_find_property(np, "external-vbus-divider", NULL)) > > > data->evdo = 1; > > > > > > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h > > > b/drivers/usb/chipidea/ci_hdrc_imx.h > > > index 635717e..409aa5ca8 100644 > > > --- a/drivers/usb/chipidea/ci_hdrc_imx.h > > > +++ b/drivers/usb/chipidea/ci_hdrc_imx.h > > > @@ -17,6 +17,7 @@ struct imx_usbmisc_data { > > > int index; > > > > > > unsigned int disable_oc:1; /* over current detect disabled */ > > > + unsigned int oc_polarity:1; /* over current polarity if oc enabled > > > +*/ > > > unsigned int evdo:1; /* set external vbus divider option */ }; > > > > > > diff --git a/drivers/usb/chipidea/usbmisc_imx.c > > > b/drivers/usb/chipidea/usbmisc_imx.c > > > index ab8b027..193dbe4 100644 > > > --- a/drivers/usb/chipidea/usbmisc_imx.c > > > +++ b/drivers/usb/chipidea/usbmisc_imx.c > > > @@ -56,6 +56,7 @@ > > > > > > #define MX6_BM_NON_BURST_SETTING BIT(1) > > > #define MX6_BM_OVER_CUR_DIS BIT(7) > > > +#define MX6_BM_OVER_CUR_POLARITY BIT(8) > > > #define MX6_BM_WAKEUP_ENABLE BIT(10) > > > #define MX6_BM_ID_WAKEUP BIT(16) > > > #define MX6_BM_VBUS_WAKEUP BIT(17) > > > @@ -266,11 +267,18 @@ static int usbmisc_imx6q_init(struct > > > imx_usbmisc_data *data) > > > > > > spin_lock_irqsave(&usbmisc->lock, flags); > > > > > > + reg = readl(usbmisc->base + data->index * 4); > > > if (data->disable_oc) { > > > - reg = readl(usbmisc->base + data->index * 4); > > > - writel(reg | MX6_BM_OVER_CUR_DIS, > > > - usbmisc->base + data->index * 4); > > > + reg |= MX6_BM_OVER_CUR_DIS; > > > + } else if (data->oc_polarity == 1) { > > > + /* High active */ > > > + reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY); > > > + } else { > > > + /* Low active */ > > > + reg &= ~MX6_BM_OVER_CUR_DIS; > > > + reg |= MX6_BM_OVER_CUR_POLARITY; > > > } > > > + writel(reg, usbmisc->base + data->index * 4); > > > > > > /* SoC non-burst setting */ > > > reg = readl(usbmisc->base + data->index * 4); @@ -365,10 +373,18 @@ > > > static int usbmisc_imx7d_init(struct imx_usbmisc_data *data) > > > return -EINVAL; > > > > > > spin_lock_irqsave(&usbmisc->lock, flags); > > > + reg = readl(usbmisc->base); > > > if (data->disable_oc) { > > > - reg = readl(usbmisc->base); > > > - writel(reg | MX6_BM_OVER_CUR_DIS, usbmisc->base); > > > + reg |= MX6_BM_OVER_CUR_DIS; > > > + } else if (data->oc_polarity == 1) { > > > + /* High active */ > > > + reg &= ~(MX6_BM_OVER_CUR_DIS | MX6_BM_OVER_CUR_POLARITY); > > > + } else { > > > + /* Low active */ > > > + reg &= ~MX6_BM_OVER_CUR_DIS; > > > + reg |= MX6_BM_OVER_CUR_POLARITY; > > > } > > > + writel(reg, usbmisc->base); > > > > > > reg = readl(usbmisc->base + MX7D_USBNC_USB_CTRL2); > > > reg &= ~MX7D_USB_VBUS_WAKEUP_SOURCE_MASK; > > > @@ -492,6 +508,10 @@ static const struct of_device_id > > usbmisc_imx_dt_ids[] = { > > > .compatible = "fsl,imx6ul-usbmisc", > > > .data = &imx6sx_usbmisc_ops, > > > }, > > > + { > > > + .compatible = "fsl,imx7d-usbmisc", > > > + .data = &imx7d_usbmisc_ops, > > > + }, > > > { /* sentinel */ } > > > }; > > > MODULE_DEVICE_TABLE(of, usbmisc_imx_dt_ids); > > > -- > > > 1.9.1 > > > > > > > > > _______________________________________________ > > > linux-arm-kernel mailing list > > > linux-arm-kernel@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > > -- > > > > Best Regards, > > Peter Chen
On Mon, Jul 18, 2016 at 07:15:44PM +0800, Li Jun wrote: > Add compatible string for imx7d-usbmisc. > > Signed-off-by: Li Jun <jun.li@nxp.com> > --- > Documentation/devicetree/bindings/usb/usbmisc-imx.txt | 1 + > 1 file changed, 1 insertion(+) Acked-by: Rob Herring <robh@kernel.org> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/usb/usbmisc-imx.txt b/Documentation/devicetree/bindings/usb/usbmisc-imx.txt index 3539d4e..f1e27fa 100644 --- a/Documentation/devicetree/bindings/usb/usbmisc-imx.txt +++ b/Documentation/devicetree/bindings/usb/usbmisc-imx.txt @@ -6,6 +6,7 @@ Required properties: "fsl,imx6q-usbmisc" for imx6q "fsl,vf610-usbmisc" for Vybrid vf610 "fsl,imx6sx-usbmisc" for imx6sx + "fsl,imx7d-usbmisc" for imx7d - reg: Should contain registers location and length Examples:
Add compatible string for imx7d-usbmisc. Signed-off-by: Li Jun <jun.li@nxp.com> --- Documentation/devicetree/bindings/usb/usbmisc-imx.txt | 1 + 1 file changed, 1 insertion(+)