Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[Scott Wood]

On Fri, 2016-08-05 at 21:20 +, york sun wrote:
> On 08/05/2016 02:09 PM, Scott Wood wrote:
> > 
> > On Fri, 2016-08-05 at 20:29 +, york sun wrote:
> > > 
> > > On 08/04/2016 08:43 PM, Michael Ellerman wrote:
> > > > 
> > > > 
> > > > Does the driver really need to use these routines? They're meant for
> > > > use
> > > > early in boot, before PCI is setup.
> > > > 
> > > > AFAICS this is just a regular driver, so when it's probed the PCI
> > > > devices should have already been scanned. In which case
> > > > pci_get_device()
> > > > could work couldn't it? (I see other edac drivers doing that).
> > > I am trying to fix this but need some help. We are dealing with PCIe
> > > controller here. Does it have a bus number assigned at this point? If
> > > yes, how can I find it? I seem not able to find out where the
> > > platform_data is filled as well. Can someone kindly point it out to me?
> > 
> > The platform data comes from add_err_dev() in
> > arch/powerpc/sysdev/fsl_pci.c.
> > 
> Thanks, Scott.
> 
> When add_err_dev() is called, pci is not scanned, is using 
> early_find_capability() justified?

The edac driver is registered with a normal device-level initcall.  The PCI
scanning appears to happen at the subsys initcall level.

-Scott

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[Scott Wood]

On Fri, 2016-08-05 at 21:20 +, york sun wrote:
> On 08/05/2016 02:09 PM, Scott Wood wrote:
> > 
> > On Fri, 2016-08-05 at 20:29 +, york sun wrote:
> > > 
> > > On 08/04/2016 08:43 PM, Michael Ellerman wrote:
> > > > 
> > > > 
> > > > Does the driver really need to use these routines? They're meant for
> > > > use
> > > > early in boot, before PCI is setup.
> > > > 
> > > > AFAICS this is just a regular driver, so when it's probed the PCI
> > > > devices should have already been scanned. In which case
> > > > pci_get_device()
> > > > could work couldn't it? (I see other edac drivers doing that).
> > > I am trying to fix this but need some help. We are dealing with PCIe
> > > controller here. Does it have a bus number assigned at this point? If
> > > yes, how can I find it? I seem not able to find out where the
> > > platform_data is filled as well. Can someone kindly point it out to me?
> > 
> > The platform data comes from add_err_dev() in
> > arch/powerpc/sysdev/fsl_pci.c.
> > 
> Thanks, Scott.
> 
> When add_err_dev() is called, pci is not scanned, is using 
> early_find_capability() justified?

The edac driver is registered with a normal device-level initcall.  The PCI
scanning appears to happen at the subsys initcall level.

-Scott

RE: [PATCH] vmxnet3: Move PCI Id to pci_ids.h

[Adit Ranadive]

On Fri, Aug 05, 2016 11:23:41AM -0700, Bjorn Helgaas  wrote:
> On Fri, Aug 05, 2016 at 11:00:39AM -0700, Adit Ranadive wrote:
> > The VMXNet3 PCI Id will be shared with our upcoming paravirtual RDMA
> > driver. Moved it to the shared location in pci_ids.h and updated the
> > driver version.
> >
> > Suggested-by: Leon Romanovsky 
> > Signed-off-by: Adit Ranadive 
> 
> Please include this in the series that adds your paravirtual RDMA
> driver.  Please do not change the driver version information in this
> patch, because that change is unrelated to moving the PCI ID.

Ok, I will. Please drop this patch then.

Thanks,
Adit

RE: [PATCH] vmxnet3: Move PCI Id to pci_ids.h

[Adit Ranadive]

On Fri, Aug 05, 2016 11:23:41AM -0700, Bjorn Helgaas  wrote:
> On Fri, Aug 05, 2016 at 11:00:39AM -0700, Adit Ranadive wrote:
> > The VMXNet3 PCI Id will be shared with our upcoming paravirtual RDMA
> > driver. Moved it to the shared location in pci_ids.h and updated the
> > driver version.
> >
> > Suggested-by: Leon Romanovsky 
> > Signed-off-by: Adit Ranadive 
> 
> Please include this in the series that adds your paravirtual RDMA
> driver.  Please do not change the driver version information in this
> patch, because that change is unrelated to moving the PCI ID.

Ok, I will. Please drop this patch then.

Thanks,
Adit

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[york sun]

On 08/05/2016 02:09 PM, Scott Wood wrote:
> On Fri, 2016-08-05 at 20:29 +, york sun wrote:
>> On 08/04/2016 08:43 PM, Michael Ellerman wrote:
>>>
>>> Does the driver really need to use these routines? They're meant for use
>>> early in boot, before PCI is setup.
>>>
>>> AFAICS this is just a regular driver, so when it's probed the PCI
>>> devices should have already been scanned. In which case pci_get_device()
>>> could work couldn't it? (I see other edac drivers doing that).
>> I am trying to fix this but need some help. We are dealing with PCIe
>> controller here. Does it have a bus number assigned at this point? If
>> yes, how can I find it? I seem not able to find out where the
>> platform_data is filled as well. Can someone kindly point it out to me?
>
>
> The platform data comes from add_err_dev() in arch/powerpc/sysdev/fsl_pci.c.
>

Thanks, Scott.

When add_err_dev() is called, pci is not scanned, is using 
early_find_capability() justified?

York

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[york sun]

On 08/05/2016 02:09 PM, Scott Wood wrote:
> On Fri, 2016-08-05 at 20:29 +, york sun wrote:
>> On 08/04/2016 08:43 PM, Michael Ellerman wrote:
>>>
>>> Does the driver really need to use these routines? They're meant for use
>>> early in boot, before PCI is setup.
>>>
>>> AFAICS this is just a regular driver, so when it's probed the PCI
>>> devices should have already been scanned. In which case pci_get_device()
>>> could work couldn't it? (I see other edac drivers doing that).
>> I am trying to fix this but need some help. We are dealing with PCIe
>> controller here. Does it have a bus number assigned at this point? If
>> yes, how can I find it? I seem not able to find out where the
>> platform_data is filled as well. Can someone kindly point it out to me?
>
>
> The platform data comes from add_err_dev() in arch/powerpc/sysdev/fsl_pci.c.
>

Thanks, Scott.

When add_err_dev() is called, pci is not scanned, is using 
early_find_capability() justified?

York

[PATCH 3/3] lkdtm: Mark lkdtm_rodata_do_nothing() notrace

[Kees Cook]

From: Michael Ellerman 

lkdtm_rodata_do_nothing() is an empty function which is generated in
order to test the non-executability of rodata.

Currently if function tracing is enabled then an mcount callsite will be
generated for lkdtm_rodata_do_nothing(), and it will appear in the list
of available functions for function tracing (available_filter_functions).

Given it's purpose purely as a test function, it seems preferable for
lkdtm_rodata_do_nothing() to be marked notrace, so it doesn't appear as
traceable.

This also avoids triggering a linker bug on powerpc:

  https://sourceware.org/bugzilla/show_bug.cgi?id=20428

When the linker sees code that needs to generate a call stub, eg. a
branch to mcount(), it assumes the section is executable and
dereferences a NULL pointer leading to a linker segfault. Marking
lkdtm_rodata_do_nothing() notrace avoids triggering the bug because the
function contains no other function calls.

Signed-off-by: Michael Ellerman 
Signed-off-by: Kees Cook 
---
 drivers/misc/lkdtm_rodata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/lkdtm_rodata.c b/drivers/misc/lkdtm_rodata.c
index 166b1db3969f..3564477b8c2d 100644
--- a/drivers/misc/lkdtm_rodata.c
+++ b/drivers/misc/lkdtm_rodata.c
@@ -4,7 +4,7 @@
  */
 #include "lkdtm.h"
 
-void lkdtm_rodata_do_nothing(void)
+void notrace lkdtm_rodata_do_nothing(void)
 {
/* Does nothing. We just want an architecture agnostic "return". */
 }
-- 
2.7.4

[PATCH 3/3] lkdtm: Mark lkdtm_rodata_do_nothing() notrace

[Kees Cook]

From: Michael Ellerman 

lkdtm_rodata_do_nothing() is an empty function which is generated in
order to test the non-executability of rodata.

Currently if function tracing is enabled then an mcount callsite will be
generated for lkdtm_rodata_do_nothing(), and it will appear in the list
of available functions for function tracing (available_filter_functions).

Given it's purpose purely as a test function, it seems preferable for
lkdtm_rodata_do_nothing() to be marked notrace, so it doesn't appear as
traceable.

This also avoids triggering a linker bug on powerpc:

  https://sourceware.org/bugzilla/show_bug.cgi?id=20428

When the linker sees code that needs to generate a call stub, eg. a
branch to mcount(), it assumes the section is executable and
dereferences a NULL pointer leading to a linker segfault. Marking
lkdtm_rodata_do_nothing() notrace avoids triggering the bug because the
function contains no other function calls.

Signed-off-by: Michael Ellerman 
Signed-off-by: Kees Cook 
---
 drivers/misc/lkdtm_rodata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/lkdtm_rodata.c b/drivers/misc/lkdtm_rodata.c
index 166b1db3969f..3564477b8c2d 100644
--- a/drivers/misc/lkdtm_rodata.c
+++ b/drivers/misc/lkdtm_rodata.c
@@ -4,7 +4,7 @@
  */
 #include "lkdtm.h"
 
-void lkdtm_rodata_do_nothing(void)
+void notrace lkdtm_rodata_do_nothing(void)
 {
/* Does nothing. We just want an architecture agnostic "return". */
 }
-- 
2.7.4

[PATCH 1/3] lkdtm: fix false positive warning from -Wmaybe-uninitialized

[Kees Cook]

The variable in use here doesn't matter (it's just used to exercise taking
up stack space), but this changes its use to pass its address instead,
to avoid a compiler warning:

drivers/misc/lkdtm_usercopy.c:54:15: warning: 'bad_stack' may be used 
uninitialized in this function [-Wmaybe-uninitialized]

Reported-by: Arnd Bergmann 
Signed-off-by: Kees Cook 
---
 drivers/misc/lkdtm_usercopy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/lkdtm_usercopy.c b/drivers/misc/lkdtm_usercopy.c
index 5a3fd76eec27..5525a204db93 100644
--- a/drivers/misc/lkdtm_usercopy.c
+++ b/drivers/misc/lkdtm_usercopy.c
@@ -49,7 +49,7 @@ static noinline void do_usercopy_stack(bool to_user, bool 
bad_frame)
 
/* This is a pointer to outside our current stack frame. */
if (bad_frame) {
-   bad_stack = do_usercopy_stack_callee((uintptr_t)bad_stack);
+   bad_stack = do_usercopy_stack_callee((uintptr_t)_stack);
} else {
/* Put start address just inside stack. */
bad_stack = task_stack_page(current) + THREAD_SIZE;
-- 
2.7.4

[PATCH 1/3] lkdtm: fix false positive warning from -Wmaybe-uninitialized

[Kees Cook]

The variable in use here doesn't matter (it's just used to exercise taking
up stack space), but this changes its use to pass its address instead,
to avoid a compiler warning:

drivers/misc/lkdtm_usercopy.c:54:15: warning: 'bad_stack' may be used 
uninitialized in this function [-Wmaybe-uninitialized]

Reported-by: Arnd Bergmann 
Signed-off-by: Kees Cook 
---
 drivers/misc/lkdtm_usercopy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/lkdtm_usercopy.c b/drivers/misc/lkdtm_usercopy.c
index 5a3fd76eec27..5525a204db93 100644
--- a/drivers/misc/lkdtm_usercopy.c
+++ b/drivers/misc/lkdtm_usercopy.c
@@ -49,7 +49,7 @@ static noinline void do_usercopy_stack(bool to_user, bool 
bad_frame)
 
/* This is a pointer to outside our current stack frame. */
if (bad_frame) {
-   bad_stack = do_usercopy_stack_callee((uintptr_t)bad_stack);
+   bad_stack = do_usercopy_stack_callee((uintptr_t)_stack);
} else {
/* Put start address just inside stack. */
bad_stack = task_stack_page(current) + THREAD_SIZE;
-- 
2.7.4

[PATCH 2/3] lkdtm: Fix targets for objcopy usage

[Kees Cook]

The targets for lkdtm's objcopy were missing which caused them to always
be rebuilt. This corrects the problem.

Reported-by: Linus Torvalds 
Signed-off-by: Kees Cook 
---
 drivers/misc/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 4387ccb79e64..7410c6d9a34d 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -69,5 +69,6 @@ OBJCOPYFLAGS :=
 OBJCOPYFLAGS_lkdtm_rodata_objcopy.o := \
--set-section-flags .text=alloc,readonly \
--rename-section .text=.rodata
-$(obj)/lkdtm_rodata_objcopy.o: $(obj)/lkdtm_rodata.o
+targets += lkdtm_rodata.o lkdtm_rodata_objcopy.o
+$(obj)/lkdtm_rodata_objcopy.o: $(obj)/lkdtm_rodata.o FORCE
$(call if_changed,objcopy)
-- 
2.7.4

[PATCH 2/3] lkdtm: Fix targets for objcopy usage

[Kees Cook]

The targets for lkdtm's objcopy were missing which caused them to always
be rebuilt. This corrects the problem.

Reported-by: Linus Torvalds 
Signed-off-by: Kees Cook 
---
 drivers/misc/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 4387ccb79e64..7410c6d9a34d 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -69,5 +69,6 @@ OBJCOPYFLAGS :=
 OBJCOPYFLAGS_lkdtm_rodata_objcopy.o := \
--set-section-flags .text=alloc,readonly \
--rename-section .text=.rodata
-$(obj)/lkdtm_rodata_objcopy.o: $(obj)/lkdtm_rodata.o
+targets += lkdtm_rodata.o lkdtm_rodata_objcopy.o
+$(obj)/lkdtm_rodata_objcopy.o: $(obj)/lkdtm_rodata.o FORCE
$(call if_changed,objcopy)
-- 
2.7.4

[PATCH 0/3] lkdtm: fixes for v4.8-rc1

[Kees Cook]

Hi Greg!

As requested, here's a patch series for 3 small fixes for lkdtm for v4.8
(instead of as a pull request).

Thanks!

-Kees


Kees Cook (2):
  lkdtm: fix false positive warning from -Wmaybe-uninitialized
  lkdtm: Fix targets for objcopy usage

Michael Ellerman (1):
  lkdtm: Mark lkdtm_rodata_do_nothing() notrace

 drivers/misc/Makefile | 3 ++-
 drivers/misc/lkdtm_rodata.c   | 2 +-
 drivers/misc/lkdtm_usercopy.c | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

[PATCH 0/3] lkdtm: fixes for v4.8-rc1

[Kees Cook]

Hi Greg!

As requested, here's a patch series for 3 small fixes for lkdtm for v4.8
(instead of as a pull request).

Thanks!

-Kees


Kees Cook (2):
  lkdtm: fix false positive warning from -Wmaybe-uninitialized
  lkdtm: Fix targets for objcopy usage

Michael Ellerman (1):
  lkdtm: Mark lkdtm_rodata_do_nothing() notrace

 drivers/misc/Makefile | 3 ++-
 drivers/misc/lkdtm_rodata.c   | 2 +-
 drivers/misc/lkdtm_usercopy.c | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

[GIT PULL] Block fixes for 4.8-rc1

[Jens Axboe]

Hi Linus,

Here's the 2nd round of block updates for this merge window. It's a mix
of fixes for changes that went in previously in this round, and fixes in
general. This pull request contains:

- Fixes for loop from Christoph

- A bdi vs gendisk lifetime fix from Dan, worth two cookies.

- A blk-mq timeout fix, when on frozen queues. From Gabriel.

- Writeback fix from Jan, ensuring that __writeback_single_inode() does
  the right thing.

- Fix for bio->bi_rw usage in f2fs from me.

- Error path deadlock fix in blk-mq sysfs registration from me.

- Floppy O_ACCMODE fix from Jiri.

- Fix to the new bio op methods from Mike. One more followup will be
  coming here, ensuring that we don't propagate the block types outside
  of block. That, and a rename of bio->bi_rw is coming right after -rc1
  is cut.

- Various little fixes

Please pull!


  git://git.kernel.dk/linux-block.git for-linus



Christoph Hellwig (2):
  loop: don't try to use AIO for discards
  loop: make do_req_filebacked more robust

Dan Williams (1):
  block: fix bdi vs gendisk lifetime mismatch

Gabriel Krisman Bertazi (1):
  blk-mq: Allow timeouts to run while queue is freezing

Hou Tao (1):
  blkcg: kill unused field nr_undestroyed_grps

Jan Kara (1):
  writeback: Write dirty times for WB_SYNC_ALL writeback

Jens Axboe (2):
  f2fs: drop bio->bi_rw manual assignment
  blk-mq: fix deadlock in blk_mq_register_disk() error path

Jiri Kosina (1):
  floppy: fix open(O_ACCMODE) for ioctl-only open

John Pittman (1):
  Include: blkdev: Removed duplicate 'struct request;' declaration.

Mike Christie (1):
  mm/block: convert rw_page users to bio op use

Paolo Valente (1):
  block: add missing group association in bio-cloning functions

Shaun Tancheff (1):
  Fixup direct bi_rw modifiers

Vegard Nossum (2):
  block: fix use-after-free in seq file
  nbd: fix race in ioctl

 block/bio.c  | 15 +
 block/blk-mq-sysfs.c | 12 ---
 block/blk-mq.c   | 15 -
 block/blk-throttle.c |  5 ---
 block/genhd.c|  3 +-
 drivers/block/brd.c  | 17 +-
 drivers/block/floppy.c   | 21 ++---
 drivers/block/loop.c | 67 
++--

 drivers/block/nbd.c  | 12 +++
 drivers/block/zram/zram_drv.c| 28 +
 drivers/nvdimm/btt.c | 18 +--
 drivers/nvdimm/pmem.c| 12 +++
 fs/block_dev.c   |  7 +++--
 fs/btrfs/extent_io.c |  8 +
 fs/f2fs/data.c   |  1 -
 fs/fs-writeback.c|  1 +
 fs/mpage.c   |  2 +-
 include/linux/backing-dev-defs.h |  1 +
 include/linux/backing-dev.h  |  1 +
 include/linux/bio.h  |  3 ++
 include/linux/blk_types.h| 22 ++---
 include/linux/blkdev.h   |  3 +-
 include/linux/fs.h   |  3 +-
 include/linux/pagemap.h  |  2 +-
 mm/backing-dev.c | 19 
 mm/filemap.c |  6 ++--
 26 files changed, 168 insertions(+), 136 deletions(-)


--
Jens Axboe

[GIT PULL] Block fixes for 4.8-rc1

[Jens Axboe]

Hi Linus,

Here's the 2nd round of block updates for this merge window. It's a mix
of fixes for changes that went in previously in this round, and fixes in
general. This pull request contains:

- Fixes for loop from Christoph

- A bdi vs gendisk lifetime fix from Dan, worth two cookies.

- A blk-mq timeout fix, when on frozen queues. From Gabriel.

- Writeback fix from Jan, ensuring that __writeback_single_inode() does
  the right thing.

- Fix for bio->bi_rw usage in f2fs from me.

- Error path deadlock fix in blk-mq sysfs registration from me.

- Floppy O_ACCMODE fix from Jiri.

- Fix to the new bio op methods from Mike. One more followup will be
  coming here, ensuring that we don't propagate the block types outside
  of block. That, and a rename of bio->bi_rw is coming right after -rc1
  is cut.

- Various little fixes

Please pull!


  git://git.kernel.dk/linux-block.git for-linus



Christoph Hellwig (2):
  loop: don't try to use AIO for discards
  loop: make do_req_filebacked more robust

Dan Williams (1):
  block: fix bdi vs gendisk lifetime mismatch

Gabriel Krisman Bertazi (1):
  blk-mq: Allow timeouts to run while queue is freezing

Hou Tao (1):
  blkcg: kill unused field nr_undestroyed_grps

Jan Kara (1):
  writeback: Write dirty times for WB_SYNC_ALL writeback

Jens Axboe (2):
  f2fs: drop bio->bi_rw manual assignment
  blk-mq: fix deadlock in blk_mq_register_disk() error path

Jiri Kosina (1):
  floppy: fix open(O_ACCMODE) for ioctl-only open

John Pittman (1):
  Include: blkdev: Removed duplicate 'struct request;' declaration.

Mike Christie (1):
  mm/block: convert rw_page users to bio op use

Paolo Valente (1):
  block: add missing group association in bio-cloning functions

Shaun Tancheff (1):
  Fixup direct bi_rw modifiers

Vegard Nossum (2):
  block: fix use-after-free in seq file
  nbd: fix race in ioctl

 block/bio.c  | 15 +
 block/blk-mq-sysfs.c | 12 ---
 block/blk-mq.c   | 15 -
 block/blk-throttle.c |  5 ---
 block/genhd.c|  3 +-
 drivers/block/brd.c  | 17 +-
 drivers/block/floppy.c   | 21 ++---
 drivers/block/loop.c | 67 
++--

 drivers/block/nbd.c  | 12 +++
 drivers/block/zram/zram_drv.c| 28 +
 drivers/nvdimm/btt.c | 18 +--
 drivers/nvdimm/pmem.c| 12 +++
 fs/block_dev.c   |  7 +++--
 fs/btrfs/extent_io.c |  8 +
 fs/f2fs/data.c   |  1 -
 fs/fs-writeback.c|  1 +
 fs/mpage.c   |  2 +-
 include/linux/backing-dev-defs.h |  1 +
 include/linux/backing-dev.h  |  1 +
 include/linux/bio.h  |  3 ++
 include/linux/blk_types.h| 22 ++---
 include/linux/blkdev.h   |  3 +-
 include/linux/fs.h   |  3 +-
 include/linux/pagemap.h  |  2 +-
 mm/backing-dev.c | 19 
 mm/filemap.c |  6 ++--
 26 files changed, 168 insertions(+), 136 deletions(-)


--
Jens Axboe

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[Scott Wood]

On Fri, 2016-08-05 at 20:29 +, york sun wrote:
> On 08/04/2016 08:43 PM, Michael Ellerman wrote:
> > 
> > Does the driver really need to use these routines? They're meant for use
> > early in boot, before PCI is setup.
> > 
> > AFAICS this is just a regular driver, so when it's probed the PCI
> > devices should have already been scanned. In which case pci_get_device()
> > could work couldn't it? (I see other edac drivers doing that).
> I am trying to fix this but need some help. We are dealing with PCIe 
> controller here. Does it have a bus number assigned at this point? If 
> yes, how can I find it? I seem not able to find out where the 
> platform_data is filled as well. Can someone kindly point it out to me?


The platform data comes from add_err_dev() in arch/powerpc/sysdev/fsl_pci.c.

-Scott

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[Scott Wood]

On Fri, 2016-08-05 at 20:29 +, york sun wrote:
> On 08/04/2016 08:43 PM, Michael Ellerman wrote:
> > 
> > Does the driver really need to use these routines? They're meant for use
> > early in boot, before PCI is setup.
> > 
> > AFAICS this is just a regular driver, so when it's probed the PCI
> > devices should have already been scanned. In which case pci_get_device()
> > could work couldn't it? (I see other edac drivers doing that).
> I am trying to fix this but need some help. We are dealing with PCIe 
> controller here. Does it have a bus number assigned at this point? If 
> yes, how can I find it? I seem not able to find out where the 
> platform_data is filled as well. Can someone kindly point it out to me?


The platform data comes from add_err_dev() in arch/powerpc/sysdev/fsl_pci.c.

-Scott

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[york sun]

On 08/04/2016 08:43 PM, Michael Ellerman wrote:
> York Sun  writes:
>
>> Two symbols are missing if mpc85xx_edac driver is compiled as module.
>>
>> Signed-off-by: York Sun 
>>
>> ---
>> Change log
>>   v3: Change subject tag
>>   v2: no change
>>
>>  arch/powerpc/kernel/pci-common.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/pci-common.c 
>> b/arch/powerpc/kernel/pci-common.c
>> index 0f7a60f..86bc484 100644
>> --- a/arch/powerpc/kernel/pci-common.c
>> +++ b/arch/powerpc/kernel/pci-common.c
>> @@ -226,6 +226,7 @@ struct pci_controller* 
>> pci_find_hose_for_OF_device(struct device_node* node)
>>  }
>>  return NULL;
>>  }
>> +EXPORT_SYMBOL(pci_find_hose_for_OF_device);
>>
>>  /*
>>   * Reads the interrupt pin to determine if interrupt is use by card.
>> @@ -1585,6 +1586,7 @@ int early_find_capability(struct pci_controller *hose, 
>> int bus, int devfn,
>>  {
>>  return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
>>  }
>> +EXPORT_SYMBOL(early_find_capability);
>
> Does the driver really need to use these routines? They're meant for use
> early in boot, before PCI is setup.
>
> AFAICS this is just a regular driver, so when it's probed the PCI
> devices should have already been scanned. In which case pci_get_device()
> could work couldn't it? (I see other edac drivers doing that).
>

I don't have deep knowledge of this driver. What I am trying is to 
separate the common DDR part and share it with ARM platforms. Along the 
way, I found the compiling error if build a module. If exposing these 
functions becomes a concern, I can live without it.

York

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[york sun]

On 08/04/2016 08:43 PM, Michael Ellerman wrote:
> York Sun  writes:
>
>> Two symbols are missing if mpc85xx_edac driver is compiled as module.
>>
>> Signed-off-by: York Sun 
>>
>> ---
>> Change log
>>   v3: Change subject tag
>>   v2: no change
>>
>>  arch/powerpc/kernel/pci-common.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/pci-common.c 
>> b/arch/powerpc/kernel/pci-common.c
>> index 0f7a60f..86bc484 100644
>> --- a/arch/powerpc/kernel/pci-common.c
>> +++ b/arch/powerpc/kernel/pci-common.c
>> @@ -226,6 +226,7 @@ struct pci_controller* 
>> pci_find_hose_for_OF_device(struct device_node* node)
>>  }
>>  return NULL;
>>  }
>> +EXPORT_SYMBOL(pci_find_hose_for_OF_device);
>>
>>  /*
>>   * Reads the interrupt pin to determine if interrupt is use by card.
>> @@ -1585,6 +1586,7 @@ int early_find_capability(struct pci_controller *hose, 
>> int bus, int devfn,
>>  {
>>  return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
>>  }
>> +EXPORT_SYMBOL(early_find_capability);
>
> Does the driver really need to use these routines? They're meant for use
> early in boot, before PCI is setup.
>
> AFAICS this is just a regular driver, so when it's probed the PCI
> devices should have already been scanned. In which case pci_get_device()
> could work couldn't it? (I see other edac drivers doing that).
>

I don't have deep knowledge of this driver. What I am trying is to 
separate the common DDR part and share it with ARM platforms. Along the 
way, I found the compiling error if build a module. If exposing these 
functions becomes a concern, I can live without it.

York

[PATCH] asus-laptop: get rid of parse_arg()

[Giedrius Statkevičius]

parse_arg() duplicates the funcionality of kstrtoint() so use the latter
function instead. There is no funcionality change except that in the
case of input being too big -ERANGE will be returned instead of -EINVAL
which is not bad because -ERANGE makes more sense here. The check for
!count is already done by the sysfs core so no need to duplicate it
again. Also, add some minor corrections to error handling to accommodate
the change in return values (parse_arg returned count if everything
succeeded whereas kstrtoint returns 0 in the same situation)

As a result of this patch asus-laptop.ko size is reduced by almost 1%:
add/remove: 0/1 grow/shrink: 1/6 up/down: 1/-149 (-148)
function old new   delta
__UNIQUE_ID_vermagic0 69  70  +1
ls_switch_store  133 117 -16
ledd_store   175 159 -16
display_store157 141 -16
ls_level_store   193 176 -17
gps_store200 178 -22
sysfs_acpi_set.isra  148 125 -23
parse_arg.part39   - -39
Total: Before=19160, After=19012, chg -0.77%

Signed-off-by: Giedrius Statkevičius 
---
 drivers/platform/x86/asus-laptop.c | 77 ++
 1 file changed, 36 insertions(+), 41 deletions(-)

diff --git a/drivers/platform/x86/asus-laptop.c 
b/drivers/platform/x86/asus-laptop.c
index 15f1311..28551f5 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -932,30 +932,19 @@ static ssize_t infos_show(struct device *dev, struct 
device_attribute *attr,
 }
 static DEVICE_ATTR_RO(infos);
 
-static int parse_arg(const char *buf, unsigned long count, int *val)
-{
-   if (!count)
-   return 0;
-   if (count > 31)
-   return -EINVAL;
-   if (sscanf(buf, "%i", val) != 1)
-   return -EINVAL;
-   return count;
-}
-
 static ssize_t sysfs_acpi_set(struct asus_laptop *asus,
  const char *buf, size_t count,
  const char *method)
 {
int rv, value;
 
-   rv = parse_arg(buf, count, );
-   if (rv <= 0)
+   rv = kstrtoint(buf, 0, );
+   if (rv < 0)
return rv;
 
if (write_acpi_int(asus->handle, method, value))
return -ENODEV;
-   return rv;
+   return count;
 }
 
 /*
@@ -975,15 +964,17 @@ static ssize_t ledd_store(struct device *dev, struct 
device_attribute *attr,
struct asus_laptop *asus = dev_get_drvdata(dev);
int rv, value;
 
-   rv = parse_arg(buf, count, );
-   if (rv > 0) {
-   if (write_acpi_int(asus->handle, METHOD_LEDD, value)) {
-   pr_warn("LED display write failed\n");
-   return -ENODEV;
-   }
-   asus->ledd_status = (u32) value;
+   rv = kstrtoint(buf, 0, );
+   if (rv < 0)
+   return rv;
+
+   if (write_acpi_int(asus->handle, METHOD_LEDD, value)) {
+   pr_warn("LED display write failed\n");
+   return -ENODEV;
}
-   return rv;
+
+   asus->ledd_status = (u32) value;
+   return count;
 }
 static DEVICE_ATTR_RW(ledd);
 
@@ -1148,10 +1139,12 @@ static ssize_t display_store(struct device *dev, struct 
device_attribute *attr,
struct asus_laptop *asus = dev_get_drvdata(dev);
int rv, value;
 
-   rv = parse_arg(buf, count, );
-   if (rv > 0)
-   asus_set_display(asus, value);
-   return rv;
+   rv = kstrtoint(buf, 0, );
+   if (rv < 0)
+   return rv;
+
+   asus_set_display(asus, value);
+   return count;
 }
 static DEVICE_ATTR_WO(display);
 
@@ -1190,11 +1183,12 @@ static ssize_t ls_switch_store(struct device *dev,
struct asus_laptop *asus = dev_get_drvdata(dev);
int rv, value;
 
-   rv = parse_arg(buf, count, );
-   if (rv > 0)
-   asus_als_switch(asus, value ? 1 : 0);
+   rv = kstrtoint(buf, 0, );
+   if (rv < 0)
+   return rv;
 
-   return rv;
+   asus_als_switch(asus, value ? 1 : 0);
+   return count;
 }
 static DEVICE_ATTR_RW(ls_switch);
 
@@ -1219,14 +1213,15 @@ static ssize_t ls_level_store(struct device *dev, 
struct device_attribute *attr,
struct asus_laptop *asus = dev_get_drvdata(dev);
int rv, value;
 
-   rv = parse_arg(buf, count, );
-   if (rv > 0) {
-   value = (0 < value) ? ((15 < value) ? 15 : value) : 0;
-   /* 0 <= value <= 15 */
-   asus_als_level(asus, value);
-   }
+   rv = kstrtoint(buf, 0, );
+   if (rv < 0)
+   return rv;
+
+   value = (0 < value) ? ((15 < value) ? 15 : value) : 0;
+   /* 0 

[PATCH] asus-laptop: get rid of parse_arg()

[Giedrius Statkevičius]

parse_arg() duplicates the funcionality of kstrtoint() so use the latter
function instead. There is no funcionality change except that in the
case of input being too big -ERANGE will be returned instead of -EINVAL
which is not bad because -ERANGE makes more sense here. The check for
!count is already done by the sysfs core so no need to duplicate it
again. Also, add some minor corrections to error handling to accommodate
the change in return values (parse_arg returned count if everything
succeeded whereas kstrtoint returns 0 in the same situation)

As a result of this patch asus-laptop.ko size is reduced by almost 1%:
add/remove: 0/1 grow/shrink: 1/6 up/down: 1/-149 (-148)
function old new   delta
__UNIQUE_ID_vermagic0 69  70  +1
ls_switch_store  133 117 -16
ledd_store   175 159 -16
display_store157 141 -16
ls_level_store   193 176 -17
gps_store200 178 -22
sysfs_acpi_set.isra  148 125 -23
parse_arg.part39   - -39
Total: Before=19160, After=19012, chg -0.77%

Signed-off-by: Giedrius Statkevičius 
---
 drivers/platform/x86/asus-laptop.c | 77 ++
 1 file changed, 36 insertions(+), 41 deletions(-)

diff --git a/drivers/platform/x86/asus-laptop.c 
b/drivers/platform/x86/asus-laptop.c
index 15f1311..28551f5 100644
--- a/drivers/platform/x86/asus-laptop.c
+++ b/drivers/platform/x86/asus-laptop.c
@@ -932,30 +932,19 @@ static ssize_t infos_show(struct device *dev, struct 
device_attribute *attr,
 }
 static DEVICE_ATTR_RO(infos);
 
-static int parse_arg(const char *buf, unsigned long count, int *val)
-{
-   if (!count)
-   return 0;
-   if (count > 31)
-   return -EINVAL;
-   if (sscanf(buf, "%i", val) != 1)
-   return -EINVAL;
-   return count;
-}
-
 static ssize_t sysfs_acpi_set(struct asus_laptop *asus,
  const char *buf, size_t count,
  const char *method)
 {
int rv, value;
 
-   rv = parse_arg(buf, count, );
-   if (rv <= 0)
+   rv = kstrtoint(buf, 0, );
+   if (rv < 0)
return rv;
 
if (write_acpi_int(asus->handle, method, value))
return -ENODEV;
-   return rv;
+   return count;
 }
 
 /*
@@ -975,15 +964,17 @@ static ssize_t ledd_store(struct device *dev, struct 
device_attribute *attr,
struct asus_laptop *asus = dev_get_drvdata(dev);
int rv, value;
 
-   rv = parse_arg(buf, count, );
-   if (rv > 0) {
-   if (write_acpi_int(asus->handle, METHOD_LEDD, value)) {
-   pr_warn("LED display write failed\n");
-   return -ENODEV;
-   }
-   asus->ledd_status = (u32) value;
+   rv = kstrtoint(buf, 0, );
+   if (rv < 0)
+   return rv;
+
+   if (write_acpi_int(asus->handle, METHOD_LEDD, value)) {
+   pr_warn("LED display write failed\n");
+   return -ENODEV;
}
-   return rv;
+
+   asus->ledd_status = (u32) value;
+   return count;
 }
 static DEVICE_ATTR_RW(ledd);
 
@@ -1148,10 +1139,12 @@ static ssize_t display_store(struct device *dev, struct 
device_attribute *attr,
struct asus_laptop *asus = dev_get_drvdata(dev);
int rv, value;
 
-   rv = parse_arg(buf, count, );
-   if (rv > 0)
-   asus_set_display(asus, value);
-   return rv;
+   rv = kstrtoint(buf, 0, );
+   if (rv < 0)
+   return rv;
+
+   asus_set_display(asus, value);
+   return count;
 }
 static DEVICE_ATTR_WO(display);
 
@@ -1190,11 +1183,12 @@ static ssize_t ls_switch_store(struct device *dev,
struct asus_laptop *asus = dev_get_drvdata(dev);
int rv, value;
 
-   rv = parse_arg(buf, count, );
-   if (rv > 0)
-   asus_als_switch(asus, value ? 1 : 0);
+   rv = kstrtoint(buf, 0, );
+   if (rv < 0)
+   return rv;
 
-   return rv;
+   asus_als_switch(asus, value ? 1 : 0);
+   return count;
 }
 static DEVICE_ATTR_RW(ls_switch);
 
@@ -1219,14 +1213,15 @@ static ssize_t ls_level_store(struct device *dev, 
struct device_attribute *attr,
struct asus_laptop *asus = dev_get_drvdata(dev);
int rv, value;
 
-   rv = parse_arg(buf, count, );
-   if (rv > 0) {
-   value = (0 < value) ? ((15 < value) ? 15 : value) : 0;
-   /* 0 <= value <= 15 */
-   asus_als_level(asus, value);
-   }
+   rv = kstrtoint(buf, 0, );
+   if (rv < 0)
+   return rv;
+
+   value = (0 < value) ? ((15 < value) ? 15 : value) : 0;
+   /* 0 <= value <= 15 */
+   

Re: [linux-mm] Drastic increase in application memory usage with Kernel version upgrade

[Pavel Machek]

On Fri 2016-08-05 20:17:36, PINTU KUMAR wrote:
> Hi,

> > On Fri, Aug 05, 2016 at 10:26:37AM +0530, PINTU KUMAR wrote:
> > > Hi All,
> > >
> > > For one of our ARM embedded product, we recently updated the Kernel
> > > version from
> > > 3.4 to 3.18 and we noticed that the same application memory usage (PSS
> > > value) gone up by ~10% and for some cases it even crossed ~50%.
> > > There is no change in platform part. All platform component was built
> > > with ARM 32-bit toolchain.
> > > However, the Kernel is changed from 32-bit to 64-bit.
> > >
> > > Is upgrading Kernel version and moving from 32-bit to 64-bit is such a 
> > > risk
> ?
> > > After the upgrade, what can we do further to reduce the application
> > > memory usage ?
> > > Is there any other factor that will help us to improve without major
> > > modifications in platform ?
> > >
> > > As a proof, we did a small experiment on our Ubuntu-32 bit machine.
> > > We upgraded Ubuntu Kernel version from 3.13 to 4.01 and we observed
> > > the
> > > following:
> > > --
> > > --
> > > -
> > > |UBUNTU-32 bit|Kernel 3.13|Kernel 4.03|DIFF   |
> > > |CALCULATOR PSS   |6057 KB|6466 KB|409 KB |
> > > --
> > > --
> > > -
> > > So, just by upgrading the Kernel version: PSS value for calculator is
> > > increased by 409KB.
> > >
> > > If anybody knows any in-sight about it please point out more details
> > > about the root cause.
> > 
> > One of culprit is [8c6e50b0290c, mm: introduce vm_ops->map_pages()].
> Ok. Thank you for your reply.
> So, if I revert this patch, will the memory usage be decreased for the 
> processes
> with Kernel 3.18 ?

I guess you should try it...

You may want to try the same kernel version, once in 32-bit and once
in 64-bit version. And you may consider moving to recent kernel.

Yes, 64-bit kernel will increase memory usage _of kernel_, but...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [linux-mm] Drastic increase in application memory usage with Kernel version upgrade

[Pavel Machek]

On Fri 2016-08-05 20:17:36, PINTU KUMAR wrote:
> Hi,

> > On Fri, Aug 05, 2016 at 10:26:37AM +0530, PINTU KUMAR wrote:
> > > Hi All,
> > >
> > > For one of our ARM embedded product, we recently updated the Kernel
> > > version from
> > > 3.4 to 3.18 and we noticed that the same application memory usage (PSS
> > > value) gone up by ~10% and for some cases it even crossed ~50%.
> > > There is no change in platform part. All platform component was built
> > > with ARM 32-bit toolchain.
> > > However, the Kernel is changed from 32-bit to 64-bit.
> > >
> > > Is upgrading Kernel version and moving from 32-bit to 64-bit is such a 
> > > risk
> ?
> > > After the upgrade, what can we do further to reduce the application
> > > memory usage ?
> > > Is there any other factor that will help us to improve without major
> > > modifications in platform ?
> > >
> > > As a proof, we did a small experiment on our Ubuntu-32 bit machine.
> > > We upgraded Ubuntu Kernel version from 3.13 to 4.01 and we observed
> > > the
> > > following:
> > > --
> > > --
> > > -
> > > |UBUNTU-32 bit|Kernel 3.13|Kernel 4.03|DIFF   |
> > > |CALCULATOR PSS   |6057 KB|6466 KB|409 KB |
> > > --
> > > --
> > > -
> > > So, just by upgrading the Kernel version: PSS value for calculator is
> > > increased by 409KB.
> > >
> > > If anybody knows any in-sight about it please point out more details
> > > about the root cause.
> > 
> > One of culprit is [8c6e50b0290c, mm: introduce vm_ops->map_pages()].
> Ok. Thank you for your reply.
> So, if I revert this patch, will the memory usage be decreased for the 
> processes
> with Kernel 3.18 ?

I guess you should try it...

You may want to try the same kernel version, once in 32-bit and once
in 64-bit version. And you may consider moving to recent kernel.

Yes, 64-bit kernel will increase memory usage _of kernel_, but...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH v2 3/3] kexec: extend kexec_file_load system call

[Thiago Jung Bauermann]

Hi,

Am Dienstag, 26 Juli 2016, 21:24:29 schrieb Thiago Jung Bauermann:
> Notes:
> This is a new version of the last patch in this series which adds
> a function where each architecture can verify if the DTB is safe
> to load:
> 
> int __weak arch_kexec_verify_buffer(enum kexec_file_type type,
> const void *buf,
> unsigned long size)
> {
> return -EINVAL;
> }
> 
> I will then provide an implementation in my powerpc patch series
> which checks that the DTB only contains nodes and properties from a
> whitelist. arch_kexec_kernel_image_load will copy these properties
> to the device tree blob the kernel was booted with (and perform
> other changes such as setting /chosen/bootargs, of course).

Is this approach ok? If so, I'll post a patch next week adding an 
arch_kexec_verify_buffer hook for powerpc to enforce the whitelist, and also 
a new version of the patches implementing kexec_file_load for powerpc on top 
of this series.

Eric, does this address your concerns?

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [PATCH v2 3/3] kexec: extend kexec_file_load system call

[Thiago Jung Bauermann]

Hi,

Am Dienstag, 26 Juli 2016, 21:24:29 schrieb Thiago Jung Bauermann:
> Notes:
> This is a new version of the last patch in this series which adds
> a function where each architecture can verify if the DTB is safe
> to load:
> 
> int __weak arch_kexec_verify_buffer(enum kexec_file_type type,
> const void *buf,
> unsigned long size)
> {
> return -EINVAL;
> }
> 
> I will then provide an implementation in my powerpc patch series
> which checks that the DTB only contains nodes and properties from a
> whitelist. arch_kexec_kernel_image_load will copy these properties
> to the device tree blob the kernel was booted with (and perform
> other changes such as setting /chosen/bootargs, of course).

Is this approach ok? If so, I'll post a patch next week adding an 
arch_kexec_verify_buffer hook for powerpc to enforce the whitelist, and also 
a new version of the patches implementing kexec_file_load for powerpc on top 
of this series.

Eric, does this address your concerns?

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

menuconfig very unfriendly with MEDIA_SUBDRV_AUTOSELECT=y

[Pavel Machek]

Hi!

I was trying to enable a driver (trivial task, right) and spent like
10 minutes scratching my hand.

menuconfig is nice, because you can search with '/' and it tells you
the dependencies, and the path to the config option.

What is not nice is that you have to write down the path, and then
follow it manually.

What is extremely ugly is that if you have option like
MEDIA_SUBDRV_AUTOSELECT=y enabled, it hides other config
options. Menuconfig tells you where your option should be, but it is
not there and you wonder what is going on...

Any ideas?

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

menuconfig very unfriendly with MEDIA_SUBDRV_AUTOSELECT=y

[Pavel Machek]

Hi!

I was trying to enable a driver (trivial task, right) and spent like
10 minutes scratching my hand.

menuconfig is nice, because you can search with '/' and it tells you
the dependencies, and the path to the config option.

What is not nice is that you have to write down the path, and then
follow it manually.

What is extremely ugly is that if you have option like
MEDIA_SUBDRV_AUTOSELECT=y enabled, it hides other config
options. Menuconfig tells you where your option should be, but it is
not there and you wonder what is going on...

Any ideas?

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: NVME regression in all kernels after 4.4.x for NVME in M2 slot for laptop?

[Keith Busch]

On Fri, Aug 05, 2016 at 12:03:23PM -0700, Marc MERLIN wrote:
> Would this patch make sense as being the reason why I can't S3 sleep
> anymore and would you have a test patch against 4.5, 4.6, or 4.7 I can
> try to see if it fixes the problem?

Hi Marc,

It might be blk-mq's hot cpu notifier is invoked during suspend and
waiting for nvme's queues to freeze, which may not happen if a request
is waiting on a stopped queue. The patch you biseceted doesn't
necessarilly fix that, but the window for when a request could get queued
like that was much shorter.

Assuming that is the problem, S3 suspends PCI hardware before IO
tasks. I'll see if I can reproduce on one of my machines and look into
a fix.

Thanks,
Keith

Re: NVME regression in all kernels after 4.4.x for NVME in M2 slot for laptop?

[Keith Busch]

On Fri, Aug 05, 2016 at 12:03:23PM -0700, Marc MERLIN wrote:
> Would this patch make sense as being the reason why I can't S3 sleep
> anymore and would you have a test patch against 4.5, 4.6, or 4.7 I can
> try to see if it fixes the problem?

Hi Marc,

It might be blk-mq's hot cpu notifier is invoked during suspend and
waiting for nvme's queues to freeze, which may not happen if a request
is waiting on a stopped queue. The patch you biseceted doesn't
necessarilly fix that, but the window for when a request could get queued
like that was much shorter.

Assuming that is the problem, S3 suspends PCI hardware before IO
tasks. I'll see if I can reproduce on one of my machines and look into
a fix.

Thanks,
Keith

Re: [PATCH v6 0/2] Block layer support ZAC/ZBC commands

[Shaun Tancheff]

On Tue, Aug 2, 2016 at 8:29 PM, Damien Le Moal  wrote:
> Hannes, Shaun,
>
> Let me add some more comments.
>
>> On Aug 2, 2016, at 23:35, Hannes Reinecke  wrote:
>>
>> On 08/01/2016 07:07 PM, Shaun Tancheff wrote:
>>> On Mon, Aug 1, 2016 at 4:41 AM, Christoph Hellwig  wrote:

 Can you please integrate this with Hannes series so that it uses
 his cache of the zone information?
>>>
>>> Adding Hannes and Damien to Cc.
>>>
>>> Christoph,
>>>
>>> I can make a patch the marshal Hannes' RB-Tree into to a block report, that 
>>> is
>>> quite simple. I can even have the open/close/reset zone commands update the
>>> RB-Tree .. the non-private parts anyway. I would prefer to do this around 
>>> the
>>> CONFIG_SD_ZBC support, offering the existing type of patch for setups that 
>>> do
>>> not need the RB-Tree to function with zoned media.

I have posted patches to integrate with the zone cache, hopefully they
make sense.

>>>
>>> I do still have concerns with the approach which I have shared in smaller
>>> forums but perhaps I have to bring them to this group.
>>>
>>> First is the memory consumption. This isn't really much of a concern for 
>>> large
>>> servers with few drives but I think the embedded NAS market will grumble as
>>> well as the large data pods trying to stuff 300+ drives in a chassis.
>>>
>>> As of now the RB-Tree needs to hold ~3 zones.
>>> sizeof() reports struct blk_zone to use 120 bytes on x86_64. This yields
>>> around 3.5 MB per zoned drive attached.
>>> Which is fine if it is really needed, but most of it is fixed information
>>> and it can be significantly condensed (I have proposed 8 bytes per zone held
>>> in an array as more than adequate). Worse is that the crucial piece of
>>> information, the current wp needed for scheduling the next write, is mostly
>>> out of date because it is updated only after the write completes and zones
>>> being actively written to must work off of the last location / size that was
>>> submitted, not completed. The work around is for that tracking to be handled
>>> in the private_data member. I am not saying that updating the wp on
>>> completing a write isn’t important, I am saying that the bi_end_io hook is
>>> the existing hook that works just fine.
>>>
>> Which _actually_ is not true; with my patches I'll update the write
>> pointer prior to submit the I/O (on the reasoning that most of the time
>> I/O will succeed) and re-read the zone information if an I/O failed.
>> (Which I'll have to do anyway as after an I/O failure the write pointer
>> status is not clearly defined.)

Apologies for my mis-characterization.

>> I have thought about condensing the RB tree information, but then I
>> figured that for 'real' SMR handling we cannot assume all zones are of
>> fixed size, and hence we need all the information there.
>> Any condensing method would assume a given structure of the zones, which
>> the standard just doesn't provide.
>> Or am I missing something here?
>
> Indeed, the standards do not mandate any particular zone configuration,
> constant zone size, etc. So writing code so that can be handled is certainly
> the right way of doing things. However, if we decide to go forward with
> mapping RESET WRITE POINTER command to DISCARD, then at least a constant
> zone size (minus the last zone as you said) must be assumed, and that
> information can be removed from the entries in the RB tree (as it will be
> saved for the sysfs "zone_size" file anyway. Adding a little code to handle
> that eventual last runt zone with a different size is not a big problem.

>> As for write pointer handling: yes, the write pointer on the zones is
>> not really useful for upper-level usage.
>> Where we do need it is to detect I/O which is crossing the write pointer
>> (eg when doing reads over the entire zone).
>> As per spec you will be getting an I/O error here, so we need to split
>> the I/O on the write pointer to get valid results back.
>
> To be precise here, the I/O splitting will be handled by the block layer
> thanks to the "chunk_sectors" setting. But that relies on a constant zone
> size assumption too.
>
> The RB-tree here is most useful for reads over or after the write pointer as
> this can have different behavior on different drives (URSWRZ bit). The RB-tree
> allows us to hide these differences to upper layers and simplify support at
> those levels.

It is unfortunate that path was chosen rather than returning Made Up Data.
However I don't think this is a file system or device mapper problem as
neither of those layers attempt to read blocks they have not written
(or discarded).
All I can think of something that probes the drive media for a partition table
or other identification...isn't the conventional space sufficient to cover
those cases?
Anyway you could just handle the error and sense codes [CHECK CONDITION /
ILLEGAL REQUEST / ATTEMPT TO READ INVALID DATA] and return MUD
when URSWRZ is 0. It 

Re: [PATCH v6 0/2] Block layer support ZAC/ZBC commands

[Shaun Tancheff]

On Tue, Aug 2, 2016 at 8:29 PM, Damien Le Moal  wrote:
> Hannes, Shaun,
>
> Let me add some more comments.
>
>> On Aug 2, 2016, at 23:35, Hannes Reinecke  wrote:
>>
>> On 08/01/2016 07:07 PM, Shaun Tancheff wrote:
>>> On Mon, Aug 1, 2016 at 4:41 AM, Christoph Hellwig  wrote:

 Can you please integrate this with Hannes series so that it uses
 his cache of the zone information?
>>>
>>> Adding Hannes and Damien to Cc.
>>>
>>> Christoph,
>>>
>>> I can make a patch the marshal Hannes' RB-Tree into to a block report, that 
>>> is
>>> quite simple. I can even have the open/close/reset zone commands update the
>>> RB-Tree .. the non-private parts anyway. I would prefer to do this around 
>>> the
>>> CONFIG_SD_ZBC support, offering the existing type of patch for setups that 
>>> do
>>> not need the RB-Tree to function with zoned media.

I have posted patches to integrate with the zone cache, hopefully they
make sense.

>>>
>>> I do still have concerns with the approach which I have shared in smaller
>>> forums but perhaps I have to bring them to this group.
>>>
>>> First is the memory consumption. This isn't really much of a concern for 
>>> large
>>> servers with few drives but I think the embedded NAS market will grumble as
>>> well as the large data pods trying to stuff 300+ drives in a chassis.
>>>
>>> As of now the RB-Tree needs to hold ~3 zones.
>>> sizeof() reports struct blk_zone to use 120 bytes on x86_64. This yields
>>> around 3.5 MB per zoned drive attached.
>>> Which is fine if it is really needed, but most of it is fixed information
>>> and it can be significantly condensed (I have proposed 8 bytes per zone held
>>> in an array as more than adequate). Worse is that the crucial piece of
>>> information, the current wp needed for scheduling the next write, is mostly
>>> out of date because it is updated only after the write completes and zones
>>> being actively written to must work off of the last location / size that was
>>> submitted, not completed. The work around is for that tracking to be handled
>>> in the private_data member. I am not saying that updating the wp on
>>> completing a write isn’t important, I am saying that the bi_end_io hook is
>>> the existing hook that works just fine.
>>>
>> Which _actually_ is not true; with my patches I'll update the write
>> pointer prior to submit the I/O (on the reasoning that most of the time
>> I/O will succeed) and re-read the zone information if an I/O failed.
>> (Which I'll have to do anyway as after an I/O failure the write pointer
>> status is not clearly defined.)

Apologies for my mis-characterization.

>> I have thought about condensing the RB tree information, but then I
>> figured that for 'real' SMR handling we cannot assume all zones are of
>> fixed size, and hence we need all the information there.
>> Any condensing method would assume a given structure of the zones, which
>> the standard just doesn't provide.
>> Or am I missing something here?
>
> Indeed, the standards do not mandate any particular zone configuration,
> constant zone size, etc. So writing code so that can be handled is certainly
> the right way of doing things. However, if we decide to go forward with
> mapping RESET WRITE POINTER command to DISCARD, then at least a constant
> zone size (minus the last zone as you said) must be assumed, and that
> information can be removed from the entries in the RB tree (as it will be
> saved for the sysfs "zone_size" file anyway. Adding a little code to handle
> that eventual last runt zone with a different size is not a big problem.

>> As for write pointer handling: yes, the write pointer on the zones is
>> not really useful for upper-level usage.
>> Where we do need it is to detect I/O which is crossing the write pointer
>> (eg when doing reads over the entire zone).
>> As per spec you will be getting an I/O error here, so we need to split
>> the I/O on the write pointer to get valid results back.
>
> To be precise here, the I/O splitting will be handled by the block layer
> thanks to the "chunk_sectors" setting. But that relies on a constant zone
> size assumption too.
>
> The RB-tree here is most useful for reads over or after the write pointer as
> this can have different behavior on different drives (URSWRZ bit). The RB-tree
> allows us to hide these differences to upper layers and simplify support at
> those levels.

It is unfortunate that path was chosen rather than returning Made Up Data.
However I don't think this is a file system or device mapper problem as
neither of those layers attempt to read blocks they have not written
(or discarded).
All I can think of something that probes the drive media for a partition table
or other identification...isn't the conventional space sufficient to cover
those cases?
Anyway you could just handle the error and sense codes [CHECK CONDITION /
ILLEGAL REQUEST / ATTEMPT TO READ INVALID DATA] and return MUD
when URSWRZ is 0. It would have the same effect as using the zone cache

[PATCH 1/1] brcmfmac: fix pmksa->bssid usage

[Nicolas Iooss]

The struct cfg80211_pmksa defines its bssid field as:

const u8 *bssid;

contrary to struct brcmf_pmksa, which uses:

u8 bssid[ETH_ALEN];

Therefore in brcmf_cfg80211_del_pmksa(), >bssid takes the address
of this field (of type u8**), not the one of its content (which would be
u8*).  Remove the & operator to make brcmf_dbg("%pM") and memcmp()
behave as expected.

This bug have been found using a custom static checker (which checks the
usage of %p... attributes at build time).  It has been introduced in
commit 6c404f34f2bd ("brcmfmac: Cleanup pmksa cache handling code"),
which replaced pmksa->bssid by >bssid while refactoring the code,
without modifying struct cfg80211_pmksa definition.

Fixes: 6c404f34f2bd ("brcmfmac: Cleanup pmksa cache handling code")
Cc: sta...@ger.kernel.org
Signed-off-by: Nicolas Iooss 
---

scripts/checkpatch.pl reports a warning: "Prefer ether_addr_equal() or
ether_addr_equal_unaligned() over memcmp()".  Because some files in
drivers/net/wireless/broadcom/brcm80211/brcmfmac/ still use memcmp()
to compare addresses and because I do not know whether pmksa->bssid is
always aligned, I did not follow this warning.

 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 2628d5e12c64..aceab77cd95a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3884,11 +3884,11 @@ brcmf_cfg80211_del_pmksa(struct wiphy *wiphy, struct 
net_device *ndev,
if (!check_vif_up(ifp->vif))
return -EIO;
 
-   brcmf_dbg(CONN, "del_pmksa - PMK bssid = %pM\n", >bssid);
+   brcmf_dbg(CONN, "del_pmksa - PMK bssid = %pM\n", pmksa->bssid);
 
npmk = le32_to_cpu(cfg->pmk_list.npmk);
for (i = 0; i < npmk; i++)
-   if (!memcmp(>bssid, [i].bssid, ETH_ALEN))
+   if (!memcmp(pmksa->bssid, [i].bssid, ETH_ALEN))
break;
 
if ((npmk > 0) && (i < npmk)) {
-- 
2.9.2

[PATCH 1/1] brcmfmac: fix pmksa->bssid usage

[Nicolas Iooss]

The struct cfg80211_pmksa defines its bssid field as:

const u8 *bssid;

contrary to struct brcmf_pmksa, which uses:

u8 bssid[ETH_ALEN];

Therefore in brcmf_cfg80211_del_pmksa(), >bssid takes the address
of this field (of type u8**), not the one of its content (which would be
u8*).  Remove the & operator to make brcmf_dbg("%pM") and memcmp()
behave as expected.

This bug have been found using a custom static checker (which checks the
usage of %p... attributes at build time).  It has been introduced in
commit 6c404f34f2bd ("brcmfmac: Cleanup pmksa cache handling code"),
which replaced pmksa->bssid by >bssid while refactoring the code,
without modifying struct cfg80211_pmksa definition.

Fixes: 6c404f34f2bd ("brcmfmac: Cleanup pmksa cache handling code")
Cc: sta...@ger.kernel.org
Signed-off-by: Nicolas Iooss 
---

scripts/checkpatch.pl reports a warning: "Prefer ether_addr_equal() or
ether_addr_equal_unaligned() over memcmp()".  Because some files in
drivers/net/wireless/broadcom/brcm80211/brcmfmac/ still use memcmp()
to compare addresses and because I do not know whether pmksa->bssid is
always aligned, I did not follow this warning.

 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 2628d5e12c64..aceab77cd95a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3884,11 +3884,11 @@ brcmf_cfg80211_del_pmksa(struct wiphy *wiphy, struct 
net_device *ndev,
if (!check_vif_up(ifp->vif))
return -EIO;
 
-   brcmf_dbg(CONN, "del_pmksa - PMK bssid = %pM\n", >bssid);
+   brcmf_dbg(CONN, "del_pmksa - PMK bssid = %pM\n", pmksa->bssid);
 
npmk = le32_to_cpu(cfg->pmk_list.npmk);
for (i = 0; i < npmk; i++)
-   if (!memcmp(>bssid, [i].bssid, ETH_ALEN))
+   if (!memcmp(pmksa->bssid, [i].bssid, ETH_ALEN))
break;
 
if ((npmk > 0) && (i < npmk)) {
-- 
2.9.2

Re: [PATCH 1/1] RDS: add __printf format attribute to error reporting functions

[Santosh Shilimkar]

On 8/5/2016 1:11 PM, Nicolas Iooss wrote:

This is helpful to detect at compile-time errors related to format
strings.

Signed-off-by: Nicolas Iooss 
---

OK.
Acked-by: Santosh Shilimkar 

Re: [PATCH 1/1] RDS: add __printf format attribute to error reporting functions

[Santosh Shilimkar]

On 8/5/2016 1:11 PM, Nicolas Iooss wrote:

This is helpful to detect at compile-time errors related to format
strings.

Signed-off-by: Nicolas Iooss 
---

OK.
Acked-by: Santosh Shilimkar 

[GIT PULL] More USB driver patches for 4.8-rc1

[Greg KH]

The following changes since commit e65805251f2db69c9f67ed8062ab82526be5a374:

  Merge branch 'irq-core-for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2016-07-25 21:35:03 
-0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ tags/usb-4.8-rc1

for you to fetch changes up to 0bf048abebb6e69be3c1630878419d80944c3cfd:

  staging: emxx_udc: allow modular build (2016-07-26 08:47:08 -0700)


More USB patches for 4.8-rc1

Here are a few more straggler patches for USB for 4.8-rc1.  Most of
these are for the usb-serial driver tree.  All of those have been in
linux-next for a long time, but missed my previous pull request to you.

The remaining change is to fix up a staging tree build error, due to
some USB gadget driver changes that went in.  I put it in this tree as
it was for a USB driver and people are reporting the build error on your
tree.

All of these have been in linux-next for this week, and longer for the
usb-serial changes.

Signed-off-by: Greg Kroah-Hartman 


Arnd Bergmann (1):
  staging: emxx_udc: allow modular build

Daniele Palmas (1):
  USB: serial: option: add support for Telit LE910 PID 0x1206

Greg Kroah-Hartman (1):
  Merge tag 'usb-serial-4.8-rc1' of 
git://git.kernel.org/.../johan/usb-serial into usb-testing

Mathieu OTHACEHE (4):
  USB: serial: ti_usb_3410_5052: remove useless comments
  USB: serial: ti_usb_3410_5052: use __packed
  USB: serial: ti_usb_3410_5052: remove ti_usb_3410_5052.h
  USB: serial: ti_usb_3410_5052: use functions rather than macros

Muhammad Falak R Wani (1):
  USB: serial: cp210x: use kmemdup

Oliver Neukum (1):
  USB: serial: use variable for status

 drivers/staging/emxx_udc/Kconfig  |   2 +-
 drivers/staging/emxx_udc/emxx_udc.c   |  36 -
 drivers/usb/serial/cp210x.c   |   4 +-
 drivers/usb/serial/generic.c  |  18 ++-
 drivers/usb/serial/option.c   |   3 +
 drivers/usb/serial/ti_usb_3410_5052.c | 271 ++
 drivers/usb/serial/ti_usb_3410_5052.h | 259 
 7 files changed, 293 insertions(+), 300 deletions(-)
 delete mode 100644 drivers/usb/serial/ti_usb_3410_5052.h

[GIT PULL] More USB driver patches for 4.8-rc1

[Greg KH]

The following changes since commit e65805251f2db69c9f67ed8062ab82526be5a374:

  Merge branch 'irq-core-for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (2016-07-25 21:35:03 
-0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ tags/usb-4.8-rc1

for you to fetch changes up to 0bf048abebb6e69be3c1630878419d80944c3cfd:

  staging: emxx_udc: allow modular build (2016-07-26 08:47:08 -0700)


More USB patches for 4.8-rc1

Here are a few more straggler patches for USB for 4.8-rc1.  Most of
these are for the usb-serial driver tree.  All of those have been in
linux-next for a long time, but missed my previous pull request to you.

The remaining change is to fix up a staging tree build error, due to
some USB gadget driver changes that went in.  I put it in this tree as
it was for a USB driver and people are reporting the build error on your
tree.

All of these have been in linux-next for this week, and longer for the
usb-serial changes.

Signed-off-by: Greg Kroah-Hartman 


Arnd Bergmann (1):
  staging: emxx_udc: allow modular build

Daniele Palmas (1):
  USB: serial: option: add support for Telit LE910 PID 0x1206

Greg Kroah-Hartman (1):
  Merge tag 'usb-serial-4.8-rc1' of 
git://git.kernel.org/.../johan/usb-serial into usb-testing

Mathieu OTHACEHE (4):
  USB: serial: ti_usb_3410_5052: remove useless comments
  USB: serial: ti_usb_3410_5052: use __packed
  USB: serial: ti_usb_3410_5052: remove ti_usb_3410_5052.h
  USB: serial: ti_usb_3410_5052: use functions rather than macros

Muhammad Falak R Wani (1):
  USB: serial: cp210x: use kmemdup

Oliver Neukum (1):
  USB: serial: use variable for status

 drivers/staging/emxx_udc/Kconfig  |   2 +-
 drivers/staging/emxx_udc/emxx_udc.c   |  36 -
 drivers/usb/serial/cp210x.c   |   4 +-
 drivers/usb/serial/generic.c  |  18 ++-
 drivers/usb/serial/option.c   |   3 +
 drivers/usb/serial/ti_usb_3410_5052.c | 271 ++
 drivers/usb/serial/ti_usb_3410_5052.h | 259 
 7 files changed, 293 insertions(+), 300 deletions(-)
 delete mode 100644 drivers/usb/serial/ti_usb_3410_5052.h

[GIT PULL] NTB bug fixes for v4.8

[Jon Mason]

Hello Linus,
Here are a few NTB improvements and bug fixes for 4.8.  Please consider
pulling them.

Thanks,
Jon



The following changes since commit 523d939ef98fd712632d93a5a2b588e477a7565e:

  Linux 4.7 (2016-07-24 12:23:50 -0700)

are available in the git repository at:

  git://github.com/jonmason/ntb tags/ntb-4.8

for you to fetch changes up to 95f1464f695055c72de6044d7c8a2a7a1e0c7ea2:

  NTB: ntb_hw_intel: use local variable pdev (2016-08-05 10:34:13 -0400)


NTB bug fixes for the ntb_tool and ntb_perf, and improvements to the
ntb_perf and ntb_pingpong for increased debugability.  Also,
modification to the ntb_transport layer to increase/decrease the number
of transport entries depending on the ring size.


Allen Hubbe (2):
  NTB: ntb_hw_intel: show BAR size in debugfs info
  NTB: ntb_hw_intel: use local variable pdev

Dave Jiang (1):
  NTB: allocate number transport entries depending on size of ring size

Logan Gunthorpe (14):
  ntb_tool: Fix infinite loop bug when writing spad/peer_spad file
  ntb_tool: BUG: Ensure the buffer size is large enough to return all spads
  ntb_perf: Allow limiting the size of the memory windows
  ntb_tool: Add memory window debug support
  ntb_transport: Check the number of spads the hardware supports
  ntb_perf: Schedule based on time not on performance
  ntb_perf: Improve thread handling to increase robustness
  ntb_perf: Return results by reading the run file
  ntb_perf: Wait for link before running test
  ntb_tool: Postpone memory window initialization for the user
  ntb_tool: Add link status and files to debugfs
  ntb_pingpong: Add a debugfs file to get the ping count
  ntb_perf: clear link_is_up flag when the link goes down.
  ntb_test: Add a selftest script for the NTB subsystem

 MAINTAINERS |   1 +
 drivers/ntb/hw/intel/ntb_hw_intel.c |  49 +++-
 drivers/ntb/ntb_transport.c |  38 ++-
 drivers/ntb/test/ntb_perf.c | 240 +++--
 drivers/ntb/test/ntb_pingpong.c |  62 -
 drivers/ntb/test/ntb_tool.c | 459 +++-
 tools/testing/selftests/ntb/ntb_test.sh | 422 +
 7 files changed, 1172 insertions(+), 99 deletions(-)
 create mode 100755 tools/testing/selftests/ntb/ntb_test.sh

[GIT PULL] NTB bug fixes for v4.8

[Jon Mason]

Hello Linus,
Here are a few NTB improvements and bug fixes for 4.8.  Please consider
pulling them.

Thanks,
Jon



The following changes since commit 523d939ef98fd712632d93a5a2b588e477a7565e:

  Linux 4.7 (2016-07-24 12:23:50 -0700)

are available in the git repository at:

  git://github.com/jonmason/ntb tags/ntb-4.8

for you to fetch changes up to 95f1464f695055c72de6044d7c8a2a7a1e0c7ea2:

  NTB: ntb_hw_intel: use local variable pdev (2016-08-05 10:34:13 -0400)


NTB bug fixes for the ntb_tool and ntb_perf, and improvements to the
ntb_perf and ntb_pingpong for increased debugability.  Also,
modification to the ntb_transport layer to increase/decrease the number
of transport entries depending on the ring size.


Allen Hubbe (2):
  NTB: ntb_hw_intel: show BAR size in debugfs info
  NTB: ntb_hw_intel: use local variable pdev

Dave Jiang (1):
  NTB: allocate number transport entries depending on size of ring size

Logan Gunthorpe (14):
  ntb_tool: Fix infinite loop bug when writing spad/peer_spad file
  ntb_tool: BUG: Ensure the buffer size is large enough to return all spads
  ntb_perf: Allow limiting the size of the memory windows
  ntb_tool: Add memory window debug support
  ntb_transport: Check the number of spads the hardware supports
  ntb_perf: Schedule based on time not on performance
  ntb_perf: Improve thread handling to increase robustness
  ntb_perf: Return results by reading the run file
  ntb_perf: Wait for link before running test
  ntb_tool: Postpone memory window initialization for the user
  ntb_tool: Add link status and files to debugfs
  ntb_pingpong: Add a debugfs file to get the ping count
  ntb_perf: clear link_is_up flag when the link goes down.
  ntb_test: Add a selftest script for the NTB subsystem

 MAINTAINERS |   1 +
 drivers/ntb/hw/intel/ntb_hw_intel.c |  49 +++-
 drivers/ntb/ntb_transport.c |  38 ++-
 drivers/ntb/test/ntb_perf.c | 240 +++--
 drivers/ntb/test/ntb_pingpong.c |  62 -
 drivers/ntb/test/ntb_tool.c | 459 +++-
 tools/testing/selftests/ntb/ntb_test.sh | 422 +
 7 files changed, 1172 insertions(+), 99 deletions(-)
 create mode 100755 tools/testing/selftests/ntb/ntb_test.sh

Re: The timer softirq on the RT kernel

[yunhong jiang]

On Fri, 05 Aug 2016 04:36:23 +0200
Mike Galbraith  wrote:

> On Thu, 2016-08-04 at 17:36 -0700, yunhong jiang wrote:
> > Hi,Mike & Steven
> > On https://lkml.org/lkml/2015/3/24/1178, the patch of
> > "timers: do not raise softirq unconditionally" is reverted. Thanks
> > for Steven's
> > detailed comments, it's quite clear explained.
> > I remember Mike has a patch trying to fix it but late
> > abandoned. Do you still have any plan to work on this? Otherwise, I
> > will have a try.
> 
> Sebastian converted the lock to raw in -rt, so nohz_full now works
> without any extra hackery.

Hi, Mike, thanks for reply and the update. I still noticed timer softirq
on my isolated CPUs, possibly something wrong on my environment. I will
re-check it.

Thanks
--jyh

> 
>   -Mike

Re: The timer softirq on the RT kernel

[yunhong jiang]

On Fri, 05 Aug 2016 04:36:23 +0200
Mike Galbraith  wrote:

> On Thu, 2016-08-04 at 17:36 -0700, yunhong jiang wrote:
> > Hi,Mike & Steven
> > On https://lkml.org/lkml/2015/3/24/1178, the patch of
> > "timers: do not raise softirq unconditionally" is reverted. Thanks
> > for Steven's
> > detailed comments, it's quite clear explained.
> > I remember Mike has a patch trying to fix it but late
> > abandoned. Do you still have any plan to work on this? Otherwise, I
> > will have a try.
> 
> Sebastian converted the lock to raw in -rt, so nohz_full now works
> without any extra hackery.

Hi, Mike, thanks for reply and the update. I still noticed timer softirq
on my isolated CPUs, possibly something wrong on my environment. I will
re-check it.

Thanks
--jyh

> 
>   -Mike

[PATCH] Staging: rtl8723au: rtw_ieee80211: Fixed operators spacing style issues

[Shiva Kerdel]

Fixed spaces around operators to fix their coding style issues.

Signed-off-by: Shiva Kerdel 
---
 drivers/staging/rtl8723au/core/rtw_ieee80211.c | 80 +-
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_ieee80211.c 
b/drivers/staging/rtl8723au/core/rtw_ieee80211.c
index 07a6490..9fa0ef1 100644
--- a/drivers/staging/rtl8723au/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8723au/core/rtw_ieee80211.c
@@ -65,7 +65,7 @@ static u8 WIFI_OFDMRATES[] = {
 
 int rtw_get_bit_value_from_ieee_value23a(u8 val)
 {
-   unsigned char dot11_rate_table[]=
+   unsigned char dot11_rate_table[] =
{2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108, 0};
 
int i = 0;
@@ -140,7 +140,7 @@ u8 *rtw_set_ie23a(u8 *pbuf, int index, uint len, const u8 
*source, uint *frlen)
return pbuf + len + 2;
 }
 
-inline u8 *rtw_set_ie23a_ch_switch (u8 *buf, u32 *buf_len, u8 ch_switch_mode,
+inline u8 *rtw_set_ie23a_ch_switch(u8 *buf, u32 *buf_len, u8 ch_switch_mode,
u8 new_ch, u8 ch_switch_cnt)
 {
u8 ie_data[3];
@@ -230,14 +230,14 @@ u8 *rtw_get_ie23a_ex(u8 *in_ie, uint in_len, u8 eid, u8 
*oui, u8 oui_len,
 
while (cnt < in_len) {
if (eid == in_ie[cnt] &&
-   (!oui || !memcmp(_ie[cnt+2], oui, oui_len))) {
+   (!oui || !memcmp(_ie[cnt + 2], oui, oui_len))) {
target_ie = _ie[cnt];
 
if (ie)
-   memcpy(ie, _ie[cnt], in_ie[cnt+1]+2);
+   memcpy(ie, _ie[cnt], in_ie[cnt + 1] + 2);
 
if (ielen)
-   *ielen = in_ie[cnt+1]+2;
+   *ielen = in_ie[cnt + 1] + 2;
break;
} else {
cnt += in_ie[cnt + 1] + 2; /* goto next */
@@ -331,7 +331,7 @@ uint rtw_get_rateset_len23a(u8 *rateset)
 {
uint i = 0;
 
-   while(1) {
+   while (1) {
if (rateset[i] == 0)
break;
 
@@ -378,7 +378,7 @@ int rtw_generate_ie23a(struct registry_priv *pregistrypriv)
wireless_mode = pregistrypriv->wireless_mode;
}
 
-   rtw_set_supported_rate23a(pdev_network->SupportedRates, wireless_mode) ;
+   rtw_set_supported_rate23a(pdev_network->SupportedRates, wireless_mode);
 
rateLen = rtw_get_rateset_len23a(pdev_network->SupportedRates);
 
@@ -533,7 +533,7 @@ int rtw_parse_wpa2_ie23a(const u8 *rsn_ie, int rsn_ie_len, 
int *group_cipher,
return _FAIL;
}
 
-   if (*rsn_ie != WLAN_EID_RSN || *(rsn_ie+1) != (u8)(rsn_ie_len - 2)) {
+   if (*rsn_ie != WLAN_EID_RSN || *(rsn_ie + 1) != (u8)(rsn_ie_len - 2)) {
return _FAIL;
}
 
@@ -791,64 +791,64 @@ u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 
short_GI_20, u8 short_GI_40,
 
if (rf_type == RF_1T1R) {
if (mcs->rx_mask[0] & BIT(7))
-   max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350):
-   ((short_GI_20)?722:650);
+   max_rate = (bw_40MHz) ? ((short_GI_40) ? 1500 : 1350) :
+   ((short_GI_20) ? 722 : 650);
else if (mcs->rx_mask[0] & BIT(6))
-   max_rate = (bw_40MHz) ? ((short_GI_40)?1350:1215):
-   ((short_GI_20)?650:585);
+   max_rate = (bw_40MHz) ? ((short_GI_40) ? 1350 : 1215) :
+   ((short_GI_20) ? 650 : 585);
else if (mcs->rx_mask[0] & BIT(5))
-   max_rate = (bw_40MHz) ? ((short_GI_40)?1200:1080):
-   ((short_GI_20)?578:520);
+   max_rate = (bw_40MHz) ? ((short_GI_40) ? 1200 : 1080) :
+   ((short_GI_20) ? 578 : 520);
else if (mcs->rx_mask[0] & BIT(4))
-   max_rate = (bw_40MHz) ? ((short_GI_40)?900:810):
-   ((short_GI_20)?433:390);
+   max_rate = (bw_40MHz) ? ((short_GI_40) ? 900 : 810) :
+   ((short_GI_20) ? 433 : 390);
else if (mcs->rx_mask[0] & BIT(3))
-   max_rate = (bw_40MHz) ? ((short_GI_40)?600:540):
-   ((short_GI_20)?289:260);
+   max_rate = (bw_40MHz) ? ((short_GI_40) ? 600 : 540) :
+   ((short_GI_20) ? 289 : 260);
else if (mcs->rx_mask[0] & BIT(2))
-   max_rate = (bw_40MHz) ? ((short_GI_40)?450:405):
-   ((short_GI_20)?217:195);
+   max_rate = (bw_40MHz) ? ((short_GI_40) ? 450 : 405) :
+   ((short_GI_20) ? 217 : 195);
else if (mcs->rx_mask[0] & 

[PATCH] Staging: rtl8723au: rtw_ieee80211: Fixed operators spacing style issues

[Shiva Kerdel]

Fixed spaces around operators to fix their coding style issues.

Signed-off-by: Shiva Kerdel 
---
 drivers/staging/rtl8723au/core/rtw_ieee80211.c | 80 +-
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_ieee80211.c 
b/drivers/staging/rtl8723au/core/rtw_ieee80211.c
index 07a6490..9fa0ef1 100644
--- a/drivers/staging/rtl8723au/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8723au/core/rtw_ieee80211.c
@@ -65,7 +65,7 @@ static u8 WIFI_OFDMRATES[] = {
 
 int rtw_get_bit_value_from_ieee_value23a(u8 val)
 {
-   unsigned char dot11_rate_table[]=
+   unsigned char dot11_rate_table[] =
{2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108, 0};
 
int i = 0;
@@ -140,7 +140,7 @@ u8 *rtw_set_ie23a(u8 *pbuf, int index, uint len, const u8 
*source, uint *frlen)
return pbuf + len + 2;
 }
 
-inline u8 *rtw_set_ie23a_ch_switch (u8 *buf, u32 *buf_len, u8 ch_switch_mode,
+inline u8 *rtw_set_ie23a_ch_switch(u8 *buf, u32 *buf_len, u8 ch_switch_mode,
u8 new_ch, u8 ch_switch_cnt)
 {
u8 ie_data[3];
@@ -230,14 +230,14 @@ u8 *rtw_get_ie23a_ex(u8 *in_ie, uint in_len, u8 eid, u8 
*oui, u8 oui_len,
 
while (cnt < in_len) {
if (eid == in_ie[cnt] &&
-   (!oui || !memcmp(_ie[cnt+2], oui, oui_len))) {
+   (!oui || !memcmp(_ie[cnt + 2], oui, oui_len))) {
target_ie = _ie[cnt];
 
if (ie)
-   memcpy(ie, _ie[cnt], in_ie[cnt+1]+2);
+   memcpy(ie, _ie[cnt], in_ie[cnt + 1] + 2);
 
if (ielen)
-   *ielen = in_ie[cnt+1]+2;
+   *ielen = in_ie[cnt + 1] + 2;
break;
} else {
cnt += in_ie[cnt + 1] + 2; /* goto next */
@@ -331,7 +331,7 @@ uint rtw_get_rateset_len23a(u8 *rateset)
 {
uint i = 0;
 
-   while(1) {
+   while (1) {
if (rateset[i] == 0)
break;
 
@@ -378,7 +378,7 @@ int rtw_generate_ie23a(struct registry_priv *pregistrypriv)
wireless_mode = pregistrypriv->wireless_mode;
}
 
-   rtw_set_supported_rate23a(pdev_network->SupportedRates, wireless_mode) ;
+   rtw_set_supported_rate23a(pdev_network->SupportedRates, wireless_mode);
 
rateLen = rtw_get_rateset_len23a(pdev_network->SupportedRates);
 
@@ -533,7 +533,7 @@ int rtw_parse_wpa2_ie23a(const u8 *rsn_ie, int rsn_ie_len, 
int *group_cipher,
return _FAIL;
}
 
-   if (*rsn_ie != WLAN_EID_RSN || *(rsn_ie+1) != (u8)(rsn_ie_len - 2)) {
+   if (*rsn_ie != WLAN_EID_RSN || *(rsn_ie + 1) != (u8)(rsn_ie_len - 2)) {
return _FAIL;
}
 
@@ -791,64 +791,64 @@ u16 rtw_mcs_rate23a(u8 rf_type, u8 bw_40MHz, u8 
short_GI_20, u8 short_GI_40,
 
if (rf_type == RF_1T1R) {
if (mcs->rx_mask[0] & BIT(7))
-   max_rate = (bw_40MHz) ? ((short_GI_40)?1500:1350):
-   ((short_GI_20)?722:650);
+   max_rate = (bw_40MHz) ? ((short_GI_40) ? 1500 : 1350) :
+   ((short_GI_20) ? 722 : 650);
else if (mcs->rx_mask[0] & BIT(6))
-   max_rate = (bw_40MHz) ? ((short_GI_40)?1350:1215):
-   ((short_GI_20)?650:585);
+   max_rate = (bw_40MHz) ? ((short_GI_40) ? 1350 : 1215) :
+   ((short_GI_20) ? 650 : 585);
else if (mcs->rx_mask[0] & BIT(5))
-   max_rate = (bw_40MHz) ? ((short_GI_40)?1200:1080):
-   ((short_GI_20)?578:520);
+   max_rate = (bw_40MHz) ? ((short_GI_40) ? 1200 : 1080) :
+   ((short_GI_20) ? 578 : 520);
else if (mcs->rx_mask[0] & BIT(4))
-   max_rate = (bw_40MHz) ? ((short_GI_40)?900:810):
-   ((short_GI_20)?433:390);
+   max_rate = (bw_40MHz) ? ((short_GI_40) ? 900 : 810) :
+   ((short_GI_20) ? 433 : 390);
else if (mcs->rx_mask[0] & BIT(3))
-   max_rate = (bw_40MHz) ? ((short_GI_40)?600:540):
-   ((short_GI_20)?289:260);
+   max_rate = (bw_40MHz) ? ((short_GI_40) ? 600 : 540) :
+   ((short_GI_20) ? 289 : 260);
else if (mcs->rx_mask[0] & BIT(2))
-   max_rate = (bw_40MHz) ? ((short_GI_40)?450:405):
-   ((short_GI_20)?217:195);
+   max_rate = (bw_40MHz) ? ((short_GI_40) ? 450 : 405) :
+   ((short_GI_20) ? 217 : 195);
else if (mcs->rx_mask[0] & BIT(1))
- 

Re: [PATCH] Fix sysrq emergency thaw

[Pavel Machek]

On Wed 2016-07-20 16:11:16, Charles Gong wrote:
> "SYSRQ + J" triggers a call to emergency_thaw_all(). Currently, this
> is an infinite loop. Once we trigger it, we'll need to do a hard
> power-cycle. There are users reporting this bug from 2012 to 2016, for
> example, at https://bugzilla.kernel.org/show_bug.cgi?id=47741.
> 
> This happens because thaw_bdev() fails to return -EINVAL in the
> non-frozen case, so fix it so that do_one_thaw() can recognize this case
> and quit from looping. I checked that none of the other thaw_bdev()
> callers check the return value.
> 
> The regression was introduced in commit 4504230a7156 ("freeze_bdev: grab
> active reference to frozen superblocks").
> 
> Reviewed-by: Chris Mason 
> Signed-off-by: Charles Gong 

Acked-by: Pavel Machek 

(even magic should be finite...)
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Fix sysrq emergency thaw

[Pavel Machek]

On Wed 2016-07-20 16:11:16, Charles Gong wrote:
> "SYSRQ + J" triggers a call to emergency_thaw_all(). Currently, this
> is an infinite loop. Once we trigger it, we'll need to do a hard
> power-cycle. There are users reporting this bug from 2012 to 2016, for
> example, at https://bugzilla.kernel.org/show_bug.cgi?id=47741.
> 
> This happens because thaw_bdev() fails to return -EINVAL in the
> non-frozen case, so fix it so that do_one_thaw() can recognize this case
> and quit from looping. I checked that none of the other thaw_bdev()
> callers check the return value.
> 
> The regression was introduced in commit 4504230a7156 ("freeze_bdev: grab
> active reference to frozen superblocks").
> 
> Reviewed-by: Chris Mason 
> Signed-off-by: Charles Gong 

Acked-by: Pavel Machek 

(even magic should be finite...)
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 1/1] RDS: add __printf format attribute to error reporting functions

[Nicolas Iooss]

This is helpful to detect at compile-time errors related to format
strings.

Signed-off-by: Nicolas Iooss 
---
 net/rds/ib.h  | 1 +
 net/rds/rds.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/net/rds/ib.h b/net/rds/ib.h
index 046f7508c06b..45ac8e8e58f4 100644
--- a/net/rds/ib.h
+++ b/net/rds/ib.h
@@ -333,6 +333,7 @@ void rds_ib_conn_path_shutdown(struct rds_conn_path *cp);
 void rds_ib_state_change(struct sock *sk);
 int rds_ib_listen_init(void);
 void rds_ib_listen_stop(void);
+__printf(2, 3)
 void __rds_ib_conn_error(struct rds_connection *conn, const char *, ...);
 int rds_ib_cm_handle_connect(struct rdma_cm_id *cm_id,
 struct rdma_cm_event *event);
diff --git a/net/rds/rds.h b/net/rds/rds.h
index b2d17f0fafa8..fd0bccb2f9f9 100644
--- a/net/rds/rds.h
+++ b/net/rds/rds.h
@@ -688,6 +688,7 @@ void __rds_conn_error(struct rds_connection *conn, const 
char *, ...);
 #define rds_conn_error(conn, fmt...) \
__rds_conn_error(conn, KERN_WARNING "RDS: " fmt)
 
+__printf(2, 3)
 void __rds_conn_path_error(struct rds_conn_path *cp, const char *, ...);
 #define rds_conn_path_error(cp, fmt...) \
__rds_conn_path_error(cp, KERN_WARNING "RDS: " fmt)
-- 
2.9.2

[PATCH 1/1] RDS: add __printf format attribute to error reporting functions

[Nicolas Iooss]

This is helpful to detect at compile-time errors related to format
strings.

Signed-off-by: Nicolas Iooss 
---
 net/rds/ib.h  | 1 +
 net/rds/rds.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/net/rds/ib.h b/net/rds/ib.h
index 046f7508c06b..45ac8e8e58f4 100644
--- a/net/rds/ib.h
+++ b/net/rds/ib.h
@@ -333,6 +333,7 @@ void rds_ib_conn_path_shutdown(struct rds_conn_path *cp);
 void rds_ib_state_change(struct sock *sk);
 int rds_ib_listen_init(void);
 void rds_ib_listen_stop(void);
+__printf(2, 3)
 void __rds_ib_conn_error(struct rds_connection *conn, const char *, ...);
 int rds_ib_cm_handle_connect(struct rdma_cm_id *cm_id,
 struct rdma_cm_event *event);
diff --git a/net/rds/rds.h b/net/rds/rds.h
index b2d17f0fafa8..fd0bccb2f9f9 100644
--- a/net/rds/rds.h
+++ b/net/rds/rds.h
@@ -688,6 +688,7 @@ void __rds_conn_error(struct rds_connection *conn, const 
char *, ...);
 #define rds_conn_error(conn, fmt...) \
__rds_conn_error(conn, KERN_WARNING "RDS: " fmt)
 
+__printf(2, 3)
 void __rds_conn_path_error(struct rds_conn_path *cp, const char *, ...);
 #define rds_conn_path_error(cp, fmt...) \
__rds_conn_path_error(cp, KERN_WARNING "RDS: " fmt)
-- 
2.9.2

Re: [PATCH v5 4/8] char: rpmb: provide a user space interface

[Pavel Machek]

On Mon 2016-07-18 23:27:49, Tomas Winkler wrote:
> The user space API is achieved via two synchronous IOCTL.

IOCTLs?

> Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is performed
> by the framework on behalf the user and second, RPMB_IOC_SEQ_CMD where
> the whole RPMB sequence including RESULT_READ is supplied by the caller.
> The latter is intended for  easier adjusting  of the  applications that
> use MMC_IOC_MULTI_CMD ioctl.

Why "  "?

> 
> Signed-off-by: Tomas Winkler 

> +
> +static long rpmb_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> +{
> + return __rpmb_ioctl(fp, cmd, (void __user *)arg);
> +}
> +
> +#ifdef CONFIG_COMPAT
> +static long rpmb_compat_ioctl(struct file *fp, unsigned int cmd,
> +   unsigned long arg)
> +{
> + return  __rpmb_ioctl(fp, cmd, compat_ptr(arg));
> +}
> +#endif /* CONFIG_COMPAT */

Description of the ioctl is missing, and it should certainly be
designed in a way that it does not need compat support.


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH v5 4/8] char: rpmb: provide a user space interface

[Pavel Machek]

On Mon 2016-07-18 23:27:49, Tomas Winkler wrote:
> The user space API is achieved via two synchronous IOCTL.

IOCTLs?

> Simplified one, RPMB_IOC_REQ_CMD, were read result cycles is performed
> by the framework on behalf the user and second, RPMB_IOC_SEQ_CMD where
> the whole RPMB sequence including RESULT_READ is supplied by the caller.
> The latter is intended for  easier adjusting  of the  applications that
> use MMC_IOC_MULTI_CMD ioctl.

Why "  "?

> 
> Signed-off-by: Tomas Winkler 

> +
> +static long rpmb_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> +{
> + return __rpmb_ioctl(fp, cmd, (void __user *)arg);
> +}
> +
> +#ifdef CONFIG_COMPAT
> +static long rpmb_compat_ioctl(struct file *fp, unsigned int cmd,
> +   unsigned long arg)
> +{
> + return  __rpmb_ioctl(fp, cmd, compat_ptr(arg));
> +}
> +#endif /* CONFIG_COMPAT */

Description of the ioctl is missing, and it should certainly be
designed in a way that it does not need compat support.


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH v5 0/8] Replay Protected Memory Block (RPMB) subsystem

[Pavel Machek]

Hi!

> Few storage technologies such is EMMC, UFS, and NVMe support RPMB
> hardware partition with common protocol and frame layout.
> The RPMB partition cannot be accessed via standard block layer, but by a
> set of specific commands: WRITE, READ, GET_WRITE_COUNTER, and
> PROGRAM_KEY.
> Such a partition provides authenticated and replay protected access,
> hence suitable as a secure storage.

...and that is suitable from locking devices from their owners, as
Nokia N9 (aka brick, because Microsoft turned off support servers)
teached me recently.

So I have to ask -- what are non-evil uses for this?

There were "secure extensions" mentioned before, but my understanding
is that it currently has severe limitations making it unsuitable for
mainline kernel. (IOW you can't event test the functionality if you
are not Intel).
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH v5 0/8] Replay Protected Memory Block (RPMB) subsystem

[Pavel Machek]

Hi!

> Few storage technologies such is EMMC, UFS, and NVMe support RPMB
> hardware partition with common protocol and frame layout.
> The RPMB partition cannot be accessed via standard block layer, but by a
> set of specific commands: WRITE, READ, GET_WRITE_COUNTER, and
> PROGRAM_KEY.
> Such a partition provides authenticated and replay protected access,
> hence suitable as a secure storage.

...and that is suitable from locking devices from their owners, as
Nokia N9 (aka brick, because Microsoft turned off support servers)
teached me recently.

So I have to ask -- what are non-evil uses for this?

There were "secure extensions" mentioned before, but my understanding
is that it currently has severe limitations making it unsuitable for
mainline kernel. (IOW you can't event test the functionality if you
are not Intel).
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC] User-defined leds

[Pavel Machek]

On Fri 2016-08-05 14:54:33, David Lechner wrote:
> On 08/05/2016 02:51 PM, Pavel Machek wrote:
> >
> >Could the device tree be used to bind LED driver to otherwise unused
> >gpio?
> 
> There is already a leds-gpio driver that does this.
> 
> https://www.kernel.org/doc/Documentation/devicetree/bindings/leds/leds-gpio.txt

Yeah, I know, that's why I suggested it :-).

What might be missing: simple way to attach leds-gpio driver to
hardware on non-devicetree machines, and perhaps teaching leds-gpio
special gpio value "no gpio" meaning ... that hardware is not actually
updated.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC] User-defined leds

[Pavel Machek]

On Fri 2016-08-05 14:54:33, David Lechner wrote:
> On 08/05/2016 02:51 PM, Pavel Machek wrote:
> >
> >Could the device tree be used to bind LED driver to otherwise unused
> >gpio?
> 
> There is already a leds-gpio driver that does this.
> 
> https://www.kernel.org/doc/Documentation/devicetree/bindings/leds/leds-gpio.txt

Yeah, I know, that's why I suggested it :-).

What might be missing: simple way to attach leds-gpio driver to
hardware on non-devicetree machines, and perhaps teaching leds-gpio
special gpio value "no gpio" meaning ... that hardware is not actually
updated.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC] User-defined leds

[David Lechner]

On 08/05/2016 02:51 PM, Pavel Machek wrote:


Could the device tree be used to bind LED driver to otherwise unused
gpio?

Pavel



There is already a leds-gpio driver that does this.

https://www.kernel.org/doc/Documentation/devicetree/bindings/leds/leds-gpio.txt

Re: [RFC] User-defined leds

[David Lechner]

On 08/05/2016 02:51 PM, Pavel Machek wrote:


Could the device tree be used to bind LED driver to otherwise unused
gpio?

Pavel



There is already a leds-gpio driver that does this.

https://www.kernel.org/doc/Documentation/devicetree/bindings/leds/leds-gpio.txt

Re: [RFC] User-defined leds

[Pavel Machek]

Hi!

> > short version: I have a use case for leds where I want to be able to use 
> > the triggers in the leds subsystem without having a physical hardware led.
> > 
> > long version: I am working on a program to make one embedded system 
> > (http://fatcatlab.com/product/evb/) compatible with another 
> > (http://mindstorms.lego.com). One has physical red/green LEDs, that use the 
> > gpio leds driver to control them. The other system does not have physical 
> > leds. However, it does have a color screen. So, my idea is
> > to create virtual LEDs on the screen that emulate the physical LEDs on the 
> > other device.
> > 
> > I would like to make a userspace program that works the same on both 
> > devices. If the leds were simple on-off, then it would of course be simpler 
> > to make the virtual leds completely in userspace. However, we are currently 
> > using other triggers (disk activity/heatbeat/etc.) with the leds. I would 
> > like for the virtual LEDs to be able to use these triggers as well.
> 
> this is funny since I have just written a ledsim.c using debugfs to emulate a 
> LED and read out its state via a simple cat command. I found that useful for 
> testing triggers in the Bluetooth subsystem and see if they behave correctly 
> without bothering to run this on real hardware.
> 

Could the device tree be used to bind LED driver to otherwise unused
gpio?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC] User-defined leds

[Pavel Machek]

Hi!

> > short version: I have a use case for leds where I want to be able to use 
> > the triggers in the leds subsystem without having a physical hardware led.
> > 
> > long version: I am working on a program to make one embedded system 
> > (http://fatcatlab.com/product/evb/) compatible with another 
> > (http://mindstorms.lego.com). One has physical red/green LEDs, that use the 
> > gpio leds driver to control them. The other system does not have physical 
> > leds. However, it does have a color screen. So, my idea is
> > to create virtual LEDs on the screen that emulate the physical LEDs on the 
> > other device.
> > 
> > I would like to make a userspace program that works the same on both 
> > devices. If the leds were simple on-off, then it would of course be simpler 
> > to make the virtual leds completely in userspace. However, we are currently 
> > using other triggers (disk activity/heatbeat/etc.) with the leds. I would 
> > like for the virtual LEDs to be able to use these triggers as well.
> 
> this is funny since I have just written a ledsim.c using debugfs to emulate a 
> LED and read out its state via a simple cat command. I found that useful for 
> testing triggers in the Bluetooth subsystem and see if they behave correctly 
> without bothering to run this on real hardware.
> 

Could the device tree be used to bind LED driver to otherwise unused
gpio?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH v2 1/3] perf/core: Add a tracepoint for perf sampling

[Alexei Starovoitov]

On Fri, Aug 05, 2016 at 12:52:09PM +0200, Peter Zijlstra wrote:
> > > > Currently overflow_handler is set at event alloc time. If we start
> > > > changing it on the fly with atomic xchg(), afaik things shouldn't
> > > > break, since each overflow_handler is run to completion and doesn't
> > > > change global state, right?
> 
> Yes, or even a simple WRITE_ONCE() to replace it, as long as we make
> sure to use a READ_ONCE() to load the pointer.
> 
> As long as we're sure to limit this poking to a single user its fairly
> simple to get right. The moment there can be concurrency a lot of fail
> can happen.

agreed.

> > So instead of normal __perf_event_output() writing into ringbuffer,
> > a bpf prog will be called that can optionally write into different
> > rb via bpf_perf_event_output. 
> 
> It could even chain and call into the original function once its done
> and have both outputs.

interesting idea. makes sense.
Also thinking about concurrency and the need to remember the original
handler somewhere, would it be cleaner api to add a bit to perf_event_attr
and use attr.config1 as bpf_fd ?
Then perf_event_open at event creation time will use bpf prog as
overflow_handler. That solves concurrency concerns and potential semantical
issues if we go with ioctl() approach.
Like if we perf_event_open() an event for a task, then bpf attach to it,
what children task and corresponding inherited events suppose to do?
Inherit overflow_handler, right? but then deatch of bpf in the parent
suppose to clear it in inherited events as well. A bit complicated.
I guess we can define it that way.
Just seems easier to do bpf attach at perf_event_open time only.

> > The question is what to pass into the
> > program to make the most use out of it. 'struct pt_regs' is done deal.
> > but perf_sample_data we cannot pass as-is, since it's kernel internal.
> 
> Urgh, does it have to be stable API? Can't we simply rely on the kernel
> headers to provide the right structure definition?

yes we can. The concern is about assumptions people will make about
perf_sample_data and the speed of access to it. From bpf program point
of view the pointer to perf_sample_data will be opaque unsafe pointer,
so any access to fields would have to be done via bpf_probe_read which
has non-trivial overhead.
If we go with the uapi mirror of perf_sample_data approach, it will be
fast, since mirror is not an actual struct. Like the 'struct __sk_buff' we
have in uapi/linux/bpf.h is a meta structure. It's not allocated anywhere
and no fields are copied. When bpf program does 'skb->vlan_present'
the verifier rewrites it at load time into corresponding access to
kernel internal 'struct sk_buff' fields with bitmask, shifts and such.
For this case we can define something like
struct bpf_perf_sample_data {
  __u64 period;
};
then bpf prog will only be able to access that signle field which verifier
will translate into 'data->period' where data is 'struct perf_sample_data *'
Later we can add other fields if necessary. The kernel is free to mess
around with perf_sample_data whichever way without impacting bpf progs.

Re: [PATCH v2 1/3] perf/core: Add a tracepoint for perf sampling

[Alexei Starovoitov]

On Fri, Aug 05, 2016 at 12:52:09PM +0200, Peter Zijlstra wrote:
> > > > Currently overflow_handler is set at event alloc time. If we start
> > > > changing it on the fly with atomic xchg(), afaik things shouldn't
> > > > break, since each overflow_handler is run to completion and doesn't
> > > > change global state, right?
> 
> Yes, or even a simple WRITE_ONCE() to replace it, as long as we make
> sure to use a READ_ONCE() to load the pointer.
> 
> As long as we're sure to limit this poking to a single user its fairly
> simple to get right. The moment there can be concurrency a lot of fail
> can happen.

agreed.

> > So instead of normal __perf_event_output() writing into ringbuffer,
> > a bpf prog will be called that can optionally write into different
> > rb via bpf_perf_event_output. 
> 
> It could even chain and call into the original function once its done
> and have both outputs.

interesting idea. makes sense.
Also thinking about concurrency and the need to remember the original
handler somewhere, would it be cleaner api to add a bit to perf_event_attr
and use attr.config1 as bpf_fd ?
Then perf_event_open at event creation time will use bpf prog as
overflow_handler. That solves concurrency concerns and potential semantical
issues if we go with ioctl() approach.
Like if we perf_event_open() an event for a task, then bpf attach to it,
what children task and corresponding inherited events suppose to do?
Inherit overflow_handler, right? but then deatch of bpf in the parent
suppose to clear it in inherited events as well. A bit complicated.
I guess we can define it that way.
Just seems easier to do bpf attach at perf_event_open time only.

> > The question is what to pass into the
> > program to make the most use out of it. 'struct pt_regs' is done deal.
> > but perf_sample_data we cannot pass as-is, since it's kernel internal.
> 
> Urgh, does it have to be stable API? Can't we simply rely on the kernel
> headers to provide the right structure definition?

yes we can. The concern is about assumptions people will make about
perf_sample_data and the speed of access to it. From bpf program point
of view the pointer to perf_sample_data will be opaque unsafe pointer,
so any access to fields would have to be done via bpf_probe_read which
has non-trivial overhead.
If we go with the uapi mirror of perf_sample_data approach, it will be
fast, since mirror is not an actual struct. Like the 'struct __sk_buff' we
have in uapi/linux/bpf.h is a meta structure. It's not allocated anywhere
and no fields are copied. When bpf program does 'skb->vlan_present'
the verifier rewrites it at load time into corresponding access to
kernel internal 'struct sk_buff' fields with bitmask, shifts and such.
For this case we can define something like
struct bpf_perf_sample_data {
  __u64 period;
};
then bpf prog will only be able to access that signle field which verifier
will translate into 'data->period' where data is 'struct perf_sample_data *'
Later we can add other fields if necessary. The kernel is free to mess
around with perf_sample_data whichever way without impacting bpf progs.

Re: [GIT PULL] Please pull NFS client updates for 4.8

[Pavel Machek]

On Sat 2016-07-30 16:32:32, Linus Torvalds wrote:
> On Sat, Jul 30, 2016 at 3:36 PM, Trond Myklebust
>  wrote:
> >
> >   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git tags/nfs-for-4.8-1
> 
> Hmm. That machine is being very very slow. It's responding to pings,
> but the whole "git pull" thing is not making any real progress.

Well, maybe it stores the git trees on nfs? :-).

SCNR,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [GIT PULL] Please pull NFS client updates for 4.8

[Pavel Machek]

On Sat 2016-07-30 16:32:32, Linus Torvalds wrote:
> On Sat, Jul 30, 2016 at 3:36 PM, Trond Myklebust
>  wrote:
> >
> >   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git tags/nfs-for-4.8-1
> 
> Hmm. That machine is being very very slow. It's responding to pings,
> but the whole "git pull" thing is not making any real progress.

Well, maybe it stores the git trees on nfs? :-).

SCNR,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

NVME regression in all kernels after 4.4.x for NVME in M2 slot for laptop?

[Marc MERLIN]

I've been stuck on 4.4.x for a while (currently 4.4.5) because any
subsequent kernel would fail to suspend or resume (S3 sleep) on my
Thinkpad P70.

Due to lack of time, I only got around to doing a git bisect now
(sorry), and did it between 4.4.0 and 4.5.0
It's my first bisect, but I hope I did it right outside of the fact that
my kernel wasn't exactly the same each time due to having my .config
file change depending on which kernel I ended up on.

However, the patch found by bisect makes sense that it would be a good
culprit.
I use an NVME 512GB SSD in my laptop, and I guess very few people use those
which could be why I'm the first/only person to report this.

Sadly because NVME changed a lot between 4.4 and 4.5 and I'm not a
kernel hacker, I can't just reverse apply the patch to 4.5 and see if it
works because I'd have to unroll a bunch of other changes too, and
that's a bit beyond my expertise and time at hand right now.

Would this patch make sense as being the reason why I can't S3 sleep
anymore and would you have a test patch against 4.5, 4.6, or 4.7 I can
try to see if it fixes the problem?
Symptom is that my red LED (the dot for in in thinkpad on the back
cover) goes flashing in weird ways when I shut the lid, but not always
the same pattern, however none are the normal on/off gentle pulsing that
indicate proper S3 sleep.
The caps lock key LED also flashes rapidly when I open the lid and the
laptop is stone dead at this point.

Boot logs on 4.4.5 kernel where sleep works fine:
[1.245549] ahci :00:17.0: version 3.0
[1.245733] ahci :00:17.0: AHCI 0001.0301 32 slots 2 ports 6 Gbps 0xc 
impl SATA mode
[1.245771] ahci :00:17.0: flags: 64bit ncq sntf pm led clo only pio 
slum part ems deso sadm sds apst 
[1.251140] scsi host0: ahci
[1.251587] scsi host1: ahci
[1.251972] scsi host2: ahci
[1.252360] scsi host3: ahci
[1.252437] ata1: DUMMY
[1.252449] ata2: DUMMY
[1.252462] ata3: SATA max UDMA/133 abar m2048@0xd584c000 port 0xd584c200 
irq 122
[1.252499] ata4: SATA max UDMA/133 abar m2048@0xd584c000 port 0xd584c280 
irq 122
[1.253374] scsi host4: pata_legacy
[1.253439] ata5: PATA max PIO4 cmd 0x1f0 ctl 0x3f6 irq 14
[1.355385]  nvme0n1: p1 p2 p3 p4 p5 p6 p7 p8
[1.570804] ata3: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[1.570877] ata4: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[1.573097] ata3.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[1.573101] ata3.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) 
filtered out
[1.573690] ata3.00: supports DRM functions and may not be fully accessible
[1.574399] ata3.00: disabling queued TRIM support
[1.574402] ata3.00: ATA-9: Samsung SSD 850 EVO 2TB, EMT01B6Q, max UDMA/133
[1.574435] ata3.00: 3907029168 sectors, multi 1: LBA48 NCQ (depth 31/32), AA
[1.575954] ata3.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[1.575958] ata3.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) 
filtered out
[1.576550] ata3.00: supports DRM functions and may not be fully accessible
[1.577209] ata3.00: disabling queued TRIM support
[1.578007] ata3.00: configured for UDMA/133
[1.578037] ata4.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[1.578040] ata4.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) 
filtered out


Patch found by bisect, attached

Thanks,
Marc
-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems 
   what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/ | PGP 1024R/763BE901
saruman:/usr/src/linux# git bisect good
25646264e15af96c5c630fc742708b1eb3339222 is the first bad commit
commit 25646264e15af96c5c630fc742708b1eb3339222
Author: Keith Busch 
Date:   Mon Jan 4 09:10:57 2016 -0700

NVMe: Remove queue freezing on resets

NVMe submits all commands through the block layer now. This means we
can let requests queue at the blk-mq hardware context since there is no
path that bypasses this anymore so we don't need to freeze the queues
anymore. The driver can simply stop the h/w queues from running during
a reset instead.

This also fixes a WARN in percpu_ref_reinit when the queue was unfrozen
with requeued requests.

Signed-off-by: Keith Busch 
Signed-off-by: Jens Axboe 


diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e31a256..8da4a8a 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1372,12 +1372,14 @@ out:
 	return ret;
 }
 
-void nvme_stop_queues(struct nvme_ctrl *ctrl)
+void nvme_freeze_queues(struct nvme_ctrl *ctrl)
 {
 	struct nvme_ns *ns;
 
 	mutex_lock(>namespaces_mutex);
 	list_for_each_entry(ns, >namespaces, list) {
+		blk_mq_freeze_queue_start(ns->queue);
+
 		

NVME regression in all kernels after 4.4.x for NVME in M2 slot for laptop?

[Marc MERLIN]

I've been stuck on 4.4.x for a while (currently 4.4.5) because any
subsequent kernel would fail to suspend or resume (S3 sleep) on my
Thinkpad P70.

Due to lack of time, I only got around to doing a git bisect now
(sorry), and did it between 4.4.0 and 4.5.0
It's my first bisect, but I hope I did it right outside of the fact that
my kernel wasn't exactly the same each time due to having my .config
file change depending on which kernel I ended up on.

However, the patch found by bisect makes sense that it would be a good
culprit.
I use an NVME 512GB SSD in my laptop, and I guess very few people use those
which could be why I'm the first/only person to report this.

Sadly because NVME changed a lot between 4.4 and 4.5 and I'm not a
kernel hacker, I can't just reverse apply the patch to 4.5 and see if it
works because I'd have to unroll a bunch of other changes too, and
that's a bit beyond my expertise and time at hand right now.

Would this patch make sense as being the reason why I can't S3 sleep
anymore and would you have a test patch against 4.5, 4.6, or 4.7 I can
try to see if it fixes the problem?
Symptom is that my red LED (the dot for in in thinkpad on the back
cover) goes flashing in weird ways when I shut the lid, but not always
the same pattern, however none are the normal on/off gentle pulsing that
indicate proper S3 sleep.
The caps lock key LED also flashes rapidly when I open the lid and the
laptop is stone dead at this point.

Boot logs on 4.4.5 kernel where sleep works fine:
[1.245549] ahci :00:17.0: version 3.0
[1.245733] ahci :00:17.0: AHCI 0001.0301 32 slots 2 ports 6 Gbps 0xc 
impl SATA mode
[1.245771] ahci :00:17.0: flags: 64bit ncq sntf pm led clo only pio 
slum part ems deso sadm sds apst 
[1.251140] scsi host0: ahci
[1.251587] scsi host1: ahci
[1.251972] scsi host2: ahci
[1.252360] scsi host3: ahci
[1.252437] ata1: DUMMY
[1.252449] ata2: DUMMY
[1.252462] ata3: SATA max UDMA/133 abar m2048@0xd584c000 port 0xd584c200 
irq 122
[1.252499] ata4: SATA max UDMA/133 abar m2048@0xd584c000 port 0xd584c280 
irq 122
[1.253374] scsi host4: pata_legacy
[1.253439] ata5: PATA max PIO4 cmd 0x1f0 ctl 0x3f6 irq 14
[1.355385]  nvme0n1: p1 p2 p3 p4 p5 p6 p7 p8
[1.570804] ata3: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[1.570877] ata4: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[1.573097] ata3.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[1.573101] ata3.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) 
filtered out
[1.573690] ata3.00: supports DRM functions and may not be fully accessible
[1.574399] ata3.00: disabling queued TRIM support
[1.574402] ata3.00: ATA-9: Samsung SSD 850 EVO 2TB, EMT01B6Q, max UDMA/133
[1.574435] ata3.00: 3907029168 sectors, multi 1: LBA48 NCQ (depth 31/32), AA
[1.575954] ata3.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[1.575958] ata3.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) 
filtered out
[1.576550] ata3.00: supports DRM functions and may not be fully accessible
[1.577209] ata3.00: disabling queued TRIM support
[1.578007] ata3.00: configured for UDMA/133
[1.578037] ata4.00: ACPI cmd ef/02:00:00:00:00:a0 (SET FEATURES) succeeded
[1.578040] ata4.00: ACPI cmd f5/00:00:00:00:00:a0 (SECURITY FREEZE LOCK) 
filtered out


Patch found by bisect, attached

Thanks,
Marc
-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems 
   what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/ | PGP 1024R/763BE901
saruman:/usr/src/linux# git bisect good
25646264e15af96c5c630fc742708b1eb3339222 is the first bad commit
commit 25646264e15af96c5c630fc742708b1eb3339222
Author: Keith Busch 
Date:   Mon Jan 4 09:10:57 2016 -0700

NVMe: Remove queue freezing on resets

NVMe submits all commands through the block layer now. This means we
can let requests queue at the blk-mq hardware context since there is no
path that bypasses this anymore so we don't need to freeze the queues
anymore. The driver can simply stop the h/w queues from running during
a reset instead.

This also fixes a WARN in percpu_ref_reinit when the queue was unfrozen
with requeued requests.

Signed-off-by: Keith Busch 
Signed-off-by: Jens Axboe 


diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e31a256..8da4a8a 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1372,12 +1372,14 @@ out:
 	return ret;
 }
 
-void nvme_stop_queues(struct nvme_ctrl *ctrl)
+void nvme_freeze_queues(struct nvme_ctrl *ctrl)
 {
 	struct nvme_ns *ns;
 
 	mutex_lock(>namespaces_mutex);
 	list_for_each_entry(ns, >namespaces, list) {
+		blk_mq_freeze_queue_start(ns->queue);
+
 		spin_lock_irq(ns->queue->queue_lock);
 		

Re: [PATCH v3 2/9] usb: gadget: align buffer size when allocating for OUT endpoint

[kbuild test robot]

Hi Felipe,

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

url:
https://github.com/0day-ci/linux/commits/Felipe-F-Tonello/Gadget-endpoint-request-allocation-and-MIDI/20160806-024520
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-x013-201631 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: the 
linux-review/Felipe-F-Tonello/Gadget-endpoint-request-allocation-and-MIDI/20160806-024520
 HEAD 5064a41cd5f89103e3b75c1a2ab9f6e98851503b builds fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

>> drivers/usb/gadget/u_f.c:17:21: error: conflicting types for 'alloc_ep_req'
struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int 
default_len)
^~~~
   In file included from drivers/usb/gadget/u_f.c:14:0:
   drivers/usb/gadget/u_f.h:63:21: note: previous declaration of 'alloc_ep_req' 
was here
struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len, int 
default_len);
^~~~

vim +/alloc_ep_req +17 drivers/usb/gadget/u_f.c

1efd54ea Andrzej Pietrasiewicz 2013-11-07  11   * published by the Free 
Software Foundation.
1efd54ea Andrzej Pietrasiewicz 2013-11-07  12   */
1efd54ea Andrzej Pietrasiewicz 2013-11-07  13  
1efd54ea Andrzej Pietrasiewicz 2013-11-07  14  #include "u_f.h"
a7ffc68f Felipe F. Tonello 2016-08-05  15  #include 
1efd54ea Andrzej Pietrasiewicz 2013-11-07  16  
1efd54ea Andrzej Pietrasiewicz 2013-11-07 @17  struct usb_request 
*alloc_ep_req(struct usb_ep *ep, int len, int default_len)
1efd54ea Andrzej Pietrasiewicz 2013-11-07  18  {
1efd54ea Andrzej Pietrasiewicz 2013-11-07  19   struct usb_request  *req;
1efd54ea Andrzej Pietrasiewicz 2013-11-07  20  

:: The code at line 17 was first introduced by commit
:: 1efd54eab2b60c68c2ce75ea635306cef847d751 usb: gadget: factor out 
alloc_ep_req

:: TO: Andrzej Pietrasiewicz <andrze...@samsung.com>
:: CC: Felipe Balbi <ba...@ti.com>

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


.config.gz
Description: Binary data

Re: [PATCH v3 2/9] usb: gadget: align buffer size when allocating for OUT endpoint

[kbuild test robot]

Hi Felipe,

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

url:
https://github.com/0day-ci/linux/commits/Felipe-F-Tonello/Gadget-endpoint-request-allocation-and-MIDI/20160806-024520
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-x013-201631 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: the 
linux-review/Felipe-F-Tonello/Gadget-endpoint-request-allocation-and-MIDI/20160806-024520
 HEAD 5064a41cd5f89103e3b75c1a2ab9f6e98851503b builds fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

>> drivers/usb/gadget/u_f.c:17:21: error: conflicting types for 'alloc_ep_req'
struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int 
default_len)
^~~~
   In file included from drivers/usb/gadget/u_f.c:14:0:
   drivers/usb/gadget/u_f.h:63:21: note: previous declaration of 'alloc_ep_req' 
was here
struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len, int 
default_len);
^~~~

vim +/alloc_ep_req +17 drivers/usb/gadget/u_f.c

1efd54ea Andrzej Pietrasiewicz 2013-11-07  11   * published by the Free 
Software Foundation.
1efd54ea Andrzej Pietrasiewicz 2013-11-07  12   */
1efd54ea Andrzej Pietrasiewicz 2013-11-07  13  
1efd54ea Andrzej Pietrasiewicz 2013-11-07  14  #include "u_f.h"
a7ffc68f Felipe F. Tonello 2016-08-05  15  #include 
1efd54ea Andrzej Pietrasiewicz 2013-11-07  16  
1efd54ea Andrzej Pietrasiewicz 2013-11-07 @17  struct usb_request 
*alloc_ep_req(struct usb_ep *ep, int len, int default_len)
1efd54ea Andrzej Pietrasiewicz 2013-11-07  18  {
1efd54ea Andrzej Pietrasiewicz 2013-11-07  19   struct usb_request  *req;
1efd54ea Andrzej Pietrasiewicz 2013-11-07  20  

:: The code at line 17 was first introduced by commit
:: 1efd54eab2b60c68c2ce75ea635306cef847d751 usb: gadget: factor out 
alloc_ep_req

:: TO: Andrzej Pietrasiewicz 
:: CC: Felipe Balbi 

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


.config.gz
Description: Binary data

Re: 4.7.0-rc7 ext4 error in dx_probe

[Darrick J. Wong]

On Fri, Aug 05, 2016 at 08:11:36PM +0200, Johannes Stezenbach wrote:
> On Fri, Aug 05, 2016 at 10:02:28AM -0700, Darrick J. Wong wrote:
> > On Fri, Aug 05, 2016 at 12:35:44PM +0200, Johannes Stezenbach wrote:
> > > On Wed, Aug 03, 2016 at 05:50:26PM +0300, Török Edwin wrote:
> > > > I have just encountered a similar problem after I've recently upgraded 
> > > > to 4.7.0:
> > > > [Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): dx_probe:740: 
> > > > inode #13295: comm python: Directory index failed checksum
> > > > [Wed Aug  3 11:08:57 2016] Aborting journal on device dm-1-8.
> > > > [Wed Aug  3 11:08:57 2016] EXT4-fs (dm-1): Remounting filesystem 
> > > > read-only
> > > > [Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): 
> > > > ext4_journal_check_start:56: Detected aborted journal
> > > 
> > > It just happened again to me, this time hitting /usr/sbin/
> > > on root fs.  Meanwhile I ran memtest86 7.0 for two nights,
> > > it didn't find anything.  I'm using hibernate regularly
> > > and I think so this only happened after a few hibernate/resume
> > > cycles, but no idea if that means anything.
> > > Now I'm back at 4.4.16 to see if it reproduces.
> > 
> > When you're back on 4.7, can you apply this patch[1] to see if it fixes
> > the problem?  I speculate that the new parallel dir lookup code enables
> > multiple threads to be verifying the same directory block buffer at the
> > same time.
> > 
> > [1] 
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/fs/ext4/inode.c?id=b47820edd1634dc1208f9212b7ecfb4230610a23
> 
> I added the patch, rebuilt and rebooted.  It will take some time
> before I'll report back since the issue is so hard to reproduce.

FWIW I could trigger it reliably by running a bunch of directory traversal
programs simultaneously on the same directory.  I have a script that fires
up multiple mutts pointing to the Maildirs for the high traffic Linux lists.

--D

> 
> Thanks,
> Johannes
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: 4.7.0-rc7 ext4 error in dx_probe

[Darrick J. Wong]

On Fri, Aug 05, 2016 at 08:11:36PM +0200, Johannes Stezenbach wrote:
> On Fri, Aug 05, 2016 at 10:02:28AM -0700, Darrick J. Wong wrote:
> > On Fri, Aug 05, 2016 at 12:35:44PM +0200, Johannes Stezenbach wrote:
> > > On Wed, Aug 03, 2016 at 05:50:26PM +0300, Török Edwin wrote:
> > > > I have just encountered a similar problem after I've recently upgraded 
> > > > to 4.7.0:
> > > > [Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): dx_probe:740: 
> > > > inode #13295: comm python: Directory index failed checksum
> > > > [Wed Aug  3 11:08:57 2016] Aborting journal on device dm-1-8.
> > > > [Wed Aug  3 11:08:57 2016] EXT4-fs (dm-1): Remounting filesystem 
> > > > read-only
> > > > [Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): 
> > > > ext4_journal_check_start:56: Detected aborted journal
> > > 
> > > It just happened again to me, this time hitting /usr/sbin/
> > > on root fs.  Meanwhile I ran memtest86 7.0 for two nights,
> > > it didn't find anything.  I'm using hibernate regularly
> > > and I think so this only happened after a few hibernate/resume
> > > cycles, but no idea if that means anything.
> > > Now I'm back at 4.4.16 to see if it reproduces.
> > 
> > When you're back on 4.7, can you apply this patch[1] to see if it fixes
> > the problem?  I speculate that the new parallel dir lookup code enables
> > multiple threads to be verifying the same directory block buffer at the
> > same time.
> > 
> > [1] 
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/fs/ext4/inode.c?id=b47820edd1634dc1208f9212b7ecfb4230610a23
> 
> I added the patch, rebuilt and rebooted.  It will take some time
> before I'll report back since the issue is so hard to reproduce.

FWIW I could trigger it reliably by running a bunch of directory traversal
programs simultaneously on the same directory.  I have a script that fires
up multiple mutts pointing to the Maildirs for the high traffic Linux lists.

--D

> 
> Thanks,
> Johannes
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: powerpc allyesconfig / allmodconfig linux-next next-20160729 - next-20160729 build failures

[Arnd Bergmann]

On Saturday, August 6, 2016 2:16:42 AM CEST Nicholas Piggin wrote:
> > 
> > diff --git a/include/asm-generic/vmlinux.lds.h 
> > b/include/asm-generic/vmlinux.lds.h
> > index 0ec807d69f18..7a3ad269fa23 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -433,7 +433,7 @@
> >   * during second ld run in second ld pass when generating System.map */
> >  #define TEXT_TEXT\
> >   ALIGN_FUNCTION();   \
> > - *(.text.hot .text .text.fixup .text.unlikely)   \
> > + *(.text.hot .text .text.* .text.fixup .text.unlikely)   \
> >   *(.ref.text)\
> >   MEM_KEEP(init.text) \
> >   MEM_KEEP(exit.text) \
> > 
> > 
> > It also got much faster again, the link time for an allyesconfig
> > kernel is now 18 minutes instead of 10 hours, but it's still
> > much worse than the 2 minutes I had earlier or the four minutes
> > with the previous patch.
> 
> Are you using the patches I just sent?

Not yet, I was still busy with the older version, and trying to
figure out exactly what went wrong in ld.bfd. FWIW, I first tried
to see if the hash tables were just too small, but as it turned
out that was not the problem. When I tried to change the default
hash table sizes, making them bigger only made things slower.

I also found the --hash-size=xxx option, which has a significant
impact on runtime speed. Interestingly again, using sizes less
than the default made things faster in practice. If we can
work out the optimum size for the kernel build, that might
shave a few minutes off the total build time.

> Either way, you also need
> to do the same for data and bss sections as you are using
> -fdata-sections too.

Right.

> I've found virtually no build time regression on powerpc or x86
> when those are taken care of properly (x86 numbers I sent are typo,
> it's not 5m20, it's 5m02).

Interesting. I wonder if it's got something to do with the
generation of the branch trampolines on ARM, as we have a lot
of them on an allyesconfig.

Is the 5m20 the total build time for the kernel, the time for
rebuilding after a trivial change, or the time to call 'ld.bfd'
once?

Are you using ld.bfd on x86 or ld.gold? For me ld.gold either
works and is really fast, or it crashes, depending on the
configuration. I also don't think it supports big-endian ARM
(which is what allyesconfig ends up using).

Arnd

Re: powerpc allyesconfig / allmodconfig linux-next next-20160729 - next-20160729 build failures

[Arnd Bergmann]

On Saturday, August 6, 2016 2:16:42 AM CEST Nicholas Piggin wrote:
> > 
> > diff --git a/include/asm-generic/vmlinux.lds.h 
> > b/include/asm-generic/vmlinux.lds.h
> > index 0ec807d69f18..7a3ad269fa23 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -433,7 +433,7 @@
> >   * during second ld run in second ld pass when generating System.map */
> >  #define TEXT_TEXT\
> >   ALIGN_FUNCTION();   \
> > - *(.text.hot .text .text.fixup .text.unlikely)   \
> > + *(.text.hot .text .text.* .text.fixup .text.unlikely)   \
> >   *(.ref.text)\
> >   MEM_KEEP(init.text) \
> >   MEM_KEEP(exit.text) \
> > 
> > 
> > It also got much faster again, the link time for an allyesconfig
> > kernel is now 18 minutes instead of 10 hours, but it's still
> > much worse than the 2 minutes I had earlier or the four minutes
> > with the previous patch.
> 
> Are you using the patches I just sent?

Not yet, I was still busy with the older version, and trying to
figure out exactly what went wrong in ld.bfd. FWIW, I first tried
to see if the hash tables were just too small, but as it turned
out that was not the problem. When I tried to change the default
hash table sizes, making them bigger only made things slower.

I also found the --hash-size=xxx option, which has a significant
impact on runtime speed. Interestingly again, using sizes less
than the default made things faster in practice. If we can
work out the optimum size for the kernel build, that might
shave a few minutes off the total build time.

> Either way, you also need
> to do the same for data and bss sections as you are using
> -fdata-sections too.

Right.

> I've found virtually no build time regression on powerpc or x86
> when those are taken care of properly (x86 numbers I sent are typo,
> it's not 5m20, it's 5m02).

Interesting. I wonder if it's got something to do with the
generation of the branch trampolines on ARM, as we have a lot
of them on an allyesconfig.

Is the 5m20 the total build time for the kernel, the time for
rebuilding after a trivial change, or the time to call 'ld.bfd'
once?

Are you using ld.bfd on x86 or ld.gold? For me ld.gold either
works and is really fast, or it crashes, depending on the
configuration. I also don't think it supports big-endian ARM
(which is what allyesconfig ends up using).

Arnd

Re: [PATCH 2/4] tools lib traceevent: Use USECS_PER_SEC instead of hardcoded number

[Steven Rostedt]

On Fri, 5 Aug 2016 15:36:55 -0300
Arnaldo Carvalho de Melo  wrote:


> [acme@jouet linux]$ cat tools/include/linux/time64.h 
> #ifndef _TOOLS_LINUX_TIME64_H
> #define _TOOLS_LINUX_TIME64_H
> 
> #define MSEC_PER_SEC  1000L
> #define USEC_PER_MSEC 1000L
> #define NSEC_PER_USEC 1000L
> #define NSEC_PER_MSEC 100L
> #define USEC_PER_SEC  100L
> #define NSEC_PER_SEC  10L
> #define FSEC_PER_SEC  1000LL
> 
> #endif /* _TOOLS_LINUX_TIME64_H */
> [acme@jouet linux]$ 
> 
> So the header to include is the same as in the kernel, the constants as
> well. We can go on adding more stuff from include/linux/time64.h as
> tools use it.

OK, can you modify the scripting-engines/trace-event-*.c to use that
too. I'm going to move the macros locally into event-parse.c, as I work
to make that ready to be a separate library.

Thanks!

-- Steve

Re: [PATCH 2/4] tools lib traceevent: Use USECS_PER_SEC instead of hardcoded number

[Steven Rostedt]

On Fri, 5 Aug 2016 15:36:55 -0300
Arnaldo Carvalho de Melo  wrote:


> [acme@jouet linux]$ cat tools/include/linux/time64.h 
> #ifndef _TOOLS_LINUX_TIME64_H
> #define _TOOLS_LINUX_TIME64_H
> 
> #define MSEC_PER_SEC  1000L
> #define USEC_PER_MSEC 1000L
> #define NSEC_PER_USEC 1000L
> #define NSEC_PER_MSEC 100L
> #define USEC_PER_SEC  100L
> #define NSEC_PER_SEC  10L
> #define FSEC_PER_SEC  1000LL
> 
> #endif /* _TOOLS_LINUX_TIME64_H */
> [acme@jouet linux]$ 
> 
> So the header to include is the same as in the kernel, the constants as
> well. We can go on adding more stuff from include/linux/time64.h as
> tools use it.

OK, can you modify the scripting-engines/trace-event-*.c to use that
too. I'm going to move the macros locally into event-parse.c, as I work
to make that ready to be a separate library.

Thanks!

-- Steve

Re: [PATCH 2/2] powerpc/pseries: Dynamically increase RMA size

[Paul Clarke]

Only nits from me...(see below)

On 08/05/2016 01:30 PM, Sukadev Bhattiprolu wrote:

Here is an updated patch to fix the build when CONFIG_PPC_PSERIES=n.
---
From d4f77a6ca7b6ea83f6588e7d541cc70bf001ae85 Mon Sep 17 00:00:00 2001
From: root 
Date: Thu, 4 Aug 2016 23:13:37 -0400
Subject: [PATCH 2/2] powerpc/pseries: Dynamically grow RMA size

When booting a very large system with a larg initrd we run out of space
for the flattened device tree (FDT). To fix this we must increase the
space allocated for the RMA region.

The RMA size is hard-coded in the 'ibm_architecture_vec[]' and increasing
the size there will apply to all systems, small and large, so we want to
increase the RMA region only when necessary.

When we run out of room for the FDT, set a new OF property, 'ibm,new-rma-size'
to the new RMA size (512MB) and issue a client-architecture-support (CAS)
call to the firmware. This will initiate a system reboot. Upon reboot we
notice the new property and update the RMA size accordingly.

Fix suggested by Michael Ellerman.

Signed-off-by: Sukadev Bhattiprolu 
---

[v2]:   - Add a comment in code regarding 'fixup_nr_cores_done'
- Fix build break when CONFIG_PPC_PSERIES=n
---
 arch/powerpc/kernel/prom_init.c | 96 -
 1 file changed, 95 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index f612a99..cbd5387 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -679,6 +679,7 @@ unsigned char ibm_architecture_vec[] = {
W(0x),  /* virt_base */
W(0x),  /* virt_size */
W(0x),  /* load_base */
+#define IBM_ARCH_VEC_MIN_RMA_OFFSET108
W(256), /* 256MB min RMA */
W(0x),  /* full client load */
0,  /* min RMA percentage of total RAM */
@@ -867,6 +868,14 @@ static void fixup_nr_cores(void)
 {
u32 cores;
unsigned char *ptcores;
+   static bool fixup_nr_cores_done = false;
+
+   /*
+* If this is a second CAS call in the same boot sequence, (see
+* increase_rma_size()), we don't need to do the fixup again.
+*/
+   if (fixup_nr_cores_done)
+   return;

/* We need to tell the FW about the number of cores we support.
 *
@@ -898,6 +907,41 @@ static void fixup_nr_cores(void)
ptcores[1] = (cores >> 16) & 0xff;
ptcores[2] = (cores >> 8) & 0xff;
ptcores[3] = cores & 0xff;
+   fixup_nr_cores_done = true;
+   }
+}
+
+static void __init fixup_rma_size(void)
+{
+   int rc;
+   u64 size;
+   unsigned char *min_rmap;
+   phandle optnode;
+   char str[64];
+
+   optnode = call_prom("finddevice", 1, 1, ADDR("/options"));
+   if (!PHANDLE_VALID(optnode))
+   prom_panic("Cannot find /options");
+
+   /*
+* If a prior boot specified a new RMA size, use that size in
+* ibm_architecture_vec[]. See also increase_rma_size().
+*/
+   size = 0ULL;
+   memset(str, 0, sizeof(str));
+   rc = prom_getprop(optnode, "ibm,new-rma-size", , sizeof(str));
+   if (rc <= 0)
+   return;
+
+   size = prom_strtoul(str, NULL);
+   min_rmap = _architecture_vec[IBM_ARCH_VEC_MIN_RMA_OFFSET];
+
+   if (size) {
+   prom_printf("Using RMA size %lu from ibm,new-rma-size.\n", 
size);
+   min_rmap[0] = (size >> 24) & 0xff;
+   min_rmap[1] = (size >> 16) & 0xff;
+   min_rmap[2] = (size >> 8) & 0xff;
+   min_rmap[3] = size & 0xff;
}
 }

@@ -911,6 +955,8 @@ static void __init prom_send_capabilities(void)

fixup_nr_cores();

+   fixup_rma_size();
+
/* try calling the ibm,client-architecture-support method */
prom_printf("Calling ibm,client-architecture-support...");
if (call_prom_ret("call-method", 3, 2, ,
@@ -946,6 +992,52 @@ static void __init prom_send_capabilities(void)
}
 #endif /* __BIG_ENDIAN__ */
 }
+
+static void __init increase_rma_size(void)
+{
+   int rc;
+   u64 size;
+   char str[64];
+   phandle optnode;
+
+   optnode = call_prom("finddevice", 1, 1, ADDR("/options"));
+   if (!PHANDLE_VALID(optnode))
+   prom_panic("Cannot find /options");
+
+   /*
+* If we already increased the RMA size, return.
+*/
+   size = 0ULL;
+   memset(str, 0, sizeof(str));
+   rc = prom_getprop(optnode, "ibm,new-rma-size", , sizeof(str));
+
+   size = prom_strtoul(str, NULL);
+   if (size == 512ULL) {


Is this preferred over strncmp?  Using a string also helps with my suggestion 
below...


+   prom_printf("RMA 

Re: pcieport AER error spam on Intel Skylake

[Alexander Duyck]

On Fri, Aug 5, 2016 at 11:15 AM, Daniel Drake  wrote:
> Hi Alexander,
>
> Reviving an old topic here...
>
> We are seeing this "problem" on an increasing number of units from the
> vendor, and searching around it can also be seen on Dell and HP
> products. Always with the same Realtek b723 wifi device. e.g.
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1521173
>
> The amount of error spam is problematic in that it slows down boot
> really significantly, while printing lots of scary messages for the
> user.
> We tried doing a PCI MSI blacklist for affected laptops but we are
> struggling to keep that blacklist updated with the increasing number
> of affected models.
>
> Enough hacks, I am wondering what we can do to solve this problem in
> the mainline kernel...
>
> On Thu, Sep 3, 2015 at 12:05 PM, Alexander Duyck
>  wrote:
>> On 09/03/2015 06:32 AM, Daniel Drake wrote:
>>>
>>> On Wed, Sep 2, 2015 at 7:57 PM, Alexander Duyck
>>>  wrote:

 Since it is correctable errors it is likely some sort of signalling
 issue.
 Could we get the output of something like an lspci -vt? Then you would be
 able to tell what the device is on the other side of the link from
 00:1c.5
 and then we could probably check to see if there has been any changes for
 the device driver on the other end of the link.
>>>
>>> "lspci -vt" reliably causes one occurance of the message, which is
>>> logged by the kernel before lspci has written anything to stdout.
>>>   pcieport :00:1c.5: AER: Corrected error received: id=00e5
>>>   pcieport :00:1c.5: PCIe Bus Error: severity=Corrected,
>>> type=Physical Layer, id=00e5(Receiver ID)
>>>   pcieport :00:1c.5:   device [8086:9d15] error
>>> status/mask=0001/2000
>>>   pcieport :00:1c.5:[ 0] Receiver Error
>>>
>>> -[:00]-+-00.0  Intel Corporation Device 1904
>>> +-02.0  Intel Corporation Device 1916
>>> +-04.0  Intel Corporation Device 1903
>>> +-08.0  Intel Corporation Device 1911
>>> +-14.0  Intel Corporation Device 9d2f
>>> +-14.2  Intel Corporation Device 9d31
>>> +-15.0  Intel Corporation Device 9d60
>>> +-15.1  Intel Corporation Device 9d61
>>> +-16.0  Intel Corporation Device 9d3a
>>> +-17.0  Intel Corporation Device 9d03
>>> +-1c.0-[01]--
>>> +-1c.4-[02]00.0  Realtek Semiconductor Co., Ltd.
>>> RTL8111/8168 PCI Express Gigabit Ethernet controller
>>> +-1c.5-[03]00.0  Realtek Semiconductor Co., Ltd. Device
>>> b723
>>> +-1f.0  Intel Corporation Device 9d48
>>> +-1f.2  Intel Corporation Device 9d21
>>> +-1f.3  Intel Corporation Device 9d70
>>> \-1f.4  Intel Corporation Device 9d23
>>>
>>> Does this mean these messages are somehow related to the Realtek b723
>>> device? That is the wifi card.
>>> Using x86_64_defconfig there is not even any driver loaded for this
>>> device, yet the messages appear quite a bit.
>>> If I use a full config with all the relevant drivers including
>>> rtlwifi, the frequency of these messages goes up a lot though.
>>
>>
>> The correctable errors are likely a result of some sort of link error
>> between the root port 00:1c.5 and the wireless adapter at 3:00.0.  What is
>> likely happening is that when the device is unused it transitions down to a
>> lower power link state like L0s or L1, and when it comes out of that state
>> it is likely triggering the PCIe error most likely as a result of something
>> during the PCIe link training sequence.
>>
>> You might want to notify the manufacturer of the laptop as they may need to
>> address an issue in their hardware, firmware, or possibly add  a workaround
>> to mask off Receiver Error reporting for their part via either a PCIe quirk
>> or driver fix.
>>
 My suspicion since this is a laptop is that something like a power
 management change might be responsible if this is a regression as I have
 seen messages like this pop up as a result of ASPM being enabled before.
>>>
>>> It's likely not a regression, this is brand new hardware and this
>>> message is seen on all kernels that we have tried (4.1, 4.2, master).
>>> pcie_aspm=off also makes these messages go away.
>>
>>
>> Correctable errors are considered a sign of the PCIe link health. In theory
>> they can be ignored since by definition they can be corrected by the
>> hardware.  One thing you could do if you aren't using the wireless card
>> would be to simply switch off the correctable error reporting by setting the
>> mask bit for it in configuration space using setpci.
>>
>> To do that what you could do is find the offset for the PCIe AER
>> configuration register for your port by doing a "lspci -vvv -s 0:1c.5" and
>> what you should get will be a dump listing the capabilities and their
>> current settings.  In 

Re: [PATCH resend] clocksource: Defer override invalidation unless clock is unstable

[John Stultz]

On Wed, Jul 27, 2016 at 6:50 AM, Kyle Walker  wrote:
> On Tue, Jul 26, 2016 at 5:36 PM, John Stultz  wrote:
>> The logic here is confusing as well. So.. if the override is not HRT
>> compatible, we check if its stable or not?  Once we're in HRT there's
>> not much likelyhood of us going into non HRT mode. I'm not sure what
>> the stability has to do with it here.
>>
>> Sorry, could you explain the case you're running into in some further
>> detail?
>
> The issue I'm running into is that the override is not HRT compatible yet.
> Though it will be later in the boot process, unless the clocksource watchdog
> marks the clocksource as unstable.
>
> The issue with the current implementation is that the override_name value is
> disabled when the tsc is first checked, before the watchdog has a chance to
> check it and mark it stable or unstable.

Hrm. Ok. I've missed that the setting of a clocksource to being
VALID_FOR_HRES happens by the watchdog and was thinking it was more
like the IS_CONTINUOUS flag.

So with that detail made clear, the patch makes more sense. I'd
definitely clear up the change log to explain that detail:
"Clocksources don't get the VALID_FOR_HRES flag until they have been
checked by a watchdog. However, when using an override, the
clocksource_select logic will clear the override value if the
clocksources is not marked VALID_FOR_HRES on that check. When using
the boot arguments clocksource=, this selection can run before
the watchdog, and can cause the override to be incorrectly cleared."

Sorry for the slow response, keeping up with the merge window the last
two weeks has been a little crazy.

thanks
-john

Re: [PATCH resend] clocksource: Defer override invalidation unless clock is unstable

[John Stultz]

On Wed, Jul 27, 2016 at 6:50 AM, Kyle Walker  wrote:
> On Tue, Jul 26, 2016 at 5:36 PM, John Stultz  wrote:
>> The logic here is confusing as well. So.. if the override is not HRT
>> compatible, we check if its stable or not?  Once we're in HRT there's
>> not much likelyhood of us going into non HRT mode. I'm not sure what
>> the stability has to do with it here.
>>
>> Sorry, could you explain the case you're running into in some further
>> detail?
>
> The issue I'm running into is that the override is not HRT compatible yet.
> Though it will be later in the boot process, unless the clocksource watchdog
> marks the clocksource as unstable.
>
> The issue with the current implementation is that the override_name value is
> disabled when the tsc is first checked, before the watchdog has a chance to
> check it and mark it stable or unstable.

Hrm. Ok. I've missed that the setting of a clocksource to being
VALID_FOR_HRES happens by the watchdog and was thinking it was more
like the IS_CONTINUOUS flag.

So with that detail made clear, the patch makes more sense. I'd
definitely clear up the change log to explain that detail:
"Clocksources don't get the VALID_FOR_HRES flag until they have been
checked by a watchdog. However, when using an override, the
clocksource_select logic will clear the override value if the
clocksources is not marked VALID_FOR_HRES on that check. When using
the boot arguments clocksource=, this selection can run before
the watchdog, and can cause the override to be incorrectly cleared."

Sorry for the slow response, keeping up with the merge window the last
two weeks has been a little crazy.

thanks
-john

Re: [PATCH 2/2] powerpc/pseries: Dynamically increase RMA size

[Paul Clarke]

Only nits from me...(see below)

On 08/05/2016 01:30 PM, Sukadev Bhattiprolu wrote:

Here is an updated patch to fix the build when CONFIG_PPC_PSERIES=n.
---
From d4f77a6ca7b6ea83f6588e7d541cc70bf001ae85 Mon Sep 17 00:00:00 2001
From: root 
Date: Thu, 4 Aug 2016 23:13:37 -0400
Subject: [PATCH 2/2] powerpc/pseries: Dynamically grow RMA size

When booting a very large system with a larg initrd we run out of space
for the flattened device tree (FDT). To fix this we must increase the
space allocated for the RMA region.

The RMA size is hard-coded in the 'ibm_architecture_vec[]' and increasing
the size there will apply to all systems, small and large, so we want to
increase the RMA region only when necessary.

When we run out of room for the FDT, set a new OF property, 'ibm,new-rma-size'
to the new RMA size (512MB) and issue a client-architecture-support (CAS)
call to the firmware. This will initiate a system reboot. Upon reboot we
notice the new property and update the RMA size accordingly.

Fix suggested by Michael Ellerman.

Signed-off-by: Sukadev Bhattiprolu 
---

[v2]:   - Add a comment in code regarding 'fixup_nr_cores_done'
- Fix build break when CONFIG_PPC_PSERIES=n
---
 arch/powerpc/kernel/prom_init.c | 96 -
 1 file changed, 95 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index f612a99..cbd5387 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -679,6 +679,7 @@ unsigned char ibm_architecture_vec[] = {
W(0x),  /* virt_base */
W(0x),  /* virt_size */
W(0x),  /* load_base */
+#define IBM_ARCH_VEC_MIN_RMA_OFFSET108
W(256), /* 256MB min RMA */
W(0x),  /* full client load */
0,  /* min RMA percentage of total RAM */
@@ -867,6 +868,14 @@ static void fixup_nr_cores(void)
 {
u32 cores;
unsigned char *ptcores;
+   static bool fixup_nr_cores_done = false;
+
+   /*
+* If this is a second CAS call in the same boot sequence, (see
+* increase_rma_size()), we don't need to do the fixup again.
+*/
+   if (fixup_nr_cores_done)
+   return;

/* We need to tell the FW about the number of cores we support.
 *
@@ -898,6 +907,41 @@ static void fixup_nr_cores(void)
ptcores[1] = (cores >> 16) & 0xff;
ptcores[2] = (cores >> 8) & 0xff;
ptcores[3] = cores & 0xff;
+   fixup_nr_cores_done = true;
+   }
+}
+
+static void __init fixup_rma_size(void)
+{
+   int rc;
+   u64 size;
+   unsigned char *min_rmap;
+   phandle optnode;
+   char str[64];
+
+   optnode = call_prom("finddevice", 1, 1, ADDR("/options"));
+   if (!PHANDLE_VALID(optnode))
+   prom_panic("Cannot find /options");
+
+   /*
+* If a prior boot specified a new RMA size, use that size in
+* ibm_architecture_vec[]. See also increase_rma_size().
+*/
+   size = 0ULL;
+   memset(str, 0, sizeof(str));
+   rc = prom_getprop(optnode, "ibm,new-rma-size", , sizeof(str));
+   if (rc <= 0)
+   return;
+
+   size = prom_strtoul(str, NULL);
+   min_rmap = _architecture_vec[IBM_ARCH_VEC_MIN_RMA_OFFSET];
+
+   if (size) {
+   prom_printf("Using RMA size %lu from ibm,new-rma-size.\n", 
size);
+   min_rmap[0] = (size >> 24) & 0xff;
+   min_rmap[1] = (size >> 16) & 0xff;
+   min_rmap[2] = (size >> 8) & 0xff;
+   min_rmap[3] = size & 0xff;
}
 }

@@ -911,6 +955,8 @@ static void __init prom_send_capabilities(void)

fixup_nr_cores();

+   fixup_rma_size();
+
/* try calling the ibm,client-architecture-support method */
prom_printf("Calling ibm,client-architecture-support...");
if (call_prom_ret("call-method", 3, 2, ,
@@ -946,6 +992,52 @@ static void __init prom_send_capabilities(void)
}
 #endif /* __BIG_ENDIAN__ */
 }
+
+static void __init increase_rma_size(void)
+{
+   int rc;
+   u64 size;
+   char str[64];
+   phandle optnode;
+
+   optnode = call_prom("finddevice", 1, 1, ADDR("/options"));
+   if (!PHANDLE_VALID(optnode))
+   prom_panic("Cannot find /options");
+
+   /*
+* If we already increased the RMA size, return.
+*/
+   size = 0ULL;
+   memset(str, 0, sizeof(str));
+   rc = prom_getprop(optnode, "ibm,new-rma-size", , sizeof(str));
+
+   size = prom_strtoul(str, NULL);
+   if (size == 512ULL) {


Is this preferred over strncmp?  Using a string also helps with my suggestion 
below...


+   prom_printf("RMA size already at %lu.\n", size);
+   return;
+ 

Re: pcieport AER error spam on Intel Skylake

[Alexander Duyck]

On Fri, Aug 5, 2016 at 11:15 AM, Daniel Drake  wrote:
> Hi Alexander,
>
> Reviving an old topic here...
>
> We are seeing this "problem" on an increasing number of units from the
> vendor, and searching around it can also be seen on Dell and HP
> products. Always with the same Realtek b723 wifi device. e.g.
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1521173
>
> The amount of error spam is problematic in that it slows down boot
> really significantly, while printing lots of scary messages for the
> user.
> We tried doing a PCI MSI blacklist for affected laptops but we are
> struggling to keep that blacklist updated with the increasing number
> of affected models.
>
> Enough hacks, I am wondering what we can do to solve this problem in
> the mainline kernel...
>
> On Thu, Sep 3, 2015 at 12:05 PM, Alexander Duyck
>  wrote:
>> On 09/03/2015 06:32 AM, Daniel Drake wrote:
>>>
>>> On Wed, Sep 2, 2015 at 7:57 PM, Alexander Duyck
>>>  wrote:

 Since it is correctable errors it is likely some sort of signalling
 issue.
 Could we get the output of something like an lspci -vt? Then you would be
 able to tell what the device is on the other side of the link from
 00:1c.5
 and then we could probably check to see if there has been any changes for
 the device driver on the other end of the link.
>>>
>>> "lspci -vt" reliably causes one occurance of the message, which is
>>> logged by the kernel before lspci has written anything to stdout.
>>>   pcieport :00:1c.5: AER: Corrected error received: id=00e5
>>>   pcieport :00:1c.5: PCIe Bus Error: severity=Corrected,
>>> type=Physical Layer, id=00e5(Receiver ID)
>>>   pcieport :00:1c.5:   device [8086:9d15] error
>>> status/mask=0001/2000
>>>   pcieport :00:1c.5:[ 0] Receiver Error
>>>
>>> -[:00]-+-00.0  Intel Corporation Device 1904
>>> +-02.0  Intel Corporation Device 1916
>>> +-04.0  Intel Corporation Device 1903
>>> +-08.0  Intel Corporation Device 1911
>>> +-14.0  Intel Corporation Device 9d2f
>>> +-14.2  Intel Corporation Device 9d31
>>> +-15.0  Intel Corporation Device 9d60
>>> +-15.1  Intel Corporation Device 9d61
>>> +-16.0  Intel Corporation Device 9d3a
>>> +-17.0  Intel Corporation Device 9d03
>>> +-1c.0-[01]--
>>> +-1c.4-[02]00.0  Realtek Semiconductor Co., Ltd.
>>> RTL8111/8168 PCI Express Gigabit Ethernet controller
>>> +-1c.5-[03]00.0  Realtek Semiconductor Co., Ltd. Device
>>> b723
>>> +-1f.0  Intel Corporation Device 9d48
>>> +-1f.2  Intel Corporation Device 9d21
>>> +-1f.3  Intel Corporation Device 9d70
>>> \-1f.4  Intel Corporation Device 9d23
>>>
>>> Does this mean these messages are somehow related to the Realtek b723
>>> device? That is the wifi card.
>>> Using x86_64_defconfig there is not even any driver loaded for this
>>> device, yet the messages appear quite a bit.
>>> If I use a full config with all the relevant drivers including
>>> rtlwifi, the frequency of these messages goes up a lot though.
>>
>>
>> The correctable errors are likely a result of some sort of link error
>> between the root port 00:1c.5 and the wireless adapter at 3:00.0.  What is
>> likely happening is that when the device is unused it transitions down to a
>> lower power link state like L0s or L1, and when it comes out of that state
>> it is likely triggering the PCIe error most likely as a result of something
>> during the PCIe link training sequence.
>>
>> You might want to notify the manufacturer of the laptop as they may need to
>> address an issue in their hardware, firmware, or possibly add  a workaround
>> to mask off Receiver Error reporting for their part via either a PCIe quirk
>> or driver fix.
>>
 My suspicion since this is a laptop is that something like a power
 management change might be responsible if this is a regression as I have
 seen messages like this pop up as a result of ASPM being enabled before.
>>>
>>> It's likely not a regression, this is brand new hardware and this
>>> message is seen on all kernels that we have tried (4.1, 4.2, master).
>>> pcie_aspm=off also makes these messages go away.
>>
>>
>> Correctable errors are considered a sign of the PCIe link health. In theory
>> they can be ignored since by definition they can be corrected by the
>> hardware.  One thing you could do if you aren't using the wireless card
>> would be to simply switch off the correctable error reporting by setting the
>> mask bit for it in configuration space using setpci.
>>
>> To do that what you could do is find the offset for the PCIe AER
>> configuration register for your port by doing a "lspci -vvv -s 0:1c.5" and
>> what you should get will be a dump listing the capabilities and their
>> current settings.  In there you should find a line like:
>> Capabilities: [148 v1] Advanced 

Re: [PATCH v2 1/2] soc: qcom: provide mechanism for drivers to access L2 registers

[Neil Leeder]

On 8/5/2016 06:00 AM, Mark Rutland wrote:
> On Thu, Aug 04, 2016 at 05:11:10PM -0400, Neil Leeder wrote:
>> L2 registers are accessed using a select register and data
>> register pair. To prevent multiple concurrent writes to the
>> select register by independent drivers, the write to the
>> select register and the associated access of the data register
>> are protected with a lock. All drivers accessing the L2
>> registers use the set and get functions provided by
>> l2-accessors to ensure correct reads and writes to L2 registers.
> 
> As of this series, this is only used by the PMU driver. Which other
> drivers do you plan to use this for?
> 
> If there's nothing else planned at the moment, it would be nicer to fold
> these into the PMU driver.
> 

I see a couple of other drivers on codeaurora.org using it: the
Error Reporting (ERP) driver and an adaptive clock generator. 
I'd guess they'll be submitted to LKML but they're not mine so I don't
know when.

As the purpose of this is to be the common interface for multiple drivers
to stop them walking over each other, I think it makes sense to keep
it separate.

> [...]
> 
>> +config QCOM_L2_ACCESSORS
>> +bool "Qualcomm Technologies L2-cache accessors"
>> +depends on ARCH_QCOM && ARM64
>> +help
>> +  Say y here to enable support for the Qualcomm Technologies
>> +  L2 accessors.
>> +  Provides support for accessing registers in the L2 cache
>> +  for Qualcomm Technologies chips.
> 
> Which chips have this?

Qualcomm Technologies ARM64 chips, so currently QDF24xx family and 
anything Kryo based. I'd assume any future chip families as well.
Given the 'depends on' line, I wasn't sure there was any benefit
to essentially duplicating that in the help text.

> 
> Have drivers select this as necessary. There's no reason for this to be
> used-selectable given this is trivial common infrastructure.

OK, I'll fix that

> 
> [...]
> 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
> 
> Nit: please sort these alphabetically.

OK

> 
> [...]
> 
>> +EXPORT_SYMBOL(set_l2_indirect_reg);
> 
> The PMU driver isn't a module, so this doesn't need to be exported.
> Until there's a modular user, please get rid of EXPORT_SYMBOL.
> 
>> +EXPORT_SYMBOL(get_l2_indirect_reg);
> 
> Likewise.

OK to both of these.

> 
> Thanks,
> Mark.
> 

Thank you for the comments.
Neil

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

Re: [PATCH v2 1/2] soc: qcom: provide mechanism for drivers to access L2 registers

[Neil Leeder]

On 8/5/2016 06:00 AM, Mark Rutland wrote:
> On Thu, Aug 04, 2016 at 05:11:10PM -0400, Neil Leeder wrote:
>> L2 registers are accessed using a select register and data
>> register pair. To prevent multiple concurrent writes to the
>> select register by independent drivers, the write to the
>> select register and the associated access of the data register
>> are protected with a lock. All drivers accessing the L2
>> registers use the set and get functions provided by
>> l2-accessors to ensure correct reads and writes to L2 registers.
> 
> As of this series, this is only used by the PMU driver. Which other
> drivers do you plan to use this for?
> 
> If there's nothing else planned at the moment, it would be nicer to fold
> these into the PMU driver.
> 

I see a couple of other drivers on codeaurora.org using it: the
Error Reporting (ERP) driver and an adaptive clock generator. 
I'd guess they'll be submitted to LKML but they're not mine so I don't
know when.

As the purpose of this is to be the common interface for multiple drivers
to stop them walking over each other, I think it makes sense to keep
it separate.

> [...]
> 
>> +config QCOM_L2_ACCESSORS
>> +bool "Qualcomm Technologies L2-cache accessors"
>> +depends on ARCH_QCOM && ARM64
>> +help
>> +  Say y here to enable support for the Qualcomm Technologies
>> +  L2 accessors.
>> +  Provides support for accessing registers in the L2 cache
>> +  for Qualcomm Technologies chips.
> 
> Which chips have this?

Qualcomm Technologies ARM64 chips, so currently QDF24xx family and 
anything Kryo based. I'd assume any future chip families as well.
Given the 'depends on' line, I wasn't sure there was any benefit
to essentially duplicating that in the help text.

> 
> Have drivers select this as necessary. There's no reason for this to be
> used-selectable given this is trivial common infrastructure.

OK, I'll fix that

> 
> [...]
> 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
> 
> Nit: please sort these alphabetically.

OK

> 
> [...]
> 
>> +EXPORT_SYMBOL(set_l2_indirect_reg);
> 
> The PMU driver isn't a module, so this doesn't need to be exported.
> Until there's a modular user, please get rid of EXPORT_SYMBOL.
> 
>> +EXPORT_SYMBOL(get_l2_indirect_reg);
> 
> Likewise.

OK to both of these.

> 
> Thanks,
> Mark.
> 

Thank you for the comments.
Neil

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

Re: pcieport AER error spam on Intel Skylake

[Bjorn Helgaas]

On Fri, Aug 05, 2016 at 12:15:53PM -0600, Daniel Drake wrote:
> Hi Alexander,
> 
> Reviving an old topic here...
> 
> We are seeing this "problem" on an increasing number of units from the
> vendor, and searching around it can also be seen on Dell and HP
> products. Always with the same Realtek b723 wifi device. e.g.
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1521173
> 
> The amount of error spam is problematic in that it slows down boot
> really significantly, while printing lots of scary messages for the
> user.
> We tried doing a PCI MSI blacklist for affected laptops but we are
> struggling to keep that blacklist updated with the increasing number
> of affected models.
> 
> Enough hacks, I am wondering what we can do to solve this problem in
> the mainline kernel...

I think this is a bug in AER:
https://bugzilla.kernel.org/show_bug.cgi?id=109691

I think I understand the problem, but I haven't had time to fix it.
The bugzilla has a pointer to more details, and it would be awesome if
somebody would jump in.

> On Thu, Sep 3, 2015 at 12:05 PM, Alexander Duyck
>  wrote:
> > On 09/03/2015 06:32 AM, Daniel Drake wrote:
> >>
> >> On Wed, Sep 2, 2015 at 7:57 PM, Alexander Duyck
> >>  wrote:
> >>>
> >>> Since it is correctable errors it is likely some sort of signalling
> >>> issue.
> >>> Could we get the output of something like an lspci -vt? Then you would be
> >>> able to tell what the device is on the other side of the link from
> >>> 00:1c.5
> >>> and then we could probably check to see if there has been any changes for
> >>> the device driver on the other end of the link.
> >>
> >> "lspci -vt" reliably causes one occurance of the message, which is
> >> logged by the kernel before lspci has written anything to stdout.
> >>   pcieport :00:1c.5: AER: Corrected error received: id=00e5
> >>   pcieport :00:1c.5: PCIe Bus Error: severity=Corrected,
> >> type=Physical Layer, id=00e5(Receiver ID)
> >>   pcieport :00:1c.5:   device [8086:9d15] error
> >> status/mask=0001/2000
> >>   pcieport :00:1c.5:[ 0] Receiver Error
> >>
> >> -[:00]-+-00.0  Intel Corporation Device 1904
> >> +-02.0  Intel Corporation Device 1916
> >> +-04.0  Intel Corporation Device 1903
> >> +-08.0  Intel Corporation Device 1911
> >> +-14.0  Intel Corporation Device 9d2f
> >> +-14.2  Intel Corporation Device 9d31
> >> +-15.0  Intel Corporation Device 9d60
> >> +-15.1  Intel Corporation Device 9d61
> >> +-16.0  Intel Corporation Device 9d3a
> >> +-17.0  Intel Corporation Device 9d03
> >> +-1c.0-[01]--
> >> +-1c.4-[02]00.0  Realtek Semiconductor Co., Ltd.
> >> RTL8111/8168 PCI Express Gigabit Ethernet controller
> >> +-1c.5-[03]00.0  Realtek Semiconductor Co., Ltd. Device
> >> b723
> >> +-1f.0  Intel Corporation Device 9d48
> >> +-1f.2  Intel Corporation Device 9d21
> >> +-1f.3  Intel Corporation Device 9d70
> >> \-1f.4  Intel Corporation Device 9d23
> >>
> >> Does this mean these messages are somehow related to the Realtek b723
> >> device? That is the wifi card.
> >> Using x86_64_defconfig there is not even any driver loaded for this
> >> device, yet the messages appear quite a bit.
> >> If I use a full config with all the relevant drivers including
> >> rtlwifi, the frequency of these messages goes up a lot though.
> >
> >
> > The correctable errors are likely a result of some sort of link error
> > between the root port 00:1c.5 and the wireless adapter at 3:00.0.  What is
> > likely happening is that when the device is unused it transitions down to a
> > lower power link state like L0s or L1, and when it comes out of that state
> > it is likely triggering the PCIe error most likely as a result of something
> > during the PCIe link training sequence.
> >
> > You might want to notify the manufacturer of the laptop as they may need to
> > address an issue in their hardware, firmware, or possibly add  a workaround
> > to mask off Receiver Error reporting for their part via either a PCIe quirk
> > or driver fix.
> >
> >>> My suspicion since this is a laptop is that something like a power
> >>> management change might be responsible if this is a regression as I have
> >>> seen messages like this pop up as a result of ASPM being enabled before.
> >>
> >> It's likely not a regression, this is brand new hardware and this
> >> message is seen on all kernels that we have tried (4.1, 4.2, master).
> >> pcie_aspm=off also makes these messages go away.
> >
> >
> > Correctable errors are considered a sign of the PCIe link health. In theory
> > they can be ignored since by definition they can be corrected by the
> > hardware.  One thing you could do if you aren't using the wireless card
> > would be to simply switch off the correctable error reporting by setting 

Re: pcieport AER error spam on Intel Skylake

[Bjorn Helgaas]

On Fri, Aug 05, 2016 at 12:15:53PM -0600, Daniel Drake wrote:
> Hi Alexander,
> 
> Reviving an old topic here...
> 
> We are seeing this "problem" on an increasing number of units from the
> vendor, and searching around it can also be seen on Dell and HP
> products. Always with the same Realtek b723 wifi device. e.g.
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1521173
> 
> The amount of error spam is problematic in that it slows down boot
> really significantly, while printing lots of scary messages for the
> user.
> We tried doing a PCI MSI blacklist for affected laptops but we are
> struggling to keep that blacklist updated with the increasing number
> of affected models.
> 
> Enough hacks, I am wondering what we can do to solve this problem in
> the mainline kernel...

I think this is a bug in AER:
https://bugzilla.kernel.org/show_bug.cgi?id=109691

I think I understand the problem, but I haven't had time to fix it.
The bugzilla has a pointer to more details, and it would be awesome if
somebody would jump in.

> On Thu, Sep 3, 2015 at 12:05 PM, Alexander Duyck
>  wrote:
> > On 09/03/2015 06:32 AM, Daniel Drake wrote:
> >>
> >> On Wed, Sep 2, 2015 at 7:57 PM, Alexander Duyck
> >>  wrote:
> >>>
> >>> Since it is correctable errors it is likely some sort of signalling
> >>> issue.
> >>> Could we get the output of something like an lspci -vt? Then you would be
> >>> able to tell what the device is on the other side of the link from
> >>> 00:1c.5
> >>> and then we could probably check to see if there has been any changes for
> >>> the device driver on the other end of the link.
> >>
> >> "lspci -vt" reliably causes one occurance of the message, which is
> >> logged by the kernel before lspci has written anything to stdout.
> >>   pcieport :00:1c.5: AER: Corrected error received: id=00e5
> >>   pcieport :00:1c.5: PCIe Bus Error: severity=Corrected,
> >> type=Physical Layer, id=00e5(Receiver ID)
> >>   pcieport :00:1c.5:   device [8086:9d15] error
> >> status/mask=0001/2000
> >>   pcieport :00:1c.5:[ 0] Receiver Error
> >>
> >> -[:00]-+-00.0  Intel Corporation Device 1904
> >> +-02.0  Intel Corporation Device 1916
> >> +-04.0  Intel Corporation Device 1903
> >> +-08.0  Intel Corporation Device 1911
> >> +-14.0  Intel Corporation Device 9d2f
> >> +-14.2  Intel Corporation Device 9d31
> >> +-15.0  Intel Corporation Device 9d60
> >> +-15.1  Intel Corporation Device 9d61
> >> +-16.0  Intel Corporation Device 9d3a
> >> +-17.0  Intel Corporation Device 9d03
> >> +-1c.0-[01]--
> >> +-1c.4-[02]00.0  Realtek Semiconductor Co., Ltd.
> >> RTL8111/8168 PCI Express Gigabit Ethernet controller
> >> +-1c.5-[03]00.0  Realtek Semiconductor Co., Ltd. Device
> >> b723
> >> +-1f.0  Intel Corporation Device 9d48
> >> +-1f.2  Intel Corporation Device 9d21
> >> +-1f.3  Intel Corporation Device 9d70
> >> \-1f.4  Intel Corporation Device 9d23
> >>
> >> Does this mean these messages are somehow related to the Realtek b723
> >> device? That is the wifi card.
> >> Using x86_64_defconfig there is not even any driver loaded for this
> >> device, yet the messages appear quite a bit.
> >> If I use a full config with all the relevant drivers including
> >> rtlwifi, the frequency of these messages goes up a lot though.
> >
> >
> > The correctable errors are likely a result of some sort of link error
> > between the root port 00:1c.5 and the wireless adapter at 3:00.0.  What is
> > likely happening is that when the device is unused it transitions down to a
> > lower power link state like L0s or L1, and when it comes out of that state
> > it is likely triggering the PCIe error most likely as a result of something
> > during the PCIe link training sequence.
> >
> > You might want to notify the manufacturer of the laptop as they may need to
> > address an issue in their hardware, firmware, or possibly add  a workaround
> > to mask off Receiver Error reporting for their part via either a PCIe quirk
> > or driver fix.
> >
> >>> My suspicion since this is a laptop is that something like a power
> >>> management change might be responsible if this is a regression as I have
> >>> seen messages like this pop up as a result of ASPM being enabled before.
> >>
> >> It's likely not a regression, this is brand new hardware and this
> >> message is seen on all kernels that we have tried (4.1, 4.2, master).
> >> pcie_aspm=off also makes these messages go away.
> >
> >
> > Correctable errors are considered a sign of the PCIe link health. In theory
> > they can be ignored since by definition they can be corrected by the
> > hardware.  One thing you could do if you aren't using the wireless card
> > would be to simply switch off the correctable error reporting by setting the
> > mask bit for it in configuration space using 

Re: mfd: dm355evm_msp: Refactoring for add_child()

[SF Markus Elfring]

> I'm unsure if you're not re-submitting because you're waiting for an
> answer for me or not.

I found your five commits (on 2016-06-29) for this patch series
sufficient in principle.
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=d313cdde71ec9a5c327a515c37a0dca2cca00de5


> If you are, here it is.  And, don't do that. :)

I find this feedback a bit strange.


> Please submit your patch-set *not* connected to this, now very
> tangled web of submissions, complete with all the Acks you've
> accrued.

I would find it nice if another one of my update suggestions
could be integrated after a bit of feedback evolved for a special
source code search pattern.

https://patchwork.kernel.org/patch/9199519/
https://patchwork.kernel.org/patch/9208925/
https://patchwork.kernel.org/patch/9210291/

It seems to be harder to achieve acceptance similar to the others.

Regards,
Markus

Re: mfd: dm355evm_msp: Refactoring for add_child()

[SF Markus Elfring]

> I'm unsure if you're not re-submitting because you're waiting for an
> answer for me or not.

I found your five commits (on 2016-06-29) for this patch series
sufficient in principle.
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=d313cdde71ec9a5c327a515c37a0dca2cca00de5


> If you are, here it is.  And, don't do that. :)

I find this feedback a bit strange.


> Please submit your patch-set *not* connected to this, now very
> tangled web of submissions, complete with all the Acks you've
> accrued.

I would find it nice if another one of my update suggestions
could be integrated after a bit of feedback evolved for a special
source code search pattern.

https://patchwork.kernel.org/patch/9199519/
https://patchwork.kernel.org/patch/9208925/
https://patchwork.kernel.org/patch/9210291/

It seems to be harder to achieve acceptance similar to the others.

Regards,
Markus

[PATCH v3 1/9] usb: gadget: fix usb_ep_align_maybe endianness and new usb_ep_align

[Felipe F. Tonello]

USB spec specifies wMaxPacketSize to be little endian (as other properties),
so when using this variable in the driver we should convert to the current
CPU endianness if necessary.

This patch also introduces usb_ep_align() which does always returns the
aligned buffer size for an endpoint. This is useful to be used by USB requests
allocator functions.

Signed-off-by: Felipe F. Tonello 
---
 include/linux/usb/gadget.h | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 612dbdfa388e..3cc93237ff98 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -418,8 +418,20 @@ static inline struct usb_gadget *dev_to_usb_gadget(struct 
device *dev)
list_for_each_entry(tmp, &(gadget)->ep_list, ep_list)
 
 /**
+ * usb_ep_align - returns @len aligned to ep's maxpacketsize.
+ * @ep: the endpoint whose maxpacketsize is used to align @len
+ * @len: buffer size's length to align to @ep's maxpacketsize
+ *
+ * This helper is used to align buffer's size to an ep's maxpacketsize.
+ */
+static inline size_t usb_ep_align(struct usb_ep *ep, size_t len)
+{
+   return round_up(len, (size_t)le16_to_cpu(ep->desc->wMaxPacketSize));
+}
+
+/**
  * usb_ep_align_maybe - returns @len aligned to ep's maxpacketsize if gadget
- * requires quirk_ep_out_aligned_size, otherwise reguens len.
+ * requires quirk_ep_out_aligned_size, otherwise returns len.
  * @g: controller to check for quirk
  * @ep: the endpoint whose maxpacketsize is used to align @len
  * @len: buffer size's length to align to @ep's maxpacketsize
@@ -430,8 +442,7 @@ static inline struct usb_gadget *dev_to_usb_gadget(struct 
device *dev)
 static inline size_t
 usb_ep_align_maybe(struct usb_gadget *g, struct usb_ep *ep, size_t len)
 {
-   return !g->quirk_ep_out_aligned_size ? len :
-   round_up(len, (size_t)ep->desc->wMaxPacketSize);
+   return g->quirk_ep_out_aligned_size ? usb_ep_align(ep, len) : len;
 }
 
 /**
-- 
2.9.0

[PATCH v3 1/9] usb: gadget: fix usb_ep_align_maybe endianness and new usb_ep_align

[Felipe F. Tonello]

USB spec specifies wMaxPacketSize to be little endian (as other properties),
so when using this variable in the driver we should convert to the current
CPU endianness if necessary.

This patch also introduces usb_ep_align() which does always returns the
aligned buffer size for an endpoint. This is useful to be used by USB requests
allocator functions.

Signed-off-by: Felipe F. Tonello 
---
 include/linux/usb/gadget.h | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 612dbdfa388e..3cc93237ff98 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -418,8 +418,20 @@ static inline struct usb_gadget *dev_to_usb_gadget(struct 
device *dev)
list_for_each_entry(tmp, &(gadget)->ep_list, ep_list)
 
 /**
+ * usb_ep_align - returns @len aligned to ep's maxpacketsize.
+ * @ep: the endpoint whose maxpacketsize is used to align @len
+ * @len: buffer size's length to align to @ep's maxpacketsize
+ *
+ * This helper is used to align buffer's size to an ep's maxpacketsize.
+ */
+static inline size_t usb_ep_align(struct usb_ep *ep, size_t len)
+{
+   return round_up(len, (size_t)le16_to_cpu(ep->desc->wMaxPacketSize));
+}
+
+/**
  * usb_ep_align_maybe - returns @len aligned to ep's maxpacketsize if gadget
- * requires quirk_ep_out_aligned_size, otherwise reguens len.
+ * requires quirk_ep_out_aligned_size, otherwise returns len.
  * @g: controller to check for quirk
  * @ep: the endpoint whose maxpacketsize is used to align @len
  * @len: buffer size's length to align to @ep's maxpacketsize
@@ -430,8 +442,7 @@ static inline struct usb_gadget *dev_to_usb_gadget(struct 
device *dev)
 static inline size_t
 usb_ep_align_maybe(struct usb_gadget *g, struct usb_ep *ep, size_t len)
 {
-   return !g->quirk_ep_out_aligned_size ? len :
-   round_up(len, (size_t)ep->desc->wMaxPacketSize);
+   return g->quirk_ep_out_aligned_size ? usb_ep_align(ep, len) : len;
 }
 
 /**
-- 
2.9.0

[PATCH v3 2/9] usb: gadget: align buffer size when allocating for OUT endpoint

[Felipe F. Tonello]

Using usb_ep_align() makes sure that the buffer size for OUT endpoints is
always aligned with wMaxPacketSize (512 usually). This makes sure
that no buffer has the wrong size, which can cause nasty bugs.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/u_f.c |  3 +++
 drivers/usb/gadget/u_f.h | 18 --
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/u_f.c b/drivers/usb/gadget/u_f.c
index 4bc7eea8bfc8..d1933b0b76c3 100644
--- a/drivers/usb/gadget/u_f.c
+++ b/drivers/usb/gadget/u_f.c
@@ -12,6 +12,7 @@
  */
 
 #include "u_f.h"
+#include 
 
 struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int default_len)
 {
@@ -20,6 +21,8 @@ struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, 
int default_len)
req = usb_ep_alloc_request(ep, GFP_ATOMIC);
if (req) {
req->length = len ?: default_len;
+   if (usb_endpoint_dir_out(ep->desc))
+   req->length = usb_ep_align(ep, req->length);
req->buf = kmalloc(req->length, GFP_ATOMIC);
if (!req->buf) {
usb_ep_free_request(ep, req);
diff --git a/drivers/usb/gadget/u_f.h b/drivers/usb/gadget/u_f.h
index 4247cc098a89..69a1d10df04f 100644
--- a/drivers/usb/gadget/u_f.h
+++ b/drivers/usb/gadget/u_f.h
@@ -47,8 +47,22 @@
 struct usb_ep;
 struct usb_request;
 
-/* Requests allocated via alloc_ep_req() must be freed by free_ep_req(). */
-struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int default_len);
+/**
+ * alloc_ep_req - returns a usb_request allocated by the gadget driver and
+ * allocates the request's buffer.
+ *
+ * @ep: the endpoint to allocate a usb_request
+ * @len: usb_requests's buffer suggested size
+ * @default_len: used if @len is not provided, ie, is 0
+ *
+ * In case @ep direction is OUT, the @len will be aligned to ep's
+ * wMaxPacketSize. In order to avoid memory leaks or drops, *always* use
+ * usb_requests's length (req->length) to refer to the allocated buffer size.
+ * Requests allocated via alloc_ep_req() *must* be freed by free_ep_req().
+ */
+struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len, int 
default_len);
+
+/* Frees a usb_request previously allocated by alloc_ep_req() */
 static inline void free_ep_req(struct usb_ep *ep, struct usb_request *req)
 {
kfree(req->buf);
-- 
2.9.0

[PATCH v3 2/9] usb: gadget: align buffer size when allocating for OUT endpoint

[Felipe F. Tonello]

Using usb_ep_align() makes sure that the buffer size for OUT endpoints is
always aligned with wMaxPacketSize (512 usually). This makes sure
that no buffer has the wrong size, which can cause nasty bugs.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/u_f.c |  3 +++
 drivers/usb/gadget/u_f.h | 18 --
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/u_f.c b/drivers/usb/gadget/u_f.c
index 4bc7eea8bfc8..d1933b0b76c3 100644
--- a/drivers/usb/gadget/u_f.c
+++ b/drivers/usb/gadget/u_f.c
@@ -12,6 +12,7 @@
  */
 
 #include "u_f.h"
+#include 
 
 struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int default_len)
 {
@@ -20,6 +21,8 @@ struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, 
int default_len)
req = usb_ep_alloc_request(ep, GFP_ATOMIC);
if (req) {
req->length = len ?: default_len;
+   if (usb_endpoint_dir_out(ep->desc))
+   req->length = usb_ep_align(ep, req->length);
req->buf = kmalloc(req->length, GFP_ATOMIC);
if (!req->buf) {
usb_ep_free_request(ep, req);
diff --git a/drivers/usb/gadget/u_f.h b/drivers/usb/gadget/u_f.h
index 4247cc098a89..69a1d10df04f 100644
--- a/drivers/usb/gadget/u_f.h
+++ b/drivers/usb/gadget/u_f.h
@@ -47,8 +47,22 @@
 struct usb_ep;
 struct usb_request;
 
-/* Requests allocated via alloc_ep_req() must be freed by free_ep_req(). */
-struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int default_len);
+/**
+ * alloc_ep_req - returns a usb_request allocated by the gadget driver and
+ * allocates the request's buffer.
+ *
+ * @ep: the endpoint to allocate a usb_request
+ * @len: usb_requests's buffer suggested size
+ * @default_len: used if @len is not provided, ie, is 0
+ *
+ * In case @ep direction is OUT, the @len will be aligned to ep's
+ * wMaxPacketSize. In order to avoid memory leaks or drops, *always* use
+ * usb_requests's length (req->length) to refer to the allocated buffer size.
+ * Requests allocated via alloc_ep_req() *must* be freed by free_ep_req().
+ */
+struct usb_request *alloc_ep_req(struct usb_ep *ep, size_t len, int 
default_len);
+
+/* Frees a usb_request previously allocated by alloc_ep_req() */
 static inline void free_ep_req(struct usb_ep *ep, struct usb_request *req)
 {
kfree(req->buf);
-- 
2.9.0

[PATCH v3 3/9] usb: gadget: f_midi: remove alignment code for OUT endpoint

[Felipe F. Tonello]

The new version of alloc_ep_req() already aligns the buffer size to
wMaxPacketSize on OUT endpoints.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_midi.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index 58fc199a18ec..39018dea7035 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -360,9 +360,8 @@ static int f_midi_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
/* allocate a bunch of read buffers and queue them all at once. */
for (i = 0; i < midi->qlen && err == 0; i++) {
struct usb_request *req =
-   midi_alloc_ep_req(midi->out_ep,
-   max_t(unsigned, midi->buflen,
-   bulk_out_desc.wMaxPacketSize));
+   midi_alloc_ep_req(midi->out_ep, midi->buflen);
+
if (req == NULL)
return -ENOMEM;
 
-- 
2.9.0

[PATCH v3 3/9] usb: gadget: f_midi: remove alignment code for OUT endpoint

[Felipe F. Tonello]

The new version of alloc_ep_req() already aligns the buffer size to
wMaxPacketSize on OUT endpoints.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_midi.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index 58fc199a18ec..39018dea7035 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -360,9 +360,8 @@ static int f_midi_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
/* allocate a bunch of read buffers and queue them all at once. */
for (i = 0; i < midi->qlen && err == 0; i++) {
struct usb_request *req =
-   midi_alloc_ep_req(midi->out_ep,
-   max_t(unsigned, midi->buflen,
-   bulk_out_desc.wMaxPacketSize));
+   midi_alloc_ep_req(midi->out_ep, midi->buflen);
+
if (req == NULL)
return -ENOMEM;
 
-- 
2.9.0

[PATCH v3 5/9] usb: gadget: f_midi: refactor state machine

[Felipe F. Tonello]

This refactor results in a cleaner state machine code and promotes
consistency, readability, and maintanability of this driver.

This refactor state machine was well tested and it is currently running in
production code and devices.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_midi.c | 204 ++-
 1 file changed, 129 insertions(+), 75 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index a7b50ac947f8..09d769e18b50 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -51,6 +51,19 @@ static const char f_midi_longname[] = "MIDI Gadget";
  */
 #define MAX_PORTS 16
 
+/* MIDI message states */
+enum {
+   STATE_INITIAL = 0,  /* pseudo state */
+   STATE_1PARAM,
+   STATE_2PARAM_1,
+   STATE_2PARAM_2,
+   STATE_SYSEX_0,
+   STATE_SYSEX_1,
+   STATE_SYSEX_2,
+   STATE_REAL_TIME,
+   STATE_FINISHED, /* pseudo state */
+};
+
 /*
  * This is a gadget, and the IN/OUT naming is from the host's perspective.
  * USB -> OUT endpoint -> rawmidi
@@ -61,13 +74,6 @@ struct gmidi_in_port {
int active;
uint8_t cable;
uint8_t state;
-#define STATE_UNKNOWN  0
-#define STATE_1PARAM   1
-#define STATE_2PARAM_1 2
-#define STATE_2PARAM_2 3
-#define STATE_SYSEX_0  4
-#define STATE_SYSEX_1  5
-#define STATE_SYSEX_2  6
uint8_t data[2];
 };
 
@@ -403,118 +409,166 @@ static int f_midi_snd_free(struct snd_device *device)
return 0;
 }
 
-static void f_midi_transmit_packet(struct usb_request *req, uint8_t p0,
-   uint8_t p1, uint8_t p2, uint8_t p3)
-{
-   unsigned length = req->length;
-   u8 *buf = (u8 *)req->buf + length;
-
-   buf[0] = p0;
-   buf[1] = p1;
-   buf[2] = p2;
-   buf[3] = p3;
-   req->length = length + 4;
-}
-
 /*
  * Converts MIDI commands to USB MIDI packets.
  */
 static void f_midi_transmit_byte(struct usb_request *req,
 struct gmidi_in_port *port, uint8_t b)
 {
-   uint8_t p0 = port->cable << 4;
+   uint8_t p[4] = { port->cable << 4, 0, 0, 0 };
+   uint8_t next_state = STATE_INITIAL;
+
+   switch (b) {
+   case 0xf8 ... 0xff:
+   /* System Real-Time Messages */
+   p[0] |= 0x0f;
+   p[1] = b;
+   next_state = port->state;
+   port->state = STATE_REAL_TIME;
+   break;
+
+   case 0xf7:
+   /* End of SysEx */
+   switch (port->state) {
+   case STATE_SYSEX_0:
+   p[0] |= 0x05;
+   p[1] = 0xf7;
+   next_state = STATE_FINISHED;
+   break;
+   case STATE_SYSEX_1:
+   p[0] |= 0x06;
+   p[1] = port->data[0];
+   p[2] = 0xf7;
+   next_state = STATE_FINISHED;
+   break;
+   case STATE_SYSEX_2:
+   p[0] |= 0x07;
+   p[1] = port->data[0];
+   p[2] = port->data[1];
+   p[3] = 0xf7;
+   next_state = STATE_FINISHED;
+   break;
+   default:
+   /* Ignore byte */
+   next_state = port->state;
+   port->state = STATE_INITIAL;
+   }
+   break;
 
-   if (b >= 0xf8) {
-   f_midi_transmit_packet(req, p0 | 0x0f, b, 0, 0);
-   } else if (b >= 0xf0) {
+   case 0xf0 ... 0xf6:
+   /* System Common Messages */
+   port->data[0] = port->data[1] = 0;
+   port->state = STATE_INITIAL;
switch (b) {
case 0xf0:
port->data[0] = b;
-   port->state = STATE_SYSEX_1;
+   port->data[1] = 0;
+   next_state = STATE_SYSEX_1;
break;
case 0xf1:
case 0xf3:
port->data[0] = b;
-   port->state = STATE_1PARAM;
+   next_state = STATE_1PARAM;
break;
case 0xf2:
port->data[0] = b;
-   port->state = STATE_2PARAM_1;
+   next_state = STATE_2PARAM_1;
break;
case 0xf4:
case 0xf5:
-   port->state = STATE_UNKNOWN;
+   next_state = STATE_INITIAL;
break;
case 0xf6:
-   f_midi_transmit_packet(req, p0 | 0x05, 0xf6, 0, 0);
-   port->state = STATE_UNKNOWN;
-   break;
-   case 0xf7:
-   

[PATCH v3 5/9] usb: gadget: f_midi: refactor state machine

[Felipe F. Tonello]

This refactor results in a cleaner state machine code and promotes
consistency, readability, and maintanability of this driver.

This refactor state machine was well tested and it is currently running in
production code and devices.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_midi.c | 204 ++-
 1 file changed, 129 insertions(+), 75 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index a7b50ac947f8..09d769e18b50 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -51,6 +51,19 @@ static const char f_midi_longname[] = "MIDI Gadget";
  */
 #define MAX_PORTS 16
 
+/* MIDI message states */
+enum {
+   STATE_INITIAL = 0,  /* pseudo state */
+   STATE_1PARAM,
+   STATE_2PARAM_1,
+   STATE_2PARAM_2,
+   STATE_SYSEX_0,
+   STATE_SYSEX_1,
+   STATE_SYSEX_2,
+   STATE_REAL_TIME,
+   STATE_FINISHED, /* pseudo state */
+};
+
 /*
  * This is a gadget, and the IN/OUT naming is from the host's perspective.
  * USB -> OUT endpoint -> rawmidi
@@ -61,13 +74,6 @@ struct gmidi_in_port {
int active;
uint8_t cable;
uint8_t state;
-#define STATE_UNKNOWN  0
-#define STATE_1PARAM   1
-#define STATE_2PARAM_1 2
-#define STATE_2PARAM_2 3
-#define STATE_SYSEX_0  4
-#define STATE_SYSEX_1  5
-#define STATE_SYSEX_2  6
uint8_t data[2];
 };
 
@@ -403,118 +409,166 @@ static int f_midi_snd_free(struct snd_device *device)
return 0;
 }
 
-static void f_midi_transmit_packet(struct usb_request *req, uint8_t p0,
-   uint8_t p1, uint8_t p2, uint8_t p3)
-{
-   unsigned length = req->length;
-   u8 *buf = (u8 *)req->buf + length;
-
-   buf[0] = p0;
-   buf[1] = p1;
-   buf[2] = p2;
-   buf[3] = p3;
-   req->length = length + 4;
-}
-
 /*
  * Converts MIDI commands to USB MIDI packets.
  */
 static void f_midi_transmit_byte(struct usb_request *req,
 struct gmidi_in_port *port, uint8_t b)
 {
-   uint8_t p0 = port->cable << 4;
+   uint8_t p[4] = { port->cable << 4, 0, 0, 0 };
+   uint8_t next_state = STATE_INITIAL;
+
+   switch (b) {
+   case 0xf8 ... 0xff:
+   /* System Real-Time Messages */
+   p[0] |= 0x0f;
+   p[1] = b;
+   next_state = port->state;
+   port->state = STATE_REAL_TIME;
+   break;
+
+   case 0xf7:
+   /* End of SysEx */
+   switch (port->state) {
+   case STATE_SYSEX_0:
+   p[0] |= 0x05;
+   p[1] = 0xf7;
+   next_state = STATE_FINISHED;
+   break;
+   case STATE_SYSEX_1:
+   p[0] |= 0x06;
+   p[1] = port->data[0];
+   p[2] = 0xf7;
+   next_state = STATE_FINISHED;
+   break;
+   case STATE_SYSEX_2:
+   p[0] |= 0x07;
+   p[1] = port->data[0];
+   p[2] = port->data[1];
+   p[3] = 0xf7;
+   next_state = STATE_FINISHED;
+   break;
+   default:
+   /* Ignore byte */
+   next_state = port->state;
+   port->state = STATE_INITIAL;
+   }
+   break;
 
-   if (b >= 0xf8) {
-   f_midi_transmit_packet(req, p0 | 0x0f, b, 0, 0);
-   } else if (b >= 0xf0) {
+   case 0xf0 ... 0xf6:
+   /* System Common Messages */
+   port->data[0] = port->data[1] = 0;
+   port->state = STATE_INITIAL;
switch (b) {
case 0xf0:
port->data[0] = b;
-   port->state = STATE_SYSEX_1;
+   port->data[1] = 0;
+   next_state = STATE_SYSEX_1;
break;
case 0xf1:
case 0xf3:
port->data[0] = b;
-   port->state = STATE_1PARAM;
+   next_state = STATE_1PARAM;
break;
case 0xf2:
port->data[0] = b;
-   port->state = STATE_2PARAM_1;
+   next_state = STATE_2PARAM_1;
break;
case 0xf4:
case 0xf5:
-   port->state = STATE_UNKNOWN;
+   next_state = STATE_INITIAL;
break;
case 0xf6:
-   f_midi_transmit_packet(req, p0 | 0x05, 0xf6, 0, 0);
-   port->state = STATE_UNKNOWN;
-   break;
-   case 0xf7:
-   switch 

Re: [PATCH resend] clocksource: Defer override invalidation unless clock is unstable

[Kyle Walker]

Good evening John,

On Wed, Jul 27, 2016 at 10:29 AM, Kyle Walker  wrote:
> The issue I'm running into is that the override is not HRT compatible yet.
> Though it will be later in the boot process, unless the clocksource watchdog
> marks the clocksource as unstable.
>
> The issue with the current implementation is that the override_name value is
> disabled when the tsc is first checked, before the watchdog has a chance to
> check it and mark it stable or unstable.
>
> Without patch:
> $ dmesg | grep -e clocksource
> 
> clocksource: refined-jiffies: 
> Kernel command line:  clocksource=tsc
> clocksource: hpet: 
> clocksource: xen: 
> clocksource: jiffies: 
> clocksource: Switched to clocksource xen
> clocksource: acpi_pm: 
> tsc: Refined TSC clocksource calibration: 2394.399 MHz
>
> clocksource: tsc: 
> clocksource: Override clocksource tsc is not HRT compatible - 
>
>
> With patch:
> $ dmesg | grep -e clocksource
> 
> clocksource: refined-jiffies:
> Kernel command line:  clocksource=tsc
> clocksource: hpet: 
> clocksource: xen: 
>
> clocksource: jiffies: 
> clocksource: Switched to clocksource xen
> clocksource: acpi_pm: 
> tsc: Refined TSC clocksource calibration: 2394.461 MHz
> clocksource: tsc: 
> clocksource: Override clocksource tsc is not currently HRT compatible
> - deferring
> clocksource: Switched to clocksource tsc
>

Is there anything else needed from my end? Please let me know if there is
any further information or clarification I can provide.

Have a great evening!
-- 
Kyle Walker

Re: [PATCH resend] clocksource: Defer override invalidation unless clock is unstable

[Kyle Walker]

Good evening John,

On Wed, Jul 27, 2016 at 10:29 AM, Kyle Walker  wrote:
> The issue I'm running into is that the override is not HRT compatible yet.
> Though it will be later in the boot process, unless the clocksource watchdog
> marks the clocksource as unstable.
>
> The issue with the current implementation is that the override_name value is
> disabled when the tsc is first checked, before the watchdog has a chance to
> check it and mark it stable or unstable.
>
> Without patch:
> $ dmesg | grep -e clocksource
> 
> clocksource: refined-jiffies: 
> Kernel command line:  clocksource=tsc
> clocksource: hpet: 
> clocksource: xen: 
> clocksource: jiffies: 
> clocksource: Switched to clocksource xen
> clocksource: acpi_pm: 
> tsc: Refined TSC clocksource calibration: 2394.399 MHz
>
> clocksource: tsc: 
> clocksource: Override clocksource tsc is not HRT compatible - 
>
>
> With patch:
> $ dmesg | grep -e clocksource
> 
> clocksource: refined-jiffies:
> Kernel command line:  clocksource=tsc
> clocksource: hpet: 
> clocksource: xen: 
>
> clocksource: jiffies: 
> clocksource: Switched to clocksource xen
> clocksource: acpi_pm: 
> tsc: Refined TSC clocksource calibration: 2394.461 MHz
> clocksource: tsc: 
> clocksource: Override clocksource tsc is not currently HRT compatible
> - deferring
> clocksource: Switched to clocksource tsc
>

Is there anything else needed from my end? Please let me know if there is
any further information or clarification I can provide.

Have a great evening!
-- 
Kyle Walker

[PATCH v3 6/9] usb: gadget: f_midi: drop substreams when disabling endpoint

[Felipe F. Tonello]

This change makes sure that the ALSA buffers are cleaned if an endpoint
becomes disabled.

Before this change, if the internal ALSA buffer did overflow, the MIDI
function would stop sending MIDI to the host.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_midi.c | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index 09d769e18b50..3a47596afcab 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -305,6 +305,19 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
}
 }
 
+static void f_midi_drop_out_substreams(struct f_midi *midi)
+{
+   unsigned int i;
+
+   for (i = 0; i < midi->in_ports; i++) {
+   struct gmidi_in_port *port = midi->in_ports_array + i;
+   struct snd_rawmidi_substream *substream = port->substream;
+
+   if (port->active && substream)
+   snd_rawmidi_drop_output(substream);
+   }
+}
+
 static int f_midi_start_ep(struct f_midi *midi,
   struct usb_function *f,
   struct usb_ep *ep)
@@ -402,6 +415,8 @@ static void f_midi_disable(struct usb_function *f)
/* release IN requests */
while (kfifo_get(>in_req_fifo, ))
free_ep_req(midi->in_ep, req);
+
+   f_midi_drop_out_substreams(midi);
 }
 
 static int f_midi_snd_free(struct snd_device *device)
@@ -571,18 +586,6 @@ static void f_midi_transmit_byte(struct usb_request *req,
port->state = next_state;
 }
 
-static void f_midi_drop_out_substreams(struct f_midi *midi)
-{
-   unsigned int i;
-
-   for (i = 0; i < midi->in_ports; i++) {
-   struct gmidi_in_port *port = midi->in_ports_array + i;
-   struct snd_rawmidi_substream *substream = port->substream;
-   if (port->active && substream)
-   snd_rawmidi_drop_output(substream);
-   }
-}
-
 static int f_midi_do_transmit(struct f_midi *midi, struct usb_ep *ep)
 {
struct usb_request *req = NULL;
-- 
2.9.0

[PATCH v3 6/9] usb: gadget: f_midi: drop substreams when disabling endpoint

[Felipe F. Tonello]

This change makes sure that the ALSA buffers are cleaned if an endpoint
becomes disabled.

Before this change, if the internal ALSA buffer did overflow, the MIDI
function would stop sending MIDI to the host.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_midi.c | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index 09d769e18b50..3a47596afcab 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -305,6 +305,19 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
}
 }
 
+static void f_midi_drop_out_substreams(struct f_midi *midi)
+{
+   unsigned int i;
+
+   for (i = 0; i < midi->in_ports; i++) {
+   struct gmidi_in_port *port = midi->in_ports_array + i;
+   struct snd_rawmidi_substream *substream = port->substream;
+
+   if (port->active && substream)
+   snd_rawmidi_drop_output(substream);
+   }
+}
+
 static int f_midi_start_ep(struct f_midi *midi,
   struct usb_function *f,
   struct usb_ep *ep)
@@ -402,6 +415,8 @@ static void f_midi_disable(struct usb_function *f)
/* release IN requests */
while (kfifo_get(>in_req_fifo, ))
free_ep_req(midi->in_ep, req);
+
+   f_midi_drop_out_substreams(midi);
 }
 
 static int f_midi_snd_free(struct snd_device *device)
@@ -571,18 +586,6 @@ static void f_midi_transmit_byte(struct usb_request *req,
port->state = next_state;
 }
 
-static void f_midi_drop_out_substreams(struct f_midi *midi)
-{
-   unsigned int i;
-
-   for (i = 0; i < midi->in_ports; i++) {
-   struct gmidi_in_port *port = midi->in_ports_array + i;
-   struct snd_rawmidi_substream *substream = port->substream;
-   if (port->active && substream)
-   snd_rawmidi_drop_output(substream);
-   }
-}
-
 static int f_midi_do_transmit(struct f_midi *midi, struct usb_ep *ep)
 {
struct usb_request *req = NULL;
-- 
2.9.0

[PATCH v3 7/9] usb: gadget: remove useless parameter in alloc_ep_req()

[Felipe F. Tonello]

The default_length parameter of alloc_ep_req was not really necessary
and gadget drivers would almost always create an inline function to pass
the same value to len and default_len.

So this patch also removes duplicate code from few drivers.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_hid.c| 10 ++
 drivers/usb/gadget/function/f_loopback.c   |  9 +
 drivers/usb/gadget/function/f_midi.c   | 10 ++
 drivers/usb/gadget/function/f_sourcesink.c | 11 ++-
 drivers/usb/gadget/u_f.c   |  7 +++
 drivers/usb/gadget/u_f.h   |  3 +--
 6 files changed, 11 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c 
b/drivers/usb/gadget/function/f_hid.c
index 51980c50546d..e82a7468252e 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -362,12 +362,6 @@ static int f_hidg_open(struct inode *inode, struct file 
*fd)
 /*-*/
 /*usb_function */
 
-static inline struct usb_request *hidg_alloc_ep_req(struct usb_ep *ep,
-   unsigned length)
-{
-   return alloc_ep_req(ep, length, length);
-}
-
 static void hidg_set_report_complete(struct usb_ep *ep, struct usb_request 
*req)
 {
struct f_hidg *hidg = (struct f_hidg *) req->context;
@@ -549,8 +543,8 @@ static int hidg_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
 */
for (i = 0; i < hidg->qlen && status == 0; i++) {
struct usb_request *req =
-   hidg_alloc_ep_req(hidg->out_ep,
- hidg->report_length);
+   alloc_ep_req(hidg->out_ep,
+   hidg->report_length);
if (req) {
req->complete = hidg_set_report_complete;
req->context  = hidg;
diff --git a/drivers/usb/gadget/function/f_loopback.c 
b/drivers/usb/gadget/function/f_loopback.c
index 3a9f8f9c77bd..701ee0f11c33 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c
@@ -306,13 +306,6 @@ static void disable_loopback(struct f_loopback *loop)
VDBG(cdev, "%s disabled\n", loop->function.name);
 }
 
-static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int len)
-{
-   struct f_loopback   *loop = ep->driver_data;
-
-   return alloc_ep_req(ep, len, loop->buflen);
-}
-
 static int alloc_requests(struct usb_composite_dev *cdev,
  struct f_loopback *loop)
 {
@@ -333,7 +326,7 @@ static int alloc_requests(struct usb_composite_dev *cdev,
if (!in_req)
goto fail;
 
-   out_req = lb_alloc_ep_req(loop->out_ep, 0);
+   out_req = alloc_ep_req(loop->out_ep, loop->buflen);
if (!out_req)
goto fail_in;
 
diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index 3a47596afcab..abf26364b46f 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -208,12 +208,6 @@ static struct usb_gadget_strings *midi_strings[] = {
NULL,
 };
 
-static inline struct usb_request *midi_alloc_ep_req(struct usb_ep *ep,
-   unsigned length)
-{
-   return alloc_ep_req(ep, length, length);
-}
-
 static const uint8_t f_midi_cin_length[] = {
0, 0, 2, 3, 3, 1, 2, 3, 3, 3, 3, 3, 2, 2, 3, 1
 };
@@ -365,7 +359,7 @@ static int f_midi_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
/* pre-allocate write usb requests to use on f_midi_transmit. */
while (kfifo_avail(>in_req_fifo)) {
struct usb_request *req =
-   midi_alloc_ep_req(midi->in_ep, midi->buflen);
+   alloc_ep_req(midi->in_ep, midi->buflen);
 
if (req == NULL)
return -ENOMEM;
@@ -379,7 +373,7 @@ static int f_midi_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
/* allocate a bunch of read buffers and queue them all at once. */
for (i = 0; i < midi->qlen && err == 0; i++) {
struct usb_request *req =
-   midi_alloc_ep_req(midi->out_ep, midi->buflen);
+   alloc_ep_req(midi->out_ep, midi->buflen);
 
if (req == NULL)
return -ENOMEM;
diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
b/drivers/usb/gadget/function/f_sourcesink.c
index df0189ddfdd5..517d78d6da78 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ 

[PATCH v3 7/9] usb: gadget: remove useless parameter in alloc_ep_req()

[Felipe F. Tonello]

The default_length parameter of alloc_ep_req was not really necessary
and gadget drivers would almost always create an inline function to pass
the same value to len and default_len.

So this patch also removes duplicate code from few drivers.

Signed-off-by: Felipe F. Tonello 
---
 drivers/usb/gadget/function/f_hid.c| 10 ++
 drivers/usb/gadget/function/f_loopback.c   |  9 +
 drivers/usb/gadget/function/f_midi.c   | 10 ++
 drivers/usb/gadget/function/f_sourcesink.c | 11 ++-
 drivers/usb/gadget/u_f.c   |  7 +++
 drivers/usb/gadget/u_f.h   |  3 +--
 6 files changed, 11 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c 
b/drivers/usb/gadget/function/f_hid.c
index 51980c50546d..e82a7468252e 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -362,12 +362,6 @@ static int f_hidg_open(struct inode *inode, struct file 
*fd)
 /*-*/
 /*usb_function */
 
-static inline struct usb_request *hidg_alloc_ep_req(struct usb_ep *ep,
-   unsigned length)
-{
-   return alloc_ep_req(ep, length, length);
-}
-
 static void hidg_set_report_complete(struct usb_ep *ep, struct usb_request 
*req)
 {
struct f_hidg *hidg = (struct f_hidg *) req->context;
@@ -549,8 +543,8 @@ static int hidg_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
 */
for (i = 0; i < hidg->qlen && status == 0; i++) {
struct usb_request *req =
-   hidg_alloc_ep_req(hidg->out_ep,
- hidg->report_length);
+   alloc_ep_req(hidg->out_ep,
+   hidg->report_length);
if (req) {
req->complete = hidg_set_report_complete;
req->context  = hidg;
diff --git a/drivers/usb/gadget/function/f_loopback.c 
b/drivers/usb/gadget/function/f_loopback.c
index 3a9f8f9c77bd..701ee0f11c33 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c
@@ -306,13 +306,6 @@ static void disable_loopback(struct f_loopback *loop)
VDBG(cdev, "%s disabled\n", loop->function.name);
 }
 
-static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int len)
-{
-   struct f_loopback   *loop = ep->driver_data;
-
-   return alloc_ep_req(ep, len, loop->buflen);
-}
-
 static int alloc_requests(struct usb_composite_dev *cdev,
  struct f_loopback *loop)
 {
@@ -333,7 +326,7 @@ static int alloc_requests(struct usb_composite_dev *cdev,
if (!in_req)
goto fail;
 
-   out_req = lb_alloc_ep_req(loop->out_ep, 0);
+   out_req = alloc_ep_req(loop->out_ep, loop->buflen);
if (!out_req)
goto fail_in;
 
diff --git a/drivers/usb/gadget/function/f_midi.c 
b/drivers/usb/gadget/function/f_midi.c
index 3a47596afcab..abf26364b46f 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -208,12 +208,6 @@ static struct usb_gadget_strings *midi_strings[] = {
NULL,
 };
 
-static inline struct usb_request *midi_alloc_ep_req(struct usb_ep *ep,
-   unsigned length)
-{
-   return alloc_ep_req(ep, length, length);
-}
-
 static const uint8_t f_midi_cin_length[] = {
0, 0, 2, 3, 3, 1, 2, 3, 3, 3, 3, 3, 2, 2, 3, 1
 };
@@ -365,7 +359,7 @@ static int f_midi_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
/* pre-allocate write usb requests to use on f_midi_transmit. */
while (kfifo_avail(>in_req_fifo)) {
struct usb_request *req =
-   midi_alloc_ep_req(midi->in_ep, midi->buflen);
+   alloc_ep_req(midi->in_ep, midi->buflen);
 
if (req == NULL)
return -ENOMEM;
@@ -379,7 +373,7 @@ static int f_midi_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
/* allocate a bunch of read buffers and queue them all at once. */
for (i = 0; i < midi->qlen && err == 0; i++) {
struct usb_request *req =
-   midi_alloc_ep_req(midi->out_ep, midi->buflen);
+   alloc_ep_req(midi->out_ep, midi->buflen);
 
if (req == NULL)
return -ENOMEM;
diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
b/drivers/usb/gadget/function/f_sourcesink.c
index df0189ddfdd5..517d78d6da78 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -291,13 

[PATCH v3 0/9] Gadget endpoint request allocation and MIDI

[Felipe F. Tonello]

As discussed with Baolin Wang, Michal Nazarewicz and Felipe Balbi. I propose
the forced buffer alignment of OUT endpoints USB requests. This is implemented
by patches #1 and #2.

That not just simplifies the driver code, but it also prevents nasty bugs when
buflen is not aligned or even less than wMaxPacketSize.

Patch #9  removes direct calls to usb_ep_alloc_request() and use alloc_ep_req()
instead. If accepted, then we should apply to all other gadgets that uses
usb_ep_alloc_request() when possible and encorage drivers to use it instead.

Changes from v2:
 * Simplified logic in patch 1
 * Added documentation to alloc_ep_req and free_ep_req
 * Improved commit message on patch 7

Changes from v1:
 * Added patches 1, 2, 7, 8 ,9.
 * Patch 3 removes max_t() for buffer alignment with wMaxPacketSize

Felipe F. Tonello (9):
  usb: gadget: fix usb_ep_align_maybe endianness and new usb_ep_align
  usb: gadget: align buffer size when allocating for OUT endpoint
  usb: gadget: f_midi: remove alignment code for OUT endpoint
  usb: gadget: f_midi: defaults buflen sizes to 512
  usb: gadget: f_midi: refactor state machine
  usb: gadget: f_midi: drop substreams when disabling endpoint
  usb: gadget: remove useless parameter in alloc_ep_req()
  usb: gadget: f_hid: use free_ep_req()
  usb: gadget: f_hid: use alloc_ep_req()

 drivers/usb/gadget/function/f_hid.c|  26 +---
 drivers/usb/gadget/function/f_loopback.c   |   9 +-
 drivers/usb/gadget/function/f_midi.c   | 240 +
 drivers/usb/gadget/function/f_sourcesink.c |  11 +-
 drivers/usb/gadget/legacy/gmidi.c  |   2 +-
 drivers/usb/gadget/u_f.c   |   6 +-
 drivers/usb/gadget/u_f.h   |  17 +-
 include/linux/usb/gadget.h |  17 +-
 8 files changed, 188 insertions(+), 140 deletions(-)

-- 
2.9.0

[PATCH v3 0/9] Gadget endpoint request allocation and MIDI

[Felipe F. Tonello]

As discussed with Baolin Wang, Michal Nazarewicz and Felipe Balbi. I propose
the forced buffer alignment of OUT endpoints USB requests. This is implemented
by patches #1 and #2.

That not just simplifies the driver code, but it also prevents nasty bugs when
buflen is not aligned or even less than wMaxPacketSize.

Patch #9  removes direct calls to usb_ep_alloc_request() and use alloc_ep_req()
instead. If accepted, then we should apply to all other gadgets that uses
usb_ep_alloc_request() when possible and encorage drivers to use it instead.

Changes from v2:
 * Simplified logic in patch 1
 * Added documentation to alloc_ep_req and free_ep_req
 * Improved commit message on patch 7

Changes from v1:
 * Added patches 1, 2, 7, 8 ,9.
 * Patch 3 removes max_t() for buffer alignment with wMaxPacketSize

Felipe F. Tonello (9):
  usb: gadget: fix usb_ep_align_maybe endianness and new usb_ep_align
  usb: gadget: align buffer size when allocating for OUT endpoint
  usb: gadget: f_midi: remove alignment code for OUT endpoint
  usb: gadget: f_midi: defaults buflen sizes to 512
  usb: gadget: f_midi: refactor state machine
  usb: gadget: f_midi: drop substreams when disabling endpoint
  usb: gadget: remove useless parameter in alloc_ep_req()
  usb: gadget: f_hid: use free_ep_req()
  usb: gadget: f_hid: use alloc_ep_req()

 drivers/usb/gadget/function/f_hid.c|  26 +---
 drivers/usb/gadget/function/f_loopback.c   |   9 +-
 drivers/usb/gadget/function/f_midi.c   | 240 +
 drivers/usb/gadget/function/f_sourcesink.c |  11 +-
 drivers/usb/gadget/legacy/gmidi.c  |   2 +-
 drivers/usb/gadget/u_f.c   |   6 +-
 drivers/usb/gadget/u_f.h   |  17 +-
 include/linux/usb/gadget.h |  17 +-
 8 files changed, 188 insertions(+), 140 deletions(-)

-- 
2.9.0