On 02/08/2017 05:40 PM, Andy Shevchenko wrote:
On Wed, 2017-02-08 at 16:17 +0100, Richard Leitner wrote:
On 02/08/2017 02:59 PM, Greg KH wrote:
On Wed, Feb 08, 2017 at 03:21:08PM +0200, Andy Shevchenko wrote:
On Wed, 2017-02-08 at 09:52 +0100, Richard Leitner wrote:
From: Richard Leitner <dev@xxxxxxxxxx>
+#define DRIVER_NAME "usb251xb"
+#define DRIVER_DESC "Microchip USB 2.0 Hi-Speed Hub Controller"
+#define DRIVER_VERSION "1.0"
Is it my MUA, or all above indentations are broken?
What do you mean?
Should the strings be aligned, like the following?
#define DRIVER_NAME "usb251xb"
#define DRIVER_DESC "Microchip USB .."
#define DRIVER_VERSION "1.0"
Yep, tab vs. space indentation.
Ok, will do that for v5.
Above doesn't make much sense. Why not to use
BIT(bit)
and
& ~BIT(bit)
in place?
I thought we already had functions to do this for you. Don't write
new
ones "by hand" either wya.
Which functions do you mean? I only found set_bit() and clear_bit()
from
atomic_ops. But those operate on "unsigned long" variables. From the
documentation:
Native atomic bit operations are defined to operate
on objects aligned to the size of an "unsigned long"
C data type, and are least of that size.
__set_bit(), __clear_bit() -- non-atomic variants, but you are right,
that (unsigned long) exactly the point I didn't propose them.
So the preferred solution is the BIT() stuff?
+ /* the first data byte transferred tells the
hub how
many data
+ * bytes will follow (byte count)
+ */
I'm not sure this is good formatted comment for USB subsystem.
Looks fine to me, why do you think it is incorrect?
I would do like
/*
* The multi-line
* comment.
*/
Capital letter, period at the end, first empty line (unlike in net
subsystem).
So what's the preferred format? Empty line at the beginning or not?
The captital letter and period looks fine. I'll apply that for v5.
+#else /* CONFIG_OF */
+static int usb251xb_get_ofdata(struct usb251xb *hub,
+ struct usb251xb_data *data)
+{
+ return 0;
+}
+#endif /* CONFIG_OF */
I don't think it's a good idea to have those ugly #ifdef.
How can it be removed?
__maybe_unused for function, device_property_*() instead of
of_property_*() calls.
Something like that. But if you are insisting this is *only* OF
available hardware or we don't care, I'll not object.
In usb3503.c and usb4604.c we have that #ifdef CONFIG_OF too. IMHO those
drivers should use the same solution here. But you guys are the ones
with tons of kernel coding experience, so just say how it should be done :-)
Thanks & regards,
Richard L
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
