Re: [PATCH] Staging: bcm2835-audio: bcm2835_ctl.c: Fixed a comment coding style issue

[Dan Carpenter]

On Sat, May 20, 2017 at 01:00:17AM +0530, srishti sharma wrote:
> Was the format of this patch acceptable ,the "from" matches the
> "signed-off-by" right , so this should be correct , were there any
> errors in this ? Should I resend it ?
> 
> Regards,
> Srishti
> 

You should capitalize your name in both since you're doing that here.

Also I really think you can figure out how to fix your email headers so
you don't need to use a From header.  Lots of people use gmail and they
were able to figure it out.

But yeah, the way you sent it is also fine.

regards,
dan carpenter

Re: [PATCH] Staging: bcm2835-audio: bcm2835_ctl.c: Fixed a comment coding style issue

[Dan Carpenter]

On Sat, May 20, 2017 at 01:00:17AM +0530, srishti sharma wrote:
> Was the format of this patch acceptable ,the "from" matches the
> "signed-off-by" right , so this should be correct , were there any
> errors in this ? Should I resend it ?
> 
> Regards,
> Srishti
> 

You should capitalize your name in both since you're doing that here.

Also I really think you can figure out how to fix your email headers so
you don't need to use a From header.  Lots of people use gmail and they
were able to figure it out.

But yeah, the way you sent it is also fine.

regards,
dan carpenter

Re: [RFC PATCH v2 08/17] cgroup: Move debug cgroup to its own file

[Waiman Long]

On 05/19/2017 03:21 PM, Tejun Heo wrote:
> Hello, Waiman.
>
> On Thu, May 18, 2017 at 11:52:18AM -0400, Waiman Long wrote:
>> The controller name is "debug" and so it is obvious what this controller
>> is for. However, the config prompt "Example controller" is indeed vague
> Yeah but it also shows up as an integral part of stable interface
> rather than e.g. /sys/kernel/debug.  This isn't of any interest to
> people who aren't developing cgroup core code.  There is no reason to
> risk growing dependencies on it.

The debug controller is used to show information relevant to the cgroup
its css'es are attached to. So it will be very hard to use if we
relocate to /sys/kernel/debug, for example. Currently, nothing in the
debug controller other than debug_cgrp_subsys are exported. I don't see
any risk of having dependency on that controller from other parts of the
kernel.

>> in meaning. So we can make the prompt more descriptive here. As for the
>> boot param, are you saying something like "cgroup_debug" has to be
>> specified in the command line even if CGROUP_DEBUG config is there for
>> the debug controller to be enabled? I am fine with that if you think it
>> is necessary.
> Yeah, I think that'd be a good idea.  cgroup_debug should do.  While
> at it, can you also please make CGROUP_DEBUG depend on DEBUG_KERNEL?
>
> Thanks.
>
Sure. I will do that.

Cheers,
Longman

Re: [RFC PATCH v2 08/17] cgroup: Move debug cgroup to its own file

[Waiman Long]

On 05/19/2017 03:21 PM, Tejun Heo wrote:
> Hello, Waiman.
>
> On Thu, May 18, 2017 at 11:52:18AM -0400, Waiman Long wrote:
>> The controller name is "debug" and so it is obvious what this controller
>> is for. However, the config prompt "Example controller" is indeed vague
> Yeah but it also shows up as an integral part of stable interface
> rather than e.g. /sys/kernel/debug.  This isn't of any interest to
> people who aren't developing cgroup core code.  There is no reason to
> risk growing dependencies on it.

The debug controller is used to show information relevant to the cgroup
its css'es are attached to. So it will be very hard to use if we
relocate to /sys/kernel/debug, for example. Currently, nothing in the
debug controller other than debug_cgrp_subsys are exported. I don't see
any risk of having dependency on that controller from other parts of the
kernel.

>> in meaning. So we can make the prompt more descriptive here. As for the
>> boot param, are you saying something like "cgroup_debug" has to be
>> specified in the command line even if CGROUP_DEBUG config is there for
>> the debug controller to be enabled? I am fine with that if you think it
>> is necessary.
> Yeah, I think that'd be a good idea.  cgroup_debug should do.  While
> at it, can you also please make CGROUP_DEBUG depend on DEBUG_KERNEL?
>
> Thanks.
>
Sure. I will do that.

Cheers,
Longman

Re: [PATCH 4/6] arch/sparc: Enable queued rwlocks for SPARC

[Peter Zijlstra]

On Fri, May 19, 2017 at 03:15:53PM -0400, David Miller wrote:
> However, I don't see what any of this has to do with the feedback
> I was giving the patch author :-)

Uhm,... I think my morning brain read things like you having doubts
about making it sparc64 only. But I could have easily misread things.
Ignore my ramblings :-)

Re: [PATCH 4/6] arch/sparc: Enable queued rwlocks for SPARC

[Peter Zijlstra]

On Fri, May 19, 2017 at 03:15:53PM -0400, David Miller wrote:
> However, I don't see what any of this has to do with the feedback
> I was giving the patch author :-)

Uhm,... I think my morning brain read things like you having doubts
about making it sparc64 only. But I could have easily misread things.
Ignore my ramblings :-)

Re: [PATCH] Staging: bcm2835-audio: bcm2835_ctl.c: Fixed a comment coding style issue

[srishti sharma]

Was the format of this patch acceptable ,the "from" matches the
"signed-off-by" right , so this should be correct , were there any
errors in this ? Should I resend it ?

Regards,
Srishti

On Thu, May 18, 2017 at 7:28 PM, Greg KH  wrote:
> On Thu, May 18, 2017 at 04:20:15PM +0530, srishti wrote:
>> From: srishti sharma 
>>
>> Fixed a trailing */ issue.
>>
>> Signed-off-by: srishti sharma 
>> ---
>>  drivers/staging/bcm2835-audio/bcm2835-ctl.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> You sent two copies of this, which am I supposed to apply?  And what
> changed from the last version?
>
> I'm dropping this, please fix and only send _one_ copy of the patch.
>
> thanks,
>
> greg k-h

Re: [PATCH] Staging: bcm2835-audio: bcm2835_ctl.c: Fixed a comment coding style issue

[srishti sharma]

Was the format of this patch acceptable ,the "from" matches the
"signed-off-by" right , so this should be correct , were there any
errors in this ? Should I resend it ?

Regards,
Srishti

On Thu, May 18, 2017 at 7:28 PM, Greg KH  wrote:
> On Thu, May 18, 2017 at 04:20:15PM +0530, srishti wrote:
>> From: srishti sharma 
>>
>> Fixed a trailing */ issue.
>>
>> Signed-off-by: srishti sharma 
>> ---
>>  drivers/staging/bcm2835-audio/bcm2835-ctl.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> You sent two copies of this, which am I supposed to apply?  And what
> changed from the last version?
>
> I'm dropping this, please fix and only send _one_ copy of the patch.
>
> thanks,
>
> greg k-h

Re: next-20170515: WARNING: CPU: 0 PID: 1 at arch/x86/mm/dump_pagetables.c:236 note_page+0x630/0x7e0

[Kees Cook]

On Fri, May 19, 2017 at 12:18 PM, Andy Lutomirski  wrote:
> On Fri, May 19, 2017 at 12:16 PM, Kees Cook  wrote:
>> On Fri, May 19, 2017 at 11:27 AM, Andy Lutomirski  wrote:
>>> One thing I've pondered: can we make some debugging mode (kmemleak,
>>> perhaps?) check that freed memory is RW at the time it's freed?  I
>>> once wrote some buggy code that freed an R page and caused an OOPS
>>> much later, and this bug here seems likely to be some code that frees
>>> RWX memory.
>>
>> Which begs for even more checks: nothing should ever make a page RWX.
>> Either R, RW, or RX only... (or X too I guess, in the future).
>
> I could see pages being RWX temporarily during boot.  OTOH if we ban
> RWX outright (after very early boot, anyway), then catching code that
> messes up and leaves pages RWX gets much easier.

Right, early boot is kind of special. It'd be nice to have there, but
I meant during normal runtime. We'd probably need to adjust
set_memory_rw/ro/nx/x around to have the correct side-effects, instead
of just controlling specific bits:

set_memory_rw() (RW_)
set_memory_ro() (R__)
set_memory_rx() (R_X)
set_memory_x() (__X)

That kind of refactoring might be not _too_ bad:

- add set_memory_rx()
- s/\bset_memory_x\b/set_memory_rx/g
- fix what breaks from expecting writable-executable memory
- adjust set_memory_rw() to drop x
- fix what breaks from expecting writable-executable memory
- adjust set_memory_ro() to drop x
- fix what breaks from expecting executable memory
- add set_memory_x() some day...

-Kees

-- 
Kees Cook
Pixel Security

Re: next-20170515: WARNING: CPU: 0 PID: 1 at arch/x86/mm/dump_pagetables.c:236 note_page+0x630/0x7e0

[Kees Cook]

On Fri, May 19, 2017 at 12:18 PM, Andy Lutomirski  wrote:
> On Fri, May 19, 2017 at 12:16 PM, Kees Cook  wrote:
>> On Fri, May 19, 2017 at 11:27 AM, Andy Lutomirski  wrote:
>>> One thing I've pondered: can we make some debugging mode (kmemleak,
>>> perhaps?) check that freed memory is RW at the time it's freed?  I
>>> once wrote some buggy code that freed an R page and caused an OOPS
>>> much later, and this bug here seems likely to be some code that frees
>>> RWX memory.
>>
>> Which begs for even more checks: nothing should ever make a page RWX.
>> Either R, RW, or RX only... (or X too I guess, in the future).
>
> I could see pages being RWX temporarily during boot.  OTOH if we ban
> RWX outright (after very early boot, anyway), then catching code that
> messes up and leaves pages RWX gets much easier.

Right, early boot is kind of special. It'd be nice to have there, but
I meant during normal runtime. We'd probably need to adjust
set_memory_rw/ro/nx/x around to have the correct side-effects, instead
of just controlling specific bits:

set_memory_rw() (RW_)
set_memory_ro() (R__)
set_memory_rx() (R_X)
set_memory_x() (__X)

That kind of refactoring might be not _too_ bad:

- add set_memory_rx()
- s/\bset_memory_x\b/set_memory_rx/g
- fix what breaks from expecting writable-executable memory
- adjust set_memory_rw() to drop x
- fix what breaks from expecting writable-executable memory
- adjust set_memory_ro() to drop x
- fix what breaks from expecting executable memory
- add set_memory_x() some day...

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH net] bonding: fix randomly populated arp target array

[Andy Gospodarek]

On Fri, May 19, 2017 at 02:46:46PM -0400, Jarod Wilson wrote:
> In commit dc9c4d0fe023, the arp_target array moved from a static global
> to a local variable. By the nature of static globals, the array used to
> be initialized to all 0. At present, it's full of random data, which
> that gets interpreted as arp_target values, when none have actually been
> specified. Systems end up booting with spew along these lines:
> 
> [   32.161783] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
> [   32.168475] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
> [   32.175089] 8021q: adding VLAN 0 to HW filter on device lacp0
> [   32.193091] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
> [   32.204892] lacp0: Setting MII monitoring interval to 100
> [   32.211071] lacp0: Removing ARP target 216.124.228.17
> [   32.216824] lacp0: Removing ARP target 218.160.255.255
> [   32.222646] lacp0: Removing ARP target 185.170.136.184
> [   32.228496] lacp0: invalid ARP target 255.255.255.255 specified for removal
> [   32.236294] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
> [   32.243987] lacp0: Removing ARP target 56.125.228.17
> [   32.249625] lacp0: Removing ARP target 218.160.255.255
> [   32.255432] lacp0: Removing ARP target 15.157.233.184
> [   32.261165] lacp0: invalid ARP target 255.255.255.255 specified for removal
> [   32.268939] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
> [   32.276632] lacp0: Removing ARP target 16.0.0.0
> [   32.281755] lacp0: Removing ARP target 218.160.255.255
> [   32.287567] lacp0: Removing ARP target 72.125.228.17
> [   32.293165] lacp0: Removing ARP target 218.160.255.255
> [   32.298970] lacp0: Removing ARP target 8.125.228.17
> [   32.304458] lacp0: Removing ARP target 218.160.255.255
> 
> None of these were actually specified as ARP targets, and the driver does
> seem to clean up the mess okay, but it's rather noisy and confusing, leaks
> values to userspace, and the 255.255.255.255 spew shows up even when debug
> prints are disabled.
> 
> The fix: just zero out arp_target at init time.
> 
> While we're in here, init arp_all_targets_value in the right place.
> 

Looks good.  Thanks, Jarod!

Acked-by: Andy Gospodarek 

> Fixes: dc9c4d0fe023 ("bonding: reduce scope of some global variables")
> CC: Mahesh Bandewar 
> CC: Jay Vosburgh 
> CC: Veaceslav Falico 
> CC: Andy Gospodarek 
> CC: net...@vger.kernel.org
> CC: sta...@vger.kernel.org
> Signed-off-by: Jarod Wilson 
> ---
>  drivers/net/bonding/bond_main.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 2be78807fd6e..73313318399c 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -4271,10 +4271,10 @@ static int bond_check_params(struct bond_params 
> *params)
>   int arp_validate_value, fail_over_mac_value, primary_reselect_value, i;
>   struct bond_opt_value newval;
>   const struct bond_opt_value *valptr;
> - int arp_all_targets_value;
> + int arp_all_targets_value = 0;
>   u16 ad_actor_sys_prio = 0;
>   u16 ad_user_port_key = 0;
> - __be32 arp_target[BOND_MAX_ARP_TARGETS];
> + __be32 arp_target[BOND_MAX_ARP_TARGETS] = { 0 };
>   int arp_ip_count;
>   int bond_mode   = BOND_MODE_ROUNDROBIN;
>   int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
> @@ -4501,7 +4501,6 @@ static int bond_check_params(struct bond_params *params)
>   arp_validate_value = 0;
>   }
>  
> - arp_all_targets_value = 0;
>   if (arp_all_targets) {
>   bond_opt_initstr(, arp_all_targets);
>   valptr = bond_opt_parse(bond_opt_get(BOND_OPT_ARP_ALL_TARGETS),
> -- 
> 2.12.1
> 

Re: [PATCH net] bonding: fix randomly populated arp target array

[Andy Gospodarek]

On Fri, May 19, 2017 at 02:46:46PM -0400, Jarod Wilson wrote:
> In commit dc9c4d0fe023, the arp_target array moved from a static global
> to a local variable. By the nature of static globals, the array used to
> be initialized to all 0. At present, it's full of random data, which
> that gets interpreted as arp_target values, when none have actually been
> specified. Systems end up booting with spew along these lines:
> 
> [   32.161783] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
> [   32.168475] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
> [   32.175089] 8021q: adding VLAN 0 to HW filter on device lacp0
> [   32.193091] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
> [   32.204892] lacp0: Setting MII monitoring interval to 100
> [   32.211071] lacp0: Removing ARP target 216.124.228.17
> [   32.216824] lacp0: Removing ARP target 218.160.255.255
> [   32.222646] lacp0: Removing ARP target 185.170.136.184
> [   32.228496] lacp0: invalid ARP target 255.255.255.255 specified for removal
> [   32.236294] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
> [   32.243987] lacp0: Removing ARP target 56.125.228.17
> [   32.249625] lacp0: Removing ARP target 218.160.255.255
> [   32.255432] lacp0: Removing ARP target 15.157.233.184
> [   32.261165] lacp0: invalid ARP target 255.255.255.255 specified for removal
> [   32.268939] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
> [   32.276632] lacp0: Removing ARP target 16.0.0.0
> [   32.281755] lacp0: Removing ARP target 218.160.255.255
> [   32.287567] lacp0: Removing ARP target 72.125.228.17
> [   32.293165] lacp0: Removing ARP target 218.160.255.255
> [   32.298970] lacp0: Removing ARP target 8.125.228.17
> [   32.304458] lacp0: Removing ARP target 218.160.255.255
> 
> None of these were actually specified as ARP targets, and the driver does
> seem to clean up the mess okay, but it's rather noisy and confusing, leaks
> values to userspace, and the 255.255.255.255 spew shows up even when debug
> prints are disabled.
> 
> The fix: just zero out arp_target at init time.
> 
> While we're in here, init arp_all_targets_value in the right place.
> 

Looks good.  Thanks, Jarod!

Acked-by: Andy Gospodarek 

> Fixes: dc9c4d0fe023 ("bonding: reduce scope of some global variables")
> CC: Mahesh Bandewar 
> CC: Jay Vosburgh 
> CC: Veaceslav Falico 
> CC: Andy Gospodarek 
> CC: net...@vger.kernel.org
> CC: sta...@vger.kernel.org
> Signed-off-by: Jarod Wilson 
> ---
>  drivers/net/bonding/bond_main.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 2be78807fd6e..73313318399c 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -4271,10 +4271,10 @@ static int bond_check_params(struct bond_params 
> *params)
>   int arp_validate_value, fail_over_mac_value, primary_reselect_value, i;
>   struct bond_opt_value newval;
>   const struct bond_opt_value *valptr;
> - int arp_all_targets_value;
> + int arp_all_targets_value = 0;
>   u16 ad_actor_sys_prio = 0;
>   u16 ad_user_port_key = 0;
> - __be32 arp_target[BOND_MAX_ARP_TARGETS];
> + __be32 arp_target[BOND_MAX_ARP_TARGETS] = { 0 };
>   int arp_ip_count;
>   int bond_mode   = BOND_MODE_ROUNDROBIN;
>   int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
> @@ -4501,7 +4501,6 @@ static int bond_check_params(struct bond_params *params)
>   arp_validate_value = 0;
>   }
>  
> - arp_all_targets_value = 0;
>   if (arp_all_targets) {
>   bond_opt_initstr(, arp_all_targets);
>   valptr = bond_opt_parse(bond_opt_get(BOND_OPT_ARP_ALL_TARGETS),
> -- 
> 2.12.1
> 

Re: [RFC PATCH v2 08/17] cgroup: Move debug cgroup to its own file

[Tejun Heo]

Hello, Waiman.

On Thu, May 18, 2017 at 11:52:18AM -0400, Waiman Long wrote:
> The controller name is "debug" and so it is obvious what this controller
> is for. However, the config prompt "Example controller" is indeed vague

Yeah but it also shows up as an integral part of stable interface
rather than e.g. /sys/kernel/debug.  This isn't of any interest to
people who aren't developing cgroup core code.  There is no reason to
risk growing dependencies on it.

> in meaning. So we can make the prompt more descriptive here. As for the
> boot param, are you saying something like "cgroup_debug" has to be
> specified in the command line even if CGROUP_DEBUG config is there for
> the debug controller to be enabled? I am fine with that if you think it
> is necessary.

Yeah, I think that'd be a good idea.  cgroup_debug should do.  While
at it, can you also please make CGROUP_DEBUG depend on DEBUG_KERNEL?

Thanks.

-- 
tejun

Re: [RFC PATCH v2 08/17] cgroup: Move debug cgroup to its own file

[Tejun Heo]

Hello, Waiman.

On Thu, May 18, 2017 at 11:52:18AM -0400, Waiman Long wrote:
> The controller name is "debug" and so it is obvious what this controller
> is for. However, the config prompt "Example controller" is indeed vague

Yeah but it also shows up as an integral part of stable interface
rather than e.g. /sys/kernel/debug.  This isn't of any interest to
people who aren't developing cgroup core code.  There is no reason to
risk growing dependencies on it.

> in meaning. So we can make the prompt more descriptive here. As for the
> boot param, are you saying something like "cgroup_debug" has to be
> specified in the command line even if CGROUP_DEBUG config is there for
> the debug controller to be enabled? I am fine with that if you think it
> is necessary.

Yeah, I think that'd be a good idea.  cgroup_debug should do.  While
at it, can you also please make CGROUP_DEBUG depend on DEBUG_KERNEL?

Thanks.

-- 
tejun

Re: [PATCH v4 15/27] fs: retrofit old error reporting API onto new infrastructure

[Jeff Layton]

On Mon, 2017-05-15 at 12:42 +0200, Jan Kara wrote:
> On Tue 09-05-17 11:49:18, Jeff Layton wrote:
> > Now that we have a better way to store and report errors that occur
> > during writeback, we need to convert the existing codebase to use it. We
> > could just adapt all of the filesystem code and related infrastructure
> > to the new API, but that's a lot of churn.
> > 
> > When it comes to setting errors in the mapping, filemap_set_wb_error is
> > a drop-in replacement for mapping_set_error. Turn that function into a
> > simple wrapper around the new one.
> > 
> > Because we want to ensure that writeback errors are always reported at
> > fsync time, inject filemap_report_wb_error calls much closer to the
> > syscall boundary, in call_fsync.
> > 
> > For fsync calls (and things like the nfsd equivalent), we either return
> > the error that the fsync operation returns, or the one returned by
> > filemap_report_wb_error. In both cases, we advance the file->f_wb_err to
> > the latest value. This allows us to provide new fsync semantics that
> > will return errors that may have occurred previously and been viewed
> > via other file descriptors.
> > 
> > The final piece of the puzzle is what to do about filemap_check_errors
> > calls that are being called directly or via filemap_* functions. Here,
> > we must take a little "creative license".
> > 
> > Since we now handle advancing the file->f_wb_err value at the generic
> > filesystem layer, we no longer need those callers to clear errors out
> > of the mapping or advance an errseq_t.
> > 
> > A lot of the existing codebase relies on being getting an error back
> > from those functions when there is a writeback problem, so we do still
> > want to have them report writeback errors somehow.
> > 
> > When reporting writeback errors, we will always report errors that have
> > occurred since a particular point in time. With the old writeback error
> > reporting, the time we used was "since it was last tested/cleared" which
> > is entirely arbitrary and potentially racy. Now, we can at least report
> > the latest error that has occurred since an arbitrary point in time
> > (represented as a sampled errseq_t value).
> > 
> > In the case where we don't have a struct file to work with, this patch
> > just has the wrappers sample the current mapping->wb_err value, and use
> > that as an arbitrary point from which to check for errors.
> 
> I think this is really dangerous and we shouldn't do this. You are quite
> likely to lose IO errors in such calls because you will ignore all errors
> that happened during previous background writeback or even for IO that
> managed to complete before we called filemap_fdatawait(). Maybe we need to
> keep the original set-clear-bit IO error reporting for these cases, until
> we can convert them to fdatawait_range_since()?
> 
> > That's probably not "correct" in all cases, particularly in the case of
> > something like filemap_fdatawait, but I'm not sure it's any worse than
> > what we already have, and this gives us a basis from which to work.
> > 
> > A lot of those callers will likely want to change to a model where they
> > sample the errseq_t much earlier (perhaps when starting a transaction),
> > store it in an appropriate place and then use that value later when
> > checking to see if an error occurred.
> > 
> > That will almost certainly take some involvement from other subsystem
> > maintainers. I'm quite open to adding new API functions to help enable
> > this if that would be helpful, but I don't really want to do that until
> > I better understand what's needed.
> > 
> > Signed-off-by: Jeff Layton 
> 
> ...
> 
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 5f7317875a67..7ce13281925f 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -187,6 +187,7 @@ static int f2fs_do_sync_file(struct file *file, loff_t 
> > start, loff_t end,
> > .nr_to_write = LONG_MAX,
> > .for_reclaim = 0,
> > };
> > +   errseq_t since = READ_ONCE(file->f_wb_err);
> >  
> > if (unlikely(f2fs_readonly(inode->i_sb)))
> > return 0;
> > @@ -265,6 +266,8 @@ static int f2fs_do_sync_file(struct file *file, loff_t 
> > start, loff_t end,
> > }
> >  
> > ret = wait_on_node_pages_writeback(sbi, ino);
> > +   if (ret == 0)
> > +   ret = filemap_check_wb_error(NODE_MAPPING(sbi), since);
> > if (ret)
> > goto out;
> 
> So this conversion looks wrong and actually points to a larger issue with
> the scheme. The problem is there are two mappings that come into play here
> - file_inode(file)->i_mapping which is the data mapping and
> NODE_MAPPING(sbi) which is the metadata mapping (and this is not a problem
> specific to f2fs. For example ext2 also uses this scheme where block
> devices' mapping is the metadata mapping). And we need to merge error
> information from these two mappings so for the stamping scheme to work,
> we'd need two stamps stored in 

Re: [PATCH v4 15/27] fs: retrofit old error reporting API onto new infrastructure

[Jeff Layton]

On Mon, 2017-05-15 at 12:42 +0200, Jan Kara wrote:
> On Tue 09-05-17 11:49:18, Jeff Layton wrote:
> > Now that we have a better way to store and report errors that occur
> > during writeback, we need to convert the existing codebase to use it. We
> > could just adapt all of the filesystem code and related infrastructure
> > to the new API, but that's a lot of churn.
> > 
> > When it comes to setting errors in the mapping, filemap_set_wb_error is
> > a drop-in replacement for mapping_set_error. Turn that function into a
> > simple wrapper around the new one.
> > 
> > Because we want to ensure that writeback errors are always reported at
> > fsync time, inject filemap_report_wb_error calls much closer to the
> > syscall boundary, in call_fsync.
> > 
> > For fsync calls (and things like the nfsd equivalent), we either return
> > the error that the fsync operation returns, or the one returned by
> > filemap_report_wb_error. In both cases, we advance the file->f_wb_err to
> > the latest value. This allows us to provide new fsync semantics that
> > will return errors that may have occurred previously and been viewed
> > via other file descriptors.
> > 
> > The final piece of the puzzle is what to do about filemap_check_errors
> > calls that are being called directly or via filemap_* functions. Here,
> > we must take a little "creative license".
> > 
> > Since we now handle advancing the file->f_wb_err value at the generic
> > filesystem layer, we no longer need those callers to clear errors out
> > of the mapping or advance an errseq_t.
> > 
> > A lot of the existing codebase relies on being getting an error back
> > from those functions when there is a writeback problem, so we do still
> > want to have them report writeback errors somehow.
> > 
> > When reporting writeback errors, we will always report errors that have
> > occurred since a particular point in time. With the old writeback error
> > reporting, the time we used was "since it was last tested/cleared" which
> > is entirely arbitrary and potentially racy. Now, we can at least report
> > the latest error that has occurred since an arbitrary point in time
> > (represented as a sampled errseq_t value).
> > 
> > In the case where we don't have a struct file to work with, this patch
> > just has the wrappers sample the current mapping->wb_err value, and use
> > that as an arbitrary point from which to check for errors.
> 
> I think this is really dangerous and we shouldn't do this. You are quite
> likely to lose IO errors in such calls because you will ignore all errors
> that happened during previous background writeback or even for IO that
> managed to complete before we called filemap_fdatawait(). Maybe we need to
> keep the original set-clear-bit IO error reporting for these cases, until
> we can convert them to fdatawait_range_since()?
> 
> > That's probably not "correct" in all cases, particularly in the case of
> > something like filemap_fdatawait, but I'm not sure it's any worse than
> > what we already have, and this gives us a basis from which to work.
> > 
> > A lot of those callers will likely want to change to a model where they
> > sample the errseq_t much earlier (perhaps when starting a transaction),
> > store it in an appropriate place and then use that value later when
> > checking to see if an error occurred.
> > 
> > That will almost certainly take some involvement from other subsystem
> > maintainers. I'm quite open to adding new API functions to help enable
> > this if that would be helpful, but I don't really want to do that until
> > I better understand what's needed.
> > 
> > Signed-off-by: Jeff Layton 
> 
> ...
> 
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 5f7317875a67..7ce13281925f 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -187,6 +187,7 @@ static int f2fs_do_sync_file(struct file *file, loff_t 
> > start, loff_t end,
> > .nr_to_write = LONG_MAX,
> > .for_reclaim = 0,
> > };
> > +   errseq_t since = READ_ONCE(file->f_wb_err);
> >  
> > if (unlikely(f2fs_readonly(inode->i_sb)))
> > return 0;
> > @@ -265,6 +266,8 @@ static int f2fs_do_sync_file(struct file *file, loff_t 
> > start, loff_t end,
> > }
> >  
> > ret = wait_on_node_pages_writeback(sbi, ino);
> > +   if (ret == 0)
> > +   ret = filemap_check_wb_error(NODE_MAPPING(sbi), since);
> > if (ret)
> > goto out;
> 
> So this conversion looks wrong and actually points to a larger issue with
> the scheme. The problem is there are two mappings that come into play here
> - file_inode(file)->i_mapping which is the data mapping and
> NODE_MAPPING(sbi) which is the metadata mapping (and this is not a problem
> specific to f2fs. For example ext2 also uses this scheme where block
> devices' mapping is the metadata mapping). And we need to merge error
> information from these two mappings so for the stamping scheme to work,
> we'd need two stamps stored in struct file. One 

Re: [PATCH] Btrfs: work around maybe-uninitialized warning

[Arnd Bergmann]

On Fri, May 19, 2017 at 8:10 PM, Liu Bo  wrote:
> On Thu, May 18, 2017 at 03:33:29PM +0200, Arnd Bergmann wrote:
>> A rewrite of btrfs_submit_direct_hook appears to have introduced a warning:
>>
>> fs/btrfs/inode.c: In function 'btrfs_submit_direct_hook':
>> fs/btrfs/inode.c:8467:14: error: 'bio' may be used uninitialized in this 
>> function [-Werror=maybe-uninitialized]
>>
>> Where the 'bio' variable was previously initialized unconditionally, it
>> is now set in the "while (submit_len > 0)" loop that would never execute
>> if submit_len is zero.
>>
>> Assuming this cannot happen in practice, we can avoid the warning
>> by simply replacing the while{} loop with a do{}while() loop so
>> the compiler knows that it will always be entered at least once.
>>
>
> Thanks for the fix.  I think it's a false positve one and I've updated it in 
> v2
> with a 'struct bio *bio = NULL' to make compiler happy, could you please help
> reveiw it?

Right, it is a false positive and adding the =NULL initialization shuts up the
warning. The reason my patch used a different approach is to make the
code more robust, see https://rusty.ozlabs.org/?p=232

Generally speaking initializing a local variable to an illegal value, and later
using the variable without a check for that original value is error-prone.
Even though the code is correct at the moment, someone else might
modify it later. My first (broken) solution avoided this by checking for
the condition that led to the warning, my newer solution is nicer as it
makes it much clearer to the reader what is going on, compared to
the NULL initialization that does not help readability but makes
it slightly harder to understand why you wrote the code specifically that
way.

   Arnd

Re: [PATCH] Btrfs: work around maybe-uninitialized warning

[Arnd Bergmann]

On Fri, May 19, 2017 at 8:10 PM, Liu Bo  wrote:
> On Thu, May 18, 2017 at 03:33:29PM +0200, Arnd Bergmann wrote:
>> A rewrite of btrfs_submit_direct_hook appears to have introduced a warning:
>>
>> fs/btrfs/inode.c: In function 'btrfs_submit_direct_hook':
>> fs/btrfs/inode.c:8467:14: error: 'bio' may be used uninitialized in this 
>> function [-Werror=maybe-uninitialized]
>>
>> Where the 'bio' variable was previously initialized unconditionally, it
>> is now set in the "while (submit_len > 0)" loop that would never execute
>> if submit_len is zero.
>>
>> Assuming this cannot happen in practice, we can avoid the warning
>> by simply replacing the while{} loop with a do{}while() loop so
>> the compiler knows that it will always be entered at least once.
>>
>
> Thanks for the fix.  I think it's a false positve one and I've updated it in 
> v2
> with a 'struct bio *bio = NULL' to make compiler happy, could you please help
> reveiw it?

Right, it is a false positive and adding the =NULL initialization shuts up the
warning. The reason my patch used a different approach is to make the
code more robust, see https://rusty.ozlabs.org/?p=232

Generally speaking initializing a local variable to an illegal value, and later
using the variable without a check for that original value is error-prone.
Even though the code is correct at the moment, someone else might
modify it later. My first (broken) solution avoided this by checking for
the condition that led to the warning, my newer solution is nicer as it
makes it much clearer to the reader what is going on, compared to
the NULL initialization that does not help readability but makes
it slightly harder to understand why you wrote the code specifically that
way.

   Arnd

Re: next-20170515: WARNING: CPU: 0 PID: 1 at arch/x86/mm/dump_pagetables.c:236 note_page+0x630/0x7e0

[Andy Lutomirski]

On Fri, May 19, 2017 at 12:16 PM, Kees Cook  wrote:
> On Fri, May 19, 2017 at 11:27 AM, Andy Lutomirski  wrote:
>> One thing I've pondered: can we make some debugging mode (kmemleak,
>> perhaps?) check that freed memory is RW at the time it's freed?  I
>> once wrote some buggy code that freed an R page and caused an OOPS
>> much later, and this bug here seems likely to be some code that frees
>> RWX memory.
>
> Which begs for even more checks: nothing should ever make a page RWX.
> Either R, RW, or RX only... (or X too I guess, in the future).

I could see pages being RWX temporarily during boot.  OTOH if we ban
RWX outright (after very early boot, anyway), then catching code that
messes up and leaves pages RWX gets much easier.

--Andy

Re: next-20170515: WARNING: CPU: 0 PID: 1 at arch/x86/mm/dump_pagetables.c:236 note_page+0x630/0x7e0

[Andy Lutomirski]

On Fri, May 19, 2017 at 12:16 PM, Kees Cook  wrote:
> On Fri, May 19, 2017 at 11:27 AM, Andy Lutomirski  wrote:
>> One thing I've pondered: can we make some debugging mode (kmemleak,
>> perhaps?) check that freed memory is RW at the time it's freed?  I
>> once wrote some buggy code that freed an R page and caused an OOPS
>> much later, and this bug here seems likely to be some code that frees
>> RWX memory.
>
> Which begs for even more checks: nothing should ever make a page RWX.
> Either R, RW, or RX only... (or X too I guess, in the future).

I could see pages being RWX temporarily during boot.  OTOH if we ban
RWX outright (after very early boot, anyway), then catching code that
messes up and leaves pages RWX gets much easier.

--Andy

Re: next-20170515: WARNING: CPU: 0 PID: 1 at arch/x86/mm/dump_pagetables.c:236 note_page+0x630/0x7e0

[Kees Cook]

On Fri, May 19, 2017 at 11:27 AM, Andy Lutomirski  wrote:
> One thing I've pondered: can we make some debugging mode (kmemleak,
> perhaps?) check that freed memory is RW at the time it's freed?  I
> once wrote some buggy code that freed an R page and caused an OOPS
> much later, and this bug here seems likely to be some code that frees
> RWX memory.

Which begs for even more checks: nothing should ever make a page RWX.
Either R, RW, or RX only... (or X too I guess, in the future).

-Kees

-- 
Kees Cook
Pixel Security

Re: next-20170515: WARNING: CPU: 0 PID: 1 at arch/x86/mm/dump_pagetables.c:236 note_page+0x630/0x7e0

[Kees Cook]

On Fri, May 19, 2017 at 11:27 AM, Andy Lutomirski  wrote:
> One thing I've pondered: can we make some debugging mode (kmemleak,
> perhaps?) check that freed memory is RW at the time it's freed?  I
> once wrote some buggy code that freed an R page and caused an OOPS
> much later, and this bug here seems likely to be some code that frees
> RWX memory.

Which begs for even more checks: nothing should ever make a page RWX.
Either R, RW, or RX only... (or X too I guess, in the future).

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 4/6] arch/sparc: Enable queued rwlocks for SPARC

[David Miller]

From: Peter Zijlstra 
Date: Fri, 19 May 2017 11:03:02 +0200

> On Thu, May 18, 2017 at 10:31:13PM -0400, David Miller wrote:
>> From: Babu Moger 
>> Date: Thu, 18 May 2017 18:36:08 -0600
>> 
>> > @@ -82,6 +82,7 @@ config SPARC64
>> >select HAVE_ARCH_AUDITSYSCALL
>> >select ARCH_SUPPORTS_ATOMIC_RMW
>> >select HAVE_NMI
>> > +  select ARCH_USE_QUEUED_RWLOCKS
>> >  
>> 
>> If you are selecting this on SPARC64 all the time, then:
>> 
>> > @@ -94,6 +94,7 @@ static inline void arch_spin_lock_flags(arch_spinlock_t 
>> > *lock, unsigned long fla
>> >: "memory");
>> >  }
>> >  
>> > +#ifndef CONFIG_QUEUED_RWLOCKS
>> >  /* Multi-reader locks, these are much saner than the 32-bit Sparc ones... 
>> > */
>> 
>> You can remove this segment of ifdef'd code altogether since it is in
>> a sparc64 specific header file.
> 
> 
> So IIRC Sparc v8 only has that single byte load-and-set (or swap)
> instruction, right? That means you can only make test-and-set spinlocks
> and then have to build the world on top of that.
> 
> I don't see qrwlock -- which assumes the spinlock implementation is fair
> -- making much sense for that.
> 
> Also, IIRC Sparc-v8 didn't really have very big SMP systems, those came
> with v9. And qspinlock only really makes sense on the bigger systems
> (not to mention that building the qspinlock on top of atomic operations
> build on test-and-set spinlocks just seems extremely dysfunctional).
> 
> 
> In any case, I think what I'm saying is that it makes sense to make this
> a Sparcv9 only feature.

I agree with you, there is no reason to try and support
queued locks on 32-bit sparc.

However, I don't see what any of this has to do with the feedback
I was giving the patch author :-)

Re: [PATCH 4/6] arch/sparc: Enable queued rwlocks for SPARC

[David Miller]

From: Peter Zijlstra 
Date: Fri, 19 May 2017 11:03:02 +0200

> On Thu, May 18, 2017 at 10:31:13PM -0400, David Miller wrote:
>> From: Babu Moger 
>> Date: Thu, 18 May 2017 18:36:08 -0600
>> 
>> > @@ -82,6 +82,7 @@ config SPARC64
>> >select HAVE_ARCH_AUDITSYSCALL
>> >select ARCH_SUPPORTS_ATOMIC_RMW
>> >select HAVE_NMI
>> > +  select ARCH_USE_QUEUED_RWLOCKS
>> >  
>> 
>> If you are selecting this on SPARC64 all the time, then:
>> 
>> > @@ -94,6 +94,7 @@ static inline void arch_spin_lock_flags(arch_spinlock_t 
>> > *lock, unsigned long fla
>> >: "memory");
>> >  }
>> >  
>> > +#ifndef CONFIG_QUEUED_RWLOCKS
>> >  /* Multi-reader locks, these are much saner than the 32-bit Sparc ones... 
>> > */
>> 
>> You can remove this segment of ifdef'd code altogether since it is in
>> a sparc64 specific header file.
> 
> 
> So IIRC Sparc v8 only has that single byte load-and-set (or swap)
> instruction, right? That means you can only make test-and-set spinlocks
> and then have to build the world on top of that.
> 
> I don't see qrwlock -- which assumes the spinlock implementation is fair
> -- making much sense for that.
> 
> Also, IIRC Sparc-v8 didn't really have very big SMP systems, those came
> with v9. And qspinlock only really makes sense on the bigger systems
> (not to mention that building the qspinlock on top of atomic operations
> build on test-and-set spinlocks just seems extremely dysfunctional).
> 
> 
> In any case, I think what I'm saying is that it makes sense to make this
> a Sparcv9 only feature.

I agree with you, there is no reason to try and support
queued locks on 32-bit sparc.

However, I don't see what any of this has to do with the feedback
I was giving the patch author :-)

[PATCH v8 1/5] firmware: add extensible driver data params

[Luis R. Rodriguez]

As the firmware API evolves we keep extending functions with more arguments.
Stop this nonsense by proving an extensible data structure which can be used
to represent both user parameters and private internal parameters.

We introduce 3 data structures:

  o struct driver_data_req_params  - used for user specified parameters
  o struct driver_data_priv_params - used for internal use only
  o struct driver_data_params - stiches both of the the above together,
only for internal use

This starts off by just making the existing APIs use the new data
structures, it will make subsequent changes easier to review which will
be adding new flexible APIs.

A side consequences is get to replace all the old internal "firmware
behavior  options" flags with enums we properly document, remove the
blinding #ifdefs, and compartamentlize the userhelper fallback code
more appropriately unde CONFIG_FW_LOADER_USER_HELPER_FALLBACK.

This commit should introduces no functional changes (TM).

Signed-off-by: Luis R. Rodriguez 
---
 drivers/base/firmware_class.c | 331 --
 include/linux/driver_data.h   |  88 +++
 2 files changed, 345 insertions(+), 74 deletions(-)
 create mode 100644 include/linux/driver_data.h

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index b9f907eedbf7..db7c0bc0ed98 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -40,6 +41,149 @@ MODULE_AUTHOR("Manuel Estrada Sainz");
 MODULE_DESCRIPTION("Multi purpose firmware loading support");
 MODULE_LICENSE("GPL");
 
+/**
+ * enum driver_data_mode - driver data mode of operation
+ * @DRIVER_DATA_SYNC: used to determine if we should look for the driver data
+ * file immediatley.
+ * @DRIVER_DATA_ASYNC: used to determine if we should schedule the search for
+ * your driver data file to be run at a later time.
+ */
+enum driver_data_mode {
+   DRIVER_DATA_SYNC = 0,
+   DRIVER_DATA_ASYNC,
+};
+
+/**
+ * enum driver_data_priv_reqs - private features only used internally
+ *
+ * @DRIVER_DATA_PRIV_REQ_FALLBACK: specifies that the driver data request
+ * will use a fallback mechanism if the kernel's direct filesystem
+ * lookup failed to find the requested driver data. If the flag
+ * %DRIVER_DATA_PRIV_REQ_FALLBACK is set but the flag
+ * %DRIVER_DATA_PRIV_REQ_FALLBACK_UEVENT is not set, it means the caller
+ * is relying on a custom interface for driver data lookup as a fallback
+ * mechanism. The custom interface is expected to find any found driver
+ * data using the exposed sysfs interface of the firmware_class. If the
+ * custom fallback mechanism is not compatible with the internal caching
+ * mechanism for driver data lookups at resume, it will be disabled.
+ * @DRIVER_DATA_PRIV_REQ_FALLBACK_UEVENT: indicates that the fallback mechanism
+ * this driver data request will rely on will be that of having the kernel
+ * issue a uevent to userspace. Userspace in turn is expected to be
+ * monitoring for uevents for the firmware_class and will use the
+ * exposted sysfs interface to upload the driver data for the caller.
+ * @DRIVER_DATA_PRIV_REQ_NO_CACHE: indicates that the driver data request
+ * should not set up and use the internal caching mechanism to assist
+ * drivers from fetching driver data at resume time after suspend.
+ */
+enum driver_data_priv_reqs {
+   DRIVER_DATA_PRIV_REQ_FALLBACK   = 1 << 0,
+   DRIVER_DATA_PRIV_REQ_FALLBACK_UEVENT= 1 << 1,
+   DRIVER_DATA_PRIV_REQ_NO_CACHE   = 1 << 2,
+};
+
+/**
+ * struct driver_data_priv_params - private driver data parameters
+ * @mode: mode of operation
+ * @priv_reqs: private set of  driver_data_reqs, private requirements for
+ * the driver data request
+ * @alloc_buf: buffer area allocated by the caller so we can place the
+ * respective driver data
+ * @alloc_buf_size: size of the @alloc_buf
+ * @old_async_cb: used only for request_firmware_nowait() since we won't change
+ * all async callbacks to get the return value on failure
+ */
+struct driver_data_priv_params {
+   enum driver_data_mode mode;
+   u64 priv_reqs;
+   void *alloc_buf;
+   size_t alloc_buf_size;
+   void (*old_async_cb)(const struct firmware *driver_data, void *context);
+};
+
+/**
+ * struct driver_data_params
+ * @driver_data: the driver data if found using the requirements specified
+ * in @req_params and @priv_params
+ * @req_params: caller's requirements for the driver data to look for
+ * @priv_params: private requirements for the driver data to look for
+ */
+struct driver_data_params {
+   const struct firmware *driver_data;
+   const struct driver_data_req_params req_params;
+   struct driver_data_priv_params 

[PATCH v8 1/5] firmware: add extensible driver data params

[Luis R. Rodriguez]

As the firmware API evolves we keep extending functions with more arguments.
Stop this nonsense by proving an extensible data structure which can be used
to represent both user parameters and private internal parameters.

We introduce 3 data structures:

  o struct driver_data_req_params  - used for user specified parameters
  o struct driver_data_priv_params - used for internal use only
  o struct driver_data_params - stiches both of the the above together,
only for internal use

This starts off by just making the existing APIs use the new data
structures, it will make subsequent changes easier to review which will
be adding new flexible APIs.

A side consequences is get to replace all the old internal "firmware
behavior  options" flags with enums we properly document, remove the
blinding #ifdefs, and compartamentlize the userhelper fallback code
more appropriately unde CONFIG_FW_LOADER_USER_HELPER_FALLBACK.

This commit should introduces no functional changes (TM).

Signed-off-by: Luis R. Rodriguez 
---
 drivers/base/firmware_class.c | 331 --
 include/linux/driver_data.h   |  88 +++
 2 files changed, 345 insertions(+), 74 deletions(-)
 create mode 100644 include/linux/driver_data.h

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index b9f907eedbf7..db7c0bc0ed98 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -40,6 +41,149 @@ MODULE_AUTHOR("Manuel Estrada Sainz");
 MODULE_DESCRIPTION("Multi purpose firmware loading support");
 MODULE_LICENSE("GPL");
 
+/**
+ * enum driver_data_mode - driver data mode of operation
+ * @DRIVER_DATA_SYNC: used to determine if we should look for the driver data
+ * file immediatley.
+ * @DRIVER_DATA_ASYNC: used to determine if we should schedule the search for
+ * your driver data file to be run at a later time.
+ */
+enum driver_data_mode {
+   DRIVER_DATA_SYNC = 0,
+   DRIVER_DATA_ASYNC,
+};
+
+/**
+ * enum driver_data_priv_reqs - private features only used internally
+ *
+ * @DRIVER_DATA_PRIV_REQ_FALLBACK: specifies that the driver data request
+ * will use a fallback mechanism if the kernel's direct filesystem
+ * lookup failed to find the requested driver data. If the flag
+ * %DRIVER_DATA_PRIV_REQ_FALLBACK is set but the flag
+ * %DRIVER_DATA_PRIV_REQ_FALLBACK_UEVENT is not set, it means the caller
+ * is relying on a custom interface for driver data lookup as a fallback
+ * mechanism. The custom interface is expected to find any found driver
+ * data using the exposed sysfs interface of the firmware_class. If the
+ * custom fallback mechanism is not compatible with the internal caching
+ * mechanism for driver data lookups at resume, it will be disabled.
+ * @DRIVER_DATA_PRIV_REQ_FALLBACK_UEVENT: indicates that the fallback mechanism
+ * this driver data request will rely on will be that of having the kernel
+ * issue a uevent to userspace. Userspace in turn is expected to be
+ * monitoring for uevents for the firmware_class and will use the
+ * exposted sysfs interface to upload the driver data for the caller.
+ * @DRIVER_DATA_PRIV_REQ_NO_CACHE: indicates that the driver data request
+ * should not set up and use the internal caching mechanism to assist
+ * drivers from fetching driver data at resume time after suspend.
+ */
+enum driver_data_priv_reqs {
+   DRIVER_DATA_PRIV_REQ_FALLBACK   = 1 << 0,
+   DRIVER_DATA_PRIV_REQ_FALLBACK_UEVENT= 1 << 1,
+   DRIVER_DATA_PRIV_REQ_NO_CACHE   = 1 << 2,
+};
+
+/**
+ * struct driver_data_priv_params - private driver data parameters
+ * @mode: mode of operation
+ * @priv_reqs: private set of  driver_data_reqs, private requirements for
+ * the driver data request
+ * @alloc_buf: buffer area allocated by the caller so we can place the
+ * respective driver data
+ * @alloc_buf_size: size of the @alloc_buf
+ * @old_async_cb: used only for request_firmware_nowait() since we won't change
+ * all async callbacks to get the return value on failure
+ */
+struct driver_data_priv_params {
+   enum driver_data_mode mode;
+   u64 priv_reqs;
+   void *alloc_buf;
+   size_t alloc_buf_size;
+   void (*old_async_cb)(const struct firmware *driver_data, void *context);
+};
+
+/**
+ * struct driver_data_params
+ * @driver_data: the driver data if found using the requirements specified
+ * in @req_params and @priv_params
+ * @req_params: caller's requirements for the driver data to look for
+ * @priv_params: private requirements for the driver data to look for
+ */
+struct driver_data_params {
+   const struct firmware *driver_data;
+   const struct driver_data_req_params req_params;
+   struct driver_data_priv_params priv_params;
+};
+
+/*

[PATCH v8 3/5] test: add new driver_data load tester

[Luis R. Rodriguez]

This adds a load tester driver test_driver_data a for the new extensible
driver_data loader API, part of firmware_class. This test driver enables
you to build your tests in userspace by exposing knobs of the exported
API to userspace and enables a trigger action to mimic a one time use
of the kernel API. This gives us the flexibility to build test case from
userspace with less kernel changes.

Signed-off-by: Luis R. Rodriguez 
---
 MAINTAINERS |1 +
 lib/Kconfig.debug   |   12 +
 lib/Makefile|1 +
 lib/test_driver_data.c  | 1278 +++
 tools/testing/selftests/firmware/Makefile   |2 +-
 tools/testing/selftests/firmware/config |1 +
 tools/testing/selftests/firmware/driver_data.sh | 1002 ++
 7 files changed, 2296 insertions(+), 1 deletion(-)
 create mode 100644 lib/test_driver_data.c
 create mode 100755 tools/testing/selftests/firmware/driver_data.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index 148d032e9401..a44bdda2ffa9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5232,6 +5232,7 @@ L:linux-kernel@vger.kernel.org
 S: Maintained
 F: Documentation/firmware_class/
 F: drivers/base/firmware*.c
+F: lib/test_driver_data.c
 F: include/linux/firmware.h
 F: include/linux/driver_data.h
 
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f6aece3b8098..4198995ca3f7 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1782,6 +1782,18 @@ config TEST_FIRMWARE
 
  If unsure, say N.
 
+config TEST_DRIVER_DATA
+   tristate "Test driver data loading via driver_data APIs"
+   default n
+   depends on FW_LOADER
+   help
+ This builds the "test_driver_data" module that creates a userspace
+ interface for testing driver data loading using the driver_data API.
+ This can be used to control the triggering of driver data loading
+ without needing an actual real device.
+
+ If unsure, say N.
+
 config TEST_UDELAY
tristate "udelay test driver"
default n
diff --git a/lib/Makefile b/lib/Makefile
index 07fbe6a75692..5a0dcca7097d 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -47,6 +47,7 @@ obj-y += kstrtox.o
 obj-$(CONFIG_TEST_BPF) += test_bpf.o
 obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o
 obj-$(CONFIG_TEST_HASH) += test_hash.o test_siphash.o
+obj-$(CONFIG_TEST_DRIVER_DATA) += test_driver_data.o
 obj-$(CONFIG_TEST_KASAN) += test_kasan.o
 obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
 obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
diff --git a/lib/test_driver_data.c b/lib/test_driver_data.c
new file mode 100644
index ..422ea6289396
--- /dev/null
+++ b/lib/test_driver_data.c
@@ -0,0 +1,1278 @@
+/*
+ * Driver data test interface
+ *
+ * Copyright (C) 2017 Luis R. Rodriguez 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or at your option) any
+ * later version; or, when distributed separately from the Linux kernel or
+ * incorporated into other software packages, subject to the following license:
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of copyleft-next (version 0.3.1 or later) as published
+ * at http://copyleft-next.org/.
+ */
+
+/*
+ * This module provides an interface to trigger and test the driver data API
+ * through a series of configurations and a few triggers. This driver
+ * lacks any extra dependencies, and will not normally be loaded by the
+ * system unless explicitly requested by name. You can also build this
+ * driver into your kernel.
+ *
+ * Although all configurations are already written for and will be supported
+ * for this test driver, ideally we should strive to see what mechanisms we
+ * can put in place to instead automatically generate this sort of test
+ * interface, test cases, and infer results. Its a simple enough interface that
+ * should hopefully enable more exploring in this area.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Used for the fallback default to test against */
+#define TEST_DRIVER_DATA "test-driver_data.bin"
+
+/*
+ * For device allocation / registration
+ */
+static DEFINE_MUTEX(reg_dev_mutex);
+static LIST_HEAD(reg_test_devs);
+
+/*
+ * num_test_devs actually represents the *next* ID of the next
+ * device we will allow to create.
+ */
+int num_test_devs;
+
+/**
+ * test_config - represents configuration for the driver_data API
+ *
+ * @name: the name of the primary driver_data file to look for
+ * @default_name: a fallback example, used to 

[PATCH v8 5/5] iwlwifi: convert to use driver data API

[Luis R. Rodriguez]

The driver data API provides support for looking for firmware
from a specific set of API ranges, so just use that. Since we
free the firmware on the callback immediately after consuming it,
this also takes avantage of that feature.

Signed-off-by: Luis R. Rodriguez 
---
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 91 ++--
 1 file changed, 31 insertions(+), 60 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index 5cfacb0bca84..028854d31f55 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -66,7 +66,7 @@
  */
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 
@@ -206,58 +206,34 @@ static int iwl_alloc_fw_desc(struct iwl_drv *drv, struct 
fw_desc *desc,
return 0;
 }
 
-static void iwl_req_fw_callback(const struct firmware *ucode_raw,
-   void *context);
+static int iwl_req_fw_callback(const struct firmware *ucode_raw, void *context,
+  int unused_error);
 
-static int iwl_request_firmware(struct iwl_drv *drv, bool first)
+static const char *iwl_get_fw_name_pre(struct iwl_drv *drv)
 {
-   const struct iwl_cfg *cfg = drv->trans->cfg;
-   char tag[8];
const char *fw_pre_name;
 
if (drv->trans->cfg->device_family == IWL_DEVICE_FAMILY_8000 &&
CSR_HW_REV_STEP(drv->trans->hw_rev) == SILICON_B_STEP)
-   fw_pre_name = cfg->fw_name_pre_next_step;
+   fw_pre_name = drv->trans->cfg->fw_name_pre_next_step;
else
-   fw_pre_name = cfg->fw_name_pre;
-
-   if (first) {
-   drv->fw_index = cfg->ucode_api_max;
-   sprintf(tag, "%d", drv->fw_index);
-   } else {
-   drv->fw_index--;
-   sprintf(tag, "%d", drv->fw_index);
-   }
-
-   if (drv->fw_index < cfg->ucode_api_min) {
-   IWL_ERR(drv, "no suitable firmware found!\n");
-
-   if (cfg->ucode_api_min == cfg->ucode_api_max) {
-   IWL_ERR(drv, "%s%d is required\n", fw_pre_name,
-   cfg->ucode_api_max);
-   } else {
-   IWL_ERR(drv, "minimum version required: %s%d\n",
-   fw_pre_name,
-   cfg->ucode_api_min);
-   IWL_ERR(drv, "maximum version supported: %s%d\n",
-   fw_pre_name,
-   cfg->ucode_api_max);
-   }
-
-   IWL_ERR(drv,
-   "check 
git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git\n");
-   return -ENOENT;
-   }
+   fw_pre_name = drv->trans->cfg->fw_name_pre;
 
-   snprintf(drv->firmware_name, sizeof(drv->firmware_name), "%s%s.ucode",
-fw_pre_name, tag);
-
-   IWL_DEBUG_INFO(drv, "attempting to load firmware '%s'\n",
-  drv->firmware_name);
+   return fw_pre_name;
+}
 
-   return request_firmware_nowait(THIS_MODULE, 1, drv->firmware_name,
-  drv->trans->dev,
-  GFP_KERNEL, drv, iwl_req_fw_callback);
+static int iwl_request_firmware(struct iwl_drv *drv)
+{
+   const char *name_pre = iwl_get_fw_name_pre(drv);
+   const struct iwl_cfg *cfg = drv->trans->cfg;
+   const struct driver_data_req_params req_params = {
+   DRIVER_DATA_API_CB(iwl_req_fw_callback, drv),
+   DRIVER_DATA_API(cfg->ucode_api_min, cfg->ucode_api_max, 
".ucode"),
+   };
+
+   return driver_data_request_async(name_pre,
+_params,
+drv->trans->dev);
 }
 
 struct fw_img_parsing {
@@ -1259,7 +1235,8 @@ static void _iwl_op_mode_stop(struct iwl_drv *drv)
  * If loaded successfully, copies the firmware into buffers
  * for the card to fetch (via DMA).
  */
-static void iwl_req_fw_callback(const struct firmware *ucode_raw, void 
*context)
+static int iwl_req_fw_callback(const struct firmware *ucode_raw, void *context,
+  int unused_error)
 {
struct iwl_drv *drv = context;
struct iwl_fw *fw = >fw;
@@ -1282,10 +1259,12 @@ static void iwl_req_fw_callback(const struct firmware 
*ucode_raw, void *context)
 
pieces = kzalloc(sizeof(*pieces), GFP_KERNEL);
if (!pieces)
-   goto out_free_fw;
+   return -ENOMEM;
 
-   if (!ucode_raw)
-   goto try_again;
+   if (!ucode_raw) {
+   err = -ENOENT;
+   goto free;
+   }
 
IWL_DEBUG_INFO(drv, "Loaded firmware file '%s' (%zd bytes).\n",
   drv->firmware_name, ucode_raw->size);
@@ 

[PATCH v8 3/5] test: add new driver_data load tester

[Luis R. Rodriguez]

This adds a load tester driver test_driver_data a for the new extensible
driver_data loader API, part of firmware_class. This test driver enables
you to build your tests in userspace by exposing knobs of the exported
API to userspace and enables a trigger action to mimic a one time use
of the kernel API. This gives us the flexibility to build test case from
userspace with less kernel changes.

Signed-off-by: Luis R. Rodriguez 
---
 MAINTAINERS |1 +
 lib/Kconfig.debug   |   12 +
 lib/Makefile|1 +
 lib/test_driver_data.c  | 1278 +++
 tools/testing/selftests/firmware/Makefile   |2 +-
 tools/testing/selftests/firmware/config |1 +
 tools/testing/selftests/firmware/driver_data.sh | 1002 ++
 7 files changed, 2296 insertions(+), 1 deletion(-)
 create mode 100644 lib/test_driver_data.c
 create mode 100755 tools/testing/selftests/firmware/driver_data.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index 148d032e9401..a44bdda2ffa9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5232,6 +5232,7 @@ L:linux-kernel@vger.kernel.org
 S: Maintained
 F: Documentation/firmware_class/
 F: drivers/base/firmware*.c
+F: lib/test_driver_data.c
 F: include/linux/firmware.h
 F: include/linux/driver_data.h
 
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f6aece3b8098..4198995ca3f7 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1782,6 +1782,18 @@ config TEST_FIRMWARE
 
  If unsure, say N.
 
+config TEST_DRIVER_DATA
+   tristate "Test driver data loading via driver_data APIs"
+   default n
+   depends on FW_LOADER
+   help
+ This builds the "test_driver_data" module that creates a userspace
+ interface for testing driver data loading using the driver_data API.
+ This can be used to control the triggering of driver data loading
+ without needing an actual real device.
+
+ If unsure, say N.
+
 config TEST_UDELAY
tristate "udelay test driver"
default n
diff --git a/lib/Makefile b/lib/Makefile
index 07fbe6a75692..5a0dcca7097d 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -47,6 +47,7 @@ obj-y += kstrtox.o
 obj-$(CONFIG_TEST_BPF) += test_bpf.o
 obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o
 obj-$(CONFIG_TEST_HASH) += test_hash.o test_siphash.o
+obj-$(CONFIG_TEST_DRIVER_DATA) += test_driver_data.o
 obj-$(CONFIG_TEST_KASAN) += test_kasan.o
 obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
 obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
diff --git a/lib/test_driver_data.c b/lib/test_driver_data.c
new file mode 100644
index ..422ea6289396
--- /dev/null
+++ b/lib/test_driver_data.c
@@ -0,0 +1,1278 @@
+/*
+ * Driver data test interface
+ *
+ * Copyright (C) 2017 Luis R. Rodriguez 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or at your option) any
+ * later version; or, when distributed separately from the Linux kernel or
+ * incorporated into other software packages, subject to the following license:
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of copyleft-next (version 0.3.1 or later) as published
+ * at http://copyleft-next.org/.
+ */
+
+/*
+ * This module provides an interface to trigger and test the driver data API
+ * through a series of configurations and a few triggers. This driver
+ * lacks any extra dependencies, and will not normally be loaded by the
+ * system unless explicitly requested by name. You can also build this
+ * driver into your kernel.
+ *
+ * Although all configurations are already written for and will be supported
+ * for this test driver, ideally we should strive to see what mechanisms we
+ * can put in place to instead automatically generate this sort of test
+ * interface, test cases, and infer results. Its a simple enough interface that
+ * should hopefully enable more exploring in this area.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Used for the fallback default to test against */
+#define TEST_DRIVER_DATA "test-driver_data.bin"
+
+/*
+ * For device allocation / registration
+ */
+static DEFINE_MUTEX(reg_dev_mutex);
+static LIST_HEAD(reg_test_devs);
+
+/*
+ * num_test_devs actually represents the *next* ID of the next
+ * device we will allow to create.
+ */
+int num_test_devs;
+
+/**
+ * test_config - represents configuration for the driver_data API
+ *
+ * @name: the name of the primary driver_data file to look for
+ * @default_name: a fallback example, used to test the optional callback
+ * 

[PATCH v8 5/5] iwlwifi: convert to use driver data API

[Luis R. Rodriguez]

The driver data API provides support for looking for firmware
from a specific set of API ranges, so just use that. Since we
free the firmware on the callback immediately after consuming it,
this also takes avantage of that feature.

Signed-off-by: Luis R. Rodriguez 
---
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 91 ++--
 1 file changed, 31 insertions(+), 60 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c 
b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index 5cfacb0bca84..028854d31f55 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -66,7 +66,7 @@
  */
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 
@@ -206,58 +206,34 @@ static int iwl_alloc_fw_desc(struct iwl_drv *drv, struct 
fw_desc *desc,
return 0;
 }
 
-static void iwl_req_fw_callback(const struct firmware *ucode_raw,
-   void *context);
+static int iwl_req_fw_callback(const struct firmware *ucode_raw, void *context,
+  int unused_error);
 
-static int iwl_request_firmware(struct iwl_drv *drv, bool first)
+static const char *iwl_get_fw_name_pre(struct iwl_drv *drv)
 {
-   const struct iwl_cfg *cfg = drv->trans->cfg;
-   char tag[8];
const char *fw_pre_name;
 
if (drv->trans->cfg->device_family == IWL_DEVICE_FAMILY_8000 &&
CSR_HW_REV_STEP(drv->trans->hw_rev) == SILICON_B_STEP)
-   fw_pre_name = cfg->fw_name_pre_next_step;
+   fw_pre_name = drv->trans->cfg->fw_name_pre_next_step;
else
-   fw_pre_name = cfg->fw_name_pre;
-
-   if (first) {
-   drv->fw_index = cfg->ucode_api_max;
-   sprintf(tag, "%d", drv->fw_index);
-   } else {
-   drv->fw_index--;
-   sprintf(tag, "%d", drv->fw_index);
-   }
-
-   if (drv->fw_index < cfg->ucode_api_min) {
-   IWL_ERR(drv, "no suitable firmware found!\n");
-
-   if (cfg->ucode_api_min == cfg->ucode_api_max) {
-   IWL_ERR(drv, "%s%d is required\n", fw_pre_name,
-   cfg->ucode_api_max);
-   } else {
-   IWL_ERR(drv, "minimum version required: %s%d\n",
-   fw_pre_name,
-   cfg->ucode_api_min);
-   IWL_ERR(drv, "maximum version supported: %s%d\n",
-   fw_pre_name,
-   cfg->ucode_api_max);
-   }
-
-   IWL_ERR(drv,
-   "check 
git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git\n");
-   return -ENOENT;
-   }
+   fw_pre_name = drv->trans->cfg->fw_name_pre;
 
-   snprintf(drv->firmware_name, sizeof(drv->firmware_name), "%s%s.ucode",
-fw_pre_name, tag);
-
-   IWL_DEBUG_INFO(drv, "attempting to load firmware '%s'\n",
-  drv->firmware_name);
+   return fw_pre_name;
+}
 
-   return request_firmware_nowait(THIS_MODULE, 1, drv->firmware_name,
-  drv->trans->dev,
-  GFP_KERNEL, drv, iwl_req_fw_callback);
+static int iwl_request_firmware(struct iwl_drv *drv)
+{
+   const char *name_pre = iwl_get_fw_name_pre(drv);
+   const struct iwl_cfg *cfg = drv->trans->cfg;
+   const struct driver_data_req_params req_params = {
+   DRIVER_DATA_API_CB(iwl_req_fw_callback, drv),
+   DRIVER_DATA_API(cfg->ucode_api_min, cfg->ucode_api_max, 
".ucode"),
+   };
+
+   return driver_data_request_async(name_pre,
+_params,
+drv->trans->dev);
 }
 
 struct fw_img_parsing {
@@ -1259,7 +1235,8 @@ static void _iwl_op_mode_stop(struct iwl_drv *drv)
  * If loaded successfully, copies the firmware into buffers
  * for the card to fetch (via DMA).
  */
-static void iwl_req_fw_callback(const struct firmware *ucode_raw, void 
*context)
+static int iwl_req_fw_callback(const struct firmware *ucode_raw, void *context,
+  int unused_error)
 {
struct iwl_drv *drv = context;
struct iwl_fw *fw = >fw;
@@ -1282,10 +1259,12 @@ static void iwl_req_fw_callback(const struct firmware 
*ucode_raw, void *context)
 
pieces = kzalloc(sizeof(*pieces), GFP_KERNEL);
if (!pieces)
-   goto out_free_fw;
+   return -ENOMEM;
 
-   if (!ucode_raw)
-   goto try_again;
+   if (!ucode_raw) {
+   err = -ENOENT;
+   goto free;
+   }
 
IWL_DEBUG_INFO(drv, "Loaded firmware file '%s' (%zd bytes).\n",
   drv->firmware_name, ucode_raw->size);
@@ -1448,9 +1427,6 @@ 

[PATCH v8 0/5] firmware: add driver data API

[Luis R. Rodriguez]

Greg,

Even though using copyleft-next was fine by Linus [0], AKASHI had brought that
he expected to see an "or" clause on the license declarations when using
copyleft-next. This was the only pending issue from the last v7 series [1]. To
be safe I re-touched the subject and Alan and Ted brought up sufficient reasons
for preferring the "or" language [2], as such this series goes with the "or"
language embraced so that it is even clearer than before that GPLv2 applies
when using copyleft-next on the Linux kernel.

This series depends on the few other UMH fallback lock changes I had submitted
earlier this month [3], and those remain without any noted issues. As usual,
all pending changes for this series are available on my linux-next tree on the
20170519-driver-data branch [4], this series was rebased on next-20170519.

Please let me know if there are any issues or questions.

[0] 
https://lkml.kernel.org/r/CA+55aFyhxcvD+q7tp+-yrSFDKfR0mOHgyEAe=f_94aklsou...@mail.gmail.com
[1] https://lkml.kernel.org/r/20170502084914.23588-1-mcg...@kernel.org
[2] 
https://lkml.kernel.org/r/CAB=ne6vnftr-or9ilft3lnbp4m35p0nfegxnprnawvbbaem...@mail.gmail.com
[3] https://lkml.kernel.org/r/20170502083107.23418-1-mcg...@kernel.org
[4] 
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20170519-driver-data

Luis R. Rodriguez (5):
  firmware: add extensible driver data params
  firmware: add extensible driver data API
  test: add new driver_data load tester
  firmware: document the extensible driver data API
  iwlwifi: convert to use driver data API

 Documentation/driver-api/firmware/driver_data.rst  |  167 +++
 Documentation/driver-api/firmware/index.rst|1 +
 Documentation/driver-api/firmware/introduction.rst |   16 +
 .../driver-api/firmware/request_firmware.rst   |2 +
 MAINTAINERS|4 +-
 drivers/base/firmware_class.c  |  751 ++--
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c   |   91 +-
 include/linux/driver_data.h|  266 
 include/linux/firmware.h   |2 +
 lib/Kconfig.debug  |   12 +
 lib/Makefile   |1 +
 lib/test_driver_data.c | 1278 
 tools/testing/selftests/firmware/Makefile  |2 +-
 tools/testing/selftests/firmware/config|1 +
 tools/testing/selftests/firmware/driver_data.sh| 1002 +++
 15 files changed, 3460 insertions(+), 136 deletions(-)
 create mode 100644 Documentation/driver-api/firmware/driver_data.rst
 create mode 100644 include/linux/driver_data.h
 create mode 100644 lib/test_driver_data.c
 create mode 100755 tools/testing/selftests/firmware/driver_data.sh

-- 
2.11.0

[PATCH v8 2/5] firmware: add extensible driver data API

[Luis R. Rodriguez]

The firmware API does not scale well: when new features are added we
either add a new exported symbol or extend the arguments of existing
routines. For the later case this means we need to traverse the kernel
with a slew of collateral evolutions to adjust old driver users. The
firmware API is also now being used for things outside of the scope of
what typically would be considered "firmware". There are other
subsystems which would like to make use of the firmware APIs for similar
things and its clearly not firmware, but have different requirements
and criteria which they'd like to be met for the requested file.

An extensible API is in order:

The driver data API accepts that there are only two types of requests:

a) synchronous requests
b) asynchronous requests

Both requests may have a different requirements which must be met. These
requirements can be described in the struct driver_data_req_params.
This struct is expected to be extended over time to support different
requirements as the kernel evolves.

After a bit of hard work the new interface has been wrapped onto the
functionality. The fallback mechanism has been kept out of the new API
currently because it requires just a bit more grooming and documentation
given new considerations and requirements.  Adding support for it will
be rather easy now that the new API sits ontop of the old one. The
request_firmware_into_buf() API also is not enabled on the new API but
it is rather easy to do so -- this call has no current existing users
upstream though. Support will be provided once we add a respective
series of test cases against it and find a proper upstream user for it.

The flexible API also adds a few new bells and whistles:

- By default the kernel will free the driver data file for you after
  your callbacks are called, you however are allowed to request that
  you wish to keep the driver data file on the requirements params. The
  new driver data API is able to free the driver data file for you by
  requiring a consumer callback for the driver data file.
- Allows both asynchronous and synchronous request to specify that
  driver data files are optional. With the old APIs we had added one
  full API call, request_firmware_direct() just for this purpose --
  the driver data request APIs allow for you to annotate that a driver
  data file is optional for both synchronous or asynchronous requests
  through the same two basic set of APIs.
- A firmware API framework is provided to enable daisy chaining a
  series of requests for firmware on a range of supported APIs.

Signed-off-by: Luis R. Rodriguez 
---
 MAINTAINERS   |   3 +-
 drivers/base/firmware_class.c | 420 ++
 include/linux/driver_data.h   | 178 ++
 include/linux/firmware.h  |   2 +
 4 files changed, 602 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f8d77c888cfe..148d032e9401 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5226,13 +5226,14 @@ F:  include/linux/firewire.h
 F: include/uapi/linux/firewire*.h
 F: tools/firewire/
 
-FIRMWARE LOADER (request_firmware)
+FIRMWARE LOADER (request_firmware, driver_data)
 M: Luis R. Rodriguez 
 L: linux-kernel@vger.kernel.org
 S: Maintained
 F: Documentation/firmware_class/
 F: drivers/base/firmware*.c
 F: include/linux/firmware.h
+F: include/linux/driver_data.h
 
 FLASH ADAPTER DRIVER (IBM Flash Adapter 900GB Full Height PCI Flash Card)
 M: Joshua Morris 
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index db7c0bc0ed98..e87e91bcd8f8 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -2,6 +2,17 @@
  * firmware_class.c - Multi purpose firmware loading support
  *
  * Copyright (c) 2003 Manuel Estrada Sainz
+ * Copyright (c) 2017 Luis R. Rodriguez 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or at your option) any
+ * later version; or, when distributed separately from the Linux kernel or
+ * incorporated into other software packages, subject to the following license:
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of copyleft-next (version 0.3.1 or later) as published
+ * at http://copyleft-next.org/.
  *
  * Please see Documentation/firmware_class/ for more information.
  *
@@ -91,6 +102,12 @@ enum driver_data_priv_reqs {
  * @alloc_buf_size: size of the @alloc_buf
  * @old_async_cb: used only for request_firmware_nowait() since we won't change
  * all async callbacks to get the return value on failure
+ * @api: used internally for keeping track of the currently evaluated API
+ * versioned file as we iterate between min API and max API.
+ 

[PATCH v8 0/5] firmware: add driver data API

[Luis R. Rodriguez]

Greg,

Even though using copyleft-next was fine by Linus [0], AKASHI had brought that
he expected to see an "or" clause on the license declarations when using
copyleft-next. This was the only pending issue from the last v7 series [1]. To
be safe I re-touched the subject and Alan and Ted brought up sufficient reasons
for preferring the "or" language [2], as such this series goes with the "or"
language embraced so that it is even clearer than before that GPLv2 applies
when using copyleft-next on the Linux kernel.

This series depends on the few other UMH fallback lock changes I had submitted
earlier this month [3], and those remain without any noted issues. As usual,
all pending changes for this series are available on my linux-next tree on the
20170519-driver-data branch [4], this series was rebased on next-20170519.

Please let me know if there are any issues or questions.

[0] 
https://lkml.kernel.org/r/CA+55aFyhxcvD+q7tp+-yrSFDKfR0mOHgyEAe=f_94aklsou...@mail.gmail.com
[1] https://lkml.kernel.org/r/20170502084914.23588-1-mcg...@kernel.org
[2] 
https://lkml.kernel.org/r/CAB=ne6vnftr-or9ilft3lnbp4m35p0nfegxnprnawvbbaem...@mail.gmail.com
[3] https://lkml.kernel.org/r/20170502083107.23418-1-mcg...@kernel.org
[4] 
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20170519-driver-data

Luis R. Rodriguez (5):
  firmware: add extensible driver data params
  firmware: add extensible driver data API
  test: add new driver_data load tester
  firmware: document the extensible driver data API
  iwlwifi: convert to use driver data API

 Documentation/driver-api/firmware/driver_data.rst  |  167 +++
 Documentation/driver-api/firmware/index.rst|1 +
 Documentation/driver-api/firmware/introduction.rst |   16 +
 .../driver-api/firmware/request_firmware.rst   |2 +
 MAINTAINERS|4 +-
 drivers/base/firmware_class.c  |  751 ++--
 drivers/net/wireless/intel/iwlwifi/iwl-drv.c   |   91 +-
 include/linux/driver_data.h|  266 
 include/linux/firmware.h   |2 +
 lib/Kconfig.debug  |   12 +
 lib/Makefile   |1 +
 lib/test_driver_data.c | 1278 
 tools/testing/selftests/firmware/Makefile  |2 +-
 tools/testing/selftests/firmware/config|1 +
 tools/testing/selftests/firmware/driver_data.sh| 1002 +++
 15 files changed, 3460 insertions(+), 136 deletions(-)
 create mode 100644 Documentation/driver-api/firmware/driver_data.rst
 create mode 100644 include/linux/driver_data.h
 create mode 100644 lib/test_driver_data.c
 create mode 100755 tools/testing/selftests/firmware/driver_data.sh

-- 
2.11.0

[PATCH v8 2/5] firmware: add extensible driver data API

[Luis R. Rodriguez]

The firmware API does not scale well: when new features are added we
either add a new exported symbol or extend the arguments of existing
routines. For the later case this means we need to traverse the kernel
with a slew of collateral evolutions to adjust old driver users. The
firmware API is also now being used for things outside of the scope of
what typically would be considered "firmware". There are other
subsystems which would like to make use of the firmware APIs for similar
things and its clearly not firmware, but have different requirements
and criteria which they'd like to be met for the requested file.

An extensible API is in order:

The driver data API accepts that there are only two types of requests:

a) synchronous requests
b) asynchronous requests

Both requests may have a different requirements which must be met. These
requirements can be described in the struct driver_data_req_params.
This struct is expected to be extended over time to support different
requirements as the kernel evolves.

After a bit of hard work the new interface has been wrapped onto the
functionality. The fallback mechanism has been kept out of the new API
currently because it requires just a bit more grooming and documentation
given new considerations and requirements.  Adding support for it will
be rather easy now that the new API sits ontop of the old one. The
request_firmware_into_buf() API also is not enabled on the new API but
it is rather easy to do so -- this call has no current existing users
upstream though. Support will be provided once we add a respective
series of test cases against it and find a proper upstream user for it.

The flexible API also adds a few new bells and whistles:

- By default the kernel will free the driver data file for you after
  your callbacks are called, you however are allowed to request that
  you wish to keep the driver data file on the requirements params. The
  new driver data API is able to free the driver data file for you by
  requiring a consumer callback for the driver data file.
- Allows both asynchronous and synchronous request to specify that
  driver data files are optional. With the old APIs we had added one
  full API call, request_firmware_direct() just for this purpose --
  the driver data request APIs allow for you to annotate that a driver
  data file is optional for both synchronous or asynchronous requests
  through the same two basic set of APIs.
- A firmware API framework is provided to enable daisy chaining a
  series of requests for firmware on a range of supported APIs.

Signed-off-by: Luis R. Rodriguez 
---
 MAINTAINERS   |   3 +-
 drivers/base/firmware_class.c | 420 ++
 include/linux/driver_data.h   | 178 ++
 include/linux/firmware.h  |   2 +
 4 files changed, 602 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f8d77c888cfe..148d032e9401 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5226,13 +5226,14 @@ F:  include/linux/firewire.h
 F: include/uapi/linux/firewire*.h
 F: tools/firewire/
 
-FIRMWARE LOADER (request_firmware)
+FIRMWARE LOADER (request_firmware, driver_data)
 M: Luis R. Rodriguez 
 L: linux-kernel@vger.kernel.org
 S: Maintained
 F: Documentation/firmware_class/
 F: drivers/base/firmware*.c
 F: include/linux/firmware.h
+F: include/linux/driver_data.h
 
 FLASH ADAPTER DRIVER (IBM Flash Adapter 900GB Full Height PCI Flash Card)
 M: Joshua Morris 
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index db7c0bc0ed98..e87e91bcd8f8 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -2,6 +2,17 @@
  * firmware_class.c - Multi purpose firmware loading support
  *
  * Copyright (c) 2003 Manuel Estrada Sainz
+ * Copyright (c) 2017 Luis R. Rodriguez 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or at your option) any
+ * later version; or, when distributed separately from the Linux kernel or
+ * incorporated into other software packages, subject to the following license:
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of copyleft-next (version 0.3.1 or later) as published
+ * at http://copyleft-next.org/.
  *
  * Please see Documentation/firmware_class/ for more information.
  *
@@ -91,6 +102,12 @@ enum driver_data_priv_reqs {
  * @alloc_buf_size: size of the @alloc_buf
  * @old_async_cb: used only for request_firmware_nowait() since we won't change
  * all async callbacks to get the return value on failure
+ * @api: used internally for keeping track of the currently evaluated API
+ * versioned file as we iterate between min API and max API.
+ * @retry_api: if the driver replied with -EAGAIN, we must ignore the passed
+ * 

[PATCH v8 4/5] firmware: document the extensible driver data API

[Luis R. Rodriguez]

This documents the driver data API.

Signed-off-by: Luis R. Rodriguez 
---
 Documentation/driver-api/firmware/driver_data.rst  | 167 +
 Documentation/driver-api/firmware/index.rst|   1 +
 Documentation/driver-api/firmware/introduction.rst |  16 ++
 .../driver-api/firmware/request_firmware.rst   |   2 +
 4 files changed, 186 insertions(+)
 create mode 100644 Documentation/driver-api/firmware/driver_data.rst

diff --git a/Documentation/driver-api/firmware/driver_data.rst 
b/Documentation/driver-api/firmware/driver_data.rst
new file mode 100644
index ..be7c7ff99151
--- /dev/null
+++ b/Documentation/driver-api/firmware/driver_data.rst
@@ -0,0 +1,167 @@
+.. _driver_data:
+
+===
+driver_data API
+===
+
+The driver data APIs provides a flexible API for general driver data file
+lookups. Its flexibility aims at mitigating collateral evolutions on the kernel
+as new functionality is introduced.
+
+Driver data modes of operation
+==
+
+There are two types of modes of operation for driver data requests:
+
+  * synchronous  - driver_data_request_sync()
+  * asynchronous - driver_data_request_async()
+
+Synchronous requests expect requests to be done immediately, asynchronous
+requests enable requests to be scheduled for a later time.
+
+Driver data request parameters
+==
+
+Variations of types of driver data requests are specified by a driver data
+request parameter data structure. The flexibility of the API is provided by
+expanding the request parameters as new functionality is needed, without
+loosely modifying or adding new exported APIs.
+
+driver_data_sync_cbs
+
+.. kernel-doc:: include/linux/driver_data.h
+   :functions: driver_data_sync_cbs
+
+driver_data_async_cbs
+-
+.. kernel-doc:: include/linux/driver_data.h
+   :functions: driver_data_async_cbs
+
+driver_data_cbs
+---
+.. kernel-doc:: include/linux/driver_data.h
+   :functions: driver_data_cbs
+
+driver_data_reqs
+
+.. kernel-doc:: include/linux/driver_data.h
+   :functions: driver_data_reqs
+
+driver_data_req_params
+--
+.. kernel-doc:: include/linux/driver_data.h
+   :functions: driver_data_req_params
+
+Synchronous driver data requests
+
+
+Synchronous driver data requests will wait until the driver data is found or
+until an error is returned.
+
+driver_data_request_sync
+
+.. kernel-doc:: drivers/base/firmware_class.c
+   :functions: driver_data_request_sync
+
+Asynchronous driver data requests
+=
+
+Asynchronous driver data requests allow driver code to not have to wait
+until the driver data or an error is returned. Function callbacks are
+required so that when the firmware or an error is found the driver is
+informed through the callbacks. Asynchronous driver data requests cannot
+be called from atomic contexts.
+
+driver_data_request_async
+-
+.. kernel-doc:: drivers/base/firmware_class.c
+   :functions: driver_data_request_async
+
+Reference counting and releasing the driver data file
+=
+
+The device and module are bumped with reference counts during the driver data
+requests. This prevents removal of the device and module making the driver data
+request call until the driver data request callbacks have completed, either
+synchronously or asynchronously. When synchronous requests are made the
+firmware_class is refcounted. When asynchronous requests are made the caller's
+module is refcounted. Asynchronous requests do not refcount the firmware_class
+module.
+
+The driver data request API enables callers to provide a callback for both
+synchronous and asynchronous requests and since consumption can be expected
+in these callbacks it frees it for you by default after callback handlers
+are issued. If you wish to keep the driver data around after your callbacks
+you must specify this through the driver data request parameter data structure.
+
+Driver data private internal functionality
+==
+
+This section documents functionality not exposed to users, but important in
+understanding how the driver data internals work.
+
+driver_data_mode
+
+.. kernel-doc:: drivers/base/firmware_class.c
+   :functions: driver_data_mode
+
+driver_data_priv_reqs
+-
+.. kernel-doc:: drivers/base/firmware_class.c
+   :functions: driver_data_priv_reqs
+
+driver_data_priv_params
+---
+.. kernel-doc:: drivers/base/firmware_class.c
+   :functions: driver_data_priv_params
+
+driver_data_params
+--
+.. kernel-doc:: drivers/base/firmware_class.c
+   :functions: driver_data_params
+
+Testing the driver_data API
+===
+
+The driver data API has a selftest 

[PATCH v8 4/5] firmware: document the extensible driver data API

[Luis R. Rodriguez]

This documents the driver data API.

Signed-off-by: Luis R. Rodriguez 
---
 Documentation/driver-api/firmware/driver_data.rst  | 167 +
 Documentation/driver-api/firmware/index.rst|   1 +
 Documentation/driver-api/firmware/introduction.rst |  16 ++
 .../driver-api/firmware/request_firmware.rst   |   2 +
 4 files changed, 186 insertions(+)
 create mode 100644 Documentation/driver-api/firmware/driver_data.rst

diff --git a/Documentation/driver-api/firmware/driver_data.rst 
b/Documentation/driver-api/firmware/driver_data.rst
new file mode 100644
index ..be7c7ff99151
--- /dev/null
+++ b/Documentation/driver-api/firmware/driver_data.rst
@@ -0,0 +1,167 @@
+.. _driver_data:
+
+===
+driver_data API
+===
+
+The driver data APIs provides a flexible API for general driver data file
+lookups. Its flexibility aims at mitigating collateral evolutions on the kernel
+as new functionality is introduced.
+
+Driver data modes of operation
+==
+
+There are two types of modes of operation for driver data requests:
+
+  * synchronous  - driver_data_request_sync()
+  * asynchronous - driver_data_request_async()
+
+Synchronous requests expect requests to be done immediately, asynchronous
+requests enable requests to be scheduled for a later time.
+
+Driver data request parameters
+==
+
+Variations of types of driver data requests are specified by a driver data
+request parameter data structure. The flexibility of the API is provided by
+expanding the request parameters as new functionality is needed, without
+loosely modifying or adding new exported APIs.
+
+driver_data_sync_cbs
+
+.. kernel-doc:: include/linux/driver_data.h
+   :functions: driver_data_sync_cbs
+
+driver_data_async_cbs
+-
+.. kernel-doc:: include/linux/driver_data.h
+   :functions: driver_data_async_cbs
+
+driver_data_cbs
+---
+.. kernel-doc:: include/linux/driver_data.h
+   :functions: driver_data_cbs
+
+driver_data_reqs
+
+.. kernel-doc:: include/linux/driver_data.h
+   :functions: driver_data_reqs
+
+driver_data_req_params
+--
+.. kernel-doc:: include/linux/driver_data.h
+   :functions: driver_data_req_params
+
+Synchronous driver data requests
+
+
+Synchronous driver data requests will wait until the driver data is found or
+until an error is returned.
+
+driver_data_request_sync
+
+.. kernel-doc:: drivers/base/firmware_class.c
+   :functions: driver_data_request_sync
+
+Asynchronous driver data requests
+=
+
+Asynchronous driver data requests allow driver code to not have to wait
+until the driver data or an error is returned. Function callbacks are
+required so that when the firmware or an error is found the driver is
+informed through the callbacks. Asynchronous driver data requests cannot
+be called from atomic contexts.
+
+driver_data_request_async
+-
+.. kernel-doc:: drivers/base/firmware_class.c
+   :functions: driver_data_request_async
+
+Reference counting and releasing the driver data file
+=
+
+The device and module are bumped with reference counts during the driver data
+requests. This prevents removal of the device and module making the driver data
+request call until the driver data request callbacks have completed, either
+synchronously or asynchronously. When synchronous requests are made the
+firmware_class is refcounted. When asynchronous requests are made the caller's
+module is refcounted. Asynchronous requests do not refcount the firmware_class
+module.
+
+The driver data request API enables callers to provide a callback for both
+synchronous and asynchronous requests and since consumption can be expected
+in these callbacks it frees it for you by default after callback handlers
+are issued. If you wish to keep the driver data around after your callbacks
+you must specify this through the driver data request parameter data structure.
+
+Driver data private internal functionality
+==
+
+This section documents functionality not exposed to users, but important in
+understanding how the driver data internals work.
+
+driver_data_mode
+
+.. kernel-doc:: drivers/base/firmware_class.c
+   :functions: driver_data_mode
+
+driver_data_priv_reqs
+-
+.. kernel-doc:: drivers/base/firmware_class.c
+   :functions: driver_data_priv_reqs
+
+driver_data_priv_params
+---
+.. kernel-doc:: drivers/base/firmware_class.c
+   :functions: driver_data_priv_params
+
+driver_data_params
+--
+.. kernel-doc:: drivers/base/firmware_class.c
+   :functions: driver_data_params
+
+Testing the driver_data API
+===
+
+The driver data API has a selftest driver: 

Re: [4.12 regression] Thinkpad X250 Touchpad and Trackpoint not recognized anymore; commit e839ffa: "Input: synaptics - add support for Intertouch devices"

[Dmitry Torokhov]

On May 19, 2017 11:46:16 AM PDT, Pascal Wichmann  
wrote:
>Hello,
>
>using the latest 4.12-rc1 kernel, the trackpoint and touchpad of my
>Thinkpad X250 are not recognized.
>
>This is caused by commit
>e839ffa  Input: synaptics - add support for Intertouch devices
>
>
>The specific problem seems to be the detection of whether to use
>synaptics_intertouch for the psmouse module or not. That parameter
>is set to -1 by default, in which case synaptics_intertouch is
>enabled on my device.
>
>When I explicitly set that parameter to 0, the trackpoint and touchpad
>are recognized and work as expected.
>
>Removing my device from smbus_pnp_ids in 4.12 rc1 fixes the issue
>without need to manually disable synaptics_intertouch in the psmouse
>module:
>
>diff -ruN a/drivers/input/mouse/synaptics.c
>b/drivers/input/mouse/synaptics.c
>--- a/drivers/input/mouse/synaptics.c  2017-05-19 19:31:48.930734395
>+0200
>+++ b/drivers/input/mouse/synaptics.c  2017-05-19 19:32:00.490782484
>+0200
>@@ -170,7 +170,6 @@
> static const char * const smbus_pnp_ids[] = {
>   /* all of the topbuttonpad_pnp_ids are valid, we just add some extras
>*/
>   "LEN0048", /* X1 Carbon 3 */
>-  "LEN0046", /* X250 */
>   "LEN004a", /* W541 */
>   "LEN200f", /* T450s */
>   NULL
>
>
>Is the ThinkPad X250 not supporting intertouch or is there another
>problem causing the input devices not to work with it enabled?
>

Do you have i801_smbus driver enabled and loaded?


Thanks.

-- 
Dmitry

Re: [4.12 regression] Thinkpad X250 Touchpad and Trackpoint not recognized anymore; commit e839ffa: "Input: synaptics - add support for Intertouch devices"

[Dmitry Torokhov]

On May 19, 2017 11:46:16 AM PDT, Pascal Wichmann  
wrote:
>Hello,
>
>using the latest 4.12-rc1 kernel, the trackpoint and touchpad of my
>Thinkpad X250 are not recognized.
>
>This is caused by commit
>e839ffa  Input: synaptics - add support for Intertouch devices
>
>
>The specific problem seems to be the detection of whether to use
>synaptics_intertouch for the psmouse module or not. That parameter
>is set to -1 by default, in which case synaptics_intertouch is
>enabled on my device.
>
>When I explicitly set that parameter to 0, the trackpoint and touchpad
>are recognized and work as expected.
>
>Removing my device from smbus_pnp_ids in 4.12 rc1 fixes the issue
>without need to manually disable synaptics_intertouch in the psmouse
>module:
>
>diff -ruN a/drivers/input/mouse/synaptics.c
>b/drivers/input/mouse/synaptics.c
>--- a/drivers/input/mouse/synaptics.c  2017-05-19 19:31:48.930734395
>+0200
>+++ b/drivers/input/mouse/synaptics.c  2017-05-19 19:32:00.490782484
>+0200
>@@ -170,7 +170,6 @@
> static const char * const smbus_pnp_ids[] = {
>   /* all of the topbuttonpad_pnp_ids are valid, we just add some extras
>*/
>   "LEN0048", /* X1 Carbon 3 */
>-  "LEN0046", /* X250 */
>   "LEN004a", /* W541 */
>   "LEN200f", /* T450s */
>   NULL
>
>
>Is the ThinkPad X250 not supporting intertouch or is there another
>problem causing the input devices not to work with it enabled?
>

Do you have i801_smbus driver enabled and loaded?


Thanks.

-- 
Dmitry

Re: Use case for TASKS_RCU

[Paul E. McKenney]

On Fri, May 19, 2017 at 10:23:31AM -0400, Steven Rostedt wrote:
> On Fri, 19 May 2017 10:04:21 -0400
> Steven Rostedt  wrote:
> 
> > On Fri, 19 May 2017 06:35:50 -0700
> > "Paul E. McKenney"  wrote:
> > 
> > > Simpler would be better!
> > > 
> > > However, is it really guaranteed that one SCHED_IDLE thread cannot
> > > preempt another?  If not, then the trampoline-freeing SCHED_IDLE thread
> > > might preempt some other SCHED_IDLE thread in the middle of a trampoline.
> > > I am not seeing anything that prevents such preemption, but it is rather
> > > early local time, so I could easily be missing something.
> > > 
> > > However, if SCHED_IDLE threads cannot preempt other threads, even other
> > > SCHED_IDLE threads, then your approach sounds quite promising to me.
> > > 
> > > Steve, Peter, thoughts?  
> > 
> > SCHED_IDLE is the swapper task. There's one on each CPU, and they don't
> > migrate. And they only get called when there's no other task running.
> 
> Peter just "schooled" me on IRC. I stand corrected (and he may respond
> to this email too). I guess any task can become SCHED_IDLE.
> 
> But that just makes this an even less likely option for
> synchronize_rcu_tasks().

Hmmm...  The goal is to make sure that any task that was preempted or
running at a given point in time passes through a voluntary context switch
(or userspace execution, or, ...).

What is the simplest way to get this job done?  To Ingo's point, I bet
that there is a simpler way than the current TASKS_RCU implementation.

Ingo, if I make it fit into 100 lines of code, would you be OK with it?
I probably need a one-line hook at task-creation time and another
at task-exit time, if that makes a difference.

Thanx, Paul

Re: Use case for TASKS_RCU

[Paul E. McKenney]

On Fri, May 19, 2017 at 10:23:31AM -0400, Steven Rostedt wrote:
> On Fri, 19 May 2017 10:04:21 -0400
> Steven Rostedt  wrote:
> 
> > On Fri, 19 May 2017 06:35:50 -0700
> > "Paul E. McKenney"  wrote:
> > 
> > > Simpler would be better!
> > > 
> > > However, is it really guaranteed that one SCHED_IDLE thread cannot
> > > preempt another?  If not, then the trampoline-freeing SCHED_IDLE thread
> > > might preempt some other SCHED_IDLE thread in the middle of a trampoline.
> > > I am not seeing anything that prevents such preemption, but it is rather
> > > early local time, so I could easily be missing something.
> > > 
> > > However, if SCHED_IDLE threads cannot preempt other threads, even other
> > > SCHED_IDLE threads, then your approach sounds quite promising to me.
> > > 
> > > Steve, Peter, thoughts?  
> > 
> > SCHED_IDLE is the swapper task. There's one on each CPU, and they don't
> > migrate. And they only get called when there's no other task running.
> 
> Peter just "schooled" me on IRC. I stand corrected (and he may respond
> to this email too). I guess any task can become SCHED_IDLE.
> 
> But that just makes this an even less likely option for
> synchronize_rcu_tasks().

Hmmm...  The goal is to make sure that any task that was preempted or
running at a given point in time passes through a voluntary context switch
(or userspace execution, or, ...).

What is the simplest way to get this job done?  To Ingo's point, I bet
that there is a simpler way than the current TASKS_RCU implementation.

Ingo, if I make it fit into 100 lines of code, would you be OK with it?
I probably need a one-line hook at task-creation time and another
at task-exit time, if that makes a difference.

Thanx, Paul

Darlehen angebot 3 %

[Frau SCHMIDT]

Sehr geehrte Damen  und Herren,

Haben Sie Interesse über einer finanziellen Darlehen zu 3%?
kontaktieren Sie mich für mehr Details und Bedingungen. ich kann all
jenen helfen, wer ein Darlehen benötigen.
Ich kann Ihnen biete ein darlehen in hohe von 10.000.000 EUR
Meine mail: info@rschmidt.online

Mit freundlichen Grüßen

Frau SCHMIDT

Darlehen angebot 3 %

[Frau SCHMIDT]

Sehr geehrte Damen  und Herren,

Haben Sie Interesse über einer finanziellen Darlehen zu 3%?
kontaktieren Sie mich für mehr Details und Bedingungen. ich kann all
jenen helfen, wer ein Darlehen benötigen.
Ich kann Ihnen biete ein darlehen in hohe von 10.000.000 EUR
Meine mail: info@rschmidt.online

Mit freundlichen Grüßen

Frau SCHMIDT

[4.12 regression] Thinkpad X250 Touchpad and Trackpoint not recognized anymore; commit e839ffa: "Input: synaptics - add support for Intertouch devices"

[Pascal Wichmann]

Hello,

using the latest 4.12-rc1 kernel, the trackpoint and touchpad of my
Thinkpad X250 are not recognized.

This is caused by commit
e839ffa  Input: synaptics - add support for Intertouch devices


The specific problem seems to be the detection of whether to use
synaptics_intertouch for the psmouse module or not. That parameter
is set to -1 by default, in which case synaptics_intertouch is
enabled on my device.

When I explicitly set that parameter to 0, the trackpoint and touchpad
are recognized and work as expected.

Removing my device from smbus_pnp_ids in 4.12 rc1 fixes the issue
without need to manually disable synaptics_intertouch in the psmouse
module:

diff -ruN a/drivers/input/mouse/synaptics.c
b/drivers/input/mouse/synaptics.c
--- a/drivers/input/mouse/synaptics.c   2017-05-19 19:31:48.930734395 +0200
+++ b/drivers/input/mouse/synaptics.c   2017-05-19 19:32:00.490782484 +0200
@@ -170,7 +170,6 @@
 static const char * const smbus_pnp_ids[] = {
/* all of the topbuttonpad_pnp_ids are valid, we just add some extras */
"LEN0048", /* X1 Carbon 3 */
-   "LEN0046", /* X250 */
"LEN004a", /* W541 */
"LEN200f", /* T450s */
NULL


Is the ThinkPad X250 not supporting intertouch or is there another
problem causing the input devices not to work with it enabled?


Regards,
Pascal




signature.asc
Description: OpenPGP digital signature

[4.12 regression] Thinkpad X250 Touchpad and Trackpoint not recognized anymore; commit e839ffa: "Input: synaptics - add support for Intertouch devices"

[Pascal Wichmann]

Hello,

using the latest 4.12-rc1 kernel, the trackpoint and touchpad of my
Thinkpad X250 are not recognized.

This is caused by commit
e839ffa  Input: synaptics - add support for Intertouch devices


The specific problem seems to be the detection of whether to use
synaptics_intertouch for the psmouse module or not. That parameter
is set to -1 by default, in which case synaptics_intertouch is
enabled on my device.

When I explicitly set that parameter to 0, the trackpoint and touchpad
are recognized and work as expected.

Removing my device from smbus_pnp_ids in 4.12 rc1 fixes the issue
without need to manually disable synaptics_intertouch in the psmouse
module:

diff -ruN a/drivers/input/mouse/synaptics.c
b/drivers/input/mouse/synaptics.c
--- a/drivers/input/mouse/synaptics.c   2017-05-19 19:31:48.930734395 +0200
+++ b/drivers/input/mouse/synaptics.c   2017-05-19 19:32:00.490782484 +0200
@@ -170,7 +170,6 @@
 static const char * const smbus_pnp_ids[] = {
/* all of the topbuttonpad_pnp_ids are valid, we just add some extras */
"LEN0048", /* X1 Carbon 3 */
-   "LEN0046", /* X250 */
"LEN004a", /* W541 */
"LEN200f", /* T450s */
NULL


Is the ThinkPad X250 not supporting intertouch or is there another
problem causing the input devices not to work with it enabled?


Regards,
Pascal




signature.asc
Description: OpenPGP digital signature

[PATCH net] bonding: fix randomly populated arp target array

[Jarod Wilson]

In commit dc9c4d0fe023, the arp_target array moved from a static global
to a local variable. By the nature of static globals, the array used to
be initialized to all 0. At present, it's full of random data, which
that gets interpreted as arp_target values, when none have actually been
specified. Systems end up booting with spew along these lines:

[   32.161783] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
[   32.168475] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
[   32.175089] 8021q: adding VLAN 0 to HW filter on device lacp0
[   32.193091] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
[   32.204892] lacp0: Setting MII monitoring interval to 100
[   32.211071] lacp0: Removing ARP target 216.124.228.17
[   32.216824] lacp0: Removing ARP target 218.160.255.255
[   32.222646] lacp0: Removing ARP target 185.170.136.184
[   32.228496] lacp0: invalid ARP target 255.255.255.255 specified for removal
[   32.236294] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
[   32.243987] lacp0: Removing ARP target 56.125.228.17
[   32.249625] lacp0: Removing ARP target 218.160.255.255
[   32.255432] lacp0: Removing ARP target 15.157.233.184
[   32.261165] lacp0: invalid ARP target 255.255.255.255 specified for removal
[   32.268939] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
[   32.276632] lacp0: Removing ARP target 16.0.0.0
[   32.281755] lacp0: Removing ARP target 218.160.255.255
[   32.287567] lacp0: Removing ARP target 72.125.228.17
[   32.293165] lacp0: Removing ARP target 218.160.255.255
[   32.298970] lacp0: Removing ARP target 8.125.228.17
[   32.304458] lacp0: Removing ARP target 218.160.255.255

None of these were actually specified as ARP targets, and the driver does
seem to clean up the mess okay, but it's rather noisy and confusing, leaks
values to userspace, and the 255.255.255.255 spew shows up even when debug
prints are disabled.

The fix: just zero out arp_target at init time.

While we're in here, init arp_all_targets_value in the right place.

Fixes: dc9c4d0fe023 ("bonding: reduce scope of some global variables")
CC: Mahesh Bandewar 
CC: Jay Vosburgh 
CC: Veaceslav Falico 
CC: Andy Gospodarek 
CC: net...@vger.kernel.org
CC: sta...@vger.kernel.org
Signed-off-by: Jarod Wilson 
---
 drivers/net/bonding/bond_main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 2be78807fd6e..73313318399c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4271,10 +4271,10 @@ static int bond_check_params(struct bond_params *params)
int arp_validate_value, fail_over_mac_value, primary_reselect_value, i;
struct bond_opt_value newval;
const struct bond_opt_value *valptr;
-   int arp_all_targets_value;
+   int arp_all_targets_value = 0;
u16 ad_actor_sys_prio = 0;
u16 ad_user_port_key = 0;
-   __be32 arp_target[BOND_MAX_ARP_TARGETS];
+   __be32 arp_target[BOND_MAX_ARP_TARGETS] = { 0 };
int arp_ip_count;
int bond_mode   = BOND_MODE_ROUNDROBIN;
int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
@@ -4501,7 +4501,6 @@ static int bond_check_params(struct bond_params *params)
arp_validate_value = 0;
}
 
-   arp_all_targets_value = 0;
if (arp_all_targets) {
bond_opt_initstr(, arp_all_targets);
valptr = bond_opt_parse(bond_opt_get(BOND_OPT_ARP_ALL_TARGETS),
-- 
2.12.1

[PATCH net] bonding: fix randomly populated arp target array

[Jarod Wilson]

In commit dc9c4d0fe023, the arp_target array moved from a static global
to a local variable. By the nature of static globals, the array used to
be initialized to all 0. At present, it's full of random data, which
that gets interpreted as arp_target values, when none have actually been
specified. Systems end up booting with spew along these lines:

[   32.161783] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
[   32.168475] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
[   32.175089] 8021q: adding VLAN 0 to HW filter on device lacp0
[   32.193091] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
[   32.204892] lacp0: Setting MII monitoring interval to 100
[   32.211071] lacp0: Removing ARP target 216.124.228.17
[   32.216824] lacp0: Removing ARP target 218.160.255.255
[   32.222646] lacp0: Removing ARP target 185.170.136.184
[   32.228496] lacp0: invalid ARP target 255.255.255.255 specified for removal
[   32.236294] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
[   32.243987] lacp0: Removing ARP target 56.125.228.17
[   32.249625] lacp0: Removing ARP target 218.160.255.255
[   32.255432] lacp0: Removing ARP target 15.157.233.184
[   32.261165] lacp0: invalid ARP target 255.255.255.255 specified for removal
[   32.268939] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
[   32.276632] lacp0: Removing ARP target 16.0.0.0
[   32.281755] lacp0: Removing ARP target 218.160.255.255
[   32.287567] lacp0: Removing ARP target 72.125.228.17
[   32.293165] lacp0: Removing ARP target 218.160.255.255
[   32.298970] lacp0: Removing ARP target 8.125.228.17
[   32.304458] lacp0: Removing ARP target 218.160.255.255

None of these were actually specified as ARP targets, and the driver does
seem to clean up the mess okay, but it's rather noisy and confusing, leaks
values to userspace, and the 255.255.255.255 spew shows up even when debug
prints are disabled.

The fix: just zero out arp_target at init time.

While we're in here, init arp_all_targets_value in the right place.

Fixes: dc9c4d0fe023 ("bonding: reduce scope of some global variables")
CC: Mahesh Bandewar 
CC: Jay Vosburgh 
CC: Veaceslav Falico 
CC: Andy Gospodarek 
CC: net...@vger.kernel.org
CC: sta...@vger.kernel.org
Signed-off-by: Jarod Wilson 
---
 drivers/net/bonding/bond_main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 2be78807fd6e..73313318399c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4271,10 +4271,10 @@ static int bond_check_params(struct bond_params *params)
int arp_validate_value, fail_over_mac_value, primary_reselect_value, i;
struct bond_opt_value newval;
const struct bond_opt_value *valptr;
-   int arp_all_targets_value;
+   int arp_all_targets_value = 0;
u16 ad_actor_sys_prio = 0;
u16 ad_user_port_key = 0;
-   __be32 arp_target[BOND_MAX_ARP_TARGETS];
+   __be32 arp_target[BOND_MAX_ARP_TARGETS] = { 0 };
int arp_ip_count;
int bond_mode   = BOND_MODE_ROUNDROBIN;
int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
@@ -4501,7 +4501,6 @@ static int bond_check_params(struct bond_params *params)
arp_validate_value = 0;
}
 
-   arp_all_targets_value = 0;
if (arp_all_targets) {
bond_opt_initstr(, arp_all_targets);
valptr = bond_opt_parse(bond_opt_get(BOND_OPT_ARP_ALL_TARGETS),
-- 
2.12.1

[GIT PULL] KVM fixes for v4.12-rc2

[Radim Krčmář]

Linus,

The following changes since commit 2ea659a9ef488125eb46da6eb571de5eae5c43f6:

  Linux 4.12-rc1 (2017-05-13 13:19:49 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/kvm.git tags/for-linus

for you to fetch changes up to 92ceb7679ab8807d3b7fbcc6daf2279036954ef5:

  KVM: x86: prevent uninitialized variable warning in check_svme() (2017-05-19 
19:59:28 +0200)


KVM fixes for v4.12-rc2

ARM:
 - A fix for a build failure introduced in -rc1 when tracepoints are
   enabled on 32-bit ARM.
 - Disabling use of stack pointer protection in the hyp code which can
   cause panics.
 - A handful of VGIC fixes.
 - A fix to the init of the redistributors on GICv3 systems that
   prevented boot with kvmtool on GICv3 systems introduced in -rc1.
 - A number of race conditions fixed in our MMU handling code.
 - A fix for the guest being able to program the debug extensions for
   the host on the 32-bit side.

PPC:
 - Fixes for build failures with PR KVM configurations.
 - A fix for a host crash that can occur on POWER9 with radix guests.

x86:
 - Fixes for nested PML and nested EPT.
 - A fix for crashes caused by reserved bits in SSE MXCSR that could
   have been set by userspace.
 - An optimization of halt polling that fixes high CPU overhead.
 - Fixes for four reports from Dan Carpenter's static checker.
 - A protection around code that shouldn't have been preemptible.
 - A fix for port IO emulation.


Christoffer Dall (2):
  KVM: arm/arm64: Fix bug when registering redist iodevs
  KVM: arm/arm64: Hold slots_lock when unregistering kvm io bus devices

Dan Carpenter (2):
  kvm: nVMX: off by one in vmx_write_pml_buffer()
  KVM: Silence underflow warning in avic_get_physical_id_entry()

Marc Zyngier (6):
  ARM: KVM: Fix tracepoint generation after move to virt/kvm/arm/
  arm64: KVM: Do not use stack-protector to compile EL2 code
  arm: KVM: Do not use stack-protector to compile HYP code
  KVM: arm/arm64: vgic-v2: Do not use Active+Pending state for a HW 
interrupt
  KVM: arm/arm64: vgic-v3: Do not use Active+Pending state for a HW 
interrupt
  KVM: arm/arm64: vgic-v3: Use PREbits to infer the number of ICH_APxRn_EL2 
registers

Paolo Bonzini (2):
  KVM: nVMX: fix EPT permissions as reported in exit qualification
  KVM: x86: lower default for halt_poll_ns

Paul Mackerras (3):
  KVM: PPC: Book3S HV: Add radix checks in real-mode hypercall handlers
  KVM: PPC: Book3S PR: Check copy_to/from_user return values
  KVM: PPC: Book3S PR: Don't include SPAPR TCE code on non-pseries platforms

Radim Krčmář (5):
  Merge branch 'kvm-ppc-fixes' of git://git.kernel.org/.../paulus/powerpc
  Merge tag 'kvm-arm-for-v4.12-rc2' of 
git://git.kernel.org/.../kvmarm/kvmarm
  KVM: x86: zero base3 of unusable segments
  KVM: x86/vPMU: fix undefined shift in intel_pmu_refresh()
  KVM: x86: prevent uninitialized variable warning in check_svme()

Suzuki K Poulose (3):
  kvm: arm/arm64: Fix race in resetting stage2 PGD
  kvm: arm/arm64: Force reading uncached stage2 PGD
  kvm: arm/arm64: Fix use after free of stage2 page table

Wanpeng Li (4):
  KVM: x86: Fix load damaged SSEx MXCSR register
  KVM: VMX: Don't enable EPT A/D feature if EPT feature is disabled
  KVM: x86: Fix potential preemption when get the current kvmclock timestamp
  KVM: X86: Fix read out-of-bounds vulnerability in kvm pio emulation

Zhichao Huang (2):
  KVM: arm: plug potential guest hardware debug leakage
  KVM: arm: rename pm_fake handler to trap_raz_wi

 arch/arm/include/asm/kvm_coproc.h|   3 +-
 arch/arm/kvm/coproc.c| 106 ---
 arch/arm/kvm/handle_exit.c   |   4 +-
 arch/arm/kvm/hyp/Makefile|   2 +
 arch/arm/kvm/hyp/switch.c|   4 +-
 arch/arm/kvm/trace.h |   8 +--
 arch/arm64/kvm/hyp/Makefile  |   2 +
 arch/powerpc/kvm/Kconfig |   2 +-
 arch/powerpc/kvm/Makefile|   4 +-
 arch/powerpc/kvm/book3s_64_vio_hv.c  |  13 +
 arch/powerpc/kvm/book3s_hv_builtin.c |   9 ++-
 arch/powerpc/kvm/book3s_pr_papr.c|  80 ++
 arch/powerpc/kvm/powerpc.c   |   4 +-
 arch/x86/include/asm/kvm_host.h  |   2 +-
 arch/x86/kernel/fpu/init.c   |   1 +
 arch/x86/kvm/emulate.c   |   2 +-
 arch/x86/kvm/paging_tmpl.h   |  35 +++-
 arch/x86/kvm/pmu_intel.c |   2 +-
 arch/x86/kvm/svm.c   |   3 +-
 arch/x86/kvm/vmx.c   |   4 +-
 arch/x86/kvm/x86.c   |  45 +++
 include/kvm/arm_vgic.h   |   5 +-
 virt/kvm/arm/hyp/vgic-v3-sr.c|  18 +++---
 virt/kvm/arm/mmu.c   |  33 +++
 virt/kvm/arm/vgic/vgic-init.c|   5 

[GIT PULL] KVM fixes for v4.12-rc2

[Radim Krčmář]

Linus,

The following changes since commit 2ea659a9ef488125eb46da6eb571de5eae5c43f6:

  Linux 4.12-rc1 (2017-05-13 13:19:49 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/kvm.git tags/for-linus

for you to fetch changes up to 92ceb7679ab8807d3b7fbcc6daf2279036954ef5:

  KVM: x86: prevent uninitialized variable warning in check_svme() (2017-05-19 
19:59:28 +0200)


KVM fixes for v4.12-rc2

ARM:
 - A fix for a build failure introduced in -rc1 when tracepoints are
   enabled on 32-bit ARM.
 - Disabling use of stack pointer protection in the hyp code which can
   cause panics.
 - A handful of VGIC fixes.
 - A fix to the init of the redistributors on GICv3 systems that
   prevented boot with kvmtool on GICv3 systems introduced in -rc1.
 - A number of race conditions fixed in our MMU handling code.
 - A fix for the guest being able to program the debug extensions for
   the host on the 32-bit side.

PPC:
 - Fixes for build failures with PR KVM configurations.
 - A fix for a host crash that can occur on POWER9 with radix guests.

x86:
 - Fixes for nested PML and nested EPT.
 - A fix for crashes caused by reserved bits in SSE MXCSR that could
   have been set by userspace.
 - An optimization of halt polling that fixes high CPU overhead.
 - Fixes for four reports from Dan Carpenter's static checker.
 - A protection around code that shouldn't have been preemptible.
 - A fix for port IO emulation.


Christoffer Dall (2):
  KVM: arm/arm64: Fix bug when registering redist iodevs
  KVM: arm/arm64: Hold slots_lock when unregistering kvm io bus devices

Dan Carpenter (2):
  kvm: nVMX: off by one in vmx_write_pml_buffer()
  KVM: Silence underflow warning in avic_get_physical_id_entry()

Marc Zyngier (6):
  ARM: KVM: Fix tracepoint generation after move to virt/kvm/arm/
  arm64: KVM: Do not use stack-protector to compile EL2 code
  arm: KVM: Do not use stack-protector to compile HYP code
  KVM: arm/arm64: vgic-v2: Do not use Active+Pending state for a HW 
interrupt
  KVM: arm/arm64: vgic-v3: Do not use Active+Pending state for a HW 
interrupt
  KVM: arm/arm64: vgic-v3: Use PREbits to infer the number of ICH_APxRn_EL2 
registers

Paolo Bonzini (2):
  KVM: nVMX: fix EPT permissions as reported in exit qualification
  KVM: x86: lower default for halt_poll_ns

Paul Mackerras (3):
  KVM: PPC: Book3S HV: Add radix checks in real-mode hypercall handlers
  KVM: PPC: Book3S PR: Check copy_to/from_user return values
  KVM: PPC: Book3S PR: Don't include SPAPR TCE code on non-pseries platforms

Radim Krčmář (5):
  Merge branch 'kvm-ppc-fixes' of git://git.kernel.org/.../paulus/powerpc
  Merge tag 'kvm-arm-for-v4.12-rc2' of 
git://git.kernel.org/.../kvmarm/kvmarm
  KVM: x86: zero base3 of unusable segments
  KVM: x86/vPMU: fix undefined shift in intel_pmu_refresh()
  KVM: x86: prevent uninitialized variable warning in check_svme()

Suzuki K Poulose (3):
  kvm: arm/arm64: Fix race in resetting stage2 PGD
  kvm: arm/arm64: Force reading uncached stage2 PGD
  kvm: arm/arm64: Fix use after free of stage2 page table

Wanpeng Li (4):
  KVM: x86: Fix load damaged SSEx MXCSR register
  KVM: VMX: Don't enable EPT A/D feature if EPT feature is disabled
  KVM: x86: Fix potential preemption when get the current kvmclock timestamp
  KVM: X86: Fix read out-of-bounds vulnerability in kvm pio emulation

Zhichao Huang (2):
  KVM: arm: plug potential guest hardware debug leakage
  KVM: arm: rename pm_fake handler to trap_raz_wi

 arch/arm/include/asm/kvm_coproc.h|   3 +-
 arch/arm/kvm/coproc.c| 106 ---
 arch/arm/kvm/handle_exit.c   |   4 +-
 arch/arm/kvm/hyp/Makefile|   2 +
 arch/arm/kvm/hyp/switch.c|   4 +-
 arch/arm/kvm/trace.h |   8 +--
 arch/arm64/kvm/hyp/Makefile  |   2 +
 arch/powerpc/kvm/Kconfig |   2 +-
 arch/powerpc/kvm/Makefile|   4 +-
 arch/powerpc/kvm/book3s_64_vio_hv.c  |  13 +
 arch/powerpc/kvm/book3s_hv_builtin.c |   9 ++-
 arch/powerpc/kvm/book3s_pr_papr.c|  80 ++
 arch/powerpc/kvm/powerpc.c   |   4 +-
 arch/x86/include/asm/kvm_host.h  |   2 +-
 arch/x86/kernel/fpu/init.c   |   1 +
 arch/x86/kvm/emulate.c   |   2 +-
 arch/x86/kvm/paging_tmpl.h   |  35 +++-
 arch/x86/kvm/pmu_intel.c |   2 +-
 arch/x86/kvm/svm.c   |   3 +-
 arch/x86/kvm/vmx.c   |   4 +-
 arch/x86/kvm/x86.c   |  45 +++
 include/kvm/arm_vgic.h   |   5 +-
 virt/kvm/arm/hyp/vgic-v3-sr.c|  18 +++---
 virt/kvm/arm/mmu.c   |  33 +++
 virt/kvm/arm/vgic/vgic-init.c|   5 

Re: [PATCH] selinux: Remove redundant check for unknown labeling behavior

[Paul Moore]

On Fri, May 19, 2017 at 1:09 PM, Matthias Kaehlcke  wrote:
> The check is already performed in ocontext_read() when the policy is
> loaded. Removing the array also fixes the following warning when
> building with clang:
>
> security/selinux/hooks.c:338:20: error: variable 'labeling_behaviors'
> is not needed and will not be emitted
> [-Werror,-Wunneeded-internal-declaration]
>
> Signed-off-by: Matthias Kaehlcke 
> ---
>  security/selinux/hooks.c | 16 
>  1 file changed, 16 deletions(-)

Merged, thanks everyone.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526d1f30..2e0227b0304f 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -398,18 +398,6 @@ static void superblock_free_security(struct super_block 
> *sb)
> kfree(sbsec);
>  }
>
> -/* The file system's label must be initialized prior to use. */
> -
> -static const char *labeling_behaviors[7] = {
> -   "uses xattr",
> -   "uses transition SIDs",
> -   "uses task SIDs",
> -   "uses genfs_contexts",
> -   "not configured for labeling",
> -   "uses mountpoint labeling",
> -   "uses native labeling",
> -};
> -
>  static inline int inode_doinit(struct inode *inode)
>  {
> return inode_doinit_with_dentry(inode, NULL);
> @@ -524,10 +512,6 @@ static int sb_finish_set_opts(struct super_block *sb)
> }
> }
>
> -   if (sbsec->behavior > ARRAY_SIZE(labeling_behaviors))
> -   printk(KERN_ERR "SELinux: initialized (dev %s, type %s), 
> unknown behavior\n",
> -  sb->s_id, sb->s_type->name);
> -
> sbsec->flags |= SE_SBINITIALIZED;
> if (selinux_is_sblabel_mnt(sb))
> sbsec->flags |= SBLABEL_MNT;
> --
> 2.13.0.303.g4ebf302169-goog
>



-- 
paul moore
www.paul-moore.com

Re: [PATCH] selinux: Remove redundant check for unknown labeling behavior

[Paul Moore]

On Fri, May 19, 2017 at 1:09 PM, Matthias Kaehlcke  wrote:
> The check is already performed in ocontext_read() when the policy is
> loaded. Removing the array also fixes the following warning when
> building with clang:
>
> security/selinux/hooks.c:338:20: error: variable 'labeling_behaviors'
> is not needed and will not be emitted
> [-Werror,-Wunneeded-internal-declaration]
>
> Signed-off-by: Matthias Kaehlcke 
> ---
>  security/selinux/hooks.c | 16 
>  1 file changed, 16 deletions(-)

Merged, thanks everyone.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526d1f30..2e0227b0304f 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -398,18 +398,6 @@ static void superblock_free_security(struct super_block 
> *sb)
> kfree(sbsec);
>  }
>
> -/* The file system's label must be initialized prior to use. */
> -
> -static const char *labeling_behaviors[7] = {
> -   "uses xattr",
> -   "uses transition SIDs",
> -   "uses task SIDs",
> -   "uses genfs_contexts",
> -   "not configured for labeling",
> -   "uses mountpoint labeling",
> -   "uses native labeling",
> -};
> -
>  static inline int inode_doinit(struct inode *inode)
>  {
> return inode_doinit_with_dentry(inode, NULL);
> @@ -524,10 +512,6 @@ static int sb_finish_set_opts(struct super_block *sb)
> }
> }
>
> -   if (sbsec->behavior > ARRAY_SIZE(labeling_behaviors))
> -   printk(KERN_ERR "SELinux: initialized (dev %s, type %s), 
> unknown behavior\n",
> -  sb->s_id, sb->s_type->name);
> -
> sbsec->flags |= SE_SBINITIALIZED;
> if (selinux_is_sblabel_mnt(sb))
> sbsec->flags |= SBLABEL_MNT;
> --
> 2.13.0.303.g4ebf302169-goog
>



-- 
paul moore
www.paul-moore.com

Re: [PATCH v3 00/11] Broadcom Stingray SOC Initial Support

[Florian Fainelli]

On 05/16/2017 09:24 PM, Anup Patel wrote:
> On Wed, May 17, 2017 at 12:23 AM, Olof Johansson  wrote:
>> Hi,
>>
>>
>>
>> On Tue, May 16, 2017 at 4:30 AM, Anup Patel  wrote:
>>> This patchset adds initial support of Broadcom Stingray SOC
>>> by reusing existing Broadcom iProc device drivers.
>>>
>>> Most of the patches in this patchset are DT patches except
>>> the Stingray clock tree support which just one patch.
>>>
>>> This patchset is based on Linux-4.12-rc1 and it is also available
>>> at stingray-v3 branch of https://github.com/Broadcom/arm64-linux.git
>>>
>>> Changes since v2:
>>>  - Remove default bootargs from chosen DT node
>>>  - Remove "linux" prefix from stdout DT attribute of chosen DT node
>>>  - Remove use of GIC_CPU_MASK_xxx() for PPIs
>>>
>>> Changes since v1:
>>>  - Rebased patches for Linux-4.12-rc1
>>>  - Removed unwanted /memreserve/ from bcm958742-base.dtsi
>>>  - Use ranges DT property to clear view of memory-layout
>>>  - Make bcm-sr.h part of clock DT bindings patch
>>>
>>> Anup Patel (3):
>>>   dt-bindings: bcm: Add Broadcom Stingray bindings document
>>>   arm64: dts: Initial DTS files for Broadcom Stingray SOC
>>>   arm64: dts: Add PL022, PL330 and SP805 DT nodes for Stingray
>>>
>>> Oza Pawandeep (1):
>>>   arm64: dts: Add I2C DT nodes for Stingray SoC
>>>
>>> Pramod Kumar (3):
>>>   arm64: dts: Add NAND DT nodes for Stingray SOC
>>>   arm64: dts: Add pinctrl DT nodes for Stingray SOC
>>>   arm64: dts: Add GPIO DT nodes for Stingray SOC
>>>
>>> Sandeep Tripathy (3):
>>>   dt-bindings: clk: Extend binding doc for Stingray SOC
>>>   clk: bcm: Add clocks for Stingray SOC
>>>   arm64: dts: Add clock DT nodes for Stingray SOC
>>>
>>> Srinath Mannam (1):
>>>   arm64: dts: Add PWM and SDHCI DT nodes for Stingray SOC
>>>
>>>  .../devicetree/bindings/arm/bcm/brcm,stingray.txt  |  12 +
>>>  .../bindings/clock/brcm,iproc-clocks.txt   |  76 
>>>  arch/arm64/boot/dts/broadcom/Makefile  |   1 +
>>>  arch/arm64/boot/dts/broadcom/stingray/Makefile |   6 +
>>>  .../boot/dts/broadcom/stingray/bcm958742-base.dtsi | 131 ++
>>>  .../boot/dts/broadcom/stingray/bcm958742k.dts  |  78 
>>>  .../boot/dts/broadcom/stingray/bcm958742t.dts  |  40 ++
>>>  .../boot/dts/broadcom/stingray/stingray-clock.dtsi | 170 
>>>  .../dts/broadcom/stingray/stingray-pinctrl.dtsi| 345 
>>>  .../arm64/boot/dts/broadcom/stingray/stingray.dtsi | 460 
>>> +
>>>  drivers/clk/bcm/Kconfig|   8 +
>>>  drivers/clk/bcm/Makefile   |   1 +
>>>  drivers/clk/bcm/clk-sr.c   | 300 ++
>>>  include/dt-bindings/clock/bcm-sr.h | 101 +
>>>  .../dt-bindings/pinctrl/brcm,pinctrl-stingray.h|  68 +++
>>>  15 files changed, 1797 insertions(+)
>>>  create mode 100644 
>>> Documentation/devicetree/bindings/arm/bcm/brcm,stingray.txt
>>>  create mode 100644 arch/arm64/boot/dts/broadcom/stingray/Makefile
>>>  create mode 100644 
>>> arch/arm64/boot/dts/broadcom/stingray/bcm958742-base.dtsi
>>>  create mode 100644 arch/arm64/boot/dts/broadcom/stingray/bcm958742k.dts
>>>  create mode 100644 arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dts
>>>  create mode 100644 
>>> arch/arm64/boot/dts/broadcom/stingray/stingray-clock.dtsi
>>>  create mode 100644 
>>> arch/arm64/boot/dts/broadcom/stingray/stingray-pinctrl.dtsi
>>>  create mode 100644 arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
>>>  create mode 100644 drivers/clk/bcm/clk-sr.c
>>>  create mode 100644 include/dt-bindings/clock/bcm-sr.h
>>>  create mode 100644 include/dt-bindings/pinctrl/brcm,pinctrl-stingray.h
>>
>> Grouping as one patchset for review is fine, but when you submit this
>> for merge you need to split it up:
>>
>> - Documentation/devicetree can go with the driver pieces or with the
>> DT changes, your choice
>> - DTS/DTSI changes should go through the Broadcom maintainers to arm-soc
>> - clk changes should go to clk maintainers.
> 
> Only PATCH3 needs to go through clk maintainers rest all patches
> are DT bindings document and DTS changes which can go through
> Broadcom tree. Right ??

Patch 2 and 3 should go through the clock maintainers, and I can take
the remaining patches through the Broadcom ARM/ARM64 SoC pull requests,
no problem.

Unless there are specific changes requested which warrant a v4, I can
coordinate with Stephen and Mike to make sure that they take the
appropriate patches.

> 
>>
>> Also, there's usually no need to split up a DT introduction and having
>> separate pieces for NAND/pinctrl/GPIO/etc: They can all go in the main
>> DT submission patch.
> 
> Yes, we have already squashed DTS patches from same author into
> one DTS patch except NAND, PINCTRL and GPIO patches.
> 
> The NAND, PINCTRL, and GPIO patches were not squashed into
> one patch for easy review but since you suggest I will squash these
> three DTS patches 

Re: [PATCH v3 00/11] Broadcom Stingray SOC Initial Support

[Florian Fainelli]

On 05/16/2017 09:24 PM, Anup Patel wrote:
> On Wed, May 17, 2017 at 12:23 AM, Olof Johansson  wrote:
>> Hi,
>>
>>
>>
>> On Tue, May 16, 2017 at 4:30 AM, Anup Patel  wrote:
>>> This patchset adds initial support of Broadcom Stingray SOC
>>> by reusing existing Broadcom iProc device drivers.
>>>
>>> Most of the patches in this patchset are DT patches except
>>> the Stingray clock tree support which just one patch.
>>>
>>> This patchset is based on Linux-4.12-rc1 and it is also available
>>> at stingray-v3 branch of https://github.com/Broadcom/arm64-linux.git
>>>
>>> Changes since v2:
>>>  - Remove default bootargs from chosen DT node
>>>  - Remove "linux" prefix from stdout DT attribute of chosen DT node
>>>  - Remove use of GIC_CPU_MASK_xxx() for PPIs
>>>
>>> Changes since v1:
>>>  - Rebased patches for Linux-4.12-rc1
>>>  - Removed unwanted /memreserve/ from bcm958742-base.dtsi
>>>  - Use ranges DT property to clear view of memory-layout
>>>  - Make bcm-sr.h part of clock DT bindings patch
>>>
>>> Anup Patel (3):
>>>   dt-bindings: bcm: Add Broadcom Stingray bindings document
>>>   arm64: dts: Initial DTS files for Broadcom Stingray SOC
>>>   arm64: dts: Add PL022, PL330 and SP805 DT nodes for Stingray
>>>
>>> Oza Pawandeep (1):
>>>   arm64: dts: Add I2C DT nodes for Stingray SoC
>>>
>>> Pramod Kumar (3):
>>>   arm64: dts: Add NAND DT nodes for Stingray SOC
>>>   arm64: dts: Add pinctrl DT nodes for Stingray SOC
>>>   arm64: dts: Add GPIO DT nodes for Stingray SOC
>>>
>>> Sandeep Tripathy (3):
>>>   dt-bindings: clk: Extend binding doc for Stingray SOC
>>>   clk: bcm: Add clocks for Stingray SOC
>>>   arm64: dts: Add clock DT nodes for Stingray SOC
>>>
>>> Srinath Mannam (1):
>>>   arm64: dts: Add PWM and SDHCI DT nodes for Stingray SOC
>>>
>>>  .../devicetree/bindings/arm/bcm/brcm,stingray.txt  |  12 +
>>>  .../bindings/clock/brcm,iproc-clocks.txt   |  76 
>>>  arch/arm64/boot/dts/broadcom/Makefile  |   1 +
>>>  arch/arm64/boot/dts/broadcom/stingray/Makefile |   6 +
>>>  .../boot/dts/broadcom/stingray/bcm958742-base.dtsi | 131 ++
>>>  .../boot/dts/broadcom/stingray/bcm958742k.dts  |  78 
>>>  .../boot/dts/broadcom/stingray/bcm958742t.dts  |  40 ++
>>>  .../boot/dts/broadcom/stingray/stingray-clock.dtsi | 170 
>>>  .../dts/broadcom/stingray/stingray-pinctrl.dtsi| 345 
>>>  .../arm64/boot/dts/broadcom/stingray/stingray.dtsi | 460 
>>> +
>>>  drivers/clk/bcm/Kconfig|   8 +
>>>  drivers/clk/bcm/Makefile   |   1 +
>>>  drivers/clk/bcm/clk-sr.c   | 300 ++
>>>  include/dt-bindings/clock/bcm-sr.h | 101 +
>>>  .../dt-bindings/pinctrl/brcm,pinctrl-stingray.h|  68 +++
>>>  15 files changed, 1797 insertions(+)
>>>  create mode 100644 
>>> Documentation/devicetree/bindings/arm/bcm/brcm,stingray.txt
>>>  create mode 100644 arch/arm64/boot/dts/broadcom/stingray/Makefile
>>>  create mode 100644 
>>> arch/arm64/boot/dts/broadcom/stingray/bcm958742-base.dtsi
>>>  create mode 100644 arch/arm64/boot/dts/broadcom/stingray/bcm958742k.dts
>>>  create mode 100644 arch/arm64/boot/dts/broadcom/stingray/bcm958742t.dts
>>>  create mode 100644 
>>> arch/arm64/boot/dts/broadcom/stingray/stingray-clock.dtsi
>>>  create mode 100644 
>>> arch/arm64/boot/dts/broadcom/stingray/stingray-pinctrl.dtsi
>>>  create mode 100644 arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
>>>  create mode 100644 drivers/clk/bcm/clk-sr.c
>>>  create mode 100644 include/dt-bindings/clock/bcm-sr.h
>>>  create mode 100644 include/dt-bindings/pinctrl/brcm,pinctrl-stingray.h
>>
>> Grouping as one patchset for review is fine, but when you submit this
>> for merge you need to split it up:
>>
>> - Documentation/devicetree can go with the driver pieces or with the
>> DT changes, your choice
>> - DTS/DTSI changes should go through the Broadcom maintainers to arm-soc
>> - clk changes should go to clk maintainers.
> 
> Only PATCH3 needs to go through clk maintainers rest all patches
> are DT bindings document and DTS changes which can go through
> Broadcom tree. Right ??

Patch 2 and 3 should go through the clock maintainers, and I can take
the remaining patches through the Broadcom ARM/ARM64 SoC pull requests,
no problem.

Unless there are specific changes requested which warrant a v4, I can
coordinate with Stephen and Mike to make sure that they take the
appropriate patches.

> 
>>
>> Also, there's usually no need to split up a DT introduction and having
>> separate pieces for NAND/pinctrl/GPIO/etc: They can all go in the main
>> DT submission patch.
> 
> Yes, we have already squashed DTS patches from same author into
> one DTS patch except NAND, PINCTRL and GPIO patches.
> 
> The NAND, PINCTRL, and GPIO patches were not squashed into
> one patch for easy review but since you suggest I will squash these
> three DTS patches into one DTS patch.
> 
> Regards,
> Anup
> 

Re: [PATCH v3 2/3] thermal: broadcom: ns-thermal: default on iProc SoCs

[Florian Fainelli]

On 04/28/2017 01:11 PM, Jon Mason wrote:
> Tweak the Kconfig description to mention support for NSP and make the
> default on for iProc based platforms.
> 
> Signed-off-by: Jon Mason 

Zhang, Eduardo, please apply, the Kconfig and DTS changes have been
queued for 4.13 in my ARM-SoC pull request for Broadcom SoCs.

Thanks!
--
Florian

Re: [PATCH v3 2/3] thermal: broadcom: ns-thermal: default on iProc SoCs

[Florian Fainelli]

On 04/28/2017 01:11 PM, Jon Mason wrote:
> Tweak the Kconfig description to mention support for NSP and make the
> default on for iProc based platforms.
> 
> Signed-off-by: Jon Mason 

Zhang, Eduardo, please apply, the Kconfig and DTS changes have been
queued for 4.13 in my ARM-SoC pull request for Broadcom SoCs.

Thanks!
--
Florian

Re: [PATCH v6 3/5] test: add new driver_data load tester

[Luis R. Rodriguez]

On Wed, May 17, 2017 at 05:45:22PM -0500, Li, Yi wrote:
> hi Luis
> 
> 
> On 5/11/2017 12:11 PM, Luis R. Rodriguez wrote:
> > On Thu, May 11, 2017 at 07:46:27PM +0900, AKASHI Takahiro wrote:
> > > Luis,
> > > 
> > > On Fri, Apr 28, 2017 at 03:45:35AM +0200, Luis R. Rodriguez wrote:
> > > > > > +To test an async call one could do::
> > > > > > +
> > > > > > +echo anything > /lib/firmware/test-driver_data.bin
> > > > > Your current shell script doesn't search for the firmware in
> > > > > /lib/firmware unless you explicitly specify $FWPATH.
> > > > This is true but that is the *test* shell script, and it purposely 
> > > > avoids the
> > > > existing firmware path to avoid overriding dummy test files on the 
> > > > production
> > > > path. So the above still stands as it is not using the test shell script
> > > > driver_data.sh.
> > > > 
> > > > I'll add a note:
> > > > 
> > > > """
> > > > Note that driver_data.sh uses its own temporary custom path for 
> > > > creating and
> > > > looking for driver data files, it does this to not overwrite any 
> > > > production
> > > > files you might have which may share the same names used by the test 
> > > > shell
> > > > script driver_data.sh. If you are not using the driver_data.sh script 
> > > > your
> > > > default path will be used.
> > > > """
> > > That looks fine, but I think we'd better change the line:
> > > 
> > > > > > +echo anything > /lib/firmware/test-driver_data.bin
> > > since it is just incorrect as far as driver_data.sh goes.
> > But that is accurate, given the default file we search for on 
> > test_driver_data.c
> > is test-driver_data.bin. It also does not create a conflict to overwrite a 
> > file
> > used on driver_data.sh as driver_data.sh uses a custom path. I think the 
> > note
> > above on custom path is sufficient for the developer or user to be aware of
> > the fact the driver_data.sh does it own thing, and that the example is just 
> > a
> > manual test case.
> > What do you mean by that its incorrect ?
> 
> I understand it now, but was on the same boat as Akashi. Renamed my 12MB
> test firmware binary to /lib/firmware/test-driver_data.bin, but
> driver_data.sh only read back 9 byes from the tmp "ABCD0123". :-)

Right, driver_data.sh is not to stress test a target file, it *builds*
tests and uses a temporary directory to avoid interfering with your own
files.

> What's the proper way to test a real image in the driver_data.sh script,
> change config_set_name?

Yes! You can review how it configures a test case to get an idea but the
name change is one, since you are doing it manually you would just avoid
resetting the firmware test path.

We could extend the script with an argument to leave the directory intact
and trust your own file given in an argument, but that would be a new test
case.

  Luis

Re: [PATCH v6 3/5] test: add new driver_data load tester

[Luis R. Rodriguez]

On Wed, May 17, 2017 at 05:45:22PM -0500, Li, Yi wrote:
> hi Luis
> 
> 
> On 5/11/2017 12:11 PM, Luis R. Rodriguez wrote:
> > On Thu, May 11, 2017 at 07:46:27PM +0900, AKASHI Takahiro wrote:
> > > Luis,
> > > 
> > > On Fri, Apr 28, 2017 at 03:45:35AM +0200, Luis R. Rodriguez wrote:
> > > > > > +To test an async call one could do::
> > > > > > +
> > > > > > +echo anything > /lib/firmware/test-driver_data.bin
> > > > > Your current shell script doesn't search for the firmware in
> > > > > /lib/firmware unless you explicitly specify $FWPATH.
> > > > This is true but that is the *test* shell script, and it purposely 
> > > > avoids the
> > > > existing firmware path to avoid overriding dummy test files on the 
> > > > production
> > > > path. So the above still stands as it is not using the test shell script
> > > > driver_data.sh.
> > > > 
> > > > I'll add a note:
> > > > 
> > > > """
> > > > Note that driver_data.sh uses its own temporary custom path for 
> > > > creating and
> > > > looking for driver data files, it does this to not overwrite any 
> > > > production
> > > > files you might have which may share the same names used by the test 
> > > > shell
> > > > script driver_data.sh. If you are not using the driver_data.sh script 
> > > > your
> > > > default path will be used.
> > > > """
> > > That looks fine, but I think we'd better change the line:
> > > 
> > > > > > +echo anything > /lib/firmware/test-driver_data.bin
> > > since it is just incorrect as far as driver_data.sh goes.
> > But that is accurate, given the default file we search for on 
> > test_driver_data.c
> > is test-driver_data.bin. It also does not create a conflict to overwrite a 
> > file
> > used on driver_data.sh as driver_data.sh uses a custom path. I think the 
> > note
> > above on custom path is sufficient for the developer or user to be aware of
> > the fact the driver_data.sh does it own thing, and that the example is just 
> > a
> > manual test case.
> > What do you mean by that its incorrect ?
> 
> I understand it now, but was on the same boat as Akashi. Renamed my 12MB
> test firmware binary to /lib/firmware/test-driver_data.bin, but
> driver_data.sh only read back 9 byes from the tmp "ABCD0123". :-)

Right, driver_data.sh is not to stress test a target file, it *builds*
tests and uses a temporary directory to avoid interfering with your own
files.

> What's the proper way to test a real image in the driver_data.sh script,
> change config_set_name?

Yes! You can review how it configures a test case to get an idea but the
name change is one, since you are doing it manually you would just avoid
resetting the firmware test path.

We could extend the script with an argument to leave the directory intact
and trust your own file given in an argument, but that would be a new test
case.

  Luis

Re: [PATCH v3 1/3] ARM: BCM: Enable thermal support for NSP SoCs

[Florian Fainelli]

On 04/28/2017 01:11 PM, Jon Mason wrote:
> Change the Northstar Plus Kconfig to select THERMAL and THERMAL_OF,
> which allows the ns-thermal driver to be selected via menuconfig.
> 
> Signed-off-by: Jon Mason 

Applied, thanks Jon
-- 
Florian

Re: [PATCH v3 1/3] ARM: BCM: Enable thermal support for NSP SoCs

[Florian Fainelli]

On 04/28/2017 01:11 PM, Jon Mason wrote:
> Change the Northstar Plus Kconfig to select THERMAL and THERMAL_OF,
> which allows the ns-thermal driver to be selected via menuconfig.
> 
> Signed-off-by: Jon Mason 

Applied, thanks Jon
-- 
Florian

Re: [PATCH v3 3/3] ARM: dts: NSP: Add Thermal Support

[Florian Fainelli]

On 04/28/2017 01:11 PM, Jon Mason wrote:
> Add thermal support via the ns-thermal driver and create a single
> thermal zone for the entire SoC.
> 
> Signed-off-by: Jon Mason 
> Acked-by: Eduardo Valentin 

Applied. thanks Jon
-- 
Florian

Re: [PATCH v3 3/3] ARM: dts: NSP: Add Thermal Support

[Florian Fainelli]

On 04/28/2017 01:11 PM, Jon Mason wrote:
> Add thermal support via the ns-thermal driver and create a single
> thermal zone for the entire SoC.
> 
> Signed-off-by: Jon Mason 
> Acked-by: Eduardo Valentin 

Applied. thanks Jon
-- 
Florian

Re: git merges of tags

[Linus Torvalds]

On Thu, May 18, 2017 at 4:23 PM, Stephen Rothwell  wrote:
>
> Just a reminder that if you are merging Linus' tree (or any tree
> really) via a tag, git was changed some time ago so that merging a tag
> will not do a fast forward (there is a good reason for this - I just
> can't recall it ATM).

The reason is that when you merge a signed tag, git squirrels away t
he signature into the merge commit, so that you can see and verify the
signage later (use "git log --show-signatures" to see the signatures
on the commits).

If you fast-forward, there isn't any new commit to add the signing data to.

> To do the fast forward, try "git merge ^{}" ...

A slightly simpler syntax might be just "tag^0", but yes, the "^{}"
thing peels off any tags.

>   (unfortunately
> doing "git merge --ff " also does not do a fast forward - it also
> doesn't fail, it unexpectedly just creates a merge commit :-().

"--ff" is the default behavior, and means "allow fast forward", but
note that it is about "allowing", not "forcing".

You can use "--ff-only" to say that you will _only_ accept a
fast-forward, and git will error out  if it needs to create a merge.

  Linus

Re: git merges of tags

[Linus Torvalds]

On Thu, May 18, 2017 at 4:23 PM, Stephen Rothwell  wrote:
>
> Just a reminder that if you are merging Linus' tree (or any tree
> really) via a tag, git was changed some time ago so that merging a tag
> will not do a fast forward (there is a good reason for this - I just
> can't recall it ATM).

The reason is that when you merge a signed tag, git squirrels away t
he signature into the merge commit, so that you can see and verify the
signage later (use "git log --show-signatures" to see the signatures
on the commits).

If you fast-forward, there isn't any new commit to add the signing data to.

> To do the fast forward, try "git merge ^{}" ...

A slightly simpler syntax might be just "tag^0", but yes, the "^{}"
thing peels off any tags.

>   (unfortunately
> doing "git merge --ff " also does not do a fast forward - it also
> doesn't fail, it unexpectedly just creates a merge commit :-().

"--ff" is the default behavior, and means "allow fast forward", but
note that it is about "allowing", not "forcing".

You can use "--ff-only" to say that you will _only_ accept a
fast-forward, and git will error out  if it needs to create a merge.

  Linus

Re: [RFC PATCH 02/11] drm: sun4i: add support for H3 mixers

[Maxime Ripard]

On Thu, May 18, 2017 at 12:43:45AM +0800, Icenowy Zheng wrote:
> From: Icenowy Zheng 
> 
> Allwinner H3 SoC has two mixers, one has 1 VI channel and 3 UI channels,
> and the other has 1 VI and 1 UI.
> 
> Add support for these two variants.
> 
> Signed-off-by: Icenowy Zheng 
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
> b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> index cb193c5f1686..d658a3a8159a 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -390,11 +390,29 @@ static const struct sun8i_mixer_cfg sun8i_v3s_mixer_cfg 
> = {
>   .ui_num = 1,
>  };
>  
> +static const struct sun8i_mixer_cfg sun8i_h3_mixer0_cfg = {
> + .vi_num = 1,
> + .ui_num = 3,
> +};
> +
> +static const struct sun8i_mixer_cfg sun8i_h3_mixer1_cfg = {
> + .vi_num = 1,
> + .ui_num = 1,
> +};
> +
>  static const struct of_device_id sun8i_mixer_of_table[] = {
>   {
>   .compatible = "allwinner,sun8i-v3s-de2-mixer",
>   .data = _v3s_mixer_cfg,
>   },
> + {
> + .compatible = "allwinner,sun8i-h3-de2-mixer0",
> + .data = _h3_mixer0_cfg
> + },
> + {
> + .compatible = "allwinner,sun8i-h3-de2-mixer1",
> + .data = _h3_mixer1_cfg
> + },

So the only difference between the two is the number of ui planes?

Why not create a property to give the number then, instead of a
compatible?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [RFC PATCH 02/11] drm: sun4i: add support for H3 mixers

[Maxime Ripard]

On Thu, May 18, 2017 at 12:43:45AM +0800, Icenowy Zheng wrote:
> From: Icenowy Zheng 
> 
> Allwinner H3 SoC has two mixers, one has 1 VI channel and 3 UI channels,
> and the other has 1 VI and 1 UI.
> 
> Add support for these two variants.
> 
> Signed-off-by: Icenowy Zheng 
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
> b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> index cb193c5f1686..d658a3a8159a 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -390,11 +390,29 @@ static const struct sun8i_mixer_cfg sun8i_v3s_mixer_cfg 
> = {
>   .ui_num = 1,
>  };
>  
> +static const struct sun8i_mixer_cfg sun8i_h3_mixer0_cfg = {
> + .vi_num = 1,
> + .ui_num = 3,
> +};
> +
> +static const struct sun8i_mixer_cfg sun8i_h3_mixer1_cfg = {
> + .vi_num = 1,
> + .ui_num = 1,
> +};
> +
>  static const struct of_device_id sun8i_mixer_of_table[] = {
>   {
>   .compatible = "allwinner,sun8i-v3s-de2-mixer",
>   .data = _v3s_mixer_cfg,
>   },
> + {
> + .compatible = "allwinner,sun8i-h3-de2-mixer0",
> + .data = _h3_mixer0_cfg
> + },
> + {
> + .compatible = "allwinner,sun8i-h3-de2-mixer1",
> + .data = _h3_mixer1_cfg
> + },

So the only difference between the two is the number of ui planes?

Why not create a property to give the number then, instead of a
compatible?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

[PATCH] efi-pstore: Fix write/erase id tracking

[Kees Cook]

Prior to the pstore interface refactoring, the "id" generated during
a backend pstore_write() was only retained by the internal pstore
inode tracking list. Additionally the "part" was ignored, so EFI
would encode this in the id. This corrects the misunderstandings
and correctly sets "id" during pstore_write(), and uses "part"
directly during pstore_erase().

Reported-by: Marta Lofstedt 
Fixes: 76cc9580e3fb ("pstore: Replace arguments for write() API")
Fixes: a61072aae693 ("pstore: Replace arguments for erase() API")
Signed-off-by: Kees Cook 
---
Since the pstore refactor broke this, I intend to push this via the
pstore tree.
---
 drivers/firmware/efi/efi-pstore.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/efi/efi-pstore.c 
b/drivers/firmware/efi/efi-pstore.c
index 44148fd4c9f2..dda2e96328c0 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -53,6 +53,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
if (sscanf(name, "dump-type%u-%u-%d-%lu-%c",
   >type, , , , _type) == 5) {
record->id = generic_id(time, part, cnt);
+   record->part = part;
record->count = cnt;
record->time.tv_sec = time;
record->time.tv_nsec = 0;
@@ -64,6 +65,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
} else if (sscanf(name, "dump-type%u-%u-%d-%lu",
   >type, , , ) == 4) {
record->id = generic_id(time, part, cnt);
+   record->part = part;
record->count = cnt;
record->time.tv_sec = time;
record->time.tv_nsec = 0;
@@ -77,6 +79,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
 * multiple logs, remains.
 */
record->id = generic_id(time, part, 0);
+   record->part = part;
record->count = 0;
record->time.tv_sec = time;
record->time.tv_nsec = 0;
@@ -241,9 +244,15 @@ static int efi_pstore_write(struct pstore_record *record)
efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
int i, ret = 0;
 
+   record->time.tv_sec = get_seconds();
+   record->time.tv_nsec = 0;
+
+   record->id = generic_id(record->time.tv_sec, record->part,
+   record->count);
+
snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lu-%c",
 record->type, record->part, record->count,
-get_seconds(), record->compressed ? 'C' : 'D');
+record->time.tv_sec, record->compressed ? 'C' : 'D');
 
for (i = 0; i < DUMP_NAME_LEN; i++)
efi_name[i] = name[i];
@@ -255,7 +264,6 @@ static int efi_pstore_write(struct pstore_record *record)
if (record->reason == KMSG_DUMP_OOPS)
efivar_run_worker();
 
-   record->id = record->part;
return ret;
 };
 
@@ -287,7 +295,7 @@ static int efi_pstore_erase_func(struct efivar_entry 
*entry, void *data)
 * holding multiple logs, remains.
 */
snprintf(name_old, sizeof(name_old), "dump-type%u-%u-%lu",
-   ed->record->type, (unsigned int)ed->record->id,
+   ed->record->type, ed->record->part,
ed->record->time.tv_sec);
 
for (i = 0; i < DUMP_NAME_LEN; i++)
@@ -320,10 +328,7 @@ static int efi_pstore_erase(struct pstore_record *record)
char name[DUMP_NAME_LEN];
efi_char16_t efi_name[DUMP_NAME_LEN];
int found, i;
-   unsigned int part;
 
-   do_div(record->id, 1000);
-   part = do_div(record->id, 100);
snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lu",
 record->type, record->part, record->count,
 record->time.tv_sec);
-- 
2.7.4


-- 
Kees Cook
Pixel Security

Re: next-20170515: WARNING: CPU: 0 PID: 1 at arch/x86/mm/dump_pagetables.c:236 note_page+0x630/0x7e0

[Andy Lutomirski]

On Fri, May 19, 2017 at 10:35 AM, Catalin Marinas
 wrote:
> On Fri, May 19, 2017 at 05:40:16PM +0200, Luis R. Rodriguez wrote:
>> If the following is a legit forced way to get query the kernel to ask it
>> who owns a page then perhaps this technique can be used in the future to
>> figure out who the hell caused this. Catalin, can you confirm? In this
>> case this is perhaps not a leaked page but I am trying to abuse the
>> kmemleak debugfs API to query who allocated the page. Is that fine?
>>
>> [0.916771] WARNING: CPU: 0 PID: 1 at arch/x86/mm/dump_pagetables.c:235 
>> note_page+0x63c/0x7e0
>> [0.917636] x86/mm: Found insecure W+X mapping at address 
>> c03d5000/0xc03d5000
>> [0.918502] Modules linked in:
>> [0.918819] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
>> 4.11.0-mcgrof-force-config #340
>> [0.919631] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
>> rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014
>> [0.920011] Call Trace:
>> [0.920011]  dump_stack+0x63/0x81
>> [0.920011]  __warn+0xcb/0xf0
>> [0.920011]  warn_slowpath_fmt+0x5a/0x80
>> [0.920011]  note_page+0x63c/0x7e0
>> [0.920011]  ptdump_walk_pgd_level_core+0x3b1/0x460
>> [0.920011]  ? 0x86c0
>> [0.920011]  ptdump_walk_pgd_level_checkwx+0x17/0x20
>> [0.920011]  mark_rodata_ro+0xf4/0x100
>> [0.920011]  ? rest_init+0x80/0x80
>> [0.920011]  kernel_init+0x2a/0x100
>> [0.920011]  ret_from_fork+0x2c/0x40
>> [0.925474] ---[ end trace dca00cd779490a2b ]---
>> [0.925959] x86/mm: Checked W+X mappings: FAILED, 1 W+X pages found.
>>
>> echo dump=0xc03d5000 > /sys/kernel/debug/kmemleak
>> dmesg | tail
>>
>> [   49.209565] kmemleak: Object 0xc03d5000 (size 335):
>> [   49.210814] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892440
>> [   49.212148] kmemleak:   min_count = 2
>> [   49.212852] kmemleak:   count = 0
>> [   49.213363] kmemleak:   flags = 0x1
>> [   49.213363] kmemleak:   checksum = 0
>> [   49.213363] kmemleak:   backtrace:
>> [   49.213363]  kmemleak_alloc+0x4a/0xa0
>> [   49.213363]  __vmalloc_node_range+0x20a/0x2b0
>> [   49.213363]  module_alloc+0x67/0xc0
>> [   49.213363]  arch_ftrace_update_trampoline+0xba/0x260
>> [   49.213363]  ftrace_startup+0x90/0x210
>> [   49.213363]  register_ftrace_function+0x4b/0x60
>> [   49.213363]  arm_kprobe+0x84/0xe0
>> [   49.213363]  register_kprobe+0x56e/0x5b0
>> [   49.213363]  init_test_probes+0x61/0x560
>> [   49.213363]  init_kprobes+0x1e3/0x206
>> [   49.213363]  do_one_initcall+0x52/0x1a0
>> [   49.213363]  kernel_init_freeable+0x178/0x200
>> [   49.213363]  kernel_init+0xe/0x100
>> [   49.213363]  ret_from_fork+0x2c/0x40
>> [   49.213363]  0x
>
> You could as well use kmemleak this way since it tracks the memory
> allocations. However, it doesn't track alloc_pages and also doesn't
> track mapping existing pages (vmap etc.)

One thing I've pondered: can we make some debugging mode (kmemleak,
perhaps?) check that freed memory is RW at the time it's freed?  I
once wrote some buggy code that freed an R page and caused an OOPS
much later, and this bug here seems likely to be some code that frees
RWX memory.

--Andy

[PATCH] efi-pstore: Fix write/erase id tracking

[Kees Cook]

Prior to the pstore interface refactoring, the "id" generated during
a backend pstore_write() was only retained by the internal pstore
inode tracking list. Additionally the "part" was ignored, so EFI
would encode this in the id. This corrects the misunderstandings
and correctly sets "id" during pstore_write(), and uses "part"
directly during pstore_erase().

Reported-by: Marta Lofstedt 
Fixes: 76cc9580e3fb ("pstore: Replace arguments for write() API")
Fixes: a61072aae693 ("pstore: Replace arguments for erase() API")
Signed-off-by: Kees Cook 
---
Since the pstore refactor broke this, I intend to push this via the
pstore tree.
---
 drivers/firmware/efi/efi-pstore.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/efi/efi-pstore.c 
b/drivers/firmware/efi/efi-pstore.c
index 44148fd4c9f2..dda2e96328c0 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -53,6 +53,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
if (sscanf(name, "dump-type%u-%u-%d-%lu-%c",
   >type, , , , _type) == 5) {
record->id = generic_id(time, part, cnt);
+   record->part = part;
record->count = cnt;
record->time.tv_sec = time;
record->time.tv_nsec = 0;
@@ -64,6 +65,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
} else if (sscanf(name, "dump-type%u-%u-%d-%lu",
   >type, , , ) == 4) {
record->id = generic_id(time, part, cnt);
+   record->part = part;
record->count = cnt;
record->time.tv_sec = time;
record->time.tv_nsec = 0;
@@ -77,6 +79,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry,
 * multiple logs, remains.
 */
record->id = generic_id(time, part, 0);
+   record->part = part;
record->count = 0;
record->time.tv_sec = time;
record->time.tv_nsec = 0;
@@ -241,9 +244,15 @@ static int efi_pstore_write(struct pstore_record *record)
efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
int i, ret = 0;
 
+   record->time.tv_sec = get_seconds();
+   record->time.tv_nsec = 0;
+
+   record->id = generic_id(record->time.tv_sec, record->part,
+   record->count);
+
snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lu-%c",
 record->type, record->part, record->count,
-get_seconds(), record->compressed ? 'C' : 'D');
+record->time.tv_sec, record->compressed ? 'C' : 'D');
 
for (i = 0; i < DUMP_NAME_LEN; i++)
efi_name[i] = name[i];
@@ -255,7 +264,6 @@ static int efi_pstore_write(struct pstore_record *record)
if (record->reason == KMSG_DUMP_OOPS)
efivar_run_worker();
 
-   record->id = record->part;
return ret;
 };
 
@@ -287,7 +295,7 @@ static int efi_pstore_erase_func(struct efivar_entry 
*entry, void *data)
 * holding multiple logs, remains.
 */
snprintf(name_old, sizeof(name_old), "dump-type%u-%u-%lu",
-   ed->record->type, (unsigned int)ed->record->id,
+   ed->record->type, ed->record->part,
ed->record->time.tv_sec);
 
for (i = 0; i < DUMP_NAME_LEN; i++)
@@ -320,10 +328,7 @@ static int efi_pstore_erase(struct pstore_record *record)
char name[DUMP_NAME_LEN];
efi_char16_t efi_name[DUMP_NAME_LEN];
int found, i;
-   unsigned int part;
 
-   do_div(record->id, 1000);
-   part = do_div(record->id, 100);
snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lu",
 record->type, record->part, record->count,
 record->time.tv_sec);
-- 
2.7.4


-- 
Kees Cook
Pixel Security

Re: next-20170515: WARNING: CPU: 0 PID: 1 at arch/x86/mm/dump_pagetables.c:236 note_page+0x630/0x7e0

[Andy Lutomirski]

On Fri, May 19, 2017 at 10:35 AM, Catalin Marinas
 wrote:
> On Fri, May 19, 2017 at 05:40:16PM +0200, Luis R. Rodriguez wrote:
>> If the following is a legit forced way to get query the kernel to ask it
>> who owns a page then perhaps this technique can be used in the future to
>> figure out who the hell caused this. Catalin, can you confirm? In this
>> case this is perhaps not a leaked page but I am trying to abuse the
>> kmemleak debugfs API to query who allocated the page. Is that fine?
>>
>> [0.916771] WARNING: CPU: 0 PID: 1 at arch/x86/mm/dump_pagetables.c:235 
>> note_page+0x63c/0x7e0
>> [0.917636] x86/mm: Found insecure W+X mapping at address 
>> c03d5000/0xc03d5000
>> [0.918502] Modules linked in:
>> [0.918819] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
>> 4.11.0-mcgrof-force-config #340
>> [0.919631] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
>> rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014
>> [0.920011] Call Trace:
>> [0.920011]  dump_stack+0x63/0x81
>> [0.920011]  __warn+0xcb/0xf0
>> [0.920011]  warn_slowpath_fmt+0x5a/0x80
>> [0.920011]  note_page+0x63c/0x7e0
>> [0.920011]  ptdump_walk_pgd_level_core+0x3b1/0x460
>> [0.920011]  ? 0x86c0
>> [0.920011]  ptdump_walk_pgd_level_checkwx+0x17/0x20
>> [0.920011]  mark_rodata_ro+0xf4/0x100
>> [0.920011]  ? rest_init+0x80/0x80
>> [0.920011]  kernel_init+0x2a/0x100
>> [0.920011]  ret_from_fork+0x2c/0x40
>> [0.925474] ---[ end trace dca00cd779490a2b ]---
>> [0.925959] x86/mm: Checked W+X mappings: FAILED, 1 W+X pages found.
>>
>> echo dump=0xc03d5000 > /sys/kernel/debug/kmemleak
>> dmesg | tail
>>
>> [   49.209565] kmemleak: Object 0xc03d5000 (size 335):
>> [   49.210814] kmemleak:   comm "swapper/0", pid 1, jiffies 4294892440
>> [   49.212148] kmemleak:   min_count = 2
>> [   49.212852] kmemleak:   count = 0
>> [   49.213363] kmemleak:   flags = 0x1
>> [   49.213363] kmemleak:   checksum = 0
>> [   49.213363] kmemleak:   backtrace:
>> [   49.213363]  kmemleak_alloc+0x4a/0xa0
>> [   49.213363]  __vmalloc_node_range+0x20a/0x2b0
>> [   49.213363]  module_alloc+0x67/0xc0
>> [   49.213363]  arch_ftrace_update_trampoline+0xba/0x260
>> [   49.213363]  ftrace_startup+0x90/0x210
>> [   49.213363]  register_ftrace_function+0x4b/0x60
>> [   49.213363]  arm_kprobe+0x84/0xe0
>> [   49.213363]  register_kprobe+0x56e/0x5b0
>> [   49.213363]  init_test_probes+0x61/0x560
>> [   49.213363]  init_kprobes+0x1e3/0x206
>> [   49.213363]  do_one_initcall+0x52/0x1a0
>> [   49.213363]  kernel_init_freeable+0x178/0x200
>> [   49.213363]  kernel_init+0xe/0x100
>> [   49.213363]  ret_from_fork+0x2c/0x40
>> [   49.213363]  0x
>
> You could as well use kmemleak this way since it tracks the memory
> allocations. However, it doesn't track alloc_pages and also doesn't
> track mapping existing pages (vmap etc.)

One thing I've pondered: can we make some debugging mode (kmemleak,
perhaps?) check that freed memory is RW at the time it's freed?  I
once wrote some buggy code that freed an R page and caused an OOPS
much later, and this bug here seems likely to be some code that frees
RWX memory.

--Andy

[PATCH v2] zlib: Mark get_unaligned16() as __maybe_unused

[Matthias Kaehlcke]

The function is not used with all configurations. Adding the attribute
fixes the following warning when building with clang and
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y:

lib/zlib_inflate/inffast.c:31:1: error: unused function 'get_unaligned16'
[-Werror,-Wunused-function]
get_unaligned16(const unsigned short *p)

Signed-off-by: Matthias Kaehlcke 
---
Changes in v2:
- explicitly include  where __maybe_unused is defined

 lib/zlib_inflate/inffast.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c
index 2c13ecc5bb2c..bae26afdc565 100644
--- a/lib/zlib_inflate/inffast.c
+++ b/lib/zlib_inflate/inffast.c
@@ -3,6 +3,7 @@
  * For conditions of distribution and use, see copyright notice in zlib.h
  */
 
+#include 
 #include 
 #include "inftrees.h"
 #include "inflate.h"
@@ -27,7 +28,7 @@ union uu {
 };
 
 /* Endian independed version */
-static inline unsigned short
+static inline unsigned short __maybe_unused
 get_unaligned16(const unsigned short *p)
 {
union uu  mm;
-- 
2.13.0.303.g4ebf302169-goog

[PATCH v2] zlib: Mark get_unaligned16() as __maybe_unused

[Matthias Kaehlcke]

The function is not used with all configurations. Adding the attribute
fixes the following warning when building with clang and
CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y:

lib/zlib_inflate/inffast.c:31:1: error: unused function 'get_unaligned16'
[-Werror,-Wunused-function]
get_unaligned16(const unsigned short *p)

Signed-off-by: Matthias Kaehlcke 
---
Changes in v2:
- explicitly include  where __maybe_unused is defined

 lib/zlib_inflate/inffast.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c
index 2c13ecc5bb2c..bae26afdc565 100644
--- a/lib/zlib_inflate/inffast.c
+++ b/lib/zlib_inflate/inffast.c
@@ -3,6 +3,7 @@
  * For conditions of distribution and use, see copyright notice in zlib.h
  */
 
+#include 
 #include 
 #include "inftrees.h"
 #include "inflate.h"
@@ -27,7 +28,7 @@ union uu {
 };
 
 /* Endian independed version */
-static inline unsigned short
+static inline unsigned short __maybe_unused
 get_unaligned16(const unsigned short *p)
 {
union uu  mm;
-- 
2.13.0.303.g4ebf302169-goog

Re: [linux-sunxi] Re: [RFC PATCH 10/11] ARM: sun8i: h3: add display engine pipeline for TVE

[Icenowy Zheng]

于 2017年5月20日 GMT+08:00 上午2:06:16, Maxime Ripard 
 写到:
>On Thu, May 18, 2017 at 12:43:53AM +0800, Icenowy Zheng wrote:
>> As we have already the support for the TV encoder on Allwinner H3,
>add
>> the display engine pipeline device tree nodes to its DTSI file.
>> 
>> The H5 pipeline has some differences and will be enabled later.
>> 
>> The currently-unused mixer0 and tcon0 are also needed, for the
>> completement of the pipeline.
>> 
>> Signed-off-by: Icenowy Zheng 
>> ---
>>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
>
>>  1 file changed, 189 insertions(+)
>> 
>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>b/arch/arm/boot/dts/sun8i-h3.dtsi
>> index b36f9f423c39..20172ef92415 100644
>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> @@ -41,6 +41,8 @@
>>   */
>>  
>>  #include "sunxi-h3-h5.dtsi"
>> +#include 
>> +#include 
>>  
>>  / {
>>  cpus {
>> @@ -72,6 +74,193 @@
>>  };
>>  };
>>  
>> +de: display-engine {
>> +compatible = "allwinner,sun8i-h3-display-engine";
>> +allwinner,pipelines = <>,
>> +  <>;
>> +status = "disabled";
>> +};
>> +
>> +soc {
>> +display_clocks: clock@100 {
>> +compatible = "allwinner,sun8i-a83t-de2-clk";
>> +reg = <0x0100 0x10>;
>> +clocks = < CLK_BUS_DE>,
>> + < CLK_DE>;
>> +clock-names = "bus",
>> +  "mod";
>> +resets = < RST_BUS_DE>;
>> +#clock-cells = <1>;
>> +#reset-cells = <1>;
>> +assigned-clocks = < CLK_DE>;
>> +assigned-clock-parents = < CLK_PLL_DE>;
>> +assigned-clock-rates = <43200>;
>
>This shouldn't be set in the DT, but evaluated at runtime when calling
>clk_set_rate.

Nope, DE2 clock doesn't need evalution, as the clock is decoupled with
DE2 mixers' output signal. (Although it seems that SoCs with larger
plane size will use higher clock.)

And setting it to 432MHz is also needed for properly 216MHz clock to TVE.

>
>> +tve0: tv-encoder@1e0 {
>> +compatible = "allwinner,sun8i-h3-tv-encoder";
>> +reg = <0x01e0 0x1000>;
>> +clocks = < CLK_BUS_TVE>, < CLK_TVE>;
>> +clock-names = "bus", "mod";
>> +resets = < RST_BUS_TVE>;
>> +status = "disabled";
>> +
>> +assigned-clocks = < CLK_TVE>;
>> +assigned-clock-parents = < CLK_PLL_DE>;
>
>Same thing here. clk_set_rate should just do the right thing.
>
>> +assigned-clock-rates = <21600>;
>
>And why are you setting it in the driver and in the DT?
>
>Maxime

Re: [linux-sunxi] Re: [RFC PATCH 10/11] ARM: sun8i: h3: add display engine pipeline for TVE

[Icenowy Zheng]

于 2017年5月20日 GMT+08:00 上午2:06:16, Maxime Ripard 
 写到:
>On Thu, May 18, 2017 at 12:43:53AM +0800, Icenowy Zheng wrote:
>> As we have already the support for the TV encoder on Allwinner H3,
>add
>> the display engine pipeline device tree nodes to its DTSI file.
>> 
>> The H5 pipeline has some differences and will be enabled later.
>> 
>> The currently-unused mixer0 and tcon0 are also needed, for the
>> completement of the pipeline.
>> 
>> Signed-off-by: Icenowy Zheng 
>> ---
>>  arch/arm/boot/dts/sun8i-h3.dtsi | 189
>
>>  1 file changed, 189 insertions(+)
>> 
>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
>b/arch/arm/boot/dts/sun8i-h3.dtsi
>> index b36f9f423c39..20172ef92415 100644
>> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> @@ -41,6 +41,8 @@
>>   */
>>  
>>  #include "sunxi-h3-h5.dtsi"
>> +#include 
>> +#include 
>>  
>>  / {
>>  cpus {
>> @@ -72,6 +74,193 @@
>>  };
>>  };
>>  
>> +de: display-engine {
>> +compatible = "allwinner,sun8i-h3-display-engine";
>> +allwinner,pipelines = <>,
>> +  <>;
>> +status = "disabled";
>> +};
>> +
>> +soc {
>> +display_clocks: clock@100 {
>> +compatible = "allwinner,sun8i-a83t-de2-clk";
>> +reg = <0x0100 0x10>;
>> +clocks = < CLK_BUS_DE>,
>> + < CLK_DE>;
>> +clock-names = "bus",
>> +  "mod";
>> +resets = < RST_BUS_DE>;
>> +#clock-cells = <1>;
>> +#reset-cells = <1>;
>> +assigned-clocks = < CLK_DE>;
>> +assigned-clock-parents = < CLK_PLL_DE>;
>> +assigned-clock-rates = <43200>;
>
>This shouldn't be set in the DT, but evaluated at runtime when calling
>clk_set_rate.

Nope, DE2 clock doesn't need evalution, as the clock is decoupled with
DE2 mixers' output signal. (Although it seems that SoCs with larger
plane size will use higher clock.)

And setting it to 432MHz is also needed for properly 216MHz clock to TVE.

>
>> +tve0: tv-encoder@1e0 {
>> +compatible = "allwinner,sun8i-h3-tv-encoder";
>> +reg = <0x01e0 0x1000>;
>> +clocks = < CLK_BUS_TVE>, < CLK_TVE>;
>> +clock-names = "bus", "mod";
>> +resets = < RST_BUS_TVE>;
>> +status = "disabled";
>> +
>> +assigned-clocks = < CLK_TVE>;
>> +assigned-clock-parents = < CLK_PLL_DE>;
>
>Same thing here. clk_set_rate should just do the right thing.
>
>> +assigned-clock-rates = <21600>;
>
>And why are you setting it in the driver and in the DT?
>
>Maxime

Re: [PATCH] nvme: Change our APST table to be no more aggressive than Intel RSTe

[Andy Lutomirski]

On Fri, May 19, 2017 at 7:18 AM, Keith Busch  wrote:
> On Thu, May 18, 2017 at 11:35:05PM -0700, Christoph Hellwig wrote:
>> On Thu, May 18, 2017 at 06:13:55PM -0700, Andy Lutomirski wrote:
>> > a) Leave the Dell quirk in place until someone from Dell or Samsung
>> > figures out what's actually going on.  Add a blanket quirk turning off
>> > the deepest sleep state on all Intel devices [1] at least until
>> > someone from Intel figures out what's going on -- Hi, Keith!  Deal
>> > with any other problems as they're reported.
>>
>> I think we should just blacklist the 60p entirely.  It also seems to
>> corrupt data 100% reliable when used with XFS.
>
> I assume you're talking about the 600p/p3100. That family of devices
> prefer 4k alignment, and patch below will enforce that, fixing all
> access issues. I wasn't planning to post it because my understanding is
> an imminent f/w update will make it unnecessary.
>
> I understand there is a different issue specific to the KBL NUC platforms
> that exposes some other errata, but I don't know much about that.

We can quirk by firmware version.  The report said:

vid : 0x8086
ssvid : 0x8086
sn : BTPY63850F281P0H
mn : INTEL SSDPEKKW010T7
fr : PSF104C

Any chance you can check what firmware versions have the issue?

--Andy

Re: [linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Jernej Škrabec]

Hi,

Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard  写到:
> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> >> Allwinner H3 features a TV encoder similar to the one in earlier
> >
> >SoCs,
> >
> >> but with some different points about clocks:
> >> - It has a mod clock and a bus clock.
> >> - The mod clock must be at a fixed rate to generate signal.
> >
> >Why?
> 
> It's experiment result by Jernej.
> 
> The clock rates in BSP kernel is also specially designed
> (PLL_DE at 432MHz) in order to be able to feed the TVE.

My experiments and search through BSP code showed that TVE seems to have 
additional fixed predivider 8. So if you want to generate 27 MHz clock, unit 
has to be feed with 216 MHz. 

TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for DE2, 
BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz. This 
clock is then divided by 8 internaly to get final 27 MHz.

Please note that I don't have any hard evidence to support that, only 
experimental data. However, only that explanation make sense to me.

BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz base 
clock. Further experiments are needed to check if there is any possibility to 
have other resolutions by manipulating clocks and give other proper settings. 
I plan to do that, but not in very near future.

Best regards,
Jernej

Re: [PATCH] nvme: Change our APST table to be no more aggressive than Intel RSTe

[Andy Lutomirski]

On Fri, May 19, 2017 at 7:18 AM, Keith Busch  wrote:
> On Thu, May 18, 2017 at 11:35:05PM -0700, Christoph Hellwig wrote:
>> On Thu, May 18, 2017 at 06:13:55PM -0700, Andy Lutomirski wrote:
>> > a) Leave the Dell quirk in place until someone from Dell or Samsung
>> > figures out what's actually going on.  Add a blanket quirk turning off
>> > the deepest sleep state on all Intel devices [1] at least until
>> > someone from Intel figures out what's going on -- Hi, Keith!  Deal
>> > with any other problems as they're reported.
>>
>> I think we should just blacklist the 60p entirely.  It also seems to
>> corrupt data 100% reliable when used with XFS.
>
> I assume you're talking about the 600p/p3100. That family of devices
> prefer 4k alignment, and patch below will enforce that, fixing all
> access issues. I wasn't planning to post it because my understanding is
> an imminent f/w update will make it unnecessary.
>
> I understand there is a different issue specific to the KBL NUC platforms
> that exposes some other errata, but I don't know much about that.

We can quirk by firmware version.  The report said:

vid : 0x8086
ssvid : 0x8086
sn : BTPY63850F281P0H
mn : INTEL SSDPEKKW010T7
fr : PSF104C

Any chance you can check what firmware versions have the issue?

--Andy

Re: [linux-sunxi] Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Jernej Škrabec]

Hi,

Dne petek, 19. maj 2017 ob 20:08:18 CEST je Icenowy Zheng napisal(a):
> 于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard  写到:
> >On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
> >> Allwinner H3 features a TV encoder similar to the one in earlier
> >
> >SoCs,
> >
> >> but with some different points about clocks:
> >> - It has a mod clock and a bus clock.
> >> - The mod clock must be at a fixed rate to generate signal.
> >
> >Why?
> 
> It's experiment result by Jernej.
> 
> The clock rates in BSP kernel is also specially designed
> (PLL_DE at 432MHz) in order to be able to feed the TVE.

My experiments and search through BSP code showed that TVE seems to have 
additional fixed predivider 8. So if you want to generate 27 MHz clock, unit 
has to be feed with 216 MHz. 

TVE has only one PLL source PLL_DE. And since 216 MHz is a bit low for DE2, 
BSP defaults to 432 MHz for PLL_DE and use divider 2 to generate 216 MHz. This 
clock is then divided by 8 internaly to get final 27 MHz.

Please note that I don't have any hard evidence to support that, only 
experimental data. However, only that explanation make sense to me.

BTW, BSP H3/H5 TV driver supports only PAL and NTSC which both use 27 MHz base 
clock. Further experiments are needed to check if there is any possibility to 
have other resolutions by manipulating clocks and give other proper settings. 
I plan to do that, but not in very near future.

Best regards,
Jernej

Re: [PATCH][drm-next] drm/pl111: make structure pl111_display_funcs static

[Eric Anholt]

Jani Nikula  writes:

> On Fri, 19 May 2017, Colin King  wrote:
>> From: Colin Ian King 
>>
>> structure pl111_display_funcs can be made static as it does not need to be
>> in global scope.  Fixes sparse warning:
>>
>> "warning: symbol 'pl111_display_funcs' was not declared. Should it
>> be static?"
>>
>> Fixes: bed41005e6174d ("drm/pl111: Initial drm/kms driver for pl111")
>
> The patch looks good and I appreciate what you're doing, but I question
> the usefulness of adding Fixes: tags for trivial stuff like this. I'd
> prefer Fixes: was reserved for actual fixes that should be backported to
> any kernels that have the commit being fixed.

Agreed -- since Fixes implies going to stable, we don't want it on
non-stable-candidates like this.  Reviewed these two and will push
without the tag in a moment.


signature.asc
Description: PGP signature

Re: [PATCH][drm-next] drm/pl111: make structure pl111_display_funcs static

[Eric Anholt]

Jani Nikula  writes:

> On Fri, 19 May 2017, Colin King  wrote:
>> From: Colin Ian King 
>>
>> structure pl111_display_funcs can be made static as it does not need to be
>> in global scope.  Fixes sparse warning:
>>
>> "warning: symbol 'pl111_display_funcs' was not declared. Should it
>> be static?"
>>
>> Fixes: bed41005e6174d ("drm/pl111: Initial drm/kms driver for pl111")
>
> The patch looks good and I appreciate what you're doing, but I question
> the usefulness of adding Fixes: tags for trivial stuff like this. I'd
> prefer Fixes: was reserved for actual fixes that should be backported to
> any kernels that have the commit being fixed.

Agreed -- since Fixes implies going to stable, we don't want it on
non-stable-candidates like this.  Reviewed these two and will push
without the tag in a moment.


signature.asc
Description: PGP signature

Re: [PATCH v2] drm/pl111: Register the clock divider and use it.

[Eric Anholt]

Stephen Boyd  writes:

> On 05/08, Eric Anholt wrote:
>> This is required for the panel to work on bcm911360, where CLCDCLK is
>> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
>> CLCDCLK, for platforms that have a settable rate on that one.
>> 
>> v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
>> COMMON_CLK.
>> 
>> Signed-off-by: Eric Anholt 
>
> Reviewed-by: Stephen Boyd 
>
> One minor comment below
>
>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c 
>> b/drivers/gpu/drm/pl111/pl111_display.c
>> index 39a5c33bce7d..2d924a6bf43c 100644
>> --- a/drivers/gpu/drm/pl111/pl111_display.c
>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
>> @@ -288,6 +296,126 @@ const struct drm_simple_display_pipe_funcs 
>> pl111_display_funcs = {
> [...]
>> +
>> +return 0;
>> +}
>> +
>> +const struct clk_ops pl111_clk_div_ops = {
>
> static?

Fixed, thanks.


signature.asc
Description: PGP signature

Re: [PATCH v2] drm/pl111: Register the clock divider and use it.

[Eric Anholt]

Stephen Boyd  writes:

> On 05/08, Eric Anholt wrote:
>> This is required for the panel to work on bcm911360, where CLCDCLK is
>> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
>> CLCDCLK, for platforms that have a settable rate on that one.
>> 
>> v2: Set SET_RATE_PARENT (caught by Linus Walleij), depend on
>> COMMON_CLK.
>> 
>> Signed-off-by: Eric Anholt 
>
> Reviewed-by: Stephen Boyd 
>
> One minor comment below
>
>> diff --git a/drivers/gpu/drm/pl111/pl111_display.c 
>> b/drivers/gpu/drm/pl111/pl111_display.c
>> index 39a5c33bce7d..2d924a6bf43c 100644
>> --- a/drivers/gpu/drm/pl111/pl111_display.c
>> +++ b/drivers/gpu/drm/pl111/pl111_display.c
>> @@ -288,6 +296,126 @@ const struct drm_simple_display_pipe_funcs 
>> pl111_display_funcs = {
> [...]
>> +
>> +return 0;
>> +}
>> +
>> +const struct clk_ops pl111_clk_div_ops = {
>
> static?

Fixed, thanks.


signature.asc
Description: PGP signature

Re: [PATCH] drm/pl111: Add a debugfs node to dump our registers.

[Eric Anholt]

Eric Engestrom  writes:

> On Wednesday, 2017-05-17 17:56:40 -0700, Eric Anholt wrote:
>> While debugging an X11 display failure, I wanted to see where we were
>> actually scanning out from.  This is probably generally useful to
>> others that might be working on this device.
>> 
>> Signed-off-by: Eric Anholt 
>> ---
>>  drivers/gpu/drm/pl111/Makefile|  2 ++
>>  drivers/gpu/drm/pl111/pl111_debugfs.c | 55 
>> +++
>>  drivers/gpu/drm/pl111/pl111_drm.h |  3 ++
>>  drivers/gpu/drm/pl111/pl111_drv.c |  4 +++
>>  4 files changed, 64 insertions(+)
>>  create mode 100644 drivers/gpu/drm/pl111/pl111_debugfs.c
>> 
>> diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
>> index 01caee727c13..59483d610ef5 100644
>> --- a/drivers/gpu/drm/pl111/Makefile
>> +++ b/drivers/gpu/drm/pl111/Makefile
>> @@ -2,4 +2,6 @@ pl111_drm-y +=   pl111_connector.o \
>>  pl111_display.o \
>>  pl111_drv.o
>>  
>> +pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
>> +
>>  obj-$(CONFIG_DRM_PL111) += pl111_drm.o
>> diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c 
>> b/drivers/gpu/drm/pl111/pl111_debugfs.c
>> new file mode 100644
>> index ..ee13a060cddf
>> --- /dev/null
>> +++ b/drivers/gpu/drm/pl111/pl111_debugfs.c
>> @@ -0,0 +1,55 @@
>> +/*
>> + *  Copyright © 2017 Broadcom
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include "pl111_drm.h"
>> +
>> +#define REGDEF(reg) { reg, #reg }
>> +static const struct {
>> +uint32_t reg;
>> +const char *name;
>> +} pl111_reg_defs[] = {
>> +REGDEF(CLCD_TIM0),
>> +REGDEF(CLCD_TIM1),
>> +REGDEF(CLCD_TIM2),
>> +REGDEF(CLCD_TIM3),
>> +REGDEF(CLCD_UBAS),
>> +REGDEF(CLCD_PL111_CNTL),
>> +REGDEF(CLCD_PL111_IENB),
>> +};
>> +
>> +int pl111_debugfs_regs(struct seq_file *m, void *unused)
>> +{
>> +struct drm_info_node *node = (struct drm_info_node *)m->private;
>> +struct drm_device *dev = node->minor->dev;
>> +struct pl111_drm_dev_private *priv = dev->dev_private;
>> +int i;
>
> nit: this will print a warning; s/int/unsigned/ ?

I don't see any.  I think the warning is only for non-constant
expressions -- we do i < ARRAY_SIZE all over the place.


signature.asc
Description: PGP signature

Re: [PATCH] drm/pl111: Add a debugfs node to dump our registers.

[Eric Anholt]

Eric Engestrom  writes:

> On Wednesday, 2017-05-17 17:56:40 -0700, Eric Anholt wrote:
>> While debugging an X11 display failure, I wanted to see where we were
>> actually scanning out from.  This is probably generally useful to
>> others that might be working on this device.
>> 
>> Signed-off-by: Eric Anholt 
>> ---
>>  drivers/gpu/drm/pl111/Makefile|  2 ++
>>  drivers/gpu/drm/pl111/pl111_debugfs.c | 55 
>> +++
>>  drivers/gpu/drm/pl111/pl111_drm.h |  3 ++
>>  drivers/gpu/drm/pl111/pl111_drv.c |  4 +++
>>  4 files changed, 64 insertions(+)
>>  create mode 100644 drivers/gpu/drm/pl111/pl111_debugfs.c
>> 
>> diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
>> index 01caee727c13..59483d610ef5 100644
>> --- a/drivers/gpu/drm/pl111/Makefile
>> +++ b/drivers/gpu/drm/pl111/Makefile
>> @@ -2,4 +2,6 @@ pl111_drm-y +=   pl111_connector.o \
>>  pl111_display.o \
>>  pl111_drv.o
>>  
>> +pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
>> +
>>  obj-$(CONFIG_DRM_PL111) += pl111_drm.o
>> diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c 
>> b/drivers/gpu/drm/pl111/pl111_debugfs.c
>> new file mode 100644
>> index ..ee13a060cddf
>> --- /dev/null
>> +++ b/drivers/gpu/drm/pl111/pl111_debugfs.c
>> @@ -0,0 +1,55 @@
>> +/*
>> + *  Copyright © 2017 Broadcom
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include "pl111_drm.h"
>> +
>> +#define REGDEF(reg) { reg, #reg }
>> +static const struct {
>> +uint32_t reg;
>> +const char *name;
>> +} pl111_reg_defs[] = {
>> +REGDEF(CLCD_TIM0),
>> +REGDEF(CLCD_TIM1),
>> +REGDEF(CLCD_TIM2),
>> +REGDEF(CLCD_TIM3),
>> +REGDEF(CLCD_UBAS),
>> +REGDEF(CLCD_PL111_CNTL),
>> +REGDEF(CLCD_PL111_IENB),
>> +};
>> +
>> +int pl111_debugfs_regs(struct seq_file *m, void *unused)
>> +{
>> +struct drm_info_node *node = (struct drm_info_node *)m->private;
>> +struct drm_device *dev = node->minor->dev;
>> +struct pl111_drm_dev_private *priv = dev->dev_private;
>> +int i;
>
> nit: this will print a warning; s/int/unsigned/ ?

I don't see any.  I think the warning is only for non-constant
expressions -- we do i < ARRAY_SIZE all over the place.


signature.asc
Description: PGP signature

Re: [GIT PULL] bcm2835 DT fixes for 4.12

[Florian Fainelli]

On 05/16/2017 03:19 PM, Eric Anholt wrote:
> Hi Florian,
> 
> The following changes since commit 2ea659a9ef488125eb46da6eb571de5eae5c43f6:
> 
>   Linux 4.12-rc1 (2017-05-13 13:19:49 -0700)
> 
> are available in the git repository at:
> 
>   git://github.com/anholt/linux tags/bcm2835-dt-fixes-2017-05-16
> 
> for you to fetch changes up to b0804ed0cadd7e38d94d2f15cdcc0d9695818856:
> 
>   ARM: dts: bcm283x: Reserve first page for firmware (2017-05-15 15:05:29 
> -0700)
> 
> 
> This pull request brings in a fix for booting on SMP Raspberry Pis,
> particularly with maxcpus=1
> 
> 

Merged, thanks Eric
-- 
-- 
Florian

Re: [GIT PULL] bcm2835 DT fixes for 4.12

[Florian Fainelli]

On 05/16/2017 03:19 PM, Eric Anholt wrote:
> Hi Florian,
> 
> The following changes since commit 2ea659a9ef488125eb46da6eb571de5eae5c43f6:
> 
>   Linux 4.12-rc1 (2017-05-13 13:19:49 -0700)
> 
> are available in the git repository at:
> 
>   git://github.com/anholt/linux tags/bcm2835-dt-fixes-2017-05-16
> 
> for you to fetch changes up to b0804ed0cadd7e38d94d2f15cdcc0d9695818856:
> 
>   ARM: dts: bcm283x: Reserve first page for firmware (2017-05-15 15:05:29 
> -0700)
> 
> 
> This pull request brings in a fix for booting on SMP Raspberry Pis,
> particularly with maxcpus=1
> 
> 

Merged, thanks Eric
-- 
-- 
Florian

Re: [linux-sunxi] Re: [RFC PATCH 02/11] drm: sun4i: add support for H3 mixers

[Jernej Škrabec]

Hi!

Dne petek, 19. maj 2017 ob 19:49:58 CEST je Icenowy Zheng napisal(a):
> 于 2017年5月20日 GMT+08:00 上午1:47:29, Maxime Ripard  写到:
> >On Thu, May 18, 2017 at 12:43:45AM +0800, Icenowy Zheng wrote:
> >> From: Icenowy Zheng 
> >> 
> >> Allwinner H3 SoC has two mixers, one has 1 VI channel and 3 UI
> >
> >channels,
> >
> >> and the other has 1 VI and 1 UI.
> >> 
> >> Add support for these two variants.
> >> 
> >> Signed-off-by: Icenowy Zheng 
> >> ---
> >> 
> >>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 18 ++
> >>  1 file changed, 18 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> >
> >b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> >
> >> index cb193c5f1686..d658a3a8159a 100644
> >> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> >> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> >> @@ -390,11 +390,29 @@ static const struct sun8i_mixer_cfg
> >
> >sun8i_v3s_mixer_cfg = {
> >
> >>.ui_num = 1,
> >>  
> >>  };
> >> 
> >> +static const struct sun8i_mixer_cfg sun8i_h3_mixer0_cfg = {
> >> +  .vi_num = 1,
> >> +  .ui_num = 3,
> >> +};
> >> +
> >> +static const struct sun8i_mixer_cfg sun8i_h3_mixer1_cfg = {
> >> +  .vi_num = 1,
> >> +  .ui_num = 1,
> >> +};
> >> +
> >> 
> >>  static const struct of_device_id sun8i_mixer_of_table[] = {
> >>  
> >>{
> >>
> >>.compatible = "allwinner,sun8i-v3s-de2-mixer",
> >>.data = _v3s_mixer_cfg,
> >>
> >>},
> >> 
> >> +  {
> >> +  .compatible = "allwinner,sun8i-h3-de2-mixer0",
> >> +  .data = _h3_mixer0_cfg
> >> +  },
> >> +  {
> >> +  .compatible = "allwinner,sun8i-h3-de2-mixer1",
> >> +  .data = _h3_mixer1_cfg
> >> +  },
> >
> >So the only difference between the two is the number of ui planes?
> 
> Not only., but currently we only implemented this.
> 
> More functions differ, but we still don't support them...
> 

As far as I can tell, they only differ in ui & vi number of planes and between 
different SoCs, max plane size.

Icenowy,
Do you know any other property they differ? I think everything else is based 
mostly on ui & vi number of planes.

Best regards,
Jernej

> >Why not create a property to give the number then, instead of a
> >compatible?
> >
> >Maxime
> 
> --
> You received this message because you are subscribed to the Google Groups
> "linux-sunxi" group. To unsubscribe from this group and stop receiving
> emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [RFC PATCH 02/11] drm: sun4i: add support for H3 mixers

[Jernej Škrabec]

Hi!

Dne petek, 19. maj 2017 ob 19:49:58 CEST je Icenowy Zheng napisal(a):
> 于 2017年5月20日 GMT+08:00 上午1:47:29, Maxime Ripard  写到:
> >On Thu, May 18, 2017 at 12:43:45AM +0800, Icenowy Zheng wrote:
> >> From: Icenowy Zheng 
> >> 
> >> Allwinner H3 SoC has two mixers, one has 1 VI channel and 3 UI
> >
> >channels,
> >
> >> and the other has 1 VI and 1 UI.
> >> 
> >> Add support for these two variants.
> >> 
> >> Signed-off-by: Icenowy Zheng 
> >> ---
> >> 
> >>  drivers/gpu/drm/sun4i/sun8i_mixer.c | 18 ++
> >>  1 file changed, 18 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> >
> >b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> >
> >> index cb193c5f1686..d658a3a8159a 100644
> >> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> >> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> >> @@ -390,11 +390,29 @@ static const struct sun8i_mixer_cfg
> >
> >sun8i_v3s_mixer_cfg = {
> >
> >>.ui_num = 1,
> >>  
> >>  };
> >> 
> >> +static const struct sun8i_mixer_cfg sun8i_h3_mixer0_cfg = {
> >> +  .vi_num = 1,
> >> +  .ui_num = 3,
> >> +};
> >> +
> >> +static const struct sun8i_mixer_cfg sun8i_h3_mixer1_cfg = {
> >> +  .vi_num = 1,
> >> +  .ui_num = 1,
> >> +};
> >> +
> >> 
> >>  static const struct of_device_id sun8i_mixer_of_table[] = {
> >>  
> >>{
> >>
> >>.compatible = "allwinner,sun8i-v3s-de2-mixer",
> >>.data = _v3s_mixer_cfg,
> >>
> >>},
> >> 
> >> +  {
> >> +  .compatible = "allwinner,sun8i-h3-de2-mixer0",
> >> +  .data = _h3_mixer0_cfg
> >> +  },
> >> +  {
> >> +  .compatible = "allwinner,sun8i-h3-de2-mixer1",
> >> +  .data = _h3_mixer1_cfg
> >> +  },
> >
> >So the only difference between the two is the number of ui planes?
> 
> Not only., but currently we only implemented this.
> 
> More functions differ, but we still don't support them...
> 

As far as I can tell, they only differ in ui & vi number of planes and between 
different SoCs, max plane size.

Icenowy,
Do you know any other property they differ? I think everything else is based 
mostly on ui & vi number of planes.

Best regards,
Jernej

> >Why not create a property to give the number then, instead of a
> >compatible?
> >
> >Maxime
> 
> --
> You received this message because you are subscribed to the Google Groups
> "linux-sunxi" group. To unsubscribe from this group and stop receiving
> emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] Btrfs: work around maybe-uninitialized warning

[Liu Bo]

On Thu, May 18, 2017 at 03:33:29PM +0200, Arnd Bergmann wrote:
> A rewrite of btrfs_submit_direct_hook appears to have introduced a warning:
> 
> fs/btrfs/inode.c: In function 'btrfs_submit_direct_hook':
> fs/btrfs/inode.c:8467:14: error: 'bio' may be used uninitialized in this 
> function [-Werror=maybe-uninitialized]
> 
> Where the 'bio' variable was previously initialized unconditionally, it
> is now set in the "while (submit_len > 0)" loop that would never execute
> if submit_len is zero.
> 
> Assuming this cannot happen in practice, we can avoid the warning
> by simply replacing the while{} loop with a do{}while() loop so
> the compiler knows that it will always be entered at least once.
>

Thanks for the fix.  I think it's a false positve one and I've updated it in v2
with a 'struct bio *bio = NULL' to make compiler happy, could you please help
reveiw it?

Thanks,
-liubo

> Fixes: 0fd27e06c61b ("Btrfs: use bio_clone_bioset_partial to simplify DIO 
> submit")
> Signed-off-by: Arnd Bergmann 
> ---
> ---
>  fs/btrfs/inode.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 8c37b4fa4cbb..c62cf9593cb3 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -8497,7 +8497,7 @@ static int btrfs_submit_direct_hook(struct 
> btrfs_dio_private *dip,
>   /* bio split */
>   ASSERT(map_length <= INT_MAX);
>   atomic_inc(>pending_bios);
> - while (submit_len > 0) {
> + do {
>   clone_len = min_t(int, submit_len, map_length);
>  
>   /*
> @@ -8540,7 +8540,7 @@ static int btrfs_submit_direct_hook(struct 
> btrfs_dio_private *dip,
> start_sector << 9, _length, NULL, 0);
>   if (ret)
>   goto out_err;
> - }
> + } while (submit_len > 0);
>  
>  submit:
>   ret = __btrfs_submit_dio_bio(bio, inode, file_offset, skip_sum,
> -- 
> 2.9.0
> 

Re: [PATCH] Btrfs: work around maybe-uninitialized warning

[Liu Bo]

On Thu, May 18, 2017 at 03:33:29PM +0200, Arnd Bergmann wrote:
> A rewrite of btrfs_submit_direct_hook appears to have introduced a warning:
> 
> fs/btrfs/inode.c: In function 'btrfs_submit_direct_hook':
> fs/btrfs/inode.c:8467:14: error: 'bio' may be used uninitialized in this 
> function [-Werror=maybe-uninitialized]
> 
> Where the 'bio' variable was previously initialized unconditionally, it
> is now set in the "while (submit_len > 0)" loop that would never execute
> if submit_len is zero.
> 
> Assuming this cannot happen in practice, we can avoid the warning
> by simply replacing the while{} loop with a do{}while() loop so
> the compiler knows that it will always be entered at least once.
>

Thanks for the fix.  I think it's a false positve one and I've updated it in v2
with a 'struct bio *bio = NULL' to make compiler happy, could you please help
reveiw it?

Thanks,
-liubo

> Fixes: 0fd27e06c61b ("Btrfs: use bio_clone_bioset_partial to simplify DIO 
> submit")
> Signed-off-by: Arnd Bergmann 
> ---
> ---
>  fs/btrfs/inode.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 8c37b4fa4cbb..c62cf9593cb3 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -8497,7 +8497,7 @@ static int btrfs_submit_direct_hook(struct 
> btrfs_dio_private *dip,
>   /* bio split */
>   ASSERT(map_length <= INT_MAX);
>   atomic_inc(>pending_bios);
> - while (submit_len > 0) {
> + do {
>   clone_len = min_t(int, submit_len, map_length);
>  
>   /*
> @@ -8540,7 +8540,7 @@ static int btrfs_submit_direct_hook(struct 
> btrfs_dio_private *dip,
> start_sector << 9, _length, NULL, 0);
>   if (ret)
>   goto out_err;
> - }
> + } while (submit_len > 0);
>  
>  submit:
>   ret = __btrfs_submit_dio_bio(bio, inode, file_offset, skip_sum,
> -- 
> 2.9.0
> 

Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Icenowy Zheng]

于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard 
 写到:
>On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
>> Allwinner H3 features a TV encoder similar to the one in earlier
>SoCs,
>> but with some different points about clocks:
>> - It has a mod clock and a bus clock.
>> - The mod clock must be at a fixed rate to generate signal.
>
>Why?

It's experiment result by Jernej.

The clock rates in BSP kernel is also specially designed
(PLL_DE at 432MHz) in order to be able to feed the TVE.

>
>Maxime

[PATCH 3/4] ueagle-atm: Delete unnecessary return statements in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 19 May 2017 19:22:12 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: void function return statements are not generally useful

Thus remove such a statement in the affected functions.

Signed-off-by: Markus Elfring 
---
 drivers/usb/atm/ueagle-atm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index 9725e6f2f301..0ca4ff3e6683 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -1058,7 +1058,6 @@ static void __uea_load_page_e4(struct uea_softc *sc, u8 
pageno, int boot)
 
 bad:
uea_err(INS_TO_USBDEV(sc), "sending DSP block %u failed\n", blockno);
-   return;
 }
 
 static void uea_load_page_e4(struct work_struct *work)
@@ -2101,7 +2100,6 @@ static void uea_dispatch_cmv_e4(struct uea_softc *sc, 
struct intr_pkt *intr)
E4_FUNCTION_TYPE(cmv->wFunction),
E4_FUNCTION_SUBTYPE(cmv->wFunction));
uea_leaves(INS_TO_USBDEV(sc));
-   return;
 }
 
 static void uea_schedule_load_page_e1(struct uea_softc *sc,
-- 
2.13.0

Re: [RFC PATCH 07/11] drm: sun4i: add support for the TV encoder in H3 SoC

[Icenowy Zheng]

于 2017年5月20日 GMT+08:00 上午2:03:30, Maxime Ripard 
 写到:
>On Thu, May 18, 2017 at 12:43:50AM +0800, Icenowy Zheng wrote:
>> Allwinner H3 features a TV encoder similar to the one in earlier
>SoCs,
>> but with some different points about clocks:
>> - It has a mod clock and a bus clock.
>> - The mod clock must be at a fixed rate to generate signal.
>
>Why?

It's experiment result by Jernej.

The clock rates in BSP kernel is also specially designed
(PLL_DE at 432MHz) in order to be able to feed the TVE.

>
>Maxime

[PATCH 3/4] ueagle-atm: Delete unnecessary return statements in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 19 May 2017 19:22:12 +0200

The script "checkpatch.pl" pointed information out like the following.

WARNING: void function return statements are not generally useful

Thus remove such a statement in the affected functions.

Signed-off-by: Markus Elfring 
---
 drivers/usb/atm/ueagle-atm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index 9725e6f2f301..0ca4ff3e6683 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -1058,7 +1058,6 @@ static void __uea_load_page_e4(struct uea_softc *sc, u8 
pageno, int boot)
 
 bad:
uea_err(INS_TO_USBDEV(sc), "sending DSP block %u failed\n", blockno);
-   return;
 }
 
 static void uea_load_page_e4(struct work_struct *work)
@@ -2101,7 +2100,6 @@ static void uea_dispatch_cmv_e4(struct uea_softc *sc, 
struct intr_pkt *intr)
E4_FUNCTION_TYPE(cmv->wFunction),
E4_FUNCTION_SUBTYPE(cmv->wFunction));
uea_leaves(INS_TO_USBDEV(sc));
-   return;
 }
 
 static void uea_schedule_load_page_e1(struct uea_softc *sc,
-- 
2.13.0

Re: [PATCH] kconfig: always use user input symbols

[Tycho Andersen]

Hi Geert,

On Fri, May 19, 2017 at 07:29:05PM +0200, Geert Uytterhoeven wrote:
> Hi Tycho,
> 
> On Fri, May 19, 2017 at 5:08 PM, Tycho Andersen  wrote:
> > ...regardless of visibility.
> >
> > When a symbol that is not visible by default (e.g. PNFS_FLEXFILE_LAYOUT)
> > has a default value, it is impossible to set the value to something not the
> > default:
> >
> > ~/packages/linux render-symbol-inputs grep FLEXFILE .config
> > CONFIG_PNFS_FLEXFILE_LAYOUT=y
> > ~/packages/linux render-symbol-inputs make oldconfig
> > scripts/kconfig/conf  --oldconfig Kconfig
> > ~/packages/linux render-symbol-inputs grep FLEXFILE .config
> > CONFIG_PNFS_FLEXFILE_LAYOUT=m
> >
> > There are two reasons for this: the symbol's user input value is only
> > considered when it is visible (hunks 2 and 3), and the user values are
> > explicitly ignored (hunk 1) if the symbols are not visible.
> >
> > It's not clear to me why hunk 1 exists. I'm sure it solve some problem, but
> > I'm not sure why we would ever want to discard user input values, and
> > causes a problem exactly as the comment describes.
> 
> This is intentional.  If a symbol is not visible, it's not meant to be changed
> by the user, as doing so may break things (at build or runtime).
> 
> E.g. running "make oldconfig" after a kernel upgrade may retain the old
> state of a variable, which is now invalid.  Ignoring invisible symbols
> avoids this.

Makes sense. Thanks!

Tycho

Re: [PATCH] kconfig: always use user input symbols

[Tycho Andersen]

Hi Geert,

On Fri, May 19, 2017 at 07:29:05PM +0200, Geert Uytterhoeven wrote:
> Hi Tycho,
> 
> On Fri, May 19, 2017 at 5:08 PM, Tycho Andersen  wrote:
> > ...regardless of visibility.
> >
> > When a symbol that is not visible by default (e.g. PNFS_FLEXFILE_LAYOUT)
> > has a default value, it is impossible to set the value to something not the
> > default:
> >
> > ~/packages/linux render-symbol-inputs grep FLEXFILE .config
> > CONFIG_PNFS_FLEXFILE_LAYOUT=y
> > ~/packages/linux render-symbol-inputs make oldconfig
> > scripts/kconfig/conf  --oldconfig Kconfig
> > ~/packages/linux render-symbol-inputs grep FLEXFILE .config
> > CONFIG_PNFS_FLEXFILE_LAYOUT=m
> >
> > There are two reasons for this: the symbol's user input value is only
> > considered when it is visible (hunks 2 and 3), and the user values are
> > explicitly ignored (hunk 1) if the symbols are not visible.
> >
> > It's not clear to me why hunk 1 exists. I'm sure it solve some problem, but
> > I'm not sure why we would ever want to discard user input values, and
> > causes a problem exactly as the comment describes.
> 
> This is intentional.  If a symbol is not visible, it's not meant to be changed
> by the user, as doing so may break things (at build or runtime).
> 
> E.g. running "make oldconfig" after a kernel upgrade may retain the old
> state of a variable, which is now invalid.  Ignoring invisible symbols
> avoids this.

Makes sense. Thanks!

Tycho

Re: [RFC PATCH 01/11] dt-bindings: update the binding for Allwinner H3 TVE support

[Icenowy Zheng]

于 2017年5月20日 GMT+08:00 上午2:02:15, Maxime Ripard 
 写到:
>On Thu, May 18, 2017 at 12:43:44AM +0800, Icenowy Zheng wrote:
>> -On SoCs other than the A33 and V3s, there is one more clock
>required:
>> +For the following compatibles:
>> +   * allwinner,sun5i-a13-tcon
>> +   * allwinner,sun6i-a31-tcon
>> +   * allwinner,sun6i-a31s-tcon
>> +   * allwinner,sun8i-a33-tcon
>> +   * allwinner,sun8i-v3s-tcon
>> +there is one more clock and one more property required:
>> + - clocks:
>> +   - 'tcon-ch0': The clock driving the TCON channel 0
>> + - clock-output-names: Name of the pixel clock created
>> +
>> +For the following compatibles:
>> +   * allwinner,sun5i-a13-tcon
>> +   * allwinner,sun6i-a31-tcon
>> +   * allwinner,sun6i-a31s-tcon
>> +   * allwinner,sun8i-h3-tcon0
>> +there is one more clock required:
>> - 'tcon-ch1': The clock driving the TCON channel 1
>
>Putting ID's in the compatible name is usually a bad idea. What is the
>difference between the two? Only that the second one doesn't have a
>clock?

Yes.

>
>That seems highly unlikely. How does it generate the pixel clock
>frequency?

Yes it seems impossible, but it's also the fact.

There's only one CLK_TCON in H3/5, which is for TCON0.

It's possible that lcd-ch1 clk is CLK_TVE, but it's still a weird situation --
Although we have a lcd-ch1 clock, we cannot touch it, otherwise
the TVE will refuse to work (the TVE can only work under 216MHz).

>
>Maxime

Re: [RFC PATCH 01/11] dt-bindings: update the binding for Allwinner H3 TVE support

[Icenowy Zheng]

于 2017年5月20日 GMT+08:00 上午2:02:15, Maxime Ripard 
 写到:
>On Thu, May 18, 2017 at 12:43:44AM +0800, Icenowy Zheng wrote:
>> -On SoCs other than the A33 and V3s, there is one more clock
>required:
>> +For the following compatibles:
>> +   * allwinner,sun5i-a13-tcon
>> +   * allwinner,sun6i-a31-tcon
>> +   * allwinner,sun6i-a31s-tcon
>> +   * allwinner,sun8i-a33-tcon
>> +   * allwinner,sun8i-v3s-tcon
>> +there is one more clock and one more property required:
>> + - clocks:
>> +   - 'tcon-ch0': The clock driving the TCON channel 0
>> + - clock-output-names: Name of the pixel clock created
>> +
>> +For the following compatibles:
>> +   * allwinner,sun5i-a13-tcon
>> +   * allwinner,sun6i-a31-tcon
>> +   * allwinner,sun6i-a31s-tcon
>> +   * allwinner,sun8i-h3-tcon0
>> +there is one more clock required:
>> - 'tcon-ch1': The clock driving the TCON channel 1
>
>Putting ID's in the compatible name is usually a bad idea. What is the
>difference between the two? Only that the second one doesn't have a
>clock?

Yes.

>
>That seems highly unlikely. How does it generate the pixel clock
>frequency?

Yes it seems impossible, but it's also the fact.

There's only one CLK_TCON in H3/5, which is for TCON0.

It's possible that lcd-ch1 clk is CLK_TVE, but it's still a weird situation --
Although we have a lcd-ch1 clock, we cannot touch it, otherwise
the TVE will refuse to work (the TVE can only work under 216MHz).

>
>Maxime

Re: [RFC PATCH 10/11] ARM: sun8i: h3: add display engine pipeline for TVE

[Maxime Ripard]

On Thu, May 18, 2017 at 12:43:53AM +0800, Icenowy Zheng wrote:
> As we have already the support for the TV encoder on Allwinner H3, add
> the display engine pipeline device tree nodes to its DTSI file.
> 
> The H5 pipeline has some differences and will be enabled later.
> 
> The currently-unused mixer0 and tcon0 are also needed, for the
> completement of the pipeline.
> 
> Signed-off-by: Icenowy Zheng 
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 189 
> 
>  1 file changed, 189 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index b36f9f423c39..20172ef92415 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -41,6 +41,8 @@
>   */
>  
>  #include "sunxi-h3-h5.dtsi"
> +#include 
> +#include 
>  
>  / {
>   cpus {
> @@ -72,6 +74,193 @@
>   };
>   };
>  
> + de: display-engine {
> + compatible = "allwinner,sun8i-h3-display-engine";
> + allwinner,pipelines = <>,
> +   <>;
> + status = "disabled";
> + };
> +
> + soc {
> + display_clocks: clock@100 {
> + compatible = "allwinner,sun8i-a83t-de2-clk";
> + reg = <0x0100 0x10>;
> + clocks = < CLK_BUS_DE>,
> +  < CLK_DE>;
> + clock-names = "bus",
> +   "mod";
> + resets = < RST_BUS_DE>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + assigned-clocks = < CLK_DE>;
> + assigned-clock-parents = < CLK_PLL_DE>;
> + assigned-clock-rates = <43200>;

This shouldn't be set in the DT, but evaluated at runtime when calling
clk_set_rate.

> + tve0: tv-encoder@1e0 {
> + compatible = "allwinner,sun8i-h3-tv-encoder";
> + reg = <0x01e0 0x1000>;
> + clocks = < CLK_BUS_TVE>, < CLK_TVE>;
> + clock-names = "bus", "mod";
> + resets = < RST_BUS_TVE>;
> + status = "disabled";
> +
> + assigned-clocks = < CLK_TVE>;
> + assigned-clock-parents = < CLK_PLL_DE>;

Same thing here. clk_set_rate should just do the right thing.

> + assigned-clock-rates = <21600>;

And why are you setting it in the driver and in the DT?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [RFC PATCH 10/11] ARM: sun8i: h3: add display engine pipeline for TVE

[Maxime Ripard]

On Thu, May 18, 2017 at 12:43:53AM +0800, Icenowy Zheng wrote:
> As we have already the support for the TV encoder on Allwinner H3, add
> the display engine pipeline device tree nodes to its DTSI file.
> 
> The H5 pipeline has some differences and will be enabled later.
> 
> The currently-unused mixer0 and tcon0 are also needed, for the
> completement of the pipeline.
> 
> Signed-off-by: Icenowy Zheng 
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 189 
> 
>  1 file changed, 189 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index b36f9f423c39..20172ef92415 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -41,6 +41,8 @@
>   */
>  
>  #include "sunxi-h3-h5.dtsi"
> +#include 
> +#include 
>  
>  / {
>   cpus {
> @@ -72,6 +74,193 @@
>   };
>   };
>  
> + de: display-engine {
> + compatible = "allwinner,sun8i-h3-display-engine";
> + allwinner,pipelines = <>,
> +   <>;
> + status = "disabled";
> + };
> +
> + soc {
> + display_clocks: clock@100 {
> + compatible = "allwinner,sun8i-a83t-de2-clk";
> + reg = <0x0100 0x10>;
> + clocks = < CLK_BUS_DE>,
> +  < CLK_DE>;
> + clock-names = "bus",
> +   "mod";
> + resets = < RST_BUS_DE>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + assigned-clocks = < CLK_DE>;
> + assigned-clock-parents = < CLK_PLL_DE>;
> + assigned-clock-rates = <43200>;

This shouldn't be set in the DT, but evaluated at runtime when calling
clk_set_rate.

> + tve0: tv-encoder@1e0 {
> + compatible = "allwinner,sun8i-h3-tv-encoder";
> + reg = <0x01e0 0x1000>;
> + clocks = < CLK_BUS_TVE>, < CLK_TVE>;
> + clock-names = "bus", "mod";
> + resets = < RST_BUS_TVE>;
> + status = "disabled";
> +
> + assigned-clocks = < CLK_TVE>;
> + assigned-clock-parents = < CLK_PLL_DE>;

Same thing here. clk_set_rate should just do the right thing.

> + assigned-clock-rates = <21600>;

And why are you setting it in the driver and in the DT?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [copyleft-next] Re: Kernel modules under new copyleft licence : (was Re: [PATCH v2] module.h: add copyleft-next >= 0.3.1 as GPL compatible)

[Luis R. Rodriguez]

On Fri, May 19, 2017 at 10:59 AM, Luis R. Rodriguez  wrote:
> On Fri, May 19, 2017 at 05:09:20PM +0200, Luis R. Rodriguez wrote:
>> On Fri, May 19, 2017 at 12:31:50PM +0100, Alan Cox wrote:
>> > > I really cannot see how you might have an attorney who wants ink on
>> > > 2A but not 1A.
>> > > I really cannot see how you might have an attorney who wants ink on
>> > > 2B but not 1B.
>> >
>> > Because their job is to protect their whomsoever they represent. They
>> > protect them drawing upon case law and providing rules based upon
>> > caselaw so that people don't have to keep bothering them.
>> >
>> > The lawyers have caselaw for "either a or b" licensing. They don't have
>> > caselaw for licence compatibility with your licence. Therefore it's a
>> > risk.
>>
>> Alright, this makes sense.
>>
>> As noted though there are a few "or" clauses, which upstream file
>> is a good template to use for copyleft-next ?
>
> There seems to be a few "or" clauses. For instance:
>
> a) you can pick either license [0]
> b) gpl on Linux, otherwise this other license below [1]
>
> To help uplift copyleft will go with b).
>
> [0] drivers/infiniband/hw/hfi1/driver.c
> [1] drivers/block/xen-blkback/blkback.c

So that yields:

 * This program is free software; you can redistribute it and/or modify it
 * under the terms of the GNU General Public License as published by the Free
 * Software Foundation; either version 2 of the License, or at your option) any
 * later version; or, when distributed separately from the Linux kernel or
 * incorporated into other software packages, subject to the following license:
 *
 * This program is free software; you can redistribute it and/or modify it
 * under the terms of copyleft-next (version 0.3.1 or later) as published
 * at http://copyleft-next.org/.

If there are issue please let me know.

  Luis

Re: [copyleft-next] Re: Kernel modules under new copyleft licence : (was Re: [PATCH v2] module.h: add copyleft-next >= 0.3.1 as GPL compatible)

[Luis R. Rodriguez]

On Fri, May 19, 2017 at 10:59 AM, Luis R. Rodriguez  wrote:
> On Fri, May 19, 2017 at 05:09:20PM +0200, Luis R. Rodriguez wrote:
>> On Fri, May 19, 2017 at 12:31:50PM +0100, Alan Cox wrote:
>> > > I really cannot see how you might have an attorney who wants ink on
>> > > 2A but not 1A.
>> > > I really cannot see how you might have an attorney who wants ink on
>> > > 2B but not 1B.
>> >
>> > Because their job is to protect their whomsoever they represent. They
>> > protect them drawing upon case law and providing rules based upon
>> > caselaw so that people don't have to keep bothering them.
>> >
>> > The lawyers have caselaw for "either a or b" licensing. They don't have
>> > caselaw for licence compatibility with your licence. Therefore it's a
>> > risk.
>>
>> Alright, this makes sense.
>>
>> As noted though there are a few "or" clauses, which upstream file
>> is a good template to use for copyleft-next ?
>
> There seems to be a few "or" clauses. For instance:
>
> a) you can pick either license [0]
> b) gpl on Linux, otherwise this other license below [1]
>
> To help uplift copyleft will go with b).
>
> [0] drivers/infiniband/hw/hfi1/driver.c
> [1] drivers/block/xen-blkback/blkback.c

So that yields:

 * This program is free software; you can redistribute it and/or modify it
 * under the terms of the GNU General Public License as published by the Free
 * Software Foundation; either version 2 of the License, or at your option) any
 * later version; or, when distributed separately from the Linux kernel or
 * incorporated into other software packages, subject to the following license:
 *
 * This program is free software; you can redistribute it and/or modify it
 * under the terms of copyleft-next (version 0.3.1 or later) as published
 * at http://copyleft-next.org/.

If there are issue please let me know.

  Luis