diff mbox

Input: cros_ec_keyb - switch from using uint8_t to u8

Message ID 20131231193442.GA25208@core.coreip.homeip.net (mailing list archive)
State New, archived
Headers show

Commit Message

Dmitry Torokhov Dec. 31, 2013, 7:34 p.m. UTC
u8 is proper in-kernel type for unsigned byte data.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/cros_ec_keyb.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Doug Anderson Jan. 2, 2014, 4:12 p.m. UTC | #1
Dmitry,

On Tue, Dec 31, 2013 at 11:34 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> u8 is proper in-kernel type for unsigned byte data.

I won't say that I keep up with all the latest trends here, but this
surprised me so I did some research.  My findings don't agree with
your statement.  Perhaps there are different standards that are used
for the input subsystem?

Specifically looking at
<https://www.kernel.org/doc/Documentation/CodingStyle>, I see:

     Therefore, the Linux-specific 'u8/u16/u32/u64' types and their
     signed equivalents which are identical to standard types are
     permitted -- although they are not mandatory in new code of your
     own.

     When editing existing code which already uses one or the other set
     of types, you should conform to the existing choices in that code.

That makes it sound like the author of that document would prefer
uint8_t but will accept u8.  It also seems like if code is consistent
about using a given type (as this code is) that it shouldn't be
changed.

I'm always happy to be enlightened, though!

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov Jan. 2, 2014, 7:27 p.m. UTC | #2
On Thu, Jan 02, 2014 at 08:12:09AM -0800, Doug Anderson wrote:
> Dmitry,
> 
> On Tue, Dec 31, 2013 at 11:34 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > u8 is proper in-kernel type for unsigned byte data.
> 
> I won't say that I keep up with all the latest trends here, but this
> surprised me so I did some research.  My findings don't agree with
> your statement.  Perhaps there are different standards that are used
> for the input subsystem?
> 
> Specifically looking at
> <https://www.kernel.org/doc/Documentation/CodingStyle>, I see:
> 
>      Therefore, the Linux-specific 'u8/u16/u32/u64' types and their
>      signed equivalents which are identical to standard types are
>      permitted -- although they are not mandatory in new code of your
>      own.
> 
>      When editing existing code which already uses one or the other set
>      of types, you should conform to the existing choices in that code.
> 
> That makes it sound like the author of that document would prefer
> uint8_t but will accept u8.  It also seems like if code is consistent
> about using a given type (as this code is) that it shouldn't be
> changed.
> 
> I'm always happy to be enlightened, though!

I prefer uXX in kernel because it matches __uXX that we publish in UAPI.
Also here is Linus's response form the discussion that introduced that
particular wording in CodingStyle [1]:

"The problem with uint32_t is that it's ugly, it used to be unportable,
and you can't use it in header files _anyway_.

In other words, there's no _point_ to the "standard type".

I really object to this whole thing. The fact is, "u8" and friends _are_
the standard types as far as the kernel is concerned.  Claiming that
they aren't is just silly.

It's the "uint32_t" kind of thing that isn't standard within the kernel.
You can't use that thing in public header files anyway due to name
scoping rules, so they have basically no redeeming features."

Thanks.
Luigi Semenzato Jan. 3, 2014, 12:13 a.m. UTC | #3
Thank you, this is useful information, and it would be even more
useful if it made it in Documentation/CodingStyle :-)



On Thu, Jan 2, 2014 at 11:27 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Jan 02, 2014 at 08:12:09AM -0800, Doug Anderson wrote:
>> Dmitry,
>>
>> On Tue, Dec 31, 2013 at 11:34 AM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > u8 is proper in-kernel type for unsigned byte data.
>>
>> I won't say that I keep up with all the latest trends here, but this
>> surprised me so I did some research.  My findings don't agree with
>> your statement.  Perhaps there are different standards that are used
>> for the input subsystem?
>>
>> Specifically looking at
>> <https://www.kernel.org/doc/Documentation/CodingStyle>, I see:
>>
>>      Therefore, the Linux-specific 'u8/u16/u32/u64' types and their
>>      signed equivalents which are identical to standard types are
>>      permitted -- although they are not mandatory in new code of your
>>      own.
>>
>>      When editing existing code which already uses one or the other set
>>      of types, you should conform to the existing choices in that code.
>>
>> That makes it sound like the author of that document would prefer
>> uint8_t but will accept u8.  It also seems like if code is consistent
>> about using a given type (as this code is) that it shouldn't be
>> changed.
>>
>> I'm always happy to be enlightened, though!
>
> I prefer uXX in kernel because it matches __uXX that we publish in UAPI.
> Also here is Linus's response form the discussion that introduced that
> particular wording in CodingStyle [1]:
>
> "The problem with uint32_t is that it's ugly, it used to be unportable,
> and you can't use it in header files _anyway_.
>
> In other words, there's no _point_ to the "standard type".
>
> I really object to this whole thing. The fact is, "u8" and friends _are_
> the standard types as far as the kernel is concerned.  Claiming that
> they aren't is just silly.
>
> It's the "uint32_t" kind of thing that isn't standard within the kernel.
> You can't use that thing in public header files anyway due to name
> scoping rules, so they have basically no redeeming features."
>
> Thanks.
>
> --
> Dmitry
>
> [1] http://marc.info/?l=linux-kernel&m=114659539715468&w=2
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 4083796..d44c5d4 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -50,7 +50,7 @@  struct cros_ec_keyb {
 	int row_shift;
 	const struct matrix_keymap_data *keymap_data;
 	bool ghost_filter;
-	uint8_t *old_kb_state;
+	u8 *old_kb_state;
 
 	struct device *dev;
 	struct input_dev *idev;
@@ -60,7 +60,7 @@  struct cros_ec_keyb {
 
 
 static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
-					  uint8_t *buf, int row)
+					  u8 *buf, int row)
 {
 	int pressed_in_row = 0;
 	int row_has_teeth = 0;
@@ -89,7 +89,7 @@  static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
  * Returns true when there is at least one combination of pressed keys that
  * results in ghosting.
  */
-static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
+static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, u8 *buf)
 {
 	int row;
 
@@ -132,7 +132,7 @@  static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
  * per column)
  */
 static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
-			 uint8_t *kb_state, int len)
+				 u8 *kb_state, int len)
 {
 	struct input_dev *idev = ckdev->idev;
 	int col, row;
@@ -189,19 +189,19 @@  static void cros_ec_keyb_close(struct input_dev *dev)
 					   &ckdev->notifier);
 }
 
-static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
+static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, u8 *kb_state)
 {
 	return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
 					  kb_state, ckdev->cols);
 }
 
 static int cros_ec_keyb_work(struct notifier_block *nb,
-		     unsigned long state, void *_notify)
+			     unsigned long state, void *_notify)
 {
 	int ret;
 	struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
 						    notifier);
-	uint8_t kb_state[ckdev->cols];
+	u8 kb_state[ckdev->cols];
 
 	ret = cros_ec_keyb_get_state(ckdev, kb_state);
 	if (ret >= 0)
@@ -282,8 +282,8 @@  static int cros_ec_keyb_probe(struct platform_device *pdev)
 /* Clear any keys in the buffer */
 static void cros_ec_keyb_clear_keyboard(struct cros_ec_keyb *ckdev)
 {
-	uint8_t old_state[ckdev->cols];
-	uint8_t new_state[ckdev->cols];
+	u8 old_state[ckdev->cols];
+	u8 new_state[ckdev->cols];
 	unsigned long duration;
 	int i, ret;