On 14/01/16 14:29, Konstantin Shkolnyy wrote:
From: Martyn Welch [mailto:martyn.welch@xxxxxxxxxxxxxxx]
Sent: Thursday, January 14, 2016 04:23
To: Konstantin Shkolnyy; Johan Hovold; Linus Walleij; Alexandre Courbot
Cc: Greg Kroah-Hartman; linux-usb@xxxxxxxxxxxxxxx; linux-
gpio@xxxxxxxxxxxxxxx
Subject: Re: [PATCH v2] USB: serial: cp210x: Adding GPIO support for CP2105
On 14/01/16 00:27, Konstantin Shkolnyy wrote:
Comments inline, not comprehensive by any measure...
-----Original Message-----
From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb-
owner@xxxxxxxxxxxxxxx] On Behalf Of Martyn Welch
Sent: Wednesday, January 13, 2016 06:31
To: Johan Hovold; Linus Walleij; Alexandre Courbot
Cc: Greg Kroah-Hartman; linux-usb@xxxxxxxxxxxxxxx; linux-
gpio@xxxxxxxxxxxxxxx; Martyn Welch
Subject: [PATCH v2] USB: serial: cp210x: Adding GPIO support for CP2105
...
+#include <linux/gpio/driver.h>
+#include <linux/bitops.h>
Enclose this in CONFIG_GPIOLIB?
...
Is there any point? I took a look at a few other drivers which
optionally support GPIO and they don't ifdef the headers.
OK, according to other comment, I need to learn about local ifdef policies. I'm sorry.
To me, knowing *why * a header is included seems beneficial, but obviously there are other considerations.
I think the contents of the headers will effectively be ignored if not
used and this won't affect module size.
I think a good linker will throw away anything that is not referenced anyway.
...
+static int cp210x_gpio_get(struct gpio_chip *gc, unsigned gpio)
+{
+ struct cp210x_port_private *port_priv =
+ container_of(gc, struct cp210x_port_private, gc);
+ struct usb_serial *serial = port_priv->serial;
+ __le32 *buf;
+ int result, i, length;
+ int size = 1;
+ unsigned int data[size];
+
+ /* Number of integers required to contain the array */
+ length = (((size - 1) | 3) + 1) / 4;
usb_control_msg can deal with any size of the buffer, so use __le16 and
remove these length calculations.
OK. (This is the process used in the other calls. Was wondering why they
are in the first place, any ideas?)
Johan Hovold previously said he didn't like these functions either. I actually submitted a patch to replace them with simpler ones.
...
Ah! OK, found them in mail archive. Will take a look.
Martyn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
