[PATCH v2 1/7] lib/hexdump.c: Fix selftests

[Alastair D'Silva]

From: Alastair D'Silva 

The overflow tests did not account for the situation where no
overflow occurs and len < rowsize.

This patch renames the cryptic variables and accounts for the
above case.

The selftests now pass.

Signed-off-by: Alastair D'Silva 
---
 lib/test_hexdump.c | 47 ++
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
index 5144899d3c6b..d78ddd62ffd0 100644
--- a/lib/test_hexdump.c
+++ b/lib/test_hexdump.c
@@ -163,45 +163,52 @@ static void __init test_hexdump_overflow(size_t buflen, 
size_t len,
 {
char test[TEST_HEXDUMP_BUF_SIZE];
char buf[TEST_HEXDUMP_BUF_SIZE];
-   int rs = rowsize, gs = groupsize;
-   int ae, he, e, f, r;
-   bool a;
+   int ascii_len, hex_len, expected_len, fill_point, ngroups, rc;
+   bool match;
 
total_tests++;
 
memset(buf, FILL_CHAR, sizeof(buf));
 
-   r = hex_dump_to_buffer(data_b, len, rs, gs, buf, buflen, ascii);
+   rc = hex_dump_to_buffer(data_b, len, rowsize, groupsize, buf, buflen, 
ascii);
 
/*
 * Caller must provide the data length multiple of groupsize. The
 * calculations below are made with that assumption in mind.
 */
-   ae = rs * 2 /* hex */ + rs / gs /* spaces */ + 1 /* space */ + len /* 
ascii */;
-   he = (gs * 2 /* hex */ + 1 /* space */) * len / gs - 1 /* no trailing 
space */;
+   ngroups = rowsize / groupsize;
+   hex_len = (groupsize * 2 /* hex */ + 1 /* spaces */) * ngroups
+ - 1 /* no trailing space */;
+   ascii_len = hex_len + 2 /* space */ + len /* ascii */;
+
+   if (len < rowsize) {
+   ngroups = len / groupsize;
+   hex_len = (groupsize * 2 /* hex */ + 1 /* spaces */) * ngroups
+ - 1 /* no trailing space */;
+   }
 
-   if (ascii)
-   e = ae;
-   else
-   e = he;
+   expected_len = (ascii) ? ascii_len : hex_len;
 
-   f = min_t(int, e + 1, buflen);
+   fill_point = min_t(int, expected_len + 1, buflen);
if (buflen) {
-   test_hexdump_prepare_test(len, rs, gs, test, sizeof(test), 
ascii);
-   test[f - 1] = '\0';
+   test_hexdump_prepare_test(len, rowsize, groupsize, test,
+ sizeof(test), ascii);
+   test[fill_point - 1] = '\0';
}
-   memset(test + f, FILL_CHAR, sizeof(test) - f);
+   memset(test + fill_point, FILL_CHAR, sizeof(test) - fill_point);
 
-   a = r == e && !memcmp(test, buf, TEST_HEXDUMP_BUF_SIZE);
+   match = rc == expected_len && !memcmp(test, buf, TEST_HEXDUMP_BUF_SIZE);
 
buf[sizeof(buf) - 1] = '\0';
 
-   if (!a) {
-   pr_err("Len: %zu buflen: %zu strlen: %zu\n",
-   len, buflen, strnlen(buf, sizeof(buf)));
-   pr_err("Result: %d '%s'\n", r, buf);
-   pr_err("Expect: %d '%s'\n", e, test);
+   if (!match) {
+   pr_err("rowsize: %u groupsize: %u ascii: %d Len: %zu buflen: 
%zu strlen: %zu\n",
+   rowsize, groupsize, ascii, len, buflen,
+   strnlen(buf, sizeof(buf)));
+   pr_err("Result: %d '%-.*s'\n", rc, (int)buflen, buf);
+   pr_err("Expect: %d '%-.*s'\n", expected_len, (int)buflen, test);
failed_tests++;
+
}
 }
 
-- 
2.21.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 0/7] Hexdump Enhancements

[Alastair D'Silva]

From: Alastair D'Silva 

Apologies for the large CC list, it's a heads up for those responsible
for subsystems where a prototype change in generic code causes a change
in those subsystems.

This series enhances hexdump.

These improve the readability of the dumped data in certain situations
(eg. wide terminals are available, many lines of empty bytes exist, etc).

The default behaviour of hexdump is unchanged, however, the prototype
for hex_dump_to_buffer() has changed, and print_hex_dump() has been
renamed to print_hex_dump_ext(), with a wrapper replacing it for
compatibility with existing code, which would have been too invasive to
change.

Hexdump selftests have be run & confirmed passed.

Changelog:
 - Fix failing selftests
 - Fix precedence bug in 'Replace ascii bool in hex_dump_to_buffer...'
 - Remove hardcoded new lengths & instead relax the checks in
   hex_dump_to_buffer, allocating the buffer from the heap instead of the
   stack.
 - Replace the skipping of lines of 0x00/0xff with skipping lines of
   repeated characters, announcing what has been skipped.
 - Add spaces as an optional N-group separator
 - Allow byte ordering to be maintained when HEXDUMP_RETAIN_BYTE_ORDERING
   is set.
 - Updated selftests to cover 'Relax rowsize checks' &
   'Optionally retain byte ordering'

Alastair D'Silva (7):
  lib/hexdump.c: Fix selftests
  lib/hexdump.c: Relax rowsize checks in hex_dump_to_buffer
  lib/hexdump.c: Optionally suppress lines of repeated bytes
  lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
  lib/hexdump.c: Allow multiple groups to be separated by lines '|'
  lib/hexdump.c: Allow multiple groups to be separated by spaces
  lib/hexdump.c: Optionally retain byte ordering

 drivers/gpu/drm/i915/intel_engine_cs.c|   2 +-
 drivers/isdn/hardware/mISDN/mISDNisar.c   |   6 +-
 drivers/mailbox/mailbox-test.c|   2 +-
 drivers/net/ethernet/amd/xgbe/xgbe-drv.c  |   2 +-
 .../net/ethernet/synopsys/dwc-xlgmac-common.c |   2 +-
 drivers/net/wireless/ath/ath10k/debug.c   |   3 +-
 .../net/wireless/intel/iwlegacy/3945-mac.c|   2 +-
 drivers/platform/chrome/wilco_ec/debugfs.c|   2 +-
 drivers/scsi/scsi_logging.c   |   8 +-
 drivers/staging/fbtft/fbtft-core.c|   2 +-
 fs/seq_file.c |   3 +-
 include/linux/printk.h|  34 ++-
 lib/hexdump.c | 260 +++---
 lib/test_hexdump.c| 146 +++---
 14 files changed, 372 insertions(+), 102 deletions(-)

-- 
2.21.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 5/7] lib/hexdump.c: Allow multiple groups to be separated by lines '|'

[Alastair D'Silva]

From: Alastair D'Silva 

With the wider display format, it can become hard to identify how many
bytes into the line you are looking at.

The patch adds new flags to hex_dump_to_buffer() and print_hex_dump() to
print vertical lines to separate every N groups of bytes.

eg.
buf:: 454d414e 43415053|4e495f45 00584544  NAMESPAC|E_INDEX.
buf:0010:  0002|   |

Signed-off-by: Alastair D'Silva 
---
 include/linux/printk.h |  3 +++
 lib/hexdump.c  | 59 --
 2 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 00a82e468643..dc693aec394c 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -482,6 +482,9 @@ enum {
 
 #define HEXDUMP_ASCII  (1 << 0)
 #define HEXDUMP_SUPPRESS_REPEATED  (1 << 1)
+#define HEXDUMP_2_GRP_LINES(1 << 2)
+#define HEXDUMP_4_GRP_LINES(1 << 3)
+#define HEXDUMP_8_GRP_LINES(1 << 4)
 
 extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
  int groupsize, char *linebuf, size_t linebuflen,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index ddd1697e5f9b..6f4d1176c332 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -77,6 +77,23 @@ char *bin2hex(char *dst, const void *src, size_t count)
 }
 EXPORT_SYMBOL(bin2hex);
 
+static const char *group_separator(int group, u64 flags)
+{
+   if (group == 0)
+   return " ";
+
+   if ((flags & HEXDUMP_8_GRP_LINES) && !((group) % 8))
+   return "|";
+
+   if ((flags & HEXDUMP_4_GRP_LINES) && !((group) % 4))
+   return "|";
+
+   if ((flags & HEXDUMP_2_GRP_LINES) && !((group) % 2))
+   return "|";
+
+   return " ";
+}
+
 /**
  * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
  * @buf: data blob to dump
@@ -87,6 +104,9 @@ EXPORT_SYMBOL(bin2hex);
  * @linebuflen: total size of @linebuf, including space for terminating NUL
  * @flags: A bitwise OR of the following flags:
  * HEXDUMP_ASCII:  include ASCII after the hex output
+ * HEXDUMP_2_GRP_LINES:insert a '|' after every 2 groups
+ * HEXDUMP_4_GRP_LINES:insert a '|' after every 4 groups
+ * HEXDUMP_8_GRP_LINES:insert a '|' after every 8 groups
  *
  * hex_dump_to_buffer() works on one "line" of output at a time, converting
  *  bytes of input to hexadecimal (and optionally printable ASCII)
@@ -118,6 +138,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
int j, lx = 0;
int ascii_column;
int ret;
+   int line_chars = 0;
 
if (!is_power_of_2(groupsize) || groupsize > 8)
groupsize = 1;
@@ -144,7 +165,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
 
for (j = 0; j < ngroups; j++) {
ret = snprintf(linebuf + lx, linebuflen - lx,
-  "%s%16.16llx", j ? " " : "",
+  "%s%16.16llx",
+  j ? group_separator(j, flags) : "",
   get_unaligned(ptr8 + j));
if (ret >= linebuflen - lx)
goto overflow1;
@@ -155,7 +177,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
 
for (j = 0; j < ngroups; j++) {
ret = snprintf(linebuf + lx, linebuflen - lx,
-  "%s%8.8x", j ? " " : "",
+  "%s%8.8x",
+  j ? group_separator(j, flags) : "",
   get_unaligned(ptr4 + j));
if (ret >= linebuflen - lx)
goto overflow1;
@@ -166,7 +189,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
 
for (j = 0; j < ngroups; j++) {
ret = snprintf(linebuf + lx, linebuflen - lx,
-  "%s%4.4x", j ? " " : "",
+  "%s%4.4x",
+  j ? group_separator(j, flags) : "",
   get_unaligned(ptr2 + j));
if (ret >= linebuflen - lx)
goto overflow1;
@@ -196,11 +220,26 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
goto overflow2;
linebuf[lx++] = ' ';
}
+
+   if (flags & HEXDUMP_2_GRP_LINES)
+   line_chars = groupsize * 2;
+   if (flags & HEXDUMP_4_GRP_LINES)
+   line_chars = groupsize * 4;
+   if (flags & HEXDUMP_8_GRP_LINES)
+   line_cha

[PATCH v2 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes

[Alastair D'Silva]

From: Alastair D'Silva 

Some buffers may only be partially filled with useful data, while the rest
is padded (typically with 0x00 or 0xff).

This patch introduces a flag to allow the supression of lines of repeated
bytes, which are replaced with '** Skipped %u bytes of value 0x%x **'

An inline wrapper function is provided for backwards compatibility with
existing code, which maintains the original behaviour.

Signed-off-by: Alastair D'Silva 
---
 include/linux/printk.h | 25 +---
 lib/hexdump.c  | 91 --
 2 files changed, 99 insertions(+), 17 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index d7c77ed1a4cb..938a67580d78 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -479,13 +479,18 @@ enum {
DUMP_PREFIX_ADDRESS,
DUMP_PREFIX_OFFSET
 };
+
 extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
  int groupsize, char *linebuf, size_t linebuflen,
  bool ascii);
+
+#define HEXDUMP_ASCII  (1 << 0)
+#define HEXDUMP_SUPPRESS_REPEATED  (1 << 1)
+
 #ifdef CONFIG_PRINTK
-extern void print_hex_dump(const char *level, const char *prefix_str,
+extern void print_hex_dump_ext(const char *level, const char *prefix_str,
   int prefix_type, int rowsize, int groupsize,
-  const void *buf, size_t len, bool ascii);
+  const void *buf, size_t len, u64 flags);
 #if defined(CONFIG_DYNAMIC_DEBUG)
 #define print_hex_dump_bytes(prefix_str, prefix_type, buf, len)\
dynamic_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true)
@@ -494,18 +499,28 @@ extern void print_hex_dump_bytes(const char *prefix_str, 
int prefix_type,
 const void *buf, size_t len);
 #endif /* defined(CONFIG_DYNAMIC_DEBUG) */
 #else
-static inline void print_hex_dump(const char *level, const char *prefix_str,
+static inline void print_hex_dump_ext(const char *level, const char 
*prefix_str,
  int prefix_type, int rowsize, int groupsize,
- const void *buf, size_t len, bool ascii)
+ const void *buf, size_t len, u64 flags)
 {
 }
 static inline void print_hex_dump_bytes(const char *prefix_str, int 
prefix_type,
const void *buf, size_t len)
 {
 }
-
 #endif
 
+static __always_inline void print_hex_dump(const char *level,
+  const char *prefix_str,
+  int prefix_type, int rowsize,
+  int groupsize, const void *buf,
+  size_t len, bool ascii)
+{
+   print_hex_dump_ext(level, prefix_str, prefix_type, rowsize, groupsize,
+   buf, len, ascii ? HEXDUMP_ASCII : 0);
+}
+
+
 #if defined(CONFIG_DYNAMIC_DEBUG)
 #define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
 groupsize, buf, len, ascii)\
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 3943507bc0e9..d61a1e4f19fa 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
 EXPORT_SYMBOL(hex_dump_to_buffer);
 
 #ifdef CONFIG_PRINTK
+
+/**
+ * Check if a buffer contains only a single byte value
+ * @buf: pointer to the buffer
+ * @len: the size of the buffer in bytes
+ * @val: outputs the value if if the bytes are identical
+ */
+static bool buf_is_all(const u8 *buf, size_t len, u8 *val_out)
+{
+   size_t i;
+   u8 val;
+
+   if (len <= 1)
+   return false;
+
+   val = buf[0];
+
+   for (i = 1; i < len; i++) {
+   if (buf[i] != val)
+   return false;
+   }
+
+   *val_out = val;
+   return true;
+}
+
+static void announce_skipped(const char *level, const char *prefix_str,
+  u8 val, size_t count)
+{
+   if (count == 0)
+   return;
+
+   printk("%s%s ** Skipped %lu bytes of value 0x%x **\n",
+  level, prefix_str, count, val);
+}
+
 /**
- * print_hex_dump - print a text hex dump to syslog for a binary blob of data
+ * print_hex_dump_ext: dump a binary blob of data to syslog in hexadecimal
  * @level: kernel log level (e.g. KERN_DEBUG)
  * @prefix_str: string to prefix each line with;
  *  caller supplies trailing spaces for alignment if desired
@@ -224,6 +260,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
  * @buf: data blob to dump
  * @len: number of bytes in the @buf
  * @ascii: include ASCII after the hex output
+ * @flags: A bitwise OR of the following flags:
+ * HEXDUMP_ASCII:  include ASCII after the hex output
+ * HEXDUMP_SUPPRESS_REPEATED:  suppress repeated lines of identical
+ *

[PATCH v2 2/7] lib/hexdump.c: Relax rowsize checks in hex_dump_to_buffer

[Alastair D'Silva]

From: Alastair D'Silva 

This patch removes the hardcoded row limits and allows for
other lengths. These lengths must still be a multiple of
groupsize.

This allows structs that are not 16/32 bytes to display on
a single line.

This patch also expands the self-tests to test row sizes
up to 64 bytes (though they can now be arbitrarily long).

Signed-off-by: Alastair D'Silva 
---
 lib/hexdump.c  | 48 --
 lib/test_hexdump.c | 52 ++
 2 files changed, 75 insertions(+), 25 deletions(-)

diff --git a/lib/hexdump.c b/lib/hexdump.c
index 81b70ed37209..3943507bc0e9 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 const char hex_asc[] = "0123456789abcdef";
@@ -80,14 +81,15 @@ EXPORT_SYMBOL(bin2hex);
  * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
  * @buf: data blob to dump
  * @len: number of bytes in the @buf
- * @rowsize: number of bytes to print per line; must be 16 or 32
+ * @rowsize: number of bytes to print per line; must be a multiple of groupsize
  * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
  * @linebuf: where to put the converted data
  * @linebuflen: total size of @linebuf, including space for terminating NUL
  * @ascii: include ASCII after the hex output
  *
- * hex_dump_to_buffer() works on one "line" of output at a time, i.e.,
- * 16 or 32 bytes of input data converted to hex + ASCII output.
+ * hex_dump_to_buffer() works on one "line" of output at a time, converting
+ *  bytes of input to hexadecimal (and optionally printable ASCII)
+ * until  bytes have been emitted.
  *
  * Given a buffer of u8 data, hex_dump_to_buffer() converts the input data
  * to a hex + ASCII dump at the supplied memory location.
@@ -116,16 +118,17 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
int ascii_column;
int ret;
 
-   if (rowsize != 16 && rowsize != 32)
-   rowsize = 16;
-
-   if (len > rowsize)  /* limit to one line at a time */
-   len = rowsize;
if (!is_power_of_2(groupsize) || groupsize > 8)
groupsize = 1;
if ((len % groupsize) != 0) /* no mixed size output */
groupsize = 1;
 
+   if (rowsize % groupsize)
+   rowsize -= rowsize % groupsize;
+
+   if (len > rowsize)  /* limit to one line at a time */
+   len = rowsize;
+
ngroups = len / groupsize;
ascii_column = rowsize * 2 + rowsize / groupsize + 1;
 
@@ -216,7 +219,7 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
  *  caller supplies trailing spaces for alignment if desired
  * @prefix_type: controls whether prefix of an offset, address, or none
  *  is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE)
- * @rowsize: number of bytes to print per line; must be 16 or 32
+ * @rowsize: number of bytes to print per line; must be a multiple of groupsize
  * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
  * @buf: data blob to dump
  * @len: number of bytes in the @buf
@@ -226,10 +229,9 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
  * to the kernel log at the specified kernel log level, with an optional
  * leading prefix.
  *
- * print_hex_dump() works on one "line" of output at a time, i.e.,
- * 16 or 32 bytes of input data converted to hex + ASCII output.
  * print_hex_dump() iterates over the entire input @buf, breaking it into
- * "line size" chunks to format and print.
+ * lines of rowsize/groupsize groups of input data converted to hex +
+ * (optionally) ASCII output.
  *
  * E.g.:
  *   print_hex_dump(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS,
@@ -246,17 +248,29 @@ void print_hex_dump(const char *level, const char 
*prefix_str, int prefix_type,
 {
const u8 *ptr = buf;
int i, linelen, remaining = len;
-   unsigned char linebuf[32 * 3 + 2 + 32 + 1];
+   unsigned char *linebuf;
+   unsigned int linebuf_len;
 
-   if (rowsize != 16 && rowsize != 32)
-   rowsize = 16;
+   if (rowsize % groupsize)
+   rowsize -= rowsize % groupsize;
+
+   /* Worst case line length:
+* 2 hex chars + space per byte in, 2 spaces, 1 char per byte in, NULL
+*/
+   linebuf_len = rowsize * 3 + 2 + rowsize + 1;
+   linebuf = kzalloc(linebuf_len, GFP_KERNEL);
+   if (!linebuf) {
+   printk("%s%shexdump: Could not alloc %u bytes for buffer\n",
+   level, prefix_str, linebuf_len);
+   return;
+   }
 
for (i = 0; i < len; i += rowsize) {
linelen = min(remaining, rowsize);
remaining -= rowsize;
 
hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
-  linebuf, sizeof(linebuf), ascii);
+

[PATCH v2 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags

[Alastair D'Silva]

From: Alastair D'Silva 

In order to support additional features in hex_dump_to_buffer, replace
the ascii bool parameter with flags.

Signed-off-by: Alastair D'Silva 
---
 drivers/gpu/drm/i915/intel_engine_cs.c|  2 +-
 drivers/isdn/hardware/mISDN/mISDNisar.c   |  6 --
 drivers/mailbox/mailbox-test.c|  2 +-
 drivers/net/ethernet/amd/xgbe/xgbe-drv.c  |  2 +-
 drivers/net/ethernet/synopsys/dwc-xlgmac-common.c |  2 +-
 drivers/net/wireless/ath/ath10k/debug.c   |  3 ++-
 drivers/net/wireless/intel/iwlegacy/3945-mac.c|  2 +-
 drivers/platform/chrome/wilco_ec/debugfs.c|  2 +-
 drivers/scsi/scsi_logging.c   |  8 +++-
 drivers/staging/fbtft/fbtft-core.c|  2 +-
 fs/seq_file.c |  3 ++-
 include/linux/printk.h|  8 
 lib/hexdump.c | 15 ---
 lib/test_hexdump.c|  5 +++--
 14 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index 49fa43ff02ba..fb133e729f9a 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m, const void 
*buf, size_t len)
WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos,
rowsize, sizeof(u32),
line, sizeof(line),
-   false) >= sizeof(line));
+   0) >= sizeof(line));
drm_printf(m, "[%04zx] %s\n", pos, line);
 
prev = buf + pos;
diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c 
b/drivers/isdn/hardware/mISDN/mISDNisar.c
index 386731ec2489..f13f34db6c17 100644
--- a/drivers/isdn/hardware/mISDN/mISDNisar.c
+++ b/drivers/isdn/hardware/mISDN/mISDNisar.c
@@ -84,7 +84,8 @@ send_mbox(struct isar_hw *isar, u8 his, u8 creg, u8 len, u8 
*msg)
 
while (l < (int)len) {
hex_dump_to_buffer(msg + l, len - l, 32, 1,
-  isar->log, 256, 1);
+  isar->log, 256,
+  HEXDUMP_ASCII);
pr_debug("%s: %s %02x: %s\n", isar->name,
 __func__, l, isar->log);
l += 32;
@@ -113,7 +114,8 @@ rcv_mbox(struct isar_hw *isar, u8 *msg)
 
while (l < (int)isar->clsb) {
hex_dump_to_buffer(msg + l, isar->clsb - l, 32,
-  1, isar->log, 256, 1);
+  1, isar->log, 256,
+  HEXDUMP_ASCII);
pr_debug("%s: %s %02x: %s\n", isar->name,
 __func__, l, isar->log);
l += 32;
diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
index 4e4ac4be6423..2f9a094d0259 100644
--- a/drivers/mailbox/mailbox-test.c
+++ b/drivers/mailbox/mailbox-test.c
@@ -213,7 +213,7 @@ static ssize_t mbox_test_message_read(struct file *filp, 
char __user *userbuf,
hex_dump_to_buffer(ptr,
   MBOX_BYTES_PER_LINE,
   MBOX_BYTES_PER_LINE, 1, touser + l,
-  MBOX_HEXDUMP_LINE_LEN, true);
+  MBOX_HEXDUMP_LINE_LEN, HEXDUMP_ASCII);
 
ptr += MBOX_BYTES_PER_LINE;
l += MBOX_HEXDUMP_LINE_LEN;
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c 
b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index 0cc911f928b1..e954a31cee0c 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -2992,7 +2992,7 @@ void xgbe_print_pkt(struct net_device *netdev, struct 
sk_buff *skb, bool tx_rx)
unsigned int len = min(skb->len - i, 32U);
 
hex_dump_to_buffer(&skb->data[i], len, 32, 1,
-  buffer, sizeof(buffer), false);
+  buffer, sizeof(buffer), 0);
netdev_dbg(netdev, "  %#06x: %s\n", i, buffer);
}
 
diff --git a/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c 
b/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c
index eb1c6b03c329..b80adfa1f890 100644
--- a/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c
+++ b/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c
@@ -349,7 +349,7 @@ void xlgmac_print_pkt(struct net_device *netdev,
unsigned int len = min(skb->len - i

[PATCH v2 7/7] lib/hexdump.c: Optionally retain byte ordering

[Alastair D'Silva]

From: Alastair D'Silva 

The behaviour of hexdump groups is to print the data out as if
it was a native-endian number.

This patch tweaks the documentation to make this clear, and also
adds the HEXDUMP_RETAIN_BYTE_ORDER flag to allow groups of
multiple bytes to be printed without affecting the ordering
of the printed bytes.

Signed-off-by: Alastair D'Silva 
---
 include/linux/printk.h |  1 +
 lib/hexdump.c  | 30 +
 lib/test_hexdump.c | 60 +-
 3 files changed, 68 insertions(+), 23 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 5231a14e4593..15277d50159c 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -488,6 +488,7 @@ enum {
 #define HEXDUMP_2_GRP_SPACES   (1 << 5)
 #define HEXDUMP_4_GRP_SPACES   (1 << 6)
 #define HEXDUMP_8_GRP_SPACES   (1 << 7)
+#define HEXDUMP_RETAIN_BYTE_ORDER  (1 << 8)
 
 extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
  int groupsize, char *linebuf, size_t linebuflen,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index febd614406d1..bfc9800630ae 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -127,7 +127,8 @@ static void separator_parameters(u64 flags, int groupsize, 
int *sep_chars,
  * @buf: data blob to dump
  * @len: number of bytes in the @buf
  * @rowsize: number of bytes to print per line; must be a multiple of groupsize
- * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
+ * @groupsize: number of bytes to convert to a native endian number and print:
+ *1, 2, 4, 8; default = 1
  * @linebuf: where to put the converted data
  * @linebuflen: total size of @linebuf, including space for terminating NUL
  * @flags: A bitwise OR of the following flags:
@@ -138,6 +139,9 @@ static void separator_parameters(u64 flags, int groupsize, 
int *sep_chars,
  * HEXDUMP_2_GRP_SPACES:   insert a ' ' after every 2 groups
  * HEXDUMP_4_GRP_SPACES:   insert a ' ' after every 4 groups
  * HEXDUMP_8_GRP_SPACES:   insert a ' ' after every 8 groups
+ * HEXDUMP_RETAIN_BYTE_ORDER:  Retain the byte ordering of groups
+ * instead of treating each group as a
+ * native-endian number
  *
  * hex_dump_to_buffer() works on one "line" of output at a time, converting
  *  bytes of input to hexadecimal (and optionally printable ASCII)
@@ -171,6 +175,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
int ret;
int sep_chars = 0;
char sep = 0;
+   bool big_endian = (flags & HEXDUMP_RETAIN_BYTE_ORDER) ? 1 : 0;
 
if (!is_power_of_2(groupsize) || groupsize > 8)
groupsize = 1;
@@ -202,10 +207,13 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
const u64 *ptr8 = buf;
 
for (j = 0; j < ngroups; j++) {
+   u64 val = big_endian ?
+   be64_to_cpu(get_unaligned(ptr8 + j)) :
+   get_unaligned(ptr8 + j);
ret = snprintf(linebuf + lx, linebuflen - lx,
   "%s%16.16llx",
   j ? group_separator(j, flags) : "",
-  get_unaligned(ptr8 + j));
+  val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -214,10 +222,14 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
const u32 *ptr4 = buf;
 
for (j = 0; j < ngroups; j++) {
+   u32 val = big_endian ?
+   be32_to_cpu(get_unaligned(ptr4 + j)) :
+   get_unaligned(ptr4 + j);
+
ret = snprintf(linebuf + lx, linebuflen - lx,
   "%s%8.8x",
   j ? group_separator(j, flags) : "",
-  get_unaligned(ptr4 + j));
+  val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -226,10 +238,14 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
const u16 *ptr2 = buf;
 
for (j = 0; j < ngroups; j++) {
+   u16 val = big_endian ?
+   be16_to_cpu(get_unaligned(ptr2 + j)) :
+   get_unaligned(ptr2 + j);
+
ret = snprintf(linebuf + lx, linebuflen - lx,
   "%s%4.

[PATCH v2 6/7] lib/hexdump.c: Allow multiple groups to be separated by spaces

[Alastair D'Silva]

From: Alastair D'Silva 

Similar to the previous patch, this patch separates groups by 2 spaces for
the hex fields, and 1 space for the ASCII field.

eg.
buf:: 454d414e 43415053  4e495f45 00584544  NAMESPAC E_INDEX.
buf:0010:  0002      

Signed-off-by: Alastair D'Silva 
---
 include/linux/printk.h |  3 ++
 lib/hexdump.c  | 65 +++---
 2 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index dc693aec394c..5231a14e4593 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -485,6 +485,9 @@ enum {
 #define HEXDUMP_2_GRP_LINES(1 << 2)
 #define HEXDUMP_4_GRP_LINES(1 << 3)
 #define HEXDUMP_8_GRP_LINES(1 << 4)
+#define HEXDUMP_2_GRP_SPACES   (1 << 5)
+#define HEXDUMP_4_GRP_SPACES   (1 << 6)
+#define HEXDUMP_8_GRP_SPACES   (1 << 7)
 
 extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
  int groupsize, char *linebuf, size_t linebuflen,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 6f4d1176c332..febd614406d1 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -91,9 +91,37 @@ static const char *group_separator(int group, u64 flags)
if ((flags & HEXDUMP_2_GRP_LINES) && !((group) % 2))
return "|";
 
+   if ((flags & HEXDUMP_8_GRP_SPACES) && !((group) % 8))
+   return "  ";
+
+   if ((flags & HEXDUMP_4_GRP_SPACES) && !((group) % 4))
+   return "  ";
+
+   if ((flags & HEXDUMP_2_GRP_SPACES) && !((group) % 2))
+   return "  ";
+
return " ";
 }
 
+static void separator_parameters(u64 flags, int groupsize, int *sep_chars,
+char *sep)
+{
+   if (flags & (HEXDUMP_2_GRP_LINES | HEXDUMP_2_GRP_SPACES))
+   *sep_chars = groupsize * 2;
+   if (flags & (HEXDUMP_4_GRP_LINES | HEXDUMP_4_GRP_SPACES))
+   *sep_chars = groupsize * 4;
+   if (flags & (HEXDUMP_8_GRP_LINES | HEXDUMP_8_GRP_SPACES))
+   *sep_chars = groupsize * 8;
+
+   if (flags & (HEXDUMP_2_GRP_LINES | HEXDUMP_4_GRP_LINES |
+  HEXDUMP_8_GRP_LINES))
+   *sep = '|';
+
+   if (flags & (HEXDUMP_2_GRP_SPACES | HEXDUMP_4_GRP_SPACES |
+  HEXDUMP_8_GRP_SPACES))
+   *sep = ' ';
+}
+
 /**
  * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
  * @buf: data blob to dump
@@ -107,6 +135,9 @@ static const char *group_separator(int group, u64 flags)
  * HEXDUMP_2_GRP_LINES:insert a '|' after every 2 groups
  * HEXDUMP_4_GRP_LINES:insert a '|' after every 4 groups
  * HEXDUMP_8_GRP_LINES:insert a '|' after every 8 groups
+ * HEXDUMP_2_GRP_SPACES:   insert a ' ' after every 2 groups
+ * HEXDUMP_4_GRP_SPACES:   insert a ' ' after every 4 groups
+ * HEXDUMP_8_GRP_SPACES:   insert a ' ' after every 8 groups
  *
  * hex_dump_to_buffer() works on one "line" of output at a time, converting
  *  bytes of input to hexadecimal (and optionally printable ASCII)
@@ -138,7 +169,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
int j, lx = 0;
int ascii_column;
int ret;
-   int line_chars = 0;
+   int sep_chars = 0;
+   char sep = 0;
 
if (!is_power_of_2(groupsize) || groupsize > 8)
groupsize = 1;
@@ -152,8 +184,14 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
len = rowsize;
 
ngroups = len / groupsize;
+
ascii_column = rowsize * 2 + rowsize / groupsize + 1;
 
+   // space separators use 2 spaces in the hex output
+   separator_parameters(flags, groupsize, &sep_chars, &sep);
+   if (sep == ' ')
+   ascii_column += rowsize / sep_chars;
+
if (!linebuflen)
goto overflow1;
 
@@ -221,24 +259,17 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
rowsize, int groupsize,
linebuf[lx++] = ' ';
}
 
-   if (flags & HEXDUMP_2_GRP_LINES)
-   line_chars = groupsize * 2;
-   if (flags & HEXDUMP_4_GRP_LINES)
-   line_chars = groupsize * 4;
-   if (flags & HEXDUMP_8_GRP_LINES)
-   line_chars = groupsize * 8;
-
for (j = 0; j < len; j++) {
if (linebuflen < lx + 2)
goto overflow2;
ch = ptr[j];
linebuf[lx++] = (isascii(ch) && isprint(ch)) ? ch : '.';
 
-   if (line_chars && ((j + 1) < len) &&
-   ((j + 1) % line_chars == 0)) {
+   if (sep_chars && ((j + 1) < len) &&
+   ((j + 1) % sep_chars == 0)) {
if (lineb

Re: [PATCH v2 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags

[Jani Nikula]

On Wed, 08 May 2019, Alastair D'Silva  wrote:
> From: Alastair D'Silva 
>
> In order to support additional features in hex_dump_to_buffer, replace
> the ascii bool parameter with flags.
>
> Signed-off-by: Alastair D'Silva 
> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c|  2 +-

For i915,

Acked-by: Jani Nikula 

>  drivers/isdn/hardware/mISDN/mISDNisar.c   |  6 --
>  drivers/mailbox/mailbox-test.c|  2 +-
>  drivers/net/ethernet/amd/xgbe/xgbe-drv.c  |  2 +-
>  drivers/net/ethernet/synopsys/dwc-xlgmac-common.c |  2 +-
>  drivers/net/wireless/ath/ath10k/debug.c   |  3 ++-
>  drivers/net/wireless/intel/iwlegacy/3945-mac.c|  2 +-
>  drivers/platform/chrome/wilco_ec/debugfs.c|  2 +-
>  drivers/scsi/scsi_logging.c   |  8 +++-
>  drivers/staging/fbtft/fbtft-core.c|  2 +-
>  fs/seq_file.c |  3 ++-
>  include/linux/printk.h|  8 
>  lib/hexdump.c | 15 ---
>  lib/test_hexdump.c|  5 +++--
>  14 files changed, 33 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
> b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 49fa43ff02ba..fb133e729f9a 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m, const void 
> *buf, size_t len)
>   WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos,
>   rowsize, sizeof(u32),
>   line, sizeof(line),
> - false) >= sizeof(line));
> + 0) >= sizeof(line));
>   drm_printf(m, "[%04zx] %s\n", pos, line);
>  
>   prev = buf + pos;
> diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c 
> b/drivers/isdn/hardware/mISDN/mISDNisar.c
> index 386731ec2489..f13f34db6c17 100644
> --- a/drivers/isdn/hardware/mISDN/mISDNisar.c
> +++ b/drivers/isdn/hardware/mISDN/mISDNisar.c
> @@ -84,7 +84,8 @@ send_mbox(struct isar_hw *isar, u8 his, u8 creg, u8 len, u8 
> *msg)
>  
>   while (l < (int)len) {
>   hex_dump_to_buffer(msg + l, len - l, 32, 1,
> -isar->log, 256, 1);
> +isar->log, 256,
> +HEXDUMP_ASCII);
>   pr_debug("%s: %s %02x: %s\n", isar->name,
>__func__, l, isar->log);
>   l += 32;
> @@ -113,7 +114,8 @@ rcv_mbox(struct isar_hw *isar, u8 *msg)
>  
>   while (l < (int)isar->clsb) {
>   hex_dump_to_buffer(msg + l, isar->clsb - l, 32,
> -1, isar->log, 256, 1);
> +1, isar->log, 256,
> +HEXDUMP_ASCII);
>   pr_debug("%s: %s %02x: %s\n", isar->name,
>__func__, l, isar->log);
>   l += 32;
> diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
> index 4e4ac4be6423..2f9a094d0259 100644
> --- a/drivers/mailbox/mailbox-test.c
> +++ b/drivers/mailbox/mailbox-test.c
> @@ -213,7 +213,7 @@ static ssize_t mbox_test_message_read(struct file *filp, 
> char __user *userbuf,
>   hex_dump_to_buffer(ptr,
>  MBOX_BYTES_PER_LINE,
>  MBOX_BYTES_PER_LINE, 1, touser + l,
> -MBOX_HEXDUMP_LINE_LEN, true);
> +MBOX_HEXDUMP_LINE_LEN, HEXDUMP_ASCII);
>  
>   ptr += MBOX_BYTES_PER_LINE;
>   l += MBOX_HEXDUMP_LINE_LEN;
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c 
> b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> index 0cc911f928b1..e954a31cee0c 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> @@ -2992,7 +2992,7 @@ void xgbe_print_pkt(struct net_device *netdev, struct 
> sk_buff *skb, bool tx_rx)
>   unsigned int len = min(skb->len - i, 32U);
>  
>   hex_dump_to_buffer(&skb->data[i], len, 32, 1,
> -buffer, sizeof(buffer), false);
> +buffer, sizeof(buffer), 0);
>   netdev_dbg(netdev, "  %#06x: %s\n", i, buffer);
>   }
>  
> diff --git a/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c 
> b/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c
> index eb1c6b03c329..b80adfa1f890 100644
> --- a/drivers/net/ethernet/syno

Re: [PATCH v2 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags

[Greg Kroah-Hartman]

On Wed, May 08, 2019 at 05:01:44PM +1000, Alastair D'Silva wrote:
> From: Alastair D'Silva 
> 
> In order to support additional features in hex_dump_to_buffer, replace
> the ascii bool parameter with flags.
> 
> Signed-off-by: Alastair D'Silva 
> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c|  2 +-
>  drivers/isdn/hardware/mISDN/mISDNisar.c   |  6 --
>  drivers/mailbox/mailbox-test.c|  2 +-
>  drivers/net/ethernet/amd/xgbe/xgbe-drv.c  |  2 +-
>  drivers/net/ethernet/synopsys/dwc-xlgmac-common.c |  2 +-
>  drivers/net/wireless/ath/ath10k/debug.c   |  3 ++-
>  drivers/net/wireless/intel/iwlegacy/3945-mac.c|  2 +-
>  drivers/platform/chrome/wilco_ec/debugfs.c|  2 +-
>  drivers/scsi/scsi_logging.c   |  8 +++-
>  drivers/staging/fbtft/fbtft-core.c|  2 +-
>  fs/seq_file.c |  3 ++-
>  include/linux/printk.h|  8 
>  lib/hexdump.c | 15 ---
>  lib/test_hexdump.c|  5 +++--
>  14 files changed, 33 insertions(+), 29 deletions(-)

For staging stuff:

Reviewed-by: Greg Kroah-Hartman 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v2 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags

[David Laight]

From: Alastair D'Silva
> Sent: 08 May 2019 08:02
> To: alast...@d-silva.org
...
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -480,13 +480,13 @@ enum {
>   DUMP_PREFIX_OFFSET
>  };
> 
> -extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
> -   int groupsize, char *linebuf, size_t linebuflen,
> -   bool ascii);
> -
>  #define HEXDUMP_ASCII(1 << 0)
>  #define HEXDUMP_SUPPRESS_REPEATED(1 << 1)

These ought to be BIT(0) and BIT(1)

> +extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
> +   int groupsize, char *linebuf, size_t linebuflen,
> +   u64 flags);

Why 'u64 flags' ?
How many flags do you envisage ??
Your HEXDUMP_ASCII (etc) flags are currently signed values and might
get sign extended causing grief.
'unsigned int flags' is probably sufficient.

I've not really looked at the code, it seems OTT in places though.

If someone copies it somewhere where the performance matters
(I've user space code which is dominated by its tracing!)
then you don't want all the function calls and conditionals
even if you want some of the functionality.

David

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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/4] staging: iio: ad7150: use FIELD_GET and GENMASK

[Ardelean, Alexandru]

On Tue, 2019-05-07 at 17:44 -0300, Melissa Wen wrote:
> [External]
> 
> 
> On 05/06, Ardelean, Alexandru wrote:
> > On Sat, 2019-05-04 at 13:43 +0300, Alexandru Ardelean wrote:
> > > [External]
> > > 
> > > 
> > > On Sat, May 4, 2019 at 1:25 AM Melissa Wen 
> > > wrote:
> > > > 
> > > > Use the bitfield macro FIELD_GET, and GENMASK to do the shift and
> > > > mask
> > > > in
> > > > one go. This makes the code more readable than explicit masking
> > > > followed
> > > > by a shift.
> > > > 
> > > 
> > > This looks neat.
> > > I'd have to remember to ack it from my work email.
> > 
> > Acked-by: Alexandru Ardelean 
> > 
> > > 
> > > One minor comment inline, which would be the object of a new patch.
> > > 
> > > > Signed-off-by: Melissa Wen 
> > > > ---
> > > >  drivers/staging/iio/cdc/ad7150.c | 6 +-
> > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/staging/iio/cdc/ad7150.c
> > > > b/drivers/staging/iio/cdc/ad7150.c
> > > > index 24601ba7db88..4ba46fb6ac02 100644
> > > > --- a/drivers/staging/iio/cdc/ad7150.c
> > > > +++ b/drivers/staging/iio/cdc/ad7150.c
> > > > @@ -5,6 +5,7 @@
> > > >   * Copyright 2010-2011 Analog Devices Inc.
> > > >   */
> > > > 
> > > > +#include 
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > @@ -44,6 +45,9 @@
> > > >  #define AD7150_SN0_REG 0x16
> > > >  #define AD7150_ID_REG  0x17
> > > > 
> > > > +/* AD7150 masks */
> > > > +#define AD7150_THRESHTYPE_MSK  GENMASK(6, 5)
> > > > +
> > > >  /**
> > > >   * struct ad7150_chip_info - instance specific chip data
> > > >   * @client: i2c client for this device
> > > > @@ -136,7 +140,7 @@ static int ad7150_read_event_config(struct
> > > > iio_dev
> > > > *indio_dev,
> > > > if (ret < 0)
> > > > return ret;
> > > > 
> > > > -   threshtype = (ret >> 5) & 0x03;
> > > > +   threshtype = FIELD_GET(AD7150_THRESHTYPE_MSK, ret);
> > > > adaptive = !!(ret & 0x80);
> > > 
> > > Why not also do something similar for the `adaptive` value ?
> 
> Hi Alexandru,
> 
> Yes, I'm working on it!  However, taking a look at the driver datasheet
> (Table
> 13, page 19), there seems to be a little mistake in choosing the variable
> name
> and its meaning,  since when bit(7) is 1 (true) the threshold is fixed,
> and not
> adaptive. For this reason, I removed it from this patchset to mature the
> solution. I will send it as a bug fix if I prove it's necessary.
> Do you have any advice or feeling about it to share?
> 

Good find.
Since this is a bug-fix, typically it's good to fix the code as-is now
[even if it isn't neat code], and then do the conversions to neat code.

Bug-fixes always take priority over cosmetic changes.
So, I would send the bug-fix as-soon-as-possible, and then update things.


> P.s.: Sorry for having previously sent an email with HTML.
> 
> Melissa
> 
> > > 
> > > > 
> > > > switch (type) {
> > > > --
> > > > 2.20.1
> > > > 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [GIT PULL] Staging/IIO driver patches for 5.2-rc1

[Greg KH]

On Tue, May 07, 2019 at 01:33:06PM -0700, Linus Torvalds wrote:
> On Tue, May 7, 2019 at 10:59 AM Greg KH  wrote:
> >
> > All of these have been in linux-next for a while with no reported
> > issues, other than an odd gcc warning for one of the new drivers that
> > should be fixed up soon.
> 
> Ok, that's truly a funky warning.
> 
> But since I don't deal well with warnings, particularly during the
> merge window, I just fixed it up in the merge.
> 
> The fix is to simply not have a bitfield base type of "unsigned char",
> and have it cross a char boundary. Instead the base type should just
> be "unsigned int".

Ah, that's how to resolve that, thanks, it wasn't obvious at all from
the odd gcc warning.

> Of course, I couldn't test my change, but it shuts the compiler up,
> and it very much looks like the right thing.

The driver author can test it out, given that it needs loads of work
anyway, whatever you did to it is fine :)

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 04/16] powerpc/xmon: use new match_string() helper/macro

[Alexandru Ardelean]

The change is purely cosmetic at this point in time, but it does highlight
the change done in lib/string.c for match_string().

Particularly for this change, if a regname is removed (replaced with NULL)
in the list, the match_string() helper will continue until the end of the
array and ignore the NULL.
This would technically allow for "reserved" regs, though here it's not the
case.

Signed-off-by: Alexandru Ardelean 
---
 arch/powerpc/xmon/xmon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index efca104ac0cb..b84a7fc1112b 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3231,7 +3231,7 @@ scanhex(unsigned long *vp)
regname[i] = c;
}
regname[i] = 0;
-   i = __match_string(regnames, N_PTREGS, regname);
+   i = match_string(regnames, regname);
if (i < 0) {
printf("invalid register name '%%%s'\n", regname);
return 0;
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 02/16] treewide: rename match_string() -> __match_string()

[Alexandru Ardelean]

This change does a rename of match_string() -> __match_string().

There are a few parts to the intention here (with this change):
1. Align with sysfs_match_string()/__sysfs_match_string()
2. This helps to group users of `match_string()` into simple users:
   a. those that use ARRAY_SIZE(_a) to specify the number of elements
   b. those that use -1 to pass a NULL terminated array of strings
   c. special users, which (after eliminating 1 & 2) are not that many
3. The final intent is to fix match_string()/__match_string() which is
   slightly broken, in the sense that passing -1 or a positive value does
   not make any difference: the iteration will stop at the first NULL
   element.

Signed-off-by: Alexandru Ardelean 
---
 arch/powerpc/xmon/xmon.c |  2 +-
 arch/x86/kernel/cpu/mtrr/if.c|  2 +-
 drivers/ata/pata_hpt366.c|  2 +-
 drivers/ata/pata_hpt37x.c|  2 +-
 drivers/base/devcon.c|  2 +-
 drivers/base/property.c  |  2 +-
 drivers/clk/bcm/clk-bcm2835.c|  6 +++---
 drivers/clk/clk.c|  4 ++--
 drivers/clk/rockchip/clk.c   |  4 ++--
 drivers/cpufreq/intel_pstate.c   |  2 +-
 drivers/gpio/gpiolib-of.c|  2 +-
 drivers/gpu/drm/drm_edid_load.c  |  2 +-
 drivers/gpu/drm/drm_panel_orientation_quirks.c   |  2 +-
 drivers/gpu/drm/i915/intel_pipe_crc.c|  2 +-
 drivers/ide/hpt366.c |  2 +-
 drivers/mfd/omap-usb-host.c  |  2 +-
 drivers/mmc/host/sdhci-xenon-phy.c   |  2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c |  2 +-
 drivers/pci/pcie/aer.c   |  2 +-
 drivers/phy/tegra/xusb.c |  2 +-
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c  |  4 ++--
 drivers/pinctrl/pinmux.c |  2 +-
 drivers/power/supply/ab8500_btemp.c  |  2 +-
 drivers/power/supply/ab8500_charger.c|  2 +-
 drivers/power/supply/ab8500_fg.c |  2 +-
 drivers/power/supply/abx500_chargalg.c   |  2 +-
 drivers/power/supply/charger-manager.c   |  4 ++--
 drivers/staging/gdm724x/gdm_tty.c|  4 ++--
 drivers/usb/common/common.c  |  4 ++--
 drivers/usb/typec/class.c| 10 +-
 drivers/usb/typec/tps6598x.c |  2 +-
 drivers/vfio/vfio.c  |  6 +++---
 drivers/video/fbdev/pxafb.c  |  2 +-
 fs/ubifs/auth.c  |  4 ++--
 include/linux/string.h   |  2 +-
 kernel/cgroup/rdma.c |  2 +-
 kernel/sched/debug.c |  2 +-
 kernel/trace/trace.c |  2 +-
 lib/string.c |  8 
 mm/mempolicy.c   |  2 +-
 mm/vmpressure.c  |  4 ++--
 security/apparmor/lsm.c  |  4 ++--
 security/integrity/ima/ima_main.c|  2 +-
 sound/firewire/oxfw/oxfw.c   |  2 +-
 sound/pci/oxygen/oxygen_mixer.c  |  2 +-
 sound/soc/codecs/max98088.c  |  2 +-
 sound/soc/codecs/max98095.c  |  2 +-
 sound/soc/soc-dapm.c |  2 +-
 48 files changed, 68 insertions(+), 68 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a0f44f992360..efca104ac0cb 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3231,7 +3231,7 @@ scanhex(unsigned long *vp)
regname[i] = c;
}
regname[i] = 0;
-   i = match_string(regnames, N_PTREGS, regname);
+   i = __match_string(regnames, N_PTREGS, regname);
if (i < 0) {
printf("invalid register name '%%%s'\n", regname);
return 0;
diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
index 4d36dcc1cf87..4ec7a5f7b94c 100644
--- a/arch/x86/kernel/cpu/mtrr/if.c
+++ b/arch/x86/kernel/cpu/mtrr/if.c
@@ -142,7 +142,7 @@ mtrr_write(struct file *file, const char __user *buf, 
size_t len, loff_t * ppos)
return -EINVAL;
ptr = skip_spaces(ptr + 5);
 
-   i = match_string(mtrr_strings, MTRR_NUM_TYPES, ptr);
+   i = __match_string(mtrr_strings, MTRR_NUM_TYPES, ptr);
if (i < 0)
return i;
 
diff --git a/drivers/ata/pata_hpt366.c b/drivers/ata/pata_hpt366.c
index a219a503c229..4ba5fc9d20be 100644
--- a/drivers/ata/pata_hpt366.c
+++ b/drivers/ata/pata_hpt366.c
@@ -180,7 +180,7 @@ static int hpt_dma_blacklisted(const struct ata_device 
*dev, char *modestr,
 
ata_id_c_s

[PATCH 07/16] device connection: use new match_string() helper/macro

[Alexandru Ardelean]

The `device_connection` struct is defined as:
struct device_connection {
struct fwnode_handle*fwnode;
const char  *endpoint[2];
const char  *id;
struct list_headlist;
};

The `endpoint` member is a static array of strings (on the struct), so
using the match_string() (which does an ARRAY_SIZE((con->endpoint)) should
be fine.

The recent change to match_string() (to ignore NULL entries up to the size
of the array) shouldn't affect this.

Signed-off-by: Alexandru Ardelean 
---
 drivers/base/devcon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index 7bc1c619b721..4a2338665585 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -70,7 +70,7 @@ void *device_connection_find_match(struct device *dev, const 
char *con_id,
mutex_lock(&devcon_lock);
 
list_for_each_entry(con, &devcon_list, list) {
-   ep = __match_string(con->endpoint, 2, devname);
+   ep = match_string(con->endpoint, devname);
if (ep < 0)
continue;
 
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 05/16] ALSA: oxygen: use new match_string() helper/macro

[Alexandru Ardelean]

The change is purely cosmetic at this point in time, but it does highlight
the change done in lib/string.c for match_string().

Particularly for this change, a control mode can be removed/added at a
different index/enum-value, and the match_string() helper will continue
until the end of the array and ignore the NULL.

Signed-off-by: Alexandru Ardelean 
---
 sound/pci/oxygen/oxygen_mixer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/pci/oxygen/oxygen_mixer.c b/sound/pci/oxygen/oxygen_mixer.c
index 13c2fb75fd71..961fd1cbc712 100644
--- a/sound/pci/oxygen/oxygen_mixer.c
+++ b/sound/pci/oxygen/oxygen_mixer.c
@@ -1086,7 +1086,7 @@ static int add_controls(struct oxygen *chip,
err = snd_ctl_add(chip->card, ctl);
if (err < 0)
return err;
-   j = __match_string(known_ctl_names, CONTROL_COUNT, 
ctl->id.name);
+   j = match_string(known_ctl_names, ctl->id.name);
if (j >= 0) {
chip->controls[j] = ctl;
ctl->private_free = oxygen_any_ctl_free;
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 00/16] treewide: fix match_string() helper when array size

[Alexandru Ardelean]

The intent of this patch series is to make a case for fixing the
match_string() string helper.

The doc-string of the `__sysfs_match_string()` helper mentions that `n`
(the size of the given array) should be:
 * @n: number of strings in the array or -1 for NULL terminated arrays

However, this is not the case.
The helper stops on the first NULL in the array, regardless of whether -1
is provided or not.

There are some advantages to allowing this behavior (NULL elements within
in the array). One example, is to allow reserved registers as NULL in an
array.
One example in the series is patch:
   x86/mtrr: use new match_string() helper + add gaps == minor fix
which uses a "?" string for values that are reserved/don't care.

Since the change is a bit big, the change was coupled with renaming
match_string() -> __match_string().
The new match_string() helper (resulted here) does an ARRAY_SIZE() over the
array, which is useful when the array is static. 

Also, this way of doing things is a way to go through all the users of this
helpers and check that nothing goes wrong, and notify them about the change
to match_string().
It's a way of grouping changes in a manage-able way.

The first patch is important, the others can be dropped.

Signed-off-by: Alexandru Ardelean 

Alexandru Ardelean (16):
  lib: fix match_string() helper when array size is positive
  treewide: rename match_string() -> __match_string()
  lib,treewide: add new match_string() helper/macro
  powerpc/xmon: use new match_string() helper/macro
  ALSA: oxygen: use new match_string() helper/macro
  x86/mtrr: use new match_string() helper + add gaps == minor fix
  device connection: use new match_string() helper/macro
  cpufreq/intel_pstate: remove NULL entry + use match_string()
  mmc: sdhci-xenon: use new match_string() helper/macro
  pinctrl: armada-37xx: use new match_string() helper/macro
  mm/vmpressure.c: use new match_string() helper/macro
  rdmacg: use new match_string() helper/macro
  drm/edid: use new match_string() helper/macro
  staging: gdm724x: use new match_string() helper/macro
  video: fbdev: pxafb: use new match_string() helper/macro
  sched: debug: use new match_string() helper/macro

 arch/powerpc/xmon/xmon.c |  2 +-
 arch/x86/kernel/cpu/mtrr/if.c| 10 ++
 drivers/ata/pata_hpt366.c|  2 +-
 drivers/ata/pata_hpt37x.c|  2 +-
 drivers/base/devcon.c|  2 +-
 drivers/base/property.c  |  2 +-
 drivers/clk/bcm/clk-bcm2835.c|  4 +---
 drivers/clk/clk.c|  4 ++--
 drivers/clk/rockchip/clk.c   |  4 ++--
 drivers/cpufreq/intel_pstate.c   |  9 -
 drivers/gpio/gpiolib-of.c|  2 +-
 drivers/gpu/drm/drm_edid_load.c  |  2 +-
 drivers/gpu/drm/drm_panel_orientation_quirks.c   |  2 +-
 drivers/gpu/drm/i915/intel_pipe_crc.c|  2 +-
 drivers/ide/hpt366.c |  2 +-
 drivers/mfd/omap-usb-host.c  |  2 +-
 drivers/mmc/host/sdhci-xenon-phy.c   | 12 ++--
 drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c |  2 +-
 drivers/pci/pcie/aer.c   |  2 +-
 drivers/phy/tegra/xusb.c |  2 +-
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c  |  4 ++--
 drivers/pinctrl/pinmux.c |  2 +-
 drivers/power/supply/ab8500_btemp.c  |  2 +-
 drivers/power/supply/ab8500_charger.c|  2 +-
 drivers/power/supply/ab8500_fg.c |  2 +-
 drivers/power/supply/abx500_chargalg.c   |  2 +-
 drivers/power/supply/charger-manager.c   |  4 ++--
 drivers/staging/gdm724x/gdm_tty.c|  3 +--
 drivers/usb/common/common.c  |  4 ++--
 drivers/usb/typec/class.c|  8 +++-
 drivers/usb/typec/tps6598x.c |  2 +-
 drivers/vfio/vfio.c  |  4 +---
 drivers/video/fbdev/pxafb.c  |  4 ++--
 fs/ubifs/auth.c  |  4 ++--
 include/linux/string.h   | 11 ++-
 kernel/cgroup/rdma.c |  2 +-
 kernel/sched/debug.c |  2 +-
 kernel/trace/trace.c |  2 +-
 lib/string.c | 13 -
 mm/mempolicy.c   |  2 +-
 mm/vmpressure.c  |  4 ++--
 security/apparmor/lsm.c  |  4 ++--
 security/integrity/ima/ima_main.c|  2 +-
 sound/firewire/oxfw/oxfw.c   |  2 +-
 sound/pci/oxygen/oxygen_mixer.c  |  2 +-
 sound/soc/codecs/max98088.c  |  2 +-
 sound/soc/codecs/max98095.c  |  2 +-
 sou

[PATCH 03/16] lib,treewide: add new match_string() helper/macro

[Alexandru Ardelean]

This change re-introduces `match_string()` as a macro that uses
ARRAY_SIZE() to compute the size of the array.
The macro is added in all the places that do
`match_string(_a, ARRAY_SIZE(_a), s)`, since the change is pretty
straightforward.

Signed-off-by: Alexandru Ardelean 
---
 drivers/clk/bcm/clk-bcm2835.c| 4 +---
 drivers/gpio/gpiolib-of.c| 2 +-
 drivers/gpu/drm/i915/intel_pipe_crc.c| 2 +-
 drivers/mfd/omap-usb-host.c  | 2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 2 +-
 drivers/pci/pcie/aer.c   | 2 +-
 drivers/usb/common/common.c  | 4 ++--
 drivers/usb/typec/class.c| 8 +++-
 drivers/usb/typec/tps6598x.c | 2 +-
 drivers/vfio/vfio.c  | 4 +---
 include/linux/string.h   | 9 +
 sound/firewire/oxfw/oxfw.c   | 2 +-
 sound/soc/codecs/max98088.c  | 2 +-
 sound/soc/codecs/max98095.c  | 2 +-
 14 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index a775f6a1f717..1ab388590ead 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -1390,9 +1390,7 @@ static struct clk_hw *bcm2835_register_clock(struct 
bcm2835_cprman *cprman,
for (i = 0; i < data->num_mux_parents; i++) {
parents[i] = data->parents[i];
 
-   ret = __match_string(cprman_parent_names,
-ARRAY_SIZE(cprman_parent_names),
-parents[i]);
+   ret = match_string(cprman_parent_names, parents[i]);
if (ret >= 0)
parents[i] = cprman->real_parent_names[ret];
}
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 27d6f04ab58e..71e886869d78 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -279,7 +279,7 @@ static struct gpio_desc *of_find_regulator_gpio(struct 
device *dev, const char *
if (!con_id)
return ERR_PTR(-ENOENT);
 
-   i = __match_string(whitelist, ARRAY_SIZE(whitelist), con_id);
+   i = match_string(whitelist, con_id);
if (i < 0)
return ERR_PTR(-ENOENT);
 
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c 
b/drivers/gpu/drm/i915/intel_pipe_crc.c
index 286fad1f0e08..6fc4f3d3d1f6 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -449,7 +449,7 @@ display_crc_ctl_parse_source(const char *buf, enum 
intel_pipe_crc_source *s)
return 0;
}
 
-   i = __match_string(pipe_crc_sources, ARRAY_SIZE(pipe_crc_sources), buf);
+   i = match_string(pipe_crc_sources, buf);
if (i < 0)
return i;
 
diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 9aaacb5bdb26..53dff34c0afc 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -509,7 +509,7 @@ static int usbhs_omap_get_dt_pdata(struct device *dev,
continue;
 
/* get 'enum usbhs_omap_port_mode' from port mode string */
-   ret = __match_string(port_modes, ARRAY_SIZE(port_modes), mode);
+   ret = match_string(port_modes, mode);
if (ret < 0) {
dev_warn(dev, "Invalid port%d-mode \"%s\" in device 
tree\n",
i, mode);
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c 
b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
index 59ce3ff35553..778b4dfd8b75 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
@@ -667,7 +667,7 @@ iwl_dbgfs_bt_force_ant_write(struct iwl_mvm *mvm, char *buf,
};
int ret, bt_force_ant_mode;
 
-   ret = __match_string(modes_str, ARRAY_SIZE(modes_str), buf);
+   ret = match_string(modes_str, buf);
if (ret < 0)
return ret;
 
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 41a0773a1cbc..2278caba109c 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -203,7 +203,7 @@ void pcie_ecrc_get_policy(char *str)
 {
int i;
 
-   i = __match_string(ecrc_policy_str, ARRAY_SIZE(ecrc_policy_str), str);
+   i = match_string(ecrc_policy_str, str);
if (i < 0)
return;
 
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index bca0c404c6ca..5a651d311d38 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -68,7 +68,7 @@ enum usb_device_speed usb_get_maximum_speed(struct device 
*dev)
if (ret < 0)
return USB_SPEED_UNKNOWN;
 
-   ret = __match_string(speed_names, ARRAY_SIZE(speed_names), 
maximum_speed);
+   ret =

[PATCH 06/16] x86/mtrr: use new match_string() helper + add gaps == minor fix

[Alexandru Ardelean]

This change is a bit more than cosmetic.

It replaces 2 values in mtrr_strings with NULL. Previously, they were
defined as "?", which is not great because you could technically pass "?",
and you would get value 2.
It's not sure whether that was intended (likely it wasn't), but this fixes
that.

Signed-off-by: Alexandru Ardelean 
---
 arch/x86/kernel/cpu/mtrr/if.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
index 4ec7a5f7b94c..e67820a044cc 100644
--- a/arch/x86/kernel/cpu/mtrr/if.c
+++ b/arch/x86/kernel/cpu/mtrr/if.c
@@ -20,8 +20,8 @@ static const char *const mtrr_strings[MTRR_NUM_TYPES] =
 {
"uncachable",   /* 0 */
"write-combining",  /* 1 */
-   "?",/* 2 */
-   "?",/* 3 */
+   NULL,   /* 2 */
+   NULL,   /* 3 */
"write-through",/* 4 */
"write-protect",/* 5 */
"write-back",   /* 6 */
@@ -29,7 +29,9 @@ static const char *const mtrr_strings[MTRR_NUM_TYPES] =
 
 const char *mtrr_attrib_to_str(int x)
 {
-   return (x <= 6) ? mtrr_strings[x] : "?";
+   if ((x >= ARRAY_SIZE(mtrr_strings)) || (mtrr_strings[x] == NULL))
+   return "?";
+   return mtrr_strings[x];
 }
 
 #ifdef CONFIG_PROC_FS
@@ -142,7 +144,7 @@ mtrr_write(struct file *file, const char __user *buf, 
size_t len, loff_t * ppos)
return -EINVAL;
ptr = skip_spaces(ptr + 5);
 
-   i = __match_string(mtrr_strings, MTRR_NUM_TYPES, ptr);
+   i = match_string(mtrr_strings, ptr);
if (i < 0)
return i;
 
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 09/16] mmc: sdhci-xenon: use new match_string() helper/macro

[Alexandru Ardelean]

The change is also cosmetic, but it also does a tighter coupling between
the enums & the string values. This way, the ARRAY_SIZE(phy_types) that is
implicitly done in the match_string() macro is also a bit safer.

Signed-off-by: Alexandru Ardelean 
---
 drivers/mmc/host/sdhci-xenon-phy.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci-xenon-phy.c 
b/drivers/mmc/host/sdhci-xenon-phy.c
index 59b7a6cac995..2a9206867fe1 100644
--- a/drivers/mmc/host/sdhci-xenon-phy.c
+++ b/drivers/mmc/host/sdhci-xenon-phy.c
@@ -135,17 +135,17 @@ struct xenon_emmc_phy_regs {
u32 logic_timing_val;
 };
 
-static const char * const phy_types[] = {
-   "emmc 5.0 phy",
-   "emmc 5.1 phy"
-};
-
 enum xenon_phy_type_enum {
EMMC_5_0_PHY,
EMMC_5_1_PHY,
NR_PHY_TYPES
 };
 
+static const char * const phy_types[NR_PHY_TYPES] = {
+   [EMMC_5_0_PHY] = "emmc 5.0 phy",
+   [EMMC_5_1_PHY] = "emmc 5.1 phy"
+};
+
 enum soc_pad_ctrl_type {
SOC_PAD_SD,
SOC_PAD_FIXED_1_8V,
@@ -821,7 +821,7 @@ static int xenon_add_phy(struct device_node *np, struct 
sdhci_host *host,
struct xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
int ret;
 
-   priv->phy_type = __match_string(phy_types, NR_PHY_TYPES, phy_name);
+   priv->phy_type = match_string(phy_types, phy_name);
if (priv->phy_type < 0) {
dev_err(mmc_dev(host->mmc),
"Unable to determine PHY name %s. Use default eMMC 5.1 
PHY\n",
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 01/16] lib: fix match_string() helper on -1 array size

[Alexandru Ardelean]

The documentation the `_match_string()` helper mentions that `n`
should be:
 * @n: number of strings in the array or -1 for NULL terminated arrays

The behavior of the function is different, in the sense that it exits on
the first NULL element in the array, regardless of whether `n` is -1 or a
positive number.

This patch changes the behavior, to exit the loop when a NULL element is
found and n == -1. Essentially, this aligns the behavior with the
doc-string.

There are currently many users of `match_string()`, and so, in order to go
through them, the next patches in the series will focus on doing some
cosmetic changes, which are aimed at grouping the users of
`match_string()`.

Signed-off-by: Alexandru Ardelean 
---
 lib/string.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/string.c b/lib/string.c
index 3ab861c1a857..76edb7bf76cb 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -648,8 +648,11 @@ int match_string(const char * const *array, size_t n, 
const char *string)
 
for (index = 0; index < n; index++) {
item = array[index];
-   if (!item)
+   if (!item) {
+   if (n != (size_t)-1)
+   continue;
break;
+   }
if (!strcmp(item, string))
return index;
}
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 08/16] cpufreq/intel_pstate: remove NULL entry + use match_string()

[Alexandru Ardelean]

The change is mostly cosmetic.

The `energy_perf_strings` array is static, so match_string() can be used
(which will implicitly do a ARRAY_SIZE(energy_perf_strings)).

The only small benefit here, is the reduction of the array size by 1
element.

Signed-off-by: Alexandru Ardelean 
---
 drivers/cpufreq/intel_pstate.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 6ed1e705bc05..ab9a0b34b900 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -593,8 +593,7 @@ static const char * const energy_perf_strings[] = {
"performance",
"balance_performance",
"balance_power",
-   "power",
-   NULL
+   "power"
 };
 static const unsigned int epp_values[] = {
HWP_EPP_PERFORMANCE,
@@ -680,8 +679,8 @@ static ssize_t 
show_energy_performance_available_preferences(
int i = 0;
int ret = 0;
 
-   while (energy_perf_strings[i] != NULL)
-   ret += sprintf(&buf[ret], "%s ", energy_perf_strings[i++]);
+   for (; i < ARRAY_SIZE(energy_perf_strings); i++)
+   ret += sprintf(&buf[ret], "%s ", energy_perf_strings[i]);
 
ret += sprintf(&buf[ret], "\n");
 
@@ -701,7 +700,7 @@ static ssize_t store_energy_performance_preference(
if (ret != 1)
return -EINVAL;
 
-   ret = __match_string(energy_perf_strings, -1, str_preference);
+   ret = match_string(energy_perf_strings, str_preference);
if (ret < 0)
return ret;
 
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 01/16] lib: fix match_string() helper when array size is positive

[Alexandru Ardelean]

The documentation the `_match_string()` helper mentions that `n`
(size of the given array) should be:
 * @n: number of strings in the array or -1 for NULL terminated arrays

The behavior of the function is different, in the sense that it exits on
the first NULL element in the array, regardless of whether `n` is -1 or a
positive number.

This patch changes the behavior, to exit the loop when a NULL element is
found and n == -1. Essentially, this aligns the behavior with the
doc-string.

There are currently many users of `match_string()`, and so, in order to go
through them, the next patches in the series will focus on doing some
cosmetic changes, which are aimed at grouping the users of
`match_string()`.

Signed-off-by: Alexandru Ardelean 
---
 lib/string.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/string.c b/lib/string.c
index 3ab861c1a857..76edb7bf76cb 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -648,8 +648,11 @@ int match_string(const char * const *array, size_t n, 
const char *string)
 
for (index = 0; index < n; index++) {
item = array[index];
-   if (!item)
+   if (!item) {
+   if (n != (size_t)-1)
+   continue;
break;
+   }
if (!strcmp(item, string))
return index;
}
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 11/16] mm/vmpressure.c: use new match_string() helper/macro

[Alexandru Ardelean]

__match_string() is called on 2 static array of strings in this file. For
this reason, the conversion to the new match_string() macro/helper, was
done in this separate commit.

Using the new match_string() helper is mostly a cosmetic change (at this
point in time). The sizes of the arrays will be computed automatically,
which would only help if they ever get expanded.

Signed-off-by: Alexandru Ardelean 
---
 mm/vmpressure.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/vmpressure.c b/mm/vmpressure.c
index d43f33139568..b8149924f078 100644
--- a/mm/vmpressure.c
+++ b/mm/vmpressure.c
@@ -378,7 +378,7 @@ int vmpressure_register_event(struct mem_cgroup *memcg,
 
/* Find required level */
token = strsep(&spec, ",");
-   level = __match_string(vmpressure_str_levels, VMPRESSURE_NUM_LEVELS, 
token);
+   level = match_string(vmpressure_str_levels, token);
if (level < 0) {
ret = level;
goto out;
@@ -387,7 +387,7 @@ int vmpressure_register_event(struct mem_cgroup *memcg,
/* Find optional mode */
token = strsep(&spec, ",");
if (token) {
-   mode = __match_string(vmpressure_str_modes, 
VMPRESSURE_NUM_MODES, token);
+   mode = match_string(vmpressure_str_modes, token);
if (mode < 0) {
ret = mode;
goto out;
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 10/16] pinctrl: armada-37xx: use new match_string() helper/macro

[Alexandru Ardelean]

The change is mostly cosmetic.

The `armada_37xx_pin_group` struct is defined as.
struct armada_37xx_pin_group {
const char  *name;
unsigned intstart_pin;
unsigned intnpins;
u32 reg_mask;
u32 val[NB_FUNCS];
unsigned intextra_pin;
unsigned intextra_npins;
const char  *funcs[NB_FUNCS];
unsigned int*pins;
};

The `funcs` field is a static array of strings, so using the
new `match_string()` helper (which does an implicit ARRAY_SIZE(gp->funcs))
should be fine.

Signed-off-by: Alexandru Ardelean 
---
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c 
b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 07a5bcaa0067..68b0db5ef5e9 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -348,7 +348,7 @@ static int armada_37xx_pmx_set_by_name(struct pinctrl_dev 
*pctldev,
dev_dbg(info->dev, "enable function %s group %s\n",
name, grp->name);
 
-   func = __match_string(grp->funcs, NB_FUNCS, name);
+   func = match_string(grp->funcs, name);
if (func < 0)
return -ENOTSUPP;
 
@@ -938,7 +938,7 @@ static int armada_37xx_fill_func(struct armada_37xx_pinctrl 
*info)
struct armada_37xx_pin_group *gp = &info->groups[g];
int f;
 
-   f = __match_string(gp->funcs, NB_FUNCS, name);
+   f = match_string(gp->funcs, name);
if (f < 0)
continue;
 
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 12/16] rdmacg: use new match_string() helper/macro

[Alexandru Ardelean]

The `rdmacg_resource_names` array is a static array of strings.
Using match_string() (which computes the array size via ARRAY_SIZE())
is possible.

The change is mostly cosmetic.
No functionality change.

Signed-off-by: Alexandru Ardelean 
---
 kernel/cgroup/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cgroup/rdma.c b/kernel/cgroup/rdma.c
index 65d4df148603..71c3d305bd1f 100644
--- a/kernel/cgroup/rdma.c
+++ b/kernel/cgroup/rdma.c
@@ -367,7 +367,7 @@ static int parse_resource(char *c, int *intval)
if (!name || !value)
return -EINVAL;
 
-   i = __match_string(rdmacg_resource_names, RDMACG_RESOURCE_MAX, name);
+   i = match_string(rdmacg_resource_names, name);
if (i < 0)
return i;
 
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 15/16] video: fbdev: pxafb: use new match_string() helper/macro

[Alexandru Ardelean]

The `lcd_types` array is a static array of strings.
Using match_string() (which computes the array size via ARRAY_SIZE())
is possible.

This reduces the array by 1 element, since the NULL (at the end of the
array) is no longer needed.

Signed-off-by: Alexandru Ardelean 
---
 drivers/video/fbdev/pxafb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index 0025781e6e1e..e657a04f5b1d 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c
@@ -2114,7 +2114,7 @@ static void pxafb_check_options(struct device *dev, 
struct pxafb_mach_info *inf)
 #if defined(CONFIG_OF)
 static const char * const lcd_types[] = {
"unknown", "mono-stn", "mono-dstn", "color-stn", "color-dstn",
-   "color-tft", "smart-panel", NULL
+   "color-tft", "smart-panel"
 };
 
 static int of_get_pxafb_display(struct device *dev, struct device_node *disp,
@@ -2129,7 +2129,7 @@ static int of_get_pxafb_display(struct device *dev, 
struct device_node *disp,
if (ret)
s = "color-tft";
 
-   i = __match_string(lcd_types, -1, s);
+   i = match_string(lcd_types, s);
if (i < 0) {
dev_err(dev, "lcd-type %s is unknown\n", s);
return i;
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 14/16] staging: gdm724x: use new match_string() helper/macro

[Alexandru Ardelean]

The `DRIVER_STRING` array is a static array of strings.
Using match_string() (which computes the array size via ARRAY_SIZE())
is possible.

The change is mostly cosmetic.
No functionality change.

Signed-off-by: Alexandru Ardelean 
---
 drivers/staging/gdm724x/gdm_tty.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/gdm724x/gdm_tty.c 
b/drivers/staging/gdm724x/gdm_tty.c
index 6e147a324652..81dd6795599f 100644
--- a/drivers/staging/gdm724x/gdm_tty.c
+++ b/drivers/staging/gdm724x/gdm_tty.c
@@ -56,8 +56,7 @@ static int gdm_tty_install(struct tty_driver *driver, struct 
tty_struct *tty)
struct gdm *gdm = NULL;
int ret;
 
-   ret = __match_string(DRIVER_STRING, TTY_MAX_COUNT,
-tty->driver->driver_name);
+   ret = match_string(DRIVER_STRING, tty->driver->driver_name);
if (ret < 0)
return -ENODEV;
 
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 16/16] sched: debug: use new match_string() helper/macro

[Alexandru Ardelean]

The `sched_feat_names` array is a static array of strings.
Using match_string() (which computes the array size via ARRAY_SIZE())
is possible.

The change is mostly cosmetic.
No functionality change.

Signed-off-by: Alexandru Ardelean 
---
 kernel/sched/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index b0efc5fe641e..6d5f370bdfde 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -111,7 +111,7 @@ static int sched_feat_set(char *cmp)
cmp += 3;
}
 
-   i = __match_string(sched_feat_names, __SCHED_FEAT_NR, cmp);
+   i = match_string(sched_feat_names, cmp);
if (i < 0)
return i;
 
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/16] lib: fix match_string() helper on -1 array size

[Ardelean, Alexandru]

On Wed, 2019-05-08 at 14:28 +0300, Alexandru Ardelean wrote:
> The documentation the `_match_string()` helper mentions that `n`
> should be:
>  * @n: number of strings in the array or -1 for NULL terminated arrays
> 
> The behavior of the function is different, in the sense that it exits on
> the first NULL element in the array, regardless of whether `n` is -1 or a
> positive number.
> 
> This patch changes the behavior, to exit the loop when a NULL element is
> found and n == -1. Essentially, this aligns the behavior with the
> doc-string.
> 
> There are currently many users of `match_string()`, and so, in order to
> go
> through them, the next patches in the series will focus on doing some
> cosmetic changes, which are aimed at grouping the users of
> `match_string()`.
> 

This is the duplicate & should be dropped.
Sorry for this.

> Signed-off-by: Alexandru Ardelean 
> ---
>  lib/string.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/string.c b/lib/string.c
> index 3ab861c1a857..76edb7bf76cb 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -648,8 +648,11 @@ int match_string(const char * const *array, size_t
> n, const char *string)
>  
>   for (index = 0; index < n; index++) {
>   item = array[index];
> - if (!item)
> + if (!item) {
> + if (n != (size_t)-1)
> + continue;
>   break;
> + }
>   if (!strcmp(item, string))
>   return index;
>   }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v2 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags

[Alastair D'Silva]

> -Original Message-
> From: David Laight 
> Sent: Wednesday, 8 May 2019 7:20 PM
> To: 'Alastair D'Silva' ; alast...@d-silva.org
> Cc: Jani Nikula ; Joonas Lahtinen
> ; Rodrigo Vivi ;
> David Airlie ; Daniel Vetter ; Dan
> Carpenter ; Karsten Keil  pingi.de>; Jassi Brar ; Tom Lendacky
> ; David S. Miller ;
> Jose Abreu ; Kalle Valo
> ; Stanislaw Gruszka ;
> Benson Leung ; Enric Balletbo i Serra
> ; James E.J. Bottomley
> ; Martin K. Petersen ;
> Greg Kroah-Hartman ; Alexander Viro
> ; Petr Mladek ; Sergey
> Senozhatsky ; Steven Rostedt
> ; Andrew Morton ;
> intel-...@lists.freedesktop.org; dri-de...@lists.freedesktop.org; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org;
> ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux-
> s...@vger.kernel.org; linux-fb...@vger.kernel.org;
> de...@driverdev.osuosl.org; linux-fsde...@vger.kernel.org
> Subject: RE: [PATCH v2 4/7] lib/hexdump.c: Replace ascii bool in
> hex_dump_to_buffer with flags
> 
> From: Alastair D'Silva
> > Sent: 08 May 2019 08:02
> > To: alast...@d-silva.org
> ...
> > --- a/include/linux/printk.h
> > +++ b/include/linux/printk.h
> > @@ -480,13 +480,13 @@ enum {
> > DUMP_PREFIX_OFFSET
> >  };
> >
> > -extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
> > - int groupsize, char *linebuf, size_t linebuflen,
> > - bool ascii);
> > -
> >  #define HEXDUMP_ASCII  (1 << 0)
> >  #define HEXDUMP_SUPPRESS_REPEATED  (1 << 1)
> 
> These ought to be BIT(0) and BIT(1)

Thanks, I'll address that.

> 
> > +extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
> > + int groupsize, char *linebuf, size_t linebuflen,
> > + u64 flags);
> 
> Why 'u64 flags' ?
> How many flags do you envisage ??
> Your HEXDUMP_ASCII (etc) flags are currently signed values and might get
> sign extended causing grief.
> 'unsigned int flags' is probably sufficient.

I was trying to avoid having to change the prototype again in the future, but 
it's not a big deal, if enough work goes in to require more than 32 bits, it 
can be updated at that point.

> 
> I've not really looked at the code, it seems OTT in places though.

I'll wait for more concrete criticisms here, this it a bit too vague to take 
any action on.

> If someone copies it somewhere where the performance matters (I've user
> space code which is dominated by its tracing!) then you don't want all the
> function calls and conditionals even if you want some of the functionality.

Calling hexdump (even in it's unaltered form) in performance critical code is 
always going to suck. As you mentioned before, it's all based around printf. A 
performance conscious user would be better off building their code around 
hex_asc_hi/lo instead (see lib/vsprintf.c:hex_string).

-- 
Alastair D'Silva   mob: 0423 762 819
skype: alastair_dsilva msn: alast...@d-silva.org
blog: http://alastair.d-silva.orgTwitter: @EvilDeece



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 13/16] drm/edid: use new match_string() helper/macro

[Alexandru Ardelean]

The `generic_edid_name` is a static array of strings.
Using match_string() (which computes the array size via ARRAY_SIZE())
is possible.

The change is mostly cosmetic.
No functionality change.

Signed-off-by: Alexandru Ardelean 
---
 drivers/gpu/drm/drm_edid_load.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 1450051972ea..66e1e325ff37 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -186,7 +186,7 @@ static void *edid_load(struct drm_connector *connector, 
const char *name,
int i, valid_extensions = 0;
bool print_bad_edid = !connector->bad_edid_counter || (drm_debug & 
DRM_UT_KMS);
 
-   builtin = __match_string(generic_edid_name, GENERIC_EDIDS, name);
+   builtin = match_string(generic_edid_name, name);
if (builtin >= 0) {
fwdata = generic_edid[builtin];
fwsize = sizeof(generic_edid[builtin]);
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 09/16] mmc: sdhci-xenon: use new match_string() helper/macro

[Dan Carpenter]

On Wed, May 08, 2019 at 02:28:35PM +0300, Alexandru Ardelean wrote:
> -static const char * const phy_types[] = {
> - "emmc 5.0 phy",
> - "emmc 5.1 phy"
> -};
> -
>  enum xenon_phy_type_enum {
>   EMMC_5_0_PHY,
>   EMMC_5_1_PHY,
>   NR_PHY_TYPES

There is no need for NR_PHY_TYPES now so you could remove that as well.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 03/16] lib,treewide: add new match_string() helper/macro

[Greg KH]

On Wed, May 08, 2019 at 04:11:28PM +0300, Andy Shevchenko wrote:
> On Wed, May 08, 2019 at 02:28:29PM +0300, Alexandru Ardelean wrote:
> > This change re-introduces `match_string()` as a macro that uses
> > ARRAY_SIZE() to compute the size of the array.
> > The macro is added in all the places that do
> > `match_string(_a, ARRAY_SIZE(_a), s)`, since the change is pretty
> > straightforward.
> 
> Can you split include/linux/ change from the rest?

That would break the build, why do you want it split out?  This makes
sense all as a single patch to me.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 03/16] lib,treewide: add new match_string() helper/macro

[Andy Shevchenko]

On Wed, May 08, 2019 at 02:28:29PM +0300, Alexandru Ardelean wrote:
> This change re-introduces `match_string()` as a macro that uses
> ARRAY_SIZE() to compute the size of the array.
> The macro is added in all the places that do
> `match_string(_a, ARRAY_SIZE(_a), s)`, since the change is pretty
> straightforward.

Can you split include/linux/ change from the rest?

> 
> Signed-off-by: Alexandru Ardelean 
> ---
>  drivers/clk/bcm/clk-bcm2835.c| 4 +---
>  drivers/gpio/gpiolib-of.c| 2 +-
>  drivers/gpu/drm/i915/intel_pipe_crc.c| 2 +-
>  drivers/mfd/omap-usb-host.c  | 2 +-
>  drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 2 +-
>  drivers/pci/pcie/aer.c   | 2 +-
>  drivers/usb/common/common.c  | 4 ++--
>  drivers/usb/typec/class.c| 8 +++-
>  drivers/usb/typec/tps6598x.c | 2 +-
>  drivers/vfio/vfio.c  | 4 +---
>  include/linux/string.h   | 9 +
>  sound/firewire/oxfw/oxfw.c   | 2 +-
>  sound/soc/codecs/max98088.c  | 2 +-
>  sound/soc/codecs/max98095.c  | 2 +-
>  14 files changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index a775f6a1f717..1ab388590ead 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -1390,9 +1390,7 @@ static struct clk_hw *bcm2835_register_clock(struct 
> bcm2835_cprman *cprman,
>   for (i = 0; i < data->num_mux_parents; i++) {
>   parents[i] = data->parents[i];
>  
> - ret = __match_string(cprman_parent_names,
> -  ARRAY_SIZE(cprman_parent_names),
> -  parents[i]);
> + ret = match_string(cprman_parent_names, parents[i]);
>   if (ret >= 0)
>   parents[i] = cprman->real_parent_names[ret];
>   }
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 27d6f04ab58e..71e886869d78 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -279,7 +279,7 @@ static struct gpio_desc *of_find_regulator_gpio(struct 
> device *dev, const char *
>   if (!con_id)
>   return ERR_PTR(-ENOENT);
>  
> - i = __match_string(whitelist, ARRAY_SIZE(whitelist), con_id);
> + i = match_string(whitelist, con_id);
>   if (i < 0)
>   return ERR_PTR(-ENOENT);
>  
> diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c 
> b/drivers/gpu/drm/i915/intel_pipe_crc.c
> index 286fad1f0e08..6fc4f3d3d1f6 100644
> --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> @@ -449,7 +449,7 @@ display_crc_ctl_parse_source(const char *buf, enum 
> intel_pipe_crc_source *s)
>   return 0;
>   }
>  
> - i = __match_string(pipe_crc_sources, ARRAY_SIZE(pipe_crc_sources), buf);
> + i = match_string(pipe_crc_sources, buf);
>   if (i < 0)
>   return i;
>  
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index 9aaacb5bdb26..53dff34c0afc 100644
> --- a/drivers/mfd/omap-usb-host.c
> +++ b/drivers/mfd/omap-usb-host.c
> @@ -509,7 +509,7 @@ static int usbhs_omap_get_dt_pdata(struct device *dev,
>   continue;
>  
>   /* get 'enum usbhs_omap_port_mode' from port mode string */
> - ret = __match_string(port_modes, ARRAY_SIZE(port_modes), mode);
> + ret = match_string(port_modes, mode);
>   if (ret < 0) {
>   dev_warn(dev, "Invalid port%d-mode \"%s\" in device 
> tree\n",
>   i, mode);
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c 
> b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> index 59ce3ff35553..778b4dfd8b75 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
> @@ -667,7 +667,7 @@ iwl_dbgfs_bt_force_ant_write(struct iwl_mvm *mvm, char 
> *buf,
>   };
>   int ret, bt_force_ant_mode;
>  
> - ret = __match_string(modes_str, ARRAY_SIZE(modes_str), buf);
> + ret = match_string(modes_str, buf);
>   if (ret < 0)
>   return ret;
>  
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 41a0773a1cbc..2278caba109c 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -203,7 +203,7 @@ void pcie_ecrc_get_policy(char *str)
>  {
>   int i;
>  
> - i = __match_string(ecrc_policy_str, ARRAY_SIZE(ecrc_policy_str), str);
> + i = match_string(ecrc_policy_str, str);
>   if (i < 0)
>   return;
>  
> diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> index bca0c404c6ca..5a651d311d38 100644
> --- a/drivers/usb/common/common.c
> +++ b

Re: [PATCH 03/16] lib,treewide: add new match_string() helper/macro

[Ardelean, Alexandru]

On Wed, 2019-05-08 at 15:18 +0200, Greg KH wrote:
> 
> 
> On Wed, May 08, 2019 at 04:11:28PM +0300, Andy Shevchenko wrote:
> > On Wed, May 08, 2019 at 02:28:29PM +0300, Alexandru Ardelean wrote:
> > > This change re-introduces `match_string()` as a macro that uses
> > > ARRAY_SIZE() to compute the size of the array.
> > > The macro is added in all the places that do
> > > `match_string(_a, ARRAY_SIZE(_a), s)`, since the change is pretty
> > > straightforward.
> > 
> > Can you split include/linux/ change from the rest?
> 
> That would break the build, why do you want it split out?  This makes
> sense all as a single patch to me.
> 

Not really.
It would be just be the new match_string() helper/macro in a new commit.
And the conversions of the simple users of match_string() (the ones using
ARRAY_SIZE()) in another commit.

Thanks
Alex

> thanks,
> 
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 09/16] mmc: sdhci-xenon: use new match_string() helper/macro

[Ardelean, Alexandru]

On Wed, 2019-05-08 at 15:20 +0300, Dan Carpenter wrote:
> 
> 
> On Wed, May 08, 2019 at 02:28:35PM +0300, Alexandru Ardelean wrote:
> > -static const char * const phy_types[] = {
> > - "emmc 5.0 phy",
> > - "emmc 5.1 phy"
> > -};
> > -
> >  enum xenon_phy_type_enum {
> >   EMMC_5_0_PHY,
> >   EMMC_5_1_PHY,
> >   NR_PHY_TYPES
> 
> There is no need for NR_PHY_TYPES now so you could remove that as well.
> 

I thought the same.
The only reason to keep NR_PHY_TYPES, is for potential future patches,
where it would be just 1 addition

 enum xenon_phy_type_enum {
  EMMC_5_0_PHY,
  EMMC_5_1_PHY,
+  EMMC_5_2_PHY,
  NR_PHY_TYPES
  }

Depending on style/preference of how to do enums (allow comma on last enum
or not allow comma on last enum value), adding new enum values woudl be 2
additions + 1 deletion lines.

 enum xenon_phy_type_enum {
  EMMC_5_0_PHY,
-  EMMC_5_1_PHY
+  EMM
C_5_1_PHY,
+  EMMC_5_2_PHY
 }

Either way (leave NR_PHY_TYPES or remove NR_PHY_TYPES) is fine from my
side.

Thanks
Alex

> regards,
> dan carpenter
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] drivers/staging/kpc2000: fix build error on xtensa

[Max Filippov]

kpc2000/kpc_dma/fileops.c includes asm/uaccess.h instead of
linux/uaccess.h, which results in the following build error on xtensa
architecture:

  In file included from
  drivers/staging/kpc2000/kpc_dma/fileops.c:11:
  arch/xtensa/include/asm/uaccess.h:
  In function ‘clear_user’:
  arch/xtensa/include/asm/uaccess.h:40:22:
  error: implicit declaration of function ‘uaccess_kernel’; ...
   #define __kernel_ok (uaccess_kernel())
^~

Include linux/uaccess.h to fix that.

Signed-off-by: Max Filippov 
---
 drivers/staging/kpc2000/kpc_dma/fileops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 5741d2b49a7d..0886ad408b0e 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -8,7 +8,7 @@
 #include /* error codes */
 #include /* size_t */
 #include 
-#include /* copy_*_user */
+#include   /* copy_*_user */
 #include   /* aio stuff */
 #include 
 #include 
-- 
2.11.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes

[Randy Dunlap]

On 5/8/19 12:01 AM, Alastair D'Silva wrote:
> From: Alastair D'Silva 
> 
> Some buffers may only be partially filled with useful data, while the rest
> is padded (typically with 0x00 or 0xff).
> 
> This patch introduces a flag to allow the supression of lines of repeated
> bytes, which are replaced with '** Skipped %u bytes of value 0x%x **'
> 
> An inline wrapper function is provided for backwards compatibility with
> existing code, which maintains the original behaviour.
> 
> Signed-off-by: Alastair D'Silva 
> ---
>  include/linux/printk.h | 25 +---
>  lib/hexdump.c  | 91 --
>  2 files changed, 99 insertions(+), 17 deletions(-)
> 

Hi,
Did you do "make htmldocs" or something similar on this?

> diff --git a/lib/hexdump.c b/lib/hexdump.c
> index 3943507bc0e9..d61a1e4f19fa 100644
> --- a/lib/hexdump.c
> +++ b/lib/hexdump.c
> @@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t len, int 
> rowsize, int groupsize,
>  EXPORT_SYMBOL(hex_dump_to_buffer);
>  
>  #ifdef CONFIG_PRINTK
> +
> +/**
> + * Check if a buffer contains only a single byte value
> + * @buf: pointer to the buffer
> + * @len: the size of the buffer in bytes
> + * @val: outputs the value if if the bytes are identical

Does this work without a function name?
Documentation/doc-guide/kernel-doc.rst says the general format is:

  /**
   * function_name() - Brief description of function.
   * @arg1: Describe the first argument.
   * @arg2: Describe the second argument.
   *One can provide multiple line descriptions
   *for arguments.
   *

> + */

>  /**
> - * print_hex_dump - print a text hex dump to syslog for a binary blob of data
> + * print_hex_dump_ext: dump a binary blob of data to syslog in hexadecimal

Also not in the general documented format.

>   * @level: kernel log level (e.g. KERN_DEBUG)
>   * @prefix_str: string to prefix each line with;
>   *  caller supplies trailing spaces for alignment if desired


-- 
~Randy
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] Drivers: hv: vmbus: Fix virt_to_hvpfn() for X86_PAE

[Sasha Levin]

On Tue, May 07, 2019 at 12:51:51PM +, Michael Kelley wrote:

From: Dexuan Cui  Sent: Tuesday, May 7, 2019 12:47 AM


In the case of X86_PAE, unsigned long is u32, but the physical address type
should be u64. Due to the bug here, the netvsc driver can not load
successfully, and sometimes the VM can panic due to memory corruption (the
hypervisor writes data to the wrong location).

Fixes: 6ba34171bcbd ("Drivers: hv: vmbus: Remove use of slow_virt_to_phys()")
Cc: sta...@vger.kernel.org
Cc: Michael Kelley 
Reported-and-tested-by: Juliana Rodrigueiro 
Signed-off-by: Dexuan Cui 


Reviewed-by:  Michael Kelley 


Queued for hyperv-fixes, thanks!

--
Thanks,
Sasha
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] kpc_i2c: Remove unused file

[Matt Sickler]

The whole file was wrapped in an #if 0.  I'm guessing it was a leftover file
from when we were first developing the driver and we just forgot about it.
---
 drivers/staging/kpc2000/kpc_i2c/Makefile  |   2 +-
 drivers/staging/kpc2000/kpc_i2c/fileops.c | 181 --
 2 files changed, 1 insertion(+), 182 deletions(-)
 delete mode 100644 drivers/staging/kpc2000/kpc_i2c/fileops.c

diff --git a/drivers/staging/kpc2000/kpc_i2c/Makefile 
b/drivers/staging/kpc2000/kpc_i2c/Makefile
index 73ec07ac7d39..63a6ce4b8e03 100644
--- a/drivers/staging/kpc2000/kpc_i2c/Makefile
+++ b/drivers/staging/kpc2000/kpc_i2c/Makefile
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-m := kpc2000_i2c.o
-kpc2000_i2c-objs := i2c_driver.o fileops.o
+kpc2000_i2c-objs := i2c_driver.o
diff --git a/drivers/staging/kpc2000/kpc_i2c/fileops.c 
b/drivers/staging/kpc2000/kpc_i2c/fileops.c
deleted file mode 100644
index e749c0994491..
--- a/drivers/staging/kpc2000/kpc_i2c/fileops.c
+++ /dev/null
@@ -1,181 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-#if 0
-#include 
-#include 
-#include   /* printk() */
-#include /* kmalloc() */
-#include   /* everything... */
-#include/* error codes */
-#include/* size_t */
-#include 
-#include/* copy_*_user */
-
-#include "i2c_driver.h"
-
-int i2c_cdev_open(struct inode *inode, struct file *filp)
-{
-  struct i2c_device *lddev;
-  
-  if(NULL == inode) {
-//printk(KERN_WARNING " i2c_cdev_open: inode is a NULL pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_open: inode is a NULL pointer\n");
-return -EINVAL;
-  }
-  if(NULL == filp) {
-//printk(KERN_WARNING " i2c_cdev_open: filp is a NULL pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_open: filp is a NULL pointer\n");
-return -EINVAL;
-  }
-  
-  lddev = container_of(inode->i_cdev, struct i2c_device, cdev);
-  //printk(KERN_DEBUG " i2c_cdev_open(filp = [%p], lddev = [%p])\n", 
filp, lddev);
-  DBG_PRINT(KERN_DEBUG, "i2c_cdev_open(filp = [%p], lddev = [%p])\n", filp, 
lddev);
-  
-  filp->private_data = lddev; /* so other methods can access it */
-  
-  return 0;/* success */
-}
-
-int i2c_cdev_close(struct inode *inode, struct file *filp)
-{
-  struct i2c_device *lddev;
-  
-  if(NULL == inode) {
-//printk(KERN_WARNING " i2c_cdev_close: inode is a NULL 
pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_close: inode is a NULL pointer\n");
-return -EINVAL;
-  }
-  if(NULL == filp) {
-//printk(KERN_WARNING " i2c_cdev_close: filp is a NULL pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_close: filp is a NULL pointer\n");
-return -EINVAL;
-  }
-  
-  lddev = filp->private_data;
-  //printk(KERN_DEBUG " i2c_cdev_close(filp = [%p], lddev = [%p])\n", 
filp, lddev);
-  DBG_PRINT(KERN_DEBUG, "i2c_cdev_close(filp = [%p], lddev = [%p])\n", filp, 
lddev);
-  
-  return 0;
-}
-
-ssize_t i2c_cdev_read(struct file *filp, char __user *buf, size_t count, 
loff_t *f_pos)
-{
-  size_t copy;
-  ssize_t ret = 0;
-  int err = 0;
-  u64 read_val;
-  char tmp_buf[48] = { 0 };
-  struct i2c_device *lddev = filp->private_data;
-
-  if(NULL == filp) {
-//printk(KERN_WARNING " i2c_cdev_read: filp is a NULL pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_read: filp is a NULL pointer\n");
-return -EINVAL;
-  }
-  if(NULL == buf) {
-//printk(KERN_WARNING " i2c_cdev_read: buf is a NULL pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_read: buf is a NULL pointer\n");
-return -EINVAL;
-  }
-  if(NULL == f_pos) {
-//printk(KERN_WARNING " i2c_cdev_read: f_pos is a NULL pointer\n");
-DBG_PRINT(KERN_WARNING, "i2c_cdev_read: f_pos is a NULL pointer\n");
-return -EINVAL;
-  }
-
-  if(count < sizeof(tmp_buf)) {
-//printk(KERN_INFO " i2c_cdev_read: buffer is too small (count = 
%d, should be at least %d bytes)\n", (int)count, (int)sizeof(tmp_buf));
-DBG_PRINT(KERN_INFO, "i2c_cdev_read: buffer is too small (count = %d, 
should be at least %d bytes)\n", (int)count, (int)sizeof(tmp_buf));
-return -EINVAL;
-  }
-  if(((*f_pos * 8) + lddev->pldev->resource[0].start) > 
lddev->pldev->resource[0].end) {
-//printk(KERN_INFO " i2c_cdev_read: bad read addr %016llx\n", 
(*f_pos * 8) + lddev->pldev->resource[0].start);
-DBG_PRINT(KERN_INFO, "i2c_cdev_read: bad read addr %016llx\n", (*f_pos * 
8) + lddev->pldev->resource[0].start);
-//printk(KERN_INFO " i2c_cdev_read: addr end %016llx\n", 
lddev->pldev->resource[0].end);
-DBG_PRINT(KERN_INFO, "i2c_cdev_read: addr end %016llx\n", 
lddev->pldev->resource[0].end);
-//printk(KERN_INFO " i2c_cdev_read: EOF reached\n");
-DBG_PRINT(KERN_INFO, "i2c_cdev_read: EOF reached\n");
-return 0;
-  }
-
-  down_read(&lddev->rw_sem);
-  
-  read_val = *(lddev->regs + *f_pos);
-  copy = clamp_t(size_t, count, 1, sizeof(tmp_buf));
-  copy = scnprintf(tmp_buf, copy, "reg: 0x%x val: 0x%llx\n", (unsigned 
int)*f_pos, read_val);
-  err = copy_to_user(buf, tmp_buf, copy);
-  i

Re: [PATCH] kpc_i2c: Remove unused file

[Greg KH]

On Thu, May 09, 2019 at 02:05:22AM +, Matt Sickler wrote:
> The whole file was wrapped in an #if 0.  I'm guessing it was a leftover file
> from when we were first developing the driver and we just forgot about it.
> ---
>  drivers/staging/kpc2000/kpc_i2c/Makefile  |   2 +-
>  drivers/staging/kpc2000/kpc_i2c/fileops.c | 181 
> --
>  2 files changed, 1 insertion(+), 182 deletions(-)
>  delete mode 100644 drivers/staging/kpc2000/kpc_i2c/fileops.c

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch does not have a Signed-off-by: line.  Please read the
  kernel file, Documentation/SubmittingPatches and resend it after
  adding that line.  Note, the line needs to be in the body of the
  email, before the patch, not at the bottom of the patch or in the
  email signature.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel