| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 10 Jul 2013 15:55:52 +0900
From: Chanwoo Choi <cw00.choi@...sung.com>
To: Laxman Dewangan <ldewangan@...dia.com>
Cc: myungjoo.ham@...sung.com, devicetree-discuss@...ts.ozlabs.org,
rob@...dley.net, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, kishon@...com, gg@...mlogic.co.uk
Subject: Re: [PATCH V2 4/4] extcon: palmas: Option to disable ID/VBUS detection
based on platform
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 <ldewangan@...dia.com>
> Acked-by: Graeme Gregory <gg@...mlogic.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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists