Re: [greybus-dev] [PATCH] staging: greybus: fixed the coding style, labels should not be indented.

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

 



On 6/3/21 4:48 PM, David Laight wrote:
From: Alex Elder
Sent: 03 June 2021 22:46

On 6/3/21 4:22 PM, David Laight wrote:
...
--- a/drivers/staging/greybus/gpio.c
+++ b/drivers/staging/greybus/gpio.c
@@ -20,9 +20,9 @@
    struct gb_gpio_line {
        /* The following has to be an array of line_max entries */
        /* --> make them just a flags field */
-     u8                      active:    1,
-                             direction: 1,   /* 0 = output, 1 = input */
-                             value:     1;   /* 0 = low, 1 = high */
+     u8                      active:1,
+                             direction:1,    /* 0 = output, 1 = input */
+                             value:1;        /* 0 = low, 1 = high */

Why are you even using bitfields at all?
If you cared about the structure size you'd not have a byte-size pad here.

Apparently I committed this, and it was part of the very first
Greybus drivers...

These would be better defined as Booleans; there are others in
the same structure after all.  That would have avoided the
checkpatch problem in the first place.

Using 'u8' can be sensible.
Boolean will be 32bit.

Not necessarily, sizeof(bool) is implementation defined.
And I thought you didn't think the size of the structure
was very important...

In any case, I'm open to changing the type of these fields,
and my preference would be bool rather than u8, because it
is completely clear what it represents.

					-Alex

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)






[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux