Re: [PATCH v2] rbd: Remove VLA usage

[kbuild test robot]

Hi Kyle,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180316]
[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/Kyle-Spiers/rbd-Remove-VLA-usage/20180317-131424
config: i386-randconfig-x017-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/block/rbd.c: In function '__rbd_notify_op_lock':
>> drivers/block/rbd.c:3105:2: error: expected ',' or ';' before 'void'
 void *p = buf;
 ^~~~
>> drivers/block/rbd.c:3110:23: error: 'p' undeclared (first use in this 
>> function); did you mean 'up'?
 ceph_start_encoding(, 2, 1, buf_size - CEPH_ENCODING_START_BLK_LEN);
  ^
  up
   drivers/block/rbd.c:3110:23: note: each undeclared identifier is reported 
only once for each function it appears in

vim +3105 drivers/block/rbd.c

b30a01f2a Ilya Dryomov 2014-05-22  3095  
ed95b21a4 Ilya Dryomov 2016-08-12  3096  static int __rbd_notify_op_lock(struct 
rbd_device *rbd_dev,
ed95b21a4 Ilya Dryomov 2016-08-12  3097 enum 
rbd_notify_op notify_op,
ed95b21a4 Ilya Dryomov 2016-08-12  3098 struct 
page ***preply_pages,
ed95b21a4 Ilya Dryomov 2016-08-12  3099 size_t 
*preply_len)
b30a01f2a Ilya Dryomov 2014-05-22  3100  {
922dab613 Ilya Dryomov 2016-05-26  3101 struct ceph_osd_client *osdc = 
_dev->rbd_client->client->osdc;
ed95b21a4 Ilya Dryomov 2016-08-12  3102 struct rbd_client_id cid = 
rbd_get_cid(rbd_dev);
1910cf8b5 Kyle Spiers  2018-03-15  3103 char buf[4 + 4 + 8 + 8 + 
CEPH_ENCODING_START_BLK_LEN];
1910cf8b5 Kyle Spiers  2018-03-15  3104 int buf_size = sizeof(buf)
ed95b21a4 Ilya Dryomov 2016-08-12 @3105 void *p = buf;
b30a01f2a Ilya Dryomov 2014-05-22  3106  
ed95b21a4 Ilya Dryomov 2016-08-12  3107 dout("%s rbd_dev %p notify_op 
%d\n", __func__, rbd_dev, notify_op);
b30a01f2a Ilya Dryomov 2014-05-22  3108  
ed95b21a4 Ilya Dryomov 2016-08-12  3109 /* encode *LockPayload 
NotifyMessage (op + ClientId) */
ed95b21a4 Ilya Dryomov 2016-08-12 @3110 ceph_start_encoding(, 2, 1, 
buf_size - CEPH_ENCODING_START_BLK_LEN);
ed95b21a4 Ilya Dryomov 2016-08-12  3111 ceph_encode_32(, notify_op);
ed95b21a4 Ilya Dryomov 2016-08-12  3112 ceph_encode_64(, cid.gid);
ed95b21a4 Ilya Dryomov 2016-08-12  3113 ceph_encode_64(, cid.handle);
76756a51e Ilya Dryomov 2014-06-20  3114  
ed95b21a4 Ilya Dryomov 2016-08-12  3115 return ceph_osdc_notify(osdc, 
_dev->header_oid,
ed95b21a4 Ilya Dryomov 2016-08-12  3116 
_dev->header_oloc, buf, buf_size,
ed95b21a4 Ilya Dryomov 2016-08-12  3117 
RBD_NOTIFY_TIMEOUT, preply_pages, preply_len);
c525f0360 Ilya Dryomov 2016-04-28  3118  }
c525f0360 Ilya Dryomov 2016-04-28  3119  

:: The code at line 3105 was first introduced by commit
:: ed95b21a4b0a71ef89306cdeb427d53cc9cb343f rbd: support for exclusive-lock 
feature

:: TO: Ilya Dryomov <idryo...@gmail.com>
:: CC: Ilya Dryomov <idryo...@gmail.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] rbd: Remove VLA usage

[kbuild test robot]

Hi Kyle,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180316]
[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/Kyle-Spiers/rbd-Remove-VLA-usage/20180317-131424
config: i386-randconfig-x017-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/block/rbd.c: In function '__rbd_notify_op_lock':
>> drivers/block/rbd.c:3105:2: error: expected ',' or ';' before 'void'
 void *p = buf;
 ^~~~
>> drivers/block/rbd.c:3110:23: error: 'p' undeclared (first use in this 
>> function); did you mean 'up'?
 ceph_start_encoding(, 2, 1, buf_size - CEPH_ENCODING_START_BLK_LEN);
  ^
  up
   drivers/block/rbd.c:3110:23: note: each undeclared identifier is reported 
only once for each function it appears in

vim +3105 drivers/block/rbd.c

b30a01f2a Ilya Dryomov 2014-05-22  3095  
ed95b21a4 Ilya Dryomov 2016-08-12  3096  static int __rbd_notify_op_lock(struct 
rbd_device *rbd_dev,
ed95b21a4 Ilya Dryomov 2016-08-12  3097 enum 
rbd_notify_op notify_op,
ed95b21a4 Ilya Dryomov 2016-08-12  3098 struct 
page ***preply_pages,
ed95b21a4 Ilya Dryomov 2016-08-12  3099 size_t 
*preply_len)
b30a01f2a Ilya Dryomov 2014-05-22  3100  {
922dab613 Ilya Dryomov 2016-05-26  3101 struct ceph_osd_client *osdc = 
_dev->rbd_client->client->osdc;
ed95b21a4 Ilya Dryomov 2016-08-12  3102 struct rbd_client_id cid = 
rbd_get_cid(rbd_dev);
1910cf8b5 Kyle Spiers  2018-03-15  3103 char buf[4 + 4 + 8 + 8 + 
CEPH_ENCODING_START_BLK_LEN];
1910cf8b5 Kyle Spiers  2018-03-15  3104 int buf_size = sizeof(buf)
ed95b21a4 Ilya Dryomov 2016-08-12 @3105 void *p = buf;
b30a01f2a Ilya Dryomov 2014-05-22  3106  
ed95b21a4 Ilya Dryomov 2016-08-12  3107 dout("%s rbd_dev %p notify_op 
%d\n", __func__, rbd_dev, notify_op);
b30a01f2a Ilya Dryomov 2014-05-22  3108  
ed95b21a4 Ilya Dryomov 2016-08-12  3109 /* encode *LockPayload 
NotifyMessage (op + ClientId) */
ed95b21a4 Ilya Dryomov 2016-08-12 @3110 ceph_start_encoding(, 2, 1, 
buf_size - CEPH_ENCODING_START_BLK_LEN);
ed95b21a4 Ilya Dryomov 2016-08-12  3111 ceph_encode_32(, notify_op);
ed95b21a4 Ilya Dryomov 2016-08-12  3112 ceph_encode_64(, cid.gid);
ed95b21a4 Ilya Dryomov 2016-08-12  3113 ceph_encode_64(, cid.handle);
76756a51e Ilya Dryomov 2014-06-20  3114  
ed95b21a4 Ilya Dryomov 2016-08-12  3115 return ceph_osdc_notify(osdc, 
_dev->header_oid,
ed95b21a4 Ilya Dryomov 2016-08-12  3116 
_dev->header_oloc, buf, buf_size,
ed95b21a4 Ilya Dryomov 2016-08-12  3117 
RBD_NOTIFY_TIMEOUT, preply_pages, preply_len);
c525f0360 Ilya Dryomov 2016-04-28  3118  }
c525f0360 Ilya Dryomov 2016-04-28  3119  

:: The code at line 3105 was first introduced by commit
:: ed95b21a4b0a71ef89306cdeb427d53cc9cb343f rbd: support for exclusive-lock 
feature

:: TO: Ilya Dryomov 
:: CC: Ilya Dryomov 

---
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] staging: typec: rt1711h typec chip driver

[李書帆]

Hi Guenter,

2018-03-17 10:16 GMT+08:00 Guenter Roeck :
> On 03/16/2018 10:40 AM, ShuFan Lee wrote:
>>
>> From: ShuFan Lee 
>>
>> Richtek RT1711H Type-C chip driver that works with
>> Type-C Port Controller Manager to provide USB PD and
>> USB Type-C functionalities.
>> Add definition of TCPC_CC_STATUS_TOGGLING.
>>
>> Signed-off-by: ShuFan Lee 
>
>
> Does this patch require DT review for the bindings ?
I see Jun is also working on the properties of DTS for TCPCI/TCPM in
his patches.
So, I plan to upload the devicetree binding file when it is ready to
move tcpci_rt1711h out of staging.
Is it OK or should I upload a version first?
>
> Gueter
>
>
>> ---
>>   drivers/staging/typec/Kconfig |   8 +
>>   drivers/staging/typec/Makefile|   1 +
>>   drivers/staging/typec/tcpci.h |   1 +
>>   drivers/staging/typec/tcpci_rt1711h.c | 329
>> ++
>>   4 files changed, 339 insertions(+)
>>   create mode 100644 drivers/staging/typec/tcpci_rt1711h.c
>>
>>   changelogs between v1 and v2
>>   - use gpiod_* instead of gpio_*
>>
>> diff --git a/drivers/staging/typec/Kconfig b/drivers/staging/typec/Kconfig
>> index 5359f556d203..3aa981fbc8f5 100644
>> --- a/drivers/staging/typec/Kconfig
>> +++ b/drivers/staging/typec/Kconfig
>> @@ -9,6 +9,14 @@ config TYPEC_TCPCI
>> help
>>   Type-C Port Controller driver for TCPCI-compliant controller.
>>   +config TYPEC_RT1711H
>> +   tristate "Richtek RT1711H Type-C chip driver"
>> +   select TYPEC_TCPCI
>> +   help
>> + Richtek RT1711H Type-C chip driver that works with
>> + Type-C Port Controller Manager to provide USB PD and USB
>> + Type-C functionalities.
>> +
>>   endif
>> endmenu
>> diff --git a/drivers/staging/typec/Makefile
>> b/drivers/staging/typec/Makefile
>> index 53d649abcb53..7803d485e1b3 100644
>> --- a/drivers/staging/typec/Makefile
>> +++ b/drivers/staging/typec/Makefile
>> @@ -1 +1,2 @@
>>   obj-$(CONFIG_TYPEC_TCPCI) += tcpci.o
>> +obj-$(CONFIG_TYPEC_RT1711H)+= tcpci_rt1711h.o
>> diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h
>> index 34c865f0dcf6..303ebde26546 100644
>> --- a/drivers/staging/typec/tcpci.h
>> +++ b/drivers/staging/typec/tcpci.h
>> @@ -59,6 +59,7 @@
>>   #define TCPC_POWER_CTRL_VCONN_ENABLE  BIT(0)
>> #define TCPC_CC_STATUS  0x1d
>> +#define TCPC_CC_STATUS_TOGGLINGBIT(5)
>>   #define TCPC_CC_STATUS_TERM   BIT(4)
>>   #define TCPC_CC_STATUS_CC2_SHIFT  2
>>   #define TCPC_CC_STATUS_CC2_MASK   0x3
>> diff --git a/drivers/staging/typec/tcpci_rt1711h.c
>> b/drivers/staging/typec/tcpci_rt1711h.c
>> new file mode 100644
>> index ..12afac363d6d
>> --- /dev/null
>> +++ b/drivers/staging/typec/tcpci_rt1711h.c
>> @@ -0,0 +1,329 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2018, Richtek Technology Corporation
>> + *
>> + * Richtek RT1711H Type-C Chip Driver
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include "tcpci.h"
>> +
>> +#define RT1711H_RTCTRL80x9B
>> +
>> +/* Autoidle timeout = (tout * 2 + 1) * 6.4ms */
>> +#define RT1711H_RTCTRL8_SET(ck300, ship_off, auto_idle, tout) \
>> +   (((ck300) << 7) | ((ship_off) << 5) | \
>> +   ((auto_idle) << 3) | ((tout) & 0x07))
>> +
>> +#define RT1711H_RTCTRL11   0x9E
>> +
>> +/* I2C timeout = (tout + 1) * 12.5ms */
>> +#define RT1711H_RTCTRL11_SET(en, tout) \
>> +(((en) << 7) | ((tout) & 0x0F))
>> +
>> +#define RT1711H_RTCTRL13   0xA0
>> +#define RT1711H_RTCTRL14   0xA1
>> +#define RT1711H_RTCTRL15   0xA2
>> +#define RT1711H_RTCTRL16   0xA3
>> +
>> +struct rt1711h_chip {
>> +   struct tcpci_data data;
>> +   struct tcpci *tcpci;
>> +   struct device *dev;
>> +   int irq;
>> +};
>> +
>> +static int rt1711h_read16(struct rt1711h_chip *chip, unsigned int reg,
>> u16 *val)
>> +{
>> +   return regmap_raw_read(chip->data.regmap, reg, val, sizeof(u16));
>> +}
>> +
>> +static int rt1711h_write16(struct rt1711h_chip *chip, unsigned int reg,
>> u16 val)
>> +{
>> +   return regmap_raw_write(chip->data.regmap, reg, ,
>> sizeof(u16));
>> +}
>> +
>> +static int rt1711h_read8(struct rt1711h_chip *chip, unsigned int reg, u8
>> *val)
>> +{
>> +   return regmap_raw_read(chip->data.regmap, reg, val, sizeof(u8));
>> +}
>> +
>> +static int rt1711h_write8(struct rt1711h_chip *chip, unsigned int reg, u8
>> val)
>> +{
>> +   return regmap_raw_write(chip->data.regmap, reg, , sizeof(u8));
>> +}
>> +
>> +static const struct regmap_config rt1711h_regmap_config = {
>> +   .reg_bits = 8,
>> +   .val_bits = 8,
>> +
>> +   .max_register = 0xFF, /* 0x80 .. 0xFF are vendor defined */
>> +};
>> +
>> +static struct rt1711h_chip 

Re: [PATCH v2] staging: typec: rt1711h typec chip driver

[李書帆]

Hi Guenter,

2018-03-17 10:16 GMT+08:00 Guenter Roeck :
> On 03/16/2018 10:40 AM, ShuFan Lee wrote:
>>
>> From: ShuFan Lee 
>>
>> Richtek RT1711H Type-C chip driver that works with
>> Type-C Port Controller Manager to provide USB PD and
>> USB Type-C functionalities.
>> Add definition of TCPC_CC_STATUS_TOGGLING.
>>
>> Signed-off-by: ShuFan Lee 
>
>
> Does this patch require DT review for the bindings ?
I see Jun is also working on the properties of DTS for TCPCI/TCPM in
his patches.
So, I plan to upload the devicetree binding file when it is ready to
move tcpci_rt1711h out of staging.
Is it OK or should I upload a version first?
>
> Gueter
>
>
>> ---
>>   drivers/staging/typec/Kconfig |   8 +
>>   drivers/staging/typec/Makefile|   1 +
>>   drivers/staging/typec/tcpci.h |   1 +
>>   drivers/staging/typec/tcpci_rt1711h.c | 329
>> ++
>>   4 files changed, 339 insertions(+)
>>   create mode 100644 drivers/staging/typec/tcpci_rt1711h.c
>>
>>   changelogs between v1 and v2
>>   - use gpiod_* instead of gpio_*
>>
>> diff --git a/drivers/staging/typec/Kconfig b/drivers/staging/typec/Kconfig
>> index 5359f556d203..3aa981fbc8f5 100644
>> --- a/drivers/staging/typec/Kconfig
>> +++ b/drivers/staging/typec/Kconfig
>> @@ -9,6 +9,14 @@ config TYPEC_TCPCI
>> help
>>   Type-C Port Controller driver for TCPCI-compliant controller.
>>   +config TYPEC_RT1711H
>> +   tristate "Richtek RT1711H Type-C chip driver"
>> +   select TYPEC_TCPCI
>> +   help
>> + Richtek RT1711H Type-C chip driver that works with
>> + Type-C Port Controller Manager to provide USB PD and USB
>> + Type-C functionalities.
>> +
>>   endif
>> endmenu
>> diff --git a/drivers/staging/typec/Makefile
>> b/drivers/staging/typec/Makefile
>> index 53d649abcb53..7803d485e1b3 100644
>> --- a/drivers/staging/typec/Makefile
>> +++ b/drivers/staging/typec/Makefile
>> @@ -1 +1,2 @@
>>   obj-$(CONFIG_TYPEC_TCPCI) += tcpci.o
>> +obj-$(CONFIG_TYPEC_RT1711H)+= tcpci_rt1711h.o
>> diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h
>> index 34c865f0dcf6..303ebde26546 100644
>> --- a/drivers/staging/typec/tcpci.h
>> +++ b/drivers/staging/typec/tcpci.h
>> @@ -59,6 +59,7 @@
>>   #define TCPC_POWER_CTRL_VCONN_ENABLE  BIT(0)
>> #define TCPC_CC_STATUS  0x1d
>> +#define TCPC_CC_STATUS_TOGGLINGBIT(5)
>>   #define TCPC_CC_STATUS_TERM   BIT(4)
>>   #define TCPC_CC_STATUS_CC2_SHIFT  2
>>   #define TCPC_CC_STATUS_CC2_MASK   0x3
>> diff --git a/drivers/staging/typec/tcpci_rt1711h.c
>> b/drivers/staging/typec/tcpci_rt1711h.c
>> new file mode 100644
>> index ..12afac363d6d
>> --- /dev/null
>> +++ b/drivers/staging/typec/tcpci_rt1711h.c
>> @@ -0,0 +1,329 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2018, Richtek Technology Corporation
>> + *
>> + * Richtek RT1711H Type-C Chip Driver
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include "tcpci.h"
>> +
>> +#define RT1711H_RTCTRL80x9B
>> +
>> +/* Autoidle timeout = (tout * 2 + 1) * 6.4ms */
>> +#define RT1711H_RTCTRL8_SET(ck300, ship_off, auto_idle, tout) \
>> +   (((ck300) << 7) | ((ship_off) << 5) | \
>> +   ((auto_idle) << 3) | ((tout) & 0x07))
>> +
>> +#define RT1711H_RTCTRL11   0x9E
>> +
>> +/* I2C timeout = (tout + 1) * 12.5ms */
>> +#define RT1711H_RTCTRL11_SET(en, tout) \
>> +(((en) << 7) | ((tout) & 0x0F))
>> +
>> +#define RT1711H_RTCTRL13   0xA0
>> +#define RT1711H_RTCTRL14   0xA1
>> +#define RT1711H_RTCTRL15   0xA2
>> +#define RT1711H_RTCTRL16   0xA3
>> +
>> +struct rt1711h_chip {
>> +   struct tcpci_data data;
>> +   struct tcpci *tcpci;
>> +   struct device *dev;
>> +   int irq;
>> +};
>> +
>> +static int rt1711h_read16(struct rt1711h_chip *chip, unsigned int reg,
>> u16 *val)
>> +{
>> +   return regmap_raw_read(chip->data.regmap, reg, val, sizeof(u16));
>> +}
>> +
>> +static int rt1711h_write16(struct rt1711h_chip *chip, unsigned int reg,
>> u16 val)
>> +{
>> +   return regmap_raw_write(chip->data.regmap, reg, ,
>> sizeof(u16));
>> +}
>> +
>> +static int rt1711h_read8(struct rt1711h_chip *chip, unsigned int reg, u8
>> *val)
>> +{
>> +   return regmap_raw_read(chip->data.regmap, reg, val, sizeof(u8));
>> +}
>> +
>> +static int rt1711h_write8(struct rt1711h_chip *chip, unsigned int reg, u8
>> val)
>> +{
>> +   return regmap_raw_write(chip->data.regmap, reg, , sizeof(u8));
>> +}
>> +
>> +static const struct regmap_config rt1711h_regmap_config = {
>> +   .reg_bits = 8,
>> +   .val_bits = 8,
>> +
>> +   .max_register = 0xFF, /* 0x80 .. 0xFF are vendor defined */
>> +};
>> +
>> +static struct rt1711h_chip *tdata_to_rt1711h(struct tcpci_data *tdata)
>> +{
>> +   return 

Re: [PATCH v2 6/6] platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake lid angle

[Gwendal Grignou]

On Wed, Mar 7, 2018 at 8:28 AM Lee Jones  wrote:

> On Wed, 21 Feb 2018, Enric Balletbo i Serra wrote:

> > From: Gwendal Grignou 
> >
> > This adds a sysfs attribute (/sys/class/chromeos/cros_ec/kb_wake_angle)
> > used to set and get the keyboard wake lid angle. This attribute is
> > present only if 2 accelerometers are controlled by the EC.
> >
> > This patch also moves the cros_ec features check before the device is
> > added so the features map obtained from the EC is ready on time.

Enric,

I found an error in the patch. During the merge, the code that set
has_kb_wake_angle in cros_ec_sensors_register() has been lost. I uploaded a
change (that also remove call for a non-existent cros-ec-angle platform
driver) at https://chromium-review.googlesource.com/#/c/866106.

Gwendal.
> >
> > Signed-off-by: Gwendal Grignou 
> > Signed-off-by: Enric Balletbo i Serra 
> > ---
> > Changes since v1:
> > - Suggested by Andy Shevchenko:
> >   - Use the previous defined to_cros_ec_dev
> >   - Use one line when fits in 80 characters.
> >   - Use DEVICE_ATTR_RW variant.
> >
> >  drivers/platform/chrome/cros_ec_sysfs.c | 81
+

> >  drivers/mfd/cros_ec_dev.c   | 19 
> >  include/linux/mfd/cros_ec.h |  1 +

> Please send me a PR with this patch contained.

> Acked-by: Lee Jones 

> --
> Lee Jones
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 6/6] platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake lid angle

[Gwendal Grignou]

On Wed, Mar 7, 2018 at 8:28 AM Lee Jones  wrote:

> On Wed, 21 Feb 2018, Enric Balletbo i Serra wrote:

> > From: Gwendal Grignou 
> >
> > This adds a sysfs attribute (/sys/class/chromeos/cros_ec/kb_wake_angle)
> > used to set and get the keyboard wake lid angle. This attribute is
> > present only if 2 accelerometers are controlled by the EC.
> >
> > This patch also moves the cros_ec features check before the device is
> > added so the features map obtained from the EC is ready on time.

Enric,

I found an error in the patch. During the merge, the code that set
has_kb_wake_angle in cros_ec_sensors_register() has been lost. I uploaded a
change (that also remove call for a non-existent cros-ec-angle platform
driver) at https://chromium-review.googlesource.com/#/c/866106.

Gwendal.
> >
> > Signed-off-by: Gwendal Grignou 
> > Signed-off-by: Enric Balletbo i Serra 
> > ---
> > Changes since v1:
> > - Suggested by Andy Shevchenko:
> >   - Use the previous defined to_cros_ec_dev
> >   - Use one line when fits in 80 characters.
> >   - Use DEVICE_ATTR_RW variant.
> >
> >  drivers/platform/chrome/cros_ec_sysfs.c | 81
+

> >  drivers/mfd/cros_ec_dev.c   | 19 
> >  include/linux/mfd/cros_ec.h |  1 +

> Please send me a PR with this patch contained.

> Acked-by: Lee Jones 

> --
> Lee Jones
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

[PATCH] Input: atmel_mxt_ts: Add hook for Chromebooks with upstream coreboot

[Jean Lucas]

Chromebooks use coreboot for system initialization. coreboot has always
had the default mainboard vendor string for Google machines set to
"Google". Google engineers set this string to "GOOGLE" in the coreboot
copies of the Chromium OS tree. The atmel_mxt_ts driver in it's current
state is set to match the latter case, i.e. it will only bind to a
Chromebook's touchscreen either if the device uses the downstream
coreboot firmware (providing the matching mainboard vendor string), or
if a user running upstream coreboot has manually set the string to
"GOOGLE". This patch adds a match for coreboot's default.

Signed-off-by: Jean Lucas 
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c 
b/drivers/input/touchscreen/atmel_mxt_ts.c
index 7659bc48f1db..8c74a3e13cca 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -3038,6 +3038,14 @@ static const struct dmi_system_id mxt_dmi_table[] = {
},
.driver_data = chromebook_platform_data,
},
+   {
+   /* Chromebooks with upstream coreboot */
+   .ident = "Chromebook",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Google"),
+   },
+   .driver_data = chromebook_platform_data,
+   },
{ }
 };
 
--

2.16.2

[PATCH] Input: atmel_mxt_ts: Add hook for Chromebooks with upstream coreboot

[Jean Lucas]

Chromebooks use coreboot for system initialization. coreboot has always
had the default mainboard vendor string for Google machines set to
"Google". Google engineers set this string to "GOOGLE" in the coreboot
copies of the Chromium OS tree. The atmel_mxt_ts driver in it's current
state is set to match the latter case, i.e. it will only bind to a
Chromebook's touchscreen either if the device uses the downstream
coreboot firmware (providing the matching mainboard vendor string), or
if a user running upstream coreboot has manually set the string to
"GOOGLE". This patch adds a match for coreboot's default.

Signed-off-by: Jean Lucas 
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c 
b/drivers/input/touchscreen/atmel_mxt_ts.c
index 7659bc48f1db..8c74a3e13cca 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -3038,6 +3038,14 @@ static const struct dmi_system_id mxt_dmi_table[] = {
},
.driver_data = chromebook_platform_data,
},
+   {
+   /* Chromebooks with upstream coreboot */
+   .ident = "Chromebook",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Google"),
+   },
+   .driver_data = chromebook_platform_data,
+   },
{ }
 };
 
--

2.16.2

Re: [PATCH 03/14] mm/hmm: HMM should have a callback before MM is destroyed v2

[John Hubbard]

On 03/16/2018 08:47 PM, John Hubbard wrote:
> On 03/16/2018 07:36 PM, John Hubbard wrote:
>> On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
>>> From: Ralph Campbell 
>>>
>>
>> 
>>
>>> +static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
>>> +{
>>> +   struct hmm *hmm = mm->hmm;
>>> +   struct hmm_mirror *mirror;
>>> +   struct hmm_mirror *mirror_next;
>>> +
>>> +   down_write(>mirrors_sem);
>>> +   list_for_each_entry_safe(mirror, mirror_next, >mirrors, list) {
>>> +   list_del_init(>list);
>>> +   if (mirror->ops->release)
>>> +   mirror->ops->release(mirror);
>>> +   }
>>> +   up_write(>mirrors_sem);
>>> +}
>>> +
>>
>> OK, as for actual code review:
>>
>> This part of the locking looks good. However, I think it can race against
>> hmm_mirror_register(), because hmm_mirror_register() will just add a new 
>> mirror regardless.
>>
>> So:
>>
>> thread 1  thread 2
>> ---
>> hmm_release   hmm_mirror_register 
>> down_write(>mirrors_sem);
>> // deletes all list items
>> up_write
>>   unblocked: adds new mirror
>>   
>>

Mark Hairgrove just pointed out some more fun facts:

1. Because hmm_mirror_register() needs to be called with an mm that has a 
non-zero
refcount, you generally cannot get an hmm_release callback, so the above race 
should
not happen.

2. We looked around, and the code is missing a call to 
mmu_notifier_unregister().
That means that it is going to leak memory and not let the mm get released 
either.

Maybe having each mirror have its own mmu notifier callback is a possible way
to solve this.

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 03/14] mm/hmm: HMM should have a callback before MM is destroyed v2

[John Hubbard]

On 03/16/2018 08:47 PM, John Hubbard wrote:
> On 03/16/2018 07:36 PM, John Hubbard wrote:
>> On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
>>> From: Ralph Campbell 
>>>
>>
>> 
>>
>>> +static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
>>> +{
>>> +   struct hmm *hmm = mm->hmm;
>>> +   struct hmm_mirror *mirror;
>>> +   struct hmm_mirror *mirror_next;
>>> +
>>> +   down_write(>mirrors_sem);
>>> +   list_for_each_entry_safe(mirror, mirror_next, >mirrors, list) {
>>> +   list_del_init(>list);
>>> +   if (mirror->ops->release)
>>> +   mirror->ops->release(mirror);
>>> +   }
>>> +   up_write(>mirrors_sem);
>>> +}
>>> +
>>
>> OK, as for actual code review:
>>
>> This part of the locking looks good. However, I think it can race against
>> hmm_mirror_register(), because hmm_mirror_register() will just add a new 
>> mirror regardless.
>>
>> So:
>>
>> thread 1  thread 2
>> ---
>> hmm_release   hmm_mirror_register 
>> down_write(>mirrors_sem);
>> // deletes all list items
>> up_write
>>   unblocked: adds new mirror
>>   
>>

Mark Hairgrove just pointed out some more fun facts:

1. Because hmm_mirror_register() needs to be called with an mm that has a 
non-zero
refcount, you generally cannot get an hmm_release callback, so the above race 
should
not happen.

2. We looked around, and the code is missing a call to 
mmu_notifier_unregister().
That means that it is going to leak memory and not let the mm get released 
either.

Maybe having each mirror have its own mmu notifier callback is a possible way
to solve this.

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH] phy: core: Allow phy_pm_runtime_xxx API calls with NULL phy

[kbuild test robot]

Hi Manu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v4.16-rc4]
[also build test WARNING on next-20180316]
[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/Manu-Gautam/phy-core-Allow-phy_pm_runtime_xxx-API-calls-with-NULL-phy/20180317-114302
config: x86_64-randconfig-x002-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/phy/phy-core.c: In function 'phy_pm_runtime_allow':
>> drivers/phy/phy-core.c:215:10: warning: 'return' with a value, in function 
>> returning void
  return 0;
 ^
   drivers/phy/phy-core.c:212:6: note: declared here
void phy_pm_runtime_allow(struct phy *phy)
 ^~~~
   drivers/phy/phy-core.c: In function 'phy_pm_runtime_forbid':
   drivers/phy/phy-core.c:227:10: warning: 'return' with a value, in function 
returning void
  return 0;
 ^
   drivers/phy/phy-core.c:224:6: note: declared here
void phy_pm_runtime_forbid(struct phy *phy)
 ^

vim +/return +215 drivers/phy/phy-core.c

   211  
   212  void phy_pm_runtime_allow(struct phy *phy)
   213  {
   214  if (!phy)
 > 215  return 0;
   216  
   217  if (!pm_runtime_enabled(>dev))
   218  return;
   219  
   220  pm_runtime_allow(>dev);
   221  }
   222  EXPORT_SYMBOL_GPL(phy_pm_runtime_allow);
   223  

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


.config.gz
Description: application/gzip

Re: [PATCH] phy: core: Allow phy_pm_runtime_xxx API calls with NULL phy

[kbuild test robot]

Hi Manu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v4.16-rc4]
[also build test WARNING on next-20180316]
[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/Manu-Gautam/phy-core-Allow-phy_pm_runtime_xxx-API-calls-with-NULL-phy/20180317-114302
config: x86_64-randconfig-x002-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/phy/phy-core.c: In function 'phy_pm_runtime_allow':
>> drivers/phy/phy-core.c:215:10: warning: 'return' with a value, in function 
>> returning void
  return 0;
 ^
   drivers/phy/phy-core.c:212:6: note: declared here
void phy_pm_runtime_allow(struct phy *phy)
 ^~~~
   drivers/phy/phy-core.c: In function 'phy_pm_runtime_forbid':
   drivers/phy/phy-core.c:227:10: warning: 'return' with a value, in function 
returning void
  return 0;
 ^
   drivers/phy/phy-core.c:224:6: note: declared here
void phy_pm_runtime_forbid(struct phy *phy)
 ^

vim +/return +215 drivers/phy/phy-core.c

   211  
   212  void phy_pm_runtime_allow(struct phy *phy)
   213  {
   214  if (!phy)
 > 215  return 0;
   216  
   217  if (!pm_runtime_enabled(>dev))
   218  return;
   219  
   220  pm_runtime_allow(>dev);
   221  }
   222  EXPORT_SYMBOL_GPL(phy_pm_runtime_allow);
   223  

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


.config.gz
Description: application/gzip

Re: [PATCH 08/14] mm/hmm: cleanup special vma handling (VM_SPECIAL)

[John Hubbard]

On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
> From: Jérôme Glisse 
> 
> Special vma (one with any of the VM_SPECIAL flags) can not be access by
> device because there is no consistent model accross device drivers on
> those vma and their backing memory.
> 
> This patch directly use hmm_range struct for hmm_pfns_special() argument
> as it is always affecting the whole vma and thus the whole range.
> 
> It also make behavior consistent after this patch both hmm_vma_fault()
> and hmm_vma_get_pfns() returns -EINVAL when facing such vma. Previously
> hmm_vma_fault() returned 0 and hmm_vma_get_pfns() return -EINVAL but
> both were filling the HMM pfn array with special entry.
> 

Hi Jerome,

This seems correct. 



> @@ -486,6 +478,14 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
>   return 0;
>  }
>  
> +static void hmm_pfns_special(struct hmm_range *range)
> +{
> + unsigned long addr = range->start, i = 0;
> +
> + for (; addr < range->end; addr += PAGE_SIZE, i++)
> + range->pfns[i] = HMM_PFN_SPECIAL;
> +}

Silly nit: the above would read more naturally, like this:

unsigned long addr, i = 0;

for (addr = range->start; addr < range->end; addr += PAGE_SIZE, i++)
range->pfns[i] = HMM_PFN_SPECIAL;

Either way,

Reviewed-by: John Hubbard 

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 08/14] mm/hmm: cleanup special vma handling (VM_SPECIAL)

[John Hubbard]

On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
> From: Jérôme Glisse 
> 
> Special vma (one with any of the VM_SPECIAL flags) can not be access by
> device because there is no consistent model accross device drivers on
> those vma and their backing memory.
> 
> This patch directly use hmm_range struct for hmm_pfns_special() argument
> as it is always affecting the whole vma and thus the whole range.
> 
> It also make behavior consistent after this patch both hmm_vma_fault()
> and hmm_vma_get_pfns() returns -EINVAL when facing such vma. Previously
> hmm_vma_fault() returned 0 and hmm_vma_get_pfns() return -EINVAL but
> both were filling the HMM pfn array with special entry.
> 

Hi Jerome,

This seems correct. 



> @@ -486,6 +478,14 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
>   return 0;
>  }
>  
> +static void hmm_pfns_special(struct hmm_range *range)
> +{
> + unsigned long addr = range->start, i = 0;
> +
> + for (; addr < range->end; addr += PAGE_SIZE, i++)
> + range->pfns[i] = HMM_PFN_SPECIAL;
> +}

Silly nit: the above would read more naturally, like this:

unsigned long addr, i = 0;

for (addr = range->start; addr < range->end; addr += PAGE_SIZE, i++)
range->pfns[i] = HMM_PFN_SPECIAL;

Either way,

Reviewed-by: John Hubbard 

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH v4] EDAC, sb_edac: Remove VLA usage

[Borislav Petkov]

On Wed, Mar 14, 2018 at 01:21:32PM -0500, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wvla, remove VLA and replace it
> with a fixed-length array instead.
> 
> Fixed as part of the directive to remove all VLAs from
> the kernel: https://lkml.org/lkml/2018/3/7/621
> 
> Also, remove max_interleave as it is no longer useful.
> 
> Reviewed-by: Mauro Carvalho Chehab 
> Signed-off-by: Gustavo A. R. Silva 
> ---
> Changes in v4:
>  - Remove max_interleave.
> 
> Changes in v3:
>  - Update macro MAX_INTERLEAVE in order to avoid too long lines.
>This change is based on Mauro's feedback.
>  - Update changelog with Mauro's Reviewed-by.
> 
> Changes in v2:
>  - Use macro max_t to compute the max of all three array sizes.
>This change is based on Borislav's feedback.
> 
>  drivers/edac/sb_edac.c | 12 +---
>  1 file changed, 5 insertions(+), 7 deletions(-)

Applied, thanks.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v4] EDAC, sb_edac: Remove VLA usage

[Borislav Petkov]

On Wed, Mar 14, 2018 at 01:21:32PM -0500, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wvla, remove VLA and replace it
> with a fixed-length array instead.
> 
> Fixed as part of the directive to remove all VLAs from
> the kernel: https://lkml.org/lkml/2018/3/7/621
> 
> Also, remove max_interleave as it is no longer useful.
> 
> Reviewed-by: Mauro Carvalho Chehab 
> Signed-off-by: Gustavo A. R. Silva 
> ---
> Changes in v4:
>  - Remove max_interleave.
> 
> Changes in v3:
>  - Update macro MAX_INTERLEAVE in order to avoid too long lines.
>This change is based on Mauro's feedback.
>  - Update changelog with Mauro's Reviewed-by.
> 
> Changes in v2:
>  - Use macro max_t to compute the max of all three array sizes.
>This change is based on Borislav's feedback.
> 
>  drivers/edac/sb_edac.c | 12 +---
>  1 file changed, 5 insertions(+), 7 deletions(-)

Applied, thanks.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH v3 18/18] infiniband: cxgb4: Eliminate duplicate barriers on weakly-ordered archs

[Timur Tabi]

On Fri, Mar 16, 2018 at 11:25 PM, Sinan Kaya  wrote:
> @@ -477,15 +477,16 @@ static inline void t4_ring_sq_db(struct t4_wq *wq, u16 
> inc, union t4_wr *wqe)
>  (u64 *)wqe);
> } else {
> pr_debug("DB wq->sq.pidx = %d\n", wq->sq.pidx);
> -   writel(PIDX_T5_V(inc) | QID_V(wq->sq.bar2_qid),
> -  wq->sq.bar2_va + SGE_UDB_KDOORBELL);
> +   __raw_writel(PIDX_T5_V(inc) | QID_V(wq->sq.bar2_qid),

I think Jason might be right that drivers shouldn't be calling the
__raw_write functions.  You definitely need to run this by the PPC
developers, specifically Ben Herrenschmidt.


-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v3 18/18] infiniband: cxgb4: Eliminate duplicate barriers on weakly-ordered archs

[Timur Tabi]

On Fri, Mar 16, 2018 at 11:25 PM, Sinan Kaya  wrote:
> @@ -477,15 +477,16 @@ static inline void t4_ring_sq_db(struct t4_wq *wq, u16 
> inc, union t4_wr *wqe)
>  (u64 *)wqe);
> } else {
> pr_debug("DB wq->sq.pidx = %d\n", wq->sq.pidx);
> -   writel(PIDX_T5_V(inc) | QID_V(wq->sq.bar2_qid),
> -  wq->sq.bar2_va + SGE_UDB_KDOORBELL);
> +   __raw_writel(PIDX_T5_V(inc) | QID_V(wq->sq.bar2_qid),

I think Jason might be right that drivers shouldn't be calling the
__raw_write functions.  You definitely need to run this by the PPC
developers, specifically Ben Herrenschmidt.


-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v3 18/18] infiniband: cxgb4: Eliminate duplicate barriers on weakly-ordered archs

[Sinan Kaya]

On 3/17/2018 12:03 AM, Sinan Kaya wrote:
> On 3/16/2018 11:40 PM, Sinan Kaya wrote:
>> I'll change writel_relaxed() with __raw_writel() in the series like you 
>> suggested
>> and also look at your other comments.
> 
> I spoke too soon.
> 
> Now that I realized, code needs to follow one of the following patterns for 
> correctness
> 
> 1)
> wmb()
> writel()/writel_relaxed()
> 
> or 
> 
> 2)
> wmb()
> __raw_wrltel()
> mmiowb()
> 
> but definitely not
> 
> wmb()
> __raw_wrltel()
> 
> Since #1 == #2, I'll stick to my current implementation of writel_relaxed()
> 
> Changing writel() to writel_relaxed() or __raw_writel() isn't enough. PowerPC 
> needs mmiowb()
> for correctness. ARM's mmiowb() implementation is empty.
> 
> So, there is no one size fits all solution with the current state of affairs.
> 
> 

I think I finally got what you mean.

Code seems to have

wmb()
writel()/writeq()
wmb()

this can be safely replaced with

wmb()
__raw_writel()/__raw_writeq()
wmb()

This will work on all arches. Below is the new version. Let me know if this is 
OK.

+++ b/drivers/infiniband/hw/cxgb4/t4.h
@@ -457,7 +457,7 @@ static inline void pio_copy(u64 __iomem *dst, u64 *src)
int count = 8;

while (count) {
-   writeq(*src, dst);
+   __raw_writeq(*src, dst);
src++;
dst++;
count--;
@@ -477,15 +477,16 @@ static inline void t4_ring_sq_db(struct t4_wq *wq, u16 
inc, union t4_wr *wqe)
 (u64 *)wqe);
} else {
pr_debug("DB wq->sq.pidx = %d\n", wq->sq.pidx);
-   writel(PIDX_T5_V(inc) | QID_V(wq->sq.bar2_qid),
-  wq->sq.bar2_va + SGE_UDB_KDOORBELL);
+   __raw_writel(PIDX_T5_V(inc) | QID_V(wq->sq.bar2_qid),
+wq->sq.bar2_va + SGE_UDB_KDOORBELL);
}

/* Flush user doorbell area writes. */
wmb();
return;
}
-   writel(QID_V(wq->sq.qid) | PIDX_V(inc), wq->db);
+   __raw_writel(QID_V(wq->sq.qid) | PIDX_V(inc), wq->db);
+   mmiowmb()
 }

 static inline void t4_ring_rq_db(struct t4_wq *wq, u16 inc,
@@ -502,15 +503,16 @@ static inline void t4_ring_rq_db(struct t4_wq *wq, u16 
inc,
 (void *)wqe);
} else {
pr_debug("DB wq->rq.pidx = %d\n", wq->rq.pidx);
-   writel(PIDX_T5_V(inc) | QID_V(wq->rq.bar2_qid),
-  wq->rq.bar2_va + SGE_UDB_KDOORBELL);
+   __raw_writel(PIDX_T5_V(inc) | QID_V(wq->rq.bar2_qid),
+wq->rq.bar2_va + SGE_UDB_KDOORBELL);
}

/* Flush user doorbell area writes. */
wmb();
return;
}
-   writel(QID_V(wq->rq.qid) | PIDX_V(inc), wq->db);
+   __raw_writel(QID_V(wq->rq.qid) | PIDX_V(inc), wq->db);
+   mmiowmb();
 }


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

Re: [PATCH v3 18/18] infiniband: cxgb4: Eliminate duplicate barriers on weakly-ordered archs

[Sinan Kaya]

On 3/17/2018 12:03 AM, Sinan Kaya wrote:
> On 3/16/2018 11:40 PM, Sinan Kaya wrote:
>> I'll change writel_relaxed() with __raw_writel() in the series like you 
>> suggested
>> and also look at your other comments.
> 
> I spoke too soon.
> 
> Now that I realized, code needs to follow one of the following patterns for 
> correctness
> 
> 1)
> wmb()
> writel()/writel_relaxed()
> 
> or 
> 
> 2)
> wmb()
> __raw_wrltel()
> mmiowb()
> 
> but definitely not
> 
> wmb()
> __raw_wrltel()
> 
> Since #1 == #2, I'll stick to my current implementation of writel_relaxed()
> 
> Changing writel() to writel_relaxed() or __raw_writel() isn't enough. PowerPC 
> needs mmiowb()
> for correctness. ARM's mmiowb() implementation is empty.
> 
> So, there is no one size fits all solution with the current state of affairs.
> 
> 

I think I finally got what you mean.

Code seems to have

wmb()
writel()/writeq()
wmb()

this can be safely replaced with

wmb()
__raw_writel()/__raw_writeq()
wmb()

This will work on all arches. Below is the new version. Let me know if this is 
OK.

+++ b/drivers/infiniband/hw/cxgb4/t4.h
@@ -457,7 +457,7 @@ static inline void pio_copy(u64 __iomem *dst, u64 *src)
int count = 8;

while (count) {
-   writeq(*src, dst);
+   __raw_writeq(*src, dst);
src++;
dst++;
count--;
@@ -477,15 +477,16 @@ static inline void t4_ring_sq_db(struct t4_wq *wq, u16 
inc, union t4_wr *wqe)
 (u64 *)wqe);
} else {
pr_debug("DB wq->sq.pidx = %d\n", wq->sq.pidx);
-   writel(PIDX_T5_V(inc) | QID_V(wq->sq.bar2_qid),
-  wq->sq.bar2_va + SGE_UDB_KDOORBELL);
+   __raw_writel(PIDX_T5_V(inc) | QID_V(wq->sq.bar2_qid),
+wq->sq.bar2_va + SGE_UDB_KDOORBELL);
}

/* Flush user doorbell area writes. */
wmb();
return;
}
-   writel(QID_V(wq->sq.qid) | PIDX_V(inc), wq->db);
+   __raw_writel(QID_V(wq->sq.qid) | PIDX_V(inc), wq->db);
+   mmiowmb()
 }

 static inline void t4_ring_rq_db(struct t4_wq *wq, u16 inc,
@@ -502,15 +503,16 @@ static inline void t4_ring_rq_db(struct t4_wq *wq, u16 
inc,
 (void *)wqe);
} else {
pr_debug("DB wq->rq.pidx = %d\n", wq->rq.pidx);
-   writel(PIDX_T5_V(inc) | QID_V(wq->rq.bar2_qid),
-  wq->rq.bar2_va + SGE_UDB_KDOORBELL);
+   __raw_writel(PIDX_T5_V(inc) | QID_V(wq->rq.bar2_qid),
+wq->rq.bar2_va + SGE_UDB_KDOORBELL);
}

/* Flush user doorbell area writes. */
wmb();
return;
}
-   writel(QID_V(wq->rq.qid) | PIDX_V(inc), wq->db);
+   __raw_writel(QID_V(wq->rq.qid) | PIDX_V(inc), wq->db);
+   mmiowmb();
 }


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

Re: [PATCH v3 18/18] infiniband: cxgb4: Eliminate duplicate barriers on weakly-ordered archs

[Timur Tabi]

On 3/16/18 6:04 PM, Steve Wise wrote:

Anybody understand why the PPC implementation of writeX_relaxed() isn't
relaxed?


You probably should ask that on the linuxppc-...@lists.ozlabs.org 
mailing list.


I've always wondered why PowerPC has non-standard I/O accessors.

--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v3 18/18] infiniband: cxgb4: Eliminate duplicate barriers on weakly-ordered archs

[Timur Tabi]

On 3/16/18 6:04 PM, Steve Wise wrote:

Anybody understand why the PPC implementation of writeX_relaxed() isn't
relaxed?


You probably should ask that on the linuxppc-...@lists.ozlabs.org 
mailing list.


I've always wondered why PowerPC has non-standard I/O accessors.

--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v3 18/18] infiniband: cxgb4: Eliminate duplicate barriers on weakly-ordered archs

[Sinan Kaya]

On 3/16/2018 11:40 PM, Sinan Kaya wrote:
> I'll change writel_relaxed() with __raw_writel() in the series like you 
> suggested
> and also look at your other comments.

I spoke too soon.

Now that I realized, code needs to follow one of the following patterns for 
correctness

1)
wmb()
writel()/writel_relaxed()

or 

2)
wmb()
__raw_wrltel()
mmiowb()

but definitely not

wmb()
__raw_wrltel()

Since #1 == #2, I'll stick to my current implementation of writel_relaxed()

Changing writel() to writel_relaxed() or __raw_writel() isn't enough. PowerPC 
needs mmiowb()
for correctness. ARM's mmiowb() implementation is empty.

So, there is no one size fits all solution with the current state of affairs.


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

Re: [PATCH v3 18/18] infiniband: cxgb4: Eliminate duplicate barriers on weakly-ordered archs

[Sinan Kaya]

On 3/16/2018 11:40 PM, Sinan Kaya wrote:
> I'll change writel_relaxed() with __raw_writel() in the series like you 
> suggested
> and also look at your other comments.

I spoke too soon.

Now that I realized, code needs to follow one of the following patterns for 
correctness

1)
wmb()
writel()/writel_relaxed()

or 

2)
wmb()
__raw_wrltel()
mmiowb()

but definitely not

wmb()
__raw_wrltel()

Since #1 == #2, I'll stick to my current implementation of writel_relaxed()

Changing writel() to writel_relaxed() or __raw_writel() isn't enough. PowerPC 
needs mmiowb()
for correctness. ARM's mmiowb() implementation is empty.

So, there is no one size fits all solution with the current state of affairs.


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

Re: [PATCH v4.16-rc5 1/1] x86/vdso: VDSO should handle clock_gettime(CLOCK_MONOTONIC_RAW) without syscall

[kbuild test robot]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180316]
[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/jason-vas-dias-gmail-com/x86-vdso-VDSO-should-handle-clock_gettime-CLOCK_MONOTONIC_RAW-without-syscall/20180317-101131
config: x86_64-kexec (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   arch/x86//entry/vdso/vclock_gettime.o: In function `__vdso_clock_gettime':
   vclock_gettime.c:(.text+0xe6): undefined reference to 
`__x86_indirect_thunk_rax'
   /usr/bin/ld: arch/x86//entry/vdso/vclock_gettime.o: relocation R_X86_64_PC32 
against undefined symbol `__x86_indirect_thunk_rax' can not be used when making 
a shared object; recompile with -fPIC
   /usr/bin/ld: final link failed: Bad value
>> collect2: error: ld returned 1 exit status
--
>> objcopy: 'arch/x86//entry/vdso/vdso64.so.dbg': No such file
--
>> arch/x86//entry/vdso/vdso32.so.dbg: undefined symbols found
--
   arch/x86/entry/vdso/vclock_gettime.o: In function `__vdso_clock_gettime':
   vclock_gettime.c:(.text+0xe6): undefined reference to 
`__x86_indirect_thunk_rax'
   /usr/bin/ld: arch/x86/entry/vdso/vclock_gettime.o: relocation R_X86_64_PC32 
against undefined symbol `__x86_indirect_thunk_rax' can not be used when making 
a shared object; recompile with -fPIC
   /usr/bin/ld: final link failed: Bad value
>> collect2: error: ld returned 1 exit status

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


.config.gz
Description: application/gzip

Re: [PATCH v4.16-rc5 1/1] x86/vdso: VDSO should handle clock_gettime(CLOCK_MONOTONIC_RAW) without syscall

[kbuild test robot]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180316]
[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/jason-vas-dias-gmail-com/x86-vdso-VDSO-should-handle-clock_gettime-CLOCK_MONOTONIC_RAW-without-syscall/20180317-101131
config: x86_64-kexec (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   arch/x86//entry/vdso/vclock_gettime.o: In function `__vdso_clock_gettime':
   vclock_gettime.c:(.text+0xe6): undefined reference to 
`__x86_indirect_thunk_rax'
   /usr/bin/ld: arch/x86//entry/vdso/vclock_gettime.o: relocation R_X86_64_PC32 
against undefined symbol `__x86_indirect_thunk_rax' can not be used when making 
a shared object; recompile with -fPIC
   /usr/bin/ld: final link failed: Bad value
>> collect2: error: ld returned 1 exit status
--
>> objcopy: 'arch/x86//entry/vdso/vdso64.so.dbg': No such file
--
>> arch/x86//entry/vdso/vdso32.so.dbg: undefined symbols found
--
   arch/x86/entry/vdso/vclock_gettime.o: In function `__vdso_clock_gettime':
   vclock_gettime.c:(.text+0xe6): undefined reference to 
`__x86_indirect_thunk_rax'
   /usr/bin/ld: arch/x86/entry/vdso/vclock_gettime.o: relocation R_X86_64_PC32 
against undefined symbol `__x86_indirect_thunk_rax' can not be used when making 
a shared object; recompile with -fPIC
   /usr/bin/ld: final link failed: Bad value
>> collect2: error: ld returned 1 exit status

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


.config.gz
Description: application/gzip

Re: [PATCH 07/14] mm/hmm: use uint64_t for HMM pfn instead of defining hmm_pfn_t to ulong

[John Hubbard]

On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
> From: Jérôme Glisse 
> 

Hi Jerome,

This one looks great. A couple of trivial typo fixes are listed below.

You can add:

Reviewed-by: John Hubbard 

> All device driver we care about are using 64bits page table entry. In
> order to match this and to avoid useless define convert all HMM pfn to
> directly use uint64_t. It is a first step on the road to allow driver
> to directly use pfn value return by HMM (saving memory and CPU cycles
> use for convertion between the two).

  used for conversion
> 
> Signed-off-by: Jérôme Glisse 
> Cc: Evgeny Baskakov 
> Cc: Ralph Campbell 
> Cc: Mark Hairgrove 
> Cc: John Hubbard 
> ---
>  include/linux/hmm.h | 46 +-
>  mm/hmm.c| 26 +-
>  2 files changed, 34 insertions(+), 38 deletions(-)
> 



> @@ -104,14 +100,14 @@ typedef unsigned long hmm_pfn_t;
>  #define HMM_PFN_SHIFT 6
>  
>  /*
> - * hmm_pfn_t_to_page() - return struct page pointed to by a valid hmm_pfn_t
> - * @pfn: hmm_pfn_t to convert to struct page
> - * Returns: struct page pointer if pfn is a valid hmm_pfn_t, NULL otherwise
> + * hmm_pfn_to_page() - return struct page pointed to by a valid HMM pfn
> + * @pfn: HMM pfn value to get corresponding struct page from
> + * Returns: struct page pointer if pfn is a valid HMM pfn, NULL otherwise
>   *
> - * If the hmm_pfn_t is valid (ie valid flag set) then return the struct page
> - * matching the pfn value stored in the hmm_pfn_t. Otherwise return NULL.
> + * If the uint64_t is valid (ie valid flag set) then return the struct page

  If the HMM pfn is valid



>  
> @@ -634,8 +634,8 @@ EXPORT_SYMBOL(hmm_vma_range_done);
>   * This is similar to a regular CPU page fault except that it will not 
> trigger
>   * any memory migration if the memory being faulted is not accessible by 
> CPUs.
>   *
> - * On error, for one virtual address in the range, the function will set the
> - * hmm_pfn_t error flag for the corresponding pfn entry.
> + * On error, for one virtual address in the range, the function will mark the
> + * correspond HMM pfn entry with error flag.

  corresponding HMM pfn entry with an error flag.

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 07/14] mm/hmm: use uint64_t for HMM pfn instead of defining hmm_pfn_t to ulong

[John Hubbard]

On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
> From: Jérôme Glisse 
> 

Hi Jerome,

This one looks great. A couple of trivial typo fixes are listed below.

You can add:

Reviewed-by: John Hubbard 

> All device driver we care about are using 64bits page table entry. In
> order to match this and to avoid useless define convert all HMM pfn to
> directly use uint64_t. It is a first step on the road to allow driver
> to directly use pfn value return by HMM (saving memory and CPU cycles
> use for convertion between the two).

  used for conversion
> 
> Signed-off-by: Jérôme Glisse 
> Cc: Evgeny Baskakov 
> Cc: Ralph Campbell 
> Cc: Mark Hairgrove 
> Cc: John Hubbard 
> ---
>  include/linux/hmm.h | 46 +-
>  mm/hmm.c| 26 +-
>  2 files changed, 34 insertions(+), 38 deletions(-)
> 



> @@ -104,14 +100,14 @@ typedef unsigned long hmm_pfn_t;
>  #define HMM_PFN_SHIFT 6
>  
>  /*
> - * hmm_pfn_t_to_page() - return struct page pointed to by a valid hmm_pfn_t
> - * @pfn: hmm_pfn_t to convert to struct page
> - * Returns: struct page pointer if pfn is a valid hmm_pfn_t, NULL otherwise
> + * hmm_pfn_to_page() - return struct page pointed to by a valid HMM pfn
> + * @pfn: HMM pfn value to get corresponding struct page from
> + * Returns: struct page pointer if pfn is a valid HMM pfn, NULL otherwise
>   *
> - * If the hmm_pfn_t is valid (ie valid flag set) then return the struct page
> - * matching the pfn value stored in the hmm_pfn_t. Otherwise return NULL.
> + * If the uint64_t is valid (ie valid flag set) then return the struct page

  If the HMM pfn is valid



>  
> @@ -634,8 +634,8 @@ EXPORT_SYMBOL(hmm_vma_range_done);
>   * This is similar to a regular CPU page fault except that it will not 
> trigger
>   * any memory migration if the memory being faulted is not accessible by 
> CPUs.
>   *
> - * On error, for one virtual address in the range, the function will set the
> - * hmm_pfn_t error flag for the corresponding pfn entry.
> + * On error, for one virtual address in the range, the function will mark the
> + * correspond HMM pfn entry with error flag.

  corresponding HMM pfn entry with an error flag.

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 03/14] mm/hmm: HMM should have a callback before MM is destroyed v2

[John Hubbard]

On 03/16/2018 07:36 PM, John Hubbard wrote:
> On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
>> From: Ralph Campbell 
>>
> 
> 
> 
>> +static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
>> +{
>> +struct hmm *hmm = mm->hmm;
>> +struct hmm_mirror *mirror;
>> +struct hmm_mirror *mirror_next;
>> +
>> +down_write(>mirrors_sem);
>> +list_for_each_entry_safe(mirror, mirror_next, >mirrors, list) {
>> +list_del_init(>list);
>> +if (mirror->ops->release)
>> +mirror->ops->release(mirror);
>> +}
>> +up_write(>mirrors_sem);
>> +}
>> +
> 
> OK, as for actual code review:
> 
> This part of the locking looks good. However, I think it can race against
> hmm_mirror_register(), because hmm_mirror_register() will just add a new 
> mirror regardless.
> 
> So:
> 
> thread 1  thread 2
> ---
> hmm_release   hmm_mirror_register 
> down_write(>mirrors_sem);
> // deletes all list items
> up_write
>   unblocked: adds new mirror
>   
> 
> ...so I think we need a way to back out of any pending hmm_mirror_register()
> calls, as part of the .release steps, right? It seems hard for the device 
> driver,
> which could be inside of hmm_mirror_register(), to handle that. Especially 
> considering
> that right now, hmm_mirror_register() will return success in this case--so
> there is no indication that anything is wrong.
> 
> Maybe hmm_mirror_register() could return an error (and not add to the mirror 
> list),
> in such a situation, how's that sound?
> 

In other words, I think this would help (not tested yet beyond a quick compile,
but it's pretty simple):

diff --git a/mm/hmm.c b/mm/hmm.c
index 7ccca5478ea1..da39f8522dca 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -66,6 +66,7 @@ struct hmm {
struct list_headmirrors;
struct mmu_notifier mmu_notifier;
struct rw_semaphore mirrors_sem;
+   boolshutting_down;
 };
 
 /*
@@ -99,6 +100,7 @@ static struct hmm *hmm_register(struct mm_struct *mm)
INIT_LIST_HEAD(>ranges);
spin_lock_init(>lock);
hmm->mm = mm;
+   hmm->shutting_down = false;
 
/*
 * We should only get here if hold the mmap_sem in write mode ie on
@@ -167,6 +169,7 @@ static void hmm_release(struct mmu_notifier *mn, struct 
mm_struct *mm)
struct hmm_mirror *mirror_next;
 
down_write(>mirrors_sem);
+   hmm->shutting_down = true;
list_for_each_entry_safe(mirror, mirror_next, >mirrors, list) {
list_del_init(>list);
if (mirror->ops->release)
@@ -227,6 +230,10 @@ int hmm_mirror_register(struct hmm_mirror *mirror, struct 
mm_struct *mm)
return -ENOMEM;
 
down_write(>hmm->mirrors_sem);
+   if (mirror->hmm->shutting_down) {
+   up_write(>hmm->mirrors_sem);
+   return -ESRCH;
+   }
list_add(>list, >hmm->mirrors);
up_write(>hmm->mirrors_sem);


thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 03/14] mm/hmm: HMM should have a callback before MM is destroyed v2

[John Hubbard]

On 03/16/2018 07:36 PM, John Hubbard wrote:
> On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
>> From: Ralph Campbell 
>>
> 
> 
> 
>> +static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
>> +{
>> +struct hmm *hmm = mm->hmm;
>> +struct hmm_mirror *mirror;
>> +struct hmm_mirror *mirror_next;
>> +
>> +down_write(>mirrors_sem);
>> +list_for_each_entry_safe(mirror, mirror_next, >mirrors, list) {
>> +list_del_init(>list);
>> +if (mirror->ops->release)
>> +mirror->ops->release(mirror);
>> +}
>> +up_write(>mirrors_sem);
>> +}
>> +
> 
> OK, as for actual code review:
> 
> This part of the locking looks good. However, I think it can race against
> hmm_mirror_register(), because hmm_mirror_register() will just add a new 
> mirror regardless.
> 
> So:
> 
> thread 1  thread 2
> ---
> hmm_release   hmm_mirror_register 
> down_write(>mirrors_sem);
> // deletes all list items
> up_write
>   unblocked: adds new mirror
>   
> 
> ...so I think we need a way to back out of any pending hmm_mirror_register()
> calls, as part of the .release steps, right? It seems hard for the device 
> driver,
> which could be inside of hmm_mirror_register(), to handle that. Especially 
> considering
> that right now, hmm_mirror_register() will return success in this case--so
> there is no indication that anything is wrong.
> 
> Maybe hmm_mirror_register() could return an error (and not add to the mirror 
> list),
> in such a situation, how's that sound?
> 

In other words, I think this would help (not tested yet beyond a quick compile,
but it's pretty simple):

diff --git a/mm/hmm.c b/mm/hmm.c
index 7ccca5478ea1..da39f8522dca 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -66,6 +66,7 @@ struct hmm {
struct list_headmirrors;
struct mmu_notifier mmu_notifier;
struct rw_semaphore mirrors_sem;
+   boolshutting_down;
 };
 
 /*
@@ -99,6 +100,7 @@ static struct hmm *hmm_register(struct mm_struct *mm)
INIT_LIST_HEAD(>ranges);
spin_lock_init(>lock);
hmm->mm = mm;
+   hmm->shutting_down = false;
 
/*
 * We should only get here if hold the mmap_sem in write mode ie on
@@ -167,6 +169,7 @@ static void hmm_release(struct mmu_notifier *mn, struct 
mm_struct *mm)
struct hmm_mirror *mirror_next;
 
down_write(>mirrors_sem);
+   hmm->shutting_down = true;
list_for_each_entry_safe(mirror, mirror_next, >mirrors, list) {
list_del_init(>list);
if (mirror->ops->release)
@@ -227,6 +230,10 @@ int hmm_mirror_register(struct hmm_mirror *mirror, struct 
mm_struct *mm)
return -ENOMEM;
 
down_write(>hmm->mirrors_sem);
+   if (mirror->hmm->shutting_down) {
+   up_write(>hmm->mirrors_sem);
+   return -ESRCH;
+   }
list_add(>list, >hmm->mirrors);
up_write(>hmm->mirrors_sem);


thanks,
-- 
John Hubbard
NVIDIA

[PATCH i2c/slave-mqueue v1] i2c: slave-mqueue: add a slave backend to receive and queue messages

[Haiyue Wang]

Some protocols over I2C are designed for bi-directional transferring
messages by using I2C Master Write protocol. Like the MCTP (Management
Component Transport Protocol) and IPMB (Intelligent Platform Management
Bus), they both require that the userspace can receive messages from
I2C dirvers under slave mode.

This new slave mqueue backend is used to receive and queue messages, it
will exposes these messages to userspace by sysfs bin file.

Signed-off-by: Haiyue Wang 
---
 Documentation/i2c/slave-mqueue-backend.rst | 125 ++
 drivers/i2c/Kconfig|  13 ++
 drivers/i2c/Makefile   |   1 +
 drivers/i2c/i2c-slave-mqueue.c | 202 +
 4 files changed, 341 insertions(+)
 create mode 100644 Documentation/i2c/slave-mqueue-backend.rst
 create mode 100644 drivers/i2c/i2c-slave-mqueue.c

diff --git a/Documentation/i2c/slave-mqueue-backend.rst 
b/Documentation/i2c/slave-mqueue-backend.rst
new file mode 100644
index 000..69d6437
--- /dev/null
+++ b/Documentation/i2c/slave-mqueue-backend.rst
@@ -0,0 +1,125 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Linux I2C slave message queue backend
+=
+
+:Author: Haiyue Wang 
+
+Some protocols over I2C/SMBus are designed for bi-directional transferring
+messages by using I2C Master Write protocol. This requires that both sides
+of the communication have slave addresses.
+
+Like MCTP (Management Component Transport Protocol) and IPMB (Intelligent
+Platform Management Bus), they both require that the userspace can receive
+messages from i2c dirvers under slave mode.
+
+This I2C slave mqueue (message queue) backend is used to receive and queue
+messages from the remote i2c intelligent device; and it will add the target
+slave address (with R/W# bit is always 0) into the message at the first byte,
+so that userspace can use this byte to dispatch the messages into different
+handling modules. Also, like IPMB, the address byte is in its message format,
+it needs it to do checksum.
+
+For messages are time related, so this backend will flush the oldest message
+to queue the newest one.
+
+Link
+
+`Intelligent Platform Management Bus
+Communications Protocol Specification
+`_
+
+`Management Component Transport Protocol (MCTP)
+SMBus/I2C Transport Binding Specification
+`_
+
+How to use
+--
+For example, the I2C5 bus has slave address 0x10, bellowing command will create
+the related message queue interface:
+
+echo slave-mqueue 0x1010 > /sys/bus/i2c/devices/i2c-5/new_device
+
+Then you can dump the messages like this:
+
+hexdump -C /sys/bus/i2c/devices/5-1010/slave-mqueue
+
+Code Example
+
+*Note: call 'lseek' before 'read', this is a requirement from kernfs' design.*
+
+::
+
+  #include 
+  #include 
+  #include 
+  #include 
+  #include 
+  #include 
+  #include 
+
+  int main(int argc, char *argv[])
+  {
+  int i, r;
+  struct pollfd pfd;
+  struct timespec ts;
+  unsigned char data[256];
+
+  pfd.fd = open(argv[1], O_RDONLY | O_NONBLOCK);
+  if (pfd.fd < 0)
+  return -1;
+
+  pfd.events = POLLPRI;
+
+  while (1) {
+  r = poll(, 1, 5000);
+
+  if (r < 0)
+  break;
+
+  if (r == 0 || !(pfd.revents & POLLPRI))
+  continue;
+
+  lseek(pfd.fd, 0, SEEK_SET);
+  r = read(pfd.fd, data, sizeof(data));
+  if (r <= 0)
+  continue;
+
+  clock_gettime(CLOCK_MONOTONIC, );
+  printf("[%ld.%.9ld] :", ts.tv_sec, ts.tv_nsec);
+  for (i = 0; i < r; i++)
+  printf(" %02x", data[i]);
+  printf("\n");
+  }
+
+  close(pfd.fd);
+
+  return 0;
+  }
+
+Result
+--
+*./a.out "/sys/bus/i2c/devices/5-1010/slave-mqueue"*
+
+::
+
+  [10183.232500449] : 20 18 c8 2c 78 01 5b
+  [10183.479358348] : 20 18 c8 2c 78 01 5b
+  [10183.726556812] : 20 18 c8 2c 78 01 5b
+  [10183.972605863] : 20 18 c8 2c 78 01 5b
+  [10184.220124772] : 20 18 c8 2c 78 01 5b
+  [10184.467764166] : 20 18 c8 2c 78 01 5b
+  [10193.233421784] : 20 18 c8 2c 7c 01 57
+  [10193.480273460] : 20 18 c8 2c 7c 01 57
+  [10193.726788733] : 20 18 c8 2c 7c 01 57
+  [10193.972781945] : 20 18 c8 2c 7c 01 57
+  [10194.220487360] : 20 18 c8 2c 7c 01 57
+  [10194.468089259] : 20 18 c8 2c 7c 01 57
+  [10203.233433099] : 20 18 c8 2c 80 01 53
+  [10203.481058715] : 20 18 c8 2c 80 01 53
+  [10203.727610472] : 20 18 c8 2c 80 01 53
+  [10203.974044856] : 20 18 c8 2c 80 01 53
+  

[PATCH i2c/slave-mqueue v1] i2c: slave-mqueue: add a slave backend to receive and queue messages

[Haiyue Wang]

Some protocols over I2C are designed for bi-directional transferring
messages by using I2C Master Write protocol. Like the MCTP (Management
Component Transport Protocol) and IPMB (Intelligent Platform Management
Bus), they both require that the userspace can receive messages from
I2C dirvers under slave mode.

This new slave mqueue backend is used to receive and queue messages, it
will exposes these messages to userspace by sysfs bin file.

Signed-off-by: Haiyue Wang 
---
 Documentation/i2c/slave-mqueue-backend.rst | 125 ++
 drivers/i2c/Kconfig|  13 ++
 drivers/i2c/Makefile   |   1 +
 drivers/i2c/i2c-slave-mqueue.c | 202 +
 4 files changed, 341 insertions(+)
 create mode 100644 Documentation/i2c/slave-mqueue-backend.rst
 create mode 100644 drivers/i2c/i2c-slave-mqueue.c

diff --git a/Documentation/i2c/slave-mqueue-backend.rst 
b/Documentation/i2c/slave-mqueue-backend.rst
new file mode 100644
index 000..69d6437
--- /dev/null
+++ b/Documentation/i2c/slave-mqueue-backend.rst
@@ -0,0 +1,125 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Linux I2C slave message queue backend
+=
+
+:Author: Haiyue Wang 
+
+Some protocols over I2C/SMBus are designed for bi-directional transferring
+messages by using I2C Master Write protocol. This requires that both sides
+of the communication have slave addresses.
+
+Like MCTP (Management Component Transport Protocol) and IPMB (Intelligent
+Platform Management Bus), they both require that the userspace can receive
+messages from i2c dirvers under slave mode.
+
+This I2C slave mqueue (message queue) backend is used to receive and queue
+messages from the remote i2c intelligent device; and it will add the target
+slave address (with R/W# bit is always 0) into the message at the first byte,
+so that userspace can use this byte to dispatch the messages into different
+handling modules. Also, like IPMB, the address byte is in its message format,
+it needs it to do checksum.
+
+For messages are time related, so this backend will flush the oldest message
+to queue the newest one.
+
+Link
+
+`Intelligent Platform Management Bus
+Communications Protocol Specification
+`_
+
+`Management Component Transport Protocol (MCTP)
+SMBus/I2C Transport Binding Specification
+`_
+
+How to use
+--
+For example, the I2C5 bus has slave address 0x10, bellowing command will create
+the related message queue interface:
+
+echo slave-mqueue 0x1010 > /sys/bus/i2c/devices/i2c-5/new_device
+
+Then you can dump the messages like this:
+
+hexdump -C /sys/bus/i2c/devices/5-1010/slave-mqueue
+
+Code Example
+
+*Note: call 'lseek' before 'read', this is a requirement from kernfs' design.*
+
+::
+
+  #include 
+  #include 
+  #include 
+  #include 
+  #include 
+  #include 
+  #include 
+
+  int main(int argc, char *argv[])
+  {
+  int i, r;
+  struct pollfd pfd;
+  struct timespec ts;
+  unsigned char data[256];
+
+  pfd.fd = open(argv[1], O_RDONLY | O_NONBLOCK);
+  if (pfd.fd < 0)
+  return -1;
+
+  pfd.events = POLLPRI;
+
+  while (1) {
+  r = poll(, 1, 5000);
+
+  if (r < 0)
+  break;
+
+  if (r == 0 || !(pfd.revents & POLLPRI))
+  continue;
+
+  lseek(pfd.fd, 0, SEEK_SET);
+  r = read(pfd.fd, data, sizeof(data));
+  if (r <= 0)
+  continue;
+
+  clock_gettime(CLOCK_MONOTONIC, );
+  printf("[%ld.%.9ld] :", ts.tv_sec, ts.tv_nsec);
+  for (i = 0; i < r; i++)
+  printf(" %02x", data[i]);
+  printf("\n");
+  }
+
+  close(pfd.fd);
+
+  return 0;
+  }
+
+Result
+--
+*./a.out "/sys/bus/i2c/devices/5-1010/slave-mqueue"*
+
+::
+
+  [10183.232500449] : 20 18 c8 2c 78 01 5b
+  [10183.479358348] : 20 18 c8 2c 78 01 5b
+  [10183.726556812] : 20 18 c8 2c 78 01 5b
+  [10183.972605863] : 20 18 c8 2c 78 01 5b
+  [10184.220124772] : 20 18 c8 2c 78 01 5b
+  [10184.467764166] : 20 18 c8 2c 78 01 5b
+  [10193.233421784] : 20 18 c8 2c 7c 01 57
+  [10193.480273460] : 20 18 c8 2c 7c 01 57
+  [10193.726788733] : 20 18 c8 2c 7c 01 57
+  [10193.972781945] : 20 18 c8 2c 7c 01 57
+  [10194.220487360] : 20 18 c8 2c 7c 01 57
+  [10194.468089259] : 20 18 c8 2c 7c 01 57
+  [10203.233433099] : 20 18 c8 2c 80 01 53
+  [10203.481058715] : 20 18 c8 2c 80 01 53
+  [10203.727610472] : 20 18 c8 2c 80 01 53
+  [10203.974044856] : 20 18 c8 2c 80 01 53
+  [10204.220734634] : 20 18 c8 2c 80 01 53
+  [10204.468461664] : 

Re: [PATCH v3 18/18] infiniband: cxgb4: Eliminate duplicate barriers on weakly-ordered archs

[Sinan Kaya]

On 3/16/2018 7:05 PM, Steve Wise wrote:
>>
>> On 3/16/2018 5:05 PM, Steve Wise wrote:
 Code includes wmb() followed by writel(). writel() already has a barrier
>>> on
 some architectures like arm64.

 This ends up CPU observing two barriers back to back before executing
>> the
 register write.

 Since code already has an explicit barrier call, changing writel() to
 writel_relaxed().

 Signed-off-by: Sinan Kaya 
>>>
>>> NAK - This isn't correct for PowerPC.  For PowerPC, writeX_relaxed() is just
>>> writeX().
>>>
>>> I was just looking at this with Chelsio developers, and they said the
>>> writeX() should be replaced with __raw_writeX(), not writeX_relaxed(), to
>>> get rid of the extra barrier for all architectures.
>>
>> OK. I can do that but isn't the problem at PowerPC adaptation?
>>
>> /*
>>  * We don't do relaxed operations yet, at least not with this semantic
>>  */
>> #define readb_relaxed(addr)  readb(addr)
>> #define readw_relaxed(addr)  readw(addr)
>> #define readl_relaxed(addr)  readl(addr)
>> #define readq_relaxed(addr)  readq(addr)
>> #define writeb_relaxed(v, addr)  writeb(v, addr)
>> #define writew_relaxed(v, addr)  writew(v, addr)
>> #define writel_relaxed(v, addr)  writel(v, addr)
>> #define writeq_relaxed(v, addr)  writeq(v, addr)
>>
>> Why don't we fix the PowerPC's relaxed operators? Is that a bigger task?
> 
> I don't know the answer, but perhaps the proper fix is to correctly implement 
> these for PPC?
> 

I found this article: https://lwn.net/Articles/698014/#PowerPC%20Implementation

Apparently, this issue was discussed at a conference in 2015.

Based on how I read this article, writel() and writel_relaxed() behave exactly
the same on PowerPC because the barrier is enforced by the time code is leaving
a critical section not during MMIO execution.

I also see that __raw_writel() is being heavily used in drivers directory.

https://elixir.bootlin.com/linux/latest/ident/__raw_writel

I'll change writel_relaxed() with __raw_writel() in the series like you 
suggested
and also look at your other comments.

This seems to be the current norm.

> 
>>
>> >From API perspective both __raw_writeX() and writeX_relaxed() are
>> correct.
>> It is just PowerPC doesn't seem the follow the definition yet.
> 
> 
> 
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

Re: [PATCH v3 18/18] infiniband: cxgb4: Eliminate duplicate barriers on weakly-ordered archs

[Sinan Kaya]

On 3/16/2018 7:05 PM, Steve Wise wrote:
>>
>> On 3/16/2018 5:05 PM, Steve Wise wrote:
 Code includes wmb() followed by writel(). writel() already has a barrier
>>> on
 some architectures like arm64.

 This ends up CPU observing two barriers back to back before executing
>> the
 register write.

 Since code already has an explicit barrier call, changing writel() to
 writel_relaxed().

 Signed-off-by: Sinan Kaya 
>>>
>>> NAK - This isn't correct for PowerPC.  For PowerPC, writeX_relaxed() is just
>>> writeX().
>>>
>>> I was just looking at this with Chelsio developers, and they said the
>>> writeX() should be replaced with __raw_writeX(), not writeX_relaxed(), to
>>> get rid of the extra barrier for all architectures.
>>
>> OK. I can do that but isn't the problem at PowerPC adaptation?
>>
>> /*
>>  * We don't do relaxed operations yet, at least not with this semantic
>>  */
>> #define readb_relaxed(addr)  readb(addr)
>> #define readw_relaxed(addr)  readw(addr)
>> #define readl_relaxed(addr)  readl(addr)
>> #define readq_relaxed(addr)  readq(addr)
>> #define writeb_relaxed(v, addr)  writeb(v, addr)
>> #define writew_relaxed(v, addr)  writew(v, addr)
>> #define writel_relaxed(v, addr)  writel(v, addr)
>> #define writeq_relaxed(v, addr)  writeq(v, addr)
>>
>> Why don't we fix the PowerPC's relaxed operators? Is that a bigger task?
> 
> I don't know the answer, but perhaps the proper fix is to correctly implement 
> these for PPC?
> 

I found this article: https://lwn.net/Articles/698014/#PowerPC%20Implementation

Apparently, this issue was discussed at a conference in 2015.

Based on how I read this article, writel() and writel_relaxed() behave exactly
the same on PowerPC because the barrier is enforced by the time code is leaving
a critical section not during MMIO execution.

I also see that __raw_writel() is being heavily used in drivers directory.

https://elixir.bootlin.com/linux/latest/ident/__raw_writel

I'll change writel_relaxed() with __raw_writel() in the series like you 
suggested
and also look at your other comments.

This seems to be the current norm.

> 
>>
>> >From API perspective both __raw_writeX() and writeX_relaxed() are
>> correct.
>> It is just PowerPC doesn't seem the follow the definition yet.
> 
> 
> 
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

Re: [PATCH 06/14] mm/hmm: remove HMM_PFN_READ flag and ignore peculiar architecture

[John Hubbard]

On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
> From: Jérôme Glisse 
> 
> Only peculiar architecture allow write without read thus assume that
> any valid pfn do allow for read. Note we do not care for write only
> because it does make sense with thing like atomic compare and exchange
> or any other operations that allow you to get the memory value through
> them.
> 
> Signed-off-by: Jérôme Glisse 
> Cc: Evgeny Baskakov 
> Cc: Ralph Campbell 
> Cc: Mark Hairgrove 
> Cc: John Hubbard 
> ---
>  include/linux/hmm.h | 14 ++
>  mm/hmm.c| 28 
>  2 files changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index b65e527dd120..4bdc58ffe9f3 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -84,7 +84,6 @@ struct hmm;
>   *
>   * Flags:
>   * HMM_PFN_VALID: pfn is valid

Maybe write it like this:

* HMM_PFN_VALID: pfn is valid. This implies that it has, at least, read 
permission.

> - * HMM_PFN_READ:  CPU page table has read permission set
>   * HMM_PFN_WRITE: CPU page table has write permission set
>   * HMM_PFN_ERROR: corresponding CPU page table entry points to poisoned 
> memory
>   * HMM_PFN_EMPTY: corresponding CPU page table entry is pte_none()
> @@ -97,13 +96,12 @@ struct hmm;
>  typedef unsigned long hmm_pfn_t;
>  
>  #define HMM_PFN_VALID (1 << 0)



>  
> @@ -536,6 +534,17 @@ int hmm_vma_get_pfns(struct hmm_range *range)
>   list_add_rcu(>list, >ranges);
>   spin_unlock(>lock);
>  
> + if (!(vma->vm_flags & VM_READ)) {
> + /*
> +  * If vma do not allow read assume it does not allow write as
> +  * only peculiar architecture allow write without read and this
> +  * is not a case we care about (some operation like atomic no
> +  * longer make sense).
> +  */
> + hmm_pfns_clear(range->pfns, range->start, range->end);
> + return 0;

1. Shouldn't we return an error here? All is not well. No one has any pfns, even
   though they tried to get some. :)

2. I think this check needs to be done much earlier, right after the "Sanity
   check, this should not happen" code in this routine.

> + }
> +
>   hmm_vma_walk.fault = false;
>   hmm_vma_walk.range = range;
>   mm_walk.private = _vma_walk;
> @@ -690,6 +699,17 @@ int hmm_vma_fault(struct hmm_range *range, bool write, 
> bool block)
>   list_add_rcu(>list, >ranges);
>   spin_unlock(>lock);
>  
> + if (!(vma->vm_flags & VM_READ)) {
> + /*
> +  * If vma do not allow read assume it does not allow write as
> +  * only peculiar architecture allow write without read and this
> +  * is not a case we care about (some operation like atomic no
> +  * longer make sense).
> +  */

For the comment wording (for this one, and the one above), how about:

/*
 * If the vma does not allow read access, then assume that 
 * it does not allow write access, either.
 */

...and then leave the more extensive explanation to the commit log. Or,
if we really want a longer explananation right here, then:

/*
 * If the vma does not allow read access, then assume that 
 * it does not allow write access, either. Architectures that
 * allow write without read access are not supported by HMM,
 * because operations such as atomic access would not work.
 */


> + hmm_pfns_clear(range->pfns, range->start, range->end);
> + return 0;
> + }

Similar points as above: it seems like an error case, and the check should be 
right near 
the beginning of the function.

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 06/14] mm/hmm: remove HMM_PFN_READ flag and ignore peculiar architecture

[John Hubbard]

On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
> From: Jérôme Glisse 
> 
> Only peculiar architecture allow write without read thus assume that
> any valid pfn do allow for read. Note we do not care for write only
> because it does make sense with thing like atomic compare and exchange
> or any other operations that allow you to get the memory value through
> them.
> 
> Signed-off-by: Jérôme Glisse 
> Cc: Evgeny Baskakov 
> Cc: Ralph Campbell 
> Cc: Mark Hairgrove 
> Cc: John Hubbard 
> ---
>  include/linux/hmm.h | 14 ++
>  mm/hmm.c| 28 
>  2 files changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index b65e527dd120..4bdc58ffe9f3 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -84,7 +84,6 @@ struct hmm;
>   *
>   * Flags:
>   * HMM_PFN_VALID: pfn is valid

Maybe write it like this:

* HMM_PFN_VALID: pfn is valid. This implies that it has, at least, read 
permission.

> - * HMM_PFN_READ:  CPU page table has read permission set
>   * HMM_PFN_WRITE: CPU page table has write permission set
>   * HMM_PFN_ERROR: corresponding CPU page table entry points to poisoned 
> memory
>   * HMM_PFN_EMPTY: corresponding CPU page table entry is pte_none()
> @@ -97,13 +96,12 @@ struct hmm;
>  typedef unsigned long hmm_pfn_t;
>  
>  #define HMM_PFN_VALID (1 << 0)



>  
> @@ -536,6 +534,17 @@ int hmm_vma_get_pfns(struct hmm_range *range)
>   list_add_rcu(>list, >ranges);
>   spin_unlock(>lock);
>  
> + if (!(vma->vm_flags & VM_READ)) {
> + /*
> +  * If vma do not allow read assume it does not allow write as
> +  * only peculiar architecture allow write without read and this
> +  * is not a case we care about (some operation like atomic no
> +  * longer make sense).
> +  */
> + hmm_pfns_clear(range->pfns, range->start, range->end);
> + return 0;

1. Shouldn't we return an error here? All is not well. No one has any pfns, even
   though they tried to get some. :)

2. I think this check needs to be done much earlier, right after the "Sanity
   check, this should not happen" code in this routine.

> + }
> +
>   hmm_vma_walk.fault = false;
>   hmm_vma_walk.range = range;
>   mm_walk.private = _vma_walk;
> @@ -690,6 +699,17 @@ int hmm_vma_fault(struct hmm_range *range, bool write, 
> bool block)
>   list_add_rcu(>list, >ranges);
>   spin_unlock(>lock);
>  
> + if (!(vma->vm_flags & VM_READ)) {
> + /*
> +  * If vma do not allow read assume it does not allow write as
> +  * only peculiar architecture allow write without read and this
> +  * is not a case we care about (some operation like atomic no
> +  * longer make sense).
> +  */

For the comment wording (for this one, and the one above), how about:

/*
 * If the vma does not allow read access, then assume that 
 * it does not allow write access, either.
 */

...and then leave the more extensive explanation to the commit log. Or,
if we really want a longer explananation right here, then:

/*
 * If the vma does not allow read access, then assume that 
 * it does not allow write access, either. Architectures that
 * allow write without read access are not supported by HMM,
 * because operations such as atomic access would not work.
 */


> + hmm_pfns_clear(range->pfns, range->start, range->end);
> + return 0;
> + }

Similar points as above: it seems like an error case, and the check should be 
right near 
the beginning of the function.

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 05/14] mm/hmm: use struct for hmm_vma_fault(), hmm_vma_get_pfns() parameters

[John Hubbard]

On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
> From: Jérôme Glisse 
> 

Hi Jerome,

I failed to find any problems in this patch, so:

Reviewed-by: John Hubbard 

There are a couple of documentation recommended typo fixes listed
below, which are very minor but as long as I'm here I'll point them out.

> Both hmm_vma_fault() and hmm_vma_get_pfns() were taking a hmm_range
> struct as parameter and were initializing that struct with others of
> their parameters. Have caller of those function do this as they are
> likely to already do and only pass this struct to both function this
> shorten function signature and make it easiers in the future to add

 easier

> new parameters by simply adding them to the structure.
> 
> Signed-off-by: Jérôme Glisse 
> Cc: Evgeny Baskakov 
> Cc: Ralph Campbell 
> Cc: Mark Hairgrove 
> Cc: John Hubbard 
> ---
>  include/linux/hmm.h | 18 -
>  mm/hmm.c| 78 
> +++--
>  2 files changed, 33 insertions(+), 63 deletions(-)



>  
>  
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 64d9e7dae712..49f0f6b337ed 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -490,11 +490,7 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
>  
>  /*
>   * hmm_vma_get_pfns() - snapshot CPU page table for a range of virtual 
> addresses
> - * @vma: virtual memory area containing the virtual address range
> - * @range: used to track snapshot validity
> - * @start: range virtual start address (inclusive)
> - * @end: range virtual end address (exclusive)
> - * @entries: array of hmm_pfn_t: provided by the caller, filled in by 
> function
> + * @range: range being snapshoted and all needed informations

Let's just say this:

* @range: range being snapshotted




> @@ -628,11 +617,7 @@ EXPORT_SYMBOL(hmm_vma_range_done);
>  
>  /*
>   * hmm_vma_fault() - try to fault some address in a virtual address range
> - * @vma: virtual memory area containing the virtual address range
> - * @range: use to track pfns array content validity
> - * @start: fault range virtual start address (inclusive)
> - * @end: fault range virtual end address (exclusive)
> - * @pfns: array of hmm_pfn_t, only entry with fault flag set will be faulted
> + * @range: range being faulted and all needed informations

Similarly here, let's just write it like this:

* @range: range being faulted


thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 05/14] mm/hmm: use struct for hmm_vma_fault(), hmm_vma_get_pfns() parameters

[John Hubbard]

On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
> From: Jérôme Glisse 
> 

Hi Jerome,

I failed to find any problems in this patch, so:

Reviewed-by: John Hubbard 

There are a couple of documentation recommended typo fixes listed
below, which are very minor but as long as I'm here I'll point them out.

> Both hmm_vma_fault() and hmm_vma_get_pfns() were taking a hmm_range
> struct as parameter and were initializing that struct with others of
> their parameters. Have caller of those function do this as they are
> likely to already do and only pass this struct to both function this
> shorten function signature and make it easiers in the future to add

 easier

> new parameters by simply adding them to the structure.
> 
> Signed-off-by: Jérôme Glisse 
> Cc: Evgeny Baskakov 
> Cc: Ralph Campbell 
> Cc: Mark Hairgrove 
> Cc: John Hubbard 
> ---
>  include/linux/hmm.h | 18 -
>  mm/hmm.c| 78 
> +++--
>  2 files changed, 33 insertions(+), 63 deletions(-)



>  
>  
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 64d9e7dae712..49f0f6b337ed 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -490,11 +490,7 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
>  
>  /*
>   * hmm_vma_get_pfns() - snapshot CPU page table for a range of virtual 
> addresses
> - * @vma: virtual memory area containing the virtual address range
> - * @range: used to track snapshot validity
> - * @start: range virtual start address (inclusive)
> - * @end: range virtual end address (exclusive)
> - * @entries: array of hmm_pfn_t: provided by the caller, filled in by 
> function
> + * @range: range being snapshoted and all needed informations

Let's just say this:

* @range: range being snapshotted




> @@ -628,11 +617,7 @@ EXPORT_SYMBOL(hmm_vma_range_done);
>  
>  /*
>   * hmm_vma_fault() - try to fault some address in a virtual address range
> - * @vma: virtual memory area containing the virtual address range
> - * @range: use to track pfns array content validity
> - * @start: fault range virtual start address (inclusive)
> - * @end: fault range virtual end address (exclusive)
> - * @pfns: array of hmm_pfn_t, only entry with fault flag set will be faulted
> + * @range: range being faulted and all needed informations

Similarly here, let's just write it like this:

* @range: range being faulted


thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 2/2] kprobe: fix: Add ftrace_ops_assist_func to kprobe blacklist

[Steven Rostedt]

On Sat, 17 Mar 2018 10:22:11 +0900
Masami Hiramatsu  wrote:

> Or, we can check it by ftrace_location_range() as below patch.

Cute ;-)

> 
> Note that as a side-effect, we can not trace functions in trace_kprobe.c
> anymore, and this means it is hard to me to make a testcase of kprobe events.
> It was the easiest (and maintainable) way to make test cases... sigh.
> 
> -
> tracing: kprobes: Prohibit probing on notrace function
> 
> From: Masami Hiramatsu 
> 
> Prohibit kprobe-events probing on notrace function.
> Since probing on the notrace function can cause recursive
> event call. In most case those are just skipped, but
> in some case it falls into infinit recursive call.
> 
> Signed-off-by: Masami Hiramatsu 
> ---
>  kernel/trace/trace_kprobe.c |   23 +++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5ce9b8cf7be3..7404b012e51a 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -487,6 +487,23 @@ disable_trace_kprobe(struct trace_kprobe *tk, struct 
> trace_event_file *file)
>   return ret;
>  }
>  
> +#ifdef CONFIG_KPROBES_ON_FTRACE
> +static bool within_notrace_func(struct trace_kprobe *tk)
> +{
> + unsigned long offset, size, addr;
> +
> + addr = kallsyms_lookup_name(trace_kprobe_symbol(tk));
> + addr += trace_kprobe_offset(tk);
> +
> + if (!kallsyms_lookup_size_offset(addr, , ))
> + return true;/* Out of range. */
> +
> + return !ftrace_location_range(addr - offset, addr - offset + size);
> +}
> +#else
> +#define within_notrace_func(tk)  (false)
> +#endif
> +
>  /* Internal register function - just handle k*probes and flags */
>  static int __register_trace_kprobe(struct trace_kprobe *tk)
>  {
> @@ -495,6 +512,12 @@ static int __register_trace_kprobe(struct trace_kprobe 
> *tk)
>   if (trace_probe_is_registered(>tp))
>   return -EINVAL;
>  
> + if (within_notrace_func(tk)) {
> + pr_warn("Could not probe notrace function %s\n",
> + trace_kprobe_symbol(tk));
> + return -EINVAL;
> + }

This will prevent probing assembly code. Do we want to do that? Or is
kprobes already prohibited from doing so?

-- Steve


> +
>   for (i = 0; i < tk->tp.nr_args; i++)
>   traceprobe_update_arg(>tp.args[i]);
>  
> 

Re: [PATCH 2/2] kprobe: fix: Add ftrace_ops_assist_func to kprobe blacklist

[Steven Rostedt]

On Sat, 17 Mar 2018 10:22:11 +0900
Masami Hiramatsu  wrote:

> Or, we can check it by ftrace_location_range() as below patch.

Cute ;-)

> 
> Note that as a side-effect, we can not trace functions in trace_kprobe.c
> anymore, and this means it is hard to me to make a testcase of kprobe events.
> It was the easiest (and maintainable) way to make test cases... sigh.
> 
> -
> tracing: kprobes: Prohibit probing on notrace function
> 
> From: Masami Hiramatsu 
> 
> Prohibit kprobe-events probing on notrace function.
> Since probing on the notrace function can cause recursive
> event call. In most case those are just skipped, but
> in some case it falls into infinit recursive call.
> 
> Signed-off-by: Masami Hiramatsu 
> ---
>  kernel/trace/trace_kprobe.c |   23 +++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5ce9b8cf7be3..7404b012e51a 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -487,6 +487,23 @@ disable_trace_kprobe(struct trace_kprobe *tk, struct 
> trace_event_file *file)
>   return ret;
>  }
>  
> +#ifdef CONFIG_KPROBES_ON_FTRACE
> +static bool within_notrace_func(struct trace_kprobe *tk)
> +{
> + unsigned long offset, size, addr;
> +
> + addr = kallsyms_lookup_name(trace_kprobe_symbol(tk));
> + addr += trace_kprobe_offset(tk);
> +
> + if (!kallsyms_lookup_size_offset(addr, , ))
> + return true;/* Out of range. */
> +
> + return !ftrace_location_range(addr - offset, addr - offset + size);
> +}
> +#else
> +#define within_notrace_func(tk)  (false)
> +#endif
> +
>  /* Internal register function - just handle k*probes and flags */
>  static int __register_trace_kprobe(struct trace_kprobe *tk)
>  {
> @@ -495,6 +512,12 @@ static int __register_trace_kprobe(struct trace_kprobe 
> *tk)
>   if (trace_probe_is_registered(>tp))
>   return -EINVAL;
>  
> + if (within_notrace_func(tk)) {
> + pr_warn("Could not probe notrace function %s\n",
> + trace_kprobe_symbol(tk));
> + return -EINVAL;
> + }

This will prevent probing assembly code. Do we want to do that? Or is
kprobes already prohibited from doing so?

-- Steve


> +
>   for (i = 0; i < tk->tp.nr_args; i++)
>   traceprobe_update_arg(>tp.args[i]);
>  
> 

Re: [PATCH 03/14] mm/hmm: HMM should have a callback before MM is destroyed v2

[John Hubbard]

On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
> From: Ralph Campbell 
> 



> +static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
> +{
> + struct hmm *hmm = mm->hmm;
> + struct hmm_mirror *mirror;
> + struct hmm_mirror *mirror_next;
> +
> + down_write(>mirrors_sem);
> + list_for_each_entry_safe(mirror, mirror_next, >mirrors, list) {
> + list_del_init(>list);
> + if (mirror->ops->release)
> + mirror->ops->release(mirror);
> + }
> + up_write(>mirrors_sem);
> +}
> +

OK, as for actual code review:

This part of the locking looks good. However, I think it can race against
hmm_mirror_register(), because hmm_mirror_register() will just add a new 
mirror regardless.

So:

thread 1  thread 2
---
hmm_release   hmm_mirror_register 
down_write(>mirrors_sem);
// deletes all list items
up_write
  unblocked: adds new mirror
  

...so I think we need a way to back out of any pending hmm_mirror_register()
calls, as part of the .release steps, right? It seems hard for the device 
driver,
which could be inside of hmm_mirror_register(), to handle that. Especially 
considering
that right now, hmm_mirror_register() will return success in this case--so
there is no indication that anything is wrong.

Maybe hmm_mirror_register() could return an error (and not add to the mirror 
list),
in such a situation, how's that sound?

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH 03/14] mm/hmm: HMM should have a callback before MM is destroyed v2

[John Hubbard]

On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
> From: Ralph Campbell 
> 



> +static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
> +{
> + struct hmm *hmm = mm->hmm;
> + struct hmm_mirror *mirror;
> + struct hmm_mirror *mirror_next;
> +
> + down_write(>mirrors_sem);
> + list_for_each_entry_safe(mirror, mirror_next, >mirrors, list) {
> + list_del_init(>list);
> + if (mirror->ops->release)
> + mirror->ops->release(mirror);
> + }
> + up_write(>mirrors_sem);
> +}
> +

OK, as for actual code review:

This part of the locking looks good. However, I think it can race against
hmm_mirror_register(), because hmm_mirror_register() will just add a new 
mirror regardless.

So:

thread 1  thread 2
---
hmm_release   hmm_mirror_register 
down_write(>mirrors_sem);
// deletes all list items
up_write
  unblocked: adds new mirror
  

...so I think we need a way to back out of any pending hmm_mirror_register()
calls, as part of the .release steps, right? It seems hard for the device 
driver,
which could be inside of hmm_mirror_register(), to handle that. Especially 
considering
that right now, hmm_mirror_register() will return success in this case--so
there is no indication that anything is wrong.

Maybe hmm_mirror_register() could return an error (and not add to the mirror 
list),
in such a situation, how's that sound?

thanks,
-- 
John Hubbard
NVIDIA

Re: [PATCH v2] staging: typec: rt1711h typec chip driver

[Guenter Roeck]

On 03/16/2018 10:40 AM, ShuFan Lee wrote:

From: ShuFan Lee 

Richtek RT1711H Type-C chip driver that works with
Type-C Port Controller Manager to provide USB PD and
USB Type-C functionalities.
Add definition of TCPC_CC_STATUS_TOGGLING.

Signed-off-by: ShuFan Lee 


Does this patch require DT review for the bindings ?

Gueter


---
  drivers/staging/typec/Kconfig |   8 +
  drivers/staging/typec/Makefile|   1 +
  drivers/staging/typec/tcpci.h |   1 +
  drivers/staging/typec/tcpci_rt1711h.c | 329 ++
  4 files changed, 339 insertions(+)
  create mode 100644 drivers/staging/typec/tcpci_rt1711h.c

  changelogs between v1 and v2
  - use gpiod_* instead of gpio_*

diff --git a/drivers/staging/typec/Kconfig b/drivers/staging/typec/Kconfig
index 5359f556d203..3aa981fbc8f5 100644
--- a/drivers/staging/typec/Kconfig
+++ b/drivers/staging/typec/Kconfig
@@ -9,6 +9,14 @@ config TYPEC_TCPCI
help
  Type-C Port Controller driver for TCPCI-compliant controller.
  
+config TYPEC_RT1711H

+   tristate "Richtek RT1711H Type-C chip driver"
+   select TYPEC_TCPCI
+   help
+ Richtek RT1711H Type-C chip driver that works with
+ Type-C Port Controller Manager to provide USB PD and USB
+ Type-C functionalities.
+
  endif
  
  endmenu

diff --git a/drivers/staging/typec/Makefile b/drivers/staging/typec/Makefile
index 53d649abcb53..7803d485e1b3 100644
--- a/drivers/staging/typec/Makefile
+++ b/drivers/staging/typec/Makefile
@@ -1 +1,2 @@
  obj-$(CONFIG_TYPEC_TCPCI) += tcpci.o
+obj-$(CONFIG_TYPEC_RT1711H)+= tcpci_rt1711h.o
diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h
index 34c865f0dcf6..303ebde26546 100644
--- a/drivers/staging/typec/tcpci.h
+++ b/drivers/staging/typec/tcpci.h
@@ -59,6 +59,7 @@
  #define TCPC_POWER_CTRL_VCONN_ENABLE  BIT(0)
  
  #define TCPC_CC_STATUS			0x1d

+#define TCPC_CC_STATUS_TOGGLINGBIT(5)
  #define TCPC_CC_STATUS_TERM   BIT(4)
  #define TCPC_CC_STATUS_CC2_SHIFT  2
  #define TCPC_CC_STATUS_CC2_MASK   0x3
diff --git a/drivers/staging/typec/tcpci_rt1711h.c 
b/drivers/staging/typec/tcpci_rt1711h.c
new file mode 100644
index ..12afac363d6d
--- /dev/null
+++ b/drivers/staging/typec/tcpci_rt1711h.c
@@ -0,0 +1,329 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018, Richtek Technology Corporation
+ *
+ * Richtek RT1711H Type-C Chip Driver
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "tcpci.h"
+
+#define RT1711H_RTCTRL80x9B
+
+/* Autoidle timeout = (tout * 2 + 1) * 6.4ms */
+#define RT1711H_RTCTRL8_SET(ck300, ship_off, auto_idle, tout) \
+   (((ck300) << 7) | ((ship_off) << 5) | \
+   ((auto_idle) << 3) | ((tout) & 0x07))
+
+#define RT1711H_RTCTRL11   0x9E
+
+/* I2C timeout = (tout + 1) * 12.5ms */
+#define RT1711H_RTCTRL11_SET(en, tout) \
+(((en) << 7) | ((tout) & 0x0F))
+
+#define RT1711H_RTCTRL13   0xA0
+#define RT1711H_RTCTRL14   0xA1
+#define RT1711H_RTCTRL15   0xA2
+#define RT1711H_RTCTRL16   0xA3
+
+struct rt1711h_chip {
+   struct tcpci_data data;
+   struct tcpci *tcpci;
+   struct device *dev;
+   int irq;
+};
+
+static int rt1711h_read16(struct rt1711h_chip *chip, unsigned int reg, u16 
*val)
+{
+   return regmap_raw_read(chip->data.regmap, reg, val, sizeof(u16));
+}
+
+static int rt1711h_write16(struct rt1711h_chip *chip, unsigned int reg, u16 
val)
+{
+   return regmap_raw_write(chip->data.regmap, reg, , sizeof(u16));
+}
+
+static int rt1711h_read8(struct rt1711h_chip *chip, unsigned int reg, u8 *val)
+{
+   return regmap_raw_read(chip->data.regmap, reg, val, sizeof(u8));
+}
+
+static int rt1711h_write8(struct rt1711h_chip *chip, unsigned int reg, u8 val)
+{
+   return regmap_raw_write(chip->data.regmap, reg, , sizeof(u8));
+}
+
+static const struct regmap_config rt1711h_regmap_config = {
+   .reg_bits = 8,
+   .val_bits = 8,
+
+   .max_register = 0xFF, /* 0x80 .. 0xFF are vendor defined */
+};
+
+static struct rt1711h_chip *tdata_to_rt1711h(struct tcpci_data *tdata)
+{
+   return container_of(tdata, struct rt1711h_chip, data);
+}
+
+static int rt1711h_init(struct tcpci *tcpci, struct tcpci_data *tdata)
+{
+   int ret;
+   struct rt1711h_chip *chip = tdata_to_rt1711h(tdata);
+
+   /* CK 300K from 320K, shipping off, auto_idle enable, tout = 32ms */
+   ret = rt1711h_write8(chip, RT1711H_RTCTRL8,
+RT1711H_RTCTRL8_SET(0, 1, 1, 2));
+   if (ret < 0)
+   return ret;
+
+   /* I2C reset : (val + 1) * 12.5ms */
+   ret = rt1711h_write8(chip, RT1711H_RTCTRL11,
+RT1711H_RTCTRL11_SET(1, 0x0F));
+   if (ret < 0)
+   return ret;
+
+   /* 

Re: [PATCH v2] staging: typec: rt1711h typec chip driver

[Guenter Roeck]

On 03/16/2018 10:40 AM, ShuFan Lee wrote:

From: ShuFan Lee 

Richtek RT1711H Type-C chip driver that works with
Type-C Port Controller Manager to provide USB PD and
USB Type-C functionalities.
Add definition of TCPC_CC_STATUS_TOGGLING.

Signed-off-by: ShuFan Lee 


Does this patch require DT review for the bindings ?

Gueter


---
  drivers/staging/typec/Kconfig |   8 +
  drivers/staging/typec/Makefile|   1 +
  drivers/staging/typec/tcpci.h |   1 +
  drivers/staging/typec/tcpci_rt1711h.c | 329 ++
  4 files changed, 339 insertions(+)
  create mode 100644 drivers/staging/typec/tcpci_rt1711h.c

  changelogs between v1 and v2
  - use gpiod_* instead of gpio_*

diff --git a/drivers/staging/typec/Kconfig b/drivers/staging/typec/Kconfig
index 5359f556d203..3aa981fbc8f5 100644
--- a/drivers/staging/typec/Kconfig
+++ b/drivers/staging/typec/Kconfig
@@ -9,6 +9,14 @@ config TYPEC_TCPCI
help
  Type-C Port Controller driver for TCPCI-compliant controller.
  
+config TYPEC_RT1711H

+   tristate "Richtek RT1711H Type-C chip driver"
+   select TYPEC_TCPCI
+   help
+ Richtek RT1711H Type-C chip driver that works with
+ Type-C Port Controller Manager to provide USB PD and USB
+ Type-C functionalities.
+
  endif
  
  endmenu

diff --git a/drivers/staging/typec/Makefile b/drivers/staging/typec/Makefile
index 53d649abcb53..7803d485e1b3 100644
--- a/drivers/staging/typec/Makefile
+++ b/drivers/staging/typec/Makefile
@@ -1 +1,2 @@
  obj-$(CONFIG_TYPEC_TCPCI) += tcpci.o
+obj-$(CONFIG_TYPEC_RT1711H)+= tcpci_rt1711h.o
diff --git a/drivers/staging/typec/tcpci.h b/drivers/staging/typec/tcpci.h
index 34c865f0dcf6..303ebde26546 100644
--- a/drivers/staging/typec/tcpci.h
+++ b/drivers/staging/typec/tcpci.h
@@ -59,6 +59,7 @@
  #define TCPC_POWER_CTRL_VCONN_ENABLE  BIT(0)
  
  #define TCPC_CC_STATUS			0x1d

+#define TCPC_CC_STATUS_TOGGLINGBIT(5)
  #define TCPC_CC_STATUS_TERM   BIT(4)
  #define TCPC_CC_STATUS_CC2_SHIFT  2
  #define TCPC_CC_STATUS_CC2_MASK   0x3
diff --git a/drivers/staging/typec/tcpci_rt1711h.c 
b/drivers/staging/typec/tcpci_rt1711h.c
new file mode 100644
index ..12afac363d6d
--- /dev/null
+++ b/drivers/staging/typec/tcpci_rt1711h.c
@@ -0,0 +1,329 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018, Richtek Technology Corporation
+ *
+ * Richtek RT1711H Type-C Chip Driver
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "tcpci.h"
+
+#define RT1711H_RTCTRL80x9B
+
+/* Autoidle timeout = (tout * 2 + 1) * 6.4ms */
+#define RT1711H_RTCTRL8_SET(ck300, ship_off, auto_idle, tout) \
+   (((ck300) << 7) | ((ship_off) << 5) | \
+   ((auto_idle) << 3) | ((tout) & 0x07))
+
+#define RT1711H_RTCTRL11   0x9E
+
+/* I2C timeout = (tout + 1) * 12.5ms */
+#define RT1711H_RTCTRL11_SET(en, tout) \
+(((en) << 7) | ((tout) & 0x0F))
+
+#define RT1711H_RTCTRL13   0xA0
+#define RT1711H_RTCTRL14   0xA1
+#define RT1711H_RTCTRL15   0xA2
+#define RT1711H_RTCTRL16   0xA3
+
+struct rt1711h_chip {
+   struct tcpci_data data;
+   struct tcpci *tcpci;
+   struct device *dev;
+   int irq;
+};
+
+static int rt1711h_read16(struct rt1711h_chip *chip, unsigned int reg, u16 
*val)
+{
+   return regmap_raw_read(chip->data.regmap, reg, val, sizeof(u16));
+}
+
+static int rt1711h_write16(struct rt1711h_chip *chip, unsigned int reg, u16 
val)
+{
+   return regmap_raw_write(chip->data.regmap, reg, , sizeof(u16));
+}
+
+static int rt1711h_read8(struct rt1711h_chip *chip, unsigned int reg, u8 *val)
+{
+   return regmap_raw_read(chip->data.regmap, reg, val, sizeof(u8));
+}
+
+static int rt1711h_write8(struct rt1711h_chip *chip, unsigned int reg, u8 val)
+{
+   return regmap_raw_write(chip->data.regmap, reg, , sizeof(u8));
+}
+
+static const struct regmap_config rt1711h_regmap_config = {
+   .reg_bits = 8,
+   .val_bits = 8,
+
+   .max_register = 0xFF, /* 0x80 .. 0xFF are vendor defined */
+};
+
+static struct rt1711h_chip *tdata_to_rt1711h(struct tcpci_data *tdata)
+{
+   return container_of(tdata, struct rt1711h_chip, data);
+}
+
+static int rt1711h_init(struct tcpci *tcpci, struct tcpci_data *tdata)
+{
+   int ret;
+   struct rt1711h_chip *chip = tdata_to_rt1711h(tdata);
+
+   /* CK 300K from 320K, shipping off, auto_idle enable, tout = 32ms */
+   ret = rt1711h_write8(chip, RT1711H_RTCTRL8,
+RT1711H_RTCTRL8_SET(0, 1, 1, 2));
+   if (ret < 0)
+   return ret;
+
+   /* I2C reset : (val + 1) * 12.5ms */
+   ret = rt1711h_write8(chip, RT1711H_RTCTRL11,
+RT1711H_RTCTRL11_SET(1, 0x0F));
+   if (ret < 0)
+   return ret;
+
+   /* tTCPCfilter : (26.7 * val) us */
+   ret = 

Re: [PATCH 04/14] mm/hmm: hmm_pfns_bad() was accessing wrong struct

[John Hubbard]

On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
> From: Jérôme Glisse 
> 
> The private field of mm_walk struct point to an hmm_vma_walk struct and
> not to the hmm_range struct desired. Fix to get proper struct pointer.
> 
> Signed-off-by: Jérôme Glisse 
> Cc: sta...@vger.kernel.org
> Cc: Evgeny Baskakov 
> Cc: Ralph Campbell 
> Cc: Mark Hairgrove 
> Cc: John Hubbard 
> ---
>  mm/hmm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 6088fa6ed137..64d9e7dae712 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -293,7 +293,8 @@ static int hmm_pfns_bad(unsigned long addr,
>   unsigned long end,
>   struct mm_walk *walk)
>  {
> - struct hmm_range *range = walk->private;
> + struct hmm_vma_walk *hmm_vma_walk = walk->private;
> + struct hmm_range *range = hmm_vma_walk->range;
>   hmm_pfn_t *pfns = range->pfns;
>   unsigned long i;
>  

This fix looks good. I also checked the other uses of walk->private, of course, 
but it was only this one that was wrong.

I think this patch also belongs in -stable, because it is a simple bug fix.

For the description, well...actually, because ->range is the first element in
struct hmm_vma_walk, you probably end up with the same pointer value, both
before and after this fix. So maybe there are no symptoms to see. Maybe that's
an argument for *not* putting it in -stable, too. I'll leave that question
to more experienced people.

Either way, you can add: 

Reviewed by: John Hubbard 

thanks,
-- 
John Hubbard
NVIDIA
 

Re: [PATCH 04/14] mm/hmm: hmm_pfns_bad() was accessing wrong struct

[John Hubbard]

On 03/16/2018 12:14 PM, jgli...@redhat.com wrote:
> From: Jérôme Glisse 
> 
> The private field of mm_walk struct point to an hmm_vma_walk struct and
> not to the hmm_range struct desired. Fix to get proper struct pointer.
> 
> Signed-off-by: Jérôme Glisse 
> Cc: sta...@vger.kernel.org
> Cc: Evgeny Baskakov 
> Cc: Ralph Campbell 
> Cc: Mark Hairgrove 
> Cc: John Hubbard 
> ---
>  mm/hmm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 6088fa6ed137..64d9e7dae712 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -293,7 +293,8 @@ static int hmm_pfns_bad(unsigned long addr,
>   unsigned long end,
>   struct mm_walk *walk)
>  {
> - struct hmm_range *range = walk->private;
> + struct hmm_vma_walk *hmm_vma_walk = walk->private;
> + struct hmm_range *range = hmm_vma_walk->range;
>   hmm_pfn_t *pfns = range->pfns;
>   unsigned long i;
>  

This fix looks good. I also checked the other uses of walk->private, of course, 
but it was only this one that was wrong.

I think this patch also belongs in -stable, because it is a simple bug fix.

For the description, well...actually, because ->range is the first element in
struct hmm_vma_walk, you probably end up with the same pointer value, both
before and after this fix. So maybe there are no symptoms to see. Maybe that's
an argument for *not* putting it in -stable, too. I'll leave that question
to more experienced people.

Either way, you can add: 

Reviewed by: John Hubbard 

thanks,
-- 
John Hubbard
NVIDIA
 

Re: [PATCH v3 1/7] watchdog: sama5d4: make use of timeout-secs provided in devicetree

[Guenter Roeck]

On 03/16/2018 06:37 AM, Marcus Folkesson wrote:

Hi,

Am I supposed to do something more to make this patchset picked up?



Did you check linux-next ?

Guenter


Thanks,
Marcus Folkesson

On Sun, Feb 11, 2018 at 09:08:41PM +0100, Marcus Folkesson wrote:

watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.

Following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt to make use of
the parameter logic.

Signed-off-by: Marcus Folkesson 
---

Notes:
 v3:
- Use wdd->timeout instead of wdt_timeout when print out
  timout in probe function.

  drivers/watchdog/sama5d4_wdt.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index 0ae947c3d7bc..255169916dbb 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -33,7 +33,7 @@ struct sama5d4_wdt {
unsigned long   last_ping;
  };
  
-static int wdt_timeout = WDT_DEFAULT_TIMEOUT;

+static int wdt_timeout;
  static bool nowayout = WATCHDOG_NOWAYOUT;
  
  module_param(wdt_timeout, int, 0);

@@ -212,7 +212,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
return -ENOMEM;
  
  	wdd = >wdd;

-   wdd->timeout = wdt_timeout;
+   wdd->timeout = WDT_DEFAULT_TIMEOUT;
wdd->info = _wdt_info;
wdd->ops = _wdt_ops;
wdd->min_timeout = MIN_WDT_TIMEOUT;
@@ -273,7 +273,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, wdt);
  
  	dev_info(>dev, "initialized (timeout = %d sec, nowayout = %d)\n",

-wdt_timeout, nowayout);
+wdd->timeout, nowayout);
  
  	return 0;

  }
--
2.15.1

Re: [PATCH v3 1/7] watchdog: sama5d4: make use of timeout-secs provided in devicetree

[Guenter Roeck]

On 03/16/2018 06:37 AM, Marcus Folkesson wrote:

Hi,

Am I supposed to do something more to make this patchset picked up?



Did you check linux-next ?

Guenter


Thanks,
Marcus Folkesson

On Sun, Feb 11, 2018 at 09:08:41PM +0100, Marcus Folkesson wrote:

watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.

Following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt to make use of
the parameter logic.

Signed-off-by: Marcus Folkesson 
---

Notes:
 v3:
- Use wdd->timeout instead of wdt_timeout when print out
  timout in probe function.

  drivers/watchdog/sama5d4_wdt.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index 0ae947c3d7bc..255169916dbb 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -33,7 +33,7 @@ struct sama5d4_wdt {
unsigned long   last_ping;
  };
  
-static int wdt_timeout = WDT_DEFAULT_TIMEOUT;

+static int wdt_timeout;
  static bool nowayout = WATCHDOG_NOWAYOUT;
  
  module_param(wdt_timeout, int, 0);

@@ -212,7 +212,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
return -ENOMEM;
  
  	wdd = >wdd;

-   wdd->timeout = wdt_timeout;
+   wdd->timeout = WDT_DEFAULT_TIMEOUT;
wdd->info = _wdt_info;
wdd->ops = _wdt_ops;
wdd->min_timeout = MIN_WDT_TIMEOUT;
@@ -273,7 +273,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, wdt);
  
  	dev_info(>dev, "initialized (timeout = %d sec, nowayout = %d)\n",

-wdt_timeout, nowayout);
+wdd->timeout, nowayout);
  
  	return 0;

  }
--
2.15.1

Re: [PATCH v7 0/2] hwmon: (ucd9000) Add gpio and debugfs interfaces

[Guenter Roeck]

On 03/16/2018 02:22 PM, Eddie James wrote:

The ucd9000 series chips have gpio pins. Add a gpio chip interface to the ucd
device so that users can query and set the state of the gpio pins.

Add a debugfs interface using the existing pmbus debugfs directory to provide
MFR_STATUS and the status of the gpi faults to users.



Series applied to hwmon-next. Thanks a lot for your patience.

Guenter


Changes since v6:
  - don't return null terminator for mfr_status

Changes since v5:
  - enclose gpio code in #ifdef GPIOLIB
  - don't initialize buffers for mfr_status; set last char to 0 instead
  - cap the size argument to bin2hex

Changes since v4:
  - max-sized buffers for smbus transfers
  - used bin2hex instead of my own code

Changes since v3:
  - remove setting of gpio_chip->owner
  - format the mfr_status data
  - switch to #ifdef rather than #if IS_ENABLED for debugfs

Changes since v2:
  - split the gpio registration into it's own function

Changes since v1:
  - dropped dev_err messages
  - made gpio chip registration conditional on having gpio pins
  - made mfr_status debugfs attribute more simple


Christopher Bostic (2):
   hwmon: (ucd9000) Add gpio chip interface
   hwmon: (ucd9000) Add debugfs attributes to provide mfr_status

  drivers/hwmon/pmbus/ucd9000.c | 350 +-
  1 file changed, 349 insertions(+), 1 deletion(-)

Re: [PATCH v7 0/2] hwmon: (ucd9000) Add gpio and debugfs interfaces

[Guenter Roeck]

On 03/16/2018 02:22 PM, Eddie James wrote:

The ucd9000 series chips have gpio pins. Add a gpio chip interface to the ucd
device so that users can query and set the state of the gpio pins.

Add a debugfs interface using the existing pmbus debugfs directory to provide
MFR_STATUS and the status of the gpi faults to users.



Series applied to hwmon-next. Thanks a lot for your patience.

Guenter


Changes since v6:
  - don't return null terminator for mfr_status

Changes since v5:
  - enclose gpio code in #ifdef GPIOLIB
  - don't initialize buffers for mfr_status; set last char to 0 instead
  - cap the size argument to bin2hex

Changes since v4:
  - max-sized buffers for smbus transfers
  - used bin2hex instead of my own code

Changes since v3:
  - remove setting of gpio_chip->owner
  - format the mfr_status data
  - switch to #ifdef rather than #if IS_ENABLED for debugfs

Changes since v2:
  - split the gpio registration into it's own function

Changes since v1:
  - dropped dev_err messages
  - made gpio chip registration conditional on having gpio pins
  - made mfr_status debugfs attribute more simple


Christopher Bostic (2):
   hwmon: (ucd9000) Add gpio chip interface
   hwmon: (ucd9000) Add debugfs attributes to provide mfr_status

  drivers/hwmon/pmbus/ucd9000.c | 350 +-
  1 file changed, 349 insertions(+), 1 deletion(-)

Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()

[Miguel Ojeda]

On Fri, Mar 16, 2018 at 9:14 PM, Linus Torvalds
 wrote:
> On Fri, Mar 16, 2018 at 1:03 PM, Miguel Ojeda
>  wrote:
>>>
>>> Kees - is there some online "gcc-4.4 checker" somewhere? This does
>>> seem to work with my gcc. I actually tested some of those files you
>>> pointed at now.
>>
>> I use this one:
>>
>>   https://godbolt.org/
>
> Well, my *test* code works on that one and -Wvla -Werror.
>
> It does not work with gcc-4.1.x, but works with gcc-4.4.x.
>
> I can't seem to see the errors any way, I wonder if
> __builtin_choose_expr() simply didn't exist back then.
>
> Odd that you can't view warnings/errors with it.
>
> But it's possible that it fails on more complex stuff in the kernel.
>
> I've done a "allmodconfig" build with that patch, and the only issue
> it found was that (real) type issue in tpm_tis_core.h.

Just tested your max() with a Python script I wrote yesterday to try a
lot of combinations for this thing. Your version gives some warnings
in some cases, like:

  warning: signed and unsigned type in conditional expression [-Wsign-compare]
   #define __cmp(a,b,op) ((a)op(b)?(a):(b))

  warning: comparison between signed and unsigned integer expressions
[-Wsign-compare]
   #define const_max(a,b)  __careful_cmp(a,b,>)

  warning: comparison of distinct pointer types lacks a cast
   (!!(sizeof((typeof(a)*)1==(typeof(b)*)1)))

But it fails on something like (with warnings):

  int a[const_max(-30, 60u)];

Sorry... :-(

Has anyone taken a look at the last one I sent? Patch attached with
the draft changes on the kernel. It compiles fine the cases Kees
cleaned up in the other patch, but also works without a explicit type,
for mixed types, and for both positive and negative values.

Cheers,
Miguel
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 3fd291503576..f83658a4f15d 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -819,6 +819,49 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 	  __UNIQUE_ID(max1_), __UNIQUE_ID(max2_),	\
 	  x, y)
 
+size_t __error_not_const_arg(void) \
+__compiletime_error("const_max() used with non-compile-time constant arg");
+size_t __error_too_big(void) \
+__compiletime_error("const_max() used with an arg too big");
+
+#define INTMAXT_MAX LLONG_MAX
+typedef int64_t intmax_t;
+
+#define const_cmp(x, y, op)   \
+__builtin_choose_expr(\
+!__builtin_constant_p(x) || !__builtin_constant_p(y), \
+__error_not_const_arg(),  \
+__builtin_choose_expr(\
+(x) > INTMAXT_MAX || (y) > INTMAXT_MAX,   \
+__error_too_big(),\
+__builtin_choose_expr(\
+(intmax_t)(x) op (intmax_t)(y),   \
+(x),  \
+(y)   \
+) \
+) \
+)
+
+/**
+ * const_min - returns minimum of two compile-time constant values
+ * @x: first compile-time constant value
+ * @y: second compile-time constant value
+ *
+ * The result has the type of the winner of the comparison. Works for any
+ * value up to LLONG_MAX.
+ */
+#define const_min(x, y) const_cmp((x), (y), <=)
+
+/**
+ * const_max - returns maximum of two compile-time constant values
+ * @x: first compile-time constant value
+ * @y: second compile-time constant value
+ *
+ * The result has the type of the winner of the comparison. Works for any
+ * value up to LLONG_MAX.
+ */
+#define const_max(x, y) const_cmp((x), (y), >=)
+
 /**
  * min3 - return minimum of three values
  * @x: first value

Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()

[Miguel Ojeda]

On Fri, Mar 16, 2018 at 9:14 PM, Linus Torvalds
 wrote:
> On Fri, Mar 16, 2018 at 1:03 PM, Miguel Ojeda
>  wrote:
>>>
>>> Kees - is there some online "gcc-4.4 checker" somewhere? This does
>>> seem to work with my gcc. I actually tested some of those files you
>>> pointed at now.
>>
>> I use this one:
>>
>>   https://godbolt.org/
>
> Well, my *test* code works on that one and -Wvla -Werror.
>
> It does not work with gcc-4.1.x, but works with gcc-4.4.x.
>
> I can't seem to see the errors any way, I wonder if
> __builtin_choose_expr() simply didn't exist back then.
>
> Odd that you can't view warnings/errors with it.
>
> But it's possible that it fails on more complex stuff in the kernel.
>
> I've done a "allmodconfig" build with that patch, and the only issue
> it found was that (real) type issue in tpm_tis_core.h.

Just tested your max() with a Python script I wrote yesterday to try a
lot of combinations for this thing. Your version gives some warnings
in some cases, like:

  warning: signed and unsigned type in conditional expression [-Wsign-compare]
   #define __cmp(a,b,op) ((a)op(b)?(a):(b))

  warning: comparison between signed and unsigned integer expressions
[-Wsign-compare]
   #define const_max(a,b)  __careful_cmp(a,b,>)

  warning: comparison of distinct pointer types lacks a cast
   (!!(sizeof((typeof(a)*)1==(typeof(b)*)1)))

But it fails on something like (with warnings):

  int a[const_max(-30, 60u)];

Sorry... :-(

Has anyone taken a look at the last one I sent? Patch attached with
the draft changes on the kernel. It compiles fine the cases Kees
cleaned up in the other patch, but also works without a explicit type,
for mixed types, and for both positive and negative values.

Cheers,
Miguel
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 3fd291503576..f83658a4f15d 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -819,6 +819,49 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 	  __UNIQUE_ID(max1_), __UNIQUE_ID(max2_),	\
 	  x, y)
 
+size_t __error_not_const_arg(void) \
+__compiletime_error("const_max() used with non-compile-time constant arg");
+size_t __error_too_big(void) \
+__compiletime_error("const_max() used with an arg too big");
+
+#define INTMAXT_MAX LLONG_MAX
+typedef int64_t intmax_t;
+
+#define const_cmp(x, y, op)   \
+__builtin_choose_expr(\
+!__builtin_constant_p(x) || !__builtin_constant_p(y), \
+__error_not_const_arg(),  \
+__builtin_choose_expr(\
+(x) > INTMAXT_MAX || (y) > INTMAXT_MAX,   \
+__error_too_big(),\
+__builtin_choose_expr(\
+(intmax_t)(x) op (intmax_t)(y),   \
+(x),  \
+(y)   \
+) \
+) \
+)
+
+/**
+ * const_min - returns minimum of two compile-time constant values
+ * @x: first compile-time constant value
+ * @y: second compile-time constant value
+ *
+ * The result has the type of the winner of the comparison. Works for any
+ * value up to LLONG_MAX.
+ */
+#define const_min(x, y) const_cmp((x), (y), <=)
+
+/**
+ * const_max - returns maximum of two compile-time constant values
+ * @x: first compile-time constant value
+ * @y: second compile-time constant value
+ *
+ * The result has the type of the winner of the comparison. Works for any
+ * value up to LLONG_MAX.
+ */
+#define const_max(x, y) const_cmp((x), (y), >=)
+
 /**
  * min3 - return minimum of three values
  * @x: first value

Re: [v6] watchdog: add SPDX identifiers for watchdog subsystem

[Guenter Roeck]

On Fri, Mar 16, 2018 at 04:14:11PM +0100, Marcus Folkesson wrote:
> - Add SPDX identifier
> - Remove boiler plate license text
> - If MODULE_LICENSE and boiler plate does not match, go for boiler plate
>   license
> 
> Signed-off-by: Marcus Folkesson 
> Acked-by: Adam Thomson 
> Acked-by: Baruch Siach 
> Acked-by: Charles Keepax 
> Acked-by: Keiji Hayashibara 
> Acked-by: Johannes Thumshirn 
> Acked-by: Florian Fainelli 
> Acked-by: Mans Rullgard 
> Acked-by: Matthias Brugger 
> Acked-by: Michal Simek 
> Acked-by: Neil Armstrong 
> Acked-by: Nicolas Ferre 
> Acked-by: Thierry Reding 
> Acked-by: Tomas Winkler 
> Acked-by: Patrice Chotard 
> Acked-by: William Breathitt Gray 
> Reviewed-by: Eric Anholt 

Reviewed-by: Guenter Roeck 

> ---
> 
> Notes:
> v6:
>   - Only include those drivers that has been acked-by.
> 
> drivers/watchdog/da9052_wdt.c
> drivers/watchdog/da9055_wdt.c
> drivers/watchdog/da9062_wdt.c
> drivers/watchdog/da9063_wdt.c
> Acked-by: Adam Thomson 
> 
> drivers/watchdog/digicolor_wdt.c
> Acked-by: Baruch Siach 
> 
> drivers/watchdog/wm831x_wdt.c
> drivers/watchdog/wm8350_wdt.c
> Acked-by: Charles Keepax 
> 
> drivers/watchdog/ar7_wdt.c
> drivers/watchdog/bcm2835_wdt.c
> drivers/watchdog/bcm47xx_wdt.c
> drivers/watchdog/bcm63xx_wdt.c
> drivers/watchdog/bcm7038_wdt.c
> drivers/watchdog/bcm_kona_wdt.c
> drivers/watchdog/mtx-1_wdt.c
> Acked-by: Florian Fainelli 
> 
> drivers/watchdog/uniphier_wdt.c
> Acked-by: Keiji Hayashibara 
> 
> drivers/watchdog/mena21_wdt
> Acked-by: Johannes Thumshirn 
> 
> drivers/watchdog/tangox_wdt.c
> Acked-by: Mans Rullgard 
> 
> drivers/watchdog/mtk_wdt.c
> Acked-by: Matthias Brugger 
> 
> drivers/watchdog/cadence_wdt.c
> drivers/watchdog/of_xilinx_wdt.c
> Acked-by: Michal Simek 
> 
> drivers/watchdog/meson_gxbb_wdt.c
> Acked-by: Neil Armstrong 
> 
> drivers/watchdog/at91rm9200_wdt.c
> drivers/watchdog/at91sam9_wdt.c
> drivers/watchdog/at91sam9_wdt.h
> drivers/watchdog/sama5d4_wdt.c
> Acked-by: Nicolas Ferre 
> 
> drivers/watchdog/tegra_wdt.c
> Acked-by: Thierry Reding 
> 
> drivers/watchdog/mei_wdt.c
> Acked-by: Tomas Winkler 
> 
> drivers/watchdog/st_lpc_wdt.c
> Acked-by: Patrice Chotard 
> 
> drivers/watchdog/ebc-c384_wdt.c
> Acked-by: William Breathitt Gray 
> 
> v5:
>   - Add missing SPDX tag for Kconfig, rc32434_wdt and rdc321x_wdt
> v4:
>   - Drop coh901327_wdt since it allready is a pending patch
> v3:
>   - Keep license text for ebc-c384_wdt
> v2:
>   - Put back removed copyright texts for meson_gxbb_wdt and coh901327_wdt
>   - Change to BSD-3-Clause for meson_gxbb_wdt
> v1: Please have an extra look at meson_gxbb_wdt.c
> 
>  drivers/watchdog/ar7_wdt.c| 14 +--
>  drivers/watchdog/at91rm9200_wdt.c |  5 +---
>  drivers/watchdog/at91sam9_wdt.c   |  5 +---
>  drivers/watchdog/at91sam9_wdt.h   |  5 +---
>  drivers/watchdog/bcm2835_wdt.c|  5 +---
>  drivers/watchdog/bcm47xx_wdt.c|  5 +---
>  drivers/watchdog/bcm63xx_wdt.c|  5 +---
>  drivers/watchdog/bcm7038_wdt.c| 12 ++
>  drivers/watchdog/bcm_kona_wdt.c   |  9 +--
>  drivers/watchdog/cadence_wdt.c|  5 +---
>  drivers/watchdog/da9052_wdt.c |  6 +
>  drivers/watchdog/da9055_wdt.c |  6 +
>  drivers/watchdog/da9062_wdt.c | 10 +---
>  drivers/watchdog/da9063_wdt.c |  5 +---
>  drivers/watchdog/digicolor_wdt.c  |  5 +---
>  drivers/watchdog/ebc-c384_wdt.c   |  1 +
>  drivers/watchdog/mei_wdt.c| 12 ++
>  drivers/watchdog/mena21_wdt.c |  4 +---
>  drivers/watchdog/meson_gxbb_wdt.c | 50 
> +--
>  drivers/watchdog/mtk_wdt.c| 11 +
>  drivers/watchdog/mtx-1_wdt.c  | 11 +
>  drivers/watchdog/of_xilinx_wdt.c  |  8 ++-
>  drivers/watchdog/st_lpc_wdt.c |  6 +
>  drivers/watchdog/tangox_wdt.c |  6 +
>  drivers/watchdog/tegra_wdt.c  | 10 +---
>  

Re: [v6] watchdog: add SPDX identifiers for watchdog subsystem

[Guenter Roeck]

On Fri, Mar 16, 2018 at 04:14:11PM +0100, Marcus Folkesson wrote:
> - Add SPDX identifier
> - Remove boiler plate license text
> - If MODULE_LICENSE and boiler plate does not match, go for boiler plate
>   license
> 
> Signed-off-by: Marcus Folkesson 
> Acked-by: Adam Thomson 
> Acked-by: Baruch Siach 
> Acked-by: Charles Keepax 
> Acked-by: Keiji Hayashibara 
> Acked-by: Johannes Thumshirn 
> Acked-by: Florian Fainelli 
> Acked-by: Mans Rullgard 
> Acked-by: Matthias Brugger 
> Acked-by: Michal Simek 
> Acked-by: Neil Armstrong 
> Acked-by: Nicolas Ferre 
> Acked-by: Thierry Reding 
> Acked-by: Tomas Winkler 
> Acked-by: Patrice Chotard 
> Acked-by: William Breathitt Gray 
> Reviewed-by: Eric Anholt 

Reviewed-by: Guenter Roeck 

> ---
> 
> Notes:
> v6:
>   - Only include those drivers that has been acked-by.
> 
> drivers/watchdog/da9052_wdt.c
> drivers/watchdog/da9055_wdt.c
> drivers/watchdog/da9062_wdt.c
> drivers/watchdog/da9063_wdt.c
> Acked-by: Adam Thomson 
> 
> drivers/watchdog/digicolor_wdt.c
> Acked-by: Baruch Siach 
> 
> drivers/watchdog/wm831x_wdt.c
> drivers/watchdog/wm8350_wdt.c
> Acked-by: Charles Keepax 
> 
> drivers/watchdog/ar7_wdt.c
> drivers/watchdog/bcm2835_wdt.c
> drivers/watchdog/bcm47xx_wdt.c
> drivers/watchdog/bcm63xx_wdt.c
> drivers/watchdog/bcm7038_wdt.c
> drivers/watchdog/bcm_kona_wdt.c
> drivers/watchdog/mtx-1_wdt.c
> Acked-by: Florian Fainelli 
> 
> drivers/watchdog/uniphier_wdt.c
> Acked-by: Keiji Hayashibara 
> 
> drivers/watchdog/mena21_wdt
> Acked-by: Johannes Thumshirn 
> 
> drivers/watchdog/tangox_wdt.c
> Acked-by: Mans Rullgard 
> 
> drivers/watchdog/mtk_wdt.c
> Acked-by: Matthias Brugger 
> 
> drivers/watchdog/cadence_wdt.c
> drivers/watchdog/of_xilinx_wdt.c
> Acked-by: Michal Simek 
> 
> drivers/watchdog/meson_gxbb_wdt.c
> Acked-by: Neil Armstrong 
> 
> drivers/watchdog/at91rm9200_wdt.c
> drivers/watchdog/at91sam9_wdt.c
> drivers/watchdog/at91sam9_wdt.h
> drivers/watchdog/sama5d4_wdt.c
> Acked-by: Nicolas Ferre 
> 
> drivers/watchdog/tegra_wdt.c
> Acked-by: Thierry Reding 
> 
> drivers/watchdog/mei_wdt.c
> Acked-by: Tomas Winkler 
> 
> drivers/watchdog/st_lpc_wdt.c
> Acked-by: Patrice Chotard 
> 
> drivers/watchdog/ebc-c384_wdt.c
> Acked-by: William Breathitt Gray 
> 
> v5:
>   - Add missing SPDX tag for Kconfig, rc32434_wdt and rdc321x_wdt
> v4:
>   - Drop coh901327_wdt since it allready is a pending patch
> v3:
>   - Keep license text for ebc-c384_wdt
> v2:
>   - Put back removed copyright texts for meson_gxbb_wdt and coh901327_wdt
>   - Change to BSD-3-Clause for meson_gxbb_wdt
> v1: Please have an extra look at meson_gxbb_wdt.c
> 
>  drivers/watchdog/ar7_wdt.c| 14 +--
>  drivers/watchdog/at91rm9200_wdt.c |  5 +---
>  drivers/watchdog/at91sam9_wdt.c   |  5 +---
>  drivers/watchdog/at91sam9_wdt.h   |  5 +---
>  drivers/watchdog/bcm2835_wdt.c|  5 +---
>  drivers/watchdog/bcm47xx_wdt.c|  5 +---
>  drivers/watchdog/bcm63xx_wdt.c|  5 +---
>  drivers/watchdog/bcm7038_wdt.c| 12 ++
>  drivers/watchdog/bcm_kona_wdt.c   |  9 +--
>  drivers/watchdog/cadence_wdt.c|  5 +---
>  drivers/watchdog/da9052_wdt.c |  6 +
>  drivers/watchdog/da9055_wdt.c |  6 +
>  drivers/watchdog/da9062_wdt.c | 10 +---
>  drivers/watchdog/da9063_wdt.c |  5 +---
>  drivers/watchdog/digicolor_wdt.c  |  5 +---
>  drivers/watchdog/ebc-c384_wdt.c   |  1 +
>  drivers/watchdog/mei_wdt.c| 12 ++
>  drivers/watchdog/mena21_wdt.c |  4 +---
>  drivers/watchdog/meson_gxbb_wdt.c | 50 
> +--
>  drivers/watchdog/mtk_wdt.c| 11 +
>  drivers/watchdog/mtx-1_wdt.c  | 11 +
>  drivers/watchdog/of_xilinx_wdt.c  |  8 ++-
>  drivers/watchdog/st_lpc_wdt.c |  6 +
>  drivers/watchdog/tangox_wdt.c |  6 +
>  drivers/watchdog/tegra_wdt.c  | 10 +---
>  drivers/watchdog/uniphier_wdt.c   | 10 +---
>  drivers/watchdog/wm831x_wdt.c |  5 +---
>  drivers/watchdog/wm8350_wdt.c |  5 +---
>  28 files changed, 31 insertions(+), 210 deletions(-)
> 
> diff --git a/drivers/watchdog/ar7_wdt.c b/drivers/watchdog/ar7_wdt.c
> index 6d5ae251e309..ee1ab12ab04f 100644
> --- a/drivers/watchdog/ar7_wdt.c
> +++ b/drivers/watchdog/ar7_wdt.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0+
>  /*
>   * drivers/watchdog/ar7_wdt.c
>   *
> @@ -8,19 +9,6 @@
>   * National Semiconductor SCx200 Watchdog support
>   * Copyright (c) 2001,2002 Christer Weinigel 
>   *
> - * 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 

Re: [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred

[kbuild test robot]

Hi Christophe,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180316]
[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/Christophe-JAILLET/net-ethernet-arc-Fix-a-potential-memory-leak-if-an-optional-regulator-is-deferred/20180317-042849
config: openrisc-allyesconfig (attached as .config)
compiler: or1k-linux-gcc (GCC) 6.0.0 20160327 (experimental)
reproduce:
wget 
https://raw.githubusercontent.com/intel/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=openrisc 

All errors (new ones prefixed by >>):

   drivers/net/ethernet/arc/emac_rockchip.c: In function 'emac_rockchip_probe':
>> drivers/net/ethernet/arc/emac_rockchip.c:173:4: error: 'ret' undeclared 
>> (first use in this function)
   ret = -EPROBE_DEFER;
   ^~~
   drivers/net/ethernet/arc/emac_rockchip.c:173:4: note: each undeclared 
identifier is reported only once for each function it appears in

vim +/ret +173 drivers/net/ethernet/arc/emac_rockchip.c

   102  
   103  static int emac_rockchip_probe(struct platform_device *pdev)
   104  {
   105  struct device *dev = >dev;
   106  struct net_device *ndev;
   107  struct rockchip_priv_data *priv;
   108  const struct of_device_id *match;
   109  u32 data;
   110  int err, interface;
   111  
   112  if (!pdev->dev.of_node)
   113  return -ENODEV;
   114  
   115  ndev = alloc_etherdev(sizeof(struct rockchip_priv_data));
   116  if (!ndev)
   117  return -ENOMEM;
   118  platform_set_drvdata(pdev, ndev);
   119  SET_NETDEV_DEV(ndev, dev);
   120  
   121  priv = netdev_priv(ndev);
   122  priv->emac.drv_name = DRV_NAME;
   123  priv->emac.drv_version = DRV_VERSION;
   124  priv->emac.set_mac_speed = emac_rockchip_set_mac_speed;
   125  
   126  interface = of_get_phy_mode(dev->of_node);
   127  
   128  /* RK3036/RK3066/RK3188 SoCs only support RMII */
   129  if (interface != PHY_INTERFACE_MODE_RMII) {
   130  dev_err(dev, "unsupported phy interface mode %d\n", 
interface);
   131  err = -ENOTSUPP;
   132  goto out_netdev;
   133  }
   134  
   135  priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node,
   136  "rockchip,grf");
   137  if (IS_ERR(priv->grf)) {
   138  dev_err(dev, "failed to retrieve global register file 
(%ld)\n",
   139  PTR_ERR(priv->grf));
   140  err = PTR_ERR(priv->grf);
   141  goto out_netdev;
   142  }
   143  
   144  match = of_match_node(emac_rockchip_dt_ids, dev->of_node);
   145  priv->soc_data = match->data;
   146  
   147  priv->emac.clk = devm_clk_get(dev, "hclk");
   148  if (IS_ERR(priv->emac.clk)) {
   149  dev_err(dev, "failed to retrieve host clock (%ld)\n",
   150  PTR_ERR(priv->emac.clk));
   151  err = PTR_ERR(priv->emac.clk);
   152  goto out_netdev;
   153  }
   154  
   155  priv->refclk = devm_clk_get(dev, "macref");
   156  if (IS_ERR(priv->refclk)) {
   157  dev_err(dev, "failed to retrieve reference clock 
(%ld)\n",
   158  PTR_ERR(priv->refclk));
   159  err = PTR_ERR(priv->refclk);
   160  goto out_netdev;
   161  }
   162  
   163  err = clk_prepare_enable(priv->refclk);
   164  if (err) {
   165  dev_err(dev, "failed to enable reference clock (%d)\n", 
err);
   166  goto out_netdev;
   167  }
   168  
   169  /* Optional regulator for PHY */
   170  priv->regulator = devm_regulator_get_optional(dev, "phy");
   171  if (IS_ERR(priv->regulator)) {
   172  if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) {
 > 173  ret = -EPROBE_DEFER;
   174  goto out_clk_disable;
   175  }
   176  dev_err(dev, "no regulator found\n");
   177  priv->regulator = NULL;
   178  }
   179  
   180  if (priv->regulator) {
   181  err = regulator_enable(priv->regulator);
   182  if (err) {
   183  dev_err(dev, "failed to

Re: [PATCH] net: ethernet: arc: Fix a potential memory leak if an optional regulator is deferred

[kbuild test robot]

Hi Christophe,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180316]
[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/Christophe-JAILLET/net-ethernet-arc-Fix-a-potential-memory-leak-if-an-optional-regulator-is-deferred/20180317-042849
config: openrisc-allyesconfig (attached as .config)
compiler: or1k-linux-gcc (GCC) 6.0.0 20160327 (experimental)
reproduce:
wget 
https://raw.githubusercontent.com/intel/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=openrisc 

All errors (new ones prefixed by >>):

   drivers/net/ethernet/arc/emac_rockchip.c: In function 'emac_rockchip_probe':
>> drivers/net/ethernet/arc/emac_rockchip.c:173:4: error: 'ret' undeclared 
>> (first use in this function)
   ret = -EPROBE_DEFER;
   ^~~
   drivers/net/ethernet/arc/emac_rockchip.c:173:4: note: each undeclared 
identifier is reported only once for each function it appears in

vim +/ret +173 drivers/net/ethernet/arc/emac_rockchip.c

   102  
   103  static int emac_rockchip_probe(struct platform_device *pdev)
   104  {
   105  struct device *dev = >dev;
   106  struct net_device *ndev;
   107  struct rockchip_priv_data *priv;
   108  const struct of_device_id *match;
   109  u32 data;
   110  int err, interface;
   111  
   112  if (!pdev->dev.of_node)
   113  return -ENODEV;
   114  
   115  ndev = alloc_etherdev(sizeof(struct rockchip_priv_data));
   116  if (!ndev)
   117  return -ENOMEM;
   118  platform_set_drvdata(pdev, ndev);
   119  SET_NETDEV_DEV(ndev, dev);
   120  
   121  priv = netdev_priv(ndev);
   122  priv->emac.drv_name = DRV_NAME;
   123  priv->emac.drv_version = DRV_VERSION;
   124  priv->emac.set_mac_speed = emac_rockchip_set_mac_speed;
   125  
   126  interface = of_get_phy_mode(dev->of_node);
   127  
   128  /* RK3036/RK3066/RK3188 SoCs only support RMII */
   129  if (interface != PHY_INTERFACE_MODE_RMII) {
   130  dev_err(dev, "unsupported phy interface mode %d\n", 
interface);
   131  err = -ENOTSUPP;
   132  goto out_netdev;
   133  }
   134  
   135  priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node,
   136  "rockchip,grf");
   137  if (IS_ERR(priv->grf)) {
   138  dev_err(dev, "failed to retrieve global register file 
(%ld)\n",
   139  PTR_ERR(priv->grf));
   140  err = PTR_ERR(priv->grf);
   141  goto out_netdev;
   142  }
   143  
   144  match = of_match_node(emac_rockchip_dt_ids, dev->of_node);
   145  priv->soc_data = match->data;
   146  
   147  priv->emac.clk = devm_clk_get(dev, "hclk");
   148  if (IS_ERR(priv->emac.clk)) {
   149  dev_err(dev, "failed to retrieve host clock (%ld)\n",
   150  PTR_ERR(priv->emac.clk));
   151  err = PTR_ERR(priv->emac.clk);
   152  goto out_netdev;
   153  }
   154  
   155  priv->refclk = devm_clk_get(dev, "macref");
   156  if (IS_ERR(priv->refclk)) {
   157  dev_err(dev, "failed to retrieve reference clock 
(%ld)\n",
   158  PTR_ERR(priv->refclk));
   159  err = PTR_ERR(priv->refclk);
   160  goto out_netdev;
   161  }
   162  
   163  err = clk_prepare_enable(priv->refclk);
   164  if (err) {
   165  dev_err(dev, "failed to enable reference clock (%d)\n", 
err);
   166  goto out_netdev;
   167  }
   168  
   169  /* Optional regulator for PHY */
   170  priv->regulator = devm_regulator_get_optional(dev, "phy");
   171  if (IS_ERR(priv->regulator)) {
   172  if (PTR_ERR(priv->regulator) == -EPROBE_DEFER) {
 > 173  ret = -EPROBE_DEFER;
   174  goto out_clk_disable;
   175  }
   176  dev_err(dev, "no regulator found\n");
   177  priv->regulator = NULL;
   178  }
   179  
   180  if (priv->regulator) {
   181  err = regulator_enable(priv->regulator);
   182  if (err) {
   183  dev_err(dev, "failed to

Re: [PATCH v3] vsprintf: Prevent crash when dereferencing invalid pointers

[Sergey Senozhatsky]

On (03/16/18 09:55), Petr Mladek wrote:
[..]
> I am not sure if it is worth it. I think that we would catch 99% of
> problems by checking the first byte.
> 
> This patch was motivated by a code clean up rather than bug reports.

OK. Then I think we really need this "the patch is just good enough" line
in the commit message and a big comment in the source code.

Another idea (just an idea) - for some pointers we know the address range
we are going to access and can check the first and the last byte. E.g. for
UUID it's check_access(ptr) and check_access(ptr + len), and so on. Won't
work for string() in general case, tho.

-ss

Re: [PATCH v3] vsprintf: Prevent crash when dereferencing invalid pointers

[Sergey Senozhatsky]

On (03/16/18 09:55), Petr Mladek wrote:
[..]
> I am not sure if it is worth it. I think that we would catch 99% of
> problems by checking the first byte.
> 
> This patch was motivated by a code clean up rather than bug reports.

OK. Then I think we really need this "the patch is just good enough" line
in the commit message and a big comment in the source code.

Another idea (just an idea) - for some pointers we know the address range
we are going to access and can check the first and the last byte. E.g. for
UUID it's check_access(ptr) and check_access(ptr + len), and so on. Won't
work for string() in general case, tho.

-ss

Re: [PATCH 2/2] kprobe: fix: Add ftrace_ops_assist_func to kprobe blacklist

[Masami Hiramatsu]

On Sat, 17 Mar 2018 09:13:34 +0900
Masami Hiramatsu  wrote:

> On Fri, 16 Mar 2018 13:53:01 -0400 (EDT)
> Mathieu Desnoyers  wrote:
> 
> > - On Mar 16, 2018, at 12:48 PM, rostedt rost...@goodmis.org wrote:
> > 
> > > On Fri, 16 Mar 2018 12:41:34 -0400
> > > Steven Rostedt  wrote:
> > > 
> > >> Yes, kprobes are dangerous. I'm not saying it shouldn't be fixed, I'm
> > >> saying that I don't have time to fix it now, but would be happy to
> > >> accept patches if someone else does so.
> > > 
> > > And looking at what I replied before for the original patch. It would
> > > probably be a good idea to blacklist directories. Like we do with
> > > function tracing. We probably should black list both kernel/tracing and
> > > kernel/events from being probed.
> > > 
> > > Did this come up at plumbers? You were there too, I don't remember
> > > discussing it there.
> > 
> > I don't remember this coming up last Plumbers nor KS neither, given
> > that we were focused on other topics.
> > 
> > Would the general approach you envision be based on emitting all code
> > generated by compilation of all objects under kernel/tracing and
> > kernel/events into a specific "nokprobes" text section of the kernel ?
> > Perhaps we could create a specific linker scripts for those directories,
> > or do you have in mind a neater way to do this ?
> 
> .kprobes.text section still exists. As I pointed in previous mail, I don't
> think we have to put all those code into that section. But if you want,
> it is acceptable to have a kconfig which push most of those ftrace related
> code into .kprobes.text section.

Or, we can check it by ftrace_location_range() as below patch.

Note that as a side-effect, we can not trace functions in trace_kprobe.c
anymore, and this means it is hard to me to make a testcase of kprobe events.
It was the easiest (and maintainable) way to make test cases... sigh.

-
tracing: kprobes: Prohibit probing on notrace function

From: Masami Hiramatsu 

Prohibit kprobe-events probing on notrace function.
Since probing on the notrace function can cause recursive
event call. In most case those are just skipped, but
in some case it falls into infinit recursive call.

Signed-off-by: Masami Hiramatsu 
---
 kernel/trace/trace_kprobe.c |   23 +++
 1 file changed, 23 insertions(+)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 5ce9b8cf7be3..7404b012e51a 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -487,6 +487,23 @@ disable_trace_kprobe(struct trace_kprobe *tk, struct 
trace_event_file *file)
return ret;
 }
 
+#ifdef CONFIG_KPROBES_ON_FTRACE
+static bool within_notrace_func(struct trace_kprobe *tk)
+{
+   unsigned long offset, size, addr;
+
+   addr = kallsyms_lookup_name(trace_kprobe_symbol(tk));
+   addr += trace_kprobe_offset(tk);
+
+   if (!kallsyms_lookup_size_offset(addr, , ))
+   return true;/* Out of range. */
+
+   return !ftrace_location_range(addr - offset, addr - offset + size);
+}
+#else
+#define within_notrace_func(tk)(false)
+#endif
+
 /* Internal register function - just handle k*probes and flags */
 static int __register_trace_kprobe(struct trace_kprobe *tk)
 {
@@ -495,6 +512,12 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
if (trace_probe_is_registered(>tp))
return -EINVAL;
 
+   if (within_notrace_func(tk)) {
+   pr_warn("Could not probe notrace function %s\n",
+   trace_kprobe_symbol(tk));
+   return -EINVAL;
+   }
+
for (i = 0; i < tk->tp.nr_args; i++)
traceprobe_update_arg(>tp.args[i]);
 

-- 
Masami Hiramatsu 

Re: [PATCH 2/2] kprobe: fix: Add ftrace_ops_assist_func to kprobe blacklist

[Masami Hiramatsu]

On Sat, 17 Mar 2018 09:13:34 +0900
Masami Hiramatsu  wrote:

> On Fri, 16 Mar 2018 13:53:01 -0400 (EDT)
> Mathieu Desnoyers  wrote:
> 
> > - On Mar 16, 2018, at 12:48 PM, rostedt rost...@goodmis.org wrote:
> > 
> > > On Fri, 16 Mar 2018 12:41:34 -0400
> > > Steven Rostedt  wrote:
> > > 
> > >> Yes, kprobes are dangerous. I'm not saying it shouldn't be fixed, I'm
> > >> saying that I don't have time to fix it now, but would be happy to
> > >> accept patches if someone else does so.
> > > 
> > > And looking at what I replied before for the original patch. It would
> > > probably be a good idea to blacklist directories. Like we do with
> > > function tracing. We probably should black list both kernel/tracing and
> > > kernel/events from being probed.
> > > 
> > > Did this come up at plumbers? You were there too, I don't remember
> > > discussing it there.
> > 
> > I don't remember this coming up last Plumbers nor KS neither, given
> > that we were focused on other topics.
> > 
> > Would the general approach you envision be based on emitting all code
> > generated by compilation of all objects under kernel/tracing and
> > kernel/events into a specific "nokprobes" text section of the kernel ?
> > Perhaps we could create a specific linker scripts for those directories,
> > or do you have in mind a neater way to do this ?
> 
> .kprobes.text section still exists. As I pointed in previous mail, I don't
> think we have to put all those code into that section. But if you want,
> it is acceptable to have a kconfig which push most of those ftrace related
> code into .kprobes.text section.

Or, we can check it by ftrace_location_range() as below patch.

Note that as a side-effect, we can not trace functions in trace_kprobe.c
anymore, and this means it is hard to me to make a testcase of kprobe events.
It was the easiest (and maintainable) way to make test cases... sigh.

-
tracing: kprobes: Prohibit probing on notrace function

From: Masami Hiramatsu 

Prohibit kprobe-events probing on notrace function.
Since probing on the notrace function can cause recursive
event call. In most case those are just skipped, but
in some case it falls into infinit recursive call.

Signed-off-by: Masami Hiramatsu 
---
 kernel/trace/trace_kprobe.c |   23 +++
 1 file changed, 23 insertions(+)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 5ce9b8cf7be3..7404b012e51a 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -487,6 +487,23 @@ disable_trace_kprobe(struct trace_kprobe *tk, struct 
trace_event_file *file)
return ret;
 }
 
+#ifdef CONFIG_KPROBES_ON_FTRACE
+static bool within_notrace_func(struct trace_kprobe *tk)
+{
+   unsigned long offset, size, addr;
+
+   addr = kallsyms_lookup_name(trace_kprobe_symbol(tk));
+   addr += trace_kprobe_offset(tk);
+
+   if (!kallsyms_lookup_size_offset(addr, , ))
+   return true;/* Out of range. */
+
+   return !ftrace_location_range(addr - offset, addr - offset + size);
+}
+#else
+#define within_notrace_func(tk)(false)
+#endif
+
 /* Internal register function - just handle k*probes and flags */
 static int __register_trace_kprobe(struct trace_kprobe *tk)
 {
@@ -495,6 +512,12 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
if (trace_probe_is_registered(>tp))
return -EINVAL;
 
+   if (within_notrace_func(tk)) {
+   pr_warn("Could not probe notrace function %s\n",
+   trace_kprobe_symbol(tk));
+   return -EINVAL;
+   }
+
for (i = 0; i < tk->tp.nr_args; i++)
traceprobe_update_arg(>tp.args[i]);
 

-- 
Masami Hiramatsu 

[PATCH 02/14] mm/hmm: fix header file if/else/endif maze v2

[jglisse]

From: Jérôme Glisse 

The #if/#else/#endif for IS_ENABLED(CONFIG_HMM) were wrong. Because
of this after multiple include there was multiple definition of both
hmm_mm_init() and hmm_mm_destroy() leading to build failure if HMM
was enabled (CONFIG_HMM set).

Changed since v1:
  - Fix the maze when CONFIG_HMM is disabled not just when it is
enabled. This fix bot build failure.
  - Improved commit message.

Signed-off-by: Jérôme Glisse 
Acked-by: Balbir Singh 
Cc: sta...@vger.kernel.org
Cc: Andrew Morton 
Cc: Ralph Campbell 
Cc: John Hubbard 
Cc: Evgeny Baskakov 
---
 include/linux/hmm.h | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 325017ad9311..36dd21fe5caf 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -498,23 +498,16 @@ struct hmm_device {
 struct hmm_device *hmm_device_new(void *drvdata);
 void hmm_device_put(struct hmm_device *hmm_device);
 #endif /* CONFIG_DEVICE_PRIVATE || CONFIG_DEVICE_PUBLIC */
-#endif /* IS_ENABLED(CONFIG_HMM) */
 
 /* Below are for HMM internal use only! Not to be used by device driver! */
-#if IS_ENABLED(CONFIG_HMM_MIRROR)
 void hmm_mm_destroy(struct mm_struct *mm);
 
 static inline void hmm_mm_init(struct mm_struct *mm)
 {
mm->hmm = NULL;
 }
-#else /* IS_ENABLED(CONFIG_HMM_MIRROR) */
-static inline void hmm_mm_destroy(struct mm_struct *mm) {}
-static inline void hmm_mm_init(struct mm_struct *mm) {}
-#endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
-
-
 #else /* IS_ENABLED(CONFIG_HMM) */
 static inline void hmm_mm_destroy(struct mm_struct *mm) {}
 static inline void hmm_mm_init(struct mm_struct *mm) {}
+#endif /* IS_ENABLED(CONFIG_HMM) */
 #endif /* LINUX_HMM_H */
-- 
2.14.3

Re: [PATCH v3 1/5] tpm: fix intermittent failure with self tests

[James Bottomley]

On Mon, 2018-03-05 at 18:56 +0200, Jarkko Sakkinen wrote:
> index 9e80a953d693..1adb976a2e37 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -537,14 +537,26 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip,
> struct tpm_space *space,
>    const char *desc)
>  {
>   const struct tpm_output_header *header = buf;
> + unsigned int delay_msec = TPM2_DURATION_SHORT;
>   int err;
>   ssize_t len;
>  
> - len = tpm_transmit(chip, space, (u8 *)buf, bufsiz, flags);
> - if (len <  0)
> - return len;
> + for (;;) {
> + len = tpm_transmit(chip, space, (u8 *)buf, bufsiz,
> flags);
> + if (len <  0)
> + return len;
> + err = be32_to_cpu(header->return_code);
> + if (err != TPM2_RC_TESTING)
> + break;
> +
> + delay_msec *= 2;
> + if (delay_msec > TPM2_DURATION_LONG) {
> + dev_err(>dev, "the self test is still
> running\n");
> + break;
> + }
> + tpm_msleep(delay_msec);
> + }

It turns out this bit is wrong ... I just discovered it testing the
RC_RETRY code.  You can't feed the buf back to tpm_transmit because the
header has already been changed to give you back the return code.  To
make this work, you have to save the header and handle area and restore
it before the command is resent.

I think the best solution for this hunk of code is to merge it with the
retry code.

James

[PATCH 02/14] mm/hmm: fix header file if/else/endif maze v2

[jglisse]

From: Jérôme Glisse 

The #if/#else/#endif for IS_ENABLED(CONFIG_HMM) were wrong. Because
of this after multiple include there was multiple definition of both
hmm_mm_init() and hmm_mm_destroy() leading to build failure if HMM
was enabled (CONFIG_HMM set).

Changed since v1:
  - Fix the maze when CONFIG_HMM is disabled not just when it is
enabled. This fix bot build failure.
  - Improved commit message.

Signed-off-by: Jérôme Glisse 
Acked-by: Balbir Singh 
Cc: sta...@vger.kernel.org
Cc: Andrew Morton 
Cc: Ralph Campbell 
Cc: John Hubbard 
Cc: Evgeny Baskakov 
---
 include/linux/hmm.h | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 325017ad9311..36dd21fe5caf 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -498,23 +498,16 @@ struct hmm_device {
 struct hmm_device *hmm_device_new(void *drvdata);
 void hmm_device_put(struct hmm_device *hmm_device);
 #endif /* CONFIG_DEVICE_PRIVATE || CONFIG_DEVICE_PUBLIC */
-#endif /* IS_ENABLED(CONFIG_HMM) */
 
 /* Below are for HMM internal use only! Not to be used by device driver! */
-#if IS_ENABLED(CONFIG_HMM_MIRROR)
 void hmm_mm_destroy(struct mm_struct *mm);
 
 static inline void hmm_mm_init(struct mm_struct *mm)
 {
mm->hmm = NULL;
 }
-#else /* IS_ENABLED(CONFIG_HMM_MIRROR) */
-static inline void hmm_mm_destroy(struct mm_struct *mm) {}
-static inline void hmm_mm_init(struct mm_struct *mm) {}
-#endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
-
-
 #else /* IS_ENABLED(CONFIG_HMM) */
 static inline void hmm_mm_destroy(struct mm_struct *mm) {}
 static inline void hmm_mm_init(struct mm_struct *mm) {}
+#endif /* IS_ENABLED(CONFIG_HMM) */
 #endif /* LINUX_HMM_H */
-- 
2.14.3

Re: [PATCH v3 1/5] tpm: fix intermittent failure with self tests

[James Bottomley]

On Mon, 2018-03-05 at 18:56 +0200, Jarkko Sakkinen wrote:
> index 9e80a953d693..1adb976a2e37 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -537,14 +537,26 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip,
> struct tpm_space *space,
>    const char *desc)
>  {
>   const struct tpm_output_header *header = buf;
> + unsigned int delay_msec = TPM2_DURATION_SHORT;
>   int err;
>   ssize_t len;
>  
> - len = tpm_transmit(chip, space, (u8 *)buf, bufsiz, flags);
> - if (len <  0)
> - return len;
> + for (;;) {
> + len = tpm_transmit(chip, space, (u8 *)buf, bufsiz,
> flags);
> + if (len <  0)
> + return len;
> + err = be32_to_cpu(header->return_code);
> + if (err != TPM2_RC_TESTING)
> + break;
> +
> + delay_msec *= 2;
> + if (delay_msec > TPM2_DURATION_LONG) {
> + dev_err(>dev, "the self test is still
> running\n");
> + break;
> + }
> + tpm_msleep(delay_msec);
> + }

It turns out this bit is wrong ... I just discovered it testing the
RC_RETRY code.  You can't feed the buf back to tpm_transmit because the
header has already been changed to give you back the return code.  To
make this work, you have to save the header and handle area and restore
it before the command is resent.

I think the best solution for this hunk of code is to merge it with the
retry code.

James

Re: [PATCH] acpi, numa: fix pxm to online numa node associations

[Dan Williams]

[ adding Toshi's correct address ]

On Thu, Mar 15, 2018 at 8:08 PM, Dan Williams  wrote:
> Commit 99759869faf1 "acpi: Add acpi_map_pxm_to_online_node()" added
> support for mapping a given proximity to its nearest, by SLIT distance,
> online node. However, it sometimes returns unexpected results due to the
> fact that it switches from comparing the PXM node to the last node that
> was closer than the current max.
>
> for_each_online_node(n) {
> dist = node_distance(node, n);
> if (dist < min_dist) {
> min_dist = dist;
> node = n;   < from this point we're using the
>   wrong node for node_distance()
>
>
> Fixes: 99759869faf1 ("acpi: Add acpi_map_pxm_to_online_node()")
> Cc: 
> Cc: Toshi Kani 
> Cc: Rafael J. Wysocki >
> Signed-off-by: Dan Williams 
> ---
> Rafael, I can take this through the nvdimm tree with your ack. I have a
> few other nvdimm fixes pending for 4.16.
>
>  drivers/acpi/numa.c |   10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
> index 8ccaae3550d2..85167603b9c9 100644
> --- a/drivers/acpi/numa.c
> +++ b/drivers/acpi/numa.c
> @@ -103,25 +103,27 @@ int acpi_map_pxm_to_node(int pxm)
>   */
>  int acpi_map_pxm_to_online_node(int pxm)
>  {
> -   int node, n, dist, min_dist;
> +   int node, min_node;
>
> node = acpi_map_pxm_to_node(pxm);
>
> if (node == NUMA_NO_NODE)
> node = 0;
>
> +   min_node = node;
> if (!node_online(node)) {
> -   min_dist = INT_MAX;
> +   int min_dist = INT_MAX, dist, n;
> +
> for_each_online_node(n) {
> dist = node_distance(node, n);
> if (dist < min_dist) {
> min_dist = dist;
> -   node = n;
> +   min_node = n;
> }
> }
> }
>
> -   return node;
> +   return min_node;
>  }
>  EXPORT_SYMBOL(acpi_map_pxm_to_online_node);
>
>

Re: [PATCH] acpi, numa: fix pxm to online numa node associations

[Dan Williams]

[ adding Toshi's correct address ]

On Thu, Mar 15, 2018 at 8:08 PM, Dan Williams  wrote:
> Commit 99759869faf1 "acpi: Add acpi_map_pxm_to_online_node()" added
> support for mapping a given proximity to its nearest, by SLIT distance,
> online node. However, it sometimes returns unexpected results due to the
> fact that it switches from comparing the PXM node to the last node that
> was closer than the current max.
>
> for_each_online_node(n) {
> dist = node_distance(node, n);
> if (dist < min_dist) {
> min_dist = dist;
> node = n;   < from this point we're using the
>   wrong node for node_distance()
>
>
> Fixes: 99759869faf1 ("acpi: Add acpi_map_pxm_to_online_node()")
> Cc: 
> Cc: Toshi Kani 
> Cc: Rafael J. Wysocki >
> Signed-off-by: Dan Williams 
> ---
> Rafael, I can take this through the nvdimm tree with your ack. I have a
> few other nvdimm fixes pending for 4.16.
>
>  drivers/acpi/numa.c |   10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/numa.c b/drivers/acpi/numa.c
> index 8ccaae3550d2..85167603b9c9 100644
> --- a/drivers/acpi/numa.c
> +++ b/drivers/acpi/numa.c
> @@ -103,25 +103,27 @@ int acpi_map_pxm_to_node(int pxm)
>   */
>  int acpi_map_pxm_to_online_node(int pxm)
>  {
> -   int node, n, dist, min_dist;
> +   int node, min_node;
>
> node = acpi_map_pxm_to_node(pxm);
>
> if (node == NUMA_NO_NODE)
> node = 0;
>
> +   min_node = node;
> if (!node_online(node)) {
> -   min_dist = INT_MAX;
> +   int min_dist = INT_MAX, dist, n;
> +
> for_each_online_node(n) {
> dist = node_distance(node, n);
> if (dist < min_dist) {
> min_dist = dist;
> -   node = n;
> +   min_node = n;
> }
> }
> }
>
> -   return node;
> +   return min_node;
>  }
>  EXPORT_SYMBOL(acpi_map_pxm_to_online_node);
>
>

Re: [PATCH v5 1/9] sysctl: Add flags to support min/max range clamping

[Luis R. Rodriguez]

On Fri, Mar 16, 2018 at 02:13:42PM -0400, Waiman Long wrote:
> When the CTL_FLAGS_CLAMP_RANGE flag is set in the ctl_table
> entry, any update from the userspace will be clamped to the given
> range without error if either the proc_dointvec_minmax() or the
> proc_douintvec_minmax() handlers is used.

I don't get it.  Why define a generic range flag when we can be mores specific 
and
you do that in your next patch. What's the point of this flag then?

  Luis

Re: [PATCH v5 1/9] sysctl: Add flags to support min/max range clamping

[Luis R. Rodriguez]

On Fri, Mar 16, 2018 at 02:13:42PM -0400, Waiman Long wrote:
> When the CTL_FLAGS_CLAMP_RANGE flag is set in the ctl_table
> entry, any update from the userspace will be clamped to the given
> range without error if either the proc_dointvec_minmax() or the
> proc_douintvec_minmax() handlers is used.

I don't get it.  Why define a generic range flag when we can be mores specific 
and
you do that in your next patch. What's the point of this flag then?

  Luis

[PATCH 1/2] regulator: add QCOM RPMh regulator driver

[David Collins]

Add the QCOM RPMh regulator driver to manage PMIC regulators
which are controlled via RPMh on some Qualcomm Technologies, Inc.
SoCs.  RPMh is a hardware block which contains several
accelerators which are used to manage various hardware resources
that are shared between the processors of the SoC.  The final
hardware state of a regulator is determined within RPMh by
performing max aggregation of the requests made by all of the
processors.

Add support for PMIC regulator control via the voltage regulator
manager (VRM) and oscillator buffer (XOB) RPMh accelerators.  VRM
supports manipulation of enable state, voltage, mode, and
headroom voltage.  XOB supports manipulation of enable state.

Signed-off-by: David Collins 
---
 drivers/regulator/Kconfig  |9 +
 drivers/regulator/Makefile |1 +
 drivers/regulator/qcom_rpmh-regulator.c| 1124 
 .../dt-bindings/regulator/qcom,rpmh-regulator.h|   40 +
 4 files changed, 1174 insertions(+)
 create mode 100644 drivers/regulator/qcom_rpmh-regulator.c
 create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 097f617..e0ecd0a 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -671,6 +671,15 @@ config REGULATOR_QCOM_RPM
  Qualcomm RPM as a module. The module will be named
  "qcom_rpm-regulator".
 
+config REGULATOR_QCOM_RPMH
+   tristate "Qualcomm Technologies, Inc. RPMh regulator driver"
+   depends on (QCOM_RPMH && QCOM_COMMAND_DB && OF) || COMPILE_TEST
+   help
+ This driver supports control of PMIC regulators via the RPMh hardware
+ block found on Qualcomm Technologies Inc. SoCs.  RPMh regulator
+ control allows for voting on regulator state between multiple
+ processors within the SoC.
+
 config REGULATOR_QCOM_SMD_RPM
tristate "Qualcomm SMD based RPM regulator driver"
depends on QCOM_SMD_RPM
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 590674f..c2274dd 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_REGULATOR_MT6323)+= mt6323-regulator.o
 obj-$(CONFIG_REGULATOR_MT6380) += mt6380-regulator.o
 obj-$(CONFIG_REGULATOR_MT6397) += mt6397-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
+obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom_rpmh-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o
 obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
diff --git a/drivers/regulator/qcom_rpmh-regulator.c 
b/drivers/regulator/qcom_rpmh-regulator.c
new file mode 100644
index 000..808f949
--- /dev/null
+++ b/drivers/regulator/qcom_rpmh-regulator.c
@@ -0,0 +1,1124 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved. */
+
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * enum rpmh_regulator_type - supported RPMh accelerator types
+ * %RPMH_REGULATOR_TYPE_VRM:   RPMh VRM accelerator which supports voting on
+ * enable, voltage, mode, and headroom voltage of
+ * LDO, SMPS, VS, and BOB type PMIC regulators.
+ * %RPMH_REGULATOR_TYPE_XOB:   RPMh XOB accelerator which supports voting on
+ * the enable state of PMIC regulators.
+ */
+enum rpmh_regulator_type {
+   RPMH_REGULATOR_TYPE_VRM,
+   RPMH_REGULATOR_TYPE_XOB,
+};
+
+/* Min and max limits of VRM resource request parameters */
+#define RPMH_VRM_MIN_UV0
+#define RPMH_VRM_MAX_UV8191000
+
+#define RPMH_VRM_HEADROOM_MIN_UV   0
+#define RPMH_VRM_HEADROOM_MAX_UV   511000
+
+#define RPMH_VRM_MODE_MIN  0
+#define RPMH_VRM_MODE_MAX  7
+
+/* Register offsets: */
+#define RPMH_REGULATOR_REG_VRM_VOLTAGE 0x0
+#define RPMH_REGULATOR_REG_ENABLE  0x4
+#define RPMH_REGULATOR_REG_VRM_MODE0x8
+#define RPMH_REGULATOR_REG_VRM_HEADROOM0xC
+
+/* Enable register values: */
+#define RPMH_REGULATOR_DISABLE 0x0
+#define RPMH_REGULATOR_ENABLE  0x1
+
+/* Number of unique hardware modes supported: */
+#define RPMH_REGULATOR_MODE_COUNT  5
+
+/**
+ * struct rpmh_regulator_mode - RPMh VRM mode attributes
+ * @pmic_mode: Raw PMIC mode value written into VRM mode voting
+ * register (i.e. RPMH_REGULATOR_MODE_*)
+ * @framework_mode:Regulator framework mode value
+ * (i.e. REGULATOR_MODE_*)
+ * @min_load_ua:   The 

[PATCH 1/2] regulator: add QCOM RPMh regulator driver

[David Collins]

Add the QCOM RPMh regulator driver to manage PMIC regulators
which are controlled via RPMh on some Qualcomm Technologies, Inc.
SoCs.  RPMh is a hardware block which contains several
accelerators which are used to manage various hardware resources
that are shared between the processors of the SoC.  The final
hardware state of a regulator is determined within RPMh by
performing max aggregation of the requests made by all of the
processors.

Add support for PMIC regulator control via the voltage regulator
manager (VRM) and oscillator buffer (XOB) RPMh accelerators.  VRM
supports manipulation of enable state, voltage, mode, and
headroom voltage.  XOB supports manipulation of enable state.

Signed-off-by: David Collins 
---
 drivers/regulator/Kconfig  |9 +
 drivers/regulator/Makefile |1 +
 drivers/regulator/qcom_rpmh-regulator.c| 1124 
 .../dt-bindings/regulator/qcom,rpmh-regulator.h|   40 +
 4 files changed, 1174 insertions(+)
 create mode 100644 drivers/regulator/qcom_rpmh-regulator.c
 create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 097f617..e0ecd0a 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -671,6 +671,15 @@ config REGULATOR_QCOM_RPM
  Qualcomm RPM as a module. The module will be named
  "qcom_rpm-regulator".
 
+config REGULATOR_QCOM_RPMH
+   tristate "Qualcomm Technologies, Inc. RPMh regulator driver"
+   depends on (QCOM_RPMH && QCOM_COMMAND_DB && OF) || COMPILE_TEST
+   help
+ This driver supports control of PMIC regulators via the RPMh hardware
+ block found on Qualcomm Technologies Inc. SoCs.  RPMh regulator
+ control allows for voting on regulator state between multiple
+ processors within the SoC.
+
 config REGULATOR_QCOM_SMD_RPM
tristate "Qualcomm SMD based RPM regulator driver"
depends on QCOM_SMD_RPM
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 590674f..c2274dd 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_REGULATOR_MT6323)+= mt6323-regulator.o
 obj-$(CONFIG_REGULATOR_MT6380) += mt6380-regulator.o
 obj-$(CONFIG_REGULATOR_MT6397) += mt6397-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
+obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom_rpmh-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o
 obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
diff --git a/drivers/regulator/qcom_rpmh-regulator.c 
b/drivers/regulator/qcom_rpmh-regulator.c
new file mode 100644
index 000..808f949
--- /dev/null
+++ b/drivers/regulator/qcom_rpmh-regulator.c
@@ -0,0 +1,1124 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved. */
+
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * enum rpmh_regulator_type - supported RPMh accelerator types
+ * %RPMH_REGULATOR_TYPE_VRM:   RPMh VRM accelerator which supports voting on
+ * enable, voltage, mode, and headroom voltage of
+ * LDO, SMPS, VS, and BOB type PMIC regulators.
+ * %RPMH_REGULATOR_TYPE_XOB:   RPMh XOB accelerator which supports voting on
+ * the enable state of PMIC regulators.
+ */
+enum rpmh_regulator_type {
+   RPMH_REGULATOR_TYPE_VRM,
+   RPMH_REGULATOR_TYPE_XOB,
+};
+
+/* Min and max limits of VRM resource request parameters */
+#define RPMH_VRM_MIN_UV0
+#define RPMH_VRM_MAX_UV8191000
+
+#define RPMH_VRM_HEADROOM_MIN_UV   0
+#define RPMH_VRM_HEADROOM_MAX_UV   511000
+
+#define RPMH_VRM_MODE_MIN  0
+#define RPMH_VRM_MODE_MAX  7
+
+/* Register offsets: */
+#define RPMH_REGULATOR_REG_VRM_VOLTAGE 0x0
+#define RPMH_REGULATOR_REG_ENABLE  0x4
+#define RPMH_REGULATOR_REG_VRM_MODE0x8
+#define RPMH_REGULATOR_REG_VRM_HEADROOM0xC
+
+/* Enable register values: */
+#define RPMH_REGULATOR_DISABLE 0x0
+#define RPMH_REGULATOR_ENABLE  0x1
+
+/* Number of unique hardware modes supported: */
+#define RPMH_REGULATOR_MODE_COUNT  5
+
+/**
+ * struct rpmh_regulator_mode - RPMh VRM mode attributes
+ * @pmic_mode: Raw PMIC mode value written into VRM mode voting
+ * register (i.e. RPMH_REGULATOR_MODE_*)
+ * @framework_mode:Regulator framework mode value
+ * (i.e. REGULATOR_MODE_*)
+ * @min_load_ua:   The minimum load current in 

[PATCH 0/2] regulator: add QCOM RPMh regulator driver

[David Collins]

Hello,

This patch series adds a driver and device tree binding documentation for
PMIC regulator control via Resource Power Manager-hardened (RPMh) on some
Qualcomm Technologies, Inc. SoCs such as SDM845.  RPMh is a hardware block
which contains several accelerators which are used to manage various
hardware resources that are shared between the processors of the SoC.  The
final hardware state of a regulator is determined within RPMh by performing
max aggregation of the requests made by all of the processors.

The RPMh regulator driver depends upon the RPMh driver [1] and command DB
driver [2] which are both still undergoing review.

Thanks,
David

[1]: https://lkml.org/lkml/2018/3/9/979
[2]: https://lkml.org/lkml/2018/3/14/787

David Collins (2):
  regulator: add QCOM RPMh regulator driver
  dt-bindings: regulator: add QCOM RPMh regulator bindings

 .../bindings/regulator/qcom,rpmh-regulator.txt |  246 +
 drivers/regulator/Kconfig  |9 +
 drivers/regulator/Makefile |1 +
 drivers/regulator/qcom_rpmh-regulator.c| 1124 
 .../dt-bindings/regulator/qcom,rpmh-regulator.h|   40 +
 5 files changed, 1420 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
 create mode 100644 drivers/regulator/qcom_rpmh-regulator.c
 create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h

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

[PATCH 2/2] dt-bindings: regulator: add QCOM RPMh regulator bindings

[David Collins]

Introduce bindings for RPMh regulator devices found on some
Qualcomm Technlogies, Inc. SoCs.  These devices allow a given
processor within the SoC to make PMIC regulator requests which
are aggregated within the RPMh hardware block along with requests
from other processors in the SoC to determine the final PMIC
regulator hardware state.

Signed-off-by: David Collins 
---
 .../bindings/regulator/qcom,rpmh-regulator.txt | 246 +
 1 file changed, 246 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt

diff --git 
a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt 
b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
new file mode 100644
index 000..2d86306
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
@@ -0,0 +1,246 @@
+Qualcomm Technologies, Inc. RPMh Regulators
+
+rpmh-regulator devices support PMIC regulator management via the VRM and XOB
+RPMh accelerators.  The APPS processor communicates with these hardware blocks
+via an RSC using command packets.  The VRM allows changing four parameters for 
a
+given regulator: enable state, output voltage, operating mode, and minimum
+headroom voltage.  The XOB allows changing only a single parameter for a given
+regulator: its enable state.
+
+===
+Required Node Structure
+===
+
+RPMh regulators must be described in two levels of device nodes.  The first
+level describes the PMIC containing the regulators and must reside within an
+RPMh device node.  The second level describes each regulator within the PMIC
+which is to be used on the board.  Each of these regulators maps to a single
+RPMh resource.
+
+The names used for regulator nodes must match those supported by a given PMIC.
+Supported regulator node names:
+   PM8998: smps1 - smps13, ldo1 - ldo28, lvs1 - lvs2
+   PMI8998:bob
+   PM8005: smps1 - smps4
+
+==
+First Level Nodes - PMIC
+==
+
+- compatible
+   Usage:  required
+   Value type: 
+   Definition: Must be one of: "qcom,pm8998-rpmh-regulators",
+   "qcom,pmi8998-rpmh-regulators" or
+   "qcom,pm8005-rpmh-regulators".
+
+- qcom,pmic-id
+   Usage:  required
+   Value type: 
+   Definition: RPMh resource name suffix used for the regulators found on
+   this PMIC.  Typical values: "a", "b", "c", "d", "e", "f".
+
+- vdd_s1-supply
+- vdd_s2-supply
+- vdd_s3-supply
+- vdd_s4-supply
+- vdd_s5-supply
+- vdd_s6-supply
+- vdd_s7-supply
+- vdd_s8-supply
+- vdd_s9-supply
+- vdd_s10-supply
+- vdd_s11-supply
+- vdd_s12-supply
+- vdd_s13-supply
+- vdd_l1_l27-supply
+- vdd_l2_l8_l17-supply
+- vdd_l3_l11-supply
+- vdd_l4_l5-supply
+- vdd_l6-supply
+- vdd_l7_l12_l14_l15-supply
+- vdd_l9-supply
+- vdd_l10_l23_l25-supply
+- vdd_l13_l19_l21-supply
+- vdd_l16_l28-supply
+- vdd_l18_l22-supply
+- vdd_l20_l24-supply
+- vdd_l26-supply
+- vdd_lvs1_lvs2-supply
+- vdd_lvs1_lvs2-supply
+   Usage:  optional (PM8998 only)
+   Value type: 
+   Definition: phandle of the parent supply regulator of one or more of the
+   regulators for this PMIC.
+
+- vdd_bob-supply
+   Usage:  optional (PMI8998 only)
+   Value type: 
+   Definition: phandle of the parent supply regulator of one or more of the
+   regulators for this PMIC.
+
+- vdd_s1-supply
+- vdd_s2-supply
+- vdd_s3-supply
+- vdd_s4-supply
+   Usage:  optional (PM8005 only)
+   Value type: 
+   Definition: phandle of the parent supply regulator of one or more of the
+   regulators for this PMIC.
+
+=
+Second Level Nodes - Regulators
+=
+
+- regulator-name
+   Usage:  optional
+   Value type: 
+   Definition: Specifies the name for this RPMh regulator.  If not
+   specified, then the regulator's name is equal to its subnode
+   name.
+
+- regulator-min-microvolt
+   Usage:  required
+   Value type: 
+   Definition: For VRM resources, this is the minimum supported voltage in
+   microvolts.  For XOB resources, this is the fixed output
+   voltage.
+
+- regulator-max-microvolt
+   Usage:  required
+   Value type: 
+   Definition: For VRM resources, this is the maximum supported voltage in
+   microvolts.  For XOB resources, this is the fixed output
+   voltage.
+
+- qcom,regulator-initial-voltage
+   Usage:  optional; VRM regulators only
+   Value type: 
+   Definition: Specifies the initial voltage in microvolts to request for a
+   VRM regulator.  Supported values are 0 to 8191000.
+
+- 

[PATCH 0/2] regulator: add QCOM RPMh regulator driver

[David Collins]

Hello,

This patch series adds a driver and device tree binding documentation for
PMIC regulator control via Resource Power Manager-hardened (RPMh) on some
Qualcomm Technologies, Inc. SoCs such as SDM845.  RPMh is a hardware block
which contains several accelerators which are used to manage various
hardware resources that are shared between the processors of the SoC.  The
final hardware state of a regulator is determined within RPMh by performing
max aggregation of the requests made by all of the processors.

The RPMh regulator driver depends upon the RPMh driver [1] and command DB
driver [2] which are both still undergoing review.

Thanks,
David

[1]: https://lkml.org/lkml/2018/3/9/979
[2]: https://lkml.org/lkml/2018/3/14/787

David Collins (2):
  regulator: add QCOM RPMh regulator driver
  dt-bindings: regulator: add QCOM RPMh regulator bindings

 .../bindings/regulator/qcom,rpmh-regulator.txt |  246 +
 drivers/regulator/Kconfig  |9 +
 drivers/regulator/Makefile |1 +
 drivers/regulator/qcom_rpmh-regulator.c| 1124 
 .../dt-bindings/regulator/qcom,rpmh-regulator.h|   40 +
 5 files changed, 1420 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
 create mode 100644 drivers/regulator/qcom_rpmh-regulator.c
 create mode 100644 include/dt-bindings/regulator/qcom,rpmh-regulator.h

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

[PATCH 2/2] dt-bindings: regulator: add QCOM RPMh regulator bindings

[David Collins]

Introduce bindings for RPMh regulator devices found on some
Qualcomm Technlogies, Inc. SoCs.  These devices allow a given
processor within the SoC to make PMIC regulator requests which
are aggregated within the RPMh hardware block along with requests
from other processors in the SoC to determine the final PMIC
regulator hardware state.

Signed-off-by: David Collins 
---
 .../bindings/regulator/qcom,rpmh-regulator.txt | 246 +
 1 file changed, 246 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt

diff --git 
a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt 
b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
new file mode 100644
index 000..2d86306
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.txt
@@ -0,0 +1,246 @@
+Qualcomm Technologies, Inc. RPMh Regulators
+
+rpmh-regulator devices support PMIC regulator management via the VRM and XOB
+RPMh accelerators.  The APPS processor communicates with these hardware blocks
+via an RSC using command packets.  The VRM allows changing four parameters for 
a
+given regulator: enable state, output voltage, operating mode, and minimum
+headroom voltage.  The XOB allows changing only a single parameter for a given
+regulator: its enable state.
+
+===
+Required Node Structure
+===
+
+RPMh regulators must be described in two levels of device nodes.  The first
+level describes the PMIC containing the regulators and must reside within an
+RPMh device node.  The second level describes each regulator within the PMIC
+which is to be used on the board.  Each of these regulators maps to a single
+RPMh resource.
+
+The names used for regulator nodes must match those supported by a given PMIC.
+Supported regulator node names:
+   PM8998: smps1 - smps13, ldo1 - ldo28, lvs1 - lvs2
+   PMI8998:bob
+   PM8005: smps1 - smps4
+
+==
+First Level Nodes - PMIC
+==
+
+- compatible
+   Usage:  required
+   Value type: 
+   Definition: Must be one of: "qcom,pm8998-rpmh-regulators",
+   "qcom,pmi8998-rpmh-regulators" or
+   "qcom,pm8005-rpmh-regulators".
+
+- qcom,pmic-id
+   Usage:  required
+   Value type: 
+   Definition: RPMh resource name suffix used for the regulators found on
+   this PMIC.  Typical values: "a", "b", "c", "d", "e", "f".
+
+- vdd_s1-supply
+- vdd_s2-supply
+- vdd_s3-supply
+- vdd_s4-supply
+- vdd_s5-supply
+- vdd_s6-supply
+- vdd_s7-supply
+- vdd_s8-supply
+- vdd_s9-supply
+- vdd_s10-supply
+- vdd_s11-supply
+- vdd_s12-supply
+- vdd_s13-supply
+- vdd_l1_l27-supply
+- vdd_l2_l8_l17-supply
+- vdd_l3_l11-supply
+- vdd_l4_l5-supply
+- vdd_l6-supply
+- vdd_l7_l12_l14_l15-supply
+- vdd_l9-supply
+- vdd_l10_l23_l25-supply
+- vdd_l13_l19_l21-supply
+- vdd_l16_l28-supply
+- vdd_l18_l22-supply
+- vdd_l20_l24-supply
+- vdd_l26-supply
+- vdd_lvs1_lvs2-supply
+- vdd_lvs1_lvs2-supply
+   Usage:  optional (PM8998 only)
+   Value type: 
+   Definition: phandle of the parent supply regulator of one or more of the
+   regulators for this PMIC.
+
+- vdd_bob-supply
+   Usage:  optional (PMI8998 only)
+   Value type: 
+   Definition: phandle of the parent supply regulator of one or more of the
+   regulators for this PMIC.
+
+- vdd_s1-supply
+- vdd_s2-supply
+- vdd_s3-supply
+- vdd_s4-supply
+   Usage:  optional (PM8005 only)
+   Value type: 
+   Definition: phandle of the parent supply regulator of one or more of the
+   regulators for this PMIC.
+
+=
+Second Level Nodes - Regulators
+=
+
+- regulator-name
+   Usage:  optional
+   Value type: 
+   Definition: Specifies the name for this RPMh regulator.  If not
+   specified, then the regulator's name is equal to its subnode
+   name.
+
+- regulator-min-microvolt
+   Usage:  required
+   Value type: 
+   Definition: For VRM resources, this is the minimum supported voltage in
+   microvolts.  For XOB resources, this is the fixed output
+   voltage.
+
+- regulator-max-microvolt
+   Usage:  required
+   Value type: 
+   Definition: For VRM resources, this is the maximum supported voltage in
+   microvolts.  For XOB resources, this is the fixed output
+   voltage.
+
+- qcom,regulator-initial-voltage
+   Usage:  optional; VRM regulators only
+   Value type: 
+   Definition: Specifies the initial voltage in microvolts to request for a
+   VRM regulator.  Supported values are 0 to 8191000.
+
+- regulator-initial-mode
+   Usage:   

Re: [linux-sunxi] [PATCH v4 7/9] clk: sunxi-ng: add support for the Allwinner H6 CCU

[Jernej Škrabec]

Hi,

Dne petek, 16. marec 2018 ob 15:02:13 CET je Icenowy Zheng napisal(a):
> The Allwinner H6 SoC has a CCU which has been largely rearranged.
> 
> Add support for it in the sunxi-ng CCU framework.
> 
> Signed-off-by: Icenowy Zheng 
> Acked-by: Maxime Ripard 
> ---
> Changes in v4:
> - Extract the device tree binding document to another patch.
> 
> Changes in v3:
> - SPDX license idetifier fix.
> - Add some comments at initialization.
> 
> Changes in v2:
> - Exported APB1 bus clock for PIO.
> - Switch to SPDX license identifier.
> 
>  drivers/clk/sunxi-ng/Kconfig  |5 +
>  drivers/clk/sunxi-ng/Makefile |1 +
>  drivers/clk/sunxi-ng/ccu-sun50i-h6.c  | 1207
> + drivers/clk/sunxi-ng/ccu-sun50i-h6.h  |  
> 56 ++
>  include/dt-bindings/clock/sun50i-h6-ccu.h |  124 +++
>  include/dt-bindings/reset/sun50i-h6-ccu.h |   73 ++
>  6 files changed, 1466 insertions(+)
>  create mode 100644 drivers/clk/sunxi-ng/ccu-sun50i-h6.c
>  create mode 100644 drivers/clk/sunxi-ng/ccu-sun50i-h6.h
>  create mode 100644 include/dt-bindings/clock/sun50i-h6-ccu.h
>  create mode 100644 include/dt-bindings/reset/sun50i-h6-ccu.h
> 
> diff --git a/drivers/clk/sunxi-ng/Kconfig b/drivers/clk/sunxi-ng/Kconfig
> index 33168f94ee39..79dfd296c3d1 100644
> --- a/drivers/clk/sunxi-ng/Kconfig
> +++ b/drivers/clk/sunxi-ng/Kconfig
> @@ -11,6 +11,11 @@ config SUN50I_A64_CCU
>   default ARM64 && ARCH_SUNXI
>   depends on (ARM64 && ARCH_SUNXI) || COMPILE_TEST
> 
> +config SUN50I_H6_CCU
> + bool "Support for the Allwinner H6 CCU"
> + default ARM64 && ARCH_SUNXI
> + depends on (ARM64 && ARCH_SUNXI) || COMPILE_TEST
> +
>  config SUN4I_A10_CCU
>   bool "Support for the Allwinner A10/A20 CCU"
>   default MACH_SUN4I
> diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
> index 4141c3fe08ae..128a40ee5c5e 100644
> --- a/drivers/clk/sunxi-ng/Makefile
> +++ b/drivers/clk/sunxi-ng/Makefile
> @@ -22,6 +22,7 @@ lib-$(CONFIG_SUNXI_CCU) += ccu_mp.o
> 
>  # SoC support
>  obj-$(CONFIG_SUN50I_A64_CCU) += ccu-sun50i-a64.o
> +obj-$(CONFIG_SUN50I_H6_CCU)  += ccu-sun50i-h6.o
>  obj-$(CONFIG_SUN4I_A10_CCU)  += ccu-sun4i-a10.o
>  obj-$(CONFIG_SUN5I_CCU)  += ccu-sun5i.o
>  obj-$(CONFIG_SUN6I_A31_CCU)  += ccu-sun6i-a31.o
> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
> b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c new file mode 100644
> index ..d5eab49e6350
> --- /dev/null
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
> @@ -0,0 +1,1207 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2017 Icenowy Zheng 
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "ccu_common.h"
> +#include "ccu_reset.h"
> +
> +#include "ccu_div.h"
> +#include "ccu_gate.h"
> +#include "ccu_mp.h"
> +#include "ccu_mult.h"
> +#include "ccu_nk.h"
> +#include "ccu_nkm.h"
> +#include "ccu_nkmp.h"
> +#include "ccu_nm.h"
> +
> +#include "ccu-sun50i-h6.h"
> +
> +/*
> + * The CPU PLL is actually NP clock, with P being /1, /2 or /4. However
> + * P should only be used for output frequencies lower than 288 MHz.
> + *
> + * For now we can just model it as a multiplier clock, and force P to /1.
> + *
> + * The M factor is present in the register's description, but not in the
> + * frequency formula, and it's documented as "M is only used for backdoor
> + * testing", so it's not modelled and then force to 0.
> + */
> +#define SUN50I_H6_PLL_CPUX_REG   0x000
> +static struct ccu_mult pll_cpux_clk = {
> + .enable = BIT(31),
> + .lock   = BIT(28),
> + .mult   = _SUNXI_CCU_MULT_MIN(8, 8, 12),
> + .common = {
> + .reg= 0x000,
> + .hw.init= CLK_HW_INIT("pll-cpux", "osc24M",
> +   _mult_ops,
> +   CLK_SET_RATE_UNGATE),
> + },
> +};
> +
> +/* Some PLLs are input * N / div1 / P. Model them as NKMP with no K */
> +#define SUN50I_H6_PLL_DDR0_REG   0x010
> +static struct ccu_nkmp pll_ddr0_clk = {
> + .enable = BIT(31),
> + .lock   = BIT(28),
> + .n  = _SUNXI_CCU_MULT_MIN(8, 8, 12),
> + .m  = _SUNXI_CCU_DIV(1, 1), /* input divider */
> + .p  = _SUNXI_CCU_DIV(0, 1), /* output divider */
> + .common = {
> + .reg= 0x010,
> + .hw.init= CLK_HW_INIT("pll-ddr0", "osc24M",
> +   _nkmp_ops,
> +   CLK_SET_RATE_UNGATE),
> + },
> +};
> +
> +#define SUN50I_H6_PLL_PERIPH0_REG0x020
> +static struct ccu_nkmp pll_periph0_clk = {
> + .enable = BIT(31),
> + .lock   = BIT(28),
> + .n  = _SUNXI_CCU_MULT_MIN(8, 8, 12),
> + .m  = _SUNXI_CCU_DIV(1, 1), 

Re: [linux-sunxi] [PATCH v4 7/9] clk: sunxi-ng: add support for the Allwinner H6 CCU

[Jernej Škrabec]

Hi,

Dne petek, 16. marec 2018 ob 15:02:13 CET je Icenowy Zheng napisal(a):
> The Allwinner H6 SoC has a CCU which has been largely rearranged.
> 
> Add support for it in the sunxi-ng CCU framework.
> 
> Signed-off-by: Icenowy Zheng 
> Acked-by: Maxime Ripard 
> ---
> Changes in v4:
> - Extract the device tree binding document to another patch.
> 
> Changes in v3:
> - SPDX license idetifier fix.
> - Add some comments at initialization.
> 
> Changes in v2:
> - Exported APB1 bus clock for PIO.
> - Switch to SPDX license identifier.
> 
>  drivers/clk/sunxi-ng/Kconfig  |5 +
>  drivers/clk/sunxi-ng/Makefile |1 +
>  drivers/clk/sunxi-ng/ccu-sun50i-h6.c  | 1207
> + drivers/clk/sunxi-ng/ccu-sun50i-h6.h  |  
> 56 ++
>  include/dt-bindings/clock/sun50i-h6-ccu.h |  124 +++
>  include/dt-bindings/reset/sun50i-h6-ccu.h |   73 ++
>  6 files changed, 1466 insertions(+)
>  create mode 100644 drivers/clk/sunxi-ng/ccu-sun50i-h6.c
>  create mode 100644 drivers/clk/sunxi-ng/ccu-sun50i-h6.h
>  create mode 100644 include/dt-bindings/clock/sun50i-h6-ccu.h
>  create mode 100644 include/dt-bindings/reset/sun50i-h6-ccu.h
> 
> diff --git a/drivers/clk/sunxi-ng/Kconfig b/drivers/clk/sunxi-ng/Kconfig
> index 33168f94ee39..79dfd296c3d1 100644
> --- a/drivers/clk/sunxi-ng/Kconfig
> +++ b/drivers/clk/sunxi-ng/Kconfig
> @@ -11,6 +11,11 @@ config SUN50I_A64_CCU
>   default ARM64 && ARCH_SUNXI
>   depends on (ARM64 && ARCH_SUNXI) || COMPILE_TEST
> 
> +config SUN50I_H6_CCU
> + bool "Support for the Allwinner H6 CCU"
> + default ARM64 && ARCH_SUNXI
> + depends on (ARM64 && ARCH_SUNXI) || COMPILE_TEST
> +
>  config SUN4I_A10_CCU
>   bool "Support for the Allwinner A10/A20 CCU"
>   default MACH_SUN4I
> diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
> index 4141c3fe08ae..128a40ee5c5e 100644
> --- a/drivers/clk/sunxi-ng/Makefile
> +++ b/drivers/clk/sunxi-ng/Makefile
> @@ -22,6 +22,7 @@ lib-$(CONFIG_SUNXI_CCU) += ccu_mp.o
> 
>  # SoC support
>  obj-$(CONFIG_SUN50I_A64_CCU) += ccu-sun50i-a64.o
> +obj-$(CONFIG_SUN50I_H6_CCU)  += ccu-sun50i-h6.o
>  obj-$(CONFIG_SUN4I_A10_CCU)  += ccu-sun4i-a10.o
>  obj-$(CONFIG_SUN5I_CCU)  += ccu-sun5i.o
>  obj-$(CONFIG_SUN6I_A31_CCU)  += ccu-sun6i-a31.o
> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
> b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c new file mode 100644
> index ..d5eab49e6350
> --- /dev/null
> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
> @@ -0,0 +1,1207 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2017 Icenowy Zheng 
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "ccu_common.h"
> +#include "ccu_reset.h"
> +
> +#include "ccu_div.h"
> +#include "ccu_gate.h"
> +#include "ccu_mp.h"
> +#include "ccu_mult.h"
> +#include "ccu_nk.h"
> +#include "ccu_nkm.h"
> +#include "ccu_nkmp.h"
> +#include "ccu_nm.h"
> +
> +#include "ccu-sun50i-h6.h"
> +
> +/*
> + * The CPU PLL is actually NP clock, with P being /1, /2 or /4. However
> + * P should only be used for output frequencies lower than 288 MHz.
> + *
> + * For now we can just model it as a multiplier clock, and force P to /1.
> + *
> + * The M factor is present in the register's description, but not in the
> + * frequency formula, and it's documented as "M is only used for backdoor
> + * testing", so it's not modelled and then force to 0.
> + */
> +#define SUN50I_H6_PLL_CPUX_REG   0x000
> +static struct ccu_mult pll_cpux_clk = {
> + .enable = BIT(31),
> + .lock   = BIT(28),
> + .mult   = _SUNXI_CCU_MULT_MIN(8, 8, 12),
> + .common = {
> + .reg= 0x000,
> + .hw.init= CLK_HW_INIT("pll-cpux", "osc24M",
> +   _mult_ops,
> +   CLK_SET_RATE_UNGATE),
> + },
> +};
> +
> +/* Some PLLs are input * N / div1 / P. Model them as NKMP with no K */
> +#define SUN50I_H6_PLL_DDR0_REG   0x010
> +static struct ccu_nkmp pll_ddr0_clk = {
> + .enable = BIT(31),
> + .lock   = BIT(28),
> + .n  = _SUNXI_CCU_MULT_MIN(8, 8, 12),
> + .m  = _SUNXI_CCU_DIV(1, 1), /* input divider */
> + .p  = _SUNXI_CCU_DIV(0, 1), /* output divider */
> + .common = {
> + .reg= 0x010,
> + .hw.init= CLK_HW_INIT("pll-ddr0", "osc24M",
> +   _nkmp_ops,
> +   CLK_SET_RATE_UNGATE),
> + },
> +};
> +
> +#define SUN50I_H6_PLL_PERIPH0_REG0x020
> +static struct ccu_nkmp pll_periph0_clk = {
> + .enable = BIT(31),
> + .lock   = BIT(28),
> + .n  = _SUNXI_CCU_MULT_MIN(8, 8, 12),
> + .m  = _SUNXI_CCU_DIV(1, 1), /* input divider */
> + .p  = 

Re: New -Werror=restrict error with incremental gcc

[Laura Abbott]

On 03/15/2018 08:11 PM, Josh Poimboeuf wrote:

On Thu, Mar 15, 2018 at 08:06:26AM -0700, Laura Abbott wrote:

This only showed up with the very latest rawhide snapshot, .17 worked and
.18 started failing. I had to download .18 manually to test locally
https://koji.fedoraproject.org/koji/packageinfo?packageID=40


I also see the error with the latest gcc master branch.  The code is
harmless, but maybe the warning is useful in other places, so here's one
way to fix it.



Works for me, you can add

Tested-by: Laura Abbott 




From: Josh Poimboeuf 
Subject: [PATCH] objtool, perf: Fix GCC 8 -Wrestrict error

Starting with recent GCC 8 builds, objtool and perf fail to build with
the following error:

   ../str_error_r.c: In function ‘str_error_r’:
   ../str_error_r.c:25:3: error: passing argument 1 to restrict-qualified 
parameter aliases with argument 5 [-Werror=restrict]
  snprintf(buf, buflen, "INTERNAL ERROR: strerror_r(%d, %p, %zd)=%d", 
errnum, buf, buflen, err);

The code seems harmless, but there's probably no benefit in printing the
'buf' pointer in this situation anyway, so just remove it to make GCC
happy.

Signed-off-by: Josh Poimboeuf 
---
  tools/lib/str_error_r.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/str_error_r.c b/tools/lib/str_error_r.c
index d6d65537b0d9..6aad8308a0ac 100644
--- a/tools/lib/str_error_r.c
+++ b/tools/lib/str_error_r.c
@@ -22,6 +22,6 @@ char *str_error_r(int errnum, char *buf, size_t buflen)
  {
int err = strerror_r(errnum, buf, buflen);
if (err)
-   snprintf(buf, buflen, "INTERNAL ERROR: strerror_r(%d, %p, 
%zd)=%d", errnum, buf, buflen, err);
+   snprintf(buf, buflen, "INTERNAL ERROR: strerror_r(%d, [buf], 
%zd)=%d", errnum, buflen, err);
return buf;
  }

Re: New -Werror=restrict error with incremental gcc

[Laura Abbott]

On 03/15/2018 08:11 PM, Josh Poimboeuf wrote:

On Thu, Mar 15, 2018 at 08:06:26AM -0700, Laura Abbott wrote:

This only showed up with the very latest rawhide snapshot, .17 worked and
.18 started failing. I had to download .18 manually to test locally
https://koji.fedoraproject.org/koji/packageinfo?packageID=40


I also see the error with the latest gcc master branch.  The code is
harmless, but maybe the warning is useful in other places, so here's one
way to fix it.



Works for me, you can add

Tested-by: Laura Abbott 




From: Josh Poimboeuf 
Subject: [PATCH] objtool, perf: Fix GCC 8 -Wrestrict error

Starting with recent GCC 8 builds, objtool and perf fail to build with
the following error:

   ../str_error_r.c: In function ‘str_error_r’:
   ../str_error_r.c:25:3: error: passing argument 1 to restrict-qualified 
parameter aliases with argument 5 [-Werror=restrict]
  snprintf(buf, buflen, "INTERNAL ERROR: strerror_r(%d, %p, %zd)=%d", 
errnum, buf, buflen, err);

The code seems harmless, but there's probably no benefit in printing the
'buf' pointer in this situation anyway, so just remove it to make GCC
happy.

Signed-off-by: Josh Poimboeuf 
---
  tools/lib/str_error_r.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/str_error_r.c b/tools/lib/str_error_r.c
index d6d65537b0d9..6aad8308a0ac 100644
--- a/tools/lib/str_error_r.c
+++ b/tools/lib/str_error_r.c
@@ -22,6 +22,6 @@ char *str_error_r(int errnum, char *buf, size_t buflen)
  {
int err = strerror_r(errnum, buf, buflen);
if (err)
-   snprintf(buf, buflen, "INTERNAL ERROR: strerror_r(%d, %p, 
%zd)=%d", errnum, buf, buflen, err);
+   snprintf(buf, buflen, "INTERNAL ERROR: strerror_r(%d, [buf], 
%zd)=%d", errnum, buflen, err);
return buf;
  }

Re: [PATCH v5 2/9] proc/sysctl: Provide additional ctl_table.flags checks

[Luis R. Rodriguez]

On Fri, Mar 16, 2018 at 02:13:43PM -0400, Waiman Long wrote:
> Checking code is added to provide the following additional
> ctl_table.flags checks:
> 
>  1) No unknown flag is allowed.
>  2) Minimum of a range cannot be larger than the maximum value.
>  3) The signed and unsigned flags are mutually exclusive.
>  4) The proc_handler should be consistent with the signed or unsigned
> flags.
> 
> Two new flags are added to indicate if the min/max values are signed
> or unsigned - CTL_FLAGS_SIGNED_RANGE & CTL_FLAGS_UNSIGNED_RANGE.
> These 2 flags can be optionally enabled for range checking purpose.
> But either one of them must be set with CTL_FLAGS_CLAMP_RANGE.
> 
> Signed-off-by: Waiman Long 
> ---

> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index e446e1f..088f032 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -134,14 +134,26 @@ struct ctl_table
>   *   the input value. No lower bound or upper bound checking will be
>   *   done if the corresponding minimum or maximum value isn't provided.
>   *
> + * @CTL_FLAGS_SIGNED_RANGE: Set to indicate that the extra1 and extra2
> + *   fields are pointers to minimum and maximum signed values of
> + *   an allowable range.
> + *
> + * @CTL_FLAGS_UNSIGNED_RANGE: Set to indicate that the extra1 and extra2
> + *   fields are pointers to minimum and maximum unsigned values of
> + *   an allowable range.
> + *
>   * At most 16 different flags are allowed.
>   */
>  enum ctl_table_flags {
>   CTL_FLAGS_CLAMP_RANGE   = BIT(0),
> - __CTL_FLAGS_MAX = BIT(1),
> + CTL_FLAGS_SIGNED_RANGE  = BIT(1),
> + CTL_FLAGS_UNSIGNED_RANGE= BIT(2),
> + __CTL_FLAGS_MAX = BIT(3),
>  };

You are adding new flags which the user can set, and yet these are used
internally.

It would be best if internal flags are just that, not flags that a user can set.

This patch should be folded with the first one.

I'm starting to loose hope on these patch sets.

  Luis

Re: [PATCH v5 2/9] proc/sysctl: Provide additional ctl_table.flags checks

[Luis R. Rodriguez]

On Fri, Mar 16, 2018 at 02:13:43PM -0400, Waiman Long wrote:
> Checking code is added to provide the following additional
> ctl_table.flags checks:
> 
>  1) No unknown flag is allowed.
>  2) Minimum of a range cannot be larger than the maximum value.
>  3) The signed and unsigned flags are mutually exclusive.
>  4) The proc_handler should be consistent with the signed or unsigned
> flags.
> 
> Two new flags are added to indicate if the min/max values are signed
> or unsigned - CTL_FLAGS_SIGNED_RANGE & CTL_FLAGS_UNSIGNED_RANGE.
> These 2 flags can be optionally enabled for range checking purpose.
> But either one of them must be set with CTL_FLAGS_CLAMP_RANGE.
> 
> Signed-off-by: Waiman Long 
> ---

> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index e446e1f..088f032 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -134,14 +134,26 @@ struct ctl_table
>   *   the input value. No lower bound or upper bound checking will be
>   *   done if the corresponding minimum or maximum value isn't provided.
>   *
> + * @CTL_FLAGS_SIGNED_RANGE: Set to indicate that the extra1 and extra2
> + *   fields are pointers to minimum and maximum signed values of
> + *   an allowable range.
> + *
> + * @CTL_FLAGS_UNSIGNED_RANGE: Set to indicate that the extra1 and extra2
> + *   fields are pointers to minimum and maximum unsigned values of
> + *   an allowable range.
> + *
>   * At most 16 different flags are allowed.
>   */
>  enum ctl_table_flags {
>   CTL_FLAGS_CLAMP_RANGE   = BIT(0),
> - __CTL_FLAGS_MAX = BIT(1),
> + CTL_FLAGS_SIGNED_RANGE  = BIT(1),
> + CTL_FLAGS_UNSIGNED_RANGE= BIT(2),
> + __CTL_FLAGS_MAX = BIT(3),
>  };

You are adding new flags which the user can set, and yet these are used
internally.

It would be best if internal flags are just that, not flags that a user can set.

This patch should be folded with the first one.

I'm starting to loose hope on these patch sets.

  Luis

Re: [PATCH v4 1/5] PCI: endpoint: BAR width should not depend on sizeof dma_addr_t

[Niklas Cassel]

On Fri, Mar 16, 2018 at 06:02:20PM +, Lorenzo Pieralisi wrote:
> On Thu, Mar 08, 2018 at 02:33:26PM +0100, Niklas Cassel wrote:
> > If a BAR supports 64-bit width or not depends on the hardware,
> > and should thus not depend on sizeof(dma_addr_t).
> > 
> > Since this driver is generic, default to always using BAR width
> > of 32-bits. 64-bit BARs can easily be tested by replacing
> > PCI_BASE_ADDRESS_MEM_TYPE_32 with PCI_BASE_ADDRESS_MEM_TYPE_64
> > in bar_flags.
> > 
> > Signed-off-by: Niklas Cassel 
> > ---
> > Note to Lorenzo/Bjorn:
> > It is not trivial to convert the bar_size + bar_flags +
> > struct pci_epf->bar member array to an array of struct resources,
> > since we need to be able to store the addresses returned
> > by dma_alloc_coherent(), which is of type dma_addr_t.
> > struct resource uses resource_size_t, which is defined as phys_addr_t.
> > E.g. ARTPEC-7 uses 64-bit dma_addr_t, but only 32-bit phys_addr_t.
> > 
> >  drivers/pci/endpoint/functions/pci-epf-test.c | 15 +--
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c 
> > b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index 800da09d9005..7c70433b11a7 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -71,6 +71,14 @@ struct pci_epf_test_data {
> >  };
> >  
> >  static int bar_size[] = { 512, 512, 1024, 16384, 131072, 1048576 };
> > +static int bar_flags[] = {
> > +   PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > +   PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > +   PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > +   PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > +   PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > +   PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32
> > +};
> 
> Niklas,
> 
> I think you are almost there, I have one question though to address
> that can even simplify the patchset.
> 
> If, according to your own commit logs (and my reading of the code), the
> Cadence driver makes a decision on the BAR size just by checking the
> corresponding region size (I would be happy to hear the reason
> underpinning that choice, BTW), why can't we do the same for DWC (ie to
> let the DWC driver decides whether a BAR should be 64 or 32 bits ?)

We could, but I think that would be a mistake.

The API that the user/"endpoint function" has available to configure
the BARs:
pci_epc_set_bar()

If the user, for some reason, wants to configure a BAR with a
64-bit width, even though the BAR size is less than 4 GB,
I think that the API should allow that.

This would not be possible if pci_epc_set_bar() would start to
ignore PCI_BASE_ADDRESS_MEM_TYPE_64 from the flags parameter
(and BAR width thus only being controlled by the size parameter).

int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, enum pci_barno bar,
dma_addr_t bar_phys, size_t size, int flags);



Best regards,
Niklas

Re: [PATCH v4 1/5] PCI: endpoint: BAR width should not depend on sizeof dma_addr_t

[Niklas Cassel]

On Fri, Mar 16, 2018 at 06:02:20PM +, Lorenzo Pieralisi wrote:
> On Thu, Mar 08, 2018 at 02:33:26PM +0100, Niklas Cassel wrote:
> > If a BAR supports 64-bit width or not depends on the hardware,
> > and should thus not depend on sizeof(dma_addr_t).
> > 
> > Since this driver is generic, default to always using BAR width
> > of 32-bits. 64-bit BARs can easily be tested by replacing
> > PCI_BASE_ADDRESS_MEM_TYPE_32 with PCI_BASE_ADDRESS_MEM_TYPE_64
> > in bar_flags.
> > 
> > Signed-off-by: Niklas Cassel 
> > ---
> > Note to Lorenzo/Bjorn:
> > It is not trivial to convert the bar_size + bar_flags +
> > struct pci_epf->bar member array to an array of struct resources,
> > since we need to be able to store the addresses returned
> > by dma_alloc_coherent(), which is of type dma_addr_t.
> > struct resource uses resource_size_t, which is defined as phys_addr_t.
> > E.g. ARTPEC-7 uses 64-bit dma_addr_t, but only 32-bit phys_addr_t.
> > 
> >  drivers/pci/endpoint/functions/pci-epf-test.c | 15 +--
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c 
> > b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index 800da09d9005..7c70433b11a7 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -71,6 +71,14 @@ struct pci_epf_test_data {
> >  };
> >  
> >  static int bar_size[] = { 512, 512, 1024, 16384, 131072, 1048576 };
> > +static int bar_flags[] = {
> > +   PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > +   PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > +   PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > +   PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > +   PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32,
> > +   PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_32
> > +};
> 
> Niklas,
> 
> I think you are almost there, I have one question though to address
> that can even simplify the patchset.
> 
> If, according to your own commit logs (and my reading of the code), the
> Cadence driver makes a decision on the BAR size just by checking the
> corresponding region size (I would be happy to hear the reason
> underpinning that choice, BTW), why can't we do the same for DWC (ie to
> let the DWC driver decides whether a BAR should be 64 or 32 bits ?)

We could, but I think that would be a mistake.

The API that the user/"endpoint function" has available to configure
the BARs:
pci_epc_set_bar()

If the user, for some reason, wants to configure a BAR with a
64-bit width, even though the BAR size is less than 4 GB,
I think that the API should allow that.

This would not be possible if pci_epc_set_bar() would start to
ignore PCI_BASE_ADDRESS_MEM_TYPE_64 from the flags parameter
(and BAR width thus only being controlled by the size parameter).

int pci_epc_set_bar(struct pci_epc *epc, u8 func_no, enum pci_barno bar,
dma_addr_t bar_phys, size_t size, int flags);



Best regards,
Niklas

Re: [PATCH 2/4] mm/hmm: fix header file if/else/endif maze

[kbuild test robot]

Hi Jérôme,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180316]
[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/jglisse-redhat-com/mm-hmm-documentation-editorial-update-to-HMM-documentation/20180317-074102
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from kernel/fork.c:40:0:
>> include/linux/hmm.h:515:20: error: redefinition of 'hmm_mm_destroy'
static inline void hmm_mm_destroy(struct mm_struct *mm) {}
   ^~
   include/linux/hmm.h:502:20: note: previous definition of 'hmm_mm_destroy' 
was here
static inline void hmm_mm_destroy(struct mm_struct *mm) {}
   ^~
>> include/linux/hmm.h:516:20: error: redefinition of 'hmm_mm_init'
static inline void hmm_mm_init(struct mm_struct *mm) {}
   ^~~
   include/linux/hmm.h:503:20: note: previous definition of 'hmm_mm_init' was 
here
static inline void hmm_mm_init(struct mm_struct *mm) {}
   ^~~

vim +/hmm_mm_destroy +515 include/linux/hmm.h

133ff0eac Jérôme Glisse 2017-09-08  509  
133ff0eac Jérôme Glisse 2017-09-08  510  static inline void hmm_mm_init(struct 
mm_struct *mm)
133ff0eac Jérôme Glisse 2017-09-08  511  {
133ff0eac Jérôme Glisse 2017-09-08  512 mm->hmm = NULL;
133ff0eac Jérôme Glisse 2017-09-08  513  }
6b368cd4a Jérôme Glisse 2017-09-08  514  #else /* IS_ENABLED(CONFIG_HMM_MIRROR) 
*/
6b368cd4a Jérôme Glisse 2017-09-08 @515  static inline void 
hmm_mm_destroy(struct mm_struct *mm) {}
6b368cd4a Jérôme Glisse 2017-09-08 @516  static inline void hmm_mm_init(struct 
mm_struct *mm) {}
6b368cd4a Jérôme Glisse 2017-09-08  517  #endif /* 
IS_ENABLED(CONFIG_HMM_MIRROR) */
133ff0eac Jérôme Glisse 2017-09-08  518  

:: The code at line 515 was first introduced by commit
:: 6b368cd4a44ce95b33f1d31f2f932e6ae707f319 mm/hmm: avoid bloating arch 
that do not make use of HMM

:: TO: Jérôme Glisse <jgli...@redhat.com>
:: CC: Linus Torvalds <torva...@linux-foundation.org>

---
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/4] mm/hmm: fix header file if/else/endif maze

[kbuild test robot]

Hi Jérôme,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180316]
[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/jglisse-redhat-com/mm-hmm-documentation-editorial-update-to-HMM-documentation/20180317-074102
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from kernel/fork.c:40:0:
>> include/linux/hmm.h:515:20: error: redefinition of 'hmm_mm_destroy'
static inline void hmm_mm_destroy(struct mm_struct *mm) {}
   ^~
   include/linux/hmm.h:502:20: note: previous definition of 'hmm_mm_destroy' 
was here
static inline void hmm_mm_destroy(struct mm_struct *mm) {}
   ^~
>> include/linux/hmm.h:516:20: error: redefinition of 'hmm_mm_init'
static inline void hmm_mm_init(struct mm_struct *mm) {}
   ^~~
   include/linux/hmm.h:503:20: note: previous definition of 'hmm_mm_init' was 
here
static inline void hmm_mm_init(struct mm_struct *mm) {}
   ^~~

vim +/hmm_mm_destroy +515 include/linux/hmm.h

133ff0eac Jérôme Glisse 2017-09-08  509  
133ff0eac Jérôme Glisse 2017-09-08  510  static inline void hmm_mm_init(struct 
mm_struct *mm)
133ff0eac Jérôme Glisse 2017-09-08  511  {
133ff0eac Jérôme Glisse 2017-09-08  512 mm->hmm = NULL;
133ff0eac Jérôme Glisse 2017-09-08  513  }
6b368cd4a Jérôme Glisse 2017-09-08  514  #else /* IS_ENABLED(CONFIG_HMM_MIRROR) 
*/
6b368cd4a Jérôme Glisse 2017-09-08 @515  static inline void 
hmm_mm_destroy(struct mm_struct *mm) {}
6b368cd4a Jérôme Glisse 2017-09-08 @516  static inline void hmm_mm_init(struct 
mm_struct *mm) {}
6b368cd4a Jérôme Glisse 2017-09-08  517  #endif /* 
IS_ENABLED(CONFIG_HMM_MIRROR) */
133ff0eac Jérôme Glisse 2017-09-08  518  

:: The code at line 515 was first introduced by commit
:: 6b368cd4a44ce95b33f1d31f2f932e6ae707f319 mm/hmm: avoid bloating arch 
that do not make use of HMM

:: TO: Jérôme Glisse 
:: CC: Linus Torvalds 

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


.config.gz
Description: application/gzip

Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()

[Miguel Ojeda]

On Fri, Mar 16, 2018 at 9:14 PM, Linus Torvalds
 wrote:
> On Fri, Mar 16, 2018 at 1:03 PM, Miguel Ojeda
>  wrote:
>>>
>>> Kees - is there some online "gcc-4.4 checker" somewhere? This does
>>> seem to work with my gcc. I actually tested some of those files you
>>> pointed at now.
>>
>> I use this one:
>>
>>   https://godbolt.org/
>
> Well, my *test* code works on that one and -Wvla -Werror.
>
> It does not work with gcc-4.1.x, but works with gcc-4.4.x.
>
> I can't seem to see the errors any way, I wonder if
> __builtin_choose_expr() simply didn't exist back then.
>
> Odd that you can't view warnings/errors with it.

Just click on the button left/bottom of the compilation window.

Cheers,
Miguel

Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()

[Miguel Ojeda]

On Fri, Mar 16, 2018 at 9:14 PM, Linus Torvalds
 wrote:
> On Fri, Mar 16, 2018 at 1:03 PM, Miguel Ojeda
>  wrote:
>>>
>>> Kees - is there some online "gcc-4.4 checker" somewhere? This does
>>> seem to work with my gcc. I actually tested some of those files you
>>> pointed at now.
>>
>> I use this one:
>>
>>   https://godbolt.org/
>
> Well, my *test* code works on that one and -Wvla -Werror.
>
> It does not work with gcc-4.1.x, but works with gcc-4.4.x.
>
> I can't seem to see the errors any way, I wonder if
> __builtin_choose_expr() simply didn't exist back then.
>
> Odd that you can't view warnings/errors with it.

Just click on the button left/bottom of the compilation window.

Cheers,
Miguel

Re: [PATCH v4 09/10] drivers: qcom: rpmh: add support for batch RPMH request

[Evan Green]

On Fri, Mar 9, 2018 at 3:26 PM Lina Iyer  wrote:

> Platform drivers need make a lot of resource state requests at the same
> time, say, at the start or end of an usecase. It can be quite
> inefficient to send each request separately. Instead they can give the
> RPMH library a batch of requests to be sent and wait on the whole
> transaction to be complete.

> rpmh_write_batch() is a blocking call that can be used to send multiple
> RPMH command sets. Each RPMH command set is set asynchronously and the
> API blocks until all the command sets are complete and receive their
> tx_done callbacks.

> Signed-off-by: Lina Iyer 
> ---

> Changes in v4:
>  - reorganize rpmh_write_batch()
>  - introduce wait_count here, instead of patch#4
> ---
>   drivers/soc/qcom/rpmh.c | 156
+++-
>   include/soc/qcom/rpmh.h |   8 +++
>   2 files changed, 162 insertions(+), 2 deletions(-)


Reviewed-by: Evan Green 

Re: [PATCH v4 09/10] drivers: qcom: rpmh: add support for batch RPMH request

[Evan Green]

On Fri, Mar 9, 2018 at 3:26 PM Lina Iyer  wrote:

> Platform drivers need make a lot of resource state requests at the same
> time, say, at the start or end of an usecase. It can be quite
> inefficient to send each request separately. Instead they can give the
> RPMH library a batch of requests to be sent and wait on the whole
> transaction to be complete.

> rpmh_write_batch() is a blocking call that can be used to send multiple
> RPMH command sets. Each RPMH command set is set asynchronously and the
> API blocks until all the command sets are complete and receive their
> tx_done callbacks.

> Signed-off-by: Lina Iyer 
> ---

> Changes in v4:
>  - reorganize rpmh_write_batch()
>  - introduce wait_count here, instead of patch#4
> ---
>   drivers/soc/qcom/rpmh.c | 156
+++-
>   include/soc/qcom/rpmh.h |   8 +++
>   2 files changed, 162 insertions(+), 2 deletions(-)


Reviewed-by: Evan Green 

Re: [PATCH v4 07/10] drivers: qcom: rpmh: cache sleep/wake state requests

[Evan Green]

On Fri, Mar 9, 2018 at 3:27 PM Lina Iyer  wrote:

> Active state requests are sent immediately to the mailbox controller,
> while sleep and wake state requests are cached in this driver to avoid
> taxing the mailbox controller repeatedly. The cached values will be sent
> to the controller when the rpmh_flush() is called.

> Generally, flushing is a system PM activity and may be called from the
> system PM drivers when the system is entering suspend or deeper sleep
> modes during cpuidle.

> Also allow invalidating the cached requests, so they may be re-populated
> again.

> Signed-off-by: Lina Iyer 
> ---

> Changes in v4:
>  - remove locking for ->dirty in invalidate
>  - fix send_single
> Changes in v3:
>  - Remove locking for flush function
>  - Improve comments
> ---
>   drivers/soc/qcom/rpmh.c | 203
+++-
>   include/soc/qcom/rpmh.h |  10 +++
>   2 files changed, 212 insertions(+), 1 deletion(-)


Reviewed-by: Evan Green 

Re: [PATCH v4 07/10] drivers: qcom: rpmh: cache sleep/wake state requests

[Evan Green]

On Fri, Mar 9, 2018 at 3:27 PM Lina Iyer  wrote:

> Active state requests are sent immediately to the mailbox controller,
> while sleep and wake state requests are cached in this driver to avoid
> taxing the mailbox controller repeatedly. The cached values will be sent
> to the controller when the rpmh_flush() is called.

> Generally, flushing is a system PM activity and may be called from the
> system PM drivers when the system is entering suspend or deeper sleep
> modes during cpuidle.

> Also allow invalidating the cached requests, so they may be re-populated
> again.

> Signed-off-by: Lina Iyer 
> ---

> Changes in v4:
>  - remove locking for ->dirty in invalidate
>  - fix send_single
> Changes in v3:
>  - Remove locking for flush function
>  - Improve comments
> ---
>   drivers/soc/qcom/rpmh.c | 203
+++-
>   include/soc/qcom/rpmh.h |  10 +++
>   2 files changed, 212 insertions(+), 1 deletion(-)


Reviewed-by: Evan Green 

Re: [PATCH v4 05/10] drivers: qcom: rpmh-rsc: write sleep/wake requests to TCS

[Evan Green]

On Fri, Mar 9, 2018 at 3:27 PM Lina Iyer  wrote:

> Sleep and wake requests are sent when the application processor
> subsystem of the SoC is entering deep sleep states like in suspend.
> These requests help lower the system power requirements when the
> resources are not in use.

> Sleep and wake requests are written to the TCS slots but are not
> triggered at the time of writing. The TCS are triggered by the firmware
> after the last of the CPUs has executed its WFI. Since these requests
> may come in different batches of requests, it is the job of this
> controller driver to find and arrange the requests into the available
> TCSes.

> Signed-off-by: Lina Iyer 
> ---
>drivers/soc/qcom/rpmh-internal.h |   9 ++-
>drivers/soc/qcom/rpmh-rsc.c  | 120
+++
>2 files changed, 128 insertions(+), 1 deletion(-)


Reviewed-by: Evan Green 

Re: [PATCH v4 05/10] drivers: qcom: rpmh-rsc: write sleep/wake requests to TCS

[Evan Green]

On Fri, Mar 9, 2018 at 3:27 PM Lina Iyer  wrote:

> Sleep and wake requests are sent when the application processor
> subsystem of the SoC is entering deep sleep states like in suspend.
> These requests help lower the system power requirements when the
> resources are not in use.

> Sleep and wake requests are written to the TCS slots but are not
> triggered at the time of writing. The TCS are triggered by the firmware
> after the last of the CPUs has executed its WFI. Since these requests
> may come in different batches of requests, it is the job of this
> controller driver to find and arrange the requests into the available
> TCSes.

> Signed-off-by: Lina Iyer 
> ---
>drivers/soc/qcom/rpmh-internal.h |   9 ++-
>drivers/soc/qcom/rpmh-rsc.c  | 120
+++
>2 files changed, 128 insertions(+), 1 deletion(-)


Reviewed-by: Evan Green 

Re: [PATCH] dcache: remove trailing whitespace

[Niklas Cassel]

On Fri, Mar 16, 2018 at 03:55:38PM +, Al Viro wrote:
> On Fri, Mar 16, 2018 at 03:34:00PM +0100, Niklas Cassel wrote:
> > Remove trailing whitespace.
> > Remove empty line and trailing whitespace after function comments.
> > Remove an extra space in one of the comments.
> > Fix a typo in of the comments.
> > 
> > Signed-off-by: Niklas Cassel <niklas.cas...@axis.com>
> > ---
> > I know that these type of patches are not really appreciated,
> > however, there is enough trailing whitespace in this file to
> > distract me from reading the real code.
> 
> No, they are not, and here's why:
> 
> Applying: dcache: remove trailing whitespace
> error: patch failed: fs/dcache.c:254
> error: fs/dcache.c: patch does not apply
> Patch failed at 0001 dcache: remove trailing whitespace
> 
> ... which, BTW, happens in *all* branches.  If you do that kind
> of stuff, at least do it against something in the public trees
> and _tell_ _what_ _it_ _is_ _against_.

Hello Al,

I always base my patches on linux-next, which is usually correct,
and if it isn't, the maintainer usually says something :)

The patch in question is based on the latest linux-next tag:
next-20180316

It appears that the following three commits are in linux-next,
but not on your for-next branch @ your vfs git tree:
61fc3c8ce7f1 dcache: remove trailing whitespace
929387149a47 dcache: fix indirectly reclaimable memory accounting
ff335768ddd0 dcache: fix indirectly reclaimable memory accounting for 
CONFIG_SLOB

They appear to come from Andrew Morton's tree.

Would you prefer me to rebase my patch against your
for-next branch instead?

> Not applied.  Please, do it sanely.  BTW, which editor is _that_
> annoying?  Anything that shoves trailing whitespace in my face
> would've been either configured (with considerable cursing at
> the people who'd set such defaults) or, should that prove
> impossible, given a boot...

I'm using emacs, but having added show-trailing-whitespace
is my personal preference, however, I quite sure that I'm
not the only one who has an editor configured like this.


Kind regards,
Niklas

Re: [PATCH] dcache: remove trailing whitespace

[Niklas Cassel]

On Fri, Mar 16, 2018 at 03:55:38PM +, Al Viro wrote:
> On Fri, Mar 16, 2018 at 03:34:00PM +0100, Niklas Cassel wrote:
> > Remove trailing whitespace.
> > Remove empty line and trailing whitespace after function comments.
> > Remove an extra space in one of the comments.
> > Fix a typo in of the comments.
> > 
> > Signed-off-by: Niklas Cassel 
> > ---
> > I know that these type of patches are not really appreciated,
> > however, there is enough trailing whitespace in this file to
> > distract me from reading the real code.
> 
> No, they are not, and here's why:
> 
> Applying: dcache: remove trailing whitespace
> error: patch failed: fs/dcache.c:254
> error: fs/dcache.c: patch does not apply
> Patch failed at 0001 dcache: remove trailing whitespace
> 
> ... which, BTW, happens in *all* branches.  If you do that kind
> of stuff, at least do it against something in the public trees
> and _tell_ _what_ _it_ _is_ _against_.

Hello Al,

I always base my patches on linux-next, which is usually correct,
and if it isn't, the maintainer usually says something :)

The patch in question is based on the latest linux-next tag:
next-20180316

It appears that the following three commits are in linux-next,
but not on your for-next branch @ your vfs git tree:
61fc3c8ce7f1 dcache: remove trailing whitespace
929387149a47 dcache: fix indirectly reclaimable memory accounting
ff335768ddd0 dcache: fix indirectly reclaimable memory accounting for 
CONFIG_SLOB

They appear to come from Andrew Morton's tree.

Would you prefer me to rebase my patch against your
for-next branch instead?

> Not applied.  Please, do it sanely.  BTW, which editor is _that_
> annoying?  Anything that shoves trailing whitespace in my face
> would've been either configured (with considerable cursing at
> the people who'd set such defaults) or, should that prove
> impossible, given a boot...

I'm using emacs, but having added show-trailing-whitespace
is my personal preference, however, I quite sure that I'm
not the only one who has an editor configured like this.


Kind regards,
Niklas

[PATCH] drm/i915: Disable some extra clang warnings

[Matthias Kaehlcke]

Commit 39bf4de89ff7 ("drm/i915: Add -Wall -Wextra to our build, set
warnings to full") enabled extra warnings for i915 to spot possible
bugs in new code, and then disabled a subset of these warnings to keep
the current code building without warnings (with gcc). Enabling the
extra warnings also enabled some additional clang-only warnings, as a
result building i915 with clang currently is extremely noisy. For now
also disable the clang warnings sign-compare, sometimes-uninitialized,
unneeded-internal-declaration and initializer-overrides. If desired
they can be re-enabled after the code has been fixed.

Fixes: 39bf4de89ff7 ("drm/i915: Add -Wall -Wextra to our build, set
warnings to full")
Signed-off-by: Matthias Kaehlcke 
---
 drivers/gpu/drm/i915/Makefile | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 091aef281963..ad05796a96ba 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -17,6 +17,10 @@ subdir-ccflags-y += $(call cc-disable-warning, 
unused-parameter)
 subdir-ccflags-y += $(call cc-disable-warning, type-limits)
 subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
 subdir-ccflags-y += $(call cc-disable-warning, implicit-fallthrough)
+subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
+subdir-ccflags-y += $(call cc-disable-warning, sometimes-uninitialized)
+subdir-ccflags-y += $(call cc-disable-warning, unneeded-internal-declaration)
+subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
 subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
 
 # Fine grained warnings disable
-- 
2.16.2.804.g6dcf76e118-goog

[PATCH] drm/i915: Disable some extra clang warnings

[Matthias Kaehlcke]

Commit 39bf4de89ff7 ("drm/i915: Add -Wall -Wextra to our build, set
warnings to full") enabled extra warnings for i915 to spot possible
bugs in new code, and then disabled a subset of these warnings to keep
the current code building without warnings (with gcc). Enabling the
extra warnings also enabled some additional clang-only warnings, as a
result building i915 with clang currently is extremely noisy. For now
also disable the clang warnings sign-compare, sometimes-uninitialized,
unneeded-internal-declaration and initializer-overrides. If desired
they can be re-enabled after the code has been fixed.

Fixes: 39bf4de89ff7 ("drm/i915: Add -Wall -Wextra to our build, set
warnings to full")
Signed-off-by: Matthias Kaehlcke 
---
 drivers/gpu/drm/i915/Makefile | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 091aef281963..ad05796a96ba 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -17,6 +17,10 @@ subdir-ccflags-y += $(call cc-disable-warning, 
unused-parameter)
 subdir-ccflags-y += $(call cc-disable-warning, type-limits)
 subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
 subdir-ccflags-y += $(call cc-disable-warning, implicit-fallthrough)
+subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
+subdir-ccflags-y += $(call cc-disable-warning, sometimes-uninitialized)
+subdir-ccflags-y += $(call cc-disable-warning, unneeded-internal-declaration)
+subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
 subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
 
 # Fine grained warnings disable
-- 
2.16.2.804.g6dcf76e118-goog

Re: general protection fault in ucma_connect

[Al Viro]

On Fri, Mar 16, 2018 at 04:59:02PM -0700, syzbot wrote:
> Hello,
> 
> syzbot hit the following crash on upstream commit
> e2c15aff5f353ba80bd3bb49840837f65fa5cc43 (Thu Mar 15 18:07:35 2018 +)
> Merge tag 'sound-4.16-rc6' of
> git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
> 
> So far this crash happened 2 times on upstream.
> C reproducer is attached.
> syzkaller reproducer is attached.
> Raw console output is attached.
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached.

May I politely inquire why am I getting Cc on the slew of ucma reports?
My last involvement with that thing that wouldn't have been absolutely
trivial had been back in 2012, and even that had been a conversion of
fget() to fdget() - nowhere near the areas implicated by those.  Anything
more recent would have no impact on the object code - replacement of
POLL{IN,RDNORM} with EPOLL{IN,RDNORM} (equal values on x86) and two
replacements of unsigned int with __poll_t, which is typedefed to
unsigned.

It's not that I've objections against helping to debug that thing (other
than general aversion to drivers/infinibarf), but I'm really curious -
just what got me volunteered from the syzbot POV?

Al, digging through tons of unpleasant code in fs/{config,debug}fs
at the moment...

Re: general protection fault in ucma_connect

[Al Viro]

On Fri, Mar 16, 2018 at 04:59:02PM -0700, syzbot wrote:
> Hello,
> 
> syzbot hit the following crash on upstream commit
> e2c15aff5f353ba80bd3bb49840837f65fa5cc43 (Thu Mar 15 18:07:35 2018 +)
> Merge tag 'sound-4.16-rc6' of
> git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
> 
> So far this crash happened 2 times on upstream.
> C reproducer is attached.
> syzkaller reproducer is attached.
> Raw console output is attached.
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached.

May I politely inquire why am I getting Cc on the slew of ucma reports?
My last involvement with that thing that wouldn't have been absolutely
trivial had been back in 2012, and even that had been a conversion of
fget() to fdget() - nowhere near the areas implicated by those.  Anything
more recent would have no impact on the object code - replacement of
POLL{IN,RDNORM} with EPOLL{IN,RDNORM} (equal values on x86) and two
replacements of unsigned int with __poll_t, which is typedefed to
unsigned.

It's not that I've objections against helping to debug that thing (other
than general aversion to drivers/infinibarf), but I'm really curious -
just what got me volunteered from the syzbot POV?

Al, digging through tons of unpleasant code in fs/{config,debug}fs
at the moment...

Re: [PATCH 2/2] kprobe: fix: Add ftrace_ops_assist_func to kprobe blacklist

[Masami Hiramatsu]

On Fri, 16 Mar 2018 13:53:01 -0400 (EDT)
Mathieu Desnoyers  wrote:

> - On Mar 16, 2018, at 12:48 PM, rostedt rost...@goodmis.org wrote:
> 
> > On Fri, 16 Mar 2018 12:41:34 -0400
> > Steven Rostedt  wrote:
> > 
> >> Yes, kprobes are dangerous. I'm not saying it shouldn't be fixed, I'm
> >> saying that I don't have time to fix it now, but would be happy to
> >> accept patches if someone else does so.
> > 
> > And looking at what I replied before for the original patch. It would
> > probably be a good idea to blacklist directories. Like we do with
> > function tracing. We probably should black list both kernel/tracing and
> > kernel/events from being probed.
> > 
> > Did this come up at plumbers? You were there too, I don't remember
> > discussing it there.
> 
> I don't remember this coming up last Plumbers nor KS neither, given
> that we were focused on other topics.
> 
> Would the general approach you envision be based on emitting all code
> generated by compilation of all objects under kernel/tracing and
> kernel/events into a specific "nokprobes" text section of the kernel ?
> Perhaps we could create a specific linker scripts for those directories,
> or do you have in mind a neater way to do this ?

.kprobes.text section still exists. As I pointed in previous mail, I don't
think we have to put all those code into that section. But if you want,
it is acceptable to have a kconfig which push most of those ftrace related
code into .kprobes.text section.

Thank,

> 
> Thanks,
> 
> Mathieu
> 
> > 
> > -- Steve
> 
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com


-- 
Masami Hiramatsu 

Re: [PATCH 2/2] kprobe: fix: Add ftrace_ops_assist_func to kprobe blacklist

[Masami Hiramatsu]

On Fri, 16 Mar 2018 13:53:01 -0400 (EDT)
Mathieu Desnoyers  wrote:

> - On Mar 16, 2018, at 12:48 PM, rostedt rost...@goodmis.org wrote:
> 
> > On Fri, 16 Mar 2018 12:41:34 -0400
> > Steven Rostedt  wrote:
> > 
> >> Yes, kprobes are dangerous. I'm not saying it shouldn't be fixed, I'm
> >> saying that I don't have time to fix it now, but would be happy to
> >> accept patches if someone else does so.
> > 
> > And looking at what I replied before for the original patch. It would
> > probably be a good idea to blacklist directories. Like we do with
> > function tracing. We probably should black list both kernel/tracing and
> > kernel/events from being probed.
> > 
> > Did this come up at plumbers? You were there too, I don't remember
> > discussing it there.
> 
> I don't remember this coming up last Plumbers nor KS neither, given
> that we were focused on other topics.
> 
> Would the general approach you envision be based on emitting all code
> generated by compilation of all objects under kernel/tracing and
> kernel/events into a specific "nokprobes" text section of the kernel ?
> Perhaps we could create a specific linker scripts for those directories,
> or do you have in mind a neater way to do this ?

.kprobes.text section still exists. As I pointed in previous mail, I don't
think we have to put all those code into that section. But if you want,
it is acceptable to have a kconfig which push most of those ftrace related
code into .kprobes.text section.

Thank,

> 
> Thanks,
> 
> Mathieu
> 
> > 
> > -- Steve
> 
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com


-- 
Masami Hiramatsu