Re: pidfd design
On Tue, Mar 19, 2019 at 8:59 PM Christian Brauner wrote: > > On Tue, Mar 19, 2019 at 07:42:52PM -0700, Daniel Colascione wrote: > > On Tue, Mar 19, 2019 at 6:52 PM Joel Fernandes > > wrote: > > > > > > On Wed, Mar 20, 2019 at 12:10:23AM +0100, Christian Brauner wrote: > > > > On Tue, Mar 19, 2019 at 03:48:32PM -0700, Daniel Colascione wrote: > > > > > On Tue, Mar 19, 2019 at 3:14 PM Christian Brauner > > > > > wrote: > > > > > > So I dislike the idea of allocating new inodes from the procfs super > > > > > > block. I would like to avoid pinning the whole pidfd concept > > > > > > exclusively > > > > > > to proc. The idea is that the pidfd API will be useable through > > > > > > procfs > > > > > > via open("/proc/") because that is what users expect and really > > > > > > wanted to have for a long time. So it makes sense to have this > > > > > > working. > > > > > > But it should really be useable without it. That's why > > > > > > translate_pid() > > > > > > and pidfd_clone() are on the table. What I'm saying is, once the > > > > > > pidfd > > > > > > api is "complete" you should be able to set CONFIG_PROCFS=N - even > > > > > > though that's crazy - and still be able to use pidfds. This is also > > > > > > a > > > > > > point akpm asked about when I did the pidfd_send_signal work. > > > > > > > > > > I agree that you shouldn't need CONFIG_PROCFS=Y to use pidfds. One > > > > > crazy idea that I was discussing with Joel the other day is to just > > > > > make CONFIG_PROCFS=Y mandatory and provide a new get_procfs_root() > > > > > system call that returned, out of thin air and independent of the > > > > > mount table, a procfs root directory file descriptor for the caller's > > > > > PID namspace and suitable for use with openat(2). > > > > > > > > Even if this works I'm pretty sure that Al and a lot of others will not > > > > be happy about this. A syscall to get an fd to /proc? > > > > Why not? procfs provides access to a lot of core kernel functionality. > > Why should you need a mountpoint to get to it? > > > > > That's not going > > > > to happen and I don't see the need for a separate syscall just for that. > > > > We need a system call for the same reason we need a getrandom(2): you > > have to bootstrap somehow when you're in a minimal environment. > > > > > > (I do see the point of making CONFIG_PROCFS=y the default btw.) > > > > I'm not proposing that we make CONFIG_PROCFS=y the default. I'm > > proposing that we *hardwire* it as the default and just declare that > > it's not possible to build a Linux kernel that doesn't include procfs. > > Why do we even have that button? > > > > > I think his point here was that he wanted a handle to procfs no matter > > > where > > > it was mounted and then can later use openat on that. Agreed that it may > > > be > > > unnecessary unless there is a usecase for it, and especially if the /proc > > > directory being the defacto mountpoint for procfs is a universal > > > convention. > > > > If it's a universal convention and, in practice, everyone needs proc > > mounted anyway, so what's the harm in hardwiring CONFIG_PROCFS=y? If > > we advertise /proc as not merely some kind of optional debug interface > > but *the* way certain kernel features are exposed --- and there's > > nothing wrong with that --- then we should give programs access to > > these core kernel features in a way that doesn't depend on userspace > > kernel configuration, and you do that by either providing a > > procfs-root-getting system call or just hardwiring the "/proc/" prefix > > into VFS. > > > > > > Inode allocation from the procfs mount for the file descriptors Joel > > > > wants is not correct. Their not really procfs file descriptors so this > > > > is a nack. We can't just hook into proc that way. > > > > > > I was not particular about using procfs mount for the FDs but that's the > > > only > > > way I knew how to do it until you pointed out anon_inode (my grep skills > > > missed that), so thank you! > > > > > > > > C'mon: /proc is used by everyone today and almost every program breaks > > > > > if it's not around. The string "/proc" is already de facto kernel ABI. > > > > > Let's just drop the pretense of /proc being optional and bake it into > > > > > the kernel proper, then give programs a way to get to /proc that isn't > > > > > tied to any particular mount configuration. This way, we don't need a > > > > > translate_pid(), since callers can just use procfs to do the same > > > > > thing. (That is, if I understand correctly what translate_pid does.) > > > > > > > > I'm not sure what you think translate_pid() is doing since you're not > > > > saying what you think it does. > > > > Examples from the old patchset: > > > > translate_pid(pid, ns, -1) - get pid in our pid namespace > > > > Ah, it's a bit different from what I had in mind. It's fair to want to > > translate PIDs between namespaces, but the only way to make the > > translate_pid under discussion robu
Re: [PATCH v2 03/11] staging: mt7621-mmc: Fix warning when reloading module with debug msgs enabled
On Mon, Mar 18, 2019 at 08:20:04PM -0600, George Hilliard wrote: > The kernel complained: > > [ 510.277151] WARNING: CPU: 0 PID: 395 at fs/proc/generic.c:360 > proc_register+0xf0/0x108 > [ 510.292891] proc_dir_entry '/proc/msdc_debug' already registered > > when doing a modprobe/rmmod/modprobe of this module if debug messages > are compiled in. Fix this by removing the proc entry when the module is > unloaded. > > Signed-off-by: George Hilliard > --- > drivers/staging/mt7621-mmc/dbg.c | 15 --- > drivers/staging/mt7621-mmc/dbg.h | 3 ++- > drivers/staging/mt7621-mmc/sd.c | 4 > 3 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/mt7621-mmc/dbg.c > b/drivers/staging/mt7621-mmc/dbg.c > index fda3ba38ba37..2310f3bcc16e 100644 > --- a/drivers/staging/mt7621-mmc/dbg.c > +++ b/drivers/staging/mt7621-mmc/dbg.c > @@ -294,9 +294,18 @@ static const struct file_operations msdc_debug_fops = { > .release= single_release, > }; > > -void msdc_debug_proc_init(void) > +// Keep ahold of the proc entry we create so it can be freed during > +// module removal > +struct proc_dir_entry *msdc_debug_proc_entry; > + > +void __init msdc_debug_proc_init(void) > { > - proc_create("msdc_debug", 0660, NULL, &msdc_debug_fops); > + msdc_debug_proc_entry = proc_create("msdc_debug", 0660, > + NULL, &msdc_debug_fops); > +} > + > +void __exit msdc_debug_proc_deinit(void) > +{ > + proc_remove(msdc_debug_proc_entry); > } > -EXPORT_SYMBOL_GPL(msdc_debug_proc_init); > #endif > diff --git a/drivers/staging/mt7621-mmc/dbg.h > b/drivers/staging/mt7621-mmc/dbg.h > index 2d447b2d92ae..d100324aa3fe 100644 > --- a/drivers/staging/mt7621-mmc/dbg.h > +++ b/drivers/staging/mt7621-mmc/dbg.h > @@ -93,7 +93,8 @@ enum msdc_dbg { > > extern unsigned int sd_debug_zone[4]; > #define TAG "msdc" > -void msdc_debug_proc_init(void); > +void __init msdc_debug_proc_init(void); > +void __exit msdc_debug_proc_deinit(void); > > u32 msdc_time_calc(u32 old_L32, u32 old_H32, u32 new_L32, u32 new_H32); > void msdc_performance(u32 opcode, u32 sizes, u32 bRx, u32 ticks); > diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c > index 9074848a8251..306b3b46f7c9 100644 > --- a/drivers/staging/mt7621-mmc/sd.c > +++ b/drivers/staging/mt7621-mmc/sd.c > @@ -1841,6 +1841,10 @@ static int __init mt_msdc_init(void) > > static void __exit mt_msdc_exit(void) > { > +#if defined(MT6575_SD_DEBUG) > + msdc_debug_proc_deinit(); > +#endif You shouldn't need a #ifdef in .c code. In the .h file provide an "empty" function for this if the option is not defined. Then you can just call this function in the .c file no matter what. thanks, greg k-h
Re: [PATCH v2 06/11] staging: mt7621-mmc: Initialize completions a single time during probe
On Mon, Mar 18, 2019 at 08:20:07PM -0600, George Hilliard wrote: > The module was initializing completions whenever it was going to wait on > them, and not when the completion was allocated. This is incorrect > according to the completion docs: > > Calling init_completion() on the same completion object twice is > most likely a bug [...] > > Re-initialization is also unnecessary because the module never uses > complete_all(). Fix this by only ever initializing the completion a > single time. > > Signed-off-by: George Hilliard > --- > drivers/staging/mt7621-mmc/sd.c | 13 + > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c > index 89fbc0a1dec7..c272aa780719 100644 > --- a/drivers/staging/mt7621-mmc/sd.c > +++ b/drivers/staging/mt7621-mmc/sd.c > @@ -467,7 +467,9 @@ static unsigned int msdc_command_start(struct msdc_host > *host, > host->cmd = cmd; > host->cmd_rsp = resp; > > - init_completion(&host->cmd_done); > + // The completion should have been consumed by the previous command > + // response handler, because the mmc requests should be serialized > + BUG_ON(completion_done(&host->cmd_done)); Adding new BUG_ON() calls is not ok. That crashes the machine and will break everyone. If this really is a potential error that could happen, then handle it. If not, then do not even put this there, it's not needed. All of the BUG_ON() entries need to be removed in the end from this code anyway. thanks, greg k-h
Re: [PATCH v2 07/11] staging: mt7621-mmc: Check for nonzero number of scatterlist entries
On Mon, Mar 18, 2019 at 08:20:08PM -0600, George Hilliard wrote: > The buffer descriptor setup loop is correct only if it is setting up at > least one bd struct. Besides, there is an error somewhere if > dma_map_sg() returns 0. So add a paranoid check for this condition. > > Signed-off-by: George Hilliard > --- > drivers/staging/mt7621-mmc/sd.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c > index c272aa780719..31c5b44bd29f 100644 > --- a/drivers/staging/mt7621-mmc/sd.c > +++ b/drivers/staging/mt7621-mmc/sd.c > @@ -593,7 +593,12 @@ static void msdc_dma_setup(struct msdc_host *host, > struct msdc_dma *dma, > struct bd *bd; > u32 j; > > - BUG_ON(sglen > MAX_BD_NUM); /* not support currently */ > + // Shouldn't happen; we configure the mmc host layer > + // based on MAX_BD_NUM: > + BUG_ON(sglen > MAX_BD_NUM); > + // Correct setup below requires at least one bd > + // (and dma_map_sg should not return 0): > + BUG_ON(sglen == 0); Same here, no new BUG_ON(). To quote one kernel developer, "BUG_ON() means that the programmer is lazy" :) thanks, greg k-h
Re: [PATCH v2] staging: rtlwifi: Fix potential NULL pointer dereference of kzalloc
On Tue, Mar 19, 2019 at 03:15:08PM -0500, Aditya Pakki wrote: > phydm.internal is allocated using kzalloc which is used multiple > times without a check for NULL pointer. This patch avoids such a > scenario. > > -- > v1: Patch collision with different things, fix as per Greg > Signed-off-by: Aditya Pakki signed-off-by has to be above the -- line. Please fix. thanks, greg k-h
Re: [PATCH v2] staging: rtlwifi: rtl8822b: fix to avoid NULL pointer dereference
On Tue, Mar 19, 2019 at 03:21:25PM -0500, Aditya Pakki wrote: > skb allocated via dev_alloc_skb can fail and return a NULL pointer. > This patch avoids such a scenario and returns, consistent with other > invocations. > > --- > v1: Patch collision with rtl_phydm.c, fix as per Greg > Signed-off-by: Aditya Pakki Same here, has to be above.
Re: [PATCH v2] staging: rtlwifi: rtl8822b: fix to avoid NULL pointer dereference
On Tue, Mar 19, 2019 at 03:21:25PM -0500, Aditya Pakki wrote: > skb allocated via dev_alloc_skb can fail and return a NULL pointer. > This patch avoids such a scenario and returns, consistent with other > invocations. > > --- > v1: Patch collision with rtl_phydm.c, fix as per Greg > Signed-off-by: Aditya Pakki And here as well, please put above ---
Re: [PATCH v2] staging: rtl8188eu: Fix potential NULL pointer dereference
On Tue, Mar 19, 2019 at 04:07:54PM -0500, Aditya Pakki wrote: > hwxmits is allocated via kcalloc and not checked for failure before its > dereference. The patch fixes this problem by returning an error in > rtl8723bs. > > --- > v1: Return error and remove print in case of failure, per Greg > Signed-off-by: Aditya Pakki Please fix.
Re: your mail
On Mon, Mar 18, 2019 at 08:20:01PM -0600, George Hilliard wrote: > Because of this change, the driver now expects a pinctrl device > reference in the mmc controller's device tree node; without it, it will > bail out. This could break existing setups that don't specify it > because it "just worked" up until now. So currently I just let the old > behavior fall away because this is a staging driver. But if this is a > problem, the old behavior could be added back as a fallback. > > This is version 2 of a patchset that I requested feedback for about a > month ago. Please review as if they are a new patchset; all the patches > were rebased several times and a couple new correctness fixes added. > > The TODO list is largely unchanged, aside from the couple of TODO > comments in the code that I have addressed. Ultimately, I think this > driver could potentially be merged with the "real" mtk-mmc driver as the > TODO suggests, but someone who is more familiar with the IP core will > have to do that. Mediatek documentation (that I can find) is very > sparse. > > This is ready to merge if there is no other feedback! > > >From George Hilliard # This line is ignored. > From: George Hilliard > Reply-To: > Subject: [PATCH v2 00/11] mt7621-mmc: Various correctness fixes > In-Reply-To: > > No subject for this email?
Re: [PATCH] s390:tty3270:move spin_lock_bh to spin_lock in tasklet
On Wed, Mar 20, 2019 at 12:12:36AM +0800, Jeff Xie wrote: > It is unnecessary to call spin_lock_bh in a tasklet. > > Signed-off-by: Jeff Xie > --- > drivers/s390/char/tty3270.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/s390/char/tty3270.c b/drivers/s390/char/tty3270.c > index 2b0c36c2..2963396 100644 > --- a/drivers/s390/char/tty3270.c > +++ b/drivers/s390/char/tty3270.c > @@ -563,7 +563,7 @@ tty3270_read_tasklet(struct raw3270_request *rrq) > char *input; > int len; > > - spin_lock_bh(&tp->view.lock); > + spin_lock(&tp->view.lock); > /* >* Two AID keys are special: For 0x7d (enter) the input line >* has to be emitted to the tty and for 0x6d the screen > @@ -590,7 +590,7 @@ tty3270_read_tasklet(struct raw3270_request *rrq) > tp->update_flags = TTY_UPDATE_ALL; > tty3270_set_timer(tp, 1); > } > - spin_unlock_bh(&tp->view.lock); > + spin_unlock(&tp->view.lock); I'm not going to take this. There is close to zero benefit, but rather high cost to review this - just because the function is named *tasklet doesn't mean it is only called in tasklet / softirq context.
[PATCH 1/3] mm/sparse: Clean up the obsolete code comment
The code comment above sparse_add_one_section() is obsolete and incorrect, clean it up and write new one. Signed-off-by: Baoquan He --- mm/sparse.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/mm/sparse.c b/mm/sparse.c index 77a0554fa5bd..0a0f82c5d969 100644 --- a/mm/sparse.c +++ b/mm/sparse.c @@ -674,9 +674,12 @@ static void free_map_bootmem(struct page *memmap) #endif /* CONFIG_SPARSEMEM_VMEMMAP */ /* - * returns the number of sections whose mem_maps were properly - * set. If this is <=0, then that means that the passed-in - * map was not consumed and must be freed. + * sparse_add_one_section - add a memory section + * @nid: The node to add section on + * @start_pfn: start pfn of the memory range + * @altmap:device page map + * + * Return 0 on success and an appropriate error code otherwise. */ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn, struct vmem_altmap *altmap) -- 2.17.2
[PATCH] extcon: fix a missing check of regmap_read
When regmap_read fails, it doesn't make sense to use the read value "val" because it can be uninitialized. The fix returns if regmap_read fails. Signed-off-by: Kangjie Lu --- drivers/extcon/extcon-axp288.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c index a983708b77a6..b2ba5f073aa7 100644 --- a/drivers/extcon/extcon-axp288.c +++ b/drivers/extcon/extcon-axp288.c @@ -143,6 +143,10 @@ static void axp288_extcon_log_rsi(struct axp288_extcon_info *info) int ret; ret = regmap_read(info->regmap, AXP288_PS_BOOT_REASON_REG, &val); + if (ret) { + dev_err(info->dev, "failed to read BOOT_REASON_REG: %d\n", ret); + return; + } for (i = 0, rsi = axp288_pwr_up_down_info; *rsi; rsi++, i++) { if (val & BIT(i)) { dev_dbg(info->dev, "%s\n", *rsi); -- 2.17.1
[PATCH 3/3] mm/sparse: Rename function related to section memmap allocation/free
These functions are used allocate/free section memmap, have nothing to do with kmalloc/free during the handling. Rename them to remove the confusion. Signed-off-by: Baoquan He --- mm/sparse.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/mm/sparse.c b/mm/sparse.c index 054b99f74181..374206212d01 100644 --- a/mm/sparse.c +++ b/mm/sparse.c @@ -579,13 +579,13 @@ void offline_mem_sections(unsigned long start_pfn, unsigned long end_pfn) #endif #ifdef CONFIG_SPARSEMEM_VMEMMAP -static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid, +static inline struct page *alloc_section_memmap(unsigned long pnum, int nid, struct vmem_altmap *altmap) { /* This will make the necessary allocations eventually. */ return sparse_mem_map_populate(pnum, nid, altmap); } -static void __kfree_section_memmap(struct page *memmap, +static void __free_section_memmap(struct page *memmap, struct vmem_altmap *altmap) { unsigned long start = (unsigned long)memmap; @@ -603,7 +603,7 @@ static void free_map_bootmem(struct page *memmap) } #endif /* CONFIG_MEMORY_HOTREMOVE */ #else -static struct page *__kmalloc_section_memmap(void) +static struct page *__alloc_section_memmap(void) { struct page *page, *ret; unsigned long memmap_size = sizeof(struct page) * PAGES_PER_SECTION; @@ -624,13 +624,13 @@ static struct page *__kmalloc_section_memmap(void) return ret; } -static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid, +static inline struct page *alloc_section_memmap(unsigned long pnum, int nid, struct vmem_altmap *altmap) { - return __kmalloc_section_memmap(); + return __alloc_section_memmap(); } -static void __kfree_section_memmap(struct page *memmap, +static void __free_section_memmap(struct page *memmap, struct vmem_altmap *altmap) { if (is_vmalloc_addr(memmap)) @@ -701,7 +701,7 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn, usemap = __kmalloc_section_usemap(); if (!usemap) return -ENOMEM; - memmap = kmalloc_section_memmap(section_nr, nid, altmap); + memmap = alloc_section_memmap(section_nr, nid, altmap); if (!memmap) { kfree(usemap); return -ENOMEM; @@ -726,7 +726,7 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn, out: if (ret < 0) { kfree(usemap); - __kfree_section_memmap(memmap, altmap); + __free_section_memmap(memmap, altmap); } return ret; } @@ -777,7 +777,7 @@ static void free_section_usemap(struct page *memmap, unsigned long *usemap, if (PageSlab(usemap_page) || PageCompound(usemap_page)) { kfree(usemap); if (memmap) - __kfree_section_memmap(memmap, altmap); + __free_section_memmap(memmap, altmap); return; } -- 2.17.2
[PATCH 2/3] mm/sparse: Optimize sparse_add_one_section()
Reorder the allocation of usemap and memmap since usemap allocation is much smaller and simpler. Otherwise hard work is done to make memmap ready, then have to rollback just because of usemap allocation failure. Signed-off-by: Baoquan He --- mm/sparse.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/mm/sparse.c b/mm/sparse.c index 0a0f82c5d969..054b99f74181 100644 --- a/mm/sparse.c +++ b/mm/sparse.c @@ -697,16 +697,17 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn, ret = sparse_index_init(section_nr, nid); if (ret < 0 && ret != -EEXIST) return ret; - ret = 0; - memmap = kmalloc_section_memmap(section_nr, nid, altmap); - if (!memmap) - return -ENOMEM; + usemap = __kmalloc_section_usemap(); - if (!usemap) { - __kfree_section_memmap(memmap, altmap); + if (!usemap) + return -ENOMEM; + memmap = kmalloc_section_memmap(section_nr, nid, altmap); + if (!memmap) { + kfree(usemap); return -ENOMEM; } + ret = 0; ms = __pfn_to_section(start_pfn); if (ms->section_mem_map & SECTION_MARKED_PRESENT) { ret = -EEXIST; -- 2.17.2
Re: [PATCH 1/3] mm/sparse: Clean up the obsolete code comment
Hi, On Wed, Mar 20, 2019 at 03:35:38PM +0800, Baoquan He wrote: > The code comment above sparse_add_one_section() is obsolete and > incorrect, clean it up and write new one. > > Signed-off-by: Baoquan He > --- > mm/sparse.c | 9 ++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/mm/sparse.c b/mm/sparse.c > index 77a0554fa5bd..0a0f82c5d969 100644 > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -674,9 +674,12 @@ static void free_map_bootmem(struct page *memmap) > #endif /* CONFIG_SPARSEMEM_VMEMMAP */ > > /* > - * returns the number of sections whose mem_maps were properly > - * set. If this is <=0, then that means that the passed-in > - * map was not consumed and must be freed. > + * sparse_add_one_section - add a memory section Please mention that this is only intended for memory hotplug > + * @nid: The node to add section on > + * @start_pfn: start pfn of the memory range > + * @altmap: device page map > + * > + * Return 0 on success and an appropriate error code otherwise. s/Return/Return:/ please > */ > int __meminit sparse_add_one_section(int nid, unsigned long start_pfn, >struct vmem_altmap *altmap) > -- > 2.17.2 > -- Sincerely yours, Mike.
[PATCH] mm/sparse: Rename function related to section memmap allocation/free
These functions are used allocate/free section memmap, have nothing to do with kmalloc/free during the handling. Rename them to remove the confusion. Signed-off-by: Baoquan He --- mm/sparse.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/mm/sparse.c b/mm/sparse.c index 054b99f74181..374206212d01 100644 --- a/mm/sparse.c +++ b/mm/sparse.c @@ -579,13 +579,13 @@ void offline_mem_sections(unsigned long start_pfn, unsigned long end_pfn) #endif #ifdef CONFIG_SPARSEMEM_VMEMMAP -static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid, +static inline struct page *alloc_section_memmap(unsigned long pnum, int nid, struct vmem_altmap *altmap) { /* This will make the necessary allocations eventually. */ return sparse_mem_map_populate(pnum, nid, altmap); } -static void __kfree_section_memmap(struct page *memmap, +static void __free_section_memmap(struct page *memmap, struct vmem_altmap *altmap) { unsigned long start = (unsigned long)memmap; @@ -603,7 +603,7 @@ static void free_map_bootmem(struct page *memmap) } #endif /* CONFIG_MEMORY_HOTREMOVE */ #else -static struct page *__kmalloc_section_memmap(void) +static struct page *__alloc_section_memmap(void) { struct page *page, *ret; unsigned long memmap_size = sizeof(struct page) * PAGES_PER_SECTION; @@ -624,13 +624,13 @@ static struct page *__kmalloc_section_memmap(void) return ret; } -static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid, +static inline struct page *alloc_section_memmap(unsigned long pnum, int nid, struct vmem_altmap *altmap) { - return __kmalloc_section_memmap(); + return __alloc_section_memmap(); } -static void __kfree_section_memmap(struct page *memmap, +static void __free_section_memmap(struct page *memmap, struct vmem_altmap *altmap) { if (is_vmalloc_addr(memmap)) @@ -701,7 +701,7 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn, usemap = __kmalloc_section_usemap(); if (!usemap) return -ENOMEM; - memmap = kmalloc_section_memmap(section_nr, nid, altmap); + memmap = alloc_section_memmap(section_nr, nid, altmap); if (!memmap) { kfree(usemap); return -ENOMEM; @@ -726,7 +726,7 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn, out: if (ret < 0) { kfree(usemap); - __kfree_section_memmap(memmap, altmap); + __free_section_memmap(memmap, altmap); } return ret; } @@ -777,7 +777,7 @@ static void free_section_usemap(struct page *memmap, unsigned long *usemap, if (PageSlab(usemap_page) || PageCompound(usemap_page)) { kfree(usemap); if (memmap) - __kfree_section_memmap(memmap, altmap); + __free_section_memmap(memmap, altmap); return; } -- 2.17.2
Re: [PATCH] mm/sparse: Rename function related to section memmap allocation/free
On 03/20/19 at 03:53pm, Baoquan He wrote: > These functions are used allocate/free section memmap, have nothing ^ 'to' missed here, will update later. > to do with kmalloc/free during the handling. Rename them to remove > the confusion. > > Signed-off-by: Baoquan He > --- > mm/sparse.c | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/mm/sparse.c b/mm/sparse.c > index 054b99f74181..374206212d01 100644 > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -579,13 +579,13 @@ void offline_mem_sections(unsigned long start_pfn, > unsigned long end_pfn) > #endif > > #ifdef CONFIG_SPARSEMEM_VMEMMAP > -static inline struct page *kmalloc_section_memmap(unsigned long pnum, int > nid, > +static inline struct page *alloc_section_memmap(unsigned long pnum, int nid, > struct vmem_altmap *altmap) > { > /* This will make the necessary allocations eventually. */ > return sparse_mem_map_populate(pnum, nid, altmap); > } > -static void __kfree_section_memmap(struct page *memmap, > +static void __free_section_memmap(struct page *memmap, > struct vmem_altmap *altmap) > { > unsigned long start = (unsigned long)memmap; > @@ -603,7 +603,7 @@ static void free_map_bootmem(struct page *memmap) > } > #endif /* CONFIG_MEMORY_HOTREMOVE */ > #else > -static struct page *__kmalloc_section_memmap(void) > +static struct page *__alloc_section_memmap(void) > { > struct page *page, *ret; > unsigned long memmap_size = sizeof(struct page) * PAGES_PER_SECTION; > @@ -624,13 +624,13 @@ static struct page *__kmalloc_section_memmap(void) > return ret; > } > > -static inline struct page *kmalloc_section_memmap(unsigned long pnum, int > nid, > +static inline struct page *alloc_section_memmap(unsigned long pnum, int nid, > struct vmem_altmap *altmap) > { > - return __kmalloc_section_memmap(); > + return __alloc_section_memmap(); > } > > -static void __kfree_section_memmap(struct page *memmap, > +static void __free_section_memmap(struct page *memmap, > struct vmem_altmap *altmap) > { > if (is_vmalloc_addr(memmap)) > @@ -701,7 +701,7 @@ int __meminit sparse_add_one_section(int nid, unsigned > long start_pfn, > usemap = __kmalloc_section_usemap(); > if (!usemap) > return -ENOMEM; > - memmap = kmalloc_section_memmap(section_nr, nid, altmap); > + memmap = alloc_section_memmap(section_nr, nid, altmap); > if (!memmap) { > kfree(usemap); > return -ENOMEM; > @@ -726,7 +726,7 @@ int __meminit sparse_add_one_section(int nid, unsigned > long start_pfn, > out: > if (ret < 0) { > kfree(usemap); > - __kfree_section_memmap(memmap, altmap); > + __free_section_memmap(memmap, altmap); > } > return ret; > } > @@ -777,7 +777,7 @@ static void free_section_usemap(struct page *memmap, > unsigned long *usemap, > if (PageSlab(usemap_page) || PageCompound(usemap_page)) { > kfree(usemap); > if (memmap) > - __kfree_section_memmap(memmap, altmap); > + __free_section_memmap(memmap, altmap); > return; > } > > -- > 2.17.2 >
Re: [PATCH v7 0/4] Enable wm8524 on i.MX8MQ-EVK
On Tue, Mar 19, 2019 at 05:48:35PM +, Daniel Baluta wrote: > On i.MX8MQ EVK we can start the party using the wm8524 codec > which gets it's data through the SAI2 interface. > > In order to make it work this patch series enables the SDMA nodes, > sets the correct pinctrl configuration and uses the simple card > machine driver to put everything together. > > Changes since v6: (after Shawn's comments) > - rebased on latest Shawn's for-next branch > - added new lines between properties and child nodes > - added new lines between child nodes > - keep pinctrl nodes sorted > - added reviewed by after SoBs in commit message > > Changes since v5: > - remove clocks and clock-names properties from wm8524 > code node as they are not used. > > Changes since v4 (after Fabio's review) > - removed mclk0 from sai2 node because it is not yet supported > by SAI driver > - s/audio-codec-0/audio-codec > - squashed patches 4 and 5 > > Changes since v3: > - use "arm64: dts: imx8mq-evk" prefix for patches specific to > 8MQ-EVK > - squash together patches for setting up SAI (previous 3/5 and > 4/5) > - add back and document "fsl, imx8mq-sdma" compatbile string > > Changes since v2: > - s/QM/MQ after Chris comments > > Changes since v1: > - added cover letter > - remove "fsl,imx8mq-sdma" compatible for sdma. > > Daniel Baluta (4): > arm64: dts: imx8mq: Add SDMA nodes > bindings: fsl-imx-sdma: Document fsl,imx8mq-sdma compatbile string > arm64: dts: imx8mq: Add SAI2 node > arm64: dts: imx8mq-evk: Enable audio codec wm8524 Applied all, thanks.
Re: [PATCH 2/3] mm/sparse: Optimize sparse_add_one_section()
On Wed, Mar 20, 2019 at 03:35:39PM +0800, Baoquan He wrote: > Reorder the allocation of usemap and memmap since usemap allocation > is much smaller and simpler. Otherwise hard work is done to make > memmap ready, then have to rollback just because of usemap allocation > failure. > > Signed-off-by: Baoquan He > --- > mm/sparse.c | 13 +++-- > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/mm/sparse.c b/mm/sparse.c > index 0a0f82c5d969..054b99f74181 100644 > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -697,16 +697,17 @@ int __meminit sparse_add_one_section(int nid, unsigned > long start_pfn, > ret = sparse_index_init(section_nr, nid); > if (ret < 0 && ret != -EEXIST) > return ret; > - ret = 0; > - memmap = kmalloc_section_memmap(section_nr, nid, altmap); > - if (!memmap) > - return -ENOMEM; > + > usemap = __kmalloc_section_usemap(); > - if (!usemap) { > - __kfree_section_memmap(memmap, altmap); > + if (!usemap) > + return -ENOMEM; > + memmap = kmalloc_section_memmap(section_nr, nid, altmap); > + if (!memmap) { > + kfree(usemap); If you are anyway changing this why not to switch to goto's for error handling? > return -ENOMEM; > } > > + ret = 0; > ms = __pfn_to_section(start_pfn); > if (ms->section_mem_map & SECTION_MARKED_PRESENT) { > ret = -EEXIST; > -- > 2.17.2 > -- Sincerely yours, Mike.
[PATCH] blk-mq: fix a hung issue when set device state to blocked and restore running
When I use dd test a SCSI device which use blk-mq in the following steps: 1.echo "blocked" >/sys/block/sda/device/state 2.dd if=/dev/sda of=/mnt/t.log bs=1M count=10 3.echo "running" >/sys/block/sda/device/state dd should finish this work after step 3, unfortunately, still hung. After step2, the key code process is like this: blk_mq_dispatch_rq_list-->scsi_queue_rq-->prep_to_mq -->if ret is BLK_STS_RESOURCE, delay run hw queue prep_to_mq will return BLK_STS_RESOURCE, and scsi_queue_rq will transter it to BLK_STS_DEV_RESOURCE. In this situtation, we should delay run hw queue. This patch fixes that. Fixes: 86ff7c2a80cd ("blk-mq: introduce BLK_STS_DEV_RESOURCE") Signed-off-by: zhengbin --- block/blk-mq.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index a9c1816..556d606 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1309,15 +1309,17 @@ bool blk_mq_dispatch_rq_list(struct request_queue *q, struct list_head *list, * returning BLK_STS_RESOURCE. Two exceptions are scsi-mq * and dm-rq. * -* If driver returns BLK_STS_RESOURCE and SCHED_RESTART -* bit is set, run queue after a delay to avoid IO stalls -* that could otherwise occur if the queue is idle. +* If driver returns BLK_STS_RESOURCE or BLK_STS_DEV_RESOURCE +* and SCHED_RESTART bit is set, run queue after a delay to +* avoid IO stalls that could otherwise occur if the queue +* is idle. */ needs_restart = blk_mq_sched_needs_restart(hctx); if (!needs_restart || (no_tag && list_empty_careful(&hctx->dispatch_wait.entry))) blk_mq_run_hw_queue(hctx, true); - else if (needs_restart && (ret == BLK_STS_RESOURCE)) + else if (needs_restart && ((ret == BLK_STS_RESOURCE) || + (ret == BLK_STS_DEV_RESOURCE))) blk_mq_delay_run_hw_queue(hctx, BLK_MQ_RESOURCE_DELAY); blk_mq_update_dispatch_busy(hctx, true); -- 2.7.4
Re: [PATCH] mm/sparse: Rename function related to section memmap allocation/free
On 03/20/19 at 03:53pm, Baoquan He wrote: > These functions are used allocate/free section memmap, have nothing > to do with kmalloc/free during the handling. Rename them to remove > the confusion. Sorry, wrong git operation caused this one sent. I intended to send out other single patch. Please ignore this one. It has been included in previous patchset. > > Signed-off-by: Baoquan He > --- > mm/sparse.c | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/mm/sparse.c b/mm/sparse.c > index 054b99f74181..374206212d01 100644 > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -579,13 +579,13 @@ void offline_mem_sections(unsigned long start_pfn, > unsigned long end_pfn) > #endif > > #ifdef CONFIG_SPARSEMEM_VMEMMAP > -static inline struct page *kmalloc_section_memmap(unsigned long pnum, int > nid, > +static inline struct page *alloc_section_memmap(unsigned long pnum, int nid, > struct vmem_altmap *altmap) > { > /* This will make the necessary allocations eventually. */ > return sparse_mem_map_populate(pnum, nid, altmap); > } > -static void __kfree_section_memmap(struct page *memmap, > +static void __free_section_memmap(struct page *memmap, > struct vmem_altmap *altmap) > { > unsigned long start = (unsigned long)memmap; > @@ -603,7 +603,7 @@ static void free_map_bootmem(struct page *memmap) > } > #endif /* CONFIG_MEMORY_HOTREMOVE */ > #else > -static struct page *__kmalloc_section_memmap(void) > +static struct page *__alloc_section_memmap(void) > { > struct page *page, *ret; > unsigned long memmap_size = sizeof(struct page) * PAGES_PER_SECTION; > @@ -624,13 +624,13 @@ static struct page *__kmalloc_section_memmap(void) > return ret; > } > > -static inline struct page *kmalloc_section_memmap(unsigned long pnum, int > nid, > +static inline struct page *alloc_section_memmap(unsigned long pnum, int nid, > struct vmem_altmap *altmap) > { > - return __kmalloc_section_memmap(); > + return __alloc_section_memmap(); > } > > -static void __kfree_section_memmap(struct page *memmap, > +static void __free_section_memmap(struct page *memmap, > struct vmem_altmap *altmap) > { > if (is_vmalloc_addr(memmap)) > @@ -701,7 +701,7 @@ int __meminit sparse_add_one_section(int nid, unsigned > long start_pfn, > usemap = __kmalloc_section_usemap(); > if (!usemap) > return -ENOMEM; > - memmap = kmalloc_section_memmap(section_nr, nid, altmap); > + memmap = alloc_section_memmap(section_nr, nid, altmap); > if (!memmap) { > kfree(usemap); > return -ENOMEM; > @@ -726,7 +726,7 @@ int __meminit sparse_add_one_section(int nid, unsigned > long start_pfn, > out: > if (ret < 0) { > kfree(usemap); > - __kfree_section_memmap(memmap, altmap); > + __free_section_memmap(memmap, altmap); > } > return ret; > } > @@ -777,7 +777,7 @@ static void free_section_usemap(struct page *memmap, > unsigned long *usemap, > if (PageSlab(usemap_page) || PageCompound(usemap_page)) { > kfree(usemap); > if (memmap) > - __kfree_section_memmap(memmap, altmap); > + __free_section_memmap(memmap, altmap); > return; > } > > -- > 2.17.2 >
Re: [PATCH 1/3] mm/sparse: Clean up the obsolete code comment
On 03/20/19 at 09:50am, Mike Rapoport wrote: > Hi, > > On Wed, Mar 20, 2019 at 03:35:38PM +0800, Baoquan He wrote: > > The code comment above sparse_add_one_section() is obsolete and > > incorrect, clean it up and write new one. > > > > Signed-off-by: Baoquan He > > --- > > mm/sparse.c | 9 ++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/mm/sparse.c b/mm/sparse.c > > index 77a0554fa5bd..0a0f82c5d969 100644 > > --- a/mm/sparse.c > > +++ b/mm/sparse.c > > @@ -674,9 +674,12 @@ static void free_map_bootmem(struct page *memmap) > > #endif /* CONFIG_SPARSEMEM_VMEMMAP */ > > > > /* > > - * returns the number of sections whose mem_maps were properly > > - * set. If this is <=0, then that means that the passed-in > > - * map was not consumed and must be freed. > > + * sparse_add_one_section - add a memory section > > Please mention that this is only intended for memory hotplug Will do. Thanks for reviewing. > > > + * @nid: The node to add section on > > + * @start_pfn: start pfn of the memory range > > + * @altmap:device page map > > + * > > + * Return 0 on success and an appropriate error code otherwise. > > s/Return/Return:/ please Thanks, will change. > > > */ > > int __meminit sparse_add_one_section(int nid, unsigned long start_pfn, > > struct vmem_altmap *altmap) > > -- > > 2.17.2 > > > > -- > Sincerely yours, > Mike. >
Re: [PATCH] extcon: fix a missing check of regmap_read
Hi, You better to edit the patch title as following in order to sustain the title format for extcon: extcon: fix a missing check of regmap_read -> extcon: axp288: Fix a missing check of regmap_read On 19. 3. 20. 오후 4:35, Kangjie Lu wrote: > When regmap_read fails, it doesn't make sense to use the read > value "val" because it can be uninitialized. > > The fix returns if regmap_read fails. > > Signed-off-by: Kangjie Lu > --- > drivers/extcon/extcon-axp288.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c > index a983708b77a6..b2ba5f073aa7 100644 > --- a/drivers/extcon/extcon-axp288.c > +++ b/drivers/extcon/extcon-axp288.c > @@ -143,6 +143,10 @@ static void axp288_extcon_log_rsi(struct > axp288_extcon_info *info) > int ret; > > ret = regmap_read(info->regmap, AXP288_PS_BOOT_REASON_REG, &val); > + if (ret) { > + dev_err(info->dev, "failed to read BOOT_REASON_REG: %d\n", ret); > + return; > + } Have to add the blank line. time. And I think that axp288_extcon_log_rsi() have to return the 'error value' instead of 'void' return type. And then should handle the return value of axp288_extcon_log_rsi() within the axp288_extcon_probe(). For example, ret = axp288_extcon_log_rsi(info) if (ret < 0) return ret; > for (i = 0, rsi = axp288_pwr_up_down_info; *rsi; rsi++, i++) { > if (val & BIT(i)) { > dev_dbg(info->dev, "%s\n", *rsi); > -- Best Regards, Chanwoo Choi Samsung Electronics
Re: [PATCH] mm/sparse: Rename function related to section memmap allocation/free
On Wed, Mar 20, 2019 at 03:53:01PM +0800, Baoquan He wrote: > These functions are used allocate/free section memmap, have nothing > to do with kmalloc/free during the handling. Rename them to remove > the confusion. > > Signed-off-by: Baoquan He Acked-by: Mike Rapoport > --- > mm/sparse.c | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/mm/sparse.c b/mm/sparse.c > index 054b99f74181..374206212d01 100644 > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -579,13 +579,13 @@ void offline_mem_sections(unsigned long start_pfn, > unsigned long end_pfn) > #endif > > #ifdef CONFIG_SPARSEMEM_VMEMMAP > -static inline struct page *kmalloc_section_memmap(unsigned long pnum, int > nid, > +static inline struct page *alloc_section_memmap(unsigned long pnum, int nid, > struct vmem_altmap *altmap) > { > /* This will make the necessary allocations eventually. */ > return sparse_mem_map_populate(pnum, nid, altmap); > } > -static void __kfree_section_memmap(struct page *memmap, > +static void __free_section_memmap(struct page *memmap, > struct vmem_altmap *altmap) > { > unsigned long start = (unsigned long)memmap; > @@ -603,7 +603,7 @@ static void free_map_bootmem(struct page *memmap) > } > #endif /* CONFIG_MEMORY_HOTREMOVE */ > #else > -static struct page *__kmalloc_section_memmap(void) > +static struct page *__alloc_section_memmap(void) > { > struct page *page, *ret; > unsigned long memmap_size = sizeof(struct page) * PAGES_PER_SECTION; > @@ -624,13 +624,13 @@ static struct page *__kmalloc_section_memmap(void) > return ret; > } > > -static inline struct page *kmalloc_section_memmap(unsigned long pnum, int > nid, > +static inline struct page *alloc_section_memmap(unsigned long pnum, int nid, > struct vmem_altmap *altmap) > { > - return __kmalloc_section_memmap(); > + return __alloc_section_memmap(); > } > > -static void __kfree_section_memmap(struct page *memmap, > +static void __free_section_memmap(struct page *memmap, > struct vmem_altmap *altmap) > { > if (is_vmalloc_addr(memmap)) > @@ -701,7 +701,7 @@ int __meminit sparse_add_one_section(int nid, unsigned > long start_pfn, > usemap = __kmalloc_section_usemap(); > if (!usemap) > return -ENOMEM; > - memmap = kmalloc_section_memmap(section_nr, nid, altmap); > + memmap = alloc_section_memmap(section_nr, nid, altmap); > if (!memmap) { > kfree(usemap); > return -ENOMEM; > @@ -726,7 +726,7 @@ int __meminit sparse_add_one_section(int nid, unsigned > long start_pfn, > out: > if (ret < 0) { > kfree(usemap); > - __kfree_section_memmap(memmap, altmap); > + __free_section_memmap(memmap, altmap); > } > return ret; > } > @@ -777,7 +777,7 @@ static void free_section_usemap(struct page *memmap, > unsigned long *usemap, > if (PageSlab(usemap_page) || PageCompound(usemap_page)) { > kfree(usemap); > if (memmap) > - __kfree_section_memmap(memmap, altmap); > + __free_section_memmap(memmap, altmap); > return; > } > > -- > 2.17.2 > -- Sincerely yours, Mike.
Re: [PATCH 1/2] clk: imx7ulp: remove snvs clock
On Thu, Feb 28, 2019 at 08:56:00AM +, Anson Huang wrote: > Since i.MX7ULP B0 chip, the SNVS module is moved into M4 > domain and its clock is also moved into PCC0 which is > contorlled by M4, Linux kernel should NOT add it into > clock tree. > > Signed-off-by: Anson Huang Applied both, thanks.
Re: [PATCH] mm/sparse: Rename function related to section memmap allocation/free
On 03/20/19 at 09:59am, Mike Rapoport wrote: > On Wed, Mar 20, 2019 at 03:53:01PM +0800, Baoquan He wrote: > > These functions are used allocate/free section memmap, have nothing > > to do with kmalloc/free during the handling. Rename them to remove > > the confusion. > > > > Signed-off-by: Baoquan He > > Acked-by: Mike Rapoport Thanks for reviewing, Mike. I makde mistake to send this one twice. You can see it has been added into the patchset. Anyway, I will add your 'Acked-by' when repost to address those issues you pointed out. Thanks Baoquan > > > --- > > mm/sparse.c | 18 +- > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/mm/sparse.c b/mm/sparse.c > > index 054b99f74181..374206212d01 100644 > > --- a/mm/sparse.c > > +++ b/mm/sparse.c > > @@ -579,13 +579,13 @@ void offline_mem_sections(unsigned long start_pfn, > > unsigned long end_pfn) > > #endif > > > > #ifdef CONFIG_SPARSEMEM_VMEMMAP > > -static inline struct page *kmalloc_section_memmap(unsigned long pnum, int > > nid, > > +static inline struct page *alloc_section_memmap(unsigned long pnum, int > > nid, > > struct vmem_altmap *altmap) > > { > > /* This will make the necessary allocations eventually. */ > > return sparse_mem_map_populate(pnum, nid, altmap); > > } > > -static void __kfree_section_memmap(struct page *memmap, > > +static void __free_section_memmap(struct page *memmap, > > struct vmem_altmap *altmap) > > { > > unsigned long start = (unsigned long)memmap; > > @@ -603,7 +603,7 @@ static void free_map_bootmem(struct page *memmap) > > } > > #endif /* CONFIG_MEMORY_HOTREMOVE */ > > #else > > -static struct page *__kmalloc_section_memmap(void) > > +static struct page *__alloc_section_memmap(void) > > { > > struct page *page, *ret; > > unsigned long memmap_size = sizeof(struct page) * PAGES_PER_SECTION; > > @@ -624,13 +624,13 @@ static struct page *__kmalloc_section_memmap(void) > > return ret; > > } > > > > -static inline struct page *kmalloc_section_memmap(unsigned long pnum, int > > nid, > > +static inline struct page *alloc_section_memmap(unsigned long pnum, int > > nid, > > struct vmem_altmap *altmap) > > { > > - return __kmalloc_section_memmap(); > > + return __alloc_section_memmap(); > > } > > > > -static void __kfree_section_memmap(struct page *memmap, > > +static void __free_section_memmap(struct page *memmap, > > struct vmem_altmap *altmap) > > { > > if (is_vmalloc_addr(memmap)) > > @@ -701,7 +701,7 @@ int __meminit sparse_add_one_section(int nid, unsigned > > long start_pfn, > > usemap = __kmalloc_section_usemap(); > > if (!usemap) > > return -ENOMEM; > > - memmap = kmalloc_section_memmap(section_nr, nid, altmap); > > + memmap = alloc_section_memmap(section_nr, nid, altmap); > > if (!memmap) { > > kfree(usemap); > > return -ENOMEM; > > @@ -726,7 +726,7 @@ int __meminit sparse_add_one_section(int nid, unsigned > > long start_pfn, > > out: > > if (ret < 0) { > > kfree(usemap); > > - __kfree_section_memmap(memmap, altmap); > > + __free_section_memmap(memmap, altmap); > > } > > return ret; > > } > > @@ -777,7 +777,7 @@ static void free_section_usemap(struct page *memmap, > > unsigned long *usemap, > > if (PageSlab(usemap_page) || PageCompound(usemap_page)) { > > kfree(usemap); > > if (memmap) > > - __kfree_section_memmap(memmap, altmap); > > + __free_section_memmap(memmap, altmap); > > return; > > } > > > > -- > > 2.17.2 > > > > -- > Sincerely yours, > Mike. >
Re: [PATCH 2/3] mm/sparse: Optimize sparse_add_one_section()
On 03/20/19 at 09:56am, Mike Rapoport wrote: > > diff --git a/mm/sparse.c b/mm/sparse.c > > index 0a0f82c5d969..054b99f74181 100644 > > --- a/mm/sparse.c > > +++ b/mm/sparse.c > > @@ -697,16 +697,17 @@ int __meminit sparse_add_one_section(int nid, > > unsigned long start_pfn, > > ret = sparse_index_init(section_nr, nid); > > if (ret < 0 && ret != -EEXIST) > > return ret; > > - ret = 0; > > - memmap = kmalloc_section_memmap(section_nr, nid, altmap); > > - if (!memmap) > > - return -ENOMEM; > > + > > usemap = __kmalloc_section_usemap(); > > - if (!usemap) { > > - __kfree_section_memmap(memmap, altmap); > > + if (!usemap) > > + return -ENOMEM; > > + memmap = kmalloc_section_memmap(section_nr, nid, altmap); > > + if (!memmap) { > > + kfree(usemap); > > If you are anyway changing this why not to switch to goto's for error > handling? OK, I am fine to switch to 'goto'. Will update and repost. Thanks. > > > return -ENOMEM; > > } > > > > + ret = 0; > > ms = __pfn_to_section(start_pfn); > > if (ms->section_mem_map & SECTION_MARKED_PRESENT) { > > ret = -EEXIST; > > -- > > 2.17.2 > > > > -- > Sincerely yours, > Mike. >
[PATCH] mm, memory_hotplug: Fix the wrong usage of N_HIGH_MEMORY
In function node_states_check_changes_online(), N_HIGH_MEMORY is used to substitute ZONE_HIGHMEM directly. This is not right. N_HIGH_MEMORY always has value '3' if CONFIG_HIGHMEM=y, while ZONE_HIGHMEM's value is not. It depends on whether CONFIG_ZONE_DMA/CONFIG_ZONE_DMA32 are enabled. Obviously it's not true for CONFIG_ZONE_DMA32 on 32bit system, and CONFIG_ZONE_DMA is also optional. Replace it with ZONE_HIGHMEM. Fixes: 8efe33f40f3e ("mm/memory_hotplug.c: simplify node_states_check_changes_online") Signed-off-by: Baoquan He --- mm/memory_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 6b05576fb4ec..09911d34a3be 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -712,7 +712,7 @@ static void node_states_check_changes_online(unsigned long nr_pages, if (zone_idx(zone) <= ZONE_NORMAL && !node_state(nid, N_NORMAL_MEMORY)) arg->status_change_nid_normal = nid; #ifdef CONFIG_HIGHMEM - if (zone_idx(zone) <= N_HIGH_MEMORY && !node_state(nid, N_HIGH_MEMORY)) + if (zone_idx(zone) <= ZONE_HIGHMEM && !node_state(nid, N_HIGH_MEMORY)) arg->status_change_nid_high = nid; #endif } -- 2.17.2
Re: [PATCH] ARM: dts: imx7s-warp: PMIC swbst boot-on/always-on
On Sat, Mar 02, 2019 at 11:32:29PM +0100, Pierre-Jean Texier wrote: > PMIC swbst regulator is used for the MikroBUS socket (pin +5V). > > We have to set the regulator to "boot-on" and "always-on" > to output a voltage of 5V on this socket. > > Signed-off-by: Pierre-Jean Texier Applied, thanks.
[PATCH] arm64: dts: sdm845: Include rpmpd DT header
In order to fix dependencies with rpmpd DT entries, the header was dropped and hardcoded values were added for opp-level, during the previous merge window. Add the header back in now and remove the hardcodings, effectively reverting commit '08585d21de9875a6064b350957faa0460a4c69a6: arm64: dts: sdm845: Fixup dependency on RPMPD includes' Signed-off-by: Rajendra Nayak --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 5308f1671824..e4b69c74fe07 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -2098,43 +2099,43 @@ compatible = "operating-points-v2"; rpmhpd_opp_ret: opp1 { - opp-level = <16>; + opp-level = ; }; rpmhpd_opp_min_svs: opp2 { - opp-level = <48>; + opp-level = ; }; rpmhpd_opp_low_svs: opp3 { - opp-level = <64>; + opp-level = ; }; rpmhpd_opp_svs: opp4 { - opp-level = <128>; + opp-level = ; }; rpmhpd_opp_svs_l1: opp5 { - opp-level = <192>; + opp-level = ; }; rpmhpd_opp_nom: opp6 { - opp-level = <256>; + opp-level = ; }; rpmhpd_opp_nom_l1: opp7 { - opp-level = <320>; + opp-level = ; }; rpmhpd_opp_nom_l2: opp8 { - opp-level = <336>; + opp-level = ; }; rpmhpd_opp_turbo: opp9 { - opp-level = <384>; + opp-level = ; }; rpmhpd_opp_turbo_l1: opp10 { - opp-level = <416>; + opp-level = ; }; }; }; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCH] Revert "svm: Fix AVIC incomplete IPI emulation"
This reverts commit bb218fbcfaaa3b115d4cd7a43c0ca164f3a96e57. As Oren Twaig pointed out the old discussion: https://patchwork.kernel.org/patch/8292231/ that the change coud potentially cause an extra IPI to be sent to the destination vcpu because the AVIC hardware already set the IRR bit before the incomplete IPI #VMEXIT with id=1 (target vcpu is not running). Since writting to ICR and ICR2 will also set the IRR. If something triggers the destination vcpu to get scheduled before the emulation finishes, then this could result in an additional IPI. Also, the issue mentioned in the commit bb218fbcfaaa was misdiagnosed. Cc: Radim Krčmář Cc: Paolo Bonzini Reported-by: Oren Twaig Signed-off-by: Suravee Suthikulpanit --- arch/x86/kvm/svm.c | 19 +++ 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index f13a3a24d360..47c4993448c7 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -4512,14 +4512,25 @@ static int avic_incomplete_ipi_interception(struct vcpu_svm *svm) kvm_lapic_reg_write(apic, APIC_ICR, icrl); break; case AVIC_IPI_FAILURE_TARGET_NOT_RUNNING: { + int i; + struct kvm_vcpu *vcpu; + struct kvm *kvm = svm->vcpu.kvm; struct kvm_lapic *apic = svm->vcpu.arch.apic; /* -* Update ICR high and low, then emulate sending IPI, -* which is handled when writing APIC_ICR. +* At this point, we expect that the AVIC HW has already +* set the appropriate IRR bits on the valid target +* vcpus. So, we just need to kick the appropriate vcpu. */ - kvm_lapic_reg_write(apic, APIC_ICR2, icrh); - kvm_lapic_reg_write(apic, APIC_ICR, icrl); + kvm_for_each_vcpu(i, vcpu, kvm) { + bool m = kvm_apic_match_dest(vcpu, apic, +icrl & KVM_APIC_SHORT_MASK, +GET_APIC_DEST_FIELD(icrh), +icrl & KVM_APIC_DEST_MASK); + + if (m && !avic_vcpu_is_running(vcpu)) + kvm_vcpu_wake_up(vcpu); + } break; } case AVIC_IPI_FAILURE_INVALID_TARGET: -- 2.17.1
Re: [PATCH] drivers: power: supply: goldfish_battery: Fix bogus SPDX identifier
A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? A: No. Q: Should I include quotations after my reply? On Tue, 19 Mar 2019, Roman Kiryanov wrote: > Hi, I am sorry for this warning. I remember I checked all changes I > sent, maybe missed this one. > > At the bottom the file says > > MODULE_LICENSE("GPL"); > > and > > // SPDX-License-Identifier: GPL-2.0 > > makes it inconsistent. No. It does not. 1) The module license is not a precise license indicator. 2) SPDX License identifiers are well defined and standardized. 'GPL' is NOT a valid SPDX identifier. The usage of SPDX identifiers and the relevance of the MODULE_LICENSE() string are well documented in: Documentation/process/license-rules.rst or for your conveniance at: https://www.kernel.org/doc/html/latest/process/license-rules.html Thanks, tglx
Re: [PATCH 2/4] clk: meson: meson8b: use a sparate clock table for Meson8m2
Hi, There is a typo in the subject "s/sparate/separate/" ! On 19/03/2019 22:51, Martin Blumenstingl wrote: > Meson8, Meson8b and Meson8m2 implement a similar clock controller. > However, there are a few differences between the three actual IP blocks. > > One example where Meson8m2 differs from Meson8b is the VPU clock setup: > - the VPU input mux can choose between "fclk_div4", "fclk_div3", > "fclk_div5" and "fclk_div7" on Meson8b > - however, on Meson8m2 it can choose between "fclk_div4", "fclk_div3", > "fclk_div5" and "gp_pll" (GP_PLL only exists on Meson8m2, it's the > predecessor of the GP0_PLL clock on GXBB/GXL/GXM)) By curiosity, what is the default (maximum) setup ? On GX & G12A, fclk_div3 is the default/max setup. > > Add a separate clk_hw_onecell_data table for Meson8m2 so these > differences can be implemented in our clock controller driver. For now > meson8m2_hw_onecell_data is a clone of our existing > meson8b_hw_onecell_data. > > Signed-off-by: Martin Blumenstingl > --- > drivers/clk/meson/meson8b.c | 193 +++- > 1 file changed, 192 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > index 576ad42252d0..c9e6ec67d649 100644 > --- a/drivers/clk/meson/meson8b.c > +++ b/drivers/clk/meson/meson8b.c > @@ -2157,6 +2157,192 @@ static struct clk_hw_onecell_data > meson8b_hw_onecell_data = { > .num = CLK_NR_CLKS, > }; > > +static struct clk_hw_onecell_data meson8m2_hw_onecell_data = { > + .hws = { > + [CLKID_XTAL] = &meson8b_xtal.hw, > + [CLKID_PLL_FIXED] = &meson8b_fixed_pll.hw, > + [CLKID_PLL_VID] = &meson8b_vid_pll.hw, > + [CLKID_PLL_SYS] = &meson8b_sys_pll.hw, > + [CLKID_FCLK_DIV2] = &meson8b_fclk_div2.hw, > + [CLKID_FCLK_DIV3] = &meson8b_fclk_div3.hw, > + [CLKID_FCLK_DIV4] = &meson8b_fclk_div4.hw, > + [CLKID_FCLK_DIV5] = &meson8b_fclk_div5.hw, > + [CLKID_FCLK_DIV7] = &meson8b_fclk_div7.hw, > + [CLKID_CPUCLK] = &meson8b_cpu_clk.hw, > + [CLKID_MPEG_SEL] = &meson8b_mpeg_clk_sel.hw, > + [CLKID_MPEG_DIV] = &meson8b_mpeg_clk_div.hw, > + [CLKID_CLK81] = &meson8b_clk81.hw, > + [CLKID_DDR] = &meson8b_ddr.hw, > + [CLKID_DOS] = &meson8b_dos.hw, > + [CLKID_ISA] = &meson8b_isa.hw, > + [CLKID_PL301] = &meson8b_pl301.hw, > + [CLKID_PERIPHS] = &meson8b_periphs.hw, > + [CLKID_SPICC] = &meson8b_spicc.hw, > + [CLKID_I2C] = &meson8b_i2c.hw, > + [CLKID_SAR_ADC] = &meson8b_sar_adc.hw, > + [CLKID_SMART_CARD] = &meson8b_smart_card.hw, > + [CLKID_RNG0]= &meson8b_rng0.hw, > + [CLKID_UART0] = &meson8b_uart0.hw, > + [CLKID_SDHC]= &meson8b_sdhc.hw, > + [CLKID_STREAM] = &meson8b_stream.hw, > + [CLKID_ASYNC_FIFO] = &meson8b_async_fifo.hw, > + [CLKID_SDIO]= &meson8b_sdio.hw, > + [CLKID_ABUF]= &meson8b_abuf.hw, > + [CLKID_HIU_IFACE] = &meson8b_hiu_iface.hw, > + [CLKID_ASSIST_MISC] = &meson8b_assist_misc.hw, > + [CLKID_SPI] = &meson8b_spi.hw, > + [CLKID_I2S_SPDIF] = &meson8b_i2s_spdif.hw, > + [CLKID_ETH] = &meson8b_eth.hw, > + [CLKID_DEMUX] = &meson8b_demux.hw, > + [CLKID_AIU_GLUE]= &meson8b_aiu_glue.hw, > + [CLKID_IEC958] = &meson8b_iec958.hw, > + [CLKID_I2S_OUT] = &meson8b_i2s_out.hw, > + [CLKID_AMCLK] = &meson8b_amclk.hw, > + [CLKID_AIFIFO2] = &meson8b_aififo2.hw, > + [CLKID_MIXER] = &meson8b_mixer.hw, > + [CLKID_MIXER_IFACE] = &meson8b_mixer_iface.hw, > + [CLKID_ADC] = &meson8b_adc.hw, > + [CLKID_BLKMV] = &meson8b_blkmv.hw, > + [CLKID_AIU] = &meson8b_aiu.hw, > + [CLKID_UART1] = &meson8b_uart1.hw, > + [CLKID_G2D] = &meson8b_g2d.hw, > + [CLKID_USB0]= &meson8b_usb0.hw, > + [CLKID_USB1]= &meson8b_usb1.hw, > + [CLKID_RESET] = &meson8b_reset.hw, > + [CLKID_NAND]= &meson8b_nand.hw, > + [CLKID_DOS_PARSER] = &meson8b_dos_parser.hw, > + [CLKID_USB] = &meson8b_usb.hw, > + [CLKID_VDIN1] = &meson8b_vdin1.hw, > + [CLKID_AHB_ARB0]
Re: [PATCH] kvm: arm: Fix handling of stage2 huge mappings
Hi Suzuki, On Tue, 19 Mar 2019 14:11:08 +, Suzuki K Poulose wrote: > > We rely on the mmu_notifier call backs to handle the split/merge > of huge pages and thus we are guaranteed that, while creating a > block mapping, either the entire block is unmapped at stage2 or it > is missing permission. > > However, we miss a case where the block mapping is split for dirty > logging case and then could later be made block mapping, if we cancel the > dirty logging. This not only creates inconsistent TLB entries for > the pages in the the block, but also leakes the table pages for > PMD level. > > Handle this corner case for the huge mappings at stage2 by > unmapping the non-huge mapping for the block. This could potentially > release the upper level table. So we need to restart the table walk > once we unmap the range. > > Fixes : ad361f093c1e31d ("KVM: ARM: Support hugetlbfs backed huge pages") > Reported-by: Zheng Xiang > Cc: Zheng Xiang > Cc: Zhengui Yu > Cc: Marc Zyngier > Cc: Christoffer Dall > Signed-off-by: Suzuki K Poulose > --- > virt/kvm/arm/mmu.c | 63 > ++ > 1 file changed, 45 insertions(+), 18 deletions(-) > > diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c > index fce0983..6ad6f19d 100644 > --- a/virt/kvm/arm/mmu.c > +++ b/virt/kvm/arm/mmu.c > @@ -1060,25 +1060,43 @@ static int stage2_set_pmd_huge(struct kvm *kvm, > struct kvm_mmu_memory_cache > { > pmd_t *pmd, old_pmd; > > +retry: > pmd = stage2_get_pmd(kvm, cache, addr); > VM_BUG_ON(!pmd); > > old_pmd = *pmd; > + /* > + * Multiple vcpus faulting on the same PMD entry, can > + * lead to them sequentially updating the PMD with the > + * same value. Following the break-before-make > + * (pmd_clear() followed by tlb_flush()) process can > + * hinder forward progress due to refaults generated > + * on missing translations. > + * > + * Skip updating the page table if the entry is > + * unchanged. > + */ > + if (pmd_val(old_pmd) == pmd_val(*new_pmd)) > + return 0; > + > if (pmd_present(old_pmd)) { > /* > - * Multiple vcpus faulting on the same PMD entry, can > - * lead to them sequentially updating the PMD with the > - * same value. Following the break-before-make > - * (pmd_clear() followed by tlb_flush()) process can > - * hinder forward progress due to refaults generated > - * on missing translations. > + * If we already have PTE level mapping for this block, > + * we must unmap it to avoid inconsistent TLB state and > + * leaking the table page. We could end up in this situation > + * if the memory slot was marked for dirty logging and was > + * reverted, leaving PTE level mappings for the pages accessed > + * during the period. So, unmap the PTE level mapping for this > + * block and retry, as we could have released the upper level > + * table in the process. >* > - * Skip updating the page table if the entry is > - * unchanged. > + * Normal THP split/merge follows mmu_notifier callbacks and do > + * get handled accordingly. >*/ > - if (pmd_val(old_pmd) == pmd_val(*new_pmd)) > - return 0; > - > + if (!pmd_thp_or_huge(old_pmd)) { > + unmap_stage2_range(kvm, addr & S2_PMD_MASK, > S2_PMD_SIZE); > + goto retry; This looks slightly dodgy. Doing this retry results in another call to stage2_get_pmd(), which may or may not result in allocating a PUD. I think this is safe as if we managed to get here, it means the whole hierarchy was already present and nothing was allocated in the first round. Somehow, I would feel more comfortable with just not even trying. Unmap, don't fix the fault, let the vcpu come again for additional punishment. But this is probably more invasive, as none of the stage2_set_p*() return value is ever evaluated. Oh well. > + } > /* >* Mapping in huge pages should only happen through a >* fault. If a page is merged into a transparent huge > @@ -1090,8 +1108,7 @@ static int stage2_set_pmd_huge(struct kvm *kvm, struct > kvm_mmu_memory_cache >* should become splitting first, unmapped, merged, >* and mapped back in on-demand. >*/ > - VM_BUG_ON(pmd_pfn(old_pmd) != pmd_pfn(*new_pmd)); > - > + WARN_ON_ONCE(pmd_pfn(old_pmd) != pmd_pfn(*new_pmd)); > pmd_clear(pmd); > kvm_tlb_flush_vmid_ipa(kvm, addr); > } else { > @@ -1107,6 +1124,7 @@ static int stage2_set_pud_huge(struct kvm *kvm, struct > kvm_mmu_memory_cache *cac > { > pud_t *pudp, old
Re: [PATCH 3/4] clk: meson: meson8b: add support for the GP_PLL clock on Meson8m2
On 19/03/2019 22:51, Martin Blumenstingl wrote: > Meson8m2 has a GP_PLL clock (similar to GP0_PLL on GXBB/GXL/GXM) which > is used as input for the VPU clocks. > The only supported frequency (based on Amlogic's vendor kernel sources) > is 364MHz which is achieved using the following parameters: > - input: XTAL (24MHz) > - M = 182 > - N = 3 > - OD = 2 ^ 2 > > Signed-off-by: Martin Blumenstingl > --- > drivers/clk/meson/meson8b.c | 62 + > drivers/clk/meson/meson8b.h | 5 ++- > 2 files changed, 66 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > index c9e6ec67d649..0d08f1ef7af8 100644 > --- a/drivers/clk/meson/meson8b.c > +++ b/drivers/clk/meson/meson8b.c > @@ -1703,6 +1703,64 @@ static struct clk_regmap meson8b_mali = { > }, > }; > > +static const struct pll_params_table meson8m2_gp_pll_params_table[] = { > + PLL_PARAMS(182, 3), > + { /* sentinel */ }, > +}; > + > +static struct clk_regmap meson8m2_gp_pll_dco = { > + .data = &(struct meson_clk_pll_data){ > + .en = { > + .reg_off = HHI_GP_PLL_CNTL, > + .shift = 30, > + .width = 1, > + }, > + .m = { > + .reg_off = HHI_GP_PLL_CNTL, > + .shift = 0, > + .width = 9, > + }, > + .n = { > + .reg_off = HHI_GP_PLL_CNTL, > + .shift = 9, > + .width = 5, > + }, > + .l = { > + .reg_off = HHI_GP_PLL_CNTL, > + .shift = 31, > + .width = 1, > + }, > + .rst = { > + .reg_off = HHI_GP_PLL_CNTL, > + .shift = 29, > + .width = 1, > + }, > + .table = meson8m2_gp_pll_params_table, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "gp_pll_dco", > + .ops = &meson_clk_pll_ops, > + .parent_names = (const char *[]){ "xtal" }, > + .num_parents = 1, > + }, > +}; > + > +static struct clk_regmap meson8m2_gp_pll = { > + .data = &(struct clk_regmap_div_data){ > + .offset = HHI_GP_PLL_CNTL, > + .shift = 16, > + .width = 2, > + .flags = CLK_DIVIDER_POWER_OF_TWO, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "gp_pll", > + .ops = &clk_regmap_divider_ops, > + .parent_names = (const char *[]){ "gp_pll_dco" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > /* Everything Else (EE) domain gates */ > > static MESON_GATE(meson8b_ddr, HHI_GCLK_MPEG0, 0); > @@ -2338,6 +2396,8 @@ static struct clk_hw_onecell_data > meson8m2_hw_onecell_data = { > [CLKID_MALI_1_DIV] = &meson8b_mali_1_div.hw, > [CLKID_MALI_1] = &meson8b_mali_1.hw, > [CLKID_MALI]= &meson8b_mali.hw, > + [CLKID_GP_PLL_DCO] = &meson8m2_gp_pll_dco.hw, > + [CLKID_GP_PLL] = &meson8m2_gp_pll.hw, > [CLK_NR_CLKS] = NULL, > }, > .num = CLK_NR_CLKS, > @@ -2500,6 +2560,8 @@ static struct clk_regmap *const meson8b_clk_regmaps[] = > { > &meson8b_mali_1_div, > &meson8b_mali_1, > &meson8b_mali, > + &meson8m2_gp_pll_dco, > + &meson8m2_gp_pll, > }; > > static const struct meson8b_clk_reset_line { > diff --git a/drivers/clk/meson/meson8b.h b/drivers/clk/meson/meson8b.h > index b8c58faeae52..a45f7102c558 100644 > --- a/drivers/clk/meson/meson8b.h > +++ b/drivers/clk/meson/meson8b.h > @@ -19,6 +19,7 @@ > * > * [0] > http://dn.odroid.com/S805/Datasheet/S805_Datasheet%20V0.8%2020150126.pdf > */ > +#define HHI_GP_PLL_CNTL 0x40 /* 0x10 offset in data > sheet */ > #define HHI_VIID_CLK_DIV 0x128 /* 0x4a offset in data sheet */ > #define HHI_VIID_CLK_CNTL0x12c /* 0x4b offset in data sheet */ > #define HHI_GCLK_MPEG0 0x140 /* 0x50 offset in data > sheet */ > @@ -146,8 +147,10 @@ > #define CLKID_MALI_1_SEL 178 > #define CLKID_MALI_1_DIV 179 > #define CLKID_MALI_1 180 > +#define CLKID_GP_PLL_DCO 181 > +#define CLKID_GP_PLL 182 > > -#define CLK_NR_CLKS 181 > +#define CLK_NR_CLKS 183 > > /* > * include the CLKID and RESETID that have > Reviewed-by: Neil Armstrong
Re: [PATCH] mm: mempolicy: make mbind() return -EIO when MPOL_MF_STRICT is specified
On Wed, Mar 20, 2019 at 02:35:56AM +0800, Yang Shi wrote: > Fixes: 6f4576e3687b ("mempolicy: apply page table walker on > queue_pages_range()") > Reported-by: Cyril Hrubis > Cc: Vlastimil Babka > Cc: sta...@vger.kernel.org > Suggested-by: Kirill A. Shutemov > Signed-off-by: Yang Shi > Signed-off-by: Oscar Salvador Hi Yang, thanks for the patch. Some observations below. > } > page = pmd_page(*pmd); > @@ -473,8 +480,15 @@ static int queue_pages_pmd(pmd_t *pmd, spinlock_t *ptl, > unsigned long addr, > ret = 1; > flags = qp->flags; > /* go to thp migration */ > - if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) > + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) { > + if (!vma_migratable(walk->vma)) { > + ret = -EIO; > + goto unlock; > + } > + > migrate_page_add(page, qp->pagelist, flags); > + } else > + ret = -EIO; if (!(flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) || !vma_migratable(walk->vma)) { ret = -EIO; goto unlock; } migrate_page_add(page, qp->pagelist, flags); unlock: spin_unlock(ptl); out: return ret; seems more clean to me? > unlock: > spin_unlock(ptl); > out: > @@ -499,8 +513,10 @@ static int queue_pages_pte_range(pmd_t *pmd, unsigned > long addr, > ptl = pmd_trans_huge_lock(pmd, vma); > if (ptl) { > ret = queue_pages_pmd(pmd, ptl, addr, end, walk); > - if (ret) > + if (ret > 0) > return 0; > + else if (ret < 0) > + return ret; I would go with the following, but that's a matter of taste I guess. if (ret < 0) return ret; else return 0; > } > > if (pmd_trans_unstable(pmd)) > @@ -521,11 +537,16 @@ static int queue_pages_pte_range(pmd_t *pmd, unsigned > long addr, > continue; > if (!queue_pages_required(page, qp)) > continue; > - migrate_page_add(page, qp->pagelist, flags); > + if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) { > + if (!vma_migratable(vma)) > + break; > + migrate_page_add(page, qp->pagelist, flags); > + } else > + break; I might be missing something, but AFAICS neither vma nor flags is going to change while we are in queue_pages_pte_range(), so, could not we move the check just above the loop? In that way, 1) we only perform the check once and 2) if we enter the loop we know that we are going to do some work, so, something like: index af171ccb56a2..7c0e44389826 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -487,6 +487,9 @@ static int queue_pages_pte_range(pmd_t *pmd, unsigned long addr, if (pmd_trans_unstable(pmd)) return 0; + if (!(flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) || !vma_migratable(vma)) + return -EIO; + pte = pte_offset_map_lock(walk->mm, pmd, addr, &ptl); for (; addr != end; pte++, addr += PAGE_SIZE) { if (!pte_present(*pte)) > } > pte_unmap_unlock(pte - 1, ptl); > cond_resched(); > - return 0; > + return addr != end ? -EIO : 0; If we can do the above, we can leave the return value as it was. -- Oscar Salvador SUSE L3
[PATCH v3] tracing: eliminate const char[] auto variables
Automatic const char[] variables cause unnecessary code generation. For example, the this_mod variable leads to 3f04: 48 b8 5f 5f 74 68 69 73 5f 6d movabs $0x6d5f736968745f5f,%rax # __this_m 3f0e: 4c 8d 44 24 02 lea0x2(%rsp),%r8 3f13: 48 8d 7c 24 10 lea0x10(%rsp),%rdi 3f18: 48 89 44 24 02 mov%rax,0x2(%rsp) 3f1d: 4c 89 e9mov%r13,%rcx 3f20: b8 65 00 00 00 mov$0x65,%eax # e 3f25: 48 c7 c2 00 00 00 00mov$0x0,%rdx 3f28: R_X86_64_32S .rodata.str1.1+0x18d 3f2c: be 48 00 00 00 mov$0x48,%esi 3f31: c7 44 24 0a 6f 64 75 6c movl $0x6c75646f,0xa(%rsp) # odul 3f39: 66 89 44 24 0e mov%ax,0xe(%rsp) i.e., the string gets built on the stack at runtime. Similar code can be found for the other instances I'm replacing here. Putting the string in .rodata reduces the combined .text+.rodata size and saves time and stack space at runtime. The simplest fix, and what I've done for the this_mod case, is to just make the variable static. However, for the "" case where the same string is used twice, that prevents the linker from merging those two literals, so instead use a macro - that also keeps the two instances automatically in sync (instead of only the compile-time strlen expression). Finally, for the two runs of spaces, it turns out that the "build these strings on the stack" is not the worst part of what gcc does - it turns print_func_help_header_irq() into "if (tgid) { /* print_event_info + five seq_printf calls */ } else { /* print event_info + another five seq_printf */}". Taking inspiration from a suggestion from Al Viro, use %.*s to make snprintf either stop after the first two spaces or print the whole string. As a bonus, the seq_printfs now fit on single lines (at least, they are not longer than the existing ones in the function just above), making it easier to see that the ascii art lines up. x86-64 defconfig + CONFIG_FUNCTION_TRACER: $ scripts/stackdelta /tmp/stackusage.{0,1} ./kernel/trace/ftrace.c ftrace_mod_callback 152 136 -16 ./kernel/trace/trace.c trace_default_header56 32 -24 ./kernel/trace/trace.c tracing_mark_raw_write 96 72 -24 ./kernel/trace/trace.c tracing_mark_write 104 80 -24 bloat-o-meter add/remove: 1/0 grow/shrink: 0/4 up/down: 14/-375 (-361) Function old new delta this_mod - 14 +14 ftrace_mod_callback 577 542 -35 tracing_mark_raw_write 444 374 -70 tracing_mark_write 616 540 -76 trace_default_header 600 406-194 Signed-off-by: Rasmus Villemoes --- I played around with various versions using field width, but the use of tgid-dependent strings in the last two cases made gcc keep iffyfying the whole thing. So I came up with this instead. kernel/trace/ftrace.c | 2 +- kernel/trace/trace.c | 34 +- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index fa79323331b2..e232dd31cfae 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -3879,7 +3879,7 @@ static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops, static bool module_exists(const char *module) { /* All modules have the symbol __this_module */ - const char this_mod[] = "__this_module"; + static const char this_mod[] = "__this_module"; char modname[MAX_PARAM_PREFIX_LEN + sizeof(this_mod) + 2]; unsigned long val; int n; diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 21153e64bf1c..b869e5a0dc79 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -3556,25 +3556,18 @@ static void print_func_help_header_irq(struct trace_buffer *buf, struct seq_file unsigned int flags) { bool tgid = flags & TRACE_ITER_RECORD_TGID; - const char tgid_space[] = " "; - const char space[] = " "; + const char *space = " "; + int prec = tgid ? 10 : 2; print_event_info(buf, m); - seq_printf(m, "# %s _-=> irqs-off\n", - tgid ? tgid_space : space); - seq_printf(m, "# %s / _=> need-resched\n", - tgid ? tgid_space : space); - seq_printf(m, "# %s| / _---=> hardirq/softirq\n", - tgid ? tgid_space : space); - seq_printf(m, "# %s|| / _--=> preempt-depth\n", - tgid ? tgid_space : space); - seq_printf(m, "#
Re: [PATCH 4/4] clk: meson: meson8b: add the VPU clock trees
On 19/03/2019 22:51, Martin Blumenstingl wrote: > The VPU clock tree is slightly different on all three supported SoCs: > > Meson8 only has an input mux (which chooses between "fclk_div4", > "fclk_div3", "fclk_div5" and "fclk_div7"), a divider and a gate. > > Meson8b has two VPU clock trees, each with an input mux (using the same > parents as the input mux on Meson8), divider and a gates. The final VPU > clock is a glitch-free mux which chooses between VPU_1 and VPU_2. > > Meson8m2 uses a similar clock tree as Meson8b but the last input clock > is different: instead of using "fclk_div7" as input Meson8m2 uses > "gp_pll". This was probably done in hardware to improve the accuracy of > the clock because fclk_div7 gives us 2550MHz / 7 = 364.286MHz while > GP_PLL can achieve 364.0MHz. This could be a reason, but they only implement frequency scaling for the VPU in the recent 4.9 kernel releases... but we should also implement it upstream once we solve how to switch these dual clocks in a clean fashion. > > Signed-off-by: Martin Blumenstingl > --- > drivers/clk/meson/meson8b.c | 167 > drivers/clk/meson/meson8b.h | 9 +- > 2 files changed, 175 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > index 0d08f1ef7af8..8e091c2d10e6 100644 > --- a/drivers/clk/meson/meson8b.c > +++ b/drivers/clk/meson/meson8b.c > @@ -1761,6 +1761,147 @@ static struct clk_regmap meson8m2_gp_pll = { > }, > }; > > +static const char * const mmeson8b_vpu_0_1_parent_names[] = { > + "fclk_div4", "fclk_div3", "fclk_div5", "fclk_div7" > +}; > + > +static const char * const mmeson8m2_vpu_0_1_parent_names[] = { > + "fclk_div4", "fclk_div3", "fclk_div5", "gp_pll" > +}; > + > +static struct clk_regmap meson8b_vpu_0_sel = { > + .data = &(struct clk_regmap_mux_data){ > + .offset = HHI_VPU_CLK_CNTL, > + .mask = 0x3, > + .shift = 9, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "vpu_0_sel", > + .ops = &clk_regmap_mux_ops, > + .parent_names = mmeson8b_vpu_0_1_parent_names, > + .num_parents = ARRAY_SIZE(mmeson8b_vpu_0_1_parent_names), > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8m2_vpu_0_sel = { > + .data = &(struct clk_regmap_mux_data){ > + .offset = HHI_VPU_CLK_CNTL, > + .mask = 0x3, > + .shift = 9, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "vpu_0_sel", > + .ops = &clk_regmap_mux_ops, > + .parent_names = mmeson8m2_vpu_0_1_parent_names, > + .num_parents = ARRAY_SIZE(mmeson8m2_vpu_0_1_parent_names), > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_vpu_0_div = { > + .data = &(struct clk_regmap_div_data){ > + .offset = HHI_VPU_CLK_CNTL, > + .shift = 0, > + .width = 7, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "vpu_0_div", > + .ops = &clk_regmap_divider_ops, > + .parent_names = (const char *[]){ "vpu_0_sel" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_vpu_0 = { > + .data = &(struct clk_regmap_gate_data){ > + .offset = HHI_VPU_CLK_CNTL, > + .bit_idx = 8, > + }, > + .hw.init = &(struct clk_init_data) { > + .name = "vpu_0", > + .ops = &clk_regmap_gate_ops, > + .parent_names = (const char *[]){ "vpu_0_div" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_vpu_1_sel = { > + .data = &(struct clk_regmap_mux_data){ > + .offset = HHI_VPU_CLK_CNTL, > + .mask = 0x3, > + .shift = 25, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "vpu_1_sel", > + .ops = &clk_regmap_mux_ops, > + .parent_names = mmeson8b_vpu_0_1_parent_names, > + .num_parents = ARRAY_SIZE(mmeson8b_vpu_0_1_parent_names), > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8m2_vpu_1_sel = { > + .data = &(struct clk_regmap_mux_data){ > + .offset = HHI_VPU_CLK_CNTL, > + .mask = 0x3, > + .shift = 25, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "vpu_1_sel", > + .ops = &clk_regmap_mux_ops, > + .parent_names = mmeson8m2_vpu_0_1_parent_names, > + .num_parents = ARRAY_SIZE(mmeson8m2_vpu_0_1_parent_names), > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_vpu_1_div = { > + .data = &(struct clk_regmap_d
Re: [PATCH V7 2/5] pwm: Add i.MX TPM PWM driver support
Hello, On Wed, Mar 20, 2019 at 05:06:21AM +, Anson Huang wrote: > i.MX7ULP has TPM(Low Power Timer/Pulse Width Modulation Module) > inside, it can support multiple PWM channels, all the channels > share same counter and period setting, but each channel can > configure its duty and polarity independently. > > There are several TPM modules in i.MX7ULP, the number of channels > in TPM modules are different, it can be read from each TPM module's > PARAM register. > > Signed-off-by: Anson Huang > --- > Changes since V6: > - merge "config" and "enable" functions into ONE function > pwm_imx_tpm_apply_hw; > - save computation for confiuring counter, the "round_state" function > will return > the registers value directly; > - improve the logic in .apply; > - return error when there is still PWM active during suspend callback. > --- > drivers/pwm/Kconfig | 11 ++ > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-imx-tpm.c | 428 > ++ > 3 files changed, 440 insertions(+) > create mode 100644 drivers/pwm/pwm-imx-tpm.c > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index 54f8238..3ea0391 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -210,6 +210,17 @@ config PWM_IMX27 > To compile this driver as a module, choose M here: the module > will be called pwm-imx27. > > +config PWM_IMX_TPM > + tristate "i.MX TPM PWM support" > + depends on ARCH_MXC || COMPILE_TEST > + depends on HAVE_CLK && HAS_IOMEM > + help > + Generic PWM framework driver for i.MX7ULP TPM module, TPM's full > + name is Low Power Timer/Pulse Width Modulation Module. > + > + To compile this driver as a module, choose M here: the module > + will be called pwm-imx-tpm. > + > config PWM_JZ4740 > tristate "Ingenic JZ47xx PWM support" > depends on MACH_INGENIC > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > index 448825e..c368599 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_PWM_HIBVT) += pwm-hibvt.o > obj-$(CONFIG_PWM_IMG)+= pwm-img.o > obj-$(CONFIG_PWM_IMX1) += pwm-imx1.o > obj-$(CONFIG_PWM_IMX27) += pwm-imx27.o > +obj-$(CONFIG_PWM_IMX_TPM)+= pwm-imx-tpm.o > obj-$(CONFIG_PWM_JZ4740) += pwm-jz4740.o > obj-$(CONFIG_PWM_LP3943) += pwm-lp3943.o > obj-$(CONFIG_PWM_LPC18XX_SCT)+= pwm-lpc18xx-sct.o > diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c > new file mode 100644 > index 000..02403d0 > --- /dev/null > +++ b/drivers/pwm/pwm-imx-tpm.c > @@ -0,0 +1,428 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright 2018-2019 NXP. > + * > + * Limitations: > + * - The TPM counter and period counter are shared between > + * multiple channels, so all channels should use same period > + * settings. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define PWM_IMX_TPM_PARAM0x4 > +#define PWM_IMX_TPM_GLOBAL 0x8 > +#define PWM_IMX_TPM_SC 0x10 > +#define PWM_IMX_TPM_CNT 0x14 > +#define PWM_IMX_TPM_MOD 0x18 > +#define PWM_IMX_TPM_CnSC(n) (0x20 + (n) * 0x8) > +#define PWM_IMX_TPM_CnV(n) (0x24 + (n) * 0x8) > + > +#define PWM_IMX_TPM_PARAM_CHAN GENMASK(7, 0) > + > +#define PWM_IMX_TPM_SC_PSGENMASK(2, 0) > +#define PWM_IMX_TPM_SC_CMOD GENMASK(4, 3) > +#define PWM_IMX_TPM_SC_CMOD_INC_EVERY_CLKBIT(3) #define PWM_IMX_TPM_SC_CMOD_INC_EVERY_CLK FIELD_PREP(PWM_IMX_TPM_SC_CMOD, 1) ? > +#define PWM_IMX_TPM_SC_CPWMS BIT(5) > + > +#define PWM_IMX_TPM_CnSC_CHF BIT(7) > +#define PWM_IMX_TPM_CnSC_MSB BIT(5) > +#define PWM_IMX_TPM_CnSC_MSA BIT(4) > + > +/* > + * The reference manual describes this field as two separate bits. The > + * samantic of the two bits isn't orthogonal though, so they are treated > + * together as a 2-bit field here. > + */ > +#define PWM_IMX_TPM_CnSC_ELS GENMASK(3, 2) > +#define PWM_IMX_TPM_CnSC_ELS_POLARITY_INVERSED 0x1 > + > +#define PWM_IMX_TPM_MOD_MOD GENMASK(15, 0) > + > +struct imx_tpm_pwm_chip { > + struct pwm_chip chip; > + struct clk *clk; > + void __iomem *base; > + struct mutex lock; > + u32 user_count; > + u32 enable_count; > + u32 real_period; > +}; > + > +struct imx_tpm_pwm_param { > + u8 prescale; > + u32 mod; > +}; > + > +static inline struct imx_tpm_pwm_chip *to_imx_tpm_pwm_chip(struct pwm_chip > *chip) > +{ > + return container_of(chip, struct imx_tpm_pwm_chip, chip); > +} > + > +static int pwm_imx_tpm_round_state(struct pwm_chip *chip, > +struct imx_tpm_pwm_param *p, > +struct pwm_state *state, > +
Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
On Fri, Mar 08, 2019 at 09:57:09AM +, Andy Tang wrote: > > > > -Original Message- > > From: Daniel Lezcano > > Sent: 2019年3月8日 17:28 > > To: Andy Tang ; Shawn Guo > > Cc: Leo Li ; robh...@kernel.org; mark.rutl...@arm.com; > > linux-arm-ker...@lists.infradead.org; devicet...@vger.kernel.org; > > linux-kernel@vger.kernel.org; linux...@vger.kernel.org; rui.zh...@intel.com; > > edubez...@gmail.com > > Subject: Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node > > > > On 08/03/2019 03:07, Andy Tang wrote: > > > > > > > > >> -Original Message- > > >> From: Daniel Lezcano > > >> Sent: 2019年3月7日 17:15 > > >> To: Andy Tang ; Shawn Guo > > >> Cc: Leo Li ; robh...@kernel.org; > > >> mark.rutl...@arm.com; linux-arm-ker...@lists.infradead.org; > > >> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; > > >> linux...@vger.kernel.org; rui.zh...@intel.com; edubez...@gmail.com > > >> Subject: Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal > > >> zone node > > >> > > > PS: In order to keep consistency to the first thermal-zone node, > > > there will be "WARNING: line over 80 characters" warnings. > > > > > > arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 43 > > +-- > > > 1 files changed, 39 insertions(+), 4 deletions(-) > > > > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi > > > b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi > > > index 661137f..9f52bc9 100644 > > > --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi > > > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi > > > @@ -129,19 +129,19 @@ > > > }; > > > > > > thermal-zones { > > > - cpu_thermal: cpu-thermal { > > > + ccu { > > > > Is this change really necessary? What does 'ccu' stand for? > > >>> I think so. ccu stands for core cluster unit. cpu is too general. > > >>> On some platforms, there are more than one core clusters. > > >>> At least we should change it to "core cluster" if short form is not > > appropriate. > > >> > > >> If the sensor is a the cluster level, 'cluster' is enough. IMHO, no > > >> need to give a description of what contains the cluster, otherwise > > >> you will end up with a 'core-gpu-cluster-l2' name. > > > If cluster is specific to core, we can use cluster instead. But I don't > > > think so. > > > Cluster may refer to "core cluster", "GPU cluster" etc. > > > So, I think "core-cluster" is ok. > > > If core was divided to several clusters, we can name it as > > > "core-cluster1", > > "core-cluster2" etc. > > > If GPU was divided to several clusters we can name it as "gpu-cluster1", > > "gpu-cluster2" etc. > > > > > > Hi Andy, > > > > I think there is a confusion around the 'cpu' term and 'cluster'. > > > > ARM would like to see the 'cluster' word to disappear, so whenever possible > > we > > should avoid it. > > > > From the hardware side, 'CPU' is usually used to describe the physical chip > > containing the cores+cache. > > > > From the software side, 'CPU' is usually used to describe the logical > > process > > unit, aka a core or a hyper-thread. > > > > As we are in the DT, so describing the hardware, the CPU refers to the group > > cores+caches. > > > > From my POV, using 'cpu' for the group of cores and 'gpu' for the graphic > > sounds ok, and so far that is what is used for the other platforms. > > > > If you change the name, that may give the feeling there is something special > > with those thermal zones. > > Thanks Daniel for your detailed explanations. > > But as you said 'CPU' is usually used to describe the physical chip. Here is how I would understand Daniel's comments: CPU = cores + caches physical chip = SoC = CPU + GPU + peripherals ... > So if we name it as CPU, it sounds like this temperature sensor is monitoring > the whole chip. > That's not true in our case. > > Take ls2088a for example: > In ls2088a SoC, there are 7 temperature sensors. Please note that they are > all located in SoC. > The placement of the temperature sensors are showed below: > > Sensor ID placement > 1 DDR controller 1 > 2 DDR controller 2 > 3 DDR controller 3 > 4 core cluster 1 > 5 core cluster 2 > 6 core cluster 3 > 7 core cluster 4 > > Apparently using CPU or CPU-cluster is not appropriate. Core-cluster is > better. So using CPU is appropriate for me, less confusing, more consistent with other platforms. Shawn
Re: [PATCH 2/2] clk: meson: meson8b: add the video decoder clock trees
On 19/03/2019 23:00, Martin Blumenstingl wrote: > This adds the four video decoder clock trees. > > VDEC_1 is split into two paths on Meson8b and Meson8m2: > - input mux called "vdec_1_sel" > - two dividers ("vdec_1_1_div" and "vdec_1_2_div") and gates ("vdec_1_1" > and "vdec_1_2") > - and an output mux (probably glitch-free) called "vdec_1" > On Meson8 the VDEC_1 tree is simpler because there's only one path: > - input mux called "vdec_1_sel" > - divider ("vdec_1_1_div") and gate ("vdec_1_1") > - (the gate is used as output directly, there's no mux) Interesting to see they kept all the dual clocks designs since Meson8b. > > The VDEC_HCODEC and VDEC_2 clocks are simple composite clocks each > consisting of an input mux, divider and a gate. > > The VDEC_HEVC clock seems to have two paths similar to the VDEC_1 clock. > However, the register offsets of the second clock path is not known. > Amlogic's 3.10 kernel (which is used as reference) sets > HHI_VDEC2_CLK_CNTL[31] to 1 before changing the VDEC_HEVC clock and back > to 0 afterwards. For now, leave a TODO comment and only add the first > path. The second path is maybe broken, who knows ! > > Signed-off-by: Martin Blumenstingl > --- > drivers/clk/meson/meson8b.c | 312 > drivers/clk/meson/meson8b.h | 17 +- > 2 files changed, 328 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > index 8e091c2d10e6..37cf0f01bb5d 100644 > --- a/drivers/clk/meson/meson8b.c > +++ b/drivers/clk/meson/meson8b.c > @@ -1902,6 +1902,257 @@ static struct clk_regmap meson8b_vpu = { > }, > }; > > +static const char * const meson8b_vdec_parent_names[] = { > + "fclk_div4", "fclk_div3", "fclk_div5", "fclk_div7", "mpll2", "mpll1" > +}; > + > +static struct clk_regmap meson8b_vdec_1_sel = { > + .data = &(struct clk_regmap_mux_data){ > + .offset = HHI_VDEC_CLK_CNTL, > + .mask = 0x3, > + .shift = 9, > + .flags = CLK_MUX_ROUND_CLOSEST, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "vdec_1_sel", > + .ops = &clk_regmap_mux_ops, > + .parent_names = meson8b_vdec_parent_names, > + .num_parents = ARRAY_SIZE(meson8b_vdec_parent_names), > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_vdec_1_1_div = { > + .data = &(struct clk_regmap_div_data){ > + .offset = HHI_VDEC_CLK_CNTL, > + .shift = 0, > + .width = 7, > + .flags = CLK_DIVIDER_ROUND_CLOSEST, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "vdec_1_1_div", > + .ops = &clk_regmap_divider_ops, > + .parent_names = (const char *[]){ "vdec_1_sel" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_vdec_1_1 = { > + .data = &(struct clk_regmap_gate_data){ > + .offset = HHI_VDEC_CLK_CNTL, > + .bit_idx = 8, > + }, > + .hw.init = &(struct clk_init_data) { > + .name = "vdec_1_1", > + .ops = &clk_regmap_gate_ops, > + .parent_names = (const char *[]){ "vdec_1_1_div" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_vdec_1_2_div = { > + .data = &(struct clk_regmap_div_data){ > + .offset = HHI_VDEC3_CLK_CNTL, > + .shift = 0, > + .width = 7, > + .flags = CLK_DIVIDER_ROUND_CLOSEST, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "vdec_1_2_div", > + .ops = &clk_regmap_divider_ops, > + .parent_names = (const char *[]){ "vdec_1_sel" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_vdec_1_2 = { > + .data = &(struct clk_regmap_gate_data){ > + .offset = HHI_VDEC3_CLK_CNTL, > + .bit_idx = 8, > + }, > + .hw.init = &(struct clk_init_data) { > + .name = "vdec_1_2", > + .ops = &clk_regmap_gate_ops, > + .parent_names = (const char *[]){ "vdec_1_2_div" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_vdec_1 = { > + .data = &(struct clk_regmap_mux_data){ > + .offset = HHI_VDEC3_CLK_CNTL, > + .mask = 0x1, > + .shift = 15, > + .flags = CLK_MUX_ROUND_CLOSEST, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "vdec_1", > + .ops = &clk_regmap_mux_ops, > + .parent_names = (const char *[]){ "vdec_1_1", "vdec_1_2" }, > + .num_parents = 2, > + .flags = CLK_SET_RATE_PA
Re: [PATCH] x86/Hyper-V: Fix definition HV_MAX_FLUSH_REP_COUNT
On Fri, 15 Mar 2019, Paolo Bonzini wrote: > On 22/02/19 11:48, lantianyu1...@gmail.com wrote: > > From: Lan Tianyu > > > > The max flush rep count of HvFlushGuestPhysicalAddressList hypercall > > is equal with how many entries of union hv_gpa_page_range can be populated > > into the input parameter page. The origin code lacks parenthesis around > > PAGE_SIZE - 2 * sizeof(u64). This patch is to fix it. > > > > Cc: > > Fixs: cc4edae4b9(x86/hyper-v: Add HvFlushGuestAddressList hypercall support) > > Signed-off-by: Lan Tianyu > > --- > > arch/x86/include/asm/hyperv-tlfs.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/x86/include/asm/hyperv-tlfs.h > > b/arch/x86/include/asm/hyperv-tlfs.h > > index 705dafc2d11a..2bdbbbcfa393 100644 > > --- a/arch/x86/include/asm/hyperv-tlfs.h > > +++ b/arch/x86/include/asm/hyperv-tlfs.h > > @@ -841,7 +841,7 @@ union hv_gpa_page_range { > > * count is equal with how many entries of union hv_gpa_page_range can > > * be populated into the input parameter page. > > */ > > -#define HV_MAX_FLUSH_REP_COUNT (PAGE_SIZE - 2 * sizeof(u64) / \ > > +#define HV_MAX_FLUSH_REP_COUNT ((PAGE_SIZE - 2 * sizeof(u64)) /\ > > sizeof(union hv_gpa_page_range)) > > > > struct hv_guest_mapping_flush_list { > > > > Queued for after he merge window, with the fixed "Fixes" tag. That's upstream already: 9cd05ad2910b ("x86/hyper-v: Fix definition of HV_MAX_FLUSH_REP_COUNT") Thanks, tglx
[PATCH] arm64: dts: juno: Enable smmu_pcie for Juno r1/r2
Though PCIe controller has been enabled on Juno r1/r2, but it misses to enable its connected SMMU. From the testing, even without set this SMMU status property to 'okay', the PCIe NIC device still works well. Since the SMMU is not enabled in DT binding and its iommu_groups is not created properly, finally this prevents to enable vfio-pci for NIC device with KVM. After reviewing the code in dts, Juno r1/r2 both have enabled PCIe controller but Juno r0 board (in juno.dts) doesn't contain PCIe controller; hence this patch only change SMMU status property to 'okay' for Juno r1/r2 dts files for the sake of only enabling it for Juno r1/r2 and avoiding touching Juno r0. Committer testing on Juno r2: Before: root@debian:~# tree /sys/kernel/iommu_groups/ /sys/kernel/iommu_groups/ ├── 0 │ ├── devices │ │ ├── 7ffb.ohci -> ../../../../devices/platform/7ffb.ohci │ │ └── 7ffc.ehci -> ../../../../devices/platform/7ffc.ehci │ ├── reserved_regions │ └── type └── 1 ├── devices │ └── 2007.etr -> ../../../../devices/platform/2007.etr ├── reserved_regions └── type 7 directories, 4 files After: root@debian:~# tree /sys/kernel/iommu_groups/ /sys/kernel/iommu_groups/ ├── 0 │ ├── devices │ │ ├── :00:00.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0 │ │ ├── :01:00.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0 │ │ ├── :02:01.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:01.0 │ │ ├── :02:02.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:02.0 │ │ ├── :02:03.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:03.0 │ │ ├── :02:0c.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:0c.0 │ │ ├── :02:10.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:10.0 │ │ ├── :02:1f.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:1f.0 │ │ ├── :03:00.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:01.0/:03:00.0 │ │ └── :08:00.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:1f.0/:08:00.0 │ ├── reserved_regions │ └── type ├── 1 │ ├── devices │ │ ├── 7ffb.ohci -> ../../../../devices/platform/7ffb.ohci │ │ └── 7ffc.ehci -> ../../../../devices/platform/7ffc.ehci │ ├── reserved_regions │ └── type └── 2 ├── devices │ └── 2007.etr -> ../../../../devices/platform/2007.etr ├── reserved_regions └── type 19 directories, 6 files Signed-off-by: Leo Yan --- arch/arm64/boot/dts/arm/juno-r1.dts | 4 arch/arm64/boot/dts/arm/juno-r2.dts | 4 2 files changed, 8 insertions(+) diff --git a/arch/arm64/boot/dts/arm/juno-r1.dts b/arch/arm64/boot/dts/arm/juno-r1.dts index b2b7ced633cf..67e161a6272a 100644 --- a/arch/arm64/boot/dts/arm/juno-r1.dts +++ b/arch/arm64/boot/dts/arm/juno-r1.dts @@ -226,6 +226,10 @@ status = "okay"; }; +&smmu_pcie { + status = "okay"; +}; + &pcie_ctlr { status = "okay"; }; diff --git a/arch/arm64/boot/dts/arm/juno-r2.dts b/arch/arm64/boot/dts/arm/juno-r2.dts index ab77adb4f3c2..0e1c5c814b01 100644 --- a/arch/arm64/boot/dts/arm/juno-r2.dts +++ b/arch/arm64/boot/dts/arm/juno-r2.dts @@ -226,6 +226,10 @@ status = "okay"; }; +&smmu_pcie { + status = "okay"; +}; + &pcie_ctlr { status = "okay"; }; -- 2.17.1
Re: [PATCH -next] phy: tegra: xusb: Make two functions static
On Tue, Mar 19, 2019 at 11:32:19PM +0800, Yue Haibing wrote: > From: YueHaibing > > Fix sparse warning: > > drivers/phy/tegra/xusb-tegra186.c:250:6: warning: symbol > 'tegra_phy_xusb_utmi_pad_power_on' was not declared. Should it be static? > drivers/phy/tegra/xusb-tegra186.c:281:6: warning: symbol > 'tegra_phy_xusb_utmi_pad_power_down' was not declared. Should it be static? > > Signed-off-by: YueHaibing > --- > drivers/phy/tegra/xusb-tegra186.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Sigh... I evidently need to improve my build scripts. Acked-by: Thierry Reding > diff --git a/drivers/phy/tegra/xusb-tegra186.c > b/drivers/phy/tegra/xusb-tegra186.c > index 11ad6e4..7a308b4 100644 > --- a/drivers/phy/tegra/xusb-tegra186.c > +++ b/drivers/phy/tegra/xusb-tegra186.c > @@ -247,7 +247,7 @@ static void tegra186_utmi_bias_pad_power_off(struct > tegra_xusb_padctl *padctl) > mutex_unlock(&padctl->lock); > } > > -void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy) > +static void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy) > { > struct tegra_xusb_lane *lane = phy_get_drvdata(phy); > struct tegra_xusb_padctl *padctl = lane->pad->padctl; > @@ -278,7 +278,7 @@ void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy) > padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index)); > } > > -void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy) > +static void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy) > { > struct tegra_xusb_lane *lane = phy_get_drvdata(phy); > struct tegra_xusb_padctl *padctl = lane->pad->padctl; > -- > 2.7.4 > > signature.asc Description: PGP signature
Re: [PATCH -next] clk: tegra: Make tegra_clk_super_mux_ops static
On Tue, Mar 19, 2019 at 11:35:04PM +0800, Yue Haibing wrote: > From: YueHaibing > > Fix sparse warning: > > drivers/clk/tegra/clk-super.c:124:22: > warning: symbol 'tegra_clk_super_mux_ops' was not declared. Should it be > static? > > Signed-off-by: YueHaibing > --- > drivers/clk/tegra/clk-super.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Thierry Reding signature.asc Description: PGP signature
RE: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node
Hi Shawn, > -Original Message- > From: Shawn Guo > Sent: 2019年3月20日 16:19 > To: Andy Tang > Cc: Daniel Lezcano ; mark.rutl...@arm.com; > devicet...@vger.kernel.org; linux...@vger.kernel.org; > linux-kernel@vger.kernel.org; Leo Li ; > edubez...@gmail.com; robh...@kernel.org; rui.zh...@intel.com; > linux-arm-ker...@lists.infradead.org > Subject: Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal zone node > > On Fri, Mar 08, 2019 at 09:57:09AM +, Andy Tang wrote: > > > > > > > -Original Message- > > > From: Daniel Lezcano > > > Sent: 2019年3月8日 17:28 > > > To: Andy Tang ; Shawn Guo > > > Cc: Leo Li ; robh...@kernel.org; > > > mark.rutl...@arm.com; linux-arm-ker...@lists.infradead.org; > > > devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; > > > linux...@vger.kernel.org; rui.zh...@intel.com; edubez...@gmail.com > > > Subject: Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal > > > zone node > > > > > > On 08/03/2019 03:07, Andy Tang wrote: > > > > > > > > > > > >> -Original Message- > > > >> From: Daniel Lezcano > > > >> Sent: 2019年3月7日 17:15 > > > >> To: Andy Tang ; Shawn Guo > > > >> > > > >> Cc: Leo Li ; robh...@kernel.org; > > > >> mark.rutl...@arm.com; linux-arm-ker...@lists.infradead.org; > > > >> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; > > > >> linux...@vger.kernel.org; rui.zh...@intel.com; > > > >> edubez...@gmail.com > > > >> Subject: Re: [PATCH v2] arm64: dts: ls1088a: add one more thermal > > > >> zone node > > > >> > > > > PS: In order to keep consistency to the first thermal-zone > > > > node, there will be "WARNING: line over 80 characters" warnings. > > > > > > > > arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 43 > > > +-- > > > > 1 files changed, 39 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi > > > > b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi > > > > index 661137f..9f52bc9 100644 > > > > --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi > > > > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi > > > > @@ -129,19 +129,19 @@ > > > > }; > > > > > > > > thermal-zones { > > > > - cpu_thermal: cpu-thermal { > > > > + ccu { > > > > > > Is this change really necessary? What does 'ccu' stand for? > > > >>> I think so. ccu stands for core cluster unit. cpu is too general. > > > >>> On some platforms, there are more than one core clusters. > > > >>> At least we should change it to "core cluster" if short form is > > > >>> not > > > appropriate. > > > >> > > > >> If the sensor is a the cluster level, 'cluster' is enough. IMHO, > > > >> no need to give a description of what contains the cluster, > > > >> otherwise you will end up with a 'core-gpu-cluster-l2' name. > > > > If cluster is specific to core, we can use cluster instead. But I don't > > > > think > so. > > > > Cluster may refer to "core cluster", "GPU cluster" etc. > > > > So, I think "core-cluster" is ok. > > > > If core was divided to several clusters, we can name it as > > > > "core-cluster1", > > > "core-cluster2" etc. > > > > If GPU was divided to several clusters we can name it as > > > > "gpu-cluster1", > > > "gpu-cluster2" etc. > > > > > > > > > Hi Andy, > > > > > > I think there is a confusion around the 'cpu' term and 'cluster'. > > > > > > ARM would like to see the 'cluster' word to disappear, so whenever > > > possible we should avoid it. > > > > > > From the hardware side, 'CPU' is usually used to describe the > > > physical chip containing the cores+cache. > > > > > > From the software side, 'CPU' is usually used to describe the > > > logical process unit, aka a core or a hyper-thread. > > > > > > As we are in the DT, so describing the hardware, the CPU refers to > > > the group > > > cores+caches. > > > > > > From my POV, using 'cpu' for the group of cores and 'gpu' for the > > > graphic sounds ok, and so far that is what is used for the other > > > platforms. > > > > > > If you change the name, that may give the feeling there is something > > > special with those thermal zones. > > > > Thanks Daniel for your detailed explanations. > > > > But as you said 'CPU' is usually used to describe the physical chip. > > Here is how I would understand Daniel's comments: > > CPU = cores + caches > physical chip = SoC = CPU + GPU + peripherals ... Agree. > > > So if we name it as CPU, it sounds like this temperature sensor is > > monitoring > the whole chip. > > That's not true in our case. > > > > Take ls2088a for example: > > In ls2088a SoC, there are 7 temperature sensors. Please note that they are > > all > located in SoC. > > The placement of the temperature sensors are showed below: > > > > Sensor ID placement > > 1 DDR controller 1 > > 2 DDR controller 2 > > 3 DDR controller 3 >
Re: [PATCH] mm, memory_hotplug: Fix the wrong usage of N_HIGH_MEMORY
On Wed 20-03-19 16:07:32, Baoquan He wrote: > In function node_states_check_changes_online(), N_HIGH_MEMORY is used > to substitute ZONE_HIGHMEM directly. This is not right. N_HIGH_MEMORY > always has value '3' if CONFIG_HIGHMEM=y, while ZONE_HIGHMEM's value > is not. It depends on whether CONFIG_ZONE_DMA/CONFIG_ZONE_DMA32 are > enabled. Obviously it's not true for CONFIG_ZONE_DMA32 on 32bit system, > and CONFIG_ZONE_DMA is also optional. > > Replace it with ZONE_HIGHMEM. N*MEMORY is confusing as hell but I am really curious whether we have ZONE_DMA32 and ZONE_HIGMEM together? That being said N.*MEMORY is intended to check for nodes rather than zones so the patch looks good to me but I think the above explanation is misleading and will add even more mud to the picture when somebody tries to understand what the heck is going on here. > Fixes: 8efe33f40f3e ("mm/memory_hotplug.c: simplify > node_states_check_changes_online") > Signed-off-by: Baoquan He > --- > mm/memory_hotplug.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index 6b05576fb4ec..09911d34a3be 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -712,7 +712,7 @@ static void node_states_check_changes_online(unsigned > long nr_pages, > if (zone_idx(zone) <= ZONE_NORMAL && !node_state(nid, N_NORMAL_MEMORY)) > arg->status_change_nid_normal = nid; > #ifdef CONFIG_HIGHMEM > - if (zone_idx(zone) <= N_HIGH_MEMORY && !node_state(nid, N_HIGH_MEMORY)) > + if (zone_idx(zone) <= ZONE_HIGHMEM && !node_state(nid, N_HIGH_MEMORY)) > arg->status_change_nid_high = nid; > #endif > } > -- > 2.17.2 -- Michal Hocko SUSE Labs
Re: [PATCH] dt-bindings: clock: axg-audio: unexpose controller inputs
On 13/02/2019 10:58, Jerome Brunet wrote: > Remove the bindings ID of the clock input of the controller. These > clocks are purely internal to the controller, exposing them was a > mistake. Actually, these should not even be in the provider and have > IDs to begin with. > > Unexpose these IDs before: > * someone starts using them (even if there no valid reason to do so) > * the actual clocks are removed. The fact that they exist is just the >result of an ugly hack. This will be resolved in CCF when we can >reference DT directly in parent table. > > Signed-off-by: Jerome Brunet > --- > drivers/clk/meson/axg-audio.h | 20 > include/dt-bindings/clock/axg-audio-clkc.h | 20 > 2 files changed, 20 insertions(+), 20 deletions(-) > > diff --git a/drivers/clk/meson/axg-audio.h b/drivers/clk/meson/axg-audio.h > index 7191b39c9d65..644f0b0fddf2 100644 > --- a/drivers/clk/meson/axg-audio.h > +++ b/drivers/clk/meson/axg-audio.h > @@ -60,6 +60,26 @@ > #define AUD_CLKID_MST5 6 > #define AUD_CLKID_MST6 7 > #define AUD_CLKID_MST7 8 > +#define AUD_CLKID_SLV_SCLK0 9 > +#define AUD_CLKID_SLV_SCLK1 10 > +#define AUD_CLKID_SLV_SCLK2 11 > +#define AUD_CLKID_SLV_SCLK3 12 > +#define AUD_CLKID_SLV_SCLK4 13 > +#define AUD_CLKID_SLV_SCLK5 14 > +#define AUD_CLKID_SLV_SCLK6 15 > +#define AUD_CLKID_SLV_SCLK7 16 > +#define AUD_CLKID_SLV_SCLK8 17 > +#define AUD_CLKID_SLV_SCLK9 18 > +#define AUD_CLKID_SLV_LRCLK0 19 > +#define AUD_CLKID_SLV_LRCLK1 20 > +#define AUD_CLKID_SLV_LRCLK2 21 > +#define AUD_CLKID_SLV_LRCLK3 22 > +#define AUD_CLKID_SLV_LRCLK4 23 > +#define AUD_CLKID_SLV_LRCLK5 24 > +#define AUD_CLKID_SLV_LRCLK6 25 > +#define AUD_CLKID_SLV_LRCLK7 26 > +#define AUD_CLKID_SLV_LRCLK8 27 > +#define AUD_CLKID_SLV_LRCLK9 28 > #define AUD_CLKID_MST_A_MCLK_SEL 59 > #define AUD_CLKID_MST_B_MCLK_SEL 60 > #define AUD_CLKID_MST_C_MCLK_SEL 61 > diff --git a/include/dt-bindings/clock/axg-audio-clkc.h > b/include/dt-bindings/clock/axg-audio-clkc.h > index fd9c362099d9..eafb0de8466b 100644 > --- a/include/dt-bindings/clock/axg-audio-clkc.h > +++ b/include/dt-bindings/clock/axg-audio-clkc.h > @@ -7,26 +7,6 @@ > #ifndef __AXG_AUDIO_CLKC_BINDINGS_H > #define __AXG_AUDIO_CLKC_BINDINGS_H > > -#define AUD_CLKID_SLV_SCLK0 9 > -#define AUD_CLKID_SLV_SCLK1 10 > -#define AUD_CLKID_SLV_SCLK2 11 > -#define AUD_CLKID_SLV_SCLK3 12 > -#define AUD_CLKID_SLV_SCLK4 13 > -#define AUD_CLKID_SLV_SCLK5 14 > -#define AUD_CLKID_SLV_SCLK6 15 > -#define AUD_CLKID_SLV_SCLK7 16 > -#define AUD_CLKID_SLV_SCLK8 17 > -#define AUD_CLKID_SLV_SCLK9 18 > -#define AUD_CLKID_SLV_LRCLK0 19 > -#define AUD_CLKID_SLV_LRCLK1 20 > -#define AUD_CLKID_SLV_LRCLK2 21 > -#define AUD_CLKID_SLV_LRCLK3 22 > -#define AUD_CLKID_SLV_LRCLK4 23 > -#define AUD_CLKID_SLV_LRCLK5 24 > -#define AUD_CLKID_SLV_LRCLK6 25 > -#define AUD_CLKID_SLV_LRCLK7 26 > -#define AUD_CLKID_SLV_LRCLK8 27 > -#define AUD_CLKID_SLV_LRCLK9 28 > #define AUD_CLKID_DDR_ARB29 > #define AUD_CLKID_PDM30 > #define AUD_CLKID_TDMIN_A31 > Applied to next/drivers
Re: [RFC 0/2] guarantee natural alignment for kmalloc()
On 3/20/19 1:43 AM, Christopher Lameter wrote: > On Tue, 19 Mar 2019, Vlastimil Babka wrote: > >> The recent thread [1] inspired me to look into guaranteeing alignment for >> kmalloc() for power-of-two sizes. Turns out it's not difficult and in most >> configuration nothing really changes as it happens implicitly. More details >> in >> the first patch. If we agree we want to do this, I will see where to update >> documentation and perhaps if there are any workarounds in the tree that can >> be >> converted to plain kmalloc() afterwards. > > This means that the alignments are no longer uniform for all kmalloc > caches and we get back to code making all sorts of assumptions about > kmalloc alignments. Natural alignment to size is rather well defined, no? Would anyone ever assume a larger one, for what reason? It's now where some make assumptions (even unknowingly) for natural There are two 'odd' sizes 96 and 192, which will keep cacheline size alignment, would anyone really expect more than 64 bytes? > Currently all kmalloc objects are aligned to KMALLOC_MIN_ALIGN. That will > no longer be the case and alignments will become inconsistent. KMALLOC_MIN_ALIGN is still the minimum, but in practice it's larger which is not a problem. Also let me stress again that nothing really changes except for SLOB, and SLUB with debug options. The natural alignment for power-of-two sizes already happens as SLAB and SLUB both allocate objects starting on the page boundary. So people make assumptions based on that, and then break with SLOB, or SLUB with debug. This patch just prevents that breakage by guaranteeing those natural assumptions at all times. > I think its valuable that alignment requirements need to be explicitly > requested. That's still possible for named caches created by kmem_cache_create(). > Lets add an array of power of two aligned kmalloc caches if that is really > necessary. Add some GFP_XXX flag to kmalloc to make it ^2 aligned maybe? That's unnecessary and wasteful, as the existing caches are already aligned in the common configurations. Requiring a flag doesn't help with the implicit assumptions going wrong. I really don't think it needs to get more complicated than adjusting the uncommon configuration, as this patch does.
[PATCH] staging: rtl8188eu: remove unused WFD defines
All defined WFD* in wifi.h are unused in the driver code, so remove them. Signed-off-by: Michael Straube --- drivers/staging/rtl8188eu/include/wifi.h | 20 1 file changed, 20 deletions(-) diff --git a/drivers/staging/rtl8188eu/include/wifi.h b/drivers/staging/rtl8188eu/include/wifi.h index 5f6b04b5016c..d059240b836f 100644 --- a/drivers/staging/rtl8188eu/include/wifi.h +++ b/drivers/staging/rtl8188eu/include/wifi.h @@ -681,26 +681,6 @@ enum ht_cap_ampdu_factor { #defineWPS_CM_SW_DISPLAY_P 0x2008 #defineWPS_CM_LCD_DISPLAY_P0x4008 -/* =WFD Section= */ -/* For Wi-Fi Display */ -#defineWFD_ATTR_DEVICE_INFO0x00 -#defineWFD_ATTR_ASSOC_BSSID0x01 -#defineWFD_ATTR_COUPLED_SINK_INFO 0x06 -#defineWFD_ATTR_LOCAL_IP_ADDR 0x08 -#defineWFD_ATTR_SESSION_INFO 0x09 -#defineWFD_ATTR_ALTER_MAC 0x0a - -/* For WFD Device Information Attribute */ -#defineWFD_DEVINFO_SOURCE 0x -#defineWFD_DEVINFO_PSINK 0x0001 -#defineWFD_DEVINFO_SSINK 0x0002 -#defineWFD_DEVINFO_DUAL0x0003 - -#defineWFD_DEVINFO_SESSION_AVAIL 0x0010 -#defineWFD_DEVINFO_WSD 0x0040 -#defineWFD_DEVINFO_PC_TDLS 0x0080 -#defineWFD_DEVINFO_HDCP_SUPPORT0x0100 - #define IP_MCAST_MAC(mac) \ ((mac[0] == 0x01) && (mac[1] == 0x00) && (mac[2] == 0x5e)) #define ICMPV6_MCAST_MAC(mac) \ -- 2.21.0
Re: [RFC RESEND PATCH 1/7] dt-bindings: soc: Add DVFSRC driver bindings
On Mon, Feb 18, 2019 at 12:55 PM Henry Chen wrote: > > Hi Rob, > > Sorry for late reply. I missed this mail before. > > On Fri, 2019-01-11 at 10:09 -0600, Rob Herring wrote: > > On Wed, Jan 02, 2019 at 10:09:52PM +0800, Henry Chen wrote: > > > Document the binding for enabling DVFSRC on MediaTek SoC. > > > > > > Signed-off-by: Henry Chen > > > --- > > > .../devicetree/bindings/soc/mediatek/dvfsrc.txt| 26 > > > ++ > > > include/dt-bindings/soc/mtk,dvfsrc.h | 18 +++ > > > 2 files changed, 44 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/soc/mediatek/dvfsrc.txt > > > create mode 100644 include/dt-bindings/soc/mtk,dvfsrc.h > > > > > > diff --git a/Documentation/devicetree/bindings/soc/mediatek/dvfsrc.txt > > > b/Documentation/devicetree/bindings/soc/mediatek/dvfsrc.txt > > > new file mode 100644 > > > index 000..402c885 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/soc/mediatek/dvfsrc.txt > > > @@ -0,0 +1,26 @@ > > > +MediaTek DVFSRC Driver > > > > Bindings are for h/w blocks, not drivers. > ok. > > > > > +The Dynamic Voltage and Frequency Scaling Resource Collector (DVFSRC) is > > > a > > > +HW module which is used to collect all the requests from both software > > > and > > > +hardware and turn into the decision of minimum operating voltage and > > > minimum > > > +DRAM frequency to fulfill those requests. > > > > Seems like the OPP table should be a child of this instead of where you > > currently have it? > Do you means the opp table that I put on scpsys likes below? > I think this opp table is used for mapping the performance state of > power domain, so I put it on scpsys device tree document. > > dvfsrc_opp_table: opp-table { > compatible = "operating-points-v2-level"; > > dvfsrc_vol_min: opp1 { > opp,level = ; > }; > > dvfsrc_freq_medium: opp2 { > opp,level = ; > }; > > dvfsrc_freq_max: opp3 { > opp,level = ; > }; > > dvfsrc_vol_max: opp4 { > opp,level = ; > }; > }; > > > > > > + > > > +Required Properties: > > > +- compatible: Should be one of the following > > > + - "mediatek,mt8183-dvfsrc": For MT8183 SoC > > > +- reg: Address range of the DVFSRC unit > > > +- dram_type: Refer to for the > > > + different dram type support. > > > > This information should come from the DDR controller or memory nodes > > probably. And we already have some properties related to DDR type. > Sorry, I don't know that before, could you give some hint or example for > that? So, I'm really not sure either... One example I could find is Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt, but it still doesn't specify memory type (it just passes parameters to ATF). You can also look at Documentation/devicetree/bindings/memory-controllers/ti/emif.txt (but that only supports lpddr2 nodes, I guess we could add more types?). Some ideas: 1. DDR controller: Maybe you can probe at runtime, what memory type is being used? After all the FW will have configured the memory controller properly, so you could read back that information, somehow. 2. memory node: Either add a new lpddr3/4/4x node in device tree (like TI's emif), or add something to the memory node? memory@4000 { device_type = "memory"; reg = <0 0x4000 0 0x8000>; }; I don't see a binding document for memory though, and no one ever seems to add anything but basic size information. > > > > > +- clock-names: Must include the following entries: > > > + "dvfsrc": DVFSRC module clock > > > +- clocks: Must contain an entry for each entry in clock-names. > > > + > > > +Example: > > > + > > > + dvfsrc_top@10012000 { > > > > Drop the '_top'. (Don't use '_' in node and property names).. > ok > > > > > + compatible = "mediatek,mt8183-dvfsrc"; > > > + reg = <0 0x10012000 0 0x1000>; > > > + clocks = <&infracfg CLK_INFRA_DVFSRC>; > > > + clock-names = "dvfsrc"; > > > + dram_type = ; > > > + }; > > > diff --git a/include/dt-bindings/soc/mtk,dvfsrc.h > > > b/include/dt-bindings/soc/mtk,dvfsrc.h > > > new file mode 100644 > > > index 000..60b3497 > > > --- /dev/null > > > +++ b/include/dt-bindings/soc/mtk,dvfsrc.h > > > @@ -0,0 +1,18 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 > > > + * > > > + * Copyright (c) 2018 MediaTek Inc. > > > + */ > > > + > > > +#ifndef _DT_BINDINGS_POWER_MTK_DVFSRC_H > > > +#define _DT_BINDINGS_POWER_MTK_DVFSRC_H > > > + > > > +#define MT8183_DVFSRC_OPP_LP4 0 > > > +#define MT8183_DVFSRC_OPP_LP4X 1 > > > +#define MT8183_DVFSRC_OPP_LP3 2 > > > + > > > +#define MT8183_DVFSRC_LEVE
Re: [RESEND PATCH v1] moduleparam: Save information about built-in modules in separate file
On Fri, Mar 15, 2019 at 7:10 PM Alexey Gladkov wrote: > > Problem: > > When a kernel module is compiled as a separate module, some important > information about the kernel module is available via .modinfo section of > the module. In contrast, when the kernel module is compiled into the > kernel, that information is not available. > > Information about built-in modules is necessary in the following cases: > > 1. When it is necessary to find out what additional parameters can be > passed to the kernel at boot time. > > 2. When you need to know which module names and their aliases are in > the kernel. This is very useful for creating an initrd image. > > Proposal: > > The proposed patch does not remove .modinfo section with module > information from the vmlinux at the build time and saves it into a > separate file after kernel linking. So, the kernel does not increase in > size and no additional information remains in it. Information is stored > in the same format as in the separate modules (null-terminated string > array). Because the .modinfo section is already exported with a separate > modules, we are not creating a new API. > > It can be easily read in the userspace: > > $ tr '\0' '\n' < kernel.builtin.modinfo > ext4.softdep=pre: crc32c > ext4.license=GPL > ext4.description=Fourth Extended Filesystem > ext4.author=Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, > Theodore Ts'o and others > ext4.alias=fs-ext4 > ext4.alias=ext3 > ext4.alias=fs-ext3 > ext4.alias=ext2 > ext4.alias=fs-ext2 > md_mod.alias=block-major-9-* > md_mod.alias=md > md_mod.description=MD RAID framework > md_mod.license=GPL > md_mod.parmtype=create_on_open:bool > md_mod.parmtype=start_dirty_degraded:int > ... > > Co-Developed-by: Gleb Fotengauer-Malinovskiy > Signed-off-by: Gleb Fotengauer-Malinovskiy > Signed-off-by: Alexey Gladkov > --- I just started to take a look. I was able to compile it for x86, but got tons of warnings for powerpc. Anyway, you do not need to send v2 soon. Please give me some more time for the through review. The following is what I got by "make -j8 ARCH=powerpc CROSS_COMPILE=powerpc-linux- defconfig all" ... CC [M] drivers/scsi/st.o AR drivers/scsi/built-in.a AR drivers/built-in.a GEN .version CHK include/generated/compile.h UPD include/generated/compile.h CC init/version.o AR init/built-in.a LD vmlinux.o MODPOST vmlinux.o powerpc-linux-ld: warning: orphan section `.gnu.hash' from `linker stubs' being placed in section `.gnu.hash'. powerpc-linux-ld: warning: orphan section `.modinfo' from `arch/powerpc/platforms/pseries/pseries_energy.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `arch/powerpc/platforms/pseries/cmm.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `arch/powerpc/platforms/pasemi/gpio_mdio.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `kernel/workqueue.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `kernel/printk/printk.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `kernel/irq/spurious.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `kernel/rcu/update.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `kernel/rcu/srcutree.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `kernel/rcu/tree.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `kernel/module.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `kernel/configs.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `fs/binfmt_script.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `fs/binfmt_elf.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `fs/compat_binfmt_elf.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `fs/mbcache.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `fs/nfs_common/nfsacl.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `fs/nfs_common/grace.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `fs/ext4/super.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `fs/ext2/super.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `fs/jbd2/journal.o' being placed in section `.modinfo'. powerpc-linux-ld: warning: orphan section `.modinfo' from `fs/fat/inode.o
Re: static analysis bug report: staging r8712u memcpy of uninitialized variable
On Mon, Mar 18, 2019 at 11:20:51AM +, Colin Ian King wrote: > Hi, > > Static analysis with cppcheck found a couple of interesting issues with > memcpy'ing of an uninitialized variable. Two occurrences of the same > issue are found in drivers/staging/rtl8712/rtl8712_cmd.c in functions > read_bbreg_hdl and read_rfreg_hdl. > > For example: > > static u8 read_bbreg_hdl(struct _adapter *padapter, u8 *pbuf) > { > u32 val; > void (*pcmd_callback)(struct _adapter *dev, struct cmd_obj > *pcmd); > struct cmd_obj *pcmd = (struct cmd_obj *)pbuf; > > if (pcmd->rsp && pcmd->rspsz > 0) > memcpy(pcmd->rsp, (u8 *)&val, pcmd->rspsz); > > > > } > > I don't understand why the contents of val is being memcpy'd to > pcmd->rsp, especially when val is uninitialized and hence contains > garbage. Any ideas? > The concern would be that it's reading a user specified amount of stack memory to pcmd->rsp and that sounds like an information leak. The pcmd_callback function pointer is always r8712_getbbrfreg_cmdrsp_callback() which frees pcmd->parmbuf and pcmd but leaks pcmd->rsp. I don't see a way for anyone to access the ->rsp memory so probably there aren't any security implications. Anyway, let me send a patch. regards, dan carpenter
Re: [PATCH 2/2] pwm: Kconfig: Enable ehrpwm driver to be compiled for ARCH_K3
Hi, On 12/03/19 3:00 PM, Uwe Kleine-König wrote: > On Tue, Mar 12, 2019 at 02:46:29PM +0530, Vignesh Raghavendra wrote: >> K3 devices have the same EHRPWM IP as OMAP SoCs. Enable driver to be built >> for K3 devices. Also, drop reference to AM33xx in help text, as IP is >> found on multiple TI SoCs. >> >> Signed-off-by: Vignesh Raghavendra >> --- >> drivers/pwm/Kconfig | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig >> index 54f8238aac0d..c054bd1dba36 100644 >> --- a/drivers/pwm/Kconfig >> +++ b/drivers/pwm/Kconfig >> @@ -467,10 +467,9 @@ config PWM_TIECAP >> >> config PWM_TIEHRPWM >> tristate "EHRPWM PWM support" >> -depends on ARCH_OMAP2PLUS || ARCH_DAVINCI_DA8XX >> +depends on ARCH_OMAP2PLUS || ARCH_DAVINCI_DA8XX || ARCH_K3 >> help >> - PWM driver support for the EHRPWM controller found on AM33XX >> - TI SOC >> + PWM driver support for the EHRPWM controller found on TI SOCs > > It would be great to be able to compile this driver with COMPILE_TEST. > From a quick look the following would be appropriate: > > depends on ARCH_OMAP2PLUS || ARCH_DAVINCI_DA8XX || ARCH_K3 || > COMPILE_TEST > depends on HAVE_CLK && HAS_IOMEM > Thanks for the suggestion! I will send a separate patch on top of this series to enable COMPILE TEST for this driver and other TI PWM drivers. > Best regards > Uwe > -- Regards Vignesh
Re: [PATCH] mm, memory_hotplug: Fix the wrong usage of N_HIGH_MEMORY
On 03/20/19 at 09:46am, Michal Hocko wrote: > On Wed 20-03-19 16:07:32, Baoquan He wrote: > > In function node_states_check_changes_online(), N_HIGH_MEMORY is used > > to substitute ZONE_HIGHMEM directly. This is not right. N_HIGH_MEMORY > > always has value '3' if CONFIG_HIGHMEM=y, while ZONE_HIGHMEM's value > > is not. It depends on whether CONFIG_ZONE_DMA/CONFIG_ZONE_DMA32 are > > enabled. Obviously it's not true for CONFIG_ZONE_DMA32 on 32bit system, > > and CONFIG_ZONE_DMA is also optional. > > > > Replace it with ZONE_HIGHMEM. > > N*MEMORY is confusing as hell but I am really curious whether we have > ZONE_DMA32 and ZONE_HIGMEM together? Not sure. AFAIK, on x86_32 it can't be. > > That being said N.*MEMORY is intended to check for nodes rather than > zones so the patch looks good to me but I think the above explanation is > misleading and will add even more mud to the picture when somebody tries > to understand what the heck is going on here. Yes, agree. I also thought this again after I sent out patch, feel log is not good. As you said, they are value of enum node_states and enum zone_type separately. How about this: ~~~ In function node_states_check_changes_online(), N_HIGH_MEMORY is used to substitute ZONE_HIGHMEM directly. This is not right. N_HIGH_MEMORY is to mark the memory state of node. Here zone index is checked, which should be compared with 'ZONE_HIGHMEM' accordingly. Replace it with ZONE_HIGHMEM. ~~~ > > > Fixes: 8efe33f40f3e ("mm/memory_hotplug.c: simplify > > node_states_check_changes_online") > > Signed-off-by: Baoquan He > > --- > > mm/memory_hotplug.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > > index 6b05576fb4ec..09911d34a3be 100644 > > --- a/mm/memory_hotplug.c > > +++ b/mm/memory_hotplug.c > > @@ -712,7 +712,7 @@ static void node_states_check_changes_online(unsigned > > long nr_pages, > > if (zone_idx(zone) <= ZONE_NORMAL && !node_state(nid, N_NORMAL_MEMORY)) > > arg->status_change_nid_normal = nid; > > #ifdef CONFIG_HIGHMEM > > - if (zone_idx(zone) <= N_HIGH_MEMORY && !node_state(nid, N_HIGH_MEMORY)) > > + if (zone_idx(zone) <= ZONE_HIGHMEM && !node_state(nid, N_HIGH_MEMORY)) > > arg->status_change_nid_high = nid; > > #endif > > } > > -- > > 2.17.2 > > -- > Michal Hocko > SUSE Labs
Re: [PATCH] mm, memory_hotplug: Fix the wrong usage of N_HIGH_MEMORY
On 20.03.19 10:06, Baoquan He wrote: > On 03/20/19 at 09:46am, Michal Hocko wrote: >> On Wed 20-03-19 16:07:32, Baoquan He wrote: >>> In function node_states_check_changes_online(), N_HIGH_MEMORY is used >>> to substitute ZONE_HIGHMEM directly. This is not right. N_HIGH_MEMORY >>> always has value '3' if CONFIG_HIGHMEM=y, while ZONE_HIGHMEM's value >>> is not. It depends on whether CONFIG_ZONE_DMA/CONFIG_ZONE_DMA32 are >>> enabled. Obviously it's not true for CONFIG_ZONE_DMA32 on 32bit system, >>> and CONFIG_ZONE_DMA is also optional. >>> >>> Replace it with ZONE_HIGHMEM. >> >> N*MEMORY is confusing as hell but I am really curious whether we have >> ZONE_DMA32 and ZONE_HIGMEM together? > > Not sure. AFAIK, on x86_32 it can't be. > >> >> That being said N.*MEMORY is intended to check for nodes rather than >> zones so the patch looks good to me but I think the above explanation is >> misleading and will add even more mud to the picture when somebody tries >> to understand what the heck is going on here. > > Yes, agree. I also thought this again after I sent out patch, feel log is not > good. As you said, they are value of enum node_states and enum zone_type > separately. > > How about this: > > ~~~ > In function node_states_check_changes_online(), N_HIGH_MEMORY is used > to substitute ZONE_HIGHMEM directly. This is not right. N_HIGH_MEMORY > is to mark the memory state of node. Here zone index is checked, which > should be compared with 'ZONE_HIGHMEM' accordingly. > > Replace it with ZONE_HIGHMEM. Reviewed-by: David Hildenbrand -- Thanks, David / dhildenb
Re: [PATCH v4 1/1] mm: introduce put_user_page*(), placeholder versions
On Tue, Mar 19, 2019 at 01:01:01PM -0700, John Hubbard wrote: > On 3/19/19 7:06 AM, Kirill A. Shutemov wrote: > > On Tue, Mar 19, 2019 at 09:47:24AM -0400, Jerome Glisse wrote: > >> On Tue, Mar 19, 2019 at 03:04:17PM +0300, Kirill A. Shutemov wrote: > >>> On Fri, Mar 08, 2019 at 01:36:33PM -0800, john.hubb...@gmail.com wrote: > From: John Hubbard > >> > >> [...] > >> > diff --git a/mm/gup.c b/mm/gup.c > index f84e22685aaa..37085b8163b1 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -28,6 +28,88 @@ struct follow_page_context { > unsigned int page_mask; > }; > > +typedef int (*set_dirty_func_t)(struct page *page); > + > +static void __put_user_pages_dirty(struct page **pages, > + unsigned long npages, > + set_dirty_func_t sdf) > +{ > +unsigned long index; > + > +for (index = 0; index < npages; index++) { > +struct page *page = compound_head(pages[index]); > + > +if (!PageDirty(page)) > +sdf(page); > >>> > >>> How is this safe? What prevents the page to be cleared under you? > >>> > >>> If it's safe to race clear_page_dirty*() it has to be stated explicitly > >>> with a reason why. It's not very clear to me as it is. > >> > >> The PageDirty() optimization above is fine to race with clear the > >> page flag as it means it is racing after a page_mkclean() and the > >> GUP user is done with the page so page is about to be write back > >> ie if (!PageDirty(page)) see the page as dirty and skip the sdf() > >> call while a split second after TestClearPageDirty() happens then > >> it means the racing clear is about to write back the page so all > >> is fine (the page was dirty and it is being clear for write back). > >> > >> If it does call the sdf() while racing with write back then we > >> just redirtied the page just like clear_page_dirty_for_io() would > >> do if page_mkclean() failed so nothing harmful will come of that > >> neither. Page stays dirty despite write back it just means that > >> the page might be write back twice in a row. > > > > Fair enough. Should we get it into a comment here? > > How's this read to you? I reworded and slightly expanded Jerome's > description: > > diff --git a/mm/gup.c b/mm/gup.c > index d1df7b8ba973..86397ae23922 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -61,6 +61,24 @@ static void __put_user_pages_dirty(struct page **pages, > for (index = 0; index < npages; index++) { > struct page *page = compound_head(pages[index]); > > + /* > +* Checking PageDirty at this point may race with > +* clear_page_dirty_for_io(), but that's OK. Two key cases: > +* > +* 1) This code sees the page as already dirty, so it skips > +* the call to sdf(). That could happen because > +* clear_page_dirty_for_io() called page_mkclean(), > +* followed by set_page_dirty(). However, now the page is > +* going to get written back, which meets the original > +* intention of setting it dirty, so all is well: > +* clear_page_dirty_for_io() goes on to call > +* TestClearPageDirty(), and write the page back. > +* > +* 2) This code sees the page as clean, so it calls sdf(). > +* The page stays dirty, despite being written back, so it > +* gets written back again in the next writeback cycle. > +* This is harmless. > +*/ > if (!PageDirty(page)) > sdf(page); Looks good to me. Other nit: effectively the same type of callback called 'spd' in set_page_dirty(). Should we rename 'sdf' to 'sdp' here too? > +void put_user_pages(struct page **pages, unsigned long npages) > +{ > +unsigned long index; > + > +for (index = 0; index < npages; index++) > +put_user_page(pages[index]); > >>> > >>> I believe there's an room for improvement for compound pages. > >>> > >>> If there's multiple consequential pages in the array that belong to the > >>> same compound page we can get away with a single atomic operation to > >>> handle them all. > >> > >> Yes maybe just add a comment with that for now and leave this kind of > >> optimization to latter ? > > > > Sounds good to me. > > > > Here's a comment for that: > > @@ -127,6 +145,11 @@ void put_user_pages(struct page **pages, unsigned long > npages) > { > unsigned long index; > > + /* > +* TODO: this can be optimized for huge pages: if a series of pages is > +* physically contiguous and part of the same compound page, then a Comound pages are always physically contiguous. I ini
Re: [PATCH] mm, memory_hotplug: Fix the wrong usage of N_HIGH_MEMORY
On 03/20/19 at 10:11am, David Hildenbrand wrote: > On 20.03.19 10:06, Baoquan He wrote: > > On 03/20/19 at 09:46am, Michal Hocko wrote: > >> On Wed 20-03-19 16:07:32, Baoquan He wrote: > >>> In function node_states_check_changes_online(), N_HIGH_MEMORY is used > >>> to substitute ZONE_HIGHMEM directly. This is not right. N_HIGH_MEMORY > >>> always has value '3' if CONFIG_HIGHMEM=y, while ZONE_HIGHMEM's value > >>> is not. It depends on whether CONFIG_ZONE_DMA/CONFIG_ZONE_DMA32 are > >>> enabled. Obviously it's not true for CONFIG_ZONE_DMA32 on 32bit system, > >>> and CONFIG_ZONE_DMA is also optional. > >>> > >>> Replace it with ZONE_HIGHMEM. > >> > >> N*MEMORY is confusing as hell but I am really curious whether we have > >> ZONE_DMA32 and ZONE_HIGMEM together? > > > > Not sure. AFAIK, on x86_32 it can't be. > > > >> > >> That being said N.*MEMORY is intended to check for nodes rather than > >> zones so the patch looks good to me but I think the above explanation is > >> misleading and will add even more mud to the picture when somebody tries > >> to understand what the heck is going on here. > > > > Yes, agree. I also thought this again after I sent out patch, feel log is > > not > > good. As you said, they are value of enum node_states and enum zone_type > > separately. > > > > How about this: > > > > ~~~ > > In function node_states_check_changes_online(), N_HIGH_MEMORY is used > > to substitute ZONE_HIGHMEM directly. This is not right. N_HIGH_MEMORY > > is to mark the memory state of node. Here zone index is checked, which > > should be compared with 'ZONE_HIGHMEM' accordingly. > > > > Replace it with ZONE_HIGHMEM. > > Reviewed-by: David Hildenbrand Thanks, both. Will use this log and repost. Thanks Baoquan
[PATCH RESEND] eventfd: prepare id to userspace via fdinfo
Finding endpoints of an IPC channel is one of essential task to understand how a user program works. Procfs and netlink socket provide enough hints to find endpoints for IPC channels like pipes, unix sockets, and pseudo terminals. However, there is no simple way to find endpoints for an eventfd file from userland. An inode number doesn't hint. Unlike pipe, all eventfd files share the same inode object. To provide the way to find endpoints of an eventfd file, this patch adds "eventfd-id" field to /proc/PID/fdinfo of eventfd as identifier. Address for eventfd context is used as id. A tool like lsof can utilize the information to print endpoints. Signed-off-by: Masatake YAMATO --- fs/eventfd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/eventfd.c b/fs/eventfd.c index 08d3bd602f73..fc63ad43d962 100644 --- a/fs/eventfd.c +++ b/fs/eventfd.c @@ -297,6 +297,7 @@ static void eventfd_show_fdinfo(struct seq_file *m, struct file *f) seq_printf(m, "eventfd-count: %16llx\n", (unsigned long long)ctx->count); spin_unlock_irq(&ctx->wqh.lock); + seq_printf(m, "eventfd-id: %p\n", ctx); } #endif -- 2.17.2
Re: regression (bisected): "modprobe parport_pc" hangs in current mainline
Hi Michal, On Sun, Mar 17, 2019 at 6:05 PM Michal Kubecek wrote: > > On Sun, Mar 17, 2019 at 05:01:37PM +, Sudip Mukherjee wrote: > > On Wed, Mar 13, 2019 at 6:45 AM Michal Kubecek wrote: > > > I encountered a regression in current (post-5.0) mainline kernel which I > > > bisected to commit 1aec4211204d ("parport: daisy: use new parport device > > > model"). Running "modprobe parport_pc" hangs up: > > > > Can you please send me your .config so that I can test it from my side. > > Attaching two versions: config-full.gz is the real life config from the > machine where I found the issue and config-mini.gz is a minimized config > I was using while bisecting the issue. (I made a mistake and thought > that I have seen the issue with snapshot before both parport commits so > that I ran a full bisect instead of simply checking the two parport > commits which came in the merge window.) Sorry, I didn't get the chance to look at it yet and have kept it pending for this weekend. But just had a quick look and I was wondering if the machine on which you are trying the modprobe has an actual parallel port or the machine is not having any parallel port. And also will you be able to send me a dmesg please. -- Regards Sudip
Re: [PATCH v3] optee: allow to work without static shared memory
Hi Volodymyr, On Mon, Mar 11, 2019 at 2:04 PM Volodymyr Babchuk wrote: > > From: Volodymyr Babchuk > > On virtualized systems it is possible that OP-TEE will provide > only dynamic shared memory support. So it is fine to boot > without static SHM enabled if dymanic one is supported. > > Signed-off-by: Volodymyr Babchuk > --- > > Changes from V2: > - rebased onto upstream > > drivers/tee/optee/core.c | 80 > 1 file changed, 49 insertions(+), 31 deletions(-) > Looks good, I'm picking this up. Thanks, Jens
[PATCH] virt: vbox: Implement passing requestor info to the host for VirtualBox 6.0.x
VirtualBox 6.0.x has a new feature where the guest kernel driver passes info about the origin of the request (e.g. userspace or kernelspace) to the hypervisor. If we do not pass this information then when running the 6.0.x userspace guest-additions tools on a 6.0.x host, some requests will get denied with a VERR_VERSION_MISMATCH error, breaking vboxservice.service and the mounting of shared folders marked to be auto-mounted. This commit implements passing the requestor info to the host, fixing this. Cc: sta...@vger.kernel.org Signed-off-by: Hans de Goede --- drivers/virt/vboxguest/vboxguest_core.c| 106 ++--- drivers/virt/vboxguest/vboxguest_core.h| 15 +-- drivers/virt/vboxguest/vboxguest_linux.c | 26 - drivers/virt/vboxguest/vboxguest_utils.c | 32 --- drivers/virt/vboxguest/vboxguest_version.h | 9 +- drivers/virt/vboxguest/vmmdev.h| 8 +- include/linux/vbox_utils.h | 12 ++- include/uapi/linux/vbox_vmmdev_types.h | 31 ++ 8 files changed, 168 insertions(+), 71 deletions(-) diff --git a/drivers/virt/vboxguest/vboxguest_core.c b/drivers/virt/vboxguest/vboxguest_core.c index 1475ed5ffcde..2ec5b34ffed7 100644 --- a/drivers/virt/vboxguest/vboxguest_core.c +++ b/drivers/virt/vboxguest/vboxguest_core.c @@ -27,6 +27,10 @@ #define GUEST_MAPPINGS_TRIES 5 +#define VBG_KERNEL_REQUEST \ + (VMMDEV_REQUESTOR_KERNEL | VMMDEV_REQUESTOR_USR_DRV | \ +VMMDEV_REQUESTOR_CON_DONT_KNOW | VMMDEV_REQUESTOR_TRUST_NOT_GIVEN) + /** * Reserves memory in which the VMM can relocate any guest mappings * that are floating around. @@ -48,7 +52,8 @@ static void vbg_guest_mappings_init(struct vbg_dev *gdev) int i, rc; /* Query the required space. */ - req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_GET_HYPERVISOR_INFO); + req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_GET_HYPERVISOR_INFO, + VBG_KERNEL_REQUEST); if (!req) return; @@ -135,7 +140,8 @@ static void vbg_guest_mappings_exit(struct vbg_dev *gdev) * Tell the host that we're going to free the memory we reserved for * it, the free it up. (Leak the memory if anything goes wrong here.) */ - req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_SET_HYPERVISOR_INFO); + req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_SET_HYPERVISOR_INFO, + VBG_KERNEL_REQUEST); if (!req) return; @@ -172,8 +178,10 @@ static int vbg_report_guest_info(struct vbg_dev *gdev) struct vmmdev_guest_info2 *req2 = NULL; int rc, ret = -ENOMEM; - req1 = vbg_req_alloc(sizeof(*req1), VMMDEVREQ_REPORT_GUEST_INFO); - req2 = vbg_req_alloc(sizeof(*req2), VMMDEVREQ_REPORT_GUEST_INFO2); + req1 = vbg_req_alloc(sizeof(*req1), VMMDEVREQ_REPORT_GUEST_INFO, +VBG_KERNEL_REQUEST); + req2 = vbg_req_alloc(sizeof(*req2), VMMDEVREQ_REPORT_GUEST_INFO2, +VBG_KERNEL_REQUEST); if (!req1 || !req2) goto out_free; @@ -187,8 +195,8 @@ static int vbg_report_guest_info(struct vbg_dev *gdev) req2->additions_minor = VBG_VERSION_MINOR; req2->additions_build = VBG_VERSION_BUILD; req2->additions_revision = VBG_SVN_REV; - /* (no features defined yet) */ - req2->additions_features = 0; + req2->additions_features = + VMMDEV_GUEST_INFO2_ADDITIONS_FEATURES_REQUESTOR_INFO; strlcpy(req2->name, VBG_VERSION_STRING, sizeof(req2->name)); @@ -230,7 +238,8 @@ static int vbg_report_driver_status(struct vbg_dev *gdev, bool active) struct vmmdev_guest_status *req; int rc; - req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_REPORT_GUEST_STATUS); + req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_REPORT_GUEST_STATUS, + VBG_KERNEL_REQUEST); if (!req) return -ENOMEM; @@ -423,7 +432,8 @@ static int vbg_heartbeat_host_config(struct vbg_dev *gdev, bool enabled) struct vmmdev_heartbeat *req; int rc; - req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_HEARTBEAT_CONFIGURE); + req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_HEARTBEAT_CONFIGURE, + VBG_KERNEL_REQUEST); if (!req) return -ENOMEM; @@ -457,7 +467,8 @@ static int vbg_heartbeat_init(struct vbg_dev *gdev) gdev->guest_heartbeat_req = vbg_req_alloc( sizeof(*gdev->guest_heartbeat_req), - VMMDEVREQ_GUEST_HEARTBEAT); + VMMDEVREQ_GUEST_HEARTBEAT, + VBG_KERNEL_REQUEST); if (!gdev->guest_heartbeat_req) return -ENOMEM; @@ -528,7 +539,8 @@ static int vbg_reset_host_event_filter(struct vbg_dev *gdev, struct vmmdev_mask *req;
Re: [PATCH] mm, memory_hotplug: Fix the wrong usage of N_HIGH_MEMORY
On Wed 20-03-19 17:06:24, Baoquan He wrote: > On 03/20/19 at 09:46am, Michal Hocko wrote: > > On Wed 20-03-19 16:07:32, Baoquan He wrote: > > > In function node_states_check_changes_online(), N_HIGH_MEMORY is used > > > to substitute ZONE_HIGHMEM directly. This is not right. N_HIGH_MEMORY > > > always has value '3' if CONFIG_HIGHMEM=y, while ZONE_HIGHMEM's value > > > is not. It depends on whether CONFIG_ZONE_DMA/CONFIG_ZONE_DMA32 are > > > enabled. Obviously it's not true for CONFIG_ZONE_DMA32 on 32bit system, > > > and CONFIG_ZONE_DMA is also optional. > > > > > > Replace it with ZONE_HIGHMEM. > > > > N*MEMORY is confusing as hell but I am really curious whether we have > > ZONE_DMA32 and ZONE_HIGMEM together? > > Not sure. AFAIK, on x86_32 it can't be. > > > > > That being said N.*MEMORY is intended to check for nodes rather than > > zones so the patch looks good to me but I think the above explanation is > > misleading and will add even more mud to the picture when somebody tries > > to understand what the heck is going on here. > > Yes, agree. I also thought this again after I sent out patch, feel log is not > good. As you said, they are value of enum node_states and enum zone_type > separately. > > How about this: > > ~~~ > In function node_states_check_changes_online(), N_HIGH_MEMORY is used > to substitute ZONE_HIGHMEM directly. This is not right. N_HIGH_MEMORY > is to mark the memory state of node. Here zone index is checked, which > should be compared with 'ZONE_HIGHMEM' accordingly. > > Replace it with ZONE_HIGHMEM. OK, this is better. Thanks! Feel free to stamp the patch with Acked-by: Michal Hocko -- Michal Hocko SUSE Labs
Re: [PATCH v4 1/1] mm: introduce put_user_page*(), placeholder versions
On Tue, Mar 19, 2019 at 12:24:00PM -0700, John Hubbard wrote: > So, I could be persuaded either way. But given the lack of an visible perf > effects, and given that this could will get removed anyway because we'll > likely end up with set_page_dirty() called at GUP time instead...it seems > like it's probably OK to just leave it as is. Apart from ugly code generated, other argument might be Spectre-like attacks on these call. I would rather avoid indirect function calls whenever possible. And I don't think opencodded versions of these functions would look much worse. -- Kirill A. Shutemov
Re: [PATCH] virt: vbox: Implement passing requestor info to the host for VirtualBox 6.0.x
On Wed, Mar 20, 2019 at 10:35:19AM +0100, Hans de Goede wrote: > VirtualBox 6.0.x has a new feature where the guest kernel driver passes > info about the origin of the request (e.g. userspace or kernelspace) to > the hypervisor. > > If we do not pass this information then when running the 6.0.x userspace > guest-additions tools on a 6.0.x host, some requests will get denied > with a VERR_VERSION_MISMATCH error, breaking vboxservice.service and > the mounting of shared folders marked to be auto-mounted. > > This commit implements passing the requestor info to the host, fixing this. > > Cc: sta...@vger.kernel.org This feels like support for a "new feature", so why would this need to go to older kernels? It's not our fault that vb implemented a non-backwards-compatible change for their new release, right? So why should we be forced to add new features to stable kernels? I have no problem to add this for 5.2, but not for older stuff. thanks, greg k-h
Re: [PATCH] virt: vbox: Implement passing requestor info to the host for VirtualBox 6.0.x
Hi, On 20-03-19 10:46, Greg Kroah-Hartman wrote: On Wed, Mar 20, 2019 at 10:35:19AM +0100, Hans de Goede wrote: VirtualBox 6.0.x has a new feature where the guest kernel driver passes info about the origin of the request (e.g. userspace or kernelspace) to the hypervisor. If we do not pass this information then when running the 6.0.x userspace guest-additions tools on a 6.0.x host, some requests will get denied with a VERR_VERSION_MISMATCH error, breaking vboxservice.service and the mounting of shared folders marked to be auto-mounted. This commit implements passing the requestor info to the host, fixing this. Cc: sta...@vger.kernel.org This feels like support for a "new feature", so why would this need to go to older kernels? It's not our fault that vb implemented a non-backwards-compatible change for their new release, right? So why should we be forced to add new features to stable kernels? From a technical point of view I completely agree with you and I'm unhappy with this breakage after vb agreed with me to keep ABI compatibility so that we could add a version of the vboxguest driver to the mainline kernel. OTOH this is going to bite users out there, which is why I added the Cc: stable. But this is entirely your call. I have no problem to add this for 5.2, but not for older stuff. Can we at least at it as a fix to 5.1 ? It is not very adventurous. Regards, Hans
Re: [PATCH v2 2/2] mfd: Add leds MFD cell for intel_soc_pmic_chtwc
On Wed, 20 Mar 2019, Lee Jones wrote: > On Tue, 12 Feb 2019, Yauhen Kharuzhy wrote: > > > Add MFD cell for LEDs driver to the Intel Cherry Trail Whiskey Cove PMIC > > mfd device driver. > > > > Signed-off-by: Yauhen Kharuzhy > > --- > > drivers/mfd/intel_soc_pmic_chtwc.c | 1 + > > 1 file changed, 1 insertion(+) > > I'll fix up the subject line for you. > > In future, please do the following to see what is expected: > > git log --oneline -- Applied, by the way, thanks. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH v2 2/2] mfd: Add leds MFD cell for intel_soc_pmic_chtwc
On Tue, 12 Feb 2019, Yauhen Kharuzhy wrote: > Add MFD cell for LEDs driver to the Intel Cherry Trail Whiskey Cove PMIC > mfd device driver. > > Signed-off-by: Yauhen Kharuzhy > --- > drivers/mfd/intel_soc_pmic_chtwc.c | 1 + > 1 file changed, 1 insertion(+) I'll fix up the subject line for you. In future, please do the following to see what is expected: git log --oneline -- > diff --git a/drivers/mfd/intel_soc_pmic_chtwc.c > b/drivers/mfd/intel_soc_pmic_chtwc.c > index 64a3aece9c5e..be84bb2aa837 100644 > --- a/drivers/mfd/intel_soc_pmic_chtwc.c > +++ b/drivers/mfd/intel_soc_pmic_chtwc.c > @@ -60,6 +60,7 @@ static struct mfd_cell cht_wc_dev[] = { > .resources = cht_wc_ext_charger_resources, > }, > { .name = "cht_wcove_region", }, > + { .name = "cht_wcove_leds", }, > }; > > /* -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH v3 1/2] mfd: sec: Put one element structure initialisation on one line
On Wed, 13 Feb 2019, Stuart Menefy wrote: > Change the layout of the initialisation of structures with one > element to a single line of code. This keeps the coding style > consistent. > > Signed-off-by: Stuart Menefy > --- > drivers/mfd/sec-core.c | 58 > +- > 1 file changed, 19 insertions(+), 39 deletions(-) Applied, thanks. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
[PATCH RESEND] ARM: dts: exynos: Fix audio (microphone) routing on Odroid XU3
The name of CODEC input widget to which microphone is connected through the "Headphone" jack is "IN12" not "IN1". This fixes microphone support on Odroid XU3. Cc: # v4.14+ Signed-off-by: Sylwester Nawrocki --- arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi index 3a37267e6134..c3c2d85267da 100644 --- a/arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi @@ -22,7 +22,7 @@ "Headphone Jack", "HPL", "Headphone Jack", "HPR", "Headphone Jack", "MICBIAS", - "IN1", "Headphone Jack", + "IN12", "Headphone Jack", "Speakers", "SPKL", "Speakers", "SPKR", "I2S Playback", "Mixer DAI TX", -- 2.17.1
Re: [PATCH v3 2/2] mfd: sec: Add support for the RTC on S2MPA01
On Wed, 13 Feb 2019, Stuart Menefy wrote: > The RTC portion of the S2MPA01 appears to have the same > register layout as the S2MPS14. > > Reviewed-by: Krzysztof Kozlowski > Signed-off-by: Stuart Menefy Tags should be in chronological order. That way they can give us more information about the submission path. I'll fix this. Applied, thanks. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
[PATCH v2] clk: hi3660: Mark clk_gate_ufs_subsys as critical
clk_gate_ufs_subsys is a system bus clock, turning off it will introduce lockup issue during system suspend flow. Let's mark clk_gate_ufs_subsys as critical clock, thus keeps it on during system suspend and resume. Fixes: d374e6fd5088 ("clk: hisilicon: Add clock driver for hi3660 SoC") Cc: sta...@vger.kernel.org Cc: Zhong Kaihua Cc: John Stultz Cc: Zhangfei Gao Suggested-by: Dong Zhang Signed-off-by: Leo Yan --- drivers/clk/hisilicon/clk-hi3660.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/clk/hisilicon/clk-hi3660.c b/drivers/clk/hisilicon/clk-hi3660.c index f40419959656..794eeff0d5d2 100644 --- a/drivers/clk/hisilicon/clk-hi3660.c +++ b/drivers/clk/hisilicon/clk-hi3660.c @@ -163,8 +163,12 @@ static const struct hisi_gate_clock hi3660_crgctrl_gate_sep_clks[] = { "clk_isp_snclk_mux", CLK_SET_RATE_PARENT, 0x50, 17, 0, }, { HI3660_CLK_GATE_ISP_SNCLK2, "clk_gate_isp_snclk2", "clk_isp_snclk_mux", CLK_SET_RATE_PARENT, 0x50, 18, 0, }, + /* +* clk_gate_ufs_subsys is a system bus clock, mark it as critical +* clock and keep it on for system suspend and resume. +*/ { HI3660_CLK_GATE_UFS_SUBSYS, "clk_gate_ufs_subsys", "clk_div_sysbus", - CLK_SET_RATE_PARENT, 0x50, 21, 0, }, + CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, 0x50, 21, 0, }, { HI3660_PCLK_GATE_DSI0, "pclk_gate_dsi0", "clk_div_cfgbus", CLK_SET_RATE_PARENT, 0x50, 28, 0, }, { HI3660_PCLK_GATE_DSI1, "pclk_gate_dsi1", "clk_div_cfgbus", -- 2.17.1
Re: [PATCH 2/3] mm/sparse: Optimize sparse_add_one_section()
Hi Mike, On 03/20/19 at 09:56am, Mike Rapoport wrote: > @@ -697,16 +697,17 @@ int __meminit sparse_add_one_section(int nid, unsigned > long start_pfn, > > ret = sparse_index_init(section_nr, nid); > > if (ret < 0 && ret != -EEXIST) > > return ret; > > - ret = 0; > > - memmap = kmalloc_section_memmap(section_nr, nid, altmap); > > - if (!memmap) > > - return -ENOMEM; > > + > > usemap = __kmalloc_section_usemap(); > > - if (!usemap) { > > - __kfree_section_memmap(memmap, altmap); > > + if (!usemap) > > + return -ENOMEM; > > + memmap = kmalloc_section_memmap(section_nr, nid, altmap); > > + if (!memmap) { > > + kfree(usemap); > > If you are anyway changing this why not to switch to goto's for error > handling? I update code change as below, could you check if it's OK to you? Thanks Baoquan >From 39b679b6f34f6acbc05351be8569d23bae3c0458 Mon Sep 17 00:00:00 2001 From: Baoquan He Date: Fri, 15 Mar 2019 16:03:52 +0800 Subject: [PATCH] mm/sparse: Optimize sparse_add_one_section() Reorder the allocation of usemap and memmap since usemap allocation is much smaller and simpler. Otherwise hard work is done to make memmap ready, then have to rollback just because of usemap allocation failure. Meanwhile update the error handler to cover usemap allocation failure too. Signed-off-by: Baoquan He --- mm/sparse.c | 23 --- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/mm/sparse.c b/mm/sparse.c index a99e0b253927..0e842b924be6 100644 --- a/mm/sparse.c +++ b/mm/sparse.c @@ -699,20 +699,21 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn, ret = sparse_index_init(section_nr, nid); if (ret < 0 && ret != -EEXIST) return ret; - ret = 0; - memmap = kmalloc_section_memmap(section_nr, nid, altmap); - if (!memmap) - return -ENOMEM; + usemap = __kmalloc_section_usemap(); - if (!usemap) { - __kfree_section_memmap(memmap, altmap); + if (!usemap) return -ENOMEM; + memmap = kmalloc_section_memmap(section_nr, nid, altmap); + if (!memmap) { + ret = -ENOMEM; + goto out2; } + ret = 0; ms = __pfn_to_section(start_pfn); if (ms->section_mem_map & SECTION_MARKED_PRESENT) { ret = -EEXIST; - goto out; + goto out2; } /* @@ -724,11 +725,11 @@ int __meminit sparse_add_one_section(int nid, unsigned long start_pfn, section_mark_present(ms); sparse_init_one_section(ms, section_nr, memmap, usemap); + return ret; out: - if (ret < 0) { - kfree(usemap); - __kfree_section_memmap(memmap, altmap); - } + __kfree_section_memmap(memmap, altmap); +out2: + kfree(usemap); return ret; } -- 2.17.2
RE: [PATCH V7 2/5] pwm: Add i.MX TPM PWM driver support
Hi, Uwe Best Regards! Anson Huang > -Original Message- > From: Uwe Kleine-König [mailto:u.kleine-koe...@pengutronix.de] > Sent: 2019年3月20日 16:19 > To: Anson Huang > Cc: thierry.red...@gmail.com; robh...@kernel.org; mark.rutl...@arm.com; > shawn...@kernel.org; s.ha...@pengutronix.de; ker...@pengutronix.de; > feste...@gmail.com; li...@armlinux.org.uk; ota...@ossystems.com.br; > ste...@agner.ch; Leonard Crestez ; > schnitzelt...@gmail.com; Robin Gong ; linux- > p...@vger.kernel.org; devicet...@vger.kernel.org; linux-arm- > ker...@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx > > Subject: Re: [PATCH V7 2/5] pwm: Add i.MX TPM PWM driver support > > Hello, > > On Wed, Mar 20, 2019 at 05:06:21AM +, Anson Huang wrote: > > i.MX7ULP has TPM(Low Power Timer/Pulse Width Modulation Module) > > inside, it can support multiple PWM channels, all the channels share > > same counter and period setting, but each channel can configure its > > duty and polarity independently. > > > > There are several TPM modules in i.MX7ULP, the number of channels in > > TPM modules are different, it can be read from each TPM module's PARAM > > register. > > > > Signed-off-by: Anson Huang > > --- > > Changes since V6: > > - merge "config" and "enable" functions into ONE function > pwm_imx_tpm_apply_hw; > > - save computation for confiuring counter, the "round_state" > function will return > > the registers value directly; > > - improve the logic in .apply; > > - return error when there is still PWM active during suspend callback. > > --- > > drivers/pwm/Kconfig | 11 ++ > > drivers/pwm/Makefile | 1 + > > drivers/pwm/pwm-imx-tpm.c | 428 > > ++ > > 3 files changed, 440 insertions(+) > > create mode 100644 drivers/pwm/pwm-imx-tpm.c > > > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index > > 54f8238..3ea0391 100644 > > --- a/drivers/pwm/Kconfig > > +++ b/drivers/pwm/Kconfig > > @@ -210,6 +210,17 @@ config PWM_IMX27 > > To compile this driver as a module, choose M here: the module > > will be called pwm-imx27. > > > > +config PWM_IMX_TPM > > + tristate "i.MX TPM PWM support" > > + depends on ARCH_MXC || COMPILE_TEST > > + depends on HAVE_CLK && HAS_IOMEM > > + help > > + Generic PWM framework driver for i.MX7ULP TPM module, TPM's > full > > + name is Low Power Timer/Pulse Width Modulation Module. > > + > > + To compile this driver as a module, choose M here: the module > > + will be called pwm-imx-tpm. > > + > > config PWM_JZ4740 > > tristate "Ingenic JZ47xx PWM support" > > depends on MACH_INGENIC > > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index > > 448825e..c368599 100644 > > --- a/drivers/pwm/Makefile > > +++ b/drivers/pwm/Makefile > > @@ -19,6 +19,7 @@ obj-$(CONFIG_PWM_HIBVT) += pwm- > hibvt.o > > obj-$(CONFIG_PWM_IMG) += pwm-img.o > > obj-$(CONFIG_PWM_IMX1) += pwm-imx1.o > > obj-$(CONFIG_PWM_IMX27)+= pwm-imx27.o > > +obj-$(CONFIG_PWM_IMX_TPM) += pwm-imx-tpm.o > > obj-$(CONFIG_PWM_JZ4740) += pwm-jz4740.o > > obj-$(CONFIG_PWM_LP3943) += pwm-lp3943.o > > obj-$(CONFIG_PWM_LPC18XX_SCT) += pwm-lpc18xx-sct.o > > diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c > new > > file mode 100644 index 000..02403d0 > > --- /dev/null > > +++ b/drivers/pwm/pwm-imx-tpm.c > > @@ -0,0 +1,428 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright 2018-2019 NXP. > > + * > > + * Limitations: > > + * - The TPM counter and period counter are shared between > > + * multiple channels, so all channels should use same period > > + * settings. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define PWM_IMX_TPM_PARAM 0x4 > > +#define PWM_IMX_TPM_GLOBAL 0x8 > > +#define PWM_IMX_TPM_SC 0x10 > > +#define PWM_IMX_TPM_CNT0x14 > > +#define PWM_IMX_TPM_MOD0x18 > > +#define PWM_IMX_TPM_CnSC(n)(0x20 + (n) * 0x8) > > +#define PWM_IMX_TPM_CnV(n) (0x24 + (n) * 0x8) > > + > > +#define PWM_IMX_TPM_PARAM_CHAN GENMASK(7, > 0) > > + > > +#define PWM_IMX_TPM_SC_PS GENMASK(2, 0) > > +#define PWM_IMX_TPM_SC_CMODGENMASK(4, 3) > > +#define PWM_IMX_TPM_SC_CMOD_INC_EVERY_CLK BIT(3) > > #define PWM_IMX_TPM_SC_CMOD_INC_EVERY_CLK > FIELD_PREP(PWM_IMX_TPM_SC_CMOD, 1) > > ? Yes, it makes more sense. > > > +#define PWM_IMX_TPM_SC_CPWMS BIT(5) > > + > > +#define PWM_IMX_TPM_CnSC_CHF BIT(7) > > +#define PWM_IMX_TPM_CnSC_MSB BIT(5) > > +#define PWM_IMX_TPM_CnSC_MSA BIT(4) > > + > > +/* > > + * The reference manual describes this field as two separate bits. > > +The > > + * samantic of the two bits isn'
RE: [PATCH] hyperv: a potential NULL pointer dereference
On Thu, 14 Mar 2019, KY Srinivasan wrote: > > -Original Message- > > From: Kangjie Lu > > Sent: Wednesday, March 13, 2019 10:47 PM > > To: k...@umn.edu > > Cc: pakki...@umn.edu; KY Srinivasan ; Haiyang Zhang > > ; Stephen Hemminger > > ; Sasha Levin ; Thomas > > Gleixner ; Ingo Molnar ; Borislav > > Petkov ; H. Peter Anvin ; x...@kernel.org; > > linux-hyp...@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: [PATCH] hyperv: a potential NULL pointer dereference > > > > In case alloc_page, the fix returns -ENOMEM to avoid the potential > > NULL pointer dereference. > > > Thanks. > > > Signed-off-by: Kangjie Lu > Signed-off-by: K. Y. Srinivasan Did you mean: Reviewed-by or Acked-by? You cannot sign off on a patch from someone else which you are not picking up and transporting it further. Thanks, tglx
Re: [PATCH 2/2] Revert "x86/hpet: Reduce HPET counter read contention"
On Mon, 18 Mar 2019, Waiman Long wrote: > On 03/18/2019 04:44 AM, Zhenzhong Duan wrote: > > https://lkml.org/lkml/2019/3/15/101 > > > I think what Thomas was asking is to provide a REALISTIC use case where > TSC is wrecked and HPET is somehow not used and we have to fall back to > use PM timer. If such use case exists, I am sure Thomas will be happy to > take it. Exactly. Thanks, tglx
Re: scif_insert_vma()
On Sun, Mar 17, 2019 at 10:47:40PM -0700, Sudeep Dutt wrote: > On Mon, 2019-03-11 at 08:45 +0200, Jarkko Sakkinen wrote: > > Hi > > > > Just wondering what will happen if kzalloc() fails in scif_mmap.c. How > > it is recovered? I don't see anything in the VMA callbacks taking care > > of this. > > Hi Jarkko, > > scif_insert_vma(..) is called from scif_mmap(..) and scif_vma_open(..). > scif_mmap(..) checks for allocation failures but scif_vma_open(..) does > not on purpose. > > The vm_operations_struct open(..)/close(..) callbacks do not allow > returning errors. The driver will take a reference to the VMA private > data structure irrespective of whether the allocation during the > open(..) callback succeeds or fails. The close(..) callback cleans up > the data structures from the mmap(..) or open(..) callbacks if any. I'm doing allocations also in SGX vma_open callback and was grepping through kernel tree for how allocations were handled. Thanks for clarifying this. In SGX's case I ended up with allowing to fail in vma open and doing SIGBUS in the #PF handler if so... /Jarkko
Re: [PATCH] dt-bindings: reset: meson-g12a: Add missing USB2 PHY resets
On Mon, 2019-03-04 at 11:49 +0100, Neil Armstrong wrote: > The G12A Documentation lacked these 2 reset lines, but they are present and > used for each USB 2 PHYs. > > Add them to the dt-bindings for the upcoming USB support. > > Fixes: dbfc54534dfc ("dt-bindings: reset: meson: add g12a bindings") > Signed-off-by: Neil Armstrong > --- > include/dt-bindings/reset/amlogic,meson-g12a-reset.h | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/include/dt-bindings/reset/amlogic,meson-g12a-reset.h > b/include/dt-bindings/reset/amlogic,meson-g12a-reset.h > index 8063e8314eef..6d487c5eba2c 100644 > --- a/include/dt-bindings/reset/amlogic,meson-g12a-reset.h > +++ b/include/dt-bindings/reset/amlogic,meson-g12a-reset.h > @@ -51,7 +51,10 @@ > #define RESET_SD_EMMC_A 44 > #define RESET_SD_EMMC_B 45 > #define RESET_SD_EMMC_C 46 > -/* 47-60 */ > +/* 47 */ > +#define RESET_USB_PHY20 48 > +#define RESET_USB_PHY21 49 > +/* 50-60 */ > #define RESET_AUDIO_CODEC61 > /* 62-63 */ > /* RESET2 */ Thank you, applied to reset/fixes with Martin's review tag.
Re: [PATCH 2/2] platform/x86: intel_pmc_core: Allow to dump debug registers on S0ix failure
On Monday, March 18, 2019 5:01:06 PM CET Rajneesh Bhardwaj wrote: > On Mon, Mar 18, 2019 at 08:18:56AM -0700, Rajat Jain wrote: > > On Mon, Mar 18, 2019 at 2:31 AM Somayaji, Vishwanath > > wrote: > > > > > > > > > > > > >-Original Message- > > > >From: Rajat Jain > > > >Sent: Thursday, March 14, 2019 3:51 AM > > > >To: Bhardwaj, Rajneesh ; Somayaji, > > > >Vishwanath > > > >; Darren Hart ; Andy > > > >Shevchenko ; platform-driver-...@vger.kernel.org; > > > >linux- > > > >ker...@vger.kernel.org > > > >Cc: Rajat Jain ; furq...@google.com; > > > >evgr...@google.com; rajatxj...@gmail.com > > > >Subject: [PATCH 2/2] platform/x86: intel_pmc_core: Allow to dump debug > > > >registers on S0ix failure > > > > > > > >Add a module parameter which when enabled, will check on resume, if the > > > >last S0ix attempt was successful. If not, the driver would provide > > > >helpful debug information (which gets latched during the failed suspend > > > >attempt) to debug the S0ix failure. > > > > > > > >This information is very useful to debug S0ix failures. Specially since > > > >the latched debug information will be lost (over-written) if the system > > > >attempts to go into runtime (or imminent) S0ix again after that failed > > > >suspend attempt. > > > > > > > >Signed-off-by: Rajat Jain > > > >--- > > > > drivers/platform/x86/intel_pmc_core.c | 86 +++ > > > > drivers/platform/x86/intel_pmc_core.h | 7 +++ > > > > 2 files changed, 93 insertions(+) > > > > > > > >diff --git a/drivers/platform/x86/intel_pmc_core.c > > > >b/drivers/platform/x86/intel_pmc_core.c > > > >index 55578d07610c..b1f4405a27ce 100644 > > > >--- a/drivers/platform/x86/intel_pmc_core.c > > > >+++ b/drivers/platform/x86/intel_pmc_core.c > > > >@@ -20,6 +20,7 @@ > > > > #include > > > > #include > > > > #include > > > >+#include > > > > #include > > > > > > > > #include > > > >@@ -890,9 +891,94 @@ static int pmc_core_remove(struct platform_device > > > >*pdev) > > > > return 0; > > > > } > > > > > > > >+#ifdef CONFIG_PM_SLEEP > > > >+ > > > >+static bool warn_on_s0ix_failures; > > > >+module_param(warn_on_s0ix_failures, bool, 0644); > > > >+MODULE_PARM_DESC(warn_on_s0ix_failures, "Check and warn for S0ix > > > >failures"); > > > >+ > > > >+static int pmc_core_suspend(struct device *dev) > > > >+{ > > > >+ struct pmc_dev *pmcdev = dev_get_drvdata(dev); > > > >+ > > > >+ /* Save PC10 and S0ix residency for checking later */ > > > >+ if (warn_on_s0ix_failures && > > > >+ !rdmsrl_safe(MSR_PKG_C10_RESIDENCY, &pmcdev->pc10_counter) > > > >&& > > > >+ !pmc_core_dev_state_get(pmcdev, &pmcdev->s0ix_counter)) > > > >+ pmcdev->check_counters = true; > > > >+ else > > > >+ pmcdev->check_counters = false; > > > >+ > > > >+ return 0; > > > >+} > > > >+ > > > >+static inline bool pc10_failed(struct pmc_dev *pmcdev) > > > >+{ > > > >+ u64 pc10_counter; > > > >+ > > > >+ if (!rdmsrl_safe(MSR_PKG_C10_RESIDENCY, &pc10_counter) && > > > >+ pc10_counter == pmcdev->pc10_counter) > > > >+ return true; > > > >+ else > > > >+ return false; > > > >+} > > > >+ > > > >+static inline bool s0ix_failed(struct pmc_dev *pmcdev) > > > >+{ > > > >+ u64 s0ix_counter; > > > >+ > > > >+ if (!pmc_core_dev_state_get(pmcdev, &s0ix_counter) && > > > >+ s0ix_counter == pmcdev->s0ix_counter) > > > >+ return true; > > > >+ else > > > >+ return false; > > > >+} > > > >+ > > > >+static int pmc_core_resume(struct device *dev) > > > >+{ > > > >+ struct pmc_dev *pmcdev = dev_get_drvdata(dev); > > > >+ > > > >+ if (!pmcdev->check_counters) > > > >+ return 0; > > > >+ > > > >+ if (pc10_failed(pmcdev)) { > > > >+ dev_info(dev, "PC10 entry had failed (PC10 cnt=0x%llx)\n", > > > >+ pmcdev->pc10_counter); > > > >+ } else if (s0ix_failed(pmcdev)) { > > > >+ > > > >+ const struct pmc_bit_map **maps = pmcdev->map- > > > >>slps0_dbg_maps; > > > >+ const struct pmc_bit_map *map; > > > >+ int offset = pmcdev->map->slps0_dbg_offset; > > > >+ u32 data; > > > >+ > > > >+ dev_warn(dev, "S0ix entry had failed (S0ix cnt=%llu)\n", > > > >+ pmcdev->s0ix_counter); > > > >+ while (*maps) { > > > >+ map = *maps; > > > >+ data = pmc_core_reg_read(pmcdev, offset); > > > >+ offset += 4; > > > >+ while (map->name) { > > > >+ dev_warn(dev, "SLP_S0_DBG: %-32s\tState: > > > >%s\n", > > > >+ map->name, > > > >+ data & map->bit_mask ? "Yes" : > > > >"No"); > > > >+ ++map; > > > >+ } > > > >+ ++maps;
Re: [PATCH] staging: rtlwifi: Fix potential NULL pointer dereference
On Wed, Mar 13, 2019 at 11:13:34AM -0500, Aditya Pakki wrote: > phydm.internal is allocated using kzalloc which is used multiple > times without a check for NULL pointer. This patch avoids such a > scenario. > > Signed-off-by: Aditya Pakki > --- > drivers/staging/rtlwifi/phydm/rtl_phydm.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/staging/rtlwifi/phydm/rtl_phydm.c > b/drivers/staging/rtlwifi/phydm/rtl_phydm.c > index 9930ed954abb..37c7fcb72b65 100644 > --- a/drivers/staging/rtlwifi/phydm/rtl_phydm.c > +++ b/drivers/staging/rtlwifi/phydm/rtl_phydm.c > @@ -181,6 +181,9 @@ static int rtl_phydm_init_priv(struct rtl_priv *rtlpriv, > rtlpriv->phydm.internal = > kzalloc(sizeof(struct phy_dm_struct), GFP_KERNEL); > > + if (!rtlpriv->phydm.internal) Don't put a blank line between the allocation and the check. They're part of the same code and it's weird to double space the code. regards, dan carpenter
Re: [PATCH] rtc: cros-ec: Fail suspend/resume if wake IRQ can't be configured
On 15/03/2019 11:51:12-0700, Stephen Boyd wrote: > If we encounter a failure during suspend where this RTC was programmed > to wakeup the system from suspend, but that wakeup couldn't be > configured because the system didn't support wakeup interrupts, we'll > run into the following warning: > > Unbalanced IRQ 166 wake disable > WARNING: CPU: 7 PID: 3071 at kernel/irq/manage.c:669 > irq_set_irq_wake+0x108/0x278 > > This happens because the suspend process isn't aborted when the RTC > fails to configure the wakeup IRQ. Instead, we continue suspending the > system and then another suspend callback fails the suspend process and > "unwinds" the previously suspended drivers by calling their resume > callbacks. When we get back to resuming this RTC driver, we'll call > disable_irq_wake() on an IRQ that hasn't been configured for wake. > > Let's just fail suspend/resume here if we can't configure the system to > wake and the user has chosen to wakeup with this device. This fixes this > warning and makes the code more robust in case there are systems out > there that can't wakeup from suspend on this line but the user has > chosen to do so. > > Cc: Enric Balletbo i Serra > Cc: Evan Green > Cc: Benson Leung > Cc: Guenter Roeck > Signed-off-by: Stephen Boyd > --- > drivers/rtc/rtc-cros-ec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Applied, thanks. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH 18/52] virtio-fs: Map cache using the values from the capabilities
* Liu Bo (obuil.li...@gmail.com) wrote: > On Mon, Dec 10, 2018 at 9:57 AM Vivek Goyal wrote: > > > > Instead of assuming we had the fixed bar for the cache, use the > > value from the capabilities. > > > > Signed-off-by: Dr. David Alan Gilbert > > --- > > fs/fuse/virtio_fs.c | 32 +--- > > 1 file changed, 17 insertions(+), 15 deletions(-) > > > > diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c > > index 60d496c16841..55bac1465536 100644 > > --- a/fs/fuse/virtio_fs.c > > +++ b/fs/fuse/virtio_fs.c > > @@ -14,11 +14,6 @@ > > #include > > #include "fuse_i.h" > > > > -enum { > > - /* PCI BAR number of the virtio-fs DAX window */ > > - VIRTIO_FS_WINDOW_BAR = 2, > > -}; > > - > > /* List of virtio-fs device instances and a lock for the list */ > > static DEFINE_MUTEX(virtio_fs_mutex); > > static LIST_HEAD(virtio_fs_instances); > > @@ -518,7 +513,7 @@ static int virtio_fs_setup_dax(struct virtio_device > > *vdev, struct virtio_fs *fs) > > struct dev_pagemap *pgmap; > > struct pci_dev *pci_dev; > > phys_addr_t phys_addr; > > - size_t len; > > + size_t bar_len; > > int ret; > > u8 have_cache, cache_bar; > > u64 cache_offset, cache_len; > > @@ -551,17 +546,13 @@ static int virtio_fs_setup_dax(struct virtio_device > > *vdev, struct virtio_fs *fs) > > } > > > > /* TODO handle case where device doesn't expose BAR? */ > > - ret = pci_request_region(pci_dev, VIRTIO_FS_WINDOW_BAR, > > -"virtio-fs-window"); > > + ret = pci_request_region(pci_dev, cache_bar, "virtio-fs-window"); > > if (ret < 0) { > > dev_err(&vdev->dev, "%s: failed to request window BAR\n", > > __func__); > > return ret; > > } > > > > - phys_addr = pci_resource_start(pci_dev, VIRTIO_FS_WINDOW_BAR); > > - len = pci_resource_len(pci_dev, VIRTIO_FS_WINDOW_BAR); > > - > > mi = devm_kzalloc(&pci_dev->dev, sizeof(*mi), GFP_KERNEL); > > if (!mi) > > return -ENOMEM; > > @@ -586,6 +577,17 @@ static int virtio_fs_setup_dax(struct virtio_device > > *vdev, struct virtio_fs *fs) > > pgmap->ref = &mi->ref; > > pgmap->type = MEMORY_DEVICE_FS_DAX; > > > > + phys_addr = pci_resource_start(pci_dev, cache_bar); > > + bar_len = pci_resource_len(pci_dev, cache_bar); > > + > > + if (cache_offset + cache_len > bar_len) { > > + dev_err(&vdev->dev, > > + "%s: cache bar shorter than cap offset+len\n", > > + __func__); > > + return -EINVAL; > > + } > > + phys_addr += cache_offset; > > + > > /* Ideally we would directly use the PCI BAR resource but > > * devm_memremap_pages() wants its own copy in pgmap. So > > * initialize a struct resource from scratch (only the start > > @@ -594,7 +596,7 @@ static int virtio_fs_setup_dax(struct virtio_device > > *vdev, struct virtio_fs *fs) > > pgmap->res = (struct resource){ > > .name = "virtio-fs dax window", > > .start = phys_addr, > > - .end = phys_addr + len, > > + .end = phys_addr + cache_len, > > Just in case you haven't noticed/fixed this problem, it should be > > + .end = phys_addr + cache_len - 1, > > because resource_size() counts %size as "end - start + 1". > The end result of the above is a "conflicting page map" warning when > specifying a second virtio-fs pci device. Thanks for spotting this! I think we'd seen that message once but not noticed where from. > I'll send a patch for this, and feel free to take it along with the > patchset if needed. > Dave > thanks, > liubo > > > }; > > > > fs->window_kaddr = devm_memremap_pages(&pci_dev->dev, pgmap); > > @@ -607,10 +609,10 @@ static int virtio_fs_setup_dax(struct virtio_device > > *vdev, struct virtio_fs *fs) > > return ret; > > > > fs->window_phys_addr = phys_addr; > > - fs->window_len = len; > > + fs->window_len = cache_len; > > > > - dev_dbg(&vdev->dev, "%s: window kaddr 0x%px phys_addr 0x%llx len > > %zu\n", > > - __func__, fs->window_kaddr, phys_addr, len); > > + dev_dbg(&vdev->dev, "%s: cache kaddr 0x%px phys_addr 0x%llx len > > %llx\n", > > + __func__, fs->window_kaddr, phys_addr, cache_len); > > > > fs->dax_dev = alloc_dax(fs, NULL, &virtio_fs_dax_ops); > > if (!fs->dax_dev) > > -- > > 2.13.6 > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH] thunderbolt: Fix to check for kmemdup failure
On Mon, Mar 18, 2019 at 05:44:17PM -0500, Aditya Pakki wrote: > Memory allocated via kmemdup might fail and return a NULL pointer. > This patch adds a check on the return value of kmemdup and passes the > error upstream. > > Signed-off-by: Aditya Pakki Applied, thanks!
[PATCH] of: Drop redundant check in linker section OF match table
Existing check of `fn` against NULL inside OF match table is redundant. Remove the check. Signed-off-by: Mukesh Ojha Cc: Rob Herring Cc: Frank Rowand Cc: Pantelis Antoniou Cc: devicet...@vger.kernel.org --- include/linux/of.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/of.h b/include/linux/of.h index e240992..b86c00a 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -1283,13 +1283,13 @@ static inline int of_get_available_child_count(const struct device_node *np) static const struct of_device_id __of_table_##name \ __used __section(__##table##_of_table) \ = { .compatible = compat, \ -.data = (fn == (fn_type)NULL) ? fn : fn } +.data = fn } #else #define _OF_DECLARE(table, name, compat, fn, fn_type) \ static const struct of_device_id __of_table_##name \ __attribute__((unused)) \ = { .compatible = compat, \ -.data = (fn == (fn_type)NULL) ? fn : fn } +.data = fn } #endif typedef int (*of_init_fn_2)(struct device_node *, struct device_node *); -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH] thunderbolt: Fix to check return value of ida_simple_get
On Tue, Mar 19, 2019 at 12:22:59PM -0500, Aditya Pakki wrote: > ida_simple_get on failure can return an error. The patch ensures that > the dev_set_name is set on non failure cases. > > Signed-off-by: Aditya Pakki > --- > drivers/thunderbolt/xdomain.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c > index e27dd8beb94b..b1768f595259 100644 > --- a/drivers/thunderbolt/xdomain.c > +++ b/drivers/thunderbolt/xdomain.c > @@ -772,7 +772,9 @@ static void enumerate_services(struct tb_xdomain *xd) > svc->dev.bus = &tb_bus_type; > svc->dev.type = &tb_service_type; > svc->dev.parent = &xd->dev; > - dev_set_name(&svc->dev, "%s.%d", dev_name(&xd->dev), svc->id); > + if (svc->id >= 0) > + dev_set_name(&svc->dev, "%s.%d", dev_name(&xd->dev), > + svc->id); I think the correct way to handle this case is to cleanup svc right after the allocation fails and then break out without enumerating more services.
Re: [PATCH V7 2/5] pwm: Add i.MX TPM PWM driver support
Hello Anson, On Wed, Mar 20, 2019 at 10:17:50AM +, Anson Huang wrote: > > On Wed, Mar 20, 2019 at 05:06:21AM +, Anson Huang wrote: > > > + /* make sure counter is disabled for programming prescale */ > > > + val = readl(tpm->base + PWM_IMX_TPM_SC); > > > + saved_cmod = FIELD_GET(PWM_IMX_TPM_SC_CMOD, val); > > > + if (saved_cmod) { > > > + val &= ~PWM_IMX_TPM_SC_CMOD; > > > + writel(val, tpm->base + PWM_IMX_TPM_SC); > > > > I thought we agreed on not stopping the counter if the PS field isn't > > changed? > > If the PS field no need to change, the round state should already return the > period > equal to current period settings, so this function will NOT be called, right? > > if (real_state.period != tpm->real_period) { > if (tpm->user_count > 1) { > ret = -EBUSY; > goto exit; > } > > pwm_imx_tpm_setup_period(chip, param); > tpm->real_period = real_state.period; > } If the PS field is already right .period might still not match as its value depends on SC.PS and MOD.MOD. > > > + val &= ~PWM_IMX_TPM_SC_PS; > > > + val |= FIELD_PREP(PWM_IMX_TPM_SC_PS, p.prescale); > > > + writel(val, tpm->base + PWM_IMX_TPM_SC); > > > + > > > + /* > > > + * set period count: according to RM, the MOD register is > > > + * updated immediately after CMOD[1:0] = 2b'00 above > > > + */ > > > > So the current period isn't completed? That's wrong. > > So you mean we have to wait for the current period to finish here? > I did NOT know this, so we have to compare the counter value with Yeah, see https://patchwork.ozlabs.org/patch/1021566/ which documents this but waits for feedback by Thierry since some time. > the MOD value until they match then proceed the period change? If the hardware doesn't support you here (usually it does) I think it's not reliable enough to try to sync in software. I assume you can get the right wave form if you don't change SC.PS but only MOD.MOD? So maybe the sanest approach is to refuse changing SC.PS if the PWM is running. Or disable (which also should wait for the current period to finish), poll for the end (or use an irq?) and then setup the new configuration. > > > +{ > > > + struct imx_tpm_pwm_chip *tpm = to_imx_tpm_pwm_chip(chip); > > > + struct pwm_state c; > > > + u32 val, sc_val; > > > + u64 tmp; > > > + > > > + pwm_imx_tpm_get_state(chip, pwm, &c); > > > + > > > + if (state.duty_cycle != c.duty_cycle) { > > > + /* set duty counter */ > > > + tmp = readl(tpm->base + PWM_IMX_TPM_MOD) & PWM_IMX_TPM_MOD_MOD; > > > + tmp *= state.duty_cycle; > > > + val = DIV_ROUND_CLOSEST_ULL(tmp, state.period); > > > + writel(val, tpm->base + PWM_IMX_TPM_CnV(pwm->hwpwm)); > > > + } > > > + > > > + if (state.enabled != c.enabled) { > > > > This is wrong. If the PWM was running (c.enabled == true) and you are > > supposed to disable (state.enabled == false) you enable the hardware once > > more. > > A little confused here, as the case you assume, inside this block, there is > another > check for state.enabled, if it is false, the polarity will be set to channel > disabled mode, > the polarity setting is combined together with the enable status here, am I > wrong? Ah, you're right. I missed that probably because I saw register accesses after the state.enabled != c.enabled check. > > > + val |= (state.polarity == PWM_POLARITY_NORMAL) ? > > > + FIELD_PREP(PWM_IMX_TPM_CnSC_ELS, 0x2) : > > > + FIELD_PREP(PWM_IMX_TPM_CnSC_ELS, 0x1); > > > > Introduce PWM_IMX_TPM_CnSC_ELS_POLARITY_NORMAL and use it > > together with PWM_IMX_TPM_CnSC_ELS_POLARITY_INVERSED here. If you > > put the FIELD_PREP into the definition the line doesn't get excessively > > long. > > > > I put the FIELD_PREP into definition, the line still long, but I agree using > definition is better. > > #define PWM_IMX_TPM_CnSC_ELS_INVERSED FIELD_PREP(PWM_IMX_TPM_CnSC_ELS, 1) > #define PWM_IMX_TPM_CnSC_ELS_NORMAL FIELD_PREP(PWM_IMX_TPM_CnSC_ELS, 2) > > val |= (state->polarity == PWM_POLARITY_NORMAL) ? > PWM_IMX_TPM_CnSC_ELS_NORMAL : > PWM_IMX_TPM_CnSC_ELS_INVERSED; > > > Maybe also add > > > > #define PWM_IMX_TPM_CnSC_ELS_INACTIVE FIELD_PREP(PWM_IMX_TPM_CnSC_ELS, > > 0) > > > > I did NOT use the FIELD_PREP(PWM_IMX_TPM_CnSC_ELS, 0) at all, so why add it? > I don't quite understand. You use it implicitly in pwm_imx_tpm_apply_hw() if state.enabled == false and c.enabled == true: val = readl(tpm->base + PWM_IMX_TPM_CnSC(pwm->hwpwm)); val &= ~(PWM_IMX_TPM_CnSC_ELS | ...); ... writel(val, tpm->base + PWM_IMX_TPM_CnSC(pwm->hwpwm)); > > > +static int pwm_imx_tpm_apply(struct pwm_chip *chip, > > > + struct pwm_device *pwm, > > > +
Re:
> On Mar 19, 2019, at 2:41 PM, Maxim Levitsky wrote: > > Date: Tue, 19 Mar 2019 14:45:45 +0200 > Subject: [PATCH 0/9] RFC: NVME VFIO mediated device > > Hi everyone! > > In this patch series, I would like to introduce my take on the problem of > doing > as fast as possible virtualization of storage with emphasis on low latency. > > In this patch series I implemented a kernel vfio based, mediated device that > allows the user to pass through a partition and/or whole namespace to a guest. Hey Maxim! I'm really excited to see this series, as it aligns to some extent with what we discussed in last year's KVM Forum VFIO BoF. There's no arguing that we need a better story to efficiently virtualise NVMe devices. So far, for Qemu-based VMs, Changpeng's vhost-user-nvme is the best attempt at that. However, I seem to recall there was some pushback from qemu-devel in the sense that they would rather see investment in virtio-blk. I'm not sure what's the latest on that work and what are the next steps. The pushback drove the discussion towards pursuing an mdev approach, which is why I'm excited to see your patches. What I'm thinking is that passing through namespaces or partitions is very restrictive. It leaves no room to implement more elaborate virtualisation stacks like replicating data across multiple devices (local or remote), storage migration, software-managed thin provisioning, encryption, deduplication, compression, etc. In summary, anything that requires software intervention in the datapath. (Worth noting: vhost-user-nvme allows all of that to be easily done in SPDK's bdev layer.) These complicated stacks should probably not be implemented in the kernel, though. So I'm wondering whether we could talk about mechanisms to allow efficient and performant userspace datapath intervention in your approach or pursue a mechanism to completely offload the device emulation to userspace (and align with what SPDK has to offer). Thoughts welcome! Felipe
RE: [RFC PATCH] x86/entry/64: randomize kernel stack offset upon syscall
From: Andy Lutomirski > Sent: 18 March 2019 20:16 ... > > As a result this patch introduces 8 bits of randomness > > (bits 4 - 11 are randomized, bits 0-3 must be zero due to stack alignment) > > after pt_regs location on the thread stack. > > The amount of randomness can be adjusted based on how much of the > > stack space we wish/can trade for security. > > Why do you need four zero bits at the bottom? x86_64 Linux only > maintains 8 byte stack alignment. ISTR that the gcc developers arbitrarily changed the alignment a few years ago. If the stack is only 8 byte aligned and you allocate a variable that requires 16 byte alignment you need gcc to generate the extra stack frame to align the stack. I don't remember seeing the relevant gcc options on the linux gcc command lines. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH 2/2] clk: meson: meson8b: add the video decoder clock trees
Hi Martin, thanks for looking into the video decoder for meson8! On Tue, Mar 19, 2019 at 11:00 PM Martin Blumenstingl wrote: > > This adds the four video decoder clock trees. > > VDEC_1 is split into two paths on Meson8b and Meson8m2: > - input mux called "vdec_1_sel" > - two dividers ("vdec_1_1_div" and "vdec_1_2_div") and gates ("vdec_1_1" > and "vdec_1_2") > - and an output mux (probably glitch-free) called "vdec_1" Yes, all vdec clocks have a glitch-free mux to be able to safely adjust the frequency on the fly, although in practice it's barely used. > On Meson8 the VDEC_1 tree is simpler because there's only one path: > - input mux called "vdec_1_sel" > - divider ("vdec_1_1_div") and gate ("vdec_1_1") > - (the gate is used as output directly, there's no mux) > > The VDEC_HCODEC and VDEC_2 clocks are simple composite clocks each > consisting of an input mux, divider and a gate. > > The VDEC_HEVC clock seems to have two paths similar to the VDEC_1 clock. > However, the register offsets of the second clock path is not known. > Amlogic's 3.10 kernel (which is used as reference) sets > HHI_VDEC2_CLK_CNTL[31] to 1 before changing the VDEC_HEVC clock and back > to 0 afterwards. For now, leave a TODO comment and only add the first > path. > Looking at aml-3.10/drivers/amlogic/amports/m8b/vdec_clk.c, it's weird indeed. They seem to copy the divider's value to the same place (HHI_VDEC2_CLK_CNTL[16~23]), and the only thing that stands out is enabling HHI_VDEC2_CLK_CNTL[31]. Then again they don't make use of that codepath at all, so who knows.. > Signed-off-by: Martin Blumenstingl > --- > drivers/clk/meson/meson8b.c | 312 > drivers/clk/meson/meson8b.h | 17 +- > 2 files changed, 328 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > index 8e091c2d10e6..37cf0f01bb5d 100644 > --- a/drivers/clk/meson/meson8b.c > +++ b/drivers/clk/meson/meson8b.c > @@ -1902,6 +1902,257 @@ static struct clk_regmap meson8b_vpu = { > }, > }; > > +static const char * const meson8b_vdec_parent_names[] = { > + "fclk_div4", "fclk_div3", "fclk_div5", "fclk_div7", "mpll2", "mpll1" > +}; > + > +static struct clk_regmap meson8b_vdec_1_sel = { > + .data = &(struct clk_regmap_mux_data){ > + .offset = HHI_VDEC_CLK_CNTL, > + .mask = 0x3, > + .shift = 9, > + .flags = CLK_MUX_ROUND_CLOSEST, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "vdec_1_sel", > + .ops = &clk_regmap_mux_ops, > + .parent_names = meson8b_vdec_parent_names, > + .num_parents = ARRAY_SIZE(meson8b_vdec_parent_names), > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_vdec_1_1_div = { > + .data = &(struct clk_regmap_div_data){ > + .offset = HHI_VDEC_CLK_CNTL, > + .shift = 0, > + .width = 7, > + .flags = CLK_DIVIDER_ROUND_CLOSEST, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "vdec_1_1_div", > + .ops = &clk_regmap_divider_ops, > + .parent_names = (const char *[]){ "vdec_1_sel" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_vdec_1_1 = { > + .data = &(struct clk_regmap_gate_data){ > + .offset = HHI_VDEC_CLK_CNTL, > + .bit_idx = 8, > + }, > + .hw.init = &(struct clk_init_data) { > + .name = "vdec_1_1", > + .ops = &clk_regmap_gate_ops, > + .parent_names = (const char *[]){ "vdec_1_1_div" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_vdec_1_2_div = { > + .data = &(struct clk_regmap_div_data){ > + .offset = HHI_VDEC3_CLK_CNTL, > + .shift = 0, > + .width = 7, > + .flags = CLK_DIVIDER_ROUND_CLOSEST, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "vdec_1_2_div", > + .ops = &clk_regmap_divider_ops, > + .parent_names = (const char *[]){ "vdec_1_sel" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +static struct clk_regmap meson8b_vdec_1_2 = { > + .data = &(struct clk_regmap_gate_data){ > + .offset = HHI_VDEC3_CLK_CNTL, > + .bit_idx = 8, > + }, > + .hw.init = &(struct clk_init_data) { > + .name = "vdec_1_2", > + .ops = &clk_regmap_gate_ops, > + .parent_names = (const char *[]){ "vdec_1_2_div" }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +
RE: [PATCH 02/25] tracing: Improve "if" macro code generation
From: > Peter Zijlstra > Sent: 18 March 2019 15:39 > > With CONFIG_PROFILE_ALL_BRANCHES, the "if" macro converts the > conditional to an array index. This can cause GCC to create horrible > code. When there are nested ifs, the generated code uses register > values to encode branching decisions. > > Make it easier for GCC to optimize by keeping the conditional as a > conditional rather than converting it to an integer. This shrinks the > generated code quite a bit, and also makes the code sane enough for > objtool to understand. ... > include/linux/compiler.h |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > --- a/include/linux/compiler.h > +++ b/include/linux/compiler.h > @@ -67,7 +67,7 @@ void ftrace_likely_update(struct ftrace_ > .line = __LINE__, \ > }; \ > __r = !!(cond); \ Is that (or maybe just the !!) needed any more?? > - __f.miss_hit[__r]++; > \ > + __r ? __f.miss_hit[1]++ : __f.miss_hit[0]++;\ > __r;\ > })) > #endif /* CONFIG_PROFILE_ALL_BRANCHES */ > David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH v2] kernel/trace: fix watchdog soft lockup
On Fri, 30 Nov 2018 at 16:18, Steven Rostedt wrote: > > On Fri, 30 Nov 2018 15:56:22 +0100 > Anders Roxell wrote: > > > When building a allmodconfig kernel for arm64 and boot that in qemu, > > CONFIG_FTRACE_STARTUP_TEST gets enabled and that takes time so the > > watchdog expires and prints out a message like this: > > 'watchdog: BUG: soft lockup - CPU#0 stuck for 22s! [swapper/0:1]' > > Depending on what the what test gets called from init_trace_selftests() > > it stays minutes in the loop. > > Rework so that function cond_resched() gets called in the > > init_trace_selftests loop. > > > > This looks fine to me. Should it be marked for stable, and pushed into > this release cycle, or wait till the next merge window? Steve, I'm sorry for the late reply and no its not urgent. Maybe it can make it into v5.2 ? Cheers, Anders > > -- Steve > > > Co-developed-by: Arnd Bergmann > > Signed-off-by: Arnd Bergmann > > Signed-off-by: Anders Roxell > > --- > > kernel/trace/trace.c | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > > index 5706599ed534..109becbc81ca 100644 > > --- a/kernel/trace/trace.c > > +++ b/kernel/trace/trace.c > > @@ -1547,6 +1547,10 @@ static __init int init_trace_selftests(void) > > pr_info("Running postponed tracer tests:\n"); > > > > list_for_each_entry_safe(p, n, &postponed_selftests, list) { > > + /* This loop can take minutes when sanitizers are enabled, so > > + * lets make sure we allow RCU processing. > > + */ > > + cond_resched(); > > ret = run_tracer_selftest(p->type); > > /* If the test fails, then warn and remove from > > available_tracers */ > > if (ret < 0) { >
Re: [PATCH 1/3] mm/sparse: Clean up the obsolete code comment
On Wed, Mar 20, 2019 at 03:35:38PM +0800, Baoquan He wrote: > /* > - * returns the number of sections whose mem_maps were properly > - * set. If this is <=0, then that means that the passed-in > - * map was not consumed and must be freed. > + * sparse_add_one_section - add a memory section > + * @nid: The node to add section on > + * @start_pfn: start pfn of the memory range > + * @altmap: device page map > + * > + * Return 0 on success and an appropriate error code otherwise. > */ I think it's worth documenting what those error codes are. Seems to be just -ENOMEM and -EEXIST, but it'd be nice for users to know what they can expect under which circumstances. Also, -EEXIST is a bad errno to return here: $ errno EEXIST EEXIST 17 File exists What file? I think we should be using -EBUSY instead in case this errno makes it back to userspace: $ errno EBUSY EBUSY 16 Device or resource busy
RE: [PATCH V7 2/5] pwm: Add i.MX TPM PWM driver support
Hi, Uwe Best Regards! Anson Huang > -Original Message- > From: Uwe Kleine-König [mailto:u.kleine-koe...@pengutronix.de] > Sent: 2019年3月20日 18:58 > To: Anson Huang > Cc: thierry.red...@gmail.com; robh...@kernel.org; mark.rutl...@arm.com; > shawn...@kernel.org; s.ha...@pengutronix.de; ker...@pengutronix.de; > feste...@gmail.com; li...@armlinux.org.uk; ota...@ossystems.com.br; > ste...@agner.ch; Leonard Crestez ; > schnitzelt...@gmail.com; Robin Gong ; linux- > p...@vger.kernel.org; devicet...@vger.kernel.org; linux-arm- > ker...@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-imx > > Subject: Re: [PATCH V7 2/5] pwm: Add i.MX TPM PWM driver support > > Hello Anson, > > On Wed, Mar 20, 2019 at 10:17:50AM +, Anson Huang wrote: > > > On Wed, Mar 20, 2019 at 05:06:21AM +, Anson Huang wrote: > > > > + /* make sure counter is disabled for programming prescale */ > > > > + val = readl(tpm->base + PWM_IMX_TPM_SC); > > > > + saved_cmod = FIELD_GET(PWM_IMX_TPM_SC_CMOD, val); > > > > + if (saved_cmod) { > > > > + val &= ~PWM_IMX_TPM_SC_CMOD; > > > > + writel(val, tpm->base + PWM_IMX_TPM_SC); > > > > > > I thought we agreed on not stopping the counter if the PS field isn't > changed? > > > > If the PS field no need to change, the round state should already > > return the period equal to current period settings, so this function will > > NOT > be called, right? > > > > if (real_state.period != tpm->real_period) { > > if (tpm->user_count > 1) { > > ret = -EBUSY; > > goto exit; > > } > > > > pwm_imx_tpm_setup_period(chip, param); > > tpm->real_period = real_state.period; > > } > > If the PS field is already right .period might still not match as its value > depends on SC.PS and MOD.MOD. Ah, my bad... I did NOT know what I was thinking... Yes, I will add the PS check to decide whether disabling counter.. > > > > > + val &= ~PWM_IMX_TPM_SC_PS; > > > > + val |= FIELD_PREP(PWM_IMX_TPM_SC_PS, p.prescale); > > > > + writel(val, tpm->base + PWM_IMX_TPM_SC); > > > > + > > > > + /* > > > > +* set period count: according to RM, the MOD register is > > > > +* updated immediately after CMOD[1:0] = 2b'00 above > > > > +*/ > > > > > > So the current period isn't completed? That's wrong. > > > > So you mean we have to wait for the current period to finish here? > > I did NOT know this, so we have to compare the counter value with > > Yeah, see > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch > work.ozlabs.org%2Fpatch%2F1021566%2F&data=02%7C01%7Canson.h > uang%40nxp.com%7C626a6f5603f74ae0d37e08d6ad22e774%7C686ea1d3bc > 2b4c6fa92cd99c5c301635%7C0%7C0%7C636886762757876916&sdata=r > wu%2BUo3GlRX8j4lXSOVuAs7n1yEuP5P2W6vhY%2BjiXdQ%3D&reserve > d=0 which documents this but waits for feedback by Thierry since some time. > > > the MOD value until they match then proceed the period change? > > If the hardware doesn't support you here (usually it does) I think it's not > reliable enough to try to sync in software. I assume you can get the right > wave form if you don't change SC.PS but only MOD.MOD? So maybe the > sanest approach is to refuse changing SC.PS if the PWM is running. > > Or disable (which also should wait for the current period to finish), poll for > the end (or use an irq?) and then setup the new configuration. Let me try to poll the TOF (timer overflow) before setup the new configuration. And will also need to add timeout for the polling, what shoud the timeout value be, 100ms? As ideally the max period can be very large, several seconds or even large, so is the 100mS good here? > > > > > +{ > > > > + struct imx_tpm_pwm_chip *tpm = to_imx_tpm_pwm_chip(chip); > > > > + struct pwm_state c; > > > > + u32 val, sc_val; > > > > + u64 tmp; > > > > + > > > > + pwm_imx_tpm_get_state(chip, pwm, &c); > > > > + > > > > + if (state.duty_cycle != c.duty_cycle) { > > > > + /* set duty counter */ > > > > + tmp = readl(tpm->base + PWM_IMX_TPM_MOD) & > PWM_IMX_TPM_MOD_MOD; > > > > + tmp *= state.duty_cycle; > > > > + val = DIV_ROUND_CLOSEST_ULL(tmp, state.period); > > > > + writel(val, tpm->base + PWM_IMX_TPM_CnV(pwm- > >hwpwm)); > > > > + } > > > > + > > > > + if (state.enabled != c.enabled) { > > > > > > This is wrong. If the PWM was running (c.enabled == true) and you > > > are supposed to disable (state.enabled == false) you enable the > > > hardware once more. > > > > A little confused here, as the case you assume, inside this block, > > there is another check for state.enabled, if it is false, the polarity > > will be set to channel disabled mode, the polarity setting is combined > together with the enable status here, am I wrong? > > Ah, y
Re: [PATCH 2/3] mm/sparse: Optimize sparse_add_one_section()
On Wed, Mar 20, 2019 at 06:13:18PM +0800, Baoquan He wrote: > + if (!memmap) { > + ret = -ENOMEM; > + goto out2; Documentation/process/coding-style: Choose label names which say what the goto does or why the goto exists. An example of a good name could be ``out_free_buffer:`` if the goto frees ``buffer``. Avoid using GW-BASIC names like ``err1:`` and ``err2:``, as you would have to renumber them if you ever add or remove exit paths, and they make correctness difficult to verify anyway.
Re: [PATCH] arm64: dts: juno: Enable smmu_pcie for Juno r1/r2
Hi Leo, On 20/03/2019 08:31, Leo Yan wrote: Though PCIe controller has been enabled on Juno r1/r2, but it misses to enable its connected SMMU. From the testing, even without set this SMMU status property to 'okay', the PCIe NIC device still works well. Since the SMMU is not enabled in DT binding and its iommu_groups is not created properly, finally this prevents to enable vfio-pci for NIC device with KVM. FWIW, whilst it might appear to be fine, there are still potential issues once the DMA API sees this SMMU and starts using it, which is why this particular change remains as one of my oldest local hacks that I've never yet considered upstreamable. The IOMMU DMA ops happen to work for light usage now as a side-effect of 122fac030e912 combined with the current top-down allocation behaviour, but as soon as IOVA usage/fragmentation leads to 32-bit DMA addresses below 0x800, or full 40-bit addresses, being allocated then data loss and other problems will happen (for the reasons explained on the other thread). Similarly, it's also going to be fragile to any internal changes in the IOVA allocator. Until we have a decent solution for handling such 'windowed' DMA restrictions (there do exist other systems with similar behaviour) I'd be very wary of enabling the PCIe SMMU for all users who may not be aware of the caveats. Given that the lack of stream IDs and SR-IOV support prevents Juno from being a realistic virtualisation platform anyway, I'm not convinced there's enough benefit to justify the risk. Thanks, Robin. After reviewing the code in dts, Juno r1/r2 both have enabled PCIe controller but Juno r0 board (in juno.dts) doesn't contain PCIe controller; hence this patch only change SMMU status property to 'okay' for Juno r1/r2 dts files for the sake of only enabling it for Juno r1/r2 and avoiding touching Juno r0. Committer testing on Juno r2: Before: root@debian:~# tree /sys/kernel/iommu_groups/ /sys/kernel/iommu_groups/ ├── 0 │ ├── devices │ │ ├── 7ffb.ohci -> ../../../../devices/platform/7ffb.ohci │ │ └── 7ffc.ehci -> ../../../../devices/platform/7ffc.ehci │ ├── reserved_regions │ └── type └── 1 ├── devices │ └── 2007.etr -> ../../../../devices/platform/2007.etr ├── reserved_regions └── type 7 directories, 4 files After: root@debian:~# tree /sys/kernel/iommu_groups/ /sys/kernel/iommu_groups/ ├── 0 │ ├── devices │ │ ├── :00:00.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0 │ │ ├── :01:00.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0 │ │ ├── :02:01.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:01.0 │ │ ├── :02:02.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:02.0 │ │ ├── :02:03.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:03.0 │ │ ├── :02:0c.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:0c.0 │ │ ├── :02:10.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:10.0 │ │ ├── :02:1f.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:1f.0 │ │ ├── :03:00.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:01.0/:03:00.0 │ │ └── :08:00.0 -> ../../../../devices/platform/4000.pcie/pci:00/:00:00.0/:01:00.0/:02:1f.0/:08:00.0 │ ├── reserved_regions │ └── type ├── 1 │ ├── devices │ │ ├── 7ffb.ohci -> ../../../../devices/platform/7ffb.ohci │ │ └── 7ffc.ehci -> ../../../../devices/platform/7ffc.ehci │ ├── reserved_regions │ └── type └── 2 ├── devices │ └── 2007.etr -> ../../../../devices/platform/2007.etr ├── reserved_regions └── type 19 directories, 6 files Signed-off-by: Leo Yan --- arch/arm64/boot/dts/arm/juno-r1.dts | 4 arch/arm64/boot/dts/arm/juno-r2.dts | 4 2 files changed, 8 insertions(+) diff --git a/arch/arm64/boot/dts/arm/juno-r1.dts b/arch/arm64/boot/dts/arm/juno-r1.dts index b2b7ced633cf..67e161a6272a 100644 --- a/arch/arm64/boot/dts/arm/juno-r1.dts +++ b/arch/arm64/boot/dts/arm/juno-r1.dts @@ -226,6 +226,10 @@ status = "okay"; }; +&smmu_pcie { + status = "okay"; +}; + &pcie_ctlr { status = "okay"; }; diff --git a/arch/arm64/boot/dts/arm/juno-r2.dts b/arch/arm64/boot/dts/arm/juno-r2.dts index ab77adb4f3c2..0e1c5c814b01 100644 --- a/arch/arm64/boot/dts/arm/juno-r2.dts +++ b/arch/arm64/boot/dts/arm/juno-r2.dts @@ -226,6 +226,10 @@ status = "okay"; }; +&smmu_pcie { + status = "okay";
Re: [PATCH 2/3] mm/sparse: Optimize sparse_add_one_section()
On Wed, Mar 20, 2019 at 06:13:18PM +0800, Baoquan He wrote: > Hi Mike, > > On 03/20/19 at 09:56am, Mike Rapoport wrote: > > @@ -697,16 +697,17 @@ int __meminit sparse_add_one_section(int nid, > unsigned long start_pfn, > > > ret = sparse_index_init(section_nr, nid); > > > if (ret < 0 && ret != -EEXIST) > > > return ret; > > > - ret = 0; > > > - memmap = kmalloc_section_memmap(section_nr, nid, altmap); > > > - if (!memmap) > > > - return -ENOMEM; > > > + > > > usemap = __kmalloc_section_usemap(); > > > - if (!usemap) { > > > - __kfree_section_memmap(memmap, altmap); > > > + if (!usemap) > > > + return -ENOMEM; > > > + memmap = kmalloc_section_memmap(section_nr, nid, altmap); > > > + if (!memmap) { > > > + kfree(usemap); > > > > If you are anyway changing this why not to switch to goto's for error > > handling? > > I update code change as below, could you check if it's OK to you? > > Thanks > Baoquan > > From 39b679b6f34f6acbc05351be8569d23bae3c0458 Mon Sep 17 00:00:00 2001 > From: Baoquan He > Date: Fri, 15 Mar 2019 16:03:52 +0800 > Subject: [PATCH] mm/sparse: Optimize sparse_add_one_section() > > Reorder the allocation of usemap and memmap since usemap allocation > is much smaller and simpler. Otherwise hard work is done to make > memmap ready, then have to rollback just because of usemap allocation > failure. > > Meanwhile update the error handler to cover usemap allocation failure > too. > > Signed-off-by: Baoquan He > --- > mm/sparse.c | 23 --- > 1 file changed, 12 insertions(+), 11 deletions(-) > > diff --git a/mm/sparse.c b/mm/sparse.c > index a99e0b253927..0e842b924be6 100644 > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -699,20 +699,21 @@ int __meminit sparse_add_one_section(int nid, unsigned > long start_pfn, > ret = sparse_index_init(section_nr, nid); > if (ret < 0 && ret != -EEXIST) > return ret; > - ret = 0; > - memmap = kmalloc_section_memmap(section_nr, nid, altmap); > - if (!memmap) > - return -ENOMEM; > + > usemap = __kmalloc_section_usemap(); > - if (!usemap) { > - __kfree_section_memmap(memmap, altmap); > + if (!usemap) > return -ENOMEM; > + memmap = kmalloc_section_memmap(section_nr, nid, altmap); > + if (!memmap) { > + ret = -ENOMEM; > + goto out2; I'd name the label out_free_usemap. > } > > + ret = 0; > ms = __pfn_to_section(start_pfn); > if (ms->section_mem_map & SECTION_MARKED_PRESENT) { > ret = -EEXIST; > - goto out; > + goto out2; > } I've missed this previously, but it seems that this check can be moved before the allocations, which simplifies the code a bit more. > > /* > @@ -724,11 +725,11 @@ int __meminit sparse_add_one_section(int nid, unsigned > long start_pfn, > section_mark_present(ms); > sparse_init_one_section(ms, section_nr, memmap, usemap); > > + return ret; > out: > - if (ret < 0) { > - kfree(usemap); > - __kfree_section_memmap(memmap, altmap); > - } > + __kfree_section_memmap(memmap, altmap); > +out2: > + kfree(usemap); > return ret; > } > > -- > 2.17.2 > -- Sincerely yours, Mike.
Re: [PATCH] rcu: Allow to eliminate softirq processing from rcutree
On 2019-03-19 20:26:13 [-0400], Joel Fernandes wrote: > > @@ -2769,19 +2782,121 @@ static void invoke_rcu_callbacks(struct rcu_data > > *rdp) > > { > > if (unlikely(!READ_ONCE(rcu_scheduler_fully_active))) > > return; > > - if (likely(!rcu_state.boost)) { > > - rcu_do_batch(rdp); > > - return; > > - } > > - invoke_rcu_callbacks_kthread(); > > + rcu_do_batch(rdp); > > Looks like a nice change, but one question... > > Consider the case where rcunosoftirq boot option is not passed. > > Before, if RCU_BOOST=y, then callbacks would be invoked in rcuc threads if > possible, by those threads being woken up from within the softirq context > (in invoke_rcu_callbacks). > > Now, if RCU_BOOST=y, then callbacks would only be invoked in softirq context > and not in the threads at all. Because rcu_softirq_enabled = false, so the > path executes: > rcu_read_unlock_special() -> > raise_softirq_irqsoff() -> > rcu_process_callbacks_si() -> > rcu_process_callbacks() -> > invoke_rcu_callbacks() -> > rcu_do_batch() > > This seems like a behavioral change to me. This makes the callbacks always > execute from the softirq context and not the threads when boosting is > configured. IMO in the very least, such behavioral change should be > documented in the change. > > One way to fix this I think could be, if boosting is enabled, then set > rcu_softirq_enabled to false by default so the callbacks are still executed > in the rcuc threads. > > Did I miss something? Sorry if I did, thanks! So with all the swaps and reorder we talking about this change: diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 0a719f726e149..82810483bfc6c 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -2306,20 +2306,6 @@ static void rcu_core_si(struct softirq_action *h) rcu_core(); } -/* - * Schedule RCU callback invocation. If the running implementation of RCU - * does not support RCU priority boosting, just do a direct call, otherwise - * wake up the per-CPU kernel kthread. Note that because we are running - * on the current CPU with softirqs disabled, the rcu_cpu_kthread_task - * cannot disappear out from under us. - */ -static void invoke_rcu_callbacks(struct rcu_data *rdp) -{ - if (unlikely(!READ_ONCE(rcu_scheduler_fully_active))) - return; - rcu_do_batch(rdp); -} - static void rcu_wake_cond(struct task_struct *t, int status) { /* @@ -2330,6 +2316,19 @@ static void rcu_wake_cond(struct task_struct *t, int status) wake_up_process(t); } +static void invoke_rcu_core_kthread(void) +{ + struct task_struct *t; + unsigned long flags; + + local_irq_save(flags); + __this_cpu_write(rcu_data.rcu_cpu_has_work, 1); + t = __this_cpu_read(rcu_data.rcu_cpu_kthread_task); + if (t != NULL && t != current) + rcu_wake_cond(t, __this_cpu_read(rcu_data.rcu_cpu_kthread_status)); + local_irq_restore(flags); +} + static bool rcu_softirq_enabled = true; static int __init rcunosoftirq_setup(char *str) @@ -2339,26 +2338,33 @@ static int __init rcunosoftirq_setup(char *str) } __setup("rcunosoftirq", rcunosoftirq_setup); +/* + * Schedule RCU callback invocation. If the running implementation of RCU + * does not support RCU priority boosting, just do a direct call, otherwise + * wake up the per-CPU kernel kthread. Note that because we are running + * on the current CPU with softirqs disabled, the rcu_cpu_kthread_task + * cannot disappear out from under us. + */ +static void invoke_rcu_callbacks(struct rcu_data *rdp) +{ + if (unlikely(!READ_ONCE(rcu_scheduler_fully_active))) + return; + if (rcu_state.boost || rcu_softirq_enabled) + invoke_rcu_core_kthread(); + rcu_do_batch(rdp); +} + /* * Wake up this CPU's rcuc kthread to do RCU core processing. */ static void invoke_rcu_core(void) { - unsigned long flags; - struct task_struct *t; - if (!cpu_online(smp_processor_id())) return; - if (rcu_softirq_enabled) { + if (rcu_softirq_enabled) raise_softirq(RCU_SOFTIRQ); - } else { - local_irq_save(flags); - __this_cpu_write(rcu_data.rcu_cpu_has_work, 1); - t = __this_cpu_read(rcu_data.rcu_cpu_kthread_task); - if (t != NULL && t != current) - rcu_wake_cond(t, __this_cpu_read(rcu_data.rcu_cpu_kthread_status)); - local_irq_restore(flags); - } + else + invoke_rcu_core_kthread(); } static void rcu_cpu_kthread_park(unsigned int cpu) @@ -2426,7 +2432,8 @@ static int __init rcu_spawn_core_kthreads(void) per_cpu(rcu_data.rcu_cpu_has_work, cpu) = 0; if (!IS_ENABLED(CONFIG_RCU_BOOST) && !rcu_softirq_enabled)
Re: [PATCH] pwm: img: Turn final 'else if' into 'else' in img_pwm_config
On Thu, Mar 07, 2019 at 03:36:28PM -0700, Nathan Chancellor wrote: > When building with -Wsometimes-uninitialized, Clang warns: > > drivers/pwm/pwm-img.c:126:13: error: variable 'timebase' is used > uninitialized whenever 'if' condition is false > [-Werror,-Wsometimes-uninitialized] > > The final else if functions as an else; make that explicit so that Clang > understands that timebase cannot be used uninitialized. > > Link: https://github.com/ClangBuiltLinux/linux/issues/400 > Signed-off-by: Nathan Chancellor > --- > drivers/pwm/pwm-img.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied, thanks! Thierry signature.asc Description: PGP signature
Re: [PATCH] pwm: tiehrpwm: Update shadow register for disabling PWMs
On Tue, Mar 12, 2019 at 02:38:46PM +0530, Vignesh Raghavendra wrote: > From: Christoph Vogtländer > > It must be made sure that immediate mode is not already set, when > modifying shadow register value in ehrpwm_pwm_disable(). Otherwise > modifications to the action-qualifier continuous S/W force > register(AQSFRC) will be done in the active register. > This may happen when both channels are being disabled. In this case, > only the first channel state will be recorded as disabled in the shadow > register. Later, when enabling the first channel again, the second > channel would be enabled as well. Setting RLDCSF to zero, first, ensures > that the shadow register is updated as desired. > > Fixes: 38dabd91ff0b ("pwm: tiehrpwm: Fix disabling of output of PWMs") > Signed-off-by: Christoph Vogtländer > [vigne...@ti.com: Improve commit message] > Signed-off-by: Vignesh Raghavendra > --- > drivers/pwm/pwm-tiehrpwm.c | 2 ++ > 1 file changed, 2 insertions(+) Applied, thanks. Thierry signature.asc Description: PGP signature
Re: [PATCH v2] rcu: Allow to eliminate softirq processing from rcutree
On 2019-03-19 12:44:19 [+0100], To Paul E. McKenney wrote: > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index 0f31b79eb6761..0a719f726e149 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c … > +/* > + * Spawn per-CPU RCU core processing kthreads. > + */ > +static int __init rcu_spawn_core_kthreads(void) > +{ > + int cpu; > + > + for_each_possible_cpu(cpu) > + per_cpu(rcu_data.rcu_cpu_has_work, cpu) = 0; > + if (!IS_ENABLED(CONFIG_RCU_BOOST) && !rcu_softirq_enabled) and this needs to become - if (!IS_ENABLED(CONFIG_RCU_BOOST) && !rcu_softirq_enabled) + if (!IS_ENABLED(CONFIG_RCU_BOOST) && rcu_softirq_enabled) With this change and hunk that I just sent to Joel I get thee three RCU modes with and without BOOST booted. Unless there is something (and Paul agrees that the Joel hunk is correct) I would post a v3 with those changes included. > + return 0; > + WARN_ONCE(smpboot_register_percpu_thread(&rcu_cpu_thread_spec), "%s: > Could not start rcub kthread, OOM is now expected behavior\n", __func__); > + return 0; > +} > +early_initcall(rcu_spawn_core_kthreads); Sebastian