[PATCH] imon_raw: respect DMA coherency

[PATCH] imon_raw: respect DMA coherency

[Oliver Neukum]

No buffer can be embedded inside a descriptor, not even a simple be64.
Use a separate kmalloc()

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/media/rc/imon_raw.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/imon_raw.c b/drivers/media/rc/imon_raw.c
index d41580f6e4c7..b7a0c0b34378 100644
--- a/drivers/media/rc/imon_raw.c
+++ b/drivers/media/rc/imon_raw.c
@@ -14,7 +14,7 @@ struct imon {
 	struct device *dev;
 	struct urb *ir_urb;
 	struct rc_dev *rcdev;
-	__be64 ir_buf;
+	__be64 *ir_buf;
 	char phys[64];
 };
 
@@ -137,10 +137,16 @@ static int imon_probe(struct usb_interface *intf,
 	if (!imon->ir_urb)
 		return -ENOMEM;
 
+	imon->ir_buf = kmalloc(sizeof(__be64), GFP_KERNEL);
+	if (!imon->ir_buf) {
+		ret = -ENOMEM;
+		goto free_urb;
+	}
+
 	imon->dev = &intf->dev;
 	usb_fill_int_urb(imon->ir_urb, udev,
 			 usb_rcvintpipe(udev, ir_ep->bEndpointAddress),
-			 &imon->ir_buf, sizeof(imon->ir_buf),
+			 imon->ir_buf, sizeof(__be64),
 			 imon_ir_rx, imon, ir_ep->bInterval);
 
 	rcdev = devm_rc_allocate_device(&intf->dev, RC_DRIVER_IR_RAW);
@@ -177,6 +183,7 @@ static int imon_probe(struct usb_interface *intf,
 
 free_urb:
 	usb_free_urb(imon->ir_urb);
+	kfree(imon->ir_buf);
 	return ret;
 }
 
@@ -186,6 +193,7 @@ static void imon_disconnect(struct usb_interface *intf)
 
 	usb_kill_urb(imon->ir_urb);
 	usb_free_urb(imon->ir_urb);
+	kfree(imon->ir_buf);
 }
 
 static const struct usb_device_id imon_table[] = {
-- 
2.35.3

Re: [PATCH] imon_raw: respect DMA coherency

[kernel test robot]

Hi Oliver,

I love your patch! Perhaps something to improve:

[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.18-rc6 next-20220512]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Oliver-Neukum/imon_raw-respect-DMA-coherency/20220512-210422
base:   git://linuxtv.org/media_tree.git master
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220513/202205130146.FkLP3KTa-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/5e1a1b1e9c8288033f5f1f1d70a3d7506114fad3
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Oliver-Neukum/imon_raw-respect-DMA-coherency/20220512-210422
        git checkout 5e1a1b1e9c8288033f5f1f1d70a3d7506114fad3
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/media/rc/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/byteorder/big_endian.h:5,
                    from arch/arc/include/uapi/asm/byteorder.h:14,
                    from include/asm-generic/bitops/le.h:6,
                    from arch/arc/include/asm/bitops.h:192,
                    from include/linux/bitops.h:33,
                    from include/linux/log2.h:12,
                    from include/asm-generic/div64.h:55,
                    from ./arch/arc/include/generated/asm/div64.h:1,
                    from include/linux/math.h:6,
                    from include/linux/math64.h:6,
                    from include/linux/time.h:6,
                    from include/linux/stat.h:19,
                    from include/linux/module.h:13,
                    from drivers/media/rc/imon_raw.c:5:
   drivers/media/rc/imon_raw.c: In function 'imon_ir_data':
>> include/uapi/linux/byteorder/big_endian.h:39:42: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
      39 | #define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
         |                                          ^
   include/linux/byteorder/generic.h:93:21: note: in expansion of macro '__be64_to_cpu'
      93 | #define be64_to_cpu __be64_to_cpu
         |                     ^~~~~~~~~~~~~
   drivers/media/rc/imon_raw.c:32:20: note: in expansion of macro 'be64_to_cpu'
      32 |         u64 data = be64_to_cpu(imon->ir_buf);
         |                    ^~~~~~~~~~~


vim +39 include/uapi/linux/byteorder/big_endian.h

5921e6f8809b16 David Howells 2012-10-13  15  
5921e6f8809b16 David Howells 2012-10-13  16  #define __constant_htonl(x) ((__force __be32)(__u32)(x))
5921e6f8809b16 David Howells 2012-10-13  17  #define __constant_ntohl(x) ((__force __u32)(__be32)(x))
5921e6f8809b16 David Howells 2012-10-13  18  #define __constant_htons(x) ((__force __be16)(__u16)(x))
5921e6f8809b16 David Howells 2012-10-13  19  #define __constant_ntohs(x) ((__force __u16)(__be16)(x))
5921e6f8809b16 David Howells 2012-10-13  20  #define __constant_cpu_to_le64(x) ((__force __le64)___constant_swab64((x)))
5921e6f8809b16 David Howells 2012-10-13  21  #define __constant_le64_to_cpu(x) ___constant_swab64((__force __u64)(__le64)(x))
5921e6f8809b16 David Howells 2012-10-13  22  #define __constant_cpu_to_le32(x) ((__force __le32)___constant_swab32((x)))
5921e6f8809b16 David Howells 2012-10-13  23  #define __constant_le32_to_cpu(x) ___constant_swab32((__force __u32)(__le32)(x))
5921e6f8809b16 David Howells 2012-10-13  24  #define __constant_cpu_to_le16(x) ((__force __le16)___constant_swab16((x)))
5921e6f8809b16 David Howells 2012-10-13  25  #define __constant_le16_to_cpu(x) ___constant_swab16((__force __u16)(__le16)(x))
5921e6f8809b16 David Howells 2012-10-13  26  #define __constant_cpu_to_be64(x) ((__force __be64)(__u64)(x))
5921e6f8809b16 David Howells 2012-10-13  27  #define __constant_be64_to_cpu(x) ((__force __u64)(__be64)(x))
5921e6f8809b16 David Howells 2012-10-13  28  #define __constant_cpu_to_be32(x) ((__force __be32)(__u32)(x))
5921e6f8809b16 David Howells 2012-10-13  29  #define __constant_be32_to_cpu(x) ((__force __u32)(__be32)(x))
5921e6f8809b16 David Howells 2012-10-13  30  #define __constant_cpu_to_be16(x) ((__force __be16)(__u16)(x))
5921e6f8809b16 David Howells 2012-10-13  31  #define __constant_be16_to_cpu(x) ((__force __u16)(__be16)(x))
5921e6f8809b16 David Howells 2012-10-13  32  #define __cpu_to_le64(x) ((__force __le64)__swab64((x)))
5921e6f8809b16 David Howells 2012-10-13  33  #define __le64_to_cpu(x) __swab64((__force __u64)(__le64)(x))
5921e6f8809b16 David Howells 2012-10-13  34  #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
5921e6f8809b16 David Howells 2012-10-13  35  #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
5921e6f8809b16 David Howells 2012-10-13  36  #define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
5921e6f8809b16 David Howells 2012-10-13  37  #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
5921e6f8809b16 David Howells 2012-10-13  38  #define __cpu_to_be64(x) ((__force __be64)(__u64)(x))
5921e6f8809b16 David Howells 2012-10-13 @39  #define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
5921e6f8809b16 David Howells 2012-10-13  40  #define __cpu_to_be32(x) ((__force __be32)(__u32)(x))
5921e6f8809b16 David Howells 2012-10-13  41  #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
5921e6f8809b16 David Howells 2012-10-13  42  #define __cpu_to_be16(x) ((__force __be16)(__u16)(x))
5921e6f8809b16 David Howells 2012-10-13  43  #define __be16_to_cpu(x) ((__force __u16)(__be16)(x))
5921e6f8809b16 David Howells 2012-10-13  44  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH] imon_raw: respect DMA coherency

[kernel test robot]

Hi Oliver,

I love your patch! Perhaps something to improve:

[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.18-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Oliver-Neukum/imon_raw-respect-DMA-coherency/20220512-210422
base:   git://linuxtv.org/media_tree.git master
config: arm-allmodconfig (https://download.01.org/0day-ci/archive/20220513/202205130246.Kh3xxrsg-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/5e1a1b1e9c8288033f5f1f1d70a3d7506114fad3
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Oliver-Neukum/imon_raw-respect-DMA-coherency/20220512-210422
        git checkout 5e1a1b1e9c8288033f5f1f1d70a3d7506114fad3
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/media/rc/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/swab.h:5,
                    from include/uapi/linux/byteorder/little_endian.h:14,
                    from include/linux/byteorder/little_endian.h:5,
                    from arch/arm/include/uapi/asm/byteorder.h:22,
                    from include/asm-generic/bitops/le.h:6,
                    from arch/arm/include/asm/bitops.h:267,
                    from include/linux/bitops.h:33,
                    from include/linux/log2.h:12,
                    from include/asm-generic/div64.h:55,
                    from arch/arm/include/asm/div64.h:107,
                    from include/linux/math.h:6,
                    from include/linux/math64.h:6,
                    from include/linux/time.h:6,
                    from include/linux/stat.h:19,
                    from include/linux/module.h:13,
                    from drivers/media/rc/imon_raw.c:5:
   drivers/media/rc/imon_raw.c: In function 'imon_ir_data':
>> include/uapi/linux/byteorder/little_endian.h:39:50: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
      39 | #define __be64_to_cpu(x) __swab64((__force __u64)(__be64)(x))
         |                                                  ^
   include/uapi/linux/swab.h:128:54: note: in definition of macro '__swab64'
     128 | #define __swab64(x) (__u64)__builtin_bswap64((__u64)(x))
         |                                                      ^
   include/linux/byteorder/generic.h:93:21: note: in expansion of macro '__be64_to_cpu'
      93 | #define be64_to_cpu __be64_to_cpu
         |                     ^~~~~~~~~~~~~
   drivers/media/rc/imon_raw.c:32:20: note: in expansion of macro 'be64_to_cpu'
      32 |         u64 data = be64_to_cpu(imon->ir_buf);
         |                    ^~~~~~~~~~~


vim +39 include/uapi/linux/byteorder/little_endian.h

5921e6f8809b16 David Howells 2012-10-13  15  
5921e6f8809b16 David Howells 2012-10-13  16  #define __constant_htonl(x) ((__force __be32)___constant_swab32((x)))
5921e6f8809b16 David Howells 2012-10-13  17  #define __constant_ntohl(x) ___constant_swab32((__force __be32)(x))
5921e6f8809b16 David Howells 2012-10-13  18  #define __constant_htons(x) ((__force __be16)___constant_swab16((x)))
5921e6f8809b16 David Howells 2012-10-13  19  #define __constant_ntohs(x) ___constant_swab16((__force __be16)(x))
5921e6f8809b16 David Howells 2012-10-13  20  #define __constant_cpu_to_le64(x) ((__force __le64)(__u64)(x))
5921e6f8809b16 David Howells 2012-10-13  21  #define __constant_le64_to_cpu(x) ((__force __u64)(__le64)(x))
5921e6f8809b16 David Howells 2012-10-13  22  #define __constant_cpu_to_le32(x) ((__force __le32)(__u32)(x))
5921e6f8809b16 David Howells 2012-10-13  23  #define __constant_le32_to_cpu(x) ((__force __u32)(__le32)(x))
5921e6f8809b16 David Howells 2012-10-13  24  #define __constant_cpu_to_le16(x) ((__force __le16)(__u16)(x))
5921e6f8809b16 David Howells 2012-10-13  25  #define __constant_le16_to_cpu(x) ((__force __u16)(__le16)(x))
5921e6f8809b16 David Howells 2012-10-13  26  #define __constant_cpu_to_be64(x) ((__force __be64)___constant_swab64((x)))
5921e6f8809b16 David Howells 2012-10-13  27  #define __constant_be64_to_cpu(x) ___constant_swab64((__force __u64)(__be64)(x))
5921e6f8809b16 David Howells 2012-10-13  28  #define __constant_cpu_to_be32(x) ((__force __be32)___constant_swab32((x)))
5921e6f8809b16 David Howells 2012-10-13  29  #define __constant_be32_to_cpu(x) ___constant_swab32((__force __u32)(__be32)(x))
5921e6f8809b16 David Howells 2012-10-13  30  #define __constant_cpu_to_be16(x) ((__force __be16)___constant_swab16((x)))
5921e6f8809b16 David Howells 2012-10-13  31  #define __constant_be16_to_cpu(x) ___constant_swab16((__force __u16)(__be16)(x))
5921e6f8809b16 David Howells 2012-10-13  32  #define __cpu_to_le64(x) ((__force __le64)(__u64)(x))
5921e6f8809b16 David Howells 2012-10-13  33  #define __le64_to_cpu(x) ((__force __u64)(__le64)(x))
5921e6f8809b16 David Howells 2012-10-13  34  #define __cpu_to_le32(x) ((__force __le32)(__u32)(x))
5921e6f8809b16 David Howells 2012-10-13  35  #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
5921e6f8809b16 David Howells 2012-10-13  36  #define __cpu_to_le16(x) ((__force __le16)(__u16)(x))
5921e6f8809b16 David Howells 2012-10-13  37  #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
5921e6f8809b16 David Howells 2012-10-13  38  #define __cpu_to_be64(x) ((__force __be64)__swab64((x)))
5921e6f8809b16 David Howells 2012-10-13 @39  #define __be64_to_cpu(x) __swab64((__force __u64)(__be64)(x))
5921e6f8809b16 David Howells 2012-10-13  40  #define __cpu_to_be32(x) ((__force __be32)__swab32((x)))
5921e6f8809b16 David Howells 2012-10-13  41  #define __be32_to_cpu(x) __swab32((__force __u32)(__be32)(x))
5921e6f8809b16 David Howells 2012-10-13  42  #define __cpu_to_be16(x) ((__force __be16)__swab16((x)))
5921e6f8809b16 David Howells 2012-10-13  43  #define __be16_to_cpu(x) __swab16((__force __u16)(__be16)(x))
5921e6f8809b16 David Howells 2012-10-13  44  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH] imon_raw: respect DMA coherency

[kernel test robot]

Hi Oliver,

I love your patch! Perhaps something to improve:

[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.18-rc6 next-20220512]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Oliver-Neukum/imon_raw-respect-DMA-coherency/20220512-210422
base:   git://linuxtv.org/media_tree.git master
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20220513/202205131229.FeABo9N9-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 11.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/intel-lab-lkp/linux/commit/5e1a1b1e9c8288033f5f1f1d70a3d7506114fad3
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Oliver-Neukum/imon_raw-respect-DMA-coherency/20220512-210422
        git checkout 5e1a1b1e9c8288033f5f1f1d70a3d7506114fad3
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=sh SHELL=/bin/bash drivers/media/rc/ drivers/platform/mellanox/ net/rxrpc/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/media/rc/imon_raw.c:32:20: sparse: sparse: cast to restricted __be64
>> drivers/media/rc/imon_raw.c:32:20: sparse: sparse: cast to restricted __be64
>> drivers/media/rc/imon_raw.c:32:20: sparse: sparse: cast to restricted __be64
>> drivers/media/rc/imon_raw.c:32:20: sparse: sparse: cast to restricted __be64
>> drivers/media/rc/imon_raw.c:32:20: sparse: sparse: cast to restricted __be64
>> drivers/media/rc/imon_raw.c:32:20: sparse: sparse: cast to restricted __be64
>> drivers/media/rc/imon_raw.c:32:20: sparse: sparse: cast to restricted __be64
>> drivers/media/rc/imon_raw.c:32:20: sparse: sparse: cast to restricted __be64
>> drivers/media/rc/imon_raw.c:32:20: sparse: sparse: cast to restricted __be64
>> drivers/media/rc/imon_raw.c:32:20: sparse: sparse: cast to restricted __be64
>> drivers/media/rc/imon_raw.c:32:20: sparse: sparse: non size-preserving pointer to integer cast

vim +32 drivers/media/rc/imon_raw.c

8a4e8f8dfc6994 Sean Young 2018-01-05  20  
8a4e8f8dfc6994 Sean Young 2018-01-05  21  /*
8d023a5787775c Sean Young 2018-10-18  22   * The first 5 bytes of data represent IR pulse or space. Each bit, starting
8d023a5787775c Sean Young 2018-10-18  23   * from highest bit in the first byte, represents 250µs of data. It is 1
8d023a5787775c Sean Young 2018-10-18  24   * for space and 0 for pulse.
8d023a5787775c Sean Young 2018-10-18  25   *
8d023a5787775c Sean Young 2018-10-18  26   * The station sends 10 packets, and the 7th byte will be number 1 to 10, so
8d023a5787775c Sean Young 2018-10-18  27   * when we receive 10 we assume all the data has arrived.
8a4e8f8dfc6994 Sean Young 2018-01-05  28   */
8a4e8f8dfc6994 Sean Young 2018-01-05  29  static void imon_ir_data(struct imon *imon)
8a4e8f8dfc6994 Sean Young 2018-01-05  30  {
183e19f5b9ee18 Sean Young 2018-08-21  31  	struct ir_raw_event rawir = {};
e70d13f7ac061d Sean Young 2019-08-09 @32  	u64 data = be64_to_cpu(imon->ir_buf);
e70d13f7ac061d Sean Young 2019-08-09  33  	u8 packet_no = data & 0xff;
8d023a5787775c Sean Young 2018-10-18  34  	int offset = 40;
8a4e8f8dfc6994 Sean Young 2018-01-05  35  	int bit;
8a4e8f8dfc6994 Sean Young 2018-01-05  36  
e70d13f7ac061d Sean Young 2019-08-09  37  	if (packet_no == 0xff)
e70d13f7ac061d Sean Young 2019-08-09  38  		return;
e70d13f7ac061d Sean Young 2019-08-09  39  
e70d13f7ac061d Sean Young 2019-08-09  40  	dev_dbg(imon->dev, "data: %*ph", 8, &imon->ir_buf);
e70d13f7ac061d Sean Young 2019-08-09  41  
e70d13f7ac061d Sean Young 2019-08-09  42  	/*
e70d13f7ac061d Sean Young 2019-08-09  43  	 * Only the first 5 bytes contain IR data. Right shift so we move
e70d13f7ac061d Sean Young 2019-08-09  44  	 * the IR bits to the lower 40 bits.
e70d13f7ac061d Sean Young 2019-08-09  45  	 */
e70d13f7ac061d Sean Young 2019-08-09  46  	data >>= 24;
8a4e8f8dfc6994 Sean Young 2018-01-05  47  
8d023a5787775c Sean Young 2018-10-18  48  	do {
e70d13f7ac061d Sean Young 2019-08-09  49  		/*
e70d13f7ac061d Sean Young 2019-08-09  50  		 * Find highest set bit which is less or equal to offset
e70d13f7ac061d Sean Young 2019-08-09  51  		 *
e70d13f7ac061d Sean Young 2019-08-09  52  		 * offset is the bit above (base 0) where we start looking.
e70d13f7ac061d Sean Young 2019-08-09  53  		 *
e70d13f7ac061d Sean Young 2019-08-09  54  		 * data & (BIT_ULL(offset) - 1) masks off any unwanted bits,
e70d13f7ac061d Sean Young 2019-08-09  55  		 * so we have just bits less than offset.
e70d13f7ac061d Sean Young 2019-08-09  56  		 *
e70d13f7ac061d Sean Young 2019-08-09  57  		 * fls will tell us the highest bit set plus 1 (or 0 if no
e70d13f7ac061d Sean Young 2019-08-09  58  		 * bits are set).
e70d13f7ac061d Sean Young 2019-08-09  59  		 */
d587cdb2a5f570 Sean Young 2019-10-07  60  		rawir.pulse = !rawir.pulse;
e70d13f7ac061d Sean Young 2019-08-09  61  		bit = fls64(data & (BIT_ULL(offset) - 1));
8d023a5787775c Sean Young 2018-10-18  62  		if (bit < offset) {
d587cdb2a5f570 Sean Young 2019-10-07  63  			dev_dbg(imon->dev, "%s: %d bits",
d587cdb2a5f570 Sean Young 2019-10-07  64  				rawir.pulse ? "pulse" : "space", offset - bit);
8d023a5787775c Sean Young 2018-10-18  65  			rawir.duration = (offset - bit) * BIT_DURATION;
8a4e8f8dfc6994 Sean Young 2018-01-05  66  			ir_raw_event_store_with_filter(imon->rcdev, &rawir);
8a4e8f8dfc6994 Sean Young 2018-01-05  67  
8a4e8f8dfc6994 Sean Young 2018-01-05  68  			offset = bit;
8d023a5787775c Sean Young 2018-10-18  69  		}
8d023a5787775c Sean Young 2018-10-18  70  
d587cdb2a5f570 Sean Young 2019-10-07  71  		data = ~data;
8d023a5787775c Sean Young 2018-10-18  72  	} while (offset > 0);
8a4e8f8dfc6994 Sean Young 2018-01-05  73  
494fce160f2dac Sean Young 2019-08-09  74  	if (packet_no == 0x0a && !imon->rcdev->idle) {
8a4e8f8dfc6994 Sean Young 2018-01-05  75  		ir_raw_event_set_idle(imon->rcdev, true);
8a4e8f8dfc6994 Sean Young 2018-01-05  76  		ir_raw_event_handle(imon->rcdev);
8a4e8f8dfc6994 Sean Young 2018-01-05  77  	}
8a4e8f8dfc6994 Sean Young 2018-01-05  78  }
8a4e8f8dfc6994 Sean Young 2018-01-05  79  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH] imon_raw: respect DMA coherency

[Sean Young]

On Thu, May 12, 2022 at 03:03:21PM +0200, Oliver Neukum wrote:
> No buffer can be embedded inside a descriptor, not even a simple be64.
> Use a separate kmalloc()

This patch needs a tiny change from be64_to_cpu() to be64_to_cpup(), I've
tested that change with the hardware.

Applied to my tree:

https://git.linuxtv.org/syoung/media_tree.git/log/?h=for-v5.20a

Thanks,

Sean

> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
>  drivers/media/rc/imon_raw.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/rc/imon_raw.c b/drivers/media/rc/imon_raw.c
> index d41580f6e4c7..b7a0c0b34378 100644
> --- a/drivers/media/rc/imon_raw.c
> +++ b/drivers/media/rc/imon_raw.c
> @@ -14,7 +14,7 @@ struct imon {
>  	struct device *dev;
>  	struct urb *ir_urb;
>  	struct rc_dev *rcdev;
> -	__be64 ir_buf;
> +	__be64 *ir_buf;
>  	char phys[64];
>  };
>  
> @@ -137,10 +137,16 @@ static int imon_probe(struct usb_interface *intf,
>  	if (!imon->ir_urb)
>  		return -ENOMEM;
>  
> +	imon->ir_buf = kmalloc(sizeof(__be64), GFP_KERNEL);
> +	if (!imon->ir_buf) {
> +		ret = -ENOMEM;
> +		goto free_urb;
> +	}
> +
>  	imon->dev = &intf->dev;
>  	usb_fill_int_urb(imon->ir_urb, udev,
>  			 usb_rcvintpipe(udev, ir_ep->bEndpointAddress),
> -			 &imon->ir_buf, sizeof(imon->ir_buf),
> +			 imon->ir_buf, sizeof(__be64),
>  			 imon_ir_rx, imon, ir_ep->bInterval);
>  
>  	rcdev = devm_rc_allocate_device(&intf->dev, RC_DRIVER_IR_RAW);
> @@ -177,6 +183,7 @@ static int imon_probe(struct usb_interface *intf,
>  
>  free_urb:
>  	usb_free_urb(imon->ir_urb);
> +	kfree(imon->ir_buf);
>  	return ret;
>  }
>  
> @@ -186,6 +193,7 @@ static void imon_disconnect(struct usb_interface *intf)
>  
>  	usb_kill_urb(imon->ir_urb);
>  	usb_free_urb(imon->ir_urb);
> +	kfree(imon->ir_buf);
>  }
>  
>  static const struct usb_device_id imon_table[] = {
> -- 
> 2.35.3

Re: [PATCH] imon_raw: respect DMA coherency

[Oliver Neukum]

On 13.05.22 18:23, Sean Young wrote:
Hi!
> On Thu, May 12, 2022 at 03:03:21PM +0200, Oliver Neukum wrote:
>> No buffer can be embedded inside a descriptor, not even a simple be64.
>> Use a separate kmalloc()
> This patch needs a tiny change from be64_to_cpu() to be64_to_cpup(), I've
> tested that change with the hardware.
Needs? It is certainly not wrong and the subsequent logging will be in the
converted order, but need

    Regards
        Oliver

Re: [PATCH] imon_raw: respect DMA coherency

[Sean Young]

On Mon, May 16, 2022 at 01:00:30PM +0200, Oliver Neukum wrote:
> On 13.05.22 18:23, Sean Young wrote:
> Hi!
> > On Thu, May 12, 2022 at 03:03:21PM +0200, Oliver Neukum wrote:
> >> No buffer can be embedded inside a descriptor, not even a simple be64.
> >> Use a separate kmalloc()
> > This patch needs a tiny change from be64_to_cpu() to be64_to_cpup(), I've
> > tested that change with the hardware.
> Needs? It is certainly not wrong and the subsequent logging will be in the
> converted order, but need

It certainly is wrong, and it doesn't compile without it, so yes it does
need it. The kernel test robot also complained about.

Thanks

Sean

Re: [PATCH] imon_raw: respect DMA coherency

[Oliver Neukum]

On 16.05.22 18:40, Sean Young wrote:
> On Mon, May 16, 2022 at 01:00:30PM +0200, Oliver Neukum wrote:
>> On 13.05.22 18:23, Sean Young wrote:
>> Hi!
>>> On Thu, May 12, 2022 at 03:03:21PM +0200, Oliver Neukum wrote:
>>>> No buffer can be embedded inside a descriptor, not even a simple be64.
>>>> Use a separate kmalloc()
>>> This patch needs a tiny change from be64_to_cpu() to be64_to_cpup(), I've
>>> tested that change with the hardware.
>> Needs? It is certainly not wrong and the subsequent logging will be in the
>> converted order, but need
> It certainly is wrong, and it doesn't compile without it, so yes it does
> need it. The kernel test robot also complained about.
>
>

Hi,

sorry, it seems I forgot a "*". You are right.

    Regards
        Oliver

Re: [PATCH] imon_raw: respect DMA coherency

[Sean Young]

Oliver,

On Tue, May 17, 2022 at 09:34:49AM +0200, Oliver Neukum wrote:
> On 16.05.22 18:40, Sean Young wrote:
> > On Mon, May 16, 2022 at 01:00:30PM +0200, Oliver Neukum wrote:
> >> On 13.05.22 18:23, Sean Young wrote:
> >> Hi!
> >>> On Thu, May 12, 2022 at 03:03:21PM +0200, Oliver Neukum wrote:
> >>>> No buffer can be embedded inside a descriptor, not even a simple be64.
> >>>> Use a separate kmalloc()
> >>> This patch needs a tiny change from be64_to_cpu() to be64_to_cpup(), I've
> >>> tested that change with the hardware.
> >> Needs? It is certainly not wrong and the subsequent logging will be in the
> >> converted order, but need
> > It certainly is wrong, and it doesn't compile without it, so yes it does
> > need it. The kernel test robot also complained about.
> >
> >
> 
> Hi,
> 
> sorry, it seems I forgot a "*". You are right.

Thank you for your patches, they fix many important issues that I missed.


Sean

Re: [PATCH] imon_raw: respect DMA coherency

[Oliver Neukum]

On 17.05.22 09:42, Sean Young wrote:
> Oliver,
>
> On Tue, May 17, 2022 at 09:34:49AM +0200, Oliver Neukum wrote:
>> On 16.05.22 18:40, Sean Young wrote:
>>> On Mon, May 16, 2022 at 01:00:30PM +0200, Oliver Neukum wrote:
>>>> On 13.05.22 18:23, Sean Young wrote:
>>>> Hi!
>>>>> On Thu, May 12, 2022 at 03:03:21PM +0200, Oliver Neukum wrote:
>>>>>> No buffer can be embedded inside a descriptor, not even a simple be64.
>>>>>> Use a separate kmalloc()
>>>>> This patch needs a tiny change from be64_to_cpu() to be64_to_cpup(), I've
>>>>> tested that change with the hardware.
>>>> Needs? It is certainly not wrong and the subsequent logging will be in the
>>>> converted order, but need
>>> It certainly is wrong, and it doesn't compile without it, so yes it does
>>> need it. The kernel test robot also complained about.
>>>
>>>
>> Hi,
>>
>> sorry, it seems I forgot a "*". You are right.
> Thank you for your patches, they fix many important issues that I missed.
>
Hi,

sorry for being a bit obnoxious about this, but there is a slight issue.
This is the old code:

static void imon_ir_data(struct imon *imon)
{
       struct ir_raw_event rawir = {};
       u64 data = be64_to_cpu(imon->ir_buf);
       u8 packet_no = data & 0xff;
       int offset = 40;
       int bit;

       if (packet_no == 0xff)
               return;

       dev_dbg(imon->dev, "data: %*ph", 8, &imon->ir_buf);

The dev_dbg() logs the data as it is in the buffer. If you use
be64_to_cpup() instead of be64_to_cpu() you reverse
the buffer on a little endian CPU and hence the debug
output will be changed.
The actual driver code is unaffected, because the
buffer is never used again, so this is not a big deal.

The error is mine by changing the type of imon->ir_buf
But the fix is not quite the best.

    Regards
        Oliver

Re: [PATCH] imon_raw: respect DMA coherency

[Sean Young]

On Tue, May 17, 2022 at 10:28:35AM +0200, Oliver Neukum wrote:
> sorry for being a bit obnoxious about this, but there is a slight issue.

Pointing out potential issues is the opposite of obnoxious :)

> This is the old code:
> 
> static void imon_ir_data(struct imon *imon)
> {
>        struct ir_raw_event rawir = {};
>        u64 data = be64_to_cpu(imon->ir_buf);
>        u8 packet_no = data & 0xff;
>        int offset = 40;
>        int bit;
> 
>        if (packet_no == 0xff)
>                return;
> 
>        dev_dbg(imon->dev, "data: %*ph", 8, &imon->ir_buf);

That should be imon->ir_buf (not &imon->ir_buf) after your changes.

> The dev_dbg() logs the data as it is in the buffer. If you use
> be64_to_cpup() instead of be64_to_cpu() you reverse
> the buffer on a little endian CPU and hence the debug
> output will be changed.

I'm confused. be64_to_cpup() does not do an in-place byte swap on little
endian. 

I've just tested and the debug message still works fine (barring the issue
above).

> The actual driver code is unaffected, because the
> buffer is never used again, so this is not a big deal.
> 
> The error is mine by changing the type of imon->ir_buf
> But the fix is not quite the best.

Your change is good.


Sean