Re: [PATCH v4 1/4] usb: dbc: early driver for xhci debug capability

[Lu Baolu]

Hi Peter,

On 11/11/2016 08:28 PM, Peter Zijlstra wrote:
> On Fri, Nov 11, 2016 at 12:33:29PM +0800, Lu Baolu wrote:
>
>> Things become complicated when it comes to USB debug port.
>> But it's still addressable.
>>
>> At this time, we can do it like this.
>>
>> write()
>> {
>>  if (in_nmi() && raw_spin_is_locked())
>>  return;
>>
>>  raw_spinlock_irqsave(, flags);
>>  
>>
> Please use raw_spin_trlock_irqsave() instead, spin_is_locked() is fairly
> icky.

Sure.

>
> Also, there's a bunch of exception contexts that do not set in_nmi().
> That is in_nmi() is really only set for #NM. #MC and #DB and
> others do not set this.

That's worth another fix patch. Let me look into it later.

>
>> This will filter some messages from NMI handler in case that
>> another thread is holding the spinlock. I have no idea about
>> how much chance could a debug user faces this. But it might
>> further be fixed with below enhancement.
>>
>> write()
>> {
>>  if (in_nmi() && raw_spin_is_locked()) {
>>  produce_a_pending_item_in_ring();
>>  return;
>>  }
>>
>>  raw_spinlock_irqsave(, flags);
>>
>>  while (!pending_item_ring_is_empty)
>>  consume_a_pending_item_in_ring();
>>
>>  
>>
>>
>> We can design the pending item ring in a producer-consumer
>> model. It's easy to avoid race between the producer and
>> consumer.
> Problem is that the consumer might never happen, those are the fun most
> bugs.
>
> Not being able to deal with random nested exception context really
> reduces the utility of this thing.
>
> Again, a UART rules. Make a virtual UART in hardware, that'd be totally
> awesome. This thing, I'm not convinced its worth having.

This is the initial work. It helps at least in cases where people need
to dump kernel messages but lacking of a console. It's also a cheap
way, people don't need to buy any third-party devices.

With more and more people trying and enhancing it, it will become
more robust and helpful.

Best regards,
Lu Baolu

Re: [PATCH v4 1/4] usb: dbc: early driver for xhci debug capability

[Lu Baolu]

Hi Peter,

On 11/11/2016 08:28 PM, Peter Zijlstra wrote:
> On Fri, Nov 11, 2016 at 12:33:29PM +0800, Lu Baolu wrote:
>
>> Things become complicated when it comes to USB debug port.
>> But it's still addressable.
>>
>> At this time, we can do it like this.
>>
>> write()
>> {
>>  if (in_nmi() && raw_spin_is_locked())
>>  return;
>>
>>  raw_spinlock_irqsave(, flags);
>>  
>>
> Please use raw_spin_trlock_irqsave() instead, spin_is_locked() is fairly
> icky.

Sure.

>
> Also, there's a bunch of exception contexts that do not set in_nmi().
> That is in_nmi() is really only set for #NM. #MC and #DB and
> others do not set this.

That's worth another fix patch. Let me look into it later.

>
>> This will filter some messages from NMI handler in case that
>> another thread is holding the spinlock. I have no idea about
>> how much chance could a debug user faces this. But it might
>> further be fixed with below enhancement.
>>
>> write()
>> {
>>  if (in_nmi() && raw_spin_is_locked()) {
>>  produce_a_pending_item_in_ring();
>>  return;
>>  }
>>
>>  raw_spinlock_irqsave(, flags);
>>
>>  while (!pending_item_ring_is_empty)
>>  consume_a_pending_item_in_ring();
>>
>>  
>>
>>
>> We can design the pending item ring in a producer-consumer
>> model. It's easy to avoid race between the producer and
>> consumer.
> Problem is that the consumer might never happen, those are the fun most
> bugs.
>
> Not being able to deal with random nested exception context really
> reduces the utility of this thing.
>
> Again, a UART rules. Make a virtual UART in hardware, that'd be totally
> awesome. This thing, I'm not convinced its worth having.

This is the initial work. It helps at least in cases where people need
to dump kernel messages but lacking of a console. It's also a cheap
way, people don't need to buy any third-party devices.

With more and more people trying and enhancing it, it will become
more robust and helpful.

Best regards,
Lu Baolu

Re: [PATCH] iproute2: ss: escape all null bytes in abstract unix domain socket

[Stephen Hemminger]

On Sat, 29 Oct 2016 22:20:19 +0300
Isaac Boukris  wrote:

> Abstract unix domain socket may embed null characters,
> these should be translated to '@' when printed by ss the
> same way the null prefix is currently being translated.
> 
> Signed-off-by: Isaac Boukris 

Applied

Re: [PATCH] iproute2: ss: escape all null bytes in abstract unix domain socket

[Stephen Hemminger]

On Sat, 29 Oct 2016 22:20:19 +0300
Isaac Boukris  wrote:

> Abstract unix domain socket may embed null characters,
> these should be translated to '@' when printed by ss the
> same way the null prefix is currently being translated.
> 
> Signed-off-by: Isaac Boukris 

Applied

kvm: suspicious RCU usage/missed lock in kvm_lapic_set_vapic_addr

[Dmitry Vyukov]

Hello,

I've got the following warning while running syzkaller fuzzer:


[ INFO: suspicious RCU usage. ]
4.9.0-rc4+ #47 Not tainted
---
./include/linux/kvm_host.h:536 suspicious rcu_dereference_check() usage!

other info that might help us debug this:

rcu_scheduler_active = 1, debug_locks = 0
1 lock held by syz-executor/6679:
 #0: [   67.230820]  (

stack backtrace:
CPU: 1 PID: 6679 Comm: syz-executor Not tainted 4.9.0-rc4+ #47
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 880039e2f6d0 81c2e46b 88003e3a5b40 
 0001 83215600 880039e2f700 81334ea9
 c9000730b000 0004 88003c4f8420 88003d3f8000
Call Trace:
 [< inline >] __dump_stack lib/dump_stack.c:15
 [] dump_stack+0xb3/0x118 lib/dump_stack.c:51
 [] lockdep_rcu_suspicious+0x139/0x180
kernel/locking/lockdep.c:4445
 [< inline >] __kvm_memslots include/linux/kvm_host.h:534
 [< inline >] kvm_memslots include/linux/kvm_host.h:541
 [] kvm_gfn_to_hva_cache_init+0xa1e/0xce0
arch/x86/kvm/../../../virt/kvm/kvm_main.c:1941
 [] kvm_lapic_set_vapic_addr+0xed/0x140
arch/x86/kvm/lapic.c:2217
 [] kvm_arch_vcpu_ioctl+0x224/0x3100 arch/x86/kvm/x86.c:3425
 [] kvm_vcpu_ioctl+0x1e2/0xdd0
arch/x86/kvm/../../../virt/kvm/kvm_main.c:2708
 [< inline >] vfs_ioctl fs/ioctl.c:43
 [] do_vfs_ioctl+0x18c/0x1040 fs/ioctl.c:679
 [< inline >] SYSC_ioctl fs/ioctl.c:694
 [] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:685
 [] entry_SYSCALL_64_fastpath+0x1f/0xc2


__kvm_memslots requires a lock to be held, but it is not held:

static inline struct kvm_memslots *__kvm_memslots(struct kvm *kvm, int as_id)
{
  return rcu_dereference_check(kvm->memslots[as_id],
srcu_read_lock_held(>srcu)
|| lockdep_is_held(>slots_lock));
}

On commit 015ed9433be2b476ec7e2e6a9a411a56e3b5b035 (Nov 11).

kvm: suspicious RCU usage/missed lock in kvm_lapic_set_vapic_addr

[Dmitry Vyukov]

Hello,

I've got the following warning while running syzkaller fuzzer:


[ INFO: suspicious RCU usage. ]
4.9.0-rc4+ #47 Not tainted
---
./include/linux/kvm_host.h:536 suspicious rcu_dereference_check() usage!

other info that might help us debug this:

rcu_scheduler_active = 1, debug_locks = 0
1 lock held by syz-executor/6679:
 #0: [   67.230820]  (

stack backtrace:
CPU: 1 PID: 6679 Comm: syz-executor Not tainted 4.9.0-rc4+ #47
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 880039e2f6d0 81c2e46b 88003e3a5b40 
 0001 83215600 880039e2f700 81334ea9
 c9000730b000 0004 88003c4f8420 88003d3f8000
Call Trace:
 [< inline >] __dump_stack lib/dump_stack.c:15
 [] dump_stack+0xb3/0x118 lib/dump_stack.c:51
 [] lockdep_rcu_suspicious+0x139/0x180
kernel/locking/lockdep.c:4445
 [< inline >] __kvm_memslots include/linux/kvm_host.h:534
 [< inline >] kvm_memslots include/linux/kvm_host.h:541
 [] kvm_gfn_to_hva_cache_init+0xa1e/0xce0
arch/x86/kvm/../../../virt/kvm/kvm_main.c:1941
 [] kvm_lapic_set_vapic_addr+0xed/0x140
arch/x86/kvm/lapic.c:2217
 [] kvm_arch_vcpu_ioctl+0x224/0x3100 arch/x86/kvm/x86.c:3425
 [] kvm_vcpu_ioctl+0x1e2/0xdd0
arch/x86/kvm/../../../virt/kvm/kvm_main.c:2708
 [< inline >] vfs_ioctl fs/ioctl.c:43
 [] do_vfs_ioctl+0x18c/0x1040 fs/ioctl.c:679
 [< inline >] SYSC_ioctl fs/ioctl.c:694
 [] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:685
 [] entry_SYSCALL_64_fastpath+0x1f/0xc2


__kvm_memslots requires a lock to be held, but it is not held:

static inline struct kvm_memslots *__kvm_memslots(struct kvm *kvm, int as_id)
{
  return rcu_dereference_check(kvm->memslots[as_id],
srcu_read_lock_held(>srcu)
|| lockdep_is_held(>slots_lock));
}

On commit 015ed9433be2b476ec7e2e6a9a411a56e3b5b035 (Nov 11).

[PATCH 09/10] ARM: dts: sun8i-h3: Add device nodes for audio codec and its analog controls

[Chen-Yu Tsai]

Now that we support the audio codec found on the Allwinner H3 SoC, add
device nodes for it.

Signed-off-by: Chen-Yu Tsai 
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index c38b028cac83..ceec979f57e4 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -485,6 +485,20 @@
status = "disabled";
};
 
+   codec: codec@01c22c00 {
+   #sound-dai-cells = <0>;
+   compatible = "allwinner,sun8i-h3-codec";
+   reg = <0x01c22c00 0x400>;
+   interrupts = ;
+   clocks = < CLK_BUS_CODEC>, < CLK_AC_DIG>;
+   clock-names = "apb", "codec";
+   resets = < RST_BUS_CODEC>;
+   dmas = < 15>, < 15>;
+   dma-names = "rx", "tx";
+   allwinner,codec-analog-controls = <_analog>;
+   status = "disabled";
+   };
+
uart0: serial@01c28000 {
compatible = "snps,dw-apb-uart";
reg = <0x01c28000 0x400>;
@@ -600,6 +614,11 @@
#reset-cells = <1>;
};
 
+   codec_analog: codec-analog@01f015c0 {
+   compatible = "allwinner,sun8i-h3-codec-analog";
+   reg = <0x01f015c0 0x4>;
+   };
+
ir: ir@01f02000 {
compatible = "allwinner,sun5i-a13-ir";
clocks = <_gates 1>, <_clk>;
-- 
2.10.2

[PATCH 02/10] ASoC: sunxi: Add support for A23/A33/H3 codec's analog path controls

[Chen-Yu Tsai]

The internal codec on A23/A33/H3 is split into 2 parts. The
analog path controls are routed through an embedded custom register
bus accessed through the PRCM block.

The SoCs share a common set of inputs, outputs, and audio paths.
The following table lists the differences.


| Feature \ SoC |  A23  |  A33  |  H3  |

| Headphone |   v   |   v   |  |

| Line Out  |   |   |  v   |

| Phone In/Out  |   v   |   v   |  |


Add an ASoC component driver for it. This should be tied to the codec
audio card as an auxiliary device. This patch adds the commont paths
and controls, and variant specific headphone out and line out.

Signed-off-by: Chen-Yu Tsai 
---
 sound/soc/sunxi/Kconfig  |   8 +
 sound/soc/sunxi/Makefile |   1 +
 sound/soc/sunxi/sun8i-codec-analog.c | 665 +++
 3 files changed, 674 insertions(+)
 create mode 100644 sound/soc/sunxi/sun8i-codec-analog.c

diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
index dd2368297fd3..6c344e16aca4 100644
--- a/sound/soc/sunxi/Kconfig
+++ b/sound/soc/sunxi/Kconfig
@@ -9,6 +9,14 @@ config SND_SUN4I_CODEC
  Select Y or M to add support for the Codec embedded in the Allwinner
  A10 and affiliated SoCs.
 
+config SND_SUN8I_CODEC_ANALOG
+   tristate "Allwinner sun8i Codec Analog Controls Support"
+   depends on MACH_SUN8I || COMPILE_TEST
+   select REGMAP
+   help
+ Say Y or M if you want to add support for the analog controls for
+ the codec embedded in newer Allwinner SoCs.
+
 config SND_SUN4I_I2S
tristate "Allwinner A10 I2S Support"
select SND_SOC_GENERIC_DMAENGINE_PCM
diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
index 604c7b842837..241c0df9ca0c 100644
--- a/sound/soc/sunxi/Makefile
+++ b/sound/soc/sunxi/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_SND_SUN4I_CODEC) += sun4i-codec.o
 obj-$(CONFIG_SND_SUN4I_I2S) += sun4i-i2s.o
 obj-$(CONFIG_SND_SUN4I_SPDIF) += sun4i-spdif.o
+obj-$(CONFIG_SND_SUN8I_CODEC_ANALOG) += sun8i-codec-analog.o
diff --git a/sound/soc/sunxi/sun8i-codec-analog.c 
b/sound/soc/sunxi/sun8i-codec-analog.c
new file mode 100644
index ..222bbd440b1e
--- /dev/null
+++ b/sound/soc/sunxi/sun8i-codec-analog.c
@@ -0,0 +1,665 @@
+/*
+ * This driver supports the analog controls for the internal codec
+ * found in Allwinner's A31s, A23, A33 and H3 SoCs.
+ *
+ * Copyright 2016 Chen-Yu Tsai 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/* Codec analog control register offsets and bit fields */
+#define SUN8I_ADDA_HP_VOLC 0x00
+#define SUN8I_ADDA_HP_VOLC_PA_CLK_GATE 7
+#define SUN8I_ADDA_HP_VOLC_HP_VOL  0
+#define SUN8I_ADDA_LOMIXSC 0x01
+#define SUN8I_ADDA_LOMIXSC_MIC16
+#define SUN8I_ADDA_LOMIXSC_MIC25
+#define SUN8I_ADDA_LOMIXSC_PHONE   4
+#define SUN8I_ADDA_LOMIXSC_PHONEN  3
+#define SUN8I_ADDA_LOMIXSC_LINEINL 2
+#define SUN8I_ADDA_LOMIXSC_DACL1
+#define SUN8I_ADDA_LOMIXSC_DACR0
+#define SUN8I_ADDA_ROMIXSC 0x02
+#define SUN8I_ADDA_ROMIXSC_MIC16
+#define SUN8I_ADDA_ROMIXSC_MIC25
+#define SUN8I_ADDA_ROMIXSC_PHONE   4
+#define SUN8I_ADDA_ROMIXSC_PHONEP  3
+#define SUN8I_ADDA_ROMIXSC_LINEINR 2
+#define SUN8I_ADDA_ROMIXSC_DACR1
+#define SUN8I_ADDA_ROMIXSC_DACL0
+#define SUN8I_ADDA_DAC_PA_SRC  0x03
+#define SUN8I_ADDA_DAC_PA_SRC_DACAREN  7
+#define SUN8I_ADDA_DAC_PA_SRC_DACALEN  6
+#define SUN8I_ADDA_DAC_PA_SRC_RMIXEN   5
+#define SUN8I_ADDA_DAC_PA_SRC_LMIXEN   4
+#define SUN8I_ADDA_DAC_PA_SRC_RHPPAMUTE3
+#define SUN8I_ADDA_DAC_PA_SRC_LHPPAMUTE2
+#define SUN8I_ADDA_DAC_PA_SRC_RHPIS1
+#define SUN8I_ADDA_DAC_PA_SRC_LHPIS0
+#define SUN8I_ADDA_PHONEIN_GCTRL   0x04
+#define SUN8I_ADDA_PHONEIN_GCTRL_PHONEPG   4
+#define 

[PATCH 00/10] ASoC: sunxi: Add support for audio codec in A23/H3 SoCs

[Chen-Yu Tsai]

Hi everyone,

This series adds support for the audio codec found in Allwinner A23 and
H3 SoCs. The design and data paths are similar to the audio codec found
in earlier SoCs such as the A31. The analog audio paths are symmetrical
with left/right channels and down-mix selectors for mono differential
output.

What deviates from previous SoCs is that the analog path controls have
been moved to a separate control bus, accessed through a message box
like register interface in the PRCM block. This necessitates writing
a separate component driver for it, which is then tied into the sound
card as an ASoC auxiliary device.

Patches 1 and 2 add the binding and driver for the analog path control
block. This is the more complete version. Previously I provided a not
fully tested version to Mylene Josserand from Free Electrons to use
with the A33. Their version was then trimmed down and switched to 
SOC_DAPM_SINGLE controls. Mine implements all the commonly used paths,
and uses the new stereo SOC_DAPM_DOUBLE controls. I also have a separate
patch for "Phone In" in case anyone wants to test them. It is not included
as my hardware does not use that input.

As for the A33, the analog controls are exactly the same as the A23, so
this driver can be reused, but the PCM and DAI bits are completely
different.

Patch 3 adds the analog path controls block to the sun6i-prcm driver as
a sub-device, for the A23. The H3 currently does not use the PRCM driver.

Patch 4 adds PCM and card support for the A23 codec to the sun4i-codec
driver.

Patch 5 adds a device node for the analog path controls block to the A23
dtsi.

Patch 6 adds a device node for the audio codec, and the phandle for the
analog path controls block to the A23 dtsi.

Patch 7 enables the audio codec for the A23 Q8 tablets. On these tablets
the headphone output is driven in DC coupled, or "direct drive", mode.

Patch 8 adds PCM and card support for the H3 codec to the sun4i-codec
driver.

Patch 9 adds device nodes for the audio codec and analog path controls
block to the H3 dtsi.

Patch 10 enables the audio codec on the Orange Pi PC. The audio output
jack on the board is tied to the line out pins on the SoC.


Please take a look and let me know what you think.

In addition, the sun4i-codec driver is getting pretty large. Maybe we
want to split the different parts into different files?


Regards
ChenYu


Chen-Yu Tsai (10):
  ASoC: sunxi: Add bindings for A23/A33/H3 codec's analog path controls
  ASoC: sunxi: Add support for A23/A33/H3 codec's analog path controls
  mfd: sun6i-prcm: Add codec analog controls sub-device for Allwinner
A23
  ASoC: sun4i-codec: Add support for A23 codec
  ARM: dts: sun8i: Add codec analog path controls node in PRCM for
A23/A33
  ARM: dts: sun8i-a23: Add device node for internal audio codec
  ARM: dts: sun8i-a23: q8-tablet: Enable internal audio codec
  ASoC: sun4i-codec: Add support for H3 codec
  ARM: dts: sun8i-h3: Add device nodes for audio codec and its analog
controls
  ARM: dts: sun8i-h3: orange-pi-pc: Enable audio codec

 .../devicetree/bindings/sound/sun4i-codec.txt  |  14 +-
 .../bindings/sound/sun8i-codec-analog.txt  |  16 +
 arch/arm/boot/dts/sun8i-a23-a33.dtsi   |   4 +
 arch/arm/boot/dts/sun8i-a23-q8-tablet.dts  |  23 +
 arch/arm/boot/dts/sun8i-a23.dtsi   |  16 +
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts |   8 +
 arch/arm/boot/dts/sun8i-h3.dtsi|  19 +
 drivers/mfd/sun6i-prcm.c   |  17 +
 sound/soc/sunxi/Kconfig|   8 +
 sound/soc/sunxi/Makefile   |   1 +
 sound/soc/sunxi/sun4i-codec.c  | 179 ++
 sound/soc/sunxi/sun8i-codec-analog.c   | 665 +
 12 files changed, 968 insertions(+), 2 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
 create mode 100644 sound/soc/sunxi/sun8i-codec-analog.c

-- 
2.10.2

[PATCH 09/10] ARM: dts: sun8i-h3: Add device nodes for audio codec and its analog controls

[Chen-Yu Tsai]

Now that we support the audio codec found on the Allwinner H3 SoC, add
device nodes for it.

Signed-off-by: Chen-Yu Tsai 
---
 arch/arm/boot/dts/sun8i-h3.dtsi | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index c38b028cac83..ceec979f57e4 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -485,6 +485,20 @@
status = "disabled";
};
 
+   codec: codec@01c22c00 {
+   #sound-dai-cells = <0>;
+   compatible = "allwinner,sun8i-h3-codec";
+   reg = <0x01c22c00 0x400>;
+   interrupts = ;
+   clocks = < CLK_BUS_CODEC>, < CLK_AC_DIG>;
+   clock-names = "apb", "codec";
+   resets = < RST_BUS_CODEC>;
+   dmas = < 15>, < 15>;
+   dma-names = "rx", "tx";
+   allwinner,codec-analog-controls = <_analog>;
+   status = "disabled";
+   };
+
uart0: serial@01c28000 {
compatible = "snps,dw-apb-uart";
reg = <0x01c28000 0x400>;
@@ -600,6 +614,11 @@
#reset-cells = <1>;
};
 
+   codec_analog: codec-analog@01f015c0 {
+   compatible = "allwinner,sun8i-h3-codec-analog";
+   reg = <0x01f015c0 0x4>;
+   };
+
ir: ir@01f02000 {
compatible = "allwinner,sun5i-a13-ir";
clocks = <_gates 1>, <_clk>;
-- 
2.10.2

[PATCH 02/10] ASoC: sunxi: Add support for A23/A33/H3 codec's analog path controls

[Chen-Yu Tsai]

The internal codec on A23/A33/H3 is split into 2 parts. The
analog path controls are routed through an embedded custom register
bus accessed through the PRCM block.

The SoCs share a common set of inputs, outputs, and audio paths.
The following table lists the differences.


| Feature \ SoC |  A23  |  A33  |  H3  |

| Headphone |   v   |   v   |  |

| Line Out  |   |   |  v   |

| Phone In/Out  |   v   |   v   |  |


Add an ASoC component driver for it. This should be tied to the codec
audio card as an auxiliary device. This patch adds the commont paths
and controls, and variant specific headphone out and line out.

Signed-off-by: Chen-Yu Tsai 
---
 sound/soc/sunxi/Kconfig  |   8 +
 sound/soc/sunxi/Makefile |   1 +
 sound/soc/sunxi/sun8i-codec-analog.c | 665 +++
 3 files changed, 674 insertions(+)
 create mode 100644 sound/soc/sunxi/sun8i-codec-analog.c

diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
index dd2368297fd3..6c344e16aca4 100644
--- a/sound/soc/sunxi/Kconfig
+++ b/sound/soc/sunxi/Kconfig
@@ -9,6 +9,14 @@ config SND_SUN4I_CODEC
  Select Y or M to add support for the Codec embedded in the Allwinner
  A10 and affiliated SoCs.
 
+config SND_SUN8I_CODEC_ANALOG
+   tristate "Allwinner sun8i Codec Analog Controls Support"
+   depends on MACH_SUN8I || COMPILE_TEST
+   select REGMAP
+   help
+ Say Y or M if you want to add support for the analog controls for
+ the codec embedded in newer Allwinner SoCs.
+
 config SND_SUN4I_I2S
tristate "Allwinner A10 I2S Support"
select SND_SOC_GENERIC_DMAENGINE_PCM
diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
index 604c7b842837..241c0df9ca0c 100644
--- a/sound/soc/sunxi/Makefile
+++ b/sound/soc/sunxi/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_SND_SUN4I_CODEC) += sun4i-codec.o
 obj-$(CONFIG_SND_SUN4I_I2S) += sun4i-i2s.o
 obj-$(CONFIG_SND_SUN4I_SPDIF) += sun4i-spdif.o
+obj-$(CONFIG_SND_SUN8I_CODEC_ANALOG) += sun8i-codec-analog.o
diff --git a/sound/soc/sunxi/sun8i-codec-analog.c 
b/sound/soc/sunxi/sun8i-codec-analog.c
new file mode 100644
index ..222bbd440b1e
--- /dev/null
+++ b/sound/soc/sunxi/sun8i-codec-analog.c
@@ -0,0 +1,665 @@
+/*
+ * This driver supports the analog controls for the internal codec
+ * found in Allwinner's A31s, A23, A33 and H3 SoCs.
+ *
+ * Copyright 2016 Chen-Yu Tsai 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/* Codec analog control register offsets and bit fields */
+#define SUN8I_ADDA_HP_VOLC 0x00
+#define SUN8I_ADDA_HP_VOLC_PA_CLK_GATE 7
+#define SUN8I_ADDA_HP_VOLC_HP_VOL  0
+#define SUN8I_ADDA_LOMIXSC 0x01
+#define SUN8I_ADDA_LOMIXSC_MIC16
+#define SUN8I_ADDA_LOMIXSC_MIC25
+#define SUN8I_ADDA_LOMIXSC_PHONE   4
+#define SUN8I_ADDA_LOMIXSC_PHONEN  3
+#define SUN8I_ADDA_LOMIXSC_LINEINL 2
+#define SUN8I_ADDA_LOMIXSC_DACL1
+#define SUN8I_ADDA_LOMIXSC_DACR0
+#define SUN8I_ADDA_ROMIXSC 0x02
+#define SUN8I_ADDA_ROMIXSC_MIC16
+#define SUN8I_ADDA_ROMIXSC_MIC25
+#define SUN8I_ADDA_ROMIXSC_PHONE   4
+#define SUN8I_ADDA_ROMIXSC_PHONEP  3
+#define SUN8I_ADDA_ROMIXSC_LINEINR 2
+#define SUN8I_ADDA_ROMIXSC_DACR1
+#define SUN8I_ADDA_ROMIXSC_DACL0
+#define SUN8I_ADDA_DAC_PA_SRC  0x03
+#define SUN8I_ADDA_DAC_PA_SRC_DACAREN  7
+#define SUN8I_ADDA_DAC_PA_SRC_DACALEN  6
+#define SUN8I_ADDA_DAC_PA_SRC_RMIXEN   5
+#define SUN8I_ADDA_DAC_PA_SRC_LMIXEN   4
+#define SUN8I_ADDA_DAC_PA_SRC_RHPPAMUTE3
+#define SUN8I_ADDA_DAC_PA_SRC_LHPPAMUTE2
+#define SUN8I_ADDA_DAC_PA_SRC_RHPIS1
+#define SUN8I_ADDA_DAC_PA_SRC_LHPIS0
+#define SUN8I_ADDA_PHONEIN_GCTRL   0x04
+#define SUN8I_ADDA_PHONEIN_GCTRL_PHONEPG   4
+#define SUN8I_ADDA_PHONEIN_GCTRL_PHONENG   0
+#define 

[PATCH 00/10] ASoC: sunxi: Add support for audio codec in A23/H3 SoCs

[Chen-Yu Tsai]

Hi everyone,

This series adds support for the audio codec found in Allwinner A23 and
H3 SoCs. The design and data paths are similar to the audio codec found
in earlier SoCs such as the A31. The analog audio paths are symmetrical
with left/right channels and down-mix selectors for mono differential
output.

What deviates from previous SoCs is that the analog path controls have
been moved to a separate control bus, accessed through a message box
like register interface in the PRCM block. This necessitates writing
a separate component driver for it, which is then tied into the sound
card as an ASoC auxiliary device.

Patches 1 and 2 add the binding and driver for the analog path control
block. This is the more complete version. Previously I provided a not
fully tested version to Mylene Josserand from Free Electrons to use
with the A33. Their version was then trimmed down and switched to 
SOC_DAPM_SINGLE controls. Mine implements all the commonly used paths,
and uses the new stereo SOC_DAPM_DOUBLE controls. I also have a separate
patch for "Phone In" in case anyone wants to test them. It is not included
as my hardware does not use that input.

As for the A33, the analog controls are exactly the same as the A23, so
this driver can be reused, but the PCM and DAI bits are completely
different.

Patch 3 adds the analog path controls block to the sun6i-prcm driver as
a sub-device, for the A23. The H3 currently does not use the PRCM driver.

Patch 4 adds PCM and card support for the A23 codec to the sun4i-codec
driver.

Patch 5 adds a device node for the analog path controls block to the A23
dtsi.

Patch 6 adds a device node for the audio codec, and the phandle for the
analog path controls block to the A23 dtsi.

Patch 7 enables the audio codec for the A23 Q8 tablets. On these tablets
the headphone output is driven in DC coupled, or "direct drive", mode.

Patch 8 adds PCM and card support for the H3 codec to the sun4i-codec
driver.

Patch 9 adds device nodes for the audio codec and analog path controls
block to the H3 dtsi.

Patch 10 enables the audio codec on the Orange Pi PC. The audio output
jack on the board is tied to the line out pins on the SoC.


Please take a look and let me know what you think.

In addition, the sun4i-codec driver is getting pretty large. Maybe we
want to split the different parts into different files?


Regards
ChenYu


Chen-Yu Tsai (10):
  ASoC: sunxi: Add bindings for A23/A33/H3 codec's analog path controls
  ASoC: sunxi: Add support for A23/A33/H3 codec's analog path controls
  mfd: sun6i-prcm: Add codec analog controls sub-device for Allwinner
A23
  ASoC: sun4i-codec: Add support for A23 codec
  ARM: dts: sun8i: Add codec analog path controls node in PRCM for
A23/A33
  ARM: dts: sun8i-a23: Add device node for internal audio codec
  ARM: dts: sun8i-a23: q8-tablet: Enable internal audio codec
  ASoC: sun4i-codec: Add support for H3 codec
  ARM: dts: sun8i-h3: Add device nodes for audio codec and its analog
controls
  ARM: dts: sun8i-h3: orange-pi-pc: Enable audio codec

 .../devicetree/bindings/sound/sun4i-codec.txt  |  14 +-
 .../bindings/sound/sun8i-codec-analog.txt  |  16 +
 arch/arm/boot/dts/sun8i-a23-a33.dtsi   |   4 +
 arch/arm/boot/dts/sun8i-a23-q8-tablet.dts  |  23 +
 arch/arm/boot/dts/sun8i-a23.dtsi   |  16 +
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts |   8 +
 arch/arm/boot/dts/sun8i-h3.dtsi|  19 +
 drivers/mfd/sun6i-prcm.c   |  17 +
 sound/soc/sunxi/Kconfig|   8 +
 sound/soc/sunxi/Makefile   |   1 +
 sound/soc/sunxi/sun4i-codec.c  | 179 ++
 sound/soc/sunxi/sun8i-codec-analog.c   | 665 +
 12 files changed, 968 insertions(+), 2 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
 create mode 100644 sound/soc/sunxi/sun8i-codec-analog.c

-- 
2.10.2

[PATCH 07/10] ARM: dts: sun8i-a23: q8-tablet: Enable internal audio codec

[Chen-Yu Tsai]

The A23 Q8 tablets have an internal mono speaker w/ external amp
which has a shutdown control tied to a GPIO pin. Both the speaker
amp and the headphone jack are tied to the HP output pins. While
the speaker is mono, the headset jack is stereo. Unfortunately
the driver does not support automatic switching of this.

In addition, the headset is DC coupled, or "direct drive" enabled.
The headset's microphone is tied to MIC2 with HBIAS providing power.
A separate internal microphone is tied to MIC1 with MBIAS providing
power.

Signed-off-by: Chen-Yu Tsai 
---
 arch/arm/boot/dts/sun8i-a23-q8-tablet.dts | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a23-q8-tablet.dts 
b/arch/arm/boot/dts/sun8i-a23-q8-tablet.dts
index 956320a6cc78..3ab5c0c09d93 100644
--- a/arch/arm/boot/dts/sun8i-a23-q8-tablet.dts
+++ b/arch/arm/boot/dts/sun8i-a23-q8-tablet.dts
@@ -48,3 +48,26 @@
model = "Q8 A23 Tablet";
compatible = "allwinner,q8-a23", "allwinner,sun8i-a23";
 };
+
+ {
+   pinctrl-0 = <_pa_pin>;
+   allwinner,pa-gpios = < 7 9 GPIO_ACTIVE_HIGH>; /* PH9 */
+   allwinner,audio-routing =
+   "Headphone", "HP",
+   "Headphone", "HPCOM",
+   "Speaker", "HP",
+   "MIC1", "Mic",
+   "MIC2", "Headset Mic",
+   "Mic",  "MBIAS",
+   "Headset Mic", "HBIAS";
+   status = "okay";
+};
+
+ {
+   codec_pa_pin: codec_pa_pin@0 {
+   allwinner,pins = "PH9";
+   allwinner,function = "gpio_out";
+   allwinner,drive = ;
+   allwinner,pull = ;
+   };
+};
-- 
2.10.2

[PATCH 06/10] ARM: dts: sun8i-a23: Add device node for internal audio codec

[Chen-Yu Tsai]

Now that we have a device tree binding and driver for the A23's
internal audio codec, add a device node for it.

Signed-off-by: Chen-Yu Tsai 
---
 arch/arm/boot/dts/sun8i-a23.dtsi | 16 
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a23.dtsi b/arch/arm/boot/dts/sun8i-a23.dtsi
index 54d045dab825..4d1f929780a8 100644
--- a/arch/arm/boot/dts/sun8i-a23.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23.dtsi
@@ -48,6 +48,22 @@
memory {
reg = <0x4000 0x4000>;
};
+
+   soc@01c0 {
+   codec: codec@01c22c00 {
+   #sound-dai-cells = <0>;
+   compatible = "allwinner,sun8i-a23-codec";
+   reg = <0x01c22c00 0x400>;
+   interrupts = ;
+   clocks = < CLK_BUS_CODEC>, < CLK_AC_DIG>;
+   clock-names = "apb", "codec";
+   resets = < RST_BUS_CODEC>;
+   dmas = < 15>, < 15>;
+   dma-names = "rx", "tx";
+   allwinner,codec-analog-controls = <_analog>;
+   status = "disabled";
+   };
+   };
 };
 
  {
-- 
2.10.2

[PATCH 08/10] ASoC: sun4i-codec: Add support for H3 codec

[Chen-Yu Tsai]

The codec on the H3 is similar to the one found on the A31. One key
difference is the analog path controls are routed through the PRCM
block. This is supported by the sun8i-codec-analog driver, and tied
into this codec driver with the audio card's aux_dev.

In addition, the H3 has no HP (headphone) and HBIAS support, and no
MIC3 input. The FIFO related registers are slightly rearranged.

Signed-off-by: Chen-Yu Tsai 
---
 .../devicetree/bindings/sound/sun4i-codec.txt  |  3 +
 sound/soc/sunxi/sun4i-codec.c  | 71 ++
 2 files changed, 74 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/sun4i-codec.txt 
b/Documentation/devicetree/bindings/sound/sun4i-codec.txt
index f7a548b604fc..3033bd8aab0f 100644
--- a/Documentation/devicetree/bindings/sound/sun4i-codec.txt
+++ b/Documentation/devicetree/bindings/sound/sun4i-codec.txt
@@ -6,6 +6,7 @@ Required properties:
- "allwinner,sun6i-a31-codec"
- "allwinner,sun7i-a20-codec"
- "allwinner,sun8i-a23-codec"
+   - "allwinner,sun8i-h3-codec"
 - reg: must contain the registers location and length
 - interrupts: must contain the codec interrupt
 - dmas: DMA channels for tx and rx dma. See the DMA client binding,
@@ -23,6 +24,7 @@ Optional properties:
 Required properties for the following compatibles:
- "allwinner,sun6i-a31-codec"
- "allwinner,sun8i-a23-codec"
+   - "allwinner,sun8i-h3-codec"
 - resets: phandle to the reset control for this device
 - allwinner,audio-routing: A list of the connections between audio components.
   Each entry is a pair of strings, the first being the
@@ -52,6 +54,7 @@ Required properties for the following compatibles:
 
 Required properties for the following compatibles:
- "allwinner,sun8i-a23-codec"
+   - "allwinner,sun8i-h3-codec"
 - allwinner,codec-analog-controls: A phandle to the codec analog controls
   block in the PRCM.
 
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 3c5ef1724163..1a5acadf65d7 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -217,6 +217,13 @@
 #define SUN8I_A23_CODEC_DAC_TXCNT  (0x1c)
 #define SUN8I_A23_CODEC_ADC_RXCNT  (0x20)
 
+/* TX FIFO moved on H3 */
+#define SUN8I_H3_CODEC_DAC_TXDATA  (0x20)
+#define SUN8I_H3_CODEC_DAC_DBG (0x48)
+#define SUN8I_H3_CODEC_ADC_DBG (0x4c)
+
+/* TODO H3 DAP (Digital Audio Processing) bits */
+
 struct sun4i_codec {
struct device   *dev;
struct regmap   *regmap;
@@ -1293,6 +1300,44 @@ static struct snd_soc_card 
*sun8i_a23_codec_create_card(struct device *dev)
return card;
 };
 
+static struct snd_soc_card *sun8i_h3_codec_create_card(struct device *dev)
+{
+   struct snd_soc_card *card;
+   int ret;
+
+   card = devm_kzalloc(dev, sizeof(*card), GFP_KERNEL);
+   if (!card)
+   return ERR_PTR(-ENOMEM);
+
+   aux_dev.codec_of_node = of_parse_phandle(dev->of_node,
+
"allwinner,codec-analog-controls",
+0);
+   if (!aux_dev.codec_of_node) {
+   dev_err(dev, "Can't find analog controls for codec.\n");
+   return ERR_PTR(-EINVAL);
+   };
+
+   card->dai_link = sun4i_codec_create_link(dev, >num_links);
+   if (!card->dai_link)
+   return ERR_PTR(-ENOMEM);
+
+   card->dev   = dev;
+   card->name  = "H3 Audio Codec";
+   card->dapm_widgets  = sun6i_codec_card_dapm_widgets;
+   card->num_dapm_widgets  = ARRAY_SIZE(sun6i_codec_card_dapm_widgets);
+   card->dapm_routes   = sun8i_codec_card_routes;
+   card->num_dapm_routes   = ARRAY_SIZE(sun8i_codec_card_routes);
+   card->aux_dev   = _dev;
+   card->num_aux_devs  = 1;
+   card->fully_routed  = true;
+
+   ret = snd_soc_of_parse_audio_routing(card, "allwinner,audio-routing");
+   if (ret)
+   dev_warn(dev, "failed to parse audio-routing: %d\n", ret);
+
+   return card;
+};
+
 static const struct regmap_config sun4i_codec_regmap_config = {
.reg_bits   = 32,
.reg_stride = 4,
@@ -1321,6 +1366,13 @@ static const struct regmap_config 
sun8i_a23_codec_regmap_config = {
.max_register   = SUN8I_A23_CODEC_ADC_RXCNT,
 };
 
+static const struct regmap_config sun8i_h3_codec_regmap_config = {
+   .reg_bits   = 32,
+   .reg_stride = 4,
+   .val_bits   = 32,
+   .max_register   = SUN8I_H3_CODEC_ADC_DBG,
+};
+
 struct sun4i_codec_quirks {
const struct regmap_config *regmap_config;
const struct snd_soc_codec_driver *codec;
@@ -1369,6 +1421,21 @@ static const struct sun4i_codec_quirks 

[PATCH 07/10] ARM: dts: sun8i-a23: q8-tablet: Enable internal audio codec

[Chen-Yu Tsai]

The A23 Q8 tablets have an internal mono speaker w/ external amp
which has a shutdown control tied to a GPIO pin. Both the speaker
amp and the headphone jack are tied to the HP output pins. While
the speaker is mono, the headset jack is stereo. Unfortunately
the driver does not support automatic switching of this.

In addition, the headset is DC coupled, or "direct drive" enabled.
The headset's microphone is tied to MIC2 with HBIAS providing power.
A separate internal microphone is tied to MIC1 with MBIAS providing
power.

Signed-off-by: Chen-Yu Tsai 
---
 arch/arm/boot/dts/sun8i-a23-q8-tablet.dts | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a23-q8-tablet.dts 
b/arch/arm/boot/dts/sun8i-a23-q8-tablet.dts
index 956320a6cc78..3ab5c0c09d93 100644
--- a/arch/arm/boot/dts/sun8i-a23-q8-tablet.dts
+++ b/arch/arm/boot/dts/sun8i-a23-q8-tablet.dts
@@ -48,3 +48,26 @@
model = "Q8 A23 Tablet";
compatible = "allwinner,q8-a23", "allwinner,sun8i-a23";
 };
+
+ {
+   pinctrl-0 = <_pa_pin>;
+   allwinner,pa-gpios = < 7 9 GPIO_ACTIVE_HIGH>; /* PH9 */
+   allwinner,audio-routing =
+   "Headphone", "HP",
+   "Headphone", "HPCOM",
+   "Speaker", "HP",
+   "MIC1", "Mic",
+   "MIC2", "Headset Mic",
+   "Mic",  "MBIAS",
+   "Headset Mic", "HBIAS";
+   status = "okay";
+};
+
+ {
+   codec_pa_pin: codec_pa_pin@0 {
+   allwinner,pins = "PH9";
+   allwinner,function = "gpio_out";
+   allwinner,drive = ;
+   allwinner,pull = ;
+   };
+};
-- 
2.10.2

[PATCH 06/10] ARM: dts: sun8i-a23: Add device node for internal audio codec

[Chen-Yu Tsai]

Now that we have a device tree binding and driver for the A23's
internal audio codec, add a device node for it.

Signed-off-by: Chen-Yu Tsai 
---
 arch/arm/boot/dts/sun8i-a23.dtsi | 16 
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a23.dtsi b/arch/arm/boot/dts/sun8i-a23.dtsi
index 54d045dab825..4d1f929780a8 100644
--- a/arch/arm/boot/dts/sun8i-a23.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23.dtsi
@@ -48,6 +48,22 @@
memory {
reg = <0x4000 0x4000>;
};
+
+   soc@01c0 {
+   codec: codec@01c22c00 {
+   #sound-dai-cells = <0>;
+   compatible = "allwinner,sun8i-a23-codec";
+   reg = <0x01c22c00 0x400>;
+   interrupts = ;
+   clocks = < CLK_BUS_CODEC>, < CLK_AC_DIG>;
+   clock-names = "apb", "codec";
+   resets = < RST_BUS_CODEC>;
+   dmas = < 15>, < 15>;
+   dma-names = "rx", "tx";
+   allwinner,codec-analog-controls = <_analog>;
+   status = "disabled";
+   };
+   };
 };
 
  {
-- 
2.10.2

[PATCH 08/10] ASoC: sun4i-codec: Add support for H3 codec

[Chen-Yu Tsai]

The codec on the H3 is similar to the one found on the A31. One key
difference is the analog path controls are routed through the PRCM
block. This is supported by the sun8i-codec-analog driver, and tied
into this codec driver with the audio card's aux_dev.

In addition, the H3 has no HP (headphone) and HBIAS support, and no
MIC3 input. The FIFO related registers are slightly rearranged.

Signed-off-by: Chen-Yu Tsai 
---
 .../devicetree/bindings/sound/sun4i-codec.txt  |  3 +
 sound/soc/sunxi/sun4i-codec.c  | 71 ++
 2 files changed, 74 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/sun4i-codec.txt 
b/Documentation/devicetree/bindings/sound/sun4i-codec.txt
index f7a548b604fc..3033bd8aab0f 100644
--- a/Documentation/devicetree/bindings/sound/sun4i-codec.txt
+++ b/Documentation/devicetree/bindings/sound/sun4i-codec.txt
@@ -6,6 +6,7 @@ Required properties:
- "allwinner,sun6i-a31-codec"
- "allwinner,sun7i-a20-codec"
- "allwinner,sun8i-a23-codec"
+   - "allwinner,sun8i-h3-codec"
 - reg: must contain the registers location and length
 - interrupts: must contain the codec interrupt
 - dmas: DMA channels for tx and rx dma. See the DMA client binding,
@@ -23,6 +24,7 @@ Optional properties:
 Required properties for the following compatibles:
- "allwinner,sun6i-a31-codec"
- "allwinner,sun8i-a23-codec"
+   - "allwinner,sun8i-h3-codec"
 - resets: phandle to the reset control for this device
 - allwinner,audio-routing: A list of the connections between audio components.
   Each entry is a pair of strings, the first being the
@@ -52,6 +54,7 @@ Required properties for the following compatibles:
 
 Required properties for the following compatibles:
- "allwinner,sun8i-a23-codec"
+   - "allwinner,sun8i-h3-codec"
 - allwinner,codec-analog-controls: A phandle to the codec analog controls
   block in the PRCM.
 
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 3c5ef1724163..1a5acadf65d7 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -217,6 +217,13 @@
 #define SUN8I_A23_CODEC_DAC_TXCNT  (0x1c)
 #define SUN8I_A23_CODEC_ADC_RXCNT  (0x20)
 
+/* TX FIFO moved on H3 */
+#define SUN8I_H3_CODEC_DAC_TXDATA  (0x20)
+#define SUN8I_H3_CODEC_DAC_DBG (0x48)
+#define SUN8I_H3_CODEC_ADC_DBG (0x4c)
+
+/* TODO H3 DAP (Digital Audio Processing) bits */
+
 struct sun4i_codec {
struct device   *dev;
struct regmap   *regmap;
@@ -1293,6 +1300,44 @@ static struct snd_soc_card 
*sun8i_a23_codec_create_card(struct device *dev)
return card;
 };
 
+static struct snd_soc_card *sun8i_h3_codec_create_card(struct device *dev)
+{
+   struct snd_soc_card *card;
+   int ret;
+
+   card = devm_kzalloc(dev, sizeof(*card), GFP_KERNEL);
+   if (!card)
+   return ERR_PTR(-ENOMEM);
+
+   aux_dev.codec_of_node = of_parse_phandle(dev->of_node,
+
"allwinner,codec-analog-controls",
+0);
+   if (!aux_dev.codec_of_node) {
+   dev_err(dev, "Can't find analog controls for codec.\n");
+   return ERR_PTR(-EINVAL);
+   };
+
+   card->dai_link = sun4i_codec_create_link(dev, >num_links);
+   if (!card->dai_link)
+   return ERR_PTR(-ENOMEM);
+
+   card->dev   = dev;
+   card->name  = "H3 Audio Codec";
+   card->dapm_widgets  = sun6i_codec_card_dapm_widgets;
+   card->num_dapm_widgets  = ARRAY_SIZE(sun6i_codec_card_dapm_widgets);
+   card->dapm_routes   = sun8i_codec_card_routes;
+   card->num_dapm_routes   = ARRAY_SIZE(sun8i_codec_card_routes);
+   card->aux_dev   = _dev;
+   card->num_aux_devs  = 1;
+   card->fully_routed  = true;
+
+   ret = snd_soc_of_parse_audio_routing(card, "allwinner,audio-routing");
+   if (ret)
+   dev_warn(dev, "failed to parse audio-routing: %d\n", ret);
+
+   return card;
+};
+
 static const struct regmap_config sun4i_codec_regmap_config = {
.reg_bits   = 32,
.reg_stride = 4,
@@ -1321,6 +1366,13 @@ static const struct regmap_config 
sun8i_a23_codec_regmap_config = {
.max_register   = SUN8I_A23_CODEC_ADC_RXCNT,
 };
 
+static const struct regmap_config sun8i_h3_codec_regmap_config = {
+   .reg_bits   = 32,
+   .reg_stride = 4,
+   .val_bits   = 32,
+   .max_register   = SUN8I_H3_CODEC_ADC_DBG,
+};
+
 struct sun4i_codec_quirks {
const struct regmap_config *regmap_config;
const struct snd_soc_codec_driver *codec;
@@ -1369,6 +1421,21 @@ static const struct sun4i_codec_quirks 
sun8i_a23_codec_quirks 

[PATCH 10/10] ARM: dts: sun8i-h3: orange-pi-pc: Enable audio codec

[Chen-Yu Tsai]

The Orange Pi PC routes the LINEOUT pins to the audio out jack on the
board. The onboard microphone is routed to MIC1, with MBIAS providing
power.

Signed-off-by: Chen-Yu Tsai 
---
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts 
b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index 3ec971285aa3..2b1e68d0f2fa 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -90,6 +90,14 @@
};
 };
 
+ {
+   allwinner,audio-routing =
+   "Line Out", "LINEOUT",
+   "MIC1", "Mic",
+   "Mic",  "MBIAS";
+   status = "okay";
+};
+
  {
status = "okay";
 };
-- 
2.10.2

[PATCH 01/10] ASoC: sunxi: Add bindings for A23/A33/H3 codec's analog path controls

[Chen-Yu Tsai]

The internal codec on A23/A33/H3 is split into 2 parts. The
analog path controls are routed through an embedded custom register
bus accessed through the PRCM block.

The SoCs share a common set of inputs, outputs, and audio paths.
The following table lists the differences.


| Feature \ SoC |  A23  |  A33  |  H3  |

| Headphone |   v   |   v   |  |

| Line Out  |   |   |  v   |

| Phone In/Out  |   v   |   v   |  |


Add a binding for this hardware.

Signed-off-by: Chen-Yu Tsai 
---
 .../devicetree/bindings/sound/sun8i-codec-analog.txt | 16 
 1 file changed, 16 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt

diff --git a/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt 
b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
new file mode 100644
index ..779b735781ba
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
@@ -0,0 +1,16 @@
+* Allwinner Codec Analog Controls
+
+Required properties:
+- compatible: must be one of the following compatibles:
+   - "allwinner,sun8i-a23-codec-analog"
+   - "allwinner,sun8i-h3-codec-analog"
+
+Required properties if not a sub-node of the PRCM node:
+- reg: must contain the registers location and length
+
+Example:
+prcm: prcm@01f01400 {
+   codec_analog: codec-analog {
+   compatible = "allwinner,sun8i-a23-codec-analog";
+   };
+};
-- 
2.10.2

[PATCH 05/10] ARM: dts: sun8i: Add codec analog path controls node in PRCM for A23/A33

[Chen-Yu Tsai]

On the A23/A33, the internal codec's analog path controls are located in
the PRCM node.

Add a sub-device node to the PRCM for it.

Signed-off-by: Chen-Yu Tsai 
---
 arch/arm/boot/dts/sun8i-a23-a33.dtsi | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi 
b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index 300a1bd5a6ec..ac0d281f4489 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -553,6 +553,10 @@
compatible = "allwinner,sun6i-a31-clock-reset";
#reset-cells = <1>;
};
+
+   codec_analog: codec-analog {
+   compatible = "allwinner,sun8i-a23-codec-analog";
+   };
};
 
cpucfg@01f01c00 {
-- 
2.10.2

[PATCH 01/10] ASoC: sunxi: Add bindings for A23/A33/H3 codec's analog path controls

[Chen-Yu Tsai]

The internal codec on A23/A33/H3 is split into 2 parts. The
analog path controls are routed through an embedded custom register
bus accessed through the PRCM block.

The SoCs share a common set of inputs, outputs, and audio paths.
The following table lists the differences.


| Feature \ SoC |  A23  |  A33  |  H3  |

| Headphone |   v   |   v   |  |

| Line Out  |   |   |  v   |

| Phone In/Out  |   v   |   v   |  |


Add a binding for this hardware.

Signed-off-by: Chen-Yu Tsai 
---
 .../devicetree/bindings/sound/sun8i-codec-analog.txt | 16 
 1 file changed, 16 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt

diff --git a/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt 
b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
new file mode 100644
index ..779b735781ba
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
@@ -0,0 +1,16 @@
+* Allwinner Codec Analog Controls
+
+Required properties:
+- compatible: must be one of the following compatibles:
+   - "allwinner,sun8i-a23-codec-analog"
+   - "allwinner,sun8i-h3-codec-analog"
+
+Required properties if not a sub-node of the PRCM node:
+- reg: must contain the registers location and length
+
+Example:
+prcm: prcm@01f01400 {
+   codec_analog: codec-analog {
+   compatible = "allwinner,sun8i-a23-codec-analog";
+   };
+};
-- 
2.10.2

[PATCH 05/10] ARM: dts: sun8i: Add codec analog path controls node in PRCM for A23/A33

[Chen-Yu Tsai]

On the A23/A33, the internal codec's analog path controls are located in
the PRCM node.

Add a sub-device node to the PRCM for it.

Signed-off-by: Chen-Yu Tsai 
---
 arch/arm/boot/dts/sun8i-a23-a33.dtsi | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi 
b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index 300a1bd5a6ec..ac0d281f4489 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -553,6 +553,10 @@
compatible = "allwinner,sun6i-a31-clock-reset";
#reset-cells = <1>;
};
+
+   codec_analog: codec-analog {
+   compatible = "allwinner,sun8i-a23-codec-analog";
+   };
};
 
cpucfg@01f01c00 {
-- 
2.10.2

[PATCH 10/10] ARM: dts: sun8i-h3: orange-pi-pc: Enable audio codec

[Chen-Yu Tsai]

The Orange Pi PC routes the LINEOUT pins to the audio out jack on the
board. The onboard microphone is routed to MIC1, with MBIAS providing
power.

Signed-off-by: Chen-Yu Tsai 
---
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts 
b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index 3ec971285aa3..2b1e68d0f2fa 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -90,6 +90,14 @@
};
 };
 
+ {
+   allwinner,audio-routing =
+   "Line Out", "LINEOUT",
+   "MIC1", "Mic",
+   "Mic",  "MBIAS";
+   status = "okay";
+};
+
  {
status = "okay";
 };
-- 
2.10.2

[PATCH 03/10] mfd: sun6i-prcm: Add codec analog controls sub-device for Allwinner A23

[Chen-Yu Tsai]

The PRCM block on the A23 contains a message box like interface to
the registers for the analog path controls of the internal codec.

Add a sub-device for it.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/mfd/sun6i-prcm.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/mfd/sun6i-prcm.c b/drivers/mfd/sun6i-prcm.c
index 011fcc555945..4abbf2ede944 100644
--- a/drivers/mfd/sun6i-prcm.c
+++ b/drivers/mfd/sun6i-prcm.c
@@ -12,6 +12,9 @@
 #include 
 #include 
 
+#define SUN8I_CODEC_ANALOG_BASE0x1c0
+#define SUN8I_CODEC_ANALOG_END (SUN8I_CODEC_ANALOG_BASE + 0x4 - 1)
+
 struct prcm_data {
int nsubdevs;
const struct mfd_cell *subdevs;
@@ -57,6 +60,14 @@ static const struct resource sun6i_a31_apb0_rstc_res[] = {
},
 };
 
+static const struct resource sun8i_codec_analog_res[] = {
+   {
+   .start  = SUN8I_CODEC_ANALOG_BASE,
+   .end= SUN8I_CODEC_ANALOG_END,
+   .flags  = IORESOURCE_MEM,
+   },
+};
+
 static const struct mfd_cell sun6i_a31_prcm_subdevs[] = {
{
.name = "sun6i-a31-ar100-clk",
@@ -109,6 +120,12 @@ static const struct mfd_cell sun8i_a23_prcm_subdevs[] = {
.num_resources = ARRAY_SIZE(sun6i_a31_apb0_rstc_res),
.resources = sun6i_a31_apb0_rstc_res,
},
+   {
+   .name   = "sun8i-codec-analog",
+   .of_compatible  = "allwinner,sun8i-a23-codec-analog",
+   .num_resources  = ARRAY_SIZE(sun8i_codec_analog_res),
+   .resources  = sun8i_codec_analog_res,
+   },
 };
 
 static const struct prcm_data sun6i_a31_prcm_data = {
-- 
2.10.2

[PATCH 04/10] ASoC: sun4i-codec: Add support for A23 codec

[Chen-Yu Tsai]

The codec in the A23 is similar to the one found on the A31. One key
difference is the analog path controls are routed through the PRCM
block. This is supported by the sun8i-codec-analog driver, and tied
into this codec driver with the audio card's aux_dev.

In addition, the A23 does not have LINEOUT, and it does not support
headset jack detection or buttons.

Signed-off-by: Chen-Yu Tsai 
---
 .../devicetree/bindings/sound/sun4i-codec.txt  |  11 ++-
 sound/soc/sunxi/sun4i-codec.c  | 108 +
 2 files changed, 117 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/sun4i-codec.txt 
b/Documentation/devicetree/bindings/sound/sun4i-codec.txt
index d91a95377f49..f7a548b604fc 100644
--- a/Documentation/devicetree/bindings/sound/sun4i-codec.txt
+++ b/Documentation/devicetree/bindings/sound/sun4i-codec.txt
@@ -5,6 +5,7 @@ Required properties:
- "allwinner,sun4i-a10-codec"
- "allwinner,sun6i-a31-codec"
- "allwinner,sun7i-a20-codec"
+   - "allwinner,sun8i-a23-codec"
 - reg: must contain the registers location and length
 - interrupts: must contain the codec interrupt
 - dmas: DMA channels for tx and rx dma. See the DMA client binding,
@@ -21,6 +22,7 @@ Optional properties:
 
 Required properties for the following compatibles:
- "allwinner,sun6i-a31-codec"
+   - "allwinner,sun8i-a23-codec"
 - resets: phandle to the reset control for this device
 - allwinner,audio-routing: A list of the connections between audio components.
   Each entry is a pair of strings, the first being the
@@ -31,10 +33,10 @@ Required properties for the following compatibles:
   "HP"
   "HPCOM"
   "LINEIN"
-  "LINEOUT"
+  "LINEOUT"(not on sun8i-a23)
   "MIC1"
   "MIC2"
-  "MIC3"
+  "MIC3"   (sun6i-a31 only)
 
   Microphone biases from the SoC:
   "HBIAS"
@@ -48,6 +50,11 @@ Required properties for the following compatibles:
   "Mic"
   "Speaker"
 
+Required properties for the following compatibles:
+   - "allwinner,sun8i-a23-codec"
+- allwinner,codec-analog-controls: A phandle to the codec analog controls
+  block in the PRCM.
+
 Example:
 codec: codec@01c22c00 {
#sound-dai-cells = <0>;
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 6379efd21f00..3c5ef1724163 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -213,6 +213,10 @@
 
 /* TODO sun6i DAP (Digital Audio Processing) bits */
 
+/* FIFO counters moved on A23 */
+#define SUN8I_A23_CODEC_DAC_TXCNT  (0x1c)
+#define SUN8I_A23_CODEC_ADC_RXCNT  (0x20)
+
 struct sun4i_codec {
struct device   *dev;
struct regmap   *regmap;
@@ -1067,6 +1071,32 @@ static struct snd_soc_codec_driver sun6i_codec_codec = {
},
 };
 
+/* sun8i A23 codec */
+static const struct snd_kcontrol_new sun8i_a23_codec_codec_controls[] = {
+   SOC_SINGLE_TLV("DAC Playback Volume", SUN4I_CODEC_DAC_DPC,
+  SUN4I_CODEC_DAC_DPC_DVOL, 0x3f, 1,
+  sun6i_codec_dvol_scale),
+};
+
+static const struct snd_soc_dapm_widget sun8i_a23_codec_codec_widgets[] = {
+   /* Digital parts of the ADCs */
+   SND_SOC_DAPM_SUPPLY("ADC Enable", SUN6I_CODEC_ADC_FIFOC,
+   SUN6I_CODEC_ADC_FIFOC_EN_AD, 0, NULL, 0),
+   /* Digital parts of the DACs */
+   SND_SOC_DAPM_SUPPLY("DAC Enable", SUN4I_CODEC_DAC_DPC,
+   SUN4I_CODEC_DAC_DPC_EN_DA, 0, NULL, 0),
+
+};
+
+static struct snd_soc_codec_driver sun8i_a23_codec_codec = {
+   .component_driver = {
+   .controls   = sun8i_a23_codec_codec_controls,
+   .num_controls   = 
ARRAY_SIZE(sun8i_a23_codec_codec_controls),
+   .dapm_widgets   = sun8i_a23_codec_codec_widgets,
+   .num_dapm_widgets   = 
ARRAY_SIZE(sun8i_a23_codec_codec_widgets),
+   },
+};
+
 static const struct snd_soc_component_driver sun4i_codec_component = {
.name = "sun4i-codec",
 };
@@ -1206,6 +1236,63 @@ static struct snd_soc_card 
*sun6i_codec_create_card(struct device *dev)
return card;
 };
 
+/* Connect digital side enables to analog side widgets */
+static const struct snd_soc_dapm_route sun8i_codec_card_routes[] = {
+   /* ADC Routes */
+   { "Left ADC", NULL, "ADC Enable" },
+   { "Right ADC", NULL, "ADC Enable" },
+   { "Codec Capture", NULL, "Left ADC" },
+   { "Codec Capture", NULL, "Right ADC" },
+
+   /* DAC Routes */
+   { "Left 

[PATCH 03/10] mfd: sun6i-prcm: Add codec analog controls sub-device for Allwinner A23

[Chen-Yu Tsai]

The PRCM block on the A23 contains a message box like interface to
the registers for the analog path controls of the internal codec.

Add a sub-device for it.

Signed-off-by: Chen-Yu Tsai 
---
 drivers/mfd/sun6i-prcm.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/mfd/sun6i-prcm.c b/drivers/mfd/sun6i-prcm.c
index 011fcc555945..4abbf2ede944 100644
--- a/drivers/mfd/sun6i-prcm.c
+++ b/drivers/mfd/sun6i-prcm.c
@@ -12,6 +12,9 @@
 #include 
 #include 
 
+#define SUN8I_CODEC_ANALOG_BASE0x1c0
+#define SUN8I_CODEC_ANALOG_END (SUN8I_CODEC_ANALOG_BASE + 0x4 - 1)
+
 struct prcm_data {
int nsubdevs;
const struct mfd_cell *subdevs;
@@ -57,6 +60,14 @@ static const struct resource sun6i_a31_apb0_rstc_res[] = {
},
 };
 
+static const struct resource sun8i_codec_analog_res[] = {
+   {
+   .start  = SUN8I_CODEC_ANALOG_BASE,
+   .end= SUN8I_CODEC_ANALOG_END,
+   .flags  = IORESOURCE_MEM,
+   },
+};
+
 static const struct mfd_cell sun6i_a31_prcm_subdevs[] = {
{
.name = "sun6i-a31-ar100-clk",
@@ -109,6 +120,12 @@ static const struct mfd_cell sun8i_a23_prcm_subdevs[] = {
.num_resources = ARRAY_SIZE(sun6i_a31_apb0_rstc_res),
.resources = sun6i_a31_apb0_rstc_res,
},
+   {
+   .name   = "sun8i-codec-analog",
+   .of_compatible  = "allwinner,sun8i-a23-codec-analog",
+   .num_resources  = ARRAY_SIZE(sun8i_codec_analog_res),
+   .resources  = sun8i_codec_analog_res,
+   },
 };
 
 static const struct prcm_data sun6i_a31_prcm_data = {
-- 
2.10.2

[PATCH 04/10] ASoC: sun4i-codec: Add support for A23 codec

[Chen-Yu Tsai]

The codec in the A23 is similar to the one found on the A31. One key
difference is the analog path controls are routed through the PRCM
block. This is supported by the sun8i-codec-analog driver, and tied
into this codec driver with the audio card's aux_dev.

In addition, the A23 does not have LINEOUT, and it does not support
headset jack detection or buttons.

Signed-off-by: Chen-Yu Tsai 
---
 .../devicetree/bindings/sound/sun4i-codec.txt  |  11 ++-
 sound/soc/sunxi/sun4i-codec.c  | 108 +
 2 files changed, 117 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/sun4i-codec.txt 
b/Documentation/devicetree/bindings/sound/sun4i-codec.txt
index d91a95377f49..f7a548b604fc 100644
--- a/Documentation/devicetree/bindings/sound/sun4i-codec.txt
+++ b/Documentation/devicetree/bindings/sound/sun4i-codec.txt
@@ -5,6 +5,7 @@ Required properties:
- "allwinner,sun4i-a10-codec"
- "allwinner,sun6i-a31-codec"
- "allwinner,sun7i-a20-codec"
+   - "allwinner,sun8i-a23-codec"
 - reg: must contain the registers location and length
 - interrupts: must contain the codec interrupt
 - dmas: DMA channels for tx and rx dma. See the DMA client binding,
@@ -21,6 +22,7 @@ Optional properties:
 
 Required properties for the following compatibles:
- "allwinner,sun6i-a31-codec"
+   - "allwinner,sun8i-a23-codec"
 - resets: phandle to the reset control for this device
 - allwinner,audio-routing: A list of the connections between audio components.
   Each entry is a pair of strings, the first being the
@@ -31,10 +33,10 @@ Required properties for the following compatibles:
   "HP"
   "HPCOM"
   "LINEIN"
-  "LINEOUT"
+  "LINEOUT"(not on sun8i-a23)
   "MIC1"
   "MIC2"
-  "MIC3"
+  "MIC3"   (sun6i-a31 only)
 
   Microphone biases from the SoC:
   "HBIAS"
@@ -48,6 +50,11 @@ Required properties for the following compatibles:
   "Mic"
   "Speaker"
 
+Required properties for the following compatibles:
+   - "allwinner,sun8i-a23-codec"
+- allwinner,codec-analog-controls: A phandle to the codec analog controls
+  block in the PRCM.
+
 Example:
 codec: codec@01c22c00 {
#sound-dai-cells = <0>;
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 6379efd21f00..3c5ef1724163 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -213,6 +213,10 @@
 
 /* TODO sun6i DAP (Digital Audio Processing) bits */
 
+/* FIFO counters moved on A23 */
+#define SUN8I_A23_CODEC_DAC_TXCNT  (0x1c)
+#define SUN8I_A23_CODEC_ADC_RXCNT  (0x20)
+
 struct sun4i_codec {
struct device   *dev;
struct regmap   *regmap;
@@ -1067,6 +1071,32 @@ static struct snd_soc_codec_driver sun6i_codec_codec = {
},
 };
 
+/* sun8i A23 codec */
+static const struct snd_kcontrol_new sun8i_a23_codec_codec_controls[] = {
+   SOC_SINGLE_TLV("DAC Playback Volume", SUN4I_CODEC_DAC_DPC,
+  SUN4I_CODEC_DAC_DPC_DVOL, 0x3f, 1,
+  sun6i_codec_dvol_scale),
+};
+
+static const struct snd_soc_dapm_widget sun8i_a23_codec_codec_widgets[] = {
+   /* Digital parts of the ADCs */
+   SND_SOC_DAPM_SUPPLY("ADC Enable", SUN6I_CODEC_ADC_FIFOC,
+   SUN6I_CODEC_ADC_FIFOC_EN_AD, 0, NULL, 0),
+   /* Digital parts of the DACs */
+   SND_SOC_DAPM_SUPPLY("DAC Enable", SUN4I_CODEC_DAC_DPC,
+   SUN4I_CODEC_DAC_DPC_EN_DA, 0, NULL, 0),
+
+};
+
+static struct snd_soc_codec_driver sun8i_a23_codec_codec = {
+   .component_driver = {
+   .controls   = sun8i_a23_codec_codec_controls,
+   .num_controls   = 
ARRAY_SIZE(sun8i_a23_codec_codec_controls),
+   .dapm_widgets   = sun8i_a23_codec_codec_widgets,
+   .num_dapm_widgets   = 
ARRAY_SIZE(sun8i_a23_codec_codec_widgets),
+   },
+};
+
 static const struct snd_soc_component_driver sun4i_codec_component = {
.name = "sun4i-codec",
 };
@@ -1206,6 +1236,63 @@ static struct snd_soc_card 
*sun6i_codec_create_card(struct device *dev)
return card;
 };
 
+/* Connect digital side enables to analog side widgets */
+static const struct snd_soc_dapm_route sun8i_codec_card_routes[] = {
+   /* ADC Routes */
+   { "Left ADC", NULL, "ADC Enable" },
+   { "Right ADC", NULL, "ADC Enable" },
+   { "Codec Capture", NULL, "Left ADC" },
+   { "Codec Capture", NULL, "Right ADC" },
+
+   /* DAC Routes */
+   { "Left DAC", NULL, "DAC 

Re: [PATCH RFC] mm: Add debug_virt_to_phys()

[Will Deacon]

On Fri, Nov 11, 2016 at 04:44:43PM -0800, Florian Fainelli wrote:
> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
> debug_virt_to_phys() which helps catch vmalloc space addresses being
> passed. This is helpful in debugging bogus drivers that just assume
> linear mappings all over the place.
> 
> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
> __virt_to_phys() as being the functional implementation of the address
> translation, so we special case the debug stub to call into
> __virt_to_phys directly.
> 
> Signed-off-by: Florian Fainelli 
> ---
>  arch/arm/include/asm/memory.h  |  4 
>  arch/arm64/include/asm/memory.h|  4 
>  include/asm-generic/memory_model.h |  4 
>  mm/debug.c | 15 +++
>  4 files changed, 27 insertions(+)

What's the interaction between this and the DEBUG_VIRTUAL patches from Laura?

http://lkml.kernel.org/r/20161102210054.16621-7-labb...@redhat.com

They seem to be tackling the exact same problem afaict.

Will

Re: [PATCH RFC] mm: Add debug_virt_to_phys()

[Will Deacon]

On Fri, Nov 11, 2016 at 04:44:43PM -0800, Florian Fainelli wrote:
> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
> debug_virt_to_phys() which helps catch vmalloc space addresses being
> passed. This is helpful in debugging bogus drivers that just assume
> linear mappings all over the place.
> 
> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
> __virt_to_phys() as being the functional implementation of the address
> translation, so we special case the debug stub to call into
> __virt_to_phys directly.
> 
> Signed-off-by: Florian Fainelli 
> ---
>  arch/arm/include/asm/memory.h  |  4 
>  arch/arm64/include/asm/memory.h|  4 
>  include/asm-generic/memory_model.h |  4 
>  mm/debug.c | 15 +++
>  4 files changed, 27 insertions(+)

What's the interaction between this and the DEBUG_VIRTUAL patches from Laura?

http://lkml.kernel.org/r/20161102210054.16621-7-labb...@redhat.com

They seem to be tackling the exact same problem afaict.

Will

Re: [PATCH] ARM: dts: vfxxx: Enable DMA for DSPI2 and DSPI3

[maitysanchayan]

On 16-11-11 20:46:12, Fabio Estevam wrote:
> On Thu, Nov 10, 2016 at 9:45 AM, Sanchayan Maity
>  wrote:
> > Enable DMA for DSPI2 and DSPI3 on Vybrid.
> 
> You missed your Signed-off-by line.

Argh...Sorry about that...Will send a follow patch with this fixed.

- Sanchayan.

Re: [PATCH] ARM: dts: vfxxx: Enable DMA for DSPI2 and DSPI3

[maitysanchayan]

On 16-11-11 20:46:12, Fabio Estevam wrote:
> On Thu, Nov 10, 2016 at 9:45 AM, Sanchayan Maity
>  wrote:
> > Enable DMA for DSPI2 and DSPI3 on Vybrid.
> 
> You missed your Signed-off-by line.

Argh...Sorry about that...Will send a follow patch with this fixed.

- Sanchayan.

Re: [PATCH] clk: Register clkdev after setup of fixed-rate and fixed-factor clocks

[Xiaolong Zhang]

On 二, 10月 25, 2016 at 08:40:08下午 +, Stephen Boyd wrote:
> On 10/22, Xiaolong Zhang wrote:
> > On 四, 10月 20, 2016 at 04:01:03下午 -0700, Stephen Boyd wrote:
> > > On 10/11, Orson Zhai wrote:
> > > > From: Xiaolong Zhang 
> > > > 
> > > > When common kernel setups fixed clock, of_clk_provider will be 
> > > > registerred.
> > > > But there is no clkdev being registerred at the same time. This will 
> > > > make
> > > > it difficult to get the clock by using clk_get(NULL, con_id).
> > > > 
> > > > Add clkdev register for fixed-rate and fixed-factor clock and ignore
> > > > the error if any.
> > > > 
> > > > Signed-off-by: Xiaolong Zhang 
> > > > Signed-off-by: Orson Zhai 
> > > > ---
> > > 
> > > Why are we using clkdev lookups for clks populated from DT?
> > > Shouldn't we be able to point to them from the consumers that
> > > would also be in DT? I'm a little lost.
> > > 
> > The clk_get interface allows the first argument as NULL. We just assure
> > consumers can get the clock from DT or by clock name.
> 
> Ok. The first argument to clk_get() really shouldn't be NULL. It
> should be the device pointer for the device that is associated
> with the clk you want to get. The clock name (second argument to
> clk_get()) should be device specific and not some globally unique
> identifier. It seems that you're using the clk_get() API
> incorrectly.
>
Sorry for late reply.
There are two paths in clk_get:
a) dev is not NULL, the calling procedure is as following
clk_get(dev, con_id)
--->of_clk_get_by_name(dev->of_node, con_id)

b) dev is NULL, the calling procedure is as following
clk_dev(NULL, con_id)
--->clk_get_sys(NULL, con_id)
--->clk_find(NULL, con_id)

I just cann't understand why you say the first argument shouldn't
be NULL. Is there other consideration in CCF?

Thanks

Xiaolong Zhang
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: [PATCH] clk: Register clkdev after setup of fixed-rate and fixed-factor clocks

[Xiaolong Zhang]

On 二, 10月 25, 2016 at 08:40:08下午 +, Stephen Boyd wrote:
> On 10/22, Xiaolong Zhang wrote:
> > On 四, 10月 20, 2016 at 04:01:03下午 -0700, Stephen Boyd wrote:
> > > On 10/11, Orson Zhai wrote:
> > > > From: Xiaolong Zhang 
> > > > 
> > > > When common kernel setups fixed clock, of_clk_provider will be 
> > > > registerred.
> > > > But there is no clkdev being registerred at the same time. This will 
> > > > make
> > > > it difficult to get the clock by using clk_get(NULL, con_id).
> > > > 
> > > > Add clkdev register for fixed-rate and fixed-factor clock and ignore
> > > > the error if any.
> > > > 
> > > > Signed-off-by: Xiaolong Zhang 
> > > > Signed-off-by: Orson Zhai 
> > > > ---
> > > 
> > > Why are we using clkdev lookups for clks populated from DT?
> > > Shouldn't we be able to point to them from the consumers that
> > > would also be in DT? I'm a little lost.
> > > 
> > The clk_get interface allows the first argument as NULL. We just assure
> > consumers can get the clock from DT or by clock name.
> 
> Ok. The first argument to clk_get() really shouldn't be NULL. It
> should be the device pointer for the device that is associated
> with the clk you want to get. The clock name (second argument to
> clk_get()) should be device specific and not some globally unique
> identifier. It seems that you're using the clk_get() API
> incorrectly.
>
Sorry for late reply.
There are two paths in clk_get:
a) dev is not NULL, the calling procedure is as following
clk_get(dev, con_id)
--->of_clk_get_by_name(dev->of_node, con_id)

b) dev is NULL, the calling procedure is as following
clk_dev(NULL, con_id)
--->clk_get_sys(NULL, con_id)
--->clk_find(NULL, con_id)

I just cann't understand why you say the first argument shouldn't
be NULL. Is there other consideration in CCF?

Thanks

Xiaolong Zhang
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: [PATCH 2/3] cpufreq: schedutil: move slow path from workqueue to SCHED_FIFO task

[Viresh Kumar]

On 12 November 2016 at 07:01, Saravana Kannan  wrote:
> Hold on a sec. I thought during LPC someone (Peter?) made a point that when
> RT thread run, we should bump the frequency to max?

I wasn't there but AFAIU, this is the case we have currently for the schedutil
governor. And we (mobile world, Linaro) want to change that it doesn't work
that well for us. So perhaps it is just the opposite of what you stated.

> So, schedutil is going
> to trigger schedutil to bump up the frequency to max, right?

How is that question related to this patch ?

--
viresh

Re: [PATCH 2/3] cpufreq: schedutil: move slow path from workqueue to SCHED_FIFO task

[Viresh Kumar]

On 12 November 2016 at 07:01, Saravana Kannan  wrote:
> Hold on a sec. I thought during LPC someone (Peter?) made a point that when
> RT thread run, we should bump the frequency to max?

I wasn't there but AFAIU, this is the case we have currently for the schedutil
governor. And we (mobile world, Linaro) want to change that it doesn't work
that well for us. So perhaps it is just the opposite of what you stated.

> So, schedutil is going
> to trigger schedutil to bump up the frequency to max, right?

How is that question related to this patch ?

--
viresh

Re: [PATCH 6/8] block: add scalable completion tracking of requests

[Jens Axboe]

On 11/10/2016 12:38 PM, Jan Kara wrote:

On Wed 09-11-16 12:52:25, Jens Axboe wrote:

On 11/09/2016 09:09 AM, Jens Axboe wrote:

On 11/09/2016 02:01 AM, Jan Kara wrote:

On Tue 08-11-16 08:25:52, Jens Axboe wrote:

On 11/08/2016 06:30 AM, Jan Kara wrote:

On Tue 01-11-16 15:08:49, Jens Axboe wrote:

For legacy block, we simply track them in the request queue. For
blk-mq, we track them on a per-sw queue basis, which we can then
sum up through the hardware queues and finally to a per device
state.

The stats are tracked in, roughly, 0.1s interval windows.

Add sysfs files to display the stats.

Signed-off-by: Jens Axboe 


This patch looks mostly good to me but I have one concern: You track
statistics in a fixed 134ms window, stats get cleared at the
beginning of
each window. Now this can interact with the writeback window and
latency
settings which are dynamic and settable from userspace - so if the
writeback code observation window gets set larger than the stats
window,
things become strange since you'll likely miss quite some observations
about read latencies. So I think you need to make sure stats window is
always larger than writeback window. Or actually, why do you have
something
like stats window and don't leave clearing of statistics completely
to the
writeback tracking code?


That's a good point, and there actually used to be a comment to that
effect in the code. I think the best solution here would be to make the
stats code mask available somewhere, and allow a consumer of the stats
to request a larger window.

Similarly, we could make the stat window be driven by the consumer, as
you suggest.

Currently there are two pending submissions that depend on the stats
code. One is this writeback series, and the other one is the hybrid
polling code. The latter does not really care about the window size as
such, since it has no monitoring window of its own, and it wants the
auto-clearing as well.

I don't mind working on additions for this, but I'd prefer if we could
layer them on top of the existing series instead of respinning it.
There's considerable test time on the existing patchset. Would that work
for you? Especially collapsing the stats and wbt windows would require
some re-architecting.


OK, that works for me. Actually, when thinking about this, I have one
more
suggestion: Do we really want to expose the wbt window as a sysfs
tunable?
I guess it is good for initial experiments but longer term having the wbt
window length be a function of target read latency might be better.
Generally you want the window length to be considerably larger than the
target latency but OTOH not too large so that the algorithm can react
reasonably quickly so that suggests it could really be autotuned (and we
scale the window anyway to adapt it to current situation).


That's not a bad idea, I have thought about that as well before. We
don't need the window tunable, and you are right, it can be a function
of the desired latency.

I'll hardwire the 100msec latency window for now and get rid of the
exposed tunable. It's harder to remove sysfs files once they have made
it into the kernel...


Killed the sysfs variable, so for now it'll be a 100msec window by
default.


OK, I guess good enough to get this merged.


Thanks Jan!

--
Jens Axboe

Re: [PATCH 6/8] block: add scalable completion tracking of requests

[Jens Axboe]

On 11/10/2016 12:38 PM, Jan Kara wrote:

On Wed 09-11-16 12:52:25, Jens Axboe wrote:

On 11/09/2016 09:09 AM, Jens Axboe wrote:

On 11/09/2016 02:01 AM, Jan Kara wrote:

On Tue 08-11-16 08:25:52, Jens Axboe wrote:

On 11/08/2016 06:30 AM, Jan Kara wrote:

On Tue 01-11-16 15:08:49, Jens Axboe wrote:

For legacy block, we simply track them in the request queue. For
blk-mq, we track them on a per-sw queue basis, which we can then
sum up through the hardware queues and finally to a per device
state.

The stats are tracked in, roughly, 0.1s interval windows.

Add sysfs files to display the stats.

Signed-off-by: Jens Axboe 


This patch looks mostly good to me but I have one concern: You track
statistics in a fixed 134ms window, stats get cleared at the
beginning of
each window. Now this can interact with the writeback window and
latency
settings which are dynamic and settable from userspace - so if the
writeback code observation window gets set larger than the stats
window,
things become strange since you'll likely miss quite some observations
about read latencies. So I think you need to make sure stats window is
always larger than writeback window. Or actually, why do you have
something
like stats window and don't leave clearing of statistics completely
to the
writeback tracking code?


That's a good point, and there actually used to be a comment to that
effect in the code. I think the best solution here would be to make the
stats code mask available somewhere, and allow a consumer of the stats
to request a larger window.

Similarly, we could make the stat window be driven by the consumer, as
you suggest.

Currently there are two pending submissions that depend on the stats
code. One is this writeback series, and the other one is the hybrid
polling code. The latter does not really care about the window size as
such, since it has no monitoring window of its own, and it wants the
auto-clearing as well.

I don't mind working on additions for this, but I'd prefer if we could
layer them on top of the existing series instead of respinning it.
There's considerable test time on the existing patchset. Would that work
for you? Especially collapsing the stats and wbt windows would require
some re-architecting.


OK, that works for me. Actually, when thinking about this, I have one
more
suggestion: Do we really want to expose the wbt window as a sysfs
tunable?
I guess it is good for initial experiments but longer term having the wbt
window length be a function of target read latency might be better.
Generally you want the window length to be considerably larger than the
target latency but OTOH not too large so that the algorithm can react
reasonably quickly so that suggests it could really be autotuned (and we
scale the window anyway to adapt it to current situation).


That's not a bad idea, I have thought about that as well before. We
don't need the window tunable, and you are right, it can be a function
of the desired latency.

I'll hardwire the 100msec latency window for now and get rid of the
exposed tunable. It's harder to remove sysfs files once they have made
it into the kernel...


Killed the sysfs variable, so for now it'll be a 100msec window by
default.


OK, I guess good enough to get this merged.


Thanks Jan!

--
Jens Axboe

Re: [PATCH 2/3] cpufreq: schedutil: move slow path from workqueue to SCHED_FIFO task

[Viresh Kumar]

On 12 November 2016 at 03:46, Rafael J. Wysocki  wrote:

>> +static void sugov_work(struct kthread_work *work)
>>  {
>> -   struct sugov_policy *sg_policy = container_of(work, struct 
>> sugov_policy, work);
>> +   struct sugov_policy *sg_policy =
>> +   container_of(work, struct sugov_policy, work);
>
> Why this change?

Mistake ..

>>  static struct sugov_policy *sugov_policy_alloc(struct cpufreq_policy 
>> *policy)
>>  {
>> struct sugov_policy *sg_policy;
>> +   struct task_struct *thread;
>> +   struct sched_param param = { .sched_priority = 50 };
>
> I'd define a symbol for the 50.  It's just one extra line of code ...

Sure.

As I asked in the cover letter, will you be fine if I send the same patch
for ondemand/conservative governors ?

Re: [PATCH 2/3] cpufreq: schedutil: move slow path from workqueue to SCHED_FIFO task

[Viresh Kumar]

On 12 November 2016 at 03:46, Rafael J. Wysocki  wrote:

>> +static void sugov_work(struct kthread_work *work)
>>  {
>> -   struct sugov_policy *sg_policy = container_of(work, struct 
>> sugov_policy, work);
>> +   struct sugov_policy *sg_policy =
>> +   container_of(work, struct sugov_policy, work);
>
> Why this change?

Mistake ..

>>  static struct sugov_policy *sugov_policy_alloc(struct cpufreq_policy 
>> *policy)
>>  {
>> struct sugov_policy *sg_policy;
>> +   struct task_struct *thread;
>> +   struct sched_param param = { .sched_priority = 50 };
>
> I'd define a symbol for the 50.  It's just one extra line of code ...

Sure.

As I asked in the cover letter, will you be fine if I send the same patch
for ondemand/conservative governors ?

Re: [PATCH 2/3] cpufreq: schedutil: move slow path from workqueue to SCHED_FIFO task

[Viresh Kumar]

On 11 November 2016 at 20:09, Peter Zijlstra  wrote:
> On Fri, Nov 11, 2016 at 03:32:04PM +0100, Tommaso Cucinotta wrote:
>> >+struct sched_param param = { .sched_priority = 50 };
>>
>> won't you have a tunable here? (sysctl?)
>
> You can use the regular userspace tools, like schedtool and chrt to set
> priorities.

I wanted to get some help from you on this Peter. The out of tree Interactive
governor has always used MAX_RT_PRIORITY - 1 here instead of 50.

But Steve started with 50. What do you think should the value be ?

--
viresh

Re: [PATCH 2/3] cpufreq: schedutil: move slow path from workqueue to SCHED_FIFO task

[Viresh Kumar]

On 11 November 2016 at 20:09, Peter Zijlstra  wrote:
> On Fri, Nov 11, 2016 at 03:32:04PM +0100, Tommaso Cucinotta wrote:
>> >+struct sched_param param = { .sched_priority = 50 };
>>
>> won't you have a tunable here? (sysctl?)
>
> You can use the regular userspace tools, like schedtool and chrt to set
> priorities.

I wanted to get some help from you on this Peter. The out of tree Interactive
governor has always used MAX_RT_PRIORITY - 1 here instead of 50.

But Steve started with 50. What do you think should the value be ?

--
viresh

Re: [PATCH 2/3] cpufreq: schedutil: move slow path from workqueue to SCHED_FIFO task

[Viresh Kumar]

On 11 November 2016 at 20:02, Tommaso Cucinotta
 wrote:

> would you have an insight, as to what runtime/deadline/period to set, and/or
> what specific timing constraints you'd like to set, just for this cpufreq
> slowpath work?

I don't have any such figures for not at least :(

Re: [PATCH 2/3] cpufreq: schedutil: move slow path from workqueue to SCHED_FIFO task

[Viresh Kumar]

On 11 November 2016 at 20:02, Tommaso Cucinotta
 wrote:

> would you have an insight, as to what runtime/deadline/period to set, and/or
> what specific timing constraints you'd like to set, just for this cpufreq
> slowpath work?

I don't have any such figures for not at least :(

Re: [PATCH 1/3] cpufreq: schedutil: enable fast switch earlier

[Viresh Kumar]

On 12 November 2016 at 03:28, Rafael J. Wysocki  wrote:

>> @@ -478,8 +484,6 @@ static void sugov_exit(struct cpufreq_policy *policy)
>> struct sugov_tunables *tunables = sg_policy->tunables;
>> unsigned int count;
>>
>> -   cpufreq_disable_fast_switch(policy);
>> -
>
> ->but why is this change necessary?
>
> sugov_stop() has been called already, so the ordering here shouldn't matter.

Because sugov_policy_free() would be using the flag fast_switch_enabled.

--
viresh

Re: [PATCH 1/3] cpufreq: schedutil: enable fast switch earlier

[Viresh Kumar]

On 12 November 2016 at 03:28, Rafael J. Wysocki  wrote:

>> @@ -478,8 +484,6 @@ static void sugov_exit(struct cpufreq_policy *policy)
>> struct sugov_tunables *tunables = sg_policy->tunables;
>> unsigned int count;
>>
>> -   cpufreq_disable_fast_switch(policy);
>> -
>
> ->but why is this change necessary?
>
> sugov_stop() has been called already, so the ordering here shouldn't matter.

Because sugov_policy_free() would be using the flag fast_switch_enabled.

--
viresh

kvm: use-after-free in irq_bypass_register_consumer

[Dmitry Vyukov]

Hello,

While running syzkaller fuzzer I got the following use-after-free report.
On commit 015ed9433be2b476ec7e2e6a9a411a56e3b5b035 (Nov 11)


BUG: KASAN: use-after-free in irq_bypass_register_consumer+0x3d1/0x420
at addr 88003b5d8820
Write of size 8 by task syz-executor/25573
CPU: 0 PID: 25573 Comm: syz-executor Not tainted 4.9.0-rc4+ #46
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 88006caaf9e0 81c2d79b 88003e80ccc0 88003b5d86b8
 88003b5d89f0 0001 88006caafa08 8165ab9c
 ed00076bb104 ed00076bb104 88003e80ccc0 88006caafa88
Call Trace:
 [] __asan_report_store8_noabort+0x17/0x20
mm/kasan/report.c:334
 [< inline >] list_add include/linux/list.h:43
 [] irq_bypass_register_consumer+0x3d1/0x420
virt/lib/irqbypass.c:217
 [< inline >] kvm_irqfd_assign
arch/x86/kvm/../../../virt/kvm/eventfd.c:417
 [] kvm_irqfd+0x109a/0x18a0
arch/x86/kvm/../../../virt/kvm/eventfd.c:572
 [] kvm_vm_ioctl+0x2e7/0x1670
arch/x86/kvm/../../../virt/kvm/kvm_main.c:2996
 [< inline >] vfs_ioctl fs/ioctl.c:43
 [] do_vfs_ioctl+0x18c/0x1040 fs/ioctl.c:679
 [< inline >] SYSC_ioctl fs/ioctl.c:694
 [] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:685
 [] entry_SYSCALL_64_fastpath+0x1f/0xc2

Object at 88003b5d86b8, in cache kmalloc-512 size: 512

Allocated:
PID = 25573
 [  359.255946] [< inline >] kzalloc include/linux/slab.h:636
 [  359.255946] [< inline >] kvm_irqfd_assign
arch/x86/kvm/../../../virt/kvm/eventfd.c:296
 [  359.255946] [] kvm_irqfd+0xa7/0x18a0
arch/x86/kvm/../../../virt/kvm/eventfd.c:572
 [  359.255946] [] kvm_vm_ioctl+0x2e7/0x1670
arch/x86/kvm/../../../virt/kvm/kvm_main.c:2996
 [  359.255946] [< inline >] vfs_ioctl fs/ioctl.c:43
 [  359.255946] [] do_vfs_ioctl+0x18c/0x1040 fs/ioctl.c:679
 [  359.255946] [< inline >] SYSC_ioctl fs/ioctl.c:694
 [  359.255946] [] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:685
 [  359.255946] [] entry_SYSCALL_64_fastpath+0x1f/0xc2

Freed:
PID = 1057
 [  359.255946] [] kfree+0xea/0x2c0 mm/slub.c:3871
 [  359.255946] [] irqfd_shutdown+0x13d/0x1a0
arch/x86/kvm/../../../virt/kvm/eventfd.c:148
 [  359.255946] [] process_one_work+0x9fc/0x1900
kernel/workqueue.c:2096
 [  359.255946] [] worker_thread+0xef/0x1480
kernel/workqueue.c:2230
 [  359.386293] [] kthread+0x244/0x2d0 kernel/kthread.c:209
 [  359.387074] [] ret_from_fork+0x2a/0x40
arch/x86/entry/entry_64.S:433

kvm: use-after-free in irq_bypass_register_consumer

[Dmitry Vyukov]

Hello,

While running syzkaller fuzzer I got the following use-after-free report.
On commit 015ed9433be2b476ec7e2e6a9a411a56e3b5b035 (Nov 11)


BUG: KASAN: use-after-free in irq_bypass_register_consumer+0x3d1/0x420
at addr 88003b5d8820
Write of size 8 by task syz-executor/25573
CPU: 0 PID: 25573 Comm: syz-executor Not tainted 4.9.0-rc4+ #46
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 88006caaf9e0 81c2d79b 88003e80ccc0 88003b5d86b8
 88003b5d89f0 0001 88006caafa08 8165ab9c
 ed00076bb104 ed00076bb104 88003e80ccc0 88006caafa88
Call Trace:
 [] __asan_report_store8_noabort+0x17/0x20
mm/kasan/report.c:334
 [< inline >] list_add include/linux/list.h:43
 [] irq_bypass_register_consumer+0x3d1/0x420
virt/lib/irqbypass.c:217
 [< inline >] kvm_irqfd_assign
arch/x86/kvm/../../../virt/kvm/eventfd.c:417
 [] kvm_irqfd+0x109a/0x18a0
arch/x86/kvm/../../../virt/kvm/eventfd.c:572
 [] kvm_vm_ioctl+0x2e7/0x1670
arch/x86/kvm/../../../virt/kvm/kvm_main.c:2996
 [< inline >] vfs_ioctl fs/ioctl.c:43
 [] do_vfs_ioctl+0x18c/0x1040 fs/ioctl.c:679
 [< inline >] SYSC_ioctl fs/ioctl.c:694
 [] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:685
 [] entry_SYSCALL_64_fastpath+0x1f/0xc2

Object at 88003b5d86b8, in cache kmalloc-512 size: 512

Allocated:
PID = 25573
 [  359.255946] [< inline >] kzalloc include/linux/slab.h:636
 [  359.255946] [< inline >] kvm_irqfd_assign
arch/x86/kvm/../../../virt/kvm/eventfd.c:296
 [  359.255946] [] kvm_irqfd+0xa7/0x18a0
arch/x86/kvm/../../../virt/kvm/eventfd.c:572
 [  359.255946] [] kvm_vm_ioctl+0x2e7/0x1670
arch/x86/kvm/../../../virt/kvm/kvm_main.c:2996
 [  359.255946] [< inline >] vfs_ioctl fs/ioctl.c:43
 [  359.255946] [] do_vfs_ioctl+0x18c/0x1040 fs/ioctl.c:679
 [  359.255946] [< inline >] SYSC_ioctl fs/ioctl.c:694
 [  359.255946] [] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:685
 [  359.255946] [] entry_SYSCALL_64_fastpath+0x1f/0xc2

Freed:
PID = 1057
 [  359.255946] [] kfree+0xea/0x2c0 mm/slub.c:3871
 [  359.255946] [] irqfd_shutdown+0x13d/0x1a0
arch/x86/kvm/../../../virt/kvm/eventfd.c:148
 [  359.255946] [] process_one_work+0x9fc/0x1900
kernel/workqueue.c:2096
 [  359.255946] [] worker_thread+0xef/0x1480
kernel/workqueue.c:2230
 [  359.386293] [] kthread+0x244/0x2d0 kernel/kthread.c:209
 [  359.387074] [] ret_from_fork+0x2a/0x40
arch/x86/entry/entry_64.S:433

Re: [PATCH v2 2/9] arm64: dts: rockchip: add pd_sd power node for rk3399

[Shawn Lin]

Hi Caesar,

On 2016/11/9 21:21, Caesar Wang wrote:

From: zhangqing 

1.add pd node for RK3399 Soc
2.create power domain tree
3.add qos node for domain
4.add the pd_sd consumers node


I'm no sure if it is worth spliting out a seperated
patch as it looks to me that you was doing 4 things within
one patch, but anyway

Tested-by: Shawn Lin 



Signed-off-by: Elaine Zhang 
Signed-off-by: Caesar Wang 
---

Changes in v2:
- v1 on https://patchwork.kernel.org/patch/9322553/
- Reviewed-on: https://chromium-review.googlesource.com/386483
- Verified on ChromeOS kernel4.4

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index b401176..e5b5b3d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -253,6 +253,7 @@
 < SCLK_SDMMC_DRV>, < SCLK_SDMMC_SAMPLE>;
clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
fifo-depth = <0x100>;
+   power-domains = < RK3399_PD_SD>;
status = "disabled";
};

@@ -691,6 +692,11 @@
status = "disabled";
};

+   qos_sd: qos@ffa74000 {
+   compatible = "syscon";
+   reg = <0x0 0xffa74000 0x0 0x20>;
+   };
+
qos_emmc: qos@ffa58000 {
compatible = "syscon";
reg = <0x0 0xffa58000 0x0 0x20>;
@@ -839,6 +845,12 @@
clocks = < ACLK_GMAC>;
pm_qos = <_gmac>;
};
+   pd_sd@RK3399_PD_SD {
+   reg = ;
+   clocks = < HCLK_SDMMC>,
+< SCLK_SDMMC>;
+   pm_qos = <_sd>;
+   };
pd_vio@RK3399_PD_VIO {
reg = ;
#address-cells = <1>;




--
Best Regards
Shawn Lin

Re: [PATCH v2 2/9] arm64: dts: rockchip: add pd_sd power node for rk3399

[Shawn Lin]

Hi Caesar,

On 2016/11/9 21:21, Caesar Wang wrote:

From: zhangqing 

1.add pd node for RK3399 Soc
2.create power domain tree
3.add qos node for domain
4.add the pd_sd consumers node


I'm no sure if it is worth spliting out a seperated
patch as it looks to me that you was doing 4 things within
one patch, but anyway

Tested-by: Shawn Lin 



Signed-off-by: Elaine Zhang 
Signed-off-by: Caesar Wang 
---

Changes in v2:
- v1 on https://patchwork.kernel.org/patch/9322553/
- Reviewed-on: https://chromium-review.googlesource.com/386483
- Verified on ChromeOS kernel4.4

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index b401176..e5b5b3d 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -253,6 +253,7 @@
 < SCLK_SDMMC_DRV>, < SCLK_SDMMC_SAMPLE>;
clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
fifo-depth = <0x100>;
+   power-domains = < RK3399_PD_SD>;
status = "disabled";
};

@@ -691,6 +692,11 @@
status = "disabled";
};

+   qos_sd: qos@ffa74000 {
+   compatible = "syscon";
+   reg = <0x0 0xffa74000 0x0 0x20>;
+   };
+
qos_emmc: qos@ffa58000 {
compatible = "syscon";
reg = <0x0 0xffa58000 0x0 0x20>;
@@ -839,6 +845,12 @@
clocks = < ACLK_GMAC>;
pm_qos = <_gmac>;
};
+   pd_sd@RK3399_PD_SD {
+   reg = ;
+   clocks = < HCLK_SDMMC>,
+< SCLK_SDMMC>;
+   pm_qos = <_sd>;
+   };
pd_vio@RK3399_PD_VIO {
reg = ;
#address-cells = <1>;




--
Best Regards
Shawn Lin

[PATCH] mm: disable numa migration faults for dax vmas

[Dan Williams]

Mark dax vmas as not migratable to exclude them from task_numa_work().
This is especially relevant for device-dax which wants to ensure
predictable access latency and not incur periodic faults.

Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: Vlastimil Babka 
Cc: "Kirill A. Shutemov" 
Reported-by: Aneesh Kumar K.V 
Signed-off-by: Dan Williams 
---
 include/linux/mempolicy.h |4 
 1 file changed, 4 insertions(+)

diff --git a/include/linux/mempolicy.h b/include/linux/mempolicy.h
index 5e5b2969d931..d72c691afaa6 100644
--- a/include/linux/mempolicy.h
+++ b/include/linux/mempolicy.h
@@ -7,6 +7,7 @@
 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -177,6 +178,9 @@ static inline bool vma_migratable(struct vm_area_struct 
*vma)
if (vma->vm_flags & (VM_IO | VM_PFNMAP))
return false;
 
+   if (vma_is_dax(vma))
+   return false;
+
 #ifndef CONFIG_ARCH_ENABLE_HUGEPAGE_MIGRATION
if (vma->vm_flags & VM_HUGETLB)
return false;

[PATCH] mm: disable numa migration faults for dax vmas

[Dan Williams]

Mark dax vmas as not migratable to exclude them from task_numa_work().
This is especially relevant for device-dax which wants to ensure
predictable access latency and not incur periodic faults.

Cc: Andrew Morton 
Cc: Michal Hocko 
Cc: Vlastimil Babka 
Cc: "Kirill A. Shutemov" 
Reported-by: Aneesh Kumar K.V 
Signed-off-by: Dan Williams 
---
 include/linux/mempolicy.h |4 
 1 file changed, 4 insertions(+)

diff --git a/include/linux/mempolicy.h b/include/linux/mempolicy.h
index 5e5b2969d931..d72c691afaa6 100644
--- a/include/linux/mempolicy.h
+++ b/include/linux/mempolicy.h
@@ -7,6 +7,7 @@
 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -177,6 +178,9 @@ static inline bool vma_migratable(struct vm_area_struct 
*vma)
if (vma->vm_flags & (VM_IO | VM_PFNMAP))
return false;
 
+   if (vma_is_dax(vma))
+   return false;
+
 #ifndef CONFIG_ARCH_ENABLE_HUGEPAGE_MIGRATION
if (vma->vm_flags & VM_HUGETLB)
return false;

Re: [PATCH v2 1/9] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Shawn Lin]

On 2016/11/9 21:21, Caesar Wang wrote:

From: Ziyuan Xu 

Control power domain for eMMC via genpd to reduce power consumption.

Signed-off-by: Elaine Zhang 
Signed-off-by: Ziyuan Xu 
Signed-off-by: Caesar Wang 


It was verified on my rk3399 evb with kernel4.4, so
free feel to add my tag,

Tested-by: Shawn Lin 

BTW, it seems my reply is bounced form Yakir's address, so please
remove him from CC list if he changed his mail address.


---

Changes in v2:
- Reviewed-on: https://chromium-review.googlesource.com/376558
- Verified on ChromeOS kernel4.4

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index cbb7f8b..b401176 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -269,6 +269,7 @@
#clock-cells = <0>;
phys = <_phy>;
phy-names = "phy_arasan";
+   power-domains = < RK3399_PD_EMMC>;
status = "disabled";
};

@@ -690,6 +691,11 @@
status = "disabled";
};

+   qos_emmc: qos@ffa58000 {
+   compatible = "syscon";
+   reg = <0x0 0xffa58000 0x0 0x20>;
+   };
+
qos_gmac: qos@ffa5c000 {
compatible = "syscon";
reg = <0x0 0xffa5c000 0x0 0x20>;
@@ -823,6 +829,11 @@
};

/* These power domains are grouped by VD_LOGIC */
+   pd_emmc@RK3399_PD_EMMC {
+   reg = ;
+   clocks = < ACLK_EMMC>;
+   pm_qos = <_emmc>;
+   };
pd_gmac@RK3399_PD_GMAC {
reg = ;
clocks = < ACLK_GMAC>;




--
Best Regards
Shawn Lin

Re: [PATCH v2 1/9] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Shawn Lin]

On 2016/11/9 21:21, Caesar Wang wrote:

From: Ziyuan Xu 

Control power domain for eMMC via genpd to reduce power consumption.

Signed-off-by: Elaine Zhang 
Signed-off-by: Ziyuan Xu 
Signed-off-by: Caesar Wang 


It was verified on my rk3399 evb with kernel4.4, so
free feel to add my tag,

Tested-by: Shawn Lin 

BTW, it seems my reply is bounced form Yakir's address, so please
remove him from CC list if he changed his mail address.


---

Changes in v2:
- Reviewed-on: https://chromium-review.googlesource.com/376558
- Verified on ChromeOS kernel4.4

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index cbb7f8b..b401176 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -269,6 +269,7 @@
#clock-cells = <0>;
phys = <_phy>;
phy-names = "phy_arasan";
+   power-domains = < RK3399_PD_EMMC>;
status = "disabled";
};

@@ -690,6 +691,11 @@
status = "disabled";
};

+   qos_emmc: qos@ffa58000 {
+   compatible = "syscon";
+   reg = <0x0 0xffa58000 0x0 0x20>;
+   };
+
qos_gmac: qos@ffa5c000 {
compatible = "syscon";
reg = <0x0 0xffa5c000 0x0 0x20>;
@@ -823,6 +829,11 @@
};

/* These power domains are grouped by VD_LOGIC */
+   pd_emmc@RK3399_PD_EMMC {
+   reg = ;
+   clocks = < ACLK_EMMC>;
+   pm_qos = <_emmc>;
+   };
pd_gmac@RK3399_PD_GMAC {
reg = ;
clocks = < ACLK_GMAC>;




--
Best Regards
Shawn Lin

Re: [PATCH 3.12 89/91] PM / devfreq: Fix incorrect type issue.

[Willy Tarreau]

On Sat, Nov 12, 2016 at 03:02:23AM +, Ben Hutchings wrote:
> On Tue, 2016-01-05 at 18:47 +0100, Jiri Slaby wrote:
> > From: Xiaolong Ye 
> > 
> > 3.12-stable review patch.  If anyone has any objections, please let
> > me know.
> > 
> > ===
> > 
> > commit 5f25f066f75a67835abb5e400471a27abd09395b upstream.
> > 
> > time_in_state in struct devfreq is defined as unsigned long, so
> > devm_kzalloc should use sizeof(unsigned long) as argument instead
> > of sizeof(unsigned int), otherwise it will cause unexpected result
> > in 64bit system.
> > 
> > Signed-off-by: Xiaolong Ye 
> > Signed-off-by: Kevin Liu 
> > Signed-off-by: MyungJoo Ham 
> > Cc: Oliver Neukum 
> > Signed-off-by: Jiri Slaby 
> [...]
> 
> This is still needed in longterm branches 3.10 - 4.1 inclusive.  I just
> queued it up for 3.16.

now queued up for 3.10, thanks Ben for letting me know.

Willy

Re: [PATCH 3.12 89/91] PM / devfreq: Fix incorrect type issue.

[Willy Tarreau]

On Sat, Nov 12, 2016 at 03:02:23AM +, Ben Hutchings wrote:
> On Tue, 2016-01-05 at 18:47 +0100, Jiri Slaby wrote:
> > From: Xiaolong Ye 
> > 
> > 3.12-stable review patch.  If anyone has any objections, please let
> > me know.
> > 
> > ===
> > 
> > commit 5f25f066f75a67835abb5e400471a27abd09395b upstream.
> > 
> > time_in_state in struct devfreq is defined as unsigned long, so
> > devm_kzalloc should use sizeof(unsigned long) as argument instead
> > of sizeof(unsigned int), otherwise it will cause unexpected result
> > in 64bit system.
> > 
> > Signed-off-by: Xiaolong Ye 
> > Signed-off-by: Kevin Liu 
> > Signed-off-by: MyungJoo Ham 
> > Cc: Oliver Neukum 
> > Signed-off-by: Jiri Slaby 
> [...]
> 
> This is still needed in longterm branches 3.10 - 4.1 inclusive.  I just
> queued it up for 3.16.

now queued up for 3.10, thanks Ben for letting me know.

Willy

kvm: WARNING in kvm_load_guest_fpu

[Dmitry Vyukov]

Hello,

The following program triggers WARNING in kvm_load_guest_fpu:
https://gist.githubusercontent.com/dvyukov/5bc076073b48772e22b5f33acbe2b743/raw/1000df869f0d58a5c6e637268453c711280b255d/gistfile1.txt

On commit 015ed9433be2b476ec7e2e6a9a411a56e3b5b035 (Nov 11).

WARNING: CPU: 3 PID: 23924 at
./arch/x86/include/asm/fpu/internal.h:164
kvm_load_guest_fpu.part.163+0xf1/0x340
Kernel panic - not syncing: panic_on_warn set ...

CPU: 3 PID: 23924 Comm: syz-executor Not tainted 4.9.0-rc4+ #44
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 88006ce67920 81c2f79b 83271fc0 88006ce679f8
 8321c300 81096631 88006ce679e8 81548463
 41b58ab3 837cd505 815482ac 815490e9
Call Trace:
 [< inline >] __dump_stack lib/dump_stack.c:15
 [] dump_stack+0xb3/0x118 lib/dump_stack.c:51
 [] panic+0x1b7/0x3a3 kernel/panic.c:179
 [] __warn+0x1c4/0x1e0 kernel/panic.c:542
 [] warn_slowpath_null+0x2c/0x40 kernel/panic.c:585
 [< inline >] copy_kernel_to_fxregs
arch/x86/include/asm/fpu/internal.h:164
 [< inline >] __copy_kernel_to_fpregs
arch/x86/include/asm/fpu/internal.h:454
 [] kvm_load_guest_fpu.part.163+0xf1/0x340
arch/x86/kvm/x86.c:7383
 [< inline >] kvm_load_guest_fpu arch/x86/kvm/x86.c:6456
 [< inline >] vcpu_enter_guest arch/x86/kvm/x86.c:6660
 [< inline >] vcpu_run arch/x86/kvm/x86.c:6826
 [] kvm_arch_vcpu_ioctl_run+0x3b13/0x5a90
arch/x86/kvm/x86.c:6984
 [] kvm_vcpu_ioctl+0x61e/0xdd0
arch/x86/kvm/../../../virt/kvm/kvm_main.c:2557
 [< inline >] vfs_ioctl fs/ioctl.c:43
 [] do_vfs_ioctl+0x18c/0x1040 fs/ioctl.c:679
 [< inline >] SYSC_ioctl fs/ioctl.c:694
 [] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:685
 [] entry_SYSCALL_64_fastpath+0x1f/0xc2
Dumping ftrace buffer:
   (ftrace buffer empty)
Kernel Offset: disabled
reboot: cpu_has_vmx: ecx=80a02021 1

kvm: WARNING in kvm_load_guest_fpu

[Dmitry Vyukov]

Hello,

The following program triggers WARNING in kvm_load_guest_fpu:
https://gist.githubusercontent.com/dvyukov/5bc076073b48772e22b5f33acbe2b743/raw/1000df869f0d58a5c6e637268453c711280b255d/gistfile1.txt

On commit 015ed9433be2b476ec7e2e6a9a411a56e3b5b035 (Nov 11).

WARNING: CPU: 3 PID: 23924 at
./arch/x86/include/asm/fpu/internal.h:164
kvm_load_guest_fpu.part.163+0xf1/0x340
Kernel panic - not syncing: panic_on_warn set ...

CPU: 3 PID: 23924 Comm: syz-executor Not tainted 4.9.0-rc4+ #44
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 88006ce67920 81c2f79b 83271fc0 88006ce679f8
 8321c300 81096631 88006ce679e8 81548463
 41b58ab3 837cd505 815482ac 815490e9
Call Trace:
 [< inline >] __dump_stack lib/dump_stack.c:15
 [] dump_stack+0xb3/0x118 lib/dump_stack.c:51
 [] panic+0x1b7/0x3a3 kernel/panic.c:179
 [] __warn+0x1c4/0x1e0 kernel/panic.c:542
 [] warn_slowpath_null+0x2c/0x40 kernel/panic.c:585
 [< inline >] copy_kernel_to_fxregs
arch/x86/include/asm/fpu/internal.h:164
 [< inline >] __copy_kernel_to_fpregs
arch/x86/include/asm/fpu/internal.h:454
 [] kvm_load_guest_fpu.part.163+0xf1/0x340
arch/x86/kvm/x86.c:7383
 [< inline >] kvm_load_guest_fpu arch/x86/kvm/x86.c:6456
 [< inline >] vcpu_enter_guest arch/x86/kvm/x86.c:6660
 [< inline >] vcpu_run arch/x86/kvm/x86.c:6826
 [] kvm_arch_vcpu_ioctl_run+0x3b13/0x5a90
arch/x86/kvm/x86.c:6984
 [] kvm_vcpu_ioctl+0x61e/0xdd0
arch/x86/kvm/../../../virt/kvm/kvm_main.c:2557
 [< inline >] vfs_ioctl fs/ioctl.c:43
 [] do_vfs_ioctl+0x18c/0x1040 fs/ioctl.c:679
 [< inline >] SYSC_ioctl fs/ioctl.c:694
 [] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:685
 [] entry_SYSCALL_64_fastpath+0x1f/0xc2
Dumping ftrace buffer:
   (ftrace buffer empty)
Kernel Offset: disabled
reboot: cpu_has_vmx: ecx=80a02021 1

Re: [PATCH RFC] mm: Add debug_virt_to_phys()

[Florian Fainelli]

Le 11/11/2016 à 17:49, Nicolas Pitre a écrit :
> On Fri, 11 Nov 2016, Florian Fainelli wrote:
> 
>> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
>> debug_virt_to_phys() which helps catch vmalloc space addresses being
>> passed. This is helpful in debugging bogus drivers that just assume
>> linear mappings all over the place.
>>
>> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
>> __virt_to_phys() as being the functional implementation of the address
>> translation, so we special case the debug stub to call into
>> __virt_to_phys directly.
>>
>> Signed-off-by: Florian Fainelli 
>> ---
>>  arch/arm/include/asm/memory.h  |  4 
>>  arch/arm64/include/asm/memory.h|  4 
>>  include/asm-generic/memory_model.h |  4 
>>  mm/debug.c | 15 +++
>>  4 files changed, 27 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
>> index 76cbd9c674df..448dec9b8b00 100644
>> --- a/arch/arm/include/asm/memory.h
>> +++ b/arch/arm/include/asm/memory.h
>> @@ -260,11 +260,15 @@ static inline unsigned long __phys_to_virt(phys_addr_t 
>> x)
>>   * translation for translating DMA addresses.  Use the driver
>>   * DMA support - see dma-mapping.h.
>>   */
>> +#ifndef CONFIG_DEBUG_VM
>>  #define virt_to_phys virt_to_phys
>>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>>  {
>>  return __virt_to_phys((unsigned long)(x));
>>  }
>> +#else
>> +#define virt_to_phys debug_virt_to_phys
>> +#endif
> [...]
> 
> Why don't you do something more like:
> 
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
> +debug_virt_to_phys(x);
>  return __virt_to_phys((unsigned long)(x));
>  }
> 
> [...]
> 
> static inline void debug_virt_to_phys(const void *address)
> {
> #ifdef CONFIG_DEBUG_VM
> BUG_ON(is_vmalloc_addr(address));
> #endif
> }
> 
> ?

This is how I started doing it initially, but to get the
is_vmalloc_addr() definition, we need to include linux/mm.h and then
everything falls apart because of the include and dependencies chain. We
could open code the is_vmalloc_addr() check because that's simple
enough, but we still need VMALLOC_START and VMALLOC_END and to get there
we need to include pgtable.h, and there are still some inclusion
problems in doing so.

The other reason was to avoid putting the same checks in architecture
specific code, except for those like ARM/ARM64/Unicore32/Microblaze
where I could not find a simple way to undefined virt_to_phys and
redefine it to debug_virt_to_phys.

Do you see an other way around this? Thanks!
-- 
Florian

Re: [PATCH RFC] mm: Add debug_virt_to_phys()

[Florian Fainelli]

Le 11/11/2016 à 17:49, Nicolas Pitre a écrit :
> On Fri, 11 Nov 2016, Florian Fainelli wrote:
> 
>> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
>> debug_virt_to_phys() which helps catch vmalloc space addresses being
>> passed. This is helpful in debugging bogus drivers that just assume
>> linear mappings all over the place.
>>
>> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
>> __virt_to_phys() as being the functional implementation of the address
>> translation, so we special case the debug stub to call into
>> __virt_to_phys directly.
>>
>> Signed-off-by: Florian Fainelli 
>> ---
>>  arch/arm/include/asm/memory.h  |  4 
>>  arch/arm64/include/asm/memory.h|  4 
>>  include/asm-generic/memory_model.h |  4 
>>  mm/debug.c | 15 +++
>>  4 files changed, 27 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
>> index 76cbd9c674df..448dec9b8b00 100644
>> --- a/arch/arm/include/asm/memory.h
>> +++ b/arch/arm/include/asm/memory.h
>> @@ -260,11 +260,15 @@ static inline unsigned long __phys_to_virt(phys_addr_t 
>> x)
>>   * translation for translating DMA addresses.  Use the driver
>>   * DMA support - see dma-mapping.h.
>>   */
>> +#ifndef CONFIG_DEBUG_VM
>>  #define virt_to_phys virt_to_phys
>>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>>  {
>>  return __virt_to_phys((unsigned long)(x));
>>  }
>> +#else
>> +#define virt_to_phys debug_virt_to_phys
>> +#endif
> [...]
> 
> Why don't you do something more like:
> 
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
> +debug_virt_to_phys(x);
>  return __virt_to_phys((unsigned long)(x));
>  }
> 
> [...]
> 
> static inline void debug_virt_to_phys(const void *address)
> {
> #ifdef CONFIG_DEBUG_VM
> BUG_ON(is_vmalloc_addr(address));
> #endif
> }
> 
> ?

This is how I started doing it initially, but to get the
is_vmalloc_addr() definition, we need to include linux/mm.h and then
everything falls apart because of the include and dependencies chain. We
could open code the is_vmalloc_addr() check because that's simple
enough, but we still need VMALLOC_START and VMALLOC_END and to get there
we need to include pgtable.h, and there are still some inclusion
problems in doing so.

The other reason was to avoid putting the same checks in architecture
specific code, except for those like ARM/ARM64/Unicore32/Microblaze
where I could not find a simple way to undefined virt_to_phys and
redefine it to debug_virt_to_phys.

Do you see an other way around this? Thanks!
-- 
Florian

Re: [git pull] vfs.git

[Yan, Zheng]

> On 12 Nov 2016, at 01:25, Linus Torvalds  
> wrote:
> 
> On Thu, Nov 10, 2016 at 10:05 PM, Al Viro  wrote:
>>Christoph's and Jan's aio fixes, fixup for generic_file_splice_read
>> (removal of pointless detritus that actually breaks it when used for gfs2
>> ->splice_read()) and fixup for generic_file_read_iter() interaction with
>> ITER_PIPE destinations.
> 
> Hmm. I also just pulled the Ceph update that has commit 8a8d56176635
> ("ceph: use default file splice read callback"). I _think_ this splice
> fix makes that ceph change unnecessary. But testing is always good.

The commit is still needed. Al only fixes ITER_PIPE interaction with direct_IO. 
(it’s a no-op)
Cephfs case is special. Depending on what capabilities client has, client is 
allowed or
disallowed to read data from page cache. MDS changes client’s capabilities 
dynamically.
We don’t want to splice read fail when client is disallowed to get page from 
page cache.

Regards
Yan, Zheng 



> Ilya? Can you double-check the current -git tree (well, what I *will*
> push out soon after it has passed my build tests)?
> 
> Because I think Ceph can go back to using generic_file_splice_read again.
> 
>  Linus

Re: [git pull] vfs.git

[Yan, Zheng]

> On 12 Nov 2016, at 01:25, Linus Torvalds  
> wrote:
> 
> On Thu, Nov 10, 2016 at 10:05 PM, Al Viro  wrote:
>>Christoph's and Jan's aio fixes, fixup for generic_file_splice_read
>> (removal of pointless detritus that actually breaks it when used for gfs2
>> ->splice_read()) and fixup for generic_file_read_iter() interaction with
>> ITER_PIPE destinations.
> 
> Hmm. I also just pulled the Ceph update that has commit 8a8d56176635
> ("ceph: use default file splice read callback"). I _think_ this splice
> fix makes that ceph change unnecessary. But testing is always good.

The commit is still needed. Al only fixes ITER_PIPE interaction with direct_IO. 
(it’s a no-op)
Cephfs case is special. Depending on what capabilities client has, client is 
allowed or
disallowed to read data from page cache. MDS changes client’s capabilities 
dynamically.
We don’t want to splice read fail when client is disallowed to get page from 
page cache.

Regards
Yan, Zheng 



> Ilya? Can you double-check the current -git tree (well, what I *will*
> push out soon after it has passed my build tests)?
> 
> Because I think Ceph can go back to using generic_file_splice_read again.
> 
>  Linus

Re: [PATCH 3.12 89/91] PM / devfreq: Fix incorrect type issue.

[Ben Hutchings]

On Tue, 2016-01-05 at 18:47 +0100, Jiri Slaby wrote:
> From: Xiaolong Ye 
> 
> 3.12-stable review patch.  If anyone has any objections, please let
> me know.
> 
> ===
> 
> commit 5f25f066f75a67835abb5e400471a27abd09395b upstream.
> 
> time_in_state in struct devfreq is defined as unsigned long, so
> devm_kzalloc should use sizeof(unsigned long) as argument instead
> of sizeof(unsigned int), otherwise it will cause unexpected result
> in 64bit system.
> 
> Signed-off-by: Xiaolong Ye 
> Signed-off-by: Kevin Liu 
> Signed-off-by: MyungJoo Ham 
> Cc: Oliver Neukum 
> Signed-off-by: Jiri Slaby 
[...]

This is still needed in longterm branches 3.10 - 4.1 inclusive.  I just
queued it up for 3.16.

Ben.

-- 
Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.



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

Re: [PATCH 3.12 89/91] PM / devfreq: Fix incorrect type issue.

[Ben Hutchings]

On Tue, 2016-01-05 at 18:47 +0100, Jiri Slaby wrote:
> From: Xiaolong Ye 
> 
> 3.12-stable review patch.  If anyone has any objections, please let
> me know.
> 
> ===
> 
> commit 5f25f066f75a67835abb5e400471a27abd09395b upstream.
> 
> time_in_state in struct devfreq is defined as unsigned long, so
> devm_kzalloc should use sizeof(unsigned long) as argument instead
> of sizeof(unsigned int), otherwise it will cause unexpected result
> in 64bit system.
> 
> Signed-off-by: Xiaolong Ye 
> Signed-off-by: Kevin Liu 
> Signed-off-by: MyungJoo Ham 
> Cc: Oliver Neukum 
> Signed-off-by: Jiri Slaby 
[...]

This is still needed in longterm branches 3.10 - 4.1 inclusive.  I just
queued it up for 3.16.

Ben.

-- 
Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.



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

Re: [PATCH V6 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping

[agustinv]

Hey Lorenzo, Hanjun,

On 2016-11-11 08:33, Hanjun Guo wrote:

Hi Lorenzo,

On 11/11/2016 01:58 AM, Lorenzo Pieralisi wrote:
On Thu, Nov 10, 2016 at 10:02:35AM -0500, agust...@codeaurora.org 
wrote:

Hey Hanjun,

On 2016-11-09 21:36, Hanjun Guo wrote:

Hi Marc, Rafael, Lorenzo,

Since we agreed to add a probe deferral if we failed to get irq
resources which mirroring the DT does (patch 1 in this patch set),
I think the last blocker to make things work both for Agustin and
me [1] is this patch, which makes the interrupt producer and 
consumer
work in ACPI, we have two different solution for one thing, we'd 
happy

to work together for one solution, could you give some suggestions
please?

[1]: 
https://mail-archive.com/linux-kernel@vger.kernel.org/msg1257419.html


Agustin, I have some comments below.

On 2016/10/29 4:48, Agustin Vega-Frias wrote:

This allows irqchip drivers to associate an ACPI DSDT device to
an IRQ domain and provides support for using the ResourceSource
in Extended IRQ Resources to find the domain and map the IRQs
specified on that domain.

Signed-off-by: Agustin Vega-Frias 
---
drivers/acpi/Makefile|   1 +
drivers/acpi/irqdomain.c | 119
+++


Could we just reuse the gsi.c and not introduce a new
file, probably we can change the gsi.c to irqdomain.c
or something similar, then reuse the code in gsi.c.


I was thinking just that after we chatted off-list.


Yes, that's a fair point.

I might revisit and see what I come up with given that we already 
have

a device argument and we could pass the IRQ source there.


I agree with the approach taken by this patch, I do not like much
passing around struct acpi_resource_source *source (in particular
the dummy struct) I do not think it is needed, I will comment on
the code.


thanks for your time to have a look:)



Hopefully there is not any buggy FW out there that does use the
resource source inappropriately otherwise we will notice on x86/ia64
(ie you can't blame FW if it breaks the kernel) but I suspect the
only way to find out is by trying, the patch has to go through 
Rafael's

review anyway before getting there so it is fine.


I think we can avoid that by not touching the logic that x86/ia64
already used, but only adding interrupt producer/consumer function.


I looked at this more today and implemented a new patch that I plan to
test over the weekend, but I wanted to let you know the approach I am
pursuing.

On the new patch use of ResourceSource when parsing ACPI Extended IRQ
Resources is conditional on CONFIG_ACPI_GENERIC_GSI. The reason for this
is two fold:

1. Since we wanted to reduce duplication and place the new APIs on the
   same source file as acpi_register_gsi, which is already under that
   config flag.
2. So the patch does not have effect on platforms not using the generic
   GSI support, including x86/ia64.

If support for this is needed outside platforms using the generic GSI
implementation, we can move these APIs out to their own source file
and eliminate the CONFIG_ACPI_GENERIC_GSI conditionality.

I'll send the new patch, hopefully some time tomorrow, but please let
me know if you have concerns with this approach.

Thanks,
Agustin

--
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a 
Linux Foundation Collaborative Project.

Re: [PATCH V6 2/3] ACPI: Add support for ResourceSource/IRQ domain mapping

[agustinv]

Hey Lorenzo, Hanjun,

On 2016-11-11 08:33, Hanjun Guo wrote:

Hi Lorenzo,

On 11/11/2016 01:58 AM, Lorenzo Pieralisi wrote:
On Thu, Nov 10, 2016 at 10:02:35AM -0500, agust...@codeaurora.org 
wrote:

Hey Hanjun,

On 2016-11-09 21:36, Hanjun Guo wrote:

Hi Marc, Rafael, Lorenzo,

Since we agreed to add a probe deferral if we failed to get irq
resources which mirroring the DT does (patch 1 in this patch set),
I think the last blocker to make things work both for Agustin and
me [1] is this patch, which makes the interrupt producer and 
consumer
work in ACPI, we have two different solution for one thing, we'd 
happy

to work together for one solution, could you give some suggestions
please?

[1]: 
https://mail-archive.com/linux-kernel@vger.kernel.org/msg1257419.html


Agustin, I have some comments below.

On 2016/10/29 4:48, Agustin Vega-Frias wrote:

This allows irqchip drivers to associate an ACPI DSDT device to
an IRQ domain and provides support for using the ResourceSource
in Extended IRQ Resources to find the domain and map the IRQs
specified on that domain.

Signed-off-by: Agustin Vega-Frias 
---
drivers/acpi/Makefile|   1 +
drivers/acpi/irqdomain.c | 119
+++


Could we just reuse the gsi.c and not introduce a new
file, probably we can change the gsi.c to irqdomain.c
or something similar, then reuse the code in gsi.c.


I was thinking just that after we chatted off-list.


Yes, that's a fair point.

I might revisit and see what I come up with given that we already 
have

a device argument and we could pass the IRQ source there.


I agree with the approach taken by this patch, I do not like much
passing around struct acpi_resource_source *source (in particular
the dummy struct) I do not think it is needed, I will comment on
the code.


thanks for your time to have a look:)



Hopefully there is not any buggy FW out there that does use the
resource source inappropriately otherwise we will notice on x86/ia64
(ie you can't blame FW if it breaks the kernel) but I suspect the
only way to find out is by trying, the patch has to go through 
Rafael's

review anyway before getting there so it is fine.


I think we can avoid that by not touching the logic that x86/ia64
already used, but only adding interrupt producer/consumer function.


I looked at this more today and implemented a new patch that I plan to
test over the weekend, but I wanted to let you know the approach I am
pursuing.

On the new patch use of ResourceSource when parsing ACPI Extended IRQ
Resources is conditional on CONFIG_ACPI_GENERIC_GSI. The reason for this
is two fold:

1. Since we wanted to reduce duplication and place the new APIs on the
   same source file as acpi_register_gsi, which is already under that
   config flag.
2. So the patch does not have effect on platforms not using the generic
   GSI support, including x86/ia64.

If support for this is needed outside platforms using the generic GSI
implementation, we can move these APIs out to their own source file
and eliminate the CONFIG_ACPI_GENERIC_GSI conditionality.

I'll send the new patch, hopefully some time tomorrow, but please let
me know if you have concerns with this approach.

Thanks,
Agustin

--
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a 
Linux Foundation Collaborative Project.

kvm: GPF in kvm_pic_set_irq

[Dmitry Vyukov]

Hello,

The following program triggers GPF in kvm_pic_set_irq in run in a parallel loop:
https://gist.githubusercontent.com/dvyukov/0d868a3fcec50758ccf638dc87827ab3/raw/526bb6285785137e5d7c89dad2bdd05c66337daf/gistfile1.txt

On commit 015ed9433be2b476ec7e2e6a9a411a56e3b5b035 (Nov 11).

general protection fault:  [#1] SMP KASAN
Dumping ftrace buffer:
   (ftrace buffer empty)
Modules linked in:
CPU: 0 PID: 3308 Comm: kvm-pit/3270 Not tainted 4.9.0-rc4+ #42
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: 88003b85adc0 task.stack: 8800387e8000
RIP: 0010:[]  []
__lock_acquire+0x125/0x3630 kernel/locking/lockdep.c:3221
RSP: 0018:8800387ef8b0  EFLAGS: 00010006
RAX: dc00 RBX: 88003b85adc0 RCX: 
RDX: 0003 RSI:  RDI: 0018
RBP: 8800387efa48 R08: 0001 R09: 
R10: 84da1600 R11: 1100070fdf2c R12: 88003b85adc0
R13: 83c19020 R14: 0001 R15: 0018
FS:  () GS:88003ec0() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 006e CR3: 6c006000 CR4: 26f0
Stack:
  8800387ef8c8 0046 8800387ef980
 0046 88003b85adc0 88003b85adc0 88003b85b5c0
 88003b85adc0 83c19020 88003b85b5e2 
Call Trace:
 [] lock_acquire+0x169/0x330 kernel/locking/lockdep.c:3746
 [< inline >] __raw_spin_lock include/linux/spinlock_api_smp.h:144
 [] _raw_spin_lock+0x33/0x50 kernel/locking/spinlock.c:151
 [< inline >] pic_lock include/linux/spinlock.h:302
 [] kvm_pic_set_irq+0x3a/0x720 arch/x86/kvm/i8259.c:197
 [] kvm_set_pic_irq+0x7c/0xa0 arch/x86/kvm/irq_comm.c:44
 [] kvm_set_irq+0x1c9/0x460
arch/x86/kvm/../../../virt/kvm/irqchip.c:101
 [] pit_do_work+0xdb/0x2d0 arch/x86/kvm/i8254.c:250
 [] kthread_worker_fn+0x37e/0x780 kernel/kthread.c:624
 [] kthread+0x244/0x2d0 kernel/kthread.c:209
 [] ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:433
Code: 0f 1f 44 00 00 f6 c4 02 0f 85 5d 0a 00 00 44 8b 35 41 b6 7a 02
45 85 f6 74 2c 4c 89 fa 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80>
3c 02 00 0f 85 6e 30 00 00 49 81 3f 60 d0 10 84 41 be 00 00
RIP  [] __lock_acquire+0x125/0x3630
kernel/locking/lockdep.c:3221
 RSP 
---[ end trace b74c04baf1b86ffa ]---
Kernel panic - not syncing: Fatal exception
Dumping ftrace buffer:
   (ftrace buffer empty)
Kernel Offset: disabled
reboot: cpu_has_vmx: ecx=80a02021 1

kvm: GPF in kvm_pic_set_irq

[Dmitry Vyukov]

Hello,

The following program triggers GPF in kvm_pic_set_irq in run in a parallel loop:
https://gist.githubusercontent.com/dvyukov/0d868a3fcec50758ccf638dc87827ab3/raw/526bb6285785137e5d7c89dad2bdd05c66337daf/gistfile1.txt

On commit 015ed9433be2b476ec7e2e6a9a411a56e3b5b035 (Nov 11).

general protection fault:  [#1] SMP KASAN
Dumping ftrace buffer:
   (ftrace buffer empty)
Modules linked in:
CPU: 0 PID: 3308 Comm: kvm-pit/3270 Not tainted 4.9.0-rc4+ #42
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: 88003b85adc0 task.stack: 8800387e8000
RIP: 0010:[]  []
__lock_acquire+0x125/0x3630 kernel/locking/lockdep.c:3221
RSP: 0018:8800387ef8b0  EFLAGS: 00010006
RAX: dc00 RBX: 88003b85adc0 RCX: 
RDX: 0003 RSI:  RDI: 0018
RBP: 8800387efa48 R08: 0001 R09: 
R10: 84da1600 R11: 1100070fdf2c R12: 88003b85adc0
R13: 83c19020 R14: 0001 R15: 0018
FS:  () GS:88003ec0() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 006e CR3: 6c006000 CR4: 26f0
Stack:
  8800387ef8c8 0046 8800387ef980
 0046 88003b85adc0 88003b85adc0 88003b85b5c0
 88003b85adc0 83c19020 88003b85b5e2 
Call Trace:
 [] lock_acquire+0x169/0x330 kernel/locking/lockdep.c:3746
 [< inline >] __raw_spin_lock include/linux/spinlock_api_smp.h:144
 [] _raw_spin_lock+0x33/0x50 kernel/locking/spinlock.c:151
 [< inline >] pic_lock include/linux/spinlock.h:302
 [] kvm_pic_set_irq+0x3a/0x720 arch/x86/kvm/i8259.c:197
 [] kvm_set_pic_irq+0x7c/0xa0 arch/x86/kvm/irq_comm.c:44
 [] kvm_set_irq+0x1c9/0x460
arch/x86/kvm/../../../virt/kvm/irqchip.c:101
 [] pit_do_work+0xdb/0x2d0 arch/x86/kvm/i8254.c:250
 [] kthread_worker_fn+0x37e/0x780 kernel/kthread.c:624
 [] kthread+0x244/0x2d0 kernel/kthread.c:209
 [] ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:433
Code: 0f 1f 44 00 00 f6 c4 02 0f 85 5d 0a 00 00 44 8b 35 41 b6 7a 02
45 85 f6 74 2c 4c 89 fa 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80>
3c 02 00 0f 85 6e 30 00 00 49 81 3f 60 d0 10 84 41 be 00 00
RIP  [] __lock_acquire+0x125/0x3630
kernel/locking/lockdep.c:3221
 RSP 
---[ end trace b74c04baf1b86ffa ]---
Kernel panic - not syncing: Fatal exception
Dumping ftrace buffer:
   (ftrace buffer empty)
Kernel Offset: disabled
reboot: cpu_has_vmx: ecx=80a02021 1

Re: [RFC 00/17] clk: Add per-controller locks to fix deadlocks

[Stephen Boyd]

On 11/04, Marek Szyprowski wrote:
> Hi Stephen,
> 
> Krzysztof has left Samsung, but we would like to continue this task, because
> the ABBA dead-locks related to global prepare lock becomes more and more
> problematic for us to workaround.

Hmm. Ok. Thanks for the info.

> 
> On 2016-09-09 02:24, Stephen Boyd wrote:
> 
> >So I'm not very fond of this design because the locking scheme is
> >pretty much out of the hands of the framework and can be easily
> >broken.
> 
> Well, switching from a single global lock to more granular locking
> is always a good approach. Please note that the global lock sooner
> or later became a serious bottleneck if one wants to make somehow
> more aggressive power management and clock gating.

I'm not so sure switching from a global lock to a more granular
lock is _always_ a great idea. Sometimes simpler code is better,
even if it doesn't scale to a million clk nodes. The largest
systems I've seen only have clocks in the hundreds, and a
majority of those aren't rate changing in parallel, so it's not
like we're suffering from VFS type scalability problems here with
tens of thousands of inodes.

That isn't to say I don't agree there's a scalability problem
here, but I'd like to actually see numbers to prove that there's
some sort of scalability problem before making drastic changes.

> 
> >  I'm biased of course, because I'd prefer we go with my
> >wwmutex design of per-clk locks[1]. Taking locks in any order
> >works fine there, and we resolve quite a few long standing
> >locking problems that we have while improving scalability. The
> >problem there is that we don't get the recursive mutex design
> >(maybe that's a benefit!).
> 
> Do you have any plan to continue working on your approach? per-clk
> wwmutex looks like an overkill on the first glance, but that's probably
> the only working solution if you want to get rid of recursive locks.
> I'm still not really convinced that we really need wwmutex here,
> especially if it is possible to guarantee the same order of locking
> operations inside the clock core. This requires a bit of cooperation
> from clock providers (with proper documentation and a list of
> DO/DON'T it shouldn't be that hard).

So far I haven't gotten around to resurrecting the wwmutex stuff.
If you have interest in doing it that's great. Having a locking
scheme with rules of DO/DON'T sounds brittle to me, unless it can
be automated to find problems. I know that I'm not going to spend
time policing that.

> 
> >Once a clk_op reenters the framework
> >with consumer APIs and tries to grab the same lock we deadlock.
> >This is why I've been slowly splitting consumers from providers
> >so we can easily identify these cases. If we had something like
> >coordinated clk rate switching, we could get rid of clk_ops
> >reentering the framework and avoid this problem (and we really do
> >need to do that).
> 
> I'm not sure that this makes really sense split consumers and
> providers. You will get recursive calls to clk core anyway with
> consumers calls if you are implementing i2c clock, for which an i2c
> bus driver does it's own clock gating (i2c controller uses
> consumer clk api).
> 
> 

I suppose this is a different topic. Regardless of the recursive
call or not, we can easily see that a clk consumer is also a clk
provider and just knowing that is useful. Once we know that, we
can look to see if they're calling clk consumer APIs from their
provider callbacks which is not desired because it makes it
impossible to get rid of the recursive lock design. If the lock
is per-clock, then recursion doesn't happen when the provider is
also a consumer. If it does, that's bad and lockdep should tell
us.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC 00/17] clk: Add per-controller locks to fix deadlocks

[Stephen Boyd]

On 11/04, Marek Szyprowski wrote:
> Hi Stephen,
> 
> Krzysztof has left Samsung, but we would like to continue this task, because
> the ABBA dead-locks related to global prepare lock becomes more and more
> problematic for us to workaround.

Hmm. Ok. Thanks for the info.

> 
> On 2016-09-09 02:24, Stephen Boyd wrote:
> 
> >So I'm not very fond of this design because the locking scheme is
> >pretty much out of the hands of the framework and can be easily
> >broken.
> 
> Well, switching from a single global lock to more granular locking
> is always a good approach. Please note that the global lock sooner
> or later became a serious bottleneck if one wants to make somehow
> more aggressive power management and clock gating.

I'm not so sure switching from a global lock to a more granular
lock is _always_ a great idea. Sometimes simpler code is better,
even if it doesn't scale to a million clk nodes. The largest
systems I've seen only have clocks in the hundreds, and a
majority of those aren't rate changing in parallel, so it's not
like we're suffering from VFS type scalability problems here with
tens of thousands of inodes.

That isn't to say I don't agree there's a scalability problem
here, but I'd like to actually see numbers to prove that there's
some sort of scalability problem before making drastic changes.

> 
> >  I'm biased of course, because I'd prefer we go with my
> >wwmutex design of per-clk locks[1]. Taking locks in any order
> >works fine there, and we resolve quite a few long standing
> >locking problems that we have while improving scalability. The
> >problem there is that we don't get the recursive mutex design
> >(maybe that's a benefit!).
> 
> Do you have any plan to continue working on your approach? per-clk
> wwmutex looks like an overkill on the first glance, but that's probably
> the only working solution if you want to get rid of recursive locks.
> I'm still not really convinced that we really need wwmutex here,
> especially if it is possible to guarantee the same order of locking
> operations inside the clock core. This requires a bit of cooperation
> from clock providers (with proper documentation and a list of
> DO/DON'T it shouldn't be that hard).

So far I haven't gotten around to resurrecting the wwmutex stuff.
If you have interest in doing it that's great. Having a locking
scheme with rules of DO/DON'T sounds brittle to me, unless it can
be automated to find problems. I know that I'm not going to spend
time policing that.

> 
> >Once a clk_op reenters the framework
> >with consumer APIs and tries to grab the same lock we deadlock.
> >This is why I've been slowly splitting consumers from providers
> >so we can easily identify these cases. If we had something like
> >coordinated clk rate switching, we could get rid of clk_ops
> >reentering the framework and avoid this problem (and we really do
> >need to do that).
> 
> I'm not sure that this makes really sense split consumers and
> providers. You will get recursive calls to clk core anyway with
> consumers calls if you are implementing i2c clock, for which an i2c
> bus driver does it's own clock gating (i2c controller uses
> consumer clk api).
> 
> 

I suppose this is a different topic. Regardless of the recursive
call or not, we can easily see that a clk consumer is also a clk
provider and just knowing that is useful. Once we know that, we
can look to see if they're calling clk consumer APIs from their
provider callbacks which is not desired because it makes it
impossible to get rid of the recursive lock design. If the lock
is per-clock, then recursion doesn't happen when the provider is
also a consumer. If it does, that's bad and lockdep should tell
us.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] [STYLE 2/2]staging:speakup:speakup_ltlk.c Preferred space around

[Walt Feasel]

Made suggested modifications from checkpatch in reference
to: CHECK: spaces preferred around that '|,+'

Signed-off-by: Walt Feasel 
---
 drivers/staging/speakup/speakup_ltlk.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/speakup/speakup_ltlk.c 
b/drivers/staging/speakup/speakup_ltlk.c
index dc4239f..2a6ccc7 100644
--- a/drivers/staging/speakup/speakup_ltlk.c
+++ b/drivers/staging/speakup/speakup_ltlk.c
@@ -45,34 +45,34 @@ static struct var_t vars[] = {
 /* These attributes will appear in /sys/accessibility/speakup/ltlk. */
 
 static struct kobj_attribute caps_start_attribute =
-   __ATTR(caps_start, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(caps_start, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute caps_stop_attribute =
-   __ATTR(caps_stop, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(caps_stop, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute freq_attribute =
-   __ATTR(freq, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(freq, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute pitch_attribute =
-   __ATTR(pitch, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(pitch, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute punct_attribute =
-   __ATTR(punct, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(punct, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute rate_attribute =
-   __ATTR(rate, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(rate, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute tone_attribute =
-   __ATTR(tone, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(tone, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute voice_attribute =
-   __ATTR(voice, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(voice, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute vol_attribute =
-   __ATTR(vol, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(vol, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 
 static struct kobj_attribute delay_time_attribute =
-   __ATTR(delay_time, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(delay_time, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute direct_attribute =
-   __ATTR(direct, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(direct, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute full_time_attribute =
-   __ATTR(full_time, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(full_time, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute jiffy_delta_attribute =
-   __ATTR(jiffy_delta, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(jiffy_delta, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute trigger_time_attribute =
-   __ATTR(trigger_time, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(trigger_time, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 
 /*
  * Create a group of attributes so that we can create and destroy them all
@@ -143,7 +143,7 @@ static void synth_interrogate(struct spk_synth *synth)
if (i > 2 && buf[i] == 0x7f)
break;
}
-   t = buf+2;
+   t = buf + 2;
for (i = 0; *t != '\r'; t++) {
rom_v[i] = *t;
if (++i >= 19)
-- 
2.1.4

[PATCH] [STYLE 2/2]staging:speakup:speakup_ltlk.c Preferred space around

[Walt Feasel]

Made suggested modifications from checkpatch in reference
to: CHECK: spaces preferred around that '|,+'

Signed-off-by: Walt Feasel 
---
 drivers/staging/speakup/speakup_ltlk.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/speakup/speakup_ltlk.c 
b/drivers/staging/speakup/speakup_ltlk.c
index dc4239f..2a6ccc7 100644
--- a/drivers/staging/speakup/speakup_ltlk.c
+++ b/drivers/staging/speakup/speakup_ltlk.c
@@ -45,34 +45,34 @@ static struct var_t vars[] = {
 /* These attributes will appear in /sys/accessibility/speakup/ltlk. */
 
 static struct kobj_attribute caps_start_attribute =
-   __ATTR(caps_start, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(caps_start, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute caps_stop_attribute =
-   __ATTR(caps_stop, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(caps_stop, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute freq_attribute =
-   __ATTR(freq, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(freq, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute pitch_attribute =
-   __ATTR(pitch, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(pitch, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute punct_attribute =
-   __ATTR(punct, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(punct, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute rate_attribute =
-   __ATTR(rate, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(rate, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute tone_attribute =
-   __ATTR(tone, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(tone, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute voice_attribute =
-   __ATTR(voice, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(voice, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute vol_attribute =
-   __ATTR(vol, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(vol, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 
 static struct kobj_attribute delay_time_attribute =
-   __ATTR(delay_time, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(delay_time, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute direct_attribute =
-   __ATTR(direct, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(direct, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute full_time_attribute =
-   __ATTR(full_time, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(full_time, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute jiffy_delta_attribute =
-   __ATTR(jiffy_delta, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(jiffy_delta, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute trigger_time_attribute =
-   __ATTR(trigger_time, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(trigger_time, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 
 /*
  * Create a group of attributes so that we can create and destroy them all
@@ -143,7 +143,7 @@ static void synth_interrogate(struct spk_synth *synth)
if (i > 2 && buf[i] == 0x7f)
break;
}
-   t = buf+2;
+   t = buf + 2;
for (i = 0; *t != '\r'; t++) {
rom_v[i] = *t;
if (++i >= 19)
-- 
2.1.4

[PATCH] [STYLE 1/2]staging:speakup:speakup_ltlk.c Block align on *

[Walt Feasel]

Made suggested modifications from checkpatch in reference
to: WARNING: Block comments should align the * on each line
Modified multi line comment to single
Removed blankline at end

Signed-off-by: Walt Feasel 
---
 drivers/staging/speakup/speakup_ltlk.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/speakup/speakup_ltlk.c 
b/drivers/staging/speakup/speakup_ltlk.c
index b474e8b..dc4239f 100644
--- a/drivers/staging/speakup/speakup_ltlk.c
+++ b/drivers/staging/speakup/speakup_ltlk.c
@@ -1,6 +1,6 @@
 /*
  * originally written by: Kirk Reiser 
-* this version considerably modified by David Borowski, david...@rogers.com
+ * this version considerably modified by David Borowski, david...@rogers.com
  *
  * Copyright (C) 1998-99  Kirk Reiser.
  * Copyright (C) 2003 David Borowski.
@@ -42,9 +42,8 @@ static struct var_t vars[] = {
V_LAST_VAR
 };
 
-/*
- * These attributes will appear in /sys/accessibility/speakup/ltlk.
- */
+/* These attributes will appear in /sys/accessibility/speakup/ltlk. */
+
 static struct kobj_attribute caps_start_attribute =
__ATTR(caps_start, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute caps_stop_attribute =
@@ -178,4 +177,3 @@ MODULE_AUTHOR("David Borowski");
 MODULE_DESCRIPTION("Speakup support for DoubleTalk LT/LiteTalk synthesizers");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
-
-- 
2.1.4

[PATCH] [STYLE 1/2]staging:speakup:speakup_ltlk.c Block align on *

[Walt Feasel]

Made suggested modifications from checkpatch in reference
to: WARNING: Block comments should align the * on each line
Modified multi line comment to single
Removed blankline at end

Signed-off-by: Walt Feasel 
---
 drivers/staging/speakup/speakup_ltlk.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/speakup/speakup_ltlk.c 
b/drivers/staging/speakup/speakup_ltlk.c
index b474e8b..dc4239f 100644
--- a/drivers/staging/speakup/speakup_ltlk.c
+++ b/drivers/staging/speakup/speakup_ltlk.c
@@ -1,6 +1,6 @@
 /*
  * originally written by: Kirk Reiser 
-* this version considerably modified by David Borowski, david...@rogers.com
+ * this version considerably modified by David Borowski, david...@rogers.com
  *
  * Copyright (C) 1998-99  Kirk Reiser.
  * Copyright (C) 2003 David Borowski.
@@ -42,9 +42,8 @@ static struct var_t vars[] = {
V_LAST_VAR
 };
 
-/*
- * These attributes will appear in /sys/accessibility/speakup/ltlk.
- */
+/* These attributes will appear in /sys/accessibility/speakup/ltlk. */
+
 static struct kobj_attribute caps_start_attribute =
__ATTR(caps_start, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute caps_stop_attribute =
@@ -178,4 +177,3 @@ MODULE_AUTHOR("David Borowski");
 MODULE_DESCRIPTION("Speakup support for DoubleTalk LT/LiteTalk synthesizers");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
-
-- 
2.1.4

Re: [PATCH 3.16 289/305] netfilter: x_tables: validate targets of jumps

[Ben Hutchings]

On Sat, 2016-08-13 at 20:30 +0200, Florian Westphal wrote:
> Ben Hutchings  wrote:
> > 3.16.37-rc1 review patch.  If anyone has any objections, please let
> > me know.
> > 
> > --
> > 
> > From: Florian Westphal 
> > 
> > commit 36472341017529e2b12573093cc0f68719300997 upstream.
> 
> [..]
> 
> > The extra overhead is negible, even with absurd cases.
> 
> Not true, the overhead is huge and increases restore time for
> large rulesets from mere seconds to minutes, see
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commi
> t/?id=f4dc77713f8016d2e8a3295e1c9c53a21f296def

I've queued both of these up for the next update.

Ben.

-- 
Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.



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

Re: [PATCH 3.16 289/305] netfilter: x_tables: validate targets of jumps

[Ben Hutchings]

On Sat, 2016-08-13 at 20:30 +0200, Florian Westphal wrote:
> Ben Hutchings  wrote:
> > 3.16.37-rc1 review patch.  If anyone has any objections, please let
> > me know.
> > 
> > --
> > 
> > From: Florian Westphal 
> > 
> > commit 36472341017529e2b12573093cc0f68719300997 upstream.
> 
> [..]
> 
> > The extra overhead is negible, even with absurd cases.
> 
> Not true, the overhead is huge and increases restore time for
> large rulesets from mere seconds to minutes, see
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commi
> t/?id=f4dc77713f8016d2e8a3295e1c9c53a21f296def

I've queued both of these up for the next update.

Ben.

-- 
Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.



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

Re: [PATCH] Security: selinux - Improvement in code readability when

[Paul Moore]

On Fri, Nov 11, 2016 at 3:48 AM, Shailendra Verma
 wrote:
> From: "Shailendra Verma" 
>
> There is no need to call kfree() if memdup_user() fails, as no memory
> was allocated and the error in the error-valued pointer should be returned.
>
> Signed-off-by: Shailendra Verma 
> ---
>  security/selinux/selinuxfs.c |5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Hello Shailendra,

Thank you for your patch, but I prefer the readability of the code as
it currently stands.

-- 
paul moore
www.paul-moore.com

Re: [PATCH] Security: selinux - Improvement in code readability when

[Paul Moore]

On Fri, Nov 11, 2016 at 3:48 AM, Shailendra Verma
 wrote:
> From: "Shailendra Verma" 
>
> There is no need to call kfree() if memdup_user() fails, as no memory
> was allocated and the error in the error-valued pointer should be returned.
>
> Signed-off-by: Shailendra Verma 
> ---
>  security/selinux/selinuxfs.c |5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Hello Shailendra,

Thank you for your patch, but I prefer the readability of the code as
it currently stands.

-- 
paul moore
www.paul-moore.com

[PATCH] [STYLE 3/3]staging:speakup:speakup_keypc.c Align around parenthesis

[Walt Feasel]

Made suggested modifications from checkpatch in reference
to: CHECK: Alignment should match open parenthesis

Signed-off-by: Walt Feasel 
---
 drivers/staging/speakup/speakup_keypc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/speakup/speakup_keypc.c 
b/drivers/staging/speakup/speakup_keypc.c
index 4def8f3..e44232f 100644
--- a/drivers/staging/speakup/speakup_keypc.c
+++ b/drivers/staging/speakup/speakup_keypc.c
@@ -266,7 +266,7 @@ static int synth_probe(struct spk_synth *synth)
if (port_forced) {
synth_port = port_forced;
pr_info("probe forced to %x by kernel command line\n",
-   synth_port);
+   synth_port);
if (synth_request_region(synth_port - 1, SYNTH_IO_EXTENT)) {
pr_warn("sorry, port already reserved\n");
return -EBUSY;
@@ -275,7 +275,7 @@ static int synth_probe(struct spk_synth *synth)
} else {
for (i = 0; synth_portlist[i]; i++) {
if (synth_request_region(synth_portlist[i],
-   SYNTH_IO_EXTENT)) {
+SYNTH_IO_EXTENT)) {
pr_warn
("request_region: failed with 0x%x, %d\n",
 synth_portlist[i], SYNTH_IO_EXTENT);
-- 
2.1.4

[PATCH] [STYLE 3/3]staging:speakup:speakup_keypc.c Align around parenthesis

[Walt Feasel]

Made suggested modifications from checkpatch in reference
to: CHECK: Alignment should match open parenthesis

Signed-off-by: Walt Feasel 
---
 drivers/staging/speakup/speakup_keypc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/speakup/speakup_keypc.c 
b/drivers/staging/speakup/speakup_keypc.c
index 4def8f3..e44232f 100644
--- a/drivers/staging/speakup/speakup_keypc.c
+++ b/drivers/staging/speakup/speakup_keypc.c
@@ -266,7 +266,7 @@ static int synth_probe(struct spk_synth *synth)
if (port_forced) {
synth_port = port_forced;
pr_info("probe forced to %x by kernel command line\n",
-   synth_port);
+   synth_port);
if (synth_request_region(synth_port - 1, SYNTH_IO_EXTENT)) {
pr_warn("sorry, port already reserved\n");
return -EBUSY;
@@ -275,7 +275,7 @@ static int synth_probe(struct spk_synth *synth)
} else {
for (i = 0; synth_portlist[i]; i++) {
if (synth_request_region(synth_portlist[i],
-   SYNTH_IO_EXTENT)) {
+SYNTH_IO_EXTENT)) {
pr_warn
("request_region: failed with 0x%x, %d\n",
 synth_portlist[i], SYNTH_IO_EXTENT);
-- 
2.1.4

Re: [GIT PULL] SCSI fixes for 4.9-rc3

[Gabriel C]

On 12.11.2016 02:08, Kashyap Desai wrote:
>> -Original Message-
>> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
>> ow...@vger.kernel.org] On Behalf Of Gabriel C
>> Sent: Friday, November 11, 2016 9:40 AM
>> To: James Bottomley; Andrew Morton; Linus Torvalds
>> Cc: linux-scsi; linux-kernel; sta...@vger.kernel.org
>> Subject: Re: [GIT PULL] SCSI fixes for 4.9-rc3
>>
>>
>>
>> On 11.11.2016 04:30, Gabriel C wrote:
>>>
>>> On 05.11.2016 14:29, James Bottomley wrote:
>>>
>>>
>>> ...
>>>
 Kashyap Desai (1):
   scsi: megaraid_sas: Fix data integrity failure for JBOD
 (passthrough) devices

 diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
 b/drivers/scsi/megaraid/megaraid_sas_base.c
 index 9ff57de..d8b1fbd 100644
 --- a/drivers/scsi/megaraid/megaraid_sas_base.c
 +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
 @@ -1700,16 +1700,13 @@ megasas_queue_command(struct Scsi_Host
>> *shost, struct scsi_cmnd *scmd)
goto out_done;
}

 -  switch (scmd->cmnd[0]) {
 -  case SYNCHRONIZE_CACHE:
 -  /*
 -   * FW takes care of flush cache on its own
 -   * No need to send it down
 -   */
 +  /*
 +   * FW takes care of flush cache on its own for Virtual Disk.
 +   * No need to send it down for VD. For JBOD send
>> SYNCHRONIZE_CACHE to FW.
 +   */
 +  if ((scmd->cmnd[0] == SYNCHRONIZE_CACHE) &&
 +MEGASAS_IS_LOGICAL(scmd)) {
scmd->result = DID_OK << 16;
goto out_done;
 -  default:
 -  break;
}

return instance->instancet->build_and_issue_cmd(instance, scmd);
>>>
>>> This patch breaks my box.. I'm not able to boot it anymore.
>>> It seems with this patch I have /dev/sda[a-z] to /dev/sdz[a-z] ?!?
>>>
>>> I'm not sure how to get an log since dracut times out and I'm dropped
>>> , after a very long time of probing 'ghost devices', in a emercency
>>> shell,
>> journalctl doesn't work also..
>>>
>>> After reverting this one I can boot normal.
>>>
>>> Box is a FUJITSU PRIMERGY TX200 S5..
> 
> Please  check now commit.  Below commit has complete fix.
> 
> http://git.kernel.org/cgit/linux/kernel/git/jejb/scsi.git/commit/?id=5e5ec1759dd663a1d5a2f10930224dd009e500e8
> 


This patch fixes the problem for me. Thank you.


Regards,

Gabriel C

Re: [GIT PULL] SCSI fixes for 4.9-rc3

[Gabriel C]

On 12.11.2016 02:08, Kashyap Desai wrote:
>> -Original Message-
>> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
>> ow...@vger.kernel.org] On Behalf Of Gabriel C
>> Sent: Friday, November 11, 2016 9:40 AM
>> To: James Bottomley; Andrew Morton; Linus Torvalds
>> Cc: linux-scsi; linux-kernel; sta...@vger.kernel.org
>> Subject: Re: [GIT PULL] SCSI fixes for 4.9-rc3
>>
>>
>>
>> On 11.11.2016 04:30, Gabriel C wrote:
>>>
>>> On 05.11.2016 14:29, James Bottomley wrote:
>>>
>>>
>>> ...
>>>
 Kashyap Desai (1):
   scsi: megaraid_sas: Fix data integrity failure for JBOD
 (passthrough) devices

 diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
 b/drivers/scsi/megaraid/megaraid_sas_base.c
 index 9ff57de..d8b1fbd 100644
 --- a/drivers/scsi/megaraid/megaraid_sas_base.c
 +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
 @@ -1700,16 +1700,13 @@ megasas_queue_command(struct Scsi_Host
>> *shost, struct scsi_cmnd *scmd)
goto out_done;
}

 -  switch (scmd->cmnd[0]) {
 -  case SYNCHRONIZE_CACHE:
 -  /*
 -   * FW takes care of flush cache on its own
 -   * No need to send it down
 -   */
 +  /*
 +   * FW takes care of flush cache on its own for Virtual Disk.
 +   * No need to send it down for VD. For JBOD send
>> SYNCHRONIZE_CACHE to FW.
 +   */
 +  if ((scmd->cmnd[0] == SYNCHRONIZE_CACHE) &&
 +MEGASAS_IS_LOGICAL(scmd)) {
scmd->result = DID_OK << 16;
goto out_done;
 -  default:
 -  break;
}

return instance->instancet->build_and_issue_cmd(instance, scmd);
>>>
>>> This patch breaks my box.. I'm not able to boot it anymore.
>>> It seems with this patch I have /dev/sda[a-z] to /dev/sdz[a-z] ?!?
>>>
>>> I'm not sure how to get an log since dracut times out and I'm dropped
>>> , after a very long time of probing 'ghost devices', in a emercency
>>> shell,
>> journalctl doesn't work also..
>>>
>>> After reverting this one I can boot normal.
>>>
>>> Box is a FUJITSU PRIMERGY TX200 S5..
> 
> Please  check now commit.  Below commit has complete fix.
> 
> http://git.kernel.org/cgit/linux/kernel/git/jejb/scsi.git/commit/?id=5e5ec1759dd663a1d5a2f10930224dd009e500e8
> 


This patch fixes the problem for me. Thank you.


Regards,

Gabriel C

[PATCH] [STYLE 2/3]staging:speakup:speakup_keypc.c Preferred space around

[Walt Feasel]

Made suggested modifications from checkpatch in reference
to: CHECK: spaces preferred around that '|,+,-'

Signed-off-by: Walt Feasel 
---
 drivers/staging/speakup/speakup_keypc.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/speakup/speakup_keypc.c 
b/drivers/staging/speakup/speakup_keypc.c
index cadbe8d..4def8f3 100644
--- a/drivers/staging/speakup/speakup_keypc.c
+++ b/drivers/staging/speakup/speakup_keypc.c
@@ -54,24 +54,24 @@ static struct var_t vars[] = {
 /* These attributes will appear in /sys/accessibility/speakup/keypc. */
 
 static struct kobj_attribute caps_start_attribute =
-   __ATTR(caps_start, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(caps_start, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute caps_stop_attribute =
-   __ATTR(caps_stop, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(caps_stop, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute pitch_attribute =
-   __ATTR(pitch, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(pitch, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute rate_attribute =
-   __ATTR(rate, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(rate, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 
 static struct kobj_attribute delay_time_attribute =
-   __ATTR(delay_time, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(delay_time, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute direct_attribute =
-   __ATTR(direct, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(direct, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute full_time_attribute =
-   __ATTR(full_time, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(full_time, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute jiffy_delta_attribute =
-   __ATTR(jiffy_delta, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(jiffy_delta, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute trigger_time_attribute =
-   __ATTR(trigger_time, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(trigger_time, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 
 /*
  * Create a group of attributes so that we can create and destroy them all
@@ -140,9 +140,9 @@ static char *oops(void)
int s1, s2, s3, s4;
 
s1 = inb_p(synth_port);
-   s2 = inb_p(synth_port+1);
-   s3 = inb_p(synth_port+2);
-   s4 = inb_p(synth_port+3);
+   s2 = inb_p(synth_port + 1);
+   s3 = inb_p(synth_port + 2);
+   s4 = inb_p(synth_port + 3);
pr_warn("synth timeout %d %d %d %d\n", s1, s2, s3, s4);
return NULL;
 }
@@ -239,7 +239,7 @@ spin_lock_irqsave(_info.spinlock, flags);
delay_time_val = delay_time->u.n.value;
spin_unlock_irqrestore(_info.spinlock, flags);
schedule_timeout(msecs_to_jiffies(delay_time_val));
-   jiff_max = jiffies+jiffy_delta_val;
+   jiff_max = jiffies + jiffy_delta_val;
}
}
timeout = 1000;
@@ -267,7 +267,7 @@ static int synth_probe(struct spk_synth *synth)
synth_port = port_forced;
pr_info("probe forced to %x by kernel command line\n",
synth_port);
-   if (synth_request_region(synth_port-1, SYNTH_IO_EXTENT)) {
+   if (synth_request_region(synth_port - 1, SYNTH_IO_EXTENT)) {
pr_warn("sorry, port already reserved\n");
return -EBUSY;
}
@@ -295,7 +295,7 @@ static int synth_probe(struct spk_synth *synth)
return -ENODEV;
}
pr_info("%s: %03x-%03x, driver version %s,\n", synth->long_name,
-   synth_port, synth_port+SYNTH_IO_EXTENT-1,
+   synth_port, synth_port + SYNTH_IO_EXTENT - 1,
synth->version);
synth->alive = 1;
return 0;
-- 
2.1.4

[PATCH] [STYLE 2/3]staging:speakup:speakup_keypc.c Preferred space around

[Walt Feasel]

Made suggested modifications from checkpatch in reference
to: CHECK: spaces preferred around that '|,+,-'

Signed-off-by: Walt Feasel 
---
 drivers/staging/speakup/speakup_keypc.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/speakup/speakup_keypc.c 
b/drivers/staging/speakup/speakup_keypc.c
index cadbe8d..4def8f3 100644
--- a/drivers/staging/speakup/speakup_keypc.c
+++ b/drivers/staging/speakup/speakup_keypc.c
@@ -54,24 +54,24 @@ static struct var_t vars[] = {
 /* These attributes will appear in /sys/accessibility/speakup/keypc. */
 
 static struct kobj_attribute caps_start_attribute =
-   __ATTR(caps_start, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(caps_start, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute caps_stop_attribute =
-   __ATTR(caps_stop, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(caps_stop, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute pitch_attribute =
-   __ATTR(pitch, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(pitch, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute rate_attribute =
-   __ATTR(rate, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(rate, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 
 static struct kobj_attribute delay_time_attribute =
-   __ATTR(delay_time, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(delay_time, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute direct_attribute =
-   __ATTR(direct, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(direct, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute full_time_attribute =
-   __ATTR(full_time, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(full_time, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute jiffy_delta_attribute =
-   __ATTR(jiffy_delta, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(jiffy_delta, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute trigger_time_attribute =
-   __ATTR(trigger_time, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
+   __ATTR(trigger_time, S_IWUSR | S_IRUGO, spk_var_show, spk_var_store);
 
 /*
  * Create a group of attributes so that we can create and destroy them all
@@ -140,9 +140,9 @@ static char *oops(void)
int s1, s2, s3, s4;
 
s1 = inb_p(synth_port);
-   s2 = inb_p(synth_port+1);
-   s3 = inb_p(synth_port+2);
-   s4 = inb_p(synth_port+3);
+   s2 = inb_p(synth_port + 1);
+   s3 = inb_p(synth_port + 2);
+   s4 = inb_p(synth_port + 3);
pr_warn("synth timeout %d %d %d %d\n", s1, s2, s3, s4);
return NULL;
 }
@@ -239,7 +239,7 @@ spin_lock_irqsave(_info.spinlock, flags);
delay_time_val = delay_time->u.n.value;
spin_unlock_irqrestore(_info.spinlock, flags);
schedule_timeout(msecs_to_jiffies(delay_time_val));
-   jiff_max = jiffies+jiffy_delta_val;
+   jiff_max = jiffies + jiffy_delta_val;
}
}
timeout = 1000;
@@ -267,7 +267,7 @@ static int synth_probe(struct spk_synth *synth)
synth_port = port_forced;
pr_info("probe forced to %x by kernel command line\n",
synth_port);
-   if (synth_request_region(synth_port-1, SYNTH_IO_EXTENT)) {
+   if (synth_request_region(synth_port - 1, SYNTH_IO_EXTENT)) {
pr_warn("sorry, port already reserved\n");
return -EBUSY;
}
@@ -295,7 +295,7 @@ static int synth_probe(struct spk_synth *synth)
return -ENODEV;
}
pr_info("%s: %03x-%03x, driver version %s,\n", synth->long_name,
-   synth_port, synth_port+SYNTH_IO_EXTENT-1,
+   synth_port, synth_port + SYNTH_IO_EXTENT - 1,
synth->version);
synth->alive = 1;
return 0;
-- 
2.1.4

Re: Source address fib invalidation on IPv6

[Jason A. Donenfeld]

Hi David,

On Fri, Nov 11, 2016 at 11:14 PM, David Ahern  wrote:
> What do you mean by 'valid dst'? ipv6 returns net->ipv6.ip6_null_entry on 
> lookup failures so yes dst is non-NULL but that does not mean the lookup 
> succeeded.

What I mean is that it returns an ordinary dst, as if that souce
address _hadn't_ been removed from the interface, even though I just
removed it. Is this buggy behavior? If so, let me know and I'll try to
track it down. The expected behavior, as far as I can see, would be
the same that ip_route_output_flow has -- returning -EINVAL when the
saddr isn't valid. At the moment, when the saddr is invalid,
ipv6_stub->ipv6_dst_lookup returns 0 and  contains a real entry.

Regards,
Jason

Re: [PATCH v3] PM / devfreq: Restart previous governor if new governor fails to start

[Chanwoo Choi]

2016-11-12 11:17 GMT+09:00 Chanwoo Choi :
> 2016-11-12 8:25 GMT+09:00 Saravana Kannan :
>> On 11/09/2016 04:10 PM, Chanwoo Choi wrote:
>>>
>>> Hi,
>>>
>>> On 2016년 11월 10일 05:34, Saravana Kannan wrote:

 On 11/08/2016 06:38 PM, Chanwoo Choi wrote:
>
> On 2016년 11월 09일 11:36, Chanwoo Choi wrote:
>>
>> Hi,
>>
>> On 2016년 11월 09일 10:33, Chanwoo Choi wrote:
>>>
>>> On 2016년 11월 09일 05:52, Saravana Kannan wrote:

 On 11/08/2016 01:02 AM, Chanwoo Choi wrote:
>
> Hi,
>
> On 2016년 11월 08일 03:57, Saravana Kannan wrote:
>>
>> On 10/26/2016 05:06 PM, Chanwoo Choi wrote:
>>>
>>> Hi,
>>>
>>> On 2016년 10월 27일 04:17, Saravana Kannan wrote:

 If the new governor fails to start, switch back to old governor
 so that the
 devfreq state is not left in some weird limbo.

 Signed-off-by: Saravana Kannan 
 ---
 * Fix NULL deref for real this time.
 * Addressed some style preferences.

  drivers/devfreq/devfreq.c | 13 +++--
  1 file changed, 11 insertions(+), 2 deletions(-)

 diff --git a/drivers/devfreq/devfreq.c
 b/drivers/devfreq/devfreq.c
 index bf3ea76..77c41a5 100644
 --- a/drivers/devfreq/devfreq.c
 +++ b/drivers/devfreq/devfreq.c
 @@ -919,7 +919,7 @@ static ssize_t governor_store(struct device
 *dev, struct device_attribute *attr,
  struct devfreq *df = to_devfreq(dev);
  int ret;
  char str_governor[DEVFREQ_NAME_LEN + 1];
 -struct devfreq_governor *governor;
 +const struct devfreq_governor *governor, *prev_governor;

  ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s",
 str_governor);
  if (ret != 1)
 @@ -944,12 +944,21 @@ static ssize_t governor_store(struct device
 *dev, struct device_attribute *attr,
  goto out;
  }
  }
 +prev_governor = df->governor;
  df->governor = governor;
  strncpy(df->governor_name, governor->name,
 DEVFREQ_NAME_LEN);
  ret = df->governor->event_handler(df, DEVFREQ_GOV_START,
 NULL);
 -if (ret)
 +if (ret) {
  dev_warn(dev, "%s: Governor %s not started(%d)\n",
   __func__, df->governor->name, ret);
 +if (prev_governor) {
>>>
>>>
>>> I think that prev_governor is always set. You don't need to check
>>> it.
>>> Why do check it?
>>
>>
>> Not true. Even higher up in this function, we check if df->governor
>> != NULL. Simple example would be that the default governor passed in 
>> while
>> adding the device isn't compiled in.
>
>
> I don't understand. If device is not registered by
> devfreq_add_device(),
> you don't change the governor by using governor_store().
>
> If you can change the governor through governor_store(),
> it means that the devfreq instance was added without any problem.
> The added devfreq instance must have the sepecific governor
> on df->governor variable.
>
> So, I think that df->governor is always set and then prev_governor
> is always set.


 Read the code more closely. add_device doesn't (and shouldn't) fail
 if the governor isn't present. After that, the governor could be 
 changed
 from user space.
>>>
>>>
>>> If governor is not present during devfreq_add_device(),
>>> the devfreq instance is not able to register the devfreq framework.
>>> So, the user never touch the sysfs[1] to change the governor
>>> because the sysfs[1] is not created by devfreq framework.
>>> [1] /sys/class/devfreq/[device name]/governor
>>>
>>> In summary, if governor is not present during devfreq_add_device(),
>>> the devfreq framework doesn't create tye sysfs[1] for governor.
>>>
>>> The user-space never change the governor through sysfs[1]
>>> because there is no any sysfs entry[1].
>>
>>
>> I checked the code of devfreq_add_device().
>> As you mentioned, if there is no governor,
>> devfreq_add_device() don't pass wihtout calling the
>> devfreq->governor->even_handler().
>
>
> Wrong expression / don't pass -> pass
>

 I think it's correct as is. Just because 

Re: Source address fib invalidation on IPv6

[Jason A. Donenfeld]

Hi David,

On Fri, Nov 11, 2016 at 11:14 PM, David Ahern  wrote:
> What do you mean by 'valid dst'? ipv6 returns net->ipv6.ip6_null_entry on 
> lookup failures so yes dst is non-NULL but that does not mean the lookup 
> succeeded.

What I mean is that it returns an ordinary dst, as if that souce
address _hadn't_ been removed from the interface, even though I just
removed it. Is this buggy behavior? If so, let me know and I'll try to
track it down. The expected behavior, as far as I can see, would be
the same that ip_route_output_flow has -- returning -EINVAL when the
saddr isn't valid. At the moment, when the saddr is invalid,
ipv6_stub->ipv6_dst_lookup returns 0 and  contains a real entry.

Regards,
Jason

Re: [PATCH v3] PM / devfreq: Restart previous governor if new governor fails to start

[Chanwoo Choi]

2016-11-12 11:17 GMT+09:00 Chanwoo Choi :
> 2016-11-12 8:25 GMT+09:00 Saravana Kannan :
>> On 11/09/2016 04:10 PM, Chanwoo Choi wrote:
>>>
>>> Hi,
>>>
>>> On 2016년 11월 10일 05:34, Saravana Kannan wrote:

 On 11/08/2016 06:38 PM, Chanwoo Choi wrote:
>
> On 2016년 11월 09일 11:36, Chanwoo Choi wrote:
>>
>> Hi,
>>
>> On 2016년 11월 09일 10:33, Chanwoo Choi wrote:
>>>
>>> On 2016년 11월 09일 05:52, Saravana Kannan wrote:

 On 11/08/2016 01:02 AM, Chanwoo Choi wrote:
>
> Hi,
>
> On 2016년 11월 08일 03:57, Saravana Kannan wrote:
>>
>> On 10/26/2016 05:06 PM, Chanwoo Choi wrote:
>>>
>>> Hi,
>>>
>>> On 2016년 10월 27일 04:17, Saravana Kannan wrote:

 If the new governor fails to start, switch back to old governor
 so that the
 devfreq state is not left in some weird limbo.

 Signed-off-by: Saravana Kannan 
 ---
 * Fix NULL deref for real this time.
 * Addressed some style preferences.

  drivers/devfreq/devfreq.c | 13 +++--
  1 file changed, 11 insertions(+), 2 deletions(-)

 diff --git a/drivers/devfreq/devfreq.c
 b/drivers/devfreq/devfreq.c
 index bf3ea76..77c41a5 100644
 --- a/drivers/devfreq/devfreq.c
 +++ b/drivers/devfreq/devfreq.c
 @@ -919,7 +919,7 @@ static ssize_t governor_store(struct device
 *dev, struct device_attribute *attr,
  struct devfreq *df = to_devfreq(dev);
  int ret;
  char str_governor[DEVFREQ_NAME_LEN + 1];
 -struct devfreq_governor *governor;
 +const struct devfreq_governor *governor, *prev_governor;

  ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s",
 str_governor);
  if (ret != 1)
 @@ -944,12 +944,21 @@ static ssize_t governor_store(struct device
 *dev, struct device_attribute *attr,
  goto out;
  }
  }
 +prev_governor = df->governor;
  df->governor = governor;
  strncpy(df->governor_name, governor->name,
 DEVFREQ_NAME_LEN);
  ret = df->governor->event_handler(df, DEVFREQ_GOV_START,
 NULL);
 -if (ret)
 +if (ret) {
  dev_warn(dev, "%s: Governor %s not started(%d)\n",
   __func__, df->governor->name, ret);
 +if (prev_governor) {
>>>
>>>
>>> I think that prev_governor is always set. You don't need to check
>>> it.
>>> Why do check it?
>>
>>
>> Not true. Even higher up in this function, we check if df->governor
>> != NULL. Simple example would be that the default governor passed in 
>> while
>> adding the device isn't compiled in.
>
>
> I don't understand. If device is not registered by
> devfreq_add_device(),
> you don't change the governor by using governor_store().
>
> If you can change the governor through governor_store(),
> it means that the devfreq instance was added without any problem.
> The added devfreq instance must have the sepecific governor
> on df->governor variable.
>
> So, I think that df->governor is always set and then prev_governor
> is always set.


 Read the code more closely. add_device doesn't (and shouldn't) fail
 if the governor isn't present. After that, the governor could be 
 changed
 from user space.
>>>
>>>
>>> If governor is not present during devfreq_add_device(),
>>> the devfreq instance is not able to register the devfreq framework.
>>> So, the user never touch the sysfs[1] to change the governor
>>> because the sysfs[1] is not created by devfreq framework.
>>> [1] /sys/class/devfreq/[device name]/governor
>>>
>>> In summary, if governor is not present during devfreq_add_device(),
>>> the devfreq framework doesn't create tye sysfs[1] for governor.
>>>
>>> The user-space never change the governor through sysfs[1]
>>> because there is no any sysfs entry[1].
>>
>>
>> I checked the code of devfreq_add_device().
>> As you mentioned, if there is no governor,
>> devfreq_add_device() don't pass wihtout calling the
>> devfreq->governor->even_handler().
>
>
> Wrong expression / don't pass -> pass
>

 I think it's correct as is. Just because the governor isn't there (or
 hasn't registered yet) doesn't 

Re: [PATCH v3] PM / devfreq: Restart previous governor if new governor fails to start

[Chanwoo Choi]

2016-11-12 8:25 GMT+09:00 Saravana Kannan :
> On 11/09/2016 04:10 PM, Chanwoo Choi wrote:
>>
>> Hi,
>>
>> On 2016년 11월 10일 05:34, Saravana Kannan wrote:
>>>
>>> On 11/08/2016 06:38 PM, Chanwoo Choi wrote:

 On 2016년 11월 09일 11:36, Chanwoo Choi wrote:
>
> Hi,
>
> On 2016년 11월 09일 10:33, Chanwoo Choi wrote:
>>
>> On 2016년 11월 09일 05:52, Saravana Kannan wrote:
>>>
>>> On 11/08/2016 01:02 AM, Chanwoo Choi wrote:

 Hi,

 On 2016년 11월 08일 03:57, Saravana Kannan wrote:
>
> On 10/26/2016 05:06 PM, Chanwoo Choi wrote:
>>
>> Hi,
>>
>> On 2016년 10월 27일 04:17, Saravana Kannan wrote:
>>>
>>> If the new governor fails to start, switch back to old governor
>>> so that the
>>> devfreq state is not left in some weird limbo.
>>>
>>> Signed-off-by: Saravana Kannan 
>>> ---
>>> * Fix NULL deref for real this time.
>>> * Addressed some style preferences.
>>>
>>>  drivers/devfreq/devfreq.c | 13 +++--
>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/devfreq.c
>>> b/drivers/devfreq/devfreq.c
>>> index bf3ea76..77c41a5 100644
>>> --- a/drivers/devfreq/devfreq.c
>>> +++ b/drivers/devfreq/devfreq.c
>>> @@ -919,7 +919,7 @@ static ssize_t governor_store(struct device
>>> *dev, struct device_attribute *attr,
>>>  struct devfreq *df = to_devfreq(dev);
>>>  int ret;
>>>  char str_governor[DEVFREQ_NAME_LEN + 1];
>>> -struct devfreq_governor *governor;
>>> +const struct devfreq_governor *governor, *prev_governor;
>>>
>>>  ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s",
>>> str_governor);
>>>  if (ret != 1)
>>> @@ -944,12 +944,21 @@ static ssize_t governor_store(struct device
>>> *dev, struct device_attribute *attr,
>>>  goto out;
>>>  }
>>>  }
>>> +prev_governor = df->governor;
>>>  df->governor = governor;
>>>  strncpy(df->governor_name, governor->name,
>>> DEVFREQ_NAME_LEN);
>>>  ret = df->governor->event_handler(df, DEVFREQ_GOV_START,
>>> NULL);
>>> -if (ret)
>>> +if (ret) {
>>>  dev_warn(dev, "%s: Governor %s not started(%d)\n",
>>>   __func__, df->governor->name, ret);
>>> +if (prev_governor) {
>>
>>
>> I think that prev_governor is always set. You don't need to check
>> it.
>> Why do check it?
>
>
> Not true. Even higher up in this function, we check if df->governor
> != NULL. Simple example would be that the default governor passed in 
> while
> adding the device isn't compiled in.


 I don't understand. If device is not registered by
 devfreq_add_device(),
 you don't change the governor by using governor_store().

 If you can change the governor through governor_store(),
 it means that the devfreq instance was added without any problem.
 The added devfreq instance must have the sepecific governor
 on df->governor variable.

 So, I think that df->governor is always set and then prev_governor
 is always set.
>>>
>>>
>>> Read the code more closely. add_device doesn't (and shouldn't) fail
>>> if the governor isn't present. After that, the governor could be changed
>>> from user space.
>>
>>
>> If governor is not present during devfreq_add_device(),
>> the devfreq instance is not able to register the devfreq framework.
>> So, the user never touch the sysfs[1] to change the governor
>> because the sysfs[1] is not created by devfreq framework.
>> [1] /sys/class/devfreq/[device name]/governor
>>
>> In summary, if governor is not present during devfreq_add_device(),
>> the devfreq framework doesn't create tye sysfs[1] for governor.
>>
>> The user-space never change the governor through sysfs[1]
>> because there is no any sysfs entry[1].
>
>
> I checked the code of devfreq_add_device().
> As you mentioned, if there is no governor,
> devfreq_add_device() don't pass wihtout calling the
> devfreq->governor->even_handler().


 Wrong expression / don't pass -> pass

>>>
>>> I think it's correct as is. Just because the governor isn't there (or
>>> hasn't registered yet) doesn't mean the device add should fail.
>>>
>>> That aside, do you care to ack this patch for the way the code is
>>> currently?
>>
>>
>> 

Re: [PATCH v3] PM / devfreq: Restart previous governor if new governor fails to start

[Chanwoo Choi]

2016-11-12 8:25 GMT+09:00 Saravana Kannan :
> On 11/09/2016 04:10 PM, Chanwoo Choi wrote:
>>
>> Hi,
>>
>> On 2016년 11월 10일 05:34, Saravana Kannan wrote:
>>>
>>> On 11/08/2016 06:38 PM, Chanwoo Choi wrote:

 On 2016년 11월 09일 11:36, Chanwoo Choi wrote:
>
> Hi,
>
> On 2016년 11월 09일 10:33, Chanwoo Choi wrote:
>>
>> On 2016년 11월 09일 05:52, Saravana Kannan wrote:
>>>
>>> On 11/08/2016 01:02 AM, Chanwoo Choi wrote:

 Hi,

 On 2016년 11월 08일 03:57, Saravana Kannan wrote:
>
> On 10/26/2016 05:06 PM, Chanwoo Choi wrote:
>>
>> Hi,
>>
>> On 2016년 10월 27일 04:17, Saravana Kannan wrote:
>>>
>>> If the new governor fails to start, switch back to old governor
>>> so that the
>>> devfreq state is not left in some weird limbo.
>>>
>>> Signed-off-by: Saravana Kannan 
>>> ---
>>> * Fix NULL deref for real this time.
>>> * Addressed some style preferences.
>>>
>>>  drivers/devfreq/devfreq.c | 13 +++--
>>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/devfreq.c
>>> b/drivers/devfreq/devfreq.c
>>> index bf3ea76..77c41a5 100644
>>> --- a/drivers/devfreq/devfreq.c
>>> +++ b/drivers/devfreq/devfreq.c
>>> @@ -919,7 +919,7 @@ static ssize_t governor_store(struct device
>>> *dev, struct device_attribute *attr,
>>>  struct devfreq *df = to_devfreq(dev);
>>>  int ret;
>>>  char str_governor[DEVFREQ_NAME_LEN + 1];
>>> -struct devfreq_governor *governor;
>>> +const struct devfreq_governor *governor, *prev_governor;
>>>
>>>  ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s",
>>> str_governor);
>>>  if (ret != 1)
>>> @@ -944,12 +944,21 @@ static ssize_t governor_store(struct device
>>> *dev, struct device_attribute *attr,
>>>  goto out;
>>>  }
>>>  }
>>> +prev_governor = df->governor;
>>>  df->governor = governor;
>>>  strncpy(df->governor_name, governor->name,
>>> DEVFREQ_NAME_LEN);
>>>  ret = df->governor->event_handler(df, DEVFREQ_GOV_START,
>>> NULL);
>>> -if (ret)
>>> +if (ret) {
>>>  dev_warn(dev, "%s: Governor %s not started(%d)\n",
>>>   __func__, df->governor->name, ret);
>>> +if (prev_governor) {
>>
>>
>> I think that prev_governor is always set. You don't need to check
>> it.
>> Why do check it?
>
>
> Not true. Even higher up in this function, we check if df->governor
> != NULL. Simple example would be that the default governor passed in 
> while
> adding the device isn't compiled in.


 I don't understand. If device is not registered by
 devfreq_add_device(),
 you don't change the governor by using governor_store().

 If you can change the governor through governor_store(),
 it means that the devfreq instance was added without any problem.
 The added devfreq instance must have the sepecific governor
 on df->governor variable.

 So, I think that df->governor is always set and then prev_governor
 is always set.
>>>
>>>
>>> Read the code more closely. add_device doesn't (and shouldn't) fail
>>> if the governor isn't present. After that, the governor could be changed
>>> from user space.
>>
>>
>> If governor is not present during devfreq_add_device(),
>> the devfreq instance is not able to register the devfreq framework.
>> So, the user never touch the sysfs[1] to change the governor
>> because the sysfs[1] is not created by devfreq framework.
>> [1] /sys/class/devfreq/[device name]/governor
>>
>> In summary, if governor is not present during devfreq_add_device(),
>> the devfreq framework doesn't create tye sysfs[1] for governor.
>>
>> The user-space never change the governor through sysfs[1]
>> because there is no any sysfs entry[1].
>
>
> I checked the code of devfreq_add_device().
> As you mentioned, if there is no governor,
> devfreq_add_device() don't pass wihtout calling the
> devfreq->governor->even_handler().


 Wrong expression / don't pass -> pass

>>>
>>> I think it's correct as is. Just because the governor isn't there (or
>>> hasn't registered yet) doesn't mean the device add should fail.
>>>
>>> That aside, do you care to ack this patch for the way the code is
>>> currently?
>>
>>
>> Reviewed-by: Chanwoo Choi 
>
>
> Thanks
>
>> After 

[PATCH] [STYLE 1/3]staging:speakup:speakup_keypc.c Block text modification

[Walt Feasel]

Modified multi line comment to single
Removed blankline at end

Signed-off-by: Walt Feasel 
---
 drivers/staging/speakup/speakup_keypc.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/speakup/speakup_keypc.c 
b/drivers/staging/speakup/speakup_keypc.c
index 5e2170b..cadbe8d 100644
--- a/drivers/staging/speakup/speakup_keypc.c
+++ b/drivers/staging/speakup/speakup_keypc.c
@@ -51,9 +51,8 @@ static struct var_t vars[] = {
V_LAST_VAR
 };
 
-/*
- * These attributes will appear in /sys/accessibility/speakup/keypc.
- */
+/* These attributes will appear in /sys/accessibility/speakup/keypc. */
+
 static struct kobj_attribute caps_start_attribute =
__ATTR(caps_start, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute caps_stop_attribute =
@@ -321,4 +320,3 @@ MODULE_AUTHOR("David Borowski");
 MODULE_DESCRIPTION("Speakup support for Keynote Gold PC synthesizers");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
-
-- 
2.1.4

[PATCH] [STYLE 1/3]staging:speakup:speakup_keypc.c Block text modification

[Walt Feasel]

Modified multi line comment to single
Removed blankline at end

Signed-off-by: Walt Feasel 
---
 drivers/staging/speakup/speakup_keypc.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/speakup/speakup_keypc.c 
b/drivers/staging/speakup/speakup_keypc.c
index 5e2170b..cadbe8d 100644
--- a/drivers/staging/speakup/speakup_keypc.c
+++ b/drivers/staging/speakup/speakup_keypc.c
@@ -51,9 +51,8 @@ static struct var_t vars[] = {
V_LAST_VAR
 };
 
-/*
- * These attributes will appear in /sys/accessibility/speakup/keypc.
- */
+/* These attributes will appear in /sys/accessibility/speakup/keypc. */
+
 static struct kobj_attribute caps_start_attribute =
__ATTR(caps_start, S_IWUSR|S_IRUGO, spk_var_show, spk_var_store);
 static struct kobj_attribute caps_stop_attribute =
@@ -321,4 +320,3 @@ MODULE_AUTHOR("David Borowski");
 MODULE_DESCRIPTION("Speakup support for Keynote Gold PC synthesizers");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
-
-- 
2.1.4

[PATCH] [STYLE]staging:speakup:speakup.h Space preferred around

[Walt Feasel]

Made suggested modifications from checkpatch in reference
to CHECK: spaces preferred around that '|,+'

Signed-off-by: Walt Feasel 
---
 drivers/staging/speakup/speakup.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/speakup/speakup.h 
b/drivers/staging/speakup/speakup.h
index df74c91..9219b4a 100644
--- a/drivers/staging/speakup/speakup.h
+++ b/drivers/staging/speakup/speakup.h
@@ -10,8 +10,8 @@
 #define MAX_DESC_LEN 72
 
 /* proc permissions */
-#define USER_R (S_IFREG|S_IRUGO)
-#define USER_W (S_IFREG|S_IWUGO)
+#define USER_R (S_IFREG | S_IRUGO)
+#define USER_W (S_IFREG | S_IWUGO)
 
 #define TOGGLE_0 .u.n = {NULL, 0, 0, 1, 0, 0, NULL }
 #define TOGGLE_1 .u.n = {NULL, 1, 0, 1, 0, 0, NULL }
@@ -24,7 +24,7 @@
 #define A_CAP 0x0007
 #define B_NUM 0x0008
 #define NUM 0x0009
-#define ALPHANUM (B_ALPHA|B_NUM)
+#define ALPHANUM (B_ALPHA | B_NUM)
 #define SOME 0x0010
 #define MOST 0x0020
 #define PUNC 0x0040
@@ -34,9 +34,9 @@
 #define B_EXNUM 0x0100
 #define CH_RPT 0x0200
 #define B_CTL 0x0400
-#define A_CTL (B_CTL+SYNTH_OK)
+#define A_CTL (B_CTL + SYNTH_OK)
 #define B_SYM 0x0800
-#define B_CAPSYM (B_CAP|B_SYM)
+#define B_CAPSYM (B_CAP | B_SYM)
 
 #define IS_WDLM(x) (spk_chartab[((u_char)x)]_WDLM)
 #define IS_CHAR(x, type) (spk_chartab[((u_char)x)])
-- 
2.1.4

[PATCH] [STYLE]staging:speakup:speakup.h Space preferred around

[Walt Feasel]

Made suggested modifications from checkpatch in reference
to CHECK: spaces preferred around that '|,+'

Signed-off-by: Walt Feasel 
---
 drivers/staging/speakup/speakup.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/speakup/speakup.h 
b/drivers/staging/speakup/speakup.h
index df74c91..9219b4a 100644
--- a/drivers/staging/speakup/speakup.h
+++ b/drivers/staging/speakup/speakup.h
@@ -10,8 +10,8 @@
 #define MAX_DESC_LEN 72
 
 /* proc permissions */
-#define USER_R (S_IFREG|S_IRUGO)
-#define USER_W (S_IFREG|S_IWUGO)
+#define USER_R (S_IFREG | S_IRUGO)
+#define USER_W (S_IFREG | S_IWUGO)
 
 #define TOGGLE_0 .u.n = {NULL, 0, 0, 1, 0, 0, NULL }
 #define TOGGLE_1 .u.n = {NULL, 1, 0, 1, 0, 0, NULL }
@@ -24,7 +24,7 @@
 #define A_CAP 0x0007
 #define B_NUM 0x0008
 #define NUM 0x0009
-#define ALPHANUM (B_ALPHA|B_NUM)
+#define ALPHANUM (B_ALPHA | B_NUM)
 #define SOME 0x0010
 #define MOST 0x0020
 #define PUNC 0x0040
@@ -34,9 +34,9 @@
 #define B_EXNUM 0x0100
 #define CH_RPT 0x0200
 #define B_CTL 0x0400
-#define A_CTL (B_CTL+SYNTH_OK)
+#define A_CTL (B_CTL + SYNTH_OK)
 #define B_SYM 0x0800
-#define B_CAPSYM (B_CAP|B_SYM)
+#define B_CAPSYM (B_CAP | B_SYM)
 
 #define IS_WDLM(x) (spk_chartab[((u_char)x)]_WDLM)
 #define IS_CHAR(x, type) (spk_chartab[((u_char)x)])
-- 
2.1.4

Re: [PATCH] PCI: rockchip: correct the use of FTS mask

[Shawn Lin]

On 2016/11/12 6:30, Bjorn Helgaas wrote:

On Tue, Oct 18, 2016 at 04:13:04PM -0700, Brian Norris wrote:

We're trying to mask out bits[23:8] while retaining [32:24, 7:0], but
we're doing the inverse. That doesn't have too much effect, since we're
setting all the [23:8] bits to 1, and the other bits are only relevant
for modes we're currently not using. But we should get this right.

Fixes: ca1989084054 ("PCI: rockchip: Fix wrong transmitted FTS count")
Signed-off-by: Brian Norris 


I assume this is correct, but I'm waiting for an ack from Shawn.


Thanks for catching this, and it was missing from my inbox somehow...

Acked-by: Shawn Lin 




---
 drivers/pci/host/pcie-rockchip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index e0b22dab9b7a..5c2e3297a3ff 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -492,7 +492,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie 
*rockchip)

/* Fix the transmitted FTS count desired to exit from L0s. */
status = rockchip_pcie_read(rockchip, PCIE_CORE_CTRL_PLC1);
-   status = (status & PCIE_CORE_CTRL_PLC1_FTS_MASK) |
+   status = (status & ~PCIE_CORE_CTRL_PLC1_FTS_MASK) |
 (PCIE_CORE_CTRL_PLC1_FTS_CNT << PCIE_CORE_CTRL_PLC1_FTS_SHIFT);
rockchip_pcie_write(rockchip, status, PCIE_CORE_CTRL_PLC1);

--
2.8.0.rc3.226.g39d4020


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html




--
Best Regards
Shawn Lin

Re: [PATCH] PCI: rockchip: correct the use of FTS mask

[Shawn Lin]

On 2016/11/12 6:30, Bjorn Helgaas wrote:

On Tue, Oct 18, 2016 at 04:13:04PM -0700, Brian Norris wrote:

We're trying to mask out bits[23:8] while retaining [32:24, 7:0], but
we're doing the inverse. That doesn't have too much effect, since we're
setting all the [23:8] bits to 1, and the other bits are only relevant
for modes we're currently not using. But we should get this right.

Fixes: ca1989084054 ("PCI: rockchip: Fix wrong transmitted FTS count")
Signed-off-by: Brian Norris 


I assume this is correct, but I'm waiting for an ack from Shawn.


Thanks for catching this, and it was missing from my inbox somehow...

Acked-by: Shawn Lin 




---
 drivers/pci/host/pcie-rockchip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index e0b22dab9b7a..5c2e3297a3ff 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -492,7 +492,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie 
*rockchip)

/* Fix the transmitted FTS count desired to exit from L0s. */
status = rockchip_pcie_read(rockchip, PCIE_CORE_CTRL_PLC1);
-   status = (status & PCIE_CORE_CTRL_PLC1_FTS_MASK) |
+   status = (status & ~PCIE_CORE_CTRL_PLC1_FTS_MASK) |
 (PCIE_CORE_CTRL_PLC1_FTS_CNT << PCIE_CORE_CTRL_PLC1_FTS_SHIFT);
rockchip_pcie_write(rockchip, status, PCIE_CORE_CTRL_PLC1);

--
2.8.0.rc3.226.g39d4020


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html




--
Best Regards
Shawn Lin

Re: [PATCH v6 05/07] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops

[Laurent Pinchart]

Hi Robin,

On Friday 11 Nov 2016 14:44:29 Robin Murphy wrote:
> On 11/11/16 01:50, Laurent Pinchart wrote:
> > On Friday 21 Oct 2016 18:52:53 Robin Murphy wrote:
> >> On 20/10/16 00:36, Magnus Damm wrote:
> >>> From: Magnus Damm 
> >>> 
> >>> Introduce an alternative set of iommu_ops suitable for 64-bit ARM
> >>> as well as 32-bit ARM when CONFIG_IOMMU_DMA=y. Also adjust the
> >>> Kconfig to depend on ARM or IOMMU_DMA.
> >>> 
> >>> Signed-off-by: Magnus Damm 
> >>> ---
> >>> 
> >>>  Changes since V5:
> >>>  - Made domain allocation/free code more consistent - thanks Joerg!
> >>>  
> >>>  Changes since V4:
> >>>  - Added Kconfig hunk to depend on ARM or IOMMU_DMA
> >>>  
> >>>  Changes since V3:
> >>>  - Removed group parameter from ipmmu_init_platform_device()
> >>>  
> >>>  Changes since V2:
> >>>  
> >>>  - Included this new patch from the following series:
> >>>[PATCH 00/04] iommu/ipmmu-vmsa: IPMMU CONFIG_IOMMU_DMA update
> >>>  
> >>>  - Use only a single iommu_ops structure with #ifdef CONFIG_IOMMU_DMA
> >>>  - Folded in #ifdefs to handle CONFIG_ARM and CONFIG_IOMMU_DMA
> >>>  - of_xlate() is now used without #ifdefs
> >>>  - Made sure code compiles on both 32-bit and 64-bit ARM.
> >>>  
> >>>  drivers/iommu/Kconfig  |1
> >>>  drivers/iommu/ipmmu-vmsa.c |  122 +---
> >>>  2 files changed, 115 insertions(+), 8 deletions(-)
> > 
> > [snip]
> > 
> >>> --- 0006/drivers/iommu/ipmmu-vmsa.c
> >>> +++ work/drivers/iommu/ipmmu-vmsa.c
> >>> 2016-10-20 08:16:48.440607110 +0900
> > 
> > [snip]
> > 
> >>> -static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
> >>> -{
> >>> - if (type != IOMMU_DOMAIN_UNMANAGED)
> >>> - return NULL;
> >> 
> >> I *think* that if we did the initial check thus:
> >>if (type != IOMMU_DOMAIN_UNMANAGED ||
> >>(IS_ENABLED(CONFIG_IOMMU_DMA) && type != IOMMU_DOMAIN_DMA))
> >>return NULL;
> > 
> > I assume you meant
> > 
> > if (type != IOMMU_DOMAIN_UNMANAGED &&
> > (!IS_ENABLED(CONFIG_IOMMU_DMA) || type != IOMMU_DOMAIN_DMA))
> > return NULL;
> > 
> > But how about just
> > 
> > if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA))
> > return NULL;
> > 
> > as type will never be set to IOMMU_DOMAIN_DMA on ARM32 ?
> 
> Actually it can be, but *only* at this point, because
> iommu_group_get_for_dev() will always attempt to allocate a default
> domain.

If I'm not mistaken iommu_group_get_for_dev() is the only function that tries 
to create an IOMMU_DOMAIN_DMA domain, and is only called in core code by 
iommu_request_dm_for_dev(). That function isn't called by the IPMMU driver, 
and with Magnus' patches the IPMMU driver calls iommu_group_get_for_dev() on 
ARM64 platforms only (in ipmmu_add_device_dma(), only compiled in when 
CONFIG_IOMMU_DMA is set, which only happens on ARM64).

> Having the additional check up-front just saves going through
> the whole IOVA domain allocation only to tear it all down again when
> get_cookie() returns -ENODEV. You're right that it's not strictly
> necessary (and that I got my DeMorganning wrong), though.
> 
> Robin.
> 
> >> it shouldn't be necessary to split the function at all - we then just
> >> wrap the {get,put}_cookie() bits in "if (type ==  IOMMU_DOMAIN_DMA)" and
> >> in the 32-bit ARM case they just don't run as that can never be true.
> >> 
> >>> -
> >>> - return __ipmmu_domain_alloc(type);
> >>> -}
> >>> -

-- 
Regards,

Laurent Pinchart

Re: [PATCH v6 05/07] iommu/ipmmu-vmsa: Add new IOMMU_DOMAIN_DMA ops

[Laurent Pinchart]

Hi Robin,

On Friday 11 Nov 2016 14:44:29 Robin Murphy wrote:
> On 11/11/16 01:50, Laurent Pinchart wrote:
> > On Friday 21 Oct 2016 18:52:53 Robin Murphy wrote:
> >> On 20/10/16 00:36, Magnus Damm wrote:
> >>> From: Magnus Damm 
> >>> 
> >>> Introduce an alternative set of iommu_ops suitable for 64-bit ARM
> >>> as well as 32-bit ARM when CONFIG_IOMMU_DMA=y. Also adjust the
> >>> Kconfig to depend on ARM or IOMMU_DMA.
> >>> 
> >>> Signed-off-by: Magnus Damm 
> >>> ---
> >>> 
> >>>  Changes since V5:
> >>>  - Made domain allocation/free code more consistent - thanks Joerg!
> >>>  
> >>>  Changes since V4:
> >>>  - Added Kconfig hunk to depend on ARM or IOMMU_DMA
> >>>  
> >>>  Changes since V3:
> >>>  - Removed group parameter from ipmmu_init_platform_device()
> >>>  
> >>>  Changes since V2:
> >>>  
> >>>  - Included this new patch from the following series:
> >>>[PATCH 00/04] iommu/ipmmu-vmsa: IPMMU CONFIG_IOMMU_DMA update
> >>>  
> >>>  - Use only a single iommu_ops structure with #ifdef CONFIG_IOMMU_DMA
> >>>  - Folded in #ifdefs to handle CONFIG_ARM and CONFIG_IOMMU_DMA
> >>>  - of_xlate() is now used without #ifdefs
> >>>  - Made sure code compiles on both 32-bit and 64-bit ARM.
> >>>  
> >>>  drivers/iommu/Kconfig  |1
> >>>  drivers/iommu/ipmmu-vmsa.c |  122 +---
> >>>  2 files changed, 115 insertions(+), 8 deletions(-)
> > 
> > [snip]
> > 
> >>> --- 0006/drivers/iommu/ipmmu-vmsa.c
> >>> +++ work/drivers/iommu/ipmmu-vmsa.c
> >>> 2016-10-20 08:16:48.440607110 +0900
> > 
> > [snip]
> > 
> >>> -static struct iommu_domain *ipmmu_domain_alloc(unsigned type)
> >>> -{
> >>> - if (type != IOMMU_DOMAIN_UNMANAGED)
> >>> - return NULL;
> >> 
> >> I *think* that if we did the initial check thus:
> >>if (type != IOMMU_DOMAIN_UNMANAGED ||
> >>(IS_ENABLED(CONFIG_IOMMU_DMA) && type != IOMMU_DOMAIN_DMA))
> >>return NULL;
> > 
> > I assume you meant
> > 
> > if (type != IOMMU_DOMAIN_UNMANAGED &&
> > (!IS_ENABLED(CONFIG_IOMMU_DMA) || type != IOMMU_DOMAIN_DMA))
> > return NULL;
> > 
> > But how about just
> > 
> > if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA))
> > return NULL;
> > 
> > as type will never be set to IOMMU_DOMAIN_DMA on ARM32 ?
> 
> Actually it can be, but *only* at this point, because
> iommu_group_get_for_dev() will always attempt to allocate a default
> domain.

If I'm not mistaken iommu_group_get_for_dev() is the only function that tries 
to create an IOMMU_DOMAIN_DMA domain, and is only called in core code by 
iommu_request_dm_for_dev(). That function isn't called by the IPMMU driver, 
and with Magnus' patches the IPMMU driver calls iommu_group_get_for_dev() on 
ARM64 platforms only (in ipmmu_add_device_dma(), only compiled in when 
CONFIG_IOMMU_DMA is set, which only happens on ARM64).

> Having the additional check up-front just saves going through
> the whole IOVA domain allocation only to tear it all down again when
> get_cookie() returns -ENODEV. You're right that it's not strictly
> necessary (and that I got my DeMorganning wrong), though.
> 
> Robin.
> 
> >> it shouldn't be necessary to split the function at all - we then just
> >> wrap the {get,put}_cookie() bits in "if (type ==  IOMMU_DOMAIN_DMA)" and
> >> in the 32-bit ARM case they just don't run as that can never be true.
> >> 
> >>> -
> >>> - return __ipmmu_domain_alloc(type);
> >>> -}
> >>> -

-- 
Regards,

Laurent Pinchart

kvm: WARNING In kvm_apic_accept_events

[Dmitry Vyukov]

Hello,

The following program triggers WARNING in kvm_apic_accept_events:
https://gist.githubusercontent.com/dvyukov/95b845a2e637485568ea1ef181a72370/raw/d90717dd67128b21715c5e794568a1600f613d97/gistfile1.txt

On commit 015ed9433be2b476ec7e2e6a9a411a56e3b5b035 (Nov 11).


[ cut here ]
WARNING: CPU: 1 PID: 23523 at arch/x86/kvm/lapic.c:2330
kvm_apic_accept_events+0x3f6/0x460
Modules linked in:[   55.632648] CPU: 1 PID: 2812 Comm: a.out Not
tainted 4.9.0-rc4+ #41
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 88006be77978 81c2f79b  
 8322d6a0 81127426 88006be779c0 8123ffef
 811acc46 091a 8322d6a0 091a
Call Trace:
 [< inline >] __dump_stack lib/dump_stack.c:15
 [] dump_stack+0xb3/0x118 lib/dump_stack.c:51
 [] __warn+0x19f/0x1e0 kernel/panic.c:550
 [] warn_slowpath_null+0x2c/0x40 kernel/panic.c:585
 [] kvm_apic_accept_events+0x3f6/0x460
arch/x86/kvm/lapic.c:2330
 [< inline >] vcpu_block arch/x86/kvm/x86.c:6793
 [< inline >] vcpu_run arch/x86/kvm/x86.c:6828
 [] kvm_arch_vcpu_ioctl_run+0x1580/0x5a90
arch/x86/kvm/x86.c:6984
 [] kvm_vcpu_ioctl+0x61e/0xdd0
arch/x86/kvm/../../../virt/kvm/kvm_main.c:2557
 [< inline >] vfs_ioctl fs/ioctl.c:43
 [] do_vfs_ioctl+0x18c/0x1040 fs/ioctl.c:679
 [< inline >] SYSC_ioctl fs/ioctl.c:694
 [] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:685
 [] entry_SYSCALL_64_fastpath+0x1f/0xc2
arch/x86/entry/entry_64.S:209
---[ end trace b2415ddd1b26dce6 ]---

kvm: WARNING In kvm_apic_accept_events

[Dmitry Vyukov]

Hello,

The following program triggers WARNING in kvm_apic_accept_events:
https://gist.githubusercontent.com/dvyukov/95b845a2e637485568ea1ef181a72370/raw/d90717dd67128b21715c5e794568a1600f613d97/gistfile1.txt

On commit 015ed9433be2b476ec7e2e6a9a411a56e3b5b035 (Nov 11).


[ cut here ]
WARNING: CPU: 1 PID: 23523 at arch/x86/kvm/lapic.c:2330
kvm_apic_accept_events+0x3f6/0x460
Modules linked in:[   55.632648] CPU: 1 PID: 2812 Comm: a.out Not
tainted 4.9.0-rc4+ #41
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 88006be77978 81c2f79b  
 8322d6a0 81127426 88006be779c0 8123ffef
 811acc46 091a 8322d6a0 091a
Call Trace:
 [< inline >] __dump_stack lib/dump_stack.c:15
 [] dump_stack+0xb3/0x118 lib/dump_stack.c:51
 [] __warn+0x19f/0x1e0 kernel/panic.c:550
 [] warn_slowpath_null+0x2c/0x40 kernel/panic.c:585
 [] kvm_apic_accept_events+0x3f6/0x460
arch/x86/kvm/lapic.c:2330
 [< inline >] vcpu_block arch/x86/kvm/x86.c:6793
 [< inline >] vcpu_run arch/x86/kvm/x86.c:6828
 [] kvm_arch_vcpu_ioctl_run+0x1580/0x5a90
arch/x86/kvm/x86.c:6984
 [] kvm_vcpu_ioctl+0x61e/0xdd0
arch/x86/kvm/../../../virt/kvm/kvm_main.c:2557
 [< inline >] vfs_ioctl fs/ioctl.c:43
 [] do_vfs_ioctl+0x18c/0x1040 fs/ioctl.c:679
 [< inline >] SYSC_ioctl fs/ioctl.c:694
 [] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:685
 [] entry_SYSCALL_64_fastpath+0x1f/0xc2
arch/x86/entry/entry_64.S:209
---[ end trace b2415ddd1b26dce6 ]---