Re: drm/radeon spamming alloc_contig_range: [xxx, yyy) PFNs busy busy

[Robin H. Johnson]

On Thu, Dec 01, 2016 at 08:38:15AM +0100, Vlastimil Babka wrote:
> >> By default config this should not be used on x86.
> > What do you mean by that statement?
> 
> I mean that the 16 mbytes for generic CMA area is not a default on x86:
> 
> config CMA_SIZE_MBYTES
>  int "Size in Mega Bytes"
>  depends on !CMA_SIZE_SEL_PERCENTAGE
>  default 0 if X86
>  default 16
d7be003a9d275299f5ee36bbdf156654f59e08e9 (v3.18-2122-gd7be003a9d27)
is there the 0MB if-x86 default was added to the tree. Prior to that, it
was 16MiB, and that's where my system picked up the value from.

I have a record of all my kconfigs, because I use oldconfig each time
(going back 8 years to 2.6.27)

# Added in 3.12.0-1-g5f258d0
CONFIG_CMA=y 
# Added in 3.16.0-rc6-00042-g67dd8f3
CONFIG_CMA_ALIGNMENT=8
CONFIG_CMA_AREAS=7
CONFIG_CMA_SIZE_MBYTES=16
CONFIG_CMA_SIZE_SEL_MBYTES=y
CONFIG_DMA_CMA=y

So the next question, is why did I pick up CMA in
3.16.0-rc6-00042-g67dd8f3... I'll poke at that.

> > Yes, I'd say if there's a fallback without much penalty, nowarn makes
> > sense. If the fallback just tries multiple addresses until success, then
> > the warning should only be issued when too many attempts have been made.
> On the other hand, if the warnings are correlated with high kernel CPU usage, 
> it's arguably better to be warned.
Keep the rate-limit on the warning for cases like this?

> >> > The rate of the problem starts slow, and also is relatively low on an 
> >> > idle
> >> > system (my screens blank at night, no xscreensaver running), but it 
> >> > still ramps
> >> > up over time (to the point of generating 2.5GB/hour of "(timestamp)
> >> > alloc_contig_range: [83e4d9, 83e4da) PFNs busy"), with various addresses 
> >> > (~100
> >> > unique ranges for a day).
> >> >
> >> > My X workload is ~50 chrome tabs and ~20 terminals (over 3x 24" monitors 
> >> > w/ 9
> >> > virtual desktops per monitor).
> >> So IIUC, except the messages, everything actually works fine?
> > There's high kernel CPU usage that seems to roughly correlate with the
> > messages, but I can't yet tell if that's due to the syslog itself, or
> > repeated alloc_contig_range requests.
> You could try running perf top.
Will do in the morning.

-- 
Robin Hugh Johnson
Gentoo Linux: Dev, Infra Lead, Foundation Trustee & Treasurer
E-Mail   : robb...@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136

Re: drm/radeon spamming alloc_contig_range: [xxx, yyy) PFNs busy busy

[Robin H. Johnson]

On Thu, Dec 01, 2016 at 08:38:15AM +0100, Vlastimil Babka wrote:
> >> By default config this should not be used on x86.
> > What do you mean by that statement?
> 
> I mean that the 16 mbytes for generic CMA area is not a default on x86:
> 
> config CMA_SIZE_MBYTES
>  int "Size in Mega Bytes"
>  depends on !CMA_SIZE_SEL_PERCENTAGE
>  default 0 if X86
>  default 16
d7be003a9d275299f5ee36bbdf156654f59e08e9 (v3.18-2122-gd7be003a9d27)
is there the 0MB if-x86 default was added to the tree. Prior to that, it
was 16MiB, and that's where my system picked up the value from.

I have a record of all my kconfigs, because I use oldconfig each time
(going back 8 years to 2.6.27)

# Added in 3.12.0-1-g5f258d0
CONFIG_CMA=y 
# Added in 3.16.0-rc6-00042-g67dd8f3
CONFIG_CMA_ALIGNMENT=8
CONFIG_CMA_AREAS=7
CONFIG_CMA_SIZE_MBYTES=16
CONFIG_CMA_SIZE_SEL_MBYTES=y
CONFIG_DMA_CMA=y

So the next question, is why did I pick up CMA in
3.16.0-rc6-00042-g67dd8f3... I'll poke at that.

> > Yes, I'd say if there's a fallback without much penalty, nowarn makes
> > sense. If the fallback just tries multiple addresses until success, then
> > the warning should only be issued when too many attempts have been made.
> On the other hand, if the warnings are correlated with high kernel CPU usage, 
> it's arguably better to be warned.
Keep the rate-limit on the warning for cases like this?

> >> > The rate of the problem starts slow, and also is relatively low on an 
> >> > idle
> >> > system (my screens blank at night, no xscreensaver running), but it 
> >> > still ramps
> >> > up over time (to the point of generating 2.5GB/hour of "(timestamp)
> >> > alloc_contig_range: [83e4d9, 83e4da) PFNs busy"), with various addresses 
> >> > (~100
> >> > unique ranges for a day).
> >> >
> >> > My X workload is ~50 chrome tabs and ~20 terminals (over 3x 24" monitors 
> >> > w/ 9
> >> > virtual desktops per monitor).
> >> So IIUC, except the messages, everything actually works fine?
> > There's high kernel CPU usage that seems to roughly correlate with the
> > messages, but I can't yet tell if that's due to the syslog itself, or
> > repeated alloc_contig_range requests.
> You could try running perf top.
Will do in the morning.

-- 
Robin Hugh Johnson
Gentoo Linux: Dev, Infra Lead, Foundation Trustee & Treasurer
E-Mail   : robb...@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136

Re: [PATCH v3 1/2] mmc: sdhci-pci: Add PCI ID for Intel byt sdio host controller sub-vended by NI

[Adrian Hunter]

On 28/11/16 21:16, Zach Brown wrote:
> Add PCI ID for Intel byt sdio host controller sub-vended by NI.
> 
> The controller has different behavior because of the board layout NI
> puts it on.
> 
> Signed-off-by: Zach Brown 

Acked-by: Adrian Hunter 

> ---
>  drivers/mmc/host/sdhci-pci-core.c | 24 
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-pci-core.c 
> b/drivers/mmc/host/sdhci-pci-core.c
> index 1d9e00a..9741505 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -375,6 +375,13 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot 
> *slot)
>   return 0;
>  }
>  
> +static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
> +{
> + slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
> +  MMC_CAP_WAIT_WHILE_BUSY;
> + return 0;
> +}
> +
>  static int byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
>   slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
> @@ -447,6 +454,15 @@ static const struct sdhci_pci_fixes sdhci_intel_byt_emmc 
> = {
>   .ops= _intel_byt_ops,
>  };
>  
> +static const struct sdhci_pci_fixes sdhci_ni_byt_sdio = {
> + .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
> + .quirks2= SDHCI_QUIRK2_HOST_OFF_CARD_ON |
> +   SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> + .allow_runtime_pm = true,
> + .probe_slot = ni_byt_sdio_probe_slot,
> + .ops= _intel_byt_ops,
> +};
> +
>  static const struct sdhci_pci_fixes sdhci_intel_byt_sdio = {
>   .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
>   .quirks2= SDHCI_QUIRK2_HOST_OFF_CARD_ON |
> @@ -1079,6 +1095,14 @@ static const struct pci_device_id pci_ids[] = {
>   {
>   .vendor = PCI_VENDOR_ID_INTEL,
>   .device = PCI_DEVICE_ID_INTEL_BYT_SDIO,
> + .subvendor  = PCI_VENDOR_ID_NI,
> + .subdevice  = 0x7884,
> + .driver_data= (kernel_ulong_t)_ni_byt_sdio,
> + },
> +
> + {
> + .vendor = PCI_VENDOR_ID_INTEL,
> + .device = PCI_DEVICE_ID_INTEL_BYT_SDIO,
>   .subvendor  = PCI_ANY_ID,
>   .subdevice  = PCI_ANY_ID,
>   .driver_data= (kernel_ulong_t)_intel_byt_sdio,
> 

Re: [PATCH v3 1/2] mmc: sdhci-pci: Add PCI ID for Intel byt sdio host controller sub-vended by NI

[Adrian Hunter]

On 28/11/16 21:16, Zach Brown wrote:
> Add PCI ID for Intel byt sdio host controller sub-vended by NI.
> 
> The controller has different behavior because of the board layout NI
> puts it on.
> 
> Signed-off-by: Zach Brown 

Acked-by: Adrian Hunter 

> ---
>  drivers/mmc/host/sdhci-pci-core.c | 24 
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-pci-core.c 
> b/drivers/mmc/host/sdhci-pci-core.c
> index 1d9e00a..9741505 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -375,6 +375,13 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot 
> *slot)
>   return 0;
>  }
>  
> +static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
> +{
> + slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
> +  MMC_CAP_WAIT_WHILE_BUSY;
> + return 0;
> +}
> +
>  static int byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
>   slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
> @@ -447,6 +454,15 @@ static const struct sdhci_pci_fixes sdhci_intel_byt_emmc 
> = {
>   .ops= _intel_byt_ops,
>  };
>  
> +static const struct sdhci_pci_fixes sdhci_ni_byt_sdio = {
> + .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
> + .quirks2= SDHCI_QUIRK2_HOST_OFF_CARD_ON |
> +   SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> + .allow_runtime_pm = true,
> + .probe_slot = ni_byt_sdio_probe_slot,
> + .ops= _intel_byt_ops,
> +};
> +
>  static const struct sdhci_pci_fixes sdhci_intel_byt_sdio = {
>   .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
>   .quirks2= SDHCI_QUIRK2_HOST_OFF_CARD_ON |
> @@ -1079,6 +1095,14 @@ static const struct pci_device_id pci_ids[] = {
>   {
>   .vendor = PCI_VENDOR_ID_INTEL,
>   .device = PCI_DEVICE_ID_INTEL_BYT_SDIO,
> + .subvendor  = PCI_VENDOR_ID_NI,
> + .subdevice  = 0x7884,
> + .driver_data= (kernel_ulong_t)_ni_byt_sdio,
> + },
> +
> + {
> + .vendor = PCI_VENDOR_ID_INTEL,
> + .device = PCI_DEVICE_ID_INTEL_BYT_SDIO,
>   .subvendor  = PCI_ANY_ID,
>   .subdevice  = PCI_ANY_ID,
>   .driver_data= (kernel_ulong_t)_intel_byt_sdio,
> 

Re: [PATCH v3 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio controller sub-vended by NI

[Adrian Hunter]

On 28/11/16 21:16, Zach Brown wrote:
> On NI 9037 boards the max SDIO frequency is limited by trace lengths
> and other layout choices. The max SDIO frequency is stored in an ACPI
> table.
> 
> The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
> f_max field of the host.
> 
> Signed-off-by: Nathan Sullivan 
> Reviewed-by: Jaeden Amero 
> Reviewed-by: Josh Cartwright 
> Signed-off-by: Zach Brown 

Acked-by: Adrian Hunter 

> ---
>  drivers/mmc/host/sdhci-pci-core.c | 32 
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-pci-core.c 
> b/drivers/mmc/host/sdhci-pci-core.c
> index 9741505..c9e51b1 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "sdhci.h"
>  #include "sdhci-pci.h"
> @@ -375,8 +376,39 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot 
> *slot)
>   return 0;
>  }
>  
> +#ifdef CONFIG_ACPI
> +static int ni_set_max_freq(struct sdhci_pci_slot *slot)
> +{
> + acpi_status status;
> + unsigned long long max_freq;
> +
> + status = acpi_evaluate_integer(ACPI_HANDLE(>chip->pdev->dev),
> +"MXFQ", NULL, _freq);
> + if (ACPI_FAILURE(status)) {
> + dev_err(>chip->pdev->dev,
> + "MXFQ not found in acpi table\n");
> + return -EINVAL;
> + }
> +
> + slot->host->mmc->f_max = max_freq * 100;
> +
> + return 0;
> +}
> +#else
> +static inline int ni_set_max_freq(struct sdhci_pci_slot *slot)
> +{
> + return 0;
> +}
> +#endif
> +
>  static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
> + int err;
> +
> + err = ni_set_max_freq(slot);
> + if (err)
> + return err;
> +
>   slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
>MMC_CAP_WAIT_WHILE_BUSY;
>   return 0;
> 

Re: [PATCH v3 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio controller sub-vended by NI

[Adrian Hunter]

On 28/11/16 21:16, Zach Brown wrote:
> On NI 9037 boards the max SDIO frequency is limited by trace lengths
> and other layout choices. The max SDIO frequency is stored in an ACPI
> table.
> 
> The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
> f_max field of the host.
> 
> Signed-off-by: Nathan Sullivan 
> Reviewed-by: Jaeden Amero 
> Reviewed-by: Josh Cartwright 
> Signed-off-by: Zach Brown 

Acked-by: Adrian Hunter 

> ---
>  drivers/mmc/host/sdhci-pci-core.c | 32 
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-pci-core.c 
> b/drivers/mmc/host/sdhci-pci-core.c
> index 9741505..c9e51b1 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "sdhci.h"
>  #include "sdhci-pci.h"
> @@ -375,8 +376,39 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot 
> *slot)
>   return 0;
>  }
>  
> +#ifdef CONFIG_ACPI
> +static int ni_set_max_freq(struct sdhci_pci_slot *slot)
> +{
> + acpi_status status;
> + unsigned long long max_freq;
> +
> + status = acpi_evaluate_integer(ACPI_HANDLE(>chip->pdev->dev),
> +"MXFQ", NULL, _freq);
> + if (ACPI_FAILURE(status)) {
> + dev_err(>chip->pdev->dev,
> + "MXFQ not found in acpi table\n");
> + return -EINVAL;
> + }
> +
> + slot->host->mmc->f_max = max_freq * 100;
> +
> + return 0;
> +}
> +#else
> +static inline int ni_set_max_freq(struct sdhci_pci_slot *slot)
> +{
> + return 0;
> +}
> +#endif
> +
>  static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
> + int err;
> +
> + err = ni_set_max_freq(slot);
> + if (err)
> + return err;
> +
>   slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
>MMC_CAP_WAIT_WHILE_BUSY;
>   return 0;
> 

Re: [PATCH 1/6] dax: fix build breakage with ext4, dax and !iomap

[Jan Kara]

On Wed 30-11-16 12:04:31, Ross Zwisler wrote:
> On Tue, Nov 29, 2016 at 09:53:03AM +0100, Jan Kara wrote:
> > On Mon 28-11-16 12:15:04, Ross Zwisler wrote:
> > > On Thu, Nov 24, 2016 at 10:02:39AM +0100, Jan Kara wrote:
> > > > On Wed 23-11-16 11:44:17, Ross Zwisler wrote:
> > > > > With the current Kconfig setup it is possible to have the following:
> > > > > 
> > > > > CONFIG_EXT4_FS=y
> > > > > CONFIG_FS_DAX=y
> > > > > CONFIG_FS_IOMAP=n # this is in fs/Kconfig & isn't user accessible
> > > > > 
> > > > > With this config we get build failures in ext4_dax_fault() because the
> > > > > iomap functions in fs/dax.c are missing:
> > > > > 
> > > > > fs/built-in.o: In function `ext4_dax_fault':
> > > > > file.c:(.text+0x7f3ac): undefined reference to `dax_iomap_fault'
> > > > > file.c:(.text+0x7f404): undefined reference to `dax_iomap_fault'
> > > > > fs/built-in.o: In function `ext4_file_read_iter':
> > > > > file.c:(.text+0x7fc54): undefined reference to `dax_iomap_rw'
> > > > > fs/built-in.o: In function `ext4_file_write_iter':
> > > > > file.c:(.text+0x7fe9a): undefined reference to `dax_iomap_rw'
> > > > > file.c:(.text+0x7feed): undefined reference to `dax_iomap_rw'
> > > > > fs/built-in.o: In function `ext4_block_zero_page_range':
> > > > > inode.c:(.text+0x85c0d): undefined reference to `iomap_zero_range'
> > > > > 
> > > > > Now that the struct buffer_head based DAX fault paths and I/O path 
> > > > > have
> > > > > been removed we really depend on iomap support being present for DAX. 
> > > > >  Make
> > > > > this explicit by selecting FS_IOMAP if we compile in DAX support.
> > > > > 
> > > > > Signed-off-by: Ross Zwisler 
> > > > 
> > > > I've sent the same patch to Ted yesterday and he will probably queue it 
> > > > on
> > > > top of ext4 iomap patches. If it doesn't happen for some reason, feel 
> > > > free
> > > > to add:
> > > > 
> > > > Reviewed-by: Jan Kara 
> > > 
> > > Cool, looks like Ted has pulled in your patch.
> > > 
> > > I think we still eventually want this patch because it cleans up our 
> > > handling
> > > of FS_IOMAP.  With your patch we select it separately in both ext4 & ext2
> > > based on whether we include DAX, and we still have #ifdefs in fs/dax.c for
> > > FS_IOMAP.
> > 
> > Actually, based on Dave's request I've also sent Ted updated version which
> > did select FS_IOMAP in CONFIG_DAX section. However Ted didn't pull that
> > patch (yet?). Anyway, I don't care whose patch gets merged, I just wanted
> > to notify you of possible conflict.
> 
> Can you please CC me on these patches in the future?  I also don't care whose
> patches end up fixing this, but I want to make sure we end up in a world where
> the "select FS_IOMAP" just happens directly for FS_DAX in fs/Kconfig so that
> I can get rid of the unnecessary #ifdefs in fs/dax.c for CONFIG_FS_IOMAP.

Sure, will do.

Honza
-- 
Jan Kara 
SUSE Labs, CR

Re: [PATCH 1/6] dax: fix build breakage with ext4, dax and !iomap

[Jan Kara]

On Wed 30-11-16 12:04:31, Ross Zwisler wrote:
> On Tue, Nov 29, 2016 at 09:53:03AM +0100, Jan Kara wrote:
> > On Mon 28-11-16 12:15:04, Ross Zwisler wrote:
> > > On Thu, Nov 24, 2016 at 10:02:39AM +0100, Jan Kara wrote:
> > > > On Wed 23-11-16 11:44:17, Ross Zwisler wrote:
> > > > > With the current Kconfig setup it is possible to have the following:
> > > > > 
> > > > > CONFIG_EXT4_FS=y
> > > > > CONFIG_FS_DAX=y
> > > > > CONFIG_FS_IOMAP=n # this is in fs/Kconfig & isn't user accessible
> > > > > 
> > > > > With this config we get build failures in ext4_dax_fault() because the
> > > > > iomap functions in fs/dax.c are missing:
> > > > > 
> > > > > fs/built-in.o: In function `ext4_dax_fault':
> > > > > file.c:(.text+0x7f3ac): undefined reference to `dax_iomap_fault'
> > > > > file.c:(.text+0x7f404): undefined reference to `dax_iomap_fault'
> > > > > fs/built-in.o: In function `ext4_file_read_iter':
> > > > > file.c:(.text+0x7fc54): undefined reference to `dax_iomap_rw'
> > > > > fs/built-in.o: In function `ext4_file_write_iter':
> > > > > file.c:(.text+0x7fe9a): undefined reference to `dax_iomap_rw'
> > > > > file.c:(.text+0x7feed): undefined reference to `dax_iomap_rw'
> > > > > fs/built-in.o: In function `ext4_block_zero_page_range':
> > > > > inode.c:(.text+0x85c0d): undefined reference to `iomap_zero_range'
> > > > > 
> > > > > Now that the struct buffer_head based DAX fault paths and I/O path 
> > > > > have
> > > > > been removed we really depend on iomap support being present for DAX. 
> > > > >  Make
> > > > > this explicit by selecting FS_IOMAP if we compile in DAX support.
> > > > > 
> > > > > Signed-off-by: Ross Zwisler 
> > > > 
> > > > I've sent the same patch to Ted yesterday and he will probably queue it 
> > > > on
> > > > top of ext4 iomap patches. If it doesn't happen for some reason, feel 
> > > > free
> > > > to add:
> > > > 
> > > > Reviewed-by: Jan Kara 
> > > 
> > > Cool, looks like Ted has pulled in your patch.
> > > 
> > > I think we still eventually want this patch because it cleans up our 
> > > handling
> > > of FS_IOMAP.  With your patch we select it separately in both ext4 & ext2
> > > based on whether we include DAX, and we still have #ifdefs in fs/dax.c for
> > > FS_IOMAP.
> > 
> > Actually, based on Dave's request I've also sent Ted updated version which
> > did select FS_IOMAP in CONFIG_DAX section. However Ted didn't pull that
> > patch (yet?). Anyway, I don't care whose patch gets merged, I just wanted
> > to notify you of possible conflict.
> 
> Can you please CC me on these patches in the future?  I also don't care whose
> patches end up fixing this, but I want to make sure we end up in a world where
> the "select FS_IOMAP" just happens directly for FS_DAX in fs/Kconfig so that
> I can get rid of the unnecessary #ifdefs in fs/dax.c for CONFIG_FS_IOMAP.

Sure, will do.

Honza
-- 
Jan Kara 
SUSE Labs, CR

RE: [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune interpreter lock around AcpiEvInitializeRegion()"

[Zheng, Lv]

Hi, Rafael

> From: rjwyso...@gmail.com [mailto:rjwyso...@gmail.com] On Behalf Of Rafael J. 
> Wysocki
> Subject: Re: [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune 
> interpreter lock around
> AcpiEvInitializeRegion()"
> 
> On Wed, Nov 30, 2016 at 8:20 AM, Lv Zheng  wrote:
> > ACPICA commit bc481e758e54f7644fd0b657119ca7763d8b6a9c
> >
> > This is a back port result of the following commit:
> >   Commit: 8633db6b027952449e155a316f4ae3a530bbe18f
> >   Subject: ACPICA: Dispatcher: Fix interpreter locking around 
> > acpi_ev_initialize_region()
> >
> > Link: https://github.com/acpica/acpica/commit/bc481e75
> > Signed-off-by: Lv Zheng 
> > Signed-off-by: Bob Moore 
> > ---
> >  drivers/acpi/acpica/dsinit.c |4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/dsinit.c b/drivers/acpi/acpica/dsinit.c
> > index 54d48b9..5de3f10 100644
> > --- a/drivers/acpi/acpica/dsinit.c
> > +++ b/drivers/acpi/acpica/dsinit.c
> > @@ -221,8 +221,8 @@
> >  */
> > status =
> > acpi_ns_walk_namespace(ACPI_TYPE_ANY, start_node, 
> > ACPI_UINT32_MAX,
> > -  0, acpi_ds_init_one_object, NULL, ,
> > -  NULL);
> > +  ACPI_NS_WALK_NO_UNLOCK,
> > +  acpi_ds_init_one_object, NULL, , 
> > NULL);
> > if (ACPI_FAILURE(status)) {
> > ACPI_EXCEPTION((AE_INFO, status, "During WalkNamespace"));
> > }
> > --
> 
> This isn't necessary IMO, the current code linux-next code looks like
> the change has been made in there already AFAICS (please double check,
> though).

The fix was in Linux, however, when it is back ported to ACPICA, Bob asked me 
to do this change.
Using ACPI_NS_WALK_NO_UNLOCK instead of meaningless 0.
So during this release cycle, this change is detected out as the only 
difference of the back ported commit.

> 
> I'm skipping this patch.

If this is skipped, it leaves us 14 lines divergences.
Hope we can have this kind of divergences eliminated.

Thanks and best regards
Lv

RE: [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune interpreter lock around AcpiEvInitializeRegion()"

[Zheng, Lv]

Hi, Rafael

> From: rjwyso...@gmail.com [mailto:rjwyso...@gmail.com] On Behalf Of Rafael J. 
> Wysocki
> Subject: Re: [PATCH 02/11] ACPICA: Back port of "ACPICA: Dispatcher: Tune 
> interpreter lock around
> AcpiEvInitializeRegion()"
> 
> On Wed, Nov 30, 2016 at 8:20 AM, Lv Zheng  wrote:
> > ACPICA commit bc481e758e54f7644fd0b657119ca7763d8b6a9c
> >
> > This is a back port result of the following commit:
> >   Commit: 8633db6b027952449e155a316f4ae3a530bbe18f
> >   Subject: ACPICA: Dispatcher: Fix interpreter locking around 
> > acpi_ev_initialize_region()
> >
> > Link: https://github.com/acpica/acpica/commit/bc481e75
> > Signed-off-by: Lv Zheng 
> > Signed-off-by: Bob Moore 
> > ---
> >  drivers/acpi/acpica/dsinit.c |4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/dsinit.c b/drivers/acpi/acpica/dsinit.c
> > index 54d48b9..5de3f10 100644
> > --- a/drivers/acpi/acpica/dsinit.c
> > +++ b/drivers/acpi/acpica/dsinit.c
> > @@ -221,8 +221,8 @@
> >  */
> > status =
> > acpi_ns_walk_namespace(ACPI_TYPE_ANY, start_node, 
> > ACPI_UINT32_MAX,
> > -  0, acpi_ds_init_one_object, NULL, ,
> > -  NULL);
> > +  ACPI_NS_WALK_NO_UNLOCK,
> > +  acpi_ds_init_one_object, NULL, , 
> > NULL);
> > if (ACPI_FAILURE(status)) {
> > ACPI_EXCEPTION((AE_INFO, status, "During WalkNamespace"));
> > }
> > --
> 
> This isn't necessary IMO, the current code linux-next code looks like
> the change has been made in there already AFAICS (please double check,
> though).

The fix was in Linux, however, when it is back ported to ACPICA, Bob asked me 
to do this change.
Using ACPI_NS_WALK_NO_UNLOCK instead of meaningless 0.
So during this release cycle, this change is detected out as the only 
difference of the back ported commit.

> 
> I'm skipping this patch.

If this is skipped, it leaves us 14 lines divergences.
Hope we can have this kind of divergences eliminated.

Thanks and best regards
Lv

[RFC PATCH 1/3]Binding: Add a new property string in ITS node to control the two-level route function

[Majun]

From: MaJun 

Add the two-level-route property in ITS node.
When this property string defined, two-level route(indirect) function
will be enabled in ITS driver, otherwise disable it.

Signed-off-by: MaJun 
---
 Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt 
b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
index 4c29cda..e9f4a9c 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
@@ -74,6 +74,8 @@ These nodes must have the following properties:
   which will generate the MSI.
 - reg: Specifies the base physical address and size of the ITS
   registers.
+- two-level-route: This is an optional property which means enable the two 
level
+  route property when look up route table.
 
 The main GIC node must contain the appropriate #address-cells,
 #size-cells and ranges properties for the reg property of all ITS
@@ -97,6 +99,7 @@ Examples:
 
gic-its@2c20 {
compatible = "arm,gic-v3-its";
+   two-level-route;
msi-controller;
#msi-cells = <1>;
reg = <0x0 0x2c20 0 0x20>;
-- 
1.7.12.4

[RFC PATCH 3/3]irqchip/gicv3-its: Add a new flag to control indirect route in ACPI mode.

[Majun]

From: MaJun 

Add a new flag to control indirect route function for ACPI mode.
To carry the user defined flags information, we used the reserved byte
in ITS MADT table

Signed-off-by: MaJun 
---
 drivers/irqchip/irq-gic-v3-its.c | 5 -
 include/acpi/actbl1.h| 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index ee54133..4420283 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1848,6 +1848,7 @@ static int __init gic_acpi_parse_madt_its(struct 
acpi_subtable_header *header,
struct fwnode_handle *dom_handle;
struct resource res;
int err;
+   u8 flags = 0;
 
its_entry = (struct acpi_madt_generic_translator *)header;
memset(, 0, sizeof(res));
@@ -1855,6 +1856,8 @@ static int __init gic_acpi_parse_madt_its(struct 
acpi_subtable_header *header,
res.end = its_entry->base_address + ACPI_GICV3_ITS_MEM_SIZE - 1;
res.flags = IORESOURCE_MEM;
 
+   flags = its_entry->flags;
+
dom_handle = irq_domain_alloc_fwnode((void *)its_entry->base_address);
if (!dom_handle) {
pr_err("ITS@%pa: Unable to allocate GICv3 ITS domain token\n",
@@ -1869,7 +1872,7 @@ static int __init gic_acpi_parse_madt_its(struct 
acpi_subtable_header *header,
goto dom_err;
}
 
-   err = its_probe_one(, dom_handle, NUMA_NO_NODE, 0);
+   err = its_probe_one(, dom_handle, NUMA_NO_NODE, flags);
if (!err)
return 0;
 
diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 796d6ba..42a08ae 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -930,7 +930,8 @@ struct acpi_madt_generic_translator {
u16 reserved;   /* reserved - must be zero */
u32 translation_id;
u64 base_address;
-   u32 reserved2;
+   u8 flags;
+   u8 reserved2[3];
 };
 
 /*
-- 
1.7.12.4

[RFC PATCH 1/3]Binding: Add a new property string in ITS node to control the two-level route function

[Majun]

From: MaJun 

Add the two-level-route property in ITS node.
When this property string defined, two-level route(indirect) function
will be enabled in ITS driver, otherwise disable it.

Signed-off-by: MaJun 
---
 Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt 
b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
index 4c29cda..e9f4a9c 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
@@ -74,6 +74,8 @@ These nodes must have the following properties:
   which will generate the MSI.
 - reg: Specifies the base physical address and size of the ITS
   registers.
+- two-level-route: This is an optional property which means enable the two 
level
+  route property when look up route table.
 
 The main GIC node must contain the appropriate #address-cells,
 #size-cells and ranges properties for the reg property of all ITS
@@ -97,6 +99,7 @@ Examples:
 
gic-its@2c20 {
compatible = "arm,gic-v3-its";
+   two-level-route;
msi-controller;
#msi-cells = <1>;
reg = <0x0 0x2c20 0 0x20>;
-- 
1.7.12.4

[RFC PATCH 3/3]irqchip/gicv3-its: Add a new flag to control indirect route in ACPI mode.

[Majun]

From: MaJun 

Add a new flag to control indirect route function for ACPI mode.
To carry the user defined flags information, we used the reserved byte
in ITS MADT table

Signed-off-by: MaJun 
---
 drivers/irqchip/irq-gic-v3-its.c | 5 -
 include/acpi/actbl1.h| 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index ee54133..4420283 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1848,6 +1848,7 @@ static int __init gic_acpi_parse_madt_its(struct 
acpi_subtable_header *header,
struct fwnode_handle *dom_handle;
struct resource res;
int err;
+   u8 flags = 0;
 
its_entry = (struct acpi_madt_generic_translator *)header;
memset(, 0, sizeof(res));
@@ -1855,6 +1856,8 @@ static int __init gic_acpi_parse_madt_its(struct 
acpi_subtable_header *header,
res.end = its_entry->base_address + ACPI_GICV3_ITS_MEM_SIZE - 1;
res.flags = IORESOURCE_MEM;
 
+   flags = its_entry->flags;
+
dom_handle = irq_domain_alloc_fwnode((void *)its_entry->base_address);
if (!dom_handle) {
pr_err("ITS@%pa: Unable to allocate GICv3 ITS domain token\n",
@@ -1869,7 +1872,7 @@ static int __init gic_acpi_parse_madt_its(struct 
acpi_subtable_header *header,
goto dom_err;
}
 
-   err = its_probe_one(, dom_handle, NUMA_NO_NODE, 0);
+   err = its_probe_one(, dom_handle, NUMA_NO_NODE, flags);
if (!err)
return 0;
 
diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 796d6ba..42a08ae 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -930,7 +930,8 @@ struct acpi_madt_generic_translator {
u16 reserved;   /* reserved - must be zero */
u32 translation_id;
u64 base_address;
-   u32 reserved2;
+   u8 flags;
+   u8 reserved2[3];
 };
 
 /*
-- 
1.7.12.4

[RFC PATCH 0/3] Add a new flag for ITS device to control indirect route

[Majun]

From: MaJun 

For current ITS driver, two level table (indirect route) is enabled when the 
memory used
for LPI route table over the limit(64KB * 2) size. But this function impact the 
performance of LPI interrupt actually because need more time to look up the 
table.

Although this function can save the memory needed, we'd better let the user
to decide enable or disable this function.

MaJun (3):
  Binding: Add a new property string in ITS node to control the two-level route 
function
  irqchip/gicv3-its:irqchip/gicv3-its: add a new flag to control indirect route 
in DT mode
  irqchip/gicv3-its:irqchip/gicv3-its: Add a new flag to control indirect route 
function in ACPI mode.

 .../bindings/interrupt-controller/arm,gic-v3.txt  |  3 +++
 drivers/irqchip/irq-gic-v3-its.c  | 19 ++-
 include/acpi/actbl1.h |  3 ++-
 3 files changed, 19 insertions(+), 6 deletions(-)

-- 
1.7.12.4

[RFC PATCH 0/3] Add a new flag for ITS device to control indirect route

[Majun]

From: MaJun 

For current ITS driver, two level table (indirect route) is enabled when the 
memory used
for LPI route table over the limit(64KB * 2) size. But this function impact the 
performance of LPI interrupt actually because need more time to look up the 
table.

Although this function can save the memory needed, we'd better let the user
to decide enable or disable this function.

MaJun (3):
  Binding: Add a new property string in ITS node to control the two-level route 
function
  irqchip/gicv3-its:irqchip/gicv3-its: add a new flag to control indirect route 
in DT mode
  irqchip/gicv3-its:irqchip/gicv3-its: Add a new flag to control indirect route 
function in ACPI mode.

 .../bindings/interrupt-controller/arm,gic-v3.txt  |  3 +++
 drivers/irqchip/irq-gic-v3-its.c  | 19 ++-
 include/acpi/actbl1.h |  3 ++-
 3 files changed, 19 insertions(+), 6 deletions(-)

-- 
1.7.12.4

[RFC PATCH 2/3] irqchip/gicv3-its: add a new flag to control indirect route in DT mode

[Majun]

From: MaJun 

Add a new flag for ITS node in DT mode so we can disable/enable the
indirect route function.

Signed-off-by: MaJun 
---
 drivers/irqchip/irq-gic-v3-its.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index d278425..ee54133 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -46,6 +46,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING  (1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375  (1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144  (1ULL << 2)
+#define ITS_FLAGS_INDIRECT_ROUTE   (1ULL << 3)
 
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING(1 << 0)
 
@@ -967,7 +968,7 @@ static bool its_parse_baser_device(struct its_node *its, 
struct its_baser *baser
bool indirect = false;
 
/* No need to enable Indirection if memory requirement < (psz*2)bytes */
-   if ((esz << ids) > (psz * 2)) {
+   if ((its->flags & ITS_FLAGS_INDIRECT_ROUTE) && ((esz << ids) > (psz * 
2))) {
/*
 * Find out whether hw supports a single or two-level table by
 * table by reading bit at offset '62' after writing '1' to it.
@@ -1673,8 +1674,8 @@ static int its_init_domain(struct fwnode_handle *handle, 
struct its_node *its)
return 0;
 }
 
-static int __init its_probe_one(struct resource *res,
-   struct fwnode_handle *handle, int numa_node)
+static int __init its_probe_one(struct resource *res, struct fwnode_handle 
*handle,
+   int numa_node, u8 flags)
 {
struct its_node *its;
void __iomem *its_base;
@@ -1716,6 +1717,7 @@ static int __init its_probe_one(struct resource *res,
its->phys_base = res->start;
its->ite_size = ((readl_relaxed(its_base + GITS_TYPER) >> 4) & 0xf) + 1;
its->numa_node = numa_node;
+   its->flags |= flags;
 
its->cmd_base = kzalloc(ITS_CMD_QUEUE_SZ, GFP_KERNEL);
if (!its->cmd_base) {
@@ -1812,6 +1814,7 @@ static int __init its_of_probe(struct device_node *node)
 {
struct device_node *np;
struct resource res;
+   u8 flags = 0;
 
for (np = of_find_matching_node(node, its_device_id); np;
 np = of_find_matching_node(np, its_device_id)) {
@@ -1826,7 +1829,10 @@ static int __init its_of_probe(struct device_node *node)
continue;
}
 
-   its_probe_one(, >fwnode, of_node_to_nid(np));
+   if (of_property_read_bool(np, "two-level-route"))
+   flags |= ITS_FLAGS_INDIRECT_ROUTE;
+
+   its_probe_one(, >fwnode, of_node_to_nid(np), flags);
}
return 0;
 }
@@ -1863,7 +1869,7 @@ static int __init gic_acpi_parse_madt_its(struct 
acpi_subtable_header *header,
goto dom_err;
}
 
-   err = its_probe_one(, dom_handle, NUMA_NO_NODE);
+   err = its_probe_one(, dom_handle, NUMA_NO_NODE, 0);
if (!err)
return 0;
 
-- 
1.7.12.4

[RFC PATCH 2/3] irqchip/gicv3-its: add a new flag to control indirect route in DT mode

[Majun]

From: MaJun 

Add a new flag for ITS node in DT mode so we can disable/enable the
indirect route function.

Signed-off-by: MaJun 
---
 drivers/irqchip/irq-gic-v3-its.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index d278425..ee54133 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -46,6 +46,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING  (1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375  (1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144  (1ULL << 2)
+#define ITS_FLAGS_INDIRECT_ROUTE   (1ULL << 3)
 
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING(1 << 0)
 
@@ -967,7 +968,7 @@ static bool its_parse_baser_device(struct its_node *its, 
struct its_baser *baser
bool indirect = false;
 
/* No need to enable Indirection if memory requirement < (psz*2)bytes */
-   if ((esz << ids) > (psz * 2)) {
+   if ((its->flags & ITS_FLAGS_INDIRECT_ROUTE) && ((esz << ids) > (psz * 
2))) {
/*
 * Find out whether hw supports a single or two-level table by
 * table by reading bit at offset '62' after writing '1' to it.
@@ -1673,8 +1674,8 @@ static int its_init_domain(struct fwnode_handle *handle, 
struct its_node *its)
return 0;
 }
 
-static int __init its_probe_one(struct resource *res,
-   struct fwnode_handle *handle, int numa_node)
+static int __init its_probe_one(struct resource *res, struct fwnode_handle 
*handle,
+   int numa_node, u8 flags)
 {
struct its_node *its;
void __iomem *its_base;
@@ -1716,6 +1717,7 @@ static int __init its_probe_one(struct resource *res,
its->phys_base = res->start;
its->ite_size = ((readl_relaxed(its_base + GITS_TYPER) >> 4) & 0xf) + 1;
its->numa_node = numa_node;
+   its->flags |= flags;
 
its->cmd_base = kzalloc(ITS_CMD_QUEUE_SZ, GFP_KERNEL);
if (!its->cmd_base) {
@@ -1812,6 +1814,7 @@ static int __init its_of_probe(struct device_node *node)
 {
struct device_node *np;
struct resource res;
+   u8 flags = 0;
 
for (np = of_find_matching_node(node, its_device_id); np;
 np = of_find_matching_node(np, its_device_id)) {
@@ -1826,7 +1829,10 @@ static int __init its_of_probe(struct device_node *node)
continue;
}
 
-   its_probe_one(, >fwnode, of_node_to_nid(np));
+   if (of_property_read_bool(np, "two-level-route"))
+   flags |= ITS_FLAGS_INDIRECT_ROUTE;
+
+   its_probe_one(, >fwnode, of_node_to_nid(np), flags);
}
return 0;
 }
@@ -1863,7 +1869,7 @@ static int __init gic_acpi_parse_madt_its(struct 
acpi_subtable_header *header,
goto dom_err;
}
 
-   err = its_probe_one(, dom_handle, NUMA_NO_NODE);
+   err = its_probe_one(, dom_handle, NUMA_NO_NODE, 0);
if (!err)
return 0;
 
-- 
1.7.12.4

Re: [PATCH] ACPI: fix the process flow for 0 which return from acpi_register_gsi

[majun (Euler7)]

sorry, ignore this one..


在 2016/12/1 15:41, Majun 写道:
> From: MaJun 
> 
> The return value 0 from acpi_register_gsi() means irq mapping failed.
> So, we should process this case in else branch.
> 
> Signed-off-by: MaJun 
> ---
>  drivers/acpi/resource.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 56241eb..9918326 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -416,7 +416,7 @@ static void acpi_dev_get_irqresource(struct resource 
> *res, u32 gsi,
>  
>   res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
>   irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
> - if (irq >= 0) {
> + if (irq > 0) {
>   res->start = irq;
>   res->end = irq;
>   } else {
> 

Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command

[Lu Baolu]

Hi,

On 12/01/2016 03:35 PM, Baolin Wang wrote:
> On 1 December 2016 at 14:35, Lu Baolu  wrote:
>> Hi,
>>
>> On 12/01/2016 02:04 PM, Baolin Wang wrote:
>>> Hi Baolu,
>>>
>>> On 1 December 2016 at 13:45, Lu Baolu  wrote:
 Hi,

 On 11/30/2016 05:02 PM, Baolin Wang wrote:
> If the hardware never responds to the stop endpoint command, the
> URBs will never be completed, and we might hang the USB subsystem.
> The original watchdog timer is used to watch if one stop endpoint
> command is timeout, if timeout, then the watchdog timer will set
> XHCI_STATE_DYING, try to halt the xHCI host, and give back all
> pending URBs.
>
> But now we already have one command timer to control command timeout,
> thus we can also use the command timer to watch the stop endpoint
> command, instead of one duplicate watchdog timer which need to be
> removed.
>
> Meanwhile we don't need the 'stop_cmds_pending' flag to identy if
> this is the last stop endpoint command of one endpoint. Since we
> can make sure we only set one stop endpoint command for one endpoint
> by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove
> this flag.
 I am afraid you can't do this. "stop_cmds_pending" was added
 to fix the problem described in the comments that you want to
 remove. But I didn't find any fix of this problem in your patch.
>>> Now we can not pending another stop endpoint command for the same one
>>> endpoint, since will check 'EP_HALT_PENDING' flag in
>>> xhci_urb_dequeue() function to avoid this. But after some
>>> investigation, I think I missed the stop endpoint command in
>>> xhci_stop_device() which did not check the 'EP_HALT_PENDING' flag,
>>> maybe need to add 'EP_HALT_PENDING' flag checking in
>>> xhci_stop_device() function. DId I miss something else? Thanks.
>> Consider below three threads running on different CPUs at the same time.
>>
>> Thread A: xhci_handle_cmd_stop_ep()  --- in interrupt handler
>> Thread B: xhci_stop_endpoint_command_timeout() --- timer expired
>> Thread C: xhci_urb_dequeue --- called by usb core
>>
>> They are serialized by xhci->lock. Let's consider below sequence:
>>
>> Thread A:
>> - delete xhci->cmd_timer), but will fail due to Thread B.
>> - clear EP_HALT_PENDING bit
>>
>> Thread B:
>> - halt the host controller
>>
>> Thread C:
>> - set EP_HALT_PENDING bit
>> - enqueue another stop endpoint command
>> - add the timer back
> Ah, thanks for you comments.
> If thread B halted the host, then the thread C xhci_urb_dequeue() will
> check host state failed, then just return and can not set another stop
> endpoint command.

Not so simple. Thread B will release xhci->lock before xhci_halt().

> But from your example, I think your meaning is we
> should not halt the host by thread B, since we have handled the stop
> endpoint command event.
>
> So I think we need to check the xhci command of this stop endpoint
> command in xhci_stop_endpoint_command_timeout(), if the xhci command
> of this stop endpoint command is not in the command list (which means
> the stop endpoint command event has been handled), then just return
> and do not halt the controller. Right?
>

I'd like suggest you to put this change into a separated patch.
It's actually a different topic from the main purpose of this patch.

Best regards,
Lu Baolu

Re: [PATCH] ACPI: fix the process flow for 0 which return from acpi_register_gsi

[majun (Euler7)]

sorry, ignore this one..


在 2016/12/1 15:41, Majun 写道:
> From: MaJun 
> 
> The return value 0 from acpi_register_gsi() means irq mapping failed.
> So, we should process this case in else branch.
> 
> Signed-off-by: MaJun 
> ---
>  drivers/acpi/resource.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 56241eb..9918326 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -416,7 +416,7 @@ static void acpi_dev_get_irqresource(struct resource 
> *res, u32 gsi,
>  
>   res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
>   irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
> - if (irq >= 0) {
> + if (irq > 0) {
>   res->start = irq;
>   res->end = irq;
>   } else {
> 

Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command

[Lu Baolu]

Hi,

On 12/01/2016 03:35 PM, Baolin Wang wrote:
> On 1 December 2016 at 14:35, Lu Baolu  wrote:
>> Hi,
>>
>> On 12/01/2016 02:04 PM, Baolin Wang wrote:
>>> Hi Baolu,
>>>
>>> On 1 December 2016 at 13:45, Lu Baolu  wrote:
 Hi,

 On 11/30/2016 05:02 PM, Baolin Wang wrote:
> If the hardware never responds to the stop endpoint command, the
> URBs will never be completed, and we might hang the USB subsystem.
> The original watchdog timer is used to watch if one stop endpoint
> command is timeout, if timeout, then the watchdog timer will set
> XHCI_STATE_DYING, try to halt the xHCI host, and give back all
> pending URBs.
>
> But now we already have one command timer to control command timeout,
> thus we can also use the command timer to watch the stop endpoint
> command, instead of one duplicate watchdog timer which need to be
> removed.
>
> Meanwhile we don't need the 'stop_cmds_pending' flag to identy if
> this is the last stop endpoint command of one endpoint. Since we
> can make sure we only set one stop endpoint command for one endpoint
> by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove
> this flag.
 I am afraid you can't do this. "stop_cmds_pending" was added
 to fix the problem described in the comments that you want to
 remove. But I didn't find any fix of this problem in your patch.
>>> Now we can not pending another stop endpoint command for the same one
>>> endpoint, since will check 'EP_HALT_PENDING' flag in
>>> xhci_urb_dequeue() function to avoid this. But after some
>>> investigation, I think I missed the stop endpoint command in
>>> xhci_stop_device() which did not check the 'EP_HALT_PENDING' flag,
>>> maybe need to add 'EP_HALT_PENDING' flag checking in
>>> xhci_stop_device() function. DId I miss something else? Thanks.
>> Consider below three threads running on different CPUs at the same time.
>>
>> Thread A: xhci_handle_cmd_stop_ep()  --- in interrupt handler
>> Thread B: xhci_stop_endpoint_command_timeout() --- timer expired
>> Thread C: xhci_urb_dequeue --- called by usb core
>>
>> They are serialized by xhci->lock. Let's consider below sequence:
>>
>> Thread A:
>> - delete xhci->cmd_timer), but will fail due to Thread B.
>> - clear EP_HALT_PENDING bit
>>
>> Thread B:
>> - halt the host controller
>>
>> Thread C:
>> - set EP_HALT_PENDING bit
>> - enqueue another stop endpoint command
>> - add the timer back
> Ah, thanks for you comments.
> If thread B halted the host, then the thread C xhci_urb_dequeue() will
> check host state failed, then just return and can not set another stop
> endpoint command.

Not so simple. Thread B will release xhci->lock before xhci_halt().

> But from your example, I think your meaning is we
> should not halt the host by thread B, since we have handled the stop
> endpoint command event.
>
> So I think we need to check the xhci command of this stop endpoint
> command in xhci_stop_endpoint_command_timeout(), if the xhci command
> of this stop endpoint command is not in the command list (which means
> the stop endpoint command event has been handled), then just return
> and do not halt the controller. Right?
>

I'd like suggest you to put this change into a separated patch.
It's actually a different topic from the main purpose of this patch.

Best regards,
Lu Baolu

Re: drm/radeon spamming alloc_contig_range: [xxx, yyy) PFNs busy busy

[Vlastimil Babka]

On 12/01/2016 08:21 AM, Michal Hocko wrote:

Forgot to CC Joonsoo. The email thread starts more or less here
http://lkml.kernel.org/r/20161130092239.gd18...@dhcp22.suse.cz

On Thu 01-12-16 08:15:07, Michal Hocko wrote:

On Wed 30-11-16 20:19:03, Robin H. Johnson wrote:
[...]
> alloc_contig_range: [83f2a3, 83f2a4) PFNs busy

Huh, do I get it right that the request was for a _single_ page? Why do
we need CMA for that?


Ugh, good point. I assumed that was just the PFNs that it failed to migrate 
away, but it seems that's indeed the whole requested range. Yeah sounds some 
part of the dma-cma chain could be smarter and attempt CMA only for e.g. costly 
orders.

Re: drm/radeon spamming alloc_contig_range: [xxx, yyy) PFNs busy busy

[Vlastimil Babka]

On 12/01/2016 08:21 AM, Michal Hocko wrote:

Forgot to CC Joonsoo. The email thread starts more or less here
http://lkml.kernel.org/r/20161130092239.gd18...@dhcp22.suse.cz

On Thu 01-12-16 08:15:07, Michal Hocko wrote:

On Wed 30-11-16 20:19:03, Robin H. Johnson wrote:
[...]
> alloc_contig_range: [83f2a3, 83f2a4) PFNs busy

Huh, do I get it right that the request was for a _single_ page? Why do
we need CMA for that?


Ugh, good point. I assumed that was just the PFNs that it failed to migrate 
away, but it seems that's indeed the whole requested range. Yeah sounds some 
part of the dma-cma chain could be smarter and attempt CMA only for e.g. costly 
orders.

[PATCH] ACPI: fix the process flow for 0 which return from acpi_register_gsi

[Majun]

From: MaJun 

The return value 0 from acpi_register_gsi() means irq mapping failed.
So, we should process this case in else branch.

Signed-off-by: MaJun 
---
 drivers/acpi/resource.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 56241eb..9918326 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -416,7 +416,7 @@ static void acpi_dev_get_irqresource(struct resource *res, 
u32 gsi,
 
res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
-   if (irq >= 0) {
+   if (irq > 0) {
res->start = irq;
res->end = irq;
} else {
-- 
1.7.12.4

[PATCH] ACPI: fix the process flow for 0 which return from acpi_register_gsi

[Majun]

From: MaJun 

The return value 0 from acpi_register_gsi() means irq mapping failed.
So, we should process this case in else branch.

Signed-off-by: MaJun 
---
 drivers/acpi/resource.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 56241eb..9918326 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -416,7 +416,7 @@ static void acpi_dev_get_irqresource(struct resource *res, 
u32 gsi,
 
res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
-   if (irq >= 0) {
+   if (irq > 0) {
res->start = irq;
res->end = irq;
} else {
-- 
1.7.12.4

Re: [alsa-devel] [PATCH 1/3 v1] ALSA: usb-audio: more tolerant packetsize

[Clemens Ladisch]

Jiada Wang wrote:
> since commit 57e6dae1087bbaa6b33d3dd8a8e90b63888939a3 the expected packetsize 
> is always limited to
> nominal + 25%. It was discovered, that some devices

Which devices?

> have a much higher jitter in used packetsizes than 25%

How high?  (Please note that the USB specification restricts the jitter
to at most one frame in consecutive packets.)

> which would result in BABBLE condition and dropping of packets.
> A better solution is so assume the jitter to be the nominal packetsize

This solution is better for this one particular device, but how does it
affect normal devices, or the Scarlett 2i4 on EHCI affected?


Regards,
Clemens

Re: [alsa-devel] [PATCH 1/3 v1] ALSA: usb-audio: more tolerant packetsize

[Clemens Ladisch]

Jiada Wang wrote:
> since commit 57e6dae1087bbaa6b33d3dd8a8e90b63888939a3 the expected packetsize 
> is always limited to
> nominal + 25%. It was discovered, that some devices

Which devices?

> have a much higher jitter in used packetsizes than 25%

How high?  (Please note that the USB specification restricts the jitter
to at most one frame in consecutive packets.)

> which would result in BABBLE condition and dropping of packets.
> A better solution is so assume the jitter to be the nominal packetsize

This solution is better for this one particular device, but how does it
affect normal devices, or the Scarlett 2i4 on EHCI affected?


Regards,
Clemens

Re: [PATCH v2] drivers/base: use READ_ONCE instead of deprecated ACCESS_ONCE

[Greg KH]

On Wed, Nov 30, 2016 at 09:02:49AM -0800, Davidlohr Bueso wrote:
> On Wed, 30 Nov 2016, Greg KH wrote:
> > What changed from v1?  Please always include it below the --- line to
> > keep maintainer's semi-sane.
> 
> If anything changed I would have -- this is only the From != SoB thing
> you were complaining about.

How am I supposed to remember that?  And that is a change, so please
list it.  Otherwise I just will drop it.

Remember, maintainers get hundreds of patches, and have no short term
memory at all, you have to remind them of everything.  That's your job.

> There's nothing to try again, this is a trivial.

a trivial what?

Anyway, you are going to have to fix it up if you want it accepted, it's
long gone from my patch queue...

thanks,

greg k-h

Re: [PATCH v2] drivers/base: use READ_ONCE instead of deprecated ACCESS_ONCE

[Greg KH]

On Wed, Nov 30, 2016 at 09:02:49AM -0800, Davidlohr Bueso wrote:
> On Wed, 30 Nov 2016, Greg KH wrote:
> > What changed from v1?  Please always include it below the --- line to
> > keep maintainer's semi-sane.
> 
> If anything changed I would have -- this is only the From != SoB thing
> you were complaining about.

How am I supposed to remember that?  And that is a change, so please
list it.  Otherwise I just will drop it.

Remember, maintainers get hundreds of patches, and have no short term
memory at all, you have to remind them of everything.  That's your job.

> There's nothing to try again, this is a trivial.

a trivial what?

Anyway, you are going to have to fix it up if you want it accepted, it's
long gone from my patch queue...

thanks,

greg k-h

Re: drm/radeon spamming alloc_contig_range: [xxx, yyy) PFNs busy busy

[Vlastimil Babka]

On 12/01/2016 07:21 AM, Robin H. Johnson wrote:

On Wed, Nov 30, 2016 at 10:24:59PM +0100, Vlastimil Babka wrote:

[add more CC's]

On 11/30/2016 09:19 PM, Robin H. Johnson wrote:
> Somewhere in the Radeon/DRM codebase, CMA page allocation has either
> regressed in the timeline of 4.5->4.9, and/or the drm/radeon code is
> doing something different with pages.

Could be that it didn't use dma_generic_alloc_coherent() before, or you didn't
have the generic CMA pool configured.

v4.9-rc7-23-gded6e842cf49:
[0.00] cma: Reserved 16 MiB at 0x00083e40
[0.00] Memory: 32883108K/33519432K available (6752K kernel code, 1244K
rwdata, 4716K rodata, 1772K init, 2720K bss, 619940K reserved, 16384K
cma-reserved)


What's the output of "grep CMA" on your
.config?


# grep CMA .config |grep -v -e SECMARK= -e CONFIG_BCMA -e CONFIG_USB_HCD_BCMA 
-e INPUT_CMA3000 -e CRYPTO_CMAC
CONFIG_CMA=y
# CONFIG_CMA_DEBUG is not set
# CONFIG_CMA_DEBUGFS is not set
CONFIG_CMA_AREAS=7
CONFIG_DMA_CMA=y
CONFIG_CMA_SIZE_MBYTES=16
CONFIG_CMA_SIZE_SEL_MBYTES=y
# CONFIG_CMA_SIZE_SEL_PERCENTAGE is not set
# CONFIG_CMA_SIZE_SEL_MIN is not set
# CONFIG_CMA_SIZE_SEL_MAX is not set
CONFIG_CMA_ALIGNMENT=8


Or any kernel boot options with cma in name?

None.



By default config this should not be used on x86.

What do you mean by that statement?


I mean that the 16 mbytes for generic CMA area is not a default on x86:

config CMA_SIZE_MBYTES
int "Size in Mega Bytes"
depends on !CMA_SIZE_SEL_PERCENTAGE
default 0 if X86
default 16

Which explains why it's rare to see these reports in the context such as yours.
I'd recommend just disabling it, as the primary use case for CMA are devices on 
mobile phones that don't have any other fallback (unlike the dma alloc).



It should be disallowed to enable CONFIG_CMA? Radeon and CMA should be
mutually exclusive?


I don't think this is a specific problem of radeon. But looks like it's a heavy 
user of the dma alloc. There might be others.



> Given that I haven't seen ANY other reports of this, I'm inclined to
> believe the problem is drm/radeon specific (if I don't start X, I can't
> reproduce the problem).

It's rather CMA specific, the allocation attemps just can't be 100% reliable due
to how CMA works. The question is if it should be spewing in the log in the
context of dma-cma, which has a fallback allocation option. It even uses
__GFP_NOWARN, perhaps the CMA path should respect that?

Yes, I'd say if there's a fallback without much penalty, nowarn makes
sense. If the fallback just tries multiple addresses until success, then
the warning should only be issued when too many attempts have been made.


On the other hand, if the warnings are correlated with high kernel CPU usage, 
it's arguably better to be warned.




> The rate of the problem starts slow, and also is relatively low on an idle
> system (my screens blank at night, no xscreensaver running), but it still 
ramps
> up over time (to the point of generating 2.5GB/hour of "(timestamp)
> alloc_contig_range: [83e4d9, 83e4da) PFNs busy"), with various addresses (~100
> unique ranges for a day).
>
> My X workload is ~50 chrome tabs and ~20 terminals (over 3x 24" monitors w/ 9
> virtual desktops per monitor).
So IIUC, except the messages, everything actually works fine?

There's high kernel CPU usage that seems to roughly correlate with the
messages, but I can't yet tell if that's due to the syslog itself, or
repeated alloc_contig_range requests.


You could try running perf top.

Re: drm/radeon spamming alloc_contig_range: [xxx, yyy) PFNs busy busy

[Vlastimil Babka]

On 12/01/2016 07:21 AM, Robin H. Johnson wrote:

On Wed, Nov 30, 2016 at 10:24:59PM +0100, Vlastimil Babka wrote:

[add more CC's]

On 11/30/2016 09:19 PM, Robin H. Johnson wrote:
> Somewhere in the Radeon/DRM codebase, CMA page allocation has either
> regressed in the timeline of 4.5->4.9, and/or the drm/radeon code is
> doing something different with pages.

Could be that it didn't use dma_generic_alloc_coherent() before, or you didn't
have the generic CMA pool configured.

v4.9-rc7-23-gded6e842cf49:
[0.00] cma: Reserved 16 MiB at 0x00083e40
[0.00] Memory: 32883108K/33519432K available (6752K kernel code, 1244K
rwdata, 4716K rodata, 1772K init, 2720K bss, 619940K reserved, 16384K
cma-reserved)


What's the output of "grep CMA" on your
.config?


# grep CMA .config |grep -v -e SECMARK= -e CONFIG_BCMA -e CONFIG_USB_HCD_BCMA 
-e INPUT_CMA3000 -e CRYPTO_CMAC
CONFIG_CMA=y
# CONFIG_CMA_DEBUG is not set
# CONFIG_CMA_DEBUGFS is not set
CONFIG_CMA_AREAS=7
CONFIG_DMA_CMA=y
CONFIG_CMA_SIZE_MBYTES=16
CONFIG_CMA_SIZE_SEL_MBYTES=y
# CONFIG_CMA_SIZE_SEL_PERCENTAGE is not set
# CONFIG_CMA_SIZE_SEL_MIN is not set
# CONFIG_CMA_SIZE_SEL_MAX is not set
CONFIG_CMA_ALIGNMENT=8


Or any kernel boot options with cma in name?

None.



By default config this should not be used on x86.

What do you mean by that statement?


I mean that the 16 mbytes for generic CMA area is not a default on x86:

config CMA_SIZE_MBYTES
int "Size in Mega Bytes"
depends on !CMA_SIZE_SEL_PERCENTAGE
default 0 if X86
default 16

Which explains why it's rare to see these reports in the context such as yours.
I'd recommend just disabling it, as the primary use case for CMA are devices on 
mobile phones that don't have any other fallback (unlike the dma alloc).



It should be disallowed to enable CONFIG_CMA? Radeon and CMA should be
mutually exclusive?


I don't think this is a specific problem of radeon. But looks like it's a heavy 
user of the dma alloc. There might be others.



> Given that I haven't seen ANY other reports of this, I'm inclined to
> believe the problem is drm/radeon specific (if I don't start X, I can't
> reproduce the problem).

It's rather CMA specific, the allocation attemps just can't be 100% reliable due
to how CMA works. The question is if it should be spewing in the log in the
context of dma-cma, which has a fallback allocation option. It even uses
__GFP_NOWARN, perhaps the CMA path should respect that?

Yes, I'd say if there's a fallback without much penalty, nowarn makes
sense. If the fallback just tries multiple addresses until success, then
the warning should only be issued when too many attempts have been made.


On the other hand, if the warnings are correlated with high kernel CPU usage, 
it's arguably better to be warned.




> The rate of the problem starts slow, and also is relatively low on an idle
> system (my screens blank at night, no xscreensaver running), but it still 
ramps
> up over time (to the point of generating 2.5GB/hour of "(timestamp)
> alloc_contig_range: [83e4d9, 83e4da) PFNs busy"), with various addresses (~100
> unique ranges for a day).
>
> My X workload is ~50 chrome tabs and ~20 terminals (over 3x 24" monitors w/ 9
> virtual desktops per monitor).
So IIUC, except the messages, everything actually works fine?

There's high kernel CPU usage that seems to roughly correlate with the
messages, but I can't yet tell if that's due to the syslog itself, or
repeated alloc_contig_range requests.


You could try running perf top.

Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command

[Baolin Wang]

On 1 December 2016 at 14:35, Lu Baolu  wrote:
> Hi,
>
> On 12/01/2016 02:04 PM, Baolin Wang wrote:
>> Hi Baolu,
>>
>> On 1 December 2016 at 13:45, Lu Baolu  wrote:
>>> Hi,
>>>
>>> On 11/30/2016 05:02 PM, Baolin Wang wrote:
 If the hardware never responds to the stop endpoint command, the
 URBs will never be completed, and we might hang the USB subsystem.
 The original watchdog timer is used to watch if one stop endpoint
 command is timeout, if timeout, then the watchdog timer will set
 XHCI_STATE_DYING, try to halt the xHCI host, and give back all
 pending URBs.

 But now we already have one command timer to control command timeout,
 thus we can also use the command timer to watch the stop endpoint
 command, instead of one duplicate watchdog timer which need to be
 removed.

 Meanwhile we don't need the 'stop_cmds_pending' flag to identy if
 this is the last stop endpoint command of one endpoint. Since we
 can make sure we only set one stop endpoint command for one endpoint
 by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove
 this flag.
>>> I am afraid you can't do this. "stop_cmds_pending" was added
>>> to fix the problem described in the comments that you want to
>>> remove. But I didn't find any fix of this problem in your patch.
>> Now we can not pending another stop endpoint command for the same one
>> endpoint, since will check 'EP_HALT_PENDING' flag in
>> xhci_urb_dequeue() function to avoid this. But after some
>> investigation, I think I missed the stop endpoint command in
>> xhci_stop_device() which did not check the 'EP_HALT_PENDING' flag,
>> maybe need to add 'EP_HALT_PENDING' flag checking in
>> xhci_stop_device() function. DId I miss something else? Thanks.
>
> Consider below three threads running on different CPUs at the same time.
>
> Thread A: xhci_handle_cmd_stop_ep()  --- in interrupt handler
> Thread B: xhci_stop_endpoint_command_timeout() --- timer expired
> Thread C: xhci_urb_dequeue --- called by usb core
>
> They are serialized by xhci->lock. Let's consider below sequence:
>
> Thread A:
> - delete xhci->cmd_timer), but will fail due to Thread B.
> - clear EP_HALT_PENDING bit
>
> Thread B:
> - halt the host controller
>
> Thread C:
> - set EP_HALT_PENDING bit
> - enqueue another stop endpoint command
> - add the timer back

Ah, thanks for you comments.
If thread B halted the host, then the thread C xhci_urb_dequeue() will
check host state failed, then just return and can not set another stop
endpoint command. But from your example, I think your meaning is we
should not halt the host by thread B, since we have handled the stop
endpoint command event.

So I think we need to check the xhci command of this stop endpoint
command in xhci_stop_endpoint_command_timeout(), if the xhci command
of this stop endpoint command is not in the command list (which means
the stop endpoint command event has been handled), then just return
and do not halt the controller. Right?

-- 
Baolin.wang
Best Regards

Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command

[Baolin Wang]

On 1 December 2016 at 14:35, Lu Baolu  wrote:
> Hi,
>
> On 12/01/2016 02:04 PM, Baolin Wang wrote:
>> Hi Baolu,
>>
>> On 1 December 2016 at 13:45, Lu Baolu  wrote:
>>> Hi,
>>>
>>> On 11/30/2016 05:02 PM, Baolin Wang wrote:
 If the hardware never responds to the stop endpoint command, the
 URBs will never be completed, and we might hang the USB subsystem.
 The original watchdog timer is used to watch if one stop endpoint
 command is timeout, if timeout, then the watchdog timer will set
 XHCI_STATE_DYING, try to halt the xHCI host, and give back all
 pending URBs.

 But now we already have one command timer to control command timeout,
 thus we can also use the command timer to watch the stop endpoint
 command, instead of one duplicate watchdog timer which need to be
 removed.

 Meanwhile we don't need the 'stop_cmds_pending' flag to identy if
 this is the last stop endpoint command of one endpoint. Since we
 can make sure we only set one stop endpoint command for one endpoint
 by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove
 this flag.
>>> I am afraid you can't do this. "stop_cmds_pending" was added
>>> to fix the problem described in the comments that you want to
>>> remove. But I didn't find any fix of this problem in your patch.
>> Now we can not pending another stop endpoint command for the same one
>> endpoint, since will check 'EP_HALT_PENDING' flag in
>> xhci_urb_dequeue() function to avoid this. But after some
>> investigation, I think I missed the stop endpoint command in
>> xhci_stop_device() which did not check the 'EP_HALT_PENDING' flag,
>> maybe need to add 'EP_HALT_PENDING' flag checking in
>> xhci_stop_device() function. DId I miss something else? Thanks.
>
> Consider below three threads running on different CPUs at the same time.
>
> Thread A: xhci_handle_cmd_stop_ep()  --- in interrupt handler
> Thread B: xhci_stop_endpoint_command_timeout() --- timer expired
> Thread C: xhci_urb_dequeue --- called by usb core
>
> They are serialized by xhci->lock. Let's consider below sequence:
>
> Thread A:
> - delete xhci->cmd_timer), but will fail due to Thread B.
> - clear EP_HALT_PENDING bit
>
> Thread B:
> - halt the host controller
>
> Thread C:
> - set EP_HALT_PENDING bit
> - enqueue another stop endpoint command
> - add the timer back

Ah, thanks for you comments.
If thread B halted the host, then the thread C xhci_urb_dequeue() will
check host state failed, then just return and can not set another stop
endpoint command. But from your example, I think your meaning is we
should not halt the host by thread B, since we have handled the stop
endpoint command event.

So I think we need to check the xhci command of this stop endpoint
command in xhci_stop_endpoint_command_timeout(), if the xhci command
of this stop endpoint command is not in the command list (which means
the stop endpoint command event has been handled), then just return
and do not halt the controller. Right?

-- 
Baolin.wang
Best Regards

[PATCH 2/2] dma: ioat: fix improper return value on failures

[Pan Bian]

Fix bug https://bugzilla.kernel.org/show_bug.cgi?id=188601. This patch
is based on "0001-dma-ioat-set-error-code-on-failures.patch". In this
patch, assign error code -ENOMEM to return variable err as long as the
call to dma_mapping_error() fails.

Signed-off-by: Pan Bian 
---
 drivers/dma/ioat/init.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 32383ef..3d589f4 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -829,16 +829,20 @@ static int ioat_xor_val_self_test(struct ioatdma_device 
*ioat_dma)
op = IOAT_OP_XOR;
 
dest_dma = dma_map_page(dev, dest, 0, PAGE_SIZE, DMA_FROM_DEVICE);
-   if (dma_mapping_error(dev, dest_dma))
+   if (dma_mapping_error(dev, dest_dma)) {
+   err = -ENOMEM;
goto free_resources;
+   }
 
for (i = 0; i < IOAT_NUM_SRC_TEST; i++)
dma_srcs[i] = DMA_ERROR_CODE;
for (i = 0; i < IOAT_NUM_SRC_TEST; i++) {
dma_srcs[i] = dma_map_page(dev, xor_srcs[i], 0, PAGE_SIZE,
   DMA_TO_DEVICE);
-   if (dma_mapping_error(dev, dma_srcs[i]))
+   if (dma_mapping_error(dev, dma_srcs[i])) {
+   err = -ENOMEM;
goto dma_unmap;
+   }
}
tx = dma->device_prep_dma_xor(dma_chan, dest_dma, dma_srcs,
  IOAT_NUM_SRC_TEST, PAGE_SIZE,
@@ -906,8 +910,10 @@ static int ioat_xor_val_self_test(struct ioatdma_device 
*ioat_dma)
for (i = 0; i < IOAT_NUM_SRC_TEST + 1; i++) {
dma_srcs[i] = dma_map_page(dev, xor_val_srcs[i], 0, PAGE_SIZE,
   DMA_TO_DEVICE);
-   if (dma_mapping_error(dev, dma_srcs[i]))
+   if (dma_mapping_error(dev, dma_srcs[i])) {
+   err = -ENOMEM;
goto dma_unmap;
+   }
}
tx = dma->device_prep_dma_xor_val(dma_chan, dma_srcs,
  IOAT_NUM_SRC_TEST + 1, PAGE_SIZE,
@@ -959,8 +965,10 @@ static int ioat_xor_val_self_test(struct ioatdma_device 
*ioat_dma)
for (i = 0; i < IOAT_NUM_SRC_TEST + 1; i++) {
dma_srcs[i] = dma_map_page(dev, xor_val_srcs[i], 0, PAGE_SIZE,
   DMA_TO_DEVICE);
-   if (dma_mapping_error(dev, dma_srcs[i]))
+   if (dma_mapping_error(dev, dma_srcs[i])) {
+   err = -ENOMEM;
goto dma_unmap;
+   }
}
tx = dma->device_prep_dma_xor_val(dma_chan, dma_srcs,
  IOAT_NUM_SRC_TEST + 1, PAGE_SIZE,
-- 
1.9.1

[PATCH 2/2] dma: ioat: fix improper return value on failures

[Pan Bian]

Fix bug https://bugzilla.kernel.org/show_bug.cgi?id=188601. This patch
is based on "0001-dma-ioat-set-error-code-on-failures.patch". In this
patch, assign error code -ENOMEM to return variable err as long as the
call to dma_mapping_error() fails.

Signed-off-by: Pan Bian 
---
 drivers/dma/ioat/init.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 32383ef..3d589f4 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -829,16 +829,20 @@ static int ioat_xor_val_self_test(struct ioatdma_device 
*ioat_dma)
op = IOAT_OP_XOR;
 
dest_dma = dma_map_page(dev, dest, 0, PAGE_SIZE, DMA_FROM_DEVICE);
-   if (dma_mapping_error(dev, dest_dma))
+   if (dma_mapping_error(dev, dest_dma)) {
+   err = -ENOMEM;
goto free_resources;
+   }
 
for (i = 0; i < IOAT_NUM_SRC_TEST; i++)
dma_srcs[i] = DMA_ERROR_CODE;
for (i = 0; i < IOAT_NUM_SRC_TEST; i++) {
dma_srcs[i] = dma_map_page(dev, xor_srcs[i], 0, PAGE_SIZE,
   DMA_TO_DEVICE);
-   if (dma_mapping_error(dev, dma_srcs[i]))
+   if (dma_mapping_error(dev, dma_srcs[i])) {
+   err = -ENOMEM;
goto dma_unmap;
+   }
}
tx = dma->device_prep_dma_xor(dma_chan, dest_dma, dma_srcs,
  IOAT_NUM_SRC_TEST, PAGE_SIZE,
@@ -906,8 +910,10 @@ static int ioat_xor_val_self_test(struct ioatdma_device 
*ioat_dma)
for (i = 0; i < IOAT_NUM_SRC_TEST + 1; i++) {
dma_srcs[i] = dma_map_page(dev, xor_val_srcs[i], 0, PAGE_SIZE,
   DMA_TO_DEVICE);
-   if (dma_mapping_error(dev, dma_srcs[i]))
+   if (dma_mapping_error(dev, dma_srcs[i])) {
+   err = -ENOMEM;
goto dma_unmap;
+   }
}
tx = dma->device_prep_dma_xor_val(dma_chan, dma_srcs,
  IOAT_NUM_SRC_TEST + 1, PAGE_SIZE,
@@ -959,8 +965,10 @@ static int ioat_xor_val_self_test(struct ioatdma_device 
*ioat_dma)
for (i = 0; i < IOAT_NUM_SRC_TEST + 1; i++) {
dma_srcs[i] = dma_map_page(dev, xor_val_srcs[i], 0, PAGE_SIZE,
   DMA_TO_DEVICE);
-   if (dma_mapping_error(dev, dma_srcs[i]))
+   if (dma_mapping_error(dev, dma_srcs[i])) {
+   err = -ENOMEM;
goto dma_unmap;
+   }
}
tx = dma->device_prep_dma_xor_val(dma_chan, dma_srcs,
  IOAT_NUM_SRC_TEST + 1, PAGE_SIZE,
-- 
1.9.1

Re: [PATCH] staging: fbtft: remove duplicate entries of ili9225

[Greg KH]

On Wed, Nov 30, 2016 at 11:10:41PM +0530, Amitesh Singh wrote:
> On Nov 30, 2016 02:26, "Greg KH"  wrote:
> >
> > On Mon, Nov 28, 2016 at 05:55:29PM +0530, Amitesh Singh wrote:
> > > Signed-off-by: Amitesh Singh 
> > > ---
> >
> > I can't take patches without a changelog text :(
> 
> did you mean the text after the commit heading?

Yes, please read Documentation/SubmittingPatches for what you should be
putting into a changelog text.

Re: [PATCH] staging: fbtft: remove duplicate entries of ili9225

[Greg KH]

On Wed, Nov 30, 2016 at 11:10:41PM +0530, Amitesh Singh wrote:
> On Nov 30, 2016 02:26, "Greg KH"  wrote:
> >
> > On Mon, Nov 28, 2016 at 05:55:29PM +0530, Amitesh Singh wrote:
> > > Signed-off-by: Amitesh Singh 
> > > ---
> >
> > I can't take patches without a changelog text :(
> 
> did you mean the text after the commit heading?

Yes, please read Documentation/SubmittingPatches for what you should be
putting into a changelog text.

Re: [PATCH net] tipc: check minimum bearer MTU

[kbuild test robot]

Hi Michal,

[auto build test ERROR on net/master]

url:
https://github.com/0day-ci/linux/commits/Michal-Kubecek/tipc-check-minimum-bearer-MTU/20161201-140555
config: i386-randconfig-s0-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=i386 

All errors (new ones prefixed by >>):

   net/tipc/bearer.o: In function `tipc_check_mtu':
>> bearer.c:(.text+0xadd): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/core.o: In function `tipc_check_mtu':
   core.c:(.text+0x239): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/link.o: In function `tipc_check_mtu':
   link.c:(.text+0x110e): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/discover.o: In function `tipc_check_mtu':
   discover.c:(.text+0x2c7): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/name_distr.o: In function `tipc_check_mtu':
   name_distr.c:(.text+0x536): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/monitor.o: In function `tipc_check_mtu':
   monitor.c:(.text+0x906): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/name_table.o: In function `tipc_check_mtu':
   name_table.c:(.text+0x875): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/net.o: In function `tipc_check_mtu':
   net.c:(.text+0xc4): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/netlink.o: In function `tipc_check_mtu':
   netlink.c:(.text+0x0): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/netlink_compat.o: In function `tipc_check_mtu':
   netlink_compat.c:(.text+0x2a9e): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/node.o: In function `tipc_check_mtu':
   node.c:(.text+0x1dd8): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/socket.o: In function `tipc_check_mtu':
   socket.c:(.text+0x62bb): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/eth_media.o: In function `tipc_check_mtu':
   eth_media.c:(.text+0x117): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/udp_media.o: In function `tipc_check_mtu':
   udp_media.c:(.text+0x10b9): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH net] tipc: check minimum bearer MTU

[kbuild test robot]

Hi Michal,

[auto build test ERROR on net/master]

url:
https://github.com/0day-ci/linux/commits/Michal-Kubecek/tipc-check-minimum-bearer-MTU/20161201-140555
config: i386-randconfig-s0-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=i386 

All errors (new ones prefixed by >>):

   net/tipc/bearer.o: In function `tipc_check_mtu':
>> bearer.c:(.text+0xadd): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/core.o: In function `tipc_check_mtu':
   core.c:(.text+0x239): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/link.o: In function `tipc_check_mtu':
   link.c:(.text+0x110e): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/discover.o: In function `tipc_check_mtu':
   discover.c:(.text+0x2c7): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/name_distr.o: In function `tipc_check_mtu':
   name_distr.c:(.text+0x536): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/monitor.o: In function `tipc_check_mtu':
   monitor.c:(.text+0x906): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/name_table.o: In function `tipc_check_mtu':
   name_table.c:(.text+0x875): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/net.o: In function `tipc_check_mtu':
   net.c:(.text+0xc4): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/netlink.o: In function `tipc_check_mtu':
   netlink.c:(.text+0x0): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/netlink_compat.o: In function `tipc_check_mtu':
   netlink_compat.c:(.text+0x2a9e): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/node.o: In function `tipc_check_mtu':
   node.c:(.text+0x1dd8): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/socket.o: In function `tipc_check_mtu':
   socket.c:(.text+0x62bb): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/eth_media.o: In function `tipc_check_mtu':
   eth_media.c:(.text+0x117): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here
   net/tipc/udp_media.o: In function `tipc_check_mtu':
   udp_media.c:(.text+0x10b9): multiple definition of `tipc_check_mtu'
   net/tipc/bcast.o:bcast.c:(.text+0x598): first defined here

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: drm/radeon spamming alloc_contig_range: [xxx, yyy) PFNs busy busy

[Michal Hocko]

Forgot to CC Joonsoo. The email thread starts more or less here
http://lkml.kernel.org/r/20161130092239.gd18...@dhcp22.suse.cz

On Thu 01-12-16 08:15:07, Michal Hocko wrote:
> On Wed 30-11-16 20:19:03, Robin H. Johnson wrote:
> [...]
> > alloc_contig_range: [83f2a3, 83f2a4) PFNs busy
> 
> Huh, do I get it right that the request was for a _single_ page? Why do
> we need CMA for that?

-- 
Michal Hocko
SUSE Labs

Re: drm/radeon spamming alloc_contig_range: [xxx, yyy) PFNs busy busy

[Michal Hocko]

Forgot to CC Joonsoo. The email thread starts more or less here
http://lkml.kernel.org/r/20161130092239.gd18...@dhcp22.suse.cz

On Thu 01-12-16 08:15:07, Michal Hocko wrote:
> On Wed 30-11-16 20:19:03, Robin H. Johnson wrote:
> [...]
> > alloc_contig_range: [83f2a3, 83f2a4) PFNs busy
> 
> Huh, do I get it right that the request was for a _single_ page? Why do
> we need CMA for that?

-- 
Michal Hocko
SUSE Labs

Re: i8042 error at booting an Intel Cherry Trail-based device

[Takashi Iwai]

On Thu, 01 Dec 2016 03:29:23 +0100,
Dmitry Torokhov wrote:
> 
> Hi Takashi,
> 
> On Mon, Nov 28, 2016 at 02:56:36PM +0100, Takashi Iwai wrote:
> > Hi Dmitry,
> > 
> > I've been testing a small machine with Intel Cherry Trail chipset, and
> > noticed that the kernel spews errors always like:
> > 
> >  i8042: PNP: No PS/2 controller found. Probing ports directly.
> >  i8042: Can't read CTR while initializing i8042
> >  i8042: probe of i8042 failed with error -5
> > 
> > Especially the second one ("Can't read CTR...") is annoying since it's
> > in KERN_ERR level and thus appears even booted with quiet boot
> > option.  Actually this is the only error message appearing at boot, so
> > I'd love to get rid of it.
> > 
> > What is the preferred way to reduce this?  For example, is a patch
> > like below OK to simply change the log level and the error code?
> 
> No, because if controller is actually present this is a hard failure and
> we should be reporting it, not suppressing it.
> 
> The issue is that we did not believe PNP data and in this case we should
> have. Unfortunately in old days there was a lot of crap in PNP/ACPI
> tables, but it could be better now. We can try, in addition to PNP
> matching, checking 8042 flag in "Fixed ACPI Description Table Boot
> Architecture Flags" in FADT and if it also shows there is no 8042 then
> bail.

That sounds promising.  Indeed FACL.dsl shows like:

[000h    4]Signature : "FACP"[Fixed ACPI 
Description Table (FADT)]
[004h 0004   4] Table Length : 010C

   Legacy Devices Supported (V2) : 0
8042 Present on ports 60/64 (V2) : 0

If a test patch gets ready, let me know, I'll give it a try.


Thanks!

Takashi

Re: i8042 error at booting an Intel Cherry Trail-based device

[Takashi Iwai]

On Thu, 01 Dec 2016 03:29:23 +0100,
Dmitry Torokhov wrote:
> 
> Hi Takashi,
> 
> On Mon, Nov 28, 2016 at 02:56:36PM +0100, Takashi Iwai wrote:
> > Hi Dmitry,
> > 
> > I've been testing a small machine with Intel Cherry Trail chipset, and
> > noticed that the kernel spews errors always like:
> > 
> >  i8042: PNP: No PS/2 controller found. Probing ports directly.
> >  i8042: Can't read CTR while initializing i8042
> >  i8042: probe of i8042 failed with error -5
> > 
> > Especially the second one ("Can't read CTR...") is annoying since it's
> > in KERN_ERR level and thus appears even booted with quiet boot
> > option.  Actually this is the only error message appearing at boot, so
> > I'd love to get rid of it.
> > 
> > What is the preferred way to reduce this?  For example, is a patch
> > like below OK to simply change the log level and the error code?
> 
> No, because if controller is actually present this is a hard failure and
> we should be reporting it, not suppressing it.
> 
> The issue is that we did not believe PNP data and in this case we should
> have. Unfortunately in old days there was a lot of crap in PNP/ACPI
> tables, but it could be better now. We can try, in addition to PNP
> matching, checking 8042 flag in "Fixed ACPI Description Table Boot
> Architecture Flags" in FADT and if it also shows there is no 8042 then
> bail.

That sounds promising.  Indeed FACL.dsl shows like:

[000h    4]Signature : "FACP"[Fixed ACPI 
Description Table (FADT)]
[004h 0004   4] Table Length : 010C

   Legacy Devices Supported (V2) : 0
8042 Present on ports 60/64 (V2) : 0

If a test patch gets ready, let me know, I'll give it a try.


Thanks!

Takashi

Re: [PATCH v2] USB: OHCI: ohci-pxa27x: remove useless functions

[Greg Kroah-Hartman]

On Wed, Nov 30, 2016 at 11:59:49PM +, csmanjuvi...@gmail.com wrote:
> From: Manjunath Goudar 
> 
> The ohci_hcd_pxa27x_drv_probe function is not doing anything other
> than calling usb_hcd_pxa27x_probe function so ohci_hcd_pxa27x_drv_probe
> function is useless that is why removed ohci_hcd_pxa27x_drv_probe
> function and renamed usb_hcd_pxa27x_probe function to
> ohci_hcd_pxa27x_drv_probe for proper naming.
> 
> The ohci_hcd_pxa27x_remove function is also not doing anything other than
> calling usb_hcd_pxa27x_remove that is why removed ohci_hcd_pxa27x_remove
> function and renamed usb_hcd_pxa27x_remove to ohci_hcd_pxa27x_remove for
> proper naming.
> 
> Signed-off-by: Manjunath Goudar 
> Cc: Alan Stern 
> Cc: Greg Kroah-Hartman 
> Cc: linux-...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> 
> Changelog v1 -> v2:
> 1.Removed warnings and errors of checkpatch.pl script.
> 2.Replaced unuseful with useless in patch commit message for proper meaning.
> 3.Removed usb_disabled() from ohci_hcd_pxa27x_drv_probe function because it
>   is already existing in ohci_pxa27x_init function
> ---
>  drivers/usb/host/ohci-pxa27x.c | 42 
> ++
>  1 file changed, 14 insertions(+), 28 deletions(-)

changelog should go below --- line.


> 
> diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
> index a667cf2..171f28f2 100644
> --- a/drivers/usb/host/ohci-pxa27x.c
> +++ b/drivers/usb/host/ohci-pxa27x.c
> @@ -157,9 +157,9 @@ static int pxa27x_ohci_select_pmm(struct pxa27x_ohci 
> *pxa_ohci, int mode)
>   uhcrhdb |= (0x7<<17);
>   break;
>   default:
> - printk( KERN_ERR
> + printk(KERN_ERR

No, don't do checkpatch cleanups in portions of the code you are not
actually changing for your "real" patch.  That's a mess.

You can send checkpatch patches as follow-on patches, but not with this
one.

thanks,

greg k-h

Re: [PATCH v2] USB: OHCI: ohci-pxa27x: remove useless functions

[Greg Kroah-Hartman]

On Wed, Nov 30, 2016 at 11:59:49PM +, csmanjuvi...@gmail.com wrote:
> From: Manjunath Goudar 
> 
> The ohci_hcd_pxa27x_drv_probe function is not doing anything other
> than calling usb_hcd_pxa27x_probe function so ohci_hcd_pxa27x_drv_probe
> function is useless that is why removed ohci_hcd_pxa27x_drv_probe
> function and renamed usb_hcd_pxa27x_probe function to
> ohci_hcd_pxa27x_drv_probe for proper naming.
> 
> The ohci_hcd_pxa27x_remove function is also not doing anything other than
> calling usb_hcd_pxa27x_remove that is why removed ohci_hcd_pxa27x_remove
> function and renamed usb_hcd_pxa27x_remove to ohci_hcd_pxa27x_remove for
> proper naming.
> 
> Signed-off-by: Manjunath Goudar 
> Cc: Alan Stern 
> Cc: Greg Kroah-Hartman 
> Cc: linux-...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> 
> Changelog v1 -> v2:
> 1.Removed warnings and errors of checkpatch.pl script.
> 2.Replaced unuseful with useless in patch commit message for proper meaning.
> 3.Removed usb_disabled() from ohci_hcd_pxa27x_drv_probe function because it
>   is already existing in ohci_pxa27x_init function
> ---
>  drivers/usb/host/ohci-pxa27x.c | 42 
> ++
>  1 file changed, 14 insertions(+), 28 deletions(-)

changelog should go below --- line.


> 
> diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
> index a667cf2..171f28f2 100644
> --- a/drivers/usb/host/ohci-pxa27x.c
> +++ b/drivers/usb/host/ohci-pxa27x.c
> @@ -157,9 +157,9 @@ static int pxa27x_ohci_select_pmm(struct pxa27x_ohci 
> *pxa_ohci, int mode)
>   uhcrhdb |= (0x7<<17);
>   break;
>   default:
> - printk( KERN_ERR
> + printk(KERN_ERR

No, don't do checkpatch cleanups in portions of the code you are not
actually changing for your "real" patch.  That's a mess.

You can send checkpatch patches as follow-on patches, but not with this
one.

thanks,

greg k-h

Re: [PATCH 4.8 00/37] 4.8.12-stable review

[Greg Kroah-Hartman]

On Wed, Nov 30, 2016 at 03:34:29PM -0800, Guenter Roeck wrote:
> On Wed, Nov 30, 2016 at 10:29:37AM +0100, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.8.12 release.
> > There are 37 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Fri Dec  2 09:27:19 UTC 2016.
> > Anything received after that time might be too late.
> > 
> 
> Build results:
>   total: 149 pass: 149 fail: 0
> Qemu test results:
>   total: 122 pass: 122 fail: 0
> 
> The results are for v4.8.11-37-g4a6a922.
> 
> Details are available at http://kerneltests.org/builders.

Great, thanks for testing both of these and letting me know.

greg k-h

Re: [PATCH 4.8 00/37] 4.8.12-stable review

[Greg Kroah-Hartman]

On Wed, Nov 30, 2016 at 03:34:29PM -0800, Guenter Roeck wrote:
> On Wed, Nov 30, 2016 at 10:29:37AM +0100, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.8.12 release.
> > There are 37 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Fri Dec  2 09:27:19 UTC 2016.
> > Anything received after that time might be too late.
> > 
> 
> Build results:
>   total: 149 pass: 149 fail: 0
> Qemu test results:
>   total: 122 pass: 122 fail: 0
> 
> The results are for v4.8.11-37-g4a6a922.
> 
> Details are available at http://kerneltests.org/builders.

Great, thanks for testing both of these and letting me know.

greg k-h

Re: drm/radeon spamming alloc_contig_range: [xxx, yyy) PFNs busy busy

[Michal Hocko]

On Wed 30-11-16 20:19:03, Robin H. Johnson wrote:
[...]
> alloc_contig_range: [83f2a3, 83f2a4) PFNs busy

Huh, do I get it right that the request was for a _single_ page? Why do
we need CMA for that?
-- 
Michal Hocko
SUSE Labs

Re: drm/radeon spamming alloc_contig_range: [xxx, yyy) PFNs busy busy

[Michal Hocko]

On Wed 30-11-16 20:19:03, Robin H. Johnson wrote:
[...]
> alloc_contig_range: [83f2a3, 83f2a4) PFNs busy

Huh, do I get it right that the request was for a _single_ page? Why do
we need CMA for that?
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 4.8 00/37] 4.8.12-stable review

[Greg Kroah-Hartman]

On Wed, Nov 30, 2016 at 09:04:28AM -0700, Shuah Khan wrote:
> On 11/30/2016 02:29 AM, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.8.12 release.
> > There are 37 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Fri Dec  2 09:27:19 UTC 2016.
> > Anything received after that time might be too late.
> > 
> > The whole patch series can be found in one patch at:
> > kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.8.12-rc1.gz
> > or in the git tree and branch at:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> > linux-4.8.y
> > and the diffstat can be found below.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Compiled and booted on my test system. No dmesg regressions.

Thanks for testing both of these and letting me know.

greg k-h

Re: [PATCH 4.8 00/37] 4.8.12-stable review

[Greg Kroah-Hartman]

On Wed, Nov 30, 2016 at 09:04:28AM -0700, Shuah Khan wrote:
> On 11/30/2016 02:29 AM, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.8.12 release.
> > There are 37 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Fri Dec  2 09:27:19 UTC 2016.
> > Anything received after that time might be too late.
> > 
> > The whole patch series can be found in one patch at:
> > kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.8.12-rc1.gz
> > or in the git tree and branch at:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> > linux-4.8.y
> > and the diffstat can be found below.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Compiled and booted on my test system. No dmesg regressions.

Thanks for testing both of these and letting me know.

greg k-h

Re: [PATCH 4.8 00/37] 4.8.12-stable review

[Greg Kroah-Hartman]

On Wed, Nov 30, 2016 at 08:39:35AM -0800, Kevin Hilman wrote:
> kernelci.org bot  writes:
> 
> > stable-rc boot: 226 boots: 6 failed, 214 passed with 5 offline, 1 conflict 
> > (v4.8.11-37-g70e9fe5f6b13)
> >
> > Full Boot Summary: 
> > https://kernelci.org/boot/all/job/stable-rc/kernel/v4.8.11-37-g70e9fe5f6b13/
> > Full Build Summary: 
> > https://kernelci.org/build/stable-rc/kernel/v4.8.11-37-g70e9fe5f6b13/
> >
> > Tree: stable-rc
> > Branch: local/linux-4.8.y
> > Git Describe: v4.8.11-37-g70e9fe5f6b13
> > Git Commit: 70e9fe5f6b13b404a1575496b9727aed109b0fca
> > Git URL: 
> > http://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> > Tested: 83 unique boards, 22 SoC families, 31 builds out of 207
> >
> > Boot Regressions Detected:
> >
> > arm:
> >
> > multi_v7_defconfig+CONFIG_EFI=y:
> > vexpress-v2p-ca15_a7:
> > lab-broonie: new failure (last pass: v4.8.11)
> >
> > multi_v7_defconfig+CONFIG_LKDTM=y:
> > vexpress-v2p-ca15-tc1:
> > lab-broonie: new failure (last pass: v4.8.11)
> > vexpress-v2p-ca15_a7:
> > lab-broonie: new failure (last pass: v4.8.11)
> >
> > multi_v7_defconfig+CONFIG_THUMB2_KERNEL=y+CONFIG_ARM_MODULE_PLTS=y:
> > vexpress-v2p-ca15-tc1:
> > lab-broonie: new failure (last pass: v4.8.11)
> > vexpress-v2p-ca15_a7:
> > lab-broonie: new failure (last pass: v4.8.11)
> 
> These appear to be lab issues due to not waiting long enough for QEMU
> jobs.  Under investigation.
> 
> (BTW, notice the new "regressions" section here.  Hopefully this will be
> more useful for seeing what's actually new failures.)

Yes, that was nice, but I couldn't tell what the real problem was,
thanks for digging into it.

> > Boot Failures Detected:
> >
> > arm:
> >
> > multi_v7_defconfig+CONFIG_PROVE_LOCKING=y
> > at91-sama5d4_xplained: 1 failed lab
> 
> This is passing in another lab, so looks like a local lab issue.
> 
> > multi_v7_defconfig
> > omap5-uevm_rootfs:nfs: 1 failed lab
> 
> Looks like local lab networking issue.
> 
> IOW, interpretation: no blocking issues.

Wonderful, thanks for letting me know.

greg k-h

Re: [PATCH 4.8 00/37] 4.8.12-stable review

[Greg Kroah-Hartman]

On Wed, Nov 30, 2016 at 08:39:35AM -0800, Kevin Hilman wrote:
> kernelci.org bot  writes:
> 
> > stable-rc boot: 226 boots: 6 failed, 214 passed with 5 offline, 1 conflict 
> > (v4.8.11-37-g70e9fe5f6b13)
> >
> > Full Boot Summary: 
> > https://kernelci.org/boot/all/job/stable-rc/kernel/v4.8.11-37-g70e9fe5f6b13/
> > Full Build Summary: 
> > https://kernelci.org/build/stable-rc/kernel/v4.8.11-37-g70e9fe5f6b13/
> >
> > Tree: stable-rc
> > Branch: local/linux-4.8.y
> > Git Describe: v4.8.11-37-g70e9fe5f6b13
> > Git Commit: 70e9fe5f6b13b404a1575496b9727aed109b0fca
> > Git URL: 
> > http://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> > Tested: 83 unique boards, 22 SoC families, 31 builds out of 207
> >
> > Boot Regressions Detected:
> >
> > arm:
> >
> > multi_v7_defconfig+CONFIG_EFI=y:
> > vexpress-v2p-ca15_a7:
> > lab-broonie: new failure (last pass: v4.8.11)
> >
> > multi_v7_defconfig+CONFIG_LKDTM=y:
> > vexpress-v2p-ca15-tc1:
> > lab-broonie: new failure (last pass: v4.8.11)
> > vexpress-v2p-ca15_a7:
> > lab-broonie: new failure (last pass: v4.8.11)
> >
> > multi_v7_defconfig+CONFIG_THUMB2_KERNEL=y+CONFIG_ARM_MODULE_PLTS=y:
> > vexpress-v2p-ca15-tc1:
> > lab-broonie: new failure (last pass: v4.8.11)
> > vexpress-v2p-ca15_a7:
> > lab-broonie: new failure (last pass: v4.8.11)
> 
> These appear to be lab issues due to not waiting long enough for QEMU
> jobs.  Under investigation.
> 
> (BTW, notice the new "regressions" section here.  Hopefully this will be
> more useful for seeing what's actually new failures.)

Yes, that was nice, but I couldn't tell what the real problem was,
thanks for digging into it.

> > Boot Failures Detected:
> >
> > arm:
> >
> > multi_v7_defconfig+CONFIG_PROVE_LOCKING=y
> > at91-sama5d4_xplained: 1 failed lab
> 
> This is passing in another lab, so looks like a local lab issue.
> 
> > multi_v7_defconfig
> > omap5-uevm_rootfs:nfs: 1 failed lab
> 
> Looks like local lab networking issue.
> 
> IOW, interpretation: no blocking issues.

Wonderful, thanks for letting me know.

greg k-h

Re: [PATCH 4.8 11/37] scsi: mpt3sas: Fix secure erase premature termination

[Greg Kroah-Hartman]

On Wed, Nov 30, 2016 at 11:49:56AM -0500, Martin K. Petersen wrote:
> > "Greg" == Greg Kroah-Hartman  writes:
> 
> Greg,
> 
> Greg> From: Andrey Grodzovsky 
> 
> Greg> commit 18f6084a989ba1b38702f9af37a2e4049a924be6 upstream.
> 
> Please also queue 7ff723ad0f87 ("scsi: mpt3sas: Unblock device after
> controller reset") which just hit Linus' tree. It's a bug fix for the
> patch above.

Now queued up, thanks.

greg k-h

[PATCH 1/1] dma: ioat: set error code on failures

[Pan Bian]

Fix bug https://bugzilla.kernel.org/show_bug.cgi?id=188591. In function
ioat_dma_self_test(), when the calls to dma_mapping_error() fails, the
value of return variable err is 0 (indicates no error). As a result, the
return value may be inconsistent with the execution status. This patch
fixes the bug by assigning -ENOMEM to err on the error path.

Signed-off-by: Pan Bian 
---
 drivers/dma/ioat/init.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 015f711..32383ef 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -340,11 +340,13 @@ static int ioat_dma_self_test(struct ioatdma_device 
*ioat_dma)
dma_src = dma_map_single(dev, src, IOAT_TEST_SIZE, DMA_TO_DEVICE);
if (dma_mapping_error(dev, dma_src)) {
dev_err(dev, "mapping src buffer failed\n");
+   err = -ENOMEM;
goto free_resources;
}
dma_dest = dma_map_single(dev, dest, IOAT_TEST_SIZE, DMA_FROM_DEVICE);
if (dma_mapping_error(dev, dma_dest)) {
dev_err(dev, "mapping dest buffer failed\n");
+   err = -ENOMEM;
goto unmap_src;
}
flags = DMA_PREP_INTERRUPT;
-- 
1.9.1

Re: [PATCH 4.8 11/37] scsi: mpt3sas: Fix secure erase premature termination

[Greg Kroah-Hartman]

On Wed, Nov 30, 2016 at 11:49:56AM -0500, Martin K. Petersen wrote:
> > "Greg" == Greg Kroah-Hartman  writes:
> 
> Greg,
> 
> Greg> From: Andrey Grodzovsky 
> 
> Greg> commit 18f6084a989ba1b38702f9af37a2e4049a924be6 upstream.
> 
> Please also queue 7ff723ad0f87 ("scsi: mpt3sas: Unblock device after
> controller reset") which just hit Linus' tree. It's a bug fix for the
> patch above.

Now queued up, thanks.

greg k-h

[PATCH 1/1] dma: ioat: set error code on failures

[Pan Bian]

Fix bug https://bugzilla.kernel.org/show_bug.cgi?id=188591. In function
ioat_dma_self_test(), when the calls to dma_mapping_error() fails, the
value of return variable err is 0 (indicates no error). As a result, the
return value may be inconsistent with the execution status. This patch
fixes the bug by assigning -ENOMEM to err on the error path.

Signed-off-by: Pan Bian 
---
 drivers/dma/ioat/init.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 015f711..32383ef 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -340,11 +340,13 @@ static int ioat_dma_self_test(struct ioatdma_device 
*ioat_dma)
dma_src = dma_map_single(dev, src, IOAT_TEST_SIZE, DMA_TO_DEVICE);
if (dma_mapping_error(dev, dma_src)) {
dev_err(dev, "mapping src buffer failed\n");
+   err = -ENOMEM;
goto free_resources;
}
dma_dest = dma_map_single(dev, dest, IOAT_TEST_SIZE, DMA_FROM_DEVICE);
if (dma_mapping_error(dev, dma_dest)) {
dev_err(dev, "mapping dest buffer failed\n");
+   err = -ENOMEM;
goto unmap_src;
}
flags = DMA_PREP_INTERRUPT;
-- 
1.9.1

Re: [PATCH 2/3 v2] ALSA: usb-audio: avoid setting of sample rate multiple times on bus

[Jiada Wang]

Hi Takashi

On 11/30/2016 05:51 PM, Takashi Iwai wrote:

On Wed, 30 Nov 2016 08:59:22 +0100,
Jiada Wang wrote:


From: Daniel Girnus 

ALSA usually calls the prepare function twice before starting the playback:
1. On hw_params call from userland and
2. internally when starting the stream.
Some device are not able to manage this and they will stop playback
if the sample rate will be configured several times over USB protocol.

Signed-off-by: Jens Lorenz 
Signed-off-by: Jiada Wang 


The sign-off from Daniel seems missing?

The code change looks OK, but it'd be nice to mention in the changelog
that, after this patch, snd_usb_init_sample_rate() is still called
properly whenever the parameter is changed since ep->need_setup_ep is
set in snd_hsb_hw_params().


I will add missing sign-off and related information in changelog in v2

Thanks,
Jiada


thanks,

Takashi


---
 sound/usb/pcm.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 44d178e..a522c9a 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -806,17 +806,18 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream 
*substream)
if (ret < 0)
goto unlock;

-   iface = usb_ifnum_to_if(subs->dev, subs->cur_audiofmt->iface);
-   alts = >altsetting[subs->cur_audiofmt->altset_idx];
-   ret = snd_usb_init_sample_rate(subs->stream->chip,
-  subs->cur_audiofmt->iface,
-  alts,
-  subs->cur_audiofmt,
-  subs->cur_rate);
-   if (ret < 0)
-   goto unlock;
-
if (subs->need_setup_ep) {
+
+   iface = usb_ifnum_to_if(subs->dev, subs->cur_audiofmt->iface);
+   alts = >altsetting[subs->cur_audiofmt->altset_idx];
+   ret = snd_usb_init_sample_rate(subs->stream->chip,
+  subs->cur_audiofmt->iface,
+  alts,
+  subs->cur_audiofmt,
+  subs->cur_rate);
+   if (ret < 0)
+   goto unlock;
+
ret = configure_endpoint(subs);
if (ret < 0)
goto unlock;
--
2.9.3

Re: [PATCH 2/3 v2] ALSA: usb-audio: avoid setting of sample rate multiple times on bus

[Jiada Wang]

Hi Takashi

On 11/30/2016 05:51 PM, Takashi Iwai wrote:

On Wed, 30 Nov 2016 08:59:22 +0100,
Jiada Wang wrote:


From: Daniel Girnus 

ALSA usually calls the prepare function twice before starting the playback:
1. On hw_params call from userland and
2. internally when starting the stream.
Some device are not able to manage this and they will stop playback
if the sample rate will be configured several times over USB protocol.

Signed-off-by: Jens Lorenz 
Signed-off-by: Jiada Wang 


The sign-off from Daniel seems missing?

The code change looks OK, but it'd be nice to mention in the changelog
that, after this patch, snd_usb_init_sample_rate() is still called
properly whenever the parameter is changed since ep->need_setup_ep is
set in snd_hsb_hw_params().


I will add missing sign-off and related information in changelog in v2

Thanks,
Jiada


thanks,

Takashi


---
 sound/usb/pcm.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 44d178e..a522c9a 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -806,17 +806,18 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream 
*substream)
if (ret < 0)
goto unlock;

-   iface = usb_ifnum_to_if(subs->dev, subs->cur_audiofmt->iface);
-   alts = >altsetting[subs->cur_audiofmt->altset_idx];
-   ret = snd_usb_init_sample_rate(subs->stream->chip,
-  subs->cur_audiofmt->iface,
-  alts,
-  subs->cur_audiofmt,
-  subs->cur_rate);
-   if (ret < 0)
-   goto unlock;
-
if (subs->need_setup_ep) {
+
+   iface = usb_ifnum_to_if(subs->dev, subs->cur_audiofmt->iface);
+   alts = >altsetting[subs->cur_audiofmt->altset_idx];
+   ret = snd_usb_init_sample_rate(subs->stream->chip,
+  subs->cur_audiofmt->iface,
+  alts,
+  subs->cur_audiofmt,
+  subs->cur_rate);
+   if (ret < 0)
+   goto unlock;
+
ret = configure_endpoint(subs);
if (ret < 0)
goto unlock;
--
2.9.3

Re: [PATCH 1/3 v1] ALSA: usb-audio: more tolerant packetsize

[Jiada Wang]

Hello Takashi

On 11/30/2016 05:54 PM, Takashi Iwai wrote:

On Wed, 30 Nov 2016 08:59:21 +0100,
Jiada Wang wrote:


From: Andreas Pape 

since commit 57e6dae1087bbaa6b33d3dd8a8e90b63888939a3 the expected packetsize 
is always limited to


Please use a form with 12 chars SHA ID plus the commit subject, e.g.
1234567890ab ("blah blah...")

I will update changelog as you have suggested in v2.



nominal + 25%. It was discovered, that some devices have a much higher jitter
in used packetsizes than 25% which would result in BABBLE condition and 
dropping of packets.
A better solution is so assume the jitter to be the nominal packetsize:
-one nearly empty packet followed by a almost double sized one.


The increase of the max frequency is supposedly OK.
A remaining question is whether this should be included in stable
kernel.  It fixes in one side, but it's quite untested in another
side.  Maybe we queue this for 4.10, and later on notify to stable
maintainer once when it's confirmed to work and be harmless.



this makes sense to me

Thanks,
Jiada

thanks,

Takashi



Signed-off-by: Andreas Pape 
Signed-off-by: Jiada Wang 
---
 sound/usb/endpoint.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index c470251..2f592dd 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -632,8 +632,8 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
ep->stride = frame_bits >> 3;
ep->silence_value = pcm_format == SNDRV_PCM_FORMAT_U8 ? 0x80 : 0;

-   /* assume max. frequency is 25% higher than nominal */
-   ep->freqmax = ep->freqn + (ep->freqn >> 2);
+   /* assume max. frequency is double than nominal */
+   ep->freqmax = ep->freqn * 2;
/* Round up freqmax to nearest integer in order to calculate maximum
 * packet size, which must represent a whole number of frames.
 * This is accomplished by adding 0x0. before converting the
--
2.9.3

Re: [PATCH 1/3 v1] ALSA: usb-audio: more tolerant packetsize

[Jiada Wang]

Hello Takashi

On 11/30/2016 05:54 PM, Takashi Iwai wrote:

On Wed, 30 Nov 2016 08:59:21 +0100,
Jiada Wang wrote:


From: Andreas Pape 

since commit 57e6dae1087bbaa6b33d3dd8a8e90b63888939a3 the expected packetsize 
is always limited to


Please use a form with 12 chars SHA ID plus the commit subject, e.g.
1234567890ab ("blah blah...")

I will update changelog as you have suggested in v2.



nominal + 25%. It was discovered, that some devices have a much higher jitter
in used packetsizes than 25% which would result in BABBLE condition and 
dropping of packets.
A better solution is so assume the jitter to be the nominal packetsize:
-one nearly empty packet followed by a almost double sized one.


The increase of the max frequency is supposedly OK.
A remaining question is whether this should be included in stable
kernel.  It fixes in one side, but it's quite untested in another
side.  Maybe we queue this for 4.10, and later on notify to stable
maintainer once when it's confirmed to work and be harmless.



this makes sense to me

Thanks,
Jiada

thanks,

Takashi



Signed-off-by: Andreas Pape 
Signed-off-by: Jiada Wang 
---
 sound/usb/endpoint.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index c470251..2f592dd 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -632,8 +632,8 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
ep->stride = frame_bits >> 3;
ep->silence_value = pcm_format == SNDRV_PCM_FORMAT_U8 ? 0x80 : 0;

-   /* assume max. frequency is 25% higher than nominal */
-   ep->freqmax = ep->freqn + (ep->freqn >> 2);
+   /* assume max. frequency is double than nominal */
+   ep->freqmax = ep->freqn * 2;
/* Round up freqmax to nearest integer in order to calculate maximum
 * packet size, which must represent a whole number of frames.
 * This is accomplished by adding 0x0. before converting the
--
2.9.3

Re: next: Commit 'mm: Prevent __alloc_pages_nodemask() RCU CPU stall ...' causing hang on sparc32 qemu

[Guenter Roeck]

Hi Paul,

On Wed, Nov 30, 2016 at 05:19:50PM -0800, Paul E. McKenney wrote:
[ ... ]

> > > > 
> > > > BUG: sleeping function called from invalid context at 
> > > > mm/page_alloc.c:3775
[ ... ]
> 
> Whew!  You had me going for a bit there.  ;-)

Bisect results are here ... the culprit is, again, commit 2d66cccd73 ("mm:
Prevent __alloc_pages_nodemask() RCU CPU stall warnings"), and reverting that
patch fixes the problem. Good that you dropped it already :-).

Guenter

---
# bad: [59ab0083490c8a871b51e893bae5806e55901d7e] Add linux-next specific files 
for 20161130
# good: [e5517c2a5a49ed5e99047008629f1cd60246ea0e] Linux 4.9-rc7
git bisect start 'HEAD' 'v4.9-rc7'
# good: [187f99e5c22bb3fab8b330f3ebbbd235d238f3f8] Merge remote-tracking branch 
'crypto/master'
git bisect good 187f99e5c22bb3fab8b330f3ebbbd235d238f3f8
# good: [36126657c908e822523b8563f9b1512937c0f342] Merge remote-tracking branch 
'tip/auto-latest'
git bisect good 36126657c908e822523b8563f9b1512937c0f342
# good: [2d2139c5c746ec61024fdfa9c36e4e034bb18e59] Merge tag 'iio-for-4.10d' of 
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio into staging-next
git bisect good 2d2139c5c746ec61024fdfa9c36e4e034bb18e59
# bad: [926a60551123048c589b45abee2a2ec4c924ab21] Merge remote-tracking branch 
'extcon/extcon-next'
git bisect bad 926a60551123048c589b45abee2a2ec4c924ab21
# bad: [1541655795a90720b8a094c8cc39f582dec17398] Merge remote-tracking branch 
'tty/tty-next'
git bisect bad 1541655795a90720b8a094c8cc39f582dec17398
# bad: [69a6720a1e54519d9bf8563764e9e93bf1bd6a84] Merge remote-tracking branch 
'kvm-arm/next'
git bisect bad 69a6720a1e54519d9bf8563764e9e93bf1bd6a84
# good: [33b8b045b93f9104c61ecad1865af961b3bef03e] Merge remote-tracking branch 
'ftrace/for-next'
git bisect good 33b8b045b93f9104c61ecad1865af961b3bef03e
# good: [8370c3d08bd98576d97514eca29970e03767a5d1] kvm: svm: Add 
kvm_fast_pio_in support
git bisect good 8370c3d08bd98576d97514eca29970e03767a5d1
# good: [0a895142323de3eebb0b753d3d8c0e768ff179d9] mm: Prevent shrink_node() 
RCU CPU stall warnings
git bisect good 0a895142323de3eebb0b753d3d8c0e768ff179d9
# bad: [f8045446ca778333e960dcb9e30a5858ff2b8c20] srcu: Force full grace-period 
ordering
git bisect bad f8045446ca778333e960dcb9e30a5858ff2b8c20
# good: [f660d64912ccadadcdce6dfb39eb06924dd93767] doc: Fix RCU requirements 
typos
git bisect good f660d64912ccadadcdce6dfb39eb06924dd93767
# good: [d2db185bfee894c573faebed93461e9938bdbb61] rcu: Remove short-term CPU 
kicking
git bisect good d2db185bfee894c573faebed93461e9938bdbb61
# bad: [2d66cccd73436ac9985a08e5c2f82e4344f72264] mm: Prevent 
__alloc_pages_nodemask() RCU CPU stall warnings
git bisect bad 2d66cccd73436ac9985a08e5c2f82e4344f72264
# good: [34c53f5cd399801b083047cc9cf2ad3ed17c3144] mm: Prevent 
shrink_node_memcg() RCU CPU stall warnings
git bisect good 34c53f5cd399801b083047cc9cf2ad3ed17c3144
# first bad commit: [2d66cccd73436ac9985a08e5c2f82e4344f72264] mm: Prevent 
__alloc_pages_nodemask() RCU CPU stall warnings

Re: next: Commit 'mm: Prevent __alloc_pages_nodemask() RCU CPU stall ...' causing hang on sparc32 qemu

[Guenter Roeck]

Hi Paul,

On Wed, Nov 30, 2016 at 05:19:50PM -0800, Paul E. McKenney wrote:
[ ... ]

> > > > 
> > > > BUG: sleeping function called from invalid context at 
> > > > mm/page_alloc.c:3775
[ ... ]
> 
> Whew!  You had me going for a bit there.  ;-)

Bisect results are here ... the culprit is, again, commit 2d66cccd73 ("mm:
Prevent __alloc_pages_nodemask() RCU CPU stall warnings"), and reverting that
patch fixes the problem. Good that you dropped it already :-).

Guenter

---
# bad: [59ab0083490c8a871b51e893bae5806e55901d7e] Add linux-next specific files 
for 20161130
# good: [e5517c2a5a49ed5e99047008629f1cd60246ea0e] Linux 4.9-rc7
git bisect start 'HEAD' 'v4.9-rc7'
# good: [187f99e5c22bb3fab8b330f3ebbbd235d238f3f8] Merge remote-tracking branch 
'crypto/master'
git bisect good 187f99e5c22bb3fab8b330f3ebbbd235d238f3f8
# good: [36126657c908e822523b8563f9b1512937c0f342] Merge remote-tracking branch 
'tip/auto-latest'
git bisect good 36126657c908e822523b8563f9b1512937c0f342
# good: [2d2139c5c746ec61024fdfa9c36e4e034bb18e59] Merge tag 'iio-for-4.10d' of 
git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio into staging-next
git bisect good 2d2139c5c746ec61024fdfa9c36e4e034bb18e59
# bad: [926a60551123048c589b45abee2a2ec4c924ab21] Merge remote-tracking branch 
'extcon/extcon-next'
git bisect bad 926a60551123048c589b45abee2a2ec4c924ab21
# bad: [1541655795a90720b8a094c8cc39f582dec17398] Merge remote-tracking branch 
'tty/tty-next'
git bisect bad 1541655795a90720b8a094c8cc39f582dec17398
# bad: [69a6720a1e54519d9bf8563764e9e93bf1bd6a84] Merge remote-tracking branch 
'kvm-arm/next'
git bisect bad 69a6720a1e54519d9bf8563764e9e93bf1bd6a84
# good: [33b8b045b93f9104c61ecad1865af961b3bef03e] Merge remote-tracking branch 
'ftrace/for-next'
git bisect good 33b8b045b93f9104c61ecad1865af961b3bef03e
# good: [8370c3d08bd98576d97514eca29970e03767a5d1] kvm: svm: Add 
kvm_fast_pio_in support
git bisect good 8370c3d08bd98576d97514eca29970e03767a5d1
# good: [0a895142323de3eebb0b753d3d8c0e768ff179d9] mm: Prevent shrink_node() 
RCU CPU stall warnings
git bisect good 0a895142323de3eebb0b753d3d8c0e768ff179d9
# bad: [f8045446ca778333e960dcb9e30a5858ff2b8c20] srcu: Force full grace-period 
ordering
git bisect bad f8045446ca778333e960dcb9e30a5858ff2b8c20
# good: [f660d64912ccadadcdce6dfb39eb06924dd93767] doc: Fix RCU requirements 
typos
git bisect good f660d64912ccadadcdce6dfb39eb06924dd93767
# good: [d2db185bfee894c573faebed93461e9938bdbb61] rcu: Remove short-term CPU 
kicking
git bisect good d2db185bfee894c573faebed93461e9938bdbb61
# bad: [2d66cccd73436ac9985a08e5c2f82e4344f72264] mm: Prevent 
__alloc_pages_nodemask() RCU CPU stall warnings
git bisect bad 2d66cccd73436ac9985a08e5c2f82e4344f72264
# good: [34c53f5cd399801b083047cc9cf2ad3ed17c3144] mm: Prevent 
shrink_node_memcg() RCU CPU stall warnings
git bisect good 34c53f5cd399801b083047cc9cf2ad3ed17c3144
# first bad commit: [2d66cccd73436ac9985a08e5c2f82e4344f72264] mm: Prevent 
__alloc_pages_nodemask() RCU CPU stall warnings

Re: [RESEND PATCH 11/11] tools/power turbostat: enable turbostat to support Knights Mill (KNM)

[Len Brown]

Piotr,
Thanks for sending the patch, I've made this change to my turbostat
branch for 4.10.

I did not apply your patch directly because for some reason it didn't
appear in patchwork for linux-pm,
only for lkml, which I do not review.

Also, your patch depended on your style update patch to use the model # macros.
Unfortunately what you did not know was that I'd already applied a
slightly different style update patch.
(and it was my fault that I did not push it upstream before my summer
sabbatical, sorry)

In general, though, a good strategy when mixing style and functionality patches
is to do the functionality first.  The reason is both that style
patches tend to conflict more,
and you don't want them to hold up the functionality.
Also, if your functionality patch does not depend on style,
it is easier to backport to distros who avoid style updates.

Fortunately, this one was trivial.

thanks,
Len Brown, Intel Open Source Technology Center

Re: [RESEND PATCH 11/11] tools/power turbostat: enable turbostat to support Knights Mill (KNM)

[Len Brown]

Piotr,
Thanks for sending the patch, I've made this change to my turbostat
branch for 4.10.

I did not apply your patch directly because for some reason it didn't
appear in patchwork for linux-pm,
only for lkml, which I do not review.

Also, your patch depended on your style update patch to use the model # macros.
Unfortunately what you did not know was that I'd already applied a
slightly different style update patch.
(and it was my fault that I did not push it upstream before my summer
sabbatical, sorry)

In general, though, a good strategy when mixing style and functionality patches
is to do the functionality first.  The reason is both that style
patches tend to conflict more,
and you don't want them to hold up the functionality.
Also, if your functionality patch does not depend on style,
it is easier to backport to distros who avoid style updates.

Fortunately, this one was trivial.

thanks,
Len Brown, Intel Open Source Technology Center

linux-next: Tree for Dec 1

[Stephen Rothwell]

Hi all,

Changes since 20161130:

New tree: openrisc

The cifs tree lost its build failure.

The net-next tree gained conflicts against the net and arm-soc trees.

The block tree gained a build failure for which I applied a merge
fix patch.

The tip tree gained a conflict against the net-next tree.

The akpm-current tree lost its build failure.

The akpm tree lost several patches that turned up in the powerpc tree.

Non-merge commits (relative to Linus' tree): 9134
 8734 files changed, 530030 insertions(+), 199813 deletions(-)



I have created today's linux-next tree at
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
(patches at http://www.kernel.org/pub/linux/kernel/next/ ).  If you
are tracking the linux-next tree using git, you should not use "git pull"
to do so as that will try to merge the new linux-next release with the
old one.  You should use "git fetch" and checkout or reset to the new
master.

You can see which trees have been included by looking in the Next/Trees
file in the source.  There are also quilt-import.log and merge.log
files in the Next directory.  Between each merge, the tree was built
with a ppc64_defconfig for powerpc and an allmodconfig (with
CONFIG_BUILD_DOCSRC=n) for x86_64, a multi_v7_defconfig for arm and a
native build of tools/perf. After the final fixups (if any), I do an
x86_64 modules_install followed by builds for x86_64 allnoconfig,
powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig
(with KALLSYMS_EXTRA_PASS=1) and pseries_le_defconfig and i386, sparc
and sparc64 defconfig.

Below is a summary of the state of the merge.

I am currently merging 246 trees (counting Linus' and 35 trees of bug
fix patches pending for the current merge release).

Stats about the size of the tree over time can be seen at
http://neuling.org/linux-next-size.html .

Status of my local build tests will be at
http://kisskb.ellerman.id.au/linux-next .  If maintainers want to give
advice about cross compilers/configs that work, we are always open to add
more builds.

Thanks to Randy Dunlap for doing many randconfig builds.  And to Paul
Gortmaker for triage and bug fixes.

-- 
Cheers,
Stephen Rothwell

$ git checkout master
$ git reset --hard stable
Merging origin/master (e2b588ab60c7 Merge tag 'pwm/for-4.9' of 
git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm)
Merging fixes/master (30066ce675d3 Merge branch 'linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6)
Merging kbuild-current/rc-fixes (152b695d7437 builddeb: fix cross-building to 
arm64 producing host-arch debs)
Merging arc-current/for-curr (7badf6fefca8 ARC: axs10x: really enable ARC PGU)
Merging arm-current/fixes (8478132a8784 Revert "arm: move exports to 
definitions")
Merging m68k-current/for-linus (7e251bb21ae0 m68k: Fix ndelay() macro)
Merging metag-fixes/fixes (35d04077ad96 metag: Only define 
atomic_dec_if_positive conditionally)
Merging powerpc-fixes/fixes (dd7b2f035ec4 powerpc/mm: Fix lazy icache flush on 
pre-POWER5)
Merging sparc/master (88abd8249ee8 Merge branch 'for-4.9-fixes' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata)
Merging net/master (aa196eed3d80 macvtap: handle ubuf refcount correctly when 
meet errors)
Merging ipsec/master (a55e23864d38 esp6: Fix integrity verification when ESN 
are used)
Merging netfilter/master (17a49cd549d9 netfilter: arp_tables: fix invoking 
32bit "iptable -P INPUT ACCEPT" failed in 64bit kernel)
Merging ipvs/master (9b6c14d51bd2 net: tcp response should set oif only if it 
is L3 master)
Merging wireless-drivers/master (fcd2042e8d36 mwifiex: printk() overflow with 
32-byte SSIDs)
Merging mac80211/master (9590112241ba tipc: fix link statistics counter errors)
Merging sound-current/for-linus (b5337cfe067e ALSA: hda/ca0132 - Add quirk for 
Alienware 15 R2 2016)
Merging pci-current/for-linus (e42010d8207f PCI: Set Read Completion Boundary 
to 128 iff Root Port supports it (_HPX))
Merging driver-core.current/driver-core-linus (a25f0944ba9b Linux 4.9-rc5)
Merging tty.current/tty-linus (a909d3e63699 Linux 4.9-rc3)
Merging usb.current/usb-linus (e5517c2a5a49 Linux 4.9-rc7)
Merging usb-gadget-fixes/fixes (05e78c6933d6 usb: gadget: f_fs: fix wrong 
parenthesis in ffs_func_req_match())
Merging usb-serial-fixes/usb-linus (2ab13292d7a3 USB: serial: cp210x: add ID 
for the Zone DPMX)
Merging usb-chipidea-fixes/ci-for-usb-stable (c7fbb09b2ea1 usb: chipidea: move 
the lock initialization to core file)
Merging phy/fixes (4320f9d4c183 phy: sun4i: check PMU presence when poking 
unknown bit of pmu)
Merging staging.current/staging-linus (a25f0944ba9b Linux 4.9-rc5)
Merging char-misc.current/char-misc-linus (a25f0944ba9b Linux 4.9-rc5)
Merging input-current/for-linus (2425f1808123 Input: change KEY_DATA from 0x275 
to 0x277)
Merging crypto-current/master (57891633eeef crypto: rsa - Add Makefile 
dependen

linux-next: Tree for Dec 1

[Stephen Rothwell]

Hi all,

Changes since 20161130:

New tree: openrisc

The cifs tree lost its build failure.

The net-next tree gained conflicts against the net and arm-soc trees.

The block tree gained a build failure for which I applied a merge
fix patch.

The tip tree gained a conflict against the net-next tree.

The akpm-current tree lost its build failure.

The akpm tree lost several patches that turned up in the powerpc tree.

Non-merge commits (relative to Linus' tree): 9134
 8734 files changed, 530030 insertions(+), 199813 deletions(-)



I have created today's linux-next tree at
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
(patches at http://www.kernel.org/pub/linux/kernel/next/ ).  If you
are tracking the linux-next tree using git, you should not use "git pull"
to do so as that will try to merge the new linux-next release with the
old one.  You should use "git fetch" and checkout or reset to the new
master.

You can see which trees have been included by looking in the Next/Trees
file in the source.  There are also quilt-import.log and merge.log
files in the Next directory.  Between each merge, the tree was built
with a ppc64_defconfig for powerpc and an allmodconfig (with
CONFIG_BUILD_DOCSRC=n) for x86_64, a multi_v7_defconfig for arm and a
native build of tools/perf. After the final fixups (if any), I do an
x86_64 modules_install followed by builds for x86_64 allnoconfig,
powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig
(with KALLSYMS_EXTRA_PASS=1) and pseries_le_defconfig and i386, sparc
and sparc64 defconfig.

Below is a summary of the state of the merge.

I am currently merging 246 trees (counting Linus' and 35 trees of bug
fix patches pending for the current merge release).

Stats about the size of the tree over time can be seen at
http://neuling.org/linux-next-size.html .

Status of my local build tests will be at
http://kisskb.ellerman.id.au/linux-next .  If maintainers want to give
advice about cross compilers/configs that work, we are always open to add
more builds.

Thanks to Randy Dunlap for doing many randconfig builds.  And to Paul
Gortmaker for triage and bug fixes.

-- 
Cheers,
Stephen Rothwell

$ git checkout master
$ git reset --hard stable
Merging origin/master (e2b588ab60c7 Merge tag 'pwm/for-4.9' of 
git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm)
Merging fixes/master (30066ce675d3 Merge branch 'linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6)
Merging kbuild-current/rc-fixes (152b695d7437 builddeb: fix cross-building to 
arm64 producing host-arch debs)
Merging arc-current/for-curr (7badf6fefca8 ARC: axs10x: really enable ARC PGU)
Merging arm-current/fixes (8478132a8784 Revert "arm: move exports to 
definitions")
Merging m68k-current/for-linus (7e251bb21ae0 m68k: Fix ndelay() macro)
Merging metag-fixes/fixes (35d04077ad96 metag: Only define 
atomic_dec_if_positive conditionally)
Merging powerpc-fixes/fixes (dd7b2f035ec4 powerpc/mm: Fix lazy icache flush on 
pre-POWER5)
Merging sparc/master (88abd8249ee8 Merge branch 'for-4.9-fixes' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata)
Merging net/master (aa196eed3d80 macvtap: handle ubuf refcount correctly when 
meet errors)
Merging ipsec/master (a55e23864d38 esp6: Fix integrity verification when ESN 
are used)
Merging netfilter/master (17a49cd549d9 netfilter: arp_tables: fix invoking 
32bit "iptable -P INPUT ACCEPT" failed in 64bit kernel)
Merging ipvs/master (9b6c14d51bd2 net: tcp response should set oif only if it 
is L3 master)
Merging wireless-drivers/master (fcd2042e8d36 mwifiex: printk() overflow with 
32-byte SSIDs)
Merging mac80211/master (9590112241ba tipc: fix link statistics counter errors)
Merging sound-current/for-linus (b5337cfe067e ALSA: hda/ca0132 - Add quirk for 
Alienware 15 R2 2016)
Merging pci-current/for-linus (e42010d8207f PCI: Set Read Completion Boundary 
to 128 iff Root Port supports it (_HPX))
Merging driver-core.current/driver-core-linus (a25f0944ba9b Linux 4.9-rc5)
Merging tty.current/tty-linus (a909d3e63699 Linux 4.9-rc3)
Merging usb.current/usb-linus (e5517c2a5a49 Linux 4.9-rc7)
Merging usb-gadget-fixes/fixes (05e78c6933d6 usb: gadget: f_fs: fix wrong 
parenthesis in ffs_func_req_match())
Merging usb-serial-fixes/usb-linus (2ab13292d7a3 USB: serial: cp210x: add ID 
for the Zone DPMX)
Merging usb-chipidea-fixes/ci-for-usb-stable (c7fbb09b2ea1 usb: chipidea: move 
the lock initialization to core file)
Merging phy/fixes (4320f9d4c183 phy: sun4i: check PMU presence when poking 
unknown bit of pmu)
Merging staging.current/staging-linus (a25f0944ba9b Linux 4.9-rc5)
Merging char-misc.current/char-misc-linus (a25f0944ba9b Linux 4.9-rc5)
Merging input-current/for-linus (2425f1808123 Input: change KEY_DATA from 0x275 
to 0x277)
Merging crypto-current/master (57891633eeef crypto: rsa - Add Makefile 
dependen

Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command

[Lu Baolu]

Hi,

On 12/01/2016 02:04 PM, Baolin Wang wrote:
> Hi Baolu,
>
> On 1 December 2016 at 13:45, Lu Baolu  wrote:
>> Hi,
>>
>> On 11/30/2016 05:02 PM, Baolin Wang wrote:
>>> If the hardware never responds to the stop endpoint command, the
>>> URBs will never be completed, and we might hang the USB subsystem.
>>> The original watchdog timer is used to watch if one stop endpoint
>>> command is timeout, if timeout, then the watchdog timer will set
>>> XHCI_STATE_DYING, try to halt the xHCI host, and give back all
>>> pending URBs.
>>>
>>> But now we already have one command timer to control command timeout,
>>> thus we can also use the command timer to watch the stop endpoint
>>> command, instead of one duplicate watchdog timer which need to be
>>> removed.
>>>
>>> Meanwhile we don't need the 'stop_cmds_pending' flag to identy if
>>> this is the last stop endpoint command of one endpoint. Since we
>>> can make sure we only set one stop endpoint command for one endpoint
>>> by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove
>>> this flag.
>> I am afraid you can't do this. "stop_cmds_pending" was added
>> to fix the problem described in the comments that you want to
>> remove. But I didn't find any fix of this problem in your patch.
> Now we can not pending another stop endpoint command for the same one
> endpoint, since will check 'EP_HALT_PENDING' flag in
> xhci_urb_dequeue() function to avoid this. But after some
> investigation, I think I missed the stop endpoint command in
> xhci_stop_device() which did not check the 'EP_HALT_PENDING' flag,
> maybe need to add 'EP_HALT_PENDING' flag checking in
> xhci_stop_device() function. DId I miss something else? Thanks.

Consider below three threads running on different CPUs at the same time.

Thread A: xhci_handle_cmd_stop_ep()  --- in interrupt handler
Thread B: xhci_stop_endpoint_command_timeout() --- timer expired
Thread C: xhci_urb_dequeue --- called by usb core

They are serialized by xhci->lock. Let's consider below sequence:

Thread A:
- delete xhci->cmd_timer), but will fail due to Thread B.
- clear EP_HALT_PENDING bit

Thread B:
- halt the host controller

Thread C:
- set EP_HALT_PENDING bit
- enqueue another stop endpoint command
- add the timer back

Best regards,
Lu Baolu

>
>> - * The timer may also fire if the host takes a very long time to respond to 
>> the
>> - * command, and the stop endpoint command completion handler cannot delete 
>> the
>> - * timer before the timer function is called.  Another endpoint 
>> cancellation may
>> - * sneak in before the timer function can grab the lock, and that may queue
>> - * another stop endpoint command and add the timer back.  So we cannot use a
>> - * simple flag to say whether there is a pending stop endpoint command for a
>> - * particular endpoint.
>> - *
>> - * Instead we use a combination of that flag and a counter for the number of
>> - * pending stop endpoint commands.  If the timer is the tail end of the last
>> - * stop endpoint command, and the endpoint's command is still pending, we 
>> assume
>> - * the host is dying.
>>
>> Best regards,
>> Lu Baolu
>>
>>> We also need to clean up the command queue before trying to halt the
>>> xHCI host in xhci_stop_endpoint_command_timeout() function.
>>>
>>> Signed-off-by: Baolin Wang 
>
>

Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command

[Lu Baolu]

Hi,

On 12/01/2016 02:04 PM, Baolin Wang wrote:
> Hi Baolu,
>
> On 1 December 2016 at 13:45, Lu Baolu  wrote:
>> Hi,
>>
>> On 11/30/2016 05:02 PM, Baolin Wang wrote:
>>> If the hardware never responds to the stop endpoint command, the
>>> URBs will never be completed, and we might hang the USB subsystem.
>>> The original watchdog timer is used to watch if one stop endpoint
>>> command is timeout, if timeout, then the watchdog timer will set
>>> XHCI_STATE_DYING, try to halt the xHCI host, and give back all
>>> pending URBs.
>>>
>>> But now we already have one command timer to control command timeout,
>>> thus we can also use the command timer to watch the stop endpoint
>>> command, instead of one duplicate watchdog timer which need to be
>>> removed.
>>>
>>> Meanwhile we don't need the 'stop_cmds_pending' flag to identy if
>>> this is the last stop endpoint command of one endpoint. Since we
>>> can make sure we only set one stop endpoint command for one endpoint
>>> by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove
>>> this flag.
>> I am afraid you can't do this. "stop_cmds_pending" was added
>> to fix the problem described in the comments that you want to
>> remove. But I didn't find any fix of this problem in your patch.
> Now we can not pending another stop endpoint command for the same one
> endpoint, since will check 'EP_HALT_PENDING' flag in
> xhci_urb_dequeue() function to avoid this. But after some
> investigation, I think I missed the stop endpoint command in
> xhci_stop_device() which did not check the 'EP_HALT_PENDING' flag,
> maybe need to add 'EP_HALT_PENDING' flag checking in
> xhci_stop_device() function. DId I miss something else? Thanks.

Consider below three threads running on different CPUs at the same time.

Thread A: xhci_handle_cmd_stop_ep()  --- in interrupt handler
Thread B: xhci_stop_endpoint_command_timeout() --- timer expired
Thread C: xhci_urb_dequeue --- called by usb core

They are serialized by xhci->lock. Let's consider below sequence:

Thread A:
- delete xhci->cmd_timer), but will fail due to Thread B.
- clear EP_HALT_PENDING bit

Thread B:
- halt the host controller

Thread C:
- set EP_HALT_PENDING bit
- enqueue another stop endpoint command
- add the timer back

Best regards,
Lu Baolu

>
>> - * The timer may also fire if the host takes a very long time to respond to 
>> the
>> - * command, and the stop endpoint command completion handler cannot delete 
>> the
>> - * timer before the timer function is called.  Another endpoint 
>> cancellation may
>> - * sneak in before the timer function can grab the lock, and that may queue
>> - * another stop endpoint command and add the timer back.  So we cannot use a
>> - * simple flag to say whether there is a pending stop endpoint command for a
>> - * particular endpoint.
>> - *
>> - * Instead we use a combination of that flag and a counter for the number of
>> - * pending stop endpoint commands.  If the timer is the tail end of the last
>> - * stop endpoint command, and the endpoint's command is still pending, we 
>> assume
>> - * the host is dying.
>>
>> Best regards,
>> Lu Baolu
>>
>>> We also need to clean up the command queue before trying to halt the
>>> xHCI host in xhci_stop_endpoint_command_timeout() function.
>>>
>>> Signed-off-by: Baolin Wang 
>
>

Re: Re: [PATCH] ipv6:ip6_xmit remove unnecessary np NULL check

[Eric Dumazet]

On Thu, 2016-12-01 at 06:14 +, Rohit Thapliyal wrote:
> Hi,
> 
Hi,

Do not top post on netdev, and do not use HTML format : it wont reach
netdev.

 
> 
> Found at just one place in ping_v6_sendmsg, where np is checked for
> NULL.
> 
> And I am not sure, if it is really required there also. 
> 
> So, I am not sure if sk_fullsock will ever return NULL in inet6_sk.
> 
Well I am sure. You can trust me. You can read git history for the
details.
> 
> sk_fullsock(__sk) ? inet_sk(__sk)->pinet6 : NULL;
> 


Yes, Lorenzo mentioned this ping_v6_sendmsg() useless code, but
apparently never sent the cleanup. This is really minor you know...

diff --git a/net/ipv6/ping.c b/net/ipv6/ping.c
index 
66e2d9dfc43a87ebed092d024e5bf2752b755d0e..775a9365725dd400c36e2510c685d08bf4a7d4b0
 100644
--- a/net/ipv6/ping.c
+++ b/net/ipv6/ping.c
@@ -125,12 +125,6 @@ static int ping_v6_sendmsg(struct sock *sk, struct msghdr 
*msg, size_t len)
return PTR_ERR(dst);
rt = (struct rt6_info *) dst;
 
-   np = inet6_sk(sk);
-   if (!np) {
-   err = -EBADF;
-   goto dst_err_out;
-   }
-
if (!fl6.flowi6_oif && ipv6_addr_is_multicast())
fl6.flowi6_oif = np->mcast_oif;
else if (!fl6.flowi6_oif)
@@ -165,7 +159,6 @@ static int ping_v6_sendmsg(struct sock *sk, struct msghdr 
*msg, size_t len)
}
release_sock(sk);
 
-dst_err_out:
dst_release(dst);
 
if (err)




> 
> Thanks, 
> 
> Rohit Thapliyal
> 
>  
> 
> - Original Message -
> 
> Sender : Eric Dumazet 
> 
> Date : 2016-11-30 00:26 (GMT+9)
> 
> Title : Re: [PATCH] ipv6:ip6_xmit remove unnecessary np NULL check
> 
>  
> 
> On Tue, 2016-11-29 at 12:02 +0530, Manjeet Pawar wrote:
> > From: Rohit Thapliyal 
> > 
> > np NULL check doesn't seem required here as it shall never
> > be NULL anyways in inet6_sk(sk).
> > 
> > Signed-off-by: Rohit Thapliyal 
> > Signed-off-by: Manjeet Pawar 
> > Signed-off-by: David Miller 
> > Reviewed-by: Akhilesh Kumar 
> > 
> > ---
> > v2->v3: Modified as per the suggestion from David Miller
> > ip6_xmit calls are made without checking NULL np
> > pointer, so no need to explicitly check NULL np in
> > ip6_xmit.
> > 
> >  include/linux/ipv6.h  | 2 +-
> >  net/ipv6/ip6_output.c | 3 +--
> >  2 files changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> > index a064997..6c9c604 100644
> > --- a/include/linux/ipv6.h
> > +++ b/include/linux/ipv6.h
> > @@ -299,7 +299,7 @@ struct tcp6_timewait_sock {
> >  
> >  static inline struct ipv6_pinfo *inet6_sk(const struct sock *__sk)
> >  {
> > -   return sk_fullsock(__sk) ? inet_sk(__sk)->pinet6 : NULL;
> > +   return inet_sk(__sk)->pinet6;
> 
> 
> David suggestion was about np being NULL or not in ip6_xmit()
> 
> But have you checked inet6_sk() was never called for a TCPv6 TIMEWAIT or
> SYN_RECV request ?
> 
> >  }
> >  
> >  static inline struct raw6_sock *raw6_sk(const struct sock *sk)
> > diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> > index 59eb4ed..f8c63ec 100644
> > --- a/net/ipv6/ip6_output.c
> > +++ b/net/ipv6/ip6_output.c
> > @@ -213,8 +213,7 @@ int ip6_xmit(const struct sock *sk, struct sk_buff 
> > *skb, struct flowi6 *fl6,
> > /*
> >  *  Fill in the IPv6 header
> >  */
> > -   if (np)
> > -   hlimit = np->hop_limit;
> > +   hlimit = np->hop_limit;
> > if (hlimit < 0)
> > hlimit = ip6_dst_hoplimit(dst);
> >  
> 
> This part is fine.
> 
> 
> 
> 
> --
> 
> Thanks and Regards,
> 
> Rohit Thapliyal
> 
> Lead Engineer
> 
> Samsung SRI-D
> Phone: +91-9717627969
> 
> 
> 
> 
> 
> 

Re: Re: [PATCH] ipv6:ip6_xmit remove unnecessary np NULL check

[Eric Dumazet]

On Thu, 2016-12-01 at 06:14 +, Rohit Thapliyal wrote:
> Hi,
> 
Hi,

Do not top post on netdev, and do not use HTML format : it wont reach
netdev.

 
> 
> Found at just one place in ping_v6_sendmsg, where np is checked for
> NULL.
> 
> And I am not sure, if it is really required there also. 
> 
> So, I am not sure if sk_fullsock will ever return NULL in inet6_sk.
> 
Well I am sure. You can trust me. You can read git history for the
details.
> 
> sk_fullsock(__sk) ? inet_sk(__sk)->pinet6 : NULL;
> 


Yes, Lorenzo mentioned this ping_v6_sendmsg() useless code, but
apparently never sent the cleanup. This is really minor you know...

diff --git a/net/ipv6/ping.c b/net/ipv6/ping.c
index 
66e2d9dfc43a87ebed092d024e5bf2752b755d0e..775a9365725dd400c36e2510c685d08bf4a7d4b0
 100644
--- a/net/ipv6/ping.c
+++ b/net/ipv6/ping.c
@@ -125,12 +125,6 @@ static int ping_v6_sendmsg(struct sock *sk, struct msghdr 
*msg, size_t len)
return PTR_ERR(dst);
rt = (struct rt6_info *) dst;
 
-   np = inet6_sk(sk);
-   if (!np) {
-   err = -EBADF;
-   goto dst_err_out;
-   }
-
if (!fl6.flowi6_oif && ipv6_addr_is_multicast())
fl6.flowi6_oif = np->mcast_oif;
else if (!fl6.flowi6_oif)
@@ -165,7 +159,6 @@ static int ping_v6_sendmsg(struct sock *sk, struct msghdr 
*msg, size_t len)
}
release_sock(sk);
 
-dst_err_out:
dst_release(dst);
 
if (err)




> 
> Thanks, 
> 
> Rohit Thapliyal
> 
>  
> 
> - Original Message -
> 
> Sender : Eric Dumazet 
> 
> Date : 2016-11-30 00:26 (GMT+9)
> 
> Title : Re: [PATCH] ipv6:ip6_xmit remove unnecessary np NULL check
> 
>  
> 
> On Tue, 2016-11-29 at 12:02 +0530, Manjeet Pawar wrote:
> > From: Rohit Thapliyal 
> > 
> > np NULL check doesn't seem required here as it shall never
> > be NULL anyways in inet6_sk(sk).
> > 
> > Signed-off-by: Rohit Thapliyal 
> > Signed-off-by: Manjeet Pawar 
> > Signed-off-by: David Miller 
> > Reviewed-by: Akhilesh Kumar 
> > 
> > ---
> > v2->v3: Modified as per the suggestion from David Miller
> > ip6_xmit calls are made without checking NULL np
> > pointer, so no need to explicitly check NULL np in
> > ip6_xmit.
> > 
> >  include/linux/ipv6.h  | 2 +-
> >  net/ipv6/ip6_output.c | 3 +--
> >  2 files changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> > index a064997..6c9c604 100644
> > --- a/include/linux/ipv6.h
> > +++ b/include/linux/ipv6.h
> > @@ -299,7 +299,7 @@ struct tcp6_timewait_sock {
> >  
> >  static inline struct ipv6_pinfo *inet6_sk(const struct sock *__sk)
> >  {
> > -   return sk_fullsock(__sk) ? inet_sk(__sk)->pinet6 : NULL;
> > +   return inet_sk(__sk)->pinet6;
> 
> 
> David suggestion was about np being NULL or not in ip6_xmit()
> 
> But have you checked inet6_sk() was never called for a TCPv6 TIMEWAIT or
> SYN_RECV request ?
> 
> >  }
> >  
> >  static inline struct raw6_sock *raw6_sk(const struct sock *sk)
> > diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> > index 59eb4ed..f8c63ec 100644
> > --- a/net/ipv6/ip6_output.c
> > +++ b/net/ipv6/ip6_output.c
> > @@ -213,8 +213,7 @@ int ip6_xmit(const struct sock *sk, struct sk_buff 
> > *skb, struct flowi6 *fl6,
> > /*
> >  *  Fill in the IPv6 header
> >  */
> > -   if (np)
> > -   hlimit = np->hop_limit;
> > +   hlimit = np->hop_limit;
> > if (hlimit < 0)
> > hlimit = ip6_dst_hoplimit(dst);
> >  
> 
> This part is fine.
> 
> 
> 
> 
> --
> 
> Thanks and Regards,
> 
> Rohit Thapliyal
> 
> Lead Engineer
> 
> Samsung SRI-D
> Phone: +91-9717627969
> 
> 
> 
> 
> 
> 

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

[Steve French]

Commit fixed to handle the ifdef CONFIG_CIFS_SMB2 disabled problem you
noted, and repushed to my for-next branch. Thx for pointing this out.

On Tue, Nov 29, 2016 at 4:27 PM, Stephen Rothwell  wrote:
> Hi all,
>
> After merging the cifs tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
>
> fs/cifs/connect.c: In function 'cifs_find_tcon':
> fs/cifs/connect.c:2610:11: error: 'struct cifs_tcon' has no member named 
> 'snapshot_time'
>if (tcon->snapshot_time != volume_info->snapshot_time)
>^
> fs/cifs/connect.c: In function 'cifs_get_tcon':
> fs/cifs/connect.c:2681:8: error: 'struct cifs_tcon' has no member named 
> 'snapshot_time'
> tcon->snapshot_time = volume_info->snapshot_time;
> ^
>
> Caused by commit
>
>   36c659cf9241 ("SMB3: parsing for new snapshot timestamp mount parm")
>
> CONFIG_CIFS_SMB2 is not set for this build ...
>
> I have used the cifs tree from next-20161129 for today.
>
> --
> Cheers,
> Stephen Rothwell



-- 
Thanks,

Steve

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

[Steve French]

Commit fixed to handle the ifdef CONFIG_CIFS_SMB2 disabled problem you
noted, and repushed to my for-next branch. Thx for pointing this out.

On Tue, Nov 29, 2016 at 4:27 PM, Stephen Rothwell  wrote:
> Hi all,
>
> After merging the cifs tree, today's linux-next build (powerpc
> ppc64_defconfig) failed like this:
>
> fs/cifs/connect.c: In function 'cifs_find_tcon':
> fs/cifs/connect.c:2610:11: error: 'struct cifs_tcon' has no member named 
> 'snapshot_time'
>if (tcon->snapshot_time != volume_info->snapshot_time)
>^
> fs/cifs/connect.c: In function 'cifs_get_tcon':
> fs/cifs/connect.c:2681:8: error: 'struct cifs_tcon' has no member named 
> 'snapshot_time'
> tcon->snapshot_time = volume_info->snapshot_time;
> ^
>
> Caused by commit
>
>   36c659cf9241 ("SMB3: parsing for new snapshot timestamp mount parm")
>
> CONFIG_CIFS_SMB2 is not set for this build ...
>
> I have used the cifs tree from next-20161129 for today.
>
> --
> Cheers,
> Stephen Rothwell



-- 
Thanks,

Steve

[PATCH 1/1] clk: clk-wm831x: fix a logic error

[Pan Bian]

Fix bug https://bugzilla.kernel.org/show_bug.cgi?id=188561. Function
wm831x_clkout_is_prepared() returns "true" when it fails to read
CLOCK_CONTROL_1. "true" means the device is already prepared. So
return "true" on the read failure seems improper.

Signed-off-by: Pan Bian 
---
 drivers/clk/clk-wm831x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c
index f4fdac5..0621fbf 100644
--- a/drivers/clk/clk-wm831x.c
+++ b/drivers/clk/clk-wm831x.c
@@ -243,7 +243,7 @@ static int wm831x_clkout_is_prepared(struct clk_hw *hw)
if (ret < 0) {
dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_1: %d\n",
ret);
-   return true;
+   return false;
}
 
return (ret & WM831X_CLKOUT_ENA) != 0;
-- 
1.9.1

[PATCH 1/1] clk: clk-wm831x: fix a logic error

[Pan Bian]

Fix bug https://bugzilla.kernel.org/show_bug.cgi?id=188561. Function
wm831x_clkout_is_prepared() returns "true" when it fails to read
CLOCK_CONTROL_1. "true" means the device is already prepared. So
return "true" on the read failure seems improper.

Signed-off-by: Pan Bian 
---
 drivers/clk/clk-wm831x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c
index f4fdac5..0621fbf 100644
--- a/drivers/clk/clk-wm831x.c
+++ b/drivers/clk/clk-wm831x.c
@@ -243,7 +243,7 @@ static int wm831x_clkout_is_prepared(struct clk_hw *hw)
if (ret < 0) {
dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_1: %d\n",
ret);
-   return true;
+   return false;
}
 
return (ret & WM831X_CLKOUT_ENA) != 0;
-- 
1.9.1

[PATCH 1/1] bluetooth: propagate the error code

[Pan Bian]

Fix bug https://bugzilla.kernel.org/show_bug.cgi?id=188551. A negative
return value means there are errors, while 0 indicates success. However,
in function bpa10x_send_frame(), it returns 0 no matter whether
usb_submit_urb() returns a negative value. As a result, the caller of
bpa10x_send_frame() may not detect the submission error. In this patch,
use "return err;" at the end to propagate the error.

Signed-off-by: Pan Bian 
---
 drivers/bluetooth/bpa10x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
index a9932fe..d2bebee 100644
--- a/drivers/bluetooth/bpa10x.c
+++ b/drivers/bluetooth/bpa10x.c
@@ -355,7 +355,7 @@ static int bpa10x_send_frame(struct hci_dev *hdev, struct 
sk_buff *skb)
 
usb_free_urb(urb);
 
-   return 0;
+   return err;
 }
 
 static int bpa10x_set_diag(struct hci_dev *hdev, bool enable)
-- 
1.9.1

[PATCH 1/1] bluetooth: propagate the error code

[Pan Bian]

Fix bug https://bugzilla.kernel.org/show_bug.cgi?id=188551. A negative
return value means there are errors, while 0 indicates success. However,
in function bpa10x_send_frame(), it returns 0 no matter whether
usb_submit_urb() returns a negative value. As a result, the caller of
bpa10x_send_frame() may not detect the submission error. In this patch,
use "return err;" at the end to propagate the error.

Signed-off-by: Pan Bian 
---
 drivers/bluetooth/bpa10x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
index a9932fe..d2bebee 100644
--- a/drivers/bluetooth/bpa10x.c
+++ b/drivers/bluetooth/bpa10x.c
@@ -355,7 +355,7 @@ static int bpa10x_send_frame(struct hci_dev *hdev, struct 
sk_buff *skb)
 
usb_free_urb(urb);
 
-   return 0;
+   return err;
 }
 
 static int bpa10x_set_diag(struct hci_dev *hdev, bool enable)
-- 
1.9.1

Re: drm/radeon spamming alloc_contig_range: [xxx, yyy) PFNs busy busy

[Robin H. Johnson]

On Wed, Nov 30, 2016 at 10:24:59PM +0100, Vlastimil Babka wrote:
> [add more CC's]
> 
> On 11/30/2016 09:19 PM, Robin H. Johnson wrote:
> > Somewhere in the Radeon/DRM codebase, CMA page allocation has either
> > regressed in the timeline of 4.5->4.9, and/or the drm/radeon code is
> > doing something different with pages.
> 
> Could be that it didn't use dma_generic_alloc_coherent() before, or you 
> didn't 
> have the generic CMA pool configured.
v4.9-rc7-23-gded6e842cf49:
[0.00] cma: Reserved 16 MiB at 0x00083e40
[0.00] Memory: 32883108K/33519432K available (6752K kernel code, 1244K
rwdata, 4716K rodata, 1772K init, 2720K bss, 619940K reserved, 16384K
cma-reserved)

> What's the output of "grep CMA" on your 
> .config?

# grep CMA .config |grep -v -e SECMARK= -e CONFIG_BCMA -e CONFIG_USB_HCD_BCMA 
-e INPUT_CMA3000 -e CRYPTO_CMAC
CONFIG_CMA=y
# CONFIG_CMA_DEBUG is not set
# CONFIG_CMA_DEBUGFS is not set
CONFIG_CMA_AREAS=7
CONFIG_DMA_CMA=y
CONFIG_CMA_SIZE_MBYTES=16
CONFIG_CMA_SIZE_SEL_MBYTES=y
# CONFIG_CMA_SIZE_SEL_PERCENTAGE is not set
# CONFIG_CMA_SIZE_SEL_MIN is not set
# CONFIG_CMA_SIZE_SEL_MAX is not set
CONFIG_CMA_ALIGNMENT=8

> Or any kernel boot options with cma in name? 
None.


> By default config this should not be used on x86.
What do you mean by that statement? 
It should be disallowed to enable CONFIG_CMA? Radeon and CMA should be
mutually exclusive?

> > Given that I haven't seen ANY other reports of this, I'm inclined to
> > believe the problem is drm/radeon specific (if I don't start X, I can't
> > reproduce the problem).
> 
> It's rather CMA specific, the allocation attemps just can't be 100% reliable 
> due 
> to how CMA works. The question is if it should be spewing in the log in the 
> context of dma-cma, which has a fallback allocation option. It even uses 
> __GFP_NOWARN, perhaps the CMA path should respect that?
Yes, I'd say if there's a fallback without much penalty, nowarn makes
sense. If the fallback just tries multiple addresses until success, then
the warning should only be issued when too many attempts have been made.

> 
> > The rate of the problem starts slow, and also is relatively low on an idle
> > system (my screens blank at night, no xscreensaver running), but it still 
> > ramps
> > up over time (to the point of generating 2.5GB/hour of "(timestamp)
> > alloc_contig_range: [83e4d9, 83e4da) PFNs busy"), with various addresses 
> > (~100
> > unique ranges for a day).
> >
> > My X workload is ~50 chrome tabs and ~20 terminals (over 3x 24" monitors w/ 
> > 9
> > virtual desktops per monitor).
> So IIUC, except the messages, everything actually works fine?
There's high kernel CPU usage that seems to roughly correlate with the
messages, but I can't yet tell if that's due to the syslog itself, or
repeated alloc_contig_range requests.

-- 
Robin Hugh Johnson
Gentoo Linux: Dev, Infra Lead, Foundation Trustee & Treasurer
E-Mail   : robb...@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136


signature.asc
Description: Digital signature

Re: drm/radeon spamming alloc_contig_range: [xxx, yyy) PFNs busy busy

[Robin H. Johnson]

On Wed, Nov 30, 2016 at 10:24:59PM +0100, Vlastimil Babka wrote:
> [add more CC's]
> 
> On 11/30/2016 09:19 PM, Robin H. Johnson wrote:
> > Somewhere in the Radeon/DRM codebase, CMA page allocation has either
> > regressed in the timeline of 4.5->4.9, and/or the drm/radeon code is
> > doing something different with pages.
> 
> Could be that it didn't use dma_generic_alloc_coherent() before, or you 
> didn't 
> have the generic CMA pool configured.
v4.9-rc7-23-gded6e842cf49:
[0.00] cma: Reserved 16 MiB at 0x00083e40
[0.00] Memory: 32883108K/33519432K available (6752K kernel code, 1244K
rwdata, 4716K rodata, 1772K init, 2720K bss, 619940K reserved, 16384K
cma-reserved)

> What's the output of "grep CMA" on your 
> .config?

# grep CMA .config |grep -v -e SECMARK= -e CONFIG_BCMA -e CONFIG_USB_HCD_BCMA 
-e INPUT_CMA3000 -e CRYPTO_CMAC
CONFIG_CMA=y
# CONFIG_CMA_DEBUG is not set
# CONFIG_CMA_DEBUGFS is not set
CONFIG_CMA_AREAS=7
CONFIG_DMA_CMA=y
CONFIG_CMA_SIZE_MBYTES=16
CONFIG_CMA_SIZE_SEL_MBYTES=y
# CONFIG_CMA_SIZE_SEL_PERCENTAGE is not set
# CONFIG_CMA_SIZE_SEL_MIN is not set
# CONFIG_CMA_SIZE_SEL_MAX is not set
CONFIG_CMA_ALIGNMENT=8

> Or any kernel boot options with cma in name? 
None.


> By default config this should not be used on x86.
What do you mean by that statement? 
It should be disallowed to enable CONFIG_CMA? Radeon and CMA should be
mutually exclusive?

> > Given that I haven't seen ANY other reports of this, I'm inclined to
> > believe the problem is drm/radeon specific (if I don't start X, I can't
> > reproduce the problem).
> 
> It's rather CMA specific, the allocation attemps just can't be 100% reliable 
> due 
> to how CMA works. The question is if it should be spewing in the log in the 
> context of dma-cma, which has a fallback allocation option. It even uses 
> __GFP_NOWARN, perhaps the CMA path should respect that?
Yes, I'd say if there's a fallback without much penalty, nowarn makes
sense. If the fallback just tries multiple addresses until success, then
the warning should only be issued when too many attempts have been made.

> 
> > The rate of the problem starts slow, and also is relatively low on an idle
> > system (my screens blank at night, no xscreensaver running), but it still 
> > ramps
> > up over time (to the point of generating 2.5GB/hour of "(timestamp)
> > alloc_contig_range: [83e4d9, 83e4da) PFNs busy"), with various addresses 
> > (~100
> > unique ranges for a day).
> >
> > My X workload is ~50 chrome tabs and ~20 terminals (over 3x 24" monitors w/ 
> > 9
> > virtual desktops per monitor).
> So IIUC, except the messages, everything actually works fine?
There's high kernel CPU usage that seems to roughly correlate with the
messages, but I can't yet tell if that's due to the syslog itself, or
repeated alloc_contig_range requests.

-- 
Robin Hugh Johnson
Gentoo Linux: Dev, Infra Lead, Foundation Trustee & Treasurer
E-Mail   : robb...@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136


signature.asc
Description: Digital signature

[PATCH 06/10] via-cuda: Avoid TREQ race condition

[Finn Thain]

When a read transaction completes, one of several things will happen:
either a new transfer is started by the driver, a new transfer request
is raised by the Cuda (i.e. TREQ asserted), or both happen at once.

When both happen at once, there is a race condition between the TREQ test
in the read_done state and the same test in cuda_start(). Moreover, the
former test uses a stale TREQ value.

Theoretically, this can result in the undesirable outcome that the
interrupt handler completes with the state machine 'idle' when it should
instead start the next transaction.

Avoid this race by calling cuda_start() first and then confirming that it
succeeded. If not, test the current TREQ value before entering the
'reading' state.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index e65c0b6..ff9062a 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -605,12 +605,11 @@ cuda_interrupt(int irq, void *arg)
memcpy(ibuf, cuda_rbuf, ibuf_len);
}
reply_ptr = cuda_rbuf;
-   if (TREQ_asserted(status)) {
+   cuda_state = idle;
+   cuda_start();
+   if (cuda_state == idle && TREQ_asserted(in_8([B]))) {
assert_TIP();
cuda_state = reading;
-   } else {
-   cuda_state = idle;
-   cuda_start();
}
break;
 
-- 
2.7.3

[PATCH 05/10] via-cuda: Fix re-initialization of reply_ptr and reading_reply

[Finn Thain]

When reading_reply is set, reply_ptr points into an adb_request struct.

Hence, when reply_ptr instead points into the global cuda_rbuf,
it must be the case that reading_reply is not set.

Unfortunately, this rule can be violated because re-initialization
of reply_ptr and reading_reply presently depends on the TREQ input.

Fix this by re-initializing reply_ptr and reading_reply as soon as they
are known to be invalid.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index acf3a95..e65c0b6 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -592,6 +592,7 @@ cuda_interrupt(int irq, void *arg)
}
current_req = req->next;
complete = 1;
+   reading_reply = 0;
} else {
/* This is tricky. We must break the spinlock to call
 * cuda_input. However, doing so means we might get
@@ -603,11 +604,10 @@ cuda_interrupt(int irq, void *arg)
ibuf_len = reply_ptr - cuda_rbuf;
memcpy(ibuf, cuda_rbuf, ibuf_len);
}
+   reply_ptr = cuda_rbuf;
if (TREQ_asserted(status)) {
assert_TIP();
cuda_state = reading;
-   reply_ptr = cuda_rbuf;
-   reading_reply = 0;
} else {
cuda_state = idle;
cuda_start();
-- 
2.7.3

[PATCH 06/10] via-cuda: Avoid TREQ race condition

[Finn Thain]

When a read transaction completes, one of several things will happen:
either a new transfer is started by the driver, a new transfer request
is raised by the Cuda (i.e. TREQ asserted), or both happen at once.

When both happen at once, there is a race condition between the TREQ test
in the read_done state and the same test in cuda_start(). Moreover, the
former test uses a stale TREQ value.

Theoretically, this can result in the undesirable outcome that the
interrupt handler completes with the state machine 'idle' when it should
instead start the next transaction.

Avoid this race by calling cuda_start() first and then confirming that it
succeeded. If not, test the current TREQ value before entering the
'reading' state.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index e65c0b6..ff9062a 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -605,12 +605,11 @@ cuda_interrupt(int irq, void *arg)
memcpy(ibuf, cuda_rbuf, ibuf_len);
}
reply_ptr = cuda_rbuf;
-   if (TREQ_asserted(status)) {
+   cuda_state = idle;
+   cuda_start();
+   if (cuda_state == idle && TREQ_asserted(in_8([B]))) {
assert_TIP();
cuda_state = reading;
-   } else {
-   cuda_state = idle;
-   cuda_start();
}
break;
 
-- 
2.7.3

[PATCH 05/10] via-cuda: Fix re-initialization of reply_ptr and reading_reply

[Finn Thain]

When reading_reply is set, reply_ptr points into an adb_request struct.

Hence, when reply_ptr instead points into the global cuda_rbuf,
it must be the case that reading_reply is not set.

Unfortunately, this rule can be violated because re-initialization
of reply_ptr and reading_reply presently depends on the TREQ input.

Fix this by re-initializing reply_ptr and reading_reply as soon as they
are known to be invalid.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index acf3a95..e65c0b6 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -592,6 +592,7 @@ cuda_interrupt(int irq, void *arg)
}
current_req = req->next;
complete = 1;
+   reading_reply = 0;
} else {
/* This is tricky. We must break the spinlock to call
 * cuda_input. However, doing so means we might get
@@ -603,11 +604,10 @@ cuda_interrupt(int irq, void *arg)
ibuf_len = reply_ptr - cuda_rbuf;
memcpy(ibuf, cuda_rbuf, ibuf_len);
}
+   reply_ptr = cuda_rbuf;
if (TREQ_asserted(status)) {
assert_TIP();
cuda_state = reading;
-   reply_ptr = cuda_rbuf;
-   reading_reply = 0;
} else {
cuda_state = idle;
cuda_start();
-- 
2.7.3

[PATCH 09/10] via-cuda: Add support for Egret system controller

[Finn Thain]

The Egret system controller was the predecessor to the Cuda and the
differences are minor.

On Cuda, byte acknowledgement requires one transition of the TACK
signal; on Egret two are needed. On Cuda, TIP is active low; on Egret
it is active high. And Cuda raises certain interrupts that Egret omits.

Accomodating these differences complicates the Cuda driver slightly
but avoids a lot of duplication (see next patch).

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 155 +--
 1 file changed, 134 insertions(+), 21 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index f1b2c5b..3254070 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -1,10 +1,10 @@
 /*
- * Device driver for the via-cuda on Apple Powermacs.
+ * Device driver for the Cuda and Egret system controllers found on PowerMacs
+ * and 68k Macs.
  *
- * The VIA (versatile interface adapter) interfaces to the CUDA,
- * a 6805 microprocessor core which controls the ADB (Apple Desktop
- * Bus) which connects to the keyboard and mouse.  The CUDA also
- * controls system power and the RTC (real time clock) chip.
+ * The Cuda or Egret is a 6805 microcontroller interfaced to the 6522 VIA.
+ * This MCU controls system power, Parameter RAM, Real Time Clock and the
+ * Apple Desktop Bus (ADB) that connects to the keyboard and mouse.
  *
  * Copyright (C) 1996 Paul Mackerras.
  */
@@ -50,10 +50,27 @@ static DEFINE_SPINLOCK(cuda_lock);
 #define IER(14*RS) /* Interrupt enable register */
 #define ANH(15*RS) /* A-side data, no handshake */
 
-/* Bits in B data register: all active low */
-#define TREQ   0x08/* Transfer request (input) */
-#define TACK   0x10/* Transfer acknowledge (output) */
-#define TIP0x20/* Transfer in progress (output) */
+/*
+ * When the Cuda design replaced the Egret, some signal names and
+ * logic sense changed. They all serve the same purposes, however.
+ *
+ *   VIA pin   |  Egret pin
+ * +--
+ *   PB3 (input)   |  Transceiver session   (active low)
+ *   PB4 (output)  |  VIA full  (active high)
+ *   PB5 (output)  |  System session(active high)
+ *
+ *   VIA pin   |  Cuda pin
+ * +--
+ *   PB3 (input)   |  Transfer request  (active low)
+ *   PB4 (output)  |  Byte acknowledge  (active low)
+ *   PB5 (output)  |  Transfer in progress  (active low)
+ */
+
+/* Bits in Port B data register */
+#define TREQ   0x08/* Transfer request */
+#define TACK   0x10/* Transfer acknowledge */
+#define TIP0x20/* Transfer in progress */
 
 /* Bits in ACR */
 #define SR_CTRL0x1c/* Shift register control bits 
*/
@@ -65,6 +82,19 @@ static DEFINE_SPINLOCK(cuda_lock);
 #define IER_CLR0   /* clear bits in IER */
 #define SR_INT 0x04/* Shift register full/empty */
 
+/* Duration of byte acknowledgement pulse (us) */
+#define EGRET_TACK_ASSERTED_DELAY  300
+#define EGRET_TACK_NEGATED_DELAY   400
+
+/* Interval from interrupt to start of session (us) */
+#define EGRET_SESSION_DELAY450
+
+#ifdef CONFIG_PPC
+#define mcu_is_egret   false
+#else
+static bool mcu_is_egret;
+#endif
+
 static inline bool TREQ_asserted(u8 portb)
 {
return !(portb & TREQ);
@@ -72,12 +102,29 @@ static inline bool TREQ_asserted(u8 portb)
 
 static inline void assert_TIP(void)
 {
-   out_8([B], in_8([B]) & ~TIP);
+   if (mcu_is_egret) {
+   udelay(EGRET_SESSION_DELAY);
+   out_8([B], in_8([B]) | TIP);
+   } else
+   out_8([B], in_8([B]) & ~TIP);
+}
+
+static inline void assert_TIP_and_TACK(void)
+{
+   if (mcu_is_egret) {
+   udelay(EGRET_SESSION_DELAY);
+   out_8([B], in_8([B]) | TIP | TACK);
+   } else
+   out_8([B], in_8([B]) & ~(TIP | TACK));
 }
 
 static inline void assert_TACK(void)
 {
-   out_8([B], in_8([B]) & ~TACK);
+   if (mcu_is_egret) {
+   udelay(EGRET_TACK_NEGATED_DELAY);
+   out_8([B], in_8([B]) | TACK);
+   } else
+   out_8([B], in_8([B]) & ~TACK);
 }
 
 static inline void toggle_TACK(void)
@@ -87,12 +134,20 @@ static inline void toggle_TACK(void)
 
 static inline void negate_TACK(void)
 {
-   out_8([B], in_8([B]) | TACK);
+   if (mcu_is_egret) {
+   udelay(EGRET_TACK_ASSERTED_DELAY);
+   out_8([B], in_8([B]) & ~TACK);
+   } else
+   out_8([B], in_8([B]) | TACK);
 }
 
 static inline void negate_TIP_and_TACK(void)
 {
-   out_8([B], in_8([B]) | TIP | TACK);
+   if (mcu_is_egret) {
+  

Re: [PATCH] tools/power/turbostat: add Denverton to has_snb_msr()

[Len Brown]

Applied.

thanks!

Len Brown, Intel Open Source Technology Center

[PATCH 01/10] via-cuda: Cleanup printk calls

[Finn Thain]

Add missing log message severity, remove old debug messages and
replace printk() loop with print_hex_dump() call.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index bad1813..98ced65 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -221,7 +221,7 @@ static int __init via_cuda_start(void)
return -EAGAIN;
 }
 
-printk("Macintosh CUDA driver v0.5 for Unified ADB.\n");
+pr_info("Macintosh CUDA driver v0.5 for Unified ADB.\n");
 
 cuda_fully_inited = 1;
 return 0;
@@ -251,7 +251,7 @@ cuda_probe(void)
int x;  \
for (x = 1000; !(cond); --x) {  \
if (x == 0) {   \
-   printk("Timeout waiting for " what "\n");   \
+   pr_err("Timeout waiting for " what "\n");   \
return -ENXIO;  \
}   \
udelay(100);\
@@ -357,6 +357,7 @@ cuda_reset_adb_bus(void)
 return 0;
 }
 #endif /* CONFIG_ADB */
+
 /* Construct and send a cuda request */
 int
 cuda_request(struct adb_request *req, void (*done)(struct adb_request *),
@@ -474,12 +475,9 @@ cuda_interrupt(int irq, void *arg)
 }
 
 status = (~in_8([B]) & (TIP|TREQ)) | (in_8([ACR]) & SR_OUT);
-/* printk("cuda_interrupt: state=%d status=%x\n", cuda_state, status); */
 switch (cuda_state) {
 case idle:
/* CUDA has sent us the first byte of data - unsolicited */
-   if (status != TREQ)
-   printk("cuda: state=idle, status=%x\n", status);
(void)in_8([SR]);
out_8([B], in_8([B]) & ~TIP);
cuda_state = reading;
@@ -489,8 +487,6 @@ cuda_interrupt(int irq, void *arg)
 
 case awaiting_reply:
/* CUDA has sent us the first byte of data of a reply */
-   if (status != TREQ)
-   printk("cuda: state=awaiting_reply, status=%x\n", status);
(void)in_8([SR]);
out_8([B], in_8([B]) & ~TIP);
cuda_state = reading;
@@ -506,9 +502,6 @@ cuda_interrupt(int irq, void *arg)
out_8([B], in_8([B]) | TIP | TACK);
cuda_state = idle;
} else {
-   /* assert status == TIP + SR_OUT */
-   if (status != TIP + SR_OUT)
-   printk("cuda: state=sent_first_byte status=%x\n", status);
out_8([SR], current_req->data[1]);
out_8([B], in_8([B]) ^ TACK);
data_index = 2;
@@ -545,9 +538,6 @@ cuda_interrupt(int irq, void *arg)
out_8([B], in_8([B]) | TACK | TIP);
cuda_state = read_done;
} else {
-   /* assert status == TIP | TREQ */
-   if (status != TIP + TREQ)
-   printk("cuda: state=reading status=%x\n", status);
out_8([B], in_8([B]) ^ TACK);
}
break;
@@ -593,7 +583,7 @@ cuda_interrupt(int irq, void *arg)
break;
 
 default:
-   printk("cuda_interrupt: unknown cuda_state %d?\n", cuda_state);
+   pr_err("cuda_interrupt: unknown cuda_state %d?\n", cuda_state);
 }
 spin_unlock(_lock);
 if (complete && req) {
@@ -614,8 +604,6 @@ cuda_interrupt(int irq, void *arg)
 static void
 cuda_input(unsigned char *buf, int nb)
 {
-int i;
-
 switch (buf[0]) {
 case ADB_PACKET:
 #ifdef CONFIG_XMON
@@ -633,9 +621,7 @@ cuda_input(unsigned char *buf, int nb)
break;
 
 default:
-   printk("data from cuda (%d bytes):", nb);
-   for (i = 0; i < nb; ++i)
-   printk(" %.2x", buf[i]);
-   printk("\n");
+   print_hex_dump(KERN_INFO, "cuda_input: ", DUMP_PREFIX_NONE, 32, 1,
+  buf, nb, false);
 }
 }
-- 
2.7.3

[PATCH 08/10] via-cuda: Initialize data_index early and increment consistently

[Finn Thain]

Initialize data_index where appropriate to improve readability and
assist debugging. This change doesn't affect driver behaviour.

I prefer to see
current_req->data[data_index++]
in place of
current_req->data[0]
or
current_req->data[1]
inasmuchas it becomes obvious what the data_index variable does.

Moreover, the actual value of data_index when examined at any given moment
tells me something about prior events, which did prove helpful.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index b801c67..f1b2c5b 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -446,12 +446,13 @@ cuda_start(void)
 /* assert cuda_state == idle */
 if (current_req == NULL)
return;
+data_index = 0;
 if (TREQ_asserted(in_8([B])))
return; /* a byte is coming in from the CUDA */
 
 /* set the shift register to shift out and send a byte */
 out_8([ACR], in_8([ACR]) | SR_OUT);
-out_8([SR], current_req->data[0]);
+out_8([SR], current_req->data[data_index++]);
 assert_TIP();
 cuda_state = sent_first_byte;
 }
@@ -524,9 +525,8 @@ cuda_interrupt(int irq, void *arg)
negate_TIP_and_TACK();
cuda_state = idle;
} else {
-   out_8([SR], current_req->data[1]);
+   out_8([SR], current_req->data[data_index++]);
toggle_TACK();
-   data_index = 2;
cuda_state = sending;
}
break;
-- 
2.7.3

[PATCH 09/10] via-cuda: Add support for Egret system controller

[Finn Thain]

The Egret system controller was the predecessor to the Cuda and the
differences are minor.

On Cuda, byte acknowledgement requires one transition of the TACK
signal; on Egret two are needed. On Cuda, TIP is active low; on Egret
it is active high. And Cuda raises certain interrupts that Egret omits.

Accomodating these differences complicates the Cuda driver slightly
but avoids a lot of duplication (see next patch).

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 155 +--
 1 file changed, 134 insertions(+), 21 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index f1b2c5b..3254070 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -1,10 +1,10 @@
 /*
- * Device driver for the via-cuda on Apple Powermacs.
+ * Device driver for the Cuda and Egret system controllers found on PowerMacs
+ * and 68k Macs.
  *
- * The VIA (versatile interface adapter) interfaces to the CUDA,
- * a 6805 microprocessor core which controls the ADB (Apple Desktop
- * Bus) which connects to the keyboard and mouse.  The CUDA also
- * controls system power and the RTC (real time clock) chip.
+ * The Cuda or Egret is a 6805 microcontroller interfaced to the 6522 VIA.
+ * This MCU controls system power, Parameter RAM, Real Time Clock and the
+ * Apple Desktop Bus (ADB) that connects to the keyboard and mouse.
  *
  * Copyright (C) 1996 Paul Mackerras.
  */
@@ -50,10 +50,27 @@ static DEFINE_SPINLOCK(cuda_lock);
 #define IER(14*RS) /* Interrupt enable register */
 #define ANH(15*RS) /* A-side data, no handshake */
 
-/* Bits in B data register: all active low */
-#define TREQ   0x08/* Transfer request (input) */
-#define TACK   0x10/* Transfer acknowledge (output) */
-#define TIP0x20/* Transfer in progress (output) */
+/*
+ * When the Cuda design replaced the Egret, some signal names and
+ * logic sense changed. They all serve the same purposes, however.
+ *
+ *   VIA pin   |  Egret pin
+ * +--
+ *   PB3 (input)   |  Transceiver session   (active low)
+ *   PB4 (output)  |  VIA full  (active high)
+ *   PB5 (output)  |  System session(active high)
+ *
+ *   VIA pin   |  Cuda pin
+ * +--
+ *   PB3 (input)   |  Transfer request  (active low)
+ *   PB4 (output)  |  Byte acknowledge  (active low)
+ *   PB5 (output)  |  Transfer in progress  (active low)
+ */
+
+/* Bits in Port B data register */
+#define TREQ   0x08/* Transfer request */
+#define TACK   0x10/* Transfer acknowledge */
+#define TIP0x20/* Transfer in progress */
 
 /* Bits in ACR */
 #define SR_CTRL0x1c/* Shift register control bits 
*/
@@ -65,6 +82,19 @@ static DEFINE_SPINLOCK(cuda_lock);
 #define IER_CLR0   /* clear bits in IER */
 #define SR_INT 0x04/* Shift register full/empty */
 
+/* Duration of byte acknowledgement pulse (us) */
+#define EGRET_TACK_ASSERTED_DELAY  300
+#define EGRET_TACK_NEGATED_DELAY   400
+
+/* Interval from interrupt to start of session (us) */
+#define EGRET_SESSION_DELAY450
+
+#ifdef CONFIG_PPC
+#define mcu_is_egret   false
+#else
+static bool mcu_is_egret;
+#endif
+
 static inline bool TREQ_asserted(u8 portb)
 {
return !(portb & TREQ);
@@ -72,12 +102,29 @@ static inline bool TREQ_asserted(u8 portb)
 
 static inline void assert_TIP(void)
 {
-   out_8([B], in_8([B]) & ~TIP);
+   if (mcu_is_egret) {
+   udelay(EGRET_SESSION_DELAY);
+   out_8([B], in_8([B]) | TIP);
+   } else
+   out_8([B], in_8([B]) & ~TIP);
+}
+
+static inline void assert_TIP_and_TACK(void)
+{
+   if (mcu_is_egret) {
+   udelay(EGRET_SESSION_DELAY);
+   out_8([B], in_8([B]) | TIP | TACK);
+   } else
+   out_8([B], in_8([B]) & ~(TIP | TACK));
 }
 
 static inline void assert_TACK(void)
 {
-   out_8([B], in_8([B]) & ~TACK);
+   if (mcu_is_egret) {
+   udelay(EGRET_TACK_NEGATED_DELAY);
+   out_8([B], in_8([B]) | TACK);
+   } else
+   out_8([B], in_8([B]) & ~TACK);
 }
 
 static inline void toggle_TACK(void)
@@ -87,12 +134,20 @@ static inline void toggle_TACK(void)
 
 static inline void negate_TACK(void)
 {
-   out_8([B], in_8([B]) | TACK);
+   if (mcu_is_egret) {
+   udelay(EGRET_TACK_ASSERTED_DELAY);
+   out_8([B], in_8([B]) & ~TACK);
+   } else
+   out_8([B], in_8([B]) | TACK);
 }
 
 static inline void negate_TIP_and_TACK(void)
 {
-   out_8([B], in_8([B]) | TIP | TACK);
+   if (mcu_is_egret) {
+   

Re: [PATCH] tools/power/turbostat: add Denverton to has_snb_msr()

[Len Brown]

Applied.

thanks!

Len Brown, Intel Open Source Technology Center

[PATCH 01/10] via-cuda: Cleanup printk calls

[Finn Thain]

Add missing log message severity, remove old debug messages and
replace printk() loop with print_hex_dump() call.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index bad1813..98ced65 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -221,7 +221,7 @@ static int __init via_cuda_start(void)
return -EAGAIN;
 }
 
-printk("Macintosh CUDA driver v0.5 for Unified ADB.\n");
+pr_info("Macintosh CUDA driver v0.5 for Unified ADB.\n");
 
 cuda_fully_inited = 1;
 return 0;
@@ -251,7 +251,7 @@ cuda_probe(void)
int x;  \
for (x = 1000; !(cond); --x) {  \
if (x == 0) {   \
-   printk("Timeout waiting for " what "\n");   \
+   pr_err("Timeout waiting for " what "\n");   \
return -ENXIO;  \
}   \
udelay(100);\
@@ -357,6 +357,7 @@ cuda_reset_adb_bus(void)
 return 0;
 }
 #endif /* CONFIG_ADB */
+
 /* Construct and send a cuda request */
 int
 cuda_request(struct adb_request *req, void (*done)(struct adb_request *),
@@ -474,12 +475,9 @@ cuda_interrupt(int irq, void *arg)
 }
 
 status = (~in_8([B]) & (TIP|TREQ)) | (in_8([ACR]) & SR_OUT);
-/* printk("cuda_interrupt: state=%d status=%x\n", cuda_state, status); */
 switch (cuda_state) {
 case idle:
/* CUDA has sent us the first byte of data - unsolicited */
-   if (status != TREQ)
-   printk("cuda: state=idle, status=%x\n", status);
(void)in_8([SR]);
out_8([B], in_8([B]) & ~TIP);
cuda_state = reading;
@@ -489,8 +487,6 @@ cuda_interrupt(int irq, void *arg)
 
 case awaiting_reply:
/* CUDA has sent us the first byte of data of a reply */
-   if (status != TREQ)
-   printk("cuda: state=awaiting_reply, status=%x\n", status);
(void)in_8([SR]);
out_8([B], in_8([B]) & ~TIP);
cuda_state = reading;
@@ -506,9 +502,6 @@ cuda_interrupt(int irq, void *arg)
out_8([B], in_8([B]) | TIP | TACK);
cuda_state = idle;
} else {
-   /* assert status == TIP + SR_OUT */
-   if (status != TIP + SR_OUT)
-   printk("cuda: state=sent_first_byte status=%x\n", status);
out_8([SR], current_req->data[1]);
out_8([B], in_8([B]) ^ TACK);
data_index = 2;
@@ -545,9 +538,6 @@ cuda_interrupt(int irq, void *arg)
out_8([B], in_8([B]) | TACK | TIP);
cuda_state = read_done;
} else {
-   /* assert status == TIP | TREQ */
-   if (status != TIP + TREQ)
-   printk("cuda: state=reading status=%x\n", status);
out_8([B], in_8([B]) ^ TACK);
}
break;
@@ -593,7 +583,7 @@ cuda_interrupt(int irq, void *arg)
break;
 
 default:
-   printk("cuda_interrupt: unknown cuda_state %d?\n", cuda_state);
+   pr_err("cuda_interrupt: unknown cuda_state %d?\n", cuda_state);
 }
 spin_unlock(_lock);
 if (complete && req) {
@@ -614,8 +604,6 @@ cuda_interrupt(int irq, void *arg)
 static void
 cuda_input(unsigned char *buf, int nb)
 {
-int i;
-
 switch (buf[0]) {
 case ADB_PACKET:
 #ifdef CONFIG_XMON
@@ -633,9 +621,7 @@ cuda_input(unsigned char *buf, int nb)
break;
 
 default:
-   printk("data from cuda (%d bytes):", nb);
-   for (i = 0; i < nb; ++i)
-   printk(" %.2x", buf[i]);
-   printk("\n");
+   print_hex_dump(KERN_INFO, "cuda_input: ", DUMP_PREFIX_NONE, 32, 1,
+  buf, nb, false);
 }
 }
-- 
2.7.3

[PATCH 08/10] via-cuda: Initialize data_index early and increment consistently

[Finn Thain]

Initialize data_index where appropriate to improve readability and
assist debugging. This change doesn't affect driver behaviour.

I prefer to see
current_req->data[data_index++]
in place of
current_req->data[0]
or
current_req->data[1]
inasmuchas it becomes obvious what the data_index variable does.

Moreover, the actual value of data_index when examined at any given moment
tells me something about prior events, which did prove helpful.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index b801c67..f1b2c5b 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -446,12 +446,13 @@ cuda_start(void)
 /* assert cuda_state == idle */
 if (current_req == NULL)
return;
+data_index = 0;
 if (TREQ_asserted(in_8([B])))
return; /* a byte is coming in from the CUDA */
 
 /* set the shift register to shift out and send a byte */
 out_8([ACR], in_8([ACR]) | SR_OUT);
-out_8([SR], current_req->data[0]);
+out_8([SR], current_req->data[data_index++]);
 assert_TIP();
 cuda_state = sent_first_byte;
 }
@@ -524,9 +525,8 @@ cuda_interrupt(int irq, void *arg)
negate_TIP_and_TACK();
cuda_state = idle;
} else {
-   out_8([SR], current_req->data[1]);
+   out_8([SR], current_req->data[data_index++]);
toggle_TACK();
-   data_index = 2;
cuda_state = sending;
}
break;
-- 
2.7.3

[PATCH 07/10] via-cuda: Use spinlock_irq_save/restore instead of enable/disable_irq

[Finn Thain]

The cuda_start() function uses spinlock_irq_save/restore for mutual
exclusion. Let's have cuda_poll() do the same when polling the VIA
interrupt.

The benefit to disabling local irqs when the interrupt is being polled
is that the interrupt handler now has the same timing properties
regardless of whether it is invoked normally or from cuda_poll().

This driver was written back when local irqs remained enabled during
execution of interrupt handlers and cuda_poll() was probably trying
to achieve the same effect by use of enable/disable_irq.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 

---

I wondered whether the use of enable/disable_irq was an attempt to reduce
interrupt latency given that VIA register accesses are slow. But if the
interrupt has not yet fired, only one VIA access will take place, which
imposes neglible latency anyway.
---
 drivers/macintosh/via-cuda.c | 16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index ff9062a..b801c67 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -459,14 +459,7 @@ cuda_start(void)
 void
 cuda_poll(void)
 {
-/* cuda_interrupt only takes a normal lock, we disable
- * interrupts here to avoid re-entering and thus deadlocking.
- */
-if (cuda_irq)
-   disable_irq(cuda_irq);
-cuda_interrupt(0, NULL);
-if (cuda_irq)
-   enable_irq(cuda_irq);
+   cuda_interrupt(0, NULL);
 }
 EXPORT_SYMBOL(cuda_poll);
 
@@ -475,13 +468,14 @@ EXPORT_SYMBOL(cuda_poll);
 static irqreturn_t
 cuda_interrupt(int irq, void *arg)
 {
+unsigned long flags;
 u8 status;
 struct adb_request *req = NULL;
 unsigned char ibuf[16];
 int ibuf_len = 0;
 int complete = 0;
 
-spin_lock(_lock);
+spin_lock_irqsave(_lock, flags);
 
 /* On powermacs, this handler is registered for the VIA IRQ. But they use
  * just the shift register IRQ -- other VIA interrupt sources are disabled.
@@ -494,7 +488,7 @@ cuda_interrupt(int irq, void *arg)
 #endif
 {
 if ((in_8([IFR]) & SR_INT) == 0) {
-spin_unlock(_lock);
+spin_unlock_irqrestore(_lock, flags);
 return IRQ_NONE;
 } else {
 out_8([IFR], SR_INT);
@@ -616,7 +610,7 @@ cuda_interrupt(int irq, void *arg)
 default:
pr_err("cuda_interrupt: unknown cuda_state %d?\n", cuda_state);
 }
-spin_unlock(_lock);
+spin_unlock_irqrestore(_lock, flags);
 if (complete && req) {
void (*done)(struct adb_request *) = req->done;
mb();
-- 
2.7.3

[PATCH 02/10] via-cuda: Remove redundant temporary variable

[Finn Thain]

There is no possibility that current_req can change during execution of
cuda_start(). This can be confirmed by inspection: cuda_lock is always
held whenever cuda_start() is called or current_req is modified.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index 98ced65..4eb8a884 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -414,19 +414,15 @@ cuda_write(struct adb_request *req)
 static void
 cuda_start(void)
 {
-struct adb_request *req;
-
 /* assert cuda_state == idle */
-/* get the packet to send */
-req = current_req;
-if (req == 0)
+if (current_req == NULL)
return;
 if ((in_8([B]) & TREQ) == 0)
return; /* a byte is coming in from the CUDA */
 
 /* set the shift register to shift out and send a byte */
 out_8([ACR], in_8([ACR]) | SR_OUT);
-out_8([SR], req->data[0]);
+out_8([SR], current_req->data[0]);
 out_8([B], in_8([B]) & ~TIP);
 cuda_state = sent_first_byte;
 }
-- 
2.7.3

[PATCH 00/10] Replace via-maciisi with via-cuda driver

[Finn Thain]

This patch series has some improvements for the the Cuda driver: cleanup,
bug fixes and new functionality.

The broken via-maciisi driver is then replaced by via-cuda. This
eliminates over 600 LoC.

Thanks to Stan Johnson for testing these patches on a Mac LC III and
a PowerMac G3.


Finn Thain (10):
  via-cuda: Cleanup printk calls
  via-cuda: Remove redundant temporary variable
  via-cuda: Add TREQ, TIP and TACK signal helpers
  via-cuda: Prevent read buffer overflow
  via-cuda: Fix re-initialization of reply_ptr and reading_reply
  via-cuda: Avoid TREQ race condition
  via-cuda: Use spinlock_irq_save/restore instead of enable/disable_irq
  via-cuda: Initialize data_index early and increment consistently
  via-cuda: Add support for Egret system controller
  m68k/mac: Replace via-maciisi driver with via-cuda driver

 arch/m68k/include/asm/macintosh.h |   2 +-
 arch/m68k/mac/config.c|  18 +-
 arch/m68k/mac/misc.c  |  72 +---
 drivers/macintosh/Kconfig |  24 +-
 drivers/macintosh/Makefile|   1 -
 drivers/macintosh/adb.c   |   4 -
 drivers/macintosh/via-cuda.c  | 294 -
 drivers/macintosh/via-maciisi.c   | 677 --
 8 files changed, 235 insertions(+), 857 deletions(-)
 delete mode 100644 drivers/macintosh/via-maciisi.c

-- 
2.7.3

[PATCH 07/10] via-cuda: Use spinlock_irq_save/restore instead of enable/disable_irq

[Finn Thain]

The cuda_start() function uses spinlock_irq_save/restore for mutual
exclusion. Let's have cuda_poll() do the same when polling the VIA
interrupt.

The benefit to disabling local irqs when the interrupt is being polled
is that the interrupt handler now has the same timing properties
regardless of whether it is invoked normally or from cuda_poll().

This driver was written back when local irqs remained enabled during
execution of interrupt handlers and cuda_poll() was probably trying
to achieve the same effect by use of enable/disable_irq.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 

---

I wondered whether the use of enable/disable_irq was an attempt to reduce
interrupt latency given that VIA register accesses are slow. But if the
interrupt has not yet fired, only one VIA access will take place, which
imposes neglible latency anyway.
---
 drivers/macintosh/via-cuda.c | 16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index ff9062a..b801c67 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -459,14 +459,7 @@ cuda_start(void)
 void
 cuda_poll(void)
 {
-/* cuda_interrupt only takes a normal lock, we disable
- * interrupts here to avoid re-entering and thus deadlocking.
- */
-if (cuda_irq)
-   disable_irq(cuda_irq);
-cuda_interrupt(0, NULL);
-if (cuda_irq)
-   enable_irq(cuda_irq);
+   cuda_interrupt(0, NULL);
 }
 EXPORT_SYMBOL(cuda_poll);
 
@@ -475,13 +468,14 @@ EXPORT_SYMBOL(cuda_poll);
 static irqreturn_t
 cuda_interrupt(int irq, void *arg)
 {
+unsigned long flags;
 u8 status;
 struct adb_request *req = NULL;
 unsigned char ibuf[16];
 int ibuf_len = 0;
 int complete = 0;
 
-spin_lock(_lock);
+spin_lock_irqsave(_lock, flags);
 
 /* On powermacs, this handler is registered for the VIA IRQ. But they use
  * just the shift register IRQ -- other VIA interrupt sources are disabled.
@@ -494,7 +488,7 @@ cuda_interrupt(int irq, void *arg)
 #endif
 {
 if ((in_8([IFR]) & SR_INT) == 0) {
-spin_unlock(_lock);
+spin_unlock_irqrestore(_lock, flags);
 return IRQ_NONE;
 } else {
 out_8([IFR], SR_INT);
@@ -616,7 +610,7 @@ cuda_interrupt(int irq, void *arg)
 default:
pr_err("cuda_interrupt: unknown cuda_state %d?\n", cuda_state);
 }
-spin_unlock(_lock);
+spin_unlock_irqrestore(_lock, flags);
 if (complete && req) {
void (*done)(struct adb_request *) = req->done;
mb();
-- 
2.7.3

[PATCH 02/10] via-cuda: Remove redundant temporary variable

[Finn Thain]

There is no possibility that current_req can change during execution of
cuda_start(). This can be confirmed by inspection: cuda_lock is always
held whenever cuda_start() is called or current_req is modified.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index 98ced65..4eb8a884 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -414,19 +414,15 @@ cuda_write(struct adb_request *req)
 static void
 cuda_start(void)
 {
-struct adb_request *req;
-
 /* assert cuda_state == idle */
-/* get the packet to send */
-req = current_req;
-if (req == 0)
+if (current_req == NULL)
return;
 if ((in_8([B]) & TREQ) == 0)
return; /* a byte is coming in from the CUDA */
 
 /* set the shift register to shift out and send a byte */
 out_8([ACR], in_8([ACR]) | SR_OUT);
-out_8([SR], req->data[0]);
+out_8([SR], current_req->data[0]);
 out_8([B], in_8([B]) & ~TIP);
 cuda_state = sent_first_byte;
 }
-- 
2.7.3

[PATCH 00/10] Replace via-maciisi with via-cuda driver

[Finn Thain]

This patch series has some improvements for the the Cuda driver: cleanup,
bug fixes and new functionality.

The broken via-maciisi driver is then replaced by via-cuda. This
eliminates over 600 LoC.

Thanks to Stan Johnson for testing these patches on a Mac LC III and
a PowerMac G3.


Finn Thain (10):
  via-cuda: Cleanup printk calls
  via-cuda: Remove redundant temporary variable
  via-cuda: Add TREQ, TIP and TACK signal helpers
  via-cuda: Prevent read buffer overflow
  via-cuda: Fix re-initialization of reply_ptr and reading_reply
  via-cuda: Avoid TREQ race condition
  via-cuda: Use spinlock_irq_save/restore instead of enable/disable_irq
  via-cuda: Initialize data_index early and increment consistently
  via-cuda: Add support for Egret system controller
  m68k/mac: Replace via-maciisi driver with via-cuda driver

 arch/m68k/include/asm/macintosh.h |   2 +-
 arch/m68k/mac/config.c|  18 +-
 arch/m68k/mac/misc.c  |  72 +---
 drivers/macintosh/Kconfig |  24 +-
 drivers/macintosh/Makefile|   1 -
 drivers/macintosh/adb.c   |   4 -
 drivers/macintosh/via-cuda.c  | 294 -
 drivers/macintosh/via-maciisi.c   | 677 --
 8 files changed, 235 insertions(+), 857 deletions(-)
 delete mode 100644 drivers/macintosh/via-maciisi.c

-- 
2.7.3

[PATCH 04/10] via-cuda: Prevent read buffer overflow

[Finn Thain]

If the Cuda driver does not enter the 'read_done' state for some
reason, it may continue in the 'reading' state until the buffer
overflows. Add a bounds check to prevent this.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index 64626d2..acf3a95 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -470,6 +470,8 @@ cuda_poll(void)
 }
 EXPORT_SYMBOL(cuda_poll);
 
+#define ARRAY_FULL(a, p)   ((p) - (a) == ARRAY_SIZE(a))
+
 static irqreturn_t
 cuda_interrupt(int irq, void *arg)
 {
@@ -558,7 +560,11 @@ cuda_interrupt(int irq, void *arg)
break;
 
 case reading:
-   *reply_ptr++ = in_8([SR]);
+   if (reading_reply ? ARRAY_FULL(current_req->reply, reply_ptr)
+ : ARRAY_FULL(cuda_rbuf, reply_ptr))
+   (void)in_8([SR]);
+   else
+   *reply_ptr++ = in_8([SR]);
if (!TREQ_asserted(status)) {
/* that's all folks */
negate_TIP_and_TACK();
-- 
2.7.3

[PATCH 04/10] via-cuda: Prevent read buffer overflow

[Finn Thain]

If the Cuda driver does not enter the 'read_done' state for some
reason, it may continue in the 'reading' state until the buffer
overflows. Add a bounds check to prevent this.

Tested-by: Stan Johnson 
Signed-off-by: Finn Thain 
---
 drivers/macintosh/via-cuda.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index 64626d2..acf3a95 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -470,6 +470,8 @@ cuda_poll(void)
 }
 EXPORT_SYMBOL(cuda_poll);
 
+#define ARRAY_FULL(a, p)   ((p) - (a) == ARRAY_SIZE(a))
+
 static irqreturn_t
 cuda_interrupt(int irq, void *arg)
 {
@@ -558,7 +560,11 @@ cuda_interrupt(int irq, void *arg)
break;
 
 case reading:
-   *reply_ptr++ = in_8([SR]);
+   if (reading_reply ? ARRAY_FULL(current_req->reply, reply_ptr)
+ : ARRAY_FULL(cuda_rbuf, reply_ptr))
+   (void)in_8([SR]);
+   else
+   *reply_ptr++ = in_8([SR]);
if (!TREQ_asserted(status)) {
/* that's all folks */
negate_TIP_and_TACK();
-- 
2.7.3