Re: [PATCH v5 4/6] x86/microcode/AMD: Check microcode container data in the late loader

[Maciej S. Szmigiero]

On 01.05.2018 10:43, Borislav Petkov wrote:
> On Tue, May 01, 2018 at 12:27:51AM +0200, Maciej S. Szmigiero wrote:
>> 1) -EINVAL maps to a valid return value of 4294967274 bytes.
>> We have a different behavior for invalid data in the container file
>> (including too large lengths) than for grave errors like a failed memory
>> allocation.
> 
> WTF?

Could you please elaborate this comment?

-EINVAL cast to unsigned int is 4294967274 and this value is also
a valid count of bytes to skip that this function can return.

The "grave errors" behavior comes from the existing code, a comment
in code above verify_and_add_patch() says:
"a grave error like a memory allocation has failed and the driver cannot
continue functioning normally. In such cases, we tear down everything
we've used up so far and exit."

>> 2) This function single caller (__load_microcode_amd()) normalized any
>> error that verify_and_add_patch() returned to UCODE_ERROR anyway,
> 
> So?

This means that there is no loss of information here.

The function these three points are about (verify_and_add_patch()) is
declared as "static", so it cannot be called from any other kernel code.

>> 3) The existing code uses a convention that zero return value means
>> 'terminate processing' for the parse_container() function in the early
>> loader which normally returns a 'bytes consumed' value, as this function
>> does.
> 
> parse_container() could very well change its convention to return
> negative on error and positive value if the loop is supposed to skip
> bytes.
> 

Yes, but then the problem from the point 1) above will be introduced
also to parse_container().

Maciej

Re: [PATCH v5 4/6] x86/microcode/AMD: Check microcode container data in the late loader

[Maciej S. Szmigiero]

On 01.05.2018 10:43, Borislav Petkov wrote:
> On Tue, May 01, 2018 at 12:27:51AM +0200, Maciej S. Szmigiero wrote:
>> 1) -EINVAL maps to a valid return value of 4294967274 bytes.
>> We have a different behavior for invalid data in the container file
>> (including too large lengths) than for grave errors like a failed memory
>> allocation.
> 
> WTF?

Could you please elaborate this comment?

-EINVAL cast to unsigned int is 4294967274 and this value is also
a valid count of bytes to skip that this function can return.

The "grave errors" behavior comes from the existing code, a comment
in code above verify_and_add_patch() says:
"a grave error like a memory allocation has failed and the driver cannot
continue functioning normally. In such cases, we tear down everything
we've used up so far and exit."

>> 2) This function single caller (__load_microcode_amd()) normalized any
>> error that verify_and_add_patch() returned to UCODE_ERROR anyway,
> 
> So?

This means that there is no loss of information here.

The function these three points are about (verify_and_add_patch()) is
declared as "static", so it cannot be called from any other kernel code.

>> 3) The existing code uses a convention that zero return value means
>> 'terminate processing' for the parse_container() function in the early
>> loader which normally returns a 'bytes consumed' value, as this function
>> does.
> 
> parse_container() could very well change its convention to return
> negative on error and positive value if the loop is supposed to skip
> bytes.
> 

Yes, but then the problem from the point 1) above will be introduced
also to parse_container().

Maciej

Re: [PATCH 1/3] dt-bindings: added new pwm-sifive driver documentation

[Rob Herring]

On Mon, Apr 30, 2018 at 12:45:20PM +0200, Andreas Färber wrote:
> Am 30.04.2018 um 10:19 schrieb Thierry Reding:
> > On Sun, Apr 29, 2018 at 02:08:07PM -0700, Wesley Terpstra wrote:
> >> On Sun, Apr 29, 2018 at 2:01 PM, Andreas Färber  wrote:
> >>> "pwm0" sounds like a zero-indexed instance of some pwm block. If 0 is
> >>> the version here, I'd suggest to make it "pwm-0" for example - you might
> >>> want to take a look at the Xilinx bindings, which use a strict x.yy 
> >>> suffix.
> >>
> >> That's fine. I'll change it to pwm-0.00 in the next patch series.
> > 
> > This should match the version that you use. If you're internal
> > versioning uses single digits, or a single version number, then I think
> > there's no need to use 0.00, because that would just be confusing.
> > However I think it'd be good to make sure it is discernible as a version
> > number. Perhaps something like sifive,pwm-v0. That seems to be a fairly
> > common scheme.
> 
> Yes. My point was not to adopt another vendor's versioning scheme but to
> adopt _some_ consistent scheme and document it, e.g., in a sifive.txt
> similar to xilinx.txt:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/xilinx.txt
> 
> It should be made clear what in the compatible string the version is
> (thus my proposal of using a dash as separator), and there you may want
> to document how to map between IP/documentation and compatibles for any
> new bindings.

Yes. And using versions in compatible strings is only accepted when 
there is a well defined versioning process. FPGAs tend to be the main 
case as most SoC vendors don't have rigorous versioning processes. I 
guess it makes sense for SiFive from the little I know about them. What 
doesn't make sense or get accepted is software folks just making up v1, 
v2, v3, etc.

Rob

Re: [PATCH 1/3] dt-bindings: added new pwm-sifive driver documentation

[Rob Herring]

On Mon, Apr 30, 2018 at 12:45:20PM +0200, Andreas Färber wrote:
> Am 30.04.2018 um 10:19 schrieb Thierry Reding:
> > On Sun, Apr 29, 2018 at 02:08:07PM -0700, Wesley Terpstra wrote:
> >> On Sun, Apr 29, 2018 at 2:01 PM, Andreas Färber  wrote:
> >>> "pwm0" sounds like a zero-indexed instance of some pwm block. If 0 is
> >>> the version here, I'd suggest to make it "pwm-0" for example - you might
> >>> want to take a look at the Xilinx bindings, which use a strict x.yy 
> >>> suffix.
> >>
> >> That's fine. I'll change it to pwm-0.00 in the next patch series.
> > 
> > This should match the version that you use. If you're internal
> > versioning uses single digits, or a single version number, then I think
> > there's no need to use 0.00, because that would just be confusing.
> > However I think it'd be good to make sure it is discernible as a version
> > number. Perhaps something like sifive,pwm-v0. That seems to be a fairly
> > common scheme.
> 
> Yes. My point was not to adopt another vendor's versioning scheme but to
> adopt _some_ consistent scheme and document it, e.g., in a sifive.txt
> similar to xilinx.txt:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/xilinx.txt
> 
> It should be made clear what in the compatible string the version is
> (thus my proposal of using a dash as separator), and there you may want
> to document how to map between IP/documentation and compatibles for any
> new bindings.

Yes. And using versions in compatible strings is only accepted when 
there is a well defined versioning process. FPGAs tend to be the main 
case as most SoC vendors don't have rigorous versioning processes. I 
guess it makes sense for SiFive from the little I know about them. What 
doesn't make sense or get accepted is software folks just making up v1, 
v2, v3, etc.

Rob

Re: bug-introducing patches (or: -rc cycles suck)

[Sasha Levin]

On Mon, Apr 30, 2018 at 09:09:18PM +0200, Willy Tarreau wrote:
>Hi Sasha,
>
>On Mon, Apr 30, 2018 at 05:58:30PM +, Sasha Levin wrote:
>>  - For some reason, the odds of a -rc commit to be targetted for -stable is
>>  over 20%, while for merge window commits it's about 3%. I can't quite
>>  explain why that happens, but this would suggest that -rc commits end up
>>  hurting -stable pretty badly.
>
>Often, merge window collects work that has been done during the previous
>cycle and which is prepared to target this merge window. Fixes that happen
>during this period very likely tend to either be remerged with the patches
>before they are submitted if they concern the code to be submitted, or are
>delayed to after the work gets merged. As a result few of the pre-rc1 patches
>get backported while the next ones mostly contain fixes. By the way, you
>probably also noticed it when backporting patches to your stable releases,
>the mainline commit almost never comes from a merge window.

I'm not sure I understand/agree with this explanation. You're saying
that commits that fix issues in newly introduced features got folded in
the feature before it was sent during the merge window, so then there
was no need for them to be tagged for stable?

This would be also true for -rc cycle patches if they fix a commit that
was introduced in that merge window: patches that fix a feature that got
in that same merge window don't need to be tagged for stable either
since the feature didn't exist in a previous release.

The way I see it is that -stable commits fix a bug that was introduced
in a feature that exists in a kernel that was already released. At that
point, the fix can come in at any point in time, whether the fix was
created during the merge window, or during an -rc cycle.

It also appears that pretty much the same ratio of commits are tagged
for -stable accross all -rc cycles, so there are no spikes at any point
during the cycle, which seems to suggest that there is no particular
relationship between when a -stable commit is created to the stage in a
release cycle of the current kernel.

>> 2. Maintainers need to stop writing patches, commiting them, and pushing them
>> in without reviews. In -rc cycles there is quite a large number of commits
>> that were either written by maintainers, commited, and merged upstream the
>> same day. These patches are very likely to introduce a new bug.
>
>Developers are humans before anything else. We probably all address most
>bug reports the same way : "ah, of course, stupid me, now that's fixed".
>Keep in mind that for the developer, the pressure has lowered now that
>the code got merged, and that mentally the fix is "on top" of the initial
>work and no more part of it. It often means a narrower mental image of
>how the fix fits in the whole code.
>
>I think that you'll also notice that fixes that address bugs introduced
>during the merge window of the same version will more often introduce
>bugs than the ones which address 6-months old bugs which require some
>deeper thinking. In short it indicates that we tend to believe we are
>better than we really are, especially very late at night.

I very much agree. I also think that "upper-level" maintainers, and
Linus in particular have to stop this behavior. Yes, folks who do these
patches are often very familiar with the subsystem, but this doesn't
mean that they don't make mistakes.

It's as if during -rc cycles all rules are void and bug fixes are now
no be collected and merged in as fast as humanly possible without any
regard to how well these fixes were tested.

With merge window stuff, Linus will make lots of noise if commits didn't
spend any time in -next (see https://lkml.org/lkml/2017/2/23/611) for
example. But it seems that -rc commits don't have that requirement.

>> I don't really have a proposal beyond "tighten up -rc cycles", but I think
>> it's a discussion worth having. We have enough data to show what parts of
>> kernel development work, and what parts are just hurting us.
>
>I'm inclined to believe that making individuals aware of their own
>mistakes can help. I personally like to try to understand how I managed
>to introduce a bug, it's always useful. Very often it's around "I was
>pretty sure it didn't require testing, the change was so obvious". We
>all know this feeling when you write 100 lines in a new file, you
>compile, and it builds without any warning and apparently works, and
>suddenly you think "uh oh, what did I do wrong?" and you have no idea
>where to start to look for possible mistakes.
>
>Probably that some statistics on mistake classifications and maybe some
>affected subsystems (if that doesn't blame anyone) could be useful.

Re: [PATCH 01/21] dt-bindings: clock: Add compatible for A64 DE2 CCU

[Chen-Yu Tsai]

On Wed, May 2, 2018 at 12:16 AM, Rob Herring  wrote:
> On Mon, Apr 30, 2018 at 05:10:38PM +0530, Jagan Teki wrote:
>> Allwinner A64 has DE2 CCU which is similar to H3/H5 SoC.
>>
>> Signed-off-by: Jagan Teki 
>> ---
>>  Documentation/devicetree/bindings/clock/sun8i-de2.txt | 1 +
>>  1 file changed, 1 insertion(+)
>
> Reviewed-by: Rob Herring 

Hi Rob,

Do we need to add this if it's just a soc-specific compatible
we add in the device tree to future proof things in case we
discover quirks later on?

AFAIK we haven't been doing this, and this is likely to create
some confusion, because they aren't actually mentioned anywhere
in the driver.

ChenYu

Re: bug-introducing patches (or: -rc cycles suck)

[Sasha Levin]

On Mon, Apr 30, 2018 at 09:09:18PM +0200, Willy Tarreau wrote:
>Hi Sasha,
>
>On Mon, Apr 30, 2018 at 05:58:30PM +, Sasha Levin wrote:
>>  - For some reason, the odds of a -rc commit to be targetted for -stable is
>>  over 20%, while for merge window commits it's about 3%. I can't quite
>>  explain why that happens, but this would suggest that -rc commits end up
>>  hurting -stable pretty badly.
>
>Often, merge window collects work that has been done during the previous
>cycle and which is prepared to target this merge window. Fixes that happen
>during this period very likely tend to either be remerged with the patches
>before they are submitted if they concern the code to be submitted, or are
>delayed to after the work gets merged. As a result few of the pre-rc1 patches
>get backported while the next ones mostly contain fixes. By the way, you
>probably also noticed it when backporting patches to your stable releases,
>the mainline commit almost never comes from a merge window.

I'm not sure I understand/agree with this explanation. You're saying
that commits that fix issues in newly introduced features got folded in
the feature before it was sent during the merge window, so then there
was no need for them to be tagged for stable?

This would be also true for -rc cycle patches if they fix a commit that
was introduced in that merge window: patches that fix a feature that got
in that same merge window don't need to be tagged for stable either
since the feature didn't exist in a previous release.

The way I see it is that -stable commits fix a bug that was introduced
in a feature that exists in a kernel that was already released. At that
point, the fix can come in at any point in time, whether the fix was
created during the merge window, or during an -rc cycle.

It also appears that pretty much the same ratio of commits are tagged
for -stable accross all -rc cycles, so there are no spikes at any point
during the cycle, which seems to suggest that there is no particular
relationship between when a -stable commit is created to the stage in a
release cycle of the current kernel.

>> 2. Maintainers need to stop writing patches, commiting them, and pushing them
>> in without reviews. In -rc cycles there is quite a large number of commits
>> that were either written by maintainers, commited, and merged upstream the
>> same day. These patches are very likely to introduce a new bug.
>
>Developers are humans before anything else. We probably all address most
>bug reports the same way : "ah, of course, stupid me, now that's fixed".
>Keep in mind that for the developer, the pressure has lowered now that
>the code got merged, and that mentally the fix is "on top" of the initial
>work and no more part of it. It often means a narrower mental image of
>how the fix fits in the whole code.
>
>I think that you'll also notice that fixes that address bugs introduced
>during the merge window of the same version will more often introduce
>bugs than the ones which address 6-months old bugs which require some
>deeper thinking. In short it indicates that we tend to believe we are
>better than we really are, especially very late at night.

I very much agree. I also think that "upper-level" maintainers, and
Linus in particular have to stop this behavior. Yes, folks who do these
patches are often very familiar with the subsystem, but this doesn't
mean that they don't make mistakes.

It's as if during -rc cycles all rules are void and bug fixes are now
no be collected and merged in as fast as humanly possible without any
regard to how well these fixes were tested.

With merge window stuff, Linus will make lots of noise if commits didn't
spend any time in -next (see https://lkml.org/lkml/2017/2/23/611) for
example. But it seems that -rc commits don't have that requirement.

>> I don't really have a proposal beyond "tighten up -rc cycles", but I think
>> it's a discussion worth having. We have enough data to show what parts of
>> kernel development work, and what parts are just hurting us.
>
>I'm inclined to believe that making individuals aware of their own
>mistakes can help. I personally like to try to understand how I managed
>to introduce a bug, it's always useful. Very often it's around "I was
>pretty sure it didn't require testing, the change was so obvious". We
>all know this feeling when you write 100 lines in a new file, you
>compile, and it builds without any warning and apparently works, and
>suddenly you think "uh oh, what did I do wrong?" and you have no idea
>where to start to look for possible mistakes.
>
>Probably that some statistics on mistake classifications and maybe some
>affected subsystems (if that doesn't blame anyone) could be useful.

Re: [PATCH 01/21] dt-bindings: clock: Add compatible for A64 DE2 CCU

[Chen-Yu Tsai]

On Wed, May 2, 2018 at 12:16 AM, Rob Herring  wrote:
> On Mon, Apr 30, 2018 at 05:10:38PM +0530, Jagan Teki wrote:
>> Allwinner A64 has DE2 CCU which is similar to H3/H5 SoC.
>>
>> Signed-off-by: Jagan Teki 
>> ---
>>  Documentation/devicetree/bindings/clock/sun8i-de2.txt | 1 +
>>  1 file changed, 1 insertion(+)
>
> Reviewed-by: Rob Herring 

Hi Rob,

Do we need to add this if it's just a soc-specific compatible
we add in the device tree to future proof things in case we
discover quirks later on?

AFAIK we haven't been doing this, and this is likely to create
some confusion, because they aren't actually mentioned anywhere
in the driver.

ChenYu

Re: [PATCH v5 2/6] x86/microcode/AMD: Add microcode container data checking functions

[Maciej S. Szmigiero]

On 01.05.2018 10:18, Borislav Petkov wrote:
> On Tue, May 01, 2018 at 12:27:17AM +0200, Maciej S. Szmigiero wrote:
>> These checking functions are supposed to be called in order:
> 
> We don't do magical rules like that - you either verify fully and
> correctly or you don't bother at all. And since you're adamant that this
> verification needs to happen, then please do it completely.
> 

These are internal functions to this driver, declared as "static", so
there is no problem if they have additional requirements with respect
to their call order.

But it is, of course, possible to do these checks also in the later
checking functions as you wish.

Maciej

Re: [PATCH v5 2/6] x86/microcode/AMD: Add microcode container data checking functions

[Maciej S. Szmigiero]

On 01.05.2018 10:18, Borislav Petkov wrote:
> On Tue, May 01, 2018 at 12:27:17AM +0200, Maciej S. Szmigiero wrote:
>> These checking functions are supposed to be called in order:
> 
> We don't do magical rules like that - you either verify fully and
> correctly or you don't bother at all. And since you're adamant that this
> verification needs to happen, then please do it completely.
> 

These are internal functions to this driver, declared as "static", so
there is no problem if they have additional requirements with respect
to their call order.

But it is, of course, possible to do these checks also in the later
checking functions as you wish.

Maciej

Re: [PATCH 08/21] bindings: display: Add compatible for A64 HDMI PHY

[Rob Herring]

On Mon, Apr 30, 2018 at 05:10:45PM +0530, Jagan Teki wrote:
> HDMI PHY on Allwinner A64 has similar like H3/H5.
> 
> Signed-off-by: Jagan Teki 
> ---
>  Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt 
> b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> index 9ea4353caadd..7dcd1d64dfe4 100644
> --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> @@ -104,6 +104,7 @@ Required properties:
>- compatible: value must be one of:
>  * allwinner,sun8i-a83t-hdmi-phy
>  * allwinner,sun8i-h3-hdmi-phy
> +* allwinner,sun50i-a64-hdmi-phy

This should be one valid combination per line. The dts shows that the h3 
string is a fallback which should be captured in the binding doc.

>- reg: base address and size of memory-mapped region
>- clocks: phandles to the clocks feeding the HDMI PHY
>  * bus: the HDMI PHY interface clock
> -- 
> 2.14.3
> 

Re: [PATCH 08/21] bindings: display: Add compatible for A64 HDMI PHY

[Rob Herring]

On Mon, Apr 30, 2018 at 05:10:45PM +0530, Jagan Teki wrote:
> HDMI PHY on Allwinner A64 has similar like H3/H5.
> 
> Signed-off-by: Jagan Teki 
> ---
>  Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt 
> b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> index 9ea4353caadd..7dcd1d64dfe4 100644
> --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> @@ -104,6 +104,7 @@ Required properties:
>- compatible: value must be one of:
>  * allwinner,sun8i-a83t-hdmi-phy
>  * allwinner,sun8i-h3-hdmi-phy
> +* allwinner,sun50i-a64-hdmi-phy

This should be one valid combination per line. The dts shows that the h3 
string is a fallback which should be captured in the binding doc.

>- reg: base address and size of memory-mapped region
>- clocks: phandles to the clocks feeding the HDMI PHY
>  * bus: the HDMI PHY interface clock
> -- 
> 2.14.3
> 

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

[Lina Iyer]

On Fri, Apr 27 2018 at 17:24 -0600, Doug Anderson wrote:

Hi,

On Fri, Apr 27, 2018 at 2:54 PM, Matthias Kaehlcke  wrote:

> Am I getting something wrong here?

The for_each_set_bit() should increment the 'i' and we would attempt to
compare the first address in the request with the next command in the
TCS cache. If they don't match we repeat the process again. If it does,
then we loop through 'j' to find if the sequence matches.

Did I miss something?


One of us is clearly in need of more caffeine or ready for the
weekend, it might be me :) Maybe another pair of eyeballs could help

I need them both. Sorry about the back and forth. I understand what the
problem is. The code doesnt look right. I seem to have messed it up.
Thanks Matthias for being patient and going through this.


to resolve this deadlock ...

My single stepping above assumes that tcs->cmd_cache[i] matches
cmd[0].addr, i.e. we either found the start of the sequence we are
looking for or another sequence that starts with the same address. My
claim is that the code returns i in either case, whether the
subsequent addresses match or not.


I haven't reviewed this patch in detail, but I attempted to be another
pair of eyes here.  Something is definitely wrong with the "for (j =
0; j < len; j++)" loop.  I believe the code that's written right now
is equivalent to this much shorter function:

+static int find_match(const struct tcs_group *tcs, const struct tcs_cmd *cmd,
+ int len)
+{
+   int i, j;
+
+   /* Check for already cached commands */
+   for_each_set_bit(i, tcs->slots, MAX_TCS_SLOTS) {
+   if (tcs->cmd_cache[i] == cmd[0].addr)
+   return i;
+   }
+
+   return -ENODATA;
+}

Specifically the test "if (tcs->cmd_cache[i] != cmd[0].addr)" does not
take "j" into account.  Thus if it was false when "j == 0" it will
continue to be false for "j == 1", "j == 2", etc.  Eventually you'll
hit the "else if (j == len - 1)" and return.

I believe that's what Matthias has been saying.  I personally haven't
looked at the rest of the patch to see how things out to be fixed, but
I'm quite convinced that the function either has a bug or should be
written as the shorter version I've written above.


Yes, this is incorrect in its current form. This is what it should be -

static int find_match(const struct tcs_group *tcs, const struct tcs_cmd *cmd,
 int len)
{
   int i, j;

   /* Check for already cached commands */
   for_each_set_bit(i, tcs->slots, MAX_TCS_SLOTS) {
   if (tcs->cmd_cache[i] != cmd[0].addr)
   continue;
   for (j = 0; j < len; j++) {
   WARN(tcs->cmd_cache[i + j] != cmd[j].addr,
"Message does not match previous sequence.\n");
   return -EINVAL;
   }
   if (j == len - 1)
   return i;
   }

   return -ENODATA;
}


Thanks,
Lina

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

[Lina Iyer]

On Fri, Apr 27 2018 at 17:24 -0600, Doug Anderson wrote:

Hi,

On Fri, Apr 27, 2018 at 2:54 PM, Matthias Kaehlcke  wrote:

> Am I getting something wrong here?

The for_each_set_bit() should increment the 'i' and we would attempt to
compare the first address in the request with the next command in the
TCS cache. If they don't match we repeat the process again. If it does,
then we loop through 'j' to find if the sequence matches.

Did I miss something?


One of us is clearly in need of more caffeine or ready for the
weekend, it might be me :) Maybe another pair of eyeballs could help

I need them both. Sorry about the back and forth. I understand what the
problem is. The code doesnt look right. I seem to have messed it up.
Thanks Matthias for being patient and going through this.


to resolve this deadlock ...

My single stepping above assumes that tcs->cmd_cache[i] matches
cmd[0].addr, i.e. we either found the start of the sequence we are
looking for or another sequence that starts with the same address. My
claim is that the code returns i in either case, whether the
subsequent addresses match or not.


I haven't reviewed this patch in detail, but I attempted to be another
pair of eyes here.  Something is definitely wrong with the "for (j =
0; j < len; j++)" loop.  I believe the code that's written right now
is equivalent to this much shorter function:

+static int find_match(const struct tcs_group *tcs, const struct tcs_cmd *cmd,
+ int len)
+{
+   int i, j;
+
+   /* Check for already cached commands */
+   for_each_set_bit(i, tcs->slots, MAX_TCS_SLOTS) {
+   if (tcs->cmd_cache[i] == cmd[0].addr)
+   return i;
+   }
+
+   return -ENODATA;
+}

Specifically the test "if (tcs->cmd_cache[i] != cmd[0].addr)" does not
take "j" into account.  Thus if it was false when "j == 0" it will
continue to be false for "j == 1", "j == 2", etc.  Eventually you'll
hit the "else if (j == len - 1)" and return.

I believe that's what Matthias has been saying.  I personally haven't
looked at the rest of the patch to see how things out to be fixed, but
I'm quite convinced that the function either has a bug or should be
written as the shorter version I've written above.


Yes, this is incorrect in its current form. This is what it should be -

static int find_match(const struct tcs_group *tcs, const struct tcs_cmd *cmd,
 int len)
{
   int i, j;

   /* Check for already cached commands */
   for_each_set_bit(i, tcs->slots, MAX_TCS_SLOTS) {
   if (tcs->cmd_cache[i] != cmd[0].addr)
   continue;
   for (j = 0; j < len; j++) {
   WARN(tcs->cmd_cache[i + j] != cmd[j].addr,
"Message does not match previous sequence.\n");
   return -EINVAL;
   }
   if (j == len - 1)
   return i;
   }

   return -ENODATA;
}


Thanks,
Lina

Re: INFO: task hung in wb_shutdown (2)

[Jens Axboe]

On 5/1/18 4:27 AM, Tetsuo Handa wrote:
> Tejun, Jan, Jens,
> 
> Can you review this patch? syzbot has hit this bug for nearly 4000 times but
> is still unable to find a reproducer. Therefore, the only way to test would be
> to apply this patch upstream and test whether the problem is solved.

I'll review it today.

-- 
Jens Axboe

Re: [PATCH 04/21] bindings: display: Add compatible for A64 DE2 pipeline

[Rob Herring]

On Mon, Apr 30, 2018 at 05:10:41PM +0530, Jagan Teki wrote:
> Allwinner A64 has DE2 pipeline similar to other Allwinner
> SOC's like A83T, H3/H5.

'dt-bindings: ' for the subject prefix.

> 
> Signed-off-by: Jagan Teki 
> ---
>  Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 1 +
>  1 file changed, 1 insertion(+)

Otherwise,

Reviewed-by: Rob Herring 

Re: INFO: task hung in wb_shutdown (2)

[Jens Axboe]

On 5/1/18 4:27 AM, Tetsuo Handa wrote:
> Tejun, Jan, Jens,
> 
> Can you review this patch? syzbot has hit this bug for nearly 4000 times but
> is still unable to find a reproducer. Therefore, the only way to test would be
> to apply this patch upstream and test whether the problem is solved.

I'll review it today.

-- 
Jens Axboe

Re: [PATCH 04/21] bindings: display: Add compatible for A64 DE2 pipeline

[Rob Herring]

On Mon, Apr 30, 2018 at 05:10:41PM +0530, Jagan Teki wrote:
> Allwinner A64 has DE2 pipeline similar to other Allwinner
> SOC's like A83T, H3/H5.

'dt-bindings: ' for the subject prefix.

> 
> Signed-off-by: Jagan Teki 
> ---
>  Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 1 +
>  1 file changed, 1 insertion(+)

Otherwise,

Reviewed-by: Rob Herring 

Re: [PATCH 01/21] dt-bindings: clock: Add compatible for A64 DE2 CCU

[Rob Herring]

On Mon, Apr 30, 2018 at 05:10:38PM +0530, Jagan Teki wrote:
> Allwinner A64 has DE2 CCU which is similar to H3/H5 SoC.
> 
> Signed-off-by: Jagan Teki 
> ---
>  Documentation/devicetree/bindings/clock/sun8i-de2.txt | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Rob Herring 

Re: [PATCH 01/21] dt-bindings: clock: Add compatible for A64 DE2 CCU

[Rob Herring]

On Mon, Apr 30, 2018 at 05:10:38PM +0530, Jagan Teki wrote:
> Allwinner A64 has DE2 CCU which is similar to H3/H5 SoC.
> 
> Signed-off-by: Jagan Teki 
> ---
>  Documentation/devicetree/bindings/clock/sun8i-de2.txt | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Rob Herring 

Re: [PATCH 04/25] arm: exynos/s3c: dts: Remove leading 0x and 0s from bindings notation

[Krzysztof Kozlowski]

On Mon, Dec 18, 2017 at 11:17:24AM +0100, Mathieu Malaterre wrote:
> On Mon, Dec 18, 2017 at 10:40 AM, Krzysztof Kozlowski  wrote:
> > On Fri, Dec 15, 2017 at 1:46 PM, Mathieu Malaterre  wrote:
> >> Improve the DTS files by removing all the leading "0x" and zeros to fix the
> >> following dtc warnings:
> >>
> >> Warning (unit_address_format): Node /XXX unit name should not have leading 
> >> "0x"
> >>
> >> and
> >>
> >> Warning (unit_address_format): Node /XXX unit name should not have leading 
> >> 0s
> >>
> >> Converted using the following command:
> >>
> >> find . -type f \( -iname *.dts -o -iname *.dtsi \) -exec sed -i -e 
> >> "s/@\([0-9a-fA-FxX\.;:#]+\)\s*{/@\L\1 {/g" -e "s/@0x\(.*\) {/@\1 {/g" -e 
> >> "s/@0+\(.*\) {/@\1 {/g" {} +^C
> >>
> >> For simplicity, two sed expressions were used to solve each warnings 
> >> separately.
> >>
> >> To make the regex expression more robust a few other issues were resolved,
> >> namely setting unit-address to lower case, and adding a whitespace before 
> >> the
> >> the opening curly brace:
> >>
> >> https://elinux.org/Device_Tree_Linux#Linux_conventions
> >>
> >> This will solve as a side effect warning:
> >>
> >> Warning (simple_bus_reg): Node /XXX@ simple-bus unit address format 
> >> error, expected ""
> >>
> >> This is a follow up to commit 4c9847b7375a ("dt-bindings: Remove leading 
> >> 0x from bindings notation")
> >>
> >> Reported-by: David Daney 
> >> Suggested-by: Rob Herring 
> >> Acked-by: Krzysztof Kozlowski 
> >
> > Ack was for different patchset, touching only three files...
> 
> So sorry, when I read your email:
> 
> https://lkml.org/lkml/2017/12/15/152
> 
> I assumed you meant for all the Exynos* and S3C* DTS files, but I did
> not check carefully which files were touched originally.
> 
> >> Signed-off-by: Mathieu Malaterre 
> >> ---
> >>  arch/arm/boot/dts/exynos3250.dtsi | 34 ++--
> >>  arch/arm/boot/dts/exynos4.dtsi| 56 +--
> >>  arch/arm/boot/dts/exynos4210.dtsi |  8 +--
> >>  arch/arm/boot/dts/exynos4412-pinctrl.dtsi |  2 +-
> >>  arch/arm/boot/dts/exynos4412.dtsi | 22 
> >>  arch/arm/boot/dts/exynos5.dtsi| 22 
> >>  arch/arm/boot/dts/exynos5250.dtsi | 64 +++---
> >>  arch/arm/boot/dts/exynos5260.dtsi | 26 -
> >>  arch/arm/boot/dts/exynos5420.dtsi | 78 
> >> +--
> >>  arch/arm/boot/dts/exynos5422-odroid-core.dtsi |  2 +-
> >>  arch/arm/boot/dts/exynos5440.dtsi | 14 ++---
> >>  arch/arm/boot/dts/s3c2416.dtsi|  8 +--
> >>  12 files changed, 168 insertions(+), 168 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/exynos3250.dtsi 
> >> b/arch/arm/boot/dts/exynos3250.dtsi
> >> index 2bd3872221a1..8d47571b3984 100644
> >> --- a/arch/arm/boot/dts/exynos3250.dtsi
> >> +++ b/arch/arm/boot/dts/exynos3250.dtsi
> >> @@ -164,31 +164,31 @@
> >> syscon = <_system_controller>;
> >> };
> >>
> >> -   pd_cam: cam-power-domain@10023C00 {
> >> +   pd_cam: cam-power-domain@10023c00 {
> >
> > This is not related to this patch and it was not present in the
> > version I acked. I also already fixed this here:
> > https://patchwork.kernel.org/patch/10113323/
> >
> > There is no changelog explaining the difference in patches. Original
> > patch was okay, why changing it?
> 
> Accept my sincere apologizes I really messed this series. I discover
> my original ARM patch did not apply lower case to all unit-address
> equally, so I added at last minute a sed expression to make all
> unit-address lower case.
> 
> I guess you can just drop this one for now.

Hi Mathieu,

Do you plan to resend this in its original form (removing 0)?

Best regards,
Krzysztof

Re: [PATCH 04/25] arm: exynos/s3c: dts: Remove leading 0x and 0s from bindings notation

[Krzysztof Kozlowski]

On Mon, Dec 18, 2017 at 11:17:24AM +0100, Mathieu Malaterre wrote:
> On Mon, Dec 18, 2017 at 10:40 AM, Krzysztof Kozlowski  wrote:
> > On Fri, Dec 15, 2017 at 1:46 PM, Mathieu Malaterre  wrote:
> >> Improve the DTS files by removing all the leading "0x" and zeros to fix the
> >> following dtc warnings:
> >>
> >> Warning (unit_address_format): Node /XXX unit name should not have leading 
> >> "0x"
> >>
> >> and
> >>
> >> Warning (unit_address_format): Node /XXX unit name should not have leading 
> >> 0s
> >>
> >> Converted using the following command:
> >>
> >> find . -type f \( -iname *.dts -o -iname *.dtsi \) -exec sed -i -e 
> >> "s/@\([0-9a-fA-FxX\.;:#]+\)\s*{/@\L\1 {/g" -e "s/@0x\(.*\) {/@\1 {/g" -e 
> >> "s/@0+\(.*\) {/@\1 {/g" {} +^C
> >>
> >> For simplicity, two sed expressions were used to solve each warnings 
> >> separately.
> >>
> >> To make the regex expression more robust a few other issues were resolved,
> >> namely setting unit-address to lower case, and adding a whitespace before 
> >> the
> >> the opening curly brace:
> >>
> >> https://elinux.org/Device_Tree_Linux#Linux_conventions
> >>
> >> This will solve as a side effect warning:
> >>
> >> Warning (simple_bus_reg): Node /XXX@ simple-bus unit address format 
> >> error, expected ""
> >>
> >> This is a follow up to commit 4c9847b7375a ("dt-bindings: Remove leading 
> >> 0x from bindings notation")
> >>
> >> Reported-by: David Daney 
> >> Suggested-by: Rob Herring 
> >> Acked-by: Krzysztof Kozlowski 
> >
> > Ack was for different patchset, touching only three files...
> 
> So sorry, when I read your email:
> 
> https://lkml.org/lkml/2017/12/15/152
> 
> I assumed you meant for all the Exynos* and S3C* DTS files, but I did
> not check carefully which files were touched originally.
> 
> >> Signed-off-by: Mathieu Malaterre 
> >> ---
> >>  arch/arm/boot/dts/exynos3250.dtsi | 34 ++--
> >>  arch/arm/boot/dts/exynos4.dtsi| 56 +--
> >>  arch/arm/boot/dts/exynos4210.dtsi |  8 +--
> >>  arch/arm/boot/dts/exynos4412-pinctrl.dtsi |  2 +-
> >>  arch/arm/boot/dts/exynos4412.dtsi | 22 
> >>  arch/arm/boot/dts/exynos5.dtsi| 22 
> >>  arch/arm/boot/dts/exynos5250.dtsi | 64 +++---
> >>  arch/arm/boot/dts/exynos5260.dtsi | 26 -
> >>  arch/arm/boot/dts/exynos5420.dtsi | 78 
> >> +--
> >>  arch/arm/boot/dts/exynos5422-odroid-core.dtsi |  2 +-
> >>  arch/arm/boot/dts/exynos5440.dtsi | 14 ++---
> >>  arch/arm/boot/dts/s3c2416.dtsi|  8 +--
> >>  12 files changed, 168 insertions(+), 168 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/exynos3250.dtsi 
> >> b/arch/arm/boot/dts/exynos3250.dtsi
> >> index 2bd3872221a1..8d47571b3984 100644
> >> --- a/arch/arm/boot/dts/exynos3250.dtsi
> >> +++ b/arch/arm/boot/dts/exynos3250.dtsi
> >> @@ -164,31 +164,31 @@
> >> syscon = <_system_controller>;
> >> };
> >>
> >> -   pd_cam: cam-power-domain@10023C00 {
> >> +   pd_cam: cam-power-domain@10023c00 {
> >
> > This is not related to this patch and it was not present in the
> > version I acked. I also already fixed this here:
> > https://patchwork.kernel.org/patch/10113323/
> >
> > There is no changelog explaining the difference in patches. Original
> > patch was okay, why changing it?
> 
> Accept my sincere apologizes I really messed this series. I discover
> my original ARM patch did not apply lower case to all unit-address
> equally, so I added at last minute a sed expression to make all
> unit-address lower case.
> 
> I guess you can just drop this one for now.

Hi Mathieu,

Do you plan to resend this in its original form (removing 0)?

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: Add "sifive" vendor prefix

[Rob Herring]

On Sat, Apr 28, 2018 at 10:37:56PM +, Wesley Terpstra wrote:
> Will do, thanks!

Don't top post to lists.

I've applied this. I assume you'll have things other than the PWM...
 
> On Sat, Apr 28, 2018, 4:21 AM Andreas Färber  wrote:
> 
> > Am 28.04.2018 um 00:59 schrieb Wesley W. Terpstra:
> > > This adds a vendor prefix "sifive" for SiFive, Inc.
> > > We make chips.
> > >
> > > Signed-off-by: Wesley W. Terpstra 
> > > ---
> > >  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> > >  1 file changed, 1 insertion(+)
> >
> > Reviewed-by: Andreas Färber 
> >
> > but please reorder the patches, so that the prefix definition comes
> > before its first use in the pwm binding.
> >
> > Regards,
> > Andreas
> >
> > --
> > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> > GF: Felix Imendörffer, Jane Smithard, Graham Norton
> > HRB 21284 (AG Nürnberg)
> >

Re: [PATCH 2/3] dt-bindings: Add "sifive" vendor prefix

[Rob Herring]

On Sat, Apr 28, 2018 at 10:37:56PM +, Wesley Terpstra wrote:
> Will do, thanks!

Don't top post to lists.

I've applied this. I assume you'll have things other than the PWM...
 
> On Sat, Apr 28, 2018, 4:21 AM Andreas Färber  wrote:
> 
> > Am 28.04.2018 um 00:59 schrieb Wesley W. Terpstra:
> > > This adds a vendor prefix "sifive" for SiFive, Inc.
> > > We make chips.
> > >
> > > Signed-off-by: Wesley W. Terpstra 
> > > ---
> > >  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> > >  1 file changed, 1 insertion(+)
> >
> > Reviewed-by: Andreas Färber 
> >
> > but please reorder the patches, so that the prefix definition comes
> > before its first use in the pwm binding.
> >
> > Regards,
> > Andreas
> >
> > --
> > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> > GF: Felix Imendörffer, Jane Smithard, Graham Norton
> > HRB 21284 (AG Nürnberg)
> >

Re: INFO: task hung in wb_shutdown (2)

[Linus Torvalds]

On Tue, May 1, 2018 at 3:27 AM Tetsuo Handa <
penguin-ker...@i-love.sakura.ne.jp> wrote:

> Can you review this patch? syzbot has hit this bug for nearly 4000 times
but
> is still unable to find a reproducer. Therefore, the only way to test
would be
> to apply this patch upstream and test whether the problem is solved.

Looks ok to me, except:

> >   smp_wmb();
> >   clear_bit(WB_shutting_down, >state);
> > + smp_mb(); /* advised by wake_up_bit() */
> > + wake_up_bit(>state, WB_shutting_down);

This whole sequence really should just be a pattern with a helper function.

And honestly, the pattern probably *should* be

 clear_bit_unlock(bit, );
 smp_mb__after_atomic()
 wake_up_bit(, bit);

which looks like it is a bit cleaner wrt memory ordering rules.

 Linus

Re: INFO: task hung in wb_shutdown (2)

[Linus Torvalds]

On Tue, May 1, 2018 at 3:27 AM Tetsuo Handa <
penguin-ker...@i-love.sakura.ne.jp> wrote:

> Can you review this patch? syzbot has hit this bug for nearly 4000 times
but
> is still unable to find a reproducer. Therefore, the only way to test
would be
> to apply this patch upstream and test whether the problem is solved.

Looks ok to me, except:

> >   smp_wmb();
> >   clear_bit(WB_shutting_down, >state);
> > + smp_mb(); /* advised by wake_up_bit() */
> > + wake_up_bit(>state, WB_shutting_down);

This whole sequence really should just be a pattern with a helper function.

And honestly, the pattern probably *should* be

 clear_bit_unlock(bit, );
 smp_mb__after_atomic()
 wake_up_bit(, bit);

which looks like it is a bit cleaner wrt memory ordering rules.

 Linus

[GIT PULL] xfs: fixes for 4.17-rc4

[Darrick J. Wong]

Hi Linus,

Here are a few more bug fixes for xfs for 4.17-rc4.  Most of them are
fixes for bad behavior.

This series has been run through a full xfstests run during LSF and
through a quick xfstests run against this morning's master, with no
major failures reported.  Let me know if there are any merge problems.

--D

The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:

  Linux 4.17-rc1 (2018-04-15 18:24:20 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git tags/xfs-4.17-fixes-1

for you to fetch changes up to 7b38460dc8e4eafba06c78f8e37099d3b34d473c:

  xfs: don't fail when converting shortform attr to long form during 
ATTR_REPLACE (2018-04-17 19:10:15 -0700)


Changes since last update:
- Enhance inode fork verifiers to prevent loading of corrupted metadata.
- Fix a crash when we try to convert extents format inodes to btree
  format, we run out of space, but forget to revert the in-core state
  changes.
- Fix file size checks when doing INSERT_RANGE that could cause files
  to end up negative size if there previously was an extent mapped at
  s_maxbytes.
- Fix a bug when doing a remove-then-add ATTR_REPLACE xattr update where
  we forget to clear ATTR_REPLACE after the remove, which causes the
  attr to be lost and the fs to shut down due to (what it thinks is)
  inconsistent in-core state.


Darrick J. Wong (2):
  xfs: prevent creating negative-sized file via INSERT_RANGE
  xfs: don't fail when converting shortform attr to long form during 
ATTR_REPLACE

Eric Sandeen (2):
  xfs: enhance dinode verifier
  xfs: set format back to extents if xfs_bmap_extents_to_btree

 fs/xfs/libxfs/xfs_attr.c  |  9 -
 fs/xfs/libxfs/xfs_bmap.c  |  4 
 fs/xfs/libxfs/xfs_inode_buf.c | 21 +
 fs/xfs/xfs_file.c | 14 +-
 4 files changed, 42 insertions(+), 6 deletions(-)

[GIT PULL] xfs: fixes for 4.17-rc4

[Darrick J. Wong]

Hi Linus,

Here are a few more bug fixes for xfs for 4.17-rc4.  Most of them are
fixes for bad behavior.

This series has been run through a full xfstests run during LSF and
through a quick xfstests run against this morning's master, with no
major failures reported.  Let me know if there are any merge problems.

--D

The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:

  Linux 4.17-rc1 (2018-04-15 18:24:20 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git tags/xfs-4.17-fixes-1

for you to fetch changes up to 7b38460dc8e4eafba06c78f8e37099d3b34d473c:

  xfs: don't fail when converting shortform attr to long form during 
ATTR_REPLACE (2018-04-17 19:10:15 -0700)


Changes since last update:
- Enhance inode fork verifiers to prevent loading of corrupted metadata.
- Fix a crash when we try to convert extents format inodes to btree
  format, we run out of space, but forget to revert the in-core state
  changes.
- Fix file size checks when doing INSERT_RANGE that could cause files
  to end up negative size if there previously was an extent mapped at
  s_maxbytes.
- Fix a bug when doing a remove-then-add ATTR_REPLACE xattr update where
  we forget to clear ATTR_REPLACE after the remove, which causes the
  attr to be lost and the fs to shut down due to (what it thinks is)
  inconsistent in-core state.


Darrick J. Wong (2):
  xfs: prevent creating negative-sized file via INSERT_RANGE
  xfs: don't fail when converting shortform attr to long form during 
ATTR_REPLACE

Eric Sandeen (2):
  xfs: enhance dinode verifier
  xfs: set format back to extents if xfs_bmap_extents_to_btree

 fs/xfs/libxfs/xfs_attr.c  |  9 -
 fs/xfs/libxfs/xfs_bmap.c  |  4 
 fs/xfs/libxfs/xfs_inode_buf.c | 21 +
 fs/xfs/xfs_file.c | 14 +-
 4 files changed, 42 insertions(+), 6 deletions(-)

Re: [alsa-devel] [PATCH v2] ASoC: da7219: read fmw property to get mclk for non-dts systems

[Pierre-Louis Bossart]

On 5/1/18 9:31 AM, Agrawal, Akshu wrote:



On 4/30/2018 9:54 PM, Pierre-Louis Bossart wrote:

On 4/30/18 4:23 AM, Akshu Agrawal wrote:

Non-dts based systems can use ACPI DSDT to pass on the mclk
to da7219.
This enables da7219 mclk to be linked to system clock.
Enable/Disable of the mclk is already handled in the codec so
platform drivers don't have to explicitly do handling of mclk.

Signed-off-by: Akshu Agrawal 
---
v2: Fixed kbuild error
  include/sound/da7219.h    | 2 ++
  sound/soc/codecs/da7219.c | 7 ++-
  2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/sound/da7219.h b/include/sound/da7219.h
index 1bfcb16..df7ddf4 100644
--- a/include/sound/da7219.h
+++ b/include/sound/da7219.h
@@ -38,6 +38,8 @@ struct da7219_pdata {
  const char *dai_clks_name;
+    const char *mclk_name;
+
  /* Mic */
  enum da7219_micbias_voltage micbias_lvl;
  enum da7219_mic_amp_in_sel mic_amp_in_sel;
diff --git a/sound/soc/codecs/da7219.c b/sound/soc/codecs/da7219.c
index 980a6a8..aed68a4 100644
--- a/sound/soc/codecs/da7219.c
+++ b/sound/soc/codecs/da7219.c
@@ -1624,6 +1624,8 @@ static struct da7219_pdata 
*da7219_fw_to_pdata(struct snd_soc_component *compone

  dev_warn(dev, "Using default clk name: %s\n",
   pdata->dai_clks_name);
+    device_property_read_string(dev, "dlg,mclk-name", 
>mclk_name);

+
  if (device_property_read_u32(dev, "dlg,micbias-lvl", _val32) 
>= 0)

  pdata->micbias_lvl = da7219_fw_micbias_lvl(dev, of_val32);
  else
@@ -1905,7 +1907,10 @@ static int da7219_probe(struct 
snd_soc_component *component)

  da7219_handle_pdata(component);
  /* Check if MCLK provided */
-    da7219->mclk = devm_clk_get(component->dev, "mclk");
+    if (da7219->pdata->mclk_name)
+    da7219->mclk = clk_get(NULL, da7219->pdata->mclk_name);
+    if (!da7219->mclk)
+    da7219->mclk = devm_clk_get(component->dev, "mclk");


this looks weird, why are you using different clk functions depending 
on the existence of a _DSD property? Why not just change the name and 
keep the same flow, e.g something like


if(!da7219->pdata->mclk_name)
 da7219->pdata->mclk_name = "mclk";
da7219->mclk = devm_clk_get(component->dev, da7219->pdata->mclk_name);




We can't use devm_clk_get as the value of dev argument has to be NULL, 
which can not be used with devm_clk_get.
System clock which are linked to mclk are registered by a separate ACPI 
device. And this exposing of DSD property is for all those platforms 
which are non-dts based.


Alternatively you could modify clk_get to use device_property instead of 
of_property.





  if (IS_ERR(da7219->mclk)) {
  if (PTR_ERR(da7219->mclk) != -ENOENT) {
  ret = PTR_ERR(da7219->mclk);

Re: [alsa-devel] [PATCH v2] ASoC: da7219: read fmw property to get mclk for non-dts systems

[Pierre-Louis Bossart]

On 5/1/18 9:31 AM, Agrawal, Akshu wrote:



On 4/30/2018 9:54 PM, Pierre-Louis Bossart wrote:

On 4/30/18 4:23 AM, Akshu Agrawal wrote:

Non-dts based systems can use ACPI DSDT to pass on the mclk
to da7219.
This enables da7219 mclk to be linked to system clock.
Enable/Disable of the mclk is already handled in the codec so
platform drivers don't have to explicitly do handling of mclk.

Signed-off-by: Akshu Agrawal 
---
v2: Fixed kbuild error
  include/sound/da7219.h    | 2 ++
  sound/soc/codecs/da7219.c | 7 ++-
  2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/sound/da7219.h b/include/sound/da7219.h
index 1bfcb16..df7ddf4 100644
--- a/include/sound/da7219.h
+++ b/include/sound/da7219.h
@@ -38,6 +38,8 @@ struct da7219_pdata {
  const char *dai_clks_name;
+    const char *mclk_name;
+
  /* Mic */
  enum da7219_micbias_voltage micbias_lvl;
  enum da7219_mic_amp_in_sel mic_amp_in_sel;
diff --git a/sound/soc/codecs/da7219.c b/sound/soc/codecs/da7219.c
index 980a6a8..aed68a4 100644
--- a/sound/soc/codecs/da7219.c
+++ b/sound/soc/codecs/da7219.c
@@ -1624,6 +1624,8 @@ static struct da7219_pdata 
*da7219_fw_to_pdata(struct snd_soc_component *compone

  dev_warn(dev, "Using default clk name: %s\n",
   pdata->dai_clks_name);
+    device_property_read_string(dev, "dlg,mclk-name", 
>mclk_name);

+
  if (device_property_read_u32(dev, "dlg,micbias-lvl", _val32) 
>= 0)

  pdata->micbias_lvl = da7219_fw_micbias_lvl(dev, of_val32);
  else
@@ -1905,7 +1907,10 @@ static int da7219_probe(struct 
snd_soc_component *component)

  da7219_handle_pdata(component);
  /* Check if MCLK provided */
-    da7219->mclk = devm_clk_get(component->dev, "mclk");
+    if (da7219->pdata->mclk_name)
+    da7219->mclk = clk_get(NULL, da7219->pdata->mclk_name);
+    if (!da7219->mclk)
+    da7219->mclk = devm_clk_get(component->dev, "mclk");


this looks weird, why are you using different clk functions depending 
on the existence of a _DSD property? Why not just change the name and 
keep the same flow, e.g something like


if(!da7219->pdata->mclk_name)
 da7219->pdata->mclk_name = "mclk";
da7219->mclk = devm_clk_get(component->dev, da7219->pdata->mclk_name);




We can't use devm_clk_get as the value of dev argument has to be NULL, 
which can not be used with devm_clk_get.
System clock which are linked to mclk are registered by a separate ACPI 
device. And this exposing of DSD property is for all those platforms 
which are non-dts based.


Alternatively you could modify clk_get to use device_property instead of 
of_property.





  if (IS_ERR(da7219->mclk)) {
  if (PTR_ERR(da7219->mclk) != -ENOENT) {
  ret = PTR_ERR(da7219->mclk);

[PATCH] ARM: dts: imx: ba16: add "mfg" Q7 SPI-NOR partition

[Sebastian Reichel]

From: Ken Lin 

Add the 4th partiton named "mfg" with a block size 64K to store
manufacturing data.

Signed-off-by: Ken Lin 
Signed-off-by: Sebastian Reichel 
---
 arch/arm/boot/dts/imx6q-ba16.dtsi | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6q-ba16.dtsi 
b/arch/arm/boot/dts/imx6q-ba16.dtsi
index bf4bdb385de9..e903c488287b 100644
--- a/arch/arm/boot/dts/imx6q-ba16.dtsi
+++ b/arch/arm/boot/dts/imx6q-ba16.dtsi
@@ -157,7 +157,12 @@
 
partition@d {
label = "spare";
-   reg = <0xd 0x13>;
+   reg = <0xd 0x32>;
+   };
+
+   partition@3f {
+   label = "mfg";
+   reg = <0x3f 0x1>;
};
};
 };
-- 
2.17.0

[PATCH] ARM: dts: imx: ba16: add "mfg" Q7 SPI-NOR partition

[Sebastian Reichel]

From: Ken Lin 

Add the 4th partiton named "mfg" with a block size 64K to store
manufacturing data.

Signed-off-by: Ken Lin 
Signed-off-by: Sebastian Reichel 
---
 arch/arm/boot/dts/imx6q-ba16.dtsi | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6q-ba16.dtsi 
b/arch/arm/boot/dts/imx6q-ba16.dtsi
index bf4bdb385de9..e903c488287b 100644
--- a/arch/arm/boot/dts/imx6q-ba16.dtsi
+++ b/arch/arm/boot/dts/imx6q-ba16.dtsi
@@ -157,7 +157,12 @@
 
partition@d {
label = "spare";
-   reg = <0xd 0x13>;
+   reg = <0xd 0x32>;
+   };
+
+   partition@3f {
+   label = "mfg";
+   reg = <0x3f 0x1>;
};
};
 };
-- 
2.17.0

Re: [PATCH RFC v5 5/6] tracepoint: Make rcuidle tracepoint callers use SRCU

[Joel Fernandes]

Missed replying to some comments..

On Tue, May 1, 2018 at 7:24 AM Steven Rostedt  wrote:

> On Mon, 30 Apr 2018 18:42:03 -0700
> Joel Fernandes  wrote:

> > In recent tests with IRQ on/off tracepoints, a large performance
> > overhead ~10% is noticed when running hackbench. This is root caused to
> > calls to rcu_irq_enter_irqson and rcu_irq_exit_irqson from the
> > tracepoint code. Following a long discussion on the list [1] about this,
> > we concluded that srcu is a better alternative for use during rcu idle.
> > Although it does involve extra barriers, its lighter than the sched-rcu
> > version which has to do additional RCU calls to notify RCU idle about
> > entry into RCU sections.
> >
> > In this patch, we change the underlying implementation of the
> > trace_*_rcuidle API to use SRCU. This has shown to improve performance
> > alot for the high frequency irq enable/disable tracepoints.

> Can you post some numbers?

Sure, I will post them in the next revision.

> > Test: Tested idle and preempt/irq tracepoints.
> >
> > [1] https://patchwork.kernel.org/patch/10344297/
> > [...]
> >  include/linux/tracepoint.h | 46 +++---
> >  kernel/tracepoint.c| 10 -
> >  2 files changed, 47 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> > index c94f466d57ef..4135e08fb5f1 100644
> > --- a/include/linux/tracepoint.h
> > +++ b/include/linux/tracepoint.h
> > @@ -15,6 +15,7 @@
> >   */
> >
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -33,6 +34,8 @@ struct trace_eval_map {
> >
> >  #define TRACEPOINT_DEFAULT_PRIO  10
> >
> > +extern struct srcu_struct tracepoint_srcu;
> > +
> >  extern int
> >  tracepoint_probe_register(struct tracepoint *tp, void *probe, void
*data);
> >  extern int
> > @@ -77,6 +80,9 @@ int unregister_tracepoint_module_notifier(struct
notifier_block *nb)
> >   */
> >  static inline void tracepoint_synchronize_unregister(void)
> >  {
> > +#ifdef CONFIG_TRACEPOINTS
> > + synchronize_srcu(_srcu);
> > +#endif

> Not related to your patch, but I find it interesting that we don't make
> this function a nop if CONFIG_TRACEPOINTS is not set. Is it because
> something might rely on our implementation that we call
> synchronize_sched here? I think that's a too tight of a coupling for
> others to rely on this, especially since it's not in the comments about
> this function.

If there's no CONFIG_TRACEPOINTS, then nothing should be replying on the
implementation?

Basically, if !TRACEPOINTS, then there shouldn't be any active rcu read
sections calling probes.

> Again, not related to this series, but something we should probably
> consider in the future. It would require auditing users of this too.

Yes, probably could be a noop in the future.



> >   synchronize_sched();
> >  }
> >
> > @@ -129,18 +135,38 @@ extern void syscall_unregfunc(void);
> >   * as "(void *, void)". The DECLARE_TRACE_NOARGS() will pass in just
> >   * "void *data", where as the DECLARE_TRACE() will pass in "void
*data, proto".
> >   */
> > -#define __DO_TRACE(tp, proto, args, cond, rcucheck)  \
> > +#define __DO_TRACE(tp, proto, args, cond, rcuidle)   \
> >   do {\
> >   struct tracepoint_func *it_func_ptr;\
> >   void *it_func;  \
> >   void *__data;   \
> > + int __maybe_unused idx = 0; \
> >   \
> >   if (!(cond))\
> >   return; \
> > - if (rcucheck)   \
> > - rcu_irq_enter_irqson(); \
> > - rcu_read_lock_sched_notrace();  \
> > - it_func_ptr = rcu_dereference_sched((tp)->funcs);   \
> > + \
> > + /*  \
> > +  * For rcuidle callers, use srcu since sched-rcu\
> > +  * doesn't work from the idle path. \
> > +  */ \
> > + if (rcuidle) {  \
> > + if (in_nmi()) { \
> > + WARN_ON_ONCE(1);\
> > + return; /* no srcu from nmi */  \
> > + }   \
> > +  

Re: [PATCH RFC v5 5/6] tracepoint: Make rcuidle tracepoint callers use SRCU

[Joel Fernandes]

Missed replying to some comments..

On Tue, May 1, 2018 at 7:24 AM Steven Rostedt  wrote:

> On Mon, 30 Apr 2018 18:42:03 -0700
> Joel Fernandes  wrote:

> > In recent tests with IRQ on/off tracepoints, a large performance
> > overhead ~10% is noticed when running hackbench. This is root caused to
> > calls to rcu_irq_enter_irqson and rcu_irq_exit_irqson from the
> > tracepoint code. Following a long discussion on the list [1] about this,
> > we concluded that srcu is a better alternative for use during rcu idle.
> > Although it does involve extra barriers, its lighter than the sched-rcu
> > version which has to do additional RCU calls to notify RCU idle about
> > entry into RCU sections.
> >
> > In this patch, we change the underlying implementation of the
> > trace_*_rcuidle API to use SRCU. This has shown to improve performance
> > alot for the high frequency irq enable/disable tracepoints.

> Can you post some numbers?

Sure, I will post them in the next revision.

> > Test: Tested idle and preempt/irq tracepoints.
> >
> > [1] https://patchwork.kernel.org/patch/10344297/
> > [...]
> >  include/linux/tracepoint.h | 46 +++---
> >  kernel/tracepoint.c| 10 -
> >  2 files changed, 47 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> > index c94f466d57ef..4135e08fb5f1 100644
> > --- a/include/linux/tracepoint.h
> > +++ b/include/linux/tracepoint.h
> > @@ -15,6 +15,7 @@
> >   */
> >
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -33,6 +34,8 @@ struct trace_eval_map {
> >
> >  #define TRACEPOINT_DEFAULT_PRIO  10
> >
> > +extern struct srcu_struct tracepoint_srcu;
> > +
> >  extern int
> >  tracepoint_probe_register(struct tracepoint *tp, void *probe, void
*data);
> >  extern int
> > @@ -77,6 +80,9 @@ int unregister_tracepoint_module_notifier(struct
notifier_block *nb)
> >   */
> >  static inline void tracepoint_synchronize_unregister(void)
> >  {
> > +#ifdef CONFIG_TRACEPOINTS
> > + synchronize_srcu(_srcu);
> > +#endif

> Not related to your patch, but I find it interesting that we don't make
> this function a nop if CONFIG_TRACEPOINTS is not set. Is it because
> something might rely on our implementation that we call
> synchronize_sched here? I think that's a too tight of a coupling for
> others to rely on this, especially since it's not in the comments about
> this function.

If there's no CONFIG_TRACEPOINTS, then nothing should be replying on the
implementation?

Basically, if !TRACEPOINTS, then there shouldn't be any active rcu read
sections calling probes.

> Again, not related to this series, but something we should probably
> consider in the future. It would require auditing users of this too.

Yes, probably could be a noop in the future.



> >   synchronize_sched();
> >  }
> >
> > @@ -129,18 +135,38 @@ extern void syscall_unregfunc(void);
> >   * as "(void *, void)". The DECLARE_TRACE_NOARGS() will pass in just
> >   * "void *data", where as the DECLARE_TRACE() will pass in "void
*data, proto".
> >   */
> > -#define __DO_TRACE(tp, proto, args, cond, rcucheck)  \
> > +#define __DO_TRACE(tp, proto, args, cond, rcuidle)   \
> >   do {\
> >   struct tracepoint_func *it_func_ptr;\
> >   void *it_func;  \
> >   void *__data;   \
> > + int __maybe_unused idx = 0; \
> >   \
> >   if (!(cond))\
> >   return; \
> > - if (rcucheck)   \
> > - rcu_irq_enter_irqson(); \
> > - rcu_read_lock_sched_notrace();  \
> > - it_func_ptr = rcu_dereference_sched((tp)->funcs);   \
> > + \
> > + /*  \
> > +  * For rcuidle callers, use srcu since sched-rcu\
> > +  * doesn't work from the idle path. \
> > +  */ \
> > + if (rcuidle) {  \
> > + if (in_nmi()) { \
> > + WARN_ON_ONCE(1);\
> > + return; /* no srcu from nmi */  \
> > + }   \
> > +  

Re: [linux-sunxi] [PATCH 3/3] arm64: allwinner: h6: enable MMC0/2 on Pine H64

[Chen-Yu Tsai]

On Mon, Apr 30, 2018 at 6:44 PM, Andre Przywara  wrote:
> Hi,
>
> On 30/04/18 10:51, Icenowy Zheng wrote:
>>
>>
>> 于 2018年4月30日 GMT+08:00 下午5:47:35, Andre Przywara  写到:
>>> Hi Icenowy,
>>>
>>> On 27/04/18 08:12, Icenowy Zheng wrote:


 于 2018年4月27日 GMT+08:00 上午12:46:26, Andre Przywara
>>>  写到:
> Hi,
>
> On 26/04/18 15:07, Icenowy Zheng wrote:
>> The Pine H64 board have a MicroSD slot connected to MMC0 controller
> of
>> the H6 SoC and a eMMC slot connected to MMC2.
>>
>> Enable them in the device tree.
>>
>> Signed-off-by: Icenowy Zheng 
>> ---
>>  .../boot/dts/allwinner/sun50i-h6-pine-h64.dts  | 32
> ++
>>  1 file changed, 32 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
> b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
>> index d36de5eb81f3..78b1cd54687c 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
>> @@ -20,6 +20,38 @@
>>   chosen {
>>   stdout-path = "serial0:115200n8";
>>   };
>> +
>> + reg_vcc3v3: vcc3v3 {
>> + compatible = "regulator-fixed";
>> + regulator-name = "vcc3v3";
>> + regulator-min-microvolt = <330>;
>> + regulator-max-microvolt = <330>;
>> + };
>> +
>> + reg_vcc1v8: vcc1v8 {
>> + compatible = "regulator-fixed";
>> + regulator-name = "vcc1v8";
>> + regulator-min-microvolt = <180>;
>> + regulator-max-microvolt = <180>;
>> + };
>> +};
>> +
>> + {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <_pins>;
>> + vmmc-supply = <_vcc3v3>;
>
> So this is actually CLDO1 on the AXP, correct?

 I remember it's coupled between two LDOs, to provide enough power.

>
>
>> + cd-gpios = < 5 6 GPIO_ACTIVE_LOW>;
>> + status = "okay";
>> +};
>> +
>> + {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <_pins>;
>> + vmmc-supply = <_vcc3v3>;
>> + vqmmc-supply = <_vcc1v8>;
>
> And this is BLDO2?

 Yes.

>
> I am just asking because I want to avoid running into the same
>>> problem
> as with the A64 before: that future DTs become incompatible with
>>> older
> kernels, because we change the power supply to point to the AXP
> regulators, which this kernel does not support yet.

 The answer is just not to keep this compatibility, as it's not
 supported option to update DT without updating kernel.
>>>
>>> Well, I recognise that statement.. ;-) and I understand that it's far
>>> easier to handle it this way. But:
>>> - Which .dtb are we going to write into the SPI flash? An older one,
>>> which covers all kernels, but lacks features? Or a newer one, which
>>> limits the bootable kernels to recent versions?
>>> - Which DT are we going to give to EFI applications?
>>> - Which DT are the BSDs suspected to take? They don't ship their own
>>> DTs
>>> (which is good!).
>>>
>>> So I understand that "shipping the DT with the kernel" is the old
>>> (embedded!) way of doing things, but I really believe we should stop
>>> relying on this and try to come up with backwards compatible DTs, which
>>> live in the firmware and get updated there. Because this is what the
>>> distros seem to expect from ARM64 boards these days.
>>
>> I think in this way we should change the way to submit
>> patches -- let DT binding patch be merged when it's ready,
>> and do not wait for driver.
>
> Yes, I agree. Ideally we would look at the hardware description, create
> a binding just based on that and submit it.
>
> Then the actual DTs and the drivers (for Linux, U-Boot, *BSD,
> you-name-it) could be done independently from each other.
>
> I think we should really aim for that. The only question is whether this
> is really practical, since the documentation is sometimes lacking and we
> may discover missing properties during driver development.
> So when we meanwhile do hand-in-hand development, we should make sure we
> don't break anything in the future.

We could do that, but for critical regulators it's a bit tricky. Like the
issue with vmmc and vqmmc, where the driver for the regulator is missing,
leading to an unusable system.

 P.S. I think the DT will update twice on the kernel side, the
 first time keep reg_vcc3v3 (as it's coupled) but use real
 regulator for reg_vcc1v8, the second time use the real
 coupled regulator for reg_vcc3v3.

>
> It looks like there are more users of those power rails, so we could
> keep those supplies connected to these fixed regulators here, even
>>> with
> AXP-805 support in the kernel.

 It's not a good choice.

>
> Or we keep this 

Re: [linux-sunxi] [PATCH 3/3] arm64: allwinner: h6: enable MMC0/2 on Pine H64

[Chen-Yu Tsai]

On Mon, Apr 30, 2018 at 6:44 PM, Andre Przywara  wrote:
> Hi,
>
> On 30/04/18 10:51, Icenowy Zheng wrote:
>>
>>
>> 于 2018年4月30日 GMT+08:00 下午5:47:35, Andre Przywara  写到:
>>> Hi Icenowy,
>>>
>>> On 27/04/18 08:12, Icenowy Zheng wrote:


 于 2018年4月27日 GMT+08:00 上午12:46:26, Andre Przywara
>>>  写到:
> Hi,
>
> On 26/04/18 15:07, Icenowy Zheng wrote:
>> The Pine H64 board have a MicroSD slot connected to MMC0 controller
> of
>> the H6 SoC and a eMMC slot connected to MMC2.
>>
>> Enable them in the device tree.
>>
>> Signed-off-by: Icenowy Zheng 
>> ---
>>  .../boot/dts/allwinner/sun50i-h6-pine-h64.dts  | 32
> ++
>>  1 file changed, 32 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
> b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
>> index d36de5eb81f3..78b1cd54687c 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
>> @@ -20,6 +20,38 @@
>>   chosen {
>>   stdout-path = "serial0:115200n8";
>>   };
>> +
>> + reg_vcc3v3: vcc3v3 {
>> + compatible = "regulator-fixed";
>> + regulator-name = "vcc3v3";
>> + regulator-min-microvolt = <330>;
>> + regulator-max-microvolt = <330>;
>> + };
>> +
>> + reg_vcc1v8: vcc1v8 {
>> + compatible = "regulator-fixed";
>> + regulator-name = "vcc1v8";
>> + regulator-min-microvolt = <180>;
>> + regulator-max-microvolt = <180>;
>> + };
>> +};
>> +
>> + {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <_pins>;
>> + vmmc-supply = <_vcc3v3>;
>
> So this is actually CLDO1 on the AXP, correct?

 I remember it's coupled between two LDOs, to provide enough power.

>
>
>> + cd-gpios = < 5 6 GPIO_ACTIVE_LOW>;
>> + status = "okay";
>> +};
>> +
>> + {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <_pins>;
>> + vmmc-supply = <_vcc3v3>;
>> + vqmmc-supply = <_vcc1v8>;
>
> And this is BLDO2?

 Yes.

>
> I am just asking because I want to avoid running into the same
>>> problem
> as with the A64 before: that future DTs become incompatible with
>>> older
> kernels, because we change the power supply to point to the AXP
> regulators, which this kernel does not support yet.

 The answer is just not to keep this compatibility, as it's not
 supported option to update DT without updating kernel.
>>>
>>> Well, I recognise that statement.. ;-) and I understand that it's far
>>> easier to handle it this way. But:
>>> - Which .dtb are we going to write into the SPI flash? An older one,
>>> which covers all kernels, but lacks features? Or a newer one, which
>>> limits the bootable kernels to recent versions?
>>> - Which DT are we going to give to EFI applications?
>>> - Which DT are the BSDs suspected to take? They don't ship their own
>>> DTs
>>> (which is good!).
>>>
>>> So I understand that "shipping the DT with the kernel" is the old
>>> (embedded!) way of doing things, but I really believe we should stop
>>> relying on this and try to come up with backwards compatible DTs, which
>>> live in the firmware and get updated there. Because this is what the
>>> distros seem to expect from ARM64 boards these days.
>>
>> I think in this way we should change the way to submit
>> patches -- let DT binding patch be merged when it's ready,
>> and do not wait for driver.
>
> Yes, I agree. Ideally we would look at the hardware description, create
> a binding just based on that and submit it.
>
> Then the actual DTs and the drivers (for Linux, U-Boot, *BSD,
> you-name-it) could be done independently from each other.
>
> I think we should really aim for that. The only question is whether this
> is really practical, since the documentation is sometimes lacking and we
> may discover missing properties during driver development.
> So when we meanwhile do hand-in-hand development, we should make sure we
> don't break anything in the future.

We could do that, but for critical regulators it's a bit tricky. Like the
issue with vmmc and vqmmc, where the driver for the regulator is missing,
leading to an unusable system.

 P.S. I think the DT will update twice on the kernel side, the
 first time keep reg_vcc3v3 (as it's coupled) but use real
 regulator for reg_vcc1v8, the second time use the real
 coupled regulator for reg_vcc3v3.

>
> It looks like there are more users of those power rails, so we could
> keep those supplies connected to these fixed regulators here, even
>>> with
> AXP-805 support in the kernel.

 It's not a good choice.

>
> Or we keep this back until we get proper AXP support in the kernel?
>>> I
> guess it's quite close to 

Re: [linux-sunxi] [PATCH 3/3] arm64: allwinner: h6: enable MMC0/2 on Pine H64

[Chen-Yu Tsai]

On Mon, Apr 30, 2018 at 5:47 PM, Andre Przywara  wrote:
> Hi Icenowy,
>
> On 27/04/18 08:12, Icenowy Zheng wrote:
>>
>>
>> 于 2018年4月27日 GMT+08:00 上午12:46:26, Andre Przywara  
>> 写到:
>>> Hi,
>>>
>>> On 26/04/18 15:07, Icenowy Zheng wrote:
 The Pine H64 board have a MicroSD slot connected to MMC0 controller
>>> of
 the H6 SoC and a eMMC slot connected to MMC2.

 Enable them in the device tree.

 Signed-off-by: Icenowy Zheng 
 ---
  .../boot/dts/allwinner/sun50i-h6-pine-h64.dts  | 32
>>> ++
  1 file changed, 32 insertions(+)

 diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
>>> b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
 index d36de5eb81f3..78b1cd54687c 100644
 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
 +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
 @@ -20,6 +20,38 @@
 chosen {
 stdout-path = "serial0:115200n8";
 };
 +
 +   reg_vcc3v3: vcc3v3 {
 +   compatible = "regulator-fixed";
 +   regulator-name = "vcc3v3";
 +   regulator-min-microvolt = <330>;
 +   regulator-max-microvolt = <330>;
 +   };
 +
 +   reg_vcc1v8: vcc1v8 {
 +   compatible = "regulator-fixed";
 +   regulator-name = "vcc1v8";
 +   regulator-min-microvolt = <180>;
 +   regulator-max-microvolt = <180>;
 +   };
 +};
 +
 + {
 +   pinctrl-names = "default";
 +   pinctrl-0 = <_pins>;
 +   vmmc-supply = <_vcc3v3>;
>>>
>>> So this is actually CLDO1 on the AXP, correct?
>>
>> I remember it's coupled between two LDOs, to provide enough power.
>>
>>>
>>>
 +   cd-gpios = < 5 6 GPIO_ACTIVE_LOW>;
 +   status = "okay";
 +};
 +
 + {
 +   pinctrl-names = "default";
 +   pinctrl-0 = <_pins>;
 +   vmmc-supply = <_vcc3v3>;
 +   vqmmc-supply = <_vcc1v8>;
>>>
>>> And this is BLDO2?
>>
>> Yes.
>>
>>>
>>> I am just asking because I want to avoid running into the same problem
>>> as with the A64 before: that future DTs become incompatible with older
>>> kernels, because we change the power supply to point to the AXP
>>> regulators, which this kernel does not support yet.
>>
>> The answer is just not to keep this compatibility, as it's not
>> supported option to update DT without updating kernel.
>
> Well, I recognise that statement.. ;-) and I understand that it's far
> easier to handle it this way. But:
> - Which .dtb are we going to write into the SPI flash? An older one,
> which covers all kernels, but lacks features? Or a newer one, which
> limits the bootable kernels to recent versions?
> - Which DT are we going to give to EFI applications?
> - Which DT are the BSDs suspected to take? They don't ship their own DTs
> (which is good!).
>
> So I understand that "shipping the DT with the kernel" is the old
> (embedded!) way of doing things, but I really believe we should stop
> relying on this and try to come up with backwards compatible DTs, which
> live in the firmware and get updated there. Because this is what the
> distros seem to expect from ARM64 boards these days.
>
>> P.S. I think the DT will update twice on the kernel side, the
>> first time keep reg_vcc3v3 (as it's coupled) but use real
>> regulator for reg_vcc1v8, the second time use the real
>> coupled regulator for reg_vcc3v3.
>>
>>>
>>> It looks like there are more users of those power rails, so we could
>>> keep those supplies connected to these fixed regulators here, even with
>>> AXP-805 support in the kernel.
>>
>> It's not a good choice.
>>
>>>
>>> Or we keep this back until we get proper AXP support in the kernel? I
>>> guess it's quite close to the existing PMICs, so it might be more a
>>> copy exercise to support the AXP-805?
>>
>> It's not a reason to keep it back.
>
> So I compared the manuals of the AXP806 and the AXP805, the register
> interface looks identical to me. I only have a (somewhat) Chinese
> version of the AXP806 manual, so couldn't really find the difference
> between the two. Do you know more about it? Is it just maybe the
> packaging and the electrical properties (like max current supported)?

>From what I could tell, they are the same. I'm not sure about max current,
but that's something not described in the device tree right now. FYI,
the AXP221s and AXP221 differ slightly in that one has increased current
capability for one or two of the DC-DCs. We treat them as if they were
the same right now.

> If the I2C register interface is really the same, we could just add the
> DT nodes for the regulator and be done.

It is. However, we currently lack a "standalone operating mode" property.
This is different from "master mode".

ChenYu

> Cheers,
> Andre.
>
>>
>>>
>>> But apart from this this looks correct to me.
>>>
>>> Cheers,
>>> Andre.
>>>
 +   

Re: [linux-sunxi] [PATCH 3/3] arm64: allwinner: h6: enable MMC0/2 on Pine H64

[Chen-Yu Tsai]

On Mon, Apr 30, 2018 at 5:47 PM, Andre Przywara  wrote:
> Hi Icenowy,
>
> On 27/04/18 08:12, Icenowy Zheng wrote:
>>
>>
>> 于 2018年4月27日 GMT+08:00 上午12:46:26, Andre Przywara  
>> 写到:
>>> Hi,
>>>
>>> On 26/04/18 15:07, Icenowy Zheng wrote:
 The Pine H64 board have a MicroSD slot connected to MMC0 controller
>>> of
 the H6 SoC and a eMMC slot connected to MMC2.

 Enable them in the device tree.

 Signed-off-by: Icenowy Zheng 
 ---
  .../boot/dts/allwinner/sun50i-h6-pine-h64.dts  | 32
>>> ++
  1 file changed, 32 insertions(+)

 diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
>>> b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
 index d36de5eb81f3..78b1cd54687c 100644
 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
 +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
 @@ -20,6 +20,38 @@
 chosen {
 stdout-path = "serial0:115200n8";
 };
 +
 +   reg_vcc3v3: vcc3v3 {
 +   compatible = "regulator-fixed";
 +   regulator-name = "vcc3v3";
 +   regulator-min-microvolt = <330>;
 +   regulator-max-microvolt = <330>;
 +   };
 +
 +   reg_vcc1v8: vcc1v8 {
 +   compatible = "regulator-fixed";
 +   regulator-name = "vcc1v8";
 +   regulator-min-microvolt = <180>;
 +   regulator-max-microvolt = <180>;
 +   };
 +};
 +
 + {
 +   pinctrl-names = "default";
 +   pinctrl-0 = <_pins>;
 +   vmmc-supply = <_vcc3v3>;
>>>
>>> So this is actually CLDO1 on the AXP, correct?
>>
>> I remember it's coupled between two LDOs, to provide enough power.
>>
>>>
>>>
 +   cd-gpios = < 5 6 GPIO_ACTIVE_LOW>;
 +   status = "okay";
 +};
 +
 + {
 +   pinctrl-names = "default";
 +   pinctrl-0 = <_pins>;
 +   vmmc-supply = <_vcc3v3>;
 +   vqmmc-supply = <_vcc1v8>;
>>>
>>> And this is BLDO2?
>>
>> Yes.
>>
>>>
>>> I am just asking because I want to avoid running into the same problem
>>> as with the A64 before: that future DTs become incompatible with older
>>> kernels, because we change the power supply to point to the AXP
>>> regulators, which this kernel does not support yet.
>>
>> The answer is just not to keep this compatibility, as it's not
>> supported option to update DT without updating kernel.
>
> Well, I recognise that statement.. ;-) and I understand that it's far
> easier to handle it this way. But:
> - Which .dtb are we going to write into the SPI flash? An older one,
> which covers all kernels, but lacks features? Or a newer one, which
> limits the bootable kernels to recent versions?
> - Which DT are we going to give to EFI applications?
> - Which DT are the BSDs suspected to take? They don't ship their own DTs
> (which is good!).
>
> So I understand that "shipping the DT with the kernel" is the old
> (embedded!) way of doing things, but I really believe we should stop
> relying on this and try to come up with backwards compatible DTs, which
> live in the firmware and get updated there. Because this is what the
> distros seem to expect from ARM64 boards these days.
>
>> P.S. I think the DT will update twice on the kernel side, the
>> first time keep reg_vcc3v3 (as it's coupled) but use real
>> regulator for reg_vcc1v8, the second time use the real
>> coupled regulator for reg_vcc3v3.
>>
>>>
>>> It looks like there are more users of those power rails, so we could
>>> keep those supplies connected to these fixed regulators here, even with
>>> AXP-805 support in the kernel.
>>
>> It's not a good choice.
>>
>>>
>>> Or we keep this back until we get proper AXP support in the kernel? I
>>> guess it's quite close to the existing PMICs, so it might be more a
>>> copy exercise to support the AXP-805?
>>
>> It's not a reason to keep it back.
>
> So I compared the manuals of the AXP806 and the AXP805, the register
> interface looks identical to me. I only have a (somewhat) Chinese
> version of the AXP806 manual, so couldn't really find the difference
> between the two. Do you know more about it? Is it just maybe the
> packaging and the electrical properties (like max current supported)?

>From what I could tell, they are the same. I'm not sure about max current,
but that's something not described in the device tree right now. FYI,
the AXP221s and AXP221 differ slightly in that one has increased current
capability for one or two of the DC-DCs. We treat them as if they were
the same right now.

> If the I2C register interface is really the same, we could just add the
> DT nodes for the regulator and be done.

It is. However, we currently lack a "standalone operating mode" property.
This is different from "master mode".

ChenYu

> Cheers,
> Andre.
>
>>
>>>
>>> But apart from this this looks correct to me.
>>>
>>> Cheers,
>>> Andre.
>>>
 +   non-removable;
 +   cap-mmc-hw-reset;
 +   status = 

Re: [PATCH RFC v5 5/6] tracepoint: Make rcuidle tracepoint callers use SRCU

[Paul E. McKenney]

On Tue, May 01, 2018 at 03:23:52PM +, Joel Fernandes wrote:
> On Tue, May 1, 2018 at 8:20 AM Paul E. McKenney 
> wrote:
> [...]
> > > > > > --- a/kernel/tracepoint.c
> > > > > > +++ b/kernel/tracepoint.c
> > > > > > @@ -31,6 +31,9 @@
> > > > > >  extern struct tracepoint * const __start___tracepoints_ptrs[];
> > > > > >  extern struct tracepoint * const __stop___tracepoints_ptrs[];
> > > > > >
> > > > > > +DEFINE_SRCU(tracepoint_srcu);
> > > > > > +EXPORT_SYMBOL_GPL(tracepoint_srcu);
> > > > > > +
> > > > > >  /* Set to 1 to enable tracepoint debug output */
> > > > > >  static const int tracepoint_debug;
> > > > > >
> > > > > > @@ -67,11 +70,16 @@ static inline void *allocate_probes(int count)
> > > > > > return p == NULL ? NULL : p->probes;
> > > > > >  }
> > > > > >
> > > > > > -static void rcu_free_old_probes(struct rcu_head *head)
> > > > > > +static void srcu_free_old_probes(struct rcu_head *head)
> > > > > >  {
> > > > > > kfree(container_of(head, struct tp_probes, rcu));
> > > > > >  }
> > > > > >
> > > > > > +static void rcu_free_old_probes(struct rcu_head *head)
> > > > > > +{
> > > > > > +   call_srcu(_srcu, head, srcu_free_old_probes);
> > > > >
> > > > > Hmm, is it OK to call call_srcu() from a call_rcu() callback? I
> guess
> > > > > it would be.
> > >
> > > > It is perfectly legal, and quite a bit simpler than setting something
> > > > up to wait for both to complete concurrently.
> > >
> > > Cool. Also in this case if we call both in sequence, then I felt there
> > > could be a race to free the old data since both callbacks would try to
> do
> > > the same thing. The same thing being freeing of the same set of old
> probes
> > > which would need some synchronization between the 2 callbacks. With the
> > > chaining, since the ordering is assured there wouldn't be a question of
> > > such a race. I could add this reasoning to the changelog as well.
> 
> > Actually, as long as you have a solid happens-before between both of the
> > callbacks and the freeing, you are in good shape.  A release-acquire would
> > work fine, as would a lock acquired in both callbacks and then acquired
> > (and possibly released) before the free.
> 
> Got it, thanks. For now, if its Ok with you and others, I will leave it in
> the chained configuration. I also feel this is temporary since in the
> future if we switch to single rcu mechanism for tracepoints (srcu), then we
> could do with just a single callback.

I have no problem with chained callbacks.  Heck, I even test them in
rcutorture.  ;-)

Thanx, Paul

Re: [PATCH RFC v5 5/6] tracepoint: Make rcuidle tracepoint callers use SRCU

[Paul E. McKenney]

On Tue, May 01, 2018 at 03:23:52PM +, Joel Fernandes wrote:
> On Tue, May 1, 2018 at 8:20 AM Paul E. McKenney 
> wrote:
> [...]
> > > > > > --- a/kernel/tracepoint.c
> > > > > > +++ b/kernel/tracepoint.c
> > > > > > @@ -31,6 +31,9 @@
> > > > > >  extern struct tracepoint * const __start___tracepoints_ptrs[];
> > > > > >  extern struct tracepoint * const __stop___tracepoints_ptrs[];
> > > > > >
> > > > > > +DEFINE_SRCU(tracepoint_srcu);
> > > > > > +EXPORT_SYMBOL_GPL(tracepoint_srcu);
> > > > > > +
> > > > > >  /* Set to 1 to enable tracepoint debug output */
> > > > > >  static const int tracepoint_debug;
> > > > > >
> > > > > > @@ -67,11 +70,16 @@ static inline void *allocate_probes(int count)
> > > > > > return p == NULL ? NULL : p->probes;
> > > > > >  }
> > > > > >
> > > > > > -static void rcu_free_old_probes(struct rcu_head *head)
> > > > > > +static void srcu_free_old_probes(struct rcu_head *head)
> > > > > >  {
> > > > > > kfree(container_of(head, struct tp_probes, rcu));
> > > > > >  }
> > > > > >
> > > > > > +static void rcu_free_old_probes(struct rcu_head *head)
> > > > > > +{
> > > > > > +   call_srcu(_srcu, head, srcu_free_old_probes);
> > > > >
> > > > > Hmm, is it OK to call call_srcu() from a call_rcu() callback? I
> guess
> > > > > it would be.
> > >
> > > > It is perfectly legal, and quite a bit simpler than setting something
> > > > up to wait for both to complete concurrently.
> > >
> > > Cool. Also in this case if we call both in sequence, then I felt there
> > > could be a race to free the old data since both callbacks would try to
> do
> > > the same thing. The same thing being freeing of the same set of old
> probes
> > > which would need some synchronization between the 2 callbacks. With the
> > > chaining, since the ordering is assured there wouldn't be a question of
> > > such a race. I could add this reasoning to the changelog as well.
> 
> > Actually, as long as you have a solid happens-before between both of the
> > callbacks and the freeing, you are in good shape.  A release-acquire would
> > work fine, as would a lock acquired in both callbacks and then acquired
> > (and possibly released) before the free.
> 
> Got it, thanks. For now, if its Ok with you and others, I will leave it in
> the chained configuration. I also feel this is temporary since in the
> future if we switch to single rcu mechanism for tracepoints (srcu), then we
> could do with just a single callback.

I have no problem with chained callbacks.  Heck, I even test them in
rcutorture.  ;-)

Thanx, Paul

Re: Suboptimal inline heuristics due to non-code sections

[Linus Torvalds]

On Tue, May 1, 2018 at 6:40 AM Josh Poimboeuf  wrote:

> But if I remove the section completely by removing the
> pushsection/popsection, then copy_overflow() gets inlined.

> So GCC's inlining decisions are somehow influenced by the existence of
> some random empty section.  This definitely seems like a GCC bug to me.

I think gcc uses the size of the string to approximate the size of an
inline asm.

So I don't think it's the "empty section" that makes gcc do this, I think
it's literally "our inline asms _look_ big".

Linus "does this section directive make me look fat?"
Torvalds

Re: Suboptimal inline heuristics due to non-code sections

[Linus Torvalds]

On Tue, May 1, 2018 at 6:40 AM Josh Poimboeuf  wrote:

> But if I remove the section completely by removing the
> pushsection/popsection, then copy_overflow() gets inlined.

> So GCC's inlining decisions are somehow influenced by the existence of
> some random empty section.  This definitely seems like a GCC bug to me.

I think gcc uses the size of the string to approximate the size of an
inline asm.

So I don't think it's the "empty section" that makes gcc do this, I think
it's literally "our inline asms _look_ big".

Linus "does this section directive make me look fat?"
Torvalds

Re: [PATCH] RDMA/qedr: fix spelling mistake: "failes" -> "fails"

[Doug Ledford]

On Tue, 2018-05-01 at 09:25 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Trivial fix to spelling mistake in DP_ERR error message
> 
> Signed-off-by: Colin Ian King 

Thanks, applied.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH] RDMA/qedr: fix spelling mistake: "failes" -> "fails"

[Doug Ledford]

On Tue, 2018-05-01 at 09:25 +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Trivial fix to spelling mistake in DP_ERR error message
> 
> Signed-off-by: Colin Ian King 

Thanks, applied.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [v5 2/2] clk: qcom: clk-rpmh: Add QCOM RPMh clock driver

[Taniya Das]

On 5/1/2018 6:13 PM, Rob Herring wrote:

On Tue, May 01, 2018 at 02:11:33PM +0530, Taniya Das wrote:

Add the RPMh clock driver to control the RPMh managed clock resources on
some of the Qualcomm Technologies, Inc. SoCs.

Signed-off-by: Amit Nischal 
Signed-off-by: Taniya Das 
---
  drivers/clk/qcom/Kconfig  |   9 +
  drivers/clk/qcom/Makefile |   1 +
  drivers/clk/qcom/clk-rpmh.c   | 387 ++
  include/dt-bindings/clock/qcom,rpmh.h |   2 -
  4 files changed, 397 insertions(+), 2 deletions(-)
  create mode 100644 drivers/clk/qcom/clk-rpmh.c



diff --git a/include/dt-bindings/clock/qcom,rpmh.h 
b/include/dt-bindings/clock/qcom,rpmh.h
index 36caab2..f48fbd6 100644
--- a/include/dt-bindings/clock/qcom,rpmh.h
+++ b/include/dt-bindings/clock/qcom,rpmh.h
@@ -18,7 +18,5 @@
  #define RPMH_RF_CLK2_A9
  #define RPMH_RF_CLK3  10
  #define RPMH_RF_CLK3_A11
-#define RPMH_RF_CLK4   12
-#define RPMH_RF_CLK4_A 13


Did you mean to have this or squash into previous patch?



Thanks, my mistake. It was meant to be squashed in the previous patch. I 
would take care of it in the next series.


--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--

Re: [v5 2/2] clk: qcom: clk-rpmh: Add QCOM RPMh clock driver

[Taniya Das]

On 5/1/2018 6:13 PM, Rob Herring wrote:

On Tue, May 01, 2018 at 02:11:33PM +0530, Taniya Das wrote:

Add the RPMh clock driver to control the RPMh managed clock resources on
some of the Qualcomm Technologies, Inc. SoCs.

Signed-off-by: Amit Nischal 
Signed-off-by: Taniya Das 
---
  drivers/clk/qcom/Kconfig  |   9 +
  drivers/clk/qcom/Makefile |   1 +
  drivers/clk/qcom/clk-rpmh.c   | 387 ++
  include/dt-bindings/clock/qcom,rpmh.h |   2 -
  4 files changed, 397 insertions(+), 2 deletions(-)
  create mode 100644 drivers/clk/qcom/clk-rpmh.c



diff --git a/include/dt-bindings/clock/qcom,rpmh.h 
b/include/dt-bindings/clock/qcom,rpmh.h
index 36caab2..f48fbd6 100644
--- a/include/dt-bindings/clock/qcom,rpmh.h
+++ b/include/dt-bindings/clock/qcom,rpmh.h
@@ -18,7 +18,5 @@
  #define RPMH_RF_CLK2_A9
  #define RPMH_RF_CLK3  10
  #define RPMH_RF_CLK3_A11
-#define RPMH_RF_CLK4   12
-#define RPMH_RF_CLK4_A 13


Did you mean to have this or squash into previous patch?



Thanks, my mistake. It was meant to be squashed in the previous patch. I 
would take care of it in the next series.


--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--

Re: [PATCH 0/4] ARM: dts: am437x boards: Correct tps65218 irq type

[Tony Lindgren]

* Keerthy  [180423 10:32]:
> 
> 
> On Friday 20 April 2018 05:08 PM, Peter Ujfalusi wrote:
> > Hi,
> > 
> > In linux-next I started to see: https://pastebin.com/wrDdptzh
> > [2.813985] [ cut here ]
> > [2.818746] WARNING: CPU: 0 PID: 19 at 
> > /home/ujfalusi/work/kernel/github.omap-audio.linux-audio/drivers/irqchip/irq-gic.c:1016
> >  gic_irq_domain_translate+0xd4/0xec
> > [2.833311] Modules linked in:
> > [2.836417] CPU: 0 PID: 19 Comm: kworker/0:2 Not tainted 
> > 4.17.0-rc1-next-20180420-00057-gffd03ee19b9b #182
> > [2.846113] Hardware name: Generic AM43 (Flattened Device Tree)
> > [2.852073] Workqueue: events deferred_probe_work_func
> > ...
> > 
> > Becasue most of the am437x boards had IRQ_TYPE_NONE for tps65218.
> > 
> > According to the datasheet [1] the interrult line is low active, so fix up 
> > the board
> > DTS files.
> > 
> > Interestingly the am437x-sk-evm had IRQ_TYPE_LEVEL_HIGH, which is not 
> > matching
> > with the datasheet.
> > 
> > [1] http://www.ti.com/lit/ds/symlink/tps65218.pdf, page 8.
> 
> For the series:
> 
> Reviewed-by: Keerthy 
> 
> Also Boot tested on AM437X-GP-EVM with the patch 1 of the series and i
> do not see the error above.

Thanks applying all into omap-for-v4.18/dt.

Tony

Re: [PATCH 0/4] ARM: dts: am437x boards: Correct tps65218 irq type

[Tony Lindgren]

* Keerthy  [180423 10:32]:
> 
> 
> On Friday 20 April 2018 05:08 PM, Peter Ujfalusi wrote:
> > Hi,
> > 
> > In linux-next I started to see: https://pastebin.com/wrDdptzh
> > [2.813985] [ cut here ]
> > [2.818746] WARNING: CPU: 0 PID: 19 at 
> > /home/ujfalusi/work/kernel/github.omap-audio.linux-audio/drivers/irqchip/irq-gic.c:1016
> >  gic_irq_domain_translate+0xd4/0xec
> > [2.833311] Modules linked in:
> > [2.836417] CPU: 0 PID: 19 Comm: kworker/0:2 Not tainted 
> > 4.17.0-rc1-next-20180420-00057-gffd03ee19b9b #182
> > [2.846113] Hardware name: Generic AM43 (Flattened Device Tree)
> > [2.852073] Workqueue: events deferred_probe_work_func
> > ...
> > 
> > Becasue most of the am437x boards had IRQ_TYPE_NONE for tps65218.
> > 
> > According to the datasheet [1] the interrult line is low active, so fix up 
> > the board
> > DTS files.
> > 
> > Interestingly the am437x-sk-evm had IRQ_TYPE_LEVEL_HIGH, which is not 
> > matching
> > with the datasheet.
> > 
> > [1] http://www.ti.com/lit/ds/symlink/tps65218.pdf, page 8.
> 
> For the series:
> 
> Reviewed-by: Keerthy 
> 
> Also Boot tested on AM437X-GP-EVM with the patch 1 of the series and i
> do not see the error above.

Thanks applying all into omap-for-v4.18/dt.

Tony

Re: [PATCH v3 2/2] nvmem: Add RAVE SP EEPROM driver

[Srinivas Kandagatla]

On 01/05/18 06:29, Andrey Smirnov wrote:

+// SPDX-License-Identifier: GPL-2.0+

...

+MODULE_LICENSE("GPL");

I think this should be
MODULE_LICENSE("GPL v2");

--srini

Re: [PATCH v3 2/2] nvmem: Add RAVE SP EEPROM driver

[Srinivas Kandagatla]

On 01/05/18 06:29, Andrey Smirnov wrote:

+// SPDX-License-Identifier: GPL-2.0+

...

+MODULE_LICENSE("GPL");

I think this should be
MODULE_LICENSE("GPL v2");

--srini

Re: [PATCH 03/24] VFS: Introduce the structs and doc for a filesystem context [ver #7]

[Randy Dunlap]

On 05/01/2018 07:29 AM, David Howells wrote:
> Randy Dunlap  wrote:
> 
>>> + (2) Parse the options and attach them to the context.  Options may be 
>>> passed
>>> + individually from userspace.
>>
>> Does this say that step (2) can be multiple small steps?
> 
> Perhaps "phase (2)" would be a better name than "step (2)".  During (2),
> multiple argument-supplying calls may be made.

Ack.

>> How does step (2) know when userspace has completed sending individual
>> options?
> 
> Moving to phase (3) terminates phase (2).  This is triggered by userspace
> writing "x create" or "x reconfigure" to the fd as things stand.
> 
>>> + (6) Return an error message attached to the context.
>>
>> where/how is this done?
> 
> That got taken out and made general - which Linus then objected to.  I need to
> reinsert this and make it fscontext-specific as most people would really like
> to have it, the mount process being able to produce so many weird and
> wonderful errors.
> 
>>> +When the VFS creates this, it allocates ->fs_context_size bytes (as 
>>> specified
>>> +by the file_system_type object) to hold both the fs_context struct and any
>>> +extra data required by the filesystem.  The fs_context struct is placed at 
>>> the
>>> +beginning of this space.  Any extra space beyond that is for use by the
>>> +filesystem.  The filesystem should wrap the struct in its own, e.g.:
>>
>>   in its own struct, 
>> e.g.:
> 
> How about "... The filesystem should wrap the struct in one of its own, e.g."?

OK.

>>> + (*) int security_fs_context_parse_option(struct fs_context *fc, char 
>>> *opt);   
>>> +
>>> + Called for each mount option.  The arguments are as for the
>>> + ->parse_option() method.  An active LSM may reject one with an error, 
>>> pass
>>> + one over and return 0 or consume one and return 1.  If consumed, the
>>
>> What does "pass one over" mean?
> 
> How about:
> 
>   An active LSM may return 0 to pass the option on to the filesystem, 1
>   to cause the option to be discarded or an error to cause the option to
>   be rejected.

Much better. Thanks.

-- 
~Randy

Re: [PATCH 03/24] VFS: Introduce the structs and doc for a filesystem context [ver #7]

[Randy Dunlap]

On 05/01/2018 07:29 AM, David Howells wrote:
> Randy Dunlap  wrote:
> 
>>> + (2) Parse the options and attach them to the context.  Options may be 
>>> passed
>>> + individually from userspace.
>>
>> Does this say that step (2) can be multiple small steps?
> 
> Perhaps "phase (2)" would be a better name than "step (2)".  During (2),
> multiple argument-supplying calls may be made.

Ack.

>> How does step (2) know when userspace has completed sending individual
>> options?
> 
> Moving to phase (3) terminates phase (2).  This is triggered by userspace
> writing "x create" or "x reconfigure" to the fd as things stand.
> 
>>> + (6) Return an error message attached to the context.
>>
>> where/how is this done?
> 
> That got taken out and made general - which Linus then objected to.  I need to
> reinsert this and make it fscontext-specific as most people would really like
> to have it, the mount process being able to produce so many weird and
> wonderful errors.
> 
>>> +When the VFS creates this, it allocates ->fs_context_size bytes (as 
>>> specified
>>> +by the file_system_type object) to hold both the fs_context struct and any
>>> +extra data required by the filesystem.  The fs_context struct is placed at 
>>> the
>>> +beginning of this space.  Any extra space beyond that is for use by the
>>> +filesystem.  The filesystem should wrap the struct in its own, e.g.:
>>
>>   in its own struct, 
>> e.g.:
> 
> How about "... The filesystem should wrap the struct in one of its own, e.g."?

OK.

>>> + (*) int security_fs_context_parse_option(struct fs_context *fc, char 
>>> *opt);   
>>> +
>>> + Called for each mount option.  The arguments are as for the
>>> + ->parse_option() method.  An active LSM may reject one with an error, 
>>> pass
>>> + one over and return 0 or consume one and return 1.  If consumed, the
>>
>> What does "pass one over" mean?
> 
> How about:
> 
>   An active LSM may return 0 to pass the option on to the filesystem, 1
>   to cause the option to be discarded or an error to cause the option to
>   be rejected.

Much better. Thanks.

-- 
~Randy

Re: [PATCH v2 0/2] net: stmmac: dwmac-meson: 100M phy mode support for AXG SoC

[David Miller]

From: Yixun Lan 
Date: Sat, 28 Apr 2018 10:21:09 +

> Due to the dwmac glue layer register changed, we need to 
> introduce a new compatible name for the Meson-AXG SoC
> to support for the RMII 100M ethernet PHY.
> 
> Change since v1 at [1]:
>   - implement set_phy_mode() for each SoC
> 
> [1] https://lkml.kernel.org/r/20180426160508.29380-1-yixun@amlogic.com

Series applied, thank you.

Re: [PATCH v2 0/2] net: stmmac: dwmac-meson: 100M phy mode support for AXG SoC

[David Miller]

From: Yixun Lan 
Date: Sat, 28 Apr 2018 10:21:09 +

> Due to the dwmac glue layer register changed, we need to 
> introduce a new compatible name for the Meson-AXG SoC
> to support for the RMII 100M ethernet PHY.
> 
> Change since v1 at [1]:
>   - implement set_phy_mode() for each SoC
> 
> [1] https://lkml.kernel.org/r/20180426160508.29380-1-yixun@amlogic.com

Series applied, thank you.

Re: [PATCH] vhost: make msg padding explicit

[David Miller]

From: "Michael S. Tsirkin" 
Date: Fri, 27 Apr 2018 19:02:05 +0300

> There's a 32 bit hole just after type. It's best to
> give it a name, this way compiler is forced to initialize
> it with rest of the structure.
> 
> Reported-by: Kevin Easton 
> Signed-off-by: Michael S. Tsirkin 

Michael, will you be sending this directly to Linus or would you like
me to apply it to net or net-next?

Thanks.

Re: [PATCH] vhost: make msg padding explicit

[David Miller]

From: "Michael S. Tsirkin" 
Date: Fri, 27 Apr 2018 19:02:05 +0300

> There's a 32 bit hole just after type. It's best to
> give it a name, this way compiler is forced to initialize
> it with rest of the structure.
> 
> Reported-by: Kevin Easton 
> Signed-off-by: Michael S. Tsirkin 

Michael, will you be sending this directly to Linus or would you like
me to apply it to net or net-next?

Thanks.

Re: [PATCH] IB/cxgb4: use skb_put_zero()/__skb_put_zero

[Doug Ledford]

On Sat, 2018-04-28 at 06:53 -0500, Steve Wise wrote:
> > -Original Message-
> > From: YueHaibing 
> > Sent: Saturday, April 28, 2018 2:31 AM
> > To: sw...@chelsio.com; dledf...@redhat.com; j...@ziepe.ca;
> > mo...@mellanox.com
> > Cc: linux-r...@vger.kernel.org; linux-kernel@vger.kernel.org; YueHaibing
> > 
> > Subject: [PATCH] IB/cxgb4: use skb_put_zero()/__skb_put_zero
> > 
> > Use the recently introduced helper to replace the pattern of
> > skb_put_zero/__skb_put() && memset().
> > 
> > Signed-off-by: YueHaibing 
> 
> Looks ok.
> 
> Reviewed-by: Steve Wise 
> 
> 

Applied to for-next, thanks.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH] IB/cxgb4: use skb_put_zero()/__skb_put_zero

[Doug Ledford]

On Sat, 2018-04-28 at 06:53 -0500, Steve Wise wrote:
> > -Original Message-
> > From: YueHaibing 
> > Sent: Saturday, April 28, 2018 2:31 AM
> > To: sw...@chelsio.com; dledf...@redhat.com; j...@ziepe.ca;
> > mo...@mellanox.com
> > Cc: linux-r...@vger.kernel.org; linux-kernel@vger.kernel.org; YueHaibing
> > 
> > Subject: [PATCH] IB/cxgb4: use skb_put_zero()/__skb_put_zero
> > 
> > Use the recently introduced helper to replace the pattern of
> > skb_put_zero/__skb_put() && memset().
> > 
> > Signed-off-by: YueHaibing 
> 
> Looks ok.
> 
> Reviewed-by: Steve Wise 
> 
> 

Applied to for-next, thanks.

-- 
Doug Ledford 
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

signature.asc
Description: This is a digitally signed message part

Re: [PATCH 3/3] seccomp: Don't special case audited processes when logging

[Paul Moore]

On Fri, Apr 27, 2018 at 3:16 PM, Tyler Hicks  wrote:
> Seccomp logging for "handled" actions such as RET_TRAP, RET_TRACE, or
> RET_ERRNO can be very noisy for processes that are being audited. This
> patch modifies the seccomp logging behavior to treat processes that are
> being inspected via the audit subsystem the same as processes that
> aren't under inspection. Handled actions will no longer be logged just
> because the process is being inspected. Since v4.14, applications have
> the ability to request logging of handled actions by using the
> SECCOMP_FILTER_FLAG_LOG flag when loading seccomp filters.
>
> With this patch, the logic for deciding if an action will be logged is:
>
>   if action == RET_ALLOW:
> do not log
>   else if action not in actions_logged:
> do not log
>   else if action == RET_KILL:
> log
>   else if action == RET_LOG:
> log
>   else if filter-requests-logging:
> log
>   else:
> do not log
>
> Reported-by: Steve Grubb 
> Signed-off-by: Tyler Hicks 
> ---
>  Documentation/userspace-api/seccomp_filter.rst |  7 ---
>  include/linux/audit.h  | 10 +-
>  kernel/auditsc.c   |  2 +-
>  kernel/seccomp.c   | 15 +--
>  4 files changed, 7 insertions(+), 27 deletions(-)
>
> diff --git a/Documentation/userspace-api/seccomp_filter.rst 
> b/Documentation/userspace-api/seccomp_filter.rst
> index 099c412..82a468b 100644
> --- a/Documentation/userspace-api/seccomp_filter.rst
> +++ b/Documentation/userspace-api/seccomp_filter.rst
> @@ -207,13 +207,6 @@ directory. Here's a description of each file in that 
> directory:
> to the file do not need to be in ordered form but reads from the file
> will be ordered in the same way as the actions_avail sysctl.
>
> -   It is important to note that the value of ``actions_logged`` does not
> -   prevent certain actions from being logged when the audit subsystem is
> -   configured to audit a task. If the action is not found in
> -   ``actions_logged`` list, the final decision on whether to audit the
> -   action for that task is ultimately left up to the audit subsystem to
> -   decide for all seccomp return values other than ``SECCOMP_RET_ALLOW``.
> -
> The ``allow`` string is not accepted in the ``actions_logged`` sysctl
> as it is not possible to log ``SECCOMP_RET_ALLOW`` actions. Attempting
> to write ``allow`` to the sysctl will result in an EINVAL being
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index b311d7d..1964fbd 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -232,7 +232,7 @@ extern void __audit_file(const struct file *);
>  extern void __audit_inode_child(struct inode *parent,
> const struct dentry *dentry,
> const unsigned char type);
> -extern void __audit_seccomp(unsigned long syscall, long signr, int code);
> +extern void audit_seccomp(unsigned long syscall, long signr, int code);
>  extern void audit_seccomp_actions_logged(const char *names, int res);
>  extern void __audit_ptrace(struct task_struct *t);
>
> @@ -303,12 +303,6 @@ static inline void audit_inode_child(struct inode 
> *parent,
>  }
>  void audit_core_dumps(long signr);
>
> -static inline void audit_seccomp(unsigned long syscall, long signr, int code)
> -{
> -   if (audit_enabled && unlikely(!audit_dummy_context()))
> -   __audit_seccomp(syscall, signr, code);
> -}
> -
>  static inline void audit_ptrace(struct task_struct *t)
>  {
> if (unlikely(!audit_dummy_context()))
> @@ -499,8 +493,6 @@ static inline void audit_inode_child(struct inode *parent,
>  { }
>  static inline void audit_core_dumps(long signr)
>  { }
> -static inline void __audit_seccomp(unsigned long syscall, long signr, int 
> code)
> -{ }
>  static inline void audit_seccomp(unsigned long syscall, long signr, int code)
>  { }
>  static inline void audit_seccomp_actions_logged(const char *names, int res)
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 3496238..1e64b91 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2464,7 +2464,7 @@ void audit_core_dumps(long signr)
> audit_log_end(ab);
>  }
>
> -void __audit_seccomp(unsigned long syscall, long signr, int code)
> +void audit_seccomp(unsigned long syscall, long signr, int code)
>  {
> struct audit_buffer *ab;

Since it is a bit unusual, it might be nice to add a comment at the
top of audit_seccomp() that the event filtering is being done in the
seccomp_log() function, and we may need to force auditing independent
of the audit_enabled and dummy context state.

> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index e28ddcc..947cc0f 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -584,18 +584,13 @@ static inline void 

Re: [PATCH 3/3] seccomp: Don't special case audited processes when logging

[Paul Moore]

On Fri, Apr 27, 2018 at 3:16 PM, Tyler Hicks  wrote:
> Seccomp logging for "handled" actions such as RET_TRAP, RET_TRACE, or
> RET_ERRNO can be very noisy for processes that are being audited. This
> patch modifies the seccomp logging behavior to treat processes that are
> being inspected via the audit subsystem the same as processes that
> aren't under inspection. Handled actions will no longer be logged just
> because the process is being inspected. Since v4.14, applications have
> the ability to request logging of handled actions by using the
> SECCOMP_FILTER_FLAG_LOG flag when loading seccomp filters.
>
> With this patch, the logic for deciding if an action will be logged is:
>
>   if action == RET_ALLOW:
> do not log
>   else if action not in actions_logged:
> do not log
>   else if action == RET_KILL:
> log
>   else if action == RET_LOG:
> log
>   else if filter-requests-logging:
> log
>   else:
> do not log
>
> Reported-by: Steve Grubb 
> Signed-off-by: Tyler Hicks 
> ---
>  Documentation/userspace-api/seccomp_filter.rst |  7 ---
>  include/linux/audit.h  | 10 +-
>  kernel/auditsc.c   |  2 +-
>  kernel/seccomp.c   | 15 +--
>  4 files changed, 7 insertions(+), 27 deletions(-)
>
> diff --git a/Documentation/userspace-api/seccomp_filter.rst 
> b/Documentation/userspace-api/seccomp_filter.rst
> index 099c412..82a468b 100644
> --- a/Documentation/userspace-api/seccomp_filter.rst
> +++ b/Documentation/userspace-api/seccomp_filter.rst
> @@ -207,13 +207,6 @@ directory. Here's a description of each file in that 
> directory:
> to the file do not need to be in ordered form but reads from the file
> will be ordered in the same way as the actions_avail sysctl.
>
> -   It is important to note that the value of ``actions_logged`` does not
> -   prevent certain actions from being logged when the audit subsystem is
> -   configured to audit a task. If the action is not found in
> -   ``actions_logged`` list, the final decision on whether to audit the
> -   action for that task is ultimately left up to the audit subsystem to
> -   decide for all seccomp return values other than ``SECCOMP_RET_ALLOW``.
> -
> The ``allow`` string is not accepted in the ``actions_logged`` sysctl
> as it is not possible to log ``SECCOMP_RET_ALLOW`` actions. Attempting
> to write ``allow`` to the sysctl will result in an EINVAL being
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index b311d7d..1964fbd 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -232,7 +232,7 @@ extern void __audit_file(const struct file *);
>  extern void __audit_inode_child(struct inode *parent,
> const struct dentry *dentry,
> const unsigned char type);
> -extern void __audit_seccomp(unsigned long syscall, long signr, int code);
> +extern void audit_seccomp(unsigned long syscall, long signr, int code);
>  extern void audit_seccomp_actions_logged(const char *names, int res);
>  extern void __audit_ptrace(struct task_struct *t);
>
> @@ -303,12 +303,6 @@ static inline void audit_inode_child(struct inode 
> *parent,
>  }
>  void audit_core_dumps(long signr);
>
> -static inline void audit_seccomp(unsigned long syscall, long signr, int code)
> -{
> -   if (audit_enabled && unlikely(!audit_dummy_context()))
> -   __audit_seccomp(syscall, signr, code);
> -}
> -
>  static inline void audit_ptrace(struct task_struct *t)
>  {
> if (unlikely(!audit_dummy_context()))
> @@ -499,8 +493,6 @@ static inline void audit_inode_child(struct inode *parent,
>  { }
>  static inline void audit_core_dumps(long signr)
>  { }
> -static inline void __audit_seccomp(unsigned long syscall, long signr, int 
> code)
> -{ }
>  static inline void audit_seccomp(unsigned long syscall, long signr, int code)
>  { }
>  static inline void audit_seccomp_actions_logged(const char *names, int res)
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 3496238..1e64b91 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2464,7 +2464,7 @@ void audit_core_dumps(long signr)
> audit_log_end(ab);
>  }
>
> -void __audit_seccomp(unsigned long syscall, long signr, int code)
> +void audit_seccomp(unsigned long syscall, long signr, int code)
>  {
> struct audit_buffer *ab;

Since it is a bit unusual, it might be nice to add a comment at the
top of audit_seccomp() that the event filtering is being done in the
seccomp_log() function, and we may need to force auditing independent
of the audit_enabled and dummy context state.

> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index e28ddcc..947cc0f 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -584,18 +584,13 @@ static inline void seccomp_log(unsigned long syscall, 
> long signr, u32 action,
> }
>
> 

Re: [PATCH] ARM: dts: am33xx: Add pinmux data for mmc1 in am335x-evm, evmsk and beaglebone

[Tony Lindgren]

* Faiz Abbas  [180411 04:48]:
> am335x-evm, am335x-evmsk and am335x-beaglebone are currently relying on
> pinmux set by the bootloader to set the correct value for mmc1. Fix
> this by adding pinmux data for the same in kernel.
> 
> Signed-off-by: Faiz Abbas 

Applying into omap-for-v4.18/dt thanks.

Tony

Re: [PATCH] ARM: dts: am33xx: Add pinmux data for mmc1 in am335x-evm, evmsk and beaglebone

[Tony Lindgren]

* Faiz Abbas  [180411 04:48]:
> am335x-evm, am335x-evmsk and am335x-beaglebone are currently relying on
> pinmux set by the bootloader to set the correct value for mmc1. Fix
> this by adding pinmux data for the same in kernel.
> 
> Signed-off-by: Faiz Abbas 

Applying into omap-for-v4.18/dt thanks.

Tony

Re: [PATCH RFC v5 3/6] srcu: Add notrace variant of srcu_dereference

[Paul E. McKenney]

On Tue, May 01, 2018 at 11:04:03AM -0400, Steven Rostedt wrote:
> On Tue, 1 May 2018 07:18:17 -0700
> "Paul E. McKenney"  wrote:
> 
> 
> > > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > > index 2ec618979b20..a1c4947be877 100644
> > > --- a/include/linux/srcu.h
> > > +++ b/include/linux/srcu.h
> > > @@ -135,6 +135,11 @@ static inline int srcu_read_lock_held(const struct 
> > > srcu_struct *sp)
> > >   */
> > >  #define srcu_dereference(p, sp) srcu_dereference_check((p), (sp), 0)
> > > 
> > > +/**
> > > + * srcu_dereference_notrace - no tracing and no lockdep calls from here
> > > + */
> > > +#define srcu_dereference_notrace(p, sp) srcu_dereference_check((p), 
> > > (sp), 1)  
> > 
> > Given that this is a one-liner, why not just use srcu_dereference_check()
> > directly, with 1 as you do above to disable lockdep?
> 
> I prefer what Joel did. It documents why we are doing this. Open coding
> the call directly will just confuse others that touch tracepoint code.

I suppose...

Reviewed-by: Paul E. McKenney 

Re: [PATCH RFC v5 3/6] srcu: Add notrace variant of srcu_dereference

[Paul E. McKenney]

On Tue, May 01, 2018 at 11:04:03AM -0400, Steven Rostedt wrote:
> On Tue, 1 May 2018 07:18:17 -0700
> "Paul E. McKenney"  wrote:
> 
> 
> > > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > > index 2ec618979b20..a1c4947be877 100644
> > > --- a/include/linux/srcu.h
> > > +++ b/include/linux/srcu.h
> > > @@ -135,6 +135,11 @@ static inline int srcu_read_lock_held(const struct 
> > > srcu_struct *sp)
> > >   */
> > >  #define srcu_dereference(p, sp) srcu_dereference_check((p), (sp), 0)
> > > 
> > > +/**
> > > + * srcu_dereference_notrace - no tracing and no lockdep calls from here
> > > + */
> > > +#define srcu_dereference_notrace(p, sp) srcu_dereference_check((p), 
> > > (sp), 1)  
> > 
> > Given that this is a one-liner, why not just use srcu_dereference_check()
> > directly, with 1 as you do above to disable lockdep?
> 
> I prefer what Joel did. It documents why we are doing this. Open coding
> the call directly will just confuse others that touch tracepoint code.

I suppose...

Reviewed-by: Paul E. McKenney 

Re: [PATCH 1/3] ARM: dra762: hwmod: Add MCAN support

[Tony Lindgren]

Hi,

* Faiz Abbas  [180408 09:59]:
> From: Lokesh Vutla 
> 
> Add MCAN hwmod data and register it for dra762 silicons.
> 
> Signed-off-by: Lokesh Vutla 
> Signed-off-by: Faiz Abbas 
> ---
>  arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 32 
> +++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c 
> b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index 62352d1..a2cd7f8 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1356,6 +1356,29 @@ static struct omap_hwmod dra7xx_mailbox13_hwmod = {
>  };
>  
>  /*
> + * 'mcan' class
> + *
> + */
> +static struct omap_hwmod_class dra76x_mcan_hwmod_class = {
> + .name   = "mcan",
> +};

Looks like you're missing the related struct omap_hwmod_class_sysconfig
entry for this with the rev and sysconfig registers.

Regards,

Tony

Re: [PATCH 1/3] ARM: dra762: hwmod: Add MCAN support

[Tony Lindgren]

Hi,

* Faiz Abbas  [180408 09:59]:
> From: Lokesh Vutla 
> 
> Add MCAN hwmod data and register it for dra762 silicons.
> 
> Signed-off-by: Lokesh Vutla 
> Signed-off-by: Faiz Abbas 
> ---
>  arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 32 
> +++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c 
> b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> index 62352d1..a2cd7f8 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
> @@ -1356,6 +1356,29 @@ static struct omap_hwmod dra7xx_mailbox13_hwmod = {
>  };
>  
>  /*
> + * 'mcan' class
> + *
> + */
> +static struct omap_hwmod_class dra76x_mcan_hwmod_class = {
> + .name   = "mcan",
> +};

Looks like you're missing the related struct omap_hwmod_class_sysconfig
entry for this with the rev and sysconfig registers.

Regards,

Tony

Re: [PATCH 4/5] kernel hacking: new config DEBUG_EXPERIENCE to apply GCC -Og optimization

[Randy Dunlap]

Good morning.

On 05/01/2018 06:00 AM, changbin...@intel.com wrote:
> From: Changbin Du 
> 
> 
> Signed-off-by: Changbin Du 
> ---
>  Makefile |  4 
>  include/linux/compiler-gcc.h |  2 +-
>  include/linux/compiler.h |  2 +-
>  lib/Kconfig.debug| 21 +
>  4 files changed, 27 insertions(+), 2 deletions(-)
> 

> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 90f35ad..2432e77d 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -211,6 +211,27 @@ config NO_AUTO_INLINE
>  
> Use only if you want to debug the kernel.
>  
> +config DEBUG_EXPERIENCE
> + bool "Optimize for better debugging experience (-Og)"
> + default n
> + select NO_AUTO_INLINE
> + depends on !CC_OPTIMIZE_FOR_SIZE
> + help
> +   This will apply GCC '-Og' optimization level get supported from

   which is supported since

> +   GCC 4.8. This optimization level offers a reasonable level of
> +   optimization while maintaining fast compilation and a good
> +   debugging experience. It is similar to '-O1' while perfer keeping

   while preferring to keep

> +   debug ability over runtime speed. The overall performance will
> +   drop a bit.
> +
> +   If enabling this option break your kernel, you should either

  breaks

> +   disable this or find a fix (mostly in the arch code). Currently
> +   this option has only be tested in qemu x86_64 guest.
> +
> +   Use only if you want to debug the kernel, especially if you want
> +   to have better kernel debugging experience with gdb facilities
> +   like kgdb and qemu.
> +
>  config ENABLE_WARN_DEPRECATED
>   bool "Enable __deprecated logic"
>   default y
> 

thanks,
-- 
~Randy

Re: [PATCH 4/5] kernel hacking: new config DEBUG_EXPERIENCE to apply GCC -Og optimization

[Randy Dunlap]

Good morning.

On 05/01/2018 06:00 AM, changbin...@intel.com wrote:
> From: Changbin Du 
> 
> 
> Signed-off-by: Changbin Du 
> ---
>  Makefile |  4 
>  include/linux/compiler-gcc.h |  2 +-
>  include/linux/compiler.h |  2 +-
>  lib/Kconfig.debug| 21 +
>  4 files changed, 27 insertions(+), 2 deletions(-)
> 

> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 90f35ad..2432e77d 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -211,6 +211,27 @@ config NO_AUTO_INLINE
>  
> Use only if you want to debug the kernel.
>  
> +config DEBUG_EXPERIENCE
> + bool "Optimize for better debugging experience (-Og)"
> + default n
> + select NO_AUTO_INLINE
> + depends on !CC_OPTIMIZE_FOR_SIZE
> + help
> +   This will apply GCC '-Og' optimization level get supported from

   which is supported since

> +   GCC 4.8. This optimization level offers a reasonable level of
> +   optimization while maintaining fast compilation and a good
> +   debugging experience. It is similar to '-O1' while perfer keeping

   while preferring to keep

> +   debug ability over runtime speed. The overall performance will
> +   drop a bit.
> +
> +   If enabling this option break your kernel, you should either

  breaks

> +   disable this or find a fix (mostly in the arch code). Currently
> +   this option has only be tested in qemu x86_64 guest.
> +
> +   Use only if you want to debug the kernel, especially if you want
> +   to have better kernel debugging experience with gdb facilities
> +   like kgdb and qemu.
> +
>  config ENABLE_WARN_DEPRECATED
>   bool "Enable __deprecated logic"
>   default y
> 

thanks,
-- 
~Randy

Re: [PATCH RFC v5 5/6] tracepoint: Make rcuidle tracepoint callers use SRCU

[Joel Fernandes]

On Tue, May 1, 2018 at 8:20 AM Paul E. McKenney 
wrote:
[...]
> > > > > --- a/kernel/tracepoint.c
> > > > > +++ b/kernel/tracepoint.c
> > > > > @@ -31,6 +31,9 @@
> > > > >  extern struct tracepoint * const __start___tracepoints_ptrs[];
> > > > >  extern struct tracepoint * const __stop___tracepoints_ptrs[];
> > > > >
> > > > > +DEFINE_SRCU(tracepoint_srcu);
> > > > > +EXPORT_SYMBOL_GPL(tracepoint_srcu);
> > > > > +
> > > > >  /* Set to 1 to enable tracepoint debug output */
> > > > >  static const int tracepoint_debug;
> > > > >
> > > > > @@ -67,11 +70,16 @@ static inline void *allocate_probes(int count)
> > > > > return p == NULL ? NULL : p->probes;
> > > > >  }
> > > > >
> > > > > -static void rcu_free_old_probes(struct rcu_head *head)
> > > > > +static void srcu_free_old_probes(struct rcu_head *head)
> > > > >  {
> > > > > kfree(container_of(head, struct tp_probes, rcu));
> > > > >  }
> > > > >
> > > > > +static void rcu_free_old_probes(struct rcu_head *head)
> > > > > +{
> > > > > +   call_srcu(_srcu, head, srcu_free_old_probes);
> > > >
> > > > Hmm, is it OK to call call_srcu() from a call_rcu() callback? I
guess
> > > > it would be.
> >
> > > It is perfectly legal, and quite a bit simpler than setting something
> > > up to wait for both to complete concurrently.
> >
> > Cool. Also in this case if we call both in sequence, then I felt there
> > could be a race to free the old data since both callbacks would try to
do
> > the same thing. The same thing being freeing of the same set of old
probes
> > which would need some synchronization between the 2 callbacks. With the
> > chaining, since the ordering is assured there wouldn't be a question of
> > such a race. I could add this reasoning to the changelog as well.

> Actually, as long as you have a solid happens-before between both of the
> callbacks and the freeing, you are in good shape.  A release-acquire would
> work fine, as would a lock acquired in both callbacks and then acquired
> (and possibly released) before the free.

Got it, thanks. For now, if its Ok with you and others, I will leave it in
the chained configuration. I also feel this is temporary since in the
future if we switch to single rcu mechanism for tracepoints (srcu), then we
could do with just a single callback.

thanks,

- Joel

Re: [PATCH RFC v5 5/6] tracepoint: Make rcuidle tracepoint callers use SRCU

[Joel Fernandes]

On Tue, May 1, 2018 at 8:20 AM Paul E. McKenney 
wrote:
[...]
> > > > > --- a/kernel/tracepoint.c
> > > > > +++ b/kernel/tracepoint.c
> > > > > @@ -31,6 +31,9 @@
> > > > >  extern struct tracepoint * const __start___tracepoints_ptrs[];
> > > > >  extern struct tracepoint * const __stop___tracepoints_ptrs[];
> > > > >
> > > > > +DEFINE_SRCU(tracepoint_srcu);
> > > > > +EXPORT_SYMBOL_GPL(tracepoint_srcu);
> > > > > +
> > > > >  /* Set to 1 to enable tracepoint debug output */
> > > > >  static const int tracepoint_debug;
> > > > >
> > > > > @@ -67,11 +70,16 @@ static inline void *allocate_probes(int count)
> > > > > return p == NULL ? NULL : p->probes;
> > > > >  }
> > > > >
> > > > > -static void rcu_free_old_probes(struct rcu_head *head)
> > > > > +static void srcu_free_old_probes(struct rcu_head *head)
> > > > >  {
> > > > > kfree(container_of(head, struct tp_probes, rcu));
> > > > >  }
> > > > >
> > > > > +static void rcu_free_old_probes(struct rcu_head *head)
> > > > > +{
> > > > > +   call_srcu(_srcu, head, srcu_free_old_probes);
> > > >
> > > > Hmm, is it OK to call call_srcu() from a call_rcu() callback? I
guess
> > > > it would be.
> >
> > > It is perfectly legal, and quite a bit simpler than setting something
> > > up to wait for both to complete concurrently.
> >
> > Cool. Also in this case if we call both in sequence, then I felt there
> > could be a race to free the old data since both callbacks would try to
do
> > the same thing. The same thing being freeing of the same set of old
probes
> > which would need some synchronization between the 2 callbacks. With the
> > chaining, since the ordering is assured there wouldn't be a question of
> > such a race. I could add this reasoning to the changelog as well.

> Actually, as long as you have a solid happens-before between both of the
> callbacks and the freeing, you are in good shape.  A release-acquire would
> work fine, as would a lock acquired in both callbacks and then acquired
> (and possibly released) before the free.

Got it, thanks. For now, if its Ok with you and others, I will leave it in
the chained configuration. I also feel this is temporary since in the
future if we switch to single rcu mechanism for tracepoints (srcu), then we
could do with just a single callback.

thanks,

- Joel

Re: [PATCH] ARM: dts: omap3-gta04: Add fixed 26MHz clock as fck for twl

[Tony Lindgren]

* H. Nikolaus Schaller  [180405 09:07]:
> The board uses 26MHz oscillator for the twl4030 HFCLK.
> This way we will not depend on the bootloader to configure the
> CFG_BOOT:HFCLK_FREQ
> 
> Signed-off-by: H. Nikolaus Schaller 

Applying into omap-for-v4.18/dt thanks.

Tony

Re: [PATCH] ARM: dts: omap3-gta04: Add fixed 26MHz clock as fck for twl

[Tony Lindgren]

* H. Nikolaus Schaller  [180405 09:07]:
> The board uses 26MHz oscillator for the twl4030 HFCLK.
> This way we will not depend on the bootloader to configure the
> CFG_BOOT:HFCLK_FREQ
> 
> Signed-off-by: H. Nikolaus Schaller 

Applying into omap-for-v4.18/dt thanks.

Tony

Re: [PATCH RFC v5 5/6] tracepoint: Make rcuidle tracepoint callers use SRCU

[Paul E. McKenney]

On Tue, May 01, 2018 at 03:16:02PM +, Joel Fernandes wrote:
> On Tue, May 1, 2018 at 7:34 AM Paul E. McKenney 
> wrote:
> 
> > On Tue, May 01, 2018 at 10:24:01AM -0400, Steven Rostedt wrote:
> > > On Mon, 30 Apr 2018 18:42:03 -0700
> > > Joel Fernandes  wrote:
> > >
> > > > In recent tests with IRQ on/off tracepoints, a large performance
> > > > overhead ~10% is noticed when running hackbench. This is root caused
> to
> > > > calls to rcu_irq_enter_irqson and rcu_irq_exit_irqson from the
> > > > tracepoint code. Following a long discussion on the list [1] about
> this,
> > > > we concluded that srcu is a better alternative for use during rcu
> idle.
> > > > Although it does involve extra barriers, its lighter than the
> sched-rcu
> > > > version which has to do additional RCU calls to notify RCU idle about
> > > > entry into RCU sections.
> > > >
> > > > In this patch, we change the underlying implementation of the
> > > > trace_*_rcuidle API to use SRCU. This has shown to improve performance
> > > > alot for the high frequency irq enable/disable tracepoints.
> 
> > [ . . . ]
> 
> > > > --- a/kernel/tracepoint.c
> > > > +++ b/kernel/tracepoint.c
> > > > @@ -31,6 +31,9 @@
> > > >  extern struct tracepoint * const __start___tracepoints_ptrs[];
> > > >  extern struct tracepoint * const __stop___tracepoints_ptrs[];
> > > >
> > > > +DEFINE_SRCU(tracepoint_srcu);
> > > > +EXPORT_SYMBOL_GPL(tracepoint_srcu);
> > > > +
> > > >  /* Set to 1 to enable tracepoint debug output */
> > > >  static const int tracepoint_debug;
> > > >
> > > > @@ -67,11 +70,16 @@ static inline void *allocate_probes(int count)
> > > > return p == NULL ? NULL : p->probes;
> > > >  }
> > > >
> > > > -static void rcu_free_old_probes(struct rcu_head *head)
> > > > +static void srcu_free_old_probes(struct rcu_head *head)
> > > >  {
> > > > kfree(container_of(head, struct tp_probes, rcu));
> > > >  }
> > > >
> > > > +static void rcu_free_old_probes(struct rcu_head *head)
> > > > +{
> > > > +   call_srcu(_srcu, head, srcu_free_old_probes);
> > >
> > > Hmm, is it OK to call call_srcu() from a call_rcu() callback? I guess
> > > it would be.
> 
> > It is perfectly legal, and quite a bit simpler than setting something
> > up to wait for both to complete concurrently.
> 
> Cool. Also in this case if we call both in sequence, then I felt there
> could be a race to free the old data since both callbacks would try to do
> the same thing. The same thing being freeing of the same set of old probes
> which would need some synchronization between the 2 callbacks. With the
> chaining, since the ordering is assured there wouldn't be a question of
> such a race. I could add this reasoning to the changelog as well.

Actually, as long as you have a solid happens-before between both of the
callbacks and the freeing, you are in good shape.  A release-acquire would
work fine, as would a lock acquired in both callbacks and then acquired
(and possibly released) before the free.

Thanx, Paul

Re: [PATCH RFC v5 5/6] tracepoint: Make rcuidle tracepoint callers use SRCU

[Paul E. McKenney]

On Tue, May 01, 2018 at 03:16:02PM +, Joel Fernandes wrote:
> On Tue, May 1, 2018 at 7:34 AM Paul E. McKenney 
> wrote:
> 
> > On Tue, May 01, 2018 at 10:24:01AM -0400, Steven Rostedt wrote:
> > > On Mon, 30 Apr 2018 18:42:03 -0700
> > > Joel Fernandes  wrote:
> > >
> > > > In recent tests with IRQ on/off tracepoints, a large performance
> > > > overhead ~10% is noticed when running hackbench. This is root caused
> to
> > > > calls to rcu_irq_enter_irqson and rcu_irq_exit_irqson from the
> > > > tracepoint code. Following a long discussion on the list [1] about
> this,
> > > > we concluded that srcu is a better alternative for use during rcu
> idle.
> > > > Although it does involve extra barriers, its lighter than the
> sched-rcu
> > > > version which has to do additional RCU calls to notify RCU idle about
> > > > entry into RCU sections.
> > > >
> > > > In this patch, we change the underlying implementation of the
> > > > trace_*_rcuidle API to use SRCU. This has shown to improve performance
> > > > alot for the high frequency irq enable/disable tracepoints.
> 
> > [ . . . ]
> 
> > > > --- a/kernel/tracepoint.c
> > > > +++ b/kernel/tracepoint.c
> > > > @@ -31,6 +31,9 @@
> > > >  extern struct tracepoint * const __start___tracepoints_ptrs[];
> > > >  extern struct tracepoint * const __stop___tracepoints_ptrs[];
> > > >
> > > > +DEFINE_SRCU(tracepoint_srcu);
> > > > +EXPORT_SYMBOL_GPL(tracepoint_srcu);
> > > > +
> > > >  /* Set to 1 to enable tracepoint debug output */
> > > >  static const int tracepoint_debug;
> > > >
> > > > @@ -67,11 +70,16 @@ static inline void *allocate_probes(int count)
> > > > return p == NULL ? NULL : p->probes;
> > > >  }
> > > >
> > > > -static void rcu_free_old_probes(struct rcu_head *head)
> > > > +static void srcu_free_old_probes(struct rcu_head *head)
> > > >  {
> > > > kfree(container_of(head, struct tp_probes, rcu));
> > > >  }
> > > >
> > > > +static void rcu_free_old_probes(struct rcu_head *head)
> > > > +{
> > > > +   call_srcu(_srcu, head, srcu_free_old_probes);
> > >
> > > Hmm, is it OK to call call_srcu() from a call_rcu() callback? I guess
> > > it would be.
> 
> > It is perfectly legal, and quite a bit simpler than setting something
> > up to wait for both to complete concurrently.
> 
> Cool. Also in this case if we call both in sequence, then I felt there
> could be a race to free the old data since both callbacks would try to do
> the same thing. The same thing being freeing of the same set of old probes
> which would need some synchronization between the 2 callbacks. With the
> chaining, since the ordering is assured there wouldn't be a question of
> such a race. I could add this reasoning to the changelog as well.

Actually, as long as you have a solid happens-before between both of the
callbacks and the freeing, you are in good shape.  A release-acquire would
work fine, as would a lock acquired in both callbacks and then acquired
(and possibly released) before the free.

Thanx, Paul

Re: [PATCH] ARM: dts: omap3-pandora: Add fixed 26MHz clock as fck for twl

[Tony Lindgren]

* H. Nikolaus Schaller  [180405 09:06]:
> The board uses 26MHz oscillator for the twl4030 HFCLK.
> This way we will not depend on the bootloader to configure the
> CFG_BOOT:HFCLK_FREQ
> 
> Signed-off-by: H. Nikolaus Schaller 

Applying into omap-for-v4.18/dt thanks.

Tony

Re: [PATCH] ARM: dts: omap3-pandora: Add fixed 26MHz clock as fck for twl

[Tony Lindgren]

* H. Nikolaus Schaller  [180405 09:06]:
> The board uses 26MHz oscillator for the twl4030 HFCLK.
> This way we will not depend on the bootloader to configure the
> CFG_BOOT:HFCLK_FREQ
> 
> Signed-off-by: H. Nikolaus Schaller 

Applying into omap-for-v4.18/dt thanks.

Tony

Re: [PATCH 2/3] seccomp: Audit attempts to modify the actions_logged sysctl

[Paul Moore]

On Fri, Apr 27, 2018 at 3:16 PM, Tyler Hicks  wrote:
> The decision to log a seccomp action will always be subject to the
> value of the kernel.seccomp.actions_logged sysctl, even for processes
> that are being inspected via the audit subsystem, in an upcoming patch.
> Therefore, we need to emit an audit record on attempts at writing to the
> actions_logged sysctl when auditing is enabled.
>
> This patch updates the write handler for the actions_logged sysctl to
> emit an audit record on attempts to write to the sysctl. Successful
> writes to the sysctl will result in a record that includes a normalized
> list of logged actions in the "actions" field and a "res" field equal to
> 0. Unsuccessful writes to the sysctl will result in a record that
> doesn't include the "actions" field and has a "res" field equal to 1.
>
> Not all unsuccessful writes to the sysctl are audited. For example, an
> audit record will not be emitted if an unprivileged process attempts to
> open the sysctl file for reading since that access control check is not
> part of the sysctl's write handler.
>
> Below are some example audit records when writing various strings to the
> actions_logged sysctl.
>
> Writing "not-a-real-action" emits:
>
>  type=CONFIG_CHANGE msg=audit(1524600971.363:119): pid=1651 uid=0
>  auid=1000 tty=pts8 ses=1 comm="tee" exe="/usr/bin/tee"
>  op=seccomp-logging res=1
>
> Writing "kill_process kill_thread errno trace log" emits:
>
>  type=CONFIG_CHANGE msg=audit(1524601023.982:131): pid=1658 uid=0
>  auid=1000 tty=pts8 ses=1 comm="tee" exe="/usr/bin/tee"
>  op=seccomp-logging actions="kill_process kill_thread errno trace log"
>  res=0

I've got some additional comments regarding the fields in the code
below, but it would be good to hear Steve comment on the "actions"
field since his userspace tools are extremely picky about what they
will accept.  It looks like you are treating the actions as an
untrusted string, which is good, so I suspect you are okay, but still
...

> Writing the string "log log errno trace kill_process kill_thread", which
> is unordered and contains the log action twice, results in the same
> value as the previous example for the actions field:
>
>  type=CONFIG_CHANGE msg=audit(1524601204.365:152): pid=1704 uid=0
>  auid=1000 tty=pts8 ses=1 comm="tee" exe="/usr/bin/tee"
>  op=seccomp-logging actions="kill_process kill_thread errno trace log"
>  res=0
>
> No audit records are generated when reading the actions_logged sysctl.
>
> Suggested-by: Steve Grubb 
> Signed-off-by: Tyler Hicks 
> ---
>  include/linux/audit.h |  3 +++
>  kernel/auditsc.c  | 37 +
>  kernel/seccomp.c  | 43 ++-
>  3 files changed, 74 insertions(+), 9 deletions(-)

...

> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 75d5b03..b311d7d 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -233,6 +233,7 @@ extern void __audit_inode_child(struct inode *parent,
> const struct dentry *dentry,
> const unsigned char type);
>  extern void __audit_seccomp(unsigned long syscall, long signr, int code);
> +extern void audit_seccomp_actions_logged(const char *names, int res);
>  extern void __audit_ptrace(struct task_struct *t);
>
>  static inline bool audit_dummy_context(void)
> @@ -502,6 +503,8 @@ static inline void __audit_seccomp(unsigned long syscall, 
> long signr, int code)
>  { }
>  static inline void audit_seccomp(unsigned long syscall, long signr, int code)
>  { }
> +static inline void audit_seccomp_actions_logged(const char *names, int res)
> +{ }
>  static inline int auditsc_get_stamp(struct audit_context *ctx,
>   struct timespec64 *t, unsigned int *serial)
>  {
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 4e0a4ac..3496238 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2478,6 +2478,43 @@ void __audit_seccomp(unsigned long syscall, long 
> signr, int code)
> audit_log_end(ab);
>  }
>
> +void audit_seccomp_actions_logged(const char *names, int res)
> +{
> +   struct tty_struct *tty;
> +   const struct cred *cred;
> +   struct audit_buffer *ab;
> +   char comm[sizeof(current->comm)];
> +
> +   if (!audit_enabled)
> +   return;
> +
> +   ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
> +   if (unlikely(!ab))
> +   return;

Instead of NULL, let's pass current->audit_context to
audit_log_start().  Yes, most of the AUDIT_CONFIG_CHANGE users pass
NULL but all of that is going to need to change because of 1) the
audit container ID work and 2) it makes sense to connect records that
are related.  Let's do it right the first time here :)

> +   cred = current_cred();
> +   tty = audit_get_tty(current);
> +   audit_log_format(ab, "pid=%d uid=%u 

Re: [PATCH 2/3] seccomp: Audit attempts to modify the actions_logged sysctl

[Paul Moore]

On Fri, Apr 27, 2018 at 3:16 PM, Tyler Hicks  wrote:
> The decision to log a seccomp action will always be subject to the
> value of the kernel.seccomp.actions_logged sysctl, even for processes
> that are being inspected via the audit subsystem, in an upcoming patch.
> Therefore, we need to emit an audit record on attempts at writing to the
> actions_logged sysctl when auditing is enabled.
>
> This patch updates the write handler for the actions_logged sysctl to
> emit an audit record on attempts to write to the sysctl. Successful
> writes to the sysctl will result in a record that includes a normalized
> list of logged actions in the "actions" field and a "res" field equal to
> 0. Unsuccessful writes to the sysctl will result in a record that
> doesn't include the "actions" field and has a "res" field equal to 1.
>
> Not all unsuccessful writes to the sysctl are audited. For example, an
> audit record will not be emitted if an unprivileged process attempts to
> open the sysctl file for reading since that access control check is not
> part of the sysctl's write handler.
>
> Below are some example audit records when writing various strings to the
> actions_logged sysctl.
>
> Writing "not-a-real-action" emits:
>
>  type=CONFIG_CHANGE msg=audit(1524600971.363:119): pid=1651 uid=0
>  auid=1000 tty=pts8 ses=1 comm="tee" exe="/usr/bin/tee"
>  op=seccomp-logging res=1
>
> Writing "kill_process kill_thread errno trace log" emits:
>
>  type=CONFIG_CHANGE msg=audit(1524601023.982:131): pid=1658 uid=0
>  auid=1000 tty=pts8 ses=1 comm="tee" exe="/usr/bin/tee"
>  op=seccomp-logging actions="kill_process kill_thread errno trace log"
>  res=0

I've got some additional comments regarding the fields in the code
below, but it would be good to hear Steve comment on the "actions"
field since his userspace tools are extremely picky about what they
will accept.  It looks like you are treating the actions as an
untrusted string, which is good, so I suspect you are okay, but still
...

> Writing the string "log log errno trace kill_process kill_thread", which
> is unordered and contains the log action twice, results in the same
> value as the previous example for the actions field:
>
>  type=CONFIG_CHANGE msg=audit(1524601204.365:152): pid=1704 uid=0
>  auid=1000 tty=pts8 ses=1 comm="tee" exe="/usr/bin/tee"
>  op=seccomp-logging actions="kill_process kill_thread errno trace log"
>  res=0
>
> No audit records are generated when reading the actions_logged sysctl.
>
> Suggested-by: Steve Grubb 
> Signed-off-by: Tyler Hicks 
> ---
>  include/linux/audit.h |  3 +++
>  kernel/auditsc.c  | 37 +
>  kernel/seccomp.c  | 43 ++-
>  3 files changed, 74 insertions(+), 9 deletions(-)

...

> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 75d5b03..b311d7d 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -233,6 +233,7 @@ extern void __audit_inode_child(struct inode *parent,
> const struct dentry *dentry,
> const unsigned char type);
>  extern void __audit_seccomp(unsigned long syscall, long signr, int code);
> +extern void audit_seccomp_actions_logged(const char *names, int res);
>  extern void __audit_ptrace(struct task_struct *t);
>
>  static inline bool audit_dummy_context(void)
> @@ -502,6 +503,8 @@ static inline void __audit_seccomp(unsigned long syscall, 
> long signr, int code)
>  { }
>  static inline void audit_seccomp(unsigned long syscall, long signr, int code)
>  { }
> +static inline void audit_seccomp_actions_logged(const char *names, int res)
> +{ }
>  static inline int auditsc_get_stamp(struct audit_context *ctx,
>   struct timespec64 *t, unsigned int *serial)
>  {
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 4e0a4ac..3496238 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2478,6 +2478,43 @@ void __audit_seccomp(unsigned long syscall, long 
> signr, int code)
> audit_log_end(ab);
>  }
>
> +void audit_seccomp_actions_logged(const char *names, int res)
> +{
> +   struct tty_struct *tty;
> +   const struct cred *cred;
> +   struct audit_buffer *ab;
> +   char comm[sizeof(current->comm)];
> +
> +   if (!audit_enabled)
> +   return;
> +
> +   ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
> +   if (unlikely(!ab))
> +   return;

Instead of NULL, let's pass current->audit_context to
audit_log_start().  Yes, most of the AUDIT_CONFIG_CHANGE users pass
NULL but all of that is going to need to change because of 1) the
audit container ID work and 2) it makes sense to connect records that
are related.  Let's do it right the first time here :)

> +   cred = current_cred();
> +   tty = audit_get_tty(current);
> +   audit_log_format(ab, "pid=%d uid=%u auid=%u tty=%s ses=%u",
> +

Re: [PATCH] ARM: omap1: osk: use device properties for at24 eeprom

[Tony Lindgren]

* Bartosz Golaszewski  [180404 05:39]:
> We want to work towards phasing out the at24_platform_data structure.
> There are few users and its contents can be represented using generic
> device properties. Using device properties only will allow us to
> significantly simplify the at24 configuration code.
> 
> Remove the at24_platform_data structure and replace it with an array
> of property entries. Drop the byte_len/size property, as the model name
> already implies the EEPROM's size.
> 
> Signed-off-by: Bartosz Golaszewski 

Applying into omap-for-v4.18/omap1 thanks.

Tony

Re: [PATCH] ARM: omap1: osk: use device properties for at24 eeprom

[Tony Lindgren]

* Bartosz Golaszewski  [180404 05:39]:
> We want to work towards phasing out the at24_platform_data structure.
> There are few users and its contents can be represented using generic
> device properties. Using device properties only will allow us to
> significantly simplify the at24 configuration code.
> 
> Remove the at24_platform_data structure and replace it with an array
> of property entries. Drop the byte_len/size property, as the model name
> already implies the EEPROM's size.
> 
> Signed-off-by: Bartosz Golaszewski 

Applying into omap-for-v4.18/omap1 thanks.

Tony

Re: [PATCH RFC v5 5/6] tracepoint: Make rcuidle tracepoint callers use SRCU

[Joel Fernandes]

On Tue, May 1, 2018 at 7:34 AM Paul E. McKenney 
wrote:

> On Tue, May 01, 2018 at 10:24:01AM -0400, Steven Rostedt wrote:
> > On Mon, 30 Apr 2018 18:42:03 -0700
> > Joel Fernandes  wrote:
> >
> > > In recent tests with IRQ on/off tracepoints, a large performance
> > > overhead ~10% is noticed when running hackbench. This is root caused
to
> > > calls to rcu_irq_enter_irqson and rcu_irq_exit_irqson from the
> > > tracepoint code. Following a long discussion on the list [1] about
this,
> > > we concluded that srcu is a better alternative for use during rcu
idle.
> > > Although it does involve extra barriers, its lighter than the
sched-rcu
> > > version which has to do additional RCU calls to notify RCU idle about
> > > entry into RCU sections.
> > >
> > > In this patch, we change the underlying implementation of the
> > > trace_*_rcuidle API to use SRCU. This has shown to improve performance
> > > alot for the high frequency irq enable/disable tracepoints.

> [ . . . ]

> > > --- a/kernel/tracepoint.c
> > > +++ b/kernel/tracepoint.c
> > > @@ -31,6 +31,9 @@
> > >  extern struct tracepoint * const __start___tracepoints_ptrs[];
> > >  extern struct tracepoint * const __stop___tracepoints_ptrs[];
> > >
> > > +DEFINE_SRCU(tracepoint_srcu);
> > > +EXPORT_SYMBOL_GPL(tracepoint_srcu);
> > > +
> > >  /* Set to 1 to enable tracepoint debug output */
> > >  static const int tracepoint_debug;
> > >
> > > @@ -67,11 +70,16 @@ static inline void *allocate_probes(int count)
> > > return p == NULL ? NULL : p->probes;
> > >  }
> > >
> > > -static void rcu_free_old_probes(struct rcu_head *head)
> > > +static void srcu_free_old_probes(struct rcu_head *head)
> > >  {
> > > kfree(container_of(head, struct tp_probes, rcu));
> > >  }
> > >
> > > +static void rcu_free_old_probes(struct rcu_head *head)
> > > +{
> > > +   call_srcu(_srcu, head, srcu_free_old_probes);
> >
> > Hmm, is it OK to call call_srcu() from a call_rcu() callback? I guess
> > it would be.

> It is perfectly legal, and quite a bit simpler than setting something
> up to wait for both to complete concurrently.

Cool. Also in this case if we call both in sequence, then I felt there
could be a race to free the old data since both callbacks would try to do
the same thing. The same thing being freeing of the same set of old probes
which would need some synchronization between the 2 callbacks. With the
chaining, since the ordering is assured there wouldn't be a question of
such a race. I could add this reasoning to the changelog as well.

thanks,

- Joel

Re: [PATCH RFC v5 5/6] tracepoint: Make rcuidle tracepoint callers use SRCU

[Joel Fernandes]

On Tue, May 1, 2018 at 7:34 AM Paul E. McKenney 
wrote:

> On Tue, May 01, 2018 at 10:24:01AM -0400, Steven Rostedt wrote:
> > On Mon, 30 Apr 2018 18:42:03 -0700
> > Joel Fernandes  wrote:
> >
> > > In recent tests with IRQ on/off tracepoints, a large performance
> > > overhead ~10% is noticed when running hackbench. This is root caused
to
> > > calls to rcu_irq_enter_irqson and rcu_irq_exit_irqson from the
> > > tracepoint code. Following a long discussion on the list [1] about
this,
> > > we concluded that srcu is a better alternative for use during rcu
idle.
> > > Although it does involve extra barriers, its lighter than the
sched-rcu
> > > version which has to do additional RCU calls to notify RCU idle about
> > > entry into RCU sections.
> > >
> > > In this patch, we change the underlying implementation of the
> > > trace_*_rcuidle API to use SRCU. This has shown to improve performance
> > > alot for the high frequency irq enable/disable tracepoints.

> [ . . . ]

> > > --- a/kernel/tracepoint.c
> > > +++ b/kernel/tracepoint.c
> > > @@ -31,6 +31,9 @@
> > >  extern struct tracepoint * const __start___tracepoints_ptrs[];
> > >  extern struct tracepoint * const __stop___tracepoints_ptrs[];
> > >
> > > +DEFINE_SRCU(tracepoint_srcu);
> > > +EXPORT_SYMBOL_GPL(tracepoint_srcu);
> > > +
> > >  /* Set to 1 to enable tracepoint debug output */
> > >  static const int tracepoint_debug;
> > >
> > > @@ -67,11 +70,16 @@ static inline void *allocate_probes(int count)
> > > return p == NULL ? NULL : p->probes;
> > >  }
> > >
> > > -static void rcu_free_old_probes(struct rcu_head *head)
> > > +static void srcu_free_old_probes(struct rcu_head *head)
> > >  {
> > > kfree(container_of(head, struct tp_probes, rcu));
> > >  }
> > >
> > > +static void rcu_free_old_probes(struct rcu_head *head)
> > > +{
> > > +   call_srcu(_srcu, head, srcu_free_old_probes);
> >
> > Hmm, is it OK to call call_srcu() from a call_rcu() callback? I guess
> > it would be.

> It is perfectly legal, and quite a bit simpler than setting something
> up to wait for both to complete concurrently.

Cool. Also in this case if we call both in sequence, then I felt there
could be a race to free the old data since both callbacks would try to do
the same thing. The same thing being freeing of the same set of old probes
which would need some synchronization between the 2 callbacks. With the
chaining, since the ordering is assured there wouldn't be a question of
such a race. I could add this reasoning to the changelog as well.

thanks,

- Joel

[PATCH] parport: parport_serial: Add WCH CH382L PCIe single parallel port support

[Colin King]

From: Colin Ian King 

The WCH CH382L PCIe adapter has 1 parallel port but unlike the similar
WCH CH328 adapter there are no serial ports connected to it.

PCIe device ID 1c00:3050:

02:00.0 Serial controller: Device 1c00:3050 (rev 10) (prog-if 05 [16850])
Subsystem: Device 1c00:3050
Flags: fast devsel, IRQ 16
I/O ports at 2000 [size=256]
Memory at d000 (32-bit, prefetchable) [size=32K]
I/O ports at 2100 [size=4]
Expansion ROM at b020 [disabled] [size=32K]
Capabilities: [60] Power Management version 3
Capabilities: [68] MSI: Enable- Count=1/32 Maskable+ 64bit+
Capabilities: [80] Express Legacy Endpoint, MSI 00
Capabilities: [100] Advanced Error Reporting
Kernel driver in use: parport_pc
Kernel modules: parport_pc, parport_serial

Signed-off-by: Colin Ian King 
---
 drivers/parport/parport_serial.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/parport/parport_serial.c b/drivers/parport/parport_serial.c
index ae9e01ef7599..b71cfe0f82e1 100644
--- a/drivers/parport/parport_serial.c
+++ b/drivers/parport/parport_serial.c
@@ -58,6 +58,7 @@ enum parport_pc_pci_cards {
timedia_9079c,
wch_ch353_1s1p,
wch_ch353_2s1p,
+   wch_ch382_0s1p,
wch_ch382_2s1p,
brainboxes_5s1p,
sunix_2s1p,
@@ -147,6 +148,7 @@ static struct parport_pc_pci cards[] = {
/* timedia_9079c */ { 1, { { 2, 3 }, } },
/* wch_ch353_1s1p*/ { 1, { { 1, -1}, } },
/* wch_ch353_2s1p*/ { 1, { { 2, -1}, } },
+   /* wch_ch382_0s1p*/ { 1, { { 2, -1}, } },
/* wch_ch382_2s1p*/ { 1, { { 2, -1}, } },
/* brainboxes_5s1p */   { 1, { { 3, -1 }, } },
/* sunix_2s1p */{ 1, { { 3, -1 }, } },
@@ -252,6 +254,7 @@ static struct pci_device_id parport_serial_pci_tbl[] = {
/* WCH CARDS */
{ 0x4348, 0x5053, PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch353_1s1p},
{ 0x4348, 0x7053, 0x4348, 0x3253, 0, 0, wch_ch353_2s1p},
+   { 0x1c00, 0x3050, 0x1c00, 0x3050, 0, 0, wch_ch382_0s1p},
{ 0x1c00, 0x3250, 0x1c00, 0x3250, 0, 0, wch_ch382_2s1p},
 
/* BrainBoxes PX272/PX306 MIO card */
@@ -494,6 +497,12 @@ static struct pciserial_board pci_parport_serial_boards[] 
= {
.base_baud  = 115200,
.uart_offset= 8,
},
+   [wch_ch382_0s1p] = {
+   .flags  = FL_BASE0,
+   .num_ports  = 0,
+   .base_baud  = 115200,
+   .uart_offset= 8,
+   },
[wch_ch382_2s1p] = {
.flags  = FL_BASE0,
.num_ports  = 2,
-- 
2.17.0

[PATCH] parport: parport_serial: Add WCH CH382L PCIe single parallel port support

[Colin King]

From: Colin Ian King 

The WCH CH382L PCIe adapter has 1 parallel port but unlike the similar
WCH CH328 adapter there are no serial ports connected to it.

PCIe device ID 1c00:3050:

02:00.0 Serial controller: Device 1c00:3050 (rev 10) (prog-if 05 [16850])
Subsystem: Device 1c00:3050
Flags: fast devsel, IRQ 16
I/O ports at 2000 [size=256]
Memory at d000 (32-bit, prefetchable) [size=32K]
I/O ports at 2100 [size=4]
Expansion ROM at b020 [disabled] [size=32K]
Capabilities: [60] Power Management version 3
Capabilities: [68] MSI: Enable- Count=1/32 Maskable+ 64bit+
Capabilities: [80] Express Legacy Endpoint, MSI 00
Capabilities: [100] Advanced Error Reporting
Kernel driver in use: parport_pc
Kernel modules: parport_pc, parport_serial

Signed-off-by: Colin Ian King 
---
 drivers/parport/parport_serial.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/parport/parport_serial.c b/drivers/parport/parport_serial.c
index ae9e01ef7599..b71cfe0f82e1 100644
--- a/drivers/parport/parport_serial.c
+++ b/drivers/parport/parport_serial.c
@@ -58,6 +58,7 @@ enum parport_pc_pci_cards {
timedia_9079c,
wch_ch353_1s1p,
wch_ch353_2s1p,
+   wch_ch382_0s1p,
wch_ch382_2s1p,
brainboxes_5s1p,
sunix_2s1p,
@@ -147,6 +148,7 @@ static struct parport_pc_pci cards[] = {
/* timedia_9079c */ { 1, { { 2, 3 }, } },
/* wch_ch353_1s1p*/ { 1, { { 1, -1}, } },
/* wch_ch353_2s1p*/ { 1, { { 2, -1}, } },
+   /* wch_ch382_0s1p*/ { 1, { { 2, -1}, } },
/* wch_ch382_2s1p*/ { 1, { { 2, -1}, } },
/* brainboxes_5s1p */   { 1, { { 3, -1 }, } },
/* sunix_2s1p */{ 1, { { 3, -1 }, } },
@@ -252,6 +254,7 @@ static struct pci_device_id parport_serial_pci_tbl[] = {
/* WCH CARDS */
{ 0x4348, 0x5053, PCI_ANY_ID, PCI_ANY_ID, 0, 0, wch_ch353_1s1p},
{ 0x4348, 0x7053, 0x4348, 0x3253, 0, 0, wch_ch353_2s1p},
+   { 0x1c00, 0x3050, 0x1c00, 0x3050, 0, 0, wch_ch382_0s1p},
{ 0x1c00, 0x3250, 0x1c00, 0x3250, 0, 0, wch_ch382_2s1p},
 
/* BrainBoxes PX272/PX306 MIO card */
@@ -494,6 +497,12 @@ static struct pciserial_board pci_parport_serial_boards[] 
= {
.base_baud  = 115200,
.uart_offset= 8,
},
+   [wch_ch382_0s1p] = {
+   .flags  = FL_BASE0,
+   .num_ports  = 0,
+   .base_baud  = 115200,
+   .uart_offset= 8,
+   },
[wch_ch382_2s1p] = {
.flags  = FL_BASE0,
.num_ports  = 2,
-- 
2.17.0

Re: [PATCH 2/2] sched: Introduce set_special_state()

[Oleg Nesterov]

On 05/01, Peter Zijlstra wrote:
>
> On Tue, May 01, 2018 at 03:59:24PM +0200, Oleg Nesterov wrote:
> > On 05/01, Peter Zijlstra wrote:
> > >
> > > The only code I found that seems to care is ptrace_attach(), where we
> > > wait for JOBCTL_TRAPPING to get cleared. That same function has a
> > > comment about hiding the STOPPED -> RUNNING -> TRACED transition. So I'm
> > > assuming it needs to observe TRACED if it observes !TRAPPING.
> >
> > Yes, exactly.
> >
> > > But I don't think there's enough barriers on that end to guarantee this.
> > > Any ->state load after wait_on_bit() is, afact, free to have happened
> > > before the ->jobctl load.
> >
> > do_wait() does set_current_state() before it checks ->state or anything 
> > else.
>
> But how are ptrace_attach() and do_wait() related?

Yes.

> I guess I'm missing
> something fairly fundamental here.

You are missing the fact that ptrace API is very old and ugly ;)

Just one example. If the debugger knows that the task is STOPPED then it has
all rights to do, say,

ptrace(PTRACE_ATTACH, pid);
BUG_ON(pid != waitpid(pid, WNOHANG));

Or even do another ptrace() request right after ptrace(PTRACE_ATTACH) returns,
without do_wait().

And unless my memory fools me, gdb even has some test-cases for this... Not 
sure,
but it certainly looks at tracee->state in /proc before it does PTRACE_ATTACH,
because if it was already STOPPED then gdb won't have any notification from the
tracee.

> Anyway, does the below look ok?

Yes, thanks.

Oleg.

Re: [PATCH 2/2] sched: Introduce set_special_state()

[Oleg Nesterov]

On 05/01, Peter Zijlstra wrote:
>
> On Tue, May 01, 2018 at 03:59:24PM +0200, Oleg Nesterov wrote:
> > On 05/01, Peter Zijlstra wrote:
> > >
> > > The only code I found that seems to care is ptrace_attach(), where we
> > > wait for JOBCTL_TRAPPING to get cleared. That same function has a
> > > comment about hiding the STOPPED -> RUNNING -> TRACED transition. So I'm
> > > assuming it needs to observe TRACED if it observes !TRAPPING.
> >
> > Yes, exactly.
> >
> > > But I don't think there's enough barriers on that end to guarantee this.
> > > Any ->state load after wait_on_bit() is, afact, free to have happened
> > > before the ->jobctl load.
> >
> > do_wait() does set_current_state() before it checks ->state or anything 
> > else.
>
> But how are ptrace_attach() and do_wait() related?

Yes.

> I guess I'm missing
> something fairly fundamental here.

You are missing the fact that ptrace API is very old and ugly ;)

Just one example. If the debugger knows that the task is STOPPED then it has
all rights to do, say,

ptrace(PTRACE_ATTACH, pid);
BUG_ON(pid != waitpid(pid, WNOHANG));

Or even do another ptrace() request right after ptrace(PTRACE_ATTACH) returns,
without do_wait().

And unless my memory fools me, gdb even has some test-cases for this... Not 
sure,
but it certainly looks at tracee->state in /proc before it does PTRACE_ATTACH,
because if it was already STOPPED then gdb won't have any notification from the
tracee.

> Anyway, does the below look ok?

Yes, thanks.

Oleg.

Re: [PATCH RFC v5 3/6] srcu: Add notrace variant of srcu_dereference

[Joel Fernandes]

On Tue, May 1, 2018 at 7:06 AM Steven Rostedt  wrote:

> On Mon, 30 Apr 2018 18:42:01 -0700
> Joel Fernandes  wrote:

> > In this series, we are making lockdep use an rcuidle tracepoint. For
> > this reason we need a notrace variant of srcu_dereference since
> > otherwise we get lockdep splats since lockdep hooks may not have run
> > yet. This patch adds the needed variant.

> This change log is rather confusing. Why would lockdep use an rcuidle
> tracepoint? I think we need to explain more here.

Patch 6/6 registers lockdep onto irq_disable and irq_enable tracepoints
which use rcuidle:
https://github.com/torvalds/linux/blob/master/kernel/trace/trace_irqsoff.c#L791

I can add more details to the change log about this.

thanks,

- Joel

Re: [PATCH RFC v5 3/6] srcu: Add notrace variant of srcu_dereference

[Joel Fernandes]

On Tue, May 1, 2018 at 7:06 AM Steven Rostedt  wrote:

> On Mon, 30 Apr 2018 18:42:01 -0700
> Joel Fernandes  wrote:

> > In this series, we are making lockdep use an rcuidle tracepoint. For
> > this reason we need a notrace variant of srcu_dereference since
> > otherwise we get lockdep splats since lockdep hooks may not have run
> > yet. This patch adds the needed variant.

> This change log is rather confusing. Why would lockdep use an rcuidle
> tracepoint? I think we need to explain more here.

Patch 6/6 registers lockdep onto irq_disable and irq_enable tracepoints
which use rcuidle:
https://github.com/torvalds/linux/blob/master/kernel/trace/trace_irqsoff.c#L791

I can add more details to the change log about this.

thanks,

- Joel

Re: [RFC/PATCH] net: ethernet: nixge: Use of_get_mac_address()

[Rob Herring]

On Thu, Apr 26, 2018 at 03:04:01PM -0700, Moritz Fischer wrote:
> On Thu, Apr 26, 2018 at 02:57:42PM -0700, Moritz Fischer wrote:
> > Make nixge driver work with 'mac-address' property instead of
> > 'address' property. There are currently no in-tree users and
> > the only users of this driver are devices that use overlays
> > we control to instantiate the device together with the corresponding
> > FPGA images.
> > 
> > Signed-off-by: Moritz Fischer 
> > ---
> > 
> > Hi David, Rob,
> > 
> > with Mike's change that enable the generic 'mac-address'
> > binding that I barely missed with the submission of this
> > driver I was wondering if we can still change the binding.
> > 
> > I'm aware that this generally is a nonono case, since the binding
> > is considered API, but since there are no users outside of our
> > devicetree overlays that we ship with our devices I thought I'd ask.

Fine by me. It really comes down to whether there are any users that 
would be impacted.

Rob

Re: [RFC/PATCH] net: ethernet: nixge: Use of_get_mac_address()

[Rob Herring]

On Thu, Apr 26, 2018 at 03:04:01PM -0700, Moritz Fischer wrote:
> On Thu, Apr 26, 2018 at 02:57:42PM -0700, Moritz Fischer wrote:
> > Make nixge driver work with 'mac-address' property instead of
> > 'address' property. There are currently no in-tree users and
> > the only users of this driver are devices that use overlays
> > we control to instantiate the device together with the corresponding
> > FPGA images.
> > 
> > Signed-off-by: Moritz Fischer 
> > ---
> > 
> > Hi David, Rob,
> > 
> > with Mike's change that enable the generic 'mac-address'
> > binding that I barely missed with the submission of this
> > driver I was wondering if we can still change the binding.
> > 
> > I'm aware that this generally is a nonono case, since the binding
> > is considered API, but since there are no users outside of our
> > devicetree overlays that we ship with our devices I thought I'd ask.

Fine by me. It really comes down to whether there are any users that 
would be impacted.

Rob

Re: [ANNOUNCE] v4.14.29-rt25

[Grygorii Strashko]

Hi Sebastian,

On 04/23/2018 02:10 PM, Sebastian Andrzej Siewior wrote:
> On 2018-04-23 11:57:39 [-0500], Grygorii Strashko wrote:
>> Sry, but I can't apply it. What's you base?
> 
> please try this patch and latest v4.14-RT

I've tried this (with  v4.14.34-rt27) and I do not see 
rcu_note_context_switch() any more.

Sry, it took some time as i found some instability -
test "stress-ng --class os --all 0 -t 5m " not always finished :(
So, I've tried to rollback to v4.14.29-rt25 and use as TI RT kernel as
pure rt-devel. Still not sure if this is some sort of regression or not.

> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index e305a8f8cd7d..0322503084a5 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -600,6 +600,9 @@ struct task_struct {
>   int migrate_disable_atomic;
>   # endif
>   #endif
> +#ifdef CONFIG_PREEMPT_RT_FULL
> + int sleeping_lock;
> +#endif
>   
>   #ifdef CONFIG_PREEMPT_RCU
>   int rcu_read_lock_nesting;
> @@ -1757,6 +1760,23 @@ static __always_inline bool need_resched(void)
>   return unlikely(tif_need_resched());
>   }
>   
> +#ifdef CONFIG_PREEMPT_RT_FULL
> +static inline void sleeping_lock_inc(void)
> +{
> + current->sleeping_lock++;
> +}
> +
> +static inline void sleeping_lock_dec(void)
> +{
> + current->sleeping_lock--;
> +}
> +
> +#else
> +
> +static inline void sleeping_lock_inc(void) { }
> +static inline void sleeping_lock_dec(void) { }
> +#endif
> +
>   /*
>* Wrappers for p->thread_info->cpu access. No-op on UP.
>*/
> diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
> index 0cb716ba3be0..bac3fb580af6 100644
> --- a/kernel/locking/rtmutex.c
> +++ b/kernel/locking/rtmutex.c
> @@ -1141,6 +1141,7 @@ void __sched rt_spin_lock_slowunlock(struct rt_mutex 
> *lock)
>   
>   void __lockfunc rt_spin_lock(spinlock_t *lock)
>   {
> + sleeping_lock_inc();
>   migrate_disable();
>   spin_acquire(>dep_map, 0, 0, _RET_IP_);
>   rt_spin_lock_fastlock(>lock, rt_spin_lock_slowlock);
> @@ -1155,6 +1156,7 @@ void __lockfunc __rt_spin_lock(struct rt_mutex *lock)
>   #ifdef CONFIG_DEBUG_LOCK_ALLOC
>   void __lockfunc rt_spin_lock_nested(spinlock_t *lock, int subclass)
>   {
> + sleeping_lock_inc();
>   migrate_disable();
>   spin_acquire(>dep_map, subclass, 0, _RET_IP_);
>   rt_spin_lock_fastlock(>lock, rt_spin_lock_slowlock);
> @@ -1168,6 +1170,7 @@ void __lockfunc rt_spin_unlock(spinlock_t *lock)
>   spin_release(>dep_map, 1, _RET_IP_);
>   rt_spin_lock_fastunlock(>lock, rt_spin_lock_slowunlock);
>   migrate_enable();
> + sleeping_lock_dec();
>   }
>   EXPORT_SYMBOL(rt_spin_unlock);
>   
> @@ -1193,12 +1196,15 @@ int __lockfunc rt_spin_trylock(spinlock_t *lock)
>   {
>   int ret;
>   
> + sleeping_lock_inc();
>   migrate_disable();
>   ret = __rt_mutex_trylock(>lock);
> - if (ret)
> + if (ret) {
>   spin_acquire(>dep_map, 0, 1, _RET_IP_);
> - else
> + } else {
>   migrate_enable();
> + sleeping_lock_dec();
> + }
>   return ret;
>   }
>   EXPORT_SYMBOL(rt_spin_trylock);
> @@ -1210,6 +1216,7 @@ int __lockfunc rt_spin_trylock_bh(spinlock_t *lock)
>   local_bh_disable();
>   ret = __rt_mutex_trylock(>lock);
>   if (ret) {
> + sleeping_lock_inc();
>   migrate_disable();
>   spin_acquire(>dep_map, 0, 1, _RET_IP_);
>   } else
> @@ -1225,6 +1232,7 @@ int __lockfunc rt_spin_trylock_irqsave(spinlock_t 
> *lock, unsigned long *flags)
>   *flags = 0;
>   ret = __rt_mutex_trylock(>lock);
>   if (ret) {
> + sleeping_lock_inc();
>   migrate_disable();
>   spin_acquire(>dep_map, 0, 1, _RET_IP_);
>   }
> diff --git a/kernel/locking/rwlock-rt.c b/kernel/locking/rwlock-rt.c
> index aebb7ce25bc6..f2e155b2c4a8 100644
> --- a/kernel/locking/rwlock-rt.c
> +++ b/kernel/locking/rwlock-rt.c
> @@ -305,12 +305,15 @@ int __lockfunc rt_read_trylock(rwlock_t *rwlock)
>   {
>   int ret;
>   
> + sleeping_lock_inc();
>   migrate_disable();
>   ret = do_read_rt_trylock(rwlock);
> - if (ret)
> + if (ret) {
>   rwlock_acquire_read(>dep_map, 0, 1, _RET_IP_);
> - else
> + } else {
>   migrate_enable();
> + sleeping_lock_dec();
> + }
>   return ret;
>   }
>   EXPORT_SYMBOL(rt_read_trylock);
> @@ -319,18 +322,22 @@ int __lockfunc rt_write_trylock(rwlock_t *rwlock)
>   {
>   int ret;
>   
> + sleeping_lock_inc();
>   migrate_disable();
>   ret = do_write_rt_trylock(rwlock);
> - if (ret)
> + if (ret) {
>   rwlock_acquire(>dep_map, 0, 1, _RET_IP_);
> - else
> + } else {
>   migrate_enable();
> + sleeping_lock_dec();
> + }
>   return ret;
>   }
>   EXPORT_SYMBOL(rt_write_trylock);
>   
>   void 

Re: [ANNOUNCE] v4.14.29-rt25

[Grygorii Strashko]

Hi Sebastian,

On 04/23/2018 02:10 PM, Sebastian Andrzej Siewior wrote:
> On 2018-04-23 11:57:39 [-0500], Grygorii Strashko wrote:
>> Sry, but I can't apply it. What's you base?
> 
> please try this patch and latest v4.14-RT

I've tried this (with  v4.14.34-rt27) and I do not see 
rcu_note_context_switch() any more.

Sry, it took some time as i found some instability -
test "stress-ng --class os --all 0 -t 5m " not always finished :(
So, I've tried to rollback to v4.14.29-rt25 and use as TI RT kernel as
pure rt-devel. Still not sure if this is some sort of regression or not.

> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index e305a8f8cd7d..0322503084a5 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -600,6 +600,9 @@ struct task_struct {
>   int migrate_disable_atomic;
>   # endif
>   #endif
> +#ifdef CONFIG_PREEMPT_RT_FULL
> + int sleeping_lock;
> +#endif
>   
>   #ifdef CONFIG_PREEMPT_RCU
>   int rcu_read_lock_nesting;
> @@ -1757,6 +1760,23 @@ static __always_inline bool need_resched(void)
>   return unlikely(tif_need_resched());
>   }
>   
> +#ifdef CONFIG_PREEMPT_RT_FULL
> +static inline void sleeping_lock_inc(void)
> +{
> + current->sleeping_lock++;
> +}
> +
> +static inline void sleeping_lock_dec(void)
> +{
> + current->sleeping_lock--;
> +}
> +
> +#else
> +
> +static inline void sleeping_lock_inc(void) { }
> +static inline void sleeping_lock_dec(void) { }
> +#endif
> +
>   /*
>* Wrappers for p->thread_info->cpu access. No-op on UP.
>*/
> diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
> index 0cb716ba3be0..bac3fb580af6 100644
> --- a/kernel/locking/rtmutex.c
> +++ b/kernel/locking/rtmutex.c
> @@ -1141,6 +1141,7 @@ void __sched rt_spin_lock_slowunlock(struct rt_mutex 
> *lock)
>   
>   void __lockfunc rt_spin_lock(spinlock_t *lock)
>   {
> + sleeping_lock_inc();
>   migrate_disable();
>   spin_acquire(>dep_map, 0, 0, _RET_IP_);
>   rt_spin_lock_fastlock(>lock, rt_spin_lock_slowlock);
> @@ -1155,6 +1156,7 @@ void __lockfunc __rt_spin_lock(struct rt_mutex *lock)
>   #ifdef CONFIG_DEBUG_LOCK_ALLOC
>   void __lockfunc rt_spin_lock_nested(spinlock_t *lock, int subclass)
>   {
> + sleeping_lock_inc();
>   migrate_disable();
>   spin_acquire(>dep_map, subclass, 0, _RET_IP_);
>   rt_spin_lock_fastlock(>lock, rt_spin_lock_slowlock);
> @@ -1168,6 +1170,7 @@ void __lockfunc rt_spin_unlock(spinlock_t *lock)
>   spin_release(>dep_map, 1, _RET_IP_);
>   rt_spin_lock_fastunlock(>lock, rt_spin_lock_slowunlock);
>   migrate_enable();
> + sleeping_lock_dec();
>   }
>   EXPORT_SYMBOL(rt_spin_unlock);
>   
> @@ -1193,12 +1196,15 @@ int __lockfunc rt_spin_trylock(spinlock_t *lock)
>   {
>   int ret;
>   
> + sleeping_lock_inc();
>   migrate_disable();
>   ret = __rt_mutex_trylock(>lock);
> - if (ret)
> + if (ret) {
>   spin_acquire(>dep_map, 0, 1, _RET_IP_);
> - else
> + } else {
>   migrate_enable();
> + sleeping_lock_dec();
> + }
>   return ret;
>   }
>   EXPORT_SYMBOL(rt_spin_trylock);
> @@ -1210,6 +1216,7 @@ int __lockfunc rt_spin_trylock_bh(spinlock_t *lock)
>   local_bh_disable();
>   ret = __rt_mutex_trylock(>lock);
>   if (ret) {
> + sleeping_lock_inc();
>   migrate_disable();
>   spin_acquire(>dep_map, 0, 1, _RET_IP_);
>   } else
> @@ -1225,6 +1232,7 @@ int __lockfunc rt_spin_trylock_irqsave(spinlock_t 
> *lock, unsigned long *flags)
>   *flags = 0;
>   ret = __rt_mutex_trylock(>lock);
>   if (ret) {
> + sleeping_lock_inc();
>   migrate_disable();
>   spin_acquire(>dep_map, 0, 1, _RET_IP_);
>   }
> diff --git a/kernel/locking/rwlock-rt.c b/kernel/locking/rwlock-rt.c
> index aebb7ce25bc6..f2e155b2c4a8 100644
> --- a/kernel/locking/rwlock-rt.c
> +++ b/kernel/locking/rwlock-rt.c
> @@ -305,12 +305,15 @@ int __lockfunc rt_read_trylock(rwlock_t *rwlock)
>   {
>   int ret;
>   
> + sleeping_lock_inc();
>   migrate_disable();
>   ret = do_read_rt_trylock(rwlock);
> - if (ret)
> + if (ret) {
>   rwlock_acquire_read(>dep_map, 0, 1, _RET_IP_);
> - else
> + } else {
>   migrate_enable();
> + sleeping_lock_dec();
> + }
>   return ret;
>   }
>   EXPORT_SYMBOL(rt_read_trylock);
> @@ -319,18 +322,22 @@ int __lockfunc rt_write_trylock(rwlock_t *rwlock)
>   {
>   int ret;
>   
> + sleeping_lock_inc();
>   migrate_disable();
>   ret = do_write_rt_trylock(rwlock);
> - if (ret)
> + if (ret) {
>   rwlock_acquire(>dep_map, 0, 1, _RET_IP_);
> - else
> + } else {
>   migrate_enable();
> + sleeping_lock_dec();
> + }
>   return ret;
>   }
>   EXPORT_SYMBOL(rt_write_trylock);
>   
>   void 

Re: [PATCH RFC v5 3/6] srcu: Add notrace variant of srcu_dereference

[Steven Rostedt]

On Tue, 1 May 2018 07:18:17 -0700
"Paul E. McKenney"  wrote:


> > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > index 2ec618979b20..a1c4947be877 100644
> > --- a/include/linux/srcu.h
> > +++ b/include/linux/srcu.h
> > @@ -135,6 +135,11 @@ static inline int srcu_read_lock_held(const struct 
> > srcu_struct *sp)
> >   */
> >  #define srcu_dereference(p, sp) srcu_dereference_check((p), (sp), 0)
> > 
> > +/**
> > + * srcu_dereference_notrace - no tracing and no lockdep calls from here
> > + */
> > +#define srcu_dereference_notrace(p, sp) srcu_dereference_check((p), (sp), 
> > 1)  
> 
> Given that this is a one-liner, why not just use srcu_dereference_check()
> directly, with 1 as you do above to disable lockdep?

I prefer what Joel did. It documents why we are doing this. Open coding
the call directly will just confuse others that touch tracepoint code.

-- Steve

Re: [PATCH RFC v5 3/6] srcu: Add notrace variant of srcu_dereference

[Steven Rostedt]

On Tue, 1 May 2018 07:18:17 -0700
"Paul E. McKenney"  wrote:


> > diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> > index 2ec618979b20..a1c4947be877 100644
> > --- a/include/linux/srcu.h
> > +++ b/include/linux/srcu.h
> > @@ -135,6 +135,11 @@ static inline int srcu_read_lock_held(const struct 
> > srcu_struct *sp)
> >   */
> >  #define srcu_dereference(p, sp) srcu_dereference_check((p), (sp), 0)
> > 
> > +/**
> > + * srcu_dereference_notrace - no tracing and no lockdep calls from here
> > + */
> > +#define srcu_dereference_notrace(p, sp) srcu_dereference_check((p), (sp), 
> > 1)  
> 
> Given that this is a one-liner, why not just use srcu_dereference_check()
> directly, with 1 as you do above to disable lockdep?

I prefer what Joel did. It documents why we are doing this. Open coding
the call directly will just confuse others that touch tracepoint code.

-- Steve

Re: [PATCH 4.4 00/44] 4.4.131-stable review

[Greg Kroah-Hartman]

On Mon, Apr 30, 2018 at 04:55:04PM -0700, Nathan Chancellor wrote:
> On Mon, Apr 30, 2018 at 12:24:11PM -0700, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.4.131 release.
> > There are 44 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Wed May  2 19:09:34 UTC 2018.
> > Anything received after that time might be too late.
> > 
> > The whole patch series can be found in one patch at:
> > 
> > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.4.131-rc1.gz
> > or in the git tree and branch at:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> > linux-4.4.y
> > and the diffstat can be found below.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Merged, compiled, and installed onto my Pixel 2 XL and OnePlus 5.
> 
> No build warnings from GCC 4.9.4, GCC 7.3.0, and Clang 5 through 7.
> 
> No initial issues in dmesg or general usage.

Thanks for testing the two of these and letting me know.

greg k-h

Re: [PATCH 4.4 00/44] 4.4.131-stable review

[Greg Kroah-Hartman]

On Mon, Apr 30, 2018 at 04:55:04PM -0700, Nathan Chancellor wrote:
> On Mon, Apr 30, 2018 at 12:24:11PM -0700, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.4.131 release.
> > There are 44 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Wed May  2 19:09:34 UTC 2018.
> > Anything received after that time might be too late.
> > 
> > The whole patch series can be found in one patch at:
> > 
> > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.4.131-rc1.gz
> > or in the git tree and branch at:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> > linux-4.4.y
> > and the diffstat can be found below.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Merged, compiled, and installed onto my Pixel 2 XL and OnePlus 5.
> 
> No build warnings from GCC 4.9.4, GCC 7.3.0, and Clang 5 through 7.
> 
> No initial issues in dmesg or general usage.

Thanks for testing the two of these and letting me know.

greg k-h

Re: [PATCH 4.16 000/113] 4.16.7-stable review

[Greg Kroah-Hartman]

On Tue, May 01, 2018 at 08:36:34AM -0500, Dan Rue wrote:
> On Mon, Apr 30, 2018 at 12:23:31PM -0700, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.16.7 release.
> > There are 113 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Wed May  2 18:39:46 UTC 2018.
> > Anything received after that time might be too late.
> 
> Results from Linaro’s test farm.
> No regressions on arm64, arm and x86_64.

Yeah, I didn't break anything!  :)

thanks for testing all of these and letting me know.

greg k-h

Re: [PATCH 4.16 000/113] 4.16.7-stable review

[Greg Kroah-Hartman]

On Tue, May 01, 2018 at 08:36:34AM -0500, Dan Rue wrote:
> On Mon, Apr 30, 2018 at 12:23:31PM -0700, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.16.7 release.
> > There are 113 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Wed May  2 18:39:46 UTC 2018.
> > Anything received after that time might be too late.
> 
> Results from Linaro’s test farm.
> No regressions on arm64, arm and x86_64.

Yeah, I didn't break anything!  :)

thanks for testing all of these and letting me know.

greg k-h

Re: [PATCH 3.18 00/25] 3.18.108-stable review

[Greg Kroah-Hartman]

On Tue, May 01, 2018 at 11:44:52AM +0530, Harsh Shandilya wrote:
> On 1 May 2018 12:53:07 AM IST, Greg Kroah-Hartman 
>  wrote:
> >This is the start of the stable review cycle for the 3.18.108 release.
> >There are 25 patches in this series, all will be posted as a response
> >to this one.  If anyone has any issues with these being applied, please
> >let me know.
> >
> >Responses should be made by Wed May  2 18:39:02 UTC 2018.
> >Anything received after that time might be too late.
> >
> >The whole patch series can be found in one patch at:
> > 
> > https://www.kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.108-rc1.gz
> >or in the git tree and branch at:
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> >linux-3.18.y
> >and the diffstat can be found below.
> >
> >thanks,
> >
> >greg k-h
> Builds and boots on the OnePlus3T, no immediate regressions. Thanks for the 
> update!

thanks for testing!

greg k-h

Re: [PATCH 3.18 00/25] 3.18.108-stable review

[Greg Kroah-Hartman]

On Tue, May 01, 2018 at 11:44:52AM +0530, Harsh Shandilya wrote:
> On 1 May 2018 12:53:07 AM IST, Greg Kroah-Hartman 
>  wrote:
> >This is the start of the stable review cycle for the 3.18.108 release.
> >There are 25 patches in this series, all will be posted as a response
> >to this one.  If anyone has any issues with these being applied, please
> >let me know.
> >
> >Responses should be made by Wed May  2 18:39:02 UTC 2018.
> >Anything received after that time might be too late.
> >
> >The whole patch series can be found in one patch at:
> > 
> > https://www.kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.108-rc1.gz
> >or in the git tree and branch at:
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> >linux-3.18.y
> >and the diffstat can be found below.
> >
> >thanks,
> >
> >greg k-h
> Builds and boots on the OnePlus3T, no immediate regressions. Thanks for the 
> update!

thanks for testing!

greg k-h