* David Brownell <[EMAIL PROTECTED]> [081016 12:34]:
> From: David Brownell <[EMAIL PROTECTED]>
> 
> Simplify handling of VBUS and ID status and the USB_PRES interrupt.
> This seems to resolve the problems I previously saw on Beagle and
> Overo boards.
> 
>  - Read the status directly, instead of trying to infer it from
>    dodgey side effects of IRQ handling and trigger tweaking.
> 
>  - Stop trying to arrange those dodgey side effects; just leave
>    the IRQ enabled at all times.
> 
>  - Check that status as part of driver probe, so the PHY can
>    always be active by the time it's needed.  (BUGFIX!)
> 
>  - Re-sequence probe() to be less racey, and to avoid duplicating
>    logic found elsewhere (notably irq handling and PHY activation).
> 
>  - Make the irq_chip be the exclusive owner of PWR_EDR (finally).
> 
>  - Get rid of needless work_struct ... in this irq handling thread,
>    it's safe and cheap to call sysfs_notify() directly.
> 
> Stop enabling IRQs on the USB subchip.  There's no IRQ handler for
> them, and since ALT_INT_REROUTE isn't being set they'll never
> arrive ... so there's no point to enabling them.
> 
> Also add some comments about other things such an otg_transciever
> driver should be doing, fixing minor omissions; and remove some
> unnecessary file inclusions.

Pushing today, I guess that's it for the USB problems for now.

Tony

> 
> Signed-off-by: David Brownell <[EMAIL PROTECTED]>
> ---
> NOTE:  applies on top of the previous patches updating IRQ
> handling and creating drivers/mfd/twl4030-irq.c ...
> 
>  drivers/i2c/chips/twl4030-usb.c |  209 ++++++++++++++++++--------------------
>  1 file changed, 104 insertions(+), 105 deletions(-)
> 
> --- a/drivers/i2c/chips/twl4030-usb.c
> +++ b/drivers/i2c/chips/twl4030-usb.c
> @@ -26,16 +26,12 @@
>  
>  #include <linux/module.h>
>  #include <linux/init.h>
> -#include <linux/time.h>
>  #include <linux/interrupt.h>
> -#include <linux/irq.h>
>  #include <linux/platform_device.h>
>  #include <linux/spinlock.h>
>  #include <linux/workqueue.h>
>  #include <linux/io.h>
> -#include <linux/usb.h>
> -#include <linux/usb/ch9.h>
> -#include <linux/usb/gadget.h>
> +#include <linux/delay.h>
>  #include <linux/usb/otg.h>
>  #include <linux/i2c/twl4030.h>
>  #include <mach/usb.h>
> @@ -237,10 +233,6 @@
>  #define PMBR1                                0x0D
>  #define GPIO_USB_4PIN_ULPI_2430C     (3 << 0)
>  
> -/* In module TWL4030_MODULE_INT */
> -#define REG_PWR_EDR1                 0x05
> -#define USB_PRES_FALLING             (1 << 4)
> -#define USB_PRES_RISING                      (1 << 5)
>  
>  /* bits in OTG_CTRL */
>  #define      OTG_XCEIV_OUTPUTS \
> @@ -256,8 +248,14 @@
>       OTG_CTRL_BITS)
>  
>  
> +enum linkstat {
> +     USB_LINK_UNKNOWN = 0,
> +     USB_LINK_NONE,
> +     USB_LINK_VBUS,
> +     USB_LINK_ID,
> +};
> +
>  struct twl4030_usb {
> -     struct work_struct      irq_work;
>       struct otg_transceiver  otg;
>       struct device           *dev;
>  
> @@ -267,8 +265,8 @@ struct twl4030_usb {
>       /* pin configuration */
>       enum twl4030_usb_mode   usb_mode;
>  
> -     unsigned                vbus:1;
>       int                     irq;
> +     u8                      linkstat;
>       u8                      asleep;
>       bool                    irq_enabled;
>  };
> @@ -326,21 +324,27 @@ static inline int twl4030_usb_write(stru
>       return ret;
>  }
>  
> -static inline int twl4030_usb_read(struct twl4030_usb *twl, u8 address)
> +static inline int twl4030_readb(struct twl4030_usb *twl, u8 module, u8 
> address)
>  {
>       u8 data;
>       int ret = 0;
>  
> -     ret = twl4030_i2c_read_u8(TWL4030_MODULE_USB, &data, address);
> +     ret = twl4030_i2c_read_u8(module, &data, address);
>       if (ret >= 0)
>               ret = data;
>       else
>               dev_warn(twl->dev,
> -                     "TWL4030:USB:Read[0x%x] Error %d\n", address, ret);
> +                     "TWL4030:readb[0x%x,0x%x] Error %d\n",
> +                                     module, address, ret);
>  
>       return ret;
>  }
>  
> +static inline int twl4030_usb_read(struct twl4030_usb *twl, u8 address)
> +{
> +     return twl4030_readb(twl, TWL4030_MODULE_USB, address);
> +}
> +
>  /*-------------------------------------------------------------------------*/
>  
>  static inline int
> @@ -357,6 +361,39 @@ twl4030_usb_clear_bits(struct twl4030_us
>  
>  /*-------------------------------------------------------------------------*/
>  
> +static enum linkstat twl4030_usb_linkstat(struct twl4030_usb *twl)
> +{
> +     int     status;
> +     int     linkstat = USB_LINK_UNKNOWN;
> +
> +     /* STS_HW_CONDITIONS */
> +     status = twl4030_readb(twl, TWL4030_MODULE_PM_MASTER, 0x0f);
> +     if (status < 0)
> +             dev_err(twl->dev, "USB link status err %d\n", status);
> +     else if (status & BIT(7))
> +             linkstat = USB_LINK_VBUS;
> +     else if (status & BIT(2))
> +             linkstat = USB_LINK_ID;
> +     else
> +             linkstat = USB_LINK_NONE;
> +
> +     dev_dbg(twl->dev, "HW_CONDITIONS 0x%02x/%d; link %d\n",
> +                     status, status, linkstat);
> +
> +     spin_lock_irq(&twl->lock);
> +     twl->linkstat = linkstat;
> +     if (linkstat == USB_LINK_ID) {
> +             twl->otg.default_a = true;
> +             twl->otg.state = OTG_STATE_A_IDLE;
> +     } else {
> +             twl->otg.default_a = false;
> +             twl->otg.state = OTG_STATE_B_IDLE;
> +     }
> +     spin_unlock_irq(&twl->lock);
> +
> +     return linkstat;
> +}
> +
>  static void twl4030_usb_set_mode(struct twl4030_usb *twl, int mode)
>  {
>       twl->usb_mode = mode;
> @@ -410,24 +447,6 @@ static void twl4030_i2c_access(struct tw
>       }
>  }
>  
> -static void usb_irq_enable(struct twl4030_usb *twl, int trigger)
> -{
> -     set_irq_type(twl->irq, trigger);
> -
> -     if (!twl->irq_enabled) {
> -             enable_irq(twl->irq);
> -             twl->irq_enabled = true;
> -     }
> -}
> -
> -static void usb_irq_disable(struct twl4030_usb *twl)
> -{
> -     if (twl->irq_enabled) {
> -             disable_irq(twl->irq);
> -             twl->irq_enabled = false;
> -     }
> -}
> -
>  static void twl4030_phy_power(struct twl4030_usb *twl, int on)
>  {
>       u8 pwr;
> @@ -448,16 +467,9 @@ static void twl4030_phy_power(struct twl
>  
>  static void twl4030_phy_suspend(struct twl4030_usb *twl, int controller_off)
>  {
> -     if (controller_off)
> -             usb_irq_disable(twl);
> -
>       if (twl->asleep)
>               return;
>  
> -     if (!controller_off)
> -             /* enable rising edge interrupt to detect cable attach */
> -             usb_irq_enable(twl, IRQ_TYPE_EDGE_RISING);
> -
>       twl4030_phy_power(twl, 0);
>       twl->asleep = 1;
>  }
> @@ -467,9 +479,6 @@ static void twl4030_phy_resume(struct tw
>       if (!twl->asleep)
>               return;
>  
> -     /* enable falling edge interrupt to detect cable detach */
> -     usb_irq_enable(twl, IRQ_TYPE_EDGE_FALLING);
> -
>       twl4030_phy_power(twl, 1);
>       twl4030_i2c_access(twl, 1);
>       twl4030_usb_set_mode(twl, twl->usb_mode);
> @@ -514,25 +523,18 @@ static ssize_t twl4030_usb_vbus_show(str
>       int ret = -EINVAL;
>  
>       spin_lock_irqsave(&twl->lock, flags);
> -     ret = sprintf(buf, "%s\n", twl->vbus ? "on" : "off");
> +     ret = sprintf(buf, "%s\n",
> +                     (twl->linkstat == USB_LINK_VBUS) ? "on" : "off");
>       spin_unlock_irqrestore(&twl->lock, flags);
>  
>       return ret;
>  }
>  static DEVICE_ATTR(vbus, 0444, twl4030_usb_vbus_show, NULL);
>  
> -static void twl4030_usb_irq_work(struct work_struct *work)
> -{
> -     struct twl4030_usb *twl = container_of(work,
> -                     struct twl4030_usb, irq_work);
> -
> -     sysfs_notify(&twl->dev->kobj, NULL, "vbus");
> -}
> -
>  static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
>  {
>       struct twl4030_usb *twl = _twl;
> -     u8 val;
> +     int status;
>  
>  #ifdef CONFIG_LOCKDEP
>       /* WORKAROUND for lockdep forcing IRQF_DISABLED on us, which
> @@ -542,27 +544,28 @@ static irqreturn_t twl4030_usb_irq(int i
>       local_irq_enable();
>  #endif
>  
> -     /* FIXME stop accessing PWR_EDR1 ... if nothing else, we
> -      * know which edges we told the IRQ to trigger on.  And
> -      * there seem to be OTG_specific registers and irqs that
> -      * provide the right info without guessing like this:
> -      * USB_INT_STS, ID_STATUS, STS_HW_CONDITIONS, etc.
> -      */
> +     status = twl4030_usb_linkstat(twl);
> +     if (status != USB_LINK_UNKNOWN) {
>  
> -     /* action based on cable attach or detach */
> -     WARN_ON(twl4030_i2c_read_u8(TWL4030_MODULE_INT,
> -                             &val, REG_PWR_EDR1) < 0);
> +             /* FIXME add a set_power() method so that B-devices can
> +              * configure the charger appropriately.  It's not always
> +              * correct to consume VBUS power, and how much current to
> +              * consume is a function of the USB configuration chosen
> +              * by the host.
> +              *
> +              * REVISIT usb_gadget_vbus_connect(...) as needed, ditto
> +              * its disconnect() sibling, when changing to/from the
> +              * USB_LINK_VBUS state.  musb_hdrc won't care until it
> +              * starts to handle softconnect right.
> +              */
> +             twl4030charger_usb_en(status == USB_LINK_VBUS);
>  
> -     if (val & USB_PRES_RISING) {
> -             twl4030_phy_resume(twl);
> -             twl4030charger_usb_en(1);
> -             twl->vbus = 1;
> -     } else {
> -             twl4030charger_usb_en(0);
> -             twl->vbus = 0;
> -             twl4030_phy_suspend(twl, 0);
> +             if (status == USB_LINK_NONE)
> +                     twl4030_phy_suspend(twl, 0);
> +             else
> +                     twl4030_phy_resume(twl);
>       }
> -     schedule_work(&twl->irq_work);
> +     sysfs_notify(&twl->dev->kobj, NULL, "vbus");
>  
>       return IRQ_HANDLED;
>  }
> @@ -608,11 +611,6 @@ static int twl4030_set_peripheral(struct
>  
>       twl->otg.state = OTG_STATE_B_IDLE;
>  
> -     twl4030_usb_set_bits(twl, USB_INT_EN_RISE,
> -                     USB_INT_SESSVALID | USB_INT_VBUSVALID);
> -     twl4030_usb_set_bits(twl, USB_INT_EN_FALL,
> -                     USB_INT_SESSVALID | USB_INT_VBUSVALID);
> -
>       return 0;
>  }
>  
> @@ -639,8 +637,7 @@ static int twl4030_set_host(struct otg_t
>       twl4030_usb_set_bits(twl, TWL4030_OTG_CTRL,
>                       TWL4030_OTG_CTRL_DMPULLDOWN
>                               | TWL4030_OTG_CTRL_DPPULLDOWN);
> -     twl4030_usb_set_bits(twl, USB_INT_EN_RISE, USB_INT_IDGND);
> -     twl4030_usb_set_bits(twl, USB_INT_EN_FALL, USB_INT_IDGND);
> +
>       twl4030_usb_set_bits(twl, FUNC_CTRL, FUNC_CTRL_SUSPENDM);
>       twl4030_usb_set_bits(twl, TWL4030_OTG_CTRL, TWL4030_OTG_CTRL_DRVVBUS);
>  
> @@ -651,8 +648,7 @@ static int __init twl4030_usb_probe(stru
>  {
>       struct twl4030_usb_data *pdata = pdev->dev.platform_data;
>       struct twl4030_usb      *twl;
> -     int status;
> -     u8                      vbus;
> +     int                     status;
>  
>       twl = kzalloc(sizeof *twl, GFP_KERNEL);
>       if (!twl)
> @@ -663,26 +659,38 @@ static int __init twl4030_usb_probe(stru
>               return -EINVAL;
>       }
>  
> -     WARN_ON(twl4030_i2c_read_u8(TWL4030_MODULE_INT,
> -                             &vbus, REG_PWR_EDR1) < 0);
> -     vbus &= USB_PRES_RISING;
> -
>       twl->dev                = &pdev->dev;
>       twl->irq                = platform_get_irq(pdev, 0);
> +     twl->otg.dev            = twl->dev;
> +     twl->otg.label          = "twl4030";
>       twl->otg.set_host       = twl4030_set_host;
>       twl->otg.set_peripheral = twl4030_set_peripheral;
>       twl->otg.set_suspend    = twl4030_set_suspend;
>       twl->usb_mode           = pdata->usb_mode;
> -     twl->vbus               = vbus ? 1 : 0;
> +     twl->asleep             = 1;
>  
>       /* init spinlock for workqueue */
>       spin_lock_init(&twl->lock);
>  
> -     /* init irq workqueue before request_irq */
> -     INIT_WORK(&twl->irq_work, twl4030_usb_irq_work);
> +     twl4030_usb_ldo_init(twl);
> +     otg_set_transceiver(&twl->otg);
> +
> +     platform_set_drvdata(pdev, twl);
> +     if (device_create_file(&pdev->dev, &dev_attr_vbus))
> +             dev_warn(&pdev->dev, "could not create sysfs file\n");
>  
> +     /* Our job is to use irqs and status from the power module
> +      * to keep the transceiver disabled when nothing's connected.
> +      *
> +      * FIXME we actually shouldn't start enabling it until the
> +      * USB controller drivers have said they're ready, by calling
> +      * set_host() and/or set_peripheral() ... OTG_capable boards
> +      * need both handles, otherwise just one suffices.
> +      */
>       twl->irq_enabled = true;
> -     status = request_irq(twl->irq, twl4030_usb_irq, 0, "twl4030_usb", twl);
> +     status = request_irq(twl->irq, twl4030_usb_irq,
> +                     IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
> +                     "twl4030_usb", twl);
>       if (status < 0) {
>               dev_dbg(&pdev->dev, "can't get IRQ %d, err %d\n",
>                       twl->irq, status);
> @@ -690,25 +698,17 @@ static int __init twl4030_usb_probe(stru
>               return status;
>       }
>  
> -     twl4030_usb_ldo_init(twl);
> -     twl4030_phy_power(twl, 1);
> -     twl4030_i2c_access(twl, 1);
> -     twl4030_usb_set_mode(twl, twl->usb_mode);
> -
> -     twl->asleep = 0;
> -
> -     if (twl->usb_mode == T2_USB_MODE_ULPI) {
> -             twl4030_i2c_access(twl, 0);
> -             twl4030_phy_suspend(twl, 0);
> -     }
> +     /* The IRQ handler just handles changes from the previous states
> +      * of the ID and VBUS pins ... in probe() we must initialize that
> +      * previous state.  The easy way:  fake an IRQ.
> +      *
> +      * REVISIT:  a real IRQ might have happened already, if PREEMPT is
> +      * enabled.  Else the IRQ may not yet be configured or enabled,
> +      * because of scheduling delays.
> +      */
> +     twl4030_usb_irq(twl->irq, twl);
>  
> -     otg_set_transceiver(&twl->otg);
> -     platform_set_drvdata(pdev, twl);
>       dev_info(&pdev->dev, "Initialized TWL4030 USB module\n");
> -
> -     if (device_create_file(&pdev->dev, &dev_attr_vbus))
> -             dev_warn(&pdev->dev, "could not create sysfs file\n");
> -
>       return 0;
>  }
>  
> @@ -717,7 +717,6 @@ static int __exit twl4030_usb_remove(str
>       struct twl4030_usb *twl = platform_get_drvdata(pdev);
>       int val;
>  
> -     usb_irq_disable(twl);
>       free_irq(twl->irq, twl);
>       device_remove_file(twl->dev, &dev_attr_vbus);
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to