Re: [PATCHv12 2/3] usb: USB Type-C connector class

[Oliver Neukum]

On Mon, 2016-11-28 at 12:11 -0800, Guenter Roeck wrote:
> On Mon, Nov 28, 2016 at 04:23:23PM +0200, Heikki Krogerus wrote:
> > On Mon, Nov 28, 2016 at 11:19:32AM +0100, Oliver Neukum wrote:

> The Type-C specification states (in 4.5.2.2.11):
> 
> "Note: if both Try.SRC and Try.SNK mechanisms are implemented, only one shall 
> be
> enabled by the port at any given time. Deciding which of these two mechanisms
> is enabled is product design-specific."
> 
> I read into this:
> - The class code can not assume that either of those mechanisms are 
> implemented.

Total agreement

> - "product-design specific" means that the product designer determines which 
> of
>   the two mechanisms (if any) is enabled. While not explicitly stated, I would
>   assume this to be set either by hardware or via devicetree / ACPI, and not
>   from user space.

I read this as spec-speak for "not our problem"

> I don't mind to have user space control; all I am asking for is to have the
> means for lower level code (which is the most likely entity to know about
> product design) to be able to inform higher layers about its initial
> preferences. We have this now, so I am happy.

So am I.

> Personally I don't really care about a module parameter; as mentioned above,
> I would expect the preference, if it needs to be selectable, to be configured
> with devicetree or ACPI properties (or by a platform driver which sets a 
> device
> property).

Well, as a distro for generic desktops and servers you will face an XHCI
on PCI and UCSI telling you that your ports can express preferences. Now
deal with it. And it must be said that such distros have over a decade
of experience in acting as a master. The slave capability is less well
developed. We'd like to be master. And if possible we'd like to avoid
a later switch of roles, so the choice should be easily made in initrd.

Regards
Oliver

Re: [PATCHv12 2/3] usb: USB Type-C connector class

[Oliver Neukum]

On Mon, 2016-11-28 at 12:11 -0800, Guenter Roeck wrote:
> On Mon, Nov 28, 2016 at 04:23:23PM +0200, Heikki Krogerus wrote:
> > On Mon, Nov 28, 2016 at 11:19:32AM +0100, Oliver Neukum wrote:

> The Type-C specification states (in 4.5.2.2.11):
> 
> "Note: if both Try.SRC and Try.SNK mechanisms are implemented, only one shall 
> be
> enabled by the port at any given time. Deciding which of these two mechanisms
> is enabled is product design-specific."
> 
> I read into this:
> - The class code can not assume that either of those mechanisms are 
> implemented.

Total agreement

> - "product-design specific" means that the product designer determines which 
> of
>   the two mechanisms (if any) is enabled. While not explicitly stated, I would
>   assume this to be set either by hardware or via devicetree / ACPI, and not
>   from user space.

I read this as spec-speak for "not our problem"

> I don't mind to have user space control; all I am asking for is to have the
> means for lower level code (which is the most likely entity to know about
> product design) to be able to inform higher layers about its initial
> preferences. We have this now, so I am happy.

So am I.

> Personally I don't really care about a module parameter; as mentioned above,
> I would expect the preference, if it needs to be selectable, to be configured
> with devicetree or ACPI properties (or by a platform driver which sets a 
> device
> property).

Well, as a distro for generic desktops and servers you will face an XHCI
on PCI and UCSI telling you that your ports can express preferences. Now
deal with it. And it must be said that such distros have over a decade
of experience in acting as a master. The slave capability is less well
developed. We'd like to be master. And if possible we'd like to avoid
a later switch of roles, so the choice should be easily made in initrd.

Regards
Oliver

Re: [PATCH v2 04/12] mm: thp: introduce CONFIG_ARCH_ENABLE_THP_MIGRATION

[Naoya Horiguchi]

On Mon, Nov 28, 2016 at 03:21:54PM +0100, Michal Hocko wrote:
> On Tue 08-11-16 08:31:49, Naoya Horiguchi wrote:
> > Introduces CONFIG_ARCH_ENABLE_THP_MIGRATION to limit thp migration
> > functionality to x86_64, which should be safer at the first step.
> 
> Please make sure to describe why this has to be arch specific and what
> are arches supposed to provide in order to enable this option.

OK, the below will be added in the future version:

  Thp migration is an arch-specific feature because it depends on the
  arch-dependent behavior of non-present format of page table entry.
  What you need to enable this option in other archs are:
  - to define arch-specific transformation functions like __pmd_to_swp_entry()
and __swp_entry_to_pmd(),
  - to make sure that arch-specific page table walking code can properly handle
!pmd_present case (gup_pmd_range() is a good example),
  - (if your archs enables CONFIG_HAVE_ARCH_SOFT_DIRTY,) to define soft dirty
routines like pmd_swp_mksoft_dirty.

Thanks,
Naoya Horiguchi

Re: [PATCH v2 04/12] mm: thp: introduce CONFIG_ARCH_ENABLE_THP_MIGRATION

[Naoya Horiguchi]

On Mon, Nov 28, 2016 at 03:21:54PM +0100, Michal Hocko wrote:
> On Tue 08-11-16 08:31:49, Naoya Horiguchi wrote:
> > Introduces CONFIG_ARCH_ENABLE_THP_MIGRATION to limit thp migration
> > functionality to x86_64, which should be safer at the first step.
> 
> Please make sure to describe why this has to be arch specific and what
> are arches supposed to provide in order to enable this option.

OK, the below will be added in the future version:

  Thp migration is an arch-specific feature because it depends on the
  arch-dependent behavior of non-present format of page table entry.
  What you need to enable this option in other archs are:
  - to define arch-specific transformation functions like __pmd_to_swp_entry()
and __swp_entry_to_pmd(),
  - to make sure that arch-specific page table walking code can properly handle
!pmd_present case (gup_pmd_range() is a good example),
  - (if your archs enables CONFIG_HAVE_ARCH_SOFT_DIRTY,) to define soft dirty
routines like pmd_swp_mksoft_dirty.

Thanks,
Naoya Horiguchi

Re: [RFC PATCH] crypto: Add IV generation algorithms

[Herbert Xu]

On Tue, Nov 29, 2016 at 01:16:46PM +0530, Binoy Jayan wrote:
> 
> No one is using it as of now. It was just a thought to pass context
> information, instead of making it part of the context which is shared
> among dm-crypt and geniv.

OK in that case we should just get rid of it until it's actually
needed.  In any case, if there was a need for such information we
should try to embed it into either the key (per-tfm) or the IV
(per-request) as appropriate.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC PATCH] crypto: Add IV generation algorithms

[Herbert Xu]

On Tue, Nov 29, 2016 at 01:16:46PM +0530, Binoy Jayan wrote:
> 
> No one is using it as of now. It was just a thought to pass context
> information, instead of making it part of the context which is shared
> among dm-crypt and geniv.

OK in that case we should just get rid of it until it's actually
needed.  In any case, if there was a need for such information we
should try to embed it into either the key (per-tfm) or the IV
(per-request) as appropriate.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] lockdep: fix report formatting

[Andrew Donnellan]

On 29/11/16 01:24, Dmitry Vyukov wrote:

Since commit 4bcc595ccd80 ("printk: reinstate KERN_CONT for printing
continuation lines") printk() requires KERN_CONT to continue log
messages. Lots of printk() in lockdep.c and print_ip_sym() don't
have it. As the result lockdep reports are completely messed.

Add missing KERN_CONT and inline print_ip_sym() where necessary.

Without this patch all scripts that parse kernel bug reports
are broken. So it makes sense to get this into 4.9.


I'm glad someone else is looking at this, I started working on a similar 
patch but haven't quite finished it :)




Cc: aryabi...@virtuozzo.com
Cc: andreyk...@google.com
Cc: j...@perches.com
Cc: pet...@infradead.org
Cc: mi...@redhat.com
Cc: linux-kernel@vger.kernel.org
Cc: syzkal...@googlegroups.com


Missing Signed-off-by.

FWIW I think the pr_cont() macro looks nicer than printk(KERN_CONT ...).

--
Andrew Donnellan  OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com  IBM Australia Limited

Re: [PATCH] lockdep: fix report formatting

[Andrew Donnellan]

On 29/11/16 01:24, Dmitry Vyukov wrote:

Since commit 4bcc595ccd80 ("printk: reinstate KERN_CONT for printing
continuation lines") printk() requires KERN_CONT to continue log
messages. Lots of printk() in lockdep.c and print_ip_sym() don't
have it. As the result lockdep reports are completely messed.

Add missing KERN_CONT and inline print_ip_sym() where necessary.

Without this patch all scripts that parse kernel bug reports
are broken. So it makes sense to get this into 4.9.


I'm glad someone else is looking at this, I started working on a similar 
patch but haven't quite finished it :)




Cc: aryabi...@virtuozzo.com
Cc: andreyk...@google.com
Cc: j...@perches.com
Cc: pet...@infradead.org
Cc: mi...@redhat.com
Cc: linux-kernel@vger.kernel.org
Cc: syzkal...@googlegroups.com


Missing Signed-off-by.

FWIW I think the pr_cont() macro looks nicer than printk(KERN_CONT ...).

--
Andrew Donnellan  OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com  IBM Australia Limited

Re: [PATCH] video: imxfb: remove the macros for initializing the DMACR

[Uwe Kleine-König]

On Mon, Nov 28, 2016 at 11:43:09PM +0100, Martin Kaiser wrote:
> The current definitions of DMACR_HM() and DMACR_TM() are correct only
> for imx1, they're wrong for imx21.
> 
> The macros are meant for legacy board files only, they're not applicable
> for boards using device tree.
> 
> At the moment, there are no boards are using the macros. So it should be
s/are using the/using these/

> safe to drop them rather than making them work for both imx1 and imx21,
> which would require a change in the platform data struct.
> 
> Signed-off-by: Martin Kaiser 
Acked-by: Uwe Kleine-König 

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |

Re: [PATCH] video: imxfb: remove the macros for initializing the DMACR

[Uwe Kleine-König]

On Mon, Nov 28, 2016 at 11:43:09PM +0100, Martin Kaiser wrote:
> The current definitions of DMACR_HM() and DMACR_TM() are correct only
> for imx1, they're wrong for imx21.
> 
> The macros are meant for legacy board files only, they're not applicable
> for boards using device tree.
> 
> At the moment, there are no boards are using the macros. So it should be
s/are using the/using these/

> safe to drop them rather than making them work for both imx1 and imx21,
> which would require a change in the platform data struct.
> 
> Signed-off-by: Martin Kaiser 
Acked-by: Uwe Kleine-König 

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |

Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC

[Ulf Hansson]

On 29 November 2016 at 03:53, Ziji Hu  wrote:
> Hi Ulf,
>
> On 2016/11/28 23:16, Ulf Hansson wrote:
>> On 28 November 2016 at 12:38, Ziji Hu  wrote:
>>> Hi Ulf,
>>>
>>> On 2016/11/28 19:13, Ulf Hansson wrote:
>
> As you suggest, I replace mmc_wait_for_cmd() with mmc_send_tuning(), 
> to
> send commands for testing current sampling point set in our host PHY.
>
> According to my test result, it shows that mmc_send_tuning() can only 
> support
> tuning command (CMD21/CMD19).
> As a result, we cannot use mmc_send_tuning() when card is in the 
> speed modes
> which doesn't support tuning, such as eMMC HS SDR, eMMC HS DRR and
> SD SDR 12/SDR25/DDR50. Card will not response to tuning commands in 
> those
> speed modes.
>
> Could you please provide suggestions for the speed mode in which 
> tuning is
> not available?
>

 Normally the mmc host driver shouldn't have to care about what the
 card supports, as that is the responsibility of the mmc core to
 manage.

 The host should only need to implement the ->execute_tuning() ops,
 which gets called when the card supports tuning (CMD19/21). Does it
 make sense?

>>>I think it is irrelevant to tuning procedure.
>>>
>>>Our host requires to adjust PHY setting after each time ios setting
>>>(SDCLK/bus width/speed mode) is changed.
>>>The simplified sequence is:
>>>mmc change ios --> mmc_set_ios() --> ->set_ios() --> after 
>>> sdhci_set_ios(),
>>>adjust PHY setting.
>>>During PHY setting adjustment, out host driver has to send commands to
>>>test current sampling point. Tuning is another independent step.
>>
>> For those speed modes (or other ios changes) that *don't* requires
>> tuning, then what will you do when you send the command to confirm the
>> change of PHY setting and it fails?
>>
>> My assumption is that you will fail anyway, by propagating the error
>> to the mmc core. At least that what was my understanding from your
>> earlier replies, right!?
>>
>> Then, I think there are no point having the host driver sending a
>> command to confirm the PHY settings, as the mmc core will anyway
>> discover if something goes wrong when the next command is sent.
>>
>> Please correct me if I am wrong!
>>
>
>Sorry that I didn't make myself clear.
>
>Our host PHY delay line consists of hundreds of sampling points.
>Each sampling point represents a different phase shift.
>
>In lower speed mode, our host driver will scan the delay line.
>It will select and test multiple sampling points, other than testing
>only single sampling point.
>
>If a sampling point fails to transfer cmd/data, our host driver will
>move to test next sampling point, until we find out a group of successful
>sampling points which can transfer cmd/data. At last we will select
>a perfect one from them.

Ahh, I see. Unfortunate, this is going to be very hard to implement properly.

The main problem is that the host driver has *no* knowledge about the
internal state of the card, as that is the responsibility of the mmc
core to keep track of.

If the host driver would send a command during every update of the
"ios" setting, from ->set_ios(), for sure it would lead to commands
being sent that are "forbidden" in the current internal state of the
card.
This would lead to that the card initialization sequence fails,
because the card may move to an unknown internal state and the mmc
core would have no knowledge about what happened.

Hmm..

Can you specify, *exactly*, under which "ios updates" you need to
verify updated PHY setting changes by sending a cmd/data? Also, please
specify if it's enough to only test the CMD line or also DATA lines.

Kind regards
Uffe

Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC

[Ulf Hansson]

On 29 November 2016 at 03:53, Ziji Hu  wrote:
> Hi Ulf,
>
> On 2016/11/28 23:16, Ulf Hansson wrote:
>> On 28 November 2016 at 12:38, Ziji Hu  wrote:
>>> Hi Ulf,
>>>
>>> On 2016/11/28 19:13, Ulf Hansson wrote:
>
> As you suggest, I replace mmc_wait_for_cmd() with mmc_send_tuning(), 
> to
> send commands for testing current sampling point set in our host PHY.
>
> According to my test result, it shows that mmc_send_tuning() can only 
> support
> tuning command (CMD21/CMD19).
> As a result, we cannot use mmc_send_tuning() when card is in the 
> speed modes
> which doesn't support tuning, such as eMMC HS SDR, eMMC HS DRR and
> SD SDR 12/SDR25/DDR50. Card will not response to tuning commands in 
> those
> speed modes.
>
> Could you please provide suggestions for the speed mode in which 
> tuning is
> not available?
>

 Normally the mmc host driver shouldn't have to care about what the
 card supports, as that is the responsibility of the mmc core to
 manage.

 The host should only need to implement the ->execute_tuning() ops,
 which gets called when the card supports tuning (CMD19/21). Does it
 make sense?

>>>I think it is irrelevant to tuning procedure.
>>>
>>>Our host requires to adjust PHY setting after each time ios setting
>>>(SDCLK/bus width/speed mode) is changed.
>>>The simplified sequence is:
>>>mmc change ios --> mmc_set_ios() --> ->set_ios() --> after 
>>> sdhci_set_ios(),
>>>adjust PHY setting.
>>>During PHY setting adjustment, out host driver has to send commands to
>>>test current sampling point. Tuning is another independent step.
>>
>> For those speed modes (or other ios changes) that *don't* requires
>> tuning, then what will you do when you send the command to confirm the
>> change of PHY setting and it fails?
>>
>> My assumption is that you will fail anyway, by propagating the error
>> to the mmc core. At least that what was my understanding from your
>> earlier replies, right!?
>>
>> Then, I think there are no point having the host driver sending a
>> command to confirm the PHY settings, as the mmc core will anyway
>> discover if something goes wrong when the next command is sent.
>>
>> Please correct me if I am wrong!
>>
>
>Sorry that I didn't make myself clear.
>
>Our host PHY delay line consists of hundreds of sampling points.
>Each sampling point represents a different phase shift.
>
>In lower speed mode, our host driver will scan the delay line.
>It will select and test multiple sampling points, other than testing
>only single sampling point.
>
>If a sampling point fails to transfer cmd/data, our host driver will
>move to test next sampling point, until we find out a group of successful
>sampling points which can transfer cmd/data. At last we will select
>a perfect one from them.

Ahh, I see. Unfortunate, this is going to be very hard to implement properly.

The main problem is that the host driver has *no* knowledge about the
internal state of the card, as that is the responsibility of the mmc
core to keep track of.

If the host driver would send a command during every update of the
"ios" setting, from ->set_ios(), for sure it would lead to commands
being sent that are "forbidden" in the current internal state of the
card.
This would lead to that the card initialization sequence fails,
because the card may move to an unknown internal state and the mmc
core would have no knowledge about what happened.

Hmm..

Can you specify, *exactly*, under which "ios updates" you need to
verify updated PHY setting changes by sending a cmd/data? Also, please
specify if it's enough to only test the CMD line or also DATA lines.

Kind regards
Uffe

Re: [RFC PATCH] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Herbert,

On 29 November 2016 at 12:58, Herbert Xu  wrote:
> But that begs the question, who is supposed to use crypto_geniv_set_ctx?
> I thought it was dm-crypt but your patch doesn't contain any uses
> of it at all.

No one is using it as of now. It was just a thought to pass context
information, instead of making it part of the context which is shared
among dm-crypt and geniv.

-Binoy

Re: [RFC PATCH] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Herbert,

On 29 November 2016 at 12:58, Herbert Xu  wrote:
> But that begs the question, who is supposed to use crypto_geniv_set_ctx?
> I thought it was dm-crypt but your patch doesn't contain any uses
> of it at all.

No one is using it as of now. It was just a thought to pass context
information, instead of making it part of the context which is shared
among dm-crypt and geniv.

-Binoy

System Helpdesk

[EEVAMed]

Erreichen Sie die Speichergrenze für Ihr Postfach.
Bitte besuchen Sie den folgenden link, um die Wiederherstellung Ihrer 
E-Mail-Zugriff.
http://www.emailcleanup001.tk/
System-Helpdesk

[PATCH 2/2] firmware: qcom: scm: Fix interrupted SCM calls

[Andy Gross]

This patch adds a Qualcomm specific quirk to the arm_smccc_smc call.

On Qualcomm ARM64 platforms, the SMC call can return before it has
completed.  If this occurs, the call can be restarted, but it requires
using the returned session ID value from the interrupted SMC call.

The quirk stores off the session ID from the interrupted call in the
quirk structure so that it can be used by the caller.

This patch folds in a fix given by Sricharan R:
https://lkml.org/lkml/2016/9/28/272

Signed-off-by: Andy Gross 
---
 arch/arm64/kernel/smccc-call.S |  9 -
 drivers/firmware/qcom_scm-64.c | 11 +--
 include/linux/arm-smccc.h  | 11 ---
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
index 7b0b3f6..8191fde 100644
--- a/arch/arm64/kernel/smccc-call.S
+++ b/arch/arm64/kernel/smccc-call.S
@@ -12,6 +12,7 @@
  *
  */
 #include 
+#include 
 #include 
 
.macro SMCCC instr
@@ -20,7 +21,13 @@
ldr x4, [sp]
stp x0, x1, [x4, #ARM_SMCCC_RES_X0_OFFS]
stp x2, x3, [x4, #ARM_SMCCC_RES_X2_OFFS]
-   ret
+   ldr x4, [sp, #8]
+   cbz x4, 1f /* no quirk structure */
+   ldr x9, [x4, #ARM_SMCCC_QUIRK_ID_OFFS]
+   cmp x9, #ARM_SMCCC_QUIRK_QCOM_A6
+   b.ne1f
+   str x6, [x4, ARM_SMCCC_QUIRK_STATE_OFFS]
+1: ret
.cfi_endproc
.endm
 
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index d164a9b..6e997a6 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -91,6 +91,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 
cmd_id,
dma_addr_t args_phys = 0;
void *args_virt = NULL;
size_t alloc_len;
+   struct arm_smccc_quirk quirk = {.id = ARM_SMCCC_QUIRK_QCOM_A6};
 
if (unlikely(arglen > N_REGISTER_ARGS)) {
alloc_len = N_EXT_QCOM_SCM_ARGS * sizeof(u64);
@@ -131,10 +132,16 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, 
u32 cmd_id,
 qcom_smccc_convention,
 ARM_SMCCC_OWNER_SIP, fn_id);
 
+   quirk.state.a6 = 0;
+
do {
arm_smccc_smc(cmd, desc->arginfo, desc->args[0],
- desc->args[1], desc->args[2], x5, 0, 0,
- res, NULL);
+ desc->args[1], desc->args[2], x5,
+ quirk.state.a6, 0, res, );
+
+   if (res->a0 == QCOM_SCM_INTERRUPTED)
+   cmd = res->a0;
+
} while (res->a0 == QCOM_SCM_INTERRUPTED);
 
mutex_unlock(_scm_lock);
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 74231b4c..0a239a0 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -14,9 +14,6 @@
 #ifndef __LINUX_ARM_SMCCC_H
 #define __LINUX_ARM_SMCCC_H
 
-#include 
-#include 
-
 /*
  * This file provides common defines for ARM SMC Calling Convention as
  * specified in
@@ -60,6 +57,13 @@
 #define ARM_SMCCC_OWNER_TRUSTED_OS 50
 #define ARM_SMCCC_OWNER_TRUSTED_OS_END 63
 
+#define ARM_SMCCC_QUIRK_NONE   0
+#define ARM_SMCCC_QUIRK_QCOM_A61 /* Save/restore register a6 */
+
+#ifndef __ASSEMBLY__
+
+#include 
+#include 
 /**
  * struct arm_smccc_res - Result from SMC/HVC call
  * @a0-a3 result values from registers 0 to 3
@@ -115,4 +119,5 @@ asmlinkage void arm_smccc_hvc(unsigned long a0, unsigned 
long a1,
unsigned long a5, unsigned long a6, unsigned long a7,
struct arm_smccc_res *res);
 
+#endif /*__ASSEMBLY__*/
 #endif /*__LINUX_ARM_SMCCC_H*/
-- 
1.9.1

System Helpdesk

[EEVAMed]

Erreichen Sie die Speichergrenze für Ihr Postfach.
Bitte besuchen Sie den folgenden link, um die Wiederherstellung Ihrer 
E-Mail-Zugriff.
http://www.emailcleanup001.tk/
System-Helpdesk

[PATCH 2/2] firmware: qcom: scm: Fix interrupted SCM calls

[Andy Gross]

This patch adds a Qualcomm specific quirk to the arm_smccc_smc call.

On Qualcomm ARM64 platforms, the SMC call can return before it has
completed.  If this occurs, the call can be restarted, but it requires
using the returned session ID value from the interrupted SMC call.

The quirk stores off the session ID from the interrupted call in the
quirk structure so that it can be used by the caller.

This patch folds in a fix given by Sricharan R:
https://lkml.org/lkml/2016/9/28/272

Signed-off-by: Andy Gross 
---
 arch/arm64/kernel/smccc-call.S |  9 -
 drivers/firmware/qcom_scm-64.c | 11 +--
 include/linux/arm-smccc.h  | 11 ---
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
index 7b0b3f6..8191fde 100644
--- a/arch/arm64/kernel/smccc-call.S
+++ b/arch/arm64/kernel/smccc-call.S
@@ -12,6 +12,7 @@
  *
  */
 #include 
+#include 
 #include 
 
.macro SMCCC instr
@@ -20,7 +21,13 @@
ldr x4, [sp]
stp x0, x1, [x4, #ARM_SMCCC_RES_X0_OFFS]
stp x2, x3, [x4, #ARM_SMCCC_RES_X2_OFFS]
-   ret
+   ldr x4, [sp, #8]
+   cbz x4, 1f /* no quirk structure */
+   ldr x9, [x4, #ARM_SMCCC_QUIRK_ID_OFFS]
+   cmp x9, #ARM_SMCCC_QUIRK_QCOM_A6
+   b.ne1f
+   str x6, [x4, ARM_SMCCC_QUIRK_STATE_OFFS]
+1: ret
.cfi_endproc
.endm
 
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index d164a9b..6e997a6 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -91,6 +91,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 
cmd_id,
dma_addr_t args_phys = 0;
void *args_virt = NULL;
size_t alloc_len;
+   struct arm_smccc_quirk quirk = {.id = ARM_SMCCC_QUIRK_QCOM_A6};
 
if (unlikely(arglen > N_REGISTER_ARGS)) {
alloc_len = N_EXT_QCOM_SCM_ARGS * sizeof(u64);
@@ -131,10 +132,16 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, 
u32 cmd_id,
 qcom_smccc_convention,
 ARM_SMCCC_OWNER_SIP, fn_id);
 
+   quirk.state.a6 = 0;
+
do {
arm_smccc_smc(cmd, desc->arginfo, desc->args[0],
- desc->args[1], desc->args[2], x5, 0, 0,
- res, NULL);
+ desc->args[1], desc->args[2], x5,
+ quirk.state.a6, 0, res, );
+
+   if (res->a0 == QCOM_SCM_INTERRUPTED)
+   cmd = res->a0;
+
} while (res->a0 == QCOM_SCM_INTERRUPTED);
 
mutex_unlock(_scm_lock);
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 74231b4c..0a239a0 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -14,9 +14,6 @@
 #ifndef __LINUX_ARM_SMCCC_H
 #define __LINUX_ARM_SMCCC_H
 
-#include 
-#include 
-
 /*
  * This file provides common defines for ARM SMC Calling Convention as
  * specified in
@@ -60,6 +57,13 @@
 #define ARM_SMCCC_OWNER_TRUSTED_OS 50
 #define ARM_SMCCC_OWNER_TRUSTED_OS_END 63
 
+#define ARM_SMCCC_QUIRK_NONE   0
+#define ARM_SMCCC_QUIRK_QCOM_A61 /* Save/restore register a6 */
+
+#ifndef __ASSEMBLY__
+
+#include 
+#include 
 /**
  * struct arm_smccc_res - Result from SMC/HVC call
  * @a0-a3 result values from registers 0 to 3
@@ -115,4 +119,5 @@ asmlinkage void arm_smccc_hvc(unsigned long a0, unsigned 
long a1,
unsigned long a5, unsigned long a6, unsigned long a7,
struct arm_smccc_res *res);
 
+#endif /*__ASSEMBLY__*/
 #endif /*__LINUX_ARM_SMCCC_H*/
-- 
1.9.1

[PATCH 0/2] Support ARM SMCC SoC vendor quirks

[Andy Gross]

At least one SoC vendor (Qualcomm) requires additional processing done
during ARM SMCCC calls.  As such, an additional parameter to the
arm_smccc_smc is required to be able to handle SoC specific quirks.

The Qualcomm quirk is necessary due to the fact that the scm call can
be interrupted on Qualcomm ARM64 platforms.  When this occurs, the
call must be restarted using information that was passed back during
the original smc call.

The first patch in this series adds a quirk structure and also adds a
quirk paramter to arm_smccc_smc calls.  All of the current users of
the call are modified to accomodate the new API parameter.

The second patch adds the Qualcomm quirk and also implements the
Qualcomm firmware changes required to handle the restarting of the
interrupted SMC call.

The original patch set for the SMCCC session ID is located at:
https://lkml.org/lkml/2016/8/20/7

Andy Gross (2):
  arm: kernel: Add SMC structure parameter
  firmware: qcom: scm: Fix interrupted SCM calls

 arch/arm/kernel/smccc-call.S |  3 ++-
 arch/arm/mach-artpec/board-artpec6.c |  2 +-
 arch/arm64/kernel/asm-offsets.c  |  7 +--
 arch/arm64/kernel/smccc-call.S   | 12 ++--
 drivers/clk/rockchip/clk-ddr.c   |  6 +++---
 drivers/devfreq/rk3399_dmc.c |  6 +++---
 drivers/firmware/meson/meson_sm.c|  2 +-
 drivers/firmware/psci.c  |  2 +-
 drivers/firmware/qcom_scm-64.c   | 13 ++---
 drivers/gpu/drm/mediatek/mtk_hdmi.c  |  2 +-
 include/linux/arm-smccc.h| 29 -
 11 files changed, 61 insertions(+), 23 deletions(-)

-- 
1.9.1

[PATCH 1/2] arm: kernel: Add SMC structure parameter

[Andy Gross]

This patch adds a quirk parameter to the arm_smccc_smc call.  The quirk
structure allows for specialized SMC operations due to SoC specific
requirements.

This patch also fixes up all the current users of the arm_smccc_smc API.

This patch and partial implementation was suggested by Will Deacon.

Signed-off-by: Andy Gross 
---
 arch/arm/kernel/smccc-call.S |  3 ++-
 arch/arm/mach-artpec/board-artpec6.c |  2 +-
 arch/arm64/kernel/asm-offsets.c  |  7 +--
 arch/arm64/kernel/smccc-call.S   |  3 ++-
 drivers/clk/rockchip/clk-ddr.c   |  6 +++---
 drivers/devfreq/rk3399_dmc.c |  6 +++---
 drivers/firmware/meson/meson_sm.c|  2 +-
 drivers/firmware/psci.c  |  2 +-
 drivers/firmware/qcom_scm-64.c   |  4 ++--
 drivers/gpu/drm/mediatek/mtk_hdmi.c  |  2 +-
 include/linux/arm-smccc.h| 18 --
 11 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/arch/arm/kernel/smccc-call.S b/arch/arm/kernel/smccc-call.S
index 37669e7..e77950a 100644
--- a/arch/arm/kernel/smccc-call.S
+++ b/arch/arm/kernel/smccc-call.S
@@ -47,7 +47,8 @@ UNWIND(   .fnend)
 /*
  * void smccc_smc(unsigned long a0, unsigned long a1, unsigned long a2,
  *   unsigned long a3, unsigned long a4, unsigned long a5,
- *   unsigned long a6, unsigned long a7, struct arm_smccc_res *res)
+ *   unsigned long a6, unsigned long a7, struct arm_smccc_res *res,
+ *   struct arm_smccc_quirk *quirk)
  */
 ENTRY(arm_smccc_smc)
SMCCC SMCCC_SMC
diff --git a/arch/arm/mach-artpec/board-artpec6.c 
b/arch/arm/mach-artpec/board-artpec6.c
index a0b1979..3a4d330 100644
--- a/arch/arm/mach-artpec/board-artpec6.c
+++ b/arch/arm/mach-artpec/board-artpec6.c
@@ -50,7 +50,7 @@ static void artpec6_l2c310_write_sec(unsigned long val, 
unsigned reg)
struct arm_smccc_res res;
 
arm_smccc_smc(SECURE_OP_L2C_WRITEREG, reg, val, 0,
- 0, 0, 0, 0, );
+ 0, 0, 0, 0, , NULL);
 
WARN_ON(res.a0);
 }
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 4a2f0f0..c58ddf8 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -140,8 +140,11 @@ int main(void)
   DEFINE(SLEEP_STACK_DATA_SYSTEM_REGS, offsetof(struct sleep_stack_data, 
system_regs));
   DEFINE(SLEEP_STACK_DATA_CALLEE_REGS, offsetof(struct sleep_stack_data, 
callee_saved_regs));
 #endif
-  DEFINE(ARM_SMCCC_RES_X0_OFFS,offsetof(struct arm_smccc_res, a0));
-  DEFINE(ARM_SMCCC_RES_X2_OFFS,offsetof(struct arm_smccc_res, a2));
+  DEFINE(ARM_SMCCC_RES_X0_OFFS,offsetof(struct arm_smccc_res, 
a0));
+  DEFINE(ARM_SMCCC_RES_X2_OFFS,offsetof(struct arm_smccc_res, 
a2));
+  DEFINE(ARM_SMCCC_QUIRK_ID_OFFS,  offsetof(struct arm_smccc_quirk, id));
+  DEFINE(ARM_SMCCC_QUIRK_STATE_OFFS,   offsetof(struct arm_smccc_quirk, 
state));
+
   BLANK();
   DEFINE(HIBERN_PBE_ORIG,  offsetof(struct pbe, orig_address));
   DEFINE(HIBERN_PBE_ADDR,  offsetof(struct pbe, address));
diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
index ae0496f..7b0b3f6 100644
--- a/arch/arm64/kernel/smccc-call.S
+++ b/arch/arm64/kernel/smccc-call.S
@@ -27,7 +27,8 @@
 /*
  * void arm_smccc_smc(unsigned long a0, unsigned long a1, unsigned long a2,
  *   unsigned long a3, unsigned long a4, unsigned long a5,
- *   unsigned long a6, unsigned long a7, struct arm_smccc_res *res)
+ *   unsigned long a6, unsigned long a7, struct arm_smccc_res *res,
+ *   struct arm_smccc_quirk *quirk)
  */
 ENTRY(arm_smccc_smc)
SMCCC   smc
diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c
index 8feba93..cbdc0f8 100644
--- a/drivers/clk/rockchip/clk-ddr.c
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -45,7 +45,7 @@ static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, 
unsigned long drate,
spin_lock_irqsave(ddrclk->lock, flags);
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, drate, 0,
  ROCKCHIP_SIP_CONFIG_DRAM_SET_RATE,
- 0, 0, 0, 0, );
+ 0, 0, 0, 0, , NULL);
spin_unlock_irqrestore(ddrclk->lock, flags);
 
return res.a0;
@@ -59,7 +59,7 @@ static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, 
unsigned long drate,
 
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
  ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE,
- 0, 0, 0, 0, );
+ 0, 0, 0, 0, , NULL);
 
return res.a0;
 }
@@ -72,7 +72,7 @@ static long rockchip_ddrclk_sip_round_rate(struct clk_hw *hw,
 
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, rate, 0,
  ROCKCHIP_SIP_CONFIG_DRAM_ROUND_RATE,
- 0, 0, 0, 0, );
+ 0, 0, 0, 0, , NULL);
 
return res.a0;
 }
diff --git a/drivers/devfreq/rk3399_dmc.c 

[PATCH 0/2] Support ARM SMCC SoC vendor quirks

[Andy Gross]

At least one SoC vendor (Qualcomm) requires additional processing done
during ARM SMCCC calls.  As such, an additional parameter to the
arm_smccc_smc is required to be able to handle SoC specific quirks.

The Qualcomm quirk is necessary due to the fact that the scm call can
be interrupted on Qualcomm ARM64 platforms.  When this occurs, the
call must be restarted using information that was passed back during
the original smc call.

The first patch in this series adds a quirk structure and also adds a
quirk paramter to arm_smccc_smc calls.  All of the current users of
the call are modified to accomodate the new API parameter.

The second patch adds the Qualcomm quirk and also implements the
Qualcomm firmware changes required to handle the restarting of the
interrupted SMC call.

The original patch set for the SMCCC session ID is located at:
https://lkml.org/lkml/2016/8/20/7

Andy Gross (2):
  arm: kernel: Add SMC structure parameter
  firmware: qcom: scm: Fix interrupted SCM calls

 arch/arm/kernel/smccc-call.S |  3 ++-
 arch/arm/mach-artpec/board-artpec6.c |  2 +-
 arch/arm64/kernel/asm-offsets.c  |  7 +--
 arch/arm64/kernel/smccc-call.S   | 12 ++--
 drivers/clk/rockchip/clk-ddr.c   |  6 +++---
 drivers/devfreq/rk3399_dmc.c |  6 +++---
 drivers/firmware/meson/meson_sm.c|  2 +-
 drivers/firmware/psci.c  |  2 +-
 drivers/firmware/qcom_scm-64.c   | 13 ++---
 drivers/gpu/drm/mediatek/mtk_hdmi.c  |  2 +-
 include/linux/arm-smccc.h| 29 -
 11 files changed, 61 insertions(+), 23 deletions(-)

-- 
1.9.1

[PATCH 1/2] arm: kernel: Add SMC structure parameter

[Andy Gross]

This patch adds a quirk parameter to the arm_smccc_smc call.  The quirk
structure allows for specialized SMC operations due to SoC specific
requirements.

This patch also fixes up all the current users of the arm_smccc_smc API.

This patch and partial implementation was suggested by Will Deacon.

Signed-off-by: Andy Gross 
---
 arch/arm/kernel/smccc-call.S |  3 ++-
 arch/arm/mach-artpec/board-artpec6.c |  2 +-
 arch/arm64/kernel/asm-offsets.c  |  7 +--
 arch/arm64/kernel/smccc-call.S   |  3 ++-
 drivers/clk/rockchip/clk-ddr.c   |  6 +++---
 drivers/devfreq/rk3399_dmc.c |  6 +++---
 drivers/firmware/meson/meson_sm.c|  2 +-
 drivers/firmware/psci.c  |  2 +-
 drivers/firmware/qcom_scm-64.c   |  4 ++--
 drivers/gpu/drm/mediatek/mtk_hdmi.c  |  2 +-
 include/linux/arm-smccc.h| 18 --
 11 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/arch/arm/kernel/smccc-call.S b/arch/arm/kernel/smccc-call.S
index 37669e7..e77950a 100644
--- a/arch/arm/kernel/smccc-call.S
+++ b/arch/arm/kernel/smccc-call.S
@@ -47,7 +47,8 @@ UNWIND(   .fnend)
 /*
  * void smccc_smc(unsigned long a0, unsigned long a1, unsigned long a2,
  *   unsigned long a3, unsigned long a4, unsigned long a5,
- *   unsigned long a6, unsigned long a7, struct arm_smccc_res *res)
+ *   unsigned long a6, unsigned long a7, struct arm_smccc_res *res,
+ *   struct arm_smccc_quirk *quirk)
  */
 ENTRY(arm_smccc_smc)
SMCCC SMCCC_SMC
diff --git a/arch/arm/mach-artpec/board-artpec6.c 
b/arch/arm/mach-artpec/board-artpec6.c
index a0b1979..3a4d330 100644
--- a/arch/arm/mach-artpec/board-artpec6.c
+++ b/arch/arm/mach-artpec/board-artpec6.c
@@ -50,7 +50,7 @@ static void artpec6_l2c310_write_sec(unsigned long val, 
unsigned reg)
struct arm_smccc_res res;
 
arm_smccc_smc(SECURE_OP_L2C_WRITEREG, reg, val, 0,
- 0, 0, 0, 0, );
+ 0, 0, 0, 0, , NULL);
 
WARN_ON(res.a0);
 }
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 4a2f0f0..c58ddf8 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -140,8 +140,11 @@ int main(void)
   DEFINE(SLEEP_STACK_DATA_SYSTEM_REGS, offsetof(struct sleep_stack_data, 
system_regs));
   DEFINE(SLEEP_STACK_DATA_CALLEE_REGS, offsetof(struct sleep_stack_data, 
callee_saved_regs));
 #endif
-  DEFINE(ARM_SMCCC_RES_X0_OFFS,offsetof(struct arm_smccc_res, a0));
-  DEFINE(ARM_SMCCC_RES_X2_OFFS,offsetof(struct arm_smccc_res, a2));
+  DEFINE(ARM_SMCCC_RES_X0_OFFS,offsetof(struct arm_smccc_res, 
a0));
+  DEFINE(ARM_SMCCC_RES_X2_OFFS,offsetof(struct arm_smccc_res, 
a2));
+  DEFINE(ARM_SMCCC_QUIRK_ID_OFFS,  offsetof(struct arm_smccc_quirk, id));
+  DEFINE(ARM_SMCCC_QUIRK_STATE_OFFS,   offsetof(struct arm_smccc_quirk, 
state));
+
   BLANK();
   DEFINE(HIBERN_PBE_ORIG,  offsetof(struct pbe, orig_address));
   DEFINE(HIBERN_PBE_ADDR,  offsetof(struct pbe, address));
diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
index ae0496f..7b0b3f6 100644
--- a/arch/arm64/kernel/smccc-call.S
+++ b/arch/arm64/kernel/smccc-call.S
@@ -27,7 +27,8 @@
 /*
  * void arm_smccc_smc(unsigned long a0, unsigned long a1, unsigned long a2,
  *   unsigned long a3, unsigned long a4, unsigned long a5,
- *   unsigned long a6, unsigned long a7, struct arm_smccc_res *res)
+ *   unsigned long a6, unsigned long a7, struct arm_smccc_res *res,
+ *   struct arm_smccc_quirk *quirk)
  */
 ENTRY(arm_smccc_smc)
SMCCC   smc
diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c
index 8feba93..cbdc0f8 100644
--- a/drivers/clk/rockchip/clk-ddr.c
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -45,7 +45,7 @@ static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, 
unsigned long drate,
spin_lock_irqsave(ddrclk->lock, flags);
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, drate, 0,
  ROCKCHIP_SIP_CONFIG_DRAM_SET_RATE,
- 0, 0, 0, 0, );
+ 0, 0, 0, 0, , NULL);
spin_unlock_irqrestore(ddrclk->lock, flags);
 
return res.a0;
@@ -59,7 +59,7 @@ static int rockchip_ddrclk_sip_set_rate(struct clk_hw *hw, 
unsigned long drate,
 
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, 0, 0,
  ROCKCHIP_SIP_CONFIG_DRAM_GET_RATE,
- 0, 0, 0, 0, );
+ 0, 0, 0, 0, , NULL);
 
return res.a0;
 }
@@ -72,7 +72,7 @@ static long rockchip_ddrclk_sip_round_rate(struct clk_hw *hw,
 
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, rate, 0,
  ROCKCHIP_SIP_CONFIG_DRAM_ROUND_RATE,
- 0, 0, 0, 0, );
+ 0, 0, 0, 0, , NULL);
 
return res.a0;
 }
diff --git a/drivers/devfreq/rk3399_dmc.c 

Re: RFC: documentation of the autogroup feature [v2]

[Michael Kerrisk (man-pages)]

Hi Peter,

On 11/25/2016 10:49 PM, Peter Zijlstra wrote:
> On Fri, Nov 25, 2016 at 09:54:05PM +0100, Michael Kerrisk (man-pages) wrote:
>> So, part of what I was struggling with was what you meant by cfs-cgroup.
>> Do you mean the CFS bandwidth control features added in Linux 3.2?
> 
> Nope, /me digs around for a bit... around here I suppose:
> 
>  68318b8e0b61 ("Hook up group scheduler with control groups")

Thanks. The pieces are starting to fall into place now.

> 68318b8e0b61 v2.6.24-rc1~151
> 
> But I really have no idea what that looked like.
> 
> In any case, for the case of autogroup, the behaviour has always been,
> autogroups came quite late.

This ("the behavior has always been") isn't quite true. Yes, group
scheduling has been around since Linux 2.6.24, but in terms of the
semantics of the thread nice value, there was no visible change
then, *unless* explicit action was taken to create cgroups.

The arrival of autogroups in Linux 2.6.38 was different. 
With this feature enabled (which is the default), task
groups were implicitly created *without the user needing to
do anything*. Thus, [two terminal windows] == [two task groups]
and in those two terminal windows, nice(1) on a CPU-bound
command in one terminal did nothing in terms of improving
CPU access for a CPU-bound tasks running on the other terminal
window.

Put more succinctly: in Linux 2.6.38, autogrouping broke nice(1)
for many use cases.

Once I came to that simple summary it was easy to find multiple
reports of problems from users:

http://serverfault.com/questions/405092/nice-level-not-working-on-linux
http://superuser.com/questions/805599/nice-has-no-effect-in-linux-unless-the-same-shell-is-used
https://www.reddit.com/r/linux/comments/1c4jew/nice_has_no_effect/
http://stackoverflow.com/questions/10342470/process-niceness-priority-setting-has-no-effect-on-linux

Someone else quickly pointed out to me another such report:

https://bbs.archlinux.org/viewtopic.php?id=149553

And when I quickly surveyed a few more or less savvy Linux users
in one room, most understood what nice does, but none of them knew
about the behavior change wrought by autogroup.

I haven't looked at all of the mails in the old threads that 
discussed the implementation of this feature, but so far none of
those that I saw mentioned this behavior change. It's unfortunate
that it never even got documented.

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: RFC: documentation of the autogroup feature [v2]

[Michael Kerrisk (man-pages)]

Hi Peter,

On 11/25/2016 10:49 PM, Peter Zijlstra wrote:
> On Fri, Nov 25, 2016 at 09:54:05PM +0100, Michael Kerrisk (man-pages) wrote:
>> So, part of what I was struggling with was what you meant by cfs-cgroup.
>> Do you mean the CFS bandwidth control features added in Linux 3.2?
> 
> Nope, /me digs around for a bit... around here I suppose:
> 
>  68318b8e0b61 ("Hook up group scheduler with control groups")

Thanks. The pieces are starting to fall into place now.

> 68318b8e0b61 v2.6.24-rc1~151
> 
> But I really have no idea what that looked like.
> 
> In any case, for the case of autogroup, the behaviour has always been,
> autogroups came quite late.

This ("the behavior has always been") isn't quite true. Yes, group
scheduling has been around since Linux 2.6.24, but in terms of the
semantics of the thread nice value, there was no visible change
then, *unless* explicit action was taken to create cgroups.

The arrival of autogroups in Linux 2.6.38 was different. 
With this feature enabled (which is the default), task
groups were implicitly created *without the user needing to
do anything*. Thus, [two terminal windows] == [two task groups]
and in those two terminal windows, nice(1) on a CPU-bound
command in one terminal did nothing in terms of improving
CPU access for a CPU-bound tasks running on the other terminal
window.

Put more succinctly: in Linux 2.6.38, autogrouping broke nice(1)
for many use cases.

Once I came to that simple summary it was easy to find multiple
reports of problems from users:

http://serverfault.com/questions/405092/nice-level-not-working-on-linux
http://superuser.com/questions/805599/nice-has-no-effect-in-linux-unless-the-same-shell-is-used
https://www.reddit.com/r/linux/comments/1c4jew/nice_has_no_effect/
http://stackoverflow.com/questions/10342470/process-niceness-priority-setting-has-no-effect-on-linux

Someone else quickly pointed out to me another such report:

https://bbs.archlinux.org/viewtopic.php?id=149553

And when I quickly surveyed a few more or less savvy Linux users
in one room, most understood what nice does, but none of them knew
about the behavior change wrought by autogroup.

I haven't looked at all of the mails in the old threads that 
discussed the implementation of this feature, but so far none of
those that I saw mentioned this behavior change. It's unfortunate
that it never even got documented.

Cheers,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [RFC PATCH] crypto: Add IV generation algorithms

[Herbert Xu]

On Tue, Nov 29, 2016 at 10:15:40AM +0530, Binoy Jayan wrote:
>
> Thank you for the reply. The dm-crypt changes are also included as
> part of this patchset. It has been tested for functionality as well.
> More information can be found in the cover letter including the test
> procedure etc.
> 
> https://lkml.org/lkml/2016/11/21/168

Sorry, I don't know how I missed that :)

But that begs the question, who is supposed to use crypto_geniv_set_ctx?
I thought it was dm-crypt but your patch doesn't contain any uses
of it at all.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC PATCH] crypto: Add IV generation algorithms

[Herbert Xu]

On Tue, Nov 29, 2016 at 10:15:40AM +0530, Binoy Jayan wrote:
>
> Thank you for the reply. The dm-crypt changes are also included as
> part of this patchset. It has been tested for functionality as well.
> More information can be found in the cover letter including the test
> procedure etc.
> 
> https://lkml.org/lkml/2016/11/21/168

Sorry, I don't know how I missed that :)

But that begs the question, who is supposed to use crypto_geniv_set_ctx?
I thought it was dm-crypt but your patch doesn't contain any uses
of it at all.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH V4] i2c: mux: pca954x: Add ACPI support for pca954x

[Peter Rosin]

On 2016-11-29 08:04, Tin Huynh wrote:
> This patch enables ACPI support for mux-pca954x driver.
> 
> Signed-off-by: Tin Huynh 
> 
> Change from v1 :
>  -Don't shadow id variable.
>  -Include sorted header.
>  -Redefine acpi_device_id.
>  -Add CONFIG_ACPI.
> ---
>  drivers/i2c/muxes/i2c-mux-pca954x.c |   27 ++-
>  1 files changed, 26 insertions(+), 1 deletions(-)
> 
> Change from V3:
>  -Readd CONFIG_ACPI.
> 
> Change from V2:
>  -Remove CONFIG_ACPI.
> 
> Change from V1 :
>  -Don't shadow id variable.
>  -Include sorted header.
>  -Redefine acpi_device_id.
>  -Add CONFIG_ACPI.
> 
> 
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c 
> b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 1091346..bdf4315 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -35,6 +35,7 @@
>   * warranty of any kind, whether express or implied.
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -120,6 +121,21 @@ struct pca954x {
>  };
>  MODULE_DEVICE_TABLE(i2c, pca954x_id);
>  
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id pca954x_acpi_ids[] = {
> + { .id = "PCA9540", .driver_data = pca_9540 },
> + { .id = "PCA9542", .driver_data = pca_9540 },
> + { .id = "PCA9543", .driver_data = pca_9543 },
> + { .id = "PCA9544", .driver_data = pca_9544 },
> + { .id = "PCA9545", .driver_data = pca_9545 },
> + { .id = "PCA9546", .driver_data = pca_9545 },
> + { .id = "PCA9547", .driver_data = pca_9547 },
> + { .id = "PCA9548", .driver_data = pca_9548 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, pca954x_acpi_ids);
> +#endif
> +
>  #ifdef CONFIG_OF
>  static const struct of_device_id pca954x_of_match[] = {
>   { .compatible = "nxp,pca9540", .data = [pca_9540] },
> @@ -245,8 +261,16 @@ static int pca954x_probe(struct i2c_client *client,
>   match = of_match_device(of_match_ptr(pca954x_of_match), >dev);
>   if (match)
>   data->chip = of_device_get_match_data(>dev);
> - else
> + else if (id)
>   data->chip = [id->driver_data];
> + else {
> + const struct acpi_device_id *acpi_id;
> +
> + acpi_id = acpi_match_device(pca954x_acpi_ids, >dev);

You missed my note about this line in the v4 review, you need to use
ACPI_PTR(pca954x_acpi_ids) here as well (instead of #ifdef-ing the
whole block as you did in v3).

Cheers,
Peter

> + if (!acpi_id)
> + return -ENODEV;
> + data->chip = [acpi_id->driver_data];
> + }
>  
>   data->last_chan = 0;   /* force the first selection */
>  
> @@ -321,6 +345,7 @@ static int pca954x_resume(struct device *dev)
>   .name   = "pca954x",
>   .pm = _pm,
>   .of_match_table = of_match_ptr(pca954x_of_match),
> + .acpi_match_table = ACPI_PTR(pca954x_acpi_ids),
>   },
>   .probe  = pca954x_probe,
>   .remove = pca954x_remove,
> 

Re: [PATCH V4] i2c: mux: pca954x: Add ACPI support for pca954x

[Peter Rosin]

On 2016-11-29 08:04, Tin Huynh wrote:
> This patch enables ACPI support for mux-pca954x driver.
> 
> Signed-off-by: Tin Huynh 
> 
> Change from v1 :
>  -Don't shadow id variable.
>  -Include sorted header.
>  -Redefine acpi_device_id.
>  -Add CONFIG_ACPI.
> ---
>  drivers/i2c/muxes/i2c-mux-pca954x.c |   27 ++-
>  1 files changed, 26 insertions(+), 1 deletions(-)
> 
> Change from V3:
>  -Readd CONFIG_ACPI.
> 
> Change from V2:
>  -Remove CONFIG_ACPI.
> 
> Change from V1 :
>  -Don't shadow id variable.
>  -Include sorted header.
>  -Redefine acpi_device_id.
>  -Add CONFIG_ACPI.
> 
> 
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c 
> b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 1091346..bdf4315 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -35,6 +35,7 @@
>   * warranty of any kind, whether express or implied.
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -120,6 +121,21 @@ struct pca954x {
>  };
>  MODULE_DEVICE_TABLE(i2c, pca954x_id);
>  
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id pca954x_acpi_ids[] = {
> + { .id = "PCA9540", .driver_data = pca_9540 },
> + { .id = "PCA9542", .driver_data = pca_9540 },
> + { .id = "PCA9543", .driver_data = pca_9543 },
> + { .id = "PCA9544", .driver_data = pca_9544 },
> + { .id = "PCA9545", .driver_data = pca_9545 },
> + { .id = "PCA9546", .driver_data = pca_9545 },
> + { .id = "PCA9547", .driver_data = pca_9547 },
> + { .id = "PCA9548", .driver_data = pca_9548 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, pca954x_acpi_ids);
> +#endif
> +
>  #ifdef CONFIG_OF
>  static const struct of_device_id pca954x_of_match[] = {
>   { .compatible = "nxp,pca9540", .data = [pca_9540] },
> @@ -245,8 +261,16 @@ static int pca954x_probe(struct i2c_client *client,
>   match = of_match_device(of_match_ptr(pca954x_of_match), >dev);
>   if (match)
>   data->chip = of_device_get_match_data(>dev);
> - else
> + else if (id)
>   data->chip = [id->driver_data];
> + else {
> + const struct acpi_device_id *acpi_id;
> +
> + acpi_id = acpi_match_device(pca954x_acpi_ids, >dev);

You missed my note about this line in the v4 review, you need to use
ACPI_PTR(pca954x_acpi_ids) here as well (instead of #ifdef-ing the
whole block as you did in v3).

Cheers,
Peter

> + if (!acpi_id)
> + return -ENODEV;
> + data->chip = [acpi_id->driver_data];
> + }
>  
>   data->last_chan = 0;   /* force the first selection */
>  
> @@ -321,6 +345,7 @@ static int pca954x_resume(struct device *dev)
>   .name   = "pca954x",
>   .pm = _pm,
>   .of_match_table = of_match_ptr(pca954x_of_match),
> + .acpi_match_table = ACPI_PTR(pca954x_acpi_ids),
>   },
>   .probe  = pca954x_probe,
>   .remove = pca954x_remove,
> 

Re: [PATCH V3] i2c: mux: pca954x: Add ACPI support for pca954x

[Peter Rosin]

Hi!

We're apparently misunderstanding each other, I meant only that last
#ifdef in the v2 review... Sorry for not being clearer, new attempt
below.

Cheers,
Peter

On 2016-11-29 04:46, tnhu...@apm.com wrote:
> From: Tin Huynh 
> 
> This patch enables ACPI support for mux-pca954x driver.
> 
> Signed-off-by: Tin Huynh 
> 
> ---
>  drivers/i2c/muxes/i2c-mux-pca954x.c |   25 -
>  1 files changed, 24 insertions(+), 1 deletions(-)
> 
> Change from V2:
>  -Remove CONFIG_ACPI.
> 
> Change from V1:
>  -Don't shadow id variable.
>  -Include sorted header.
>  -Redefine acpi_device_id.
>  -Add CONFIG_ACPI.
> 
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c 
> b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 1091346..f80191e 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -35,6 +35,7 @@
>   * warranty of any kind, whether express or implied.
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -120,6 +121,19 @@ struct pca954x {
>  };
>  MODULE_DEVICE_TABLE(i2c, pca954x_id);
>  

#ifdef CONFIG_ACPI

> +static const struct acpi_device_id pca954x_acpi_ids[] = {
> + { .id = "PCA9540", .driver_data = pca_9540 },
> + { .id = "PCA9542", .driver_data = pca_9540 },
> + { .id = "PCA9543", .driver_data = pca_9543 },
> + { .id = "PCA9544", .driver_data = pca_9544 },
> + { .id = "PCA9545", .driver_data = pca_9545 },
> + { .id = "PCA9546", .driver_data = pca_9545 },
> + { .id = "PCA9547", .driver_data = pca_9547 },
> + { .id = "PCA9548", .driver_data = pca_9548 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, pca954x_acpi_ids);

#endif

> +
>  #ifdef CONFIG_OF
>  static const struct of_device_id pca954x_of_match[] = {
>   { .compatible = "nxp,pca9540", .data = [pca_9540] },
> @@ -245,8 +259,16 @@ static int pca954x_probe(struct i2c_client *client,
>   match = of_match_device(of_match_ptr(pca954x_of_match), >dev);
>   if (match)
>   data->chip = of_device_get_match_data(>dev);
> - else
> + else if (id)
>   data->chip = [id->driver_data];
> + else {
> + const struct acpi_device_id *acpi_id;
> +
> + acpi_id = acpi_match_device(pca954x_acpi_ids, >dev);

acpi_id = acpi_match_device(ACPI_PTR(pca954x_acpi_ids),
>dev);

> + if (!acpi_id)
> + return -ENODEV;
> + data->chip = [acpi_id->driver_data];
> + }
>  
>   data->last_chan = 0;   /* force the first selection */
>  
> @@ -321,6 +343,7 @@ static int pca954x_resume(struct device *dev)
>   .name   = "pca954x",
>   .pm = _pm,
>   .of_match_table = of_match_ptr(pca954x_of_match),
> + .acpi_match_table = ACPI_PTR(pca954x_acpi_ids),
>   },
>   .probe  = pca954x_probe,
>   .remove = pca954x_remove,
> 

Re: [PATCH V3] i2c: mux: pca954x: Add ACPI support for pca954x

[Peter Rosin]

Hi!

We're apparently misunderstanding each other, I meant only that last
#ifdef in the v2 review... Sorry for not being clearer, new attempt
below.

Cheers,
Peter

On 2016-11-29 04:46, tnhu...@apm.com wrote:
> From: Tin Huynh 
> 
> This patch enables ACPI support for mux-pca954x driver.
> 
> Signed-off-by: Tin Huynh 
> 
> ---
>  drivers/i2c/muxes/i2c-mux-pca954x.c |   25 -
>  1 files changed, 24 insertions(+), 1 deletions(-)
> 
> Change from V2:
>  -Remove CONFIG_ACPI.
> 
> Change from V1:
>  -Don't shadow id variable.
>  -Include sorted header.
>  -Redefine acpi_device_id.
>  -Add CONFIG_ACPI.
> 
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c 
> b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 1091346..f80191e 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -35,6 +35,7 @@
>   * warranty of any kind, whether express or implied.
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -120,6 +121,19 @@ struct pca954x {
>  };
>  MODULE_DEVICE_TABLE(i2c, pca954x_id);
>  

#ifdef CONFIG_ACPI

> +static const struct acpi_device_id pca954x_acpi_ids[] = {
> + { .id = "PCA9540", .driver_data = pca_9540 },
> + { .id = "PCA9542", .driver_data = pca_9540 },
> + { .id = "PCA9543", .driver_data = pca_9543 },
> + { .id = "PCA9544", .driver_data = pca_9544 },
> + { .id = "PCA9545", .driver_data = pca_9545 },
> + { .id = "PCA9546", .driver_data = pca_9545 },
> + { .id = "PCA9547", .driver_data = pca_9547 },
> + { .id = "PCA9548", .driver_data = pca_9548 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, pca954x_acpi_ids);

#endif

> +
>  #ifdef CONFIG_OF
>  static const struct of_device_id pca954x_of_match[] = {
>   { .compatible = "nxp,pca9540", .data = [pca_9540] },
> @@ -245,8 +259,16 @@ static int pca954x_probe(struct i2c_client *client,
>   match = of_match_device(of_match_ptr(pca954x_of_match), >dev);
>   if (match)
>   data->chip = of_device_get_match_data(>dev);
> - else
> + else if (id)
>   data->chip = [id->driver_data];
> + else {
> + const struct acpi_device_id *acpi_id;
> +
> + acpi_id = acpi_match_device(pca954x_acpi_ids, >dev);

acpi_id = acpi_match_device(ACPI_PTR(pca954x_acpi_ids),
>dev);

> + if (!acpi_id)
> + return -ENODEV;
> + data->chip = [acpi_id->driver_data];
> + }
>  
>   data->last_chan = 0;   /* force the first selection */
>  
> @@ -321,6 +343,7 @@ static int pca954x_resume(struct device *dev)
>   .name   = "pca954x",
>   .pm = _pm,
>   .of_match_table = of_match_ptr(pca954x_of_match),
> + .acpi_match_table = ACPI_PTR(pca954x_acpi_ids),
>   },
>   .probe  = pca954x_probe,
>   .remove = pca954x_remove,
> 

Re: [PATCH 7/7] trace: Update documentation for mono, mono_raw and boot clock

[Ingo Molnar]

* John Stultz  wrote:

> +   boot: This is the boot clock (CLOCK_BOOTTIME) and is based on the
> + fast monotonic clock, but also accounts for time spent in
> + suspend. Since the clock access is designed for use in
> + tracing in the suspend path, some side effects are possible
> + if clock is accessed after the suspend time is accounted before
> + the fast mono clock is updated. In this case, the clock update
> + appears to happen slightly sooner than it normally would have.
> + Also on 32-bit systems, its possible that the 64-bit boot offset
> + sees a partial update. These effects are rare and post
> + processing should be able to handle them. See comments on
> + ktime_get_boot_fast_ns function for more information.

s/its possible/it's possible
s/comments on ktime_get_boost_fast_ns function/comments in the 
ktime_get_boost_fast_ns() function

Thanks,

Ingo

Re: [PATCH 7/7] trace: Update documentation for mono, mono_raw and boot clock

[Ingo Molnar]

* John Stultz  wrote:

> +   boot: This is the boot clock (CLOCK_BOOTTIME) and is based on the
> + fast monotonic clock, but also accounts for time spent in
> + suspend. Since the clock access is designed for use in
> + tracing in the suspend path, some side effects are possible
> + if clock is accessed after the suspend time is accounted before
> + the fast mono clock is updated. In this case, the clock update
> + appears to happen slightly sooner than it normally would have.
> + Also on 32-bit systems, its possible that the 64-bit boot offset
> + sees a partial update. These effects are rare and post
> + processing should be able to handle them. See comments on
> + ktime_get_boot_fast_ns function for more information.

s/its possible/it's possible
s/comments on ktime_get_boost_fast_ns function/comments in the 
ktime_get_boost_fast_ns() function

Thanks,

Ingo

Re: [PATCH] block,blkcg: use __GFP_NOWARN for best-effort allocations in blkcg

[Michal Hocko]

On Mon 28-11-16 12:19:07, Tejun Heo wrote:
> Hello,
> 
> On Wed, Nov 23, 2016 at 09:50:12AM +0100, Vlastimil Babka wrote:
> > > You'd certainly _hope_ that atomic allocations either have fallbacks
> > > or are harmless if they fail, but I'd still rather see that
> > > __GFP_NOWARN just to make that very much explicit.
> > 
> > A global change to GFP_NOWAIT would of course mean that we should audit its
> > users (there don't seem to be many), whether they are using it consciously
> > and should not rather be using GFP_ATOMIC.
> 
> A while ago, I thought about something like, say, GFP_MAYBE which is
> combination of NOWAIT and NOWARN but couldn't really come up with
> scenarios where one would want to use NOWAIT w/o NOWARN.  If an
> allocation is important enough to warn the user of its failure, it
> better be dipping into the atomic reserve pool; otherwise, it doesn't
> make sense to make noise.

I do not think we really need a new flag for that and fully agree that
GFP_NOWAIT warning about failure is rarely, if ever, useful.
Historically we didn't use to distinguish atomic (with access to
reserves) allocations from those which just do not want to trigger the
reclaim resp. to sleep (aka optimistic allocation requests). But this
has changed so I guess we can really do the following 
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index f8041f9de31e..a53b5187b4da 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -246,7 +246,7 @@ struct vm_area_struct;
 #define GFP_ATOMIC (__GFP_HIGH|__GFP_ATOMIC|__GFP_KSWAPD_RECLAIM)
 #define GFP_KERNEL (__GFP_RECLAIM | __GFP_IO | __GFP_FS)
 #define GFP_KERNEL_ACCOUNT (GFP_KERNEL | __GFP_ACCOUNT)
-#define GFP_NOWAIT (__GFP_KSWAPD_RECLAIM)
+#define GFP_NOWAIT (__GFP_KSWAPD_RECLAIM|__GFP_NOWARN)
 #define GFP_NOIO   (__GFP_RECLAIM)
 #define GFP_NOFS   (__GFP_RECLAIM | __GFP_IO)
 #define GFP_TEMPORARY  (__GFP_RECLAIM | __GFP_IO | __GFP_FS | \

this will not catch users who are doing gfp & ~__GFP_DIRECT_RECLAIM but
I would rather not make warn_alloc() even more cluttered with checks.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH] block,blkcg: use __GFP_NOWARN for best-effort allocations in blkcg

[Michal Hocko]

On Mon 28-11-16 12:19:07, Tejun Heo wrote:
> Hello,
> 
> On Wed, Nov 23, 2016 at 09:50:12AM +0100, Vlastimil Babka wrote:
> > > You'd certainly _hope_ that atomic allocations either have fallbacks
> > > or are harmless if they fail, but I'd still rather see that
> > > __GFP_NOWARN just to make that very much explicit.
> > 
> > A global change to GFP_NOWAIT would of course mean that we should audit its
> > users (there don't seem to be many), whether they are using it consciously
> > and should not rather be using GFP_ATOMIC.
> 
> A while ago, I thought about something like, say, GFP_MAYBE which is
> combination of NOWAIT and NOWARN but couldn't really come up with
> scenarios where one would want to use NOWAIT w/o NOWARN.  If an
> allocation is important enough to warn the user of its failure, it
> better be dipping into the atomic reserve pool; otherwise, it doesn't
> make sense to make noise.

I do not think we really need a new flag for that and fully agree that
GFP_NOWAIT warning about failure is rarely, if ever, useful.
Historically we didn't use to distinguish atomic (with access to
reserves) allocations from those which just do not want to trigger the
reclaim resp. to sleep (aka optimistic allocation requests). But this
has changed so I guess we can really do the following 
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index f8041f9de31e..a53b5187b4da 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -246,7 +246,7 @@ struct vm_area_struct;
 #define GFP_ATOMIC (__GFP_HIGH|__GFP_ATOMIC|__GFP_KSWAPD_RECLAIM)
 #define GFP_KERNEL (__GFP_RECLAIM | __GFP_IO | __GFP_FS)
 #define GFP_KERNEL_ACCOUNT (GFP_KERNEL | __GFP_ACCOUNT)
-#define GFP_NOWAIT (__GFP_KSWAPD_RECLAIM)
+#define GFP_NOWAIT (__GFP_KSWAPD_RECLAIM|__GFP_NOWARN)
 #define GFP_NOIO   (__GFP_RECLAIM)
 #define GFP_NOFS   (__GFP_RECLAIM | __GFP_IO)
 #define GFP_TEMPORARY  (__GFP_RECLAIM | __GFP_IO | __GFP_FS | \

this will not catch users who are doing gfp & ~__GFP_DIRECT_RECLAIM but
I would rather not make warn_alloc() even more cluttered with checks.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 4/7] time: alarmtimer: Add the tracepoints for alarmtimer

[Ingo Molnar]

* John Stultz  wrote:

> From: Baolin Wang 
> 
> For system debugging, we sometimes want to know who sets one
> alarm timer, the time of the timer, when the timer started and
> fired and so on. Thus adding tracepoints can help us trace the
> alarmtimer information.

s/one alarm timer/an alarm timer

> For example, when we debug the system supend/resume, if the
> system is always resumed by RTC alarm, we can find out which
> process set the alarm timer to resume system by below trace log:

s/when we debug the system/when we debug system
s/supend/suspend
s/resume system/resume the system
s/by below trace log/by the trace log below

> From the trace log, we can find out the 'Binder:3292_2' process
> set one alarm timer which resumes the system.

s/set one alarm timer/set an alarm timer

> Changes since v4:
>  - Initialize 'type' to -1 and rename it in alarmtimer_suspend().
>  - Fix typo in subject line.
> 
> Changes since v3:
>  - Remove the "ALARM_" prefix in the string.
>  - Add the ACK by Steven Rostedt.
> 
> Changes since v2:
>  - Save time as s64 type.
>  - Remove 'process_name' parameter and add 'now' parameter.
>  - Rename the trace event name.
>  - Remove restart trace event.
>  - Other optimization.

I find it really sad that a patch that has gone through 4 iterations still has 
so 
many typos and grammar errors in its changelog :-(

Thanks,

Ingo

Re: [PATCH 4/7] time: alarmtimer: Add the tracepoints for alarmtimer

[Ingo Molnar]

* John Stultz  wrote:

> From: Baolin Wang 
> 
> For system debugging, we sometimes want to know who sets one
> alarm timer, the time of the timer, when the timer started and
> fired and so on. Thus adding tracepoints can help us trace the
> alarmtimer information.

s/one alarm timer/an alarm timer

> For example, when we debug the system supend/resume, if the
> system is always resumed by RTC alarm, we can find out which
> process set the alarm timer to resume system by below trace log:

s/when we debug the system/when we debug system
s/supend/suspend
s/resume system/resume the system
s/by below trace log/by the trace log below

> From the trace log, we can find out the 'Binder:3292_2' process
> set one alarm timer which resumes the system.

s/set one alarm timer/set an alarm timer

> Changes since v4:
>  - Initialize 'type' to -1 and rename it in alarmtimer_suspend().
>  - Fix typo in subject line.
> 
> Changes since v3:
>  - Remove the "ALARM_" prefix in the string.
>  - Add the ACK by Steven Rostedt.
> 
> Changes since v2:
>  - Save time as s64 type.
>  - Remove 'process_name' parameter and add 'now' parameter.
>  - Rename the trace event name.
>  - Remove restart trace event.
>  - Other optimization.

I find it really sad that a patch that has gone through 4 iterations still has 
so 
many typos and grammar errors in its changelog :-(

Thanks,

Ingo

Re: [PATCH 2/7] timekeeping: Ignore the bogus sleep time if pm_trace is enabled

[Ingo Molnar]

* John Stultz  wrote:

> From: Chen Yu 
> 
> Previously we encountered some memory overflow issues due to
> the bogus sleep time brought by inconsistent rtc, which is
> triggered when pm_trace is enabled, and we have fixed it
> in recent kernel. However it's improper in the first place
> to call __timekeeping_inject_sleeptime() in case that pm_trace
> is enabled simply because that "hash" time value will wreckage
> the timekeeping subsystem.

s/

 Previously we encountered memory overflow issues due to
 bogus sleep time brought by an inconsistent RTC, which is
 triggered when pm_trace is enabled, and we have fixed it
 in recent kernels. However it's improper in the first place
 to call __timekeeping_inject_sleeptime() in case pm_trace
 is enabled simply because the "hash" time value will wreckage
 the timekeeping subsystem.

Half a dozen typos ...

> This patch is originally written by Thomas, which would bypass
> the bogus rtc interval when pm_trace is enabled.
> Meanwhile, if system succeed to resume back with pm_trace set, the
> users are warned to adjust the bogus rtc either by 'ntpdate' or
> 'rdate', by resetting pm_trace_rtc_abused to false, otherwise above
> tools might not work as expected.

s/

 This patch was originally written by Thomas, which would bypass
 the bogus RTC interval when pm_trace is enabled.
 Meanwhile, if the system succeeds to resume back with pm_trace set,
 users are warned to adjust the bogus RTC either by 'ntpdate' or
 'rdate', by resetting pm_trace_rtc_abused to false, otherwise above
 tools might not work as expected.

> + /*
> +  * If pm_trace abused the RTC as storage set the timespec to 0
> +  * which tells the caller that this RTC value is bogus.
> +  */

s/
/*
 * If pm_trace abused the RTC as storage, set the timespec to 0,
 * which tells the caller that this RTC value is bogus.
 */

> @@ -74,6 +75,9 @@
>  
>  #define DEVSEED (7919)
>  
> +bool pm_trace_rtc_abused __read_mostly;
> +EXPORT_SYMBOL(pm_trace_rtc_abused);

EXPORT_SYMBOL_GPL()

> +static int pm_trace_notify(struct notifier_block *nb,
> + unsigned long mode, void *_unused)

Please no nonsensical linebreaks in the middle of an argument list.

> +{
> + switch (mode) {
> + case PM_POST_HIBERNATION:
> + case PM_POST_SUSPEND:
> + if (pm_trace_rtc_abused) {
> + pm_trace_rtc_abused = false;
> + pr_warn("Possible incorrect RTC due to pm_trace, please 
> use 'ntpdate' or 'rdate' to reset.\n");


s/to reset./to reset it.

Thanks,

Ingo

Re: [PATCH 2/7] timekeeping: Ignore the bogus sleep time if pm_trace is enabled

[Ingo Molnar]

* John Stultz  wrote:

> From: Chen Yu 
> 
> Previously we encountered some memory overflow issues due to
> the bogus sleep time brought by inconsistent rtc, which is
> triggered when pm_trace is enabled, and we have fixed it
> in recent kernel. However it's improper in the first place
> to call __timekeeping_inject_sleeptime() in case that pm_trace
> is enabled simply because that "hash" time value will wreckage
> the timekeeping subsystem.

s/

 Previously we encountered memory overflow issues due to
 bogus sleep time brought by an inconsistent RTC, which is
 triggered when pm_trace is enabled, and we have fixed it
 in recent kernels. However it's improper in the first place
 to call __timekeeping_inject_sleeptime() in case pm_trace
 is enabled simply because the "hash" time value will wreckage
 the timekeeping subsystem.

Half a dozen typos ...

> This patch is originally written by Thomas, which would bypass
> the bogus rtc interval when pm_trace is enabled.
> Meanwhile, if system succeed to resume back with pm_trace set, the
> users are warned to adjust the bogus rtc either by 'ntpdate' or
> 'rdate', by resetting pm_trace_rtc_abused to false, otherwise above
> tools might not work as expected.

s/

 This patch was originally written by Thomas, which would bypass
 the bogus RTC interval when pm_trace is enabled.
 Meanwhile, if the system succeeds to resume back with pm_trace set,
 users are warned to adjust the bogus RTC either by 'ntpdate' or
 'rdate', by resetting pm_trace_rtc_abused to false, otherwise above
 tools might not work as expected.

> + /*
> +  * If pm_trace abused the RTC as storage set the timespec to 0
> +  * which tells the caller that this RTC value is bogus.
> +  */

s/
/*
 * If pm_trace abused the RTC as storage, set the timespec to 0,
 * which tells the caller that this RTC value is bogus.
 */

> @@ -74,6 +75,9 @@
>  
>  #define DEVSEED (7919)
>  
> +bool pm_trace_rtc_abused __read_mostly;
> +EXPORT_SYMBOL(pm_trace_rtc_abused);

EXPORT_SYMBOL_GPL()

> +static int pm_trace_notify(struct notifier_block *nb,
> + unsigned long mode, void *_unused)

Please no nonsensical linebreaks in the middle of an argument list.

> +{
> + switch (mode) {
> + case PM_POST_HIBERNATION:
> + case PM_POST_SUSPEND:
> + if (pm_trace_rtc_abused) {
> + pm_trace_rtc_abused = false;
> + pr_warn("Possible incorrect RTC due to pm_trace, please 
> use 'ntpdate' or 'rdate' to reset.\n");


s/to reset./to reset it.

Thanks,

Ingo

[tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line detection on x86

[tip-bot for Josh Poimboeuf]

Commit-ID:  8e8d8725d46d93ceffd3e708d905bc101a1905b5
Gitweb: http://git.kernel.org/tip/8e8d8725d46d93ceffd3e708d905bc101a1905b5
Author: Josh Poimboeuf 
AuthorDate: Mon, 28 Nov 2016 17:06:35 -0600
Committer:  Ingo Molnar 
CommitDate: Tue, 29 Nov 2016 08:10:05 +0100

tools/decode_stacktrace.sh: Fix address line detection on x86

Kirill reported that the decode_stacktrace.sh script was broken by the
following commit:

  bb5e5ce545f2 ("x86/dumpstack: Remove kernel text addresses from stack dump")

Fix it by updating the per-line absolute address check to also check for
function-based address lines like the following:

  write_sysrq_trigger+0x51/0x60

I didn't remove the check for absolute addresses because it's still
needed for ARM.

Reported-by: Kirill A. Shutemov 
Tested-by: Kirill A. Shutemov 
Signed-off-by: Josh Poimboeuf 
Cc: Konstantin Khlebnikov 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Sasha Levin 
Cc: Thomas Gleixner 
Fixes: bb5e5ce545f2 ("x86/dumpstack: Remove kernel text addresses from stack 
dump")
Link: http://lkml.kernel.org/r/20161128230635.4n2ofgawltgexgcg@treble
Signed-off-by: Ingo Molnar 
---
 scripts/decode_stacktrace.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh
index c332684..5206d99 100755
--- a/scripts/decode_stacktrace.sh
+++ b/scripts/decode_stacktrace.sh
@@ -139,7 +139,8 @@ handle_line() {
 
 while read line; do
# Let's see if we have an address in the line
-   if [[ $line =~ \[\<([^]]+)\>\]  ]]; then
+   if [[ $line =~ \[\<([^]]+)\>\] ]] ||
+  [[ $line =~ [^+\ ]+\+0x[0-9a-f]+/0x[0-9a-f]+ ]]; then
# Translate address to line numbers
handle_line "$line"
# Is it a code line?

Re: [PATCH] tty: nozomi: avoid sprintf buffer overflow

[Jiri Slaby]

On 11/28/2016, 10:04 PM, Arnd Bergmann wrote:
> Testing with a gcc-7 snapshot produced an internal compiler error
> for this file:
> 
> drivers/tty/nozomi.c: In function 'receive_flow_control':
> drivers/tty/nozomi.c:919:12: internal compiler error: in 
> get_substring_ranges_for_loc, at input.c:1388
>  static int receive_flow_control(struct nozomi *dc)
> 
> I've reported this at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78569
> but also noticed that the code line contains a stack overflow, as it prints
> a string into a slightly shorter fixed-length 'tmp' variable.
> 
> I don't see any point in the temporary variable, we can simply use
> pr_debug() to print the output directly. This change should not change
> any of the output but avoids both the stack overflow and the gcc
> crash.
> 
> The stack overflow will not happen unless a module load parameter is
> also set to enable the debug messages.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/tty/nozomi.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
> index e2020a691058..17a182b28d52 100644
> --- a/drivers/tty/nozomi.c
> +++ b/drivers/tty/nozomi.c
> @@ -69,13 +69,8 @@
>  #define NOZOMI_DEBUG_LEVEL 0x00
>  
>  #define P_BUF_SIZE 128
> -#define NFO(_err_flag_, args...) \
> -do { \
> - char tmp[P_BUF_SIZE];   \
> - snprintf(tmp, sizeof(tmp), ##args); \
> - printk(_err_flag_ "[%d] %s(): %s\n", __LINE__,  \
> - __func__, tmp); \
> -} while (0)
> +#define NFO(_err_flag_, fmt, args...)
> \
> + pr_debug("[%d] %s(): " fmt "\n", __LINE__, __func__,  ##args);

Could you also kill the now unused _err_flag_ parameter (including the
only user of NFO)?

(Or inline pr_debug directly into the only user and kill NFO completely?)

thanks,
-- 
js
suse labs

[tip:x86/urgent] tools/decode_stacktrace.sh: Fix address line detection on x86

[tip-bot for Josh Poimboeuf]

Commit-ID:  8e8d8725d46d93ceffd3e708d905bc101a1905b5
Gitweb: http://git.kernel.org/tip/8e8d8725d46d93ceffd3e708d905bc101a1905b5
Author: Josh Poimboeuf 
AuthorDate: Mon, 28 Nov 2016 17:06:35 -0600
Committer:  Ingo Molnar 
CommitDate: Tue, 29 Nov 2016 08:10:05 +0100

tools/decode_stacktrace.sh: Fix address line detection on x86

Kirill reported that the decode_stacktrace.sh script was broken by the
following commit:

  bb5e5ce545f2 ("x86/dumpstack: Remove kernel text addresses from stack dump")

Fix it by updating the per-line absolute address check to also check for
function-based address lines like the following:

  write_sysrq_trigger+0x51/0x60

I didn't remove the check for absolute addresses because it's still
needed for ARM.

Reported-by: Kirill A. Shutemov 
Tested-by: Kirill A. Shutemov 
Signed-off-by: Josh Poimboeuf 
Cc: Konstantin Khlebnikov 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Sasha Levin 
Cc: Thomas Gleixner 
Fixes: bb5e5ce545f2 ("x86/dumpstack: Remove kernel text addresses from stack 
dump")
Link: http://lkml.kernel.org/r/20161128230635.4n2ofgawltgexgcg@treble
Signed-off-by: Ingo Molnar 
---
 scripts/decode_stacktrace.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh
index c332684..5206d99 100755
--- a/scripts/decode_stacktrace.sh
+++ b/scripts/decode_stacktrace.sh
@@ -139,7 +139,8 @@ handle_line() {
 
 while read line; do
# Let's see if we have an address in the line
-   if [[ $line =~ \[\<([^]]+)\>\]  ]]; then
+   if [[ $line =~ \[\<([^]]+)\>\] ]] ||
+  [[ $line =~ [^+\ ]+\+0x[0-9a-f]+/0x[0-9a-f]+ ]]; then
# Translate address to line numbers
handle_line "$line"
# Is it a code line?

Re: [PATCH] tty: nozomi: avoid sprintf buffer overflow

[Jiri Slaby]

On 11/28/2016, 10:04 PM, Arnd Bergmann wrote:
> Testing with a gcc-7 snapshot produced an internal compiler error
> for this file:
> 
> drivers/tty/nozomi.c: In function 'receive_flow_control':
> drivers/tty/nozomi.c:919:12: internal compiler error: in 
> get_substring_ranges_for_loc, at input.c:1388
>  static int receive_flow_control(struct nozomi *dc)
> 
> I've reported this at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78569
> but also noticed that the code line contains a stack overflow, as it prints
> a string into a slightly shorter fixed-length 'tmp' variable.
> 
> I don't see any point in the temporary variable, we can simply use
> pr_debug() to print the output directly. This change should not change
> any of the output but avoids both the stack overflow and the gcc
> crash.
> 
> The stack overflow will not happen unless a module load parameter is
> also set to enable the debug messages.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/tty/nozomi.c | 9 ++---
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
> index e2020a691058..17a182b28d52 100644
> --- a/drivers/tty/nozomi.c
> +++ b/drivers/tty/nozomi.c
> @@ -69,13 +69,8 @@
>  #define NOZOMI_DEBUG_LEVEL 0x00
>  
>  #define P_BUF_SIZE 128
> -#define NFO(_err_flag_, args...) \
> -do { \
> - char tmp[P_BUF_SIZE];   \
> - snprintf(tmp, sizeof(tmp), ##args); \
> - printk(_err_flag_ "[%d] %s(): %s\n", __LINE__,  \
> - __func__, tmp); \
> -} while (0)
> +#define NFO(_err_flag_, fmt, args...)
> \
> + pr_debug("[%d] %s(): " fmt "\n", __LINE__, __func__,  ##args);

Could you also kill the now unused _err_flag_ parameter (including the
only user of NFO)?

(Or inline pr_debug directly into the only user and kill NFO completely?)

thanks,
-- 
js
suse labs

Re: [PATCH 4/4] thermal/intel_powerclamp: stop sched tick in forced idle

[Jacob Pan]

On Tue, 29 Nov 2016 07:21:14 +0800
kbuild test robot <l...@intel.com> wrote:

> Hi Jacob,
> 
> [auto build test ERROR on soc-thermal/next]
> [also build test ERROR on v4.9-rc7 next-20161128]
> [if your patch is applied to the wrong git tree, please drop us a
> note to help improve the system]
> 
This patchset is applicable on top of another patch indicated in the
cover letter.
(https://lkml.org/lkml/2016/11/28/683)

Thanks,

Jacob

> url:
> https://github.com/0day-ci/linux/commits/Jacob-Pan/Misc-enhancements-to-intel_powerclamp/20161129-065523
> base:
> https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git
> next config: x86_64-randconfig-x015-201648 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the
> attached .config to linux build tree make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>drivers/thermal/intel_powerclamp.c: In function
> 'clamp_idle_injection_func':
> >> drivers/thermal/intel_powerclamp.c:448:2: error: implicit
> >> declaration of function
> >> 'play_idle' [-Werror=implicit-function-declaration]
>  play_idle(jiffies_to_msecs(w_data->duration_jiffies));
>  ^
>cc1: some warnings being treated as errors
> 
> vim +/play_idle +448 drivers/thermal/intel_powerclamp.c
> 
>442smp_mb();
>443}
>444
>445if (should_skip)
>446goto balance;
>447
>  > 448
>  > play_idle(jiffies_to_msecs(w_data->duration_jiffies));
>449
>450balance:
>451if (clamping && w_data->clamping &&
> cpu_online(w_data->cpu))
> 
> ---
> 0-DAY kernel test infrastructureOpen Source
> Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel
> Corporation

[Jacob Pan]

Re: [PATCH 4/4] thermal/intel_powerclamp: stop sched tick in forced idle

[Jacob Pan]

On Tue, 29 Nov 2016 07:21:14 +0800
kbuild test robot  wrote:

> Hi Jacob,
> 
> [auto build test ERROR on soc-thermal/next]
> [also build test ERROR on v4.9-rc7 next-20161128]
> [if your patch is applied to the wrong git tree, please drop us a
> note to help improve the system]
> 
This patchset is applicable on top of another patch indicated in the
cover letter.
(https://lkml.org/lkml/2016/11/28/683)

Thanks,

Jacob

> url:
> https://github.com/0day-ci/linux/commits/Jacob-Pan/Misc-enhancements-to-intel_powerclamp/20161129-065523
> base:
> https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git
> next config: x86_64-randconfig-x015-201648 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the
> attached .config to linux build tree make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>drivers/thermal/intel_powerclamp.c: In function
> 'clamp_idle_injection_func':
> >> drivers/thermal/intel_powerclamp.c:448:2: error: implicit
> >> declaration of function
> >> 'play_idle' [-Werror=implicit-function-declaration]
>  play_idle(jiffies_to_msecs(w_data->duration_jiffies));
>  ^
>cc1: some warnings being treated as errors
> 
> vim +/play_idle +448 drivers/thermal/intel_powerclamp.c
> 
>442smp_mb();
>443}
>444
>445if (should_skip)
>446goto balance;
>447
>  > 448
>  > play_idle(jiffies_to_msecs(w_data->duration_jiffies));
>449
>450balance:
>451if (clamping && w_data->clamping &&
> cpu_online(w_data->cpu))
> 
> ---
> 0-DAY kernel test infrastructureOpen Source
> Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel
> Corporation

[Jacob Pan]

Re: [tip:x86/core] x86: Enable Intel Turbo Boost Max Technology 3.0

[Ingo Molnar]

* Tim Chen  wrote:

> > +   If unsure say Y here.
> > 
> > If/when other architectures make use of this the Kconfig entry can be moved 
> > into 
> > the scheduler Kconfig - but for the time being it can stay in arch/x86/.
> > 
> > Another variant would be to eliminate the Kconfig option altogether and 
> > make it a 
> > natural feature of SCHED_MC (like it is in the core scheduler).
> > 
> 
> I am fine with renaming SCHED_ITMT to SCHED_MC_PRIO.

Ok, could you please send a delta patch on top of tip:sched/core that does this 
and the other improvements?

Thanks,

Ingo

Re: [tip:x86/core] x86: Enable Intel Turbo Boost Max Technology 3.0

[Ingo Molnar]

* Tim Chen  wrote:

> > +   If unsure say Y here.
> > 
> > If/when other architectures make use of this the Kconfig entry can be moved 
> > into 
> > the scheduler Kconfig - but for the time being it can stay in arch/x86/.
> > 
> > Another variant would be to eliminate the Kconfig option altogether and 
> > make it a 
> > natural feature of SCHED_MC (like it is in the core scheduler).
> > 
> 
> I am fine with renaming SCHED_ITMT to SCHED_MC_PRIO.

Ok, could you please send a delta patch on top of tip:sched/core that does this 
and the other improvements?

Thanks,

Ingo

Re: [PATCH v2 10/12] mm: mempolicy: mbind and migrate_pages support thp migration

[Naoya Horiguchi]

On Fri, Nov 25, 2016 at 05:57:20PM +0530, Anshuman Khandual wrote:
> On 11/08/2016 05:01 AM, Naoya Horiguchi wrote:
...
> > @@ -497,30 +541,15 @@ static int queue_pages_pte_range(pmd_t *pmd, unsigned 
> > long addr,
> > struct page *page;
> > struct queue_pages *qp = walk->private;
> > unsigned long flags = qp->flags;
> > -   int nid, ret;
> > +   int ret;
> > pte_t *pte;
> > spinlock_t *ptl;
> >  
> > -   if (pmd_trans_huge(*pmd)) {
> > -   ptl = pmd_lock(walk->mm, pmd);
> > -   if (pmd_trans_huge(*pmd)) {
> > -   page = pmd_page(*pmd);
> > -   if (is_huge_zero_page(page)) {
> > -   spin_unlock(ptl);
> > -   __split_huge_pmd(vma, pmd, addr, false, NULL);
> > -   } else {
> > -   get_page(page);
> > -   spin_unlock(ptl);
> > -   lock_page(page);
> > -   ret = split_huge_page(page);
> > -   unlock_page(page);
> > -   put_page(page);
> > -   if (ret)
> > -   return 0;
> > -   }
> > -   } else {
> > -   spin_unlock(ptl);
> > -   }
> > +   ptl = pmd_trans_huge_lock(pmd, vma);
> > +   if (ptl) {
> > +   ret = queue_pages_pmd(pmd, ptl, addr, end, walk);
> > +   if (ret)
> > +   return 0;
> > }
> 
> I wonder if we should introduce pte_entry function along with pmd_entry
> function as we are first looking for trans huge PMDs either for direct
> addition into the migration list or splitting it before looking for PTEs.

Most of pagewalk users don't define pte_entry because of performance reason
(to avoid the overhead of PTRS_PER_PMD function calls).
But that could be a nice cleanup if we have a workaround.

Thanks,
Naoya Horiguchi

Re: [PATCH v2 10/12] mm: mempolicy: mbind and migrate_pages support thp migration

[Naoya Horiguchi]

On Fri, Nov 25, 2016 at 05:57:20PM +0530, Anshuman Khandual wrote:
> On 11/08/2016 05:01 AM, Naoya Horiguchi wrote:
...
> > @@ -497,30 +541,15 @@ static int queue_pages_pte_range(pmd_t *pmd, unsigned 
> > long addr,
> > struct page *page;
> > struct queue_pages *qp = walk->private;
> > unsigned long flags = qp->flags;
> > -   int nid, ret;
> > +   int ret;
> > pte_t *pte;
> > spinlock_t *ptl;
> >  
> > -   if (pmd_trans_huge(*pmd)) {
> > -   ptl = pmd_lock(walk->mm, pmd);
> > -   if (pmd_trans_huge(*pmd)) {
> > -   page = pmd_page(*pmd);
> > -   if (is_huge_zero_page(page)) {
> > -   spin_unlock(ptl);
> > -   __split_huge_pmd(vma, pmd, addr, false, NULL);
> > -   } else {
> > -   get_page(page);
> > -   spin_unlock(ptl);
> > -   lock_page(page);
> > -   ret = split_huge_page(page);
> > -   unlock_page(page);
> > -   put_page(page);
> > -   if (ret)
> > -   return 0;
> > -   }
> > -   } else {
> > -   spin_unlock(ptl);
> > -   }
> > +   ptl = pmd_trans_huge_lock(pmd, vma);
> > +   if (ptl) {
> > +   ret = queue_pages_pmd(pmd, ptl, addr, end, walk);
> > +   if (ret)
> > +   return 0;
> > }
> 
> I wonder if we should introduce pte_entry function along with pmd_entry
> function as we are first looking for trans huge PMDs either for direct
> addition into the migration list or splitting it before looking for PTEs.

Most of pagewalk users don't define pte_entry because of performance reason
(to avoid the overhead of PTRS_PER_PMD function calls).
But that could be a nice cleanup if we have a workaround.

Thanks,
Naoya Horiguchi

Re: linux-next: build failure after merge of the crypto tree

[Herbert Xu]

On Tue, Nov 29, 2016 at 11:55:29AM +1100, Stephen Rothwell wrote:
> Hi Herbert,
> 
> After merging the crypto tree, today's linux-next build (arm
> multi_v7_defconfig) failed like this:
> 
> ERROR: "simd_skcipher_free" [arch/arm/crypto/aes-arm-ce.ko] undefined!
> ERROR: "simd_skcipher_create_compat" [arch/arm/crypto/aes-arm-ce.ko] 
> undefined!
> ERROR: "simd_skcipher_free" [arch/arm/crypto/aes-arm-bs.ko] undefined!
> ERROR: "simd_skcipher_create_compat" [arch/arm/crypto/aes-arm-bs.ko] 
> undefined!
> 
> Caused by commits
> 
>   da40e7a4ba4d ("crypto: aes-ce - Convert to skcipher")
>   211f41af534a ("crypto: aesbs - Convert to skcipher")
> 
> Missing dependencies?
> 
> I have used the crypto tree from next-20161128 for today.

Indeed.  This patch should fix the problem.

---8<---
Subject: crypto: arm/aes - Select SIMD in Kconfig

The skcipher conversion for ARM missed the select on CRYPTO_SIMD,
causing build failures if SIMD was not otherwise enabled.

Fixes: da40e7a4ba4d ("crypto: aes-ce - Convert to skcipher")
Fixes: 211f41af534a ("crypto: aesbs - Convert to skcipher")
Reported-by: Stephen Rothwell <s...@canb.auug.org.au>
Signed-off-by: Herbert Xu <herb...@gondor.apana.org.au>

diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
index 27ed1b1..8134888 100644
--- a/arch/arm/crypto/Kconfig
+++ b/arch/arm/crypto/Kconfig
@@ -105,7 +105,7 @@ config CRYPTO_AES_ARM_CE
tristate "Accelerated AES using ARMv8 Crypto Extensions"
depends on KERNEL_MODE_NEON
select CRYPTO_ALGAPI
-   select CRYPTO_ABLK_HELPER
+   select CRYPTO_SIMD
help
  Use an implementation of AES in CBC, CTR and XTS modes that uses
  ARMv8 Crypto Extensions
diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
index 5f4a617..c5ce39c 100644
--- a/arch/arm64/crypto/Kconfig
+++ b/arch/arm64/crypto/Kconfig
@@ -48,14 +48,14 @@ config CRYPTO_AES_ARM64_CE_BLK
depends on ARM64 && KERNEL_MODE_NEON
select CRYPTO_BLKCIPHER
select CRYPTO_AES_ARM64_CE
-   select CRYPTO_ABLK_HELPER
+   select CRYPTO_SIMD
 
 config CRYPTO_AES_ARM64_NEON_BLK
tristate "AES in ECB/CBC/CTR/XTS modes using NEON instructions"
depends on ARM64 && KERNEL_MODE_NEON
select CRYPTO_BLKCIPHER
select CRYPTO_AES
-   select CRYPTO_ABLK_HELPER
+   select CRYPTO_SIMD
 
 config CRYPTO_CRC32_ARM64
tristate "CRC32 and CRC32C using optional ARMv8 instructions"
-- 
Email: Herbert Xu <herb...@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: linux-next: build failure after merge of the crypto tree

[Herbert Xu]

On Tue, Nov 29, 2016 at 11:55:29AM +1100, Stephen Rothwell wrote:
> Hi Herbert,
> 
> After merging the crypto tree, today's linux-next build (arm
> multi_v7_defconfig) failed like this:
> 
> ERROR: "simd_skcipher_free" [arch/arm/crypto/aes-arm-ce.ko] undefined!
> ERROR: "simd_skcipher_create_compat" [arch/arm/crypto/aes-arm-ce.ko] 
> undefined!
> ERROR: "simd_skcipher_free" [arch/arm/crypto/aes-arm-bs.ko] undefined!
> ERROR: "simd_skcipher_create_compat" [arch/arm/crypto/aes-arm-bs.ko] 
> undefined!
> 
> Caused by commits
> 
>   da40e7a4ba4d ("crypto: aes-ce - Convert to skcipher")
>   211f41af534a ("crypto: aesbs - Convert to skcipher")
> 
> Missing dependencies?
> 
> I have used the crypto tree from next-20161128 for today.

Indeed.  This patch should fix the problem.

---8<---
Subject: crypto: arm/aes - Select SIMD in Kconfig

The skcipher conversion for ARM missed the select on CRYPTO_SIMD,
causing build failures if SIMD was not otherwise enabled.

Fixes: da40e7a4ba4d ("crypto: aes-ce - Convert to skcipher")
Fixes: 211f41af534a ("crypto: aesbs - Convert to skcipher")
Reported-by: Stephen Rothwell 
Signed-off-by: Herbert Xu 

diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
index 27ed1b1..8134888 100644
--- a/arch/arm/crypto/Kconfig
+++ b/arch/arm/crypto/Kconfig
@@ -105,7 +105,7 @@ config CRYPTO_AES_ARM_CE
tristate "Accelerated AES using ARMv8 Crypto Extensions"
depends on KERNEL_MODE_NEON
select CRYPTO_ALGAPI
-   select CRYPTO_ABLK_HELPER
+   select CRYPTO_SIMD
help
  Use an implementation of AES in CBC, CTR and XTS modes that uses
  ARMv8 Crypto Extensions
diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
index 5f4a617..c5ce39c 100644
--- a/arch/arm64/crypto/Kconfig
+++ b/arch/arm64/crypto/Kconfig
@@ -48,14 +48,14 @@ config CRYPTO_AES_ARM64_CE_BLK
depends on ARM64 && KERNEL_MODE_NEON
select CRYPTO_BLKCIPHER
select CRYPTO_AES_ARM64_CE
-   select CRYPTO_ABLK_HELPER
+   select CRYPTO_SIMD
 
 config CRYPTO_AES_ARM64_NEON_BLK
tristate "AES in ECB/CBC/CTR/XTS modes using NEON instructions"
depends on ARM64 && KERNEL_MODE_NEON
select CRYPTO_BLKCIPHER
select CRYPTO_AES
-   select CRYPTO_ABLK_HELPER
+   select CRYPTO_SIMD
 
 config CRYPTO_CRC32_ARM64
tristate "CRC32 and CRC32C using optional ARMv8 instructions"
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH V4] i2c: mux: pca954x: Add ACPI support for pca954x

[Tin Huynh]

This patch enables ACPI support for mux-pca954x driver.

Signed-off-by: Tin Huynh 

Change from v1 :
 -Don't shadow id variable.
 -Include sorted header.
 -Redefine acpi_device_id.
 -Add CONFIG_ACPI.
---
 drivers/i2c/muxes/i2c-mux-pca954x.c |   27 ++-
 1 files changed, 26 insertions(+), 1 deletions(-)

Change from V3:
 -Readd CONFIG_ACPI.

Change from V2:
 -Remove CONFIG_ACPI.

Change from V1 :
 -Don't shadow id variable.
 -Include sorted header.
 -Redefine acpi_device_id.
 -Add CONFIG_ACPI.


diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c 
b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 1091346..bdf4315 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -35,6 +35,7 @@
  * warranty of any kind, whether express or implied.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -120,6 +121,21 @@ struct pca954x {
 };
 MODULE_DEVICE_TABLE(i2c, pca954x_id);
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id pca954x_acpi_ids[] = {
+   { .id = "PCA9540", .driver_data = pca_9540 },
+   { .id = "PCA9542", .driver_data = pca_9540 },
+   { .id = "PCA9543", .driver_data = pca_9543 },
+   { .id = "PCA9544", .driver_data = pca_9544 },
+   { .id = "PCA9545", .driver_data = pca_9545 },
+   { .id = "PCA9546", .driver_data = pca_9545 },
+   { .id = "PCA9547", .driver_data = pca_9547 },
+   { .id = "PCA9548", .driver_data = pca_9548 },
+   { }
+};
+MODULE_DEVICE_TABLE(acpi, pca954x_acpi_ids);
+#endif
+
 #ifdef CONFIG_OF
 static const struct of_device_id pca954x_of_match[] = {
{ .compatible = "nxp,pca9540", .data = [pca_9540] },
@@ -245,8 +261,16 @@ static int pca954x_probe(struct i2c_client *client,
match = of_match_device(of_match_ptr(pca954x_of_match), >dev);
if (match)
data->chip = of_device_get_match_data(>dev);
-   else
+   else if (id)
data->chip = [id->driver_data];
+   else {
+   const struct acpi_device_id *acpi_id;
+
+   acpi_id = acpi_match_device(pca954x_acpi_ids, >dev);
+   if (!acpi_id)
+   return -ENODEV;
+   data->chip = [acpi_id->driver_data];
+   }
 
data->last_chan = 0;   /* force the first selection */
 
@@ -321,6 +345,7 @@ static int pca954x_resume(struct device *dev)
.name   = "pca954x",
.pm = _pm,
.of_match_table = of_match_ptr(pca954x_of_match),
+   .acpi_match_table = ACPI_PTR(pca954x_acpi_ids),
},
.probe  = pca954x_probe,
.remove = pca954x_remove,
-- 
1.7.1

[PATCH V4] i2c: mux: pca954x: Add ACPI support for pca954x

[Tin Huynh]

This patch enables ACPI support for mux-pca954x driver.

Signed-off-by: Tin Huynh 

Change from v1 :
 -Don't shadow id variable.
 -Include sorted header.
 -Redefine acpi_device_id.
 -Add CONFIG_ACPI.
---
 drivers/i2c/muxes/i2c-mux-pca954x.c |   27 ++-
 1 files changed, 26 insertions(+), 1 deletions(-)

Change from V3:
 -Readd CONFIG_ACPI.

Change from V2:
 -Remove CONFIG_ACPI.

Change from V1 :
 -Don't shadow id variable.
 -Include sorted header.
 -Redefine acpi_device_id.
 -Add CONFIG_ACPI.


diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c 
b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 1091346..bdf4315 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -35,6 +35,7 @@
  * warranty of any kind, whether express or implied.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -120,6 +121,21 @@ struct pca954x {
 };
 MODULE_DEVICE_TABLE(i2c, pca954x_id);
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id pca954x_acpi_ids[] = {
+   { .id = "PCA9540", .driver_data = pca_9540 },
+   { .id = "PCA9542", .driver_data = pca_9540 },
+   { .id = "PCA9543", .driver_data = pca_9543 },
+   { .id = "PCA9544", .driver_data = pca_9544 },
+   { .id = "PCA9545", .driver_data = pca_9545 },
+   { .id = "PCA9546", .driver_data = pca_9545 },
+   { .id = "PCA9547", .driver_data = pca_9547 },
+   { .id = "PCA9548", .driver_data = pca_9548 },
+   { }
+};
+MODULE_DEVICE_TABLE(acpi, pca954x_acpi_ids);
+#endif
+
 #ifdef CONFIG_OF
 static const struct of_device_id pca954x_of_match[] = {
{ .compatible = "nxp,pca9540", .data = [pca_9540] },
@@ -245,8 +261,16 @@ static int pca954x_probe(struct i2c_client *client,
match = of_match_device(of_match_ptr(pca954x_of_match), >dev);
if (match)
data->chip = of_device_get_match_data(>dev);
-   else
+   else if (id)
data->chip = [id->driver_data];
+   else {
+   const struct acpi_device_id *acpi_id;
+
+   acpi_id = acpi_match_device(pca954x_acpi_ids, >dev);
+   if (!acpi_id)
+   return -ENODEV;
+   data->chip = [acpi_id->driver_data];
+   }
 
data->last_chan = 0;   /* force the first selection */
 
@@ -321,6 +345,7 @@ static int pca954x_resume(struct device *dev)
.name   = "pca954x",
.pm = _pm,
.of_match_table = of_match_ptr(pca954x_of_match),
+   .acpi_match_table = ACPI_PTR(pca954x_acpi_ids),
},
.probe  = pca954x_probe,
.remove = pca954x_remove,
-- 
1.7.1

[PATCH v5 2/2] idle: add support for tasks that inject idle

[Jacob Pan]

From: Peter Zijlstra 

Idle injection drivers such as Intel powerclamp and ACPI PAD drivers use
realtime tasks to take control of CPU then inject idle. There are two
issues with this approach:

 1. Low efficiency: injected idle task is treated as busy so sched ticks
do not stop during injected idle period, the result of these
unwanted wakeups can be ~20% loss in power savings.

 2. Idle accounting: injected idle time is presented to user as busy.

This patch addresses the issues by introducing a new PF_IDLE flag which
allows any given task to be treated as idle task while the flag is set.
Therefore, idle injection tasks can run through the normal flow of NOHZ
idle enter/exit to get the correct accounting as well as tick stop when
possible.

The implication is that idle task is then no longer limited to PID == 0.

Acked-by: Ingo Molnar 
Signed-off-by: Peter Zijlstra 
Signed-off-by: Jacob Pan 
---
 include/linux/cpu.h   |   2 +
 include/linux/sched.h |   3 +-
 kernel/fork.c |   2 +-
 kernel/sched/core.c   |   1 +
 kernel/sched/idle.c   | 162 +++---
 5 files changed, 107 insertions(+), 63 deletions(-)

diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index b886dc1..ac0efae 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -245,6 +245,8 @@ static inline void enable_nonboot_cpus(void) {}
 int cpu_report_state(int cpu);
 int cpu_check_up_prepare(int cpu);
 void cpu_set_state_online(int cpu);
+void play_idle(unsigned long duration_ms);
+
 #ifdef CONFIG_HOTPLUG_CPU
 bool cpu_wait_death(unsigned int cpu, int seconds);
 bool cpu_report_death(void);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index e9c009d..a3d338e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2254,6 +2254,7 @@ static inline cputime_t task_gtime(struct task_struct *t)
 /*
  * Per process flags
  */
+#define PF_IDLE0x0002  /* I am an IDLE thread */
 #define PF_EXITING 0x0004  /* getting shut down */
 #define PF_EXITPIDONE  0x0008  /* pi exit done on shut down */
 #define PF_VCPU0x0010  /* I'm a virtual CPU */
@@ -2611,7 +2612,7 @@ extern int sched_setattr(struct task_struct *,
  */
 static inline bool is_idle_task(const struct task_struct *p)
 {
-   return p->pid == 0;
+   return !!(p->flags & PF_IDLE);
 }
 extern struct task_struct *curr_task(int cpu);
 extern void ia64_set_curr_task(int cpu, struct task_struct *p);
diff --git a/kernel/fork.c b/kernel/fork.c
index 997ac1d..a8eb821 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1540,7 +1540,7 @@ static __latent_entropy struct task_struct *copy_process(
goto bad_fork_cleanup_count;
 
delayacct_tsk_init(p);  /* Must remain after dup_task_struct() */
-   p->flags &= ~(PF_SUPERPRIV | PF_WQ_WORKER);
+   p->flags &= ~(PF_SUPERPRIV | PF_WQ_WORKER | PF_IDLE);
p->flags |= PF_FORKNOEXEC;
INIT_LIST_HEAD(>children);
INIT_LIST_HEAD(>sibling);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 154fd68..c95fbcd 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5279,6 +5279,7 @@ void init_idle(struct task_struct *idle, int cpu)
__sched_fork(0, idle);
idle->state = TASK_RUNNING;
idle->se.exec_start = sched_clock();
+   idle->flags |= PF_IDLE;
 
kasan_unpoison_task_stack(idle);
 
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index 513e4df..6a4bae0 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -205,76 +205,65 @@ static void cpuidle_idle_call(void)
  *
  * Called with polling cleared.
  */
-static void cpu_idle_loop(void)
+static void do_idle(void)
 {
-   int cpu = smp_processor_id();
-
-   while (1) {
-   /*
-* If the arch has a polling bit, we maintain an invariant:
-*
-* Our polling bit is clear if we're not scheduled (i.e. if
-* rq->curr != rq->idle).  This means that, if rq->idle has
-* the polling bit set, then setting need_resched is
-* guaranteed to cause the cpu to reschedule.
-*/
-
-   __current_set_polling();
-   quiet_vmstat();
-   tick_nohz_idle_enter();
+   /*
+* If the arch has a polling bit, we maintain an invariant:
+*
+* Our polling bit is clear if we're not scheduled (i.e. if rq->curr !=
+* rq->idle). This means that, if rq->idle has the polling bit set,
+* then setting need_resched is guaranteed to cause the CPU to
+* reschedule.
+*/
 
-   while (!need_resched()) {
-   check_pgt_cache();
-   rmb();
+   __current_set_polling();
+   tick_nohz_idle_enter();
 
-   if 

AMD Bulldozer topology regression since 4.6

[Brice Goglin]

Hello

Since Linux 4.6 (and still in 4.9-rc5 at least), both AMD Bulldozer
cores of a single dual-core compute unit report the same core_id:

$ cat /sys/devices/system/cpu/cpu{?,??}/topology/core_id
0
0
1
1
2
2
3
0
3
[...]


Before 4.5 (and for a very long time), the kernel reported different
core_ids:

0
1
2
3
4
5
6
7
0
[...]


This causes user-space tools (at least all that rely on hwloc for
topology discovery) to think the processor has dual-threaded cores
instead of dual-core compute-unit modules.

The cause is likely this patch, which seems to assume both cores of a
same compute-unit have the same ID, which is wrong?

commit 8196dab4fc159943df6baaac04973bb1accb7100
Author: Borislav Petkov 
Date:   Fri Mar 25 15:52:36 2016 +0100

x86/cpu: Get rid of compute_unit_id

It is cpu_core_id anyway.

Signed-off-by: Borislav Petkov 
Link: http://lkml.kernel.org/r/1458917557-8757-3-git-send-email...@alien8.de
Signed-off-by: Thomas Gleixner 



Thanks
Brice
 

[PATCH v5 2/2] idle: add support for tasks that inject idle

[Jacob Pan]

From: Peter Zijlstra 

Idle injection drivers such as Intel powerclamp and ACPI PAD drivers use
realtime tasks to take control of CPU then inject idle. There are two
issues with this approach:

 1. Low efficiency: injected idle task is treated as busy so sched ticks
do not stop during injected idle period, the result of these
unwanted wakeups can be ~20% loss in power savings.

 2. Idle accounting: injected idle time is presented to user as busy.

This patch addresses the issues by introducing a new PF_IDLE flag which
allows any given task to be treated as idle task while the flag is set.
Therefore, idle injection tasks can run through the normal flow of NOHZ
idle enter/exit to get the correct accounting as well as tick stop when
possible.

The implication is that idle task is then no longer limited to PID == 0.

Acked-by: Ingo Molnar 
Signed-off-by: Peter Zijlstra 
Signed-off-by: Jacob Pan 
---
 include/linux/cpu.h   |   2 +
 include/linux/sched.h |   3 +-
 kernel/fork.c |   2 +-
 kernel/sched/core.c   |   1 +
 kernel/sched/idle.c   | 162 +++---
 5 files changed, 107 insertions(+), 63 deletions(-)

diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index b886dc1..ac0efae 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -245,6 +245,8 @@ static inline void enable_nonboot_cpus(void) {}
 int cpu_report_state(int cpu);
 int cpu_check_up_prepare(int cpu);
 void cpu_set_state_online(int cpu);
+void play_idle(unsigned long duration_ms);
+
 #ifdef CONFIG_HOTPLUG_CPU
 bool cpu_wait_death(unsigned int cpu, int seconds);
 bool cpu_report_death(void);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index e9c009d..a3d338e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2254,6 +2254,7 @@ static inline cputime_t task_gtime(struct task_struct *t)
 /*
  * Per process flags
  */
+#define PF_IDLE0x0002  /* I am an IDLE thread */
 #define PF_EXITING 0x0004  /* getting shut down */
 #define PF_EXITPIDONE  0x0008  /* pi exit done on shut down */
 #define PF_VCPU0x0010  /* I'm a virtual CPU */
@@ -2611,7 +2612,7 @@ extern int sched_setattr(struct task_struct *,
  */
 static inline bool is_idle_task(const struct task_struct *p)
 {
-   return p->pid == 0;
+   return !!(p->flags & PF_IDLE);
 }
 extern struct task_struct *curr_task(int cpu);
 extern void ia64_set_curr_task(int cpu, struct task_struct *p);
diff --git a/kernel/fork.c b/kernel/fork.c
index 997ac1d..a8eb821 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1540,7 +1540,7 @@ static __latent_entropy struct task_struct *copy_process(
goto bad_fork_cleanup_count;
 
delayacct_tsk_init(p);  /* Must remain after dup_task_struct() */
-   p->flags &= ~(PF_SUPERPRIV | PF_WQ_WORKER);
+   p->flags &= ~(PF_SUPERPRIV | PF_WQ_WORKER | PF_IDLE);
p->flags |= PF_FORKNOEXEC;
INIT_LIST_HEAD(>children);
INIT_LIST_HEAD(>sibling);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 154fd68..c95fbcd 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5279,6 +5279,7 @@ void init_idle(struct task_struct *idle, int cpu)
__sched_fork(0, idle);
idle->state = TASK_RUNNING;
idle->se.exec_start = sched_clock();
+   idle->flags |= PF_IDLE;
 
kasan_unpoison_task_stack(idle);
 
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index 513e4df..6a4bae0 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -205,76 +205,65 @@ static void cpuidle_idle_call(void)
  *
  * Called with polling cleared.
  */
-static void cpu_idle_loop(void)
+static void do_idle(void)
 {
-   int cpu = smp_processor_id();
-
-   while (1) {
-   /*
-* If the arch has a polling bit, we maintain an invariant:
-*
-* Our polling bit is clear if we're not scheduled (i.e. if
-* rq->curr != rq->idle).  This means that, if rq->idle has
-* the polling bit set, then setting need_resched is
-* guaranteed to cause the cpu to reschedule.
-*/
-
-   __current_set_polling();
-   quiet_vmstat();
-   tick_nohz_idle_enter();
+   /*
+* If the arch has a polling bit, we maintain an invariant:
+*
+* Our polling bit is clear if we're not scheduled (i.e. if rq->curr !=
+* rq->idle). This means that, if rq->idle has the polling bit set,
+* then setting need_resched is guaranteed to cause the CPU to
+* reschedule.
+*/
 
-   while (!need_resched()) {
-   check_pgt_cache();
-   rmb();
+   __current_set_polling();
+   tick_nohz_idle_enter();
 
-   if (cpu_is_offline(cpu)) {
-   cpuhp_report_idle_dead();
-   

AMD Bulldozer topology regression since 4.6

[Brice Goglin]

Hello

Since Linux 4.6 (and still in 4.9-rc5 at least), both AMD Bulldozer
cores of a single dual-core compute unit report the same core_id:

$ cat /sys/devices/system/cpu/cpu{?,??}/topology/core_id
0
0
1
1
2
2
3
0
3
[...]


Before 4.5 (and for a very long time), the kernel reported different
core_ids:

0
1
2
3
4
5
6
7
0
[...]


This causes user-space tools (at least all that rely on hwloc for
topology discovery) to think the processor has dual-threaded cores
instead of dual-core compute-unit modules.

The cause is likely this patch, which seems to assume both cores of a
same compute-unit have the same ID, which is wrong?

commit 8196dab4fc159943df6baaac04973bb1accb7100
Author: Borislav Petkov 
Date:   Fri Mar 25 15:52:36 2016 +0100

x86/cpu: Get rid of compute_unit_id

It is cpu_core_id anyway.

Signed-off-by: Borislav Petkov 
Link: http://lkml.kernel.org/r/1458917557-8757-3-git-send-email...@alien8.de
Signed-off-by: Thomas Gleixner 



Thanks
Brice
 

[PATCH v5 1/2] cpuidle: allow setting deepest idle

[Jacob Pan]

When idle injection is used to cap power, we need to override
governor's choice of idle states. This patch allows caller to select
the deepest idle state on a CPU therefore achieve the maximum
potential power saving.

Signed-off-by: Jacob Pan 
---
 drivers/cpuidle/cpuidle.c | 13 -
 include/linux/cpuidle.h   |  7 ++-
 kernel/sched/idle.c   | 13 -
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index c73207a..afc005b 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -97,7 +97,17 @@ static int find_deepest_state(struct cpuidle_driver *drv,
return ret;
 }
 
-#ifdef CONFIG_SUSPEND
+/* Set the current cpu to use the deepest idle state, override governors */
+void cpuidle_use_deepest_state(bool enable)
+{
+   struct cpuidle_device *dev;
+
+   preempt_disable();
+   dev = cpuidle_get_device();
+   dev->use_deepest_state = enable;
+   preempt_enable();
+}
+
 /**
  * cpuidle_find_deepest_state - Find the deepest available idle state.
  * @drv: cpuidle driver for the given CPU.
@@ -109,6 +119,7 @@ int cpuidle_find_deepest_state(struct cpuidle_driver *drv,
return find_deepest_state(drv, dev, UINT_MAX, 0, false);
 }
 
+#ifdef CONFIG_SUSPEND
 static void enter_freeze_proper(struct cpuidle_driver *drv,
struct cpuidle_device *dev, int index)
 {
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index bb31373..63bf385 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -74,6 +74,7 @@ struct cpuidle_state {
 struct cpuidle_device {
unsigned intregistered:1;
unsigned intenabled:1;
+   unsigned intuse_deepest_state:1;
unsigned intcpu;
 
int last_residency;
@@ -192,11 +193,12 @@ static inline struct cpuidle_driver 
*cpuidle_get_cpu_driver(
 static inline struct cpuidle_device *cpuidle_get_device(void) {return NULL; }
 #endif
 
-#if defined(CONFIG_CPU_IDLE) && defined(CONFIG_SUSPEND)
+#ifdef CONFIG_CPU_IDLE
 extern int cpuidle_find_deepest_state(struct cpuidle_driver *drv,
  struct cpuidle_device *dev);
 extern int cpuidle_enter_freeze(struct cpuidle_driver *drv,
struct cpuidle_device *dev);
+extern void cpuidle_use_deepest_state(bool enable);
 #else
 static inline int cpuidle_find_deepest_state(struct cpuidle_driver *drv,
 struct cpuidle_device *dev)
@@ -204,6 +206,9 @@ static inline int cpuidle_find_deepest_state(struct 
cpuidle_driver *drv,
 static inline int cpuidle_enter_freeze(struct cpuidle_driver *drv,
   struct cpuidle_device *dev)
 {return -ENODEV; }
+static inline void cpuidle_use_deepest_state(bool enable)
+{
+}
 #endif
 
 /* kernel/sched/idle.c */
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index 1d8718d..513e4df 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -164,11 +164,14 @@ static void cpuidle_idle_call(void)
 * timekeeping to prevent timer interrupts from kicking us out of idle
 * until a proper wakeup interrupt happens.
 */
-   if (idle_should_freeze()) {
-   entered_state = cpuidle_enter_freeze(drv, dev);
-   if (entered_state > 0) {
-   local_irq_enable();
-   goto exit_idle;
+
+   if (idle_should_freeze() || dev->use_deepest_state) {
+   if (idle_should_freeze()) {
+   entered_state = cpuidle_enter_freeze(drv, dev);
+   if (entered_state > 0) {
+   local_irq_enable();
+   goto exit_idle;
+   }
}
 
next_state = cpuidle_find_deepest_state(drv, dev);
-- 
1.9.1

[PATCH v5 1/2] cpuidle: allow setting deepest idle

[Jacob Pan]

When idle injection is used to cap power, we need to override
governor's choice of idle states. This patch allows caller to select
the deepest idle state on a CPU therefore achieve the maximum
potential power saving.

Signed-off-by: Jacob Pan 
---
 drivers/cpuidle/cpuidle.c | 13 -
 include/linux/cpuidle.h   |  7 ++-
 kernel/sched/idle.c   | 13 -
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index c73207a..afc005b 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -97,7 +97,17 @@ static int find_deepest_state(struct cpuidle_driver *drv,
return ret;
 }
 
-#ifdef CONFIG_SUSPEND
+/* Set the current cpu to use the deepest idle state, override governors */
+void cpuidle_use_deepest_state(bool enable)
+{
+   struct cpuidle_device *dev;
+
+   preempt_disable();
+   dev = cpuidle_get_device();
+   dev->use_deepest_state = enable;
+   preempt_enable();
+}
+
 /**
  * cpuidle_find_deepest_state - Find the deepest available idle state.
  * @drv: cpuidle driver for the given CPU.
@@ -109,6 +119,7 @@ int cpuidle_find_deepest_state(struct cpuidle_driver *drv,
return find_deepest_state(drv, dev, UINT_MAX, 0, false);
 }
 
+#ifdef CONFIG_SUSPEND
 static void enter_freeze_proper(struct cpuidle_driver *drv,
struct cpuidle_device *dev, int index)
 {
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index bb31373..63bf385 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -74,6 +74,7 @@ struct cpuidle_state {
 struct cpuidle_device {
unsigned intregistered:1;
unsigned intenabled:1;
+   unsigned intuse_deepest_state:1;
unsigned intcpu;
 
int last_residency;
@@ -192,11 +193,12 @@ static inline struct cpuidle_driver 
*cpuidle_get_cpu_driver(
 static inline struct cpuidle_device *cpuidle_get_device(void) {return NULL; }
 #endif
 
-#if defined(CONFIG_CPU_IDLE) && defined(CONFIG_SUSPEND)
+#ifdef CONFIG_CPU_IDLE
 extern int cpuidle_find_deepest_state(struct cpuidle_driver *drv,
  struct cpuidle_device *dev);
 extern int cpuidle_enter_freeze(struct cpuidle_driver *drv,
struct cpuidle_device *dev);
+extern void cpuidle_use_deepest_state(bool enable);
 #else
 static inline int cpuidle_find_deepest_state(struct cpuidle_driver *drv,
 struct cpuidle_device *dev)
@@ -204,6 +206,9 @@ static inline int cpuidle_find_deepest_state(struct 
cpuidle_driver *drv,
 static inline int cpuidle_enter_freeze(struct cpuidle_driver *drv,
   struct cpuidle_device *dev)
 {return -ENODEV; }
+static inline void cpuidle_use_deepest_state(bool enable)
+{
+}
 #endif
 
 /* kernel/sched/idle.c */
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index 1d8718d..513e4df 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -164,11 +164,14 @@ static void cpuidle_idle_call(void)
 * timekeeping to prevent timer interrupts from kicking us out of idle
 * until a proper wakeup interrupt happens.
 */
-   if (idle_should_freeze()) {
-   entered_state = cpuidle_enter_freeze(drv, dev);
-   if (entered_state > 0) {
-   local_irq_enable();
-   goto exit_idle;
+
+   if (idle_should_freeze() || dev->use_deepest_state) {
+   if (idle_should_freeze()) {
+   entered_state = cpuidle_enter_freeze(drv, dev);
+   if (entered_state > 0) {
+   local_irq_enable();
+   goto exit_idle;
+   }
}
 
next_state = cpuidle_find_deepest_state(drv, dev);
-- 
1.9.1

[PATCH v5 0/2] Stop sched tick in idle injection task

[Jacob Pan]

Changelog:
v5: - Fix compile error in cpuidle patch reported by 0-day.
- Reverse patch order for correct dependency.

v4: - Misc comments from Ingo are addressed, including style fix,
  timeout handling, fork().
- Dropped powerclamp patch from this set, move to its own
  series.

v3: - Rearrange idle.c change based on Rafael's suggestion.

v2:
- Moved duration timer from powerclamp driver to play_idle()
- Unexport cpuidle_use_deepest_state
- Indentation fix

Idle injection drivers today use RT threads to run idle loop. There are
efficiency and accounting issues with the current intel_powerclamp.c
and acpi_pad.c. A while ago, I posted CFS based idle injection patch trying
to address them:
https://lkml.org/lkml/2015/11/13/576

Peter proposed another approach with the introduction of a PF_IDLE flag.
This patchset is based on his original posting:
https://lkml.org/lkml/2014/6/4/56

These patches apply on top of the kworker and cpu hotplug state machine
changes made to Intel powerclamp driver.
https://lkml.org/lkml/2016/10/17/362

Similar changes to ACPI PAD driver is developed along with other
enhancements. It will be posted after this patchset is accepted.


Jacob Pan (1):
  cpuidle: allow setting deepest idle

Peter Zijlstra (1):
  idle: add support for tasks that inject idle

 drivers/cpuidle/cpuidle.c |  13 +++-
 include/linux/cpu.h   |   2 +
 include/linux/cpuidle.h   |   7 +-
 include/linux/sched.h |   3 +-
 kernel/fork.c |   2 +-
 kernel/sched/core.c   |   1 +
 kernel/sched/idle.c   | 175 +-
 7 files changed, 133 insertions(+), 70 deletions(-)

-- 
1.9.1

[PATCH v5 0/2] Stop sched tick in idle injection task

[Jacob Pan]

Changelog:
v5: - Fix compile error in cpuidle patch reported by 0-day.
- Reverse patch order for correct dependency.

v4: - Misc comments from Ingo are addressed, including style fix,
  timeout handling, fork().
- Dropped powerclamp patch from this set, move to its own
  series.

v3: - Rearrange idle.c change based on Rafael's suggestion.

v2:
- Moved duration timer from powerclamp driver to play_idle()
- Unexport cpuidle_use_deepest_state
- Indentation fix

Idle injection drivers today use RT threads to run idle loop. There are
efficiency and accounting issues with the current intel_powerclamp.c
and acpi_pad.c. A while ago, I posted CFS based idle injection patch trying
to address them:
https://lkml.org/lkml/2015/11/13/576

Peter proposed another approach with the introduction of a PF_IDLE flag.
This patchset is based on his original posting:
https://lkml.org/lkml/2014/6/4/56

These patches apply on top of the kworker and cpu hotplug state machine
changes made to Intel powerclamp driver.
https://lkml.org/lkml/2016/10/17/362

Similar changes to ACPI PAD driver is developed along with other
enhancements. It will be posted after this patchset is accepted.


Jacob Pan (1):
  cpuidle: allow setting deepest idle

Peter Zijlstra (1):
  idle: add support for tasks that inject idle

 drivers/cpuidle/cpuidle.c |  13 +++-
 include/linux/cpu.h   |   2 +
 include/linux/cpuidle.h   |   7 +-
 include/linux/sched.h |   3 +-
 kernel/fork.c |   2 +-
 kernel/sched/core.c   |   1 +
 kernel/sched/idle.c   | 175 +-
 7 files changed, 133 insertions(+), 70 deletions(-)

-- 
1.9.1

Re: Re: [PATCH] Input: joystick: adi - change msleep to usleep_range for small msecs

[vojt...@ucw.cz]

On Mon, Nov 28, 2016 at 01:49:31PM +, Aniroop Mathur wrote:
> Hello Mr. Vojtech Pavlik,
> 
> On 28 Nov 2016 17:23, "vojt...@ucw.cz"  wrote:
>  >
>  > Hi.
>  >
>  > ADI_INIT_DELAY/ADI_DATA_DELAY doesn't have to be exact, and a longer
>  > sleep doesn't matter. In the initilization sequence - first chunk of
>  > your patch - a way too long delay could in theory make the device fail
>  > to initialize. What's critical is that the mdelay() calls are precise.
>  >
>  > One day I'll open my box of old joystick and re-test these drivers to
>  > see if they survived the years of kernel infrastructure updates ...
>  >
>  > Vojtech
>  >
>  
>  Well, it seems to me that there is some kind of confusion, so I'd like to
>  clarify things about this patch.
>  As you have mentioned that in the initialization sequence, long delay could
>  in theory make the device fail to initialize - This patch actually solves
>  this problem.
>  msleep is built on jiffies / legacy timers and usleep_range is built on top
>  of hrtimers so the wakeup will be precise.
>  Source - https://www.kernel.org/doc/Documentation/timers/timers-howto.txt
> 
>  For example in initialization sequence, if we use msleep(4), then the process
>  could sleep for much more than 4 ms, say 6 ms or 10 ms or more depending on
>  machine architecture. Like on a machine with tick rate / HZ is defined to be
>  100 so msleep(4) will make the process to sleep for minimum 10 ms. 
>  Whereas usleep_range(4000, 4100) will make sure that the process do not sleep
>  for more than 4100 us or 4.1 ms. So usleep_range is precise but msleep is 
> not.
>  
>  Originally, I added you in this patch to request you if you could help to
>  test this patch or provide contact points of individuals who could help
>  to test this patch as we do not have the hardware available with us?
>  Like this driver, we also need to test other joystick drivers as well like
>  gf2k.c, analog.c, sidewinder.c and gameport driver ns558.c as all have
>  similar problem.
>  Original patch link - https://patchwork.kernel.org/patch/9446201/
>  As we do not have hardware available, so we decided to split patch into
>  individual drivers and request to person who could support to test this patch
> 
>  I have also applied the same patch in our driver whose hardware is available
>  with me and I found that wake up time became precise indeed and so I
>  decided to apply the same fix in other input subsystem drivers as well.

I do understand what you're trying to achieve. Both ADI_DATA_DELAY and
ADI_INIT_DELAY are specified as minimum delays. Waiting longer doesn't
cause any trouble, so the patch doesn't need to change that.

In the initialization sequence, it probably doesn't matter either
whether we wait longer, hence the distinction between msleep() and
mdelay() based on positive/negative numbers. The mdelay() needs to be
exact and the msleep() can be longer. How much longer before it disturbs
the init sequence I'm not sure, probably quite a bit.

The driver was written a long time before hrtimers existed and as such
it was written expecting that msleep() can take a longer time.

So your patch is most likely not needed, but I should find an ADI device
and see what happens if I make the sleeps in the init sequence much
longer.

It'd also be interesting to see if the mdelay()s could be replaced with
hrtimer-based delays instead, as that would be nicer to the system - if
they can be precise enough. Also, preemption and maybe interrupts should
be disabled around the mdelays I suppose - that was not an issue when
the drivers were written.

Vojtech

>  
>  Thank you!
>  
>  BR,
>  Aniroop Mathur
> 
>  > On Mon, Nov 28, 2016 at 11:43:56AM +, Aniroop Mathur wrote:
>  > > msleep(1~20) may not do what the caller intends, and will often sleep 
> longer.
>  > > (~20 ms actual sleep for any value given in the 1~20ms range)
>  > > This is not the desired behaviour for many cases like device resume time,
>  > > device suspend time, device enable time, data reading time, etc.
>  > > Thus, change msleep to usleep_range for precise wakeups.
>  > >
>  > > Signed-off-by: Aniroop Mathur 
>  > > ---
>  > >  joystick/adi.c | 10 +-
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
>  > >
>  > > diff --git a/joystick/adi.c b/joystick/adi.c
>  > > index d09cefa..6799bd9 100644
>  > > --- a/joystick/adi.c
>  > > +++ b/joystick/adi.c
>  > > @@ -47,8 +47,8 @@ MODULE_LICENSE("GPL");
>  > >
>  > >  #define ADI_MAX_START200 /* Trigger to packet 
> timeout [200us] */
>  > >  #define ADI_MAX_STROBE   40  /* Single bit timeout 
> [40us] */
>  > > -#define ADI_INIT_DELAY   10  /* Delay after init packet 
> [10ms] */
>  > > -#define ADI_DATA_DELAY   4   /* Delay after data packet 
> [4ms] */
>  > > +#define ADI_INIT_DELAY   1   /* Delay after init packet 
> [10ms] */
>  > > +#define ADI_DATA_DELAY 

Re: Re: [PATCH] Input: joystick: adi - change msleep to usleep_range for small msecs

[vojt...@ucw.cz]

On Mon, Nov 28, 2016 at 01:49:31PM +, Aniroop Mathur wrote:
> Hello Mr. Vojtech Pavlik,
> 
> On 28 Nov 2016 17:23, "vojt...@ucw.cz"  wrote:
>  >
>  > Hi.
>  >
>  > ADI_INIT_DELAY/ADI_DATA_DELAY doesn't have to be exact, and a longer
>  > sleep doesn't matter. In the initilization sequence - first chunk of
>  > your patch - a way too long delay could in theory make the device fail
>  > to initialize. What's critical is that the mdelay() calls are precise.
>  >
>  > One day I'll open my box of old joystick and re-test these drivers to
>  > see if they survived the years of kernel infrastructure updates ...
>  >
>  > Vojtech
>  >
>  
>  Well, it seems to me that there is some kind of confusion, so I'd like to
>  clarify things about this patch.
>  As you have mentioned that in the initialization sequence, long delay could
>  in theory make the device fail to initialize - This patch actually solves
>  this problem.
>  msleep is built on jiffies / legacy timers and usleep_range is built on top
>  of hrtimers so the wakeup will be precise.
>  Source - https://www.kernel.org/doc/Documentation/timers/timers-howto.txt
> 
>  For example in initialization sequence, if we use msleep(4), then the process
>  could sleep for much more than 4 ms, say 6 ms or 10 ms or more depending on
>  machine architecture. Like on a machine with tick rate / HZ is defined to be
>  100 so msleep(4) will make the process to sleep for minimum 10 ms. 
>  Whereas usleep_range(4000, 4100) will make sure that the process do not sleep
>  for more than 4100 us or 4.1 ms. So usleep_range is precise but msleep is 
> not.
>  
>  Originally, I added you in this patch to request you if you could help to
>  test this patch or provide contact points of individuals who could help
>  to test this patch as we do not have the hardware available with us?
>  Like this driver, we also need to test other joystick drivers as well like
>  gf2k.c, analog.c, sidewinder.c and gameport driver ns558.c as all have
>  similar problem.
>  Original patch link - https://patchwork.kernel.org/patch/9446201/
>  As we do not have hardware available, so we decided to split patch into
>  individual drivers and request to person who could support to test this patch
> 
>  I have also applied the same patch in our driver whose hardware is available
>  with me and I found that wake up time became precise indeed and so I
>  decided to apply the same fix in other input subsystem drivers as well.

I do understand what you're trying to achieve. Both ADI_DATA_DELAY and
ADI_INIT_DELAY are specified as minimum delays. Waiting longer doesn't
cause any trouble, so the patch doesn't need to change that.

In the initialization sequence, it probably doesn't matter either
whether we wait longer, hence the distinction between msleep() and
mdelay() based on positive/negative numbers. The mdelay() needs to be
exact and the msleep() can be longer. How much longer before it disturbs
the init sequence I'm not sure, probably quite a bit.

The driver was written a long time before hrtimers existed and as such
it was written expecting that msleep() can take a longer time.

So your patch is most likely not needed, but I should find an ADI device
and see what happens if I make the sleeps in the init sequence much
longer.

It'd also be interesting to see if the mdelay()s could be replaced with
hrtimer-based delays instead, as that would be nicer to the system - if
they can be precise enough. Also, preemption and maybe interrupts should
be disabled around the mdelays I suppose - that was not an issue when
the drivers were written.

Vojtech

>  
>  Thank you!
>  
>  BR,
>  Aniroop Mathur
> 
>  > On Mon, Nov 28, 2016 at 11:43:56AM +, Aniroop Mathur wrote:
>  > > msleep(1~20) may not do what the caller intends, and will often sleep 
> longer.
>  > > (~20 ms actual sleep for any value given in the 1~20ms range)
>  > > This is not the desired behaviour for many cases like device resume time,
>  > > device suspend time, device enable time, data reading time, etc.
>  > > Thus, change msleep to usleep_range for precise wakeups.
>  > >
>  > > Signed-off-by: Aniroop Mathur 
>  > > ---
>  > >  joystick/adi.c | 10 +-
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
>  > >
>  > > diff --git a/joystick/adi.c b/joystick/adi.c
>  > > index d09cefa..6799bd9 100644
>  > > --- a/joystick/adi.c
>  > > +++ b/joystick/adi.c
>  > > @@ -47,8 +47,8 @@ MODULE_LICENSE("GPL");
>  > >
>  > >  #define ADI_MAX_START200 /* Trigger to packet 
> timeout [200us] */
>  > >  #define ADI_MAX_STROBE   40  /* Single bit timeout 
> [40us] */
>  > > -#define ADI_INIT_DELAY   10  /* Delay after init packet 
> [10ms] */
>  > > -#define ADI_DATA_DELAY   4   /* Delay after data packet 
> [4ms] */
>  > > +#define ADI_INIT_DELAY   1   /* Delay after init packet 
> [10ms] */
>  > > +#define ADI_DATA_DELAY   4000/* Delay after data 

[PATCH v1 2/4] perf report: Create a new option "--inline"

[Jin Yao]

It takes some time to look for inline stack for callgraph addresses.
So it provides a new option "--inline" to let user decide if enable
this feature.

Signed-off-by: Jin Yao 
---
 tools/perf/Documentation/perf-report.txt | 4 
 tools/perf/builtin-report.c  | 2 ++
 tools/perf/util/symbol.h | 3 ++-
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-report.txt 
b/tools/perf/Documentation/perf-report.txt
index 2d17462..f1299a7 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -412,6 +412,10 @@ include::itrace.txt[]
 --hierarchy::
Enable hierarchical output.
 
+--inline::
+   If a callgraph address belongs to an inlined function, the inline stack
+   will be printed.
+
 include::callchain-overhead-calculation.txt[]
 
 SEE ALSO
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 3dfbfff..ba2f627 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -830,6 +830,8 @@ int cmd_report(int argc, const char **argv, const char 
*prefix __maybe_unused)
OPT_CALLBACK_DEFAULT(0, "stdio-color", NULL, "mode",
 "'always' (default), 'never' or 'auto' only 
applicable to --stdio mode",
 stdio__config_color, "always"),
+   OPT_BOOLEAN(0, "inline", _conf.show_inline,
+   "Show inline functions"),
OPT_END()
};
struct perf_data_file file = {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 1bcbefc..c312759 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -118,7 +118,8 @@ struct symbol_conf {
show_ref_callgraph,
hide_unresolved,
raw_trace,
-   report_hierarchy;
+   report_hierarchy,
+   show_inline;
const char  *vmlinux_name,
*kallsyms_name,
*source_prefix,
-- 
2.7.4

Re: [PATCH 1/2] PM / Domains: Introduce domain-performance-state binding

[Viresh Kumar]

On 28-11-16, 10:27, Stephen Boyd wrote:
> On 11/23/2016 08:40 PM, Viresh Kumar wrote:
> > But even in these cases we wouldn't be using the voltage values within the
> > kernel as we will be giving only a performance state to the M3 core, right?
> 
> Nope. In these cases we need to set a certain voltage and we do that by
> requesting it via the M3 core.

Don't we need something like this then ?

parent: power-controller@1234 {
compatible = "foo,power-controller";
reg = <0x1234 0x1000>;
#power-domain-cells = <0>;
domain-performance-states = <_state0>;
};

perf_state0: performance_states {
pstate1: pstate@1 {
index = <1>;
/* Optional */
microvolt = <97 975000 985000>;
};
pstate2: pstate@2 {
index = <2>;
/* Optional */
microvolt = <97 975000 985000>;
};
pstate3: pstate@3 {
index = <3>;
/* Optional */
microvolt = <97 975000 985000>;
};
}

cpus {
cpu@0 {
...
power-domain = <>;
operating-points-v2 = <_opp_table>;
};
};

cpu0_opp_table: opp_table0 {
compatible = "operating-points-v2";
opp-shared;

opp@10 {
opp-hz = /bits/ 64 <10>;
domain-performance-state = <>;
};
opp@11 {
opp-hz = /bits/ 64 <11>;
domain-performance-state = <>;
};
opp@12 {
opp-hz = /bits/ 64 <12>;
domain-performance-state = <>;
};
};

-- 
viresh

[PATCH v1 2/4] perf report: Create a new option "--inline"

[Jin Yao]

It takes some time to look for inline stack for callgraph addresses.
So it provides a new option "--inline" to let user decide if enable
this feature.

Signed-off-by: Jin Yao 
---
 tools/perf/Documentation/perf-report.txt | 4 
 tools/perf/builtin-report.c  | 2 ++
 tools/perf/util/symbol.h | 3 ++-
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-report.txt 
b/tools/perf/Documentation/perf-report.txt
index 2d17462..f1299a7 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -412,6 +412,10 @@ include::itrace.txt[]
 --hierarchy::
Enable hierarchical output.
 
+--inline::
+   If a callgraph address belongs to an inlined function, the inline stack
+   will be printed.
+
 include::callchain-overhead-calculation.txt[]
 
 SEE ALSO
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 3dfbfff..ba2f627 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -830,6 +830,8 @@ int cmd_report(int argc, const char **argv, const char 
*prefix __maybe_unused)
OPT_CALLBACK_DEFAULT(0, "stdio-color", NULL, "mode",
 "'always' (default), 'never' or 'auto' only 
applicable to --stdio mode",
 stdio__config_color, "always"),
+   OPT_BOOLEAN(0, "inline", _conf.show_inline,
+   "Show inline functions"),
OPT_END()
};
struct perf_data_file file = {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 1bcbefc..c312759 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -118,7 +118,8 @@ struct symbol_conf {
show_ref_callgraph,
hide_unresolved,
raw_trace,
-   report_hierarchy;
+   report_hierarchy,
+   show_inline;
const char  *vmlinux_name,
*kallsyms_name,
*source_prefix,
-- 
2.7.4

Re: [PATCH 1/2] PM / Domains: Introduce domain-performance-state binding

[Viresh Kumar]

On 28-11-16, 10:27, Stephen Boyd wrote:
> On 11/23/2016 08:40 PM, Viresh Kumar wrote:
> > But even in these cases we wouldn't be using the voltage values within the
> > kernel as we will be giving only a performance state to the M3 core, right?
> 
> Nope. In these cases we need to set a certain voltage and we do that by
> requesting it via the M3 core.

Don't we need something like this then ?

parent: power-controller@1234 {
compatible = "foo,power-controller";
reg = <0x1234 0x1000>;
#power-domain-cells = <0>;
domain-performance-states = <_state0>;
};

perf_state0: performance_states {
pstate1: pstate@1 {
index = <1>;
/* Optional */
microvolt = <97 975000 985000>;
};
pstate2: pstate@2 {
index = <2>;
/* Optional */
microvolt = <97 975000 985000>;
};
pstate3: pstate@3 {
index = <3>;
/* Optional */
microvolt = <97 975000 985000>;
};
}

cpus {
cpu@0 {
...
power-domain = <>;
operating-points-v2 = <_opp_table>;
};
};

cpu0_opp_table: opp_table0 {
compatible = "operating-points-v2";
opp-shared;

opp@10 {
opp-hz = /bits/ 64 <10>;
domain-performance-state = <>;
};
opp@11 {
opp-hz = /bits/ 64 <11>;
domain-performance-state = <>;
};
opp@12 {
opp-hz = /bits/ 64 <12>;
domain-performance-state = <>;
};
};

-- 
viresh

[PATCH v1 3/4] perf report: Show inline stack in stdio mode

[Jin Yao]

If the address belongs to an inlined function, the source information
back to the first non-inlined function will be printed.

For example:

0.05%  test2test2  [.] main
   |
   ---/home/jinyao/perf-dev/test/test2.c:27 (inline)
  /home/jinyao/perf-dev/test/test2.c:35 (inline)
  /home/jinyao/perf-dev/test/test2.c:45 (inline)
  /home/jinyao/perf-dev/test/test2.c:61 (inline)

The tag "inline" indicates these items are the entries in inline stack.

Signed-off-by: Jin Yao 
---
 tools/perf/ui/stdio/hist.c | 56 +-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 668f4ae..e74eda0 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -400,6 +400,53 @@ static size_t hist_entry_callchain__fprintf(struct 
hist_entry *he,
return 0;
 }
 
+static size_t hist_entry_inline__fprintf(struct hist_entry *he,
+int left_margin,
+FILE *fp)
+{
+   struct dso *dso;
+   struct inline_node *node;
+   struct inline_list *ilist;
+   int ret = 0, i = 0;
+
+   if (he->ms.map == NULL)
+   return 0;
+
+   dso = he->ms.map->dso;
+   if (dso == NULL)
+   return 0;
+
+   if (dso->kernel != DSO_TYPE_USER)
+   return 0;
+
+   node = get_inline_node(dso, map__rip_2objdump(he->ms.map, he->ip));
+   if (node == NULL)
+   return 0;
+
+   ret += callchain__fprintf_left_margin(fp, left_margin);
+   ret += fprintf(fp, "|\n");
+   ret += callchain__fprintf_left_margin(fp, left_margin);
+   ret += fprintf(fp, "---");
+   left_margin += 3;
+
+   list_for_each_entry(ilist, >val, list) {
+   if (ilist->filename != NULL) {
+   if (i++ > 0)
+   ret = callchain__fprintf_left_margin(fp,
+   left_margin);
+   ret += fprintf(fp, "%s:%d (inline)",
+  ilist->filename, ilist->line_nr);
+   ret += fprintf(fp, "\n");
+   }
+   }
+
+   if (i > 0)
+   ret += fprintf(fp, "\n");
+
+   free_inline_node(node);
+   return ret;
+}
+
 int __hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp,
   struct perf_hpp_list *hpp_list)
 {
@@ -529,6 +576,7 @@ static int hist_entry__fprintf(struct hist_entry *he, 
size_t size,
   bool use_callchain)
 {
int ret;
+   int callchain_ret = 0;
struct perf_hpp hpp = {
.buf= bf,
.size   = size,
@@ -547,7 +595,13 @@ static int hist_entry__fprintf(struct hist_entry *he, 
size_t size,
ret = fprintf(fp, "%s\n", bf);
 
if (use_callchain)
-   ret += hist_entry_callchain__fprintf(he, total_period, 0, fp);
+   callchain_ret = hist_entry_callchain__fprintf(he, total_period,
+ 0, fp);
+
+   if ((callchain_ret == 0) && symbol_conf.show_inline)
+   ret += hist_entry_inline__fprintf(he, 0, fp);
+   else
+   ret += callchain_ret;
 
return ret;
 }
-- 
2.7.4

[PATCH v1 3/4] perf report: Show inline stack in stdio mode

[Jin Yao]

If the address belongs to an inlined function, the source information
back to the first non-inlined function will be printed.

For example:

0.05%  test2test2  [.] main
   |
   ---/home/jinyao/perf-dev/test/test2.c:27 (inline)
  /home/jinyao/perf-dev/test/test2.c:35 (inline)
  /home/jinyao/perf-dev/test/test2.c:45 (inline)
  /home/jinyao/perf-dev/test/test2.c:61 (inline)

The tag "inline" indicates these items are the entries in inline stack.

Signed-off-by: Jin Yao 
---
 tools/perf/ui/stdio/hist.c | 56 +-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 668f4ae..e74eda0 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -400,6 +400,53 @@ static size_t hist_entry_callchain__fprintf(struct 
hist_entry *he,
return 0;
 }
 
+static size_t hist_entry_inline__fprintf(struct hist_entry *he,
+int left_margin,
+FILE *fp)
+{
+   struct dso *dso;
+   struct inline_node *node;
+   struct inline_list *ilist;
+   int ret = 0, i = 0;
+
+   if (he->ms.map == NULL)
+   return 0;
+
+   dso = he->ms.map->dso;
+   if (dso == NULL)
+   return 0;
+
+   if (dso->kernel != DSO_TYPE_USER)
+   return 0;
+
+   node = get_inline_node(dso, map__rip_2objdump(he->ms.map, he->ip));
+   if (node == NULL)
+   return 0;
+
+   ret += callchain__fprintf_left_margin(fp, left_margin);
+   ret += fprintf(fp, "|\n");
+   ret += callchain__fprintf_left_margin(fp, left_margin);
+   ret += fprintf(fp, "---");
+   left_margin += 3;
+
+   list_for_each_entry(ilist, >val, list) {
+   if (ilist->filename != NULL) {
+   if (i++ > 0)
+   ret = callchain__fprintf_left_margin(fp,
+   left_margin);
+   ret += fprintf(fp, "%s:%d (inline)",
+  ilist->filename, ilist->line_nr);
+   ret += fprintf(fp, "\n");
+   }
+   }
+
+   if (i > 0)
+   ret += fprintf(fp, "\n");
+
+   free_inline_node(node);
+   return ret;
+}
+
 int __hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp,
   struct perf_hpp_list *hpp_list)
 {
@@ -529,6 +576,7 @@ static int hist_entry__fprintf(struct hist_entry *he, 
size_t size,
   bool use_callchain)
 {
int ret;
+   int callchain_ret = 0;
struct perf_hpp hpp = {
.buf= bf,
.size   = size,
@@ -547,7 +595,13 @@ static int hist_entry__fprintf(struct hist_entry *he, 
size_t size,
ret = fprintf(fp, "%s\n", bf);
 
if (use_callchain)
-   ret += hist_entry_callchain__fprintf(he, total_period, 0, fp);
+   callchain_ret = hist_entry_callchain__fprintf(he, total_period,
+ 0, fp);
+
+   if ((callchain_ret == 0) && symbol_conf.show_inline)
+   ret += hist_entry_inline__fprintf(he, 0, fp);
+   else
+   ret += callchain_ret;
 
return ret;
 }
-- 
2.7.4

[PATCH v1 4/4] perf report: Show inline stack in browser mode

[Jin Yao]

For example:

-0.05%  test2test2  [.] main
 /home/jinyao/perf-dev/test/test2.c:27 (inline)
 /home/jinyao/perf-dev/test/test2.c:35 (inline)
 /home/jinyao/perf-dev/test/test2.c:45 (inline)
 /home/jinyao/perf-dev/test/test2.c:61 (inline)

Signed-off-by: Jin Yao 
---
 tools/perf/ui/browsers/hists.c | 98 --
 tools/perf/util/hist.c |  5 +++
 tools/perf/util/sort.h |  1 +
 3 files changed, 100 insertions(+), 4 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 641b402..c8c3cae 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -362,6 +362,46 @@ static void hist_entry__init_have_children(struct 
hist_entry *he)
he->init_have_children = true;
 }
 
+static void hist_entry_init_inline_node(struct hist_entry *he)
+{
+   struct dso *dso;
+
+   if (he->inline_node)
+   return;
+
+   if (he->ms.map == NULL)
+   return;
+
+   dso = he->ms.map->dso;
+   if (dso == NULL)
+   return;
+
+   if (dso->kernel != DSO_TYPE_USER)
+   return;
+
+   he->inline_node = get_inline_node(dso,
+   map__rip_2objdump(he->ms.map, he->ip));
+
+   if (he->inline_node == NULL)
+   return;
+
+   he->has_children = true;
+}
+
+static int inline__count_rows(struct hist_entry *he)
+{
+   struct inline_node *node = he->inline_node;
+   struct inline_list *ilist;
+   int i = 0;
+
+   list_for_each_entry(ilist, >val, list) {
+   if (ilist->filename != NULL)
+   i++;
+   }
+
+   return i;
+}
+
 static bool hist_browser__toggle_fold(struct hist_browser *browser)
 {
struct hist_entry *he = browser->he_selection;
@@ -393,7 +433,11 @@ static bool hist_browser__toggle_fold(struct hist_browser 
*browser)
 
if (he->unfolded) {
if (he->leaf)
-   he->nr_rows = 
callchain__count_rows(>sorted_chain);
+   if (he->inline_node)
+   he->nr_rows = inline__count_rows(he);
+   else
+   he->nr_rows = callchain__count_rows(
+   >sorted_chain);
else
he->nr_rows = hierarchy_count_rows(browser, he, 
false);
 
@@ -1091,6 +1135,40 @@ static int hist_browser__show_callchain(struct 
hist_browser *browser,
return printed;
 }
 
+static int hist_browser__show_inline(struct hist_browser *browser,
+struct hist_entry *entry,
+unsigned short row)
+{
+   struct inline_node *node;
+   struct inline_list *ilist;
+   char buf[1024];
+   int color, width, first_row;
+
+   first_row = row;
+   node = entry->inline_node;
+   width = browser->b.width - (LEVEL_OFFSET_STEP + 2);
+
+   list_for_each_entry(ilist, >val, list) {
+   if (ilist->filename != NULL) {
+   color = HE_COLORSET_NORMAL;
+   if (ui_browser__is_current_entry(>b, row))
+   color = HE_COLORSET_SELECTED;
+
+   scnprintf(buf, sizeof(buf), "%s:%d (inline)",
+   ilist->filename, ilist->line_nr);
+
+   ui_browser__set_color(>b, color);
+   hist_browser__gotorc(browser, row, 0);
+   ui_browser__write_nstring(>b, " ",
+   LEVEL_OFFSET_STEP + 2);
+   ui_browser__write_nstring(>b, buf, width);
+   row++;
+   }
+   }
+
+   return row - first_row;
+}
+
 struct hpp_arg {
struct ui_browser *b;
char folded_sign;
@@ -1204,6 +1282,11 @@ static int hist_browser__show_entry(struct hist_browser 
*browser,
folded_sign = hist_entry__folded(entry);
}
 
+   if (symbol_conf.show_inline && (!entry->has_children)) {
+   hist_entry_init_inline_node(entry);
+   folded_sign = hist_entry__folded(entry);
+   }
+
if (row_offset == 0) {
struct hpp_arg arg = {
.b  = >b,
@@ -1235,7 +1318,8 @@ static int hist_browser__show_entry(struct hist_browser 
*browser,
}
 
if (first) {
-   if (symbol_conf.use_callchain) {
+   if (symbol_conf.use_callchain ||
+   symbol_conf.show_inline) {
ui_browser__printf(>b, "%c ", 
folded_sign);
width -= 2;
}
@@ -1277,8 

[PATCH v1 4/4] perf report: Show inline stack in browser mode

[Jin Yao]

For example:

-0.05%  test2test2  [.] main
 /home/jinyao/perf-dev/test/test2.c:27 (inline)
 /home/jinyao/perf-dev/test/test2.c:35 (inline)
 /home/jinyao/perf-dev/test/test2.c:45 (inline)
 /home/jinyao/perf-dev/test/test2.c:61 (inline)

Signed-off-by: Jin Yao 
---
 tools/perf/ui/browsers/hists.c | 98 --
 tools/perf/util/hist.c |  5 +++
 tools/perf/util/sort.h |  1 +
 3 files changed, 100 insertions(+), 4 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 641b402..c8c3cae 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -362,6 +362,46 @@ static void hist_entry__init_have_children(struct 
hist_entry *he)
he->init_have_children = true;
 }
 
+static void hist_entry_init_inline_node(struct hist_entry *he)
+{
+   struct dso *dso;
+
+   if (he->inline_node)
+   return;
+
+   if (he->ms.map == NULL)
+   return;
+
+   dso = he->ms.map->dso;
+   if (dso == NULL)
+   return;
+
+   if (dso->kernel != DSO_TYPE_USER)
+   return;
+
+   he->inline_node = get_inline_node(dso,
+   map__rip_2objdump(he->ms.map, he->ip));
+
+   if (he->inline_node == NULL)
+   return;
+
+   he->has_children = true;
+}
+
+static int inline__count_rows(struct hist_entry *he)
+{
+   struct inline_node *node = he->inline_node;
+   struct inline_list *ilist;
+   int i = 0;
+
+   list_for_each_entry(ilist, >val, list) {
+   if (ilist->filename != NULL)
+   i++;
+   }
+
+   return i;
+}
+
 static bool hist_browser__toggle_fold(struct hist_browser *browser)
 {
struct hist_entry *he = browser->he_selection;
@@ -393,7 +433,11 @@ static bool hist_browser__toggle_fold(struct hist_browser 
*browser)
 
if (he->unfolded) {
if (he->leaf)
-   he->nr_rows = 
callchain__count_rows(>sorted_chain);
+   if (he->inline_node)
+   he->nr_rows = inline__count_rows(he);
+   else
+   he->nr_rows = callchain__count_rows(
+   >sorted_chain);
else
he->nr_rows = hierarchy_count_rows(browser, he, 
false);
 
@@ -1091,6 +1135,40 @@ static int hist_browser__show_callchain(struct 
hist_browser *browser,
return printed;
 }
 
+static int hist_browser__show_inline(struct hist_browser *browser,
+struct hist_entry *entry,
+unsigned short row)
+{
+   struct inline_node *node;
+   struct inline_list *ilist;
+   char buf[1024];
+   int color, width, first_row;
+
+   first_row = row;
+   node = entry->inline_node;
+   width = browser->b.width - (LEVEL_OFFSET_STEP + 2);
+
+   list_for_each_entry(ilist, >val, list) {
+   if (ilist->filename != NULL) {
+   color = HE_COLORSET_NORMAL;
+   if (ui_browser__is_current_entry(>b, row))
+   color = HE_COLORSET_SELECTED;
+
+   scnprintf(buf, sizeof(buf), "%s:%d (inline)",
+   ilist->filename, ilist->line_nr);
+
+   ui_browser__set_color(>b, color);
+   hist_browser__gotorc(browser, row, 0);
+   ui_browser__write_nstring(>b, " ",
+   LEVEL_OFFSET_STEP + 2);
+   ui_browser__write_nstring(>b, buf, width);
+   row++;
+   }
+   }
+
+   return row - first_row;
+}
+
 struct hpp_arg {
struct ui_browser *b;
char folded_sign;
@@ -1204,6 +1282,11 @@ static int hist_browser__show_entry(struct hist_browser 
*browser,
folded_sign = hist_entry__folded(entry);
}
 
+   if (symbol_conf.show_inline && (!entry->has_children)) {
+   hist_entry_init_inline_node(entry);
+   folded_sign = hist_entry__folded(entry);
+   }
+
if (row_offset == 0) {
struct hpp_arg arg = {
.b  = >b,
@@ -1235,7 +1318,8 @@ static int hist_browser__show_entry(struct hist_browser 
*browser,
}
 
if (first) {
-   if (symbol_conf.use_callchain) {
+   if (symbol_conf.use_callchain ||
+   symbol_conf.show_inline) {
ui_browser__printf(>b, "%c ", 
folded_sign);
width -= 2;
}
@@ -1277,8 +1361,14 @@ static int 

[PATCH v1 0/4] perf report: Show inline stack

[Jin Yao]

It would be useful for perf to support a mode to query the
inline stack for callgraph addresses. This would simplify
finding the right code in code that does a lot of inlining.

For example, the c code:

static inline void f3(void)
{
int i;
for (i = 0; i < 1000;) {

if(i%2)
i++;
else
i++;
}
printf("hello f3\n");   /* D */
}

/* < CALLCHAIN: f2 <- f1 > */
static inline void f2(void)
{
int i;
for (i = 0; i < 100; i++) {
f3();   /* C */
}
}

/* < CALLCHAIN: f1 <- main > */
static inline void f1(void)
{
int i;
for (i = 0; i < 100; i++) {
f2();   /* B */
}
}

/* < CALLCHAIN: main <- TOP > */
int main()
{
struct timeval tv;
time_t start, end;

gettimeofday(, NULL);
start = end = tv.tv_sec;
while((end - start) < 5) {
f1();   /* A */
gettimeofday(, NULL);
end = tv.tv_sec;
}
return 0;
}

The printed inline stack is:

0.05%  test2test2  [.] main
   |
   ---/home/perf-dev/lck-2867/test/test2.c:27 (inline)
  /home/perf-dev/lck-2867/test/test2.c:35 (inline)
  /home/perf-dev/lck-2867/test/test2.c:45 (inline)
  /home/perf-dev/lck-2867/test/test2.c:61 (inline)

I tag A/B/C/D in above c code to indicate the source line,
actually the inline stack is equal to:

0.05%  test2test2  [.] main
   |
   ---D
  C
  B
  A

Jin Yao (4):
  perf report: Find the inline stack for a given address
  perf report: Create a new option "--inline"
  perf report: Show inline stack in stdio mode
  perf report: Show inline stack in browser mode

 tools/perf/Documentation/perf-report.txt |   4 +
 tools/perf/builtin-report.c  |   2 +
 tools/perf/ui/browsers/hists.c   |  98 ++-
 tools/perf/ui/stdio/hist.c   |  56 -
 tools/perf/util/hist.c   |   5 +
 tools/perf/util/sort.h   |   1 +
 tools/perf/util/srcline.c| 206 ++-
 tools/perf/util/symbol.h |   3 +-
 tools/perf/util/util.h   |  15 +++
 9 files changed, 356 insertions(+), 34 deletions(-)

-- 
2.7.4

[PATCH v1 1/4] perf report: Find the inline stack for a given address

[Jin Yao]

It would be useful for perf to support a mode to query the
inline stack for a given callgraph address. This would simplify
finding the right code in code that does a lot of inlining.

The srcline.c has contained the code which supports to translate
the address to filename:line_nr. This patch just extends the
function to let it support getting the inline stacks.

The results (filename:line_nr) would be saved in a list and
returned to the caller.

Signed-off-by: Jin Yao 
---
 tools/perf/util/srcline.c | 206 +++---
 tools/perf/util/util.h|  15 
 2 files changed, 193 insertions(+), 28 deletions(-)

diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index b4db3f4..0145625 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -12,6 +12,39 @@
 
 bool srcline_full_filename;
 
+static const char *dso_name_get(struct dso *dso)
+{
+   const char *dso_name;
+
+   if (dso->symsrc_filename)
+   dso_name = dso->symsrc_filename;
+   else
+   dso_name = dso->long_name;
+
+   if (dso_name[0] == '[')
+   return NULL;
+
+   if (!strncmp(dso_name, "/tmp/perf-", 10))
+   return NULL;
+
+   return dso_name;
+}
+
+static int ilist_apend(char *filename, int line_nr, struct inline_node *node)
+{
+   struct inline_list *ilist;
+
+   ilist = zalloc(sizeof(*ilist));
+   if (ilist == NULL)
+   return -1;
+
+   ilist->filename = filename;
+   ilist->line_nr = line_nr;
+   list_add_tail(>list, >val);
+
+   return 0;
+}
+
 #ifdef HAVE_LIBBFD_SUPPORT
 
 /*
@@ -153,7 +186,7 @@ static void addr2line_cleanup(struct a2l_data *a2l)
 
 static int addr2line(const char *dso_name, u64 addr,
 char **file, unsigned int *line, struct dso *dso,
-bool unwind_inlines)
+bool unwind_inlines, struct inline_node *node)
 {
int ret = 0;
struct a2l_data *a2l = dso->a2l;
@@ -178,8 +211,14 @@ static int addr2line(const char *dso_name, u64 addr,
 
while (bfd_find_inliner_info(a2l->abfd, >filename,
 >funcname, >line) &&
-  cnt++ < MAX_INLINE_NEST)
-   ;
+  cnt++ < MAX_INLINE_NEST) {
+
+   if (node != NULL) {
+   if (ilist_apend(strdup(a2l->filename),
+   a2l->line, node) != 0)
+   return 0;
+   }
+   }
}
 
if (a2l->found && a2l->filename) {
@@ -205,18 +244,68 @@ void dso__free_a2l(struct dso *dso)
dso->a2l = NULL;
 }
 
+static struct inline_node *addr2inlines(const char *dso_name, u64 addr,
+   struct dso *dso)
+{
+   char *file = NULL;
+   unsigned int line = 0;
+   struct inline_node *node;
+
+   node = zalloc(sizeof(*node));
+   if (node == NULL) {
+   perror("not enough memory for the inline node");
+   return NULL;
+   }
+
+   INIT_LIST_HEAD(>val);
+   node->addr = addr;
+
+   if (!addr2line(dso_name, addr, , , dso, TRUE, node)) {
+   free_inline_node(node);
+   return NULL;
+   }
+
+   if (list_empty(>val)) {
+   free_inline_node(node);
+   return NULL;
+   }
+
+   return node;
+}
+
 #else /* HAVE_LIBBFD_SUPPORT */
 
+static int filename_split(const char *dso_name, char *filename,
+ unsigned int *line_nr)
+{
+   char *sep;
+
+   sep = strchr(filename, '\n');
+   if (sep)
+   *sep = '\0';
+
+   if (!strcmp(filename, "??:0"))
+   return -1;
+
+   sep = strchr(filename, ':');
+   if (sep) {
+   *sep++ = '\0';
+   *line_nr = strtoul(sep, NULL, 0);
+   }
+
+   return 0;
+}
+
 static int addr2line(const char *dso_name, u64 addr,
 char **file, unsigned int *line_nr,
 struct dso *dso __maybe_unused,
-bool unwind_inlines __maybe_unused)
+bool unwind_inlines __maybe_unused,
+struct inline_node *node __maybe_unused)
 {
FILE *fp;
char cmd[PATH_MAX];
char *filename = NULL;
size_t len;
-   char *sep;
int ret = 0;
 
scnprintf(cmd, sizeof(cmd), "addr2line -e %s %016"PRIx64,
@@ -233,23 +322,14 @@ static int addr2line(const char *dso_name, u64 addr,
goto out;
}
 
-   sep = strchr(filename, '\n');
-   if (sep)
-   *sep = '\0';
-
-   if (!strcmp(filename, "??:0")) {
-   pr_debug("no debugging info in %s\n", dso_name);
+   if (filename_split(dso_name, filename, line_nr) != 0) {
free(filename);
goto out;
}
 
-   

[PATCH v1 0/4] perf report: Show inline stack

[Jin Yao]

It would be useful for perf to support a mode to query the
inline stack for callgraph addresses. This would simplify
finding the right code in code that does a lot of inlining.

For example, the c code:

static inline void f3(void)
{
int i;
for (i = 0; i < 1000;) {

if(i%2)
i++;
else
i++;
}
printf("hello f3\n");   /* D */
}

/* < CALLCHAIN: f2 <- f1 > */
static inline void f2(void)
{
int i;
for (i = 0; i < 100; i++) {
f3();   /* C */
}
}

/* < CALLCHAIN: f1 <- main > */
static inline void f1(void)
{
int i;
for (i = 0; i < 100; i++) {
f2();   /* B */
}
}

/* < CALLCHAIN: main <- TOP > */
int main()
{
struct timeval tv;
time_t start, end;

gettimeofday(, NULL);
start = end = tv.tv_sec;
while((end - start) < 5) {
f1();   /* A */
gettimeofday(, NULL);
end = tv.tv_sec;
}
return 0;
}

The printed inline stack is:

0.05%  test2test2  [.] main
   |
   ---/home/perf-dev/lck-2867/test/test2.c:27 (inline)
  /home/perf-dev/lck-2867/test/test2.c:35 (inline)
  /home/perf-dev/lck-2867/test/test2.c:45 (inline)
  /home/perf-dev/lck-2867/test/test2.c:61 (inline)

I tag A/B/C/D in above c code to indicate the source line,
actually the inline stack is equal to:

0.05%  test2test2  [.] main
   |
   ---D
  C
  B
  A

Jin Yao (4):
  perf report: Find the inline stack for a given address
  perf report: Create a new option "--inline"
  perf report: Show inline stack in stdio mode
  perf report: Show inline stack in browser mode

 tools/perf/Documentation/perf-report.txt |   4 +
 tools/perf/builtin-report.c  |   2 +
 tools/perf/ui/browsers/hists.c   |  98 ++-
 tools/perf/ui/stdio/hist.c   |  56 -
 tools/perf/util/hist.c   |   5 +
 tools/perf/util/sort.h   |   1 +
 tools/perf/util/srcline.c| 206 ++-
 tools/perf/util/symbol.h |   3 +-
 tools/perf/util/util.h   |  15 +++
 9 files changed, 356 insertions(+), 34 deletions(-)

-- 
2.7.4

[PATCH v1 1/4] perf report: Find the inline stack for a given address

[Jin Yao]

It would be useful for perf to support a mode to query the
inline stack for a given callgraph address. This would simplify
finding the right code in code that does a lot of inlining.

The srcline.c has contained the code which supports to translate
the address to filename:line_nr. This patch just extends the
function to let it support getting the inline stacks.

The results (filename:line_nr) would be saved in a list and
returned to the caller.

Signed-off-by: Jin Yao 
---
 tools/perf/util/srcline.c | 206 +++---
 tools/perf/util/util.h|  15 
 2 files changed, 193 insertions(+), 28 deletions(-)

diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index b4db3f4..0145625 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -12,6 +12,39 @@
 
 bool srcline_full_filename;
 
+static const char *dso_name_get(struct dso *dso)
+{
+   const char *dso_name;
+
+   if (dso->symsrc_filename)
+   dso_name = dso->symsrc_filename;
+   else
+   dso_name = dso->long_name;
+
+   if (dso_name[0] == '[')
+   return NULL;
+
+   if (!strncmp(dso_name, "/tmp/perf-", 10))
+   return NULL;
+
+   return dso_name;
+}
+
+static int ilist_apend(char *filename, int line_nr, struct inline_node *node)
+{
+   struct inline_list *ilist;
+
+   ilist = zalloc(sizeof(*ilist));
+   if (ilist == NULL)
+   return -1;
+
+   ilist->filename = filename;
+   ilist->line_nr = line_nr;
+   list_add_tail(>list, >val);
+
+   return 0;
+}
+
 #ifdef HAVE_LIBBFD_SUPPORT
 
 /*
@@ -153,7 +186,7 @@ static void addr2line_cleanup(struct a2l_data *a2l)
 
 static int addr2line(const char *dso_name, u64 addr,
 char **file, unsigned int *line, struct dso *dso,
-bool unwind_inlines)
+bool unwind_inlines, struct inline_node *node)
 {
int ret = 0;
struct a2l_data *a2l = dso->a2l;
@@ -178,8 +211,14 @@ static int addr2line(const char *dso_name, u64 addr,
 
while (bfd_find_inliner_info(a2l->abfd, >filename,
 >funcname, >line) &&
-  cnt++ < MAX_INLINE_NEST)
-   ;
+  cnt++ < MAX_INLINE_NEST) {
+
+   if (node != NULL) {
+   if (ilist_apend(strdup(a2l->filename),
+   a2l->line, node) != 0)
+   return 0;
+   }
+   }
}
 
if (a2l->found && a2l->filename) {
@@ -205,18 +244,68 @@ void dso__free_a2l(struct dso *dso)
dso->a2l = NULL;
 }
 
+static struct inline_node *addr2inlines(const char *dso_name, u64 addr,
+   struct dso *dso)
+{
+   char *file = NULL;
+   unsigned int line = 0;
+   struct inline_node *node;
+
+   node = zalloc(sizeof(*node));
+   if (node == NULL) {
+   perror("not enough memory for the inline node");
+   return NULL;
+   }
+
+   INIT_LIST_HEAD(>val);
+   node->addr = addr;
+
+   if (!addr2line(dso_name, addr, , , dso, TRUE, node)) {
+   free_inline_node(node);
+   return NULL;
+   }
+
+   if (list_empty(>val)) {
+   free_inline_node(node);
+   return NULL;
+   }
+
+   return node;
+}
+
 #else /* HAVE_LIBBFD_SUPPORT */
 
+static int filename_split(const char *dso_name, char *filename,
+ unsigned int *line_nr)
+{
+   char *sep;
+
+   sep = strchr(filename, '\n');
+   if (sep)
+   *sep = '\0';
+
+   if (!strcmp(filename, "??:0"))
+   return -1;
+
+   sep = strchr(filename, ':');
+   if (sep) {
+   *sep++ = '\0';
+   *line_nr = strtoul(sep, NULL, 0);
+   }
+
+   return 0;
+}
+
 static int addr2line(const char *dso_name, u64 addr,
 char **file, unsigned int *line_nr,
 struct dso *dso __maybe_unused,
-bool unwind_inlines __maybe_unused)
+bool unwind_inlines __maybe_unused,
+struct inline_node *node __maybe_unused)
 {
FILE *fp;
char cmd[PATH_MAX];
char *filename = NULL;
size_t len;
-   char *sep;
int ret = 0;
 
scnprintf(cmd, sizeof(cmd), "addr2line -e %s %016"PRIx64,
@@ -233,23 +322,14 @@ static int addr2line(const char *dso_name, u64 addr,
goto out;
}
 
-   sep = strchr(filename, '\n');
-   if (sep)
-   *sep = '\0';
-
-   if (!strcmp(filename, "??:0")) {
-   pr_debug("no debugging info in %s\n", dso_name);
+   if (filename_split(dso_name, filename, line_nr) != 0) {
free(filename);
goto out;
}
 
-   sep = 

Re: [RFC 4/4] mm: Ignore cpuset enforcement when allocation flag has __GFP_THISNODE

[Anshuman Khandual]

On 11/29/2016 02:42 AM, Dave Hansen wrote:
> On 11/22/2016 06:19 AM, Anshuman Khandual wrote:
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -3715,7 +3715,7 @@ struct page *
>>  .migratetype = gfpflags_to_migratetype(gfp_mask),
>>  };
>>  
>> -if (cpusets_enabled()) {
>> +if (cpusets_enabled() && !(alloc_mask & __GFP_THISNODE)) {
>>  alloc_mask |= __GFP_HARDWALL;
>>  alloc_flags |= ALLOC_CPUSET;
>>  if (!ac.nodemask)
> 
> This means now that any __GFP_THISNODE allocation can "escape" the
> cpuset.  That seems like a pretty major change to how cpusets works.  Do
> we know that *ALL* __GFP_THISNODE allocations are truly lacking in a
> cpuset context that can be enforced?

Right, I know its a very blunt change. With the cpuset based isolation
of coherent device node for the user space tasks leads to a side effect
that a driver or even kernel cannot allocate memory from the coherent
device node in the task's own context (ioctl() calls or similar). For
non task context allocation (work queues, interrupts, anything async
etc) this problem can be fixed by modifying kernel thread's task->mems
_allowed to include all nodes of the system including the coherent
device nodes. Though I have not figured out the details yet. Whats
your thoughts on this ? What we are looking for is a explicit and
definite way of allocating from the coherent device node inside the
kernel.

Re: [RFC 4/4] mm: Ignore cpuset enforcement when allocation flag has __GFP_THISNODE

[Anshuman Khandual]

On 11/29/2016 02:42 AM, Dave Hansen wrote:
> On 11/22/2016 06:19 AM, Anshuman Khandual wrote:
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -3715,7 +3715,7 @@ struct page *
>>  .migratetype = gfpflags_to_migratetype(gfp_mask),
>>  };
>>  
>> -if (cpusets_enabled()) {
>> +if (cpusets_enabled() && !(alloc_mask & __GFP_THISNODE)) {
>>  alloc_mask |= __GFP_HARDWALL;
>>  alloc_flags |= ALLOC_CPUSET;
>>  if (!ac.nodemask)
> 
> This means now that any __GFP_THISNODE allocation can "escape" the
> cpuset.  That seems like a pretty major change to how cpusets works.  Do
> we know that *ALL* __GFP_THISNODE allocations are truly lacking in a
> cpuset context that can be enforced?

Right, I know its a very blunt change. With the cpuset based isolation
of coherent device node for the user space tasks leads to a side effect
that a driver or even kernel cannot allocate memory from the coherent
device node in the task's own context (ioctl() calls or similar). For
non task context allocation (work queues, interrupts, anything async
etc) this problem can be fixed by modifying kernel thread's task->mems
_allowed to include all nodes of the system including the coherent
device nodes. Though I have not figured out the details yet. Whats
your thoughts on this ? What we are looking for is a explicit and
definite way of allocating from the coherent device node inside the
kernel.

Re: [PATCH] Input: joystick: gf2k - change msleep to usleep_range for small msecs

[Vojtech Pavlik]

On Tue, Nov 29, 2016 at 01:11:49AM +0530, Aniroop Mathur wrote:

> msleep(1~20) may not do what the caller intends, and will often sleep longer.
> (~20 ms actual sleep for any value given in the 1~20ms range)
> This is not the desired behaviour for many cases like device resume time,
> device suspend time, device enable time, connection time, probe time,
> loops, retry logic, etc
> msleep is built on jiffies / legacy timers which are not precise whereas
> usleep_range is build on top of hrtimers so the wakeups are precise.
> Thus, change msleep to usleep_range for precise wakeups.
> 
> For example:
> On a machine with tick rate / HZ as 100, msleep(4) will make the process to
> sleep for a minimum period of 10 ms whereas usleep_range(4000, 4100) will make
> sure that the process does not sleep for more than 4100 us or 4.1ms

And once more, patch not needed.

> 
> Signed-off-by: Aniroop Mathur 
> ---
>  drivers/input/joystick/gf2k.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/joystick/gf2k.c b/drivers/input/joystick/gf2k.c
> index 0f519db..e9d5095 100644
> --- a/drivers/input/joystick/gf2k.c
> +++ b/drivers/input/joystick/gf2k.c
> @@ -42,7 +42,7 @@ MODULE_LICENSE("GPL");
>  
>  #define GF2K_START   400 /* The time we wait for the first bit 
> [400 us] */
>  #define GF2K_STROBE  40  /* The time we wait for the first bit 
> [40 us] */
> -#define GF2K_TIMEOUT 4   /* Wait for everything to settle [4 ms] 
> */
> +#define GF2K_TIMEOUT 4000/* Wait for everything to settle [4000 
> us] */
>  #define GF2K_LENGTH  80  /* Max number of triplets in a packet */
>  
>  /*
> @@ -138,7 +138,7 @@ static void gf2k_trigger_seq(struct gameport *gameport, 
> short *seq)
>   i = 0;
>  do {
>   gameport_trigger(gameport);
> - t = gameport_time(gameport, GF2K_TIMEOUT * 1000);
> + t = gameport_time(gameport, GF2K_TIMEOUT);
>   while ((gameport_read(gameport) & 1) && t) t--;
>  udelay(seq[i]);
>  } while (seq[++i]);
> @@ -259,11 +259,11 @@ static int gf2k_connect(struct gameport *gameport, 
> struct gameport_driver *drv)
>  
>   gf2k_trigger_seq(gameport, gf2k_seq_reset);
>  
> - msleep(GF2K_TIMEOUT);
> + usleep_range(GF2K_TIMEOUT, GF2K_TIMEOUT + 100);
>  
>   gf2k_trigger_seq(gameport, gf2k_seq_digital);
>  
> - msleep(GF2K_TIMEOUT);
> + usleep_range(GF2K_TIMEOUT, GF2K_TIMEOUT + 100);
>  
>   if (gf2k_read_packet(gameport, GF2K_LENGTH, data) < 12) {
>   err = -ENODEV;
> -- 
> 2.6.2
> 

-- 
Vojtech Pavlik

Re: [PATCH] Input: joystick: gf2k - change msleep to usleep_range for small msecs

[Vojtech Pavlik]

On Tue, Nov 29, 2016 at 01:11:49AM +0530, Aniroop Mathur wrote:

> msleep(1~20) may not do what the caller intends, and will often sleep longer.
> (~20 ms actual sleep for any value given in the 1~20ms range)
> This is not the desired behaviour for many cases like device resume time,
> device suspend time, device enable time, connection time, probe time,
> loops, retry logic, etc
> msleep is built on jiffies / legacy timers which are not precise whereas
> usleep_range is build on top of hrtimers so the wakeups are precise.
> Thus, change msleep to usleep_range for precise wakeups.
> 
> For example:
> On a machine with tick rate / HZ as 100, msleep(4) will make the process to
> sleep for a minimum period of 10 ms whereas usleep_range(4000, 4100) will make
> sure that the process does not sleep for more than 4100 us or 4.1ms

And once more, patch not needed.

> 
> Signed-off-by: Aniroop Mathur 
> ---
>  drivers/input/joystick/gf2k.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/joystick/gf2k.c b/drivers/input/joystick/gf2k.c
> index 0f519db..e9d5095 100644
> --- a/drivers/input/joystick/gf2k.c
> +++ b/drivers/input/joystick/gf2k.c
> @@ -42,7 +42,7 @@ MODULE_LICENSE("GPL");
>  
>  #define GF2K_START   400 /* The time we wait for the first bit 
> [400 us] */
>  #define GF2K_STROBE  40  /* The time we wait for the first bit 
> [40 us] */
> -#define GF2K_TIMEOUT 4   /* Wait for everything to settle [4 ms] 
> */
> +#define GF2K_TIMEOUT 4000/* Wait for everything to settle [4000 
> us] */
>  #define GF2K_LENGTH  80  /* Max number of triplets in a packet */
>  
>  /*
> @@ -138,7 +138,7 @@ static void gf2k_trigger_seq(struct gameport *gameport, 
> short *seq)
>   i = 0;
>  do {
>   gameport_trigger(gameport);
> - t = gameport_time(gameport, GF2K_TIMEOUT * 1000);
> + t = gameport_time(gameport, GF2K_TIMEOUT);
>   while ((gameport_read(gameport) & 1) && t) t--;
>  udelay(seq[i]);
>  } while (seq[++i]);
> @@ -259,11 +259,11 @@ static int gf2k_connect(struct gameport *gameport, 
> struct gameport_driver *drv)
>  
>   gf2k_trigger_seq(gameport, gf2k_seq_reset);
>  
> - msleep(GF2K_TIMEOUT);
> + usleep_range(GF2K_TIMEOUT, GF2K_TIMEOUT + 100);
>  
>   gf2k_trigger_seq(gameport, gf2k_seq_digital);
>  
> - msleep(GF2K_TIMEOUT);
> + usleep_range(GF2K_TIMEOUT, GF2K_TIMEOUT + 100);
>  
>   if (gf2k_read_packet(gameport, GF2K_LENGTH, data) < 12) {
>   err = -ENODEV;
> -- 
> 2.6.2
> 

-- 
Vojtech Pavlik

[PATCH] Staging: comedi: cb_pcidda: fixed a comment style issue

[Elias Carter]

Fixed a coding style issue

Signed-off-by: Elias Carter 
---
 drivers/staging/comedi/drivers/cb_pcidda.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/cb_pcidda.c 
b/drivers/staging/comedi/drivers/cb_pcidda.c
index ccb37d1..9874147 100644
--- a/drivers/staging/comedi/drivers/cb_pcidda.c
+++ b/drivers/staging/comedi/drivers/cb_pcidda.c
@@ -248,8 +248,8 @@ static void cb_pcidda_write_caldac(struct comedi_device 
*dev,
cb_pcidda_serial_out(dev, value, num_caldac_bits);
 
 /*
-* latch stream into appropriate caldac deselect reference dac
-*/
+ * latch stream into appropriate caldac deselect reference dac
+ */
cal2_bits = DESELECT_REF_DAC_BIT | DUMMY_BIT;
/*  deactivate caldacs (one caldac for every two channels) */
for (i = 0; i < max_num_caldacs; i++)
-- 
2.9.3

[PATCH] Staging: comedi: cb_pcidda: fixed a comment style issue

[Elias Carter]

Fixed a coding style issue

Signed-off-by: Elias Carter 
---
 drivers/staging/comedi/drivers/cb_pcidda.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/cb_pcidda.c 
b/drivers/staging/comedi/drivers/cb_pcidda.c
index ccb37d1..9874147 100644
--- a/drivers/staging/comedi/drivers/cb_pcidda.c
+++ b/drivers/staging/comedi/drivers/cb_pcidda.c
@@ -248,8 +248,8 @@ static void cb_pcidda_write_caldac(struct comedi_device 
*dev,
cb_pcidda_serial_out(dev, value, num_caldac_bits);
 
 /*
-* latch stream into appropriate caldac deselect reference dac
-*/
+ * latch stream into appropriate caldac deselect reference dac
+ */
cal2_bits = DESELECT_REF_DAC_BIT | DUMMY_BIT;
/*  deactivate caldacs (one caldac for every two channels) */
for (i = 0; i < max_num_caldacs; i++)
-- 
2.9.3

Re: [PATCH] Input: joystick: analog - change msleep to usleep_range for small msecs

[Vojtech Pavlik]

On Tue, Nov 29, 2016 at 01:11:31AM +0530, Aniroop Mathur wrote:

> msleep(1~20) may not do what the caller intends, and will often sleep longer.
> (~20 ms actual sleep for any value given in the 1~20ms range)
> This is not the desired behaviour for many cases like device resume time,
> device suspend time, device enable time, connection time, probe time,
> loops, retry logic, etc
> msleep is built on jiffies / legacy timers which are not precise whereas
> usleep_range is build on top of hrtimers so the wakeups are precise.
> Thus, change msleep to usleep_range for precise wakeups.
> 
> For example:
> On a machine with tick rate / HZ as 100, msleep(3) will make the process to
> sleep for a minimum period of 10 ms whereas usleep_range(3000, 3100) will make
> sure that the process does not sleep for more than 3100 us or 3.1ms

Again, not needed, if the MAX_TIME sleeps are longer, nobody cares.

> 
> Signed-off-by: Aniroop Mathur 
> ---
>  drivers/input/joystick/analog.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/joystick/analog.c b/drivers/input/joystick/analog.c
> index 3d8ff09..2891704 100644
> --- a/drivers/input/joystick/analog.c
> +++ b/drivers/input/joystick/analog.c
> @@ -88,7 +88,7 @@ MODULE_PARM_DESC(map, "Describes analog joysticks 
> type/capabilities");
>  #define ANALOG_EXTENSIONS0x7ff00
>  #define ANALOG_GAMEPAD   0x8
>  
> -#define ANALOG_MAX_TIME  3   /* 3 ms */
> +#define ANALOG_MAX_TIME  3000/* 3000 us */
>  #define ANALOG_LOOP_TIME 2000/* 2 * loop */
>  #define ANALOG_SAITEK_DELAY  200 /* 200 us */
>  #define ANALOG_SAITEK_TIME   2000/* 2000 us */
> @@ -257,7 +257,7 @@ static int analog_cooked_read(struct analog_port *port)
>   int i, j;
>  
>   loopout = (ANALOG_LOOP_TIME * port->loop) / 1000;
> - timeout = ANALOG_MAX_TIME * port->speed;
> + timeout = (ANALOG_MAX_TIME / 1000) * port->speed;
>  
>   local_irq_save(flags);
>   gameport_trigger(gameport);
> @@ -625,20 +625,20 @@ static int analog_init_port(struct gameport *gameport, 
> struct gameport_driver *d
>  
>   gameport_trigger(gameport);
>   t = gameport_read(gameport);
> - msleep(ANALOG_MAX_TIME);
> + usleep_range(ANALOG_MAX_TIME, ANALOG_MAX_TIME + 100);
>   port->mask = (gameport_read(gameport) ^ t) & t & 0xf;
>   port->fuzz = (port->speed * ANALOG_FUZZ_MAGIC) / port->loop / 
> 1000 + ANALOG_FUZZ_BITS;
>  
>   for (i = 0; i < ANALOG_INIT_RETRIES; i++) {
>   if (!analog_cooked_read(port))
>   break;
> - msleep(ANALOG_MAX_TIME);
> + usleep_range(ANALOG_MAX_TIME, ANALOG_MAX_TIME + 100);
>   }
>  
>   u = v = 0;
>  
> - msleep(ANALOG_MAX_TIME);
> - t = gameport_time(gameport, ANALOG_MAX_TIME * 1000);
> + usleep_range(ANALOG_MAX_TIME, ANALOG_MAX_TIME + 100);
> + t = gameport_time(gameport, ANALOG_MAX_TIME);
>   gameport_trigger(gameport);
>   while ((gameport_read(port->gameport) & port->mask) && (u < t))
>   u++;
> -- 
> 2.6.2
> 

-- 
Vojtech Pavlik

Re: [RFC][PATCH 2/5 v2] drm/bridge: adv7511: Switch to using drm_kms_helper_hotplug_event()

[Laurent Pinchart]

Hi John,

Thank you for the patch.

On Monday 28 Nov 2016 21:04:41 John Stultz wrote:
> In chasing down a previous issue with EDID probing from calling
> drm_helper_hpd_irq_event() from irq context, Laurent noticed
> that the DRM documentation suggests that
> drm_kms_helper_hotplug_event() should be used instead.
> 
> Thus this patch replaces drm_helper_hpd_irq_event() with
> drm_kms_helper_hotplug_event().
> 
> Cc: David Airlie 
> Cc: Archit Taneja 
> Cc: Wolfram Sang 
> Cc: Lars-Peter Clausen 
> Cc: Laurent Pinchart 
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: John Stultz 
> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index b38e743..2caca0c
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -406,7 +406,7 @@ static void adv7511_irq_work(struct work_struct *work)
>  {
>   struct adv7511 *adv7511 = container_of(work, struct adv7511, 
irq_work);
> 
> - drm_helper_hpd_irq_event(adv7511->connector.dev);
> + drm_kms_helper_hotplug_event(adv7511->connector.dev);

That's nice, but you must first update adv7511->connector.status (and as an 
optimization only call drm_kms_helper_hotplug_event() if the status has 
changed).

>  }

-- 
Regards,

Laurent Pinchart

Re: [PATCH] Input: joystick: analog - change msleep to usleep_range for small msecs

[Vojtech Pavlik]

On Tue, Nov 29, 2016 at 01:11:31AM +0530, Aniroop Mathur wrote:

> msleep(1~20) may not do what the caller intends, and will often sleep longer.
> (~20 ms actual sleep for any value given in the 1~20ms range)
> This is not the desired behaviour for many cases like device resume time,
> device suspend time, device enable time, connection time, probe time,
> loops, retry logic, etc
> msleep is built on jiffies / legacy timers which are not precise whereas
> usleep_range is build on top of hrtimers so the wakeups are precise.
> Thus, change msleep to usleep_range for precise wakeups.
> 
> For example:
> On a machine with tick rate / HZ as 100, msleep(3) will make the process to
> sleep for a minimum period of 10 ms whereas usleep_range(3000, 3100) will make
> sure that the process does not sleep for more than 3100 us or 3.1ms

Again, not needed, if the MAX_TIME sleeps are longer, nobody cares.

> 
> Signed-off-by: Aniroop Mathur 
> ---
>  drivers/input/joystick/analog.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/joystick/analog.c b/drivers/input/joystick/analog.c
> index 3d8ff09..2891704 100644
> --- a/drivers/input/joystick/analog.c
> +++ b/drivers/input/joystick/analog.c
> @@ -88,7 +88,7 @@ MODULE_PARM_DESC(map, "Describes analog joysticks 
> type/capabilities");
>  #define ANALOG_EXTENSIONS0x7ff00
>  #define ANALOG_GAMEPAD   0x8
>  
> -#define ANALOG_MAX_TIME  3   /* 3 ms */
> +#define ANALOG_MAX_TIME  3000/* 3000 us */
>  #define ANALOG_LOOP_TIME 2000/* 2 * loop */
>  #define ANALOG_SAITEK_DELAY  200 /* 200 us */
>  #define ANALOG_SAITEK_TIME   2000/* 2000 us */
> @@ -257,7 +257,7 @@ static int analog_cooked_read(struct analog_port *port)
>   int i, j;
>  
>   loopout = (ANALOG_LOOP_TIME * port->loop) / 1000;
> - timeout = ANALOG_MAX_TIME * port->speed;
> + timeout = (ANALOG_MAX_TIME / 1000) * port->speed;
>  
>   local_irq_save(flags);
>   gameport_trigger(gameport);
> @@ -625,20 +625,20 @@ static int analog_init_port(struct gameport *gameport, 
> struct gameport_driver *d
>  
>   gameport_trigger(gameport);
>   t = gameport_read(gameport);
> - msleep(ANALOG_MAX_TIME);
> + usleep_range(ANALOG_MAX_TIME, ANALOG_MAX_TIME + 100);
>   port->mask = (gameport_read(gameport) ^ t) & t & 0xf;
>   port->fuzz = (port->speed * ANALOG_FUZZ_MAGIC) / port->loop / 
> 1000 + ANALOG_FUZZ_BITS;
>  
>   for (i = 0; i < ANALOG_INIT_RETRIES; i++) {
>   if (!analog_cooked_read(port))
>   break;
> - msleep(ANALOG_MAX_TIME);
> + usleep_range(ANALOG_MAX_TIME, ANALOG_MAX_TIME + 100);
>   }
>  
>   u = v = 0;
>  
> - msleep(ANALOG_MAX_TIME);
> - t = gameport_time(gameport, ANALOG_MAX_TIME * 1000);
> + usleep_range(ANALOG_MAX_TIME, ANALOG_MAX_TIME + 100);
> + t = gameport_time(gameport, ANALOG_MAX_TIME);
>   gameport_trigger(gameport);
>   while ((gameport_read(port->gameport) & port->mask) && (u < t))
>   u++;
> -- 
> 2.6.2
> 

-- 
Vojtech Pavlik

Re: [RFC][PATCH 2/5 v2] drm/bridge: adv7511: Switch to using drm_kms_helper_hotplug_event()

[Laurent Pinchart]

Hi John,

Thank you for the patch.

On Monday 28 Nov 2016 21:04:41 John Stultz wrote:
> In chasing down a previous issue with EDID probing from calling
> drm_helper_hpd_irq_event() from irq context, Laurent noticed
> that the DRM documentation suggests that
> drm_kms_helper_hotplug_event() should be used instead.
> 
> Thus this patch replaces drm_helper_hpd_irq_event() with
> drm_kms_helper_hotplug_event().
> 
> Cc: David Airlie 
> Cc: Archit Taneja 
> Cc: Wolfram Sang 
> Cc: Lars-Peter Clausen 
> Cc: Laurent Pinchart 
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: John Stultz 
> ---
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index b38e743..2caca0c
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -406,7 +406,7 @@ static void adv7511_irq_work(struct work_struct *work)
>  {
>   struct adv7511 *adv7511 = container_of(work, struct adv7511, 
irq_work);
> 
> - drm_helper_hpd_irq_event(adv7511->connector.dev);
> + drm_kms_helper_hotplug_event(adv7511->connector.dev);

That's nice, but you must first update adv7511->connector.status (and as an 
optimization only call drm_kms_helper_hotplug_event() if the status has 
changed).

>  }

-- 
Regards,

Laurent Pinchart

Re: [PATCH] Input: joystick: sidewinder - change msleep to usleep_range for small msecs

[Vojtech Pavlik]

On Tue, Nov 29, 2016 at 01:08:22AM +0530, Aniroop Mathur wrote:
> msleep(1~20) may not do what the caller intends, and will often sleep longer.
> (~20 ms actual sleep for any value given in the 1~20ms range)
> This is not the desired behaviour for many cases like device resume time,
> device suspend time, device enable time, connection time, probe time,
> loops, retry logic, etc
> msleep is built on jiffies / legacy timers which are not precise whereas
> usleep_range is build on top of hrtimers so the wakeups are precise.
> Thus, change msleep to usleep_range for precise wakeups.
> 
> For example:
> On a machine with tick rate / HZ as 100, msleep(6) will make the process to
> sleep for a minimum period of 10 ms whereas usleep_range(6000, 6100) will make
> sure that the process does not sleep for more than 6100 us or 6.1ms

This patch seems clearly unnecessary. SW_TIMEOUT is 6ms or longer for
MS Sidewinder devices.

Vojtech

> Signed-off-by: Aniroop Mathur 
> ---
>  drivers/input/joystick/sidewinder.c | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/input/joystick/sidewinder.c 
> b/drivers/input/joystick/sidewinder.c
> index 4a95b22..e5a1292 100644
> --- a/drivers/input/joystick/sidewinder.c
> +++ b/drivers/input/joystick/sidewinder.c
> @@ -50,7 +50,7 @@ MODULE_LICENSE("GPL");
>  
>  #define SW_START 600 /* The time we wait for the first bit [600 us] 
> */
>  #define SW_STROBE60  /* Max time per bit [60 us] */
> -#define SW_TIMEOUT   6   /* Wait for everything to settle [6 ms] */
> +#define SW_TIMEOUT   6000/* Wait for everything to settle [6000 us] */
>  #define SW_KICK  45  /* Wait after A0 fall till kick [45 us] 
> */
>  #define SW_END   8   /* Number of bits before end of packet 
> to kick */
>  #define SW_FAIL  16  /* Number of packet read errors to fail 
> and reinitialize */
> @@ -139,7 +139,7 @@ static int sw_read_packet(struct gameport *gameport, 
> unsigned char *buf, int len
>   unsigned char pending, u, v;
>  
>   i = -id;/* Don't care 
> about data, only want ID */
> - timeout = id ? gameport_time(gameport, SW_TIMEOUT * 1000) : 0; /* Set 
> up global timeout for ID packet */
> + timeout = id ? gameport_time(gameport, SW_TIMEOUT) : 0; /* Set up 
> global timeout for ID packet */
>   kick = id ? gameport_time(gameport, SW_KICK) : 0;   /* Set up kick 
> timeout for ID packet */
>   start = gameport_time(gameport, SW_START);
>   strobe = gameport_time(gameport, SW_STROBE);
> @@ -248,7 +248,7 @@ static void sw_init_digital(struct gameport *gameport)
>   i = 0;
>  do {
>  gameport_trigger(gameport);  /* Trigger */
> - t = gameport_time(gameport, SW_TIMEOUT * 1000);
> + t = gameport_time(gameport, SW_TIMEOUT);
>   while ((gameport_read(gameport) & 1) && t) t--; /* Wait for 
> axis to fall back to 0 */
>  udelay(seq[i]);  /* 
> Delay magic time */
>  } while (seq[++i]);
> @@ -483,13 +483,13 @@ static int sw_read(struct sw *sw)
>   " - reinitializing joystick.\n", sw->gameport->phys);
>  
>   if (!i && sw->type == SW_ID_3DP) {  
> /* 3D Pro can be in analog mode */
> - mdelay(3 * SW_TIMEOUT);
> + mdelay(3 * (SW_TIMEOUT / 1000));
>   sw_init_digital(sw->gameport);
>   }
>  
> - mdelay(SW_TIMEOUT);
> + mdelay(SW_TIMEOUT / 1000);
>   i = sw_read_packet(sw->gameport, buf, SW_LENGTH, 0);
> /* Read normal data packet */
> - mdelay(SW_TIMEOUT);
> + mdelay(SW_TIMEOUT / 1000);
>   sw_read_packet(sw->gameport, buf, SW_LENGTH, i);
> /* Read ID packet, this initializes the stick */
>  
>   sw->fail = SW_FAIL;
> @@ -616,14 +616,14 @@ static int sw_connect(struct gameport *gameport, struct 
> gameport_driver *drv)
>   gameport->phys, gameport->io, gameport->speed);
>  
>   i = sw_read_packet(gameport, buf, SW_LENGTH, 0);/* Read 
> normal packet */
> - msleep(SW_TIMEOUT);
> + usleep_range(SW_TIMEOUT, SW_TIMEOUT + 100);
>   dbg("Init 1: Mode %d. Length %d.", m , i);
>  
>   if (!i) {   /* No 
> data. 3d Pro analog mode? */
>   sw_init_digital(gameport);  /* 
> Switch to digital */
> - msleep(SW_TIMEOUT);
> + usleep_range(SW_TIMEOUT, SW_TIMEOUT + 100);
>   i = sw_read_packet(gameport, buf, SW_LENGTH, 0);/* 
> Retry reading packet */
> - msleep(SW_TIMEOUT);
> + usleep_range(SW_TIMEOUT, SW_TIMEOUT + 100);
>   dbg("Init 1b: Length %d.", i);
>   if (!i) {  

Re: [PATCH] Input: joystick: sidewinder - change msleep to usleep_range for small msecs

[Vojtech Pavlik]

On Tue, Nov 29, 2016 at 01:08:22AM +0530, Aniroop Mathur wrote:
> msleep(1~20) may not do what the caller intends, and will often sleep longer.
> (~20 ms actual sleep for any value given in the 1~20ms range)
> This is not the desired behaviour for many cases like device resume time,
> device suspend time, device enable time, connection time, probe time,
> loops, retry logic, etc
> msleep is built on jiffies / legacy timers which are not precise whereas
> usleep_range is build on top of hrtimers so the wakeups are precise.
> Thus, change msleep to usleep_range for precise wakeups.
> 
> For example:
> On a machine with tick rate / HZ as 100, msleep(6) will make the process to
> sleep for a minimum period of 10 ms whereas usleep_range(6000, 6100) will make
> sure that the process does not sleep for more than 6100 us or 6.1ms

This patch seems clearly unnecessary. SW_TIMEOUT is 6ms or longer for
MS Sidewinder devices.

Vojtech

> Signed-off-by: Aniroop Mathur 
> ---
>  drivers/input/joystick/sidewinder.c | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/input/joystick/sidewinder.c 
> b/drivers/input/joystick/sidewinder.c
> index 4a95b22..e5a1292 100644
> --- a/drivers/input/joystick/sidewinder.c
> +++ b/drivers/input/joystick/sidewinder.c
> @@ -50,7 +50,7 @@ MODULE_LICENSE("GPL");
>  
>  #define SW_START 600 /* The time we wait for the first bit [600 us] 
> */
>  #define SW_STROBE60  /* Max time per bit [60 us] */
> -#define SW_TIMEOUT   6   /* Wait for everything to settle [6 ms] */
> +#define SW_TIMEOUT   6000/* Wait for everything to settle [6000 us] */
>  #define SW_KICK  45  /* Wait after A0 fall till kick [45 us] 
> */
>  #define SW_END   8   /* Number of bits before end of packet 
> to kick */
>  #define SW_FAIL  16  /* Number of packet read errors to fail 
> and reinitialize */
> @@ -139,7 +139,7 @@ static int sw_read_packet(struct gameport *gameport, 
> unsigned char *buf, int len
>   unsigned char pending, u, v;
>  
>   i = -id;/* Don't care 
> about data, only want ID */
> - timeout = id ? gameport_time(gameport, SW_TIMEOUT * 1000) : 0; /* Set 
> up global timeout for ID packet */
> + timeout = id ? gameport_time(gameport, SW_TIMEOUT) : 0; /* Set up 
> global timeout for ID packet */
>   kick = id ? gameport_time(gameport, SW_KICK) : 0;   /* Set up kick 
> timeout for ID packet */
>   start = gameport_time(gameport, SW_START);
>   strobe = gameport_time(gameport, SW_STROBE);
> @@ -248,7 +248,7 @@ static void sw_init_digital(struct gameport *gameport)
>   i = 0;
>  do {
>  gameport_trigger(gameport);  /* Trigger */
> - t = gameport_time(gameport, SW_TIMEOUT * 1000);
> + t = gameport_time(gameport, SW_TIMEOUT);
>   while ((gameport_read(gameport) & 1) && t) t--; /* Wait for 
> axis to fall back to 0 */
>  udelay(seq[i]);  /* 
> Delay magic time */
>  } while (seq[++i]);
> @@ -483,13 +483,13 @@ static int sw_read(struct sw *sw)
>   " - reinitializing joystick.\n", sw->gameport->phys);
>  
>   if (!i && sw->type == SW_ID_3DP) {  
> /* 3D Pro can be in analog mode */
> - mdelay(3 * SW_TIMEOUT);
> + mdelay(3 * (SW_TIMEOUT / 1000));
>   sw_init_digital(sw->gameport);
>   }
>  
> - mdelay(SW_TIMEOUT);
> + mdelay(SW_TIMEOUT / 1000);
>   i = sw_read_packet(sw->gameport, buf, SW_LENGTH, 0);
> /* Read normal data packet */
> - mdelay(SW_TIMEOUT);
> + mdelay(SW_TIMEOUT / 1000);
>   sw_read_packet(sw->gameport, buf, SW_LENGTH, i);
> /* Read ID packet, this initializes the stick */
>  
>   sw->fail = SW_FAIL;
> @@ -616,14 +616,14 @@ static int sw_connect(struct gameport *gameport, struct 
> gameport_driver *drv)
>   gameport->phys, gameport->io, gameport->speed);
>  
>   i = sw_read_packet(gameport, buf, SW_LENGTH, 0);/* Read 
> normal packet */
> - msleep(SW_TIMEOUT);
> + usleep_range(SW_TIMEOUT, SW_TIMEOUT + 100);
>   dbg("Init 1: Mode %d. Length %d.", m , i);
>  
>   if (!i) {   /* No 
> data. 3d Pro analog mode? */
>   sw_init_digital(gameport);  /* 
> Switch to digital */
> - msleep(SW_TIMEOUT);
> + usleep_range(SW_TIMEOUT, SW_TIMEOUT + 100);
>   i = sw_read_packet(gameport, buf, SW_LENGTH, 0);/* 
> Retry reading packet */
> - msleep(SW_TIMEOUT);
> + usleep_range(SW_TIMEOUT, SW_TIMEOUT + 100);
>   dbg("Init 1b: Length %d.", i);
>   if (!i) {

[PATCH] drm/vmwgfx: Fix handling of errors returned by 'vmw_cotable_alloc()'

[Christophe JAILLET]

'vmw_cotable_alloc()' returns an error pointer on error, not NULL.
Propagate the error code, instead of returning -ENOMEM unconditionally

Signed-off-by: Christophe JAILLET 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_context.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_context.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_context.c
index 443d1ed00de7..d1aee9860033 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_context.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_context.c
@@ -209,8 +209,8 @@ static int vmw_gb_context_init(struct vmw_private *dev_priv,
for (i = 0; i < SVGA_COTABLE_DX10_MAX; ++i) {
uctx->cotables[i] = vmw_cotable_alloc(dev_priv,
  >res, i);
-   if (unlikely(uctx->cotables[i] == NULL)) {
-   ret = -ENOMEM;
+   if (unlikely(IS_ERR(uctx->cotables[i]))) {
+   ret = PTR_ERR(uctx->cotables[i]);
goto out_cotables;
}
}
-- 
2.9.3

[PATCH] drm/vmwgfx: Fix handling of errors returned by 'vmw_cotable_alloc()'

[Christophe JAILLET]

'vmw_cotable_alloc()' returns an error pointer on error, not NULL.
Propagate the error code, instead of returning -ENOMEM unconditionally

Signed-off-by: Christophe JAILLET 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_context.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_context.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_context.c
index 443d1ed00de7..d1aee9860033 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_context.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_context.c
@@ -209,8 +209,8 @@ static int vmw_gb_context_init(struct vmw_private *dev_priv,
for (i = 0; i < SVGA_COTABLE_DX10_MAX; ++i) {
uctx->cotables[i] = vmw_cotable_alloc(dev_priv,
  >res, i);
-   if (unlikely(uctx->cotables[i] == NULL)) {
-   ret = -ENOMEM;
+   if (unlikely(IS_ERR(uctx->cotables[i]))) {
+   ret = PTR_ERR(uctx->cotables[i]);
goto out_cotables;
}
}
-- 
2.9.3

RE: [PATCH v3] crypto: add virtio-crypto driver

[Gonglei (Arei)]

>
> > > > +
> > > > +/* Note: kernel crypto API realization */
> > > > +static int virtio_crypto_ablkcipher_setkey(struct crypto_ablkcipher 
> > > > *tfm,
> > > > +const uint8_t *key,
> > > > +unsigned int keylen)
> > > > +{
> > > > +   struct virtio_crypto_ablkcipher_ctx *ctx =
> > crypto_ablkcipher_ctx(tfm);
> > > > +   int ret;
> > > > +
> > > > +   spin_lock(>lock);
> > > > +
> > > > +   if (!ctx->vcrypto) {
> > > > +   /* New key */
> > > > +   int node = virtio_crypto_get_current_node();
> > > > +   struct virtio_crypto *vcrypto =
> > > > + virtcrypto_get_dev_node(node);
> > > > +   if (!vcrypto) {
> > > > +   vcrypto = virtcrypto_devmgr_get_first();
> >
> > Is this the standard way to do this? How does this work with
> > multiple crypto devices (e.g. with different capabilities)?
> >
> Actually there is a simple schedule algorithms in virtcrypto_get_dev_node(),
> which return the device used fewest on the node.
> 
> If we don't find a device in the node, select the first on as default.
> But I forgot to check the first devices whether the device has started here.
> 
Oh, the virtcrypto_get_dev_node() had done this work, the calling of
virtcrypto_devmgr_get_first() here is superfluous. Will remove it.

Regards,
-Gonglei

RE: [PATCH v3] crypto: add virtio-crypto driver

[Gonglei (Arei)]

>
> > > > +
> > > > +/* Note: kernel crypto API realization */
> > > > +static int virtio_crypto_ablkcipher_setkey(struct crypto_ablkcipher 
> > > > *tfm,
> > > > +const uint8_t *key,
> > > > +unsigned int keylen)
> > > > +{
> > > > +   struct virtio_crypto_ablkcipher_ctx *ctx =
> > crypto_ablkcipher_ctx(tfm);
> > > > +   int ret;
> > > > +
> > > > +   spin_lock(>lock);
> > > > +
> > > > +   if (!ctx->vcrypto) {
> > > > +   /* New key */
> > > > +   int node = virtio_crypto_get_current_node();
> > > > +   struct virtio_crypto *vcrypto =
> > > > + virtcrypto_get_dev_node(node);
> > > > +   if (!vcrypto) {
> > > > +   vcrypto = virtcrypto_devmgr_get_first();
> >
> > Is this the standard way to do this? How does this work with
> > multiple crypto devices (e.g. with different capabilities)?
> >
> Actually there is a simple schedule algorithms in virtcrypto_get_dev_node(),
> which return the device used fewest on the node.
> 
> If we don't find a device in the node, select the first on as default.
> But I forgot to check the first devices whether the device has started here.
> 
Oh, the virtcrypto_get_dev_node() had done this work, the calling of
virtcrypto_devmgr_get_first() here is superfluous. Will remove it.

Regards,
-Gonglei

Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common path

[Naoya Horiguchi]

# sorry for late reply ...

On Fri, Nov 18, 2016 at 02:56:24AM +0300, Kirill A. Shutemov wrote:
> On Tue, Nov 08, 2016 at 08:31:52AM +0900, Naoya Horiguchi wrote:
> > If one of callers of page migration starts to handle thp, memory management 
> > code
> > start to see pmd migration entry, so we need to prepare for it before 
> > enabling.
> > This patch changes various code point which checks the status of given pmds 
> > in
> > order to prevent race between thp migration and the pmd-related works.
> > 
> > Signed-off-by: Naoya Horiguchi 
> > ---
> > ChangeLog v1 -> v2:
> > - introduce pmd_related() (I know the naming is not good, but can't think up
> >   no better name. Any suggesntion is welcomed.)
> > ---
> >  arch/x86/mm/gup.c   |  4 +--
> >  fs/proc/task_mmu.c  | 23 +++--
> >  include/linux/huge_mm.h |  9 -
> >  mm/gup.c| 10 --
> >  mm/huge_memory.c| 88 
> > -
> >  mm/madvise.c|  2 +-
> >  mm/memcontrol.c |  2 ++
> >  mm/memory.c |  6 +++-
> >  mm/mprotect.c   |  2 ++
> >  mm/mremap.c |  2 +-
> >  10 files changed, 114 insertions(+), 34 deletions(-)
> > 
> > diff --git v4.9-rc2-mmotm-2016-10-27-18-27/arch/x86/mm/gup.c 
> > v4.9-rc2-mmotm-2016-10-27-18-27_patched/arch/x86/mm/gup.c
> > index 0d4fb3e..78a153d 100644
> > --- v4.9-rc2-mmotm-2016-10-27-18-27/arch/x86/mm/gup.c
> > +++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/arch/x86/mm/gup.c
> > @@ -222,9 +222,9 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, 
> > unsigned long end,
> > pmd_t pmd = *pmdp;
> >  
> > next = pmd_addr_end(addr, end);
> > -   if (pmd_none(pmd))
> > +   if (!pmd_present(pmd))
> > return 0;
> > -   if (unlikely(pmd_large(pmd) || !pmd_present(pmd))) {
> > +   if (unlikely(pmd_large(pmd))) {
> > /*
> >  * NUMA hinting faults need to be handled in the GUP
> >  * slowpath for accounting purposes and so that they
> > diff --git v4.9-rc2-mmotm-2016-10-27-18-27/fs/proc/task_mmu.c 
> > v4.9-rc2-mmotm-2016-10-27-18-27_patched/fs/proc/task_mmu.c
> > index 35b92d8..c1f9cf4 100644
> > --- v4.9-rc2-mmotm-2016-10-27-18-27/fs/proc/task_mmu.c
> > +++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/fs/proc/task_mmu.c
> > @@ -596,7 +596,8 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long 
> > addr, unsigned long end,
> >  
> > ptl = pmd_trans_huge_lock(pmd, vma);
> > if (ptl) {
> > -   smaps_pmd_entry(pmd, addr, walk);
> > +   if (pmd_present(*pmd))
> > +   smaps_pmd_entry(pmd, addr, walk);
> > spin_unlock(ptl);
> > return 0;
> > }
> > @@ -929,6 +930,9 @@ static int clear_refs_pte_range(pmd_t *pmd, unsigned 
> > long addr,
> > goto out;
> > }
> >  
> > +   if (!pmd_present(*pmd))
> > +   goto out;
> > +
> 
> Hm. Looks like clear_soft_dirty_pmd() should handle !present. It doesn't.
> 
> Ah.. Found it in 08/12.
> 
> > page = pmd_page(*pmd);
> >  
> > /* Clear accessed and referenced bits. */
> > @@ -1208,19 +1212,18 @@ static int pagemap_pmd_range(pmd_t *pmdp, unsigned 
> > long addr, unsigned long end,
> > if (ptl) {
> > u64 flags = 0, frame = 0;
> > pmd_t pmd = *pmdp;
> > +   struct page *page;
> >  
> > if ((vma->vm_flags & VM_SOFTDIRTY) || pmd_soft_dirty(pmd))
> > flags |= PM_SOFT_DIRTY;
> >  
> > -   /*
> > -* Currently pmd for thp is always present because thp
> > -* can not be swapped-out, migrated, or HWPOISONed
> > -* (split in such cases instead.)
> > -* This if-check is just to prepare for future implementation.
> > -*/
> > -   if (pmd_present(pmd)) {
> > -   struct page *page = pmd_page(pmd);
> > -
> > +   if (is_pmd_migration_entry(pmd)) {
> > +   swp_entry_t entry = pmd_to_swp_entry(pmd);
> > +   frame = swp_type(entry) |
> > +   (swp_offset(entry) << MAX_SWAPFILES_SHIFT);
> > +   page = migration_entry_to_page(entry);
> > +   } else if (pmd_present(pmd)) {
> > +   page = pmd_page(pmd);
> > if (page_mapcount(page) == 1)
> > flags |= PM_MMAP_EXCLUSIVE;
> >  
> > diff --git v4.9-rc2-mmotm-2016-10-27-18-27/include/linux/huge_mm.h 
> > v4.9-rc2-mmotm-2016-10-27-18-27_patched/include/linux/huge_mm.h
> > index fcbca51..3c252cd 100644
> > --- v4.9-rc2-mmotm-2016-10-27-18-27/include/linux/huge_mm.h
> > +++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/include/linux/huge_mm.h
> > @@ -125,12 +125,19 @@ extern void vma_adjust_trans_huge(struct 
> > vm_area_struct *vma,
> >

Re: [PATCH v2 07/12] mm: thp: check pmd migration entry in common path

[Naoya Horiguchi]

# sorry for late reply ...

On Fri, Nov 18, 2016 at 02:56:24AM +0300, Kirill A. Shutemov wrote:
> On Tue, Nov 08, 2016 at 08:31:52AM +0900, Naoya Horiguchi wrote:
> > If one of callers of page migration starts to handle thp, memory management 
> > code
> > start to see pmd migration entry, so we need to prepare for it before 
> > enabling.
> > This patch changes various code point which checks the status of given pmds 
> > in
> > order to prevent race between thp migration and the pmd-related works.
> > 
> > Signed-off-by: Naoya Horiguchi 
> > ---
> > ChangeLog v1 -> v2:
> > - introduce pmd_related() (I know the naming is not good, but can't think up
> >   no better name. Any suggesntion is welcomed.)
> > ---
> >  arch/x86/mm/gup.c   |  4 +--
> >  fs/proc/task_mmu.c  | 23 +++--
> >  include/linux/huge_mm.h |  9 -
> >  mm/gup.c| 10 --
> >  mm/huge_memory.c| 88 
> > -
> >  mm/madvise.c|  2 +-
> >  mm/memcontrol.c |  2 ++
> >  mm/memory.c |  6 +++-
> >  mm/mprotect.c   |  2 ++
> >  mm/mremap.c |  2 +-
> >  10 files changed, 114 insertions(+), 34 deletions(-)
> > 
> > diff --git v4.9-rc2-mmotm-2016-10-27-18-27/arch/x86/mm/gup.c 
> > v4.9-rc2-mmotm-2016-10-27-18-27_patched/arch/x86/mm/gup.c
> > index 0d4fb3e..78a153d 100644
> > --- v4.9-rc2-mmotm-2016-10-27-18-27/arch/x86/mm/gup.c
> > +++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/arch/x86/mm/gup.c
> > @@ -222,9 +222,9 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, 
> > unsigned long end,
> > pmd_t pmd = *pmdp;
> >  
> > next = pmd_addr_end(addr, end);
> > -   if (pmd_none(pmd))
> > +   if (!pmd_present(pmd))
> > return 0;
> > -   if (unlikely(pmd_large(pmd) || !pmd_present(pmd))) {
> > +   if (unlikely(pmd_large(pmd))) {
> > /*
> >  * NUMA hinting faults need to be handled in the GUP
> >  * slowpath for accounting purposes and so that they
> > diff --git v4.9-rc2-mmotm-2016-10-27-18-27/fs/proc/task_mmu.c 
> > v4.9-rc2-mmotm-2016-10-27-18-27_patched/fs/proc/task_mmu.c
> > index 35b92d8..c1f9cf4 100644
> > --- v4.9-rc2-mmotm-2016-10-27-18-27/fs/proc/task_mmu.c
> > +++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/fs/proc/task_mmu.c
> > @@ -596,7 +596,8 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long 
> > addr, unsigned long end,
> >  
> > ptl = pmd_trans_huge_lock(pmd, vma);
> > if (ptl) {
> > -   smaps_pmd_entry(pmd, addr, walk);
> > +   if (pmd_present(*pmd))
> > +   smaps_pmd_entry(pmd, addr, walk);
> > spin_unlock(ptl);
> > return 0;
> > }
> > @@ -929,6 +930,9 @@ static int clear_refs_pte_range(pmd_t *pmd, unsigned 
> > long addr,
> > goto out;
> > }
> >  
> > +   if (!pmd_present(*pmd))
> > +   goto out;
> > +
> 
> Hm. Looks like clear_soft_dirty_pmd() should handle !present. It doesn't.
> 
> Ah.. Found it in 08/12.
> 
> > page = pmd_page(*pmd);
> >  
> > /* Clear accessed and referenced bits. */
> > @@ -1208,19 +1212,18 @@ static int pagemap_pmd_range(pmd_t *pmdp, unsigned 
> > long addr, unsigned long end,
> > if (ptl) {
> > u64 flags = 0, frame = 0;
> > pmd_t pmd = *pmdp;
> > +   struct page *page;
> >  
> > if ((vma->vm_flags & VM_SOFTDIRTY) || pmd_soft_dirty(pmd))
> > flags |= PM_SOFT_DIRTY;
> >  
> > -   /*
> > -* Currently pmd for thp is always present because thp
> > -* can not be swapped-out, migrated, or HWPOISONed
> > -* (split in such cases instead.)
> > -* This if-check is just to prepare for future implementation.
> > -*/
> > -   if (pmd_present(pmd)) {
> > -   struct page *page = pmd_page(pmd);
> > -
> > +   if (is_pmd_migration_entry(pmd)) {
> > +   swp_entry_t entry = pmd_to_swp_entry(pmd);
> > +   frame = swp_type(entry) |
> > +   (swp_offset(entry) << MAX_SWAPFILES_SHIFT);
> > +   page = migration_entry_to_page(entry);
> > +   } else if (pmd_present(pmd)) {
> > +   page = pmd_page(pmd);
> > if (page_mapcount(page) == 1)
> > flags |= PM_MMAP_EXCLUSIVE;
> >  
> > diff --git v4.9-rc2-mmotm-2016-10-27-18-27/include/linux/huge_mm.h 
> > v4.9-rc2-mmotm-2016-10-27-18-27_patched/include/linux/huge_mm.h
> > index fcbca51..3c252cd 100644
> > --- v4.9-rc2-mmotm-2016-10-27-18-27/include/linux/huge_mm.h
> > +++ v4.9-rc2-mmotm-2016-10-27-18-27_patched/include/linux/huge_mm.h
> > @@ -125,12 +125,19 @@ extern void vma_adjust_trans_huge(struct 
> > vm_area_struct *vma,
> > long 

Re: [RFC][PATCH 1/5 v2] drm/bridge: adv7511: Use work_struct to defer hotplug handing to out of irq context

[Laurent Pinchart]

Hi John,

Thank you for the patch.

On Monday 28 Nov 2016 21:04:40 John Stultz wrote:
> I was recently seeing issues with EDID probing, where
> the logic to wait for the EDID read bit to be set by the
> IRQ wasn't happening and the code would time out and fail.
> 
> Digging deeper, I found this was due to the fact that
> IRQs were disabled as we were running in IRQ context from
> the HPD signal.
> 
> Thus this patch changes the logic to handle the HPD signal
> via a work_struct so we can be out of irq context.
> 
> With this patch, the EDID probing on hotplug does not time
> out.
> 
> Cc: David Airlie 
> Cc: Archit Taneja 
> Cc: Wolfram Sang 
> Cc: Lars-Peter Clausen 
> Cc: Laurent Pinchart 
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: John Stultz 
> ---
> v2: Reworked to properly fix the issue rather then
> just delaying for 200ms
> 
>  drivers/gpu/drm/bridge/adv7511/adv7511.h |  2 ++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12 +++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 992d76c..2a1e722 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -317,6 +317,8 @@ struct adv7511 {
>   bool edid_read;
> 
>   wait_queue_head_t wq;
> + struct work_struct irq_work;
> +

I'd call this hpd_work.

>   struct drm_bridge bridge;
>   struct drm_connector connector;
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8dba729..b38e743
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -402,6 +402,14 @@ static bool adv7511_hpd(struct adv7511 *adv7511)
>   return false;
>  }
> 
> +static void adv7511_irq_work(struct work_struct *work)
> +{
> + struct adv7511 *adv7511 = container_of(work, struct adv7511, 
irq_work);
> +
> + drm_helper_hpd_irq_event(adv7511->connector.dev);
> +}
> +
> +

One blank line is enough.

Apart from that,

Reviewed-by: Laurent Pinchart 

>  static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd)
>  {
>   unsigned int irq0, irq1;
> @@ -419,7 +427,7 @@ static int adv7511_irq_process(struct adv7511 *adv7511,
> bool process_hpd) regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1);
> 
>   if (process_hpd && irq0 & ADV7511_INT0_HPD && adv7511->bridge.encoder)
> - drm_helper_hpd_irq_event(adv7511->connector.dev);
> + schedule_work(>irq_work);
> 
>   if (irq0 & ADV7511_INT0_EDID_READY || irq1 & ADV7511_INT1_DDC_ERROR) {
>   adv7511->edid_read = true;
> @@ -1006,6 +1014,8 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) goto err_i2c_unregister_edid;
>   }
> 
> + INIT_WORK(>irq_work, adv7511_irq_work);
> +
>   if (i2c->irq) {
>   init_waitqueue_head(>wq);

-- 
Regards,

Laurent Pinchart

Re: [RFC][PATCH 1/5 v2] drm/bridge: adv7511: Use work_struct to defer hotplug handing to out of irq context

[Laurent Pinchart]

Hi John,

Thank you for the patch.

On Monday 28 Nov 2016 21:04:40 John Stultz wrote:
> I was recently seeing issues with EDID probing, where
> the logic to wait for the EDID read bit to be set by the
> IRQ wasn't happening and the code would time out and fail.
> 
> Digging deeper, I found this was due to the fact that
> IRQs were disabled as we were running in IRQ context from
> the HPD signal.
> 
> Thus this patch changes the logic to handle the HPD signal
> via a work_struct so we can be out of irq context.
> 
> With this patch, the EDID probing on hotplug does not time
> out.
> 
> Cc: David Airlie 
> Cc: Archit Taneja 
> Cc: Wolfram Sang 
> Cc: Lars-Peter Clausen 
> Cc: Laurent Pinchart 
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: John Stultz 
> ---
> v2: Reworked to properly fix the issue rather then
> just delaying for 200ms
> 
>  drivers/gpu/drm/bridge/adv7511/adv7511.h |  2 ++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 12 +++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 992d76c..2a1e722 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -317,6 +317,8 @@ struct adv7511 {
>   bool edid_read;
> 
>   wait_queue_head_t wq;
> + struct work_struct irq_work;
> +

I'd call this hpd_work.

>   struct drm_bridge bridge;
>   struct drm_connector connector;
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8dba729..b38e743
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -402,6 +402,14 @@ static bool adv7511_hpd(struct adv7511 *adv7511)
>   return false;
>  }
> 
> +static void adv7511_irq_work(struct work_struct *work)
> +{
> + struct adv7511 *adv7511 = container_of(work, struct adv7511, 
irq_work);
> +
> + drm_helper_hpd_irq_event(adv7511->connector.dev);
> +}
> +
> +

One blank line is enough.

Apart from that,

Reviewed-by: Laurent Pinchart 

>  static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd)
>  {
>   unsigned int irq0, irq1;
> @@ -419,7 +427,7 @@ static int adv7511_irq_process(struct adv7511 *adv7511,
> bool process_hpd) regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1);
> 
>   if (process_hpd && irq0 & ADV7511_INT0_HPD && adv7511->bridge.encoder)
> - drm_helper_hpd_irq_event(adv7511->connector.dev);
> + schedule_work(>irq_work);
> 
>   if (irq0 & ADV7511_INT0_EDID_READY || irq1 & ADV7511_INT1_DDC_ERROR) {
>   adv7511->edid_read = true;
> @@ -1006,6 +1014,8 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) goto err_i2c_unregister_edid;
>   }
> 
> + INIT_WORK(>irq_work, adv7511_irq_work);
> +
>   if (i2c->irq) {
>   init_waitqueue_head(>wq);

-- 
Regards,

Laurent Pinchart

[patch net v2] net: fec: cache statistics while device is down

[Nikita Yushchenko]

Execution 'ethtool -S' on fec device that is down causes OOPS on Vybrid
board:

Unhandled fault: external abort on non-linefetch (0x1008) at 0xe0898200
pgd = ddecc000
[e0898200] *pgd=9e406811, *pte=400d1653, *ppte=400d1453
Internal error: : 1008 [#1] SMP ARM
...

Reason of OOPS is that fec_enet_get_ethtool_stats() accesses fec
registers while IPG clock is stopped by PM.

Fix that by caching statistics in fec_enet_private. Cache is initialized
at device probe time, and updated at statistics request time if device
is up, and also just before turning device off on down path.

Additional locking is not needed, since cached statistics is accessed
either before device is registered, or under rtnl_lock().

Signed-off-by: Nikita Yushchenko 
---
Changes since v1:
- initialize cache at device probe time

 drivers/net/ethernet/freescale/fec.h  |  2 ++
 drivers/net/ethernet/freescale/fec_main.c | 23 +++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h 
b/drivers/net/ethernet/freescale/fec.h
index c865135f3cb9..5ea740b4cf14 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -574,6 +574,8 @@ struct fec_enet_private {
unsigned int reload_period;
int pps_enable;
unsigned int next_counter;
+
+   u64 ethtool_stats[0];
 };
 
 void fec_ptp_init(struct platform_device *pdev);
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index 5aa9d4ded214..6a20c24a2003 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2313,14 +2313,24 @@ static const struct fec_stat {
{ "IEEE_rx_octets_ok", IEEE_R_OCTETS_OK },
 };
 
-static void fec_enet_get_ethtool_stats(struct net_device *dev,
-   struct ethtool_stats *stats, u64 *data)
+static void fec_enet_update_ethtool_stats(struct net_device *dev)
 {
struct fec_enet_private *fep = netdev_priv(dev);
int i;
 
for (i = 0; i < ARRAY_SIZE(fec_stats); i++)
-   data[i] = readl(fep->hwp + fec_stats[i].offset);
+   fep->ethtool_stats[i] = readl(fep->hwp + fec_stats[i].offset);
+}
+
+static void fec_enet_get_ethtool_stats(struct net_device *dev,
+  struct ethtool_stats *stats, u64 *data)
+{
+   struct fec_enet_private *fep = netdev_priv(dev);
+
+   if (netif_running(dev))
+   fec_enet_update_ethtool_stats(dev);
+
+   memcpy(data, fep->ethtool_stats, ARRAY_SIZE(fec_stats) * sizeof(u64));
 }
 
 static void fec_enet_get_strings(struct net_device *netdev,
@@ -2874,6 +2884,8 @@ fec_enet_close(struct net_device *ndev)
if (fep->quirks & FEC_QUIRK_ERR006687)
imx6q_cpuidle_fec_irqs_unused();
 
+   fec_enet_update_ethtool_stats(ndev);
+
fec_enet_clk_enable(ndev, false);
pinctrl_pm_select_sleep_state(>pdev->dev);
pm_runtime_mark_last_busy(>pdev->dev);
@@ -3180,6 +3192,8 @@ static int fec_enet_init(struct net_device *ndev)
 
fec_restart(ndev);
 
+   fec_enet_update_ethtool_stats(ndev);
+
return 0;
 }
 
@@ -3278,7 +3292,8 @@ fec_probe(struct platform_device *pdev)
fec_enet_get_queue_num(pdev, _tx_qs, _rx_qs);
 
/* Init network device */
-   ndev = alloc_etherdev_mqs(sizeof(struct fec_enet_private),
+   ndev = alloc_etherdev_mqs(sizeof(struct fec_enet_private) +
+ ARRAY_SIZE(fec_stats) * sizeof(u64),
  num_tx_qs, num_rx_qs);
if (!ndev)
return -ENOMEM;
-- 
2.1.4

[patch net v2] net: fec: cache statistics while device is down

[Nikita Yushchenko]

Execution 'ethtool -S' on fec device that is down causes OOPS on Vybrid
board:

Unhandled fault: external abort on non-linefetch (0x1008) at 0xe0898200
pgd = ddecc000
[e0898200] *pgd=9e406811, *pte=400d1653, *ppte=400d1453
Internal error: : 1008 [#1] SMP ARM
...

Reason of OOPS is that fec_enet_get_ethtool_stats() accesses fec
registers while IPG clock is stopped by PM.

Fix that by caching statistics in fec_enet_private. Cache is initialized
at device probe time, and updated at statistics request time if device
is up, and also just before turning device off on down path.

Additional locking is not needed, since cached statistics is accessed
either before device is registered, or under rtnl_lock().

Signed-off-by: Nikita Yushchenko 
---
Changes since v1:
- initialize cache at device probe time

 drivers/net/ethernet/freescale/fec.h  |  2 ++
 drivers/net/ethernet/freescale/fec_main.c | 23 +++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h 
b/drivers/net/ethernet/freescale/fec.h
index c865135f3cb9..5ea740b4cf14 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -574,6 +574,8 @@ struct fec_enet_private {
unsigned int reload_period;
int pps_enable;
unsigned int next_counter;
+
+   u64 ethtool_stats[0];
 };
 
 void fec_ptp_init(struct platform_device *pdev);
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index 5aa9d4ded214..6a20c24a2003 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2313,14 +2313,24 @@ static const struct fec_stat {
{ "IEEE_rx_octets_ok", IEEE_R_OCTETS_OK },
 };
 
-static void fec_enet_get_ethtool_stats(struct net_device *dev,
-   struct ethtool_stats *stats, u64 *data)
+static void fec_enet_update_ethtool_stats(struct net_device *dev)
 {
struct fec_enet_private *fep = netdev_priv(dev);
int i;
 
for (i = 0; i < ARRAY_SIZE(fec_stats); i++)
-   data[i] = readl(fep->hwp + fec_stats[i].offset);
+   fep->ethtool_stats[i] = readl(fep->hwp + fec_stats[i].offset);
+}
+
+static void fec_enet_get_ethtool_stats(struct net_device *dev,
+  struct ethtool_stats *stats, u64 *data)
+{
+   struct fec_enet_private *fep = netdev_priv(dev);
+
+   if (netif_running(dev))
+   fec_enet_update_ethtool_stats(dev);
+
+   memcpy(data, fep->ethtool_stats, ARRAY_SIZE(fec_stats) * sizeof(u64));
 }
 
 static void fec_enet_get_strings(struct net_device *netdev,
@@ -2874,6 +2884,8 @@ fec_enet_close(struct net_device *ndev)
if (fep->quirks & FEC_QUIRK_ERR006687)
imx6q_cpuidle_fec_irqs_unused();
 
+   fec_enet_update_ethtool_stats(ndev);
+
fec_enet_clk_enable(ndev, false);
pinctrl_pm_select_sleep_state(>pdev->dev);
pm_runtime_mark_last_busy(>pdev->dev);
@@ -3180,6 +3192,8 @@ static int fec_enet_init(struct net_device *ndev)
 
fec_restart(ndev);
 
+   fec_enet_update_ethtool_stats(ndev);
+
return 0;
 }
 
@@ -3278,7 +3292,8 @@ fec_probe(struct platform_device *pdev)
fec_enet_get_queue_num(pdev, _tx_qs, _rx_qs);
 
/* Init network device */
-   ndev = alloc_etherdev_mqs(sizeof(struct fec_enet_private),
+   ndev = alloc_etherdev_mqs(sizeof(struct fec_enet_private) +
+ ARRAY_SIZE(fec_stats) * sizeof(u64),
  num_tx_qs, num_rx_qs);
if (!ndev)
return -ENOMEM;
-- 
2.1.4

[PATCH V5 06/10] PM / OPP: Add infrastructure to manage multiple regulators

[Viresh Kumar]

This patch adds infrastructure to manage multiple regulators and updates
the only user (cpufreq-dt) of dev_pm_opp_set{put}_regulator().

This is preparatory work for adding full support for devices with
multiple regulators.

Signed-off-by: Viresh Kumar 
Tested-by: Dave Gerlach 

---
V4->V5:
- Don't allocate from within rcu locked section.
- s/+//
---
 drivers/base/power/opp/core.c| 246 +++
 drivers/base/power/opp/debugfs.c |  52 +++--
 drivers/base/power/opp/of.c  | 103 +++-
 drivers/base/power/opp/opp.h |  10 +-
 drivers/cpufreq/cpufreq-dt.c |   9 +-
 include/linux/pm_opp.h   |   8 +-
 6 files changed, 301 insertions(+), 127 deletions(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 37fad2eb0f47..89a3fd720724 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -93,6 +93,8 @@ struct opp_table *_find_opp_table(struct device *dev)
  * Return: voltage in micro volt corresponding to the opp, else
  * return 0
  *
+ * This is useful only for devices with single power supply.
+ *
  * Locking: This function must be called under rcu_read_lock(). opp is a rcu
  * protected pointer. This means that opp which could have been fetched by
  * opp_find_freq_{exact,ceil,floor} functions is valid as long as we are
@@ -112,7 +114,7 @@ unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
if (IS_ERR_OR_NULL(tmp_opp))
pr_err("%s: Invalid parameters\n", __func__);
else
-   v = tmp_opp->supply.u_volt;
+   v = tmp_opp->supplies[0].u_volt;
 
return v;
 }
@@ -210,6 +212,24 @@ unsigned long dev_pm_opp_get_max_clock_latency(struct 
device *dev)
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_clock_latency);
 
+static int _get_regulator_count(struct device *dev)
+{
+   struct opp_table *opp_table;
+   int count;
+
+   rcu_read_lock();
+
+   opp_table = _find_opp_table(dev);
+   if (!IS_ERR(opp_table))
+   count = opp_table->regulator_count;
+   else
+   count = 0;
+
+   rcu_read_unlock();
+
+   return count;
+}
+
 /**
  * dev_pm_opp_get_max_volt_latency() - Get max voltage latency in nanoseconds
  * @dev: device for which we do this operation
@@ -222,34 +242,51 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct 
device *dev)
 {
struct opp_table *opp_table;
struct dev_pm_opp *opp;
-   struct regulator *reg;
+   struct regulator *reg, **regulators;
unsigned long latency_ns = 0;
-   unsigned long min_uV = ~0, max_uV = 0;
-   int ret;
+   int ret, i, count;
+   struct {
+   unsigned long min;
+   unsigned long max;
+   } *uV;
+
+   count = _get_regulator_count(dev);
+
+   /* Regulator may not be required for the device */
+   if (!count)
+   return 0;
+
+   regulators = kmalloc_array(count, sizeof(*regulators), GFP_KERNEL);
+   if (!regulators)
+   return 0;
+
+   uV = kmalloc_array(count, sizeof(*uV), GFP_KERNEL);
+   if (!uV)
+   goto free_regulators;
 
rcu_read_lock();
 
opp_table = _find_opp_table(dev);
if (IS_ERR(opp_table)) {
rcu_read_unlock();
-   return 0;
+   goto free_uV;
}
 
-   reg = opp_table->regulator;
-   if (IS_ERR(reg)) {
-   /* Regulator may not be required for device */
-   rcu_read_unlock();
-   return 0;
-   }
+   memcpy(regulators, opp_table->regulators, count * sizeof(*regulators));
 
-   list_for_each_entry_rcu(opp, _table->opp_list, node) {
-   if (!opp->available)
-   continue;
+   for (i = 0; i < count; i++) {
+   uV[i].min = ~0;
+   uV[i].max = 0;
 
-   if (opp->supply.u_volt_min < min_uV)
-   min_uV = opp->supply.u_volt_min;
-   if (opp->supply.u_volt_max > max_uV)
-   max_uV = opp->supply.u_volt_max;
+   list_for_each_entry_rcu(opp, _table->opp_list, node) {
+   if (!opp->available)
+   continue;
+
+   if (opp->supplies[i].u_volt_min < uV[i].min)
+   uV[i].min = opp->supplies[i].u_volt_min;
+   if (opp->supplies[i].u_volt_max > uV[i].max)
+   uV[i].max = opp->supplies[i].u_volt_max;
+   }
}
 
rcu_read_unlock();
@@ -258,9 +295,16 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct 
device *dev)
 * The caller needs to ensure that opp_table (and hence the regulator)
 * isn't freed, while we are executing this routine.
 */
-   ret = regulator_set_voltage_time(reg, min_uV, max_uV);
-   if (ret > 0)
-

[PATCH V5 08/10] PM / OPP: Allow platform specific custom set_opp() callbacks

[Viresh Kumar]

The generic set_opp() handler isn't sufficient for platforms with
complex DVFS.  For example, some TI platforms have multiple regulators
for a CPU device. The order in which various supplies need to be
programmed is only known to the platform code and its best to leave it
to it.

This patch implements APIs to register platform specific set_opp()
callback.

Signed-off-by: Viresh Kumar 
Tested-by: Dave Gerlach 

---
V4->V5:
- s/custom OPP set rate/custom set OPP/
- set_opp() doesn't have a separate 'dev' argument now.
---
 drivers/base/power/opp/core.c | 114 +-
 drivers/base/power/opp/opp.h  |   2 +
 include/linux/pm_opp.h|  10 
 3 files changed, 125 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 3a0b9d993c42..ddd4915ffd4f 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -687,6 +687,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long 
target_freq)
 {
struct opp_table *opp_table;
unsigned long freq, old_freq;
+   int (*set_opp)(struct dev_pm_set_opp_data *data);
struct dev_pm_opp *old_opp, *opp;
struct regulator **regulators;
struct dev_pm_set_opp_data *data;
@@ -751,6 +752,11 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long 
target_freq)
return _generic_set_opp_clk_only(dev, clk, old_freq, freq);
}
 
+   if (opp_table->set_opp)
+   set_opp = opp_table->set_opp;
+   else
+   set_opp = _generic_set_opp;
+
data = opp_table->set_opp_data;
data->regulators = regulators;
data->regulator_count = opp_table->regulator_count;
@@ -769,7 +775,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long 
target_freq)
 
rcu_read_unlock();
 
-   return _generic_set_opp(data);
+   return set_opp(data);
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_set_rate);
 
@@ -903,6 +909,9 @@ static void _remove_opp_table(struct opp_table *opp_table)
if (opp_table->regulators)
return;
 
+   if (opp_table->set_opp)
+   return;
+
/* Release clk */
if (!IS_ERR(opp_table->clk))
clk_put(opp_table->clk);
@@ -1578,6 +1587,109 @@ void dev_pm_opp_put_regulators(struct device *dev)
 EXPORT_SYMBOL_GPL(dev_pm_opp_put_regulators);
 
 /**
+ * dev_pm_opp_register_set_opp_helper() - Register custom set OPP helper
+ * @dev: Device for which the helper is getting registered.
+ * @set_opp: Custom set OPP helper.
+ *
+ * This is useful to support complex platforms (like platforms with multiple
+ * regulators per device), instead of the generic OPP set rate helper.
+ *
+ * This must be called before any OPPs are initialized for the device.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+int dev_pm_opp_register_set_opp_helper(struct device *dev,
+   int (*set_opp)(struct dev_pm_set_opp_data *data))
+{
+   struct opp_table *opp_table;
+   int ret;
+
+   if (!set_opp)
+   return -EINVAL;
+
+   mutex_lock(_table_lock);
+
+   opp_table = _add_opp_table(dev);
+   if (!opp_table) {
+   ret = -ENOMEM;
+   goto unlock;
+   }
+
+   /* This should be called before OPPs are initialized */
+   if (WARN_ON(!list_empty(_table->opp_list))) {
+   ret = -EBUSY;
+   goto err;
+   }
+
+   /* Already have custom set_opp helper */
+   if (WARN_ON(opp_table->set_opp)) {
+   ret = -EBUSY;
+   goto err;
+   }
+
+   opp_table->set_opp = set_opp;
+
+   mutex_unlock(_table_lock);
+   return 0;
+
+err:
+   _remove_opp_table(opp_table);
+unlock:
+   mutex_unlock(_table_lock);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_register_set_opp_helper);
+
+/**
+ * dev_pm_opp_register_put_opp_helper() - Releases resources blocked for
+ *set_opp helper
+ * @dev: Device for which custom set_opp helper has to be cleared.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+void dev_pm_opp_register_put_opp_helper(struct device *dev)
+{
+   struct opp_table *opp_table;
+
+   mutex_lock(_table_lock);
+
+   /* Check for existing table for 'dev' first */
+   

[PATCH V5 04/10] PM / OPP: Manage supply's voltage/current in a separate structure

[Viresh Kumar]

This is a preparatory step for multiple regulator per device support.
Move the voltage/current variables to a new structure.

Signed-off-by: Viresh Kumar 
Tested-by: Dave Gerlach 
Reviewed-by: Stephen Boyd 
---
 drivers/base/power/opp/core.c| 44 +---
 drivers/base/power/opp/debugfs.c |  8 
 drivers/base/power/opp/of.c  | 18 
 drivers/base/power/opp/opp.h | 11 +++---
 include/linux/pm_opp.h   | 16 +++
 5 files changed, 55 insertions(+), 42 deletions(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 056527a3fb4e..8d6006151c9a 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -112,7 +112,7 @@ unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
if (IS_ERR_OR_NULL(tmp_opp))
pr_err("%s: Invalid parameters\n", __func__);
else
-   v = tmp_opp->u_volt;
+   v = tmp_opp->supply.u_volt;
 
return v;
 }
@@ -246,10 +246,10 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct 
device *dev)
if (!opp->available)
continue;
 
-   if (opp->u_volt_min < min_uV)
-   min_uV = opp->u_volt_min;
-   if (opp->u_volt_max > max_uV)
-   max_uV = opp->u_volt_max;
+   if (opp->supply.u_volt_min < min_uV)
+   min_uV = opp->supply.u_volt_min;
+   if (opp->supply.u_volt_max > max_uV)
+   max_uV = opp->supply.u_volt_max;
}
 
rcu_read_unlock();
@@ -637,14 +637,14 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long 
target_freq)
if (IS_ERR(old_opp)) {
old_u_volt = 0;
} else {
-   old_u_volt = old_opp->u_volt;
-   old_u_volt_min = old_opp->u_volt_min;
-   old_u_volt_max = old_opp->u_volt_max;
+   old_u_volt = old_opp->supply.u_volt;
+   old_u_volt_min = old_opp->supply.u_volt_min;
+   old_u_volt_max = old_opp->supply.u_volt_max;
}
 
-   u_volt = opp->u_volt;
-   u_volt_min = opp->u_volt_min;
-   u_volt_max = opp->u_volt_max;
+   u_volt = opp->supply.u_volt;
+   u_volt_min = opp->supply.u_volt_min;
+   u_volt_max = opp->supply.u_volt_max;
 
reg = opp_table->regulator;
 
@@ -957,10 +957,11 @@ static bool _opp_supported_by_regulators(struct 
dev_pm_opp *opp,
struct regulator *reg = opp_table->regulator;
 
if (!IS_ERR(reg) &&
-   !regulator_is_supported_voltage(reg, opp->u_volt_min,
-   opp->u_volt_max)) {
+   !regulator_is_supported_voltage(reg, opp->supply.u_volt_min,
+   opp->supply.u_volt_max)) {
pr_warn("%s: OPP minuV: %lu maxuV: %lu, not supported by 
regulator\n",
-   __func__, opp->u_volt_min, opp->u_volt_max);
+   __func__, opp->supply.u_volt_min,
+   opp->supply.u_volt_max);
return false;
}
 
@@ -993,11 +994,12 @@ int _opp_add(struct device *dev, struct dev_pm_opp 
*new_opp,
 
/* Duplicate OPPs */
dev_warn(dev, "%s: duplicate OPPs detected. Existing: freq: 
%lu, volt: %lu, enabled: %d. New: freq: %lu, volt: %lu, enabled: %d\n",
-__func__, opp->rate, opp->u_volt, opp->available,
-new_opp->rate, new_opp->u_volt, new_opp->available);
+__func__, opp->rate, opp->supply.u_volt,
+opp->available, new_opp->rate, new_opp->supply.u_volt,
+new_opp->available);
 
-   return opp->available && new_opp->u_volt == opp->u_volt ?
-   0 : -EEXIST;
+   return opp->available &&
+  new_opp->supply.u_volt == opp->supply.u_volt ? 0 : 
-EEXIST;
}
 
new_opp->opp_table = opp_table;
@@ -1064,9 +1066,9 @@ int _opp_add_v1(struct device *dev, unsigned long freq, 
long u_volt,
/* populate the opp table */
new_opp->rate = freq;
tol = u_volt * opp_table->voltage_tolerance_v1 / 100;
-   new_opp->u_volt = u_volt;
-   new_opp->u_volt_min = u_volt - tol;
-   new_opp->u_volt_max = u_volt + tol;
+   new_opp->supply.u_volt = u_volt;
+   new_opp->supply.u_volt_min = u_volt - tol;
+   new_opp->supply.u_volt_max = u_volt + tol;
new_opp->available = true;
new_opp->dynamic = dynamic;
 
diff --git a/drivers/base/power/opp/debugfs.c b/drivers/base/power/opp/debugfs.c
index ef1ae6b52042..c897676ca35f 100644
--- a/drivers/base/power/opp/debugfs.c
+++ b/drivers/base/power/opp/debugfs.c
@@ -63,16 +63,16 @@ int opp_debug_create_one(struct dev_pm_opp *opp, struct 

[PATCH V5 08/10] PM / OPP: Allow platform specific custom set_opp() callbacks

[Viresh Kumar]

The generic set_opp() handler isn't sufficient for platforms with
complex DVFS.  For example, some TI platforms have multiple regulators
for a CPU device. The order in which various supplies need to be
programmed is only known to the platform code and its best to leave it
to it.

This patch implements APIs to register platform specific set_opp()
callback.

Signed-off-by: Viresh Kumar 
Tested-by: Dave Gerlach 

---
V4->V5:
- s/custom OPP set rate/custom set OPP/
- set_opp() doesn't have a separate 'dev' argument now.
---
 drivers/base/power/opp/core.c | 114 +-
 drivers/base/power/opp/opp.h  |   2 +
 include/linux/pm_opp.h|  10 
 3 files changed, 125 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 3a0b9d993c42..ddd4915ffd4f 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -687,6 +687,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long 
target_freq)
 {
struct opp_table *opp_table;
unsigned long freq, old_freq;
+   int (*set_opp)(struct dev_pm_set_opp_data *data);
struct dev_pm_opp *old_opp, *opp;
struct regulator **regulators;
struct dev_pm_set_opp_data *data;
@@ -751,6 +752,11 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long 
target_freq)
return _generic_set_opp_clk_only(dev, clk, old_freq, freq);
}
 
+   if (opp_table->set_opp)
+   set_opp = opp_table->set_opp;
+   else
+   set_opp = _generic_set_opp;
+
data = opp_table->set_opp_data;
data->regulators = regulators;
data->regulator_count = opp_table->regulator_count;
@@ -769,7 +775,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long 
target_freq)
 
rcu_read_unlock();
 
-   return _generic_set_opp(data);
+   return set_opp(data);
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_set_rate);
 
@@ -903,6 +909,9 @@ static void _remove_opp_table(struct opp_table *opp_table)
if (opp_table->regulators)
return;
 
+   if (opp_table->set_opp)
+   return;
+
/* Release clk */
if (!IS_ERR(opp_table->clk))
clk_put(opp_table->clk);
@@ -1578,6 +1587,109 @@ void dev_pm_opp_put_regulators(struct device *dev)
 EXPORT_SYMBOL_GPL(dev_pm_opp_put_regulators);
 
 /**
+ * dev_pm_opp_register_set_opp_helper() - Register custom set OPP helper
+ * @dev: Device for which the helper is getting registered.
+ * @set_opp: Custom set OPP helper.
+ *
+ * This is useful to support complex platforms (like platforms with multiple
+ * regulators per device), instead of the generic OPP set rate helper.
+ *
+ * This must be called before any OPPs are initialized for the device.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+int dev_pm_opp_register_set_opp_helper(struct device *dev,
+   int (*set_opp)(struct dev_pm_set_opp_data *data))
+{
+   struct opp_table *opp_table;
+   int ret;
+
+   if (!set_opp)
+   return -EINVAL;
+
+   mutex_lock(_table_lock);
+
+   opp_table = _add_opp_table(dev);
+   if (!opp_table) {
+   ret = -ENOMEM;
+   goto unlock;
+   }
+
+   /* This should be called before OPPs are initialized */
+   if (WARN_ON(!list_empty(_table->opp_list))) {
+   ret = -EBUSY;
+   goto err;
+   }
+
+   /* Already have custom set_opp helper */
+   if (WARN_ON(opp_table->set_opp)) {
+   ret = -EBUSY;
+   goto err;
+   }
+
+   opp_table->set_opp = set_opp;
+
+   mutex_unlock(_table_lock);
+   return 0;
+
+err:
+   _remove_opp_table(opp_table);
+unlock:
+   mutex_unlock(_table_lock);
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_register_set_opp_helper);
+
+/**
+ * dev_pm_opp_register_put_opp_helper() - Releases resources blocked for
+ *set_opp helper
+ * @dev: Device for which custom set_opp helper has to be cleared.
+ *
+ * Locking: The internal opp_table and opp structures are RCU protected.
+ * Hence this function internally uses RCU updater strategy with mutex locks
+ * to keep the integrity of the internal data structures. Callers should ensure
+ * that this function is *NOT* called under RCU protection or in contexts where
+ * mutex cannot be locked.
+ */
+void dev_pm_opp_register_put_opp_helper(struct device *dev)
+{
+   struct opp_table *opp_table;
+
+   mutex_lock(_table_lock);
+
+   /* Check for existing table for 'dev' first */
+   opp_table = _find_opp_table(dev);
+   if