Re: [PATCH 1/3] init: Declare rodata_enabled and mark_rodata_ro() at all time
Hi Christophe, On 31.01.2024 21:07, Christophe Leroy wrote: > Le 31/01/2024 à 16:17, Marek Szyprowski a écrit : >> [Vous ne recevez pas souvent de courriers de m.szyprow...@samsung.com. >> Découvrez pourquoi ceci est important à >> https://aka.ms/LearnAboutSenderIdentification ] >> >> On 31.01.2024 12:58, Christophe Leroy wrote: >>> Le 30/01/2024 à 18:48, Marek Szyprowski a écrit : >>>> [Vous ne recevez pas souvent de courriers de m.szyprow...@samsung.com. >>>> Découvrez pourquoi ceci est important à >>>> https://aka.ms/LearnAboutSenderIdentification ] >>>> >>>> On 30.01.2024 12:03, Christophe Leroy wrote: >>>>> Le 30/01/2024 à 10:16, Chen-Yu Tsai a écrit : >>>>>> [Vous ne recevez pas souvent de courriers de we...@chromium.org. >>>>>> D?couvrez pourquoi ceci est important ? >>>>>> https://aka.ms/LearnAboutSenderIdentification ] >>>>>> >>>>>> On Mon, Jan 29, 2024 at 12:09:50PM -0800, Luis Chamberlain wrote: >>>>>>> On Thu, Dec 21, 2023 at 10:02:46AM +0100, Christophe Leroy wrote: >>>>>>>> Declaring rodata_enabled and mark_rodata_ro() at all time >>>>>>>> helps removing related #ifdefery in C files. >>>>>>>> >>>>>>>> Signed-off-by: Christophe Leroy >>>>>>> Very nice cleanup, thanks!, applied and pushed >>>>>>> >>>>>>> Luis >>>>>> On next-20240130, which has your modules-next branch, and thus this >>>>>> series and the other "module: Use set_memory_rox()" series applied, >>>>>> my kernel crashes in some very weird way. Reverting your branch >>>>>> makes the crash go away. >>>>>> >>>>>> I thought I'd report it right away. Maybe you folks would know what's >>>>>> happening here? This is on arm64. >>>>> That's strange, it seems to bug in module_bug_finalize() which is >>>>> _before_ calls to module_enable_ro() and such. >>>>> >>>>> Can you try to revert the 6 patches one by one to see which one >>>>> introduces the problem ? >>>>> >>>>> In reality, only patch 677bfb9db8a3 really change things. Other ones are >>>>> more on less only cleanup. >>>> I've also run into this issue with today's (20240130) linux-next on my >>>> test farm. The issue is not fully reproducible, so it was a bit hard to >>>> bisect it automatically. I've spent some time on manual testing and it >>>> looks that reverting the following 2 commits on top of linux-next fixes >>>> the problem: >>>> >>>> 65929884f868 ("modules: Remove #ifdef CONFIG_STRICT_MODULE_RWX around >>>> rodata_enabled") >>>> 677bfb9db8a3 ("module: Don't ignore errors from set_memory_XX()") >>>> >>>> This in fact means that commit 677bfb9db8a3 is responsible for this >>>> regression, as 65929884f868 has to be reverted only because the latter >>>> depends on it. Let me know what I can do to help debugging this issue. >>>> >>> Thanks for the bisect. I suspect you hit one of the errors and something >>> goes wrong in the error path. >>> >>> To confirm this assumption, could you try with the following change on >>> top of everything ? >> >> Yes, this is the problem. I've added printing a mod->name to the log. >> Here is a log from kernel build from next-20240130 (sometimes it even >> boots to shell): >> >> # dmesg | grep module_set_memory >> [ 8.061525] module_set_memory(6, , 0) name ipv6 >> returned -22 >> [ 8.067543] WARNING: CPU: 3 PID: 1 at kernel/module/strict_rwx.c:22 >> module_set_memory+0x9c/0xb8 > Would be good if you could show the backtrace too so that we know who is > the caller. I guess what you show here is what you get on the screen ? > The backtrace should be available throught 'dmesg'. Here are relevant parts of the boot log: [ 8.096850] [ cut here ] [ 8.096939] module_set_memory(6, , 0) name ipv6 returned -22 [ 8.102947] WARNING: CPU: 4 PID: 1 at kernel/module/strict_rwx.c:22 module_set_memory+0x9c/0xb8 [ 8.111561] Modules linked in: [ 8.114596] CPU: 4 PID: 1 Comm: systemd Not tainted 6.8.0-rc2-next-20240130-dirty #14429 [ 8.122654] Hardware name: Khadas VIM3 (DT) [ 8.126815] pstate: 6005 (nZCv daif -PAN -UAO -TCO -DIT -
Re: [PATCH 1/3] init: Declare rodata_enabled and mark_rodata_ro() at all time
Hi Christophe, On 31.01.2024 12:58, Christophe Leroy wrote: > Le 30/01/2024 à 18:48, Marek Szyprowski a écrit : >> [Vous ne recevez pas souvent de courriers de m.szyprow...@samsung.com. >> Découvrez pourquoi ceci est important à >> https://aka.ms/LearnAboutSenderIdentification ] >> >> On 30.01.2024 12:03, Christophe Leroy wrote: >>> Le 30/01/2024 à 10:16, Chen-Yu Tsai a écrit : >>>> [Vous ne recevez pas souvent de courriers de we...@chromium.org. D?couvrez >>>> pourquoi ceci est important ? >>>> https://aka.ms/LearnAboutSenderIdentification ] >>>> >>>> On Mon, Jan 29, 2024 at 12:09:50PM -0800, Luis Chamberlain wrote: >>>>> On Thu, Dec 21, 2023 at 10:02:46AM +0100, Christophe Leroy wrote: >>>>>> Declaring rodata_enabled and mark_rodata_ro() at all time >>>>>> helps removing related #ifdefery in C files. >>>>>> >>>>>> Signed-off-by: Christophe Leroy >>>>> Very nice cleanup, thanks!, applied and pushed >>>>> >>>>> Luis >>>> On next-20240130, which has your modules-next branch, and thus this >>>> series and the other "module: Use set_memory_rox()" series applied, >>>> my kernel crashes in some very weird way. Reverting your branch >>>> makes the crash go away. >>>> >>>> I thought I'd report it right away. Maybe you folks would know what's >>>> happening here? This is on arm64. >>> That's strange, it seems to bug in module_bug_finalize() which is >>> _before_ calls to module_enable_ro() and such. >>> >>> Can you try to revert the 6 patches one by one to see which one >>> introduces the problem ? >>> >>> In reality, only patch 677bfb9db8a3 really change things. Other ones are >>> more on less only cleanup. >> I've also run into this issue with today's (20240130) linux-next on my >> test farm. The issue is not fully reproducible, so it was a bit hard to >> bisect it automatically. I've spent some time on manual testing and it >> looks that reverting the following 2 commits on top of linux-next fixes >> the problem: >> >> 65929884f868 ("modules: Remove #ifdef CONFIG_STRICT_MODULE_RWX around >> rodata_enabled") >> 677bfb9db8a3 ("module: Don't ignore errors from set_memory_XX()") >> >> This in fact means that commit 677bfb9db8a3 is responsible for this >> regression, as 65929884f868 has to be reverted only because the latter >> depends on it. Let me know what I can do to help debugging this issue. >> > Thanks for the bisect. I suspect you hit one of the errors and something > goes wrong in the error path. > > To confirm this assumption, could you try with the following change on > top of everything ? Yes, this is the problem. I've added printing a mod->name to the log. Here is a log from kernel build from next-20240130 (sometimes it even boots to shell): # dmesg | grep module_set_memory [ 8.061525] module_set_memory(6, , 0) name ipv6 returned -22 [ 8.067543] WARNING: CPU: 3 PID: 1 at kernel/module/strict_rwx.c:22 module_set_memory+0x9c/0xb8 [ 8.097821] pc : module_set_memory+0x9c/0xb8 [ 8.102068] lr : module_set_memory+0x9c/0xb8 [ 8.183101] module_set_memory+0x9c/0xb8 [ 8.472862] module_set_memory(6, , 0) name x_tables returned -22 [ 8.479215] WARNING: CPU: 2 PID: 1 at kernel/module/strict_rwx.c:22 module_set_memory+0x9c/0xb8 [ 8.510978] pc : module_set_memory+0x9c/0xb8 [ 8.515225] lr : module_set_memory+0x9c/0xb8 [ 8.596259] module_set_memory+0x9c/0xb8 [ 10.529879] module_set_memory(6, , 0) name dm_mod returned -22 [ 10.536087] WARNING: CPU: 3 PID: 127 at kernel/module/strict_rwx.c:22 module_set_memory+0x9c/0xb8 [ 10.568254] pc : module_set_memory+0x9c/0xb8 [ 10.572501] lr : module_set_memory+0x9c/0xb8 [ 10.653535] module_set_memory+0x9c/0xb8 [ 10.853177] module_set_memory(6, , 0) name fuse returned -22 [ 10.859196] WARNING: CPU: 5 PID: 130 at kernel/module/strict_rwx.c:22 module_set_memory+0x9c/0xb8 [ 10.891382] pc : module_set_memory+0x9c/0xb8 [ 10.895629] lr : module_set_memory+0x9c/0xb8 [ 10.976663] module_set_memory+0x9c/0xb8 > diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c > index a14df9655dbe..fdf8484154dd 100644 > --- a/kernel/module/strict_rwx.c > +++ b/kernel/module/strict_rwx.c > @@ -15,9 +15,12 @@ static int module_set_memory(const struct module > *mod, enum mod_mem_type type, > int (*set_memory)(unsigned long start, int > num_pages)) >{ > const struct module_memory *mod_mem = >mem[type]; > + int err; > > set_vm_flush_reset_perms(mod_mem->base); > - return set_memory((unsigned long)mod_mem->base, mod_mem->size >> > PAGE_SHIFT); > + err = set_memory((unsigned long)mod_mem->base, mod_mem->size >> > PAGE_SHIFT); > + WARN(err, "module_set_memory(%d, %px, %x) returned %d\n", type, > mod_mem->base, mod_mem->size, err); > + return err; >} > >/* > > > Thanks for your help > Christophe Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 1/3] init: Declare rodata_enabled and mark_rodata_ro() at all time
Dear All, On 30.01.2024 12:03, Christophe Leroy wrote: > Le 30/01/2024 à 10:16, Chen-Yu Tsai a écrit : >> [Vous ne recevez pas souvent de courriers de we...@chromium.org. D?couvrez >> pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ] >> >> On Mon, Jan 29, 2024 at 12:09:50PM -0800, Luis Chamberlain wrote: >>> On Thu, Dec 21, 2023 at 10:02:46AM +0100, Christophe Leroy wrote: >>>> Declaring rodata_enabled and mark_rodata_ro() at all time >>>> helps removing related #ifdefery in C files. >>>> >>>> Signed-off-by: Christophe Leroy >>> Very nice cleanup, thanks!, applied and pushed >>> >>> Luis >> On next-20240130, which has your modules-next branch, and thus this >> series and the other "module: Use set_memory_rox()" series applied, >> my kernel crashes in some very weird way. Reverting your branch >> makes the crash go away. >> >> I thought I'd report it right away. Maybe you folks would know what's >> happening here? This is on arm64. > That's strange, it seems to bug in module_bug_finalize() which is > _before_ calls to module_enable_ro() and such. > > Can you try to revert the 6 patches one by one to see which one > introduces the problem ? > > In reality, only patch 677bfb9db8a3 really change things. Other ones are > more on less only cleanup. I've also run into this issue with today's (20240130) linux-next on my test farm. The issue is not fully reproducible, so it was a bit hard to bisect it automatically. I've spent some time on manual testing and it looks that reverting the following 2 commits on top of linux-next fixes the problem: 65929884f868 ("modules: Remove #ifdef CONFIG_STRICT_MODULE_RWX around rodata_enabled") 677bfb9db8a3 ("module: Don't ignore errors from set_memory_XX()") This in fact means that commit 677bfb9db8a3 is responsible for this regression, as 65929884f868 has to be reverted only because the latter depends on it. Let me know what I can do to help debugging this issue. Here is the stack trace I've got on Khadas VIM3 ARM64 board: Unable to handle kernel paging request at virtual address 80007bfeeb30 Mem abort info: ESR = 0x9647 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x07: level 3 translation fault Data abort info: ISV = 0, ISS = 0x0047, ISS2 = 0x CM = 0, WnR = 1, TnD = 0, TagAccess = 0 GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 swapper pgtable: 4k pages, 48-bit VAs, pgdp=0a35a000 [80007bfeeb30] pgd=1000f4806003, p4d=1000f4806003, pud=17ed1003, pmd=17ed2003, pte= Internal error: Oops: 9647 [#1] PREEMPT SMP Modules linked in: CPU: 4 PID: 182 Comm: (udev-worker) Not tainted 6.8.0-rc2-next-20240130 #14391 Hardware name: Khadas VIM3 (DT) pstate: 6005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : module_bug_finalize+0xb0/0xdc lr : module_bug_finalize+0x70/0xdc ... Call trace: module_bug_finalize+0xb0/0xdc load_module+0x182c/0x1c88 init_module_from_file+0x84/0xc0 idempotent_init_module+0x180/0x250 __arm64_sys_finit_module+0x64/0xa0 invoke_syscall+0x48/0x114 el0_svc_common.constprop.0+0xc0/0xe0 do_el0_svc+0x1c/0x28 el0_svc+0x4c/0xe4 el0t_64_sync_handler+0xc0/0xc4 el0t_64_sync+0x190/0x194 Code: 9116e003 f942dc01 a93e8c41 c89ffc73 (f9000433) ---[ end trace ]--- Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v4 05/10] signal: Introduce TRAP_PERF si_code and si_perf to siginfo
iginfo.h > +++ b/include/uapi/asm-generic/siginfo.h > @@ -91,6 +91,8 @@ union __sifields { > char _dummy_pkey[__ADDR_BND_PKEY_PAD]; > __u32 _pkey; > } _addr_pkey; > + /* used when si_code=TRAP_PERF */ > + __u64 _perf; > }; > } _sigfault; > > @@ -155,6 +157,7 @@ typedef struct siginfo { > #define si_lower_sifields._sigfault._addr_bnd._lower > #define si_upper_sifields._sigfault._addr_bnd._upper > #define si_pkey _sifields._sigfault._addr_pkey._pkey > +#define si_perf _sifields._sigfault._perf > #define si_band _sifields._sigpoll._band > #define si_fd _sifields._sigpoll._fd > #define si_call_addr_sifields._sigsys._call_addr > @@ -253,7 +256,8 @@ typedef struct siginfo { > #define TRAP_BRANCH 3 /* process taken branch trap */ > #define TRAP_HWBKPT 4 /* hardware breakpoint/watchpoint */ > #define TRAP_UNK5 /* undiagnosed trap */ > -#define NSIGTRAP 5 > +#define TRAP_PERF6 /* perf event with sigtrap=1 */ > +#define NSIGTRAP 6 > > /* >* There is an additional set of SIGTRAP si_codes used by ptrace > diff --git a/include/uapi/linux/signalfd.h b/include/uapi/linux/signalfd.h > index 83429a05b698..7e333042c7e3 100644 > --- a/include/uapi/linux/signalfd.h > +++ b/include/uapi/linux/signalfd.h > @@ -39,6 +39,8 @@ struct signalfd_siginfo { > __s32 ssi_syscall; > __u64 ssi_call_addr; > __u32 ssi_arch; > + __u32 __pad3; > + __u64 ssi_perf; > > /* >* Pad strcture to 128 bytes. Remember to update the > @@ -49,7 +51,7 @@ struct signalfd_siginfo { >* comes out of a read(2) and we really don't want to have >* a compat on read(2). >*/ > - __u8 __pad[28]; > + __u8 __pad[16]; > }; > > > diff --git a/kernel/signal.c b/kernel/signal.c > index f2718350bf4b..7061e4957650 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -1199,6 +1199,7 @@ static inline bool has_si_pid_and_uid(struct > kernel_siginfo *info) > case SIL_FAULT_MCEERR: > case SIL_FAULT_BNDERR: > case SIL_FAULT_PKUERR: > + case SIL_PERF_EVENT: > case SIL_SYS: > ret = false; > break; > @@ -2531,6 +2532,7 @@ static void hide_si_addr_tag_bits(struct ksignal *ksig) > case SIL_FAULT_MCEERR: > case SIL_FAULT_BNDERR: > case SIL_FAULT_PKUERR: > + case SIL_PERF_EVENT: > ksig->info.si_addr = arch_untagged_si_addr( > ksig->info.si_addr, ksig->sig, ksig->info.si_code); > break; > @@ -3341,6 +3343,10 @@ void copy_siginfo_to_external32(struct compat_siginfo > *to, > #endif > to->si_pkey = from->si_pkey; > break; > + case SIL_PERF_EVENT: > + to->si_addr = ptr_to_compat(from->si_addr); > + to->si_perf = from->si_perf; > + break; > case SIL_CHLD: > to->si_pid = from->si_pid; > to->si_uid = from->si_uid; > @@ -3421,6 +3427,10 @@ static int > post_copy_siginfo_from_user32(kernel_siginfo_t *to, > #endif > to->si_pkey = from->si_pkey; > break; > + case SIL_PERF_EVENT: > + to->si_addr = compat_ptr(from->si_addr); > + to->si_perf = from->si_perf; > + break; > case SIL_CHLD: > to->si_pid= from->si_pid; > to->si_uid= from->si_uid; > @@ -4601,6 +4611,7 @@ static inline void siginfo_buildtime_checks(void) > CHECK_OFFSET(si_lower); > CHECK_OFFSET(si_upper); > CHECK_OFFSET(si_pkey); > + CHECK_OFFSET(si_perf); > > /* sigpoll */ > CHECK_OFFSET(si_band); Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 5/7] mfd: sec: Simplify getting of_device_id match data
On 20.04.2021 09:03, Krzysztof Kozlowski wrote: > On 20/04/2021 07:25, Marek Szyprowski wrote: >> On 19.04.2021 10:17, Krzysztof Kozlowski wrote: >>> Use of_device_get_match_data() to make the code slightly smaller. >>> >>> Signed-off-by: Krzysztof Kozlowski >>> --- >>>drivers/mfd/sec-core.c | 9 +++-- >>>1 file changed, 3 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c >>> index 8d55992da19e..3126c39f3203 100644 >>> --- a/drivers/mfd/sec-core.c >>> +++ b/drivers/mfd/sec-core.c >>> @@ -10,6 +10,7 @@ >>>#include >>>#include >>>#include >>> +#include >>>#include >>>#include >>>#include >>> @@ -324,12 +325,8 @@ static inline unsigned long >>> sec_i2c_get_driver_data(struct i2c_client *i2c, >>> const struct i2c_device_id *id) >>>{ >>>#ifdef CONFIG_OF >>> - if (i2c->dev.of_node) { >>> - const struct of_device_id *match; >>> - >>> - match = of_match_node(sec_dt_match, i2c->dev.of_node); >>> - return (unsigned long)match->data; >>> - } >>> + if (i2c->dev.of_node) >>> + return (unsigned long)of_device_get_match_data(>dev); >>>#endif >> Does it make any sense to keep the #ifdef CONFIG_OF after this change? > Good point, it was only to hide usage of of_device_id table. > >> I would also skip (i2c->dev.of_node) check, because >> of_device_get_match_data() already does that (although indirectly). > First, the enum sec_device_type would need to be changed so it starts > from 1, not 0. It's because the value returned by this function is later > assigned to that enum and there is no way currently to differentiate > between NULL and S5M8767X. > > Second, it wouldn't make the code smaller; > > unsigned long data; > data = of_device_get_match_data(>dev); > if (data) > return data; Then maybe one should go further and remove legacy, non-of based initialization, because it is not used at all. This will simplify it even more. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 5/7] mfd: sec: Simplify getting of_device_id match data
On 19.04.2021 10:17, Krzysztof Kozlowski wrote: > Use of_device_get_match_data() to make the code slightly smaller. > > Signed-off-by: Krzysztof Kozlowski > --- > drivers/mfd/sec-core.c | 9 +++-- > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/mfd/sec-core.c b/drivers/mfd/sec-core.c > index 8d55992da19e..3126c39f3203 100644 > --- a/drivers/mfd/sec-core.c > +++ b/drivers/mfd/sec-core.c > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -324,12 +325,8 @@ static inline unsigned long > sec_i2c_get_driver_data(struct i2c_client *i2c, > const struct i2c_device_id *id) > { > #ifdef CONFIG_OF > - if (i2c->dev.of_node) { > - const struct of_device_id *match; > - > - match = of_match_node(sec_dt_match, i2c->dev.of_node); > - return (unsigned long)match->data; > - } > + if (i2c->dev.of_node) > + return (unsigned long)of_device_get_match_data(>dev); > #endif Does it make any sense to keep the #ifdef CONFIG_OF after this change? I would also skip (i2c->dev.of_node) check, because of_device_get_match_data() already does that (although indirectly). > return id->driver_data; > } Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] iommu: exynos: remove unneeded local variable initialization
On 08.04.2021 22:16, Krzysztof Kozlowski wrote: > The initialization of 'fault_addr' local variable is not needed as it is > shortly after overwritten. > > Addresses-Coverity: Unused value > Signed-off-by: Krzysztof Kozlowski Acked-by: Marek Szyprowski > --- > drivers/iommu/exynos-iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c > index de324b4eedfe..8fa9a591fb96 100644 > --- a/drivers/iommu/exynos-iommu.c > +++ b/drivers/iommu/exynos-iommu.c > @@ -407,7 +407,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void > *dev_id) > struct sysmmu_drvdata *data = dev_id; > const struct sysmmu_fault_info *finfo; > unsigned int i, n, itype; > - sysmmu_iova_t fault_addr = -1; > + sysmmu_iova_t fault_addr; > unsigned short reg_status, reg_clear; > int ret = -ENOSYS; > Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] iio: buffer: use sysfs_attr_init() on allocated attrs
On 02.04.2021 19:42, Alexandru Ardelean wrote: > When dynamically allocating sysfs attributes, it's a good idea to call > sysfs_attr_init() on them to initialize lock_class_keys. > This change does that. > > The lock_class_keys are set when the CONFIG_DEBUG_LOCK_ALLOC symbol is > enabled. Which is [likely] one reason why I did not see this during > development. > > I also am not able to see this even with CONFIG_DEBUG_LOCK_ALLOC enabled, > so this may [likely] be reproduce-able on some system configurations. > > This was reported via: > > https://lore.kernel.org/linux-iio/CA+U=DsrsvGgXEF30-vXuXS_k=-mjsjibweezwkb1hjvn1p9...@mail.gmail.com/T/#u > > Fixes: 15097c7a1adc ("iio: buffer: wrap all buffer attributes into > iio_dev_attr") > Reported-by: Marek Szyprowski > Signed-off-by: Alexandru Ardelean > --- > > @Marek: could you maybe test this on your setup? > > I haven't been able to reproduce this on mine. Works fine with this fix. Thanks! Tested-by: Marek Szyprowski > Thanks > Alex > > drivers/iio/industrialio-buffer.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/iio/industrialio-buffer.c > b/drivers/iio/industrialio-buffer.c > index ee5aab9d4a23..06b2ea087408 100644 > --- a/drivers/iio/industrialio-buffer.c > +++ b/drivers/iio/industrialio-buffer.c > @@ -1309,6 +1309,7 @@ static struct attribute *iio_buffer_wrap_attr(struct > iio_buffer *buffer, > iio_attr->buffer = buffer; > memcpy(_attr->dev_attr, dattr, sizeof(iio_attr->dev_attr)); > iio_attr->dev_attr.attr.name = kstrdup_const(attr->name, GFP_KERNEL); > + sysfs_attr_init(_attr->dev_attr.attr); > > list_add(_attr->l, >buffer_attr_list); > Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v6 14/24] iio: buffer: wrap all buffer attributes into iio_dev_attr
+ struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; > > return sprintf(buf, "%d\n", iio_buffer_is_active(buffer)); > } > @@ -1154,7 +1150,7 @@ static ssize_t iio_buffer_store_enable(struct device > *dev, > int ret; > bool requested_state; > struct iio_dev *indio_dev = dev_to_iio_dev(dev); > - struct iio_buffer *buffer = indio_dev->buffer; > + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; > bool inlist; > > ret = strtobool(buf, _state); > @@ -1183,8 +1179,7 @@ static ssize_t iio_buffer_show_watermark(struct device > *dev, >struct device_attribute *attr, >char *buf) > { > - struct iio_dev *indio_dev = dev_to_iio_dev(dev); > - struct iio_buffer *buffer = indio_dev->buffer; > + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; > > return sprintf(buf, "%u\n", buffer->watermark); > } > @@ -1195,7 +1190,7 @@ static ssize_t iio_buffer_store_watermark(struct device > *dev, > size_t len) > { > struct iio_dev *indio_dev = dev_to_iio_dev(dev); > - struct iio_buffer *buffer = indio_dev->buffer; > + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; > unsigned int val; > int ret; > > @@ -1228,8 +1223,7 @@ static ssize_t iio_dma_show_data_available(struct > device *dev, > struct device_attribute *attr, > char *buf) > { > - struct iio_dev *indio_dev = dev_to_iio_dev(dev); > - struct iio_buffer *buffer = indio_dev->buffer; > + struct iio_buffer *buffer = to_iio_dev_attr(attr)->buffer; > > return sprintf(buf, "%zu\n", iio_buffer_data_available(buffer)); > } > @@ -1254,6 +1248,26 @@ static struct attribute *iio_buffer_attrs[] = { > _attr_data_available.attr, > }; > > +#define to_dev_attr(_attr) container_of(_attr, struct device_attribute, attr) > + > +static struct attribute *iio_buffer_wrap_attr(struct iio_buffer *buffer, > + struct attribute *attr) > +{ > + struct device_attribute *dattr = to_dev_attr(attr); > + struct iio_dev_attr *iio_attr; > + > + iio_attr = kzalloc(sizeof(*iio_attr), GFP_KERNEL); > + if (!iio_attr) > + return NULL; > + > + iio_attr->buffer = buffer; > + memcpy(_attr->dev_attr, dattr, sizeof(iio_attr->dev_attr)); > + > + list_add(_attr->l, >buffer_attr_list); > + > + return _attr->dev_attr.attr; > +} > + > static int iio_buffer_register_legacy_sysfs_groups(struct iio_dev > *indio_dev, > struct attribute > **buffer_attrs, > int buffer_attrcount, > @@ -1329,7 +1343,7 @@ static int __iio_buffer_alloc_sysfs_and_mask(struct > iio_buffer *buffer, > } > > scan_el_attrcount = 0; > - INIT_LIST_HEAD(>scan_el_dev_attr_list); > + INIT_LIST_HEAD(>buffer_attr_list); > channels = indio_dev->channels; > if (channels) { > /* new magic */ > @@ -1376,9 +1390,19 @@ static int __iio_buffer_alloc_sysfs_and_mask(struct > iio_buffer *buffer, > > buffer_attrcount += ARRAY_SIZE(iio_buffer_attrs); > > - attrn = buffer_attrcount; > + for (i = 0; i < buffer_attrcount; i++) { > + struct attribute *wrapped; > + > + wrapped = iio_buffer_wrap_attr(buffer, attr[i]); > + if (!wrapped) { > + ret = -ENOMEM; > + goto error_free_scan_mask; > + } > + attr[i] = wrapped; > + } > > - list_for_each_entry(p, >scan_el_dev_attr_list, l) > + attrn = 0; > + list_for_each_entry(p, >buffer_attr_list, l) > attr[attrn++] = >dev_attr.attr; > > buffer->buffer_group.name = kasprintf(GFP_KERNEL, "buffer%d", index); > @@ -1412,7 +1436,7 @@ static int __iio_buffer_alloc_sysfs_and_mask(struct > iio_buffer *buffer, > error_free_scan_mask: > bitmap_free(buffer->scan_mask); > error_cleanup_dynamic: > - iio_free_chan_devattr_list(>scan_el_dev_attr_list); > + iio_free_chan_devattr_list(>buffer_attr_list); > > return ret; > } > @@ -1443,7 +1467,7 @@ static void __iio_buffer_free_sysfs_and_mask(struct > iio_buffer *buffer) > bitmap_free(buffer->scan_mask); > kfree(buffer->buffer_group.name); > kfree(buffer->buffer_group.attrs); > - iio_free_chan_devattr_list(>scan_el_dev_attr_list); > + iio_free_chan_devattr_list(>buffer_attr_list); > } > > void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev) > diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h > index 3e555e58475b..41044320e581 100644 > --- a/include/linux/iio/buffer_impl.h > +++ b/include/linux/iio/buffer_impl.h > @@ -97,8 +97,8 @@ struct iio_buffer { > /* @scan_timestamp: Does the scan mode include a timestamp. */ > bool scan_timestamp; > > - /* @scan_el_dev_attr_list: List of scan element related attributes. */ > - struct list_head scan_el_dev_attr_list; > + /* @buffer_attr_list: List of buffer attributes. */ > + struct list_head buffer_attr_list; > > /* >* @buffer_group: Attributes of the new buffer group. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] PCI: dwc: Move forward the iATU detection process
On 26.03.2021 18:05, Marek Szyprowski wrote: > On 25.03.2021 21:19, Bjorn Helgaas wrote: >> On Thu, Mar 25, 2021 at 10:24:28AM +0100, Marek Szyprowski wrote: >>> On 25.01.2021 05:48, Zhiqiang Hou wrote: >>>> From: Hou Zhiqiang >>>> >>>> In the dw_pcie_ep_init(), it depends on the detected iATU region >>>> numbers to allocate the in/outbound window management bit map. >>>> It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number >>>> of iATU windows"). >>>> >>>> So this patch move the iATU region detection into a new function, >>>> move forward the detection to the very beginning of functions >>>> dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it >>>> from the dw_pcie_setup(), since it's more like a software >>>> perspective initialization step than hardware setup. >>>> >>>> Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") >>>> Signed-off-by: Hou Zhiqiang >>> This patch causes exynos-pcie to hang during the initialization. It >>> looks that some resources are not enabled yet, so calling >>> dw_pcie_iatu_detect() much earlier causes a hang. When I have some >>> time, >>> I will try to identify what is needed to call it properly. >> Thanks, I dropped it for now. We can add it back after we figure out >> what the exynos issue is. > Thanks, I will try to identify at which point of initialization it is > safe to call iATU region detection. I've just checked and it is enough to move the dw_pcie_iatu_detect(pci); after pp->ops->host_init(pp); in dw_pcie_host_init() to fix driver operation on Exynos SoCs with the $subject patch applied. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] PCI: dwc: Move forward the iATU detection process
On 25.03.2021 21:19, Bjorn Helgaas wrote: > On Thu, Mar 25, 2021 at 10:24:28AM +0100, Marek Szyprowski wrote: >> On 25.01.2021 05:48, Zhiqiang Hou wrote: >>> From: Hou Zhiqiang >>> >>> In the dw_pcie_ep_init(), it depends on the detected iATU region >>> numbers to allocate the in/outbound window management bit map. >>> It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number >>> of iATU windows"). >>> >>> So this patch move the iATU region detection into a new function, >>> move forward the detection to the very beginning of functions >>> dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it >>> from the dw_pcie_setup(), since it's more like a software >>> perspective initialization step than hardware setup. >>> >>> Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") >>> Signed-off-by: Hou Zhiqiang >> This patch causes exynos-pcie to hang during the initialization. It >> looks that some resources are not enabled yet, so calling >> dw_pcie_iatu_detect() much earlier causes a hang. When I have some time, >> I will try to identify what is needed to call it properly. > Thanks, I dropped it for now. We can add it back after we figure out > what the exynos issue is. Thanks, I will try to identify at which point of initialization it is safe to call iATU region detection. > For reference, here's the patch I dropped (I had made some minor > corrections to the commit log): > > commit fd4162f05194 ("PCI: dwc: Move iATU detection earlier") > Author: Hou Zhiqiang > Date: Mon Jan 25 12:48:03 2021 +0800 > > PCI: dwc: Move iATU detection earlier > > dw_pcie_ep_init() depends on the detected iATU region numbers to allocate > the in/outbound window management bitmap. It fails after 281f1f99cf3a > ("PCI: dwc: Detect number of iATU windows"). > > Move the iATU region detection into a new function, move the detection to > the very beginning of dw_pcie_host_init() and dw_pcie_ep_init(). Also > remove it from the dw_pcie_setup(), since it's more like a software > initialization step than hardware setup. > > Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") > Link: > https://lore.kernel.org/r/20210125044803.4310-1-zhiqiang@nxp.com > Tested-by: Kunihiko Hayashi > Signed-off-by: Hou Zhiqiang > Signed-off-by: Bjorn Helgaas > Reviewed-by: Rob Herring > Cc: sta...@vger.kernel.org # v5.11+ > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c > b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 1c25d8337151..8d028a88b375 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -705,6 +705,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > } > } > > + dw_pcie_iatu_detect(pci); > + > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); > if (!res) > return -EINVAL; > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c > b/drivers/pci/controller/dwc/pcie-designware-host.c > index 7e55b2b66182..52f6887179cd 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -319,6 +319,8 @@ int dw_pcie_host_init(struct pcie_port *pp) > return PTR_ERR(pci->dbi_base); > } > > + dw_pcie_iatu_detect(pci); > + > bridge = devm_pci_alloc_host_bridge(dev, 0); > if (!bridge) > return -ENOMEM; > diff --git a/drivers/pci/controller/dwc/pcie-designware.c > b/drivers/pci/controller/dwc/pcie-designware.c > index 004cb860e266..a945f0c0e73d 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -660,11 +660,9 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie > *pci) > pci->num_ob_windows = ob; > } > > -void dw_pcie_setup(struct dw_pcie *pci) > +void dw_pcie_iatu_detect(struct dw_pcie *pci) > { > - u32 val; > struct device *dev = pci->dev; > - struct device_node *np = dev->of_node; > struct platform_device *pdev = to_platform_device(dev); > > if (pci->version >= 0x480A || (!pci->version && > @@ -693,6 +691,13 @@ void dw_pcie_setup(struct dw_pcie *pci) > > dev_info(pci->dev, "Detected iATU regions: %u outbound, %u inbound", >pci->num_ob_windows, pci->num_ib_windows); &
Re: [PATCH net-next] net: stmmac: Fix kernel panic due to NULL pointer dereference of fpe_cfg
On 26.03.2021 10:10, mohammad.athari.ism...@intel.com wrote: > From: Mohammad Athari Bin Ismail > > In this patch, "net: stmmac: support FPE link partner hand-shaking > procedure", priv->plat->fpe_cfg wouldn`t be "devm_kzalloc"ed if > dma_cap->frpsel is 0 (Flexible Rx Parser is not supported in SoC) in > tc_init(). So, fpe_cfg will be remain as NULL and accessing it will cause > kernel panic. > > To fix this, move the "devm_kzalloc"ing of priv->plat->fpe_cfg before > dma_cap->frpsel checking in tc_init(). Additionally, checking of > priv->dma_cap.fpesel is added before calling stmmac_fpe_link_state_handle() > as only FPE supported SoC is allowed to call the function. > > Below is the kernel panic dump reported by Marek Szyprowski > : > > meson8b-dwmac ff3f.ethernet eth0: PHY [0.0:00] driver [RTL8211F Gigabit > Ethernet] (irq=35) > meson8b-dwmac ff3f.ethernet eth0: No Safety Features support found > meson8b-dwmac ff3f.ethernet eth0: PTP not supported by HW > meson8b-dwmac ff3f.ethernet eth0: configuring for phy/rgmii link mode > Unable to handle kernel NULL pointer dereference at virtual address > 0001 > Mem abort info: > ... > user pgtable: 4k pages, 48-bit VAs, pgdp=044eb000 > [0001] pgd=, p4d= > Internal error: Oops: 9604 [#1] PREEMPT SMP > Modules linked in: dw_hdmi_i2s_audio dw_hdmi_cec meson_gxl realtek > meson_gxbb_wdt snd_soc_meson_axg_sound_card dwmac_generic axg_audio > meson_dw_hdmi crct10dif_ce snd_soc_meson_card_utils snd_soc_meson_axg_tdmout > panfrost rc_odroid gpu_sched reset_meson_audio_arb meson_ir > snd_soc_meson_g12a_tohdmitx snd_soc_meson_axg_frddr sclk_div clk_phase > snd_soc_meson_codec_glue dwmac_meson8b snd_soc_meson_axg_fifo stmmac_platform > meson_rng meson_drm stmmac rtc_meson_vrtc rng_core meson_canvas pwm_meson > dw_hdmi mdio_mux_meson_g12a pcs_xpcs snd_soc_meson_axg_tdm_interface > snd_soc_meson_axg_tdm_formatter nvmem_meson_efuse display_connector > CPU: 1 PID: 7 Comm: kworker/u8:0 Not tainted 5.12.0-rc4-next-20210325+ > Hardware name: Hardkernel ODROID-C4 (DT) > Workqueue: events_power_efficient phylink_resolve > pstate: 2049 (nzCv daif +PAN -UAO -TCO BTYPE=--) > pc : stmmac_mac_link_up+0x14c/0x348 [stmmac] > lr : stmmac_mac_link_up+0x284/0x348 [stmmac] ... > Call trace: > stmmac_mac_link_up+0x14c/0x348 [stmmac] > phylink_resolve+0x104/0x420 > process_one_work+0x2a8/0x718 > worker_thread+0x48/0x460 > kthread+0x134/0x160 > ret_from_fork+0x10/0x18 > Code: b971ba60 350007c0 f958c260 f9402000 (39400401) > ---[ end trace 0c9deb6c510228aa ]--- > > Fixes: 5a5586112b92 ("net: stmmac: support FPE link partner hand-shaking > procedure") > Reported-by: Marek Szyprowski > Signed-off-by: Mohammad Athari Bin Ismail This fixes my issue. Thanks! Tested-by: Marek Szyprowski > --- > .../net/ethernet/stmicro/stmmac/stmmac_main.c | 6 -- > .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 20 +-- > 2 files changed, 14 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 170296820af0..27faf5e49360 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -997,7 +997,8 @@ static void stmmac_mac_link_down(struct phylink_config > *config, > stmmac_eee_init(priv); > stmmac_set_eee_pls(priv, priv->hw, false); > > - stmmac_fpe_link_state_handle(priv, false); > + if (priv->dma_cap.fpesel) > + stmmac_fpe_link_state_handle(priv, false); > } > > static void stmmac_mac_link_up(struct phylink_config *config, > @@ -1097,7 +1098,8 @@ static void stmmac_mac_link_up(struct phylink_config > *config, > stmmac_set_eee_pls(priv, priv->hw, true); > } > > - stmmac_fpe_link_state_handle(priv, true); > + if (priv->dma_cap.fpesel) > + stmmac_fpe_link_state_handle(priv, true); > } > > static const struct phylink_mac_ops stmmac_phylink_mac_ops = { > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c > b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c > index 1d84ee359808..4e70efc45458 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c > @@ -254,6 +254,16 @@ static int tc_init(struct stmmac_priv *priv) >priv->flow_entries_max); > } > > + if (!priv->plat->fpe_cfg) { > + priv->plat->fpe_cfg = devm_kzalloc(priv-&
Re: [PATCH] clk: bcm: rpi: Don't register as OF provider if !dev->np
On 25.03.2021 19:57, Nicolas Saenz Julienne wrote: > There are two ways clk-raspberrypi might be registered: through > device-tree or through an explicit platform device registration. The > latter happens after firmware/raspberrypi's probe, and it's limited to > RPi3s, which solely use the ARM clock to scale CPU's frequency. That > clock is matched with cpu0's device thanks to the ARM clock being > registered as a clkdev. > > In that scenario, don't register the device as an OF clock provider, as > it makes no sense and will cause trouble. > > Fixes: d4b4f1b6b97e ("clk: bcm: rpi: Add DT provider for the clocks") > Reported-by: Marek Szyprowski > Signed-off-by: Nicolas Saenz Julienne Tested-by: Marek Szyprowski > --- > drivers/clk/bcm/clk-raspberrypi.c | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/clk/bcm/clk-raspberrypi.c > b/drivers/clk/bcm/clk-raspberrypi.c > index f89b9cfc4309..27e85687326f 100644 > --- a/drivers/clk/bcm/clk-raspberrypi.c > +++ b/drivers/clk/bcm/clk-raspberrypi.c > @@ -337,10 +337,12 @@ static int raspberrypi_clk_probe(struct platform_device > *pdev) > if (ret) > return ret; > > - ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, > - clk_data); > - if (ret) > - return ret; > + if (dev->of_node) { > + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, > + clk_data); > + if (ret) > + return ret; > + } > > rpi->cpufreq = platform_device_register_data(dev, "raspberrypi-cpufreq", >-1, NULL, 0); Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH net-next] net: stmmac: support FPE link partner hand-shaking procedure
entries_max, priv->tc_off_max); > + > + if (!priv->plat->fpe_cfg) { > + priv->plat->fpe_cfg = devm_kzalloc(priv->device, > +sizeof(*priv->plat->fpe_cfg), > +GFP_KERNEL); > + if (!priv->plat->fpe_cfg) > + return -ENOMEM; > + } else { > + memset(priv->plat->fpe_cfg, 0, sizeof(*priv->plat->fpe_cfg)); > + } > + > return 0; > } > > @@ -829,13 +840,10 @@ static int tc_setup_taprio(struct stmmac_priv *priv, > if (fpe && !priv->dma_cap.fpesel) > return -EOPNOTSUPP; > > - ret = stmmac_fpe_configure(priv, priv->ioaddr, > -priv->plat->tx_queues_to_use, > -priv->plat->rx_queues_to_use, fpe); > - if (ret && fpe) { > - netdev_err(priv->dev, "failed to enable Frame Preemption\n"); > - return ret; > - } > + /* Actual FPE register configuration will be done after FPE handshake > + * is success. > + */ > + priv->plat->fpe_cfg->enable = fpe; > > ret = stmmac_est_configure(priv, priv->ioaddr, priv->plat->est, > priv->plat->clk_ptp_rate); > @@ -845,12 +853,29 @@ static int tc_setup_taprio(struct stmmac_priv *priv, > } > > netdev_info(priv->dev, "configured EST\n"); > + > + if (fpe) { > + stmmac_fpe_handshake(priv, true); > + netdev_info(priv->dev, "start FPE handshake\n"); > + } > + > return 0; > > disable: > priv->plat->est->enable = false; > stmmac_est_configure(priv, priv->ioaddr, priv->plat->est, >priv->plat->clk_ptp_rate); > + > + priv->plat->fpe_cfg->enable = false; > + stmmac_fpe_configure(priv, priv->ioaddr, > + priv->plat->tx_queues_to_use, > + priv->plat->rx_queues_to_use, > + false); > + netdev_info(priv->dev, "disabled FPE\n"); > + > + stmmac_fpe_handshake(priv, false); > + netdev_info(priv->dev, "stop FPE handshake\n"); > + > return ret; > } > > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h > index 10abc80b601e..072f269b1618 100644 > --- a/include/linux/stmmac.h > +++ b/include/linux/stmmac.h > @@ -144,6 +144,32 @@ struct stmmac_txq_cfg { > int tbs_en; > }; > > +/* FPE link state */ > +enum stmmac_fpe_state { > + FPE_STATE_OFF = 0, > + FPE_STATE_CAPABLE = 1, > + FPE_STATE_ENTERING_ON = 2, > + FPE_STATE_ON = 3, > +}; > + > +/* FPE link-partner hand-shaking mPacket type */ > +enum stmmac_mpacket_type { > + MPACKET_VERIFY = 0, > + MPACKET_RESPONSE = 1, > +}; > + > +enum stmmac_fpe_task_state_t { > + __FPE_REMOVING, > + __FPE_TASK_SCHED, > +}; > + > +struct stmmac_fpe_cfg { > + bool enable;/* FPE enable */ > + bool hs_enable; /* FPE handshake enable */ > + enum stmmac_fpe_state lp_fpe_state; /* Link Partner FPE state */ > + enum stmmac_fpe_state lo_fpe_state; /* Local station FPE state */ > +}; > + > struct plat_stmmacenet_data { > int bus_id; > int phy_addr; > @@ -155,6 +181,7 @@ struct plat_stmmacenet_data { > struct device_node *mdio_node; > struct stmmac_dma_cfg *dma_cfg; > struct stmmac_est *est; > + struct stmmac_fpe_cfg *fpe_cfg; > int clk_csr; > int has_gmac; > int enh_desc; Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] clk: Mark fwnodes when their clock provider is added
Hi On 10.02.2021 12:44, Tudor Ambarus wrote: > This is a follow-up for: > commit 3c9ea42802a1 ("clk: Mark fwnodes when their clock provider is > added/removed") > > The above commit updated the deprecated of_clk_add_provider(), > but missed to update the preferred of_clk_add_hw_provider(). > Update it now. > > Signed-off-by: Tudor Ambarus This patch, which landed in linux-next as commit 6579c8d97ad7 ("clk: Mark fwnodes when their clock provider is added") causes the following NULL pointer dereference on Raspberry Pi 3b+ boards: --->8--- raspberrypi-firmware soc:firmware: Attached to firmware from 2020-01-06T13:05:25 Unable to handle kernel NULL pointer dereference at virtual address 0050 Mem abort info: ESR = 0x9604 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 Data abort info: ISV = 0, ISS = 0x0004 CM = 0, WnR = 0 [0050] user address but active_mm is swapper Internal error: Oops: 9604 [#1] PREEMPT SMP Modules linked in: CPU: 0 PID: 10 Comm: kworker/0:1 Not tainted 5.12.0-rc4+ #2764 Hardware name: Raspberry Pi 3 Model B (DT) Workqueue: events deferred_probe_work_func pstate: 0005 (nzcv daif -PAN -UAO -TCO BTYPE=--) pc : of_clk_add_hw_provider+0xac/0xe8 lr : of_clk_add_hw_provider+0x94/0xe8 sp : 8000130936b0 x29: 8000130936b0 x28: 800012494e04 x27: 3b18cb05 x26: 3aa5c010 x25: x24: x23: 3aa1e380 x22: 8000106830d0 x21: 80001233f180 x20: 0018 x19: x18: 8000124d38b0 x17: 0013 x16: 0014 x15: 8000125758b0 x14: 000184e0 x13: 292e x12: 80001258dd98 x11: 0001 x10: 0101010101010101 x9 : 80001233f288 x8 : 7f7f7f7f7f7f7f7f x7 : fefefefeff6c626f x6 : 5d636d8080808080 x5 : 006d635d x4 : x3 : x2 : 540eb5edae191600 x1 : x0 : Call trace: of_clk_add_hw_provider+0xac/0xe8 devm_of_clk_add_hw_provider+0x5c/0xb8 raspberrypi_clk_probe+0x110/0x210 platform_probe+0x90/0xd8 really_probe+0x108/0x3c0 driver_probe_device+0x60/0xc0 __device_attach_driver+0x9c/0xd0 bus_for_each_drv+0x70/0xc8 __device_attach+0xec/0x150 device_initial_probe+0x10/0x18 bus_probe_device+0x94/0xa0 device_add+0x47c/0x780 platform_device_add+0x110/0x248 platform_device_register_full+0x120/0x150 rpi_firmware_probe+0x158/0x1f8 platform_probe+0x90/0xd8 really_probe+0x108/0x3c0 driver_probe_device+0x60/0xc0 __device_attach_driver+0x9c/0xd0 bus_for_each_drv+0x70/0xc8 __device_attach+0xec/0x150 device_initial_probe+0x10/0x18 bus_probe_device+0x94/0xa0 deferred_probe_work_func+0x70/0xa8 process_one_work+0x2a8/0x718 worker_thread+0x48/0x460 kthread+0x134/0x160 ret_from_fork+0x10/0x18 Code: b1006294 54c0 b140069f 5488 (3940e280) ---[ end trace 7ead5ec2f0c51cfe ]--- This patch mainly revealed that clk/bcm/clk-raspberrypi.c driver calls devm_of_clk_add_hw_provider(), with a device pointer, which has a NULL dev->of_node. I'm not sure if adding a check for a NULL np in of_clk_add_hw_provider() is a right fix, though. > --- > drivers/clk/clk.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 27ff90eacb1f..9370e4dfecae 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -4594,6 +4594,8 @@ int of_clk_add_hw_provider(struct device_node *np, > if (ret < 0) > of_clk_del_provider(np); > > + fwnode_dev_initialized(>fwnode, true); > + > return ret; > } > EXPORT_SYMBOL_GPL(of_clk_add_hw_provider); Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] PCI: dwc: Move forward the iATU detection process
Hi, On 25.01.2021 05:48, Zhiqiang Hou wrote: > From: Hou Zhiqiang > > In the dw_pcie_ep_init(), it depends on the detected iATU region > numbers to allocate the in/outbound window management bit map. > It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number > of iATU windows"). > > So this patch move the iATU region detection into a new function, > move forward the detection to the very beginning of functions > dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it > from the dw_pcie_setup(), since it's more like a software > perspective initialization step than hardware setup. > > Fixes: 281f1f99cf3a ("PCI: dwc: Detect number of iATU windows") > Signed-off-by: Hou Zhiqiang This patch causes exynos-pcie to hang during the initialization. It looks that some resources are not enabled yet, so calling dw_pcie_iatu_detect() much earlier causes a hang. When I have some time, I will try to identify what is needed to call it properly. > --- > drivers/pci/controller/dwc/pcie-designware-ep.c | 2 ++ > drivers/pci/controller/dwc/pcie-designware-host.c | 2 ++ > drivers/pci/controller/dwc/pcie-designware.c | 11 --- > drivers/pci/controller/dwc/pcie-designware.h | 1 + > 4 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c > b/drivers/pci/controller/dwc/pcie-designware-ep.c > index bcd1cd9ba8c8..fcf935bf6f5e 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -707,6 +707,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > } > } > > + dw_pcie_iatu_detect(pci); > + > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); > if (!res) > return -EINVAL; > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c > b/drivers/pci/controller/dwc/pcie-designware-host.c > index 8a84c005f32b..8eae817c138d 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -316,6 +316,8 @@ int dw_pcie_host_init(struct pcie_port *pp) > return PTR_ERR(pci->dbi_base); > } > > + dw_pcie_iatu_detect(pci); > + > bridge = devm_pci_alloc_host_bridge(dev, 0); > if (!bridge) > return -ENOMEM; > diff --git a/drivers/pci/controller/dwc/pcie-designware.c > b/drivers/pci/controller/dwc/pcie-designware.c > index 5b72a5448d2e..5b9bf02d918b 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -654,11 +654,9 @@ static void dw_pcie_iatu_detect_regions(struct dw_pcie > *pci) > pci->num_ob_windows = ob; > } > > -void dw_pcie_setup(struct dw_pcie *pci) > +void dw_pcie_iatu_detect(struct dw_pcie *pci) > { > - u32 val; > struct device *dev = pci->dev; > - struct device_node *np = dev->of_node; > struct platform_device *pdev = to_platform_device(dev); > > if (pci->version >= 0x480A || (!pci->version && > @@ -687,6 +685,13 @@ void dw_pcie_setup(struct dw_pcie *pci) > > dev_info(pci->dev, "Detected iATU regions: %u outbound, %u inbound", >pci->num_ob_windows, pci->num_ib_windows); > +} > + > +void dw_pcie_setup(struct dw_pcie *pci) > +{ > + u32 val; > + struct device *dev = pci->dev; > + struct device_node *np = dev->of_node; > > if (pci->link_gen > 0) > dw_pcie_link_set_max_speed(pci, pci->link_gen); > diff --git a/drivers/pci/controller/dwc/pcie-designware.h > b/drivers/pci/controller/dwc/pcie-designware.h > index 5d979953800d..867369d4c4f7 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -305,6 +305,7 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 > func_no, int index, > void dw_pcie_disable_atu(struct dw_pcie *pci, int index, >enum dw_pcie_region_type type); > void dw_pcie_setup(struct dw_pcie *pci); > +void dw_pcie_iatu_detect(struct dw_pcie *pci); > > static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val) > { Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v3] usb: dwc3: gadget: Prevent EP queuing while stopping transfers
gt;>>> [ 195.887755] RDX: dfff RSI: ffea RDI: >>>>>>>> >>>>>>>> [ 195.894893] RBP: 9ce8c8f2b028 R08: a0732288 R09: >>>>>>>> 9ffb >>>>>>>> [ 195.902034] R10: e000 R11: 3fff R12: >>>>>>>> 00041006 >>>>>>>> [ 195.909170] R13: a53c00373c24 R14: 9ce8c11dadb0 R15: >>>>>>>> 9ce8c2861700 >>>>>>>> [ 195.916310] FS: () GS:9ce8fe20() >>>>>>>> knlGS: >>>>>>>> [ 195.924409] CS: 0010 DS: ES: CR0: 80050033 >>>>>>>> [ 195.930161] CR2: f7f694a0 CR3: 38e0c000 CR4: >>>>>>>> 001006f0 >>>>>>>> [ 195.937300] Call Trace: >>>>>>>> [ 195.939755] __dwc3_gadget_ep_enable+0x2d4/0x4e0 >>>>>>>> [ 195.944393] ? dwc3_remove_requests.constprop.0+0x86/0x170 >>>>>>> Odd that this change would affect the USB enablment path, as they were >>>>>>> focused on the pullup disable path. Would you happen to have any >>>>>>> downstream changes on top of v5.12-rc4 we could review to see if they >>>>>>> are still required? (ie where is the dwc3_remove_requests() coming from >>>>>>> during ep enable) >>>>>> You may check my branch [1] on GH. Basically you may be interested in >>>>>> the commit: >>>>>> 0f86df1294ee7523060cc16eafaf4898c693eab0 REVERTME: usb: dwc3: gadget: >>>>>> skip endpoints ep[18]{in,out} >>>>>> Otherwise it's a clean v5.12-rc4 with a revert and another USB PHY >>>>>> suspend fix (which also shouldn't affect this). >>>>> Can you link your GH reference? >>>> Oops, sorry. >>>> Here we are: >>>> >>>> [1]: https://github.com/andy-shev/linux/tree/eds-acpi >>>> >>> Thanks, I took a look and even tried it on my device running 5.12-rc4, >>> but wasn't able to see the same problem. Could you help collect the >>> ftrace after enabling the tracing KCONFIG and running the below sequence? >>> >>> 1. Mount debugfs >>> 2. Set up tracing instance >>> >>> mkdir /sys/kernel/debug/tracing/instances/usb >>> echo 1 > >>> /sys/kernel/debug/tracing/instances/usb/events/dwc3/dwc3_complete_trb/enable >>> echo 1 > >>> /sys/kernel/debug/tracing/instances/usb/events/dwc3/dwc3_ctrl_req/enable >>> echo 1 > >>> /sys/kernel/debug/tracing/instances/usb/events/dwc3/dwc3_ep_dequeue/enable >>> echo 1 > >>> /sys/kernel/debug/tracing/instances/usb/events/dwc3/dwc3_ep_queue/enable >>> echo 1 > >>> /sys/kernel/debug/tracing/instances/usb/events/dwc3/dwc3_gadget_ep_cmd/enable >>> echo 1 > >>> /sys/kernel/debug/tracing/instances/usb/events/dwc3/dwc3_gadget_ep_disable/enable >>> echo 1 > >>> /sys/kernel/debug/tracing/instances/usb/events/dwc3/dwc3_gadget_ep_enable/enable >>> echo 1 > >>> /sys/kernel/debug/tracing/instances/usb/events/dwc3/dwc3_gadget_giveback/enable >>> echo 1 > >>> /sys/kernel/debug/tracing/instances/usb/events/dwc3/dwc3_prepare_trb/enable >>> echo 1 > /sys/kernel/debug/tracing/instances/usb/tracing_on >>> >>> 3. Run your test, which should include: >>> - echo "" > /sys/kernel/config/usb_gadget/g1/UDC >>> - echo > /sys/kernel/config/usb_gadget/g1/UDC >>> >>> 4. Collect the trace output: >>> cat /sys/kernel/debug/tracing/instances/usb/trace >> Here we are (I cherry-picked again reverted patch, other stays the same) [2]. >> On top I put a warning, so you may see timestamps. >> >> Dunno how long it will stay there, please confirm that you got it. >> >> [2]: https://paste.ubuntu.com/p/jNF565ypPp/ >> > Hi Andy, > > Would you be able to give the below change a try? > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 80606b8..cd58bd5 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -791,10 +791,6 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep > *dep) > reg &= ~DWC3_DALEPENA_EP(dep->number); > dwc3_writel(dwc->regs, DWC3_DALEPENA, reg); > > -dep->stream_capable = false; > -dep->type = 0; > -dep->flags = 0; > - > /* Clear out the ep descriptors for non-ep0 */ > if (dep->number > 1) { > dep->endpoint.comp_desc = NULL; > @@ -803,6 +799,10 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep > *dep) > > dwc3_remove_requests(dwc, dep); > > +dep->stream_capable = false; > +dep->type = 0; > +dep->flags = 0; > + > return 0; > } Feel free to add my: Tested-by: Marek Szyprowski The above change fixed the issue I was about to complain, introduced (or revealed?) by the $subject commit. Before applying the above fix I've observed following warning during system power off: [ cut here ] dwc3 1540.usb: No resource for ep2in WARNING: CPU: 0 PID: 162 at drivers/usb/dwc3/gadget.c:361 dwc3_send_gadget_ep_cmd+0x95c/0xae8 Modules linked in: cpufreq_powersave cpufreq_conservative brcmfmac brcmutil cfg80211 crct10dif_ce s3fwrn5_i2c s3fwrn5 nci nfc s5p_mfc hci_uart s5p_jpeg btqca btbcm exynos_gsc v4l2_mem2mem videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_common bluetooth videodev mc panfrost gpu_sched ecdh_generic ecc rfkill ip_tables x_tables ipv6 CPU: 0 PID: 162 Comm: irq/151-dwc3 Not tainted 5.12.0-rc3+ #2713 Hardware name: Samsung TM2E board (DT) pstate: 4085 (nZcv daIf -PAN -UAO -TCO BTYPE=--) pc : dwc3_send_gadget_ep_cmd+0x95c/0xae8 ... Call trace: dwc3_send_gadget_ep_cmd+0x95c/0xae8 __dwc3_gadget_ep_enable+0x40c/0x578 dwc3_gadget_ep_enable+0x5c/0xf0 usb_ep_enable+0x40/0x1d0 ecm_set_alt+0xa8/0x1b8 composite_setup+0x8b8/0x1840 dwc3_ep0_delegate_req+0x38/0x58 dwc3_ep0_interrupt+0xd2c/0x1048 dwc3_thread_interrupt+0xe7c/0x10f8 irq_thread_fn+0x28/0x98 irq_thread+0x17c/0x298 kthread+0x134/0x160 ret_from_fork+0x10/0x18 irq event stamp: 1220 hardirqs last enabled at (1219): [] _raw_spin_unlock_irq+0x3c/0x80 hardirqs last disabled at (1220): [] _raw_spin_lock_irqsave+0xb4/0x148 softirqs last enabled at (924): [] _stext+0x4e8/0x614 softirqs last disabled at (901): [] irq_exit+0x19c/0x1a8 ---[ end trace 84804a1a38a2c9f7 ]--- Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v3] amba: Remove deferred device addition
Hi Saravana, On 05.03.2021 19:02, Saravana Kannan wrote: > On Fri, Mar 5, 2021 at 3:45 AM Marek Szyprowski > wrote: >> On 04.03.2021 20:51, Saravana Kannan wrote: >>> The uevents generated for an amba device need PID and CID information >>> that's available only when the amba device is powered on, clocked and >>> out of reset. So, if those resources aren't available, the information >>> can't be read to generate the uevents. To workaround this requirement, >>> if the resources weren't available, the device addition was deferred and >>> retried periodically. >>> >>> However, this deferred addition retry isn't based on resources becoming >>> available. Instead, it's retried every 5 seconds and causes arbitrary >>> probe delays for amba devices and their consumers. >>> >>> Also, maintaining a separate deferred-probe like mechanism is >>> maintenance headache. >>> >>> With this commit, instead of deferring the device addition, we simply >>> defer the generation of uevents for the device and probing of the device >>> (because drivers needs PID and CID to match) until the PID and CID >>> information can be read. This allows us to delete all the amba specific >>> deferring code and also avoid the arbitrary probing delays. >>> >>> Cc: Rob Herring >>> Cc: Ulf Hansson >>> Cc: John Stultz >>> Cc: Saravana Kannan >>> Cc: Linus Walleij >>> Cc: Sudeep Holla >>> Cc: Nicolas Saenz Julienne >>> Cc: Geert Uytterhoeven >>> Cc: Marek Szyprowski >>> Cc: Russell King >>> Signed-off-by: Saravana Kannan >>> --- >>> >>> v1 -> v2: >>> - Dropped RFC tag >>> - Complete rewrite to not use stub devices. >>> v2 -> v3: >>> - Flipped the if() condition for hard-coded periphids. >>> - Added a stub driver to handle the case where all amba drivers are >>> modules loaded by uevents. >>> - Cc Marek after I realized I forgot to add him. >>> >>> Marek, >>> >>> Would you mind testing this? It looks okay with my limited testing. >> It looks it works fine on my test systems. I've checked current >> linux-next and this patch. You can add: >> >> Tested-by: Marek Szyprowski > Hi Marek, > > Thanks! Does your test set up have amda drivers that are loaded based > on uevents? That's the one I couldn't test. I've checked both, the built-in and all amba drivers compiled as modules, loaded by udev. Both works fine here. >> I've briefly scanned the code and I'm curious how does it work. Does it >> depend on the recently introduced "fw_devlink=on" feature? I don't see >> other mechanism, which would trigger matching amba device if pm domains, >> clocks or resets were not available on time to read pid/cid while adding >> a device... > No, it does not depend on fw_devlink or device links in any way. > > When a device is attempted to be probed (when it's added or during > deferred probe), it's matched with all the drivers on the bus. > When a new driver is registered to a bus, all devices in that bus are > matched with the driver to see if they'll work together. > That's how match is called. And match() can return -EPROBE_DEFER and > that'll cause the device to be put in the deferred probe list by > driver core. > > The tricky part in this patch was the uevent handling and the > chicken-and-egg issue I talk about in the comments. Thanks for the explanation. This EPROBE_DEFER support in match() callback must be something added after I crafted that periodic retry based workaround. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v3] amba: Remove deferred device addition
Hi Saravana, On 04.03.2021 20:51, Saravana Kannan wrote: > The uevents generated for an amba device need PID and CID information > that's available only when the amba device is powered on, clocked and > out of reset. So, if those resources aren't available, the information > can't be read to generate the uevents. To workaround this requirement, > if the resources weren't available, the device addition was deferred and > retried periodically. > > However, this deferred addition retry isn't based on resources becoming > available. Instead, it's retried every 5 seconds and causes arbitrary > probe delays for amba devices and their consumers. > > Also, maintaining a separate deferred-probe like mechanism is > maintenance headache. > > With this commit, instead of deferring the device addition, we simply > defer the generation of uevents for the device and probing of the device > (because drivers needs PID and CID to match) until the PID and CID > information can be read. This allows us to delete all the amba specific > deferring code and also avoid the arbitrary probing delays. > > Cc: Rob Herring > Cc: Ulf Hansson > Cc: John Stultz > Cc: Saravana Kannan > Cc: Linus Walleij > Cc: Sudeep Holla > Cc: Nicolas Saenz Julienne > Cc: Geert Uytterhoeven > Cc: Marek Szyprowski > Cc: Russell King > Signed-off-by: Saravana Kannan > --- > > v1 -> v2: > - Dropped RFC tag > - Complete rewrite to not use stub devices. > v2 -> v3: > - Flipped the if() condition for hard-coded periphids. > - Added a stub driver to handle the case where all amba drivers are >modules loaded by uevents. > - Cc Marek after I realized I forgot to add him. > > Marek, > > Would you mind testing this? It looks okay with my limited testing. It looks it works fine on my test systems. I've checked current linux-next and this patch. You can add: Tested-by: Marek Szyprowski I've briefly scanned the code and I'm curious how does it work. Does it depend on the recently introduced "fw_devlink=on" feature? I don't see other mechanism, which would trigger matching amba device if pm domains, clocks or resets were not available on time to read pid/cid while adding a device... Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] Revert "ARM: dts: exynos: Remove 'opp-shared' from Exynos4412 bus OPP-tables"
Hi Markus, On 22.02.2021 10:54, Markus Reichl wrote: > This reverts commit a23beead41a18c3be3ca409cb52f35bc02e601b9. > > I'm running an Odroid-X2 as headless 24/7 server. > With plain stable 5.10.1 I had 54 up days without problems. > With opp-shared removed on kernels before and now on 5.11 > my system freezes after some days on disk activity to eMMC > (rsync, apt upgrade). > > The spontaneous hangs are not easy to reproduce but testing this > for several months now I am quite confident that there is something > wrong with this patch. > > Signed-off-by: Markus Reichl Thanks for the report. IMHO a straight revert is a bad idea. I would prefer to keep current opp definitions and disable the affected devfreq devices (probably right bus would be enough) or try to identify which transitions are responsible for that issue. I know that it would take some time to identify them, but that would be the best solution. Reverting leads to incorrect hardware description, what in turn confuses the driver and framework, what in turn hides a real problem. Another problem related to devfreq on Exynos4412 has been introduced recently by the commit 86ad9a24f21e ("PM / devfreq: Add required OPPs support to passive governor"). You can see lots of the messages like this one: devfreq soc:bus-acp: failed to update devfreq using passive governor I didn't have time to check what's wrong there, but I consider devfreq on Exynos a little bit broken, so another solution would be just to disable it in the exynos_defconfig. > --- > arch/arm/boot/dts/exynos4412.dtsi | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/arm/boot/dts/exynos4412.dtsi > b/arch/arm/boot/dts/exynos4412.dtsi > index a142fe84010b..6246df278431 100644 > --- a/arch/arm/boot/dts/exynos4412.dtsi > +++ b/arch/arm/boot/dts/exynos4412.dtsi > @@ -405,6 +405,7 @@ > > bus_dmc_opp_table: opp-table1 { > compatible = "operating-points-v2"; > + opp-shared; > > opp-1 { > opp-hz = /bits/ 64 <1>; > @@ -431,6 +432,7 @@ > > bus_acp_opp_table: opp-table2 { > compatible = "operating-points-v2"; > + opp-shared; > > opp-1 { > opp-hz = /bits/ 64 <1>; > @@ -500,6 +502,7 @@ > > bus_leftbus_opp_table: opp-table3 { > compatible = "operating-points-v2"; > + opp-shared; > > opp-1 { > opp-hz = /bits/ 64 <1>; > @@ -522,6 +525,7 @@ > > bus_display_opp_table: opp-table4 { > compatible = "operating-points-v2"; > + opp-shared; > > opp-16000 { > opp-hz = /bits/ 64 <16000>; > @@ -533,6 +537,7 @@ > > bus_fsys_opp_table: opp-table5 { > compatible = "operating-points-v2"; > + opp-shared; > > opp-1 { > opp-hz = /bits/ 64 <1>; > @@ -544,6 +549,7 @@ > > bus_peri_opp_table: opp-table6 { > compatible = "operating-points-v2"; > + opp-shared; > > opp-5000 { > opp-hz = /bits/ 64 <5000>; Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] dt-bindings: iio: samsung,exynos-adc: add common clock properties
Hi Krzysztof, On 12.02.2021 17:38, Krzysztof Kozlowski wrote: > Add common properties appearing in DTSes (assigned-clocks and similar) > to fix dtbs_check warnings like: > >arch/arm/boot/dts/exynos3250-artik5-eval.dt.yaml: > adc@126c: assigned-clock-rates: [[600]] is not of type 'object' >arch/arm/boot/dts/exynos3250-artik5-eval.dt.yaml: > adc@126c: assigned-clocks: [[7, 238]] is not of type 'object' Does it mean that assigned-clocks related properties have to be added to almost all bindings? IMHO this is an over-engineering and this has to be handled somewhere else... > Signed-off-by: Krzysztof Kozlowski > --- > .../bindings/iio/adc/samsung,exynos-adc.yaml | 12 > 1 file changed, 12 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml > b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml > index c65921e66dc1..ce03132f8ebc 100644 > --- a/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/samsung,exynos-adc.yaml > @@ -27,6 +27,18 @@ properties: > reg: > maxItems: 1 > > + assigned-clocks: > +minItems: 1 > +maxItems: 3 > + > + assigned-clock-parents: > +minItems: 1 > +maxItems: 3 > + > + assigned-clock-rates: > +minItems: 1 > +maxItems: 3 > + > clocks: > description: > Phandle to ADC bus clock. For Exynos3250 additional clock is needed. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] misc: fastrpc: fix incorrect usage of dma_map_sgtable
Hi On 08.02.2021 21:04, Jonathan Marek wrote: > dma_map_sgtable() returns 0 on success, which is the opposite of what this > code was doing. > > Fixes: 7cd7edb89437 ("misc: fastrpc: fix common struct sg_table related > issues") > Signed-off-by: Jonathan Marek Right, I'm really sorry for this regression. Acked-by: Marek Szyprowski > --- > drivers/misc/fastrpc.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index 70eb5ed942d0..f12e909034ac 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -520,12 +520,13 @@ fastrpc_map_dma_buf(struct dma_buf_attachment > *attachment, > { > struct fastrpc_dma_buf_attachment *a = attachment->priv; > struct sg_table *table; > + int ret; > > table = >sgt; > > - if (!dma_map_sgtable(attachment->dev, table, dir, 0)) > - return ERR_PTR(-ENOMEM); > - > + ret = dma_map_sgtable(attachment->dev, table, dir, 0); > + if (ret) > + table = ERR_PTR(ret); > return table; > } > Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v4 0/8] Make fw_devlink=on more forgiving
Hi Saravana, On 05.02.2021 23:26, Saravana Kannan wrote: > There are a lot of devices/drivers where they never have a struct device > created for them or the driver initializes the hardware without ever > binding to the struct device. > > This series is intended to avoid any boot regressions due to such > devices/drivers when fw_devlink=on and also address the handling of > optional suppliers. > > Patch 1 and 2 addresses the issue of firmware nodes that look like > they'll have struct devices created for them, but will never actually > have struct devices added for them. For example, DT nodes with a > compatible property that don't have devices added for them. > > Patch 3 and 4 allow for handling optional DT bindings. > > Patch 5 sets up a generic API to handle drivers that never bind with > their devices. > > Patch 6 through 8 update different frameworks to use the new API. This patchset fixes probing issue observed on various Exynos based boards even with commit c09a3e6c97f0 ("soc: samsung: pm_domains: Convert to regular platform driver") reverted. Thanks! Tested-by: Marek Szyprowski Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
Hi Saravana, On 06.02.2021 05:32, Saravana Kannan wrote: > On Fri, Feb 5, 2021 at 9:55 AM Saravana Kannan wrote: >> On Fri, Feb 5, 2021 at 9:52 AM Geert Uytterhoeven >> wrote: >>> On Fri, Feb 5, 2021 at 6:20 PM Saravana Kannan wrote: >>>> On Fri, Feb 5, 2021 at 2:20 AM Geert Uytterhoeven >>>> wrote: >>>>> On Fri, Feb 5, 2021 at 11:06 AM Saravana Kannan >>>>> wrote: >>>>>> On Fri, Feb 5, 2021 at 12:06 AM Geert Uytterhoeven >>>>>> wrote: >>>>>>> On Fri, Feb 5, 2021 at 8:38 AM Marek Szyprowski >>>>>>> wrote: >>>>>>>> On 04.02.2021 22:31, Saravana Kannan wrote: >>>>>>>>> On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski >>>>>>>>> wrote: >>>>>>>>>> On 21.01.2021 23:57, Saravana Kannan wrote: >>>>>>>>>>> This allows fw_devlink to create device links between consumers of >>>>>>>>>>> an >>>>>>>>>>> interrupt and the supplier of the interrupt. >>>>>>>>>>> >>>>>>>>>>> Cc: Marc Zyngier >>>>>>>>>>> Cc: Kevin Hilman >>>>>>>>>>> Cc: Greg Kroah-Hartman >>>>>>>>>>> Reviewed-by: Rob Herring >>>>>>>>>>> Reviewed-by: Thierry Reding >>>>>>>>>>> Reviewed-by: Linus Walleij >>>>>>>>>>> Signed-off-by: Saravana Kannan >>>>>>>>>> This patch landed some time ago in linux-next as commit 4104ca776ba3 >>>>>>>>>> ("of: property: Add fw_devlink support for interrupts"). It breaks >>>>>>>>>> MMC >>>>>>>>>> host controller operation on ARM Juno R1 board (the mmci@5 device >>>>>>>>>> defined in arch/arm64/boot/dts/arm/juno-motherboard.dtsi). I didn't >>>>>>>>> I grepped around and it looks like the final board file is this or >>>>>>>>> whatever includes it? >>>>>>>>> arch/arm64/boot/dts/arm/juno-base.dtsi >>>>>>>> The final board file is arch/arm64/boot/dts/arm/juno-r1.dts >>>>>>>>> This patch just finds the interrupt-parent and then tries to use that >>>>>>>>> as a supplier if "interrupts" property is listed. But the only >>>>>>>>> interrupt parent I can see is: >>>>>>>>> gic: interrupt-controller@2c01 { >>>>>>>>> compatible = "arm,gic-400", "arm,cortex-a15-gic"; >>>>>>>>> >>>>>>>>> And the driver uses IRQCHIP_DECLARE() and hence should be pretty much >>>>>>>>> a NOP since those suppliers are never devices and are ignored. >>>>>>>>> $ git grep "arm,gic-400" -- drivers/ >>>>>>>>> drivers/irqchip/irq-gic.c:IRQCHIP_DECLARE(gic_400, "arm,gic-400", >>>>>>>>> gic_of_init); >>>>>>>>> >>>>>>>>> This doesn't make any sense. Am I looking at the right files? Am I >>>>>>>>> missing something? >>>>>>>> Okay, I've added displaying a list of deferred devices when mounting >>>>>>>> rootfs fails and got following items: >>>>>>>> >>>>>>>> Deferred devices: >>>>>>>> 1800.ethernetplatform: probe deferral - supplier >>>>>>>> bus@800:motherboard-bus not ready >>>>>>>> 1c05.mmciamba: probe deferral - supplier >>>>>>>> bus@800:motherboard-bus not ready >>>>>>>> 1c1d.gpioamba: probe deferral - supplier >>>>>>>> bus@800:motherboard-bus not ready >>>>>>>> 2b60.iommu platform: probe deferral - wait for supplier >>>>>>>> scpi-power-domains >>>>>>>> 7ff5.hdlcd platform: probe deferral - wait for supplier scpi-clk >>>>>>>> 7ff6.hdlcd platform: probe deferral - wait for supplier scpi-clk >>>>>>>> 1c06.kmi amba: probe deferral - supplier >>>>>>>> bus@800:motherboard-bus no
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
Hi Saravana, On 04.02.2021 22:31, Saravana Kannan wrote: > On Thu, Feb 4, 2021 at 3:52 AM Marek Szyprowski > wrote: >> On 21.01.2021 23:57, Saravana Kannan wrote: >>> This allows fw_devlink to create device links between consumers of an >>> interrupt and the supplier of the interrupt. >>> >>> Cc: Marc Zyngier >>> Cc: Kevin Hilman >>> Cc: Greg Kroah-Hartman >>> Reviewed-by: Rob Herring >>> Reviewed-by: Thierry Reding >>> Reviewed-by: Linus Walleij >>> Signed-off-by: Saravana Kannan >> This patch landed some time ago in linux-next as commit 4104ca776ba3 >> ("of: property: Add fw_devlink support for interrupts"). It breaks MMC >> host controller operation on ARM Juno R1 board (the mmci@5 device >> defined in arch/arm64/boot/dts/arm/juno-motherboard.dtsi). I didn't > I grepped around and it looks like the final board file is this or > whatever includes it? > arch/arm64/boot/dts/arm/juno-base.dtsi The final board file is arch/arm64/boot/dts/arm/juno-r1.dts > This patch just finds the interrupt-parent and then tries to use that > as a supplier if "interrupts" property is listed. But the only > interrupt parent I can see is: > gic: interrupt-controller@2c01 { > compatible = "arm,gic-400", "arm,cortex-a15-gic"; > > And the driver uses IRQCHIP_DECLARE() and hence should be pretty much > a NOP since those suppliers are never devices and are ignored. > $ git grep "arm,gic-400" -- drivers/ > drivers/irqchip/irq-gic.c:IRQCHIP_DECLARE(gic_400, "arm,gic-400", > gic_of_init); > > This doesn't make any sense. Am I looking at the right files? Am I > missing something? Okay, I've added displaying a list of deferred devices when mounting rootfs fails and got following items: Deferred devices: 1800.ethernet platform: probe deferral - supplier bus@800:motherboard-bus not ready 1c05.mmci amba: probe deferral - supplier bus@800:motherboard-bus not ready 1c1d.gpio amba: probe deferral - supplier bus@800:motherboard-bus not ready 2b60.iommu platform: probe deferral - wait for supplier scpi-power-domains 7ff5.hdlcd platform: probe deferral - wait for supplier scpi-clk 7ff6.hdlcd platform: probe deferral - wait for supplier scpi-clk 1c06.kmi amba: probe deferral - supplier bus@800:motherboard-bus not ready 1c07.kmi amba: probe deferral - supplier bus@800:motherboard-bus not ready 1c17.rtc amba: probe deferral - supplier bus@800:motherboard-bus not ready 1c0f.wdt amba: probe deferral - supplier bus@800:motherboard-bus not ready gpio-keys Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0) I don't see the 'bus@800:motherboard-bus' on the deferred devices list, so it looks that device core added a link to something that is not a platform device... Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v2 2/2] of: property: Add fw_devlink support for interrupts
Hi Saravana, On 21.01.2021 23:57, Saravana Kannan wrote: > This allows fw_devlink to create device links between consumers of an > interrupt and the supplier of the interrupt. > > Cc: Marc Zyngier > Cc: Kevin Hilman > Cc: Greg Kroah-Hartman > Reviewed-by: Rob Herring > Reviewed-by: Thierry Reding > Reviewed-by: Linus Walleij > Signed-off-by: Saravana Kannan This patch landed some time ago in linux-next as commit 4104ca776ba3 ("of: property: Add fw_devlink support for interrupts"). It breaks MMC host controller operation on ARM Juno R1 board (the mmci@5 device defined in arch/arm64/boot/dts/arm/juno-motherboard.dtsi). I didn't check further what's wrong there as without MMC mounting rootfs fails in my test system. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v1 0/2] Make fw_devlink=on more forgiving
Hi Saravana, On 01.02.2021 10:02, Saravana Kannan wrote: > On Mon, Feb 1, 2021 at 12:05 AM Marek Szyprowski > wrote: >> On 30.01.2021 05:08, Saravana Kannan wrote: >>> On Fri, Jan 29, 2021 at 8:03 PM Saravana Kannan >>> wrote: >>>> This patch series solves two general issues with fw_devlink=on >>>> >>>> Patch 1/2 addresses the issue of firmware nodes that look like they'll >>>> have struct devices created for them, but will never actually have >>>> struct devices added for them. For example, DT nodes with a compatible >>>> property that don't have devices added for them. >>>> >>>> Patch 2/2 address (for static kernels) the issue of optional suppliers >>>> that'll never have a driver registered for them. So, if the device could >>>> have probed with fw_devlink=permissive with a static kernel, this patch >>>> should allow those devices to probe with a fw_devlink=on. This doesn't >>>> solve it for the case where modules are enabled because there's no way >>>> to tell if a driver will never be registered or it's just about to be >>>> registered. I have some other ideas for that, but it'll have to come >>>> later thinking about it a bit. >>>> >>>> These two patches might remove the need for several other patches that >>>> went in as fixes for commit e590474768f1 ("driver core: Set >>>> fw_devlink=on by default"), but I think all those fixes are good >>>> changes. So I think we should leave those in. >>>> >>>> Marek, Geert, >>>> >>>> Can you try this series on a static kernel with your OF_POPULATED >>>> changes reverted? I just want to make sure these patches can identify >>>> and fix those cases. >>>> >>>> Tudor, >>>> >>>> You should still make the clock driver fix (because it's a bug), but I >>>> think this series will fix your issue too (even without the clock driver >>>> fix). Can you please give this a shot? >>> Marek, Geert, Tudor, >>> >>> Forgot to say that this will probably fix your issues only in a static >>> kernel. So please try this with a static kernel. If you can also try >>> and confirm that this does not fix the issue for a modular kernel, >>> that'd be good too. >> I've checked those patches on top of linux next-20210129 with >> c09a3e6c97f0 ("soc: samsung: pm_domains: Convert to regular platform >> driver") commit reverted. > Hi Marek, > > Thanks for testing! > >> Sadly it doesn't help. > That sucks. I even partly "tested" it out on my platform (that needs > CONFIG_MODULES) by commenting out the CONFIG_MODULES check. And I saw > some device links getting dropped. Well, my fault. I've missed the fact that I have to disable CONFIG_MODULES to let it work. This is not really a fix for my case, because the exynos_defconfig has modules enabled (mainly for WiFi and media drivers). However disabling the CONFIG_MODULES indeed helped a bit. Most of the devices got finally probed. There are only 4 left in the deferred_devices list: sound 12e2.sysmmu 12d0.hdmi 12c1.mixer The last two (12c1.mixer and 12d0.hdmi) are consumers of the 12e2.sysmmu, which is a consumer of the 10023c20.power-domain. That power domain in turn is a consumer (child) of another power domain (10023c80.power-domain): # dmesg | grep 10023c20.power-domain [ 0.354435] platform 10023c20.power-domain: Linked as a consumer to 10023c80.power-domain [ 0.489573] platform 12d0.hdmi: Linked as a consumer to 10023c20.power-domain [ 0.497143] platform 12c1.mixer: Linked as a consumer to 10023c20.power-domain [ 0.580874] platform 12e2.sysmmu: Linked as a consumer to 10023c20.power-domain [ 0.601655] platform 12e2.sysmmu: probe deferral - supplier 10023c20.power-domain not ready [ 2.744884] platform 12c1.mixer: probe deferral - supplier 10023c20.power-domain not ready [ 2.766726] platform 12d0.hdmi: probe deferral - supplier 10023c20.power-domain not ready ... So a dependency chain of 2 power domains is still not resolved properly. I didn't have time to check what's wrong with the sound node. Simple grepping of the messages for the 'sound' string don't give any results. The above tests has been done on the Odroid U3 board (arch/arm/boot/dts/exynos4412-odroidu3.dts). Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v1 0/2] Make fw_devlink=on more forgiving
Hi Saravana, On 30.01.2021 05:08, Saravana Kannan wrote: > On Fri, Jan 29, 2021 at 8:03 PM Saravana Kannan wrote: >> This patch series solves two general issues with fw_devlink=on >> >> Patch 1/2 addresses the issue of firmware nodes that look like they'll >> have struct devices created for them, but will never actually have >> struct devices added for them. For example, DT nodes with a compatible >> property that don't have devices added for them. >> >> Patch 2/2 address (for static kernels) the issue of optional suppliers >> that'll never have a driver registered for them. So, if the device could >> have probed with fw_devlink=permissive with a static kernel, this patch >> should allow those devices to probe with a fw_devlink=on. This doesn't >> solve it for the case where modules are enabled because there's no way >> to tell if a driver will never be registered or it's just about to be >> registered. I have some other ideas for that, but it'll have to come >> later thinking about it a bit. >> >> These two patches might remove the need for several other patches that >> went in as fixes for commit e590474768f1 ("driver core: Set >> fw_devlink=on by default"), but I think all those fixes are good >> changes. So I think we should leave those in. >> >> Marek, Geert, >> >> Can you try this series on a static kernel with your OF_POPULATED >> changes reverted? I just want to make sure these patches can identify >> and fix those cases. >> >> Tudor, >> >> You should still make the clock driver fix (because it's a bug), but I >> think this series will fix your issue too (even without the clock driver >> fix). Can you please give this a shot? > Marek, Geert, Tudor, > > Forgot to say that this will probably fix your issues only in a static > kernel. So please try this with a static kernel. If you can also try > and confirm that this does not fix the issue for a modular kernel, > that'd be good too. I've checked those patches on top of linux next-20210129 with c09a3e6c97f0 ("soc: samsung: pm_domains: Convert to regular platform driver") commit reverted. Sadly it doesn't help. All devices that belong to the Exynos power domains are never probed and stay endlessly on the deferred devices list. I've used static kernel build - the one from exynos_defconfig. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH RFC 1/2] dt: pci: designware-pcie.txt: convert it to yaml
Hi Mauro, On 26.01.2021 08:35, Mauro Carvalho Chehab wrote: > Convert the file into a JSON description at the yaml format. > > Signed-off-by: Mauro Carvalho Chehab > --- > .../bindings/pci/amlogic,meson-pcie.txt | 4 +- > .../bindings/pci/axis,artpec6-pcie.txt| 2 +- > .../bindings/pci/designware,pcie.yaml | 194 ++ > .../bindings/pci/designware-pcie.txt | 77 --- > .../bindings/pci/fsl,imx6q-pcie.txt | 2 +- > .../bindings/pci/hisilicon-histb-pcie.txt | 2 +- > .../bindings/pci/hisilicon-pcie.txt | 2 +- > .../devicetree/bindings/pci/kirin-pcie.txt| 2 +- > .../bindings/pci/layerscape-pci.txt | 2 +- > .../bindings/pci/nvidia,tegra194-pcie.txt | 4 +- > .../devicetree/bindings/pci/pci-armada8k.txt | 2 +- > .../devicetree/bindings/pci/pci-keystone.txt | 10 +- > .../devicetree/bindings/pci/pcie-al.txt | 2 +- > .../devicetree/bindings/pci/qcom,pcie.txt | 14 +- > .../bindings/pci/samsung,exynos5440-pcie.txt | 4 +- You must have used an old tree for preparing this patchset. The above file is gone in v5.11-rc1 and there is Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml instead. > .../pci/socionext,uniphier-pcie-ep.yaml | 2 +- > .../devicetree/bindings/pci/ti-pci.txt| 4 +- > .../devicetree/bindings/pci/uniphier-pcie.txt | 2 +- > MAINTAINERS | 2 +- > 19 files changed, 225 insertions(+), 108 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/pci/designware,pcie.yaml > delete mode 100644 Documentation/devicetree/bindings/pci/designware-pcie.txt > ... Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] regulator: core: avoid regulator_resolve_supply() race condition
Hi Mark, On 21.01.2021 16:44, Mark Brown wrote: > On Thu, Jan 21, 2021 at 10:41:59AM +0100, Marek Szyprowski wrote: >> On 18.01.2021 21:49, Mark Brown wrote: >>> Does this help (completely untested): >> Sadly nope. I get same warning: > Try this instead: > > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c > index 3ae5ccd9277d..31503776dbd7 100644 > --- a/drivers/regulator/core.c > +++ b/drivers/regulator/core.c > @@ -1823,17 +1823,6 @@ static int regulator_resolve_supply(struct > regulator_dev *rdev) > if (rdev->supply) > return 0; > > - /* > - * Recheck rdev->supply with rdev->mutex lock held to avoid a race > - * between rdev->supply null check and setting rdev->supply in > - * set_supply() from concurrent tasks. > - */ > - regulator_lock(rdev); > - > - /* Supply just resolved by a concurrent task? */ > - if (rdev->supply) > - goto out; > - > r = regulator_dev_lookup(dev, rdev->supply_name); > if (IS_ERR(r)) { > ret = PTR_ERR(r); > @@ -1885,12 +1874,29 @@ static int regulator_resolve_supply(struct > regulator_dev *rdev) > goto out; > } > > + /* > + * Recheck rdev->supply with rdev->mutex lock held to avoid a race > + * between rdev->supply null check and setting rdev->supply in > + * set_supply() from concurrent tasks. > + */ > + regulator_lock(rdev); > + > + /* Supply just resolved by a concurrent task? */ > + if (rdev->supply) { > + regulator_unlock(rdev); > + put_device(>dev); > + return ret; > + } > + > ret = set_supply(rdev, r); > if (ret < 0) { > + regulator_unlock(rdev); > put_device(>dev); > - goto out; > + return ret; > } > > + regulator_unlock(rdev); > + > /* >* In set_machine_constraints() we may have turned this regulator on >* but we couldn't propagate to the supply if it hadn't been resolved > @@ -1901,12 +1907,11 @@ static int regulator_resolve_supply(struct > regulator_dev *rdev) > if (ret < 0) { > _regulator_put(rdev->supply); > rdev->supply = NULL; > - goto out; > + goto out_rdev_lock; drivers/regulator/core.c:1910:4: error: label ‘out_rdev_lock’ used but not defined > } > } > > out: > - regulator_unlock(rdev); > return ret; > } > It looks that it finally fixes the locking issue, with the above goto removed completely to fix build. Feel free to add: Reported-by: Marek Szyprowski Tested-by: Marek Szyprowski Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [TEST PATCH v1] driver: core: Make fw_devlink=on more forgiving
Hi Saravana, On 21.01.2021 09:22, Saravana Kannan wrote: > This patch is for test purposes only and pretty experimental. Code might > not be optimized, clean, formatted properly, etc. > > Please review it only for functional bugs like locking bugs, wrong > logic, etc. > > It's basically trying to figure out which devices will never probe and > ignore them. Might not always work. > > Marek, Geert, Marc, > > Can you please try this patch INSTEAD of the other workarounds we found? I've checked the latest linux-next with this patch and commit c09a3e6c97f0 ("soc: samsung: pm_domains: Convert to regular platform driver") reverted. Sadly it doesn't help. All devices that belongs to the Exynos power domains are not probed at all ("supplier 10023cXX.power-domain not ready"). > Jon, Michael, > > I'm explicitly not including you in the "To" because this patch won't > work for your issues. > > Cc: Marek Szyprowski > Cc: Geert Uytterhoeven > Cc: Marc Zyngier > Signed-off-by: Saravana Kannan > --- > drivers/base/base.h | 3 ++ > drivers/base/core.c | 117 +++- > drivers/base/dd.c | 24 +++++ > 3 files changed, 142 insertions(+), 2 deletions(-) > > > [...] Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] regulator: core: avoid regulator_resolve_supply() race condition
Hi Mark, On 18.01.2021 21:49, Mark Brown wrote: > On Tue, Jan 12, 2021 at 10:34:19PM +0100, Marek Szyprowski wrote: >> == >> WARNING: possible circular locking dependency detected >> 5.11.0-rc1-8-geaa7995c529b #10095 Not tainted >> -- >> swapper/0/1 is trying to acquire lock: >> c12e1b80 (regulator_list_mutex){+.+.}-{3:3}, at: >> regulator_lock_dependent+0x4c/0x2b0 > If you're sending backtraces or other enormous reports like this please > run them through addr2line first so that things are a bit more leigible. Well, I had a little time to process that issue, so I just copy-pasted the kernel log with the hope it will be useful. The trace is really long, but the function call stack is imho readable. If you need more details about any specific trace, just ask. I don't know any good method of processing the raw kernel logs with addr2line and keeping things readable. >> but task is already holding lock: >> df7190c0 (regulator_ww_class_mutex){+.+.}-{3:3}, at: >> regulator_resolve_supply+0x44/0x318 >> >> which lock already depends on the new lock. > Does this help (completely untested): Sadly nope. I get same warning: == WARNING: possible circular locking dependency detected 5.11.0-rc3-next-20210118-5-g56a65ff7ca8b #10162 Not tainted -- swapper/0/1 is trying to acquire lock: c12e1e40 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent+0x4c/0x2b4 but task is already holding lock: df4fe8c0 (regulator_ww_class_mutex){+.+.}-{3:3}, at: regulator_resolve_supply+0x98/0x320 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (regulator_ww_class_mutex){+.+.}-{3:3}: ww_mutex_lock+0x48/0x88 regulator_lock_recursive+0x84/0x1f4 regulator_lock_dependent+0x188/0x2b4 regulator_enable+0x30/0xe4 dwc3_exynos_probe+0x17c/0x2c0 platform_probe+0x80/0xc0 really_probe+0x1d4/0x4ec driver_probe_device+0x78/0x1d8 device_driver_attach+0x58/0x60 __driver_attach+0xfc/0x160 bus_for_each_dev+0x6c/0xb8 bus_add_driver+0x170/0x20c driver_register+0x78/0x10c do_one_initcall+0x88/0x438 kernel_init_freeable+0x190/0x1e0 kernel_init+0x8/0x118 ret_from_fork+0x14/0x38 0x0 -> #1 (regulator_ww_class_acquire){+.+.}-{0:0}: regulator_enable+0x30/0xe4 dwc3_exynos_probe+0x17c/0x2c0 platform_probe+0x80/0xc0 really_probe+0x1d4/0x4ec driver_probe_device+0x78/0x1d8 device_driver_attach+0x58/0x60 __driver_attach+0xfc/0x160 bus_for_each_dev+0x6c/0xb8 bus_add_driver+0x170/0x20c driver_register+0x78/0x10c do_one_initcall+0x88/0x438 kernel_init_freeable+0x190/0x1e0 kernel_init+0x8/0x118 ret_from_fork+0x14/0x38 0x0 -> #0 (regulator_list_mutex){+.+.}-{3:3}: lock_acquire+0x314/0x5d0 __mutex_lock+0xa4/0xb60 mutex_lock_nested+0x1c/0x24 regulator_lock_dependent+0x4c/0x2b4 regulator_enable+0x30/0xe4 regulator_resolve_supply+0x1d0/0x320 regulator_register_resolve_supply+0x14/0x78 class_for_each_device+0x68/0xe8 regulator_register+0xa30/0xca0 devm_regulator_register+0x40/0x70 tps65090_regulator_probe+0x150/0x648 platform_probe+0x80/0xc0 really_probe+0x1d4/0x4ec driver_probe_device+0x78/0x1d8 bus_for_each_drv+0x78/0xbc __device_attach+0xe8/0x180 bus_probe_device+0x88/0x90 device_add+0x4c8/0x7ec platform_device_add+0x120/0x25c mfd_add_devices+0x580/0x60c tps65090_i2c_probe+0xb8/0x184 i2c_device_probe+0x234/0x2a4 really_probe+0x1d4/0x4ec driver_probe_device+0x78/0x1d8 bus_for_each_drv+0x78/0xbc __device_attach+0xe8/0x180 bus_probe_device+0x88/0x90 device_add+0x4c8/0x7ec i2c_new_client_device+0x15c/0x27c of_i2c_register_devices+0x114/0x184 i2c_register_adapter+0x1d8/0x6dc ec_i2c_probe+0xc8/0x124 platform_probe+0x80/0xc0 really_probe+0x1d4/0x4ec driver_probe_device+0x78/0x1d8 bus_for_each_drv+0x78/0xbc __device_attach+0xe8/0x180 bus_probe_device+0x88/0x90 device_add+0x4c8/0x7ec of_platform_device_create_pdata+0x90/0xc8 of_platform_bus_create+0x1a0/0x4ec of_platform_populate+0x88/0x120 devm_of_platform_populate+0x40/0x80 cros_ec_register+0x174/0x308 cros_ec_spi_probe+0x16c/0x1ec spi_probe+0x88/0xac really_probe+0x1d4/0x4ec driver_probe_device+0x78/0x1d8
Re: [PATCH] soc: samsung: pm_domains: Convert to regular platform driver
Hi Saravana, On 13.01.2021 21:27, Saravana Kannan wrote: > On Wed, Jan 13, 2021 at 3:03 AM Marek Szyprowski > wrote: >> When Exynos power domain driver was introduced, the only way to ensure >> that power domains will be instantiated before the devices which belongs >> to them was to initialize them early enough, before the devices are >> instantiated in the system. This in turn required not to use any platform >> device infrastructure at all, as there have been no way to ensure proper >> probe order between devices. >> >> This has been finally changed and patch e590474768f1 ("driver core: Set >> fw_devlink=on by default") ensures that each device will be probbed only >> when its resource providers are ready. This allows to convert Exynos >> power domain driver to regular platform driver. >> >> This is also required by the mentioned commit to enable probing any >> device which belongs to the Exynos power domains, as otherwise the core >> won't notice that the power domains are in fact available. >> >> Signed-off-by: Marek Szyprowski >> --- >> Some more comments are in the following thread: >> https://protect2.fireeye.com/v1/url?k=8ac052ac-d55b6ba4-8ac1d9e3-0cc47a31c8b4-9068b559b0fd155d=1=b393c3ff-16ba-48a4-9d72-6805d02971d5=https%3A%2F%2Flore.kernel.org%2Flkml%2F2556a69b-5da5-bf80-e051-df2d02fbc40f%40samsung.com%2F >> --- >> ... > Skimmed through this patch and at a high level, it looks good for what > it's trying to do. Thanks for doing this! > > Btw, I assume that this won't work with fw_devlink=off/permissive > (default since 5.10 or earlier)? My concern is that we might > temporarily set fw_devlink=permissive by default if the other > breakages aren't fixed in time for 5.12? How do you want to handle that? I've applied my patch on top of vanilla v5.10 and checked on my test boards. Surprisingly everything works fine, so something must have been changed during the last few years as the power domain driver in the current form has been written long time ago. I remember that the moment when platform devices are created from the of nodes has been change at some point, so maybe this is somehow related. Anyway, the platform driver for Exynos power domains registered from core_initcall works fine with v5.10 kernel. I have no strong opinion on the way of merging this fix. It can go via Samsung tree, so in the end the v5.12-rc1 will have both my fix and your change, but won't be fully bisectable in-between. Krzysztof, what's your opinion? Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana, On 13.01.2021 20:23, Saravana Kannan wrote: > On Tue, Jan 12, 2021 at 11:04 PM Marek Szyprowski > wrote: >> On 12.01.2021 21:51, Saravana Kannan wrote: >>> On Mon, Jan 11, 2021 at 11:11 PM Marek Szyprowski >>> wrote: >>>> On 11.01.2021 22:47, Saravana Kannan wrote: >>>>> On Mon, Jan 11, 2021 at 6:18 AM Marek Szyprowski >>>>> wrote: >>>>>> On 11.01.2021 12:12, Marek Szyprowski wrote: >>>>>>> On 18.12.2020 04:17, Saravana Kannan wrote: >>>>>>>> Cyclic dependencies in some firmware was one of the last remaining >>>>>>>> reasons fw_devlink=on couldn't be set by default. Now that cyclic >>>>>>>> dependencies don't block probing, set fw_devlink=on by default. >>>>>>>> >>>>>>>> Setting fw_devlink=on by default brings a bunch of benefits (currently, >>>>>>>> only for systems with device tree firmware): >>>>>>>> * Significantly cuts down deferred probes. >>>>>>>> * Device probe is effectively attempted in graph order. >>>>>>>> * Makes it much easier to load drivers as modules without having to >>>>>>>> worry about functional dependencies between modules (depmod is >>>>>>>> still >>>>>>>> needed for symbol dependencies). >>>>>>>> >>>>>>>> If this patch prevents some devices from probing, it's very likely due >>>>>>>> to the system having one or more device drivers that "probe"/set up a >>>>>>>> device (DT node with compatible property) without creating a struct >>>>>>>> device for it. If we hit such cases, the device drivers need to be >>>>>>>> fixed so that they populate struct devices and probe them like normal >>>>>>>> device drivers so that the driver core is aware of the devices and >>>>>>>> their >>>>>>>> status. See [1] for an example of such a case. >>>>>>>> >>>>>>>> [1] - >>>>>>>> https://protect2.fireeye.com/v1/url?k=68f5d8ba-376ee1f5-68f453f5-0cc47a30d446-324e64700545ab93=1=fb455b9e-c8c7-40d0-8e3c-d9d3713d519b=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAGETcx9PiX%3D%3DmLxB9PO8Myyk6u2vhPVwTMsA5NkD-ywH5xhusw%40mail.gmail.com%2F >>>>>>>> Signed-off-by: Saravana Kannan >>>>>>> This patch landed recently in linux next-20210111 as commit >>>>>>> e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it >>>>>>> breaks Exynos IOMMU operation, what causes lots of devices being >>>>>>> deferred and not probed at all. I've briefly checked and noticed that >>>>>>> exynos_sysmmu_probe() is never called after this patch. This is really >>>>>>> strange for me, as the SYSMMU controllers on Exynos platform are >>>>>>> regular platform devices registered by the OF code. The driver code is >>>>>>> here: drivers/iommu/exynos-iommu.c, example dts: >>>>>>> arch/arm/boot/dts/exynos3250.dtsi (compatible = >>>>>>> "samsung,exynos-sysmmu"). >>>>>> Okay, I found the source of this problem. It is caused by Exynos power >>>>>> domain driver, which is not platform driver yet. I will post a patch, >>>>>> which converts it to the platform driver. >>>>> Thanks Marek! Hopefully the debug logs I added were sufficient to >>>>> figure out the reason. >>>> Frankly, it took me a while to figure out that device core waits for the >>>> power domain devices. Maybe it would be possible to add some more debug >>>> messages or hints? Like the reason of the deferred probe in >>>> /sys/kernel/debug/devices_deferred ? >>> There's already a /sys/devices/...//waiting_for_supplier file >>> that tells you if the device is waiting for a supplier device to be >>> added. That file goes away once the device probes. If the file has 1, >>> then it's waiting for the supplier device to be added (like your >>> case). If it's 0, then the device is just waiting on one of the >>> existing suppliers to probe. You can find the existing suppliers >>> through /sys/devices/...//supplier:*/supplier. Also, flip >>> these dev_dbg() to dev_info() if you need more details about deferred >>&g
[PATCH] soc: samsung: pm_domains: Convert to regular platform driver
When Exynos power domain driver was introduced, the only way to ensure that power domains will be instantiated before the devices which belongs to them was to initialize them early enough, before the devices are instantiated in the system. This in turn required not to use any platform device infrastructure at all, as there have been no way to ensure proper probe order between devices. This has been finally changed and patch e590474768f1 ("driver core: Set fw_devlink=on by default") ensures that each device will be probbed only when its resource providers are ready. This allows to convert Exynos power domain driver to regular platform driver. This is also required by the mentioned commit to enable probing any device which belongs to the Exynos power domains, as otherwise the core won't notice that the power domains are in fact available. Signed-off-by: Marek Szyprowski --- Some more comments are in the following thread: https://lore.kernel.org/lkml/2556a69b-5da5-bf80-e051-df2d02fbc...@samsung.com/ --- drivers/soc/samsung/pm_domains.c | 97 1 file changed, 48 insertions(+), 49 deletions(-) diff --git a/drivers/soc/samsung/pm_domains.c b/drivers/soc/samsung/pm_domains.c index ab8582971bfc..5ec0c13f0aaf 100644 --- a/drivers/soc/samsung/pm_domains.c +++ b/drivers/soc/samsung/pm_domains.c @@ -16,7 +16,7 @@ #include #include #include -#include +#include struct exynos_pm_domain_config { /* Value for LOCAL_PWR_CFG and STATUS fields for each domain */ @@ -73,15 +73,15 @@ static int exynos_pd_power_off(struct generic_pm_domain *domain) return exynos_pd_power(domain, false); } -static const struct exynos_pm_domain_config exynos4210_cfg __initconst = { +static const struct exynos_pm_domain_config exynos4210_cfg = { .local_pwr_cfg = 0x7, }; -static const struct exynos_pm_domain_config exynos5433_cfg __initconst = { +static const struct exynos_pm_domain_config exynos5433_cfg = { .local_pwr_cfg = 0xf, }; -static const struct of_device_id exynos_pm_domain_of_match[] __initconst = { +static const struct of_device_id exynos_pm_domain_of_match[] = { { .compatible = "samsung,exynos4210-pd", .data = _cfg, @@ -92,7 +92,7 @@ static const struct of_device_id exynos_pm_domain_of_match[] __initconst = { { }, }; -static __init const char *exynos_get_domain_name(struct device_node *node) +static const char *exynos_get_domain_name(struct device_node *node) { const char *name; @@ -101,60 +101,44 @@ static __init const char *exynos_get_domain_name(struct device_node *node) return kstrdup_const(name, GFP_KERNEL); } -static __init int exynos4_pm_init_power_domain(void) +static int exynos_pd_probe(struct platform_device *pdev) { - struct device_node *np; - const struct of_device_id *match; - - for_each_matching_node_and_match(np, exynos_pm_domain_of_match, ) { - const struct exynos_pm_domain_config *pm_domain_cfg; - struct exynos_pm_domain *pd; - int on; + const struct exynos_pm_domain_config *pm_domain_cfg; + struct device *dev = >dev; + struct device_node *np = dev->of_node; + struct of_phandle_args child, parent; + struct exynos_pm_domain *pd; + int on, ret; - pm_domain_cfg = match->data; + pm_domain_cfg = of_device_get_match_data(dev); + pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL); + if (!pd) + return -ENOMEM; - pd = kzalloc(sizeof(*pd), GFP_KERNEL); - if (!pd) { - of_node_put(np); - return -ENOMEM; - } - pd->pd.name = exynos_get_domain_name(np); - if (!pd->pd.name) { - kfree(pd); - of_node_put(np); - return -ENOMEM; - } + pd->pd.name = exynos_get_domain_name(np); + if (!pd->pd.name) + return -ENOMEM; - pd->base = of_iomap(np, 0); - if (!pd->base) { - pr_warn("%s: failed to map memory\n", __func__); - kfree_const(pd->pd.name); - kfree(pd); - continue; - } - - pd->pd.power_off = exynos_pd_power_off; - pd->pd.power_on = exynos_pd_power_on; - pd->local_pwr_cfg = pm_domain_cfg->local_pwr_cfg; + pd->base = of_iomap(np, 0); + if (!pd->base) { + kfree_const(pd->pd.name); + return -ENODEV; + } - on = readl_relaxed(pd->base + 0x4) & pd->local_pwr_cfg; + pd->pd.power_off = exynos_pd_power_off; + pd->pd.power_on = exynos_pd_power_on; +
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana, On 12.01.2021 21:51, Saravana Kannan wrote: > On Mon, Jan 11, 2021 at 11:11 PM Marek Szyprowski > wrote: >> On 11.01.2021 22:47, Saravana Kannan wrote: >>> On Mon, Jan 11, 2021 at 6:18 AM Marek Szyprowski >>> wrote: >>>> On 11.01.2021 12:12, Marek Szyprowski wrote: >>>>> On 18.12.2020 04:17, Saravana Kannan wrote: >>>>>> Cyclic dependencies in some firmware was one of the last remaining >>>>>> reasons fw_devlink=on couldn't be set by default. Now that cyclic >>>>>> dependencies don't block probing, set fw_devlink=on by default. >>>>>> >>>>>> Setting fw_devlink=on by default brings a bunch of benefits (currently, >>>>>> only for systems with device tree firmware): >>>>>> * Significantly cuts down deferred probes. >>>>>> * Device probe is effectively attempted in graph order. >>>>>> * Makes it much easier to load drivers as modules without having to >>>>>> worry about functional dependencies between modules (depmod is still >>>>>> needed for symbol dependencies). >>>>>> >>>>>> If this patch prevents some devices from probing, it's very likely due >>>>>> to the system having one or more device drivers that "probe"/set up a >>>>>> device (DT node with compatible property) without creating a struct >>>>>> device for it. If we hit such cases, the device drivers need to be >>>>>> fixed so that they populate struct devices and probe them like normal >>>>>> device drivers so that the driver core is aware of the devices and their >>>>>> status. See [1] for an example of such a case. >>>>>> >>>>>> [1] - >>>>>> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ >>>>>> Signed-off-by: Saravana Kannan >>>>> This patch landed recently in linux next-20210111 as commit >>>>> e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it >>>>> breaks Exynos IOMMU operation, what causes lots of devices being >>>>> deferred and not probed at all. I've briefly checked and noticed that >>>>> exynos_sysmmu_probe() is never called after this patch. This is really >>>>> strange for me, as the SYSMMU controllers on Exynos platform are >>>>> regular platform devices registered by the OF code. The driver code is >>>>> here: drivers/iommu/exynos-iommu.c, example dts: >>>>> arch/arm/boot/dts/exynos3250.dtsi (compatible = "samsung,exynos-sysmmu"). >>>> Okay, I found the source of this problem. It is caused by Exynos power >>>> domain driver, which is not platform driver yet. I will post a patch, >>>> which converts it to the platform driver. >>> Thanks Marek! Hopefully the debug logs I added were sufficient to >>> figure out the reason. >> Frankly, it took me a while to figure out that device core waits for the >> power domain devices. Maybe it would be possible to add some more debug >> messages or hints? Like the reason of the deferred probe in >> /sys/kernel/debug/devices_deferred ? > There's already a /sys/devices/...//waiting_for_supplier file > that tells you if the device is waiting for a supplier device to be > added. That file goes away once the device probes. If the file has 1, > then it's waiting for the supplier device to be added (like your > case). If it's 0, then the device is just waiting on one of the > existing suppliers to probe. You can find the existing suppliers > through /sys/devices/...//supplier:*/supplier. Also, flip > these dev_dbg() to dev_info() if you need more details about deferred > probing. Frankly speaking I doubt that anyone will find those. Even experienced developer might need some time to figure it out. I expect that such information will be at least in the mentioned /sys/kernel/debug/devices_deferred file. We already have infrastructure for putting the deferred probe reason there, see dev_err_probe() function. Even such a simple change makes the debugging this issue much easier: diff --git a/drivers/base/core.c b/drivers/base/core.c index cd8e518fadd6..ceb5aed5a84c 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -937,12 +937,13 @@ int device_links_check_suppliers(struct device *dev) mutex_lock(_link_lock); if (dev->fwnode && !list_empty(>fwnode->suppliers) && !fw_devlink_is_permissive()) { - dev
Re: [PATCH] regulator: core: avoid regulator_resolve_supply() race condition
m [] (cros_ec_register+0x174/0x308) [] (cros_ec_register) from [] (cros_ec_spi_probe+0x16c/0x1ec) [] (cros_ec_spi_probe) from [] (spi_probe+0x88/0xac) [] (spi_probe) from [] (really_probe+0x1c4/0x4e4) [] (really_probe) from [] (driver_probe_device+0x78/0x1d8) [] (driver_probe_device) from [] (device_driver_attach+0x58/0x60) [] (device_driver_attach) from [] (__driver_attach+0xfc/0x160) [] (__driver_attach) from [] (bus_for_each_dev+0x6c/0xb8) [] (bus_for_each_dev) from [] (bus_add_driver+0x170/0x20c) [] (bus_add_driver) from [] (driver_register+0x78/0x10c) [] (driver_register) from [] (do_one_initcall+0x88/0x438) [] (do_one_initcall) from [] (kernel_init_freeable+0x18c/0x1dc) [] (kernel_init_freeable) from [] (kernel_init+0x8/0x118) [] (kernel_init) from [] (ret_from_fork+0x14/0x38) Exception stack(0xc1ce3fb0 to 0xc1ce3ff8) 3fa0: 3fc0: 3fe0: 0013 I didn't analyze it yet if this warning is really an issue or just a false positive. If you have any hints or comments let me know. > --- > drivers/regulator/core.c | 39 --- > 1 file changed, 28 insertions(+), 11 deletions(-) > > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c > index fee9241..3ae5ccd 100644 > --- a/drivers/regulator/core.c > +++ b/drivers/regulator/core.c > @@ -1813,23 +1813,34 @@ static int regulator_resolve_supply(struct > regulator_dev *rdev) > { > struct regulator_dev *r; > struct device *dev = rdev->dev.parent; > - int ret; > + int ret = 0; > > /* No supply to resolve? */ > if (!rdev->supply_name) > return 0; > > - /* Supply already resolved? */ > + /* Supply already resolved? (fast-path without locking contention) */ > if (rdev->supply) > return 0; > > + /* > + * Recheck rdev->supply with rdev->mutex lock held to avoid a race > + * between rdev->supply null check and setting rdev->supply in > + * set_supply() from concurrent tasks. > + */ > + regulator_lock(rdev); > + > + /* Supply just resolved by a concurrent task? */ > + if (rdev->supply) > + goto out; > + > r = regulator_dev_lookup(dev, rdev->supply_name); > if (IS_ERR(r)) { > ret = PTR_ERR(r); > > /* Did the lookup explicitly defer for us? */ > if (ret == -EPROBE_DEFER) > - return ret; > + goto out; > > if (have_full_constraints()) { > r = dummy_regulator_rdev; > @@ -1837,15 +1848,18 @@ static int regulator_resolve_supply(struct > regulator_dev *rdev) > } else { > dev_err(dev, "Failed to resolve %s-supply for %s\n", > rdev->supply_name, rdev->desc->name); > - return -EPROBE_DEFER; > + ret = -EPROBE_DEFER; > + goto out; > } > } > > if (r == rdev) { > dev_err(dev, "Supply for %s (%s) resolved to itself\n", > rdev->desc->name, rdev->supply_name); > - if (!have_full_constraints()) > - return -EINVAL; > + if (!have_full_constraints()) { > + ret = -EINVAL; > + goto out; > + } > r = dummy_regulator_rdev; > get_device(>dev); > } > @@ -1859,7 +1873,8 @@ static int regulator_resolve_supply(struct > regulator_dev *rdev) > if (r->dev.parent && r->dev.parent != rdev->dev.parent) { > if (!device_is_bound(r->dev.parent)) { > put_device(>dev); > - return -EPROBE_DEFER; > + ret = -EPROBE_DEFER; > + goto out; > } > } > > @@ -1867,13 +1882,13 @@ static int regulator_resolve_supply(struct > regulator_dev *rdev) > ret = regulator_resolve_supply(r); > if (ret < 0) { > put_device(>dev); > - return ret; > + goto out; > } > > ret = set_supply(rdev, r); > if (ret < 0) { > put_device(>dev); > - return ret; > + goto out; > } > > /* > @@ -1886,11 +1901,13 @@ static int regulator_resolve_supply(struct > regulator_dev *rdev) > if (ret < 0) { > _regulator_put(rdev->supply); > rdev->supply = NULL; > - return ret; > + goto out; > } > } > > - return 0; > +out: > + regulator_unlock(rdev); > + return ret; > } > > /* Internal regulator request function */ Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On 11.01.2021 22:47, Saravana Kannan wrote: > On Mon, Jan 11, 2021 at 6:18 AM Marek Szyprowski > wrote: >> On 11.01.2021 12:12, Marek Szyprowski wrote: >>> On 18.12.2020 04:17, Saravana Kannan wrote: >>>> Cyclic dependencies in some firmware was one of the last remaining >>>> reasons fw_devlink=on couldn't be set by default. Now that cyclic >>>> dependencies don't block probing, set fw_devlink=on by default. >>>> >>>> Setting fw_devlink=on by default brings a bunch of benefits (currently, >>>> only for systems with device tree firmware): >>>> * Significantly cuts down deferred probes. >>>> * Device probe is effectively attempted in graph order. >>>> * Makes it much easier to load drivers as modules without having to >>>> worry about functional dependencies between modules (depmod is still >>>> needed for symbol dependencies). >>>> >>>> If this patch prevents some devices from probing, it's very likely due >>>> to the system having one or more device drivers that "probe"/set up a >>>> device (DT node with compatible property) without creating a struct >>>> device for it. If we hit such cases, the device drivers need to be >>>> fixed so that they populate struct devices and probe them like normal >>>> device drivers so that the driver core is aware of the devices and their >>>> status. See [1] for an example of such a case. >>>> >>>> [1] - >>>> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ >>>> Signed-off-by: Saravana Kannan >>> This patch landed recently in linux next-20210111 as commit >>> e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it >>> breaks Exynos IOMMU operation, what causes lots of devices being >>> deferred and not probed at all. I've briefly checked and noticed that >>> exynos_sysmmu_probe() is never called after this patch. This is really >>> strange for me, as the SYSMMU controllers on Exynos platform are >>> regular platform devices registered by the OF code. The driver code is >>> here: drivers/iommu/exynos-iommu.c, example dts: >>> arch/arm/boot/dts/exynos3250.dtsi (compatible = "samsung,exynos-sysmmu"). >> Okay, I found the source of this problem. It is caused by Exynos power >> domain driver, which is not platform driver yet. I will post a patch, >> which converts it to the platform driver. > Thanks Marek! Hopefully the debug logs I added were sufficient to > figure out the reason. Frankly, it took me a while to figure out that device core waits for the power domain devices. Maybe it would be possible to add some more debug messages or hints? Like the reason of the deferred probe in /sys/kernel/debug/devices_deferred ? Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On 11.01.2021 12:12, Marek Szyprowski wrote: > On 18.12.2020 04:17, Saravana Kannan wrote: >> Cyclic dependencies in some firmware was one of the last remaining >> reasons fw_devlink=on couldn't be set by default. Now that cyclic >> dependencies don't block probing, set fw_devlink=on by default. >> >> Setting fw_devlink=on by default brings a bunch of benefits (currently, >> only for systems with device tree firmware): >> * Significantly cuts down deferred probes. >> * Device probe is effectively attempted in graph order. >> * Makes it much easier to load drivers as modules without having to >> worry about functional dependencies between modules (depmod is still >> needed for symbol dependencies). >> >> If this patch prevents some devices from probing, it's very likely due >> to the system having one or more device drivers that "probe"/set up a >> device (DT node with compatible property) without creating a struct >> device for it. If we hit such cases, the device drivers need to be >> fixed so that they populate struct devices and probe them like normal >> device drivers so that the driver core is aware of the devices and their >> status. See [1] for an example of such a case. >> >> [1] - >> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ >> Signed-off-by: Saravana Kannan > > This patch landed recently in linux next-20210111 as commit > e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it > breaks Exynos IOMMU operation, what causes lots of devices being > deferred and not probed at all. I've briefly checked and noticed that > exynos_sysmmu_probe() is never called after this patch. This is really > strange for me, as the SYSMMU controllers on Exynos platform are > regular platform devices registered by the OF code. The driver code is > here: drivers/iommu/exynos-iommu.c, example dts: > arch/arm/boot/dts/exynos3250.dtsi (compatible = "samsung,exynos-sysmmu"). Okay, I found the source of this problem. It is caused by Exynos power domain driver, which is not platform driver yet. I will post a patch, which converts it to the platform driver. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana, On 18.12.2020 04:17, Saravana Kannan wrote: > Cyclic dependencies in some firmware was one of the last remaining > reasons fw_devlink=on couldn't be set by default. Now that cyclic > dependencies don't block probing, set fw_devlink=on by default. > > Setting fw_devlink=on by default brings a bunch of benefits (currently, > only for systems with device tree firmware): > * Significantly cuts down deferred probes. > * Device probe is effectively attempted in graph order. > * Makes it much easier to load drivers as modules without having to >worry about functional dependencies between modules (depmod is still >needed for symbol dependencies). > > If this patch prevents some devices from probing, it's very likely due > to the system having one or more device drivers that "probe"/set up a > device (DT node with compatible property) without creating a struct > device for it. If we hit such cases, the device drivers need to be > fixed so that they populate struct devices and probe them like normal > device drivers so that the driver core is aware of the devices and their > status. See [1] for an example of such a case. > > [1] - > https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ > Signed-off-by: Saravana Kannan This patch landed recently in linux next-20210111 as commit e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it breaks Exynos IOMMU operation, what causes lots of devices being deferred and not probed at all. I've briefly checked and noticed that exynos_sysmmu_probe() is never called after this patch. This is really strange for me, as the SYSMMU controllers on Exynos platform are regular platform devices registered by the OF code. The driver code is here: drivers/iommu/exynos-iommu.c, example dts: arch/arm/boot/dts/exynos3250.dtsi (compatible = "samsung,exynos-sysmmu"). > --- > drivers/base/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 4cc030361165..803bfa6eb823 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -1457,7 +1457,7 @@ static void device_links_purge(struct device *dev) > #define FW_DEVLINK_FLAGS_RPM(FW_DEVLINK_FLAGS_ON | \ >DL_FLAG_PM_RUNTIME) > > -static u32 fw_devlink_flags = FW_DEVLINK_FLAGS_PERMISSIVE; > +static u32 fw_devlink_flags = FW_DEVLINK_FLAGS_ON; > static int __init fw_devlink_setup(char *arg) > { > if (!arg) Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v12] ARM: uncompress: Validate start of physical memory against passed DTB
Hi Geert, On 04.01.2021 14:01, Geert Uytterhoeven wrote: > Currently, the start address of physical memory is obtained by masking > the program counter with a fixed mask of 0xf800. This mask value > was chosen as a balance between the requirements of different platforms. > However, this does require that the start address of physical memory is > a multiple of 128 MiB, precluding booting Linux on platforms where this > requirement is not fulfilled. > > Fix this limitation by validating the masked address against the memory > information in the passed DTB. Only use the start address > from DTB when masking would yield an out-of-range address, prefer the > traditional method in all other cases. Note that this applies only to the > explicitly passed DTB on modern systems, and not to a DTB appended to > the kernel, or to ATAGS. The appended DTB may need to be augmented by > information from ATAGS, which may need to rely on knowledge of the start > address of physical memory itself. > > This allows to boot Linux on r7s9210/rza2mevb using the 64 MiB of SDRAM > on the RZA2MEVB sub board, which is located at 0x0C00 (CS3 space), > i.e. not at a multiple of 128 MiB. > > Suggested-by: Nicolas Pitre > Suggested-by: Ard Biesheuvel > Signed-off-by: Geert Uytterhoeven > Reviewed-by: Ard Biesheuvel > Acked-by: Nicolas Pitre I've checked all of my arm 32bit test systems and they still boot fine with this patch. Feel free to add my: Tested-by: Marek Szyprowski although I didn't test exactly the new features added by it. > --- > Submitted to RMK's patch tracker. > > v12: >- Drop unneeded initialization of r in get_val(), > > v11: >- Add Reviewed-by, Acked-by, >- Document GOT fixup caveat, > > v10: >- Update for commit 9443076e4330a14a ("ARM: p2v: reduce p2v alignment > requirement to 2 MiB"), >- Use OF_DT_MAGIC macro, >- Rename fdt_get_mem_start() to fdt_check_mem_start(), >- Skip validation if there is an appended DTB, >- Pass mem_start to fdt_check_mem_start() to streamline code, >- Optimize register allocation, >- Update CONFIG_AUTO_ZRELADDR help text, >- Check all memory nodes and ranges (not just the first one), and > "linux,usable-memory", similar to early_init_dt_scan_memory(), >- Drop Reviewed-by, Tested-by tags, > > v9: >- Add minlen parameter to getprop(), for better validation of > memory properties, >- Only use start of memory from the DTB if masking would yield an > out-of-range address, to fix kdump, as suggested by Ard. > > v8: >- Rebase on top of commit 893ab00439a45513 ("kbuild: remove cc-option > test of -fno-stack-protector"), > > v7: >- Rebase on top of commit 161e04a5bae58a65 ("ARM: decompressor: split > off _edata and stack base into separate object"), > > v6: >- Rebase on top of commit 7ae4a78daacf240a ("ARM: 8969/1: > decompressor: simplify libfdt builds"), >- Include instead of , > > v5: >- Add Tested-by, Reviewed-by, >- Round up start of memory to satisfy 16 MiB alignment rule, > > v4: >- Fix stack location after commit 184bf653a7a452c1 ("ARM: > decompressor: factor out routine to obtain the inflated image > size"), > > v3: >- Add Reviewed-by, >- Fix ATAGs with appended DTB, >- Add Tested-by, > > v2: >- Use "cmp r0, #-1", instead of "cmn r0, #1", >- Add missing stack setup, >- Support appended DTB. > --- > arch/arm/Kconfig | 7 +- > arch/arm/boot/compressed/Makefile | 5 +- > .../arm/boot/compressed/fdt_check_mem_start.c | 131 ++ > arch/arm/boot/compressed/head.S | 36 - > 4 files changed, 172 insertions(+), 7 deletions(-) > create mode 100644 arch/arm/boot/compressed/fdt_check_mem_start.c > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 138248999df7421e..9860057775ee72a9 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -1875,9 +1875,10 @@ config AUTO_ZRELADDR > help > ZRELADDR is the physical address where the decompressed kernel > image will be placed. If AUTO_ZRELADDR is selected, the address > - will be determined at run-time by masking the current IP with > - 0xf800. This assumes the zImage being placed in the first 128MB > - from start of memory. > + will be determined at run-time, either by masking the current IP > + with 0xf800, or, if invalid, from the DTB passed in r2. > + This assumes the zImage being placed in the first 128MB from > +
Re: [PATCH 9/9] mfd: sec-irq: Do not enforce (incorrect) interrupt trigger type
On 21.12.2020 08:55, Krzysztof Kozlowski wrote: > On Mon, Dec 21, 2020 at 08:36:02AM +0100, Marek Szyprowski wrote: >> On 18.12.2020 15:22, Krzysztof Kozlowski wrote: >>> On Fri, Dec 18, 2020 at 02:25:39PM +0100, Marek Szyprowski wrote: >>>> On 10.12.2020 22:29, Krzysztof Kozlowski wrote: >>>>> Interrupt line can be configured on different hardware in different way, >>>>> even inverted. Therefore driver should not enforce specific trigger >>>>> type - edge falling - but instead rely on Devicetree to configure it. >>>>> >>>>> The Samsung PMIC drivers are used only on Devicetree boards. >>>>> >>>>> Additionally, the PMIC datasheets describe the interrupt line as active >>>>> low with a requirement of acknowledge from the CPU therefore the edge >>>>> falling is not correct. >>>>> >>>>> Signed-off-by: Krzysztof Kozlowski >>>> Tested-by: Marek Szyprowski >>>> >>>> It looks that this together with DTS change fixes RTC alarm failure that >>>> I've observed from time to time on TM2e board! >>> Great! I'll add this to the commit msg. >>> >>> Thanks for testing. >> BTW, while playing with this, maybe it would make sense to fix the >> reported interrupt type for the PMIC sub-interrupts: >> >> # grep s2mps /proc/interrupts >> 120: 0 gpa0 7 Level s2mps13 >> 121: 0 s2mps13 10 Edge rtc-alarm0 > I also spotted this. It's a virtual interrupt and I am not sure whether > we can actually configure it when the hardware does not allow to set the > type (the regmap_irq_type requires register offsets). I know that it is virtual, but maybe the regmap code could simply copy the interrupt type from its parent interrupt? Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 9/9] mfd: sec-irq: Do not enforce (incorrect) interrupt trigger type
Hi Krzysztof, On 18.12.2020 15:22, Krzysztof Kozlowski wrote: > On Fri, Dec 18, 2020 at 02:25:39PM +0100, Marek Szyprowski wrote: >> On 10.12.2020 22:29, Krzysztof Kozlowski wrote: >>> Interrupt line can be configured on different hardware in different way, >>> even inverted. Therefore driver should not enforce specific trigger >>> type - edge falling - but instead rely on Devicetree to configure it. >>> >>> The Samsung PMIC drivers are used only on Devicetree boards. >>> >>> Additionally, the PMIC datasheets describe the interrupt line as active >>> low with a requirement of acknowledge from the CPU therefore the edge >>> falling is not correct. >>> >>> Signed-off-by: Krzysztof Kozlowski >> Tested-by: Marek Szyprowski >> >> It looks that this together with DTS change fixes RTC alarm failure that >> I've observed from time to time on TM2e board! > Great! I'll add this to the commit msg. > > Thanks for testing. BTW, while playing with this, maybe it would make sense to fix the reported interrupt type for the PMIC sub-interrupts: # grep s2mps /proc/interrupts 120: 0 gpa0 7 Level s2mps13 121: 0 s2mps13 10 Edge rtc-alarm0 Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] gcc-plugins: simplify GCC plugin-dev capability test
On 18.12.2020 16:03, Jon Hunter wrote: > On 18/12/2020 10:05, Marek Szyprowski wrote: >> On 18.12.2020 10:43, Masahiro Yamada wrote: >>> On Fri, Dec 18, 2020 at 4:58 PM Marek Szyprowski >>> wrote: >>>> On 03.12.2020 13:57, Masahiro Yamada wrote: >>>>> Linus pointed out a third of the time in the Kconfig parse stage comes >>>>> from the single invocation of cc1plus in scripts/gcc-plugin.sh [1], >>>>> and directly testing plugin-version.h for existence cuts down the >>>>> overhead a lot. [2] >>>>> >>>>> This commit takes one step further to kill the build test entirely. >>>>> >>>>> The small piece of code was probably intended to test the C++ designated >>>>> initializer, which was not supported until C++20. >>>>> >>>>> In fact, with -pedantic option given, both GCC and Clang emit a warning. >>>>> >>>>> $ echo 'class test { public: int test; } test = { .test = 1 };' | g++ -x >>>>> c++ -pedantic - -fsyntax-only >>>>> :1:43: warning: C++ designated initializers only available with >>>>> '-std=c++2a' or '-std=gnu++2a' [-Wpedantic] >>>>> $ echo 'class test { public: int test; } test = { .test = 1 };' | clang++ >>>>> -x c++ -pedantic - -fsyntax-only >>>>> :1:43: warning: designated initializers are a C++20 extension >>>>> [-Wc++20-designator] >>>>> class test { public: int test; } test = { .test = 1 }; >>>>> ^ >>>>> 1 warning generated. >>>>> >>>>> Otherwise, modern C++ compilers should be able to build the code, and >>>>> hopefully skipping this test should not make any practical problem. >>>>> >>>>> Checking the existence of plugin-version.h is still needed to ensure >>>>> the plugin-dev package is installed. The test code is now small enough >>>>> to be embedded in scripts/gcc-plugins/Kconfig. >>>>> >>>>> [1] >>>>> https://protect2.fireeye.com/v1/url?k=03db90e1-5c40a828-03da1bae-0cc47a336fae-4cc36f5830aeb78d=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAHk-%3DwjU4DCuwQ4pXshRbwDCUQB31ScaeuDo1tjoZ0_PjhLHzQ%40mail.gmail.com%2F >>>>> [2] >>>>> https://protect2.fireeye.com/v1/url?k=965b670a-c9c05fc3-965aec45-0cc47a336fae-e34339513ff747c0=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAHk-%3DwhK0aQxs6Q5ijJmYF1n2ch8cVFSUzU5yUM_HOjig%3D%2Bvnw%40mail.gmail.com%2F >>>>> >>>>> Reported-by: Linus Torvalds >>>>> Signed-off-by: Masahiro Yamada >>>> This patch landed in linux next-20201217 as commit 1e860048c53e >>>> ("gcc-plugins: simplify GCC plugin-dev capability test"). >>>> >>>> It causes a build break with my tests setup, but I'm not sure weather it >>>> is really an issue of this commit or a toolchain I use. However I've >>>> checked various versions of the gcc cross-compilers released by Linaro >>>> at >>>> https://protect2.fireeye.com/v1/url?k=053727b6-5aac1f7f-0536acf9-0cc47a336fae-5bd799e7ce6b1b9b=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Freleases.linaro.org%2Fcomponents%2Ftoolchain%2Fbinaries%2F >>>> and all >>>> fails with the same error: >>>> >>>> $ make ARCH=arm >>>> CROSS_COMPILE=../../cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/arm-none-eabi- >>>> zImage >>>> HOSTCXX scripts/gcc-plugins/arm_ssp_per_task_plugin.so >>>> In file included from >>>> /home/mszyprow/dev/cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.1/plugin/include/gcc-plugin.h:28:0, >>>> from scripts/gcc-plugins/gcc-common.h:7, >>>> from scripts/gcc-plugins/arm_ssp_per_task_plugin.c:3: >>>> /home/mszyprow/dev/cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.1/plugin/include/system.h:687:10: >>>> fatal error: gmp.h: No such file or directory >>>> #include >>>> ^~~ >>>> compilation terminated. >>>> scripts/gcc-plugins/Makefile:47: recipe for target >>>> 'scripts/gcc-plugins/arm_ssp_per_task_plugin.so' failed >>>> make[2]: *** [scripts/gcc-plugins/arm_ssp_per_task_plugin.so] Error 1 >>>> scripts/Makefile.build:496: recipe for target
Re: [PATCH 9/9] mfd: sec-irq: Do not enforce (incorrect) interrupt trigger type
On 10.12.2020 22:29, Krzysztof Kozlowski wrote: > Interrupt line can be configured on different hardware in different way, > even inverted. Therefore driver should not enforce specific trigger > type - edge falling - but instead rely on Devicetree to configure it. > > The Samsung PMIC drivers are used only on Devicetree boards. > > Additionally, the PMIC datasheets describe the interrupt line as active > low with a requirement of acknowledge from the CPU therefore the edge > falling is not correct. > > Signed-off-by: Krzysztof Kozlowski Tested-by: Marek Szyprowski It looks that this together with DTS change fixes RTC alarm failure that I've observed from time to time on TM2e board! > --- > > This patch should wait till DTS changes are merged, as it relies on > proper Devicetree. > --- > drivers/mfd/sec-irq.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c > index a98c5d165039..760f88a865ab 100644 > --- a/drivers/mfd/sec-irq.c > +++ b/drivers/mfd/sec-irq.c > @@ -480,8 +480,7 @@ int sec_irq_init(struct sec_pmic_dev *sec_pmic) > } > > ret = devm_regmap_add_irq_chip(sec_pmic->dev, sec_pmic->regmap_pmic, > -sec_pmic->irq, > -IRQF_TRIGGER_FALLING | IRQF_ONESHOT, > +sec_pmic->irq, IRQF_ONESHOT, > sec_pmic->irq_base, sec_irq_chip, > _pmic->irq_data); > if (ret != 0) { Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 7/9] arm64: dts: exynos: correct PMIC interrupt trigger level on TM2
On 10.12.2020 22:29, Krzysztof Kozlowski wrote: > The Samsung PMIC datasheets describe the interrupt line as active low > with a requirement of acknowledge from the CPU. Without specifying the > interrupt type in Devicetree, kernel might apply some fixed > configuration, not necessarily working for this hardware. > > Fixes: 01e5d2352152 ("arm64: dts: exynos: Add dts file for Exynos5433-based > TM2 board") > Signed-off-by: Krzysztof Kozlowski Tested-by: Marek Szyprowski > --- > arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi > b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi > index 03486a8ffc67..4c5106a0860d 100644 > --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi > +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi > @@ -388,7 +388,7 @@ _0 { > pmic@66 { > compatible = "samsung,s2mps13-pmic"; > interrupt-parent = <>; > - interrupts = <7 IRQ_TYPE_NONE>; > + interrupts = <7 IRQ_TYPE_LEVEL_LOW>; > reg = <0x66>; > samsung,s2mps11-wrstbi-ground; > Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] gcc-plugins: simplify GCC plugin-dev capability test
On 18.12.2020 10:43, Masahiro Yamada wrote: > On Fri, Dec 18, 2020 at 4:58 PM Marek Szyprowski > wrote: >> On 03.12.2020 13:57, Masahiro Yamada wrote: >>> Linus pointed out a third of the time in the Kconfig parse stage comes >>> from the single invocation of cc1plus in scripts/gcc-plugin.sh [1], >>> and directly testing plugin-version.h for existence cuts down the >>> overhead a lot. [2] >>> >>> This commit takes one step further to kill the build test entirely. >>> >>> The small piece of code was probably intended to test the C++ designated >>> initializer, which was not supported until C++20. >>> >>> In fact, with -pedantic option given, both GCC and Clang emit a warning. >>> >>> $ echo 'class test { public: int test; } test = { .test = 1 };' | g++ -x >>> c++ -pedantic - -fsyntax-only >>> :1:43: warning: C++ designated initializers only available with >>> '-std=c++2a' or '-std=gnu++2a' [-Wpedantic] >>> $ echo 'class test { public: int test; } test = { .test = 1 };' | clang++ >>> -x c++ -pedantic - -fsyntax-only >>> :1:43: warning: designated initializers are a C++20 extension >>> [-Wc++20-designator] >>> class test { public: int test; } test = { .test = 1 }; >>> ^ >>> 1 warning generated. >>> >>> Otherwise, modern C++ compilers should be able to build the code, and >>> hopefully skipping this test should not make any practical problem. >>> >>> Checking the existence of plugin-version.h is still needed to ensure >>> the plugin-dev package is installed. The test code is now small enough >>> to be embedded in scripts/gcc-plugins/Kconfig. >>> >>> [1] >>> https://protect2.fireeye.com/v1/url?k=03db90e1-5c40a828-03da1bae-0cc47a336fae-4cc36f5830aeb78d=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAHk-%3DwjU4DCuwQ4pXshRbwDCUQB31ScaeuDo1tjoZ0_PjhLHzQ%40mail.gmail.com%2F >>> [2] >>> https://protect2.fireeye.com/v1/url?k=965b670a-c9c05fc3-965aec45-0cc47a336fae-e34339513ff747c0=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAHk-%3DwhK0aQxs6Q5ijJmYF1n2ch8cVFSUzU5yUM_HOjig%3D%2Bvnw%40mail.gmail.com%2F >>> >>> Reported-by: Linus Torvalds >>> Signed-off-by: Masahiro Yamada >> This patch landed in linux next-20201217 as commit 1e860048c53e >> ("gcc-plugins: simplify GCC plugin-dev capability test"). >> >> It causes a build break with my tests setup, but I'm not sure weather it >> is really an issue of this commit or a toolchain I use. However I've >> checked various versions of the gcc cross-compilers released by Linaro >> at >> https://protect2.fireeye.com/v1/url?k=053727b6-5aac1f7f-0536acf9-0cc47a336fae-5bd799e7ce6b1b9b=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Freleases.linaro.org%2Fcomponents%2Ftoolchain%2Fbinaries%2F >> and all >> fails with the same error: >> >> $ make ARCH=arm >> CROSS_COMPILE=../../cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/arm-none-eabi- >> zImage >> HOSTCXX scripts/gcc-plugins/arm_ssp_per_task_plugin.so >> In file included from >> /home/mszyprow/dev/cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.1/plugin/include/gcc-plugin.h:28:0, >>from scripts/gcc-plugins/gcc-common.h:7, >>from scripts/gcc-plugins/arm_ssp_per_task_plugin.c:3: >> /home/mszyprow/dev/cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.1/plugin/include/system.h:687:10: >> fatal error: gmp.h: No such file or directory >>#include >> ^~~ >> compilation terminated. >> scripts/gcc-plugins/Makefile:47: recipe for target >> 'scripts/gcc-plugins/arm_ssp_per_task_plugin.so' failed >> make[2]: *** [scripts/gcc-plugins/arm_ssp_per_task_plugin.so] Error 1 >> scripts/Makefile.build:496: recipe for target 'scripts/gcc-plugins' failed >> make[1]: *** [scripts/gcc-plugins] Error 2 >> Makefile:1190: recipe for target 'scripts' failed >> make: *** [scripts] Error 2 >> >> Compilation works if I use the cross-gcc provided by >> gcc-7-arm-linux-gnueabi/gcc-arm-linux-gnueabi Ubuntu packages, which is: >> >> $ arm-linux-gnueabi-gcc --version >> arm-linux-gnueabi-gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0 >> > > I can compile gcc-plugins with Linaro toolchians. > > The version of mine is this: > > masahiro@oscar:~/ref/linux-next$ > ~/tools/arm-linaro-7.5/bin/arm-linux-gnueabihf-gcc --version > arm-linux-gnueabihf-gcc (Linaro GCC 7.5-2019.12) 7.5.0 > Copyright (C) 2017 Free Software Foundation, Inc. > This is free software; see the source for copying conditions. There is NO > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > > > > > Maybe, it depends on the host environment? > > > Please try this: > > $ sudo apt install libgmp-dev Indeed, it was missing on my setup. Sorry for the noise. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] gcc-plugins: simplify GCC plugin-dev capability test
Hi, On 03.12.2020 13:57, Masahiro Yamada wrote: > Linus pointed out a third of the time in the Kconfig parse stage comes > from the single invocation of cc1plus in scripts/gcc-plugin.sh [1], > and directly testing plugin-version.h for existence cuts down the > overhead a lot. [2] > > This commit takes one step further to kill the build test entirely. > > The small piece of code was probably intended to test the C++ designated > initializer, which was not supported until C++20. > > In fact, with -pedantic option given, both GCC and Clang emit a warning. > > $ echo 'class test { public: int test; } test = { .test = 1 };' | g++ -x c++ > -pedantic - -fsyntax-only > :1:43: warning: C++ designated initializers only available with > '-std=c++2a' or '-std=gnu++2a' [-Wpedantic] > $ echo 'class test { public: int test; } test = { .test = 1 };' | clang++ -x > c++ -pedantic - -fsyntax-only > :1:43: warning: designated initializers are a C++20 extension > [-Wc++20-designator] > class test { public: int test; } test = { .test = 1 }; >^ > 1 warning generated. > > Otherwise, modern C++ compilers should be able to build the code, and > hopefully skipping this test should not make any practical problem. > > Checking the existence of plugin-version.h is still needed to ensure > the plugin-dev package is installed. The test code is now small enough > to be embedded in scripts/gcc-plugins/Kconfig. > > [1] > https://lore.kernel.org/lkml/CAHk-=wju4dcuwq4pxshrbwdcuqb31scaeudo1tjoz0_pjhl...@mail.gmail.com/ > [2] > https://lore.kernel.org/lkml/CAHk-=whK0aQxs6Q5ijJmYF1n2ch8cVFSUzU5yUM_HOjig=+v...@mail.gmail.com/ > > Reported-by: Linus Torvalds > Signed-off-by: Masahiro Yamada This patch landed in linux next-20201217 as commit 1e860048c53e ("gcc-plugins: simplify GCC plugin-dev capability test"). It causes a build break with my tests setup, but I'm not sure weather it is really an issue of this commit or a toolchain I use. However I've checked various versions of the gcc cross-compilers released by Linaro at https://releases.linaro.org/components/toolchain/binaries/ and all fails with the same error: $ make ARCH=arm CROSS_COMPILE=../../cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/arm-none-eabi- zImage HOSTCXX scripts/gcc-plugins/arm_ssp_per_task_plugin.so In file included from /home/mszyprow/dev/cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.1/plugin/include/gcc-plugin.h:28:0, from scripts/gcc-plugins/gcc-common.h:7, from scripts/gcc-plugins/arm_ssp_per_task_plugin.c:3: /home/mszyprow/dev/cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.1/plugin/include/system.h:687:10: fatal error: gmp.h: No such file or directory #include ^~~ compilation terminated. scripts/gcc-plugins/Makefile:47: recipe for target 'scripts/gcc-plugins/arm_ssp_per_task_plugin.so' failed make[2]: *** [scripts/gcc-plugins/arm_ssp_per_task_plugin.so] Error 1 scripts/Makefile.build:496: recipe for target 'scripts/gcc-plugins' failed make[1]: *** [scripts/gcc-plugins] Error 2 Makefile:1190: recipe for target 'scripts' failed make: *** [scripts] Error 2 Compilation works if I use the cross-gcc provided by gcc-7-arm-linux-gnueabi/gcc-arm-linux-gnueabi Ubuntu packages, which is: $ arm-linux-gnueabi-gcc --version arm-linux-gnueabi-gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0 Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 3/3] driver core: platform: use bus_type functions
e); > + } > + > +out: > + if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) { > + dev_warn(_dev, "probe deferral not supported\n"); > + ret = -ENXIO; > + } > + > + return ret; > +} > + > +static int platform_remove(struct device *_dev) > +{ > + struct platform_driver *drv = to_platform_driver(_dev->driver); > + struct platform_device *dev = to_platform_device(_dev); > + int ret = 0; > + > + if (drv->remove) > + ret = drv->remove(dev); > + dev_pm_domain_detach(_dev, true); > + > + return ret; > +} > + > +static void platform_shutdown(struct device *_dev) > +{ > + struct platform_driver *drv = to_platform_driver(_dev->driver); > + struct platform_device *dev = to_platform_device(_dev); > + > + if (drv->shutdown) > + drv->shutdown(dev); > +} This will be called on unbound devices, what causes crash (observed on today's linux-next): Will now restart. 8<--- cut here --- Unable to handle kernel paging request at virtual address fff4 pgd = 289f4b67 [fff4] *pgd=6841, *pte=, *ppte= Internal error: Oops: 27 [#1] SMP ARM Modules linked in: usb_f_ecm g_ether usb_f_rndis u_ether libcomposite brcmfmac sha256_generic libsha256 sha256_arm cfg80211 brcmutil panel_samsung_s6e8aa0 s5p_csi CPU: 0 PID: 1715 Comm: reboot Not tainted 5.10.0-rc7-next-20201211-00069-g1e8aa883315f #9935 Hardware name: Samsung Exynos (Flattened Device Tree) PC is at platform_shutdown+0x8/0x18 LR is at device_shutdown+0x18c/0x25c ... Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 4348404a DAC: 0051 Process reboot (pid: 1715, stack limit = 0xd050391e) Stack: (0xc3405e60 to 0xc3406000) [] (platform_shutdown) from [] (device_shutdown+0x18c/0x25c) [] (device_shutdown) from [] (kernel_restart+0xc/0x68) [] (kernel_restart) from [] (__do_sys_reboot+0x154/0x1f0) [] (__do_sys_reboot) from [] (ret_fast_syscall+0x0/0x58) Exception stack(0xc3405fa8 to 0xc3405ff0) ... ---[ end trace f39e94d5d6fd45bf ]--- > + > + > int platform_dma_configure(struct device *dev) > { > enum dev_dma_attr attr; > @@ -1375,6 +1370,9 @@ struct bus_type platform_bus_type = { > .dev_groups = platform_dev_groups, > .match = platform_match, > .uevent = platform_uevent, > + .probe = platform_probe, > + .remove = platform_remove, > + .shutdown = platform_shutdown, > .dma_configure = platform_dma_configure, > .pm = _dev_pm_ops, > }; Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 5/9] ARM: dts: exynos: correct PMIC interrupt trigger level on Arndale Octa
On 10.12.2020 22:28, Krzysztof Kozlowski wrote: > The Samsung PMIC datasheets describe the interrupt line as active low > with a requirement of acknowledge from the CPU. The falling edge > interrupt will mostly work but it's not correct. > > Fixes: 1fed2252713e ("ARM: dts: fix pinctrl for s2mps11-irq on > exynos5420-arndale-octa") > Signed-off-by: Krzysztof Kozlowski > --- > arch/arm/boot/dts/exynos5420-arndale-octa.dts | 2 +- Tested-by: Marek Szyprowski > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/exynos5420-arndale-octa.dts > b/arch/arm/boot/dts/exynos5420-arndale-octa.dts > index bf457d0c02eb..1aad4859c5f1 100644 > --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts > +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts > @@ -349,7 +349,7 @@ pmic@66 { > reg = <0x66>; > > interrupt-parent = <>; > - interrupts = <2 IRQ_TYPE_EDGE_FALLING>; > + interrupts = <2 IRQ_TYPE_LEVEL_LOW>; > pinctrl-names = "default"; > pinctrl-0 = <_irq>; > Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 3/9] ARM: dts: exynos: correct PMIC interrupt trigger level on Rinato
On 10.12.2020 22:28, Krzysztof Kozlowski wrote: > The Samsung PMIC datasheets describe the interrupt line as active low > with a requirement of acknowledge from the CPU. Without specifying the > interrupt type in Devicetree, kernel might apply some fixed > configuration, not necessarily working for this hardware. > > Fixes: faaf348ef468 ("ARM: dts: Add board dts file for exynos3250-rinato") > Signed-off-by: Krzysztof Kozlowski > --- > arch/arm/boot/dts/exynos3250-rinato.dts | 2 +- Tested-by: Marek Szyprowski > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts > b/arch/arm/boot/dts/exynos3250-rinato.dts > index a26e3e582a7e..d64ccf4b7d32 100644 > --- a/arch/arm/boot/dts/exynos3250-rinato.dts > +++ b/arch/arm/boot/dts/exynos3250-rinato.dts > @@ -270,7 +270,7 @@ _0 { > pmic@66 { > compatible = "samsung,s2mps14-pmic"; > interrupt-parent = <>; > - interrupts = <7 IRQ_TYPE_NONE>; > + interrupts = <7 IRQ_TYPE_LEVEL_LOW>; > reg = <0x66>; > wakeup-source; > Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 6/9] ARM: dts: exynos: correct PMIC interrupt trigger level on Odroid XU3 family
On 10.12.2020 22:29, Krzysztof Kozlowski wrote: > The Samsung PMIC datasheets describe the interrupt line as active low > with a requirement of acknowledge from the CPU. The falling edge > interrupt will mostly work but it's not correct. > > Fixes: aac4e0615341 ("ARM: dts: odroidxu3: Enable wake alarm of S2MPS11 RTC") > Signed-off-by: Krzysztof Kozlowski > --- > arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 2 +- Tested-by: Marek Szyprowski > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi > b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi > index d0df560eb0db..6d690b1db099 100644 > --- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi > +++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi > @@ -509,7 +509,7 @@ pmic@66 { > samsung,s2mps11-acokb-ground; > > interrupt-parent = <>; > - interrupts = <4 IRQ_TYPE_EDGE_FALLING>; > + interrupts = <4 IRQ_TYPE_LEVEL_LOW>; > pinctrl-names = "default"; > pinctrl-0 = <_irq>; > Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 1/9] ARM: dts: exynos: correct PMIC interrupt trigger level on Artik 5
On 10.12.2020 22:28, Krzysztof Kozlowski wrote: > The Samsung PMIC datasheets describe the interrupt line as active low > with a requirement of acknowledge from the CPU. Without specifying the > interrupt type in Devicetree, kernel might apply some fixed > configuration, not necessarily working for this hardware. > > Fixes: b004a34bd0ff ("ARM: dts: exynos: Add exynos3250-artik5 dtsi file for > ARTIK5 module") > Signed-off-by: Krzysztof Kozlowski > --- > arch/arm/boot/dts/exynos3250-artik5.dtsi | 2 +- Tested-by: Marek Szyprowski > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/exynos3250-artik5.dtsi > b/arch/arm/boot/dts/exynos3250-artik5.dtsi > index 04290ec4583a..829c05b2c405 100644 > --- a/arch/arm/boot/dts/exynos3250-artik5.dtsi > +++ b/arch/arm/boot/dts/exynos3250-artik5.dtsi > @@ -79,7 +79,7 @@ _0 { > pmic@66 { > compatible = "samsung,s2mps14-pmic"; > interrupt-parent = <>; > - interrupts = <5 IRQ_TYPE_NONE>; > + interrupts = <5 IRQ_TYPE_LEVEL_LOW>; > pinctrl-names = "default"; > pinctrl-0 = <_irq>; > reg = <0x66>; Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] Input: cyapa - do not call input_device_enabled from power mode handler
On 11.12.2020 08:09, Dmitry Torokhov wrote: > Input device's user counter is supposed to be accessed only while holding > input->mutex. Commit d69f0a43c677 ("Input: use input_device_enabled()") > recently switched cyapa to using the dedicated API and it uncovered the > fact that cyapa driver violated this constraint. > > This patch removes checks whether the input device is open when clearing > device queues when changing device's power mode as there is no harm in > sending input events through closed input device - the events will simply > be dropped by the input core. > > Note that there are more places in cyapa driver that call > input_device_enabled() without holding input->mutex, those are left > unfixed for now. > > Reported-by: Marek Szyprowski > Signed-off-by: Dmitry Torokhov > --- > > Marek, could you please try this one? The warning is still there: [ cut here ] WARNING: CPU: 1 PID: 1787 at drivers/input/input.c:2230 input_device_enabled+0x68/0x6c Modules linked in: cmac bnep mwifiex_sdio mwifiex sha256_generic libsha256 sha256_arm btmrvl_sdio btmrvl cfg80211 bluetooth s5p_mfc exynos_gsc v4l2_mem2mem videob CPU: 1 PID: 1787 Comm: rtcwake Not tainted 5.10.0-rc7-next-20201210-1-g70a81f43fddf #2204 Hardware name: Samsung Exynos (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0xb4/0xd4) [] (dump_stack) from [] (__warn+0xd8/0x11c) [] (__warn) from [] (warn_slowpath_fmt+0xb0/0xb8) [] (warn_slowpath_fmt) from [] (input_device_enabled+0x68/0x6c) [] (input_device_enabled) from [] (cyapa_reinitialize+0x4c/0x154) [] (cyapa_reinitialize) from [] (cyapa_resume+0x48/0x98) [] (cyapa_resume) from [] (dpm_run_callback+0xb0/0x3c8) [] (dpm_run_callback) from [] (device_resume+0xbc/0x260) [] (device_resume) from [] (dpm_resume+0x14c/0x51c) [] (dpm_resume) from [] (dpm_resume_end+0xc/0x18) [] (dpm_resume_end) from [] (suspend_devices_and_enter+0x1b4/0xbd4) [] (suspend_devices_and_enter) from [] (pm_suspend+0x314/0x42c) [] (pm_suspend) from [] (state_store+0x6c/0xc8) [] (state_store) from [] (kernfs_fop_write+0x10c/0x228) [] (kernfs_fop_write) from [] (vfs_write+0xc8/0x530) [] (vfs_write) from [] (ksys_write+0x60/0xd8) [] (ksys_write) from [] (ret_fast_syscall+0x0/0x2c) Exception stack(0xc3923fa8 to 0xc3923ff0) irq event stamp: 54139 hardirqs last enabled at (54147): [] vprintk_emit+0x2b8/0x308 hardirqs last disabled at (54154): [] vprintk_emit+0x27c/0x308 softirqs last enabled at (50722): [] __do_softirq+0x528/0x684 softirqs last disabled at (50671): [] irq_exit+0x1ec/0x1f8 ---[ end trace 1fbefe3f239ae597 ]--- > drivers/input/mouse/cyapa_gen3.c |5 + > drivers/input/mouse/cyapa_gen5.c |3 +-- > 2 files changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/input/mouse/cyapa_gen3.c > b/drivers/input/mouse/cyapa_gen3.c > index a97f4acb6452..4a9022faf945 100644 > --- a/drivers/input/mouse/cyapa_gen3.c > +++ b/drivers/input/mouse/cyapa_gen3.c > @@ -907,7 +907,6 @@ static u16 cyapa_get_wait_time_for_pwr_cmd(u8 pwr_mode) > static int cyapa_gen3_set_power_mode(struct cyapa *cyapa, u8 power_mode, > u16 always_unused, enum cyapa_pm_stage pm_stage) > { > - struct input_dev *input = cyapa->input; > u8 power; > int tries; > int sleep_time; > @@ -953,7 +952,6 @@ static int cyapa_gen3_set_power_mode(struct cyapa *cyapa, > u8 power_mode, >* depending on the command's content. >*/ > if (cyapa->operational && > - input && input_device_enabled(input) && > (pm_stage == CYAPA_PM_RUNTIME_SUSPEND || >pm_stage == CYAPA_PM_RUNTIME_RESUME)) { > /* Try to polling in 120Hz, read may fail, just ignore it. */ > @@ -1223,8 +1221,7 @@ static int cyapa_gen3_try_poll_handler(struct cyapa > *cyapa) > (data.finger_btn & OP_DATA_VALID) != OP_DATA_VALID) > return -EINVAL; > > - return cyapa_gen3_event_process(cyapa, ); > - > + return cyapa->input ? cyapa_gen3_event_process(cyapa, ) : 0; > } > > static int cyapa_gen3_initialize(struct cyapa *cyapa) { return 0; } > diff --git a/drivers/input/mouse/cyapa_gen5.c > b/drivers/input/mouse/cyapa_gen5.c > index abf42f77b4c5..afc5aa4dcf47 100644 > --- a/drivers/input/mouse/cyapa_gen5.c > +++ b/drivers/input/mouse/cyapa_gen5.c > @@ -518,8 +518,7 @@ int cyapa_empty_pip_output_data(struct cyapa *cyapa, > *len = length; > /* Response found, success. */ > return 0; > - } else if (cyapa->operational && > -input && input_device_enabled(input) && > +
[PATCH] regulator: max14577: Add proper module aliases strings
Add proper modalias structures to let this driver load automatically if compiled as module, because max14577 MFD driver creates MFD cells with such compatible strings. Signed-off-by: Marek Szyprowski --- drivers/regulator/max14577-regulator.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/regulator/max14577-regulator.c b/drivers/regulator/max14577-regulator.c index e34face736f4..1d78b455cc48 100644 --- a/drivers/regulator/max14577-regulator.c +++ b/drivers/regulator/max14577-regulator.c @@ -269,3 +269,5 @@ module_exit(max14577_regulator_exit); MODULE_AUTHOR("Krzysztof Kozlowski "); MODULE_DESCRIPTION("Maxim 14577/77836 regulator driver"); MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:max14577-regulator"); +MODULE_ALIAS("platform:max77836-regulator"); -- 2.17.1
[PATCH] extcon: max77693: Fix modalias string
The platform device driver name is "max77693-muic", so advertise it properly in the modalias string. This fixes automated module loading when this driver is compiled as a module. Fixes: db1b9037424b ("extcon: MAX77693: Add extcon-max77693 driver to support Maxim MAX77693 MUIC device") Signed-off-by: Marek Szyprowski --- drivers/extcon/extcon-max77693.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c index 4a410fd2ea9a..92af97e00828 100644 --- a/drivers/extcon/extcon-max77693.c +++ b/drivers/extcon/extcon-max77693.c @@ -1277,4 +1277,4 @@ module_platform_driver(max77693_muic_driver); MODULE_DESCRIPTION("Maxim MAX77693 Extcon driver"); MODULE_AUTHOR("Chanwoo Choi "); MODULE_LICENSE("GPL"); -MODULE_ALIAS("platform:extcon-max77693"); +MODULE_ALIAS("platform:max77693-muic"); -- 2.17.1
Re: [PATCH v4 2/7] Input: use input_device_enabled()
Hi Andrzej, On 07.12.2020 16:50, Andrzej Pietrasiewicz wrote: > Hi Marek, > > W dniu 07.12.2020 o 14:32, Marek Szyprowski pisze: >> Hi Andrzej, >> >> On 08.06.2020 13:22, Andrzej Pietrasiewicz wrote: >>> Use the newly added helper in relevant input drivers. >>> >>> Signed-off-by: Andrzej Pietrasiewicz >> >> This patch landed recently in linux-next as commit d69f0a43c677 ("Input: >> use input_device_enabled()"). Sadly it causes following warning during >> system suspend/resume cycle on ARM 32bit Samsung Exynos5250-based Snow >> Chromebook with kernel compiled from exynos_defconfig: >> >> [ cut here ] >> WARNING: CPU: 0 PID: 1777 at drivers/input/input.c:2230 >> input_device_enabled+0x68/0x6c >> Modules linked in: cmac bnep mwifiex_sdio mwifiex sha256_generic >> libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl bluetooth s5p_mfc >> exynos_gsc v4l2_mem2mem videob >> CPU: 0 PID: 1777 Comm: rtcwake Not tainted >> 5.10.0-rc6-next-20201207-1-g49a0dc04c46d-dirty #9902 >> Hardware name: Samsung Exynos (Flattened Device Tree) >> [] (unwind_backtrace) from [] (show_stack+0x10/0x14) >> [] (show_stack) from [] (dump_stack+0xb4/0xd4) >> [] (dump_stack) from [] (__warn+0xd8/0x11c) >> [] (__warn) from [] (warn_slowpath_fmt+0xb0/0xb8) >> [] (warn_slowpath_fmt) from [] >> (input_device_enabled+0x68/0x6c) >> [] (input_device_enabled) from [] > > Apparently you are hitting this line of code in drivers/input/input.c: > > lockdep_assert_held(>mutex); > > Inspecting input device's "users" member should happen under dev's lock. > This check and warning has been introduced by this patch. I assume that the suspend/resume paths are correct, but it looks that they were not tested with this patch thus it has not been noticed that they are not called under the input's lock. This needs a fix. Dmitry: how would you like to handle this issue? Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v4 2/7] Input: use input_device_enabled()
(cyapa_resume) from [] (dpm_run_callback+0xb0/0x3c8) [] (dpm_run_callback) from [] (device_resume+0xbc/0x260) [] (device_resume) from [] (dpm_resume+0x14c/0x51c) [] (dpm_resume) from [] (dpm_resume_end+0xc/0x18) [] (dpm_resume_end) from [] (suspend_devices_and_enter+0x1b4/0xbd4) [] (suspend_devices_and_enter) from [] (pm_suspend+0x314/0x42c) [] (pm_suspend) from [] (state_store+0x6c/0xc8) [] (state_store) from [] (kernfs_fop_write+0x10c/0x228) [] (kernfs_fop_write) from [] (vfs_write+0xc8/0x530) [] (vfs_write) from [] (ksys_write+0x60/0xd8) [] (ksys_write) from [] (ret_fast_syscall+0x0/0x2c) Exception stack(0xc45bffa8 to 0xc45bfff0) ... irq event stamp: 55829 hardirqs last enabled at (55837): [] vprintk_emit+0x2d8/0x32c hardirqs last disabled at (55844): [] vprintk_emit+0x29c/0x32c softirqs last enabled at (53552): [] __do_softirq+0x528/0x684 softirqs last disabled at (53541): [] irq_exit+0x1ec/0x1f8 ---[ end trace 6687a21e6b7e94ab ]--- [ cut here ] WARNING: CPU: 1 PID: 1777 at drivers/input/input.c:2230 input_device_enabled+0x68/0x6c Modules linked in: cmac bnep mwifiex_sdio mwifiex sha256_generic libsha256 sha256_arm cfg80211 btmrvl_sdio btmrvl bluetooth s5p_mfc exynos_gsc v4l2_mem2mem videob CPU: 1 PID: 1777 Comm: rtcwake Tainted: G W 5.10.0-rc6-next-20201207-1-g49a0dc04c46d-dirty #9902 Hardware name: Samsung Exynos (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0xb4/0xd4) [] (dump_stack) from [] (__warn+0xd8/0x11c) [] (__warn) from [] (warn_slowpath_fmt+0xb0/0xb8) [] (warn_slowpath_fmt) from [] (input_device_enabled+0x68/0x6c) [] (input_device_enabled) from [] (cyapa_gen3_set_power_mode+0x128/0x1cc) [] (cyapa_gen3_set_power_mode) from [] (cyapa_reinitialize+0xa8/0x154) [] (cyapa_reinitialize) from [] (cyapa_resume+0x48/0x98) [] (cyapa_resume) from [] (dpm_run_callback+0xb0/0x3c8) [] (dpm_run_callback) from [] (device_resume+0xbc/0x260) [] (device_resume) from [] (dpm_resume+0x14c/0x51c) [] (dpm_resume) from [] (dpm_resume_end+0xc/0x18) [] (dpm_resume_end) from [] (suspend_devices_and_enter+0x1b4/0xbd4) [] (suspend_devices_and_enter) from [] (pm_suspend+0x314/0x42c) [] (pm_suspend) from [] (state_store+0x6c/0xc8) [] (state_store) from [] (kernfs_fop_write+0x10c/0x228) [] (kernfs_fop_write) from [] (vfs_write+0xc8/0x530) [] (vfs_write) from [] (ksys_write+0x60/0xd8) [] (ksys_write) from [] (ret_fast_syscall+0x0/0x2c) Exception stack(0xc45bffa8 to 0xc45bfff0) ... irq event stamp: 56143 hardirqs last enabled at (56151): [] vprintk_emit+0x2d8/0x32c hardirqs last disabled at (56158): [] vprintk_emit+0x29c/0x32c softirqs last enabled at (53552): [] __do_softirq+0x528/0x684 softirqs last disabled at (53541): [] irq_exit+0x1ec/0x1f8 ---[ end trace 6687a21e6b7e94ac ]--- Let me know how I can help debugging this issue. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] mm/memblock:use a more appropriate order calculation when free memblock pages
Hi All, On 04.12.2020 14:42, Qian Cai wrote: > On Thu, 2020-12-03 at 23:23 +0800, carver4...@163.com wrote: >> From: Hailong Liu >> >> When system in the booting stage, pages span from [start, end] of a memblock >> are freed to buddy in a order as large as possible (less than MAX_ORDER) at >> first, then decrease gradually to a proper order(less than end) in a loop. >> >> However, *min(MAX_ORDER - 1UL, __ffs(start))* can not get the largest order >> in some cases. >> Instead, *__ffs(end - start)* may be more appropriate and meaningful. >> >> Signed-off-by: Hailong Liu > Reverting this commit on the top of today's linux-next fixed boot crashes on > multiple NUMA systems. I confirm. Reverting commit 4df001639c84 ("mm/memblock: use a more appropriate order calculation when free memblock pages") on top of linux next-20201204 fixed booting of my ARM32bit test systems. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v4 00/16] Overhaul multi-page lookups for THP
Hi On 03.12.2020 16:46, Marek Szyprowski wrote: > On 25.11.2020 03:32, Matthew Wilcox wrote: >> On Tue, Nov 17, 2020 at 11:43:02PM +, Matthew Wilcox wrote: >>> On Tue, Nov 17, 2020 at 07:15:13PM +, Matthew Wilcox wrote: >>>> I find both of these functions exceptionally confusing. Does this >>>> make it easier to understand? >>> Never mind, this is buggy. I'll send something better tomorrow. >> That took a week, not a day. *sigh*. At least this is shorter. >> >> commit 1a02863ce04fd325922d6c3db6d01e18d55f966b >> Author: Matthew Wilcox (Oracle) >> Date: Tue Nov 17 10:45:18 2020 -0500 >> >> fix mm-truncateshmem-handle-truncates-that-split-thps.patch > > This patch landed in todays linux-next (20201203) as commit > 8678b27f4b8b ("8678b27f4b8bfc130a13eb9e9f27171bcd8c0b3b"). Sadly it > breaks booting of ANY of my ARM 32bit test systems, which use initrd. > ARM64bit based systems boot fine. Here is example of the crash: One more thing. Reverting those two: 1b1aa968b0b6 mm-truncateshmem-handle-truncates-that-split-thps-fix-fix 8678b27f4b8b mm-truncateshmem-handle-truncates-that-split-thps-fix on top of linux next-20201203 fixes the boot issues. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v4 00/16] Overhaul multi-page lookups for THP
Hi On 25.11.2020 03:32, Matthew Wilcox wrote: > On Tue, Nov 17, 2020 at 11:43:02PM +, Matthew Wilcox wrote: >> On Tue, Nov 17, 2020 at 07:15:13PM +, Matthew Wilcox wrote: >>> I find both of these functions exceptionally confusing. Does this >>> make it easier to understand? >> Never mind, this is buggy. I'll send something better tomorrow. > That took a week, not a day. *sigh*. At least this is shorter. > > commit 1a02863ce04fd325922d6c3db6d01e18d55f966b > Author: Matthew Wilcox (Oracle) > Date: Tue Nov 17 10:45:18 2020 -0500 > > fix mm-truncateshmem-handle-truncates-that-split-thps.patch This patch landed in todays linux-next (20201203) as commit 8678b27f4b8b ("8678b27f4b8bfc130a13eb9e9f27171bcd8c0b3b"). Sadly it breaks booting of ANY of my ARM 32bit test systems, which use initrd. ARM64bit based systems boot fine. Here is example of the crash: Waiting 2 sec before mounting root device... RAMDISK: squashfs filesystem found at block 0 RAMDISK: Loading 37861KiB [1 disk] into ram disk... / / / / done. using deprecated initrd support, will be removed in 2021. [ cut here ] kernel BUG at fs/inode.c:531! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM Modules linked in: CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.10.0-rc6-next-20201203 #2131 Hardware name: Samsung Exynos (Flattened Device Tree) Workqueue: events delayed_fput PC is at clear_inode+0x74/0x88 LR is at clear_inode+0x14/0x88 pc : [] lr : [] psr: 21d3 sp : c1d2be68 ip : c1736ff4 fp : c1208f14 r10: c1208ec8 r9 : c20020c0 r8 : c209b0d8 r7 : c02f759c r6 : c0c13940 r5 : c209b244 r4 : c209b0d8 r3 : 24f9 r2 : r1 : r0 : c209b244 Flags: nzCv IRQs off FIQs off Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 4000404a DAC: 0051 Process kworker/0:1 (pid: 12, stack limit = 0x(ptrval)) Stack: (0xc1d2be68 to 0xc1d2c000) ... [] (clear_inode) from [] (evict+0x12c/0x13c) [] (evict) from [] (__dentry_kill+0xb0/0x188) [] (__dentry_kill) from [] (dput+0x2d8/0x67c) [] (dput) from [] (__fput+0xd4/0x24c) [] (__fput) from [] (delayed_fput+0x3c/0x48) [] (delayed_fput) from [] (process_one_work+0x234/0x7e4) [] (process_one_work) from [] (worker_thread+0x44/0x51c) [] (worker_thread) from [] (kthread+0x158/0x1a0) [] (kthread) from [] (ret_from_fork+0x14/0x38) Exception stack(0xc1d2bfb0 to 0xc1d2bff8) ... ---[ end trace b3c68905048e7f9b ]--- note: kworker/0:1[12] exited with preempt_count 1 BUG: sleeping function called from invalid context at ./include/linux/percpu-rwsem.h:49 in_atomic(): 0, irqs_disabled(): 128, non_block: 0, pid: 12, name: kworker/0:1 INFO: lockdep is turned off. irq event stamp: 7498 hardirqs last enabled at (7497): [] free_unref_page+0x80/0x88 hardirqs last disabled at (7498): [] _raw_spin_lock_irq+0x24/0x5c softirqs last enabled at (6234): [] linkwatch_do_dev+0x20/0x80 softirqs last disabled at (6232): [] rfc2863_policy+0x30/0xa4 CPU: 0 PID: 12 Comm: kworker/0:1 Tainted: G D 5.10.0-rc6-next-20201203 #2131 Hardware name: Samsung Exynos (Flattened Device Tree) Workqueue: events delayed_fput [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0xb4/0xd4) [] (dump_stack) from [] (___might_sleep+0x288/0x2d8) [] (___might_sleep) from [] (exit_signals+0x38/0x428) [] (exit_signals) from [] (do_exit+0xe4/0xc88) [] (do_exit) from [] (die+0x238/0x30c) [] (die) from [] (do_undefinstr+0xbc/0x26c) [] (do_undefinstr) from [] (__und_svc_finish+0x0/0x44) Exception stack(0xc1d2be18 to 0xc1d2be60) VFS: Mounted root (squashfs filesystem) readonly on device 1:0. be00: c209b244 be20: 24f9 c209b0d8 c209b244 c0c13940 c02f759c c209b0d8 c20020c0 be40: c1208ec8 c1208f14 c1736ff4 c1d2be68 c02fb2d4 c02fb334 21d3 [] (__und_svc_finish) from [] (clear_inode+0x74/0x88) [] (clear_inode) from [] (evict+0x12c/0x13c) [] (evict) from [] (__dentry_kill+0xb0/0x188) [] (__dentry_kill) from [] (dput+0x2d8/0x67c) [] (dput) from [] (__fput+0xd4/0x24c) [] (__fput) from [] (delayed_fput+0x3c/0x48) [] (delayed_fput) from [] (process_one_work+0x234/0x7e4) [] (process_one_work) from [] (worker_thread+0x44/0x51c) [] (worker_thread) from [] (kthread+0x158/0x1a0) [] (kthread) from [] (ret_from_fork+0x14/0x38) Exception stack(0xc1d2bfb0 to 0xc1d2bff8) bfa0: bfc0: bfe0: 0013 EXT4-fs (mmcblk0p6): INFO: recovery required on readonly filesystem EXT4-fs (mmcblk0p6): write access will be enabled during recovery EXT4-fs (mmcblk0p6): recovery complete EXT4-fs (mmcblk0p6): mounted filesystem with ordered data mode. Opts: (null) VFS: Mounted root (ext4 filesystem) readonly on device 179:6. Trying to move old root to /initrd ... I suppose this issue can be also reprod
[PATCH v2] phy: samsung: Merge Kconfig for Exynos5420 and Exynos5250
Exynos5420 variant of USB2 PHY is handled by the same code as the Exynos5250 one. Introducing a separate Kconfig symbol for it was an over-engineering, which turned out to cause build break for certain configurations: ERROR: modpost: "exynos5420_usb2_phy_config" [drivers/phy/samsung/phy-exynos-usb2.ko] undefined! Fix this by removing PHY_EXYNOS5420_USB2 symbol and using PHY_EXYNOS5250_USB2 also for Exynos5420 SoCs. Reported-by: Markus Reichl Fixes: 81b534f7e9b2 ("phy: samsung: Add support for the Exynos5420 variant of the USB2 PHY") Signed-off-by: Marek Szyprowski Acked-by: Krzysztof Kozlowski Reviewed-by: Jaehoon Chung --- v2: - reworded subject --- drivers/phy/samsung/Kconfig| 7 +-- drivers/phy/samsung/phy-samsung-usb2.c | 2 -- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig index 0f51d3bf38cc..e20d2fcc9fe7 100644 --- a/drivers/phy/samsung/Kconfig +++ b/drivers/phy/samsung/Kconfig @@ -64,12 +64,7 @@ config PHY_EXYNOS4X12_USB2 config PHY_EXYNOS5250_USB2 bool depends on PHY_SAMSUNG_USB2 - default SOC_EXYNOS5250 - -config PHY_EXYNOS5420_USB2 - bool - depends on PHY_SAMSUNG_USB2 - default SOC_EXYNOS5420 + default SOC_EXYNOS5250 || SOC_EXYNOS5420 config PHY_S5PV210_USB2 bool "Support for S5PV210" diff --git a/drivers/phy/samsung/phy-samsung-usb2.c b/drivers/phy/samsung/phy-samsung-usb2.c index 3908153f2ce5..ec2befabeea6 100644 --- a/drivers/phy/samsung/phy-samsung-usb2.c +++ b/drivers/phy/samsung/phy-samsung-usb2.c @@ -127,8 +127,6 @@ static const struct of_device_id samsung_usb2_phy_of_match[] = { .compatible = "samsung,exynos5250-usb2-phy", .data = _usb2_phy_config, }, -#endif -#ifdef CONFIG_PHY_EXYNOS5420_USB2 { .compatible = "samsung,exynos5420-usb2-phy", .data = _usb2_phy_config, -- 2.17.1
[PATCH] phy: samsung: Fix build break in USB2 PHY driver for Exynos5420 SoCs
Exynos5420 variant of USB2 PHY is handled by the same code as the Exynos5250 one. Introducing a separate Kconfig symbol for it was an over-engineering, which turned out to cause build break for certain configurations: ERROR: modpost: "exynos5420_usb2_phy_config" [drivers/phy/samsung/phy-exynos-usb2.ko] undefined! Fix this by removing PHY_EXYNOS5420_USB2 symbol and using PHY_EXYNOS5250_USB2 also for Exynos5420 SoCs. Reported-by: Markus Reichl Fixes: 81b534f7e9b2 ("phy: samsung: Add support for the Exynos5420 variant of the USB2 PHY") Signed-off-by: Marek Szyprowski --- Vinod: this a fix to the patch merged yesterday. If you want me to resend a fixed initial patch, let me know. --- drivers/phy/samsung/Kconfig| 7 +-- drivers/phy/samsung/phy-samsung-usb2.c | 2 -- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig index 0f51d3bf38cc..e20d2fcc9fe7 100644 --- a/drivers/phy/samsung/Kconfig +++ b/drivers/phy/samsung/Kconfig @@ -64,12 +64,7 @@ config PHY_EXYNOS4X12_USB2 config PHY_EXYNOS5250_USB2 bool depends on PHY_SAMSUNG_USB2 - default SOC_EXYNOS5250 - -config PHY_EXYNOS5420_USB2 - bool - depends on PHY_SAMSUNG_USB2 - default SOC_EXYNOS5420 + default SOC_EXYNOS5250 || SOC_EXYNOS5420 config PHY_S5PV210_USB2 bool "Support for S5PV210" diff --git a/drivers/phy/samsung/phy-samsung-usb2.c b/drivers/phy/samsung/phy-samsung-usb2.c index 3908153f2ce5..ec2befabeea6 100644 --- a/drivers/phy/samsung/phy-samsung-usb2.c +++ b/drivers/phy/samsung/phy-samsung-usb2.c @@ -127,8 +127,6 @@ static const struct of_device_id samsung_usb2_phy_of_match[] = { .compatible = "samsung,exynos5250-usb2-phy", .data = _usb2_phy_config, }, -#endif -#ifdef CONFIG_PHY_EXYNOS5420_USB2 { .compatible = "samsung,exynos5420-usb2-phy", .data = _usb2_phy_config, -- 2.17.1
Re: linux-next: Tree for Nov 30 (drivers/pci/controller/dwc/pcie-designware-host.c)
Hi On 01.12.2020 12:34, Lorenzo Pieralisi wrote: > On Mon, Nov 30, 2020 at 08:44:55PM -0800, Randy Dunlap wrote: >> On 11/30/20 12:36 AM, Stephen Rothwell wrote: >>> Changes since 20201127: >> on x86_64: >> >> WARNING: unmet direct dependencies detected for PCIE_DW_HOST >>Depends on [n]: PCI [=y] && PCI_MSI_IRQ_DOMAIN [=n] >>Selected by [y]: >>- PCI_EXYNOS [=y] && PCI [=y] && (ARCH_EXYNOS || COMPILE_TEST [=y]) > > [...] > >> caused by: >> commit f0a6743028f938cdd34e0c3249d3f0e6bfa04073 >> Author: Jaehoon Chung >> Date: Fri Nov 13 18:01:39 2020 +0100 >> >> PCI: dwc: exynos: Rework the driver to support Exynos5433 varian >> >> >> which removed "depends on PCI_MSI_IRQ_DOMAIN from config PCI_EXYNOS. > Fixed up and squashed in the original commit - we should probably rework > the DWC driver dependencies on PCI_MSI_IRQ_DOMAIN to really fix it, for > the time being this should do. Thanks! I wasn't aware of that hidden dependency. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v2 5/6] media: uvcvideo: Use dma_alloc_noncontiguos API
gt, uvc_urb->pages, > + PAGE_ALIGN(stream->urb_size) >> PAGE_SHIFT, 0, > + stream->urb_size, GFP_KERNEL)) { > + vunmap(uvc_urb->buffer); > + dma_free_noncontiguous(dma_dev, stream->urb_size, > +uvc_urb->pages, uvc_urb->dma); > + return false; > + } > + > + return true; > +} > +#else > +static bool uvc_alloc_urb_buffer(struct uvc_streaming *stream, > + struct uvc_urb *uvc_urb, gfp_t gfp_flags) > +{ > + uvc_urb->buffer = kmalloc(stream->urb_size, gfp_flags | __GFP_NOWARN); > + > + return uvc_urb->buffer != NULL; > +} > +#endif > + > /* >* Allocate transfer buffers. This function can be called with buffers >* already allocated when resuming from suspend, in which case it will > @@ -1607,19 +1674,11 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming > *stream, > > /* Retry allocations until one succeed. */ > for (; npackets > 1; npackets /= 2) { > + stream->urb_size = psize * npackets; > for (i = 0; i < UVC_URBS; ++i) { > struct uvc_urb *uvc_urb = >uvc_urb[i]; > > - stream->urb_size = psize * npackets; > -#ifndef CONFIG_DMA_NONCOHERENT > - uvc_urb->buffer = usb_alloc_coherent( > - stream->dev->udev, stream->urb_size, > - gfp_flags | __GFP_NOWARN, _urb->dma); > -#else > - uvc_urb->buffer = > - kmalloc(stream->urb_size, gfp_flags | __GFP_NOWARN); > -#endif > - if (!uvc_urb->buffer) { > + if (!uvc_alloc_urb_buffer(stream, uvc_urb, gfp_flags)) { > uvc_free_urb_buffers(stream); > break; > } > diff --git a/drivers/media/usb/uvc/uvcvideo.h > b/drivers/media/usb/uvc/uvcvideo.h > index a3dfacf069c4..3e6618a2ac82 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -532,6 +532,8 @@ struct uvc_urb { > > char *buffer; > dma_addr_t dma; > + struct page **pages; > + struct sg_table sgt; > > unsigned int async_operations; > struct uvc_copy_op copy_operations[UVC_MAX_PACKETS]; Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
[PATCH] spi: Fix potential NULL pointer dereference in spi_shutdown()
Shutdown bus function might be called on the unbound device, so add a check if there is a driver before calling its shutdown function. This fixes following kernel panic obserbed during system reboot: Unable to handle kernel NULL pointer dereference at virtual address 0018 ... Call trace: spi_shutdown+0x10/0x38 kernel_restart_prepare+0x34/0x40 kernel_restart+0x14/0x88 __do_sys_reboot+0x148/0x248 __arm64_sys_reboot+0x1c/0x28 el0_svc_common.constprop.3+0x74/0x198 do_el0_svc+0x20/0x98 el0_sync_handler+0x140/0x1a8 el0_sync+0x140/0x180 Code: f9403402 d1008041 f15f 9a9f1021 (f9400c21) ---[ end trace 266c07205a2d632e ]--- Fixes: 9db34ee64ce4 ("spi: Use bus_type functions for probe, remove and shutdown)" Signed-off-by: Marek Szyprowski --- drivers/spi/spi.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 0c3f3a962448..cd3c395b4e90 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -426,10 +426,12 @@ static int spi_remove(struct device *dev) static void spi_shutdown(struct device *dev) { - const struct spi_driver *sdrv = to_spi_driver(dev->driver); + if (dev->driver) { + const struct spi_driver *sdrv = to_spi_driver(dev->driver); - if (sdrv->shutdown) - sdrv->shutdown(to_spi_device(dev)); + if (sdrv->shutdown) + sdrv->shutdown(to_spi_device(dev)); + } } struct bus_type spi_bus_type = { -- 2.17.1
Re: [PATCH v2 2/3] spi: Use bus_type functions for probe, remove and shutdown
Hi Uwe, On 19.11.2020 17:16, Uwe Kleine-König wrote: > The eventual goal is to get rid of the callbacks in struct > device_driver. Other than not using driver callbacks there should be no > side effect of this patch. > > Signed-off-by: Uwe Kleine-König This patch landed recently in linux-next as commit 9db34ee64ce4 ("spi: Use bus_type functions for probe, remove and shutdown"). It causes a regression on some of my test boards: Unable to handle kernel NULL pointer dereference at virtual address 0018 Mem abort info: ESR = 0x9604 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 Data abort info: ISV = 0, ISS = 0x0004 CM = 0, WnR = 0 user pgtable: 4k pages, 48-bit VAs, pgdp=318ed000 [0018] pgd=, p4d= Internal error: Oops: 9604 [#1] PREEMPT SMP Modules linked in: cpufreq_powersave cpufreq_conservative brcmfmac brcmutil cfg80211 crct10dif_ce s3fwrn5_i2c s3fwrn5 nci nfc s5p_mfc s5p_jpeg hci_uart btqca btbc buf2_dma_contig videobuf2_memops videobuf2_v4l2 bluetooth videobuf2_common videodev panfrost gpu_sched ecdh_generic mc ecc rfkill ip_tables x_tables ipv6 CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 5.10.0-rc5-next-20201124+ #9771 Hardware name: Samsung TM2E board (DT) pstate: 6005 (nZCv daif -PAN -UAO -TCO BTYPE=--) pc : spi_shutdown+0x10/0x38 lr : device_shutdown+0x10c/0x350 sp : 80001311bc70 ... Call trace: spi_shutdown+0x10/0x38 kernel_restart_prepare+0x34/0x40 kernel_restart+0x14/0x88 __do_sys_reboot+0x148/0x248 __arm64_sys_reboot+0x1c/0x28 el0_svc_common.constprop.3+0x74/0x198 do_el0_svc+0x20/0x98 el0_sync_handler+0x140/0x1a8 el0_sync+0x140/0x180 Code: f9403402 d1008041 f15f 9a9f1021 (f9400c21) ---[ end trace 266c07205a2d632e ]--- Kernel panic - not syncing: Oops: Fatal exception Kernel Offset: disabled CPU features: 0x0240022,65006087 Memory Limit: none ---[ end Kernel panic - not syncing: Oops: Fatal exception ]--- > --- > drivers/spi/spi.c | 33 - > 1 file changed, 16 insertions(+), 17 deletions(-) > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index 5becf6c2c409..e8c0a000ee19 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -374,16 +374,7 @@ static int spi_uevent(struct device *dev, struct > kobj_uevent_env *env) > return add_uevent_var(env, "MODALIAS=%s%s", SPI_MODULE_PREFIX, > spi->modalias); > } > > -struct bus_type spi_bus_type = { > - .name = "spi", > - .dev_groups = spi_dev_groups, > - .match = spi_match_device, > - .uevent = spi_uevent, > -}; > -EXPORT_SYMBOL_GPL(spi_bus_type); > - > - > -static int spi_drv_probe(struct device *dev) > +static int spi_probe(struct device *dev) > { > const struct spi_driver *sdrv = to_spi_driver(dev->driver); > struct spi_device *spi = to_spi_device(dev); > @@ -414,7 +405,7 @@ static int spi_drv_probe(struct device *dev) > return ret; > } > > -static int spi_drv_remove(struct device *dev) > +static int spi_remove(struct device *dev) > { > const struct spi_driver *sdrv = to_spi_driver(dev->driver); > int ret = 0; > @@ -426,13 +417,25 @@ static int spi_drv_remove(struct device *dev) > return ret; > } > > -static void spi_drv_shutdown(struct device *dev) > +static void spi_shutdown(struct device *dev) > { > const struct spi_driver *sdrv = to_spi_driver(dev->driver); > > - sdrv->shutdown(to_spi_device(dev)); > + if (sdrv->shutdown) > + sdrv->shutdown(to_spi_device(dev)); > } In the above function dev->driver might be NULL, so its use in to_spi_driver() and sdrv->shutdown leads to NULL pointer dereference. I didn't check the details, but a simple check for NULL dev->driver and return is enough to fix this issue. I can send such fix if you want. > +struct bus_type spi_bus_type = { > + .name = "spi", > + .dev_groups = spi_dev_groups, > + .match = spi_match_device, > + .uevent = spi_uevent, > + .probe = spi_probe, > + .remove = spi_remove, > + .shutdown = spi_shutdown, > +}; > +EXPORT_SYMBOL_GPL(spi_bus_type); > + > /** >* __spi_register_driver - register a SPI driver >* @owner: owner module of the driver to register > @@ -445,10 +448,6 @@ int __spi_register_driver(struct module *owner, struct > spi_driver *sdrv) > { > sdrv->driver.owner = owner; > sdrv->driver.bus = _bus_type; > - sdrv->driver.probe = spi_drv_probe; > - sdrv->driver.remove = spi_drv_remove; > - if (sdrv->shutdown) > - sdrv->driver.shutdown = spi_drv_shutdown; > return driver_register(>driver); > } > EXPORT_SYMBOL_GPL(__spi_register_driver); Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 2/2] ARM: dts: exynos: use Exynos5420 dedicated USB2 PHY compatible
Hi Krzysztof, On 20.11.2020 12:05, Krzysztof Kozlowski wrote: > On Fri, Nov 20, 2020 at 09:56:37AM +0100, Marek Szyprowski wrote: >> USB2.0 PHY in Exynos5420 differs from Exynos5250 variant a bit, so use the >> recently introduced dedicated compatible for Exynos5420. >> >> Signed-off-by: Marek Szyprowski >> --- >> arch/arm/boot/dts/exynos54xx.dtsi | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm/boot/dts/exynos54xx.dtsi >> b/arch/arm/boot/dts/exynos54xx.dtsi >> index fe9d34c23374..2ddb7a5f12b3 100644 >> --- a/arch/arm/boot/dts/exynos54xx.dtsi >> +++ b/arch/arm/boot/dts/exynos54xx.dtsi >> @@ -188,7 +188,7 @@ >> compatible = "samsung,exynos4210-ehci"; >> reg = <0x1211 0x100>; >> interrupts = ; >> -phys = <_phy 1>; >> +phys = <_phy 0>; >> phy-names = "host"; >> }; >> >> @@ -196,12 +196,12 @@ >> compatible = "samsung,exynos4210-ohci"; >> reg = <0x1212 0x100>; >> interrupts = ; >> -phys = <_phy 1>; >> +phys = <_phy 0>; >> phy-names = "host"; >> }; >> >> usb2_phy: phy@1213 { >> -compatible = "samsung,exynos5250-usb2-phy"; >> +compatible = "samsung,exynos5420-usb2-phy"; > The DTS change will wait till PHY driver adjustements get merged... or > if the difference is not critical, maybe using both compatibles (5420 > and 5250) would have sense? It won't work easily with both compatibles, because in the 5420 variant I've also changed the PHY indices (5420 has no device and second hsic phy). IMHO the dts change can wait for the next release. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
[PATCH v4 4/5 REBASED RESEND] phy: samsung: phy-exynos-pcie: rework driver to support Exynos5433 PCIe PHY
From: Jaehoon Chung Exynos5440 SoC support has been dropped since commit 8c83315da1cf ("ARM: dts: exynos: Remove Exynos5440"). Rework this driver to support PCIe PHY variant found in the Exynos5433 SoCs. Signed-off-by: Jaehoon Chung [mszyprow: reworked the driver to support only Exynos5433 variant, rebased onto current kernel code, rewrote commit message] Signed-off-by: Marek Szyprowski Acked-by: Krzysztof Kozlowski Reviewed-by: Jingoo Han --- Resend reason: rebased onto current -next branch --- drivers/phy/samsung/phy-exynos-pcie.c | 301 ++ 1 file changed, 112 insertions(+), 189 deletions(-) diff --git a/drivers/phy/samsung/phy-exynos-pcie.c b/drivers/phy/samsung/phy-exynos-pcie.c index c98fff5c1ac8..d91de323dd0e 100644 --- a/drivers/phy/samsung/phy-exynos-pcie.c +++ b/drivers/phy/samsung/phy-exynos-pcie.c @@ -4,70 +4,41 @@ * * Phy provider for PCIe controller on Exynos SoC series * - * Copyright (C) 2017 Samsung Electronics Co., Ltd. + * Copyright (C) 2017-2020 Samsung Electronics Co., Ltd. * Jaehoon Chung */ -#include #include -#include -#include #include -#include -#include #include #include #include #include -/* PCIe Purple registers */ -#define PCIE_PHY_GLOBAL_RESET 0x000 -#define PCIE_PHY_COMMON_RESET 0x004 -#define PCIE_PHY_CMN_REG 0x008 -#define PCIE_PHY_MAC_RESET 0x00c -#define PCIE_PHY_PLL_LOCKED0x010 -#define PCIE_PHY_TRSVREG_RESET 0x020 -#define PCIE_PHY_TRSV_RESET0x024 - -/* PCIe PHY registers */ -#define PCIE_PHY_IMPEDANCE 0x004 -#define PCIE_PHY_PLL_DIV_0 0x008 -#define PCIE_PHY_PLL_BIAS 0x00c -#define PCIE_PHY_DCC_FEEDBACK 0x014 -#define PCIE_PHY_PLL_DIV_1 0x05c -#define PCIE_PHY_COMMON_POWER 0x064 -#define PCIE_PHY_COMMON_PD_CMN BIT(3) -#define PCIE_PHY_TRSV0_EMP_LVL 0x084 -#define PCIE_PHY_TRSV0_DRV_LVL 0x088 -#define PCIE_PHY_TRSV0_RXCDR 0x0ac -#define PCIE_PHY_TRSV0_POWER 0x0c4 -#define PCIE_PHY_TRSV0_PD_TSV BIT(7) -#define PCIE_PHY_TRSV0_LVCC0x0dc -#define PCIE_PHY_TRSV1_EMP_LVL 0x144 -#define PCIE_PHY_TRSV1_RXCDR 0x16c -#define PCIE_PHY_TRSV1_POWER 0x184 -#define PCIE_PHY_TRSV1_PD_TSV BIT(7) -#define PCIE_PHY_TRSV1_LVCC0x19c -#define PCIE_PHY_TRSV2_EMP_LVL 0x204 -#define PCIE_PHY_TRSV2_RXCDR 0x22c -#define PCIE_PHY_TRSV2_POWER 0x244 -#define PCIE_PHY_TRSV2_PD_TSV BIT(7) -#define PCIE_PHY_TRSV2_LVCC0x25c -#define PCIE_PHY_TRSV3_EMP_LVL 0x2c4 -#define PCIE_PHY_TRSV3_RXCDR 0x2ec -#define PCIE_PHY_TRSV3_POWER 0x304 -#define PCIE_PHY_TRSV3_PD_TSV BIT(7) -#define PCIE_PHY_TRSV3_LVCC0x31c - -struct exynos_pcie_phy_data { - const struct phy_ops*ops; -}; +#define PCIE_PHY_OFFSET(x) ((x) * 0x4) + +/* Sysreg FSYS register offsets and bits for Exynos5433 */ +#define PCIE_EXYNOS5433_PHY_MAC_RESET 0x0208 +#define PCIE_MAC_RESET_MASK0xFF +#define PCIE_MAC_RESET BIT(4) +#define PCIE_EXYNOS5433_PHY_L1SUB_CM_CON 0x1010 +#define PCIE_REFCLK_GATING_EN BIT(0) +#define PCIE_EXYNOS5433_PHY_COMMON_RESET 0x1020 +#define PCIE_PHY_RESET BIT(0) +#define PCIE_EXYNOS5433_PHY_GLOBAL_RESET 0x1040 +#define PCIE_GLOBAL_RESET BIT(0) +#define PCIE_REFCLKBIT(1) +#define PCIE_REFCLK_MASK 0x16 +#define PCIE_APP_REQ_EXIT_L1_MODE BIT(5) + +/* PMU PCIE PHY isolation control */ +#define EXYNOS5433_PMU_PCIE_PHY_OFFSET 0x730 /* For Exynos pcie phy */ struct exynos_pcie_phy { - const struct exynos_pcie_phy_data *drv_data; - void __iomem *phy_base; - void __iomem *blk_base; /* For exynos5440 */ + void __iomem *base; + struct regmap *pmureg; + struct regmap *fsysreg; }; static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset) @@ -75,153 +46,103 @@ static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset) writel(val, base + offset); } -static u32 exynos_pcie_phy_readl(void __iomem *base, u32 offset) -{ - return readl(base + offset); -} - -/* For Exynos5440 specific functions */ -static int exynos5440_pcie_phy_init(struct phy *phy) +/* Exynos5433 specific functions */ +static int exynos5433_pcie_phy_init(struct phy *phy) { struct exynos_pcie_phy *ep = phy_get_drvdata(phy); - /* DCC feedback control off */ - exynos_pcie_phy_writel(ep->phy_base, 0x29, PCIE_PHY_DCC_FEEDBACK); - - /* set TX/RX impedance */ - exynos_pcie_phy_writel(ep->phy_base, 0xd5, PCIE_PHY_IMPEDANCE); - - /* set 50Mhz PHY clock */ - exynos_pcie_phy_writel(ep
Re: [PATCH v4 4/5] phy: samsung: phy-exynos-pcie: rework driver to support Exynos5433 PCIe PHY
Hi Vinod, On 20.11.2020 10:41, Vinod Koul wrote: > On 13-11-20, 18:01, Marek Szyprowski wrote: >> From: Jaehoon Chung >> >> Exynos5440 SoC support has been dropped since commit 8c83315da1cf ("ARM: >> dts: exynos: Remove Exynos5440"). Rework this driver to support PCIe PHY >> variant found in the Exynos5433 SoCs. > I am expecting this series to go thru PCI tree, so: > > Acked-By: Vinod Koul Frankly, the PHY driver can also go via PHY tree without causing any issue. The old driver is not used at all, so there is no runtime dependency. This will help avoiding the merge conflict: yesterday I've noticed that this patch conflicts with the commit 2f0c9fac3be6 ("phy: samsung: convert to devm_platform_ioremap_resource") in phy-next. The resolution is simple (use all the code from the new driver), but if needed I can resend the PHY driver after a rebase onto the current next tree. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
[PATCH 1/2] phy: samsung: Add support for the Exynos5420 variant of the USB2 PHY
Exynos5420 differs a bit from Exynos5250 in USB2 PHY related registers in the PMU region. Add a variant for the Exynos5420 case. Till now, USB2 PHY worked only because the bootloader enabled the PHY, but then driver messed USB 3.0 DRD related registers during the suspend/resume cycle. Signed-off-by: Marek Szyprowski --- .../devicetree/bindings/phy/samsung-phy.txt | 1 + drivers/phy/samsung/Kconfig | 7 ++- drivers/phy/samsung/phy-exynos5250-usb2.c | 48 +-- drivers/phy/samsung/phy-samsung-usb2.c| 6 +++ drivers/phy/samsung/phy-samsung-usb2.h| 1 + 5 files changed, 48 insertions(+), 15 deletions(-) diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt index 7510830a79bd..8f51aee91101 100644 --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt @@ -47,6 +47,7 @@ Required properties: - "samsung,exynos4210-usb2-phy" - "samsung,exynos4x12-usb2-phy" - "samsung,exynos5250-usb2-phy" + - "samsung,exynos5420-usb2-phy" - "samsung,s5pv210-usb2-phy" - reg : a list of registers used by phy driver - first and obligatory is the location of phy modules registers diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig index e20d2fcc9fe7..0f51d3bf38cc 100644 --- a/drivers/phy/samsung/Kconfig +++ b/drivers/phy/samsung/Kconfig @@ -64,7 +64,12 @@ config PHY_EXYNOS4X12_USB2 config PHY_EXYNOS5250_USB2 bool depends on PHY_SAMSUNG_USB2 - default SOC_EXYNOS5250 || SOC_EXYNOS5420 + default SOC_EXYNOS5250 + +config PHY_EXYNOS5420_USB2 + bool + depends on PHY_SAMSUNG_USB2 + default SOC_EXYNOS5420 config PHY_S5PV210_USB2 bool "Support for S5PV210" diff --git a/drivers/phy/samsung/phy-exynos5250-usb2.c b/drivers/phy/samsung/phy-exynos5250-usb2.c index 4f53b711fd6f..e198010e1bfd 100644 --- a/drivers/phy/samsung/phy-exynos5250-usb2.c +++ b/drivers/phy/samsung/phy-exynos5250-usb2.c @@ -117,9 +117,9 @@ /* Isolation, configured in the power management unit */ #define EXYNOS_5250_USB_ISOL_OTG_OFFSET0x704 -#define EXYNOS_5250_USB_ISOL_OTG BIT(0) #define EXYNOS_5250_USB_ISOL_HOST_OFFSET 0x708 -#define EXYNOS_5250_USB_ISOL_HOST BIT(0) +#define EXYNOS_5420_USB_ISOL_HOST_OFFSET 0x70C +#define EXYNOS_5250_USB_ISOL_ENABLEBIT(0) /* Mode swtich register */ #define EXYNOS_5250_MODE_SWITCH_OFFSET 0x230 @@ -132,7 +132,6 @@ enum exynos4x12_phy_id { EXYNOS5250_HOST, EXYNOS5250_HSIC0, EXYNOS5250_HSIC1, - EXYNOS5250_NUM_PHYS, }; /* @@ -176,20 +175,19 @@ static void exynos5250_isol(struct samsung_usb2_phy_instance *inst, bool on) { struct samsung_usb2_phy_driver *drv = inst->drv; u32 offset; - u32 mask; + u32 mask = EXYNOS_5250_USB_ISOL_ENABLE; - switch (inst->cfg->id) { - case EXYNOS5250_DEVICE: + if (drv->cfg == _usb2_phy_config && + inst->cfg->id == EXYNOS5250_DEVICE) offset = EXYNOS_5250_USB_ISOL_OTG_OFFSET; - mask = EXYNOS_5250_USB_ISOL_OTG; - break; - case EXYNOS5250_HOST: + else if (drv->cfg == _usb2_phy_config && +inst->cfg->id == EXYNOS5250_HOST) offset = EXYNOS_5250_USB_ISOL_HOST_OFFSET; - mask = EXYNOS_5250_USB_ISOL_HOST; - break; - default: + else if (drv->cfg == _usb2_phy_config && +inst->cfg->id == EXYNOS5250_HOST) + offset = EXYNOS_5420_USB_ISOL_HOST_OFFSET; + else return; - } regmap_update_bits(drv->reg_pmu, offset, mask, on ? 0 : mask); } @@ -390,9 +388,31 @@ static const struct samsung_usb2_common_phy exynos5250_phys[] = { }, }; +static const struct samsung_usb2_common_phy exynos5420_phys[] = { + { + .label = "host", + .id = EXYNOS5250_HOST, + .power_on = exynos5250_power_on, + .power_off = exynos5250_power_off, + }, + { + .label = "hsic", + .id = EXYNOS5250_HSIC0, + .power_on = exynos5250_power_on, + .power_off = exynos5250_power_off, + }, +}; + const struct samsung_usb2_phy_config exynos5250_usb2_phy_config = { .has_mode_switch= 1, - .num_phys = EXYNOS5250_NUM_PHYS, + .num_phys = ARRAY_SIZE(exynos5250_phys), .phys = exynos5250_phys, .rate_to_clk= exynos5250_rate_to_clk, }; + +const struct samsung_u
[PATCH 2/2] ARM: dts: exynos: use Exynos5420 dedicated USB2 PHY compatible
USB2.0 PHY in Exynos5420 differs from Exynos5250 variant a bit, so use the recently introduced dedicated compatible for Exynos5420. Signed-off-by: Marek Szyprowski --- arch/arm/boot/dts/exynos54xx.dtsi | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/exynos54xx.dtsi b/arch/arm/boot/dts/exynos54xx.dtsi index fe9d34c23374..2ddb7a5f12b3 100644 --- a/arch/arm/boot/dts/exynos54xx.dtsi +++ b/arch/arm/boot/dts/exynos54xx.dtsi @@ -188,7 +188,7 @@ compatible = "samsung,exynos4210-ehci"; reg = <0x1211 0x100>; interrupts = ; - phys = <_phy 1>; + phys = <_phy 0>; phy-names = "host"; }; @@ -196,12 +196,12 @@ compatible = "samsung,exynos4210-ohci"; reg = <0x1212 0x100>; interrupts = ; - phys = <_phy 1>; + phys = <_phy 0>; phy-names = "host"; }; usb2_phy: phy@1213 { - compatible = "samsung,exynos5250-usb2-phy"; + compatible = "samsung,exynos5420-usb2-phy"; reg = <0x1213 0x100>; #phy-cells = <1>; }; -- 2.17.1
[PATCH 0/2] Fix USB2 PHY operation on Exynos542x
Dear All, This patchset finally fixes the last remaining issue related to the system suspend/resume on Odroid XU3/XU4/HC1 board family. It can be observed that system suspend/resume fails on XU4/HC1 when kernel is compiled from multi_v7_defconfig. Chaning the configuration a bit - switching Exynos USB2 PHY driver to be built-in surprisingly fixed that issue. An investigation revealed that the Exynos USB2 PHY driver poked wrong registers in the PMU area on Exynos5420 SoCs breaking the USB3.0 DRD PHY operation, what caused the suspend failure. Fix this by learning the Exynos USB2 PHY driver about the Exynos5420 variant. Best regards, Marek Szyprowski Patch summary: Marek Szyprowski (2): phy: samsung: add support for the Exynos5420 variant of the USB2 PHY ARM: dts: exynos: use Exynos5420 dedicated USB2 PHY compatible .../devicetree/bindings/phy/samsung-phy.txt | 1 + arch/arm/boot/dts/exynos54xx.dtsi | 6 +-- drivers/phy/samsung/Kconfig | 7 ++- drivers/phy/samsung/phy-exynos5250-usb2.c | 48 +-- drivers/phy/samsung/phy-samsung-usb2.c| 6 +++ drivers/phy/samsung/phy-samsung-usb2.h| 1 + 6 files changed, 51 insertions(+), 18 deletions(-) -- 2.17.1
[PATCH] interconnect: fix memory trashing in of_count_icc_providers()
of_count_icc_providers() function uses for_each_available_child_of_node() helper to recursively check all the available nodes. This helper already properly handles child nodes' reference count, so there is no need to do it explicitely. Remove the excessive call to of_node_put(). This fixes memory trashing when CONFIG_OF_DYNAMIC is enabled (for example arm/multi_v7_defconfig). Fixes: b1d681d8d324 ("interconnect: Add sync state support") Signed-off-by: Marek Szyprowski --- drivers/interconnect/core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index 974a66725d09..5ad519c9f239 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -1083,7 +1083,6 @@ static int of_count_icc_providers(struct device_node *np) count++; count += of_count_icc_providers(child); } - of_node_put(np); return count; } -- 2.17.1
Re: [PATCH net-next v2] r8153_ecm: avoid to be prior to r8152 driver
Hi On 18.11.2020 07:43, Hayes Wang wrote: > Avoid r8153_ecm is compiled as built-in, if r8152 driver is compiled > as modules. Otherwise, the r8153_ecm would be used, even though the > device is supported by r8152 driver. > > Fixes: c1aedf015ebd ("net/usb/r8153_ecm: support ECM mode for RTL8153") > Reported-by: Marek Szyprowski > Signed-off-by: Hayes Wang Yes, this looks like a proper fix. Tested-by: Marek Szyprowski > --- > v2: > Use a separate Kconfig entry for r8153_ecm with proper dependencies. > > drivers/net/usb/Kconfig | 9 + > drivers/net/usb/Makefile | 3 ++- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig > index b46993d5f997..1e3719028780 100644 > --- a/drivers/net/usb/Kconfig > +++ b/drivers/net/usb/Kconfig > @@ -628,4 +628,13 @@ config USB_NET_AQC111 > This driver should work with at least the following devices: > * Aquantia AQtion USB to 5GbE > > +config USB_RTL8153_ECM > + tristate "RTL8153 ECM support" > + depends on USB_NET_CDCETHER && (USB_RTL8152 || USB_RTL8152=n) > + default y > + help > + This option supports ECM mode for RTL8153 ethernet adapter, when > + CONFIG_USB_RTL8152 is not set, or the RTL8153 device is not > + supported by r8152 driver. > + > endif # USB_NET_DRIVERS > diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile > index 99381e6bea78..4964f7b326fb 100644 > --- a/drivers/net/usb/Makefile > +++ b/drivers/net/usb/Makefile > @@ -13,7 +13,7 @@ obj-$(CONFIG_USB_LAN78XX) += lan78xx.o > obj-$(CONFIG_USB_NET_AX8817X) += asix.o > asix-y := asix_devices.o asix_common.o ax88172a.o > obj-$(CONFIG_USB_NET_AX88179_178A) += ax88179_178a.o > -obj-$(CONFIG_USB_NET_CDCETHER) += cdc_ether.o r8153_ecm.o > +obj-$(CONFIG_USB_NET_CDCETHER) += cdc_ether.o > obj-$(CONFIG_USB_NET_CDC_EEM) += cdc_eem.o > obj-$(CONFIG_USB_NET_DM9601)+= dm9601.o > obj-$(CONFIG_USB_NET_SR9700)+= sr9700.o > @@ -41,3 +41,4 @@ obj-$(CONFIG_USB_NET_QMI_WWAN) += qmi_wwan.o > obj-$(CONFIG_USB_NET_CDC_MBIM) += cdc_mbim.o > obj-$(CONFIG_USB_NET_CH9200)+= ch9200.o > obj-$(CONFIG_USB_NET_AQC111)+= aqc111.o > +obj-$(CONFIG_USB_RTL8153_ECM)+= r8153_ecm.o Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 3/4] s5p-mfc: Use display delay and display enable std controls
On 09.11.2020 18:35, Stanimir Varbanov wrote: > Use the standard display_delay and display_delay_enable controls, > the legacy private MFC controls are kept for backward compatibility. > > Signed-off-by: Stanimir Varbanov Acked-by: Marek Szyprowski > --- > drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 16 > 1 file changed, 16 insertions(+) > > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c > b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c > index 61e144a35201..4a3e8e9bbff2 100644 > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c > @@ -167,6 +167,13 @@ static struct mfc_control controls[] = { > .step = 1, > .default_value = 0, > }, > + { > + .id = V4L2_CID_MPEG_VIDEO_DECODER_DISPLAY_DELAY, > + .type = V4L2_CTRL_TYPE_INTEGER, > + .minimum = 0, > + .maximum = 16383, > + .default_value = 0, > + }, > { > .id = > V4L2_CID_MPEG_MFC51_VIDEO_DECODER_H264_DISPLAY_DELAY_ENABLE, > .type = V4L2_CTRL_TYPE_BOOLEAN, > @@ -176,6 +183,13 @@ static struct mfc_control controls[] = { > .step = 1, > .default_value = 0, > }, > + { > + .id = V4L2_CID_MPEG_VIDEO_DECODER_DISPLAY_DELAY_ENABLE, > + .type = V4L2_CTRL_TYPE_BOOLEAN, > + .minimum = 0, > + .maximum = 1, > + .default_value = 0, > + }, > { > .id = V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER, > .type = V4L2_CTRL_TYPE_BOOLEAN, > @@ -690,9 +704,11 @@ static int s5p_mfc_dec_s_ctrl(struct v4l2_ctrl *ctrl) > > switch (ctrl->id) { > case V4L2_CID_MPEG_MFC51_VIDEO_DECODER_H264_DISPLAY_DELAY: > + case V4L2_CID_MPEG_VIDEO_DECODER_DISPLAY_DELAY: > ctx->display_delay = ctrl->val; > break; > case V4L2_CID_MPEG_MFC51_VIDEO_DECODER_H264_DISPLAY_DELAY_ENABLE: > + case V4L2_CID_MPEG_VIDEO_DECODER_DISPLAY_DELAY_ENABLE: > ctx->display_delay_enable = ctrl->val; > break; > case V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER: Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH net-next] r8153_ecm: avoid to be prior to r8152 driver
Hi On 16.11.2020 07:52, Hayes Wang wrote: > Avoid r8153_ecm is compiled as built-in, if r8152 driver is compiled > as modules. Otherwise, the r8153_ecm would be used, even though the > device is supported by r8152 driver. > > Fixes: c1aedf015ebd ("net/usb/r8153_ecm: support ECM mode for RTL8153") > Reported-by: Marek Szyprowski > Signed-off-by: Hayes Wang Yes, this fixes this issue, although I would prefer a separate Kconfig entry for r8153_ecm with proper dependencies instead of this ifdefs in Makefile. Tested-by: Marek Szyprowski > --- > drivers/net/usb/Makefile | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile > index 99381e6bea78..98f4c100955e 100644 > --- a/drivers/net/usb/Makefile > +++ b/drivers/net/usb/Makefile > @@ -13,7 +13,7 @@ obj-$(CONFIG_USB_LAN78XX) += lan78xx.o > obj-$(CONFIG_USB_NET_AX8817X) += asix.o > asix-y := asix_devices.o asix_common.o ax88172a.o > obj-$(CONFIG_USB_NET_AX88179_178A) += ax88179_178a.o > -obj-$(CONFIG_USB_NET_CDCETHER) += cdc_ether.o r8153_ecm.o > +obj-$(CONFIG_USB_NET_CDCETHER) += cdc_ether.o > obj-$(CONFIG_USB_NET_CDC_EEM) += cdc_eem.o > obj-$(CONFIG_USB_NET_DM9601)+= dm9601.o > obj-$(CONFIG_USB_NET_SR9700)+= sr9700.o > @@ -41,3 +41,11 @@ obj-$(CONFIG_USB_NET_QMI_WWAN) += qmi_wwan.o > obj-$(CONFIG_USB_NET_CDC_MBIM) += cdc_mbim.o > obj-$(CONFIG_USB_NET_CH9200)+= ch9200.o > obj-$(CONFIG_USB_NET_AQC111)+= aqc111.o > + > +ifdef CONFIG_USB_NET_CDCETHER > +ifeq ($(CONFIG_USB_RTL8152), m) > +obj-$(CONFIG_USB_RTL8152)+= r8153_ecm.o > +else > +obj-$(CONFIG_USB_NET_CDCETHER) += r8153_ecm.o > +endif > +endif Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
[PATCH v4 1/5] dt-bindings: PCI: exynos: drop samsung,exynos5440-pcie binding
Exynos5440 SoC support has been dropped since commit 8c83315da1cf ("ARM: dts: exynos: Remove Exynos5440"). Drop the obsolete bindings for exynos5440-pcie. Signed-off-by: Marek Szyprowski Reviewed-by: Rob Herring Reviewed-by: Krzysztof Kozlowski Reviewed-by: Jingoo Han --- .../bindings/pci/samsung,exynos5440-pcie.txt | 58 --- 1 file changed, 58 deletions(-) delete mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt deleted file mode 100644 index 651d957d1051.. --- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt +++ /dev/null @@ -1,58 +0,0 @@ -* Samsung Exynos 5440 PCIe interface - -This PCIe host controller is based on the Synopsys DesignWare PCIe IP -and thus inherits all the common properties defined in designware-pcie.txt. - -Required properties: -- compatible: "samsung,exynos5440-pcie" -- reg: base addresses and lengths of the PCIe controller, -- reg-names : First name should be set to "elbi". - And use the "config" instead of getting the configuration address space - from "ranges". - NOTE: When using the "config" property, reg-names must be set. -- interrupts: A list of interrupt outputs for level interrupt, - pulse interrupt, special interrupt. -- phys: From PHY binding. Phandle for the generic PHY. - Refer to Documentation/devicetree/bindings/phy/samsung-phy.txt - -For other common properties, refer to - Documentation/devicetree/bindings/pci/designware-pcie.txt - -Example: - -SoC-specific DT Entry (with using PHY framework): - - pcie_phy0: pcie-phy@27 { - ... - reg = <0x27 0x1000>, <0x271000 0x40>; - reg-names = "phy", "block"; - ... - }; - - pcie@29 { - compatible = "samsung,exynos5440-pcie", "snps,dw-pcie"; - reg = <0x29 0x1000>, <0x4000 0x1000>; - reg-names = "elbi", "config"; - clocks = < 28>, < 27>; - clock-names = "pcie", "pcie_bus"; - #address-cells = <3>; - #size-cells = <2>; - device_type = "pci"; - phys = <_phy0>; - ranges = <0x8100 0 0 0x60001000 0 0x0001 - 0x8200 0 0x60011000 0x60011000 0 0x1ffef000>; - #interrupt-cells = <1>; - interrupt-map-mask = <0 0 0 0>; - interrupt-map = <0 0 0 0 GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>; - num-lanes = <4>; - }; - -Board-specific DT Entry: - - pcie@29 { - reset-gpio = <_ctrl 5 0>; - }; - - pcie@2a { - reset-gpio = <_ctrl 22 0>; - }; -- 2.17.1
[PATCH v4 4/5] phy: samsung: phy-exynos-pcie: rework driver to support Exynos5433 PCIe PHY
From: Jaehoon Chung Exynos5440 SoC support has been dropped since commit 8c83315da1cf ("ARM: dts: exynos: Remove Exynos5440"). Rework this driver to support PCIe PHY variant found in the Exynos5433 SoCs. Signed-off-by: Jaehoon Chung [mszyprow: reworked the driver to support only Exynos5433 variant, rebased onto current kernel code, rewrote commit message] Signed-off-by: Marek Szyprowski Acked-by: Krzysztof Kozlowski Reviewed-by: Jingoo Han --- drivers/phy/samsung/phy-exynos-pcie.c | 304 ++ 1 file changed, 112 insertions(+), 192 deletions(-) diff --git a/drivers/phy/samsung/phy-exynos-pcie.c b/drivers/phy/samsung/phy-exynos-pcie.c index 7e28b1aea0d1..d91de323dd0e 100644 --- a/drivers/phy/samsung/phy-exynos-pcie.c +++ b/drivers/phy/samsung/phy-exynos-pcie.c @@ -4,70 +4,41 @@ * * Phy provider for PCIe controller on Exynos SoC series * - * Copyright (C) 2017 Samsung Electronics Co., Ltd. + * Copyright (C) 2017-2020 Samsung Electronics Co., Ltd. * Jaehoon Chung */ -#include #include -#include -#include #include -#include -#include #include #include #include #include -/* PCIe Purple registers */ -#define PCIE_PHY_GLOBAL_RESET 0x000 -#define PCIE_PHY_COMMON_RESET 0x004 -#define PCIE_PHY_CMN_REG 0x008 -#define PCIE_PHY_MAC_RESET 0x00c -#define PCIE_PHY_PLL_LOCKED0x010 -#define PCIE_PHY_TRSVREG_RESET 0x020 -#define PCIE_PHY_TRSV_RESET0x024 - -/* PCIe PHY registers */ -#define PCIE_PHY_IMPEDANCE 0x004 -#define PCIE_PHY_PLL_DIV_0 0x008 -#define PCIE_PHY_PLL_BIAS 0x00c -#define PCIE_PHY_DCC_FEEDBACK 0x014 -#define PCIE_PHY_PLL_DIV_1 0x05c -#define PCIE_PHY_COMMON_POWER 0x064 -#define PCIE_PHY_COMMON_PD_CMN BIT(3) -#define PCIE_PHY_TRSV0_EMP_LVL 0x084 -#define PCIE_PHY_TRSV0_DRV_LVL 0x088 -#define PCIE_PHY_TRSV0_RXCDR 0x0ac -#define PCIE_PHY_TRSV0_POWER 0x0c4 -#define PCIE_PHY_TRSV0_PD_TSV BIT(7) -#define PCIE_PHY_TRSV0_LVCC0x0dc -#define PCIE_PHY_TRSV1_EMP_LVL 0x144 -#define PCIE_PHY_TRSV1_RXCDR 0x16c -#define PCIE_PHY_TRSV1_POWER 0x184 -#define PCIE_PHY_TRSV1_PD_TSV BIT(7) -#define PCIE_PHY_TRSV1_LVCC0x19c -#define PCIE_PHY_TRSV2_EMP_LVL 0x204 -#define PCIE_PHY_TRSV2_RXCDR 0x22c -#define PCIE_PHY_TRSV2_POWER 0x244 -#define PCIE_PHY_TRSV2_PD_TSV BIT(7) -#define PCIE_PHY_TRSV2_LVCC0x25c -#define PCIE_PHY_TRSV3_EMP_LVL 0x2c4 -#define PCIE_PHY_TRSV3_RXCDR 0x2ec -#define PCIE_PHY_TRSV3_POWER 0x304 -#define PCIE_PHY_TRSV3_PD_TSV BIT(7) -#define PCIE_PHY_TRSV3_LVCC0x31c - -struct exynos_pcie_phy_data { - const struct phy_ops*ops; -}; +#define PCIE_PHY_OFFSET(x) ((x) * 0x4) + +/* Sysreg FSYS register offsets and bits for Exynos5433 */ +#define PCIE_EXYNOS5433_PHY_MAC_RESET 0x0208 +#define PCIE_MAC_RESET_MASK0xFF +#define PCIE_MAC_RESET BIT(4) +#define PCIE_EXYNOS5433_PHY_L1SUB_CM_CON 0x1010 +#define PCIE_REFCLK_GATING_EN BIT(0) +#define PCIE_EXYNOS5433_PHY_COMMON_RESET 0x1020 +#define PCIE_PHY_RESET BIT(0) +#define PCIE_EXYNOS5433_PHY_GLOBAL_RESET 0x1040 +#define PCIE_GLOBAL_RESET BIT(0) +#define PCIE_REFCLKBIT(1) +#define PCIE_REFCLK_MASK 0x16 +#define PCIE_APP_REQ_EXIT_L1_MODE BIT(5) + +/* PMU PCIE PHY isolation control */ +#define EXYNOS5433_PMU_PCIE_PHY_OFFSET 0x730 /* For Exynos pcie phy */ struct exynos_pcie_phy { - const struct exynos_pcie_phy_data *drv_data; - void __iomem *phy_base; - void __iomem *blk_base; /* For exynos5440 */ + void __iomem *base; + struct regmap *pmureg; + struct regmap *fsysreg; }; static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset) @@ -75,153 +46,103 @@ static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset) writel(val, base + offset); } -static u32 exynos_pcie_phy_readl(void __iomem *base, u32 offset) -{ - return readl(base + offset); -} - -/* For Exynos5440 specific functions */ -static int exynos5440_pcie_phy_init(struct phy *phy) +/* Exynos5433 specific functions */ +static int exynos5433_pcie_phy_init(struct phy *phy) { struct exynos_pcie_phy *ep = phy_get_drvdata(phy); - /* DCC feedback control off */ - exynos_pcie_phy_writel(ep->phy_base, 0x29, PCIE_PHY_DCC_FEEDBACK); - - /* set TX/RX impedance */ - exynos_pcie_phy_writel(ep->phy_base, 0xd5, PCIE_PHY_IMPEDANCE); - - /* set 50Mhz PHY clock */ - exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_PLL_DIV_0); - exynos
[PATCH v4 2/5] dt-bindings: PCI: exynos: add the samsung,exynos-pcie binding
Add dt-bindings for the Samsung Exynos PCIe controller (Exynos5433 variant). Based on the text dt-binding posted by Jaehoon Chung. Signed-off-by: Marek Szyprowski Reviewed-by: Krzysztof Kozlowski Reviewed-by: Rob Herring --- .../bindings/pci/samsung,exynos-pcie.yaml | 119 ++ 1 file changed, 119 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml new file mode 100644 index ..1810bf722350 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml @@ -0,0 +1,119 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pci/samsung,exynos-pcie.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Samsung SoC series PCIe Host Controller Device Tree Bindings + +maintainers: + - Marek Szyprowski + - Jaehoon Chung + +description: |+ + Exynos5433 SoC PCIe host controller is based on the Synopsys DesignWare + PCIe IP and thus inherits all the common properties defined in + designware-pcie.txt. + +allOf: + - $ref: /schemas/pci/pci-bus.yaml# + +properties: + compatible: +const: samsung,exynos5433-pcie + + reg: +items: + - description: Data Bus Interface (DBI) registers. + - description: External Local Bus interface (ELBI) registers. + - description: PCIe configuration space region. + + reg-names: +items: + - const: dbi + - const: elbi + - const: config + + interrupts: +maxItems: 1 + + clocks: +items: + - description: PCIe bridge clock + - description: PCIe bus clock + + clock-names: +items: + - const: pcie + - const: pcie_bus + + phys: +maxItems: 1 + + vdd10-supply: +description: + Phandle to a regulator that provides 1.0V power to the PCIe block. + + vdd18-supply: +description: + Phandle to a regulator that provides 1.8V power to the PCIe block. + + num-lanes: +const: 1 + + num-viewport: +const: 3 + +required: + - reg + - reg-names + - interrupts + - "#address-cells" + - "#size-cells" + - "#interrupt-cells" + - interrupt-map + - interrupt-map-mask + - ranges + - bus-range + - device_type + - num-lanes + - num-viewport + - clocks + - clock-names + - phys + - vdd10-supply + - vdd18-supply + +unevaluatedProperties: false + +examples: + - | +#include +#include +#include + +pcie: pcie@1570 { +compatible = "samsung,exynos5433-pcie"; +reg = <0x1570 0x1000>, <0x156b 0x1000>, <0x0c00 0x1000>; +reg-names = "dbi", "elbi", "config"; +#address-cells = <3>; +#size-cells = <2>; +#interrupt-cells = <1>; +device_type = "pci"; +interrupts = ; +clocks = <_fsys CLK_PCIE>, <_fsys CLK_PCLK_PCIE_PHY>; +clock-names = "pcie", "pcie_bus"; +phys = <_phy>; +pinctrl-0 = <_bus _wlanen>; +pinctrl-names = "default"; +num-lanes = <1>; +num-viewport = <3>; +bus-range = <0x00 0xff>; +ranges = <0x8100 0 0 0x0c001000 0 0x0001>, + <0x8200 0 0x0c011000 0x0c011000 0 0x03feefff>; +vdd10-supply = <_reg>; +vdd18-supply = <_reg>; +interrupt-map-mask = <0 0 0 0>; +interrupt-map = <0 0 0 0 GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>; +}; +... -- 2.17.1
[PATCH v4 3/5] dt-bindings: phy: exynos: add the samsung,exynos-pcie-phy binding
Add dt-bindings for the Samsung Exynos PCIe PHY controller (Exynos5433 variant). Based on the text dt-binding posted by Jaehoon Chung. Signed-off-by: Marek Szyprowski Reviewed-by: Krzysztof Kozlowski Reviewed-by: Rob Herring --- .../bindings/phy/samsung,exynos-pcie-phy.yaml | 51 +++ 1 file changed, 51 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml new file mode 100644 index ..ac0af40be52d --- /dev/null +++ b/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml @@ -0,0 +1,51 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/phy/samsung,exynos-pcie-phy.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Samsung SoC series PCIe PHY Device Tree Bindings + +maintainers: + - Marek Szyprowski + - Jaehoon Chung + +properties: + "#phy-cells": +const: 0 + + compatible: +const: samsung,exynos5433-pcie-phy + + reg: +maxItems: 1 + + samsung,pmu-syscon: +$ref: '/schemas/types.yaml#/definitions/phandle' +description: phandle for PMU system controller interface, used to + control PMU registers bits for PCIe PHY + + samsung,fsys-sysreg: +$ref: '/schemas/types.yaml#/definitions/phandle' +description: phandle for FSYS sysreg interface, used to control + sysreg registers bits for PCIe PHY + +required: + - "#phy-cells" + - compatible + - reg + - samsung,pmu-syscon + - samsung,fsys-sysreg + +additionalProperties: false + +examples: + - | +pcie_phy: pcie-phy@1568 { +compatible = "samsung,exynos5433-pcie-phy"; +reg = <0x1568 0x1000>; +samsung,pmu-syscon = <_system_controller>; +samsung,fsys-sysreg = <_fsys>; +#phy-cells = <0>; +}; +... -- 2.17.1
[PATCH v4 5/5] PCI: dwc: exynos: Rework the driver to support Exynos5433 variant
From: Jaehoon Chung Exynos5440 SoC support has been dropped since commit 8c83315da1cf ("ARM: dts: exynos: Remove Exynos5440"). Rework this driver to support DWC PCIe variant found in the Exynos5433 SoCs. The main difference in Exynos5433 variant is lack of the MSI support (the MSI interrupt is not even routed to the CPU). Signed-off-by: Jaehoon Chung [mszyprow: reworked the driver to support only Exynos5433 variant, simplified code, rebased onto current kernel code, added regulator support, converted to the regular platform driver, removed MSI related code, rewrote commit message, added help] Signed-off-by: Marek Szyprowski Acked-by: Krzysztof Kozlowski Acked-by: Jingoo Han Reviewed-by: Rob Herring --- drivers/pci/controller/dwc/Kconfig | 10 +- drivers/pci/controller/dwc/pci-exynos.c | 353 ++-- drivers/pci/quirks.c| 1 + 3 files changed, 147 insertions(+), 217 deletions(-) diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig index bc049865f8e0..b0d41a80edfc 100644 --- a/drivers/pci/controller/dwc/Kconfig +++ b/drivers/pci/controller/dwc/Kconfig @@ -83,10 +83,14 @@ config PCIE_DW_PLAT_EP selected. config PCI_EXYNOS - bool "Samsung Exynos PCIe controller" - depends on SOC_EXYNOS5440 || COMPILE_TEST - depends on PCI_MSI_IRQ_DOMAIN + tristate "Samsung Exynos PCIe controller" + depends on ARCH_EXYNOS || COMPILE_TEST select PCIE_DW_HOST + help + Enables support for the PCIe controller in the Samsung Exynos SoCs + to work in host mode. The PCI controller is based on the DesignWare + hardware and therefore the driver re-uses the DesignWare core + functions to implement the driver. config PCI_IMX6 bool "Freescale i.MX6/7/8 PCIe controller" diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c index 5c10a5432896..c24dab383654 100644 --- a/drivers/pci/controller/dwc/pci-exynos.c +++ b/drivers/pci/controller/dwc/pci-exynos.c @@ -2,26 +2,23 @@ /* * PCIe host controller driver for Samsung Exynos SoCs * - * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Copyright (C) 2013-2020 Samsung Electronics Co., Ltd. * https://www.samsung.com * * Author: Jingoo Han + *Jaehoon Chung */ #include #include -#include #include #include #include #include -#include #include #include #include -#include -#include -#include +#include #include "pcie-designware.h" @@ -37,102 +34,43 @@ #define PCIE_IRQ_SPECIAL 0x008 #define PCIE_IRQ_EN_PULSE 0x00c #define PCIE_IRQ_EN_LEVEL 0x010 -#define IRQ_MSI_ENABLE BIT(2) #define PCIE_IRQ_EN_SPECIAL0x014 -#define PCIE_PWR_RESET 0x018 +#define PCIE_SW_WAKE 0x018 +#define PCIE_BUS_ENBIT(1) #define PCIE_CORE_RESET0x01c #define PCIE_CORE_RESET_ENABLE BIT(0) #define PCIE_STICKY_RESET 0x020 #define PCIE_NONSTICKY_RESET 0x024 #define PCIE_APP_INIT_RESET0x028 #define PCIE_APP_LTSSM_ENABLE 0x02c -#define PCIE_ELBI_RDLH_LINKUP 0x064 +#define PCIE_ELBI_RDLH_LINKUP 0x074 +#define PCIE_ELBI_XMLH_LINKUP BIT(4) #define PCIE_ELBI_LTSSM_ENABLE 0x1 #define PCIE_ELBI_SLV_AWMISC 0x11c #define PCIE_ELBI_SLV_ARMISC 0x120 #define PCIE_ELBI_SLV_DBI_ENABLE BIT(21) -struct exynos_pcie_mem_res { - void __iomem *elbi_base; /* DT 0th resource: PCIe CTRL */ -}; - -struct exynos_pcie_clk_res { - struct clk *clk; - struct clk *bus_clk; -}; - struct exynos_pcie { - struct dw_pcie *pci; - struct exynos_pcie_mem_res *mem_res; - struct exynos_pcie_clk_res *clk_res; - const struct exynos_pcie_ops*ops; - int reset_gpio; - + struct dw_pcie pci; + void __iomem*elbi_base; + struct clk *clk; + struct clk *bus_clk; struct phy *phy; + struct regulator_bulk_data supplies[2]; }; -struct exynos_pcie_ops { - int (*get_mem_resources)(struct platform_device *pdev, - struct exynos_pcie *ep); - int (*get_clk_resources)(struct exynos_pcie *ep); - int (*init_clk_resources)(struct exynos_pcie *ep); - void (*deinit_clk_resources)(struct exynos_pcie *ep); -}; - -static int exynos5440_pcie_get_mem_resources(struct platform_device *pdev, -struct exynos_pcie *ep) -{ - struct dw_pcie *pci = ep->pci; - struct device *dev = pci->dev; - - ep->m
[PATCH v4 0/5] Add DW PCIe support for Exynos5433 SoCs
Dear All, This patchset is a resurrection of the DW PCIe support for the Exynos5433 SoCs posted long time ago here: https://lkml.org/lkml/2016/12/26/6 and later here: https://lkml.org/lkml/2017/12/21/296 . In meantime the support for the Exynos5440 SoCs has been completely dropped from mainline kernel, as those SoCs never reached the market. The PCIe driver for Exynos5440 variant however has not been removed yet. This patchset simply reworks it to support the Exynos5433 variant. The lack of the need to support both variants significantly simplifies the driver code. This patchset is based on the commit b90c53b06597 ("PCI: dwc: Detect number of iATU windows") available on the following branch: git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git pci-more-dwc-cleanup Best regards, Marek Szyprowski Changelog: v4: - fixed topics to better match the common style of the each subsystem - rebased onto "[PATCH v2 00/16] PCI: dwc: Another round of clean-ups" - collected tags, dropped merged dts patch v3: https://lore.kernel.org/linux-samsung-soc/20201029134017.27400-1-m.szyprow...@samsung.com/ - rebased onto "[00/13] PCI: dwc: Another round of clean-ups" patchset: https://patchwork.kernel.org/project/linux-samsung-soc/cover/20201028204646.356535-1-r...@kernel.org/ - fixed issues pointed by Rob in the driver logic: * removed DBI_RO_WR_EN register poking * made driver a standard module - fixed section mismatch issue - added "num-viewport = <3>" property to dts and bindings to fix warning v2: https://lore.kernel.org/linux-samsung-soc/20201023075744.26200-1-m.szyprow...@samsung.com/ - fixed issues in dt-bindings pointed by Krzysztof and Rob v1: https://lore.kernel.org/linux-samsung-soc/20201019094715.15343-1-m.szyprow...@samsung.com/ - initial version of this resurrected patchset Patch summary: Jaehoon Chung (2): phy: samsung: phy-exynos-pcie: rework driver to support Exynos5433 PCIe PHY PCI: dwc: exynos: Rework the driver to support Exynos5433 variant Marek Szyprowski (3): dt-bindings: PCI: exynos: drop samsung,exynos5440-pcie binding dt-bindings: PCI: exynos: add the samsung,exynos-pcie binding dt-bindings: phy: exynos: add the samsung,exynos-pcie-phy binding .../bindings/pci/samsung,exynos-pcie.yaml | 119 ++ .../bindings/pci/samsung,exynos5440-pcie.txt | 58 --- .../bindings/phy/samsung,exynos-pcie-phy.yaml | 51 +++ drivers/pci/controller/dwc/Kconfig| 10 +- drivers/pci/controller/dwc/pci-exynos.c | 353 +++--- drivers/pci/quirks.c | 1 + drivers/phy/samsung/phy-exynos-pcie.c | 304 ++- 7 files changed, 429 insertions(+), 467 deletions(-) create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml delete mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml -- 2.17.1
Re: [PATCH net-next v2 RESEND] net/usb/r8153_ecm: support ECM mode for RTL8153
Hi Hayes, On 04.11.2020 03:19, Hayes Wang wrote: > Support ECM mode based on cdc_ether with relative mii functions, > when CONFIG_USB_RTL8152 is not set, or the device is not supported > by r8152 driver. > > Both r8152 and r8153_ecm would check the return value of > rtl8152_get_version() in porbe(). If rtl8152_get_version() > return none zero value, the r8152 is used for the device > with vendor mode. Otherwise, the r8153_ecm is used for the > device with ECM mode. > > Signed-off-by: Hayes Wang This patch landed recently in linux-next and breaks ethernet operation on Samsung Exynos5422 Odroid XU4/HC1 boards when kernel is compiled from arm/configs/multi_v7_defconfig. The main problem is that the hardware is bound to r8153_ecm driver, not to the r8152. Manually switching the drivers by "echo 4-1:2.0 >/sys/bus/usb/drivers/r8153_ecm/unbind && echo 4-1:2.0 >/sys/bus/usb/drivers/r8152/bind" fixes ethernet operation. This is because in multi_v7_defconfig r8153_ecm driver is built-in (as it is tied to CONFIG_USB_NET_CDCETHER), while the r8152 driver is compiled as module and loaded when r8153_ecm has already bound. I think that r8153_ecm driver should have a separate Kconfig symbol, which matches the r8152 driver (either both are built-in or both as modules), otherwise those 2 drivers cannot properly detect their cases. > --- > drivers/net/usb/Makefile| 2 +- > drivers/net/usb/r8152.c | 30 +-- > drivers/net/usb/r8153_ecm.c | 162 > include/linux/usb/r8152.h | 37 > 4 files changed, 204 insertions(+), 27 deletions(-) > create mode 100644 drivers/net/usb/r8153_ecm.c > create mode 100644 include/linux/usb/r8152.h > > > ... Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v9 0/5] Exynos: Simple QoS for exynos-bus using interconnect
Hi Sylwester, On 13.11.2020 11:32, Sylwester Nawrocki wrote: > On 13.11.2020 10:07, Chanwoo Choi wrote: >> On 11/13/20 5:48 PM, Georgi Djakov wrote: >>> On 11/12/20 16:09, Sylwester Nawrocki wrote: > [...] >>> Good work Sylwester! Thank you and all the reviewers! What would be the >>> merge >>> path for this patchset? Looks like there is no build dependency between >>> patches. >>> Should i take just patches 2,3 or also patch 1? Chanwoo? >> Hi Georgi, >> >> If you take the patch 2,3, I'll apply patch 1,4 to devfreq.git. >> Hi Sylwester, >> First of all, thanks for your work to finish it for a long time. >> I'm very happy about finishing this work. It is very necessary feature >> for the QoS. Once again, thank for your work. > I would also like to thank everyone for provided feedback! > > As far as building is concerned the patches could be applied in any > order. I think we could also apply the drm/exynos patch in same > merge window. There could be runtime (or git bisect) regression > only in case when INTERCONNECT is enabled and only (or as first) > the dts and drm/exynos patches are applied. > > Hmm, maybe it's better to hold on with the drm patch, INTERCONNECT > is disabled in arch/arm/configs/{multi_v7_defconfig, exynos_defconfig} > but it is enabled in arch/arm64/configs/defconfig. I don't think we need to delay DRM patch. Exynos DRM mixer is not available on ARM64 SoCs, so this won't be an issue. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] mm/highmem: Take kmap_high_get() properly into account
Hi Thomas, On 12.11.2020 11:59, Thomas Gleixner wrote: > kunmap_local() warns when the virtual address to unmap is below > PAGE_OFFSET. This is correct except for the case that the mapping was > obtained via kmap_high_get() because the PKMAP addresses are right below > PAGE_OFFSET. > > Cure it by skipping the WARN_ON() when the unmap was handled by > kunmap_high(). > > Fixes: 298fa1ad5571 ("highmem: Provide generic variant of kmap_atomic*") > Reported-by: vto...@googlemail.com > Signed-off-by: Thomas Gleixner This fixes the issue I've reported here: https://lore.kernel.org/dri-devel/c07bae0c-68dd-2693-948f-00e8a50f3...@samsung.com/ Thanks! Tested-by: Marek Szyprowski > --- > mm/highmem.c | 19 +-- > 1 file changed, 13 insertions(+), 6 deletions(-) > > --- a/mm/highmem.c > +++ b/mm/highmem.c > @@ -426,12 +426,15 @@ static inline void *arch_kmap_local_high > #endif > > /* Unmap a local mapping which was obtained by kmap_high_get() */ > -static inline void kmap_high_unmap_local(unsigned long vaddr) > +static inline bool kmap_high_unmap_local(unsigned long vaddr) > { > #ifdef ARCH_NEEDS_KMAP_HIGH_GET > - if (vaddr >= PKMAP_ADDR(0) && vaddr < PKMAP_ADDR(LAST_PKMAP)) > + if (vaddr >= PKMAP_ADDR(0) && vaddr < PKMAP_ADDR(LAST_PKMAP)) { > kunmap_high(pte_page(pkmap_page_table[PKMAP_NR(vaddr)])); > + return true; > + } > #endif > + return false; > } > > static inline int kmap_local_calc_idx(int idx) > @@ -491,10 +494,14 @@ void kunmap_local_indexed(void *vaddr) > > if (addr < __fix_to_virt(FIX_KMAP_END) || > addr > __fix_to_virt(FIX_KMAP_BEGIN)) { > - WARN_ON_ONCE(addr < PAGE_OFFSET); > - > - /* Handle mappings which were obtained by kmap_high_get() */ > - kmap_high_unmap_local(addr); > + /* > + * Handle mappings which were obtained by kmap_high_get() > + * first as the virtual address of such mappings is below > + * PAGE_OFFSET. Warn for all other addresses which are in > + * the user space part of the virtual address space. > + */ > + if (!kmap_high_unmap_local(addr)) > + WARN_ON_ONCE(addr < PAGE_OFFSET); > return; > } > > > > Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [patch V3 10/37] ARM: highmem: Switch to generic kmap atomic
Hi Thomas, On 03.11.2020 10:27, Thomas Gleixner wrote: > No reason having the same code in every architecture. > > Signed-off-by: Thomas Gleixner > Cc: Russell King > Cc: Arnd Bergmann > Cc: linux-arm-ker...@lists.infradead.org This patch landed in linux-next 20201109 as commit 2a15ba82fa6c ("ARM: highmem: Switch to generic kmap atomic"). However it causes a following warning on my test boards (Samsung Exynos SoC based): Run /sbin/init as init process INIT: version 2.88 booting [ cut here ] WARNING: CPU: 3 PID: 120 at mm/highmem.c:502 kunmap_local_indexed+0x194/0x1d0 Modules linked in: CPU: 3 PID: 120 Comm: init Not tainted 5.10.0-rc2-00010-g2a15ba82fa6c #1924 Hardware name: Samsung Exynos (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0xb4/0xd4) [] (dump_stack) from [] (__warn+0x98/0x104) [] (__warn) from [] (warn_slowpath_fmt+0xb0/0xb8) [] (warn_slowpath_fmt) from [] (kunmap_local_indexed+0x194/0x1d0) [] (kunmap_local_indexed) from [] (remove_arg_zero+0xa0/0x158) [] (remove_arg_zero) from [] (load_script+0x250/0x318) [] (load_script) from [] (bprm_execve+0x3d0/0x930) [] (bprm_execve) from [] (do_execveat_common+0x174/0x184) [] (do_execveat_common) from [] (sys_execve+0x30/0x38) [] (sys_execve) from [] (ret_fast_syscall+0x0/0x28) Exception stack(0xc4561fa8 to 0xc4561ff0) 1fa0: b6f2bab8 bef7dac4 bef7dac4 bef7d8fc 004b9b58 bef7dac8 1fc0: b6f2bab8 bef7dac4 bef7d8fc 000b 004b8000 004bac44 bef7da3c bef7d8dc 1fe0: 002f bef7d89c b6d6dc74 b6d6d65c irq event stamp: 1283 hardirqs last enabled at (1293): [] console_unlock+0x430/0x6b0 hardirqs last disabled at (1302): [] console_unlock+0x428/0x6b0 softirqs last enabled at (1282): [] __do_softirq+0x528/0x674 softirqs last disabled at (1269): [] irq_exit+0x1dc/0x1e8 ---[ end trace 6f32a2fb4294655f ]--- I can do more tests to help fixing this issue. Just let me know what to do. ... Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 1/2] cpufreq: dt: Don't (ab)use dev_pm_opp_get_opp_table() to create OPP table
Hi Viresh, On 10.11.2020 07:00, Viresh Kumar wrote: > On 09-11-20, 13:42, Marek Szyprowski wrote: >> This patch landed in linux next-20201109 as commit e8f7703f8fe5 >> ("cpufreq: dt: Don't (ab)use dev_pm_opp_get_opp_table() to create OPP >> table"). Sadly it causes regression on some Samsung Exynos based boards: >> >> 8<--- cut here --- >> Unable to handle kernel paging request at virtual address ff37 >> pgd = (ptrval) >> [ff37] *pgd=4841, *pte=, *ppte= >> Internal error: Oops: 27 [#1] PREEMPT SMP ARM >> Modules linked in: >> usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 >> CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.10.0-rc1-7-ge8f7703f8fe5 >> #1908 >> Hardware name: Samsung Exynos (Flattened Device Tree) >> PC is at dev_pm_opp_put_regulators+0x8/0xf0 >> LR is at dt_cpufreq_probe+0x19c/0x3fc > Does this fix it for you ? Yes, thanks! Reported-by: Marek Szyprowski Tested-by: Marek Szyprowski > diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c > index 66b3db5efb53..5aa3d4e3140d 100644 > --- a/drivers/cpufreq/cpufreq-dt.c > +++ b/drivers/cpufreq/cpufreq-dt.c > @@ -228,7 +228,7 @@ static int dt_cpufreq_early_init(struct device *dev, int > cpu) > if (ret != -EPROBE_DEFER) > dev_err(cpu_dev, "failed to set regulators: > %d\n", > ret); > - goto out; > + goto free_cpumask; > } > } > > @@ -293,6 +293,7 @@ static int dt_cpufreq_early_init(struct device *dev, int > cpu) > dev_pm_opp_of_cpumask_remove_table(priv->cpus); > if (priv->opp_table) > dev_pm_opp_put_regulators(priv->opp_table); > +free_cpumask: > free_cpumask_var(priv->cpus); > return ret; > } > > Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH 1/2] cpufreq: dt: Don't (ab)use dev_pm_opp_get_opp_table() to create OPP table
Hi Viresh, On 06.11.2020 07:24, Viresh Kumar wrote: > Initially, the helper dev_pm_opp_get_opp_table() was supposed to be used > only for the OPP core's internal use (it tries to find an existing OPP > table and if it doesn't find one, then it allocates the OPP table). > > Sometime back, the cpufreq-dt driver started using it to make sure all > the relevant resources required by the OPP core are available earlier > during initialization process to properly propagate -EPROBE_DEFER. > > It worked but it also abused the API to create an OPP table, which > should be created with the help of other helpers provided by the OPP > core. > > The OPP core will be updated in a later commit to limit the scope of > dev_pm_opp_get_opp_table() to only finding an existing OPP table and not > create one. This commit updates the cpufreq-dt driver before that > happens. > > Now the cpufreq-dt driver creates the OPP and cpufreq tables for all the > CPUs from driver's init callback itself. > > Cc: Stephan Gerhold > Signed-off-by: Viresh Kumar This patch landed in linux next-20201109 as commit e8f7703f8fe5 ("cpufreq: dt: Don't (ab)use dev_pm_opp_get_opp_table() to create OPP table"). Sadly it causes regression on some Samsung Exynos based boards: 8<--- cut here --- Unable to handle kernel paging request at virtual address ff37 pgd = (ptrval) [ff37] *pgd=4841, *pte=, *ppte= Internal error: Oops: 27 [#1] PREEMPT SMP ARM Modules linked in: usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 CPU: 3 PID: 1 Comm: swapper/0 Not tainted 5.10.0-rc1-7-ge8f7703f8fe5 #1908 Hardware name: Samsung Exynos (Flattened Device Tree) PC is at dev_pm_opp_put_regulators+0x8/0xf0 LR is at dt_cpufreq_probe+0x19c/0x3fc pc : [] lr : [] psr: a013 ... Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) Stack: (0xc1d29da8 to 0xc1d2a000) ... [] (dev_pm_opp_put_regulators) from [] (dt_cpufreq_probe+0x19c/0x3fc) [] (dt_cpufreq_probe) from [] (platform_drv_probe+0x6c/0xa4) [] (platform_drv_probe) from [] (really_probe+0x200/0x4fc) [] (really_probe) from [] (driver_probe_device+0x78/0x1fc) [] (driver_probe_device) from [] (device_driver_attach+0x58/0x60) [] (device_driver_attach) from [] (__driver_attach+0xdc/0x174) [] (__driver_attach) from [] (bus_for_each_dev+0x68/0xb4) [] (bus_for_each_dev) from [] (bus_add_driver+0x158/0x214) [] (bus_add_driver) from [] (driver_register+0x78/0x110) [] (driver_register) from [] (do_one_initcall+0x8c/0x42c) [] (do_one_initcall) from [] (kernel_init_freeable+0x190/0x1dc) [] (kernel_init_freeable) from [] (kernel_init+0x8/0x118) [] (kernel_init) from [] (ret_from_fork+0x14/0x20) Exception stack(0xc1d29fb0 to 0xc1d29ff8) ... ---[ end trace 5879c43bc748d0d3 ]--- Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b CPU0: stopping Reverting this patch and d44aca126b03 ("cpufreq: dt: dev_pm_opp_put_regulators() accepts NULL argument"), which depends on it, fixes the panic on current linux-next. > --- > drivers/cpufreq/cpufreq-dt.c | 158 +++---- > 1 file changed, 68 insertions(+), 90 deletions(-) > > ... Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] PM / devfreq: passive: Update frequency when start governor
Hi Chanwoo, On 03.11.2020 08:06, Chanwoo Choi wrote: > If the parent device changes the their frequency before registering > the passive device, the passive device cannot receive the notification > from parent device and then the passive device cannot be able to > set the proper frequency according to the frequency of parent device. > > So, when start the passive governor, update the frequency > according to the frequency of parent device. > > Signed-off-by: Chanwoo Choi This patch landed in linux next-20201109 as commit 9b38a4f6b664 ("PM / devfreq: passive: Update frequency when start governor"). Sadly it causes the following warning on various Exynos boards with default exynos_defconfig build: exynos-bus: new bus device registered: soc:bus_dmc ( 5 KHz ~ 40 KHz) exynos-bus: new bus device registered: soc:bus_leftbus ( 5 KHz ~ 20 KHz) [ cut here ] WARNING: CPU: 1 PID: 22 at drivers/devfreq/devfreq.c:411 devfreq_update_target+0xdc/0xec Modules linked in: CPU: 1 PID: 22 Comm: kworker/1:1 Not tainted 5.10.0-rc2-next-20201109 #1898 Hardware name: Samsung Exynos (Flattened Device Tree) Workqueue: events deferred_probe_work_func [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0xb4/0xd4) [] (dump_stack) from [] (__warn+0xd8/0x11c) [] (__warn) from [] (warn_slowpath_fmt+0xb0/0xb8) [] (warn_slowpath_fmt) from [] (devfreq_update_target+0xdc/0xec) [] (devfreq_update_target) from [] (devfreq_passive_event_handler+0x80/0xc4) [] (devfreq_passive_event_handler) from [] (devfreq_add_device+0x4f8/0x5a0) [] (devfreq_add_device) from [] (devm_devfreq_add_device+0x48/0x84) [] (devm_devfreq_add_device) from [] (exynos_bus_probe+0x2ac/0x5c8) [] (exynos_bus_probe) from [] (platform_drv_probe+0x6c/0xa4) [] (platform_drv_probe) from [] (really_probe+0x200/0x4fc) [] (really_probe) from [] (driver_probe_device+0x78/0x1fc) [] (driver_probe_device) from [] (bus_for_each_drv+0x74/0xb8) [] (bus_for_each_drv) from [] (__device_attach+0xe4/0x17c) [] (__device_attach) from [] (bus_probe_device+0x88/0x90) [] (bus_probe_device) from [] (deferred_probe_work_func+0x3c/0xd0) [] (deferred_probe_work_func) from [] (process_one_work+0x234/0x7e4) [] (process_one_work) from [] (worker_thread+0x44/0x51c) [] (worker_thread) from [] (kthread+0x158/0x1a0) [] (kthread) from [] (ret_from_fork+0x14/0x38) Exception stack(0xc1e27fb0 to 0xc1e27ff8) 7fa0: 7fc0: 7fe0: 0013 irq event stamp: 12147 hardirqs last enabled at (12171): [] console_unlock+0x430/0x6b0 dwc2 1248.hsotg: new device is high-speed hardirqs last disabled at (12176): [] console_unlock+0x428/0x6b0 softirqs last enabled at (12168): [] __do_softirq+0x528/0x674 softirqs last disabled at (12155): [] irq_exit+0x1dc/0x1e8 ---[ end trace 89c1007ec29b9250 ]--- Reverting it on top of the linux-next fixes the warning, but I assume that the proper fix would require locking changes during the devfreq registration. > --- > drivers/devfreq/governor_passive.c | 15 +++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/devfreq/governor_passive.c > b/drivers/devfreq/governor_passive.c > index 63332e4a65ae..375aa636027c 100644 > --- a/drivers/devfreq/governor_passive.c > +++ b/drivers/devfreq/governor_passive.c > @@ -141,6 +141,21 @@ static int devfreq_passive_event_handler(struct devfreq > *devfreq, > if (!p_data->this) > p_data->this = devfreq; > > + /* > + * If the parent device changes the their frequency before > + * registering the passive device, the passive device cannot > + * receive the notification from parent device and then the > + * passive device cannot be able to set the proper frequency > + * according to the frequency of parent device. > + * > + * When start the passive governor, update the frequency > + * according to the frequency of parent device. > + */ > + ret = devfreq_update_target(devfreq, parent->previous_freq); > + if (ret < 0) > + dev_warn(>dev, > + "failed to update devfreq using passive governor\n"); > + > nb->notifier_call = devfreq_passive_notifier_call; > ret = devfreq_register_notifier(parent, nb, > DEVFREQ_TRANSITION_NOTIFIER); Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
[PATCH] arm64: dts: exynos: Use fixed index for the MMC devices
Recently introduced asynchronous probe on the MMC devices can shuffle block IDs in the system. Pin them to values equal to the physical MMC bus number to ease booting in environments where UUIDs are not practical. Use newly introduced aliases for mmcblk devices from commit fa2d0aa96941 ("mmc: core: Allow setting slot index via device tree alias"). Suggested-by: Markus Reichl Signed-off-by: Marek Szyprowski --- arch/arm64/boot/dts/exynos/exynos5433.dtsi | 6 ++ arch/arm64/boot/dts/exynos/exynos7.dtsi| 3 +++ 2 files changed, 9 insertions(+) diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi index 0a886bb6c806..3a37ad97fcdb 100644 --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi @@ -23,6 +23,12 @@ interrupt-parent = <>; + aliases { + mmc0 = _0; + mmc1 = _1; + mmc2 = _2; + }; + arm_a53_pmu { compatible = "arm,cortex-a53-pmu"; interrupts = , diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi b/arch/arm64/boot/dts/exynos/exynos7.dtsi index 48cd3a04fd07..3e319ec64997 100644 --- a/arch/arm64/boot/dts/exynos/exynos7.dtsi +++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi @@ -16,6 +16,9 @@ #size-cells = <2>; aliases { + mmc0 = _0; + mmc1 = _1; + mmc2 = _2; pinctrl0 = _alive; pinctrl1 = _bus0; pinctrl2 = _nfc; -- 2.17.1
[PATCH] ARM: dts: exynos: Use fixed index for the MMC devices
Recently introduced asynchronous probe on the MMC devices can shuffle block IDs in the system. Pin them to values equal to the physical MMC bus number to ease booting in environments where UUIDs are not practical. Use newly introduced aliases for mmcblk devices from commit fa2d0aa96941 ("mmc: core: Allow setting slot index via device tree alias"). Suggested-by: Markus Reichl Signed-off-by: Marek Szyprowski --- arch/arm/boot/dts/exynos3250.dtsi | 3 +++ arch/arm/boot/dts/exynos4.dtsi| 2 ++ arch/arm/boot/dts/exynos4210.dtsi | 1 + arch/arm/boot/dts/exynos4412.dtsi | 1 + arch/arm/boot/dts/exynos5250.dtsi | 4 arch/arm/boot/dts/exynos5260.dtsi | 3 +++ arch/arm/boot/dts/exynos5410.dtsi | 3 +++ arch/arm/boot/dts/exynos5420.dtsi | 3 +++ 8 files changed, 20 insertions(+) diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi index 75ed82600ec8..510080bb4102 100644 --- a/arch/arm/boot/dts/exynos3250.dtsi +++ b/arch/arm/boot/dts/exynos3250.dtsi @@ -28,6 +28,9 @@ aliases { pinctrl0 = _0; pinctrl1 = _1; + mmc0 = _0; + mmc1 = _1; + mmc2 = _2; mshc0 = _0; mshc1 = _1; mshc2 = _2; diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi index a1e54449f33f..e266f890eea4 100644 --- a/arch/arm/boot/dts/exynos4.dtsi +++ b/arch/arm/boot/dts/exynos4.dtsi @@ -45,6 +45,8 @@ fimc1 = _1; fimc2 = _2; fimc3 = _3; + mmc1 = _1; + mmc2 = _2; serial0 = _0; serial1 = _1; serial2 = _2; diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi index fddc661ded28..f1d0d5959b7f 100644 --- a/arch/arm/boot/dts/exynos4210.dtsi +++ b/arch/arm/boot/dts/exynos4210.dtsi @@ -23,6 +23,7 @@ compatible = "samsung,exynos4210", "samsung,exynos4"; aliases { + mmc0 = _0; pinctrl0 = _0; pinctrl1 = _1; pinctrl2 = _2; diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi index fa8e8d6bc4d5..9fcf7383eb9d 100644 --- a/arch/arm/boot/dts/exynos4412.dtsi +++ b/arch/arm/boot/dts/exynos4412.dtsi @@ -28,6 +28,7 @@ pinctrl3 = _3; fimc-lite0 = _lite_0; fimc-lite1 = _lite_1; + mmc0 = _0; mshc0 = _0; }; diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index 84677332a5a2..0a0436f92fac 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -30,6 +30,10 @@ gsc1 = _1; gsc2 = _2; gsc3 = _3; + mmc0 = _0; + mmc1 = _1; + mmc2 = _2; + mmc3 = _3; mshc0 = _0; mshc1 = _1; mshc2 = _2; diff --git a/arch/arm/boot/dts/exynos5260.dtsi b/arch/arm/boot/dts/exynos5260.dtsi index 973448c4ad93..64bf1d8dc33b 100644 --- a/arch/arm/boot/dts/exynos5260.dtsi +++ b/arch/arm/boot/dts/exynos5260.dtsi @@ -21,6 +21,9 @@ i2c1 = _1; i2c2 = _2; i2c3 = _3; + mmc0 = _0; + mmc1 = _1; + mmc2 = _2; pinctrl0 = _0; pinctrl1 = _1; pinctrl2 = _2; diff --git a/arch/arm/boot/dts/exynos5410.dtsi b/arch/arm/boot/dts/exynos5410.dtsi index 584ce62361b1..503859153769 100644 --- a/arch/arm/boot/dts/exynos5410.dtsi +++ b/arch/arm/boot/dts/exynos5410.dtsi @@ -24,6 +24,9 @@ pinctrl1 = _1; pinctrl2 = _2; pinctrl3 = _3; + mmc0 = _0; + mmc1 = _1; + mmc2 = _2; }; cpus { diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi index 23a8fd5c8a6e..3a3eadd890fb 100644 --- a/arch/arm/boot/dts/exynos5420.dtsi +++ b/arch/arm/boot/dts/exynos5420.dtsi @@ -19,6 +19,9 @@ compatible = "samsung,exynos5420", "samsung,exynos5"; aliases { + mmc0 = _0; + mmc1 = _1; + mmc2 = _2; mshc0 = _0; mshc1 = _1; mshc2 = _2; -- 2.17.1
Re: [PATCH v3 2/6] dt-bindings: pci: add the samsung,exynos-pcie binding
Hi Rob, On 04.11.2020 22:35, Rob Herring wrote: > On Thu, Oct 29, 2020 at 02:40:13PM +0100, Marek Szyprowski wrote: >> Add dt-bindings for the Samsung Exynos PCIe controller (Exynos5433 >> variant). Based on the text dt-binding posted by Jaehoon Chung. >> >> Signed-off-by: Marek Szyprowski >> Reviewed-by: Krzysztof Kozlowski >> --- >> .../bindings/pci/samsung,exynos-pcie.yaml | 119 ++ >> 1 file changed, 119 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/pci/samsung,exynos-pcie.yaml >> ... >> + num-viewport: >> +const: 3 > I'm confused why you need this. This is only used with the iATU except > for keystone. Platforms like Exynos with their own child bus config > space accessors don't have an iATU. Frankly I have no idea, I don't know much about the PCI internals. After rebasing onto your latest DW PCI changes I've noticed a following warning message: exynos-pcie 1570.pcie: Resources exceed number of ATU entries (2) Here is a complete log: # dmesg | grep pci ehci-pci: EHCI PCI platform driver ohci-pci: OHCI PCI platform driver exynos-pcie 1570.pcie: host bridge /soc@0/pcie@1570 ranges: exynos-pcie 1570.pcie: IO 0x000c001000..0x000c010fff -> 0x00 exynos-pcie 1570.pcie: MEM 0x000c011000..0x000ffe -> 0x000c011000 exynos-pcie 1570.pcie: Resources exceed number of ATU entries (2) exynos-pcie 1570.pcie: Link up exynos-pcie 1570.pcie: PCI host bridge to bus :00 pci_bus :00: root bus resource [bus 00-ff] pci_bus :00: root bus resource [io 0x-0x] pci_bus :00: root bus resource [mem 0x0c011000-0x0ffe] pci :00:00.0: [144d:a5e3] type 01 class 0x060400 pci :00:00.0: PME# supported from D0 D3hot D3cold pci :01:00.0: [14e4:43e9] type 00 class 0x028000 pci :01:00.0: reg 0x10: [mem 0x-0x7fff 64bit] pci :01:00.0: reg 0x18: [mem 0x-0x003f 64bit] pci :01:00.0: supports D1 D2 pci :01:00.0: PME# supported from D0 D1 D2 D3hot D3cold pci :00:00.0: BAR 14: assigned [mem 0x0c20-0x0c7f] pci :01:00.0: BAR 2: assigned [mem 0x0c40-0x0c7f 64bit] pci :01:00.0: BAR 0: assigned [mem 0x0c20-0x0c207fff 64bit] pci :00:00.0: PCI bridge to [bus 01-ff] pci :00:00.0: bridge window [mem 0x0c20-0x0c7f] pci :00:00.0: MSI quirk detected; MSI disabled pcieport :00:00.0: PME: Signaling with IRQ 97 brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac4358-pcie for chip BCM4358/1 When I've increased the numer of viewports it has gone. If this is not the proper solution, I will removed it. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v2] ARM: dts: exynos: Add a placeholder for a MAC address
Hi Anand, On 05.11.2020 09:06, Anand Moon wrote: > On Mon, 2 Nov 2020 at 21:53, Marek Szyprowski > wrote: >> On 01.11.2020 15:07, Anand Moon wrote: >>> On Thu, 1 Oct 2020 at 19:25, Łukasz Stelmach wrote: >>>> Add a placeholder for a MAC address. A bootloader may fill it >>>> to set the MAC address and override EEPROM settings. >>>> >>>> Signed-off-by: Łukasz Stelmach >>>> --- >>>> Changes in v2: >>>>- use local-mac-address and leave mac-address to be added by a >>>> bootloader >>>> >>>>arch/arm/boot/dts/exynos5422-odroidxu3.dts | 18 ++ >>>>1 file changed, 18 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3.dts >>>> b/arch/arm/boot/dts/exynos5422-odroidxu3.dts >>>> index db0bc17a667b..d0f6ac5fa79d 100644 >>>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3.dts >>>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3.dts >>>> @@ -70,3 +70,21 @@ { >>>>_dwc3_1 { >>>> dr_mode = "peripheral"; >>>>}; >>>> + >>>> + { >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + >>>> + hub@1 { >>>> + compatible = "usb8087,0024"; >>>> + reg = <1>; >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + >>>> + ethernet: usbether@1 { >>>> + compatible = "usb0c45,6310"; >>>> + reg = <1>; >>>> + local-mac-address = [00 00 00 00 00 00]; /* Filled >>>> in by a bootloader */ >>>> + }; >>>> + }; >>>> +}; >>>> -- >>>> 2.26.2 >>>> >>> Thanks for this patch, can you share some example on how to set the >>> mac address via u-boot bootargs >> A little bit hacky script to set permanent board unique MAC address: >> >> # setexp.b u0 *0x1014; setexp.b u1 *0x1015; setexp.b u2 >> *0x1016; setexp.b u3 *0x1017; setenv ethaddr >> 0:0:${u0}:${u1}:${u2}:${u3}; setenv usbethaddr ${ethaddr}; >> > OK this command worked for me. > >> Then if there is proper ethernet0 alias set, u-boot will then >> automatically save the configured MAC address to the device tree. I've >> just check this on recent u-boot v2020.10 and Odroid U3 board. >> >> Lukasz will send updated patch soon (with proper alias entry). >> >> If you want to hack setting MAC address manually, this will work with >> the current patch: >> >> # setexp.b u0 *0x1014; setexp.b u1 *0x1015; setexp.b u2 >> *0x1016; setexp.b u3 *0x1017; fdt addr ${fdtaddr}; fdt set >> /soc/usb@1211/hub@1/usbether@1 local-mac-address [ 0 0 ${u0} ${u1} >> ${u2} ${u3} ] >> > So do we need a similar patch for u-boot ? I've not sure that this ethaddr hack/workaround should be added to mainline uboot. Some other exynos based board have proper MAC address stored in EEPROM (for example Odroid XU4/HC1). I would leave it for the users to add it manually if it is really needed for now. > I am getting following error on Odroid U3+ and U-Boot 2020.10 > > Odroid # setexp.b u0 *0x1014; setexp.b u1 *0x1015; setexp.b > u2 *0x1016; setexp.b u3 *0x1017; fdt addr ${fdtaddr}; fdt set > /soc/usb@1211/hub@1/usbether@1 local-mac-address [ 0 0 ${u0} ${u1} > ${u2} ${u3} ] > No FDT memory address configured. Please configure > the FDT address via "fdt addr " command. > Aborting! > > Also added these command to boot.scr but still observing the failure You need to use proper env for setting fdt address (the "fdt addr ${fdtaddr}" command). For some versions it was ${fdt_addr}, the other used ${fdtaddr}. Please check which one is used for loading dtb and adjust the script. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] ARM: dts: exynos: Assign a fixed index to mmc devices on exynos4412 based ODROID boards
On 04.11.2020 14:13, Marek Szyprowski wrote: > On 04.11.2020 14:06, Markus Reichl wrote: >> Am 04.11.20 um 13:25 schrieb Marek Szyprowski: >>> On 04.11.2020 11:25, Markus Reichl wrote: >>>> Recently introduced async probe on mmc devices can shuffle block IDs. >>>> Pin them to fixed values to ease booting in evironments where UUIDs >>>> ar not practical. >>>> Use newly introduced aliases for mmcblk devices from [1]. >>>> >>>> [1] >>>> https://patchwork.kernel.org/patch/11747669/ >>>> >>>> Signed-off-by: Markus Reichl >>>> --- >>>> arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 5 + >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >>>> b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >>>> index a5c1ce1e396c..aa10d5bc7e1c 100644 >>>> --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >>>> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >>>> @@ -13,6 +13,11 @@ >>>> #include "exynos-mfc-reserved-memory.dtsi" >>>> / { >>>> + aliases { >>>> + mmc0 = _2; >>>> + mmc1 = _0; >>> Like in the OdroidXU3-family patch, I would use 0 for the eMMC (mshc_0) >>> and 2 for the SD-card (sdhci_2). >> How to deal then with sdhci_0 (from exynos4.dtsi) vc. mshc_0 (from >> exynos4412.dts)? > sdhci_0 and mshc_0 both operate on the same physical MMC0 bus, so this > is not an issue. They cannot be used simultaneously. The latter is just > faster, the first one has been left there mainly for the software > compatibility. I've thought a bit more on this and I would simply prefer to add generic MMC aliases to the top-level Exynos dtsi files (3250, 4210, 4412, 5250, 5410, 5420) to keep Linux logical MMC bus numbers in sync with the HW bus numbers on all boards. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] ARM: dts: exynos: Assign a fixed index to mmc devices on exynos4412 based ODROID boards
Hi Markus, On 04.11.2020 14:06, Markus Reichl wrote: > Am 04.11.20 um 13:25 schrieb Marek Szyprowski: >> On 04.11.2020 11:25, Markus Reichl wrote: >>> Recently introduced async probe on mmc devices can shuffle block IDs. >>> Pin them to fixed values to ease booting in evironments where UUIDs >>> ar not practical. >>> Use newly introduced aliases for mmcblk devices from [1]. >>> >>> [1] >>> https://patchwork.kernel.org/patch/11747669/ >>> >>> Signed-off-by: Markus Reichl >>> --- >>> arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 5 + >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >>> b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >>> index a5c1ce1e396c..aa10d5bc7e1c 100644 >>> --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >>> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >>> @@ -13,6 +13,11 @@ >>> #include "exynos-mfc-reserved-memory.dtsi" >>> / { >>> + aliases { >>> + mmc0 = _2; >>> + mmc1 = _0; >> >> Like in the OdroidXU3-family patch, I would use 0 for the eMMC (mshc_0) >> and 2 for the SD-card (sdhci_2). > > How to deal then with sdhci_0 (from exynos4.dtsi) vc. mshc_0 (from > exynos4412.dts)? sdhci_0 and mshc_0 both operate on the same physical MMC0 bus, so this is not an issue. They cannot be used simultaneously. The latter is just faster, the first one has been left there mainly for the software compatibility. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] ARM: dts: exynos: Assign a fixed index to mmc devices on ODROID XU3/4 boards
Hi Markus, On 04.11.2020 13:42, Markus Reichl wrote: > Am 04.11.20 um 13:24 schrieb Marek Szyprowski: >> On 04.11.2020 11:08, Markus Reichl wrote: >>> Recently introduced async probe on mmc devices can shuffle block IDs. >>> Pin them to fixed values to ease booting in evironments where UUIDs >>> are not practical. Use newly introduced aliases for mmcblk devices >>> from [1]. >>> >>> [1] >>> https://patchwork.kernel.org/patch/11747669/ >> >> Wow, this is a long standing issue, called by others 'a feature'. Good >> that this has been finally solved. >> >>> Signed-off-by: Markus Reichl >>> --- >>> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 5 + >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> index e35af40a55cb..91d2840ac8ca 100644 >>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> @@ -13,6 +13,11 @@ >>> #include "exynos5422-odroid-core.dtsi" >>> / { >>> + aliases { >>> + mmc0 = _2; >>> + mmc1 = _0; >> >> Frankly, I would keep the MMC numbers the same as in u-boot and >> datasheets. 0 for the build-in eMMC and 2 for the SD-card. This would be >> much more natural. On the other hand, I would agree to do it differently >> only on Odroid HC1/HD2/MC1, which don't have build-in eMMC - just use 0 >> there for the SD-card. > > This would break present and long standing boot ordering in mainline, > which is > mmcblk0 = SD-card and > mmcblk1 = eMMC > > Still desired? Well, previously (before v4.0? I don't remember exactly when there was a first change), the order was exactly opposite. I think that going for the MMC bus numbers from the datasheet/schematic is the most appropriate approach. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] ARM: dts: exynos: Assign a fixed index to mmc devices on exynos4412 based ODROID boards
Hi Markus, On 04.11.2020 11:25, Markus Reichl wrote: > Recently introduced async probe on mmc devices can shuffle block IDs. > Pin them to fixed values to ease booting in evironments where UUIDs ar not > practical. > Use newly introduced aliases for mmcblk devices from [1]. > > [1] > https://patchwork.kernel.org/patch/11747669/ > > Signed-off-by: Markus Reichl > --- > arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > index a5c1ce1e396c..aa10d5bc7e1c 100644 > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi > @@ -13,6 +13,11 @@ > #include "exynos-mfc-reserved-memory.dtsi" > > / { > + aliases { > + mmc0 = _2; > + mmc1 = _0; Like in the OdroidXU3-family patch, I would use 0 for the eMMC (mshc_0) and 2 for the SD-card (sdhci_2). > + }; > + > chosen { > stdout-path = _1; > }; Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] ARM: dts: exynos: Assign a fixed index to mmc devices on ODROID XU3/4 boards
Hi Markus, On 04.11.2020 11:08, Markus Reichl wrote: > Recently introduced async probe on mmc devices can shuffle block IDs. > Pin them to fixed values to ease booting in evironments where UUIDs > are not practical. Use newly introduced aliases for mmcblk devices from [1]. > > [1] > https://patchwork.kernel.org/patch/11747669/ Wow, this is a long standing issue, called by others 'a feature'. Good that this has been finally solved. > Signed-off-by: Markus Reichl > --- > arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi > b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi > index e35af40a55cb..91d2840ac8ca 100644 > --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi > +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi > @@ -13,6 +13,11 @@ > #include "exynos5422-odroid-core.dtsi" > > / { > + aliases { > + mmc0 = _2; > + mmc1 = _0; Frankly, I would keep the MMC numbers the same as in u-boot and datasheets. 0 for the build-in eMMC and 2 for the SD-card. This would be much more natural. On the other hand, I would agree to do it differently only on Odroid HC1/HD2/MC1, which don't have build-in eMMC - just use 0 there for the SD-card. > + }; > + > gpio-keys { > compatible = "gpio-keys"; > pinctrl-names = "default"; Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH v2] ARM: dts: exynos: Add a placeholder for a MAC address
Hi Anand, On 01.11.2020 15:07, Anand Moon wrote: > Hi Lukasz, > > On Thu, 1 Oct 2020 at 19:25, Łukasz Stelmach wrote: >> Add a placeholder for a MAC address. A bootloader may fill it >> to set the MAC address and override EEPROM settings. >> >> Signed-off-by: Łukasz Stelmach >> --- >> Changes in v2: >> - use local-mac-address and leave mac-address to be added by a bootloader >> >> arch/arm/boot/dts/exynos5422-odroidxu3.dts | 18 ++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3.dts >> b/arch/arm/boot/dts/exynos5422-odroidxu3.dts >> index db0bc17a667b..d0f6ac5fa79d 100644 >> --- a/arch/arm/boot/dts/exynos5422-odroidxu3.dts >> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3.dts >> @@ -70,3 +70,21 @@ { >> _dwc3_1 { >> dr_mode = "peripheral"; >> }; >> + >> + { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + hub@1 { >> + compatible = "usb8087,0024"; >> + reg = <1>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + ethernet: usbether@1 { >> + compatible = "usb0c45,6310"; >> + reg = <1>; >> + local-mac-address = [00 00 00 00 00 00]; /* Filled >> in by a bootloader */ >> + }; >> + }; >> +}; >> -- >> 2.26.2 >> > Thanks for this patch, can you share some example on how to set the > mac address via u-boot bootargs A little bit hacky script to set permanent board unique MAC address: # setexp.b u0 *0x1014; setexp.b u1 *0x1015; setexp.b u2 *0x1016; setexp.b u3 *0x1017; setenv ethaddr 0:0:${u0}:${u1}:${u2}:${u3}; setenv usbethaddr ${ethaddr}; Then if there is proper ethernet0 alias set, u-boot will then automatically save the configured MAC address to the device tree. I've just check this on recent u-boot v2020.10 and Odroid U3 board. Lukasz will send updated patch soon (with proper alias entry). If you want to hack setting MAC address manually, this will work with the current patch: # setexp.b u0 *0x1014; setexp.b u1 *0x1015; setexp.b u2 *0x10000016; setexp.b u3 *0x1017; fdt addr ${fdtaddr}; fdt set /soc/usb@1211/hub@1/usbether@1 local-mac-address [ 0 0 ${u0} ${u1} ${u2} ${u3} ] > also can you update this patch for exynos5422-odroidxu3-lite.dts and > exynos4412-odroidu3.dts. Also odroid-x2 and odroid-xu. Lukasz will take care of them. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland