Re: [PATCH v4] extcon: Palmas Extcon Driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Tuesday 07 May 2013 10:51 AM, Kishon Vijay Abraham I wrote:
Hi,

On Tuesday 07 May 2013 06:13 AM, myungjoo.ham wrote:
From: Graeme Gregory <gg@xxxxxxxxxxxxxxx>

This is the driver for the USB comparator built into the palmas chip. It
handles the various USB OTG events that can be generated by cable
insertion/removal.

Signed-off-by: Graeme Gregory <gg@xxxxxxxxxxxxxxx>
Signed-off-by: Moiz Sonasath <m-sonasath@xxxxxx>
Signed-off-by: Ruchika Kharwar <ruchika@xxxxxx>
Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx>
[kishon@xxxxxx: adapted palmas usb driver to use the extcon framework]
Signed-off-by: Sebastien Guiriec <s-guiriec@xxxxxx>

Here goes some comments in the code:

[]

diff --git a/drivers/extcon/extcon-palmas.c
b/drivers/extcon/extcon-palmas.c
new file mode 100644
index 0000000..3ef042f
--- /dev/null
+++ b/drivers/extcon/extcon-palmas.c
@@ -0,0 +1,389 @@
+/*
+ * Palmas USB transceiver driver
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated -
http://www.ti.com
+ * 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.
+ *
+ * Author: Graeme Gregory <gg@xxxxxxxxxxxxxxx>
+ * Author: Kishon Vijay Abraham I <kishon@xxxxxx>
+ *
+ * Based on twl6030_usb.c
+ *
+ * Author: Hema HK <hemahk@xxxxxx>
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */

Why the email addresses are masked in the source code?
(I'm just curious as this is the first time to see such in kernel source)

That was not intentional. Took the previous version from the net. My bad.

+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/usb/phy_companion.h>
+#include <linux/regulator/consumer.h>
+#include <linux/err.h>
+#include <linux/notifier.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/mfd/palmas.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/extcon/extcon_palmas.h>
+
+static const char *palmas_extcon_cable[] = {
+    [0] = "USB",
+    [1] = "USB-HOST",

    [1] = "USB-Host",

+    NULL,
+};
+
+static const int mutually_exclusive[] = {0x3, 0x0};
+
+static void palmas_usb_wakeup(struct palmas *palmas, int enable)
+{
+    if (enable)
+        palmas_write(palmas, PALMAS_USB_OTG_BASE, PALMAS_USB_WAKEUP,
+            PALMAS_USB_WAKEUP_ID_WK_UP_COMP);
+    else
+        palmas_write(palmas, PALMAS_USB_OTG_BASE, PALMAS_USB_WAKEUP,
0);
+}
+
+static ssize_t palmas_usb_vbus_show(struct device *dev,
+            struct device_attribute *attr, char *buf)
+{
+    unsigned long        flags;
+    int            ret = -EINVAL;
+    struct palmas_usb    *palmas_usb = dev_get_drvdata(dev);
+
+    spin_lock_irqsave(&palmas_usb->lock, flags);

This spinlock is used in this sysfs-show function only.
Is this really needed?
The only protected value here is "linkstat", which is "readonly"
in this protected context.
Thus, removing the spin_lock and updating like the following should
be the same with less overhead:

    int linkstat = palmas_usb->linkstat;
    switch(linkstat) {

hmm.. ok.
I think this is to do with disabling interrupts while reporting the VBUS state. (linkstat is updated in irq handler). So this should be fine I guess?

Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux