Re: [RFC] usb-phy-generic: Add support to SMSC USB3315
Hi Peter On Fri, 2017-06-09 at 08:26 +, Peter Chen wrote: > > > > > I have a look at your patches > > (http://www.spinics.net/lists/linux-usb/msg157134.html) > > and I wonder if power sequence is applicable only on hub node? > > No, power sequence can be used for any devices which needs to carry out power > on > before probe. > > > hub are probed too > > late (after ci_ulpi_init). Do you think it is possible to read a power > > sequence in dts > > from ci_ulpi_init? > > > > I think the suitable place for power sequence is at ulpi_of_register, some > ULPI PHYs > need to carry out power on before reading ID. > I do some test to verify that ulpi_of_register is called before hw_phymode_configure but it is not the case. Can you confirm that ULPI phys works with IMX6? It seems that IMX53 and IMX6 use the same chipidea controller and should have the same behaviour. So I wonder if the issue I encounter can be a SOC issue. Thanks > Peter -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb-phy-generic: Add support to SMSC USB3315
Hi Peter I have a look at your patches (http://www.spinics.net/lists/linux-usb/msg157134.html) and I wonder if power sequence is applicable only on hub node? hub are probed too late (after ci_ulpi_init). Do you think it is possible to read a power sequence in dts from ci_ulpi_init? Thanks Fabien -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb-phy-generic: Add support to SMSC USB3315
On Wed, 2017-06-07 at 09:43 +0800, Peter Chen wrote: > On Tue, Jun 06, 2017 at 07:36:10PM +0200, Fabien Lahoudere wrote: > > Hi Peter, > > > > On Tue, 2017-06-06 at 09:55 +0800, Peter Chen wrote: > > > On Mon, Jun 05, 2017 at 11:52:26AM +0200, Fabien Lahoudere wrote: > > > > On Mon, 2017-06-05 at 17:43 +0800, Peter Chen wrote: > > > > > On Mon, Jun 05, 2017 at 10:57:00AM +0200, Fabien Lahoudere wrote: > > > > > > On Fri, 2017-06-02 at 15:00 -0700, Stephen Boyd wrote: > > > > > > > On 05/26, Fabien Lahoudere wrote: > > > > > > > > Hello > > > > > > > > > > > > > > > > I modify ci_hrdc_imx_probe to bypass "data->phy = > > > > > > > > devm_usb_get_phy_by_phandle(&pdev- > > > > > > > > > dev, > > > > > > > > > > > > > > > > "fsl,usbphy", 0);". Everything works as expected and call > > > > > > > > ci_ulpi_init. > > > > > > > > > > > > > > > > The problem is that in ci_ulpi_init, before calling "ci->ulpi = > > > > > > > > ulpi_register_interface(ci- > > > > > > > > > dev, > > > > > > > > > > > > > > > > &ci->ulpi_ops);" (to initialize our phy), > > > > > > > > "hw_phymode_configure(ci);" is called > > > > > > > > which is > > > > > > > > the > > > > > > > > original function that make our system to hang. > > > > > > > > > > > > > > > > Our phy is not initialised before calling > > > > > > > > ulpi_register_interface so I don't > > > > > > > > understand > > > > > > > > how > > > > > > > > the > > > > > > > > phy > > > > > > > > can reply if it is not out of reset state. > > > > > > > > > > > > > > I haven't see any problem in hw_phymode_configure(). What's the > > > > > > > value of ci->platdata->phy_mode? USBPHY_INTERFACE_MODE_ULPI? If > > > > > > > you phy needs to be taken out of reset to reply to the ulpi reads > > > > > > > of the vendor/product ids, then it sounds like you have a similar > > > > > > > situation to what I had. I needed to turn on some regulators to > > > > > > > get those reads to work, otherwise they would fail, but knowing > > > > > > > what needed to be turned on basically meant I needed to probe the > > > > > > > ulpi driver so probing the ids wasn't going to be useful. So on > > > > > > > my device the reads for the ids go through, but they get all > > > > > > > zeroes back, which is actually ok because there aren't any bits > > > > > > > set on my devices anyway. After the reads see 0, we fallback to > > > > > > > DT matching, which avoids the "bring it out of reset/power it on" > > > > > > > sorts of problems entirely. > > > > > > > > > > > > > > > > > > > Yes the phy mode is configured to USBPHY_INTERFACE_MODE_ULPI. > > > > > > Indeed, this phy need to be out of reset to work. For example > > > > > > everything works fine if I > > > > > > call > > > > > > "_ci_usb_phy_init(ci);" before calling "hw_phymode_configure(ci);" > > > > > > This function only init reset GPIO and clock. > > > > > > > > > > > > For information, the original patch I have to fix the issue: > > > > > > > > > > > > diff --git a/drivers/usb/chipidea/core.c > > > > > > b/drivers/usb/chipidea/core.c > > > > > > index 79ad8e9..21aaff1 100644 > > > > > > --- a/drivers/usb/chipidea/core.c > > > > > > +++ b/drivers/usb/chipidea/core.c > > > > > > @@ -391,6 +391,7 @@ static int ci_usb_phy_init(struct ci_hdrc *ci) > > > > > > case USBPHY_INTERFACE_MODE_UTMI: > > > > > > case USBPHY_INTERFACE_MODE_UTMIW: > > > > > > case USBPHY_INTERFACE_MODE_HSIC: > > > > > > + case USBPHY_INTERFACE_MODE_ULPI: >
Re: [RFC] usb-phy-generic: Add support to SMSC USB3315
Hi Peter, On Tue, 2017-06-06 at 09:55 +0800, Peter Chen wrote: > On Mon, Jun 05, 2017 at 11:52:26AM +0200, Fabien Lahoudere wrote: > > On Mon, 2017-06-05 at 17:43 +0800, Peter Chen wrote: > > > On Mon, Jun 05, 2017 at 10:57:00AM +0200, Fabien Lahoudere wrote: > > > > On Fri, 2017-06-02 at 15:00 -0700, Stephen Boyd wrote: > > > > > On 05/26, Fabien Lahoudere wrote: > > > > > > Hello > > > > > > > > > > > > I modify ci_hrdc_imx_probe to bypass "data->phy = > > > > > > devm_usb_get_phy_by_phandle(&pdev- > > > > > > >dev, > > > > > > "fsl,usbphy", 0);". Everything works as expected and call > > > > > > ci_ulpi_init. > > > > > > > > > > > > The problem is that in ci_ulpi_init, before calling "ci->ulpi = > > > > > > ulpi_register_interface(ci- > > > > > > > dev, > > > > > > > > > > > > &ci->ulpi_ops);" (to initialize our phy), > > > > > > "hw_phymode_configure(ci);" is called which is > > > > > > the > > > > > > original function that make our system to hang. > > > > > > > > > > > > Our phy is not initialised before calling ulpi_register_interface > > > > > > so I don't understand > > > > > > how > > > > > > the > > > > > > phy > > > > > > can reply if it is not out of reset state. > > > > > > > > > > I haven't see any problem in hw_phymode_configure(). What's the > > > > > value of ci->platdata->phy_mode? USBPHY_INTERFACE_MODE_ULPI? If > > > > > you phy needs to be taken out of reset to reply to the ulpi reads > > > > > of the vendor/product ids, then it sounds like you have a similar > > > > > situation to what I had. I needed to turn on some regulators to > > > > > get those reads to work, otherwise they would fail, but knowing > > > > > what needed to be turned on basically meant I needed to probe the > > > > > ulpi driver so probing the ids wasn't going to be useful. So on > > > > > my device the reads for the ids go through, but they get all > > > > > zeroes back, which is actually ok because there aren't any bits > > > > > set on my devices anyway. After the reads see 0, we fallback to > > > > > DT matching, which avoids the "bring it out of reset/power it on" > > > > > sorts of problems entirely. > > > > > > > > > > > > > Yes the phy mode is configured to USBPHY_INTERFACE_MODE_ULPI. > > > > Indeed, this phy need to be out of reset to work. For example > > > > everything works fine if I > > > > call > > > > "_ci_usb_phy_init(ci);" before calling "hw_phymode_configure(ci);" > > > > This function only init reset GPIO and clock. > > > > > > > > For information, the original patch I have to fix the issue: > > > > > > > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c > > > > index 79ad8e9..21aaff1 100644 > > > > --- a/drivers/usb/chipidea/core.c > > > > +++ b/drivers/usb/chipidea/core.c > > > > @@ -391,6 +391,7 @@ static int ci_usb_phy_init(struct ci_hdrc *ci) > > > > case USBPHY_INTERFACE_MODE_UTMI: > > > > case USBPHY_INTERFACE_MODE_UTMIW: > > > > case USBPHY_INTERFACE_MODE_HSIC: > > > > + case USBPHY_INTERFACE_MODE_ULPI: > > > > ret = _ci_usb_phy_init(ci); > > > > if (!ret) > > > > hw_wait_phy_stable(); > > > > @@ -398,7 +399,6 @@ static int ci_usb_phy_init(struct ci_hdrc *ci) > > > > return ret; > > > > hw_phymode_configure(ci); > > > > break; > > > > - case USBPHY_INTERFACE_MODE_ULPI: > > > > case USBPHY_INTERFACE_MODE_SERIAL: > > > > hw_phymode_configure(ci); > > > > ret = _ci_usb_phy_init(ci); > > > > -- > > > > > > Currently, the hw_phymode_configure is called twice for ULPI PHY, the > > > two execution are between _ci_usb_phy_init, would you test which one > > > causes hang? If
Re: [RFC] usb-phy-generic: Add support to SMSC USB3315
On Mon, 2017-06-05 at 17:43 +0800, Peter Chen wrote: > On Mon, Jun 05, 2017 at 10:57:00AM +0200, Fabien Lahoudere wrote: > > On Fri, 2017-06-02 at 15:00 -0700, Stephen Boyd wrote: > > > On 05/26, Fabien Lahoudere wrote: > > > > Hello > > > > > > > > I modify ci_hrdc_imx_probe to bypass "data->phy = > > > > devm_usb_get_phy_by_phandle(&pdev->dev, > > > > "fsl,usbphy", 0);". Everything works as expected and call ci_ulpi_init. > > > > > > > > The problem is that in ci_ulpi_init, before calling "ci->ulpi = > > > > ulpi_register_interface(ci- > > > > >dev, > > > > &ci->ulpi_ops);" (to initialize our phy), "hw_phymode_configure(ci);" > > > > is called which is the > > > > original function that make our system to hang. > > > > > > > > Our phy is not initialised before calling ulpi_register_interface so I > > > > don't understand how > > > > the > > > > phy > > > > can reply if it is not out of reset state. > > > > > > I haven't see any problem in hw_phymode_configure(). What's the > > > value of ci->platdata->phy_mode? USBPHY_INTERFACE_MODE_ULPI? If > > > you phy needs to be taken out of reset to reply to the ulpi reads > > > of the vendor/product ids, then it sounds like you have a similar > > > situation to what I had. I needed to turn on some regulators to > > > get those reads to work, otherwise they would fail, but knowing > > > what needed to be turned on basically meant I needed to probe the > > > ulpi driver so probing the ids wasn't going to be useful. So on > > > my device the reads for the ids go through, but they get all > > > zeroes back, which is actually ok because there aren't any bits > > > set on my devices anyway. After the reads see 0, we fallback to > > > DT matching, which avoids the "bring it out of reset/power it on" > > > sorts of problems entirely. > > > > > > > Yes the phy mode is configured to USBPHY_INTERFACE_MODE_ULPI. > > Indeed, this phy need to be out of reset to work. For example everything > > works fine if I call > > "_ci_usb_phy_init(ci);" before calling "hw_phymode_configure(ci);" > > This function only init reset GPIO and clock. > > > > For information, the original patch I have to fix the issue: > > > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c > > index 79ad8e9..21aaff1 100644 > > --- a/drivers/usb/chipidea/core.c > > +++ b/drivers/usb/chipidea/core.c > > @@ -391,6 +391,7 @@ static int ci_usb_phy_init(struct ci_hdrc *ci) > > case USBPHY_INTERFACE_MODE_UTMI: > > case USBPHY_INTERFACE_MODE_UTMIW: > > case USBPHY_INTERFACE_MODE_HSIC: > > + case USBPHY_INTERFACE_MODE_ULPI: > > ret = _ci_usb_phy_init(ci); > > if (!ret) > > hw_wait_phy_stable(); > > @@ -398,7 +399,6 @@ static int ci_usb_phy_init(struct ci_hdrc *ci) > > return ret; > > hw_phymode_configure(ci); > > break; > > - case USBPHY_INTERFACE_MODE_ULPI: > > case USBPHY_INTERFACE_MODE_SERIAL: > > hw_phymode_configure(ci); > > ret = _ci_usb_phy_init(ci); > > -- > > Currently, the hw_phymode_configure is called twice for ULPI PHY, the > two execution are between _ci_usb_phy_init, would you test which one > causes hang? If the second causes hang, you can make a patch for > hw_phymode_configure that if the required PORTSC_PTS is the same > the value in register, do noop. The first one hangs, _ci_usb_phy_init is not called due to hang. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb-phy-generic: Add support to SMSC USB3315
On Fri, 2017-06-02 at 15:00 -0700, Stephen Boyd wrote: > On 05/26, Fabien Lahoudere wrote: > > Hello > > > > I modify ci_hrdc_imx_probe to bypass "data->phy = > > devm_usb_get_phy_by_phandle(&pdev->dev, > > "fsl,usbphy", 0);". Everything works as expected and call ci_ulpi_init. > > > > The problem is that in ci_ulpi_init, before calling "ci->ulpi = > > ulpi_register_interface(ci->dev, > > &ci->ulpi_ops);" (to initialize our phy), "hw_phymode_configure(ci);" is > > called which is the > > original function that make our system to hang. > > > > Our phy is not initialised before calling ulpi_register_interface so I > > don't understand how the > > phy > > can reply if it is not out of reset state. > > I haven't see any problem in hw_phymode_configure(). What's the > value of ci->platdata->phy_mode? USBPHY_INTERFACE_MODE_ULPI? If > you phy needs to be taken out of reset to reply to the ulpi reads > of the vendor/product ids, then it sounds like you have a similar > situation to what I had. I needed to turn on some regulators to > get those reads to work, otherwise they would fail, but knowing > what needed to be turned on basically meant I needed to probe the > ulpi driver so probing the ids wasn't going to be useful. So on > my device the reads for the ids go through, but they get all > zeroes back, which is actually ok because there aren't any bits > set on my devices anyway. After the reads see 0, we fallback to > DT matching, which avoids the "bring it out of reset/power it on" > sorts of problems entirely. > Yes the phy mode is configured to USBPHY_INTERFACE_MODE_ULPI. Indeed, this phy need to be out of reset to work. For example everything works fine if I call "_ci_usb_phy_init(ci);" before calling "hw_phymode_configure(ci);" This function only init reset GPIO and clock. For information, the original patch I have to fix the issue: diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 79ad8e9..21aaff1 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -391,6 +391,7 @@ static int ci_usb_phy_init(struct ci_hdrc *ci) case USBPHY_INTERFACE_MODE_UTMI: case USBPHY_INTERFACE_MODE_UTMIW: case USBPHY_INTERFACE_MODE_HSIC: + case USBPHY_INTERFACE_MODE_ULPI: ret = _ci_usb_phy_init(ci); if (!ret) hw_wait_phy_stable(); @@ -398,7 +399,6 @@ static int ci_usb_phy_init(struct ci_hdrc *ci) return ret; hw_phymode_configure(ci); break; - case USBPHY_INTERFACE_MODE_ULPI: case USBPHY_INTERFACE_MODE_SERIAL: hw_phymode_configure(ci); ret = _ci_usb_phy_init(ci); -- So if some ULPI phys need to be initialised before calling "hw_phymode_configure", is it acceptable if I separate "case USBPHY_INTERFACE_MODE_ULPI:" and add a DT binding ("init_phy_first") to define the order to call both functions? Something like: case USBPHY_INTERFACE_MODE_ULPI: if (ci->platdata->init_phy_first) { ret = _ci_usb_phy_init(ci); if (!ret) hw_wait_phy_stable(); else return ret; } hw_phymode_configure(ci); if (!ci->platdata->init_phy_first) { ret = _ci_usb_phy_init(ci); if (ret) return ret; } break; This approach will not modify current behaviour but allow to initialize phy first on demand. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb-phy-generic: Add support to SMSC USB3315
Hello I modify ci_hrdc_imx_probe to bypass "data->phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0);". Everything works as expected and call ci_ulpi_init. The problem is that in ci_ulpi_init, before calling "ci->ulpi = ulpi_register_interface(ci->dev, &ci->ulpi_ops);" (to initialize our phy), "hw_phymode_configure(ci);" is called which is the original function that make our system to hang. Our phy is not initialised before calling ulpi_register_interface so I don't understand how the phy can reply if it is not out of reset state. The conclusion is that using ulpi_bus to manage our phy doesn't improve and we reach the same issue. I will try to check if we don't do bad things in hw_phymode_configure. If anyone have an idea it is welcome?? Fabien On Thu, 2017-05-25 at 12:36 +0200, Fabien Lahoudere wrote: > On Tue, 2017-05-23 at 14:00 -0700, Stephen Boyd wrote: > > On 05/23, Fabien Lahoudere wrote: > > > Hi, > > > > > > We investigate on the topic and now our device tree look like: > > > > > > in imx53.dtsi: > > > > > > usbh2: usb@53f80400 { > > > compatible = "fsl,imx53-usb", "fsl,imx27-usb"; > > > reg = <0x53f80400 0x0200>; > > > interrupts = <16>; > > > clocks = <&clks IMX5_CLK_USBOH3_GATE>; > > > fsl,usbmisc = <&usbmisc 2>; > > > dr_mode = "host"; > > > status = "disabled"; > > > }; > > > > > > usbmisc: usbmisc@53f80800 { > > > #index-cells = <1>; > > > compatible = "fsl,imx53-usbmisc"; > > > reg = <0x53f80800 0x200>; > > > clocks = <&clks IMX5_CLK_USBOH3_GATE>; > > > }; > > > > > > and in our dts: > > > > > > &usbh2 { > > > pinctrl-names = "default"; > > > pinctrl-0 = <&pinctrl_usbh2>; > > > disable-int60ck; > > > dr_mode = "host"; > > > //fsl,usbphy = <&usbphy2>; > > > vbus-supply = <®_usbh2_vbus>; > > > status = "okay"; > > > ulpi { > > > phy { > > > compatible = "smsc,usb3315-ulpi"; > > > reset-gpios = <&gpio4 4 GPIO_ACTIVE_LOW>; > > > clock-names = "main_clk"; > > > /* > > > * Hardware uses CKO2 at 24MHz at several places. > > > Set the parent > > > * clock of CKO2 to OSC. > > > */ > > > clock-frequency = <2400>; > > > clocks = <&clks IMX5_CLK_CKO2>; > > > assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, > > > <&clks IMX5_CLK_OSC>; > > > assigned-clock-parents = <&clks IMX5_CLK_OSC>; > > > status = "okay"; > > > }; > > > }; > > > }; > > > > > > And we create a basic driver to check what happened: > > > > > > static int smsc_usb3315_phy_probe(struct ulpi *ulpi) > > > { > > > printk(KERN_ERR "Fabien: %s:%d-%s\n", __FILE__, __LINE__, > > > __func__); > > > > > > return 0; > > > } > > > > > > static const struct of_device_id smsc_usb3315_phy_match[] = { > > > { .compatible = "smsc,usb3315-phy", }, > > > { } > > > }; > > > MODULE_DEVICE_TABLE(of, smsc_usb3315_phy_match); > > > > > > static struct ulpi_driver smsc_usb3315_phy_driver = { > > > .probe = smsc_usb3315_phy_probe, > > > .driver = { > > > .name = "smsc_usb3315_phy", > > > .of_match_table = smsc_usb3315_phy_match, > > > }, > > > }; > > > module_ulpi_driver(smsc_usb3315_phy_driver); > > > > > > /*MODULE_ALIAS("platform:usb_phy_generic");*/ > > > MODULE_AUTHOR("GE Healthcare"); > > > MODULE_DESCRIPTION("SMSC USB 3315 ULPI Phy driver"); > > > MODULE_LICENSE("GPL v2"); > > > > > > I checked that the driver is registered by > > > drivers/usb/common/ulpi.c:__ulpi_register_d
Re: [RFC] usb-phy-generic: Add support to SMSC USB3315
On Tue, 2017-05-23 at 14:00 -0700, Stephen Boyd wrote: > On 05/23, Fabien Lahoudere wrote: > > Hi, > > > > We investigate on the topic and now our device tree look like: > > > > in imx53.dtsi: > > > > usbh2: usb@53f80400 { > > compatible = "fsl,imx53-usb", "fsl,imx27-usb"; > > reg = <0x53f80400 0x0200>; > > interrupts = <16>; > > clocks = <&clks IMX5_CLK_USBOH3_GATE>; > > fsl,usbmisc = <&usbmisc 2>; > > dr_mode = "host"; > > status = "disabled"; > > }; > > > > usbmisc: usbmisc@53f80800 { > > #index-cells = <1>; > > compatible = "fsl,imx53-usbmisc"; > > reg = <0x53f80800 0x200>; > > clocks = <&clks IMX5_CLK_USBOH3_GATE>; > > }; > > > > and in our dts: > > > > &usbh2 { > > pinctrl-names = "default"; > > pinctrl-0 = <&pinctrl_usbh2>; > > disable-int60ck; > > dr_mode = "host"; > > //fsl,usbphy = <&usbphy2>; > > vbus-supply = <®_usbh2_vbus>; > > status = "okay"; > > ulpi { > > phy { > > compatible = "smsc,usb3315-ulpi"; > > reset-gpios = <&gpio4 4 GPIO_ACTIVE_LOW>; > > clock-names = "main_clk"; > > /* > > * Hardware uses CKO2 at 24MHz at several places. > > Set the parent > > * clock of CKO2 to OSC. > > */ > > clock-frequency = <2400>; > > clocks = <&clks IMX5_CLK_CKO2>; > > assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, <&clks > > IMX5_CLK_OSC>; > > assigned-clock-parents = <&clks IMX5_CLK_OSC>; > > status = "okay"; > > }; > > }; > > }; > > > > And we create a basic driver to check what happened: > > > > static int smsc_usb3315_phy_probe(struct ulpi *ulpi) > > { > > printk(KERN_ERR "Fabien: %s:%d-%s\n", __FILE__, __LINE__, __func__); > > > > return 0; > > } > > > > static const struct of_device_id smsc_usb3315_phy_match[] = { > > { .compatible = "smsc,usb3315-phy", }, > > { } > > }; > > MODULE_DEVICE_TABLE(of, smsc_usb3315_phy_match); > > > > static struct ulpi_driver smsc_usb3315_phy_driver = { > > .probe = smsc_usb3315_phy_probe, > > .driver = { > > .name = "smsc_usb3315_phy", > > .of_match_table = smsc_usb3315_phy_match, > > }, > > }; > > module_ulpi_driver(smsc_usb3315_phy_driver); > > > > /*MODULE_ALIAS("platform:usb_phy_generic");*/ > > MODULE_AUTHOR("GE Healthcare"); > > MODULE_DESCRIPTION("SMSC USB 3315 ULPI Phy driver"); > > MODULE_LICENSE("GPL v2"); > > > > I checked that the driver is registered by > > drivers/usb/common/ulpi.c:__ulpi_register_driver > > successfully. > > Does the ulpi device have some vendor/product ids associated > with it? The design is made to only fallback to matching the > device to driver based on DT if the ulpi vendor id is 0. > Otherwise, if vendor is non-zero you'll need to have a > ulpi_device_id id table in your ulpi_driver structure. > Hi, Thanks Stephen for your reply. Indeed we have a vendor/product so I modify my code but without effect. After looking at the ulpi source code in the kernel, it seems that I need to call ulpi_register_interface. ci_hdrc_probe should be called to execute the ulpi init. The problem is that we replace "fsl,usbphy = <&usbphy2>;" by an ulpi node but ci_hdrc_imx_probe fail because of "data->phy = devm_usb_get_phy_by_phandle(&pdev->dev, "fsl,usbphy", 0);" So I will try to adapt ci_hdrc_imx_probe to continue phy initialisation if fsl,usbphy is missing. Is it the good way to proceed? Thanks for any advice Fabien -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] usb-phy-generic: Add support to SMSC USB3315
Hi, We investigate on the topic and now our device tree look like: in imx53.dtsi: usbh2: usb@53f80400 { compatible = "fsl,imx53-usb", "fsl,imx27-usb"; reg = <0x53f80400 0x0200>; interrupts = <16>; clocks = <&clks IMX5_CLK_USBOH3_GATE>; fsl,usbmisc = <&usbmisc 2>; dr_mode = "host"; status = "disabled"; }; usbmisc: usbmisc@53f80800 { #index-cells = <1>; compatible = "fsl,imx53-usbmisc"; reg = <0x53f80800 0x200>; clocks = <&clks IMX5_CLK_USBOH3_GATE>; }; and in our dts: &usbh2 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_usbh2>; disable-int60ck; dr_mode = "host"; //fsl,usbphy = <&usbphy2>; vbus-supply = <®_usbh2_vbus>; status = "okay"; ulpi { phy { compatible = "smsc,usb3315-ulpi"; reset-gpios = <&gpio4 4 GPIO_ACTIVE_LOW>; clock-names = "main_clk"; /* * Hardware uses CKO2 at 24MHz at several places. Set the parent * clock of CKO2 to OSC. */ clock-frequency = <2400>; clocks = <&clks IMX5_CLK_CKO2>; assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>; assigned-clock-parents = <&clks IMX5_CLK_OSC>; status = "okay"; }; }; }; And we create a basic driver to check what happened: static int smsc_usb3315_phy_probe(struct ulpi *ulpi) { printk(KERN_ERR "Fabien: %s:%d-%s\n", __FILE__, __LINE__, __func__); return 0; } static const struct of_device_id smsc_usb3315_phy_match[] = { { .compatible = "smsc,usb3315-phy", }, { } }; MODULE_DEVICE_TABLE(of, smsc_usb3315_phy_match); static struct ulpi_driver smsc_usb3315_phy_driver = { .probe = smsc_usb3315_phy_probe, .driver = { .name = "smsc_usb3315_phy", .of_match_table = smsc_usb3315_phy_match, }, }; module_ulpi_driver(smsc_usb3315_phy_driver); /*MODULE_ALIAS("platform:usb_phy_generic");*/ MODULE_AUTHOR("GE Healthcare"); MODULE_DESCRIPTION("SMSC USB 3315 ULPI Phy driver"); MODULE_LICENSE("GPL v2"); I checked that the driver is registered by drivers/usb/common/ulpi.c:__ulpi_register_driver successfully. But our probe function (smsc_usb3315_phy_probe) is never called. Our understanding is that we need to use the ulpi_bus instead of devm_usb_get_phy_by_phandle(&pdev- >dev, "fsl,usbphy", 0); in drivers/usb/chipidea/ci_hdrc_imx.c. Is our approach good? How can we use this bus from our controller probe function ? Thanks Fabien On Thu, 2017-04-20 at 16:50 +0800, Peter Chen wrote: > On Wed, Apr 19, 2017 at 06:14:13AM +, Peter Senna Tschudin wrote: > > We need the SMSC USB3315 clock and regulator to always be initialized. > > We also need the PHY driver to take the PHY out of reset. This patch > > extends the existing USB generic nop phy driver to include a new > > initialization path. > > > > A new compatible string "smsc,usb3315" is used to decide which > > initialization path to use. > > > > Hi Peter, > > This is an ULPI PHY, so it is not suitable using generic USB PHY. > Taking a look of drivers/phy/phy-qcom-usb-hs.c, you may have some > clues. > > Peter > > > CC: Peter Chen > > CC: Stephen Boyd > > CC: Fabien Lahoudere > > Signed-off-by: Peter Senna Tschudin > > --- > > > > This is a follow-up of previous discussion: > > https://www.spinics.net/lists/linux-usb/msg146680.html > > > > drivers/usb/phy/phy-generic.c | 33 + > > drivers/usb/phy/phy-generic.h | 1 + > > 2 files changed, 30 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c > > index 89d6e7a..6ea9ce4 100644 > > --- a/drivers/usb/phy/phy-generic.c > > +++ b/drivers/usb/phy/phy-generic.c > > @@ -151,6 +151,9 @@ int usb_gen_phy_init(struct usb_phy *phy) > > struct usb_phy_generic *nop = dev_get_drvdata(phy->dev); > > int ret; > > > > + if (nop->init_done) > > + return 0; > > + > > if (!IS_ERR(nop->vcc)) { > > if (regulator_enable(nop->vcc)) > > dev_err(phy-&
[PATCH v6 0/2] Add USB configuration for imx53
Changes in V2: - Patches sent to early with bad contents Changes in V3: - Change subject - Split "configure imx for ULPI phy" for disable-oc code Changes in V4: - Fix "Change switch order" commit message - Indent switch/case (set case on the same column as switch) - Remove useless test in "Change switch order" Changes in V5: - Squash "Change switch order" and "configure imx for ULPI phy" - Add device tree binding documentation Changes in v6: - Remove dt binding because we can disable the feature by using an existing binding Fabien Lahoudere (2): usb: chipidea: imx: configure imx for ULPI phy usb: chipidea: imx: Disable internal 60Mhz clock with ULPI PHY drivers/usb/chipidea/ci_hdrc_imx.c | 4 ++ drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 86 +++--- 3 files changed, 77 insertions(+), 14 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/2] usb: chipidea: imx: configure imx for ULPI phy
In order to use ULPI phy with usb host 2 and 3, we need to configure controller register to enable ULPI features. Each USB controller have different behaviour, so in order to avoid to have several "swicth(data->index)" and lock/unlock, we prefer to get the index switch and then test for features if they exist for this index. This patch also remove useless test of reg and val. Those two values cannot be NULL. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/ci_hdrc_imx.c | 4 ++ drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 75 +++--- 3 files changed, 66 insertions(+), 14 deletions(-) diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index 0991794..5f4a815 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include "ci.h" @@ -146,6 +147,9 @@ 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 20d02a5..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_H1BIT(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_ENBIT(26) +#define MX53_USB_CTRL_1_UH3_ULPI_ENBIT(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_MHZ0x01 @@ -199,31 +208,69 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val |= MX53_USB_PLL_DIV_24_MHZ; writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); - if (data->disable_oc) { - spin_lock_irqsave(&usbmisc->lock, flags); - switch (data->index) { - case 0: + spin_lock_irqsave(&usbmisc->lock, flags); + + switch (data->index) { + case 0: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; - break; - case 1: + writel(val, reg); + } + break; + case 1: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; - break; - case 2: + writel(val, reg); + } + 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); + } +
[PATCH v6 2/2] usb: chipidea: imx: Disable internal 60Mhz clock with ULPI PHY
The internal 60Mhz clock for host2 and host3 are useless in ULPI phy mode, so we disable it when configuring ULPI PHY node for those host. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/usbmisc_imx.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 11f51bd..e77a4ed 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -53,6 +53,9 @@ #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_USB_CLKONOFF_CTRL_OFFSET 0x24 +#define MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF BIT(21) +#define MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF BIT(22) #define MX53_BM_OVER_CUR_DIS_H1BIT(5) #define MX53_BM_OVER_CUR_DIS_OTG BIT(8) #define MX53_BM_OVER_CUR_DIS_UHx BIT(30) @@ -239,6 +242,10 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); + /* Disable internal 60Mhz clock */ + reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF; + writel(val, reg); } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; @@ -260,6 +267,10 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); + /* Disable internal 60Mhz clock */ + reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF; + writel(val, reg); } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] usb: chipidea: imx: Add binding to disable USB 60Mhz clock
Hi, On 23/09/16 21:47, Rob Herring wrote: On Wed, Sep 21, 2016 at 11:07:07AM +0200, Fabien Lahoudere wrote: This binding allow to disable the internal 60Mhz clock for USB host2 or host3. Signed-off-by: Fabien Lahoudere --- Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 1 + drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++ drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 13 + 4 files changed, 17 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt index 0e03344..f83da66 100644 --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt @@ -84,6 +84,7 @@ i.mx specific properties - over-current-active-high: over current signal polarity is high active, typically over current signal polarity is low active. - external-vbus-divider: enables off-chip resistor divider for Vbus +- disable-int60ck: disable internal 60MHz clock for usb host2 or host3 on imx53 Doesn't this depend on something else like the type of phy connected? If not, when can you do this or not? We can disable it in OTG mode and with ULPI phy and Sascha Hauer think we can do it without dt binding for example based on PHY mode. So I will remove the binding and just disable clock if ULPI is selected. Rob Fabien -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] usb: chipidea: imx: Add binding to disable USB 60Mhz clock
Hi, On 26/09/16 10:18, Sascha Hauer wrote: On Wed, Sep 21, 2016 at 11:07:07AM +0200, Fabien Lahoudere wrote: This binding allow to disable the internal 60Mhz clock for USB host2 or host3. Signed-off-by: Fabien Lahoudere --- Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 1 + drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++ drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 13 + 4 files changed, 17 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt index 0e03344..f83da66 100644 --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt @@ -84,6 +84,7 @@ i.mx specific properties - over-current-active-high: over current signal polarity is high active, typically over current signal polarity is low active. - external-vbus-divider: enables off-chip resistor divider for Vbus +- disable-int60ck: disable internal 60MHz clock for usb host2 or host3 on imx53 Why do we need a binding for this? I would assume the driver should know whether this clock is in use or not. If it doesn't that's a problem we should solve. Yes you are right because we can disable this clock for OTG and with ULPI PHY. I think that it will be better to have a dt binding but if it is useless I can remove it and disable clock when ULPI mode is enabled. Sascha Fabien -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/2] usb: chipidea: imx: Add binding to disable USB 60Mhz clock
This binding allow to disable the internal 60Mhz clock for USB host2 or host3. Signed-off-by: Fabien Lahoudere --- Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 1 + drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++ drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 13 + 4 files changed, 17 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt index 0e03344..f83da66 100644 --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt @@ -84,6 +84,7 @@ i.mx specific properties - over-current-active-high: over current signal polarity is high active, typically over current signal polarity is low active. - external-vbus-divider: enables off-chip resistor divider for Vbus +- disable-int60ck: disable internal 60MHz clock for usb host2 or host3 on imx53 Example: diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index 96c0e33..89a9d98 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -147,6 +147,8 @@ 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_find_property(np, "disable-int60ck", NULL)) + data->disable_int60ck = 1; if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) data->ulpi = 1; diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h index d666c9f..43bafae 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.h +++ b/drivers/usb/chipidea/ci_hdrc_imx.h @@ -20,6 +20,7 @@ struct imx_usbmisc_data { 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 */ + unsigned int disable_int60ck:1; /* disable 60 MHZ clock */ }; 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 11f51bd..a781f87 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -53,6 +53,9 @@ #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_USB_CLKONOFF_CTRL_OFFSET 0x24 +#define MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF BIT(21) +#define MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF BIT(22) #define MX53_BM_OVER_CUR_DIS_H1BIT(5) #define MX53_BM_OVER_CUR_DIS_OTG BIT(8) #define MX53_BM_OVER_CUR_DIS_UHx BIT(30) @@ -240,6 +243,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); } + if (data->disable_int60ck) { + reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF; + writel(val, reg); + } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; @@ -261,6 +269,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); } + if (data->disable_int60ck) { + reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF; + 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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/2] usb: chipidea: imx: configure imx for ULPI phy
In order to use ULPI phy with usb host 2 and 3, we need to configure controller register to enable ULPI features. Each USB controller have different behaviour, so in order to avoid to have several "swicth(data->index)" and lock/unlock, we prefer to get the index switch and then test for features if they exist for this index. This patch also remove useless test of reg and val. Those two values cannot be NULL. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/ci_hdrc_imx.c | 5 +++ drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 75 +++--- 3 files changed, 67 insertions(+), 14 deletions(-) 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 #include #include +#include #include #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 20d02a5..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_H1BIT(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_ENBIT(26) +#define MX53_USB_CTRL_1_UH3_ULPI_ENBIT(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_MHZ0x01 @@ -199,31 +208,69 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val |= MX53_USB_PLL_DIV_24_MHZ; writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); - if (data->disable_oc) { - spin_lock_irqsave(&usbmisc->lock, flags); - switch (data->index) { - case 0: + spin_lock_irqsave(&usbmisc->lock, flags); + + switch (data->index) { + case 0: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; - break; - case 1: + writel(val, reg); + } + break; + case 1: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; - break; - case 2: + writel(val, reg); + } + 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); + } +
[PATCH v5 0/2] usb: chipidea: imx: Add USB configuration for imx53
Changes in V2: - Patches sent to early with bad contents Changes in V3: - Change subject - Split "configure imx for ULPI phy" for disable-oc code Changes in V4: - Fix "Change switch order" commit message - Indent switch/case (set case on the same column as switch) - Remove useless test in "Change switch order" Changes in V5: - Squash "Change switch order" and "configure imx for ULPI phy" - Add device tree binding documentation Fabien Lahoudere (2): usb: chipidea: imx: configure imx for ULPI phy usb: chipidea: imx: Add binding to disable USB 60Mhz clock .../devicetree/bindings/usb/ci-hdrc-usb2.txt | 1 + drivers/usb/chipidea/ci_hdrc_imx.c | 7 ++ drivers/usb/chipidea/ci_hdrc_imx.h | 2 + drivers/usb/chipidea/usbmisc_imx.c | 88 ++ 4 files changed, 84 insertions(+), 14 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 --- 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 #include #include +#include #include #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_H1BIT(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_ENBIT(26) +#define MX53_USB_CTRL_1_UH3_ULPI_ENBIT(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_MHZ0x01 @@ -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_
Re: [PATCH v4 1/3] usb: chipidea: imx: Change switch order
Hi, On 21/09/16 08:57, Peter Chen wrote: On Mon, Sep 19, 2016 at 01:45:38PM +0200, Fabien Lahoudere wrote: Each USB controller have different behaviour, so in order to avoid to have several "swicth(data->index)" and lock/unlock, we prefer to get the index switch and then test for features if they exist for this index. This patch also remove useless test of reg and val. Those two values cannot be NULL. Sorry, I can't see the benefits of this change. Considering certain controller doesn't have oc feature, with current code it only needs one comparison (flag disable_oc), but with your changes, it needs two comparisons. The benefits are visible with next patches only but you ask me to split it, so I do. Thanks Fabien Peter Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/usbmisc_imx.c | 38 -- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 20d02a5..9549821 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -199,31 +199,41 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val |= MX53_USB_PLL_DIV_24_MHZ; writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); - if (data->disable_oc) { - spin_lock_irqsave(&usbmisc->lock, flags); - switch (data->index) { - case 0: + spin_lock_irqsave(&usbmisc->lock, flags); + + switch (data->index) { + case 0: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; - break; - case 1: + writel(val, reg); + } + break; + case 1: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; - break; - case 2: + writel(val, reg); + } + break; + case 2: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; - break; - case 3: + writel(val, reg); + } + break; + case 3: + if (data->disable_oc) { 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); + } + break; } + spin_unlock_irqrestore(&usbmisc->lock, flags); + return 0; } -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/3] usb: chipidea: imx: Change switch order
Each USB controller have different behaviour, so in order to avoid to have several "swicth(data->index)" and lock/unlock, we prefer to get the index switch and then test for features if they exist for this index. This patch also remove useless test of reg and val. Those two values cannot be NULL. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/usbmisc_imx.c | 38 -- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 20d02a5..9549821 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -199,31 +199,41 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val |= MX53_USB_PLL_DIV_24_MHZ; writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); - if (data->disable_oc) { - spin_lock_irqsave(&usbmisc->lock, flags); - switch (data->index) { - case 0: + spin_lock_irqsave(&usbmisc->lock, flags); + + switch (data->index) { + case 0: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; - break; - case 1: + writel(val, reg); + } + break; + case 1: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; - break; - case 2: + writel(val, reg); + } + break; + case 2: + if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; - break; - case 3: + writel(val, reg); + } + break; + case 3: + if (data->disable_oc) { 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); + } + break; } + spin_unlock_irqrestore(&usbmisc->lock, flags); + return 0; } -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/3] usb: chipidea: imx: configure imx for ULPI phy
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 --- 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 #include #include +#include #include #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_H1BIT(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_ENBIT(26) +#define MX53_USB_CTRL_1_UH3_ULPI_ENBIT(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_MHZ0x01 @@ -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); + } 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 -- To unsubscribe from this li
[PATCH v4 3/3] usb: chipidea: imx: Add binding to disable USB 60Mhz clock
This binding allow to disable the internal 60Mhz clock for USB host2 and host3. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++ drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 13 + 3 files changed, 16 insertions(+) diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index 96c0e33..89a9d98 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -147,6 +147,8 @@ 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_find_property(np, "disable-int60ck", NULL)) + data->disable_int60ck = 1; if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) data->ulpi = 1; diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h index d666c9f..43bafae 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.h +++ b/drivers/usb/chipidea/ci_hdrc_imx.h @@ -20,6 +20,7 @@ struct imx_usbmisc_data { 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 */ + unsigned int disable_int60ck:1; /* disable 60 MHZ clock */ }; 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 11f51bd..a781f87 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -53,6 +53,9 @@ #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_USB_CLKONOFF_CTRL_OFFSET 0x24 +#define MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF BIT(21) +#define MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF BIT(22) #define MX53_BM_OVER_CUR_DIS_H1BIT(5) #define MX53_BM_OVER_CUR_DIS_OTG BIT(8) #define MX53_BM_OVER_CUR_DIS_UHx BIT(30) @@ -240,6 +243,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); } + if (data->disable_int60ck) { + reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF; + writel(val, reg); + } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; @@ -261,6 +269,11 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); } + if (data->disable_int60ck) { + reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF; + 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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/3] usb: chipidea: imx: Add USB configuration for imx53
Changes in V2: - Patches sent to early with bad contents Changes in V3: - Change subject - Split "configure imx for ULPI phy" for disable-oc code Changes in V4: - Fix "Change switch order" commit message - Indent switch/case (set case on the same column as switch) - Remove useless test in "Change switch order" Fabien Lahoudere (3): usb: chipidea: imx: Change switch order usb: chipidea: imx: configure imx for ULPI phy usb: chipidea: imx: Add binding to disable USB 60Mhz clock drivers/usb/chipidea/ci_hdrc_imx.c | 7 +++ drivers/usb/chipidea/ci_hdrc_imx.h | 2 + drivers/usb/chipidea/usbmisc_imx.c | 88 -- 3 files changed, 83 insertions(+), 14 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/15] usb: chipidea: imx: Change switch order
Please forget this submission. On 19/09/16 12:18, Fabien Lahoudere wrote: Each USB controller have different behaviour, so in order to avoid to have several "swicth(data->index)" and lock/unlock, we prefer to get the index and then test for features if they exist for this index. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/usbmisc_imx.c | 44 +- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 20d02a5..ed324d1 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -199,31 +199,45 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val |= MX53_USB_PLL_DIV_24_MHZ; writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); - if (data->disable_oc) { - spin_lock_irqsave(&usbmisc->lock, flags); - switch (data->index) { + spin_lock_irqsave(&usbmisc->lock, flags); + + switch (data->index) { case 0: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (reg && val) + writel(val, reg); + } break; case 1: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (reg && val) + writel(val, reg); + } break; case 2: - reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); + } break; case 3: - reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); + } break; - } - if (reg && val) - writel(val, reg); - spin_unlock_irqrestore(&usbmisc->lock, flags); } + spin_unlock_irqrestore(&usbmisc->lock, flags); + return 0; } -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/3] usb: chipidea: imx: Change switch order
Each USB controller have different behaviour, so in order to avoid to have several "swicth(data->index)" and lock/unlock, we prefer to get the index and then test for features if they exist for this index. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/usbmisc_imx.c | 44 +- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 20d02a5..ed324d1 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -199,31 +199,45 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val |= MX53_USB_PLL_DIV_24_MHZ; writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); - if (data->disable_oc) { - spin_lock_irqsave(&usbmisc->lock, flags); - switch (data->index) { + spin_lock_irqsave(&usbmisc->lock, flags); + + switch (data->index) { case 0: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (reg && val) + writel(val, reg); + } break; case 1: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (reg && val) + writel(val, reg); + } break; case 2: - reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); + } break; case 3: - reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); + } break; - } - if (reg && val) - writel(val, reg); - spin_unlock_irqrestore(&usbmisc->lock, flags); } + spin_unlock_irqrestore(&usbmisc->lock, flags); + return 0; } -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/3] usb: chipidea: imx: configure imx for ULPI phy
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 --- 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 #include #include +#include #include #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 ed324d1..5472900 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_H1BIT(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_ENBIT(26) +#define MX53_USB_CTRL_1_UH3_ULPI_ENBIT(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_MHZ0x01 @@ -219,6 +228,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); + } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; @@ -227,6 +250,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
[PATCH v3 3/3] usb: chipidea: imx: Add binding to disable USB 60Mhz clock
This binding allow to disable the internal 60Mhz clock for USB host2 and host3. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++ drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 17 + 3 files changed, 20 insertions(+) diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index 96c0e33..89a9d98 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -147,6 +147,8 @@ 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_find_property(np, "disable-int60ck", NULL)) + data->disable_int60ck = 1; if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) data->ulpi = 1; diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h index d666c9f..43bafae 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.h +++ b/drivers/usb/chipidea/ci_hdrc_imx.h @@ -20,6 +20,7 @@ struct imx_usbmisc_data { 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 */ + unsigned int disable_int60ck:1; /* disable 60 MHZ clock */ }; 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 5472900..bffc667 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -53,6 +53,9 @@ #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_USB_CLKONOFF_CTRL_OFFSET 0x24 +#define MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF BIT(21) +#define MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF BIT(22) #define MX53_BM_OVER_CUR_DIS_H1BIT(5) #define MX53_BM_OVER_CUR_DIS_OTG BIT(8) #define MX53_BM_OVER_CUR_DIS_UHx BIT(30) @@ -242,6 +245,13 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); } + if (data->disable_int60ck) { + reg = usbmisc->base + + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) + | MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF; + writel(val, reg); + } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; @@ -264,6 +274,13 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); } + if (data->disable_int60ck) { + reg = usbmisc->base + + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) + | MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF; + 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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/15] usb: chipidea: imx: Change switch order
Each USB controller have different behaviour, so in order to avoid to have several "swicth(data->index)" and lock/unlock, we prefer to get the index and then test for features if they exist for this index. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/usbmisc_imx.c | 44 +- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 20d02a5..ed324d1 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -199,31 +199,45 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val |= MX53_USB_PLL_DIV_24_MHZ; writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); - if (data->disable_oc) { - spin_lock_irqsave(&usbmisc->lock, flags); - switch (data->index) { + spin_lock_irqsave(&usbmisc->lock, flags); + + switch (data->index) { case 0: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (reg && val) + writel(val, reg); + } break; case 1: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (reg && val) + writel(val, reg); + } break; case 2: - reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); + } break; case 3: - reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + if (reg && val) + writel(val, reg); + } break; - } - if (reg && val) - writel(val, reg); - spin_unlock_irqrestore(&usbmisc->lock, flags); } + spin_unlock_irqrestore(&usbmisc->lock, flags); + return 0; } -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ULPI phy issue with
Hi, I have splitted the patch to submit only imx53 configuration. I will follow Stephen submission to test it with my board and add needed code to solve my clock issue. Thanks Fabien On 14/09/16 12:14, Peter Chen wrote: On Tue, Sep 13, 2016 at 08:05:32PM +0200, Fabien Lahoudere wrote: Hi Peter, This is our device tree imx-ppd.dts: I may know the reason why you meet hang at current flow, you are using generic phy driver, and the PHY clock is enabled at phy_init which is called later than setting portsc.pts. The current flow to enable ULPI phy is like below: This explains why the patch works if USBPHY_INTERFACE_MODE_ULPI works as USBPHY_INTERFACE_MODE_UTMI. because _ci_usb_phy_init(ci) initialise clock and generator. Yes. 1. Enable ULPI and choose its clock select at usbmisc, which you have already done. 2. Enable the input clock for ULPI This is done by _ci_usb_phy_init(ci); and this function only do this so I think we should have in ci_usb_phy_init(ci);: case USBPHY_INTERFACE_MODE_ULPI: ret = _ci_usb_phy_init(ci); if (!ret) hw_wait_phy_stable(); else return ret; 3. set portsc.pts hw_phymode_configure(ci); ci_ulpi_phy_init(ci); // to init ULPI specific config once portsc.pts is enabled break; You may need to have a ULPI PHY driver which do some power sequence (clock, regulator, etc) before setting portsc.pts and visit ULPI register. This is already done by _ci_usb_phy_init(ci); In conclusion, I think USBPHY_INTERFACE_MODE_ULPI should work as USBPHY_INTERFACE_MODE_UTMI but with an extra function to visit ULPI register (ci_ulpi_phy_init(ci);). Am I wrong? Currently, you are using generic PHY driver which does not intends for ULPI PHY uses, looks at usb/phy/phy-upi.c and usb/common/ulpi.c, both need to read id at its initialization. I am also a little confused how Stephen Boyd's ULPI driver for qualcomm platform, I will cc on discussion. As you ask me earlier maybe he init clock from bootloader. Yes, his PHY input clock is always on. Stephen boyd's ulpi support at chipidea is the correct way. For your case, you can create a generic ULPI PHY driver, and registered it on ulpi bus, but you still need power sequence for your case. I am afraid you may use your hack temporarily at local, once Stephen's patch and power sequence patch set are queued, you can submit your changes. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] ARM: imx53: Add binding to disable USB 60Mhz clock
This binding allow to disable the internal 60Mhz clock for USB host2 and host3. Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/ci_hdrc_imx.c | 2 ++ drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 17 + 3 files changed, 20 insertions(+) diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index 96c0e33..89a9d98 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -147,6 +147,8 @@ 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_find_property(np, "disable-int60ck", NULL)) + data->disable_int60ck = 1; if (of_usb_get_phy_mode(np) == USBPHY_INTERFACE_MODE_ULPI) data->ulpi = 1; diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h index d666c9f..43bafae 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.h +++ b/drivers/usb/chipidea/ci_hdrc_imx.h @@ -20,6 +20,7 @@ struct imx_usbmisc_data { 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 */ + unsigned int disable_int60ck:1; /* disable 60 MHZ clock */ }; 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 5472900..bffc667 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -53,6 +53,9 @@ #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_USB_CLKONOFF_CTRL_OFFSET 0x24 +#define MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF BIT(21) +#define MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF BIT(22) #define MX53_BM_OVER_CUR_DIS_H1BIT(5) #define MX53_BM_OVER_CUR_DIS_OTG BIT(8) #define MX53_BM_OVER_CUR_DIS_UHx BIT(30) @@ -242,6 +245,13 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); } + if (data->disable_int60ck) { + reg = usbmisc->base + + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) + | MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF; + writel(val, reg); + } if (data->disable_oc) { reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; @@ -264,6 +274,13 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) | MX53_USB_UHx_CTRL_ULPI_INT_EN; writel(val, reg); } + if (data->disable_int60ck) { + reg = usbmisc->base + + MX53_USB_CLKONOFF_CTRL_OFFSET; + val = readl(reg) + | MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF; + 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 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] ARM: imx53: configure imx for ULPI phy
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 --- drivers/usb/chipidea/ci_hdrc_imx.c | 5 +++ drivers/usb/chipidea/ci_hdrc_imx.h | 1 + drivers/usb/chipidea/usbmisc_imx.c | 81 +++--- 3 files changed, 72 insertions(+), 15 deletions(-) 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 #include #include +#include #include #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 20d02a5..5472900 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_H1BIT(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_ENBIT(26) +#define MX53_USB_CTRL_1_UH3_ULPI_ENBIT(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_MHZ0x01 @@ -199,31 +208,73 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) val |= MX53_USB_PLL_DIV_24_MHZ; writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); - if (data->disable_oc) { - spin_lock_irqsave(&usbmisc->lock, flags); - switch (data->index) { + spin_lock_irqsave(&usbmisc->lock, flags); + + switch (data->index) { case 0: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; + if (reg && val) + writel(val, reg); + } break; case 1: - reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (data->disable_oc) { + reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; + val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; + if (reg && val) + writel(val, reg); + } break; case 2: - reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; - val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; + 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); +
Re: ULPI phy issue with
Hi Peter, First thank you for your help. On 13/09/16 08:52, Peter Chen wrote: On Mon, Sep 12, 2016 at 12:06:10PM +0200, Fabien Lahoudere wrote: On 12/09/16 11:46, Peter Chen wrote: On Mon, Sep 12, 2016 at 11:13:01AM +0200, Fabien Lahoudere wrote: Hi, Yes, please send the patch and tell me which dts and ULPI code you are using. We need to understand why the system is hang when configure the PHY mode at portsc for your case, afaik, the PHY mode needs to be configured as ULPI mode before using ULPI view port to access ULPI PHY. Patch have been sent "usb: imx53 - Allow to configure ULPI mode for usb host 2 and 3", in reply to this thread. Besides, do you enable this ULPI PHY at u-boot? Yes, I configure imx53 in linux kernel in order to use ULPI mode. I remind you that the patch works fine with my device. I know you are using it at kernel. I just want to know if you enable it at bootloader, besides, I need to know more about your platform since the system hang usually dues to without PHY clock, so would you send platform stuffs like dts (which dts), etc? No USB are not used in uboot. This is our device tree imx-ppd.dts: I may know the reason why you meet hang at current flow, you are using generic phy driver, and the PHY clock is enabled at phy_init which is called later than setting portsc.pts. The current flow to enable ULPI phy is like below: This explains why the patch works if USBPHY_INTERFACE_MODE_ULPI works as USBPHY_INTERFACE_MODE_UTMI. because _ci_usb_phy_init(ci) initialise clock and generator. 1. Enable ULPI and choose its clock select at usbmisc, which you have already done. 2. Enable the input clock for ULPI This is done by _ci_usb_phy_init(ci); and this function only do this so I think we should have in ci_usb_phy_init(ci);: case USBPHY_INTERFACE_MODE_ULPI: ret = _ci_usb_phy_init(ci); if (!ret) hw_wait_phy_stable(); else return ret; 3. set portsc.pts hw_phymode_configure(ci); ci_ulpi_phy_init(ci); // to init ULPI specific config once portsc.pts is enabled break; You may need to have a ULPI PHY driver which do some power sequence (clock, regulator, etc) before setting portsc.pts and visit ULPI register. This is already done by _ci_usb_phy_init(ci); In conclusion, I think USBPHY_INTERFACE_MODE_ULPI should work as USBPHY_INTERFACE_MODE_UTMI but with an extra function to visit ULPI register (ci_ulpi_phy_init(ci);). Am I wrong? I am also a little confused how Stephen Boyd's ULPI driver for qualcomm platform, I will cc on discussion. As you ask me earlier maybe he init clock from bootloader. BR, Fabien Peter /* * Copyright 2014 General Electric Company * * The code contained herein is licensed under the GNU General Public * License. You may obtain a copy of the GNU General Public License * Version 2 or later at the following locations: * * http://www.opensource.org/licenses/gpl-license.html * http://www.gnu.org/copyleft/gpl.html */ /dts-v1/; #include "imx53.dtsi" / { model = "Freescale i.MX53 CPUV0 PPD rev6"; compatible = "fsl,imx53-cpuvo", "fsl,imx53"; aliases { spi0 = &cspi; spi1 = &ecspi1; spi2 = &ecspi2; }; chosen { stdout-path = "&uart1:115200n8"; }; memory { reg = <0x7000 0x2000>, <0xb000 0x2000>; }; cko2_11M: sgtl_clock_cko2 { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <11289600>; }; sgtlsound: sound { compatible = "fsl,imx53-cpuvo-sgtl5000", "fsl,imx-audio-sgtl5000"; model = "imx53-cpuvo-sgtl5000"; ssi-controller = <&ssi2>; audio-codec = <&sgtl5000>; audio-routing = "MIC_IN", "Mic Jack", "Mic Jack", "Mic Bias", "Headphone Jack", "HP_OUT"; mux-int-port = <2>; mux-ext-port = <6>; status = "okay"; }; reg_sgtl5k: regulator-sgtl5k { compatible = "regulator-fixed"; regulator-name = "regulator-sgtl5k"; regulator-min-microvolt = <330>; regulator-max-microvolt = <330>; regulator-always-on; }; reg_usb_otg_vbus: regulator-usb-otg-vbus { compatible = "regulator-fixed";
Re: ULPI phy issue with
On 12/09/16 11:46, Peter Chen wrote: On Mon, Sep 12, 2016 at 11:13:01AM +0200, Fabien Lahoudere wrote: Hi, Yes, please send the patch and tell me which dts and ULPI code you are using. We need to understand why the system is hang when configure the PHY mode at portsc for your case, afaik, the PHY mode needs to be configured as ULPI mode before using ULPI view port to access ULPI PHY. Patch have been sent "usb: imx53 - Allow to configure ULPI mode for usb host 2 and 3", in reply to this thread. Besides, do you enable this ULPI PHY at u-boot? Yes, I configure imx53 in linux kernel in order to use ULPI mode. I remind you that the patch works fine with my device. I know you are using it at kernel. I just want to know if you enable it at bootloader, besides, I need to know more about your platform since the system hang usually dues to without PHY clock, so would you send platform stuffs like dts (which dts), etc? No USB are not used in uboot. This is our device tree imx-ppd.dts: /* * Copyright 2014 General Electric Company * * The code contained herein is licensed under the GNU General Public * License. You may obtain a copy of the GNU General Public License * Version 2 or later at the following locations: * * http://www.opensource.org/licenses/gpl-license.html * http://www.gnu.org/copyleft/gpl.html */ /dts-v1/; #include "imx53.dtsi" / { model = "Freescale i.MX53 CPUV0 PPD rev6"; compatible = "fsl,imx53-cpuvo", "fsl,imx53"; aliases { spi0 = &cspi; spi1 = &ecspi1; spi2 = &ecspi2; }; chosen { stdout-path = "&uart1:115200n8"; }; memory { reg = <0x7000 0x2000>, <0xb000 0x2000>; }; cko2_11M: sgtl_clock_cko2 { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <11289600>; }; sgtlsound: sound { compatible = "fsl,imx53-cpuvo-sgtl5000", "fsl,imx-audio-sgtl5000"; model = "imx53-cpuvo-sgtl5000"; ssi-controller = <&ssi2>; audio-codec = <&sgtl5000>; audio-routing = "MIC_IN", "Mic Jack", "Mic Jack", "Mic Bias", "Headphone Jack", "HP_OUT"; mux-int-port = <2>; mux-ext-port = <6>; status = "okay"; }; reg_sgtl5k: regulator-sgtl5k { compatible = "regulator-fixed"; regulator-name = "regulator-sgtl5k"; regulator-min-microvolt = <330>; regulator-max-microvolt = <330>; regulator-always-on; }; reg_usb_otg_vbus: regulator-usb-otg-vbus { compatible = "regulator-fixed"; regulator-name = "usbotg_vbus"; regulator-min-microvolt = <500>; regulator-max-microvolt = <500>; pinctrl-0 = <&pinctrl_usb_otg_vbus>; gpio = <&gpio4 15 GPIO_ACTIVE_HIGH>; enable-active-high; regulator-always-on; }; reg_usb_vbus: regulator-usb-vbus { compatible = "regulator-fixed"; regulator-name = "usbh1_vbus"; regulator-min-microvolt = <500>; regulator-max-microvolt = <500>; pinctrl-0 = <&pinctrl_usbh1_vbus>; gpio = <&gpio1 0 GPIO_ACTIVE_HIGH>; enable-active-high; regulator-always-on; }; reg_usbh2_vbus: regulator-usbh2-vbus { compatible = "regulator-fixed"; regulator-name = "usbh2_vbus"; regulator-min-microvolt = <500>; regulator-max-microvolt = <500>; pinctrl-names = "default"; pinctrl-0 = <&pinctrl_usbh2_vbus>; gpio = <&gpio3 31 GPIO_ACTIVE_HIGH>; enable-active-high; regulator-boot-on; }; reg_usbh3_vbus: regulator-usbh3-vbus { compatible = "regulator-fixed"; regulator-name = "usbh3_vbus"; regulator-min-microvolt = <500>; regulator-max-microvolt = <500>; pinctrl-names =
Re: ULPI phy issue with
Hi, On 10/09/16 05:35, Peter Chen wrote: On Fri, Sep 09, 2016 at 12:50:45PM +0200, Fabien Lahoudere wrote: Hi, Thanks for your reply. On 09/09/16 09:42, Peter Chen wrote: On Thu, Sep 08, 2016 at 10:00:22AM +0200, Fabien Lahoudere wrote: Hi, I have an imx53 based boards with an ULPI phy (smsc 3315) connected to usbh2 and usbh3 and encounter problem with patch "usb: chipidea: coordinate usb phy initialization for different phy type". In drivers/usb/chipidea/core.c:ci_usb_phy_init, if I call hw_phymode_configure() before usb_phy_init(), the system freeze. If I call usb_phy_init() before hw_phymode_configure(), everything works fine. So I wonder if someone test ULPI phy? and if the following piece of patch (that works in my case) could be acceptable : Do you use upstream kernel? I wonder if current drivers/usb/phy/phy-ulpi.c code can work well with DT? I use next-20160905 and my patch works fine if I call usb_phy_init() before hw_phymode_configure(). So my issue with the current kernel appear before calling phy-ulpi functions. Stephen Boyd posted ULPI support for chipidea recently, would you please try if it works for you: usb: chipidea: Add support for ULPI PHY bus I try the patch and it did not works in my case. When I call _ci_usb_phy_init() before hw_phymode_configure(). I have a timeout returned by ulpi_write() in function ulpi_register(). So the result is worst. If I call _ci_usb_phy_init() after hw_phymode_configure(). The result is still the same. The system freeze in hw_phymode_configure() when doing : hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc); So I think that the register can be written if the phy is not powered but don 't understand why. If you want I can submit my patch to discuss about it. Yes, please send the patch and tell me which dts and ULPI code you are using. We need to understand why the system is hang when configure the PHY mode at portsc for your case, afaik, the PHY mode needs to be configured as ULPI mode before using ULPI view port to access ULPI PHY. Patch have been sent "usb: imx53 - Allow to configure ULPI mode for usb host 2 and 3", in reply to this thread. Besides, do you enable this ULPI PHY at u-boot? Yes, I configure imx53 in linux kernel in order to use ULPI mode. I remind you that the patch works fine with my device. Thanks Fabien -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] usb: imx53 - Allow to configure ULPI mode for usb host 2 and 3
In order to use ULPI phy (smsc 3315) we have to configure imx53 ULPI mode in usbmisc_imx.c. It takes configuration from device tree through ci_hdrc_imx.*. ... usbphy2: usbphy@2 { compatible = "usb-nop-xceiv"; reset-gpios = <&gpio4 4 GPIO_ACTIVE_LOW>; clock-names = "main_clk"; /* * Hardware uses CKO2 at 24MHz at several places. Set the parent * clock of CKO2 to OSC. */ clock-frequency = <2400>; clocks = <&clks IMX5_CLK_CKO2>; assigned-clocks = <&clks IMX5_CLK_CKO2_SEL>, <&clks IMX5_CLK_OSC>; assigned-clock-parents = <&clks IMX5_CLK_OSC>; status = "okay"; }; ... &usbh2 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_usbh2>; phy_type = "ulpi"; disable-int60ck; dr_mode = "host"; fsl,usbphy = <&usbphy2>; vbus-supply = <®_usbh2_vbus>; status = "okay"; }; ... After those changes, the system freeze when in hw_phymode_configure() when doing : hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc); If we call calling usb_phy_init() before hw_phymode_configure() (see core.c), the freeze is solved and we can enumerate USB device and everything works fine Signed-off-by: Fabien Lahoudere --- drivers/usb/chipidea/ci_hdrc_imx.c | 7 +++ drivers/usb/chipidea/ci_hdrc_imx.h | 2 + drivers/usb/chipidea/core.c| 2 +- drivers/usb/chipidea/usbmisc_imx.c | 101 +++-- 4 files changed, 96 insertions(+), 16 deletions(-) diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c index 0991794..89a9d98 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.c +++ b/drivers/usb/chipidea/ci_hdrc_imx.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include "ci.h" @@ -146,6 +147,12 @@ 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_find_property(np, "disable-int60ck", NULL)) + data->disable_int60ck = 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..43bafae 100644 --- a/drivers/usb/chipidea/ci_hdrc_imx.h +++ b/drivers/usb/chipidea/ci_hdrc_imx.h @@ -19,6 +19,8 @@ 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 */ + unsigned int disable_int60ck:1; /* disable 60 MHZ clock */ }; int imx_usbmisc_init(struct imx_usbmisc_data *); diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 69426e6..21218f8 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -383,6 +383,7 @@ static int ci_usb_phy_init(struct ci_hdrc *ci) case USBPHY_INTERFACE_MODE_UTMI: case USBPHY_INTERFACE_MODE_UTMIW: case USBPHY_INTERFACE_MODE_HSIC: + case USBPHY_INTERFACE_MODE_ULPI: ret = _ci_usb_phy_init(ci); if (!ret) hw_wait_phy_stable(); @@ -390,7 +391,6 @@ static int ci_usb_phy_init(struct ci_hdrc *ci) return ret; hw_phymode_configure(ci); break; - case USBPHY_INTERFACE_MODE_ULPI: case USBPHY_INTERFACE_MODE_SERIAL: hw_phymode_configure(ci); ret = _ci_usb_phy_init(ci); diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index 20d02a5..54d348a 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -46,11 +46,23 @@ #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_USB_CLKONOFF_CTRL_OFFSET 0x24 +#define MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF BIT(21) +#define MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF BIT(22)
Re: ULPI phy issue with
Hi, Thanks for your reply. On 09/09/16 09:42, Peter Chen wrote: On Thu, Sep 08, 2016 at 10:00:22AM +0200, Fabien Lahoudere wrote: Hi, I have an imx53 based boards with an ULPI phy (smsc 3315) connected to usbh2 and usbh3 and encounter problem with patch "usb: chipidea: coordinate usb phy initialization for different phy type". In drivers/usb/chipidea/core.c:ci_usb_phy_init, if I call hw_phymode_configure() before usb_phy_init(), the system freeze. If I call usb_phy_init() before hw_phymode_configure(), everything works fine. So I wonder if someone test ULPI phy? and if the following piece of patch (that works in my case) could be acceptable : Do you use upstream kernel? I wonder if current drivers/usb/phy/phy-ulpi.c code can work well with DT? I use next-20160905 and my patch works fine if I call usb_phy_init() before hw_phymode_configure(). So my issue with the current kernel appear before calling phy-ulpi functions. Stephen Boyd posted ULPI support for chipidea recently, would you please try if it works for you: usb: chipidea: Add support for ULPI PHY bus I try the patch and it did not works in my case. When I call _ci_usb_phy_init() before hw_phymode_configure(). I have a timeout returned by ulpi_write() in function ulpi_register(). So the result is worst. If I call _ci_usb_phy_init() after hw_phymode_configure(). The result is still the same. The system freeze in hw_phymode_configure() when doing : hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc); So I think that the register can be written if the phy is not powered but don 't understand why. If you want I can submit my patch to discuss about it. Thanks Fabien -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
ULPI phy issue with
Hi, I have an imx53 based boards with an ULPI phy (smsc 3315) connected to usbh2 and usbh3 and encounter problem with patch "usb: chipidea: coordinate usb phy initialization for different phy type". In drivers/usb/chipidea/core.c:ci_usb_phy_init, if I call hw_phymode_configure() before usb_phy_init(), the system freeze. If I call usb_phy_init() before hw_phymode_configure(), everything works fine. So I wonder if someone test ULPI phy? and if the following piece of patch (that works in my case) could be acceptable : ### diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 69426e6..21218f8 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -383,6 +383,7 @@ static int ci_usb_phy_init(struct ci_hdrc *ci) case USBPHY_INTERFACE_MODE_UTMI: case USBPHY_INTERFACE_MODE_UTMIW: case USBPHY_INTERFACE_MODE_HSIC: + case USBPHY_INTERFACE_MODE_ULPI: ret = _ci_usb_phy_init(ci); if (!ret) hw_wait_phy_stable(); @@ -390,7 +391,6 @@ static int ci_usb_phy_init(struct ci_hdrc *ci) return ret; hw_phymode_configure(ci); break; - case USBPHY_INTERFACE_MODE_ULPI: case USBPHY_INTERFACE_MODE_SERIAL: hw_phymode_configure(ci); ret = _ci_usb_phy_init(ci); ### If not, the patch says that actual solution this is not the best solution so what could be the better solution? Thanks Fabien -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] Add driver for smsc usb251x i2c configuration
This driver copy the configuration of the controller EEPROM via i2c. Configuration information is available in Documentation/usb/usb251x.txt Signed-off-by: Fabien Lahoudere --- Documentation/devicetree/bindings/usb/usb251x.txt | 27 +++ drivers/usb/misc/Kconfig | 9 + drivers/usb/misc/Makefile | 1 + drivers/usb/misc/usb251x.c| 265 ++ 4 files changed, 302 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/usb251x.txt create mode 100644 drivers/usb/misc/usb251x.c diff --git a/Documentation/devicetree/bindings/usb/usb251x.txt b/Documentation/devicetree/bindings/usb/usb251x.txt new file mode 100644 index 000..2b0863a3 --- /dev/null +++ b/Documentation/devicetree/bindings/usb/usb251x.txt @@ -0,0 +1,27 @@ +SMSC USB 2.0 Hi-Speed Hub Controller + +Required properties: +- compatible = "smsc,usb251x"; +- reg = <0x2c>; + +Optional properties: +- smsc,usb251x-cfg-data1 : u8, set configuration data 1 (byte 0x06) +- smsc,usb251x-cfg-data2 : u8, set configuration data 2 (byte 0x07) +- smsc,usb251x-cfg-data3 : u8, set configuration data 3 (byte 0x08) +- smsc,usb251x-portmap12 : u8, set port mapping for ports 1 and 2 (byte 0xfb) +- smsc,usb251x-portmap34 : u8, set port mapping for ports 3 and 4 (byte 0xfc) +- smsc,usb251x-portmap56 : u8, set port mapping for ports 5 and 6 (byte 0xfd) +- smsc,usb251x-portmap7 : u8, set port mapping for port 7 (byte 0xfe) +- smsc,usb251x-status-command : u8, configure smbus behaviour (byte 0xff) + +Example: + + usb251x: usb251x@2c { + compatible = "smsc,usb251x"; + reg = <0x2c>; + smsc,usb251x-cfg-data3 = <0x09>; + smsc,usb251x-portmap12 = <0x21>; + smsc,usb251x-portmap12 = <0x43>; + smsc,usb251x-status-command = <0x1>; + }; + diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig index eb8f8d3..89c532f 100644 --- a/drivers/usb/misc/Kconfig +++ b/drivers/usb/misc/Kconfig @@ -240,6 +240,15 @@ config USB_HSIC_USB3503 help This option enables support for SMSC USB3503 HSIC to USB 2.0 Driver. +config USB_USB251X + tristate "USB251X device configurable via I2C" + depends on I2C + help + This option enables support for configuring SMSC USB251X USB hub. +This driver write the hub configuration in EEPROM via i2C. Fields can be +configured through device tree. +See Documentation/devicetree/bindings/usb/usb251x.txt + config USB_LINK_LAYER_TEST tristate "USB Link Layer Test driver" help diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile index 3d79faa..dac251a 100644 --- a/drivers/usb/misc/Makefile +++ b/drivers/usb/misc/Makefile @@ -27,5 +27,6 @@ obj-$(CONFIG_USB_HSIC_USB3503)+= usb3503.o obj-$(CONFIG_USB_CHAOSKEY) += chaoskey.o obj-$(CONFIG_UCSI) += ucsi.o +obj-$(CONFIG_USB_USB251X) += usb251x.o obj-$(CONFIG_USB_SISUSBVGA)+= sisusbvga/ obj-$(CONFIG_USB_LINK_LAYER_TEST) += lvstest.o diff --git a/drivers/usb/misc/usb251x.c b/drivers/usb/misc/usb251x.c new file mode 100644 index 000..b3750fc --- /dev/null +++ b/drivers/usb/misc/usb251x.c @@ -0,0 +1,265 @@ +/* + * drivers/usb/misc/usb251x.c + * + * driver for SMSC USB251X USB Hub + * + * Authors: Rick Bronson + * Fabien Lahoudere + * + * Register initialization is based on code examples provided by Philips + * Copyright (c) 2005 Koninklijke Philips Electronics N.V. + * + * This file is licensed under + * the terms of the GNU General Public License version 2. This program + * is licensed "as is" without any warranty of any kind, whether express + * or implied. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +/* registers */ +#define USB251X_VENDOR_ID_LSB 0x00 +#define USB251X_VENDOR_ID_MSB 0x01 +#define USB251X_PRODUCT_ID_LSB 0x02 +#define USB251X_PRODUCT_ID_MSB 0x03 +#define USB251X_DEVICE_ID_LSB 0x04 +#define USB251X_DEVICE_ID_MSB 0x05 +#define USB251X_CONFIGURATION_DATA_BYTE_1 0x06 +#define USB251X_CONFIGURATION_DATA_BYTE_2 0x07 +#define USB251X_CONFIGURATION_DATA_BYTE_3 0x08 +#define USB251X_NON_REMOVABLE_DEVICES 0x09 +#define USB251X_PORT_DISABLE_SELF 0x0A +#define USB251X_PORT_DISABLE_BUS 0x0B +#define USB251X_MAX_POWER_SELF 0x0C +#define USB251X_MAX_POWER_BUS 0x0D +#define USB251X_HUB_CONTROLLER_MAX_CURRENT_SELF 0x0E +#define USB251X_HUB_CONTROLLER_MAX_CURRENT_BUS 0x0F +#define USB251X_POWER_ON_TIME 0x10 +#define USB251X_LANGUAGE_ID_HIGH 0x11 +#define USB251X_LANGUAGE_ID_LOW 0x12 +#define USB251X_MANUFACTURER_STRING_LENGTH 0x13 +#define USB251X_PRODUCT_STRING_LENGTH 0x14 +#define USB251X_SERIAL_STRING_LENGTH 0x15 +#define USB251X_MANUFACTURER_STRING 0x16 +#define USB251X_PRODUCT_ST