Re: [PATCH] USB: EHCI: don't reread PORTSC after disabling port power

[Marc Kleine-Budde]

On 10/20/2017 09:47 PM, Alan Stern wrote:
>> On Fri, Oct 20, 2017 at 05:27:09PM -0200, Fabio Estevam wrote:
>>> On Fri, Oct 20, 2017 at 5:20 PM, Uwe Kleine-K�nig
>>> <u.kleine-koe...@pengutronix.de> wrote:
>>>
>>>> It also works. However I wonder if it's right that I'm spammed by
>>>> over-current messages now (independent of which fix I choose) as long as
>>>> there is something connected to the port that draws too much power:
>>>>
>>>> [   53.406833] usb usb1-port1: over-current condition
>>>> [   53.631749] usb usb1-port1: over-current condition
>>>> [   53.856720] usb usb1-port1: over-current condition
>>>> [   54.081732] usb usb1-port1: over-current condition
>>>> [   54.306727] usb usb1-port1: over-current condition
>>>> [   54.531722] usb usb1-port1: over-current condition
>>>> [   54.756722] usb usb1-port1: over-current condition
>>>>
>>>> It seems to be intended or am I missing something?
> 
> Well, it does report a potentially hardware-damaging condition.  But 
> you are right that excessive repetition doesn't do any good.
> 
>>> Does it help if you pass 'disable-over-current' property?
>>
>> I assume it does, but that's not the point. It seems to work just fine,
>> because the messages come in iff there is an over-current condition on
>> that port.
>>
>> I guess what is really wanted here is that the loop
>>
>>start:
>>  printk(overcurrent-event)
>>  disable port power
>>  sleep
>>  enable port power
>>  goto start
>>
>> gets a bit smarter to not print the message even if the port signals a
>> new overcurrent event after power was reset.
> 
> Maybe we should keep track of the time interval between overcurrent
> events on each port.  If we see more than one event in a 10-second
> window, leave the port permanently powered-off.  (But then how would we
> recover?)
> 
> On the other hand, how often do people encounter overcurrent 
> situations?  If the hardware is working properly, not very often.
> 
> I'm open to suggestions...

One of our $CUSTOMERS has the requirement to keep the USB turned off
after an OC event occurred.

regards,
Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH net-next 4/5] treewide: replace dev->trans_start update with helper

[Marc Kleine-Budde]

On 05/03/2016 04:33 PM, Florian Westphal wrote:
> Replace all trans_start updates with netif_trans_update helper.
> change was done via spatch:
> 
> struct net_device *d;
> @@
> - d->trans_start = jiffies
> + netif_trans_update(d)
> 
> Compile tested only.
> 
> Cc: user-mode-linux-de...@lists.sourceforge.net
> Cc: linux-xte...@linux-xtensa.org
> Cc: linux1394-de...@lists.sourceforge.net
> Cc: linux-r...@vger.kernel.org
> Cc: net...@vger.kernel.org
> Cc: mpt-fusionlinux@broadcom.com
> Cc: linux-s...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Cc: linux-par...@vger.kernel.org
> Cc: linux-o...@vger.kernel.org
> Cc: linux-h...@vger.kernel.org
> Cc: linux-usb@vger.kernel.org
> Cc: linux-wirel...@vger.kernel.org
> Cc: linux-s...@vger.kernel.org
> Cc: de...@driverdev.osuosl.org
> Cc: b.a.t.m@lists.open-mesh.org
> Cc: linux-blueto...@vger.kernel.org
> Signed-off-by: Florian Westphal <f...@strlen.de>
> ---
>  Checkpatch complains about whitespace damage, but
>  this extra whitespace already exists before this patch.
> 
>  drivers/net/can/mscan/mscan.c  | 4 ++--
>  drivers/net/can/usb/ems_usb.c  | 4 ++--
>  drivers/net/can/usb/esd_usb2.c | 4 ++--
>  drivers/net/can/usb/peak_usb/pcan_usb_core.c   | 4 ++--

> diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
> index e36b740..acb708f 100644
> --- a/drivers/net/can/mscan/mscan.c
> +++ b/drivers/net/can/mscan/mscan.c
> @@ -276,7 +276,7 @@ static netdev_tx_t mscan_start_xmit(struct sk_buff *skb, 
> struct net_device *dev)
>   out_8(>cantflg, 1 << buf_id);
>  
>   if (!test_bit(F_TX_PROGRESS, >flags))
> - dev->trans_start = jiffies;
> + netif_trans_update(dev);
>  
>   list_add_tail(>tx_queue[buf_id].list, >tx_head);
>  
> @@ -469,7 +469,7 @@ static irqreturn_t mscan_isr(int irq, void *dev_id)
>   clear_bit(F_TX_PROGRESS, >flags);
>   priv->cur_pri = 0;
>   } else {
> - dev->trans_start = jiffies;
> + netif_trans_update(dev);
>   }
>  
>   if (!test_bit(F_TX_WAIT_ALL, >flags))
> diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
> index 3400fd1..71f0e79 100644
> --- a/drivers/net/can/usb/ems_usb.c
> +++ b/drivers/net/can/usb/ems_usb.c
> @@ -521,7 +521,7 @@ static void ems_usb_write_bulk_callback(struct urb *urb)
>   if (urb->status)
>   netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status);
>  
> - netdev->trans_start = jiffies;
> + netif_trans_update(netdev);
>  
>   /* transmission complete interrupt */
>   netdev->stats.tx_packets++;
> @@ -835,7 +835,7 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff 
> *skb, struct net_device *ne
>   stats->tx_dropped++;
>   }
>   } else {
> - netdev->trans_start = jiffies;
> + netif_trans_update(netdev);
>  
>   /* Slow down tx path */
>   if (atomic_read(>active_tx_urbs) >= MAX_TX_URBS ||
> diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
> index 113e64f..784a900 100644
> --- a/drivers/net/can/usb/esd_usb2.c
> +++ b/drivers/net/can/usb/esd_usb2.c
> @@ -480,7 +480,7 @@ static void esd_usb2_write_bulk_callback(struct urb *urb)
>   if (urb->status)
>   netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status);
>  
> - netdev->trans_start = jiffies;
> + netif_trans_update(netdev);
>  }
>  
>  static ssize_t show_firmware(struct device *d,
> @@ -820,7 +820,7 @@ static netdev_tx_t esd_usb2_start_xmit(struct sk_buff 
> *skb,
>   goto releasebuf;
>   }
>  
> - netdev->trans_start = jiffies;
> + netif_trans_update(netdev);
>  
>   /*
>* Release our reference to this URB, the USB core will eventually free
> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c 
> b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> index 5a2e341..bfb91d8 100644
> --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
> @@ -274,7 +274,7 @@ static void peak_usb_write_bulk_callback(struct urb *urb)
>   netdev->stats.tx_bytes += context->data_len;
>  
>   /* prevent tx timeout */
> - netdev->trans_start = jiffies;
> + netif_trans_update(netdev);
>       break;
>  
>   default:
> @@ -373,7 +373,7 

Re: [PATCH v4 2/2] usb: chipidea: imx: refine clock operations to adapt for all platforms

[Marc Kleine-Budde]

On 10/14/2015 03:57 AM, Peter Chen wrote:
> Some i.mx platforms need three clocks to let controller work, but
> others only need one, refine clock operation to adapt for all
> platforms, it fixes a regression found at i.mx27.
> 
> Signed-off-by: Peter Chen <peter.c...@freescale.com>
> Tested-by: Fabio Estevam <fabio.este...@freescale.com>
> Cc: <sta...@vger.kernel.org> #v4.1+
> ---
>  drivers/usb/chipidea/ci_hdrc_imx.c | 132 
> -
>  1 file changed, 114 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
> b/drivers/usb/chipidea/ci_hdrc_imx.c
> index 6ccbf60..82b1dfe 100644
> --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> @@ -84,6 +84,12 @@ struct ci_hdrc_imx_data {
>   struct imx_usbmisc_data *usbmisc_data;
>   bool supports_runtime_pm;
>   bool in_lpm;
> + /* SoC before i.mx6 (except imx23/imx28) needs three clks */
> + bool need_three_clks;
> + struct clk *clk_ipg;
> + struct clk *clk_ahb;
> + struct clk *clk_per;
> + /* - */
>  };
>  
>  /* Common functions shared by usbmisc drivers */
> @@ -135,6 +141,103 @@ static struct imx_usbmisc_data 
> *usbmisc_get_init_data(struct device *dev)
>  }
>  
>  /* End of common functions shared by usbmisc drivers*/
> +static int imx_get_clks(struct device *dev)
> +{
> + struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + data->clk_ipg = devm_clk_get(dev, "ipg");
> + if (IS_ERR(data->clk_ipg)) {
> + /* If the platform only needs one clocks */
> + data->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(data->clk)) {
> + ret = PTR_ERR(data->clk);
> + dev_err(dev,
> + "Failed to get clock, err=%d\n", ret);
> + dev_err(dev,
> + "Failed to get ipg clock, err=%d\n", ret);

Nitpick, one error message should be enough.

> + return ret;
> + }
> + return ret;
> + }
> +
> + data->clk_ahb = devm_clk_get(dev, "ahb");
> + if (IS_ERR(data->clk_ahb)) {
> + ret = PTR_ERR(data->clk_ahb);
> + dev_err(dev,
> + "Failed to get ahb clock, err=%d\n", ret);
> + return ret;
> + }
> +
> + data->clk_per = devm_clk_get(dev, "per");
> + if (IS_ERR(data->clk_per)) {
> + ret = PTR_ERR(data->clk_per);
> + dev_err(dev,
> + "Failed to get per clock, err=%d\n", ret);
> + return ret;
> + }
> +
> + data->need_three_clks = true;
> + return ret;
> +}
> +
> +static int imx_prepare_enable_clks(struct device *dev)
> +{
> + struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + if (data->need_three_clks) {
> + ret = clk_prepare_enable(data->clk_ipg);
> + if (ret) {
> + dev_err(dev,
> + "Failed to prepare/enable ipg clk, err=%d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(data->clk_ahb);
> + if (ret) {
> + dev_err(dev,
> + "Failed to prepare/enable ahb clk, err=%d\n",
> + ret);
> + clk_disable_unprepare(data->clk_ipg);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(data->clk_per);
> + if (ret) {
> + dev_err(dev,
> + "Failed to prepare/enable per clk, err=%d\n",
> + ret);
> + clk_disable_unprepare(data->clk_ahb);
> + clk_disable_unprepare(data->clk_ipg);
> + return ret;
> + }
> + } else {
> + ret = clk_prepare_enable(data->clk);
> + if (ret) {
> + dev_err(dev,
> + "Failed to prepare/enable clk, err=%d\n",
> + ret);
> + return ret;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static void imx_disable_unprepare_clks(struct device *dev)
> +{
> + struct ci_hdrc_im

Re: [PATCH 2/5] can: kvaser_usb: Read all messages in a bulk-in URB buffer

[Marc Kleine-Budde]

On 02/26/2015 04:22 PM, Ahmed S. Darwish wrote:
 From: Ahmed S. Darwish ahmed.darw...@valeo.com
 
 The Kvaser firmware can only read and write messages that are
 not crossing the USB endpoint's wMaxPacketSize boundary. While
 receiving commands from the CAN device, if the next command in
 the same URB buffer crossed that max packet size boundary, the
 firmware puts a zero-length placeholder command in its place
 then moves the real command to the next boundary mark.
 
 The driver did not recognize such behavior, leading to missing
 a good number of rx events during a heavy rx load session.
 
 Moreover, a tx URB context only gets freed upon receiving its
 respective tx ACK event. Over time, the free tx URB contexts
 pool gets depleted due to the missing ACK events. Consequently,
 the netif transmission queue gets __permanently__ stopped; no
 frames could be sent again except after restarting the CAN
 newtwork interface.
 
 Signed-off-by: Ahmed S. Darwish ahmed.darw...@valeo.com

Can you please send a fix for this endianess errors (against
linux-can-fixes-for-4.0-20150314):

 drivers/net/can/usb/kvaser_usb.c:595:39: warning: restricted __le16 degrades 
 to integer
 drivers/net/can/usb/kvaser_usb.c:1332:31: warning: restricted __le16 degrades 
 to integer

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 4/4] can: kvaser_usb: Retry first bulk transfer on -ETIMEDOUT

[Marc Kleine-Budde]

On 01/12/2015 11:14 AM, Ahmed S. Darwish wrote:
 On Sun, Jan 11, 2015 at 09:51:10PM +0100, Marc Kleine-Budde wrote:
 On 01/11/2015 09:45 PM, Ahmed S. Darwish wrote:
 From: Ahmed S. Darwish ahmed.darw...@valeo.com

 (This is a draft patch, I'm not sure if this fixes the USB
 bug or only its psymptom. Feedback from the linux-usb folks
 is really appreciated.)

 When plugging the Kvaser USB/CAN dongle the first time, everything
 works as expected and all of the transfers from and to the USB
 device succeeds.

 Meanwhile, after unplugging the device and plugging it again, the
 first bulk transfer _always_ returns an -ETIMEDOUT. The following
 behaviour was observied:

 - Setting higher timeout values for the first bulk transfer never
   solved the issue.

 - Unloading, then loading, our kvaser_usb module in question
   __always__ solved the issue.

 - Checking first bulk transfer status, and retry the transfer
   again in case of an -ETIMEDOUT also __always__ solved the issue.
   This is what the patch below does.

 - In the testing done so far, this issue appears only on laptops
   but never on PCs (possibly power related?)

 Signed-off-by: Ahmed S. Darwish ahmed.darw...@valeo.com

 Does this patch apply apply between 3 and 4? If not, please re-arrange
 the series. As this is a bug fix, patches 1, 2 and 4 will go via
 net/master, 3 will go via net-next/master.
 
 Since no one complained earlier, I guess this issue only affects
 USBCAN devices. That's why I've based it above patch #3: adding
 USBCAN hardware support.
 
 Nonetheless, it won't do any harm for the current Leaf-only
 driver. So _if_ this is the correct fix, I will update the commit
 log, refactor the check into a 'do { } while()' loop, and then
 base it above the Leaf-only net/master fixes on patch #1, and #2.
 
 Any feedback on the USB side of things?

Maybe you have to change the subject of this patch to be more visible on
the USB list and/or add the right USB people on Cc.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 4/4] can: kvaser_usb: Retry first bulk transfer on -ETIMEDOUT

[Marc Kleine-Budde]

On 01/11/2015 09:45 PM, Ahmed S. Darwish wrote:
 From: Ahmed S. Darwish ahmed.darw...@valeo.com
 
 (This is a draft patch, I'm not sure if this fixes the USB
 bug or only its psymptom. Feedback from the linux-usb folks
 is really appreciated.)
 
 When plugging the Kvaser USB/CAN dongle the first time, everything
 works as expected and all of the transfers from and to the USB
 device succeeds.
 
 Meanwhile, after unplugging the device and plugging it again, the
 first bulk transfer _always_ returns an -ETIMEDOUT. The following
 behaviour was observied:
 
 - Setting higher timeout values for the first bulk transfer never
   solved the issue.
 
 - Unloading, then loading, our kvaser_usb module in question
   __always__ solved the issue.
 
 - Checking first bulk transfer status, and retry the transfer
   again in case of an -ETIMEDOUT also __always__ solved the issue.
   This is what the patch below does.
 
 - In the testing done so far, this issue appears only on laptops
   but never on PCs (possibly power related?)
 
 Signed-off-by: Ahmed S. Darwish ahmed.darw...@valeo.com

Does this patch apply apply between 3 and 4? If not, please re-arrange
the series. As this is a bug fix, patches 1, 2 and 4 will go via
net/master, 3 will go via net-next/master.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [RFC PATCH 0/2] usb: Reuse fsl driver code for synopsys usb controller

[Marc Kleine-Budde]

On 04/20/2014 06:27 PM, Punnaiah Choudary Kalluri wrote:
 Zynq soc contains a dual role usb controller and this IP is from synopsys. We
 observed that there is driver available for this controller from freescale and
 decided to reuse this driver for zynq use.

Have a look drivers/usb/chipidea. It's maintained by Peter Chen (Cc'ed)
and is in better shape than the freescale driver.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v10 01/15] usb: doc: phy-mxs: Add more compatible strings

[Marc Kleine-Budde]

On 02/21/2014 10:40 AM, Peter Chen wrote:
  

  Required properties:
 -- compatible: Should be fsl,imx23-usbphy
 +- compatible: fsl,imx23-usbphy for imx23 and imx28, fsl,imx6q-
 usbphy
 +  for imx6dq and imx6dl, fsl,imx6sl-usbphy for imx6sl

 Minor nit, but could we restructure this as something like the following,
 with each string on a new line:

 - compatible: should contain:
 * fsl,imx23-usbphy for imx23 and imx28
 * fsl,imx6q-usbphy for imx6dq and imx6dl
 * fsl,imx6sl-usbphy for imx6sl

 It makes it a bit easier to read.
 
 Thanks, will change like above.
 

 I see the existing fsl,imx23-usbphy is used as a fallback for
 fsl,imx28-usbphy, fsl,imx6q-usbphy, and fsl,imx6sl-usbphy in
 existing DTs.

 Is this expected going forward? It might be worth mentioning.

 
 These SoCs used the same FSL imx PHY, but different versions.
 imx23/imx28 are the first version, more improvements are at
 later SoCs (like imx6x) version. Keep fsl,imx23-usbphy at
 imx6 dts will be user know it is from imx23's. If you think
 it does not need, I can delete fsl,imx23-usbphy from imx6 dts.

You should go after compatibility here. List (all) phys that are
comaptible, start with most specific end with most generic.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 4/8] usb: ehci: add freescale imx28 special write register method

[Marc Kleine-Budde]

On 01/09/2014 04:22 AM, Greg KH wrote:
 On Thu, Jan 09, 2014 at 09:36:09AM +0800, Peter Chen wrote:
 On Tue, Jan 07, 2014 at 04:20:25PM -0800, Greg KH wrote:
 On Mon, Jan 06, 2014 at 09:42:26AM +0100, Marc Kleine-Budde wrote:
 Hello Peter and Greg,

 On 01/06/2014 03:10 AM, Peter Chen wrote:
 According to Freescale imx28 Errata, ENGR119653 USB: ARM to USB
 register error issue, All USB register write operations must
 use the ARM SWP instruction. So, we implement a special ehci_write
 for imx28.

 Discussion for it at below:
 http://marc.info/?l=linux-usbm=137996395529294w=2

 Signed-off-by: Peter Chen peter.c...@freescale.com
 Acked-by: Alan Stern st...@rowland.harvard.edu
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 Tested-by: Marc Kleine-Budde m...@pengutronix.de

 please add stable on Cc for this and the next two patches:

 [PATCH 4/8] usb: ehci: add freescale imx28 special write register method
 [PATCH 5/8] usb: chipidea: add freescale imx28 special write register 
 method
 [PATCH 6/8] usb: chipidea: imx: set CI_HDRC_IMX28_WRITE_FIX for imx28

 How do those patches meet the Documentation/stable_kernel_rules.txt
 guidelines?



 - It must be obviously correct and tested.
 It has Marc Kleine-Budde's tested-by tag.

 - It cannot be bigger than 100 lines, with context.
 I think it is.

 - It must fix only one thing.
 It only fixes the imx28 special write problem.

 - It must fix a real bug that bothers people (not a, This could be a
 problem... type thing).
 Robert Hodaszi reported this problem at below link:
 http://marc.info/?l=linux-usbm=137996395529294w=2
 
 You are adding new functionality for something that never worked before
 (i.e. new features), which is not ok for stable kernel patches, with the
 exception of new quirks or device ids.
 
 sorry, this is something new, not a stable kernel patch.

Without this fix, the mx28 works...most of the time. However under
certain load scenarios the driver breaks. This is, IMHO, a fix.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 05/10] usb: chipidea: add OTG fsm operation functions implemenation.

[Marc Kleine-Budde]

On 01/08/2014 10:06 AM, Li Jun wrote:
 Add OTG HNP and SRP operation functions implementation:
 - charge vbus
 - drive vbus
 - connection signaling
 - drive sof
 - start data pulse
 - add fsm timer
 - delete fsm timer
 - start host
 - start gadget
 
 Signed-off-by: Li Jun b47...@freescale.com
 ---
  drivers/usb/chipidea/bits.h|   11 ++
  drivers/usb/chipidea/otg_fsm.c |  311 
 
  drivers/usb/chipidea/otg_fsm.h |8 +
  3 files changed, 330 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
 index a857131..4347414 100644
 --- a/drivers/usb/chipidea/bits.h
 +++ b/drivers/usb/chipidea/bits.h
 @@ -44,9 +44,14 @@
  #define DEVICEADDR_USBADR (0x7FUL  25)
  
  /* PORTSC */
 +#define PORTSC_CCS BIT(0)
 +#define PORTSC_CSC BIT(1)
 +#define PORTSC_PEC BIT(3)
 +#define PORTSC_OCC BIT(5)
  #define PORTSC_FPRBIT(6)
  #define PORTSC_SUSP   BIT(7)
  #define PORTSC_HSPBIT(9)
 +#define PORTSC_PP BIT(12)
  #define PORTSC_PTC(0x0FUL  16)
  #define PORTSC_PHCD(d) ((d) ? BIT(22) : BIT(23))
  /* PTS and PTW for non lpm version only */
 @@ -55,6 +60,9 @@
  #define PORTSC_PTWBIT(28)
  #define PORTSC_STSBIT(29)
  
 +#define PORTSC_W1C_BITS  \
 + (PORTSC_CSC | PORTSC_PEC | PORTSC_OCC)
 +
  /* DEVLC */
  #define DEVLC_PSPD(0x03UL  25)
  #define DEVLC_PSPD_HS (0x02UL  25)
 @@ -69,7 +77,10 @@
  #define PTS_HSIC  4
  
  /* OTGSC */
 +#define OTGSC_VD   BIT(0)
 +#define OTGSC_VC   BIT(1)
  #define OTGSC_IDPU BIT(5)
 +#define OTGSC_HADP BIT(6)
  #define OTGSC_ID   BIT(8)
  #define OTGSC_AVV  BIT(9)
  #define OTGSC_ASV  BIT(10)
 diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
 index 1f8907d..31a046d 100644
 --- a/drivers/usb/chipidea/otg_fsm.c
 +++ b/drivers/usb/chipidea/otg_fsm.c
 @@ -19,12 +19,322 @@
  #include linux/usb/otg-fsm.h
  #include linux/usb/gadget.h
  #include linux/usb/chipidea.h
 +#include linux/regulator/consumer.h
  
  #include ci.h
  #include bits.h
  #include otg.h
  #include otg_fsm.h
  
 +static struct list_head active_timers;
 +
 +#define HA_DATA_PULSE 1
 +
 +/* FSM timers */
 +struct ci_otg_fsm_timer *a_wait_vrise_tmr, *a_wait_vfall_tmr, 
 *a_wait_bcon_tmr,
 + *a_aidl_bdis_tmr, *a_bidl_adis_tmr, *b_ase0_brst_tmr,
 + *b_se0_srp_tmr, *b_srp_fail_tmr, *b_data_pulse_tmr;

IIRC the USB spec says there is only one OTG port per machine, but I
think you shouldn't add new code that uses so many global variables,
better embed these into a per device struct.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 4/8] usb: ehci: add freescale imx28 special write register method

[Marc Kleine-Budde]

On 01/08/2014 01:23 AM, Greg KH wrote:
 On Mon, Jan 06, 2014 at 10:10:41AM +0800, Peter Chen wrote:
 According to Freescale imx28 Errata, ENGR119653 USB: ARM to USB
 register error issue, All USB register write operations must
 use the ARM SWP instruction. So, we implement a special ehci_write
 for imx28.

 Discussion for it at below:
 http://marc.info/?l=linux-usbm=137996395529294w=2

 Signed-off-by: Peter Chen peter.c...@freescale.com
 Acked-by: Alan Stern st...@rowland.harvard.edu
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 Tested-by: Marc Kleine-Budde m...@pengutronix.de
 ---
  drivers/usb/host/ehci.h |   18 +-
  1 files changed, 17 insertions(+), 1 deletions(-)

 diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
 index c35a6e2b..e26099b 100644
 --- a/drivers/usb/host/ehci.h
 +++ b/drivers/usb/host/ehci.h
 @@ -225,6 +225,7 @@ struct ehci_hcd {/* one per 
 controller */
  unsignedhas_synopsys_hc_bug:1; /* Synopsys HC */
  unsignedframe_index_bug:1; /* MosChip (AKA NetMos) */
  unsignedneed_oc_pp_cycle:1; /* MPC834X port power */
 +unsignedimx28_write_fix:1; /* For Freescale i.MX28 */
  
  /* required for usb32 quirk */
  #define OHCI_CTRL_HCFS  (3  6)
 @@ -728,6 +729,18 @@ static inline unsigned int ehci_readl(const struct 
 ehci_hcd *ehci,
  #endif
  }
  
 +#ifdef CONFIG_SOC_IMX28
 +static inline void imx28_ehci_writel(const unsigned int val,
 +volatile __u32 __iomem *addr)
 +{
 +__asm__ (swp %0, %0, [%1] : : r(val), r(addr));
 +}
 +#else
 +static inline void imx28_ehci_writel(const unsigned int val,
 +volatile __u32 __iomem *addr)
 +{
 +}
 +#endif
  static inline void ehci_writel(const struct ehci_hcd *ehci,
  const unsigned int val, __u32 __iomem *regs)
  {
 @@ -736,7 +749,10 @@ static inline void ehci_writel(const struct ehci_hcd 
 *ehci,
  writel_be(val, regs) :
  writel(val, regs);
  #else
 -writel(val, regs);
 +if (IS_ENABLED(CONFIG_SOC_IMX28)  ehci-imx28_write_fix)
 +imx28_ehci_writel(val, regs);
 +else
 +writel(val, regs);
  #endif
 
 This IS_ENABLED() isn't needed at all, so please remove it.

It's an optimisation for the hot path, if kernel isn't build for a mx28
the newly added if() is completely optimized out. The same argument
applies to the next patch.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 4/8] usb: ehci: add freescale imx28 special write register method

[Marc Kleine-Budde]

Hello Peter and Greg,

On 01/06/2014 03:10 AM, Peter Chen wrote:
 According to Freescale imx28 Errata, ENGR119653 USB: ARM to USB
 register error issue, All USB register write operations must
 use the ARM SWP instruction. So, we implement a special ehci_write
 for imx28.
 
 Discussion for it at below:
 http://marc.info/?l=linux-usbm=137996395529294w=2
 
 Signed-off-by: Peter Chen peter.c...@freescale.com
 Acked-by: Alan Stern st...@rowland.harvard.edu
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 Tested-by: Marc Kleine-Budde m...@pengutronix.de

please add stable on Cc for this and the next two patches:

[PATCH 4/8] usb: ehci: add freescale imx28 special write register method
[PATCH 5/8] usb: chipidea: add freescale imx28 special write register method
[PATCH 6/8] usb: chipidea: imx: set CI_HDRC_IMX28_WRITE_FIX for imx28

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v5 08/15] usb: phy-mxs: Add implementation of nofity_suspend and notify_resume

[Marc Kleine-Budde]

On 12/09/2013 07:30 AM, Peter Chen wrote:
 Implementation of notify_suspend and notify_resume will be different
 according to mxs_phy_data-flags.
 
 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---
  drivers/usb/phy/phy-mxs-usb.c |   55 
 ++---
  1 files changed, 51 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
 index 0ef930a..e3df53f 100644
 --- a/drivers/usb/phy/phy-mxs-usb.c
 +++ b/drivers/usb/phy/phy-mxs-usb.c
 @@ -166,8 +166,8 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
  static int mxs_phy_on_connect(struct usb_phy *phy,
   enum usb_device_speed speed)
  {
 - dev_dbg(phy-dev, %s speed device has connected\n,
 - (speed == USB_SPEED_HIGH) ? high : non-high);
 + dev_dbg(phy-dev, %s device has connected\n,
 + (speed == USB_SPEED_HIGH) ? HS : FS/LS);

What about creating a mxs_phy_dbg() function that adds the HS FS/LS
prefix when printing?

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v5 09/15] usb: phy-mxs: Enable IC fixes for related SoCs

[Marc Kleine-Budde]

On 12/09/2013 07:30 AM, Peter Chen wrote:
 Some PHY bugs are fixed by IC logic, but these bits are not
 enabled by default, so we enable them at driver.

Which bugs are fixed by enabling this bit? Is it only suspend related?
Can you document them or better add a pointer to the documentation.

Further I don't like the idea of adding code, or enabling a feature on
certain hardware, that is broken in the first place and fixing it in a
later patch. Think about squashing it into the correct patch.

 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---
  drivers/usb/phy/phy-mxs-usb.c |   28 ++--
  1 files changed, 26 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
 index e3df53f..d1c319b 100644
 --- a/drivers/usb/phy/phy-mxs-usb.c
 +++ b/drivers/usb/phy/phy-mxs-usb.c
 @@ -31,6 +31,10 @@
  #define HW_USBPHY_CTRL_SET   0x34
  #define HW_USBPHY_CTRL_CLR   0x38
  
 +#define HW_USBPHY_IP 0x90
 +#define HW_USBPHY_IP_SET 0x94
 +#define HW_USBPHY_IP_CLR 0x98
 +
  #define BM_USBPHY_CTRL_SFTRSTBIT(31)
  #define BM_USBPHY_CTRL_CLKGATE   BIT(30)
  #define BM_USBPHY_CTRL_ENAUTOSET_USBCLKS BIT(26)
 @@ -42,6 +46,8 @@
  #define BM_USBPHY_CTRL_ENUTMILEVEL2  BIT(14)
  #define BM_USBPHY_CTRL_ENHOSTDISCONDETECTBIT(1)
  
 +#define BM_USBPHY_IP_FIX   (BIT(17) | BIT(18))
 +
  #define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
  
  /* Do disconnection between PHY and controller without vbus */
 @@ -63,6 +69,9 @@
  /* The SoCs who have anatop module */
  #define MXS_PHY_HAS_ANATOP   BIT(3)
  
 +/* IC has bug fixes logic */
 +#define MXS_PHY_NEED_IP_FIX  BIT(4)
 +
  struct mxs_phy_data {
   unsigned int flags;
  };
 @@ -74,12 +83,14 @@ static const struct mxs_phy_data imx23_phy_data = {
  static const struct mxs_phy_data imx6q_phy_data = {
   .flags = MXS_PHY_SENDING_SOF_TOO_FAST |
   MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
 - MXS_PHY_HAS_ANATOP,
 + MXS_PHY_HAS_ANATOP |
 + MXS_PHY_NEED_IP_FIX,
  };
  
  static const struct mxs_phy_data imx6sl_phy_data = {
   .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
 - MXS_PHY_HAS_ANATOP,
 + MXS_PHY_HAS_ANATOP |
 + MXS_PHY_NEED_IP_FIX,
  };
  
  static const struct of_device_id mxs_phy_dt_ids[] = {
 @@ -97,6 +108,16 @@ struct mxs_phy {
   struct regmap *regmap_anatop;
  };
  
 +static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
 +{
 + return mxs_phy-data == imx6q_phy_data;
 +}
 +
 +static inline bool is_imx6sl_phy(struct mxs_phy *mxs_phy)
 +{
 + return mxs_phy-data == imx6sl_phy_data;
 +}

Are the two is_imx6* functions still needed?

 +
  static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
  {
   int ret;
 @@ -123,6 +144,9 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
   BM_USBPHY_CTRL_ENUTMILEVEL3,
  base + HW_USBPHY_CTRL_SET);
  
 + if (mxs_phy-data-flags  MXS_PHY_NEED_IP_FIX)
 + writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
 +
   return 0;
  }

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v5 12/15] usb: phy: Add set_wakeup API

[Marc Kleine-Budde]

On 12/09/2013 07:31 AM, Peter Chen wrote:
 This API is used to set wakeup enable at PHY registers, in that
 case, the PHY can be waken up from suspend due to external events,

I' no native speaker, but I think it's to be woken up.

 like vbus change, dp/dm change and id change.
 
 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---
  include/linux/usb/phy.h |   16 
  1 files changed, 16 insertions(+), 0 deletions(-)
 
 diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
 index a747960..c6ebe1d 100644
 --- a/include/linux/usb/phy.h
 +++ b/include/linux/usb/phy.h
 @@ -111,6 +111,13 @@ struct usb_phy {
   int (*set_suspend)(struct usb_phy *x,
   int suspend);
  
 + /*
 +  * Set wakeup enable for PHY, in that case, the PHY can be
 +  * waken up from suspend status due to external events,
 +  * like vbus change, dp/dm change and id.
 +  */
 + int (*set_wakeup)(struct usb_phy *x, bool enabled);
 +
   /* notify phy connect status change */
   int (*notify_connect)(struct usb_phy *x,
   enum usb_device_speed speed);
 @@ -270,6 +277,15 @@ usb_phy_set_suspend(struct usb_phy *x, int suspend)
  }
  
  static inline int
 +usb_phy_set_wakeup(struct usb_phy *x, bool enabled)
 +{
 + if (x  x-set_wakeup)

can x be NULL?

 + return x-set_wakeup(x, enabled);
 + else
 + return 0;
 +}
 +
 +static inline int
  usb_phy_notify_connect(struct usb_phy *x, enum usb_device_speed speed)
  {
   if (x  x-notify_connect)
 

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v5 15/15] usb: phy-mxs: Add sync time after controller clear phcd

[Marc Kleine-Budde]

On 12/09/2013 07:31 AM, Peter Chen wrote:
 After clear portsc.phcd, PHY needs 200us stable time for switch
 32K clock to AHB clock.

If this is a general bugbix, please move it to the start of this series
and add stable on Cc. I think I hit the bug on the second USB port of a
custom MX28 board, the stmp_reset_block() in the USB phy will fail.

Is the problem documented somewhere?

Is it possible to add the delay in the clock framework? I think only the
regulator framework has a configurable delay for the regulator to stabilize.

 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---
  drivers/usb/phy/phy-mxs-usb.c |   11 +++
  1 files changed, 11 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
 index e18fdf3..7ae5225 100644
 --- a/drivers/usb/phy/phy-mxs-usb.c
 +++ b/drivers/usb/phy/phy-mxs-usb.c
 @@ -151,6 +151,15 @@ static inline bool is_imx6sl_phy(struct mxs_phy *mxs_phy)
   return mxs_phy-data == imx6sl_phy_data;
  }
  
 +/*
 + * PHY needs some 32K cycles to switch from 32K clock to
 + * bus (such as AHB/AXI, etc) clock.
 + */
 +static void mxs_phy_clock_switch(void)
 +{
 + usleep_range(300, 400);
 +}
 +
  static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
  {
   int ret;
 @@ -276,6 +285,7 @@ static int mxs_phy_init(struct usb_phy *phy)
  {
   struct mxs_phy *mxs_phy = to_mxs_phy(phy);
  
 + mxs_phy_clock_switch();
   clk_prepare_enable(mxs_phy-clk);
   return mxs_phy_hw_init(mxs_phy);
  }
 @@ -300,6 +310,7 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
  x-io_priv + HW_USBPHY_CTRL_SET);
   clk_disable_unprepare(mxs_phy-clk);
   } else {
 + mxs_phy_clock_switch();
   clk_prepare_enable(mxs_phy-clk);
   writel(BM_USBPHY_CTRL_CLKGATE,
  x-io_priv + HW_USBPHY_CTRL_CLR);
 

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 3/7] usb: ehci: add freescale imx28 special write register method

[Marc Kleine-Budde]

On 12/09/2013 03:04 AM, Greg KH wrote:
 On Thu, Dec 05, 2013 at 03:20:51PM +0800, Peter Chen wrote:
 According to Freescale imx28 Errata, ENGR119653 USB: ARM to USB
 register error issue, All USB register write operations must
 use the ARM SWP instruction. So, we implement a special ehci_write
 for imx28.

 Discussion for it at below:
 http://marc.info/?l=linux-usbm=137996395529294w=2

 Signed-off-by: Peter Chen peter.c...@freescale.com
 Acked-by: Alan Stern st...@rowland.harvard.edu
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 Tested-by: Marc Kleine-Budde m...@pengutronix.de
 ---
  drivers/usb/host/ehci.h |   18 +-
  1 files changed, 17 insertions(+), 1 deletions(-)
 
 How is this a regression that needs to be fixed in 3.13-final?

Yes, all three (3/7, 4/7, 5/7) should go into v3.13, add you please add
stable on Cc as well.

Thanks,
Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v5 09/15] usb: phy-mxs: Enable IC fixes for related SoCs

[Marc Kleine-Budde]

On 12/09/2013 10:07 AM, Peter Chen wrote:
 On Mon, Dec 09, 2013 at 09:38:17AM +0100, Marc Kleine-Budde wrote:
 On 12/09/2013 07:30 AM, Peter Chen wrote:
 Some PHY bugs are fixed by IC logic, but these bits are not
 enabled by default, so we enable them at driver.

 Which bugs are fixed by enabling this bit? Is it only suspend related?
 Can you document them or better add a pointer to the documentation.
 
 I will add more, in fact, it fixes the bug which flag BIT(1) and BIT(2)
 stands for.
 

 Further I don't like the idea of adding code, or enabling a feature on
 certain hardware, that is broken in the first place and fixing it in a
 later patch. Think about squashing it into the correct patch.
 
 No fixes are related with this patch, you can see there is no -
 at this patch.

Yes, there isn't any broken code (thus no -), but you first enable a
feature in the hardware and in a later patch (this one) make it work
properly.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v5 15/15] usb: phy-mxs: Add sync time after controller clear phcd

[Marc Kleine-Budde]

On 12/09/2013 10:19 AM, Peter Chen wrote:
 On Mon, Dec 09, 2013 at 10:05:17AM +0100, Marc Kleine-Budde wrote:
 On 12/09/2013 07:31 AM, Peter Chen wrote:
 After clear portsc.phcd, PHY needs 200us stable time for switch
 32K clock to AHB clock.

 If this is a general bugbix, please move it to the start of this series
 and add stable on Cc. I think I hit the bug on the second USB port of a
 custom MX28 board, the stmp_reset_block() in the USB phy will fail.
 
 Not a bug-fix, just the requirement for the PHY leaves low power mode.
 Does the bug exists with my this patchset, or just current mainline code?

I see, I found this strange thing on v3.8, but can try on v3.13-rc3.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 0/2] usb: chipidea: streamline regmap handling

[Marc Kleine-Budde]

On 12/06/2013 07:39 AM, Peter Chen wrote:
 On Thu, Dec 05, 2013 at 12:32:55PM +0100, Marc Kleine-Budde wrote:
 Hello Peter,

 these patches are for your ci-for-usb-next branch. The repmap handling is
 streamlined and the kernel shrinks by 706 bytes on i.MX28:

 add/remove: 0/0 grow/shrink: 0/30 up/down: 0/-706 (-706)
 function old new   delta
 hw_wait_reg  164 160  -4
 hw_port_test_get  24  20  -4
 hw_ep_flush  132 128  -4
 ep_enable392 388  -4
 ep_disable   252 248  -4
 ci_otg_work  248 244  -4
 ci_otg_role   24  20  -4
 ci_hdrc_host_init148 144  -4
 ci_hdrc_gadget_init  736 732  -4
 ci_handle_vbus_change 92  88  -4
 isr_setup_status_complete156 148  -8
 hw_port_test_set  64  56  -8
 host_start   360 352  -8
 ci_udc_pullup 84  76  -8
 ci_hdrc_remove   100  92  -8
 ci_udc_wakeup172 160 -12
 udc_id_switch_for_host76  60 -16
 udc_id_switch_for_device  80  64 -16
 ci_hdrc_otg_destroy   92  76 -16
 ci_hdrc_enter_lpm204 188 -16
 ci_irq   264 244 -20
 hw_device_state  120  96 -24
 _ep_queue.isra  10561028 -28
 ci_regs_nolpm 76  19 -57
 ci_regs_lpm   76  19 -57
 hw_device_reset  416 352 -64
 ep_set_halt  500 436 -64
 hw_alloc_regmap  204 136 -68
 ci_hdrc_probe   15121444 -68
 udc_irq 32363136-100

 
 Hi Marc,
 
 Thanks for doing that, would you explain more why the function
 code size can be smaller with your two patches?

In the functions one instruction (4 Bytes) is saved per register access.
When the array is embedded into the hw_bank there is one indirection less:

For example, the original code:
 870:   e591301cldr r3, [r1, #28]
 874:   e593302cldr r3, [r3, #44]   ; 0x2c
 878:   e5932000ldr r2, [r3]
 87c:   e5d13030ldrbr3, [r1, #48]   ; 0x30

With my first patch applied:
 7d8:   e5913048ldr r3, [r1, #72]   ; 0x48
 7dc:   e5932000ldr r2, [r3]
 7e0:   e5d130b4ldrbr3, [r1, #180]  ; 0xb4

With the second patch applied, we save some mem in ci_regs_nolpm and
ci_regs_lpm, because an entry only takes 1 Byte instead of 4.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 2/2] usb: chipidea: mark register map as const and convert to u8

[Marc Kleine-Budde]

On 12/06/2013 07:49 AM, Peter Chen wrote:
 On Thu, Dec 05, 2013 at 12:32:57PM +0100, Marc Kleine-Budde wrote:
 This patch makes the controller register map ci_regs_nolpm and ci_regs_lpm as
 const. Further, as all offset fit into a single byte, the type is changed
 from uintptr_t to u8.

 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 ---
  drivers/usb/chipidea/core.c | 80 
 ++---
  1 file changed, 40 insertions(+), 40 deletions(-)

 diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
 index 02929ee..dae16b3 100644
 --- a/drivers/usb/chipidea/core.c
 +++ b/drivers/usb/chipidea/core.c
 @@ -75,48 +75,48 @@
  #include otg.h
  
  /* Controller register map */
 -static uintptr_t ci_regs_nolpm[] = {
 -[CAP_CAPLENGTH] = 0x000UL,
 -[CAP_HCCPARAMS] = 0x008UL,
 -[CAP_DCCPARAMS] = 0x024UL,
 -[CAP_TESTMODE]  = 0x038UL,
 -[OP_USBCMD] = 0x000UL,
 -[OP_USBSTS] = 0x004UL,
 -[OP_USBINTR]= 0x008UL,
 -[OP_DEVICEADDR] = 0x014UL,
 -[OP_ENDPTLISTADDR]  = 0x018UL,
 -[OP_PORTSC] = 0x044UL,
 -[OP_DEVLC]  = 0x084UL,
 -[OP_OTGSC]  = 0x064UL,
 -[OP_USBMODE]= 0x068UL,
 -[OP_ENDPTSETUPSTAT] = 0x06CUL,
 -[OP_ENDPTPRIME] = 0x070UL,
 -[OP_ENDPTFLUSH] = 0x074UL,
 -[OP_ENDPTSTAT]  = 0x078UL,
 -[OP_ENDPTCOMPLETE]  = 0x07CUL,
 -[OP_ENDPTCTRL]  = 0x080UL,
 +static const u8 ci_regs_nolpm[] = {
 +[CAP_CAPLENGTH] = 0x00U,
 +[CAP_HCCPARAMS] = 0x08U,
 +[CAP_DCCPARAMS] = 0x24U,
 +[CAP_TESTMODE]  = 0x38U,
 +[OP_USBCMD] = 0x00U,
 +[OP_USBSTS] = 0x04U,
 +[OP_USBINTR]= 0x08U,
 +[OP_DEVICEADDR] = 0x14U,
 +[OP_ENDPTLISTADDR]  = 0x18U,
 +[OP_PORTSC] = 0x44U,
 +[OP_DEVLC]  = 0x84U,
 +[OP_OTGSC]  = 0x64U,
 +[OP_USBMODE]= 0x68U,
 +[OP_ENDPTSETUPSTAT] = 0x6CU,
 +[OP_ENDPTPRIME] = 0x70U,
 +[OP_ENDPTFLUSH] = 0x74U,
 +[OP_ENDPTSTAT]  = 0x78U,
 +[OP_ENDPTCOMPLETE]  = 0x7CU,
 +[OP_ENDPTCTRL]  = 0x80U,
  };
  
 -static uintptr_t ci_regs_lpm[] = {
 -[CAP_CAPLENGTH] = 0x000UL,
 -[CAP_HCCPARAMS] = 0x008UL,
 -[CAP_DCCPARAMS] = 0x024UL,
 -[CAP_TESTMODE]  = 0x0FCUL,
 -[OP_USBCMD] = 0x000UL,
 -[OP_USBSTS] = 0x004UL,
 -[OP_USBINTR]= 0x008UL,
 -[OP_DEVICEADDR] = 0x014UL,
 -[OP_ENDPTLISTADDR]  = 0x018UL,
 -[OP_PORTSC] = 0x044UL,
 -[OP_DEVLC]  = 0x084UL,
 -[OP_OTGSC]  = 0x0C4UL,
 -[OP_USBMODE]= 0x0C8UL,
 -[OP_ENDPTSETUPSTAT] = 0x0D8UL,
 -[OP_ENDPTPRIME] = 0x0DCUL,
 -[OP_ENDPTFLUSH] = 0x0E0UL,
 -[OP_ENDPTSTAT]  = 0x0E4UL,
 -[OP_ENDPTCOMPLETE]  = 0x0E8UL,
 -[OP_ENDPTCTRL]  = 0x0ECUL,
 +static const u8 ci_regs_lpm[] = {
 +[CAP_CAPLENGTH] = 0x00U,
 +[CAP_HCCPARAMS] = 0x08U,
 +[CAP_DCCPARAMS] = 0x24U,
 +[CAP_TESTMODE]  = 0xFCU,
 +[OP_USBCMD] = 0x00U,
 +[OP_USBSTS] = 0x04U,
 +[OP_USBINTR]= 0x08U,
 +[OP_DEVICEADDR] = 0x14U,
 +[OP_ENDPTLISTADDR]  = 0x18U,
 +[OP_PORTSC] = 0x44U,
 +[OP_DEVLC]  = 0x84U,
 +[OP_OTGSC]  = 0xC4U,
 +[OP_USBMODE]= 0xC8U,
 +[OP_ENDPTSETUPSTAT] = 0xD8U,
 +[OP_ENDPTPRIME] = 0xDCU,
 +[OP_ENDPTFLUSH] = 0xE0U,
 +[OP_ENDPTSTAT]  = 0xE4U,
 +[OP_ENDPTCOMPLETE]  = 0xE8U,
 +[OP_ENDPTCTRL]  = 0xECU,
  };
  
 
 Add const is a good fix, but I can't see much benefit of
 change uintptr_t to u8 except reduce the kernel data segment.
 Using uintprt_t make the code easier to read as the reader
 can know it stands for pointer.

No, it's not a pointer, it's an offset that added to the address __iomem
pointer later. And it should never be used directly, it only used to
setup the regmap.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

[PATCH v2 0/2] usb: chipidea: streamline regmap handling

[Marc Kleine-Budde]

Hello Peter,

these patches are for your ci-for-usb-next branch. The repmap handling is
streamlined and the kernel shrinks by 706 bytes on i.MX28. Moving the regmap
into hw_bank saves on indirection instruction per register access.

add/remove: 0/0 grow/shrink: 0/30 up/down: 0/-706 (-706)
function old new   delta
hw_wait_reg  164 160  -4
hw_port_test_get  24  20  -4
hw_ep_flush  132 128  -4
ep_enable392 388  -4
ep_disable   252 248  -4
ci_otg_work  248 244  -4
ci_otg_role   24  20  -4
ci_hdrc_host_init148 144  -4
ci_hdrc_gadget_init  736 732  -4
ci_handle_vbus_change 92  88  -4
isr_setup_status_complete156 148  -8
hw_port_test_set  64  56  -8
host_start   360 352  -8
ci_udc_pullup 84  76  -8
ci_hdrc_remove   100  92  -8
ci_udc_wakeup172 160 -12
udc_id_switch_for_host76  60 -16
udc_id_switch_for_device  80  64 -16
ci_hdrc_otg_destroy   92  76 -16
ci_hdrc_enter_lpm204 188 -16
ci_irq   264 244 -20
hw_device_state  120  96 -24
_ep_queue.isra  10561028 -28
ci_regs_nolpm 76  19 -57
ci_regs_lpm   76  19 -57
hw_device_reset  416 352 -64
ep_set_halt  500 436 -64
hw_alloc_regmap  204 136 -68
ci_hdrc_probe   15121444 -68
udc_irq 32363136-100

regards,
Marc

--
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 v2 2/2] usb: chipidea: mark register map as const and convert to u8

[Marc Kleine-Budde]

This patch makes the controller register map ci_regs_nolpm and ci_regs_lpm as
const. Further, as all offset fit into a single byte, the type is changed
from uintptr_t to u8.

Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
no changes since v1

 drivers/usb/chipidea/core.c | 80 ++---
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 02929ee..dae16b3 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -75,48 +75,48 @@
 #include otg.h
 
 /* Controller register map */
-static uintptr_t ci_regs_nolpm[] = {
-   [CAP_CAPLENGTH] = 0x000UL,
-   [CAP_HCCPARAMS] = 0x008UL,
-   [CAP_DCCPARAMS] = 0x024UL,
-   [CAP_TESTMODE]  = 0x038UL,
-   [OP_USBCMD] = 0x000UL,
-   [OP_USBSTS] = 0x004UL,
-   [OP_USBINTR]= 0x008UL,
-   [OP_DEVICEADDR] = 0x014UL,
-   [OP_ENDPTLISTADDR]  = 0x018UL,
-   [OP_PORTSC] = 0x044UL,
-   [OP_DEVLC]  = 0x084UL,
-   [OP_OTGSC]  = 0x064UL,
-   [OP_USBMODE]= 0x068UL,
-   [OP_ENDPTSETUPSTAT] = 0x06CUL,
-   [OP_ENDPTPRIME] = 0x070UL,
-   [OP_ENDPTFLUSH] = 0x074UL,
-   [OP_ENDPTSTAT]  = 0x078UL,
-   [OP_ENDPTCOMPLETE]  = 0x07CUL,
-   [OP_ENDPTCTRL]  = 0x080UL,
+static const u8 ci_regs_nolpm[] = {
+   [CAP_CAPLENGTH] = 0x00U,
+   [CAP_HCCPARAMS] = 0x08U,
+   [CAP_DCCPARAMS] = 0x24U,
+   [CAP_TESTMODE]  = 0x38U,
+   [OP_USBCMD] = 0x00U,
+   [OP_USBSTS] = 0x04U,
+   [OP_USBINTR]= 0x08U,
+   [OP_DEVICEADDR] = 0x14U,
+   [OP_ENDPTLISTADDR]  = 0x18U,
+   [OP_PORTSC] = 0x44U,
+   [OP_DEVLC]  = 0x84U,
+   [OP_OTGSC]  = 0x64U,
+   [OP_USBMODE]= 0x68U,
+   [OP_ENDPTSETUPSTAT] = 0x6CU,
+   [OP_ENDPTPRIME] = 0x70U,
+   [OP_ENDPTFLUSH] = 0x74U,
+   [OP_ENDPTSTAT]  = 0x78U,
+   [OP_ENDPTCOMPLETE]  = 0x7CU,
+   [OP_ENDPTCTRL]  = 0x80U,
 };
 
-static uintptr_t ci_regs_lpm[] = {
-   [CAP_CAPLENGTH] = 0x000UL,
-   [CAP_HCCPARAMS] = 0x008UL,
-   [CAP_DCCPARAMS] = 0x024UL,
-   [CAP_TESTMODE]  = 0x0FCUL,
-   [OP_USBCMD] = 0x000UL,
-   [OP_USBSTS] = 0x004UL,
-   [OP_USBINTR]= 0x008UL,
-   [OP_DEVICEADDR] = 0x014UL,
-   [OP_ENDPTLISTADDR]  = 0x018UL,
-   [OP_PORTSC] = 0x044UL,
-   [OP_DEVLC]  = 0x084UL,
-   [OP_OTGSC]  = 0x0C4UL,
-   [OP_USBMODE]= 0x0C8UL,
-   [OP_ENDPTSETUPSTAT] = 0x0D8UL,
-   [OP_ENDPTPRIME] = 0x0DCUL,
-   [OP_ENDPTFLUSH] = 0x0E0UL,
-   [OP_ENDPTSTAT]  = 0x0E4UL,
-   [OP_ENDPTCOMPLETE]  = 0x0E8UL,
-   [OP_ENDPTCTRL]  = 0x0ECUL,
+static const u8 ci_regs_lpm[] = {
+   [CAP_CAPLENGTH] = 0x00U,
+   [CAP_HCCPARAMS] = 0x08U,
+   [CAP_DCCPARAMS] = 0x24U,
+   [CAP_TESTMODE]  = 0xFCU,
+   [OP_USBCMD] = 0x00U,
+   [OP_USBSTS] = 0x04U,
+   [OP_USBINTR]= 0x08U,
+   [OP_DEVICEADDR] = 0x14U,
+   [OP_ENDPTLISTADDR]  = 0x18U,
+   [OP_PORTSC] = 0x44U,
+   [OP_DEVLC]  = 0x84U,
+   [OP_OTGSC]  = 0xC4U,
+   [OP_USBMODE]= 0xC8U,
+   [OP_ENDPTSETUPSTAT] = 0xD8U,
+   [OP_ENDPTPRIME] = 0xDCU,
+   [OP_ENDPTFLUSH] = 0xE0U,
+   [OP_ENDPTSTAT]  = 0xE4U,
+   [OP_ENDPTCOMPLETE]  = 0xE8U,
+   [OP_ENDPTCTRL]  = 0xECU,
 };
 
 static int hw_alloc_regmap(struct ci_hdrc *ci, bool is_lpm)
-- 
1.8.4.3

--
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 v2 1/2] usb: chipidea: move malloced regmap directly into struct hw_bank

[Marc Kleine-Budde]

Without this patch a seperate chunk of memory is allocated for the regmap
array. As the regmap is always used it makes no sense to allocate a seperate
memory block for it, this patch moves the regmap array directly into the struct
hw_bank.

Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
changes since v1:
- fix regmap array size (tnx Peter)
- remove REG_BITS define

 drivers/usb/chipidea/ci.h   | 63 +
 drivers/usb/chipidea/core.c |  8 --
 2 files changed, 30 insertions(+), 41 deletions(-)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 1c94fc5..a71dc1c 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -26,6 +26,35 @@
 #define ENDPT_MAX  32
 
 /**
+ * REGISTERS
+ */
+/* register indices */
+enum ci_hw_regs {
+   CAP_CAPLENGTH,
+   CAP_HCCPARAMS,
+   CAP_DCCPARAMS,
+   CAP_TESTMODE,
+   CAP_LAST = CAP_TESTMODE,
+   OP_USBCMD,
+   OP_USBSTS,
+   OP_USBINTR,
+   OP_DEVICEADDR,
+   OP_ENDPTLISTADDR,
+   OP_PORTSC,
+   OP_DEVLC,
+   OP_OTGSC,
+   OP_USBMODE,
+   OP_ENDPTSETUPSTAT,
+   OP_ENDPTPRIME,
+   OP_ENDPTFLUSH,
+   OP_ENDPTSTAT,
+   OP_ENDPTCOMPLETE,
+   OP_ENDPTCTRL,
+   /* endptctrl1..15 follow */
+   OP_LAST = OP_ENDPTCTRL + ENDPT_MAX / 2,
+};
+
+/**
  * STRUCTURES
  */
 /**
@@ -98,7 +127,7 @@ struct hw_bank {
void __iomem*cap;
void __iomem*op;
size_t  size;
-   void __iomem**regmap;
+   void __iomem*regmap[OP_LAST + 1];
 };
 
 /**
@@ -209,38 +238,6 @@ static inline void ci_role_stop(struct ci_hdrc *ci)
ci-roles[role]-stop(ci);
 }
 
-/**
- * REGISTERS
- */
-/* register size */
-#define REG_BITS   (32)
-
-/* register indices */
-enum ci_hw_regs {
-   CAP_CAPLENGTH,
-   CAP_HCCPARAMS,
-   CAP_DCCPARAMS,
-   CAP_TESTMODE,
-   CAP_LAST = CAP_TESTMODE,
-   OP_USBCMD,
-   OP_USBSTS,
-   OP_USBINTR,
-   OP_DEVICEADDR,
-   OP_ENDPTLISTADDR,
-   OP_PORTSC,
-   OP_DEVLC,
-   OP_OTGSC,
-   OP_USBMODE,
-   OP_ENDPTSETUPSTAT,
-   OP_ENDPTPRIME,
-   OP_ENDPTFLUSH,
-   OP_ENDPTSTAT,
-   OP_ENDPTCOMPLETE,
-   OP_ENDPTCTRL,
-   /* endptctrl1..15 follow */
-   OP_LAST = OP_ENDPTCTRL + ENDPT_MAX / 2,
-};
-
 /**
  * hw_read: reads from a hw register
  * @reg:  register index
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 5075407..02929ee 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -123,13 +123,6 @@ static int hw_alloc_regmap(struct ci_hdrc *ci, bool is_lpm)
 {
int i;
 
-   kfree(ci-hw_bank.regmap);
-
-   ci-hw_bank.regmap = kzalloc((OP_LAST + 1) * sizeof(void *),
-GFP_KERNEL);
-   if (!ci-hw_bank.regmap)
-   return -ENOMEM;
-
for (i = 0; i  OP_ENDPTCTRL; i++)
ci-hw_bank.regmap[i] =
(i = CAP_LAST ? ci-hw_bank.cap : ci-hw_bank.op) +
@@ -677,7 +670,6 @@ static int ci_hdrc_remove(struct platform_device *pdev)
ci_role_destroy(ci);
ci_hdrc_enter_lpm(ci, true);
ci_usb_phy_destroy(ci);
-   kfree(ci-hw_bank.regmap);
 
return 0;
 }
-- 
1.8.4.3

--
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/2] usb: chipidea: move malloced regmap directly into struct hw_bank

[Marc Kleine-Budde]

Without this patch a seperate chunk of memory is allocated for the regmap
array. As the regmap is always used it makes no sense to allocate a seperate
memory block for it, this patch moves the regmap array directly into the struct
hw_bank.

Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 drivers/usb/chipidea/ci.h   | 66 ++---
 drivers/usb/chipidea/core.c |  8 --
 2 files changed, 33 insertions(+), 41 deletions(-)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 1c94fc5..ef99d91 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -26,6 +26,38 @@
 #define ENDPT_MAX  32
 
 /**
+ * REGISTERS
+ */
+/* register size */
+#define REG_BITS   (32)
+
+/* register indices */
+enum ci_hw_regs {
+   CAP_CAPLENGTH,
+   CAP_HCCPARAMS,
+   CAP_DCCPARAMS,
+   CAP_TESTMODE,
+   CAP_LAST = CAP_TESTMODE,
+   OP_USBCMD,
+   OP_USBSTS,
+   OP_USBINTR,
+   OP_DEVICEADDR,
+   OP_ENDPTLISTADDR,
+   OP_PORTSC,
+   OP_DEVLC,
+   OP_OTGSC,
+   OP_USBMODE,
+   OP_ENDPTSETUPSTAT,
+   OP_ENDPTPRIME,
+   OP_ENDPTFLUSH,
+   OP_ENDPTSTAT,
+   OP_ENDPTCOMPLETE,
+   OP_ENDPTCTRL,
+   /* endptctrl1..15 follow */
+   OP_LAST = OP_ENDPTCTRL + ENDPT_MAX / 2,
+};
+
+/**
  * STRUCTURES
  */
 /**
@@ -98,7 +130,7 @@ struct hw_bank {
void __iomem*cap;
void __iomem*op;
size_t  size;
-   void __iomem**regmap;
+   void __iomem*regmap[OP_LAST];
 };
 
 /**
@@ -209,38 +241,6 @@ static inline void ci_role_stop(struct ci_hdrc *ci)
ci-roles[role]-stop(ci);
 }
 
-/**
- * REGISTERS
- */
-/* register size */
-#define REG_BITS   (32)
-
-/* register indices */
-enum ci_hw_regs {
-   CAP_CAPLENGTH,
-   CAP_HCCPARAMS,
-   CAP_DCCPARAMS,
-   CAP_TESTMODE,
-   CAP_LAST = CAP_TESTMODE,
-   OP_USBCMD,
-   OP_USBSTS,
-   OP_USBINTR,
-   OP_DEVICEADDR,
-   OP_ENDPTLISTADDR,
-   OP_PORTSC,
-   OP_DEVLC,
-   OP_OTGSC,
-   OP_USBMODE,
-   OP_ENDPTSETUPSTAT,
-   OP_ENDPTPRIME,
-   OP_ENDPTFLUSH,
-   OP_ENDPTSTAT,
-   OP_ENDPTCOMPLETE,
-   OP_ENDPTCTRL,
-   /* endptctrl1..15 follow */
-   OP_LAST = OP_ENDPTCTRL + ENDPT_MAX / 2,
-};
-
 /**
  * hw_read: reads from a hw register
  * @reg:  register index
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 5075407..02929ee 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -123,13 +123,6 @@ static int hw_alloc_regmap(struct ci_hdrc *ci, bool is_lpm)
 {
int i;
 
-   kfree(ci-hw_bank.regmap);
-
-   ci-hw_bank.regmap = kzalloc((OP_LAST + 1) * sizeof(void *),
-GFP_KERNEL);
-   if (!ci-hw_bank.regmap)
-   return -ENOMEM;
-
for (i = 0; i  OP_ENDPTCTRL; i++)
ci-hw_bank.regmap[i] =
(i = CAP_LAST ? ci-hw_bank.cap : ci-hw_bank.op) +
@@ -677,7 +670,6 @@ static int ci_hdrc_remove(struct platform_device *pdev)
ci_role_destroy(ci);
ci_hdrc_enter_lpm(ci, true);
ci_usb_phy_destroy(ci);
-   kfree(ci-hw_bank.regmap);
 
return 0;
 }
-- 
1.8.4.3

--
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 2/2] usb: chipidea: mark register map as const and convert to u8

[Marc Kleine-Budde]

This patch makes the controller register map ci_regs_nolpm and ci_regs_lpm as
const. Further, as all offset fit into a single byte, the type is changed
from uintptr_t to u8.

Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 drivers/usb/chipidea/core.c | 80 ++---
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 02929ee..dae16b3 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -75,48 +75,48 @@
 #include otg.h
 
 /* Controller register map */
-static uintptr_t ci_regs_nolpm[] = {
-   [CAP_CAPLENGTH] = 0x000UL,
-   [CAP_HCCPARAMS] = 0x008UL,
-   [CAP_DCCPARAMS] = 0x024UL,
-   [CAP_TESTMODE]  = 0x038UL,
-   [OP_USBCMD] = 0x000UL,
-   [OP_USBSTS] = 0x004UL,
-   [OP_USBINTR]= 0x008UL,
-   [OP_DEVICEADDR] = 0x014UL,
-   [OP_ENDPTLISTADDR]  = 0x018UL,
-   [OP_PORTSC] = 0x044UL,
-   [OP_DEVLC]  = 0x084UL,
-   [OP_OTGSC]  = 0x064UL,
-   [OP_USBMODE]= 0x068UL,
-   [OP_ENDPTSETUPSTAT] = 0x06CUL,
-   [OP_ENDPTPRIME] = 0x070UL,
-   [OP_ENDPTFLUSH] = 0x074UL,
-   [OP_ENDPTSTAT]  = 0x078UL,
-   [OP_ENDPTCOMPLETE]  = 0x07CUL,
-   [OP_ENDPTCTRL]  = 0x080UL,
+static const u8 ci_regs_nolpm[] = {
+   [CAP_CAPLENGTH] = 0x00U,
+   [CAP_HCCPARAMS] = 0x08U,
+   [CAP_DCCPARAMS] = 0x24U,
+   [CAP_TESTMODE]  = 0x38U,
+   [OP_USBCMD] = 0x00U,
+   [OP_USBSTS] = 0x04U,
+   [OP_USBINTR]= 0x08U,
+   [OP_DEVICEADDR] = 0x14U,
+   [OP_ENDPTLISTADDR]  = 0x18U,
+   [OP_PORTSC] = 0x44U,
+   [OP_DEVLC]  = 0x84U,
+   [OP_OTGSC]  = 0x64U,
+   [OP_USBMODE]= 0x68U,
+   [OP_ENDPTSETUPSTAT] = 0x6CU,
+   [OP_ENDPTPRIME] = 0x70U,
+   [OP_ENDPTFLUSH] = 0x74U,
+   [OP_ENDPTSTAT]  = 0x78U,
+   [OP_ENDPTCOMPLETE]  = 0x7CU,
+   [OP_ENDPTCTRL]  = 0x80U,
 };
 
-static uintptr_t ci_regs_lpm[] = {
-   [CAP_CAPLENGTH] = 0x000UL,
-   [CAP_HCCPARAMS] = 0x008UL,
-   [CAP_DCCPARAMS] = 0x024UL,
-   [CAP_TESTMODE]  = 0x0FCUL,
-   [OP_USBCMD] = 0x000UL,
-   [OP_USBSTS] = 0x004UL,
-   [OP_USBINTR]= 0x008UL,
-   [OP_DEVICEADDR] = 0x014UL,
-   [OP_ENDPTLISTADDR]  = 0x018UL,
-   [OP_PORTSC] = 0x044UL,
-   [OP_DEVLC]  = 0x084UL,
-   [OP_OTGSC]  = 0x0C4UL,
-   [OP_USBMODE]= 0x0C8UL,
-   [OP_ENDPTSETUPSTAT] = 0x0D8UL,
-   [OP_ENDPTPRIME] = 0x0DCUL,
-   [OP_ENDPTFLUSH] = 0x0E0UL,
-   [OP_ENDPTSTAT]  = 0x0E4UL,
-   [OP_ENDPTCOMPLETE]  = 0x0E8UL,
-   [OP_ENDPTCTRL]  = 0x0ECUL,
+static const u8 ci_regs_lpm[] = {
+   [CAP_CAPLENGTH] = 0x00U,
+   [CAP_HCCPARAMS] = 0x08U,
+   [CAP_DCCPARAMS] = 0x24U,
+   [CAP_TESTMODE]  = 0xFCU,
+   [OP_USBCMD] = 0x00U,
+   [OP_USBSTS] = 0x04U,
+   [OP_USBINTR]= 0x08U,
+   [OP_DEVICEADDR] = 0x14U,
+   [OP_ENDPTLISTADDR]  = 0x18U,
+   [OP_PORTSC] = 0x44U,
+   [OP_DEVLC]  = 0x84U,
+   [OP_OTGSC]  = 0xC4U,
+   [OP_USBMODE]= 0xC8U,
+   [OP_ENDPTSETUPSTAT] = 0xD8U,
+   [OP_ENDPTPRIME] = 0xDCU,
+   [OP_ENDPTFLUSH] = 0xE0U,
+   [OP_ENDPTSTAT]  = 0xE4U,
+   [OP_ENDPTCOMPLETE]  = 0xE8U,
+   [OP_ENDPTCTRL]  = 0xECU,
 };
 
 static int hw_alloc_regmap(struct ci_hdrc *ci, bool is_lpm)
-- 
1.8.4.3

--
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 4/7] usb: chipidea: add freescale imx28 special write register method

[Marc Kleine-Budde]

On 12/05/2013 01:16 PM, Peter Chen wrote:
 Personally I think you should include the text of the errata
 in one of the files.

 Yes, some may be interested in detail, not only the solution,
 will do it next time.
 
 I'm not suggesting you change the guts of the patch, but...

 Having read the errata it isn't 100% clear to me that using SWP
 is necessary for all register accesses.
 
 Only write USB controller register is needed.
 
 Since SWP is a locked bus cycle it will have performance penalties,
 depending on the number of write operations this might be measurable.

 AFAICT the reason that SWP works is that does a read cycle prior
 to the write that ensures that logic is all in the correct state
 (I think the read returns garbage).
 There is no requirement for a locked bus cycle, or for the cycles
 to be adjacent (from the cpu's point of view). Just that there
 can be no other cycles on what I assume is a peripheral bus.

 So if the code can guarantee no other cycles on the same bus (the
 ones that are documented in the errata to cause problems) then
 there is no need to use the SWP instruction.
 Typically this would be single cpu systems doing a series of
 accesses (the first could be a read) with interrupts hard disabled.

 
 Thanks for your analysis.
 
 As far as I know, the workaround for this problem is it needs a read
 usb controller register operation before every write, the swp instruction
 may be the easiest way to implement it, so the design team suggests it,
 this patch just implements that workaround.
 
 We can't guarantee there is no other access on the AHB without disable 
 interrupt, would you think below operation will be more efficient than swp,
 or any better solutions?
 
 local_irq_save(flags);
 readl(addr);
 writel(val, addr);
 local_irq_restore(flags);

Does this work without modifications on PREEMPT_RT?

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v3 10/10] usb: chipidea: imx: Enable CI_HDRC_IMX_EHCI_QUIRK

[Marc Kleine-Budde]

On 12/04/2013 02:40 AM, Peter Chen wrote:
 There are some re-work for this serial, I will do it.
 But I don't get your mean why it needs to squash into another patch?

Let's look at the code again:

 - if (imx_platform_flag-flags  CI_HDRC_IMX_SUPPORT_RUNTIME_PM)
 + if (imx_platform_flag-flags  CI_HDRC_IMX_SUPPORT_RUNTIME_PM) {
   pdata.flags |= CI_HDRC_SUPPORTS_RUNTIME_PM;
   data-supports_runtime_pm = true;
   }

Without that patch, the driver will not compile.

before:

if ()
/* code */
}

after:

if () {
/* code */
}

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 09/17] usb: phy-mxs: Enable IC fixes for related SoCs

[Marc Kleine-Budde]

On 12/03/2013 08:37 AM, Peter Chen wrote:
 Some PHY bugs are fixed by IC logic, but these bits are not
 enabled by default, so we enable them at driver.
 
 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---
  drivers/usb/phy/phy-mxs-usb.c |   20 
  1 files changed, 20 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
 index 8738890..0908d74 100644
 --- a/drivers/usb/phy/phy-mxs-usb.c
 +++ b/drivers/usb/phy/phy-mxs-usb.c
 @@ -31,6 +31,10 @@
  #define HW_USBPHY_CTRL_SET   0x34
  #define HW_USBPHY_CTRL_CLR   0x38
  
 +#define HW_USBPHY_IP 0x90
 +#define HW_USBPHY_IP_SET 0x94
 +#define HW_USBPHY_IP_CLR 0x98
 +
  #define BM_USBPHY_CTRL_SFTRSTBIT(31)
  #define BM_USBPHY_CTRL_CLKGATE   BIT(30)
  #define BM_USBPHY_CTRL_ENAUTOSET_USBCLKS BIT(26)
 @@ -42,6 +46,8 @@
  #define BM_USBPHY_CTRL_ENUTMILEVEL2  BIT(14)
  #define BM_USBPHY_CTRL_ENHOSTDISCONDETECTBIT(1)
  
 +#define BM_USBPHY_IP_FIX   (BIT(17) | BIT(18))
 +
  #define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
  
  /* Do disconnection between PHY and controller without vbus */
 @@ -97,6 +103,16 @@ struct mxs_phy {
   struct regmap *regmap_anatop;
  };
  
 +static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
 +{
 + return mxs_phy-data == imx6q_phy_data;
 +}
 +
 +static inline bool is_imx6sl_phy(struct mxs_phy *mxs_phy)
 +{
 + return mxs_phy-data == imx6sl_phy_data;
 +}

Why don't you use a a BIT() here as in Patch 2/17, too?

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 16/17] usb: phy-mxs: fix the problem by only using 1st controller's register

[Marc Kleine-Budde]

On 12/03/2013 08:37 AM, Peter Chen wrote:
 We fix the problem that we only use the 1st controller's related
 registers at mxs_phy_disconnect_line, but in fact, it needs to
 access registers according to different PHYs.

Are you fixing the code that has been added in this series before? If
so, please squash into the patch. Don't add the broken code in the first
place.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 02/17] usb: phy-mxs: Add platform judgement code

[Marc Kleine-Budde]

On 12/03/2013 08:36 AM, Peter Chen wrote:
 The mxs-phy has several bugs and features at different
 versions, the driver code can get it through of_device_id.data.
 
 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---
  drivers/usb/phy/phy-mxs-usb.c |   58 ++--
  1 files changed, 49 insertions(+), 9 deletions(-)
 
 diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
 index 545844b..f6cbc78 100644
 --- a/drivers/usb/phy/phy-mxs-usb.c
 +++ b/drivers/usb/phy/phy-mxs-usb.c
 @@ -1,5 +1,5 @@
  /*
 - * Copyright 2012 Freescale Semiconductor, Inc.
 + * Copyright 2012-2013 Freescale Semiconductor, Inc.
   * Copyright (C) 2012 Marek Vasut ma...@denx.de
   * on behalf of DENX Software Engineering GmbH
   *
 @@ -20,6 +20,7 @@
  #include linux/delay.h
  #include linux/err.h
  #include linux/io.h
 +#include linux/of_device.h
  
  #define DRIVER_NAME mxs_phy
  
 @@ -34,13 +35,55 @@
  #define BM_USBPHY_CTRL_ENUTMILEVEL2  BIT(14)
  #define BM_USBPHY_CTRL_ENHOSTDISCONDETECTBIT(1)
  
 +#define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
 +
 +/* Do disconnection between PHY and controller without vbus */
 +#define MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS BIT(0)
 +
 +/*
 + * The PHY will be in messy if there is an wakeup after putting
   ^^
   a
Do you mean messy state?

 + * bus to suspend (set portsc.suspendM) but before setting PHY to low
 + * power mode (set portsc.phcd).
 + */

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 09/17] usb: phy-mxs: Enable IC fixes for related SoCs

[Marc Kleine-Budde]

On 12/03/2013 09:38 AM, Peter Chen wrote:
  

 On 12/03/2013 08:37 AM, Peter Chen wrote:
 Some PHY bugs are fixed by IC logic, but these bits are not
 enabled by default, so we enable them at driver.

 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---
  drivers/usb/phy/phy-mxs-usb.c |   20 
  1 files changed, 20 insertions(+), 0 deletions(-)

 diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-
 usb.c
 index 8738890..0908d74 100644
 --- a/drivers/usb/phy/phy-mxs-usb.c
 +++ b/drivers/usb/phy/phy-mxs-usb.c
 @@ -31,6 +31,10 @@
  #define HW_USBPHY_CTRL_SET 0x34
  #define HW_USBPHY_CTRL_CLR 0x38

 +#define HW_USBPHY_IP   0x90
 +#define HW_USBPHY_IP_SET   0x94
 +#define HW_USBPHY_IP_CLR   0x98
 +
  #define BM_USBPHY_CTRL_SFTRST  BIT(31)
  #define BM_USBPHY_CTRL_CLKGATE BIT(30)
  #define BM_USBPHY_CTRL_ENAUTOSET_USBCLKS   BIT(26)
 @@ -42,6 +46,8 @@
  #define BM_USBPHY_CTRL_ENUTMILEVEL2BIT(14)
  #define BM_USBPHY_CTRL_ENHOSTDISCONDETECT  BIT(1)

 +#define BM_USBPHY_IP_FIX   (BIT(17) | BIT(18))
 +
  #define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)

  /* Do disconnection between PHY and controller without vbus */
 @@ -97,6 +103,16 @@ struct mxs_phy {
 struct regmap *regmap_anatop;
  };

 +static inline bool is_imx6q_phy(struct mxs_phy *mxs_phy)
 +{
 +   return mxs_phy-data == imx6q_phy_data;
 +}
 +
 +static inline bool is_imx6sl_phy(struct mxs_phy *mxs_phy)
 +{
 +   return mxs_phy-data == imx6sl_phy_data;
 +}

 Why don't you use a a BIT() here as in Patch 2/17, too?

  
 Thanks, I will define the two PHY problems as the register bit
 position at 2/17, and use those two MACROs at this patch.
 Is it your point?

I was wondering if the driver looks more uniform if you use something
like this:

#define MXS_PHY_NEED_IP_FIX BIT(3)

  static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
  {
   int ret;
 @@ -123,6 +139,10 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
   BM_USBPHY_CTRL_ENUTMILEVEL3,
  base + HW_USBPHY_CTRL_SET);
  
 + /* Enable IC solution */
 + if (is_imx6q_phy(mxs_phy) || is_imx6sl_phy(mxs_phy))

if (mxs_phy-data-flags  MXS_PHY_NEED_IP_FIX)

 + writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
 +
   return 0;
  }

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 17/17] usb: phy-mxs: do not set PWD.RXPWD1PT1 for low speed connection

[Marc Kleine-Budde]

On 12/03/2013 08:37 AM, Peter Chen wrote:
 At very rare cases, the SoF will not send out after resume with
 low speed connection. The workaround is do not power down
 PWD.RXPWD1PT1 bit during the suspend.

Is this also a fix for newly added code? If so please also squash.

 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---
  drivers/usb/phy/phy-mxs-usb.c |   47 
 -
  1 files changed, 46 insertions(+), 1 deletions(-)
 
 diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
 index 542b6ec..5ae4a57 100644
 --- a/drivers/usb/phy/phy-mxs-usb.c
 +++ b/drivers/usb/phy/phy-mxs-usb.c
 @@ -69,6 +69,9 @@
  #define ANADIG_USB2_LOOPBACK_SET 0x244
  #define ANADIG_USB2_LOOPBACK_CLR 0x248
  
 +#define ANADIG_USB1_MISC 0x1f0
 +#define ANADIG_USB2_MISC 0x250
 +
  #define BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG BIT(12)
  #define BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG_SL BIT(11)
  
 @@ -80,6 +83,11 @@
  #define BM_ANADIG_USB2_LOOPBACK_UTMI_DIG_TST1BIT(2)
  #define BM_ANADIG_USB2_LOOPBACK_TSTI_TX_EN   BIT(5)
  
 +#define BM_ANADIG_USB1_MISC_RX_VPIN_FS   BIT(29)
 +#define BM_ANADIG_USB1_MISC_RX_VMIN_FS   BIT(28)
 +#define BM_ANADIG_USB2_MISC_RX_VPIN_FS   BIT(29)
 +#define BM_ANADIG_USB2_MISC_RX_VMIN_FS   BIT(28)
 +
  #define to_mxs_phy(p) container_of((p), struct mxs_phy, phy)
  
  /* Do disconnection between PHY and controller without vbus */
 @@ -296,12 +304,49 @@ static void mxs_phy_shutdown(struct usb_phy *phy)
   clk_disable_unprepare(mxs_phy-clk);
  }
  
 +static bool mxs_phy_is_low_speed_connection(struct mxs_phy *mxs_phy)
 +{
 + unsigned int line_state;
 + /* bit definition is the same for all controllers */
 + unsigned int dp_bit = BM_ANADIG_USB1_MISC_RX_VPIN_FS,
 +  dm_bit = BM_ANADIG_USB1_MISC_RX_VMIN_FS;
 + unsigned int reg = ANADIG_USB1_MISC;
 +
 + /* If the SoCs don't have anatop, quit */
 + if (!mxs_phy-regmap_anatop)
 + return false;
 +
 + if (mxs_phy-port_id == 0)
 + reg = ANADIG_USB1_MISC;
 + else if (mxs_phy-port_id == 1)
 + reg = ANADIG_USB2_MISC;
 +
 + regmap_read(mxs_phy-regmap_anatop, reg, line_state);
 +
 + if ((line_state  (dp_bit | dm_bit)) ==  dm_bit)
 + return true;
 + else
 + return false;
 +}
 +
  static int mxs_phy_suspend(struct usb_phy *x, int suspend)
  {
   struct mxs_phy *mxs_phy = to_mxs_phy(x);
 + bool low_speed_connection, vbus_is_on;
 +
 + low_speed_connection = mxs_phy_is_low_speed_connection(mxs_phy);
 + vbus_is_on = mxs_phy_get_vbus_status(mxs_phy);
  
   if (suspend) {
 - writel(0x, x-io_priv + HW_USBPHY_PWD);
 + /*
 +  * FIXME: Do not power down RXPWD1PT1 bit for low speed

Is this FIXME still true?

 +  * connect. The low speed connection will have problem at
 +  * very rare cases during usb suspend and resume process.
 +  */
 + if (low_speed_connection  vbus_is_on)
 + writel(0xfffb, x-io_priv + HW_USBPHY_PWD);
 + else
 + writel(0x, x-io_priv + HW_USBPHY_PWD);
   writel(BM_USBPHY_CTRL_CLKGATE,
  x-io_priv + HW_USBPHY_CTRL_SET);
   clk_disable_unprepare(mxs_phy-clk);
 

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 17/17] usb: phy-mxs: do not set PWD.RXPWD1PT1 for low speed connection

[Marc Kleine-Budde]

On 12/03/2013 10:19 AM, Peter Chen wrote:
 On 12/03/2013 08:37 AM, Peter Chen wrote:
 At very rare cases, the SoF will not send out after resume with
 low speed connection. The workaround is do not power down
 PWD.RXPWD1PT1 bit during the suspend.

 Is this also a fix for newly added code? If so please also squash.

 No, it is a workaround for new problem.

If this would be a fix, it's better to put this as first patch into a
series, so that it can be applied to the stable trees easier.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/3 v4] usb: chipidea: Reallocate regmap only if lpm is detected

[Marc Kleine-Budde]

On 12/03/2013 09:01 AM, Chris Ruehl wrote:
 usb: chipidea: Reallocate regmap only if lpm is detected
 
 The regmap only needs to reallocate if the hw_read on the CAP register shows
 lpm is used. Therefore the if() statement check the change.
 
 Signed-off-by: Chris Ruehl chris.ru...@gtsys.com.hk
 Acked-by: Peter Chen peter.c...@freescale.com
 ---
  drivers/usb/chipidea/core.c |7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
 index 5d8981c..9a5ef20 100644
 --- a/drivers/usb/chipidea/core.c
 +++ b/drivers/usb/chipidea/core.c
 @@ -208,7 +208,8 @@ static int hw_device_init(struct ci_hdrc *ci, void 
 __iomem *base)
   reg = hw_read(ci, CAP_HCCPARAMS, HCCPARAMS_LEN) 
   __ffs(HCCPARAMS_LEN);
   ci-hw_bank.lpm  = reg;
 - hw_alloc_regmap(ci, !!reg);
 + if (reg)
 + hw_alloc_regmap(ci, !!reg);
   ci-hw_bank.size = ci-hw_bank.op - ci-hw_bank.abs;
   ci-hw_bank.size += OP_LAST;
   ci-hw_bank.size /= sizeof(u32);
 @@ -642,6 +643,10 @@ static int ci_hdrc_probe(struct platform_device *pdev)
   : CI_ROLE_GADGET;
   }
  
 + /* only update vbus status for peripheral */
 + if (ci-role == CI_ROLE_GADGET)
 + ci_handle_vbus_change(ci);
 +

This change seems unrelated to me.

Marc

   ret = ci_role_start(ci, ci-role);
   if (ret) {
   dev_err(dev, can't start %s role\n, ci_role(ci)-name);
 


-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] USB: switch maintainership of chipidea to Peter

[Marc Kleine-Budde]

On 12/03/2013 06:21 AM, Peter Chen wrote:
 On Mon, Dec 02, 2013 at 07:38:46PM -0800, Greg KH wrote:
 On Tue, Dec 03, 2013 at 11:00:30AM +0800, Peter Chen wrote:
 On Mon, Dec 02, 2013 at 03:43:48PM -0800, Greg KH wrote:
 From: Greg Kroah-Hartman gre...@linuxfoundation.org

 Alexander isn't able to maintain the Chipidea code anymore, and as Peter
 has been acting as the de-facto maintainer anyway, make it official.

 Cc: Alexander Shishkin alexander.shish...@linux.intel.com
 Cc: Peter Chen peter.c...@freescale.com
 Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org

 ---

 Alex and Peter, can I get your signed-off-by: or ack for this patch?

 diff --git a/MAINTAINERS b/MAINTAINERS
 index 4afcfb4c..e3e79afd 100644
 --- a/MAINTAINERS
 +++ b/MAINTAINERS
 @@ -2137,7 +2137,7 @@ S:   Maintained
  F:Documentation/zh_CN/
  
  CHIPIDEA USB HIGH SPEED DUAL ROLE CONTROLLER
 -M:Alexander Shishkin alexander.shish...@linux.intel.com
 +M:Peter Chen peter.c...@freescale.com
  L:linux-usb@vger.kernel.org
  S:Maintained
  F:drivers/usb/chipidea/
 --

 Acked-by: Peter Chen peter.c...@freescale.com

FWIW:
Acked-by: Marc Kleine-Budde m...@pengutronix.de


 Greg, below is my git tree, please help add it, thanks.
 T:  git://github.com/hzpeterchen/linux-usb.git

 Ugh, github, really?  I'm not going to pull from a github tree, sorry.
 Can you apply for a kernel.org one?

 Or you can just send patches, which is probably good for a while anyway.

 
 Thanks, I will use the way which send patches to you directly.
 The github can be used for someone who wants to work based on
 the newest chipidea code.
 
 I will collect chipidea dev patches for 3.14, and bug-fixes for 3.13-rcX.

Great! Do you have the names for those branches, yet? I'm especially
interested in branches for usb and usb-next, which will have only
final patches applied. So that these trees don't need to be rebased.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] USB: switch maintainership of chipidea to Peter

[Marc Kleine-Budde]

On 12/03/2013 04:38 AM, Greg KH wrote:
 On Tue, Dec 03, 2013 at 11:00:30AM +0800, Peter Chen wrote:
 On Mon, Dec 02, 2013 at 03:43:48PM -0800, Greg KH wrote:
 From: Greg Kroah-Hartman gre...@linuxfoundation.org

 Alexander isn't able to maintain the Chipidea code anymore, and as Peter
 has been acting as the de-facto maintainer anyway, make it official.

 Cc: Alexander Shishkin alexander.shish...@linux.intel.com
 Cc: Peter Chen peter.c...@freescale.com
 Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org

 ---

 Alex and Peter, can I get your signed-off-by: or ack for this patch?

 diff --git a/MAINTAINERS b/MAINTAINERS
 index 4afcfb4c..e3e79afd 100644
 --- a/MAINTAINERS
 +++ b/MAINTAINERS
 @@ -2137,7 +2137,7 @@ S:Maintained
  F: Documentation/zh_CN/
  
  CHIPIDEA USB HIGH SPEED DUAL ROLE CONTROLLER
 -M: Alexander Shishkin alexander.shish...@linux.intel.com
 +M: Peter Chen peter.c...@freescale.com
  L: linux-usb@vger.kernel.org
  S: Maintained
  F: drivers/usb/chipidea/
 --

 Acked-by: Peter Chen peter.c...@freescale.com

 Greg, below is my git tree, please help add it, thanks.
 T:   git://github.com/hzpeterchen/linux-usb.git
 
 Ugh, github, really?  I'm not going to pull from a github tree, sorry.
 Can you apply for a kernel.org one?

github can be used without all that fancy web-2.0 interface. If Peter
sends you a standard plain $(git request-pull) it's not much different
from a non github git account.

just my 2¢,
Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] USB: switch maintainership of chipidea to Peter

[Marc Kleine-Budde]

On 12/03/2013 02:54 PM, Peter Chen wrote:
 Great! Do you have the names for those branches, yet? I'm especially
 interested in branches for usb and usb-next, which will have only
 final patches applied. So that these trees don't need to be rebased.

 I plan to create branch ci-for-usb-next which is rebased on Greg's usb-next 
 tree,
 branch ci-for-usb-stable which is rebased on Greg's usb-linus, branch 
 ci-for-usb-dev
 for chipidea development patch. I hope all related patches will
 be there within one week.

The less rebasing the better for any downstream developer.

From my point of view you can already queue the series reescale imx28
special write register method and the patch imx: set
CI_HDRC_IMX28_WRITE_FIX for imx28 for v3.13-rcX.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] USB: switch maintainership of chipidea to Peter

[Marc Kleine-Budde]

On 12/03/2013 03:21 PM, Greg KH wrote:
 Ugh, github, really?  I'm not going to pull from a github tree, sorry.
 Can you apply for a kernel.org one?

 github can be used without all that fancy web-2.0 interface.
 
 I know, I use it all the time, even with that interface, that's not what
 I'm objecting to.
 
 If Peter sends you a standard plain $(git request-pull) it's not much
 different from a non github git account.
 
 We don't take kernel pulls from github accounts, that is why we have
 git.kernel.org for, which is authenticated a bit more than random
 github accounts.

Thanks for the clarification.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v6 0/3] usb: ehci+chipidea: fix imx28 ENGR119653 USB: ARM to USB register error issue

[Marc Kleine-Budde]

On 11/26/2013 01:11 PM, Peter Chen wrote:
 Changes for v6:
 - Add volatile for arm swp instruction, since we need io access
 to fix this problem
 - Add kernel style comment for imx28_write_fix
 
 Peter Chen (3):
   usb: ehci: add freescale imx28 special write register method
   usb: chipidea: add freescale imx28 special write register method
   usb: chipidea: imx: set CI_HDRC_IMX28_WRITE_FIX for imx28
 
  drivers/usb/chipidea/ci.h  |   26 --
  drivers/usb/chipidea/ci_hdrc_imx.c |   32 ++--
  drivers/usb/chipidea/core.c|2 ++
  drivers/usb/chipidea/host.c|1 +
  drivers/usb/host/ehci.h|   18 +-
  include/linux/usb/chipidea.h   |1 +
  6 files changed, 71 insertions(+), 9 deletions(-)

Alexander, this is Peter Chens's and my final version of the patch. Can
you please have a look at it?

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/1] usb: chipidea: fix nobody cared IRQ when booting with host role

[Marc Kleine-Budde]

On 11/22/2013 09:14 AM, Peter Chen wrote:
 If we connect Male-A-To-Male-A cable between otg-host and host pc,
 the ci-vbus_active is set wrongly, and cause the controller run
 at peripheral mode when we load gadget module (ci_udc_start will be run),
 but the software runs at host mode due to id = 0. The ehci_irq
 can't handle suspend (USBi_SLI) interrupt which is enabled for
 peripheral mode, it causes no one will handle irq error.
 
 Cc: sta...@vger.kernel.org # 3.12
 Acked-by: Michael Grzeschik m...@pengutronix.de
 Reported-by: Marc Kleine-Budde m...@pengutronix.de
 Tested-by: Marc Kleine-Budde m...@pengutronix.de
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 Signed-off-by: Peter Chen peter.c...@freescale.com

Alexander, please have a look at this patch.

Thanks,
Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v6 0/3] usb: ehci+chipidea: fix imx28 ENGR119653 USB: ARM to USB register error issue

[Marc Kleine-Budde]

On 11/26/2013 01:11 PM, Peter Chen wrote:
 Changes for v6:
 - Add volatile for arm swp instruction, since we need io access
 to fix this problem
 - Add kernel style comment for imx28_write_fix

Looks good now. I'll send you my Tested-by this afternoon.

tnx,
Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v6 0/3] usb: ehci+chipidea: fix imx28 ENGR119653 USB: ARM to USB register error issue

[Marc Kleine-Budde]

On 11/26/2013 01:44 PM, Marc Kleine-Budde wrote:
 On 11/26/2013 01:11 PM, Peter Chen wrote:
 Changes for v6:
 - Add volatile for arm swp instruction, since we need io access
 to fix this problem
 - Add kernel style comment for imx28_write_fix
 
 Looks good now. I'll send you my Tested-by this afternoon.

Tested-by: Marc Kleine-Budde m...@pengutronix.de

on $CUSTOMER mx28 hardware.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v5 0/3] usb: ehci+chipidea: fix imx28 ENGR119653 USB: ARM to USB register error issue

[Marc Kleine-Budde]

On 11/24/2013 11:57 AM, Peter Chen wrote:
 On Fri, Nov 22, 2013 at 10:53:53AM +0100, Marc Kleine-Budde wrote:
 Hello,

 this series implements the suggested workaround for freescale's imx28 Errata,
 ENGR119653 USB: ARM to USB register error issue.

 changes since v4:
 - fix signature of imx28_ehci_writel(), don't use volatile, make use of 
 __iomem
 
 The __iomem is just sparse thing, and not related to gcc, our 
 imx28_ehci_writel
 uses assembly directly, and not call writel any more. The definition of
 __raw_writel still uses volatile:

Yes
 
 include/asm-generic/io.h:
 static inline void __raw_writel(u32 b, volatile void __iomem *addr)
 {
   *(volatile u32 __force *) addr = b;
 }

What does that mean for our imx28_ehci_writel? Do we need volatile here?

 +static inline void imx28_ehci_writel(const unsigned int val, __u32 __iomem 
 *addr)
 +{
 + __asm__ (swp %0, %0, [%1] : : r(val), r(addr));
 +}

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

[PATCH v5 0/3] usb: ehci+chipidea: fix imx28 ENGR119653 USB: ARM to USB register error issue

[Marc Kleine-Budde]

Hello,

this series implements the suggested workaround for freescale's imx28 Errata,
ENGR119653 USB: ARM to USB register error issue.

changes since v4:
- fix signature of imx28_ehci_writel(), don't use volatile, make use of __iomem
- avoid new #ifdef by using if (IS_ENABLED())

Marc


--
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/3] usb: ehci: add freescale imx28 special write register method

[Marc Kleine-Budde]

From: Peter Chen peter.c...@freescale.com

According to Freescale imx28 Errata, ENGR119653 USB: ARM to USB
register error issue, All USB register write operations must
use the ARM SWP instruction. So, we implement a special ehci_write
for imx28.

Discussion for it at below:
http://marc.info/?l=linux-usbm=137996395529294w=2

Signed-off-by: Peter Chen peter.c...@freescale.com
Acked-by: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 drivers/usb/host/ehci.h | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index e316058..e055234 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -200,6 +200,7 @@ struct ehci_hcd {   /* one per controller */
unsignedhas_synopsys_hc_bug:1; /* Synopsys HC */
unsignedframe_index_bug:1; /* MosChip (AKA NetMos) */
unsignedneed_oc_pp_cycle:1; /* MPC834X port power */
+   unsignedimx28_write_fix:1; /* For Freescale i.MX28 */
 
/* required for usb32 quirk */
#define OHCI_CTRL_HCFS  (3  6)
@@ -676,6 +677,17 @@ static inline unsigned int ehci_readl(const struct 
ehci_hcd *ehci,
 #endif
 }
 
+#ifdef CONFIG_SOC_IMX28
+static inline void imx28_ehci_writel(const unsigned int val, __u32 __iomem 
*addr)
+{
+   __asm__ (swp %0, %0, [%1] : : r(val), r(addr));
+}
+#else
+static inline void imx28_ehci_writel(const unsigned int val, __u32 __iomem 
*addr)
+{
+}
+#endif
+
 static inline void ehci_writel(const struct ehci_hcd *ehci,
const unsigned int val, __u32 __iomem *regs)
 {
@@ -684,7 +696,10 @@ static inline void ehci_writel(const struct ehci_hcd *ehci,
writel_be(val, regs) :
writel(val, regs);
 #else
-   writel(val, regs);
+   if (IS_ENABLED(CONFIG_SOC_IMX28)  ehci-imx28_write_fix)
+   imx28_ehci_writel(val, regs);
+   else
+   writel(val, regs);
 #endif
 }
 
-- 
1.8.4.2

--
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 3/3] usb: chipidea: imx: set CI_HDRC_IMX28_WRITE_FIX for imx28

[Marc Kleine-Budde]

From: Peter Chen peter.c...@freescale.com

Due to imx28 needs ARM swp instruction for writing, we set
CI_HDRC_IMX28_WRITE_FIX for imx28.

Signed-off-by: Peter Chen peter.c...@freescale.com
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 drivers/usb/chipidea/ci_hdrc_imx.c | 32 ++--
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
b/drivers/usb/chipidea/ci_hdrc_imx.c
index 023d3cb..68f7f5e 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -23,6 +23,26 @@
 #include ci.h
 #include ci_hdrc_imx.h
 
+#define CI_HDRC_IMX_IMX28_WRITE_FIX BIT(0)
+
+struct ci_hdrc_imx_platform_flag {
+   unsigned int flags;
+};
+
+static const struct ci_hdrc_imx_platform_flag imx27_usb_data = {
+};
+
+static const struct ci_hdrc_imx_platform_flag imx28_usb_data = {
+   .flags = CI_HDRC_IMX_IMX28_WRITE_FIX,
+};
+
+static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
+   { .compatible = fsl,imx28-usb, .data = imx28_usb_data},
+   { .compatible = fsl,imx27-usb, .data = imx27_usb_data},
+   { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ci_hdrc_imx_dt_ids);
+
 struct ci_hdrc_imx_data {
struct usb_phy *phy;
struct platform_device *ci_pdev;
@@ -82,6 +102,9 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
  CI_HDRC_DISABLE_STREAMING,
};
int ret;
+   const struct of_device_id *of_id =
+   of_match_device(ci_hdrc_imx_dt_ids, pdev-dev);
+   const struct ci_hdrc_imx_platform_flag *imx_platform_flag = of_id-data;
 
data = devm_kzalloc(pdev-dev, sizeof(*data), GFP_KERNEL);
if (!data) {
@@ -115,6 +138,9 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 
pdata.phy = data-phy;
 
+   if (imx_platform_flag-flags  CI_HDRC_IMX_IMX28_WRITE_FIX)
+   pdata.flags |= CI_HDRC_IMX28_WRITE_FIX;
+
if (!pdev-dev.dma_mask)
pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
if (!pdev-dev.coherent_dma_mask)
@@ -174,12 +200,6 @@ static int ci_hdrc_imx_remove(struct platform_device *pdev)
return 0;
 }
 
-static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
-   { .compatible = fsl,imx27-usb, },
-   { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, ci_hdrc_imx_dt_ids);
-
 static struct platform_driver ci_hdrc_imx_driver = {
.probe = ci_hdrc_imx_probe,
.remove = ci_hdrc_imx_remove,
-- 
1.8.4.2

--
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] usb: chipidea: fix nobody cared IRQ when booting with host role

[Marc Kleine-Budde]

On 11/22/2013 09:12 AM, Peter Chen wrote:
 I've moved the comment in ci_hdrc_probe() in front of the if. Peter can
 you
 please add a meaningfull description to the patch, as I don't fully
 understand
 how it really fixes the problem. Can you add stable on Cc for the final
 patch?

 Tested on imx28-evk and custom board.

 
 I go to send out a patch for it now.

Thanks, I have posted an inproved version of ENGR119653 USB: ARM to USB
register error issue in return :)

Thanks,
Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

chipidea on mx28-evk: irq 237: nobody cared

[Marc Kleine-Budde]

Hello,

I'm using Greg's linux-usb/next on mx28-evk with mxs-defconfig. I
switched on OTG support and ether gadget and compiled it into the kernel.

During every boot when the ether gadget is loaded the IRQ of the usb0
fires and nobody cares:

 [3.336843] ci_hdrc ci_hdrc.0: ChipIdea HDRC found, lpm: 0; cap: c8920100 
 op: c8920140
 [3.336951] ci_hdrc ci_hdrc.0: It is OTG capable controller
 [3.345476] ci_hdrc ci_hdrc.0: EHCI Host Controller
 [3.351998] ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus number 
 1
 [3.379715] ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00
 [3.397057] hub 1-0:1.0: USB hub found
 [3.401761] hub 1-0:1.0: 1 port detected
 [3.413366] ci_hdrc ci_hdrc.1: ChipIdea HDRC found, lpm: 0; cap: c8940100 
 op: c8940140
 [3.413476] ci_hdrc ci_hdrc.1: It is OTG capable controller
 [3.417595] ci_hdrc ci_hdrc.1: EHCI Host Controller
 [3.422925] ci_hdrc ci_hdrc.1: new USB bus registered, assigned bus number 
 2
 [3.449654] ci_hdrc ci_hdrc.1: USB 2.0 started, EHCI 1.00
 [3.460242] hub 2-0:1.0: USB hub found
 [3.464310] hub 2-0:1.0: 1 port detected
 [3.475410] using random self ethernet address
 [3.480264] using random host ethernet address
 [3.488022] usb0: HOST MAC 66:02:bb:f7:e7:eb
 [3.493850] usb0: MAC 26:a9:b8:cc:5c:2d
 [3.497987] using random self ethernet address
 [3.503321] using random host ethernet address
 [3.510076] g_ether gadget: Ethernet Gadget, version: Memorial Day 2008
 [3.516749] g_ether gadget: g_ether ready
 [4.842006] irq 237: nobody cared (try booting with the irqpoll option)
 [4.848855] CPU: 0 PID: 1 Comm: swapper Not tainted 3.12.0-rc6 #149
 [4.855218] [c00144c8] (unwind_backtrace+0x0/0xf0) from [c0011ce4] 
 (show_stack+0x10/0x14)
 [4.863808] [c0011ce4] (show_stack+0x10/0x14) from [c00513a0] 
 (__report_bad_irq+0x20/0xc0)
 [4.872463] [c00513a0] (__report_bad_irq+0x20/0xc0) from [c0051848] 
 (note_interrupt+0x1d4/0x238)
 [4.881636] [c0051848] (note_interrupt+0x1d4/0x238) from [c004fac4] 
 (handle_irq_event_percpu+0xc4/0x264)
 [4.891500] [c004fac4] (handle_irq_event_percpu+0xc4/0x264) from 
 [c004fca0] (handle_irq_event+0x3c/0x5c)
 [4.901365] [c004fca0] (handle_irq_event+0x3c/0x5c) from [c0051fd0] 
 (handle_level_irq+0x8c/0xe8)
 [4.910535] [c0051fd0] (handle_level_irq+0x8c/0xe8) from [c004f358] 
 (generic_handle_irq+0x20/0x30)
 [4.919880] [c004f358] (generic_handle_irq+0x20/0x30) from [c000fde0] 
 (handle_IRQ+0x30/0x84)
 [4.928705] [c000fde0] (handle_IRQ+0x30/0x84) from [c0012764] 
 (__irq_svc+0x44/0x54)
 [4.936757] [c0012764] (__irq_svc+0x44/0x54) from [c0020248] 
 (__do_softirq+0x90/0x26c)
 [4.945058] [c0020248] (__do_softirq+0x90/0x26c) from [c00204f0] 
 (do_softirq+0x68/0x70)
 [4.953442] [c00204f0] (do_softirq+0x68/0x70) from [c00207e4] 
 (irq_exit+0xa4/0xf4)
 [4.961395] [c00207e4] (irq_exit+0xa4/0xf4) from [c000fde4] 
 (handle_IRQ+0x34/0x84)
 [4.969353] [c000fde4] (handle_IRQ+0x34/0x84) from [c0012764] 
 (__irq_svc+0x44/0x54)
 [4.977396] [c0012764] (__irq_svc+0x44/0x54) from [c0062c44] 
 (lock_acquire+0xac/0x104)
 [4.985715] [c0062c44] (lock_acquire+0xac/0x104) from [c046f76c] 
 (mutex_lock_nested+0x48/0x2d0)
 [4.994815] [c046f76c] (mutex_lock_nested+0x48/0x2d0) from [c02c211c] 
 (device_add+0x36c/0x608)
 [5.003831] [c02c211c] (device_add+0x36c/0x608) from [c035a990] 
 (mousedev_create+0x1f8/0x25c)
 [5.012759] [c035a990] (mousedev_create+0x1f8/0x25c) from [c067ca1c] 
 (mousedev_init+0x14/0x60)
 [5.021758] [c067ca1c] (mousedev_init+0x14/0x60) from [c00088ac] 
 (do_one_initcall+0xe8/0x154)
 [5.030672] [c00088ac] (do_one_initcall+0xe8/0x154) from [c0659ac8] 
 (kernel_init_freeable+0xec/0x1b4)
 [5.040278] [c0659ac8] (kernel_init_freeable+0xec/0x1b4) from 
 [c0468188] (kernel_init+0x8/0xe4)
 [5.049363] [c0468188] (kernel_init+0x8/0xe4) from [c000efa0] 
 (ret_from_fork+0x14/0x34)
 [5.057723] handlers:
 [5.060030] [c03472b8] ci_irq
 [5.063205] Disabling IRQ #237

IRQ 237 belongs to the usb0, the OTG capable port.

 grep ci_hdrc /proc/interrupts
 237: 10 -  93  ci_hdrc_imx
 238:  0 -  92  ci_hdrc_imx

I can reproduce the problem on another mx28 hardware, it happens only if
a host type USB cable is plugged into the OTG port and connected to
another machine's host port. This leads to the ID pin is pulled to
ground and the chipidea driver comes up host mode. However the ether
gadget seems to do something on the bus, which triggers an interrupt in
the udc, but the multiplexer in the chipidea core will call the host
role, due to the ID pin.

If the system boot without a cable or with a device type cable (attached
to another host or not doesn't matter), the ID pin stays floating, the
driver comes in peripheral role and everything works.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions

Re: chipidea on mx28-evk: irq 237: nobody cared

[Marc Kleine-Budde]

On 11/14/2013 02:17 PM, Fabio Estevam wrote:
 Hi Marc,
 
 On Thu, Nov 14, 2013 at 10:46 AM, Marc Kleine-Budde m...@pengutronix.de 
 wrote:
 Hello,

 I'm using Greg's linux-usb/next on mx28-evk with mxs-defconfig. I
 switched on OTG support and ether gadget and compiled it into the kernel.

 During every boot when the ether gadget is loaded the IRQ of the usb0
 fires and nobody cares:
 
 I got the same issue before on mx28evk, please see:
 
 commit 69c02f9593b62a27ccee11293b21ad889a59cb5e
 Author: Fabio Estevam fabio.este...@freescale.com
 Date:   Wed Aug 21 10:27:03 2013 -0300
 
 ARM: dts: imx28-evk: Allow usb peripheral mode to work
 
 Please try setting the IOMUX of the pin id on your board.

This patch doesn't help. On the mx28-evk nothing is attached to any USB
port.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: chipidea on mx28-evk: irq 237: nobody cared

[Marc Kleine-Budde]

On 11/14/2013 02:45 PM, Peter Chen wrote:
 
  
 [3.497987] using random self ethernet address
 [3.503321] using random host ethernet address
 [3.510076] g_ether gadget: Ethernet Gadget, version: Memorial Day
 2008
 [3.516749] g_ether gadget: g_ether ready
 [4.842006] irq 237: nobody cared (try booting with the irqpoll
 option)
 [4.848855] CPU: 0 PID: 1 Comm: swapper Not tainted 3.12.0-rc6 #149
 [4.855218] [c00144c8] (unwind_backtrace+0x0/0xf0) from
 [c0011ce4] (show_stack+0x10/0x14)
 [4.863808] [c0011ce4] (show_stack+0x10/0x14) from [c00513a0]
 (__report_bad_irq+0x20/0xc0)
 [4.872463] [c00513a0] (__report_bad_irq+0x20/0xc0) from
 [c0051848] (note_interrupt+0x1d4/0x238)
 [4.881636] [c0051848] (note_interrupt+0x1d4/0x238) from
 [c004fac4] (handle_irq_event_percpu+0xc4/0x264)
 [4.891500] [c004fac4] (handle_irq_event_percpu+0xc4/0x264) from
 [c004fca0] (handle_irq_event+0x3c/0x5c)
 [4.901365] [c004fca0] (handle_irq_event+0x3c/0x5c) from
 [c0051fd0] (handle_level_irq+0x8c/0xe8)
 [4.910535] [c0051fd0] (handle_level_irq+0x8c/0xe8) from
 [c004f358] (generic_handle_irq+0x20/0x30)
 [4.919880] [c004f358] (generic_handle_irq+0x20/0x30) from
 [c000fde0] (handle_IRQ+0x30/0x84)
 [4.928705] [c000fde0] (handle_IRQ+0x30/0x84) from [c0012764]
 (__irq_svc+0x44/0x54)
 [4.936757] [c0012764] (__irq_svc+0x44/0x54) from [c0020248]
 (__do_softirq+0x90/0x26c)
 [4.945058] [c0020248] (__do_softirq+0x90/0x26c) from [c00204f0]
 (do_softirq+0x68/0x70)
 [4.953442] [c00204f0] (do_softirq+0x68/0x70) from [c00207e4]
 (irq_exit+0xa4/0xf4)
 [4.961395] [c00207e4] (irq_exit+0xa4/0xf4) from [c000fde4]
 (handle_IRQ+0x34/0x84)
 [4.969353] [c000fde4] (handle_IRQ+0x34/0x84) from [c0012764]
 (__irq_svc+0x44/0x54)
 [4.977396] [c0012764] (__irq_svc+0x44/0x54) from [c0062c44]
 (lock_acquire+0xac/0x104)
 [4.985715] [c0062c44] (lock_acquire+0xac/0x104) from [c046f76c]
 (mutex_lock_nested+0x48/0x2d0)
 [4.994815] [c046f76c] (mutex_lock_nested+0x48/0x2d0) from
 [c02c211c] (device_add+0x36c/0x608)
 [5.003831] [c02c211c] (device_add+0x36c/0x608) from [c035a990]
 (mousedev_create+0x1f8/0x25c)
 [5.012759] [c035a990] (mousedev_create+0x1f8/0x25c) from
 [c067ca1c] (mousedev_init+0x14/0x60)
 [5.021758] [c067ca1c] (mousedev_init+0x14/0x60) from [c00088ac]
 (do_one_initcall+0xe8/0x154)
 [5.030672] [c00088ac] (do_one_initcall+0xe8/0x154) from
 [c0659ac8] (kernel_init_freeable+0xec/0x1b4)
 [5.040278] [c0659ac8] (kernel_init_freeable+0xec/0x1b4) from
 [c0468188] (kernel_init+0x8/0xe4)
 [5.049363] [c0468188] (kernel_init+0x8/0xe4) from [c000efa0]
 (ret_from_fork+0x14/0x34)
 [5.057723] handlers:
 [5.060030] [c03472b8] ci_irq
 [5.063205] Disabling IRQ #237

 IRQ 237 belongs to the usb0, the OTG capable port.

 grep ci_hdrc /proc/interrupts
 237: 10 -  93  ci_hdrc_imx
 238:  0 -  92  ci_hdrc_imx

 I can reproduce the problem on another mx28 hardware, it happens only if
 a host type USB cable is plugged into the OTG port and connected to
 another machine's host port.
 
 What your otg port looks like? It is a Micro-B-Female port?
 If ID pin is low, it will be considered as host role, when you
 connect to another machine's host port, both sides are host, it can't 
 be enumerated.

Yes, I know, both sides are in the host role. Granted my fault, but
never the less, we get an IRQ storm and the IRQ gets disabled. You have
to reboot to get the system working again.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 1/3] usb: ehci: add freescale imx28 special write register method

[Marc Kleine-Budde]

On 10/30/2013 04:06 AM, Peter Chen wrote:
 According to Freescale imx28 Errata, ENGR119653 USB: ARM to USB
 register error issue, All USB register write operations must
 use the ARM SWP instruction. So, we implement a special ehci_write
 for imx28.
 
 Discussion for it at below:
 http://marc.info/?l=linux-usbm=137996395529294w=2
 
 Signed-off-by: Peter Chen peter.c...@freescale.com
 Acked-by: Alan Stern st...@rowland.harvard.edu
 ---
  drivers/usb/host/ehci.h |   13 +
  1 files changed, 13 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
 index e8f41c5..535aa8b 100644
 --- a/drivers/usb/host/ehci.h
 +++ b/drivers/usb/host/ehci.h
 @@ -225,6 +225,7 @@ struct ehci_hcd { /* one per controller */
   unsignedhas_synopsys_hc_bug:1; /* Synopsys HC */
   unsignedframe_index_bug:1; /* MosChip (AKA NetMos) */
   unsignedneed_oc_pp_cycle:1; /* MPC834X port power */
 + unsignedimx28_write_fix:1; /* For Freescale i.MX28 */
  
   /* required for usb32 quirk */
   #define OHCI_CTRL_HCFS  (3  6)
 @@ -728,6 +729,13 @@ static inline unsigned int ehci_readl(const struct 
 ehci_hcd *ehci,
  #endif
  }
  
 +#ifdef CONFIG_SOC_IMX28
 +static inline void imx28_ehci_writel(u32 val32, volatile u32 *addr)

You should annotate the addr pointer with __iomem, or better use the
signature of ehci_writel():

static inline void imx28_ehci_writel(const unsinged int val32, __u32
__iomem *addr)

Marc

 +{
 + __asm__ (swp %0, %0, [%1] : : r(val32), r(addr));
 +}
 +#endif
 +
  static inline void ehci_writel(const struct ehci_hcd *ehci,
   const unsigned int val, __u32 __iomem *regs)
  {
 @@ -735,6 +743,11 @@ static inline void ehci_writel(const struct ehci_hcd 
 *ehci,
   ehci_big_endian_mmio(ehci) ?
   writel_be(val, regs) :
   writel(val, regs);
 +#elif defined(CONFIG_SOC_IMX28)
 + if (ehci-imx28_write_fix)
 + imx28_ehci_writel(val, regs);
 + else
 + writel(val, regs);
  #else
   writel(val, regs);
  #endif
 


-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 2/3] usb: chipidea: add freescale imx28 special write register method

[Marc Kleine-Budde]

On 10/30/2013 04:06 AM, Peter Chen wrote:
 According to Freescale imx28 Errata, ENGR119653 USB: ARM to USB
 register error issue, All USB register write operations must
 use the ARM SWP instruction. So, we implement special hw_write
 and hw_test_and_clear for imx28.
 
 Discussion for it at below:
 http://marc.info/?l=linux-usbm=137996395529294w=2
 
 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---
 Changes for v4:
 - introducing __hw_write to encapsulate low level write,
 it can reduce the duplicated code.
 
  drivers/usb/chipidea/ci.h|   26 --
  drivers/usb/chipidea/core.c  |2 ++
  drivers/usb/chipidea/host.c  |1 +
  include/linux/usb/chipidea.h |1 +
  4 files changed, 28 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
 index 1c94fc5..f9b1914 100644
 --- a/drivers/usb/chipidea/ci.h
 +++ b/drivers/usb/chipidea/ci.h
 @@ -173,6 +173,8 @@ struct ci_hdrc {
   struct dentry   *debugfs;
   boolid_event;
   boolb_sess_valid_event;
 + /* imx28 needs swp instruction for writing */
 + boolimx28_write_fix;
  };
  
  static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
 @@ -253,6 +255,26 @@ static inline u32 hw_read(struct ci_hdrc *ci, enum 
 ci_hw_regs reg, u32 mask)
   return ioread32(ci-hw_bank.regmap[reg])  mask;
  }
  
 +#ifdef CONFIG_SOC_IMX28
 +static inline void imx28_ci_writel(u32 val32, volatile u32 *addr)

same applies here:

static inline void imx28_ci_writel(u32 val32, void __iomem *addr)

Marc
 +{
 + __asm__ (swp %0, %0, [%1] : : r(val32), r(addr));
 +}
 +#endif
 +
 +static inline void __hw_write(struct ci_hdrc *ci, u32 val,
 + void __iomem *addr)
 +{
 +#ifdef CONFIG_SOC_IMX28
 + if (ci-imx28_write_fix)
 + imx28_ci_writel(val, addr);
 + else
 + iowrite32(val, addr);
 +#else
 + iowrite32(val, addr);
 +#endif
 +}
 +
  /**
   * hw_write: writes to a hw register
   * @reg:  register index
 @@ -266,7 +288,7 @@ static inline void hw_write(struct ci_hdrc *ci, enum 
 ci_hw_regs reg,
   data = (ioread32(ci-hw_bank.regmap[reg])  ~mask)
   | (data  mask);
  
 - iowrite32(data, ci-hw_bank.regmap[reg]);
 + __hw_write(ci, data, ci-hw_bank.regmap[reg]);
  }
  
  /**
 @@ -281,7 +303,7 @@ static inline u32 hw_test_and_clear(struct ci_hdrc *ci, 
 enum ci_hw_regs reg,
  {
   u32 val = ioread32(ci-hw_bank.regmap[reg])  mask;
  
 - iowrite32(val, ci-hw_bank.regmap[reg]);
 + __hw_write(ci, val, ci-hw_bank.regmap[reg]);
   return val;
  }
  
 diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
 index 06204b7..357a059 100644
 --- a/drivers/usb/chipidea/core.c
 +++ b/drivers/usb/chipidea/core.c
 @@ -551,6 +551,8 @@ static int ci_hdrc_probe(struct platform_device *pdev)
  
   ci-dev = dev;
   ci-platdata = dev-platform_data;
 + ci-imx28_write_fix = !!(ci-platdata-flags 
 + CI_HDRC_IMX28_WRITE_FIX);
  
   ret = hw_device_init(ci, base);
   if (ret  0) {
 diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
 index 59e6020..06fd042 100644
 --- a/drivers/usb/chipidea/host.c
 +++ b/drivers/usb/chipidea/host.c
 @@ -65,6 +65,7 @@ static int host_start(struct ci_hdrc *ci)
   ehci-caps = ci-hw_bank.cap;
   ehci-has_hostpc = ci-hw_bank.lpm;
   ehci-has_tdi_phy_lpm = ci-hw_bank.lpm;
 + ehci-imx28_write_fix = ci-imx28_write_fix;
  
   if (ci-platdata-reg_vbus) {
   ret = regulator_enable(ci-platdata-reg_vbus);
 diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
 index 7d39967..708bd11 100644
 --- a/include/linux/usb/chipidea.h
 +++ b/include/linux/usb/chipidea.h
 @@ -24,6 +24,7 @@ struct ci_hdrc_platform_data {
* but otg is not supported (no register otgsc).
*/
  #define CI_HDRC_DUAL_ROLE_NOT_OTGBIT(4)
 +#define CI_HDRC_IMX28_WRITE_FIX  BIT(5)
   enum usb_dr_modedr_mode;
  #define CI_HDRC_CONTROLLER_RESET_EVENT   0
  #define CI_HDRC_CONTROLLER_STOPPED_EVENT 1
 


-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 3/3] usb: chipidea: imx: set CI_HDRC_IMX28_WRITE_FIX for imx28

[Marc Kleine-Budde]

On 10/30/2013 04:06 AM, Peter Chen wrote:
 Due to imx28 needs ARM swp instruction for writing, we set
 CI_HDRC_IMX28_WRITE_FIX for imx28.
 
 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---
  drivers/usb/chipidea/ci_hdrc_imx.c |   32 ++--
  1 files changed, 26 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
 b/drivers/usb/chipidea/ci_hdrc_imx.c
 index 023d3cb..68f7f5e 100644
 --- a/drivers/usb/chipidea/ci_hdrc_imx.c
 +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
 @@ -23,6 +23,26 @@
  #include ci.h
  #include ci_hdrc_imx.h
  
 +#define CI_HDRC_IMX_IMX28_WRITE_FIX BIT(0)
 +
 +struct ci_hdrc_imx_platform_flag {
 + unsigned int flags;
 +};
 +
 +static const struct ci_hdrc_imx_platform_flag imx27_usb_data = {
 +};
 +
 +static const struct ci_hdrc_imx_platform_flag imx28_usb_data = {
 + .flags = CI_HDRC_IMX_IMX28_WRITE_FIX,
 +};
 +
 +static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
 + { .compatible = fsl,imx28-usb, .data = imx28_usb_data},
 + { .compatible = fsl,imx27-usb, .data = imx27_usb_data},
   ^^^
Nitpick, please add ,  or a single space.

Marc

 + { /* sentinel */ }
 +};
 +MODULE_DEVICE_TABLE(of, ci_hdrc_imx_dt_ids);
 +
  struct ci_hdrc_imx_data {
   struct usb_phy *phy;
   struct platform_device *ci_pdev;
 @@ -82,6 +102,9 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
 CI_HDRC_DISABLE_STREAMING,
   };
   int ret;
 + const struct of_device_id *of_id =
 + of_match_device(ci_hdrc_imx_dt_ids, pdev-dev);
 + const struct ci_hdrc_imx_platform_flag *imx_platform_flag = of_id-data;
  
   data = devm_kzalloc(pdev-dev, sizeof(*data), GFP_KERNEL);
   if (!data) {
 @@ -115,6 +138,9 @@ static int ci_hdrc_imx_probe(struct platform_device *pdev)
  
   pdata.phy = data-phy;
  
 + if (imx_platform_flag-flags  CI_HDRC_IMX_IMX28_WRITE_FIX)
 + pdata.flags |= CI_HDRC_IMX28_WRITE_FIX;
 +
   if (!pdev-dev.dma_mask)
   pdev-dev.dma_mask = pdev-dev.coherent_dma_mask;
   if (!pdev-dev.coherent_dma_mask)
 @@ -174,12 +200,6 @@ static int ci_hdrc_imx_remove(struct platform_device 
 *pdev)
   return 0;
  }
  
 -static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
 - { .compatible = fsl,imx27-usb, },
 - { /* sentinel */ }
 -};
 -MODULE_DEVICE_TABLE(of, ci_hdrc_imx_dt_ids);
 -
  static struct platform_driver ci_hdrc_imx_driver = {
   .probe = ci_hdrc_imx_probe,
   .remove = ci_hdrc_imx_remove,
 


-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v12 10/13] usb: chipidea: add wait vbus lower than OTGSC_BSV before role starts

[Marc Kleine-Budde]

On 07/11/2013 08:27 AM, Peter Chen wrote:
 When the gadget role starts, we need to make sure the vbus is lower
 than OTGSC_BSV, or there will be an vbus interrupt since we use
 B_SESSION_VALID as vbus interrupt to indicate connect and disconnect.
 When the host role starts, it may not be useful to wait vbus to lower
 than OTGSC_BSV, but it can indicate some hardware problems like the
 vbus is still higher than OTGSC_BSV after we disconnect to host some
 time later (500 jiffies currently), which is obvious not correct.

Jiffies ist not constant. I think you have to specify the timeout in
fractions of HZ. The system ticks with HZ jiffies per second. So a
timeout of 500ms is HZ / 2.

Marc

 
 Signed-off-by: Peter Chen peter.c...@freescale.com
 ---
  drivers/usb/chipidea/ci.h   |3 +++
  drivers/usb/chipidea/core.c |   32 
  drivers/usb/chipidea/otg.c  |4 
  3 files changed, 39 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
 index 3160363..df27696 100644
 --- a/drivers/usb/chipidea/ci.h
 +++ b/drivers/usb/chipidea/ci.h
 @@ -308,4 +308,7 @@ int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
  
  u8 hw_port_test_get(struct ci_hdrc *ci);
  
 +int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
 + u32 value, unsigned long timeout);
 +
  #endif   /* __DRIVERS_USB_CHIPIDEA_CI_H */
 diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
 index 7a07300..1c39541 100644
 --- a/drivers/usb/chipidea/core.c
 +++ b/drivers/usb/chipidea/core.c
 @@ -295,6 +295,38 @@ int hw_device_reset(struct ci_hdrc *ci, u32 mode)
   return 0;
  }
  
 +/**
 + * hw_wait_reg: wait the register value
 + *
 + * Sometimes, it needs to wait register value before going on.
 + * Eg, when switch to device mode, the vbus value should be lower
 + * than OTGSC_BSV before connects to host.
 + *
 + * @ci: the controller
 + * @reg: register index
 + * @mask: mast bit
 + * @value: the bit value to wait
 + * @timeout: timeout to indicate an error


Better: timeout in jiffies

 + *
 + * This function returns an error code if timeout
 + */
 +int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
 + u32 value, unsigned long timeout)
 +{
 + unsigned long elapse = jiffies + timeout;
 +
 + while (hw_read(ci, reg, mask) != value) {
 + if (time_after(jiffies, elapse)) {
 + dev_err(ci-dev, timeout waiting for %08x in %d\n,
 + mask, reg);
 + return -ETIMEDOUT;
 + }
 + msleep(20);
 + }
 +
 + return 0;
 +}
 +
  static irqreturn_t ci_irq(int irq, void *data)
  {
   struct ci_hdrc *ci = data;
 diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
 index c74194d..8aa0241 100644
 --- a/drivers/usb/chipidea/otg.c
 +++ b/drivers/usb/chipidea/otg.c
 @@ -67,6 +67,7 @@ void ci_handle_vbus_change(struct ci_hdrc *ci)
   usb_gadget_vbus_disconnect(ci-gadget);
  }
  
 +#define CI_VBUS_STABLE_TIMEOUT 500
  static void ci_handle_id_switch(struct ci_hdrc *ci)
  {
   enum ci_role role = ci_otg_role(ci);
 @@ -76,6 +77,9 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
   ci_role(ci)-name, ci-roles[role]-name);
  
   ci_role_stop(ci);
 + /* wait vbus lower than OTGSC_BSV */
 + hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
 + CI_VBUS_STABLE_TIMEOUT);
   ci_role_start(ci, role);
   }
  }
 


-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v12 10/13] usb: chipidea: add wait vbus lower than OTGSC_BSV before role starts

[Marc Kleine-Budde]

On 07/11/2013 11:25 AM, Peter Chen wrote:
 On Thu, Jul 11, 2013 at 09:24:56AM +0200, Marc Kleine-Budde wrote:
 On 07/11/2013 08:27 AM, Peter Chen wrote:
 When the gadget role starts, we need to make sure the vbus is lower
 than OTGSC_BSV, or there will be an vbus interrupt since we use
 B_SESSION_VALID as vbus interrupt to indicate connect and disconnect.
 When the host role starts, it may not be useful to wait vbus to lower
 than OTGSC_BSV, but it can indicate some hardware problems like the
 vbus is still higher than OTGSC_BSV after we disconnect to host some
 time later (500 jiffies currently), which is obvious not correct.

 Jiffies ist not constant. I think you have to specify the timeout in
 fractions of HZ. The system ticks with HZ jiffies per second. So a
 timeout of 500ms is HZ / 2.
 
 Thanks, I will use msecs_to_jiffies.

even better.

 
 +/**
 + * hw_wait_reg: wait the register value
 + *
 + * Sometimes, it needs to wait register value before going on.
 + * Eg, when switch to device mode, the vbus value should be lower
 + * than OTGSC_BSV before connects to host.
 + *
 + * @ci: the controller
 + * @reg: register index
 + * @mask: mast bit
 + * @value: the bit value to wait
 + * @timeout: timeout to indicate an error
 

 Better: timeout in jiffies

 
 As I will use msecs_to_jiffies, @timeout: timeout in millisecond.

Good idea - you might want to use timeout_ms.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2 1/2] usb: chipidea: big-endian support

[Marc Kleine-Budde]

On 05/09/2013 11:22 PM, Svetoslav Neykov wrote:
 Hi Marc,
 
 Marc Kleine-Budde [mailto:m...@pengutronix.de] (On Thursday, March 28, 2013
 4:16 PM)
 On 03/28/2013 10:28 AM, Alexander Shishkin wrote:
 Svetoslav Neykov svetos...@neykov.name writes:

 Convert between big-endian and little-endian format when accessing
 the usb controller structures which are little-endian by
 specification. Fix cases where the little-endian memory layout is
 taken for granted. The patch doesn't have any effect on the already
 supported little-endian architectures.

 Has anyone tested how the cpu_to_le32 and vice versa effects the
 load/store operations? Does the compiler generate full 32 bit accesses
 on mips (and big endian arm) or is a byte-shift-or pattern used?
 
 Better late than never... I have checked your question, the value is loaded
 in a register and then swapped, so the read is performed only once.

Thanks for checking this.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 4/4] staging: dwc2: load parameters from the devicetree

[Marc Kleine-Budde]

On 04/11/2013 06:43 PM, Matthijs Kooijman wrote:
 Each of the parameters in the dwc2_core_params struct can now be changed
 using devicetree parameters.
 
 Signed-off-by: Matthijs Kooijman matth...@stdin.nl
 ---
  Documentation/devicetree/bindings/staging/dwc2.txt |  36 
  drivers/staging/dwc2/platform.c| 100 
 +
  2 files changed, 136 insertions(+)
 
 This just adds all existing parameters to the devicetree for now. It's
 not unlikely that some of these parameters gets dropped later or turn
 out to be unsuitable for setting from the devicetree, but for now I've
 just added them all without closely looking at each.
 
 diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt 
 b/Documentation/devicetree/bindings/staging/dwc2.txt
 index 3649c88..05d9c08 100644
 --- a/Documentation/devicetree/bindings/staging/dwc2.txt
 +++ b/Documentation/devicetree/bindings/staging/dwc2.txt
 @@ -5,6 +5,39 @@ Required properties:
  - compatible : snps,dwc2
  - reg : Should contain 1 register range (address and length)
  - interrupts : Should contain 1 interrupt
 +Optional properties:
 + - dma-enable
 + - otg-cap
 + - otg-ver
 + - dma-enable
 + - dma-desc-enable
 + - speed
 + - enable-dynamic-fifo
 + - en-multiple-tx-fifo
 + - host-rx-fifo-size
 + - host-nperio-tx-fifo-size
 + - host-perio-tx-fifo-size
 + - max-transfer-size
 + - max-packet-count
 + - host-channels
 + - phy-type
 + - phy-utmi-width
 + - phy-ulpi-ddr
 + - phy-ulpi-ext-vbus
 + - i2c-enable
 + - ulpi-fs-ls
 + - host-support-fs-ls-low-power
 + - host-ls-low-power-phy-clk
 + - ts-dline
 + - reload-ctl
 + - ahb-single
 +
 +Each of the above properties take a single cell containing an integer. See
 +struct dwc2_core_params in dwc2/core.h for their meanings and possible 
 values.
 +
 +If absent, these values have either a driver default or are autodetected.
 +Where possible, the values set through the devicetree are checked against
 +hardware limits and automatically capped or adjusted as needed.
  
  Example:
  
 @@ -12,4 +45,7 @@ Example:
  compatible = ralink,rt3050-otg, snps,dwc2;
  reg = 0x101c 4;
  interrupts = 18;
 +
 + # Force slave mode, even when dma is advertised to work
 + dma-enable = 0;
  };
 diff --git a/drivers/staging/dwc2/platform.c b/drivers/staging/dwc2/platform.c
 index e65fa0d..9e05865 100644
 --- a/drivers/staging/dwc2/platform.c
 +++ b/drivers/staging/dwc2/platform.c
 @@ -40,6 +40,8 @@
  #include linux/device.h
  #include linux/dma-mapping.h
  #include linux/platform_device.h
 +#include linux/of.h
 +#include asm/byteorder.h
  
  #include core.h
  #include hcd.h
 @@ -69,6 +71,101 @@ static int dwc2_driver_remove(struct platform_device *dev)
  }
  
  /**
 + * dwc2_load_property() - Load a single property from the devicetree
 + * node into the given variable.
 + *
 + * @dev: Platform device
 + * @res: The variable to put the loaded value into
 + * @name: The name of the devicetree property to load
 + */
 +static void dwc2_load_property(struct platform_device *dev, int *res,
 +const char *name)
 +{
 + int len;
 + const u32 *val;
 +
 + val = of_get_property(dev-dev.of_node, name, len);
 + if (!val)
 + return;
 +
 + if (len != sizeof(*val)) {
 + dev_warn(dev-dev,
 +  Invalid value in devicetree for %s property, should 
 be a single integer\n,
 +  name);
 + return;
 + }
 +
 + *res = be32_to_cpu(*val);

What about using of_property_read_u32() instead of open coding it here?

 +
 + dev_dbg(dev-dev, Loaded %s parameter from devicetree: %d\n,
 + name, *res);
 +}

Marc


-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 12/27] usb: chipidea: usbmisc: rename file, struct and functions to usbmisc_imx

[Marc Kleine-Budde]

On 03/30/2013 01:46 AM, Alexander Shishkin wrote:
 From: Michael Grzeschik m.grzesc...@pengutronix.de
 
 This driver will be used for every Freescale SoC which has this misc
 memory layout to control the basic usb handling. So better name this
 driver, function and struct names in a more generic way.
 
 Reported-by: Fabio Estevam feste...@gmail.com
 Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 Signed-off-by: Alexander Shishkin alexander.shish...@linux.intel.com
 
 Conflicts:
   drivers/usb/chipidea/usbmisc_imx.c

You've hopefully resolved the conflict before creating a this patch, so
remove this line :)

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 12/27] usb: chipidea: usbmisc: rename file, struct and functions to usbmisc_imx

[Marc Kleine-Budde]

On 03/30/2013 05:11 PM, Alexander Shishkin wrote:
 Marc Kleine-Budde m...@pengutronix.de writes:
 
 On 03/30/2013 01:46 AM, Alexander Shishkin wrote:
 From: Michael Grzeschik m.grzesc...@pengutronix.de

 This driver will be used for every Freescale SoC which has this misc
 memory layout to control the basic usb handling. So better name this
 driver, function and struct names in a more generic way.

 Reported-by: Fabio Estevam feste...@gmail.com
 Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 Signed-off-by: Alexander Shishkin alexander.shish...@linux.intel.com

 Conflicts:
 drivers/usb/chipidea/usbmisc_imx.c

 You've hopefully resolved the conflict before creating a this patch, so
 remove this line :)
 
 Thanks, captain! :) See the v2 series.

I started to read my inbox from older to newer mails and send this
before discovering you had already send a v2.

sorry for the noise,
Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2 1/2] usb: chipidea: big-endian support

[Marc Kleine-Budde]

On 03/28/2013 10:28 AM, Alexander Shishkin wrote:
 Svetoslav Neykov svetos...@neykov.name writes:
 
 Convert between big-endian and little-endian format when accessing
 the usb controller structures which are little-endian by
 specification. Fix cases where the little-endian memory layout is
 taken for granted. The patch doesn't have any effect on the already
 supported little-endian architectures.

Has anyone tested how the cpu_to_le32 and vice versa effects the
load/store operations? Does the compiler generate full 32 bit accesses
on mips (and big endian arm) or is a byte-shift-or pattern used?

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v7 0/6] usb: chipidea: udc: bugfixes

[Marc Kleine-Budde]

On 03/27/2013 05:35 PM, Alexander Shishkin wrote:
 Michael Grzeschik m.grzesc...@pengutronix.de writes:
 
 Hi,

 this series solve some issues with the chipidea udc.

 The series is based on v3.9-rc4.

 Michael Grzeschik (6):
   usb: chipidea: udc: add attribute aligned(4) to shared memory structs
   usb: chipidea: udc: only clear active and halted bits in qhead
   usb: chipidea: udc: move ZLT flag change to ep_enable
   usb: chipidea: udc: read status of td only once in hardware_dequeue
   usb: chipidea: udc: don't truncate requests to single tds
   usb: chipidea: udc: move _ep_queue into an unlocked function
 
 Actually, do any of these patches fix tangible bugs, that can be
 reproduced? If so, you should certainly mention that. But somehow I doubt
 that, because otherwise the driver will be totally unusable.
 
 What I see is patches that bring some parts of udc code in accordance
 with the spec, which is a good thing to do, but doesn't fix any real
 bugs that bother people. Or does it?

Patch 1/7 is essential to make the udc stable on armv5, should be added
to the patch description, though.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v7 0/6] usb: chipidea: udc: bugfixes

[Marc Kleine-Budde]

On 03/27/2013 05:37 PM, Marc Kleine-Budde wrote:
 On 03/27/2013 05:35 PM, Alexander Shishkin wrote:
 Michael Grzeschik m.grzesc...@pengutronix.de writes:

 Hi,

 this series solve some issues with the chipidea udc.

 The series is based on v3.9-rc4.

 Michael Grzeschik (6):
   usb: chipidea: udc: add attribute aligned(4) to shared memory structs
   usb: chipidea: udc: only clear active and halted bits in qhead
   usb: chipidea: udc: move ZLT flag change to ep_enable
   usb: chipidea: udc: read status of td only once in hardware_dequeue
   usb: chipidea: udc: don't truncate requests to single tds
   usb: chipidea: udc: move _ep_queue into an unlocked function

 Actually, do any of these patches fix tangible bugs, that can be
 reproduced? If so, you should certainly mention that. But somehow I doubt
 that, because otherwise the driver will be totally unusable.

 What I see is patches that bring some parts of udc code in accordance
 with the spec, which is a good thing to do, but doesn't fix any real
 bugs that bother people. Or does it?
 
 Patch 1/7 is essential to make the udc stable on armv5, should be added
 to the patch description, though.

I mean 1/6.

usb: chipidea: udc: add attribute aligned(4) to shared memory structs

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: Two remain problems at chipidea driver

[Marc Kleine-Budde]

On 03/20/2013 12:04 PM, Alexander Shishkin wrote:
 Peter Chen peter.c...@freescale.com writes:
 
 On Fri, Mar 15, 2013 at 05:17:08PM +0200, Alexander Shishkin wrote:

 Eg, for tablet or phone, the dr_mode may be gadget, but the
 otg_capable = 1.

 No, because dr_mode indicates controller's capability, and not the
 current mode of operation. Why would anyone want to put *that* in a
 DT?


 OK, now I totally understand your mind of this problem. In fact, dr_mode
 is NOT controller's capability, even at its original place:
 (Documentation/devicetree/bindings/usb/fsl-usb.txt or nvidia, 
 tegra20-ehci.txt)
 dr_mode is the usb working mode.

 When we design USB system, the requirements are differ from products
 to products. 
 The phone/tablet device may only wants itself as gadget
 device, it needs to be no-response when there is a usb device plug in
 (eg usb keyboard with Micro B-to-A cable). 

 The car entertainment system or other Standard-A port system do not want
 to be enumerated when it plugs to notebook using Standard A-to-A cable.
 
 Bah. Of course, you're right. We're stuck with dr_mode till people learn
 to design middleware stacks that can handle being both host and
 peripheral.
 
 So, currently, even most of controllers are otg-capable, still most
 of designs are one working mode designed. The reason why we design 
 the dr_mode is that we want controller working mode to be decided 
 by DT without re-compile the kernel by build out the host/gadget driver.
 
 Ok, so then how about introducing *one* more parameter, something like
 dr_cap, which
 1) when specified, supersedes DCCPARAMS, so no need to read that
 register any more;
 2) when unspecified, use DCCPARAMS;
 3) can be one of host, peripheral, otg, dual_role:
- host, peripheral: initialize one role only, stick to that, no otg;
- dual_role: initialize both roles, no otg;
- otg: both roles, ci-is_otg == true.
 
 Another question now is, do we need dual_role variant for the dr_mode
 parameter?

What's the difference between the newly proposed dr_cap and the dr_mode
parameter?

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: Two remain problems at chipidea driver

[Marc Kleine-Budde]

On 03/20/2013 12:23 PM, Alexander Shishkin wrote:
 Marc Kleine-Budde m...@pengutronix.de writes:
 
 On 03/20/2013 12:04 PM, Alexander Shishkin wrote:
 Peter Chen peter.c...@freescale.com writes:

 On Fri, Mar 15, 2013 at 05:17:08PM +0200, Alexander Shishkin wrote:

 Eg, for tablet or phone, the dr_mode may be gadget, but the
 otg_capable = 1.

 No, because dr_mode indicates controller's capability, and not the
 current mode of operation. Why would anyone want to put *that* in a
 DT?


 OK, now I totally understand your mind of this problem. In fact, dr_mode
 is NOT controller's capability, even at its original place:
 (Documentation/devicetree/bindings/usb/fsl-usb.txt or nvidia, 
 tegra20-ehci.txt)
 dr_mode is the usb working mode.

 When we design USB system, the requirements are differ from products
 to products. 
 The phone/tablet device may only wants itself as gadget
 device, it needs to be no-response when there is a usb device plug in
 (eg usb keyboard with Micro B-to-A cable). 

 The car entertainment system or other Standard-A port system do not want
 to be enumerated when it plugs to notebook using Standard A-to-A cable.

 Bah. Of course, you're right. We're stuck with dr_mode till people learn
 to design middleware stacks that can handle being both host and
 peripheral.

 So, currently, even most of controllers are otg-capable, still most
 of designs are one working mode designed. The reason why we design 
 the dr_mode is that we want controller working mode to be decided 
 by DT without re-compile the kernel by build out the host/gadget driver.

 Ok, so then how about introducing *one* more parameter, something like
 dr_cap, which
 1) when specified, supersedes DCCPARAMS, so no need to read that
 register any more;
 2) when unspecified, use DCCPARAMS;
 3) can be one of host, peripheral, otg, dual_role:
- host, peripheral: initialize one role only, stick to that, no otg;
- dual_role: initialize both roles, no otg;
- otg: both roles, ci-is_otg == true.

 Another question now is, do we need dual_role variant for the dr_mode
 parameter?

 What's the difference between the newly proposed dr_cap and the dr_mode
 parameter?
 
 See the discussion above.
 
 dr_cap is what the device can actually do (host, peripheral, etc). Tells
 us which roles to initialize and wether we can access OTGSC on this
 device.
 dr_mode is what function of the device we'll be using on this particular
 board.

Sorry, I don't get why the driver needs to know what the chipidea can do
in theory (dr_cap). IMHO it should be sufficient to tell the driver what
that exact hardware it runs on can do (dr_mode). What the hardware can
do depends on the actual chipidea implementation used in that SoC and
the board the SoC is soldered on.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: Two remain problems at chipidea driver

[Marc Kleine-Budde]

On 03/20/2013 02:37 PM, Alexander Shishkin wrote:
 Marc Kleine-Budde m...@pengutronix.de writes:
 
 On 03/20/2013 12:23 PM, Alexander Shishkin wrote:
 Marc Kleine-Budde m...@pengutronix.de writes:

 On 03/20/2013 12:04 PM, Alexander Shishkin wrote:
 Peter Chen peter.c...@freescale.com writes:

 On Fri, Mar 15, 2013 at 05:17:08PM +0200, Alexander Shishkin wrote:

 Eg, for tablet or phone, the dr_mode may be gadget, but the
 otg_capable = 1.

 No, because dr_mode indicates controller's capability, and not the
 current mode of operation. Why would anyone want to put *that* in a
 DT?


 OK, now I totally understand your mind of this problem. In fact, dr_mode
 is NOT controller's capability, even at its original place:
 (Documentation/devicetree/bindings/usb/fsl-usb.txt or nvidia, 
 tegra20-ehci.txt)
 dr_mode is the usb working mode.

 When we design USB system, the requirements are differ from products
 to products. 
 The phone/tablet device may only wants itself as gadget
 device, it needs to be no-response when there is a usb device plug in
 (eg usb keyboard with Micro B-to-A cable). 

 The car entertainment system or other Standard-A port system do not want
 to be enumerated when it plugs to notebook using Standard A-to-A cable.

 Bah. Of course, you're right. We're stuck with dr_mode till people learn
 to design middleware stacks that can handle being both host and
 peripheral.

 So, currently, even most of controllers are otg-capable, still most
 of designs are one working mode designed. The reason why we design 
 the dr_mode is that we want controller working mode to be decided 
 by DT without re-compile the kernel by build out the host/gadget driver.

 Ok, so then how about introducing *one* more parameter, something like
 dr_cap, which
 1) when specified, supersedes DCCPARAMS, so no need to read that
 register any more;
 2) when unspecified, use DCCPARAMS;
 3) can be one of host, peripheral, otg, dual_role:
- host, peripheral: initialize one role only, stick to that, no otg;
- dual_role: initialize both roles, no otg;
- otg: both roles, ci-is_otg == true.

 Another question now is, do we need dual_role variant for the dr_mode
 parameter?

 What's the difference between the newly proposed dr_cap and the dr_mode
 parameter?

 See the discussion above.

 dr_cap is what the device can actually do (host, peripheral, etc). Tells
 us which roles to initialize and wether we can access OTGSC on this
 device.
 dr_mode is what function of the device we'll be using on this particular
 board.

 Sorry, I don't get why the driver needs to know what the chipidea can do
 in theory (dr_cap). IMHO it should be sufficient to tell the driver what
 that exact hardware it runs on can do (dr_mode). What the hardware can
 do depends on the actual chipidea implementation used in that SoC and
 the board the SoC is soldered on.
 
 Again, see the discussion above.
 
 In real world products (that is, phones and tablets as opposed to jolly
 fun development boards), vendors will want to limit the usb
 functionality to peripheral only or host only or whatever, because the
 middleware stack can only do one thing or because they don't want to go
 through with otg certification or you name it. Meanwhile, the controller

It's very tempting to describe the software capabilities by the device
tree. But the device tree is supposed to describe the hardware only.

 and the whole device can still support otg. And we need to know that if
 we're to try to detect vbus session, because that is done via OTGSC
 which is only available in otg configurations.

Do we need to access the OTGSC register on a OTG capable SoC which is
limited to device-only by other hardware constraints (like no ID Pin)?
In other words, is Svetoslav's patch[1] usb: chipidea: Don't access OTG
related registers sufficient?

[1] http://patchwork.linux-mips.org/patch/4940/

This patch restricts the access to the OTGSC register to the dr_mode=otg
case, which of course is only valid, if the hardware (SoC, board, etc)
really supports otg.

 The alternative is to have N more platform tweaks for
 * dccparams_is_borked,
 * otgsc_is_borked,
 and a lot of luck trying to get that right in the probe(). This is, of
 course a preferred approach for anyone who supports phandle creep in
 DT.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: Two remain problems at chipidea driver

[Marc Kleine-Budde]

On 03/20/2013 03:44 PM, Felipe Balbi wrote:
 Hi,
 
 On Wed, Mar 20, 2013 at 04:26:02PM +0200, Alexander Shishkin wrote:
 dr_cap is what the device can actually do (host, peripheral, etc). Tells
 us which roles to initialize and wether we can access OTGSC on this
 device.
 dr_mode is what function of the device we'll be using on this particular
 board.

 Sorry, I don't get why the driver needs to know what the chipidea can do
 in theory (dr_cap). IMHO it should be sufficient to tell the driver what
 that exact hardware it runs on can do (dr_mode). What the hardware can
 do depends on the actual chipidea implementation used in that SoC and
 the board the SoC is soldered on.

 Again, see the discussion above.

 In real world products (that is, phones and tablets as opposed to jolly
 fun development boards), vendors will want to limit the usb
 functionality to peripheral only or host only or whatever, because the
 middleware stack can only do one thing or because they don't want to go
 through with otg certification or you name it. Meanwhile, the controller

 that's not entirely true. A manufacturer can decide to skip OTG
 certification but still support Dual Role. Look at the whole Android
 Accessory Kit, for example.

 Sure, I was just making an example of how device capabilities can differ
 from device's intended function.

 and the whole device can still support otg. And we need to know that if
 we're to try to detect vbus session, because that is done via OTGSC
 which is only available in otg configurations.

 well, if it's only available in OTG configurations, then you make the
 same assumption in driver. If driver was compiled with OTG, you check
 OTGSC; otherwise don't.

 I'd kind of like to support different configurations in runtime and have
 as few compilation options as possible. Of course, if it means extra
 spaghetti, there's a trade off right there.
 
 right, that's what I did with drivers/usb/dwc3/, it helped cut down
 ifdeferry to a minimum. But when chromebook with Exynos5 showed up, we
 _had_ to allow manufacturers to ship the notebook without the peripheral
 side, since they'd never, ever use it. Since the code was already
 prepared for that, it was pretty simple and there's no ifdef hell
 anywhere. Below you will find original commit. The main idea is that, if
 you want a distro-like kernel, then you always build with everything
 (DRD), but if you're building a real product, as you said, you may not
 want to ship both modes unless you're really going to use them.

With the dr_mode property in the DT, you can build one kernel that
supports host, device and otg at the same time, but still limit a
particular hardware to device only mode.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: Two remain problems at chipidea driver

[Marc Kleine-Budde]

On 03/20/2013 05:02 PM, Felipe Balbi wrote:
 On Wed, Mar 20, 2013 at 04:58:05PM +0100, Marc Kleine-Budde wrote:
 On 03/20/2013 03:44 PM, Felipe Balbi wrote:
 Hi,

 On Wed, Mar 20, 2013 at 04:26:02PM +0200, Alexander Shishkin wrote:
 dr_cap is what the device can actually do (host, peripheral, etc). 
 Tells
 us which roles to initialize and wether we can access OTGSC on this
 device.
 dr_mode is what function of the device we'll be using on this 
 particular
 board.

 Sorry, I don't get why the driver needs to know what the chipidea can do
 in theory (dr_cap). IMHO it should be sufficient to tell the driver what
 that exact hardware it runs on can do (dr_mode). What the hardware can
 do depends on the actual chipidea implementation used in that SoC and
 the board the SoC is soldered on.

 Again, see the discussion above.

 In real world products (that is, phones and tablets as opposed to jolly
 fun development boards), vendors will want to limit the usb
 functionality to peripheral only or host only or whatever, because the
 middleware stack can only do one thing or because they don't want to go
 through with otg certification or you name it. Meanwhile, the controller

 that's not entirely true. A manufacturer can decide to skip OTG
 certification but still support Dual Role. Look at the whole Android
 Accessory Kit, for example.

 Sure, I was just making an example of how device capabilities can differ
 from device's intended function.

 and the whole device can still support otg. And we need to know that if
 we're to try to detect vbus session, because that is done via OTGSC
 which is only available in otg configurations.

 well, if it's only available in OTG configurations, then you make the
 same assumption in driver. If driver was compiled with OTG, you check
 OTGSC; otherwise don't.

 I'd kind of like to support different configurations in runtime and have
 as few compilation options as possible. Of course, if it means extra
 spaghetti, there's a trade off right there.

 right, that's what I did with drivers/usb/dwc3/, it helped cut down
 ifdeferry to a minimum. But when chromebook with Exynos5 showed up, we
 _had_ to allow manufacturers to ship the notebook without the peripheral
 side, since they'd never, ever use it. Since the code was already
 prepared for that, it was pretty simple and there's no ifdef hell
 anywhere. Below you will find original commit. The main idea is that, if
 you want a distro-like kernel, then you always build with everything
 (DRD), but if you're building a real product, as you said, you may not
 want to ship both modes unless you're really going to use them.

 With the dr_mode property in the DT, you can build one kernel that
 supports host, device and otg at the same time, but still limit a
 particular hardware to device only mode.
 
 that's alright. We do that with dwc3 as well. But what if you want a
 kernel with host-only ? You don't want to waste precious memory
 initializing data you will never use ;-)

Sure, even the mainline chipidea driver already allows us to build it
host or device only :D

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: Two remain problems at chipidea driver

[Marc Kleine-Budde]

On 03/14/2013 09:34 AM, Peter Chen wrote:
 Hi Alex and all,
 
 Currently, we have two problems to block chipidea driver coming
 development.
 
 As there are so many chipidea versions, we impossible to collect
 all to make a decision, it is better to cover most of the cases,
 and using device tree (or platform data) to cover exceptions
 if they exist.
 
 1. USB Mode Problem
 How can we decide USB mode (gadget, host and otg) at driver, and
 how to read OTGSC register? Below is my pinion.
 
 - We get gadget or host support from CAP_DCCPARAMS(DCCPARAMS_DC or 
 DCCPARAMS_HC),

IIRC This is broken on mx25. The host only port gets into an error state
if you read this register. :(

 If both DCCPARAMS_DC and DCCPARAMS_HC are 1, then the mode is otg.
 If DCCPARAMS_DC = 1 and DCCPARAMS_HC = 0, the mode is gadget.
 If DCCPARAMS_DC = 0 and DCCPARAMS_HC = 1, the mode is host.
 If DCCPARAMS_DC = 0 and DCCPARAMS_HC = 0, prompt an error 
 and try to get it from DT.
 
 - Override the value using DT, please do not consider too much between
 dule_role and otg. We just consider all controllers which supports host
 and gadget at the same time as otg device. The exception may not be existed
 or be too long to use.

 - For how to read OTGSC register, we need another flag to indicate
 it is otg capable (DCCPARAMS_DC = 1 and DCCPARAMS_HC = 1), if it is
 otg capable, read OTGSC is allowed. Here, OTG capable device can work
 as gadget only mode.

There is a core on mips which doesn't support otg. From my point of
view, support for dual_role should be a separate patch, ideally by
Svetoslav (Cc'ed) who has this hardware, after we've cleaned up all
dr-mode related stuff.

 2. Chipidea Core Driver DT Support
 I agree we move some core things, like vbus, operation_mode, phy to core
 driver using DT. But for clock, it is better still exist at glue
 layer, clock is input for chipidea core, chipidea core doesn't need
 to know its clock from IC point. Like clock, the wakeup setting, low
 power sequence are platform specific, they are designed by individual
 companies.

The IP core has (IIRC) 3 clock inputs (AHB, PER, IPG), depending on
Vendor and SoC Version the driver has to cope with 0-3 of these clocks.
Iff we put the clock handling into the core driver, there have to be all
three clocks (and sophisticated error handling to handle non existing
clocks).

 Let me know your opinion about these two problems and your plan for them.
 Thanks.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: Two remain problems at chipidea driver

[Marc Kleine-Budde]

On 03/14/2013 11:31 AM, Alexander Shishkin wrote:
 Peter Chen peter.c...@freescale.com writes:
 
 Hi Alex and all,

 Currently, we have two problems to block chipidea driver coming
 development.

 As there are so many chipidea versions, we impossible to collect
 all to make a decision, it is better to cover most of the cases,
 and using device tree (or platform data) to cover exceptions
 if they exist.

 1. USB Mode Problem
 How can we decide USB mode (gadget, host and otg) at driver, and
 how to read OTGSC register? Below is my pinion.

 - We get gadget or host support from CAP_DCCPARAMS(DCCPARAMS_DC or 
 DCCPARAMS_HC),
 If both DCCPARAMS_DC and DCCPARAMS_HC are 1, then the mode is otg.
 If DCCPARAMS_DC = 1 and DCCPARAMS_HC = 0, the mode is gadget.
 If DCCPARAMS_DC = 0 and DCCPARAMS_HC = 1, the mode is host.
 If DCCPARAMS_DC = 0 and DCCPARAMS_HC = 0, prompt an error 
 and try to get it from DT.

 - Override the value using DT, please do not consider too much between
 dule_role and otg. We just consider all controllers which supports host
 and gadget at the same time as otg device. The exception may not be existed
 or be too long to use.
 
 The mips platform support patchset that Svetoslav is doing is for such a
 device. In any case, I see no reason to not support all the possible
 devices.
 
 - For how to read OTGSC register, we need another flag to indicate
 it is otg capable (DCCPARAMS_DC = 1 and DCCPARAMS_HC = 1), if it is
 otg capable, read OTGSC is allowed. Here, OTG capable device can work
 as gadget only mode.
 
 I'd say, in the core driver:
   1) get dr_mode from platform data

+ get dr_mode from DT.

   2) only if that's DR_MODE_UNKNOWN (iirc), read DCCPARAMS, since it's
   not guaranteed to work across all chipideas;
   3) if dr_mode == OTG or dr_mode == UNKNOWN and DCCPARAMS has both DC
   and HC, set ci-otg, which determines if we can access otgsc
   4) if dr_mode == DUAL_ROLE, don't set ci-otg.
 
 Do you see any problems with this?
 
 2. Chipidea Core Driver DT Support
 I agree we move some core things, like vbus, operation_mode, phy to core
 driver using DT. But for clock, it is better still exist at glue
 layer, clock is input for chipidea core, chipidea core doesn't need
 to know its clock from IC point. Like clock, the wakeup setting, low
 power sequence are platform specific, they are designed by individual
 companies.
 
 Hmm, if the clock is external to chipidea, I guess it should be ok to
 keep it in the platform code as long as it is entirely contained in the
 platform code, that is, no awkward passing *clk pointers to the core and
 back.
 
 Let me know your opinion about these two problems and your plan for them.
 Thanks.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Marc Kleine-Budde]

On 03/13/2013 04:17 PM, Peter Ujfalusi wrote:
 On 03/12/2013 04:20 PM, Roger Quadros wrote:
 Adding Peter to the loop. I faintly remember him mentioning this issue before
 for beagle. We really need the deferred probe mechanism or we need to resort
 to device registering order.
 
 Yes, BeagleBoard is a good example. Long story short: we have external
 dependency and the correct way to handle that is via deferred probe.

Sure.

 As of now we are not ready to kill the legacy support but over time we should
 move as much as we can to DT only mode.

I was assuming you're already on DT only :)

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Marc Kleine-Budde]

On 03/12/2013 12:24 PM, Roger Quadros wrote:
 Add clk_rate parameter to platform data. If supplied, the
 NOP phy driver will program the clock to that rate during probe.
 
 Also add 2 flags, needs_vcc and needs_reset.
 If the flag is set and the regulator couldn't be found
 then the driver will bail out with -EPROBE_DEFER.

Is there a platform which fills out pdata.needs_vcc and
pdata.needs_reset? IMHO it makes no sense to add features for the non DT
case, if there isn't any user for it.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Marc Kleine-Budde]

On 03/12/2013 03:12 PM, Roger Quadros wrote:
 On 03/12/2013 01:54 PM, Marc Kleine-Budde wrote:
 On 03/12/2013 12:24 PM, Roger Quadros wrote:
 Add clk_rate parameter to platform data. If supplied, the
 NOP phy driver will program the clock to that rate during probe.

 Also add 2 flags, needs_vcc and needs_reset.
 If the flag is set and the regulator couldn't be found
 then the driver will bail out with -EPROBE_DEFER.

 Is there a platform which fills out pdata.needs_vcc and
 pdata.needs_reset? IMHO it makes no sense to add features for the non DT
 case, if there isn't any user for it.

 
 There can be a user in the non DT case as well. Consider the following 
 example:

I'm just saying, let the implementation up to a real user for the non DT
case. Beagleboard is ARM and there's no point of implementing non DT
fall backs for ARM, IMHO.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Marc Kleine-Budde]

On 03/12/2013 03:28 PM, Roger Quadros wrote:
 On 03/12/2013 04:17 PM, Marc Kleine-Budde wrote:
 On 03/12/2013 03:12 PM, Roger Quadros wrote:
 On 03/12/2013 01:54 PM, Marc Kleine-Budde wrote:
 On 03/12/2013 12:24 PM, Roger Quadros wrote:
 Add clk_rate parameter to platform data. If supplied, the
 NOP phy driver will program the clock to that rate during probe.

 Also add 2 flags, needs_vcc and needs_reset.
 If the flag is set and the regulator couldn't be found
 then the driver will bail out with -EPROBE_DEFER.

 Is there a platform which fills out pdata.needs_vcc and
 pdata.needs_reset? IMHO it makes no sense to add features for the non DT
 case, if there isn't any user for it.


 There can be a user in the non DT case as well. Consider the following 
 example:

 I'm just saying, let the implementation up to a real user for the non DT
 case. Beagleboard is ARM and there's no point of implementing non DT
 fall backs for ARM, IMHO.

 
 Why do you say so?
 
 It doesn't depend on the CPU architecture. It depends on how the board 
 designer
 wired the board. A non ARM platform could also face the same problem as 
 beagle.

Using -EPROBE_DEFER ist the way to solve the problem.

 It is not a non DT fallback. I believe many are still using non DT boot and it
 needs to work at least till we move all functionality to purely DT.

From my point of view, it makes no sense today to implement new features
which have a fallback for the non-DT case, if there isn't a real user of
this feature. So IMHO don't add needs_vcc and needs_reset flags to the
pdata, just take the information from the DT. As soon as there is a
non-DT user of this feature she/he can implement it or even better
switch to DT.

just my 2¢,
Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/8] usb: phy: nop: Add some parameters to platform data

[Marc Kleine-Budde]

On 03/12/2013 04:20 PM, Roger Quadros wrote:
 On 03/12/2013 04:42 PM, Marc Kleine-Budde wrote:
 On 03/12/2013 03:28 PM, Roger Quadros wrote:
 On 03/12/2013 04:17 PM, Marc Kleine-Budde wrote:
 On 03/12/2013 03:12 PM, Roger Quadros wrote:
 On 03/12/2013 01:54 PM, Marc Kleine-Budde wrote:
 On 03/12/2013 12:24 PM, Roger Quadros wrote:
 Add clk_rate parameter to platform data. If supplied, the
 NOP phy driver will program the clock to that rate during probe.

 Also add 2 flags, needs_vcc and needs_reset.
 If the flag is set and the regulator couldn't be found
 then the driver will bail out with -EPROBE_DEFER.

 Is there a platform which fills out pdata.needs_vcc and
 pdata.needs_reset? IMHO it makes no sense to add features for the non DT
 case, if there isn't any user for it.


 There can be a user in the non DT case as well. Consider the following 
 example:

 I'm just saying, let the implementation up to a real user for the non DT
 case. Beagleboard is ARM and there's no point of implementing non DT
 fall backs for ARM, IMHO.


 Why do you say so?

 It doesn't depend on the CPU architecture. It depends on how the board 
 designer
 wired the board. A non ARM platform could also face the same problem as 
 beagle.

 Using -EPROBE_DEFER ist the way to solve the problem.
 
 did you mean isn't? If you did then what is the other option?

You're using -EPROBE_DEFER, that's all perfectly fine.

 It is not a non DT fallback. I believe many are still using non DT boot and 
 it
 needs to work at least till we move all functionality to purely DT.

 From my point of view, it makes no sense today to implement new features
 which have a fallback for the non-DT case, if there isn't a real user of
 this feature. So IMHO don't add needs_vcc and needs_reset flags to the
 pdata, just take the information from the DT. As soon as there is a
 non-DT user of this feature she/he can implement it or even better
 switch to DT.
 
 Adding Peter to the loop. I faintly remember him mentioning this issue before
 for beagle. We really need the deferred probe mechanism or we need to resort
 to device registering order.

 The first user for needs_vcc flag will be the beagleboard file. I just didn't
 implement it in this patch [1].

Okay, I wrongly assumed that there wouldn't be added any new features to
the old non-DT board files.

 On second thoughts, since [1] does work on beagleboard without requiring the
 needs_vcc flag, I think we can just live without it.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Marc Kleine-Budde]

On 02/05/2013 08:26 AM, Felipe Balbi wrote:
 Hi,
 
 On Mon, Feb 04, 2013 at 05:58:48PM +0200, Roger Quadros wrote:
 The PHY clock, clock rate, VCC regulator and RESET regulator
 can now be provided via device tree.

 Signed-off-by: Roger Quadros rog...@ti.com
 ---
  .../devicetree/bindings/usb/usb-nop-xceiv.txt  |   34 
 
  drivers/usb/otg/nop-usb-xceiv.c|   31 ++
  2 files changed, 65 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt

 diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt 
 b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
 new file mode 100644
 index 000..d7e2726
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
 @@ -0,0 +1,34 @@
 +USB NOP PHY
 +
 +Required properties:
 +- compatible: should be usb-nop-xceiv
 +
 +Optional properties:
 +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
 +  /bindings/clock/clock-bindings.txt
 +  This property is required if clock-frequency is specified.
 +
 +- clock-names: Should be main_clk
 +
 +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
 +  be configured to.
 +
 +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
 +
 +- reset-supply: phandle to the regulator that provides power to the PHY.
 +
 +Example:
 +
 +hsusb1_phy {
 +compatible = usb-nop-xceiv;
 +clock-frequency = 1920;
 +clocks = osc 0;
 +clock-names = main_clk;
 +vcc-supply = hsusb1_vcc_regulator;
 +reset-supply = hsusb1_reset_regulator;
 +};
 +
 +hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator
 +and expects that clock to be configured to 19.2MHz by the NOP PHY driver.
 +hsusb1_vcc_regulator provides power to the PHY and hsusb1_reset_regulator
 +controls RESET.
 diff --git a/drivers/usb/otg/nop-usb-xceiv.c 
 b/drivers/usb/otg/nop-usb-xceiv.c
 index ac027a1..adbb7ab 100644
 --- a/drivers/usb/otg/nop-usb-xceiv.c
 +++ b/drivers/usb/otg/nop-usb-xceiv.c
 @@ -34,6 +34,7 @@
  #include linux/slab.h
  #include linux/clk.h
  #include linux/regulator/consumer.h
 +#include linux/of.h
  
  struct nop_usb_xceiv {
  struct usb_phy  phy;
 @@ -138,8 +139,19 @@ static int nop_set_host(struct usb_otg *otg, struct 
 usb_bus *host)
  return 0;
  }
  
 +static void nop_xeiv_get_dt_pdata(struct device *dev,
 
 asking to remove, but xeiv != xceiv :-)
 
 +struct nop_usb_xceiv_platform_data *pdata)
 +{
 +struct device_node *node = dev-of_node;
 +u32 clk_rate;
 +
 +if (!of_property_read_u32(node, clock-frequency, clk_rate))
 +pdata-clk_rate = clk_rate;
 +}
 +
  static int nop_usb_xceiv_probe(struct platform_device *pdev)
  {
 +struct device *dev = pdev-dev;
  struct nop_usb_xceiv_platform_data *pdata = pdev-dev.platform_data;
  struct nop_usb_xceiv*nop;
  enum usb_phy_type   type = USB_PHY_TYPE_USB2;
 @@ -153,6 +165,17 @@ static int nop_usb_xceiv_probe(struct platform_device 
 *pdev)
  if (!nop-phy.otg)
  return -ENOMEM;
  
 +if (dev-of_node) {
 +pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
 +if (!pdata) {
 +dev_err(dev, Memory allocation failure\n);
 +return -ENOMEM;
 +}
 +nop_xeiv_get_dt_pdata(dev, pdata);
 
 actually, I would prefer to not create pdata at all. I mean, ideally
 pdata would be used to initialize fields in your own structure, so first
 move clk_rate to your own private structure, copy pdata's clk_rate value
 to that, then you don't need this hackery when using DT.

As far as I can see, clk_rate is never used, but in the probe function.
Why should it be saved into the private data structure at all?

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 02/13] USB: phy: nop: Defer probe if device needs VCC/RESET

[Marc Kleine-Budde]

On 02/05/2013 10:43 AM, Roger Quadros wrote:
 On 02/05/2013 11:09 AM, Felipe Balbi wrote:
 On Tue, Feb 05, 2013 at 10:44:05AM +0200, Roger Quadros wrote:
 diff --git a/include/linux/usb/nop-usb-xceiv.h 
 b/include/linux/usb/nop-usb-xceiv.h
 index 3265b61..148d351 100644
 --- a/include/linux/usb/nop-usb-xceiv.h
 +++ b/include/linux/usb/nop-usb-xceiv.h
 @@ -6,6 +6,10 @@
   struct nop_usb_xceiv_platform_data {
   enum usb_phy_type type;
   unsigned long clk_rate;
 +
 +/* if set fails with -EPROBE_DEFER if can't get regulator */
 +unsigned int needs_vcc:1;
 +unsigned int needs_reset:1;

 how about u8 here?

 Not sure. Bitfields are usually defined as unsigned int.

 IIRC the benefit is that compiler can try to optimize those flags. I
 mean, if you have 32 1-bit flags, compiler will combine those in a
 single u32. Someone correct me if I'm wrong.

 
 Yes you are right. Kishon was asking me to use u8 instead of unsigned int, 
 which
 I don't think is necessary. AFAIK, it is a norm to use unsigned int when 
 defining
 a bitfield. Compilers are known to behave funny with bitfields. I don't mind
 using bool for each.

In the current version (rogerq/v3.8-usbhost17-dt) of this patch you've
put both needs_* into the private data struct and the pdata, but it's
only used inside the probe function.

regards,
Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Marc Kleine-Budde]

On 02/04/2013 04:58 PM, Roger Quadros wrote:
 The PHY clock, clock rate, VCC regulator and RESET regulator
 can now be provided via device tree.
 
 Signed-off-by: Roger Quadros rog...@ti.com
 ---
  .../devicetree/bindings/usb/usb-nop-xceiv.txt  |   34 
 
  drivers/usb/otg/nop-usb-xceiv.c|   31 ++
  2 files changed, 65 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
 
 diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt 
 b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
 new file mode 100644
 index 000..d7e2726
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
 @@ -0,0 +1,34 @@
 +USB NOP PHY
 +
 +Required properties:
 +- compatible: should be usb-nop-xceiv
 +
 +Optional properties:
 +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
 +  /bindings/clock/clock-bindings.txt
 +  This property is required if clock-frequency is specified.
 +
 +- clock-names: Should be main_clk
 +
 +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
 +  be configured to.
 +
 +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
 +
 +- reset-supply: phandle to the regulator that provides power to the PHY.
 +
 +Example:
 +
 + hsusb1_phy {
 + compatible = usb-nop-xceiv;
 + clock-frequency = 1920;

Why do you hardcode the clock frequency here? You should use
clk_get_rate() to get the frequency from the clock tree.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 01/13] usb: phy: nop: Add device tree support and binding information

[Marc Kleine-Budde]

On 03/08/2013 11:46 AM, Marc Kleine-Budde wrote:
 On 02/04/2013 04:58 PM, Roger Quadros wrote:
 The PHY clock, clock rate, VCC regulator and RESET regulator
 can now be provided via device tree.

 Signed-off-by: Roger Quadros rog...@ti.com
 ---
  .../devicetree/bindings/usb/usb-nop-xceiv.txt  |   34 
 
  drivers/usb/otg/nop-usb-xceiv.c|   31 ++
  2 files changed, 65 insertions(+), 0 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt

 diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt 
 b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
 new file mode 100644
 index 000..d7e2726
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
 @@ -0,0 +1,34 @@
 +USB NOP PHY
 +
 +Required properties:
 +- compatible: should be usb-nop-xceiv
 +
 +Optional properties:
 +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree
 +  /bindings/clock/clock-bindings.txt
 +  This property is required if clock-frequency is specified.
 +
 +- clock-names: Should be main_clk
 +
 +- clock-frequency: the clock frequency (in Hz) that the PHY clock must
 +  be configured to.
 +
 +- vcc-supply: phandle to the regulator that provides RESET to the PHY.
 +
 +- reset-supply: phandle to the regulator that provides power to the PHY.
 +
 +Example:
 +
 +hsusb1_phy {
 +compatible = usb-nop-xceiv;
 +clock-frequency = 1920;
 
 Why do you hardcode the clock frequency here? You should use
 clk_get_rate() to get the frequency from the clock tree.

What about declaring a fixed-clock node in the device tree? Then it
should be possible to keep the driver free of any omap specific code.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 3/4] USB mxs-phy: use readl(), writel() instead of the _relaxed() versions

[Marc Kleine-Budde]

On 02/28/2013 12:25 PM, Alexander Shishkin wrote:
 Marc Kleine-Budde m...@pengutronix.de writes:
 
 This patch converts the mxs-phy driver from readl_relaxed(), writel_relaxed()
 to the plain readl(), writel() functions, which are available on all 
 platforms.
 This is done to enable compile time testing on non ARM platforms.

 Reported-by: Alexander Shishkin alexander.shish...@linux.intel.com
 
 I think it was Felipe who reported this actually.

Fixed. I'm about to repost this series with crediting Felipe as reporter.

Which tree would you like to be this series based on? It depends on the
USB otg: use try_module_get in all usb_get_phy functions and add
missing module_put patch which is part of your usb/fixes branch, but
not in usb/next yet.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 3/4] USB mxs-phy: use readl(), writel() instead of the _relaxed() versions

[Marc Kleine-Budde]

On 03/08/2013 05:28 PM, Felipe Balbi wrote:
 Hi,
 
 On Fri, Mar 08, 2013 at 05:05:58PM +0100, Marc Kleine-Budde wrote:
 On 02/28/2013 12:25 PM, Alexander Shishkin wrote:
 Marc Kleine-Budde m...@pengutronix.de writes:

 This patch converts the mxs-phy driver from readl_relaxed(), 
 writel_relaxed()
 to the plain readl(), writel() functions, which are available on all 
 platforms.
 This is done to enable compile time testing on non ARM platforms.

 Reported-by: Alexander Shishkin alexander.shish...@linux.intel.com

 I think it was Felipe who reported this actually.

 Fixed. I'm about to repost this series with crediting Felipe as reporter.

 Which tree would you like to be this series based on? It depends on the
 USB otg: use try_module_get in all usb_get_phy functions and add
 missing module_put patch which is part of your usb/fixes branch, but
 not in usb/next yet.
 
 look at my testing branch. I have them all already. You didn't get an
 email yet because I'm waiting for -rc2 to be tagged.

Thanks.

 I have fixed the Reported-by tag, no issues there.
 
 Anyway, as soon as v3.9-rc2 is tagged, I will rebase testing on top of
 that and move all patches to 'next', then everything will be stable from
 that point on.
 
 If you want to have changes to any of the patches, now is the time,
 after I move patches to 'next' the tree becomes immutable.

Nope, I just wanted to keep track of the patches.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 5/5] USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy

[Marc Kleine-Budde]

On 03/08/2013 05:33 PM, Felipe Balbi wrote:
 On Thu, Feb 28, 2013 at 11:57:04AM +0100, Marc Kleine-Budde wrote:
 @@ -147,19 +146,20 @@ static int ci13xxx_imx_probe(struct platform_device 
 *pdev)
  return ret;
  }
  
 -phy_np = of_parse_phandle(pdev-dev.of_node, fsl,usbphy, 0);
 -if (phy_np) {
 -data-phy_np = phy_np;
 -phy_pdev = of_find_device_by_node(phy_np);
 -if (phy_pdev) {
 -struct usb_phy *phy;
 -phy = pdev_to_phy(phy_pdev);
 -if (phy 
 -try_module_get(phy_pdev-dev.driver-owner)) {
 -usb_phy_init(phy);
 -data-phy = phy;
 -}
 +phy = devm_usb_get_phy_by_phandle(pdev-dev, fsl,usbphy, 0);
 
 very nice, but should be done at chipidea core.

Any suggestions for the phandle name? chipidea,usbphy?

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 3/5] USB chipidea: introduce dual role mode pdata flags

[Marc Kleine-Budde]

On 03/08/2013 05:46 PM, Alexander Shishkin wrote:
 On 8 March 2013 18:33, Felipe Balbi ba...@ti.com wrote:
 Hi,

 On Thu, Feb 28, 2013 at 11:57:02AM +0100, Marc Kleine-Budde wrote:
 @@ -487,14 +488,23 @@ static int ci_hdrc_probe(struct platform_device *pdev)
   return -ENODEV;
   }

 + /* For now we treat dual-role as otg */
 + dr_mode = ci-platdata-dr_mode;
 + if (dr_mode == USB_DR_MODE_UNKNOWN || dr_mode == 
 USB_DR_MODE_DUAL_ROLE)
 + dr_mode = USB_DR_MODE_OTG;
 +
   /* initialize role(s) before the interrupt is requested */
 - ret = ci_hdrc_host_init(ci);
 - if (ret)
 - dev_info(dev, doesn't support host\n);
 + if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {

 this is not something you should be passing via pdata; chipidea core
 should know how to read this data by itself. Meaning that chipidea core
 should be taught about devicetree. But make it optional since now all
 users use DT.
 
 And I don't think I like the idea of chipidea core calling into device
 tree code directly.

Hmmmthis means draw :)

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 3/3] USB mxs-phy: Register phy with framework

[Marc Kleine-Budde]

On 02/28/2013 09:01 AM, Felipe Balbi wrote:
 hi,
 
 On Wed, Feb 27, 2013 at 03:16:30PM +0100, Marc Kleine-Budde wrote:
 From: Sascha Hauer s.ha...@pengutronix.de

 We now have usb_add_phy_dev(), so use it to register with the framework
 to be able to find the phy from the USB driver.

 Reviewed-by: Kishon Vijay Abraham I kis...@ti.com
 Reviewed-by: Peter Chen peter.c...@freescale.com
 Acked-by: Felipe Balbi ba...@ti.com
 Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 
 any chance you can move away from {write,read}[bwl]_relaxed() so we can
 build this driver on other architectures ?

The hardware is in the ARM imx2{2,8} only. Another option would be to
add an depends on ARCH_ARM.

Marc


-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 2/3] USB: add devicetree helpers for determining dr_mode and phy_type

[Marc Kleine-Budde]

On 02/28/2013 08:57 AM, Felipe Balbi wrote:
 On Wed, Feb 27, 2013 at 03:16:29PM +0100, Marc Kleine-Budde wrote:
 From: Michael Grzeschik m.grzesc...@pengutronix.de

 This adds two little devicetree helper functions for determining the dr_mode
 (host, peripheral, otg, dual-role) and phy_type (utmi, ulpi,...) from the
 devicetree.

 Cc: Felipe Balbi ba...@ti.com
 Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
 Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 
 causes build breakage:
 
 include/linux/usb/of.h:21:32: error: return type is an incomplete type
 include/linux/usb/of.h: In function ‘of_usb_get_dr_mode’:
 include/linux/usb/of.h:23:9: error: ‘USB_DR_MODE_UNKNOWN’ undeclared (first 
 use in this function)
 include/linux/usb/of.h:23:9: note: each undeclared identifier is reported 
 only once for each function it appears in
 include/linux/usb/of.h:23:2: warning: ‘return’ with a value, in function 
 returning void [enabled by default]
 make[1]: *** [drivers/usb/usb-common.o] Error 1

Doh! That occurs only for non DT kernels, but who doesn't use device
tree these days?

Fixed,
Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

[PATCH 2/4] USB: add devicetree helpers for determining dr_mode and phy_type

[Marc Kleine-Budde]

From: Michael Grzeschik m.grzesc...@pengutronix.de

This adds two little devicetree helper functions for determining the dr_mode
(host, peripheral, otg, dual-role) and phy_type (utmi, ulpi,...) from the
devicetree.

Cc: Felipe Balbi ba...@ti.com
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 drivers/usb/phy/Makefile |1 +
 drivers/usb/phy/of.c |   47 ++
 drivers/usb/usb-common.c |   37 
 include/linux/usb/of.h   |   28 +++
 include/linux/usb/otg.h  |8 
 include/linux/usb/phy.h  |9 +
 6 files changed, 130 insertions(+)
 create mode 100644 drivers/usb/phy/of.c
 create mode 100644 include/linux/usb/of.h

diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 9fa6327..e1be1e8 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -5,6 +5,7 @@
 ccflags-$(CONFIG_USB_DEBUG) := -DDEBUG
 
 obj-$(CONFIG_USB_OTG_UTILS)+= phy.o
+obj-$(CONFIG_OF)   += of.o
 obj-$(CONFIG_OMAP_USB2)+= omap-usb2.o
 obj-$(CONFIG_OMAP_USB3)+= omap-usb3.o
 obj-$(CONFIG_OMAP_CONTROL_USB) += omap-control-usb.o
diff --git a/drivers/usb/phy/of.c b/drivers/usb/phy/of.c
new file mode 100644
index 000..e6f3b74
--- /dev/null
+++ b/drivers/usb/phy/of.c
@@ -0,0 +1,47 @@
+/*
+ * USB of helper code
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/of.h
+#include linux/usb/of.h
+#include linux/usb/otg.h
+
+static const char *usbphy_modes[] = {
+   [USBPHY_INTERFACE_MODE_UNKNOWN] = ,
+   [USBPHY_INTERFACE_MODE_UTMI]= utmi,
+   [USBPHY_INTERFACE_MODE_UTMIW]   = utmi_wide,
+   [USBPHY_INTERFACE_MODE_ULPI]= ulpi,
+   [USBPHY_INTERFACE_MODE_SERIAL]  = serial,
+   [USBPHY_INTERFACE_MODE_HSIC]= hsic,
+};
+
+/**
+ * of_usb_get_phy_mode - Get phy mode for given device_node
+ * @np:Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'phy_type',
+ * and returns the correspondig enum usb_phy_interface
+ */
+enum usb_phy_interface of_usb_get_phy_mode(struct device_node *np)
+{
+   const char *phy_type;
+   int err, i;
+
+   err = of_property_read_string(np, phy_type, phy_type);
+   if (err  0)
+   return USBPHY_INTERFACE_MODE_UNKNOWN;
+
+   for (i = 0; i  ARRAY_SIZE(usbphy_modes); i++)
+   if (!strcmp(phy_type, usbphy_modes[i]))
+   return i;
+
+   return USBPHY_INTERFACE_MODE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_phy_mode);
diff --git a/drivers/usb/usb-common.c b/drivers/usb/usb-common.c
index d29503e..18b994b 100644
--- a/drivers/usb/usb-common.c
+++ b/drivers/usb/usb-common.c
@@ -14,6 +14,9 @@
 #include linux/kernel.h
 #include linux/module.h
 #include linux/usb/ch9.h
+#include linux/of.h
+#include linux/usb/of.h
+#include linux/usb/otg.h
 
 const char *usb_speed_string(enum usb_device_speed speed)
 {
@@ -32,4 +35,38 @@ const char *usb_speed_string(enum usb_device_speed speed)
 }
 EXPORT_SYMBOL_GPL(usb_speed_string);
 
+#ifdef CONFIG_OF
+static const char *usb_dr_modes[] = {
+   [USB_DR_MODE_UNKNOWN]   = ,
+   [USB_DR_MODE_HOST]  = host,
+   [USB_DR_MODE_PERIPHERAL]= peripheral,
+   [USB_DR_MODE_OTG]   = otg,
+   [USB_DR_MODE_DUAL_ROLE] = dual-role,
+};
+
+/**
+ * of_usb_get_dr_mode - Get dual role mode for given device_node
+ * @np:Pointer to the given device_node
+ *
+ * The function gets phy interface string from property 'dr_mode',
+ * and returns the correspondig enum usb_dr_mode
+ */
+enum usb_dr_mode of_usb_get_dr_mode(struct device_node *np)
+{
+   const char *dr_mode;
+   int err, i;
+
+   err = of_property_read_string(np, dr_mode, dr_mode);
+   if (err  0)
+   return USB_DR_MODE_UNKNOWN;
+
+   for (i = 0; i  ARRAY_SIZE(usb_dr_modes); i++)
+   if (!strcmp(dr_mode, usb_dr_modes[i]))
+   return i;
+
+   return USB_DR_MODE_UNKNOWN;
+}
+EXPORT_SYMBOL_GPL(of_usb_get_dr_mode);
+#endif
+
 MODULE_LICENSE(GPL);
diff --git a/include/linux/usb/of.h b/include/linux/usb/of.h
new file mode 100644
index 000..fc98f68
--- /dev/null
+++ b/include/linux/usb/of.h
@@ -0,0 +1,28 @@
+/*
+ * OF helpers for usb devices.
+ *
+ * This file is released under the GPLv2
+ */
+
+#ifndef __LINUX_USB_OF_H
+#define __LINUX_USB_OF_H
+
+#include linux/usb/phy.h
+#include linux/usb/otg.h
+
+#ifdef CONFIG_OF
+enum usb_phy_interface

[PATCH 4/4] USB mxs-phy: Register phy with framework

[Marc Kleine-Budde]

From: Sascha Hauer s.ha...@pengutronix.de

We now have usb_add_phy_dev(), so use it to register with the framework
to be able to find the phy from the USB driver.

Reviewed-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Peter Chen peter.c...@freescale.com
Acked-by: Felipe Balbi ba...@ti.com
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 drivers/usb/otg/mxs-phy.c |9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
index dff10f2..9d4381e 100644
--- a/drivers/usb/otg/mxs-phy.c
+++ b/drivers/usb/otg/mxs-phy.c
@@ -127,6 +127,7 @@ static int mxs_phy_probe(struct platform_device *pdev)
void __iomem *base;
struct clk *clk;
struct mxs_phy *mxs_phy;
+   int ret;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -166,11 +167,19 @@ static int mxs_phy_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, mxs_phy-phy);
 
+   ret = usb_add_phy_dev(mxs_phy-phy);
+   if (ret)
+   return ret;
+
return 0;
 }
 
 static int mxs_phy_remove(struct platform_device *pdev)
 {
+   struct mxs_phy *mxs_phy = platform_get_drvdata(pdev);
+
+   usb_remove_phy(mxs_phy-phy);
+
platform_set_drvdata(pdev, NULL);
 
return 0;
-- 
1.7.10.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 3/4] USB mxs-phy: use readl(), writel() instead of the _relaxed() versions

[Marc Kleine-Budde]

This patch converts the mxs-phy driver from readl_relaxed(), writel_relaxed()
to the plain readl(), writel() functions, which are available on all platforms.
This is done to enable compile time testing on non ARM platforms.

Reported-by: Alexander Shishkin alexander.shish...@linux.intel.com
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 drivers/usb/otg/mxs-phy.c |   32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c
index b0d9f11..dff10f2 100644
--- a/drivers/usb/otg/mxs-phy.c
+++ b/drivers/usb/otg/mxs-phy.c
@@ -48,12 +48,12 @@ static void mxs_phy_hw_init(struct mxs_phy *mxs_phy)
stmp_reset_block(base + HW_USBPHY_CTRL);
 
/* Power up the PHY */
-   writel_relaxed(0, base + HW_USBPHY_PWD);
+   writel(0, base + HW_USBPHY_PWD);
 
/* enable FS/LS device */
-   writel_relaxed(BM_USBPHY_CTRL_ENUTMILEVEL2 |
-   BM_USBPHY_CTRL_ENUTMILEVEL3,
-   base + HW_USBPHY_CTRL_SET);
+   writel(BM_USBPHY_CTRL_ENUTMILEVEL2 |
+  BM_USBPHY_CTRL_ENUTMILEVEL3,
+  base + HW_USBPHY_CTRL_SET);
 }
 
 static int mxs_phy_init(struct usb_phy *phy)
@@ -70,8 +70,8 @@ static void mxs_phy_shutdown(struct usb_phy *phy)
 {
struct mxs_phy *mxs_phy = to_mxs_phy(phy);
 
-   writel_relaxed(BM_USBPHY_CTRL_CLKGATE,
-   phy-io_priv + HW_USBPHY_CTRL_SET);
+   writel(BM_USBPHY_CTRL_CLKGATE,
+  phy-io_priv + HW_USBPHY_CTRL_SET);
 
clk_disable_unprepare(mxs_phy-clk);
 }
@@ -81,15 +81,15 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend)
struct mxs_phy *mxs_phy = to_mxs_phy(x);
 
if (suspend) {
-   writel_relaxed(0x, x-io_priv + HW_USBPHY_PWD);
-   writel_relaxed(BM_USBPHY_CTRL_CLKGATE,
-   x-io_priv + HW_USBPHY_CTRL_SET);
+   writel(0x, x-io_priv + HW_USBPHY_PWD);
+   writel(BM_USBPHY_CTRL_CLKGATE,
+  x-io_priv + HW_USBPHY_CTRL_SET);
clk_disable_unprepare(mxs_phy-clk);
} else {
clk_prepare_enable(mxs_phy-clk);
-   writel_relaxed(BM_USBPHY_CTRL_CLKGATE,
-   x-io_priv + HW_USBPHY_CTRL_CLR);
-   writel_relaxed(0, x-io_priv + HW_USBPHY_PWD);
+   writel(BM_USBPHY_CTRL_CLKGATE,
+  x-io_priv + HW_USBPHY_CTRL_CLR);
+   writel(0, x-io_priv + HW_USBPHY_PWD);
}
 
return 0;
@@ -102,8 +102,8 @@ static int mxs_phy_on_connect(struct usb_phy *phy,
(speed == USB_SPEED_HIGH) ? high : non-high);
 
if (speed == USB_SPEED_HIGH)
-   writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
-   phy-io_priv + HW_USBPHY_CTRL_SET);
+   writel(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
+  phy-io_priv + HW_USBPHY_CTRL_SET);
 
return 0;
 }
@@ -115,8 +115,8 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy,
(speed == USB_SPEED_HIGH) ? high : non-high);
 
if (speed == USB_SPEED_HIGH)
-   writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
-   phy-io_priv + HW_USBPHY_CTRL_CLR);
+   writel(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
+  phy-io_priv + HW_USBPHY_CTRL_CLR);
 
return 0;
 }
-- 
1.7.10.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/4] USB: move bulk of otg/otg.c to phy/phy.c

[Marc Kleine-Budde]

From: Sascha Hauer s.ha...@pengutronix.de

Most of otg/otg.c is not otg specific, but phy specific, so move it
to the phy directory.

Cc: Felipe Balbi ba...@ti.com
Reported-by: Kishon Vijay Abraham I kis...@ti.com
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 drivers/usb/otg/otg.c|  427 
 drivers/usb/phy/Makefile |1 +
 drivers/usb/phy/phy.c|  438 ++
 3 files changed, 439 insertions(+), 427 deletions(-)
 create mode 100644 drivers/usb/phy/phy.c

diff --git a/drivers/usb/otg/otg.c b/drivers/usb/otg/otg.c
index 2bd03d2..358cfd9 100644
--- a/drivers/usb/otg/otg.c
+++ b/drivers/usb/otg/otg.c
@@ -8,436 +8,9 @@
  * the Free Software Foundation; either version 2 of the License, or
  * (at your option) any later version.
  */
-
-#include linux/kernel.h
 #include linux/export.h
-#include linux/err.h
-#include linux/device.h
-#include linux/module.h
-#include linux/slab.h
-#include linux/of.h
-
 #include linux/usb/otg.h
 
-static LIST_HEAD(phy_list);
-static LIST_HEAD(phy_bind_list);
-static DEFINE_SPINLOCK(phy_lock);
-
-static struct usb_phy *__usb_find_phy(struct list_head *list,
-   enum usb_phy_type type)
-{
-   struct usb_phy  *phy = NULL;
-
-   list_for_each_entry(phy, list, head) {
-   if (phy-type != type)
-   continue;
-
-   return phy;
-   }
-
-   return ERR_PTR(-ENODEV);
-}
-
-static struct usb_phy *__usb_find_phy_dev(struct device *dev,
-   struct list_head *list, u8 index)
-{
-   struct usb_phy_bind *phy_bind = NULL;
-
-   list_for_each_entry(phy_bind, list, list) {
-   if (!(strcmp(phy_bind-dev_name, dev_name(dev))) 
-   phy_bind-index == index) {
-   if (phy_bind-phy)
-   return phy_bind-phy;
-   else
-   return ERR_PTR(-EPROBE_DEFER);
-   }
-   }
-
-   return ERR_PTR(-ENODEV);
-}
-
-static struct usb_phy *__of_usb_find_phy(struct device_node *node)
-{
-   struct usb_phy  *phy;
-
-   list_for_each_entry(phy, phy_list, head) {
-   if (node != phy-dev-of_node)
-   continue;
-
-   return phy;
-   }
-
-   return ERR_PTR(-ENODEV);
-}
-
-static void devm_usb_phy_release(struct device *dev, void *res)
-{
-   struct usb_phy *phy = *(struct usb_phy **)res;
-
-   usb_put_phy(phy);
-}
-
-static int devm_usb_phy_match(struct device *dev, void *res, void *match_data)
-{
-   return res == match_data;
-}
-
-/**
- * devm_usb_get_phy - find the USB PHY
- * @dev - device that requests this phy
- * @type - the type of the phy the controller requires
- *
- * Gets the phy using usb_get_phy(), and associates a device with it using
- * devres. On driver detach, release function is invoked on the devres data,
- * then, devres data is freed.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *devm_usb_get_phy(struct device *dev, enum usb_phy_type type)
-{
-   struct usb_phy **ptr, *phy;
-
-   ptr = devres_alloc(devm_usb_phy_release, sizeof(*ptr), GFP_KERNEL);
-   if (!ptr)
-   return NULL;
-
-   phy = usb_get_phy(type);
-   if (!IS_ERR(phy)) {
-   *ptr = phy;
-   devres_add(dev, ptr);
-   } else
-   devres_free(ptr);
-
-   return phy;
-}
-EXPORT_SYMBOL(devm_usb_get_phy);
-
-/**
- * usb_get_phy - find the USB PHY
- * @type - the type of the phy the controller requires
- *
- * Returns the phy driver, after getting a refcount to it; or
- * -ENODEV if there is no such phy.  The caller is responsible for
- * calling usb_put_phy() to release that count.
- *
- * For use by USB host and peripheral drivers.
- */
-struct usb_phy *usb_get_phy(enum usb_phy_type type)
-{
-   struct usb_phy  *phy = NULL;
-   unsigned long   flags;
-
-   spin_lock_irqsave(phy_lock, flags);
-
-   phy = __usb_find_phy(phy_list, type);
-   if (IS_ERR(phy) || !try_module_get(phy-dev-driver-owner)) {
-   pr_err(unable to find transceiver of type %s\n,
-   usb_phy_type_string(type));
-   goto err0;
-   }
-
-   get_device(phy-dev);
-
-err0:
-   spin_unlock_irqrestore(phy_lock, flags);
-
-   return phy;
-}
-EXPORT_SYMBOL(usb_get_phy);
-
- /**
- * devm_usb_get_phy_by_phandle - find the USB PHY by phandle
- * @dev - device that requests this phy
- * @phandle - name of the property holding the phy phandle value
- * @index - the index of the phy
- *
- * Returns the phy driver associated with the given phandle value,
- * after getting a refcount to it, -ENODEV if there is no such phy or
- * -EPROBE_DEFER if there is a phandle to the phy, but the device is
- * not yet loaded. While at that, it also associates the device with
- * the phy

[PATCH 0/4] otg-for-v3.10-v2: separate phy code and add DT helper

[Marc Kleine-Budde]

Hello,

this series depends on the bugfix patch USB otg: use try_module_get in all
usb_get_phy functions and add missing module_put (a.k.a tags/otg-for-v3.9-v1)
posted earlier and is inteded for v3.10. It separates the phy from the otg code
and adds DT helper functions. In mxs-phy the {read,write}l_relaxed() functions
are replaced by the non _relaxed version to improve compile time coverage.
Further mxs-phy makes now use of the new usb_add_phy_dev() function to register
it's phy.

regards,
Marc

---

changes since v1:
- fix compile time breakage on non DT platforms (tnx, Alexander)
- convert mxs-phy to non _relaxed {read,write}l_relaxed() functions
  (as requested by Alexander)

---

The following changes since commit 6bef020b4aebd7886281fb7fb37c788d5a365eea:

  USB otg: use try_module_get in all usb_get_phy functions and add missing 
module_put (2013-02-27 12:53:15 +0100)

are available in the git repository at:

  git://git.pengutronix.de/git/mkl/linux.git tags/otg-for-v3.10-v2

for you to fetch changes up to f5678b135967ea98256ee5df9a360b5769861d23:

  USB mxs-phy: Register phy with framework (2013-02-28 11:36:45 +0100)


USB otg, phy: separate phy and add DT helper

Move phy related code into separate file and add device tree helper functions.


Marc Kleine-Budde (1):
  USB mxs-phy: use readl(), writel() instead of the _relaxed() versions

Michael Grzeschik (1):
  USB: add devicetree helpers for determining dr_mode and phy_type

Sascha Hauer (2):
  USB: move bulk of otg/otg.c to phy/phy.c
  USB mxs-phy: Register phy with framework

 drivers/usb/otg/mxs-phy.c |   41 +++--
 drivers/usb/otg/otg.c |  427 ---
 drivers/usb/phy/Makefile  |2 +
 drivers/usb/phy/of.c  |   47 +
 drivers/usb/phy/phy.c |  438 +
 drivers/usb/usb-common.c  |   37 
 include/linux/usb/of.h|   28 +++
 include/linux/usb/otg.h   |8 +
 include/linux/usb/phy.h   |9 +
 9 files changed, 594 insertions(+), 443 deletions(-)
 create mode 100644 drivers/usb/phy/of.c
 create mode 100644 drivers/usb/phy/phy.c
 create mode 100644 include/linux/usb/of.h

--
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 3/3] USB mxs-phy: Register phy with framework

[Marc Kleine-Budde]

On 02/28/2013 10:40 AM, Alexander Shishkin wrote:
 Marc Kleine-Budde m...@pengutronix.de writes:
 
 On 02/28/2013 09:01 AM, Felipe Balbi wrote:
 hi,

 On Wed, Feb 27, 2013 at 03:16:30PM +0100, Marc Kleine-Budde wrote:
 From: Sascha Hauer s.ha...@pengutronix.de

 We now have usb_add_phy_dev(), so use it to register with the framework
 to be able to find the phy from the USB driver.

 Reviewed-by: Kishon Vijay Abraham I kis...@ti.com
 Reviewed-by: Peter Chen peter.c...@freescale.com
 Acked-by: Felipe Balbi ba...@ti.com
 Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de

 any chance you can move away from {write,read}[bwl]_relaxed() so we can
 build this driver on other architectures ?

 The hardware is in the ARM imx2{2,8} only. Another option would be to
 add an depends on ARCH_ARM.
 
 Doesn't mean that we shouldn't be able to compile-test the driver.

Fixed in the just posted otg-for-v3.10-v2 series.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

[PATCH 0/5] chipidea-for-v3.10-v2: USB chipidea: make use of DT helpers in chipidea driver improve driver

[Marc Kleine-Budde]

Hello,

this series depends on the series [PATCH 0/3] otg-for-v3.10-v2: separate phy
code and add DT helper (a.k.a. tags/otg-for-v3.10-v2) posted earlier and is
intended for v3.10. The chipidea driver is converted to make use of the DT
helper functions.

No changes since v1, just rebased to otg-for-v3.10-v2, in case someone wants to
pull it.

regards,
Marc

---

The following changes since commit f5678b135967ea98256ee5df9a360b5769861d23:

  USB mxs-phy: Register phy with framework (2013-02-28 11:36:45 +0100)

are available in the git repository at:

  git://git.pengutronix.de/git/mkl/linux.git tags/chipidea-for-v3.10-v2

for you to fetch changes up to 11e56207b94d65f92acdee17c795753378c581c6:

  USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy (2013-02-28 
11:36:48 +0100)


USB chipidea: make use of DT helpers in chipidea driver

Make use of DT helper functions for handling the dr_mode and phy_type property.


Michael Grzeschik (2):
  USB chipidea: ci13xxx-imx: create dynamic platformdata
  USB chipidea: add PTW and PTS handling

Sascha Hauer (3):
  USB chipidea: introduce dual role mode pdata flags
  USB chipidea i.MX: introduce dr_mode property
  USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy

 .../devicetree/bindings/usb/ci13xxx-imx.txt|6 ++
 drivers/usb/chipidea/bits.h|   14 +++-
 drivers/usb/chipidea/ci13xxx_imx.c |   67 +++-
 drivers/usb/chipidea/core.c|   61 --
 include/linux/usb/chipidea.h   |3 +-
 5 files changed, 112 insertions(+), 39 deletions(-)


--
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 5/5] USB chipidea i.MX: use devm_usb_get_phy_by_phandle to get phy

[Marc Kleine-Budde]

From: Sascha Hauer s.ha...@pengutronix.de

This patch replaces the hand crafted code to retrieve the phy's phandle from
the DT by the helper function devm_usb_get_phy_by_phandle() which has been
added in commit:

5d3c28b usb: otg: add device tree support to otg library

Reviewed-by: Kishon Vijay Abraham I kis...@ti.com
Reviewed-by: Peter Chen peter.c...@freescale.com
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 drivers/usb/chipidea/ci13xxx_imx.c |   38 
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/chipidea/ci13xxx_imx.c 
b/drivers/usb/chipidea/ci13xxx_imx.c
index b598bb8..6e720ce 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -30,7 +30,6 @@
((struct usb_phy *)platform_get_drvdata(pdev))
 
 struct ci13xxx_imx_data {
-   struct device_node *phy_np;
struct usb_phy *phy;
struct platform_device *ci_pdev;
struct clk *clk;
@@ -90,12 +89,12 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
 {
struct ci13xxx_imx_data *data;
struct ci13xxx_platform_data *pdata;
-   struct platform_device *plat_ci, *phy_pdev;
-   struct device_node *phy_np;
+   struct platform_device *plat_ci;
struct resource *res;
struct regulator *reg_vbus;
struct pinctrl *pinctrl;
int ret;
+   struct usb_phy *phy;
 
if (of_find_property(pdev-dev.of_node, fsl,usbmisc, NULL)
 !usbmisc_ops)
@@ -147,19 +146,20 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
return ret;
}
 
-   phy_np = of_parse_phandle(pdev-dev.of_node, fsl,usbphy, 0);
-   if (phy_np) {
-   data-phy_np = phy_np;
-   phy_pdev = of_find_device_by_node(phy_np);
-   if (phy_pdev) {
-   struct usb_phy *phy;
-   phy = pdev_to_phy(phy_pdev);
-   if (phy 
-   try_module_get(phy_pdev-dev.driver-owner)) {
-   usb_phy_init(phy);
-   data-phy = phy;
-   }
+   phy = devm_usb_get_phy_by_phandle(pdev-dev, fsl,usbphy, 0);
+   if (PTR_ERR(phy) == -EPROBE_DEFER) {
+   ret = -EPROBE_DEFER;
+   goto err_clk;
+   }
+
+   if (!IS_ERR(phy)) {
+   ret = usb_phy_init(phy);
+   if (ret) {
+   dev_err(pdev-dev, unable to init phy: %d\n, ret);
+   goto err_clk;
}
+
+   data-phy = phy;
}
 
/* we only support host now, so enable vbus here */
@@ -170,7 +170,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
dev_err(pdev-dev,
Failed to enable vbus regulator, err=%d\n,
ret);
-   goto put_np;
+   goto err_clk;
}
data-reg_vbus = reg_vbus;
} else {
@@ -222,9 +222,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
 err:
if (reg_vbus)
regulator_disable(reg_vbus);
-put_np:
-   if (phy_np)
-   of_node_put(phy_np);
+err_clk:
clk_disable_unprepare(data-clk);
return ret;
 }
@@ -244,8 +242,6 @@ static int ci13xxx_imx_remove(struct platform_device *pdev)
module_put(data-phy-dev-driver-owner);
}
 
-   of_node_put(data-phy_np);
-
clk_disable_unprepare(data-clk);
 
platform_set_drvdata(pdev, NULL);
-- 
1.7.10.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 4/5] USB chipidea i.MX: introduce dr_mode property

[Marc Kleine-Budde]

From: Sascha Hauer s.ha...@pengutronix.de

The dr_mode devicetree property allows to explicitly specify the
host/peripheral/otg mode. This is necessary for boards without proper
ID pin handling.

Reviewed-by: Peter Chen peter.c...@freescale.com
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 Documentation/devicetree/bindings/usb/ci13xxx-imx.txt |1 +
 drivers/usb/chipidea/ci13xxx_imx.c|1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt 
b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
index dd42ccd..493a414 100644
--- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
@@ -9,6 +9,7 @@ Recommended properies:
 - phy_type: the type of the phy connected to the core. Should be one
   of utmi, utmi_wide, ulpi, serial or hsic. Without this
   property the PORTSC register won't be touched
+- dr_mode: One of host, peripheral or otg. Defaults to otg
 
 Optional properties:
 - fsl,usbphy: phandler of usb phy that connects to the only one port
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c 
b/drivers/usb/chipidea/ci13xxx_imx.c
index ebc1148..b598bb8 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -114,6 +114,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
   CI13XXX_DISABLE_STREAMING;
 
pdata-phy_mode = of_usb_get_phy_mode(pdev-dev.of_node);
+   pdata-dr_mode = of_usb_get_dr_mode(pdev-dev.of_node);
 
data = devm_kzalloc(pdev-dev, sizeof(*data), GFP_KERNEL);
if (!data) {
-- 
1.7.10.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/5] USB chipidea: ci13xxx-imx: create dynamic platformdata

[Marc Kleine-Budde]

From: Michael Grzeschik m.grzesc...@pengutronix.de

This patch removes the limitation of having only one instance of the
ci13xxx-imx platformdata and makes different configurations possible.

Reviewed-by: Peter Chen peter.c...@freescale.com
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 drivers/usb/chipidea/ci13xxx_imx.c |   25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/chipidea/ci13xxx_imx.c 
b/drivers/usb/chipidea/ci13xxx_imx.c
index 8c29122..69024e0 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -85,17 +85,10 @@ EXPORT_SYMBOL_GPL(usbmisc_get_init_data);
 
 /* End of common functions shared by usbmisc drivers*/
 
-static struct ci13xxx_platform_data ci13xxx_imx_platdata  = {
-   .name   = ci13xxx_imx,
-   .flags  = CI13XXX_REQUIRE_TRANSCEIVER |
- CI13XXX_PULLUP_ON_VBUS |
- CI13XXX_DISABLE_STREAMING,
-   .capoffset  = DEF_CAPOFFSET,
-};
-
 static int ci13xxx_imx_probe(struct platform_device *pdev)
 {
struct ci13xxx_imx_data *data;
+   struct ci13xxx_platform_data *pdata;
struct platform_device *plat_ci, *phy_pdev;
struct device_node *phy_np;
struct resource *res;
@@ -107,6 +100,18 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
 !usbmisc_ops)
return -EPROBE_DEFER;
 
+   pdata = devm_kzalloc(pdev-dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata) {
+   dev_err(pdev-dev, Failed to allocate CI13xxx-IMX pdata!\n);
+   return -ENOMEM;
+   }
+
+   pdata-name = ci13xxx_imx;
+   pdata-capoffset = DEF_CAPOFFSET;
+   pdata-flags = CI13XXX_REQUIRE_TRANSCEIVER |
+  CI13XXX_PULLUP_ON_VBUS |
+  CI13XXX_DISABLE_STREAMING;
+
data = devm_kzalloc(pdev-dev, sizeof(*data), GFP_KERNEL);
if (!data) {
dev_err(pdev-dev, Failed to allocate CI13xxx-IMX data!\n);
@@ -168,7 +173,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
reg_vbus = NULL;
}
 
-   ci13xxx_imx_platdata.phy = data-phy;
+   pdata-phy = data-phy;
 
if (!pdev-dev.dma_mask) {
pdev-dev.dma_mask = devm_kzalloc(pdev-dev,
@@ -193,7 +198,7 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
 
plat_ci = ci13xxx_add_device(pdev-dev,
pdev-resource, pdev-num_resources,
-   ci13xxx_imx_platdata);
+   pdata);
if (IS_ERR(plat_ci)) {
ret = PTR_ERR(plat_ci);
dev_err(pdev-dev,
-- 
1.7.10.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 2/5] USB chipidea: add PTW and PTS handling

[Marc Kleine-Budde]

From: Michael Grzeschik m.grzesc...@pengutronix.de

This patch makes it possible to configure the PTW and PTS bits inside
the portsc register for host and device mode before the driver starts
and the phy can be addressed as hardware implementation is designed.

Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 .../devicetree/bindings/usb/ci13xxx-imx.txt|5 +++
 drivers/usb/chipidea/bits.h|   14 ++-
 drivers/usb/chipidea/ci13xxx_imx.c |3 ++
 drivers/usb/chipidea/core.c|   39 
 include/linux/usb/chipidea.h   |1 +
 5 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt 
b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
index 5778b9c..dd42ccd 100644
--- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
+++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
@@ -5,6 +5,11 @@ Required properties:
 - reg: Should contain registers location and length
 - interrupts: Should contain controller interrupt
 
+Recommended properies:
+- phy_type: the type of the phy connected to the core. Should be one
+  of utmi, utmi_wide, ulpi, serial or hsic. Without this
+  property the PORTSC register won't be touched
+
 Optional properties:
 - fsl,usbphy: phandler of usb phy that connects to the only one port
 - fsl,usbmisc: phandler of non-core register device, with one argument
diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index 050de85..d8ffc2f 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -48,10 +48,22 @@
 #define PORTSC_SUSP   BIT(7)
 #define PORTSC_HSPBIT(9)
 #define PORTSC_PTC(0x0FUL  16)
+/* PTS and PTW for non lpm version only */
+#define PORTSC_PTS(d) d)  0x3)  30) | (((d)  0x4) ? BIT(25) : 
0))
+#define PORTSC_PTWBIT(28)
 
 /* DEVLC */
 #define DEVLC_PSPD(0x03UL  25)
-#defineDEVLC_PSPD_HS  (0x02UL  25)
+#define DEVLC_PSPD_HS (0x02UL  25)
+#define DEVLC_PTW BIT(27)
+#define DEVLC_STS BIT(28)
+#define DEVLC_PTS(d)  (((d)  0x7)  29)
+
+/* Encoding for DEVLC_PTS and PORTSC_PTS */
+#define PTS_UTMI  0
+#define PTS_ULPI  2
+#define PTS_SERIAL3
+#define PTS_HSIC  4
 
 /* OTGSC */
 #define OTGSC_IDPU   BIT(5)
diff --git a/drivers/usb/chipidea/ci13xxx_imx.c 
b/drivers/usb/chipidea/ci13xxx_imx.c
index 69024e0..ebc1148 100644
--- a/drivers/usb/chipidea/ci13xxx_imx.c
+++ b/drivers/usb/chipidea/ci13xxx_imx.c
@@ -21,6 +21,7 @@
 #include linux/clk.h
 #include linux/regulator/consumer.h
 #include linux/pinctrl/consumer.h
+#include linux/usb/of.h
 
 #include ci.h
 #include ci13xxx_imx.h
@@ -112,6 +113,8 @@ static int ci13xxx_imx_probe(struct platform_device *pdev)
   CI13XXX_PULLUP_ON_VBUS |
   CI13XXX_DISABLE_STREAMING;
 
+   pdata-phy_mode = of_usb_get_phy_mode(pdev-dev.of_node);
+
data = devm_kzalloc(pdev-dev, sizeof(*data), GFP_KERNEL);
if (!data) {
dev_err(pdev-dev, Failed to allocate CI13xxx-IMX data!\n);
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 57cae1f..04d68cb 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -67,6 +67,8 @@
 #include linux/usb/gadget.h
 #include linux/usb/otg.h
 #include linux/usb/chipidea.h
+#include linux/usb/of.h
+#include linux/phy.h
 
 #include ci.h
 #include udc.h
@@ -211,6 +213,41 @@ static int hw_device_init(struct ci13xxx *ci, void __iomem 
*base)
return 0;
 }
 
+static void hw_phymode_configure(struct ci13xxx *ci)
+{
+   u32 portsc, lpm;
+
+   switch (ci-platdata-phy_mode) {
+   case USBPHY_INTERFACE_MODE_UTMI:
+   portsc = PORTSC_PTS(PTS_UTMI);
+   lpm = DEVLC_PTS(PTS_UTMI);
+   break;
+   case USBPHY_INTERFACE_MODE_UTMIW:
+   portsc = PORTSC_PTS(PTS_UTMI) | PORTSC_PTW;
+   lpm = DEVLC_PTS(PTS_UTMI) | DEVLC_PTW;
+   break;
+   case USBPHY_INTERFACE_MODE_ULPI:
+   portsc = PORTSC_PTS(PTS_ULPI);
+   lpm = DEVLC_PTS(PTS_ULPI);
+   break;
+   case USBPHY_INTERFACE_MODE_SERIAL:
+   portsc = PORTSC_PTS(PTS_SERIAL);
+   lpm = DEVLC_PTS(PTS_SERIAL);
+   break;
+   case USBPHY_INTERFACE_MODE_HSIC:
+   portsc = PORTSC_PTS(PTS_HSIC);
+   lpm = DEVLC_PTS(PTS_HSIC);
+   break;
+   default:
+   return;
+   }
+
+   if (ci-hw_bank.lpm)
+   hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
+   else
+   hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc

[PATCH 3/5] USB chipidea: introduce dual role mode pdata flags

[Marc Kleine-Budde]

From: Sascha Hauer s.ha...@pengutronix.de

Even if a chipidea core is otg capable the board may not. This allows
to explicitly set the core to host/peripheral mode. Without these
flags the driver falls back to the old behaviour.

Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
---
 drivers/usb/chipidea/core.c  |   22 --
 include/linux/usb/chipidea.h |2 +-
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 04d68cb..ec27060 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -435,6 +435,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
struct resource *res;
void __iomem*base;
int ret;
+   enum usb_dr_mode dr_mode;
 
if (!dev-platform_data) {
dev_err(dev, platform data missing\n);
@@ -487,14 +488,23 @@ static int ci_hdrc_probe(struct platform_device *pdev)
return -ENODEV;
}
 
+   /* For now we treat dual-role as otg */
+   dr_mode = ci-platdata-dr_mode;
+   if (dr_mode == USB_DR_MODE_UNKNOWN || dr_mode == USB_DR_MODE_DUAL_ROLE)
+   dr_mode = USB_DR_MODE_OTG;
+
/* initialize role(s) before the interrupt is requested */
-   ret = ci_hdrc_host_init(ci);
-   if (ret)
-   dev_info(dev, doesn't support host\n);
+   if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
+   ret = ci_hdrc_host_init(ci);
+   if (ret)
+   dev_info(dev, doesn't support host\n);
+   }
 
-   ret = ci_hdrc_gadget_init(ci);
-   if (ret)
-   dev_info(dev, doesn't support gadget\n);
+   if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
+   ret = ci_hdrc_gadget_init(ci);
+   if (ret)
+   dev_info(dev, doesn't support gadget\n);
+   }
 
if (!ci-roles[CI_ROLE_HOST]  !ci-roles[CI_ROLE_GADGET]) {
dev_err(dev, no supported roles\n);
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index 1a2aa18..b314647 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -20,7 +20,7 @@ struct ci13xxx_platform_data {
 #define CI13XXX_REQUIRE_TRANSCEIVERBIT(1)
 #define CI13XXX_PULLUP_ON_VBUS BIT(2)
 #define CI13XXX_DISABLE_STREAMING  BIT(3)
-
+   enum usb_dr_modedr_mode;
 #define CI13XXX_CONTROLLER_RESET_EVENT 0
 #define CI13XXX_CONTROLLER_STOPPED_EVENT   1
void(*notify_event) (struct ci13xxx *ci, unsigned event);
-- 
1.7.10.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: About chipidea tree

[Marc Kleine-Budde]

On 02/28/2013 12:16 PM, Alexander Shishkin wrote:
 Peter Chen peter.c...@freescale.com writes:
 
 On Tue, Feb 26, 2013 at 02:47:34PM +0100, Marc Kleine-Budde wrote:
 On 02/26/2013 02:25 PM, Alexander Shishkin wrote:
 Marc Kleine-Budde m...@pengutronix.de writes:

 On 02/26/2013 11:56 AM, Alexander Shishkin wrote:
 Peter Chen peter.c...@freescale.com writes:

 Hi Alex,

 Hi,

 Do we have a chipidea repo which is queued for mainline?
 We have several patchsets for chipidea these monthes, I
 don't know their status. For me, I would like based
 on your tree if it exists.

 I thought about it, but then it seems like having a separate branch is
 bound to be confusing to most people. I'd much rather prefer everything
 go to usb-next, and this is my current plan. Since Greg will start
 applying new stuff to usb-next after -rc1 is tagged, I'll send my
 current stash of patches for inclusion then. If your patchset, for

 Can we have a look at your queued patches?

 Sure,
 http://marc.info/?l=linux-usbm=135902434508839

 example, has conflicts with my stuff that's not merged, I'll try to take
 care of resolving the conflicts and submit everything to Greg. In other
 words, it should be always ok to base your chipidea patchsets on
 usb-next.

 Let me know if this sounds reasonable to you.

 Michael has already done that work (some S-o-b form Michael are missing)
 and rebased Sascha's and Peter's patches to current linus/master, see
 this tree :

 http://git.pengutronix.de/?p=mgr/linux.git;a=shortlog;h=refs/heads/chipidea-for-v3.10

 Taking a quick look, quite some of those patches are not ready for
 inclusion yet. So if the question is, do we need a -next tree for all
 the chipidea patchsets floating around, it might be a good thing. But
 it's not what Peter was asking in the first place.

 I suggest that we have a branch that holds all chipidea patches that are
 ready for mainline. Otherwise it's really hard to code any new features

 I agree.

 Alex, you can have a repo at github or any other places which is based
 on usb-next, and add it to MAINTAINERS. We can develop the new feature
 based on your repo. Greg can pull it directly if he agrees or you can send
 your queued patchset before every merge windows.
 
 Ok, let's try this. I have a linux-ci repo on github, might as well do
 something useful with it [1], [2]. Currently, the branch called
 ci-for-greg is where I stack patches that I'm planning to be sending
 (via email) to Greg when the time is right. The policy is such that
 it'll be rebased on top of Greg's usb-next and probably often, so no
 fast forwards. Also patches may be dropped from it if necessary. Since
 the branch is not for pulling, the no rebase rule doesn't apply.
 
 If you have comments/suggestions/etc for a patch that is in this branch,
 please reply to the email with that patch on the mailing list and not
 via github infrastructure.
 
 Sounds reasonable?

go ahead.

Marc
-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/2] USB mxs-phy: Register phy with framework

[Marc Kleine-Budde]

On 02/27/2013 09:10 AM, Felipe Balbi wrote:
 On Thu, Feb 14, 2013 at 07:43:54PM +0100, Sascha Hauer wrote:
 On Thu, Feb 14, 2013 at 08:00:11PM +0200, Felipe Balbi wrote:
 Hi,

 On Thu, Feb 14, 2013 at 05:23:37PM +0100, Sascha Hauer wrote:
 On Thu, Feb 14, 2013 at 12:37:29PM +0200, Felipe Balbi wrote:
 Hi,

 On Thu, Jan 31, 2013 at 12:32:16PM +0100, Sascha Hauer wrote:
 We now have usb_add_phy_dev(), so use it to register with the framework
 to be able to find the phy from the USB driver.

 Signed-off-by: Sascha Hauer s.ha...@pengutronix.de

 Sascha, are you taking this through your tree or you want me to queue it
 up ?

 I would prefer if either you could take it or Alexander Shishkin along
 with the other patches in:

 [PATCH v4] USB: add devicetree helpers for determining dr_mode and phy_type

 Could you have a look at them? Alexander is asking for an ack for 1/9,
 2/9, 3/9, 8/9. I just realized you are not on Cc for these. I could
 bounce you the patches in case you don't have them in your mailbox.

 Ok, I'll look at them, no need to bounce. You _do_ realize, however,
 that Greg has already closed all his trees for v3.9, right ? So anything
 from now on will be queued for v3.10.

 I'm fine with that. I'm just a bit insistent because the chipidea patches are
 floating around for half a year now. Once I have the warm feeling that
 the patches are handled I have time ;)
 
 I'm getting ready to queue patches but, when looking at this thread, it
 seems like another version should've been sent. I'll skip this and look
 at the other chipidea threads,

I'm currently rebasing Saschas patches to Greg's usb-net.

Marc

---
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 5/9] USB: chipidea: add PTW and PTS handling

[Marc Kleine-Budde]

On 02/14/2013 02:07 PM, Alexander Shishkin wrote:
 Sascha Hauer s.ha...@pengutronix.de writes:
 
 From: Michael Grzeschik m.grzesc...@pengutronix.de

 This patch makes it possible to configure the PTW and PTS bits inside
 the portsc register for host and device mode before the driver starts
 and the phy can be addressed as hardware implementation is designed.

 Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
 Signed-off-by: Marc Kleine-Budde m...@pengutronix.de
 Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
 ---
  .../devicetree/bindings/usb/ci13xxx-imx.txt|5 +++
  drivers/usb/chipidea/bits.h|   14 ++-
  drivers/usb/chipidea/ci13xxx_imx.c |3 ++
  drivers/usb/chipidea/core.c|   39 
 
  include/linux/usb/chipidea.h   |1 +
  5 files changed, 61 insertions(+), 1 deletion(-)

 diff --git a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt 
 b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
 index 5778b9c..dd42ccd 100644
 --- a/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
 +++ b/Documentation/devicetree/bindings/usb/ci13xxx-imx.txt
 @@ -5,6 +5,11 @@ Required properties:
  - reg: Should contain registers location and length
  - interrupts: Should contain controller interrupt
  
 +Recommended properies:
 +- phy_type: the type of the phy connected to the core. Should be one
 +  of utmi, utmi_wide, ulpi, serial or hsic. Without this
 +  property the PORTSC register won't be touched
 +
 
 Looks like this bit belongs to patch 3/9, where you're adding devicetree
 hooks. Otherwise looks good.

Nope. Patch 3/9 adds the device tree helper code. But this patch adds
the functionality to the chipidea imx glue code, so the update of the
devicetree docs belongs into this patch.

Marc

-- 
Pengutronix e.K.  | Marc Kleine-Budde   |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |



signature.asc
Description: OpenPGP digital signature