From: Laxman Dewangan <ldewangan@nvidia.com>
To: Kishon Vijay Abraham I <kishon@ti.com>
Cc: "myungjoo.ham@samsung.com" <myungjoo.ham@samsung.com>,
"cw00.choi@samsung.com" <cw00.choi@samsung.com>,
"balbi@ti.com" <balbi@ti.com>,
"devicetree-discuss@lists.ozlabs.org"
<devicetree-discuss@lists.ozlabs.org>,
"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"grant.likely@linaro.org" <grant.likely@linaro.org>,
"rob.herring@calxeda.com" <rob.herring@calxeda.com>,
"rob@landley.net" <rob@landley.net>,
"gg@slimlogic.co.uk" <gg@slimlogic.co.uk>,
"ruchika@ti.com" <ruchika@ti.com>,
"tony@atomide.com" <tony@atomide.com>,
"sameo@linux.intel.com" <sameo@linux.intel.com>,
"broonie@opensource.wolfsonmicro.com"
<broonie@opensource.wolfsonmicro.com>
Subject: Re: [PATCH v4] extcon: Palmas Extcon Driver
Date: Mon, 6 May 2013 19:56:55 +0530 [thread overview]
Message-ID: <5187BDAF.2040303@nvidia.com> (raw)
In-Reply-To: <1367846225-15685-1-git-send-email-kishon@ti.com>
On Monday 06 May 2013 06:47 PM, Kishon Vijay Abraham I wrote:
> +
> +static irqreturn_t palmas_vbus_wakeup_irq(int irq, void *_palmas_usb)
Can we name the function to palams_vbus_irq_handler() for better
understanding? Reserve the wakeup word for the suspend-wakeups.
> +
> +
> +static irqreturn_t palmas_id_wakeup_irq(int irq, void *_palmas_usb)
Same here for better name.
> +
> +static void palmas_set_vbus_work(struct work_struct *data)
> +{
> + int ret;
> + struct palmas_usb *palmas_usb = container_of(data, struct palmas_usb,
> + set_vbus_work);
> +
> + if (IS_ERR_OR_NULL(palmas_usb->vbus_reg)) {
> + dev_err(palmas_usb->dev, "invalid regulator\n");
> + return;
> + }
This error will keep coming if the vbus is not require as workqueue get
scheduled always. I think we should remove it.
> +
> +static void palmas_dt_to_pdata(struct device_node *node,
> + struct palmas_usb_platform_data *pdata)
> +{
> + pdata->no_control_vbus = of_property_read_bool(node,
> + "ti,no_control_vbus");
Can we change the variable names to enable_control_bus and logic
accordingly as it looks more appropriate and easy to understand?
> +
> + palmas_usb->irq1 = regmap_irq_get_virq(palmas->irq_data,
> + PALMAS_ID_OTG_IRQ);
> + palmas_usb->irq2 = regmap_irq_get_virq(palmas->irq_data,
> + PALMAS_ID_IRQ);
> + palmas_usb->irq3 = regmap_irq_get_virq(palmas->irq_data,
> + PALMAS_VBUS_OTG_IRQ);
> + palmas_usb->irq4 = regmap_irq_get_virq(palmas->irq_data,
> + PALMAS_VBUS_IRQ);
> +
Better to name irq1, irq2 in more logical names for easy understanding.
> +
> + if (device_create_file(&pdev->dev, &dev_attr_vbus))
> + dev_warn(&pdev->dev, "could not create sysfs file\n");
> +
> + palmas_usb->edev.name = "palmas_usb";
> + palmas_usb->edev.supported_cable = palmas_extcon_cable;
> + palmas_usb->edev.mutually_exclusive = mutually_exclusive;
> +
> + ret = extcon_dev_register(&palmas_usb->edev, palmas_usb->dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register extcon device\n");
> + return ret;
It need to destroy sysfs also.
> + }
> +
> + /* init spinlock for workqueue */
> + spin_lock_init(&palmas_usb->lock);
It is already done above.
> +
> + INIT_WORK(&palmas_usb->set_vbus_work, palmas_set_vbus_work);
Better to create the workqueu when control_vbus is require.
> +
> +
> diff --git a/include/linux/extcon/extcon_palmas.h b/include/linux/extcon/extcon_palmas.h
> new file mode 100644
> index 0000000..a5119c9
> --- /dev/null
> +++ b/include/linux/extcon/extcon_palmas.h
I think it can be use palama.h only. No need to have one more header for
this.
> @@ -0,0 +1,26 @@
>
>
> - u8 linkstat;
> + int mailboxstat;
Do we really require mailboxstat?
next prev parent reply other threads:[~2013-05-06 14:32 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-07 13:21 [PATCH v2 0/4] usb: added palmas-usb driver and a few misc fixes Kishon Vijay Abraham I
2013-03-07 13:21 ` [PATCH v2 1/4] usb: dwc3: set dma_mask for dwc3_omap device Kishon Vijay Abraham I
2013-03-07 13:21 ` [PATCH v2 2/4] usb: dwc3: dwc3-omap: return -EPROBE_DEFER if probe has not yet executed Kishon Vijay Abraham I
2013-03-07 13:21 ` [PATCH v2 3/4] USB: Palmas OTG Transceiver Driver Kishon Vijay Abraham I
2013-03-14 13:56 ` Felipe Balbi
2013-03-14 14:53 ` kishon
2013-03-25 9:32 ` [PATCH v3] USB: PHY: Palmas USB " Kishon Vijay Abraham I
2013-03-25 9:46 ` Laxman Dewangan
2013-03-26 6:03 ` Kishon Vijay Abraham I
2013-03-26 9:01 ` Graeme Gregory
2013-03-26 9:12 ` Laxman Dewangan
2013-03-26 9:27 ` Graeme Gregory
2013-03-26 9:34 ` Laxman Dewangan
2013-03-26 9:51 ` Graeme Gregory
2013-03-26 11:28 ` Laxman Dewangan
2013-03-26 16:22 ` Stephen Warren
2013-03-26 16:57 ` Graeme Gregory
2013-03-26 20:23 ` Stephen Warren
2013-03-27 11:03 ` Graeme Gregory
2013-03-26 10:21 ` Felipe Balbi
2013-03-26 10:28 ` Laxman Dewangan
2013-03-26 12:07 ` Felipe Balbi
2013-03-26 16:14 ` Stephen Warren
2013-03-26 10:19 ` Felipe Balbi
2013-05-06 13:17 ` [PATCH v4] extcon: Palmas Extcon Driver Kishon Vijay Abraham I
2013-05-06 14:26 ` Laxman Dewangan [this message]
2013-05-07 5:06 ` Kishon Vijay Abraham I
2013-05-06 14:40 ` Mark Brown
2013-05-07 5:12 ` Kishon Vijay Abraham I
2013-05-07 7:58 ` Mark Brown
2013-05-07 9:47 ` Kishon Vijay Abraham I
2013-05-07 9:49 ` Graeme Gregory
2013-05-07 10:45 ` Mark Brown
2013-05-14 9:18 ` Kishon Vijay Abraham I
2013-05-14 9:54 ` Graeme Gregory
2013-05-14 18:43 ` Laxman Dewangan
2013-05-07 0:43 ` myungjoo.ham
2013-05-07 5:21 ` Kishon Vijay Abraham I
2013-05-22 6:23 ` Kishon Vijay Abraham I
2013-05-07 6:10 ` Chanwoo Choi
2013-05-07 6:25 ` Kishon Vijay Abraham I
2013-05-07 6:57 ` Chanwoo Choi
2013-05-07 7:05 ` Chanwoo Choi
2013-05-07 8:17 ` Kishon Vijay Abraham I
2013-03-07 13:21 ` [PATCH v2 4/4] usb: musb: omap2430: replace *_* with *-* in property names Kishon Vijay Abraham I
2013-03-14 13:57 ` Felipe Balbi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5187BDAF.2040303@nvidia.com \
--to=ldewangan@nvidia.com \
--cc=balbi@ti.com \
--cc=broonie@opensource.wolfsonmicro.com \
--cc=cw00.choi@samsung.com \
--cc=devicetree-discuss@lists.ozlabs.org \
--cc=gg@slimlogic.co.uk \
--cc=grant.likely@linaro.org \
--cc=kishon@ti.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=myungjoo.ham@samsung.com \
--cc=rob.herring@calxeda.com \
--cc=rob@landley.net \
--cc=ruchika@ti.com \
--cc=sameo@linux.intel.com \
--cc=tony@atomide.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).