* [PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate
From: Fabio Estevam @ 2016-10-28 19:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161028181316.GI16026@codeaurora.org>
On Fri, Oct 28, 2016 at 4:13 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> The clk-fixes branch is for patches that fix problems in code
> merged during the merge window as well as small one-liners and
> things that are causing great pain for people on the latest
> release. Given that this fixes a regression in v4.8 and we're
> trying to stabilize v4.9 it looked like it could wait until
> v4.10.
The regression affects 4.8 and 4.9.
> So is there something going on here where the pain is too much to
> wait for the next merge window? If so the commit text should
Yes, people reported HDMI issues because of this bug:
https://www.spinics.net/lists/arm-kernel/msg535304.html
> mention something about what's causing that pain. Perhaps by
> referencing the commit that merged outside of clk tree that
> caused problems.
This patch clearly states that the problem is caused by ba7f4f557eb6
("clk: imx: correct AV PLL rate formula").
Since this is a regression, I don't understand why we need to wait
until 4.10 to get it applied.
^ permalink raw reply
* [PATCH v2 3/5] fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
From: kbuild test robot @ 2016-10-28 19:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <202cdeff42a2de149c471630110a8b2657ccf5ca.1477669745.git.stillcompiling@gmail.com>
Hi Joshua,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.9-rc2 next-20161028]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Joshua-Clayton/lib-add-bitrev8x4/20161029-012535
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All error/warnings (new ones prefixed by >>):
In file included from drivers/fpga/cyclone-ps-spi.c:13:0:
drivers/fpga/cyclone-ps-spi.c: In function 'rev_buf':
>> include/linux/bitrev.h:12:21: error: implicit declaration of function '__arch_bitrev8x4' [-Werror=implicit-function-declaration]
#define __bitrev8x4 __arch_bitrev8x4
^
>> include/linux/bitrev.h:101:2: note: in expansion of macro '__bitrev8x4'
__bitrev8x4(__x); \
^~~~~~~~~~~
>> drivers/fpga/cyclone-ps-spi.c:75:11: note: in expansion of macro 'bitrev8x4'
*fw32 = bitrev8x4(*fw32);
^~~~~~~~~
In file included from include/linux/delay.h:10:0,
from drivers/fpga/cyclone-ps-spi.c:14:
drivers/fpga/cyclone-ps-spi.c: In function 'cyclonespi_write':
include/linux/kernel.h:739:16: warning: comparison of distinct pointer types lacks a cast
(void) (&min1 == &min2); \
^
include/linux/kernel.h:742:2: note: in expansion of macro '__min'
__min(typeof(x), typeof(y), \
^~~~~
drivers/fpga/cyclone-ps-spi.c:89:19: note: in expansion of macro 'min'
size_t stride = min(fw_data_end - fw_data, SZ_4K);
^~~
cc1: some warnings being treated as errors
vim +/__arch_bitrev8x4 +12 include/linux/bitrev.h
556d2f05 Yalin Wang 2014-11-03 6 #ifdef CONFIG_HAVE_ARCH_BITREVERSE
556d2f05 Yalin Wang 2014-11-03 7 #include <asm/bitrev.h>
556d2f05 Yalin Wang 2014-11-03 8
556d2f05 Yalin Wang 2014-11-03 9 #define __bitrev32 __arch_bitrev32
556d2f05 Yalin Wang 2014-11-03 10 #define __bitrev16 __arch_bitrev16
556d2f05 Yalin Wang 2014-11-03 11 #define __bitrev8 __arch_bitrev8
69ff2a81 Joshua Clayton 2016-10-28 @12 #define __bitrev8x4 __arch_bitrev8x4
a5cfc1ec Akinobu Mita 2006-12-08 13
556d2f05 Yalin Wang 2014-11-03 14 #else
556d2f05 Yalin Wang 2014-11-03 15 extern u8 const byte_rev_table[256];
556d2f05 Yalin Wang 2014-11-03 16 static inline u8 __bitrev8(u8 byte)
a5cfc1ec Akinobu Mita 2006-12-08 17 {
a5cfc1ec Akinobu Mita 2006-12-08 18 return byte_rev_table[byte];
a5cfc1ec Akinobu Mita 2006-12-08 19 }
a5cfc1ec Akinobu Mita 2006-12-08 20
556d2f05 Yalin Wang 2014-11-03 21 static inline u16 __bitrev16(u16 x)
556d2f05 Yalin Wang 2014-11-03 22 {
556d2f05 Yalin Wang 2014-11-03 23 return (__bitrev8(x & 0xff) << 8) | __bitrev8(x >> 8);
556d2f05 Yalin Wang 2014-11-03 24 }
556d2f05 Yalin Wang 2014-11-03 25
556d2f05 Yalin Wang 2014-11-03 26 static inline u32 __bitrev32(u32 x)
556d2f05 Yalin Wang 2014-11-03 27 {
556d2f05 Yalin Wang 2014-11-03 28 return (__bitrev16(x & 0xffff) << 16) | __bitrev16(x >> 16);
556d2f05 Yalin Wang 2014-11-03 29 }
556d2f05 Yalin Wang 2014-11-03 30
69ff2a81 Joshua Clayton 2016-10-28 31 static inline u32 __bitrev8x4(u32 x)
69ff2a81 Joshua Clayton 2016-10-28 32 {
69ff2a81 Joshua Clayton 2016-10-28 33 return(__bitrev8(x & 0xff) |
69ff2a81 Joshua Clayton 2016-10-28 34 (__bitrev8((x >> 8) & 0xff) << 8) |
69ff2a81 Joshua Clayton 2016-10-28 35 (__bitrev8((x >> 16) & 0xff) << 16) |
69ff2a81 Joshua Clayton 2016-10-28 36 (__bitrev8((x >> 24) & 0xff) << 24));
69ff2a81 Joshua Clayton 2016-10-28 37 }
69ff2a81 Joshua Clayton 2016-10-28 38
556d2f05 Yalin Wang 2014-11-03 39 #endif /* CONFIG_HAVE_ARCH_BITREVERSE */
556d2f05 Yalin Wang 2014-11-03 40
556d2f05 Yalin Wang 2014-11-03 41 #define __constant_bitrev32(x) \
556d2f05 Yalin Wang 2014-11-03 42 ({ \
556d2f05 Yalin Wang 2014-11-03 43 u32 __x = x; \
556d2f05 Yalin Wang 2014-11-03 44 __x = (__x >> 16) | (__x << 16); \
556d2f05 Yalin Wang 2014-11-03 45 __x = ((__x & (u32)0xFF00FF00UL) >> 8) | ((__x & (u32)0x00FF00FFUL) << 8); \
556d2f05 Yalin Wang 2014-11-03 46 __x = ((__x & (u32)0xF0F0F0F0UL) >> 4) | ((__x & (u32)0x0F0F0F0FUL) << 4); \
556d2f05 Yalin Wang 2014-11-03 47 __x = ((__x & (u32)0xCCCCCCCCUL) >> 2) | ((__x & (u32)0x33333333UL) << 2); \
556d2f05 Yalin Wang 2014-11-03 48 __x = ((__x & (u32)0xAAAAAAAAUL) >> 1) | ((__x & (u32)0x55555555UL) << 1); \
556d2f05 Yalin Wang 2014-11-03 49 __x; \
556d2f05 Yalin Wang 2014-11-03 50 })
556d2f05 Yalin Wang 2014-11-03 51
556d2f05 Yalin Wang 2014-11-03 52 #define __constant_bitrev16(x) \
556d2f05 Yalin Wang 2014-11-03 53 ({ \
556d2f05 Yalin Wang 2014-11-03 54 u16 __x = x; \
556d2f05 Yalin Wang 2014-11-03 55 __x = (__x >> 8) | (__x << 8); \
556d2f05 Yalin Wang 2014-11-03 56 __x = ((__x & (u16)0xF0F0U) >> 4) | ((__x & (u16)0x0F0FU) << 4); \
556d2f05 Yalin Wang 2014-11-03 57 __x = ((__x & (u16)0xCCCCU) >> 2) | ((__x & (u16)0x3333U) << 2); \
556d2f05 Yalin Wang 2014-11-03 58 __x = ((__x & (u16)0xAAAAU) >> 1) | ((__x & (u16)0x5555U) << 1); \
556d2f05 Yalin Wang 2014-11-03 59 __x; \
556d2f05 Yalin Wang 2014-11-03 60 })
556d2f05 Yalin Wang 2014-11-03 61
69ff2a81 Joshua Clayton 2016-10-28 62 #define __constant_bitrev8x4(x) \
69ff2a81 Joshua Clayton 2016-10-28 63 ({ \
69ff2a81 Joshua Clayton 2016-10-28 64 u32 __x = x; \
69ff2a81 Joshua Clayton 2016-10-28 65 __x = ((__x & (u32)0xF0F0F0F0UL) >> 4) | ((__x & (u32)0x0F0F0F0FUL) << 4); \
69ff2a81 Joshua Clayton 2016-10-28 66 __x = ((__x & (u32)0xCCCCCCCCUL) >> 2) | ((__x & (u32)0x33333333UL) << 2); \
69ff2a81 Joshua Clayton 2016-10-28 67 __x = ((__x & (u32)0xAAAAAAAAUL) >> 1) | ((__x & (u32)0x55555555UL) << 1); \
69ff2a81 Joshua Clayton 2016-10-28 68 __x; \
69ff2a81 Joshua Clayton 2016-10-28 69 })
69ff2a81 Joshua Clayton 2016-10-28 70
556d2f05 Yalin Wang 2014-11-03 71 #define __constant_bitrev8(x) \
556d2f05 Yalin Wang 2014-11-03 72 ({ \
556d2f05 Yalin Wang 2014-11-03 73 u8 __x = x; \
556d2f05 Yalin Wang 2014-11-03 74 __x = (__x >> 4) | (__x << 4); \
556d2f05 Yalin Wang 2014-11-03 75 __x = ((__x & (u8)0xCCU) >> 2) | ((__x & (u8)0x33U) << 2); \
556d2f05 Yalin Wang 2014-11-03 76 __x = ((__x & (u8)0xAAU) >> 1) | ((__x & (u8)0x55U) << 1); \
556d2f05 Yalin Wang 2014-11-03 77 __x; \
556d2f05 Yalin Wang 2014-11-03 78 })
556d2f05 Yalin Wang 2014-11-03 79
556d2f05 Yalin Wang 2014-11-03 80 #define bitrev32(x) \
556d2f05 Yalin Wang 2014-11-03 81 ({ \
556d2f05 Yalin Wang 2014-11-03 82 u32 __x = x; \
556d2f05 Yalin Wang 2014-11-03 83 __builtin_constant_p(__x) ? \
556d2f05 Yalin Wang 2014-11-03 84 __constant_bitrev32(__x) : \
556d2f05 Yalin Wang 2014-11-03 85 __bitrev32(__x); \
556d2f05 Yalin Wang 2014-11-03 86 })
556d2f05 Yalin Wang 2014-11-03 87
556d2f05 Yalin Wang 2014-11-03 88 #define bitrev16(x) \
556d2f05 Yalin Wang 2014-11-03 89 ({ \
556d2f05 Yalin Wang 2014-11-03 90 u16 __x = x; \
556d2f05 Yalin Wang 2014-11-03 91 __builtin_constant_p(__x) ? \
556d2f05 Yalin Wang 2014-11-03 92 __constant_bitrev16(__x) : \
556d2f05 Yalin Wang 2014-11-03 93 __bitrev16(__x); \
556d2f05 Yalin Wang 2014-11-03 94 })
a5cfc1ec Akinobu Mita 2006-12-08 95
69ff2a81 Joshua Clayton 2016-10-28 96 #define bitrev8x4(x) \
69ff2a81 Joshua Clayton 2016-10-28 97 ({ \
69ff2a81 Joshua Clayton 2016-10-28 98 u32 __x = x; \
69ff2a81 Joshua Clayton 2016-10-28 99 __builtin_constant_p(__x) ? \
69ff2a81 Joshua Clayton 2016-10-28 100 __constant_bitrev8x4(__x) : \
69ff2a81 Joshua Clayton 2016-10-28 @101 __bitrev8x4(__x); \
69ff2a81 Joshua Clayton 2016-10-28 102 })
69ff2a81 Joshua Clayton 2016-10-28 103
556d2f05 Yalin Wang 2014-11-03 104 #define bitrev8(x) \
:::::: The code at line 12 was first introduced by commit
:::::: 69ff2a81709e0969b2d1a0efaa9a010e0093917c lib: add bitrev8x4()
:::::: TO: Joshua Clayton <stillcompiling@gmail.com>
:::::: CC: 0day robot <fengguang.wu@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 52470 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161029/eb4a95b7/attachment-0001.gz>
^ permalink raw reply
* [PATCH v2 3/5] fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
From: kbuild test robot @ 2016-10-28 18:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <202cdeff42a2de149c471630110a8b2657ccf5ca.1477669745.git.stillcompiling@gmail.com>
Hi Joshua,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.9-rc2 next-20161028]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Joshua-Clayton/lib-add-bitrev8x4/20161029-012535
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
In file included from include/linux/delay.h:10:0,
from drivers/fpga/cyclone-ps-spi.c:14:
drivers/fpga/cyclone-ps-spi.c: In function 'cyclonespi_write':
>> drivers/fpga/cyclone-ps-spi.c:89:46: error: 'SZ_4K' undeclared (first use in this function)
size_t stride = min(fw_data_end - fw_data, SZ_4K);
^
include/linux/kernel.h:738:2: note: in definition of macro '__min'
t2 min2 = (y); \
^~
>> drivers/fpga/cyclone-ps-spi.c:89:19: note: in expansion of macro 'min'
size_t stride = min(fw_data_end - fw_data, SZ_4K);
^~~
drivers/fpga/cyclone-ps-spi.c:89:46: note: each undeclared identifier is reported only once for each function it appears in
size_t stride = min(fw_data_end - fw_data, SZ_4K);
^
include/linux/kernel.h:738:2: note: in definition of macro '__min'
t2 min2 = (y); \
^~
>> drivers/fpga/cyclone-ps-spi.c:89:19: note: in expansion of macro 'min'
size_t stride = min(fw_data_end - fw_data, SZ_4K);
^~~
vim +/SZ_4K +89 drivers/fpga/cyclone-ps-spi.c
8 * Works on Cyclone V. Should work on cyclone series.
9 * May work on other Altera fpgas.
10 *
11 */
12
13 #include <linux/bitrev.h>
> 14 #include <linux/delay.h>
15 #include <linux/fpga/fpga-mgr.h>
16 #include <linux/gpio/consumer.h>
17 #include <linux/module.h>
18 #include <linux/of_gpio.h>
19 #include <linux/spi/spi.h>
20
21 #define FPGA_RESET_TIME 50 /* time in usecs to trigger FPGA config */
22 #define FPGA_MIN_DELAY 250 /* min usecs to wait for config status */
23
24 struct cyclonespi_conf {
25 struct gpio_desc *config;
26 struct gpio_desc *status;
27 struct spi_device *spi;
28 };
29
30 static const struct of_device_id of_ef_match[] = {
31 { .compatible = "altr,cyclone-ps-spi-fpga-mgr", },
32 {}
33 };
34 MODULE_DEVICE_TABLE(of, of_ef_match);
35
36 static enum fpga_mgr_states cyclonespi_state(struct fpga_manager *mgr)
37 {
38 return mgr->state;
39 }
40
41 static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
42 const char *buf, size_t count)
43 {
44 struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
45
46 if (flags & FPGA_MGR_PARTIAL_RECONFIG) {
47 dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
48 return -EINVAL;
49 }
50
51 gpiod_set_value(conf->config, 0);
52 usleep_range(FPGA_RESET_TIME, FPGA_RESET_TIME + 20);
53 if (gpiod_get_value(conf->status) == 1) {
54 dev_err(&mgr->dev, "Status pin should be low.\n");
55 return -EIO;
56 }
57
58 gpiod_set_value(conf->config, 1);
59 usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
60 if (gpiod_get_value(conf->status) == 0) {
61 dev_err(&mgr->dev, "Status pin not ready.\n");
62 return -EIO;
63 }
64
65 return 0;
66 }
67
68 static void rev_buf(void *buf, size_t len)
69 {
70 u32 *fw32 = (u32 *)buf;
71 const u32 *fw_end = (u32 *)(buf + len);
72
73 /* set buffer to lsb first */
74 while (fw32 < fw_end) {
75 *fw32 = bitrev8x4(*fw32);
76 fw32++;
77 }
78 }
79
80 static int cyclonespi_write(struct fpga_manager *mgr, const char *buf,
81 size_t count)
82 {
83 struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
84 const char *fw_data = buf;
85 const char *fw_data_end = fw_data + count;
86
87 while (fw_data < fw_data_end) {
88 int ret;
> 89 size_t stride = min(fw_data_end - fw_data, SZ_4K);
90
91 rev_buf((void *)fw_data, stride);
92 ret = spi_write(conf->spi, fw_data, stride);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 56833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161029/6a9787c0/attachment-0001.gz>
^ permalink raw reply
* [PATCH v2 1/5] lib: add bitrev8x4()
From: kbuild test robot @ 2016-10-28 18:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <bc92eb1507448731163ae67fc888668d327f9168.1477669745.git.stillcompiling@gmail.com>
Hi Joshua,
[auto build test WARNING on linus/master]
[also build test WARNING on v4.9-rc2 next-20161028]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Joshua-Clayton/lib-add-bitrev8x4/20161029-012535
config: arm-s5pv210_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All warnings (new ones prefixed by >>):
In file included from include/linux/bitrev.h:7:0,
from include/linux/crc32.h:9,
from lib/crc32.c:29:
arch/arm/include/asm/bitrev.h: In function '__arch_bitrev8x4':
>> arch/arm/include/asm/bitrev.h:23:1: warning: no return statement in function returning non-void [-Wreturn-type]
}
^
vim +23 arch/arm/include/asm/bitrev.h
7 return x;
8 }
9
10 static __always_inline __attribute_const__ u16 __arch_bitrev16(u16 x)
11 {
12 return __arch_bitrev32((u32)x) >> 16;
13 }
14
15 static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
16 {
17 return __arch_bitrev32((u32)x) >> 24;
18 }
19
20 static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
21 {
22 __asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
> 23 }
24
25 #endif
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 10244 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161029/c7c967b4/attachment-0001.gz>
^ permalink raw reply
* [PATCH V2] arm64: Add support of R_AARCH64_PREL32 relocation in purgatory
From: Geoff Levand @ 2016-10-28 18:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f970f7938109c40833fc45e3d6d72b1dcdf14595.1477627895.git.panand@redhat.com>
On 10/27/2016 09:16 PM, Pratyush Anand wrote:
> gcc version in fedora koji is 6.2.1-2.fc25. kexec-tools compiled with this
> gcc produced another relocation error:
>
> machine_apply_elf_rel: ERROR Unknown type: 261
>
> This patch fixes the above error.
Looks good. Simon, please apply.
Reviewed-by: Geoff Levand <geoff@infradead.org>
^ permalink raw reply
* SMR masking and PCI
From: Stuart Yoder @ 2016-10-28 18:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c39b785a-0f18-fc0a-ce08-7ebe3cfaf8c5@arm.com>
> -----Original Message-----
> From: Stuart Yoder
> Sent: Friday, October 28, 2016 12:12 PM
> To: 'Robin Murphy' <robin.murphy@arm.com>; Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel at lists.infradead.org; Will Deacon <will.deacon@arm.com>; Diana Madalina Craciun
> <diana.craciun@nxp.com>; Nipun Gupta <nipun.gupta@nxp.com>; iommu at lists.linux-foundation.org
> Subject: RE: SMR masking and PCI
>
>
>
> > -----Original Message-----
> > From: Robin Murphy [mailto:robin.murphy at arm.com]
> > Sent: Friday, October 28, 2016 11:17 AM
> > To: Stuart Yoder <stuart.yoder@nxp.com>; Mark Rutland <mark.rutland@arm.com>
> > Cc: linux-arm-kernel at lists.infradead.org; Will Deacon <will.deacon@arm.com>; Diana Madalina Craciun
> > <diana.craciun@nxp.com>; Nipun Gupta <nipun.gupta@nxp.com>; iommu at lists.linux-foundation.org
> > Subject: Re: SMR masking and PCI
> >
> > Hi Stuart,
> >
> > On 27/10/16 18:10, Stuart Yoder wrote:
> > > Hi Robin,
> > >
> > > A question about how the SMR masking defined in the arm,smmu binding
> > > relates to the PCI iommu-map.
> > >
> > > The #iommu-cells property defines the number of cells an "IOMMU specifier"
> > > takes and 2 is specified to be:
> > >
> > > SMMUs with stream matching support and complex masters
> > > may use a value of 2, where the second cell represents
> > > an SMR mask to combine with the ID in the first cell.
> > >
> > > An iommu-map entry is defined as:
> > >
> > > (rid-base,iommu,iommu-base,length)
> > >
> > > What seems to be currently missing in the iommu-map support is
> > > the possibility the case where #iommu-cells=<2>.
> >
> > Indeed. The bindings have so far rather implicitly assumed the case of
> > #{msi,iommu}-cells = 1, and the code has followed suit.
> >
> > > In this case iommu-base which is an IOMMU specifier should
> > > occupy 2 cells. For example on an ls2085a we would want:
> > >
> > > iommu-map = <0x0 0x6 0x7 0x3ff 0x1
> > > 0x100 0x6 0x8 0x3ff 0x1>;
> > >
> > > ...to mask our stream IDs to 10 bits.
> > >
> > > This should work in theory and comply with the bindings, no?
> >
> > In theory, but now consider:
> >
> > iommu-map = <0x0 0x6 0x7 0x3ff 0x2>;
> >
> > faced with ID 1. The input base is 0, the output base is the 2-cell
> > value 0x7000003ff, so the final ID value should be 0x700000400, right?
>
> No. The second cell as per the SMMU binding is the SMR mask...applied
> by the SMMU before matching stream IDs.
I think I now understand what you mean. I missed that you envisioned the
ID and mask being returned as a single unit and concatenated together...and
are split apart later by the SMMU driver.
> In our case we want to mask off the upper TBU ID bits that the SMMU tags
> onto the stream ID in our RID->SID LUT table.
>
> RID = 0
> SID in LUT and seen by SMMU = 7
> SMMU-500 TBU appends bits, making SID something like: 0xC07
> SMR mask of 0x3ff should be applied making the SID: 0x7
>
> > > of_pci_map_rid() seems to have a hardcoded assumption that
> > > each field in the map is 4 bytes.
> >
> > It does. I guess we should explicitly check that #{msi,iommu}-cells = 1
> > on the target node, and maybe clarify in the binding that that cell
> > should represent a plain linear ID value (although that's pretty obvious
> > from context IMO).
> >
> > > (Also, I guess that msi-map is not affected by this since it
> > > is not related to the IOMMU...but we do have common code
> > > handling both maps.)
> >
> > I'd say it's definitely affected, since #msi-cells can equally be more
> > than 1, and encodes an equally opaque value.
> >
> > It seems pretty reasonable to me that we could extend the binding to
> > accommodate #cells > 1 iff length == 1. Mark?
>
> I'm not following why the length matters.
Never mind the comment...think I follow now. Supporting #cells > 1 if
length == 1 sounds good.
Stuart
^ permalink raw reply
* [PATCH v2 3/3] reset: Add the TI SCI reset driver
From: Andrew F. Davis @ 2016-10-28 18:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANLsYkyiC9o9h_mq4D-6pMKNVAKQEnXR5rswiCVa8-mbtgRoJg@mail.gmail.com>
On 10/28/2016 12:43 PM, Mathieu Poirier wrote:
> On 27 October 2016 at 15:49, Andrew F. Davis <afd@ti.com> wrote:
>> Some TI Keystone family of SoCs contain a system controller (like the
>> Power Management Micro Controller (PMMC) on K2G SoCs) that manage the
>> low-level device control (like clocks, resets etc) for the various
>> hardware modules present on the SoC. These device control operations
>> are provided to the host processor OS through a communication protocol
>> called the TI System Control Interface (TI SCI) protocol.
>>
>> This patch adds a reset driver that communicates to the system
>> controller over the TI SCI protocol for performing reset management
>> of various devices present on the SoC. Various reset functionalities
>> are achieved by the means of different TI SCI device operations
>> provided by the TI SCI framework.
>>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>> [s-anna at ti.com: documentation changes, revised commit message]
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> Signed-off-by: Nishanth Menon <nm@ti.com>
>> ---
>> MAINTAINERS | 1 +
>> drivers/reset/Kconfig | 9 ++
>> drivers/reset/Makefile | 1 +
>> drivers/reset/reset-ti-sci.c | 262 +++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 273 insertions(+)
>> create mode 100644 drivers/reset/reset-ti-sci.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index accf991..b93d91a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11901,6 +11901,7 @@ F: include/dt-bindings/clock/k2g.h
>> F: drivers/clk/keystone/sci-clk.c
>> F: Documentation/devicetree/bindings/reset/ti,sci-reset.txt
>> F: include/dt-bindings/reset/k2g.h
>> +F: drivers/reset/reset-ti-sci.c
>>
>> THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
>> M: Hans Verkuil <hverkuil@xs4all.nl>
>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>> index 06d9fa2..4c21c9d 100644
>> --- a/drivers/reset/Kconfig
>> +++ b/drivers/reset/Kconfig
>> @@ -66,6 +66,15 @@ config RESET_SUNXI
>> help
>> This enables the reset driver for Allwinner SoCs.
>>
>> +config RESET_TI_SCI
>> + tristate "TI System Control Interface (TI-SCI) reset driver"
>> + depends on RESET_CONTROLLER
>> + depends on TI_SCI_PROTOCOL
>> + help
>> + This enables the reset driver support over TI System Control Interface
>> + available on some new TI SoCs. If you wish to use reset resources
>> + managed by the TI System Controller, say Y here. Otherwise, say N.
>> +
>> config TI_SYSCON_RESET
>> tristate "TI SYSCON Reset Driver"
>> depends on HAS_IOMEM
>> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
>> index bbe7026..36321f2 100644
>> --- a/drivers/reset/Makefile
>> +++ b/drivers/reset/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
>> obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
>> obj-$(CONFIG_RESET_STM32) += reset-stm32.o
>> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
>> +obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
>> obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
>> obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
>> obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
>> diff --git a/drivers/reset/reset-ti-sci.c b/drivers/reset/reset-ti-sci.c
>> new file mode 100644
>> index 0000000..42ccf12
>> --- /dev/null
>> +++ b/drivers/reset/reset-ti-sci.c
>> @@ -0,0 +1,262 @@
>> +/*
>> + * Texas Instrument's System Control Interface (TI-SCI) reset driver
>> + *
>> + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
>> + * Andrew F. Davis <afd@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/idr.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset-controller.h>
>> +#include <linux/soc/ti/ti_sci_protocol.h>
>> +
>> +/**
>> + * struct ti_sci_reset_control - reset control structure
>> + * @dev_id: SoC-specific device identifier
>> + * @reset_mask: reset mask to use for toggling reset
>> + */
>> +struct ti_sci_reset_control {
>> + u32 dev_id;
>> + u32 reset_mask;
>> +};
>> +
>> +/**
>> + * struct ti_sci_reset_data - reset controller information structure
>> + * @rcdev: reset controller entity
>> + * @dev: reset controller device pointer
>> + * @sci: TI SCI handle used for communication with system controller
>> + * @idr: idr structure for mapping ids to reset control structures
>> + */
>> +struct ti_sci_reset_data {
>> + struct reset_controller_dev rcdev;
>> + struct device *dev;
>> + const struct ti_sci_handle *sci;
>> + struct idr idr;
>> +};
>> +
>> +#define to_ti_sci_reset_data(p) \
>> + container_of((p), struct ti_sci_reset_data, rcdev)
>> +
>> +/**
>> + * ti_sci_reset_set() - program a device's reset
>> + * @rcdev: reset controller entity
>> + * @id: ID of the reset to toggle
>> + * @assert: boolean flag to indicate assert or deassert
>> + *
>> + * This is a common internal function used to assert or deassert a device's
>> + * reset using the TI SCI protocol. The device's reset is asserted if the
>> + * @assert argument is true, or deasserted if @assert argument is false.
>> + * The mechanism itself is a read-modify-write procedure, the current device
>> + * reset register is read using a TI SCI device operation, the new value is
>> + * set or un-set using the reset's mask, and the new reset value written by
>> + * using another TI SCI device operation.
>> + *
>> + * Return: 0 for successful request, else a corresponding error value
>> + */
>> +static int ti_sci_reset_set(struct reset_controller_dev *rcdev,
>> + unsigned long id, bool assert)
>> +{
>> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
>> + const struct ti_sci_handle *sci = data->sci;
>> + const struct ti_sci_dev_ops *dev_ops = &sci->ops.dev_ops;
>> + struct ti_sci_reset_control *control;
>> + u32 reset_state;
>> + int ret;
>> +
>> + control = idr_find(&data->idr, id);
>> + if (!control)
>> + return -EINVAL;
>> +
>> + ret = dev_ops->get_device_resets(sci, control->dev_id,
>> + &reset_state);
>> + if (ret)
>> + return ret;
>> +
>> + if (assert)
>> + reset_state |= control->reset_mask;
>> + else
>> + reset_state &= ~control->reset_mask;
>> +
>> + return dev_ops->set_device_resets(sci, control->dev_id,
>> + reset_state);
>> +}
>> +
>> +/**
>> + * ti_sci_reset_assert() - assert device reset
>> + * @rcdev: reset controller entity
>> + * @id: ID of the reset to be asserted
>> + *
>> + * This function implements the reset driver op to assert a device's reset
>> + * using the TI SCI protocol. This invokes the function ti_sci_reset_set()
>> + * with the corresponding parameters as passed in, but with the @assert
>> + * argument set to true for asserting the reset.
>> + *
>> + * Return: 0 for successful request, else a corresponding error value
>> + */
>> +static int ti_sci_reset_assert(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + return ti_sci_reset_set(rcdev, id, true);
>> +}
>> +
>> +/**
>> + * ti_sci_reset_deassert() - deassert device reset
>> + * @rcdev: reset controller entity
>> + * @id: ID of the reset to be deasserted
>> + *
>> + * This function implements the reset driver op to deassert a device's reset
>> + * using the TI SCI protocol. This invokes the function ti_sci_reset_set()
>> + * with the corresponding parameters as passed in, but with the @assert
>> + * argument set to false for deasserting the reset.
>> + *
>> + * Return: 0 for successful request, else a corresponding error value
>> + */
>> +static int ti_sci_reset_deassert(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + return ti_sci_reset_set(rcdev, id, false);
>> +}
>> +
>> +/**
>> + * ti_sci_reset_status() - check device reset status
>> + * @rcdev: reset controller entity
>> + * @id: ID of reset to be checked
>> + *
>> + * This function implements the reset driver op to return the status of a
>> + * device's reset using the TI SCI protocol. The reset register value is read
>> + * by invoking the TI SCI device opertation .get_device_resets(), and the
>> + * status of the specific reset is extracted and returned using this reset's
>> + * reset mask.
>> + *
>> + * Return: 0 if reset is deasserted, or a non-zero value if reset is asserted
>> + */
>> +static int ti_sci_reset_status(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
>> + const struct ti_sci_handle *sci = data->sci;
>> + const struct ti_sci_dev_ops *dev_ops = &sci->ops.dev_ops;
>> + struct ti_sci_reset_control *control;
>> + u32 reset_state;
>> + int ret;
>> +
>> + control = idr_find(&data->idr, id);
>> + if (!control)
>> + return -EINVAL;
>> +
>> + ret = dev_ops->get_device_resets(sci, control->dev_id,
>> + &reset_state);
>> + if (ret)
>> + return ret;
>> +
>> + return reset_state & control->reset_mask;
>> +}
>> +
>> +static struct reset_control_ops ti_sci_reset_ops = {
>> + .assert = ti_sci_reset_assert,
>> + .deassert = ti_sci_reset_deassert,
>> + .status = ti_sci_reset_status,
>> +};
>> +
>> +/**
>> + * ti_sci_reset_of_xlate() - translate a set of OF arguments to a reset ID
>> + * @rcdev: reset controller entity
>> + * @reset_spec: OF reset argument specifier
>> + *
>> + * This function performs the translation of the reset argument specifier
>> + * values defined in a reset consumer device node. The function allocates a
>> + * reset control structure for that device reset, and will be used by the
>> + * driver for performing any reset functions on that reset. An idr structure
>> + * is allocated and used to map to the reset control structure. This idr
>> + * is used by the driver to do reset lookups.
>> + *
>> + * Return: 0 for successful request, else a corresponding error value
>> + */
>> +static int ti_sci_reset_of_xlate(struct reset_controller_dev *rcdev,
>> + const struct of_phandle_args *reset_spec)
>> +{
>> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
>> + struct ti_sci_reset_control *control;
>> +
>> + if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
>> + return -EINVAL;
>> +
>> + control = devm_kzalloc(data->dev, sizeof(*control), GFP_KERNEL);
>> + if (!control)
>> + return -ENOMEM;
>> +
>> + control->dev_id = reset_spec->args[0];
>> + control->reset_mask = reset_spec->args[1];
>> +
>> + return idr_alloc(&data->idr, control, 0, 0, GFP_KERNEL);
>> +}
>> +
>> +static const struct of_device_id ti_sci_reset_of_match[] = {
>> + { .compatible = "ti,sci-reset", },
>> + { /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, ti_sci_reset_of_match);
>> +
>> +static int ti_sci_reset_probe(struct platform_device *pdev)
>> +{
>> + struct ti_sci_reset_data *data;
>> +
>> + if (!pdev->dev.of_node)
>> + return -ENODEV;
>> +
>> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + data->sci = devm_ti_sci_get_handle(&pdev->dev);
>> + if (IS_ERR(data->sci))
>> + return PTR_ERR(data->sci);
>> +
>> + data->rcdev.ops = &ti_sci_reset_ops;
>> + data->rcdev.owner = THIS_MODULE;
>> + data->rcdev.of_node = pdev->dev.of_node;
>> + data->rcdev.of_reset_n_cells = 2;
>> + data->rcdev.of_xlate = ti_sci_reset_of_xlate;
>> + data->dev = &pdev->dev;
>> + idr_init(&data->idr);
>
> Hello Andrew,
>
> For my own education, is there a specific reason to use a struct idr
> as opposed to keeping a pointer to a struct ti_sci_reset_control in
> truct ti_sci_reset_data? I'm not opposed to the way you've done
> things but simply keeping a pointer sound more intuitive to me.
>
> Thanks,
> Mathieu
>
Hi Mathieu,
There is one ti_sci_reset_data per reset controller, and each reset
controller can have many resets (each described by a
ti_sci_reset_control instance). When a consumer requests a reset we need
to both generate a unique ID to give back to the consumer to reference
this reset and then internally map this ID to a new ti_sci_reset_control
instance for later look-up when the consumer uses this reset, IDR does
both of these for us.
Thanks,
Andrew
>> +
>> + platform_set_drvdata(pdev, data);
>> +
>> + return reset_controller_register(&data->rcdev);
>> +}
>> +
>> +static int ti_sci_reset_remove(struct platform_device *pdev)
>> +{
>> + struct ti_sci_reset_data *data = platform_get_drvdata(pdev);
>> +
>> + reset_controller_unregister(&data->rcdev);
>> +
>> + idr_destroy(&data->idr);
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver ti_sci_reset_driver = {
>> + .probe = ti_sci_reset_probe,
>> + .remove = ti_sci_reset_remove,
>> + .driver = {
>> + .name = "ti-sci-reset",
>> + .of_match_table = ti_sci_reset_of_match,
>> + },
>> +};
>> +module_platform_driver(ti_sci_reset_driver);
>> +
>> +MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
>> +MODULE_DESCRIPTION("TI System Control Interface (TI SCI) Reset driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.10.1
>>
^ permalink raw reply
* [PATCH] fpga zynq: Check the bitstream for validity
From: Moritz Fischer @ 2016-10-28 18:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161028165555.GA17998@obsidianresearch.com>
On Fri, Oct 28, 2016 at 10:55:55AM -0600, Jason Gunthorpe wrote:
> On Fri, Oct 28, 2016 at 06:36:06PM +0200, Matthias Brugger wrote:
>
> > Sure but we are checking here that the bitstream passed to the kernel is
> > correct.
>
> The intent to check if it *possible* that the bitstream is
> correct. Correct means that DONE will assert after programming. A 4
> byte bitstream will never assert DONE.
>
> Arguably the threshold should be larger but we don't know what the
> true minimum is.
>
> > + /* All valid bitstreams are multiples of 32 bits */
> > + if (!count || (count % 4) != 0)
> > + return -EINVAL;
> > +
>
> Too clever for my taste.
>
> Aside from this, is the general idea even OK? In my world I cannonize
> the bitstream to start with the sync word, but others may not be doing
> that.
>
> I designed this patch based on the prior work to remove the
> auto-detection, eg that the driver should be passed a canonized
> bitstream.
I'm fine with checking for boundary cases where the bitstreams are
obviously too small or wrong size, I'm not convinced that checking using
internal knowledge about the bistream format inside the kernel is the
right place.
> The problem with the way it is now is how hard it is to figure out
> what the right bitstream format should be. Clear instructions to
> canonize by droping the header before the sync word and byteswap so
> the sync word is in the given order is much clearer..
I don't know about you, but for my designs I can just use what drops out
of my Vivado build. Are you unhappy with the way we document which
format to use, or that we don't slice off the beginning (that gets
ignored by hardware?).
Thanks for addressing the issues with the length calculations,
Moritz
^ permalink raw reply
* [PATCH v12 RESEND 0/4] generic TEE subsystem
From: Mark Brown @ 2016-10-28 18:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1e532aeb-4944-62e4-c8c4-1e23438b92cd@ti.com>
On Fri, Oct 28, 2016 at 10:43:24AM -0500, Andrew F. Davis wrote:
> Do we see this as a chicken and egg situation, or is there any harm
> beyond the pains of supporting an out-of-tree driver for a while, to
> wait until we have at least one other TEE to add to this subsystem
> before merging?
We haven't been overburneded with TEE vendors wanting to get their
driver code into mainline - do we have any reasonable prospect of other
TEE vendors with an interest in mainline turning up in any kind of
reasonable timeframe?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161028/3adb9948/attachment.sig>
^ permalink raw reply
* [PATCH v6] soc: qcom: add l2 cache perf events driver
From: Leeder, Neil @ 2016-10-28 18:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161028160213.GH1076@arm.com>
On 10/28/2016 12:02 PM, Will Deacon wrote:
> On Tue, Oct 04, 2016 at 12:25:54PM -0400, Neil Leeder wrote:
>> Thanks Mark. I'll move it, rebase on 4.9-rc1 and run perf fuzzer.
>
> Did the fuzzer explode, or do you have a new version you can post?
>
> Will
Hi Will,
I was delayed by some logistical problems, but the fuzzer ran fine and I
will post the updated patch shortly - sorry for the delay.
Neil
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH v2] spi: spi-fsl-dspi: Add DMA support for Vybrid
From: Mark Brown @ 2016-10-28 18:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161017062308.GA3478@Sanchayan-Arch.localdomain>
On Mon, Oct 17, 2016 at 11:53:08AM +0530, maitysanchayan at gmail.com wrote:
> Hello,
>
> Ping?
Please don't send content free pings and please allow a reasonable time
for review. People get busy, go on holiday, attend conferences and so
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review. If there have been
review comments then people may be waiting for those to be addressed.
Sending content free pings just adds to the mail volume (if they are
seen at all) and if something has gone wrong you'll have to resend the
patches anyway.
Please don't top post, reply in line with needed context. This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161028/2a48a59a/attachment.sig>
^ permalink raw reply
* [PATCH v2] phy: sun4i: check PMU presence when poking unknown bit of pmu
From: Hans de Goede @ 2016-10-28 18:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161028162701.4531-1-icenowy@aosc.xyz>
Hi,
On 28-10-16 18:27, Icenowy Zheng wrote:
> Allwinner SoC's PHY 0, when used as OTG controller, have no pmu part.
> The code that poke some unknown bit of PMU for H3/A64 didn't check
> the PHY, and will cause kernel oops when PHY 0 is used.
>
> This patch will check whether the pmu is not NULL before poking.
>
> Fixes: b3e0d141ca9f (phy: sun4i: add support for A64 usb phy)
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Patch LGTM too:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
> ---
> drivers/phy/phy-sun4i-usb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
> index b9342a2..fec34f5 100644
> --- a/drivers/phy/phy-sun4i-usb.c
> +++ b/drivers/phy/phy-sun4i-usb.c
> @@ -264,7 +264,7 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> return ret;
> }
>
> - if (data->cfg->enable_pmu_unk1) {
> + if (phy->pmu && data->cfg->enable_pmu_unk1) {
> val = readl(phy->pmu + REG_PMU_UNK1);
> writel(val & ~2, phy->pmu + REG_PMU_UNK1);
> }
>
^ permalink raw reply
* [PATCH v3 1/2] clk: imx: fix integer overflow in AV PLL round rate
From: Stephen Boyd @ 2016-10-28 18:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOMZO5Co32XUpNFz4Hd2DEmNnMBvD5fKmsyjcmsgawea4dJR9g@mail.gmail.com>
On 10/28, Fabio Estevam wrote:
> On Thu, Oct 27, 2016 at 11:41 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 10/12, Emil Lundmark wrote:
> >> Since 'parent_rate * mfn' may overflow 32 bits, the result should be
> >> stored using 64 bits.
> >>
> >> The problem was discovered when trying to set the rate of the audio PLL
> >> (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but
> >> the actual rate returned was 192.000570 MHz. The round rate function should
> >> have been able to return 196.608 MHz, i.e., the desired rate.
> >>
> >> Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula")
> >> Cc: Anson Huang <b20788@freescale.com>
> >> Signed-off-by: Emil Lundmark <emil@limesaudio.com>
> >> ---
> >
> > Applied to clk-next
>
> This one fixes a regression caused by ba7f4f557eb6 ("clk: imx: correct
> AV PLL rate formula").
>
> So it should go to clk-fixes instead with the stable tag:
>
> Cc: <stable@vger.kernel.org> # 4.8.x
>
The clk-fixes branch is for patches that fix problems in code
merged during the merge window as well as small one-liners and
things that are causing great pain for people on the latest
release. Given that this fixes a regression in v4.8 and we're
trying to stabilize v4.9 it looked like it could wait until
v4.10.
So is there something going on here where the pain is too much to
wait for the next merge window? If so the commit text should
mention something about what's causing that pain. Perhaps by
referencing the commit that merged outside of clk tree that
caused problems.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [linux-sunxi] [PATCH RESEND 1/2] dt: bindings: add allwinner,otg-routed property for phy-sun4i-usb
From: Hans de Goede @ 2016-10-28 18:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <86c3fad4-e0c1-9aaf-76c5-b9428110464f@redhat.com>
HI,
On 26-10-16 12:14, Hans de Goede wrote:
> Hi,
>
> On 26-10-16 10:52, Icenowy Zheng wrote:
>>
>>
>> 26.10.2016, 16:28, "Hans de Goede" <hdegoede@redhat.com>:
>>> Hi,
>>>
>>> On 25-10-16 06:11, Icenowy Zheng wrote:
>>>> On some newer Allwinner SoCs (H3 or A64), the PHY0 can be either routed to
>>>> the MUSB controller (which is an OTG controller) or the OHCI/EHCI pair
>>>> (which is a Host-only controller, but more stable and easy to implement).
>>>>
>>>> This property marks whether on a certain board which controller should be
>>>> attached to the PHY.
>>>>
>>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>>>
>>> Icenowy, I appreciate your work on this, but we really need full otg
>>> support with dynamic switching rather then hardwiring the routing, so
>>> this cannot go in as is.
>>
>> Now I have both PHY0 controllers' drivers.
>>
>> In the tree of https://github.com/Icenowy/linux/tree/ice-a64-v6.1 , I have already
>> enabled MUSB controller.
>>
>> And this patchset is for those prefer a stable USB host implement to dual-role
>> implementation. MUSB is a good UDC, but not a good host controller. My USB
>> sound card cannot work on MUSB on A33. Even connecting a R8's MUSB (Serial
>> Gadget) to an A33's MUSB cannot work.
>
> The idea is for dual-role setups to used the MUSB in gadget mode and the EHCI/OHCI
> pair when in host mode. So for otg setups you would runtime change the mux
> from one controller to the other based on the id pin value.
>
> Take a look at drivers/phy/phy-sun4i-usb.c, around line 512:
>
> if (id_det != data->id_det) {
> ...
> }
>
> This deals with id_det changes (including the initial id_det "change"
> for hardwired host-only ports). This currently assumes that the musb
> will be used for host mode too, we will want to change this to
> something like this:
>
> if (id_det != data->id_det) {
> if (data->cfg->separate_phy0_host_controller) {
> if (id_det) {
> /* Change to gadget mode (id_det == 1), switch phy mux to musb */
> actual code to switch phy mux to musb...
> } else {
> /* Change to host mode (id_det == 0), switch phy mux to ehci/ohci */
> actual code to switch phy mux to ehci/ohci...
> }
> }
> /* old code */
> }
>
> Note this will then still rely on the musb code to actually turn
> the regulator on, so you do need to have the musb driver build and
> loaded. This can be fixed but lets start with the above.
>
> If you combine this with dr_mode = "host"; in the dts, then
> sun4i_usb_phy0_get_id_det() will return 0 so on its first run
> sun4i_usb_phy0_id_vbus_det_scan() will throw the mux to the ehci/ohci
> and everything should work as you want without needing the custom
> "allwinner,otg-routed" property, and we should be more or less
> ready to support full otg on other boards.
I've just found further proof that the musb on the H3 at least
only is intended for gadget mode and that we must dynamically
switch for host-mode. If you look at:
drivers/usb/sunxi_usb/include/sunxi_udc.h
In the h3 sdk then you will see that for the H3 a different fifo
endpoint table is used, as the total fifo space is only 4k where
as previous SoCs had 8k. This means that we need to have 2
different ep tables in drivers/usb/musb/sunxi.c and select by
compatible.
Regards,
Hans
^ permalink raw reply
* [PATCH] staging: vc04_services: call sg_init_table to init scatterlist
From: Michael Zoran @ 2016-10-28 17:58 UTC (permalink / raw)
To: linux-arm-kernel
Call the sg_init_table function to correctly initialze
the DMA scatterlist. This function is required to completely
initialize the list and is mandatory if DMA debugging is
enabled in the build configuration.
One of the purposes of sg_init_table is to set
the magic "cookie" on each list element and ensure
the chain end is marked.
Signed-off-by: Michael Zoran <mzoran@crowfest.net>
---
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 6fa2b5a..21b26e5 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -464,6 +464,12 @@ create_pagelist(char __user *buf, size_t count, unsigned short type,
pagelist->type = type;
pagelist->offset = offset;
+ /*
+ * Initialize the scatterlist so that the magic cookie
+ * is filled if debugging is enabled
+ */
+ sg_init_table(scatterlist, num_pages);
+ /* Now set the pages for each scatterlist */
for (i = 0; i < num_pages; i++)
sg_set_page(scatterlist + i, pages[i], PAGE_SIZE, 0);
--
2.10.1
^ permalink raw reply related
* [PATCH] [ARM] Fix stack alignment when processing backtraces
From: Jason Gunthorpe @ 2016-10-28 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161018170510.GA12248@obsidianresearch.com>
On Tue, Oct 18, 2016 at 11:05:10AM -0600, Jason Gunthorpe wrote:
> The dumpstm helper within c_backtrace pushed 5 dwords onto the stack
> causing the stack to become unaligned and then calls printk. This
> causes memory corruption in the kernel which assumes AAPCS calling
> convention.
>
> Since this bit of asm doesn't use the standard prologue just add
> another register to restore alignment.
>
> Fixes: 7ab3f8d595a1b ("[ARM] Add ability to dump exception stacks to kernel backtraces")
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> arch/arm/lib/backtrace.S | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> In my case the kernel was hitting a WARN_ON during boot and then
> reliably failed to start the compiled-in initramfs.
>
> I'm inferring that the stack misalignment caused some kind of memory
> corruption which wiped out the unpacked initramfs.
>
> Saw with gcc 5.4.0 on a kirkwood armv5te
Since there are no comments, I will send this to RMK's patch system..
Thanks,
Jason
> diff --git a/arch/arm/lib/backtrace.S b/arch/arm/lib/backtrace.S
> index fab5a50503ae..25e1cce19991 100644
> +++ b/arch/arm/lib/backtrace.S
> @@ -116,7 +116,8 @@ ENDPROC(c_backtrace)
> #define reg r5
> #define stack r6
>
> -.Ldumpstm: stmfd sp!, {instr, reg, stack, r7, lr}
> + /* Must maintain 8 byte stack alignment */
> +.Ldumpstm: stmfd sp!, {r3, instr, reg, stack, r7, lr}
> mov stack, r0
> mov instr, r1
> mov reg, #10
> @@ -140,7 +141,7 @@ ENDPROC(c_backtrace)
> teq r7, #0
> adrne r0, .Lcr
> blne printk
> - ldmfd sp!, {instr, reg, stack, r7, pc}
> + ldmfd sp!, {r3, instr, reg, stack, r7, pc}
>
> .Lfp: .asciz " r%d:%08x%s"
> .Lcr: .asciz "\n"
^ permalink raw reply
* [PATCH] ASoC: samsung: Drop AC97 drivers
From: Mark Brown @ 2016-10-28 17:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477671072-742-1-git-send-email-s.nawrocki@samsung.com>
On Fri, Oct 28, 2016 at 06:11:12PM +0200, Sylwester Nawrocki wrote:
> The AC97 drivers are broken and it seems these have not been used
> for a long time. This patch removes the unused code, i.e. Samsung
> SoC AC97 controller driver and related machine drivers:
> ln2440sbc_alc650, smdk2443_wm9710, smdk_wm9713.
This needs rebasing against some of the recent fixes, sorry.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161028/cddf7f0f/attachment.sig>
^ permalink raw reply
* [PATCH v2 3/3] reset: Add the TI SCI reset driver
From: Mathieu Poirier @ 2016-10-28 17:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161027214941.24641-4-afd@ti.com>
On 27 October 2016 at 15:49, Andrew F. Davis <afd@ti.com> wrote:
> Some TI Keystone family of SoCs contain a system controller (like the
> Power Management Micro Controller (PMMC) on K2G SoCs) that manage the
> low-level device control (like clocks, resets etc) for the various
> hardware modules present on the SoC. These device control operations
> are provided to the host processor OS through a communication protocol
> called the TI System Control Interface (TI SCI) protocol.
>
> This patch adds a reset driver that communicates to the system
> controller over the TI SCI protocol for performing reset management
> of various devices present on the SoC. Various reset functionalities
> are achieved by the means of different TI SCI device operations
> provided by the TI SCI framework.
>
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> [s-anna at ti.com: documentation changes, revised commit message]
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> MAINTAINERS | 1 +
> drivers/reset/Kconfig | 9 ++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-ti-sci.c | 262 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 273 insertions(+)
> create mode 100644 drivers/reset/reset-ti-sci.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index accf991..b93d91a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11901,6 +11901,7 @@ F: include/dt-bindings/clock/k2g.h
> F: drivers/clk/keystone/sci-clk.c
> F: Documentation/devicetree/bindings/reset/ti,sci-reset.txt
> F: include/dt-bindings/reset/k2g.h
> +F: drivers/reset/reset-ti-sci.c
>
> THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
> M: Hans Verkuil <hverkuil@xs4all.nl>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 06d9fa2..4c21c9d 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -66,6 +66,15 @@ config RESET_SUNXI
> help
> This enables the reset driver for Allwinner SoCs.
>
> +config RESET_TI_SCI
> + tristate "TI System Control Interface (TI-SCI) reset driver"
> + depends on RESET_CONTROLLER
> + depends on TI_SCI_PROTOCOL
> + help
> + This enables the reset driver support over TI System Control Interface
> + available on some new TI SoCs. If you wish to use reset resources
> + managed by the TI System Controller, say Y here. Otherwise, say N.
> +
> config TI_SYSCON_RESET
> tristate "TI SYSCON Reset Driver"
> depends on HAS_IOMEM
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index bbe7026..36321f2 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
> obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
> obj-$(CONFIG_RESET_STM32) += reset-stm32.o
> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> +obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
> obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
> obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
> obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
> diff --git a/drivers/reset/reset-ti-sci.c b/drivers/reset/reset-ti-sci.c
> new file mode 100644
> index 0000000..42ccf12
> --- /dev/null
> +++ b/drivers/reset/reset-ti-sci.c
> @@ -0,0 +1,262 @@
> +/*
> + * Texas Instrument's System Control Interface (TI-SCI) reset driver
> + *
> + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
> + * Andrew F. Davis <afd@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/idr.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/soc/ti/ti_sci_protocol.h>
> +
> +/**
> + * struct ti_sci_reset_control - reset control structure
> + * @dev_id: SoC-specific device identifier
> + * @reset_mask: reset mask to use for toggling reset
> + */
> +struct ti_sci_reset_control {
> + u32 dev_id;
> + u32 reset_mask;
> +};
> +
> +/**
> + * struct ti_sci_reset_data - reset controller information structure
> + * @rcdev: reset controller entity
> + * @dev: reset controller device pointer
> + * @sci: TI SCI handle used for communication with system controller
> + * @idr: idr structure for mapping ids to reset control structures
> + */
> +struct ti_sci_reset_data {
> + struct reset_controller_dev rcdev;
> + struct device *dev;
> + const struct ti_sci_handle *sci;
> + struct idr idr;
> +};
> +
> +#define to_ti_sci_reset_data(p) \
> + container_of((p), struct ti_sci_reset_data, rcdev)
> +
> +/**
> + * ti_sci_reset_set() - program a device's reset
> + * @rcdev: reset controller entity
> + * @id: ID of the reset to toggle
> + * @assert: boolean flag to indicate assert or deassert
> + *
> + * This is a common internal function used to assert or deassert a device's
> + * reset using the TI SCI protocol. The device's reset is asserted if the
> + * @assert argument is true, or deasserted if @assert argument is false.
> + * The mechanism itself is a read-modify-write procedure, the current device
> + * reset register is read using a TI SCI device operation, the new value is
> + * set or un-set using the reset's mask, and the new reset value written by
> + * using another TI SCI device operation.
> + *
> + * Return: 0 for successful request, else a corresponding error value
> + */
> +static int ti_sci_reset_set(struct reset_controller_dev *rcdev,
> + unsigned long id, bool assert)
> +{
> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
> + const struct ti_sci_handle *sci = data->sci;
> + const struct ti_sci_dev_ops *dev_ops = &sci->ops.dev_ops;
> + struct ti_sci_reset_control *control;
> + u32 reset_state;
> + int ret;
> +
> + control = idr_find(&data->idr, id);
> + if (!control)
> + return -EINVAL;
> +
> + ret = dev_ops->get_device_resets(sci, control->dev_id,
> + &reset_state);
> + if (ret)
> + return ret;
> +
> + if (assert)
> + reset_state |= control->reset_mask;
> + else
> + reset_state &= ~control->reset_mask;
> +
> + return dev_ops->set_device_resets(sci, control->dev_id,
> + reset_state);
> +}
> +
> +/**
> + * ti_sci_reset_assert() - assert device reset
> + * @rcdev: reset controller entity
> + * @id: ID of the reset to be asserted
> + *
> + * This function implements the reset driver op to assert a device's reset
> + * using the TI SCI protocol. This invokes the function ti_sci_reset_set()
> + * with the corresponding parameters as passed in, but with the @assert
> + * argument set to true for asserting the reset.
> + *
> + * Return: 0 for successful request, else a corresponding error value
> + */
> +static int ti_sci_reset_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return ti_sci_reset_set(rcdev, id, true);
> +}
> +
> +/**
> + * ti_sci_reset_deassert() - deassert device reset
> + * @rcdev: reset controller entity
> + * @id: ID of the reset to be deasserted
> + *
> + * This function implements the reset driver op to deassert a device's reset
> + * using the TI SCI protocol. This invokes the function ti_sci_reset_set()
> + * with the corresponding parameters as passed in, but with the @assert
> + * argument set to false for deasserting the reset.
> + *
> + * Return: 0 for successful request, else a corresponding error value
> + */
> +static int ti_sci_reset_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return ti_sci_reset_set(rcdev, id, false);
> +}
> +
> +/**
> + * ti_sci_reset_status() - check device reset status
> + * @rcdev: reset controller entity
> + * @id: ID of reset to be checked
> + *
> + * This function implements the reset driver op to return the status of a
> + * device's reset using the TI SCI protocol. The reset register value is read
> + * by invoking the TI SCI device opertation .get_device_resets(), and the
> + * status of the specific reset is extracted and returned using this reset's
> + * reset mask.
> + *
> + * Return: 0 if reset is deasserted, or a non-zero value if reset is asserted
> + */
> +static int ti_sci_reset_status(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
> + const struct ti_sci_handle *sci = data->sci;
> + const struct ti_sci_dev_ops *dev_ops = &sci->ops.dev_ops;
> + struct ti_sci_reset_control *control;
> + u32 reset_state;
> + int ret;
> +
> + control = idr_find(&data->idr, id);
> + if (!control)
> + return -EINVAL;
> +
> + ret = dev_ops->get_device_resets(sci, control->dev_id,
> + &reset_state);
> + if (ret)
> + return ret;
> +
> + return reset_state & control->reset_mask;
> +}
> +
> +static struct reset_control_ops ti_sci_reset_ops = {
> + .assert = ti_sci_reset_assert,
> + .deassert = ti_sci_reset_deassert,
> + .status = ti_sci_reset_status,
> +};
> +
> +/**
> + * ti_sci_reset_of_xlate() - translate a set of OF arguments to a reset ID
> + * @rcdev: reset controller entity
> + * @reset_spec: OF reset argument specifier
> + *
> + * This function performs the translation of the reset argument specifier
> + * values defined in a reset consumer device node. The function allocates a
> + * reset control structure for that device reset, and will be used by the
> + * driver for performing any reset functions on that reset. An idr structure
> + * is allocated and used to map to the reset control structure. This idr
> + * is used by the driver to do reset lookups.
> + *
> + * Return: 0 for successful request, else a corresponding error value
> + */
> +static int ti_sci_reset_of_xlate(struct reset_controller_dev *rcdev,
> + const struct of_phandle_args *reset_spec)
> +{
> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
> + struct ti_sci_reset_control *control;
> +
> + if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
> + return -EINVAL;
> +
> + control = devm_kzalloc(data->dev, sizeof(*control), GFP_KERNEL);
> + if (!control)
> + return -ENOMEM;
> +
> + control->dev_id = reset_spec->args[0];
> + control->reset_mask = reset_spec->args[1];
> +
> + return idr_alloc(&data->idr, control, 0, 0, GFP_KERNEL);
> +}
> +
> +static const struct of_device_id ti_sci_reset_of_match[] = {
> + { .compatible = "ti,sci-reset", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, ti_sci_reset_of_match);
> +
> +static int ti_sci_reset_probe(struct platform_device *pdev)
> +{
> + struct ti_sci_reset_data *data;
> +
> + if (!pdev->dev.of_node)
> + return -ENODEV;
> +
> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->sci = devm_ti_sci_get_handle(&pdev->dev);
> + if (IS_ERR(data->sci))
> + return PTR_ERR(data->sci);
> +
> + data->rcdev.ops = &ti_sci_reset_ops;
> + data->rcdev.owner = THIS_MODULE;
> + data->rcdev.of_node = pdev->dev.of_node;
> + data->rcdev.of_reset_n_cells = 2;
> + data->rcdev.of_xlate = ti_sci_reset_of_xlate;
> + data->dev = &pdev->dev;
> + idr_init(&data->idr);
Hello Andrew,
For my own education, is there a specific reason to use a struct idr
as opposed to keeping a pointer to a struct ti_sci_reset_control in
truct ti_sci_reset_data? I'm not opposed to the way you've done
things but simply keeping a pointer sound more intuitive to me.
Thanks,
Mathieu
> +
> + platform_set_drvdata(pdev, data);
> +
> + return reset_controller_register(&data->rcdev);
> +}
> +
> +static int ti_sci_reset_remove(struct platform_device *pdev)
> +{
> + struct ti_sci_reset_data *data = platform_get_drvdata(pdev);
> +
> + reset_controller_unregister(&data->rcdev);
> +
> + idr_destroy(&data->idr);
> +
> + return 0;
> +}
> +
> +static struct platform_driver ti_sci_reset_driver = {
> + .probe = ti_sci_reset_probe,
> + .remove = ti_sci_reset_remove,
> + .driver = {
> + .name = "ti-sci-reset",
> + .of_match_table = ti_sci_reset_of_match,
> + },
> +};
> +module_platform_driver(ti_sci_reset_driver);
> +
> +MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
> +MODULE_DESCRIPTION("TI System Control Interface (TI SCI) Reset driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.10.1
>
^ permalink raw reply
* SMR masking and PCI
From: Mark Rutland @ 2016-10-28 17:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c39b785a-0f18-fc0a-ce08-7ebe3cfaf8c5@arm.com>
Hi,
On Fri, Oct 28, 2016 at 05:16:37PM +0100, Robin Murphy wrote:
> On 27/10/16 18:10, Stuart Yoder wrote:
> > A question about how the SMR masking defined in the arm,smmu binding
> > relates to the PCI iommu-map.
> >
> > The #iommu-cells property defines the number of cells an "IOMMU specifier"
> > takes and 2 is specified to be:
> >
> > SMMUs with stream matching support and complex masters
> > may use a value of 2, where the second cell represents
> > an SMR mask to combine with the ID in the first cell.
> >
> > An iommu-map entry is defined as:
> >
> > (rid-base,iommu,iommu-base,length)
> >
> > What seems to be currently missing in the iommu-map support is
> > the possibility the case where #iommu-cells=<2>.
>
> Indeed. The bindings have so far rather implicitly assumed the case of
> #{msi,iommu}-cells = 1, and the code has followed suit.
The intention was that neither the iommu or msi bindings had such a
requirement, but evidently I did not specify the intended behaviour
thoroughly enough.
I had intended that the offset was added to the final cell of the
iommu-specifier (i.e. that the iommu-specifier was treated as a large
number).
You can handle this case by adding additional entries in the map table,
with a single entry length.
> > In this case iommu-base which is an IOMMU specifier should
> > occupy 2 cells. For example on an ls2085a we would want:
> >
> > iommu-map = <0x0 0x6 0x7 0x3ff 0x1
> > 0x100 0x6 0x8 0x3ff 0x1>;
> >
> > ...to mask our stream IDs to 10 bits.
> >
> > This should work in theory and comply with the bindings, no?
>
> In theory, but now consider:
>
> iommu-map = <0x0 0x6 0x7 0x3ff 0x2>;
>
> faced with ID 1. The input base is 0, the output base is the 2-cell
> value 0x7000003ff, so the final ID value should be 0x700000400, right?
That was the intended behaviour, yes.
> > (Also, I guess that msi-map is not affected by this since it
> > is not related to the IOMMU...but we do have common code
> > handling both maps.)
>
> I'd say it's definitely affected, since #msi-cells can equally be more
> than 1, and encodes an equally opaque value.
Yes.
> It seems pretty reasonable to me that we could extend the binding to
> accommodate #cells > 1 iff length == 1. Mark?
I will try to come up with the wording to make the above explicit, for
both bindings.
Thanks,
Mark.
^ permalink raw reply
* [RFC][PATCH] arm64: Add support for CONFIG_DEBUG_VIRTUAL
From: Laura Abbott @ 2016-10-28 17:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161028144951.GI5806@leverpostej>
On 10/28/2016 07:49 AM, Mark Rutland wrote:
> Hi Laura,
>
> On Thu, Oct 27, 2016 at 05:18:12PM -0700, Laura Abbott wrote:
>> x86 has an option CONFIG_DEBUG_VIRTUAL to do additional checks
>> on virt_to_phys calls. The goal is to catch users who are calling
>> virt_to_phys on non-linear addresses immediately. As features
>> such as CONFIG_VMAP_STACK get enabled for arm64, this becomes
>> increasingly important. Add checks to catch bad virt_to_phys
>> usage.
>>
>> Signed-off-by: Laura Abbott <labbott@redhat.com>
>> ---
>> This has been on my TODO list for a while. It caught a few bugs with
>> CONFIG_VMAP_STACK on x86 so when that eventually gets added
>> for arm64 it will be useful to have. This caught one driver calling __pa on an
>> ioremapped address already.
>
> This is fantastic; thanks for putting this together!
>
>> RFC for a couple of reasons:
>>
>> 1) This is basically a direct port of the x86 approach.
>> 2) I needed some #ifndef __ASSEMBLY__ which I don't like to throw around.
>> 3) I'm not quite sure about the bounds check for the VIRTUAL_BUG_ON with KASLR,
>> specifically the _end check.
>> 4) Is it worth actually keeping this as DEBUG_VIRTUAL vs. folding it into
>> another option?
>
> I think it's worth aligning with x86, so modulo a couple of comments
> below, (1) and (4) seem fine. I think (2) just requires an additional
> shuffle.
>
>> ---
>> arch/arm64/include/asm/memory.h | 11 ++++++++++-
>> arch/arm64/mm/Makefile | 2 +-
>> arch/arm64/mm/physaddr.c | 17 +++++++++++++++++
>> lib/Kconfig.debug | 2 +-
>> mm/cma.c | 2 +-
>> 5 files changed, 30 insertions(+), 4 deletions(-)
>> create mode 100644 arch/arm64/mm/physaddr.c
>>
>> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
>> index ba62df8..9805adc 100644
>> --- a/arch/arm64/include/asm/memory.h
>> +++ b/arch/arm64/include/asm/memory.h
>> @@ -106,11 +106,19 @@
>> * private definitions which should NOT be used outside memory.h
>> * files. Use virt_to_phys/phys_to_virt/__pa/__va instead.
>> */
>> -#define __virt_to_phys(x) ({ \
>> +#define __virt_to_phys_nodebug(x) ({ \
>> phys_addr_t __x = (phys_addr_t)(x); \
>> __x & BIT(VA_BITS - 1) ? (__x & ~PAGE_OFFSET) + PHYS_OFFSET : \
>> (__x - kimage_voffset); })
>>
>> +#ifdef CONFIG_DEBUG_VIRTUAL
>> +#ifndef __ASSEMBLY__
>> +extern unsigned long __virt_to_phys(unsigned long x);
>> +#endif
>> +#else
>> +#define __virt_to_phys(x) __virt_to_phys_nodebug(x)
>> +#endif
>> +
>> #define __phys_to_virt(x) ((unsigned long)((x) - PHYS_OFFSET) | PAGE_OFFSET)
>> #define __phys_to_kimg(x) ((unsigned long)((x) + kimage_voffset))
>
> I think we can move all the existing conversion logic here (including
> into page_to_phys/phys_to_page) into the existing #ifndef __ASSEMBLY__
> block at the end of the file. Assembly clearly can't use these, and we
> already have virt_to_phys and others in that #ifndef.
>
> Assuming that works, would you mind doing that as a preparatory patch?
>
Sure.
>> @@ -202,6 +210,7 @@ static inline void *phys_to_virt(phys_addr_t x)
>> * Drivers should NOT use these either.
>> */
>> #define __pa(x) __virt_to_phys((unsigned long)(x))
>> +#define __pa_nodebug(x) __virt_to_phys_nodebug((unsigned long)(x))
>> #define __va(x) ((void *)__phys_to_virt((phys_addr_t)(x)))
>> #define pfn_to_kaddr(pfn) __va((pfn) << PAGE_SHIFT)
>> #define virt_to_pfn(x) __phys_to_pfn(__virt_to_phys(x))
>> diff --git a/arch/arm64/mm/Makefile b/arch/arm64/mm/Makefile
>> index 54bb209..bcea84e 100644
>> --- a/arch/arm64/mm/Makefile
>> +++ b/arch/arm64/mm/Makefile
>> @@ -5,6 +5,6 @@ obj-y := dma-mapping.o extable.o fault.o init.o \
>> obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
>> obj-$(CONFIG_ARM64_PTDUMP) += dump.o
>> obj-$(CONFIG_NUMA) += numa.o
>> -
>> +obj-$(CONFIG_DEBUG_VIRTUAL) += physaddr.o
>> obj-$(CONFIG_KASAN) += kasan_init.o
>> KASAN_SANITIZE_kasan_init.o := n
>> diff --git a/arch/arm64/mm/physaddr.c b/arch/arm64/mm/physaddr.c
>> new file mode 100644
>> index 0000000..6c271e2
>> --- /dev/null
>> +++ b/arch/arm64/mm/physaddr.c
>> @@ -0,0 +1,17 @@
>> +#include <linux/mm.h>
>> +
>> +#include <asm/memory.h>
>> +
>> +unsigned long __virt_to_phys(unsigned long x)
>> +{
>> + phys_addr_t __x = (phys_addr_t)x;
>> +
>> + if (__x & BIT(VA_BITS - 1)) {
>> + /* The bit check ensures this is the right range */
>> + return (__x & ~PAGE_OFFSET) + PHYS_OFFSET;
>> + } else {
>> + VIRTUAL_BUG_ON(x < kimage_vaddr || x > (unsigned long)_end);
>
> IIUC, in (3) you were asking if the last check should be '>' or '>='?
>
> To match high_memory, I suspect the latter, as _end doesn't fall within
> the mapped virtual address space.
>
I was actually concerned about if _end would be correct with KASLR.
Ard confirmed that it gets fixed up to be correct. I'll change the
check to check for >=.
>> + return (__x - kimage_voffset);
>> + }
>> +}
>> +EXPORT_SYMBOL(__virt_to_phys);
>
> It's a bit annoying that we have to duplicate the logic here to add the
> VIRTUAL_BUG_ON(), but I see that x86 also do this, and I don't have a
> better suggestion.
>
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 33bc56c..e5634bb 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -604,7 +604,7 @@ config DEBUG_VM_PGFLAGS
>>
>> config DEBUG_VIRTUAL
>> bool "Debug VM translations"
>> - depends on DEBUG_KERNEL && X86
>> + depends on DEBUG_KERNEL && (X86 || ARM64)
>
> I have no strong feelings about this, but perhaps it's nicer to have
> something like ARCH_HAS_DEBUG_VIRTUAL?
>
Yes, if this ever gets added for other arches this will start to get
unwieldy. I'll look at cleaning that up.
>> help
>> Enable some costly sanity checks in virtual to page code. This can
>> catch mistakes with virt_to_page() and friends.
>> diff --git a/mm/cma.c b/mm/cma.c
>> index 384c2cb..2345803 100644
>> --- a/mm/cma.c
>> +++ b/mm/cma.c
>> @@ -235,7 +235,7 @@ int __init cma_declare_contiguous(phys_addr_t base,
>> phys_addr_t highmem_start;
>> int ret = 0;
>>
>> -#ifdef CONFIG_X86
>> +#if defined(CONFIG_X86) || defined(CONFIG_ARM64)
>
> Rather than an architecture list, can we depend on DEBUG_VIRTUAL? Are
> there other checks that we're trying to avoid?
>
> ... or could we avoid ifdeffery entirely with something like:
>
> /*
> * We can't use __pa(high_memory) directly, since high_memory
> * isn't a valid direct map VA, and DEBUG_VIRTUAL will (validly)
> * complain. Find the boundary by adding one to the last valid
> * address.
> */
> highmem_start = __pa(high_memory - 1) + 1;
>
I like getting rid of the #ifdef there. Maybe future cleanup could turn
this into a #define, HIGHMEM_PHYS since it seems to be used in quite
a few places in the kernel.
> Thanks,
> Mark.
>
Thanks,
Laura
>> /*
>> * high_memory isn't direct mapped memory so retrieving its physical
>> * address isn't appropriate. But it would be useful to check the
>> --
>> 2.7.4
>>
^ permalink raw reply
* [PATCH v2 5/5] fpga manager: cyclone-ps-spi: make delay variable
From: Moritz Fischer @ 2016-10-28 17:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2239ffc8aba7d053825d2bea122a3c16cdd9f6e1.1477669745.git.stillcompiling@gmail.com>
Hi Joshua,
looks good to me; however, I think since you're adding initial support,
I'd squash this together with [3/5].
On Fri, Oct 28, 2016 at 09:56:42AM -0700, Joshua Clayton wrote:
> The status pin may not show ready in the time described in the
> Altetera manual. check the value several times before giving up
s/Altetera/Altera
> For the hardware I am working on, the status pin takes 250 us,
> 5 times as long as described by Altera.
>
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
> ---
> drivers/fpga/cyclone-ps-spi.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/fpga/cyclone-ps-spi.c b/drivers/fpga/cyclone-ps-spi.c
> index 4b70d5c..c368223 100644
> --- a/drivers/fpga/cyclone-ps-spi.c
> +++ b/drivers/fpga/cyclone-ps-spi.c
> @@ -20,6 +20,7 @@
>
> #define FPGA_RESET_TIME 50 /* time in usecs to trigger FPGA config */
> -#define FPGA_MIN_DELAY 250 /* min usecs to wait for config status */
> +#define FPGA_MIN_DELAY 50 /* min usecs to wait for config status */
> +#define FPGA_MAX_DELAY 1000 /* max usecs to wait for config status */
>
> struct cyclonespi_conf {
> struct gpio_desc *config;
> @@ -42,6 +43,7 @@ static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
> const char *buf, size_t count)
> {
> struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
> + int i;
>
> if (flags & FPGA_MGR_PARTIAL_RECONFIG) {
> dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
> @@ -56,13 +58,14 @@ static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
> }
>
> gpiod_set_value(conf->config, 1);
> - usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
> - if (gpiod_get_value(conf->status) == 0) {
> - dev_err(&mgr->dev, "Status pin not ready.\n");
> - return -EIO;
> + for (i = 0; i < (FPGA_MAX_DELAY / FPGA_MIN_DELAY); i++) {
> + usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
> + if (gpiod_get_value(conf->status))
> + return 0;
> }
>
> - return 0;
> + dev_err(&mgr->dev, "Status pin not ready.\n");
> + return -EIO;
> }
>
> static void rev_buf(void *buf, size_t len)
> --
> 2.7.4
>
Cheers,
Moritz
^ permalink raw reply
* [PATCH v5 1/7] drm: sunxi: Add a basic DRM driver for Allwinner DE2
From: Jean-Francois Moine @ 2016-10-28 17:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161027220316.y7u3h4qsfftryrmp@lukather>
On Fri, 28 Oct 2016 00:03:16 +0200
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> On Tue, Oct 25, 2016 at 04:14:41PM +0200, Jean-Francois Moine wrote:
> > > > +Display controller
> > > > +==================
> > > > +
> > > > +Required properties:
> > > > +
> > > > +- compatible: value should be one of the following
> > > > + "allwinner,sun8i-a83t-display-engine"
> > > > + "allwinner,sun8i-h3-display-engine"
> > > > +
> > > > +- clocks: must include clock specifiers corresponding to entries in the
> > > > + clock-names property.
> > > > +
> > > > +- clock-names: must contain
> > > > + "gate": for DE activation
> > > > + "clock": DE clock
> > >
> > > We've been calling them bus and mod.
> >
> > I can understand "bus" (which is better than "apb"), but why "mod"?
>
> Allwinner has been calling the clocks that are supposed to generate
> the external signals (depending on where you were looking) module or
> mod clocks (which is also why we have mod in the clock
> compatibles). The module 1 clocks being used for the audio and the
> module 0 for the rest (SPI, MMC, NAND, display, etc.)
I did not find any 'module' in the H3 documentation.
So, is it really a good name?
> > > > +
> > > > +- resets: phandle to the reset of the device
> > > > +
> > > > +- ports: phandle's to the LCD ports
> > >
> > > Please use the OF graph.
> >
> > These ports are references to the graph of nodes. See
> > http://www.kernelhub.org/?msg=911825&p=2
>
> In an OF-graph, your phandle to the LCD controller would be replaced
> by an output endpoint.
This is the DE controller. There is no endpoint link at this level.
The Device Engine just handles the planes of the LCDs, but, indeed,
the LCDs must know about the DE and the DE must know about the LCDs.
There are 2 ways to realize this knowledge in the DT:
1) either the DE has one or two phandle's to the LCDs,
2) or the LCDs have a phandle to the DE.
I chose the 1st way, the DE ports pointing to the endpoint of the LCDs
which is part of the video link (OF-graph LCD <-> connector).
It would be possible to have phandles to the LCDs themselves, but this
asks for more code.
The second way is also possible, but it also complexifies a bit the
exchanges DE <-> LCD.
> > [snip]
> > > > +struct tcon {
> > > > + u32 gctl;
> > > > +#define TCON_GCTL_TCON_En BIT(31)
[snip]
> > > > + u32 fill_ctl; /* 0x300 */
> > > > + u32 fill_start0;
> > > > + u32 fill_end0;
> > > > + u32 fill_data0;
> > > > +};
> > >
> > > Please use defines instead of the structures.
> >
> > I think that structures are more readable.
>
> That's not really the point. No one in the kernel uses it (and even
> you use defines for registers offset in some places of that
> patch). And then you have Andr? arguments.
I am not convinced, but I'll do as you said.
> > > > +void de2_disable_vblank(struct drm_device *drm, unsigned crtc)
> > > > +{
> > > > + struct priv *priv = drm->dev_private;
> > > > + struct lcd *lcd = priv->lcds[crtc];
> > > > +
> > > > + tcon_write(lcd->mmio, gint0,
> > > > + tcon_read(lcd->mmio, gint0) &
> > > > + ~TCON_GINT0_TCON1_Vb_Int_En);
> > > > +}
> > > > +
> > > > +/* panel functions */
> > >
> > > Panel functions? In the CRTC driver?
> >
> > Yes, dumb panel.
>
> What do you mean by that? Using a Parallel/RGB interface?
Sorry, I though this was a well-known name. The 'dump panel' was used
in the documentation of my previous ARM machine as the video frame sent
to the HDMI controller. 'video_frame' is OK for you?
[snip]
> > > > + ret = clk_prepare_enable(lcd->clk);
> > > > + if (ret)
> > > > + goto err2;
> > >
> > > Is there any reason not to do that in the enable / disable? Leaving
> > > clocks running while the device has no guarantee that it's going to be
> > > used seems like a waste of resources.
> >
> > If the machine does not need video (network server, router..), it is simpler
> > to prevent the video driver to be loaded (DT, module black list...).
>
> You might not have control on any of it, or you might just have no
> monitor attached for example. Recompiling the kernel or updating the
> DT when you want to plug an HDMI monitor seems like a poor UX :)
OK, I will check if this works.
> > > > +static const struct {
> > > > + char chan;
> > > > + char layer;
> > > > + char pipe;
> > > > +} plane2layer[DE2_N_PLANES] = {
> > > > + [DE2_PRIMARY_PLANE] = {0, 0, 0},
> > > > + [DE2_CURSOR_PLANE] = {1, 0, 1},
> > > > + [DE2_VI_PLANE] = {0, 1, 0},
> > > > +};
> > >
> > > Comments?
> >
> > This
> > primary plane is channel 0 (VI), layer 0, pipe 0
> > cursor plane is channel 1 (UI), layer 0, pipe 1
> > overlay plane is channel 0 (VI), layer 1, pipe 0
> > or the full explanation:
> > Constraints:
> > The VI channels can do RGB or YUV, while UI channels can do RGB
> > only.
> > The LCD 0 has 1 VI channel and 4 UI channels, while
> > LCD 1 has only 1 VI channel and 1 UI channel.
> > The cursor must go to a channel bigger than the primary channel,
> > otherwise it is not transparent.
> > First try:
> > Letting the primary plane (usually RGB) in the 2nd channel (UI),
> > as this is done in the legacy driver, asks for the cursor to go
> > to the next channel (UI), but this one does not exist in LCD1.
> > Retained layout:
> > So, we must use only 2 channels for the same behaviour on LCD0
> > (H3) and LCD1 (A83T)
> > The retained combination is:
> > - primary plane in the first channel (VI),
> > - cursor plane inthe 2nd channel (UI), and
> > - overlay plane in the 1st channel (VI).
> >
> > Note that there could be 3 overlay planes (a channel has 4
> > layers), but I am not sure that the A83T or the H3 could
> > support 3 simultaneous video streams...
>
> Do you know if the pipe works in the old display engine?
>
> Especially about the two-steps composition that wouldn't allow you to
> have alpha on all the planes?
>
> If it is similar, I think hardcoding the pipe number is pretty bad,
> because that would restrict the combination of planes and formats,
> while some other might have worked.
>From what I understood about the DE2, the pipes just define the priority
of the overlay channels (one pipe for one channel).
With the cursor constraint, there must be at least 2 channels in
order (primary, cursor). Then, with these 2 channels/pipes, there can be
6 so-called overlay planes (3 RGB/YUV and 3 RGB only).
Enabling the pipes 2 and 3 (LCD 0 only) would offer 8 more planes, but
RGB only. Then, it might be useful to have dynamic pipes.
[snip]
> > > > +void de2_de_plane_update(struct priv *priv,
> > > > + int lcd_num, int plane_ix,
> > > > + struct drm_plane_state *state,
> > > > + struct drm_plane_state *old_state)
> > > > +{
> > > > + struct drm_framebuffer *fb = state->fb;
> > > > + struct drm_gem_cma_object *gem;
> > > > + void __iomem *mux_o = priv->mmio;
> > > > + void __iomem *chan_o;
> > > > + u32 size = WH(state->crtc_w, state->crtc_h);
> > > > + u32 coord;
> > > > + u32 screen_size;
> > > > + u32 data, fcolor;
> > > > + u32 ui_sel, alpha_glob;
> > > > + int chan, layer, x, y;
> > > > + unsigned fmt;
> > > > + unsigned long flags;
> > > > +
> > > > + chan = plane2layer[plane_ix].chan;
> > > > + layer = plane2layer[plane_ix].layer;
> > > > +
> > > > + mux_o += (lcd_num == 0) ? DE_MUX0_BASE : DE_MUX1_BASE;
> > > > + chan_o = mux_o;
> > > > + chan_o += DE_MUX_CHAN_REGS + DE_MUX_CHAN_SZ * chan;
> > > > +
> > > > + x = state->crtc_x >= 0 ? state->crtc_x : 0;
> > > > + y = state->crtc_y >= 0 ? state->crtc_y : 0;
> > > > + coord = XY(x, y);
> > > > +
> > > > + /* handle the cursor move */
> > > > + if (plane_ix == DE2_CURSOR_PLANE
> > > > + && fb == old_state->fb) {
> > > > + spin_lock_irqsave(&de_lock, flags);
> > > > + de_lcd_select(priv, lcd_num, mux_o);
> > > > + if (chan == 0)
> > > > + vi_write(chan_o, cfg[layer].coord, coord);
> > > > + else
> > > > + ui_write(chan_o, cfg[layer].coord, coord);
> > > > + spin_unlock_irqrestore(&de_lock, flags);
> > > > + return;
> > > > + }
> > > > +
> > > > + gem = drm_fb_cma_get_gem_obj(fb, 0);
> > > > +
> > > > + ui_sel = alpha_glob = 0;
> > > > + switch (fb->pixel_format) {
> > > > + case DRM_FORMAT_ARGB8888:
> > > > + fmt = DE2_FORMAT_ARGB_8888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + break;
> > > > + case DRM_FORMAT_BGRA8888:
> > > > + fmt = DE2_FORMAT_BGRA_8888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + break;
> > > > + case DRM_FORMAT_XRGB8888:
> > > > + fmt = DE2_FORMAT_XRGB_8888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + alpha_glob = (1 << UI_CFG_ATTR_alpmod_SHIFT) |
> > > > + (0xff << UI_CFG_ATTR_alpha_SHIFT);
> > > > + break;
> > > > + case DRM_FORMAT_RGB888:
> > > > + fmt = DE2_FORMAT_RGB_888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + break;
> > > > + case DRM_FORMAT_BGR888:
> > > > + fmt = DE2_FORMAT_BGR_888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + break;
> > > > + case DRM_FORMAT_YUYV:
> > > > + fmt = DE2_FORMAT_YUV422_I_YUYV;
> > > > + break;
> > > > + case DRM_FORMAT_YVYU:
> > > > + fmt = DE2_FORMAT_YUV422_I_YVYU;
> > > > + break;
> > > > + case DRM_FORMAT_YUV422:
> > > > + fmt = DE2_FORMAT_YUV422_P;
> > > > + break;
> > > > + case DRM_FORMAT_YUV420:
> > > > + fmt = DE2_FORMAT_YUV420_P;
> > > > + break;
> > > > + case DRM_FORMAT_UYVY:
> > > > + fmt = DE2_FORMAT_YUV422_I_UYVY;
> > > > + break;
> > > > + default:
> > > > + pr_err("format %.4s not yet treated\n",
> > > > + (char *) &fb->pixel_format);
> > > > + return;
> > > > + }
> > > > +
> > > > + spin_lock_irqsave(&de_lock, flags);
> > > > +
> > > > + screen_size = plane_ix == DE2_PRIMARY_PLANE ?
> > > > + size :
> > > > + glb_read(mux_o + DE_MUX_GLB_REGS, size);
> > > > +
> > > > + /* prepare the activation of alpha blending (1 bit per plane) */
> > > > + fcolor = bld_read(mux_o + DE_MUX_BLD_REGS, fcolor_ctl)
> > > > + | (0x100 << plane2layer[plane_ix].pipe);
> > > > +
> > > > + de_lcd_select(priv, lcd_num, mux_o);
> > > > +
> > > > + if (chan == 0) { /* VI channel */
> > > > + int i;
> > > > +
> > > > + data = VI_CFG_ATTR_en | (fmt << VI_CFG_ATTR_fmt_SHIFT) |
> > > > + ui_sel;
> > > > + vi_write(chan_o, cfg[layer].attr, data);
> > > > + vi_write(chan_o, cfg[layer].size, size);
> > > > + vi_write(chan_o, cfg[layer].coord, coord);
> > > > + for (i = 0; i < VI_N_PLANES; i++) {
> > > > + vi_write(chan_o, cfg[layer].pitch[i],
> > > > + fb->pitches[i] ? fb->pitches[i] :
> > > > + fb->pitches[0]);
> > > > + vi_write(chan_o, cfg[layer].top_laddr[i],
> > > > + gem->paddr + fb->offsets[i]);
> > > > + vi_write(chan_o, fcolor[layer], 0xff000000);
> > > > + }
> > > > + if (layer == 0)
> > > > + vi_write(chan_o, ovl_size[0], screen_size);
> > > > +
> > > > + } else { /* UI channel */
> > > > + data = UI_CFG_ATTR_en | (fmt << UI_CFG_ATTR_fmt_SHIFT) |
> > > > + alpha_glob;
> > > > + ui_write(chan_o, cfg[layer].attr, data);
> > > > + ui_write(chan_o, cfg[layer].size, size);
> > > > + ui_write(chan_o, cfg[layer].coord, coord);
> > > > + ui_write(chan_o, cfg[layer].pitch, fb->pitches[0]);
> > > > + ui_write(chan_o, cfg[layer].top_laddr,
> > > > + gem->paddr + fb->offsets[0]);
> > > > + if (layer == 0)
> > > > + ui_write(chan_o, ovl_size, screen_size);
> > > > + }
> > > > + bld_write(mux_o + DE_MUX_BLD_REGS, fcolor_ctl, fcolor);
> > > > +
> > > > + spin_unlock_irqrestore(&de_lock, flags);
> > > > +}
> > >
> > > Splitting that into functions would make it a bit more trivial and
> > > readable.
> >
> > Not sure: there is a lot of common data and different I/O accesses.
>
> You could still have different ones to set the buffers, formats and
> coordinates for example.
I will have a look...
[snip]
> > > > +static int __init de2_drm_init(void)
> > > > +{
> > > > + int ret;
> > > > +
> > > > +/* uncomment to activate the drm traces at startup time */
> > > > +/* drm_debug = DRM_UT_CORE | DRM_UT_DRIVER | DRM_UT_KMS |
> > > > + DRM_UT_PRIME | DRM_UT_ATOMIC; */
> > >
> > > That's useless.
> >
> > Right, but it seems that some people don't know how to debug a DRM
> > driver. This is only a reminder.
> >
> > > > + DRM_DEBUG_DRIVER("\n");
> > > > +
> > > > + ret = platform_driver_register(&de2_lcd_platform_driver);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + ret = platform_driver_register(&de2_drm_platform_driver);
> > > > + if (ret < 0)
> > > > + platform_driver_unregister(&de2_lcd_platform_driver);
> > > > +
> > > > + return ret;
> > > > +}
> > >
> > > And that really shouldn't be done that way.
> >
> > May you explain?
>
> This goes against the whole idea of the device and driver
> model. Drivers should only register themselves, device should be
> created by buses (or by using some external components if the bus
> can't: DT, ACPI, etc.). If there's a match, you get probed.
>
> A driver that creates its own device just to probe itself violates
> that.
In this function (module init), there is no driver yet.
The module contains 2 drivers: the DE (planes) and the LCD (CRTC),
and there is no macro to handle such modules.
> > > > +int de2_plane_init(struct drm_device *drm, struct lcd *lcd)
> > > > +{
> > > > + int ret, possible_crtcs = 1 << lcd->crtc_idx;
> > > > +
> > > > + ret = de2_one_plane_init(drm, &lcd->planes[DE2_PRIMARY_PLANE],
> > > > + DRM_PLANE_TYPE_PRIMARY, possible_crtcs,
> > > > + ui_formats, ARRAY_SIZE(ui_formats));
> > > > + if (ret >= 0)
> > > > + ret = de2_one_plane_init(drm, &lcd->planes[DE2_CURSOR_PLANE],
> > > > + DRM_PLANE_TYPE_CURSOR, possible_crtcs,
> > > > + ui_formats, ARRAY_SIZE(ui_formats));
> > >
> > > Nothing looks really special about that cursor plane. Any reasion not
> > > to make it an overlay?
> >
> > As explained above (channel/layer/pipe plane definitions), the cursor
> > cannot go in a channel lower or equal to the one of the primary plane.
> > Then, it must be known and, so, have an explicit plane.
>
> If you were to make it a plane, you could use atomic_check to check
> this and make sure this doesn't happen. And you would gain a generic
> plane that can be used for other purposes if needed.
The function drm_crtc_init_with_planes() offers a cursor plane for free.
On the other side, having 6 overlay planes is more than the SoCs can
support.
--
Ken ar c'henta? | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply
* [PATCH v4 8/8] arm64: Enable CONFIG_ARM64_SW_TTBR0_PAN
From: Catalin Marinas @ 2016-10-28 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477675636-3957-1-git-send-email-catalin.marinas@arm.com>
This patch adds the Kconfig option to enable support for TTBR0 PAN
emulation. The option is default off because of a slight performance hit
when enabled, caused by the additional TTBR0_EL1 switching during user
access operations or exception entry/exit code.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/Kconfig | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 969ef880d234..6b9a8446fc43 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -790,6 +790,14 @@ config SETEND_EMULATION
If unsure, say Y
endif
+config ARM64_SW_TTBR0_PAN
+ bool "Emulate Privileged Access Never using TTBR0_EL1 switching"
+ help
+ Enabling this option prevents the kernel from accessing
+ user-space memory directly by pointing TTBR0_EL1 to a reserved
+ zeroed area and reserved ASID. The user access routines
+ restore the valid TTBR0_EL1 temporarily.
+
menu "ARMv8.1 architectural features"
config ARM64_HW_AFDBM
^ permalink raw reply related
* [PATCH v4 7/8] arm64: xen: Enable user access before a privcmd hvc call
From: Catalin Marinas @ 2016-10-28 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477675636-3957-1-git-send-email-catalin.marinas@arm.com>
Privcmd calls are issued by the userspace. The kernel needs to enable
access to TTBR0_EL1 as the hypervisor would issue stage 1 translations
to user memory via AT instructions. Since AT instructions are not
affected by the PAN bit (ARMv8.1), we only need the explicit
uaccess_enable/disable if the TTBR0 PAN option is enabled.
Reviewed-by: Julien Grall <julien.grall@arm.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/xen/hypercall.S | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
index 329c8027b0a9..b41aff25426d 100644
--- a/arch/arm64/xen/hypercall.S
+++ b/arch/arm64/xen/hypercall.S
@@ -49,6 +49,7 @@
#include <linux/linkage.h>
#include <asm/assembler.h>
+#include <asm/uaccess.h>
#include <xen/interface/xen.h>
@@ -91,6 +92,20 @@ ENTRY(privcmd_call)
mov x2, x3
mov x3, x4
mov x4, x5
+ /*
+ * Privcmd calls are issued by the userspace. The kernel needs to
+ * enable access to TTBR0_EL1 as the hypervisor would issue stage 1
+ * translations to user memory via AT instructions. Since AT
+ * instructions are not affected by the PAN bit (ARMv8.1), we only
+ * need the explicit uaccess_enable/disable if the TTBR0 PAN emulation
+ * is enabled (it implies that hardware UAO and PAN disabled).
+ */
+ uaccess_ttbr0_enable x6, x7
hvc XEN_IMM
+
+ /*
+ * Disable userspace access from kernel once the hyp call completed.
+ */
+ uaccess_ttbr0_disable x6
ret
ENDPROC(privcmd_call);
^ permalink raw reply related
* [PATCH v4 6/8] arm64: Handle faults caused by inadvertent user access with PAN enabled
From: Catalin Marinas @ 2016-10-28 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477675636-3957-1-git-send-email-catalin.marinas@arm.com>
When TTBR0_EL1 is set to the reserved page, an erroneous kernel access
to user space would generate a translation fault. This patch adds the
checks for the software-set PSR_PAN_BIT to emulate a permission fault
and report it accordingly.
Cc: Will Deacon <will.deacon@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/mm/fault.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index d035cc594445..a78a5c401806 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -269,13 +269,19 @@ static int __do_page_fault(struct mm_struct *mm, unsigned long addr,
return fault;
}
-static inline bool is_permission_fault(unsigned int esr)
+static inline bool is_permission_fault(unsigned int esr, struct pt_regs *regs)
{
unsigned int ec = ESR_ELx_EC(esr);
unsigned int fsc_type = esr & ESR_ELx_FSC_TYPE;
- return (ec == ESR_ELx_EC_DABT_CUR && fsc_type == ESR_ELx_FSC_PERM) ||
- (ec == ESR_ELx_EC_IABT_CUR && fsc_type == ESR_ELx_FSC_PERM);
+ if (ec != ESR_ELx_EC_DABT_CUR && ec != ESR_ELx_EC_IABT_CUR)
+ return false;
+
+ if (system_uses_ttbr0_pan())
+ return fsc_type == ESR_ELx_FSC_FAULT &&
+ (regs->pstate & PSR_PAN_BIT);
+ else
+ return fsc_type == ESR_ELx_FSC_PERM;
}
static bool is_el0_instruction_abort(unsigned int esr)
@@ -315,7 +321,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
mm_flags |= FAULT_FLAG_WRITE;
}
- if (is_permission_fault(esr) && (addr < USER_DS)) {
+ if (addr < USER_DS && is_permission_fault(esr, regs)) {
/* regs->orig_addr_limit may be 0 if we entered from EL0 */
if (regs->orig_addr_limit == KERNEL_DS)
die("Accessing user space memory with fs=KERNEL_DS", regs, esr);
^ permalink raw reply related
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