Hi Laxman, 

On 07/10/2013 03:15 PM, Laxman Dewangan wrote:
> Based on system design, platform needs to detect the VBUS or ID or
> both. Provide option to select this through platform data to
> disable part of cable detection through palmas-usb.
> 
> Signed-off-by: Laxman Dewangan <ldewan...@nvidia.com>
> Acked-by: Graeme Gregory <g...@slimlogic.co.uk>
> ---
> No changes from V1.
> 
>  .../devicetree/bindings/extcon/extcon-palmas.txt   |    2 +
>  drivers/extcon/extcon-palmas.c                     |   65 
> +++++++++++++-------
>  include/linux/mfd/palmas.h                         |    4 +
>  3 files changed, 48 insertions(+), 23 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/extcon/extcon-palmas.txt 
> b/Documentation/devicetree/bindings/extcon/extcon-palmas.txt
> index f110e75..8e593ec 100644
> --- a/Documentation/devicetree/bindings/extcon/extcon-palmas.txt
> +++ b/Documentation/devicetree/bindings/extcon/extcon-palmas.txt
> @@ -6,6 +6,8 @@ Required Properties:
>  
>  Optional Properties:
>   - ti,wakeup : To enable the wakeup comparator in probe
> + - ti,disable-id-detection: Do not perform ID detection.
> + - ti,disable-vbus-detection: Do not pwerform VBUS detection.

Fix typo.
- pwerform -> perform

>  
>  palmas-usb {
>         compatible = "ti,twl6035-usb", "ti,palmas-usb";
> diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
> index dc7ebf3..4747d11 100644
> --- a/drivers/extcon/extcon-palmas.c
> +++ b/drivers/extcon/extcon-palmas.c
> @@ -122,11 +122,14 @@ static void palmas_enable_irq(struct palmas_usb 
> *palmas_usb)
>               PALMAS_USB_ID_INT_EN_HI_SET_ID_GND |
>               PALMAS_USB_ID_INT_EN_HI_SET_ID_FLOAT);
>  
> -     palmas_vbus_irq_handler(palmas_usb->vbus_irq, palmas_usb);
> +     if (palmas_usb->enable_vbus_detection)
> +             palmas_vbus_irq_handler(palmas_usb->vbus_irq, palmas_usb);
>  
>       /* cold plug for host mode needs this delay */
> -     msleep(30);
> -     palmas_id_irq_handler(palmas_usb->id_irq, palmas_usb);
> +     if (palmas_usb->enable_id_detection) {
> +             msleep(30);
> +             palmas_id_irq_handler(palmas_usb->id_irq, palmas_usb);
> +     }
>  }
>  
>  static int palmas_usb_probe(struct platform_device *pdev)
> @@ -144,6 +147,10 @@ static int palmas_usb_probe(struct platform_device *pdev)
>                       return -ENOMEM;
>  
>               pdata->wakeup = of_property_read_bool(node, "ti,wakeup");
> +             pdata->disable_id_detection = of_property_read_bool(node,
> +                                             "ti,disable-id-detection");
> +             pdata->disable_vbus_detection = of_property_read_bool(node,
> +                                             "ti,disable-vbus-detection");
>       } else if (!pdata) {
>               return -EINVAL;
>       }
> @@ -154,6 +161,8 @@ static int palmas_usb_probe(struct platform_device *pdev)
>  
>       palmas->usb = palmas_usb;
>       palmas_usb->palmas = palmas;
> +     palmas_usb->enable_vbus_detection = !pdata->disable_vbus_detection;
> +     palmas_usb->enable_id_detection =  !pdata->disable_id_detection;
>  
>       palmas_usb->dev  = &pdev->dev;
>  
> @@ -180,26 +189,32 @@ static int palmas_usb_probe(struct platform_device 
> *pdev)
>               return status;
>       }
>  
> -     status = devm_request_threaded_irq(palmas_usb->dev, palmas_usb->id_irq,
> -                     NULL, palmas_id_irq_handler,
> -                     IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING |
> -                     IRQF_ONESHOT | IRQF_EARLY_RESUME,
> -                     "palmas_usb_id", palmas_usb);
> -     if (status < 0) {
> -             dev_err(&pdev->dev, "can't get IRQ %d, err %d\n",
> +     if (palmas_usb->enable_id_detection) {
> +             status = devm_request_threaded_irq(palmas_usb->dev,
> +                             palmas_usb->id_irq,
> +                             NULL, palmas_id_irq_handler,
> +                             IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING |
> +                             IRQF_ONESHOT | IRQF_EARLY_RESUME,
> +                             "palmas_usb_id", palmas_usb);
> +             if (status < 0) {
> +                     dev_err(&pdev->dev, "can't get IRQ %d, err %d\n",
>                                       palmas_usb->id_irq, status);
> -             goto fail_extcon;
> +                     goto fail_extcon;
> +             }
>       }
>  
> -     status = devm_request_threaded_irq(palmas_usb->dev,
> -                     palmas_usb->vbus_irq, NULL, palmas_vbus_irq_handler,
> -                     IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING |
> -                     IRQF_ONESHOT | IRQF_EARLY_RESUME,
> -                     "palmas_usb_vbus", palmas_usb);
> -     if (status < 0) {
> -             dev_err(&pdev->dev, "can't get IRQ %d, err %d\n",
> +     if (palmas_usb->enable_vbus_detection) {
> +             status = devm_request_threaded_irq(palmas_usb->dev,
> +                             palmas_usb->vbus_irq, NULL,
> +                             palmas_vbus_irq_handler,
> +                             IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING |
> +                             IRQF_ONESHOT | IRQF_EARLY_RESUME,
> +                             "palmas_usb_vbus", palmas_usb);
> +             if (status < 0) {
> +                     dev_err(&pdev->dev, "can't get IRQ %d, err %d\n",
>                                       palmas_usb->vbus_irq, status);
> -             goto fail_extcon;
> +                     goto fail_extcon;
> +             }
>       }
>  
>       palmas_enable_irq(palmas_usb);
> @@ -227,8 +242,10 @@ static int palmas_usb_suspend(struct device *dev)
>       struct palmas_usb *palmas_usb = dev_get_drvdata(dev);
>  
>       if (device_may_wakeup(dev)) {
> -             enable_irq_wake(palmas_usb->vbus_irq);
> -             enable_irq_wake(palmas_usb->id_irq);
> +             if (palmas_usb->enable_vbus_detection)
> +                     enable_irq_wake(palmas_usb->vbus_irq);
> +             if (palmas_usb->enable_id_detection)
> +                     enable_irq_wake(palmas_usb->id_irq);
>       }
>       return 0;
>  }
> @@ -238,8 +255,10 @@ static int palmas_usb_resume(struct device *dev)
>       struct palmas_usb *palmas_usb = dev_get_drvdata(dev);
>  
>       if (device_may_wakeup(dev)) {
> -             disable_irq_wake(palmas_usb->vbus_irq);
> -             disable_irq_wake(palmas_usb->id_irq);
> +             if (palmas_usb->enable_vbus_detection)
> +                     disable_irq_wake(palmas_usb->vbus_irq);
> +             if (palmas_usb->enable_id_detection)
> +                     disable_irq_wake(palmas_usb->id_irq);
>       }
>       return 0;
>  };
> diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h
> index 03c22ca..d9ef94c 100644
> --- a/include/linux/mfd/palmas.h
> +++ b/include/linux/mfd/palmas.h
> @@ -204,6 +204,8 @@ struct palmas_pmic_platform_data {
>  struct palmas_usb_platform_data {
>       /* Do we enable the wakeup comparator on probe */
>       int wakeup;
> +     bool disable_vbus_detection;
> +     bool disable_id_detection;
>  };
>  
>  struct palmas_resource_platform_data {
> @@ -377,6 +379,8 @@ struct palmas_usb {
>       int vbus_irq;
>  
>       enum palmas_usb_state linkstat;
> +     bool enable_vbus_detection;
> +     bool enable_id_detection;
>  };
>  
>  #define comparator_to_palmas(x) container_of((x), struct palmas_usb, 
> comparator)
> 

Should you define duplicate meaning variables in each other structure?
- disable_vbus_detection - enable_vbus_detection
- disable_id_detection - enable_id_detection

I think that it isn' efficient code. I'd like you to simplify this patch
by using only one variable instead of duplicate meaning variables.

Cheers,
Chanwoo Choi




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to