Re: [PATCH 4/5] zram: remove zram_meta structure

[Sergey Senozhatsky]

On (04/04/17 14:40), Minchan Kim wrote:
> > [..]
> > > -struct zram_meta {
> > > +struct zram {
> > >   struct zram_table_entry *table;
> > >   struct zs_pool *mem_pool;
> > > -};
> > > -
> > > -struct zram {
> > > - struct zram_meta *meta;
> > >   struct zcomp *comp;
> > >   struct gendisk *disk;
> > >   /* Prevent concurrent execution of device init */
> > 
> > 
> > we still have several zram_meta_FOO() left overs in zram_drv.c
> 
> I intentionally used that term because I don't think better name
> to wrap logis which allocates table and mempool.

ah, it was intentional. um, OK, let's keep zram_meta_foo().

-ss

Re: [PATCH 4/5] zram: remove zram_meta structure

[Sergey Senozhatsky]

On (04/04/17 14:40), Minchan Kim wrote:
> > [..]
> > > -struct zram_meta {
> > > +struct zram {
> > >   struct zram_table_entry *table;
> > >   struct zs_pool *mem_pool;
> > > -};
> > > -
> > > -struct zram {
> > > - struct zram_meta *meta;
> > >   struct zcomp *comp;
> > >   struct gendisk *disk;
> > >   /* Prevent concurrent execution of device init */
> > 
> > 
> > we still have several zram_meta_FOO() left overs in zram_drv.c
> 
> I intentionally used that term because I don't think better name
> to wrap logis which allocates table and mempool.

ah, it was intentional. um, OK, let's keep zram_meta_foo().

-ss

[PATCH] net: ibm: emac: remove unused sysrq handler for 'c' key

[Eric Biggers]

From: Eric Biggers 

Since commit d6580a9f1523 ("kexec: sysrq: simplify sysrq-c handler"),
the sysrq handler for the 'c' key has been sysrq_crash_op.  Debugging
code in the ibm_emac driver also tries to register a handler for the 'c'
key, but this has no effect because register_sysrq_key() doesn't replace
existing handlers.  Since evidently no one has cared enough to fix this
in the last 8 years, and it's very rare for drivers to register sysrq
handlers (for good reason), just remove the dead code.

Signed-off-by: Eric Biggers 
---
 drivers/net/ethernet/ibm/emac/Makefile |   1 -
 drivers/net/ethernet/ibm/emac/core.c   |   7 -
 drivers/net/ethernet/ibm/emac/debug.c  | 270 -
 drivers/net/ethernet/ibm/emac/debug.h  |  23 ---
 drivers/net/ethernet/ibm/emac/mal.c|   4 -
 drivers/tty/sysrq.c|   2 +-
 6 files changed, 1 insertion(+), 306 deletions(-)
 delete mode 100644 drivers/net/ethernet/ibm/emac/debug.c

diff --git a/drivers/net/ethernet/ibm/emac/Makefile 
b/drivers/net/ethernet/ibm/emac/Makefile
index eba21835d90d..98768ba0955a 100644
--- a/drivers/net/ethernet/ibm/emac/Makefile
+++ b/drivers/net/ethernet/ibm/emac/Makefile
@@ -8,4 +8,3 @@ ibm_emac-y := mal.o core.o phy.o
 ibm_emac-$(CONFIG_IBM_EMAC_ZMII) += zmii.o
 ibm_emac-$(CONFIG_IBM_EMAC_RGMII) += rgmii.o
 ibm_emac-$(CONFIG_IBM_EMAC_TAH) += tah.o
-ibm_emac-$(CONFIG_IBM_EMAC_DEBUG) += debug.o
diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index c44036d5761a..0f9cd92fdc5e 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -3173,8 +3173,6 @@ static int emac_probe(struct platform_device *ofdev)
printk("%s: found %s PHY (0x%02x)\n", ndev->name,
   dev->phy.def->name, dev->phy.address);
 
-   emac_dbg_register(dev);
-
/* Life is good */
return 0;
 
@@ -3243,7 +3241,6 @@ static int emac_remove(struct platform_device *ofdev)
mal_unregister_commac(dev->mal, >commac);
emac_put_deps(dev);
 
-   emac_dbg_unregister(dev);
iounmap(dev->emacp);
 
if (dev->wol_irq)
@@ -3326,9 +3323,6 @@ static int __init emac_init(void)
 
printk(KERN_INFO DRV_DESC ", version " DRV_VERSION "\n");
 
-   /* Init debug stuff */
-   emac_init_debug();
-
/* Build EMAC boot list */
emac_make_bootlist();
 
@@ -3373,7 +3367,6 @@ static void __exit emac_exit(void)
rgmii_exit();
zmii_exit();
mal_exit();
-   emac_fini_debug();
 
/* Destroy EMAC boot list */
for (i = 0; i < EMAC_BOOT_LIST_SIZE; i++)
diff --git a/drivers/net/ethernet/ibm/emac/debug.c 
b/drivers/net/ethernet/ibm/emac/debug.c
deleted file mode 100644
index a559f326bf63..
--- a/drivers/net/ethernet/ibm/emac/debug.c
+++ /dev/null
@@ -1,270 +0,0 @@
-/*
- * drivers/net/ethernet/ibm/emac/debug.c
- *
- * Driver for PowerPC 4xx on-chip ethernet controller, debug print routines.
- *
- * Copyright 2007 Benjamin Herrenschmidt, IBM Corp.
- *
- *
- * Based on the arch/ppc version of the driver:
- *
- * Copyright (c) 2004, 2005 Zultys Technologies
- * Eugene Surovegin  or 
- *
- * This program is free software; you can redistribute  it and/or modify it
- * under  the terms of  the GNU General  Public License as published by the
- * Free Software Foundation;  either version 2 of the  License, or (at your
- * option) any later version.
- *
- */
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include "core.h"
-
-static DEFINE_SPINLOCK(emac_dbg_lock);
-
-static void emac_desc_dump(struct emac_instance *p)
-{
-   int i;
-   printk("** EMAC %s TX BDs **\n"
-  " tx_cnt = %d tx_slot = %d ack_slot = %d\n",
-  p->ofdev->dev.of_node->full_name,
-  p->tx_cnt, p->tx_slot, p->ack_slot);
-   for (i = 0; i < NUM_TX_BUFF / 2; ++i)
-   printk
-   ("bd[%2d] 0x%08x %c 0x%04x %4u - bd[%2d] 0x%08x %c 0x%04x 
%4u\n",
-i, p->tx_desc[i].data_ptr, p->tx_skb[i] ? 'V' : ' ',
-p->tx_desc[i].ctrl, p->tx_desc[i].data_len,
-NUM_TX_BUFF / 2 + i,
-p->tx_desc[NUM_TX_BUFF / 2 + i].data_ptr,
-p->tx_skb[NUM_TX_BUFF / 2 + i] ? 'V' : ' ',
-p->tx_desc[NUM_TX_BUFF / 2 + i].ctrl,
-p->tx_desc[NUM_TX_BUFF / 2 + i].data_len);
-
-   printk("** EMAC %s RX BDs **\n"
-  " rx_slot = %d flags = 0x%lx rx_skb_size = %d rx_sync_size = 
%d\n"
-  " rx_sg_skb = 0x%p\n",
-  p->ofdev->dev.of_node->full_name,
-  p->rx_slot, p->commac.flags, p->rx_skb_size,
-  p->rx_sync_size, p->rx_sg_skb);
-   for (i = 0; i < NUM_RX_BUFF / 2; ++i)
-   printk
-  

[PATCH] net: ibm: emac: remove unused sysrq handler for 'c' key

[Eric Biggers]

From: Eric Biggers 

Since commit d6580a9f1523 ("kexec: sysrq: simplify sysrq-c handler"),
the sysrq handler for the 'c' key has been sysrq_crash_op.  Debugging
code in the ibm_emac driver also tries to register a handler for the 'c'
key, but this has no effect because register_sysrq_key() doesn't replace
existing handlers.  Since evidently no one has cared enough to fix this
in the last 8 years, and it's very rare for drivers to register sysrq
handlers (for good reason), just remove the dead code.

Signed-off-by: Eric Biggers 
---
 drivers/net/ethernet/ibm/emac/Makefile |   1 -
 drivers/net/ethernet/ibm/emac/core.c   |   7 -
 drivers/net/ethernet/ibm/emac/debug.c  | 270 -
 drivers/net/ethernet/ibm/emac/debug.h  |  23 ---
 drivers/net/ethernet/ibm/emac/mal.c|   4 -
 drivers/tty/sysrq.c|   2 +-
 6 files changed, 1 insertion(+), 306 deletions(-)
 delete mode 100644 drivers/net/ethernet/ibm/emac/debug.c

diff --git a/drivers/net/ethernet/ibm/emac/Makefile 
b/drivers/net/ethernet/ibm/emac/Makefile
index eba21835d90d..98768ba0955a 100644
--- a/drivers/net/ethernet/ibm/emac/Makefile
+++ b/drivers/net/ethernet/ibm/emac/Makefile
@@ -8,4 +8,3 @@ ibm_emac-y := mal.o core.o phy.o
 ibm_emac-$(CONFIG_IBM_EMAC_ZMII) += zmii.o
 ibm_emac-$(CONFIG_IBM_EMAC_RGMII) += rgmii.o
 ibm_emac-$(CONFIG_IBM_EMAC_TAH) += tah.o
-ibm_emac-$(CONFIG_IBM_EMAC_DEBUG) += debug.o
diff --git a/drivers/net/ethernet/ibm/emac/core.c 
b/drivers/net/ethernet/ibm/emac/core.c
index c44036d5761a..0f9cd92fdc5e 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -3173,8 +3173,6 @@ static int emac_probe(struct platform_device *ofdev)
printk("%s: found %s PHY (0x%02x)\n", ndev->name,
   dev->phy.def->name, dev->phy.address);
 
-   emac_dbg_register(dev);
-
/* Life is good */
return 0;
 
@@ -3243,7 +3241,6 @@ static int emac_remove(struct platform_device *ofdev)
mal_unregister_commac(dev->mal, >commac);
emac_put_deps(dev);
 
-   emac_dbg_unregister(dev);
iounmap(dev->emacp);
 
if (dev->wol_irq)
@@ -3326,9 +3323,6 @@ static int __init emac_init(void)
 
printk(KERN_INFO DRV_DESC ", version " DRV_VERSION "\n");
 
-   /* Init debug stuff */
-   emac_init_debug();
-
/* Build EMAC boot list */
emac_make_bootlist();
 
@@ -3373,7 +3367,6 @@ static void __exit emac_exit(void)
rgmii_exit();
zmii_exit();
mal_exit();
-   emac_fini_debug();
 
/* Destroy EMAC boot list */
for (i = 0; i < EMAC_BOOT_LIST_SIZE; i++)
diff --git a/drivers/net/ethernet/ibm/emac/debug.c 
b/drivers/net/ethernet/ibm/emac/debug.c
deleted file mode 100644
index a559f326bf63..
--- a/drivers/net/ethernet/ibm/emac/debug.c
+++ /dev/null
@@ -1,270 +0,0 @@
-/*
- * drivers/net/ethernet/ibm/emac/debug.c
- *
- * Driver for PowerPC 4xx on-chip ethernet controller, debug print routines.
- *
- * Copyright 2007 Benjamin Herrenschmidt, IBM Corp.
- *
- *
- * Based on the arch/ppc version of the driver:
- *
- * Copyright (c) 2004, 2005 Zultys Technologies
- * Eugene Surovegin  or 
- *
- * This program is free software; you can redistribute  it and/or modify it
- * under  the terms of  the GNU General  Public License as published by the
- * Free Software Foundation;  either version 2 of the  License, or (at your
- * option) any later version.
- *
- */
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include "core.h"
-
-static DEFINE_SPINLOCK(emac_dbg_lock);
-
-static void emac_desc_dump(struct emac_instance *p)
-{
-   int i;
-   printk("** EMAC %s TX BDs **\n"
-  " tx_cnt = %d tx_slot = %d ack_slot = %d\n",
-  p->ofdev->dev.of_node->full_name,
-  p->tx_cnt, p->tx_slot, p->ack_slot);
-   for (i = 0; i < NUM_TX_BUFF / 2; ++i)
-   printk
-   ("bd[%2d] 0x%08x %c 0x%04x %4u - bd[%2d] 0x%08x %c 0x%04x 
%4u\n",
-i, p->tx_desc[i].data_ptr, p->tx_skb[i] ? 'V' : ' ',
-p->tx_desc[i].ctrl, p->tx_desc[i].data_len,
-NUM_TX_BUFF / 2 + i,
-p->tx_desc[NUM_TX_BUFF / 2 + i].data_ptr,
-p->tx_skb[NUM_TX_BUFF / 2 + i] ? 'V' : ' ',
-p->tx_desc[NUM_TX_BUFF / 2 + i].ctrl,
-p->tx_desc[NUM_TX_BUFF / 2 + i].data_len);
-
-   printk("** EMAC %s RX BDs **\n"
-  " rx_slot = %d flags = 0x%lx rx_skb_size = %d rx_sync_size = 
%d\n"
-  " rx_sg_skb = 0x%p\n",
-  p->ofdev->dev.of_node->full_name,
-  p->rx_slot, p->commac.flags, p->rx_skb_size,
-  p->rx_sync_size, p->rx_sg_skb);
-   for (i = 0; i < NUM_RX_BUFF / 2; ++i)
-   printk
-   ("bd[%2d] 0x%08x %c 0x%04x %4u - bd[%2d] 0x%08x %c 0x%04x 
%4u\n",
-i, 

Re: [PATCH 2/3] pwm: pwm-samsung: fix suspend/resume support

[kbuild test robot]

Hi Bartlomiej,

[auto build test ERROR on hwmon/hwmon-next]
[also build test ERROR on v4.11-rc5 next-20170403]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Bartlomiej-Zolnierkiewicz/pwm-pwm-samsung-fix-suspend-resume-support/20170404-121010
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git 
hwmon-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers//pwm/pwm-samsung.c: In function 'pwm_samsung_resume':
>> drivers//pwm/pwm-samsung.c:626:30: error: passing argument 1 of 
>> 'pwm_samsung_manual_update' from incompatible pointer type 
>> [-Werror=incompatible-pointer-types]
   pwm_samsung_manual_update(chip, pwm);
 ^~~~
   drivers//pwm/pwm-samsung.c:288:13: note: expected 'struct samsung_pwm_chip 
*' but argument is of type 'struct pwm_chip *'
static void pwm_samsung_manual_update(struct samsung_pwm_chip *chip,
^
   cc1: some warnings being treated as errors

vim +/pwm_samsung_manual_update +626 drivers//pwm/pwm-samsung.c

   620  our_chip->inverter_mask & 
BIT(i));
   621  
   622  if (chan->period_ns) {
   623  __pwm_samsung_config(chip, pwm, chan->duty_ns,
   624   chan->period_ns, true);
   625  /* needed to make PWM disable work on 
Odroid-XU3 */
 > 626  pwm_samsung_manual_update(chip, pwm);
   627  }
   628  
   629  if (our_chip->disabled_mask & BIT(i))

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH 2/3] pwm: pwm-samsung: fix suspend/resume support

[kbuild test robot]

Hi Bartlomiej,

[auto build test ERROR on hwmon/hwmon-next]
[also build test ERROR on v4.11-rc5 next-20170403]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Bartlomiej-Zolnierkiewicz/pwm-pwm-samsung-fix-suspend-resume-support/20170404-121010
base:   
https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git 
hwmon-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers//pwm/pwm-samsung.c: In function 'pwm_samsung_resume':
>> drivers//pwm/pwm-samsung.c:626:30: error: passing argument 1 of 
>> 'pwm_samsung_manual_update' from incompatible pointer type 
>> [-Werror=incompatible-pointer-types]
   pwm_samsung_manual_update(chip, pwm);
 ^~~~
   drivers//pwm/pwm-samsung.c:288:13: note: expected 'struct samsung_pwm_chip 
*' but argument is of type 'struct pwm_chip *'
static void pwm_samsung_manual_update(struct samsung_pwm_chip *chip,
^
   cc1: some warnings being treated as errors

vim +/pwm_samsung_manual_update +626 drivers//pwm/pwm-samsung.c

   620  our_chip->inverter_mask & 
BIT(i));
   621  
   622  if (chan->period_ns) {
   623  __pwm_samsung_config(chip, pwm, chan->duty_ns,
   624   chan->period_ns, true);
   625  /* needed to make PWM disable work on 
Odroid-XU3 */
 > 626  pwm_samsung_manual_update(chip, pwm);
   627  }
   628  
   629  if (our_chip->disabled_mask & BIT(i))

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH 4/5] zram: remove zram_meta structure

[Minchan Kim]

On Tue, Apr 04, 2017 at 11:31:15AM +0900, Sergey Senozhatsky wrote:
< snip >

> 
> [..]
> > -struct zram_meta {
> > +struct zram {
> > struct zram_table_entry *table;
> > struct zs_pool *mem_pool;
> > -};
> > -
> > -struct zram {
> > -   struct zram_meta *meta;
> > struct zcomp *comp;
> > struct gendisk *disk;
> > /* Prevent concurrent execution of device init */
> 
> 
> we still have several zram_meta_FOO() left overs in zram_drv.c

I intentionally used that term because I don't think better name
to wrap logis which allocates table and mempool.
Feel free to suggest if you have better.

Thanks.

Re: [PATCH 4/5] zram: remove zram_meta structure

[Minchan Kim]

On Tue, Apr 04, 2017 at 11:31:15AM +0900, Sergey Senozhatsky wrote:
< snip >

> 
> [..]
> > -struct zram_meta {
> > +struct zram {
> > struct zram_table_entry *table;
> > struct zs_pool *mem_pool;
> > -};
> > -
> > -struct zram {
> > -   struct zram_meta *meta;
> > struct zcomp *comp;
> > struct gendisk *disk;
> > /* Prevent concurrent execution of device init */
> 
> 
> we still have several zram_meta_FOO() left overs in zram_drv.c

I intentionally used that term because I don't think better name
to wrap logis which allocates table and mempool.
Feel free to suggest if you have better.

Thanks.

Re: [PATCH v2 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[kbuild test robot]

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: i386-randconfig-r0-201714 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from drivers/usb/dwc3/dwc3-of-simple.c:32:0:
   include/linux/reset.h:106:1: error: expected identifier or '(' before '{' 
token
{
^
   include/linux/reset.h: In function 'devm_reset_control_array_get':
   include/linux/reset.h:127:9: warning: return makes integer from pointer 
without a cast [-Wint-conversion]
 return optional ? NULL : ERR_PTR(-ENOTSUPP);
^
   include/linux/reset.h: In function 'of_reset_control_array_get':
   include/linux/reset.h:135:9: warning: return makes integer from pointer 
without a cast [-Wint-conversion]
 return optional ? NULL : ERR_PTR(-ENOTSUPP);
^
   drivers/usb/dwc3/dwc3-of-simple.c: At top level:
>> include/linux/reset.h:105:19: warning: 'of_reset_control_get_count' used but 
>> never defined
static inline int of_reset_control_get_count(struct device_node *node);
  ^

vim +/of_reset_control_get_count +105 include/linux/reset.h

bb475230 Ramiro Oliveira 2017-01-13   99
struct device *dev, const char *id,
bb475230 Ramiro Oliveira 2017-01-13  100
int index, bool shared, bool optional)
5bcd0b7f Axel Lin2015-09-01  101  {
0ca10b60 Philipp Zabel   2017-03-20  102return optional ? NULL : 
ERR_PTR(-ENOTSUPP);
5bcd0b7f Axel Lin2015-09-01  103  }
5bcd0b7f Axel Lin2015-09-01  104  
045cc544 Vivek Gautam2017-04-03 @105  static inline int 
of_reset_control_get_count(struct device_node *node);
045cc544 Vivek Gautam2017-04-03  106  {
045cc544 Vivek Gautam2017-04-03  107return -ENOTSUPP;
045cc544 Vivek Gautam2017-04-03  108  }
045cc544 Vivek Gautam2017-04-03  109  
d5652c65 Vivek Gautam2017-04-03  110  static inline int 
reset_control_array_assert(int num_rsts,
d5652c65 Vivek Gautam2017-04-03  111struct 
reset_control_array *resets)
d5652c65 Vivek Gautam2017-04-03  112  {
d5652c65 Vivek Gautam2017-04-03  113return 0;
d5652c65 Vivek Gautam2017-04-03  114  }
d5652c65 Vivek Gautam2017-04-03  115  
d5652c65 Vivek Gautam2017-04-03  116  static inline int 
reset_control_array_deassert(int num_rsts,
d5652c65 Vivek Gautam2017-04-03  117struct 
reset_control_array *resets)
d5652c65 Vivek Gautam2017-04-03  118  {
d5652c65 Vivek Gautam2017-04-03  119return 0;
d5652c65 Vivek Gautam2017-04-03  120  }
d5652c65 Vivek Gautam2017-04-03  121  
d5652c65 Vivek Gautam2017-04-03  122  static inline
d5652c65 Vivek Gautam2017-04-03  123  int 
devm_reset_control_array_get(struct device *dev, int num_rsts,
d5652c65 Vivek Gautam2017-04-03  124struct 
reset_control_array *resets,
d5652c65 Vivek Gautam2017-04-03  125bool 
shared, bool optional)
d5652c65 Vivek Gautam2017-04-03  126  {
d5652c65 Vivek Gautam2017-04-03 @127return optional ? NULL : 
ERR_PTR(-ENOTSUPP);
d5652c65 Vivek Gautam2017-04-03  128  }
d5652c65 Vivek Gautam2017-04-03  129  
d5652c65 Vivek Gautam2017-04-03  130  static inline

:: The code at line 105 was first introduced by commit
:: 045cc544119a744cda92da0644066f18cb3760b4 reset: Add API to count number 
of reset available with device

:: TO: Vivek Gautam <vivek.gau...@codeaurora.org>
:: CC: 0day robot <fengguang...@intel.com>

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v2 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[kbuild test robot]

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: i386-randconfig-r0-201714 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from drivers/usb/dwc3/dwc3-of-simple.c:32:0:
   include/linux/reset.h:106:1: error: expected identifier or '(' before '{' 
token
{
^
   include/linux/reset.h: In function 'devm_reset_control_array_get':
   include/linux/reset.h:127:9: warning: return makes integer from pointer 
without a cast [-Wint-conversion]
 return optional ? NULL : ERR_PTR(-ENOTSUPP);
^
   include/linux/reset.h: In function 'of_reset_control_array_get':
   include/linux/reset.h:135:9: warning: return makes integer from pointer 
without a cast [-Wint-conversion]
 return optional ? NULL : ERR_PTR(-ENOTSUPP);
^
   drivers/usb/dwc3/dwc3-of-simple.c: At top level:
>> include/linux/reset.h:105:19: warning: 'of_reset_control_get_count' used but 
>> never defined
static inline int of_reset_control_get_count(struct device_node *node);
  ^

vim +/of_reset_control_get_count +105 include/linux/reset.h

bb475230 Ramiro Oliveira 2017-01-13   99
struct device *dev, const char *id,
bb475230 Ramiro Oliveira 2017-01-13  100
int index, bool shared, bool optional)
5bcd0b7f Axel Lin2015-09-01  101  {
0ca10b60 Philipp Zabel   2017-03-20  102return optional ? NULL : 
ERR_PTR(-ENOTSUPP);
5bcd0b7f Axel Lin2015-09-01  103  }
5bcd0b7f Axel Lin2015-09-01  104  
045cc544 Vivek Gautam2017-04-03 @105  static inline int 
of_reset_control_get_count(struct device_node *node);
045cc544 Vivek Gautam2017-04-03  106  {
045cc544 Vivek Gautam2017-04-03  107return -ENOTSUPP;
045cc544 Vivek Gautam2017-04-03  108  }
045cc544 Vivek Gautam2017-04-03  109  
d5652c65 Vivek Gautam2017-04-03  110  static inline int 
reset_control_array_assert(int num_rsts,
d5652c65 Vivek Gautam2017-04-03  111struct 
reset_control_array *resets)
d5652c65 Vivek Gautam2017-04-03  112  {
d5652c65 Vivek Gautam2017-04-03  113return 0;
d5652c65 Vivek Gautam2017-04-03  114  }
d5652c65 Vivek Gautam2017-04-03  115  
d5652c65 Vivek Gautam2017-04-03  116  static inline int 
reset_control_array_deassert(int num_rsts,
d5652c65 Vivek Gautam2017-04-03  117struct 
reset_control_array *resets)
d5652c65 Vivek Gautam2017-04-03  118  {
d5652c65 Vivek Gautam2017-04-03  119return 0;
d5652c65 Vivek Gautam2017-04-03  120  }
d5652c65 Vivek Gautam2017-04-03  121  
d5652c65 Vivek Gautam2017-04-03  122  static inline
d5652c65 Vivek Gautam2017-04-03  123  int 
devm_reset_control_array_get(struct device *dev, int num_rsts,
d5652c65 Vivek Gautam2017-04-03  124struct 
reset_control_array *resets,
d5652c65 Vivek Gautam2017-04-03  125bool 
shared, bool optional)
d5652c65 Vivek Gautam2017-04-03  126  {
d5652c65 Vivek Gautam2017-04-03 @127return optional ? NULL : 
ERR_PTR(-ENOTSUPP);
d5652c65 Vivek Gautam2017-04-03  128  }
d5652c65 Vivek Gautam2017-04-03  129  
d5652c65 Vivek Gautam2017-04-03  130  static inline

:: The code at line 105 was first introduced by commit
:: 045cc544119a744cda92da0644066f18cb3760b4 reset: Add API to count number 
of reset available with device

:: TO: Vivek Gautam 
:: CC: 0day robot 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH 11/16] fpga: intel: fme: add partial reconfiguration sub feature support

[Wu Hao]

On Mon, Apr 03, 2017 at 03:26:14PM -0500, Alan Tull wrote:
> On Fri, Mar 31, 2017 at 3:50 AM, Wu Hao  wrote:
> > On Fri, Mar 31, 2017 at 12:11:12PM +0800, Xiao Guangrong wrote:
> >> On 31/03/2017 4:30 AM, Alan Tull wrote:
> >> >On Thu, Mar 30, 2017 at 7:08 AM, Wu Hao  wrote:
> >> >>From: Kang Luwei 
> >> >>
> >> >>Partial Reconfiguration (PR) is the most important function for FME. It
> >> >>allows reconfiguration for given Port/Accelerated Function Unit (AFU).
> >> >>
> >> >>This patch adds support for PR sub feature. In this patch, it registers
> >> >>a fpga_mgr and implements fpga_manager_ops, and invoke fpga_mgr_buf_load
> >> >>for PR operation once PR request received via ioctl. Below user space
> >> >>interfaces are exposed by this sub feature.
> >> >>
> >> >>Sysfs interface:
> >> >>* /sys/class/fpga///interface_id
> >> >>  Read-only. Indicate the hardware interface information. Userspace
> >> >>  applications need to check this interface to select correct green
> >> >>  bitstream format before PR.
> >> >>
> >> >>Ioctl interface:
> >> >>* FPGA_FME_PORT_PR
> >> >>  Do partial reconfiguration per information from userspace, including
> >> >>  target port(AFU), buffer size and address info. It returns the PR 
> >> >> status
> >> >>  (PR error code if failed) to userspace.
> >> >>
> >> >>Signed-off-by: Tim Whisonant 
> >> >>Signed-off-by: Enno Luebbers 
> >> >>Signed-off-by: Shiva Rao 
> >> >>Signed-off-by: Christopher Rauer 
> >> >>Signed-off-by: Alan Tull 
> >> >
> >> >Hi Wu Hao,
> >> >
> >> >Thanks for submitting your patches.
> >> >
> >> >I think there's been a misunderstanding of the meaning of
> >> >'Signed-off-by' [1].  I have not signed off on this code or had a hand
> >> >in its development.  But I'm happy to get to review it now.  It will
> >> >take a bit of time; I expect to be replying next week.
> >>
> >> Hi Alan,
> >>
> >> Sorry to confuse you, i think it's because you helped Chris a lot to
> >> implement this interface and we'd like to include your credit as this
> >> way. If you dislike, it will be dropped. :)
> >>
> >> Thanks for your review in advance.
> >>
> >
> > Hi Alan,
> >
> > Sorry about this, we should ask you firstly before doing it this way.
> > Let me know if you don't like it, I will drop it in the next version.
> 
> Yes please drop the signed-off-by: me in the next version.  Also, you
> don't need to cc my Intel email address.
> 

Sure, will do.

Hao

Re: [PATCH 11/16] fpga: intel: fme: add partial reconfiguration sub feature support

[Wu Hao]

On Mon, Apr 03, 2017 at 03:26:14PM -0500, Alan Tull wrote:
> On Fri, Mar 31, 2017 at 3:50 AM, Wu Hao  wrote:
> > On Fri, Mar 31, 2017 at 12:11:12PM +0800, Xiao Guangrong wrote:
> >> On 31/03/2017 4:30 AM, Alan Tull wrote:
> >> >On Thu, Mar 30, 2017 at 7:08 AM, Wu Hao  wrote:
> >> >>From: Kang Luwei 
> >> >>
> >> >>Partial Reconfiguration (PR) is the most important function for FME. It
> >> >>allows reconfiguration for given Port/Accelerated Function Unit (AFU).
> >> >>
> >> >>This patch adds support for PR sub feature. In this patch, it registers
> >> >>a fpga_mgr and implements fpga_manager_ops, and invoke fpga_mgr_buf_load
> >> >>for PR operation once PR request received via ioctl. Below user space
> >> >>interfaces are exposed by this sub feature.
> >> >>
> >> >>Sysfs interface:
> >> >>* /sys/class/fpga///interface_id
> >> >>  Read-only. Indicate the hardware interface information. Userspace
> >> >>  applications need to check this interface to select correct green
> >> >>  bitstream format before PR.
> >> >>
> >> >>Ioctl interface:
> >> >>* FPGA_FME_PORT_PR
> >> >>  Do partial reconfiguration per information from userspace, including
> >> >>  target port(AFU), buffer size and address info. It returns the PR 
> >> >> status
> >> >>  (PR error code if failed) to userspace.
> >> >>
> >> >>Signed-off-by: Tim Whisonant 
> >> >>Signed-off-by: Enno Luebbers 
> >> >>Signed-off-by: Shiva Rao 
> >> >>Signed-off-by: Christopher Rauer 
> >> >>Signed-off-by: Alan Tull 
> >> >
> >> >Hi Wu Hao,
> >> >
> >> >Thanks for submitting your patches.
> >> >
> >> >I think there's been a misunderstanding of the meaning of
> >> >'Signed-off-by' [1].  I have not signed off on this code or had a hand
> >> >in its development.  But I'm happy to get to review it now.  It will
> >> >take a bit of time; I expect to be replying next week.
> >>
> >> Hi Alan,
> >>
> >> Sorry to confuse you, i think it's because you helped Chris a lot to
> >> implement this interface and we'd like to include your credit as this
> >> way. If you dislike, it will be dropped. :)
> >>
> >> Thanks for your review in advance.
> >>
> >
> > Hi Alan,
> >
> > Sorry about this, we should ask you firstly before doing it this way.
> > Let me know if you don't like it, I will drop it in the next version.
> 
> Yes please drop the signed-off-by: me in the next version.  Also, you
> don't need to cc my Intel email address.
> 

Sure, will do.

Hao

Re: [PATCH 01/16] docs: fpga: add a document for Intel FPGA driver overview

[Wu Hao]

On Mon, Apr 03, 2017 at 03:44:17PM -0500, Alan Tull wrote:
> On Sun, Apr 2, 2017 at 9:41 AM, Moritz Fischer  wrote:
> > On Sat, Apr 01, 2017 at 07:16:19PM +0800, Wu Hao wrote:
> >> On Fri, Mar 31, 2017 at 01:38:06PM -0500, Alan Tull wrote:
> >> > On Fri, Mar 31, 2017 at 1:24 PM,   
> >> > wrote:
> >> > >
> >> > >
> >> > > On Thu, 30 Mar 2017, Wu Hao wrote:
> >> > >
> >> > >
> >> > > Hi Wu Hao,
> >> > >
> >> > > Great documentation. I'm looking forward to diving into the rest of the
> >> > > patches. Please see my comments inline.
> >> > >
> >> > > Matthew Gerlach
> >> > >
> >> > >
> >> > >> Add a document for Intel FPGA driver overview.
> >> > >>
> >> > >> Signed-off-by: Enno Luebbers 
> >> > >> Signed-off-by: Xiao Guangrong 
> >> > >> Signed-off-by: Wu Hao 
> >> > >> ---
> >> > >> Documentation/fpga/intel-fpga.txt | 259
> >> > >> ++
> >> > >> 1 file changed, 259 insertions(+)
> >> > >> create mode 100644 Documentation/fpga/intel-fpga.txt
> >> > >>
> >> > >> diff --git a/Documentation/fpga/intel-fpga.txt
> >> > >> b/Documentation/fpga/intel-fpga.txt
> >> > >> new file mode 100644
> >> > >> index 000..9396cea
> >> > >> --- /dev/null
> >> > >> +++ b/Documentation/fpga/intel-fpga.txt
> >> > >> @@ -0,0 +1,259 @@
> >> > >>
> >> > >> +===
> >> > >> +Intel FPGA driver Overview
> >> > >>
> >> > >> +---
> >> > >> +Enno Luebbers 
> >> > >> +Xiao Guangrong 
> >> > >> +Wu Hao 
> >> > >> +
> >> > >> +The Intel FPGA driver provides interfaces for userspace applications 
> >> > >> to
> >> > >> +configure, enumerate, open, and access FPGA accelerators on platforms
> >> > >> equipped
> >> > >> +with Intel(R) FPGA solutions and enables system level management
> >> > >> functions such
> >> > >> +as FPGA reconfiguration, power management, and virtualization.
> >> > >> +
> >> > >
> >> > >
> >> > > From a Linux kernel perspective, I'm not sure this is the best name for
> >> > > this code.  The name gives me the impression that it is a driver for 
> >> > > all
> >> > > Intel FPGAs, but not all Intel FPGAs are connected to the processor 
> >> > > over a
> >> > > PCIe bus.  The processor could be directely connected like the Arria10
> >> > > SOCFPGA.  Such a processor could certainly benefit from this 
> >> > > accelerator
> >> > > usage model.  In an extreme case, couldn't a processor in the FPGA,
> >> > > running Linux, also benefit from this accelerator model?  Is this code 
> >> > > a
> >> > > "FPGA Accelerator Framework"?
> >> > >
> >> > >> +HW Architecture
> >> > >> +===
> >> > >> +From the OS's point of view, the FPGA hardware appears as a regular 
> >> > >> PCIe
> >> > >> device.
> >> > >> +The FPGA device memory is organized using a predefined data structure
> >> > >> (Device
> >> > >> +Feature List). Features supported by the particular FPGA device are
> >> > >> exposed
> >> > >> +through these data structures, as illustrated below:
> >> > >> +
> >> > >> +  +---+  +-+
> >> > >> +  |  PF   |  | VF  |
> >> > >> +  +---+  +-+
> >> > >> +  ^^ ^  ^
> >> > >> +  || |  |
> >> > >> ++-||-|--|---+
> >> > >> +| || |  |   |
> >> > >> +|  +-+ +---+ +---+  +---+   |
> >> > >> +|  | FME | | Port0 | | Port1 |  | Port2 |   |
> >> > >> +|  +-+ +---+ +---+  +---+   |
> >> > >> +|  ^ ^  ^   |
> >> > >> +|  | |  |   |
> >> > >> +|  +---+ +--+   +---+   |
> >> > >> +|  |  AFU  | |  AFU |   |  AFU  |   |
> >> > >> +|  +---+ +--+   +---+   |
> >> > >> +|   |
> >> > >> +| FPGA PCIe Device  |
> >> > >> ++---+
> >> > >> +
> >> > >> +The driver supports PCIe SR-IOV to create virtual functions (VFs) 
> >> > >> which
> >> > >> can be
> >> > >> +used to assign individual accelerators to virtual machines .
> >> > >
> >> > >
> >> > > Does this HW Architecture require an Intel FPGA?  Couldn't any vendors 
> >> > > FPGA
> >> > > be used as long as it presented itself the PCIe bus the same and 
> >> > > contained
> >> > > an appropriate Device Feature List?
> >
> > I think this is a good (and important) 

Re: [PATCH 01/16] docs: fpga: add a document for Intel FPGA driver overview

[Wu Hao]

On Mon, Apr 03, 2017 at 03:44:17PM -0500, Alan Tull wrote:
> On Sun, Apr 2, 2017 at 9:41 AM, Moritz Fischer  wrote:
> > On Sat, Apr 01, 2017 at 07:16:19PM +0800, Wu Hao wrote:
> >> On Fri, Mar 31, 2017 at 01:38:06PM -0500, Alan Tull wrote:
> >> > On Fri, Mar 31, 2017 at 1:24 PM,   
> >> > wrote:
> >> > >
> >> > >
> >> > > On Thu, 30 Mar 2017, Wu Hao wrote:
> >> > >
> >> > >
> >> > > Hi Wu Hao,
> >> > >
> >> > > Great documentation. I'm looking forward to diving into the rest of the
> >> > > patches. Please see my comments inline.
> >> > >
> >> > > Matthew Gerlach
> >> > >
> >> > >
> >> > >> Add a document for Intel FPGA driver overview.
> >> > >>
> >> > >> Signed-off-by: Enno Luebbers 
> >> > >> Signed-off-by: Xiao Guangrong 
> >> > >> Signed-off-by: Wu Hao 
> >> > >> ---
> >> > >> Documentation/fpga/intel-fpga.txt | 259
> >> > >> ++
> >> > >> 1 file changed, 259 insertions(+)
> >> > >> create mode 100644 Documentation/fpga/intel-fpga.txt
> >> > >>
> >> > >> diff --git a/Documentation/fpga/intel-fpga.txt
> >> > >> b/Documentation/fpga/intel-fpga.txt
> >> > >> new file mode 100644
> >> > >> index 000..9396cea
> >> > >> --- /dev/null
> >> > >> +++ b/Documentation/fpga/intel-fpga.txt
> >> > >> @@ -0,0 +1,259 @@
> >> > >>
> >> > >> +===
> >> > >> +Intel FPGA driver Overview
> >> > >>
> >> > >> +---
> >> > >> +Enno Luebbers 
> >> > >> +Xiao Guangrong 
> >> > >> +Wu Hao 
> >> > >> +
> >> > >> +The Intel FPGA driver provides interfaces for userspace applications 
> >> > >> to
> >> > >> +configure, enumerate, open, and access FPGA accelerators on platforms
> >> > >> equipped
> >> > >> +with Intel(R) FPGA solutions and enables system level management
> >> > >> functions such
> >> > >> +as FPGA reconfiguration, power management, and virtualization.
> >> > >> +
> >> > >
> >> > >
> >> > > From a Linux kernel perspective, I'm not sure this is the best name for
> >> > > this code.  The name gives me the impression that it is a driver for 
> >> > > all
> >> > > Intel FPGAs, but not all Intel FPGAs are connected to the processor 
> >> > > over a
> >> > > PCIe bus.  The processor could be directely connected like the Arria10
> >> > > SOCFPGA.  Such a processor could certainly benefit from this 
> >> > > accelerator
> >> > > usage model.  In an extreme case, couldn't a processor in the FPGA,
> >> > > running Linux, also benefit from this accelerator model?  Is this code 
> >> > > a
> >> > > "FPGA Accelerator Framework"?
> >> > >
> >> > >> +HW Architecture
> >> > >> +===
> >> > >> +From the OS's point of view, the FPGA hardware appears as a regular 
> >> > >> PCIe
> >> > >> device.
> >> > >> +The FPGA device memory is organized using a predefined data structure
> >> > >> (Device
> >> > >> +Feature List). Features supported by the particular FPGA device are
> >> > >> exposed
> >> > >> +through these data structures, as illustrated below:
> >> > >> +
> >> > >> +  +---+  +-+
> >> > >> +  |  PF   |  | VF  |
> >> > >> +  +---+  +-+
> >> > >> +  ^^ ^  ^
> >> > >> +  || |  |
> >> > >> ++-||-|--|---+
> >> > >> +| || |  |   |
> >> > >> +|  +-+ +---+ +---+  +---+   |
> >> > >> +|  | FME | | Port0 | | Port1 |  | Port2 |   |
> >> > >> +|  +-+ +---+ +---+  +---+   |
> >> > >> +|  ^ ^  ^   |
> >> > >> +|  | |  |   |
> >> > >> +|  +---+ +--+   +---+   |
> >> > >> +|  |  AFU  | |  AFU |   |  AFU  |   |
> >> > >> +|  +---+ +--+   +---+   |
> >> > >> +|   |
> >> > >> +| FPGA PCIe Device  |
> >> > >> ++---+
> >> > >> +
> >> > >> +The driver supports PCIe SR-IOV to create virtual functions (VFs) 
> >> > >> which
> >> > >> can be
> >> > >> +used to assign individual accelerators to virtual machines .
> >> > >
> >> > >
> >> > > Does this HW Architecture require an Intel FPGA?  Couldn't any vendors 
> >> > > FPGA
> >> > > be used as long as it presented itself the PCIe bus the same and 
> >> > > contained
> >> > > an appropriate Device Feature List?
> >
> > I think this is a good (and important) point. Especially when sysfs
> > entries & ioctls constituting ABI depend on it.
> >
> >> > >
> >> > >> +
> >> > >> +FME (FPGA Management Engine)
> >> > >> +
> >> > >> 

[PATCH] usb: dwc3: of-simple: fix noderef.cocci warnings

[kbuild test robot]

drivers/usb/dwc3/dwc3-of-simple.c:53:43-49: ERROR: application of sizeof to 
pointer

 sizeof when applied to a pointer typed expression gives the size of
 the pointer

Generated by: scripts/coccinelle/misc/noderef.cocci

CC: Vivek Gautam 
Signed-off-by: Fengguang Wu 
---

 dwc3-of-simple.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -50,7 +50,7 @@ static int dwc3_of_simple_reset_init(str
 
simple->num_resets = count;
 
-   simple->resets = devm_kcalloc(dev, count, sizeof(simple->resets),
+   simple->resets = devm_kcalloc(dev, count, sizeof(*simple->resets),
GFP_KERNEL);
if (!simple->resets)
return -ENOMEM;

[PATCH] usb: dwc3: of-simple: fix noderef.cocci warnings

[kbuild test robot]

drivers/usb/dwc3/dwc3-of-simple.c:53:43-49: ERROR: application of sizeof to 
pointer

 sizeof when applied to a pointer typed expression gives the size of
 the pointer

Generated by: scripts/coccinelle/misc/noderef.cocci

CC: Vivek Gautam 
Signed-off-by: Fengguang Wu 
---

 dwc3-of-simple.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -50,7 +50,7 @@ static int dwc3_of_simple_reset_init(str
 
simple->num_resets = count;
 
-   simple->resets = devm_kcalloc(dev, count, sizeof(simple->resets),
+   simple->resets = devm_kcalloc(dev, count, sizeof(*simple->resets),
GFP_KERNEL);
if (!simple->resets)
return -ENOMEM;

Re: [PATCH v2 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[kbuild test robot]

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next


coccinelle warnings: (new ones prefixed by >>)

>> drivers/usb/dwc3/dwc3-of-simple.c:53:43-49: ERROR: application of sizeof to 
>> pointer

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH v2 3/4] usb: dwc3: of-simple: Add support to get resets for the device

[kbuild test robot]

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next


coccinelle warnings: (new ones prefixed by >>)

>> drivers/usb/dwc3/dwc3-of-simple.c:53:43-49: ERROR: application of sizeof to 
>> pointer

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH 01/16] docs: fpga: add a document for Intel FPGA driver overview

[Wu Hao]

On Sun, Apr 02, 2017 at 07:41:46AM -0700, Moritz Fischer wrote:
> On Sat, Apr 01, 2017 at 07:16:19PM +0800, Wu Hao wrote:
> > On Fri, Mar 31, 2017 at 01:38:06PM -0500, Alan Tull wrote:
> > > On Fri, Mar 31, 2017 at 1:24 PM,   wrote:
> > > >
> > > >
> > > > On Thu, 30 Mar 2017, Wu Hao wrote:
> > > >
> > > >
> > > > Hi Wu Hao,
> > > >
> > > > Great documentation. I'm looking forward to diving into the rest of the
> > > > patches. Please see my comments inline.
> > > >
> > > > Matthew Gerlach
> > > >
> > > >
> > > >> Add a document for Intel FPGA driver overview.
> > > >>
> > > >> Signed-off-by: Enno Luebbers 
> > > >> Signed-off-by: Xiao Guangrong 
> > > >> Signed-off-by: Wu Hao 
> > > >> ---
> > > >> Documentation/fpga/intel-fpga.txt | 259
> > > >> ++
> > > >> 1 file changed, 259 insertions(+)
> > > >> create mode 100644 Documentation/fpga/intel-fpga.txt
> > > >>
> > > >> diff --git a/Documentation/fpga/intel-fpga.txt
> > > >> b/Documentation/fpga/intel-fpga.txt
> > > >> new file mode 100644
> > > >> index 000..9396cea
> > > >> --- /dev/null
> > > >> +++ b/Documentation/fpga/intel-fpga.txt
> > > >> @@ -0,0 +1,259 @@
> > > >>
> > > >> +===
> > > >> +Intel FPGA driver Overview
> > > >>
> > > >> +---
> > > >> +Enno Luebbers 
> > > >> +Xiao Guangrong 
> > > >> +Wu Hao 
> > > >> +
> > > >> +The Intel FPGA driver provides interfaces for userspace applications 
> > > >> to
> > > >> +configure, enumerate, open, and access FPGA accelerators on platforms
> > > >> equipped
> > > >> +with Intel(R) FPGA solutions and enables system level management
> > > >> functions such
> > > >> +as FPGA reconfiguration, power management, and virtualization.
> > > >> +
> > > >
> > > >
> > > > From a Linux kernel perspective, I'm not sure this is the best name for
> > > > this code.  The name gives me the impression that it is a driver for all
> > > > Intel FPGAs, but not all Intel FPGAs are connected to the processor 
> > > > over a
> > > > PCIe bus.  The processor could be directely connected like the Arria10
> > > > SOCFPGA.  Such a processor could certainly benefit from this accelerator
> > > > usage model.  In an extreme case, couldn't a processor in the FPGA,
> > > > running Linux, also benefit from this accelerator model?  Is this code a
> > > > "FPGA Accelerator Framework"?
> > > >
> > > >> +HW Architecture
> > > >> +===
> > > >> +From the OS's point of view, the FPGA hardware appears as a regular 
> > > >> PCIe
> > > >> device.
> > > >> +The FPGA device memory is organized using a predefined data structure
> > > >> (Device
> > > >> +Feature List). Features supported by the particular FPGA device are
> > > >> exposed
> > > >> +through these data structures, as illustrated below:
> > > >> +
> > > >> +  +---+  +-+
> > > >> +  |  PF   |  | VF  |
> > > >> +  +---+  +-+
> > > >> +  ^^ ^  ^
> > > >> +  || |  |
> > > >> ++-||-|--|---+
> > > >> +| || |  |   |
> > > >> +|  +-+ +---+ +---+  +---+   |
> > > >> +|  | FME | | Port0 | | Port1 |  | Port2 |   |
> > > >> +|  +-+ +---+ +---+  +---+   |
> > > >> +|  ^ ^  ^   |
> > > >> +|  | |  |   |
> > > >> +|  +---+ +--+   +---+   |
> > > >> +|  |  AFU  | |  AFU |   |  AFU  |   |
> > > >> +|  +---+ +--+   +---+   |
> > > >> +|   |
> > > >> +| FPGA PCIe Device  |
> > > >> ++---+
> > > >> +
> > > >> +The driver supports PCIe SR-IOV to create virtual functions (VFs) 
> > > >> which
> > > >> can be
> > > >> +used to assign individual accelerators to virtual machines .
> > > >
> > > >
> > > > Does this HW Architecture require an Intel FPGA?  Couldn't any vendors 
> > > > FPGA
> > > > be used as long as it presented itself the PCIe bus the same and 
> > > > contained
> > > > an appropriate Device Feature List?
> 
> I think this is a good (and important) point. Especially when sysfs
> entries & ioctls constituting ABI depend on it.

Thanks for your feedback and comments.

I'm not sure if any vendors FPGA will resue the same. But if the same FME
or Port/AFU module 

Re: [PATCH 01/16] docs: fpga: add a document for Intel FPGA driver overview

[Wu Hao]

On Sun, Apr 02, 2017 at 07:41:46AM -0700, Moritz Fischer wrote:
> On Sat, Apr 01, 2017 at 07:16:19PM +0800, Wu Hao wrote:
> > On Fri, Mar 31, 2017 at 01:38:06PM -0500, Alan Tull wrote:
> > > On Fri, Mar 31, 2017 at 1:24 PM,   wrote:
> > > >
> > > >
> > > > On Thu, 30 Mar 2017, Wu Hao wrote:
> > > >
> > > >
> > > > Hi Wu Hao,
> > > >
> > > > Great documentation. I'm looking forward to diving into the rest of the
> > > > patches. Please see my comments inline.
> > > >
> > > > Matthew Gerlach
> > > >
> > > >
> > > >> Add a document for Intel FPGA driver overview.
> > > >>
> > > >> Signed-off-by: Enno Luebbers 
> > > >> Signed-off-by: Xiao Guangrong 
> > > >> Signed-off-by: Wu Hao 
> > > >> ---
> > > >> Documentation/fpga/intel-fpga.txt | 259
> > > >> ++
> > > >> 1 file changed, 259 insertions(+)
> > > >> create mode 100644 Documentation/fpga/intel-fpga.txt
> > > >>
> > > >> diff --git a/Documentation/fpga/intel-fpga.txt
> > > >> b/Documentation/fpga/intel-fpga.txt
> > > >> new file mode 100644
> > > >> index 000..9396cea
> > > >> --- /dev/null
> > > >> +++ b/Documentation/fpga/intel-fpga.txt
> > > >> @@ -0,0 +1,259 @@
> > > >>
> > > >> +===
> > > >> +Intel FPGA driver Overview
> > > >>
> > > >> +---
> > > >> +Enno Luebbers 
> > > >> +Xiao Guangrong 
> > > >> +Wu Hao 
> > > >> +
> > > >> +The Intel FPGA driver provides interfaces for userspace applications 
> > > >> to
> > > >> +configure, enumerate, open, and access FPGA accelerators on platforms
> > > >> equipped
> > > >> +with Intel(R) FPGA solutions and enables system level management
> > > >> functions such
> > > >> +as FPGA reconfiguration, power management, and virtualization.
> > > >> +
> > > >
> > > >
> > > > From a Linux kernel perspective, I'm not sure this is the best name for
> > > > this code.  The name gives me the impression that it is a driver for all
> > > > Intel FPGAs, but not all Intel FPGAs are connected to the processor 
> > > > over a
> > > > PCIe bus.  The processor could be directely connected like the Arria10
> > > > SOCFPGA.  Such a processor could certainly benefit from this accelerator
> > > > usage model.  In an extreme case, couldn't a processor in the FPGA,
> > > > running Linux, also benefit from this accelerator model?  Is this code a
> > > > "FPGA Accelerator Framework"?
> > > >
> > > >> +HW Architecture
> > > >> +===
> > > >> +From the OS's point of view, the FPGA hardware appears as a regular 
> > > >> PCIe
> > > >> device.
> > > >> +The FPGA device memory is organized using a predefined data structure
> > > >> (Device
> > > >> +Feature List). Features supported by the particular FPGA device are
> > > >> exposed
> > > >> +through these data structures, as illustrated below:
> > > >> +
> > > >> +  +---+  +-+
> > > >> +  |  PF   |  | VF  |
> > > >> +  +---+  +-+
> > > >> +  ^^ ^  ^
> > > >> +  || |  |
> > > >> ++-||-|--|---+
> > > >> +| || |  |   |
> > > >> +|  +-+ +---+ +---+  +---+   |
> > > >> +|  | FME | | Port0 | | Port1 |  | Port2 |   |
> > > >> +|  +-+ +---+ +---+  +---+   |
> > > >> +|  ^ ^  ^   |
> > > >> +|  | |  |   |
> > > >> +|  +---+ +--+   +---+   |
> > > >> +|  |  AFU  | |  AFU |   |  AFU  |   |
> > > >> +|  +---+ +--+   +---+   |
> > > >> +|   |
> > > >> +| FPGA PCIe Device  |
> > > >> ++---+
> > > >> +
> > > >> +The driver supports PCIe SR-IOV to create virtual functions (VFs) 
> > > >> which
> > > >> can be
> > > >> +used to assign individual accelerators to virtual machines .
> > > >
> > > >
> > > > Does this HW Architecture require an Intel FPGA?  Couldn't any vendors 
> > > > FPGA
> > > > be used as long as it presented itself the PCIe bus the same and 
> > > > contained
> > > > an appropriate Device Feature List?
> 
> I think this is a good (and important) point. Especially when sysfs
> entries & ioctls constituting ABI depend on it.

Thanks for your feedback and comments.

I'm not sure if any vendors FPGA will resue the same. But if the same FME
or Port/AFU module implemented in their hardwares, then they should be
able to use our FME/AFU drivers.

AFU driver creates interfaces for FPGA accelerators. And FME driver creates
interfaces for FPGA 

[PATCH] mm/mmap: Replace SHM_HUGE_MASK with MAP_HUGE_MASK inside mmap_pgoff

[Anshuman Khandual]

The commit 091d0d55b286 ("shm: fix null pointer deref when userspace
specifies invalid hugepage size") had replaced MAP_HUGE_MASK with
SHM_HUGE_MASK. Though both of them contain the same numeric value of
0x3f, MAP_HUGE_MASK flag sounds more appropriate than the other one
in the context. Hence change it back.

Acked-by: Balbir Singh 
Acked-by: Michal Hocko 
Signed-off-by: Anshuman Khandual 
---
Posted this last year (https://patchwork.kernel.org/patch/8768891/) and
then forgot to follow up. Sorry about that.

 mm/mmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index bfbe885..f82741e 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1479,7 +1479,7 @@ unsigned long do_mmap(struct file *file, unsigned long 
addr,
struct user_struct *user = NULL;
struct hstate *hs;
 
-   hs = hstate_sizelog((flags >> MAP_HUGE_SHIFT) & SHM_HUGE_MASK);
+   hs = hstate_sizelog((flags >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK);
if (!hs)
return -EINVAL;
 
-- 
1.8.5.2

[PATCH] mm/mmap: Replace SHM_HUGE_MASK with MAP_HUGE_MASK inside mmap_pgoff

[Anshuman Khandual]

The commit 091d0d55b286 ("shm: fix null pointer deref when userspace
specifies invalid hugepage size") had replaced MAP_HUGE_MASK with
SHM_HUGE_MASK. Though both of them contain the same numeric value of
0x3f, MAP_HUGE_MASK flag sounds more appropriate than the other one
in the context. Hence change it back.

Acked-by: Balbir Singh 
Acked-by: Michal Hocko 
Signed-off-by: Anshuman Khandual 
---
Posted this last year (https://patchwork.kernel.org/patch/8768891/) and
then forgot to follow up. Sorry about that.

 mm/mmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index bfbe885..f82741e 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1479,7 +1479,7 @@ unsigned long do_mmap(struct file *file, unsigned long 
addr,
struct user_struct *user = NULL;
struct hstate *hs;
 
-   hs = hstate_sizelog((flags >> MAP_HUGE_SHIFT) & SHM_HUGE_MASK);
+   hs = hstate_sizelog((flags >> MAP_HUGE_SHIFT) & MAP_HUGE_MASK);
if (!hs)
return -EINVAL;
 
-- 
1.8.5.2

Re: [PATCH 1/5] zram: handle multiple pages attached bio's bvec

[Sergey Senozhatsky]

On (04/03/17 14:17), Minchan Kim wrote:
> Johannes Thumshirn reported system goes the panic when using NVMe over
> Fabrics loopback target with zram.
> 
> The reason is zram expects each bvec in bio contains a single page
> but nvme can attach a huge bulk of pages attached to the bio's bvec
> so that zram's index arithmetic could be wrong so that out-of-bound
> access makes panic.
> 
> It can be solved by limiting max_sectors with SECTORS_PER_PAGE like
> [1] but it makes zram slow because bio should split with each pages
> so this patch makes zram aware of multiple pages in a bvec so it
> could solve without any regression.
> 
> [1] 0bc315381fe9, zram: set physical queue limits to avoid array out of
> bounds accesses
> 
> Cc: Jens Axboe 
> Cc: Hannes Reinecke 
> Reported-by: Johannes Thumshirn 
> Tested-by: Johannes Thumshirn 
> Reviewed-by: Johannes Thumshirn 
> Signed-off-by: Johannes Thumshirn 
> Signed-off-by: Minchan Kim 

Reviewed-by: Sergey Senozhatsky 


> + unsigned int remained = bvec.bv_len;
...
> + } while (remained);

a tiny nitpick,  "-ed" in variable name looks a bit unusual.

-ss

Re: [PATCH 1/5] zram: handle multiple pages attached bio's bvec

[Sergey Senozhatsky]

On (04/03/17 14:17), Minchan Kim wrote:
> Johannes Thumshirn reported system goes the panic when using NVMe over
> Fabrics loopback target with zram.
> 
> The reason is zram expects each bvec in bio contains a single page
> but nvme can attach a huge bulk of pages attached to the bio's bvec
> so that zram's index arithmetic could be wrong so that out-of-bound
> access makes panic.
> 
> It can be solved by limiting max_sectors with SECTORS_PER_PAGE like
> [1] but it makes zram slow because bio should split with each pages
> so this patch makes zram aware of multiple pages in a bvec so it
> could solve without any regression.
> 
> [1] 0bc315381fe9, zram: set physical queue limits to avoid array out of
> bounds accesses
> 
> Cc: Jens Axboe 
> Cc: Hannes Reinecke 
> Reported-by: Johannes Thumshirn 
> Tested-by: Johannes Thumshirn 
> Reviewed-by: Johannes Thumshirn 
> Signed-off-by: Johannes Thumshirn 
> Signed-off-by: Minchan Kim 

Reviewed-by: Sergey Senozhatsky 


> + unsigned int remained = bvec.bv_len;
...
> + } while (remained);

a tiny nitpick,  "-ed" in variable name looks a bit unusual.

-ss

Re: [PATCH 2/2] extcon: Add new extcon_register_notifier_all() to monitor all external connectors

[Chanwoo Choi]

Hi,

On 2017년 04월 03일 20:14, Hans de Goede wrote:
> Hi,
>
> On 03-04-17 09:24, Chanwoo Choi wrote:
>> Hi,
>>
>> On 2017년 03월 30일 23:58, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 30-03-17 11:20, Chanwoo Choi wrote:
 On 2017년 03월 30일 18:04, Hans de Goede wrote:
>
> 
>
> Also this should be moved outside of the area of the
> function holding the edev->lock spinlock, since we don't
> pass state we do not need the lock and the called
> notifier function may very well want to call extcon_get_state
> to find out the state of one or more of the cables,
> which takes the lock.

 The extcon uses the spinlock for the short delay.
 Extcon should update the status of external connector
 to the extcon consumer as soon as possible.
>>>
>>> Right, what I'm suggestion actually also applies to the
>>> current cable notification, what I'm suggesting is to
>>> move the notification like this:
>>>
>>> --- a/drivers/extcon/extcon.c
>>> +++ b/drivers/extcon/extcon.c
>>> @@ -448,8 +448,6 @@ int extcon_sync(struct extcon_dev *edev, unsigned int 
>>> id)
>>> spin_lock_irqsave(>lock, flags);
>>>
>>> state = !!(edev->state & BIT(index));
>>> -   raw_notifier_call_chain(>nh[index], state, edev);
>>> -   raw_notifier_call_chain(>nh_all, 0, edev);
>>>
>>> /* This could be in interrupt handler */
>>> prop_buf = (char *)get_zeroed_page(GFP_ATOMIC);
>>> @@ -482,6 +480,10 @@ int extcon_sync(struct extcon_dev *edev, unsigned int 
>>> id)
>>>
>>> /* Unlock early before uevent */
>>> spin_unlock_irqrestore(>lock, flags);
>>> +
>>> + raw_notifier_call_chain(>nh[index], state, edev);
>>> + raw_notifier_call_chain(>nh_all, 0, edev);
>>> +
>>> kobject_uevent_env(>dev.kobj, KOBJ_CHANGE, envp);
>>> free_page((unsigned long)prop_buf);
>>>
>>>
>>> So that the nb.notifier_call function can call extcon_get_state
>>> to find out what cable is plugged in without deadlocking because
>>> extcon_get_state does spin_lock_irqsave(>lock, flags) too.
>>>
>>> This is esp. important for the edev->nh_all notifier chain
>>> since when used in charger drivers the callback will want to call
>>> extcon_get_state for all of: EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_DCP,
>>> EXTCON_CHG_USB_CDP, etc. to find out how much current it can draw
>>> from the port.
>>>
>>> AFAICT tell there is no race in moving this outside of the locked
>>> section of extcon_sync() and it avoids potential deadlocks in the
>>> nb.notifier_call function.
>>
>>
>> Actually, I knew that if the extcon consumer driver uses
>> the extcon_get_state() in the callback function, there is a deadlock
>> between extcon_sync() and extcon_get_state(). So, all extcon consumer
>> uses the workqueue when receiving the notfication from extcon.
>>
>> But, extcon_sync() have to call the number of callback functions
>> in the notifier chanin. If one specific extcon consumer spend many
>> time in the own callback function, other extcon consumer might receive
>> the notification late. So, I think that each extcon consumer
>> better to use the work in the their callback function.
>>
>> As I already said, I think that extcon focus on sending the notification
>> to all of extcon consumers.
>
> Ok, then lets keep your patches as they are, I've added the patches
> from your extcon-test branch to my local repository, modified the drivers
> which I've pending upstream which need this to use the new functionality
> and tested things.
>
> Everything looks and works good with these patches, so please add my:
>
> Acked-and-Tested-by: Hans de Goede 
>
> to them.
>
> It would be great if you can push these patches to extcon-next and
> then create a stable branch with these patches which other subsys
> maintainers can merge, so that I can start submitting patches which
> need this upstream (and also do a cleanup patch for the current axp288
> charger code).
>

Sure, After reviewing the patches, I'll make the immutable branch
and send the pull request for other subsystem maintainer as you mentioned.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH 2/2] extcon: Add new extcon_register_notifier_all() to monitor all external connectors

[Chanwoo Choi]

Hi,

On 2017년 04월 03일 20:14, Hans de Goede wrote:
> Hi,
>
> On 03-04-17 09:24, Chanwoo Choi wrote:
>> Hi,
>>
>> On 2017년 03월 30일 23:58, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 30-03-17 11:20, Chanwoo Choi wrote:
 On 2017년 03월 30일 18:04, Hans de Goede wrote:
>
> 
>
> Also this should be moved outside of the area of the
> function holding the edev->lock spinlock, since we don't
> pass state we do not need the lock and the called
> notifier function may very well want to call extcon_get_state
> to find out the state of one or more of the cables,
> which takes the lock.

 The extcon uses the spinlock for the short delay.
 Extcon should update the status of external connector
 to the extcon consumer as soon as possible.
>>>
>>> Right, what I'm suggestion actually also applies to the
>>> current cable notification, what I'm suggesting is to
>>> move the notification like this:
>>>
>>> --- a/drivers/extcon/extcon.c
>>> +++ b/drivers/extcon/extcon.c
>>> @@ -448,8 +448,6 @@ int extcon_sync(struct extcon_dev *edev, unsigned int 
>>> id)
>>> spin_lock_irqsave(>lock, flags);
>>>
>>> state = !!(edev->state & BIT(index));
>>> -   raw_notifier_call_chain(>nh[index], state, edev);
>>> -   raw_notifier_call_chain(>nh_all, 0, edev);
>>>
>>> /* This could be in interrupt handler */
>>> prop_buf = (char *)get_zeroed_page(GFP_ATOMIC);
>>> @@ -482,6 +480,10 @@ int extcon_sync(struct extcon_dev *edev, unsigned int 
>>> id)
>>>
>>> /* Unlock early before uevent */
>>> spin_unlock_irqrestore(>lock, flags);
>>> +
>>> + raw_notifier_call_chain(>nh[index], state, edev);
>>> + raw_notifier_call_chain(>nh_all, 0, edev);
>>> +
>>> kobject_uevent_env(>dev.kobj, KOBJ_CHANGE, envp);
>>> free_page((unsigned long)prop_buf);
>>>
>>>
>>> So that the nb.notifier_call function can call extcon_get_state
>>> to find out what cable is plugged in without deadlocking because
>>> extcon_get_state does spin_lock_irqsave(>lock, flags) too.
>>>
>>> This is esp. important for the edev->nh_all notifier chain
>>> since when used in charger drivers the callback will want to call
>>> extcon_get_state for all of: EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_DCP,
>>> EXTCON_CHG_USB_CDP, etc. to find out how much current it can draw
>>> from the port.
>>>
>>> AFAICT tell there is no race in moving this outside of the locked
>>> section of extcon_sync() and it avoids potential deadlocks in the
>>> nb.notifier_call function.
>>
>>
>> Actually, I knew that if the extcon consumer driver uses
>> the extcon_get_state() in the callback function, there is a deadlock
>> between extcon_sync() and extcon_get_state(). So, all extcon consumer
>> uses the workqueue when receiving the notfication from extcon.
>>
>> But, extcon_sync() have to call the number of callback functions
>> in the notifier chanin. If one specific extcon consumer spend many
>> time in the own callback function, other extcon consumer might receive
>> the notification late. So, I think that each extcon consumer
>> better to use the work in the their callback function.
>>
>> As I already said, I think that extcon focus on sending the notification
>> to all of extcon consumers.
>
> Ok, then lets keep your patches as they are, I've added the patches
> from your extcon-test branch to my local repository, modified the drivers
> which I've pending upstream which need this to use the new functionality
> and tested things.
>
> Everything looks and works good with these patches, so please add my:
>
> Acked-and-Tested-by: Hans de Goede 
>
> to them.
>
> It would be great if you can push these patches to extcon-next and
> then create a stable branch with these patches which other subsys
> maintainers can merge, so that I can start submitting patches which
> need this upstream (and also do a cleanup patch for the current axp288
> charger code).
>

Sure, After reviewing the patches, I'll make the immutable branch
and send the pull request for other subsystem maintainer as you mentioned.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH 4/5] zram: remove zram_meta structure

[Minchan Kim]

On Tue, Apr 04, 2017 at 11:31:15AM +0900, Sergey Senozhatsky wrote:
> On (04/03/17 14:17), Minchan Kim wrote:
> [..]
> > -static struct zram_meta *zram_meta_alloc(char *pool_name, u64 disksize)
> > +static bool zram_meta_alloc(struct zram *zram, u64 disksize)
> >  {
> > size_t num_pages;
> > -   struct zram_meta *meta = kmalloc(sizeof(*meta), GFP_KERNEL);
> > -
> > -   if (!meta)
> > -   return NULL;
> >  
> > num_pages = disksize >> PAGE_SHIFT;
> > -   meta->table = vzalloc(num_pages * sizeof(*meta->table));
> > -   if (!meta->table) {
> > -   pr_err("Error allocating zram address table\n");
> > -   goto out_error;
> > -   }
> > +   zram->table = vzalloc(num_pages * sizeof(*zram->table));
> > +   if (!zram->table)
> > +   return false;
> >  
> > -   meta->mem_pool = zs_create_pool(pool_name);
> > -   if (!meta->mem_pool) {
> > -   pr_err("Error creating memory pool\n");
> > -   goto out_error;
> > +   zram->mem_pool = zs_create_pool(zram->disk->disk_name);
> > +   if (!zram->mem_pool) {
> > +   vfree(zram->table);
> > +   return false;
> > }
> >  
> > -   return meta;
> > -
> > -out_error:
> > -   vfree(meta->table);
> > -   kfree(meta);
> > -   return NULL;
> > +   return true;
> >  }
> 
> [..]
> 
> > @@ -1020,7 +989,6 @@ static ssize_t disksize_store(struct device *dev,
> >  {
> > u64 disksize;
> > struct zcomp *comp;
> > -   struct zram_meta *meta;
> > struct zram *zram = dev_to_zram(dev);
> > int err;
> >  
> > @@ -1029,8 +997,7 @@ static ssize_t disksize_store(struct device *dev,
> > return -EINVAL;
> >  
> > disksize = PAGE_ALIGN(disksize);
> > -   meta = zram_meta_alloc(zram->disk->disk_name, disksize);
> > -   if (!meta)
> > +   if (!zram_meta_alloc(zram, disksize))
> > return -ENOMEM;
> >  
> > comp = zcomp_create(zram->compressor);
> > @@ -1048,7 +1015,6 @@ static ssize_t disksize_store(struct device *dev,
> > goto out_destroy_comp;
> > }
> >  
> > -   zram->meta = meta;
> > zram->comp = comp;
> > zram->disksize = disksize;
> > set_capacity(zram->disk, zram->disksize >> SECTOR_SHIFT);
> > @@ -1061,7 +1027,7 @@ static ssize_t disksize_store(struct device *dev,
> > up_write(>init_lock);
> > zcomp_destroy(comp);
> >  out_free_meta:
> > -   zram_meta_free(meta, disksize);
> > +   zram_meta_free(zram, disksize);
> > return err;
> >  }
> 
> OK, I don't think it's the same.
> 
> we used to have
> 
>   struct zram_meta *zram_meta_alloc()
>   {
>   meta->table = vzalloc()
>   meta->mem_pool = zs_create_pool();
>   return meta;
>   }
> 
> 
>   disksize_store()
>   {
>   meta = zram_meta_alloc();
>   if (init_done(zram)) {
>   pr_info("Cannot change disksize for initialized 
> device\n");
>   goto out_destroy_comp;
>   }
> 
>   zram->meta = meta;
>   ^^
>   }
> 
> now we have
> 
>   struct zram_meta *zram_meta_alloc()
>   {
>   zram->table = vzalloc()
>   zram->mem_pool = zs_create_pool();
>   return true;
>   }
> 
> 
>   disksize_store()
>   {
>   zram_meta_alloc();
>   ^
>   if (init_done(zram)) {
>   pr_info("Cannot change disksize for initialized 
> device\n");
>   goto out_destroy_comp;
>   }
>   }
> 
> 
> by the time we call init_done(zram) on already init device zram->table
> and zram->mem_pool are overwritten and lost. right?

Right you are.
I will fix it!

> 
> 
> [..]
> > -struct zram_meta {
> > +struct zram {
> > struct zram_table_entry *table;
> > struct zs_pool *mem_pool;
> > -};
> > -
> > -struct zram {
> > -   struct zram_meta *meta;
> > struct zcomp *comp;
> > struct gendisk *disk;
> > /* Prevent concurrent execution of device init */
> 
> 
> we still have several zram_meta_FOO() left overs in zram_drv.c
> 
>   -ss

Re: [PATCH 4/5] zram: remove zram_meta structure

[Minchan Kim]

On Tue, Apr 04, 2017 at 11:31:15AM +0900, Sergey Senozhatsky wrote:
> On (04/03/17 14:17), Minchan Kim wrote:
> [..]
> > -static struct zram_meta *zram_meta_alloc(char *pool_name, u64 disksize)
> > +static bool zram_meta_alloc(struct zram *zram, u64 disksize)
> >  {
> > size_t num_pages;
> > -   struct zram_meta *meta = kmalloc(sizeof(*meta), GFP_KERNEL);
> > -
> > -   if (!meta)
> > -   return NULL;
> >  
> > num_pages = disksize >> PAGE_SHIFT;
> > -   meta->table = vzalloc(num_pages * sizeof(*meta->table));
> > -   if (!meta->table) {
> > -   pr_err("Error allocating zram address table\n");
> > -   goto out_error;
> > -   }
> > +   zram->table = vzalloc(num_pages * sizeof(*zram->table));
> > +   if (!zram->table)
> > +   return false;
> >  
> > -   meta->mem_pool = zs_create_pool(pool_name);
> > -   if (!meta->mem_pool) {
> > -   pr_err("Error creating memory pool\n");
> > -   goto out_error;
> > +   zram->mem_pool = zs_create_pool(zram->disk->disk_name);
> > +   if (!zram->mem_pool) {
> > +   vfree(zram->table);
> > +   return false;
> > }
> >  
> > -   return meta;
> > -
> > -out_error:
> > -   vfree(meta->table);
> > -   kfree(meta);
> > -   return NULL;
> > +   return true;
> >  }
> 
> [..]
> 
> > @@ -1020,7 +989,6 @@ static ssize_t disksize_store(struct device *dev,
> >  {
> > u64 disksize;
> > struct zcomp *comp;
> > -   struct zram_meta *meta;
> > struct zram *zram = dev_to_zram(dev);
> > int err;
> >  
> > @@ -1029,8 +997,7 @@ static ssize_t disksize_store(struct device *dev,
> > return -EINVAL;
> >  
> > disksize = PAGE_ALIGN(disksize);
> > -   meta = zram_meta_alloc(zram->disk->disk_name, disksize);
> > -   if (!meta)
> > +   if (!zram_meta_alloc(zram, disksize))
> > return -ENOMEM;
> >  
> > comp = zcomp_create(zram->compressor);
> > @@ -1048,7 +1015,6 @@ static ssize_t disksize_store(struct device *dev,
> > goto out_destroy_comp;
> > }
> >  
> > -   zram->meta = meta;
> > zram->comp = comp;
> > zram->disksize = disksize;
> > set_capacity(zram->disk, zram->disksize >> SECTOR_SHIFT);
> > @@ -1061,7 +1027,7 @@ static ssize_t disksize_store(struct device *dev,
> > up_write(>init_lock);
> > zcomp_destroy(comp);
> >  out_free_meta:
> > -   zram_meta_free(meta, disksize);
> > +   zram_meta_free(zram, disksize);
> > return err;
> >  }
> 
> OK, I don't think it's the same.
> 
> we used to have
> 
>   struct zram_meta *zram_meta_alloc()
>   {
>   meta->table = vzalloc()
>   meta->mem_pool = zs_create_pool();
>   return meta;
>   }
> 
> 
>   disksize_store()
>   {
>   meta = zram_meta_alloc();
>   if (init_done(zram)) {
>   pr_info("Cannot change disksize for initialized 
> device\n");
>   goto out_destroy_comp;
>   }
> 
>   zram->meta = meta;
>   ^^
>   }
> 
> now we have
> 
>   struct zram_meta *zram_meta_alloc()
>   {
>   zram->table = vzalloc()
>   zram->mem_pool = zs_create_pool();
>   return true;
>   }
> 
> 
>   disksize_store()
>   {
>   zram_meta_alloc();
>   ^
>   if (init_done(zram)) {
>   pr_info("Cannot change disksize for initialized 
> device\n");
>   goto out_destroy_comp;
>   }
>   }
> 
> 
> by the time we call init_done(zram) on already init device zram->table
> and zram->mem_pool are overwritten and lost. right?

Right you are.
I will fix it!

> 
> 
> [..]
> > -struct zram_meta {
> > +struct zram {
> > struct zram_table_entry *table;
> > struct zs_pool *mem_pool;
> > -};
> > -
> > -struct zram {
> > -   struct zram_meta *meta;
> > struct zcomp *comp;
> > struct gendisk *disk;
> > /* Prevent concurrent execution of device init */
> 
> 
> we still have several zram_meta_FOO() left overs in zram_drv.c
> 
>   -ss

[PATCH v2 1/2] usb: gadget: udc: avoid use of freed pointer

[Gustavo A. R. Silva]

Rewrite udc_free_dma_chain() function to avoid use of pointer after free.

Addresses-Coverity-ID: 1091172
Acked-by: Michal Nazarewicz 
Reviewed-by: Greg Kroah-Hartman 
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 Remove 'td->next = 0x00' inside for loop.
 Remove unnecessary pointer nullification after free.
 Rename variable addr_aux to addr_next.

 drivers/usb/gadget/udc/amd5536udc.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index ea03ca7..821d088 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -611,21 +611,20 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
 static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
 {
int ret_val = 0;
-   struct udc_data_dma *td;
-   struct udc_data_dma *td_last = NULL;
+   struct udc_data_dma *td = req->td_data;
unsigned int i;
 
+   dma_addr_t addr_next = 0x00;
+   dma_addr_t addr = (dma_addr_t)td->next;
+
DBG(dev, "free chain req = %p\n", req);
 
/* do not free first desc., will be done by free for request */
-   td_last = req->td_data;
-   td = phys_to_virt(td_last->next);
-
for (i = 1; i < req->chain_len; i++) {
-   pci_pool_free(dev->data_requests, td,
- (dma_addr_t)td_last->next);
-   td_last = td;
-   td = phys_to_virt(td_last->next);
+   td = phys_to_virt(addr);
+   addr_next = (dma_addr_t)td->next;
+   pci_pool_free(dev->data_requests, td, addr);
+   addr = addr_next;
}
 
return ret_val;
-- 
2.5.0

[PATCH v2 1/2] usb: gadget: udc: avoid use of freed pointer

[Gustavo A. R. Silva]

Rewrite udc_free_dma_chain() function to avoid use of pointer after free.

Addresses-Coverity-ID: 1091172
Acked-by: Michal Nazarewicz 
Reviewed-by: Greg Kroah-Hartman 
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 Remove 'td->next = 0x00' inside for loop.
 Remove unnecessary pointer nullification after free.
 Rename variable addr_aux to addr_next.

 drivers/usb/gadget/udc/amd5536udc.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index ea03ca7..821d088 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -611,21 +611,20 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
 static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
 {
int ret_val = 0;
-   struct udc_data_dma *td;
-   struct udc_data_dma *td_last = NULL;
+   struct udc_data_dma *td = req->td_data;
unsigned int i;
 
+   dma_addr_t addr_next = 0x00;
+   dma_addr_t addr = (dma_addr_t)td->next;
+
DBG(dev, "free chain req = %p\n", req);
 
/* do not free first desc., will be done by free for request */
-   td_last = req->td_data;
-   td = phys_to_virt(td_last->next);
-
for (i = 1; i < req->chain_len; i++) {
-   pci_pool_free(dev->data_requests, td,
- (dma_addr_t)td_last->next);
-   td_last = td;
-   td = phys_to_virt(td_last->next);
+   td = phys_to_virt(addr);
+   addr_next = (dma_addr_t)td->next;
+   pci_pool_free(dev->data_requests, td, addr);
+   addr = addr_next;
}
 
return ret_val;
-- 
2.5.0

Re: [PATCH 2/5] zram: partial IO refactoring

[Minchan Kim]

Hi Sergey,

On Tue, Apr 04, 2017 at 11:17:06AM +0900, Sergey Senozhatsky wrote:
> Hello,
> 
> On (04/03/17 14:17), Minchan Kim wrote:
> > +static bool zram_special_page_read(struct zram *zram, u32 index,
> > +   struct page *page,
> > +   unsigned int offset, unsigned int len)
> > +{
> > +   struct zram_meta *meta = zram->meta;
> > +
> > +   bit_spin_lock(ZRAM_ACCESS, >table[index].value);
> > +   if (unlikely(!meta->table[index].handle) ||
> > +   zram_test_flag(meta, index, ZRAM_SAME)) {
> > +   void *mem;
> > +
> > +   bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
> > +   mem = kmap_atomic(page);
> > +   zram_fill_page(mem + offset, len, meta->table[index].element);
> > +   kunmap_atomic(mem);
> > +   return true;
> > +   }
> > +   bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
> > +
> > +   return false;
> > +}
> > +
> > +static bool zram_special_page_write(struct zram *zram, u32 index,
> > +   struct page *page)
> > +{
> > +   unsigned long element;
> > +   void *mem = kmap_atomic(page);
> > +
> > +   if (page_same_filled(mem, )) {
> > +   struct zram_meta *meta = zram->meta;
> > +
> > +   kunmap_atomic(mem);
> > +   /* Free memory associated with this sector now. */
> > +   bit_spin_lock(ZRAM_ACCESS, >table[index].value);
> > +   zram_free_page(zram, index);
> > +   zram_set_flag(meta, index, ZRAM_SAME);
> > +   zram_set_element(meta, index, element);
> > +   bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
> > +
> > +   atomic64_inc(>stats.same_pages);
> > +   return true;
> > +   }
> > +   kunmap_atomic(mem);
> > +
> > +   return false;
> > +}
> 
> zram_special_page_read() and zram_special_page_write() have a slightly
> different locking semantics.
> 
> zram_special_page_read() copy-out ZRAM_SAME page having slot unlocked
> (can the slot got overwritten in the meantime?), while

IMHO, yes, it can be overwritten but it doesn't make corruption of kernel.
I mean if such race happens, it's user fault who should protect the race.
zRAM is dumb block device so it can read/write block user request but
one thing we should keep the promise is it shouldn't corrupt the kernel.
Such pov, zram_special_page_read wouldn't be a problem to return
stale data, I think.

> zram_special_page_write() keeps the slot locked through out the entire
> operation.

zram_special_page_write is something different because it updates
zram_table's slot via zram_set_[flag|element] so it should be protected
by zram.

> 
> >  static void zram_meta_free(struct zram_meta *meta, u64 disksize)
> >  {
> > size_t num_pages = disksize >> PAGE_SHIFT;
> > @@ -504,169 +548,104 @@ static void zram_free_page(struct zram *zram, 
> > size_t index)
> > zram_set_obj_size(meta, index, 0);
> >  }
> >  
> > -static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
> > +static int zram_decompress_page(struct zram *zram, struct page *page, u32 
> > index)
> >  {
> > -   int ret = 0;
> > -   unsigned char *cmem;
> > -   struct zram_meta *meta = zram->meta;
> > +   int ret;
> > unsigned long handle;
> > unsigned int size;
> > +   void *src, *dst;
> > +   struct zram_meta *meta = zram->meta;
> > +
> > +   if (zram_special_page_read(zram, index, page, 0, PAGE_SIZE))
> > +   return 0;
> >  
> > bit_spin_lock(ZRAM_ACCESS, >table[index].value);
> > handle = meta->table[index].handle;
> > size = zram_get_obj_size(meta, index);
> >  
> > -   if (!handle || zram_test_flag(meta, index, ZRAM_SAME)) {
> > -   bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
> > -   zram_fill_page(mem, PAGE_SIZE, meta->table[index].element);
> > -   return 0;
> > -   }
> > -
> > -   cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_RO);
> > +   src = zs_map_object(meta->mem_pool, handle, ZS_MM_RO);
> > if (size == PAGE_SIZE) {
> > -   copy_page(mem, cmem);
> > +   dst = kmap_atomic(page);
> > +   copy_page(dst, src);
> > +   kunmap_atomic(dst);
> > +   ret = 0;
> > } else {
> > struct zcomp_strm *zstrm = zcomp_stream_get(zram->comp);
> >  
> > -   ret = zcomp_decompress(zstrm, cmem, size, mem);
> > +   dst = kmap_atomic(page);
> > +   ret = zcomp_decompress(zstrm, src, size, dst);
> > +   kunmap_atomic(dst);
> > zcomp_stream_put(zram->comp);
> > }
> > zs_unmap_object(meta->mem_pool, handle);
> > bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
> >  
> > /* Should NEVER happen. Return bio error if it does. */
> > -   if (unlikely(ret)) {
> > +   if (unlikely(ret))
> > pr_err("Decompression failed! err=%d, page=%u\n", ret, index);
> > -   return ret;
> > -   }
> >  
> > -   return 0;
> > +   return ret;
> >  }
> >  
> >  static int 

Re: [PATCH 3/5] zram: use zram_slot_lock instead of raw bit_spin_lock op

[Minchan Kim]

On Tue, Apr 04, 2017 at 11:18:51AM +0900, Sergey Senozhatsky wrote:
> On (04/03/17 14:17), Minchan Kim wrote:
> > With this clean-up phase, I want to use zram's wrapper function
> > to lock table access which is more consistent with other zram's
> > functions.
> > 
> > Signed-off-by: Minchan Kim 
> 
> Reviewed-by: Sergey Senozhatsky 

Thanks!

Re: [PATCH 2/5] zram: partial IO refactoring

[Minchan Kim]

Hi Sergey,

On Tue, Apr 04, 2017 at 11:17:06AM +0900, Sergey Senozhatsky wrote:
> Hello,
> 
> On (04/03/17 14:17), Minchan Kim wrote:
> > +static bool zram_special_page_read(struct zram *zram, u32 index,
> > +   struct page *page,
> > +   unsigned int offset, unsigned int len)
> > +{
> > +   struct zram_meta *meta = zram->meta;
> > +
> > +   bit_spin_lock(ZRAM_ACCESS, >table[index].value);
> > +   if (unlikely(!meta->table[index].handle) ||
> > +   zram_test_flag(meta, index, ZRAM_SAME)) {
> > +   void *mem;
> > +
> > +   bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
> > +   mem = kmap_atomic(page);
> > +   zram_fill_page(mem + offset, len, meta->table[index].element);
> > +   kunmap_atomic(mem);
> > +   return true;
> > +   }
> > +   bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
> > +
> > +   return false;
> > +}
> > +
> > +static bool zram_special_page_write(struct zram *zram, u32 index,
> > +   struct page *page)
> > +{
> > +   unsigned long element;
> > +   void *mem = kmap_atomic(page);
> > +
> > +   if (page_same_filled(mem, )) {
> > +   struct zram_meta *meta = zram->meta;
> > +
> > +   kunmap_atomic(mem);
> > +   /* Free memory associated with this sector now. */
> > +   bit_spin_lock(ZRAM_ACCESS, >table[index].value);
> > +   zram_free_page(zram, index);
> > +   zram_set_flag(meta, index, ZRAM_SAME);
> > +   zram_set_element(meta, index, element);
> > +   bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
> > +
> > +   atomic64_inc(>stats.same_pages);
> > +   return true;
> > +   }
> > +   kunmap_atomic(mem);
> > +
> > +   return false;
> > +}
> 
> zram_special_page_read() and zram_special_page_write() have a slightly
> different locking semantics.
> 
> zram_special_page_read() copy-out ZRAM_SAME page having slot unlocked
> (can the slot got overwritten in the meantime?), while

IMHO, yes, it can be overwritten but it doesn't make corruption of kernel.
I mean if such race happens, it's user fault who should protect the race.
zRAM is dumb block device so it can read/write block user request but
one thing we should keep the promise is it shouldn't corrupt the kernel.
Such pov, zram_special_page_read wouldn't be a problem to return
stale data, I think.

> zram_special_page_write() keeps the slot locked through out the entire
> operation.

zram_special_page_write is something different because it updates
zram_table's slot via zram_set_[flag|element] so it should be protected
by zram.

> 
> >  static void zram_meta_free(struct zram_meta *meta, u64 disksize)
> >  {
> > size_t num_pages = disksize >> PAGE_SHIFT;
> > @@ -504,169 +548,104 @@ static void zram_free_page(struct zram *zram, 
> > size_t index)
> > zram_set_obj_size(meta, index, 0);
> >  }
> >  
> > -static int zram_decompress_page(struct zram *zram, char *mem, u32 index)
> > +static int zram_decompress_page(struct zram *zram, struct page *page, u32 
> > index)
> >  {
> > -   int ret = 0;
> > -   unsigned char *cmem;
> > -   struct zram_meta *meta = zram->meta;
> > +   int ret;
> > unsigned long handle;
> > unsigned int size;
> > +   void *src, *dst;
> > +   struct zram_meta *meta = zram->meta;
> > +
> > +   if (zram_special_page_read(zram, index, page, 0, PAGE_SIZE))
> > +   return 0;
> >  
> > bit_spin_lock(ZRAM_ACCESS, >table[index].value);
> > handle = meta->table[index].handle;
> > size = zram_get_obj_size(meta, index);
> >  
> > -   if (!handle || zram_test_flag(meta, index, ZRAM_SAME)) {
> > -   bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
> > -   zram_fill_page(mem, PAGE_SIZE, meta->table[index].element);
> > -   return 0;
> > -   }
> > -
> > -   cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_RO);
> > +   src = zs_map_object(meta->mem_pool, handle, ZS_MM_RO);
> > if (size == PAGE_SIZE) {
> > -   copy_page(mem, cmem);
> > +   dst = kmap_atomic(page);
> > +   copy_page(dst, src);
> > +   kunmap_atomic(dst);
> > +   ret = 0;
> > } else {
> > struct zcomp_strm *zstrm = zcomp_stream_get(zram->comp);
> >  
> > -   ret = zcomp_decompress(zstrm, cmem, size, mem);
> > +   dst = kmap_atomic(page);
> > +   ret = zcomp_decompress(zstrm, src, size, dst);
> > +   kunmap_atomic(dst);
> > zcomp_stream_put(zram->comp);
> > }
> > zs_unmap_object(meta->mem_pool, handle);
> > bit_spin_unlock(ZRAM_ACCESS, >table[index].value);
> >  
> > /* Should NEVER happen. Return bio error if it does. */
> > -   if (unlikely(ret)) {
> > +   if (unlikely(ret))
> > pr_err("Decompression failed! err=%d, page=%u\n", ret, index);
> > -   return ret;
> > -   }
> >  
> > -   return 0;
> > +   return ret;
> >  }
> >  
> >  static int 

Re: [PATCH 3/5] zram: use zram_slot_lock instead of raw bit_spin_lock op

[Minchan Kim]

On Tue, Apr 04, 2017 at 11:18:51AM +0900, Sergey Senozhatsky wrote:
> On (04/03/17 14:17), Minchan Kim wrote:
> > With this clean-up phase, I want to use zram's wrapper function
> > to lock table access which is more consistent with other zram's
> > functions.
> > 
> > Signed-off-by: Minchan Kim 
> 
> Reviewed-by: Sergey Senozhatsky 

Thanks!

FWD: gathered list of available stocks

[rcde sai]

Hi

I was browsing thru website and gathered list of available stocks.

I have chosen three of them and I have attached all three of them with a 
specified specs.

Payment will be semi-annually. Please do let me know if we can get this going 
A.S.A.P.

Thank You

Misu Del Copo Dei Capi 
Datacenter Black SEE


03 April 2017
Generated by Black SEE


available stocks_and_paymentsemi-annually.html
Description: Binary data

FWD: gathered list of available stocks

[rcde sai]

Hi

I was browsing thru website and gathered list of available stocks.

I have chosen three of them and I have attached all three of them with a 
specified specs.

Payment will be semi-annually. Please do let me know if we can get this going 
A.S.A.P.

Thank You

Misu Del Copo Dei Capi 
Datacenter Black SEE


03 April 2017
Generated by Black SEE


available stocks_and_paymentsemi-annually.html
Description: Binary data

Re: [PATCH 5/5] zram: introduce zram data accessor

[Sergey Senozhatsky]

On (04/03/17 14:17), Minchan Kim wrote:
> With element, sometime I got confused handle and element access.
> It might be my bad but I think it's time to introduce accessor
> to prevent future idiot like me.
> This patch is just clean-up patch so it shouldn't change any
> behavior.
> 
> Signed-off-by: Minchan Kim 

Reviewed-by: Sergey Senozhatsky 

-ss

Re: [PATCH 5/5] zram: introduce zram data accessor

[Sergey Senozhatsky]

On (04/03/17 14:17), Minchan Kim wrote:
> With element, sometime I got confused handle and element access.
> It might be my bad but I think it's time to introduce accessor
> to prevent future idiot like me.
> This patch is just clean-up patch so it shouldn't change any
> behavior.
> 
> Signed-off-by: Minchan Kim 

Reviewed-by: Sergey Senozhatsky 

-ss

[PATCH v2 2/2] usb: gadget: udc: remove unnecessary variable and update function prototype

[Gustavo A. R. Silva]

Remove unnecessary variable and update function prototype.

Acked-by: Michal Nazarewicz 
Reviewed-by: Greg Kroah-Hartman 
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 None.

 drivers/usb/gadget/udc/amd5536udc.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 821d088..67dd209 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -608,9 +608,8 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
 }
 
 /* frees pci pool descriptors of a DMA chain */
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
+static void udc_free_dma_chain(struct udc *dev, struct udc_request *req)
 {
-   int ret_val = 0;
struct udc_data_dma *td = req->td_data;
unsigned int i;
 
@@ -626,8 +625,6 @@ static int udc_free_dma_chain(struct udc *dev, struct 
udc_request *req)
pci_pool_free(dev->data_requests, td, addr);
addr = addr_next;
}
-
-   return ret_val;
 }
 
 /* Frees request packet, called by gadget driver */
-- 
2.5.0

[PATCH v2 2/2] usb: gadget: udc: remove unnecessary variable and update function prototype

[Gustavo A. R. Silva]

Remove unnecessary variable and update function prototype.

Acked-by: Michal Nazarewicz 
Reviewed-by: Greg Kroah-Hartman 
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 None.

 drivers/usb/gadget/udc/amd5536udc.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc.c 
b/drivers/usb/gadget/udc/amd5536udc.c
index 821d088..67dd209 100644
--- a/drivers/usb/gadget/udc/amd5536udc.c
+++ b/drivers/usb/gadget/udc/amd5536udc.c
@@ -608,9 +608,8 @@ udc_alloc_request(struct usb_ep *usbep, gfp_t gfp)
 }
 
 /* frees pci pool descriptors of a DMA chain */
-static int udc_free_dma_chain(struct udc *dev, struct udc_request *req)
+static void udc_free_dma_chain(struct udc *dev, struct udc_request *req)
 {
-   int ret_val = 0;
struct udc_data_dma *td = req->td_data;
unsigned int i;
 
@@ -626,8 +625,6 @@ static int udc_free_dma_chain(struct udc *dev, struct 
udc_request *req)
pci_pool_free(dev->data_requests, td, addr);
addr = addr_next;
}
-
-   return ret_val;
 }
 
 /* Frees request packet, called by gadget driver */
-- 
2.5.0

Re: [PATCH v6 06/11] powerpc/perf: IMC pmu cpumask and cpu hotplug support

[Daniel Axtens]

Madhavan Srinivasan  writes:

> From: Hemant Kumar 
>
> Adds cpumask attribute to be used by each IMC pmu. Only one cpu (any
> online CPU) from each chip for nest PMUs is designated to read counters.
>
> On CPU hotplug, dying CPU is checked to see whether it is one of the
> designated cpus, if yes, next online cpu from the same chip (for nest
> units) is designated as new cpu to read counters. For this purpose, we
> introduce a new state : CPUHP_AP_PERF_POWERPC_NEST_ONLINE.
>
> Signed-off-by: Anju T Sudhakar 
> Signed-off-by: Hemant Kumar 
> Signed-off-by: Madhavan Srinivasan 
> ---
>  arch/powerpc/include/asm/opal-api.h|  13 ++-
>  arch/powerpc/include/asm/opal.h|   3 +
>  arch/powerpc/perf/imc-pmu.c| 155 
> -
>  arch/powerpc/platforms/powernv/opal-wrappers.S |   1 +
>  include/linux/cpuhotplug.h |   1 +
>  5 files changed, 171 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/opal-api.h 
> b/arch/powerpc/include/asm/opal-api.h
> index a0aa285869b5..23fc51e9d71d 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -168,7 +168,8 @@
>  #define OPAL_INT_SET_MFRR125
>  #define OPAL_PCI_TCE_KILL126
>  #define OPAL_NMMU_SET_PTCR   127
> -#define OPAL_LAST127
> +#define OPAL_NEST_IMC_COUNTERS_CONTROL   149

A couple of things:

 - why is this 149 rather than 128?

 - CONTROL seems like it's inviting a very broad and under-specified
   API. I notice most of the other opal calls are reasonably narrow:
   often defining two calls for get/set rather than a single control
   call.

I don't have cycles to review the OPAL/skiboot side and I'm very much
open to being convinced here, I just want to avoid the situation where
we're passing around complicated data structures in a way that is
difficult to synchronise if we could avoid it.

> +#define OPAL_LAST149
>  
>  /* Device tree flags */
>  
> @@ -928,6 +929,16 @@ enum {
>   OPAL_PCI_TCE_KILL_ALL,
>  };
>  
> +/* Operation argument to OPAL_NEST_IMC_COUNTERS_CONTROL */
> +enum {
> + OPAL_NEST_IMC_PRODUCTION_MODE = 1,
> +};
If there's only one mode, why do we need to specify it? As far as I can
tell no extra mode is added later in the series...

... looking at the opal-side patches, would it be better to just specify
the modes you intend to support in future here, and rely on opal
returning OPAL_PARAMETER when they're not supported?
> +
> +enum {
> + OPAL_NEST_IMC_STOP,
> + OPAL_NEST_IMC_START,
> +};
Again, would it be better to have a stop/start call rather than a
generic control call? 

Also, the enum values (STOP=0, START=1) seem to be switched vs OPAL,
where https://patchwork.ozlabs.org/patch/746292/ has STOP=1, START=0. Or
am I missing something?

> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif /* __OPAL_API_H */
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 1ff03a6da76e..d93d08204243 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -227,6 +227,9 @@ int64_t opal_pci_tce_kill(uint64_t phb_id, uint32_t 
> kill_type,
> uint64_t dma_addr, uint32_t npages);
>  int64_t opal_nmmu_set_ptcr(uint64_t chip_id, uint64_t ptcr);
>  
> +int64_t opal_nest_imc_counters_control(uint64_t mode, uint64_t value1,
> + uint64_t value2, uint64_t value3);
> +

This prototype does not make me feel better about the state of the API.

Looking at the opal side, I would be much more comfortable if you could
nail down what you intend to have these support now, even if OPAL bails
with OPAL_PARAMETER if they're specified.

Also I think u64 and u32 are preferred, although I see the rest of the
file uses the long form.

>  /* Internal functions */
>  extern int early_init_dt_scan_opal(unsigned long node, const char *uname,
>  int depth, void *data);
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index bd5bf66bd920..b28835b861b3 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -17,6 +17,7 @@
>  
>  struct perchip_nest_info nest_perchip_info[IMC_MAX_CHIPS];
>  struct imc_pmu *per_nest_pmu_arr[IMC_MAX_PMUS];
> +static cpumask_t nest_imc_cpumask;
>  
>  /* Needed for sanity check */
>  extern u64 nest_max_offset;
> @@ -32,6 +33,152 @@ static struct attribute_group imc_format_group = {
>   .attrs = imc_format_attrs,
>  };
>  
> +/* Get the cpumask printed to a buffer "buf" */
> +static ssize_t imc_pmu_cpumask_get_attr(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + cpumask_t *active_mask;
> +

Re: [PATCH v6 06/11] powerpc/perf: IMC pmu cpumask and cpu hotplug support

[Daniel Axtens]

Madhavan Srinivasan  writes:

> From: Hemant Kumar 
>
> Adds cpumask attribute to be used by each IMC pmu. Only one cpu (any
> online CPU) from each chip for nest PMUs is designated to read counters.
>
> On CPU hotplug, dying CPU is checked to see whether it is one of the
> designated cpus, if yes, next online cpu from the same chip (for nest
> units) is designated as new cpu to read counters. For this purpose, we
> introduce a new state : CPUHP_AP_PERF_POWERPC_NEST_ONLINE.
>
> Signed-off-by: Anju T Sudhakar 
> Signed-off-by: Hemant Kumar 
> Signed-off-by: Madhavan Srinivasan 
> ---
>  arch/powerpc/include/asm/opal-api.h|  13 ++-
>  arch/powerpc/include/asm/opal.h|   3 +
>  arch/powerpc/perf/imc-pmu.c| 155 
> -
>  arch/powerpc/platforms/powernv/opal-wrappers.S |   1 +
>  include/linux/cpuhotplug.h |   1 +
>  5 files changed, 171 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/opal-api.h 
> b/arch/powerpc/include/asm/opal-api.h
> index a0aa285869b5..23fc51e9d71d 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -168,7 +168,8 @@
>  #define OPAL_INT_SET_MFRR125
>  #define OPAL_PCI_TCE_KILL126
>  #define OPAL_NMMU_SET_PTCR   127
> -#define OPAL_LAST127
> +#define OPAL_NEST_IMC_COUNTERS_CONTROL   149

A couple of things:

 - why is this 149 rather than 128?

 - CONTROL seems like it's inviting a very broad and under-specified
   API. I notice most of the other opal calls are reasonably narrow:
   often defining two calls for get/set rather than a single control
   call.

I don't have cycles to review the OPAL/skiboot side and I'm very much
open to being convinced here, I just want to avoid the situation where
we're passing around complicated data structures in a way that is
difficult to synchronise if we could avoid it.

> +#define OPAL_LAST149
>  
>  /* Device tree flags */
>  
> @@ -928,6 +929,16 @@ enum {
>   OPAL_PCI_TCE_KILL_ALL,
>  };
>  
> +/* Operation argument to OPAL_NEST_IMC_COUNTERS_CONTROL */
> +enum {
> + OPAL_NEST_IMC_PRODUCTION_MODE = 1,
> +};
If there's only one mode, why do we need to specify it? As far as I can
tell no extra mode is added later in the series...

... looking at the opal-side patches, would it be better to just specify
the modes you intend to support in future here, and rely on opal
returning OPAL_PARAMETER when they're not supported?
> +
> +enum {
> + OPAL_NEST_IMC_STOP,
> + OPAL_NEST_IMC_START,
> +};
Again, would it be better to have a stop/start call rather than a
generic control call? 

Also, the enum values (STOP=0, START=1) seem to be switched vs OPAL,
where https://patchwork.ozlabs.org/patch/746292/ has STOP=1, START=0. Or
am I missing something?

> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif /* __OPAL_API_H */
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 1ff03a6da76e..d93d08204243 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -227,6 +227,9 @@ int64_t opal_pci_tce_kill(uint64_t phb_id, uint32_t 
> kill_type,
> uint64_t dma_addr, uint32_t npages);
>  int64_t opal_nmmu_set_ptcr(uint64_t chip_id, uint64_t ptcr);
>  
> +int64_t opal_nest_imc_counters_control(uint64_t mode, uint64_t value1,
> + uint64_t value2, uint64_t value3);
> +

This prototype does not make me feel better about the state of the API.

Looking at the opal side, I would be much more comfortable if you could
nail down what you intend to have these support now, even if OPAL bails
with OPAL_PARAMETER if they're specified.

Also I think u64 and u32 are preferred, although I see the rest of the
file uses the long form.

>  /* Internal functions */
>  extern int early_init_dt_scan_opal(unsigned long node, const char *uname,
>  int depth, void *data);
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index bd5bf66bd920..b28835b861b3 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -17,6 +17,7 @@
>  
>  struct perchip_nest_info nest_perchip_info[IMC_MAX_CHIPS];
>  struct imc_pmu *per_nest_pmu_arr[IMC_MAX_PMUS];
> +static cpumask_t nest_imc_cpumask;
>  
>  /* Needed for sanity check */
>  extern u64 nest_max_offset;
> @@ -32,6 +33,152 @@ static struct attribute_group imc_format_group = {
>   .attrs = imc_format_attrs,
>  };
>  
> +/* Get the cpumask printed to a buffer "buf" */
> +static ssize_t imc_pmu_cpumask_get_attr(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + cpumask_t *active_mask;
> +
> + active_mask = _imc_cpumask;
> + return cpumap_print_to_pagebuf(true, buf, active_mask);
> +}
> +
> +static 

Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets

[Vivek Gautam]

On 04/04/2017 09:44 AM, kbuild test robot wrote:

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-x004-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
 # save the attached .config to linux build tree
 make ARCH=x86_64

All warnings (new ones prefixed by >>):

drivers/reset/core.c: In function 'reset_control_array_deassert':

drivers/reset/core.c:526:2: warning: this 'for' clause does not guard... 
[-Wmisleading-indentation]

  for (i = 0; i < num_rsts; i++)
  ^~~
drivers/reset/core.c:528:3: note: ...this statement, but the latter is 
misleadingly indented as if it is guarded by the 'for'
   if (ret)
   ^~


Right, gonna fix these warnings in the next version of the patch.
Thanks


vim +/for +526 drivers/reset/core.c

510 /**
511  * reset_control_array_deassert: deassert a list of resets
512  *
513  * @resets: reset control array holding info about a list of resets
514  * @num_rsts: number of resets to be deasserted.
515  *
516  * Returns 0 on success or error number on failure.
517  */
518 int reset_control_array_deassert(int num_rsts,
519 struct reset_control_array *resets)
520 {
521 int ret, i;
522 
523 if (WARN_ON(IS_ERR_OR_NULL(resets)))
524 return -EINVAL;
525 
  > 526  for (i = 0; i < num_rsts; i++)
527 ret = reset_control_deassert(resets[i].rst);
528 if (ret)
529 goto err;
530 
531 return 0;
532 
533 err:
534 while (--i >= 0)

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


BRs
Vivek

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets

[Vivek Gautam]

On 04/04/2017 09:44 AM, kbuild test robot wrote:

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-x004-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
 # save the attached .config to linux build tree
 make ARCH=x86_64

All warnings (new ones prefixed by >>):

drivers/reset/core.c: In function 'reset_control_array_deassert':

drivers/reset/core.c:526:2: warning: this 'for' clause does not guard... 
[-Wmisleading-indentation]

  for (i = 0; i < num_rsts; i++)
  ^~~
drivers/reset/core.c:528:3: note: ...this statement, but the latter is 
misleadingly indented as if it is guarded by the 'for'
   if (ret)
   ^~


Right, gonna fix these warnings in the next version of the patch.
Thanks


vim +/for +526 drivers/reset/core.c

510 /**
511  * reset_control_array_deassert: deassert a list of resets
512  *
513  * @resets: reset control array holding info about a list of resets
514  * @num_rsts: number of resets to be deasserted.
515  *
516  * Returns 0 on success or error number on failure.
517  */
518 int reset_control_array_deassert(int num_rsts,
519 struct reset_control_array *resets)
520 {
521 int ret, i;
522 
523 if (WARN_ON(IS_ERR_OR_NULL(resets)))
524 return -EINVAL;
525 
  > 526  for (i = 0; i < num_rsts; i++)
527 ret = reset_control_deassert(resets[i].rst);
528 if (ret)
529 goto err;
530 
531 return 0;
532 
533 err:
534 while (--i >= 0)

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


BRs
Vivek

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 1/4] reset: Add API to count number of reset available with device

[kbuild test robot]

Hi Vivek,

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: i386-randconfig-r0-201714 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from drivers/usb/dwc2/platform.c:48:0:
>> include/linux/reset.h:84:1: error: expected identifier or '(' before '{' 
>> token
{
^
   include/linux/reset.h:83:19: warning: 'of_reset_control_get_count' declared 
'static' but never defined [-Wunused-function]
static inline int of_reset_control_get_count(struct device_node *node);
  ^

vim +84 include/linux/reset.h

78  int index, bool shared, bool 
optional)
79  {
80  return optional ? NULL : ERR_PTR(-ENOTSUPP);
81  }
82  
83  static inline int of_reset_control_get_count(struct device_node *node);
  > 84  {
85  return -ENOTSUPP;
86  }
87  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v2 1/4] reset: Add API to count number of reset available with device

[kbuild test robot]

Hi Vivek,

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: i386-randconfig-r0-201714 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from drivers/usb/dwc2/platform.c:48:0:
>> include/linux/reset.h:84:1: error: expected identifier or '(' before '{' 
>> token
{
^
   include/linux/reset.h:83:19: warning: 'of_reset_control_get_count' declared 
'static' but never defined [-Wunused-function]
static inline int of_reset_control_get_count(struct device_node *node);
  ^

vim +84 include/linux/reset.h

78  int index, bool shared, bool 
optional)
79  {
80  return optional ? NULL : ERR_PTR(-ENOTSUPP);
81  }
82  
83  static inline int of_reset_control_get_count(struct device_node *node);
  > 84  {
85  return -ENOTSUPP;
86  }
87  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets

[kbuild test robot]

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-x004-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/reset/core.c: In function 'reset_control_array_deassert':
>> drivers/reset/core.c:526:2: warning: this 'for' clause does not guard... 
>> [-Wmisleading-indentation]
 for (i = 0; i < num_rsts; i++)
 ^~~
   drivers/reset/core.c:528:3: note: ...this statement, but the latter is 
misleadingly indented as if it is guarded by the 'for'
  if (ret)
  ^~

vim +/for +526 drivers/reset/core.c

   510  /**
   511   * reset_control_array_deassert: deassert a list of resets
   512   *
   513   * @resets: reset control array holding info about a list of resets
   514   * @num_rsts: number of resets to be deasserted.
   515   *
   516   * Returns 0 on success or error number on failure.
   517   */
   518  int reset_control_array_deassert(int num_rsts,
   519  struct reset_control_array *resets)
   520  {
   521  int ret, i;
   522  
   523  if (WARN_ON(IS_ERR_OR_NULL(resets)))
   524  return -EINVAL;
   525  
 > 526  for (i = 0; i < num_rsts; i++)
   527  ret = reset_control_deassert(resets[i].rst);
   528  if (ret)
   529  goto err;
   530  
   531  return 0;
   532  
   533  err:
   534  while (--i >= 0)

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets

[kbuild test robot]

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-x004-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/reset/core.c: In function 'reset_control_array_deassert':
>> drivers/reset/core.c:526:2: warning: this 'for' clause does not guard... 
>> [-Wmisleading-indentation]
 for (i = 0; i < num_rsts; i++)
 ^~~
   drivers/reset/core.c:528:3: note: ...this statement, but the latter is 
misleadingly indented as if it is guarded by the 'for'
  if (ret)
  ^~

vim +/for +526 drivers/reset/core.c

   510  /**
   511   * reset_control_array_deassert: deassert a list of resets
   512   *
   513   * @resets: reset control array holding info about a list of resets
   514   * @num_rsts: number of resets to be deasserted.
   515   *
   516   * Returns 0 on success or error number on failure.
   517   */
   518  int reset_control_array_deassert(int num_rsts,
   519  struct reset_control_array *resets)
   520  {
   521  int ret, i;
   522  
   523  if (WARN_ON(IS_ERR_OR_NULL(resets)))
   524  return -EINVAL;
   525  
 > 526  for (i = 0; i < num_rsts; i++)
   527  ret = reset_control_deassert(resets[i].rst);
   528  if (ret)
   529  goto err;
   530  
   531  return 0;
   532  
   533  err:
   534  while (--i >= 0)

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets

[kbuild test robot]

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-x008-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/reset/core.c: In function 'reset_control_array_deassert':
   drivers/reset/core.c:526:2: warning: this 'for' clause does not guard... 
[-Wmisleading-indentation]
 for (i = 0; i < num_rsts; i++)
 ^~~
   drivers/reset/core.c:528:3: note: ...this statement, but the latter is 
misleadingly indented as if it is guarded by the 'for'
  if (ret)
  ^~
>> drivers/reset/core.c:531:9: warning: 'ret' may be used uninitialized in this 
>> function [-Wmaybe-uninitialized]
 return 0;
^

vim +/ret +531 drivers/reset/core.c

   520  {
   521  int ret, i;
   522  
   523  if (WARN_ON(IS_ERR_OR_NULL(resets)))
   524  return -EINVAL;
   525  
 > 526  for (i = 0; i < num_rsts; i++)
   527  ret = reset_control_deassert(resets[i].rst);
   528  if (ret)
   529  goto err;
   530  
 > 531  return 0;
   532  
   533  err:
   534  while (--i >= 0)

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v2 2/4] reset: Add APIs to manage array of resets

[kbuild test robot]

Hi Vivek,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.11-rc5 next-20170403]
[cannot apply to pza/reset/next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Vivek-Gautam/reset-APIs-to-manage-a-list-of-resets/20170404-111639
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-x008-201714 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/reset/core.c: In function 'reset_control_array_deassert':
   drivers/reset/core.c:526:2: warning: this 'for' clause does not guard... 
[-Wmisleading-indentation]
 for (i = 0; i < num_rsts; i++)
 ^~~
   drivers/reset/core.c:528:3: note: ...this statement, but the latter is 
misleadingly indented as if it is guarded by the 'for'
  if (ret)
  ^~
>> drivers/reset/core.c:531:9: warning: 'ret' may be used uninitialized in this 
>> function [-Wmaybe-uninitialized]
 return 0;
^

vim +/ret +531 drivers/reset/core.c

   520  {
   521  int ret, i;
   522  
   523  if (WARN_ON(IS_ERR_OR_NULL(resets)))
   524  return -EINVAL;
   525  
 > 526  for (i = 0; i < num_rsts; i++)
   527  ret = reset_control_deassert(resets[i].rst);
   528  if (ret)
   529  goto err;
   530  
 > 531  return 0;
   532  
   533  err:
   534  while (--i >= 0)

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [RFC][PATCH] spin loop arch primitives for busy waiting

[Nicholas Piggin]

On Tue, 4 Apr 2017 13:02:33 +1000
Nicholas Piggin  wrote:

> On Mon, 3 Apr 2017 17:43:05 -0700
> Linus Torvalds  wrote:
> 

> > But that depends on architectures having some pattern that we *can*
> > abstract. Would some "begin/in-loop/end" pattern like the above be
> > sufficient?  
> 
> Yes. begin/in/end would be sufficient for powerpc SMT priority, and
> for x86, and it looks like sparc64 too. So we could do that if you
> prefer.

How's this? I changed your name a bit just so we have a common spin_
prefix. With example powerpc implementation and one caller converted
to see the effect.

---
 arch/powerpc/include/asm/processor.h | 17 +
 include/linux/processor.h| 48 
 kernel/sched/idle.c  |  7 +-
 3 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/processor.h

diff --git a/arch/powerpc/include/asm/processor.h 
b/arch/powerpc/include/asm/processor.h
index e9bbd450d966..1274dc818e74 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -402,6 +402,23 @@ static inline unsigned long __pack_fe01(unsigned int 
fpmode)
 
 #ifdef CONFIG_PPC64
 #define cpu_relax()do { HMT_low(); HMT_medium(); barrier(); } while (0)
+
+#ifndef spin_begin
+#define spin_begin()   HMT_low()
+#endif
+
+#ifndef spin_cpu_relax
+#define spin_cpu_relax()   barrier()
+#endif
+
+#ifndef spin_cpu_yield
+#define spin_cpu_yield()
+#endif
+
+#ifndef spin_end
+#define spin_end() HMT_medium()
+#endif
+
 #else
 #define cpu_relax()barrier()
 #endif
diff --git a/include/linux/processor.h b/include/linux/processor.h
new file mode 100644
index ..65e5635d0069
--- /dev/null
+++ b/include/linux/processor.h
@@ -0,0 +1,48 @@
+/* Misc low level processor primitives */
+#ifndef _LINUX_PROCESSOR_H
+#define _LINUX_PROCESSOR_H
+
+#include 
+
+/*
+ * spin_begin is used before beginning a busy-wait loop, and must be paired
+ * with spin_end when the loop is exited. spin_cpu_relax must be called
+ * within the loop.
+ *
+ * These loop body should be as small and fast as possible, on the order of
+ * tens of instructions/cycles as a guide. It should and avoid calling
+ * cpu_relax, or any "spin" or sleep type of primitive including nested uses
+ * of these primitives. It should not lock or take any other resource.
+ * Violations of this will not cause a bug, but may cause sub optimal
+ * performance.
+ *
+ * These loops are optimized to be used where wait times are expected to be
+ * less than the cost of a context switch (and associated overhead).
+ *
+ * Detection of resource owner and decision to spin or sleep or guest-yield
+ * (e.g., spin lock holder vcpu preempted, or mutex owner not on CPU) can be
+ * tested within the busy loop body if necessary.
+ */
+#ifndef spin_begin
+#define spin_begin()
+#endif
+
+#ifndef spin_cpu_relax
+#define spin_cpu_relax() cpu_relax()
+#endif
+
+/*
+ * spin_cpu_yield may be called to yield (undirected) to the hypervisor if
+ * necessary. This should be used if the wait is expected to take longer
+ * than context switch overhead, but we can't sleep or do a directed yield.
+ */
+#ifndef spin_cpu_yield
+#define spin_cpu_yield() cpu_relax_yield()
+#endif
+
+#ifndef spin_end
+#define spin_end()
+#endif
+
+#endif /* _LINUX_PROCESSOR_H */
+
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index ac6d5176463d..99a032d9f4a9 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -63,9 +64,13 @@ static noinline int __cpuidle cpu_idle_poll(void)
trace_cpu_idle_rcuidle(0, smp_processor_id());
local_irq_enable();
stop_critical_timings();
+
+   spin_begin();
while (!tif_need_resched() &&
(cpu_idle_force_poll || tick_check_broadcast_expired()))
-   cpu_relax();
+   spin_cpu_relax();
+   spin_end();
+
start_critical_timings();
trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
rcu_idle_exit();
-- 
2.11.0

Re: [RFC][PATCH] spin loop arch primitives for busy waiting

[Nicholas Piggin]

On Tue, 4 Apr 2017 13:02:33 +1000
Nicholas Piggin  wrote:

> On Mon, 3 Apr 2017 17:43:05 -0700
> Linus Torvalds  wrote:
> 

> > But that depends on architectures having some pattern that we *can*
> > abstract. Would some "begin/in-loop/end" pattern like the above be
> > sufficient?  
> 
> Yes. begin/in/end would be sufficient for powerpc SMT priority, and
> for x86, and it looks like sparc64 too. So we could do that if you
> prefer.

How's this? I changed your name a bit just so we have a common spin_
prefix. With example powerpc implementation and one caller converted
to see the effect.

---
 arch/powerpc/include/asm/processor.h | 17 +
 include/linux/processor.h| 48 
 kernel/sched/idle.c  |  7 +-
 3 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/processor.h

diff --git a/arch/powerpc/include/asm/processor.h 
b/arch/powerpc/include/asm/processor.h
index e9bbd450d966..1274dc818e74 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -402,6 +402,23 @@ static inline unsigned long __pack_fe01(unsigned int 
fpmode)
 
 #ifdef CONFIG_PPC64
 #define cpu_relax()do { HMT_low(); HMT_medium(); barrier(); } while (0)
+
+#ifndef spin_begin
+#define spin_begin()   HMT_low()
+#endif
+
+#ifndef spin_cpu_relax
+#define spin_cpu_relax()   barrier()
+#endif
+
+#ifndef spin_cpu_yield
+#define spin_cpu_yield()
+#endif
+
+#ifndef spin_end
+#define spin_end() HMT_medium()
+#endif
+
 #else
 #define cpu_relax()barrier()
 #endif
diff --git a/include/linux/processor.h b/include/linux/processor.h
new file mode 100644
index ..65e5635d0069
--- /dev/null
+++ b/include/linux/processor.h
@@ -0,0 +1,48 @@
+/* Misc low level processor primitives */
+#ifndef _LINUX_PROCESSOR_H
+#define _LINUX_PROCESSOR_H
+
+#include 
+
+/*
+ * spin_begin is used before beginning a busy-wait loop, and must be paired
+ * with spin_end when the loop is exited. spin_cpu_relax must be called
+ * within the loop.
+ *
+ * These loop body should be as small and fast as possible, on the order of
+ * tens of instructions/cycles as a guide. It should and avoid calling
+ * cpu_relax, or any "spin" or sleep type of primitive including nested uses
+ * of these primitives. It should not lock or take any other resource.
+ * Violations of this will not cause a bug, but may cause sub optimal
+ * performance.
+ *
+ * These loops are optimized to be used where wait times are expected to be
+ * less than the cost of a context switch (and associated overhead).
+ *
+ * Detection of resource owner and decision to spin or sleep or guest-yield
+ * (e.g., spin lock holder vcpu preempted, or mutex owner not on CPU) can be
+ * tested within the busy loop body if necessary.
+ */
+#ifndef spin_begin
+#define spin_begin()
+#endif
+
+#ifndef spin_cpu_relax
+#define spin_cpu_relax() cpu_relax()
+#endif
+
+/*
+ * spin_cpu_yield may be called to yield (undirected) to the hypervisor if
+ * necessary. This should be used if the wait is expected to take longer
+ * than context switch overhead, but we can't sleep or do a directed yield.
+ */
+#ifndef spin_cpu_yield
+#define spin_cpu_yield() cpu_relax_yield()
+#endif
+
+#ifndef spin_end
+#define spin_end()
+#endif
+
+#endif /* _LINUX_PROCESSOR_H */
+
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index ac6d5176463d..99a032d9f4a9 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -63,9 +64,13 @@ static noinline int __cpuidle cpu_idle_poll(void)
trace_cpu_idle_rcuidle(0, smp_processor_id());
local_irq_enable();
stop_critical_timings();
+
+   spin_begin();
while (!tif_need_resched() &&
(cpu_idle_force_poll || tick_check_broadcast_expired()))
-   cpu_relax();
+   spin_cpu_relax();
+   spin_end();
+
start_critical_timings();
trace_cpu_idle_rcuidle(PWR_EVENT_EXIT, smp_processor_id());
rcu_idle_exit();
-- 
2.11.0

Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

[Mike Galbraith]

On Mon, 2017-04-03 at 21:11 +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 03, 2017 at 07:56:32PM +0200, Mike Galbraith wrote:
> > On Mon, 2017-04-03 at 16:18 +0200, Christoph Hellwig wrote:
> > > Mike,
> > > 
> > > can you try the patch below?
> > 
> > No more spinning kworker woes, but I still have a warning on hibernate,
> > threadirqs invariant.  I'm also seeing intermittent post hibernate hang
> > funnies in virgin source +- this patch, and without threadirqs.
> > 
> > [  110.223953] WARNING: CPU: 5 PID: 452 at drivers/pci/msi.c:1261 
> > pci_irq_vector+0xb1/0xe0
> > 
> > > > -Mike
> 
> I just sent a patch fixing that.
> However I think we want to print a message when MSI fails to work so we
> know guest is falling back on legacy interrupts.

The warning persists.

[  137.656423] WARNING: CPU: 1 PID: 535 at drivers/pci/msi.c:1261 
pci_irq_vector+0xb1/0xe0

WRT the post hibernate hang business, that is apparently not part of
the 4.11 woes (at least not solely), as 4.10.8 did not survive a 10
hibernate cycle loop.  RT is better at reproducing trouble (shrug, it
frequently is), but it matters not whether I'm running 4.10, master or
master-rt, they will all hang.

WRT gripe, I wedged virtio_pci-fix-msix-vector-tracking-on-cleanup in
on top, but it wasn't impressed.

-Mike

Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

[Mike Galbraith]

On Mon, 2017-04-03 at 21:11 +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 03, 2017 at 07:56:32PM +0200, Mike Galbraith wrote:
> > On Mon, 2017-04-03 at 16:18 +0200, Christoph Hellwig wrote:
> > > Mike,
> > > 
> > > can you try the patch below?
> > 
> > No more spinning kworker woes, but I still have a warning on hibernate,
> > threadirqs invariant.  I'm also seeing intermittent post hibernate hang
> > funnies in virgin source +- this patch, and without threadirqs.
> > 
> > [  110.223953] WARNING: CPU: 5 PID: 452 at drivers/pci/msi.c:1261 
> > pci_irq_vector+0xb1/0xe0
> > 
> > > > -Mike
> 
> I just sent a patch fixing that.
> However I think we want to print a message when MSI fails to work so we
> know guest is falling back on legacy interrupts.

The warning persists.

[  137.656423] WARNING: CPU: 1 PID: 535 at drivers/pci/msi.c:1261 
pci_irq_vector+0xb1/0xe0

WRT the post hibernate hang business, that is apparently not part of
the 4.11 woes (at least not solely), as 4.10.8 did not survive a 10
hibernate cycle loop.  RT is better at reproducing trouble (shrug, it
frequently is), but it matters not whether I'm running 4.10, master or
master-rt, they will all hang.

WRT gripe, I wedged virtio_pci-fix-msix-vector-tracking-on-cleanup in
on top, but it wasn't impressed.

-Mike

Re: [PATCH v6 05/11] powerpc/perf: Generic imc pmu event functions

[Daniel Axtens]

Hi,

So a major complaint I have is that you're changing prototypes of
functions from earlier patches.

This makes my life a lot harder: I get my head around what a function is
and does and then suddenly the prototype changes, the behaviour changes,
and I have to re-evaluate everything I thought I knew about the
function. The diffs are also usually quite unhelpful.

It would be far better, from my point of view, to squash related commits
together. You're adding a large-ish driver: we might as well review one
large, complete commit rather than several smaller incomplete commits.

There are a lot of interrelated benefits to this - just from the patches
I've reviewed so far, if there were fewer, larger commits then:

 - I would only have to read a function once to get a full picture of
   what it does

 - comments in function headers wouldn't get out of sync with function
   bodies

 - there would be less movement of variables from file to file

 - earlier comments wouldn't be invalidated by things I learn later. For
   example I suggested different names for imc_event_info_{str,val}
   based on their behaviour when first added in patch 3. Here you change
   the behaviour of imc_event_info_val - it would have been helpful to
   see the fuller picture when I was first reviewing as I would have
   been able to make more helpful suggestions.

and so on.

Anyway, further comments in line.

> From: Hemant Kumar 
>
> Since, the IMC counters' data are periodically fed to a memory location,
> the functions to read/update, start/stop, add/del can be generic and can
> be used by all IMC PMU units.
>
> This patch adds a set of generic imc pmu related event functions to be
> used  by each imc pmu unit. Add code to setup format attribute and to
> register imc pmus. Add a event_init function for nest_imc events.
>
> Signed-off-by: Anju T Sudhakar 
> Signed-off-by: Hemant Kumar 
> Signed-off-by: Madhavan Srinivasan 
> ---
>  arch/powerpc/include/asm/imc-pmu.h|   1 +
>  arch/powerpc/perf/imc-pmu.c   | 137 
> ++
>  arch/powerpc/platforms/powernv/opal-imc.c |  30 ++-
>  3 files changed, 164 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/imc-pmu.h 
> b/arch/powerpc/include/asm/imc-pmu.h
> index a3d4f1bf9492..e13f51047522 100644
> --- a/arch/powerpc/include/asm/imc-pmu.h
> +++ b/arch/powerpc/include/asm/imc-pmu.h
> @@ -65,4 +65,5 @@ struct imc_pmu {
>  #define IMC_DOMAIN_NEST  1
>  #define IMC_DOMAIN_UNKNOWN   -1
>  
> +int imc_get_domain(struct device_node *pmu_dev);
>  #endif /* PPC_POWERNV_IMC_PMU_DEF_H */
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index ec464c76b749..bd5bf66bd920 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -18,6 +18,132 @@
>  struct perchip_nest_info nest_perchip_info[IMC_MAX_CHIPS];
>  struct imc_pmu *per_nest_pmu_arr[IMC_MAX_PMUS];
>  
> +/* Needed for sanity check */
> +extern u64 nest_max_offset;
Should this be in a header file?

> +
> +PMU_FORMAT_ATTR(event, "config:0-20");
> +static struct attribute *imc_format_attrs[] = {
> + _attr_event.attr,
> + NULL,
> +};
> +
> +static struct attribute_group imc_format_group = {
> + .name = "format",
> + .attrs = imc_format_attrs,
> +};
> +
> +static int nest_imc_event_init(struct perf_event *event)
> +{
> + int chip_id;
> + u32 config = event->attr.config;
> + struct perchip_nest_info *pcni;
> +
> + if (event->attr.type != event->pmu->type)
> + return -ENOENT;
> +
> + /* Sampling not supported */
> + if (event->hw.sample_period)
> + return -EINVAL;
> +
> + /* unsupported modes and filters */
> + if (event->attr.exclude_user   ||
> + event->attr.exclude_kernel ||
> + event->attr.exclude_hv ||
> + event->attr.exclude_idle   ||
> + event->attr.exclude_host   ||
> + event->attr.exclude_guest)
> + return -EINVAL;
> +
> + if (event->cpu < 0)
> + return -EINVAL;
> +
> + /* Sanity check for config (event offset) */
> + if (config > nest_max_offset)
> + return -EINVAL;
config is a u32, nest_max_offset is a u64. Is there a reason for this?
(Also, config is an unintuitive name for the local variable - the data
is stored in the attribute config space but the value represents an
offset into the reserved memory region.)

> +
> + chip_id = topology_physical_package_id(event->cpu);
> + pcni = _perchip_info[chip_id];
> +
> + /*
> +  * Memory for Nest HW counter data could be in multiple pages.
> +  * Hence check and pick the right base page from the event offset,
> +  * and then add to it.
> +  */
> + event->hw.event_base = pcni->vbase[config/PAGE_SIZE] +
> + (config & 

Re: [PATCH v6 05/11] powerpc/perf: Generic imc pmu event functions

[Daniel Axtens]

Hi,

So a major complaint I have is that you're changing prototypes of
functions from earlier patches.

This makes my life a lot harder: I get my head around what a function is
and does and then suddenly the prototype changes, the behaviour changes,
and I have to re-evaluate everything I thought I knew about the
function. The diffs are also usually quite unhelpful.

It would be far better, from my point of view, to squash related commits
together. You're adding a large-ish driver: we might as well review one
large, complete commit rather than several smaller incomplete commits.

There are a lot of interrelated benefits to this - just from the patches
I've reviewed so far, if there were fewer, larger commits then:

 - I would only have to read a function once to get a full picture of
   what it does

 - comments in function headers wouldn't get out of sync with function
   bodies

 - there would be less movement of variables from file to file

 - earlier comments wouldn't be invalidated by things I learn later. For
   example I suggested different names for imc_event_info_{str,val}
   based on their behaviour when first added in patch 3. Here you change
   the behaviour of imc_event_info_val - it would have been helpful to
   see the fuller picture when I was first reviewing as I would have
   been able to make more helpful suggestions.

and so on.

Anyway, further comments in line.

> From: Hemant Kumar 
>
> Since, the IMC counters' data are periodically fed to a memory location,
> the functions to read/update, start/stop, add/del can be generic and can
> be used by all IMC PMU units.
>
> This patch adds a set of generic imc pmu related event functions to be
> used  by each imc pmu unit. Add code to setup format attribute and to
> register imc pmus. Add a event_init function for nest_imc events.
>
> Signed-off-by: Anju T Sudhakar 
> Signed-off-by: Hemant Kumar 
> Signed-off-by: Madhavan Srinivasan 
> ---
>  arch/powerpc/include/asm/imc-pmu.h|   1 +
>  arch/powerpc/perf/imc-pmu.c   | 137 
> ++
>  arch/powerpc/platforms/powernv/opal-imc.c |  30 ++-
>  3 files changed, 164 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/imc-pmu.h 
> b/arch/powerpc/include/asm/imc-pmu.h
> index a3d4f1bf9492..e13f51047522 100644
> --- a/arch/powerpc/include/asm/imc-pmu.h
> +++ b/arch/powerpc/include/asm/imc-pmu.h
> @@ -65,4 +65,5 @@ struct imc_pmu {
>  #define IMC_DOMAIN_NEST  1
>  #define IMC_DOMAIN_UNKNOWN   -1
>  
> +int imc_get_domain(struct device_node *pmu_dev);
>  #endif /* PPC_POWERNV_IMC_PMU_DEF_H */
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index ec464c76b749..bd5bf66bd920 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -18,6 +18,132 @@
>  struct perchip_nest_info nest_perchip_info[IMC_MAX_CHIPS];
>  struct imc_pmu *per_nest_pmu_arr[IMC_MAX_PMUS];
>  
> +/* Needed for sanity check */
> +extern u64 nest_max_offset;
Should this be in a header file?

> +
> +PMU_FORMAT_ATTR(event, "config:0-20");
> +static struct attribute *imc_format_attrs[] = {
> + _attr_event.attr,
> + NULL,
> +};
> +
> +static struct attribute_group imc_format_group = {
> + .name = "format",
> + .attrs = imc_format_attrs,
> +};
> +
> +static int nest_imc_event_init(struct perf_event *event)
> +{
> + int chip_id;
> + u32 config = event->attr.config;
> + struct perchip_nest_info *pcni;
> +
> + if (event->attr.type != event->pmu->type)
> + return -ENOENT;
> +
> + /* Sampling not supported */
> + if (event->hw.sample_period)
> + return -EINVAL;
> +
> + /* unsupported modes and filters */
> + if (event->attr.exclude_user   ||
> + event->attr.exclude_kernel ||
> + event->attr.exclude_hv ||
> + event->attr.exclude_idle   ||
> + event->attr.exclude_host   ||
> + event->attr.exclude_guest)
> + return -EINVAL;
> +
> + if (event->cpu < 0)
> + return -EINVAL;
> +
> + /* Sanity check for config (event offset) */
> + if (config > nest_max_offset)
> + return -EINVAL;
config is a u32, nest_max_offset is a u64. Is there a reason for this?
(Also, config is an unintuitive name for the local variable - the data
is stored in the attribute config space but the value represents an
offset into the reserved memory region.)

> +
> + chip_id = topology_physical_package_id(event->cpu);
> + pcni = _perchip_info[chip_id];
> +
> + /*
> +  * Memory for Nest HW counter data could be in multiple pages.
> +  * Hence check and pick the right base page from the event offset,
> +  * and then add to it.
> +  */
> + event->hw.event_base = pcni->vbase[config/PAGE_SIZE] +
> + (config & ~PAGE_MASK);
> +
> + return 0;
> +}
> +
> +static void imc_read_counter(struct perf_event *event)
> +{
> + 

[PATCH v2 2/2] usb: misc: refactor code

[Gustavo A. R. Silva]

Code refactoring to make the flow easier to follow.

Cc: Alan Stern 
Cc: Greg Kroah-Hartman 
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 Remove unfruitful changes in previous patch.
 Revert change to comment.

 drivers/usb/misc/usbtest.c | 49 --
 1 file changed, 21 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 7bfb6b78..88f627e 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -124,6 +124,20 @@ static struct usb_device *testdev_to_usbdev(struct 
usbtest_dev *test)
 
 /*-*/
 
+static inline void endpoint_update(int edi,
+  struct usb_host_endpoint **in,
+  struct usb_host_endpoint **out,
+  struct usb_host_endpoint *e)
+{
+   if (edi) {
+   if (!*in)
+   *in = e;
+   } else {
+   if (!*out)
+   *out = e;
+   }
+}
+
 static int
 get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)
 {
@@ -151,47 +165,26 @@ get_endpoints(struct usbtest_dev *dev, struct 
usb_interface *intf)
 */
for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
struct usb_host_endpoint*e;
+   int edi;
 
e = alt->endpoint + ep;
+   edi = usb_endpoint_dir_in(>desc);
+
switch (usb_endpoint_type(>desc)) {
case USB_ENDPOINT_XFER_BULK:
-   break;
+   endpoint_update(edi, , , e);
+   continue;
case USB_ENDPOINT_XFER_INT:
if (dev->info->intr)
-   goto try_intr;
+   endpoint_update(edi, _in, _out, 
e);
continue;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
-   goto try_iso;
+   endpoint_update(edi, _in, _out, 
e);
/* FALLTHROUGH */
default:
continue;
}
-   if (usb_endpoint_dir_in(>desc)) {
-   if (!in)
-   in = e;
-   } else {
-   if (!out)
-   out = e;
-   }
-   continue;
-try_intr:
-   if (usb_endpoint_dir_in(>desc)) {
-   if (!int_in)
-   int_in = e;
-   } else {
-   if (!int_out)
-   int_out = e;
-   }
-   continue;
-try_iso:
-   if (usb_endpoint_dir_in(>desc)) {
-   if (!iso_in)
-   iso_in = e;
-   } else {
-   if (!iso_out)
-   iso_out = e;
-   }
}
if ((in && out)  ||  iso_in || iso_out || int_in || int_out)
goto found;
-- 
2.5.0

[PATCH v2 2/2] usb: misc: refactor code

[Gustavo A. R. Silva]

Code refactoring to make the flow easier to follow.

Cc: Alan Stern 
Cc: Greg Kroah-Hartman 
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 Remove unfruitful changes in previous patch.
 Revert change to comment.

 drivers/usb/misc/usbtest.c | 49 --
 1 file changed, 21 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 7bfb6b78..88f627e 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -124,6 +124,20 @@ static struct usb_device *testdev_to_usbdev(struct 
usbtest_dev *test)
 
 /*-*/
 
+static inline void endpoint_update(int edi,
+  struct usb_host_endpoint **in,
+  struct usb_host_endpoint **out,
+  struct usb_host_endpoint *e)
+{
+   if (edi) {
+   if (!*in)
+   *in = e;
+   } else {
+   if (!*out)
+   *out = e;
+   }
+}
+
 static int
 get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)
 {
@@ -151,47 +165,26 @@ get_endpoints(struct usbtest_dev *dev, struct 
usb_interface *intf)
 */
for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
struct usb_host_endpoint*e;
+   int edi;
 
e = alt->endpoint + ep;
+   edi = usb_endpoint_dir_in(>desc);
+
switch (usb_endpoint_type(>desc)) {
case USB_ENDPOINT_XFER_BULK:
-   break;
+   endpoint_update(edi, , , e);
+   continue;
case USB_ENDPOINT_XFER_INT:
if (dev->info->intr)
-   goto try_intr;
+   endpoint_update(edi, _in, _out, 
e);
continue;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
-   goto try_iso;
+   endpoint_update(edi, _in, _out, 
e);
/* FALLTHROUGH */
default:
continue;
}
-   if (usb_endpoint_dir_in(>desc)) {
-   if (!in)
-   in = e;
-   } else {
-   if (!out)
-   out = e;
-   }
-   continue;
-try_intr:
-   if (usb_endpoint_dir_in(>desc)) {
-   if (!int_in)
-   int_in = e;
-   } else {
-   if (!int_out)
-   int_out = e;
-   }
-   continue;
-try_iso:
-   if (usb_endpoint_dir_in(>desc)) {
-   if (!iso_in)
-   iso_in = e;
-   } else {
-   if (!iso_out)
-   iso_out = e;
-   }
}
if ((in && out)  ||  iso_in || iso_out || int_in || int_out)
goto found;
-- 
2.5.0

[PATCH v2 1/2] usb: misc: add missing continue in switch

[Gustavo A. R. Silva]

Add missing continue in switch.

Addresses-Coverity-ID: 1248733
Cc: Alan Stern 
Cc: Greg Kroah-Hartman 
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 None.

 drivers/usb/misc/usbtest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 3525626..7bfb6b78 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -159,6 +159,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
case USB_ENDPOINT_XFER_INT:
if (dev->info->intr)
goto try_intr;
+   continue;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
goto try_iso;
-- 
2.5.0

[PATCH v2 1/2] usb: misc: add missing continue in switch

[Gustavo A. R. Silva]

Add missing continue in switch.

Addresses-Coverity-ID: 1248733
Cc: Alan Stern 
Cc: Greg Kroah-Hartman 
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 None.

 drivers/usb/misc/usbtest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 3525626..7bfb6b78 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -159,6 +159,7 @@ get_endpoints(struct usbtest_dev *dev, struct usb_interface 
*intf)
case USB_ENDPOINT_XFER_INT:
if (dev->info->intr)
goto try_intr;
+   continue;
case USB_ENDPOINT_XFER_ISOC:
if (dev->info->iso)
goto try_iso;
-- 
2.5.0

[PATCH v3] regulator: hi655x: Correct dependency/boot failure

[Jeremy Linton]

The hi655x-regulator driver consumes a similarly named platform device.
Adding that to the module device table, allows modprobe to locate this
driver once the device is created. Without this the sd/mmc device fails
to start, resulting in boot failures.

Signed-off-by: Jeremy Linton 
---
 drivers/regulator/hi655x-regulator.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/regulator/hi655x-regulator.c 
b/drivers/regulator/hi655x-regulator.c
index 065c100..36ae54b 100644
--- a/drivers/regulator/hi655x-regulator.c
+++ b/drivers/regulator/hi655x-regulator.c
@@ -214,7 +214,14 @@ static int hi655x_regulator_probe(struct platform_device 
*pdev)
return 0;
 }
 
+static const struct platform_device_id hi655x_regulator_table[] = {
+   { .name = "hi655x-regulator" },
+   {},
+};
+MODULE_DEVICE_TABLE(platform, hi655x_regulator_table);
+
 static struct platform_driver hi655x_regulator_driver = {
+   .id_table = hi655x_regulator_table,
.driver = {
.name   = "hi655x-regulator",
},
-- 
2.10.2

[PATCH v3] regulator: hi655x: Correct dependency/boot failure

[Jeremy Linton]

The hi655x-regulator driver consumes a similarly named platform device.
Adding that to the module device table, allows modprobe to locate this
driver once the device is created. Without this the sd/mmc device fails
to start, resulting in boot failures.

Signed-off-by: Jeremy Linton 
---
 drivers/regulator/hi655x-regulator.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/regulator/hi655x-regulator.c 
b/drivers/regulator/hi655x-regulator.c
index 065c100..36ae54b 100644
--- a/drivers/regulator/hi655x-regulator.c
+++ b/drivers/regulator/hi655x-regulator.c
@@ -214,7 +214,14 @@ static int hi655x_regulator_probe(struct platform_device 
*pdev)
return 0;
 }
 
+static const struct platform_device_id hi655x_regulator_table[] = {
+   { .name = "hi655x-regulator" },
+   {},
+};
+MODULE_DEVICE_TABLE(platform, hi655x_regulator_table);
+
 static struct platform_driver hi655x_regulator_driver = {
+   .id_table = hi655x_regulator_table,
.driver = {
.name   = "hi655x-regulator",
},
-- 
2.10.2

Re: AMD IOMMU causing filesystem corruption

[Samuel Sieb]

On 04/03/2017 02:39 PM, Joerg Roedel wrote:

You have a system based on the AMD Stoney platform, on which the PCI-ATS
feature of the GPU is broken, as we recently found out.

Can you please test whether the attached patch fixes the issue on your
machine?


Yes, that works, thank you!

Now I'm curious what Windows does.  Either they don't use that feature 
or they already knew to avoid it.  In which case, why did AMD take so 
long to let the kernel developers know?

Re: AMD IOMMU causing filesystem corruption

[Samuel Sieb]

On 04/03/2017 02:39 PM, Joerg Roedel wrote:

You have a system based on the AMD Stoney platform, on which the PCI-ATS
feature of the GPU is broken, as we recently found out.

Can you please test whether the attached patch fixes the issue on your
machine?


Yes, that works, thank you!

Now I'm curious what Windows does.  Either they don't use that feature 
or they already knew to avoid it.  In which case, why did AMD take so 
long to let the kernel developers know?

Re: [PATCH v2 6/6] hpet: fix style issue about braces and alignment

[Joe Perches]

On Mon, 2017-04-03 at 14:15 +0200, Corentin Labbe wrote:
> This patch fix all style issue for braces and alignment
[]
> diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
[]
> @@ -255,9 +255,9 @@ static int hpet_open(struct inode *inode, struct file 
> *file)
>  
>   for (devp = NULL, hpetp = hpets; hpetp && !devp; hpetp = hpetp->hp_next)
>   for (i = 0; i < hpetp->hp_ntimer; i++)
> - if (hpetp->hp_dev[i].hd_flags & HPET_OPEN)
> + if (hpetp->hp_dev[i].hd_flags & HPET_OPEN) {
>   continue;
> - else {
> + } else {
>   devp = >hp_dev[i];
>   break;
>   }

Perhaps this is clearer as:

for (devp = NULL, hpetp = hpets; hpetp && !devp; hpetp = 
hpetp->hp_next) {
for (i = 0; i < hpetp->hp_ntimer; i++) {
if (!(hpetp->hp_dev[i].hd_flags & HPET_OPEN)) {
devp = >hp_dev[i];
break;
}
}
}

> @@ -304,9 +304,9 @@ hpet_read(struct file *file, char __user *buf, size_t 
> count, loff_t *ppos)
>   devp->hd_irqdata = 0;
>   spin_unlock_irq(_lock);
>  
> - if (data)
> + if (data) {
>   break;
> - else if (file->f_flags & O_NONBLOCK) {
> + } else if (file->f_flags & O_NONBLOCK) {

break; else is almost always better as break
and reduced indentation

Re: [PATCH v2 6/6] hpet: fix style issue about braces and alignment

[Joe Perches]

On Mon, 2017-04-03 at 14:15 +0200, Corentin Labbe wrote:
> This patch fix all style issue for braces and alignment
[]
> diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
[]
> @@ -255,9 +255,9 @@ static int hpet_open(struct inode *inode, struct file 
> *file)
>  
>   for (devp = NULL, hpetp = hpets; hpetp && !devp; hpetp = hpetp->hp_next)
>   for (i = 0; i < hpetp->hp_ntimer; i++)
> - if (hpetp->hp_dev[i].hd_flags & HPET_OPEN)
> + if (hpetp->hp_dev[i].hd_flags & HPET_OPEN) {
>   continue;
> - else {
> + } else {
>   devp = >hp_dev[i];
>   break;
>   }

Perhaps this is clearer as:

for (devp = NULL, hpetp = hpets; hpetp && !devp; hpetp = 
hpetp->hp_next) {
for (i = 0; i < hpetp->hp_ntimer; i++) {
if (!(hpetp->hp_dev[i].hd_flags & HPET_OPEN)) {
devp = >hp_dev[i];
break;
}
}
}

> @@ -304,9 +304,9 @@ hpet_read(struct file *file, char __user *buf, size_t 
> count, loff_t *ppos)
>   devp->hd_irqdata = 0;
>   spin_unlock_irq(_lock);
>  
> - if (data)
> + if (data) {
>   break;
> - else if (file->f_flags & O_NONBLOCK) {
> + } else if (file->f_flags & O_NONBLOCK) {

break; else is almost always better as break
and reduced indentation

[git pull] drm fixes for v4.11-rc6

[Dave Airlie]

Hi Linus,

I'm going to be away for the latter part of this week, and early next
week, so I probably won't get to dequeue any more fixes before -rc6,
if anyone has any urgent fixes they can get Daniel's help and maybe
send them to you direct, I don't know of anything that is that urgent,
hopefully I won't have a pile of stuff for rc7 and maintainers are a
bit more weary of what they send me.

This is just mostly stuff that missed rc5, from vmwgfx and msm drivers.

Dave.

The following changes since commit a71c9a1c779f2499fb2afc0553e543f18aff6edf:

  Linux 4.11-rc5 (2017-04-02 17:23:54 -0700)

are available in the git repository at:

  git://people.freedesktop.org/~airlied/linux tags/drm-fixes-for-v4.11-rc6

for you to fetch changes up to 130e35e4bbceb2c04ff0ad9b1a0bcef7acc11498:

  Merge branch 'msm-fixes-4.11-rc6' of
git://people.freedesktop.org/~robclark/linux into drm-fixes
(2017-04-04 10:13:40 +1000)


vmwgfx and msm fixes


Archit Taneja (2):
  drm/msm/dsi: Fix bug in dsi_mgr_phy_enable
  drm/msm/mdp5: Update SSPP_MAX value

Arnd Bergmann (1):
  drm/msm: adreno: fix build error without debugfs

Dave Airlie (2):
  Merge branch 'vmwgfx-fixes-4.11' of
git://people.freedesktop.org/~thomash/linux into drm-fixes
  Merge branch 'msm-fixes-4.11-rc6' of
git://people.freedesktop.org/~robclark/linux into drm-fixes

Jordan Crouse (3):
  drm/msm: Fix wrong pointer check in a5xx_destroy
  drm/msm: Don't allow zero sized buffer objects
  drm/msm: Make sure to detach the MMU during GPU cleanup

Li Qiang (1):
  drm/vmwgfx: fix integer overflow in vmw_surface_define_ioctl()

Murray McAllister (2):
  drm/vmwgfx: NULL pointer dereference in vmw_surface_define_ioctl()
  drm/vmwgfx: avoid calling vzalloc with a 0 size in vmw_get_cap_3d_ioctl()

Thomas Hellstrom (4):
  drm/vmwgfx: Type-check lookups of fence objects
  drm/ttm, drm/vmwgfx: Relax permission checking when opening surfaces
  drm/ttm: Avoid calling drm_ht_remove from atomic context
  drm/vmwgfx: Remove getparam error message

Vinay Simha BN (1):
  drm/msm/hdmi: redefinitions of macros not required

 drivers/gpu/drm/msm/adreno/a5xx_gpu.c|  6 ++-
 drivers/gpu/drm/msm/adreno/adreno_gpu.c  | 29 
 drivers/gpu/drm/msm/dsi/dsi_manager.c|  2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_audio.c|  7 ---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h |  3 +-
 drivers/gpu/drm/msm/msm_gem.c|  6 +++
 drivers/gpu/drm/msm/msm_gpu.c|  3 --
 drivers/gpu/drm/ttm/ttm_object.c | 14 +++---
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c| 79 
 drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c|  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c  | 31 ++---
 include/drm/ttm/ttm_object.h |  5 +-
 13 files changed, 112 insertions(+), 81 deletions(-)

[git pull] drm fixes for v4.11-rc6

[Dave Airlie]

Hi Linus,

I'm going to be away for the latter part of this week, and early next
week, so I probably won't get to dequeue any more fixes before -rc6,
if anyone has any urgent fixes they can get Daniel's help and maybe
send them to you direct, I don't know of anything that is that urgent,
hopefully I won't have a pile of stuff for rc7 and maintainers are a
bit more weary of what they send me.

This is just mostly stuff that missed rc5, from vmwgfx and msm drivers.

Dave.

The following changes since commit a71c9a1c779f2499fb2afc0553e543f18aff6edf:

  Linux 4.11-rc5 (2017-04-02 17:23:54 -0700)

are available in the git repository at:

  git://people.freedesktop.org/~airlied/linux tags/drm-fixes-for-v4.11-rc6

for you to fetch changes up to 130e35e4bbceb2c04ff0ad9b1a0bcef7acc11498:

  Merge branch 'msm-fixes-4.11-rc6' of
git://people.freedesktop.org/~robclark/linux into drm-fixes
(2017-04-04 10:13:40 +1000)


vmwgfx and msm fixes


Archit Taneja (2):
  drm/msm/dsi: Fix bug in dsi_mgr_phy_enable
  drm/msm/mdp5: Update SSPP_MAX value

Arnd Bergmann (1):
  drm/msm: adreno: fix build error without debugfs

Dave Airlie (2):
  Merge branch 'vmwgfx-fixes-4.11' of
git://people.freedesktop.org/~thomash/linux into drm-fixes
  Merge branch 'msm-fixes-4.11-rc6' of
git://people.freedesktop.org/~robclark/linux into drm-fixes

Jordan Crouse (3):
  drm/msm: Fix wrong pointer check in a5xx_destroy
  drm/msm: Don't allow zero sized buffer objects
  drm/msm: Make sure to detach the MMU during GPU cleanup

Li Qiang (1):
  drm/vmwgfx: fix integer overflow in vmw_surface_define_ioctl()

Murray McAllister (2):
  drm/vmwgfx: NULL pointer dereference in vmw_surface_define_ioctl()
  drm/vmwgfx: avoid calling vzalloc with a 0 size in vmw_get_cap_3d_ioctl()

Thomas Hellstrom (4):
  drm/vmwgfx: Type-check lookups of fence objects
  drm/ttm, drm/vmwgfx: Relax permission checking when opening surfaces
  drm/ttm: Avoid calling drm_ht_remove from atomic context
  drm/vmwgfx: Remove getparam error message

Vinay Simha BN (1):
  drm/msm/hdmi: redefinitions of macros not required

 drivers/gpu/drm/msm/adreno/a5xx_gpu.c|  6 ++-
 drivers/gpu/drm/msm/adreno/adreno_gpu.c  | 29 
 drivers/gpu/drm/msm/dsi/dsi_manager.c|  2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_audio.c|  7 ---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_pipe.h |  3 +-
 drivers/gpu/drm/msm/msm_gem.c|  6 +++
 drivers/gpu/drm/msm/msm_gpu.c|  3 --
 drivers/gpu/drm/ttm/ttm_object.c | 14 +++---
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c| 79 
 drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c|  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c  | 31 ++---
 include/drm/ttm/ttm_object.h |  5 +-
 13 files changed, 112 insertions(+), 81 deletions(-)

linux-next: build failure after merge of the keys tree

[Stephen Rothwell]

Hi David,

After merging the keys tree, today's linux-next build (x86_64
allmodconfig) failed like this:

security/integrity/digsig.c: In function 'integrity_init_keyring':
security/integrity/digsig.c:46:30: error: passing argument 7 of 'keyring_alloc' 
from incompatible pointer type [-Werror=incompatible-pointer-types]
 #define restrict_link_to_ima restrict_link_by_builtin_and_secondary_trusted
  ^
security/integrity/digsig.c:95:9: note: in expansion of macro 
'restrict_link_to_ima'
 restrict_link_to_ima, NULL);
 ^
In file included from include/linux/cred.h:17:0,
 from security/integrity/digsig.c:18:
include/linux/key.h:311:20: note: expected 'struct key_restriction *' but 
argument is of type 'int (*)(struct key *, const struct key_type *, const union 
key_payload *, struct key *)'
 extern struct key *keyring_alloc(const char *description, kuid_t uid, kgid_t 
gid,
^

Caused by commits

  aaf66c883813 ("KEYS: Split role of the keyring pointer for keyring restrict 
functions")
  c5faca6b4a58 ("KEYS: Use structure to capture key restriction function and 
data")

I have used the version from next-20170403 for today.

-- 
Cheers,
Stephen Rothwell

linux-next: build failure after merge of the keys tree

[Stephen Rothwell]

Hi David,

After merging the keys tree, today's linux-next build (x86_64
allmodconfig) failed like this:

security/integrity/digsig.c: In function 'integrity_init_keyring':
security/integrity/digsig.c:46:30: error: passing argument 7 of 'keyring_alloc' 
from incompatible pointer type [-Werror=incompatible-pointer-types]
 #define restrict_link_to_ima restrict_link_by_builtin_and_secondary_trusted
  ^
security/integrity/digsig.c:95:9: note: in expansion of macro 
'restrict_link_to_ima'
 restrict_link_to_ima, NULL);
 ^
In file included from include/linux/cred.h:17:0,
 from security/integrity/digsig.c:18:
include/linux/key.h:311:20: note: expected 'struct key_restriction *' but 
argument is of type 'int (*)(struct key *, const struct key_type *, const union 
key_payload *, struct key *)'
 extern struct key *keyring_alloc(const char *description, kuid_t uid, kgid_t 
gid,
^

Caused by commits

  aaf66c883813 ("KEYS: Split role of the keyring pointer for keyring restrict 
functions")
  c5faca6b4a58 ("KEYS: Use structure to capture key restriction function and 
data")

I have used the version from next-20170403 for today.

-- 
Cheers,
Stephen Rothwell

Re: [PATCHv2] phy: cpcap-usb: Add CPCAP PMIC USB support

[Tony Lindgren]

Hi,

* Kishon Vijay Abraham I  [170330 04:51]:
> On Monday 27 March 2017 08:35 PM, Tony Lindgren wrote:
> > Seems this can be also done when implementing PM runtime handling.
> > If you want some of these changes done for the initial patch,
> > please let me know.
> 
> I think it's better we get extcon stuff in the initial patch so that we don't
> have to maintain some piece of code for legacy dt. Others can be added later.

Sure, I'll take a look at that hopefully this week at some
point.

Regards,

Tony

Re: [PATCHv2] phy: cpcap-usb: Add CPCAP PMIC USB support

[Tony Lindgren]

Hi,

* Kishon Vijay Abraham I  [170330 04:51]:
> On Monday 27 March 2017 08:35 PM, Tony Lindgren wrote:
> > Seems this can be also done when implementing PM runtime handling.
> > If you want some of these changes done for the initial patch,
> > please let me know.
> 
> I think it's better we get extcon stuff in the initial patch so that we don't
> have to maintain some piece of code for legacy dt. Others can be added later.

Sure, I'll take a look at that hopefully this week at some
point.

Regards,

Tony

[PATCH 1/2] platform/x86: silead_dmi - do not treat all devices as i2c_clients

[Dmitry Torokhov]

I2C bus has both i2c clients and adapter devices, so we must be careful in
notifier code and verify that we are actually dealing with an i2c client
before using it as such.

Fixes: cef9dd85acd7 ("platform/x86: add support for devices with Silead...")
Signed-off-by: Dmitry Torokhov 
---
 drivers/platform/x86/silead_dmi.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/silead_dmi.c 
b/drivers/platform/x86/silead_dmi.c
index 02e11fdbf375..7f1049951d1c 100644
--- a/drivers/platform/x86/silead_dmi.c
+++ b/drivers/platform/x86/silead_dmi.c
@@ -75,9 +75,8 @@ static const struct dmi_system_id silead_ts_dmi_table[] = {
{ },
 };
 
-static void silead_ts_dmi_add_props(struct device *dev)
+static void silead_ts_dmi_add_props(struct i2c_client *client)
 {
-   struct i2c_client *client = to_i2c_client(dev);
const struct dmi_system_id *dmi_id;
const struct silead_ts_dmi_data *ts_data;
int error;
@@ -87,11 +86,13 @@ static void silead_ts_dmi_add_props(struct device *dev)
return;
 
ts_data = dmi_id->driver_data;
-   if (has_acpi_companion(dev) &&
+   if (has_acpi_companion(>dev) &&
!strncmp(ts_data->acpi_name, client->name, I2C_NAME_SIZE)) {
-   error = device_add_properties(dev, ts_data->properties);
+   error = device_add_properties(>dev,
+ ts_data->properties);
if (error)
-   dev_err(dev, "failed to add properties: %d\n", error);
+   dev_err(>dev,
+   "failed to add properties: %d\n", error);
}
 }
 
@@ -99,10 +100,13 @@ static int silead_ts_dmi_notifier_call(struct 
notifier_block *nb,
   unsigned long action, void *data)
 {
struct device *dev = data;
+   struct i2c_client *client;
 
switch (action) {
case BUS_NOTIFY_ADD_DEVICE:
-   silead_ts_dmi_add_props(dev);
+   client = i2c_verify_client(dev);
+   if (client)
+   silead_ts_dmi_add_props(client);
break;
 
default:
-- 
2.12.2.715.g7642488e1d-goog

[PATCH 1/2] platform/x86: silead_dmi - do not treat all devices as i2c_clients

[Dmitry Torokhov]

I2C bus has both i2c clients and adapter devices, so we must be careful in
notifier code and verify that we are actually dealing with an i2c client
before using it as such.

Fixes: cef9dd85acd7 ("platform/x86: add support for devices with Silead...")
Signed-off-by: Dmitry Torokhov 
---
 drivers/platform/x86/silead_dmi.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/silead_dmi.c 
b/drivers/platform/x86/silead_dmi.c
index 02e11fdbf375..7f1049951d1c 100644
--- a/drivers/platform/x86/silead_dmi.c
+++ b/drivers/platform/x86/silead_dmi.c
@@ -75,9 +75,8 @@ static const struct dmi_system_id silead_ts_dmi_table[] = {
{ },
 };
 
-static void silead_ts_dmi_add_props(struct device *dev)
+static void silead_ts_dmi_add_props(struct i2c_client *client)
 {
-   struct i2c_client *client = to_i2c_client(dev);
const struct dmi_system_id *dmi_id;
const struct silead_ts_dmi_data *ts_data;
int error;
@@ -87,11 +86,13 @@ static void silead_ts_dmi_add_props(struct device *dev)
return;
 
ts_data = dmi_id->driver_data;
-   if (has_acpi_companion(dev) &&
+   if (has_acpi_companion(>dev) &&
!strncmp(ts_data->acpi_name, client->name, I2C_NAME_SIZE)) {
-   error = device_add_properties(dev, ts_data->properties);
+   error = device_add_properties(>dev,
+ ts_data->properties);
if (error)
-   dev_err(dev, "failed to add properties: %d\n", error);
+   dev_err(>dev,
+   "failed to add properties: %d\n", error);
}
 }
 
@@ -99,10 +100,13 @@ static int silead_ts_dmi_notifier_call(struct 
notifier_block *nb,
   unsigned long action, void *data)
 {
struct device *dev = data;
+   struct i2c_client *client;
 
switch (action) {
case BUS_NOTIFY_ADD_DEVICE:
-   silead_ts_dmi_add_props(dev);
+   client = i2c_verify_client(dev);
+   if (client)
+   silead_ts_dmi_add_props(client);
break;
 
default:
-- 
2.12.2.715.g7642488e1d-goog

[PATCH 2/2] platform/x86: silead_dmi - abort early if DMI does not match

[Dmitry Torokhov]

There is no point in registering I2C bus notifier if DMI data does not
match.

Signed-off-by: Dmitry Torokhov 
---
 drivers/platform/x86/silead_dmi.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/silead_dmi.c 
b/drivers/platform/x86/silead_dmi.c
index 7f1049951d1c..0eeb49e51c43 100644
--- a/drivers/platform/x86/silead_dmi.c
+++ b/drivers/platform/x86/silead_dmi.c
@@ -75,21 +75,16 @@ static const struct dmi_system_id silead_ts_dmi_table[] = {
{ },
 };
 
+static const struct silead_ts_dmi_data *silead_ts_data;
+
 static void silead_ts_dmi_add_props(struct i2c_client *client)
 {
-   const struct dmi_system_id *dmi_id;
-   const struct silead_ts_dmi_data *ts_data;
int error;
 
-   dmi_id = dmi_first_match(silead_ts_dmi_table);
-   if (!dmi_id)
-   return;
-
-   ts_data = dmi_id->driver_data;
if (has_acpi_companion(>dev) &&
-   !strncmp(ts_data->acpi_name, client->name, I2C_NAME_SIZE)) {
+   !strncmp(silead_ts_data->acpi_name, client->name, I2C_NAME_SIZE)) {
error = device_add_properties(>dev,
- ts_data->properties);
+ silead_ts_data->properties);
if (error)
dev_err(>dev,
"failed to add properties: %d\n", error);
@@ -122,8 +117,15 @@ static struct notifier_block silead_ts_dmi_notifier = {
 
 static int __init silead_ts_dmi_init(void)
 {
+   const struct dmi_system_id *dmi_id;
int error;
 
+   dmi_id = dmi_first_match(silead_ts_dmi_table);
+   if (!dmi_id)
+   return 0; /* Not an error */
+
+   silead_ts_data = dmi_id->driver_data;
+
error = bus_register_notifier(_bus_type, _ts_dmi_notifier);
if (error)
pr_err("%s: failed to register i2c bus notifier: %d\n",
-- 
2.12.2.715.g7642488e1d-goog

[PATCH 2/2] platform/x86: silead_dmi - abort early if DMI does not match

[Dmitry Torokhov]

There is no point in registering I2C bus notifier if DMI data does not
match.

Signed-off-by: Dmitry Torokhov 
---
 drivers/platform/x86/silead_dmi.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/silead_dmi.c 
b/drivers/platform/x86/silead_dmi.c
index 7f1049951d1c..0eeb49e51c43 100644
--- a/drivers/platform/x86/silead_dmi.c
+++ b/drivers/platform/x86/silead_dmi.c
@@ -75,21 +75,16 @@ static const struct dmi_system_id silead_ts_dmi_table[] = {
{ },
 };
 
+static const struct silead_ts_dmi_data *silead_ts_data;
+
 static void silead_ts_dmi_add_props(struct i2c_client *client)
 {
-   const struct dmi_system_id *dmi_id;
-   const struct silead_ts_dmi_data *ts_data;
int error;
 
-   dmi_id = dmi_first_match(silead_ts_dmi_table);
-   if (!dmi_id)
-   return;
-
-   ts_data = dmi_id->driver_data;
if (has_acpi_companion(>dev) &&
-   !strncmp(ts_data->acpi_name, client->name, I2C_NAME_SIZE)) {
+   !strncmp(silead_ts_data->acpi_name, client->name, I2C_NAME_SIZE)) {
error = device_add_properties(>dev,
- ts_data->properties);
+ silead_ts_data->properties);
if (error)
dev_err(>dev,
"failed to add properties: %d\n", error);
@@ -122,8 +117,15 @@ static struct notifier_block silead_ts_dmi_notifier = {
 
 static int __init silead_ts_dmi_init(void)
 {
+   const struct dmi_system_id *dmi_id;
int error;
 
+   dmi_id = dmi_first_match(silead_ts_dmi_table);
+   if (!dmi_id)
+   return 0; /* Not an error */
+
+   silead_ts_data = dmi_id->driver_data;
+
error = bus_register_notifier(_bus_type, _ts_dmi_notifier);
if (error)
pr_err("%s: failed to register i2c bus notifier: %d\n",
-- 
2.12.2.715.g7642488e1d-goog

Re: [PATCH v5 4/6] watchdog: iTCO_wdt: cleanup set/unset no_reboot calls

[Guenter Roeck]

On 04/03/2017 05:24 PM, Kuppuswamy Sathyanarayanan wrote:

Both iTCO_wdt_unset_NO_REBOOT_bit() and iTCO_wdt_unset_NO_REBOOT_bit()


I think you mean s/set/unset/ once.


functions has lot of common code between them. So merging these two
functions would remove these unnecessary code duplications. This patch
fixes this issue by creating single update function to handle both
set/unset functionalities.

Signed-off-by: Kuppuswamy Sathyanarayanan 

---
 drivers/watchdog/iTCO_wdt.c | 53 -
 1 file changed, 19 insertions(+), 34 deletions(-)

diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index 521ae95..cddfa00 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -172,50 +172,35 @@ static inline u32 no_reboot_bit(struct iTCO_wdt_private 
*p)
return enable_bit;
 }

-static void iTCO_wdt_set_NO_REBOOT_bit(struct iTCO_wdt_private *p)
+static int iTCO_wdt_update_no_reboot_flag(struct iTCO_wdt_private *p,


Why not stick with bit ?


+ bool status)


I think 'set' would be better than 'status'.


 {
-   u32 val32;
+   u32 val32 = 0, newval32 = 0;

-   /* Set the NO_REBOOT bit: this disables reboots */
if (p->iTCO_version >= 2) {
if (p->update_noreboot_flag)
-   p->update_noreboot_flag(true);
+   return p->update_noreboot_flag(status);
else {
val32 = readl(p->gcs_pmc);
-   val32 |= no_reboot_bit(p);
-   writel(val32, p->gcs_pmc);
-   }
-   } else if (p->iTCO_version == 1) {
-   pci_read_config_dword(p->pci_dev, 0xd4, );
-   val32 |= no_reboot_bit(p);
-   pci_write_config_dword(p->pci_dev, 0xd4, val32);
-   }
-}
-
-static int iTCO_wdt_unset_NO_REBOOT_bit(struct iTCO_wdt_private *p)
-{
-   u32 enable_bit = no_reboot_bit(p);
-   u32 val32 = 0;
+   if (status)
+   val32 |= no_reboot_bit(p);
+   else
+   val32 &= ~no_reboot_bit(p);

-   /* Unset the NO_REBOOT bit: this enables reboots */
-   if (p->iTCO_version >= 2) {
-   if (p->update_noreboot_flag)
-   return p->update_noreboot_flag(false);
-   else {
-   val32 = readl(p->gcs_pmc);
-   val32 &= ~enable_bit;
writel(val32, p->gcs_pmc);
-   val32 = readl(p->gcs_pmc);
+   newval32 = readl(p->gcs_pmc);
}
} else if (p->iTCO_version == 1) {
pci_read_config_dword(p->pci_dev, 0xd4, );
-   val32 &= ~enable_bit;
+   if (status)
+   val32 |= no_reboot_bit(p);
+   else
+   val32 &= ~no_reboot_bit(p);
pci_write_config_dword(p->pci_dev, 0xd4, val32);
-
-   pci_read_config_dword(p->pci_dev, 0xd4, );
+   pci_read_config_dword(p->pci_dev, 0xd4, );
}

-   if (val32 & enable_bit)
+   if (val32 != newval32)
return -EIO;

return 0;
@@ -231,7 +216,7 @@ static int iTCO_wdt_start(struct watchdog_device *wd_dev)
iTCO_vendor_pre_start(p->smi_res, wd_dev->timeout);

/* disable chipset's NO_REBOOT bit */
-   if (iTCO_wdt_unset_NO_REBOOT_bit(p)) {
+   if (iTCO_wdt_update_no_reboot_flag(p, false)) {
spin_unlock(>io_lock);
pr_err("failed to reset NO_REBOOT flag, reboot disabled by 
hardware/BIOS\n");
return -EIO;
@@ -272,7 +257,7 @@ static int iTCO_wdt_stop(struct watchdog_device *wd_dev)
val = inw(TCO1_CNT(p));

/* Set the NO_REBOOT bit to prevent later reboots, just for sure */
-   iTCO_wdt_set_NO_REBOOT_bit(p);
+   iTCO_wdt_update_no_reboot_flag(p, true);

spin_unlock(>io_lock);

@@ -452,14 +437,14 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
}

/* Check chipset's NO_REBOOT bit */
-   if (iTCO_wdt_unset_NO_REBOOT_bit(p) &&
+   if (iTCO_wdt_update_no_reboot_flag(p, false) &&
iTCO_vendor_check_noreboot_on()) {
pr_info("unable to reset NO_REBOOT flag, device disabled by 
hardware/BIOS\n");
return -ENODEV; /* Cannot reset NO_REBOOT bit */
}

/* Set the NO_REBOOT bit to prevent later reboots, just for sure */
-   iTCO_wdt_set_NO_REBOOT_bit(p);
+   iTCO_wdt_update_no_reboot_flag(p, true);

/* The TCO logic uses the TCO_EN bit in the SMI_EN register */
if (!devm_request_region(dev, p->smi_res->start,

Re: [PATCH v5 4/6] watchdog: iTCO_wdt: cleanup set/unset no_reboot calls

[Guenter Roeck]

On 04/03/2017 05:24 PM, Kuppuswamy Sathyanarayanan wrote:

Both iTCO_wdt_unset_NO_REBOOT_bit() and iTCO_wdt_unset_NO_REBOOT_bit()


I think you mean s/set/unset/ once.


functions has lot of common code between them. So merging these two
functions would remove these unnecessary code duplications. This patch
fixes this issue by creating single update function to handle both
set/unset functionalities.

Signed-off-by: Kuppuswamy Sathyanarayanan 

---
 drivers/watchdog/iTCO_wdt.c | 53 -
 1 file changed, 19 insertions(+), 34 deletions(-)

diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index 521ae95..cddfa00 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -172,50 +172,35 @@ static inline u32 no_reboot_bit(struct iTCO_wdt_private 
*p)
return enable_bit;
 }

-static void iTCO_wdt_set_NO_REBOOT_bit(struct iTCO_wdt_private *p)
+static int iTCO_wdt_update_no_reboot_flag(struct iTCO_wdt_private *p,


Why not stick with bit ?


+ bool status)


I think 'set' would be better than 'status'.


 {
-   u32 val32;
+   u32 val32 = 0, newval32 = 0;

-   /* Set the NO_REBOOT bit: this disables reboots */
if (p->iTCO_version >= 2) {
if (p->update_noreboot_flag)
-   p->update_noreboot_flag(true);
+   return p->update_noreboot_flag(status);
else {
val32 = readl(p->gcs_pmc);
-   val32 |= no_reboot_bit(p);
-   writel(val32, p->gcs_pmc);
-   }
-   } else if (p->iTCO_version == 1) {
-   pci_read_config_dword(p->pci_dev, 0xd4, );
-   val32 |= no_reboot_bit(p);
-   pci_write_config_dword(p->pci_dev, 0xd4, val32);
-   }
-}
-
-static int iTCO_wdt_unset_NO_REBOOT_bit(struct iTCO_wdt_private *p)
-{
-   u32 enable_bit = no_reboot_bit(p);
-   u32 val32 = 0;
+   if (status)
+   val32 |= no_reboot_bit(p);
+   else
+   val32 &= ~no_reboot_bit(p);

-   /* Unset the NO_REBOOT bit: this enables reboots */
-   if (p->iTCO_version >= 2) {
-   if (p->update_noreboot_flag)
-   return p->update_noreboot_flag(false);
-   else {
-   val32 = readl(p->gcs_pmc);
-   val32 &= ~enable_bit;
writel(val32, p->gcs_pmc);
-   val32 = readl(p->gcs_pmc);
+   newval32 = readl(p->gcs_pmc);
}
} else if (p->iTCO_version == 1) {
pci_read_config_dword(p->pci_dev, 0xd4, );
-   val32 &= ~enable_bit;
+   if (status)
+   val32 |= no_reboot_bit(p);
+   else
+   val32 &= ~no_reboot_bit(p);
pci_write_config_dword(p->pci_dev, 0xd4, val32);
-
-   pci_read_config_dword(p->pci_dev, 0xd4, );
+   pci_read_config_dword(p->pci_dev, 0xd4, );
}

-   if (val32 & enable_bit)
+   if (val32 != newval32)
return -EIO;

return 0;
@@ -231,7 +216,7 @@ static int iTCO_wdt_start(struct watchdog_device *wd_dev)
iTCO_vendor_pre_start(p->smi_res, wd_dev->timeout);

/* disable chipset's NO_REBOOT bit */
-   if (iTCO_wdt_unset_NO_REBOOT_bit(p)) {
+   if (iTCO_wdt_update_no_reboot_flag(p, false)) {
spin_unlock(>io_lock);
pr_err("failed to reset NO_REBOOT flag, reboot disabled by 
hardware/BIOS\n");
return -EIO;
@@ -272,7 +257,7 @@ static int iTCO_wdt_stop(struct watchdog_device *wd_dev)
val = inw(TCO1_CNT(p));

/* Set the NO_REBOOT bit to prevent later reboots, just for sure */
-   iTCO_wdt_set_NO_REBOOT_bit(p);
+   iTCO_wdt_update_no_reboot_flag(p, true);

spin_unlock(>io_lock);

@@ -452,14 +437,14 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
}

/* Check chipset's NO_REBOOT bit */
-   if (iTCO_wdt_unset_NO_REBOOT_bit(p) &&
+   if (iTCO_wdt_update_no_reboot_flag(p, false) &&
iTCO_vendor_check_noreboot_on()) {
pr_info("unable to reset NO_REBOOT flag, device disabled by 
hardware/BIOS\n");
return -ENODEV; /* Cannot reset NO_REBOOT bit */
}

/* Set the NO_REBOOT bit to prevent later reboots, just for sure */
-   iTCO_wdt_set_NO_REBOOT_bit(p);
+   iTCO_wdt_update_no_reboot_flag(p, true);

/* The TCO logic uses the TCO_EN bit in the SMI_EN register */
if (!devm_request_region(dev, p->smi_res->start,

Re: [PATCH 2/4] mfd: cpcap: Use handle_reread flag for interrupts

[Tony Lindgren]

* Lee Jones  [170403 07:34]:
> On Mon, 03 Apr 2017, Tony Lindgren wrote:
> 
> > Hi,
> > 
> > * Lee Jones  [170403 03:23]:
> > > On Wed, 22 Mar 2017, Tony Lindgren wrote:
> > > 
> > > > On CPCAP we need to keep reading interrupts until there are no
> > > > more interrupts. Otherwise the PMIC interrupt to the SoC will at
> > > > some point stop toggling. This seems to happen because new CPCAP
> > > > device interrupts show up while we're handling.
> > > > 
> > > > Cc: Charles Keepax 
> > > > Cc: Lee Jones 
> > > > Cc: Marcel Partap 
> > > > Cc: Michael Scott 
> > > > Tested-by: Sebastian Reichel 
> > > > Signed-off-by: Tony Lindgren 
> > > > ---
> > > >  drivers/mfd/motorola-cpcap.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > 
> > > Applied, thanks.
> > 
> > Sorry this one depends on patch 1/4 for .handle_reread. So this
> > one should be reverted or dropped. That will cause trivial merge
> > conflicts with patches 2 and 4.
> > 
> > Please let me know if you want me to resend just patches 2 and 4
> > as patches 1 and 2 need more work.
> 
> Yes, I found that out (and replied to the regmap patch).
> 
> I'll revert them all for now.

OK thanks.

> Awaiting a subsequent revision.

Sent as "[PATCHv3 0/3] CPCAP PMIC IRQ fix and related changes"
with regmap changes dropped as I figured out what the real issue
with lost interrupts was :)

Regards,

Tony

Re: [PATCH 2/4] mfd: cpcap: Use handle_reread flag for interrupts

[Tony Lindgren]

* Lee Jones  [170403 07:34]:
> On Mon, 03 Apr 2017, Tony Lindgren wrote:
> 
> > Hi,
> > 
> > * Lee Jones  [170403 03:23]:
> > > On Wed, 22 Mar 2017, Tony Lindgren wrote:
> > > 
> > > > On CPCAP we need to keep reading interrupts until there are no
> > > > more interrupts. Otherwise the PMIC interrupt to the SoC will at
> > > > some point stop toggling. This seems to happen because new CPCAP
> > > > device interrupts show up while we're handling.
> > > > 
> > > > Cc: Charles Keepax 
> > > > Cc: Lee Jones 
> > > > Cc: Marcel Partap 
> > > > Cc: Michael Scott 
> > > > Tested-by: Sebastian Reichel 
> > > > Signed-off-by: Tony Lindgren 
> > > > ---
> > > >  drivers/mfd/motorola-cpcap.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > 
> > > Applied, thanks.
> > 
> > Sorry this one depends on patch 1/4 for .handle_reread. So this
> > one should be reverted or dropped. That will cause trivial merge
> > conflicts with patches 2 and 4.
> > 
> > Please let me know if you want me to resend just patches 2 and 4
> > as patches 1 and 2 need more work.
> 
> Yes, I found that out (and replied to the regmap patch).
> 
> I'll revert them all for now.

OK thanks.

> Awaiting a subsequent revision.

Sent as "[PATCHv3 0/3] CPCAP PMIC IRQ fix and related changes"
with regmap changes dropped as I figured out what the real issue
with lost interrupts was :)

Regards,

Tony

[PATCHv3 0/3] CPCAP PMIC IRQ fix and related changes

[Tony Lindgren]

Hi all,

Here's v3 set of fixes to make CPCAP PMIC interrupts work reliably when
used with multiple drivers. While working on the ADC, charger and
USB PHY drivers I noticed that the PMIC interrupt to the SoC would
eventually stop working.

All these can wait for v4.12 merge window as these issues don't show
up currently. I'll send the related interrupt triggering dts change
separately.

Regards,

Tony

Changes since v2:

- Replaced first two regmap_irq related patches with proper interrupt
  triggering configuration after noticing the dts configuration did
  not get passed in

- Dropped regmap from the patch series subject line

Changes since v1:

- Updated regap-irq patch to use out_runtime_put in regmap_irq_thread
  also if pm_runtime_get() fails

- Clarify patch description for regmap-irq changes to make clear
  this is an issue with the CPCAP PMIC and not SoC GPIO edge/level
  handling based on comments from Charles Keepax
  

- Collected acks

Tony Lindgren (3):
  mfd: cpcap: Fix interrupt to use level interrupt
  mfd: cpcap: Use ack_invert interrupts
  mfd: cpcap: Fix bad use of IRQ sense register

 drivers/mfd/motorola-cpcap.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.12.2

[PATCHv3 0/3] CPCAP PMIC IRQ fix and related changes

[Tony Lindgren]

Hi all,

Here's v3 set of fixes to make CPCAP PMIC interrupts work reliably when
used with multiple drivers. While working on the ADC, charger and
USB PHY drivers I noticed that the PMIC interrupt to the SoC would
eventually stop working.

All these can wait for v4.12 merge window as these issues don't show
up currently. I'll send the related interrupt triggering dts change
separately.

Regards,

Tony

Changes since v2:

- Replaced first two regmap_irq related patches with proper interrupt
  triggering configuration after noticing the dts configuration did
  not get passed in

- Dropped regmap from the patch series subject line

Changes since v1:

- Updated regap-irq patch to use out_runtime_put in regmap_irq_thread
  also if pm_runtime_get() fails

- Clarify patch description for regmap-irq changes to make clear
  this is an issue with the CPCAP PMIC and not SoC GPIO edge/level
  handling based on comments from Charles Keepax
  

- Collected acks

Tony Lindgren (3):
  mfd: cpcap: Fix interrupt to use level interrupt
  mfd: cpcap: Use ack_invert interrupts
  mfd: cpcap: Fix bad use of IRQ sense register

 drivers/mfd/motorola-cpcap.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.12.2

[PATCH 2/3] mfd: cpcap: Use ack_invert interrupts

[Tony Lindgren]

We should use ack_invert as the int_read_and_clear() in the Motorola
kernel tree does "ireg_val & ~mreg_val" before writing to the mask
register.

Fixes: 56e1d40d3bea ("mfd: cpcap: Add minimal support")
Cc: Charles Keepax 
Cc: Marcel Partap 
Cc: Michael Scott 
Tested-by: Sebastian Reichel 
Signed-off-by: Tony Lindgren 
---
 drivers/mfd/motorola-cpcap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
--- a/drivers/mfd/motorola-cpcap.c
+++ b/drivers/mfd/motorola-cpcap.c
@@ -71,6 +71,7 @@ static struct regmap_irq_chip 
cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
.ack_base = CPCAP_REG_MI1,
.mask_base = CPCAP_REG_MIM1,
.use_ack = true,
+   .ack_invert = true,
},
{
.name = "cpcap-m2",
@@ -79,6 +80,7 @@ static struct regmap_irq_chip 
cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
.ack_base = CPCAP_REG_MI2,
.mask_base = CPCAP_REG_MIM2,
.use_ack = true,
+   .ack_invert = true,
},
{
.name = "cpcap1-4",
@@ -88,6 +90,7 @@ static struct regmap_irq_chip 
cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
.mask_base = CPCAP_REG_INTM1,
.type_base = CPCAP_REG_INTS1,
.use_ack = true,
+   .ack_invert = true,
},
 };
 
-- 
2.12.2

[PATCH 2/3] mfd: cpcap: Use ack_invert interrupts

[Tony Lindgren]

We should use ack_invert as the int_read_and_clear() in the Motorola
kernel tree does "ireg_val & ~mreg_val" before writing to the mask
register.

Fixes: 56e1d40d3bea ("mfd: cpcap: Add minimal support")
Cc: Charles Keepax 
Cc: Marcel Partap 
Cc: Michael Scott 
Tested-by: Sebastian Reichel 
Signed-off-by: Tony Lindgren 
---
 drivers/mfd/motorola-cpcap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
--- a/drivers/mfd/motorola-cpcap.c
+++ b/drivers/mfd/motorola-cpcap.c
@@ -71,6 +71,7 @@ static struct regmap_irq_chip 
cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
.ack_base = CPCAP_REG_MI1,
.mask_base = CPCAP_REG_MIM1,
.use_ack = true,
+   .ack_invert = true,
},
{
.name = "cpcap-m2",
@@ -79,6 +80,7 @@ static struct regmap_irq_chip 
cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
.ack_base = CPCAP_REG_MI2,
.mask_base = CPCAP_REG_MIM2,
.use_ack = true,
+   .ack_invert = true,
},
{
.name = "cpcap1-4",
@@ -88,6 +90,7 @@ static struct regmap_irq_chip 
cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
.mask_base = CPCAP_REG_INTM1,
.type_base = CPCAP_REG_INTS1,
.use_ack = true,
+   .ack_invert = true,
},
 };
 
-- 
2.12.2

[PATCH 1/3] mfd: cpcap: Fix interrupt to use level interrupt

[Tony Lindgren]

I made a mistake assuming the device tree configuration for interrupt
triggering was somehow passed to the SPI device but it's not.

In the Motorola Linux kernel tree CPCAP PMIC is configured as a rising
edge triggered interrupt, but then then it's interrupt handler keeps
looping until the GPIO line goes down. So the CPCAP interrupt is clearly
a level interrupt and not an edge interrupt.

Earlier when I tried to configure it as level interrupt using the
device tree, I did not account that the triggering only gets passed
to the SPI core and it also needs to be specified in the CPCAP driver
when we do devm_regmap_add_irq_chip().

Fixes: 56e1d40d3bea ("mfd: cpcap: Add minimal support")
Cc: Charles Keepax 
Cc: Marcel Partap 
Cc: Michael Scott 
Cc: Sebastian Reichel 
Signed-off-by: Tony Lindgren 
---
 drivers/mfd/motorola-cpcap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
--- a/drivers/mfd/motorola-cpcap.c
+++ b/drivers/mfd/motorola-cpcap.c
@@ -126,7 +126,7 @@ static int cpcap_init_irq_chip(struct cpcap_ddata *cpcap, 
int irq_chip,
 
ret = devm_regmap_add_irq_chip(>spi->dev, cpcap->regmap,
   cpcap->spi->irq,
-  IRQF_TRIGGER_RISING |
+  irq_get_trigger_type(cpcap->spi->irq) |
   IRQF_SHARED, -1,
   chip, >irqdata[irq_chip]);
if (ret) {
-- 
2.12.2

[PATCH 1/3] mfd: cpcap: Fix interrupt to use level interrupt

[Tony Lindgren]

I made a mistake assuming the device tree configuration for interrupt
triggering was somehow passed to the SPI device but it's not.

In the Motorola Linux kernel tree CPCAP PMIC is configured as a rising
edge triggered interrupt, but then then it's interrupt handler keeps
looping until the GPIO line goes down. So the CPCAP interrupt is clearly
a level interrupt and not an edge interrupt.

Earlier when I tried to configure it as level interrupt using the
device tree, I did not account that the triggering only gets passed
to the SPI core and it also needs to be specified in the CPCAP driver
when we do devm_regmap_add_irq_chip().

Fixes: 56e1d40d3bea ("mfd: cpcap: Add minimal support")
Cc: Charles Keepax 
Cc: Marcel Partap 
Cc: Michael Scott 
Cc: Sebastian Reichel 
Signed-off-by: Tony Lindgren 
---
 drivers/mfd/motorola-cpcap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
--- a/drivers/mfd/motorola-cpcap.c
+++ b/drivers/mfd/motorola-cpcap.c
@@ -126,7 +126,7 @@ static int cpcap_init_irq_chip(struct cpcap_ddata *cpcap, 
int irq_chip,
 
ret = devm_regmap_add_irq_chip(>spi->dev, cpcap->regmap,
   cpcap->spi->irq,
-  IRQF_TRIGGER_RISING |
+  irq_get_trigger_type(cpcap->spi->irq) |
   IRQF_SHARED, -1,
   chip, >irqdata[irq_chip]);
if (ret) {
-- 
2.12.2

[PATCH 3/3] mfd: cpcap: Fix bad use of IRQ sense register

[Tony Lindgren]

The cpcap INTS registers are for getting the value of the line,
not for configuring the type.

Fixes: 56e1d40d3bea ("mfd: cpcap: Add minimal support")
Cc: Charles Keepax 
Cc: Marcel Partap 
Cc: Michael Scott 
Cc: Sebastian Reichel 
Reviewed-By: Sebastian Reichel 
Tested-by: Sebastian Reichel 
Acked-by: Lee Jones 
Signed-off-by: Tony Lindgren 
---
 drivers/mfd/motorola-cpcap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
--- a/drivers/mfd/motorola-cpcap.c
+++ b/drivers/mfd/motorola-cpcap.c
@@ -88,7 +88,6 @@ static struct regmap_irq_chip 
cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
.status_base = CPCAP_REG_INT1,
.ack_base = CPCAP_REG_INT1,
.mask_base = CPCAP_REG_INTM1,
-   .type_base = CPCAP_REG_INTS1,
.use_ack = true,
.ack_invert = true,
},
-- 
2.12.2

[PATCH 3/3] mfd: cpcap: Fix bad use of IRQ sense register

[Tony Lindgren]

The cpcap INTS registers are for getting the value of the line,
not for configuring the type.

Fixes: 56e1d40d3bea ("mfd: cpcap: Add minimal support")
Cc: Charles Keepax 
Cc: Marcel Partap 
Cc: Michael Scott 
Cc: Sebastian Reichel 
Reviewed-By: Sebastian Reichel 
Tested-by: Sebastian Reichel 
Acked-by: Lee Jones 
Signed-off-by: Tony Lindgren 
---
 drivers/mfd/motorola-cpcap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
--- a/drivers/mfd/motorola-cpcap.c
+++ b/drivers/mfd/motorola-cpcap.c
@@ -88,7 +88,6 @@ static struct regmap_irq_chip 
cpcap_irq_chip[CPCAP_NR_IRQ_CHIPS] = {
.status_base = CPCAP_REG_INT1,
.ack_base = CPCAP_REG_INT1,
.mask_base = CPCAP_REG_INTM1,
-   .type_base = CPCAP_REG_INTS1,
.use_ack = true,
.ack_invert = true,
},
-- 
2.12.2

Re: [PATCH 1/2] Alps HID I2C T4 device support

[Nikolaus Rath]

Hi Ota,

> -Support Alps HID I2C T4 Touchpad device.
> -Laptop names that use this Touchpad:HP Zbook Studio, Elitebook Folio G1, 
> Elitebook 1030 G1, Elitebook 1040 G3
>
> Signed-off-by: Masaki Ota 
> ---
>  drivers/hid/hid-alps.c | 500 
> +++--
>  drivers/hid/hid-core.c |   3 +-
>  drivers/hid/hid-ids.h  |   1 +
>  3 files changed, 403 insertions(+), 101 deletions(-)
 
I tried your patch on an HP Elitebook, but with rather limited
success. Before, I was able to use the touchpad in limited fashion
(https://bugs.freedesktop.org/show_bug.cgi?id=100345). With your patch
(applied on top of 4.10), the touchpad no longer reacts at all.

That said, I didn't find a patch 2/2 anywhere.. is there something
missing?

Thanks,
-Nikolaus

-- 
GPG Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

 »Time flies like an arrow, fruit flies like a Banana.«

Re: [PATCH 1/2] Alps HID I2C T4 device support

[Nikolaus Rath]

Hi Ota,

> -Support Alps HID I2C T4 Touchpad device.
> -Laptop names that use this Touchpad:HP Zbook Studio, Elitebook Folio G1, 
> Elitebook 1030 G1, Elitebook 1040 G3
>
> Signed-off-by: Masaki Ota 
> ---
>  drivers/hid/hid-alps.c | 500 
> +++--
>  drivers/hid/hid-core.c |   3 +-
>  drivers/hid/hid-ids.h  |   1 +
>  3 files changed, 403 insertions(+), 101 deletions(-)
 
I tried your patch on an HP Elitebook, but with rather limited
success. Before, I was able to use the touchpad in limited fashion
(https://bugs.freedesktop.org/show_bug.cgi?id=100345). With your patch
(applied on top of 4.10), the touchpad no longer reacts at all.

That said, I didn't find a patch 2/2 anywhere.. is there something
missing?

Thanks,
-Nikolaus

-- 
GPG Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

 »Time flies like an arrow, fruit flies like a Banana.«

Re: dwc2_hc_chhltd_intr_dma - ChHltd set errors?

[John Stultz]

On Mon, Apr 3, 2017 at 5:54 AM, Minas Harutyunyan
 wrote:
> On 4/3/2017 9:23 AM, John Youn wrote:
>> On 03/31/2017 04:04 PM, John Stultz wrote:
>>> On Thu, Mar 2, 2017 at 12:00 PM, John Stultz  wrote:
 Hey John,
   We've noticed that when using usb ethernet adapters on HiKey, we
 occasionally see errors like:

 dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 0 - ChHltd set,
 but reason is unknown
 dwc2 f72c.usb: hcint 0x0002, intsts 0x06200029

 dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 11 - ChHltd set,
 but reason is unknown
 dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029

 Sometimes followed up by a usb error in the driver, something like:
 asix 1-1.2:1.0 eth0: asix_rx_fixup() Bad Header Length 0x36000807, offset 
 68

 Curious if you've seen any reports like this?
>
> The core version is less than 2.71a, am I right?

So it looks like its reporting 0x4f54300a for hsotg->regs + GSNPSID
which looks like DWC2_CORE_REV_3_00a

> Please send full debug log to do more investigation.

Full dmesg, or is there special debugging you want me to enable?

> Also send us regdump after connecting ethernet adapter.

Sorry, can you clarify how to generate this?

thanks
-john

Re: dwc2_hc_chhltd_intr_dma - ChHltd set errors?

[John Stultz]

On Mon, Apr 3, 2017 at 5:54 AM, Minas Harutyunyan
 wrote:
> On 4/3/2017 9:23 AM, John Youn wrote:
>> On 03/31/2017 04:04 PM, John Stultz wrote:
>>> On Thu, Mar 2, 2017 at 12:00 PM, John Stultz  wrote:
 Hey John,
   We've noticed that when using usb ethernet adapters on HiKey, we
 occasionally see errors like:

 dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 0 - ChHltd set,
 but reason is unknown
 dwc2 f72c.usb: hcint 0x0002, intsts 0x06200029

 dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 11 - ChHltd set,
 but reason is unknown
 dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029

 Sometimes followed up by a usb error in the driver, something like:
 asix 1-1.2:1.0 eth0: asix_rx_fixup() Bad Header Length 0x36000807, offset 
 68

 Curious if you've seen any reports like this?
>
> The core version is less than 2.71a, am I right?

So it looks like its reporting 0x4f54300a for hsotg->regs + GSNPSID
which looks like DWC2_CORE_REV_3_00a

> Please send full debug log to do more investigation.

Full dmesg, or is there special debugging you want me to enable?

> Also send us regdump after connecting ethernet adapter.

Sorry, can you clarify how to generate this?

thanks
-john

Re: [RFC PATCH 0/4] fs: introduce new writeback error tracking infrastructure and convert ext4 to use it

[NeilBrown]

On Mon, Apr 03 2017, Jeff Layton wrote:

> On Mon, 2017-04-03 at 12:16 -0700, Matthew Wilcox wrote:
>> On Mon, Apr 03, 2017 at 01:47:37PM -0400, Jeff Layton wrote:
>> > > I wonder whether it's even worth supporting both EIO and ENOSPC for a
>> > > writeback problem.  If I understand correctly, at the time of write(),
>> > > filesystems check to see if they have enough blocks to satisfy the
>> > > request, so ENOSPC only comes up in the writeback context for thinly
>> > > provisioned devices.
>> > 
>> > No, ENOSPC on writeback can certainly happen with network filesystems.
>> > NFS and CIFS have no way to reserve space. You wouldn't want to have to
>> > do an extra RPC on every buffered write. :)
>> 
>> Aaah, yes, network filesystems.  I would indeed not want to do an extra
>> RPC on every write to a hole (it's a hole vs non-hole question, rather
>> than a buffered/unbuffered question ... unless you're WAFLing and not
>> reclaiming quickly enough, I suppose).
>> 
>> So, OK, that makes sense, we should keep allowing filesystems to report
>> ENOSPC as a writeback error.  But I think much of the argument below
>> still holds, and we should continue to have a prior EIO to be reported
>> over a new ENOSPC (even if the program has already consumed the EIO).
>> 
>
> I'm fine with that (though I'd like Neil's thoughts before we decide
> anything) there.

I'd like there be a well defined time when old errors were forgotten.
It does make sense for EIO to persist even if ENOSPC or EDQUOT is
received, but not forever.
Clearing the remembered errors when put_write_access() causes
i_writecount to reach zero is one option (as suggested), but I'm not
sure I'm happy with it.

Local filesystems, or network filesystems which receive strong write
delegations, should only ever return EIO to fsync.  We should
concentrate on them first, I think.  As there is only one possible
error, the seq counter is sufficient to "clear" it once it has been
reported to fsync() (or write()?).

Other network filesystems could return a whole host of errors: ENOSPC
EDQUOT ESTALE EPERM EFBIG ...
Do we want to limit exactly which errors are allowed in generic code, or
do we just support EIO generically and expect the filesystem to sort out
the details for anything else?

One possible approach a filesystem could take is just to allow a single
async writeback error.  After that error, all subsequent write()
system calls become synchronous. As write() or fsync() is called on each
file descriptor (which could possibly have sent the write which caused
the error), an error is returned and that fact is counted.  Once we have
returned as many errors as there are open file descriptors
(i_writecount?), and have seen a successful write, the filesystem
forgets all recorded errors and switches back to async writes (for that
inode).   NFS does this switch-to-sync-on-error.  See nfs_need_check_write().

The "which could possibly have sent the write which caused the error" is
an explicit reference to NFS.  NFS doesn't use the AS_EIO/AS_ENOSPC
flags to return async errors.  It allocates an nfs_open_context for each
user who opens a given inode, and stores an error in there.  Each dirty
pages is associated with one of these, so errors a sure to go to the
correct user, though not necessarily the correct fd at present.

When we specify the new behaviour we should be careful to be as vague as
possible while still saying what we need.  This allows filesystems some
flexibility.

  If an error happens during writeback, the next write() or fsync() (or
  ) on the file descriptor to which data was written will return -1
  with errno set to EIO or some other relevant error.  Other file
  descriptors open on the same file may receive EIO or some other error
  on a subsequent appropriate system call.
  It should not be assumed that close() will return an error.  fsync()
  must be called before close() if writeback errors are important to the
  application.


Thanks,
NeilBrown


signature.asc
Description: PGP signature

Re: [RFC PATCH 0/4] fs: introduce new writeback error tracking infrastructure and convert ext4 to use it

[NeilBrown]

On Mon, Apr 03 2017, Jeff Layton wrote:

> On Mon, 2017-04-03 at 12:16 -0700, Matthew Wilcox wrote:
>> On Mon, Apr 03, 2017 at 01:47:37PM -0400, Jeff Layton wrote:
>> > > I wonder whether it's even worth supporting both EIO and ENOSPC for a
>> > > writeback problem.  If I understand correctly, at the time of write(),
>> > > filesystems check to see if they have enough blocks to satisfy the
>> > > request, so ENOSPC only comes up in the writeback context for thinly
>> > > provisioned devices.
>> > 
>> > No, ENOSPC on writeback can certainly happen with network filesystems.
>> > NFS and CIFS have no way to reserve space. You wouldn't want to have to
>> > do an extra RPC on every buffered write. :)
>> 
>> Aaah, yes, network filesystems.  I would indeed not want to do an extra
>> RPC on every write to a hole (it's a hole vs non-hole question, rather
>> than a buffered/unbuffered question ... unless you're WAFLing and not
>> reclaiming quickly enough, I suppose).
>> 
>> So, OK, that makes sense, we should keep allowing filesystems to report
>> ENOSPC as a writeback error.  But I think much of the argument below
>> still holds, and we should continue to have a prior EIO to be reported
>> over a new ENOSPC (even if the program has already consumed the EIO).
>> 
>
> I'm fine with that (though I'd like Neil's thoughts before we decide
> anything) there.

I'd like there be a well defined time when old errors were forgotten.
It does make sense for EIO to persist even if ENOSPC or EDQUOT is
received, but not forever.
Clearing the remembered errors when put_write_access() causes
i_writecount to reach zero is one option (as suggested), but I'm not
sure I'm happy with it.

Local filesystems, or network filesystems which receive strong write
delegations, should only ever return EIO to fsync.  We should
concentrate on them first, I think.  As there is only one possible
error, the seq counter is sufficient to "clear" it once it has been
reported to fsync() (or write()?).

Other network filesystems could return a whole host of errors: ENOSPC
EDQUOT ESTALE EPERM EFBIG ...
Do we want to limit exactly which errors are allowed in generic code, or
do we just support EIO generically and expect the filesystem to sort out
the details for anything else?

One possible approach a filesystem could take is just to allow a single
async writeback error.  After that error, all subsequent write()
system calls become synchronous. As write() or fsync() is called on each
file descriptor (which could possibly have sent the write which caused
the error), an error is returned and that fact is counted.  Once we have
returned as many errors as there are open file descriptors
(i_writecount?), and have seen a successful write, the filesystem
forgets all recorded errors and switches back to async writes (for that
inode).   NFS does this switch-to-sync-on-error.  See nfs_need_check_write().

The "which could possibly have sent the write which caused the error" is
an explicit reference to NFS.  NFS doesn't use the AS_EIO/AS_ENOSPC
flags to return async errors.  It allocates an nfs_open_context for each
user who opens a given inode, and stores an error in there.  Each dirty
pages is associated with one of these, so errors a sure to go to the
correct user, though not necessarily the correct fd at present.

When we specify the new behaviour we should be careful to be as vague as
possible while still saying what we need.  This allows filesystems some
flexibility.

  If an error happens during writeback, the next write() or fsync() (or
  ) on the file descriptor to which data was written will return -1
  with errno set to EIO or some other relevant error.  Other file
  descriptors open on the same file may receive EIO or some other error
  on a subsequent appropriate system call.
  It should not be assumed that close() will return an error.  fsync()
  must be called before close() if writeback errors are important to the
  application.


Thanks,
NeilBrown


signature.asc
Description: PGP signature

Re: [PATCH 1/4] regmap: irq: Fix lost interrupts by introducing handle_reread

[Tony Lindgren]

Hi,

* Tony Lindgren  [170328 10:13]:
> * Mark Brown  [170328 09:51]:
> > On Tue, Mar 28, 2017 at 08:47:41AM -0700, Tony Lindgren wrote:
> > > * Mark Brown  [170328 08:21]:
> > 
> > > > Right, my thinking here is that by pushing into genirq we minimise the
> > > > need even further since it'll also be available to drivers not using
> > > > regmap-irq.
> > 
> > > > > like, handle until we get IRQ_NONE? :)
> > 
> > > > Well, that's what the per driver emulation does so...  yeah.  Probably
> > > > with an upper limit on the number of times we do that.
> > 
> > > OK let's first see how that would work. I'll send a patch
> > > for that.
> > 
> > Thanks.  Can you keep me on the CC please?  It's something I keep
> > thinking about looking at myself.
> 
> Sure will do, you'll get some shared flames on it :)

So I found the real problem after thinking what you guys commented
on the "level device connected to an edge only GPIO". I added some
printks to verify that's not the case just to find out that the dts
GPIO interrupt edge configuration got only passed to the SPI driver :)

So the level IRQ changes I did earlier to the dts file did nothing.

The fix is simply to call devm_regmap_add_irq_chip() with
irq_get_trigger_type(cpcap->spi->irq) to get the configured
triggering passed properly from the dts.

So I'll drop the genirq/regmap_irq related hacks and resend just
the minimal MFD fixes. Similar misconfiguration may be the root
cause for other drivers too..

Regards,

Tony

Re: [PATCH 1/4] regmap: irq: Fix lost interrupts by introducing handle_reread

[Tony Lindgren]

Hi,

* Tony Lindgren  [170328 10:13]:
> * Mark Brown  [170328 09:51]:
> > On Tue, Mar 28, 2017 at 08:47:41AM -0700, Tony Lindgren wrote:
> > > * Mark Brown  [170328 08:21]:
> > 
> > > > Right, my thinking here is that by pushing into genirq we minimise the
> > > > need even further since it'll also be available to drivers not using
> > > > regmap-irq.
> > 
> > > > > like, handle until we get IRQ_NONE? :)
> > 
> > > > Well, that's what the per driver emulation does so...  yeah.  Probably
> > > > with an upper limit on the number of times we do that.
> > 
> > > OK let's first see how that would work. I'll send a patch
> > > for that.
> > 
> > Thanks.  Can you keep me on the CC please?  It's something I keep
> > thinking about looking at myself.
> 
> Sure will do, you'll get some shared flames on it :)

So I found the real problem after thinking what you guys commented
on the "level device connected to an edge only GPIO". I added some
printks to verify that's not the case just to find out that the dts
GPIO interrupt edge configuration got only passed to the SPI driver :)

So the level IRQ changes I did earlier to the dts file did nothing.

The fix is simply to call devm_regmap_add_irq_chip() with
irq_get_trigger_type(cpcap->spi->irq) to get the configured
triggering passed properly from the dts.

So I'll drop the genirq/regmap_irq related hacks and resend just
the minimal MFD fixes. Similar misconfiguration may be the root
cause for other drivers too..

Regards,

Tony

Re: [RFC][PATCH] spin loop arch primitives for busy waiting

[Nicholas Piggin]

On Mon, 3 Apr 2017 17:43:05 -0700
Linus Torvalds  wrote:

> On Mon, Apr 3, 2017 at 4:50 PM, Nicholas Piggin  wrote:

> > If you have any ideas, I'd be open to them.  
> 
> So the idea would be that maybe we can just make those things
> explicit. IOW, instead of having that magical looping construct that
> does other magical hidden things as part of the loop, maybe we can
> just have a
> 
>begin_cpu_relax();
>while (!cond)
>cpu_relax();
>end_cpu_relax();
> 
> and then architectures can decide how they implement it. So for x86,
> the begin/end macros would be empty. For ppc, maybe begin/end would be
> the "lower and raise priority", while cpu_relax() itself is an empty
> thing.
> 
> Or maybe "begin" just clears a counter, while "cpu_relax()" does some
> "increase iterations, and lower priority after X iterations", and then
> "end" raises the priority again.
> 
> The "do magic having a special loop" approach disturbs me. I'd much
> rather have more explicit hooks that allow people to do their own loop
> semantics (including having a "return" to exit early).

I guess so. Users will still have to read the documentation rather than
just throw it in ad hoc because it seems like a good idea.

For example powerpc must not use any other primitives that might change
SMT priority in the idle loop. For x86 if you do too much work, the rep
; nop component becomes relatively small and you lose benefit (10 cycles
latency pre-skylake).

I would suggest keeping standalone cpu_relax() for incidental code in
drivers and things (powerpc may add some extra nops between SMT low
and SMT normal priorities to improve it a little), and this would
be used by important core code and primitives.

> But that depends on architectures having some pattern that we *can*
> abstract. Would some "begin/in-loop/end" pattern like the above be
> sufficient?

Yes. begin/in/end would be sufficient for powerpc SMT priority, and
for x86, and it looks like sparc64 too. So we could do that if you
prefer.

After this, I might look at optimizing the loop code itself (optimizing
exit condition, de-pipelining, etc). That would require spin loop
primitives like this again. BUT they would not have funny restrictions
on exit conditions because they wouldn't do SMT priority. If I get
positive numbers for that, would you be opposed to having such
primitives (just for the important core spin loops)?

> I think s390 might have issues too, since they tried to have that
> "cpu_relax_yield" thing (which is only used by stop_machine), and
> they've tried cpu_relax_lowlatency() and other games.

There are some considerations with hv/guest yielding, yes. I'm not
touching that yet, but it needs to be looked at. Generic code has no
chance of looking at primitives available and deciding which should
be best used (not least because they aren't documented).

I think for the most part, busy loops shouldn't be done if they tend
to be more expensive than a context switch + associated cache costs,
so hypervisors are okay there. It's just rare cases where we *have*
to spin. Directed spinning or yielding for a resource is possibly more
general pattern and something to look at adding to these spin APIs, but
for now they should be okay to just remain within the loop body.

Summary: hypervisor guests should not be affected by the new
APIs, but we could look at augmenting them later with some hv hints.

Thanks,
Nick

Re: [RFC][PATCH] spin loop arch primitives for busy waiting

[Nicholas Piggin]

On Mon, 3 Apr 2017 17:43:05 -0700
Linus Torvalds  wrote:

> On Mon, Apr 3, 2017 at 4:50 PM, Nicholas Piggin  wrote:

> > If you have any ideas, I'd be open to them.  
> 
> So the idea would be that maybe we can just make those things
> explicit. IOW, instead of having that magical looping construct that
> does other magical hidden things as part of the loop, maybe we can
> just have a
> 
>begin_cpu_relax();
>while (!cond)
>cpu_relax();
>end_cpu_relax();
> 
> and then architectures can decide how they implement it. So for x86,
> the begin/end macros would be empty. For ppc, maybe begin/end would be
> the "lower and raise priority", while cpu_relax() itself is an empty
> thing.
> 
> Or maybe "begin" just clears a counter, while "cpu_relax()" does some
> "increase iterations, and lower priority after X iterations", and then
> "end" raises the priority again.
> 
> The "do magic having a special loop" approach disturbs me. I'd much
> rather have more explicit hooks that allow people to do their own loop
> semantics (including having a "return" to exit early).

I guess so. Users will still have to read the documentation rather than
just throw it in ad hoc because it seems like a good idea.

For example powerpc must not use any other primitives that might change
SMT priority in the idle loop. For x86 if you do too much work, the rep
; nop component becomes relatively small and you lose benefit (10 cycles
latency pre-skylake).

I would suggest keeping standalone cpu_relax() for incidental code in
drivers and things (powerpc may add some extra nops between SMT low
and SMT normal priorities to improve it a little), and this would
be used by important core code and primitives.

> But that depends on architectures having some pattern that we *can*
> abstract. Would some "begin/in-loop/end" pattern like the above be
> sufficient?

Yes. begin/in/end would be sufficient for powerpc SMT priority, and
for x86, and it looks like sparc64 too. So we could do that if you
prefer.

After this, I might look at optimizing the loop code itself (optimizing
exit condition, de-pipelining, etc). That would require spin loop
primitives like this again. BUT they would not have funny restrictions
on exit conditions because they wouldn't do SMT priority. If I get
positive numbers for that, would you be opposed to having such
primitives (just for the important core spin loops)?

> I think s390 might have issues too, since they tried to have that
> "cpu_relax_yield" thing (which is only used by stop_machine), and
> they've tried cpu_relax_lowlatency() and other games.

There are some considerations with hv/guest yielding, yes. I'm not
touching that yet, but it needs to be looked at. Generic code has no
chance of looking at primitives available and deciding which should
be best used (not least because they aren't documented).

I think for the most part, busy loops shouldn't be done if they tend
to be more expensive than a context switch + associated cache costs,
so hypervisors are okay there. It's just rare cases where we *have*
to spin. Directed spinning or yielding for a resource is possibly more
general pattern and something to look at adding to these spin APIs, but
for now they should be okay to just remain within the loop body.

Summary: hypervisor guests should not be affected by the new
APIs, but we could look at augmenting them later with some hv hints.

Thanks,
Nick

[PATCH v5 3/5] dt-bindings: Add TI SCI PM Domains

[Dave Gerlach]

Add a generic power domain implementation, TI SCI PM Domains, that
will hook into the genpd framework and allow the TI SCI protocol to
control device power states.

Also, provide macros representing each device index as understood
by TI SCI to be used in the device node power-domain references.
These are identifiers for the K2G devices managed by the PMMC.

Acked-by: Santosh Shilimkar 
Reviewed-by: Ulf Hansson 
Acked-by: Rob Herring 
Signed-off-by: Nishanth Menon 
Signed-off-by: Dave Gerlach 
---
v4->v5: Drop last sentence of first paragraph to avoid describing Linux
driver behavior in dt binding doc.

 .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 57 ++
 MAINTAINERS|  2 +
 include/dt-bindings/genpd/k2g.h| 90 ++
 3 files changed, 149 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
 create mode 100644 include/dt-bindings/genpd/k2g.h

diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt 
b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
new file mode 100644
index ..c705db07d820
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
@@ -0,0 +1,57 @@
+Texas Instruments TI-SCI Generic Power Domain
+-
+
+Some TI SoCs contain a system controller (like the PMMC, etc...) that is
+responsible for controlling the state of the IPs that are present.
+Communication between the host processor running an OS and the system
+controller happens through a protocol known as TI-SCI [1].
+
+[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
+
+PM Domain Node
+==
+The PM domain node represents the global PM domain managed by the PMMC, which
+in this case is the implementation as documented by the generic PM domain
+bindings in Documentation/devicetree/bindings/power/power_domain.txt.  Because
+this relies on the TI SCI protocol to communicate with the PMMC it must be a
+child of the pmmc node.
+
+Required Properties:
+
+- compatible: should be "ti,sci-pm-domain"
+- #power-domain-cells: Must be 1 so that an id can be provided in each
+  device node.
+
+Example (K2G):
+-
+   pmmc: pmmc {
+   compatible = "ti,k2g-sci";
+   ...
+
+   k2g_pds: power-controller {
+   compatible = "ti,sci-pm-domain";
+   #power-domain-cells = <1>;
+   };
+   };
+
+PM Domain Consumers
+===
+Hardware blocks belonging to a PM domain should contain a "power-domains"
+property that is a phandle pointing to the corresponding PM domain node
+along with an index representing the device id to be passed to the PMMC
+for device control.
+
+Required Properties:
+
+- power-domains: phandle pointing to the corresponding PM domain node
+and an ID representing the device.
+
+See dt-bindings/genpd/k2g.h for the list of valid identifiers for k2g.
+
+Example (K2G):
+
+   uart0: serial@02530c00 {
+   compatible = "ns16550a";
+   ...
+   power-domains = <_pds K2G_DEV_UART0>;
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index 1b0a87ab..ae43f3e95b47 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12382,6 +12382,8 @@ S:  Maintained
 F: Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
 F: drivers/firmware/ti_sci*
 F: include/linux/soc/ti/ti_sci_protocol.h
+F: Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
+F: include/dt-bindings/genpd/k2g.h
 
 THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
 M: Hans Verkuil 
diff --git a/include/dt-bindings/genpd/k2g.h b/include/dt-bindings/genpd/k2g.h
new file mode 100644
index ..1f31f17e19eb
--- /dev/null
+++ b/include/dt-bindings/genpd/k2g.h
@@ -0,0 +1,90 @@
+/*
+ * TI K2G SoC Device definitions
+ *
+ * Copyright (C) 2015-2017 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _DT_BINDINGS_GENPD_K2G_H
+#define _DT_BINDINGS_GENPD_K2G_H
+
+/* Documented in http://processors.wiki.ti.com/index.php/TISCI */
+
+#define K2G_DEV_PMMC0  0x
+#define K2G_DEV_MLB0   0x0001

[PATCH v5 0/5] ARM: K2G: Add support for TI-SCI Generic PM Domains

[Dave Gerlach]

Hi,
This is v5 of the series to add support for TI-SCI Generic PM Domains with
all ACKs in place and ready for Santosh to merge.

Previous versions can be found here:

v4: https://www.spinics.net/lists/arm-kernel/msg566778.html
v3: https://www.spinics.net/lists/kernel/msg2413975.html
v2: https://www.spinics.net/lists/kernel/msg2364612.html
v1: http://www.spinics.net/lists/arm-kernel/msg525204.html

Mostly just a resend of v4 with all ACKs and Reviewed-by tags in place with
the exception of a dropped sentence in patch 3 to avoid referencing Linux
specific behavior in the DT binding doc. Otherwise everything else is
unmodified.

Ulf and Santosh, I kept your Reviewed-by and Acked-by tags in Patch 3 because
the change was so minor and I still wanted to give you credit for taking the
time to review the patch, any issues and I'd be happy to change that.

Regards,
Dave

Dave Gerlach (5):
  PM / Domains: Add generic data pointer to genpd data struct
  PM / Domains: Do not check if simple providers have phandle cells
  dt-bindings: Add TI SCI PM Domains
  soc: ti: Add ti_sci_pm_domains driver
  ARM: keystone: Drop PM domain support for k2g

 .../devicetree/bindings/soc/ti/sci-pm-domain.txt   |  57 ++
 MAINTAINERS|   3 +
 arch/arm/mach-keystone/Kconfig |   1 +
 arch/arm/mach-keystone/pm_domain.c |   4 +-
 drivers/base/power/domain.c|   2 -
 drivers/soc/ti/Kconfig |  12 ++
 drivers/soc/ti/Makefile|   1 +
 drivers/soc/ti/ti_sci_pm_domains.c | 202 +
 include/dt-bindings/genpd/k2g.h|  90 +
 include/linux/pm_domain.h  |   1 +
 10 files changed, 370 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
 create mode 100644 drivers/soc/ti/ti_sci_pm_domains.c
 create mode 100644 include/dt-bindings/genpd/k2g.h

-- 
2.11.0