Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 3/8] PCI: Rename device node parameter of of_pci_get_host_bridge_resources()
From: Vladimir Zapolskiy @ 2018-05-15 10:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4df503604897aebbcb6e7ab98ef5f24916c6d382.1526375226.git.jan.kiszka@siemens.com>

On 05/15/2018 12:07 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> We will add a real device parameter to this function soon.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Tested-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Reviewed-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

--
With best wishes,
Vladimir

^ permalink raw reply

* [PATCH v4 4/8] PCI: Replace dev_node parameter of of_pci_get_host_bridge_resources with device
From: Vladimir Zapolskiy @ 2018-05-15 10:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5f2226585c6af9920b266d0503e32042d4c9e440.1526375226.git.jan.kiszka@siemens.com>

On 05/15/2018 12:07 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Another step towards a managed version of
> of_pci_get_host_bridge_resources(): Feed in the underlying device,
> rather than just the OF node. This will allow to use managed resource
> allocation internally later on.
> 
> CC: Jingoo Han <jingoohan1@gmail.com>
> CC: Joao Pinto <Joao.Pinto@synopsys.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Tested-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Reviewed-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

--
With best wishes,
Vladimir

^ permalink raw reply

* [PATCH v4 5/8] PCI: Replace pr_*() with dev_*() in of_pci_get_host_bridge_resources()
From: Vladimir Zapolskiy @ 2018-05-15 10:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <95f6ca417e2bc74d15f707e577b098e46b4b3ba4.1526375226.git.jan.kiszka@siemens.com>

On 05/15/2018 12:07 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Now that we have a device reference, make use of it for printing.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Tested-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Reviewed-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

--
With best wishes,
Vladimir

^ permalink raw reply

* [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()
From: Vladimir Zapolskiy @ 2018-05-15 10:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <3aeb2ed038cbce8fe744b614dc19d414555a7e8f.1526375226.git.jan.kiszka@siemens.com>

On 05/15/2018 12:07 PM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.
> 
> CC: Jingoo Han <jingoohan1@gmail.com>
> CC: Joao Pinto <Joao.Pinto@synopsys.com>
> CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Now it is correct, thanks.

Tested-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Reviewed-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

--
With best wishes,
Vladimir

^ permalink raw reply

* [PATCH 1/2] arm64: dts: berlin4ct: use SPDX-License-Identifier
From: Jisheng Zhang @ 2018-05-15 10:08 UTC (permalink / raw)
  To: linux-arm-kernel

Follow the recent trend for the license description, and also fix the
wrongly stated X11 to MIT.

As already pointed on the DT ML, the X11 license text [1] is explicitly
for the X Consortium and has a couple of extra clauses. The MIT
license text [2] is actually what the current DT files claim.

[1] https://spdx.org/licenses/X11.html
[2] https://spdx.org/licenses/MIT.html

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 arch/arm64/boot/dts/marvell/berlin4ct.dtsi | 39 +---------------------
 1 file changed, 1 insertion(+), 38 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/berlin4ct.dtsi b/arch/arm64/boot/dts/marvell/berlin4ct.dtsi
index d2f88b92d8e2..216767e2edf6 100644
--- a/arch/arm64/boot/dts/marvell/berlin4ct.dtsi
+++ b/arch/arm64/boot/dts/marvell/berlin4ct.dtsi
@@ -1,45 +1,8 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*
  * Copyright (C) 2015 Marvell Technology Group Ltd.
  *
  * Author: Jisheng Zhang <jszhang@marvell.com>
- *
- * This file is dual-licensed: you can use it either under the terms
- * of the GPLv2 or the X11 license, at your option. Note that this dual
- * licensing only applies to this file, and not this project as a
- * whole.
- *
- *  a) This library 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 library 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.
- *
- * Or, alternatively,
- *
- *  b) Permission is hereby granted, free of charge, to any person
- *     obtaining a copy of this software and associated documentation
- *     files (the "Software"), to deal in the Software without
- *     restriction, including without limitation the rights to use,
- *     copy, modify, merge, publish, distribute, sublicense, and/or
- *     sell copies of the Software, and to permit persons to whom the
- *     Software is furnished to do so, subject to the following
- *     conditions:
- *
- *     The above copyright notice and this permission notice shall be
- *     included in all copies or substantial portions of the Software.
- *
- *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
- *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
- *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
- *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
-- 
2.17.0

^ permalink raw reply related

* [PATCH 2/2] arm64: dts: berlin4ct-*.dts: use SPDX-License-Identifier
From: Jisheng Zhang @ 2018-05-15 10:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180515180826.2ab39f0c@xhacker.debian>

Follow the recent trend for the license description, and also fix the
wrongly stated X11 to MIT.

As already pointed on the DT ML, the X11 license text [1] is explicitly
for the X Consortium and has a couple of extra clauses. The MIT
license text [2] is actually what the current DT files claim.

[1] https://spdx.org/licenses/X11.html
[2] https://spdx.org/licenses/MIT.html

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 arch/arm64/boot/dts/marvell/berlin4ct-dmp.dts | 39 +------------------
 arch/arm64/boot/dts/marvell/berlin4ct-stb.dts | 39 +------------------
 2 files changed, 2 insertions(+), 76 deletions(-)

diff --git a/arch/arm64/boot/dts/marvell/berlin4ct-dmp.dts b/arch/arm64/boot/dts/marvell/berlin4ct-dmp.dts
index fae6c6924705..c64a179ebbb7 100644
--- a/arch/arm64/boot/dts/marvell/berlin4ct-dmp.dts
+++ b/arch/arm64/boot/dts/marvell/berlin4ct-dmp.dts
@@ -1,45 +1,8 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*
  * Copyright (C) 2015 Marvell Technology Group Ltd.
  *
  * Author: Jisheng Zhang <jszhang@marvell.com>
- *
- * This file is dual-licensed: you can use it either under the terms
- * of the GPLv2 or the X11 license, at your option. Note that this dual
- * licensing only applies to this file, and not this project as a
- * whole.
- *
- *  a) This library 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 library 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.
- *
- * Or, alternatively,
- *
- *  b) Permission is hereby granted, free of charge, to any person
- *     obtaining a copy of this software and associated documentation
- *     files (the "Software"), to deal in the Software without
- *     restriction, including without limitation the rights to use,
- *     copy, modify, merge, publish, distribute, sublicense, and/or
- *     sell copies of the Software, and to permit persons to whom the
- *     Software is furnished to do so, subject to the following
- *     conditions:
- *
- *     The above copyright notice and this permission notice shall be
- *     included in all copies or substantial portions of the Software.
- *
- *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
- *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
- *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
- *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
 /dts-v1/;
diff --git a/arch/arm64/boot/dts/marvell/berlin4ct-stb.dts b/arch/arm64/boot/dts/marvell/berlin4ct-stb.dts
index d47edad13e68..277dccfa05cb 100644
--- a/arch/arm64/boot/dts/marvell/berlin4ct-stb.dts
+++ b/arch/arm64/boot/dts/marvell/berlin4ct-stb.dts
@@ -1,45 +1,8 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*
  * Copyright (C) 2015 Marvell Technology Group Ltd.
  *
  * Author: Jisheng Zhang <jszhang@marvell.com>
- *
- * This file is dual-licensed: you can use it either under the terms
- * of the GPLv2 or the X11 license, at your option. Note that this dual
- * licensing only applies to this file, and not this project as a
- * whole.
- *
- *  a) This library 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 library 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.
- *
- * Or, alternatively,
- *
- *  b) Permission is hereby granted, free of charge, to any person
- *     obtaining a copy of this software and associated documentation
- *     files (the "Software"), to deal in the Software without
- *     restriction, including without limitation the rights to use,
- *     copy, modify, merge, publish, distribute, sublicense, and/or
- *     sell copies of the Software, and to permit persons to whom the
- *     Software is furnished to do so, subject to the following
- *     conditions:
- *
- *     The above copyright notice and this permission notice shall be
- *     included in all copies or substantial portions of the Software.
- *
- *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
- *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
- *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
- *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
 /dts-v1/;
-- 
2.17.0

^ permalink raw reply related

* [PATCH 1/2] ARM: dts: berlin*.dtsi: use SPDX-License-Identifier for berlin SoCs
From: Jisheng Zhang @ 2018-05-15 10:12 UTC (permalink / raw)
  To: linux-arm-kernel

Follow the recent trend for the license description, and also fix the
wrongly stated X11 to MIT.

As already pointed on the DT ML, the X11 license text [1] is explicitly
for the X Consortium and has a couple of extra clauses. The MIT
license text [2] is actually what the current DT files claim.

[1] https://spdx.org/licenses/X11.html
[2] https://spdx.org/licenses/MIT.html

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 arch/arm/boot/dts/berlin2.dtsi   | 33 +-------------------------------
 arch/arm/boot/dts/berlin2cd.dtsi | 33 +-------------------------------
 arch/arm/boot/dts/berlin2q.dtsi  | 33 +-------------------------------
 3 files changed, 3 insertions(+), 96 deletions(-)

diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi
index 787a9d042ad9..db67377af266 100644
--- a/arch/arm/boot/dts/berlin2.dtsi
+++ b/arch/arm/boot/dts/berlin2.dtsi
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*
  * Device Tree Include file for Marvell Armada 1500 (Berlin BG2) SoC
  *
@@ -5,38 +6,6 @@
  *
  * based on GPL'ed 2.6 kernel sources
  *  (c) Marvell International Ltd.
- *
- * This file is dual-licensed: you can use it either under the terms
- * of the GPL or the X11 license, at your option. Note that this dual
- * licensing only applies to this file, and not this project as a
- * whole.
- *
- *  a) This file is licensed under the terms of the GNU General Public
- *     License version 2. This program is licensed "as is" without any
- *     warranty of any kind, whether express or implied.
- *
- * Or, alternatively,
- *
- *  b) Permission is hereby granted, free of charge, to any person
- *     obtaining a copy of this software and associated documentation
- *     files (the "Software"), to deal in the Software without
- *     restriction, including without limitation the rights to use,
- *     copy, modify, merge, publish, distribute, sublicense, and/or
- *     sell copies of the Software, and to permit persons to whom the
- *     Software is furnished to do so, subject to the following
- *     conditions:
- *
- *     The above copyright notice and this permission notice shall be
- *     included in all copies or substantial portions of the Software.
- *
- *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
- *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
- *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
- *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
 #include <dt-bindings/clock/berlin2.h>
diff --git a/arch/arm/boot/dts/berlin2cd.dtsi b/arch/arm/boot/dts/berlin2cd.dtsi
index 501c59d97eae..387d02803690 100644
--- a/arch/arm/boot/dts/berlin2cd.dtsi
+++ b/arch/arm/boot/dts/berlin2cd.dtsi
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*
  * Device Tree Include file for Marvell Armada 1500-mini (Berlin BG2CD) SoC
  *
@@ -5,38 +6,6 @@
  *
  * based on GPL'ed 2.6 kernel sources
  *  (c) Marvell International Ltd.
- *
- * This file is dual-licensed: you can use it either under the terms
- * of the GPL or the X11 license, at your option. Note that this dual
- * licensing only applies to this file, and not this project as a
- * whole.
- *
- *  a) This file is licensed under the terms of the GNU General Public
- *     License version 2. This program is licensed "as is" without any
- *     warranty of any kind, whether express or implied.
- *
- * Or, alternatively,
- *
- *  b) Permission is hereby granted, free of charge, to any person
- *     obtaining a copy of this software and associated documentation
- *     files (the "Software"), to deal in the Software without
- *     restriction, including without limitation the rights to use,
- *     copy, modify, merge, publish, distribute, sublicense, and/or
- *     sell copies of the Software, and to permit persons to whom the
- *     Software is furnished to do so, subject to the following
- *     conditions:
- *
- *     The above copyright notice and this permission notice shall be
- *     included in all copies or substantial portions of the Software.
- *
- *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
- *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
- *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
- *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
 #include <dt-bindings/clock/berlin2.h>
diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index f11254e70f57..82e176011d36 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -1,37 +1,6 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*
  * Copyright (C) 2014 Antoine T?nart <antoine.tenart@free-electrons.com>
- *
- * This file is dual-licensed: you can use it either under the terms
- * of the GPL or the X11 license, at your option. Note that this dual
- * licensing only applies to this file, and not this project as a
- * whole.
- *
- *  a) This file is licensed under the terms of the GNU General Public
- *     License version 2. This program is licensed "as is" without any
- *     warranty of any kind, whether express or implied.
- *
- * Or, alternatively,
- *
- *  b) Permission is hereby granted, free of charge, to any person
- *     obtaining a copy of this software and associated documentation
- *     files (the "Software"), to deal in the Software without
- *     restriction, including without limitation the rights to use,
- *     copy, modify, merge, publish, distribute, sublicense, and/or
- *     sell copies of the Software, and to permit persons to whom the
- *     Software is furnished to do so, subject to the following
- *     conditions:
- *
- *     The above copyright notice and this permission notice shall be
- *     included in all copies or substantial portions of the Software.
- *
- *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
- *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
- *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
- *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
 #include <dt-bindings/clock/berlin2q.h>
-- 
2.17.0

^ permalink raw reply related

* [PATCH 08/18] arm64: convert raw syscall invocation to C
From: Mark Rutland @ 2018-05-15 10:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180515100158.GC14007@isilmar-4.linta.de>

On Tue, May 15, 2018 at 12:01:58PM +0200, Dominik Brodowski wrote:
> On Tue, May 15, 2018 at 09:22:23AM +0100, Mark Rutland wrote:
> > On Mon, May 14, 2018 at 10:24:45PM +0200, Dominik Brodowski wrote:
> > > On Mon, May 14, 2018 at 12:41:10PM +0100, Mark Rutland wrote:
> > > > I agree it would be nicer if it had a wrapper that took a pt_regs, even
> > > > if it does nothing with it.
> > > > 
> > > > We can't use SYSCALL_DEFINE0() due to the fault injection muck, we'd
> > > > need a ksys_ni_syscall() for our traps.c logic, and adding this
> > > > uniformly would involve some arch-specific rework for x86, too, so I
> > > > decided it was not worth the effort.
> > > 
> > > Couldn't you just open-code the "return -ENOSYS;" in traps.c?
> > 
> > I guess so. I was just worried that debug logic might be added to the generic
> > ni_syscall() in future, and wanted to avoid potential divergence.
> > 
> > > Error injection has no reasonable stable ABI/API expectations, so that's not
> > > a show-stopper either.
> > 
> > If people are happy with using SYSCALL_DEFINE0() for ni_syscall, I'm happy to
> > do that -- it's just that we'll need a fixup for x86 as that will change the
> > symbol name.
> 
> For me, it's less about using SYSCALL_DEFINE0() for ni_syscall, but more
> about keeping the syscall invokation easy. Therefore, we do pass a pointer
> struct pt_regs to sys_ni_syscall() on x86, even though it does not expect
> it.
> 
> 	/* this is a lie, but it does not hurt as sys_ni_syscall just returns -EINVAL */
> 	extern asmlinkage long sys_ni_syscall(const struct pt_regs *);

Oh, sure, we do the same on arm64 in this series.

Having a pt_regs wrapper for it (e.g. using SYSCALL_DEFINE0()) would
allow us to avoid that lie (which might be best for CFI stuff), would
allow us to avoid some name mangling on arm64, and would seemingly
confuse people less.

Thanks,
Mark.

^ permalink raw reply

* [PATCH 2/2] ARM: dts: berlin*-dts: use SPDX-License-Identifier for berlin based board
From: Jisheng Zhang @ 2018-05-15 10:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180515181216.6d82f7fc@xhacker.debian>

Follow the recent trend for the license description, and also fix the
wrongly stated X11 to MIT.

As already pointed on the DT ML, the X11 license text [1] is explicitly
for the X Consortium and has a couple of extra clauses. The MIT
license text [2] is actually what the current DT files claim.

[1] https://spdx.org/licenses/X11.html
[2] https://spdx.org/licenses/MIT.html

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts    | 33 +------------------
 .../boot/dts/berlin2cd-google-chromecast.dts  | 33 +------------------
 arch/arm/boot/dts/berlin2q-marvell-dmp.dts    | 33 +------------------
 3 files changed, 3 insertions(+), 96 deletions(-)

diff --git a/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts b/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts
index 1c475796d17f..64a297759eb9 100644
--- a/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts
+++ b/arch/arm/boot/dts/berlin2-sony-nsz-gs7.dts
@@ -1,39 +1,8 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*
  * Device Tree file for Sony NSZ-GS7
  *
  * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
- *
- * This file is dual-licensed: you can use it either under the terms
- * of the GPL or the X11 license, at your option. Note that this dual
- * licensing only applies to this file, and not this project as a
- * whole.
- *
- *  a) This file is licensed under the terms of the GNU General Public
- *     License version 2. This program is licensed "as is" without any
- *     warranty of any kind, whether express or implied.
- *
- * Or, alternatively,
- *
- *  b) Permission is hereby granted, free of charge, to any person
- *     obtaining a copy of this software and associated documentation
- *     files (the "Software"), to deal in the Software without
- *     restriction, including without limitation the rights to use,
- *     copy, modify, merge, publish, distribute, sublicense, and/or
- *     sell copies of the Software, and to permit persons to whom the
- *     Software is furnished to do so, subject to the following
- *     conditions:
- *
- *     The above copyright notice and this permission notice shall be
- *     included in all copies or substantial portions of the Software.
- *
- *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
- *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
- *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
- *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
 /dts-v1/;
diff --git a/arch/arm/boot/dts/berlin2cd-google-chromecast.dts b/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
index ca24def0ce13..540115172d3a 100644
--- a/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
+++ b/arch/arm/boot/dts/berlin2cd-google-chromecast.dts
@@ -1,39 +1,8 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*
  * Device Tree file for Google Chromecast
  *
  * Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
- *
- * This file is dual-licensed: you can use it either under the terms
- * of the GPL or the X11 license, at your option. Note that this dual
- * licensing only applies to this file, and not this project as a
- * whole.
- *
- *  a) This file is licensed under the terms of the GNU General Public
- *     License version 2. This program is licensed "as is" without any
- *     warranty of any kind, whether express or implied.
- *
- * Or, alternatively,
- *
- *  b) Permission is hereby granted, free of charge, to any person
- *     obtaining a copy of this software and associated documentation
- *     files (the "Software"), to deal in the Software without
- *     restriction, including without limitation the rights to use,
- *     copy, modify, merge, publish, distribute, sublicense, and/or
- *     sell copies of the Software, and to permit persons to whom the
- *     Software is furnished to do so, subject to the following
- *     conditions:
- *
- *     The above copyright notice and this permission notice shall be
- *     included in all copies or substantial portions of the Software.
- *
- *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
- *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
- *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
- *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
 /dts-v1/;
diff --git a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
index 57aa5f8a7c77..c162f98cb8e8 100644
--- a/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
+++ b/arch/arm/boot/dts/berlin2q-marvell-dmp.dts
@@ -1,37 +1,6 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
 /*
  * Copyright (C) 2014 Antoine T?nart <antoine.tenart@free-electrons.com>
- *
- * This file is dual-licensed: you can use it either under the terms
- * of the GPL or the X11 license, at your option. Note that this dual
- * licensing only applies to this file, and not this project as a
- * whole.
- *
- *  a) This file is licensed under the terms of the GNU General Public
- *     License version 2. This program is licensed "as is" without any
- *     warranty of any kind, whether express or implied.
- *
- * Or, alternatively,
- *
- *  b) Permission is hereby granted, free of charge, to any person
- *     obtaining a copy of this software and associated documentation
- *     files (the "Software"), to deal in the Software without
- *     restriction, including without limitation the rights to use,
- *     copy, modify, merge, publish, distribute, sublicense, and/or
- *     sell copies of the Software, and to permit persons to whom the
- *     Software is furnished to do so, subject to the following
- *     conditions:
- *
- *     The above copyright notice and this permission notice shall be
- *     included in all copies or substantial portions of the Software.
- *
- *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
- *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
- *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
- *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
 /dts-v1/;
-- 
2.17.0

^ permalink raw reply related

* [PATCH] ARM: dts: berlin2q: move PMU node from soc to root
From: Jisheng Zhang @ 2018-05-15 10:19 UTC (permalink / raw)
  To: linux-arm-kernel

Fix "make dtbs W=1" warns about missing reg or ranges property.

Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
 arch/arm/boot/dts/berlin2q.dtsi | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index 82e176011d36..c1bf23a4412b 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -62,6 +62,18 @@
 		};
 	};
 
+	pmu {
+		compatible = "arm,cortex-a9-pmu";
+		interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-affinity = <&cpu0>,
+				     <&cpu1>,
+				     <&cpu2>,
+				     <&cpu3>;
+	};
+
 	refclk: oscillator {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
@@ -76,18 +88,6 @@
 		ranges = <0 0xf7000000 0x1000000>;
 		interrupt-parent = <&gic>;
 
-		pmu {
-			compatible = "arm,cortex-a9-pmu";
-			interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>,
-				     <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-affinity = <&cpu0>,
-					     <&cpu1>,
-					     <&cpu2>,
-					     <&cpu3>;
-		};
-
 		sdhci0: sdhci at ab0000 {
 			compatible = "mrvl,pxav3-mmc";
 			reg = <0xab0000 0x200>;
-- 
2.17.0

^ permalink raw reply related

* [PATCH v2 26/26] drm/bridge: establish a link between the bridge supplier and consumer
From: Daniel Vetter @ 2018-05-15 10:22 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <73fa1ca3-28e4-96c5-1fc6-23e9c0cebb49@axentia.se>

On Mon, May 14, 2018 at 10:40 PM, Peter Rosin <peda@axentia.se> wrote:
> On 2018-05-14 18:28, Daniel Vetter wrote:
>> On Fri, May 11, 2018 at 09:37:47AM +0200, Peter Rosin wrote:
>>> On 2018-05-10 10:10, Andrzej Hajda wrote:
>>>> On 04.05.2018 15:52, Peter Rosin wrote:
>>>>> If the bridge supplier is unbound, this will bring the bridge consumer
>>>>> down along with the bridge. Thus, there will no longer linger any
>>>>> dangling pointers from the bridge consumer (the drm_device) to some
>>>>> non-existent bridge supplier.
>>>>>
>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>> ---
>>>>>  drivers/gpu/drm/drm_bridge.c | 18 ++++++++++++++++++
>>>>>  include/drm/drm_bridge.h     |  2 ++
>>>>>  2 files changed, 20 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
>>>>> index 78d186b6831b..0259f0a3ff27 100644
>>>>> --- a/drivers/gpu/drm/drm_bridge.c
>>>>> +++ b/drivers/gpu/drm/drm_bridge.c
>>>>> @@ -26,6 +26,7 @@
>>>>>  #include <linux/mutex.h>
>>>>>
>>>>>  #include <drm/drm_bridge.h>
>>>>> +#include <drm/drm_device.h>
>>>>>  #include <drm/drm_encoder.h>
>>>>>
>>>>>  #include "drm_crtc_internal.h"
>>>>> @@ -127,12 +128,25 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
>>>>>    if (bridge->dev)
>>>>>            return -EBUSY;
>>>>>
>>>>> +  if (encoder->dev->dev != bridge->odev) {
>>>>
>>>> I wonder why device_link_add does not handle this case (self dependency)
>>>> silently as noop, as it seems to be a correct behavior.
>>>
>>> It's kind-of a silly corner-case though, so perfectly understandable
>>> that it isn't handled.
>>>
>>>>> +          bridge->link = device_link_add(encoder->dev->dev,
>>>>> +                                         bridge->odev, 0);
>>>>> +          if (!bridge->link) {
>>>>> +                  dev_err(bridge->odev, "failed to link bridge to %s\n",
>>>>> +                          dev_name(encoder->dev->dev));
>>>>> +                  return -EINVAL;
>>>>> +          }
>>>>> +  }
>>>>> +
>>>>>    bridge->dev = encoder->dev;
>>>>>    bridge->encoder = encoder;
>>>>>
>>>>>    if (bridge->funcs->attach) {
>>>>>            ret = bridge->funcs->attach(bridge);
>>>>>            if (ret < 0) {
>>>>> +                  if (bridge->link)
>>>>> +                          device_link_del(bridge->link);
>>>>> +                  bridge->link = NULL;
>>>>>                    bridge->dev = NULL;
>>>>>                    bridge->encoder = NULL;
>>>>>                    return ret;
>>>>> @@ -159,6 +173,10 @@ void drm_bridge_detach(struct drm_bridge *bridge)
>>>>>    if (bridge->funcs->detach)
>>>>>            bridge->funcs->detach(bridge);
>>>>>
>>>>> +  if (bridge->link)
>>>>> +          device_link_del(bridge->link);
>>>>> +  bridge->link = NULL;
>>>>> +
>>>>>    bridge->dev = NULL;
>>>>>  }
>>>>>
>>>>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>>>>> index b656e505d11e..804189c63a4c 100644
>>>>> --- a/include/drm/drm_bridge.h
>>>>> +++ b/include/drm/drm_bridge.h
>>>>> @@ -261,6 +261,7 @@ struct drm_bridge_timings {
>>>>>   * @list: to keep track of all added bridges
>>>>>   * @timings: the timing specification for the bridge, if any (may
>>>>>   * be NULL)
>>>>> + * @link: drm consumer <-> bridge supplier
>>>>
>>>> Nitpick: "<->" suggests symmetry, maybe "device link from drm consumer
>>>> to the bridge" would be better.
>>>
>>> I meant "<->" to indicate that the link is bidirectional, not that the
>>> relationship is in any way symmetric. I wasn't aware of any implication
>>> of a symmetric relationship when using "<->", do you have a reference?
>>> But I guess the different arrow notations in math are somewhat overloaded
>>> and that someone at some point must have used "<->" to indicate a
>>> symmetric relationship...
>>
>> Yeah I agree with Andrzej here, for me <-> implies a symmetric
>> relationship. Spelling it out like Andrzej suggested sounds like the
>> better idea.
>> -Daniel
>
> Ok, I guess that means I have to do a v3 after all. Or can this
> trivial documentation update be done by the committer? I hate to
> spam everyone with another volley...
>
> Or perhaps I should squash patches 2-23 that are all rather similar
> and mechanic? I separated them to allow for easier review from
> individual driver maintainers, but that didn't seem to happen
> anyway...

Do another volley of the full set, or in-reply-to to just replace the
patch that needs to be respun (but some people don't like that).

When resending just make sure you're picking up all the acks/r-bs you
have already.
-Daniel
> Cheers,
> Peter
>
>>
>>>
>>>> Anyway:
>>>> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
>>>
>>> Thanks!
>>>
>>> Cheers,
>>> Peter
>>>
>>>>  --
>>>> Regards
>>>> Andrzej
>>>>
>>>>>   * @funcs: control functions
>>>>>   * @driver_private: pointer to the bridge driver's internal context
>>>>>   */
>>>>> @@ -271,6 +272,7 @@ struct drm_bridge {
>>>>>    struct drm_bridge *next;
>>>>>    struct list_head list;
>>>>>    const struct drm_bridge_timings *timings;
>>>>> +  struct device_link *link;
>>>>>
>>>>>    const struct drm_bridge_funcs *funcs;
>>>>>    void *driver_private;
>>>>
>>>>
>>>
>>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply

* [PATCH 0/4] KVM: arm/arm64: Fix locking issues
From: Christoffer Dall @ 2018-05-15 10:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180511142015.21953-1-andre.przywara@arm.com>

On Fri, May 11, 2018 at 03:20:11PM +0100, Andre Przywara wrote:
> Jan recently reported lockdep complaints regarding various locks in our
> VGIC emulation [1][2].
> This boiled down to two separate issues:
> - When promoting the vgic_irq->irq_lock to require IRQs being disabled,
>   we forgot to amend some instances of this lock on the way. Also this
>   needs to be applied to dependent locks as well. The first two patches
>   fix that. The patch split is designed to simplify backporting.
>   Those patches have been posted before, I am resending them as part
>   of this series.
> - Calling kvm_read_guest() requires us to be inside an SRCU critical
>   section. On some architectures we are always in it when handling VCPU
>   exits, but on ARM we need to lock it individually. Patches 3 and 4
>   fix that, the split is again made to ease backporting.
>   Each of the hunks fix an indiviual commit, but I refrained from
>   splitting this down into eight patches just to put proper Fixes: tags
>   on it. Eventually those commits are part of one out of two series, I put
>   the respective kernel release version as a tag to the Cc: stable line.
> 
> I couldn't reproduce the full lockdep splat on my setup, but at least
> could show one instance and prove that these patches fixes that.
> 
> 
For the series:

Acked-by: Christoffer Dall <christoffer.dall@arm.com>


Thanks for fixing this,
-Christoffer

^ permalink raw reply

* [RFC PATCH 00/10] Add persistent clock support
From: Daniel Lezcano @ 2018-05-15 10:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1526285602.git.baolin.wang@linaro.org>

On Mon, May 14, 2018 at 04:55:26PM +0800, Baolin Wang wrote:
> Hi,
> 
> We will meet below issues when compensating the suspend time for the timekeeping.
> 
> 1. We have too many different ways of dealing with persistent timekeeping
> across architectures, so it is hard for one driver to compatable with different
> architectures.
> 
> 2. On some platforms (such as Spreadtrum platform), we registered the high
> resolution timer as one clocksource to update the OS time, but the high
> resolution timer will be stopped in suspend state. So we use another one
> always-on timer (but low resolution) to calculate the suspend time to
> compensate the OS time. Though we can register the always-on timer as one
> clocksource, we need re-calculate the mult/shift with one larger conversion
> range to calculate the suspend time and need update the clock in case of
> running over the always-on timer.

First, can you elaborate what you mean by 'suspend state' ? On which power
domain the clocksource belongs to?


-- 

 <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply

* [PATCH v4 2/2] ThunderX2: Add Cavium ThunderX2 SoC UNCORE PMU driver
From: Ganapatrao Kulkarni @ 2018-05-15 10:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKTKpr7Q7T9ZCTBi=LQR=XaAoihbBA3OKCO7yFobzNmR8EfyjQ@mail.gmail.com>

Hi Mark,


On Sat, May 5, 2018 at 12:16 AM, Ganapatrao Kulkarni <gklkml16@gmail.com> wrote:
> Hi Mark,
>
> On Thu, Apr 26, 2018 at 4:29 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>> Hi,
>>
>> On Wed, Apr 25, 2018 at 02:30:47PM +0530, Ganapatrao Kulkarni wrote:
>>> +
>>> +/* L3c and DMC has 16 and 8 channels per socket respectively.
>>> + * Each Channel supports UNCORE PMU device and consists of
>>> + * 4 independent programmable counters. Counters are 32 bit
>>> + * and does not support overflow interrupt, they needs to be
>>> + * sampled before overflow(i.e, at every 2 seconds).
>>> + */
>>> +
>>> +#define UNCORE_MAX_COUNTERS          4
>>> +#define UNCORE_L3_MAX_TILES          16
>>> +#define UNCORE_DMC_MAX_CHANNELS              8
>>> +
>>> +#define UNCORE_HRTIMER_INTERVAL              (2 * NSEC_PER_SEC)
>>
>> How was a period of two seconds chosen?
>
> It has been suggested from hw team  to sample before  3 to 4 seconds.
>
>>
>> What's the maximum clock speed for the L3C and DMC?
>
> L3C at 1.5GHz and DMC at 1.2GHz
>>
>> Given that all channels compete for access to the muxed register
>> interface, I suspect we need to try more often than once every 2
>> seconds...
>
> 2 seconds seems to be sufficient. So far testing looks good.
>
>>
>> [...]
>>
>>> +struct active_timer {
>>> +     struct perf_event *event;
>>> +     struct hrtimer hrtimer;
>>> +};
>>> +
>>> +/*
>>> + * pmu on each socket has 2 uncore devices(dmc and l3),
>>> + * each uncore device has up to 16 channels, each channel can sample
>>> + * events independently with counters up to 4.
>>> + *
>>> + * struct thunderx2_pmu_uncore_channel created per channel.
>>> + * struct thunderx2_pmu_uncore_dev per uncore device.
>>> + */
>>> +struct thunderx2_pmu_uncore_channel {
>>> +     struct thunderx2_pmu_uncore_dev *uncore_dev;
>>> +     struct pmu pmu;
>>
>> Can we put the pmu first in the struct, please?
>
> ok
>>
>>> +     int counter;
>>
>> AFAICT, this counter field is never used.
>
> hmm ok, will remove.
>>
>>> +     int channel;
>>> +     DECLARE_BITMAP(counter_mask, UNCORE_MAX_COUNTERS);
>>> +     struct active_timer *active_timers;
>>
>> You should only need a single timer per channel, rather than one per
>> event.
>>
>> I think you can get rid of the active_timer structure, and have:
>>
>>         struct perf_event *events[UNCORE_MAX_COUNTERS];
>>         struct hrtimer timer;
>>
>
> thanks, will change as suggested.
>
>>> +     /* to sync counter alloc/release */
>>> +     raw_spinlock_t lock;
>>> +};
>>> +
>>> +struct thunderx2_pmu_uncore_dev {
>>> +     char *name;
>>> +     struct device *dev;
>>> +     enum thunderx2_uncore_type type;
>>> +     unsigned long base;
>>
>> This should be:
>>
>> void __iomem *base;
>
> ok
>>
>> [...]
>>
>>> +static ssize_t cpumask_show(struct device *dev,
>>> +                             struct device_attribute *attr, char *buf)
>>> +{
>>> +     struct cpumask cpu_mask;
>>> +     struct thunderx2_pmu_uncore_channel *pmu_uncore =
>>> +             pmu_to_thunderx2_pmu_uncore(dev_get_drvdata(dev));
>>> +
>>> +     /* Pick first online cpu from the node */
>>> +     cpumask_clear(&cpu_mask);
>>> +     cpumask_set_cpu(cpumask_first(
>>> +                     cpumask_of_node(pmu_uncore->uncore_dev->node)),
>>> +                     &cpu_mask);
>>> +
>>> +     return cpumap_print_to_pagebuf(true, buf, &cpu_mask);
>>> +}
>>
>> AFAICT cpumask_of_node() returns a mask that can include offline CPUs.
>>
>> Regardless, I don't think that you can keep track of the management CPU
>> this way. Please keep track of the CPU the PMU should be managed by,
>> either with a cpumask or int embedded within the PMU structure.
>
> thanks, will add hotplug callbacks.
>>
>> At hotplug time, you'll need to update the management CPU, calling
>> perf_pmu_migrate_context() when it is offlined.
>
> ok
>>
>> [...]
>>
>>> +static int alloc_counter(struct thunderx2_pmu_uncore_channel *pmu_uncore)
>>> +{
>>> +     int counter;
>>> +
>>> +     raw_spin_lock(&pmu_uncore->lock);
>>> +     counter = find_first_zero_bit(pmu_uncore->counter_mask,
>>> +                             pmu_uncore->uncore_dev->max_counters);
>>> +     if (counter == pmu_uncore->uncore_dev->max_counters) {
>>> +             raw_spin_unlock(&pmu_uncore->lock);
>>> +             return -ENOSPC;
>>> +     }
>>> +     set_bit(counter, pmu_uncore->counter_mask);
>>> +     raw_spin_unlock(&pmu_uncore->lock);
>>> +     return counter;
>>> +}
>>> +
>>> +static void free_counter(struct thunderx2_pmu_uncore_channel *pmu_uncore,
>>> +                                     int counter)
>>> +{
>>> +     raw_spin_lock(&pmu_uncore->lock);
>>> +     clear_bit(counter, pmu_uncore->counter_mask);
>>> +     raw_spin_unlock(&pmu_uncore->lock);
>>> +}
>>
>> I don't believe that locking is required in either of these, as the perf
>> core serializes pmu::add() and pmu::del(), where these get called.

without this locking, i am seeing "BUG: scheduling while atomic" when
i run perf with more events together than the maximum counters
supported

>
> thanks, it seems to be not required.
>>
>>> +
>>> +/*
>>> + * DMC and L3 counter interface is muxed across all channels.
>>> + * hence we need to select the channel before accessing counter
>>> + * data/control registers.
>>
>> Are there separate interfaces for all-dmc-channels and all-l3c-channels?
>
> separate interface for DMC and L3c.
> channels within DMC/L3C are muxed.
>
>>
>> ... or is a single interface used by all-dmc-and-l3c-channels?
>>
>>> + *
>>> + *  L3 Tile and DMC channel selection is through SMC call
>>> + *  SMC call arguments,
>>> + *   x0 = THUNDERX2_SMC_CALL_ID      (Vendor SMC call Id)
>>> + *   x1 = THUNDERX2_SMC_SET_CHANNEL  (Id to set DMC/L3C channel)
>>> + *   x2 = Node id
>>
>> How do we map Linux node IDs to the firmware's view of node IDs?
>>
>> I don't believe the two are necessarily the same -- Linux's node IDs are
>> a Linux-specific construct.
>
> both are same, it is numa node id from ACPI/firmware.
>
>>
>> It would be much nicer if we could pass something based on the MPIDR,
>> which is a known HW construct, or if this implicitly affected the
>> current node.
>
> IMO,  node id is sufficient.
>
>>
>> It would be vastly more sane for this to not be muxed at all. :/
>
> i am helpless due to crappy hw design!
>
>>
>>> + *   x3 = DMC(1)/L3C(0)
>>> + *   x4 = channel Id
>>> + */
>>> +static void uncore_select_channel(struct perf_event *event)
>>> +{
>>> +     struct arm_smccc_res res;
>>> +     struct thunderx2_pmu_uncore_channel *pmu_uncore =
>>> +             pmu_to_thunderx2_pmu_uncore(event->pmu);
>>> +
>>> +     arm_smccc_smc(THUNDERX2_SMC_CALL_ID, THUNDERX2_SMC_SET_CHANNEL,
>>> +                     pmu_uncore->uncore_dev->node,
>>> +                     pmu_uncore->uncore_dev->type,
>>> +                     pmu_uncore->channel, 0, 0, 0, &res);
>>> +
>>> +}
>>
>> Why aren't we checking the return value of the SMC call?
>
> thanks, i will add.
>
>>
>> The muxing and SMC sound quite scary. :/
>
> i too agree!, however i have no other option since the muxing register
> is a secure register.
>>
>>> +
>>> +static void uncore_start_event_l3c(struct perf_event *event, int flags)
>>> +{
>>> +     u32 val;
>>> +     struct hw_perf_event *hwc = &event->hw;
>>> +
>>> +     /* event id encoded in bits [07:03] */
>>> +     val = GET_EVENTID(event) << 3;
>>> +     reg_writel(val, hwc->config_base);
>>> +
>>> +     if (flags & PERF_EF_RELOAD) {
>>> +             u64 prev_raw_count =
>>> +                     local64_read(&event->hw.prev_count);
>>> +             reg_writel(prev_raw_count, hwc->event_base);
>>> +     }
>>> +     local64_set(&event->hw.prev_count,
>>> +                     reg_readl(hwc->event_base));
>>
>> It would be simpler to ignore PERF_EF_RELOAD, and always reprogram the
>> prev_count and HW to zero.
>
> ok, will change
>>
>>> +}
>>> +
>>> +static void uncore_start_event_dmc(struct perf_event *event, int flags)
>>> +{
>>> +     u32 val, event_shift = 8;
>>> +     struct hw_perf_event *hwc = &event->hw;
>>> +
>>> +     /* enable and start counters and read current value in prev_count */
>>> +     val = reg_readl(hwc->config_base);
>>> +
>>> +     /* 8 bits for each counter,
>>> +      * bits [05:01] of a counter to set event type.
>>> +      */
>>> +     reg_writel((val & ~(0x1f << (((GET_COUNTERID(event)) *
>>> +             event_shift) + 1))) |
>>> +             (GET_EVENTID(event) <<
>>> +              (((GET_COUNTERID(event)) * event_shift) + 1)),
>>> +             hwc->config_base);
>>
>> This would be far more legible if val were constructed before the
>> reg_writel(), especially if there were a helper for the event field
>> shifting, e.g.
>>
>>         #define DMC_EVENT_CFG(idx, val) ((val) << (((idx) * 8) + 1))
>>
>>         int id = GET_COUNTERID(event);
>>         int event = GET_EVENTID(event);
>>
>>         val = reg_readl(hwc->config_base);
>>         val &= ~DMC_EVENT_CFG(id, 0x1f);
>>         val |= DMCC_EVENT_CFG(id, event);
>>         reg_writel(val, hwc->config_base));
>>
>> What are bits 7:6 and 1 used for?
>
> not used/reserved bits.
>
>>
>>> +
>>> +     if (flags & PERF_EF_RELOAD) {
>>> +             u64 prev_raw_count =
>>> +                     local64_read(&event->hw.prev_count);
>>> +             reg_writel(prev_raw_count, hwc->event_base);
>>> +     }
>>> +     local64_set(&event->hw.prev_count,
>>> +                     reg_readl(hwc->event_base));
>>
>>
>> As with the L3C events, it would be simpler to always reprogram the
>> prev_count and HW to zero.
>
> ok
>>
>>> +}
>>> +
>>> +static void uncore_stop_event_l3c(struct perf_event *event)
>>> +{
>>> +     reg_writel(0, event->hw.config_base);
>>> +}
>>> +
>>> +static void uncore_stop_event_dmc(struct perf_event *event)
>>> +{
>>> +     u32 val, event_shift = 8;
>>> +     struct hw_perf_event *hwc = &event->hw;
>>> +
>>> +     val = reg_readl(hwc->config_base);
>>> +     reg_writel((val & ~(0xff << ((GET_COUNTERID(event)) * event_shift))),
>>> +                     hwc->config_base);
>>
>>
>> This could be simplified with the helper proposed above.
>
> ok
>>
>>> +}
>>> +
>>> +static void init_cntr_base_l3c(struct perf_event *event,
>>> +             struct thunderx2_pmu_uncore_dev *uncore_dev)
>>> +{
>>> +
>>> +     struct hw_perf_event *hwc = &event->hw;
>>> +
>>> +     /* counter ctrl/data reg offset at 8 */
>>
>> Offset 8, or stride 8?
>
> stride 8
>>
>> What does the register layout look like?
>>
>>> +     hwc->config_base = uncore_dev->base
>>> +             + L3C_COUNTER_CTL + (8 * GET_COUNTERID(event));
>>> +     hwc->event_base =  uncore_dev->base
>>> +             + L3C_COUNTER_DATA + (8 * GET_COUNTERID(event));
>>> +}
>>> +
>>> +static void init_cntr_base_dmc(struct perf_event *event,
>>> +             struct thunderx2_pmu_uncore_dev *uncore_dev)
>>> +{
>>> +
>>> +     struct hw_perf_event *hwc = &event->hw;
>>> +
>>> +     hwc->config_base = uncore_dev->base
>>> +             + DMC_COUNTER_CTL;
>>> +     /* counter data reg offset at 0xc */
>>
>> A stride of 0xc seems unusual.
>>
>> What does the register layout look like?
>
> the offsets are at 0x640, 0x64c, 0x658, 0x664,
>>
>>> +     hwc->event_base =  uncore_dev->base
>>> +             + DMC_COUNTER_DATA + (0xc * GET_COUNTERID(event));
>>> +}
>>> +
>>> +static void thunderx2_uncore_update(struct perf_event *event)
>>> +{
>>> +     s64 prev, new = 0;
>>> +     u64 delta;
>>> +     struct hw_perf_event *hwc = &event->hw;
>>> +     struct thunderx2_pmu_uncore_channel *pmu_uncore;
>>> +     enum thunderx2_uncore_type type;
>>> +
>>> +     pmu_uncore = pmu_to_thunderx2_pmu_uncore(event->pmu);
>>> +     type = pmu_uncore->uncore_dev->type;
>>
>> AFAICT this variable is not used below.
>
> thanks.
>>
>>> +
>>> +     if (pmu_uncore->uncore_dev->select_channel)
>>> +             pmu_uncore->uncore_dev->select_channel(event);
>>
>> This should always be non-NULL, right?
>
> yes, unwanted check at the moment, will remove.
>
>>
>> [...]
>>
>>> +static bool thunderx2_uncore_validate_event_group(struct perf_event *event)
>>> +{
>>> +     struct pmu *pmu = event->pmu;
>>> +     struct perf_event *leader = event->group_leader;
>>> +     struct perf_event *sibling;
>>> +     int counters = 0;
>>> +
>>> +     if (leader->pmu != event->pmu && !is_software_event(leader))
>>> +             return false;
>>> +
>>> +     counters++;
>>
>> I don't think this is right when event != leader and the leader is a SW
>> event. In that case, the leader doesn't take a HW counter.
>
> I think this check to avoid the grouping of  multiple hw PMUs ,
> however it is allowed to group sw events along with hw PMUs.
>
>>
>>> +
>>> +     for_each_sibling_event(sibling, event->group_leader) {
>>> +             if (is_software_event(sibling))
>>> +                     continue;
>>> +             if (sibling->pmu != pmu)
>>> +                     return false;
>>> +             counters++;
>>> +     }
>>> +
>>> +     /*
>>> +      * If the group requires more counters than the HW has,
>>> +      * it cannot ever be scheduled.
>>> +      */
>>> +     return counters <= UNCORE_MAX_COUNTERS;
>>> +}
>>
>> [...]
>>
>>> +static int thunderx2_uncore_event_init(struct perf_event *event)
>>> +{
>>> +     struct hw_perf_event *hwc = &event->hw;
>>> +     struct thunderx2_pmu_uncore_channel *pmu_uncore;
>>
>>> +     pmu_uncore = pmu_to_thunderx2_pmu_uncore(event->pmu);
>>> +
>>> +     if (!pmu_uncore)
>>> +             return -ENODEV;
>>
>> This cannot happen, given pmu_to_thunderx2_pmu_uncore() is a wrapper
>> around container_of().
>
> thanks, unnecessary check!
>>
>>> +
>>> +     /* Pick first online cpu from the node */
>>> +     event->cpu = cpumask_first(
>>> +                     cpumask_of_node(pmu_uncore->uncore_dev->node));
>>
>> I don't believe this is safe. You must keep track of which CPU is
>> managing the PMU, with hotplug callbacks.
>
> ok, will add callbacks.
>>
>> [...]
>>
>>> +static void thunderx2_uncore_start(struct perf_event *event, int flags)
>>> +{
>>> +     struct hw_perf_event *hwc = &event->hw;
>>> +     struct thunderx2_pmu_uncore_channel *pmu_uncore;
>>> +     struct thunderx2_pmu_uncore_dev *uncore_dev;
>>> +     unsigned long irqflags;
>>> +     struct active_timer *active_timer;
>>> +
>>> +     hwc->state = 0;
>>> +     pmu_uncore = pmu_to_thunderx2_pmu_uncore(event->pmu);
>>> +     uncore_dev = pmu_uncore->uncore_dev;
>>> +
>>> +     raw_spin_lock_irqsave(&uncore_dev->lock, irqflags);
>>> +
>>> +     if (uncore_dev->select_channel)
>>> +             uncore_dev->select_channel(event);
>>> +     uncore_dev->start_event(event, flags);
>>> +     raw_spin_unlock_irqrestore(&uncore_dev->lock, irqflags);
>>> +
>>> +     perf_event_update_userpage(event);
>>> +     active_timer = &pmu_uncore->active_timers[GET_COUNTERID(event)];
>>> +     active_timer->event = event;
>>> +
>>> +     hrtimer_start(&active_timer->hrtimer,
>>> +                     ns_to_ktime(uncore_dev->hrtimer_interval),
>>> +                     HRTIMER_MODE_REL_PINNED);
>>> +}
>>
>> Please use a single hrtimer, and update *all* of the events when it
>> fires.
>
> sure
>>
>> I *think* that can be done in the pmu::pmu_enable() and
>> pmu::pmu_disable() callbacks.
>
> ok
>>
>> Are there control bits to enable/disable all counters, or can that only
>> be done through the event configuration registers?
>
> only through config register.
>>
>>> +static void thunderx2_uncore_stop(struct perf_event *event, int flags)
>>> +{
>>> +     struct hw_perf_event *hwc = &event->hw;
>>> +     struct thunderx2_pmu_uncore_channel *pmu_uncore;
>>> +     struct thunderx2_pmu_uncore_dev *uncore_dev;
>>> +     unsigned long irqflags;
>>> +
>>> +     if (hwc->state & PERF_HES_UPTODATE)
>>> +             return;
>>> +
>>> +     pmu_uncore = pmu_to_thunderx2_pmu_uncore(event->pmu);
>>> +     uncore_dev = pmu_uncore->uncore_dev;
>>> +
>>> +     raw_spin_lock_irqsave(&uncore_dev->lock, irqflags);
>>> +
>>> +     if (uncore_dev->select_channel)
>>> +             uncore_dev->select_channel(event);
>>
>> AFAICT this cannot be NULL.
>
> ok.
>>
>> [...]
>>
>>> +static int thunderx2_pmu_uncore_register(
>>> +             struct thunderx2_pmu_uncore_channel *pmu_uncore)
>>> +{
>>> +     struct device *dev = pmu_uncore->uncore_dev->dev;
>>> +     char *name = pmu_uncore->uncore_dev->name;
>>> +     int channel = pmu_uncore->channel;
>>> +
>>> +     /* Perf event registration */
>>> +     pmu_uncore->pmu = (struct pmu) {
>>> +             .attr_groups    = pmu_uncore->uncore_dev->attr_groups,
>>> +             .task_ctx_nr    = perf_invalid_context,
>>> +             .event_init     = thunderx2_uncore_event_init,
>>> +             .add            = thunderx2_uncore_add,
>>> +             .del            = thunderx2_uncore_del,
>>> +             .start          = thunderx2_uncore_start,
>>> +             .stop           = thunderx2_uncore_stop,
>>> +             .read           = thunderx2_uncore_read,
>>> +     };
>>> +
>>> +     pmu_uncore->pmu.name = devm_kasprintf(dev, GFP_KERNEL,
>>> +                     "%s_%d", name, channel);
>>
>> Does the channel idx take the NUMA node into account?
>
> name already has node id suffix.
>>
>> [...]
>>
>>> +static int thunderx2_pmu_uncore_add(struct thunderx2_pmu_uncore_dev *uncore_dev,
>>> +             int channel)
>>> +{
>>
>>> +     /* we can run up to (max_counters * max_channels) events
>>> +      * simultaneously, allocate hrtimers per channel.
>>> +      */
>>> +     pmu_uncore->active_timers = devm_kzalloc(uncore_dev->dev,
>>> +                     sizeof(struct active_timer) * uncore_dev->max_counters,
>>> +                     GFP_KERNEL);
>>
>> Please just fold a single hrtimer into the thunderx2_pmu_uncore_channel
>> structure, and you can get rid of this allocation...
>
> sure, i will rewrite code to have timer per channel and all active
> counters are updated when timer fires.
>
>>
>>> +
>>> +     for (counter = 0; counter < uncore_dev->max_counters; counter++) {
>>> +             hrtimer_init(&pmu_uncore->active_timers[counter].hrtimer,
>>> +                             CLOCK_MONOTONIC,
>>> +                             HRTIMER_MODE_REL);
>>> +             pmu_uncore->active_timers[counter].hrtimer.function =
>>> +                             thunderx2_uncore_hrtimer_callback;
>>> +     }
>>
>> ... and simplify this initialization.
>
> ok
>>
>> [...]
>>
>>> +static struct thunderx2_pmu_uncore_dev *init_pmu_uncore_dev(
>>> +             struct device *dev, acpi_handle handle,
>>> +             struct acpi_device *adev, u32 type)
>>> +{
>>> +     struct thunderx2_pmu_uncore_dev *uncore_dev;
>>> +     unsigned long base;
>>
>>> +     base = (unsigned long)devm_ioremap_resource(dev, &res);
>>> +     if (IS_ERR((void *)base)) {
>>> +             dev_err(dev, "PMU type %d: Fail to map resource\n", type);
>>> +             return NULL;
>>> +     }
>>
>> Please treat this as a void __iomem *base.
>
> ok
>>
>> [...]
>>
>>> +static int thunderx2_uncore_probe(struct platform_device *pdev)
>>> +{
>>> +     struct device *dev = &pdev->dev;
>>> +     struct arm_smccc_res res;
>>> +
>>> +     set_dev_node(dev, acpi_get_node(ACPI_HANDLE(dev)));
>>> +
>>> +     /* Make sure firmware supports DMC/L3C set channel smc call */
>>> +     arm_smccc_smc(THUNDERX2_SMC_CALL_ID, THUNDERX2_SMC_SET_CHANNEL,
>>> +                     dev_to_node(dev), 0, 0, 0, 0, 0, &res);
>>> +     if (res.a0) {
>>> +             dev_err(dev, "No Firmware support for PMU UNCORE(%d)\n",
>>> +                             dev_to_node(dev));
>>> +             return -ENODEV;
>>> +     }
>>
>> Please re-use the uncore_select_channel() wrapper rather than
>> open-coding this.
>
> ok.
>>
>> Which FW supports this interface?
>
> it is through vendor specific ATF runtime services.
>>
>> Thanks,
>> Mark.
>
> thanks,
> Ganapat

thanks
Ganapat

^ permalink raw reply

* [GIT PULL 1/4] Rockchip driver updates for 4.18 round 1
From: Heiko Stuebner @ 2018-05-15 10:38 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd, Kevin Olof,

please find below and in the replies the main pull requests for Rockchip
stuff for 4.18. Depending on timing there may or may not be a second
round but so far I don't have anything big on the radar.

The signed tags should explain the individual contents, so please pull.


Thanks
Heiko


The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:

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

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git tags/v4.18-rockchip-drivers-1

for you to fetch changes up to 9e59c5f66c624b43c766a9fe3b2430e0e976bf0e:

  soc: rockchip: power-domain: Fix wrong value when power up pd with writemask (2018-05-14 11:53:26 +0200)

----------------------------------------------------------------
Fix for an issue introduced in 2016 where some powerdomains could only
be turned off but not on again.

----------------------------------------------------------------
Finley Xiao (1):
      soc: rockchip: power-domain: Fix wrong value when power up pd with writemask

 drivers/soc/rockchip/pm_domains.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

^ permalink raw reply

* [GIT PULL 2/4] Rockchip dts32 updates for 4.18 round 1
From: Heiko Stuebner @ 2018-05-15 10:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2000491.bLgWuj0xar@phil>

The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:

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

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git tags/v4.18-rockchip-dts32-1

for you to fetch changes up to 08624814cbec12b1ce877bf80f6990ad2b9cdcd7:

  ARM: dts: rockchip: default serial for rk3288 Tinker Board (2018-04-20 14:55:07 +0200)

----------------------------------------------------------------
Fixed pin numbers for uart4 on rk3288, iommu clocks and small changes
over multiple boards like default serial setting for rk3288-tinker,
output selection for the dp83867 on the phycore-som and the newly
added pwm-backlight delay properties for veyron boards.

----------------------------------------------------------------
Daniel Schultz (1):
      ARM: dts: rockchip: Add dp83867 CLK_OUT muxing on rk3288-phycore-som

Enric Balletbo i Serra (2):
      ARM: dts: rockchip: set PWM delay backlight settings for Veyron
      ARM: dts: rockchip: set PWM delay backlight settings for Minnie

Heinrich Schuchardt (1):
      ARM: dts: rockchip: default serial for rk3288 Tinker Board

Jacob Chen (1):
      ARM: dts: rockchip: fix uart4 pin-numbers for rk3288

Jeffy Chen (1):
      ARM: dts: rockchip: add clocks in iommu nodes

 arch/arm/boot/dts/rk3036.dtsi                   |  2 ++
 arch/arm/boot/dts/rk322x.dtsi                   |  8 ++++++++
 arch/arm/boot/dts/rk3288-phycore-som.dtsi       |  1 +
 arch/arm/boot/dts/rk3288-tinker.dts             |  4 ++++
 arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi |  3 ++-
 arch/arm/boot/dts/rk3288-veyron-minnie.dts      |  2 ++
 arch/arm/boot/dts/rk3288.dtsi                   | 20 ++++++++++++++++----
 7 files changed, 35 insertions(+), 5 deletions(-)

^ permalink raw reply

* [GIT PULL 3/4] Rockchip dts64 updates for 4.18 round 1
From: Heiko Stuebner @ 2018-05-15 10:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2000491.bLgWuj0xar@phil>

The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:

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

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git tags/v4.18-rockchip-dts64-1

for you to fetch changes up to 17bd0737948aa841b76278a601217b914aa5f18e:

  arm64: dts: rockchip: enable hdmi on rk3399-puma-haikou (2018-05-03 14:38:20 +0200)

----------------------------------------------------------------
All iommus got their clocks added and rk3399 got support for its
usb3-phy otg-port and better ajustment for the cpll child clocks.
On the board side, all rk3399 got their typec phys enabled - which
is needed for better usb support, the sapphire board got some more
properties moved to the excavator baseboard where they really belong,
kevin got a fix to use a real devicetree compatible and puma-haikou
got its hdmi port enabled.

----------------------------------------------------------------
Dmitry Torokhov (1):
      arm64: dts: rockchip: use canonical compatible for touchpad/touchscreen on gru-kevin

Enric Balletbo i Serra (5):
      arm64: dts: rockchip: enable typec-phy for rk3399-sapphire
      arm64: dts: rockchip: enable typec-phy for rk3399-firefly
      arm64: dts: rockchip: enable typec-phy1 for rk3399-puma
      arm64: dts: rockchip: enable typec-phy0 for rk3399-puma-haikou
      arm64: dts: rockchip: add usb3-phy otg-port support for rk3399

Jakob Unterwurzacher (1):
      arm64: dts: rockchip: enable hdmi on rk3399-puma-haikou

Jeffy Chen (1):
      arm64: dts: rockchip: add clocks in iommu nodes

Lin Huang (1):
      arm64: dts: rockchip: assign clock rate for cpll child clocks on rk3399

Vicente Bergas (2):
      arm64: dts: rockchip: move rk3399-sapphire PCIe to excavator baseboard
      arm64: dts: rockchip: remove PCIe assigned-clocks in excavator baseboard

 arch/arm64/boot/dts/rockchip/rk3328.dtsi           | 10 ++++++
 arch/arm64/boot/dts/rockchip/rk3368.dtsi           | 10 ++++++
 arch/arm64/boot/dts/rockchip/rk3399-firefly.dts    |  8 +++++
 arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts  |  4 +--
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi       |  8 +++--
 .../arm64/boot/dts/rockchip/rk3399-puma-haikou.dts | 25 +++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi      |  4 +++
 .../dts/rockchip/rk3399-sapphire-excavator.dts     | 12 ++++++++
 arch/arm64/boot/dts/rockchip/rk3399-sapphire.dtsi  | 23 +++++---------
 arch/arm64/boot/dts/rockchip/rk3399.dtsi           | 36 ++++++++++++++++------
 10 files changed, 111 insertions(+), 29 deletions(-)

^ permalink raw reply

* [PATCH 06/18] arm64: move sve_user_{enable, disable} to <asm/fpsimd.h>
From: Mark Rutland @ 2018-05-15 10:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180514110649.GC7753@e103592.cambridge.arm.com>

On Mon, May 14, 2018 at 12:06:50PM +0100, Dave Martin wrote:
> On Mon, May 14, 2018 at 10:46:28AM +0100, Mark Rutland wrote:
> > In subsequent patches, we'll want to make use of sve_user_enable() and
> > sve_user_disable() outside of kernel/fpsimd.c. Let's move these to
> > <asm/fpsimd.h> where we can make use of them.
> > 
> > To avoid ifdeffery in sequences like:
> > 
> > if (system_supports_sve() && some_condition
> > 	sve_user_disable();
> > 
> > ... empty stubs are provided when support for SVE is not enabled.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Dave Martin <dave.martin@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm64/include/asm/fpsimd.h | 17 ++++++++++++++++-
> >  arch/arm64/kernel/fpsimd.c      | 11 -----------
> >  2 files changed, 16 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
> > index aa7162ae93e3..7377d7593c06 100644
> > --- a/arch/arm64/include/asm/fpsimd.h
> > +++ b/arch/arm64/include/asm/fpsimd.h
> > @@ -16,11 +16,13 @@
> >  #ifndef __ASM_FP_H
> >  #define __ASM_FP_H
> >  
> > -#include <asm/ptrace.h>
> >  #include <asm/errno.h>
> > +#include <asm/ptrace.h>
> > +#include <asm/sysreg.h>
> >  
> >  #ifndef __ASSEMBLY__
> >  
> > +#include <linux/build_bug.h>
> >  #include <linux/cache.h>
> >  #include <linux/init.h>
> >  #include <linux/stddef.h>
> > @@ -81,6 +83,16 @@ extern int sve_set_vector_length(struct task_struct *task,
> >  extern int sve_set_current_vl(unsigned long arg);
> >  extern int sve_get_current_vl(void);
> >  
> > +static inline void sve_user_disable(void)
> > +{
> > +	sysreg_clear_set(cpacr_el1, CPACR_EL1_ZEN_EL0EN, 0);
> > +}
> > +
> > +static inline void sve_user_enable(void)
> > +{
> > +	sysreg_clear_set(cpacr_el1, 0, CPACR_EL1_ZEN_EL0EN);
> > +}
> > +
> >  /*
> >   * Probing and setup functions.
> >   * Calls to these functions must be serialised with one another.
> > @@ -107,6 +119,9 @@ static inline int sve_get_current_vl(void)
> >  	return -EINVAL;
> >  }
> >  
> > +static inline void sve_user_disable(void) { }
> > +static inline void sve_user_enable(void) { }
> > +
> 
> Alternatively, just move the full definitions outside the #ifdef
> CONFIG_ARM64_SVE.

Can do, though I was trying to keep the exsting pattern with empty
inlines for the !CONFIG_ARM64_SVE case.

> 
> All calls to these should be shadowed by an if
> (system_supports_sve()) in any case, and setting/clearing ZEN_EL0EN
> in the CPACR_EL1 ought to be harmless now that the meaning of these
> bits architecturally committed.
> 
> Ideally we would have a BUG_ON(!system_supports_sve()) in those
> functions, but we won't won't to pay the cost in a production kernel.

Earlier I'd put BUILD_BUG() in the body for the !CONFIG_ARM64_SVE case,
to catch that kind of thing -- I could restore that.

> >  static inline void sve_init_vq_map(void) { }
> >  static inline void sve_update_vq_map(void) { }
> >  static inline int sve_verify_vq_map(void) { return 0; }
> > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> > index 088940387a4d..79a81c7d85c6 100644
> > --- a/arch/arm64/kernel/fpsimd.c
> > +++ b/arch/arm64/kernel/fpsimd.c
> > @@ -159,7 +159,6 @@ static void sve_free(struct task_struct *task)
> >  	__sve_free(task);
> >  }
> >  
> > -
> 
> Hmmm, Ack.  Check for conflicts with the KVM FPSIMD rework [1] (though
> trivial).

I'll assume that Ack stands regardless. :)

Thanks,
Mark.

^ permalink raw reply

* [GIT PULL 4/4] Rockchip arm64 defconfig updates for 4.18 round 1
From: Heiko Stuebner @ 2018-05-15 10:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2000491.bLgWuj0xar@phil>

The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:

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

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git tags/v4.18-rockchip-defconfig64-1

for you to fetch changes up to 214f2c319a140f8a0121f362ad8e73ea1576d5ad:

  arm64: defconfig: enable rockchip efuse (2018-05-09 16:33:09 +0200)

----------------------------------------------------------------
Enablement of Rockchip-specific efuse, io-domain and typec drivers
as well as general cros-ec, hid, touchscreen bluetooth and wifi-drivers
on 64bit arm platforms.

----------------------------------------------------------------
Enric Balletbo i Serra (3):
      arm64: defconfig: Enable typec-phy and extcon-usbc-cros-ec for rk3399
      arm64: defconfig: Enable Rockchip io-domain driver
      arm64: defconfig: Enable ChromeOS EC drivers for supported Chromebooks.

Ezequiel Garcia (4):
      arm64: defconfig: Enable HID over I2C drivers
      arm64: defconfig: Enable Atmel Maxtouch driver
      arm64: defconfig: Enable Marvell WiFi-Ex PCIe driver
      arm64: defconfig: Enable bluetooth USB support

Heiko Stuebner (1):
      arm64: defconfig: enable rockchip efuse

 arch/arm64/configs/defconfig | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

^ permalink raw reply

* [PATCH v7 14/16] KVM: arm64: Remove redundant *exit_code changes in fpsimd_guest_exit()
From: Christoffer Dall @ 2018-05-15 10:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525882385-29181-15-git-send-email-Dave.Martin@arm.com>

On Wed, May 09, 2018 at 05:13:03PM +0100, Dave Martin wrote:
> In fixup_guest_exit(), there are a couple of cases where after
> checking what the exit code was, we assign it explicitly with the
> value it already had.
> 
> Assuming this is not indicative of a bug, these assignments are not
> needed.
> 
> This patch removes the redundant assignments, and simplifies some
> if-nesting that becomes trivial as a result.
> 
> No functional change.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/kvm/hyp/switch.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index a6a8c7d..18d0faa 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -403,12 +403,8 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
>  		if (valid) {
>  			int ret = __vgic_v2_perform_cpuif_access(vcpu);
>  
> -			if (ret == 1) {
> -				if (__skip_instr(vcpu))
> -					return true;
> -				else
> -					*exit_code = ARM_EXCEPTION_TRAP;
> -			}
> +			if (ret ==  1 && __skip_instr(vcpu))
> +				return true;
>  
>  			if (ret == -1) {
>  				/* Promote an illegal access to an
> @@ -430,12 +426,8 @@ static bool __hyp_text fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
>  	     kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_CP15_32)) {
>  		int ret = __vgic_v3_perform_cpuif_access(vcpu);
>  
> -		if (ret == 1) {
> -			if (__skip_instr(vcpu))
> -				return true;
> -			else
> -				*exit_code = ARM_EXCEPTION_TRAP;
> -		}
> +		if (ret == 1 && __skip_instr(vcpu))
> +			return true;
>  	}
>  
>  	/* Return to the host kernel and handle the exit */

I can't seem to easily convince myself that we couldn't have the
ARM_EXIT_WITH_SERROR_BIT set here and thus instead should be using
ARM_EXCEPTION_CODE() in the upper check.

Marc, can you help?

Thanks,
-Christoffer

^ permalink raw reply

* [PATCH v7 08/16] KVM: arm64: Optimise FPSIMD handling to reduce guest/host thrashing
From: Christoffer Dall @ 2018-05-15 10:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525882385-29181-9-git-send-email-Dave.Martin@arm.com>

On Wed, May 09, 2018 at 05:12:57PM +0100, Dave Martin wrote:
> This patch refactors KVM to align the host and guest FPSIMD
> save/restore logic with each other for arm64.  This reduces the
> number of redundant save/restore operations that must occur, and
> reduces the common-case IRQ blackout time during guest exit storms
> by saving the host state lazily and optimising away the need to
> restore the host state before returning to the run loop.
> 
> Four hooks are defined in order to enable this:
> 
>  * kvm_arch_vcpu_run_map_fp():
>    Called on PID change to map necessary bits of current to Hyp.
> 
>  * kvm_arch_vcpu_load_fp():
>    Set up FP/SIMD for entering the KVM run loop (parse as
>    "vcpu_load fp").
> 
>  * kvm_arch_vcpu_ctxsync_fp():
>    Get FP/SIMD into a safe state for re-enabling interrupts after a
>    guest exit back to the run loop.
> 
>    For arm64 specifically, this involves updating the host kernel's
>    FPSIMD context tracking metadata so that kernel-mode NEON use
>    will cause the vcpu's FPSIMD state to be saved back correctly
>    into the vcpu struct.  This must be done before re-enabling
>    interrupts because kernel-mode NEON may be used by softirqs.
> 
>  * kvm_arch_vcpu_put_fp():
>    Save guest FP/SIMD state back to memory and dissociate from the
>    CPU ("vcpu_put fp").
> 
> Also, the arm64 FPSIMD context switch code is updated to enable it
> to save back FPSIMD state for a vcpu, not just current.  A few
> helpers drive this:
> 
>  * fpsimd_bind_state_to_cpu(struct user_fpsimd_state *fp):
>    mark this CPU as having context fp (which may belong to a vcpu)
>    currently loaded in its registers.  This is the non-task
>    equivalent of the static function fpsimd_bind_to_cpu() in
>    fpsimd.c.
> 
>  * task_fpsimd_save():
>    exported to allow KVM to save the guest's FPSIMD state back to
>    memory on exit from the run loop.
> 
>  * fpsimd_flush_state():
>    invalidate any context's FPSIMD state that is currently loaded.
>    Used to disassociate the vcpu from the CPU regs on run loop exit.
> 
> These changes allow the run loop to enable interrupts (and thus
> softirqs that may use kernel-mode NEON) without having to save the
> guest's FPSIMD state eagerly.
> 
> Some new vcpu_arch fields are added to make all this work.  Because
> host FPSIMD state can now be saved back directly into current's
> thread_struct as appropriate, host_cpu_context is no longer used
> for preserving the FPSIMD state.  However, it is still needed for
> preserving other things such as the host's system registers.  To
> avoid ABI churn, the redundant storage space in host_cpu_context is
> not removed for now.
> 
> arch/arm is not addressed by this patch and continues to use its
> current save/restore logic.  It could provide implementations of
> the helpers later if desired.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> 
> ---
> 
> Dropped Reviewed-bys due to non-trivial changes.
> 
> Changes since v6:
> 
>  * Don't define kvm_arch_vcpu_run_pid_change() unless CONFIG_KVM=y.
> 
>    <asm/kvm_host.h> may be used for its declarations even with
>    CONFIG_KVM=n (e.g., in asm-offsets.c).
> 
>    This patch avoids conflicts with the core headers in this config.
> 
>  * Rebind current's FP state to the cpu in vcpu_put() if it is
>    still loaded, to ensure that the SVE trapping setup for userspace is
>    properly restored.
> 
> Requested by Marc Zyngier:
> 
>  * Add a comment to explain the purpose of update_fp_enabled().
> 
>  * Migrate vcpu_arch.flags definitions to kvm_host.h.
> 
>  * Eliminate magic NULL semantics for vcpu_arch.host_fpsimd_state so
>    that we can just assign this pointer once in the pid_change hook.
> 
>    A new flag KVM_ARM64_FP_HOST flag is added to capture the former
>    semantics of vcpu->arch.host_fpsimd_state != NULL.
> ---
>  arch/arm/include/asm/kvm_host.h   |   8 +++
>  arch/arm64/include/asm/fpsimd.h   |   6 ++
>  arch/arm64/include/asm/kvm_host.h |  21 +++++++
>  arch/arm64/kernel/fpsimd.c        |  17 +++++-
>  arch/arm64/kvm/Kconfig            |   1 +
>  arch/arm64/kvm/Makefile           |   2 +-
>  arch/arm64/kvm/fpsimd.c           | 112 ++++++++++++++++++++++++++++++++++++++
>  arch/arm64/kvm/hyp/switch.c       |  51 +++++++++--------
>  virt/kvm/arm/arm.c                |   4 ++
>  9 files changed, 192 insertions(+), 30 deletions(-)
>  create mode 100644 arch/arm64/kvm/fpsimd.c
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index c7c28c8..4cac8d1 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -303,6 +303,14 @@ int kvm_arm_vcpu_arch_get_attr(struct kvm_vcpu *vcpu,
>  int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu,
>  			       struct kvm_device_attr *attr);
>  
> +/*
> + * VFP/NEON switching is all done by the hyp switch code, so no need to
> + * coordinate with host context handling for this state:
> + */
> +static inline void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu) {}
> +static inline void kvm_arch_vcpu_park_fp(struct kvm_vcpu *vcpu) {}
> +static inline void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu) {}
> +
>  /* All host FP/SIMD state is restored on guest exit, so nothing to save: */
>  static inline void kvm_fpsimd_flush_cpu_state(void) {}
>  
> diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
> index aa7162a..3e00f70 100644
> --- a/arch/arm64/include/asm/fpsimd.h
> +++ b/arch/arm64/include/asm/fpsimd.h
> @@ -41,6 +41,8 @@ struct task_struct;
>  extern void fpsimd_save_state(struct user_fpsimd_state *state);
>  extern void fpsimd_load_state(struct user_fpsimd_state *state);
>  
> +extern void fpsimd_save(void);
> +
>  extern void fpsimd_thread_switch(struct task_struct *next);
>  extern void fpsimd_flush_thread(void);
>  
> @@ -49,7 +51,11 @@ extern void fpsimd_preserve_current_state(void);
>  extern void fpsimd_restore_current_state(void);
>  extern void fpsimd_update_current_state(struct user_fpsimd_state const *state);
>  
> +extern void fpsimd_bind_task_to_cpu(void);
> +extern void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *state);
> +
>  extern void fpsimd_flush_task_state(struct task_struct *target);
> +extern void fpsimd_flush_cpu_state(void);
>  extern void sve_flush_cpu_state(void);
>  
>  /* Maximum VL that SVE VL-agnostic software can transparently support */
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 146c167..b3fe730 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -30,6 +30,7 @@
>  #include <asm/kvm.h>
>  #include <asm/kvm_asm.h>
>  #include <asm/kvm_mmio.h>
> +#include <asm/thread_info.h>
>  
>  #define __KVM_HAVE_ARCH_INTC_INITIALIZED
>  
> @@ -238,6 +239,10 @@ struct kvm_vcpu_arch {
>  
>  	/* Pointer to host CPU context */
>  	kvm_cpu_context_t *host_cpu_context;
> +
> +	struct thread_info *host_thread_info;	/* hyp VA */
> +	struct user_fpsimd_state *host_fpsimd_state;	/* hyp VA */
> +
>  	struct {
>  		/* {Break,watch}point registers */
>  		struct kvm_guest_debug_arch regs;
> @@ -295,6 +300,9 @@ struct kvm_vcpu_arch {
>  
>  /* vcpu_arch flags field values: */
>  #define KVM_ARM64_DEBUG_DIRTY		(1 << 0)
> +#define KVM_ARM64_FP_ENABLED		(1 << 1) /* guest FP regs loaded */
> +#define KVM_ARM64_FP_HOST		(1 << 2) /* host FP regs loaded */
> +#define KVM_ARM64_HOST_SVE_IN_USE	(1 << 3) /* backup for host TIF_SVE */
>  
>  #define vcpu_gp_regs(v)		(&(v)->arch.ctxt.gp_regs)
>  
> @@ -423,6 +431,19 @@ static inline void __cpu_init_stage2(void)
>  		  "PARange is %d bits, unsupported configuration!", parange);
>  }
>  
> +/* Guest/host FPSIMD coordination helpers */
> +int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu);
> +void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu);
> +void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu);
> +void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
> +
> +#ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
> +static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
> +{
> +	return kvm_arch_vcpu_run_map_fp(vcpu);
> +}
> +#endif
> +
>  /*
>   * All host FP/SIMD state is restored on guest exit, so nothing needs
>   * doing here except in the SVE case:
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 60404eb..bc2892a 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -265,7 +265,7 @@ static void task_fpsimd_load(void)
>   *
>   * Softirqs (and preemption) must be disabled.
>   */
> -static void fpsimd_save(void)
> +void fpsimd_save(void)
>  {
>  	struct user_fpsimd_state *st = __this_cpu_read(fpsimd_last_state.st);
>  
> @@ -988,7 +988,7 @@ void fpsimd_signal_preserve_current_state(void)
>   * Associate current's FPSIMD context with this cpu
>   * Preemption must be disabled when calling this function.
>   */
> -static void fpsimd_bind_task_to_cpu(void)
> +void fpsimd_bind_task_to_cpu(void)
>  {
>  	struct fpsimd_last_state_struct *last =
>  		this_cpu_ptr(&fpsimd_last_state);
> @@ -1008,6 +1008,17 @@ static void fpsimd_bind_task_to_cpu(void)
>  	}
>  }
>  
> +void fpsimd_bind_state_to_cpu(struct user_fpsimd_state *st)
> +{
> +	struct fpsimd_last_state_struct *last =
> +		this_cpu_ptr(&fpsimd_last_state);
> +
> +	WARN_ON(!in_softirq() && !irqs_disabled());
> +
> +	last->st = st;
> +	last->sve_in_use = false;
> +}
> +
>  /*
>   * Load the userland FPSIMD state of 'current' from memory, but only if the
>   * FPSIMD state already held in the registers is /not/ the most recent FPSIMD
> @@ -1060,7 +1071,7 @@ void fpsimd_flush_task_state(struct task_struct *t)
>  	t->thread.fpsimd_cpu = NR_CPUS;
>  }
>  
> -static inline void fpsimd_flush_cpu_state(void)
> +void fpsimd_flush_cpu_state(void)
>  {
>  	__this_cpu_write(fpsimd_last_state.st, NULL);
>  }
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index a2e3a5a..47b23bf 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -39,6 +39,7 @@ config KVM
>  	select HAVE_KVM_IRQ_ROUTING
>  	select IRQ_BYPASS_MANAGER
>  	select HAVE_KVM_IRQ_BYPASS
> +	select HAVE_KVM_VCPU_RUN_PID_CHANGE
>  	---help---
>  	  Support hosting virtualized guest machines.
>  	  We don't support KVM with 16K page tables yet, due to the multiple
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index 93afff9..0f2a135 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -19,7 +19,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/psci.o $(KVM)/arm/perf.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += inject_fault.o regmap.o va_layout.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += hyp.o hyp-init.o handle_exit.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += guest.o debug.o reset.o sys_regs.o sys_regs_generic_v8.o
> -kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o
> +kvm-$(CONFIG_KVM_ARM_HOST) += vgic-sys-reg-v3.o fpsimd.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/aarch32.o
>  
>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic.o
> diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> new file mode 100644
> index 0000000..cdd78a77
> --- /dev/null
> +++ b/arch/arm64/kvm/fpsimd.c
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * arch/arm64/kvm/fpsimd.c: Guest/host FPSIMD context coordination helpers
> + *
> + * Copyright 2018 Arm Limited
> + * Author: Dave Martin <Dave.Martin@arm.com>
> + */
> +#include <linux/bottom_half.h>
> +#include <linux/sched.h>
> +#include <linux/thread_info.h>
> +#include <linux/kvm_host.h>
> +#include <asm/kvm_asm.h>
> +#include <asm/kvm_host.h>
> +#include <asm/kvm_mmu.h>
> +
> +/*
> + * Called on entry to KVM_RUN unless this vcpu previously ran at least
> + * once and the most recent prior KVM_RUN for this vcpu was called from
> + * the same task as current (highly likely).
> + *
> + * This is guaranteed to execute before kvm_arch_vcpu_load_fp(vcpu),
> + * such that on entering hyp the relevant parts of current are already
> + * mapped.
> + */
> +int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)
> +{
> +	int ret;
> +
> +	struct thread_info *ti = &current->thread_info;
> +	struct user_fpsimd_state *fpsimd = &current->thread.uw.fpsimd_state;
> +
> +	/*
> +	 * Make sure the host task thread flags and fpsimd state are
> +	 * visible to hyp:
> +	 */
> +	ret = create_hyp_mappings(ti, ti + 1, PAGE_HYP);
> +	if (ret)
> +		goto error;
> +
> +	ret = create_hyp_mappings(fpsimd, fpsimd + 1, PAGE_HYP);
> +	if (ret)
> +		goto error;
> +
> +	vcpu->arch.host_thread_info = kern_hyp_va(ti);
> +	vcpu->arch.host_fpsimd_state = kern_hyp_va(fpsimd);
> +error:
> +	return ret;
> +}
> +
> +/*
> + * Prepare vcpu for saving the host's FPSIMD state and loading the guest's.
> + * The actual loading is done by the FPSIMD access trap taken to hyp.
> + *
> + * Here, we just set the correct metadata to indicate that the FPSIMD
> + * state in the cpu regs (if any) belongs to current on the host.
> + *
> + * TIF_SVE is backed up here, since it may get clobbered with guest state.
> + * This flag is restored by kvm_arch_vcpu_put_fp(vcpu).
> + */
> +void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
> +{
> +	BUG_ON(system_supports_sve());
> +	BUG_ON(!current->mm);
> +
> +	vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED | KVM_ARM64_HOST_SVE_IN_USE);
> +	vcpu->arch.flags |= KVM_ARM64_FP_HOST;
> +	if (test_thread_flag(TIF_SVE))
> +		vcpu->arch.flags |= KVM_ARM64_HOST_SVE_IN_USE;
> +}
> +
> +/*
> + * If the guest FPSIMD state was loaded, update the host's context
> + * tracking data mark the CPU FPSIMD regs as dirty for vcpu so that they

nit: by marking the CPU FPSIMD regs as dirty and belonging to the
vcpu... ?

Otherwise:

Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>

> + * will be written back if the kernel clobbers them due to kernel-mode
> + * NEON before re-entry into the guest.
> + */
> +void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu)
> +{
> +	WARN_ON_ONCE(!irqs_disabled());
> +
> +	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
> +		fpsimd_bind_state_to_cpu(&vcpu->arch.ctxt.gp_regs.fp_regs);
> +		clear_thread_flag(TIF_FOREIGN_FPSTATE);
> +		clear_thread_flag(TIF_SVE);
> +	}
> +}
> +
> +/*
> + * Write back the vcpu FPSIMD regs if they are dirty, and invalidate the
> + * cpu FPSIMD regs so that they can't be spuriously reused if this vcpu
> + * disappears and another task or vcpu appears that recycles the same
> + * struct fpsimd_state.
> + */
> +void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
> +{
> +	local_bh_disable();
> +
> +	update_thread_flag(TIF_SVE,
> +			   vcpu->arch.flags & KVM_ARM64_HOST_SVE_IN_USE);
> +
> +	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) {
> +		/* Clean guest FP state to memory and invalidate cpu view */
> +		fpsimd_save();
> +		fpsimd_flush_cpu_state();
> +		set_thread_flag(TIF_FOREIGN_FPSTATE);
> +	} else if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) {
> +		/* Ensure user trap controls are correctly restored */
> +		fpsimd_bind_task_to_cpu();
> +	}
> +
> +	local_bh_enable();
> +}
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index c0796c4..118f300 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -23,19 +23,21 @@
>  
>  #include <asm/kvm_asm.h>
>  #include <asm/kvm_emulate.h>
> +#include <asm/kvm_host.h>
>  #include <asm/kvm_hyp.h>
>  #include <asm/kvm_mmu.h>
>  #include <asm/fpsimd.h>
>  #include <asm/debug-monitors.h>
> +#include <asm/thread_info.h>
>  
> -static bool __hyp_text __fpsimd_enabled_nvhe(void)
> +/* Check whether the FP regs were dirtied while in the host-side run loop: */
> +static bool __hyp_text update_fp_enabled(struct kvm_vcpu *vcpu)
>  {
> -	return !(read_sysreg(cptr_el2) & CPTR_EL2_TFP);
> -}
> +	if (vcpu->arch.host_thread_info->flags & _TIF_FOREIGN_FPSTATE)
> +		vcpu->arch.flags &= ~(KVM_ARM64_FP_ENABLED |
> +				      KVM_ARM64_FP_HOST);
>  
> -static bool fpsimd_enabled_vhe(void)
> -{
> -	return !!(read_sysreg(cpacr_el1) & CPACR_EL1_FPEN);
> +	return !!(vcpu->arch.flags & KVM_ARM64_FP_ENABLED);
>  }
>  
>  /* Save the 32-bit only FPSIMD system register state */
> @@ -92,7 +94,10 @@ static void activate_traps_vhe(struct kvm_vcpu *vcpu)
>  
>  	val = read_sysreg(cpacr_el1);
>  	val |= CPACR_EL1_TTA;
> -	val &= ~(CPACR_EL1_FPEN | CPACR_EL1_ZEN);
> +	val &= ~CPACR_EL1_ZEN;
> +	if (!update_fp_enabled(vcpu))
> +		val &= ~CPACR_EL1_FPEN;
> +
>  	write_sysreg(val, cpacr_el1);
>  
>  	write_sysreg(kvm_get_hyp_vector(), vbar_el1);
> @@ -105,7 +110,10 @@ static void __hyp_text __activate_traps_nvhe(struct kvm_vcpu *vcpu)
>  	__activate_traps_common(vcpu);
>  
>  	val = CPTR_EL2_DEFAULT;
> -	val |= CPTR_EL2_TTA | CPTR_EL2_TFP | CPTR_EL2_TZ;
> +	val |= CPTR_EL2_TTA | CPTR_EL2_TZ;
> +	if (!update_fp_enabled(vcpu))
> +		val |= CPTR_EL2_TFP;
> +
>  	write_sysreg(val, cptr_el2);
>  }
>  
> @@ -321,8 +329,6 @@ static bool __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
>  void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
>  				    struct kvm_vcpu *vcpu)
>  {
> -	kvm_cpu_context_t *host_ctxt;
> -
>  	if (has_vhe())
>  		write_sysreg(read_sysreg(cpacr_el1) | CPACR_EL1_FPEN,
>  			     cpacr_el1);
> @@ -332,14 +338,19 @@ void __hyp_text __hyp_switch_fpsimd(u64 esr __always_unused,
>  
>  	isb();
>  
> -	host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
> -	__fpsimd_save_state(&host_ctxt->gp_regs.fp_regs);
> +	if (vcpu->arch.flags & KVM_ARM64_FP_HOST) {
> +		__fpsimd_save_state(vcpu->arch.host_fpsimd_state);
> +		vcpu->arch.flags &= ~KVM_ARM64_FP_HOST;
> +	}
> +
>  	__fpsimd_restore_state(&vcpu->arch.ctxt.gp_regs.fp_regs);
>  
>  	/* Skip restoring fpexc32 for AArch64 guests */
>  	if (!(read_sysreg(hcr_el2) & HCR_RW))
>  		write_sysreg(vcpu->arch.ctxt.sys_regs[FPEXC32_EL2],
>  			     fpexc32_el2);
> +
> +	vcpu->arch.flags |= KVM_ARM64_FP_ENABLED;
>  }
>  
>  /*
> @@ -418,7 +429,6 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_cpu_context *host_ctxt;
>  	struct kvm_cpu_context *guest_ctxt;
> -	bool fp_enabled;
>  	u64 exit_code;
>  
>  	host_ctxt = vcpu->arch.host_cpu_context;
> @@ -440,19 +450,14 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
>  		/* And we're baaack! */
>  	} while (fixup_guest_exit(vcpu, &exit_code));
>  
> -	fp_enabled = fpsimd_enabled_vhe();
> -
>  	sysreg_save_guest_state_vhe(guest_ctxt);
>  
>  	__deactivate_traps(vcpu);
>  
>  	sysreg_restore_host_state_vhe(host_ctxt);
>  
> -	if (fp_enabled) {
> -		__fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
> -		__fpsimd_restore_state(&host_ctxt->gp_regs.fp_regs);
> +	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED)
>  		__fpsimd_save_fpexc32(vcpu);
> -	}
>  
>  	__debug_switch_to_host(vcpu);
>  
> @@ -464,7 +469,6 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_cpu_context *host_ctxt;
>  	struct kvm_cpu_context *guest_ctxt;
> -	bool fp_enabled;
>  	u64 exit_code;
>  
>  	vcpu = kern_hyp_va(vcpu);
> @@ -496,8 +500,6 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
>  		/* And we're baaack! */
>  	} while (fixup_guest_exit(vcpu, &exit_code));
>  
> -	fp_enabled = __fpsimd_enabled_nvhe();
> -
>  	__sysreg_save_state_nvhe(guest_ctxt);
>  	__sysreg32_save_state(vcpu);
>  	__timer_disable_traps(vcpu);
> @@ -508,11 +510,8 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
>  
>  	__sysreg_restore_state_nvhe(host_ctxt);
>  
> -	if (fp_enabled) {
> -		__fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
> -		__fpsimd_restore_state(&host_ctxt->gp_regs.fp_regs);
> +	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED)
>  		__fpsimd_save_fpexc32(vcpu);
> -	}
>  
>  	/*
>  	 * This must come after restoring the host sysregs, since a non-VHE
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index a4c1b76..bee226c 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -363,10 +363,12 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>  	kvm_vgic_load(vcpu);
>  	kvm_timer_vcpu_load(vcpu);
>  	kvm_vcpu_load_sysregs(vcpu);
> +	kvm_arch_vcpu_load_fp(vcpu);
>  }
>  
>  void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>  {
> +	kvm_arch_vcpu_put_fp(vcpu);
>  	kvm_vcpu_put_sysregs(vcpu);
>  	kvm_timer_vcpu_put(vcpu);
>  	kvm_vgic_put(vcpu);
> @@ -778,6 +780,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  		if (static_branch_unlikely(&userspace_irqchip_in_use))
>  			kvm_timer_sync_hwstate(vcpu);
>  
> +		kvm_arch_vcpu_ctxsync_fp(vcpu);
> +
>  		/*
>  		 * We may have taken a host interrupt in HYP mode (ie
>  		 * while executing the guest). This interrupt is still
> -- 
> 2.1.4
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply

* [PATCH v7 07/16] KVM: arm64: Repurpose vcpu_arch.debug_flags for general-purpose flags
From: Christoffer Dall @ 2018-05-15 10:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525882385-29181-8-git-send-email-Dave.Martin@arm.com>

On Wed, May 09, 2018 at 05:12:56PM +0100, Dave Martin wrote:
> In struct vcpu_arch, the debug_flags field is used to store
> debug-related flags about the vcpu state.
> 
> Since we are about to add some more flags related to FPSIMD and
> SVE, it makes sense to add them to the existing flags field rather
> than adding new fields.  Since there is only one debug_flags flag
> defined so far, there is plenty of free space for expansion.
> 
> In preparation for adding more flags, this patch renames the
> debug_flags field to simply "flags", and updates comments
> appropriately.
> 
> The flag definitions are also moved to <asm/kvm_host.h>, since
> their presence in <asm/kvm_asm.h> was for purely historical
> reasons:  these definitions are not used from asm any more, and not
> very likely to be as more Hyp asm is migrated to C.
> 
> KVM_ARM64_DEBUG_DIRTY_SHIFT has not been used since commit
> 1ea66d27e7b0 ("arm64: KVM: Move away from the assembly version of
> the world switch"), so this patch gets rid of that too.
> 
> No functional change.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
> 
Acked-by: Christoffer Dall <christoffer.dall@arm.com>

^ permalink raw reply

* [PATCH v7 16/16] KVM: arm64: Invoke FPSIMD context switch trap from C
From: Christoffer Dall @ 2018-05-15 10:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1525882385-29181-17-git-send-email-Dave.Martin@arm.com>

On Wed, May 09, 2018 at 05:13:05PM +0100, Dave Martin wrote:
> The conversion of the FPSIMD context switch trap code to C has added
> some overhead to calling it, due to the need to save registers that
> the procedure call standard defines as caller-saved.
> 
> So, perhaps it is no longer worth invoking this trap handler quite
> so early.
> 
> Instead, we can invoke it from fixup_guest_exit(), with little
> likelihood of increasing the overhead much further.
> 
> As a convenience, this patch gives __hyp_switch_fpsimd() the same
> return semantics fixup_guest_exit().  For now there is no
> possibility of a spurious FPSIMD trap, so the function always
> returns true, but this allows it to be tail-called with a single
> return statement.
> 
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>

^ permalink raw reply

* [PATCH v2 26/26] drm/bridge: establish a link between the bridge supplier and consumer
From: Peter Rosin @ 2018-05-15 11:09 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAKMK7uECSUo5k6uG3-y+yKQTGxB3FfGcwzMT+ZP5uux2SbpfUg@mail.gmail.com>

On 2018-05-15 12:22, Daniel Vetter wrote:
> On Mon, May 14, 2018 at 10:40 PM, Peter Rosin <peda@axentia.se> wrote:
>> On 2018-05-14 18:28, Daniel Vetter wrote:
>>> On Fri, May 11, 2018 at 09:37:47AM +0200, Peter Rosin wrote:
>>>> On 2018-05-10 10:10, Andrzej Hajda wrote:
>>>>> On 04.05.2018 15:52, Peter Rosin wrote:
>>>>>> If the bridge supplier is unbound, this will bring the bridge consumer
>>>>>> down along with the bridge. Thus, there will no longer linger any
>>>>>> dangling pointers from the bridge consumer (the drm_device) to some
>>>>>> non-existent bridge supplier.
>>>>>>
>>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>>> ---
>>>>>>  drivers/gpu/drm/drm_bridge.c | 18 ++++++++++++++++++
>>>>>>  include/drm/drm_bridge.h     |  2 ++
>>>>>>  2 files changed, 20 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
>>>>>> index 78d186b6831b..0259f0a3ff27 100644
>>>>>> --- a/drivers/gpu/drm/drm_bridge.c
>>>>>> +++ b/drivers/gpu/drm/drm_bridge.c
>>>>>> @@ -26,6 +26,7 @@
>>>>>>  #include <linux/mutex.h>
>>>>>>
>>>>>>  #include <drm/drm_bridge.h>
>>>>>> +#include <drm/drm_device.h>
>>>>>>  #include <drm/drm_encoder.h>
>>>>>>
>>>>>>  #include "drm_crtc_internal.h"
>>>>>> @@ -127,12 +128,25 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
>>>>>>    if (bridge->dev)
>>>>>>            return -EBUSY;
>>>>>>
>>>>>> +  if (encoder->dev->dev != bridge->odev) {
>>>>>
>>>>> I wonder why device_link_add does not handle this case (self dependency)
>>>>> silently as noop, as it seems to be a correct behavior.
>>>>
>>>> It's kind-of a silly corner-case though, so perfectly understandable
>>>> that it isn't handled.
>>>>
>>>>>> +          bridge->link = device_link_add(encoder->dev->dev,
>>>>>> +                                         bridge->odev, 0);
>>>>>> +          if (!bridge->link) {
>>>>>> +                  dev_err(bridge->odev, "failed to link bridge to %s\n",
>>>>>> +                          dev_name(encoder->dev->dev));
>>>>>> +                  return -EINVAL;
>>>>>> +          }
>>>>>> +  }
>>>>>> +
>>>>>>    bridge->dev = encoder->dev;
>>>>>>    bridge->encoder = encoder;
>>>>>>
>>>>>>    if (bridge->funcs->attach) {
>>>>>>            ret = bridge->funcs->attach(bridge);
>>>>>>            if (ret < 0) {
>>>>>> +                  if (bridge->link)
>>>>>> +                          device_link_del(bridge->link);
>>>>>> +                  bridge->link = NULL;
>>>>>>                    bridge->dev = NULL;
>>>>>>                    bridge->encoder = NULL;
>>>>>>                    return ret;
>>>>>> @@ -159,6 +173,10 @@ void drm_bridge_detach(struct drm_bridge *bridge)
>>>>>>    if (bridge->funcs->detach)
>>>>>>            bridge->funcs->detach(bridge);
>>>>>>
>>>>>> +  if (bridge->link)
>>>>>> +          device_link_del(bridge->link);
>>>>>> +  bridge->link = NULL;
>>>>>> +
>>>>>>    bridge->dev = NULL;
>>>>>>  }
>>>>>>
>>>>>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>>>>>> index b656e505d11e..804189c63a4c 100644
>>>>>> --- a/include/drm/drm_bridge.h
>>>>>> +++ b/include/drm/drm_bridge.h
>>>>>> @@ -261,6 +261,7 @@ struct drm_bridge_timings {
>>>>>>   * @list: to keep track of all added bridges
>>>>>>   * @timings: the timing specification for the bridge, if any (may
>>>>>>   * be NULL)
>>>>>> + * @link: drm consumer <-> bridge supplier
>>>>>
>>>>> Nitpick: "<->" suggests symmetry, maybe "device link from drm consumer
>>>>> to the bridge" would be better.
>>>>
>>>> I meant "<->" to indicate that the link is bidirectional, not that the
>>>> relationship is in any way symmetric. I wasn't aware of any implication
>>>> of a symmetric relationship when using "<->", do you have a reference?
>>>> But I guess the different arrow notations in math are somewhat overloaded
>>>> and that someone at some point must have used "<->" to indicate a
>>>> symmetric relationship...
>>>
>>> Yeah I agree with Andrzej here, for me <-> implies a symmetric
>>> relationship. Spelling it out like Andrzej suggested sounds like the
>>> better idea.
>>> -Daniel
>>
>> Ok, I guess that means I have to do a v3 after all. Or can this
>> trivial documentation update be done by the committer? I hate to
>> spam everyone with another volley...
>>
>> Or perhaps I should squash patches 2-23 that are all rather similar
>> and mechanic? I separated them to allow for easier review from
>> individual driver maintainers, but that didn't seem to happen
>> anyway...
> 
> Do another volley of the full set, or in-reply-to to just replace the
> patch that needs to be respun (but some people don't like that).
> 
> When resending just make sure you're picking up all the acks/r-bs you
> have already.

Right, I always try to do that. One Ack that I did not include in v2
was the one you had on v1 24/24 (i.e. this patch). The reason I did
not add your Ack for v2 even on the patch where it obviously applied
was that I didn't know if you'd barf on the odev name.

But it was (and still is) a bit unclear if that was on Ack on the
last patch only, or if it was for the whole series? I think it might
have been for the whole series, but I'm not sure and I hate to be a
presumptuous idiot...

Cheers,
Peter

> -Daniel
>> Cheers,
>> Peter
>>
>>>
>>>>
>>>>> Anyway:
>>>>> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
>>>>
>>>> Thanks!
>>>>
>>>> Cheers,
>>>> Peter
>>>>
>>>>>  --
>>>>> Regards
>>>>> Andrzej
>>>>>
>>>>>>   * @funcs: control functions
>>>>>>   * @driver_private: pointer to the bridge driver's internal context
>>>>>>   */
>>>>>> @@ -271,6 +272,7 @@ struct drm_bridge {
>>>>>>    struct drm_bridge *next;
>>>>>>    struct list_head list;
>>>>>>    const struct drm_bridge_timings *timings;
>>>>>> +  struct device_link *link;
>>>>>>
>>>>>>    const struct drm_bridge_funcs *funcs;
>>>>>>    void *driver_private;
>>>>>
>>>>>
>>>>
>>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 

^ permalink raw reply

* [PATCH v2 4/4] ARM: PWM: add allwinner sun8i pwm support.
From: Maxime Ripard @ 2018-05-15 11:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAJeuY7-Dz1u9J8GMu=OdzJ2cJUnE3wEobJoQz6RV-ibjBjoW4A@mail.gmail.com>

Hi,

On Mon, May 14, 2018 at 10:45:44PM +0800, Hao Zhang wrote:
> 2018-02-26 17:00 GMT+08:00 Maxime Ripard <maxime.ripard@bootlin.com>:
> > Thanks for respinning this serie. It looks mostly good, but you still
> > have a quite significant number of checkpatch (--strict) warnings that
> > you should address.
> 
> Thanks for reviews :) ,i'm sorry for that, it will be fixed next
> time.  and, besides, in what situation were the checkpatch warning
> can be ignore?

The only one that can be reasonably be ignored is the long line
warning, and only if complying to the limit would make it less easy to
understand.

> >
> > On Sun, Feb 25, 2018 at 09:53:08PM +0800, hao_zhang wrote:
> >> +#define CAPTURE_IRQ_ENABLE_REG       0x0010
> >> +#define CFIE(ch)     BIT(ch << 1 + 1)
> >> +#define CRIE(ch)     BIT(ch << 1)
> >
> > You should also put your argument between parentheses here (and in all
> > your other macros).
> 
> Do you mean like this ?
> #define CFIE(ch)     BIT((ch) << 1 + 1)
> #define CRIE(ch)     BIT((ch) << 1)

Yep, exactly. Otherwise, if you do something like CRIE(1 + 1), the
result will be BIT(1 + 1 << 1), which will expand to 3, instead of 4.

Also, CFIE looks a bit weird here, is it the offset that is
incremented, or the value? You should probably have parentheses to
make it explicit.

> >
> >> +static const u16 div_m_table[] = {
> >> +     1,
> >> +     2,
> >> +     4,
> >> +     8,
> >> +     16,
> >> +     32,
> >> +     64,
> >> +     128,
> >> +     256
> >> +};
> >
> > If this is just a power of two, you can use either the power of two /
> > ilog2 to switch back and forth, instead of using that table.
> 
> I think using table is more explicit and extended...

If you didn't have a simple mapping between the register values and
the divider value, then yeah, sure. But it's not the case here.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180515/23ba351d/attachment.sig>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox