Re: [PATCH 2/5] i2c: Add STM32F4 I2C driver

[Maxime Coquelin]

Hi Cedric,

2016-06-02 17:35 GMT+02:00 M'boumba Cedric Madianga :
> Hi,
>
>>> +
>>> +/**
>>> + * stm32f4_i2c_xfer() - Transfer combined I2C message
>>> + * @i2c_adap: Adapter pointer to the controller
>>> + * @msgs: Pointer to data to be written.
>>> + * @num: Number of messages to be executed
>>> + */
>>> +static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg 
>>> msgs[],
>>> +   int num)
>>> +{
>>> +   struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
>>> +   int ret, i;
>>> +
>>> +   i2c_dev->busy = true;
>>> +
>>> +   ret = clk_prepare_enable(i2c_dev->clk);
>>> +   if (ret) {
>>> +   dev_err(i2c_dev->dev, "Failed to prepare_enable clock\n");
>>> +   return ret;
>>> +   }
>>> +
>>> +   stm32f4_i2c_hw_config(i2c_dev);
>> Maybe you could call this only at probe and resume time?
>> You would save some register accesses.
> Some clarification about this point.
> We need to call stm32f4_i2c_hw_config before each I2C transfer as at
> the end of I2C communication the peripheral is automatically disabled
> and configuration registers are reset.

Ok, but I wonder how the IP knows this is the last i2c message to be sent?
Or maybe it gets re-initialized as soon as the clk is disabled?

Thanks for the inputs,
Maxime

Re: [PATCH 2/5] i2c: Add STM32F4 I2C driver

[Maxime Coquelin]

Hi Cedric,

2016-06-02 17:35 GMT+02:00 M'boumba Cedric Madianga :
> Hi,
>
>>> +
>>> +/**
>>> + * stm32f4_i2c_xfer() - Transfer combined I2C message
>>> + * @i2c_adap: Adapter pointer to the controller
>>> + * @msgs: Pointer to data to be written.
>>> + * @num: Number of messages to be executed
>>> + */
>>> +static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg 
>>> msgs[],
>>> +   int num)
>>> +{
>>> +   struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
>>> +   int ret, i;
>>> +
>>> +   i2c_dev->busy = true;
>>> +
>>> +   ret = clk_prepare_enable(i2c_dev->clk);
>>> +   if (ret) {
>>> +   dev_err(i2c_dev->dev, "Failed to prepare_enable clock\n");
>>> +   return ret;
>>> +   }
>>> +
>>> +   stm32f4_i2c_hw_config(i2c_dev);
>> Maybe you could call this only at probe and resume time?
>> You would save some register accesses.
> Some clarification about this point.
> We need to call stm32f4_i2c_hw_config before each I2C transfer as at
> the end of I2C communication the peripheral is automatically disabled
> and configuration registers are reset.

Ok, but I wonder how the IP knows this is the last i2c message to be sent?
Or maybe it gets re-initialized as soon as the clk is disabled?

Thanks for the inputs,
Maxime

Re: [PATCH 0/4] mtd: nand: Add support for Evatronix NANDFLASH-CTRL

[Boris Brezillon]

Hi Ricard,

I was not in Cc of this series, so you're either developing an old
kernel version, or you didn't check the MAINTAINERS file (or didn't run
get_maintainer.pl on your series). And please, next time make sure
patches 1 to X are sent in replies to your cover letter.

On Thu, 2 Jun 2016 09:46:31 +0200
Ricard Wanderlof  wrote:

> This patch set adds a driver and relevant devicetree bindings for the
> Evatronix NANDFLASH-CTRL NAND flash controller IP. This controller is
> used in the Axis ARTPEC-6 SoC.
> 
> The driver supports BCH ECC using the controller's hardware, but there is
> also an option to use software BCH ECC. However, the ECC layouts are not
> compatible so it's not possible to mix them. The main advantage to using
> software ECC is that there are more OOB bytes free, as the hardware is
> slightly wasteful on OOB space.
> 
> BCH ECC from 4 to 32 bits over 256, 512 or 1024 byte ECC blocks is supported.
> 
> Only large-page flash chips are supported, using 4 or 5 address cycles.
> 
> The driver has been extensively tested using hardware ECC on 2 Mbit flash 
> chips,
> with 8 bit ECC over 512 bytes ECC blocks.

I'll to review the driver soon.

Regards,

Boris

> 
> Ricard Wanderlof (4):
>   of: Add device tree bindings for Evatronix NANDFLASH-CTRL
>   dts: Add Evatronix NAND flash driver to ARTPEC-6 dtsi
>   mtd: nand: Add support for Evatronix NANDFLASH-CTRL
>   MAINTAINERS: mtd: Add maintainer for Evatronix NAND flash driver
> 
>  .../devicetree/bindings/mtd/evatronix-nand.txt |   44 +
>  .../devicetree/bindings/vendor-prefixes.txt|1 +
>  MAINTAINERS|6 +
>  arch/arm/boot/dts/artpec6.dtsi |   19 +
>  drivers/mtd/nand/Kconfig   |6 +
>  drivers/mtd/nand/Makefile  |1 +
>  drivers/mtd/nand/evatronix_nand.c  | 1909 
> 
>  7 files changed, 1986 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/evatronix-nand.txt
>  create mode 100644 drivers/mtd/nand/evatronix_nand.c
> 



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

Re: [PATCH 0/4] mtd: nand: Add support for Evatronix NANDFLASH-CTRL

[Boris Brezillon]

Hi Ricard,

I was not in Cc of this series, so you're either developing an old
kernel version, or you didn't check the MAINTAINERS file (or didn't run
get_maintainer.pl on your series). And please, next time make sure
patches 1 to X are sent in replies to your cover letter.

On Thu, 2 Jun 2016 09:46:31 +0200
Ricard Wanderlof  wrote:

> This patch set adds a driver and relevant devicetree bindings for the
> Evatronix NANDFLASH-CTRL NAND flash controller IP. This controller is
> used in the Axis ARTPEC-6 SoC.
> 
> The driver supports BCH ECC using the controller's hardware, but there is
> also an option to use software BCH ECC. However, the ECC layouts are not
> compatible so it's not possible to mix them. The main advantage to using
> software ECC is that there are more OOB bytes free, as the hardware is
> slightly wasteful on OOB space.
> 
> BCH ECC from 4 to 32 bits over 256, 512 or 1024 byte ECC blocks is supported.
> 
> Only large-page flash chips are supported, using 4 or 5 address cycles.
> 
> The driver has been extensively tested using hardware ECC on 2 Mbit flash 
> chips,
> with 8 bit ECC over 512 bytes ECC blocks.

I'll to review the driver soon.

Regards,

Boris

> 
> Ricard Wanderlof (4):
>   of: Add device tree bindings for Evatronix NANDFLASH-CTRL
>   dts: Add Evatronix NAND flash driver to ARTPEC-6 dtsi
>   mtd: nand: Add support for Evatronix NANDFLASH-CTRL
>   MAINTAINERS: mtd: Add maintainer for Evatronix NAND flash driver
> 
>  .../devicetree/bindings/mtd/evatronix-nand.txt |   44 +
>  .../devicetree/bindings/vendor-prefixes.txt|1 +
>  MAINTAINERS|6 +
>  arch/arm/boot/dts/artpec6.dtsi |   19 +
>  drivers/mtd/nand/Kconfig   |6 +
>  drivers/mtd/nand/Makefile  |1 +
>  drivers/mtd/nand/evatronix_nand.c  | 1909 
> 
>  7 files changed, 1986 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/evatronix-nand.txt
>  create mode 100644 drivers/mtd/nand/evatronix_nand.c
> 



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

authenc methods vs FIPS in light of unencrypted associated data

[Marcus Meissner]

Hi,

In February I already tagged some authenc ciphers for FIPS compatibility.

I currently revisit this to get testmgr running all the tests in strict FIPS 
mode.

The authenc() class is troublesome.

There is a HASH + ENC part of this method, but you can also add associated data,
which is not encrypted. (using the ctx->null cipher in crypto/authenc.c)

But in FIPS mode the crypto_authenc_init_tfm does:

null = crypto_get_default_null_skcipher();

which results in error, as the crypto_alloc_blkcipher("ecb(cipher_null)", 0, 0);
results in failure due to "ecb(cipher_null)" not FIPS compliant.

How to handle this?

I think GCM also does not encrypt, just hashes, the associated data, it just 
does
copy the content itself and does not use a virtual cipher.

Ciao, Marcus

authenc methods vs FIPS in light of unencrypted associated data

[Marcus Meissner]

Hi,

In February I already tagged some authenc ciphers for FIPS compatibility.

I currently revisit this to get testmgr running all the tests in strict FIPS 
mode.

The authenc() class is troublesome.

There is a HASH + ENC part of this method, but you can also add associated data,
which is not encrypted. (using the ctx->null cipher in crypto/authenc.c)

But in FIPS mode the crypto_authenc_init_tfm does:

null = crypto_get_default_null_skcipher();

which results in error, as the crypto_alloc_blkcipher("ecb(cipher_null)", 0, 0);
results in failure due to "ecb(cipher_null)" not FIPS compliant.

How to handle this?

I think GCM also does not encrypt, just hashes, the associated data, it just 
does
copy the content itself and does not use a virtual cipher.

Ciao, Marcus

[PATCH] MAINTAINERS: Remove Muli Ben-Yahuda from Calgary x86-64 IOMMU entry

[Krzysztof Kozlowski]

Muli Ben-Yahuda's email bounces so remove him from Calgary IOMMU. He is
already present in CREDITS for that.

Cc: disc...@x86-64.org
Cc: Jon D. Mason 
Signed-off-by: Krzysztof Kozlowski 
---
 MAINTAINERS | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4f74a5fabc40..a471a5e1080a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2771,7 +2771,6 @@ F:include/net/caif/
 F: net/caif/
 
 CALGARY x86-64 IOMMU
-M: Muli Ben-Yehuda 
 M: "Jon D. Mason" 
 L: disc...@x86-64.org
 S: Maintained
-- 
1.9.1

[PATCH] MAINTAINERS: Remove Muli Ben-Yahuda from Calgary x86-64 IOMMU entry

[Krzysztof Kozlowski]

Muli Ben-Yahuda's email bounces so remove him from Calgary IOMMU. He is
already present in CREDITS for that.

Cc: disc...@x86-64.org
Cc: Jon D. Mason 
Signed-off-by: Krzysztof Kozlowski 
---
 MAINTAINERS | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4f74a5fabc40..a471a5e1080a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2771,7 +2771,6 @@ F:include/net/caif/
 F: net/caif/
 
 CALGARY x86-64 IOMMU
-M: Muli Ben-Yehuda 
 M: "Jon D. Mason" 
 L: disc...@x86-64.org
 S: Maintained
-- 
1.9.1

Re: [RFC PATCH 3/3] sched/fair: Change @running of __update_load_avg() to @update_util

[Dietmar Eggemann]

On 01/06/16 21:11, Peter Zijlstra wrote:
> On Wed, Jun 01, 2016 at 08:39:22PM +0100, Dietmar Eggemann wrote:
>> The information whether a se/cfs_rq should get its load and
>> utilization (se representing a task and root cfs_rq) or only its load
>> (se representing a task group and cfs_rq owned by this se) updated can
>> be passed into __update_load_avg() to avoid the additional if/else
>> condition to set update_util.
>>
>> @running is changed to @update_util which now carries the information if
>> the utilization of the se/cfs_rq should be updated and if the se/cfs_rq
>> is running or not.
>>
>> Signed-off-by: Dietmar Eggemann 
>> ---
>>  kernel/sched/fair.c | 42 +-
>>  1 file changed, 21 insertions(+), 21 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 3ae8e79fb687..a1c13975cf56 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -2669,6 +2669,10 @@ static u32 __compute_runnable_contrib(u64 n)
>>  
>>  #define cap_scale(v, s) ((v)*(s) >> SCHED_CAPACITY_SHIFT)
>>  
>> +#define upd_util_se(se, rng) ((entity_is_task(se) << 1) | (rng))
> 
> Just saying that on first reading that went: Random Number Generator, uh
> what?!
> 
> So maybe pick better names?

Yeah, can do. What about?

#define update_util_se(se, running) ((entity_is_task(se) << 1) | (running))
#define update_util_rq(cfs_rq) ((rq_is_root(cfs_rq) << 1) | !!(cfs_rq)->curr)

 

Re: [RFC PATCH 3/3] sched/fair: Change @running of __update_load_avg() to @update_util

[Dietmar Eggemann]

On 01/06/16 21:11, Peter Zijlstra wrote:
> On Wed, Jun 01, 2016 at 08:39:22PM +0100, Dietmar Eggemann wrote:
>> The information whether a se/cfs_rq should get its load and
>> utilization (se representing a task and root cfs_rq) or only its load
>> (se representing a task group and cfs_rq owned by this se) updated can
>> be passed into __update_load_avg() to avoid the additional if/else
>> condition to set update_util.
>>
>> @running is changed to @update_util which now carries the information if
>> the utilization of the se/cfs_rq should be updated and if the se/cfs_rq
>> is running or not.
>>
>> Signed-off-by: Dietmar Eggemann 
>> ---
>>  kernel/sched/fair.c | 42 +-
>>  1 file changed, 21 insertions(+), 21 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 3ae8e79fb687..a1c13975cf56 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -2669,6 +2669,10 @@ static u32 __compute_runnable_contrib(u64 n)
>>  
>>  #define cap_scale(v, s) ((v)*(s) >> SCHED_CAPACITY_SHIFT)
>>  
>> +#define upd_util_se(se, rng) ((entity_is_task(se) << 1) | (rng))
> 
> Just saying that on first reading that went: Random Number Generator, uh
> what?!
> 
> So maybe pick better names?

Yeah, can do. What about?

#define update_util_se(se, running) ((entity_is_task(se) << 1) | (running))
#define update_util_rq(cfs_rq) ((rq_is_root(cfs_rq) << 1) | !!(cfs_rq)->curr)

 

Re: [PATCH -v4 5/7] locking, arch: Update spin_unlock_wait()

[Boqun Feng]

On Thu, Jun 02, 2016 at 11:11:07PM +0800, Boqun Feng wrote:
[snip]
> 
> OK, I will resend a new patch making spin_unlock_wait() align the
> semantics in your series.
> 

I realize that if my patch goes first then it's more safe and convenient
to keep the two smp_mb()s in ppc arch_spin_unlock_wait(). I will only do
fix and clean up in my patch and leave the semantics changing part to
you ;-)

> Regards,
> Boqun




signature.asc
Description: PGP signature

Re: [PATCH -v4 5/7] locking, arch: Update spin_unlock_wait()

[Boqun Feng]

On Thu, Jun 02, 2016 at 11:11:07PM +0800, Boqun Feng wrote:
[snip]
> 
> OK, I will resend a new patch making spin_unlock_wait() align the
> semantics in your series.
> 

I realize that if my patch goes first then it's more safe and convenient
to keep the two smp_mb()s in ppc arch_spin_unlock_wait(). I will only do
fix and clean up in my patch and leave the semantics changing part to
you ;-)

> Regards,
> Boqun




signature.asc
Description: PGP signature

Re: [RFC PATCH 1/3] sched/fair: Aggregate task utilization only on root cfs_rq

[Dietmar Eggemann]

On 02/06/16 10:23, Juri Lelli wrote:

>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 218f8e83db73..212becd3708f 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -2705,6 +2705,7 @@ __update_load_avg(u64 now, int cpu, struct sched_avg 
>> *sa,
>>  u32 contrib;
>>  unsigned int delta_w, scaled_delta_w, decayed = 0;
>>  unsigned long scale_freq, scale_cpu;
>> +int update_util = 0;
>>  
>>  delta = now - sa->last_update_time;
>>  /*
>> @@ -2725,6 +2726,12 @@ __update_load_avg(u64 now, int cpu, struct sched_avg 
>> *sa,
>>  return 0;
>>  sa->last_update_time = now;
>>  
>> +if (cfs_rq) {
>> +if (_of(cfs_rq)->cfs == cfs_rq)
> 
> Maybe we can wrap this sort of checks in a static inline improving
> readability?

Something like this?

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 218f8e83db73..01b0fa689ef9 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -251,6 +251,18 @@ static inline struct rq *rq_of(struct cfs_rq *cfs_rq)
return cfs_rq->rq;
 }
 
+/* cfs_rq "owned" by the root task group */
+static inline struct cfs_rq *root_rq_of(struct cfs_rq *cfs_rq)
+{
+   return _of(cfs_rq)->cfs;
+}
+
+/* Is this cfs_rq "owned" by the root task group ? */
+static inline bool rq_is_root(struct cfs_rq *cfs_rq)
+{
+   return root_rq_of(cfs_rq) == cfs_rq;
+}
+
 /* An entity is a task if it doesn't "own" a runqueue */
 #define entity_is_task(se) (!se->my_q)
 
@@ -376,6 +388,16 @@ static inline struct rq *rq_of(struct cfs_rq *cfs_rq)
return container_of(cfs_rq, struct rq, cfs);
 }
 
+static inline struct cfs_rq *root_rq_of(struct cfs_rq *cfs_rq)
+{
+   return cfs_rq;
+}
+
+static inline bool rq_is_root(struct cfs_rq *cfs_rq)
+{
+   return true;
+}
+
 #define entity_is_task(se) 1

Re: [RFC PATCH 1/3] sched/fair: Aggregate task utilization only on root cfs_rq

[Dietmar Eggemann]

On 02/06/16 10:23, Juri Lelli wrote:

>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 218f8e83db73..212becd3708f 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -2705,6 +2705,7 @@ __update_load_avg(u64 now, int cpu, struct sched_avg 
>> *sa,
>>  u32 contrib;
>>  unsigned int delta_w, scaled_delta_w, decayed = 0;
>>  unsigned long scale_freq, scale_cpu;
>> +int update_util = 0;
>>  
>>  delta = now - sa->last_update_time;
>>  /*
>> @@ -2725,6 +2726,12 @@ __update_load_avg(u64 now, int cpu, struct sched_avg 
>> *sa,
>>  return 0;
>>  sa->last_update_time = now;
>>  
>> +if (cfs_rq) {
>> +if (_of(cfs_rq)->cfs == cfs_rq)
> 
> Maybe we can wrap this sort of checks in a static inline improving
> readability?

Something like this?

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 218f8e83db73..01b0fa689ef9 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -251,6 +251,18 @@ static inline struct rq *rq_of(struct cfs_rq *cfs_rq)
return cfs_rq->rq;
 }
 
+/* cfs_rq "owned" by the root task group */
+static inline struct cfs_rq *root_rq_of(struct cfs_rq *cfs_rq)
+{
+   return _of(cfs_rq)->cfs;
+}
+
+/* Is this cfs_rq "owned" by the root task group ? */
+static inline bool rq_is_root(struct cfs_rq *cfs_rq)
+{
+   return root_rq_of(cfs_rq) == cfs_rq;
+}
+
 /* An entity is a task if it doesn't "own" a runqueue */
 #define entity_is_task(se) (!se->my_q)
 
@@ -376,6 +388,16 @@ static inline struct rq *rq_of(struct cfs_rq *cfs_rq)
return container_of(cfs_rq, struct rq, cfs);
 }
 
+static inline struct cfs_rq *root_rq_of(struct cfs_rq *cfs_rq)
+{
+   return cfs_rq;
+}
+
+static inline bool rq_is_root(struct cfs_rq *cfs_rq)
+{
+   return true;
+}
+
 #define entity_is_task(se) 1

Re: [BUG/REGRESSION] THP: broken page count after commit aa88b68c

[Kirill A. Shutemov]

On Thu, Jun 02, 2016 at 05:21:41PM +0200, Gerald Schaefer wrote:
> Christian Borntraeger reported a kernel panic after corrupt page counts,
> and it turned out to be a regression introduced with commit aa88b68c
> "thp: keep huge zero page pinned until tlb flush", at least on s390.
> 
> put_huge_zero_page() was moved over from zap_huge_pmd() to release_pages(),
> and it was replaced by tlb_remove_page(). However, release_pages() might
> not always be triggered by (the arch-specific) tlb_remove_page().
> 
> On s390 we call free_page_and_swap_cache() from tlb_remove_page(), and not
> tlb_flush_mmu() -> free_pages_and_swap_cache() like the generic version,
> because we don't use the MMU-gather logic. Although both functions have very
> similar names, they are doing very unsimilar things, in particular
> free_page_xxx is just doing a put_page(), while free_pages_xxx calls
> release_pages().
> 
> This of course results in very harmful put_page()s on the huge zero page,
> on architectures where tlb_remove_page() is implemented in this way. It
> seems to affect only s390 and sh, but sh doesn't have THP support, so
> the problem (currently) probably only exists on s390.
> 
> The following quick hack fixed the issue:
> 
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 0d457e7..c99463a 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -252,7 +252,10 @@ static inline void free_swap_cache(struct page *page)
>  void free_page_and_swap_cache(struct page *page)
>  {
>   free_swap_cache(page);
> - put_page(page);
> + if (is_huge_zero_page(page))
> + put_huge_zero_page();
> + else
> + put_page(page);
>  }
>  
>  /*

The fix looks good to me.

> But of course there might be a better solution, and there still are some
> questions left:
> - Why does free_page_xxx() behave so differently from free_pages_xxx()?

I don't see it behave too deiferently. It just try to batch freeing to
lower locking overhead.

> - Would it be OK to implement free_page_xxx() by calling free_pages_xxx()
>   with nr = 1, similar to free_page() vs. free_pages()?
> - Would it be OK to replace the put_page() in free_page_xxx() with a call
>   to release_pages() with nr = 1?

release_pages() somewhat suboptimal for nr=1. I guess we can fix this with
shortcut to put_page() at start of release_page() if nr == 1.

> - Would it be better to fix this in the arch-specific tlb_remove_page(),
>   by calling free_pages_xxx() with nr = 1 instead of free_page_xxx()?
> 
> Regards,
> Gerald
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

-- 
 Kirill A. Shutemov

Re: [BUG/REGRESSION] THP: broken page count after commit aa88b68c

[Kirill A. Shutemov]

On Thu, Jun 02, 2016 at 05:21:41PM +0200, Gerald Schaefer wrote:
> Christian Borntraeger reported a kernel panic after corrupt page counts,
> and it turned out to be a regression introduced with commit aa88b68c
> "thp: keep huge zero page pinned until tlb flush", at least on s390.
> 
> put_huge_zero_page() was moved over from zap_huge_pmd() to release_pages(),
> and it was replaced by tlb_remove_page(). However, release_pages() might
> not always be triggered by (the arch-specific) tlb_remove_page().
> 
> On s390 we call free_page_and_swap_cache() from tlb_remove_page(), and not
> tlb_flush_mmu() -> free_pages_and_swap_cache() like the generic version,
> because we don't use the MMU-gather logic. Although both functions have very
> similar names, they are doing very unsimilar things, in particular
> free_page_xxx is just doing a put_page(), while free_pages_xxx calls
> release_pages().
> 
> This of course results in very harmful put_page()s on the huge zero page,
> on architectures where tlb_remove_page() is implemented in this way. It
> seems to affect only s390 and sh, but sh doesn't have THP support, so
> the problem (currently) probably only exists on s390.
> 
> The following quick hack fixed the issue:
> 
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 0d457e7..c99463a 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -252,7 +252,10 @@ static inline void free_swap_cache(struct page *page)
>  void free_page_and_swap_cache(struct page *page)
>  {
>   free_swap_cache(page);
> - put_page(page);
> + if (is_huge_zero_page(page))
> + put_huge_zero_page();
> + else
> + put_page(page);
>  }
>  
>  /*

The fix looks good to me.

> But of course there might be a better solution, and there still are some
> questions left:
> - Why does free_page_xxx() behave so differently from free_pages_xxx()?

I don't see it behave too deiferently. It just try to batch freeing to
lower locking overhead.

> - Would it be OK to implement free_page_xxx() by calling free_pages_xxx()
>   with nr = 1, similar to free_page() vs. free_pages()?
> - Would it be OK to replace the put_page() in free_page_xxx() with a call
>   to release_pages() with nr = 1?

release_pages() somewhat suboptimal for nr=1. I guess we can fix this with
shortcut to put_page() at start of release_page() if nr == 1.

> - Would it be better to fix this in the arch-specific tlb_remove_page(),
>   by calling free_pages_xxx() with nr = 1 instead of free_page_xxx()?
> 
> Regards,
> Gerald
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

-- 
 Kirill A. Shutemov

[PATCH tty-next] devpts: Make each mount of devpts an independent filesystem.

[Eric W. Biederman]

The /dev/ptmx device node is changed to lookup the directory entry
"pts" in the same directory as the /dev/ptmx device node was opened
in.  If there is a "pts" entry and that entry is a devpts filesystem
/dev/ptmx uses that filesystem.  Otherwise the open of /dev/ptmx
fails.

The DEVPTS_MULTIPLE_INSTANCES configuration option is removed,
so that userspace can now safely depend on each mount of devpts
creating a new instance of the filesystem.

Each mount of devpts is now a separate and equal filesystem.

Reserved ttys are now available to all instances of devpts where the
mounter is in the initial mount namespace.

A new vfs helper path_pts is introduced that finds a directory entry
named "pts" in the directory of the passed in path, and changes the
passed in path to point to it.  The helper path_pts uses a function
path_parent_directory that was factored out of follow_dotdot.

In the implementation of devpts:
- devpts_mnt is killed as it is no longer meaningful if all
  mounts of devpts are equal.
- pts_sb_from_inode is replaced by just inode->i_sb as all
  cached inodes in the tty layer are now from the devpts
  filesystem.
- devpts_add_ref is rolled into the new function devpts_ptmx.
  And the unnecessary inode hold is removed.
- devpts_del_ref is renamed devpts_release and reduced
  to just a deacrivate_super.
- The newinstance mount option continues to be accepted but is now ignored.

In devpts_fs.h definitions for when !CONFIG_UNIX98_PTYS are removed
as they are never used.

Documentation/filesystems/devices.txt is updated to describe
the current situation.

This has been verified to work properly on openwrt-15.05, centos5,
centos6, centos7, debian-6.0.2, debian-7.9, debian-8.2,
ubuntu-14.04.3, ubuntu-15.10, fedora23, magia-5, mint-17.3,
opensuse-42.1, slackware-14.1, gentoo-20151225 (13.0?),
archlinux-2015-12-01.  With the caveat that on centos6 and on
slackware-14.1 that there wind up being two instances of the devpts
filesystem mounted on /dev/pts, the lower copy does not end up getting
used.

Signed-off-by: "Eric W. Biederman" 
---
 Documentation/filesystems/devpts.txt | 145 +++---
 drivers/tty/Kconfig  |  11 --
 drivers/tty/pty.c|  15 +--
 fs/devpts/inode.c| 191 ++-
 fs/namei.c   |  49 +++--
 include/linux/devpts_fs.h|   9 +-
 include/linux/namei.h|   2 +
 7 files changed, 126 insertions(+), 296 deletions(-)

diff --git a/Documentation/filesystems/devpts.txt 
b/Documentation/filesystems/devpts.txt
index 30d2fcb32f72..9f94fe276dea 100644
--- a/Documentation/filesystems/devpts.txt
+++ b/Documentation/filesystems/devpts.txt
@@ -1,141 +1,26 @@
+Each mount of the devpts filesystem is now distinct such that ptys
+and their indicies allocated in one mount are independent from ptys
+and their indicies in all other mounts.
 
-To support containers, we now allow multiple instances of devpts filesystem,
-such that indices of ptys allocated in one instance are independent of indices
-allocated in other instances of devpts.
+All mounts of the devpts filesystem now create a /dev/pts/ptmx node
+with permissions .
 
-To preserve backward compatibility, this support for multiple instances is
-enabled only if:
+To retain backwards compatibility the a ptmx device node (aka any node
+created with "mknod name c 5 2") when opened will look for an instance
+of devpts under the name "pts" in the same directory as the ptmx device
+node.
 
-   - CONFIG_DEVPTS_MULTIPLE_INSTANCES=y, and
-   - '-o newinstance' mount option is specified while mounting devpts
-
-IOW, devpts now supports both single-instance and multi-instance semantics.
-
-If CONFIG_DEVPTS_MULTIPLE_INSTANCES=n, there is no change in behavior and
-this referred to as the "legacy" mode. In this mode, the new mount options
-(-o newinstance and -o ptmxmode) will be ignored with a 'bogus option' message
-on console.
-
-If CONFIG_DEVPTS_MULTIPLE_INSTANCES=y and devpts is mounted without the
-'newinstance' option (as in current start-up scripts) the new mount binds
-to the initial kernel mount of devpts. This mode is referred to as the
-'single-instance' mode and the current, single-instance semantics are
-preserved, i.e PTYs are common across the system.
-
-The only difference between this single-instance mode and the legacy mode
-is the presence of new, '/dev/pts/ptmx' node with permissions , which
-can safely be ignored.
-
-If CONFIG_DEVPTS_MULTIPLE_INSTANCES=y and 'newinstance' option is specified,
-the mount is considered to be in the multi-instance mode and a new instance
-of the devpts fs is created. Any ptys created in this instance are independent
-of ptys in other instances of devpts. Like in the single-instance mode, the
-/dev/pts/ptmx node is present. To effectively use the multi-instance mode,
-open of /dev/ptmx must be a redirected to '/dev/pts/ptmx' 

[PATCH tty-next] devpts: Make each mount of devpts an independent filesystem.

[Eric W. Biederman]

The /dev/ptmx device node is changed to lookup the directory entry
"pts" in the same directory as the /dev/ptmx device node was opened
in.  If there is a "pts" entry and that entry is a devpts filesystem
/dev/ptmx uses that filesystem.  Otherwise the open of /dev/ptmx
fails.

The DEVPTS_MULTIPLE_INSTANCES configuration option is removed,
so that userspace can now safely depend on each mount of devpts
creating a new instance of the filesystem.

Each mount of devpts is now a separate and equal filesystem.

Reserved ttys are now available to all instances of devpts where the
mounter is in the initial mount namespace.

A new vfs helper path_pts is introduced that finds a directory entry
named "pts" in the directory of the passed in path, and changes the
passed in path to point to it.  The helper path_pts uses a function
path_parent_directory that was factored out of follow_dotdot.

In the implementation of devpts:
- devpts_mnt is killed as it is no longer meaningful if all
  mounts of devpts are equal.
- pts_sb_from_inode is replaced by just inode->i_sb as all
  cached inodes in the tty layer are now from the devpts
  filesystem.
- devpts_add_ref is rolled into the new function devpts_ptmx.
  And the unnecessary inode hold is removed.
- devpts_del_ref is renamed devpts_release and reduced
  to just a deacrivate_super.
- The newinstance mount option continues to be accepted but is now ignored.

In devpts_fs.h definitions for when !CONFIG_UNIX98_PTYS are removed
as they are never used.

Documentation/filesystems/devices.txt is updated to describe
the current situation.

This has been verified to work properly on openwrt-15.05, centos5,
centos6, centos7, debian-6.0.2, debian-7.9, debian-8.2,
ubuntu-14.04.3, ubuntu-15.10, fedora23, magia-5, mint-17.3,
opensuse-42.1, slackware-14.1, gentoo-20151225 (13.0?),
archlinux-2015-12-01.  With the caveat that on centos6 and on
slackware-14.1 that there wind up being two instances of the devpts
filesystem mounted on /dev/pts, the lower copy does not end up getting
used.

Signed-off-by: "Eric W. Biederman" 
---
 Documentation/filesystems/devpts.txt | 145 +++---
 drivers/tty/Kconfig  |  11 --
 drivers/tty/pty.c|  15 +--
 fs/devpts/inode.c| 191 ++-
 fs/namei.c   |  49 +++--
 include/linux/devpts_fs.h|   9 +-
 include/linux/namei.h|   2 +
 7 files changed, 126 insertions(+), 296 deletions(-)

diff --git a/Documentation/filesystems/devpts.txt 
b/Documentation/filesystems/devpts.txt
index 30d2fcb32f72..9f94fe276dea 100644
--- a/Documentation/filesystems/devpts.txt
+++ b/Documentation/filesystems/devpts.txt
@@ -1,141 +1,26 @@
+Each mount of the devpts filesystem is now distinct such that ptys
+and their indicies allocated in one mount are independent from ptys
+and their indicies in all other mounts.
 
-To support containers, we now allow multiple instances of devpts filesystem,
-such that indices of ptys allocated in one instance are independent of indices
-allocated in other instances of devpts.
+All mounts of the devpts filesystem now create a /dev/pts/ptmx node
+with permissions .
 
-To preserve backward compatibility, this support for multiple instances is
-enabled only if:
+To retain backwards compatibility the a ptmx device node (aka any node
+created with "mknod name c 5 2") when opened will look for an instance
+of devpts under the name "pts" in the same directory as the ptmx device
+node.
 
-   - CONFIG_DEVPTS_MULTIPLE_INSTANCES=y, and
-   - '-o newinstance' mount option is specified while mounting devpts
-
-IOW, devpts now supports both single-instance and multi-instance semantics.
-
-If CONFIG_DEVPTS_MULTIPLE_INSTANCES=n, there is no change in behavior and
-this referred to as the "legacy" mode. In this mode, the new mount options
-(-o newinstance and -o ptmxmode) will be ignored with a 'bogus option' message
-on console.
-
-If CONFIG_DEVPTS_MULTIPLE_INSTANCES=y and devpts is mounted without the
-'newinstance' option (as in current start-up scripts) the new mount binds
-to the initial kernel mount of devpts. This mode is referred to as the
-'single-instance' mode and the current, single-instance semantics are
-preserved, i.e PTYs are common across the system.
-
-The only difference between this single-instance mode and the legacy mode
-is the presence of new, '/dev/pts/ptmx' node with permissions , which
-can safely be ignored.
-
-If CONFIG_DEVPTS_MULTIPLE_INSTANCES=y and 'newinstance' option is specified,
-the mount is considered to be in the multi-instance mode and a new instance
-of the devpts fs is created. Any ptys created in this instance are independent
-of ptys in other instances of devpts. Like in the single-instance mode, the
-/dev/pts/ptmx node is present. To effectively use the multi-instance mode,
-open of /dev/ptmx must be a redirected to '/dev/pts/ptmx' using a symlink or

Re: [PATCH] tracing: Add *iter check for NULL

[Namhyung Kim]

Hello,

On Wed, Jun 1, 2016 at 5:31 PM,   wrote:
> From: xingzhen 
>
> 3debb0a9ddb adding a "__used" to the variable in the
> __trace_printk_fmt section. Sometimes it will cause
> *iter to be NULL, then strcmp in lookup_format and
> strcpy in hold_module_trace_bprintk_format will panic.
>
> Signed-off-by: xingzhen 

Acked-by: Namhyung Kim 

Thanks,
Namhyung


> ---
>  kernel/trace/trace_printk.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
> index f96f038..82ecfb5 100644
> --- a/kernel/trace/trace_printk.c
> +++ b/kernel/trace/trace_printk.c
> @@ -55,6 +55,8 @@ void hold_module_trace_bprintk_format(const char **start, 
> const char **end)
>
> mutex_lock(_mutex);
> for (iter = start; iter < end; iter++) {
> +   if (!*iter)
> +   goto err;
> struct trace_bprintk_fmt *tb_fmt = lookup_format(*iter);
> if (tb_fmt) {
> *iter = tb_fmt->fmt;
> @@ -75,6 +77,7 @@ void hold_module_trace_bprintk_format(const char **start, 
> const char **end)
> *iter = fmt;
>
> }
> +err:
> mutex_unlock(_mutex);
>  }
>
> --
> 1.9.1
>

Re: [PATCH] tracing: Add *iter check for NULL

[Namhyung Kim]

Hello,

On Wed, Jun 1, 2016 at 5:31 PM,   wrote:
> From: xingzhen 
>
> 3debb0a9ddb adding a "__used" to the variable in the
> __trace_printk_fmt section. Sometimes it will cause
> *iter to be NULL, then strcmp in lookup_format and
> strcpy in hold_module_trace_bprintk_format will panic.
>
> Signed-off-by: xingzhen 

Acked-by: Namhyung Kim 

Thanks,
Namhyung


> ---
>  kernel/trace/trace_printk.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
> index f96f038..82ecfb5 100644
> --- a/kernel/trace/trace_printk.c
> +++ b/kernel/trace/trace_printk.c
> @@ -55,6 +55,8 @@ void hold_module_trace_bprintk_format(const char **start, 
> const char **end)
>
> mutex_lock(_mutex);
> for (iter = start; iter < end; iter++) {
> +   if (!*iter)
> +   goto err;
> struct trace_bprintk_fmt *tb_fmt = lookup_format(*iter);
> if (tb_fmt) {
> *iter = tb_fmt->fmt;
> @@ -75,6 +77,7 @@ void hold_module_trace_bprintk_format(const char **start, 
> const char **end)
> *iter = fmt;
>
> }
> +err:
> mutex_unlock(_mutex);
>  }
>
> --
> 1.9.1
>

Applied "ASoC: hdmi-codec: select CONFIG_HDMI" to the asoc tree

[Mark Brown]

The patch

   ASoC: hdmi-codec: select CONFIG_HDMI

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 6de7df8d1b1a45a07d6ecc6b4f94179e1e68f5ec Mon Sep 17 00:00:00 2001
From: Arnd Bergmann 
Date: Tue, 31 May 2016 23:12:00 +0200
Subject: [PATCH] ASoC: hdmi-codec: select CONFIG_HDMI

SND_SOC_HDMI_CODEC can be enabled without HDMI support, leading
to a link error:

In function `hdmi_codec_hw_params':
sound/soc/codecs/hdmi-codec.c:188: undefined reference to 
`hdmi_audio_infoframe_init'
sound/built-in.o:(.debug_addr+0x1a5c0): undefined reference to 
`hdmi_audio_infoframe_init'

This changes the Kconfig file to select HDMI, as the other codec using
hdmi_audio_infoframe_init already does.

Signed-off-by: Arnd Bergmann 
Acked-by: Jyri Sarha 
Signed-off-by: Mark Brown 
---
 sound/soc/codecs/Kconfig | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 4d82a58ff6b0..f3fb98f0a995 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -483,9 +483,10 @@ config SND_SOC_DMIC
tristate
 
 config SND_SOC_HDMI_CODEC
-   tristate
-   select SND_PCM_ELD
-   select SND_PCM_IEC958
+   tristate
+   select SND_PCM_ELD
+   select SND_PCM_IEC958
+   select HDMI
 
 config SND_SOC_ES8328
tristate "Everest Semi ES8328 CODEC"
-- 
2.8.1

Re: [PATCH 00/11] cpufreq: Keep policy->freq_table sorted

[Viresh Kumar]

On 2 June 2016 at 20:38, Rafael J. Wysocki  wrote:
> On Thu, Jun 2, 2016 at 4:19 PM, Viresh Kumar  wrote:
>> Hi Rafael,
>>
>> This series fixes all cpufreq drivers that provide a 'target_index'
>> callback or in other words, which provide a freq-table to cpufreq core,
>> to make sure they *only* use the 'index' argument to ->target_index()
>> with the policy->freq_table.
>>
>> This change allows us to remove the (duplicate) sorted-freq-table, which
>> was added by following series:
>>
>> [PATCH V2 0/2] cpufreq: Use sorted frequency tables
>
> Which I'm not going to apply.
>
> If this series depended on that one, please rework it.

Hi Rafael,

The first 9 patches can be applied without any dependency
on the earlier series, but not the last two.

The target of this series is to make freq-table lookup faster
and so most of its patches would make sense only if we
are trying to solve that problem. Otherwise they may not
be required at all.

I hope that you will be replying to the earlier series [1] as well,
to convey the concerns you have with it.

Thanks

--
viresh

[1] [PATCH V2 0/2] cpufreq: Use sorted frequency tables

Applied "ASoC: hdmi-codec: select CONFIG_HDMI" to the asoc tree

[Mark Brown]

The patch

   ASoC: hdmi-codec: select CONFIG_HDMI

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 6de7df8d1b1a45a07d6ecc6b4f94179e1e68f5ec Mon Sep 17 00:00:00 2001
From: Arnd Bergmann 
Date: Tue, 31 May 2016 23:12:00 +0200
Subject: [PATCH] ASoC: hdmi-codec: select CONFIG_HDMI

SND_SOC_HDMI_CODEC can be enabled without HDMI support, leading
to a link error:

In function `hdmi_codec_hw_params':
sound/soc/codecs/hdmi-codec.c:188: undefined reference to 
`hdmi_audio_infoframe_init'
sound/built-in.o:(.debug_addr+0x1a5c0): undefined reference to 
`hdmi_audio_infoframe_init'

This changes the Kconfig file to select HDMI, as the other codec using
hdmi_audio_infoframe_init already does.

Signed-off-by: Arnd Bergmann 
Acked-by: Jyri Sarha 
Signed-off-by: Mark Brown 
---
 sound/soc/codecs/Kconfig | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 4d82a58ff6b0..f3fb98f0a995 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -483,9 +483,10 @@ config SND_SOC_DMIC
tristate
 
 config SND_SOC_HDMI_CODEC
-   tristate
-   select SND_PCM_ELD
-   select SND_PCM_IEC958
+   tristate
+   select SND_PCM_ELD
+   select SND_PCM_IEC958
+   select HDMI
 
 config SND_SOC_ES8328
tristate "Everest Semi ES8328 CODEC"
-- 
2.8.1

Re: [PATCH 00/11] cpufreq: Keep policy->freq_table sorted

[Viresh Kumar]

On 2 June 2016 at 20:38, Rafael J. Wysocki  wrote:
> On Thu, Jun 2, 2016 at 4:19 PM, Viresh Kumar  wrote:
>> Hi Rafael,
>>
>> This series fixes all cpufreq drivers that provide a 'target_index'
>> callback or in other words, which provide a freq-table to cpufreq core,
>> to make sure they *only* use the 'index' argument to ->target_index()
>> with the policy->freq_table.
>>
>> This change allows us to remove the (duplicate) sorted-freq-table, which
>> was added by following series:
>>
>> [PATCH V2 0/2] cpufreq: Use sorted frequency tables
>
> Which I'm not going to apply.
>
> If this series depended on that one, please rework it.

Hi Rafael,

The first 9 patches can be applied without any dependency
on the earlier series, but not the last two.

The target of this series is to make freq-table lookup faster
and so most of its patches would make sense only if we
are trying to solve that problem. Otherwise they may not
be required at all.

I hope that you will be replying to the earlier series [1] as well,
to convey the concerns you have with it.

Thanks

--
viresh

[1] [PATCH V2 0/2] cpufreq: Use sorted frequency tables

Re: [RFC PATCH v1 3/3] regulator: qcom_smd: add linear range to pm8941 lnldo

[Mark Brown]

On Thu, Jun 02, 2016 at 03:57:42PM +0100, Srinivas Kandagatla wrote:
> On 02/06/16 15:49, Mark Brown wrote:

> > Why is this better than using a separate set of ops for the driver?

> Am ok either way, it would be just few more lines for separate set of ops.

It's more natural to use a separate set of ops, and we can optimise a
few things if we know the regulator is a fixed voltage one.


signature.asc
Description: PGP signature

Re: [RFC PATCH v1 3/3] regulator: qcom_smd: add linear range to pm8941 lnldo

[Mark Brown]

On Thu, Jun 02, 2016 at 03:57:42PM +0100, Srinivas Kandagatla wrote:
> On 02/06/16 15:49, Mark Brown wrote:

> > Why is this better than using a separate set of ops for the driver?

> Am ok either way, it would be just few more lines for separate set of ops.

It's more natural to use a separate set of ops, and we can optimise a
few things if we know the regulator is a fixed voltage one.


signature.asc
Description: PGP signature

Re: [PATCH] ASoC: sgtl5000: only check VDDD-supply, not revision

[Fabio Estevam]

Hi Clemens,

On Thu, Jun 2, 2016 at 9:47 AM, Clemens Gruber
 wrote:
> Instead of checking the SGTL5000 chip revision, we should only check if
> the VDDD regulator exists and only call sgtl5000_replace_vddd_with_ldo
> if the regulator is missing.
> Otherwise, the user reads in the kernel log that the internal LDO is
> used, even though he did follow the NXP recommendation to use external
> VDDD and also specified VDDD-supply in the devicetree.
>
> Also remove the comment, which incorrectly states that external VDDD is
> only supported for SGTL5000 chip revisions < 0x11.
> Official NXP documentation recommends using external VDDD and not the
> internal LDO due to the SGTL5000 erratum ER1. This also applies to
> revisions >= 0x11.
>
> Tested on an i.MX6Q board with SGTL5000 rev 0x11 and external VDDD.

Patch looks good to me.

Eric,

Sometime ago you were looking at this. What do you think about this patch?

Thanks

[PATCH -next 1/2] virtio: Start feature MTU support

[Aaron Conole]

This commit adds the feature bit and associated mtu device entry for the
virtio network device. Future commits will make use of these bits to
support negotiated MTU.

Signed-off-by: Aaron Conole 
---
 include/uapi/linux/virtio_net.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index ec32293..751ff59 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -73,6 +73,8 @@ struct virtio_net_config {
 * Legal values are between 1 and 0x8000
 */
__u16 max_virtqueue_pairs;
+   /* Default maximum transmit unit advice */
+   __u16 mtu;
 } __attribute__((packed));
 
 /*
-- 
2.5.5

[PATCH -next 1/2] virtio: Start feature MTU support

[Aaron Conole]

This commit adds the feature bit and associated mtu device entry for the
virtio network device. Future commits will make use of these bits to
support negotiated MTU.

Signed-off-by: Aaron Conole 
---
 include/uapi/linux/virtio_net.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index ec32293..751ff59 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -73,6 +73,8 @@ struct virtio_net_config {
 * Legal values are between 1 and 0x8000
 */
__u16 max_virtqueue_pairs;
+   /* Default maximum transmit unit advice */
+   __u16 mtu;
 } __attribute__((packed));
 
 /*
-- 
2.5.5

Re: [PATCH] ASoC: sgtl5000: only check VDDD-supply, not revision

[Fabio Estevam]

Hi Clemens,

On Thu, Jun 2, 2016 at 9:47 AM, Clemens Gruber
 wrote:
> Instead of checking the SGTL5000 chip revision, we should only check if
> the VDDD regulator exists and only call sgtl5000_replace_vddd_with_ldo
> if the regulator is missing.
> Otherwise, the user reads in the kernel log that the internal LDO is
> used, even though he did follow the NXP recommendation to use external
> VDDD and also specified VDDD-supply in the devicetree.
>
> Also remove the comment, which incorrectly states that external VDDD is
> only supported for SGTL5000 chip revisions < 0x11.
> Official NXP documentation recommends using external VDDD and not the
> internal LDO due to the SGTL5000 erratum ER1. This also applies to
> revisions >= 0x11.
>
> Tested on an i.MX6Q board with SGTL5000 rev 0x11 and external VDDD.

Patch looks good to me.

Eric,

Sometime ago you were looking at this. What do you think about this patch?

Thanks

[PATCH -next 0/2] virtio-net: Advised MTU feature

[Aaron Conole]

The following series adds the ability for a hypervisor to set an MTU on the
guest during feature negotiation phase. This is useful for VM orchestration
when, for instance, tunneling is involved and the MTU of the various systems
should be homogenous.

The first patch adds the feature bit as described in the proposed VFIO spec
addition found at
https://lists.oasis-open.org/archives/virtio-dev/201603/msg1.html

The second patch adds a user of the bit, and a warning when the guest changes
the MTU from the hypervisor advised MTU. Future patches may add more thorough
error handling.

Aaron Conole (2):
  virtio: Start feature MTU support
  virtio_net: Read the advised MTU

 drivers/net/virtio_net.c| 7 +++
 include/uapi/linux/virtio_net.h | 2 ++
 2 files changed, 9 insertions(+)

-- 
2.5.5

[PATCH -next 0/2] virtio-net: Advised MTU feature

[Aaron Conole]

The following series adds the ability for a hypervisor to set an MTU on the
guest during feature negotiation phase. This is useful for VM orchestration
when, for instance, tunneling is involved and the MTU of the various systems
should be homogenous.

The first patch adds the feature bit as described in the proposed VFIO spec
addition found at
https://lists.oasis-open.org/archives/virtio-dev/201603/msg1.html

The second patch adds a user of the bit, and a warning when the guest changes
the MTU from the hypervisor advised MTU. Future patches may add more thorough
error handling.

Aaron Conole (2):
  virtio: Start feature MTU support
  virtio_net: Read the advised MTU

 drivers/net/virtio_net.c| 7 +++
 include/uapi/linux/virtio_net.h | 2 ++
 2 files changed, 9 insertions(+)

-- 
2.5.5

Re: [RFC PATCH 0/3] Aggregate task utilization only on root cfs_rq

[Dietmar Eggemann]

On 01/06/16 21:10, Peter Zijlstra wrote:
> On Wed, Jun 01, 2016 at 08:39:19PM +0100, Dietmar Eggemann wrote:
>> This is an alternative approach to '[RFC PATCH v2] sched: reflect
>> sched_entity movement into task_group's utilization' which requires
>> '[RFC PATCH] sched: fix hierarchical order in rq->leaf_cfs_rq_list'
> 
> Awesome, a 3rd alternative :-)

Yeah, sorry, but at least it runs op tip/sched/core w/o any other PELT
related changes.

Moreover, it's less code changes compared to the reflect-thing.

And Yuyang's implementation depended on the optimization of
_update_load_avg() (1us->1ms) which IMHO turned out to be not working.

Re: [RFC PATCH 0/3] Aggregate task utilization only on root cfs_rq

[Dietmar Eggemann]

On 01/06/16 21:10, Peter Zijlstra wrote:
> On Wed, Jun 01, 2016 at 08:39:19PM +0100, Dietmar Eggemann wrote:
>> This is an alternative approach to '[RFC PATCH v2] sched: reflect
>> sched_entity movement into task_group's utilization' which requires
>> '[RFC PATCH] sched: fix hierarchical order in rq->leaf_cfs_rq_list'
> 
> Awesome, a 3rd alternative :-)

Yeah, sorry, but at least it runs op tip/sched/core w/o any other PELT
related changes.

Moreover, it's less code changes compared to the reflect-thing.

And Yuyang's implementation depended on the optimization of
_update_load_avg() (1us->1ms) which IMHO turned out to be not working.

RE: [PATCH] r8152: Add support for setting MAC to system's Auxiliary MAC address

[Mario_Limonciello]

> >
> > This isn't something part of ACPI - it's been added specifically for a
> > selection of Dell machines.
> 
> Ah, but isn't ACPI supposed to be a "standard"?  :)
> 

Heh.
It's also possible to get this from an SMM routine.  Lesser of two evils to 
fetch the information this way, right? :)

> And please wrap your email lines, there is a "standard" for that...

I'm unfortunately not limited to an evil mail client at my workplace since our 
mail server migration.   My apologies, I've got it set to wrap at 76 characters 
and I'm trying to make it as LKML friendly as possible.

> 
> > I would rather not hardcode to the specific DMI model strings of those
> > Dell machines as it's certainly going to be a feature that expands to
> > more machines.  Since it is Dell specific though, if you would rather
> > me just match to the sys vendor Dell Inc., that seems like a pretty
> > good compromise to me.
> 
> You need to only do this on machines you "know" have this set to a correct
> value, otherwise if some other random BIOS happens to set that field to
> some random value, you will have problems.

Pali had recommended in another message to check the buffer header.  I was 
intending to do this along with check ACPI buffer output type, and output size 
in the next revision I submitted.  By switching to hex2bin, I'll also validate 
that the string has correct values (0-F or 0-f).  If somehow all of that fails, 
the set_ethernet_addr  checks if the address is valid.  If it's invalid it will 
generate a random one.

> 
> > > And finally, this seems odd overall given that a MAC address should
> > > be associated with the specific network device, not the overall system.
> >
> > The whole reasoning behind this is to bring the behavior that E-Docks
> > had to TBT docks.
> 
> What is "TBT"?

Thunderbolt

> 
> > With E-docks they were really just extensions of the pins for the
> > already onboard NIC of the system.  When you docked in an E-dock you
> > inherently would use the same MAC everywhere you went.  If you had two
> > cubes your network admin would see your same MAC in both.
> >
> > With TBT docks and this patch not present docking in different places
> > will give you the MAC of the dock rather than something persistent
> > that your network admin could identify.  Solving this was something
> > that was explicitly asked for in case studies during conceptualization
> > of these docks and is a feature present in the Realtek Windows driver.
> 
> But you are dealing with different "devices" here, thunderbolt is a PCI
> device, not a "pin pass-through", and to treat it differently like you want 
> to is
> going to cause confusion as well.

Is it not also going to be confusing if someone boots Windows and Linux on the 
same laptop and has a different behavior with their dock's MAC address?  I'm 
trying to get parity here for organizations that are supporting both Windows 
and Linux for their employees.

> > In addition to limiting to Dell DMI system vendor string how about I do two
> more things about this:
> >
> > 1) Add a module parameter to disable this behavior altogether in r8152 if
> it's not desired by the user or admin (but leave it on by default).
> 
> No module parameters, this isn't the 1990's anymore, and you aren't going to
> be modifying module config files for everyone, are you?
> 
> And this seems like a "default" that should be turned off anyway, as it goes
> against the model of all of our other network devices in Linux.
> 
> > 2) Track whether this is the first or second USB NIC plugged in.  Only 
> > offer it
> on the first NIC detected by r8152.  When the second NIC is plugged in don't
> match from ACPI.
> > There would be a question of what to do if the first NIC is removed and
> added back if it should get the persistent system MAC or not.
> > I'd say yes, just make sure that only one NIC can have it at a time.
> 
> You are going to get things very complex very quickly if you try to do this.

It's really not that hard, track a module wide static variable whether the 
feature is in use.  Track in each device whether the feature was in use.  If it 
in use, don't assign the next device plugged in via the ACPI string.  If a 
device is removed that has the feature activated, change the module wide static 
variable.

> 
> What's wrong with a "simple" script to set the mac address from userspace if
> the user wants something like this?  Provide it as a system package and then
> no kernel changes are needed at all.  Much easier to support on your end
> (you don't have to maintain this odd kernel code for
> 10+ years), the default behavior is as Linux users expect, and your
> limited number of people who want this crazy behaviour can install your
> script if they want it.
> 

This was my original approach.  It involved a network manager script, network 
manager code changes to support this, and exposing this somewhere in a platform 
module (like dell-laptop).  I was told I'm better off doing it directly 

Re: [PATCH 1/3] pci: introduce read_bridge/write_bridge pci ops

[Arnd Bergmann]

On Thursday, June 2, 2016 9:00:01 AM CEST Bjorn Helgaas wrote:
> On Wed, Jun 01, 2016 at 10:37:28PM +0200, Arnd Bergmann wrote:
> > On Wednesday, June 1, 2016 2:04:30 PM CEST Bjorn Helgaas wrote:
> > > On Wed, Jun 01, 2016 at 05:41:53PM +0200, Arnd Bergmann wrote:
> > > > On Wednesday, June 1, 2016 10:09:29 AM CEST Bjorn Helgaas wrote:
> > > > > Hi Arnd,
> > > > > 
> > > > > On Wed, Jun 01, 2016 at 02:31:22PM +0200, Arnd Bergmann wrote:
> > > > > > A lot of PCI host bridges require different methods for initiating
> > > > > > type 0 and type 1 config space accesses, leading to duplication of
> > > > > > code.
> > > > > > 
> > > > > > This adds support for the two different kinds at the pci_ops
> > > > > > level, with the newly added map_bridge/read_bridge/write_bridge
> > > > > > operations for type 1 accesses.
> > > > > > 
> > > > > > When these are not set, we fall back to the regular 
> > > > > > map_bus/read/write
> > > > > > operations, so all existing drivers keep working, and bridges that
> > > > > > have identical operations continue to only require one set.
> > > > > 
> > > > > This adds new config accessor functions to struct pci_ops and makes
> > > > > the callers responsible for figuring out which one to use.  The
> > > > > benefit is to reduce code duplication in some host bridge drivers
> > > > > (DesignWare and MVEBU so far).
> > > > > 
> > > > > From a design perspective, I'm not comfortable with moving this burden
> > > > > from the host bridge drivers to the callers of the config accessors.
> > > > ...
> > > 
> > > > Maybe we can simply change them to use the normal API and come up with
> > > > a way to make the pci_ops harder to misuse? Would it make you feel 
> > > > better
> > > > if we also renamed .read/.write into .read_type0/.write_type0 or 
> > > > something
> > > > like that?
> > > 
> > > I'm trying to get a better feel for the tradeoff here.  It seems like
> > > an API complication vs. code duplication.
> > > 
> > > I don't really think the callers should have to figure out which
> > > accessor to use.  How much of a benefit do we really gain by
> > > complicating the callers?  We've managed for quite a few years with
> > > the current scheme, and it seems like only a couple new ARM platforms
> > > would benefit.
> > 
> > I just did a count of the implementations of pci_ops: I found 107
> > instances of 'struct pci_ops', and 67 of them treat type0 and type1
> > access differently in some form.
> > 
> > I'd estimate that about half of them, or roughly a third of the total
> > instances would benefit from my change, if we were to do them again.
> > Clearly there is no need to change the existing code here when it works,
> > unless the benefit is very clear and the code is actively maintained.
> > 
> > In some cases, the difference is only that the root bus has a limited
> > set of devices that are allowed to be accessed, so there would
> > likely be no benefit of this, compared to e.g. yet another callback
> > that checks the validity.
> > Some other instances have type0 registers at a different memory location
> > from type1, some use different layout inside of that space, and some
> > are completely different.
> 
> The type0/type1 distinction still seems out of place to me at the call
> site.  Is there any other reason a caller would care about the
> difference between type0 and type1?

Another idea based on my RFC patches to make pci_host_bridge the primary
structure for probing PCI: we could split out the old 'bus::pci_ops' with
the traditional read/write interface from a new structure that becomes
pci_host_bridge::pci_host_bridge_ops, and also contains the other callbacks
that we recently added to pci_ops, alongside type0/type1 accessors.

We could then have a set of default pci_ops that call
pci_host_bridge_ops->type0_read/type0_write/type1_read/type1_write,
and those in turn get a pci_host_bridge as an argument along with the
bus, device, function and register numbers instead of bus pointer
and devfn/where.

This way all existing code can keep working, but we can convert host
drivers (if desired) to provide only pci_host_bridge_ops and no
pci_ops, while making it easier to define those with a more modern
interface.

Arnd

RE: [PATCH] r8152: Add support for setting MAC to system's Auxiliary MAC address

[Mario_Limonciello]

> >
> > This isn't something part of ACPI - it's been added specifically for a
> > selection of Dell machines.
> 
> Ah, but isn't ACPI supposed to be a "standard"?  :)
> 

Heh.
It's also possible to get this from an SMM routine.  Lesser of two evils to 
fetch the information this way, right? :)

> And please wrap your email lines, there is a "standard" for that...

I'm unfortunately not limited to an evil mail client at my workplace since our 
mail server migration.   My apologies, I've got it set to wrap at 76 characters 
and I'm trying to make it as LKML friendly as possible.

> 
> > I would rather not hardcode to the specific DMI model strings of those
> > Dell machines as it's certainly going to be a feature that expands to
> > more machines.  Since it is Dell specific though, if you would rather
> > me just match to the sys vendor Dell Inc., that seems like a pretty
> > good compromise to me.
> 
> You need to only do this on machines you "know" have this set to a correct
> value, otherwise if some other random BIOS happens to set that field to
> some random value, you will have problems.

Pali had recommended in another message to check the buffer header.  I was 
intending to do this along with check ACPI buffer output type, and output size 
in the next revision I submitted.  By switching to hex2bin, I'll also validate 
that the string has correct values (0-F or 0-f).  If somehow all of that fails, 
the set_ethernet_addr  checks if the address is valid.  If it's invalid it will 
generate a random one.

> 
> > > And finally, this seems odd overall given that a MAC address should
> > > be associated with the specific network device, not the overall system.
> >
> > The whole reasoning behind this is to bring the behavior that E-Docks
> > had to TBT docks.
> 
> What is "TBT"?

Thunderbolt

> 
> > With E-docks they were really just extensions of the pins for the
> > already onboard NIC of the system.  When you docked in an E-dock you
> > inherently would use the same MAC everywhere you went.  If you had two
> > cubes your network admin would see your same MAC in both.
> >
> > With TBT docks and this patch not present docking in different places
> > will give you the MAC of the dock rather than something persistent
> > that your network admin could identify.  Solving this was something
> > that was explicitly asked for in case studies during conceptualization
> > of these docks and is a feature present in the Realtek Windows driver.
> 
> But you are dealing with different "devices" here, thunderbolt is a PCI
> device, not a "pin pass-through", and to treat it differently like you want 
> to is
> going to cause confusion as well.

Is it not also going to be confusing if someone boots Windows and Linux on the 
same laptop and has a different behavior with their dock's MAC address?  I'm 
trying to get parity here for organizations that are supporting both Windows 
and Linux for their employees.

> > In addition to limiting to Dell DMI system vendor string how about I do two
> more things about this:
> >
> > 1) Add a module parameter to disable this behavior altogether in r8152 if
> it's not desired by the user or admin (but leave it on by default).
> 
> No module parameters, this isn't the 1990's anymore, and you aren't going to
> be modifying module config files for everyone, are you?
> 
> And this seems like a "default" that should be turned off anyway, as it goes
> against the model of all of our other network devices in Linux.
> 
> > 2) Track whether this is the first or second USB NIC plugged in.  Only 
> > offer it
> on the first NIC detected by r8152.  When the second NIC is plugged in don't
> match from ACPI.
> > There would be a question of what to do if the first NIC is removed and
> added back if it should get the persistent system MAC or not.
> > I'd say yes, just make sure that only one NIC can have it at a time.
> 
> You are going to get things very complex very quickly if you try to do this.

It's really not that hard, track a module wide static variable whether the 
feature is in use.  Track in each device whether the feature was in use.  If it 
in use, don't assign the next device plugged in via the ACPI string.  If a 
device is removed that has the feature activated, change the module wide static 
variable.

> 
> What's wrong with a "simple" script to set the mac address from userspace if
> the user wants something like this?  Provide it as a system package and then
> no kernel changes are needed at all.  Much easier to support on your end
> (you don't have to maintain this odd kernel code for
> 10+ years), the default behavior is as Linux users expect, and your
> limited number of people who want this crazy behaviour can install your
> script if they want it.
> 

This was my original approach.  It involved a network manager script, network 
manager code changes to support this, and exposing this somewhere in a platform 
module (like dell-laptop).  I was told I'm better off doing it directly 

Re: [PATCH 1/3] pci: introduce read_bridge/write_bridge pci ops

[Arnd Bergmann]

On Thursday, June 2, 2016 9:00:01 AM CEST Bjorn Helgaas wrote:
> On Wed, Jun 01, 2016 at 10:37:28PM +0200, Arnd Bergmann wrote:
> > On Wednesday, June 1, 2016 2:04:30 PM CEST Bjorn Helgaas wrote:
> > > On Wed, Jun 01, 2016 at 05:41:53PM +0200, Arnd Bergmann wrote:
> > > > On Wednesday, June 1, 2016 10:09:29 AM CEST Bjorn Helgaas wrote:
> > > > > Hi Arnd,
> > > > > 
> > > > > On Wed, Jun 01, 2016 at 02:31:22PM +0200, Arnd Bergmann wrote:
> > > > > > A lot of PCI host bridges require different methods for initiating
> > > > > > type 0 and type 1 config space accesses, leading to duplication of
> > > > > > code.
> > > > > > 
> > > > > > This adds support for the two different kinds at the pci_ops
> > > > > > level, with the newly added map_bridge/read_bridge/write_bridge
> > > > > > operations for type 1 accesses.
> > > > > > 
> > > > > > When these are not set, we fall back to the regular 
> > > > > > map_bus/read/write
> > > > > > operations, so all existing drivers keep working, and bridges that
> > > > > > have identical operations continue to only require one set.
> > > > > 
> > > > > This adds new config accessor functions to struct pci_ops and makes
> > > > > the callers responsible for figuring out which one to use.  The
> > > > > benefit is to reduce code duplication in some host bridge drivers
> > > > > (DesignWare and MVEBU so far).
> > > > > 
> > > > > From a design perspective, I'm not comfortable with moving this burden
> > > > > from the host bridge drivers to the callers of the config accessors.
> > > > ...
> > > 
> > > > Maybe we can simply change them to use the normal API and come up with
> > > > a way to make the pci_ops harder to misuse? Would it make you feel 
> > > > better
> > > > if we also renamed .read/.write into .read_type0/.write_type0 or 
> > > > something
> > > > like that?
> > > 
> > > I'm trying to get a better feel for the tradeoff here.  It seems like
> > > an API complication vs. code duplication.
> > > 
> > > I don't really think the callers should have to figure out which
> > > accessor to use.  How much of a benefit do we really gain by
> > > complicating the callers?  We've managed for quite a few years with
> > > the current scheme, and it seems like only a couple new ARM platforms
> > > would benefit.
> > 
> > I just did a count of the implementations of pci_ops: I found 107
> > instances of 'struct pci_ops', and 67 of them treat type0 and type1
> > access differently in some form.
> > 
> > I'd estimate that about half of them, or roughly a third of the total
> > instances would benefit from my change, if we were to do them again.
> > Clearly there is no need to change the existing code here when it works,
> > unless the benefit is very clear and the code is actively maintained.
> > 
> > In some cases, the difference is only that the root bus has a limited
> > set of devices that are allowed to be accessed, so there would
> > likely be no benefit of this, compared to e.g. yet another callback
> > that checks the validity.
> > Some other instances have type0 registers at a different memory location
> > from type1, some use different layout inside of that space, and some
> > are completely different.
> 
> The type0/type1 distinction still seems out of place to me at the call
> site.  Is there any other reason a caller would care about the
> difference between type0 and type1?

Another idea based on my RFC patches to make pci_host_bridge the primary
structure for probing PCI: we could split out the old 'bus::pci_ops' with
the traditional read/write interface from a new structure that becomes
pci_host_bridge::pci_host_bridge_ops, and also contains the other callbacks
that we recently added to pci_ops, alongside type0/type1 accessors.

We could then have a set of default pci_ops that call
pci_host_bridge_ops->type0_read/type0_write/type1_read/type1_write,
and those in turn get a pci_host_bridge as an argument along with the
bus, device, function and register numbers instead of bus pointer
and devfn/where.

This way all existing code can keep working, but we can convert host
drivers (if desired) to provide only pci_host_bridge_ops and no
pci_ops, while making it easier to define those with a more modern
interface.

Arnd

[PATCH -next 2/2] virtio_net: Read the advised MTU

[Aaron Conole]

This patch checks the feature bit for the VIRTIO_NET_F_MTU feature. If it
exists, read the advised MTU and use it.

No proper error handling is provided for the case where a user changes the
negotiated MTU. A future commit will add proper error handling. Instead, a
warning is emitted if the guest changes the device MTU after previously
being given advice.

Signed-off-by: Aaron Conole 
---
 drivers/net/virtio_net.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index e0638e5..ef5ee01 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1896,6 +1896,12 @@ static int virtnet_probe(struct virtio_device *vdev)
if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
vi->has_cvq = true;
 
+   if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
+   dev->mtu = virtio_cread16(vdev,
+ offsetof(struct virtio_net_config,
+  mtu));
+   }
+
if (vi->any_header_sg)
dev->needed_headroom = vi->hdr_len;
 
@@ -2067,6 +2073,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
VIRTIO_NET_F_CTRL_MAC_ADDR,
VIRTIO_F_ANY_LAYOUT,
+   VIRTIO_NET_F_MTU,
 };
 
 static struct virtio_driver virtio_net_driver = {
-- 
2.5.5

[PATCH -next 2/2] virtio_net: Read the advised MTU

[Aaron Conole]

This patch checks the feature bit for the VIRTIO_NET_F_MTU feature. If it
exists, read the advised MTU and use it.

No proper error handling is provided for the case where a user changes the
negotiated MTU. A future commit will add proper error handling. Instead, a
warning is emitted if the guest changes the device MTU after previously
being given advice.

Signed-off-by: Aaron Conole 
---
 drivers/net/virtio_net.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index e0638e5..ef5ee01 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1896,6 +1896,12 @@ static int virtnet_probe(struct virtio_device *vdev)
if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
vi->has_cvq = true;
 
+   if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
+   dev->mtu = virtio_cread16(vdev,
+ offsetof(struct virtio_net_config,
+  mtu));
+   }
+
if (vi->any_header_sg)
dev->needed_headroom = vi->hdr_len;
 
@@ -2067,6 +2073,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
VIRTIO_NET_F_CTRL_MAC_ADDR,
VIRTIO_F_ANY_LAYOUT,
+   VIRTIO_NET_F_MTU,
 };
 
 static struct virtio_driver virtio_net_driver = {
-- 
2.5.5

Re: [PATCH 1/3] Convert efi_call_virt to efi_call_virt_generic

[Matt Fleming]

On Wed, 18 May, at 02:11:39PM, Alex Thorlton wrote:
> This commit makes a few slight modifications to the efi_call_virt macro
> to get it to work with function pointers that are stored in locations
> other than efi.systab->runtime, and renames the macro to
> efi_call_virt_generic.  The majority of the changes here are to pull
> these macros up into header files so that they can be accessed from
> outside of drivers/firmware/efi/runtime-wrappers.c.
> 
> The most significant change not directly related to the code move is to
> add an extra "p" argument into the appropriate efi_call macros, and use
> that new argument in place of the, formerly hard-coded,
> efi.systab->runtime pointer.
> 
> The last piece of the puzzle was to add an efi_call_virt macro back into
> drivers/firmware/efi/runtime-wrappers.c to wrap around the new
> efi_call_virt_generic macro - this was mainly to keep the code from
> looking too cluttered by adding a bunch of extra references to
> efi.systab->runtime everywhere.
> 
> Note that I also broke up the code in the efi_call_virt_generic macro a
> bit in the process of moving it.
> 
> Signed-off-by: Alex Thorlton 
> Cc: Matt Fleming 
> Cc: Borislav Petkov 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: "H. Peter Anvin" 
> Cc: Mike Travis 
> Cc: Russ Anderson 
> Cc: Dimitri Sivanich 
> Cc: x...@kernel.org
> Cc: linux-...@vger.kernel.org
> ---
>  arch/x86/include/asm/efi.h  |  4 +--
>  drivers/firmware/efi/runtime-wrappers.c | 53 
> +++--
>  include/linux/efi.h | 51 +++
>  3 files changed, 63 insertions(+), 45 deletions(-)
> 
> diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> index 78d1e74..f310f0b 100644
> --- a/arch/x86/include/asm/efi.h
> +++ b/arch/x86/include/asm/efi.h
> @@ -81,8 +81,8 @@ struct efi_scratch {
>   }   \
>  })
>  
> -#define arch_efi_call_virt(f, args...)   
> \
> - efi_call((void *)efi.systab->runtime->f, args)  \
> +#define arch_efi_call_virt(p, f, args...)
> \
> + efi_call((void *)p->f, args)\
>  
>  #define arch_efi_call_virt_teardown()
> \
>  ({   \

Oops, you're missing updates to the 32-bit version and ARM/arm64,
which results in this,

drivers/firmware/efi/runtime-wrappers.c: In function ‘virt_efi_get_time’:
arch/x86/include/asm/efi.h:46:4: error: ‘efi_efi’ undeclared (first use in this 
function)
  ((efi_##f##_t __attribute__((regparm(0)))*)   \
^

but that's easily fixed up.

> diff --git a/drivers/firmware/efi/runtime-wrappers.c 
> b/drivers/firmware/efi/runtime-wrappers.c
> index 23bef6b..e8bc493 100644
> --- a/drivers/firmware/efi/runtime-wrappers.c
> +++ b/drivers/firmware/efi/runtime-wrappers.c
> @@ -22,7 +22,16 @@
>  #include 
>  #include 
>  
> -static void efi_call_virt_check_flags(unsigned long flags, const char *call)
> +/*
> + * Wrap around the new efi_call_virt_generic macros so that the
> + * code doesn't get too cluttered
> + */
> +#define efi_call_virt(f, args...)   \
> + efi_call_virt_generic(efi.systab->runtime, f, args)
> +#define __efi_call_virt(f, args...) \
> + __efi_call_virt_generic(efi.systab->runtime, f, args)
> +
> +void efi_call_virt_check_flags(unsigned long flags, const char *call)
>  {
>   unsigned long cur_flags, mismatch;
>  

I'm not crazy about using "generic" in the name. How about
efi_call_virt_function() or efi_call_virt_pointer()?

Re: [PATCH 1/3] Convert efi_call_virt to efi_call_virt_generic

[Matt Fleming]

On Wed, 18 May, at 02:11:39PM, Alex Thorlton wrote:
> This commit makes a few slight modifications to the efi_call_virt macro
> to get it to work with function pointers that are stored in locations
> other than efi.systab->runtime, and renames the macro to
> efi_call_virt_generic.  The majority of the changes here are to pull
> these macros up into header files so that they can be accessed from
> outside of drivers/firmware/efi/runtime-wrappers.c.
> 
> The most significant change not directly related to the code move is to
> add an extra "p" argument into the appropriate efi_call macros, and use
> that new argument in place of the, formerly hard-coded,
> efi.systab->runtime pointer.
> 
> The last piece of the puzzle was to add an efi_call_virt macro back into
> drivers/firmware/efi/runtime-wrappers.c to wrap around the new
> efi_call_virt_generic macro - this was mainly to keep the code from
> looking too cluttered by adding a bunch of extra references to
> efi.systab->runtime everywhere.
> 
> Note that I also broke up the code in the efi_call_virt_generic macro a
> bit in the process of moving it.
> 
> Signed-off-by: Alex Thorlton 
> Cc: Matt Fleming 
> Cc: Borislav Petkov 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: "H. Peter Anvin" 
> Cc: Mike Travis 
> Cc: Russ Anderson 
> Cc: Dimitri Sivanich 
> Cc: x...@kernel.org
> Cc: linux-...@vger.kernel.org
> ---
>  arch/x86/include/asm/efi.h  |  4 +--
>  drivers/firmware/efi/runtime-wrappers.c | 53 
> +++--
>  include/linux/efi.h | 51 +++
>  3 files changed, 63 insertions(+), 45 deletions(-)
> 
> diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
> index 78d1e74..f310f0b 100644
> --- a/arch/x86/include/asm/efi.h
> +++ b/arch/x86/include/asm/efi.h
> @@ -81,8 +81,8 @@ struct efi_scratch {
>   }   \
>  })
>  
> -#define arch_efi_call_virt(f, args...)   
> \
> - efi_call((void *)efi.systab->runtime->f, args)  \
> +#define arch_efi_call_virt(p, f, args...)
> \
> + efi_call((void *)p->f, args)\
>  
>  #define arch_efi_call_virt_teardown()
> \
>  ({   \

Oops, you're missing updates to the 32-bit version and ARM/arm64,
which results in this,

drivers/firmware/efi/runtime-wrappers.c: In function ‘virt_efi_get_time’:
arch/x86/include/asm/efi.h:46:4: error: ‘efi_efi’ undeclared (first use in this 
function)
  ((efi_##f##_t __attribute__((regparm(0)))*)   \
^

but that's easily fixed up.

> diff --git a/drivers/firmware/efi/runtime-wrappers.c 
> b/drivers/firmware/efi/runtime-wrappers.c
> index 23bef6b..e8bc493 100644
> --- a/drivers/firmware/efi/runtime-wrappers.c
> +++ b/drivers/firmware/efi/runtime-wrappers.c
> @@ -22,7 +22,16 @@
>  #include 
>  #include 
>  
> -static void efi_call_virt_check_flags(unsigned long flags, const char *call)
> +/*
> + * Wrap around the new efi_call_virt_generic macros so that the
> + * code doesn't get too cluttered
> + */
> +#define efi_call_virt(f, args...)   \
> + efi_call_virt_generic(efi.systab->runtime, f, args)
> +#define __efi_call_virt(f, args...) \
> + __efi_call_virt_generic(efi.systab->runtime, f, args)
> +
> +void efi_call_virt_check_flags(unsigned long flags, const char *call)
>  {
>   unsigned long cur_flags, mismatch;
>  

I'm not crazy about using "generic" in the name. How about
efi_call_virt_function() or efi_call_virt_pointer()?

Re: [PATCH] tracing: expose current->comm to kprobe events

[Namhyung Kim]

Hello,

On Wed, Jun 1, 2016 at 3:17 PM, Omar Sandoval  wrote:
> From: Omar Sandoval 
>
> ftrace is very quick to give up on saving the task command line (see
> `trace_save_cmdline()`). The workaround for events which really care
> about the command line is to explicitly assign it as part of the entry.
> However, this doesn't work for kprobe events, as there's no
> straightforward way to get access to current->comm. Add a kprobe event
> variable $comm which provides exactly that.
>
> Signed-off-by: Omar Sandoval 
> ---
> This is a problem I ran into while trying to debug an unrelated issue. I
> was considering trying to make ftrace try harder to save the command
> line, but that would most likely be at the expense at most events which
> don't care about the task comm. This is much less work.
>
>  Documentation/trace/kprobetrace.txt |  2 ++
>  kernel/trace/trace_kprobe.c | 13 +
>  kernel/trace/trace_probe.c  |  5 +
>  kernel/trace/trace_probe.h  |  2 ++
>  kernel/trace/trace_uprobe.c |  7 +++
>  5 files changed, 29 insertions(+)
>
> diff --git a/Documentation/trace/kprobetrace.txt 
> b/Documentation/trace/kprobetrace.txt
> index d68ea5fc812b..90914cbc1e51 100644
> --- a/Documentation/trace/kprobetrace.txt
> +++ b/Documentation/trace/kprobetrace.txt
> @@ -40,6 +40,7 @@ Synopsis of kprobe_events
>$stackN  : Fetch Nth entry of stack (N >= 0)
>$stack   : Fetch stack address.
>$retval  : Fetch return value.(*)
> +  $comm: Fetch current task comm.(***)
>+|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
>NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
>FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
> @@ -48,6 +49,7 @@ Synopsis of kprobe_events
>
>(*) only for return probe.
>(**) this is useful for fetching a field of data structures.
> +  (***) you probably want this as a string, i.e., $comm:string
>
>  Types
>  -
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5546eec0505f..e3a087552d49 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -214,6 +214,18 @@ static void FETCH_FUNC_NAME(memory, string_size)(struct 
> pt_regs *regs,
>  }
>  NOKPROBE_SYMBOL(FETCH_FUNC_NAME(memory, string_size));
>
> +#define DEFINE_FETCH_comm(type)\
> +void FETCH_FUNC_NAME(comm, type)(struct pt_regs *regs, \
> +void *data, void *dest)\
> +{  \
> +   fetch_memory_##type(regs, current->comm, dest); \
> +}  \
> +NOKPROBE_SYMBOL(FETCH_FUNC_NAME(comm, type));
> +
> +DEFINE_BASIC_FETCH_FUNCS(comm)
> +DEFINE_FETCH_comm(string)
> +DEFINE_FETCH_comm(string_size)

I think you can define default type fetch functions (such as u8, ...)
to NULL to make sure it's used as string type only.


> +
>  #define DEFINE_FETCH_symbol(type)  \
>  void FETCH_FUNC_NAME(symbol, type)(struct pt_regs *regs, void *data, void 
> *dest)\
>  {  \
> @@ -587,6 +599,7 @@ static int create_trace_kprobe(int argc, char **argv)
>  *  $retval : fetch return value
>  *  $stack  : fetch stack address
>  *  $stackN : fetch Nth of stack (N:0-)
> +*  $comm   : fetch current task comm
>  *  @ADDR   : fetch memory at ADDR (ADDR should be in kernel)
>  *  @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data 
> symbol)
>  *  %REG: fetch register REG
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 1d372fa6fefb..918775fcacd3 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -348,6 +348,11 @@ static int parse_probe_vars(char *arg, const struct 
> fetch_type *t,
> }
> } else
> ret = -EINVAL;
> +   } else if (strcmp(arg, "comm") == 0) {
> +   if (is_kprobe)
> +   f->fn = t->fetch[FETCH_MTD_comm];
> +   else
> +   ret = -EINVAL;

I think it should work with uprobes too.  Why not enabling it to uprobes?

Thanks,
Namhyung


> } else
> ret = -EINVAL;
>
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index f6398db09114..3f1a987e6dc1 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -102,6 +102,7 @@ enum {
> FETCH_MTD_reg = 0,
> FETCH_MTD_stack,
> FETCH_MTD_retval,
> +   FETCH_MTD_comm,
> FETCH_MTD_memory,
> FETCH_MTD_symbol,
> FETCH_MTD_deref,
> @@ -213,6 +214,7 @@ 

Re: [PATCH] tracing: expose current->comm to kprobe events

[Namhyung Kim]

Hello,

On Wed, Jun 1, 2016 at 3:17 PM, Omar Sandoval  wrote:
> From: Omar Sandoval 
>
> ftrace is very quick to give up on saving the task command line (see
> `trace_save_cmdline()`). The workaround for events which really care
> about the command line is to explicitly assign it as part of the entry.
> However, this doesn't work for kprobe events, as there's no
> straightforward way to get access to current->comm. Add a kprobe event
> variable $comm which provides exactly that.
>
> Signed-off-by: Omar Sandoval 
> ---
> This is a problem I ran into while trying to debug an unrelated issue. I
> was considering trying to make ftrace try harder to save the command
> line, but that would most likely be at the expense at most events which
> don't care about the task comm. This is much less work.
>
>  Documentation/trace/kprobetrace.txt |  2 ++
>  kernel/trace/trace_kprobe.c | 13 +
>  kernel/trace/trace_probe.c  |  5 +
>  kernel/trace/trace_probe.h  |  2 ++
>  kernel/trace/trace_uprobe.c |  7 +++
>  5 files changed, 29 insertions(+)
>
> diff --git a/Documentation/trace/kprobetrace.txt 
> b/Documentation/trace/kprobetrace.txt
> index d68ea5fc812b..90914cbc1e51 100644
> --- a/Documentation/trace/kprobetrace.txt
> +++ b/Documentation/trace/kprobetrace.txt
> @@ -40,6 +40,7 @@ Synopsis of kprobe_events
>$stackN  : Fetch Nth entry of stack (N >= 0)
>$stack   : Fetch stack address.
>$retval  : Fetch return value.(*)
> +  $comm: Fetch current task comm.(***)
>+|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
>NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
>FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
> @@ -48,6 +49,7 @@ Synopsis of kprobe_events
>
>(*) only for return probe.
>(**) this is useful for fetching a field of data structures.
> +  (***) you probably want this as a string, i.e., $comm:string
>
>  Types
>  -
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5546eec0505f..e3a087552d49 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -214,6 +214,18 @@ static void FETCH_FUNC_NAME(memory, string_size)(struct 
> pt_regs *regs,
>  }
>  NOKPROBE_SYMBOL(FETCH_FUNC_NAME(memory, string_size));
>
> +#define DEFINE_FETCH_comm(type)\
> +void FETCH_FUNC_NAME(comm, type)(struct pt_regs *regs, \
> +void *data, void *dest)\
> +{  \
> +   fetch_memory_##type(regs, current->comm, dest); \
> +}  \
> +NOKPROBE_SYMBOL(FETCH_FUNC_NAME(comm, type));
> +
> +DEFINE_BASIC_FETCH_FUNCS(comm)
> +DEFINE_FETCH_comm(string)
> +DEFINE_FETCH_comm(string_size)

I think you can define default type fetch functions (such as u8, ...)
to NULL to make sure it's used as string type only.


> +
>  #define DEFINE_FETCH_symbol(type)  \
>  void FETCH_FUNC_NAME(symbol, type)(struct pt_regs *regs, void *data, void 
> *dest)\
>  {  \
> @@ -587,6 +599,7 @@ static int create_trace_kprobe(int argc, char **argv)
>  *  $retval : fetch return value
>  *  $stack  : fetch stack address
>  *  $stackN : fetch Nth of stack (N:0-)
> +*  $comm   : fetch current task comm
>  *  @ADDR   : fetch memory at ADDR (ADDR should be in kernel)
>  *  @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data 
> symbol)
>  *  %REG: fetch register REG
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 1d372fa6fefb..918775fcacd3 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -348,6 +348,11 @@ static int parse_probe_vars(char *arg, const struct 
> fetch_type *t,
> }
> } else
> ret = -EINVAL;
> +   } else if (strcmp(arg, "comm") == 0) {
> +   if (is_kprobe)
> +   f->fn = t->fetch[FETCH_MTD_comm];
> +   else
> +   ret = -EINVAL;

I think it should work with uprobes too.  Why not enabling it to uprobes?

Thanks,
Namhyung


> } else
> ret = -EINVAL;
>
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index f6398db09114..3f1a987e6dc1 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -102,6 +102,7 @@ enum {
> FETCH_MTD_reg = 0,
> FETCH_MTD_stack,
> FETCH_MTD_retval,
> +   FETCH_MTD_comm,
> FETCH_MTD_memory,
> FETCH_MTD_symbol,
> FETCH_MTD_deref,
> @@ -213,6 +214,7 @@ DEFINE_FETCH_##method(u64)
>  ASSIGN_FETCH_FUNC(reg, ftype), 

Re: [PATCH 2/2] aer: add support aer interrupt with none MSI/MSI-X/INTx mode

[Murali Karicheri]

On 06/02/2016 09:55 AM, Bjorn Helgaas wrote:
> On Thu, Jun 02, 2016 at 05:01:19AM +, Po Liu wrote:
>>>  -Original Message-
>>>  From: Bjorn Helgaas [mailto:helg...@kernel.org]
>>>  Sent: Thursday, June 02, 2016 11:48 AM
>>>  To: Po Liu
>>>  Cc: linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
>>>  linux-kernel@vger.kernel.org; devicet...@vger.kernel.org; Arnd Bergmann;
>>>  Roy Zang; Marc Zyngier; Stuart Yoder; Yang-Leo Li; Minghuan Lian; Bjorn
>>>  Helgaas; Shawn Guo; Mingkai Hu; Rob Herring
>>>  Subject: Re: [PATCH 2/2] aer: add support aer interrupt with none
>>>  MSI/MSI-X/INTx mode
>>>  
>>>  [+cc Rob]
>>>  
>>>  Hi Po,
>>>  
>>>  On Thu, May 26, 2016 at 02:00:06PM +0800, Po Liu wrote:
>>>  > On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
>>>  > When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
>>>  > maybe there is interrupt line for aer pme etc. Search the interrupt
>>>  > number in the fdt file.
>>>  
>>>  My understanding is that AER interrupt signaling can be done via INTx,
>>>  MSI, or MSI-X (PCIe spec r3.0, sec 6.2.4.1.2).  Apparently your device
>>>  doesn't support MSI or MSI-X.  Are you saying it doesn't support INTx
>>>  either?  How is the interrupt you're requesting here different from INTx?
>>
>> Layerscape use none of MSI or MSI-X or INTx to indicate the devices
>> or root error in RC mode. But use an independent SPI interrupt(arm
>> interrupt controller) line.  
> 
> The Root Port is a PCI device and should follow the normal PCI rules
> for interrupts.  As far as I understand, that means it should use MSI,
> MSI-X, or INTx.  If your Root Port doesn't use MSI or MSI-X, it should
> use INTx, the PCI_INTERRUPT_PIN register should tell us which (INTA/
> INTB/etc.), and PCI_COMMAND_INTX_DISABLE should work to disable it.
> That's all from the PCI point of view, of course.
> 
Bjorn.

I am faced with the same issue on Keystone PCI hardware and it has been
on my TODO list  for quite some time. Keystone PCI hardware also doesn't
use MSI or MSI-X or INTx for reporting errors received at the root port,
but use a platform interrupt instead (not complaint to PCI standard as
per PCI base spec). So I would need similar change to have the error
interrupt passed to the aer driver. So there are hardware out there
like Keystone which requires to support this through platform IRQ.

> What's on the other end of those interrupts is outside the scope of
> PCI.  An INTx interrupt (either a conventional PCI wire or a PCIe
> virtual INTx wire) might be connected to an IOAPIC, an ARM SPI, or
> something else.  Drivers should not care about how it is connected,
> and that's why I don't think this code really fits in portdrv.
> 
> Maybe your Root Port is non-compliant in some way and maybe we need a
> quirk or something to work around that hardware defect.  But I'm not
> convinced yet that we have that sort of problem.  It seems like we
> just don't have the correct DT description.

Is quirk is what is suggested here to support this?

Murali
> 
>> And at same time, AER capability list
>> in PCIe register descriptors. And also, The Root Error
>> Command/status register was proper to enable/disable the AER
>> interrupt.
> 
> I'm not sure what you're saying here.  Here's what I think you said;
> please correct me if I'm wrong:
> 
>   - Your Root Port has an AER capability.
> 
>   - Your Root Port does not support MSI or MSI-X.  (This would
> actually be a spec violation because the PCIe spec r3.0, sec 7.7
> says "All PCI Express device Functions that are capable of
> generating interrupts must implement MSI or MSI-X or both.")
>   
>   - The three enable bits in the Root Error Command Register enable or
> disable generation of interrupts.  These enable bits control all
> interrupts, whether MSI, MSI-X, or INTx.
> 
>   - The Root Error Status Register contains an "Advanced Error
> Interrupt Message Number" field, but that's only applicable if MSI
> or MSI-X is enabled, and if your device doesn't support MSI or
> MSI-X, this field doesn't apply.
> 
>> This hardware implementation make sense in some platforms, and this
>> was described in the function init_service_irqs() in the
>> drivers/pci/pcie/portdrv_core.c in current kernel as:
> 
>>
>> 241  * We're not going to use MSI-X, so try MSI and fall back to INTx.   
>>   
>> 242  * If neither MSI/MSI-X nor INTx available, try other interrupt.  On 
>>  
>> 243  * some platforms, root port doesn't support MSI/MSI-X/INTx in RC 
>> mode
>> 244  */  
>>
>>
>> So I think this was the proper place to update the irq number before aer 
>> service
>> driver was loaded.
>>
>>>  
>>>  My guess is that your device *does* support INTx, and we should use that.
>>>  ACPI has the generic _PRT that describes INTx routing.  There must be
>>>  something similar for DT, but I don't know what it is.  

Re: [PATCH 2/2] aer: add support aer interrupt with none MSI/MSI-X/INTx mode

[Murali Karicheri]

On 06/02/2016 09:55 AM, Bjorn Helgaas wrote:
> On Thu, Jun 02, 2016 at 05:01:19AM +, Po Liu wrote:
>>>  -Original Message-
>>>  From: Bjorn Helgaas [mailto:helg...@kernel.org]
>>>  Sent: Thursday, June 02, 2016 11:48 AM
>>>  To: Po Liu
>>>  Cc: linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
>>>  linux-kernel@vger.kernel.org; devicet...@vger.kernel.org; Arnd Bergmann;
>>>  Roy Zang; Marc Zyngier; Stuart Yoder; Yang-Leo Li; Minghuan Lian; Bjorn
>>>  Helgaas; Shawn Guo; Mingkai Hu; Rob Herring
>>>  Subject: Re: [PATCH 2/2] aer: add support aer interrupt with none
>>>  MSI/MSI-X/INTx mode
>>>  
>>>  [+cc Rob]
>>>  
>>>  Hi Po,
>>>  
>>>  On Thu, May 26, 2016 at 02:00:06PM +0800, Po Liu wrote:
>>>  > On some platforms, root port doesn't support MSI/MSI-X/INTx in RC mode.
>>>  > When chip support the aer interrupt with none MSI/MSI-X/INTx mode,
>>>  > maybe there is interrupt line for aer pme etc. Search the interrupt
>>>  > number in the fdt file.
>>>  
>>>  My understanding is that AER interrupt signaling can be done via INTx,
>>>  MSI, or MSI-X (PCIe spec r3.0, sec 6.2.4.1.2).  Apparently your device
>>>  doesn't support MSI or MSI-X.  Are you saying it doesn't support INTx
>>>  either?  How is the interrupt you're requesting here different from INTx?
>>
>> Layerscape use none of MSI or MSI-X or INTx to indicate the devices
>> or root error in RC mode. But use an independent SPI interrupt(arm
>> interrupt controller) line.  
> 
> The Root Port is a PCI device and should follow the normal PCI rules
> for interrupts.  As far as I understand, that means it should use MSI,
> MSI-X, or INTx.  If your Root Port doesn't use MSI or MSI-X, it should
> use INTx, the PCI_INTERRUPT_PIN register should tell us which (INTA/
> INTB/etc.), and PCI_COMMAND_INTX_DISABLE should work to disable it.
> That's all from the PCI point of view, of course.
> 
Bjorn.

I am faced with the same issue on Keystone PCI hardware and it has been
on my TODO list  for quite some time. Keystone PCI hardware also doesn't
use MSI or MSI-X or INTx for reporting errors received at the root port,
but use a platform interrupt instead (not complaint to PCI standard as
per PCI base spec). So I would need similar change to have the error
interrupt passed to the aer driver. So there are hardware out there
like Keystone which requires to support this through platform IRQ.

> What's on the other end of those interrupts is outside the scope of
> PCI.  An INTx interrupt (either a conventional PCI wire or a PCIe
> virtual INTx wire) might be connected to an IOAPIC, an ARM SPI, or
> something else.  Drivers should not care about how it is connected,
> and that's why I don't think this code really fits in portdrv.
> 
> Maybe your Root Port is non-compliant in some way and maybe we need a
> quirk or something to work around that hardware defect.  But I'm not
> convinced yet that we have that sort of problem.  It seems like we
> just don't have the correct DT description.

Is quirk is what is suggested here to support this?

Murali
> 
>> And at same time, AER capability list
>> in PCIe register descriptors. And also, The Root Error
>> Command/status register was proper to enable/disable the AER
>> interrupt.
> 
> I'm not sure what you're saying here.  Here's what I think you said;
> please correct me if I'm wrong:
> 
>   - Your Root Port has an AER capability.
> 
>   - Your Root Port does not support MSI or MSI-X.  (This would
> actually be a spec violation because the PCIe spec r3.0, sec 7.7
> says "All PCI Express device Functions that are capable of
> generating interrupts must implement MSI or MSI-X or both.")
>   
>   - The three enable bits in the Root Error Command Register enable or
> disable generation of interrupts.  These enable bits control all
> interrupts, whether MSI, MSI-X, or INTx.
> 
>   - The Root Error Status Register contains an "Advanced Error
> Interrupt Message Number" field, but that's only applicable if MSI
> or MSI-X is enabled, and if your device doesn't support MSI or
> MSI-X, this field doesn't apply.
> 
>> This hardware implementation make sense in some platforms, and this
>> was described in the function init_service_irqs() in the
>> drivers/pci/pcie/portdrv_core.c in current kernel as:
> 
>>
>> 241  * We're not going to use MSI-X, so try MSI and fall back to INTx.   
>>   
>> 242  * If neither MSI/MSI-X nor INTx available, try other interrupt.  On 
>>  
>> 243  * some platforms, root port doesn't support MSI/MSI-X/INTx in RC 
>> mode
>> 244  */  
>>
>>
>> So I think this was the proper place to update the irq number before aer 
>> service
>> driver was loaded.
>>
>>>  
>>>  My guess is that your device *does* support INTx, and we should use that.
>>>  ACPI has the generic _PRT that describes INTx routing.  There must be
>>>  something similar for DT, but I don't know what it is.  

Re: [PATCH V2 2/6] cpufreq: Remove cpufreq_frequency_get_table()

[Viresh Kumar]

On 2 June 2016 at 20:29, Javi Merino  wrote:
> In 5a31d594a973 ("cpufreq: Allow freq_table to be obtained for offline
> CPUs") you did the opposite: don't use cpufreq_cpu_get_raw() because
> it won't give you the policy of a cpu that is offline.  Now you are
> arguing that we should go back to cpufreq_cpu_get() which implicitly
> calls cpufreq_cpu_get_raw().  Won't we hit the same issue that
> 5a31d594a973 was trying to prevent: that we can't get a freq_table for
> a cpu that is offline?

Yes, that should be fixed. Thanks for letting me know about it :)

Re: [PATCH V2 2/6] cpufreq: Remove cpufreq_frequency_get_table()

[Viresh Kumar]

On 2 June 2016 at 20:29, Javi Merino  wrote:
> In 5a31d594a973 ("cpufreq: Allow freq_table to be obtained for offline
> CPUs") you did the opposite: don't use cpufreq_cpu_get_raw() because
> it won't give you the policy of a cpu that is offline.  Now you are
> arguing that we should go back to cpufreq_cpu_get() which implicitly
> calls cpufreq_cpu_get_raw().  Won't we hit the same issue that
> 5a31d594a973 was trying to prevent: that we can't get a freq_table for
> a cpu that is offline?

Yes, that should be fixed. Thanks for letting me know about it :)

Re: [PATCH 2/5] i2c: Add STM32F4 I2C driver

[M'boumba Cedric Madianga]

Hi,

>> +
>> +/**
>> + * stm32f4_i2c_xfer() - Transfer combined I2C message
>> + * @i2c_adap: Adapter pointer to the controller
>> + * @msgs: Pointer to data to be written.
>> + * @num: Number of messages to be executed
>> + */
>> +static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg 
>> msgs[],
>> +   int num)
>> +{
>> +   struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
>> +   int ret, i;
>> +
>> +   i2c_dev->busy = true;
>> +
>> +   ret = clk_prepare_enable(i2c_dev->clk);
>> +   if (ret) {
>> +   dev_err(i2c_dev->dev, "Failed to prepare_enable clock\n");
>> +   return ret;
>> +   }
>> +
>> +   stm32f4_i2c_hw_config(i2c_dev);
> Maybe you could call this only at probe and resume time?
> You would save some register accesses.
Some clarification about this point.
We need to call stm32f4_i2c_hw_config before each I2C transfer as at
the end of I2C communication the peripheral is automatically disabled
and configuration registers are reset.


BR,
Cedric

Re: [PATCH 2/5] i2c: Add STM32F4 I2C driver

[M'boumba Cedric Madianga]

Hi,

>> +
>> +/**
>> + * stm32f4_i2c_xfer() - Transfer combined I2C message
>> + * @i2c_adap: Adapter pointer to the controller
>> + * @msgs: Pointer to data to be written.
>> + * @num: Number of messages to be executed
>> + */
>> +static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg 
>> msgs[],
>> +   int num)
>> +{
>> +   struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
>> +   int ret, i;
>> +
>> +   i2c_dev->busy = true;
>> +
>> +   ret = clk_prepare_enable(i2c_dev->clk);
>> +   if (ret) {
>> +   dev_err(i2c_dev->dev, "Failed to prepare_enable clock\n");
>> +   return ret;
>> +   }
>> +
>> +   stm32f4_i2c_hw_config(i2c_dev);
> Maybe you could call this only at probe and resume time?
> You would save some register accesses.
Some clarification about this point.
We need to call stm32f4_i2c_hw_config before each I2C transfer as at
the end of I2C communication the peripheral is automatically disabled
and configuration registers are reset.


BR,
Cedric

[PATCH v2 0/5] nbd: fixes for nbd

[Pranay Kr. Srivastava]

This patch series fixes the following

1) fix might_sleep warning on socket shutdown:
   Fix sock_shutdown to avoid calling kernel_sock_shutdown
   while holding spin_lock.

2) cleanup nbd_set_socket
   Cleanup nbd_set_socket to use spin_lock instead of
   irq version and remove the goto statement in favour
   of a simple if-else statement.

3) fix various coding standard warnings
   Make shutdown get called in a process context instead, using
   system_wq.

4) make nbd device wait for its users.
   When a timeout or error occurs then nbd driver simply kills
   the block device. Many filesystem(s) example ext2/ext3 don't
   expect their buffer heads to disappear like that. Fix this
   by making nbd device wait for its users.

   Introduced a new field to check if the device is currently
   in use or not. This helps to check if the kref_put should
   be done on device release or not.
   
   This field needs to be atomic as the release function may
   be called from NBD_DO_IT as well as from device's release
   function.

5) use device_attr macros for sysfs attribute
   use DEVICE_ATTR_RO for sysfs pid attribute.

Changelog for v2:
1) fix might_sleep warning on socket shutdown
   use bool timedout instead of atomic

2) cleanup nbd_set_socket
   Added this new patch to this series.

3) fix various coding standard warnings
   No Change.

4) make nbd device wait for its users
   Earlier version used to do a final kref put when
   the kref->counter == 2. This required a check of
   the internal atomic counter of kref which was ugly.

   v2 of this patch make this more readable and doesn't
   do manual check of the internal counter used by kref.

5) use device_attr macros for sysfs attribute
   No Change.

Pranay Kr. Srivastava (5):
  fix might_sleep warning on socket shutdown.
  cleanup nbd_set_socket
  fix various coding standard warnings
  make nbd device wait for its users.
  use device_attr macros for sysfs attribute

 drivers/block/nbd.c | 173 +---
 1 file changed, 124 insertions(+), 49 deletions(-)

-- 
2.6.2

[PATCH v2 0/5] nbd: fixes for nbd

[Pranay Kr. Srivastava]

This patch series fixes the following

1) fix might_sleep warning on socket shutdown:
   Fix sock_shutdown to avoid calling kernel_sock_shutdown
   while holding spin_lock.

2) cleanup nbd_set_socket
   Cleanup nbd_set_socket to use spin_lock instead of
   irq version and remove the goto statement in favour
   of a simple if-else statement.

3) fix various coding standard warnings
   Make shutdown get called in a process context instead, using
   system_wq.

4) make nbd device wait for its users.
   When a timeout or error occurs then nbd driver simply kills
   the block device. Many filesystem(s) example ext2/ext3 don't
   expect their buffer heads to disappear like that. Fix this
   by making nbd device wait for its users.

   Introduced a new field to check if the device is currently
   in use or not. This helps to check if the kref_put should
   be done on device release or not.
   
   This field needs to be atomic as the release function may
   be called from NBD_DO_IT as well as from device's release
   function.

5) use device_attr macros for sysfs attribute
   use DEVICE_ATTR_RO for sysfs pid attribute.

Changelog for v2:
1) fix might_sleep warning on socket shutdown
   use bool timedout instead of atomic

2) cleanup nbd_set_socket
   Added this new patch to this series.

3) fix various coding standard warnings
   No Change.

4) make nbd device wait for its users
   Earlier version used to do a final kref put when
   the kref->counter == 2. This required a check of
   the internal atomic counter of kref which was ugly.

   v2 of this patch make this more readable and doesn't
   do manual check of the internal counter used by kref.

5) use device_attr macros for sysfs attribute
   No Change.

Pranay Kr. Srivastava (5):
  fix might_sleep warning on socket shutdown.
  cleanup nbd_set_socket
  fix various coding standard warnings
  make nbd device wait for its users.
  use device_attr macros for sysfs attribute

 drivers/block/nbd.c | 173 +---
 1 file changed, 124 insertions(+), 49 deletions(-)

-- 
2.6.2

[PATCH v2 1/5] nbd: fix might_sleep warning on socket shutdown.

[Pranay Kr. Srivastava]

spinlocked ranges should be small and not contain calls into huge
subfunctions. Fix my mistake and just get the pointer to the socket
instead of doing everything with spinlock held.

Reported-by: Mikulas Patocka 
Signed-off-by: Markus Pargmann 

Changelog:
Pranay Kr. Srivastava:

1) Use spin_lock instead of irq version for sock_shutdown.

2) Use system work queue to actually trigger the shutdown of
   socket. This solves the issue when kernel_sendmsg is currently
   blocked while a timeout occurs.

Signed-off-by: Pranay Kr. Srivastava 
---
 drivers/block/nbd.c | 65 ++---
 1 file changed, 42 insertions(+), 23 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 31e73a7..0339d40 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -39,6 +39,7 @@
 #include 
 
 #include 
+#include 
 
 struct nbd_device {
u32 flags;
@@ -69,6 +70,10 @@ struct nbd_device {
 #if IS_ENABLED(CONFIG_DEBUG_FS)
struct dentry *dbg_dir;
 #endif
+   /*
+*This is specifically for calling sock_shutdown, for now.
+*/
+   struct work_struct ws_shutdown;
 };
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
@@ -95,6 +100,11 @@ static int max_part;
  */
 static DEFINE_SPINLOCK(nbd_lock);
 
+/*
+ * Shutdown function for nbd_dev work struct.
+ */
+static void nbd_ws_func_shutdown(struct work_struct *);
+
 static inline struct device *nbd_to_dev(struct nbd_device *nbd)
 {
return disk_to_dev(nbd->disk);
@@ -172,39 +182,35 @@ static void nbd_end_request(struct nbd_device *nbd, 
struct request *req)
  */
 static void sock_shutdown(struct nbd_device *nbd)
 {
-   spin_lock_irq(>sock_lock);
-
-   if (!nbd->sock) {
-   spin_unlock_irq(>sock_lock);
-   return;
-   }
+   struct socket *sock;
 
-   dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
-   kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
-   sockfd_put(nbd->sock);
+   spin_lock(>sock_lock);
+   sock = nbd->sock;
nbd->sock = NULL;
-   spin_unlock_irq(>sock_lock);
+   spin_unlock(>sock_lock);
+
+   if (!sock)
+   return;
 
del_timer(>timeout_timer);
+   dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
+   kernel_sock_shutdown(sock, SHUT_RDWR);
+   sockfd_put(sock);
 }
 
 static void nbd_xmit_timeout(unsigned long arg)
 {
struct nbd_device *nbd = (struct nbd_device *)arg;
-   unsigned long flags;
 
if (list_empty(>queue_head))
return;
-
-   spin_lock_irqsave(>sock_lock, flags);
-
nbd->timedout = true;
-
-   if (nbd->sock)
-   kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
-
-   spin_unlock_irqrestore(>sock_lock, flags);
-
+   schedule_work(>ws_shutdown);
+   /*
+* Make sure sender thread sees nbd->timedout.
+*/
+   smp_wmb();
+   wake_up(>waiting_wq);
dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down 
connection\n");
 }
 
@@ -592,7 +598,11 @@ static int nbd_thread_send(void *data)
spin_unlock_irq(>queue_lock);
 
/* handle request */
-   nbd_handle_req(nbd, req);
+   if (nbd->timedout) {
+   req->errors++;
+   nbd_end_request(nbd, req);
+   } else
+   nbd_handle_req(nbd, req);
}
 
nbd->task_send = NULL;
@@ -672,6 +682,7 @@ static void nbd_reset(struct nbd_device *nbd)
set_capacity(nbd->disk, 0);
nbd->flags = 0;
nbd->xmit_timeout = 0;
+   INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown);
queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue);
del_timer_sync(>timeout_timer);
 }
@@ -804,15 +815,15 @@ static int __nbd_ioctl(struct block_device *bdev, struct 
nbd_device *nbd,
nbd_dev_dbg_close(nbd);
kthread_stop(thread);
 
-   mutex_lock(>tx_lock);
-
sock_shutdown(nbd);
+   mutex_lock(>tx_lock);
nbd_clear_que(nbd);
kill_bdev(bdev);
nbd_bdev_reset(bdev);
 
if (nbd->disconnect) /* user requested, ignore socket errors */
error = 0;
+
if (nbd->timedout)
error = -ETIMEDOUT;
 
@@ -863,6 +874,14 @@ static const struct block_device_operations nbd_fops =
.compat_ioctl = nbd_ioctl,
 };
 
+static void nbd_ws_func_shutdown(struct work_struct *ws_nbd)
+{
+   struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device,
+   ws_shutdown);
+
+   sock_shutdown(nbd_dev);
+}
+
 #if IS_ENABLED(CONFIG_DEBUG_FS)
 
 static int nbd_dbg_tasks_show(struct seq_file *s, void *unused)
-- 
2.6.2

[PATCH v2 1/5] nbd: fix might_sleep warning on socket shutdown.

[Pranay Kr. Srivastava]

spinlocked ranges should be small and not contain calls into huge
subfunctions. Fix my mistake and just get the pointer to the socket
instead of doing everything with spinlock held.

Reported-by: Mikulas Patocka 
Signed-off-by: Markus Pargmann 

Changelog:
Pranay Kr. Srivastava:

1) Use spin_lock instead of irq version for sock_shutdown.

2) Use system work queue to actually trigger the shutdown of
   socket. This solves the issue when kernel_sendmsg is currently
   blocked while a timeout occurs.

Signed-off-by: Pranay Kr. Srivastava 
---
 drivers/block/nbd.c | 65 ++---
 1 file changed, 42 insertions(+), 23 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 31e73a7..0339d40 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -39,6 +39,7 @@
 #include 
 
 #include 
+#include 
 
 struct nbd_device {
u32 flags;
@@ -69,6 +70,10 @@ struct nbd_device {
 #if IS_ENABLED(CONFIG_DEBUG_FS)
struct dentry *dbg_dir;
 #endif
+   /*
+*This is specifically for calling sock_shutdown, for now.
+*/
+   struct work_struct ws_shutdown;
 };
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
@@ -95,6 +100,11 @@ static int max_part;
  */
 static DEFINE_SPINLOCK(nbd_lock);
 
+/*
+ * Shutdown function for nbd_dev work struct.
+ */
+static void nbd_ws_func_shutdown(struct work_struct *);
+
 static inline struct device *nbd_to_dev(struct nbd_device *nbd)
 {
return disk_to_dev(nbd->disk);
@@ -172,39 +182,35 @@ static void nbd_end_request(struct nbd_device *nbd, 
struct request *req)
  */
 static void sock_shutdown(struct nbd_device *nbd)
 {
-   spin_lock_irq(>sock_lock);
-
-   if (!nbd->sock) {
-   spin_unlock_irq(>sock_lock);
-   return;
-   }
+   struct socket *sock;
 
-   dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
-   kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
-   sockfd_put(nbd->sock);
+   spin_lock(>sock_lock);
+   sock = nbd->sock;
nbd->sock = NULL;
-   spin_unlock_irq(>sock_lock);
+   spin_unlock(>sock_lock);
+
+   if (!sock)
+   return;
 
del_timer(>timeout_timer);
+   dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n");
+   kernel_sock_shutdown(sock, SHUT_RDWR);
+   sockfd_put(sock);
 }
 
 static void nbd_xmit_timeout(unsigned long arg)
 {
struct nbd_device *nbd = (struct nbd_device *)arg;
-   unsigned long flags;
 
if (list_empty(>queue_head))
return;
-
-   spin_lock_irqsave(>sock_lock, flags);
-
nbd->timedout = true;
-
-   if (nbd->sock)
-   kernel_sock_shutdown(nbd->sock, SHUT_RDWR);
-
-   spin_unlock_irqrestore(>sock_lock, flags);
-
+   schedule_work(>ws_shutdown);
+   /*
+* Make sure sender thread sees nbd->timedout.
+*/
+   smp_wmb();
+   wake_up(>waiting_wq);
dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down 
connection\n");
 }
 
@@ -592,7 +598,11 @@ static int nbd_thread_send(void *data)
spin_unlock_irq(>queue_lock);
 
/* handle request */
-   nbd_handle_req(nbd, req);
+   if (nbd->timedout) {
+   req->errors++;
+   nbd_end_request(nbd, req);
+   } else
+   nbd_handle_req(nbd, req);
}
 
nbd->task_send = NULL;
@@ -672,6 +682,7 @@ static void nbd_reset(struct nbd_device *nbd)
set_capacity(nbd->disk, 0);
nbd->flags = 0;
nbd->xmit_timeout = 0;
+   INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown);
queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue);
del_timer_sync(>timeout_timer);
 }
@@ -804,15 +815,15 @@ static int __nbd_ioctl(struct block_device *bdev, struct 
nbd_device *nbd,
nbd_dev_dbg_close(nbd);
kthread_stop(thread);
 
-   mutex_lock(>tx_lock);
-
sock_shutdown(nbd);
+   mutex_lock(>tx_lock);
nbd_clear_que(nbd);
kill_bdev(bdev);
nbd_bdev_reset(bdev);
 
if (nbd->disconnect) /* user requested, ignore socket errors */
error = 0;
+
if (nbd->timedout)
error = -ETIMEDOUT;
 
@@ -863,6 +874,14 @@ static const struct block_device_operations nbd_fops =
.compat_ioctl = nbd_ioctl,
 };
 
+static void nbd_ws_func_shutdown(struct work_struct *ws_nbd)
+{
+   struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device,
+   ws_shutdown);
+
+   sock_shutdown(nbd_dev);
+}
+
 #if IS_ENABLED(CONFIG_DEBUG_FS)
 
 static int nbd_dbg_tasks_show(struct seq_file *s, void *unused)
-- 
2.6.2

[BUG/REGRESSION] THP: broken page count after commit aa88b68c

[Gerald Schaefer]

Christian Borntraeger reported a kernel panic after corrupt page counts,
and it turned out to be a regression introduced with commit aa88b68c
"thp: keep huge zero page pinned until tlb flush", at least on s390.

put_huge_zero_page() was moved over from zap_huge_pmd() to release_pages(),
and it was replaced by tlb_remove_page(). However, release_pages() might
not always be triggered by (the arch-specific) tlb_remove_page().

On s390 we call free_page_and_swap_cache() from tlb_remove_page(), and not
tlb_flush_mmu() -> free_pages_and_swap_cache() like the generic version,
because we don't use the MMU-gather logic. Although both functions have very
similar names, they are doing very unsimilar things, in particular
free_page_xxx is just doing a put_page(), while free_pages_xxx calls
release_pages().

This of course results in very harmful put_page()s on the huge zero page,
on architectures where tlb_remove_page() is implemented in this way. It
seems to affect only s390 and sh, but sh doesn't have THP support, so
the problem (currently) probably only exists on s390.

The following quick hack fixed the issue:

diff --git a/mm/swap_state.c b/mm/swap_state.c
index 0d457e7..c99463a 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -252,7 +252,10 @@ static inline void free_swap_cache(struct page *page)
 void free_page_and_swap_cache(struct page *page)
 {
free_swap_cache(page);
-   put_page(page);
+   if (is_huge_zero_page(page))
+   put_huge_zero_page();
+   else
+   put_page(page);
 }
 
 /*

But of course there might be a better solution, and there still are some
questions left:
- Why does free_page_xxx() behave so differently from free_pages_xxx()?
- Would it be OK to implement free_page_xxx() by calling free_pages_xxx()
  with nr = 1, similar to free_page() vs. free_pages()?
- Would it be OK to replace the put_page() in free_page_xxx() with a call
  to release_pages() with nr = 1?
- Would it be better to fix this in the arch-specific tlb_remove_page(),
  by calling free_pages_xxx() with nr = 1 instead of free_page_xxx()?

Regards,
Gerald

[BUG/REGRESSION] THP: broken page count after commit aa88b68c

[Gerald Schaefer]

Christian Borntraeger reported a kernel panic after corrupt page counts,
and it turned out to be a regression introduced with commit aa88b68c
"thp: keep huge zero page pinned until tlb flush", at least on s390.

put_huge_zero_page() was moved over from zap_huge_pmd() to release_pages(),
and it was replaced by tlb_remove_page(). However, release_pages() might
not always be triggered by (the arch-specific) tlb_remove_page().

On s390 we call free_page_and_swap_cache() from tlb_remove_page(), and not
tlb_flush_mmu() -> free_pages_and_swap_cache() like the generic version,
because we don't use the MMU-gather logic. Although both functions have very
similar names, they are doing very unsimilar things, in particular
free_page_xxx is just doing a put_page(), while free_pages_xxx calls
release_pages().

This of course results in very harmful put_page()s on the huge zero page,
on architectures where tlb_remove_page() is implemented in this way. It
seems to affect only s390 and sh, but sh doesn't have THP support, so
the problem (currently) probably only exists on s390.

The following quick hack fixed the issue:

diff --git a/mm/swap_state.c b/mm/swap_state.c
index 0d457e7..c99463a 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -252,7 +252,10 @@ static inline void free_swap_cache(struct page *page)
 void free_page_and_swap_cache(struct page *page)
 {
free_swap_cache(page);
-   put_page(page);
+   if (is_huge_zero_page(page))
+   put_huge_zero_page();
+   else
+   put_page(page);
 }
 
 /*

But of course there might be a better solution, and there still are some
questions left:
- Why does free_page_xxx() behave so differently from free_pages_xxx()?
- Would it be OK to implement free_page_xxx() by calling free_pages_xxx()
  with nr = 1, similar to free_page() vs. free_pages()?
- Would it be OK to replace the put_page() in free_page_xxx() with a call
  to release_pages() with nr = 1?
- Would it be better to fix this in the arch-specific tlb_remove_page(),
  by calling free_pages_xxx() with nr = 1 instead of free_page_xxx()?

Regards,
Gerald

Re: [PATCH] rds: fix an infoleak in rds_inc_info_copy

[Santosh Shilimkar]

On 6/2/2016 1:11 AM, Kangjie Lu wrote:

The last field "flags" of object "minfo" is not initialized.
Copying this object out may leak kernel stack data.
Assign 0 to it to avoid leak.

Signed-off-by: Kangjie Lu 
---
 net/rds/recv.c | 2 ++
 1 file changed, 2 insertions(+)


Acked-by: Santosh Shilimkar 

Re: [PATCH] rds: fix an infoleak in rds_inc_info_copy

[Santosh Shilimkar]

On 6/2/2016 1:11 AM, Kangjie Lu wrote:

The last field "flags" of object "minfo" is not initialized.
Copying this object out may leak kernel stack data.
Assign 0 to it to avoid leak.

Signed-off-by: Kangjie Lu 
---
 net/rds/recv.c | 2 ++
 1 file changed, 2 insertions(+)


Acked-by: Santosh Shilimkar 

Re: [RFC PATCH 1/1] clk: imx7d: move clk setting out of imx7d_clocks_init

[Dong Aisheng]

On Fri, Apr 29, 2016 at 07:04:33PM -0700, Stefan Agner wrote:
> On 2016-04-29 02:45, Dong Aisheng wrote:
> > During kernel early booting(e.g. in time_init()), there's only one
> > init idle task running, and the idle sched class indicates that it's
> > not valid to schedule for idle task. If it happens the kernel
> > will complain with a error message as follows:
> > [0.00] bad: scheduling from the idle thread!
> > 
> > We can observe this warning on an i.MX7D SDB board. See full log below.
> > It is caused by imx7d_clocks_init function called in time_init
> > invokes a lot clk_prepare_enable to enable many clocks and it happens
> > that the Audio/Video PLLs need large delay causes a sleep.
> > 
> > Since we should not sleep during time_init, this patch fundamentally
> > moves all clk_prepare_enable and clk_set_parent out of imx7d_clocks_init
> > and use a postcore init function imx7d_clocks_setup to do it later instead.
> > Then we simply reply on the bootloader settings to do early boot.
> 
> Is this really a good idea? What happens if something requests a clock
> before postcore initcalls get called? E.g. clock source is initialized
> before that, which might request a clock...
> 

I think clock source driver usually will do clock configuration by itself
and it does not depends on imx7d_clocks_init.
e.g. smp_twd.c / timer-imx-gpt.c

However, one exception is that if the clock source needs change clock
parent, it may have to be done in its driver too.
Up till now we still have no such requirement and no board do like that.

> Ok, we can just say that all those clocks need to be bootloader on, but
> how do we know? Do we catch if the bootloader does not adhere to that? 
> 

clk source driver may need to handle that work and it seems we already
do that work in timer-imx-gpt.c.

But as i replied in another earlier email, clk source driver itself
may also result in a possible sleep with clock operation which is
a potential break too.
We may need discuss to find a suitable generic solution.

Regards
Dong Aisheng

> --
> Stefan
> 
> 
> > 
> > [0.00] bad: scheduling from the idle thread!
> > [0.00] CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW  
> > 4.6.0-rc3-00064-gded8bc08fb45-dirty #836
> > [0.00] Hardware name: Freescale i.MX7 Dual (Device Tree)
> > [0.00] Backtrace:
> > [0.00] [] (dump_backtrace) from []
> > (show_stack+0x18/0x1c)
> > [0.00]  r6:6053 r5: r4:c0d21f9c r3:
> > [0.00] [] (show_stack) from []
> > (dump_stack+0xb4/0xe8)
> > [0.00] [] (dump_stack) from []
> > (dequeue_task_idle+0x20/0x30)
> > [0.00]  r10:c08f5554 r9:c0d02b0c r8:c0d06844 r7:ef7c14d0
> > r6:0001 r5:ef7c14c0
> > [0.00]  r4:ef7c14c0 r3:
> > [0.00] [] (dequeue_task_idle) from []
> > (deactivate_task+0x64/0x68)
> > [0.00]  r4:c0d06500 r3:c0154368
> > [0.00] [] (deactivate_task) from []
> > (__schedule+0x2e8/0x738)
> > [0.00]  r6:c0d06500 r5:ef7c14c0 r4:c0c744c0 r3:0002
> > [0.00] [] (__schedule) from [] 
> > (schedule+0x48/0xa0)
> > [0.00]  r10:01b6 r9:0003 r8:0036 r7:
> > r6:0007a120 r5:
> > [0.00]  r4:c0d0
> > [0.00] [] (schedule) from []
> > (schedule_hrtimeout_range_clock+0xac/0x12c)
> > [0.00]  r4:0006ddd0 r3:c0d06500
> > [0.00] [] (schedule_hrtimeout_range_clock) from
> > [] (schedule_hrtimeout_range+0x24/0x2c)
> > [0.00]  r7:c0d5d434 r6:c0d02100 r5:8ad1 r4:ef00c040
> > [0.00] [] (schedule_hrtimeout_range) from
> > [] (usleep_range+0x54/0x5c)
> > [0.00] [] (usleep_range) from []
> > (clk_pllv3_wait_lock+0x7c/0xb4)
> > [0.00] [] (clk_pllv3_wait_lock) from []
> > (clk_pllv3_prepare+0x2c/0x30)
> > [0.00]  r6: r5:c15603f4 r4:ef007680 r3:201b
> > [0.00] [] (clk_pllv3_prepare) from []
> > (clk_core_prepare+0x98/0xc8)
> > [0.00] [] (clk_core_prepare) from []
> > (clk_prepare+0x20/0x38)
> > [0.00]  r5:c15603f4 r4:ef00c100
> > [0.00] [] (clk_prepare) from []
> > (imx7d_clocks_init+0x6108/0x6188)
> > [0.00]  r4:ef00c100 r3:0001
> > [0.00] [] (imx7d_clocks_init) from []
> > (of_clk_init+0x10c/0x1d4)
> > [0.00]  r10:0001 r9:c0d01f68 r8: r7:c0d01f60
> > r6:ef7e3d60 r5:ef004340
> > [0.00]  r4:0002
> > [0.00] [] (of_clk_init) from []
> > (time_init+0x2c/0x38)
> > [0.00]  r10:efffcac0 r9:c0c5ca48 r8:c0d76000 r7:c0d028c0
> > r6: r5:c0d76000
> > [0.00]  r4:
> > [0.00] [] (time_init) from []
> > (start_kernel+0x218/0x398)
> > [0.00] [] (start_kernel) from [<8000807c>] (0x8000807c)
> > [0.00]  r10: r9:410fc075 r8:8000406a r7:c0d07e8c
> > r6:c0c5ca44 r5:c0d0296c
> > [0.00]  r4:c0d76294
> > [0.00] Architected cp15 timer(s) running at 8.00MHz (virt).
> > [0.00] clocksource: 

Re: [RFC PATCH 1/1] clk: imx7d: move clk setting out of imx7d_clocks_init

[Dong Aisheng]

On Fri, Apr 29, 2016 at 07:04:33PM -0700, Stefan Agner wrote:
> On 2016-04-29 02:45, Dong Aisheng wrote:
> > During kernel early booting(e.g. in time_init()), there's only one
> > init idle task running, and the idle sched class indicates that it's
> > not valid to schedule for idle task. If it happens the kernel
> > will complain with a error message as follows:
> > [0.00] bad: scheduling from the idle thread!
> > 
> > We can observe this warning on an i.MX7D SDB board. See full log below.
> > It is caused by imx7d_clocks_init function called in time_init
> > invokes a lot clk_prepare_enable to enable many clocks and it happens
> > that the Audio/Video PLLs need large delay causes a sleep.
> > 
> > Since we should not sleep during time_init, this patch fundamentally
> > moves all clk_prepare_enable and clk_set_parent out of imx7d_clocks_init
> > and use a postcore init function imx7d_clocks_setup to do it later instead.
> > Then we simply reply on the bootloader settings to do early boot.
> 
> Is this really a good idea? What happens if something requests a clock
> before postcore initcalls get called? E.g. clock source is initialized
> before that, which might request a clock...
> 

I think clock source driver usually will do clock configuration by itself
and it does not depends on imx7d_clocks_init.
e.g. smp_twd.c / timer-imx-gpt.c

However, one exception is that if the clock source needs change clock
parent, it may have to be done in its driver too.
Up till now we still have no such requirement and no board do like that.

> Ok, we can just say that all those clocks need to be bootloader on, but
> how do we know? Do we catch if the bootloader does not adhere to that? 
> 

clk source driver may need to handle that work and it seems we already
do that work in timer-imx-gpt.c.

But as i replied in another earlier email, clk source driver itself
may also result in a possible sleep with clock operation which is
a potential break too.
We may need discuss to find a suitable generic solution.

Regards
Dong Aisheng

> --
> Stefan
> 
> 
> > 
> > [0.00] bad: scheduling from the idle thread!
> > [0.00] CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW  
> > 4.6.0-rc3-00064-gded8bc08fb45-dirty #836
> > [0.00] Hardware name: Freescale i.MX7 Dual (Device Tree)
> > [0.00] Backtrace:
> > [0.00] [] (dump_backtrace) from []
> > (show_stack+0x18/0x1c)
> > [0.00]  r6:6053 r5: r4:c0d21f9c r3:
> > [0.00] [] (show_stack) from []
> > (dump_stack+0xb4/0xe8)
> > [0.00] [] (dump_stack) from []
> > (dequeue_task_idle+0x20/0x30)
> > [0.00]  r10:c08f5554 r9:c0d02b0c r8:c0d06844 r7:ef7c14d0
> > r6:0001 r5:ef7c14c0
> > [0.00]  r4:ef7c14c0 r3:
> > [0.00] [] (dequeue_task_idle) from []
> > (deactivate_task+0x64/0x68)
> > [0.00]  r4:c0d06500 r3:c0154368
> > [0.00] [] (deactivate_task) from []
> > (__schedule+0x2e8/0x738)
> > [0.00]  r6:c0d06500 r5:ef7c14c0 r4:c0c744c0 r3:0002
> > [0.00] [] (__schedule) from [] 
> > (schedule+0x48/0xa0)
> > [0.00]  r10:01b6 r9:0003 r8:0036 r7:
> > r6:0007a120 r5:
> > [0.00]  r4:c0d0
> > [0.00] [] (schedule) from []
> > (schedule_hrtimeout_range_clock+0xac/0x12c)
> > [0.00]  r4:0006ddd0 r3:c0d06500
> > [0.00] [] (schedule_hrtimeout_range_clock) from
> > [] (schedule_hrtimeout_range+0x24/0x2c)
> > [0.00]  r7:c0d5d434 r6:c0d02100 r5:8ad1 r4:ef00c040
> > [0.00] [] (schedule_hrtimeout_range) from
> > [] (usleep_range+0x54/0x5c)
> > [0.00] [] (usleep_range) from []
> > (clk_pllv3_wait_lock+0x7c/0xb4)
> > [0.00] [] (clk_pllv3_wait_lock) from []
> > (clk_pllv3_prepare+0x2c/0x30)
> > [0.00]  r6: r5:c15603f4 r4:ef007680 r3:201b
> > [0.00] [] (clk_pllv3_prepare) from []
> > (clk_core_prepare+0x98/0xc8)
> > [0.00] [] (clk_core_prepare) from []
> > (clk_prepare+0x20/0x38)
> > [0.00]  r5:c15603f4 r4:ef00c100
> > [0.00] [] (clk_prepare) from []
> > (imx7d_clocks_init+0x6108/0x6188)
> > [0.00]  r4:ef00c100 r3:0001
> > [0.00] [] (imx7d_clocks_init) from []
> > (of_clk_init+0x10c/0x1d4)
> > [0.00]  r10:0001 r9:c0d01f68 r8: r7:c0d01f60
> > r6:ef7e3d60 r5:ef004340
> > [0.00]  r4:0002
> > [0.00] [] (of_clk_init) from []
> > (time_init+0x2c/0x38)
> > [0.00]  r10:efffcac0 r9:c0c5ca48 r8:c0d76000 r7:c0d028c0
> > r6: r5:c0d76000
> > [0.00]  r4:
> > [0.00] [] (time_init) from []
> > (start_kernel+0x218/0x398)
> > [0.00] [] (start_kernel) from [<8000807c>] (0x8000807c)
> > [0.00]  r10: r9:410fc075 r8:8000406a r7:c0d07e8c
> > r6:c0c5ca44 r5:c0d0296c
> > [0.00]  r4:c0d76294
> > [0.00] Architected cp15 timer(s) running at 8.00MHz (virt).
> > [0.00] clocksource: 

Re: [PATCH 09/32] bcm2837-rpi-3-b.dts for 32bit arm

[Arnd Bergmann]

On Thursday, June 2, 2016 5:11:31 PM CEST Gerd Hoffmann wrote:
> 
> > > > > Reference to ../../../arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
> > > > > directly in the Makefile?
> 
> Actually tried that, and to my surprise this worked fine for both "make
> dtbs" and "make dtbs_install".
> 
> So we should just do that I guess ...

Right.

> > > Yes, in theory.  No, in practice.  As far I know the rpi3 is the only
> > > 64bit soc where a almost identical 32bit version exists, so running
> > > 32bit kernels on a 64bit processor actually happens in practice and I
> > > expect this to continue.  If you want create sdcard images which run on
> > > any rpi variant this is pretty much the only reasonable way to do it.
> > 
> > I think the Allwinner A64 and the Samsung s5p6818 are other examples
> > for this, where the initial run of boards all run 32-bit kernels
> > for much of the same reasons. If users want to run a 32-bit distro
> > on rpi-3 and on e.g. orange-pi, I don't see why they wouldn't also run
> > the same binary on A64.
> 
> ... and others can join the party on a case-by-case basis.
> 
> I still expect for the majority of arm64 boards it is not very useful,
> so I don't think we should build all of them unconditionally.

Well, they are still controlled by Kconfig symbols. Some of them
are shared between arm and arm64 and you would just get all dtbs in that
case, while other symbols are specific to one of the two.

Just for fun, which machines do we actually get if we decide to
just enter the arch/arm64/boot/dts/ directory in an allmodconfig
build?

Arnd

Re: [PATCH 09/32] bcm2837-rpi-3-b.dts for 32bit arm

[Arnd Bergmann]

On Thursday, June 2, 2016 5:11:31 PM CEST Gerd Hoffmann wrote:
> 
> > > > > Reference to ../../../arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
> > > > > directly in the Makefile?
> 
> Actually tried that, and to my surprise this worked fine for both "make
> dtbs" and "make dtbs_install".
> 
> So we should just do that I guess ...

Right.

> > > Yes, in theory.  No, in practice.  As far I know the rpi3 is the only
> > > 64bit soc where a almost identical 32bit version exists, so running
> > > 32bit kernels on a 64bit processor actually happens in practice and I
> > > expect this to continue.  If you want create sdcard images which run on
> > > any rpi variant this is pretty much the only reasonable way to do it.
> > 
> > I think the Allwinner A64 and the Samsung s5p6818 are other examples
> > for this, where the initial run of boards all run 32-bit kernels
> > for much of the same reasons. If users want to run a 32-bit distro
> > on rpi-3 and on e.g. orange-pi, I don't see why they wouldn't also run
> > the same binary on A64.
> 
> ... and others can join the party on a case-by-case basis.
> 
> I still expect for the majority of arm64 boards it is not very useful,
> so I don't think we should build all of them unconditionally.

Well, they are still controlled by Kconfig symbols. Some of them
are shared between arm and arm64 and you would just get all dtbs in that
case, while other symbols are specific to one of the two.

Just for fun, which machines do we actually get if we decide to
just enter the arch/arm64/boot/dts/ directory in an allmodconfig
build?

Arnd

Re: [PATCH v3 3/3] ACPI / button: Add quirks for initial lid state notification

[Benjamin Tissoires]

On Thu, Jun 2, 2016 at 4:01 PM, Bastien Nocera  wrote:
> On Thu, 2016-06-02 at 01:08 +, Zheng, Lv wrote:
>> Hi,
>>
>> > From: Bastien Nocera [mailto:had...@hadess.net]
>> > Subject: Re: [PATCH v3 3/3] ACPI / button: Add quirks for initial
>> > lid state
>> > notification
>> >
>> > On Wed, 2016-06-01 at 18:10 +0800, Lv Zheng wrote:
>> > > Linux userspace (systemd-logind) keeps on rechecking lid state
>> > > when the
>> > > lid state is closed. If it failed to update the lid state to open
>> > > after
>> > > boot/resume, the system suspending right after the boot/resume
>> > > could
>> > be
>> > > resulted.
>> > > Graphics drivers also uses the lid notifications to implment
>> > > MODESET_ON_LID_OPEN option.
>> >
>> > "implement"
>> [Lv Zheng]
>> Thanks for pointing out, I'll send an UPDATE to this.
>>
>> >
>> > > Before the situation is improved from the userspace and from the
>> > graphics
>> > > driver, users can simply configure ACPI button driver to send
>> > > initial
>> > > "open" lid state using button.lid_init_state=open to avoid such
>> > > kind of
>> > > issues. And our ultimate target should be making
>> > > button.lid_init_state=ignore the default behavior. This patch
>> > > implements
>> > > the 2 options and keep the old behavior
>> > > (button.lid_init_state=method).
>> >
>> > I still don't think it's reasonable to expect any changes in user-
>> > space
>> > unless you start documenting what the API to user-space actually
>> > is.
>> [Lv Zheng]
>> IMO, the ACPI lid driver should be responsible for sending lid key
>> event (especially "close") to the user space.
>> So if someone need to implement an ACPI lid key event quirk, we could
>> help to implement it from the kernel space.
>> And since the initial lid state is not stable, we have to stop doing
>> quirks around it inside of the Linux kernel, or inside of the
>> customized AML tables.
>> User space can still access /proc/acpi/button/lid/LID0/state, but
>> should stop thinking that it is reliable.
>>
>> These are what I can conclude from the bugs.

After further thoughts, I also think it is a bad idea to request user
space to change behavior with respect to the LID switch event we
forward from the keyboard:
- it looks like Windows doesn't care about LID open on some
(entry-level) platforms: the Samsung N210 is one of the first netbooks
from 2010. The Surface (pro or not) are tablets. On these low cost
systems, we can easily assume that the user needs to have the LID open
to have the system working. You can't connect a docking station, and
they are probably not used in a professional environment.
- for the high end machines (think professional), we actually need to
have a valid LID state given that the machines can be used on a
docking station, so LID closed. If we do not send a reliable LID state
for those laptops we will break user space and more likely annoy
users: we might light up the closed internal monitor and migrate all
the currently open applications to this screen.

So I think Windows might be able to detect those 2 categories of
environments and behave accordingly.

Then, if we want to express to user space that the LID switch state is
not reliable, we should stop setting it up as an input device with a
EV_SWITCH in it. The kernel has to be reliable, and we can't start
saying that this particular switch in a system might not be reliable.
In this, I join Bastien's point of view where we need to start
deprecating and document what needs to be done, and introduce a new
way of reporting the LID events to user space (by using a
KEY_LID_CLOSE for instance).

I still think this patch is necessary. Until we manage to understand
what is going on on Windows for the non reliable LID state, we can
always ask users to use the button.lid_init_state=open to prevent the
freeze loops they are seeing.
The deprecation process could be to send the open state at resume on
the SW_LID event, and send both the close and a KEY_LID_CLOSE event on
close (no KEY_* on open).

For professional laptops (with docking capability), I can't see how we
could avoid forwarding a reliable state, and we need them to stick to
button.lid_init_state=method (keep SW_LID and not send the
KEY_LID_CLOSE event so userspace knows it's reliable).

Hope this helps,
Benjamin


>
> There's still no documentation for user-space in the patch, and no way
> to disable the "legacy" support (disabling access to the cached LID
> state, especially through the input layer which is what logind and
> upower use).
>
> You can't expect user-space to change in major ways for those few
> devices if the API doesn't force them to, through deprecation notices
> and documentation.
>
> Cheers

Re: [PATCH v3 3/3] ACPI / button: Add quirks for initial lid state notification

[Benjamin Tissoires]

On Thu, Jun 2, 2016 at 4:01 PM, Bastien Nocera  wrote:
> On Thu, 2016-06-02 at 01:08 +, Zheng, Lv wrote:
>> Hi,
>>
>> > From: Bastien Nocera [mailto:had...@hadess.net]
>> > Subject: Re: [PATCH v3 3/3] ACPI / button: Add quirks for initial
>> > lid state
>> > notification
>> >
>> > On Wed, 2016-06-01 at 18:10 +0800, Lv Zheng wrote:
>> > > Linux userspace (systemd-logind) keeps on rechecking lid state
>> > > when the
>> > > lid state is closed. If it failed to update the lid state to open
>> > > after
>> > > boot/resume, the system suspending right after the boot/resume
>> > > could
>> > be
>> > > resulted.
>> > > Graphics drivers also uses the lid notifications to implment
>> > > MODESET_ON_LID_OPEN option.
>> >
>> > "implement"
>> [Lv Zheng]
>> Thanks for pointing out, I'll send an UPDATE to this.
>>
>> >
>> > > Before the situation is improved from the userspace and from the
>> > graphics
>> > > driver, users can simply configure ACPI button driver to send
>> > > initial
>> > > "open" lid state using button.lid_init_state=open to avoid such
>> > > kind of
>> > > issues. And our ultimate target should be making
>> > > button.lid_init_state=ignore the default behavior. This patch
>> > > implements
>> > > the 2 options and keep the old behavior
>> > > (button.lid_init_state=method).
>> >
>> > I still don't think it's reasonable to expect any changes in user-
>> > space
>> > unless you start documenting what the API to user-space actually
>> > is.
>> [Lv Zheng]
>> IMO, the ACPI lid driver should be responsible for sending lid key
>> event (especially "close") to the user space.
>> So if someone need to implement an ACPI lid key event quirk, we could
>> help to implement it from the kernel space.
>> And since the initial lid state is not stable, we have to stop doing
>> quirks around it inside of the Linux kernel, or inside of the
>> customized AML tables.
>> User space can still access /proc/acpi/button/lid/LID0/state, but
>> should stop thinking that it is reliable.
>>
>> These are what I can conclude from the bugs.

After further thoughts, I also think it is a bad idea to request user
space to change behavior with respect to the LID switch event we
forward from the keyboard:
- it looks like Windows doesn't care about LID open on some
(entry-level) platforms: the Samsung N210 is one of the first netbooks
from 2010. The Surface (pro or not) are tablets. On these low cost
systems, we can easily assume that the user needs to have the LID open
to have the system working. You can't connect a docking station, and
they are probably not used in a professional environment.
- for the high end machines (think professional), we actually need to
have a valid LID state given that the machines can be used on a
docking station, so LID closed. If we do not send a reliable LID state
for those laptops we will break user space and more likely annoy
users: we might light up the closed internal monitor and migrate all
the currently open applications to this screen.

So I think Windows might be able to detect those 2 categories of
environments and behave accordingly.

Then, if we want to express to user space that the LID switch state is
not reliable, we should stop setting it up as an input device with a
EV_SWITCH in it. The kernel has to be reliable, and we can't start
saying that this particular switch in a system might not be reliable.
In this, I join Bastien's point of view where we need to start
deprecating and document what needs to be done, and introduce a new
way of reporting the LID events to user space (by using a
KEY_LID_CLOSE for instance).

I still think this patch is necessary. Until we manage to understand
what is going on on Windows for the non reliable LID state, we can
always ask users to use the button.lid_init_state=open to prevent the
freeze loops they are seeing.
The deprecation process could be to send the open state at resume on
the SW_LID event, and send both the close and a KEY_LID_CLOSE event on
close (no KEY_* on open).

For professional laptops (with docking capability), I can't see how we
could avoid forwarding a reliable state, and we need them to stick to
button.lid_init_state=method (keep SW_LID and not send the
KEY_LID_CLOSE event so userspace knows it's reliable).

Hope this helps,
Benjamin


>
> There's still no documentation for user-space in the patch, and no way
> to disable the "legacy" support (disabling access to the cached LID
> state, especially through the input layer which is what logind and
> upower use).
>
> You can't expect user-space to change in major ways for those few
> devices if the API doesn't force them to, through deprecation notices
> and documentation.
>
> Cheers

[PATCH v2 4/5]nbd: make nbd device wait for its users.

[Pranay Kr. Srivastava]

When a timeout occurs or a recv fails, then
instead of abruplty killing nbd block device
wait for it's users to finish.

This is more required when filesystem(s) like
ext2 or ext3 don't expect their buffer heads to
disappear while the filesystem is mounted.

Each open of a nbd device is refcounted, while
the userland program [nbd-client] doing the
NBD_DO_IT ioctl would now wait for any other users
of this device before invalidating the nbd device.

Signed-off-by: Pranay Kr. Srivastava 
---
 drivers/block/nbd.c | 58 +
 1 file changed, 58 insertions(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index d1d898d..4da40dc 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -70,10 +70,13 @@ struct nbd_device {
 #if IS_ENABLED(CONFIG_DEBUG_FS)
struct dentry *dbg_dir;
 #endif
+   atomic_t inuse;
/*
 *This is specifically for calling sock_shutdown, for now.
 */
struct work_struct ws_shutdown;
+   struct kref users;
+   struct completion user_completion;
 };
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
@@ -104,6 +107,7 @@ static DEFINE_SPINLOCK(nbd_lock);
  * Shutdown function for nbd_dev work struct.
  */
 static void nbd_ws_func_shutdown(struct work_struct *);
+static void nbd_kref_release(struct kref *);
 
 static inline struct device *nbd_to_dev(struct nbd_device *nbd)
 {
@@ -682,6 +686,8 @@ static void nbd_reset(struct nbd_device *nbd)
nbd->flags = 0;
nbd->xmit_timeout = 0;
INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown);
+   init_completion(>user_completion);
+   kref_init(>users);
queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue);
del_timer_sync(>timeout_timer);
 }
@@ -815,6 +821,14 @@ static int __nbd_ioctl(struct block_device *bdev, struct 
nbd_device *nbd,
kthread_stop(thread);
 
sock_shutdown(nbd);
+   /*
+* kref_init initializes with ref count as 1,
+* nbd_client, or the user-land program executing
+* this ioctl will make the refcount to 2[at least]
+* so subtracting 2 from refcount.
+*/
+   kref_sub(>users, 2, nbd_kref_release);
+   wait_for_completion(>user_completion);
mutex_lock(>tx_lock);
nbd_clear_que(nbd);
kill_bdev(bdev);
@@ -865,13 +879,56 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t 
mode,
 
return error;
 }
+static void nbd_kref_release(struct kref *kref_users)
+{
+   struct nbd_device *nbd = container_of(kref_users, struct nbd_device,
+   users);
+   pr_debug("Releasing kref [%s]\n", __func__);
+   atomic_set(>inuse, 0);
+   complete(>user_completion);
+
+}
+
+static int nbd_open(struct block_device *bdev, fmode_t mode)
+{
+   struct nbd_device *nbd_dev = bdev->bd_disk->private_data;
+
+   if (kref_get_unless_zero(_dev->users))
+   atomic_set(_dev->inuse, 1);
+
+   pr_debug("Opening nbd_dev %s. Active users = %u\n",
+   bdev->bd_disk->disk_name,
+   atomic_read(_dev->users.refcount) - 1);
+   return 0;
+}
+
+static void nbd_release(struct gendisk *disk, fmode_t mode)
+{
+   struct nbd_device *nbd_dev = disk->private_data;
+   /*
+   *kref_init initializes ref count to 1, so we
+   *we check for refcount to be 2 for a final put.
+   *
+   *kref needs to be re-initialized just here as the
+   *other process holding it must see the ref count as 2.
+   */
+   if (atomic_read(_dev->inuse))
+   kref_put(_dev->users,  nbd_kref_release);
+
+   pr_debug("Closing nbd_dev %s. Active users = %u\n",
+   disk->disk_name,
+   atomic_read(_dev->users.refcount) - 1);
+}
 
 static const struct block_device_operations nbd_fops = {
.owner =THIS_MODULE,
.ioctl =nbd_ioctl,
.compat_ioctl = nbd_ioctl,
+   .open = nbd_open,
+   .release =  nbd_release
 };
 
+
 static void nbd_ws_func_shutdown(struct work_struct *ws_nbd)
 {
struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device,
@@ -1107,6 +1164,7 @@ static int __init nbd_init(void)
disk->fops = _fops;
disk->private_data = _dev[i];
sprintf(disk->disk_name, "nbd%d", i);
+   atomic_set(_dev[i].inuse, 0);
nbd_reset(_dev[i]);
add_disk(disk);
}
-- 
2.6.2

[PATCH v2 4/5]nbd: make nbd device wait for its users.

[Pranay Kr. Srivastava]

When a timeout occurs or a recv fails, then
instead of abruplty killing nbd block device
wait for it's users to finish.

This is more required when filesystem(s) like
ext2 or ext3 don't expect their buffer heads to
disappear while the filesystem is mounted.

Each open of a nbd device is refcounted, while
the userland program [nbd-client] doing the
NBD_DO_IT ioctl would now wait for any other users
of this device before invalidating the nbd device.

Signed-off-by: Pranay Kr. Srivastava 
---
 drivers/block/nbd.c | 58 +
 1 file changed, 58 insertions(+)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index d1d898d..4da40dc 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -70,10 +70,13 @@ struct nbd_device {
 #if IS_ENABLED(CONFIG_DEBUG_FS)
struct dentry *dbg_dir;
 #endif
+   atomic_t inuse;
/*
 *This is specifically for calling sock_shutdown, for now.
 */
struct work_struct ws_shutdown;
+   struct kref users;
+   struct completion user_completion;
 };
 
 #if IS_ENABLED(CONFIG_DEBUG_FS)
@@ -104,6 +107,7 @@ static DEFINE_SPINLOCK(nbd_lock);
  * Shutdown function for nbd_dev work struct.
  */
 static void nbd_ws_func_shutdown(struct work_struct *);
+static void nbd_kref_release(struct kref *);
 
 static inline struct device *nbd_to_dev(struct nbd_device *nbd)
 {
@@ -682,6 +686,8 @@ static void nbd_reset(struct nbd_device *nbd)
nbd->flags = 0;
nbd->xmit_timeout = 0;
INIT_WORK(>ws_shutdown, nbd_ws_func_shutdown);
+   init_completion(>user_completion);
+   kref_init(>users);
queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue);
del_timer_sync(>timeout_timer);
 }
@@ -815,6 +821,14 @@ static int __nbd_ioctl(struct block_device *bdev, struct 
nbd_device *nbd,
kthread_stop(thread);
 
sock_shutdown(nbd);
+   /*
+* kref_init initializes with ref count as 1,
+* nbd_client, or the user-land program executing
+* this ioctl will make the refcount to 2[at least]
+* so subtracting 2 from refcount.
+*/
+   kref_sub(>users, 2, nbd_kref_release);
+   wait_for_completion(>user_completion);
mutex_lock(>tx_lock);
nbd_clear_que(nbd);
kill_bdev(bdev);
@@ -865,13 +879,56 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t 
mode,
 
return error;
 }
+static void nbd_kref_release(struct kref *kref_users)
+{
+   struct nbd_device *nbd = container_of(kref_users, struct nbd_device,
+   users);
+   pr_debug("Releasing kref [%s]\n", __func__);
+   atomic_set(>inuse, 0);
+   complete(>user_completion);
+
+}
+
+static int nbd_open(struct block_device *bdev, fmode_t mode)
+{
+   struct nbd_device *nbd_dev = bdev->bd_disk->private_data;
+
+   if (kref_get_unless_zero(_dev->users))
+   atomic_set(_dev->inuse, 1);
+
+   pr_debug("Opening nbd_dev %s. Active users = %u\n",
+   bdev->bd_disk->disk_name,
+   atomic_read(_dev->users.refcount) - 1);
+   return 0;
+}
+
+static void nbd_release(struct gendisk *disk, fmode_t mode)
+{
+   struct nbd_device *nbd_dev = disk->private_data;
+   /*
+   *kref_init initializes ref count to 1, so we
+   *we check for refcount to be 2 for a final put.
+   *
+   *kref needs to be re-initialized just here as the
+   *other process holding it must see the ref count as 2.
+   */
+   if (atomic_read(_dev->inuse))
+   kref_put(_dev->users,  nbd_kref_release);
+
+   pr_debug("Closing nbd_dev %s. Active users = %u\n",
+   disk->disk_name,
+   atomic_read(_dev->users.refcount) - 1);
+}
 
 static const struct block_device_operations nbd_fops = {
.owner =THIS_MODULE,
.ioctl =nbd_ioctl,
.compat_ioctl = nbd_ioctl,
+   .open = nbd_open,
+   .release =  nbd_release
 };
 
+
 static void nbd_ws_func_shutdown(struct work_struct *ws_nbd)
 {
struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device,
@@ -1107,6 +1164,7 @@ static int __init nbd_init(void)
disk->fops = _fops;
disk->private_data = _dev[i];
sprintf(disk->disk_name, "nbd%d", i);
+   atomic_set(_dev[i].inuse, 0);
nbd_reset(_dev[i]);
add_disk(disk);
}
-- 
2.6.2

[PATCH v2 5/5]nbd: use device_attr macros for sysfs attribute

[Pranay Kr. Srivastava]

This patch changes the pid sysfs device attribute to use
DEVICE_ATTR_* macro.

Signed-off-by: Pranay Kr. Srivastava 
---
 drivers/block/nbd.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 4da40dc..323ab26 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -449,10 +449,8 @@ static ssize_t pid_show(struct device *dev,
return sprintf(buf, "%d\n", task_pid_nr(nbd->task_recv));
 }
 
-static struct device_attribute pid_attr = {
-   .attr = { .name = "pid", .mode = S_IRUGO},
-   .show = pid_show,
-};
+
+static DEVICE_ATTR_RO(pid);
 
 static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev)
 {
@@ -465,7 +463,7 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct 
block_device *bdev)
 
nbd->task_recv = current;
 
-   ret = device_create_file(disk_to_dev(nbd->disk), _attr);
+   ret = device_create_file(disk_to_dev(nbd->disk), _attr_pid);
if (ret) {
dev_err(disk_to_dev(nbd->disk), "device_create_file failed!\n");
 
@@ -488,7 +486,7 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct 
block_device *bdev)
 
nbd_size_clear(nbd, bdev);
 
-   device_remove_file(disk_to_dev(nbd->disk), _attr);
+   device_remove_file(disk_to_dev(nbd->disk), _attr_pid);
 
nbd->task_recv = NULL;
 
-- 
2.6.2

[PATCH v2 2/5]nbd: cleanup nbd_set_socket

[Pranay Kr. Srivastava]

This patch
1) uses spin_lock instead of irq version.

2) removes the goto statement in case a socket
   is already assigned with simple if-else statement.

Signed-off-by: Pranay Kr. Srivastava 
---
 drivers/block/nbd.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 0339d40..da2b0a4 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -657,17 +657,14 @@ static int nbd_set_socket(struct nbd_device *nbd, struct 
socket *sock)
 {
int ret = 0;
 
-   spin_lock_irq(>sock_lock);
+   spin_lock(>sock_lock);
 
-   if (nbd->sock) {
+   if (nbd->sock)
ret = -EBUSY;
-   goto out;
-   }
-
-   nbd->sock = sock;
+   else
+   nbd->sock = sock;
 
-out:
-   spin_unlock_irq(>sock_lock);
+   spin_unlock(>sock_lock);
 
return ret;
 }
-- 
2.6.2

Re: [RESEND PATCH 0/8] clk: core: support clocks which requires parents on

[Dong Aisheng]

On Wed, Apr 20, 2016 at 05:34:32PM +0800, Dong Aisheng wrote:
> This is a RESEND patch series.
> The original one can be found at here:
> http://www.spinics.net/lists/arm-kernel/msg435136.html
> It has no functional changes but improve the commit message and
> comments a bit.
> 
> This patch series adds support in clock framework for clocks which all the
> operations like gate/ungate, set_rate, set_parent must require its parent
> clock on.
> 
> This special HW requirement can be found on Freescale i.MX7D platform.
> Besides i.MX7D, it seems lpc18xx clock has a similar requirement.
> (see: http://www.spinics.net/lists/arm-kernel/msg439307.html)
> And there may be other SoCs in the same situation.
> 
> Current clock core can not support such type of clock well.
> 
> This patch introduce a new flag CLK_SET_PARENT_ON to handle this special
> case in clock core that enable its parent clock firstly for each operation
> and disable it later after operation complete.
> 
> There're two special cases for handling this issue:
> 
> One is fixing the possible disabling clocks while its parent is already off
> during kernel booting phase in clk_disable_unused_subtree()
> Since this state misalign only happens during booting time,
> so we simply enable the parent clocks before disable it if flag is set.
> For normal clk_disable after kernel booting, there's no such issue
> since the clock tree is already created which makes sure no such issue exist.
> 
> Another special case is for set_parent() operation.
> It requires both parent, old one and new one, to be enabled at the same
> time during the operation.
> 
> Patch 1~4 does this work.
> 
> After clk core supports mx7d type clocks, we don't have to enable all clock
> by default anymore, then we remove the enable of all clocks code and instead
> only enable set of minimum required clocks.
> 
> Patch 5~8 does this work.
> 
> The whole patch series is based on for-next branch of clock tree:
> git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
> 

Hi Stephen,

Could you give some comments about this patch series?

Regards
Dong Aisheng

> Dong Aisheng (8):
>   clk: introduce clk_core_enable_lock and clk_core_disable_lock
> functions
>   clk: move clk_disable_unused after clk_core_disable_unprepare function
>   clk: core: support clocks which requires parents on (part 1)
>   clk: core: support clocks which requires parents on (part 2)
>   clk: imx: re-order and concentrate the same type of clk api
>   clk: imx: add clk api for supporting clk_OPS_PARENT_ON clocks
>   clk: imx7d: using api with flag CLK_OPS_PARENT_ON
>   clk: imx7d: only enable minimum required clocks
> 
>  drivers/clk/clk.c| 329 +++
>  drivers/clk/imx/clk-imx7d.c  | 732 
> ++-
>  drivers/clk/imx/clk.h|  90 --
>  include/linux/clk-provider.h |   2 +
>  4 files changed, 623 insertions(+), 530 deletions(-)
> 
> -- 
> 1.9.1
> 
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 5/5]nbd: use device_attr macros for sysfs attribute

[Pranay Kr. Srivastava]

This patch changes the pid sysfs device attribute to use
DEVICE_ATTR_* macro.

Signed-off-by: Pranay Kr. Srivastava 
---
 drivers/block/nbd.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 4da40dc..323ab26 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -449,10 +449,8 @@ static ssize_t pid_show(struct device *dev,
return sprintf(buf, "%d\n", task_pid_nr(nbd->task_recv));
 }
 
-static struct device_attribute pid_attr = {
-   .attr = { .name = "pid", .mode = S_IRUGO},
-   .show = pid_show,
-};
+
+static DEVICE_ATTR_RO(pid);
 
 static int nbd_thread_recv(struct nbd_device *nbd, struct block_device *bdev)
 {
@@ -465,7 +463,7 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct 
block_device *bdev)
 
nbd->task_recv = current;
 
-   ret = device_create_file(disk_to_dev(nbd->disk), _attr);
+   ret = device_create_file(disk_to_dev(nbd->disk), _attr_pid);
if (ret) {
dev_err(disk_to_dev(nbd->disk), "device_create_file failed!\n");
 
@@ -488,7 +486,7 @@ static int nbd_thread_recv(struct nbd_device *nbd, struct 
block_device *bdev)
 
nbd_size_clear(nbd, bdev);
 
-   device_remove_file(disk_to_dev(nbd->disk), _attr);
+   device_remove_file(disk_to_dev(nbd->disk), _attr_pid);
 
nbd->task_recv = NULL;
 
-- 
2.6.2

[PATCH v2 2/5]nbd: cleanup nbd_set_socket

[Pranay Kr. Srivastava]

This patch
1) uses spin_lock instead of irq version.

2) removes the goto statement in case a socket
   is already assigned with simple if-else statement.

Signed-off-by: Pranay Kr. Srivastava 
---
 drivers/block/nbd.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 0339d40..da2b0a4 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -657,17 +657,14 @@ static int nbd_set_socket(struct nbd_device *nbd, struct 
socket *sock)
 {
int ret = 0;
 
-   spin_lock_irq(>sock_lock);
+   spin_lock(>sock_lock);
 
-   if (nbd->sock) {
+   if (nbd->sock)
ret = -EBUSY;
-   goto out;
-   }
-
-   nbd->sock = sock;
+   else
+   nbd->sock = sock;
 
-out:
-   spin_unlock_irq(>sock_lock);
+   spin_unlock(>sock_lock);
 
return ret;
 }
-- 
2.6.2

Re: [RESEND PATCH 0/8] clk: core: support clocks which requires parents on

[Dong Aisheng]

On Wed, Apr 20, 2016 at 05:34:32PM +0800, Dong Aisheng wrote:
> This is a RESEND patch series.
> The original one can be found at here:
> http://www.spinics.net/lists/arm-kernel/msg435136.html
> It has no functional changes but improve the commit message and
> comments a bit.
> 
> This patch series adds support in clock framework for clocks which all the
> operations like gate/ungate, set_rate, set_parent must require its parent
> clock on.
> 
> This special HW requirement can be found on Freescale i.MX7D platform.
> Besides i.MX7D, it seems lpc18xx clock has a similar requirement.
> (see: http://www.spinics.net/lists/arm-kernel/msg439307.html)
> And there may be other SoCs in the same situation.
> 
> Current clock core can not support such type of clock well.
> 
> This patch introduce a new flag CLK_SET_PARENT_ON to handle this special
> case in clock core that enable its parent clock firstly for each operation
> and disable it later after operation complete.
> 
> There're two special cases for handling this issue:
> 
> One is fixing the possible disabling clocks while its parent is already off
> during kernel booting phase in clk_disable_unused_subtree()
> Since this state misalign only happens during booting time,
> so we simply enable the parent clocks before disable it if flag is set.
> For normal clk_disable after kernel booting, there's no such issue
> since the clock tree is already created which makes sure no such issue exist.
> 
> Another special case is for set_parent() operation.
> It requires both parent, old one and new one, to be enabled at the same
> time during the operation.
> 
> Patch 1~4 does this work.
> 
> After clk core supports mx7d type clocks, we don't have to enable all clock
> by default anymore, then we remove the enable of all clocks code and instead
> only enable set of minimum required clocks.
> 
> Patch 5~8 does this work.
> 
> The whole patch series is based on for-next branch of clock tree:
> git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
> 

Hi Stephen,

Could you give some comments about this patch series?

Regards
Dong Aisheng

> Dong Aisheng (8):
>   clk: introduce clk_core_enable_lock and clk_core_disable_lock
> functions
>   clk: move clk_disable_unused after clk_core_disable_unprepare function
>   clk: core: support clocks which requires parents on (part 1)
>   clk: core: support clocks which requires parents on (part 2)
>   clk: imx: re-order and concentrate the same type of clk api
>   clk: imx: add clk api for supporting clk_OPS_PARENT_ON clocks
>   clk: imx7d: using api with flag CLK_OPS_PARENT_ON
>   clk: imx7d: only enable minimum required clocks
> 
>  drivers/clk/clk.c| 329 +++
>  drivers/clk/imx/clk-imx7d.c  | 732 
> ++-
>  drivers/clk/imx/clk.h|  90 --
>  include/linux/clk-provider.h |   2 +
>  4 files changed, 623 insertions(+), 530 deletions(-)
> 
> -- 
> 1.9.1
> 
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/5]nbd: fix various coding standard warnings

[Pranay Kr. Srivastava]

1 )nbd: fix checkpatch trailing space warning.

2) nbd: fix checkpatch warning use linux/uaccess.h

3) nbd : fix checkpatch pointer declaration warning

4) nbd: fix checkpatch warning no newline after decleration.

5) nbd: fix checkpatch warning no newline after decleration.

6) nbd : fix checkpatch line over 80 char warning

7) nbd: fix checkpatch trailing whitespace warning.

8) nbd: fix checkpatch trailing whitespace warning.

9) nbd : fix checkpatch structure declaration braces on next line warning.

10) nbd : fix checkpatch trailing whitespace warning

11) nbd : fix checkpatch printk warning

12) nbd: fix checkpatch no extra line after decleration warning

13) nbd: fix checkpatch printk warning to pr_info

14) nbd: fix checkpatch no new line after decleration warning

15) nbd: fix checkpatch printk warning to pr_info

Signed-off-by: Pranay Kr. Srivastava 
---
 drivers/block/nbd.c | 29 -
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index da2b0a4..d1d898d 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -3,7 +3,7 @@
  *
  * Note that you can not swap over this thing, yet. Seems to work but
  * deadlocks sometimes - you can not swap over TCP in general.
- * 
+ *
  * Copyright 1997-2000, 2008 Pavel Machek 
  * Parts copyright 2001 Steven Whitehouse 
  *
@@ -35,7 +35,7 @@
 #include 
 #include 
 
-#include 
+#include 
 #include 
 
 #include 
@@ -43,7 +43,7 @@
 
 struct nbd_device {
u32 flags;
-   struct socket * sock;   /* If == NULL, device is not ready, yet */
+   struct socket *sock;/* If == NULL, device is not ready, yet */
int magic;
 
spinlock_t queue_lock;
@@ -272,6 +272,7 @@ static inline int sock_send_bvec(struct nbd_device *nbd, 
struct bio_vec *bvec,
 {
int result;
void *kaddr = kmap(bvec->bv_page);
+
result = sock_xmit(nbd, 1, kaddr + bvec->bv_offset,
   bvec->bv_len, flags);
kunmap(bvec->bv_page);
@@ -369,6 +370,7 @@ static inline int sock_recv_bvec(struct nbd_device *nbd, 
struct bio_vec *bvec)
 {
int result;
void *kaddr = kmap(bvec->bv_page);
+
result = sock_xmit(nbd, 0, kaddr + bvec->bv_offset, bvec->bv_len,
MSG_WAITALL);
kunmap(bvec->bv_page);
@@ -611,8 +613,8 @@ static int nbd_thread_send(void *data)
 }
 
 /*
- * We always wait for result of write, for now. It would be nice to make it 
optional
- * in future
+ * We always wait for result of write, for now. It would be nice to make it
+ * optional in future
  * if ((rq_data_dir(req) == WRITE) && (nbd->flags & NBD_WRITE_NOCHK))
  *   { printk( "Warning: Ignoring result!\n"); nbd_end_request( req ); }
  */
@@ -621,7 +623,7 @@ static void nbd_request_handler(struct request_queue *q)
__releases(q->queue_lock) __acquires(q->queue_lock)
 {
struct request *req;
-   
+
while ((req = blk_fetch_request(q)) != NULL) {
struct nbd_device *nbd;
 
@@ -737,7 +739,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct 
nbd_device *nbd,
nbd_send_req(nbd, );
return 0;
}
- 
+
case NBD_CLEAR_SOCK:
sock_shutdown(nbd);
nbd_clear_que(nbd);
@@ -864,8 +866,7 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t 
mode,
return error;
 }
 
-static const struct block_device_operations nbd_fops =
-{
+static const struct block_device_operations nbd_fops = {
.owner =THIS_MODULE,
.ioctl =nbd_ioctl,
.compat_ioctl = nbd_ioctl,
@@ -1008,7 +1009,7 @@ static void nbd_dbg_close(void)
 #endif
 
 /*
- * And here should be modules and kernel interface 
+ * And here should be modules and kernel interface
  *  (Just smiley confuses emacs :-)
  */
 
@@ -1021,7 +1022,7 @@ static int __init nbd_init(void)
BUILD_BUG_ON(sizeof(struct nbd_request) != 28);
 
if (max_part < 0) {
-   printk(KERN_ERR "nbd: max_part must be >= 0\n");
+   pr_err("nbd: max_part must be >= 0\n");
return -EINVAL;
}
 
@@ -1052,6 +1053,7 @@ static int __init nbd_init(void)
 
for (i = 0; i < nbds_max; i++) {
struct gendisk *disk = alloc_disk(1 << part_shift);
+
if (!disk)
goto out;
nbd_dev[i].disk = disk;
@@ -1082,12 +1084,13 @@ static int __init nbd_init(void)
goto out;
}
 
-   printk(KERN_INFO "nbd: registered device at major %d\n", NBD_MAJOR);
+   pr_info("nbd: registered device at major %d\n", NBD_MAJOR);
 
nbd_dbg_init();
 
for (i = 0; i < nbds_max; i++) {
struct gendisk *disk = nbd_dev[i].disk;
+
nbd_dev[i].magic = NBD_MAGIC;
INIT_LIST_HEAD(_dev[i].waiting_queue);

[PATCH v2 3/5]nbd: fix various coding standard warnings

[Pranay Kr. Srivastava]

1 )nbd: fix checkpatch trailing space warning.

2) nbd: fix checkpatch warning use linux/uaccess.h

3) nbd : fix checkpatch pointer declaration warning

4) nbd: fix checkpatch warning no newline after decleration.

5) nbd: fix checkpatch warning no newline after decleration.

6) nbd : fix checkpatch line over 80 char warning

7) nbd: fix checkpatch trailing whitespace warning.

8) nbd: fix checkpatch trailing whitespace warning.

9) nbd : fix checkpatch structure declaration braces on next line warning.

10) nbd : fix checkpatch trailing whitespace warning

11) nbd : fix checkpatch printk warning

12) nbd: fix checkpatch no extra line after decleration warning

13) nbd: fix checkpatch printk warning to pr_info

14) nbd: fix checkpatch no new line after decleration warning

15) nbd: fix checkpatch printk warning to pr_info

Signed-off-by: Pranay Kr. Srivastava 
---
 drivers/block/nbd.c | 29 -
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index da2b0a4..d1d898d 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -3,7 +3,7 @@
  *
  * Note that you can not swap over this thing, yet. Seems to work but
  * deadlocks sometimes - you can not swap over TCP in general.
- * 
+ *
  * Copyright 1997-2000, 2008 Pavel Machek 
  * Parts copyright 2001 Steven Whitehouse 
  *
@@ -35,7 +35,7 @@
 #include 
 #include 
 
-#include 
+#include 
 #include 
 
 #include 
@@ -43,7 +43,7 @@
 
 struct nbd_device {
u32 flags;
-   struct socket * sock;   /* If == NULL, device is not ready, yet */
+   struct socket *sock;/* If == NULL, device is not ready, yet */
int magic;
 
spinlock_t queue_lock;
@@ -272,6 +272,7 @@ static inline int sock_send_bvec(struct nbd_device *nbd, 
struct bio_vec *bvec,
 {
int result;
void *kaddr = kmap(bvec->bv_page);
+
result = sock_xmit(nbd, 1, kaddr + bvec->bv_offset,
   bvec->bv_len, flags);
kunmap(bvec->bv_page);
@@ -369,6 +370,7 @@ static inline int sock_recv_bvec(struct nbd_device *nbd, 
struct bio_vec *bvec)
 {
int result;
void *kaddr = kmap(bvec->bv_page);
+
result = sock_xmit(nbd, 0, kaddr + bvec->bv_offset, bvec->bv_len,
MSG_WAITALL);
kunmap(bvec->bv_page);
@@ -611,8 +613,8 @@ static int nbd_thread_send(void *data)
 }
 
 /*
- * We always wait for result of write, for now. It would be nice to make it 
optional
- * in future
+ * We always wait for result of write, for now. It would be nice to make it
+ * optional in future
  * if ((rq_data_dir(req) == WRITE) && (nbd->flags & NBD_WRITE_NOCHK))
  *   { printk( "Warning: Ignoring result!\n"); nbd_end_request( req ); }
  */
@@ -621,7 +623,7 @@ static void nbd_request_handler(struct request_queue *q)
__releases(q->queue_lock) __acquires(q->queue_lock)
 {
struct request *req;
-   
+
while ((req = blk_fetch_request(q)) != NULL) {
struct nbd_device *nbd;
 
@@ -737,7 +739,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct 
nbd_device *nbd,
nbd_send_req(nbd, );
return 0;
}
- 
+
case NBD_CLEAR_SOCK:
sock_shutdown(nbd);
nbd_clear_que(nbd);
@@ -864,8 +866,7 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t 
mode,
return error;
 }
 
-static const struct block_device_operations nbd_fops =
-{
+static const struct block_device_operations nbd_fops = {
.owner =THIS_MODULE,
.ioctl =nbd_ioctl,
.compat_ioctl = nbd_ioctl,
@@ -1008,7 +1009,7 @@ static void nbd_dbg_close(void)
 #endif
 
 /*
- * And here should be modules and kernel interface 
+ * And here should be modules and kernel interface
  *  (Just smiley confuses emacs :-)
  */
 
@@ -1021,7 +1022,7 @@ static int __init nbd_init(void)
BUILD_BUG_ON(sizeof(struct nbd_request) != 28);
 
if (max_part < 0) {
-   printk(KERN_ERR "nbd: max_part must be >= 0\n");
+   pr_err("nbd: max_part must be >= 0\n");
return -EINVAL;
}
 
@@ -1052,6 +1053,7 @@ static int __init nbd_init(void)
 
for (i = 0; i < nbds_max; i++) {
struct gendisk *disk = alloc_disk(1 << part_shift);
+
if (!disk)
goto out;
nbd_dev[i].disk = disk;
@@ -1082,12 +1084,13 @@ static int __init nbd_init(void)
goto out;
}
 
-   printk(KERN_INFO "nbd: registered device at major %d\n", NBD_MAJOR);
+   pr_info("nbd: registered device at major %d\n", NBD_MAJOR);
 
nbd_dbg_init();
 
for (i = 0; i < nbds_max; i++) {
struct gendisk *disk = nbd_dev[i].disk;
+
nbd_dev[i].magic = NBD_MAGIC;
INIT_LIST_HEAD(_dev[i].waiting_queue);
spin_lock_init(_dev[i].queue_lock);
@@ -1135,7 +1138,7 @@ 

Re: [PATCH v2 00/27] fb/drm: omapdss: Clean up the headers and separate the two stack

[Tony Lindgren]

* Tomi Valkeinen  [160602 05:28]:
> 
> Tony, can you have a look at the arch/arm parts here and give your ack
> if they're fine? They should be quite small and display specific, so I
> don't see much chance for conflict there.

Looks good to me, but these are going to conflict with the board-*.c
removal patches. And those we want to have easily revertable in case
of issues.

Peter, can you do an immutable minimal branch of just the pdata
changes affecting the board-*.c files? That way both Tomi and I
can merge that in.

Regards,

Tony

Re: [PATCH v2 00/27] fb/drm: omapdss: Clean up the headers and separate the two stack

[Tony Lindgren]

* Tomi Valkeinen  [160602 05:28]:
> 
> Tony, can you have a look at the arch/arm parts here and give your ack
> if they're fine? They should be quite small and display specific, so I
> don't see much chance for conflict there.

Looks good to me, but these are going to conflict with the board-*.c
removal patches. And those we want to have easily revertable in case
of issues.

Peter, can you do an immutable minimal branch of just the pdata
changes affecting the board-*.c files? That way both Tomi and I
can merge that in.

Regards,

Tony

Re: linux-next memleak after IO on dax mountpoint

[David Drysdale]

On Sat, May 28, 2016 at 5:05 AM, Xiong Zhou  wrote:
> On Fri, May 27, 2016 at 04:46:17PM +0800, Xiong Zhou wrote:
> ...
>> Still working on to id which commit in this merge causes this issuer,
>
> Narrowed down to:
>
> 37e5823 block: add offset in blk_add_request_payload()
> e048948 blk-mq: Export tagset iter function
> 58b4560 nvme: add helper nvme_map_len()
> 03b5929 nvme: rewrite discard support
> 8093f7c nvme: add helper nvme_setup_cmd()
> 21f033f NVMe: Skip async events for degraded controllers
> 82b4552 nvme: Use blk-mq helper for IO termination
> 93e9d8e block: add ability to flag write back caching on a device
> 519a7e1 dm: switch to using blk_queue_write_cache()
> bb8d261 nvme: introduce a controller state machine
> 92911a5 nvme: tighten up state check for namespace scanning
> 5955be2 nvme: move namespace scanning to core
> f866fc4 nvme: move AER handling to common code
> 0bf77e9 nvme: switch to RCU freeing the namespace
> 9082e87 block: remove struct bio_batch

FWIW, I'm also seeing kmemleak report a leak with v4.7-rc1, in
a different scenario (just normal desktop use).  Not done much
digging so far, but this commit (9082e87bf) looks like it might be
relevant -- lots of the following:

unreferenced object 0x8800c288e900 (size 256):
  comm "dconf-service", pid 1461, jiffies 4294895636 (age 48.028s)
  hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 c0 a4 c0 c6 00 88 ff ff  
02 20 00 20 00 00 00 00 11 00 00 00 00 00 00 00  . . 
  backtrace:
[] kmemleak_alloc+0x28/0x50
[] kmem_cache_alloc+0xfc/0x360
[] mempool_alloc_slab+0x15/0x20
[] mempool_alloc+0x6e/0x170
[] bio_alloc_bioset+0xb8/0x230
[] next_bio+0x24/0x50
[] blkdev_issue_zeroout+0xdf/0x1d0
[] ext4_issue_zeroout+0x39/0x50
[] ext4_ext_zeroout+0x2f/0x40
[] ext4_ext_map_blocks+0x1870/0x2190
[] ext4_map_blocks+0x111/0x620
[] ext4_writepages+0x7c8/0x10a0
[] do_writepages+0x21/0x30
[] __filemap_fdatawrite_range+0xaa/0xf0
[] filemap_write_and_wait_range+0x2d/0x70
[] ext4_sync_file+0x18d/0x500


> 38f2525 block: add __blkdev_issue_discard
> 57aac2f lightnvm: fix "warning: ‘ret’ may be used uninitialized"
> ecfb40c lightnvm: handle submit_io failure
> 1145e63 lightnvm: implement nvm_submit_ppa_list
> 22e8c97 lightnvm: move block fold outside of get_bb_tbl()
> 7f7c5d0 lightnvm: avoid memory leak when lun_map kcalloc fails
> 5136061 lightnvm: introduce nvm_for_each_lun_ppa() macro
> e11903f lightnvm: refactor device ops->get_bb_tbl()
> 5ebc7d9 lightnvm: make nvm_set_rqd_ppalist() aware of vblks
> a63d5cf lightnvm: move responsibility for bad blk mgmt to target
> 00ee6cc lightnvm: refactor set_bb_tbl for accepting ppa list
> 003fad3 lightnvm: enable metadata to be sent to device
> 04a8aa1 lightnvm: expose gennvm_mark_blk to targets
>
>
> These commits can not be reverted cleanly.

Re: linux-next memleak after IO on dax mountpoint

[David Drysdale]

On Sat, May 28, 2016 at 5:05 AM, Xiong Zhou  wrote:
> On Fri, May 27, 2016 at 04:46:17PM +0800, Xiong Zhou wrote:
> ...
>> Still working on to id which commit in this merge causes this issuer,
>
> Narrowed down to:
>
> 37e5823 block: add offset in blk_add_request_payload()
> e048948 blk-mq: Export tagset iter function
> 58b4560 nvme: add helper nvme_map_len()
> 03b5929 nvme: rewrite discard support
> 8093f7c nvme: add helper nvme_setup_cmd()
> 21f033f NVMe: Skip async events for degraded controllers
> 82b4552 nvme: Use blk-mq helper for IO termination
> 93e9d8e block: add ability to flag write back caching on a device
> 519a7e1 dm: switch to using blk_queue_write_cache()
> bb8d261 nvme: introduce a controller state machine
> 92911a5 nvme: tighten up state check for namespace scanning
> 5955be2 nvme: move namespace scanning to core
> f866fc4 nvme: move AER handling to common code
> 0bf77e9 nvme: switch to RCU freeing the namespace
> 9082e87 block: remove struct bio_batch

FWIW, I'm also seeing kmemleak report a leak with v4.7-rc1, in
a different scenario (just normal desktop use).  Not done much
digging so far, but this commit (9082e87bf) looks like it might be
relevant -- lots of the following:

unreferenced object 0x8800c288e900 (size 256):
  comm "dconf-service", pid 1461, jiffies 4294895636 (age 48.028s)
  hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 c0 a4 c0 c6 00 88 ff ff  
02 20 00 20 00 00 00 00 11 00 00 00 00 00 00 00  . . 
  backtrace:
[] kmemleak_alloc+0x28/0x50
[] kmem_cache_alloc+0xfc/0x360
[] mempool_alloc_slab+0x15/0x20
[] mempool_alloc+0x6e/0x170
[] bio_alloc_bioset+0xb8/0x230
[] next_bio+0x24/0x50
[] blkdev_issue_zeroout+0xdf/0x1d0
[] ext4_issue_zeroout+0x39/0x50
[] ext4_ext_zeroout+0x2f/0x40
[] ext4_ext_map_blocks+0x1870/0x2190
[] ext4_map_blocks+0x111/0x620
[] ext4_writepages+0x7c8/0x10a0
[] do_writepages+0x21/0x30
[] __filemap_fdatawrite_range+0xaa/0xf0
[] filemap_write_and_wait_range+0x2d/0x70
[] ext4_sync_file+0x18d/0x500


> 38f2525 block: add __blkdev_issue_discard
> 57aac2f lightnvm: fix "warning: ‘ret’ may be used uninitialized"
> ecfb40c lightnvm: handle submit_io failure
> 1145e63 lightnvm: implement nvm_submit_ppa_list
> 22e8c97 lightnvm: move block fold outside of get_bb_tbl()
> 7f7c5d0 lightnvm: avoid memory leak when lun_map kcalloc fails
> 5136061 lightnvm: introduce nvm_for_each_lun_ppa() macro
> e11903f lightnvm: refactor device ops->get_bb_tbl()
> 5ebc7d9 lightnvm: make nvm_set_rqd_ppalist() aware of vblks
> a63d5cf lightnvm: move responsibility for bad blk mgmt to target
> 00ee6cc lightnvm: refactor set_bb_tbl for accepting ppa list
> 003fad3 lightnvm: enable metadata to be sent to device
> 04a8aa1 lightnvm: expose gennvm_mark_blk to targets
>
>
> These commits can not be reverted cleanly.

Re: [PATCH] r8152: Add support for setting MAC to system's Auxiliary MAC address

[Greg KH]

On Thu, Jun 02, 2016 at 02:10:37AM +, mario_limoncie...@dell.com wrote:
> > -Original Message-
> > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > Sent: Wednesday, June 1, 2016 6:06 PM
> > To: Limonciello, Mario 
> > Cc: hayesw...@realtek.com; LKML ; Netdev
> > ; Linux USB ;
> > pali.ro...@gmail.com; anthony.w...@canonical.com
> > Subject: Re: [PATCH] r8152: Add support for setting MAC to system's 
> > Auxiliary
> > MAC address
> > 
> > On Wed, Jun 01, 2016 at 04:50:44PM -0500, Mario Limonciello wrote:
> > > Dell systems with Type-C ports have support for a persistent system
> > > specific MAC address when used with Dell Type-C docks and dongles.
> > > This means a dock plugged into two different systems will show
> > > different (but persistent) MAC addresses.  Dell Type-C docks and
> > > dongles use the
> > > r8152 driver.
> > >
> > > This information for the system's persistent MAC address is burned in
> > > when the HW is built and avilable under _SB\AMAC in the DSDT at runtime.
> > >
> > > More information about the technology is available here:
> > > http://www.dell.com/support/article/us/en/04/SLN301147
> > >
> > > Signed-off-by: Mario Limonciello 
> > > ---
> > >  drivers/net/usb/Kconfig |  1 +
> > >  drivers/net/usb/r8152.c | 37 +
> > >  2 files changed, 38 insertions(+)
> > >
> > > diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig index
> > > cdde590..c320930 100644
> > > --- a/drivers/net/usb/Kconfig
> > > +++ b/drivers/net/usb/Kconfig
> > > @@ -98,6 +98,7 @@ config USB_RTL8150
> > >  config USB_RTL8152
> > >   tristate "Realtek RTL8152/RTL8153 Based USB Ethernet Adapters"
> > >   select MII
> > > + depends on ACPI
> > >   help
> > > This option adds support for Realtek RTL8152 based USB 2.0
> > > 10/100 Ethernet adapters and RTL8153 based USB 3.0 10/100/1000
> > > diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index
> > > 3f9f6ed..62af3b4 100644
> > > --- a/drivers/net/usb/r8152.c
> > > +++ b/drivers/net/usb/r8152.c
> > > @@ -26,6 +26,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >
> > >  /* Information for net-next */
> > >  #define NETNEXT_VERSION  "08"
> > > @@ -1030,6 +1031,39 @@ out1:
> > >   return ret;
> > >  }
> > >
> > > +static u8 amac_ascii_to_hex(int c)
> > > +{
> > > + if (c <= 0x39)
> > > + return (u8)(c - 0x30);
> > > + else if (c <= 0x46)
> > > + return (u8)(c - 0x37);
> > > + return (u8)(c - 0x57);
> > > +}
> > 
> > We really don't have such a function somewhere in the kernel already?
> > 
> > And why 'int', isn't "c" really a u8?
> > 
> > > +static void set_auxiliary_addr(struct sockaddr *sa) {
> > > + acpi_status status;
> > > + acpi_handle handle;
> > > + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> > > + union acpi_object *obj;
> > > + int i;
> > > + char *ptr;
> > > +
> > > + acpi_get_handle(NULL, "\\_SB", );
> > > + status = acpi_evaluate_object(handle, "AMAC", NULL, );
> > 
> > Is this field in the ACPI standard, or should this only be "trusted" on a 
> > limited
> > number of machines (i.e. with Dell DMI strings?)
> > 
> 
> This isn't something part of ACPI - it's been added specifically for a
> selection of Dell machines.  

Ah, but isn't ACPI supposed to be a "standard"?  :)

And please wrap your email lines, there is a "standard" for that...

> I would rather not hardcode to the specific DMI model strings of those
> Dell machines as it's certainly going to be a feature that expands to
> more machines.  Since it is Dell specific though, if you would rather
> me just match to the sys vendor Dell Inc., that seems like a pretty
> good compromise to me.

You need to only do this on machines you "know" have this set to a
correct value, otherwise if some other random BIOS happens to set that
field to some random value, you will have problems.

> > And finally, this seems odd overall given that a MAC address should be
> > associated with the specific network device, not the overall system.
> 
> The whole reasoning behind this is to bring the behavior that E-Docks
> had to TBT docks.

What is "TBT"?

> With E-docks they were really just extensions of the pins for the
> already onboard NIC of the system.  When you docked in an E-dock you
> inherently would use the same MAC everywhere you went.  If you had two
> cubes your network admin would see your same MAC in both.
> 
> With TBT docks and this patch not present docking in different places
> will give you the MAC of the dock rather than something persistent
> that your network admin could identify.  Solving this was something
> that was explicitly asked for in case studies during conceptualization
> of these docks and is a feature present in the Realtek Windows driver.

But you are dealing with different "devices" here, 

Re: [PATCH] r8152: Add support for setting MAC to system's Auxiliary MAC address

[Greg KH]

On Thu, Jun 02, 2016 at 02:10:37AM +, mario_limoncie...@dell.com wrote:
> > -Original Message-
> > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > Sent: Wednesday, June 1, 2016 6:06 PM
> > To: Limonciello, Mario 
> > Cc: hayesw...@realtek.com; LKML ; Netdev
> > ; Linux USB ;
> > pali.ro...@gmail.com; anthony.w...@canonical.com
> > Subject: Re: [PATCH] r8152: Add support for setting MAC to system's 
> > Auxiliary
> > MAC address
> > 
> > On Wed, Jun 01, 2016 at 04:50:44PM -0500, Mario Limonciello wrote:
> > > Dell systems with Type-C ports have support for a persistent system
> > > specific MAC address when used with Dell Type-C docks and dongles.
> > > This means a dock plugged into two different systems will show
> > > different (but persistent) MAC addresses.  Dell Type-C docks and
> > > dongles use the
> > > r8152 driver.
> > >
> > > This information for the system's persistent MAC address is burned in
> > > when the HW is built and avilable under _SB\AMAC in the DSDT at runtime.
> > >
> > > More information about the technology is available here:
> > > http://www.dell.com/support/article/us/en/04/SLN301147
> > >
> > > Signed-off-by: Mario Limonciello 
> > > ---
> > >  drivers/net/usb/Kconfig |  1 +
> > >  drivers/net/usb/r8152.c | 37 +
> > >  2 files changed, 38 insertions(+)
> > >
> > > diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig index
> > > cdde590..c320930 100644
> > > --- a/drivers/net/usb/Kconfig
> > > +++ b/drivers/net/usb/Kconfig
> > > @@ -98,6 +98,7 @@ config USB_RTL8150
> > >  config USB_RTL8152
> > >   tristate "Realtek RTL8152/RTL8153 Based USB Ethernet Adapters"
> > >   select MII
> > > + depends on ACPI
> > >   help
> > > This option adds support for Realtek RTL8152 based USB 2.0
> > > 10/100 Ethernet adapters and RTL8153 based USB 3.0 10/100/1000
> > > diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index
> > > 3f9f6ed..62af3b4 100644
> > > --- a/drivers/net/usb/r8152.c
> > > +++ b/drivers/net/usb/r8152.c
> > > @@ -26,6 +26,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >
> > >  /* Information for net-next */
> > >  #define NETNEXT_VERSION  "08"
> > > @@ -1030,6 +1031,39 @@ out1:
> > >   return ret;
> > >  }
> > >
> > > +static u8 amac_ascii_to_hex(int c)
> > > +{
> > > + if (c <= 0x39)
> > > + return (u8)(c - 0x30);
> > > + else if (c <= 0x46)
> > > + return (u8)(c - 0x37);
> > > + return (u8)(c - 0x57);
> > > +}
> > 
> > We really don't have such a function somewhere in the kernel already?
> > 
> > And why 'int', isn't "c" really a u8?
> > 
> > > +static void set_auxiliary_addr(struct sockaddr *sa) {
> > > + acpi_status status;
> > > + acpi_handle handle;
> > > + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> > > + union acpi_object *obj;
> > > + int i;
> > > + char *ptr;
> > > +
> > > + acpi_get_handle(NULL, "\\_SB", );
> > > + status = acpi_evaluate_object(handle, "AMAC", NULL, );
> > 
> > Is this field in the ACPI standard, or should this only be "trusted" on a 
> > limited
> > number of machines (i.e. with Dell DMI strings?)
> > 
> 
> This isn't something part of ACPI - it's been added specifically for a
> selection of Dell machines.  

Ah, but isn't ACPI supposed to be a "standard"?  :)

And please wrap your email lines, there is a "standard" for that...

> I would rather not hardcode to the specific DMI model strings of those
> Dell machines as it's certainly going to be a feature that expands to
> more machines.  Since it is Dell specific though, if you would rather
> me just match to the sys vendor Dell Inc., that seems like a pretty
> good compromise to me.

You need to only do this on machines you "know" have this set to a
correct value, otherwise if some other random BIOS happens to set that
field to some random value, you will have problems.

> > And finally, this seems odd overall given that a MAC address should be
> > associated with the specific network device, not the overall system.
> 
> The whole reasoning behind this is to bring the behavior that E-Docks
> had to TBT docks.

What is "TBT"?

> With E-docks they were really just extensions of the pins for the
> already onboard NIC of the system.  When you docked in an E-dock you
> inherently would use the same MAC everywhere you went.  If you had two
> cubes your network admin would see your same MAC in both.
> 
> With TBT docks and this patch not present docking in different places
> will give you the MAC of the dock rather than something persistent
> that your network admin could identify.  Solving this was something
> that was explicitly asked for in case studies during conceptualization
> of these docks and is a feature present in the Realtek Windows driver.

But you are dealing with different "devices" here, thunderbolt is a PCI
device, not a "pin pass-through", and to treat it differently like you
want to is going to cause confusion as well.

> > What 

Re: [RFC PATCH 1/3] pci, acpi: Match PCI config space accessors against platfrom specific ECAM quirks.

[Arnd Bergmann]

On Thursday, June 2, 2016 3:35:34 PM CEST Tomasz Nowicki wrote:
> On 02.06.2016 14:32, Arnd Bergmann wrote:
> > On Thursday, June 2, 2016 2:07:43 PM CEST Tomasz Nowicki wrote:
> >> On 02.06.2016 13:42, Arnd Bergmann wrote:
> >>> On Thursday, June 2, 2016 10:41:01 AM CEST Tomasz Nowicki wrote:
>  +struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root)
>  +{
>  +   int bus_num = root->secondary.start;
>  +   int domain = root->segment;
>  +   struct pci_cfg_fixup *f;
>  +
>  +   if (!mcfg_table)
>  +   return _generic_ecam_ops;
>  +
>  +   /*
>  +* Match against platform specific quirks and return 
>  corresponding
>  +* CAM ops.
>  +*
>  +* First match against PCI topology  then use OEM ID 
>  and
>  +* OEM revision from MCFG table standard header.
>  +*/
>  +   for (f = __start_acpi_mcfg_fixups; f < __end_acpi_mcfg_fixups; 
>  f++) {
>  +   if ((f->domain == domain || f->domain == 
>  PCI_MCFG_DOMAIN_ANY) &&
>  +   (f->bus_num == bus_num || f->bus_num == 
>  PCI_MCFG_BUS_ANY) &&
>  +   (!strncmp(f->oem_id, mcfg_table->header.oem_id,
>  + ACPI_OEM_ID_SIZE)) &&
>  +   (f->oem_revision == mcfg_table->header.oem_revision))
>  +   return f->ops;
>  +   }
>  +   /* No quirks, use ECAM */
>  +   return _generic_ecam_ops;
>  +}
>  +
> int pci_mcfg_lookup(struct acpi_pci_root *root)
> >>>
> >>> Can you explain the use of pci_ecam_ops instead of pci_ops here?
> >>>
> >>
> >> I wanted to get associated bus_shift and use it to setup configuration
> >> region properly before calling pci_ecam_create. Please see next patch.
> >>
> >
> > I see. It feels really odd to do it this way though, since having a
> > nonstandard bus_shift essentially means not using anything resembling
> > ECAM to start with.
> >
> > I realize that a lot of the host bridges are not ECAM, but because
> > of this, it would be more logical to have their own pci_ops instead
> > of pci_ecam_ops.
> 
> Well, we have bus_shift there to express bus shift differentiation. So I 
> would say we should change just structure name to prevent misunderstanding.

I'm not really convinced here. We use the bus_shift for two
completely different things in the end: for sizing the MMIO window
that gets mapped by ACPI and for the pci_ecam_map_bus() function
that isn't actually used for the typical fixups that override the
pci_ops.

I see now that this sneaks in an .init callback for the quirk
through the backdoor, by adding it to the pci_ecam_ops. I think
that is not good: if the idea is to have the config space access
be adapted to various quirks that is one thing, but if we actually
need a function to be called for the quirk we should do just that
and have it be obvious. That function can then override the
pci_ops.

Arnd

Re: [RFC PATCH 1/3] pci, acpi: Match PCI config space accessors against platfrom specific ECAM quirks.

[Arnd Bergmann]

On Thursday, June 2, 2016 3:35:34 PM CEST Tomasz Nowicki wrote:
> On 02.06.2016 14:32, Arnd Bergmann wrote:
> > On Thursday, June 2, 2016 2:07:43 PM CEST Tomasz Nowicki wrote:
> >> On 02.06.2016 13:42, Arnd Bergmann wrote:
> >>> On Thursday, June 2, 2016 10:41:01 AM CEST Tomasz Nowicki wrote:
>  +struct pci_ecam_ops *pci_mcfg_get_ops(struct acpi_pci_root *root)
>  +{
>  +   int bus_num = root->secondary.start;
>  +   int domain = root->segment;
>  +   struct pci_cfg_fixup *f;
>  +
>  +   if (!mcfg_table)
>  +   return _generic_ecam_ops;
>  +
>  +   /*
>  +* Match against platform specific quirks and return 
>  corresponding
>  +* CAM ops.
>  +*
>  +* First match against PCI topology  then use OEM ID 
>  and
>  +* OEM revision from MCFG table standard header.
>  +*/
>  +   for (f = __start_acpi_mcfg_fixups; f < __end_acpi_mcfg_fixups; 
>  f++) {
>  +   if ((f->domain == domain || f->domain == 
>  PCI_MCFG_DOMAIN_ANY) &&
>  +   (f->bus_num == bus_num || f->bus_num == 
>  PCI_MCFG_BUS_ANY) &&
>  +   (!strncmp(f->oem_id, mcfg_table->header.oem_id,
>  + ACPI_OEM_ID_SIZE)) &&
>  +   (f->oem_revision == mcfg_table->header.oem_revision))
>  +   return f->ops;
>  +   }
>  +   /* No quirks, use ECAM */
>  +   return _generic_ecam_ops;
>  +}
>  +
> int pci_mcfg_lookup(struct acpi_pci_root *root)
> >>>
> >>> Can you explain the use of pci_ecam_ops instead of pci_ops here?
> >>>
> >>
> >> I wanted to get associated bus_shift and use it to setup configuration
> >> region properly before calling pci_ecam_create. Please see next patch.
> >>
> >
> > I see. It feels really odd to do it this way though, since having a
> > nonstandard bus_shift essentially means not using anything resembling
> > ECAM to start with.
> >
> > I realize that a lot of the host bridges are not ECAM, but because
> > of this, it would be more logical to have their own pci_ops instead
> > of pci_ecam_ops.
> 
> Well, we have bus_shift there to express bus shift differentiation. So I 
> would say we should change just structure name to prevent misunderstanding.

I'm not really convinced here. We use the bus_shift for two
completely different things in the end: for sizing the MMIO window
that gets mapped by ACPI and for the pci_ecam_map_bus() function
that isn't actually used for the typical fixups that override the
pci_ops.

I see now that this sneaks in an .init callback for the quirk
through the backdoor, by adding it to the pci_ecam_ops. I think
that is not good: if the idea is to have the config space access
be adapted to various quirks that is one thing, but if we actually
need a function to be called for the quirk we should do just that
and have it be obvious. That function can then override the
pci_ops.

Arnd

[PATCH v1 2/2] ARM: dts: keystone: add interrupt property to PCI controller bindings

[Murali Karicheri]

Now that Keystone PCIe controller supports error interrupt handling
add interrupt property to PCI controller DT bindings to enable
error interrupt handling.

Signed-off-by: Murali Karicheri 
---
 - v1 - no change from initial version
 - applies to master v4.7-rcx at kernel.org git repo
 arch/arm/boot/dts/keystone-k2e.dtsi | 2 ++
 arch/arm/boot/dts/keystone.dtsi | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/keystone-k2e.dtsi 
b/arch/arm/boot/dts/keystone-k2e.dtsi
index 5374c9a..9a51b8c 100644
--- a/arch/arm/boot/dts/keystone-k2e.dtsi
+++ b/arch/arm/boot/dts/keystone-k2e.dtsi
@@ -104,6 +104,8 @@
num-lanes = <2>;
bus-range = <0x00 0xff>;
 
+   /* error interrupt */
+   interrupts = ;
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0 0 0 1 _intc1 0>, /* INT A */
diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index f627a1c..e23f46d 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -302,6 +302,8 @@
num-lanes = <2>;
bus-range = <0x00 0xff>;
 
+   /* error interrupt */
+   interrupts = ;
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0 0 0 1 _intc0 0>, /* INT A */
-- 
1.9.1

[PATCH v1 1/2] ARM: dts: keystone: remove bogus IO resource entry from PCI binding

[Murali Karicheri]

The PCI DT bindings contain a bogus entry for IO space which is not
supported on Keystone. The current bogus entry has an invalid size
and throws following error during boot.

[0.420713] keystone-pcie 21021000.pcie: error -22: failed to map
   resource [io  0x-0x40003fff]

So remove it from the dts. While at it also add a bus-range
value that eliminates following log at boot up.

[0.420659] No bus range found for /soc/pcie@2102, using [bus 00-ff]

Signed-off-by: Murali Karicheri 
---
 v1 - fixed subject to something more meaningful
 applies to master v4.7-rcx at kernel.org git repo
 arch/arm/boot/dts/keystone-k2e.dtsi | 5 +++--
 arch/arm/boot/dts/keystone.dtsi | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/keystone-k2e.dtsi 
b/arch/arm/boot/dts/keystone-k2e.dtsi
index 96b349f..5374c9a 100644
--- a/arch/arm/boot/dts/keystone-k2e.dtsi
+++ b/arch/arm/boot/dts/keystone-k2e.dtsi
@@ -96,12 +96,13 @@
#address-cells = <3>;
#size-cells = <2>;
reg =  <0x21021000 0x2000>, <0x2102 0x1000>, 
<0x02620128 4>;
-   ranges = <0x8100 0 0 0x2326 0x4000 0x4000
-   0x8200 0 0x6000 0x6000 0 
0x1000>;
+   ranges = <0x8200 0 0x6000 0x6000
+ 0 0x1000>;
 
status = "disabled";
device_type = "pci";
num-lanes = <2>;
+   bus-range = <0x00 0xff>;
 
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index e34b226..f627a1c 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -294,12 +294,13 @@
#address-cells = <3>;
#size-cells = <2>;
reg =  <0x21801000 0x2000>, <0x2180 0x1000>, 
<0x02620128 4>;
-   ranges = <0x8100 0 0 0x2325 0 0x4000
-   0x8200 0 0x5000 0x5000 0 
0x1000>;
+   ranges = <0x8200 0 0x5000 0x5000
+ 0 0x1000>;
 
status = "disabled";
device_type = "pci";
num-lanes = <2>;
+   bus-range = <0x00 0xff>;
 
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
-- 
1.9.1

[PATCH v1 2/2] ARM: dts: keystone: add interrupt property to PCI controller bindings

[Murali Karicheri]

Now that Keystone PCIe controller supports error interrupt handling
add interrupt property to PCI controller DT bindings to enable
error interrupt handling.

Signed-off-by: Murali Karicheri 
---
 - v1 - no change from initial version
 - applies to master v4.7-rcx at kernel.org git repo
 arch/arm/boot/dts/keystone-k2e.dtsi | 2 ++
 arch/arm/boot/dts/keystone.dtsi | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/keystone-k2e.dtsi 
b/arch/arm/boot/dts/keystone-k2e.dtsi
index 5374c9a..9a51b8c 100644
--- a/arch/arm/boot/dts/keystone-k2e.dtsi
+++ b/arch/arm/boot/dts/keystone-k2e.dtsi
@@ -104,6 +104,8 @@
num-lanes = <2>;
bus-range = <0x00 0xff>;
 
+   /* error interrupt */
+   interrupts = ;
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0 0 0 1 _intc1 0>, /* INT A */
diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index f627a1c..e23f46d 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -302,6 +302,8 @@
num-lanes = <2>;
bus-range = <0x00 0xff>;
 
+   /* error interrupt */
+   interrupts = ;
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0 0 0 1 _intc0 0>, /* INT A */
-- 
1.9.1

[PATCH v1 1/2] ARM: dts: keystone: remove bogus IO resource entry from PCI binding

[Murali Karicheri]

The PCI DT bindings contain a bogus entry for IO space which is not
supported on Keystone. The current bogus entry has an invalid size
and throws following error during boot.

[0.420713] keystone-pcie 21021000.pcie: error -22: failed to map
   resource [io  0x-0x40003fff]

So remove it from the dts. While at it also add a bus-range
value that eliminates following log at boot up.

[0.420659] No bus range found for /soc/pcie@2102, using [bus 00-ff]

Signed-off-by: Murali Karicheri 
---
 v1 - fixed subject to something more meaningful
 applies to master v4.7-rcx at kernel.org git repo
 arch/arm/boot/dts/keystone-k2e.dtsi | 5 +++--
 arch/arm/boot/dts/keystone.dtsi | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/keystone-k2e.dtsi 
b/arch/arm/boot/dts/keystone-k2e.dtsi
index 96b349f..5374c9a 100644
--- a/arch/arm/boot/dts/keystone-k2e.dtsi
+++ b/arch/arm/boot/dts/keystone-k2e.dtsi
@@ -96,12 +96,13 @@
#address-cells = <3>;
#size-cells = <2>;
reg =  <0x21021000 0x2000>, <0x2102 0x1000>, 
<0x02620128 4>;
-   ranges = <0x8100 0 0 0x2326 0x4000 0x4000
-   0x8200 0 0x6000 0x6000 0 
0x1000>;
+   ranges = <0x8200 0 0x6000 0x6000
+ 0 0x1000>;
 
status = "disabled";
device_type = "pci";
num-lanes = <2>;
+   bus-range = <0x00 0xff>;
 
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
index e34b226..f627a1c 100644
--- a/arch/arm/boot/dts/keystone.dtsi
+++ b/arch/arm/boot/dts/keystone.dtsi
@@ -294,12 +294,13 @@
#address-cells = <3>;
#size-cells = <2>;
reg =  <0x21801000 0x2000>, <0x2180 0x1000>, 
<0x02620128 4>;
-   ranges = <0x8100 0 0 0x2325 0 0x4000
-   0x8200 0 0x5000 0x5000 0 
0x1000>;
+   ranges = <0x8200 0 0x5000 0x5000
+ 0 0x1000>;
 
status = "disabled";
device_type = "pci";
num-lanes = <2>;
+   bus-range = <0x00 0xff>;
 
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
-- 
1.9.1

[PATCH] of: use pr_fmt prefix for all console printing

[Rob Herring]

Clean-up all the DT printk functions to use common pr_fmt prefix.

Some print statements such as kmalloc errors were redundant, so just
drop those.

Cc: Frank Rowand 
Cc: Pantelis Antoniou 
Signed-off-by: Rob Herring 
---
 drivers/of/address.c | 49 ++--
 drivers/of/base.c| 11 ++
 drivers/of/dynamic.c | 47 +-
 drivers/of/fdt.c | 12 +--
 drivers/of/fdt_address.c | 35 ---
 drivers/of/irq.c |  2 ++
 drivers/of/of_numa.c | 22 ++--
 drivers/of/of_pci.c  |  6 --
 drivers/of/of_reserved_mem.c | 22 ++--
 drivers/of/overlay.c | 43 +++---
 drivers/of/platform.c| 16 +++
 drivers/of/resolver.c|  2 ++
 12 files changed, 130 insertions(+), 137 deletions(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 0a553c0..02b2903 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -1,4 +1,6 @@
 
+#define pr_fmt(fmt)"OF: " fmt
+
 #include 
 #include 
 #include 
@@ -24,10 +26,10 @@ static int __of_address_to_resource(struct device_node *dev,
 #ifdef DEBUG
 static void of_dump_addr(const char *s, const __be32 *addr, int na)
 {
-   printk(KERN_DEBUG "%s", s);
+   pr_debug("%s", s);
while (na--)
-   printk(" %08x", be32_to_cpu(*(addr++)));
-   printk("\n");
+   pr_cont(" %08x", be32_to_cpu(*(addr++)));
+   pr_cont("\n");
 }
 #else
 static void of_dump_addr(const char *s, const __be32 *addr, int na) { }
@@ -68,7 +70,7 @@ static u64 of_bus_default_map(__be32 *addr, const __be32 
*range,
s  = of_read_number(range + na + pna, ns);
da = of_read_number(addr, na);
 
-   pr_debug("OF: default map, cp=%llx, s=%llx, da=%llx\n",
+   pr_debug("default map, cp=%llx, s=%llx, da=%llx\n",
 (unsigned long long)cp, (unsigned long long)s,
 (unsigned long long)da);
 
@@ -156,7 +158,7 @@ static u64 of_bus_pci_map(__be32 *addr, const __be32 
*range, int na, int ns,
s  = of_read_number(range + na + pna, ns);
da = of_read_number(addr + 1, na - 1);
 
-   pr_debug("OF: PCI map, cp=%llx, s=%llx, da=%llx\n",
+   pr_debug("PCI map, cp=%llx, s=%llx, da=%llx\n",
 (unsigned long long)cp, (unsigned long long)s,
 (unsigned long long)da);
 
@@ -381,7 +383,7 @@ static u64 of_bus_isa_map(__be32 *addr, const __be32 
*range, int na, int ns,
s  = of_read_number(range + na + pna, ns);
da = of_read_number(addr + 1, na - 1);
 
-   pr_debug("OF: ISA map, cp=%llx, s=%llx, da=%llx\n",
+   pr_debug("ISA map, cp=%llx, s=%llx, da=%llx\n",
 (unsigned long long)cp, (unsigned long long)s,
 (unsigned long long)da);
 
@@ -504,17 +506,17 @@ static int of_translate_one(struct device_node *parent, 
struct of_bus *bus,
 */
ranges = of_get_property(parent, rprop, );
if (ranges == NULL && !of_empty_ranges_quirk(parent)) {
-   pr_debug("OF: no ranges; cannot translate\n");
+   pr_debug("no ranges; cannot translate\n");
return 1;
}
if (ranges == NULL || rlen == 0) {
offset = of_read_number(addr, na);
memset(addr, 0, pna * 4);
-   pr_debug("OF: empty ranges; 1:1 translation\n");
+   pr_debug("empty ranges; 1:1 translation\n");
goto finish;
}
 
-   pr_debug("OF: walking ranges...\n");
+   pr_debug("walking ranges...\n");
 
/* Now walk through the ranges */
rlen /= 4;
@@ -525,14 +527,14 @@ static int of_translate_one(struct device_node *parent, 
struct of_bus *bus,
break;
}
if (offset == OF_BAD_ADDR) {
-   pr_debug("OF: not found !\n");
+   pr_debug("not found !\n");
return 1;
}
memcpy(addr, ranges + na, 4 * pna);
 
  finish:
-   of_dump_addr("OF: parent translation for:", addr, pna);
-   pr_debug("OF: with offset: %llx\n", (unsigned long long)offset);
+   of_dump_addr("parent translation for:", addr, pna);
+   pr_debug("with offset: %llx\n", (unsigned long long)offset);
 
/* Translate it into parent bus space */
return pbus->translate(addr, offset, pna);
@@ -557,7 +559,7 @@ static u64 __of_translate_address(struct device_node *dev,
int na, ns, pna, pns;
u64 result = OF_BAD_ADDR;
 
-   pr_debug("OF: ** translation for device %s **\n", 
of_node_full_name(dev));
+   pr_debug("** translation for device %s **\n", of_node_full_name(dev));
 
/* Increase refcount at current level */
of_node_get(dev);
@@ -571,14 +573,14 @@ static u64 

Re: [PATCH 09/32] bcm2837-rpi-3-b.dts for 32bit arm

[Gerd Hoffmann]

  Hi,

> > > > Reference to ../../../arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
> > > > directly in the Makefile?

Actually tried that, and to my surprise this worked fine for both "make
dtbs" and "make dtbs_install".

So we should just do that I guess ...

> > Yes, in theory.  No, in practice.  As far I know the rpi3 is the only
> > 64bit soc where a almost identical 32bit version exists, so running
> > 32bit kernels on a 64bit processor actually happens in practice and I
> > expect this to continue.  If you want create sdcard images which run on
> > any rpi variant this is pretty much the only reasonable way to do it.
> 
> I think the Allwinner A64 and the Samsung s5p6818 are other examples
> for this, where the initial run of boards all run 32-bit kernels
> for much of the same reasons. If users want to run a 32-bit distro
> on rpi-3 and on e.g. orange-pi, I don't see why they wouldn't also run
> the same binary on A64.

... and others can join the party on a case-by-case basis.

I still expect for the majority of arm64 boards it is not very useful,
so I don't think we should build all of them unconditionally.

cheers,
  Gerd

[PATCH] of: use pr_fmt prefix for all console printing

[Rob Herring]

Clean-up all the DT printk functions to use common pr_fmt prefix.

Some print statements such as kmalloc errors were redundant, so just
drop those.

Cc: Frank Rowand 
Cc: Pantelis Antoniou 
Signed-off-by: Rob Herring 
---
 drivers/of/address.c | 49 ++--
 drivers/of/base.c| 11 ++
 drivers/of/dynamic.c | 47 +-
 drivers/of/fdt.c | 12 +--
 drivers/of/fdt_address.c | 35 ---
 drivers/of/irq.c |  2 ++
 drivers/of/of_numa.c | 22 ++--
 drivers/of/of_pci.c  |  6 --
 drivers/of/of_reserved_mem.c | 22 ++--
 drivers/of/overlay.c | 43 +++---
 drivers/of/platform.c| 16 +++
 drivers/of/resolver.c|  2 ++
 12 files changed, 130 insertions(+), 137 deletions(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 0a553c0..02b2903 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -1,4 +1,6 @@
 
+#define pr_fmt(fmt)"OF: " fmt
+
 #include 
 #include 
 #include 
@@ -24,10 +26,10 @@ static int __of_address_to_resource(struct device_node *dev,
 #ifdef DEBUG
 static void of_dump_addr(const char *s, const __be32 *addr, int na)
 {
-   printk(KERN_DEBUG "%s", s);
+   pr_debug("%s", s);
while (na--)
-   printk(" %08x", be32_to_cpu(*(addr++)));
-   printk("\n");
+   pr_cont(" %08x", be32_to_cpu(*(addr++)));
+   pr_cont("\n");
 }
 #else
 static void of_dump_addr(const char *s, const __be32 *addr, int na) { }
@@ -68,7 +70,7 @@ static u64 of_bus_default_map(__be32 *addr, const __be32 
*range,
s  = of_read_number(range + na + pna, ns);
da = of_read_number(addr, na);
 
-   pr_debug("OF: default map, cp=%llx, s=%llx, da=%llx\n",
+   pr_debug("default map, cp=%llx, s=%llx, da=%llx\n",
 (unsigned long long)cp, (unsigned long long)s,
 (unsigned long long)da);
 
@@ -156,7 +158,7 @@ static u64 of_bus_pci_map(__be32 *addr, const __be32 
*range, int na, int ns,
s  = of_read_number(range + na + pna, ns);
da = of_read_number(addr + 1, na - 1);
 
-   pr_debug("OF: PCI map, cp=%llx, s=%llx, da=%llx\n",
+   pr_debug("PCI map, cp=%llx, s=%llx, da=%llx\n",
 (unsigned long long)cp, (unsigned long long)s,
 (unsigned long long)da);
 
@@ -381,7 +383,7 @@ static u64 of_bus_isa_map(__be32 *addr, const __be32 
*range, int na, int ns,
s  = of_read_number(range + na + pna, ns);
da = of_read_number(addr + 1, na - 1);
 
-   pr_debug("OF: ISA map, cp=%llx, s=%llx, da=%llx\n",
+   pr_debug("ISA map, cp=%llx, s=%llx, da=%llx\n",
 (unsigned long long)cp, (unsigned long long)s,
 (unsigned long long)da);
 
@@ -504,17 +506,17 @@ static int of_translate_one(struct device_node *parent, 
struct of_bus *bus,
 */
ranges = of_get_property(parent, rprop, );
if (ranges == NULL && !of_empty_ranges_quirk(parent)) {
-   pr_debug("OF: no ranges; cannot translate\n");
+   pr_debug("no ranges; cannot translate\n");
return 1;
}
if (ranges == NULL || rlen == 0) {
offset = of_read_number(addr, na);
memset(addr, 0, pna * 4);
-   pr_debug("OF: empty ranges; 1:1 translation\n");
+   pr_debug("empty ranges; 1:1 translation\n");
goto finish;
}
 
-   pr_debug("OF: walking ranges...\n");
+   pr_debug("walking ranges...\n");
 
/* Now walk through the ranges */
rlen /= 4;
@@ -525,14 +527,14 @@ static int of_translate_one(struct device_node *parent, 
struct of_bus *bus,
break;
}
if (offset == OF_BAD_ADDR) {
-   pr_debug("OF: not found !\n");
+   pr_debug("not found !\n");
return 1;
}
memcpy(addr, ranges + na, 4 * pna);
 
  finish:
-   of_dump_addr("OF: parent translation for:", addr, pna);
-   pr_debug("OF: with offset: %llx\n", (unsigned long long)offset);
+   of_dump_addr("parent translation for:", addr, pna);
+   pr_debug("with offset: %llx\n", (unsigned long long)offset);
 
/* Translate it into parent bus space */
return pbus->translate(addr, offset, pna);
@@ -557,7 +559,7 @@ static u64 __of_translate_address(struct device_node *dev,
int na, ns, pna, pns;
u64 result = OF_BAD_ADDR;
 
-   pr_debug("OF: ** translation for device %s **\n", 
of_node_full_name(dev));
+   pr_debug("** translation for device %s **\n", of_node_full_name(dev));
 
/* Increase refcount at current level */
of_node_get(dev);
@@ -571,14 +573,14 @@ static u64 __of_translate_address(struct device_node *dev,
/* Count address cells & copy 

Re: [PATCH 09/32] bcm2837-rpi-3-b.dts for 32bit arm

[Gerd Hoffmann]

  Hi,

> > > > Reference to ../../../arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts
> > > > directly in the Makefile?

Actually tried that, and to my surprise this worked fine for both "make
dtbs" and "make dtbs_install".

So we should just do that I guess ...

> > Yes, in theory.  No, in practice.  As far I know the rpi3 is the only
> > 64bit soc where a almost identical 32bit version exists, so running
> > 32bit kernels on a 64bit processor actually happens in practice and I
> > expect this to continue.  If you want create sdcard images which run on
> > any rpi variant this is pretty much the only reasonable way to do it.
> 
> I think the Allwinner A64 and the Samsung s5p6818 are other examples
> for this, where the initial run of boards all run 32-bit kernels
> for much of the same reasons. If users want to run a 32-bit distro
> on rpi-3 and on e.g. orange-pi, I don't see why they wouldn't also run
> the same binary on A64.

... and others can join the party on a case-by-case basis.

I still expect for the majority of arm64 boards it is not very useful,
so I don't think we should build all of them unconditionally.

cheers,
  Gerd

Re: [PATCH v2 0/4] *** rtl8712: Replace semaphores with mutex / completions ***

[Larry Finger]

On 06/02/2016 02:43 AM, Arnd Bergmann wrote:

On Thursday, June 2, 2016 9:54:06 AM CEST Binoy Jayan wrote:

Hi,

These are a set of patches [v2] which removes semaphores from:

drivers/staging/rtl8712

They build correctly (individually and as a whole).
NB: I have not tested this as I do not have the following hardware:

"RealTek RTL8712U (RTL8192SU) Wireless LAN NIC driver"

Rework on comments w.r.t. PATCH v1:

  - Removed wrapper functions _wait_completion, _down_sema and _enter_pwrlock
  - Updated changelog to explain use of mutex_lock instead of
mutex_lock_interruptible in [PATCH v2 4/4]



All four look great now,

Reviewed-by: Arnd Bergmann 



I agree.

Tested-by: Larry Finger 

Larry

Re: [PATCH v2 0/4] *** rtl8712: Replace semaphores with mutex / completions ***

[Larry Finger]

On 06/02/2016 02:43 AM, Arnd Bergmann wrote:

On Thursday, June 2, 2016 9:54:06 AM CEST Binoy Jayan wrote:

Hi,

These are a set of patches [v2] which removes semaphores from:

drivers/staging/rtl8712

They build correctly (individually and as a whole).
NB: I have not tested this as I do not have the following hardware:

"RealTek RTL8712U (RTL8192SU) Wireless LAN NIC driver"

Rework on comments w.r.t. PATCH v1:

  - Removed wrapper functions _wait_completion, _down_sema and _enter_pwrlock
  - Updated changelog to explain use of mutex_lock instead of
mutex_lock_interruptible in [PATCH v2 4/4]



All four look great now,

Reviewed-by: Arnd Bergmann 



I agree.

Tested-by: Larry Finger 

Larry

Re: [PATCH v3 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L

[Nick Dyer]

Hi Dmitry-

On 01/06/2016 19:17, Dmitry Torokhov wrote:
> On Wed, Jun 01, 2016 at 05:39:44PM +0100, Nick Dyer wrote:
>> This is a series of patches to add diagnostic data support to the Atmel
>> maXTouch driver. It's a rewrite of the previous implementation which output 
>> via
>> debugfs: it now uses a V4L2 device in a similar way to the sur40 driver.
>
> I do not have any objections other than some nits form the input side;
> majority of the review should come from V4L2 side here...

Thanks for the review. I've hopefully fixed the issues you raised and
pushed it to
https://github.com/ndyer/linux/commits/diagnostic-v4l-20160602

I will wait for the V4L2 folks to comment before posting a [PATCH v4]

cheers

Nick

Re: [PATCH v3 0/8] Input: atmel_mxt_ts - output raw touch diagnostic data via V4L

[Nick Dyer]

Hi Dmitry-

On 01/06/2016 19:17, Dmitry Torokhov wrote:
> On Wed, Jun 01, 2016 at 05:39:44PM +0100, Nick Dyer wrote:
>> This is a series of patches to add diagnostic data support to the Atmel
>> maXTouch driver. It's a rewrite of the previous implementation which output 
>> via
>> debugfs: it now uses a V4L2 device in a similar way to the sur40 driver.
>
> I do not have any objections other than some nits form the input side;
> majority of the review should come from V4L2 side here...

Thanks for the review. I've hopefully fixed the issues you raised and
pushed it to
https://github.com/ndyer/linux/commits/diagnostic-v4l-20160602

I will wait for the V4L2 folks to comment before posting a [PATCH v4]

cheers

Nick

Re: zone_reclaimable() leads to livelock in __alloc_pages_slowpath()

[Michal Hocko]

On Wed 01-06-16 23:38:30, Oleg Nesterov wrote:
> On 06/01, Michal Hocko wrote:
> >
> > On Wed 01-06-16 01:56:26, Oleg Nesterov wrote:
> > > On 05/31, Michal Hocko wrote:
> > > >
> > > > On Sun 29-05-16 23:25:40, Oleg Nesterov wrote:
> > > > >
> > > > > This single change in get_scan_count() under for_each_evictable_lru() 
> > > > > loop
> > > > >
> > > > >   -   size = lruvec_lru_size(lruvec, lru);
> > > > >   +   size = zone_page_state_snapshot(lruvec_zone(lruvec), 
> > > > > NR_LRU_BASE + lru);
> > > > >
> > > > > fixes the problem too.
> > > > >
> > > > > Without this change shrink*() continues to scan the LRU_ACTIVE_FILE 
> > > > > list
> > > > > while it is empty. LRU_INACTIVE_FILE is not empty (just a few pages) 
> > > > > but
> > > > > we do not even try to scan it, lruvec_lru_size() returns zero.
> > > >
> > > > OK, you seem to be really seeing a different issue than me.
> > >
> > > quite possibly, but
> > >
> > > > My debugging
> > > > patch was showing when nothing was really isolated from the LRU lists
> > > > (both for shrink_{in}active_list.
> > >
> > > in my debugging session too. LRU_ACTIVE_FILE was empty, so there is 
> > > nothing to
> > > isolate even if shrink_active_list() is (wrongly called) with nr_to_scan 
> > > != 0.
> > > LRU_INACTIVE_FILE is not empty but it is not scanned because nr_to_scan 
> > > == 0.
> > >
> > > But I am afraid I misunderstood you, and you meant something else.
> >
> > What I wanted to say is that my debugging hasn't shown a single case
> > when nothing would be isolated. Which seems to be the case for you.
> 
> Ah, got it, thanks. Yes, I see that there is no "nothing scanned" in
> oom-test.qcow_serial.log.gz from 
> http://marc.info/?l=linux-kernel=146417822608902
> you sent. I applied this patch and I do see "nothing scanned".
> 
> But, unlike you, I do not see the messages from free-pages... perhaps you
> have more active tasks. To remind, I tested this with the single user-space
> process, /bin/sh running with pid==1, then I did "while true; do ./oom; done".

Well, I was booting into a standard init which will have a couple of
processes. So yes this would make a slight difference.
 
> So of course I do not know if you see another issue or the same, but now I am
> wondering if the change in get_scan_count() above fixes the problem for you.

I have played with it but the interfering freed pages just ruined the
whole zone_reclaimable expectations.
 
> Probably not, but the fact you do not see "nothing scanned" can't prove this,
> it is possible that shrink_*_list() was not called because vm_stat == 0 but
> zone_reclaimable() sees the per-cpu counter. In this case 0db2cb8da89d can
> make a difference, but see below.
> 
> > > > But I am thinking whether we should simply revert 0db2cb8da89d ("mm,
> > > > vmscan: make zone_reclaimable_pages more precise") in 4.6 stable tree.
> > > > Does that help as well?
> > >
> > > I'll test this tomorrow,
> 
> So it doesn't help.

OK, so we at least know this is not a regression.

> > but even if it helps I am not sure... Yes, this
> > > way zone_reclaimable() and get_scan_count() will see the same numbers, but
> > > how this can help to make zone_reclaimable() == F at the end?
> >
> > It won't in some cases.
> 
> And unless I am notally confused  hit exactly this case.
> 
> > And that has been the case for ages so I do not
> > think we need any steps for the stable.
> 
> OK, agreed.
> 
> > What meant to address is a
> > potential regression caused by 0db2cb8da89d which would make this more
> > likely because of the mismatch
> 
> Again, I can be easily wrong, but I do not see how 0db2cb8da89d could make
> the things worse...
> 
> Unless both get_scan_count() and zone_reclaimable() use "snapshot" variant,
> we can't guarantee zone_reclaimable() becomes false. The fact that they see
> different numbers (after 0db2cb8da89d) doesn't really matter.
> 
> Anyway, this was already fixed, so lets forget it ;)

Yes, especially as this doesn't seem to be a regression.

Thanks for your effort anyway.
-- 
Michal Hocko
SUSE Labs

Re: zone_reclaimable() leads to livelock in __alloc_pages_slowpath()

[Michal Hocko]

On Wed 01-06-16 23:38:30, Oleg Nesterov wrote:
> On 06/01, Michal Hocko wrote:
> >
> > On Wed 01-06-16 01:56:26, Oleg Nesterov wrote:
> > > On 05/31, Michal Hocko wrote:
> > > >
> > > > On Sun 29-05-16 23:25:40, Oleg Nesterov wrote:
> > > > >
> > > > > This single change in get_scan_count() under for_each_evictable_lru() 
> > > > > loop
> > > > >
> > > > >   -   size = lruvec_lru_size(lruvec, lru);
> > > > >   +   size = zone_page_state_snapshot(lruvec_zone(lruvec), 
> > > > > NR_LRU_BASE + lru);
> > > > >
> > > > > fixes the problem too.
> > > > >
> > > > > Without this change shrink*() continues to scan the LRU_ACTIVE_FILE 
> > > > > list
> > > > > while it is empty. LRU_INACTIVE_FILE is not empty (just a few pages) 
> > > > > but
> > > > > we do not even try to scan it, lruvec_lru_size() returns zero.
> > > >
> > > > OK, you seem to be really seeing a different issue than me.
> > >
> > > quite possibly, but
> > >
> > > > My debugging
> > > > patch was showing when nothing was really isolated from the LRU lists
> > > > (both for shrink_{in}active_list.
> > >
> > > in my debugging session too. LRU_ACTIVE_FILE was empty, so there is 
> > > nothing to
> > > isolate even if shrink_active_list() is (wrongly called) with nr_to_scan 
> > > != 0.
> > > LRU_INACTIVE_FILE is not empty but it is not scanned because nr_to_scan 
> > > == 0.
> > >
> > > But I am afraid I misunderstood you, and you meant something else.
> >
> > What I wanted to say is that my debugging hasn't shown a single case
> > when nothing would be isolated. Which seems to be the case for you.
> 
> Ah, got it, thanks. Yes, I see that there is no "nothing scanned" in
> oom-test.qcow_serial.log.gz from 
> http://marc.info/?l=linux-kernel=146417822608902
> you sent. I applied this patch and I do see "nothing scanned".
> 
> But, unlike you, I do not see the messages from free-pages... perhaps you
> have more active tasks. To remind, I tested this with the single user-space
> process, /bin/sh running with pid==1, then I did "while true; do ./oom; done".

Well, I was booting into a standard init which will have a couple of
processes. So yes this would make a slight difference.
 
> So of course I do not know if you see another issue or the same, but now I am
> wondering if the change in get_scan_count() above fixes the problem for you.

I have played with it but the interfering freed pages just ruined the
whole zone_reclaimable expectations.
 
> Probably not, but the fact you do not see "nothing scanned" can't prove this,
> it is possible that shrink_*_list() was not called because vm_stat == 0 but
> zone_reclaimable() sees the per-cpu counter. In this case 0db2cb8da89d can
> make a difference, but see below.
> 
> > > > But I am thinking whether we should simply revert 0db2cb8da89d ("mm,
> > > > vmscan: make zone_reclaimable_pages more precise") in 4.6 stable tree.
> > > > Does that help as well?
> > >
> > > I'll test this tomorrow,
> 
> So it doesn't help.

OK, so we at least know this is not a regression.

> > but even if it helps I am not sure... Yes, this
> > > way zone_reclaimable() and get_scan_count() will see the same numbers, but
> > > how this can help to make zone_reclaimable() == F at the end?
> >
> > It won't in some cases.
> 
> And unless I am notally confused  hit exactly this case.
> 
> > And that has been the case for ages so I do not
> > think we need any steps for the stable.
> 
> OK, agreed.
> 
> > What meant to address is a
> > potential regression caused by 0db2cb8da89d which would make this more
> > likely because of the mismatch
> 
> Again, I can be easily wrong, but I do not see how 0db2cb8da89d could make
> the things worse...
> 
> Unless both get_scan_count() and zone_reclaimable() use "snapshot" variant,
> we can't guarantee zone_reclaimable() becomes false. The fact that they see
> different numbers (after 0db2cb8da89d) doesn't really matter.
> 
> Anyway, this was already fixed, so lets forget it ;)

Yes, especially as this doesn't seem to be a regression.

Thanks for your effort anyway.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 00/11] cpufreq: Keep policy->freq_table sorted

[Rafael J. Wysocki]

On Thu, Jun 2, 2016 at 4:19 PM, Viresh Kumar  wrote:
> Hi Rafael,
>
> This series fixes all cpufreq drivers that provide a 'target_index'
> callback or in other words, which provide a freq-table to cpufreq core,
> to make sure they *only* use the 'index' argument to ->target_index()
> with the policy->freq_table.
>
> This change allows us to remove the (duplicate) sorted-freq-table, which
> was added by following series:
>
> [PATCH V2 0/2] cpufreq: Use sorted frequency tables

Which I'm not going to apply.

If this series depended on that one, please rework it.

Thanks,
Rafael

Re: [PATCH 00/11] cpufreq: Keep policy->freq_table sorted

[Rafael J. Wysocki]

On Thu, Jun 2, 2016 at 4:19 PM, Viresh Kumar  wrote:
> Hi Rafael,
>
> This series fixes all cpufreq drivers that provide a 'target_index'
> callback or in other words, which provide a freq-table to cpufreq core,
> to make sure they *only* use the 'index' argument to ->target_index()
> with the policy->freq_table.
>
> This change allows us to remove the (duplicate) sorted-freq-table, which
> was added by following series:
>
> [PATCH V2 0/2] cpufreq: Use sorted frequency tables

Which I'm not going to apply.

If this series depended on that one, please rework it.

Thanks,
Rafael

Re: [PATCH -v4 5/7] locking, arch: Update spin_unlock_wait()

[Boqun Feng]

On Thu, Jun 02, 2016 at 04:44:24PM +0200, Peter Zijlstra wrote:
> On Thu, Jun 02, 2016 at 10:24:40PM +0800, Boqun Feng wrote:
> > On Thu, Jun 02, 2016 at 01:52:02PM +0200, Peter Zijlstra wrote:
> > About spin_unlock_wait() on ppc, I actually have a fix pending review:
> > 
> > http://lkml.kernel.org/r/1461130033-70898-1-git-send-email-boqun.f...@gmail.com
> 
> Please use the normal commit quoting style:
> 
>   d86b8da04dfa ("arm64: spinlock: serialise spin_unlock_wait against 
> concurrent lockers")
> 

Good point ;-)

> > that patch fixed a different problem when people want to pair a
> > spin_unlock_wait() with a spin_lock().
> 
> Argh, indeed, and I think qspinlock is still broken there :/ But my poor
> brain is about to give in for the day.
> 
> Let me go ponder that some :/
> 

An intial thought of the fix is making queued_spin_unlock_wait() an
atomic-nop too:

static inline void queued_spin_unlock_wait(struct qspinlock *lock)
{
struct __qspinlock *l = (struct __qspinlock *)lock;

while (!cmpxchg(>locked, 0, 0))
cpu_relax();
}

This could make queued_spin_unlock_wait() a WRITE, with a smp_mb()
preceding it, it would act like a RELEASE, which can be paired with
spin_lock().

Just food for thought. ;-)

> > I think we still need that fix, and there are two conflicts with this
> > series:
> > 
> > 1.  arch_spin_unlock_wait() code for PPC32 was deleted, and
> > consolidated into one.
> 
> Nice.
> 
> > 2.  I actually downgraded spin_unlock_wait() to !ACQUIRE ;-)
> 
> Fail ;-)
> 
> > I can think of two ways to solve thoes conflicts:
> > 
> > 1.  Modify my patch to make spin_unlock_wait() an ACQUIRE, and it
> > can be merged in powerpc tree, and possible go into to mainline
> > before 4.7. Then there is no need for this series to have code
> > for ppc, therefore no conflict.
> 
> Hardly any other unlock_wait is an acquire, everyone is 'broken' :-/
> 
> > or
> > 
> > 2.  I can rebase my patch on this series, and it can be added in
> > this series, and will go into mainline at 4.8.
> > 
> > 
> > Michael and Peter, any thought?
> 
> I'm fine with it going in early, I can rebase, no problem.

OK, I will resend a new patch making spin_unlock_wait() align the
semantics in your series.

Regards,
Boqun


signature.asc
Description: PGP signature

RE: [PATCH] r8152: Add support for setting MAC to system's Auxiliary MAC address

[Mario_Limonciello]

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Thursday, June 2, 2016 10:01 AM
> To: Limonciello, Mario 
> Cc: pali.ro...@gmail.com; hayesw...@realtek.com; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org; linux-
> u...@vger.kernel.org; anthony.w...@canonical.com
> Subject: Re: [PATCH] r8152: Add support for setting MAC to system's
> Auxiliary MAC address
> 
> > >
> > > > +   pr_info("r8152: Using system auxiliary MAC address");
> > >
> > > It would be great to write also mac address into that pr_info
> 
> And since there could be multiple r8152 in the system, it would be good to
> indicate which of them is having its MAC changed. So
> netdev_info() or dev_info().
> 
>   Andrew

Thanks, will do that in next submission too.