* Re: [PATCH -next 02/15] thermal: bcm2835: use devm_platform_ioremap_resource() to simplify code
From: Stefan Wahren @ 2019-09-05 18:41 UTC (permalink / raw)
To: YueHaibing, miquel.raynal, rui.zhang, edubezval, daniel.lezcano,
amit.kucheria, eric, f.fainelli, rjui, sbranden, mmayer,
computersforpeace, gregory.0xf0, matthias.bgg, agross, heiko,
mcoquelin.stm32, alexandre.torgue, marc.w.gonzalez, mans, talel,
jun.nie, shawnguo, phil, gregkh, david.hernandezsanchez,
horms+renesas, wsa+renesas
Cc: linux-pm, linux-arm-msm, linux-kernel, linux-rockchip,
linux-mediatek, linux-rpi-kernel, bcm-kernel-feedback-list,
linux-stm32, linux-arm-kernel
In-Reply-To: <20190904122939.23780-3-yuehaibing@huawei.com>
Am 04.09.19 um 14:29 schrieb YueHaibing:
> Use devm_platform_ioremap_resource() to simplify the code a bit.
> This is detected by coccinelle.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Acked-by: Stefan Wahren <wahrenst@gmx.net>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH -next 06/36] spi: bcm2835: use devm_platform_ioremap_resource() to simplify code
From: Stefan Wahren @ 2019-09-05 18:39 UTC (permalink / raw)
To: YueHaibing, broonie, f.fainelli, rjui, sbranden, eric, shc_work,
agross, khilman, matthias.bgg, shawnguo, s.hauer, kernel,
festevam, linux-imx, avifishman70, tmaimon77, tali.perry1,
venture, yuenn, benjaminfair, kgene, krzk, andi, palmer,
paul.walmsley, baohua, mripard, wens, ldewangan, thierry.reding,
jonathanh, yamada.masahiro, michal.simek
Cc: linux-samsung-soc, linux-arm-msm, openbmc, linux-mediatek,
linux-kernel, linux-spi, bcm-kernel-feedback-list,
linux-rpi-kernel, linux-tegra, linux-amlogic, linux-riscv,
linux-arm-kernel
In-Reply-To: <20190904135918.25352-7-yuehaibing@huawei.com>
Am 04.09.19 um 15:58 schrieb YueHaibing:
> Use devm_platform_ioremap_resource() to simplify the code a bit.
> This is detected by coccinelle.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Acked-by: Stefan Wahren <wahrenst@gmx.net>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH -next 05/36] spi: bcm2835aux: use devm_platform_ioremap_resource() to simplify code
From: Stefan Wahren @ 2019-09-05 18:38 UTC (permalink / raw)
To: YueHaibing, broonie, f.fainelli, rjui, sbranden, eric, shc_work,
agross, khilman, matthias.bgg, shawnguo, s.hauer, kernel,
festevam, linux-imx, avifishman70, tmaimon77, tali.perry1,
venture, yuenn, benjaminfair, kgene, krzk, andi, palmer,
paul.walmsley, baohua, mripard, wens, ldewangan, thierry.reding,
jonathanh, yamada.masahiro, michal.simek
Cc: linux-samsung-soc, linux-arm-msm, openbmc, linux-mediatek,
linux-kernel, linux-spi, bcm-kernel-feedback-list,
linux-rpi-kernel, linux-tegra, linux-amlogic, linux-riscv,
linux-arm-kernel
In-Reply-To: <20190904135918.25352-6-yuehaibing@huawei.com>
Am 04.09.19 um 15:58 schrieb YueHaibing:
> Use devm_platform_ioremap_resource() to simplify the code a bit.
> This is detected by coccinelle.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Acked-by: Stefan Wahren <wahrenst@gmx.net>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v12 01/12] lib: introduce copy_struct_{to,from}_user helpers
From: Christian Brauner @ 2019-09-05 18:35 UTC (permalink / raw)
To: Al Viro
Cc: linux-ia64, linux-sh, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, Shuah Khan, linux-arch, linux-s390, Tycho Andersen,
Aleksa Sarai, Jiri Olsa, Alexander Shishkin, Ingo Molnar,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, Aleksa Sarai,
Andy Lutomirski, Shuah Khan, Namhyung Kim, David Drysdale,
Christian Brauner, J. Bruce Fields, linux-parisc, linux-m68k,
linux-api, Chanho Min, Jeff Layton, Oleg Nesterov, Eric Biederman,
linux-alpha, linux-fsdevel, Andrew Morton, Linus Torvalds,
containers
In-Reply-To: <20190905182801.GR1131@ZenIV.linux.org.uk>
On Thu, Sep 05, 2019 at 07:28:01PM +0100, Al Viro wrote:
> On Thu, Sep 05, 2019 at 08:23:03PM +0200, Christian Brauner wrote:
>
> > Because every caller of that function right now has that limit set
> > anyway iirc. So we can either remove it from here and place it back for
> > the individual callers or leave it in the helper.
> > Also, I'm really asking, why not? Is it unreasonable to have an upper
> > bound on the size (for a long time probably) or are you disagreeing with
> > PAGE_SIZE being used? PAGE_SIZE limit is currently used by sched, perf,
> > bpf, and clone3 and in a few other places.
>
> For a primitive that can be safely used with any size (OK, any within
> the usual 2Gb limit)? Why push the random policy into the place where
> it doesn't belong?
Ah, the "not in the helper part" makes sense.
As long as leave the check for the callers themselves.
>
> Seriously, what's the point? If they want to have a large chunk of
> userland memory zeroed or checked for non-zeroes - why would that
> be a problem?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] arm: dts and dtsi: Cleanup 'SPDX-License-Identifier:' uses
From: Joe Perches @ 2019-09-05 18:30 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: linux-spdx, linux-kernel, devicetree
Use the typical form and location for SPDX-License-Identifier:
Done with command line and perl script:
$ git grep -n 'SPDX-License-Identifier: ' -- '*.dtsi' '*.dts' | \
grep -v ':1:' | perl move_spdx.pl
$ cat move_spdx.pl
while (<>) {
/^([^:]+):([^:]+):(.*)/;
my ($file, $line, $spdx) = ($1, $2, $3);
$spdx =~ s/^\s*\/?\*\s*//;
$spdx =~ s/\s*\*\/\s*$//;
$spdx = "// $spdx";
open(FH, '<', $file) or die $!;
my @lines = <FH>;
close FH;
open(FH, '>', $file) or die $!;
print FH "$spdx\n";
my $count = 0;
foreach (@lines) {
$count++;
next if ($count == $line);
next if ($count == $line - 1 && $_ =~ /^\s*\*\s*$/);
next if ($count == $line + 1 && $_ =~ /^\s*\*\s*$/);
print FH "$_";
}
close FH;
}
And some typing for a few files with multiple spaces.
Signed-off-by: Joe Perches <joe@perches.com>
---
arch/arm/boot/dts/am335x-osd3358-sm-red.dts | 2 +-
arch/arm/boot/dts/am335x-pdu001.dts | 3 +--
arch/arm/boot/dts/aspeed-bmc-microsoft-olympus.dts | 2 +-
arch/arm/boot/dts/bcm953012hr.dts | 3 +--
arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts | 3 +--
arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dts | 3 +--
arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts | 3 +--
arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts | 3 +--
arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus.dts | 3 +--
arch/arm64/boot/dts/intel/socfpga_agilex_socdk.dts | 2 +-
arch/arm64/boot/dts/mediatek/mt2712-evb.dts | 3 +--
arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 3 +--
arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts | 3 +--
arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts | 3 +--
arch/arm64/boot/dts/mediatek/mt7622.dtsi | 3 +--
arch/arm64/boot/dts/realtek/rtd1295-mele-v9.dts | 3 +--
arch/arm64/boot/dts/realtek/rtd1295-probox2-ava.dts | 3 +--
arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts | 3 +--
arch/arm64/boot/dts/realtek/rtd1295.dtsi | 3 +--
arch/arm64/boot/dts/realtek/rtd129x.dtsi | 3 +--
arch/arm64/boot/dts/sprd/sc2731.dtsi | 3 +--
arch/arm64/boot/dts/sprd/sc9860.dtsi | 3 +--
arch/arm64/boot/dts/sprd/sp9860g-1h10.dts | 3 +--
arch/arm64/boot/dts/sprd/whale2.dtsi | 3 +--
arch/arm64/boot/dts/zte/zx296718-pcbox.dts | 3 +--
25 files changed, 25 insertions(+), 47 deletions(-)
diff --git a/arch/arm/boot/dts/am335x-osd3358-sm-red.dts b/arch/arm/boot/dts/am335x-osd3358-sm-red.dts
index f47cc9fea253..2d50ac4a8960 100644
--- a/arch/arm/boot/dts/am335x-osd3358-sm-red.dts
+++ b/arch/arm/boot/dts/am335x-osd3358-sm-red.dts
@@ -1,4 +1,4 @@
-//SPDX-License-Identifier: GPL-2.0
+// SPDX-License-Identifier: GPL-2.0
/* Copyright (C) 2018 Octavo Systems LLC - http://www.octavosystems.com/
*
* This program is free software; you can redistribute it and/or modify
diff --git a/arch/arm/boot/dts/am335x-pdu001.dts b/arch/arm/boot/dts/am335x-pdu001.dts
index 3141255f72c2..869d0257d9f8 100644
--- a/arch/arm/boot/dts/am335x-pdu001.dts
+++ b/arch/arm/boot/dts/am335x-pdu001.dts
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0+
/*
* pdu001.dts
*
@@ -6,8 +7,6 @@
* Copyright (C) 2018 EETS GmbH - http://www.eets.ch/
*
* Copyright (C) 2011, Texas Instruments, Incorporated - http://www.ti.com/
- *
- * SPDX-License-Identifier: GPL-2.0+
*/
/dts-v1/;
diff --git a/arch/arm/boot/dts/aspeed-bmc-microsoft-olympus.dts b/arch/arm/boot/dts/aspeed-bmc-microsoft-olympus.dts
index 73319917cb74..f8f9f71b878c 100644
--- a/arch/arm/boot/dts/aspeed-bmc-microsoft-olympus.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-microsoft-olympus.dts
@@ -1,4 +1,4 @@
-//SPDX-License-Identifier: GPL-2.0+
+// SPDX-License-Identifier: GPL-2.0+
/dts-v1/;
diff --git a/arch/arm/boot/dts/bcm953012hr.dts b/arch/arm/boot/dts/bcm953012hr.dts
index 9140be7ec053..6dee6974d4a0 100644
--- a/arch/arm/boot/dts/bcm953012hr.dts
+++ b/arch/arm/boot/dts/bcm953012hr.dts
@@ -1,6 +1,5 @@
+// SPDX-License-Identifier: BSD-3-Clause
/*
- * SPDX-License-Identifier: BSD-3-Clause
- *
* Copyright(c) 2017 Broadcom
*
* Redistribution and use in source and binary forms, with or without
diff --git a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
index 2b760f90f38c..fcc20e6d405c 100644
--- a/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
+++ b/arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts
@@ -1,7 +1,6 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
* Copyright 2017-2018 Sean Wang <sean.wang@mediatek.com>
- *
- * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
*/
/dts-v1/;
diff --git a/arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dts b/arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dts
index a8b2f0f1c11d..50f2fb30d2d4 100644
--- a/arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-libretech-all-h3-cc.dts
@@ -1,7 +1,6 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
* Copyright (C) 2017 Chen-Yu Tsai <wens@csie.org>
- *
- * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
*/
/dts-v1/;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts
index 72d6961dc312..2ca36580436c 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-lts.dts
@@ -1,6 +1,5 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
- * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
- *
* Copyright (c) 2018 ARM Ltd.
*/
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
index 1069e7012c9c..af44cd0e9b8e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
@@ -1,8 +1,7 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
/*
* Copyright (C) Harald Geyer <harald@ccbib.org>
* based on sun50i-a64-olinuxino.dts by Jagan Teki <jteki@openedev.com>
- *
- * SPDX-License-Identifier: (GPL-2.0 OR MIT)
*/
/dts-v1/;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus.dts
index db6ea7b58999..a56ac6a28df0 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus.dts
@@ -1,8 +1,7 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR X11)
/*
* Copyright (C) 2016 ARM Ltd.
* Copyright (C) 2018 Hauke Mehrtens <hauke@hauke-m.de>
- *
- * SPDX-License-Identifier: (GPL-2.0+ OR X11)
*/
/dts-v1/;
diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex_socdk.dts b/arch/arm64/boot/dts/intel/socfpga_agilex_socdk.dts
index 7814a9e8eb08..f5ff8571eb13 100644
--- a/arch/arm64/boot/dts/intel/socfpga_agilex_socdk.dts
+++ b/arch/arm64/boot/dts/intel/socfpga_agilex_socdk.dts
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (C) 2019, Intel Corporation
*/
diff --git a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
index 2b91daf5c1a6..4a55f3a71c12 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
@@ -1,8 +1,7 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
/*
* Copyright (c) 2017 MediaTek Inc.
* Author: YT Shen <yt.shen@mediatek.com>
- *
- * SPDX-License-Identifier: (GPL-2.0 OR MIT)
*/
/dts-v1/;
diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index 43307bad3f0d..96777f0373ec 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -1,8 +1,7 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
/*
* Copyright (c) 2017 MediaTek Inc.
* Author: YT Shen <yt.shen@mediatek.com>
- *
- * SPDX-License-Identifier: (GPL-2.0 OR MIT)
*/
#include <dt-bindings/clock/mt2712-clk.h>
diff --git a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
index 83e10591e0e5..0cab2bd5e79f 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
@@ -1,8 +1,7 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
/*
* Copyright (c) 2018 MediaTek Inc.
* Author: Ryder Lee <ryder.lee@mediatek.com>
- *
- * SPDX-License-Identifier: (GPL-2.0 OR MIT)
*/
/dts-v1/;
diff --git a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
index 3f783348c66a..b13ab8b3411f 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
@@ -1,9 +1,8 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
/*
* Copyright (c) 2017 MediaTek Inc.
* Author: Ming Huang <ming.huang@mediatek.com>
* Sean Wang <sean.wang@mediatek.com>
- *
- * SPDX-License-Identifier: (GPL-2.0 OR MIT)
*/
/dts-v1/;
diff --git a/arch/arm64/boot/dts/mediatek/mt7622.dtsi b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
index dac51e98204c..4ed2e231f693 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
@@ -1,9 +1,8 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
/*
* Copyright (c) 2017 MediaTek Inc.
* Author: Ming Huang <ming.huang@mediatek.com>
* Sean Wang <sean.wang@mediatek.com>
- *
- * SPDX-License-Identifier: (GPL-2.0 OR MIT)
*/
#include <dt-bindings/interrupt-controller/irq.h>
diff --git a/arch/arm64/boot/dts/realtek/rtd1295-mele-v9.dts b/arch/arm64/boot/dts/realtek/rtd1295-mele-v9.dts
index bd584e99fff9..88687591ac83 100644
--- a/arch/arm64/boot/dts/realtek/rtd1295-mele-v9.dts
+++ b/arch/arm64/boot/dts/realtek/rtd1295-mele-v9.dts
@@ -1,7 +1,6 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
* Copyright (c) 2017 Andreas Färber
- *
- * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
*/
/dts-v1/;
diff --git a/arch/arm64/boot/dts/realtek/rtd1295-probox2-ava.dts b/arch/arm64/boot/dts/realtek/rtd1295-probox2-ava.dts
index 8e2b0e75298a..4e917d4f880f 100644
--- a/arch/arm64/boot/dts/realtek/rtd1295-probox2-ava.dts
+++ b/arch/arm64/boot/dts/realtek/rtd1295-probox2-ava.dts
@@ -1,7 +1,6 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
* Copyright (c) 2017 Andreas Färber
- *
- * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
*/
/dts-v1/;
diff --git a/arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts b/arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts
index da19faab29d5..8b52c907f9bf 100644
--- a/arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts
+++ b/arch/arm64/boot/dts/realtek/rtd1295-zidoo-x9s.dts
@@ -1,7 +1,6 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
* Copyright (c) 2016-2017 Andreas Färber
- *
- * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
*/
/dts-v1/;
diff --git a/arch/arm64/boot/dts/realtek/rtd1295.dtsi b/arch/arm64/boot/dts/realtek/rtd1295.dtsi
index 41d7858da826..0c413e710930 100644
--- a/arch/arm64/boot/dts/realtek/rtd1295.dtsi
+++ b/arch/arm64/boot/dts/realtek/rtd1295.dtsi
@@ -1,9 +1,8 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
* Realtek RTD1295 SoC
*
* Copyright (c) 2016-2017 Andreas Färber
- *
- * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
*/
#include "rtd129x.dtsi"
diff --git a/arch/arm64/boot/dts/realtek/rtd129x.dtsi b/arch/arm64/boot/dts/realtek/rtd129x.dtsi
index b9cb92466fc7..ea109855718b 100644
--- a/arch/arm64/boot/dts/realtek/rtd129x.dtsi
+++ b/arch/arm64/boot/dts/realtek/rtd129x.dtsi
@@ -1,9 +1,8 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
* Realtek RTD1293/RTD1295/RTD1296 SoC
*
* Copyright (c) 2016-2017 Andreas Färber
- *
- * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
*/
/memreserve/ 0x0000000000000000 0x0000000000030000;
diff --git a/arch/arm64/boot/dts/sprd/sc2731.dtsi b/arch/arm64/boot/dts/sprd/sc2731.dtsi
index e15409f55f43..fc4e2b1e160e 100644
--- a/arch/arm64/boot/dts/sprd/sc2731.dtsi
+++ b/arch/arm64/boot/dts/sprd/sc2731.dtsi
@@ -1,9 +1,8 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
* Spreadtrum SC2731 PMIC dts file
*
* Copyright (C) 2018, Spreadtrum Communications Inc.
- *
- * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
*/
&adi_bus {
diff --git a/arch/arm64/boot/dts/sprd/sc9860.dtsi b/arch/arm64/boot/dts/sprd/sc9860.dtsi
index e27eb3ed1d47..9dad0cefafc0 100644
--- a/arch/arm64/boot/dts/sprd/sc9860.dtsi
+++ b/arch/arm64/boot/dts/sprd/sc9860.dtsi
@@ -1,9 +1,8 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
* Spreadtrum SC9860 SoC
*
* Copyright (C) 2016, Spreadtrum Communications Inc.
- *
- * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
*/
#include <dt-bindings/interrupt-controller/arm-gic.h>
diff --git a/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts b/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
index 6b95fd94cee3..4423acd0c9c0 100644
--- a/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
+++ b/arch/arm64/boot/dts/sprd/sp9860g-1h10.dts
@@ -1,9 +1,8 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
* Spreadtrum SP9860g board
*
* Copyright (C) 2017, Spreadtrum Communications Inc.
- *
- * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
*/
/dts-v1/;
diff --git a/arch/arm64/boot/dts/sprd/whale2.dtsi b/arch/arm64/boot/dts/sprd/whale2.dtsi
index 79b9591c37aa..b6e1378e2297 100644
--- a/arch/arm64/boot/dts/sprd/whale2.dtsi
+++ b/arch/arm64/boot/dts/sprd/whale2.dtsi
@@ -1,9 +1,8 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
* Spreadtrum Whale2 platform peripherals
*
* Copyright (C) 2016, Spreadtrum Communications Inc.
- *
- * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
*/
#include <dt-bindings/clock/sprd,sc9860-clk.h>
diff --git a/arch/arm64/boot/dts/zte/zx296718-pcbox.dts b/arch/arm64/boot/dts/zte/zx296718-pcbox.dts
index e02509f7082b..8fd6cf518a2f 100644
--- a/arch/arm64/boot/dts/zte/zx296718-pcbox.dts
+++ b/arch/arm64/boot/dts/zte/zx296718-pcbox.dts
@@ -1,8 +1,7 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
* Copyright (C) 2017 Sanechips Technology Co., Ltd.
* Copyright 2017 Linaro Ltd.
- *
- * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
*/
/dts-v1/;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v12 01/12] lib: introduce copy_struct_{to,from}_user helpers
From: Al Viro @ 2019-09-05 18:28 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-ia64, linux-sh, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, Shuah Khan, linux-arch, linux-s390, Tycho Andersen,
Aleksa Sarai, Jiri Olsa, Alexander Shishkin, Ingo Molnar,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, Aleksa Sarai,
Andy Lutomirski, Shuah Khan, Namhyung Kim, David Drysdale,
Christian Brauner, J. Bruce Fields, linux-parisc, linux-m68k,
linux-api, Chanho Min, Jeff Layton, Oleg Nesterov, Eric Biederman,
linux-alpha, linux-fsdevel, Andrew Morton, Linus Torvalds,
containers
In-Reply-To: <20190905182303.7f6bxpa2enbgcegv@wittgenstein>
On Thu, Sep 05, 2019 at 08:23:03PM +0200, Christian Brauner wrote:
> Because every caller of that function right now has that limit set
> anyway iirc. So we can either remove it from here and place it back for
> the individual callers or leave it in the helper.
> Also, I'm really asking, why not? Is it unreasonable to have an upper
> bound on the size (for a long time probably) or are you disagreeing with
> PAGE_SIZE being used? PAGE_SIZE limit is currently used by sched, perf,
> bpf, and clone3 and in a few other places.
For a primitive that can be safely used with any size (OK, any within
the usual 2Gb limit)? Why push the random policy into the place where
it doesn't belong?
Seriously, what's the point? If they want to have a large chunk of
userland memory zeroed or checked for non-zeroes - why would that
be a problem?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v12 01/12] lib: introduce copy_struct_{to,from}_user helpers
From: Christian Brauner @ 2019-09-05 18:23 UTC (permalink / raw)
To: Al Viro
Cc: linux-ia64, linux-sh, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, Shuah Khan, linux-arch, linux-s390, Tycho Andersen,
Aleksa Sarai, Jiri Olsa, Alexander Shishkin, Ingo Molnar,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, Aleksa Sarai,
Andy Lutomirski, Shuah Khan, Namhyung Kim, David Drysdale,
Christian Brauner, J. Bruce Fields, linux-parisc, linux-m68k,
linux-api, Chanho Min, Jeff Layton, Oleg Nesterov, Eric Biederman,
linux-alpha, linux-fsdevel, Andrew Morton, Linus Torvalds,
containers
In-Reply-To: <20190905180750.GQ1131@ZenIV.linux.org.uk>
On Thu, Sep 05, 2019 at 07:07:50PM +0100, Al Viro wrote:
> On Thu, Sep 05, 2019 at 06:19:22AM +1000, Aleksa Sarai wrote:
> > +/*
> > + * "memset(p, 0, size)" but for user space buffers. Caller must have already
> > + * checked access_ok(p, size).
> > + */
> > +static int __memzero_user(void __user *p, size_t s)
> > +{
> > + const char zeros[BUFFER_SIZE] = {};
> > + while (s > 0) {
> > + size_t n = min(s, sizeof(zeros));
> > +
> > + if (__copy_to_user(p, zeros, n))
> > + return -EFAULT;
> > +
> > + p += n;
> > + s -= n;
> > + }
> > + return 0;
> > +}
>
> That's called clear_user().
>
> > +int copy_struct_to_user(void __user *dst, size_t usize,
> > + const void *src, size_t ksize)
> > +{
> > + size_t size = min(ksize, usize);
> > + size_t rest = abs(ksize - usize);
> > +
> > + if (unlikely(usize > PAGE_SIZE))
> > + return -EFAULT;
>
> Why?
Because every caller of that function right now has that limit set
anyway iirc. So we can either remove it from here and place it back for
the individual callers or leave it in the helper.
Also, I'm really asking, why not? Is it unreasonable to have an upper
bound on the size (for a long time probably) or are you disagreeing with
PAGE_SIZE being used? PAGE_SIZE limit is currently used by sched, perf,
bpf, and clone3 and in a few other places.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RESEND v3 1/1] i2c: qcom-geni: Provide an option to disable DMA processing
From: Wolfram Sang @ 2019-09-05 18:20 UTC (permalink / raw)
To: Lee Jones
Cc: mark.rutland, devicetree, linux-kernel, agross, robh+dt,
bjorn.andersson, vkoul, alokc, linux-i2c, linux-arm-msm,
linux-arm-kernel
In-Reply-To: <20190905144122.5689-1-lee.jones@linaro.org>
[-- Attachment #1.1: Type: text/plain, Size: 1983 bytes --]
On Thu, Sep 05, 2019 at 03:41:22PM +0100, Lee Jones wrote:
> We have a production-level laptop (Lenovo Yoga C630) which is exhibiting
> a rather horrific bug. When I2C HID devices are being scanned for at
> boot-time the QCom Geni based I2C (Serial Engine) attempts to use DMA.
> When it does, the laptop reboots and the user never sees the OS.
$subject is still wrong. And a paragraph that you are/were debugging the
root cause but to no avail so DMA gets disabled for now would be good for a
hotfix this late in the cycle.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
> drivers/i2c/busses/i2c-qcom-geni.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index a89bfce5388e..17abf60c94ae 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -355,11 +355,13 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
> {
> dma_addr_t rx_dma;
> unsigned long time_left;
> - void *dma_buf;
> + void *dma_buf = NULL;
> struct geni_se *se = &gi2c->se;
> size_t len = msg->len;
>
> - dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
> + if (!of_machine_is_compatible("lenovo,yoga-c630"))
> + dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
> +
> if (dma_buf)
> geni_se_select_mode(se, GENI_SE_DMA);
> else
> @@ -394,11 +396,13 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
> {
> dma_addr_t tx_dma;
> unsigned long time_left;
> - void *dma_buf;
> + void *dma_buf = NULL;
> struct geni_se *se = &gi2c->se;
> size_t len = msg->len;
>
> - dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
> + if (!of_machine_is_compatible("lenovo,yoga-c630"))
> + dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
> +
> if (dma_buf)
> geni_se_select_mode(se, GENI_SE_DMA);
> else
> --
> 2.17.1
>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v12 01/12] lib: introduce copy_struct_{to,from}_user helpers
From: Al Viro @ 2019-09-05 18:07 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, Jiri Olsa, linux-arch, linux-s390, Tycho Andersen,
Aleksa Sarai, Shuah Khan, Alexander Shishkin, Ingo Molnar,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k,
Andy Lutomirski, Shuah Khan, Namhyung Kim, David Drysdale,
Christian Brauner, J. Bruce Fields, linux-parisc, linux-api,
Chanho Min, Jeff Layton, Oleg Nesterov, Eric Biederman,
linux-alpha, linux-fsdevel, Andrew Morton, Linus Torvalds,
containers
In-Reply-To: <20190904201933.10736-2-cyphar@cyphar.com>
On Thu, Sep 05, 2019 at 06:19:22AM +1000, Aleksa Sarai wrote:
> +/*
> + * "memset(p, 0, size)" but for user space buffers. Caller must have already
> + * checked access_ok(p, size).
> + */
> +static int __memzero_user(void __user *p, size_t s)
> +{
> + const char zeros[BUFFER_SIZE] = {};
> + while (s > 0) {
> + size_t n = min(s, sizeof(zeros));
> +
> + if (__copy_to_user(p, zeros, n))
> + return -EFAULT;
> +
> + p += n;
> + s -= n;
> + }
> + return 0;
> +}
That's called clear_user().
> +int copy_struct_to_user(void __user *dst, size_t usize,
> + const void *src, size_t ksize)
> +{
> + size_t size = min(ksize, usize);
> + size_t rest = abs(ksize - usize);
> +
> + if (unlikely(usize > PAGE_SIZE))
> + return -EFAULT;
Why?
> + } else if (usize > ksize) {
> + if (__memzero_user(dst + size, rest))
> + return -EFAULT;
> + }
> + /* Copy the interoperable parts of the struct. */
> + if (__copy_to_user(dst, src, size))
> + return -EFAULT;
Why not simply clear_user() and copy_to_user()?
> +int copy_struct_from_user(void *dst, size_t ksize,
> + const void __user *src, size_t usize)
> +{
> + size_t size = min(ksize, usize);
> + size_t rest = abs(ksize - usize);
Cute, but... you would be just as well without that 'rest' thing.
> +
> + if (unlikely(usize > PAGE_SIZE))
> + return -EFAULT;
Again, why?
> + if (unlikely(!access_ok(src, usize)))
> + return -EFAULT;
Why not simply copy_from_user() here?
> + /* Deal with trailing bytes. */
> + if (usize < ksize)
> + memset(dst + size, 0, rest);
> + else if (usize > ksize) {
> + const void __user *addr = src + size;
> + char buffer[BUFFER_SIZE] = {};
> +
> + while (rest > 0) {
> + size_t bufsize = min(rest, sizeof(buffer));
> +
> + if (__copy_from_user(buffer, addr, bufsize))
> + return -EFAULT;
> + if (memchr_inv(buffer, 0, bufsize))
> + return -E2BIG;
Frankly, that looks like a candidate for is_all_zeroes_user().
With the loop like above serving as a dumb default. And on
badly alighed address it _will_ be dumb. Probably too much
so - something like
if ((unsigned long)addr & 1) {
u8 v;
if (get_user(v, (__u8 __user *)addr))
return -EFAULT;
if (v)
return -E2BIG;
addr++;
}
if ((unsigned long)addr & 2) {
u16 v;
if (get_user(v, (__u16 __user *)addr))
return -EFAULT;
if (v)
return -E2BIG;
addr +=2;
}
if ((unsigned long)addr & 4) {
u32 v;
if (get_user(v, (__u32 __user *)addr))
return -EFAULT;
if (v)
return -E2BIG;
}
<read the rest like you currently do>
would be saner, and things like x86 could trivially add an
asm variant - it's not hard. Incidentally, memchr_inv() is
an overkill in this case...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 3/4] dt-bindings: Add Qualcomm USB SuperSpeed PHY bindings
From: Jack Pham @ 2019-09-05 17:58 UTC (permalink / raw)
To: Jorge Ramirez
Cc: mark.rutland, robh, kishon, gregkh, linux-usb, khasim.mohammed,
linux-kernel, Stephen Boyd, devicetree, Bjorn Andersson,
linux-arm-msm, andy.gross, shawn.guo, linux-arm-kernel
In-Reply-To: <c9481b7d-4805-25c6-f40f-9cbfc40afc93@linaro.org>
Hi Jorge, Bjorn,
On Thu, Sep 05, 2019 at 09:18:57AM +0200, Jorge Ramirez wrote:
> On 9/4/19 01:34, Bjorn Andersson wrote:
> > On Tue 03 Sep 14:45 PDT 2019, Stephen Boyd wrote:
> >
> >> Quoting Jack Pham (2019-09-03 10:39:24)
> >>> On Mon, Sep 02, 2019 at 08:23:04AM +0200, Jorge Ramirez wrote:
> >>>> On 8/30/19 20:28, Stephen Boyd wrote:
> >>>>> Quoting Bjorn Andersson (2019-08-30 09:45:20)
> >>>>>> On Fri 30 Aug 09:01 PDT 2019, Stephen Boyd wrote:
> >>>>>>
> >>>>>>>>>
> >>>>>>>>> The USB-C connector is attached both to the HS and SS PHYs, so I think
> >>>>>>>>> you should represent this external to this node and use of_graph to
> >>>>>>>>> query it.
> >>>>>>>>
> >>>>>>>> but AFAICS we wont be able to retrieve the vbux-supply from an external
> >>>>>>>> node (that interface does not exist).
> >>>>>>>>
> >>>>>>>> rob, do you have a suggestion?
> >>>>>>>
> >>>>>>> Shouldn't the vbus supply be in the phy? Or is this a situation where
> >>>>>>> the phy itself doesn't have the vbus supply going to it because the PMIC
> >>>>>>> gets in the way and handles the vbus for the connector by having the SoC
> >>>>>>> communicate with the PMIC about when to turn the vbus on and off, etc?
> >>>>>>>
> >>>>>>
> >>>>>> That's correct, the VBUS comes out of the PMIC and goes directly to the
> >>>>>> connector.
> >>>>>>
> >>>>>> The additional complicating factor here is that the connector is wired
> >>>>>> to a USB2 phy as well, so we need to wire up detection and vbus control
> >>>>>> to both of them - but I think this will be fine, if we can only figure
> >>>>>> out a sane way of getting hold of the vbus-supply.
> >>>>>>
> >>>>>
> >>>>> Does it really matter to describe this situation though? Maybe it's
> >>>>> simpler to throw the vbus supply into the phy and control it from the
> >>>>> phy driver, even if it never really goes there. Or put it into the
> >>>>> toplevel usb controller?
> >>>>>
> >>>> that would work for me - the connector definition seemed a better way to
> >>>> explain the connectivity but since we cant retrieve the supply from the
> >>>> external node is not of much functional use.
> >>>>
> >>>> but please let me know how to proceed. shall I add the supply back to
> >>>> the phy?
> >>
> >> So does the vbus actually go to the phy? I thought it never went there
> >> and the power for the phy was different (and possibly lower in voltage).
> >>
> >
> > No, the PHYs use different - lower voltage - supplies to operate. VBUS
> > is coming from a 5V supply straight to the connector and plug-detect
> > logic (which is passive in this design).
> >
> >>>
> >>> Putting it in the toplevel usb node makes sense to me, since that's
> >>> usually the driver that knows when it's switching into host mode and
> >>> needs to turn on VBUS. The dwc3-qcom driver & bindings currently don't
> >>> do this but there's precedent in a couple of the other dwc3 "glues"--see
> >>> Documentation/devicetree/bindings/usb/{amlogic\,dwc3,omap-usb}.txt
> >>>
> >>> One exception is if the PMIC is also USB-PD capable and can do power
> >>> role swap, in which case the VBUS control needs to be done by the TCPM,
> >>> so that'd be a case where having vbus-supply in the connector node might
> >>> make more sense.
> >>>
> >>
> >> The other way is to implement the code to get the vbus supply out of a
> >> connector. Then any driver can do the work if it knows it needs to and
> >> we don't have to care that the vbus isn't going somewhere. I suppose
> >> that would need an of_regulator_get() sort of API that can get the
> >> regulator out of there? Or to make the connector into a struct device
> >> that can get the regulator out per some generic connector driver and
> >> then pass it through to the USB controller when it asks for it. Maybe
> >> try to prototype that out?
> >>
> >
> > The examples given in the DT bindings describes the connector as a child
> > of a PMIC, with of_graph somehow tying it to the various inputs. But in
> > these examples vbus is handled by implicitly inside the MFD, where
> > extcon is informed about the plug event they toggle vbus as well.
> >
> > In our case we have a extcon-usb-gpio to detect mode, which per Jorge's
> > proposal will trickle down to the PHY and become a regulator calls on
> > either some external regulator or more typically one of the chargers in
> > the system.
Interesting you mention extcon-usb-gpio. I thought extcon at least from
bindings perspective is passé now. Maybe this is what you need (just
landed in usb-next):
usb: common: add USB GPIO based connection detection driver
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=4602f3bff2669012c1147eecfe74c121765f5c56
dt-bindings: usb: add binding for USB GPIO based connection detection driver
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=f651c73e71f53f65e9846677d79d8e120452b59f
Fortunately this new driver might check the right boxes for you:
- usb connector binding
- ID detect GPIO
- vbus-supply regulator
With that, I think you can also keep the connector subnode out of the
SSPHY node well, and similarly get rid of the vbus toggle handling from
the PHY driver.
The big thing missing now is that this driver replaces extcon
completely, so we'll need handling in dwc3/dwc3-qcom to retrieve the
role switch state to know when host mode is entered. I saw this a while
back but don't think it got picked up:
https://patchwork.kernel.org/patch/10909981/
> > So if we come up with a struct device for the connector and some API for
> > toggling the vbus we're going to have to fairly abstract entities
> > representing pretty much the same thing - and in a design with a mux we
> > would have a different setup.
>
> I am a bit unclear - not sure if we have gone full circle on this
> subject. what is then the direction to get this merged?
>
> I did have look last week and the level of effort to support regulators
> on external nodes is not neglectable meaning that I might not have the
> time to deliver that feature (perhaps someone else wishes to take over?)
>
> >
> > Regards,
> > Bjorn
> >
>
Jack
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 01/11] kselftest: arm64: add skeleton Makefile
From: Cristian Marussi @ 2019-09-05 17:57 UTC (permalink / raw)
To: Amit Kachhap, linux-kselftest@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, shuah@kernel.org
Cc: andreyknvl@google.com, Dave P Martin
In-Reply-To: <cce97298-7a27-c470-6fc5-873b4447ecc9@arm.com>
Hi Amit
On 03/09/2019 10:26, Amit Kachhap wrote:
>
> Hi Cristian,
>
> On 9/2/19 4:59 PM, Cristian Marussi wrote:
>> Add a new arm64-specific empty subsystem amongst TARGETS of KSFT build
>> framework; keep these new arm64 KSFT testcases separated into distinct
>> subdirs inside tools/testing/selftests/arm64/ depending on the specific
>> subsystem targeted.
>>
>> Add into toplevel arm64 KSFT Makefile a mechanism to guess the effective
>> location of Kernel headers as installed by KSFT framework.
>>
>> Merge with
>>
>> commit 9ce1263033cd ("selftests, arm64: add a selftest for passing
>> tagged pointers to kernel")
>>
>> while moving such KSFT tags tests inside their own subdirectory
>> (arm64/tags).
>>
>> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
>> ---
>> v4 --> v5
>> - rebased on arm64/for-next/core
>> - merged this patch with KSFT arm64 tags patch, while moving the latter
>> into its own subdir
>> - moved kernel header includes search mechanism from KSFT arm64
>> SIGNAL Makefile
> This approach breaks the compilation of individual test cases which need
> to export includes individually.
>
> make -C tools/testing/selftests/arm64/signal
>
> ../../lib.mk:25: ../../../../scripts/subarch.include: No such file or
> directory
> Makefile:25: warning: overriding recipe for target 'clean'
> ../../lib.mk:123: warning: ignoring old recipe for target 'clean'
> make: *** No rule to make target '../../../../scripts/subarch.include'.
> Stop.
>
> However tags test works well,
> make -C tools/testing/selftests/arm64/tags
>
> aarch64-none-linux-gnu-gcc tags_test.c -o
> /home/amikac01/work/MTE_WORK/linux-server/linux/tools/testing/selftests/arm64/tags/tags_test
>
>
> Thanks,
> Amit Daniel
>
So at the end I think I'll opt for the following in V6 regarding the issue of being able to build specific
KSFT arm64 subsystems while properly searching kernel headers (and keeping compatible with the KSFT
framework completely):
- only arm64 toplevel KSFT Makefile searches for the kernel headers location for all and propagates down the info
- you can also now optionally specify which arm64 subsystem to build (to avoid have to build, say, all of signal/
if you are not interested into....a sort of standalone mode without all the burden of the old standalone mode)
So you can issue:
$ make TARGETS=arm64 kselftest
or similarly:
$ make -C tools/testing/selftests TARGETS=arm64 \
INSTALL_PATH=<your-installation-path> install
or select subsystems:
$ make -C tools/testing/selftests TARGETS=arm64 SUBTARGETS="tags signal" \
INSTALL_PATH=<your-installation-path> install
with all of the above looking for the K headers in the proper place and without
duplicating the search code in multiple places. (bugs apart :D)
Thanks
Cristian
>> - export proper top_srcdir ENV for lib.mk
>> v3 --> v4
>> - comment reword
>> - simplified documentation in README
>> - dropped README about standalone
>> ---
>> tools/testing/selftests/Makefile | 1 +
>> tools/testing/selftests/arm64/Makefile | 70 +++++++++++++++++--
>> tools/testing/selftests/arm64/README | 20 ++++++
>> tools/testing/selftests/arm64/tags/Makefile | 10 +++
>> .../arm64/{ => tags}/run_tags_test.sh | 0
>> .../selftests/arm64/{ => tags}/tags_test.c | 0
>> 6 files changed, 95 insertions(+), 6 deletions(-)
>> create mode 100644 tools/testing/selftests/arm64/README
>> create mode 100644 tools/testing/selftests/arm64/tags/Makefile
>> rename tools/testing/selftests/arm64/{ => tags}/run_tags_test.sh (100%)
>> rename tools/testing/selftests/arm64/{ => tags}/tags_test.c (100%)
>>
>> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
>> index 25b43a8c2b15..1722dae9381a 100644
>> --- a/tools/testing/selftests/Makefile
>> +++ b/tools/testing/selftests/Makefile
>> @@ -1,5 +1,6 @@
>> # SPDX-License-Identifier: GPL-2.0
>> TARGETS = android
>> +TARGETS += arm64
>> TARGETS += bpf
>> TARGETS += breakpoints
>> TARGETS += capabilities
>> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
>> index a61b2e743e99..5dbb0ffdfc9a 100644
>> --- a/tools/testing/selftests/arm64/Makefile
>> +++ b/tools/testing/selftests/arm64/Makefile
>> @@ -1,11 +1,69 @@
>> # SPDX-License-Identifier: GPL-2.0
>> +# Copyright (C) 2019 ARM Limited
>>
>> -# ARCH can be overridden by the user for cross compiling
>> -ARCH ?= $(shell uname -m 2>/dev/null || echo not)
>> +# When ARCH not overridden for crosscompiling, lookup machine
>> +ARCH ?= $(shell uname -m)
>> +ARCH := $(shell echo $(ARCH) | sed -e s/aarch64/arm64/)
>>
>> -ifneq (,$(filter $(ARCH),aarch64 arm64))
>> -TEST_GEN_PROGS := tags_test
>> -TEST_PROGS := run_tags_test.sh
>> +ifeq ("x$(ARCH)", "xarm64")
>> +SUBDIRS := tags
>> +else
>> +SUBDIRS :=
>> endif
>>
>> -include ../lib.mk
>> +CFLAGS := -Wall -O2 -g
>> +
>> +# A proper top_srcdir is needed by KSFT(lib.mk)
>> +top_srcdir = ../../../../..
>> +
>> +# Additional include paths needed by kselftest.h and local headers
>> +CFLAGS += -I$(top_srcdir)/tools/testing/selftests/
>> +
>> +# Guessing where the Kernel headers could have been installed
>> +# depending on ENV config
>> +ifeq ($(KBUILD_OUTPUT),)
>> +khdr_dir = $(top_srcdir)/usr/include
>> +else
>> +# the KSFT preferred location when KBUILD_OUTPUT is set
>> +khdr_dir = $(KBUILD_OUTPUT)/kselftest/usr/include
>> +endif
>> +
>> +CFLAGS += -I$(khdr_dir)
>> +
>> +export CC
>> +export CFLAGS
>> +export top_srcdir
>> +
>> +all:
>> + @for DIR in $(SUBDIRS); do \
>> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
>> + mkdir -p $$BUILD_TARGET; \
>> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
>> + done
>> +
>> +install: all
>> + @for DIR in $(SUBDIRS); do \
>> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
>> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
>> + done
>> +
>> +run_tests: all
>> + @for DIR in $(SUBDIRS); do \
>> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
>> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
>> + done
>> +
>> +# Avoid any output on non arm64 on emit_tests
>> +emit_tests: all
>> + @for DIR in $(SUBDIRS); do \
>> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
>> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
>> + done
>> +
>> +clean:
>> + @for DIR in $(SUBDIRS); do \
>> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
>> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
>> + done
>> +
>> +.PHONY: all clean install run_tests emit_tests
>> diff --git a/tools/testing/selftests/arm64/README b/tools/testing/selftests/arm64/README
>> new file mode 100644
>> index 000000000000..aca892e62a6c
>> --- /dev/null
>> +++ b/tools/testing/selftests/arm64/README
>> @@ -0,0 +1,20 @@
>> +KSelfTest ARM64
>> +===============
>> +
>> +- These tests are arm64 specific and so not built or run but just skipped
>> + completely when env-variable ARCH is found to be different than 'arm64'
>> + and `uname -m` reports other than 'aarch64'.
>> +
>> +- Holding true the above, ARM64 KSFT tests can be run within the KSelfTest
>> + framework using standard Linux top-level-makefile targets:
>> +
>> + $ make TARGETS=arm64 kselftest-clean
>> + $ make TARGETS=arm64 kselftest
>> +
>> + or
>> +
>> + $ make -C tools/testing/selftests TARGETS=arm64 \
>> + INSTALL_PATH=<your-installation-path> install
>> +
>> + Further details on building and running KFST can be found in:
>> + Documentation/dev-tools/kselftest.rst
>> diff --git a/tools/testing/selftests/arm64/tags/Makefile b/tools/testing/selftests/arm64/tags/Makefile
>> new file mode 100644
>> index 000000000000..76205533135b
>> --- /dev/null
>> +++ b/tools/testing/selftests/arm64/tags/Makefile
>> @@ -0,0 +1,10 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# ARCH can be overridden by the user for cross compiling
>> +ARCH ?= $(shell uname -m 2>/dev/null || echo not)
>> +
>> +ifneq (,$(filter $(ARCH),aarch64 arm64))
>> +TEST_GEN_PROGS := tags_test
>> +TEST_PROGS := run_tags_test.sh
>> +endif
>> +
>> +include ../../lib.mk
>> diff --git a/tools/testing/selftests/arm64/run_tags_test.sh b/tools/testing/selftests/arm64/tags/run_tags_test.sh
>> similarity index 100%
>> rename from tools/testing/selftests/arm64/run_tags_test.sh
>> rename to tools/testing/selftests/arm64/tags/run_tags_test.sh
>> diff --git a/tools/testing/selftests/arm64/tags_test.c b/tools/testing/selftests/arm64/tags/tags_test.c
>> similarity index 100%
>> rename from tools/testing/selftests/arm64/tags_test.c
>> rename to tools/testing/selftests/arm64/tags/tags_test.c
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Applied "ASoC: SOF: imx8: Fix COMPILE_TEST error" to the asoc tree
From: Mark Brown @ 2019-09-05 17:38 UTC (permalink / raw)
To: YueHaibing
Cc: pierre-louis.bossart, alsa-devel, daniel.baluta, s.hauer, tiwai,
yuehaibing, lgirdwood, linux-kernel, Hulk Robot, Mark Brown,
linux-imx, kernel, shawnguo, perex, festevam, linux-arm-kernel
In-Reply-To: <20190905064400.24800-1-yuehaibing@huawei.com>
The patch
ASoC: SOF: imx8: Fix COMPILE_TEST error
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From f4df4e4042b045c6ddbaff878a17ae169fe68ba6 Mon Sep 17 00:00:00 2001
From: YueHaibing <yuehaibing@huawei.com>
Date: Thu, 5 Sep 2019 14:44:00 +0800
Subject: [PATCH] ASoC: SOF: imx8: Fix COMPILE_TEST error
When do compile test, if SND_SOC_SOF_OF is not set, we get:
sound/soc/sof/imx/imx8.o: In function `imx8_dsp_handle_request':
imx8.c:(.text+0xb0): undefined reference to `snd_sof_ipc_msgs_rx'
sound/soc/sof/imx/imx8.o: In function `imx8_ipc_msg_data':
imx8.c:(.text+0xf4): undefined reference to `sof_mailbox_read'
sound/soc/sof/imx/imx8.o: In function `imx8_dsp_handle_reply':
imx8.c:(.text+0x160): undefined reference to `sof_mailbox_read'
Make SND_SOC_SOF_IMX_TOPLEVEL always depends on SND_SOC_SOF_OF
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: 202acc565a1f ("ASoC: SOF: imx: Add i.MX8 HW support")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20190905064400.24800-1-yuehaibing@huawei.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/sof/imx/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig
index fd73d8402dbf..5acae75f5750 100644
--- a/sound/soc/sof/imx/Kconfig
+++ b/sound/soc/sof/imx/Kconfig
@@ -2,7 +2,8 @@
config SND_SOC_SOF_IMX_TOPLEVEL
bool "SOF support for NXP i.MX audio DSPs"
- depends on ARM64 && SND_SOC_SOF_OF || COMPILE_TEST
+ depends on ARM64|| COMPILE_TEST
+ depends on SND_SOC_SOF_OF
help
This adds support for Sound Open Firmware for NXP i.MX platforms.
Say Y if you have such a device.
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v3 3/4] arm64: use both ZONE_DMA and ZONE_DMA32
From: Nicolas Saenz Julienne @ 2019-09-05 17:28 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-rpi-kernel, f.fainelli, marc.zyngier, Will Deacon,
linux-kernel, robh+dt, linux-mm, mbrugger, wahrenst, linux-riscv,
phill, robin.murphy, hch, linux-arm-kernel, m.szyprowski
In-Reply-To: <20190905171939.GF31268@arrakis.emea.arm.com>
[-- Attachment #1.1: Type: text/plain, Size: 3644 bytes --]
On Thu, 2019-09-05 at 18:19 +0100, Catalin Marinas wrote:
> On Mon, Sep 02, 2019 at 04:10:41PM +0200, Nicolas Saenz Julienne wrote:
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index 8956c22634dd..f02a4945aeac 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -50,6 +50,13 @@
> > s64 memstart_addr __ro_after_init = -1;
> > EXPORT_SYMBOL(memstart_addr);
> >
> > +/*
> > + * We create both ZONE_DMA and ZONE_DMA32. ZONE_DMA covers the first 1G of
> > + * memory as some devices, namely the Raspberry Pi 4, have peripherals with
> > + * this limited view of the memory. ZONE_DMA32 will cover the rest of the
> > 32
> > + * bit addressable memory area.
> > + */
> > +phys_addr_t arm64_dma_phys_limit __ro_after_init;
> > phys_addr_t arm64_dma32_phys_limit __ro_after_init;
> >
> > #ifdef CONFIG_KEXEC_CORE
> > @@ -164,9 +171,9 @@ static void __init reserve_elfcorehdr(void)
> > }
> > #endif /* CONFIG_CRASH_DUMP */
> > /*
> > - * Return the maximum physical address for ZONE_DMA32 (DMA_BIT_MASK(32)).
> > It
> > - * currently assumes that for memory starting above 4G, 32-bit devices will
> > - * use a DMA offset.
> > + * Return the maximum physical address for ZONE_DMA32 (DMA_BIT_MASK(32))
> > and
> > + * ZONE_DMA (DMA_BIT_MASK(30)) respectively. It currently assumes that for
> > + * memory starting above 4G, 32-bit devices will use a DMA offset.
> > */
> > static phys_addr_t __init max_zone_dma32_phys(void)
> > {
> > @@ -174,12 +181,23 @@ static phys_addr_t __init max_zone_dma32_phys(void)
> > return min(offset + (1ULL << 32), memblock_end_of_DRAM());
> > }
> >
> > +static phys_addr_t __init max_zone_dma_phys(void)
> > +{
> > + phys_addr_t offset = memblock_start_of_DRAM() & GENMASK_ULL(63, 32);
> > +
> > + return min(offset + (1ULL << ARCH_ZONE_DMA_BITS),
> > + memblock_end_of_DRAM());
> > +}
>
> I think we could squash these two functions into a single one with a
> "bits" argument that is either 32 or ARCH_ZONE_DMA_BITS.
Hi Catalin, thanks for the review.
Agree, it'll look nicer.
> > +
> > #ifdef CONFIG_NUMA
> >
> > static void __init zone_sizes_init(unsigned long min, unsigned long max)
> > {
> > unsigned long max_zone_pfns[MAX_NR_ZONES] = {0};
> >
> > +#ifdef CONFIG_ZONE_DMA
> > + max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
> > +#endif
> > #ifdef CONFIG_ZONE_DMA32
> > max_zone_pfns[ZONE_DMA32] = PFN_DOWN(arm64_dma32_phys_limit);
> > #endif
> > @@ -195,13 +213,17 @@ static void __init zone_sizes_init(unsigned long min,
> > unsigned long max)
> > struct memblock_region *reg;
> > unsigned long zone_size[MAX_NR_ZONES], zhole_size[MAX_NR_ZONES];
> > unsigned long max_dma32 = min;
> > + unsigned long max_dma = min;
> >
> > memset(zone_size, 0, sizeof(zone_size));
> >
> > - /* 4GB maximum for 32-bit only capable devices */
> > +#ifdef CONFIG_ZONE_DMA
> > + max_dma = PFN_DOWN(arm64_dma_phys_limit);
> > + zone_size[ZONE_DMA] = max_dma - min;
> > +#endifmax_dma32
> > #ifdef CONFIG_ZONE_DMA32
> > max_dma32 = PFN_DOWN(arm64_dma32_phys_limit);
> > - zone_size[ZONE_DMA32] = max_dma32 - min;
> > + zone_size[ZONE_DMA32] = max_dma32 - max_dma;
> > #endif
> > zone_size[ZONE_NORMAL] = max - max_dma32;
>
> Does this still work if we have ZONE_DMA32 disabled but ZONE_DMA
> enabled? You could use a max(max_dma32, max_dma) or just update
> max_dma32 to max_dma in the CONFIG_ZONE_DMA block.
You're right, I missed that scenario. I'll fix it and give it a test for the
next series.
Regards,
Nicolas
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 4/4] mm: refresh ZONE_DMA and ZONE_DMA32 comments in 'enum zone_type'
From: Catalin Marinas @ 2019-09-05 17:21 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: Albert Ou, mbrugger, marc.zyngier, robin.murphy, Palmer Dabbelt,
linux-kernel, linux-mm, robh+dt, wahrenst, Paul Walmsley,
m.szyprowski, f.fainelli, linux-riscv, phill, will, hch,
linux-arm-kernel, linux-rpi-kernel
In-Reply-To: <20190902141043.27210-5-nsaenzjulienne@suse.de>
On Mon, Sep 02, 2019 at 04:10:42PM +0200, Nicolas Saenz Julienne wrote:
> These zones usage has evolved with time and the comments were outdated.
> This joins both ZONE_DMA and ZONE_DMA32 explanation and gives up to date
> examples on how they are used on different architectures.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 3/4] arm64: use both ZONE_DMA and ZONE_DMA32
From: Catalin Marinas @ 2019-09-05 17:19 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: f.fainelli, mbrugger, marc.zyngier, robin.murphy, linux-kernel,
linux-mm, robh+dt, wahrenst, m.szyprowski, linux-riscv, phill,
Will Deacon, hch, linux-arm-kernel, linux-rpi-kernel
In-Reply-To: <20190902141043.27210-4-nsaenzjulienne@suse.de>
On Mon, Sep 02, 2019 at 04:10:41PM +0200, Nicolas Saenz Julienne wrote:
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 8956c22634dd..f02a4945aeac 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -50,6 +50,13 @@
> s64 memstart_addr __ro_after_init = -1;
> EXPORT_SYMBOL(memstart_addr);
>
> +/*
> + * We create both ZONE_DMA and ZONE_DMA32. ZONE_DMA covers the first 1G of
> + * memory as some devices, namely the Raspberry Pi 4, have peripherals with
> + * this limited view of the memory. ZONE_DMA32 will cover the rest of the 32
> + * bit addressable memory area.
> + */
> +phys_addr_t arm64_dma_phys_limit __ro_after_init;
> phys_addr_t arm64_dma32_phys_limit __ro_after_init;
>
> #ifdef CONFIG_KEXEC_CORE
> @@ -164,9 +171,9 @@ static void __init reserve_elfcorehdr(void)
> }
> #endif /* CONFIG_CRASH_DUMP */
> /*
> - * Return the maximum physical address for ZONE_DMA32 (DMA_BIT_MASK(32)). It
> - * currently assumes that for memory starting above 4G, 32-bit devices will
> - * use a DMA offset.
> + * Return the maximum physical address for ZONE_DMA32 (DMA_BIT_MASK(32)) and
> + * ZONE_DMA (DMA_BIT_MASK(30)) respectively. It currently assumes that for
> + * memory starting above 4G, 32-bit devices will use a DMA offset.
> */
> static phys_addr_t __init max_zone_dma32_phys(void)
> {
> @@ -174,12 +181,23 @@ static phys_addr_t __init max_zone_dma32_phys(void)
> return min(offset + (1ULL << 32), memblock_end_of_DRAM());
> }
>
> +static phys_addr_t __init max_zone_dma_phys(void)
> +{
> + phys_addr_t offset = memblock_start_of_DRAM() & GENMASK_ULL(63, 32);
> +
> + return min(offset + (1ULL << ARCH_ZONE_DMA_BITS),
> + memblock_end_of_DRAM());
> +}
I think we could squash these two functions into a single one with a
"bits" argument that is either 32 or ARCH_ZONE_DMA_BITS.
> +
> #ifdef CONFIG_NUMA
>
> static void __init zone_sizes_init(unsigned long min, unsigned long max)
> {
> unsigned long max_zone_pfns[MAX_NR_ZONES] = {0};
>
> +#ifdef CONFIG_ZONE_DMA
> + max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
> +#endif
> #ifdef CONFIG_ZONE_DMA32
> max_zone_pfns[ZONE_DMA32] = PFN_DOWN(arm64_dma32_phys_limit);
> #endif
> @@ -195,13 +213,17 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
> struct memblock_region *reg;
> unsigned long zone_size[MAX_NR_ZONES], zhole_size[MAX_NR_ZONES];
> unsigned long max_dma32 = min;
> + unsigned long max_dma = min;
>
> memset(zone_size, 0, sizeof(zone_size));
>
> - /* 4GB maximum for 32-bit only capable devices */
> +#ifdef CONFIG_ZONE_DMA
> + max_dma = PFN_DOWN(arm64_dma_phys_limit);
> + zone_size[ZONE_DMA] = max_dma - min;
> +#endif
> #ifdef CONFIG_ZONE_DMA32
> max_dma32 = PFN_DOWN(arm64_dma32_phys_limit);
> - zone_size[ZONE_DMA32] = max_dma32 - min;
> + zone_size[ZONE_DMA32] = max_dma32 - max_dma;
> #endif
> zone_size[ZONE_NORMAL] = max - max_dma32;
Does this still work if we have ZONE_DMA32 disabled but ZONE_DMA
enabled? You could use a max(max_dma32, max_dma) or just update
max_dma32 to max_dma in the CONFIG_ZONE_DMA block.
> @@ -213,11 +235,17 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
>
> if (start >= max)
> continue;
> -
> +#ifdef CONFIG_ZONE_DMA
> + if (start < max_dma) {
> + unsigned long dma_end = min_not_zero(end, max_dma);
> + zhole_size[ZONE_DMA] -= dma_end - start;
> + }
> +#endif
> #ifdef CONFIG_ZONE_DMA32
> if (start < max_dma32) {
> - unsigned long dma_end = min(end, max_dma32);
> - zhole_size[ZONE_DMA32] -= dma_end - start;
> + unsigned long dma32_end = min(end, max_dma32);
> + unsigned long dma32_start = max(start, max_dma);
> + zhole_size[ZONE_DMA32] -= dma32_end - dma32_start;
> }
> #endif
> if (end > max_dma32) {
Similar comment here.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/1] mm/pgtable/debug: Add test validating architecture page table helpers
From: Gerald Schaefer @ 2019-09-05 17:06 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, linux-mm, Dave Hansen,
Paul Mackerras, sparclinux, Thomas Gleixner, linux-s390,
Michael Ellerman, x86, Russell King - ARM Linux, Matthew Wilcox,
Steven Price, Jason Gunthorpe, linux-arm-kernel, linux-snps-arc,
Kees Cook, Masahiro Yamada, Mark Brown, Dan Williams,
Vlastimil Babka, Sri Krishna chowdary, Ard Biesheuvel,
Greg Kroah-Hartman, linux-mips, Ralf Baechle, linux-kernel,
Paul Burton, Mike Rapoport, Vineet Gupta, Martin Schwidefsky,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20e3044d-2af5-b27b-7653-cec53bdec941@arm.com>
On Thu, 5 Sep 2019 14:48:14 +0530
Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> > [...]
> >> +
> >> +#if !defined(__PAGETABLE_PMD_FOLDED) && !defined(__ARCH_HAS_4LEVEL_HACK)
> >> +static void pud_clear_tests(pud_t *pudp)
> >> +{
> >> + memset(pudp, RANDOM_NZVALUE, sizeof(pud_t));
> >> + pud_clear(pudp);
> >> + WARN_ON(!pud_none(READ_ONCE(*pudp)));
> >> +}
> >
> > For pgd/p4d/pud_clear(), we only clear if the page table level is present
> > and not folded. The memset() here overwrites the table type bits, so
> > pud_clear() will not clear anything on s390 and the pud_none() check will
> > fail.
> > Would it be possible to OR a (larger) random value into the table, so that
> > the lower 12 bits would be preserved?
>
> So the suggestion is instead of doing memset() on entry with RANDOM_NZVALUE,
> it should OR a large random value preserving lower 12 bits. Hmm, this should
> still do the trick for other platforms, they just need non zero value. So on
> s390, the lower 12 bits on the page table entry already has valid value while
> entering this function which would make sure that pud_clear() really does
> clear the entry ?
Yes, in theory the table entry on s390 would have the type set in the last
4 bits, so preserving those would be enough. If it does not conflict with
others, I would still suggest preserving all 12 bits since those would contain
arch-specific flags in general, just to be sure. For s390, the pte/pmd tests
would also work with the memset, but for consistency I think the same logic
should be used in all pxd_clear_tests.
However, there is another issue on s390 which will make this only work
for pud_clear_tests(), and not for the p4d/pgd_tests. The problem is that
mm_alloc() will only give you a 3-level page table initially on s390.
This means that pudp == p4dp == pgdp, and so the p4d/pgd_tests will
both see the pud level (of course this also affects other tests).
Not sure yet how to fix this, i.e. how to initialize/update the page table
to 5 levels. We can handle 5 level page tables, and it would be good if
all levels could be tested, but using mm_alloc() to establish the page
tables might not work on s390. One option could be to provide an arch-hook
or weak function to allocate/initialize the mm.
IIUC, the (dummy) mm is really only needed to provide an mm->pgd as starting
point, right?
Regards,
Gerald
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 2/4] arm64: rename variables used to calculate ZONE_DMA32's size
From: Catalin Marinas @ 2019-09-05 17:05 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: f.fainelli, mbrugger, marc.zyngier, robin.murphy, linux-kernel,
linux-mm, robh+dt, wahrenst, m.szyprowski, linux-riscv, phill,
will, hch, linux-arm-kernel, linux-rpi-kernel
In-Reply-To: <20190902141043.27210-3-nsaenzjulienne@suse.de>
On Mon, Sep 02, 2019 at 04:10:40PM +0200, Nicolas Saenz Julienne wrote:
> Let the name indicate that they are used to calculate ZONE_DMA32's size
> as opposed to ZONE_DMA.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 1/4] arm64: mm: use arm64_dma_phys_limit instead of calling max_zone_dma_phys()
From: Catalin Marinas @ 2019-09-05 17:05 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: f.fainelli, mbrugger, marc.zyngier, robin.murphy, linux-kernel,
linux-mm, robh+dt, wahrenst, m.szyprowski, linux-riscv, phill,
will, hch, linux-arm-kernel, linux-rpi-kernel
In-Reply-To: <20190902141043.27210-2-nsaenzjulienne@suse.de>
On Mon, Sep 02, 2019 at 04:10:39PM +0200, Nicolas Saenz Julienne wrote:
> By the time we call zones_sizes_init() arm64_dma_phys_limit already
> contains the result of max_zone_dma_phys(). We use the variable instead
> of calling the function directly to save some precious cpu time.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: next/master boot: 310 boots: 11 failed, 292 passed with 6 offline, 1 untried/unknown (next-20190904)
From: Catalin Marinas @ 2019-09-05 17:02 UTC (permalink / raw)
To: Mark Brown
Cc: Steven Liu, kernel-build-reports, Kevin Hilman, linux-mediatek,
Matthias Brugger, Will Deacon, Christoph Hellwig,
linux-arm-kernel
In-Reply-To: <20190905153938.GB4053@sirena.co.uk>
On Thu, Sep 05, 2019 at 04:39:38PM +0100, Mark Brown wrote:
> On Wed, Sep 04, 2019 at 05:15:53PM -0700, Kevin Hilman wrote:
> > [ + Steven Liu who donated this board to my kernelCI lab ]
>
> Also adding Christoph since this was bisected to his commit and Catalin
> and Will since this was an architecture change.
Thanks.
> > Bisected down to this commit[1], full bisect log here[2]. It didn't
> > revert cleanly on top of next-20190904, so I didn't get any further.
>
> > [1]
> > 419e2f1838819e954071dfa1d1f820ab3386ada1 is the first bad commit
> > commit 419e2f1838819e954071dfa1d1f820ab3386ada1
> > Author: Christoph Hellwig <hch@lst.de>
> > Date: Mon Aug 26 09:03:44 2019 +0200
I stared at this commit and the ones around it for a while but couldn't
see anything obvious. AFAICT this commit only replaces some macros, no
functional change.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v12 01/12] lib: introduce copy_struct_{to,from}_user helpers
From: Aleksa Sarai @ 2019-09-05 17:01 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-ia64, linux-sh, Alexander Shishkin, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, Jiri Olsa, linux-arch, linux-s390, Tycho Andersen,
Aleksa Sarai, Shuah Khan, Ingo Molnar, linux-arm-kernel,
linux-mips, linux-xtensa, Kees Cook, Arnd Bergmann, Jann Horn,
linuxppc-dev, linux-m68k, Al Viro, Andy Lutomirski, Shuah Khan,
Namhyung Kim, David Drysdale, Christian Brauner, J. Bruce Fields,
linux-parisc, linux-api, Chanho Min, Jeff Layton, Oleg Nesterov,
Eric Biederman, linux-alpha, linux-fsdevel, Andrew Morton,
Linus Torvalds, containers
In-Reply-To: <20190905094305.GJ2349@hirez.programming.kicks-ass.net>
[-- Attachment #1.1: Type: text/plain, Size: 1727 bytes --]
On 2019-09-05, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Sep 05, 2019 at 07:26:22PM +1000, Aleksa Sarai wrote:
> > On 2019-09-05, Peter Zijlstra <peterz@infradead.org> wrote:
> > > On Thu, Sep 05, 2019 at 06:19:22AM +1000, Aleksa Sarai wrote:
> > > > +
> > > > + while (rest > 0) {
> > > > + size_t bufsize = min(rest, sizeof(buffer));
> > > > +
> > > > + if (__copy_from_user(buffer, addr, bufsize))
> > > > + return -EFAULT;
> > > > + if (memchr_inv(buffer, 0, bufsize))
> > > > + return -E2BIG;
> > > > +
> > > > + addr += bufsize;
> > > > + rest -= bufsize;
> > > > + }
> > >
> > > The perf implementation uses get_user(); but if that is too slow, surely
> > > we can do something with uaccess_try() here?
> >
> > Is there a non-x86-specific way to do that (unless I'm mistaken only x86
> > has uaccess_try() or the other *_try() wrappers)? The main "performance
> > improvement" (if you can even call it that) is that we use memchr_inv()
> > which finds non-matching characters more efficiently than just doing a
> > loop.
>
> Oh, you're right, that's x86 only :/
Though, I just had an idea -- am I wrong to think that the following
would work just as well (without the need for an intermediate buffer)?
if (memchr_inv((const char __force *) src + size, 0, rest))
return -E2BIG;
Or is this type of thing very much frowned upon? What if it was a
separate memchr_inv_user() instead -- I feel as though there's not a
strong argument for needing to use a buffer when we're single-passing
the __user buffer and doing a basic boolean check.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] pinctrl: bcm: remove redundant assignment to pointer log
From: Ray Jui @ 2019-09-05 16:46 UTC (permalink / raw)
To: Colin King, Linus Walleij, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, linux-gpio, linux-arm-kernel
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20190905140919.29283-1-colin.king@canonical.com>
On 9/5/19 7:09 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The pointer log is being initialized with a value that is never read
> and is being re-assigned a little later on. The assignment is
> redundant and hence can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/pinctrl/bcm/pinctrl-cygnus-mux.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/pinctrl/bcm/pinctrl-cygnus-mux.c b/drivers/pinctrl/bcm/pinctrl-cygnus-mux.c
> index 44df35942a43..dcab2204c60c 100644
> --- a/drivers/pinctrl/bcm/pinctrl-cygnus-mux.c
> +++ b/drivers/pinctrl/bcm/pinctrl-cygnus-mux.c
> @@ -923,7 +923,6 @@ static int cygnus_mux_log_init(struct cygnus_pinctrl *pinctrl)
> if (!pinctrl->mux_log)
> return -ENOMEM;
>
> - log = pinctrl->mux_log;
Yes, this indeed looks completely redundant.
> for (i = 0; i < CYGNUS_NUM_IOMUX_REGS; i++) {
> for (j = 0; j < CYGNUS_NUM_MUX_PER_REG; j++) {
> log = &pinctrl->mux_log[i * CYGNUS_NUM_MUX_PER_REG
>
Change looks good to me. Thanks!
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL] Second Round of Renesas ARM Based SoC Fixes for v5.3
From: Arnd Bergmann @ 2019-09-05 16:05 UTC (permalink / raw)
To: Simon Horman
Cc: Kevin Hilman, Magnus Damm, Linux-Renesas, SoC Team, arm-soc,
Olof Johansson, Linux ARM
In-Reply-To: <cover.1567675986.git.horms+renesas@verge.net.au>
On Thu, Sep 5, 2019 at 11:35 AM Simon Horman <horms+renesas@verge.net.au> wrote:
>
> Hi Olof, Hi Kevin, Hi Arnd,
>
> Please consider these second round of Renesas ARM based SoC fixes for v5.3.
>
> This pull request is based on the previous round of
> such requests, tagged as renesas-next-20190813-v5.3-rc1,
> which you have already pulled.
Pulled into fixes, thanks!
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [V3, 2/2] media: i2c: Add Omnivision OV02A10 camera sensor driver
From: Sakari Ailus @ 2019-09-05 16:05 UTC (permalink / raw)
To: Tomasz Figa
Cc: Mark Rutland, devicetree, Nicolas Boichat, srv_heupstream,
shengnan.wang, Louis Kuo, Sj Huang, Rob Herring,
moderated list:ARM/Mediatek SoC support, Dongchun Zhu,
Matthias Brugger, Cao Bing Bu, Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>, ,
Linux Media Mailing List
In-Reply-To: <CAAFQd5Bh-11D9RR9WVH5A3DbXZoxWhbMhXSNKUV25mempMi+ag@mail.gmail.com>
On Thu, Sep 05, 2019 at 07:53:37PM +0900, Tomasz Figa wrote:
> On Thu, Sep 5, 2019 at 7:45 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Dongchun,
> >
> > On Thu, Sep 05, 2019 at 05:41:05PM +0800, Dongchun Zhu wrote:
> >
> > ...
> >
> > > > > + ret = regulator_bulk_enable(OV02A10_NUM_SUPPLIES, ov02a10->supplies);
> > > > > + if (ret < 0) {
> > > > > + dev_err(dev, "Failed to enable regulators\n");
> > > > > + goto disable_clk;
> > > > > + }
> > > > > + msleep_range(7);
> > > >
> > > > This has some potential of clashing with more generic functions in the
> > > > future. Please use usleep_range directly, or msleep.
> > > >
> > >
> > > Did you mean using usleep_range(7*1000, 8*1000), as used in patch v1?
> > > https://patchwork.kernel.org/patch/10957225/
> >
> > Yes, please.
>
> Why not just msleep()?
msleep() is usually less accurate. I'm not sure it makes a big different in
this case. Perhaps, if someone wants that the sensor is powered on and
streaming as soon as possible.
--
Sakari Ailus
sakari.ailus@linux.intel.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2] KVM: arm/arm64: vgic: Allow more than 256 vcpus for KVM_IRQ_LINE
From: Marc Zyngier @ 2019-09-05 16:02 UTC (permalink / raw)
To: Peter Maydell, James Morse, Julien Thierry, Suzuki K Poulose,
Zenghui Yu, Will Deacon, Eric Auger
Cc: qemu-arm, kvmarm, linux-arm-kernel, kvm
While parts of the VGIC support a large number of vcpus (we
bravely allow up to 512), other parts are more limited.
One of these limits is visible in the KVM_IRQ_LINE ioctl, which
only allows 256 vcpus to be signalled when using the CPU or PPI
types. Unfortunately, we've cornered ourselves badly by allocating
all the bits in the irq field.
Since the irq_type subfield (8 bit wide) is currently only taking
the values 0, 1 and 2 (and we have been careful not to allow anything
else), let's reduce this field to only 4 bits, and allocate the
remaining 4 bits to a vcpu2_index, which acts as a multiplier:
vcpu_id = 256 * vcpu2_index + vcpu_index
With that, and a new capability (KVM_CAP_ARM_IRQ_LINE_LAYOUT_2)
allowing this to be discovered, it becomes possible to inject
PPIs to up to 4096 vcpus. But please just don't.
Whilst we're there, add a clarification about the use of KVM_IRQ_LINE
on arm, which is not completely conditionned by KVM_CAP_IRQCHIP.
Reported-by: Zenghui Yu <yuzenghui@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
* From v1 (https://lore.kernel.org/r/20190818140710.23920-1-maz@kernel.org)
- Always say that we support the new layout, no matter whether
we have an in-kernel irqchip or not
- Clarify use of KVM_IRQ_LINE wrt KVM_CAP_IRQCHIP
- Collected RBs
Documentation/virt/kvm/api.txt | 12 ++++++++++--
arch/arm/include/uapi/asm/kvm.h | 4 +++-
arch/arm64/include/uapi/asm/kvm.h | 4 +++-
include/uapi/linux/kvm.h | 1 +
virt/kvm/arm/arm.c | 2 ++
5 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/Documentation/virt/kvm/api.txt b/Documentation/virt/kvm/api.txt
index 2d067767b617..25931ca1cb38 100644
--- a/Documentation/virt/kvm/api.txt
+++ b/Documentation/virt/kvm/api.txt
@@ -753,8 +753,8 @@ in-kernel irqchip (GIC), and for in-kernel irqchip can tell the GIC to
use PPIs designated for specific cpus. The irq field is interpreted
like this:
- bits: | 31 ... 24 | 23 ... 16 | 15 ... 0 |
- field: | irq_type | vcpu_index | irq_id |
+ bits: | 31 ... 28 | 27 ... 24 | 23 ... 16 | 15 ... 0 |
+ field: | vcpu2_index | irq_type | vcpu_index | irq_id |
The irq_type field has the following values:
- irq_type[0]: out-of-kernel GIC: irq_id 0 is IRQ, irq_id 1 is FIQ
@@ -766,6 +766,14 @@ The irq_type field has the following values:
In both cases, level is used to assert/deassert the line.
+When KVM_CAP_ARM_IRQ_LINE_LAYOUT_2 is supported, the target vcpu is
+identified as (256 * vcpu2_index + vcpu_index). Otherwise, vcpu2_index
+must be zero.
+
+Note that on arm/arm64, the KVM_CAP_IRQCHIP capability only conditions
+injection of interrupts for the in-kernel irqchip. KVM_IRQ_LINE can always
+be used for a userspace interrupt controller.
+
struct kvm_irq_level {
union {
__u32 irq; /* GSI */
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
index a4217c1a5d01..2769360f195c 100644
--- a/arch/arm/include/uapi/asm/kvm.h
+++ b/arch/arm/include/uapi/asm/kvm.h
@@ -266,8 +266,10 @@ struct kvm_vcpu_events {
#define KVM_DEV_ARM_ITS_CTRL_RESET 4
/* KVM_IRQ_LINE irq field index values */
+#define KVM_ARM_IRQ_VCPU2_SHIFT 28
+#define KVM_ARM_IRQ_VCPU2_MASK 0xf
#define KVM_ARM_IRQ_TYPE_SHIFT 24
-#define KVM_ARM_IRQ_TYPE_MASK 0xff
+#define KVM_ARM_IRQ_TYPE_MASK 0xf
#define KVM_ARM_IRQ_VCPU_SHIFT 16
#define KVM_ARM_IRQ_VCPU_MASK 0xff
#define KVM_ARM_IRQ_NUM_SHIFT 0
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 9a507716ae2f..67c21f9bdbad 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -325,8 +325,10 @@ struct kvm_vcpu_events {
#define KVM_ARM_VCPU_TIMER_IRQ_PTIMER 1
/* KVM_IRQ_LINE irq field index values */
+#define KVM_ARM_IRQ_VCPU2_SHIFT 28
+#define KVM_ARM_IRQ_VCPU2_MASK 0xf
#define KVM_ARM_IRQ_TYPE_SHIFT 24
-#define KVM_ARM_IRQ_TYPE_MASK 0xff
+#define KVM_ARM_IRQ_TYPE_MASK 0xf
#define KVM_ARM_IRQ_VCPU_SHIFT 16
#define KVM_ARM_IRQ_VCPU_MASK 0xff
#define KVM_ARM_IRQ_NUM_SHIFT 0
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 5e3f12d5359e..5414b6588fbb 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -996,6 +996,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_ARM_PTRAUTH_ADDRESS 171
#define KVM_CAP_ARM_PTRAUTH_GENERIC 172
#define KVM_CAP_PMU_EVENT_FILTER 173
+#define KVM_CAP_ARM_IRQ_LINE_LAYOUT_2 174
#ifdef KVM_CAP_IRQ_ROUTING
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 35a069815baf..86c6aa1cb58e 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -196,6 +196,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_MP_STATE:
case KVM_CAP_IMMEDIATE_EXIT:
case KVM_CAP_VCPU_EVENTS:
+ case KVM_CAP_ARM_IRQ_LINE_LAYOUT_2:
r = 1;
break;
case KVM_CAP_ARM_SET_DEVICE_ADDR:
@@ -888,6 +889,7 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_level,
irq_type = (irq >> KVM_ARM_IRQ_TYPE_SHIFT) & KVM_ARM_IRQ_TYPE_MASK;
vcpu_idx = (irq >> KVM_ARM_IRQ_VCPU_SHIFT) & KVM_ARM_IRQ_VCPU_MASK;
+ vcpu_idx += ((irq >> KVM_ARM_IRQ_VCPU2_SHIFT) & KVM_ARM_IRQ_VCPU2_MASK) * (KVM_ARM_IRQ_VCPU_MASK + 1);
irq_num = (irq >> KVM_ARM_IRQ_NUM_SHIFT) & KVM_ARM_IRQ_NUM_MASK;
trace_kvm_irq_line(irq_type, vcpu_idx, irq_num, irq_level->level);
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [V3, 2/2] media: i2c: Add Omnivision OV02A10 camera sensor driver
From: Dongchun Zhu @ 2019-09-05 15:54 UTC (permalink / raw)
To: Tomasz Figa
Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, shengnan.wang,
louis.kuo, sj.huang, robh+dt, linux-mediatek, sakari.ailus,
matthias.bgg, bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <20190821103038.GA148543@chromium.org>
Hi Tomasz,
Thanks very much for your careful review.
Reply to the comments see belows.
Any missing please let me know.
On Wed, 2019-08-21 at 19:30 +0900, Tomasz Figa wrote:
> Hi Dongchun,
>
> On Mon, Aug 19, 2019 at 11:43:31AM +0800, dongchun.zhu@mediatek.com wrote:
> > From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> >
> > This patch adds a V4L2 sub-device driver for OV02A10 image sensor.
> > The OV02A10 is a 1/5" CMOS sensor from Omnivision,
> > which supports output format: 10-bit Raw.
> > The OV02A10 has a single MIPI lane interface and use the I2C bus
> > for control and the CSI-2 bus for data.
> >
> > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > ---
> > MAINTAINERS | 1 +
> > drivers/media/i2c/Kconfig | 11 +
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/ov02a10.c | 1018 +++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 1031 insertions(+)
> > create mode 100644 drivers/media/i2c/ov02a10.c
> >
>
> Thanks for the patch! Please see my comments inline.
>
> [snip]
>
> > +#define CHIP_ID 0x2509
> > +#define OV02A10_REG_CHIP_ID_H 0x02
> > +#define OV02A10_REG_CHIP_ID_L 0x03
> > +#define OV02A10_ID(_msb, _lsb) ((_msb) << 8 | (_lsb))
> > +
> > +/* Bit[1] vertical upside down */
> > +/* Bit[0] horizontal mirror */
> > +#define REG_MIRROR_FLIP_CONTROL 0x3f
> > +
> > +/* Orientation */
> > +#define REG_CONFIG_MIRROR_FLIP 0x03
> > +
> > +#define REG_PAGE_SWITCH 0xfd
> > +#define REG_GLOBAL_EFFECTIVE 0x01
> > +#define REG_ENABLE BIT(0)
> > +
> > +#define REG_SC_CTRL_MODE 0xac
> > +#define SC_CTRL_MODE_STANDBY 0x00
> > +#define SC_CTRL_MODE_STREAMING 0x01
> > +
> > +#define OV02A10_REG_EXPOSURE_H 0x03
> > +#define OV02A10_REG_EXPOSURE_L 0x04
> > +#define OV02A10_EXPOSURE_MIN 4
> > +#define OV02A10_EXPOSURE_STEP 1
> > +
> > +#define OV02A10_REG_VTS_H 0x05
> > +#define OV02A10_REG_VTS_L 0x06
> > +#define OV02A10_VTS_MAX 0x209f
> > +#define OV02A10_VTS_MIN 0x04cf
> > +#define OV02A10_BASIC_LINE 1224
> > +
> > +#define OV02A10_REG_GAIN 0x24
> > +#define OV02A10_GAIN_MIN 0x10
> > +#define OV02A10_GAIN_MAX 0xf8
> > +#define OV02A10_GAIN_STEP 0x01
> > +#define OV02A10_GAIN_DEFAULT 0x40
> > +
> > +#define REG_NULL 0xff
> > +
> > +#define OV02A10_LANES 1
> > +#define OV02A10_BITS_PER_SAMPLE 10
> > +
>
> I think there is something wrong with the indentation in the code above.
> Please use tabs wherever possible.
>
Sorry for the typo.
Fixed in next release.
> [snip]
>
> > +
> > +#define to_ov02a10(sd) container_of(sd, struct ov02a10, subdev)
>
> Please use a static inline function for added compile-time type checks.
>
Fixed in next release.
> > +
> > +static inline void msleep_range(unsigned int delay_base)
> > +{
> > + usleep_range(delay_base * 1000, delay_base * 1000 + 500);
> > +}
>
> Why not just use msleep()?
>
Generally for shorter sleep(e.g.less than 10ms), usleep_range is
preferred. We would use msleep for bigger sleep.
> > +
> > +/* MIPI color bar enable output */
> > +static const struct regval ov02a10_test_pattern_enable_regs[] = {
> > + {0xfd, 0x01},
> > + {0x0d, 0x00},
> > + {0xb6, 0x01},
> > + {0x01, 0x01},
> > + {0xfd, 0x01},
> > + {0xac, 0x01},
> > + {REG_NULL, 0x00}
> > +};
> > +
> > +/* MIPI color bar disable output */
> > +static const struct regval ov02a10_test_pattern_disable_regs[] = {
> > + {0xfd, 0x01},
> > + {0x0d, 0x00},
> > + {0xb6, 0x00},
> > + {0x01, 0x01},
> > + {0xfd, 0x01},
> > + {0xac, 0x01},
> > + {REG_NULL, 0x00}
> > +};
>
> Hmm, only the register 0xb6 seems to here. Could we just set it directly,
> without these arrays?
>
R0x0d is another color bar control register.
But R0x01 and R0xac both are essential, which could make color bar
register writing behavior global effective.
> > +
> > +/*
> > + * xvclk 24Mhz
>
> This seems to assume 24MHz, but the driver allows a range in probe. Is that
> correct?
>
That's OK.
As the input clock for sensor, the frequency of Master Clock(xvclk) is
regularly 24MHz. But actually the sensor xvclk could allow some little
tolerance. For instance, 1%.
> [snip]]
>
> > +/* Write a register */
> > +static int ov02a10_write_reg(struct ov02a10 *ov02a10, u8 addr, u8 val)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + u8 buf[2] = {addr, val};
> > + int ret;
> > +
> > + ret = i2c_master_send(client, buf, 2);
> > +
> > + if (ret != 2) {
> > + dev_err(&client->dev, "%s: error: reg=%x, val=%x\n",
> > + __func__, addr, val);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
>
> Could this be replaced with i2c_smbus_write_byte_data()?
>
We would have a try using this generic API.
> > +
> > +static int ov02a10_write_array(struct ov02a10 *ov02a10,
> > + const struct regval *regs)
> > +{
> > + u32 i;
> > + int ret;
> > +
> > + for (i = 0; regs[i].addr != REG_NULL; i++) {
> > + ret = ov02a10_write_reg(ov02a10, regs[i].addr, regs[i].val);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/* Read a register */
> > +static int ov02a10_read_reg(struct ov02a10 *ov02a10, u8 reg, u8 *val)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + u8 data = reg;
> > + struct i2c_msg msg = {
> > + .addr = client->addr,
> > + .flags = 0,
> > + .len = 1,
> > + .buf = &data,
> > + };
> > + int ret;
> > +
> > + ret = i2c_transfer(client->adapter, &msg, 1);
> > + if (ret < 0)
> > + goto err_wr;
> > +
> > + msg.flags = I2C_M_RD;
> > + ret = i2c_transfer(client->adapter, &msg, 1);
> > + if (ret < 0)
> > + goto err_rd;
>
> Could we just have 2 messages in an array and just call i2c_transfer() once
> for both write and read?
>
> Or actually it sounds like the i2c_smbus_read_byte_data() helper could work
> here.
>
Understood. We would have a try, too.
> > +
> > + *val = data;
> > + return 0;
> > +
> > +err_rd:
> > + dev_err(&client->dev, "i2c_transfer --I2C_M_RD failed\n");
> > +err_wr:
> > + dev_err(&client->dev, "read error: reg=0x%02x: %d\n", reg, ret);
> > + return ret;
> > +}
> > +
> > +static void ov02a10_fill_fmt(const struct ov02a10_mode *mode,
> > + struct v4l2_mbus_framefmt *fmt)
> > +{
> > + fmt->width = mode->width;
> > + fmt->height = mode->height;
> > + fmt->field = V4L2_FIELD_NONE;
> > +}
> > +
> > +static int ov02a10_set_fmt(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *fmt)
> > +{
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > + struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
> > +
> > + mutex_lock(&ov02a10->mutex);
> > +
> > + if (ov02a10->streaming) {
> > + mutex_unlock(&ov02a10->mutex);
> > + return -EBUSY;
> > + }
> > +
> > + /* Only one sensor mode supported */
> > + mbus_fmt->code = ov02a10->fmt.code;
> > + ov02a10_fill_fmt(ov02a10->cur_mode, mbus_fmt);
> > + ov02a10->fmt = fmt->format;
> > +
> > + mutex_unlock(&ov02a10->mutex);
> > +
> > + return 0;
> > +}
> > +
> > +static int ov02a10_get_fmt(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_format *fmt)
> > +{
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > + struct v4l2_mbus_framefmt *mbus_fmt = &fmt->format;
> > +
> > + mutex_lock(&ov02a10->mutex);
> > +
> > + fmt->format = ov02a10->fmt;
> > + mbus_fmt->code = ov02a10->fmt.code;
> > + ov02a10_fill_fmt(ov02a10->cur_mode, mbus_fmt);
> > +
> > + mutex_unlock(&ov02a10->mutex);
> > +
> > + return 0;
> > +}
> > +
> > +static int ov02a10_enum_mbus_code(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_mbus_code_enum *code)
> > +{
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +
> > + if (code->index >= ARRAY_SIZE(supported_modes) || !(code->index))
>
> Hmm, ARRAY_SIZE(supported_modes) is 1 and we don't allow code->index to be
> 0 either. Is there a code->index value that wouldn't return an error here?
>
Fixed in next release.
> > + return -EINVAL;
> > +
> > + code->code = ov02a10->fmt.code;
> > +
> > + return 0;
> > +}
> > +
> > +static int ov02a10_enum_frame_sizes(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_pad_config *cfg,
> > + struct v4l2_subdev_frame_size_enum *fse)
> > +{
> > + if (fse->index >= ARRAY_SIZE(supported_modes) || !(fse->index))
>
> Same here.
>
Fixed in next release.
> > + return -EINVAL;
> > +
> > + fse->min_width = supported_modes[fse->index].width;
> > + fse->max_width = supported_modes[fse->index].width;
> > + fse->max_height = supported_modes[fse->index].height;
> > + fse->min_height = supported_modes[fse->index].height;
> > +
> > + return 0;
> > +}
> > +
> > +static int __ov02a10_power_on(struct ov02a10 *ov02a10)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + struct device *dev = &client->dev;
> > + int ret;
> > +
> > + ret = clk_prepare_enable(ov02a10->xvclk);
> > + if (ret < 0) {
> > + dev_err(dev, "Failed to enable xvclk\n");
> > + return ret;
> > + }
>
> Is it really correct to enable the clock before the regulators?
>
> According to the datasheet, it should be:
> - PD pin HIGH,
> - nRST pin LOW,
> - DVDDIO and AVDD28 power up and stabilize,
> - clock enabled,
> - min 5 ms delay,
> - PD pin LOW,
> - min 4 ms delay,
> - nRST pin HIGH,
> - min 5 ms delay,
> - I2C interface ready.
>
xvclk as the clock source of sensor, we could enable it initially to let
PLL ready. In fact, the power up sequence mainly focus on PD, RST, and
IOVDD/AVDD28/DVDD pins.
> > +
> > + /* Note: set 0 is high, set 1 is low */
>
> Why is that? If there is some inverter on the way that should be handled
> outside of this driver. (GPIO DT bindings have flags for this purpose.
>
> If the pins are nRESET and nPOWERDOWN in the hardware datasheet, we should
> call them like this in the driver too (+/- the lowercase and underscore
> convention).
>
> According to the datasheet, the reset pin is called RST and inverted, so we should
> call it n_rst, but the powerdown signal, called PD, is not inverted, so pd
> would be the right name.
>
Sorry. Could make some more explanations about this?
> > + gpiod_set_value_cansleep(ov02a10->reset_gpio, 1);
> > + gpiod_set_value_cansleep(ov02a10->powerdown_gpio, 0);
> > +
> > + ret = regulator_bulk_enable(OV02A10_NUM_SUPPLIES, ov02a10->supplies);
> > + if (ret < 0) {
> > + dev_err(dev, "Failed to enable regulators\n");
> > + goto disable_clk;
> > + }
> > + msleep_range(7);
> > +
> > + gpiod_set_value_cansleep(ov02a10->powerdown_gpio, 1);
> > + msleep_range(10);
> > +
> > + gpiod_set_value_cansleep(ov02a10->reset_gpio, 0);
> > + msleep_range(10);
> > +
> > + return 0;
> > +
> > +disable_clk:
> > + clk_disable_unprepare(ov02a10->xvclk);
> > +
> > + return ret;
> > +}
> > +
> > +static void __ov02a10_power_off(struct ov02a10 *ov02a10)
> > +{
> > + clk_disable_unprepare(ov02a10->xvclk);
> > + gpiod_set_value_cansleep(ov02a10->reset_gpio, 1);
> > + gpiod_set_value_cansleep(ov02a10->powerdown_gpio, 1);
> > + regulator_bulk_disable(OV02A10_NUM_SUPPLIES, ov02a10->supplies);
>
> This also doesn't seem to match my datasheet. The sequence there is:
> - nRST goes LOW,
> - clock stops,
> - PD goes HIGH,
> - regulators are powerd down.
>
Got it.
This would be fixed in next release.
> > +}
> > +
> > +static int __ov02a10_start_stream(struct ov02a10 *ov02a10)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + int ret;
> > +
> > + /* Apply default values of current mode */
> > + ret = ov02a10_write_array(ov02a10, ov02a10->cur_mode->reg_list);
> > + if (ret)
> > + return ret;
> > +
> > + /* Apply customized values from user */
> > + ret = __v4l2_ctrl_handler_setup(ov02a10->subdev.ctrl_handler);
> > + if (ret)
> > + return ret;
> > +
> > + /* Set orientation to 180 degree */
> > + if (ov02a10->upside_down) {
> > + ret = ov02a10_write_reg(ov02a10, REG_MIRROR_FLIP_CONTROL,
> > + REG_CONFIG_MIRROR_FLIP);
> > + if (ret) {
> > + dev_err(&client->dev, "%s failed to set orientation\n",
> > + __func__);
> > + return ret;
> > + }
> > + ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> > + REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + /* Set stream on register */
> > + return ov02a10_write_reg(ov02a10,
> > + REG_SC_CTRL_MODE, SC_CTRL_MODE_STREAMING);
> > +}
> > +
> > +static int __ov02a10_stop_stream(struct ov02a10 *ov02a10)
> > +{
> > + return ov02a10_write_reg(ov02a10,
> > + REG_SC_CTRL_MODE, SC_CTRL_MODE_STANDBY);
> > +}
> > +
> > +static int ov02a10_entity_init_cfg(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_pad_config *cfg)
> > +{
> > + struct v4l2_subdev_format fmt = { 0 };
> > +
> > + fmt.which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
> > + fmt.format.width = 1600;
> > + fmt.format.height = 1200;
>
> Where do these values come from? Should we have some macros for them?
>
Fixed in next release.
> > +
> > + ov02a10_set_fmt(subdev, cfg, &fmt);
> > +
> > + return 0;
> > +}
> > +
> > +static int ov02a10_s_stream(struct v4l2_subdev *sd, int on)
> > +{
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + int ret = 0;
> > +
> > + mutex_lock(&ov02a10->mutex);
> > +
> > + if (ov02a10->streaming == on)
> > + goto unlock_and_return;
> > +
> > + if (on) {
> > + ret = pm_runtime_get_sync(&client->dev);
> > + if (ret < 0) {
> > + pm_runtime_put_noidle(&client->dev);
> > + goto unlock_and_return;
> > + }
> > +
> > + ret = __ov02a10_start_stream(ov02a10);
> > + if (ret) {
> > + __ov02a10_stop_stream(ov02a10);
> > + ov02a10->streaming = !on;
> > + goto err_rpm_put;
> > + }
> > + } else {
> > + __ov02a10_stop_stream(ov02a10);
> > + pm_runtime_put(&client->dev);
> > + }
> > +
> > + ov02a10->streaming = on;
> > + mutex_unlock(&ov02a10->mutex);
> > +
> > + return ret;
> > +
> > +err_rpm_put:
> > + pm_runtime_put(&client->dev);
> > +unlock_and_return:
> > + mutex_unlock(&ov02a10->mutex);
> > +
> > + return ret;
> > +}
> > +
> > +static int ov02a10_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> > +{
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > + struct v4l2_mbus_framefmt *try_fmt = v4l2_subdev_get_try_format(sd,
> > + fh->pad,
> > + 0);
>
> Please separate the initialization from the declaration, because there
> isn't just enough space on this line anymore.
>
As Sakari pointed out, indeed this function is repeated executed since
init_cfg is implemented already. Thus we would omit the open callback in
next release.
> > +
> > + mutex_lock(&ov02a10->mutex);
> > + /* Initialize try_fmt */
> > + try_fmt->code = ov02a10->fmt.code;
> > + ov02a10_fill_fmt(&supported_modes[0], try_fmt);
> > +
> > + mutex_unlock(&ov02a10->mutex);
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused ov02a10_runtime_resume(struct device *dev)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +
> > + return __ov02a10_power_on(ov02a10);
> > +}
> > +
> > +static int __maybe_unused ov02a10_runtime_suspend(struct device *dev)
> > +{
> > + struct i2c_client *client = to_i2c_client(dev);
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +
> > + __ov02a10_power_off(ov02a10);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct dev_pm_ops ov02a10_pm_ops = {
> > + SET_RUNTIME_PM_OPS(ov02a10_runtime_suspend,
> > + ov02a10_runtime_resume, NULL)
>
> Don't we need to implement and provide system PM ops too?
>
Fixed in next release.
> > +};
> > +
> > +static int ov02a10_set_test_pattern(struct ov02a10 *ov02a10, s32 value)
> > +{
> > + if (value)
> > + return ov02a10_write_array(ov02a10,
> > + ov02a10_test_pattern_enable_regs);
> > +
> > + return ov02a10_write_array(ov02a10,
> > + ov02a10_test_pattern_disable_regs);
> > +}
> > +
> > +static int ov02a10_set_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct ov02a10 *ov02a10 = container_of(ctrl->handler,
> > + struct ov02a10, ctrl_handler);
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + s64 max_expo;
> > + int ret;
> > +
> > + /* Propagate change of current control to all related controls */
> > + if (ctrl->id == V4L2_CID_VBLANK) {
> > + /* Update max exposure while meeting expected vblanking */
> > + max_expo = ov02a10->cur_mode->height + ctrl->val - 4;
> > + __v4l2_ctrl_modify_range(ov02a10->exposure,
> > + ov02a10->exposure->minimum, max_expo,
> > + ov02a10->exposure->step,
> > + ov02a10->exposure->default_value);
> > + }
> > +
> > + /* V4L2 controls values will be applied only when power is already up */
> > + if (!pm_runtime_get_if_in_use(&client->dev))
> > + return 0;
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_EXPOSURE:
> > + ret = ov02a10_write_reg(ov02a10, REG_PAGE_SWITCH, REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_EXPOSURE_H,
> > + ((ctrl->val >> 8) & 0xFF));
> > + if (!ret) {
> > + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_EXPOSURE_L,
> > + (ctrl->val & 0xFF));
> > + if (ret < 0)
> > + return ret;
> > + }
> > + ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> > + REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + break;
> > + case V4L2_CID_ANALOGUE_GAIN:
> > + ret = ov02a10_write_reg(ov02a10, REG_PAGE_SWITCH, REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_GAIN,
> > + (ctrl->val & 0xFF));
> > + if (ret < 0)
> > + return ret;
> > + ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> > + REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + break;
> > + case V4L2_CID_VBLANK:
> > + ret = ov02a10_write_reg(ov02a10, REG_PAGE_SWITCH, REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_VTS_H,
> > + (((ctrl->val +
> > + ov02a10->cur_mode->height -
> > + OV02A10_BASIC_LINE) >> 8)
> > + & 0xFF));
> > + if (!ret) {
> > + ret = ov02a10_write_reg(ov02a10, OV02A10_REG_VTS_L,
> > + ((ctrl->val +
> > + ov02a10->cur_mode->height -
> > + OV02A10_BASIC_LINE) & 0xFF));
> > + if (ret < 0)
> > + return ret;
> > + }
> > + ret = ov02a10_write_reg(ov02a10, REG_GLOBAL_EFFECTIVE,
> > + REG_ENABLE);
> > + if (ret < 0)
> > + return ret;
> > + break;
> > + case V4L2_CID_TEST_PATTERN:
> > + ret = ov02a10_set_test_pattern(ov02a10, ctrl->val);
> > + if (ret < 0)
> > + return ret;
> > + break;
> > + default:
> > + dev_warn(&client->dev, "%s Unhandled id:0x%x, val:0x%x\n",
> > + __func__, ctrl->id, ctrl->val);
> > + ret = -EINVAL;
> > + break;
>
> We shouldn't need to handle this, as the control framework wouldn't call us
> with a control that we didn't register explicitly.
>
I see other sensors like ov5645 and ov8856 also have this error ctrl id
handle. Did you mean that "default:..." could be omitted?
> > + };
> > +
> > + pm_runtime_put(&client->dev);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct v4l2_subdev_video_ops ov02a10_video_ops = {
> > + .s_stream = ov02a10_s_stream,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops ov02a10_pad_ops = {
> > + .init_cfg = ov02a10_entity_init_cfg,
> > + .enum_mbus_code = ov02a10_enum_mbus_code,
> > + .enum_frame_size = ov02a10_enum_frame_sizes,
> > + .get_fmt = ov02a10_get_fmt,
> > + .set_fmt = ov02a10_set_fmt,
> > +};
> > +
> > +static const struct v4l2_subdev_ops ov02a10_subdev_ops = {
> > + .video = &ov02a10_video_ops,
> > + .pad = &ov02a10_pad_ops,
> > +};
> > +
> > +static const struct media_entity_operations ov02a10_subdev_entity_ops = {
> > + .link_validate = v4l2_subdev_link_validate,
> > +};
> > +
> > +static const struct v4l2_subdev_internal_ops ov02a10_internal_ops = {
> > + .open = ov02a10_open,
> > +};
> > +
> > +static const struct v4l2_ctrl_ops ov02a10_ctrl_ops = {
> > + .s_ctrl = ov02a10_set_ctrl,
> > +};
> > +
> > +static int ov02a10_initialize_controls(struct ov02a10 *ov02a10)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + const struct ov02a10_mode *mode;
> > + struct v4l2_ctrl_handler *handler;
> > + struct v4l2_ctrl *ctrl;
> > + u64 exposure_max;
> > + u32 pixel_rate, h_blank;
> > + int ret;
> > +
> > + handler = &ov02a10->ctrl_handler;
> > + mode = ov02a10->cur_mode;
> > + ret = v4l2_ctrl_handler_init(handler, 10);
>
> I can see 6 controls registered below.
>
I think handler num could be little more than the implemented num.
But as you suggested, this would be fixed in next release.
> > + if (ret)
> > + return ret;
> > + handler->lock = &ov02a10->mutex;
> > +
> > + ctrl = v4l2_ctrl_new_int_menu(handler, NULL, V4L2_CID_LINK_FREQ,
> > + 0, 0, link_freq_menu_items);
> > + if (ctrl)
> > + ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > + pixel_rate = (link_freq_menu_items[0] * 2 * OV02A10_LANES) /
> > + OV02A10_BITS_PER_SAMPLE;
> > + v4l2_ctrl_new_std(handler, NULL, V4L2_CID_PIXEL_RATE,
> > + 0, pixel_rate, 1, pixel_rate);
> > +
> > + h_blank = mode->hts_def - mode->width;
> > + ov02a10->hblank = v4l2_ctrl_new_std(handler, NULL, V4L2_CID_HBLANK,
> > + h_blank, h_blank, 1, h_blank);
> > + if (ov02a10->hblank)
> > + ov02a10->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > + ov02a10->vblank = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> > + V4L2_CID_VBLANK, mode->vts_def -
> > + mode->height,
> > + OV02A10_VTS_MAX - mode->height, 1,
> > + mode->vts_def - mode->height);
> > +
> > + exposure_max = mode->vts_def - 4;
> > + ov02a10->exposure = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> > + V4L2_CID_EXPOSURE,
> > + OV02A10_EXPOSURE_MIN,
> > + exposure_max,
> > + OV02A10_EXPOSURE_STEP,
> > + mode->exp_def);
> > +
> > + ov02a10->anal_gain = v4l2_ctrl_new_std(handler, &ov02a10_ctrl_ops,
> > + V4L2_CID_ANALOGUE_GAIN,
> > + OV02A10_GAIN_MIN,
> > + OV02A10_GAIN_MAX,
> > + OV02A10_GAIN_STEP,
> > + OV02A10_GAIN_DEFAULT);
> > +
> > + ov02a10->test_pattern =
> > + v4l2_ctrl_new_std_menu_items(handler,
> > + &ov02a10_ctrl_ops,
> > + V4L2_CID_TEST_PATTERN,
> > + ARRAY_SIZE(ov02a10_test_pattern_menu) -
> > + 1, 0, 0, ov02a10_test_pattern_menu);
> > +
> > + if (handler->error) {
> > + ret = handler->error;
> > + dev_err(&client->dev,
> > + "Failed to init controls(%d)\n", ret);
> > + goto err_free_handler;
> > + }
> > +
> > + ov02a10->subdev.ctrl_handler = handler;
> > +
> > + return 0;
> > +
> > +err_free_handler:
> > + v4l2_ctrl_handler_free(handler);
> > +
> > + return ret;
> > +}
> > +
> > +static int ov02a10_check_sensor_id(struct ov02a10 *ov02a10)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + u16 id;
> > + u8 pid = 0;
> > + u8 ver = 0;
> > + int ret;
> > +
> > + /* Check sensor revision */
> > + ret = ov02a10_read_reg(ov02a10, OV02A10_REG_CHIP_ID_H, &pid);
> > + if (ret)
> > + return ret;
> > +
> > + ret = ov02a10_read_reg(ov02a10, OV02A10_REG_CHIP_ID_L, &ver);
> > + if (ret)
> > + return ret;
> > +
> > + id = OV02A10_ID(pid, ver);
> > + if (id != CHIP_ID) {
> > + dev_err(&client->dev, "Unexpected sensor id(%04x)\n", id);
> > + return ret;
> > + }
> > +
> > + dev_info(&client->dev, "Detected OV%04X sensor\n", id);
> > +
> > + return 0;
> > +}
> > +
> > +static int ov02a10_configure_regulators(struct ov02a10 *ov02a10)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&ov02a10->subdev);
> > + unsigned int i;
> > +
> > + for (i = 0; i < OV02A10_NUM_SUPPLIES; i++)
> > + ov02a10->supplies[i].supply = ov02a10_supply_names[i];
> > +
> > + return devm_regulator_bulk_get(&client->dev,
> > + OV02A10_NUM_SUPPLIES,
> > + ov02a10->supplies);
> > +}
>
> I think we can just have this directly inside probe.
>
Understood.
We would have a try.
> > +
> > +static int ov02a10_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct ov02a10 *ov02a10;
> > + u32 rotation;
> > + u32 xclk_freq;
> > + int ret;
> > +
> > + ov02a10 = devm_kzalloc(dev, sizeof(*ov02a10), GFP_KERNEL);
> > + if (!ov02a10)
> > + return -ENOMEM;
> > +
> > + v4l2_i2c_subdev_init(&ov02a10->subdev, client, &ov02a10_subdev_ops);
> > + ov02a10->fmt.code = MEDIA_BUS_FMT_SBGGR10_1X10;
> > +
> > + /* Optional indication of physical rotation of sensor */
> > + ret = fwnode_property_read_u32(dev_fwnode(dev), "rotation",
> > + &rotation);
> > + if (!ret) {
> > + switch (rotation) {
> > + case 180:
> > + ov02a10->upside_down = true;
> > + ov02a10->fmt.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> > + break;
> > + case 0:
> > + break;
> > + default:
> > + dev_warn(dev, "%u degrees rotation is not supported, ignoring...\n",
> > + rotation);
> > + }
> > + }
> > +
> > + /* Get system clock (xvclk) */
> > + ov02a10->xvclk = devm_clk_get(dev, "xvclk");
> > + if (IS_ERR(ov02a10->xvclk)) {
> > + dev_err(dev, "Failed to get xvclk\n");
> > + return -EINVAL;
> > + }
>
> Hmm, it's called eclk in my datasheet.
>
Does this really matter?
Should we follow the common naming, refer to other sensors?
> > +
> > + ret = of_property_read_u32(dev->of_node, "clock-frequency", &xclk_freq);
> > + if (ret) {
> > + dev_err(dev, "Failed to get xclk frequency\n");
> > + return ret;
> > + }
> > +
> > + /* External clock must be 24MHz, allow 1% tolerance */
> > + if (xclk_freq < 23760000 || xclk_freq > 24240000) {
>
> How do we support a range of frequencies? I don't see the driver calculate
> any register values based on this frequency. Are you sure that the register
> arrays don't assume one specific frequency?
>
In fact, clock-frequency defined in DT is always 24MHz, which is the
clock frequency that sensor requires to keep itself work normally.
> > + dev_err(dev, "external clock frequency %u is not supported\n",
> > + xclk_freq);
> > + return -EINVAL;
> > + }
> > + dev_dbg(dev, "external clock frequency %u\n", xclk_freq);
> > +
> > + ret = clk_set_rate(ov02a10->xvclk, xclk_freq);
> > + if (ret) {
> > + dev_err(dev, "Failed to set xvclk frequency (24MHz)\n");
> > + return ret;
> > + }
> > +
> > + ov02a10->powerdown_gpio = devm_gpiod_get(dev, "powerdown",
> > + GPIOD_OUT_LOW);
>
> Hmm, shouldn't this be HIGH? At least the datasheet has it so for the
> powered down state.
>
The gpio state for this API callback seems to be inverse.
> > + if (IS_ERR(ov02a10->powerdown_gpio)) {
> > + dev_err(dev, "Failed to get powerdown-gpios\n");
> > + return -EINVAL;
> > + }
> > +
> > + ov02a10->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
>
> Also LOW here, similarly to the above.
>
Ditto.
> > + if (IS_ERR(ov02a10->reset_gpio)) {
> > + dev_err(dev, "Failed to get reset-gpios\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = ov02a10_configure_regulators(ov02a10);
> > + if (ret) {
> > + dev_err(dev, "Failed to get power regulators\n");
> > + return ret;
> > + }
> > +
> > + mutex_init(&ov02a10->mutex);
> > + ov02a10->cur_mode = &supported_modes[0];
> > + ret = ov02a10_initialize_controls(ov02a10);
> > + if (ret) {
> > + dev_err(dev, "Failed to initialize controls\n");
> > + goto err_destroy_mutex;
> > + }
> > +
> > + ret = __ov02a10_power_on(ov02a10);
> > + if (ret)
> > + goto err_free_handler;
> > +
> > + ret = ov02a10_check_sensor_id(ov02a10);
> > + if (ret)
> > + goto err_power_off;
> > +
> > + ov02a10->subdev.internal_ops = &ov02a10_internal_ops;
> > + ov02a10->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > + ov02a10->subdev.entity.ops = &ov02a10_subdev_entity_ops;
> > + ov02a10->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > + ov02a10->pad.flags = MEDIA_PAD_FL_SOURCE;
> > + ret = media_entity_pads_init(&ov02a10->subdev.entity, 1, &ov02a10->pad);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to init entity pads: %d", ret);
> > + goto err_power_off;
> > + }
> > +
> > + ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > + if (ret) {
> > + dev_err(dev, "failed to register V4L2 subdev: %d",
> > + ret);
> > + goto err_clean_entity;
> > + }
> > +
> > + pm_runtime_set_active(dev);
> > + pm_runtime_enable(dev);
> > + pm_runtime_idle(dev);
> > +
> > + dev_info(dev, "ov02a10 probe --\n");
>
> Please remove this.
>
Fixed in next release.
> > + return 0;
> > +
> > +err_clean_entity:
> > + media_entity_cleanup(&ov02a10->subdev.entity);
> > +err_power_off:
> > + __ov02a10_power_off(ov02a10);
> > +err_free_handler:
> > + v4l2_ctrl_handler_free(ov02a10->subdev.ctrl_handler);
> > +err_destroy_mutex:
> > + mutex_destroy(&ov02a10->mutex);
> > +
> > + return ret;
> > +}
> > +
> > +static int ov02a10_remove(struct i2c_client *client)
> > +{
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > +
> > + v4l2_async_unregister_subdev(sd);
> > + media_entity_cleanup(&sd->entity);
> > + v4l2_ctrl_handler_free(sd->ctrl_handler);
> > + pm_runtime_disable(&client->dev);
> > + if (!pm_runtime_status_suspended(&client->dev))
> > + __ov02a10_power_off(ov02a10);
> > + pm_runtime_set_suspended(&client->dev);
> > + mutex_destroy(&ov02a10->mutex);
> > +
> > + return 0;
> > +}
> > +
> > +#if IS_ENABLED(CONFIG_OF)
> > +static const struct of_device_id ov02a10_of_match[] = {
> > + { .compatible = "ovti,ov02a10" },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, ov02a10_of_match);
> > +#endif
> > +
> > +static struct i2c_driver ov02a10_i2c_driver = {
> > + .driver = {
> > + .name = "ov02a10",
> > + .pm = &ov02a10_pm_ops,
> > + .of_match_table = ov02a10_of_match,
>
> Please use of_match_ptr() wrapper.
>
Sorry. I am a little confused now.
It seems that Sakari had one different ideas about this.
https://patchwork.kernel.org/patch/10957225/
> Best regards,
> Tomasz
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox