Re: [PATCH v8 06/10] tpm: Replace device number bitmap with IDR

2016-03-14 Thread Jarkko Sakkinen
On Sun, Mar 13, 2016 at 06:54:36PM -0400, Stefan Berger wrote:
> Replace the device number bitmap with IDR. Extend the number of devices we
> can create to 64k.
> Since an IDR allows us to associate a pointer with an ID, we use this now
> to rewrite tpm_chip_find_get() to simply look up the chip pointer by the
> given device ID.
> 
> Protect the IDR calls with a mutex.

I've merged patches up to this to my next branch i.e. you don't have
to carry them in the seres anymore.

/Jarkko

> Signed-off-by: Stefan Berger 
> Reviewed-by: Jason Gunthorpe 
> Reviewed-by: Jarkko Sakkinen 
> Tested-by: Jarkko Sakkinen 
> Signed-off-by: Jarkko Sakkinen 
> ---
>  drivers/char/tpm/tpm-chip.c  | 84 
> +---
>  drivers/char/tpm/tpm-interface.c |  1 +
>  drivers/char/tpm/tpm.h   |  5 +--
>  3 files changed, 48 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 5880377..f62c851 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -29,9 +29,8 @@
>  #include "tpm.h"
>  #include "tpm_eventlog.h"
>  
> -static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
> -static LIST_HEAD(tpm_chip_list);
> -static DEFINE_SPINLOCK(driver_lock);
> +DEFINE_IDR(dev_nums_idr);
> +static DEFINE_MUTEX(idr_lock);
>  
>  struct class *tpm_class;
>  dev_t tpm_devt;
> @@ -88,20 +87,30 @@ EXPORT_SYMBOL_GPL(tpm_put_ops);
>*/
>  struct tpm_chip *tpm_chip_find_get(int chip_num)
>  {
> - struct tpm_chip *pos, *chip = NULL;
> + struct tpm_chip *chip, *res = NULL;
> + int chip_prev;
> +
> + mutex_lock(_lock);
> +
> + if (chip_num == TPM_ANY_NUM) {
> + chip_num = 0;
> + do {
> + chip_prev = chip_num;
> + chip = idr_get_next(_nums_idr, _num);
> + if (chip && !tpm_try_get_ops(chip)) {
> + res = chip;
> + break;
> + }
> + } while (chip_prev != chip_num);
> + } else {
> + chip = idr_find_slowpath(_nums_idr, chip_num);
> + if (chip && !tpm_try_get_ops(chip))
> + res = chip;
> + }
>  
> - rcu_read_lock();
> - list_for_each_entry_rcu(pos, _chip_list, list) {
> - if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
> - continue;
> + mutex_unlock(_lock);
>  
> - /* rcu prevents chip from being free'd */
> - if (!tpm_try_get_ops(pos))
> - chip = pos;
> - break;
> - }
> - rcu_read_unlock();
> - return chip;
> + return res;
>  }
>  
>  /**
> @@ -114,9 +123,10 @@ static void tpm_dev_release(struct device *dev)
>  {
>   struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
>  
> - spin_lock(_lock);
> - clear_bit(chip->dev_num, dev_mask);
> - spin_unlock(_lock);
> + mutex_lock(_lock);
> + idr_remove(_nums_idr, chip->dev_num);
> + mutex_unlock(_lock);
> +
>   kfree(chip);
>  }
>  
> @@ -142,21 +152,18 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
>  
>   mutex_init(>tpm_mutex);
>   init_rwsem(>ops_sem);
> - INIT_LIST_HEAD(>list);
>  
>   chip->ops = ops;
>  
> - spin_lock(_lock);
> - chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
> - spin_unlock(_lock);
> -
> - if (chip->dev_num >= TPM_NUM_DEVICES) {
> + mutex_lock(_lock);
> + rc = idr_alloc(_nums_idr, NULL, 0, TPM_NUM_DEVICES, GFP_KERNEL);
> + mutex_unlock(_lock);
> + if (rc < 0) {
>   dev_err(dev, "No available tpm device numbers\n");
>   kfree(chip);
> - return ERR_PTR(-ENOMEM);
> + return ERR_PTR(rc);
>   }
> -
> - set_bit(chip->dev_num, dev_mask);
> + chip->dev_num = rc;
>  
>   device_initialize(>dev);
>  
> @@ -242,19 +249,28 @@ static int tpm_add_char_device(struct tpm_chip *chip)
>   return rc;
>   }
>  
> + /* Make the chip available. */
> + mutex_lock(_lock);
> + idr_replace(_nums_idr, chip, chip->dev_num);
> + mutex_unlock(_lock);
> +
>   return rc;
>  }
>  
>  static void tpm_del_char_device(struct tpm_chip *chip)
>  {
>   cdev_del(>cdev);
> + device_del(>dev);
> +
> + /* Make the chip unavailable. */
> + mutex_lock(_lock);
> + idr_replace(_nums_idr, NULL, chip->dev_num);
> + mutex_unlock(_lock);
>  
>   /* Make the driver uncallable. */
>   down_write(>ops_sem);
>   chip->ops = NULL;
>   up_write(>ops_sem);
> -
> - device_del(>dev);
>  }
>  
>  static int tpm1_chip_register(struct tpm_chip *chip)
> @@ -309,11 +325,6 @@ int tpm_chip_register(struct tpm_chip *chip)
>   if (rc)
>   goto 

Re: [PATCH v8 06/10] tpm: Replace device number bitmap with IDR

2016-03-14 Thread Jarkko Sakkinen
On Sun, Mar 13, 2016 at 06:54:36PM -0400, Stefan Berger wrote:
> Replace the device number bitmap with IDR. Extend the number of devices we
> can create to 64k.
> Since an IDR allows us to associate a pointer with an ID, we use this now
> to rewrite tpm_chip_find_get() to simply look up the chip pointer by the
> given device ID.
> 
> Protect the IDR calls with a mutex.

I've merged patches up to this to my next branch i.e. you don't have
to carry them in the seres anymore.

/Jarkko

> Signed-off-by: Stefan Berger 
> Reviewed-by: Jason Gunthorpe 
> Reviewed-by: Jarkko Sakkinen 
> Tested-by: Jarkko Sakkinen 
> Signed-off-by: Jarkko Sakkinen 
> ---
>  drivers/char/tpm/tpm-chip.c  | 84 
> +---
>  drivers/char/tpm/tpm-interface.c |  1 +
>  drivers/char/tpm/tpm.h   |  5 +--
>  3 files changed, 48 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 5880377..f62c851 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -29,9 +29,8 @@
>  #include "tpm.h"
>  #include "tpm_eventlog.h"
>  
> -static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
> -static LIST_HEAD(tpm_chip_list);
> -static DEFINE_SPINLOCK(driver_lock);
> +DEFINE_IDR(dev_nums_idr);
> +static DEFINE_MUTEX(idr_lock);
>  
>  struct class *tpm_class;
>  dev_t tpm_devt;
> @@ -88,20 +87,30 @@ EXPORT_SYMBOL_GPL(tpm_put_ops);
>*/
>  struct tpm_chip *tpm_chip_find_get(int chip_num)
>  {
> - struct tpm_chip *pos, *chip = NULL;
> + struct tpm_chip *chip, *res = NULL;
> + int chip_prev;
> +
> + mutex_lock(_lock);
> +
> + if (chip_num == TPM_ANY_NUM) {
> + chip_num = 0;
> + do {
> + chip_prev = chip_num;
> + chip = idr_get_next(_nums_idr, _num);
> + if (chip && !tpm_try_get_ops(chip)) {
> + res = chip;
> + break;
> + }
> + } while (chip_prev != chip_num);
> + } else {
> + chip = idr_find_slowpath(_nums_idr, chip_num);
> + if (chip && !tpm_try_get_ops(chip))
> + res = chip;
> + }
>  
> - rcu_read_lock();
> - list_for_each_entry_rcu(pos, _chip_list, list) {
> - if (chip_num != TPM_ANY_NUM && chip_num != pos->dev_num)
> - continue;
> + mutex_unlock(_lock);
>  
> - /* rcu prevents chip from being free'd */
> - if (!tpm_try_get_ops(pos))
> - chip = pos;
> - break;
> - }
> - rcu_read_unlock();
> - return chip;
> + return res;
>  }
>  
>  /**
> @@ -114,9 +123,10 @@ static void tpm_dev_release(struct device *dev)
>  {
>   struct tpm_chip *chip = container_of(dev, struct tpm_chip, dev);
>  
> - spin_lock(_lock);
> - clear_bit(chip->dev_num, dev_mask);
> - spin_unlock(_lock);
> + mutex_lock(_lock);
> + idr_remove(_nums_idr, chip->dev_num);
> + mutex_unlock(_lock);
> +
>   kfree(chip);
>  }
>  
> @@ -142,21 +152,18 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
>  
>   mutex_init(>tpm_mutex);
>   init_rwsem(>ops_sem);
> - INIT_LIST_HEAD(>list);
>  
>   chip->ops = ops;
>  
> - spin_lock(_lock);
> - chip->dev_num = find_first_zero_bit(dev_mask, TPM_NUM_DEVICES);
> - spin_unlock(_lock);
> -
> - if (chip->dev_num >= TPM_NUM_DEVICES) {
> + mutex_lock(_lock);
> + rc = idr_alloc(_nums_idr, NULL, 0, TPM_NUM_DEVICES, GFP_KERNEL);
> + mutex_unlock(_lock);
> + if (rc < 0) {
>   dev_err(dev, "No available tpm device numbers\n");
>   kfree(chip);
> - return ERR_PTR(-ENOMEM);
> + return ERR_PTR(rc);
>   }
> -
> - set_bit(chip->dev_num, dev_mask);
> + chip->dev_num = rc;
>  
>   device_initialize(>dev);
>  
> @@ -242,19 +249,28 @@ static int tpm_add_char_device(struct tpm_chip *chip)
>   return rc;
>   }
>  
> + /* Make the chip available. */
> + mutex_lock(_lock);
> + idr_replace(_nums_idr, chip, chip->dev_num);
> + mutex_unlock(_lock);
> +
>   return rc;
>  }
>  
>  static void tpm_del_char_device(struct tpm_chip *chip)
>  {
>   cdev_del(>cdev);
> + device_del(>dev);
> +
> + /* Make the chip unavailable. */
> + mutex_lock(_lock);
> + idr_replace(_nums_idr, NULL, chip->dev_num);
> + mutex_unlock(_lock);
>  
>   /* Make the driver uncallable. */
>   down_write(>ops_sem);
>   chip->ops = NULL;
>   up_write(>ops_sem);
> -
> - device_del(>dev);
>  }
>  
>  static int tpm1_chip_register(struct tpm_chip *chip)
> @@ -309,11 +325,6 @@ int tpm_chip_register(struct tpm_chip *chip)
>   if (rc)
>   goto out_err;
>  
> - /* Make the chip available. */
> - spin_lock(_lock);
> - list_add_tail_rcu(>list, _chip_list);
> - spin_unlock(_lock);
> -
>   

Re: coccinelle: generalized removal of unnecessary pointer casts?

2016-03-14 Thread Julia Lawall


On Mon, 14 Mar 2016, Joe Perches wrote:

> On Mon, 2016-03-14 at 21:43 +0100, Julia Lawall wrote:
> > On Mon, 14 Mar 2016, Joe Perches wrote:
> > > I wrote a little cocci script to remove unnecessary
> > > casts for memset and memcpy (below) and tested it on
> > > linux kernel's drivers/staging/ directory.
> > > 
> > > For instance, when dst and src are already pointers:
> > > 
> > > - memcpy((u8 *)dst, (u8 *)src, r8712_get_wlan_bssid_ex_sz(src));
> > > + memcpy(dst, src, r8712_get_wlan_bssid_ex_sz(src));
> > > 
> > > It works ok, (it doesn't remove unnecessary parentheses
> > > around the pointers) but it makes me wonder if there's a
> > > generalized spatch mechanism to remove casts when an
> > > arbitrary function takes a void * in any argument
> > > position and a call to that function uses a cast of a
> > > pointer to any pointer type for that argument.
> > > 
> > > $ cat remove_mem_casts.cocci 
> > > @@
> > > type t;
> > > t *p;
> > > type v;
> > > expression e1;
> > > expression e2;
> > > @@
> > > 
> > > - memset((v*)p, e1, e2)
> > > + memset(p, e1, e2)
> > > 
> > > @@
> > > type t;
> > > t *p;
> > > type v;
> > > expression e1;
> > > expression e2;
> > > @@
> > > 
> > > - memcpy((v*)p, e1, e2)
> > > + memcpy(p, e1, e2)
> > > 
> > > @@
> > > type t;
> > > t *p;
> > > type v;
> > > expression e1;
> > > expression e2;
> > > @@
> > > 
> > > - memcpy(e1, (v*)p, e2)
> > > + memcpy(e1, p, e2)
> > > 
> > > @@
> > > type t1;
> > > type t2;
> > > t1 *p1;
> > > t2 *p2;
> > > type v1;
> > > type v2;
> > > expression e1;
> > > @@
> > > 
> > > - memcpy((v1*)p1, (v2*)p2, e1)
> > > + memcpy(p1, p2, e1)
> > 
> > This should do everything:
> > 
> > @@
> > identifier f;
> > expression *e;
> > type T;
> > @@
> > 
> > f(...,
> > - (T *)(
> >   e
> > - )
> >   ,...)
> > 
> > @@
> > identifier f;
> > expression *e;
> > type T;
> > @@
> > 
> > f(...,
> > - (T *)
> >   e
> >   ,...)
> > 
> > julia
> 
> Hi Julia,
> 
> I think your proposed script is not correct.
> The function must take a void * argument.
> There's no validation of that here.

OK, that could be added, but I wonder why it is necessary?  Isn't one 
pointer type just as good as any other, since the value will just get 
casted to the pointer type of the parameter in the end anyway?

julia

Re: coccinelle: generalized removal of unnecessary pointer casts?

2016-03-14 Thread Julia Lawall


On Mon, 14 Mar 2016, Joe Perches wrote:

> On Mon, 2016-03-14 at 21:43 +0100, Julia Lawall wrote:
> > On Mon, 14 Mar 2016, Joe Perches wrote:
> > > I wrote a little cocci script to remove unnecessary
> > > casts for memset and memcpy (below) and tested it on
> > > linux kernel's drivers/staging/ directory.
> > > 
> > > For instance, when dst and src are already pointers:
> > > 
> > > - memcpy((u8 *)dst, (u8 *)src, r8712_get_wlan_bssid_ex_sz(src));
> > > + memcpy(dst, src, r8712_get_wlan_bssid_ex_sz(src));
> > > 
> > > It works ok, (it doesn't remove unnecessary parentheses
> > > around the pointers) but it makes me wonder if there's a
> > > generalized spatch mechanism to remove casts when an
> > > arbitrary function takes a void * in any argument
> > > position and a call to that function uses a cast of a
> > > pointer to any pointer type for that argument.
> > > 
> > > $ cat remove_mem_casts.cocci 
> > > @@
> > > type t;
> > > t *p;
> > > type v;
> > > expression e1;
> > > expression e2;
> > > @@
> > > 
> > > - memset((v*)p, e1, e2)
> > > + memset(p, e1, e2)
> > > 
> > > @@
> > > type t;
> > > t *p;
> > > type v;
> > > expression e1;
> > > expression e2;
> > > @@
> > > 
> > > - memcpy((v*)p, e1, e2)
> > > + memcpy(p, e1, e2)
> > > 
> > > @@
> > > type t;
> > > t *p;
> > > type v;
> > > expression e1;
> > > expression e2;
> > > @@
> > > 
> > > - memcpy(e1, (v*)p, e2)
> > > + memcpy(e1, p, e2)
> > > 
> > > @@
> > > type t1;
> > > type t2;
> > > t1 *p1;
> > > t2 *p2;
> > > type v1;
> > > type v2;
> > > expression e1;
> > > @@
> > > 
> > > - memcpy((v1*)p1, (v2*)p2, e1)
> > > + memcpy(p1, p2, e1)
> > 
> > This should do everything:
> > 
> > @@
> > identifier f;
> > expression *e;
> > type T;
> > @@
> > 
> > f(...,
> > - (T *)(
> >   e
> > - )
> >   ,...)
> > 
> > @@
> > identifier f;
> > expression *e;
> > type T;
> > @@
> > 
> > f(...,
> > - (T *)
> >   e
> >   ,...)
> > 
> > julia
> 
> Hi Julia,
> 
> I think your proposed script is not correct.
> The function must take a void * argument.
> There's no validation of that here.

OK, that could be added, but I wonder why it is necessary?  Isn't one 
pointer type just as good as any other, since the value will just get 
casted to the pointer type of the parameter in the end anyway?

julia

[GIT PULL] remoteproc updates for v4.6

2016-03-14 Thread Bjorn Andersson
The following changes since commit 92e963f50fc74041b5e9e744c330dca48e04f08d:

  Linux 4.5-rc1 (2016-01-24 13:06:47 -0800)

are available in the git repository at:

  git://github.com/andersson/remoteproc.git tags/rproc-v4.6

for you to fetch changes up to 69ae9895d3fed49ee830c491b3d51c72966a8a4c:

  MAINTAINERS: Add co-maintainer for remoteproc subsystems (2016-03-10 12:20:59 
+0700)


remoteproc updates for v4.6

New driver for controlling ST's remote processors and a couple of minor
fixes. Also includes the addition of myself as co-maintainer.


Bjorn Andersson (1):
  MAINTAINERS: Add co-maintainer for remoteproc subsystems

Dave Gerlach (1):
  remoteproc/wkup_m3: Use MODULE_DEVICE_TABLE to export alias

Lee Jones (4):
  remoteproc: debugfs: Return error on invalid 'count' value
  remoteproc: dt: Provide bindings for ST's Remote Processor Controller 
driver
  remoteproc: debugfs: Add ability to boot remote processor using debugfs
  remoteproc: Supply controller driver for ST's Remote Processors

Stefan Agner (1):
  remoteproc: report error if resource table doesn't exist

 .../devicetree/bindings/remoteproc/st-rproc.txt|  41 +++
 MAINTAINERS|   3 +
 drivers/remoteproc/Kconfig |   9 +
 drivers/remoteproc/Makefile|   1 +
 drivers/remoteproc/remoteproc_core.c   |   4 +-
 drivers/remoteproc/remoteproc_debugfs.c|  36 ++-
 drivers/remoteproc/st_remoteproc.c | 297 +
 drivers/remoteproc/wkup_m3_rproc.c |   1 +
 8 files changed, 390 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/st-rproc.txt
 create mode 100644 drivers/remoteproc/st_remoteproc.c


[GIT PULL] remoteproc updates for v4.6

2016-03-14 Thread Bjorn Andersson
The following changes since commit 92e963f50fc74041b5e9e744c330dca48e04f08d:

  Linux 4.5-rc1 (2016-01-24 13:06:47 -0800)

are available in the git repository at:

  git://github.com/andersson/remoteproc.git tags/rproc-v4.6

for you to fetch changes up to 69ae9895d3fed49ee830c491b3d51c72966a8a4c:

  MAINTAINERS: Add co-maintainer for remoteproc subsystems (2016-03-10 12:20:59 
+0700)


remoteproc updates for v4.6

New driver for controlling ST's remote processors and a couple of minor
fixes. Also includes the addition of myself as co-maintainer.


Bjorn Andersson (1):
  MAINTAINERS: Add co-maintainer for remoteproc subsystems

Dave Gerlach (1):
  remoteproc/wkup_m3: Use MODULE_DEVICE_TABLE to export alias

Lee Jones (4):
  remoteproc: debugfs: Return error on invalid 'count' value
  remoteproc: dt: Provide bindings for ST's Remote Processor Controller 
driver
  remoteproc: debugfs: Add ability to boot remote processor using debugfs
  remoteproc: Supply controller driver for ST's Remote Processors

Stefan Agner (1):
  remoteproc: report error if resource table doesn't exist

 .../devicetree/bindings/remoteproc/st-rproc.txt|  41 +++
 MAINTAINERS|   3 +
 drivers/remoteproc/Kconfig |   9 +
 drivers/remoteproc/Makefile|   1 +
 drivers/remoteproc/remoteproc_core.c   |   4 +-
 drivers/remoteproc/remoteproc_debugfs.c|  36 ++-
 drivers/remoteproc/st_remoteproc.c | 297 +
 drivers/remoteproc/wkup_m3_rproc.c |   1 +
 8 files changed, 390 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/st-rproc.txt
 create mode 100644 drivers/remoteproc/st_remoteproc.c


Re: [PATCH V7 00/12] Add T210 support in Tegra soctherm

2016-03-14 Thread Wei Ni


On 2016年03月15日 05:01, Eduardo Valentin wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Mar 11, 2016 at 11:09:11AM +0800, Wei Ni wrote:
>> This patchset adds following functions for tegra_soctherm driver:
>> 1. add T210 support.
> 
> It would be good to update the compatible string in the binding
> documentation.

The commit "193c9d23a0f0 Documentation: DT bindings: add more Tegra chip
compatible strings" already add compatible string for Tegra210.

> 
>> 2. export debugfs to show some registers.
>> 3. add thermtrip funciton.
>> 4. add suspend/resume function.
> 
> Thanks for your effort in keeping up with upstream. I think overall the
> series is good, I had only very few comments (in response to each
> patch).

Thanks for your review. I will check these comments, and update my patches in
next few days.

Wei.

> 
> BR,
> 
> Eduardo Valentin
> 
> 
>>
>>
>> -- 
>> 1.9.1
>>
> 
> * Unknown Key
> * 0x7DA4E256
> 


Re: [PATCH V7 00/12] Add T210 support in Tegra soctherm

2016-03-14 Thread Wei Ni


On 2016年03月15日 05:01, Eduardo Valentin wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Mar 11, 2016 at 11:09:11AM +0800, Wei Ni wrote:
>> This patchset adds following functions for tegra_soctherm driver:
>> 1. add T210 support.
> 
> It would be good to update the compatible string in the binding
> documentation.

The commit "193c9d23a0f0 Documentation: DT bindings: add more Tegra chip
compatible strings" already add compatible string for Tegra210.

> 
>> 2. export debugfs to show some registers.
>> 3. add thermtrip funciton.
>> 4. add suspend/resume function.
> 
> Thanks for your effort in keeping up with upstream. I think overall the
> series is good, I had only very few comments (in response to each
> patch).

Thanks for your review. I will check these comments, and update my patches in
next few days.

Wei.

> 
> BR,
> 
> Eduardo Valentin
> 
> 
>>
>>
>> -- 
>> 1.9.1
>>
> 
> * Unknown Key
> * 0x7DA4E256
> 


Re: [lkp] [drm/i915] 0bbca274a3: 3.6% piglit.time.elapsed_time

2016-03-14 Thread Ye Xiaolong
On Mon, Mar 14, 2016 at 03:37:51PM +0200, Ville Syrjälä wrote:
>On Mon, Mar 14, 2016 at 10:14:37AM +0800, kernel test robot wrote:
>> FYI, we noticed that piglit.time.elapsed_time 3.6% regression on
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>> commit 0bbca274a31c2366414d8d3f95e03d1c4674d93f ("drm/i915: Actually retry 
>> with bit-banging after GMBUS timeout")
>
>Increase in system time is somewhat expected, since we'll do more
>bit banging now. Any chance of comparison against bffce907d640^
>aka. 0aeb904888f5 ("drm/i915: simplify gmbus xfer error checks") ?
>I would hope that the result would be more or less the same as with
>0bbca274a31c ("drm/i915: Actually retry with bit-banging after GMBUS timeout")

Hi, ville

FYI, here is the piglit test result of eb904888f52f48 on "lkp-u410: Ivy Bridge 
4G".

eb904888f52f48  0bbca274a31c2366414d8d3f95
  --
 %stddev %change %stddev
 \  |\
59 ±  0% -73.1% 16 ±  0%  
piglit.time.percent_of_cpu_this_job_got
  4640 ±  1%+209.6%  14363 ±  0%  
piglit.time.voluntary_context_switches
 62.75 ±  0%+140.7% 151.03 ±  0%  piglit.time.elapsed_time
 62.75 ±  0%+140.7% 151.03 ±  0%  piglit.time.elapsed_time.max
 33.61 ±  0% -40.3%  20.05 ±  0%  piglit.time.system_time

Thanks,
Xiaolong.

>
>> 
>> 
>> =
>> compiler/group/kconfig/rootfs/tbox_group/testcase:
>>   gcc-4.9/igt-039/x86_64-rhel/debian-x86_64-2015-02-07.cgz/snb-black/piglit
>> 
>> commit: 
>>   2f791908a70e95768596f5bb9e6de4f441d7bf13
>>   0bbca274a31c2366414d8d3f95e03d1c4674d93f
>> 
>> 2f791908a70e9576 0bbca274a31c2366414d8d3f95 
>>  -- 
>>  %stddev %change %stddev
>>  \  |\  
>> 188.79 ±  0%  +3.6% 195.58 ±  0%  piglit.time.elapsed_time
>> 188.79 ±  0%  +3.6% 195.58 ±  0%  piglit.time.elapsed_time.max
>>   5.00 ±  0% +60.0%   8.00 ±  0%  
>> piglit.time.percent_of_cpu_this_job_got
>>   7.86 ±  0% +83.5%  14.42 ±  0%  piglit.time.system_time
>>   5.00 ±  0% +60.0%   8.00 ±  0%  
>> time.percent_of_cpu_this_job_got
>>   7.86 ±  0% +83.5%  14.42 ±  0%  time.system_time
>> 
>> =
>> compiler/group/kconfig/rootfs/tbox_group/testcase:
>>   gcc-4.9/igt-036/x86_64-rhel/debian-x86_64-2015-02-07.cgz/lkp-u410/piglit
>> 
>> commit: 
>>   2f791908a70e95768596f5bb9e6de4f441d7bf13
>>   0bbca274a31c2366414d8d3f95e03d1c4674d93f
>> 
>> 2f791908a70e9576 0bbca274a31c2366414d8d3f95 
>>  -- 
>>  %stddev %change %stddev
>>  \  |\  
>> 141.15 ±  0%  +6.9% 150.94 ±  0%  piglit.time.elapsed_time
>> 141.15 ±  0%  +6.9% 150.94 ±  0%  piglit.time.elapsed_time.max
>>  10.00 ±  0% +60.0%  16.00 ±  0%  
>> piglit.time.percent_of_cpu_this_job_got
>>  10.61 ±  0% +88.8%  20.04 ±  0%  piglit.time.system_time
>>  10.00 ±  0% +60.0%  16.00 ±  0%  
>> time.percent_of_cpu_this_job_got
>>  10.61 ±  0% +88.8%  20.04 ±  0%  time.system_time
>> 
>> 
>> snb-black: Sandy Bridge
>> Memory: 8G
>> 
>> lkp-u410: Ivy Bridge
>> Memory: 4G
>> 
>> 
>> 
>>  piglit.time.system_time
>> 
>>   21 ++-+
>>   20 O+ O  O  O  O  O  O  O  O  O  O  O  O O  O  O  O  O  O  O  O  O  O |
>>  |  |
>>   19 ++ |
>>   18 ++ |
>>   17 ++ |
>>   16 ++ |
>>  |  |
>>   15 ++ |
>>   14 ++ |
>>   13 ++ |
>>   12 ++ |
>>  |  |
>>   11 *+.*..*..*..*..*..*..*..*..*..*..*..*.*..*..*..*..*..*..*..*..*..*..*..*
>>   10 ++-+
>> 
>> 
>>  piglit.time.percent_of_cpu_this_job_got
>> 
>>   16 O+-O--O--O--O--O--O--O--O--O--O--O--O-O--O--O--O--O--O--O--O--O--O-+
>>  |   

Re: [lkp] [drm/i915] 0bbca274a3: 3.6% piglit.time.elapsed_time

2016-03-14 Thread Ye Xiaolong
On Mon, Mar 14, 2016 at 03:37:51PM +0200, Ville Syrjälä wrote:
>On Mon, Mar 14, 2016 at 10:14:37AM +0800, kernel test robot wrote:
>> FYI, we noticed that piglit.time.elapsed_time 3.6% regression on
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>> commit 0bbca274a31c2366414d8d3f95e03d1c4674d93f ("drm/i915: Actually retry 
>> with bit-banging after GMBUS timeout")
>
>Increase in system time is somewhat expected, since we'll do more
>bit banging now. Any chance of comparison against bffce907d640^
>aka. 0aeb904888f5 ("drm/i915: simplify gmbus xfer error checks") ?
>I would hope that the result would be more or less the same as with
>0bbca274a31c ("drm/i915: Actually retry with bit-banging after GMBUS timeout")

Hi, ville

FYI, here is the piglit test result of eb904888f52f48 on "lkp-u410: Ivy Bridge 
4G".

eb904888f52f48  0bbca274a31c2366414d8d3f95
  --
 %stddev %change %stddev
 \  |\
59 ±  0% -73.1% 16 ±  0%  
piglit.time.percent_of_cpu_this_job_got
  4640 ±  1%+209.6%  14363 ±  0%  
piglit.time.voluntary_context_switches
 62.75 ±  0%+140.7% 151.03 ±  0%  piglit.time.elapsed_time
 62.75 ±  0%+140.7% 151.03 ±  0%  piglit.time.elapsed_time.max
 33.61 ±  0% -40.3%  20.05 ±  0%  piglit.time.system_time

Thanks,
Xiaolong.

>
>> 
>> 
>> =
>> compiler/group/kconfig/rootfs/tbox_group/testcase:
>>   gcc-4.9/igt-039/x86_64-rhel/debian-x86_64-2015-02-07.cgz/snb-black/piglit
>> 
>> commit: 
>>   2f791908a70e95768596f5bb9e6de4f441d7bf13
>>   0bbca274a31c2366414d8d3f95e03d1c4674d93f
>> 
>> 2f791908a70e9576 0bbca274a31c2366414d8d3f95 
>>  -- 
>>  %stddev %change %stddev
>>  \  |\  
>> 188.79 ±  0%  +3.6% 195.58 ±  0%  piglit.time.elapsed_time
>> 188.79 ±  0%  +3.6% 195.58 ±  0%  piglit.time.elapsed_time.max
>>   5.00 ±  0% +60.0%   8.00 ±  0%  
>> piglit.time.percent_of_cpu_this_job_got
>>   7.86 ±  0% +83.5%  14.42 ±  0%  piglit.time.system_time
>>   5.00 ±  0% +60.0%   8.00 ±  0%  
>> time.percent_of_cpu_this_job_got
>>   7.86 ±  0% +83.5%  14.42 ±  0%  time.system_time
>> 
>> =
>> compiler/group/kconfig/rootfs/tbox_group/testcase:
>>   gcc-4.9/igt-036/x86_64-rhel/debian-x86_64-2015-02-07.cgz/lkp-u410/piglit
>> 
>> commit: 
>>   2f791908a70e95768596f5bb9e6de4f441d7bf13
>>   0bbca274a31c2366414d8d3f95e03d1c4674d93f
>> 
>> 2f791908a70e9576 0bbca274a31c2366414d8d3f95 
>>  -- 
>>  %stddev %change %stddev
>>  \  |\  
>> 141.15 ±  0%  +6.9% 150.94 ±  0%  piglit.time.elapsed_time
>> 141.15 ±  0%  +6.9% 150.94 ±  0%  piglit.time.elapsed_time.max
>>  10.00 ±  0% +60.0%  16.00 ±  0%  
>> piglit.time.percent_of_cpu_this_job_got
>>  10.61 ±  0% +88.8%  20.04 ±  0%  piglit.time.system_time
>>  10.00 ±  0% +60.0%  16.00 ±  0%  
>> time.percent_of_cpu_this_job_got
>>  10.61 ±  0% +88.8%  20.04 ±  0%  time.system_time
>> 
>> 
>> snb-black: Sandy Bridge
>> Memory: 8G
>> 
>> lkp-u410: Ivy Bridge
>> Memory: 4G
>> 
>> 
>> 
>>  piglit.time.system_time
>> 
>>   21 ++-+
>>   20 O+ O  O  O  O  O  O  O  O  O  O  O  O O  O  O  O  O  O  O  O  O  O |
>>  |  |
>>   19 ++ |
>>   18 ++ |
>>   17 ++ |
>>   16 ++ |
>>  |  |
>>   15 ++ |
>>   14 ++ |
>>   13 ++ |
>>   12 ++ |
>>  |  |
>>   11 *+.*..*..*..*..*..*..*..*..*..*..*..*.*..*..*..*..*..*..*..*..*..*..*..*
>>   10 ++-+
>> 
>> 
>>  piglit.time.percent_of_cpu_this_job_got
>> 
>>   16 O+-O--O--O--O--O--O--O--O--O--O--O--O-O--O--O--O--O--O--O--O--O--O-+
>>  |   

Re: [PATCH 12/13] thermal: convert tegra_thermal to use devm_thermal_zone_of_sensor_register

2016-03-14 Thread Wei Ni


On 2016年03月15日 05:16, Eduardo Valentin wrote:
> * PGP Signed by an unknown key
> 
> On Thu, Mar 10, 2016 at 04:46:55PM +0800, Wei Ni wrote:
>>
>>
>> On 2016年03月10日 05:35, Eduardo Valentin wrote:
>>> This changes the driver to use the devm_ version
>>> of thermal_zone_of_sensor_register and cleans
>>> up the  local points and unregister calls.
>>>
>>> Cc: Zhang Rui 
>>> Cc: Stephen Warren 
>>> Cc: Thierry Reding 
>>> Cc: Alexandre Courbot 
>>> Cc: linux...@vger.kernel.org
>>> Cc: linux-te...@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Eduardo Valentin 
>>> ---
>>>  drivers/thermal/tegra_soctherm.c | 31 +--
>>>  1 file changed, 9 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/thermal/tegra_soctherm.c 
>>> b/drivers/thermal/tegra_soctherm.c
>>> index 74ea576..0018ccd 100644
>>> --- a/drivers/thermal/tegra_soctherm.c
>>> +++ b/drivers/thermal/tegra_soctherm.c
>>> @@ -168,7 +168,7 @@ struct tegra_soctherm {
>>> struct clk *clock_soctherm;
>>> void __iomem *regs;
>>>  
>>> -   struct thermal_zone_device *thermctl_tzs[4];
>>> +#define ZONE_NUMBER4
>>>  };
>>>  
>>>  struct tsensor_shared_calibration {
>>> @@ -342,7 +342,7 @@ static const struct thermctl_zone_desc 
>>> t124_thermctl_temp_zones[] = {
>>>  static int tegra_soctherm_probe(struct platform_device *pdev)
>>>  {
>>> struct tegra_soctherm *tegra;
>>> -   struct thermal_zone_device *tz;
>>> +   struct thermal_zone_device *z;
>>> struct tsensor_shared_calibration shared_calib;
>>> struct resource *res;
>>> unsigned int i;
>>> @@ -408,36 +408,29 @@ static int tegra_soctherm_probe(struct 
>>> platform_device *pdev)
>>>  
>>> /* Initialize thermctl sensors */
>>>  
>>> -   for (i = 0; i < ARRAY_SIZE(tegra->thermctl_tzs); ++i) {
>>> +   for (i = 0; i < ZONE_NUMBER; ++i) {
>>> struct tegra_thermctl_zone *zone =
>>> devm_kzalloc(>dev, sizeof(*zone), GFP_KERNEL);
>>> if (!zone) {
>>> err = -ENOMEM;
>>> -   goto unregister_tzs;
>>> +   goto disable_clocks;
>>> }
>>>  
>>> zone->reg = tegra->regs + t124_thermctl_temp_zones[i].offset;
>>> zone->shift = t124_thermctl_temp_zones[i].shift;
>>>  
>>> -   tz = thermal_zone_of_sensor_register(>dev, i, zone,
>>> -_of_thermal_ops);
>>> -   if (IS_ERR(tz)) {
>>> -   err = PTR_ERR(tz);
>>> +   z = devm_thermal_zone_of_sensor_register(>dev, i, zone,
>>
>> I prefer to still use "tz", it seems this line isn't over 80 characters, or 
>> we
>> can add newline.
> 
> 
> Yeah,
> 
> 
>>
>>> +_of_thermal_ops);
> 
> CHECK: Alignment should match open parenthesis
> #423: FILE: drivers/thermal/tegra_soctherm.c:423:
> + tz = devm_thermal_zone_of_sensor_register(>dev, i, zone,
> +  _of_thermal_ops);
> 
> and if you align it, then, you get the warning:
> WARNING: line over 80 characters
> #423: FILE: drivers/thermal/tegra_soctherm.c:423:
> +   
> _of_thermal_ops);
> 
> And I did not want to add either of the above to the driver. 
> 
> But if you prefer tz over z, we can keep the first (check) then.
> 
> What do you prefer?

Hmm, I see.
Ok, it's better to use "z" to resolve it.

Wei.
> 
> 
> 
> 
> 
> * Unknown Key
> * 0x7DA4E256
> 


Re: [PATCH 12/13] thermal: convert tegra_thermal to use devm_thermal_zone_of_sensor_register

2016-03-14 Thread Wei Ni


On 2016年03月15日 05:16, Eduardo Valentin wrote:
> * PGP Signed by an unknown key
> 
> On Thu, Mar 10, 2016 at 04:46:55PM +0800, Wei Ni wrote:
>>
>>
>> On 2016年03月10日 05:35, Eduardo Valentin wrote:
>>> This changes the driver to use the devm_ version
>>> of thermal_zone_of_sensor_register and cleans
>>> up the  local points and unregister calls.
>>>
>>> Cc: Zhang Rui 
>>> Cc: Stephen Warren 
>>> Cc: Thierry Reding 
>>> Cc: Alexandre Courbot 
>>> Cc: linux...@vger.kernel.org
>>> Cc: linux-te...@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Eduardo Valentin 
>>> ---
>>>  drivers/thermal/tegra_soctherm.c | 31 +--
>>>  1 file changed, 9 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/thermal/tegra_soctherm.c 
>>> b/drivers/thermal/tegra_soctherm.c
>>> index 74ea576..0018ccd 100644
>>> --- a/drivers/thermal/tegra_soctherm.c
>>> +++ b/drivers/thermal/tegra_soctherm.c
>>> @@ -168,7 +168,7 @@ struct tegra_soctherm {
>>> struct clk *clock_soctherm;
>>> void __iomem *regs;
>>>  
>>> -   struct thermal_zone_device *thermctl_tzs[4];
>>> +#define ZONE_NUMBER4
>>>  };
>>>  
>>>  struct tsensor_shared_calibration {
>>> @@ -342,7 +342,7 @@ static const struct thermctl_zone_desc 
>>> t124_thermctl_temp_zones[] = {
>>>  static int tegra_soctherm_probe(struct platform_device *pdev)
>>>  {
>>> struct tegra_soctherm *tegra;
>>> -   struct thermal_zone_device *tz;
>>> +   struct thermal_zone_device *z;
>>> struct tsensor_shared_calibration shared_calib;
>>> struct resource *res;
>>> unsigned int i;
>>> @@ -408,36 +408,29 @@ static int tegra_soctherm_probe(struct 
>>> platform_device *pdev)
>>>  
>>> /* Initialize thermctl sensors */
>>>  
>>> -   for (i = 0; i < ARRAY_SIZE(tegra->thermctl_tzs); ++i) {
>>> +   for (i = 0; i < ZONE_NUMBER; ++i) {
>>> struct tegra_thermctl_zone *zone =
>>> devm_kzalloc(>dev, sizeof(*zone), GFP_KERNEL);
>>> if (!zone) {
>>> err = -ENOMEM;
>>> -   goto unregister_tzs;
>>> +   goto disable_clocks;
>>> }
>>>  
>>> zone->reg = tegra->regs + t124_thermctl_temp_zones[i].offset;
>>> zone->shift = t124_thermctl_temp_zones[i].shift;
>>>  
>>> -   tz = thermal_zone_of_sensor_register(>dev, i, zone,
>>> -_of_thermal_ops);
>>> -   if (IS_ERR(tz)) {
>>> -   err = PTR_ERR(tz);
>>> +   z = devm_thermal_zone_of_sensor_register(>dev, i, zone,
>>
>> I prefer to still use "tz", it seems this line isn't over 80 characters, or 
>> we
>> can add newline.
> 
> 
> Yeah,
> 
> 
>>
>>> +_of_thermal_ops);
> 
> CHECK: Alignment should match open parenthesis
> #423: FILE: drivers/thermal/tegra_soctherm.c:423:
> + tz = devm_thermal_zone_of_sensor_register(>dev, i, zone,
> +  _of_thermal_ops);
> 
> and if you align it, then, you get the warning:
> WARNING: line over 80 characters
> #423: FILE: drivers/thermal/tegra_soctherm.c:423:
> +   
> _of_thermal_ops);
> 
> And I did not want to add either of the above to the driver. 
> 
> But if you prefer tz over z, we can keep the first (check) then.
> 
> What do you prefer?

Hmm, I see.
Ok, it's better to use "z" to resolve it.

Wei.
> 
> 
> 
> 
> 
> * Unknown Key
> * 0x7DA4E256
> 


Re: linux-next: manual merge of the aio tree with the vfs tree

2016-03-14 Thread Al Viro
On Mon, Mar 14, 2016 at 11:24:38AM -0400, Benjamin LaHaise wrote:
> On Mon, Mar 14, 2016 at 08:35:23AM +0100, Christoph Hellwig wrote:
> > The aio changes have either been reviewed negatively or not at all.  That 
> > tree should be dropped.
> 
> That isn't solely your decision.  If you have comments, please provide 
> constructive feedback, as there are users and use-cases that need this 
> kind of functionality.

"This kind of functionality" being what, exactly?  Bypassing code review?
It's not that you've made trivial mistakes; everyone does from time to time.
But failing to post patches with non-trivial interactions outside of the
subsystem you are familiar with (be it on fsdevel or privately to people who
work with the areas involved) *AND* failing to recognize that the lack
of review might be a problem even after having been explicitly told so...

For fuck sake, you should know better.  You are not a newbie with a pet set
of half-baked patches for his pet application, refering to (unspecified)
"users that need this kind of functionality" and getting very surprised when
those mean, rude folks on development lists inform them that code review is
more than just a good idea.


Re: linux-next: manual merge of the aio tree with the vfs tree

2016-03-14 Thread Al Viro
On Mon, Mar 14, 2016 at 11:24:38AM -0400, Benjamin LaHaise wrote:
> On Mon, Mar 14, 2016 at 08:35:23AM +0100, Christoph Hellwig wrote:
> > The aio changes have either been reviewed negatively or not at all.  That 
> > tree should be dropped.
> 
> That isn't solely your decision.  If you have comments, please provide 
> constructive feedback, as there are users and use-cases that need this 
> kind of functionality.

"This kind of functionality" being what, exactly?  Bypassing code review?
It's not that you've made trivial mistakes; everyone does from time to time.
But failing to post patches with non-trivial interactions outside of the
subsystem you are familiar with (be it on fsdevel or privately to people who
work with the areas involved) *AND* failing to recognize that the lack
of review might be a problem even after having been explicitly told so...

For fuck sake, you should know better.  You are not a newbie with a pet set
of half-baked patches for his pet application, refering to (unspecified)
"users that need this kind of functionality" and getting very surprised when
those mean, rude folks on development lists inform them that code review is
more than just a good idea.


Re: [PATCH V2 2/2] mailbox: Introduce TI message manager driver

2016-03-14 Thread Jassi Brar
On Tue, Mar 8, 2016 at 8:07 PM, Nishanth Menon  wrote:
> Jassi,
>
> On Tue, Mar 8, 2016 at 1:10 AM, Jassi Brar  wrote:
>> On Tue, Mar 8, 2016 at 2:18 AM, Nishanth Menon  wrote:
>>> On 03/07/2016 12:31 PM, Jassi Brar wrote:
 On Fri, Mar 4, 2016 at 8:05 PM, Nishanth Menon  wrote:
>>
>>> +static int ti_msgmgr_send_data(struct mbox_chan *chan, void *data)
>>> +{
>>> +   struct device *dev = chan->mbox->dev;
>>> +   struct ti_msgmgr_inst *inst = dev_get_drvdata(dev);
>>> +   const struct ti_msgmgr_desc *desc;
>>> +   struct ti_queue_inst *qinst = chan->con_priv;
>>> +   int msg_count, num_words, trail_bytes;
>>> +   struct ti_msgmgr_message *message = data;
>>> +   void __iomem *data_reg;
>>> +   u32 *word_data;
>>> +
>>> +   if (WARN_ON(!inst)) {
>>> +   dev_err(dev, "no platform drv data??\n");
>>> +   return -EINVAL;
>>> +   }
>>> +   desc = inst->desc;
>>> +
>>> +   if (desc->max_message_size < message->len) {
>>> +   dev_err(dev, "Queue %s message length %d > max %d\n",
>>> +   qinst->name, message->len, 
>>> desc->max_message_size);
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   /* Are we able to send this or not? */
>>> +   msg_count = ti_msgmgr_queue_get_num_messages(qinst);
>>> +   if (msg_count >= desc->max_messages) {
>>> +   dev_warn(dev, "Queue %s is full (%d messages)\n", 
>>> qinst->name,
>>> +msg_count);
>>> +   return -EBUSY;
>>> +   }
>> This seems fishy. mailbox api always submit 1 'complete' message to be
>> sent and checks for completion by last_tx_done() before calling
>> send_data() again. Controller drivers are not supposed to queue
>> messages - mailbox core does. So you should never be unable to send a
>> message.
>
>
> OK-> to explain this, few reasons: (queue messages check and usage of
> last_tx_done are kind of intertwined answer..
> a) we need to remember that the message manager has a shared RAM.
> multiple transmitter over other queues can be sharing the same.
> unfortunately, we dont get a threshold kind of interrupt or status that
> I am able to exploit in the current incarnation of the solution. The
> best we can do in the full system is to constrain the number of messages
> that are max pending simultaneously in each of the queue from various
> transmitters in the SoC.
> b) last_tx_done() is checked if TXDONE_BY_POLL, not if TXDONE_BY_ACK
> right? which is how this'd work since txdone_poll is false -> that is
> how we want this mechanism to work once the far end is ready for next
> message, it acks. I do see your point about being tied to protocol - I
> dont like it either.. in fact, I'd prefer that client registration
> mention what kind of handshaking is necessary, but: a) that is not how
> mailbox framework is constructed at the moment(we state txdone_poll at
> mailbox registration, not at client usage) and b) I have no real need
> for multiple clients to users of message manager who actually need
> non-ACK usage - even for the foreseeable future (at least 1 next
> generation of SoC) - if such a need does arise in the future, I will
> have to rework framework and make this capability at the registration
> time of the client - allowing each client path to use different
> mechanisms on hardware such as these that need it.
> c) message manager can actually queue more than one message(depending on
> client capability). Even though, at this point, we are not really
> capable of doing it(again from what I can see for immediate future),
> mailbox framework by checking last_tx_done forces a single message
> sequencing - which is not really exploiting the capability of the
> hardware - in theory, we should be able to queue max num messages, hit
> cpuidle and snooze away while the remote entity chomp away data at it's
> own pace and finally give us a notification back - but again, we can
> argue it is indeed protocol dependent, so setting txdone_poll to false
> actually enables that to be done in user. Again - i have no immediate
> need for any queued multiple transfer needs yet.. even if i need to, in
> the future, it can easily be done by the client by maintaining code as
> is - txdone_poll is false.
>
 All I suggest is that the controller does not queue more than 1
 message at a time, which means the controller driver allows for
 maximum possible resources taken by a message.
 The buffering is already done by the core, and if for your 'batch
 dispatch' thing the client could simply flush them to remote by

Re: [PATCH V2 2/2] mailbox: Introduce TI message manager driver

2016-03-14 Thread Jassi Brar
On Tue, Mar 8, 2016 at 8:07 PM, Nishanth Menon  wrote:
> Jassi,
>
> On Tue, Mar 8, 2016 at 1:10 AM, Jassi Brar  wrote:
>> On Tue, Mar 8, 2016 at 2:18 AM, Nishanth Menon  wrote:
>>> On 03/07/2016 12:31 PM, Jassi Brar wrote:
 On Fri, Mar 4, 2016 at 8:05 PM, Nishanth Menon  wrote:
>>
>>> +static int ti_msgmgr_send_data(struct mbox_chan *chan, void *data)
>>> +{
>>> +   struct device *dev = chan->mbox->dev;
>>> +   struct ti_msgmgr_inst *inst = dev_get_drvdata(dev);
>>> +   const struct ti_msgmgr_desc *desc;
>>> +   struct ti_queue_inst *qinst = chan->con_priv;
>>> +   int msg_count, num_words, trail_bytes;
>>> +   struct ti_msgmgr_message *message = data;
>>> +   void __iomem *data_reg;
>>> +   u32 *word_data;
>>> +
>>> +   if (WARN_ON(!inst)) {
>>> +   dev_err(dev, "no platform drv data??\n");
>>> +   return -EINVAL;
>>> +   }
>>> +   desc = inst->desc;
>>> +
>>> +   if (desc->max_message_size < message->len) {
>>> +   dev_err(dev, "Queue %s message length %d > max %d\n",
>>> +   qinst->name, message->len, 
>>> desc->max_message_size);
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   /* Are we able to send this or not? */
>>> +   msg_count = ti_msgmgr_queue_get_num_messages(qinst);
>>> +   if (msg_count >= desc->max_messages) {
>>> +   dev_warn(dev, "Queue %s is full (%d messages)\n", 
>>> qinst->name,
>>> +msg_count);
>>> +   return -EBUSY;
>>> +   }
>> This seems fishy. mailbox api always submit 1 'complete' message to be
>> sent and checks for completion by last_tx_done() before calling
>> send_data() again. Controller drivers are not supposed to queue
>> messages - mailbox core does. So you should never be unable to send a
>> message.
>
>
> OK-> to explain this, few reasons: (queue messages check and usage of
> last_tx_done are kind of intertwined answer..
> a) we need to remember that the message manager has a shared RAM.
> multiple transmitter over other queues can be sharing the same.
> unfortunately, we dont get a threshold kind of interrupt or status that
> I am able to exploit in the current incarnation of the solution. The
> best we can do in the full system is to constrain the number of messages
> that are max pending simultaneously in each of the queue from various
> transmitters in the SoC.
> b) last_tx_done() is checked if TXDONE_BY_POLL, not if TXDONE_BY_ACK
> right? which is how this'd work since txdone_poll is false -> that is
> how we want this mechanism to work once the far end is ready for next
> message, it acks. I do see your point about being tied to protocol - I
> dont like it either.. in fact, I'd prefer that client registration
> mention what kind of handshaking is necessary, but: a) that is not how
> mailbox framework is constructed at the moment(we state txdone_poll at
> mailbox registration, not at client usage) and b) I have no real need
> for multiple clients to users of message manager who actually need
> non-ACK usage - even for the foreseeable future (at least 1 next
> generation of SoC) - if such a need does arise in the future, I will
> have to rework framework and make this capability at the registration
> time of the client - allowing each client path to use different
> mechanisms on hardware such as these that need it.
> c) message manager can actually queue more than one message(depending on
> client capability). Even though, at this point, we are not really
> capable of doing it(again from what I can see for immediate future),
> mailbox framework by checking last_tx_done forces a single message
> sequencing - which is not really exploiting the capability of the
> hardware - in theory, we should be able to queue max num messages, hit
> cpuidle and snooze away while the remote entity chomp away data at it's
> own pace and finally give us a notification back - but again, we can
> argue it is indeed protocol dependent, so setting txdone_poll to false
> actually enables that to be done in user. Again - i have no immediate
> need for any queued multiple transfer needs yet.. even if i need to, in
> the future, it can easily be done by the client by maintaining code as
> is - txdone_poll is false.
>
 All I suggest is that the controller does not queue more than 1
 message at a time, which means the controller driver allows for
 maximum possible resources taken by a message.
 The buffering is already done by the core, and if for your 'batch
 dispatch' thing the client could simply flush them to remote by
 pretending it got the ack (which is no worse than simply 

Re: linux-next: Tree for Mar 14 (mips qemu failure bisected)

2016-03-14 Thread Guenter Roeck
On Mon, Mar 14, 2016 at 07:37:29AM -0700, Guenter Roeck wrote:
> On Mon, Mar 14, 2016 at 05:40:37PM +1100, Stephen Rothwell wrote:
> > Hi all,
> > 
> > Changes since 20160311:
> > 
> > The vfs tree gained a conflict against Linus' tree. I also applied a
> > patch for a known runtime bug.
> > 
> > The tip tree gained a conflict against the mips tree.
> > 
> > The aio tree still had a build failure so I removed several commits
> > from it.  It also gained a conflict against the vfs tree.
> > 
> > Non-merge commits (relative to Linus' tree): 11202
> >  8646 files changed, 426680 insertions(+), 211740 deletions(-)
> > 
> 
> To give people an idea what to expect in the merge window, here are my current
> build and runtime test results. Some of the runtime failures are due to the
> newly introduced i2c bug, but many (including the arm64 boot failures) have
> been around for a while.
> 
[ ... ]

> Qemu test results:
>   total: 96 pass: 69 fail: 27
> Failed tests:
[ ... ]
>   mips:mips_malta_smp_defconfig

I bisected this failure to commit bb11cff327e54 ("MIPS: Make smp CMP, CPS and MT
use the new generic IPI functions". Bisect log is attached.

>   mips64:smp:mips_malta64_defconfig
>   mips:mipsel_malta_smp_defconfig
>   mips:mipsel_malta64_smp_defconfig

If necessary I can repeat the bisect for those. Please let me know.

Thanks,
Guenter

---
Bisect log:

# bad: [4342eec3c5a2402ca5de3d6e56f541fe1c5171e2] Add linux-next specific files 
for 20160314
# good: [f6cede5b49e822ebc41a099fe41ab4989f64e2cb] Linux 4.5-rc7
git bisect start 'HEAD' 'v4.5-rc7'
# good: [0525c3e26ec2c43cd509433be3be25210a0154ef] Merge remote-tracking branch 
'drm-tegra/drm/tegra/for-next'
git bisect good 0525c3e26ec2c43cd509433be3be25210a0154ef
# bad: [385128a1b49762c1b9515c9f6294aeebbc55b956] Merge remote-tracking branch 
'usb-chipidea-next/ci-for-usb-next'
git bisect bad 385128a1b49762c1b9515c9f6294aeebbc55b956
# good: [dfdb27baab4fc45c9399a991270413d0fb1c694a] Merge remote-tracking branch 
'spi/for-next'
git bisect good dfdb27baab4fc45c9399a991270413d0fb1c694a
# bad: [e368d7d2a0dce6d6795ead1fc8a09bcca8a4a565] Merge branch 'timers/nohz'
git bisect bad e368d7d2a0dce6d6795ead1fc8a09bcca8a4a565
# good: [ced30bc9129777d715057d06fc8dbdfd3b81e94d] Merge tag 
'perf-core-for-mingo-20160310' of 
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core
git bisect good ced30bc9129777d715057d06fc8dbdfd3b81e94d
# bad: [656a61d4d9cbb8dfc2d007281190b2eccebad522] manual merge of mm/pkeys
git bisect bad 656a61d4d9cbb8dfc2d007281190b2eccebad522
# good: [16f7379f2da43f29d9faa2f474745e2705a3f510] Merge branch 'efi/core'
git bisect good 16f7379f2da43f29d9faa2f474745e2705a3f510
# bad: [a7fb9a8169be9a55e0cfb98346aece1b51c016fa] Merge branch 'locking/core'
git bisect bad a7fb9a8169be9a55e0cfb98346aece1b51c016fa
# good: [2a07870511829977d02609dac6450017b0419ea9] irqchip/mips-gic: Use 
gic_vpes instead of NR_CPUS
git bisect good 2a07870511829977d02609dac6450017b0419ea9
# good: [eaff0e7003cca6c2748b67ead2d4b1a8ad858fc7] locking/pvqspinlock: Move 
lock stealing count tracking code into pv_queued_spin_steal_lock()
git bisect good eaff0e7003cca6c2748b67ead2d4b1a8ad858fc7
# good: [013e379a3094ff2898f8d33cfbff1573d471ee14] tools/lib/lockdep: Fix link 
creation warning
git bisect good 013e379a3094ff2898f8d33cfbff1573d471ee14
# bad: [7eb8c99db26cc6499bfb1eba72dffc4730570752] MIPS: Delete smp-gic.c
git bisect bad 7eb8c99db26cc6499bfb1eba72dffc4730570752
# good: [fbde2d7d8290d8c642d591a471356abda2446874] MIPS: Add generic SMP IPI 
support
git bisect good fbde2d7d8290d8c642d591a471356abda2446874
# bad: [bb11cff327e54179c13446c4022ed4ed7d4871c7] MIPS: Make smp CMP, CPS and 
MT use the new generic IPI functions
git bisect bad bb11cff327e54179c13446c4022ed4ed7d4871c7
# first bad commit: [bb11cff327e54179c13446c4022ed4ed7d4871c7] MIPS: Make smp 
CMP, CPS and MT use the new generic IPI functions


Re: linux-next: Tree for Mar 14 (mips qemu failure bisected)

2016-03-14 Thread Guenter Roeck
On Mon, Mar 14, 2016 at 07:37:29AM -0700, Guenter Roeck wrote:
> On Mon, Mar 14, 2016 at 05:40:37PM +1100, Stephen Rothwell wrote:
> > Hi all,
> > 
> > Changes since 20160311:
> > 
> > The vfs tree gained a conflict against Linus' tree. I also applied a
> > patch for a known runtime bug.
> > 
> > The tip tree gained a conflict against the mips tree.
> > 
> > The aio tree still had a build failure so I removed several commits
> > from it.  It also gained a conflict against the vfs tree.
> > 
> > Non-merge commits (relative to Linus' tree): 11202
> >  8646 files changed, 426680 insertions(+), 211740 deletions(-)
> > 
> 
> To give people an idea what to expect in the merge window, here are my current
> build and runtime test results. Some of the runtime failures are due to the
> newly introduced i2c bug, but many (including the arm64 boot failures) have
> been around for a while.
> 
[ ... ]

> Qemu test results:
>   total: 96 pass: 69 fail: 27
> Failed tests:
[ ... ]
>   mips:mips_malta_smp_defconfig

I bisected this failure to commit bb11cff327e54 ("MIPS: Make smp CMP, CPS and MT
use the new generic IPI functions". Bisect log is attached.

>   mips64:smp:mips_malta64_defconfig
>   mips:mipsel_malta_smp_defconfig
>   mips:mipsel_malta64_smp_defconfig

If necessary I can repeat the bisect for those. Please let me know.

Thanks,
Guenter

---
Bisect log:

# bad: [4342eec3c5a2402ca5de3d6e56f541fe1c5171e2] Add linux-next specific files 
for 20160314
# good: [f6cede5b49e822ebc41a099fe41ab4989f64e2cb] Linux 4.5-rc7
git bisect start 'HEAD' 'v4.5-rc7'
# good: [0525c3e26ec2c43cd509433be3be25210a0154ef] Merge remote-tracking branch 
'drm-tegra/drm/tegra/for-next'
git bisect good 0525c3e26ec2c43cd509433be3be25210a0154ef
# bad: [385128a1b49762c1b9515c9f6294aeebbc55b956] Merge remote-tracking branch 
'usb-chipidea-next/ci-for-usb-next'
git bisect bad 385128a1b49762c1b9515c9f6294aeebbc55b956
# good: [dfdb27baab4fc45c9399a991270413d0fb1c694a] Merge remote-tracking branch 
'spi/for-next'
git bisect good dfdb27baab4fc45c9399a991270413d0fb1c694a
# bad: [e368d7d2a0dce6d6795ead1fc8a09bcca8a4a565] Merge branch 'timers/nohz'
git bisect bad e368d7d2a0dce6d6795ead1fc8a09bcca8a4a565
# good: [ced30bc9129777d715057d06fc8dbdfd3b81e94d] Merge tag 
'perf-core-for-mingo-20160310' of 
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core
git bisect good ced30bc9129777d715057d06fc8dbdfd3b81e94d
# bad: [656a61d4d9cbb8dfc2d007281190b2eccebad522] manual merge of mm/pkeys
git bisect bad 656a61d4d9cbb8dfc2d007281190b2eccebad522
# good: [16f7379f2da43f29d9faa2f474745e2705a3f510] Merge branch 'efi/core'
git bisect good 16f7379f2da43f29d9faa2f474745e2705a3f510
# bad: [a7fb9a8169be9a55e0cfb98346aece1b51c016fa] Merge branch 'locking/core'
git bisect bad a7fb9a8169be9a55e0cfb98346aece1b51c016fa
# good: [2a07870511829977d02609dac6450017b0419ea9] irqchip/mips-gic: Use 
gic_vpes instead of NR_CPUS
git bisect good 2a07870511829977d02609dac6450017b0419ea9
# good: [eaff0e7003cca6c2748b67ead2d4b1a8ad858fc7] locking/pvqspinlock: Move 
lock stealing count tracking code into pv_queued_spin_steal_lock()
git bisect good eaff0e7003cca6c2748b67ead2d4b1a8ad858fc7
# good: [013e379a3094ff2898f8d33cfbff1573d471ee14] tools/lib/lockdep: Fix link 
creation warning
git bisect good 013e379a3094ff2898f8d33cfbff1573d471ee14
# bad: [7eb8c99db26cc6499bfb1eba72dffc4730570752] MIPS: Delete smp-gic.c
git bisect bad 7eb8c99db26cc6499bfb1eba72dffc4730570752
# good: [fbde2d7d8290d8c642d591a471356abda2446874] MIPS: Add generic SMP IPI 
support
git bisect good fbde2d7d8290d8c642d591a471356abda2446874
# bad: [bb11cff327e54179c13446c4022ed4ed7d4871c7] MIPS: Make smp CMP, CPS and 
MT use the new generic IPI functions
git bisect bad bb11cff327e54179c13446c4022ed4ed7d4871c7
# first bad commit: [bb11cff327e54179c13446c4022ed4ed7d4871c7] MIPS: Make smp 
CMP, CPS and MT use the new generic IPI functions


Re: linux-next: manual merge of the aio tree with the vfs tree

2016-03-14 Thread Al Viro
On Tue, Mar 15, 2016 at 05:07:12AM +, Al Viro wrote:

> There *is* a reason for code review.  Or, at least, asking somebody familiar
> with the code you are working with whether some assumption you are making
> is true or false.  Me, for example, in our conversation regarding earlier 
> parts
> of aio.git queue about a week ago.  Or at any other point.

While we are at it, 150a0b49 ("aio: add support for async openat()") is also
crap.  fs_struct and files_struct is nowhere near enough.  And yes, I realize
that your application probably doesn't step into it.  Which means that these
patches are just fine for your private kernel.  _Not_ for mainline.

Reviewed-and-NAKed-by: Al Viro 


Re: linux-next: manual merge of the aio tree with the vfs tree

2016-03-14 Thread Al Viro
On Tue, Mar 15, 2016 at 05:07:12AM +, Al Viro wrote:

> There *is* a reason for code review.  Or, at least, asking somebody familiar
> with the code you are working with whether some assumption you are making
> is true or false.  Me, for example, in our conversation regarding earlier 
> parts
> of aio.git queue about a week ago.  Or at any other point.

While we are at it, 150a0b49 ("aio: add support for async openat()") is also
crap.  fs_struct and files_struct is nowhere near enough.  And yes, I realize
that your application probably doesn't step into it.  Which means that these
patches are just fine for your private kernel.  _Not_ for mainline.

Reviewed-and-NAKed-by: Al Viro 


Re: linux-next: manual merge of the aio tree with the vfs tree

2016-03-14 Thread Al Viro
On Tue, Mar 15, 2016 at 04:34:48AM +, Al Viro wrote:

> Incidentally, why the hell has that thing never landed in my mailbox?  Not
> directly, not Cc'd, not via fsdevel either.
> 
> Ben, what the fuck going on?  OK, you don't feel necessary to mention
> that to me (or have me take a look through it).  Your business.  You also
> don't bother to bring it on fsdevel.  Again, your estimates of the
> usefulness of said list and review there are your business.  But it looks
> like you also do not bother to check what has landed in the mainline two
> months ago.  WTF?

While we are at it, aio.git commit in question is crap anyway.  What is the
semantics of that LOOKUP_NONBLOCK thing and what makes you think that
it will *not* block prior to reaching do_last()?  LOOKUP_RCU that was
originally there?  Sorry, wrong.  RCU pathwalk will happily fall back to
non-RCU one if it can do so without restart from scratch.  And proceed
to lock directories, hit the disk over nbd over wet string, do automounts,
etc.  Anything and everything.  IOW, this is complete BS and had been such
for at least ~5 years.

There *is* a reason for code review.  Or, at least, asking somebody familiar
with the code you are working with whether some assumption you are making
is true or false.  Me, for example, in our conversation regarding earlier parts
of aio.git queue about a week ago.  Or at any other point.

Al "Really annoyed" Viro


Re: linux-next: manual merge of the aio tree with the vfs tree

2016-03-14 Thread Al Viro
On Tue, Mar 15, 2016 at 04:34:48AM +, Al Viro wrote:

> Incidentally, why the hell has that thing never landed in my mailbox?  Not
> directly, not Cc'd, not via fsdevel either.
> 
> Ben, what the fuck going on?  OK, you don't feel necessary to mention
> that to me (or have me take a look through it).  Your business.  You also
> don't bother to bring it on fsdevel.  Again, your estimates of the
> usefulness of said list and review there are your business.  But it looks
> like you also do not bother to check what has landed in the mainline two
> months ago.  WTF?

While we are at it, aio.git commit in question is crap anyway.  What is the
semantics of that LOOKUP_NONBLOCK thing and what makes you think that
it will *not* block prior to reaching do_last()?  LOOKUP_RCU that was
originally there?  Sorry, wrong.  RCU pathwalk will happily fall back to
non-RCU one if it can do so without restart from scratch.  And proceed
to lock directories, hit the disk over nbd over wet string, do automounts,
etc.  Anything and everything.  IOW, this is complete BS and had been such
for at least ~5 years.

There *is* a reason for code review.  Or, at least, asking somebody familiar
with the code you are working with whether some assumption you are making
is true or false.  Me, for example, in our conversation regarding earlier parts
of aio.git queue about a week ago.  Or at any other point.

Al "Really annoyed" Viro


Re: linux-next: manual merge of the aio tree with the vfs tree

2016-03-14 Thread Stephen Rothwell
Hi Al,

On Tue, 15 Mar 2016 04:34:48 + Al Viro  wrote:
>
> > between commit:
> > 
> >   5955102c9984 ("wrappers for ->i_mutex access")
> > 
> > from the vfs tree and commit:
> 
> What.
> 
> The.
> 
> Hell?
> 
> The first commit is in the mainline, not in vfs tree.  And it had been
> there since before -rc1.

Sorry, my mistake.

-- 
Cheers,
Stephen Rothwell


Re: linux-next: manual merge of the aio tree with the vfs tree

2016-03-14 Thread Stephen Rothwell
Hi Al,

On Tue, 15 Mar 2016 04:34:48 + Al Viro  wrote:
>
> > between commit:
> > 
> >   5955102c9984 ("wrappers for ->i_mutex access")
> > 
> > from the vfs tree and commit:
> 
> What.
> 
> The.
> 
> Hell?
> 
> The first commit is in the mainline, not in vfs tree.  And it had been
> there since before -rc1.

Sorry, my mistake.

-- 
Cheers,
Stephen Rothwell


Re: [PATCH v6 0/3] thermal: mediatek: Add cpu dynamic power cooling model.

2016-03-14 Thread dawei chien
On Thu, 2015-12-17 at 09:52 +0800, Viresh Kumar wrote:
> On 16-12-15, 21:29, Dawei Chien wrote:
> > Use Intelligent Power Allocation (IPA) technical to add dynamic power model
> > for binding CPU thermal zone. The power allocator governor allocates power
> > budget to control CPU temperature.
> >
> > Power Allocator governor is able to keep SOC temperature within a defined
> > temperature range to avoid SOC overheat and keep it's performance.
> > mt8173-cpufreq.c need to register its' own power model with power allocator
> > thermal governor, so that power allocator governor can allocates suitable
> > power budget to control CPU temperature.
> >
> > Binding document is refer to this patchset
> > https://lkml.org/lkml/2015/11/30/239
> >
> > Change since V5:
> > 1. Remove thermal sensor ID from phandles
> 
> Though you should have included this in the new version, but still
> 
> Acked-by: Viresh Kumar 
> 
> --
> viresh

Hi Viresh,
Would you please pull this patch to your tree since following patch
already pulled in, thank you.

https://lkml.org/lkml/2015/11/30/239

BR,
Dawei



Re: [PATCH v6 0/3] thermal: mediatek: Add cpu dynamic power cooling model.

2016-03-14 Thread dawei chien
On Thu, 2015-12-17 at 09:52 +0800, Viresh Kumar wrote:
> On 16-12-15, 21:29, Dawei Chien wrote:
> > Use Intelligent Power Allocation (IPA) technical to add dynamic power model
> > for binding CPU thermal zone. The power allocator governor allocates power
> > budget to control CPU temperature.
> >
> > Power Allocator governor is able to keep SOC temperature within a defined
> > temperature range to avoid SOC overheat and keep it's performance.
> > mt8173-cpufreq.c need to register its' own power model with power allocator
> > thermal governor, so that power allocator governor can allocates suitable
> > power budget to control CPU temperature.
> >
> > Binding document is refer to this patchset
> > https://lkml.org/lkml/2015/11/30/239
> >
> > Change since V5:
> > 1. Remove thermal sensor ID from phandles
> 
> Though you should have included this in the new version, but still
> 
> Acked-by: Viresh Kumar 
> 
> --
> viresh

Hi Viresh,
Would you please pull this patch to your tree since following patch
already pulled in, thank you.

https://lkml.org/lkml/2015/11/30/239

BR,
Dawei



Re: linux-next: manual merge of the aio tree with the vfs tree

2016-03-14 Thread Al Viro
On Tue, Mar 15, 2016 at 03:06:40PM +1100, Stephen Rothwell wrote:
> Hi Ben,
> 
> Today's linux-next merge of the aio tree got a conflict in:
> 
>   fs/namei.c
> 
> between commit:
> 
>   5955102c9984 ("wrappers for ->i_mutex access")
> 
> from the vfs tree and commit:
> 
>   5d3d80fcf992 ("aio: add support for in-submit openat")
> 
> from the aio tree.

What.

The.

Hell?

The first commit is in the mainline, not in vfs tree.  And it had been
there since before -rc1.

Incidentally, why the hell has that thing never landed in my mailbox?  Not
directly, not Cc'd, not via fsdevel either.

Ben, what the fuck going on?  OK, you don't feel necessary to mention
that to me (or have me take a look through it).  Your business.  You also
don't bother to bring it on fsdevel.  Again, your estimates of the
usefulness of said list and review there are your business.  But it looks
like you also do not bother to check what has landed in the mainline two
months ago.  WTF?


Re: linux-next: manual merge of the aio tree with the vfs tree

2016-03-14 Thread Al Viro
On Tue, Mar 15, 2016 at 03:06:40PM +1100, Stephen Rothwell wrote:
> Hi Ben,
> 
> Today's linux-next merge of the aio tree got a conflict in:
> 
>   fs/namei.c
> 
> between commit:
> 
>   5955102c9984 ("wrappers for ->i_mutex access")
> 
> from the vfs tree and commit:
> 
>   5d3d80fcf992 ("aio: add support for in-submit openat")
> 
> from the aio tree.

What.

The.

Hell?

The first commit is in the mainline, not in vfs tree.  And it had been
there since before -rc1.

Incidentally, why the hell has that thing never landed in my mailbox?  Not
directly, not Cc'd, not via fsdevel either.

Ben, what the fuck going on?  OK, you don't feel necessary to mention
that to me (or have me take a look through it).  Your business.  You also
don't bother to bring it on fsdevel.  Again, your estimates of the
usefulness of said list and review there are your business.  But it looks
like you also do not bother to check what has landed in the mainline two
months ago.  WTF?


Re: [PATCH v2 7/7] ARM: dts: Add initial gpio setting of MMC2 device for exynos3250-monk

2016-03-14 Thread Chanwoo Choi
Hi,

On 2016년 03월 15일 13:18, kbuild test robot wrote:
> Hi Chanwoo,
> 
> [auto build test ERROR on v4.5-rc7]
> [also build test ERROR on next-20160314]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improving the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Chanwoo-Choi/ARM-dts-Add-new-Exynos3250-based-ARTIK5-module-dtsi-file/20160315-101326
> config: arm-exynos_defconfig (attached as .config)
> reproduce:
> wget 
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
>  -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm 
> 
> All errors (new ones prefixed by >>):
> 
>>> Error: arch/arm/boot/dts/exynos3250-monk.dts:564.9-10 syntax error
>FATAL ERROR: Unable to parse input tree

>FATAL ERROR: Unable to parse input tree

This patch depend on patch[1]. I send the separate patch-set.
To build this patch, patch[1] should be applied. It is my mistake.
[1] https://lkml.org/lkml/2016/3/14/245

Best Regards,
Chanwoo Choi



Re: [PATCH v2 7/7] ARM: dts: Add initial gpio setting of MMC2 device for exynos3250-monk

2016-03-14 Thread Chanwoo Choi
Hi,

On 2016년 03월 15일 13:18, kbuild test robot wrote:
> Hi Chanwoo,
> 
> [auto build test ERROR on v4.5-rc7]
> [also build test ERROR on next-20160314]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improving the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Chanwoo-Choi/ARM-dts-Add-new-Exynos3250-based-ARTIK5-module-dtsi-file/20160315-101326
> config: arm-exynos_defconfig (attached as .config)
> reproduce:
> wget 
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
>  -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm 
> 
> All errors (new ones prefixed by >>):
> 
>>> Error: arch/arm/boot/dts/exynos3250-monk.dts:564.9-10 syntax error
>FATAL ERROR: Unable to parse input tree

>FATAL ERROR: Unable to parse input tree

This patch depend on patch[1]. I send the separate patch-set.
To build this patch, patch[1] should be applied. It is my mistake.
[1] https://lkml.org/lkml/2016/3/14/245

Best Regards,
Chanwoo Choi



[PATCH v2 04/04] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency

2016-03-14 Thread Magnus Damm
From: Magnus Damm 

Neither the ARM page table code enabled by IOMMU_IO_PGTABLE_LPAE
nor the IPMMU_VMSA driver actually depends on ARM_LPAE, so get
rid of the dependency.

Tested with ipmmu-vmsa on r8a7794 ALT and a kernel config using:
 # CONFIG_ARM_LPAE is not set

Signed-off-by: Magnus Damm 
Acked-by: Laurent Pinchart 
---

 Changes since V1:
 - Rebased on top of ARCH_RENESAS change
 - Added Acked-by from Laurent

 This time the result also compiles on x86. Need to be
 applied as last patch in the following series:
 [PATCH v2 00/04] iommu/ipmmu-vmsa: IPMMU multi-arch update V2
 
 drivers/iommu/Kconfig |1 -
 1 file changed, 1 deletion(-)

--- 0001/drivers/iommu/Kconfig
+++ work/drivers/iommu/Kconfig  2016-03-15 12:28:45.210513000 +0900
@@ -284,7 +284,6 @@ config EXYNOS_IOMMU_DEBUG
 
 config IPMMU_VMSA
bool "Renesas VMSA-compatible IPMMU"
-   depends on ARM_LPAE
depends on ARCH_RENESAS || COMPILE_TEST
select IOMMU_API
select IOMMU_IO_PGTABLE_LPAE


[PATCH v2 04/04] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency

2016-03-14 Thread Magnus Damm
From: Magnus Damm 

Neither the ARM page table code enabled by IOMMU_IO_PGTABLE_LPAE
nor the IPMMU_VMSA driver actually depends on ARM_LPAE, so get
rid of the dependency.

Tested with ipmmu-vmsa on r8a7794 ALT and a kernel config using:
 # CONFIG_ARM_LPAE is not set

Signed-off-by: Magnus Damm 
Acked-by: Laurent Pinchart 
---

 Changes since V1:
 - Rebased on top of ARCH_RENESAS change
 - Added Acked-by from Laurent

 This time the result also compiles on x86. Need to be
 applied as last patch in the following series:
 [PATCH v2 00/04] iommu/ipmmu-vmsa: IPMMU multi-arch update V2
 
 drivers/iommu/Kconfig |1 -
 1 file changed, 1 deletion(-)

--- 0001/drivers/iommu/Kconfig
+++ work/drivers/iommu/Kconfig  2016-03-15 12:28:45.210513000 +0900
@@ -284,7 +284,6 @@ config EXYNOS_IOMMU_DEBUG
 
 config IPMMU_VMSA
bool "Renesas VMSA-compatible IPMMU"
-   depends on ARM_LPAE
depends on ARCH_RENESAS || COMPILE_TEST
select IOMMU_API
select IOMMU_IO_PGTABLE_LPAE


[PATCH v2 03/04] iommu/ipmmu-vmsa: Break out 32-bit ARM mapping code

2016-03-14 Thread Magnus Damm
From: Magnus Damm 

Make the driver compile on more than just 32-bit ARM
by breaking out and wrapping ARM specific functions
in #ifdefs. Not pretty, but needed to be able to use
the driver on other architectures like ARM64.

Signed-off-by: Magnus Damm 
---

 Changes since V1:
 - Rebased to work without patch 2 and 3 from V1 series

 drivers/iommu/ipmmu-vmsa.c |   94 +---
 1 file changed, 62 insertions(+), 32 deletions(-)

--- 0004/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-15 12:25:45.040513000 +0900
@@ -22,8 +22,10 @@
 #include 
 #include 
 
+#ifdef CONFIG_ARM
 #include 
 #include 
+#endif
 
 #include "io-pgtable.h"
 
@@ -38,7 +40,9 @@ struct ipmmu_vmsa_device {
DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
 
+#ifdef CONFIG_ARM
struct dma_iommu_mapping *mapping;
+#endif
 };
 
 struct ipmmu_vmsa_domain {
@@ -615,6 +619,60 @@ static int ipmmu_find_utlbs(struct ipmmu
return 0;
 }
 
+#ifdef CONFIG_ARM
+static int ipmmu_map_attach(struct device *dev, struct ipmmu_vmsa_device *mmu)
+{
+   int ret;
+
+   /*
+* Create the ARM mapping, used by the ARM DMA mapping core to allocate
+* VAs. This will allocate a corresponding IOMMU domain.
+*
+* TODO:
+* - Create one mapping per context (TLB).
+* - Make the mapping size configurable ? We currently use a 2GB mapping
+*   at a 1GB offset to ensure that NULL VAs will fault.
+*/
+   if (!mmu->mapping) {
+   struct dma_iommu_mapping *mapping;
+
+   mapping = arm_iommu_create_mapping(_bus_type,
+  SZ_1G, SZ_2G);
+   if (IS_ERR(mapping)) {
+   dev_err(mmu->dev, "failed to create ARM IOMMU 
mapping\n");
+   return PTR_ERR(mapping);
+   }
+
+   mmu->mapping = mapping;
+   }
+
+   /* Attach the ARM VA mapping to the device. */
+   ret = arm_iommu_attach_device(dev, mmu->mapping);
+   if (ret < 0) {
+   dev_err(dev, "Failed to attach device to VA mapping\n");
+   arm_iommu_release_mapping(mmu->mapping);
+   }
+
+   return ret;
+}
+static inline void ipmmu_detach(struct device *dev)
+{
+   arm_iommu_detach_device(dev);
+}
+static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu)
+{
+   arm_iommu_release_mapping(mmu->mapping);
+}
+#else
+static inline int ipmmu_map_attach(struct device *dev,
+  struct ipmmu_vmsa_device *mmu)
+{
+   return 0;
+}
+static inline void ipmmu_detach(struct device *dev) {}
+static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu) {}
+#endif
+
 static int ipmmu_add_device(struct device *dev)
 {
struct ipmmu_vmsa_archdata *archdata;
@@ -695,41 +753,13 @@ static int ipmmu_add_device(struct devic
archdata->num_utlbs = num_utlbs;
dev->archdata.iommu = archdata;
 
-   /*
-* Create the ARM mapping, used by the ARM DMA mapping core to allocate
-* VAs. This will allocate a corresponding IOMMU domain.
-*
-* TODO:
-* - Create one mapping per context (TLB).
-* - Make the mapping size configurable ? We currently use a 2GB mapping
-*   at a 1GB offset to ensure that NULL VAs will fault.
-*/
-   if (!mmu->mapping) {
-   struct dma_iommu_mapping *mapping;
-
-   mapping = arm_iommu_create_mapping(_bus_type,
-  SZ_1G, SZ_2G);
-   if (IS_ERR(mapping)) {
-   dev_err(mmu->dev, "failed to create ARM IOMMU 
mapping\n");
-   ret = PTR_ERR(mapping);
-   goto error;
-   }
-
-   mmu->mapping = mapping;
-   }
-
-   /* Attach the ARM VA mapping to the device. */
-   ret = arm_iommu_attach_device(dev, mmu->mapping);
-   if (ret < 0) {
-   dev_err(dev, "Failed to attach device to VA mapping\n");
+   ret = ipmmu_map_attach(dev, mmu);
+   if (ret < 0)
goto error;
-   }
 
return 0;
 
 error:
-   arm_iommu_release_mapping(mmu->mapping);
-
kfree(dev->archdata.iommu);
kfree(utlbs);
 
@@ -745,7 +775,7 @@ static void ipmmu_remove_device(struct d
 {
struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
 
-   arm_iommu_detach_device(dev);
+   ipmmu_detach(dev);
iommu_group_remove_device(dev);
 
kfree(archdata->utlbs);
@@ -856,7 +886,7 @@ static int ipmmu_remove(struct platform_
list_del(>list);
spin_unlock(_devices_lock);
 
-   arm_iommu_release_mapping(mmu->mapping);
+   ipmmu_release_mapping(mmu);
 
ipmmu_device_reset(mmu);
 


[PATCH v2] watchdog: don't run proc_watchdog_update if new value is same as old

2016-03-14 Thread Joshua Hunt


While working on a script to restore all sysctl params before a series of
tests I found that writing any value into the
/proc/sys/kernel/{nmi_watchdog,soft_watchdog,watchdog,watchdog_thresh}
causes them to call proc_watchdog_update().

[  955.756196] NMI watchdog: enabled on all CPUs, permanently consumes one 
hw-PMU counter.
[  955.765994] NMI watchdog: enabled on all CPUs, permanently consumes one 
hw-PMU counter.
[  955.774619] NMI watchdog: enabled on all CPUs, permanently consumes one 
hw-PMU counter.
[  955.783182] NMI watchdog: enabled on all CPUs, permanently consumes one 
hw-PMU counter.

There doesn't appear to be a reason for doing this work every time a write
occurs, so only do it when the values change.

Signed-off-by: Josh Hunt 
Cc:  # 4.1.x-
---
 kernel/watchdog.c |9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

v2: Update changelog to no longer include mention of soft lockup, and add 
stable.

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index b3ace6e..9acb29f 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -923,6 +923,9 @@ static int proc_watchdog_common(int which, struct ctl_table 
*table, int write,
 * both lockup detectors are disabled if proc_watchdog_update()
 * returns an error.
 */
+   if (old == new)
+   goto out;
+
err = proc_watchdog_update();
}
 out:
@@ -967,7 +970,7 @@ int proc_soft_watchdog(struct ctl_table *table, int write,
 int proc_watchdog_thresh(struct ctl_table *table, int write,
 void __user *buffer, size_t *lenp, loff_t *ppos)
 {
-   int err, old;
+   int err, old, new;
 
get_online_cpus();
mutex_lock(_proc_mutex);
@@ -987,6 +990,10 @@ int proc_watchdog_thresh(struct ctl_table *table, int 
write,
/*
 * Update the sample period. Restore on failure.
 */
+   new = ACCESS_ONCE(watchdog_thresh);
+   if (old == new)
+   goto out;
+
set_sample_period();
err = proc_watchdog_update();
if (err) {
-- 
1.7.9.5



Re: [PATCH v2 7/7] ARM: dts: Add initial gpio setting of MMC2 device for exynos3250-monk

2016-03-14 Thread kbuild test robot
Hi Chanwoo,

[auto build test ERROR on v4.5-rc7]
[also build test ERROR on next-20160314]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Chanwoo-Choi/ARM-dts-Add-new-Exynos3250-based-ARTIK5-module-dtsi-file/20160315-101326
config: arm-exynos_defconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> Error: arch/arm/boot/dts/exynos3250-monk.dts:564.9-10 syntax error
   FATAL ERROR: Unable to parse input tree

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


.config.gz
Description: Binary data


[PATCH v2 03/04] iommu/ipmmu-vmsa: Break out 32-bit ARM mapping code

2016-03-14 Thread Magnus Damm
From: Magnus Damm 

Make the driver compile on more than just 32-bit ARM
by breaking out and wrapping ARM specific functions
in #ifdefs. Not pretty, but needed to be able to use
the driver on other architectures like ARM64.

Signed-off-by: Magnus Damm 
---

 Changes since V1:
 - Rebased to work without patch 2 and 3 from V1 series

 drivers/iommu/ipmmu-vmsa.c |   94 +---
 1 file changed, 62 insertions(+), 32 deletions(-)

--- 0004/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-15 12:25:45.040513000 +0900
@@ -22,8 +22,10 @@
 #include 
 #include 
 
+#ifdef CONFIG_ARM
 #include 
 #include 
+#endif
 
 #include "io-pgtable.h"
 
@@ -38,7 +40,9 @@ struct ipmmu_vmsa_device {
DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
 
+#ifdef CONFIG_ARM
struct dma_iommu_mapping *mapping;
+#endif
 };
 
 struct ipmmu_vmsa_domain {
@@ -615,6 +619,60 @@ static int ipmmu_find_utlbs(struct ipmmu
return 0;
 }
 
+#ifdef CONFIG_ARM
+static int ipmmu_map_attach(struct device *dev, struct ipmmu_vmsa_device *mmu)
+{
+   int ret;
+
+   /*
+* Create the ARM mapping, used by the ARM DMA mapping core to allocate
+* VAs. This will allocate a corresponding IOMMU domain.
+*
+* TODO:
+* - Create one mapping per context (TLB).
+* - Make the mapping size configurable ? We currently use a 2GB mapping
+*   at a 1GB offset to ensure that NULL VAs will fault.
+*/
+   if (!mmu->mapping) {
+   struct dma_iommu_mapping *mapping;
+
+   mapping = arm_iommu_create_mapping(_bus_type,
+  SZ_1G, SZ_2G);
+   if (IS_ERR(mapping)) {
+   dev_err(mmu->dev, "failed to create ARM IOMMU 
mapping\n");
+   return PTR_ERR(mapping);
+   }
+
+   mmu->mapping = mapping;
+   }
+
+   /* Attach the ARM VA mapping to the device. */
+   ret = arm_iommu_attach_device(dev, mmu->mapping);
+   if (ret < 0) {
+   dev_err(dev, "Failed to attach device to VA mapping\n");
+   arm_iommu_release_mapping(mmu->mapping);
+   }
+
+   return ret;
+}
+static inline void ipmmu_detach(struct device *dev)
+{
+   arm_iommu_detach_device(dev);
+}
+static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu)
+{
+   arm_iommu_release_mapping(mmu->mapping);
+}
+#else
+static inline int ipmmu_map_attach(struct device *dev,
+  struct ipmmu_vmsa_device *mmu)
+{
+   return 0;
+}
+static inline void ipmmu_detach(struct device *dev) {}
+static inline void ipmmu_release_mapping(struct ipmmu_vmsa_device *mmu) {}
+#endif
+
 static int ipmmu_add_device(struct device *dev)
 {
struct ipmmu_vmsa_archdata *archdata;
@@ -695,41 +753,13 @@ static int ipmmu_add_device(struct devic
archdata->num_utlbs = num_utlbs;
dev->archdata.iommu = archdata;
 
-   /*
-* Create the ARM mapping, used by the ARM DMA mapping core to allocate
-* VAs. This will allocate a corresponding IOMMU domain.
-*
-* TODO:
-* - Create one mapping per context (TLB).
-* - Make the mapping size configurable ? We currently use a 2GB mapping
-*   at a 1GB offset to ensure that NULL VAs will fault.
-*/
-   if (!mmu->mapping) {
-   struct dma_iommu_mapping *mapping;
-
-   mapping = arm_iommu_create_mapping(_bus_type,
-  SZ_1G, SZ_2G);
-   if (IS_ERR(mapping)) {
-   dev_err(mmu->dev, "failed to create ARM IOMMU 
mapping\n");
-   ret = PTR_ERR(mapping);
-   goto error;
-   }
-
-   mmu->mapping = mapping;
-   }
-
-   /* Attach the ARM VA mapping to the device. */
-   ret = arm_iommu_attach_device(dev, mmu->mapping);
-   if (ret < 0) {
-   dev_err(dev, "Failed to attach device to VA mapping\n");
+   ret = ipmmu_map_attach(dev, mmu);
+   if (ret < 0)
goto error;
-   }
 
return 0;
 
 error:
-   arm_iommu_release_mapping(mmu->mapping);
-
kfree(dev->archdata.iommu);
kfree(utlbs);
 
@@ -745,7 +775,7 @@ static void ipmmu_remove_device(struct d
 {
struct ipmmu_vmsa_archdata *archdata = dev->archdata.iommu;
 
-   arm_iommu_detach_device(dev);
+   ipmmu_detach(dev);
iommu_group_remove_device(dev);
 
kfree(archdata->utlbs);
@@ -856,7 +886,7 @@ static int ipmmu_remove(struct platform_
list_del(>list);
spin_unlock(_devices_lock);
 
-   arm_iommu_release_mapping(mmu->mapping);
+   ipmmu_release_mapping(mmu);
 
ipmmu_device_reset(mmu);
 


[PATCH v2] watchdog: don't run proc_watchdog_update if new value is same as old

2016-03-14 Thread Joshua Hunt


While working on a script to restore all sysctl params before a series of
tests I found that writing any value into the
/proc/sys/kernel/{nmi_watchdog,soft_watchdog,watchdog,watchdog_thresh}
causes them to call proc_watchdog_update().

[  955.756196] NMI watchdog: enabled on all CPUs, permanently consumes one 
hw-PMU counter.
[  955.765994] NMI watchdog: enabled on all CPUs, permanently consumes one 
hw-PMU counter.
[  955.774619] NMI watchdog: enabled on all CPUs, permanently consumes one 
hw-PMU counter.
[  955.783182] NMI watchdog: enabled on all CPUs, permanently consumes one 
hw-PMU counter.

There doesn't appear to be a reason for doing this work every time a write
occurs, so only do it when the values change.

Signed-off-by: Josh Hunt 
Cc:  # 4.1.x-
---
 kernel/watchdog.c |9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

v2: Update changelog to no longer include mention of soft lockup, and add 
stable.

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index b3ace6e..9acb29f 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -923,6 +923,9 @@ static int proc_watchdog_common(int which, struct ctl_table 
*table, int write,
 * both lockup detectors are disabled if proc_watchdog_update()
 * returns an error.
 */
+   if (old == new)
+   goto out;
+
err = proc_watchdog_update();
}
 out:
@@ -967,7 +970,7 @@ int proc_soft_watchdog(struct ctl_table *table, int write,
 int proc_watchdog_thresh(struct ctl_table *table, int write,
 void __user *buffer, size_t *lenp, loff_t *ppos)
 {
-   int err, old;
+   int err, old, new;
 
get_online_cpus();
mutex_lock(_proc_mutex);
@@ -987,6 +990,10 @@ int proc_watchdog_thresh(struct ctl_table *table, int 
write,
/*
 * Update the sample period. Restore on failure.
 */
+   new = ACCESS_ONCE(watchdog_thresh);
+   if (old == new)
+   goto out;
+
set_sample_period();
err = proc_watchdog_update();
if (err) {
-- 
1.7.9.5



Re: [PATCH v2 7/7] ARM: dts: Add initial gpio setting of MMC2 device for exynos3250-monk

2016-03-14 Thread kbuild test robot
Hi Chanwoo,

[auto build test ERROR on v4.5-rc7]
[also build test ERROR on next-20160314]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Chanwoo-Choi/ARM-dts-Add-new-Exynos3250-based-ARTIK5-module-dtsi-file/20160315-101326
config: arm-exynos_defconfig (attached as .config)
reproduce:
wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> Error: arch/arm/boot/dts/exynos3250-monk.dts:564.9-10 syntax error
   FATAL ERROR: Unable to parse input tree

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


.config.gz
Description: Binary data


[PATCH v2 01/04] iommu/ipmmu-vmsa: Remove platform data handling

2016-03-14 Thread Magnus Damm
From: Magnus Damm 

The IPMMU driver is using DT these days, and platform data is no longer
used by the driver. Remove unused code.

Signed-off-by: Magnus Damm 
Reviewed-by: Laurent Pinchart 
---

 Changes since V1:
 - Added Reviewed-by from Laurent

 drivers/iommu/ipmmu-vmsa.c |5 -
 1 file changed, 5 deletions(-)

--- 0001/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-15 10:59:25.590513000 +0900
@@ -766,11 +766,6 @@ static int ipmmu_probe(struct platform_d
int irq;
int ret;
 
-   if (!IS_ENABLED(CONFIG_OF) && !pdev->dev.platform_data) {
-   dev_err(>dev, "missing platform data\n");
-   return -EINVAL;
-   }
-
mmu = devm_kzalloc(>dev, sizeof(*mmu), GFP_KERNEL);
if (!mmu) {
dev_err(>dev, "cannot allocate device data\n");


[PATCH v2 01/04] iommu/ipmmu-vmsa: Remove platform data handling

2016-03-14 Thread Magnus Damm
From: Magnus Damm 

The IPMMU driver is using DT these days, and platform data is no longer
used by the driver. Remove unused code.

Signed-off-by: Magnus Damm 
Reviewed-by: Laurent Pinchart 
---

 Changes since V1:
 - Added Reviewed-by from Laurent

 drivers/iommu/ipmmu-vmsa.c |5 -
 1 file changed, 5 deletions(-)

--- 0001/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-15 10:59:25.590513000 +0900
@@ -766,11 +766,6 @@ static int ipmmu_probe(struct platform_d
int irq;
int ret;
 
-   if (!IS_ENABLED(CONFIG_OF) && !pdev->dev.platform_data) {
-   dev_err(>dev, "missing platform data\n");
-   return -EINVAL;
-   }
-
mmu = devm_kzalloc(>dev, sizeof(*mmu), GFP_KERNEL);
if (!mmu) {
dev_err(>dev, "cannot allocate device data\n");


[PATCH v2 02/04] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context

2016-03-14 Thread Magnus Damm
From: Magnus Damm 

Introduce a bitmap for context handing and convert the
interrupt routine to go handle all registered contexts.

At this point the number of contexts are still limited.

Also remove the use of the ARM specific mapping variable
from ipmmu_irq() to allow compile on ARM64.

Signed-off-by: Magnus Damm 
---

 Changes since V1: (Thanks to Laurent for feedback!)
 - Use simple find_first_zero()/set_bit()/clear_bit() for context management.
 - For allocation rely on spinlock held when calling ipmmu_domain_init_context()
 - For test/free use atomic bitops
 - Return IRQ_HANDLED if any of the contexts generated interrupts

 drivers/iommu/ipmmu-vmsa.c |   47 
 1 file changed, 35 insertions(+), 12 deletions(-)

--- 0003/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-15 12:42:18.940513000 +0900
@@ -8,6 +8,7 @@
  * the Free Software Foundation; version 2 of the License.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -26,12 +27,16 @@
 
 #include "io-pgtable.h"
 
+#define IPMMU_CTX_MAX 1
+
 struct ipmmu_vmsa_device {
struct device *dev;
void __iomem *base;
struct list_head list;
 
unsigned int num_utlbs;
+   DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
+   struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
 
struct dma_iommu_mapping *mapping;
 };
@@ -296,6 +301,7 @@ static struct iommu_gather_ops ipmmu_gat
 static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
 {
u64 ttbr;
+   int ret;
 
/*
 * Allocate the page table operations.
@@ -325,10 +331,17 @@ static int ipmmu_domain_init_context(str
return -EINVAL;
 
/*
-* TODO: When adding support for multiple contexts, find an unused
-* context.
+* Find an unused context.
 */
-   domain->context_id = 0;
+   ret = find_first_zero_bit(domain->mmu->ctx, IPMMU_CTX_MAX);
+   if (ret == IPMMU_CTX_MAX) {
+   free_io_pgtable_ops(domain->iop);
+   return -EBUSY;
+   }
+
+   domain->context_id = ret;
+   domain->mmu->domains[ret] = domain;
+   set_bit(ret, domain->mmu->ctx);
 
/* TTBR0 */
ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
@@ -372,6 +385,8 @@ static int ipmmu_domain_init_context(str
 
 static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
 {
+   clear_bit(domain->context_id, domain->mmu->ctx);
+
/*
 * Disable the context. Flush the TLB as required when modifying the
 * context registers.
@@ -389,10 +404,15 @@ static void ipmmu_domain_destroy_context
 static irqreturn_t ipmmu_domain_irq(struct ipmmu_vmsa_domain *domain)
 {
const u32 err_mask = IMSTR_MHIT | IMSTR_ABORT | IMSTR_PF | IMSTR_TF;
-   struct ipmmu_vmsa_device *mmu = domain->mmu;
+   struct ipmmu_vmsa_device *mmu;
u32 status;
u32 iova;
 
+   if (!domain)
+   return IRQ_NONE;
+
+   mmu = domain->mmu;
+
status = ipmmu_ctx_read(domain, IMSTR);
if (!(status & err_mask))
return IRQ_NONE;
@@ -437,16 +457,18 @@ static irqreturn_t ipmmu_domain_irq(stru
 static irqreturn_t ipmmu_irq(int irq, void *dev)
 {
struct ipmmu_vmsa_device *mmu = dev;
-   struct iommu_domain *io_domain;
-   struct ipmmu_vmsa_domain *domain;
-
-   if (!mmu->mapping)
-   return IRQ_NONE;
+   irqreturn_t status = IRQ_NONE;
+   unsigned int i;
 
-   io_domain = mmu->mapping->domain;
-   domain = to_vmsa_domain(io_domain);
+   /* Check interrupts for all active contexts */
+   for (i = 0; i < IPMMU_CTX_MAX; i++) {
+   if (!test_bit(i, mmu->ctx))
+   continue;
+   if (ipmmu_domain_irq(mmu->domains[i]) == IRQ_HANDLED)
+   status = IRQ_HANDLED;
+   }
 
-   return ipmmu_domain_irq(domain);
+   return status;
 }
 
 /* 
-
@@ -774,6 +796,7 @@ static int ipmmu_probe(struct platform_d
 
mmu->dev = >dev;
mmu->num_utlbs = 32;
+   bitmap_zero(mmu->ctx, IPMMU_CTX_MAX);
 
/* Map I/O memory and request IRQ. */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);


[PATCH v2 00/04] iommu/ipmmu-vmsa: IPMMU multi-arch update V2

2016-03-14 Thread Magnus Damm
iommu/ipmmu-vmsa: IPMMU multi-arch update V2

[PATCH v2 01/04] iommu/ipmmu-vmsa: Remove platform data handling
[PATCH v2 02/04] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for 
context
[PATCH v2 03/04] iommu/ipmmu-vmsa: Break out 32-bit ARM mapping code
[PATCH v2 04/04] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency

These patches update the IPMMU driver with a few minor changes
to support build on multiple architectures.

With these patches applied the driver is known to compile without issues
on 32-bit ARM, 64-bit ARM and x86_64.

Changes since V1:
 - Got rid of patch 2 and 3 from initial series
 - Updated bitmap code locking and also used lighter bitop functions
 - Updated the Kconfig bits to apply on top of ARCH_RENESAS

Signed-off-by: Magnus Damm <damm+rene...@opensource.se>
---

 Built on top of next-20160314

 drivers/iommu/Kconfig  |1 
 drivers/iommu/ipmmu-vmsa.c |  146 +---
 2 files changed, 97 insertions(+), 50 deletions(-)


[PATCH v2 02/04] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context

2016-03-14 Thread Magnus Damm
From: Magnus Damm 

Introduce a bitmap for context handing and convert the
interrupt routine to go handle all registered contexts.

At this point the number of contexts are still limited.

Also remove the use of the ARM specific mapping variable
from ipmmu_irq() to allow compile on ARM64.

Signed-off-by: Magnus Damm 
---

 Changes since V1: (Thanks to Laurent for feedback!)
 - Use simple find_first_zero()/set_bit()/clear_bit() for context management.
 - For allocation rely on spinlock held when calling ipmmu_domain_init_context()
 - For test/free use atomic bitops
 - Return IRQ_HANDLED if any of the contexts generated interrupts

 drivers/iommu/ipmmu-vmsa.c |   47 
 1 file changed, 35 insertions(+), 12 deletions(-)

--- 0003/drivers/iommu/ipmmu-vmsa.c
+++ work/drivers/iommu/ipmmu-vmsa.c 2016-03-15 12:42:18.940513000 +0900
@@ -8,6 +8,7 @@
  * the Free Software Foundation; version 2 of the License.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -26,12 +27,16 @@
 
 #include "io-pgtable.h"
 
+#define IPMMU_CTX_MAX 1
+
 struct ipmmu_vmsa_device {
struct device *dev;
void __iomem *base;
struct list_head list;
 
unsigned int num_utlbs;
+   DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
+   struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
 
struct dma_iommu_mapping *mapping;
 };
@@ -296,6 +301,7 @@ static struct iommu_gather_ops ipmmu_gat
 static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
 {
u64 ttbr;
+   int ret;
 
/*
 * Allocate the page table operations.
@@ -325,10 +331,17 @@ static int ipmmu_domain_init_context(str
return -EINVAL;
 
/*
-* TODO: When adding support for multiple contexts, find an unused
-* context.
+* Find an unused context.
 */
-   domain->context_id = 0;
+   ret = find_first_zero_bit(domain->mmu->ctx, IPMMU_CTX_MAX);
+   if (ret == IPMMU_CTX_MAX) {
+   free_io_pgtable_ops(domain->iop);
+   return -EBUSY;
+   }
+
+   domain->context_id = ret;
+   domain->mmu->domains[ret] = domain;
+   set_bit(ret, domain->mmu->ctx);
 
/* TTBR0 */
ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
@@ -372,6 +385,8 @@ static int ipmmu_domain_init_context(str
 
 static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
 {
+   clear_bit(domain->context_id, domain->mmu->ctx);
+
/*
 * Disable the context. Flush the TLB as required when modifying the
 * context registers.
@@ -389,10 +404,15 @@ static void ipmmu_domain_destroy_context
 static irqreturn_t ipmmu_domain_irq(struct ipmmu_vmsa_domain *domain)
 {
const u32 err_mask = IMSTR_MHIT | IMSTR_ABORT | IMSTR_PF | IMSTR_TF;
-   struct ipmmu_vmsa_device *mmu = domain->mmu;
+   struct ipmmu_vmsa_device *mmu;
u32 status;
u32 iova;
 
+   if (!domain)
+   return IRQ_NONE;
+
+   mmu = domain->mmu;
+
status = ipmmu_ctx_read(domain, IMSTR);
if (!(status & err_mask))
return IRQ_NONE;
@@ -437,16 +457,18 @@ static irqreturn_t ipmmu_domain_irq(stru
 static irqreturn_t ipmmu_irq(int irq, void *dev)
 {
struct ipmmu_vmsa_device *mmu = dev;
-   struct iommu_domain *io_domain;
-   struct ipmmu_vmsa_domain *domain;
-
-   if (!mmu->mapping)
-   return IRQ_NONE;
+   irqreturn_t status = IRQ_NONE;
+   unsigned int i;
 
-   io_domain = mmu->mapping->domain;
-   domain = to_vmsa_domain(io_domain);
+   /* Check interrupts for all active contexts */
+   for (i = 0; i < IPMMU_CTX_MAX; i++) {
+   if (!test_bit(i, mmu->ctx))
+   continue;
+   if (ipmmu_domain_irq(mmu->domains[i]) == IRQ_HANDLED)
+   status = IRQ_HANDLED;
+   }
 
-   return ipmmu_domain_irq(domain);
+   return status;
 }
 
 /* 
-
@@ -774,6 +796,7 @@ static int ipmmu_probe(struct platform_d
 
mmu->dev = >dev;
mmu->num_utlbs = 32;
+   bitmap_zero(mmu->ctx, IPMMU_CTX_MAX);
 
/* Map I/O memory and request IRQ. */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);


[PATCH v2 00/04] iommu/ipmmu-vmsa: IPMMU multi-arch update V2

2016-03-14 Thread Magnus Damm
iommu/ipmmu-vmsa: IPMMU multi-arch update V2

[PATCH v2 01/04] iommu/ipmmu-vmsa: Remove platform data handling
[PATCH v2 02/04] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for 
context
[PATCH v2 03/04] iommu/ipmmu-vmsa: Break out 32-bit ARM mapping code
[PATCH v2 04/04] iommu/ipmmu-vmsa: Drop LPAE Kconfig dependency

These patches update the IPMMU driver with a few minor changes
to support build on multiple architectures.

With these patches applied the driver is known to compile without issues
on 32-bit ARM, 64-bit ARM and x86_64.

Changes since V1:
 - Got rid of patch 2 and 3 from initial series
 - Updated bitmap code locking and also used lighter bitop functions
 - Updated the Kconfig bits to apply on top of ARCH_RENESAS

Signed-off-by: Magnus Damm 
---

 Built on top of next-20160314

 drivers/iommu/Kconfig  |1 
 drivers/iommu/ipmmu-vmsa.c |  146 +---
 2 files changed, 97 insertions(+), 50 deletions(-)


linux-next: manual merge of the aio tree with the vfs tree

2016-03-14 Thread Stephen Rothwell
Hi Ben,

Today's linux-next merge of the aio tree got a conflict in:

  fs/namei.c

between commit:

  5955102c9984 ("wrappers for ->i_mutex access")

from the vfs tree and commit:

  5d3d80fcf992 ("aio: add support for in-submit openat")

from the aio tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell

diff --cc fs/namei.c
index 794f81dce766,260782f5a868..
--- a/fs/namei.c
+++ b/fs/namei.c
@@@ -3125,9 -3079,15 +3125,15 @@@ retry_lookup
 * dropping this one anyway.
 */
}
+ 
+   if (nd->flags & LOOKUP_NONBLOCK) {
+   error = -EAGAIN;
+   goto out;
+   }
+   
 -  mutex_lock(>d_inode->i_mutex);
 +  inode_lock(dir->d_inode);
error = lookup_open(nd, , file, op, got_write, opened);
 -  mutex_unlock(>d_inode->i_mutex);
 +  inode_unlock(dir->d_inode);
  
if (error <= 0) {
if (error)


linux-next: manual merge of the aio tree with the vfs tree

2016-03-14 Thread Stephen Rothwell
Hi Ben,

Today's linux-next merge of the aio tree got a conflict in:

  fs/namei.c

between commit:

  5955102c9984 ("wrappers for ->i_mutex access")

from the vfs tree and commit:

  5d3d80fcf992 ("aio: add support for in-submit openat")

from the aio tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell

diff --cc fs/namei.c
index 794f81dce766,260782f5a868..
--- a/fs/namei.c
+++ b/fs/namei.c
@@@ -3125,9 -3079,15 +3125,15 @@@ retry_lookup
 * dropping this one anyway.
 */
}
+ 
+   if (nd->flags & LOOKUP_NONBLOCK) {
+   error = -EAGAIN;
+   goto out;
+   }
+   
 -  mutex_lock(>d_inode->i_mutex);
 +  inode_lock(dir->d_inode);
error = lookup_open(nd, , file, op, got_write, opened);
 -  mutex_unlock(>d_inode->i_mutex);
 +  inode_unlock(dir->d_inode);
  
if (error <= 0) {
if (error)


Re: [PATCH] gpio: 74x164: Implement gpiochip.set_multiple()

2016-03-14 Thread Phil Reid

On 14/03/2016 11:19 PM, Geert Uytterhoeven wrote:

This allows to set multiple outputs using a single SPI transfer.

Signed-off-by: Geert Uytterhoeven 


Reviewed-by: Phil Reid 


I do have a general question about GPIO drivers.
pca953x does not update the cached data unless the write operation
was successful. Which I folowed with the implement of set_multiple.
However a number of other drivers update regardless.
eg chip->buffer is updated even if the write_config fails.

What is the preferred approach?


---
  drivers/gpio/gpio-74x164.c | 24 
  1 file changed, 24 insertions(+)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index c81224ff2dca988b..62291a81c97f7140 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -75,6 +75,29 @@ static void gen_74x164_set_value(struct gpio_chip *gc,
mutex_unlock(>lock);
  }

+static void gen_74x164_set_multiple(struct gpio_chip *gc, unsigned long *mask,
+   unsigned long *bits)
+{
+   struct gen_74x164_chip *chip = gpiochip_get_data(gc);
+   unsigned int i, idx, shift;
+   u8 bank, bankmask;
+
+   mutex_lock(>lock);
+   for (i = 0, bank = chip->registers - 1; i < chip->registers;
+i++, bank--) {
+   idx = i / sizeof(*mask);
+   shift = i % sizeof(*mask) * BITS_PER_BYTE;
+   bankmask = mask[idx] >> shift;
+   if (!bankmask)
+   continue;
+
+   chip->buffer[bank] &= ~bankmask;
+   chip->buffer[bank] |= bankmask & (bits[idx] >> shift);
+   }
+   __gen_74x164_write_config(chip);
+   mutex_unlock(>lock);
+}
+
  static int gen_74x164_direction_output(struct gpio_chip *gc,
unsigned offset, int val)
  {
@@ -114,6 +137,7 @@ static int gen_74x164_probe(struct spi_device *spi)
chip->gpio_chip.direction_output = gen_74x164_direction_output;
chip->gpio_chip.get = gen_74x164_get_value;
chip->gpio_chip.set = gen_74x164_set_value;
+   chip->gpio_chip.set_multiple = gen_74x164_set_multiple;
chip->gpio_chip.base = -1;

chip->registers = nregs;




--
Regards
Phil Reid



Re: [PATCH] gpio: 74x164: Implement gpiochip.set_multiple()

2016-03-14 Thread Phil Reid

On 14/03/2016 11:19 PM, Geert Uytterhoeven wrote:

This allows to set multiple outputs using a single SPI transfer.

Signed-off-by: Geert Uytterhoeven 


Reviewed-by: Phil Reid 


I do have a general question about GPIO drivers.
pca953x does not update the cached data unless the write operation
was successful. Which I folowed with the implement of set_multiple.
However a number of other drivers update regardless.
eg chip->buffer is updated even if the write_config fails.

What is the preferred approach?


---
  drivers/gpio/gpio-74x164.c | 24 
  1 file changed, 24 insertions(+)

diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c
index c81224ff2dca988b..62291a81c97f7140 100644
--- a/drivers/gpio/gpio-74x164.c
+++ b/drivers/gpio/gpio-74x164.c
@@ -75,6 +75,29 @@ static void gen_74x164_set_value(struct gpio_chip *gc,
mutex_unlock(>lock);
  }

+static void gen_74x164_set_multiple(struct gpio_chip *gc, unsigned long *mask,
+   unsigned long *bits)
+{
+   struct gen_74x164_chip *chip = gpiochip_get_data(gc);
+   unsigned int i, idx, shift;
+   u8 bank, bankmask;
+
+   mutex_lock(>lock);
+   for (i = 0, bank = chip->registers - 1; i < chip->registers;
+i++, bank--) {
+   idx = i / sizeof(*mask);
+   shift = i % sizeof(*mask) * BITS_PER_BYTE;
+   bankmask = mask[idx] >> shift;
+   if (!bankmask)
+   continue;
+
+   chip->buffer[bank] &= ~bankmask;
+   chip->buffer[bank] |= bankmask & (bits[idx] >> shift);
+   }
+   __gen_74x164_write_config(chip);
+   mutex_unlock(>lock);
+}
+
  static int gen_74x164_direction_output(struct gpio_chip *gc,
unsigned offset, int val)
  {
@@ -114,6 +137,7 @@ static int gen_74x164_probe(struct spi_device *spi)
chip->gpio_chip.direction_output = gen_74x164_direction_output;
chip->gpio_chip.get = gen_74x164_get_value;
chip->gpio_chip.set = gen_74x164_set_value;
+   chip->gpio_chip.set_multiple = gen_74x164_set_multiple;
chip->gpio_chip.base = -1;

chip->registers = nregs;




--
Regards
Phil Reid



Re: [PATCH] watchdog: don't run proc_watchdog_update if new value is same as old

2016-03-14 Thread Josh Hunt

On 03/14/2016 11:29 AM, Don Zickus wrote:


Hi Josh,

I believe Uli thought the below patch might fix it.

Cheers,
Don


Don

It looks like I was incorrect when I said 4.5 was getting the soft 
lockup. I originally found this problem on 4.1.19 and saw both the 
problem my patch solves and the soft lockups there. I thought when I 
checked 4.5 that I saw both issues there as well, but going back and 
checking now that is not the case. I only see the issue my patch 
resolves on 4.5.


With that info my changelog is incorrect now as it states I saw a soft 
lockup on the head. I will submit a v2 of my patch with the updated 
changelog. I'll also cc stable this time as I'd like to see this fix end 
up there as well.


As for the soft lockups showing up on 4.1, I tried Uli's patch and it 
did not help. After that I did a git bisect to figure out when the soft 
lockup was fixed and it appears to be resolved after one of the commits 
in this series:


commit 81a4beef91ba4a9e8ad6054ca9933dff7e25ff28
Author: Ulrich Obergfell 
Date:   Fri Sep 4 15:45:15 2015 -0700

watchdog: introduce watchdog_park_threads() and 
watchdog_unpark_threads()


I didn't identify the exact commit.

It would be nice to resolve the soft lockup for the stable folks since 
4.1 and 4.4 are longterm stable releases and would see this problem.


I did not have time to debug it any more outside of this bisection 
today. If you have something you'd like me to try which may work for the 
stable kernels I'm happy to test it.


For the record I'm able to reproduce the soft lockup on 4.1 doing:

while :; do echo 1 > /proc/sys/kernel/nmi_watchdog; sleep .1; done & 
sleep 30 && kill %1 && sleep 5


Josh



Re: [PATCH] watchdog: don't run proc_watchdog_update if new value is same as old

2016-03-14 Thread Josh Hunt

On 03/14/2016 11:29 AM, Don Zickus wrote:


Hi Josh,

I believe Uli thought the below patch might fix it.

Cheers,
Don


Don

It looks like I was incorrect when I said 4.5 was getting the soft 
lockup. I originally found this problem on 4.1.19 and saw both the 
problem my patch solves and the soft lockups there. I thought when I 
checked 4.5 that I saw both issues there as well, but going back and 
checking now that is not the case. I only see the issue my patch 
resolves on 4.5.


With that info my changelog is incorrect now as it states I saw a soft 
lockup on the head. I will submit a v2 of my patch with the updated 
changelog. I'll also cc stable this time as I'd like to see this fix end 
up there as well.


As for the soft lockups showing up on 4.1, I tried Uli's patch and it 
did not help. After that I did a git bisect to figure out when the soft 
lockup was fixed and it appears to be resolved after one of the commits 
in this series:


commit 81a4beef91ba4a9e8ad6054ca9933dff7e25ff28
Author: Ulrich Obergfell 
Date:   Fri Sep 4 15:45:15 2015 -0700

watchdog: introduce watchdog_park_threads() and 
watchdog_unpark_threads()


I didn't identify the exact commit.

It would be nice to resolve the soft lockup for the stable folks since 
4.1 and 4.4 are longterm stable releases and would see this problem.


I did not have time to debug it any more outside of this bisection 
today. If you have something you'd like me to try which may work for the 
stable kernels I'm happy to test it.


For the record I'm able to reproduce the soft lockup on 4.1 doing:

while :; do echo 1 > /proc/sys/kernel/nmi_watchdog; sleep .1; done & 
sleep 30 && kill %1 && sleep 5


Josh



RE: [RFC qemu 0/4] A PV solution for live migration optimization

2016-03-14 Thread Li, Liang Z
> > > Hi,
> > >   I'm just catching back up on this thread; so without reference to
> > > any particular previous mail in the thread.
> > >
> > >   1) How many of the free pages do we tell the host about?
> > >  Your main change is telling the host about all the
> > >  free pages.
> >
> > Yes, all the guest's free pages.
> >
> > >  If we tell the host about all the free pages, then we might
> > >  end up needing to allocate more pages and update the host
> > >  with pages we now want to use; that would have to wait for the
> > >  host to acknowledge that use of these pages, since if we don't
> > >  wait for it then it might have skipped migrating a page we
> > >  just started using (I don't understand how your series solves that).
> > >  So the guest probably needs to keep some free pages - how many?
> >
> > Actually, there is no need to care about whether the free pages will be
> used by the host.
> > We only care about some of the free pages we get reused by the guest,
> right?
> >
> > The dirty page logging can be used to solve this, starting the dirty
> > page logging before getting the free pages informant from guest. Even
> > some of the free pages are modified by the guest during the process of
> > getting the free pages information, these modified pages will be traced by
> the dirty page logging mechanism. So in the following
> migration_bitmap_sync() function.
> > The pages in the free pages bitmap, but latter was modified, will be
> > reset to dirty. We won't omit any dirtied pages.
> >
> > So, guest doesn't need to keep any free pages.
> 
> OK, yes, that works; so we do:
>   * enable dirty logging
>   * ask guest for free pages
>   * initialise the migration bitmap as everything-free
>   * then later we do the normal sync-dirty bitmap stuff and it all just works.
> 
> That's nice and simple.
> 
> > >   2) Clearing out caches
> > >  Does it make sense to clean caches?  They're apparently useful data
> > >  so if we clean them it's likely to slow the guest down; I guess
> > >  they're also likely to be fairly static data - so at least fairly
> > >  easy to migrate.
> > >  The answer here partially depends on what you want from your
> migration;
> > >  if you're after the fastest possible migration time it might make
> > >  sense to clean the caches and avoid migrating them; but that might
> > >  be at the cost of more disruption to the guest - there's a trade off
> > >  somewhere and it's not clear to me how you set that depending on
> your
> > >  guest/network/reqirements.
> > >
> >
> > Yes, clean the caches is an option.  Let the users decide using it or not.
> >
> > >   3) Why is ballooning slow?
> > >  You've got a figure of 5s to balloon on an 8GB VM - but an
> > >  8GB VM isn't huge; so I worry about how long it would take
> > >  on a big VM.   We need to understand why it's slow
> > >* is it due to the guest shuffling pages around?
> > >* is it due to the virtio-balloon protocol sending one page
> > >  at a time?
> > >  + Do balloon pages normally clump in physical memory
> > > - i.e. would a 'large balloon' message help
> > > - or do we need a bitmap because it tends not to clump?
> > >
> >
> > I didn't do a comprehensive test. But I found most of the time
> > spending on allocating the pages and sending the PFNs to guest, I
> > don't know that's the most time consuming operation, allocating the pages
> or sending the PFNs.
> 
> It might be a good idea to analyse it a bit more to convince people where the
> problem is.
> 

Yes, I will try to measure the time spending on different parts.

> > >* is it due to the madvise on the host?
> > >  If we were using the normal balloon messages, then we
> > >  could, during migration, just route those to the migration
> > >  code rather than bothering with the madvise.
> > >  If they're clumping together we could just turn that into
> > >  one big madvise; if they're not then would we benefit from
> > >  a call that lets us madvise lots of areas?
> > >
> >
> > My test showed madvise() is not the main reason for the long time,
> > only taken 10% of the total  inflating balloon operation time.
> > Big madvise can more or less improve the performance.
> 
> OK; 10% of the total is still pretty big even for your 8GB VM.
> 
> > >   4) Speeding up the migration of those free pages
> > > You're using the bitmap to avoid migrating those free pages; HPe's
> > > patchset is reconstructing a bitmap from the balloon data;  OK, so
> > > this all makes sense to avoid migrating them - I'd also been thinking
> > > of using pagemap to spot zero pages that would help find other zero'd
> > > pages, but perhaps ballooned is enough?
> > >
> > Could you describe your ideal with more details?
> 
> At the moment the migration code spends a fair amount of time 

RE: [RFC qemu 0/4] A PV solution for live migration optimization

2016-03-14 Thread Li, Liang Z
> > > Hi,
> > >   I'm just catching back up on this thread; so without reference to
> > > any particular previous mail in the thread.
> > >
> > >   1) How many of the free pages do we tell the host about?
> > >  Your main change is telling the host about all the
> > >  free pages.
> >
> > Yes, all the guest's free pages.
> >
> > >  If we tell the host about all the free pages, then we might
> > >  end up needing to allocate more pages and update the host
> > >  with pages we now want to use; that would have to wait for the
> > >  host to acknowledge that use of these pages, since if we don't
> > >  wait for it then it might have skipped migrating a page we
> > >  just started using (I don't understand how your series solves that).
> > >  So the guest probably needs to keep some free pages - how many?
> >
> > Actually, there is no need to care about whether the free pages will be
> used by the host.
> > We only care about some of the free pages we get reused by the guest,
> right?
> >
> > The dirty page logging can be used to solve this, starting the dirty
> > page logging before getting the free pages informant from guest. Even
> > some of the free pages are modified by the guest during the process of
> > getting the free pages information, these modified pages will be traced by
> the dirty page logging mechanism. So in the following
> migration_bitmap_sync() function.
> > The pages in the free pages bitmap, but latter was modified, will be
> > reset to dirty. We won't omit any dirtied pages.
> >
> > So, guest doesn't need to keep any free pages.
> 
> OK, yes, that works; so we do:
>   * enable dirty logging
>   * ask guest for free pages
>   * initialise the migration bitmap as everything-free
>   * then later we do the normal sync-dirty bitmap stuff and it all just works.
> 
> That's nice and simple.
> 
> > >   2) Clearing out caches
> > >  Does it make sense to clean caches?  They're apparently useful data
> > >  so if we clean them it's likely to slow the guest down; I guess
> > >  they're also likely to be fairly static data - so at least fairly
> > >  easy to migrate.
> > >  The answer here partially depends on what you want from your
> migration;
> > >  if you're after the fastest possible migration time it might make
> > >  sense to clean the caches and avoid migrating them; but that might
> > >  be at the cost of more disruption to the guest - there's a trade off
> > >  somewhere and it's not clear to me how you set that depending on
> your
> > >  guest/network/reqirements.
> > >
> >
> > Yes, clean the caches is an option.  Let the users decide using it or not.
> >
> > >   3) Why is ballooning slow?
> > >  You've got a figure of 5s to balloon on an 8GB VM - but an
> > >  8GB VM isn't huge; so I worry about how long it would take
> > >  on a big VM.   We need to understand why it's slow
> > >* is it due to the guest shuffling pages around?
> > >* is it due to the virtio-balloon protocol sending one page
> > >  at a time?
> > >  + Do balloon pages normally clump in physical memory
> > > - i.e. would a 'large balloon' message help
> > > - or do we need a bitmap because it tends not to clump?
> > >
> >
> > I didn't do a comprehensive test. But I found most of the time
> > spending on allocating the pages and sending the PFNs to guest, I
> > don't know that's the most time consuming operation, allocating the pages
> or sending the PFNs.
> 
> It might be a good idea to analyse it a bit more to convince people where the
> problem is.
> 

Yes, I will try to measure the time spending on different parts.

> > >* is it due to the madvise on the host?
> > >  If we were using the normal balloon messages, then we
> > >  could, during migration, just route those to the migration
> > >  code rather than bothering with the madvise.
> > >  If they're clumping together we could just turn that into
> > >  one big madvise; if they're not then would we benefit from
> > >  a call that lets us madvise lots of areas?
> > >
> >
> > My test showed madvise() is not the main reason for the long time,
> > only taken 10% of the total  inflating balloon operation time.
> > Big madvise can more or less improve the performance.
> 
> OK; 10% of the total is still pretty big even for your 8GB VM.
> 
> > >   4) Speeding up the migration of those free pages
> > > You're using the bitmap to avoid migrating those free pages; HPe's
> > > patchset is reconstructing a bitmap from the balloon data;  OK, so
> > > this all makes sense to avoid migrating them - I'd also been thinking
> > > of using pagemap to spot zero pages that would help find other zero'd
> > > pages, but perhaps ballooned is enough?
> > >
> > Could you describe your ideal with more details?
> 
> At the moment the migration code spends a fair amount of time 

Re: [PATCH 20/22] atari_scsi: Set a reasonable default for cmd_per_lun

2016-03-14 Thread Finn Thain

On Mon, 14 Mar 2016, Hannes Reinecke wrote:

> On 03/14/2016 05:27 AM, Finn Thain wrote:
> > This setting does not need to be conditional on Atari ST or TT.
> > 
> > Without TCQ support, cmd_per_lun == 2 is probably reasonable...
> > 
> > Signed-off-by: Finn Thain 
> > 
> > ---
> >  drivers/scsi/atari_scsi.c |3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > Index: linux/drivers/scsi/atari_scsi.c
> > ===
> > --- linux.orig/drivers/scsi/atari_scsi.c2016-03-14 15:26:45.0 
> > +1100
> > +++ linux/drivers/scsi/atari_scsi.c 2016-03-14 15:26:55.0 +1100
> > @@ -750,6 +750,7 @@ static struct scsi_host_template atari_s
> > .eh_abort_handler   = atari_scsi_abort,
> > .eh_bus_reset_handler   = atari_scsi_bus_reset,
> > .this_id= 7,
> > +   .cmd_per_lun= 2,
> > .use_clustering = DISABLE_CLUSTERING,
> > .cmd_size   = NCR5380_CMD_SIZE,
> >  };
> _2_ ? Are you being overly cheeky here?
> I sincerely doubt the driver is capable of submitting two
> simultaneous commands ...

Right. The LLD has LU busy flags to prevent a LU from being issued more 
than one command.

> Care to explain?

It seemed harmless and it is consistent with the all of the other 5380 
drivers.

I don't know why it was done that way. Perhaps it was done to create a 
pipeline. That is, to keep a small number of commands in the LLD issue 
queue so that the NCR5380_main() work item does not have to terminate and 
then get requeued needlessly.

-- 

> 
> Cheers,
> 
> Hannes
> 


Re: [PATCH 20/22] atari_scsi: Set a reasonable default for cmd_per_lun

2016-03-14 Thread Finn Thain

On Mon, 14 Mar 2016, Hannes Reinecke wrote:

> On 03/14/2016 05:27 AM, Finn Thain wrote:
> > This setting does not need to be conditional on Atari ST or TT.
> > 
> > Without TCQ support, cmd_per_lun == 2 is probably reasonable...
> > 
> > Signed-off-by: Finn Thain 
> > 
> > ---
> >  drivers/scsi/atari_scsi.c |3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > Index: linux/drivers/scsi/atari_scsi.c
> > ===
> > --- linux.orig/drivers/scsi/atari_scsi.c2016-03-14 15:26:45.0 
> > +1100
> > +++ linux/drivers/scsi/atari_scsi.c 2016-03-14 15:26:55.0 +1100
> > @@ -750,6 +750,7 @@ static struct scsi_host_template atari_s
> > .eh_abort_handler   = atari_scsi_abort,
> > .eh_bus_reset_handler   = atari_scsi_bus_reset,
> > .this_id= 7,
> > +   .cmd_per_lun= 2,
> > .use_clustering = DISABLE_CLUSTERING,
> > .cmd_size   = NCR5380_CMD_SIZE,
> >  };
> _2_ ? Are you being overly cheeky here?
> I sincerely doubt the driver is capable of submitting two
> simultaneous commands ...

Right. The LLD has LU busy flags to prevent a LU from being issued more 
than one command.

> Care to explain?

It seemed harmless and it is consistent with the all of the other 5380 
drivers.

I don't know why it was done that way. Perhaps it was done to create a 
pipeline. That is, to keep a small number of commands in the LLD issue 
queue so that the NCR5380_main() work item does not have to terminate and 
then get requeued needlessly.

-- 

> 
> Cheers,
> 
> Hannes
> 


Re: [PATCH 16/22] ncr5380: Fix register decoding for debugging

2016-03-14 Thread Finn Thain

On Mon, 14 Mar 2016, Hannes Reinecke wrote:

> Well ... using individual printk() like here might end up in each
> call to be broken up into individual lines.
> You might want to consider using a line buffer here or, better
> still, move to seq_file or debugfs output.

This routine is meant to be called with the lock held in irq mode. I made 
it the caller's responsibility to do the locking. It is the only way to 
get a point-in-time snapshot of all of the device registers.

-- 

> 
> But as the original code did that, too:
> 
> Reviewed-by: Hannes Reinecke 
> 
> Cheers,
> 
> Hannes
> 


Re: [PATCH 16/22] ncr5380: Fix register decoding for debugging

2016-03-14 Thread Finn Thain

On Mon, 14 Mar 2016, Hannes Reinecke wrote:

> Well ... using individual printk() like here might end up in each
> call to be broken up into individual lines.
> You might want to consider using a line buffer here or, better
> still, move to seq_file or debugfs output.

This routine is meant to be called with the lock held in irq mode. I made 
it the caller's responsibility to do the locking. It is the only way to 
get a point-in-time snapshot of all of the device registers.

-- 

> 
> But as the original code did that, too:
> 
> Reviewed-by: Hannes Reinecke 
> 
> Cheers,
> 
> Hannes
> 


Re: [PATCH 14/22] ncr5380: Add MAX_LUN limit

2016-03-14 Thread Finn Thain

On Mon, 14 Mar 2016, Christoph Hellwig wrote:

> On Mon, Mar 14, 2016 at 03:27:14PM +1100, Finn Thain wrote:
> > The driver has a limit of eight LUs because of the byte-sized bitfield
> > that is used for busy flags. Reject commands with LUN > 7.
> 
> Please just set the max_lun parameter in the host template, which will 
> take care of this.

The reason I didn't do that initially was that I could not convince myself 
that all command submission paths would enforce the shost->max_lun limit. 
But I'll take your word for it.

I don't think the host template is the right place for this (there are 
about ten of these that would need to be changed anyway) because 
scsi_host_alloc() assigns shost->max_lun = 8. So I'll add the 
instance->max_lun = 7 assignment to NCR5380_init().

-- 


Re: [PATCH 14/22] ncr5380: Add MAX_LUN limit

2016-03-14 Thread Finn Thain

On Mon, 14 Mar 2016, Christoph Hellwig wrote:

> On Mon, Mar 14, 2016 at 03:27:14PM +1100, Finn Thain wrote:
> > The driver has a limit of eight LUs because of the byte-sized bitfield
> > that is used for busy flags. Reject commands with LUN > 7.
> 
> Please just set the max_lun parameter in the host template, which will 
> take care of this.

The reason I didn't do that initially was that I could not convince myself 
that all command submission paths would enforce the shost->max_lun limit. 
But I'll take your word for it.

I don't think the host template is the right place for this (there are 
about ten of these that would need to be changed anyway) because 
scsi_host_alloc() assigns shost->max_lun = 8. So I'll add the 
instance->max_lun = 7 assignment to NCR5380_init().

-- 


Re: [PATCH 09/22] ncr5380: Adopt uniform DMA setup convention

2016-03-14 Thread Finn Thain

On Mon, 14 Mar 2016, Hannes Reinecke wrote:

> > @@ -1555,8 +1555,7 @@ static int NCR5380_transfer_dma(struct S
> > NCR5380_read(RESET_PARITY_INTERRUPT_REG);
> > *data = d + c;
> > *count = 0;
> > -   *phase = NCR5380_read(STATUS_REG) & PHASE_MASK;
> > -   return foo;
> > +   return result;
> >  }
> >  
> >  /*
> 
> Don't you miss a phase update here?

I guess I missed explaining the change in the commit log.

The *phase assignment is redundant because after NCR5380_transfer_dma() 
returns control to NCR5380_information_transfer(), the latter routine then 
also returns, and so *phase is dead.

-- 

> 
> Cheers,
> 
> Hannes
> 


Re: [PATCH 09/22] ncr5380: Adopt uniform DMA setup convention

2016-03-14 Thread Finn Thain

On Mon, 14 Mar 2016, Hannes Reinecke wrote:

> > @@ -1555,8 +1555,7 @@ static int NCR5380_transfer_dma(struct S
> > NCR5380_read(RESET_PARITY_INTERRUPT_REG);
> > *data = d + c;
> > *count = 0;
> > -   *phase = NCR5380_read(STATUS_REG) & PHASE_MASK;
> > -   return foo;
> > +   return result;
> >  }
> >  
> >  /*
> 
> Don't you miss a phase update here?

I guess I missed explaining the change in the commit log.

The *phase assignment is redundant because after NCR5380_transfer_dma() 
returns control to NCR5380_information_transfer(), the latter routine then 
also returns, and so *phase is dead.

-- 

> 
> Cheers,
> 
> Hannes
> 


Re: [PATCH 04/22] atari_NCR5380: Remove DMA_MIN_SIZE macro

2016-03-14 Thread Finn Thain

On Mon, 14 Mar 2016, Hannes Reinecke wrote:

> I still would keep the 'DMA_MIN_LEN' definition around for sun3_scsi.c; 
> makes it a bit more obvious what the magic number '129' is supposed to 
> mean.

OK.

-- 


Re: [PATCH 04/22] atari_NCR5380: Remove DMA_MIN_SIZE macro

2016-03-14 Thread Finn Thain

On Mon, 14 Mar 2016, Hannes Reinecke wrote:

> I still would keep the 'DMA_MIN_LEN' definition around for sun3_scsi.c; 
> makes it a bit more obvious what the magic number '129' is supposed to 
> mean.

OK.

-- 


Re: [PATCH V4 0/3] basic busy polling support for vhost_net

2016-03-14 Thread Jason Wang


On 03/10/2016 02:48 PM, Michael Rapoport wrote:
> Hi Greg,
>
>> > Greg Kurz  wrote on 03/09/2016 09:26:45 PM:
>>> > > On Fri,  4 Mar 2016 06:24:50 -0500
>>> > > Jason Wang  wrote:
>> > 
>>> > > This series tries to add basic busy polling for vhost net. The idea is
>>> > > simple: at the end of tx/rx processing, busy polling for new tx added
>>> > > descriptor and rx receive socket for a while. The maximum number of
>>> > > time (in us) could be spent on busy polling was specified ioctl.
>>> > > 
>>> > > Test A were done through:
>>> > > 
>>> > > - 50 us as busy loop timeout
>>> > > - Netperf 2.6
>>> > > - Two machines with back to back connected mlx4
>> > 
>> > Hi Jason,
>> > 
>> > Could this also improve performance if both VMs are
>> > on the same host system ?
> I've experimented a little with Jason's patches and guest-to-guest netperf 
> when both guests were on the same host, and I saw improvements for that 
> case.
>  

Good to know this, I haven't tested this before but from the codes, it
should work for VM2VM case too.

Thanks a lot for the testing.


Re: [PATCH V4 0/3] basic busy polling support for vhost_net

2016-03-14 Thread Jason Wang


On 03/10/2016 02:48 PM, Michael Rapoport wrote:
> Hi Greg,
>
>> > Greg Kurz  wrote on 03/09/2016 09:26:45 PM:
>>> > > On Fri,  4 Mar 2016 06:24:50 -0500
>>> > > Jason Wang  wrote:
>> > 
>>> > > This series tries to add basic busy polling for vhost net. The idea is
>>> > > simple: at the end of tx/rx processing, busy polling for new tx added
>>> > > descriptor and rx receive socket for a while. The maximum number of
>>> > > time (in us) could be spent on busy polling was specified ioctl.
>>> > > 
>>> > > Test A were done through:
>>> > > 
>>> > > - 50 us as busy loop timeout
>>> > > - Netperf 2.6
>>> > > - Two machines with back to back connected mlx4
>> > 
>> > Hi Jason,
>> > 
>> > Could this also improve performance if both VMs are
>> > on the same host system ?
> I've experimented a little with Jason's patches and guest-to-guest netperf 
> when both guests were on the same host, and I saw improvements for that 
> case.
>  

Good to know this, I haven't tested this before but from the codes, it
should work for VM2VM case too.

Thanks a lot for the testing.


[PATCH v2 3/5] dt-bindings: Add documentation for GM20B GPU

2016-03-14 Thread Alexandre Courbot
GM20B's definition is mostly similar to GK20A's, but requires an
additional clock.

Signed-off-by: Alexandre Courbot 
---
 .../devicetree/bindings/gpu/nvidia,gk20a.txt   | 27 --
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt 
b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
index 1e3748337319..d9ad6b87fbbc 100644
--- a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
+++ b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
@@ -1,9 +1,10 @@
-NVIDIA GK20A Graphics Processing Unit
+NVIDIA Tegra Graphics Processing Units
 
 Required properties:
 - compatible: "nvidia,"
   Currently recognized values:
   - nvidia,gk20a
+  - nvidia,gm20b
 - reg: Physical base address and length of the controller's registers.
   Must contain two entries:
   - first entry for bar0
@@ -19,6 +20,9 @@ Required properties:
 - clock-names: Must include the following entries:
   - gpu
   - pwr
+If the compatible string is "nvidia,gm20b", then the following clock
+is also required:
+  - ref
 - resets: Must contain an entry for each entry in reset-names.
   See ../reset/reset.txt for details.
 - reset-names: Must include the following entries:
@@ -27,7 +31,7 @@ Required properties:
 Optional properties:
 - iommus: A reference to the IOMMU. See ../iommu/iommu.txt for details.
 
-Example:
+Example for GK20A:
 
gpu@0,5700 {
compatible = "nvidia,gk20a";
@@ -45,3 +49,22 @@ Example:
iommus = < TEGRA_SWGROUP_GPU>;
status = "disabled";
};
+
+Example for GM20B:
+
+   gpu@0,5700 {
+   compatible = "nvidia,gm20b";
+   reg = <0x0 0x5700 0x0 0x0100>,
+ <0x0 0x5800 0x0 0x0100>;
+   interrupts = ,
+;
+   interrupt-names = "stall", "nonstall";
+   clocks = <_car TEGRA210_CLK_GPU>,
+<_car TEGRA210_CLK_PLL_P_OUT5>,
+<_car TEGRA210_CLK_PLL_G_REF>;
+   clock-names = "gpu", "pwr", "ref";
+   resets = <_car 184>;
+   reset-names = "gpu";
+   iommus = < TEGRA_SWGROUP_GPU>;
+   status = "disabled";
+   };
-- 
2.7.2



[PATCH v2 1/5] dt-bindings: gk20a: Fix typo in compatible name

2016-03-14 Thread Alexandre Courbot
The correct compatible name is "nvidia,gk20a".

Signed-off-by: Alexandre Courbot 
---
 Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt 
b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
index 23bfe8e1f7cc..914f0ff4020e 100644
--- a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
+++ b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
@@ -1,9 +1,9 @@
 NVIDIA GK20A Graphics Processing Unit
 
 Required properties:
-- compatible: "nvidia,-"
+- compatible: "nvidia,"
   Currently recognized values:
-  - nvidia,tegra124-gk20a
+  - nvidia,gk20a
 - reg: Physical base address and length of the controller's registers.
   Must contain two entries:
   - first entry for bar0
-- 
2.7.2



[PATCH v2 3/5] dt-bindings: Add documentation for GM20B GPU

2016-03-14 Thread Alexandre Courbot
GM20B's definition is mostly similar to GK20A's, but requires an
additional clock.

Signed-off-by: Alexandre Courbot 
---
 .../devicetree/bindings/gpu/nvidia,gk20a.txt   | 27 --
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt 
b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
index 1e3748337319..d9ad6b87fbbc 100644
--- a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
+++ b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
@@ -1,9 +1,10 @@
-NVIDIA GK20A Graphics Processing Unit
+NVIDIA Tegra Graphics Processing Units
 
 Required properties:
 - compatible: "nvidia,"
   Currently recognized values:
   - nvidia,gk20a
+  - nvidia,gm20b
 - reg: Physical base address and length of the controller's registers.
   Must contain two entries:
   - first entry for bar0
@@ -19,6 +20,9 @@ Required properties:
 - clock-names: Must include the following entries:
   - gpu
   - pwr
+If the compatible string is "nvidia,gm20b", then the following clock
+is also required:
+  - ref
 - resets: Must contain an entry for each entry in reset-names.
   See ../reset/reset.txt for details.
 - reset-names: Must include the following entries:
@@ -27,7 +31,7 @@ Required properties:
 Optional properties:
 - iommus: A reference to the IOMMU. See ../iommu/iommu.txt for details.
 
-Example:
+Example for GK20A:
 
gpu@0,5700 {
compatible = "nvidia,gk20a";
@@ -45,3 +49,22 @@ Example:
iommus = < TEGRA_SWGROUP_GPU>;
status = "disabled";
};
+
+Example for GM20B:
+
+   gpu@0,5700 {
+   compatible = "nvidia,gm20b";
+   reg = <0x0 0x5700 0x0 0x0100>,
+ <0x0 0x5800 0x0 0x0100>;
+   interrupts = ,
+;
+   interrupt-names = "stall", "nonstall";
+   clocks = <_car TEGRA210_CLK_GPU>,
+<_car TEGRA210_CLK_PLL_P_OUT5>,
+<_car TEGRA210_CLK_PLL_G_REF>;
+   clock-names = "gpu", "pwr", "ref";
+   resets = <_car 184>;
+   reset-names = "gpu";
+   iommus = < TEGRA_SWGROUP_GPU>;
+   status = "disabled";
+   };
-- 
2.7.2



[PATCH v2 1/5] dt-bindings: gk20a: Fix typo in compatible name

2016-03-14 Thread Alexandre Courbot
The correct compatible name is "nvidia,gk20a".

Signed-off-by: Alexandre Courbot 
---
 Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt 
b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
index 23bfe8e1f7cc..914f0ff4020e 100644
--- a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
+++ b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
@@ -1,9 +1,9 @@
 NVIDIA GK20A Graphics Processing Unit
 
 Required properties:
-- compatible: "nvidia,-"
+- compatible: "nvidia,"
   Currently recognized values:
-  - nvidia,tegra124-gk20a
+  - nvidia,gk20a
 - reg: Physical base address and length of the controller's registers.
   Must contain two entries:
   - first entry for bar0
-- 
2.7.2



[PATCH v2 0/5] arm64: tegra: fix DT definitions for GM20B GPU

2016-03-14 Thread Alexandre Courbot
Small series that fixes/completes DT bindings for Tegra GPUs and add two
missing entries required for the Tegra210 GPU to operate properly.

Changes since v1:
- Renamed "pllg_ref" clock to "ref" in DT bindings

Alexandre Courbot (5):
  dt-bindings: gk20a: Fix typo in compatible name
  dt-bindings: gk20a: Document iommus property
  dt-bindings: Add documentation for GM20B GPU
  arm64: tegra210: Add reference clock to GM20B
  arm64: tegra210: Add IOMMU node to GM20B

 .../devicetree/bindings/gpu/nvidia,gk20a.txt   | 35 +++---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi   |  8 +++--
 2 files changed, 37 insertions(+), 6 deletions(-)

-- 
2.7.2



[PATCH v2 0/5] arm64: tegra: fix DT definitions for GM20B GPU

2016-03-14 Thread Alexandre Courbot
Small series that fixes/completes DT bindings for Tegra GPUs and add two
missing entries required for the Tegra210 GPU to operate properly.

Changes since v1:
- Renamed "pllg_ref" clock to "ref" in DT bindings

Alexandre Courbot (5):
  dt-bindings: gk20a: Fix typo in compatible name
  dt-bindings: gk20a: Document iommus property
  dt-bindings: Add documentation for GM20B GPU
  arm64: tegra210: Add reference clock to GM20B
  arm64: tegra210: Add IOMMU node to GM20B

 .../devicetree/bindings/gpu/nvidia,gk20a.txt   | 35 +++---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi   |  8 +++--
 2 files changed, 37 insertions(+), 6 deletions(-)

-- 
2.7.2



[PATCH v2 4/5] arm64: tegra210: Add reference clock to GM20B

2016-03-14 Thread Alexandre Courbot
This clock is required for the GPU to operate.

Signed-off-by: Alexandre Courbot 
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi 
b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 362c269946ff..04898cb25f0c 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -309,8 +309,9 @@
 ;
interrupt-names = "stall", "nonstall";
clocks = <_car TEGRA210_CLK_GPU>,
-<_car TEGRA210_CLK_PLL_P_OUT5>;
-   clock-names = "gpu", "pwr";
+<_car TEGRA210_CLK_PLL_P_OUT5>,
+<_car TEGRA210_CLK_PLL_G_REF>;
+   clock-names = "gpu", "pwr", "ref";
resets = <_car 184>;
reset-names = "gpu";
status = "disabled";
-- 
2.7.2



Re: [PATCH v3] rtc: s3c: Don't print an error on probe deferral

2016-03-14 Thread Javier Martinez Canillas
Hello Krzysztof,

On 03/14/2016 11:26 PM, Krzysztof Kozlowski wrote:
> On 15.03.2016 10:59, Javier Martinez Canillas wrote:
  
if (info->data->needs_src_clk) {
info->rtc_src_clk = devm_clk_get(>dev, "rtc_src");
if (IS_ERR(info->rtc_src_clk)) {
 -  dev_err(>dev,
 -  "failed to find rtc source clock\n");
 +  ret = PTR_ERR(info->rtc_src_clk);
 +  if (ret != -EPROBE_DEFER)
 +  dev_err(>dev,
 +  "failed to find rtc source clock\n");
 +  else
 +  dev_dbg(>dev,
 +  "probe deferred due to missing rtc src 
 clk\n");
clk_disable_unprepare(info->rtc_clk);
 -  return PTR_ERR(info->rtc_src_clk);
 +  return ret;
}
clk_prepare_enable(info->rtc_src_clk);
}

>>>
>>> The error path starts looking complicated. This has now 4 indentation
>>> levels...
>>>
>>
>> Yeah, I don't think we can get rid of the 4 indentation levels since
>> the function already has 3 and a check for the errno code is needed.
> 
> Probably handling of the clocks in the driver could be simplified a
> little bit (the if(needs_src_clk) appears in few places)... but this is
> out of scope for this patch.
>

Agreed, I meant without introducing an unrelated change.
 
>>
>>> I agree for removal of error in case of probe deferral because it might
>>> be misleading but I don't see much benefit of a debug message.
>>>
>>
>> But yes, we can at least get rid of the else statement. I don't have a
>> strong opinion about the debug information, I left it to avoid someone
>> to tell me that I was removing a useful log.
> 
> Although dev_dbg doesn't harm... but isn't driver core printing debug
> message already?
>

I don't think it does or at least I didn't find it when looking
at the devm_clk_get() call chain.
 
> BR,
> Krzysztof
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America


[PATCH v2 4/5] arm64: tegra210: Add reference clock to GM20B

2016-03-14 Thread Alexandre Courbot
This clock is required for the GPU to operate.

Signed-off-by: Alexandre Courbot 
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi 
b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 362c269946ff..04898cb25f0c 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -309,8 +309,9 @@
 ;
interrupt-names = "stall", "nonstall";
clocks = <_car TEGRA210_CLK_GPU>,
-<_car TEGRA210_CLK_PLL_P_OUT5>;
-   clock-names = "gpu", "pwr";
+<_car TEGRA210_CLK_PLL_P_OUT5>,
+<_car TEGRA210_CLK_PLL_G_REF>;
+   clock-names = "gpu", "pwr", "ref";
resets = <_car 184>;
reset-names = "gpu";
status = "disabled";
-- 
2.7.2



Re: [PATCH v3] rtc: s3c: Don't print an error on probe deferral

2016-03-14 Thread Javier Martinez Canillas
Hello Krzysztof,

On 03/14/2016 11:26 PM, Krzysztof Kozlowski wrote:
> On 15.03.2016 10:59, Javier Martinez Canillas wrote:
  
if (info->data->needs_src_clk) {
info->rtc_src_clk = devm_clk_get(>dev, "rtc_src");
if (IS_ERR(info->rtc_src_clk)) {
 -  dev_err(>dev,
 -  "failed to find rtc source clock\n");
 +  ret = PTR_ERR(info->rtc_src_clk);
 +  if (ret != -EPROBE_DEFER)
 +  dev_err(>dev,
 +  "failed to find rtc source clock\n");
 +  else
 +  dev_dbg(>dev,
 +  "probe deferred due to missing rtc src 
 clk\n");
clk_disable_unprepare(info->rtc_clk);
 -  return PTR_ERR(info->rtc_src_clk);
 +  return ret;
}
clk_prepare_enable(info->rtc_src_clk);
}

>>>
>>> The error path starts looking complicated. This has now 4 indentation
>>> levels...
>>>
>>
>> Yeah, I don't think we can get rid of the 4 indentation levels since
>> the function already has 3 and a check for the errno code is needed.
> 
> Probably handling of the clocks in the driver could be simplified a
> little bit (the if(needs_src_clk) appears in few places)... but this is
> out of scope for this patch.
>

Agreed, I meant without introducing an unrelated change.
 
>>
>>> I agree for removal of error in case of probe deferral because it might
>>> be misleading but I don't see much benefit of a debug message.
>>>
>>
>> But yes, we can at least get rid of the else statement. I don't have a
>> strong opinion about the debug information, I left it to avoid someone
>> to tell me that I was removing a useful log.
> 
> Although dev_dbg doesn't harm... but isn't driver core printing debug
> message already?
>

I don't think it does or at least I didn't find it when looking
at the devm_clk_get() call chain.
 
> BR,
> Krzysztof
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America


[PATCH v2 2/5] dt-bindings: gk20a: Document iommus property

2016-03-14 Thread Alexandre Courbot
GK20A can optionally make use of an IOMMU.

Signed-off-by: Alexandre Courbot 
---
 Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt 
b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
index 914f0ff4020e..1e3748337319 100644
--- a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
+++ b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
@@ -24,6 +24,9 @@ Required properties:
 - reset-names: Must include the following entries:
   - gpu
 
+Optional properties:
+- iommus: A reference to the IOMMU. See ../iommu/iommu.txt for details.
+
 Example:
 
gpu@0,5700 {
@@ -39,5 +42,6 @@ Example:
clock-names = "gpu", "pwr";
resets = <_car 184>;
reset-names = "gpu";
+   iommus = < TEGRA_SWGROUP_GPU>;
status = "disabled";
};
-- 
2.7.2



[PATCH v2 5/5] arm64: tegra210: Add IOMMU node to GM20B

2016-03-14 Thread Alexandre Courbot
Nouveau can take advantage of this declaration to remove the need for
contiguous memory.

Signed-off-by: Alexandre Courbot 
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi 
b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 04898cb25f0c..478543f74863 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -314,6 +314,9 @@
clock-names = "gpu", "pwr", "ref";
resets = <_car 184>;
reset-names = "gpu";
+
+   iommus = < TEGRA_SWGROUP_GPU>;
+
status = "disabled";
};
 
-- 
2.7.2



[PATCH v2 2/5] dt-bindings: gk20a: Document iommus property

2016-03-14 Thread Alexandre Courbot
GK20A can optionally make use of an IOMMU.

Signed-off-by: Alexandre Courbot 
---
 Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt 
b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
index 914f0ff4020e..1e3748337319 100644
--- a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
+++ b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
@@ -24,6 +24,9 @@ Required properties:
 - reset-names: Must include the following entries:
   - gpu
 
+Optional properties:
+- iommus: A reference to the IOMMU. See ../iommu/iommu.txt for details.
+
 Example:
 
gpu@0,5700 {
@@ -39,5 +42,6 @@ Example:
clock-names = "gpu", "pwr";
resets = <_car 184>;
reset-names = "gpu";
+   iommus = < TEGRA_SWGROUP_GPU>;
status = "disabled";
};
-- 
2.7.2



[PATCH v2 5/5] arm64: tegra210: Add IOMMU node to GM20B

2016-03-14 Thread Alexandre Courbot
Nouveau can take advantage of this declaration to remove the need for
contiguous memory.

Signed-off-by: Alexandre Courbot 
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi 
b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 04898cb25f0c..478543f74863 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -314,6 +314,9 @@
clock-names = "gpu", "pwr", "ref";
resets = <_car 184>;
reset-names = "gpu";
+
+   iommus = < TEGRA_SWGROUP_GPU>;
+
status = "disabled";
};
 
-- 
2.7.2



Re: [PATCH V2 net 0/6] net: hns: hns driver updates

2016-03-14 Thread David Miller
From: Daode Huang 
Date: Tue, 15 Mar 2016 09:56:02 +0800

> Could you please help me to review this patch set?

I am not reviewing anything until you guys sort out your submission
scheme, and resend these fresh using that central maintainer.

I am also not the only person in the world who is supposed to review
all of this stuff, other developers need to help with the review
process as well.

So it is never appropriate to ask me, and only me, to review your
work.

Thanks.


Re: [PATCH] epoll: add exclusive wakeups flag

2016-03-14 Thread Jason Baron
Hi Michael,

On 03/14/2016 07:26 PM, Michael Kerrisk (man-pages) wrote:
> Hi Jason,
> 
> On 03/15/2016 11:35 AM, Jason Baron wrote:
>> Hi Michael,
>>
>> On 03/14/2016 05:03 PM, Michael Kerrisk (man-pages) wrote:
>>> Hi Jason,
>>>
>>> On 03/15/2016 09:01 AM, Michael Kerrisk (man-pages) wrote:
 Hi Jason,

 On 03/15/2016 08:32 AM, Jason Baron wrote:
>
>
> On 03/14/2016 01:47 PM, Michael Kerrisk (man-pages) wrote:
>> [Restoring CC, which I see I accidentally dropped, one iteration back.]
>>>
>>> [...]
>>>
>> Returning to the second sentence in this description:
>>
>>   When a wakeup event occurs and multiple epoll file descrip‐
>>   tors are attached to the same target file using EPOLLEXCLU‐
>>   SIVE, one or  more  of  the  epoll  file  descriptors  will
>>   receive  an  event with epoll_wait(2).
>>
>> There is a point that is unclear to me: what does "target file" refer to?
>> Is it an open file description (aka open file table entry) or an inode?
>> I suspect the former, but it was not clear in your original text.
>>
>
> So from epoll's perspective, the wakeups are associated with a 'wait
> queue'. So if the open() and subsequent EPOLL_CTL_ADD (which is done via
> file->poll()) results in adding to the same 'wait queue' then we will
> get 'exclusive' wakeup behavior.
>
> So in general, I think the answer here is that its associated with the
> inode (I coudn't say with 100% certainty without really looking at all
> file->poll() implementations). Certainly, with the 'FIFO' example below,
> the two scenarios will have the same behavior with respect to
> EPOLLEXCLUSIVE.
>>>
>>> So, I was actually a little surprised by this, and went away and tested
>>> this point. It appears to me that that the two scenarios described below
>>> do NOT have the same behavior with respect to EPOLLEXCLUSIVE. See below.
>>>
 So, in both scenarios, *one or more* processes will get a wakeup?
 (I'll try to add something to the text to clarify the detail we're 
 discussing.)

> Also, the 'non-exclusive' mode would be subject to the same question of
> which wait queue is the epfd is associated with...

 I'm not sure of the point you are trying to make here?

 Cheers,

 Michael


>> To make this point even clearer, here are two scenarios I'm thinking of.
>> In each case, we're talking of monitoring the read end of a FIFO.
>>
>> ===
>>
>> Scenario 1:
>>
>> We have three processes each of which
>> 1. Creates an epoll instance
>> 2. Opens the read end of the FIFO
>> 3. Adds the read end of the FIFO to the epoll instance, specifying
>>EPOLLEXCLUSIVE
>>
>> When input becomes available on the FIFO, how many processes
>> get a wakeup?
>>>
>>> When I test this scenario, all three processes get a wakeup.
>>>
>> ===
>>
>> Scenario 3
>>
>> A parent process opens the read end of a FIFO and then calls
>> fork() three times to create three children. Each child then:
>>
>> 1. Creates an epoll instance
>> 2. Adds the read end of the FIFO to the epoll instance, specifying
>> EPOLLEXCLUSIVE
>>
>> When input becomes available on the FIFO, how many processes
>> get a wakeup?
>>>
>>> When I test this scenario, one process gets a wakeup.
>>>
>>> In other words, "target file" appears to mean open file description
>>> (aka open file table entry), not inode.
>>>
>>> This is actually what I suspected might be the case, but now I am
>>> puzzled. Given what I've discovered and what you suggest are the
>>> semantics, is the implementation correct? (I suspect that it is,
>>> but it is at odds with your statement above. My test programs are
>>> inline below.
>>>
>>> Cheers,
>>>
>>> Michael
>>>
>>
>> Thanks for the test cases. So in your first test case, you are exiting
>> immediately after the epoll_wait() returns. So this is actually causing
>> the next wakeup. 
> 
> Can I just check my understanding of the rationale for the preceding 
> point. The next process is getting woken up, because the previous process
> did not "consume" the event (that is, the input is still available on the 
> FIFO). Right?
> 
>> And then the 2nd thread returns from epoll_wait() and
>> this causes the 3rd wakeup.
> 
> I added the sleep() calls, but still things don't seem to happen
> quite as you suggest. In the first scenario, after the first process
> terminates, *all* of the remaining processes wake from epoll_wait().
> What's happening in this case? (This smells like a possible bug.)

Yes, you are right. When the first process exits() and thus closes
the read-side of the pipe, it will wake up all the other calls that
are in epoll_wait(). When the file closes, since there are no more
fds referencing it, the pipe_release() routine does a wakeup specifying

Re: [PATCH V2 net 0/6] net: hns: hns driver updates

2016-03-14 Thread David Miller
From: Daode Huang 
Date: Tue, 15 Mar 2016 09:56:02 +0800

> Could you please help me to review this patch set?

I am not reviewing anything until you guys sort out your submission
scheme, and resend these fresh using that central maintainer.

I am also not the only person in the world who is supposed to review
all of this stuff, other developers need to help with the review
process as well.

So it is never appropriate to ask me, and only me, to review your
work.

Thanks.


Re: [PATCH] epoll: add exclusive wakeups flag

2016-03-14 Thread Jason Baron
Hi Michael,

On 03/14/2016 07:26 PM, Michael Kerrisk (man-pages) wrote:
> Hi Jason,
> 
> On 03/15/2016 11:35 AM, Jason Baron wrote:
>> Hi Michael,
>>
>> On 03/14/2016 05:03 PM, Michael Kerrisk (man-pages) wrote:
>>> Hi Jason,
>>>
>>> On 03/15/2016 09:01 AM, Michael Kerrisk (man-pages) wrote:
 Hi Jason,

 On 03/15/2016 08:32 AM, Jason Baron wrote:
>
>
> On 03/14/2016 01:47 PM, Michael Kerrisk (man-pages) wrote:
>> [Restoring CC, which I see I accidentally dropped, one iteration back.]
>>>
>>> [...]
>>>
>> Returning to the second sentence in this description:
>>
>>   When a wakeup event occurs and multiple epoll file descrip‐
>>   tors are attached to the same target file using EPOLLEXCLU‐
>>   SIVE, one or  more  of  the  epoll  file  descriptors  will
>>   receive  an  event with epoll_wait(2).
>>
>> There is a point that is unclear to me: what does "target file" refer to?
>> Is it an open file description (aka open file table entry) or an inode?
>> I suspect the former, but it was not clear in your original text.
>>
>
> So from epoll's perspective, the wakeups are associated with a 'wait
> queue'. So if the open() and subsequent EPOLL_CTL_ADD (which is done via
> file->poll()) results in adding to the same 'wait queue' then we will
> get 'exclusive' wakeup behavior.
>
> So in general, I think the answer here is that its associated with the
> inode (I coudn't say with 100% certainty without really looking at all
> file->poll() implementations). Certainly, with the 'FIFO' example below,
> the two scenarios will have the same behavior with respect to
> EPOLLEXCLUSIVE.
>>>
>>> So, I was actually a little surprised by this, and went away and tested
>>> this point. It appears to me that that the two scenarios described below
>>> do NOT have the same behavior with respect to EPOLLEXCLUSIVE. See below.
>>>
 So, in both scenarios, *one or more* processes will get a wakeup?
 (I'll try to add something to the text to clarify the detail we're 
 discussing.)

> Also, the 'non-exclusive' mode would be subject to the same question of
> which wait queue is the epfd is associated with...

 I'm not sure of the point you are trying to make here?

 Cheers,

 Michael


>> To make this point even clearer, here are two scenarios I'm thinking of.
>> In each case, we're talking of monitoring the read end of a FIFO.
>>
>> ===
>>
>> Scenario 1:
>>
>> We have three processes each of which
>> 1. Creates an epoll instance
>> 2. Opens the read end of the FIFO
>> 3. Adds the read end of the FIFO to the epoll instance, specifying
>>EPOLLEXCLUSIVE
>>
>> When input becomes available on the FIFO, how many processes
>> get a wakeup?
>>>
>>> When I test this scenario, all three processes get a wakeup.
>>>
>> ===
>>
>> Scenario 3
>>
>> A parent process opens the read end of a FIFO and then calls
>> fork() three times to create three children. Each child then:
>>
>> 1. Creates an epoll instance
>> 2. Adds the read end of the FIFO to the epoll instance, specifying
>> EPOLLEXCLUSIVE
>>
>> When input becomes available on the FIFO, how many processes
>> get a wakeup?
>>>
>>> When I test this scenario, one process gets a wakeup.
>>>
>>> In other words, "target file" appears to mean open file description
>>> (aka open file table entry), not inode.
>>>
>>> This is actually what I suspected might be the case, but now I am
>>> puzzled. Given what I've discovered and what you suggest are the
>>> semantics, is the implementation correct? (I suspect that it is,
>>> but it is at odds with your statement above. My test programs are
>>> inline below.
>>>
>>> Cheers,
>>>
>>> Michael
>>>
>>
>> Thanks for the test cases. So in your first test case, you are exiting
>> immediately after the epoll_wait() returns. So this is actually causing
>> the next wakeup. 
> 
> Can I just check my understanding of the rationale for the preceding 
> point. The next process is getting woken up, because the previous process
> did not "consume" the event (that is, the input is still available on the 
> FIFO). Right?
> 
>> And then the 2nd thread returns from epoll_wait() and
>> this causes the 3rd wakeup.
> 
> I added the sleep() calls, but still things don't seem to happen
> quite as you suggest. In the first scenario, after the first process
> terminates, *all* of the remaining processes wake from epoll_wait().
> What's happening in this case? (This smells like a possible bug.)

Yes, you are right. When the first process exits() and thus closes
the read-side of the pipe, it will wake up all the other calls that
are in epoll_wait(). When the file closes, since there are no more
fds referencing it, the pipe_release() routine does a wakeup specifying

Re: [PATCH v2 0/7] ARM: dts: Add new Exynos3250-based ARTIK5 module dtsi file

2016-03-14 Thread Krzysztof Kozlowski
On 15.03.2016 11:39, Chanwoo Choi wrote:
> On 2016년 03월 15일 11:35, Krzysztof Kozlowski wrote:
>> On 15.03.2016 11:08, Chanwoo Choi wrote:
>>> This patchset add the support for Device Tree source for Samsung ARTIK5 
>>> module[1]
>>> based on Exynos3250 SoC and development board[2]. The ARTIK5 module includes
>>> the follwoing devices:
>>> - Application Processor (Samsung Exynos3250)
>>> - WiFi/BT Combo chip
>>> - PMIC (Samsung S2MPS14)
>>> - eMMC (4GB)
>>> - DRAM LPDDR3 (512MB)
>>> - Connectors pin (60 Pins x 3 set)
>>>
>>> Also, this patchset add the ARTIK5 development board[2] dts file which 
>>> includes
>>> the ARTIK5 module[1] and have the devices such as sound codec, sd card port,
>>> ethernet port, uart port and so on.
>>>
>>> [1] https://www.artik.io/hardware/artik-5
>>> [2] 
>>> http://www.digikey.com/product-detail/en/SIP-KITNXB001/1510-1316-ND/5825102
>>
>> I guess the rest of patches depend on new clocks, right?
> 
> Yes. The new clocks(uart2, mmc2) are used for artik5 module.
> - uart2 is used for serial log for debug
> - mmc2 is for external sd card

Okay, so an ack from clock folks or a tag to pull would be nice.

Unfortunately merge window started so probably everything will wait...

BR,
Krzysztof



Re: [PATCH v2 0/7] ARM: dts: Add new Exynos3250-based ARTIK5 module dtsi file

2016-03-14 Thread Krzysztof Kozlowski
On 15.03.2016 11:39, Chanwoo Choi wrote:
> On 2016년 03월 15일 11:35, Krzysztof Kozlowski wrote:
>> On 15.03.2016 11:08, Chanwoo Choi wrote:
>>> This patchset add the support for Device Tree source for Samsung ARTIK5 
>>> module[1]
>>> based on Exynos3250 SoC and development board[2]. The ARTIK5 module includes
>>> the follwoing devices:
>>> - Application Processor (Samsung Exynos3250)
>>> - WiFi/BT Combo chip
>>> - PMIC (Samsung S2MPS14)
>>> - eMMC (4GB)
>>> - DRAM LPDDR3 (512MB)
>>> - Connectors pin (60 Pins x 3 set)
>>>
>>> Also, this patchset add the ARTIK5 development board[2] dts file which 
>>> includes
>>> the ARTIK5 module[1] and have the devices such as sound codec, sd card port,
>>> ethernet port, uart port and so on.
>>>
>>> [1] https://www.artik.io/hardware/artik-5
>>> [2] 
>>> http://www.digikey.com/product-detail/en/SIP-KITNXB001/1510-1316-ND/5825102
>>
>> I guess the rest of patches depend on new clocks, right?
> 
> Yes. The new clocks(uart2, mmc2) are used for artik5 module.
> - uart2 is used for serial log for debug
> - mmc2 is for external sd card

Okay, so an ack from clock folks or a tag to pull would be nice.

Unfortunately merge window started so probably everything will wait...

BR,
Krzysztof



Re: [GIT PULL] NOHZ updates for v4.6

2016-03-14 Thread Linus Torvalds
On Mon, Mar 14, 2016 at 5:32 AM, Ingo Molnar  wrote:
> +/**
> + * fetch_or - perform *ptr |= mask and return old value of *ptr
> + * @ptr: pointer to value
> + * @mask: mask to OR on the value
> + *
> + * cmpxchg based fetch_or, macro so it works for different integer types
> + */
> +#ifndef fetch_or
> +#define fetch_or(ptr, mask)\
> +({ typeof(*(ptr)) __old, __val = *(ptr);   \
> +   for (;;) {  \
> +   __old = cmpxchg((ptr), __val, __val | (mask));  \
> +   if (__old == __val) \
> +   break;  \
> +   __val = __old;  \
> +   }   \
> +   __old;  \
> +})
> +#endif

This is garbage.

This macro re-uses the "mask" argument potentially many many times, so
semantically it's very dubious.

That may have been acceptable in the old situation when this was
internal to sched.c, but now the code was moved to a generic header
file. And this kind of broken macro is *not* acceptable in that
context any more.

It is now in asm-generic/atomic.h, so it should now conform to the
rest of the code there. Try to model it around ATOMIC_OP_RETURN(),
although obviously this fetch_or() function returns the value *before*
the logical 'or' rather than the end result.

It would be lovely if it were piossible to just use an "atomic_t"
type, but it looks like it is used on thread_info->flags. Which
doesn't have a good type, sadly.

As a result, the code then makes a big deal about how this works with
any integer type, but then the new code that uses it uses a stupid
type that isn't appropriate. Why would it be using an "unsigned long",
when

 - it holds a fixed number of bits that don't depend on architecture
and certainly is not 64 (or even close to 32).

 - the structure it is in was just preceded by an "int", so on 64-bit
it generates pointless padding in addition to the pointless 64-bit
field.

The only reason to use a "unsigned long" is because "fetch_or()" would
be hardcoded to that type, which doesn't seem to be true.

Now, in practice, the code looks like it should *work* fine, so I'm
going to pull this, but I do want to lodge a protest on sloppiness and
general and unnecessary uglicity of this code.

So please get this fixed.

  Linus


Re: [GIT PULL] NOHZ updates for v4.6

2016-03-14 Thread Linus Torvalds
On Mon, Mar 14, 2016 at 5:32 AM, Ingo Molnar  wrote:
> +/**
> + * fetch_or - perform *ptr |= mask and return old value of *ptr
> + * @ptr: pointer to value
> + * @mask: mask to OR on the value
> + *
> + * cmpxchg based fetch_or, macro so it works for different integer types
> + */
> +#ifndef fetch_or
> +#define fetch_or(ptr, mask)\
> +({ typeof(*(ptr)) __old, __val = *(ptr);   \
> +   for (;;) {  \
> +   __old = cmpxchg((ptr), __val, __val | (mask));  \
> +   if (__old == __val) \
> +   break;  \
> +   __val = __old;  \
> +   }   \
> +   __old;  \
> +})
> +#endif

This is garbage.

This macro re-uses the "mask" argument potentially many many times, so
semantically it's very dubious.

That may have been acceptable in the old situation when this was
internal to sched.c, but now the code was moved to a generic header
file. And this kind of broken macro is *not* acceptable in that
context any more.

It is now in asm-generic/atomic.h, so it should now conform to the
rest of the code there. Try to model it around ATOMIC_OP_RETURN(),
although obviously this fetch_or() function returns the value *before*
the logical 'or' rather than the end result.

It would be lovely if it were piossible to just use an "atomic_t"
type, but it looks like it is used on thread_info->flags. Which
doesn't have a good type, sadly.

As a result, the code then makes a big deal about how this works with
any integer type, but then the new code that uses it uses a stupid
type that isn't appropriate. Why would it be using an "unsigned long",
when

 - it holds a fixed number of bits that don't depend on architecture
and certainly is not 64 (or even close to 32).

 - the structure it is in was just preceded by an "int", so on 64-bit
it generates pointless padding in addition to the pointless 64-bit
field.

The only reason to use a "unsigned long" is because "fetch_or()" would
be hardcoded to that type, which doesn't seem to be true.

Now, in practice, the code looks like it should *work* fine, so I'm
going to pull this, but I do want to lodge a protest on sloppiness and
general and unnecessary uglicity of this code.

So please get this fixed.

  Linus


Re: [PATCH v2 5/7] ARM: dts: Add exynos3250-artik5 dtsi file for ARTIK5 module

2016-03-14 Thread Chanwoo Choi
On 2016년 03월 15일 11:33, Krzysztof Kozlowski wrote:
> On 15.03.2016 11:08, Chanwoo Choi wrote:
>> This patch adds the support for Device Tree source for Samsung ARTIK5 
>> module[1]
>> based on Exynos3250 SoC. The ARTIK5 module includes the follwoing devices:
>> - Application Processor (Samsung Exynos3250)
>> - WiFi/BT Combo chip (Broadcom4354)
>> - PMIC (Samsung S2MPS14)
>> - eMMC (4GB)
>> - DRAM LPDDR3 (512MB)
>> - Connectors pin (60 Pins x 3 set)
>>
>> Also, this patch adds the ARTIK5 evaluation board[2] dts file which includes
>> the ARTIK5 module[1] and have the devices such as sound codec, sd card port,
>> ethernet port, uart port and so on.
>>
>> [1] https://www.artik.io/hardware/artik-5
>> [2] http://www.digikey.com/product-search/en?FV=ffecca14
>>
>> Signed-off-by: Chanwoo Choi 
>> Signed-off-by: Jaehoon Chung 
>> Signed-off-by: Andi Shyti 
>> ---
>>  .../bindings/arm/samsung/samsung-boards.txt|   2 +
>>  arch/arm/boot/dts/Makefile |   1 +
>>  arch/arm/boot/dts/exynos3250-artik5-eval.dts   |  26 ++
>>  arch/arm/boot/dts/exynos3250-artik5.dtsi   | 334 
>> +
>>  4 files changed, 363 insertions(+)
>>  create mode 100644 arch/arm/boot/dts/exynos3250-artik5-eval.dts
>>  create mode 100644 arch/arm/boot/dts/exynos3250-artik5.dtsi
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt 
>> b/Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt
>> index 12129c011c8f..f5deace2b380 100644
>> --- a/Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt
>> +++ b/Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt
>> @@ -2,6 +2,8 @@
>>  
>>  Required root node properties:
>>  - compatible = should be one or more of the following.
>> +- "samsung,artik5"  - for Exynos3250-based Samsung ARTIK5 module.
>> +- "samsung,artik5-eval" - for Exynos3250-based Samsung ARTIK5 eval 
>> board.
>>  - "samsung,monk"- for Exynos3250-based Samsung Simband board.
>>  - "samsung,rinato"  - for Exynos3250-based Samsung Gear2 board.
>>  - "samsung,smdkv310"- for Exynos4210-based Samsung SMDKV310 eval 
>> board.
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index a4a6d70e8b26..85cd586ea3d2 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -108,6 +108,7 @@ dtb-$(CONFIG_ARCH_DIGICOLOR) += \
>>  dtb-$(CONFIG_ARCH_EFM32) += \
>>  efm32gg-dk3750.dtb
>>  dtb-$(CONFIG_ARCH_EXYNOS3) += \
>> +exynos3250-artik5-eval.dtb \
>>  exynos3250-monk.dtb \
>>  exynos3250-rinato.dtb
>>  dtb-$(CONFIG_ARCH_EXYNOS4) += \
>> diff --git a/arch/arm/boot/dts/exynos3250-artik5-eval.dts 
>> b/arch/arm/boot/dts/exynos3250-artik5-eval.dts
>> new file mode 100644
>> index ..b476154590a5
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/exynos3250-artik5-eval.dts
>> @@ -0,0 +1,26 @@
>> +/*
>> + * Samsung's Exynos3250 based ARTIK5 evaluation board device tree source
>> + *
>> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
>> + *  http://www.samsung.com
>> + *
>> + * Device tree source file for Samsung's ARTIK5 evaluation board
>> + * which is based on Samsung Exynos3250 SoC.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +/dts-v1/;
>> +#include "exynos3250-artik5.dtsi"
>> +
>> +/ {
>> +model = "Samsung ARTIK5 evaluation board";
>> +compatible = "samsung,artik5-eval", "samsung,artik5",
>> +"samsung,exynos3250", "samsung,exynos3";
>> +};
>> +
>> +_2 {
>> +status = "okay";
>> +};
>> diff --git a/arch/arm/boot/dts/exynos3250-artik5.dtsi 
>> b/arch/arm/boot/dts/exynos3250-artik5.dtsi
>> new file mode 100644
>> index ..206625ba8cf2
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/exynos3250-artik5.dtsi
>> @@ -0,0 +1,334 @@
>> +/*
>> + * Samsung's Exynos3250 based ARTIK5 module device tree source
>> + *
>> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
>> + *  http://www.samsung.com
>> + *
>> + * Device tree source file for Samsung's ARTIK5 module which is based on
>> + * Samsung Exynos3250 SoC.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include "exynos3250.dtsi"
>> +#include 
>> +#include 
>> +#include 
> 
> Alphabetical order for dt-bindings includes: interrupt at the end.

OK. I'll keep it.

> 
>> +
>> +/ {
>> +compatible = "samsung,artik5", "samsung,exynos3250", "samsung,exynos3";
>> +
>> +chosen {
>> +stdout-path = _2;
>> +};
>> +
>> +memory {
>> +reg = <0x4000 0x1ff0>;
>> +};

Re: [PATCH v2 5/7] ARM: dts: Add exynos3250-artik5 dtsi file for ARTIK5 module

2016-03-14 Thread Chanwoo Choi
On 2016년 03월 15일 11:33, Krzysztof Kozlowski wrote:
> On 15.03.2016 11:08, Chanwoo Choi wrote:
>> This patch adds the support for Device Tree source for Samsung ARTIK5 
>> module[1]
>> based on Exynos3250 SoC. The ARTIK5 module includes the follwoing devices:
>> - Application Processor (Samsung Exynos3250)
>> - WiFi/BT Combo chip (Broadcom4354)
>> - PMIC (Samsung S2MPS14)
>> - eMMC (4GB)
>> - DRAM LPDDR3 (512MB)
>> - Connectors pin (60 Pins x 3 set)
>>
>> Also, this patch adds the ARTIK5 evaluation board[2] dts file which includes
>> the ARTIK5 module[1] and have the devices such as sound codec, sd card port,
>> ethernet port, uart port and so on.
>>
>> [1] https://www.artik.io/hardware/artik-5
>> [2] http://www.digikey.com/product-search/en?FV=ffecca14
>>
>> Signed-off-by: Chanwoo Choi 
>> Signed-off-by: Jaehoon Chung 
>> Signed-off-by: Andi Shyti 
>> ---
>>  .../bindings/arm/samsung/samsung-boards.txt|   2 +
>>  arch/arm/boot/dts/Makefile |   1 +
>>  arch/arm/boot/dts/exynos3250-artik5-eval.dts   |  26 ++
>>  arch/arm/boot/dts/exynos3250-artik5.dtsi   | 334 
>> +
>>  4 files changed, 363 insertions(+)
>>  create mode 100644 arch/arm/boot/dts/exynos3250-artik5-eval.dts
>>  create mode 100644 arch/arm/boot/dts/exynos3250-artik5.dtsi
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt 
>> b/Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt
>> index 12129c011c8f..f5deace2b380 100644
>> --- a/Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt
>> +++ b/Documentation/devicetree/bindings/arm/samsung/samsung-boards.txt
>> @@ -2,6 +2,8 @@
>>  
>>  Required root node properties:
>>  - compatible = should be one or more of the following.
>> +- "samsung,artik5"  - for Exynos3250-based Samsung ARTIK5 module.
>> +- "samsung,artik5-eval" - for Exynos3250-based Samsung ARTIK5 eval 
>> board.
>>  - "samsung,monk"- for Exynos3250-based Samsung Simband board.
>>  - "samsung,rinato"  - for Exynos3250-based Samsung Gear2 board.
>>  - "samsung,smdkv310"- for Exynos4210-based Samsung SMDKV310 eval 
>> board.
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index a4a6d70e8b26..85cd586ea3d2 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -108,6 +108,7 @@ dtb-$(CONFIG_ARCH_DIGICOLOR) += \
>>  dtb-$(CONFIG_ARCH_EFM32) += \
>>  efm32gg-dk3750.dtb
>>  dtb-$(CONFIG_ARCH_EXYNOS3) += \
>> +exynos3250-artik5-eval.dtb \
>>  exynos3250-monk.dtb \
>>  exynos3250-rinato.dtb
>>  dtb-$(CONFIG_ARCH_EXYNOS4) += \
>> diff --git a/arch/arm/boot/dts/exynos3250-artik5-eval.dts 
>> b/arch/arm/boot/dts/exynos3250-artik5-eval.dts
>> new file mode 100644
>> index ..b476154590a5
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/exynos3250-artik5-eval.dts
>> @@ -0,0 +1,26 @@
>> +/*
>> + * Samsung's Exynos3250 based ARTIK5 evaluation board device tree source
>> + *
>> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
>> + *  http://www.samsung.com
>> + *
>> + * Device tree source file for Samsung's ARTIK5 evaluation board
>> + * which is based on Samsung Exynos3250 SoC.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +/dts-v1/;
>> +#include "exynos3250-artik5.dtsi"
>> +
>> +/ {
>> +model = "Samsung ARTIK5 evaluation board";
>> +compatible = "samsung,artik5-eval", "samsung,artik5",
>> +"samsung,exynos3250", "samsung,exynos3";
>> +};
>> +
>> +_2 {
>> +status = "okay";
>> +};
>> diff --git a/arch/arm/boot/dts/exynos3250-artik5.dtsi 
>> b/arch/arm/boot/dts/exynos3250-artik5.dtsi
>> new file mode 100644
>> index ..206625ba8cf2
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/exynos3250-artik5.dtsi
>> @@ -0,0 +1,334 @@
>> +/*
>> + * Samsung's Exynos3250 based ARTIK5 module device tree source
>> + *
>> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
>> + *  http://www.samsung.com
>> + *
>> + * Device tree source file for Samsung's ARTIK5 module which is based on
>> + * Samsung Exynos3250 SoC.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include "exynos3250.dtsi"
>> +#include 
>> +#include 
>> +#include 
> 
> Alphabetical order for dt-bindings includes: interrupt at the end.

OK. I'll keep it.

> 
>> +
>> +/ {
>> +compatible = "samsung,artik5", "samsung,exynos3250", "samsung,exynos3";
>> +
>> +chosen {
>> +stdout-path = _2;
>> +};
>> +
>> +memory {
>> +reg = <0x4000 0x1ff0>;
>> +};
>> +
>> +firmware@0205F000 {
> 
> I meant same case for every hex, 

Re: [PATCH v2 0/7] ARM: dts: Add new Exynos3250-based ARTIK5 module dtsi file

2016-03-14 Thread Chanwoo Choi
On 2016년 03월 15일 11:35, Krzysztof Kozlowski wrote:
> On 15.03.2016 11:08, Chanwoo Choi wrote:
>> This patchset add the support for Device Tree source for Samsung ARTIK5 
>> module[1]
>> based on Exynos3250 SoC and development board[2]. The ARTIK5 module includes
>> the follwoing devices:
>> - Application Processor (Samsung Exynos3250)
>> - WiFi/BT Combo chip
>> - PMIC (Samsung S2MPS14)
>> - eMMC (4GB)
>> - DRAM LPDDR3 (512MB)
>> - Connectors pin (60 Pins x 3 set)
>>
>> Also, this patchset add the ARTIK5 development board[2] dts file which 
>> includes
>> the ARTIK5 module[1] and have the devices such as sound codec, sd card port,
>> ethernet port, uart port and so on.
>>
>> [1] https://www.artik.io/hardware/artik-5
>> [2] 
>> http://www.digikey.com/product-detail/en/SIP-KITNXB001/1510-1316-ND/5825102
> 
> I guess the rest of patches depend on new clocks, right?

Yes. The new clocks(uart2, mmc2) are used for artik5 module.
- uart2 is used for serial log for debug
- mmc2 is for external sd card

Best Regards,
Chanwoo Choi



Re: [PATCH v2 0/7] ARM: dts: Add new Exynos3250-based ARTIK5 module dtsi file

2016-03-14 Thread Chanwoo Choi
On 2016년 03월 15일 11:35, Krzysztof Kozlowski wrote:
> On 15.03.2016 11:08, Chanwoo Choi wrote:
>> This patchset add the support for Device Tree source for Samsung ARTIK5 
>> module[1]
>> based on Exynos3250 SoC and development board[2]. The ARTIK5 module includes
>> the follwoing devices:
>> - Application Processor (Samsung Exynos3250)
>> - WiFi/BT Combo chip
>> - PMIC (Samsung S2MPS14)
>> - eMMC (4GB)
>> - DRAM LPDDR3 (512MB)
>> - Connectors pin (60 Pins x 3 set)
>>
>> Also, this patchset add the ARTIK5 development board[2] dts file which 
>> includes
>> the ARTIK5 module[1] and have the devices such as sound codec, sd card port,
>> ethernet port, uart port and so on.
>>
>> [1] https://www.artik.io/hardware/artik-5
>> [2] 
>> http://www.digikey.com/product-detail/en/SIP-KITNXB001/1510-1316-ND/5825102
> 
> I guess the rest of patches depend on new clocks, right?

Yes. The new clocks(uart2, mmc2) are used for artik5 module.
- uart2 is used for serial log for debug
- mmc2 is for external sd card

Best Regards,
Chanwoo Choi



Re: [PATCH v2 7/7] ARM: dts: Add initial gpio setting of MMC2 device for exynos3250-monk

2016-03-14 Thread Krzysztof Kozlowski
On 15.03.2016 11:08, Chanwoo Choi wrote:
> This patch adds initial pin configuration of MMC2 device on exynos3250-monk
> board because the MMC2 gpio pin (gpk2[0-6]) are NC (not connected) state.
> 
> Suggested-by: Krzysztof Kozlowski 
> Signed-off-by: Chanwoo Choi 
> ---
>  arch/arm/boot/dts/exynos3250-monk.dts | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>

The patch should go in before setting these pins to MMC2 (so before
patch 4/7) so there won't be any change in behaviour.

Reviewed-by: Krzysztof Kozlowski 



Best regards,

Krzysztof



Re: [PATCH v2 7/7] ARM: dts: Add initial gpio setting of MMC2 device for exynos3250-monk

2016-03-14 Thread Krzysztof Kozlowski
On 15.03.2016 11:08, Chanwoo Choi wrote:
> This patch adds initial pin configuration of MMC2 device on exynos3250-monk
> board because the MMC2 gpio pin (gpk2[0-6]) are NC (not connected) state.
> 
> Suggested-by: Krzysztof Kozlowski 
> Signed-off-by: Chanwoo Choi 
> ---
>  arch/arm/boot/dts/exynos3250-monk.dts | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>

The patch should go in before setting these pins to MMC2 (so before
patch 4/7) so there won't be any change in behaviour.

Reviewed-by: Krzysztof Kozlowski 



Best regards,

Krzysztof



Re: [PATCH 1/4] acpi,pci,irq: reduce resource requirements

2016-03-14 Thread Bjorn Helgaas
On Mon, Mar 14, 2016 at 10:28:11PM -0400, Sinan Kaya wrote:
> On 3/14/2016 9:48 PM, Bjorn Helgaas wrote:
> 
> Yes. I was talking about PCIe on ARM64. 
> 
> > If you go to Fry's and buy a conventional PCI card, it is going to
> > pull INTA# low to assert an interrupt.  It doesn't matter whether you
> > put it in an x86 system or an arm64 system.
> 
> I don't see INTA# of the PCIe card at the system level. The PCIe wire 
> interrupt gets converted to the system level interrupt by the PCIe controller.

That's why I said *conventional PCI*.  If you have a conventional PCI
device below either a conventional PCI host controller or a PCIe-to-PCI
bridge, there are real INTx wires, not virtual wires, and they are
level/low.  But I think you pointed out the key below (that the
Interrupt resource in a PNP0C0F device encodes the trigger type).

> >> > I pasted the code here again. It looks like you want to validate that
> >> > PCI interrupts are always level low. 
> > I don't really care whether PCI interrupts are always level low.  What
> > matters is that the PCI interrupt line matches the configuration of
> > the interrupt controller input.
> > 
> 
> Agreed. But the interrupt controller configuration is system specific. How 
> would
> you check interrupt line against what the interrupt controller requires
> on each architecture as this is common code?
> 
> 
> > If the PCI interrupt can be a different type, e.g., level high, and
> > there's a way to discover that, we can check that against the
> > interrupt controller configuration.
> > 
> > This is all in the context of conventional PCI, and we're probably
> > talking about arm64 PCIe systems, not conventional PCI. 
> 
> INTx interrupts are TLP messages on PCIe as you already know. There is no 
> INTA 
> interrupt wire.

Yes, that's why I mentioned PCIe sec 2.2.8.1 below.

> "6.1.2. PCI Compatible INTx Emulation" section of the PCIe spec describes
> INTx emulation on PCIe.
>  ...
> 
> > I'm not sure what an Interrupt Link device means in PCIe.  I suppose it 
> > would have
> > to connect an INTx virtual wire to a system interrupt?  The PCIe spec
> > says this sort of mapping is system implementation specific (r3.0, sec
> > 2.2.8.1).
> 
> The INTx messages are converted to the system interrupt by the PCIe 
> controller.
> The interrupt type between the PCIe controller and the ARM GIC interrupt 
> controller is dictated by the ARM GIC Interrupt Controller Specification for
> ARM64.
> 
> Here is what ACPI table looks like for reference
> 
>   Name(_PRT, Package(){
>   Package(){0x0, 0, \_SB.LN0A, 0},  // Slot 0, INTA
>   Package(){0x0, 1, \_SB.LN0B, 0},  // Slot 0, INTB
>   Package(){0x0, 2, \_SB.LN0C, 0},  // Slot 0, INTC
>   Package(){0x0, 3, \_SB.LN0D, 0}   // Slot 0, INTD
>   }) 
> 
> Device(LN0A){
>   Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link
>   Name(_UID, 1)
>   Name(_PRS, ResourceTemplate(){
>   Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive, , ,) {0x123}
>   })

I forgot that the link already include the trigger mode in it.  Maybe we
can check for that instead of assuming level/low.

Bjorn


Re: [PATCH 1/4] acpi,pci,irq: reduce resource requirements

2016-03-14 Thread Bjorn Helgaas
On Mon, Mar 14, 2016 at 10:28:11PM -0400, Sinan Kaya wrote:
> On 3/14/2016 9:48 PM, Bjorn Helgaas wrote:
> 
> Yes. I was talking about PCIe on ARM64. 
> 
> > If you go to Fry's and buy a conventional PCI card, it is going to
> > pull INTA# low to assert an interrupt.  It doesn't matter whether you
> > put it in an x86 system or an arm64 system.
> 
> I don't see INTA# of the PCIe card at the system level. The PCIe wire 
> interrupt gets converted to the system level interrupt by the PCIe controller.

That's why I said *conventional PCI*.  If you have a conventional PCI
device below either a conventional PCI host controller or a PCIe-to-PCI
bridge, there are real INTx wires, not virtual wires, and they are
level/low.  But I think you pointed out the key below (that the
Interrupt resource in a PNP0C0F device encodes the trigger type).

> >> > I pasted the code here again. It looks like you want to validate that
> >> > PCI interrupts are always level low. 
> > I don't really care whether PCI interrupts are always level low.  What
> > matters is that the PCI interrupt line matches the configuration of
> > the interrupt controller input.
> > 
> 
> Agreed. But the interrupt controller configuration is system specific. How 
> would
> you check interrupt line against what the interrupt controller requires
> on each architecture as this is common code?
> 
> 
> > If the PCI interrupt can be a different type, e.g., level high, and
> > there's a way to discover that, we can check that against the
> > interrupt controller configuration.
> > 
> > This is all in the context of conventional PCI, and we're probably
> > talking about arm64 PCIe systems, not conventional PCI. 
> 
> INTx interrupts are TLP messages on PCIe as you already know. There is no 
> INTA 
> interrupt wire.

Yes, that's why I mentioned PCIe sec 2.2.8.1 below.

> "6.1.2. PCI Compatible INTx Emulation" section of the PCIe spec describes
> INTx emulation on PCIe.
>  ...
> 
> > I'm not sure what an Interrupt Link device means in PCIe.  I suppose it 
> > would have
> > to connect an INTx virtual wire to a system interrupt?  The PCIe spec
> > says this sort of mapping is system implementation specific (r3.0, sec
> > 2.2.8.1).
> 
> The INTx messages are converted to the system interrupt by the PCIe 
> controller.
> The interrupt type between the PCIe controller and the ARM GIC interrupt 
> controller is dictated by the ARM GIC Interrupt Controller Specification for
> ARM64.
> 
> Here is what ACPI table looks like for reference
> 
>   Name(_PRT, Package(){
>   Package(){0x0, 0, \_SB.LN0A, 0},  // Slot 0, INTA
>   Package(){0x0, 1, \_SB.LN0B, 0},  // Slot 0, INTB
>   Package(){0x0, 2, \_SB.LN0C, 0},  // Slot 0, INTC
>   Package(){0x0, 3, \_SB.LN0D, 0}   // Slot 0, INTD
>   }) 
> 
> Device(LN0A){
>   Name(_HID, EISAID("PNP0C0F")) // PCI interrupt link
>   Name(_UID, 1)
>   Name(_PRS, ResourceTemplate(){
>   Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive, , ,) {0x123}
>   })

I forgot that the link already include the trigger mode in it.  Maybe we
can check for that instead of assuming level/low.

Bjorn


Re: [PATCH 04/06] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context

2016-03-14 Thread Magnus Damm
Hi Laurent,

On Tue, Dec 29, 2015 at 9:14 AM, Laurent Pinchart
 wrote:
> Hi Magnus,
>
> Thank you for the patch.

Thanks for your feedback!

> On Tuesday 15 December 2015 21:02:49 Magnus Damm wrote:
>> From: Magnus Damm 
>>
>> Introduce a bitmap for context handing and convert the
>> interrupt routine to go handle all registered contexts.
>>
>> At this point the number of contexts are still limited.
>
> That's all nice, but without seeing support for multiple contexts it's hard to
> tell if the implementation is correct for multiple context purpose.
>
>> The purpose of this patch is to remove the use of the
>> ARM specific mapping variable from ipmmu_irq().
>
> Why do you want to do that ?

The purpose of this series is to be able to use the IPMMU driver on
other architectures than 32-bit ARM. The main goal is to use the
driver on 64-bit ARM where the mapping variable does not exist.

>> Signed-off-by: Magnus Damm 
>> ---
>>
>>  drivers/iommu/ipmmu-vmsa.c |   37 ++---
>>  1 file changed, 26 insertions(+), 11 deletions(-)
>>
>> --- 0007/drivers/iommu/ipmmu-vmsa.c
>> +++ work/drivers/iommu/ipmmu-vmsa.c   2015-12-15 13:14:35.540513000 +0900
>> @@ -8,6 +8,7 @@
>>   * the Free Software Foundation; version 2 of the License.
>>   */
>>
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -26,12 +27,16 @@
>>
>>  #include "io-pgtable.h"
>>
>> +#define IPMMU_CTX_MAX 1
>> +
>>  struct ipmmu_vmsa_device {
>>   struct device *dev;
>>   void __iomem *base;
>>   struct list_head list;
>>
>>   unsigned int num_utlbs;
>> + DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
>
> We have up to 4 context on Gen2 and 8 on Gen3, a bitmap might be slightly
> overkill.

Maybe so, but I'd rather use something standard than rolling my own.
Can you think of a better data structure?

>> + struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
>>
>>   struct dma_iommu_mapping *mapping;
>>  };
>> @@ -319,6 +324,7 @@ static struct iommu_gather_ops ipmmu_gat
>>  static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
>>  {
>>   phys_addr_t ttbr;
>> + int ret;
>>
>>   /*
>>* Allocate the page table operations.
>> @@ -348,10 +354,16 @@ static int ipmmu_domain_init_context(str
>>   return -EINVAL;
>>
>>   /*
>> -  * TODO: When adding support for multiple contexts, find an unused
>> -  * context.
>> +  * Find an unused context.
>
> We need to support multiple devices per context or we will very soon run out
> of contexts. How to pick a proper context is a topic that needs to be
> researched, I believe IOMMU groups might come into play.

The experimental 64-bit ARM patches for this driver (on top of this
series) that I've posted makes use of IOMMU groups.

>>*/
>> - domain->context_id = 0;
>> + ret = bitmap_find_free_region(domain->mmu->ctx, IPMMU_CTX_MAX, 0);
>> + if (ret < 0) {
>> + free_io_pgtable_ops(domain->iop);
>> + return ret;
>> + }
>> +
>> + domain->context_id = ret;
>> + domain->mmu->domains[ret] = domain;
>
> This requires locking to protect against races with the interrupt handler.

Hm, it seems that I mistakenly assumed that bitmap_find_free_region()
was built on top of atomic set_bit() and managed the bitmap in an
atomic way. I believe you are correct that locking is needed. Will
fix.

>>
>>   /* TTBR0 */
>>   ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
>> @@ -395,6 +407,8 @@ static int ipmmu_domain_init_context(str
>>
>>  static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
>>  {
>> + bitmap_release_region(domain->mmu->ctx, domain->context_id, 0);
>> +
>>   /*
>>* Disable the context. Flush the TLB as required when modifying the
>>* context registers.
>> @@ -460,16 +474,16 @@ static irqreturn_t ipmmu_domain_irq(stru
>>  static irqreturn_t ipmmu_irq(int irq, void *dev)
>>  {
>>   struct ipmmu_vmsa_device *mmu = dev;
>> - struct iommu_domain *io_domain;
>> - struct ipmmu_vmsa_domain *domain;
>> -
>> - if (!mmu->mapping)
>> - return IRQ_NONE;
>> + irqreturn_t status = IRQ_NONE;
>> + unsigned int k;
>
> i is a perfectly fine loop counter :-)
>
>> - io_domain = mmu->mapping->domain;
>> - domain = to_vmsa_domain(io_domain);
>> + /* Check interrupts for all active contexts */
>> + for (k = find_first_bit(mmu->ctx, IPMMU_CTX_MAX);
>> +  k < IPMMU_CTX_MAX && status == IRQ_NONE;
>> +  k = find_next_bit(mmu->ctx, IPMMU_CTX_MAX, k))
>
> You can just loop over mmu->domains and skip NULL entries.

You are right, that may be easier!

>> + status = ipmmu_domain_irq(mmu->domains[k]);
>
> Only the status of the last domain is taken into account.

Will fix, thanks!

/ magnus


Re: [PATCH 3.10 00/18] 3.10.101-stable review

2016-03-14 Thread Guenter Roeck

On 03/14/2016 10:52 AM, Greg Kroah-Hartman wrote:

This is the start of the stable review cycle for the 3.10.101 release.
There are 18 patches in this series, all will be posted as a response
to this one.  If anyone has any issues with these being applied, please
let me know.

Responses should be made by Wed Mar 16 17:50:35 UTC 2016.
Anything received after that time might be too late.



Build results:
total: 122 pass: 120 fail: 2
Failed builds:
mn10300:asb2303_defconfig
mn10300:asb2364_defconfig
Qemu test results:
total: 72 pass: 72 fail: 0

Details are available at http://kerneltests.org/builders.

Guenter



Re: [PATCH v2 6/7] ARM: dts: Add MSHC2 dt node for SD card for exynos3250-artik5-eval board

2016-03-14 Thread Krzysztof Kozlowski
On 15.03.2016 11:08, Chanwoo Choi wrote:
> From: Jaehoon Chung 
> 
> This patch adds MSHC (Mobile Storage Host Controller) dt node for
> Exynos3250 SoC. MSHC is an interface between the system and the SD card
> 
> Signed-off-by: Jaehoon Chung 
> Signed-off-by: Chanwoo Choi 
> ---
>  arch/arm/boot/dts/exynos3250-artik5-eval.dts | 17 +
>  1 file changed, 17 insertions(+)
> 

Reviewed-by: Krzysztof Kozlowski 



Best regards,

Krzysztof



Re: [PATCH 04/06] iommu/ipmmu-vmsa: Rework interrupt code and use bitmap for context

2016-03-14 Thread Magnus Damm
Hi Laurent,

On Tue, Dec 29, 2015 at 9:14 AM, Laurent Pinchart
 wrote:
> Hi Magnus,
>
> Thank you for the patch.

Thanks for your feedback!

> On Tuesday 15 December 2015 21:02:49 Magnus Damm wrote:
>> From: Magnus Damm 
>>
>> Introduce a bitmap for context handing and convert the
>> interrupt routine to go handle all registered contexts.
>>
>> At this point the number of contexts are still limited.
>
> That's all nice, but without seeing support for multiple contexts it's hard to
> tell if the implementation is correct for multiple context purpose.
>
>> The purpose of this patch is to remove the use of the
>> ARM specific mapping variable from ipmmu_irq().
>
> Why do you want to do that ?

The purpose of this series is to be able to use the IPMMU driver on
other architectures than 32-bit ARM. The main goal is to use the
driver on 64-bit ARM where the mapping variable does not exist.

>> Signed-off-by: Magnus Damm 
>> ---
>>
>>  drivers/iommu/ipmmu-vmsa.c |   37 ++---
>>  1 file changed, 26 insertions(+), 11 deletions(-)
>>
>> --- 0007/drivers/iommu/ipmmu-vmsa.c
>> +++ work/drivers/iommu/ipmmu-vmsa.c   2015-12-15 13:14:35.540513000 +0900
>> @@ -8,6 +8,7 @@
>>   * the Free Software Foundation; version 2 of the License.
>>   */
>>
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -26,12 +27,16 @@
>>
>>  #include "io-pgtable.h"
>>
>> +#define IPMMU_CTX_MAX 1
>> +
>>  struct ipmmu_vmsa_device {
>>   struct device *dev;
>>   void __iomem *base;
>>   struct list_head list;
>>
>>   unsigned int num_utlbs;
>> + DECLARE_BITMAP(ctx, IPMMU_CTX_MAX);
>
> We have up to 4 context on Gen2 and 8 on Gen3, a bitmap might be slightly
> overkill.

Maybe so, but I'd rather use something standard than rolling my own.
Can you think of a better data structure?

>> + struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
>>
>>   struct dma_iommu_mapping *mapping;
>>  };
>> @@ -319,6 +324,7 @@ static struct iommu_gather_ops ipmmu_gat
>>  static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
>>  {
>>   phys_addr_t ttbr;
>> + int ret;
>>
>>   /*
>>* Allocate the page table operations.
>> @@ -348,10 +354,16 @@ static int ipmmu_domain_init_context(str
>>   return -EINVAL;
>>
>>   /*
>> -  * TODO: When adding support for multiple contexts, find an unused
>> -  * context.
>> +  * Find an unused context.
>
> We need to support multiple devices per context or we will very soon run out
> of contexts. How to pick a proper context is a topic that needs to be
> researched, I believe IOMMU groups might come into play.

The experimental 64-bit ARM patches for this driver (on top of this
series) that I've posted makes use of IOMMU groups.

>>*/
>> - domain->context_id = 0;
>> + ret = bitmap_find_free_region(domain->mmu->ctx, IPMMU_CTX_MAX, 0);
>> + if (ret < 0) {
>> + free_io_pgtable_ops(domain->iop);
>> + return ret;
>> + }
>> +
>> + domain->context_id = ret;
>> + domain->mmu->domains[ret] = domain;
>
> This requires locking to protect against races with the interrupt handler.

Hm, it seems that I mistakenly assumed that bitmap_find_free_region()
was built on top of atomic set_bit() and managed the bitmap in an
atomic way. I believe you are correct that locking is needed. Will
fix.

>>
>>   /* TTBR0 */
>>   ttbr = domain->cfg.arm_lpae_s1_cfg.ttbr[0];
>> @@ -395,6 +407,8 @@ static int ipmmu_domain_init_context(str
>>
>>  static void ipmmu_domain_destroy_context(struct ipmmu_vmsa_domain *domain)
>>  {
>> + bitmap_release_region(domain->mmu->ctx, domain->context_id, 0);
>> +
>>   /*
>>* Disable the context. Flush the TLB as required when modifying the
>>* context registers.
>> @@ -460,16 +474,16 @@ static irqreturn_t ipmmu_domain_irq(stru
>>  static irqreturn_t ipmmu_irq(int irq, void *dev)
>>  {
>>   struct ipmmu_vmsa_device *mmu = dev;
>> - struct iommu_domain *io_domain;
>> - struct ipmmu_vmsa_domain *domain;
>> -
>> - if (!mmu->mapping)
>> - return IRQ_NONE;
>> + irqreturn_t status = IRQ_NONE;
>> + unsigned int k;
>
> i is a perfectly fine loop counter :-)
>
>> - io_domain = mmu->mapping->domain;
>> - domain = to_vmsa_domain(io_domain);
>> + /* Check interrupts for all active contexts */
>> + for (k = find_first_bit(mmu->ctx, IPMMU_CTX_MAX);
>> +  k < IPMMU_CTX_MAX && status == IRQ_NONE;
>> +  k = find_next_bit(mmu->ctx, IPMMU_CTX_MAX, k))
>
> You can just loop over mmu->domains and skip NULL entries.

You are right, that may be easier!

>> + status = ipmmu_domain_irq(mmu->domains[k]);
>
> Only the status of the last domain is taken into account.

Will fix, thanks!

/ magnus


Re: [PATCH 3.10 00/18] 3.10.101-stable review

2016-03-14 Thread Guenter Roeck

On 03/14/2016 10:52 AM, Greg Kroah-Hartman wrote:

This is the start of the stable review cycle for the 3.10.101 release.
There are 18 patches in this series, all will be posted as a response
to this one.  If anyone has any issues with these being applied, please
let me know.

Responses should be made by Wed Mar 16 17:50:35 UTC 2016.
Anything received after that time might be too late.



Build results:
total: 122 pass: 120 fail: 2
Failed builds:
mn10300:asb2303_defconfig
mn10300:asb2364_defconfig
Qemu test results:
total: 72 pass: 72 fail: 0

Details are available at http://kerneltests.org/builders.

Guenter



Re: [PATCH v2 6/7] ARM: dts: Add MSHC2 dt node for SD card for exynos3250-artik5-eval board

2016-03-14 Thread Krzysztof Kozlowski
On 15.03.2016 11:08, Chanwoo Choi wrote:
> From: Jaehoon Chung 
> 
> This patch adds MSHC (Mobile Storage Host Controller) dt node for
> Exynos3250 SoC. MSHC is an interface between the system and the SD card
> 
> Signed-off-by: Jaehoon Chung 
> Signed-off-by: Chanwoo Choi 
> ---
>  arch/arm/boot/dts/exynos3250-artik5-eval.dts | 17 +
>  1 file changed, 17 insertions(+)
> 

Reviewed-by: Krzysztof Kozlowski 



Best regards,

Krzysztof



  1   2   3   4   5   6   7   8   9   10   >