auxdisplay: ht16k33: use le16_to_cpup() to fetch LE16 data
Subsystems (2)
PATCH 1/1 auxdisplay: ht16k33: use le16_to_cpup() to fetch LE16 data
The data read from the device is 3 little-endian words, so let's annotate them as such and use le16_to_cpu() to convert them to host endianness - it might turn out to be a bit more performant, and it expresses the conversion more clearly.
--- drivers/auxdisplay/ht16k33.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/auxdisplay/ht16k33.c +4 -3
@@ -254,18 +254,19 @@ static bool ht16k33_keypad_scan(struct ht16k33_keypad *keypad) {const unsigned short *keycodes = keypad->dev->keycode;u16 new_state[HT16K33_MATRIX_KEYPAD_MAX_COLS ]; -u8 data[HT16K33_MATRIX_KEYPAD_MAX_COLS *2 ]; +__le16 data[HT16K33_MATRIX_KEYPAD_MAX_COLS ];unsigned long bits_changed;int row, col, code;bool pressed = false; -if (i2c_smbus_read_i2c_block_data (keypad->client,0x40 ,6 , data) !=6 ) { +if (i2c_smbus_read_i2c_block_data (keypad->client,0x40 , +sizeof (data), (u8 *)data) !=6 ) {dev_err (&keypad->client->dev,"Failed to read key data\n" );return false; }for (col =0 ; col < keypad->cols; col++) { - new_state[col] = (data[col *2 +1 ] <<8 ) | data[col *2 ]; + new_state[col] =le16_to_cpu (data[col]);if (new_state[col]) pressed = true; bits_changed = keypad->last_key_state[col] ^ new_state[col];
Signature · 5 lines
2.12.2.564.g063fe858b8-goog -- Dmitry
Robin van der Gracht
maintainerQuoted 4 lines · On Wed, 29 Mar 2017 00:42:08 -0700 Dmitry Torokhov <[email protected]>…
The data read from the device is 3 little-endian words, so let's annotate them as such and use
le16_to_cpu()to convert them to host endianness - it might turn out to be a bit more performant, and it expresses the conversion more clearly.
Agreed
Quoted 13 lines
Signed-off-by Dmitry Torokhov <[email protected]>--- drivers/auxdisplay/ht16k33.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)drivers/auxdisplay/ht16k33.c +0
-0@@ -254,18 +254,19 @@ static bool ht16k33_keypad_scan(struct ht16k33_keypad *keypad) {const unsigned short *keycodes = keypad->dev->keycode;
drivers/auxdisplay/ht16k33.c static bool ht16k33_keypad_scan(struct ht16k33_keypad *keypad) · line 263
Quoted 10 lines
drivers/auxdisplay/ht16k33.c +3
-2u16 new_state[HT16K33_MATRIX_KEYPAD_MAX_COLS ]; -u8 data[HT16K33_MATRIX_KEYPAD_MAX_COLS *2 ]; +__le16 data[HT16K33_MATRIX_KEYPAD_MAX_COLS ];unsigned long bits_changed;int row, col, code;bool pressed = false; -if (i2c_smbus_read_i2c_block_data (keypad->client,0x40 ,6 , data) !=6 ) { +if (i2c_smbus_read_i2c_block_data (keypad->client,0x40 , +sizeof (data), (u8 *)data) !=6 ) {
Why use sizeof(data) for length but check against the hardcoded 6?
drivers/auxdisplay/ht16k33.c static bool ht16k33_keypad_scan(struct ht16k33_keypad *keypad) · line 269
Quoted 7 lines
drivers/auxdisplay/ht16k33.c +1
-1dev_err (&keypad->client->dev,"Failed to read key data\n" );return false; }for (col =0 ; col < keypad->cols; col++) { - new_state[col] = (data[col *2 +1 ] <<8 ) | data[col *2 ]; + new_state[col] =le16_to_cpu (data[col]);
nice!
drivers/auxdisplay/ht16k33.c static bool ht16k33_keypad_scan(struct ht16k33_keypad *keypad) · line 272
Quoted 3 lines
drivers/auxdisplay/ht16k33.c +0
-0if (new_state[col]) pressed = true; bits_changed = keypad->last_key_state[col] ^ new_state[col];
Robin
Quoted 30 lines · Robin van der Gracht
Quoted 4 lines · On Wed, 29 Mar 2017 00:42:08 -0700 Dmitry Torokhov <[email protected]>…
The data read from the device is 3 little-endian words, so let's annotate them as such and use
le16_to_cpu()to convert them to host endianness - it might turn out to be a bit more performant, and it expresses the conversion more clearly.Agreed
Quoted 20 lines
Signed-off-by Dmitry Torokhov <[email protected]>--- drivers/auxdisplay/ht16k33.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)drivers/auxdisplay/ht16k33.c +1
-1@@ -254,18 +254,19 @@ static bool ht16k33_keypad_scan(struct ht16k33_keypad *keypad) {const unsigned short *keycodes = keypad->dev->keycode;u16 new_state[HT16K33_MATRIX_KEYPAD_MAX_COLS ]; -u8 data[HT16K33_MATRIX_KEYPAD_MAX_COLS *2 ]; +__le16 data[HT16K33_MATRIX_KEYPAD_MAX_COLS ];unsigned long bits_changed;int row, col, code;bool pressed = false;
Quoted 5 lines
drivers/auxdisplay/ht16k33.c static bool ht16k33_keypad_scan(struct ht16k33_keypad *keypad) · line 263
Quoted 3 lines
drivers/auxdisplay/ht16k33.c +2
-1-if (i2c_smbus_read_i2c_block_data (keypad->client,0x40 ,6 , data) !=6 ) { +if (i2c_smbus_read_i2c_block_data (keypad->client,0x40 , +sizeof (data), (u8 *)data) !=6 ) {Why use sizeof(data) for length but check against the hardcoded 6?
It was quite late into the night ;) V2 is coming.
Quoted 16 lines
drivers/auxdisplay/ht16k33.c static bool ht16k33_keypad_scan(struct ht16k33_keypad *keypad) · line 269
Quoted 8 lines
drivers/auxdisplay/ht16k33.c +1
-1dev_err (&keypad->client->dev,"Failed to read key data\n" );return false; }for (col =0 ; col < keypad->cols; col++) { - new_state[col] = (data[col *2 +1 ] <<8 ) | data[col *2 ]; + new_state[col] =le16_to_cpu (data[col]);nice!
drivers/auxdisplay/ht16k33.c static bool ht16k33_keypad_scan(struct ht16k33_keypad *keypad) · line 272
Quoted 3 lines
drivers/auxdisplay/ht16k33.c +0
-0if (new_state[col]) pressed = true; bits_changed = keypad->last_key_state[col] ^ new_state[col];Robin
Signature · 1 line
Dmitry