linux-kernel - Re: [PATCH v4 2/3] usb: chipidea: imx: configure imx for ULPI phy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Wed, 21 Sep 2016 09:54:39 +0200
From:   Fabien Lahoudere <fabien.lahoudere@...labora.co.uk>
To:     Peter Chen <hzpeterchen@...il.com>
Cc:     linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/3] usb: chipidea: imx: configure imx for ULPI phy

Hi,

On 21/09/16 09:12, Peter Chen wrote:
> On Mon, Sep 19, 2016 at 01:45:39PM +0200, Fabien Lahoudere wrote:
>> In order to use ULPI phy with usb host 2 and 3, we need to configure
>> controller register to enable ULPI features.
>>
>> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@...labora.co.uk>
>> ---
>>  drivers/usb/chipidea/ci_hdrc_imx.c |  5 +++++
>>  drivers/usb/chipidea/ci_hdrc_imx.h |  1 +
>>  drivers/usb/chipidea/usbmisc_imx.c | 37 +++++++++++++++++++++++++++++++++++++
>>  3 files changed, 43 insertions(+)
>>
>> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
>> index 0991794..96c0e33 100644
>> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
>> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/pm_runtime.h>
>>  #include <linux/dma-mapping.h>
>>  #include <linux/usb/chipidea.h>
>> +#include <linux/usb/of.h>
>>  #include <linux/clk.h>
>>
>>  #include "ci.h"
>> @@ -146,6 +147,10 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
>>  	if (of_find_property(np, "external-vbus-divider", NULL))
>>  		data->evdo = 1;
>>
>> +
>> +	if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI)
>> +		data->ulpi = 1;
>> +
>>  	return data;
>>  }
>>
>> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
>> index 409aa5ca8..d666c9f 100644
>> --- a/drivers/usb/chipidea/ci_hdrc_imx.h
>> +++ b/drivers/usb/chipidea/ci_hdrc_imx.h
>> @@ -19,6 +19,7 @@ struct imx_usbmisc_data {
>>  	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 */
>> +	unsigned int ulpi:1; /* connected to an ULPI phy */
>>  };
>>
>>  int imx_usbmisc_init(struct imx_usbmisc_data *);
>> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
>> index 9549821..11f51bd 100644
>> --- a/drivers/usb/chipidea/usbmisc_imx.c
>> +++ b/drivers/usb/chipidea/usbmisc_imx.c
>> @@ -46,11 +46,20 @@
>>
>>  #define MX53_USB_OTG_PHY_CTRL_0_OFFSET	0x08
>>  #define MX53_USB_OTG_PHY_CTRL_1_OFFSET	0x0c
>> +#define MX53_USB_CTRL_1_OFFSET	        0x10
>> +#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK (0x11 << 2)
>> +#define MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI BIT(2)
>> +#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK (0x11 << 6)
>> +#define MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI BIT(6)
>>  #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 MX53_USB_CTRL_1_UH2_ULPI_EN	BIT(26)
>> +#define MX53_USB_CTRL_1_UH3_ULPI_EN	BIT(27)
>> +#define MX53_USB_UHx_CTRL_WAKE_UP_EN	BIT(7)
>> +#define MX53_USB_UHx_CTRL_ULPI_INT_EN	BIT(8)
>>  #define MX53_USB_PHYCTRL1_PLLDIV_MASK	0x3
>>  #define MX53_USB_PLL_DIV_24_MHZ		0x01
>>
>> @@ -217,6 +226,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
>>  		}
>>  		break;
>>  	case 2:
>> +		if (data->ulpi) {
>> +			/* set USBH2 into ULPI-mode. */
>> +			reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
>> +			val = readl(reg) | MX53_USB_CTRL_1_UH2_ULPI_EN;
>> +			/* select ULPI clock */
>> +			val &= ~MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_MASK;
>> +			val |= MX53_USB_CTRL_1_H2_XCVR_CLK_SEL_ULPI;
>> +			writel(val, reg);
>> +			/* Set interrupt wake up enable */
>> +			reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
>> +			val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN
>> +				| MX53_USB_UHx_CTRL_ULPI_INT_EN;
>> +			writel(val, reg);
>> +		}
>
> Ok, it seems your 1st patch is just let code structure change be easy
> in this one, and I may give the wrong comments that you had improved
> disable oc code before.
>
> If you agree with me, would you squash these two into one, and send
> again. (delete "reg && val" is necessary).

ok I will squash. =-D

>
> Peter
>>  		if (data->disable_oc) {
>>  			reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET;
>>  			val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
>> @@ -224,6 +247,20 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
>>  		}
>>  		break;
>>  	case 3:
>> +		if (data->ulpi) {
>> +			/* set USBH3 into ULPI-mode. */
>> +			reg = usbmisc->base + MX53_USB_CTRL_1_OFFSET;
>> +			val = readl(reg) | MX53_USB_CTRL_1_UH3_ULPI_EN;
>> +			/* select ULPI clock */
>> +			val &= ~MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_MASK;
>> +			val |= MX53_USB_CTRL_1_H3_XCVR_CLK_SEL_ULPI;
>> +			writel(val, reg);
>> +			/* Set interrupt wake up enable */
>> +			reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
>> +			val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN
>> +				| MX53_USB_UHx_CTRL_ULPI_INT_EN;
>> +			writel(val, reg);
>> +		}
>>  		if (data->disable_oc) {
>>  			reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET;
>>  			val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx;
>> --
>> 2.1.4
>>
>