Re: [PATCH 4/4] cpu_cooling: Drop static-power related stuff

[Daniel Lezcano]

On 16/11/2017 03:47, Viresh Kumar wrote:
> On 15-11-17, 19:17, Rafael J. Wysocki wrote:
>> However, I would like to see a clear declaration from whoever is
>> maintaining that code today that there is a plan in place to upstream
>> it and that this plan will actually be acted on.  And, better yet,
>> *when* that can be expected to happen.
> 
> Exactly what I have been advocating.
> 
> And there is bunch of other places where such examples can be seen.
> For example multiple regulator support in the OPP framework, which I
> added an year ago hasn't seen a user yet. And I am pushing the TI guys
> (who wanted it badly) to upstream their code before we remove that as
> well :)
> 
> Again, someone has to come up and take responsibility of getting
> static power platform support upstream in a definite amount of time.

Instead of removing entirely the code, why not convert this to a DT
based info and put the Juno values back ?


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH 4/4] cpu_cooling: Drop static-power related stuff

[Daniel Lezcano]

On 16/11/2017 03:47, Viresh Kumar wrote:
> On 15-11-17, 19:17, Rafael J. Wysocki wrote:
>> However, I would like to see a clear declaration from whoever is
>> maintaining that code today that there is a plan in place to upstream
>> it and that this plan will actually be acted on.  And, better yet,
>> *when* that can be expected to happen.
> 
> Exactly what I have been advocating.
> 
> And there is bunch of other places where such examples can be seen.
> For example multiple regulator support in the OPP framework, which I
> added an year ago hasn't seen a user yet. And I am pushing the TI guys
> (who wanted it badly) to upstream their code before we remove that as
> well :)
> 
> Again, someone has to come up and take responsibility of getting
> static power platform support upstream in a definite amount of time.

Instead of removing entirely the code, why not convert this to a DT
based info and put the Juno values back ?


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH 4.4 06/56] dt-bindings: clockgen: Add compatible string for LS1012A

[Greg Kroah-Hartman]

On Fri, Nov 17, 2017 at 05:57:43AM +, Harninder Rai wrote:
> > 
> > No, not at all, if you want a patch in a specific stable tree, just email 
> > the git
> > commit id to sta...@vger.kernel.org and let me know what
> > tree(s) it should be applied to.
> 
> You can drop this patch. Thanks Greg

I think I already did :)

Re: [alsa-devel] [PATCH] ASoC: Intel: Add defaults for SND_SST_ options and machine drivers

[Takashi Iwai]

On Fri, 17 Nov 2017 00:01:06 +0100,
Pierre-Louis Bossart wrote:
> 
> 
> 
> On 11/16/2017 04:24 PM, Linus Torvalds wrote:
> > On Thu, Nov 16, 2017 at 2:16 PM, Pierre-Louis Bossart
> >  wrote:
> >> Add 'default m' when sensible
> > No.
> >
> > This is not sensible, and is not at all what I suggested.
> >
> > Actual new code should *never* be default 'm' or 'y'.
> >
> > You may think it's supremely important because you maintain it, but so
> > does EVERY other developer for their code, and no, we don'[t want to
> > just default everything on.
> >
> > So the only thing that should be on by default is stuff that either
> >
> >   (a) is a new option for something that used to not have an option at
> > all and was always enabled
> >
> >   IOW, this is a "it used to always be there, we default it on now
> > that we've made it optional".
> >
> >   (b) stuff that doesn't actually generate any code, but is there as a 
> > choice.
> >
> >   So this is stuff like "show me the config options for vendor XYZ".
> >
> >   (c) stuff that really is so common that it is the majority of users.
> >
> >   This is stuff like USB, or block devices, or "enable networking".
> >
> > But some individual driver for some hardware that nobody has? No.
> >
> > So things like this is pure garbage:
> >
> >   config SND_SST_ATOM_HIFI2_PLATFORM
> >  tristate "Intel ASoC SST driver for HiFi2 platforms
> > (*field, *trail)"
> >  default m
> >
> > because clearly "Intel ASoC SST driver for HiFi2 platforms" should not
> > at all default to being on for everybody.
> It's only on for people who select X86 and ASoC (which isn't on by
> default). With make defconfig, none of this is selected and I
> *thought* making those suboptions default to m would help distros who
> don't necessarily know the status of each driver, plus what was
> selected is known to be conflict-free. I guess I will follow the
> network example and state 'this is recommended'.
> 
> This is also not *new* code, what I tried here is to reverse the
> selection since the existing Kconfigs selected a target and inferred
> what SOC options are needed, and it needs to be the other way around
> to share machine drivers between SST and SOF drivers. Not to mention
> that there is no reason for machine driver configurations to be
> dependent on SOC-level configs like LPSS, this should be handled
> elsewhere.
> 
> I will move the TOPLEVEL config to follow what you suggested with the
> network example, this config does not have any impact on the
> code. will send a v2 tomorrow. If you want to send me your config i'll
> be happy to check the changes are ok. Thanks.

Well, think of creating a config from scratch on the tree with your
patch and user just selects the default choice.  What will happen?
Then ASoC Intel drivers appear there.  That should be avoided.

Also another problem is that the helper modules are selected by
CONFIG_SND_SOC_INTEL_TOPLEVEL.  With default on, these modules are
also enabled unconditionally.  That is, even if the board driver's
default is off, you'll still get these modules with the default
choice.

Lasts but not least, default=m looks definitely strange.  Distros set
CONFIG_SND=m, so it'll be module in anyway.  That is, you don't have
to care about modularity in that level at all.


A usual pattern is something like:

config SOME_TOPLEVEL
bool "some toplevel gateway config"
default y
help 

config BACKEND_A
tristate
select MORE_STUFF

config HELPER_DRIVERS
tristate
select ANOTHER_STUFF


if SOME_TOPLEVEL

config DRIVER_A
tristate "driver a"
depends on PLATFORM_A
select BACKEND_A
select HELPER_DRIVERS
help ...

config DRIVER_B
tristate "driver b"
depends on SOMETHING_ELSE
select BACKEND_B
select HELPER_DRIVERS
help ...



endif


Here we set the top-level config default=y, so that you'll go into the
board driver selection as default.  If you know that you don't need it
and are annoyed to deselect all, you can filter out at this point.
That's the *only* meaning of the toplevel config: just filtering.
(In the above it's bool, but it can be a tristate, too; but the
 purpose is still only for filtering.)

The rest drivers have no default, thus turned off.  So the default are
without any drivers.  And note that CONFIG_SOME_TOPLEVEL doesn't
select any others.  Thus it works nothing but a gatekeeper.


Takashi

Re: [PATCH 4.4 06/56] dt-bindings: clockgen: Add compatible string for LS1012A

[Greg Kroah-Hartman]

On Fri, Nov 17, 2017 at 05:57:43AM +, Harninder Rai wrote:
> > 
> > No, not at all, if you want a patch in a specific stable tree, just email 
> > the git
> > commit id to sta...@vger.kernel.org and let me know what
> > tree(s) it should be applied to.
> 
> You can drop this patch. Thanks Greg

I think I already did :)

Re: [alsa-devel] [PATCH] ASoC: Intel: Add defaults for SND_SST_ options and machine drivers

[Takashi Iwai]

On Fri, 17 Nov 2017 00:01:06 +0100,
Pierre-Louis Bossart wrote:
> 
> 
> 
> On 11/16/2017 04:24 PM, Linus Torvalds wrote:
> > On Thu, Nov 16, 2017 at 2:16 PM, Pierre-Louis Bossart
> >  wrote:
> >> Add 'default m' when sensible
> > No.
> >
> > This is not sensible, and is not at all what I suggested.
> >
> > Actual new code should *never* be default 'm' or 'y'.
> >
> > You may think it's supremely important because you maintain it, but so
> > does EVERY other developer for their code, and no, we don'[t want to
> > just default everything on.
> >
> > So the only thing that should be on by default is stuff that either
> >
> >   (a) is a new option for something that used to not have an option at
> > all and was always enabled
> >
> >   IOW, this is a "it used to always be there, we default it on now
> > that we've made it optional".
> >
> >   (b) stuff that doesn't actually generate any code, but is there as a 
> > choice.
> >
> >   So this is stuff like "show me the config options for vendor XYZ".
> >
> >   (c) stuff that really is so common that it is the majority of users.
> >
> >   This is stuff like USB, or block devices, or "enable networking".
> >
> > But some individual driver for some hardware that nobody has? No.
> >
> > So things like this is pure garbage:
> >
> >   config SND_SST_ATOM_HIFI2_PLATFORM
> >  tristate "Intel ASoC SST driver for HiFi2 platforms
> > (*field, *trail)"
> >  default m
> >
> > because clearly "Intel ASoC SST driver for HiFi2 platforms" should not
> > at all default to being on for everybody.
> It's only on for people who select X86 and ASoC (which isn't on by
> default). With make defconfig, none of this is selected and I
> *thought* making those suboptions default to m would help distros who
> don't necessarily know the status of each driver, plus what was
> selected is known to be conflict-free. I guess I will follow the
> network example and state 'this is recommended'.
> 
> This is also not *new* code, what I tried here is to reverse the
> selection since the existing Kconfigs selected a target and inferred
> what SOC options are needed, and it needs to be the other way around
> to share machine drivers between SST and SOF drivers. Not to mention
> that there is no reason for machine driver configurations to be
> dependent on SOC-level configs like LPSS, this should be handled
> elsewhere.
> 
> I will move the TOPLEVEL config to follow what you suggested with the
> network example, this config does not have any impact on the
> code. will send a v2 tomorrow. If you want to send me your config i'll
> be happy to check the changes are ok. Thanks.

Well, think of creating a config from scratch on the tree with your
patch and user just selects the default choice.  What will happen?
Then ASoC Intel drivers appear there.  That should be avoided.

Also another problem is that the helper modules are selected by
CONFIG_SND_SOC_INTEL_TOPLEVEL.  With default on, these modules are
also enabled unconditionally.  That is, even if the board driver's
default is off, you'll still get these modules with the default
choice.

Lasts but not least, default=m looks definitely strange.  Distros set
CONFIG_SND=m, so it'll be module in anyway.  That is, you don't have
to care about modularity in that level at all.


A usual pattern is something like:

config SOME_TOPLEVEL
bool "some toplevel gateway config"
default y
help 

config BACKEND_A
tristate
select MORE_STUFF

config HELPER_DRIVERS
tristate
select ANOTHER_STUFF


if SOME_TOPLEVEL

config DRIVER_A
tristate "driver a"
depends on PLATFORM_A
select BACKEND_A
select HELPER_DRIVERS
help ...

config DRIVER_B
tristate "driver b"
depends on SOMETHING_ELSE
select BACKEND_B
select HELPER_DRIVERS
help ...



endif


Here we set the top-level config default=y, so that you'll go into the
board driver selection as default.  If you know that you don't need it
and are annoyed to deselect all, you can filter out at this point.
That's the *only* meaning of the toplevel config: just filtering.
(In the above it's bool, but it can be a tristate, too; but the
 purpose is still only for filtering.)

The rest drivers have no default, thus turned off.  So the default are
without any drivers.  And note that CONFIG_SOME_TOPLEVEL doesn't
select any others.  Thus it works nothing but a gatekeeper.


Takashi

Re: [PATCH v1] mm: relax deferred struct page requirements

[Heiko Carstens]

On Thu, Nov 16, 2017 at 08:46:01PM -0500, Pavel Tatashin wrote:
> There is no need to have ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT,
> as all the page initialization code is in common code.
> 
> Also, there is no need to depend on MEMORY_HOTPLUG, as initialization code
> does not really use hotplug memory functionality. So, we can remove this
> requirement as well.
> 
> This patch allows to use deferred struct page initialization on all
> platforms with memblock allocator.
> 
> Tested on x86, arm64, and sparc. Also, verified that code compiles on
> PPC with CONFIG_MEMORY_HOTPLUG disabled.
> 
> Signed-off-by: Pavel Tatashin 
> ---
>  arch/powerpc/Kconfig | 1 -
>  arch/s390/Kconfig| 1 -
>  arch/x86/Kconfig | 1 -
>  mm/Kconfig   | 7 +--
>  4 files changed, 1 insertion(+), 9 deletions(-)

For s390 the s390 bit:

Acked-by: Heiko Carstens 

Re: [PATCH v1] mm: relax deferred struct page requirements

[Heiko Carstens]

On Thu, Nov 16, 2017 at 08:46:01PM -0500, Pavel Tatashin wrote:
> There is no need to have ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT,
> as all the page initialization code is in common code.
> 
> Also, there is no need to depend on MEMORY_HOTPLUG, as initialization code
> does not really use hotplug memory functionality. So, we can remove this
> requirement as well.
> 
> This patch allows to use deferred struct page initialization on all
> platforms with memblock allocator.
> 
> Tested on x86, arm64, and sparc. Also, verified that code compiles on
> PPC with CONFIG_MEMORY_HOTPLUG disabled.
> 
> Signed-off-by: Pavel Tatashin 
> ---
>  arch/powerpc/Kconfig | 1 -
>  arch/s390/Kconfig| 1 -
>  arch/x86/Kconfig | 1 -
>  mm/Kconfig   | 7 +--
>  4 files changed, 1 insertion(+), 9 deletions(-)

For s390 the s390 bit:

Acked-by: Heiko Carstens 

Re: [PATCH v7 06/13] slimbus: Add messaging APIs to slimbus framework

[Vinod Koul]

On Wed, Nov 15, 2017 at 02:10:36PM +, srinivas.kandaga...@linaro.org wrote:

> +void slim_msg_response(struct slim_controller *ctrl, u8 *reply, u8 tid, u8 
> len)
> +{
> + struct slim_msg_txn *txn;
> + struct slim_val_inf *msg;
> + unsigned long flags;
> +
> + spin_lock_irqsave(>txn_lock, flags);

Do you need this to be a _irqsave variant? What is the context of io
transfers in this case

> +/**
> + * slim_do_transfer() - Process a slimbus-messaging transaction
> + *
> + * @ctrl: Controller handle
> + * @txn: Transaction to be sent over SLIMbus
> + *
> + * Called by controller to transmit messaging transactions not dealing with
> + * Interface/Value elements. (e.g. transmittting a message to assign logical
> + * address to a slave device
> + *
> + * Return: -ETIMEDOUT: If transmission of this message timed out
> + *   (e.g. due to bus lines not being clocked or driven by controller)
> + *   -ENOTCONN: If the transmitted message was not ACKed by destination
> + *   device.

I am preferring ENODATA in SDW for this case, as Slaves didnt respond or
ACK.

ENOTCONN is defined as /* Transport endpoint is not connected */ which is
not the case here, connected yes but not responded.

> +static int slim_val_inf_sanity(struct slim_controller *ctrl,
> +struct slim_val_inf *msg, u8 mc)
> +{
> + if (!msg || msg->num_bytes > 16 ||
> + (msg->start_offset + msg->num_bytes) > 0xC00)
> + goto reterr;
> + switch (mc) {
> + case SLIM_MSG_MC_REQUEST_VALUE:
> + case SLIM_MSG_MC_REQUEST_INFORMATION:
> + if (msg->rbuf != NULL)
> + return 0;
> + break;

empty line here and after each break make it look better

> + case SLIM_MSG_MC_CHANGE_VALUE:
> + case SLIM_MSG_MC_CLEAR_INFORMATION:
> + if (msg->wbuf != NULL)
> + return 0;
> + break;
> + case SLIM_MSG_MC_REQUEST_CHANGE_VALUE:
> + case SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION:
> + if (msg->rbuf != NULL && msg->wbuf != NULL)
> + return 0;
> + break;
> + default:
> + break;

this seems superflous and we can just fall thru to error below.

> + }
> +reterr:
> + dev_err(ctrl->dev, "Sanity check failed:msg:offset:0x%x, mc:%d\n",
> + msg->start_offset, mc);
> + return -EINVAL;

...

> +static int slim_xfer_msg(struct slim_controller *ctrl,
> +  struct slim_device *sbdev,
> +  struct slim_val_inf *msg, u8 mc)
> +{
> + DEFINE_SLIM_LDEST_TXN(txn_stack, mc, 6, sbdev->laddr, msg);
> + struct slim_msg_txn *txn = _stack;
> + int ret;
> + u16 sl;
> +
> + ret = slim_val_inf_sanity(ctrl, msg, mc);
> + if (ret)
> + return ret;
> +
> + sl = slim_slicesize(msg->num_bytes);
> +
> + dev_dbg(ctrl->dev, "SB xfer msg:os:%x, len:%d, MC:%x, sl:%x\n",
> + msg->start_offset, msg->num_bytes, mc, sl);

better to add tracing support for these debug prints

> +
> + txn->ec = ((sl | (1 << 3)) | ((msg->start_offset & 0xFFF) << 4));
> +
> + switch (mc) {
> + case SLIM_MSG_MC_REQUEST_CHANGE_VALUE:
> + case SLIM_MSG_MC_CHANGE_VALUE:
> + case SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION:
> + case SLIM_MSG_MC_CLEAR_INFORMATION:
> + txn->rl += msg->num_bytes;
> + default:
> + break;
> + }
> +
> + if (slim_tid_txn(txn->mt, txn->mc))
> + txn->rl++;
> +
> + return slim_do_transfer(ctrl, txn);
> +}
> +
> +/**
> + * slim_request_val_element() - request value element
> + *
> + * @sb: client handle requesting elemental message reads, writes.
> + * @msg: Input structure for start-offset, number of bytes to read.
> + * context: can sleep
> + *
> + * Return: -EINVAL: Invalid parameters
> + *   -ETIMEDOUT: If transmission of this message timed out (e.g. due to
> + *bus lines not being clocked or driven by controller)
> + *   -ENOTCONN: If the transmitted message was not ACKed by the device.
> + */
> +int slim_request_val_element(struct slim_device *sb,
> + struct slim_val_inf *msg)
> +{
> + struct slim_controller *ctrl = sb->ctrl;
> +
> + if (!ctrl)
> + return -EINVAL;
> +
> + return slim_xfer_msg(ctrl, sb, msg, SLIM_MSG_MC_REQUEST_VALUE);
> +}
> +EXPORT_SYMBOL_GPL(slim_request_val_element);
> +
> +/**
> + * slim_request_inf_element() - Request a info element
> + *
> + * @sb: client handle requesting elemental message reads.
> + * @msg: Input structure for start-offset, number of bytes to read
> + *   wbuf will contain information element(s) bit masks to be cleared.
> + *   rbuf will return what the information element value was
> + */
> +int slim_request_inf_element(struct slim_device *sb,
> + struct slim_val_inf *msg)
> +{
> + struct slim_controller *ctrl = sb->ctrl;
> +
> + if (!ctrl)
> + return 

Re: [PATCH v7 06/13] slimbus: Add messaging APIs to slimbus framework

[Vinod Koul]

On Wed, Nov 15, 2017 at 02:10:36PM +, srinivas.kandaga...@linaro.org wrote:

> +void slim_msg_response(struct slim_controller *ctrl, u8 *reply, u8 tid, u8 
> len)
> +{
> + struct slim_msg_txn *txn;
> + struct slim_val_inf *msg;
> + unsigned long flags;
> +
> + spin_lock_irqsave(>txn_lock, flags);

Do you need this to be a _irqsave variant? What is the context of io
transfers in this case

> +/**
> + * slim_do_transfer() - Process a slimbus-messaging transaction
> + *
> + * @ctrl: Controller handle
> + * @txn: Transaction to be sent over SLIMbus
> + *
> + * Called by controller to transmit messaging transactions not dealing with
> + * Interface/Value elements. (e.g. transmittting a message to assign logical
> + * address to a slave device
> + *
> + * Return: -ETIMEDOUT: If transmission of this message timed out
> + *   (e.g. due to bus lines not being clocked or driven by controller)
> + *   -ENOTCONN: If the transmitted message was not ACKed by destination
> + *   device.

I am preferring ENODATA in SDW for this case, as Slaves didnt respond or
ACK.

ENOTCONN is defined as /* Transport endpoint is not connected */ which is
not the case here, connected yes but not responded.

> +static int slim_val_inf_sanity(struct slim_controller *ctrl,
> +struct slim_val_inf *msg, u8 mc)
> +{
> + if (!msg || msg->num_bytes > 16 ||
> + (msg->start_offset + msg->num_bytes) > 0xC00)
> + goto reterr;
> + switch (mc) {
> + case SLIM_MSG_MC_REQUEST_VALUE:
> + case SLIM_MSG_MC_REQUEST_INFORMATION:
> + if (msg->rbuf != NULL)
> + return 0;
> + break;

empty line here and after each break make it look better

> + case SLIM_MSG_MC_CHANGE_VALUE:
> + case SLIM_MSG_MC_CLEAR_INFORMATION:
> + if (msg->wbuf != NULL)
> + return 0;
> + break;
> + case SLIM_MSG_MC_REQUEST_CHANGE_VALUE:
> + case SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION:
> + if (msg->rbuf != NULL && msg->wbuf != NULL)
> + return 0;
> + break;
> + default:
> + break;

this seems superflous and we can just fall thru to error below.

> + }
> +reterr:
> + dev_err(ctrl->dev, "Sanity check failed:msg:offset:0x%x, mc:%d\n",
> + msg->start_offset, mc);
> + return -EINVAL;

...

> +static int slim_xfer_msg(struct slim_controller *ctrl,
> +  struct slim_device *sbdev,
> +  struct slim_val_inf *msg, u8 mc)
> +{
> + DEFINE_SLIM_LDEST_TXN(txn_stack, mc, 6, sbdev->laddr, msg);
> + struct slim_msg_txn *txn = _stack;
> + int ret;
> + u16 sl;
> +
> + ret = slim_val_inf_sanity(ctrl, msg, mc);
> + if (ret)
> + return ret;
> +
> + sl = slim_slicesize(msg->num_bytes);
> +
> + dev_dbg(ctrl->dev, "SB xfer msg:os:%x, len:%d, MC:%x, sl:%x\n",
> + msg->start_offset, msg->num_bytes, mc, sl);

better to add tracing support for these debug prints

> +
> + txn->ec = ((sl | (1 << 3)) | ((msg->start_offset & 0xFFF) << 4));
> +
> + switch (mc) {
> + case SLIM_MSG_MC_REQUEST_CHANGE_VALUE:
> + case SLIM_MSG_MC_CHANGE_VALUE:
> + case SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION:
> + case SLIM_MSG_MC_CLEAR_INFORMATION:
> + txn->rl += msg->num_bytes;
> + default:
> + break;
> + }
> +
> + if (slim_tid_txn(txn->mt, txn->mc))
> + txn->rl++;
> +
> + return slim_do_transfer(ctrl, txn);
> +}
> +
> +/**
> + * slim_request_val_element() - request value element
> + *
> + * @sb: client handle requesting elemental message reads, writes.
> + * @msg: Input structure for start-offset, number of bytes to read.
> + * context: can sleep
> + *
> + * Return: -EINVAL: Invalid parameters
> + *   -ETIMEDOUT: If transmission of this message timed out (e.g. due to
> + *bus lines not being clocked or driven by controller)
> + *   -ENOTCONN: If the transmitted message was not ACKed by the device.
> + */
> +int slim_request_val_element(struct slim_device *sb,
> + struct slim_val_inf *msg)
> +{
> + struct slim_controller *ctrl = sb->ctrl;
> +
> + if (!ctrl)
> + return -EINVAL;
> +
> + return slim_xfer_msg(ctrl, sb, msg, SLIM_MSG_MC_REQUEST_VALUE);
> +}
> +EXPORT_SYMBOL_GPL(slim_request_val_element);
> +
> +/**
> + * slim_request_inf_element() - Request a info element
> + *
> + * @sb: client handle requesting elemental message reads.
> + * @msg: Input structure for start-offset, number of bytes to read
> + *   wbuf will contain information element(s) bit masks to be cleared.
> + *   rbuf will return what the information element value was
> + */
> +int slim_request_inf_element(struct slim_device *sb,
> + struct slim_val_inf *msg)
> +{
> + struct slim_controller *ctrl = sb->ctrl;
> +
> + if (!ctrl)
> + return 

Re: 4.1 EOL

[Tuncer Ayaz]

On 11/16/17, Jani Nikula  wrote:
> On Wed, 15 Nov 2017, Tuncer Ayaz  wrote:
> > I don't follow why you think it's a different platform and how I
> > might have "more" definitely shown v4.1 to be good, but I'll trust
> > your judgement as a drm dev and not argue :).
>
> You apparently have Sandy Bridge, the referenced reports are about
> Broadwell and Skylake. Even if the symptoms you see are the same,
> the root causes might be wildly different, needing a different fix.

Thanks for taking time to explain and clear my confusion :).

I checked the comments of the other reporter with a Sandy Bridge
system, and they haven't provided a proper trace. Hence, you're
absolutely right.

> I've learned the hard way not to make assumptions without detailed
> information, which in this case I don't have. As in, I don't even
> know for sure if you have Sandy Bridge or not, although it's alluded
> to in your message.

I do (Sandy Bridge), sorry for not being clearer about that.

> From my point of view, you're shouting regression while giving us
> nothing to work with. You need to help us to help you.

Like I said, will do.

Re: 4.1 EOL

[Tuncer Ayaz]

On 11/16/17, Jani Nikula  wrote:
> On Wed, 15 Nov 2017, Tuncer Ayaz  wrote:
> > I don't follow why you think it's a different platform and how I
> > might have "more" definitely shown v4.1 to be good, but I'll trust
> > your judgement as a drm dev and not argue :).
>
> You apparently have Sandy Bridge, the referenced reports are about
> Broadwell and Skylake. Even if the symptoms you see are the same,
> the root causes might be wildly different, needing a different fix.

Thanks for taking time to explain and clear my confusion :).

I checked the comments of the other reporter with a Sandy Bridge
system, and they haven't provided a proper trace. Hence, you're
absolutely right.

> I've learned the hard way not to make assumptions without detailed
> information, which in this case I don't have. As in, I don't even
> know for sure if you have Sandy Bridge or not, although it's alluded
> to in your message.

I do (Sandy Bridge), sorry for not being clearer about that.

> From my point of view, you're shouting regression while giving us
> nothing to work with. You need to help us to help you.

Like I said, will do.

Re: [RFC v5 10/11] [media] vb2: add out-fence support to QBUF

[Alexandre Courbot]

On Thursday, November 16, 2017 2:10:56 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
an out_fence and send its fd to userspace on the fence_fd field as a
return arg for the QBUF call.

The fence is signaled on buffer_done(), when the job on the buffer is
finished.

With out-fences we do not allow drivers to requeue buffers through vb2,
instead we flag an error on the buffer, signals it fence with error and


s/it/its


return it to userspace.

v6
- get rid of the V4L2_EVENT_OUT_FENCE event. We always keep the
ordering in vb2 for queueing in the driver, so the event is not
necessary anymore and the out_fence_fd is sent back to userspace
on QBUF call return arg
- do not allow requeueing with out-fences, instead mark the buffer
with an error and wake up to userspace.
- send the out_fence_fd back to userspace on the fence_fd field

v5:
- delay fd_install to DQ_EVENT (Hans)
- if queue is fully ordered send OUT_FENCE event right away
(Brian)
- rename 'q->ordered' to 'q->ordered_in_driver'
- merge change to implement OUT_FENCE event here

v4:
- return the out_fence_fd in the BUF_QUEUED event(Hans)

v3: - add WARN_ON_ONCE(q->ordered) on requeueing (Hans)
- set the OUT_FENCE flag if there is a fence pending (Hans)
- call fd_install() after vb2_core_qbuf() (Hans)
- clean up fence if vb2_core_qbuf() fails (Hans)
- add list to store sync_file and fence for the next queued buffer

v2: check if the queue is ordered.

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/videobuf2-core.c | 42 
+---

 drivers/media/v4l2-core/videobuf2-v4l2.c | 21 +++-
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c

index 8b4f0e9bcb36..2eb5ffa8e028 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -354,6 +354,7 @@ static int __vb2_queue_alloc(struct 
vb2_queue *q, enum vb2_memory memory,

vb->planes[plane].length = plane_sizes[plane];
vb->planes[plane].min_length = plane_sizes[plane];
}
+   vb->out_fence_fd = -1;
q->bufs[vb->index] = vb;
 
 		/* Allocate video buffer memory for the MMAP type */
@@ -934,10 +935,26 @@ void vb2_buffer_done(struct vb2_buffer 
*vb, enum vb2_buffer_state state)

case VB2_BUF_STATE_QUEUED:
return;
case VB2_BUF_STATE_REQUEUEING:
-   if (q->start_streaming_called)
-   __enqueue_in_driver(vb);
-   return;
+
+   if (!vb->out_fence) {
+   if (q->start_streaming_called)
+   __enqueue_in_driver(vb);
+   return;
+   }
+
+   /* Do not allow requeuing with explicit synchronization,
+* report it as an error to userspace */
+   state = VB2_BUF_STATE_ERROR;
+
+   /* fall through */
default:
+   if (state == VB2_BUF_STATE_ERROR)
+   dma_fence_set_error(vb->out_fence, -EFAULT);
+   dma_fence_signal(vb->out_fence);
+   dma_fence_put(vb->out_fence);
+   vb->out_fence = NULL;
+   vb->out_fence_fd = -1;
+
/* Inform any processes that may be waiting for buffers */
wake_up(>done_wq);
break;
@@ -1325,12 +1342,18 @@ EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
 int vb2_setup_out_fence(struct vb2_queue *q, unsigned int index)
 {
struct vb2_buffer *vb;
+   u64 context;
 
 	vb = q->bufs[index];
 
 	vb->out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
 
-	vb->out_fence = vb2_fence_alloc(q->out_fence_context);

+   if (q->ordered_in_driver)
+   context = q->out_fence_context;
+   else
+   context = dma_fence_context_alloc(1);
+
+   vb->out_fence = vb2_fence_alloc(context);
if (!vb->out_fence) {
put_unused_fd(vb->out_fence_fd);
return -ENOMEM;
@@ -1594,6 +1617,9 @@ int vb2_core_qbuf(struct vb2_queue *q, 
unsigned int index, void *pb,

if (pb)
call_void_bufop(q, fill_user_buffer, vb, pb);
 
+	fd_install(vb->out_fence_fd, vb->sync_file->file);


What happens if the buffer does not have an output fence? Shouldn't this be 
protected by a check on whether the buffer has been queued with 
V4L2_BUF_FLAG_OUT_FENCE?


(... or maybe move this to vb2_setup_out_fence() after all, where we know 
for sure an output fence exists).



+   vb->sync_file = NULL;
+
dprintk(2, "qbuf of buffer %d succeeded\n", vb->index);
   

Re: [patch 1/7] Documentation: Add license-rules.rst to describe how to properly identify file licenses

[Philippe Ombredanne]

On Thu, Nov 16, 2017 at 4:15 PM, Jonas Oberg  wrote:
> Hi,
>
>> One other thing that occurred to me is that documentation files, too,
>> are copyrightable and should have license identifiers.
>
> Would it make sense to take an incremental approach to this? Get the
> source code and identifiers worked on by Thomas et al through first, then
> think about and fix up potential other issues, like the top level COPYING
> file, or documentation :-)

I could not agree more... code first!
FWIW I scanned the whole docs with scancode as part of this exercise.
They are rather ... messy license-wise, but even though I got through
it eventually they also generally less critical license-wise IMHO.

You can see some details of these scans in [1] though they are not
100% up to date: I did not post every intermediate scans and review
there as things are moving at a fast pace.

There are probably more pressing things to fix such as discrepancies
between a MODULE_LICENSE and the licensing of a file when they do not
match.

Here [2] the top level comment is a plain GPL-2.0 "only" while the
MODULE_LICENSE  is a GPL-2.0+ "or later"  (based on the plain "GPL"
definition in module.h [3] and this is just  one of many examples of this
weirdness.

Or fix the non-standard redefinition of the MODULE_LICENSE macro as
DRIVER_LICENSE as in [2]  and found elsewhere with
grep -r . -e "DRIVER_LICENSE"

These break the otherwise nicely grepable MODULE_LICENSE macros
with this kind of warty redirection I stumbled upon while reviewing kernel
license scans:

#define DRIVER_LICENSE "GPL"
[...]
MODULE_LICENSE(DRIVER_LICENSE);

[1] https://github.com/pombredanne/linux-kernel-scans/
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/alpha/kernel/srm_env.c?h=v4.14#n13
[3] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/module.h?h=v4.14#n174
[4] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/gma500/psb_drv.h?h=v4.14#n39
-- 
Cordially
Philippe Ombredanne

Re: [RFC v5 10/11] [media] vb2: add out-fence support to QBUF

[Alexandre Courbot]

On Thursday, November 16, 2017 2:10:56 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
an out_fence and send its fd to userspace on the fence_fd field as a
return arg for the QBUF call.

The fence is signaled on buffer_done(), when the job on the buffer is
finished.

With out-fences we do not allow drivers to requeue buffers through vb2,
instead we flag an error on the buffer, signals it fence with error and


s/it/its


return it to userspace.

v6
- get rid of the V4L2_EVENT_OUT_FENCE event. We always keep the
ordering in vb2 for queueing in the driver, so the event is not
necessary anymore and the out_fence_fd is sent back to userspace
on QBUF call return arg
- do not allow requeueing with out-fences, instead mark the buffer
with an error and wake up to userspace.
- send the out_fence_fd back to userspace on the fence_fd field

v5:
- delay fd_install to DQ_EVENT (Hans)
- if queue is fully ordered send OUT_FENCE event right away
(Brian)
- rename 'q->ordered' to 'q->ordered_in_driver'
- merge change to implement OUT_FENCE event here

v4:
- return the out_fence_fd in the BUF_QUEUED event(Hans)

v3: - add WARN_ON_ONCE(q->ordered) on requeueing (Hans)
- set the OUT_FENCE flag if there is a fence pending (Hans)
- call fd_install() after vb2_core_qbuf() (Hans)
- clean up fence if vb2_core_qbuf() fails (Hans)
- add list to store sync_file and fence for the next queued buffer

v2: check if the queue is ordered.

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/videobuf2-core.c | 42 
+---

 drivers/media/v4l2-core/videobuf2-v4l2.c | 21 +++-
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c

index 8b4f0e9bcb36..2eb5ffa8e028 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -354,6 +354,7 @@ static int __vb2_queue_alloc(struct 
vb2_queue *q, enum vb2_memory memory,

vb->planes[plane].length = plane_sizes[plane];
vb->planes[plane].min_length = plane_sizes[plane];
}
+   vb->out_fence_fd = -1;
q->bufs[vb->index] = vb;
 
 		/* Allocate video buffer memory for the MMAP type */
@@ -934,10 +935,26 @@ void vb2_buffer_done(struct vb2_buffer 
*vb, enum vb2_buffer_state state)

case VB2_BUF_STATE_QUEUED:
return;
case VB2_BUF_STATE_REQUEUEING:
-   if (q->start_streaming_called)
-   __enqueue_in_driver(vb);
-   return;
+
+   if (!vb->out_fence) {
+   if (q->start_streaming_called)
+   __enqueue_in_driver(vb);
+   return;
+   }
+
+   /* Do not allow requeuing with explicit synchronization,
+* report it as an error to userspace */
+   state = VB2_BUF_STATE_ERROR;
+
+   /* fall through */
default:
+   if (state == VB2_BUF_STATE_ERROR)
+   dma_fence_set_error(vb->out_fence, -EFAULT);
+   dma_fence_signal(vb->out_fence);
+   dma_fence_put(vb->out_fence);
+   vb->out_fence = NULL;
+   vb->out_fence_fd = -1;
+
/* Inform any processes that may be waiting for buffers */
wake_up(>done_wq);
break;
@@ -1325,12 +1342,18 @@ EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
 int vb2_setup_out_fence(struct vb2_queue *q, unsigned int index)
 {
struct vb2_buffer *vb;
+   u64 context;
 
 	vb = q->bufs[index];
 
 	vb->out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
 
-	vb->out_fence = vb2_fence_alloc(q->out_fence_context);

+   if (q->ordered_in_driver)
+   context = q->out_fence_context;
+   else
+   context = dma_fence_context_alloc(1);
+
+   vb->out_fence = vb2_fence_alloc(context);
if (!vb->out_fence) {
put_unused_fd(vb->out_fence_fd);
return -ENOMEM;
@@ -1594,6 +1617,9 @@ int vb2_core_qbuf(struct vb2_queue *q, 
unsigned int index, void *pb,

if (pb)
call_void_bufop(q, fill_user_buffer, vb, pb);
 
+	fd_install(vb->out_fence_fd, vb->sync_file->file);


What happens if the buffer does not have an output fence? Shouldn't this be 
protected by a check on whether the buffer has been queued with 
V4L2_BUF_FLAG_OUT_FENCE?


(... or maybe move this to vb2_setup_out_fence() after all, where we know 
for sure an output fence exists).



+   vb->sync_file = NULL;
+
dprintk(2, "qbuf of buffer %d succeeded\n", vb->index);
return 0;
 
@@ -1860,6 +1886,12 @@ static void 

Re: [patch 1/7] Documentation: Add license-rules.rst to describe how to properly identify file licenses

[Philippe Ombredanne]

On Thu, Nov 16, 2017 at 4:15 PM, Jonas Oberg  wrote:
> Hi,
>
>> One other thing that occurred to me is that documentation files, too,
>> are copyrightable and should have license identifiers.
>
> Would it make sense to take an incremental approach to this? Get the
> source code and identifiers worked on by Thomas et al through first, then
> think about and fix up potential other issues, like the top level COPYING
> file, or documentation :-)

I could not agree more... code first!
FWIW I scanned the whole docs with scancode as part of this exercise.
They are rather ... messy license-wise, but even though I got through
it eventually they also generally less critical license-wise IMHO.

You can see some details of these scans in [1] though they are not
100% up to date: I did not post every intermediate scans and review
there as things are moving at a fast pace.

There are probably more pressing things to fix such as discrepancies
between a MODULE_LICENSE and the licensing of a file when they do not
match.

Here [2] the top level comment is a plain GPL-2.0 "only" while the
MODULE_LICENSE  is a GPL-2.0+ "or later"  (based on the plain "GPL"
definition in module.h [3] and this is just  one of many examples of this
weirdness.

Or fix the non-standard redefinition of the MODULE_LICENSE macro as
DRIVER_LICENSE as in [2]  and found elsewhere with
grep -r . -e "DRIVER_LICENSE"

These break the otherwise nicely grepable MODULE_LICENSE macros
with this kind of warty redirection I stumbled upon while reviewing kernel
license scans:

#define DRIVER_LICENSE "GPL"
[...]
MODULE_LICENSE(DRIVER_LICENSE);

[1] https://github.com/pombredanne/linux-kernel-scans/
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/alpha/kernel/srm_env.c?h=v4.14#n13
[3] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/module.h?h=v4.14#n174
[4] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/gma500/psb_drv.h?h=v4.14#n39
-- 
Cordially
Philippe Ombredanne

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Christoffer Dall]

On Fri, Nov 17, 2017 at 07:18:45AM +, Liuwenliang (Abbott Liu) wrote:
> On 16/11/17  22:41 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
> >No, it doesn't. It cannot work, because Cortex-A9 predates the invention
> >of the 64bit accessor. I suspect that you are testing stuff in QEMU,
> >which is giving you a SW model that always supports LPAE. I suggest you
> >test this code on *real* HW, and not only on QEMU.
> 
> I am sorry. My test is fault. I only defined TTBR0 as __ACCESS_CP15_64,
> but I don't use the definition TTBR0 as __ACCESS_CP15_64. 
> 
> Now I use the definition TTBR0 as __ACCESS_CP15_64 on CPU supporting
> LPAE(vexpress_a9)

What does a "CPU supporting LPAE(vexpress_a9) mean?  As Marc pointed
out, a Cortex-A9 doesn't support LPAE.  If you configure your kernel
with LPAE it's not going to work on a Cortex-A9.

> I find it doesn't work and report undefined instruction error
> when execute "mrrc" instruction.
> 
> So, you are right that 64bit accessor of TTBR0 cannot work on LPAE.
> 

It's the other way around.  It doesn't work WITHOUT LPAE, it only works
WITH LPAE.

The ARM ARM explains this quite clearly:

"Accessing TTBR0

To access TTBR0 in an implementation that does not include the Large
Physical Address Extension, or bits[31:0] of TTBR0 in an implementation
that includes the Large Physical Address Extension, software reads or
writes the CP15 registers with  set to 0,  set to c2, 
set to c0, and  set to 0. For example:

MRC p15, 0, , c2, c0, 0
  ; Read 32-bit TTBR0 into Rt
MCR p15, 0, , c2, c0, 0
  ; Write Rt to 32-bit TTBR0

In an implementation that includes the Large Physical Address Extension,
to access all 64 bits of TTBR0, software performs a 64-bit read or write
of the CP15 registers with  set to c2 and  set to 0. For
example:

MRRC p15, 0, , , c2
  ; Read 64-bit TTBR0 into Rt (low word) and Rt2 (high word)
MCRR p15, 0, , , c2
  ; Write Rt (low word) and Rt2 (high word) to 64-bit TTBR0

In these MRRC and MCRR instructions, Rt holds the least-significant word
of TTBR0, and Rt2 holds the most-significant word."

That is, if your processor (like the Cortex-A9) does NOT support LPAE,
all you have is the 32-bit accessors (MRC and MCR).

If your processor does support LPAE (like a Cortex-A15 for example),
then you have both the 32-bit accessors (MRC and MCR) and the 64-bit
accessors (MRRC, MCRR), and using the 32-bit accessor will simply access
the lower 32-bits of the 64-bit register.

Hope this helps,
-Christoffer

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Christoffer Dall]

On Fri, Nov 17, 2017 at 07:18:45AM +, Liuwenliang (Abbott Liu) wrote:
> On 16/11/17  22:41 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
> >No, it doesn't. It cannot work, because Cortex-A9 predates the invention
> >of the 64bit accessor. I suspect that you are testing stuff in QEMU,
> >which is giving you a SW model that always supports LPAE. I suggest you
> >test this code on *real* HW, and not only on QEMU.
> 
> I am sorry. My test is fault. I only defined TTBR0 as __ACCESS_CP15_64,
> but I don't use the definition TTBR0 as __ACCESS_CP15_64. 
> 
> Now I use the definition TTBR0 as __ACCESS_CP15_64 on CPU supporting
> LPAE(vexpress_a9)

What does a "CPU supporting LPAE(vexpress_a9) mean?  As Marc pointed
out, a Cortex-A9 doesn't support LPAE.  If you configure your kernel
with LPAE it's not going to work on a Cortex-A9.

> I find it doesn't work and report undefined instruction error
> when execute "mrrc" instruction.
> 
> So, you are right that 64bit accessor of TTBR0 cannot work on LPAE.
> 

It's the other way around.  It doesn't work WITHOUT LPAE, it only works
WITH LPAE.

The ARM ARM explains this quite clearly:

"Accessing TTBR0

To access TTBR0 in an implementation that does not include the Large
Physical Address Extension, or bits[31:0] of TTBR0 in an implementation
that includes the Large Physical Address Extension, software reads or
writes the CP15 registers with  set to 0,  set to c2, 
set to c0, and  set to 0. For example:

MRC p15, 0, , c2, c0, 0
  ; Read 32-bit TTBR0 into Rt
MCR p15, 0, , c2, c0, 0
  ; Write Rt to 32-bit TTBR0

In an implementation that includes the Large Physical Address Extension,
to access all 64 bits of TTBR0, software performs a 64-bit read or write
of the CP15 registers with  set to c2 and  set to 0. For
example:

MRRC p15, 0, , , c2
  ; Read 64-bit TTBR0 into Rt (low word) and Rt2 (high word)
MCRR p15, 0, , , c2
  ; Write Rt (low word) and Rt2 (high word) to 64-bit TTBR0

In these MRRC and MCRR instructions, Rt holds the least-significant word
of TTBR0, and Rt2 holds the most-significant word."

That is, if your processor (like the Cortex-A9) does NOT support LPAE,
all you have is the 32-bit accessors (MRC and MCR).

If your processor does support LPAE (like a Cortex-A15 for example),
then you have both the 32-bit accessors (MRC and MCR) and the 64-bit
accessors (MRRC, MCRR), and using the 32-bit accessor will simply access
the lower 32-bits of the 64-bit register.

Hope this helps,
-Christoffer

Re: [RFC PATCH 1/2] mm: introduce MAP_FIXED_SAFE

[Florian Weimer]

On 11/16/2017 11:18 AM, Michal Hocko wrote:

+   if (flags & MAP_FIXED_SAFE) {
+   struct vm_area_struct *vma = find_vma(mm, addr);
+
+   if (vma && vma->vm_start <= addr)
+   return -ENOMEM;
+   }


Could you pick a different error code which cannot also be caused by a 
an unrelated, possibly temporary condition?  Maybe EBUSY or EEXIST?


This would definitely help with application-based randomization of 
mappings, and there, actual ENOMEM and this error would have to be 
handled differently.


Thanks,
Florian

Re: [RFC PATCH 1/2] mm: introduce MAP_FIXED_SAFE

[Florian Weimer]

On 11/16/2017 11:18 AM, Michal Hocko wrote:

+   if (flags & MAP_FIXED_SAFE) {
+   struct vm_area_struct *vma = find_vma(mm, addr);
+
+   if (vma && vma->vm_start <= addr)
+   return -ENOMEM;
+   }


Could you pick a different error code which cannot also be caused by a 
an unrelated, possibly temporary condition?  Maybe EBUSY or EEXIST?


This would definitely help with application-based randomization of 
mappings, and there, actual ENOMEM and this error would have to be 
handled differently.


Thanks,
Florian

Re: [RFC v5 09/11] [media] vb2: add infrastructure to support out-fences

[Alexandre Courbot]

On Friday, November 17, 2017 4:19:00 PM JST, Alexandre Courbot wrote:

On Thursday, November 16, 2017 2:10:55 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

Add vb2_setup_out_fence() and the needed members to struct vb2_buffer.

v3:
- Do not hold yet another ref to the out_fence (Brian Starkey)

v2: - change it to reflect fd_install at DQEVENT ...


out_fence_fd is allocated in this patch but not used anywhere 
for the moment.
For consistency, maybe move its allocation to the next patch, 
or move the call
to fd_install() here if that is possible? In both cases, the 
call to get_unused_fd() can be moved right before fd_install() 
so you don't need to

call put_unused_fd() in the error paths below.


Aha, just realized that fd_install() was called in qbuf() :) Other comments 
probably still hold though.




... same thing for sync_file too. Maybe this patch can just be merged into
the next one? The current patch just creates an incomplete 
version of vb2_setup_out_fence() for which no user exist yet.



+
+   vb->out_fence = vb2_fence_alloc(q->out_fence_context);
+   if (!vb->out_fence) {
+   put_unused_fd(vb->out_fence_fd);
+   return -ENOMEM; ...

Re: [RFC v5 09/11] [media] vb2: add infrastructure to support out-fences

[Alexandre Courbot]

On Friday, November 17, 2017 4:19:00 PM JST, Alexandre Courbot wrote:

On Thursday, November 16, 2017 2:10:55 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

Add vb2_setup_out_fence() and the needed members to struct vb2_buffer.

v3:
- Do not hold yet another ref to the out_fence (Brian Starkey)

v2: - change it to reflect fd_install at DQEVENT ...


out_fence_fd is allocated in this patch but not used anywhere 
for the moment.
For consistency, maybe move its allocation to the next patch, 
or move the call
to fd_install() here if that is possible? In both cases, the 
call to get_unused_fd() can be moved right before fd_install() 
so you don't need to

call put_unused_fd() in the error paths below.


Aha, just realized that fd_install() was called in qbuf() :) Other comments 
probably still hold though.




... same thing for sync_file too. Maybe this patch can just be merged into
the next one? The current patch just creates an incomplete 
version of vb2_setup_out_fence() for which no user exist yet.



+
+   vb->out_fence = vb2_fence_alloc(q->out_fence_context);
+   if (!vb->out_fence) {
+   put_unused_fd(vb->out_fence_fd);
+   return -ENOMEM; ...

Re: [PATCH] kbuild: define KBUILD_MODNAME even if multiple modules share objects

[Cao jin]

On 11/16/2017 03:46 PM, Masahiro Yamada wrote:
> Currently, KBUILD_MODNAME is defined only when $(modname) contains
> just one word.  If an object is shared between multiple modules,
> undefined KBUILD_MODNAME causes a build error.
> 
> A simple test case is as follows:
> 
>   obj-m += foo.o
>   obj-m += bar.o
>   foo-objs := foo-bar-common.o foo-main.o
>   bar-objs := foo-bar-common.o bar-main.o
> 
> In this case, we do not know what to define for KBUILD_MODNAME when
> compiling foo-bar-common.o ("foo" or "bar" ?), so one reasonable
> solution is let it fall back to $(basetarget) (= "foo-bar-common").
> 
> It would be better to avoid such a design where possible, but we
> already have such a case, for example,
> drivers/net/ethernet/cavium/liquidio/Makefile
> 
> I slightly refactored implementation; we can check $(word 2, $(modname))
> instead of $(filter 1,$(words $(modname))).
> 
> Signed-off-by: Masahiro Yamada 
> ---
> 
>  scripts/Makefile.lib | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 5fbc46d..9f9a7df 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -86,8 +86,7 @@ subdir-ym   := $(addprefix $(obj)/,$(subdir-ym))
>  #   differ in different configs.
>  name-fix = $(squote)$(quote)$(subst $(comma),_,$(subst 
> -,_,$1))$(quote)$(squote)
>  basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget))
> -modname_flags  = $(if $(filter 1,$(words $(modname))),\
> - -DKBUILD_MODNAME=$(call name-fix,$(modname)))
> +modname_flags  = -DKBUILD_MODNAME=$(call name-fix,$(if $(word 
> 2,$(modname)),$(basetarget),$(modname)))
>  
>  orig_c_flags   = $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) \
>   $(ccflags-y) $(CFLAGS_$(basetarget).o)
> 

I like this style of the refactor, because it is a bit more readable
according to human mind,  and it also behave the same as before in the
case drivers/net/ethernet/cavium/liquidio/Makefile: modname =
$(basetarget). So:

Tested-by: Cao jin 
Reviewed-by: Cao jin 

-- 
Sincerely,
Cao jin

Re: [PATCH] kbuild: define KBUILD_MODNAME even if multiple modules share objects

[Cao jin]

On 11/16/2017 03:46 PM, Masahiro Yamada wrote:
> Currently, KBUILD_MODNAME is defined only when $(modname) contains
> just one word.  If an object is shared between multiple modules,
> undefined KBUILD_MODNAME causes a build error.
> 
> A simple test case is as follows:
> 
>   obj-m += foo.o
>   obj-m += bar.o
>   foo-objs := foo-bar-common.o foo-main.o
>   bar-objs := foo-bar-common.o bar-main.o
> 
> In this case, we do not know what to define for KBUILD_MODNAME when
> compiling foo-bar-common.o ("foo" or "bar" ?), so one reasonable
> solution is let it fall back to $(basetarget) (= "foo-bar-common").
> 
> It would be better to avoid such a design where possible, but we
> already have such a case, for example,
> drivers/net/ethernet/cavium/liquidio/Makefile
> 
> I slightly refactored implementation; we can check $(word 2, $(modname))
> instead of $(filter 1,$(words $(modname))).
> 
> Signed-off-by: Masahiro Yamada 
> ---
> 
>  scripts/Makefile.lib | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 5fbc46d..9f9a7df 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -86,8 +86,7 @@ subdir-ym   := $(addprefix $(obj)/,$(subdir-ym))
>  #   differ in different configs.
>  name-fix = $(squote)$(quote)$(subst $(comma),_,$(subst 
> -,_,$1))$(quote)$(squote)
>  basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget))
> -modname_flags  = $(if $(filter 1,$(words $(modname))),\
> - -DKBUILD_MODNAME=$(call name-fix,$(modname)))
> +modname_flags  = -DKBUILD_MODNAME=$(call name-fix,$(if $(word 
> 2,$(modname)),$(basetarget),$(modname)))
>  
>  orig_c_flags   = $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) \
>   $(ccflags-y) $(CFLAGS_$(basetarget).o)
> 

I like this style of the refactor, because it is a bit more readable
according to human mind,  and it also behave the same as before in the
case drivers/net/ethernet/cavium/liquidio/Makefile: modname =
$(basetarget). So:

Tested-by: Cao jin 
Reviewed-by: Cao jin 

-- 
Sincerely,
Cao jin

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 16/11/17  22:41 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
>No, it doesn't. It cannot work, because Cortex-A9 predates the invention
>of the 64bit accessor. I suspect that you are testing stuff in QEMU,
>which is giving you a SW model that always supports LPAE. I suggest you
>test this code on *real* HW, and not only on QEMU.

I am sorry. My test is fault. I only defined TTBR0 as __ACCESS_CP15_64,
but I don't use the definition TTBR0 as __ACCESS_CP15_64. 

Now I use the definition TTBR0 as __ACCESS_CP15_64 on CPU supporting
LPAE(vexpress_a9), I find it doesn't work and report undefined instruction error
when execute "mrrc" instruction.

So, you are right that 64bit accessor of TTBR0 cannot work on LPAE.

Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory

[Liuwenliang (Abbott Liu)]

On 16/11/17  22:41 Marc Zyngier [mailto:marc.zyng...@arm.com] wrote:
>No, it doesn't. It cannot work, because Cortex-A9 predates the invention
>of the 64bit accessor. I suspect that you are testing stuff in QEMU,
>which is giving you a SW model that always supports LPAE. I suggest you
>test this code on *real* HW, and not only on QEMU.

I am sorry. My test is fault. I only defined TTBR0 as __ACCESS_CP15_64,
but I don't use the definition TTBR0 as __ACCESS_CP15_64. 

Now I use the definition TTBR0 as __ACCESS_CP15_64 on CPU supporting
LPAE(vexpress_a9), I find it doesn't work and report undefined instruction error
when execute "mrrc" instruction.

So, you are right that 64bit accessor of TTBR0 cannot work on LPAE.

Re: [RFC v5 09/11] [media] vb2: add infrastructure to support out-fences

[Alexandre Courbot]

On Thursday, November 16, 2017 2:10:55 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

Add vb2_setup_out_fence() and the needed members to struct vb2_buffer.

v3:
- Do not hold yet another ref to the out_fence (Brian Starkey)

v2: - change it to reflect fd_install at DQEVENT
- add fence context for out-fences

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/videobuf2-core.c | 28 
 include/media/videobuf2-core.h   | 20 
 2 files changed, 48 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c

index 26de4c80717d..8b4f0e9bcb36 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -24,8 +24,10 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 

+#include 
 #include 
 
 #include 
@@ -1320,6 +1322,32 @@ int vb2_core_prepare_buf(struct 
vb2_queue *q, unsigned int index, void *pb)

 }
 EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
 
+int vb2_setup_out_fence(struct vb2_queue *q, unsigned int index)

+{
+   struct vb2_buffer *vb;
+
+   vb = q->bufs[index];
+
+   vb->out_fence_fd = get_unused_fd_flags(O_CLOEXEC);


out_fence_fd is allocated in this patch but not used anywhere for the 
moment.
For consistency, maybe move its allocation to the next patch, or move the 
call
to fd_install() here if that is possible? In both cases, the call to 
get_unused_fd() can be moved right before fd_install() so you don't need to

call put_unused_fd() in the error paths below.

... same thing for sync_file too. Maybe this patch can just be merged into
the next one? The current patch just creates an incomplete version of 
vb2_setup_out_fence() for which no user exist yet.



+
+   vb->out_fence = vb2_fence_alloc(q->out_fence_context);
+   if (!vb->out_fence) {
+   put_unused_fd(vb->out_fence_fd);
+   return -ENOMEM;
+   }
+
+   vb->sync_file = sync_file_create(vb->out_fence);
+   if (!vb->sync_file) {
+   put_unused_fd(vb->out_fence_fd);
+   dma_fence_put(vb->out_fence);
+   vb->out_fence = NULL;
+   return -ENOMEM;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(vb2_setup_out_fence);
+
 /**
  * vb2_start_streaming() - Attempt to start streaming.
  * @q: videobuf2 queue
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 5f48c7be7770..a9b0697bd782 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -259,6 +259,10 @@ struct vb2_buffer {
 *  using the buffer (queueing to the driver)
 * fence_cb:fence callback information
 * fence_cb_lock:   protect callback signal/remove
+* out_fence_fd:the out_fence_fd to be shared with userspace.
+* out_fence:   the out-fence associated with the buffer once
+*  it is queued to the driver.
+* sync_file:   the sync file to wrap the out fence
 */
enum vb2_buffer_state   state;
 
@@ -269,6 +273,10 @@ struct vb2_buffer {

struct dma_fence_cb fence_cb;
spinlock_t  fence_cb_lock;
 
+	int			out_fence_fd;

+   struct dma_fence*out_fence;
+   struct sync_file*sync_file;
+
 #ifdef CONFIG_VIDEO_ADV_DEBUG
/*
 * Counters for how often these buffer-related ops are
@@ -514,6 +522,7 @@ struct vb2_buf_ops {
  * @last_buffer_dequeued: used in poll() and DQBUF to 
immediately return if the

  * last decoded buffer was already dequeued. Set for capture queues
  * when a buffer with the V4L2_BUF_FLAG_LAST is dequeued.
+ * @out_fence_context: the fence context for the out fences
  * @last_fence:last in-fence received. Used to keep ordering.
  * @fileio:file io emulator internal data, used only if emulator is active
  * @threadio:  thread io internal data, used only if thread is active
@@ -569,6 +578,7 @@ struct vb2_queue {
unsigned intcopy_timestamp:1;
unsigned intlast_buffer_dequeued:1;
 
+	u64out_fence_context;

struct dma_fence*last_fence;
 
 	struct vb2_fileio_data		*fileio;
@@ -740,6 +750,16 @@ int vb2_core_create_bufs(struct vb2_queue 
*q, enum vb2_memory memory,
 int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int 
index, void *pb);
 
 /**

+ * vb2_setup_out_fence() - setup new out-fence
+ * @q: The vb2_queue where to setup it
+ * @index: index of the buffer
+ *
+ * Setup the file descriptor, the fence and the sync_file for the next
+ * buffer to be queued and add everything to the tail of the 
q->out_fence_list.

+ */
+int vb2_setup_out_fence(struct vb2_queue *q, unsigned int index);
+
+/**
  * vb2_core_qbuf() - 

Re: [RFC v5 09/11] [media] vb2: add infrastructure to support out-fences

[Alexandre Courbot]

On Thursday, November 16, 2017 2:10:55 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

Add vb2_setup_out_fence() and the needed members to struct vb2_buffer.

v3:
- Do not hold yet another ref to the out_fence (Brian Starkey)

v2: - change it to reflect fd_install at DQEVENT
- add fence context for out-fences

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/videobuf2-core.c | 28 
 include/media/videobuf2-core.h   | 20 
 2 files changed, 48 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c

index 26de4c80717d..8b4f0e9bcb36 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -24,8 +24,10 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 

+#include 
 #include 
 
 #include 
@@ -1320,6 +1322,32 @@ int vb2_core_prepare_buf(struct 
vb2_queue *q, unsigned int index, void *pb)

 }
 EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
 
+int vb2_setup_out_fence(struct vb2_queue *q, unsigned int index)

+{
+   struct vb2_buffer *vb;
+
+   vb = q->bufs[index];
+
+   vb->out_fence_fd = get_unused_fd_flags(O_CLOEXEC);


out_fence_fd is allocated in this patch but not used anywhere for the 
moment.
For consistency, maybe move its allocation to the next patch, or move the 
call
to fd_install() here if that is possible? In both cases, the call to 
get_unused_fd() can be moved right before fd_install() so you don't need to

call put_unused_fd() in the error paths below.

... same thing for sync_file too. Maybe this patch can just be merged into
the next one? The current patch just creates an incomplete version of 
vb2_setup_out_fence() for which no user exist yet.



+
+   vb->out_fence = vb2_fence_alloc(q->out_fence_context);
+   if (!vb->out_fence) {
+   put_unused_fd(vb->out_fence_fd);
+   return -ENOMEM;
+   }
+
+   vb->sync_file = sync_file_create(vb->out_fence);
+   if (!vb->sync_file) {
+   put_unused_fd(vb->out_fence_fd);
+   dma_fence_put(vb->out_fence);
+   vb->out_fence = NULL;
+   return -ENOMEM;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(vb2_setup_out_fence);
+
 /**
  * vb2_start_streaming() - Attempt to start streaming.
  * @q: videobuf2 queue
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 5f48c7be7770..a9b0697bd782 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -259,6 +259,10 @@ struct vb2_buffer {
 *  using the buffer (queueing to the driver)
 * fence_cb:fence callback information
 * fence_cb_lock:   protect callback signal/remove
+* out_fence_fd:the out_fence_fd to be shared with userspace.
+* out_fence:   the out-fence associated with the buffer once
+*  it is queued to the driver.
+* sync_file:   the sync file to wrap the out fence
 */
enum vb2_buffer_state   state;
 
@@ -269,6 +273,10 @@ struct vb2_buffer {

struct dma_fence_cb fence_cb;
spinlock_t  fence_cb_lock;
 
+	int			out_fence_fd;

+   struct dma_fence*out_fence;
+   struct sync_file*sync_file;
+
 #ifdef CONFIG_VIDEO_ADV_DEBUG
/*
 * Counters for how often these buffer-related ops are
@@ -514,6 +522,7 @@ struct vb2_buf_ops {
  * @last_buffer_dequeued: used in poll() and DQBUF to 
immediately return if the

  * last decoded buffer was already dequeued. Set for capture queues
  * when a buffer with the V4L2_BUF_FLAG_LAST is dequeued.
+ * @out_fence_context: the fence context for the out fences
  * @last_fence:last in-fence received. Used to keep ordering.
  * @fileio:file io emulator internal data, used only if emulator is active
  * @threadio:  thread io internal data, used only if thread is active
@@ -569,6 +578,7 @@ struct vb2_queue {
unsigned intcopy_timestamp:1;
unsigned intlast_buffer_dequeued:1;
 
+	u64out_fence_context;

struct dma_fence*last_fence;
 
 	struct vb2_fileio_data		*fileio;
@@ -740,6 +750,16 @@ int vb2_core_create_bufs(struct vb2_queue 
*q, enum vb2_memory memory,
 int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int 
index, void *pb);
 
 /**

+ * vb2_setup_out_fence() - setup new out-fence
+ * @q: The vb2_queue where to setup it
+ * @index: index of the buffer
+ *
+ * Setup the file descriptor, the fence and the sync_file for the next
+ * buffer to be queued and add everything to the tail of the 
q->out_fence_list.

+ */
+int vb2_setup_out_fence(struct vb2_queue *q, unsigned int index);
+
+/**
  * vb2_core_qbuf() - Queue a buffer from userspace
  *
  * @q: videobuf2 

RE: [PATCH 12/16] nsproxy: convert nsproxy.count to refcount_t

[Reshetova, Elena]

> The middle of the merge window is the wrong time to send patches as
> maintaner attention is going to making certain the merge goes smoothly
> and nothing is missed.

Sorry Eric, please feel free to ignore the patch until you have time. 
It is very difficult to figure out the correct time, since it varies vary much
by the maintainer when they want patches, so I am sending them to be
"in the mailbox" until people believe it is the right time. 

Best Regards,
Elena.


> 
> Eric
> 
> 
> 
> Elena Reshetova  writes:
> 
> > atomic_t variables are currently used to implement reference
> > counters with the following properties:
> >  - counter is initialized to 1 using atomic_set()
> >  - a resource is freed upon counter reaching zero
> >  - once counter reaches zero, its further
> >increments aren't allowed
> >  - counter schema uses basic atomic operations
> >(set, inc, inc_not_zero, dec_and_test, etc.)
> >
> > Such atomic variables should be converted to a newly provided
> > refcount_t type and API that prevents accidental counter overflows
> > and underflows. This is important since overflows and underflows
> > can lead to use-after-free situation and be exploitable.
> >
> > The variable nsproxy.count is used as pure reference counter.
> > Convert it to refcount_t and fix up the operations.
> >
> > **Important note for maintainers:
> >
> > Some functions from refcount_t API defined in lib/refcount.c
> > have different memory ordering guarantees than their atomic
> > counterparts.
> > The full comparison can be seen in
> > https://lkml.org/lkml/2017/11/15/57 and it is hopefully soon
> > in state to be merged to the documentation tree.
> > Normally the differences should not matter since refcount_t provides
> > enough guarantees to satisfy the refcounting use cases, but in
> > some rare cases it might matter.
> > Please double check that you don't have some undocumented
> > memory guarantees for this variable usage.
> >
> > For the nsproxy.count it might make a difference
> > in following places:
> >  - put_nsproxy() and switch_task_namespaces(): decrement in
> >refcount_dec_and_test() only provides RELEASE ordering
> >and control dependency on success vs. fully ordered
> >atomic counterpart
> >
> > Suggested-by: Kees Cook 
> > Reviewed-by: David Windsor 
> > Reviewed-by: Hans Liljestrand 
> > Signed-off-by: Elena Reshetova 
> > ---
> >  include/linux/nsproxy.h | 6 +++---
> >  kernel/nsproxy.c| 6 +++---
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
> > index 2ae1b1a..90f09ff 100644
> > --- a/include/linux/nsproxy.h
> > +++ b/include/linux/nsproxy.h
> > @@ -29,7 +29,7 @@ struct fs_struct;
> >   * nsproxy is copied.
> >   */
> >  struct nsproxy {
> > -   atomic_t count;
> > +   refcount_t count;
> > struct uts_namespace *uts_ns;
> > struct ipc_namespace *ipc_ns;
> > struct mnt_namespace *mnt_ns;
> > @@ -75,14 +75,14 @@ int __init nsproxy_cache_init(void);
> >
> >  static inline void put_nsproxy(struct nsproxy *ns)
> >  {
> > -   if (atomic_dec_and_test(>count)) {
> > +   if (refcount_dec_and_test(>count)) {
> > free_nsproxy(ns);
> > }
> >  }
> >
> >  static inline void get_nsproxy(struct nsproxy *ns)
> >  {
> > -   atomic_inc(>count);
> > +   refcount_inc(>count);
> >  }
> >
> >  #endif
> > diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
> > index f6c5d33..5bfe691 100644
> > --- a/kernel/nsproxy.c
> > +++ b/kernel/nsproxy.c
> > @@ -31,7 +31,7 @@
> >  static struct kmem_cache *nsproxy_cachep;
> >
> >  struct nsproxy init_nsproxy = {
> > -   .count  = ATOMIC_INIT(1),
> > +   .count  = REFCOUNT_INIT(1),
> > .uts_ns = _uts_ns,
> >  #if defined(CONFIG_POSIX_MQUEUE) || defined(CONFIG_SYSVIPC)
> > .ipc_ns = _ipc_ns,
> > @@ -52,7 +52,7 @@ static inline struct nsproxy *create_nsproxy(void)
> >
> > nsproxy = kmem_cache_alloc(nsproxy_cachep, GFP_KERNEL);
> > if (nsproxy)
> > -   atomic_set(>count, 1);
> > +   refcount_set(>count, 1);
> > return nsproxy;
> >  }
> >
> > @@ -225,7 +225,7 @@ void switch_task_namespaces(struct task_struct *p, 
> > struct
> nsproxy *new)
> > p->nsproxy = new;
> > task_unlock(p);
> >
> > -   if (ns && atomic_dec_and_test(>count))
> > +   if (ns && refcount_dec_and_test(>count))
> > free_nsproxy(ns);
> >  }

RE: [PATCH 12/16] nsproxy: convert nsproxy.count to refcount_t

[Reshetova, Elena]

> The middle of the merge window is the wrong time to send patches as
> maintaner attention is going to making certain the merge goes smoothly
> and nothing is missed.

Sorry Eric, please feel free to ignore the patch until you have time. 
It is very difficult to figure out the correct time, since it varies vary much
by the maintainer when they want patches, so I am sending them to be
"in the mailbox" until people believe it is the right time. 

Best Regards,
Elena.


> 
> Eric
> 
> 
> 
> Elena Reshetova  writes:
> 
> > atomic_t variables are currently used to implement reference
> > counters with the following properties:
> >  - counter is initialized to 1 using atomic_set()
> >  - a resource is freed upon counter reaching zero
> >  - once counter reaches zero, its further
> >increments aren't allowed
> >  - counter schema uses basic atomic operations
> >(set, inc, inc_not_zero, dec_and_test, etc.)
> >
> > Such atomic variables should be converted to a newly provided
> > refcount_t type and API that prevents accidental counter overflows
> > and underflows. This is important since overflows and underflows
> > can lead to use-after-free situation and be exploitable.
> >
> > The variable nsproxy.count is used as pure reference counter.
> > Convert it to refcount_t and fix up the operations.
> >
> > **Important note for maintainers:
> >
> > Some functions from refcount_t API defined in lib/refcount.c
> > have different memory ordering guarantees than their atomic
> > counterparts.
> > The full comparison can be seen in
> > https://lkml.org/lkml/2017/11/15/57 and it is hopefully soon
> > in state to be merged to the documentation tree.
> > Normally the differences should not matter since refcount_t provides
> > enough guarantees to satisfy the refcounting use cases, but in
> > some rare cases it might matter.
> > Please double check that you don't have some undocumented
> > memory guarantees for this variable usage.
> >
> > For the nsproxy.count it might make a difference
> > in following places:
> >  - put_nsproxy() and switch_task_namespaces(): decrement in
> >refcount_dec_and_test() only provides RELEASE ordering
> >and control dependency on success vs. fully ordered
> >atomic counterpart
> >
> > Suggested-by: Kees Cook 
> > Reviewed-by: David Windsor 
> > Reviewed-by: Hans Liljestrand 
> > Signed-off-by: Elena Reshetova 
> > ---
> >  include/linux/nsproxy.h | 6 +++---
> >  kernel/nsproxy.c| 6 +++---
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
> > index 2ae1b1a..90f09ff 100644
> > --- a/include/linux/nsproxy.h
> > +++ b/include/linux/nsproxy.h
> > @@ -29,7 +29,7 @@ struct fs_struct;
> >   * nsproxy is copied.
> >   */
> >  struct nsproxy {
> > -   atomic_t count;
> > +   refcount_t count;
> > struct uts_namespace *uts_ns;
> > struct ipc_namespace *ipc_ns;
> > struct mnt_namespace *mnt_ns;
> > @@ -75,14 +75,14 @@ int __init nsproxy_cache_init(void);
> >
> >  static inline void put_nsproxy(struct nsproxy *ns)
> >  {
> > -   if (atomic_dec_and_test(>count)) {
> > +   if (refcount_dec_and_test(>count)) {
> > free_nsproxy(ns);
> > }
> >  }
> >
> >  static inline void get_nsproxy(struct nsproxy *ns)
> >  {
> > -   atomic_inc(>count);
> > +   refcount_inc(>count);
> >  }
> >
> >  #endif
> > diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
> > index f6c5d33..5bfe691 100644
> > --- a/kernel/nsproxy.c
> > +++ b/kernel/nsproxy.c
> > @@ -31,7 +31,7 @@
> >  static struct kmem_cache *nsproxy_cachep;
> >
> >  struct nsproxy init_nsproxy = {
> > -   .count  = ATOMIC_INIT(1),
> > +   .count  = REFCOUNT_INIT(1),
> > .uts_ns = _uts_ns,
> >  #if defined(CONFIG_POSIX_MQUEUE) || defined(CONFIG_SYSVIPC)
> > .ipc_ns = _ipc_ns,
> > @@ -52,7 +52,7 @@ static inline struct nsproxy *create_nsproxy(void)
> >
> > nsproxy = kmem_cache_alloc(nsproxy_cachep, GFP_KERNEL);
> > if (nsproxy)
> > -   atomic_set(>count, 1);
> > +   refcount_set(>count, 1);
> > return nsproxy;
> >  }
> >
> > @@ -225,7 +225,7 @@ void switch_task_namespaces(struct task_struct *p, 
> > struct
> nsproxy *new)
> > p->nsproxy = new;
> > task_unlock(p);
> >
> > -   if (ns && atomic_dec_and_test(>count))
> > +   if (ns && refcount_dec_and_test(>count))
> > free_nsproxy(ns);
> >  }

[PATCH] kmemcheck: add scheduling point to kmemleak_scan

[Yisheng Xie]

kmemleak_scan will scan struct page for each node and it can be really
large and resulting in a soft lockup. We have seen a soft lockup when do
scan while compile kernel:

 [  220.561051] watchdog: BUG: soft lockup - CPU#53 stuck for 22s! [bash:10287]
 [...]
 [  220.753837] Call Trace:
 [  220.756296]  kmemleak_scan+0x21a/0x4c0
 [  220.760034]  kmemleak_write+0x312/0x350
 [  220.763866]  ? do_wp_page+0x147/0x4c0
 [  220.767521]  full_proxy_write+0x5a/0xa0
 [  220.771351]  __vfs_write+0x33/0x150
 [  220.774833]  ? __inode_security_revalidate+0x4c/0x60
 [  220.779782]  ? selinux_file_permission+0xda/0x130
 [  220.784479]  ? _cond_resched+0x15/0x30
 [  220.788221]  vfs_write+0xad/0x1a0
 [  220.791529]  SyS_write+0x52/0xc0
 [  220.794758]  do_syscall_64+0x61/0x1a0
 [  220.798411]  entry_SYSCALL64_slow_path+0x25/0x25

Fix this by adding cond_resched every 1024 pages.

Signed-off-by: Yisheng Xie 
---
 mm/kmemleak.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index e4738d5..e9f2e86 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -1523,6 +1523,8 @@ static void kmemleak_scan(void)
if (page_count(page) == 0)
continue;
scan_block(page, page + 1, NULL);
+   if (!(pfn % 1024))
+   cond_resched();
}
}
put_online_mems();
-- 
1.7.12.4

[PATCH] kmemcheck: add scheduling point to kmemleak_scan

[Yisheng Xie]

kmemleak_scan will scan struct page for each node and it can be really
large and resulting in a soft lockup. We have seen a soft lockup when do
scan while compile kernel:

 [  220.561051] watchdog: BUG: soft lockup - CPU#53 stuck for 22s! [bash:10287]
 [...]
 [  220.753837] Call Trace:
 [  220.756296]  kmemleak_scan+0x21a/0x4c0
 [  220.760034]  kmemleak_write+0x312/0x350
 [  220.763866]  ? do_wp_page+0x147/0x4c0
 [  220.767521]  full_proxy_write+0x5a/0xa0
 [  220.771351]  __vfs_write+0x33/0x150
 [  220.774833]  ? __inode_security_revalidate+0x4c/0x60
 [  220.779782]  ? selinux_file_permission+0xda/0x130
 [  220.784479]  ? _cond_resched+0x15/0x30
 [  220.788221]  vfs_write+0xad/0x1a0
 [  220.791529]  SyS_write+0x52/0xc0
 [  220.794758]  do_syscall_64+0x61/0x1a0
 [  220.798411]  entry_SYSCALL64_slow_path+0x25/0x25

Fix this by adding cond_resched every 1024 pages.

Signed-off-by: Yisheng Xie 
---
 mm/kmemleak.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index e4738d5..e9f2e86 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -1523,6 +1523,8 @@ static void kmemleak_scan(void)
if (page_count(page) == 0)
continue;
scan_block(page, page + 1, NULL);
+   if (!(pfn % 1024))
+   cond_resched();
}
}
put_online_mems();
-- 
1.7.12.4

Re: [RFC v5 08/11] [media] vb2: add videobuf2 dma-buf fence helpers

[Alexandre Courbot]

On Friday, November 17, 2017 4:02:56 PM JST, Alexandre Courbot wrote:

On Thursday, November 16, 2017 2:10:54 AM JST, Gustavo Padovan wrote:

From: Javier Martinez Canillas 

Add a videobuf2-fence.h header file that contains different helpers
for DMA buffer sharing explicit fence support in videobuf2.

v2: - use fence context provided by the caller in vb2_fence_alloc()

Signed-off-by: Javier Martinez Canillas  ...


It is probably not a good idea to define that struct here since it will be
deduplicated for every source file that includes it.

Maybe change it to a simple declaration, and move the definition to
videobuf2-core.c or a dedicated videobuf2-fence.c file?


+
+static inline struct dma_fence *vb2_fence_alloc(u64 context)
+{
+   struct dma_fence *vb2_fence = kzalloc(sizeof(*vb2_fence), GFP_KERNEL);
+ ...


Not sure we gain a lot by having this function static inline, but your call.



Looking at the following patch, since it seems that this function is only 
to be

called from vb2_setup_out_fence() anyway, you may as well make it static in
videobuf2-core.c or even inline it in vb2_setup_out_fence() - it would only
add a few extra lines to this function.

Re: [RFC v5 08/11] [media] vb2: add videobuf2 dma-buf fence helpers

[Alexandre Courbot]

On Friday, November 17, 2017 4:02:56 PM JST, Alexandre Courbot wrote:

On Thursday, November 16, 2017 2:10:54 AM JST, Gustavo Padovan wrote:

From: Javier Martinez Canillas 

Add a videobuf2-fence.h header file that contains different helpers
for DMA buffer sharing explicit fence support in videobuf2.

v2: - use fence context provided by the caller in vb2_fence_alloc()

Signed-off-by: Javier Martinez Canillas  ...


It is probably not a good idea to define that struct here since it will be
deduplicated for every source file that includes it.

Maybe change it to a simple declaration, and move the definition to
videobuf2-core.c or a dedicated videobuf2-fence.c file?


+
+static inline struct dma_fence *vb2_fence_alloc(u64 context)
+{
+   struct dma_fence *vb2_fence = kzalloc(sizeof(*vb2_fence), GFP_KERNEL);
+ ...


Not sure we gain a lot by having this function static inline, but your call.



Looking at the following patch, since it seems that this function is only 
to be

called from vb2_setup_out_fence() anyway, you may as well make it static in
videobuf2-core.c or even inline it in vb2_setup_out_fence() - it would only
add a few extra lines to this function.

Re: [alsa-devel] [PATCH v7 05/13] slimbus: core: add support to device tree helper

[Vinod Koul]

On Wed, Nov 15, 2017 at 02:10:35PM +, srinivas.kandaga...@linaro.org wrote:
> +
> + for_each_child_of_node(ctrl->dev->of_node, node) {
> + struct slim_device *sbdev;
> + struct slim_eaddr e_addr;
> + const char *compat = NULL;
> + int reg[2], ret;
> + int manf_id, prod_code;
> +
> + compat = of_get_property(node, "compatible", NULL);
> + if (!compat)
> + continue;
> +
> + ret = sscanf(compat, "slim%x,%x", _id, _code);
> + if (ret != 2) {
> + dev_err(dev, "Manf ID & Product code not found %s\n",
> + compat);
> + continue;
> + }

can we do xxx_property_read_u32() on compat and then use bit manupliations
to get ids rather than scanf.

> +
> + ret = of_property_read_u32_array(node, "reg", reg, 2);

I typically prefer using device_property_read_u32() but then this is of_
code so I will leave this upto you...

-- 
~Vinod

Re: [alsa-devel] [PATCH v7 05/13] slimbus: core: add support to device tree helper

[Vinod Koul]

On Wed, Nov 15, 2017 at 02:10:35PM +, srinivas.kandaga...@linaro.org wrote:
> +
> + for_each_child_of_node(ctrl->dev->of_node, node) {
> + struct slim_device *sbdev;
> + struct slim_eaddr e_addr;
> + const char *compat = NULL;
> + int reg[2], ret;
> + int manf_id, prod_code;
> +
> + compat = of_get_property(node, "compatible", NULL);
> + if (!compat)
> + continue;
> +
> + ret = sscanf(compat, "slim%x,%x", _id, _code);
> + if (ret != 2) {
> + dev_err(dev, "Manf ID & Product code not found %s\n",
> + compat);
> + continue;
> + }

can we do xxx_property_read_u32() on compat and then use bit manupliations
to get ids rather than scanf.

> +
> + ret = of_property_read_u32_array(node, "reg", reg, 2);

I typically prefer using device_property_read_u32() but then this is of_
code so I will leave this upto you...

-- 
~Vinod

Re: [RFC 00/19] KVM: s390/crypto/vfio: guest dedicated crypto adapters

[Pierre Morel]

On 17/11/2017 00:35, Tony Krowiak wrote:

On 11/16/2017 03:25 PM, Pierre Morel wrote:

On 16/11/2017 18:03, Cornelia Huck wrote:

On Thu, 16 Nov 2017 17:06:58 +0100
Pierre Morel  wrote:


On 16/11/2017 16:23, Tony Krowiak wrote:

On 11/14/2017 08:57 AM, Cornelia Huck wrote:

On Tue, 31 Oct 2017 15:39:09 -0400
Tony Krowiak  wrote:

On 10/13/2017 01:38 PM, Tony Krowiak wrote:
Ping

Tony Krowiak (19):
 KVM: s390: SIE considerations for AP Queue virtualization
 KVM: s390: refactor crypto initialization
 s390/zcrypt: new AP matrix bus
 s390/zcrypt: create an AP matrix device on the AP matrix bus
 s390/zcrypt: base implementation of AP matrix device driver
 s390/zcrypt: register matrix device with VFIO mediated device
   framework
 KVM: s390: introduce AP matrix configuration interface
 s390/zcrypt: support for assigning adapters to matrix mdev
 s390/zcrypt: validate adapter assignment
 s390/zcrypt: sysfs interfaces supporting AP domain assignment
 s390/zcrypt: validate domain assignment
 s390/zcrypt: sysfs support for control domain assignment
 s390/zcrypt: validate control domain assignment
 KVM: s390: Connect the AP mediated matrix device to KVM
 s390/zcrypt: introduce ioctl access to VFIO AP Matrix driver
 KVM: s390: interface to configure KVM guest's AP matrix
 KVM: s390: validate input to AP matrix config interface
 KVM: s390: New ioctl to configure KVM guest's AP matrix
 s390/facilities: enable AP facilities needed by guest
I think the approach is fine, and the code also looks fine for the 
most

part. Some comments:

- various patches can be squashed together to give a better
    understanding at a glance

Which patches would you squash?

- this needs documentation (as I already said)

My plan is to take the cover letter patch and incorporate that into
documentation,
then replace the cover letter patch with a more concise summary.
- some of the driver/device modelling feels a bit awkward 
(commented in
    patches) -- I'm not sure that my proposal is better, but I 
think we

    should make sure the interdependencies are modeled correctly

I am responding to each patch review individually.


I think that instead of responding to each patch individually we should
have a discussion on the design because I think a lot could change and
discussing about each patch as they may be completely redesigned for 
the

next version may not be very useful.

How do you suggest this discussion should be structured? Aren't the patches
themselves an ultimate expression of the design? A lot could change, but
can't those issues can be dealt with and discussed as we move forward?




So I totally agree with Conny on that we should stabilize the
bus/device/driver modeling.

I think it would be here a good place to start the discussion on things
like we started to discuss, Harald and I, off-line:
- why a matrix bus, in which case we can avoid it


I thought it had been agreed that we should be able to ditch it?


I have not see any comment on the matrix bus.

As stated in a previous email responding to Connie, I decided to scrap the
AP matrix bus. There will only ever be one matrix device that serves two
purposes: To hold the APQNs of the queue devices bound to the VFIO AP 
matrix

device driver; to serve as a parent of the mediated devices created for
guests requiring access to the APQNs reserved for their use. So, instead
of an AP matrix bus creating the matrix device, it will be created by the
VFIO AP matrix driver in /sys/devices/ap_matrix/ during driver 
initialization.


Sorry, I did not see the mail, this of course change a lot of things...







- which kind of devices we need


What is still unclear? Which card generations to support?


No, I mean the relation bus/device/driver/mdev...

I think that is pretty well spelled out in the cover letter
patch and in the descriptions of the patches themselves. What is it
you don't understand?


If we have no matrix bus anymore I prefer to wait for the cover letter 
of V2 to discuss this.








- how to handle the repartition of queues on boot, reset and hotplug

What do you mean by repartition of queues on boot?


That's something I'd like to see a writeup for.


yes, and it may have an influence on the bus/device/driver/mdev design

I don't understand the need to avoid implementation details. If you recall,
the original design was modeled on AP queue devices. It was only after
implementing that design that the shortcomings were revealed which is
why we decided to base the model on the AP matrix. Keep in mind, this is
an RFC, not a final patch set. I would expect some change from the
implementation herein. In fact, I've already made many changes based on
Connie's and Christian's review comments, none of which resulted in an
overhaul of the design.






- interaction with the host drivers


The driver model should already handle that, no?

Re: [RFC 00/19] KVM: s390/crypto/vfio: guest dedicated crypto adapters

[Pierre Morel]

On 17/11/2017 00:35, Tony Krowiak wrote:

On 11/16/2017 03:25 PM, Pierre Morel wrote:

On 16/11/2017 18:03, Cornelia Huck wrote:

On Thu, 16 Nov 2017 17:06:58 +0100
Pierre Morel  wrote:


On 16/11/2017 16:23, Tony Krowiak wrote:

On 11/14/2017 08:57 AM, Cornelia Huck wrote:

On Tue, 31 Oct 2017 15:39:09 -0400
Tony Krowiak  wrote:

On 10/13/2017 01:38 PM, Tony Krowiak wrote:
Ping

Tony Krowiak (19):
 KVM: s390: SIE considerations for AP Queue virtualization
 KVM: s390: refactor crypto initialization
 s390/zcrypt: new AP matrix bus
 s390/zcrypt: create an AP matrix device on the AP matrix bus
 s390/zcrypt: base implementation of AP matrix device driver
 s390/zcrypt: register matrix device with VFIO mediated device
   framework
 KVM: s390: introduce AP matrix configuration interface
 s390/zcrypt: support for assigning adapters to matrix mdev
 s390/zcrypt: validate adapter assignment
 s390/zcrypt: sysfs interfaces supporting AP domain assignment
 s390/zcrypt: validate domain assignment
 s390/zcrypt: sysfs support for control domain assignment
 s390/zcrypt: validate control domain assignment
 KVM: s390: Connect the AP mediated matrix device to KVM
 s390/zcrypt: introduce ioctl access to VFIO AP Matrix driver
 KVM: s390: interface to configure KVM guest's AP matrix
 KVM: s390: validate input to AP matrix config interface
 KVM: s390: New ioctl to configure KVM guest's AP matrix
 s390/facilities: enable AP facilities needed by guest
I think the approach is fine, and the code also looks fine for the 
most

part. Some comments:

- various patches can be squashed together to give a better
    understanding at a glance

Which patches would you squash?

- this needs documentation (as I already said)

My plan is to take the cover letter patch and incorporate that into
documentation,
then replace the cover letter patch with a more concise summary.
- some of the driver/device modelling feels a bit awkward 
(commented in
    patches) -- I'm not sure that my proposal is better, but I 
think we

    should make sure the interdependencies are modeled correctly

I am responding to each patch review individually.


I think that instead of responding to each patch individually we should
have a discussion on the design because I think a lot could change and
discussing about each patch as they may be completely redesigned for 
the

next version may not be very useful.

How do you suggest this discussion should be structured? Aren't the patches
themselves an ultimate expression of the design? A lot could change, but
can't those issues can be dealt with and discussed as we move forward?




So I totally agree with Conny on that we should stabilize the
bus/device/driver modeling.

I think it would be here a good place to start the discussion on things
like we started to discuss, Harald and I, off-line:
- why a matrix bus, in which case we can avoid it


I thought it had been agreed that we should be able to ditch it?


I have not see any comment on the matrix bus.

As stated in a previous email responding to Connie, I decided to scrap the
AP matrix bus. There will only ever be one matrix device that serves two
purposes: To hold the APQNs of the queue devices bound to the VFIO AP 
matrix

device driver; to serve as a parent of the mediated devices created for
guests requiring access to the APQNs reserved for their use. So, instead
of an AP matrix bus creating the matrix device, it will be created by the
VFIO AP matrix driver in /sys/devices/ap_matrix/ during driver 
initialization.


Sorry, I did not see the mail, this of course change a lot of things...







- which kind of devices we need


What is still unclear? Which card generations to support?


No, I mean the relation bus/device/driver/mdev...

I think that is pretty well spelled out in the cover letter
patch and in the descriptions of the patches themselves. What is it
you don't understand?


If we have no matrix bus anymore I prefer to wait for the cover letter 
of V2 to discuss this.








- how to handle the repartition of queues on boot, reset and hotplug

What do you mean by repartition of queues on boot?


That's something I'd like to see a writeup for.


yes, and it may have an influence on the bus/device/driver/mdev design

I don't understand the need to avoid implementation details. If you recall,
the original design was modeled on AP queue devices. It was only after
implementing that design that the shortcomings were revealed which is
why we decided to base the model on the AP matrix. Keep in mind, this is
an RFC, not a final patch set. I would expect some change from the
implementation herein. In fact, I've already made many changes based on
Connie's and Christian's review comments, none of which resulted in an
overhaul of the design.






- interaction with the host drivers


The driver model should already handle that, no?


yes it should, but it is not clear for me.

What is it 

Re: [PATCH] mailbox: txdone_method shouldn't always be reset

[Bjorn Andersson]

On Thu 16 Nov 22:47 PST 2017, Jassi Brar wrote:
> On 16 Nov 2017 23:12, "Bjorn Andersson"  wrote:
> On Thu 16 Nov 09:06 PST 2017, Jassi Brar wrote:
> > diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> > index 674b35f..95e480e 100644
> > --- a/drivers/mailbox/mailbox.c
> > +++ b/drivers/mailbox/mailbox.c
> > @@ -124,7 +124,8 @@ static enum hrtimer_restart txdone_hrtimer(struct
> > hrtimer *hrtimer)
> > for (i = 0; i < mbox->num_chans; i++) {
> > struct mbox_chan *chan = >chans[i];
> >
> > -   if (chan->active_req && chan->cl) {
> > +   if (chan->active_req && chan->cl &&
> > +   chan->txdone_method == TXDONE_BY_POLL) {
> 
> The hrtimer code will crash before reaching this point if the channel
> wasn't TXDONE_BY_POLL when it was created, so this part is not needed.
> 
> 
> We have one timer for all channels of a controller. While this channel may
> be run by ACK, some other might need to be POLLed. And we want to avoid
> polling this channel.
> 

Oh, you're right.

But the fact that the timer function will poll channels that are
"upgraded" to ACK is a separate issue.

> 
> > txdone = chan->mbox->ops->last_tx_done(chan);
> > if (txdone)
> > tx_tick(chan, 0);
> > @@ -418,7 +419,7 @@ void mbox_free_channel(struct mbox_chan *chan)
> > spin_lock_irqsave(>lock, flags);
> > chan->cl = NULL;
> > chan->active_req = NULL;
> > -   if (chan->txdone_method == TXDONE_BY_ACK)
> > +   if (chan->txdone_method == TXDONE_BY_ACK &&
> chan->mbox->txdone_poll)
> 
> This should be enough, but the logic here is not obvious. This relies on
> the fact that a mbox with txdone_poll would have been initialized as
> txdone = POLL and that txdone now being ACK would imply that it was
> "upgraded" in the request path.
> 
> So at least this would have to be captured in a comment.
> 
> 
> Sure I'll add a comment.
> 

Thanks

Regards,
Bjorn

Re: [PATCH] mailbox: txdone_method shouldn't always be reset

[Bjorn Andersson]

On Thu 16 Nov 22:47 PST 2017, Jassi Brar wrote:
> On 16 Nov 2017 23:12, "Bjorn Andersson"  wrote:
> On Thu 16 Nov 09:06 PST 2017, Jassi Brar wrote:
> > diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> > index 674b35f..95e480e 100644
> > --- a/drivers/mailbox/mailbox.c
> > +++ b/drivers/mailbox/mailbox.c
> > @@ -124,7 +124,8 @@ static enum hrtimer_restart txdone_hrtimer(struct
> > hrtimer *hrtimer)
> > for (i = 0; i < mbox->num_chans; i++) {
> > struct mbox_chan *chan = >chans[i];
> >
> > -   if (chan->active_req && chan->cl) {
> > +   if (chan->active_req && chan->cl &&
> > +   chan->txdone_method == TXDONE_BY_POLL) {
> 
> The hrtimer code will crash before reaching this point if the channel
> wasn't TXDONE_BY_POLL when it was created, so this part is not needed.
> 
> 
> We have one timer for all channels of a controller. While this channel may
> be run by ACK, some other might need to be POLLed. And we want to avoid
> polling this channel.
> 

Oh, you're right.

But the fact that the timer function will poll channels that are
"upgraded" to ACK is a separate issue.

> 
> > txdone = chan->mbox->ops->last_tx_done(chan);
> > if (txdone)
> > tx_tick(chan, 0);
> > @@ -418,7 +419,7 @@ void mbox_free_channel(struct mbox_chan *chan)
> > spin_lock_irqsave(>lock, flags);
> > chan->cl = NULL;
> > chan->active_req = NULL;
> > -   if (chan->txdone_method == TXDONE_BY_ACK)
> > +   if (chan->txdone_method == TXDONE_BY_ACK &&
> chan->mbox->txdone_poll)
> 
> This should be enough, but the logic here is not obvious. This relies on
> the fact that a mbox with txdone_poll would have been initialized as
> txdone = POLL and that txdone now being ACK would imply that it was
> "upgraded" in the request path.
> 
> So at least this would have to be captured in a comment.
> 
> 
> Sure I'll add a comment.
> 

Thanks

Regards,
Bjorn

Re: [RFC v5 08/11] [media] vb2: add videobuf2 dma-buf fence helpers

[Alexandre Courbot]

On Thursday, November 16, 2017 2:10:54 AM JST, Gustavo Padovan wrote:

From: Javier Martinez Canillas 

Add a videobuf2-fence.h header file that contains different helpers
for DMA buffer sharing explicit fence support in videobuf2.

v2: - use fence context provided by the caller in vb2_fence_alloc()

Signed-off-by: Javier Martinez Canillas 
Signed-off-by: Gustavo Padovan 
---
 include/media/videobuf2-fence.h | 48 
+

 1 file changed, 48 insertions(+)
 create mode 100644 include/media/videobuf2-fence.h

diff --git a/include/media/videobuf2-fence.h 
b/include/media/videobuf2-fence.h

new file mode 100644
index ..b49cc1bf6bb4
--- /dev/null
+++ b/include/media/videobuf2-fence.h
@@ -0,0 +1,48 @@
+/*
+ * videobuf2-fence.h - DMA buffer sharing fence helpers for videobuf 2
+ *
+ * Copyright (C) 2016 Samsung Electronics
+ *
+ * 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.
+ */
+
+#include 
+#include 
+
+static DEFINE_SPINLOCK(vb2_fence_lock);
+
+static inline const char *vb2_fence_get_driver_name(struct 
dma_fence *fence)

+{
+   return "vb2_fence";
+}
+
+static inline const char *vb2_fence_get_timeline_name(struct 
dma_fence *fence)

+{
+   return "vb2_fence_timeline";
+}
+
+static inline bool vb2_fence_enable_signaling(struct dma_fence *fence)
+{
+   return true;
+}
+
+static const struct dma_fence_ops vb2_fence_ops = {
+   .get_driver_name = vb2_fence_get_driver_name,
+   .get_timeline_name = vb2_fence_get_timeline_name,
+   .enable_signaling = vb2_fence_enable_signaling,
+   .wait = dma_fence_default_wait,
+};


It is probably not a good idea to define that struct here since it will be
deduplicated for every source file that includes it.

Maybe change it to a simple declaration, and move the definition to
videobuf2-core.c or a dedicated videobuf2-fence.c file?


+
+static inline struct dma_fence *vb2_fence_alloc(u64 context)
+{
+   struct dma_fence *vb2_fence = kzalloc(sizeof(*vb2_fence), GFP_KERNEL);
+
+   if (!vb2_fence)
+   return NULL;
+
+   dma_fence_init(vb2_fence, _fence_ops, _fence_lock, context, 1);
+
+   return vb2_fence;
+}


Not sure we gain a lot by having this function static inline, but your 
call.

Re: [RFC v5 08/11] [media] vb2: add videobuf2 dma-buf fence helpers

[Alexandre Courbot]

On Thursday, November 16, 2017 2:10:54 AM JST, Gustavo Padovan wrote:

From: Javier Martinez Canillas 

Add a videobuf2-fence.h header file that contains different helpers
for DMA buffer sharing explicit fence support in videobuf2.

v2: - use fence context provided by the caller in vb2_fence_alloc()

Signed-off-by: Javier Martinez Canillas 
Signed-off-by: Gustavo Padovan 
---
 include/media/videobuf2-fence.h | 48 
+

 1 file changed, 48 insertions(+)
 create mode 100644 include/media/videobuf2-fence.h

diff --git a/include/media/videobuf2-fence.h 
b/include/media/videobuf2-fence.h

new file mode 100644
index ..b49cc1bf6bb4
--- /dev/null
+++ b/include/media/videobuf2-fence.h
@@ -0,0 +1,48 @@
+/*
+ * videobuf2-fence.h - DMA buffer sharing fence helpers for videobuf 2
+ *
+ * Copyright (C) 2016 Samsung Electronics
+ *
+ * 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.
+ */
+
+#include 
+#include 
+
+static DEFINE_SPINLOCK(vb2_fence_lock);
+
+static inline const char *vb2_fence_get_driver_name(struct 
dma_fence *fence)

+{
+   return "vb2_fence";
+}
+
+static inline const char *vb2_fence_get_timeline_name(struct 
dma_fence *fence)

+{
+   return "vb2_fence_timeline";
+}
+
+static inline bool vb2_fence_enable_signaling(struct dma_fence *fence)
+{
+   return true;
+}
+
+static const struct dma_fence_ops vb2_fence_ops = {
+   .get_driver_name = vb2_fence_get_driver_name,
+   .get_timeline_name = vb2_fence_get_timeline_name,
+   .enable_signaling = vb2_fence_enable_signaling,
+   .wait = dma_fence_default_wait,
+};


It is probably not a good idea to define that struct here since it will be
deduplicated for every source file that includes it.

Maybe change it to a simple declaration, and move the definition to
videobuf2-core.c or a dedicated videobuf2-fence.c file?


+
+static inline struct dma_fence *vb2_fence_alloc(u64 context)
+{
+   struct dma_fence *vb2_fence = kzalloc(sizeof(*vb2_fence), GFP_KERNEL);
+
+   if (!vb2_fence)
+   return NULL;
+
+   dma_fence_init(vb2_fence, _fence_ops, _fence_lock, context, 1);
+
+   return vb2_fence;
+}


Not sure we gain a lot by having this function static inline, but your 
call.

Re: [PATCH V1 1/4] qcom: spmi-wled: Add support for qcom wled driver

[Bjorn Andersson]

On Thu 16 Nov 22:36 PST 2017, kgu...@codeaurora.org wrote:

> On 2017-11-16 22:25, Bjorn Andersson wrote:
> > On Thu 16 Nov 04:18 PST 2017, Kiran Gunda wrote:
> > 
> > > WLED driver provides the interface to the display driver to
> > > adjust the brightness of the display backlight.
> > > 
> > 
> > Hi Kiran,
> > 
> > This driver has a lot in common with the already upstream pm8941-wled.c,
> > because it's just a new revision of the same block.
> > 
> > Please extend the existing driver rather than providing a new one
> > (and yes, renaming the file is okay).
> > 
> > Regards,
> > Bjorn
> 
> Hi Bjorn,
> 
> Yes this driver design is similar to pm8941, however the WLED HW block
> has undergone quite a few changes in analog and digital from PM8941 to
> PM8998.

I can see that, looking at the documentation.

> Few of them include splitting one module into wled-ctrl and wled-sink
> peripherals, changes in the register offsets and the bit
> interpretation.

This is typical and something we need to handle in all these drivers, to
avoid having one driver per platform.

> Hence we concluded that it was better to have a new driver to support
> this new gen WELD module and decouple it from the pm8941.

Okay, I can see how it's easier to not have to case about anything but
pmi8998 in this driver, but where do you add the support for other WLED
versions? What about PMI8994? Will there not be similar differences
(registers that has moved around) in the future?

> Also, going forward this driver will support AMOLED AVDD rail (not
> supported by pm8941) touching a few more registers/configuration and
> newer PMICs.

Is this a feature that was introduced in PMI8998? Will this support not
be dependent on the pmic version?

> So spinning off a new driver would make it cleaner and easier to
> extend further.
> 

It's for sure easier at this point in time, but your argumentation
implies that PMI8998+1 should go into it's own driver as well.

I suspect that if you're going to reuse this driver for future PMIC
versions you will have to deal with register layout differences and new
feature set, and as such I'm not convinced that a new driver is needed.


Can you give any concrete examples of where it is not possible or
undesirable to maintain the pm8941 support in the same driver?

Regards,
Bjorn

Re: [PATCH V1 1/4] qcom: spmi-wled: Add support for qcom wled driver

[Bjorn Andersson]

On Thu 16 Nov 22:36 PST 2017, kgu...@codeaurora.org wrote:

> On 2017-11-16 22:25, Bjorn Andersson wrote:
> > On Thu 16 Nov 04:18 PST 2017, Kiran Gunda wrote:
> > 
> > > WLED driver provides the interface to the display driver to
> > > adjust the brightness of the display backlight.
> > > 
> > 
> > Hi Kiran,
> > 
> > This driver has a lot in common with the already upstream pm8941-wled.c,
> > because it's just a new revision of the same block.
> > 
> > Please extend the existing driver rather than providing a new one
> > (and yes, renaming the file is okay).
> > 
> > Regards,
> > Bjorn
> 
> Hi Bjorn,
> 
> Yes this driver design is similar to pm8941, however the WLED HW block
> has undergone quite a few changes in analog and digital from PM8941 to
> PM8998.

I can see that, looking at the documentation.

> Few of them include splitting one module into wled-ctrl and wled-sink
> peripherals, changes in the register offsets and the bit
> interpretation.

This is typical and something we need to handle in all these drivers, to
avoid having one driver per platform.

> Hence we concluded that it was better to have a new driver to support
> this new gen WELD module and decouple it from the pm8941.

Okay, I can see how it's easier to not have to case about anything but
pmi8998 in this driver, but where do you add the support for other WLED
versions? What about PMI8994? Will there not be similar differences
(registers that has moved around) in the future?

> Also, going forward this driver will support AMOLED AVDD rail (not
> supported by pm8941) touching a few more registers/configuration and
> newer PMICs.

Is this a feature that was introduced in PMI8998? Will this support not
be dependent on the pmic version?

> So spinning off a new driver would make it cleaner and easier to
> extend further.
> 

It's for sure easier at this point in time, but your argumentation
implies that PMI8998+1 should go into it's own driver as well.

I suspect that if you're going to reuse this driver for future PMIC
versions you will have to deal with register layout differences and new
feature set, and as such I'm not convinced that a new driver is needed.


Can you give any concrete examples of where it is not possible or
undesirable to maintain the pm8941 support in the same driver?

Regards,
Bjorn

Re: [PATCH] pinctrl: intel: save HOSTSW_OWN register over suspend/resume

[Mika Westerberg]

On Thu, Nov 16, 2017 at 09:27:51PM +0800, Chris Chiu wrote:
> On Thu, Nov 16, 2017 at 8:44 PM, Mika Westerberg
>  wrote:
> > On Wed, Nov 15, 2017 at 06:19:56PM +0800, Chris Chiu wrote:
> >> Hi Mika,
> >> I've confirmed with Asus and they said it's the latest BIOS for
> >> shipment and verified OK on Windows. So their BIOS team will not do
> >> anything for this.
> >
> > I'll ask around if our Windows people know anything about this. My gut
> > feeling is that the Windows driver does not touch HOSTSW_OWN either.
> 
> Thanks. Please let me know if you need any information. I still keep
> the machine.

Got confirmation from Windows people. So Windows pretty much saves and
restores the same registers than we do (padcfg + ie).

Have you tried whether s2idle works instead of S3 suspend? You can try
it like

  # echo freeze > /sys/power/state

If that works, I'm guessing that this system uses s2idle and that's also
what Windows uses and could explain why it works in Windows.

Re: [PATCH] pinctrl: intel: save HOSTSW_OWN register over suspend/resume

[Mika Westerberg]

On Thu, Nov 16, 2017 at 09:27:51PM +0800, Chris Chiu wrote:
> On Thu, Nov 16, 2017 at 8:44 PM, Mika Westerberg
>  wrote:
> > On Wed, Nov 15, 2017 at 06:19:56PM +0800, Chris Chiu wrote:
> >> Hi Mika,
> >> I've confirmed with Asus and they said it's the latest BIOS for
> >> shipment and verified OK on Windows. So their BIOS team will not do
> >> anything for this.
> >
> > I'll ask around if our Windows people know anything about this. My gut
> > feeling is that the Windows driver does not touch HOSTSW_OWN either.
> 
> Thanks. Please let me know if you need any information. I still keep
> the machine.

Got confirmation from Windows people. So Windows pretty much saves and
restores the same registers than we do (padcfg + ie).

Have you tried whether s2idle works instead of S3 suspend? You can try
it like

  # echo freeze > /sys/power/state

If that works, I'm guessing that this system uses s2idle and that's also
what Windows uses and could explain why it works in Windows.

Re: [PATCH 02/13] x86/paravirt: Fix output constraint macro names

[Juergen Gross]

On 16/11/17 21:50, Josh Poimboeuf wrote:
> On Wed, Oct 25, 2017 at 11:33:43AM +0200, Juergen Gross wrote:
>> On 04/10/17 17:58, Josh Poimboeuf wrote:
>>> Some of the paravirt '*_CLOBBERS' macros refer to output constraints
>>> instead of clobbers, which makes the code extra confusing.  Rename the
>>> output constraint related macros to '*_OUTPUTS'.
>>>
>>> Signed-off-by: Josh Poimboeuf 
>>
>> I'm fine with the changes, but you might want to rename the "call_clbr"
>> parameter of PVOP_[V]CALL, too, e.g. to "outputs".
> 
> Yeah, good catch.
> 
>> You could then drop the "CALL_" from the macros, too.
> 
> Hm, which macros are you referring to, and why?

Good question. I think I didn't take the *CALLEE* macros into account.
So please ignore this remark.


Juergen

Re: [PATCH 02/13] x86/paravirt: Fix output constraint macro names

[Juergen Gross]

On 16/11/17 21:50, Josh Poimboeuf wrote:
> On Wed, Oct 25, 2017 at 11:33:43AM +0200, Juergen Gross wrote:
>> On 04/10/17 17:58, Josh Poimboeuf wrote:
>>> Some of the paravirt '*_CLOBBERS' macros refer to output constraints
>>> instead of clobbers, which makes the code extra confusing.  Rename the
>>> output constraint related macros to '*_OUTPUTS'.
>>>
>>> Signed-off-by: Josh Poimboeuf 
>>
>> I'm fine with the changes, but you might want to rename the "call_clbr"
>> parameter of PVOP_[V]CALL, too, e.g. to "outputs".
> 
> Yeah, good catch.
> 
>> You could then drop the "CALL_" from the macros, too.
> 
> Hm, which macros are you referring to, and why?

Good question. I think I didn't take the *CALLEE* macros into account.
So please ignore this remark.


Juergen

[PATCH] sched: use unsigned int for one-bit bitfield in sched_dl_entity

[Xin Long]

This patch is to fix the 'dubious one-bit signed bitfield' error reported
by sparse, when using 'make C=2'.

Fixes: 799ba82de01e ("sched/deadline: Use C bitfields for the state flags")
Signed-off-by: Xin Long 
---
 include/linux/sched.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a5dc7c9..3e35a37 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -473,10 +473,10 @@ struct sched_dl_entity {
 * conditions between the inactive timer handler and the wakeup
 * code.
 */
-   int dl_throttled  : 1;
-   int dl_boosted: 1;
-   int dl_yielded: 1;
-   int dl_non_contending : 1;
+   unsigned intdl_throttled  : 1,
+   dl_boosted: 1,
+   dl_yielded: 1,
+   dl_non_contending : 1;
 
/*
 * Bandwidth enforcement timer. Each -deadline task has its
-- 
2.1.0

[PATCH] sched: use unsigned int for one-bit bitfield in sched_dl_entity

[Xin Long]

This patch is to fix the 'dubious one-bit signed bitfield' error reported
by sparse, when using 'make C=2'.

Fixes: 799ba82de01e ("sched/deadline: Use C bitfields for the state flags")
Signed-off-by: Xin Long 
---
 include/linux/sched.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a5dc7c9..3e35a37 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -473,10 +473,10 @@ struct sched_dl_entity {
 * conditions between the inactive timer handler and the wakeup
 * code.
 */
-   int dl_throttled  : 1;
-   int dl_boosted: 1;
-   int dl_yielded: 1;
-   int dl_non_contending : 1;
+   unsigned intdl_throttled  : 1,
+   dl_boosted: 1,
+   dl_yielded: 1,
+   dl_non_contending : 1;
 
/*
 * Bandwidth enforcement timer. Each -deadline task has its
-- 
2.1.0

Re: [RFC v5 07/11] [media] vb2: add in-fence support to QBUF

[Alexandre Courbot]

Hi Gustavo,

I am coming a bit late in this series' review, so apologies if some of my
comments have already have been discussed in an earlier revision.

On Thursday, November 16, 2017 2:10:53 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

Receive in-fence from userspace and add support for waiting on them
before queueing the buffer to the driver. Buffers can't be queued to the
driver before its fences signal. And a buffer can't be queue to the driver
out of the order they were queued from userspace. That means that even if
it fence signal it must wait all other buffers, ahead of it in the queue,
to signal first.

To make that possible we use fence_array to keep that ordering. Basically
we create a fence_array that contains both the current fence and the fence
from the previous buffer (which might be a fence array as well). The base
fence class for the fence_array becomes the new buffer fence, waiting on
that one guarantees that it won't be queued out of order.

v6:
- With fences always keep the order userspace queues the buffers.
- Protect in_fence manipulation with a lock (Brian Starkey)
- check if fences have the same context before adding a fence array
- Fix last_fence ref unbalance in __set_in_fence() (Brian Starkey)
- Clean up fence if __set_in_fence() fails (Brian Starkey)
- treat -EINVAL from dma_fence_add_callback() (Brian Starkey)

v5: - use fence_array to keep buffers ordered in vb2 core when
needed (Brian Starkey)
- keep backward compat on the reserved2 field (Brian Starkey)
- protect fence callback removal with lock (Brian Starkey)

v4:
- Add a comment about dma_fence_add_callback() not returning a
error (Hans)
- Call dma_fence_put(vb->in_fence) if fence signaled (Hans)
- select SYNC_FILE under config VIDEOBUF2_CORE (Hans)
- Move dma_fence_is_signaled() check to __enqueue_in_driver() (Hans)
- Remove list_for_each_entry() in __vb2_core_qbuf() (Hans)
-  Remove if (vb->state != VB2_BUF_STATE_QUEUED) from
vb2_start_streaming() (Hans)
- set IN_FENCE flags on __fill_v4l2_buffer (Hans)
- Queue buffers to the driver as soon as they are ready (Hans)
- call fill_user_buffer() after queuing the buffer (Hans)
- add err: label to clean up fence
- add dma_fence_wait() before calling vb2_start_streaming()

v3: - document fence parameter
- remove ternary if at vb2_qbuf() return (Mauro)
- do not change if conditions behaviour (Mauro)

v2:
- fix vb2_queue_or_prepare_buf() ret check
- remove check for VB2_MEMORY_DMABUF only (Javier)
- check num of ready buffers to start streaming
- when queueing, start from the first ready buffer
- handle queue cancel

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/Kconfig  |   1 +
 drivers/media/v4l2-core/videobuf2-core.c | 202 
---

 drivers/media/v4l2-core/videobuf2-v4l2.c |  29 -
 include/media/videobuf2-core.h   |  17 ++-
 4 files changed, 231 insertions(+), 18 deletions(-)

diff --git a/drivers/media/v4l2-core/Kconfig 
b/drivers/media/v4l2-core/Kconfig

index a35c33686abf..3f988c407c80 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -83,6 +83,7 @@ config VIDEOBUF_DVB
 # Used by drivers that need Videobuf2 modules
 config VIDEOBUF2_CORE
select DMA_SHARED_BUFFER
+   select SYNC_FILE
tristate
 
 config VIDEOBUF2_MEMOPS
diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c

index 60f8b582396a..26de4c80717d 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 

 #include 
@@ -346,6 +347,7 @@ static int __vb2_queue_alloc(struct 
vb2_queue *q, enum vb2_memory memory,

vb->index = q->num_buffers + buffer;
vb->type = q->type;
vb->memory = memory;
+   spin_lock_init(>fence_cb_lock);
for (plane = 0; plane < num_planes; ++plane) {
vb->planes[plane].length = plane_sizes[plane];
vb->planes[plane].min_length = plane_sizes[plane];
@@ -1222,6 +1224,9 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
struct vb2_queue *q = vb->vb2_queue;
 
+	if (vb->in_fence && !dma_fence_is_signaled(vb->in_fence))

+   return;
+
vb->state = VB2_BUF_STATE_ACTIVE;
atomic_inc(>owned_by_drv_count);
 
@@ -1273,6 +1278,23 @@ static int __buf_prepare(struct 
vb2_buffer *vb, const void *pb)

return 0;
 }
 
+static int __get_num_ready_buffers(struct vb2_queue *q)

+{
+   struct vb2_buffer *vb;
+   int ready_count = 0;
+   unsigned long 

Re: [RFC v5 07/11] [media] vb2: add in-fence support to QBUF

[Alexandre Courbot]

Hi Gustavo,

I am coming a bit late in this series' review, so apologies if some of my
comments have already have been discussed in an earlier revision.

On Thursday, November 16, 2017 2:10:53 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

Receive in-fence from userspace and add support for waiting on them
before queueing the buffer to the driver. Buffers can't be queued to the
driver before its fences signal. And a buffer can't be queue to the driver
out of the order they were queued from userspace. That means that even if
it fence signal it must wait all other buffers, ahead of it in the queue,
to signal first.

To make that possible we use fence_array to keep that ordering. Basically
we create a fence_array that contains both the current fence and the fence
from the previous buffer (which might be a fence array as well). The base
fence class for the fence_array becomes the new buffer fence, waiting on
that one guarantees that it won't be queued out of order.

v6:
- With fences always keep the order userspace queues the buffers.
- Protect in_fence manipulation with a lock (Brian Starkey)
- check if fences have the same context before adding a fence array
- Fix last_fence ref unbalance in __set_in_fence() (Brian Starkey)
- Clean up fence if __set_in_fence() fails (Brian Starkey)
- treat -EINVAL from dma_fence_add_callback() (Brian Starkey)

v5: - use fence_array to keep buffers ordered in vb2 core when
needed (Brian Starkey)
- keep backward compat on the reserved2 field (Brian Starkey)
- protect fence callback removal with lock (Brian Starkey)

v4:
- Add a comment about dma_fence_add_callback() not returning a
error (Hans)
- Call dma_fence_put(vb->in_fence) if fence signaled (Hans)
- select SYNC_FILE under config VIDEOBUF2_CORE (Hans)
- Move dma_fence_is_signaled() check to __enqueue_in_driver() (Hans)
- Remove list_for_each_entry() in __vb2_core_qbuf() (Hans)
-  Remove if (vb->state != VB2_BUF_STATE_QUEUED) from
vb2_start_streaming() (Hans)
- set IN_FENCE flags on __fill_v4l2_buffer (Hans)
- Queue buffers to the driver as soon as they are ready (Hans)
- call fill_user_buffer() after queuing the buffer (Hans)
- add err: label to clean up fence
- add dma_fence_wait() before calling vb2_start_streaming()

v3: - document fence parameter
- remove ternary if at vb2_qbuf() return (Mauro)
- do not change if conditions behaviour (Mauro)

v2:
- fix vb2_queue_or_prepare_buf() ret check
- remove check for VB2_MEMORY_DMABUF only (Javier)
- check num of ready buffers to start streaming
- when queueing, start from the first ready buffer
- handle queue cancel

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/Kconfig  |   1 +
 drivers/media/v4l2-core/videobuf2-core.c | 202 
---

 drivers/media/v4l2-core/videobuf2-v4l2.c |  29 -
 include/media/videobuf2-core.h   |  17 ++-
 4 files changed, 231 insertions(+), 18 deletions(-)

diff --git a/drivers/media/v4l2-core/Kconfig 
b/drivers/media/v4l2-core/Kconfig

index a35c33686abf..3f988c407c80 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -83,6 +83,7 @@ config VIDEOBUF_DVB
 # Used by drivers that need Videobuf2 modules
 config VIDEOBUF2_CORE
select DMA_SHARED_BUFFER
+   select SYNC_FILE
tristate
 
 config VIDEOBUF2_MEMOPS
diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c

index 60f8b582396a..26de4c80717d 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 

 #include 
@@ -346,6 +347,7 @@ static int __vb2_queue_alloc(struct 
vb2_queue *q, enum vb2_memory memory,

vb->index = q->num_buffers + buffer;
vb->type = q->type;
vb->memory = memory;
+   spin_lock_init(>fence_cb_lock);
for (plane = 0; plane < num_planes; ++plane) {
vb->planes[plane].length = plane_sizes[plane];
vb->planes[plane].min_length = plane_sizes[plane];
@@ -1222,6 +1224,9 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
struct vb2_queue *q = vb->vb2_queue;
 
+	if (vb->in_fence && !dma_fence_is_signaled(vb->in_fence))

+   return;
+
vb->state = VB2_BUF_STATE_ACTIVE;
atomic_inc(>owned_by_drv_count);
 
@@ -1273,6 +1278,23 @@ static int __buf_prepare(struct 
vb2_buffer *vb, const void *pb)

return 0;
 }
 
+static int __get_num_ready_buffers(struct vb2_queue *q)

+{
+   struct vb2_buffer *vb;
+   int ready_count = 0;
+   unsigned long flags;
+
+   /* count num of buffers ready in front of the 

Re: [PATCH] mm/shmem: set default tmpfs size according to memcg limit

[Yafang Shao]

2017-11-17 12:43 GMT+08:00 Shakeel Butt :
> On Thu, Nov 16, 2017 at 7:09 PM, Yafang Shao  wrote:
>> Currently the default tmpfs size is totalram_pages / 2 if mount tmpfs
>> without "-o size=XXX".
>> When we mount tmpfs in a container(i.e. docker), it is also
>> totalram_pages / 2 regardless of the memory limit on this container.
>> That may easily cause OOM if tmpfs occupied too much memory when swap is
>> off.
>> So when we mount tmpfs in a memcg, the default size should be limited by
>> the memcg memory.limit.
>>
>
> The pages of the tmpfs files are charged to the memcg of allocators
> which can be in memcg different from the memcg in which the mount
> operation happened.

Yes.
But the issue is tmpfs files contributed to memory.usage_in_bytes
should be limited.
Let me take an example.
The physical memory size is 1G, and we create a memory cgroup then set the
memory.limit_in_bytes of it to 256M.
Then in this memory cgroup we do bellow test:
 1. mount -t tmpfs tmpfs /mount
 the size of which will be 1G / 2 by default.
  2. write files into this tmpfs
 as the limit of this memory cgroup is 256M while the size of
tmpfs is 512M,
 these files will occupy the while memory in this cgroup and
finally out of memory.


> So, tying the size of a tmpfs mount where it was
> mounted does not make much sense.
>
> Also mount operation which requires CAP_SYS_ADMIN, is usually
> performed by node controller (or job loader) which don't necessarily
> run in the memcg of the actual job.
>
>> Signed-off-by: Yafang Shao 
>> ---
>>  include/linux/memcontrol.h |  1 +
>>  mm/memcontrol.c|  2 +-
>>  mm/shmem.c | 20 +++-
>>  3 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
>> index 69966c4..79c6709 100644
>> --- a/include/linux/memcontrol.h
>> +++ b/include/linux/memcontrol.h
>> @@ -265,6 +265,7 @@ struct mem_cgroup {
>> /* WARNING: nodeinfo must be the last member here */
>>  };
>>
>> +extern struct mutex memcg_limit_mutex;
>>  extern struct mem_cgroup *root_mem_cgroup;
>>
>>  static inline bool mem_cgroup_disabled(void)
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 661f046..ad32f3c 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -2464,7 +2464,7 @@ static inline int 
>> mem_cgroup_move_swap_account(swp_entry_t entry,
>>  }
>>  #endif
>>
>> -static DEFINE_MUTEX(memcg_limit_mutex);
>> +DEFINE_MUTEX(memcg_limit_mutex);
>
> This mutex is only needed for updating the limit.
>

Thanks for explaination :)

>>
>>  static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
>>unsigned long limit)
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index 07a1d22..1c320dd 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -35,6 +35,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  #include  /* for arch/microblaze update_mmu_cache() */
>>
>> @@ -108,7 +109,24 @@ struct shmem_falloc {
>>  #ifdef CONFIG_TMPFS
>>  static unsigned long shmem_default_max_blocks(void)
>>  {
>> -   return totalram_pages / 2;
>> +   unsigned long size;
>> +
>> +#ifdef CONFIG_MEMCG
>> +   struct mem_cgroup *memcg = mem_cgroup_from_task(current);
>> +
>> +   if (memcg == NULL || memcg == root_mem_cgroup)
>> +   size = totalram_pages / 2;
>> +   else {
>> +   mutex_lock(_limit_mutex);
>> +   size = memcg->memory.limit > totalram_pages ?
>> +totalram_pages / 2 : memcg->memory.limit / 
>> 2;
>> +   mutex_unlock(_limit_mutex);
>> +   }
>> +#else
>> +   size = totalram_pages / 2;
>> +#endif
>> +
>> +   return size;
>>  }
>>
>>  static unsigned long shmem_default_max_inodes(void)
>> --
>> 1.8.3.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe cgroups" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mm/shmem: set default tmpfs size according to memcg limit

[Yafang Shao]

2017-11-17 12:43 GMT+08:00 Shakeel Butt :
> On Thu, Nov 16, 2017 at 7:09 PM, Yafang Shao  wrote:
>> Currently the default tmpfs size is totalram_pages / 2 if mount tmpfs
>> without "-o size=XXX".
>> When we mount tmpfs in a container(i.e. docker), it is also
>> totalram_pages / 2 regardless of the memory limit on this container.
>> That may easily cause OOM if tmpfs occupied too much memory when swap is
>> off.
>> So when we mount tmpfs in a memcg, the default size should be limited by
>> the memcg memory.limit.
>>
>
> The pages of the tmpfs files are charged to the memcg of allocators
> which can be in memcg different from the memcg in which the mount
> operation happened.

Yes.
But the issue is tmpfs files contributed to memory.usage_in_bytes
should be limited.
Let me take an example.
The physical memory size is 1G, and we create a memory cgroup then set the
memory.limit_in_bytes of it to 256M.
Then in this memory cgroup we do bellow test:
 1. mount -t tmpfs tmpfs /mount
 the size of which will be 1G / 2 by default.
  2. write files into this tmpfs
 as the limit of this memory cgroup is 256M while the size of
tmpfs is 512M,
 these files will occupy the while memory in this cgroup and
finally out of memory.


> So, tying the size of a tmpfs mount where it was
> mounted does not make much sense.
>
> Also mount operation which requires CAP_SYS_ADMIN, is usually
> performed by node controller (or job loader) which don't necessarily
> run in the memcg of the actual job.
>
>> Signed-off-by: Yafang Shao 
>> ---
>>  include/linux/memcontrol.h |  1 +
>>  mm/memcontrol.c|  2 +-
>>  mm/shmem.c | 20 +++-
>>  3 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
>> index 69966c4..79c6709 100644
>> --- a/include/linux/memcontrol.h
>> +++ b/include/linux/memcontrol.h
>> @@ -265,6 +265,7 @@ struct mem_cgroup {
>> /* WARNING: nodeinfo must be the last member here */
>>  };
>>
>> +extern struct mutex memcg_limit_mutex;
>>  extern struct mem_cgroup *root_mem_cgroup;
>>
>>  static inline bool mem_cgroup_disabled(void)
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 661f046..ad32f3c 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -2464,7 +2464,7 @@ static inline int 
>> mem_cgroup_move_swap_account(swp_entry_t entry,
>>  }
>>  #endif
>>
>> -static DEFINE_MUTEX(memcg_limit_mutex);
>> +DEFINE_MUTEX(memcg_limit_mutex);
>
> This mutex is only needed for updating the limit.
>

Thanks for explaination :)

>>
>>  static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
>>unsigned long limit)
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index 07a1d22..1c320dd 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -35,6 +35,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  #include  /* for arch/microblaze update_mmu_cache() */
>>
>> @@ -108,7 +109,24 @@ struct shmem_falloc {
>>  #ifdef CONFIG_TMPFS
>>  static unsigned long shmem_default_max_blocks(void)
>>  {
>> -   return totalram_pages / 2;
>> +   unsigned long size;
>> +
>> +#ifdef CONFIG_MEMCG
>> +   struct mem_cgroup *memcg = mem_cgroup_from_task(current);
>> +
>> +   if (memcg == NULL || memcg == root_mem_cgroup)
>> +   size = totalram_pages / 2;
>> +   else {
>> +   mutex_lock(_limit_mutex);
>> +   size = memcg->memory.limit > totalram_pages ?
>> +totalram_pages / 2 : memcg->memory.limit / 
>> 2;
>> +   mutex_unlock(_limit_mutex);
>> +   }
>> +#else
>> +   size = totalram_pages / 2;
>> +#endif
>> +
>> +   return size;
>>  }
>>
>>  static unsigned long shmem_default_max_inodes(void)
>> --
>> 1.8.3.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe cgroups" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] staging: comedi: ni_atmio: Handle return value of pnp_*

[arvindY]

hi,

On Thursday 16 November 2017 07:27 PM, Ian Abbott wrote:

On 16/11/17 04:32, Arvind Yadav wrote:

pnp_irq() and pnp_port_start() can fail here and we must check
its return value.

Signed-off-by: Arvind Yadav 
---
  drivers/staging/comedi/drivers/ni_atmio.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/staging/comedi/drivers/ni_atmio.c 
b/drivers/staging/comedi/drivers/ni_atmio.c

index 2d62a8c..dead159 100644
--- a/drivers/staging/comedi/drivers/ni_atmio.c
+++ b/drivers/staging/comedi/drivers/ni_atmio.c
@@ -308,6 +308,9 @@ static int ni_atmio_attach(struct comedi_device 
*dev,

iobase = pnp_port_start(isapnp_dev, 0);
  irq = pnp_irq(isapnp_dev, 0);
+if (irq == -1 || !iobase)
+return -ENOMEM;
+
  comedi_set_hw_dev(dev, _dev->dev);
  }



Can they fail here?  ni_isapnp_find_board() has already checked they 
are valid.

Yes, you are right. It will not fail.

~arvind

Re: [PATCH] staging: comedi: ni_atmio: Handle return value of pnp_*

[arvindY]

hi,

On Thursday 16 November 2017 07:27 PM, Ian Abbott wrote:

On 16/11/17 04:32, Arvind Yadav wrote:

pnp_irq() and pnp_port_start() can fail here and we must check
its return value.

Signed-off-by: Arvind Yadav 
---
  drivers/staging/comedi/drivers/ni_atmio.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/staging/comedi/drivers/ni_atmio.c 
b/drivers/staging/comedi/drivers/ni_atmio.c

index 2d62a8c..dead159 100644
--- a/drivers/staging/comedi/drivers/ni_atmio.c
+++ b/drivers/staging/comedi/drivers/ni_atmio.c
@@ -308,6 +308,9 @@ static int ni_atmio_attach(struct comedi_device 
*dev,

iobase = pnp_port_start(isapnp_dev, 0);
  irq = pnp_irq(isapnp_dev, 0);
+if (irq == -1 || !iobase)
+return -ENOMEM;
+
  comedi_set_hw_dev(dev, _dev->dev);
  }



Can they fail here?  ni_isapnp_find_board() has already checked they 
are valid.

Yes, you are right. It will not fail.

~arvind

Re: [PATCH V1 1/4] qcom: spmi-wled: Add support for qcom wled driver

[kgunda]

On 2017-11-16 22:25, Bjorn Andersson wrote:

On Thu 16 Nov 04:18 PST 2017, Kiran Gunda wrote:


WLED driver provides the interface to the display driver to
adjust the brightness of the display backlight.



Hi Kiran,

This driver has a lot in common with the already upstream 
pm8941-wled.c,

because it's just a new revision of the same block.

Please extend the existing driver rather than providing a new one
(and yes, renaming the file is okay).

Regards,
Bjorn


Hi Bjorn,

Yes this driver design is similar to pm8941, however the WLED HW block 
has undergone quite a few changes in
analog and digital from PM8941 to PM8998. Few of them include splitting 
one module into wled-ctrl
and wled-sink peripherals, changes in the register offsets and the bit 
interpretation. Hence we
concluded that it was better to have a new driver to support this new 
gen WELD module and decouple
it from the pm8941. Also, going forward this driver will support AMOLED 
AVDD rail (not supported by pm8941)
touching a few more registers/configuration and newer PMICs. So spinning 
off a new driver would make it

cleaner and easier to extend further.

Thanks,
Kiran


--
To unsubscribe from this list: send the line "unsubscribe 
linux-arm-msm" in

the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V1 1/4] qcom: spmi-wled: Add support for qcom wled driver

[kgunda]

On 2017-11-16 22:25, Bjorn Andersson wrote:

On Thu 16 Nov 04:18 PST 2017, Kiran Gunda wrote:


WLED driver provides the interface to the display driver to
adjust the brightness of the display backlight.



Hi Kiran,

This driver has a lot in common with the already upstream 
pm8941-wled.c,

because it's just a new revision of the same block.

Please extend the existing driver rather than providing a new one
(and yes, renaming the file is okay).

Regards,
Bjorn


Hi Bjorn,

Yes this driver design is similar to pm8941, however the WLED HW block 
has undergone quite a few changes in
analog and digital from PM8941 to PM8998. Few of them include splitting 
one module into wled-ctrl
and wled-sink peripherals, changes in the register offsets and the bit 
interpretation. Hence we
concluded that it was better to have a new driver to support this new 
gen WELD module and decouple
it from the pm8941. Also, going forward this driver will support AMOLED 
AVDD rail (not supported by pm8941)
touching a few more registers/configuration and newer PMICs. So spinning 
off a new driver would make it

cleaner and easier to extend further.

Thanks,
Kiran


--
To unsubscribe from this list: send the line "unsubscribe 
linux-arm-msm" in

the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] soc: qcom: Introduce QMI helpers

[Bjorn Andersson]

On Thu 16 Nov 04:11 PST 2017, Srinivas Kandagatla wrote:
> On 15/11/17 20:10, Bjorn Andersson wrote:
[..]
> > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> > index 91b70b170a82..9718f1c41e3d 100644
> > --- a/drivers/soc/qcom/Kconfig
> > +++ b/drivers/soc/qcom/Kconfig
> > @@ -37,6 +37,7 @@ config QCOM_PM
> >   config QCOM_QMI_HELPERS
> > tristate
> > +   depends on ARCH_QCOM
> 
> This bit is confusing!!, first patch added this symbol and this patch added
> the depends. either we move this to first patch or move out the
> QCOM_QMI_HELPERS from first patch and include in this patch
> 

Ohh, that's not right. The depends should be in the same patch.

And we don't really depends on ARCH_QCOM here, but without it Kconfig
doesn't know that make won't enter drivers/soc/qcom so we will end up
with a link error. I'm hoping we can fix this issue, to get some more
compile testing of these things.

> > help
> >   Helper library for handling QMI encoded messages.  QMI encoded
> >   messages are used in communication between the majority of QRTR
> > diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
[..]
> >   obj-$(CONFIG_QCOM_QMI_HELPERS)+= qmi_helpers.o
> >   qmi_helpers-y += qmi_encdec.o
> > +qmi_helpers-y  += qmi_interface.o
> for better reading.. i would suggest..
> qmi_helpers-y += qmi_encdec.o qmi_interface.o
> 

Sounds reasonable.

> 
> >   obj-$(CONFIG_QCOM_SMD_RPM)+= smd-rpm.o
> >   obj-$(CONFIG_QCOM_SMEM) +=smem.o
> >   obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
> > diff --git a/drivers/soc/qcom/qmi_interface.c 
> > b/drivers/soc/qcom/qmi_interface.c
[..]
> > +#include 
> 
> Do we need this?
> 

I don't think so.

[..]
> > +/**
> > + * qmi_recv_new_server() - handler of NEW_SERVER control message
> > + * @qmi:   qmi handle
> > + * @node:  node of the new server
> > + * @port:  port of the new server
> > + *
> service and instance is not documented here.
> 

Thanks for noticing, will fix these.

[..]
> > +/**
> > + * qmi_handle_init() - initialize a QMI client handle
> > + * @qmi:   QMI handle to initialize
> > + * @max_msg_len: maximum size of incoming message
> do we need this??
> 

We need a buffer into which we can receive incoming packets, so either
we allocate a large enough buffer up front or we have to ask qrtr before
each packet how much space we will need.

I think largest possible buffer is 64kB, but typically we need much
less. And the IDL compiler seems to output this constant, so it seems
reasonable to pass it here.

> 
> > + * @handlers:  NULL-terminated list of QMI message handlers
> > + *
> recv_buf_size and ops not documented
> 

Looks like I've renamed max_msg_len to recv_buf_size, and forgot to add
ops. Will fix.

[..]
> > +
> > +/**
> > + * qmi_send_indication() - send an indication QMI message
> > + * @qmi:   QMI client handle
> > + * @sq:destination sockaddr
> > + * @txn:   transaction object to use for the message
> 
> txn is not required here?
> 

No. Indications are fire-and-forget messages, but still need a
transaction id, so it's confusing for the client to have to create a
txn, use it and then cancel it (or to add another txn operation for
this). Therefor I'm hiding the txn handling inside this function.

Will fix the kerneldoc.

[..]
> > diff --git a/include/linux/soc/qcom/qmi.h b/include/linux/soc/qcom/qmi.h
> > index 5df7edfc6bfd..9b4f4fa5bed6 100644
> > --- a/include/linux/soc/qcom/qmi.h
> > +++ b/include/linux/soc/qcom/qmi.h
> 
> Looks like this patch added few things like list, wq and so to this header
> file, should we not add headers for those ??
> 

Will review these.

Thanks for the review!

Regards,
Bjorn

Re: [PATCH v3 2/5] soc: qcom: Introduce QMI helpers

[Bjorn Andersson]

On Thu 16 Nov 04:11 PST 2017, Srinivas Kandagatla wrote:
> On 15/11/17 20:10, Bjorn Andersson wrote:
[..]
> > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> > index 91b70b170a82..9718f1c41e3d 100644
> > --- a/drivers/soc/qcom/Kconfig
> > +++ b/drivers/soc/qcom/Kconfig
> > @@ -37,6 +37,7 @@ config QCOM_PM
> >   config QCOM_QMI_HELPERS
> > tristate
> > +   depends on ARCH_QCOM
> 
> This bit is confusing!!, first patch added this symbol and this patch added
> the depends. either we move this to first patch or move out the
> QCOM_QMI_HELPERS from first patch and include in this patch
> 

Ohh, that's not right. The depends should be in the same patch.

And we don't really depends on ARCH_QCOM here, but without it Kconfig
doesn't know that make won't enter drivers/soc/qcom so we will end up
with a link error. I'm hoping we can fix this issue, to get some more
compile testing of these things.

> > help
> >   Helper library for handling QMI encoded messages.  QMI encoded
> >   messages are used in communication between the majority of QRTR
> > diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
[..]
> >   obj-$(CONFIG_QCOM_QMI_HELPERS)+= qmi_helpers.o
> >   qmi_helpers-y += qmi_encdec.o
> > +qmi_helpers-y  += qmi_interface.o
> for better reading.. i would suggest..
> qmi_helpers-y += qmi_encdec.o qmi_interface.o
> 

Sounds reasonable.

> 
> >   obj-$(CONFIG_QCOM_SMD_RPM)+= smd-rpm.o
> >   obj-$(CONFIG_QCOM_SMEM) +=smem.o
> >   obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
> > diff --git a/drivers/soc/qcom/qmi_interface.c 
> > b/drivers/soc/qcom/qmi_interface.c
[..]
> > +#include 
> 
> Do we need this?
> 

I don't think so.

[..]
> > +/**
> > + * qmi_recv_new_server() - handler of NEW_SERVER control message
> > + * @qmi:   qmi handle
> > + * @node:  node of the new server
> > + * @port:  port of the new server
> > + *
> service and instance is not documented here.
> 

Thanks for noticing, will fix these.

[..]
> > +/**
> > + * qmi_handle_init() - initialize a QMI client handle
> > + * @qmi:   QMI handle to initialize
> > + * @max_msg_len: maximum size of incoming message
> do we need this??
> 

We need a buffer into which we can receive incoming packets, so either
we allocate a large enough buffer up front or we have to ask qrtr before
each packet how much space we will need.

I think largest possible buffer is 64kB, but typically we need much
less. And the IDL compiler seems to output this constant, so it seems
reasonable to pass it here.

> 
> > + * @handlers:  NULL-terminated list of QMI message handlers
> > + *
> recv_buf_size and ops not documented
> 

Looks like I've renamed max_msg_len to recv_buf_size, and forgot to add
ops. Will fix.

[..]
> > +
> > +/**
> > + * qmi_send_indication() - send an indication QMI message
> > + * @qmi:   QMI client handle
> > + * @sq:destination sockaddr
> > + * @txn:   transaction object to use for the message
> 
> txn is not required here?
> 

No. Indications are fire-and-forget messages, but still need a
transaction id, so it's confusing for the client to have to create a
txn, use it and then cancel it (or to add another txn operation for
this). Therefor I'm hiding the txn handling inside this function.

Will fix the kerneldoc.

[..]
> > diff --git a/include/linux/soc/qcom/qmi.h b/include/linux/soc/qcom/qmi.h
> > index 5df7edfc6bfd..9b4f4fa5bed6 100644
> > --- a/include/linux/soc/qcom/qmi.h
> > +++ b/include/linux/soc/qcom/qmi.h
> 
> Looks like this patch added few things like list, wq and so to this header
> file, should we not add headers for those ??
> 

Will review these.

Thanks for the review!

Regards,
Bjorn

Urgent Circuit Board Prototyping Service

[Sammy]

Hi ,
I haven't heard from you for a long time. How are things with you?

Do you have new projects that need PCB?
I really hope that we can have chance to make boards for you and I am sure you 
won't be disappointed.
You may have many suppliers but we are glad to send our quotation for your 
reference.

Thank you in advance and looking forward to your reply.

Best Regards,
Sammy Chang
Shenzhen Plus Circuits Limited
Overseas Sales
Mobile/Whatsapp: +86 13580982656

Urgent Circuit Board Prototyping Service

[Sammy]

Hi ,
I haven't heard from you for a long time. How are things with you?

Do you have new projects that need PCB?
I really hope that we can have chance to make boards for you and I am sure you 
won't be disappointed.
You may have many suppliers but we are glad to send our quotation for your 
reference.

Thank you in advance and looking forward to your reply.

Best Regards,
Sammy Chang
Shenzhen Plus Circuits Limited
Overseas Sales
Mobile/Whatsapp: +86 13580982656

Re: [PATCH v3 1/6] PM / core: Add LEAVE_SUSPENDED driver flag

[Ulf Hansson]

[...]

>> > +++ linux-pm/include/linux/pm.h
>> > @@ -559,6 +559,7 @@ struct pm_subsys_data {
>> >   * NEVER_SKIP: Do not skip system suspend/resume callbacks for the device.
>> >   * SMART_PREPARE: Check the return value of the driver's ->prepare 
>> > callback.
>> >   * SMART_SUSPEND: No need to resume the device from runtime suspend.
>> > + * LEAVE_SUSPENDED: Avoid resuming the device during system resume if 
>> > possible.
>> >   *
>> >   * Setting SMART_PREPARE instructs bus types and PM domains which may want
>> >   * system suspend/resume callbacks to be skipped for the device to return 
>> > 0 from
>> > @@ -572,10 +573,14 @@ struct pm_subsys_data {
>> >   * necessary from the driver's perspective.  It also may cause them to 
>> > skip
>> >   * invocations of the ->suspend_late and ->suspend_noirq callbacks 
>> > provided by
>> >   * the driver if they decide to leave the device in runtime suspend.
>> > + *
>> > + * Setting LEAVE_SUSPENDED informs the PM core and middle-layer code that 
>> > the
>> > + * driver prefers the device to be left in runtime suspend after system 
>> > resume.
>> >   */
>>
>> Question: Can LEAVE_SUSPENDED and NEVER_SKIP be valid combination? I
>> guess not!? Should we validate for wrong combinations?
>
> Why not?  There's no real overlap between them.

Except that NEVER_SKIP, documentation wise, tells you that your
suspend and resume callbacks will never be skipped. :-)

[...]

>> Second, have you considered setting the default value of
>> dev->power.may_skip_resume to true?
>
> Yes.
>
>> That would means the subsystem
>> instead need to implement an opt-out method. I am thinking that it may
>> not be an issue, since we anyway at this point, don't have drivers
>> using the LEAVE_SUSPENDED flag.
>
> Opt-out doesn't work because of the need to invoke the "noirq" callbacks.

I am not sure I follow that.

Whatever needs to be fixed on the subsystem level, that could be done
before the driver starts using the LEAVE_SUSPENDED flag. No?

>
>> [...]
>>
>> > +However, it may be desirable to leave some devices in runtime suspend 
>> > after
>> > +system transitions to the working state and device drivers can use the
>> > +``DPM_FLAG_LEAVE_SUSPENDED`` flag to indicate to the PM core (and 
>> > middle-layer
>> > +code) that this is the case.  Whether or not the devices will actually be 
>> > left
>> > +in suspend may depend on their state before the given system 
>> > suspend-resume
>> > +cycle and on the type of the system transition under way.  In particular,
>> > +devices are not left suspended if that transition is a restore from 
>> > hibernation,
>> > +as device states are not guaranteed to be reflected by the information 
>> > stored in
>> > +the hibernation image in that case.
>> > +
>> > +The middle-layer code involved in the handling of the device has to 
>> > indicate to
>> > +the PM core if the device may be left in suspend with the help of its
>> > +:c:member:`power.may_skip_resume` status bit.  That has to happen in the 
>> > "noirq"
>> > +phase of the preceding system-wide suspend (or analogous) transition.  The
>>
>> Does it have to be managed in the "noirq" phase? Wouldn't be perfectly
>> okay do this in the suspend and suspend_late phases as well?
>
> The wording is slightly misleading I think.
>
> In fact technically may_skip_resume may be set earlier, but the core checks it
> in the "noirq" phase only anyway.

Yeah, okay.

Kind regards
Uffe

Re: [PATCH v3 1/6] PM / core: Add LEAVE_SUSPENDED driver flag

[Ulf Hansson]

[...]

>> > +++ linux-pm/include/linux/pm.h
>> > @@ -559,6 +559,7 @@ struct pm_subsys_data {
>> >   * NEVER_SKIP: Do not skip system suspend/resume callbacks for the device.
>> >   * SMART_PREPARE: Check the return value of the driver's ->prepare 
>> > callback.
>> >   * SMART_SUSPEND: No need to resume the device from runtime suspend.
>> > + * LEAVE_SUSPENDED: Avoid resuming the device during system resume if 
>> > possible.
>> >   *
>> >   * Setting SMART_PREPARE instructs bus types and PM domains which may want
>> >   * system suspend/resume callbacks to be skipped for the device to return 
>> > 0 from
>> > @@ -572,10 +573,14 @@ struct pm_subsys_data {
>> >   * necessary from the driver's perspective.  It also may cause them to 
>> > skip
>> >   * invocations of the ->suspend_late and ->suspend_noirq callbacks 
>> > provided by
>> >   * the driver if they decide to leave the device in runtime suspend.
>> > + *
>> > + * Setting LEAVE_SUSPENDED informs the PM core and middle-layer code that 
>> > the
>> > + * driver prefers the device to be left in runtime suspend after system 
>> > resume.
>> >   */
>>
>> Question: Can LEAVE_SUSPENDED and NEVER_SKIP be valid combination? I
>> guess not!? Should we validate for wrong combinations?
>
> Why not?  There's no real overlap between them.

Except that NEVER_SKIP, documentation wise, tells you that your
suspend and resume callbacks will never be skipped. :-)

[...]

>> Second, have you considered setting the default value of
>> dev->power.may_skip_resume to true?
>
> Yes.
>
>> That would means the subsystem
>> instead need to implement an opt-out method. I am thinking that it may
>> not be an issue, since we anyway at this point, don't have drivers
>> using the LEAVE_SUSPENDED flag.
>
> Opt-out doesn't work because of the need to invoke the "noirq" callbacks.

I am not sure I follow that.

Whatever needs to be fixed on the subsystem level, that could be done
before the driver starts using the LEAVE_SUSPENDED flag. No?

>
>> [...]
>>
>> > +However, it may be desirable to leave some devices in runtime suspend 
>> > after
>> > +system transitions to the working state and device drivers can use the
>> > +``DPM_FLAG_LEAVE_SUSPENDED`` flag to indicate to the PM core (and 
>> > middle-layer
>> > +code) that this is the case.  Whether or not the devices will actually be 
>> > left
>> > +in suspend may depend on their state before the given system 
>> > suspend-resume
>> > +cycle and on the type of the system transition under way.  In particular,
>> > +devices are not left suspended if that transition is a restore from 
>> > hibernation,
>> > +as device states are not guaranteed to be reflected by the information 
>> > stored in
>> > +the hibernation image in that case.
>> > +
>> > +The middle-layer code involved in the handling of the device has to 
>> > indicate to
>> > +the PM core if the device may be left in suspend with the help of its
>> > +:c:member:`power.may_skip_resume` status bit.  That has to happen in the 
>> > "noirq"
>> > +phase of the preceding system-wide suspend (or analogous) transition.  The
>>
>> Does it have to be managed in the "noirq" phase? Wouldn't be perfectly
>> okay do this in the suspend and suspend_late phases as well?
>
> The wording is slightly misleading I think.
>
> In fact technically may_skip_resume may be set earlier, but the core checks it
> in the "noirq" phase only anyway.

Yeah, okay.

Kind regards
Uffe

Re: [PATCH v3 1/5] soc: qcom: Introduce QMI encoder/decoder

[Bjorn Andersson]

On Thu 16 Nov 04:10 PST 2017, Srinivas Kandagatla wrote:
> On 15/11/17 20:10, Bjorn Andersson wrote:
[..]
> > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> > index b00bccddcd3b..91b70b170a82 100644
> > --- a/drivers/soc/qcom/Kconfig
> > +++ b/drivers/soc/qcom/Kconfig
> > @@ -35,6 +35,14 @@ config QCOM_PM
> >   modes. It interface with various system drivers to put the cores in
> >   low power modes.
> > +config QCOM_QMI_HELPERS
> Should'nt this be part of second patch? atleast the patch subject suggests
> that its just enc/decoder but you are actually adding qmi helper symbol ?
> Or change the order of the patch.
> 

The encoder/decoder is, and should be, compilable on its own; so this
config option does ensure that the newly added c-file isn't just dead
code - although it's unlikely to matter in practice.

I don't see a reason for adding a QCOM_QMI_ENCDEC here and then rename
that in the following commit and I don't see a reason to split it in two
config options. So I think this is reasonable...

> > +   tristate
> we added help to this symbol but there is no promt text associated with it,
> so it will not show up as in menuconfig.. may be
> 
> tristate "QMI Helpers"
> 
> would be better
> 

The idea is that users of these functions would "select"
QCOM_QMI_HELPERS.

I don't think there is a particular point in allowing the user to select
this without any of the clients and a "depends on" in the client just
results in the user being forced to find this option to be able to
enable any of the clients.

[..]
> > diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
[..]
> > +#include 
> 
> do we need this?
> 

Nope.

[..]
> > +/**
> > + * skip_to_next_elem() - Skip to next element in the structure to be 
> > encoded
> > + * @ei_array: Struct info describing the element to be skipped.
> > + * @level: Depth level of encoding/decoding to identify nested structures.
> > + *
> > + * Returns struct info of the next element that can be encoded.
> > + *
> 
> Should be Return: according to Documentation/doc-guide/kernel-doc.rst
> 
> Same comment appies to most of the functions..
> 

Thanks, I have been looking for that document! Will update the comments
throughout the patch series.

[..]
> > +/**
> > + * qmi_decode_string_elem() - Decodes elements of string data type
> > + * @ei_array: Struct info array descibing the string element.
> > + * @buf_dst: Buffer to store the decoded element.
> > + * @buf_src: Buffer containing the elements in QMI wire format.
> > + * @tlv_len: Total size of the encoded inforation corresponding to
> > + *   this string element.
> > + * @dec_level: Depth of the string element from the main structure.
> > + *
> > +
> 
> bad Line??

Looks like it.

> > + * Returns the total size of the decoded data elements on success, negative
> > + * errno on error.
> > +
> Dito..
> > + *
> > + * This function decodes the string element of maximum length
> > + * "ei_array->elem_len" from the source buffer "buf_src" and puts it into
> > + * the destination buffer "buf_dst". This function returns number of bytes
> > + * decoded from the input buffer.

And this should go above the "return" description.

[..]
> > +
> > +   rc = qmi_decode_basic_elem(buf_dst, buf_src + decoded_bytes,
> > +  string_len, temp_ei->elem_size);
> > +   *((char *)buf_dst + string_len) = '\0';
> > +   decoded_bytes += rc;
> An empty line here could be nice... same for most of the functions..
> 

I agree, I'll run over the file again.

There are a few other style things that I would like to change, e.g.
replacing u32 with size_t in a bunch of places; but I also didn't want
to change too much from the downstream version.

But fixing the spacing in the initial commit makes sense.

[..]
> > diff --git a/include/linux/soc/qcom/qmi.h b/include/linux/soc/qcom/qmi.h
[..]
> > +#include 
> 
> Is this include needed as part of this patch?
> 

This should be able to go in the next patch.

> > +#include 
> > +
> 
> > +
> > +enum qmi_array_type {
> > +   NO_ARRAY,
> > +   STATIC_ARRAY,
> > +   VAR_LEN_ARRAY,
> > +};
> > +
> > +/**
> > + * struct qmi_elem_info - describes how to encode a single QMI element
> > + * @data_type: Data type of this element.
> > + * @elem_len:  Array length of this element, if an array.
> > + * @elem_size: Size of a single instance of this data type.
> > + * @is_array:  Array type of this element.
> > + * @tlv_type:  QMI message specific type to identify which element
> > + * is present in an incoming message.
> > + * @offset:Specifies the offset of the first instance of this
> > + * element in the data structure.
> > + * @ei_array:  Null-terminated array of @qmi_elem_info to describe 
> > nested
> > + * structures.
> > + */
> > +struct qmi_elem_info {
> > +   enum qmi_elem_type data_type;
> > +   u32 elem_len;
> > +   u32 elem_size;
> > +   enum qmi_array_type is_array;
> 
> naming 

Re: [PATCH v3 1/5] soc: qcom: Introduce QMI encoder/decoder

[Bjorn Andersson]

On Thu 16 Nov 04:10 PST 2017, Srinivas Kandagatla wrote:
> On 15/11/17 20:10, Bjorn Andersson wrote:
[..]
> > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> > index b00bccddcd3b..91b70b170a82 100644
> > --- a/drivers/soc/qcom/Kconfig
> > +++ b/drivers/soc/qcom/Kconfig
> > @@ -35,6 +35,14 @@ config QCOM_PM
> >   modes. It interface with various system drivers to put the cores in
> >   low power modes.
> > +config QCOM_QMI_HELPERS
> Should'nt this be part of second patch? atleast the patch subject suggests
> that its just enc/decoder but you are actually adding qmi helper symbol ?
> Or change the order of the patch.
> 

The encoder/decoder is, and should be, compilable on its own; so this
config option does ensure that the newly added c-file isn't just dead
code - although it's unlikely to matter in practice.

I don't see a reason for adding a QCOM_QMI_ENCDEC here and then rename
that in the following commit and I don't see a reason to split it in two
config options. So I think this is reasonable...

> > +   tristate
> we added help to this symbol but there is no promt text associated with it,
> so it will not show up as in menuconfig.. may be
> 
> tristate "QMI Helpers"
> 
> would be better
> 

The idea is that users of these functions would "select"
QCOM_QMI_HELPERS.

I don't think there is a particular point in allowing the user to select
this without any of the clients and a "depends on" in the client just
results in the user being forced to find this option to be able to
enable any of the clients.

[..]
> > diff --git a/drivers/soc/qcom/qmi_encdec.c b/drivers/soc/qcom/qmi_encdec.c
[..]
> > +#include 
> 
> do we need this?
> 

Nope.

[..]
> > +/**
> > + * skip_to_next_elem() - Skip to next element in the structure to be 
> > encoded
> > + * @ei_array: Struct info describing the element to be skipped.
> > + * @level: Depth level of encoding/decoding to identify nested structures.
> > + *
> > + * Returns struct info of the next element that can be encoded.
> > + *
> 
> Should be Return: according to Documentation/doc-guide/kernel-doc.rst
> 
> Same comment appies to most of the functions..
> 

Thanks, I have been looking for that document! Will update the comments
throughout the patch series.

[..]
> > +/**
> > + * qmi_decode_string_elem() - Decodes elements of string data type
> > + * @ei_array: Struct info array descibing the string element.
> > + * @buf_dst: Buffer to store the decoded element.
> > + * @buf_src: Buffer containing the elements in QMI wire format.
> > + * @tlv_len: Total size of the encoded inforation corresponding to
> > + *   this string element.
> > + * @dec_level: Depth of the string element from the main structure.
> > + *
> > +
> 
> bad Line??

Looks like it.

> > + * Returns the total size of the decoded data elements on success, negative
> > + * errno on error.
> > +
> Dito..
> > + *
> > + * This function decodes the string element of maximum length
> > + * "ei_array->elem_len" from the source buffer "buf_src" and puts it into
> > + * the destination buffer "buf_dst". This function returns number of bytes
> > + * decoded from the input buffer.

And this should go above the "return" description.

[..]
> > +
> > +   rc = qmi_decode_basic_elem(buf_dst, buf_src + decoded_bytes,
> > +  string_len, temp_ei->elem_size);
> > +   *((char *)buf_dst + string_len) = '\0';
> > +   decoded_bytes += rc;
> An empty line here could be nice... same for most of the functions..
> 

I agree, I'll run over the file again.

There are a few other style things that I would like to change, e.g.
replacing u32 with size_t in a bunch of places; but I also didn't want
to change too much from the downstream version.

But fixing the spacing in the initial commit makes sense.

[..]
> > diff --git a/include/linux/soc/qcom/qmi.h b/include/linux/soc/qcom/qmi.h
[..]
> > +#include 
> 
> Is this include needed as part of this patch?
> 

This should be able to go in the next patch.

> > +#include 
> > +
> 
> > +
> > +enum qmi_array_type {
> > +   NO_ARRAY,
> > +   STATIC_ARRAY,
> > +   VAR_LEN_ARRAY,
> > +};
> > +
> > +/**
> > + * struct qmi_elem_info - describes how to encode a single QMI element
> > + * @data_type: Data type of this element.
> > + * @elem_len:  Array length of this element, if an array.
> > + * @elem_size: Size of a single instance of this data type.
> > + * @is_array:  Array type of this element.
> > + * @tlv_type:  QMI message specific type to identify which element
> > + * is present in an incoming message.
> > + * @offset:Specifies the offset of the first instance of this
> > + * element in the data structure.
> > + * @ei_array:  Null-terminated array of @qmi_elem_info to describe 
> > nested
> > + * structures.
> > + */
> > +struct qmi_elem_info {
> > +   enum qmi_elem_type data_type;
> > +   u32 elem_len;
> > +   u32 elem_size;
> > +   enum qmi_array_type is_array;
> 
> naming 

Re: [PATCH] Bluetooth: hci_serdev: Init hci_uart proto_lock to avoid oops

[Marcel Holtmann]

Hi Lukas,

> John Stultz reports a boot time crash with the HiKey board (which uses
> hci_serdev) occurring in hci_uart_tx_wakeup().  That function is
> contained in hci_ldisc.c, but also called from the newer hci_serdev.c.
> It acquires the proto_lock in struct hci_uart and it turns out that we
> forgot to init the lock in the serdev code path, thus causing the crash.
> 
> John bisected the crash to commit 67d2f8781b9f ("Bluetooth: hci_ldisc:
> Allow sleeping while proto locks are held"), but the issue was present
> before and the commit merely exposed it.  (Perhaps by luck, the crash
> did not occur with rwlocks.)
> 
> Init the proto_lock in the serdev code path to avoid the oops.
> 
> Stack trace for posterity:
> 
> Unable to handle kernel read from unreadable memory at 406f127000
> [00406f127000] user address but active_mm is swapper
> Internal error: Oops: 9605 [#1] PREEMPT SMP
> Hardware name: HiKey Development Board (DT)
> Call trace:
> hci_uart_tx_wakeup+0x38/0x148
> hci_uart_send_frame+0x28/0x38
> hci_send_frame+0x64/0xc0
> hci_cmd_work+0x98/0x110
> process_one_work+0x134/0x330
> worker_thread+0x130/0x468
> kthread+0xf8/0x128
> ret_from_fork+0x10/0x18
> 
> Link: https://lkml.org/lkml/2017/11/15/908
> Reported-and-tested-by: John Stultz 
> Cc: Ronald Tschalär 
> Cc: Rob Herring 
> Cc: Sumit Semwal 
> Signed-off-by: Lukas Wunner 
> ---
> @Rob (and everyone else):  I'm not sure if this is in fact the correct
> approach, or if we should instead duplicate hci_uart_tx_wakeup() in
> hci_serdev.c (sans locking?), much as we've duplicated a lot of other
> functions there.  Let me know what your preference is.  Thanks!
> 
> drivers/bluetooth/hci_serdev.c | 1 +
> 1 file changed, 1 insertion(+)

patch has been applied to bluetooth-next tree.

Regards

Marcel

Re: [PATCH] Bluetooth: hci_serdev: Init hci_uart proto_lock to avoid oops

[Marcel Holtmann]

Hi Lukas,

> John Stultz reports a boot time crash with the HiKey board (which uses
> hci_serdev) occurring in hci_uart_tx_wakeup().  That function is
> contained in hci_ldisc.c, but also called from the newer hci_serdev.c.
> It acquires the proto_lock in struct hci_uart and it turns out that we
> forgot to init the lock in the serdev code path, thus causing the crash.
> 
> John bisected the crash to commit 67d2f8781b9f ("Bluetooth: hci_ldisc:
> Allow sleeping while proto locks are held"), but the issue was present
> before and the commit merely exposed it.  (Perhaps by luck, the crash
> did not occur with rwlocks.)
> 
> Init the proto_lock in the serdev code path to avoid the oops.
> 
> Stack trace for posterity:
> 
> Unable to handle kernel read from unreadable memory at 406f127000
> [00406f127000] user address but active_mm is swapper
> Internal error: Oops: 9605 [#1] PREEMPT SMP
> Hardware name: HiKey Development Board (DT)
> Call trace:
> hci_uart_tx_wakeup+0x38/0x148
> hci_uart_send_frame+0x28/0x38
> hci_send_frame+0x64/0xc0
> hci_cmd_work+0x98/0x110
> process_one_work+0x134/0x330
> worker_thread+0x130/0x468
> kthread+0xf8/0x128
> ret_from_fork+0x10/0x18
> 
> Link: https://lkml.org/lkml/2017/11/15/908
> Reported-and-tested-by: John Stultz 
> Cc: Ronald Tschalär 
> Cc: Rob Herring 
> Cc: Sumit Semwal 
> Signed-off-by: Lukas Wunner 
> ---
> @Rob (and everyone else):  I'm not sure if this is in fact the correct
> approach, or if we should instead duplicate hci_uart_tx_wakeup() in
> hci_serdev.c (sans locking?), much as we've duplicated a lot of other
> functions there.  Let me know what your preference is.  Thanks!
> 
> drivers/bluetooth/hci_serdev.c | 1 +
> 1 file changed, 1 insertion(+)

patch has been applied to bluetooth-next tree.

Regards

Marcel

Re: [PATCH v3 4/5] remoteproc: qcom: Introduce sysmon

[Bjorn Andersson]

On Thu 16 Nov 12:05 PST 2017, Chris Lew wrote:
> > +   req.event = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN;
> 
> Are there plans to add the other SSR events to sysmon notifiers? I think the
> SSCTL service expects to receive events about remote procs starting as well.
> 

We could easily add support here to send out the AFTER_BOOTUP
notification, beyond that we would need to extend the subdev notifiers
as well.

But the downstream code paths does confuse me, can you confirm that
these messages are actually sent to the remote ssctl services?

Regards,
Bjorn

Re: [PATCH v3 4/5] remoteproc: qcom: Introduce sysmon

[Bjorn Andersson]

On Thu 16 Nov 12:05 PST 2017, Chris Lew wrote:
> > +   req.event = SSCTL_SSR_EVENT_BEFORE_SHUTDOWN;
> 
> Are there plans to add the other SSR events to sysmon notifiers? I think the
> SSCTL service expects to receive events about remote procs starting as well.
> 

We could easily add support here to send out the AFTER_BOOTUP
notification, beyond that we would need to extend the subdev notifiers
as well.

But the downstream code paths does confuse me, can you confirm that
these messages are actually sent to the remote ssctl services?

Regards,
Bjorn

RE: [PATCH 4.4 06/56] dt-bindings: clockgen: Add compatible string for LS1012A

[Harninder Rai]

> 
> No, not at all, if you want a patch in a specific stable tree, just email the 
> git
> commit id to sta...@vger.kernel.org and let me know what
> tree(s) it should be applied to.

You can drop this patch. Thanks Greg
> 
> thanks,
> 
> greg k-h

RE: [PATCH 4.4 06/56] dt-bindings: clockgen: Add compatible string for LS1012A

[Harninder Rai]

> 
> No, not at all, if you want a patch in a specific stable tree, just email the 
> git
> commit id to sta...@vger.kernel.org and let me know what
> tree(s) it should be applied to.

You can drop this patch. Thanks Greg
> 
> thanks,
> 
> greg k-h

Re: [RFC v5 03/11] [media] vb2: add 'ordered_in_driver' property to queues

[Alexandre Courbot]

On Thursday, November 16, 2017 2:10:49 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

We use ordered_in_driver property to optimize for the case where
the driver can deliver the buffers in an ordered fashion. When it
is ordered we can use the same fence context for all fences, but
when it is not we need to a new context for each out-fence.


"we need to a new context" looks like it is missing a word.

Re: [RFC v5 03/11] [media] vb2: add 'ordered_in_driver' property to queues

[Alexandre Courbot]

On Thursday, November 16, 2017 2:10:49 AM JST, Gustavo Padovan wrote:

From: Gustavo Padovan 

We use ordered_in_driver property to optimize for the case where
the driver can deliver the buffers in an ordered fashion. When it
is ordered we can use the same fence context for all fences, but
when it is not we need to a new context for each out-fence.


"we need to a new context" looks like it is missing a word.

Re: [PATCH] f2fs: let f2fs also gc atomic file to avoid loop gc

[Chao Yu]

On 2017/11/17 11:30, Yunlong Song wrote:
> How about add file_write_and_wait_range in __write_node_page as following:
> if (atomic && !test_opt(sbi, NOBARRIER)) {
>  file_write_and_wait_range(file, 0, LLONG_MAX);

Nope, GCed encrypted data wouldn't be cached in inode page cache.

I don't think adding raw code to flush data here is a good implementation,
it incurs complicated lock dependency relation. Instead, IMO, it will be
better to use inmem_lock to avoid race in between GC and atomic commit flow.

Thanks,

>  fio.op_flags |= REQ_PREFLUSH | REQ_FUA;
> }
> 
> The all the GCed data will be flushed to non-volatile before last node 
> write with REQ_PREFLUSH | REQ_FUA.
> 
> On 2017/11/17 11:20, Chao Yu wrote:
>> On 2017/11/17 11:04, Yunlong Song wrote:
>>> The atomic commit will trigger:
>>>   -f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true)
>>>   -file_write_and_wait_range(file, 0, LLONG_MAX)
>>>   -fsync_node_pages
>>>   -__write_node_page
>>>   -REQ_PREFLUSH | REQ_FUA
>>>
>>> So data is flushed to non-volatile before  last node write with > 
>>> REQ_PREFLUSH | REQ_FUA,
>> I mean GCed data.
>>
>> - file_write_and_wait_range
>>  - move_data_block
>>   - f2fs_submit_page_write
>>- f2fs_update_data_blkaddr
>> - set_page_dirty
>>   - fsync_node_pages
>>
>> Thanks,
>>
>>> we do not need to worry about the inconsistent problem. Right?
>>>
>>> On 2017/11/17 10:49, Chao Yu wrote:
 On 2017/11/17 8:58, Yunlong Song wrote:
> Is there any problem if just deleting the judgement condition in this 
> patch?
 IIRC, dirty node comes from data segment GC can be writebacked & flushed 
 during
 atomic commit, but related data will still be in inner bio cache, after 
 later
 SPOR, data would be inconsistent.

 Thanks,

> On 2017/11/8 17:28, Chao Yu wrote:
>> On 2017/11/8 10:34, Yunlong Song wrote:
>>> If some files are opened with atomic flag and have not commited yet, at
>>> the same time, if all the target victim segments have at least one page
>>> of these atomic files, then f2fs gc will fail to do gc and hangs in the
>>> process of go to gc_more, since gc_date_segment will not move any data
>>> and get_valid_blocks will never be 0, then do_garbage_collect will
>>> always return 0.
>> Oh, I added this judgment condition to avoid ruining atomic write by data
>> GC, could we find another way to solve this issue? BTW, if there is 
>> direct
>> IO, we will also skip data segment GC.
>>
>> Thanks,
>>
>>> Signed-off-by: Yunlong Song 
>>> ---
>>> fs/f2fs/gc.c | 6 --
>>> 1 file changed, 6 deletions(-)
>>>
>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>> index 5d5bba4..3fdcd04 100644
>>> --- a/fs/f2fs/gc.c
>>> +++ b/fs/f2fs/gc.c
>>> @@ -621,9 +621,6 @@ static void move_data_block(struct inode *inode, 
>>> block_t bidx,
>>> if (!check_valid_map(F2FS_I_SB(inode), segno, off))
>>> goto out;
>>> 
>>> -   if (f2fs_is_atomic_file(inode))
>>> -   goto out;
>>> -
>>> set_new_dnode(, inode, NULL, NULL, 0);
>>> err = get_dnode_of_data(, bidx, LOOKUP_NODE);
>>> if (err)
>>> @@ -718,9 +715,6 @@ static void move_data_page(struct inode *inode, 
>>> block_t bidx, int gc_type,
>>> if (!check_valid_map(F2FS_I_SB(inode), segno, off))
>>> goto out;
>>> 
>>> -   if (f2fs_is_atomic_file(inode))
>>> -   goto out;
>>> -
>>> if (gc_type == BG_GC) {
>>> if (PageWriteback(page))
>>> goto out;
>>>
>> .
>>
 .

>>
>> .
>>
> 

Re: [PATCH] f2fs: let f2fs also gc atomic file to avoid loop gc

[Chao Yu]

On 2017/11/17 11:30, Yunlong Song wrote:
> How about add file_write_and_wait_range in __write_node_page as following:
> if (atomic && !test_opt(sbi, NOBARRIER)) {
>  file_write_and_wait_range(file, 0, LLONG_MAX);

Nope, GCed encrypted data wouldn't be cached in inode page cache.

I don't think adding raw code to flush data here is a good implementation,
it incurs complicated lock dependency relation. Instead, IMO, it will be
better to use inmem_lock to avoid race in between GC and atomic commit flow.

Thanks,

>  fio.op_flags |= REQ_PREFLUSH | REQ_FUA;
> }
> 
> The all the GCed data will be flushed to non-volatile before last node 
> write with REQ_PREFLUSH | REQ_FUA.
> 
> On 2017/11/17 11:20, Chao Yu wrote:
>> On 2017/11/17 11:04, Yunlong Song wrote:
>>> The atomic commit will trigger:
>>>   -f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true)
>>>   -file_write_and_wait_range(file, 0, LLONG_MAX)
>>>   -fsync_node_pages
>>>   -__write_node_page
>>>   -REQ_PREFLUSH | REQ_FUA
>>>
>>> So data is flushed to non-volatile before  last node write with > 
>>> REQ_PREFLUSH | REQ_FUA,
>> I mean GCed data.
>>
>> - file_write_and_wait_range
>>  - move_data_block
>>   - f2fs_submit_page_write
>>- f2fs_update_data_blkaddr
>> - set_page_dirty
>>   - fsync_node_pages
>>
>> Thanks,
>>
>>> we do not need to worry about the inconsistent problem. Right?
>>>
>>> On 2017/11/17 10:49, Chao Yu wrote:
 On 2017/11/17 8:58, Yunlong Song wrote:
> Is there any problem if just deleting the judgement condition in this 
> patch?
 IIRC, dirty node comes from data segment GC can be writebacked & flushed 
 during
 atomic commit, but related data will still be in inner bio cache, after 
 later
 SPOR, data would be inconsistent.

 Thanks,

> On 2017/11/8 17:28, Chao Yu wrote:
>> On 2017/11/8 10:34, Yunlong Song wrote:
>>> If some files are opened with atomic flag and have not commited yet, at
>>> the same time, if all the target victim segments have at least one page
>>> of these atomic files, then f2fs gc will fail to do gc and hangs in the
>>> process of go to gc_more, since gc_date_segment will not move any data
>>> and get_valid_blocks will never be 0, then do_garbage_collect will
>>> always return 0.
>> Oh, I added this judgment condition to avoid ruining atomic write by data
>> GC, could we find another way to solve this issue? BTW, if there is 
>> direct
>> IO, we will also skip data segment GC.
>>
>> Thanks,
>>
>>> Signed-off-by: Yunlong Song 
>>> ---
>>> fs/f2fs/gc.c | 6 --
>>> 1 file changed, 6 deletions(-)
>>>
>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>> index 5d5bba4..3fdcd04 100644
>>> --- a/fs/f2fs/gc.c
>>> +++ b/fs/f2fs/gc.c
>>> @@ -621,9 +621,6 @@ static void move_data_block(struct inode *inode, 
>>> block_t bidx,
>>> if (!check_valid_map(F2FS_I_SB(inode), segno, off))
>>> goto out;
>>> 
>>> -   if (f2fs_is_atomic_file(inode))
>>> -   goto out;
>>> -
>>> set_new_dnode(, inode, NULL, NULL, 0);
>>> err = get_dnode_of_data(, bidx, LOOKUP_NODE);
>>> if (err)
>>> @@ -718,9 +715,6 @@ static void move_data_page(struct inode *inode, 
>>> block_t bidx, int gc_type,
>>> if (!check_valid_map(F2FS_I_SB(inode), segno, off))
>>> goto out;
>>> 
>>> -   if (f2fs_is_atomic_file(inode))
>>> -   goto out;
>>> -
>>> if (gc_type == BG_GC) {
>>> if (PageWriteback(page))
>>> goto out;
>>>
>> .
>>
 .

>>
>> .
>>
> 

Re: [PATCH 10/13] x86/alternative: Support indirect call replacement

[Juergen Gross]

On 16/11/17 22:19, Josh Poimboeuf wrote:
> On Wed, Oct 25, 2017 at 01:25:02PM +0200, Juergen Gross wrote:
>> On 04/10/17 17:58, Josh Poimboeuf wrote:
>>> Add alternative patching support for replacing an instruction with an
>>> indirect call.  This will be needed for the paravirt alternatives.
>>>
>>> Signed-off-by: Josh Poimboeuf 
>>> ---
>>>  arch/x86/kernel/alternative.c | 22 +++---
>>>  1 file changed, 15 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
>>> index 3344d3382e91..81c577c7deba 100644
>>> --- a/arch/x86/kernel/alternative.c
>>> +++ b/arch/x86/kernel/alternative.c
>>> @@ -410,20 +410,28 @@ void __init_or_module noinline 
>>> apply_alternatives(struct alt_instr *start,
>>> insnbuf_sz = a->replacementlen;
>>>  
>>> /*
>>> -* 0xe8 is a relative jump; fix the offset.
>>> -*
>>> -* Instruction length is checked before the opcode to avoid
>>> -* accessing uninitialized bytes for zero-length replacements.
>>> +* Fix the address offsets for call and jump instructions which
>>> +* use PC-relative addressing.
>>>  */
>>> if (a->replacementlen == 5 && *insnbuf == 0xe8) {
>>> +   /* direct call */
>>> *(s32 *)(insnbuf + 1) += replacement - instr;
>>> -   DPRINTK("Fix CALL offset: 0x%x, CALL 0x%lx",
>>> +   DPRINTK("Fix direct CALL offset: 0x%x, CALL 0x%lx",
>>> *(s32 *)(insnbuf + 1),
>>> (unsigned long)instr + *(s32 *)(insnbuf + 1) + 
>>> 5);
>>> -   }
>>>  
>>> -   if (a->replacementlen && is_jmp(replacement[0]))
>>> +   } else if (a->replacementlen == 6 && *insnbuf == 0xff &&
>>> +  *(insnbuf+1) == 0x15) {
>>> +   /* indirect call */
>>> +   *(s32 *)(insnbuf + 2) += replacement - instr;
>>> +   DPRINTK("Fix indirect CALL offset: 0x%x, CALL *0x%lx",
>>> +   *(s32 *)(insnbuf + 2),
>>> +   (unsigned long)instr + *(s32 *)(insnbuf + 2) + 
>>> 6);
>>> +
>>> +   } else if (a->replacementlen && is_jmp(replacement[0])) {
>>
>> Is this correct? Without your patch this was:
>>
>> if (*insnbuf == 0xe8) ...
>> if (is_jmp(replacement[0])) ...
>>
>> Now you have:
>>
>> if (*insnbuf == 0xe8) ...
>> else if (*insnbuf == 0xff15) ...
>> else if (is_jmp(replacement[0])) ...
>>
>> So only one or none of the three variants will be executed. In the past
>> it could be none, one or both.
> 
> It can't be a call *and* a jump.  It's either one or the other.
> 
> Maybe it's a little confusing that the jump check uses replacement[0]
> while the other checks use *insnbuf?  They have the same value, so the

Right, I was fooled by that.

> same variable should probably be used everywhere for consistency.  I can
> make them more consistent.
> 

I'd appreciate that. :-)


Juergen

Re: [PATCH 10/13] x86/alternative: Support indirect call replacement

[Juergen Gross]

On 16/11/17 22:19, Josh Poimboeuf wrote:
> On Wed, Oct 25, 2017 at 01:25:02PM +0200, Juergen Gross wrote:
>> On 04/10/17 17:58, Josh Poimboeuf wrote:
>>> Add alternative patching support for replacing an instruction with an
>>> indirect call.  This will be needed for the paravirt alternatives.
>>>
>>> Signed-off-by: Josh Poimboeuf 
>>> ---
>>>  arch/x86/kernel/alternative.c | 22 +++---
>>>  1 file changed, 15 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
>>> index 3344d3382e91..81c577c7deba 100644
>>> --- a/arch/x86/kernel/alternative.c
>>> +++ b/arch/x86/kernel/alternative.c
>>> @@ -410,20 +410,28 @@ void __init_or_module noinline 
>>> apply_alternatives(struct alt_instr *start,
>>> insnbuf_sz = a->replacementlen;
>>>  
>>> /*
>>> -* 0xe8 is a relative jump; fix the offset.
>>> -*
>>> -* Instruction length is checked before the opcode to avoid
>>> -* accessing uninitialized bytes for zero-length replacements.
>>> +* Fix the address offsets for call and jump instructions which
>>> +* use PC-relative addressing.
>>>  */
>>> if (a->replacementlen == 5 && *insnbuf == 0xe8) {
>>> +   /* direct call */
>>> *(s32 *)(insnbuf + 1) += replacement - instr;
>>> -   DPRINTK("Fix CALL offset: 0x%x, CALL 0x%lx",
>>> +   DPRINTK("Fix direct CALL offset: 0x%x, CALL 0x%lx",
>>> *(s32 *)(insnbuf + 1),
>>> (unsigned long)instr + *(s32 *)(insnbuf + 1) + 
>>> 5);
>>> -   }
>>>  
>>> -   if (a->replacementlen && is_jmp(replacement[0]))
>>> +   } else if (a->replacementlen == 6 && *insnbuf == 0xff &&
>>> +  *(insnbuf+1) == 0x15) {
>>> +   /* indirect call */
>>> +   *(s32 *)(insnbuf + 2) += replacement - instr;
>>> +   DPRINTK("Fix indirect CALL offset: 0x%x, CALL *0x%lx",
>>> +   *(s32 *)(insnbuf + 2),
>>> +   (unsigned long)instr + *(s32 *)(insnbuf + 2) + 
>>> 6);
>>> +
>>> +   } else if (a->replacementlen && is_jmp(replacement[0])) {
>>
>> Is this correct? Without your patch this was:
>>
>> if (*insnbuf == 0xe8) ...
>> if (is_jmp(replacement[0])) ...
>>
>> Now you have:
>>
>> if (*insnbuf == 0xe8) ...
>> else if (*insnbuf == 0xff15) ...
>> else if (is_jmp(replacement[0])) ...
>>
>> So only one or none of the three variants will be executed. In the past
>> it could be none, one or both.
> 
> It can't be a call *and* a jump.  It's either one or the other.
> 
> Maybe it's a little confusing that the jump check uses replacement[0]
> while the other checks use *insnbuf?  They have the same value, so the

Right, I was fooled by that.

> same variable should probably be used everywhere for consistency.  I can
> make them more consistent.
> 

I'd appreciate that. :-)


Juergen

[PATCH 1/4] ARM: dts: uniphier: use macros in dt-bindings header

[Masahiro Yamada]

The dt-bindings header was applied to the driver subsystem.  I had to
wait for a merge window to use it from DT.

Signed-off-by: Masahiro Yamada 
---

 arch/arm/boot/dts/uniphier-ld4-ref.dts  | 2 +-
 arch/arm/boot/dts/uniphier-ld4.dtsi | 2 ++
 arch/arm/boot/dts/uniphier-ld6b-ref.dts | 2 +-
 arch/arm/boot/dts/uniphier-pro4-ref.dts | 2 +-
 arch/arm/boot/dts/uniphier-pro4.dtsi| 2 ++
 arch/arm/boot/dts/uniphier-pxs2.dtsi| 1 +
 arch/arm/boot/dts/uniphier-sld8-ref.dts | 2 +-
 arch/arm/boot/dts/uniphier-sld8.dtsi| 2 ++
 8 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/uniphier-ld4-ref.dts 
b/arch/arm/boot/dts/uniphier-ld4-ref.dts
index 0056852..a3afd0c 100644
--- a/arch/arm/boot/dts/uniphier-ld4-ref.dts
+++ b/arch/arm/boot/dts/uniphier-ld4-ref.dts
@@ -56,7 +56,7 @@
  {
xirq1 {
gpio-hog;
-   gpios = <121 0>;
+   gpios = ;
input;
};
 };
diff --git a/arch/arm/boot/dts/uniphier-ld4.dtsi 
b/arch/arm/boot/dts/uniphier-ld4.dtsi
index 01fc3e1..1b98778 100644
--- a/arch/arm/boot/dts/uniphier-ld4.dtsi
+++ b/arch/arm/boot/dts/uniphier-ld4.dtsi
@@ -7,6 +7,8 @@
  * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
  */
 
+#include 
+
 / {
compatible = "socionext,uniphier-ld4";
#address-cells = <1>;
diff --git a/arch/arm/boot/dts/uniphier-ld6b-ref.dts 
b/arch/arm/boot/dts/uniphier-ld6b-ref.dts
index 0e510a7..811b999 100644
--- a/arch/arm/boot/dts/uniphier-ld6b-ref.dts
+++ b/arch/arm/boot/dts/uniphier-ld6b-ref.dts
@@ -58,7 +58,7 @@
  {
xirq4 {
gpio-hog;
-   gpios = <124 0>;
+   gpios = ;
input;
};
 };
diff --git a/arch/arm/boot/dts/uniphier-pro4-ref.dts 
b/arch/arm/boot/dts/uniphier-pro4-ref.dts
index be99467..6a004e5 100644
--- a/arch/arm/boot/dts/uniphier-pro4-ref.dts
+++ b/arch/arm/boot/dts/uniphier-pro4-ref.dts
@@ -58,7 +58,7 @@
  {
xirq2 {
gpio-hog;
-   gpios = <122 0>;
+   gpios = ;
input;
};
 };
diff --git a/arch/arm/boot/dts/uniphier-pro4.dtsi 
b/arch/arm/boot/dts/uniphier-pro4.dtsi
index 7955c3a..b682a42 100644
--- a/arch/arm/boot/dts/uniphier-pro4.dtsi
+++ b/arch/arm/boot/dts/uniphier-pro4.dtsi
@@ -7,6 +7,8 @@
  * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
  */
 
+#include 
+
 / {
compatible = "socionext,uniphier-pro4";
#address-cells = <1>;
diff --git a/arch/arm/boot/dts/uniphier-pxs2.dtsi 
b/arch/arm/boot/dts/uniphier-pxs2.dtsi
index d82d6d8..eafe4dd 100644
--- a/arch/arm/boot/dts/uniphier-pxs2.dtsi
+++ b/arch/arm/boot/dts/uniphier-pxs2.dtsi
@@ -7,6 +7,7 @@
  * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
  */
 
+#include 
 #include 
 
 / {
diff --git a/arch/arm/boot/dts/uniphier-sld8-ref.dts 
b/arch/arm/boot/dts/uniphier-sld8-ref.dts
index 1c0e707..e052ea3 100644
--- a/arch/arm/boot/dts/uniphier-sld8-ref.dts
+++ b/arch/arm/boot/dts/uniphier-sld8-ref.dts
@@ -56,7 +56,7 @@
  {
xirq0 {
gpio-hog;
-   gpios = <120 0>;
+   gpios = ;
input;
};
 };
diff --git a/arch/arm/boot/dts/uniphier-sld8.dtsi 
b/arch/arm/boot/dts/uniphier-sld8.dtsi
index 7188536..89c01cc 100644
--- a/arch/arm/boot/dts/uniphier-sld8.dtsi
+++ b/arch/arm/boot/dts/uniphier-sld8.dtsi
@@ -7,6 +7,8 @@
  * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
  */
 
+#include 
+
 / {
compatible = "socionext,uniphier-sld8";
#address-cells = <1>;
-- 
2.7.4

[PATCH 1/4] ARM: dts: uniphier: use macros in dt-bindings header

[Masahiro Yamada]

The dt-bindings header was applied to the driver subsystem.  I had to
wait for a merge window to use it from DT.

Signed-off-by: Masahiro Yamada 
---

 arch/arm/boot/dts/uniphier-ld4-ref.dts  | 2 +-
 arch/arm/boot/dts/uniphier-ld4.dtsi | 2 ++
 arch/arm/boot/dts/uniphier-ld6b-ref.dts | 2 +-
 arch/arm/boot/dts/uniphier-pro4-ref.dts | 2 +-
 arch/arm/boot/dts/uniphier-pro4.dtsi| 2 ++
 arch/arm/boot/dts/uniphier-pxs2.dtsi| 1 +
 arch/arm/boot/dts/uniphier-sld8-ref.dts | 2 +-
 arch/arm/boot/dts/uniphier-sld8.dtsi| 2 ++
 8 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/uniphier-ld4-ref.dts 
b/arch/arm/boot/dts/uniphier-ld4-ref.dts
index 0056852..a3afd0c 100644
--- a/arch/arm/boot/dts/uniphier-ld4-ref.dts
+++ b/arch/arm/boot/dts/uniphier-ld4-ref.dts
@@ -56,7 +56,7 @@
  {
xirq1 {
gpio-hog;
-   gpios = <121 0>;
+   gpios = ;
input;
};
 };
diff --git a/arch/arm/boot/dts/uniphier-ld4.dtsi 
b/arch/arm/boot/dts/uniphier-ld4.dtsi
index 01fc3e1..1b98778 100644
--- a/arch/arm/boot/dts/uniphier-ld4.dtsi
+++ b/arch/arm/boot/dts/uniphier-ld4.dtsi
@@ -7,6 +7,8 @@
  * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
  */
 
+#include 
+
 / {
compatible = "socionext,uniphier-ld4";
#address-cells = <1>;
diff --git a/arch/arm/boot/dts/uniphier-ld6b-ref.dts 
b/arch/arm/boot/dts/uniphier-ld6b-ref.dts
index 0e510a7..811b999 100644
--- a/arch/arm/boot/dts/uniphier-ld6b-ref.dts
+++ b/arch/arm/boot/dts/uniphier-ld6b-ref.dts
@@ -58,7 +58,7 @@
  {
xirq4 {
gpio-hog;
-   gpios = <124 0>;
+   gpios = ;
input;
};
 };
diff --git a/arch/arm/boot/dts/uniphier-pro4-ref.dts 
b/arch/arm/boot/dts/uniphier-pro4-ref.dts
index be99467..6a004e5 100644
--- a/arch/arm/boot/dts/uniphier-pro4-ref.dts
+++ b/arch/arm/boot/dts/uniphier-pro4-ref.dts
@@ -58,7 +58,7 @@
  {
xirq2 {
gpio-hog;
-   gpios = <122 0>;
+   gpios = ;
input;
};
 };
diff --git a/arch/arm/boot/dts/uniphier-pro4.dtsi 
b/arch/arm/boot/dts/uniphier-pro4.dtsi
index 7955c3a..b682a42 100644
--- a/arch/arm/boot/dts/uniphier-pro4.dtsi
+++ b/arch/arm/boot/dts/uniphier-pro4.dtsi
@@ -7,6 +7,8 @@
  * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
  */
 
+#include 
+
 / {
compatible = "socionext,uniphier-pro4";
#address-cells = <1>;
diff --git a/arch/arm/boot/dts/uniphier-pxs2.dtsi 
b/arch/arm/boot/dts/uniphier-pxs2.dtsi
index d82d6d8..eafe4dd 100644
--- a/arch/arm/boot/dts/uniphier-pxs2.dtsi
+++ b/arch/arm/boot/dts/uniphier-pxs2.dtsi
@@ -7,6 +7,7 @@
  * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
  */
 
+#include 
 #include 
 
 / {
diff --git a/arch/arm/boot/dts/uniphier-sld8-ref.dts 
b/arch/arm/boot/dts/uniphier-sld8-ref.dts
index 1c0e707..e052ea3 100644
--- a/arch/arm/boot/dts/uniphier-sld8-ref.dts
+++ b/arch/arm/boot/dts/uniphier-sld8-ref.dts
@@ -56,7 +56,7 @@
  {
xirq0 {
gpio-hog;
-   gpios = <120 0>;
+   gpios = ;
input;
};
 };
diff --git a/arch/arm/boot/dts/uniphier-sld8.dtsi 
b/arch/arm/boot/dts/uniphier-sld8.dtsi
index 7188536..89c01cc 100644
--- a/arch/arm/boot/dts/uniphier-sld8.dtsi
+++ b/arch/arm/boot/dts/uniphier-sld8.dtsi
@@ -7,6 +7,8 @@
  * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
  */
 
+#include 
+
 / {
compatible = "socionext,uniphier-sld8";
#address-cells = <1>;
-- 
2.7.4

Re: [git pull] drm amdgpu-dc merge

[Dave Airlie]

gmail for some reason ate my email formatting, apparantly preediting
in gedit, then pasting in here doesn't work so well.

Dave.

On 17 November 2017 at 14:05, Dave Airlie  wrote:
> Hi Linus,
>
> This is the pull request for the AMD DC (display code) layer which is
> a requirement
> to program the display engines on the new Vega and Raven based GPUs.
> It also contains
> support for all amdgpu supported GPUs (CIK, VI, Polaris), which you
> have to enable.
> It is also a kms atomic modesetting compatible driver (unlike the
> current in-tree display code).
>
> I've kept it separate from drm-next because it may have some things
> that cause you to reject it.
>
> Background story:
> AMD have an internal team creating a shared OS codebase for display at
> hw bring up time
> using information from their hardware teams. This process doesn't lead
> to the most Linux
> friendly/looking code but we have worked together on cleaning a lot of
> it up and dealing
> with sparse/smatch/checkpatch, and having their team internally adhere
> to Linux coding standards.
>
> This tree is a complete history rebased since they started opening it,
> we decided
> not to squash it down as the history may have some value. Some of the
> commits therefore
> might not reach kernel standards, and we are steadily training people
> in AMD to better
> write commit msgs.
>
> There is a major bunch of generated bandwidth calculation and
> verification code that comes
> from their hardware team. On Vega and before this is float
> calculations, on Raven (DCN10)
> this is double based. They do the required things to do FP in the
> kernel, and I could
> understand this might raise some issues. Rewriting the bandwidth would
> be a major undertaken
> in reverification, it's non-trivial to work out if a display can
> handle the complete set
> of mode information thrown at it.
>
> Future story:
> There is a TODO list with this, and it address most of the remaining
> things that would be
> nice to refine/remove. The DCN10 code is still under development
> internally and they push
> out a lot of patches quite regularly and are supporting this code base
> with their display
> team. I think we've reached the point where keeping it out of tree is
> going to motivate
> distributions to start carrying the code, so I'd prefer we get it in
> tree. I think this
> code is slightly better than STAGING quality but not massively so, I'd
> really like to see
> that float/double magic gone and fixed point used, but AMD don't seem
> to think the accuracy
> and revalidation of the code is worth the effort.
>
> Dave.
>
>
> The following changes since commit 6c94804fde4415f3938778155d8e665e6870a46d:
>
>   Merge tag 'drm-misc-next-2017-10-16' of
> git://anongit.freedesktop.org/drm/drm-misc into drm-next (2017-10-17
> 10:10:17 +1000)
>
> are available in the git repository at:
>
>   git://people.freedesktop.org/~airlied/linux tags/drm-for-v4.15-amd-dc
>
> for you to fetch changes up to 49e37ba07a3ae697086c0a1a32c113a1f177d138:
>
>   Merge branch 'drm-next-4.15-dc' of
> git://people.freedesktop.org/~agd5f/linux into drm-next (2017-11-16
> 12:39:40 +1000)
>
> 
> amdgpu DC display code for Vega.
>
> 
> AMD\ktsao (1):
>   drm/amd/display: remove DCN1 guard as DCN1 is already open sourced.
>
> Alex Deucher (30):
>   drm/amd/dc/dm: remove redundant display structs
>   drm/amd/display: Enable DCE12 support
>   drm/amd/display: Remove DCE12 guards
>   drm/amdgpu/soc15: enable dc on vega10
>   drm/amd/display: decouple per-crtc-plane model
>   drm/amd/display: fix nullptr on vega initialization
>   drm/amdgpu/display: Enable DCN in DC
>   drm/amdgpu/soc15: enable DC ip module for Raven
>   drm/amd/display/dc: Make dce110_validate_bandwidth static (v2)
>   drm/amd/display/dc: make dce120_link_encoder_create static
>   drm/amd/display/dm: add KV, KB, ML (v2)
>   drm/amdgpu: add DCE8 APUs to dc_supported check
>   drm/amd/display/dc: add DIGG for KV
>   drm/amd/display/dc: add DCE_VERSION for DCE8 APUs
>   drm/amd/disply/dc: add resource support for DCE8 APUs (v2)
>   drm/amdgpu/cik: add IP modules for DC for APUs
>   drm/amdgpu: disable DC on KB/ML for now
>   drm/amdgpu: drop experimental flag for vega10
>   drm/amd/display: fix typo in function name
>   drm/amd/display: whitespace cleanup in amdgpu_dm.c/h
>   drm/amd/display: make a bunch of stuff in amdgpu_dm.c static
>   drm/amd/display: drop unused functions in amdgpu_dm.c
>   drm/amd/display: drop unused functions in amdgpu_dm_services.c
>   drm/amd/display: whitespace cleanup in amdgpu_dm_mst_types.c/h
>   drm/amd/display: make log_dpcd static
>   drm/amd/display: whitespace cleanup in amdgpu_dm_irq.c/h
>   drm/amd/display: remove unused functions in amdgpu_dm_irq.c
>

Re: [git pull] drm amdgpu-dc merge

[Dave Airlie]

gmail for some reason ate my email formatting, apparantly preediting
in gedit, then pasting in here doesn't work so well.

Dave.

On 17 November 2017 at 14:05, Dave Airlie  wrote:
> Hi Linus,
>
> This is the pull request for the AMD DC (display code) layer which is
> a requirement
> to program the display engines on the new Vega and Raven based GPUs.
> It also contains
> support for all amdgpu supported GPUs (CIK, VI, Polaris), which you
> have to enable.
> It is also a kms atomic modesetting compatible driver (unlike the
> current in-tree display code).
>
> I've kept it separate from drm-next because it may have some things
> that cause you to reject it.
>
> Background story:
> AMD have an internal team creating a shared OS codebase for display at
> hw bring up time
> using information from their hardware teams. This process doesn't lead
> to the most Linux
> friendly/looking code but we have worked together on cleaning a lot of
> it up and dealing
> with sparse/smatch/checkpatch, and having their team internally adhere
> to Linux coding standards.
>
> This tree is a complete history rebased since they started opening it,
> we decided
> not to squash it down as the history may have some value. Some of the
> commits therefore
> might not reach kernel standards, and we are steadily training people
> in AMD to better
> write commit msgs.
>
> There is a major bunch of generated bandwidth calculation and
> verification code that comes
> from their hardware team. On Vega and before this is float
> calculations, on Raven (DCN10)
> this is double based. They do the required things to do FP in the
> kernel, and I could
> understand this might raise some issues. Rewriting the bandwidth would
> be a major undertaken
> in reverification, it's non-trivial to work out if a display can
> handle the complete set
> of mode information thrown at it.
>
> Future story:
> There is a TODO list with this, and it address most of the remaining
> things that would be
> nice to refine/remove. The DCN10 code is still under development
> internally and they push
> out a lot of patches quite regularly and are supporting this code base
> with their display
> team. I think we've reached the point where keeping it out of tree is
> going to motivate
> distributions to start carrying the code, so I'd prefer we get it in
> tree. I think this
> code is slightly better than STAGING quality but not massively so, I'd
> really like to see
> that float/double magic gone and fixed point used, but AMD don't seem
> to think the accuracy
> and revalidation of the code is worth the effort.
>
> Dave.
>
>
> The following changes since commit 6c94804fde4415f3938778155d8e665e6870a46d:
>
>   Merge tag 'drm-misc-next-2017-10-16' of
> git://anongit.freedesktop.org/drm/drm-misc into drm-next (2017-10-17
> 10:10:17 +1000)
>
> are available in the git repository at:
>
>   git://people.freedesktop.org/~airlied/linux tags/drm-for-v4.15-amd-dc
>
> for you to fetch changes up to 49e37ba07a3ae697086c0a1a32c113a1f177d138:
>
>   Merge branch 'drm-next-4.15-dc' of
> git://people.freedesktop.org/~agd5f/linux into drm-next (2017-11-16
> 12:39:40 +1000)
>
> 
> amdgpu DC display code for Vega.
>
> 
> AMD\ktsao (1):
>   drm/amd/display: remove DCN1 guard as DCN1 is already open sourced.
>
> Alex Deucher (30):
>   drm/amd/dc/dm: remove redundant display structs
>   drm/amd/display: Enable DCE12 support
>   drm/amd/display: Remove DCE12 guards
>   drm/amdgpu/soc15: enable dc on vega10
>   drm/amd/display: decouple per-crtc-plane model
>   drm/amd/display: fix nullptr on vega initialization
>   drm/amdgpu/display: Enable DCN in DC
>   drm/amdgpu/soc15: enable DC ip module for Raven
>   drm/amd/display/dc: Make dce110_validate_bandwidth static (v2)
>   drm/amd/display/dc: make dce120_link_encoder_create static
>   drm/amd/display/dm: add KV, KB, ML (v2)
>   drm/amdgpu: add DCE8 APUs to dc_supported check
>   drm/amd/display/dc: add DIGG for KV
>   drm/amd/display/dc: add DCE_VERSION for DCE8 APUs
>   drm/amd/disply/dc: add resource support for DCE8 APUs (v2)
>   drm/amdgpu/cik: add IP modules for DC for APUs
>   drm/amdgpu: disable DC on KB/ML for now
>   drm/amdgpu: drop experimental flag for vega10
>   drm/amd/display: fix typo in function name
>   drm/amd/display: whitespace cleanup in amdgpu_dm.c/h
>   drm/amd/display: make a bunch of stuff in amdgpu_dm.c static
>   drm/amd/display: drop unused functions in amdgpu_dm.c
>   drm/amd/display: drop unused functions in amdgpu_dm_services.c
>   drm/amd/display: whitespace cleanup in amdgpu_dm_mst_types.c/h
>   drm/amd/display: make log_dpcd static
>   drm/amd/display: whitespace cleanup in amdgpu_dm_irq.c/h
>   drm/amd/display: remove unused functions in amdgpu_dm_irq.c
>   

Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type

[Tan Xiaojun]

On 2017/11/17 10:13, Tan Xiaojun wrote:
> On 2017/11/16 23:23, Sudeep Holla wrote:
>>
>>
>> On 16/11/17 12:58, Tan Xiaojun wrote:
>>> Since commit dfea747d2aba ("drivers: base: cacheinfo: support DT
>>> overrides for cache properties"), we can set the correct cacheinfo
>>> via DT. But the cache type can't be set in the same way.
>>>
>>> I found this may be a problem in recent tests. I tested L3 cache
>>> node setting in DT in Hisilicon D03/D05 board. And I got cacheinfo
>>> via sysfs below:
>>
>> I assume L3 is outer non-architected system cache.
>>
> 
> Yes.
> 
>>>
>>> $ cat /sys/devices/system/cpu/cpu*/cache/index3/
>>> allocation_policy  level  power/
>>> shared_cpu_map uevent write_policy
>>> coherency_line_sizenumber_of_sets shared_cpu_list
>>> size   ways_of_associativity
>>>
>>> This is incomplete, no type file to display type info. Because L3
>>> cache is uncore, we can't get correct type info from system
>>> register, and will get a default type "CACHE_TYPE_NOCACHE". Then
>>> use "lscpu" will print an error like below:
>>>
>>> $ lscpu
>>> lscpu: cannot open /sys/devices/system/cpu/cpu0/cache/index3/type:
>>> No such file or directory
>>>
>>> So I think maybe we can set correct cache type via DT too.
>>>
>>> Signed-off-by: Tan Xiaojun 
>>> ---
>>>  drivers/base/cacheinfo.c | 10 ++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
>>> index eb3af27..3e650dc 100644
>>> --- a/drivers/base/cacheinfo.c
>>> +++ b/drivers/base/cacheinfo.c
>>> @@ -122,6 +122,15 @@ static inline int get_cacheinfo_idx(enum cache_type 
>>> type)
>>> return type;
>>>  }
>>>  
>>> +static void cache_type(struct cacheinfo *this_leaf)
>>> +{
>>> +   const __be32 *cache_type;
>>> +
>>> +   cache_type = of_get_property(this_leaf->of_node, "type", NULL);
>>
>> NACK for this:
>>
>> 1. We don't have any DT binding for the "type"
>> 2. Even if we had, we will never have such a binding that magical
>>returns the enum used in Linux implementation. That's not how DT
>>bindings are designed.
>>
>> Please refer ePAPR or Devicetree Specification Release 0.1 from
>> devicetree.org, we have cache-unified boolean for type.
>>
>> Let me know if the below patch works for you, I will submit it
>> preferably with your tested-by.
>>
>> Regards,
>> Sudeep
>>
> 
> OK. That's fine. I will test this.
> By the way, Arm64 tend to use acpi way to boot now. Is there some
> suitable solution for acpi?
> 
> Thanks.
> Xiaojun.
> 

Test passed, this is indeed a better solution.

Thanks.
Xiaojun.

>> -->8
>>
>> diff --git i/drivers/base/cacheinfo.c w/drivers/base/cacheinfo.c
>> index eb3af2739537..07532d83be0b 100644
>> --- i/drivers/base/cacheinfo.c
>> +++ w/drivers/base/cacheinfo.c
>> @@ -186,6 +186,11 @@ static void cache_associativity(struct cacheinfo
>> *this_leaf)
>> this_leaf->ways_of_associativity = (size / nr_sets) /
>> line_size;
>>  }
>>
>> +static bool cache_node_is_unified(struct cacheinfo *this_leaf)
>> +{
>> +   return of_property_read_bool(this_leaf->of_node, "cache-unified");
>> +}
>> +
>>  static void cache_of_override_properties(unsigned int cpu)
>>  {
>> int index;
>> @@ -194,6 +199,14 @@ static void cache_of_override_properties(unsigned
>> int cpu)
>>
>> for (index = 0; index < cache_leaves(cpu); index++) {
>> this_leaf = this_cpu_ci->info_list + index;
>> +   /*
>> +* init_cache_level must setup the cache level correctly
>> +* overriding the architecturally specified levels, so
>> +* if type is NONE at this stage, it should be unified
>> +*/
>> +   if (this_leaf->type == CACHE_TYPE_NOCACHE &&
>> +   cache_node_is_unified(this_leaf))
>> +   this_leaf->type = CACHE_TYPE_UNIFIED;
>> cache_size(this_leaf);
>> cache_get_line_size(this_leaf);
>> cache_nr_sets(this_leaf);
>>
>>
>> .
>>
> 
> 
> 
> .
> 

Re: [PATCH] drivers: base: cacheinfo: support DT overrides for cache type

[Tan Xiaojun]

On 2017/11/17 10:13, Tan Xiaojun wrote:
> On 2017/11/16 23:23, Sudeep Holla wrote:
>>
>>
>> On 16/11/17 12:58, Tan Xiaojun wrote:
>>> Since commit dfea747d2aba ("drivers: base: cacheinfo: support DT
>>> overrides for cache properties"), we can set the correct cacheinfo
>>> via DT. But the cache type can't be set in the same way.
>>>
>>> I found this may be a problem in recent tests. I tested L3 cache
>>> node setting in DT in Hisilicon D03/D05 board. And I got cacheinfo
>>> via sysfs below:
>>
>> I assume L3 is outer non-architected system cache.
>>
> 
> Yes.
> 
>>>
>>> $ cat /sys/devices/system/cpu/cpu*/cache/index3/
>>> allocation_policy  level  power/
>>> shared_cpu_map uevent write_policy
>>> coherency_line_sizenumber_of_sets shared_cpu_list
>>> size   ways_of_associativity
>>>
>>> This is incomplete, no type file to display type info. Because L3
>>> cache is uncore, we can't get correct type info from system
>>> register, and will get a default type "CACHE_TYPE_NOCACHE". Then
>>> use "lscpu" will print an error like below:
>>>
>>> $ lscpu
>>> lscpu: cannot open /sys/devices/system/cpu/cpu0/cache/index3/type:
>>> No such file or directory
>>>
>>> So I think maybe we can set correct cache type via DT too.
>>>
>>> Signed-off-by: Tan Xiaojun 
>>> ---
>>>  drivers/base/cacheinfo.c | 10 ++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
>>> index eb3af27..3e650dc 100644
>>> --- a/drivers/base/cacheinfo.c
>>> +++ b/drivers/base/cacheinfo.c
>>> @@ -122,6 +122,15 @@ static inline int get_cacheinfo_idx(enum cache_type 
>>> type)
>>> return type;
>>>  }
>>>  
>>> +static void cache_type(struct cacheinfo *this_leaf)
>>> +{
>>> +   const __be32 *cache_type;
>>> +
>>> +   cache_type = of_get_property(this_leaf->of_node, "type", NULL);
>>
>> NACK for this:
>>
>> 1. We don't have any DT binding for the "type"
>> 2. Even if we had, we will never have such a binding that magical
>>returns the enum used in Linux implementation. That's not how DT
>>bindings are designed.
>>
>> Please refer ePAPR or Devicetree Specification Release 0.1 from
>> devicetree.org, we have cache-unified boolean for type.
>>
>> Let me know if the below patch works for you, I will submit it
>> preferably with your tested-by.
>>
>> Regards,
>> Sudeep
>>
> 
> OK. That's fine. I will test this.
> By the way, Arm64 tend to use acpi way to boot now. Is there some
> suitable solution for acpi?
> 
> Thanks.
> Xiaojun.
> 

Test passed, this is indeed a better solution.

Thanks.
Xiaojun.

>> -->8
>>
>> diff --git i/drivers/base/cacheinfo.c w/drivers/base/cacheinfo.c
>> index eb3af2739537..07532d83be0b 100644
>> --- i/drivers/base/cacheinfo.c
>> +++ w/drivers/base/cacheinfo.c
>> @@ -186,6 +186,11 @@ static void cache_associativity(struct cacheinfo
>> *this_leaf)
>> this_leaf->ways_of_associativity = (size / nr_sets) /
>> line_size;
>>  }
>>
>> +static bool cache_node_is_unified(struct cacheinfo *this_leaf)
>> +{
>> +   return of_property_read_bool(this_leaf->of_node, "cache-unified");
>> +}
>> +
>>  static void cache_of_override_properties(unsigned int cpu)
>>  {
>> int index;
>> @@ -194,6 +199,14 @@ static void cache_of_override_properties(unsigned
>> int cpu)
>>
>> for (index = 0; index < cache_leaves(cpu); index++) {
>> this_leaf = this_cpu_ci->info_list + index;
>> +   /*
>> +* init_cache_level must setup the cache level correctly
>> +* overriding the architecturally specified levels, so
>> +* if type is NONE at this stage, it should be unified
>> +*/
>> +   if (this_leaf->type == CACHE_TYPE_NOCACHE &&
>> +   cache_node_is_unified(this_leaf))
>> +   this_leaf->type = CACHE_TYPE_UNIFIED;
>> cache_size(this_leaf);
>> cache_get_line_size(this_leaf);
>> cache_nr_sets(this_leaf);
>>
>>
>> .
>>
> 
> 
> 
> .
> 

[PATCH 2/4] arm64: dts: uniphier: remove unnecessary interrupt-parent

[Masahiro Yamada]

These were added to make the ARM64 branch self-contained because
updates for ARM and ARM64 are supposed to be sent as separate
pull requests.

Now, they were merged together in Linus' tree and interrupt-parent
from the arch/arm/boot/dts/uniphier-support-card.dtsi is visible from
ARM64 DT files by the cross-arch reference.

Signed-off-by: Masahiro Yamada 
---

 arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts | 1 -
 arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts | 1 -
 arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts | 1 -
 3 files changed, 3 deletions(-)

diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts 
b/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts
index dd7193a..6bdefb2 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts
@@ -40,7 +40,6 @@
 };
 
  {
-   interrupt-parent = <>;
interrupts = <0 8>;
 };
 
diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts 
b/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts
index d99e373..254d679 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts
@@ -40,7 +40,6 @@
 };
 
  {
-   interrupt-parent = <>;
interrupts = <0 8>;
 };
 
diff --git a/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts 
b/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts
index 864feeb..5c5e9cb 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts
+++ b/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts
@@ -38,7 +38,6 @@
 };
 
  {
-   interrupt-parent = <>;
interrupts = <0 8>;
 };
 
-- 
2.7.4

[PATCH 2/4] arm64: dts: uniphier: remove unnecessary interrupt-parent

[Masahiro Yamada]

These were added to make the ARM64 branch self-contained because
updates for ARM and ARM64 are supposed to be sent as separate
pull requests.

Now, they were merged together in Linus' tree and interrupt-parent
from the arch/arm/boot/dts/uniphier-support-card.dtsi is visible from
ARM64 DT files by the cross-arch reference.

Signed-off-by: Masahiro Yamada 
---

 arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts | 1 -
 arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts | 1 -
 arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts | 1 -
 3 files changed, 3 deletions(-)

diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts 
b/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts
index dd7193a..6bdefb2 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts
@@ -40,7 +40,6 @@
 };
 
  {
-   interrupt-parent = <>;
interrupts = <0 8>;
 };
 
diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts 
b/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts
index d99e373..254d679 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts
@@ -40,7 +40,6 @@
 };
 
  {
-   interrupt-parent = <>;
interrupts = <0 8>;
 };
 
diff --git a/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts 
b/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts
index 864feeb..5c5e9cb 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts
+++ b/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts
@@ -38,7 +38,6 @@
 };
 
  {
-   interrupt-parent = <>;
interrupts = <0 8>;
 };
 
-- 
2.7.4

[PATCH 3/4] arm64: dts: uniphier: use macros in dt-bindings header

[Masahiro Yamada]

The dt-bindings header was applied to the driver subsystem.  I had to
wait for a merge window to use it from DT.

Signed-off-by: Masahiro Yamada 
---

 arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts | 2 +-
 arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi| 3 ++-
 arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts | 2 +-
 arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi| 3 ++-
 arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi| 3 ++-
 5 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts 
b/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts
index 6bdefb2..54c5317 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts
@@ -50,7 +50,7 @@
  {
xirq0 {
gpio-hog;
-   gpios = <120 0>;
+   gpios = ;
input;
};
 };
diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi 
b/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
index 1c63d0a..ce40eb5 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
@@ -8,6 +8,7 @@
  */
 
 #include 
+#include 
 
 /memreserve/ 0x8000 0x0200;
 
@@ -100,7 +101,7 @@
 
emmc_pwrseq: emmc-pwrseq {
compatible = "mmc-pwrseq-emmc";
-   reset-gpios = < 26 GPIO_ACTIVE_LOW>;
+   reset-gpios = < UNIPHIER_GPIO_PORT(3, 2) GPIO_ACTIVE_LOW>;
};
 
timer {
diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts 
b/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts
index 254d679..6933710 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts
@@ -50,7 +50,7 @@
  {
xirq0 {
gpio-hog;
-   gpios = <120 0>;
+   gpios = ;
input;
};
 };
diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi 
b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
index 5c81070..8a3276b 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
@@ -8,6 +8,7 @@
  */
 
 #include 
+#include 
 #include 
 
 /memreserve/ 0x8000 0x0200;
@@ -172,7 +173,7 @@
 
emmc_pwrseq: emmc-pwrseq {
compatible = "mmc-pwrseq-emmc";
-   reset-gpios = < 26 GPIO_ACTIVE_LOW>;
+   reset-gpios = < UNIPHIER_GPIO_PORT(3, 2) GPIO_ACTIVE_LOW>;
};
 
timer {
diff --git a/arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi 
b/arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi
index 48e7331..d2beadd 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi
@@ -8,6 +8,7 @@
  */
 
 #include 
+#include 
 
 /memreserve/ 0x8000 0x0200;
 
@@ -128,7 +129,7 @@
 
emmc_pwrseq: emmc-pwrseq {
compatible = "mmc-pwrseq-emmc";
-   reset-gpios = < 47 GPIO_ACTIVE_LOW>;
+   reset-gpios = < UNIPHIER_GPIO_PORT(5, 7) GPIO_ACTIVE_LOW>;
};
 
timer {
-- 
2.7.4

[PATCH 4/4] arm64: dts: uniphier: add GPIO hog definition for PXs3

[Masahiro Yamada]

Commit 15e85695e500 ("arm64: dts: uniphier: add GPIO hog definition")
missed to update the PXs3 DTS for some reason.  Do it now.

Signed-off-by: Masahiro Yamada 
---

 arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts 
b/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts
index 5c5e9cb..c36106f 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts
+++ b/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts
@@ -45,6 +45,14 @@
status = "okay";
 };
 
+ {
+   xirq4 {
+   gpio-hog;
+   gpios = ;
+   input;
+   };
+};
+
  {
status = "okay";
 };
-- 
2.7.4

[PATCH 3/4] arm64: dts: uniphier: use macros in dt-bindings header

[Masahiro Yamada]

The dt-bindings header was applied to the driver subsystem.  I had to
wait for a merge window to use it from DT.

Signed-off-by: Masahiro Yamada 
---

 arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts | 2 +-
 arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi| 3 ++-
 arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts | 2 +-
 arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi| 3 ++-
 arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi| 3 ++-
 5 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts 
b/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts
index 6bdefb2..54c5317 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld11-ref.dts
@@ -50,7 +50,7 @@
  {
xirq0 {
gpio-hog;
-   gpios = <120 0>;
+   gpios = ;
input;
};
 };
diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi 
b/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
index 1c63d0a..ce40eb5 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld11.dtsi
@@ -8,6 +8,7 @@
  */
 
 #include 
+#include 
 
 /memreserve/ 0x8000 0x0200;
 
@@ -100,7 +101,7 @@
 
emmc_pwrseq: emmc-pwrseq {
compatible = "mmc-pwrseq-emmc";
-   reset-gpios = < 26 GPIO_ACTIVE_LOW>;
+   reset-gpios = < UNIPHIER_GPIO_PORT(3, 2) GPIO_ACTIVE_LOW>;
};
 
timer {
diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts 
b/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts
index 254d679..6933710 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld20-ref.dts
@@ -50,7 +50,7 @@
  {
xirq0 {
gpio-hog;
-   gpios = <120 0>;
+   gpios = ;
input;
};
 };
diff --git a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi 
b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
index 5c81070..8a3276b 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-ld20.dtsi
@@ -8,6 +8,7 @@
  */
 
 #include 
+#include 
 #include 
 
 /memreserve/ 0x8000 0x0200;
@@ -172,7 +173,7 @@
 
emmc_pwrseq: emmc-pwrseq {
compatible = "mmc-pwrseq-emmc";
-   reset-gpios = < 26 GPIO_ACTIVE_LOW>;
+   reset-gpios = < UNIPHIER_GPIO_PORT(3, 2) GPIO_ACTIVE_LOW>;
};
 
timer {
diff --git a/arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi 
b/arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi
index 48e7331..d2beadd 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi
+++ b/arch/arm64/boot/dts/socionext/uniphier-pxs3.dtsi
@@ -8,6 +8,7 @@
  */
 
 #include 
+#include 
 
 /memreserve/ 0x8000 0x0200;
 
@@ -128,7 +129,7 @@
 
emmc_pwrseq: emmc-pwrseq {
compatible = "mmc-pwrseq-emmc";
-   reset-gpios = < 47 GPIO_ACTIVE_LOW>;
+   reset-gpios = < UNIPHIER_GPIO_PORT(5, 7) GPIO_ACTIVE_LOW>;
};
 
timer {
-- 
2.7.4

[PATCH 4/4] arm64: dts: uniphier: add GPIO hog definition for PXs3

[Masahiro Yamada]

Commit 15e85695e500 ("arm64: dts: uniphier: add GPIO hog definition")
missed to update the PXs3 DTS for some reason.  Do it now.

Signed-off-by: Masahiro Yamada 
---

 arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts 
b/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts
index 5c5e9cb..c36106f 100644
--- a/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts
+++ b/arch/arm64/boot/dts/socionext/uniphier-pxs3-ref.dts
@@ -45,6 +45,14 @@
status = "okay";
 };
 
+ {
+   xirq4 {
+   gpio-hog;
+   gpios = ;
+   input;
+   };
+};
+
  {
status = "okay";
 };
-- 
2.7.4

Re: [PATCH] virto_net: remove empty file 'virtio_net.'

[David Miller]

From: Jason Wang 
Date: Fri, 17 Nov 2017 10:55:44 +0800

> 
> 
> On 2017年11月17日 10:46, Joel Stanley wrote:
>> Looks like this was mistakenly added to the tree as part of
>> commit 186b3c998c50 ("virtio-net: support XDP_REDIRECT").
>>
>> Signed-off-by: Joel Stanley 
>> ---
>>   drivers/net/virtio_net. | 0
>>   1 file changed, 0 insertions(+), 0 deletions(-)
>>   delete mode 100644 drivers/net/virtio_net.
>>
>> diff --git a/drivers/net/virtio_net. b/drivers/net/virtio_net.
>> deleted file mode 100644
>> index e69de29bb2d1..
> 
> My bad, don't know what happens at that time.
> 
> This is for -net.
> 
> Acked-by: Jason Wang 

Applied, thanks everyone.

Re: [PATCH] virto_net: remove empty file 'virtio_net.'

[David Miller]

From: Jason Wang 
Date: Fri, 17 Nov 2017 10:55:44 +0800

> 
> 
> On 2017年11月17日 10:46, Joel Stanley wrote:
>> Looks like this was mistakenly added to the tree as part of
>> commit 186b3c998c50 ("virtio-net: support XDP_REDIRECT").
>>
>> Signed-off-by: Joel Stanley 
>> ---
>>   drivers/net/virtio_net. | 0
>>   1 file changed, 0 insertions(+), 0 deletions(-)
>>   delete mode 100644 drivers/net/virtio_net.
>>
>> diff --git a/drivers/net/virtio_net. b/drivers/net/virtio_net.
>> deleted file mode 100644
>> index e69de29bb2d1..
> 
> My bad, don't know what happens at that time.
> 
> This is for -net.
> 
> Acked-by: Jason Wang 

Applied, thanks everyone.

Re: [lkp-robot] [fw_cfg] 05b5d5161b: WARNING:at_drivers/firmware/qemu_fw_cfg.c:#fw_cfg_dma_transfer

[Ye Xiaolong]

On 11/16, Michael S. Tsirkin wrote:
>On Thu, Nov 16, 2017 at 08:58:13AM +0800, kernel test robot wrote:
>> 
>> FYI, we noticed the following commit (built with gcc-6):
>> 
>> commit: 05b5d5161b9e6c72e1d06f36614edbdbfe192cc7 ("fw_cfg: do DMA read 
>> operation")
>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
>> 
>> in testcase: boot
>> 
>> on test machine: qemu-system-x86_64 -enable-kvm -cpu host -smp 2 -m 4G
>> 
>> caused below changes (please refer to attached dmesg/kmsg for entire 
>> log/backtrace):
>
>So this most likely indicates compatibility issues with the specific
>qemu version. Can you please tell us which qemu version was used
>for this testing?

root@lkp-nex04 ~# qemu-system-x86_64 --version
QEMU emulator version 2.10.0(Debian 1:2.10.0+dfsg-1)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

Thanks,
Xiaolong
>

>Thanks!
>
>
>> 
>> ++++
>> || 
>> 102b01757e | 05b5d5161b |
>> ++++
>> | boot_successes | 0 
>>  | 0  |
>> | boot_failures  | 12
>>  | 12 |
>> | genirq:Flags_mismatch_irq##(ttyS0)vs.#(sir_ir) | 12
>>  | 12 |
>> | WARNING:at_drivers/firmware/qemu_fw_cfg.c:#fw_cfg_dma_transfer | 0 
>>  | 8  |
>> | RIP:fw_cfg_dma_transfer| 0 
>>  | 8  |
>> | WARNING:at_fs/sysfs/dir.c:#sysfs_warn_dup  | 0 
>>  | 8  |
>> | RIP:sysfs_warn_dup | 0 
>>  | 8  |
>> | WARNING:at_lib/kobject.c:#kobject_add_internal | 0 
>>  | 8  |
>> | RIP:kobject_add_internal   | 0 
>>  | 8  |
>> ++++
>> 
>> 
>> 
>> [  156.143041] WARNING: CPU: 0 PID: 1 at drivers/firmware/qemu_fw_cfg.c:163 
>> fw_cfg_dma_transfer+0x55d/0x600
>> [  156.143041] CPU: 0 PID: 1 Comm: swapper Not tainted 4.14.0-7-g05b5d51 
>> #1
>> [  156.143041] task: 88013f02e000 task.stack: c9008000
>> [  156.143041] RIP: 0010:fw_cfg_dma_transfer+0x55d/0x600
>> [  156.143041] RSP: :c900bc08 EFLAGS: 00010202
>> [  156.143041] RAX: 001c RBX: 880116d761c8 RCX: 
>> 
>> [  156.143041] RDX:  RSI: 0004 RDI: 
>> 0202
>> [  156.143041] RBP: 00245ac7fec4 R08: 0001 R09: 
>> 
>> [  156.143041] R10: c906be20 R11: 99779f29 R12: 
>> 0004
>> [  156.143041] R13: 0004 R14: bbfdc000 R15: 
>> 
>> [  156.143041] FS:  () GS:83252000() 
>> knlGS:
>> [  156.143041] CS:  0010 DS:  ES:  CR0: 80050033
>> [  156.143041] CR2:  CR3: 03215000 CR4: 
>> 06b0
>> [  156.143041] Call Trace:
>> [  156.143041]  ? fw_cfg_read_blob+0x192/0x2d0
>> [  156.143041]  ? fw_cfg_register_dir_entries+0xaa/0x560
>> [  156.143041]  ? fw_cfg_sysfs_probe+0x408/0x590
>> [  156.143041]  ? fw_cfg_sysfs_read_raw+0xa0/0xa0
>> [  156.143041]  ? platform_drv_probe+0x98/0x180
>> [  156.143041]  ? platform_drv_remove+0x70/0x70
>> [  156.143041]  ? really_probe+0x2ca/0x770
>> [  156.143041]  ? driver_probe_device+0x170/0x170
>> [  156.143041]  ? driver_probe_device+0xf8/0x170
>> [  156.143041]  ? driver_probe_device+0x170/0x170
>> [  156.143041]  ? __driver_attach+0x189/0x1f0
>> [  156.143041]  ? bus_for_each_dev+0xc3/0x140
>> [  156.143041]  ? driver_attach+0x26/0x30
>> [  156.143041]  ? bus_add_driver+0x1fd/0x420
>> [  156.143041]  ? firmware_map_add_early+0xef/0xef
>> [  156.143041]  ? driver_register+0x146/0x1c0
>> [  156.143041]  ? __platform_driver_register+0x42/0x50
>> [  156.143041]  ? fw_cfg_sysfs_init+0x85/0x104
>> [  156.143041]  ? firmware_map_add_early+0xef/0xef
>> [  156.143041]  ? do_one_initcall+0x132/0x339
>> [  156.143041]  ? kernel_init_freeable+0x269/0x425
>> [  156.143041]  ? rest_init+0x150/0x150
>> [  156.143041]  ? kernel_init+0x17/0x220
>> [  156.143041]  ? rest_init+0x150/0x150
>> [  156.143041]  ? rest_init+0x150/0x150
>> [  156.143041]  ? ret_from_fork+0x25/0x30
>> [  156.143041] Code: 48 c7 c6 a0 e2 ac 82 48 c7 c7 e1 33 15 83 48 83 05 10 
>> e0 03 04 01 48 83 05 a0 e0 03 04 01 e8 b1 80 dd fe 48 83 05 a3 e0 03 04 01 
>> <0f> ff 48 83 05 a1 e0 03 04 01 49 c7 c5 c2 ff ff ff e9 67 fe ff 
>> [  156.143041] ---[ end trace a9d40b19c3eadcfd ]---
>> 
>> 
>> To reproduce:
>> 
>> git clone https://github.com/intel/lkp-tests.git
>> 

Re: [lkp-robot] [fw_cfg] 05b5d5161b: WARNING:at_drivers/firmware/qemu_fw_cfg.c:#fw_cfg_dma_transfer

[Ye Xiaolong]

On 11/16, Michael S. Tsirkin wrote:
>On Thu, Nov 16, 2017 at 08:58:13AM +0800, kernel test robot wrote:
>> 
>> FYI, we noticed the following commit (built with gcc-6):
>> 
>> commit: 05b5d5161b9e6c72e1d06f36614edbdbfe192cc7 ("fw_cfg: do DMA read 
>> operation")
>> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master
>> 
>> in testcase: boot
>> 
>> on test machine: qemu-system-x86_64 -enable-kvm -cpu host -smp 2 -m 4G
>> 
>> caused below changes (please refer to attached dmesg/kmsg for entire 
>> log/backtrace):
>
>So this most likely indicates compatibility issues with the specific
>qemu version. Can you please tell us which qemu version was used
>for this testing?

root@lkp-nex04 ~# qemu-system-x86_64 --version
QEMU emulator version 2.10.0(Debian 1:2.10.0+dfsg-1)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

Thanks,
Xiaolong
>

>Thanks!
>
>
>> 
>> ++++
>> || 
>> 102b01757e | 05b5d5161b |
>> ++++
>> | boot_successes | 0 
>>  | 0  |
>> | boot_failures  | 12
>>  | 12 |
>> | genirq:Flags_mismatch_irq##(ttyS0)vs.#(sir_ir) | 12
>>  | 12 |
>> | WARNING:at_drivers/firmware/qemu_fw_cfg.c:#fw_cfg_dma_transfer | 0 
>>  | 8  |
>> | RIP:fw_cfg_dma_transfer| 0 
>>  | 8  |
>> | WARNING:at_fs/sysfs/dir.c:#sysfs_warn_dup  | 0 
>>  | 8  |
>> | RIP:sysfs_warn_dup | 0 
>>  | 8  |
>> | WARNING:at_lib/kobject.c:#kobject_add_internal | 0 
>>  | 8  |
>> | RIP:kobject_add_internal   | 0 
>>  | 8  |
>> ++++
>> 
>> 
>> 
>> [  156.143041] WARNING: CPU: 0 PID: 1 at drivers/firmware/qemu_fw_cfg.c:163 
>> fw_cfg_dma_transfer+0x55d/0x600
>> [  156.143041] CPU: 0 PID: 1 Comm: swapper Not tainted 4.14.0-7-g05b5d51 
>> #1
>> [  156.143041] task: 88013f02e000 task.stack: c9008000
>> [  156.143041] RIP: 0010:fw_cfg_dma_transfer+0x55d/0x600
>> [  156.143041] RSP: :c900bc08 EFLAGS: 00010202
>> [  156.143041] RAX: 001c RBX: 880116d761c8 RCX: 
>> 
>> [  156.143041] RDX:  RSI: 0004 RDI: 
>> 0202
>> [  156.143041] RBP: 00245ac7fec4 R08: 0001 R09: 
>> 
>> [  156.143041] R10: c906be20 R11: 99779f29 R12: 
>> 0004
>> [  156.143041] R13: 0004 R14: bbfdc000 R15: 
>> 
>> [  156.143041] FS:  () GS:83252000() 
>> knlGS:
>> [  156.143041] CS:  0010 DS:  ES:  CR0: 80050033
>> [  156.143041] CR2:  CR3: 03215000 CR4: 
>> 06b0
>> [  156.143041] Call Trace:
>> [  156.143041]  ? fw_cfg_read_blob+0x192/0x2d0
>> [  156.143041]  ? fw_cfg_register_dir_entries+0xaa/0x560
>> [  156.143041]  ? fw_cfg_sysfs_probe+0x408/0x590
>> [  156.143041]  ? fw_cfg_sysfs_read_raw+0xa0/0xa0
>> [  156.143041]  ? platform_drv_probe+0x98/0x180
>> [  156.143041]  ? platform_drv_remove+0x70/0x70
>> [  156.143041]  ? really_probe+0x2ca/0x770
>> [  156.143041]  ? driver_probe_device+0x170/0x170
>> [  156.143041]  ? driver_probe_device+0xf8/0x170
>> [  156.143041]  ? driver_probe_device+0x170/0x170
>> [  156.143041]  ? __driver_attach+0x189/0x1f0
>> [  156.143041]  ? bus_for_each_dev+0xc3/0x140
>> [  156.143041]  ? driver_attach+0x26/0x30
>> [  156.143041]  ? bus_add_driver+0x1fd/0x420
>> [  156.143041]  ? firmware_map_add_early+0xef/0xef
>> [  156.143041]  ? driver_register+0x146/0x1c0
>> [  156.143041]  ? __platform_driver_register+0x42/0x50
>> [  156.143041]  ? fw_cfg_sysfs_init+0x85/0x104
>> [  156.143041]  ? firmware_map_add_early+0xef/0xef
>> [  156.143041]  ? do_one_initcall+0x132/0x339
>> [  156.143041]  ? kernel_init_freeable+0x269/0x425
>> [  156.143041]  ? rest_init+0x150/0x150
>> [  156.143041]  ? kernel_init+0x17/0x220
>> [  156.143041]  ? rest_init+0x150/0x150
>> [  156.143041]  ? rest_init+0x150/0x150
>> [  156.143041]  ? ret_from_fork+0x25/0x30
>> [  156.143041] Code: 48 c7 c6 a0 e2 ac 82 48 c7 c7 e1 33 15 83 48 83 05 10 
>> e0 03 04 01 48 83 05 a0 e0 03 04 01 e8 b1 80 dd fe 48 83 05 a3 e0 03 04 01 
>> <0f> ff 48 83 05 a1 e0 03 04 01 49 c7 c5 c2 ff ff ff e9 67 fe ff 
>> [  156.143041] ---[ end trace a9d40b19c3eadcfd ]---
>> 
>> 
>> To reproduce:
>> 
>> git clone https://github.com/intel/lkp-tests.git
>> 

Re: [PATCH] rdma: Add Jason as a co-maintainer

[Leon Romanovsky]

On Thu, Nov 16, 2017 at 01:44:00PM -0700, Jason Gunthorpe wrote:
> As was discussed in September and October, add Jason along with
> Doug to have a team maintainership model for the RDMA subystem.
>
> Mellanox Technologies will be funding Jason's independent work on
> the maintainership.
>
> Signed-off-by: Jason Gunthorpe 
> ---
>  .mailmap| 2 ++
>  MAINTAINERS | 1 +
>  2 files changed, 3 insertions(+)
>
> My updated PGP key with the new email addresses is in the key servers
> and available here:
>
>  http://www.ziepe.ca/pgp/jgunthorpe.gpg
>
> diff --git a/.mailmap b/.mailmap
> index c021f29779a7a1..1469ff0d3f4d55 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -73,6 +73,8 @@ James E Wilson 
>  James Hogan  
>  James Hogan  
>  James Ketrenos 
> +Jason Gunthorpe  
> +Jason Gunthorpe  
>  Javi Merino  
>   
>  Jean Tourrilhes 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7f9c4f3fc9419d..d4e621e350f2cf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6827,6 +6827,7 @@ F:  drivers/ipack/
>
>  INFINIBAND SUBSYSTEM
>  M:   Doug Ledford 
> +M:   Jason Gunthorpe 


Jason,

I want to warn that you will have hard to use your @mellanox.com address
for any external to Mellanox communication, because of questionable IT
innovation - they rewrite links in all coming emails.

It will make your replies looking very bad and it will be unreadable for the 
people
who are using plain text readers to read emails.

The example of it, you can see here [1].

We (top Mellanox upstreamers) tried to fight for it for the more than half a 
year,
and gave up - using my external email for everything.

Thanks

[1] https://marc.info/?l=linux-rdma=151077009200628=2


>  L:   linux-r...@vger.kernel.org
>  W:   http://www.openfabrics.org/
>  Q:   http://patchwork.kernel.org/project/linux-rdma/list/
> --
> 2.7.4
>




signature.asc
Description: PGP signature

Re: [PATCH] rdma: Add Jason as a co-maintainer

[Leon Romanovsky]

On Thu, Nov 16, 2017 at 01:44:00PM -0700, Jason Gunthorpe wrote:
> As was discussed in September and October, add Jason along with
> Doug to have a team maintainership model for the RDMA subystem.
>
> Mellanox Technologies will be funding Jason's independent work on
> the maintainership.
>
> Signed-off-by: Jason Gunthorpe 
> ---
>  .mailmap| 2 ++
>  MAINTAINERS | 1 +
>  2 files changed, 3 insertions(+)
>
> My updated PGP key with the new email addresses is in the key servers
> and available here:
>
>  http://www.ziepe.ca/pgp/jgunthorpe.gpg
>
> diff --git a/.mailmap b/.mailmap
> index c021f29779a7a1..1469ff0d3f4d55 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -73,6 +73,8 @@ James E Wilson 
>  James Hogan  
>  James Hogan  
>  James Ketrenos 
> +Jason Gunthorpe  
> +Jason Gunthorpe  
>  Javi Merino  
>   
>  Jean Tourrilhes 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7f9c4f3fc9419d..d4e621e350f2cf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6827,6 +6827,7 @@ F:  drivers/ipack/
>
>  INFINIBAND SUBSYSTEM
>  M:   Doug Ledford 
> +M:   Jason Gunthorpe 


Jason,

I want to warn that you will have hard to use your @mellanox.com address
for any external to Mellanox communication, because of questionable IT
innovation - they rewrite links in all coming emails.

It will make your replies looking very bad and it will be unreadable for the 
people
who are using plain text readers to read emails.

The example of it, you can see here [1].

We (top Mellanox upstreamers) tried to fight for it for the more than half a 
year,
and gave up - using my external email for everything.

Thanks

[1] https://marc.info/?l=linux-rdma=151077009200628=2


>  L:   linux-r...@vger.kernel.org
>  W:   http://www.openfabrics.org/
>  Q:   http://patchwork.kernel.org/project/linux-rdma/list/
> --
> 2.7.4
>




signature.asc
Description: PGP signature

wlcore: wl18xx: Trouble suspending caused by wifi being enabled?

[John Stultz]

Hey Folks,
  So, for awhile recently, I've noticed my HiKey board (which uses the
TI WL1835MOD chip for wifi) has had trouble when it tries to suspend.
Basically it keeps on waking up while suspending, and then
re-suspending over and over until it lucks out in whatever race is
going on and manages to suspend before the wifi wakes it up.

Here's an example suspend-immediate-wake cycle:

[  241.975754] PM: suspend entry (deep)
[  241.979590] PM: Syncing filesystems ... done.
[  241.997363] Freezing user space processes ... (elapsed 0.003 seconds) done.
[  242.007903] OOM killer disabled.
[  242.011157] Freezing remaining freezable tasks ... (elapsed 0.002
seconds) done.
[  242.020715] Suspending console(s) (use no_console_suspend to debug)
[  242.028155] wlan0: deauthenticating from 70:3a:cb:12:90:28 by local
choice (Reason: 3=DEAUTH_LEAVING)
[  242.029797] dwc2 f72c.usb: suspending usb gadget configfs-gadget
[  242.029885] dwc2 f72c.usb: dwc2_hsotg_ep_disable: called for ep0
[  242.029893] dwc2 f72c.usb: dwc2_hsotg_ep_disable: called for ep0
[  242.071475] wlcore: down
[  242.071795] queueing ieee80211 work while going to suspend
[  242.071808] wlcore: down
[  242.072120] queueing ieee80211 work while going to suspend
[  242.073796] PM: Wakeup pending, aborting suspend
[  242.073810] PM: Some devices failed to suspend, or early wake event detected
[  242.091277] mmc_host mmc2: Bus speed (slot 0) = 2480Hz (slot
req 40Hz, actual 40HZ div = 31)
[  242.128593] mmc_host mmc2: Bus speed (slot 0) = 2480Hz (slot
req 2500Hz, actual 2480HZ div = 0)
[  242.129197] dwc2 f72c.usb: resuming usb gadget configfs-gadget
[  242.337364] dwc2 f72c.usb: new device is high-speed
[  242.413119] dwc2 f72c.usb: new device is high-speed
[  242.452173] wlcore: PHY firmware version: Rev 8.2.0.0.237
[  242.484959] dwc2 f72c.usb: new address 8
[  242.499012] wlcore: firmware booted (Rev 8.9.0.0.70)
[  242.506240] configfs-gadget gadget: high-speed config #1: b
[  242.627752] OOM killer enabled.
[  242.630899] Restarting tasks ... done.
[  242.647180] PM: suspend exit

Eventually it will luck out and manage to suspend itself, but it can
take close to a minute.  If I disable wifi the system reliably
suspends every time.

This used to not be the case, but its been so long, I'm not really
sure when this issue cropped up.

I wanted to see if anyone else had similar trouble or maybe had ideas
how to chase this down?

thanks
-john

wlcore: wl18xx: Trouble suspending caused by wifi being enabled?

[John Stultz]

Hey Folks,
  So, for awhile recently, I've noticed my HiKey board (which uses the
TI WL1835MOD chip for wifi) has had trouble when it tries to suspend.
Basically it keeps on waking up while suspending, and then
re-suspending over and over until it lucks out in whatever race is
going on and manages to suspend before the wifi wakes it up.

Here's an example suspend-immediate-wake cycle:

[  241.975754] PM: suspend entry (deep)
[  241.979590] PM: Syncing filesystems ... done.
[  241.997363] Freezing user space processes ... (elapsed 0.003 seconds) done.
[  242.007903] OOM killer disabled.
[  242.011157] Freezing remaining freezable tasks ... (elapsed 0.002
seconds) done.
[  242.020715] Suspending console(s) (use no_console_suspend to debug)
[  242.028155] wlan0: deauthenticating from 70:3a:cb:12:90:28 by local
choice (Reason: 3=DEAUTH_LEAVING)
[  242.029797] dwc2 f72c.usb: suspending usb gadget configfs-gadget
[  242.029885] dwc2 f72c.usb: dwc2_hsotg_ep_disable: called for ep0
[  242.029893] dwc2 f72c.usb: dwc2_hsotg_ep_disable: called for ep0
[  242.071475] wlcore: down
[  242.071795] queueing ieee80211 work while going to suspend
[  242.071808] wlcore: down
[  242.072120] queueing ieee80211 work while going to suspend
[  242.073796] PM: Wakeup pending, aborting suspend
[  242.073810] PM: Some devices failed to suspend, or early wake event detected
[  242.091277] mmc_host mmc2: Bus speed (slot 0) = 2480Hz (slot
req 40Hz, actual 40HZ div = 31)
[  242.128593] mmc_host mmc2: Bus speed (slot 0) = 2480Hz (slot
req 2500Hz, actual 2480HZ div = 0)
[  242.129197] dwc2 f72c.usb: resuming usb gadget configfs-gadget
[  242.337364] dwc2 f72c.usb: new device is high-speed
[  242.413119] dwc2 f72c.usb: new device is high-speed
[  242.452173] wlcore: PHY firmware version: Rev 8.2.0.0.237
[  242.484959] dwc2 f72c.usb: new address 8
[  242.499012] wlcore: firmware booted (Rev 8.9.0.0.70)
[  242.506240] configfs-gadget gadget: high-speed config #1: b
[  242.627752] OOM killer enabled.
[  242.630899] Restarting tasks ... done.
[  242.647180] PM: suspend exit

Eventually it will luck out and manage to suspend itself, but it can
take close to a minute.  If I disable wifi the system reliably
suspends every time.

This used to not be the case, but its been so long, I'm not really
sure when this issue cropped up.

I wanted to see if anyone else had similar trouble or maybe had ideas
how to chase this down?

thanks
-john

Towards 4.14 LTS

[Tom Gall]

At Linaro we’ve been putting effort into regularly running kernel tests over 
arm, arm64 and x86_64 targets. On those targets we’re running mainline, -next, 
4.4, and 4.9 kernels and yes we are adding to this list as the hardware 
capacity grows.

For test buckets we’re using just LTP, kselftest and libhugetlbfs and
like kernels we will add to this list. 

With the 4.14 cycle being a little ‘different’ in so much as the goal to 
have it be an LTS kernel I think it’s important to take a look at some 
4.14 test results. 

Grab a beverage, this is a bit of a long post. But quick summery 4.14 as 
released looks just as good as 4.13, for the test buckets I named above.

I’ve enclosed our short form report. We break down the boards/arch combos for
each bucket pass/skip or potentially fails. Pretty straight forward. Skips
generally happen for a few reasons
1) crappy test cases
2) test isn’t appropriate (x86 specific tests so don’t run elsewhere)

With this, we have a decent baseline for 4.14 and other kernels going
forward. 

Summary


kernel: 4.14.0
git repo: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git branch: master
git commit: bebc6082da0a9f5d47a1ea2edc099bf671058bd4
git describe: v4.14
Test details: https://qa-reports.linaro.org/lkft/linux-mainline-oe/build/v4.14


No regressions (compared to build v4.14-rc8)

Boards, architectures and test suites:
-

hi6220-hikey - arm64
* boot - pass: 20
* kselftest - skip: 16, pass: 38
* libhugetlbfs - skip: 1, pass: 90
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - pass: 76
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - pass: 60
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - skip: 1, pass: 21
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - pass: 14
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 122, pass: 983
* ltp-timers-tests - pass: 12

juno-r2 - arm64
* boot - pass: 20
* kselftest - skip: 15, pass: 38
* libhugetlbfs - skip: 1, pass: 90
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - pass: 76
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - pass: 60
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - pass: 22
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - pass: 10
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 156, pass: 943
* ltp-timers-tests - pass: 12

x15 - arm
* boot - pass: 20
* kselftest - skip: 17, pass: 36
* libhugetlbfs - skip: 1, pass: 87
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - pass: 60
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - skip: 2, pass: 20
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - skip: 1, pass: 13
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 66, pass: 1040
* ltp-timers-tests - pass: 12

dell-poweredge-r200 - x86_64
* boot - pass: 19
* kselftest - skip: 11, pass: 54
* libhugetlbfs - skip: 1, pass: 76
* ltp-cap_bounds-tests - pass: 1
* ltp-containers-tests - pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - skip: 1, pass: 61
* ltp-fs_bind-tests - pass: 1
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - pass: 22
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 8
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - pass: 9
* ltp-securebits-tests - pass: 3
* ltp-syscalls-tests - skip: 163, pass: 962

Lots of green.


Let’s now talk about coverage, the pandora’s box of validation. It’s never
perfect. There’s a bazillion different build combos. Even tools can
make a difference. We’ve seen a case where the dhcp client from open embedded 
didn’t trigger a network regression in one of the LTS RCs but Debian’s dhclient
did.

Of no surprise between what we and others have, it’s not perfect coverage,
and there are only so many build, boot and run cycles to execute the test 
buckets with various combinations so we need to stay sensible as far as 
kernel configs go. 

Does this kind of system actually FIND anything and is it useful for 
watching for 4.14 regressions as fixes are introduced?

I would assert the answer is yes. We do have data for a couple of kernel
cycles but it’s also somewhat dirty as we have been in the process of 

Towards 4.14 LTS

[Tom Gall]

At Linaro we’ve been putting effort into regularly running kernel tests over 
arm, arm64 and x86_64 targets. On those targets we’re running mainline, -next, 
4.4, and 4.9 kernels and yes we are adding to this list as the hardware 
capacity grows.

For test buckets we’re using just LTP, kselftest and libhugetlbfs and
like kernels we will add to this list. 

With the 4.14 cycle being a little ‘different’ in so much as the goal to 
have it be an LTS kernel I think it’s important to take a look at some 
4.14 test results. 

Grab a beverage, this is a bit of a long post. But quick summery 4.14 as 
released looks just as good as 4.13, for the test buckets I named above.

I’ve enclosed our short form report. We break down the boards/arch combos for
each bucket pass/skip or potentially fails. Pretty straight forward. Skips
generally happen for a few reasons
1) crappy test cases
2) test isn’t appropriate (x86 specific tests so don’t run elsewhere)

With this, we have a decent baseline for 4.14 and other kernels going
forward. 

Summary


kernel: 4.14.0
git repo: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git branch: master
git commit: bebc6082da0a9f5d47a1ea2edc099bf671058bd4
git describe: v4.14
Test details: https://qa-reports.linaro.org/lkft/linux-mainline-oe/build/v4.14


No regressions (compared to build v4.14-rc8)

Boards, architectures and test suites:
-

hi6220-hikey - arm64
* boot - pass: 20
* kselftest - skip: 16, pass: 38
* libhugetlbfs - skip: 1, pass: 90
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - pass: 76
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - pass: 60
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - skip: 1, pass: 21
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - pass: 14
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 122, pass: 983
* ltp-timers-tests - pass: 12

juno-r2 - arm64
* boot - pass: 20
* kselftest - skip: 15, pass: 38
* libhugetlbfs - skip: 1, pass: 90
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - pass: 76
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - pass: 60
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - pass: 22
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - pass: 10
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 156, pass: 943
* ltp-timers-tests - pass: 12

x15 - arm
* boot - pass: 20
* kselftest - skip: 17, pass: 36
* libhugetlbfs - skip: 1, pass: 87
* ltp-cap_bounds-tests - pass: 2
* ltp-containers-tests - pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - pass: 60
* ltp-fs_bind-tests - pass: 2
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - skip: 2, pass: 20
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 9
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - skip: 1, pass: 13
* ltp-securebits-tests - pass: 4
* ltp-syscalls-tests - skip: 66, pass: 1040
* ltp-timers-tests - pass: 12

dell-poweredge-r200 - x86_64
* boot - pass: 19
* kselftest - skip: 11, pass: 54
* libhugetlbfs - skip: 1, pass: 76
* ltp-cap_bounds-tests - pass: 1
* ltp-containers-tests - pass: 64
* ltp-fcntl-locktests-tests - pass: 2
* ltp-filecaps-tests - pass: 2
* ltp-fs-tests - skip: 1, pass: 61
* ltp-fs_bind-tests - pass: 1
* ltp-fs_perms_simple-tests - pass: 19
* ltp-fsx-tests - pass: 2
* ltp-hugetlb-tests - pass: 22
* ltp-io-tests - pass: 3
* ltp-ipc-tests - pass: 8
* ltp-math-tests - pass: 11
* ltp-nptl-tests - pass: 2
* ltp-pty-tests - pass: 4
* ltp-sched-tests - pass: 9
* ltp-securebits-tests - pass: 3
* ltp-syscalls-tests - skip: 163, pass: 962

Lots of green.


Let’s now talk about coverage, the pandora’s box of validation. It’s never
perfect. There’s a bazillion different build combos. Even tools can
make a difference. We’ve seen a case where the dhcp client from open embedded 
didn’t trigger a network regression in one of the LTS RCs but Debian’s dhclient
did.

Of no surprise between what we and others have, it’s not perfect coverage,
and there are only so many build, boot and run cycles to execute the test 
buckets with various combinations so we need to stay sensible as far as 
kernel configs go. 

Does this kind of system actually FIND anything and is it useful for 
watching for 4.14 regressions as fixes are introduced?

I would assert the answer is yes. We do have data for a couple of kernel
cycles but it’s also somewhat dirty as we have been in the process of