[PATCH] usb: gadget: uvc: Fix return value in case of error
If this memory allocation fail, we will return 0, which means success. Return -ENOMEM instead. Signed-off-by: Christophe JAILLET --- drivers/usb/gadget/function/uvc_configfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c index 66753ba..31125a4 100644 --- a/drivers/usb/gadget/function/uvc_configfs.c +++ b/drivers/usb/gadget/function/uvc_configfs.c @@ -2023,7 +2023,7 @@ static int uvcg_streaming_class_allow_link(struct config_item *src, if (!data) { kfree(*class_array); *class_array = NULL; - ret = PTR_ERR(data); + ret = -ENOMEM; goto unlock; } cl_arr = *class_array; -- 2.7.4 --- L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast. https://www.avast.com/antivirus
[for-next][PATCH 4/7] tracing: Use outer () on __get_str() definition
From: Daniel Bristot de Oliveira __get_str(str)'s definition includes a (char *) operator overloading that is not protected with outer (). This patch adds () around __get_str()'s definition, enabling some code cleanup. Link: http://lkml.kernel.org/r/20ac1a10c2ec4ccd23e4a8ef34101fb6e4157d37.1467407618.git.bris...@redhat.com Cc: Trond Myklebust Cc: Anna Schumaker Cc: Ingo Molnar Suggested-by: Steven Rostedt Reviewed-by: Steven Rostedt Signed-off-by: Daniel Bristot de Oliveira Signed-off-by: Steven Rostedt --- include/trace/perf.h | 2 +- include/trace/trace_events.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/trace/perf.h b/include/trace/perf.h index 88de5c205e86..04fe68bbe767 100644 --- a/include/trace/perf.h +++ b/include/trace/perf.h @@ -15,7 +15,7 @@ ((__entry->__data_loc_##field >> 16) & 0x) #undef __get_str -#define __get_str(field) (char *)__get_dynamic_array(field) +#define __get_str(field) ((char *)__get_dynamic_array(field)) #undef __get_bitmask #define __get_bitmask(field) (char *)__get_dynamic_array(field) diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h index 80679a9fae65..467e12f780d8 100644 --- a/include/trace/trace_events.h +++ b/include/trace/trace_events.h @@ -256,7 +256,7 @@ TRACE_MAKE_SYSTEM_STR(); ((__entry->__data_loc_##field >> 16) & 0x) #undef __get_str -#define __get_str(field) (char *)__get_dynamic_array(field) +#define __get_str(field) ((char *)__get_dynamic_array(field)) #undef __get_bitmask #define __get_bitmask(field) \ -- 2.8.1
[for-next][PATCH 5/7] tracing, RAS: Cleanup on __get_str() usage
From: Daniel Bristot de Oliveira __get_str(msg) does not need (char *) operator overloading to access mgs's elements anymore. This patch substitutes ((char *)__get_str(msg))[0] usage to __get_str(msg)[0]. It is just a code cleanup, no changes on tracepoint ABI. Link: http://lkml.kernel.org/r/6f2db5be7705da2cb483923320c91283d7c712a7.1467407618.git.bris...@redhat.com Cc: Trond Myklebust Cc: Anna Schumaker Cc: Ingo Molnar Reviewed-by: Steven Rostedt Signed-off-by: Daniel Bristot de Oliveira Signed-off-by: Steven Rostedt --- include/ras/ras_event.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h index 1443d79e4fe6..1791a12cfa85 100644 --- a/include/ras/ras_event.h +++ b/include/ras/ras_event.h @@ -147,7 +147,7 @@ TRACE_EVENT(mc_event, __entry->error_count, mc_event_error_type(__entry->error_type), __entry->error_count > 1 ? "s" : "", - ((char *)__get_str(msg))[0] ? " " : "", + __get_str(msg)[0] ? " " : "", __get_str(msg), __get_str(label), __entry->mc_index, @@ -157,7 +157,7 @@ TRACE_EVENT(mc_event, __entry->address, 1 << __entry->grain_bits, __entry->syndrome, - ((char *)__get_str(driver_detail))[0] ? " " : "", + __get_str(driver_detail)[0] ? " " : "", __get_str(driver_detail)) ); -- 2.8.1
[for-next][PATCH 1/7] tracing: Using for_each_set_bit() to simplify trace_pid_write()
From: Wei Yongjun Using for_each_set_bit() to simplify the code. Link: http://lkml.kernel.org/r/1467645004-11169-1-git-send-email-weiyj...@163.com Signed-off-by: Wei Yongjun Signed-off-by: Steven Rostedt --- kernel/trace/trace.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 5fd53a7847bc..dade4c9559cc 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -517,13 +517,9 @@ int trace_pid_write(struct trace_pid_list *filtered_pids, if (filtered_pids) { /* copy the current bits to the new max */ - pid = find_first_bit(filtered_pids->pids, -filtered_pids->pid_max); - while (pid < filtered_pids->pid_max) { + for_each_set_bit(pid, filtered_pids->pids, +filtered_pids->pid_max) { set_bit(pid, pid_list->pids); - pid = find_next_bit(filtered_pids->pids, - filtered_pids->pid_max, - pid + 1); nr_pids++; } } -- 2.8.1
[for-next][PATCH 0/7] tracing: Some last updates for 4.8
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next Head SHA1: 78aebca2c955c1c5aeb48e12645e13fe3c3461f2 Daniel Bristot de Oliveira (4): tracing: Use outer () on __get_str() definition tracing, RAS: Cleanup on __get_str() usage tracing: Use __get_str() when manipulating strings printk, tracing: Avoiding unneeded blank lines Namhyung Kim (1): ftrace: Reduce size of function graph entries Tom Zanussi (1): tracing: Have HIST_TRIGGERS select TRACING Wei Yongjun (1): tracing: Using for_each_set_bit() to simplify trace_pid_write() fs/nfs/nfs4trace.h| 4 ++-- fs/nfs/nfstrace.h | 4 ++-- include/linux/ftrace.h| 12 include/ras/ras_event.h | 4 ++-- include/trace/events/printk.h | 12 ++-- include/trace/perf.h | 2 +- include/trace/trace_events.h | 2 +- kernel/trace/Kconfig | 1 + kernel/trace/trace.c | 8 ++-- kernel/trace/trace.h | 11 +++ kernel/trace/trace_entries.h | 4 ++-- 11 files changed, 42 insertions(+), 22 deletions(-)
[for-next][PATCH 2/7] tracing: Have HIST_TRIGGERS select TRACING
From: Tom Zanussi The kbuild test robot reported a compile error if HIST_TRIGGERS was enabled but nothing else that selected TRACING was configured in. HIST_TRIGGERS should directly select it and not rely on anything else to do it. Link: http://lkml.kernel.org/r/57791866.8080...@linux.intel.com Reported-by: kbuild test robot Fixes: 7ef224d1d0e3a ("tracing: Add 'hist' event trigger command") Signed-off-by: Tom Zanussi Signed-off-by: Steven Rostedt --- kernel/trace/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig index fafeaf803bd0..f4b86e8ca1e7 100644 --- a/kernel/trace/Kconfig +++ b/kernel/trace/Kconfig @@ -542,6 +542,7 @@ config HIST_TRIGGERS bool "Histogram triggers" depends on ARCH_HAVE_NMI_SAFE_CMPXCHG select TRACING_MAP + select TRACING default n help Hist triggers allow one or more arbitrary trace event fields -- 2.8.1
[for-next][PATCH 7/7] printk, tracing: Avoiding unneeded blank lines
From: Daniel Bristot de Oliveira Printk messages often finish with '\n' to cause a new line. But as each tracepoint is already printed in a new line, printk messages that finish with '\n' ends up adding a blank line to the trace output. For example: kworker/0:1-86[000] d...46.006949: console: [ 46.006946] usb 1-3: USB disconnect, device number 3 kworker/2:2-374 [002] d...48.699342: console: [ 48.699339] usb 1-3: new high-speed USB device number 4 using ehci-pci kworker/2:2-374 [002] d...49.041450: console: [ 49.041448] usb 1-3: New USB device found, idVendor=5986, idProduct=0 To avoid unneeded blank lines, this patch checks if the printk message finishes with '\n', if so, it cut is off the '\n' to avoid blank lines. In a patched kernel, the same messages are printed without extra blank lines. For example: kworker/0:4-185 [000] d...23.641738: console: [ 23.641736] usb 1-3: USB disconnect, device number 3 kworker/0:4-185 [000] d...24.918703: console: [ 24.918700] usb 1-3: new high-speed USB device number 4 using ehci-pci kworker/0:4-185 [000] d...25.228308: console: [ 25.228306] usb 1-3: New USB device found, idVendor=5986, idProduct=02d5 Link: http://lkml.kernel.org/r/c350fb2521baaf681a1b4d67981ca0e900108e8e.1467407618.git.bris...@redhat.com Cc: Trond Myklebust Cc: Anna Schumaker Cc: Ingo Molnar Reviewed-by: Steven Rostedt Signed-off-by: Daniel Bristot de Oliveira Signed-off-by: Steven Rostedt --- include/trace/events/printk.h | 8 1 file changed, 8 insertions(+) diff --git a/include/trace/events/printk.h b/include/trace/events/printk.h index 542a7558154a..f350170059c6 100644 --- a/include/trace/events/printk.h +++ b/include/trace/events/printk.h @@ -16,6 +16,14 @@ TRACE_EVENT(console, ), TP_fast_assign( + /* +* Each trace entry is printed in a new line. +* If the msg finishes with '\n', cut it off +* to avoid blank lines in the trace. +*/ + if ((len > 0) && (text[len-1] == '\n')) + len -= 1; + memcpy(__get_str(msg), text, len); __get_str(msg)[len] = 0; ), -- 2.8.1
[for-next][PATCH 6/7] tracing: Use __get_str() when manipulating strings
From: Daniel Bristot de Oliveira Use __get_str(str) rather than __get_dynamic_array(str) when deadling with strings. It is just a code cleanup, no changes on tracepoint ABI. Link: http://lkml.kernel.org/r/ea260df91817411cca2a1f3db2abd88860094788.1467407618.git.bris...@redhat.com Cc: Trond Myklebust Cc: Anna Schumaker Cc: Ingo Molnar Cc: linux-...@vger.kernel.org Suggested-by: Steven Rostedt Reviewed-by: Steven Rostedt Signed-off-by: Daniel Bristot de Oliveira Signed-off-by: Steven Rostedt --- fs/nfs/nfs4trace.h| 4 ++-- fs/nfs/nfstrace.h | 4 ++-- include/trace/events/printk.h | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/nfs/nfs4trace.h b/fs/nfs/nfs4trace.h index 9c150b153782..cfb8f7ce5cf6 100644 --- a/fs/nfs/nfs4trace.h +++ b/fs/nfs/nfs4trace.h @@ -1235,8 +1235,8 @@ DECLARE_EVENT_CLASS(nfs4_idmap_event, len = 0; __entry->error = error < 0 ? error : 0; __entry->id = id; - memcpy(__get_dynamic_array(name), name, len); - ((char *)__get_dynamic_array(name))[len] = 0; + memcpy(__get_str(name), name, len); + __get_str(name)[len] = 0; ), TP_printk( diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h index 0b9e5cc9a747..31c7763b94d5 100644 --- a/fs/nfs/nfstrace.h +++ b/fs/nfs/nfstrace.h @@ -707,9 +707,9 @@ TRACE_EVENT(nfs_sillyrename_unlink, __entry->dev = dir->i_sb->s_dev; __entry->dir = NFS_FILEID(dir); __entry->error = error; - memcpy(__get_dynamic_array(name), + memcpy(__get_str(name), data->args.name.name, len); - ((char *)__get_dynamic_array(name))[len] = 0; + __get_str(name)[len] = 0; ), TP_printk( diff --git a/include/trace/events/printk.h b/include/trace/events/printk.h index c008bc99f9fa..542a7558154a 100644 --- a/include/trace/events/printk.h +++ b/include/trace/events/printk.h @@ -16,8 +16,8 @@ TRACE_EVENT(console, ), TP_fast_assign( - memcpy(__get_dynamic_array(msg), text, len); - ((char *)__get_dynamic_array(msg))[len] = 0; + memcpy(__get_str(msg), text, len); + __get_str(msg)[len] = 0; ), TP_printk("%s", __get_str(msg)) -- 2.8.1
[for-next][PATCH 3/7] ftrace: Reduce size of function graph entries
From: Namhyung Kim Currently ftrace_graph_ent{,_entry} and ftrace_graph_ret{,_entry} struct can have padding bytes at the end due to alignment in 64-bit data type. As these data are recorded so frequently, those paddings waste non-negligible space. As the ring buffer maintains alignment properly for each architecture, just to remove the extra padding using 'packed' attribute. ftrace_graph_ent_entry: 24 -> 20 ftrace_graph_ret_entry: 48 -> 44 Also I moved the 'overrun' field in struct ftrace_graph_ret to minimize the padding in the middle. Tested on x86_64 only. Link: http://lkml.kernel.org/r/1467197808-13578-1-git-send-email-namhy...@kernel.org Cc: Ingo Molnar Cc: linux-a...@vger.kernel.org Signed-off-by: Namhyung Kim Signed-off-by: Steven Rostedt --- include/linux/ftrace.h | 12 kernel/trace/trace.h | 11 +++ kernel/trace/trace_entries.h | 4 ++-- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index 66a36a815f0a..7d565afe35d2 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -754,23 +754,27 @@ static inline void ftrace_init(void) { } /* * Structure that defines an entry function trace. + * It's already packed but the attribute "packed" is needed + * to remove extra padding at the end. */ struct ftrace_graph_ent { unsigned long func; /* Current function */ int depth; -}; +} __packed; /* * Structure that defines a return function trace. + * It's already packed but the attribute "packed" is needed + * to remove extra padding at the end. */ struct ftrace_graph_ret { unsigned long func; /* Current function */ - unsigned long long calltime; - unsigned long long rettime; /* Number of functions that overran the depth limit for current task */ unsigned long overrun; + unsigned long long calltime; + unsigned long long rettime; int depth; -}; +} __packed; /* Type of the callback handlers for tracing function graph*/ typedef void (*trace_func_graph_ret_t)(struct ftrace_graph_ret *); /* return */ diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h index c1de3f493cd3..f783df416726 100644 --- a/kernel/trace/trace.h +++ b/kernel/trace/trace.h @@ -80,6 +80,12 @@ enum trace_type { FTRACE_ENTRY(name, struct_name, id, PARAMS(tstruct), PARAMS(print), \ filter) +#undef FTRACE_ENTRY_PACKED +#define FTRACE_ENTRY_PACKED(name, struct_name, id, tstruct, print, \ + filter) \ + FTRACE_ENTRY(name, struct_name, id, PARAMS(tstruct), PARAMS(print), \ +filter) __packed + #include "trace_entries.h" /* @@ -1625,6 +1631,11 @@ int set_tracer_flag(struct trace_array *tr, unsigned int mask, int enabled); #define FTRACE_ENTRY_DUP(call, struct_name, id, tstruct, print, filter) \ FTRACE_ENTRY(call, struct_name, id, PARAMS(tstruct), PARAMS(print), \ filter) +#undef FTRACE_ENTRY_PACKED +#define FTRACE_ENTRY_PACKED(call, struct_name, id, tstruct, print, filter) \ + FTRACE_ENTRY(call, struct_name, id, PARAMS(tstruct), PARAMS(print), \ +filter) + #include "trace_entries.h" #if defined(CONFIG_PERF_EVENTS) && defined(CONFIG_FUNCTION_TRACER) diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h index ee7b94a4810a..5c30efcda5e6 100644 --- a/kernel/trace/trace_entries.h +++ b/kernel/trace/trace_entries.h @@ -72,7 +72,7 @@ FTRACE_ENTRY_REG(function, ftrace_entry, ); /* Function call entry */ -FTRACE_ENTRY(funcgraph_entry, ftrace_graph_ent_entry, +FTRACE_ENTRY_PACKED(funcgraph_entry, ftrace_graph_ent_entry, TRACE_GRAPH_ENT, @@ -88,7 +88,7 @@ FTRACE_ENTRY(funcgraph_entry, ftrace_graph_ent_entry, ); /* Function return entry */ -FTRACE_ENTRY(funcgraph_exit, ftrace_graph_ret_entry, +FTRACE_ENTRY_PACKED(funcgraph_exit, ftrace_graph_ret_entry, TRACE_GRAPH_RET, -- 2.8.1
[PATCH] drm/hdlcd: Delete an unnecessary check before drm_fbdev_cma_hotplug_event()
From: Markus Elfring Date: Sat, 16 Jul 2016 09:10:40 +0200 The drm_fbdev_cma_hotplug_event() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/gpu/drm/arm/hdlcd_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c index 28b8cd1..9b1aefe 100644 --- a/drivers/gpu/drm/arm/hdlcd_drv.c +++ b/drivers/gpu/drm/arm/hdlcd_drv.c @@ -102,8 +102,7 @@ static void hdlcd_fb_output_poll_changed(struct drm_device *drm) { struct hdlcd_drm_private *hdlcd = drm->dev_private; - if (hdlcd->fbdev) - drm_fbdev_cma_hotplug_event(hdlcd->fbdev); + drm_fbdev_cma_hotplug_event(hdlcd->fbdev); } static const struct drm_mode_config_funcs hdlcd_mode_config_funcs = { -- 2.9.1
Re: [PATCH 2/2] nbd: Disallow ioctls on disconnected block device
Hi, On Fri, Jun 24, 2016 at 2:59 PM, Markus Pargmann wrote: > After NBD_DO_IT exited the block device may still be used. Make sure > that we handle intended disconnects differently and do not allow any > changed of the nbd device. > > This patch should avoid that the nbd-client connects to a different server > and the users of the block device are suddenly reading/writing from a > different backend device. > > For timeouts it is still possible to setup a new socket so that the > connection may be refreshed without creating problems for all users. But Shouldn't time out be checked for last end point? For example, consider the following steps 1) Timeout occurs but server[nbd-s1] comes up again albeit with a different network address. 2) The previous network address of server [nbd-s1] has now been assigned to another new nbd server [nbd-s2] 3) A new nbd-client tries to setup the socket again, Negotiation would be done again [correct?]. If correct then wouldn't we be sending data to wrong device this time? So instead can't we put a mechanism in place for network address + mac to be same for allowing clients to reconnect? Do let me know if this is not of concern. 4) If 3) doesn't apply then let's disallow all ioctls until nbd device is reset. > > Signed-off-by: Markus Pargmann > --- > drivers/block/nbd.c | 30 -- > 1 file changed, 24 insertions(+), 6 deletions(-) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 620660f3ff0f..39358efac73e 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -708,6 +708,18 @@ static void nbd_dev_dbg_close(struct nbd_device *nbd); > static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, >unsigned int cmd, unsigned long arg) > { > + /* > +* After a disconnect was instructed, do not allow any further actions > +* on the block device that would lead to a new connected endpoint. > +* This condition stays until nbd_reset was called either because all > +* users closed the device or because of CLEAR_SOCK. > +*/ > + if (nbd->disconnect && > + cmd != NBD_CLEAR_SOCK && cmd != NBD_PRINT_DEBUG) { > + dev_info(disk_to_dev(nbd->disk), "Device is still busy after > instructing a disconnect\n"); > + return -EBUSY; > + } > + > switch (cmd) { > case NBD_DISCONNECT: { > struct request sreq; > @@ -733,11 +745,15 @@ static int __nbd_ioctl(struct block_device *bdev, > struct nbd_device *nbd, > } > > case NBD_CLEAR_SOCK: > - sock_shutdown(nbd); > - nbd_clear_que(nbd); > - BUG_ON(!list_empty(&nbd->queue_head)); > - BUG_ON(!list_empty(&nbd->waiting_queue)); > - kill_bdev(bdev); > + if (nbd->disconnect) { > + nbd_reset(nbd); > + } else { > + sock_shutdown(nbd); > + nbd_clear_que(nbd); > + BUG_ON(!list_empty(&nbd->queue_head)); > + BUG_ON(!list_empty(&nbd->waiting_queue)); > + kill_bdev(bdev); > + } > return 0; > > case NBD_SET_SOCK: { > @@ -812,8 +828,10 @@ static int __nbd_ioctl(struct block_device *bdev, struct > nbd_device *nbd, > mutex_lock(&nbd->tx_lock); > nbd->task_recv = NULL; > > - if (nbd->disconnect) /* user requested, ignore socket errors > */ > + if (nbd->disconnect) { /* user requested, ignore socket > errors */ > + sock_shutdown(nbd); > error = 0; > + } > if (nbd->timedout) > error = -ETIMEDOUT; > > -- > 2.1.4 > -- ---P.K.S
[PATCH] net/mlx5_core/health: Remove deprecated create_singlethread_workqueue
The workqueue health->wq was used as per device private health thread. This was done so that system error handling could be processed concurrently. The workqueue has a single workitem(&health->work) and hence doesn't require ordering. It is involved in handling the health of the deviceand is not being used on a memory reclaim path. Hence, the singlethreaded workqueue has been replaced with the use of system_wq. System workqueues have been able to handle high level of concurrency for a long time now and hence it's not required to have a singlethreaded workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue created with create_singlethread_workqueue(), system_wq allows multiple work items to overlap executions even on the same CPU; however, a per-cpu workqueue doesn't have any CPU locality or global ordering guarantee unless the target CPU is explicitly specified and thus the increase of local concurrency shouldn't make any difference. Work item has been flushed in mlx5_health_cleanup() to ensure that there are no pending tasks while disconnecting the driver. Signed-off-by: Bhaktipriya Shridhar --- drivers/net/ethernet/mellanox/mlx5/core/health.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c index 42d16b9..9acbccf 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/health.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c @@ -267,7 +267,7 @@ static void poll_health(unsigned long data) if (in_fatal(dev) && !health->sick) { health->sick = true; print_health_info(dev); - queue_work(health->wq, &health->work); + schedule_work(&health->work); } } @@ -296,7 +296,7 @@ void mlx5_health_cleanup(struct mlx5_core_dev *dev) { struct mlx5_core_health *health = &dev->priv.health; - destroy_workqueue(health->wq); + flush_work(&health->work); } int mlx5_health_init(struct mlx5_core_dev *dev) @@ -311,10 +311,7 @@ int mlx5_health_init(struct mlx5_core_dev *dev) strcpy(name, "mlx5_health"); strcat(name, dev_name(&dev->pdev->dev)); - health->wq = create_singlethread_workqueue(name); kfree(name); - if (!health->wq) - return -ENOMEM; INIT_WORK(&health->work, health_care); -- 2.1.4
[PATCH] spi: spi-sh: Remove deprecated create_singlethread_workqueue
The workqueue has a single workitem(&ss->ws) and hence doesn't require ordering. Also, it is not being used on a memory reclaim path. Hence, the singlethreaded workqueue has been replaced with the use of system_wq. System workqueues have been able to handle high level of concurrency for a long time now and hence it's not required to have a singlethreaded workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue created with create_singlethread_workqueue(), system_wq allows multiple work items to overlap executions even on the same CPU; however, a per-cpu workqueue doesn't have any CPU locality or global ordering guarantee unless the target CPU is explicitly specified and thus the increase of local concurrency shouldn't make any difference. Work item has been flushed in spi_sh_remove() to ensure that there are no pending tasks while disconnecting the driver. Signed-off-by: Bhaktipriya Shridhar --- drivers/spi/spi-sh.c | 16 +++- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/drivers/spi/spi-sh.c b/drivers/spi/spi-sh.c index 5025011..2bf53f0 100644 --- a/drivers/spi/spi-sh.c +++ b/drivers/spi/spi-sh.c @@ -82,7 +82,6 @@ struct spi_sh_data { int irq; struct spi_master *master; struct list_head queue; - struct workqueue_struct *workqueue; struct work_struct ws; unsigned long cr1; wait_queue_head_t wait; @@ -380,7 +379,7 @@ static int spi_sh_transfer(struct spi_device *spi, struct spi_message *mesg) spi_sh_clear_bit(ss, SPI_SH_SSA, SPI_SH_CR1); list_add_tail(&mesg->queue, &ss->queue); - queue_work(ss->workqueue, &ss->ws); + schedule_work(&ss->ws); spin_unlock_irqrestore(&ss->lock, flags); @@ -425,7 +424,7 @@ static int spi_sh_remove(struct platform_device *pdev) struct spi_sh_data *ss = platform_get_drvdata(pdev); spi_unregister_master(ss->master); - destroy_workqueue(ss->workqueue); + flush_work(&ss->ws); free_irq(ss->irq, ss); return 0; @@ -484,18 +483,11 @@ static int spi_sh_probe(struct platform_device *pdev) spin_lock_init(&ss->lock); INIT_WORK(&ss->ws, spi_sh_work); init_waitqueue_head(&ss->wait); - ss->workqueue = create_singlethread_workqueue( - dev_name(master->dev.parent)); - if (ss->workqueue == NULL) { - dev_err(&pdev->dev, "create workqueue error\n"); - ret = -EBUSY; - goto error1; - } ret = request_irq(irq, spi_sh_irq, 0, "spi_sh", ss); if (ret < 0) { dev_err(&pdev->dev, "request_irq error\n"); - goto error2; + goto error1; } master->num_chipselect = 2; @@ -514,8 +506,6 @@ static int spi_sh_probe(struct platform_device *pdev) error3: free_irq(irq, ss); - error2: - destroy_workqueue(ss->workqueue); error1: spi_master_put(master); -- 2.1.4
[PATCH] drm/ast: Delete an unnecessary check before drm_gem_object_unreference_unlocked()
From: Markus Elfring Date: Sat, 16 Jul 2016 09:54:22 +0200 The drm_gem_object_unreference_unlocked() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/gpu/drm/ast/ast_main.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 7bc3aa6..904beaa 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -295,9 +295,8 @@ static int ast_get_dram_info(struct drm_device *dev) static void ast_user_framebuffer_destroy(struct drm_framebuffer *fb) { struct ast_framebuffer *ast_fb = to_ast_framebuffer(fb); - if (ast_fb->obj) - drm_gem_object_unreference_unlocked(ast_fb->obj); + drm_gem_object_unreference_unlocked(ast_fb->obj); drm_framebuffer_cleanup(fb); kfree(fb); } -- 2.9.1
Re: [PATCH] spi: spi-sh: Remove deprecated create_singlethread_workqueue
Please ignore this mail. I have already sent a patch for this driver Thanks, Bhaktipriya On Sat, Jul 16, 2016 at 1:35 PM, Bhaktipriya Shridhar wrote: > The workqueue has a single workitem(&ss->ws) and hence doesn't require > ordering. Also, it is not being used on a memory reclaim path. Hence, the > singlethreaded workqueue has been replaced with the use of system_wq. > > System workqueues have been able to handle high level of concurrency > for a long time now and hence it's not required to have a singlethreaded > workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue > created with create_singlethread_workqueue(), system_wq allows multiple > work items to overlap executions even on the same CPU; however, a > per-cpu workqueue doesn't have any CPU locality or global ordering > guarantee unless the target CPU is explicitly specified and thus the > increase of local concurrency shouldn't make any difference. > > Work item has been flushed in spi_sh_remove() to ensure that > there are no pending tasks while disconnecting the driver. > > Signed-off-by: Bhaktipriya Shridhar > --- > drivers/spi/spi-sh.c | 16 +++- > 1 file changed, 3 insertions(+), 13 deletions(-) > > diff --git a/drivers/spi/spi-sh.c b/drivers/spi/spi-sh.c > index 5025011..2bf53f0 100644 > --- a/drivers/spi/spi-sh.c > +++ b/drivers/spi/spi-sh.c > @@ -82,7 +82,6 @@ struct spi_sh_data { > int irq; > struct spi_master *master; > struct list_head queue; > - struct workqueue_struct *workqueue; > struct work_struct ws; > unsigned long cr1; > wait_queue_head_t wait; > @@ -380,7 +379,7 @@ static int spi_sh_transfer(struct spi_device *spi, struct > spi_message *mesg) > spi_sh_clear_bit(ss, SPI_SH_SSA, SPI_SH_CR1); > > list_add_tail(&mesg->queue, &ss->queue); > - queue_work(ss->workqueue, &ss->ws); > + schedule_work(&ss->ws); > > spin_unlock_irqrestore(&ss->lock, flags); > > @@ -425,7 +424,7 @@ static int spi_sh_remove(struct platform_device *pdev) > struct spi_sh_data *ss = platform_get_drvdata(pdev); > > spi_unregister_master(ss->master); > - destroy_workqueue(ss->workqueue); > + flush_work(&ss->ws); > free_irq(ss->irq, ss); > > return 0; > @@ -484,18 +483,11 @@ static int spi_sh_probe(struct platform_device *pdev) > spin_lock_init(&ss->lock); > INIT_WORK(&ss->ws, spi_sh_work); > init_waitqueue_head(&ss->wait); > - ss->workqueue = create_singlethread_workqueue( > - dev_name(master->dev.parent)); > - if (ss->workqueue == NULL) { > - dev_err(&pdev->dev, "create workqueue error\n"); > - ret = -EBUSY; > - goto error1; > - } > > ret = request_irq(irq, spi_sh_irq, 0, "spi_sh", ss); > if (ret < 0) { > dev_err(&pdev->dev, "request_irq error\n"); > - goto error2; > + goto error1; > } > > master->num_chipselect = 2; > @@ -514,8 +506,6 @@ static int spi_sh_probe(struct platform_device *pdev) > > error3: > free_irq(irq, ss); > - error2: > - destroy_workqueue(ss->workqueue); > error1: > spi_master_put(master); > > -- > 2.1.4 >
Re: [PATCH v2 2/6] [media] Documentation: Add HSV format
On 07/15/2016 08:11 PM, Laurent Pinchart wrote: > Hi Ricardo, > > Thank you for the patch. > > On Friday 15 Jul 2016 18:13:15 Ricardo Ribalda Delgado wrote: >> Describe the HSV formats >> >> Signed-off-by: Ricardo Ribalda Delgado >> --- >> Documentation/media/uapi/v4l/hsv-formats.rst | 19 ++ >> Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst | 253 ++ >> Documentation/media/uapi/v4l/pixfmt.rst| 1 + >> Documentation/media/uapi/v4l/v4l2.rst | 5 + >> 4 files changed, 278 insertions(+) >> create mode 100644 Documentation/media/uapi/v4l/hsv-formats.rst >> create mode 100644 Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst >> >> diff --git a/Documentation/media/uapi/v4l/hsv-formats.rst >> b/Documentation/media/uapi/v4l/hsv-formats.rst new file mode 100644 >> index ..f0f2615eaa95 >> --- /dev/null >> +++ b/Documentation/media/uapi/v4l/hsv-formats.rst >> @@ -0,0 +1,19 @@ >> +.. -*- coding: utf-8; mode: rst -*- >> + >> +.. _hsv-formats: >> + >> +*** >> +HSV Formats >> +*** >> + >> +These formats store the color information of the image >> +in a geometrical representation. The colors are mapped into a >> +cylinder, where the angle is the HUE, the height is the VALUE >> +and the distance to the center is the SATURATION. This is a very >> +useful format for image segmentation algorithms. >> + >> + >> +.. toctree:: >> +:maxdepth: 1 >> + >> +pixfmt-packed-hsv >> diff --git a/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst >> b/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst new file mode 100644 >> index ..b297aa4f7ba6 >> --- /dev/null >> +++ b/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst >> @@ -0,0 +1,253 @@ >> +.. -*- coding: utf-8; mode: rst -*- >> + >> +.. _packed-hsv: >> + >> +** >> +Packed HSV formats >> +** >> + >> +*man Packed HSV formats(2)* >> + >> +Packed HSV formats >> + >> + >> +Description >> +=== >> + >> +The HUE (h) is meassured in degrees, one LSB represents two degrees. > > Is this common ? I have a device that can handle HSV data, I need to check > how > it maps the hue values to binary, but I'm pretty sure they cover the full > 0-255 range. We would then have to support the two formats. Separate 4CCs are > an option, but reporting the range separately (possibly through the > colorspace > API) might be better. Any thought on that ? It's either a separate 4cc or we do something with the ycbcr_enc field (reinterpreted as hsv_enc). I'm not sure, I would have to think some more about that. Regards, Hans
[PATCH] drm/ttm: Remove create_singlethread_workqueue
swap_queue was created to handle shrinking in low memory situations. A separate workqueue was used in order to avoid other workqueue tasks from being blocked since work items on swap_queue spend a lot of time waiting for the GPU. Since these long-running work items aren't involved in memory reclaim in any way, system_long_wq has been used. Work item has been flushed in ttm_mem_global_release() to ensure that nothing is pending when the driver is disconnected. Signed-off-by: Bhaktipriya Shridhar --- drivers/gpu/drm/ttm/ttm_memory.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c index a1803fb..b985482 100644 --- a/drivers/gpu/drm/ttm/ttm_memory.c +++ b/drivers/gpu/drm/ttm/ttm_memory.c @@ -366,7 +366,6 @@ int ttm_mem_global_init(struct ttm_mem_global *glob) struct ttm_mem_zone *zone; spin_lock_init(&glob->lock); - glob->swap_queue = create_singlethread_workqueue("ttm_swap"); INIT_WORK(&glob->work, ttm_shrink_work); ret = kobject_init_and_add( &glob->kobj, &ttm_mem_glob_kobj_type, ttm_get_kobj(), "memory_accounting"); @@ -412,9 +411,7 @@ void ttm_mem_global_release(struct ttm_mem_global *glob) ttm_page_alloc_fini(); ttm_dma_page_alloc_fini(); - flush_workqueue(glob->swap_queue); - destroy_workqueue(glob->swap_queue); - glob->swap_queue = NULL; + flush_work(&glob->work); for (i = 0; i < glob->num_zones; ++i) { zone = glob->zones[i]; kobject_del(&zone->kobj); @@ -443,7 +440,7 @@ static void ttm_check_swapping(struct ttm_mem_global *glob) spin_unlock(&glob->lock); if (unlikely(needs_swapping)) - (void)queue_work(glob->swap_queue, &glob->work); + (void)queue_work(system_long_wq, &glob->work); } -- 2.1.4
Re: [PATCH 1/1] irqdomain: Export __irq_domain_alloc_irqs() and irq_domain_free_irqs()
On Sat, 16 Jul 2016 04:33:59 +0300 Alexander Popov wrote: > On 08.07.2016 11:34, Alexander Popov wrote: > > On 06.07.2016 14:17, Thomas Gleixner wrote: > >> On Fri, 1 Jul 2016, Alexander Popov wrote: > >> > >>> Export __irq_domain_alloc_irqs() and irq_domain_free_irqs() for being > >>> able to work with irq_domain hierarchy in modules. > >> > >> We usually export only when we have a proper use case which is supposed to > >> go > >> into the kernel tree proper. What's yours? > > > > Hello, Thomas, > > > > I work at Positive Technologies ( https://www.ptsecurity.com/ ). We develop > > a bare-metal hypervisor, which targets x86_64 and supports Linux as a guest > > OS. > > > > Intel VT-x allows hypervisor to inject interrupts into virtual machines. > > We want to handle these interrupts in guest Linux. > > > > So I wrote a simple kernel module creating an irq_domain, which has > > x86_vector_domain as a parent in the hierarchy. In this module I just call: > > - irq_domain_alloc_irqs() to allocate irqs and allow calling request_irq() > >for them; > > - irqd_cfg(irq_get_irq_data()) to get the APIC vectors of the allocated > > irqs; > > - irq_domain_free_irqs() to free the resources at the end. > > > > It allows to handle interrupts injected by the hypervisor in guest Linux > > easily, > > without emulating MSI-capable PCI device at the hypervisor side. > > > > Everything works fine if __irq_domain_alloc_irqs() and > > irq_domain_free_irqs() > > are exported. Is it a proper use-case? > > Hello again, Thomas, > > Did I properly answer your question? Will you accept my patch exporting these > two functions? > > > Do you think my module could be useful for the mainline in some form? > > It took me some time to understand irq_domain hierarchy design, so I can > > prepare some patch or share my code to help others. > > Do you think my paravirtualization code registering a child irq_domain > for x86_vector_domain could bring any profit to the mainline? > I would be glad to put effort and do it. I think that without any in-tree modular users of these symbols, the incentive for exporting those is pretty low. I can't really say anything about your particular use-case, but I'd really to see some code before taking that kind of patch. Thanks, M. -- Jazz is not dead. It just smells funny.
Re: [PATCH v2 2/6] [media] Documentation: Add HSV format
Hi Laurent It is actually a very good comment. :) In our case we have implemented the format ourselves in the FPGA and we support both 0-255 and 0-179 Hue ranges. After some weeks of use, only the 0-179 range is used in userpace. The reasons for this is mainly that it is the format used by OpenCV http://docs.opencv.org/3.1.0/de/d25/imgproc_color_conversions.html#color_convert_rgb_hsv&gsc.tab=0 , but also because it is very efficient to convert from 0-360 to 0-180 and the lose of color resolution (256/180) does not lead to (human) perceptible differences. All that said, I would not mind to implement also the 0-255 range, but I do not know which API should be the best way to do it. quantization? it looks nice, but it is not really a quantization... a control? a bit messy fourcc? seems good... I am open to anything :), but I am not the right guy for making the decision. Hans, could you help me? Thanks! On Fri, Jul 15, 2016 at 8:11 PM, Laurent Pinchart wrote: > Hi Ricardo, > > Thank you for the patch. > > On Friday 15 Jul 2016 18:13:15 Ricardo Ribalda Delgado wrote: >> Describe the HSV formats >> >> Signed-off-by: Ricardo Ribalda Delgado >> --- >> Documentation/media/uapi/v4l/hsv-formats.rst | 19 ++ >> Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst | 253 ++ >> Documentation/media/uapi/v4l/pixfmt.rst| 1 + >> Documentation/media/uapi/v4l/v4l2.rst | 5 + >> 4 files changed, 278 insertions(+) >> create mode 100644 Documentation/media/uapi/v4l/hsv-formats.rst >> create mode 100644 Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst >> >> diff --git a/Documentation/media/uapi/v4l/hsv-formats.rst >> b/Documentation/media/uapi/v4l/hsv-formats.rst new file mode 100644 >> index ..f0f2615eaa95 >> --- /dev/null >> +++ b/Documentation/media/uapi/v4l/hsv-formats.rst >> @@ -0,0 +1,19 @@ >> +.. -*- coding: utf-8; mode: rst -*- >> + >> +.. _hsv-formats: >> + >> +*** >> +HSV Formats >> +*** >> + >> +These formats store the color information of the image >> +in a geometrical representation. The colors are mapped into a >> +cylinder, where the angle is the HUE, the height is the VALUE >> +and the distance to the center is the SATURATION. This is a very >> +useful format for image segmentation algorithms. >> + >> + >> +.. toctree:: >> +:maxdepth: 1 >> + >> +pixfmt-packed-hsv >> diff --git a/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst >> b/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst new file mode 100644 >> index ..b297aa4f7ba6 >> --- /dev/null >> +++ b/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst >> @@ -0,0 +1,253 @@ >> +.. -*- coding: utf-8; mode: rst -*- >> + >> +.. _packed-hsv: >> + >> +** >> +Packed HSV formats >> +** >> + >> +*man Packed HSV formats(2)* >> + >> +Packed HSV formats >> + >> + >> +Description >> +=== >> + >> +The HUE (h) is meassured in degrees, one LSB represents two degrees. > > Is this common ? I have a device that can handle HSV data, I need to check how > it maps the hue values to binary, but I'm pretty sure they cover the full > 0-255 range. We would then have to support the two formats. Separate 4CCs are > an option, but reporting the range separately (possibly through the colorspace > API) might be better. Any thought on that ? > >> +The SATURATION (s) and the VALUE (v) are measured in percentage of the >> +cylinder: 0 being the smallest value and 255 the maximum. >> + >> + >> +The values are packed in 24 or 32 bit formats. >> + >> + >> +.. flat-table:: Packed HSV Image Formats >> +:header-rows: 2 >> +:stub-columns: 0 >> + >> + >> +- .. row 1 >> + >> + - Identifier >> + >> + - Code >> + >> + - >> + - :cspan:`7` Byte 0 in memory >> + > > Do we really need all those blank lines ? > > [snip] > > -- > Regards, > > Laurent Pinchart > -- Ricardo Ribalda
[PATCH] dwc_eth_qos: Remove deprecated create_singlethread_workqueue
alloc_workqueue replaces deprecated create_singlethread_workqueue(). A dedicated workqueue has been used since the workitem viz lp->txtimeout_reinit is involved in reinitialization if a TX timeout occurs, which is necessary to guarantee forward progress in packet processing. As a network device can be used during memory reclaim, the workqueue needs forward progress guarantee under memory pressure. WQ_MEM_RECLAIM has been set to ensure this. Since there is only a single work item, explicit concurrency limit is unnecessary here. Signed-off-by: Bhaktipriya Shridhar --- drivers/net/ethernet/synopsys/dwc_eth_qos.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/synopsys/dwc_eth_qos.c b/drivers/net/ethernet/synopsys/dwc_eth_qos.c index 158213c..44c4cd6 100644 --- a/drivers/net/ethernet/synopsys/dwc_eth_qos.c +++ b/drivers/net/ethernet/synopsys/dwc_eth_qos.c @@ -2934,7 +2934,8 @@ static int dwceqos_probe(struct platform_device *pdev) (unsigned long)ndev); tasklet_disable(&lp->tx_bdreclaim_tasklet); - lp->txtimeout_handler_wq = create_singlethread_workqueue(DRIVER_NAME); + lp->txtimeout_handler_wq = alloc_workqueue(DRIVER_NAME, + WQ_MEM_RECLAIM, 0); INIT_WORK(&lp->txtimeout_reinit, dwceqos_reinit_for_txtimeout); platform_set_drvdata(pdev, ndev); -- 2.1.4
Re: [PATCH 1/5] namespaces: move user_ns into ns_common
Hi, [auto build test WARNING on net/master] [also build test WARNING on v4.7-rc7] [cannot apply to next-20160715] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Andrey-Vagin/namespaces-move-user_ns-into-ns_common/20160716-093057 config: openrisc-allyesconfig (attached as .config) compiler: or32-linux-gcc (GCC) 4.5.1-or32-1.0rc1 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=openrisc All warnings (new ones prefixed by >>): kernel/user.c:53:2: error: unknown field 'ns' specified in initializer kernel/user.c:53:2: warning: missing braces around initializer kernel/user.c:53:2: warning: (near initialization for 'init_user_ns.') kernel/user.c:53:2: error: incompatible types when initializing type 'struct user_namespace *' using type 'enum ' kernel/user.c:55:2: error: unknown field 'ns' specified in initializer >> kernel/user.c:55:2: warning: initialization makes integer from pointer >> without a cast vim +55 kernel/user.c f76d207a Eric W. Biederman 2012-08-30 47 .count = 4294967295U, f76d207a Eric W. Biederman 2012-08-30 48 }, f76d207a Eric W. Biederman 2012-08-30 49 }, c61a2810 Eric W. Biederman 2012-12-28 50 .count = ATOMIC_INIT(3), 783291e6 Eric W. Biederman 2011-11-17 51 .owner = GLOBAL_ROOT_UID, 783291e6 Eric W. Biederman 2011-11-17 52 .group = GLOBAL_ROOT_GID, 435d5f4b Al Viro 2014-10-31 @53 .ns.inum = PROC_USER_INIT_INO, 33c42940 Al Viro 2014-11-01 54 #ifdef CONFIG_USER_NS 33c42940 Al Viro 2014-11-01 @55 .ns.ops = &userns_operations, 33c42940 Al Viro 2014-11-01 56 #endif 9cc46516 Eric W. Biederman 2014-12-02 57 .flags = USERNS_INIT_FLAGS, 6bd364d8 Xiao Guangrong2013-12-13 58 #ifdef CONFIG_PERSISTENT_KEYRINGS :: The code at line 55 was first introduced by commit :: 33c429405a2c8d9e42afb9fee88a63cfb2de1e98 copy address of proc_ns_ops into ns_common :: TO: Al Viro :: CC: Al Viro --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
[PATCH] ARM: pxa: fix GPIO double shifts
The commit 9bf448c66d4b ("ARM: pxa: use generic gpio operation instead of gpio register") from Oct 17, 2011, leads to the following static checker warning: arch/arm/mach-pxa/spitz_pm.c:172 spitz_charger_wakeup() warn: double left shift '!gpio_get_value(SPITZ_GPIO_KEY_INT) << (1 << ((SPITZ_GPIO_KEY_INT) & 31))' As Dan reported, the value is shifted three times : - once by gpio_get_value(), which returns either 0 or BIT(gpio) - once by the shift operation '<<' - a last time by GPIO_bit(gpio) which is BIT(gpio) Therefore the calculation lead to a chained or operator of : - (1 << gpio) << (1 << gpio) = (2^gpio)^gpio = 2 ^ (gpio * gpio) It is be sheer luck the former statement works, only because each gpio used is strictly smaller than 6, and therefore 2^(gpio^2) never overflows a 32 bits value, and because it is used as a boolean value to check a gpio activation. Fixes: 9bf448c66d4b ("ARM: pxa: use generic gpio operation instead of gpio register") Reported-by: Dan Carpenter Signed-off-by: Robert Jarzmik --- arch/arm/mach-pxa/corgi_pm.c | 8 +++- arch/arm/mach-pxa/spitz_pm.c | 5 ++--- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/arch/arm/mach-pxa/corgi_pm.c b/arch/arm/mach-pxa/corgi_pm.c index d9206811be9b..a7bc7e18158e 100644 --- a/arch/arm/mach-pxa/corgi_pm.c +++ b/arch/arm/mach-pxa/corgi_pm.c @@ -135,11 +135,9 @@ static unsigned long corgi_charger_wakeup(void) { unsigned long ret; - ret = (!gpio_get_value(CORGI_GPIO_AC_IN) << GPIO_bit(CORGI_GPIO_AC_IN)) - | (!gpio_get_value(CORGI_GPIO_KEY_INT) - << GPIO_bit(CORGI_GPIO_KEY_INT)) - | (!gpio_get_value(CORGI_GPIO_WAKEUP) - << GPIO_bit(CORGI_GPIO_WAKEUP)); + ret = !gpio_get_value(CORGI_GPIO_AC_IN) + | !gpio_get_value(CORGI_GPIO_KEY_INT) + | !gpio_get_value(CORGI_GPIO_WAKEUP); return ret; } diff --git a/arch/arm/mach-pxa/spitz_pm.c b/arch/arm/mach-pxa/spitz_pm.c index ea9f9034cb54..5a4c45b5a526 100644 --- a/arch/arm/mach-pxa/spitz_pm.c +++ b/arch/arm/mach-pxa/spitz_pm.c @@ -168,9 +168,8 @@ static int spitz_should_wakeup(unsigned int resume_on_alarm) static unsigned long spitz_charger_wakeup(void) { unsigned long ret; - ret = ((!gpio_get_value(SPITZ_GPIO_KEY_INT) - << GPIO_bit(SPITZ_GPIO_KEY_INT)) - | gpio_get_value(SPITZ_GPIO_SYNC)); + ret = !gpio_get_value(SPITZ_GPIO_KEY_INT) + | gpio_get_value(SPITZ_GPIO_SYNC); return ret; } -- 2.1.4
[PATCH] [media] s5p-mfc: Remove deprecated create_singlethread_workqueue
alloc_workqueue replaces deprecated create_singlethread_workqueue(). The MFC device driver is a v4l2 driver which can encode/decode video raw/elementary streams and has support for all popular video codecs. The driver's watchdog_workqueue has been replaced with system_wq since it queues a single work item, &dev->watchdog_work, which calls for no ordering requirement. The work item is involved in running the watchdog timer and is not being used on a memory reclaim path. Work item has been flushed in s5p_mfc_remove() to ensure that there are no pending tasks while disconnecting the driver. Signed-off-by: Bhaktipriya Shridhar --- drivers/media/platform/s5p-mfc/s5p_mfc.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index b16466f..1bc27ec 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -152,7 +152,7 @@ static void s5p_mfc_watchdog(unsigned long arg) * error. Now it is time to kill all instances and * reset the MFC. */ mfc_err("Time out during waiting for HW\n"); - queue_work(dev->watchdog_workqueue, &dev->watchdog_work); + schedule_work(&dev->watchdog_work); } dev->watchdog_timer.expires = jiffies + msecs_to_jiffies(MFC_WATCHDOG_INTERVAL); @@ -1238,7 +1238,6 @@ static int s5p_mfc_probe(struct platform_device *pdev) platform_set_drvdata(pdev, dev); dev->hw_lock = 0; - dev->watchdog_workqueue = create_singlethread_workqueue(S5P_MFC_NAME); INIT_WORK(&dev->watchdog_work, s5p_mfc_watchdog_worker); atomic_set(&dev->watchdog_cnt, 0); init_timer(&dev->watchdog_timer); @@ -1284,8 +1283,7 @@ static int s5p_mfc_remove(struct platform_device *pdev) v4l2_info(&dev->v4l2_dev, "Removing %s\n", pdev->name); del_timer_sync(&dev->watchdog_timer); - flush_workqueue(dev->watchdog_workqueue); - destroy_workqueue(dev->watchdog_workqueue); + flush_work(&dev->watchdog_work); video_unregister_device(dev->vfd_enc); video_unregister_device(dev->vfd_dec); -- 2.1.4
[PATCH] [media] pvrusb2: Remove deprecated create_singlethread_workqueue
The workqueue "workqueue" is involved in polling the pvrusb2 hardware (pvr2_hdw). It has a single work item(&hdw->workpoll) and hence doesn't require ordering. Also, it is not being used on a memory reclaim path. Hence, the singlethreaded workqueue has been replaced with the use of system_wq. System workqueues have been able to handle high level of concurrency for a long time now and hence it's not required to have a singlethreaded workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue created with create_singlethread_workqueue(), system_wq allows multiple work items to overlap executions even on the same CPU; however, a per-cpu workqueue doesn't have any CPU locality or global ordering guarantee unless the target CPU is explicitly specified and thus the increase of local concurrency shouldn't make any difference. Work item has been flushed in pvr2_hdw_destroy to ensure that there are no pending tasks while disconnecting the driver. Signed-off-by: Bhaktipriya Shridhar --- drivers/media/usb/pvrusb2/pvrusb2-hdw-internal.h | 1 - drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 23 +++ 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw-internal.h b/drivers/media/usb/pvrusb2/pvrusb2-hdw-internal.h index 60141b1..23473a2 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw-internal.h +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw-internal.h @@ -170,7 +170,6 @@ struct pvr2_hdw { const struct pvr2_device_desc *hdw_desc; /* Kernel worker thread handling */ - struct workqueue_struct *workqueue; struct work_struct workpoll; /* Update driver state */ /* Video spigot */ diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c index 83e9a3e..85e39ec 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c @@ -2624,7 +2624,6 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface *intf, if (cnt1 >= sizeof(hdw->name)) cnt1 = sizeof(hdw->name)-1; hdw->name[cnt1] = 0; - hdw->workqueue = create_singlethread_workqueue(hdw->name); INIT_WORK(&hdw->workpoll,pvr2_hdw_worker_poll); pvr2_trace(PVR2_TRACE_INIT,"Driver unit number is %d, name is %s", @@ -2651,11 +2650,7 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface *intf, del_timer_sync(&hdw->decoder_stabilization_timer); del_timer_sync(&hdw->encoder_run_timer); del_timer_sync(&hdw->encoder_wait_timer); - if (hdw->workqueue) { - flush_workqueue(hdw->workqueue); - destroy_workqueue(hdw->workqueue); - hdw->workqueue = NULL; - } + flush_work(&hdw->workpoll); usb_free_urb(hdw->ctl_read_urb); usb_free_urb(hdw->ctl_write_urb); kfree(hdw->ctl_read_buffer); @@ -2712,11 +2707,7 @@ void pvr2_hdw_destroy(struct pvr2_hdw *hdw) { if (!hdw) return; pvr2_trace(PVR2_TRACE_INIT,"pvr2_hdw_destroy: hdw=%p",hdw); - if (hdw->workqueue) { - flush_workqueue(hdw->workqueue); - destroy_workqueue(hdw->workqueue); - hdw->workqueue = NULL; - } + flush_work(&hdw->workpoll); del_timer_sync(&hdw->quiescent_timer); del_timer_sync(&hdw->decoder_stabilization_timer); del_timer_sync(&hdw->encoder_run_timer); @@ -4439,7 +4430,7 @@ static void pvr2_hdw_quiescent_timeout(unsigned long data) hdw->state_decoder_quiescent = !0; trace_stbit("state_decoder_quiescent",hdw->state_decoder_quiescent); hdw->state_stale = !0; - queue_work(hdw->workqueue,&hdw->workpoll); + schedule_work(&hdw->workpoll); } @@ -4450,7 +4441,7 @@ static void pvr2_hdw_decoder_stabilization_timeout(unsigned long data) hdw->state_decoder_ready = !0; trace_stbit("state_decoder_ready", hdw->state_decoder_ready); hdw->state_stale = !0; - queue_work(hdw->workqueue, &hdw->workpoll); + schedule_work(&hdw->workpoll); } @@ -4461,7 +4452,7 @@ static void pvr2_hdw_encoder_wait_timeout(unsigned long data) hdw->state_encoder_waitok = !0; trace_stbit("state_encoder_waitok",hdw->state_encoder_waitok); hdw->state_stale = !0; - queue_work(hdw->workqueue,&hdw->workpoll); + schedule_work(&hdw->workpoll); } @@ -4473,7 +4464,7 @@ static void pvr2_hdw_encoder_run_timeout(unsigned long data) hdw->state_encoder_runok = !0; trace_stbit("state_encoder_runok",hdw->state_encoder_runok); hdw->state_stale = !0; - queue_work(hdw->workqueue,&hdw->workpoll); + schedule_work(&hdw->workpoll); } } @@ -4987,7 +4978,7 @@ static void pvr2_hdw_state_sched(struct pvr2_hdw *hdw) if (hdw->state_stale) return; h
RE: [PATCH v3 5/8] thunderbolt: Communication with the ICM (firmware)
> -Original Message- > From: Levy, Amir (Jer) > Sent: Thursday, July 14, 2016 17:50 > To: Winkler, Tomas ; > andreas.noe...@gmail.com; gre...@linuxfoundation.org; > bhelg...@google.com > Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; > net...@vger.kernel.org; thunderbolt-linux ; > Westerberg, Mika > Subject: RE: [PATCH v3 5/8] thunderbolt: Communication with the ICM > (firmware) > > Hi Tomas, > Thanks for your comments. > > On Thu, Jul 14 2016, 03:44 PM, Winkler, Tomas wrote: > > > +/* NHI genetlink commands */ > > > +enum { > > > + NHI_CMD_UNSPEC, > > > + NHI_CMD_SUBSCRIBE, > > > + NHI_CMD_UNSUBSCRIBE, > > > + NHI_CMD_QUERY_INFORMATION, > > > + NHI_CMD_MSG_TO_ICM, > > > + NHI_CMD_MSG_FROM_ICM, > > > + NHI_CMD_MAILBOX, > > > + NHI_CMD_APPROVE_TBT_NETWORKING, > > > + NHI_CMD_ICM_IN_SAFE_MODE, > > > + __NHI_CMD_MAX, > > > +}; > > > +#define NHI_CMD_MAX (__NHI_CMD_MAX - 1) > > NHI_CMD_MAX = NHI_CMD_ICM_IN_SAFE_MODE ? > > > > This template is used a lot with (generic) netlink. > Few examples: > http://lxr.free-electrons.com/source/drivers/acpi/event.c#L62 > http://lxr.free-electrons.com/source/include/uapi/linux/irda.h#L224 > It is easier to maintain - adding entry in the list will automatically update > MAX. Fair enough. > [...] > > > > + u32 status; > > > + > > > + status = ioread32(nhi_ctxt->iobase + REG_FW_STS); > > > + > > > + if (status & REG_FW_STS_NVM_AUTH_DONE) > > > + break; > > > + msleep(30); > > > > 30 is big number, for polling, what is behind this? > > > > The NVM authentication can take time for ICM. > This number comes from experiments. This deserve some comment, this look very random. > [...] > > > > +static struct tbt_nhi_ctxt *nhi_search_ctxt(u32 id) { > > > + struct tbt_nhi_ctxt *nhi_ctxt; > > > + > > > + list_for_each_entry(nhi_ctxt, &controllers_list, node) > > > + if (nhi_ctxt->id == id) > > > + return nhi_ctxt; > > > > Don't you need to lock this list with the controllers_list_rwsem ? > > > > This is a helper function for searching the list. > The callers take and release the lock. Since this doesn't fit the patterns in your code, the function deserves a comment, that it should be used under lock. > > [...] > > > > + bool nvm_auth_on_boot : 1; > > Don't use bool with bit fields use u32 > > What is the concern here? > If it is size of bool, it doesn't matter for this struct. > It is more readable that the expected value is bool, and it is used a lot in > kernel, for example: > http://lxr.free-electrons.com/source/include/linux/pm.h#L558 Hmm, actually a nice feature I wasn't aware of, save some space and got bool type checking. Thanks
[PATCH] s390/cio/chp : Remove deprecated create_singlethread_workqueue
The workqueue "chp_wq" is involved in performing pending configure tasks for channel paths. It has a single work item(&cfg_work) and hence doesn't require ordering. Also, it is not being used on a memory reclaim path. Hence, the singlethreaded workqueue has been replaced with the use of system_wq. System workqueues have been able to handle high level of concurrency for a long time now and hence it's not required to have a singlethreaded workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue created with create_singlethread_workqueue(), system_wq allows multiple work items to overlap executions even on the same CPU; however, a per-cpu workqueue doesn't have any CPU locality or global ordering guarantee unless the target CPU is explicitly specified and thus the increase of local concurrency shouldn't make any difference. Signed-off-by: Bhaktipriya Shridhar --- drivers/s390/cio/chp.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/s390/cio/chp.c b/drivers/s390/cio/chp.c index 50597f9..92413c4 100644 --- a/drivers/s390/cio/chp.c +++ b/drivers/s390/cio/chp.c @@ -47,8 +47,6 @@ static DEFINE_MUTEX(info_lock); /* Time after which channel-path status may be outdated. */ static unsigned long chp_info_expires; -/* Workqueue to perform pending configure tasks. */ -static struct workqueue_struct *chp_wq; static struct work_struct cfg_work; /* Wait queue for configure completion events. */ @@ -714,7 +712,7 @@ static void cfg_func(struct work_struct *work) wake_up_interruptible(&cfg_wait_queue); return; } - queue_work(chp_wq, &cfg_work); + schedule_work(&cfg_work); } /** @@ -732,7 +730,7 @@ void chp_cfg_schedule(struct chp_id chpid, int configure) cfg_set_task(chpid, configure ? cfg_configure : cfg_deconfigure); cfg_busy = 1; mutex_unlock(&cfg_lock); - queue_work(chp_wq, &cfg_work); + schedule_work(&cfg_work); } /** @@ -766,11 +764,6 @@ static int __init chp_init(void) ret = crw_register_handler(CRW_RSC_CPATH, chp_process_crw); if (ret) return ret; - chp_wq = create_singlethread_workqueue("cio_chp"); - if (!chp_wq) { - crw_unregister_handler(CRW_RSC_CPATH); - return -ENOMEM; - } INIT_WORK(&cfg_work, cfg_func); init_waitqueue_head(&cfg_wait_queue); if (info_update()) -- 2.1.4
[PATCH] [media] gspca: sonixj: Remove deprecated create_singlethread_workqueue
The workqueue "work_thread" is involved in updating the JPEG quality of the gspca_dev. It has a single work item(&sd->work) and hence doesn't require ordering. Also, it is not being used on a memory reclaim path. Hence, the singlethreaded workqueue has been replaced with the use of system_wq. System workqueues have been able to handle high level of concurrency for a long time now and hence it's not required to have a singlethreaded workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue created with create_singlethread_workqueue(), system_wq allows multiple work items to overlap executions even on the same CPU; however, a per-cpu workqueue doesn't have any CPU locality or global ordering guarantee unless the target CPU is explicitly specified and thus the increase of local concurrency shouldn't make any difference. Work item has been flushed in sd_stop0() to ensure that there are no pending tasks while disconnecting the driver. Signed-off-by: Bhaktipriya Shridhar --- drivers/media/usb/gspca/sonixj.c | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/media/usb/gspca/sonixj.c b/drivers/media/usb/gspca/sonixj.c index fd1c870..d49d76e 100644 --- a/drivers/media/usb/gspca/sonixj.c +++ b/drivers/media/usb/gspca/sonixj.c @@ -54,7 +54,6 @@ struct sd { u32 exposure; struct work_struct work; - struct workqueue_struct *work_thread; u32 pktsz; /* (used by pkt_scan) */ u16 npkt; @@ -2485,7 +2484,6 @@ static int sd_start(struct gspca_dev *gspca_dev) sd->pktsz = sd->npkt = 0; sd->nchg = sd->short_mark = 0; - sd->work_thread = create_singlethread_workqueue(MODULE_NAME); return gspca_dev->usb_err; } @@ -2569,12 +2567,9 @@ static void sd_stop0(struct gspca_dev *gspca_dev) { struct sd *sd = (struct sd *) gspca_dev; - if (sd->work_thread != NULL) { - mutex_unlock(&gspca_dev->usb_lock); - destroy_workqueue(sd->work_thread); - mutex_lock(&gspca_dev->usb_lock); - sd->work_thread = NULL; - } + mutex_unlock(&gspca_dev->usb_lock); + flush_work(&sd->work); + mutex_lock(&gspca_dev->usb_lock); } static void do_autogain(struct gspca_dev *gspca_dev) @@ -2785,7 +2780,7 @@ marker_found: new_qual = QUALITY_MAX; if (new_qual != sd->quality) { sd->quality = new_qual; - queue_work(sd->work_thread, &sd->work); + schedule_work(&sd->work); } } } else { -- 2.1.4
[PATCH] [media] gspca: vicam: Remove deprecated create_singlethread_workqueue
The workqueue "work_thread" is involved in streaming the camera data. It has a single work item(&sd->work_struct) and hence doesn't require ordering. Also, it is not being used on a memory reclaim path. Hence, the singlethreaded workqueue has been replaced with the use of system_wq. System workqueues have been able to handle high level of concurrency for a long time now and hence it's not required to have a singlethreaded workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue created with create_singlethread_workqueue(), system_wq allows multiple work items to overlap executions even on the same CPU; however, a per-cpu workqueue doesn't have any CPU locality or global ordering guarantee unless the target CPU is explicitly specified and thus the increase of local concurrency shouldn't make any difference. Work item has been flushed in sd_stop0() to ensure that there are no pending tasks while disconnecting the driver. Signed-off-by: Bhaktipriya Shridhar --- drivers/media/usb/gspca/vicam.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/media/usb/gspca/vicam.c b/drivers/media/usb/gspca/vicam.c index 103f6c4..8860510 100644 --- a/drivers/media/usb/gspca/vicam.c +++ b/drivers/media/usb/gspca/vicam.c @@ -47,7 +47,6 @@ MODULE_FIRMWARE(VICAM_FIRMWARE); struct sd { struct gspca_dev gspca_dev; /* !! must be the first item */ struct work_struct work_struct; - struct workqueue_struct *work_thread; }; /* The vicam sensor has a resolution of 512 x 244, with I believe square @@ -278,9 +277,7 @@ static int sd_start(struct gspca_dev *gspca_dev) if (ret < 0) return ret; - /* Start the workqueue function to do the streaming */ - sd->work_thread = create_singlethread_workqueue(MODULE_NAME); - queue_work(sd->work_thread, &sd->work_struct); + schedule_work(&sd->work_struct); return 0; } @@ -294,8 +291,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev) /* wait for the work queue to terminate */ mutex_unlock(&gspca_dev->usb_lock); /* This waits for vicam_dostream to finish */ - destroy_workqueue(dev->work_thread); - dev->work_thread = NULL; + flush_work(&dev->work_struct); mutex_lock(&gspca_dev->usb_lock); if (gspca_dev->present) -- 2.1.4
[PATCH] [media] gspca: jl2005bcd: Remove deprecated create_singlethread_workqueue
The workqueue "work_thread" is involved in streaming the camera data. It has a single work item(&sd->work_struct) and hence doesn't require ordering. Also, it is not being used on a memory reclaim path. Hence, the singlethreaded workqueue has been replaced with the use of system_wq. System workqueues have been able to handle high level of concurrency for a long time now and hence it's not required to have a singlethreaded workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue created with create_singlethread_workqueue(), system_wq allows multiple work items to overlap executions even on the same CPU; however, a per-cpu workqueue doesn't have any CPU locality or global ordering guarantee unless the target CPU is explicitly specified and thus the increase of local concurrency shouldn't make any difference. Work item has been flushed in sd_stop0() to ensure that there are no pending tasks while disconnecting the driver. Signed-off-by: Bhaktipriya Shridhar --- drivers/media/usb/gspca/jl2005bcd.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/media/usb/gspca/jl2005bcd.c b/drivers/media/usb/gspca/jl2005bcd.c index 5b481fa..ac295f0 100644 --- a/drivers/media/usb/gspca/jl2005bcd.c +++ b/drivers/media/usb/gspca/jl2005bcd.c @@ -45,7 +45,6 @@ struct sd { const struct v4l2_pix_format *cap_mode; /* Driver stuff */ struct work_struct work_struct; - struct workqueue_struct *work_thread; u8 frame_brightness; int block_size; /* block size of camera */ int vga;/* 1 if vga cam, 0 if cif cam */ @@ -477,9 +476,7 @@ static int sd_start(struct gspca_dev *gspca_dev) return -1; } - /* Start the workqueue function to do the streaming */ - sd->work_thread = create_singlethread_workqueue(MODULE_NAME); - queue_work(sd->work_thread, &sd->work_struct); + schedule_work(&sd->work_struct); return 0; } @@ -493,8 +490,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev) /* wait for the work queue to terminate */ mutex_unlock(&gspca_dev->usb_lock); /* This waits for sq905c_dostream to finish */ - destroy_workqueue(dev->work_thread); - dev->work_thread = NULL; + flush_work(&dev->work_struct); mutex_lock(&gspca_dev->usb_lock); } -- 2.1.4
Re: [PATCH] i2c: efm32: fix a failure path in efm32_i2c_probe()
On Sat, Jul 16, 2016 at 02:36:38AM +0300, Alexey Khoroshilov wrote: > There is the only failure path in efm32_i2c_probe(), > where clk_disable_unprepare() is missed. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Alexey Khoroshilov oops, this is wrong since the beginning. Fixes: 1b5b23718b84 ("i2c: efm32: new bus driver") Acked-by: Uwe Kleine-König Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | http://www.pengutronix.de/ |
[PATCH] [media] gspca: finepix: Remove deprecated create_singlethread_workqueue
The workqueue "work_thread" is involved in streaming the camera data. It has a single work item(&dev->work_struct) and hence doesn't require ordering. Also, it is not being used on a memory reclaim path. Hence, the singlethreaded workqueue has been replaced with the use of system_wq. System workqueues have been able to handle high level of concurrency for a long time now and hence it's not required to have a singlethreaded workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue created with create_singlethread_workqueue(), system_wq allows multiple work items to overlap executions even on the same CPU; however, a per-cpu workqueue doesn't have any CPU locality or global ordering guarantee unless the target CPU is explicitly specified and thus the increase of local concurrency shouldn't make any difference. Work item has been flushed in sd_stop0() to ensure that there are no pending tasks while disconnecting the driver. Signed-off-by: Bhaktipriya Shridhar --- drivers/media/usb/gspca/finepix.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/media/usb/gspca/finepix.c b/drivers/media/usb/gspca/finepix.c index 52bdb56..ae9a55d 100644 --- a/drivers/media/usb/gspca/finepix.c +++ b/drivers/media/usb/gspca/finepix.c @@ -41,7 +41,6 @@ struct usb_fpix { struct gspca_dev gspca_dev; /* !! must be the first item */ struct work_struct work_struct; - struct workqueue_struct *work_thread; }; /* Delay after which claim the next frame. If the delay is too small, @@ -226,9 +225,7 @@ static int sd_start(struct gspca_dev *gspca_dev) /* Again, reset bulk in endpoint */ usb_clear_halt(gspca_dev->dev, gspca_dev->urb[0]->pipe); - /* Start the workqueue function to do the streaming */ - dev->work_thread = create_singlethread_workqueue(MODULE_NAME); - queue_work(dev->work_thread, &dev->work_struct); + schedule_work(&dev->work_struct); return 0; } @@ -241,9 +238,8 @@ static void sd_stop0(struct gspca_dev *gspca_dev) /* wait for the work queue to terminate */ mutex_unlock(&gspca_dev->usb_lock); - destroy_workqueue(dev->work_thread); + flush_work(&dev->work_struct); mutex_lock(&gspca_dev->usb_lock); - dev->work_thread = NULL; } /* Table of supported USB devices */ -- 2.1.4
Re: [patch] mtd: maps: sa1100-flash: potential NULL dereference
I like the Fixes tag because it was my invention. :) It's a separate thing from -stable. It's nice for reviewing so you can see the original intent of the patch you're fixing. Also it forces you to find the original authors and CC them so hopefully they Ack the patch. The other thing is it lets you collect data about which patches introduce bugs and how quickly they get fixed. So for example, lwn.net recently had an article about bug that are backported into the -stable tree. regards, dan carpenter
Re: [RESEND PATCH] arm: colibri_pxa270_defconfig: disable IDE subsystem
Bartlomiej Zolnierkiewicz writes: > This patch disables deprecated IDE subsystem in colibri_pxa270_defconfig > (no IDE host drivers are selected in this config so there is no valid > reason to enable IDE subsystem itself). Hi Bartlomiej, It seems I have missed that serie a year ago ... I can't remember why I have not taken it in at that time, I'm still going through my mails ... If I don't find anything relevant, and given that I'm not expecting anyone to object, I'm going to queue that in pxa/for-next within the next week. Cheers. -- Robert
[PATCH 0/2] Remove improper workqueue usage
This patch set fixes the improper usage of the workqueue API. This includes dropping the freeing of workqueue and removing the deprecated create_singlethread_workqueue instance. Bhaktipriya Shridhar (2): [media] cx25821: Drop Freeing of Workqueue [media] cx25821: Remove deprecated create_singlethread_workqueue drivers/media/pci/cx25821/cx25821-audio-upstream.c | 13 ++--- drivers/media/pci/cx25821/cx25821.h| 1 - 2 files changed, 2 insertions(+), 12 deletions(-) -- 2.1.4
[PATCH 1/2] [media] cx25821: Drop Freeing of Workqueue
Workqueues shouldn't be freed. destroy_workqueue should be used instead. destroy_workqueue safely destroys a workqueue and ensures that all pending work items are done before destroying the workqueue. Signed-off-by: Bhaktipriya Shridhar --- drivers/media/pci/cx25821/cx25821-audio-upstream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/pci/cx25821/cx25821-audio-upstream.c b/drivers/media/pci/cx25821/cx25821-audio-upstream.c index 68dbc2d..05bd957 100644 --- a/drivers/media/pci/cx25821/cx25821-audio-upstream.c +++ b/drivers/media/pci/cx25821/cx25821-audio-upstream.c @@ -242,7 +242,7 @@ void cx25821_stop_upstream_audio(struct cx25821_dev *dev) dev->_audioframe_count = 0; dev->_audiofile_status = END_OF_FILE; - kfree(dev->_irq_audio_queues); + destroy_workqueue(dev->_irq_audio_queues); dev->_irq_audio_queues = NULL; kfree(dev->_audiofilename); -- 2.1.4
[PATCH 2/2] [media] cx25821: Remove deprecated create_singlethread_workqueue
The workqueue "_irq_audio_queues" runs the audio upstream handler. It has a single work item(&dev->_audio_work_entry) and hence doesn't require ordering. Also, it is not being used on a memory reclaim path. Hence, the singlethreaded workqueue has been replaced with the use of system_wq. System workqueues have been able to handle high level of concurrency for a long time now and hence it's not required to have a singlethreaded workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue created with create_singlethread_workqueue(), system_wq allows multiple work items to overlap executions even on the same CPU; however, a per-cpu workqueue doesn't have any CPU locality or global ordering guarantee unless the target CPU is explicitly specified and thus the increase of local concurrency shouldn't make any difference. Signed-off-by: Bhaktipriya Shridhar --- drivers/media/pci/cx25821/cx25821-audio-upstream.c | 11 +-- drivers/media/pci/cx25821/cx25821.h| 1 - 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/drivers/media/pci/cx25821/cx25821-audio-upstream.c b/drivers/media/pci/cx25821/cx25821-audio-upstream.c index 05bd957..1a86dff 100644 --- a/drivers/media/pci/cx25821/cx25821-audio-upstream.c +++ b/drivers/media/pci/cx25821/cx25821-audio-upstream.c @@ -446,8 +446,7 @@ static int cx25821_audio_upstream_irq(struct cx25821_dev *dev, int chan_num, dev->_audioframe_index = dev->_last_index_irq; - queue_work(dev->_irq_audio_queues, - &dev->_audio_work_entry); + schedule_work(&dev->_audio_work_entry); } if (dev->_is_first_audio_frame) { @@ -639,14 +638,6 @@ int cx25821_audio_upstream_init(struct cx25821_dev *dev, int channel_select) /* Work queue */ INIT_WORK(&dev->_audio_work_entry, cx25821_audioups_handler); - dev->_irq_audio_queues = - create_singlethread_workqueue("cx25821_audioworkqueue"); - - if (!dev->_irq_audio_queues) { - printk(KERN_DEBUG - pr_fmt("ERROR: create_singlethread_workqueue() for Audio FAILED!\n")); - return -ENOMEM; - } dev->_last_index_irq = 0; dev->_audio_is_running = 0; diff --git a/drivers/media/pci/cx25821/cx25821.h b/drivers/media/pci/cx25821/cx25821.h index a513b68..c813d88 100644 --- a/drivers/media/pci/cx25821/cx25821.h +++ b/drivers/media/pci/cx25821/cx25821.h @@ -294,7 +294,6 @@ struct cx25821_dev { u32 audio_upstream_riscbuf_size; u32 audio_upstream_databuf_size; int _audioframe_index; - struct workqueue_struct *_irq_audio_queues; struct work_struct _audio_work_entry; char *input_audiofilename; -- 2.1.4
[no subject]
Hi Markus, Can you take a look at this. Let me know if this looks ok, I'll resend the whole series again. Regards,
[PATCH v5 2/4] nbd: fix might_sleep warning on socket shutdown.
From: "Pranay Kr. Srivastava" spinlocked ranges should be small and not contain calls into huge subfunctions. Fix my mistake and just get the pointer to the socket instead of doing everything with spinlock held. Reported-by: Mikulas Patocka Signed-off-by: Markus Pargmann Changelog: Pranay Kr. Srivastava: 1) Use spin_lock instead of irq version for sock_shutdown. 2) Use system work queue to actually trigger the shutdown of socket. This solves the issue when kernel_sendmsg is currently blocked while a timeout occurs. v5) removed unnecessary code as per review of v4). Signed-off-by: Pranay Kr Srivastava --- drivers/block/nbd.c | 53 +++-- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 766c401..4919760 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -39,6 +39,7 @@ #include #include +#include struct nbd_device { u32 flags; @@ -69,6 +70,10 @@ struct nbd_device { #if IS_ENABLED(CONFIG_DEBUG_FS) struct dentry *dbg_dir; #endif + /* +*This is specifically for calling sock_shutdown, for now. +*/ + struct work_struct ws_shutdown; }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -95,6 +100,11 @@ static int max_part; */ static DEFINE_SPINLOCK(nbd_lock); +/* + * Shutdown function for nbd_dev work struct. + */ +static void nbd_ws_func_shutdown(struct work_struct *); + static inline struct device *nbd_to_dev(struct nbd_device *nbd) { return disk_to_dev(nbd->disk); @@ -172,39 +182,30 @@ static void nbd_end_request(struct nbd_device *nbd, struct request *req) */ static void sock_shutdown(struct nbd_device *nbd) { - spin_lock_irq(&nbd->sock_lock); - - if (!nbd->sock) { - spin_unlock_irq(&nbd->sock_lock); - return; - } + struct socket *sock; - dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - sockfd_put(nbd->sock); + spin_lock(&nbd->sock_lock); + sock = nbd->sock; nbd->sock = NULL; - spin_unlock_irq(&nbd->sock_lock); + spin_unlock(&nbd->sock_lock); + + if (!sock) + return; del_timer(&nbd->timeout_timer); + dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); + kernel_sock_shutdown(sock, SHUT_RDWR); + sockfd_put(sock); } static void nbd_xmit_timeout(unsigned long arg) { struct nbd_device *nbd = (struct nbd_device *)arg; - unsigned long flags; if (list_empty(&nbd->queue_head)) return; - - spin_lock_irqsave(&nbd->sock_lock, flags); - nbd->timedout = true; - - if (nbd->sock) - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); - - spin_unlock_irqrestore(&nbd->sock_lock, flags); - + schedule_work(&nbd->ws_shutdown); dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down connection\n"); } @@ -663,6 +664,7 @@ static void nbd_reset(struct nbd_device *nbd) set_capacity(nbd->disk, 0); nbd->flags = 0; nbd->xmit_timeout = 0; + INIT_WORK(&nbd->ws_shutdown, nbd_ws_func_shutdown); queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); del_timer_sync(&nbd->timeout_timer); } @@ -798,10 +800,9 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_dev_dbg_close(nbd); kthread_stop(thread); + sock_shutdown(nbd); mutex_lock(&nbd->tx_lock); nbd->task_recv = NULL; - - sock_shutdown(nbd); nbd_clear_que(nbd); kill_bdev(bdev); nbd_bdev_reset(bdev); @@ -857,6 +858,14 @@ static const struct block_device_operations nbd_fops = { .compat_ioctl = nbd_ioctl, }; +static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) +{ + struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device, + ws_shutdown); + + sock_shutdown(nbd_dev); +} + #if IS_ENABLED(CONFIG_DEBUG_FS) static int nbd_dbg_tasks_show(struct seq_file *s, void *unused) -- 2.6.2
Re: [Nbd] [PATCH 2/2] nbd: Disallow ioctls on disconnected block device
On 16 Jul 2016, at 08:42, Pranay Srivastava wrote: > So instead can't we put a mechanism in place for network address + mac > to be same > for allowing clients to reconnect? Do let me know if this is not of concern. MAC address?! nbd clients connect over IP, and if a router reboots between them, you could easily see two packets from the same client come from different MAC addresses. Similarly all clients not on the same L2 network will carry the same MAC address. So MAC address is a very poor indicator of 'same client'. IP address is also a poor indicator (think NAT) but is substantially less bad. -- Alex Bligh
[PATCH] [media] ad9389b: Remove deprecated create_singlethread_workqueue
The workqueue work_queue is involved in EDID (Extended Display Identification Data) handling. It has a single work item(&state->edid_handler) and hence doesn't require ordering. It is not being used on a memory reclaim path. Hence, the singlethreaded workqueue has been replaced with the use of system_wq. &state->edid_handler is a self requeueing work item and it has been been sync cancelled in ad9389b_remove() to ensure that nothing is pending when the driver is disconnected. Signed-off-by: Bhaktipriya Shridhar --- drivers/media/i2c/ad9389b.c | 20 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c index 0462f46..1ac552d 100644 --- a/drivers/media/i2c/ad9389b.c +++ b/drivers/media/i2c/ad9389b.c @@ -98,7 +98,6 @@ struct ad9389b_state { struct ad9389b_state_edid edid; /* Running counter of the number of detected EDIDs (for debugging) */ unsigned edid_detect_counter; - struct workqueue_struct *work_queue; struct delayed_work edid_handler; /* work entry */ }; @@ -843,8 +842,7 @@ static void ad9389b_edid_handler(struct work_struct *work) v4l2_dbg(1, debug, sd, "%s: edid read failed\n", __func__); ad9389b_s_power(sd, false); ad9389b_s_power(sd, true); - queue_delayed_work(state->work_queue, - &state->edid_handler, EDID_DELAY); + schedule_delayed_work(&state->edid_handler, EDID_DELAY); return; } } @@ -933,8 +931,7 @@ static void ad9389b_update_monitor_present_status(struct v4l2_subdev *sd) ad9389b_setup(sd); ad9389b_notify_monitor_detect(sd); state->edid.read_retries = EDID_MAX_RETRIES; - queue_delayed_work(state->work_queue, - &state->edid_handler, EDID_DELAY); + schedule_delayed_work(&state->edid_handler, EDID_DELAY); } else if (!(status & MASK_AD9389B_HPD_DETECT)) { v4l2_dbg(1, debug, sd, "%s: hotplug not detected\n", __func__); state->have_monitor = false; @@ -1065,8 +1062,7 @@ static bool ad9389b_check_edid_status(struct v4l2_subdev *sd) ad9389b_wr(sd, 0xc9, 0xf); ad9389b_wr(sd, 0xc4, state->edid.segments); state->edid.read_retries = EDID_MAX_RETRIES; - queue_delayed_work(state->work_queue, - &state->edid_handler, EDID_DELAY); + schedule_delayed_work(&state->edid_handler, EDID_DELAY); return false; } @@ -1170,13 +1166,6 @@ static int ad9389b_probe(struct i2c_client *client, const struct i2c_device_id * goto err_entity; } - state->work_queue = create_singlethread_workqueue(sd->name); - if (state->work_queue == NULL) { - v4l2_err(sd, "could not create workqueue\n"); - err = -ENOMEM; - goto err_unreg; - } - INIT_DELAYED_WORK(&state->edid_handler, ad9389b_edid_handler); state->dv_timings = dv1080p60; @@ -1211,9 +1200,8 @@ static int ad9389b_remove(struct i2c_client *client) ad9389b_s_stream(sd, false); ad9389b_s_audio_stream(sd, false); ad9389b_init_setup(sd); - cancel_delayed_work(&state->edid_handler); + cancel_delayed_work_sync(&state->edid_handler); i2c_unregister_device(state->edid_i2c_client); - destroy_workqueue(state->work_queue); v4l2_device_unregister_subdev(sd); media_entity_cleanup(&sd->entity); v4l2_ctrl_handler_free(sd->ctrl_handler); -- 2.1.4
Re: [Nbd] [PATCH 2/2] nbd: Disallow ioctls on disconnected block device
On Sat, Jul 16, 2016 at 3:02 PM, Alex Bligh wrote: > > On 16 Jul 2016, at 08:42, Pranay Srivastava wrote: > >> So instead can't we put a mechanism in place for network address + mac >> to be same >> for allowing clients to reconnect? Do let me know if this is not of concern. > > MAC address?! nbd clients connect over IP, and if a router reboots > between them, you could easily see two packets from the same client > come from different MAC addresses. Similarly all clients not on > the same L2 network will carry the same MAC address. So MAC address > is a very poor indicator of 'same client'. > > IP address is also a poor indicator (think NAT) but is substantially > less bad. Okay. So how about we include some negotiated key which goes in with every request which the server could maintain for clients that can be checked while resetting the connection with the same server? So am I correct that this situation can indeed happen or the server will throw an error back to client in case the troubled nbd-client is trying to reconnect to the original server but requests are going to another server? If yes to above query then what is the best effort we can do to avoid such scenarios? > > -- > Alex Bligh > > > > -- ---P.K.S
Re: [PATCH 4/5] mm: show node_pages_scanned per node, not zone
On Fri, Jul 15, 2016 at 02:09:24PM +0100, Mel Gorman wrote: > From: Minchan Kim > > The node_pages_scanned represents the number of scanned pages > of node for reclaim so it's pointless to show it as kilobytes. > > As well, node_pages_scanned is per-node value, not per-zone. > > This patch changes node_pages_scanned per-zone-killobytes > with per-node-count. > > Signed-off-by: Minchan Kim > Signed-off-by: Mel Gorman > --- > mm/page_alloc.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index f80a0e57dcc8..7edd311a63f1 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -4345,6 +4345,7 @@ void show_free_areas(unsigned int filter) > #endif > " writeback_tmp:%lukB" > " unstable:%lukB" > + " pages_scanned:%lu" > " all_unreclaimable? %s" > "\n", > pgdat->node_id, > @@ -4367,6 +4368,7 @@ void show_free_areas(unsigned int filter) > K(node_page_state(pgdat, NR_SHMEM)), > K(node_page_state(pgdat, NR_WRITEBACK_TEMP)), > K(node_page_state(pgdat, NR_UNSTABLE_NFS)), > + node_page_state(zone->zone_pgdat, NR_PAGES_SCANNED), Oops, It should be pgdat, not zone->zone_pgdat. Andrew, please fold it. >From 0b058f64335764b82439a3c24fad8573cc1474d7 Mon Sep 17 00:00:00 2001 From: Minchan Kim Date: Sat, 16 Jul 2016 19:07:51 +0900 Subject: [PATCH] mm: fix node_pages_scanned Use pgdat for node stat instead of zone->pgdat. Signed-off-by: Minchan Kim --- mm/page_alloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 7edd311..7547b6b 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -4368,7 +4368,7 @@ void show_free_areas(unsigned int filter) K(node_page_state(pgdat, NR_SHMEM)), K(node_page_state(pgdat, NR_WRITEBACK_TEMP)), K(node_page_state(pgdat, NR_UNSTABLE_NFS)), - node_page_state(zone->zone_pgdat, NR_PAGES_SCANNED), + node_page_state(pgdat, NR_PAGES_SCANNED), !pgdat_reclaimable(pgdat) ? "yes" : "no"); } -- 1.9.1
Re: [PATCH v5 2/4] nbd: fix might_sleep warning on socket shutdown.
Hi Markus, On Sat, Jul 16, 2016 at 2:52 PM, Pranay Kr Srivastava wrote: > From: "Pranay Kr. Srivastava" > > spinlocked ranges should be small and not contain calls into huge > subfunctions. Fix my mistake and just get the pointer to the socket > instead of doing everything with spinlock held. > > Reported-by: Mikulas Patocka > Signed-off-by: Markus Pargmann > > Changelog: > Pranay Kr. Srivastava: > > 1) Use spin_lock instead of irq version for sock_shutdown. > > 2) Use system work queue to actually trigger the shutdown of >socket. This solves the issue when kernel_sendmsg is currently >blocked while a timeout occurs. > > v5) >removed unnecessary code as per review of v4). > > Signed-off-by: Pranay Kr Srivastava > --- > drivers/block/nbd.c | 53 > +++-- > 1 file changed, 31 insertions(+), 22 deletions(-) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 766c401..4919760 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -39,6 +39,7 @@ > #include > > #include > +#include > > struct nbd_device { > u32 flags; > @@ -69,6 +70,10 @@ struct nbd_device { > #if IS_ENABLED(CONFIG_DEBUG_FS) > struct dentry *dbg_dir; > #endif > + /* > +*This is specifically for calling sock_shutdown, for now. > +*/ > + struct work_struct ws_shutdown; > }; > > #if IS_ENABLED(CONFIG_DEBUG_FS) > @@ -95,6 +100,11 @@ static int max_part; > */ > static DEFINE_SPINLOCK(nbd_lock); > > +/* > + * Shutdown function for nbd_dev work struct. > + */ > +static void nbd_ws_func_shutdown(struct work_struct *); > + > static inline struct device *nbd_to_dev(struct nbd_device *nbd) > { > return disk_to_dev(nbd->disk); > @@ -172,39 +182,30 @@ static void nbd_end_request(struct nbd_device *nbd, > struct request *req) > */ > static void sock_shutdown(struct nbd_device *nbd) > { > - spin_lock_irq(&nbd->sock_lock); > - > - if (!nbd->sock) { > - spin_unlock_irq(&nbd->sock_lock); > - return; > - } > + struct socket *sock; > > - dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); > - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); > - sockfd_put(nbd->sock); > + spin_lock(&nbd->sock_lock); > + sock = nbd->sock; > nbd->sock = NULL; > - spin_unlock_irq(&nbd->sock_lock); > + spin_unlock(&nbd->sock_lock); > + > + if (!sock) > + return; > > del_timer(&nbd->timeout_timer); > + dev_warn(disk_to_dev(nbd->disk), "shutting down socket\n"); > + kernel_sock_shutdown(sock, SHUT_RDWR); > + sockfd_put(sock); > } > > static void nbd_xmit_timeout(unsigned long arg) > { > struct nbd_device *nbd = (struct nbd_device *)arg; > - unsigned long flags; > > if (list_empty(&nbd->queue_head)) > return; > - > - spin_lock_irqsave(&nbd->sock_lock, flags); > - > nbd->timedout = true; > - > - if (nbd->sock) > - kernel_sock_shutdown(nbd->sock, SHUT_RDWR); > - > - spin_unlock_irqrestore(&nbd->sock_lock, flags); > - > + schedule_work(&nbd->ws_shutdown); > dev_err(nbd_to_dev(nbd), "Connection timed out, shutting down > connection\n"); > } > > @@ -663,6 +664,7 @@ static void nbd_reset(struct nbd_device *nbd) > set_capacity(nbd->disk, 0); > nbd->flags = 0; > nbd->xmit_timeout = 0; > + INIT_WORK(&nbd->ws_shutdown, nbd_ws_func_shutdown); > queue_flag_clear_unlocked(QUEUE_FLAG_DISCARD, nbd->disk->queue); > del_timer_sync(&nbd->timeout_timer); > } > @@ -798,10 +800,9 @@ static int __nbd_ioctl(struct block_device *bdev, struct > nbd_device *nbd, > nbd_dev_dbg_close(nbd); > kthread_stop(thread); > > + sock_shutdown(nbd); > mutex_lock(&nbd->tx_lock); > nbd->task_recv = NULL; > - > - sock_shutdown(nbd); > nbd_clear_que(nbd); > kill_bdev(bdev); > nbd_bdev_reset(bdev); > @@ -857,6 +858,14 @@ static const struct block_device_operations nbd_fops = { > .compat_ioctl = nbd_ioctl, > }; > > +static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) > +{ > + struct nbd_device *nbd_dev = container_of(ws_nbd, struct nbd_device, > + ws_shutdown); > + > + sock_shutdown(nbd_dev); > +} > + > #if IS_ENABLED(CONFIG_DEBUG_FS) > > static int nbd_dbg_tasks_show(struct seq_file *s, void *unused) > -- > 2.6.2 > Made the changes you suggested. Let me know if these look alright so I can resend the whole series. -- ---P.K.S
Re: [PATCH] s390/cio/chp : Remove deprecated create_singlethread_workqueue
On Sat, 16 Jul 2016, Bhaktipriya Shridhar wrote: > The workqueue "chp_wq" is involved in performing pending > configure tasks for channel paths. > > It has a single work item(&cfg_work) and hence doesn't require > ordering. Also, it is not being used on a memory reclaim path. > Hence, the singlethreaded workqueue has been replaced with the use of > system_wq. > > System workqueues have been able to handle high level of concurrency > for a long time now and hence it's not required to have a singlethreaded > workqueue just to gain concurrency. Unlike a dedicated per-cpu workqueue > created with create_singlethread_workqueue(), system_wq allows multiple > work items to overlap executions even on the same CPU; however, a > per-cpu workqueue doesn't have any CPU locality or global ordering > guarantee unless the target CPU is explicitly specified and thus the > increase of local concurrency shouldn't make any difference. > > Signed-off-by: Bhaktipriya Shridhar Applied, thanks! Sebastian
[PATCH v5 3/4] make nbd device wait for its users
When a timeout occurs or a recv fails, then instead of abruptly killing nbd block device wait for its users to finish. This is more required when filesystem(s) like ext2 or ext3 don't expect their buffer heads to disappear while the filesystem is mounted. Each open is now refcounted with the device being released for re-use only when there are no "other users". A timedout or a disconnected device, if in use, can't be used until it has been resetted. The reset happens when all tasks having this bdev open closes this bdev. Behavioral Change: 1) NBD_DO_IT will not wait for the device to be reset. Hence the nbd-client "may exit" while some other process is using this device without actually doing the reset on this device , hence thus making it unusable until all such user space processes have stopped using this device. 2) There's a window where the nbd-client will not be able to change / issue ioctls to the nbd device. This is when there's been a disconnect issued or a timeout has occured, however there are "other" user processes which currently have this nbd device opened. Signed-off-by: Pranay Kr Srivastava --- drivers/block/nbd.c | 37 - 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 4919760..fe36280 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -74,6 +74,7 @@ struct nbd_device { *This is specifically for calling sock_shutdown, for now. */ struct work_struct ws_shutdown; + atomic_t users; /* Users that opened the block device */ }; #if IS_ENABLED(CONFIG_DEBUG_FS) @@ -699,6 +700,9 @@ static void nbd_dev_dbg_close(struct nbd_device *nbd); static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, unsigned int cmd, unsigned long arg) { + if (nbd->disconnect || nbd->timedout) + return -EBUSY; + switch (cmd) { case NBD_DISCONNECT: { struct request sreq; @@ -728,7 +732,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, nbd_clear_que(nbd); BUG_ON(!list_empty(&nbd->queue_head)); BUG_ON(!list_empty(&nbd->waiting_queue)); - kill_bdev(bdev); return 0; case NBD_SET_SOCK: { @@ -804,16 +807,12 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, mutex_lock(&nbd->tx_lock); nbd->task_recv = NULL; nbd_clear_que(nbd); - kill_bdev(bdev); - nbd_bdev_reset(bdev); if (nbd->disconnect) /* user requested, ignore socket errors */ error = 0; if (nbd->timedout) error = -ETIMEDOUT; - nbd_reset(nbd); - return error; } @@ -852,10 +851,38 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t mode, return error; } +static int nbd_open(struct block_device *bdev, fmode_t mode) +{ + struct nbd_device *nbd = bdev->bd_disk->private_data; + + atomic_inc(&nbd->users); + + return 0; +} + +static void nbd_release(struct gendisk *disk, fmode_t mode) +{ + struct nbd_device *nbd = disk->private_data; + struct block_device *bdev = bdget (part_devt( + dev_to_part(nbd_to_dev(nbd; + + WARN_ON(!bdev); + if (atomic_dec_and_test(&nbd->users)) { + if (bdev) { + nbd_bdev_reset(bdev); + kill_bdev(bdev); + bdput(bdev); + } + nbd_reset(nbd); + } +} + static const struct block_device_operations nbd_fops = { .owner =THIS_MODULE, .ioctl =nbd_ioctl, .compat_ioctl = nbd_ioctl, + .open = nbd_open, + .release = nbd_release, }; static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) -- 2.6.2
[PATCH v3 1/9] [media] videodev2.h Add HSV formats
These formats store the color information of the image in a geometrical representation. The colors are mapped into a cylinder, where the angle is the HUE, the height is the VALUE and the distance to the center is the SATURATION. This is a very useful format for image segmentation algorithms. Signed-off-by: Ricardo Ribalda Delgado --- drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++ include/uapi/linux/videodev2.h | 4 2 files changed, 6 insertions(+) diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index f899bf1c5fc0..54670cd59212 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -1238,6 +1238,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) case V4L2_PIX_FMT_TM6000: descr = "A/V + VBI Mux Packet"; break; case V4L2_PIX_FMT_CIT_YYVYUY: descr = "GSPCA CIT YYVYUY"; break; case V4L2_PIX_FMT_KONICA420:descr = "GSPCA KONICA420"; break; + case V4L2_PIX_FMT_HSV24:descr = "24-bit HSV 8-8-8"; break; + case V4L2_PIX_FMT_HSV32:descr = "32-bit XHSV 8-8-8-8"; break; case V4L2_SDR_FMT_CU8: descr = "Complex U8"; break; case V4L2_SDR_FMT_CU16LE: descr = "Complex U16LE"; break; case V4L2_SDR_FMT_CS8: descr = "Complex S8"; break; diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 724f43e69d03..c7fb760386cf 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -580,6 +580,10 @@ struct v4l2_pix_format { #define V4L2_PIX_FMT_SRGGB12 v4l2_fourcc('R', 'G', '1', '2') /* 12 RGRG.. GBGB.. */ #define V4L2_PIX_FMT_SBGGR16 v4l2_fourcc('B', 'Y', 'R', '2') /* 16 BGBG.. GRGR.. */ +/* HSV formats */ +#define V4L2_PIX_FMT_HSV24 v4l2_fourcc('H', 'S', 'V', '3') +#define V4L2_PIX_FMT_HSV32 v4l2_fourcc('H', 'S', 'V', '4') + /* compressed formats */ #define V4L2_PIX_FMT_MJPEGv4l2_fourcc('M', 'J', 'P', 'G') /* Motion-JPEG */ #define V4L2_PIX_FMT_JPEG v4l2_fourcc('J', 'P', 'E', 'G') /* JFIF JPEG */ -- 2.8.1
[PATCH v3 0/9] Add HSV format
HSV formats are extremely useful for image segmentation. This set of patches makes v4l2 aware of this kind of formats. Vivid changes have been divided to ease the reviewing process. We are working on patches for Gstreamer and OpenCV that will make use of these formats. We still need to decide if and how we will support HUE range 0-255 Changelog: v3: Fix wrong handling of some YUV formats when brightness != 128 Suggested by Laurent Pinchart -Remove unneeded empty lines on .rst file Thanks! Suggested by Hans Verkuil -Rebase over master and docs-next -Introduce TPG_COLOR_ENC_LUMA for gray formats -CodeStyle Thanks! v2: Suggested by Mauro Carvalho Chehab , -Rebase on top of docs-next (port documentation to .rst) Ricardo Ribalda Delgado (9): [media] videodev2.h Add HSV formats [media] Documentation: Add HSV format [media] Documentation: Add Ricardo Ribalda [media] vivid: code refactor for color encoding [media] vivid: Add support for HSV formats [media] vivid: Rename variable [media] vivid: Introduce TPG_COLOR_ENC_LUMA [media] vivid: Fix YUV555 and YUV565 handling [media] vivid: Local optimization Documentation/media/uapi/v4l/hsv-formats.rst | 19 + Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst | 158 Documentation/media/uapi/v4l/pixfmt.rst| 1 + Documentation/media/uapi/v4l/v4l2.rst | 9 + drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 399 + drivers/media/platform/vivid/vivid-core.h | 2 +- drivers/media/platform/vivid/vivid-vid-common.c| 66 ++-- drivers/media/v4l2-core/v4l2-ioctl.c | 2 + include/media/v4l2-tpg.h | 9 +- include/uapi/linux/videodev2.h | 4 + 10 files changed, 499 insertions(+), 170 deletions(-) create mode 100644 Documentation/media/uapi/v4l/hsv-formats.rst create mode 100644 Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst -- 2.8.1
[PATCH v3 6/9] [media] vivid: Rename variable
r_y and g_u now also contain the H and V components on the HSV formats. Rename the variables to reflect this. Signed-off-by: Ricardo Ribalda Delgado --- drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 209 +- 1 file changed, 105 insertions(+), 104 deletions(-) diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c index 53e512f5a6b6..ba3fe65ceb98 100644 --- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c +++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c @@ -1014,7 +1014,7 @@ static void gen_twopix(struct tpg_data *tpg, { unsigned offset = odd * tpg->twopixelsize[0] / 2; u8 alpha = tpg->alpha_component; - u8 r_y, g_u, b_v; + u8 r_y_h, g_u_s, b_v; if (tpg->alpha_red_only && color != TPG_COLOR_CSC_RED && color != TPG_COLOR_100_RED && @@ -1022,161 +1022,161 @@ static void gen_twopix(struct tpg_data *tpg, alpha = 0; if (color == TPG_COLOR_RANDOM) precalculate_color(tpg, color); - r_y = tpg->colors[color][0]; /* R or precalculated Y, H */ - g_u = tpg->colors[color][1]; /* G or precalculated U, V */ + r_y_h = tpg->colors[color][0]; /* R or precalculated Y, H */ + g_u_s = tpg->colors[color][1]; /* G or precalculated U, V */ b_v = tpg->colors[color][2]; /* B or precalculated V */ switch (tpg->fourcc) { case V4L2_PIX_FMT_GREY: - buf[0][offset] = r_y; + buf[0][offset] = r_y_h; break; case V4L2_PIX_FMT_Y16: /* -* Ideally both bytes should be set to r_y, but then you won't +* Ideally both bytes should be set to r_y_h, but then you won't * be able to detect endian problems. So keep it 0 except for -* the corner case where r_y is 0xff so white really will be +* the corner case where r_y_h is 0xff so white really will be * white (0x). */ - buf[0][offset] = r_y == 0xff ? r_y : 0; - buf[0][offset+1] = r_y; + buf[0][offset] = r_y_h == 0xff ? r_y_h : 0; + buf[0][offset+1] = r_y_h; break; case V4L2_PIX_FMT_Y16_BE: /* See comment for V4L2_PIX_FMT_Y16 above */ - buf[0][offset] = r_y; - buf[0][offset+1] = r_y == 0xff ? r_y : 0; + buf[0][offset] = r_y_h; + buf[0][offset+1] = r_y_h == 0xff ? r_y_h : 0; break; case V4L2_PIX_FMT_YUV422M: case V4L2_PIX_FMT_YUV422P: case V4L2_PIX_FMT_YUV420: case V4L2_PIX_FMT_YUV420M: - buf[0][offset] = r_y; + buf[0][offset] = r_y_h; if (odd) { - buf[1][0] = (buf[1][0] + g_u) / 2; + buf[1][0] = (buf[1][0] + g_u_s) / 2; buf[2][0] = (buf[2][0] + b_v) / 2; buf[1][1] = buf[1][0]; buf[2][1] = buf[2][0]; break; } - buf[1][0] = g_u; + buf[1][0] = g_u_s; buf[2][0] = b_v; break; case V4L2_PIX_FMT_YVU422M: case V4L2_PIX_FMT_YVU420: case V4L2_PIX_FMT_YVU420M: - buf[0][offset] = r_y; + buf[0][offset] = r_y_h; if (odd) { buf[1][0] = (buf[1][0] + b_v) / 2; - buf[2][0] = (buf[2][0] + g_u) / 2; + buf[2][0] = (buf[2][0] + g_u_s) / 2; buf[1][1] = buf[1][0]; buf[2][1] = buf[2][0]; break; } buf[1][0] = b_v; - buf[2][0] = g_u; + buf[2][0] = g_u_s; break; case V4L2_PIX_FMT_NV12: case V4L2_PIX_FMT_NV12M: case V4L2_PIX_FMT_NV16: case V4L2_PIX_FMT_NV16M: - buf[0][offset] = r_y; + buf[0][offset] = r_y_h; if (odd) { - buf[1][0] = (buf[1][0] + g_u) / 2; + buf[1][0] = (buf[1][0] + g_u_s) / 2; buf[1][1] = (buf[1][1] + b_v) / 2; break; } - buf[1][0] = g_u; + buf[1][0] = g_u_s; buf[1][1] = b_v; break; case V4L2_PIX_FMT_NV21: case V4L2_PIX_FMT_NV21M: case V4L2_PIX_FMT_NV61: case V4L2_PIX_FMT_NV61M: - buf[0][offset] = r_y; + buf[0][offset] = r_y_h; if (odd) { buf[1][0] = (buf[1][0] + b_v) / 2; - buf[1][1] = (buf[1][1] + g_u) / 2; + buf[1][1] = (buf[1][1] + g_u_s) / 2; break;
[PATCH v3 3/9] [media] Documentation: Add Ricardo Ribalda
My initials were on the Changelog, but there was no link to my name. Signed-off-by: Ricardo Ribalda Delgado --- Documentation/media/uapi/v4l/v4l2.rst | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/media/uapi/v4l/v4l2.rst b/Documentation/media/uapi/v4l/v4l2.rst index 6d23bc987f51..330674a0f553 100644 --- a/Documentation/media/uapi/v4l/v4l2.rst +++ b/Documentation/media/uapi/v4l/v4l2.rst @@ -64,6 +64,10 @@ Authors, in alphabetical order: - SDR API. +- Ribalda, Ricardo + + - Introduce HSV formats and other minor changes. + - Rubli, Martin - Designed and documented the VIDIOC_ENUM_FRAMESIZES and VIDIOC_ENUM_FRAMEINTERVALS ioctls. -- 2.8.1
Re: [PATCH v5 3/4] make nbd device wait for its users
Hi Markus On Sat, Jul 16, 2016 at 4:06 PM, Pranay Kr Srivastava wrote: > When a timeout occurs or a recv fails, then > instead of abruptly killing nbd block device > wait for its users to finish. > > This is more required when filesystem(s) like > ext2 or ext3 don't expect their buffer heads to > disappear while the filesystem is mounted. > > Each open is now refcounted with the device being released > for re-use only when there are no "other users". > > A timedout or a disconnected device, if in use, can't > be used until it has been resetted. The reset happens > when all tasks having this bdev open closes this bdev. > > Behavioral Change: > > 1) NBD_DO_IT will not wait for the device to be reset. Hence > the nbd-client "may exit" while some other process is using > this device without actually doing the reset on this device > , hence thus making it unusable until all such user space > processes have stopped using this device. > > 2) There's a window where the nbd-client will not be able to > change / issue ioctls to the nbd device. This is when there's > been a disconnect issued or a timeout has occured, however > there are "other" user processes which currently have this > nbd device opened. > > Signed-off-by: Pranay Kr Srivastava > --- > drivers/block/nbd.c | 37 - > 1 file changed, 32 insertions(+), 5 deletions(-) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 4919760..fe36280 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -74,6 +74,7 @@ struct nbd_device { > *This is specifically for calling sock_shutdown, for now. > */ > struct work_struct ws_shutdown; > + atomic_t users; /* Users that opened the block device */ > }; > > #if IS_ENABLED(CONFIG_DEBUG_FS) > @@ -699,6 +700,9 @@ static void nbd_dev_dbg_close(struct nbd_device *nbd); > static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd, >unsigned int cmd, unsigned long arg) > { > + if (nbd->disconnect || nbd->timedout) > + return -EBUSY; > + > switch (cmd) { > case NBD_DISCONNECT: { > struct request sreq; > @@ -728,7 +732,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct > nbd_device *nbd, > nbd_clear_que(nbd); > BUG_ON(!list_empty(&nbd->queue_head)); > BUG_ON(!list_empty(&nbd->waiting_queue)); > - kill_bdev(bdev); > return 0; > > case NBD_SET_SOCK: { > @@ -804,16 +807,12 @@ static int __nbd_ioctl(struct block_device *bdev, > struct nbd_device *nbd, > mutex_lock(&nbd->tx_lock); > nbd->task_recv = NULL; > nbd_clear_que(nbd); > - kill_bdev(bdev); > - nbd_bdev_reset(bdev); > > if (nbd->disconnect) /* user requested, ignore socket errors > */ > error = 0; > if (nbd->timedout) > error = -ETIMEDOUT; > > - nbd_reset(nbd); > - > return error; > } > > @@ -852,10 +851,38 @@ static int nbd_ioctl(struct block_device *bdev, fmode_t > mode, > return error; > } > > +static int nbd_open(struct block_device *bdev, fmode_t mode) > +{ > + struct nbd_device *nbd = bdev->bd_disk->private_data; > + > + atomic_inc(&nbd->users); > + > + return 0; > +} > + > +static void nbd_release(struct gendisk *disk, fmode_t mode) > +{ > + struct nbd_device *nbd = disk->private_data; > + struct block_device *bdev = bdget (part_devt( > + > dev_to_part(nbd_to_dev(nbd; > + > + WARN_ON(!bdev); > + if (atomic_dec_and_test(&nbd->users)) { > + if (bdev) { > + nbd_bdev_reset(bdev); > + kill_bdev(bdev); > + bdput(bdev); > + } > + nbd_reset(nbd); > + } > +} > + > static const struct block_device_operations nbd_fops = { > .owner =THIS_MODULE, > .ioctl =nbd_ioctl, > .compat_ioctl = nbd_ioctl, > + .open = nbd_open, > + .release = nbd_release, > }; > > static void nbd_ws_func_shutdown(struct work_struct *ws_nbd) > -- > 2.6.2 > I've taken the patch you had send earlier , with atomics, and modified that. I've added the ioctl handling part as part of this patch instead of keeping it separate. Let me know of any changes that should be done. I'll send the whole series again later after taking the reviews of everyone. -- ---P.K.S
[PATCH v3 7/9] [media] vivid: Introduce TPG_COLOR_ENC_LUMA
Simplifies handling of Gray formats. Signed-off-by: Ricardo Ribalda Delgado --- drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 26 +++-- drivers/media/platform/vivid/vivid-vid-common.c | 6 +++--- include/media/v4l2-tpg.h| 1 + 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c index ba3fe65ceb98..e91bf3cbaab9 100644 --- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c +++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c @@ -234,10 +234,12 @@ bool tpg_s_fourcc(struct tpg_data *tpg, u32 fourcc) case V4L2_PIX_FMT_XBGR32: case V4L2_PIX_FMT_ARGB32: case V4L2_PIX_FMT_ABGR32: + tpg->color_enc = TGP_COLOR_ENC_RGB; + break; case V4L2_PIX_FMT_GREY: case V4L2_PIX_FMT_Y16: case V4L2_PIX_FMT_Y16_BE: - tpg->color_enc = TGP_COLOR_ENC_RGB; + tpg->color_enc = TGP_COLOR_ENC_LUMA; break; case V4L2_PIX_FMT_YUV444: case V4L2_PIX_FMT_YUV555: @@ -825,9 +827,9 @@ static void precalculate_color(struct tpg_data *tpg, int k) g <<= 4; b <<= 4; } - if (tpg->qual == TPG_QUAL_GRAY || tpg->fourcc == V4L2_PIX_FMT_GREY || - tpg->fourcc == V4L2_PIX_FMT_Y16 || - tpg->fourcc == V4L2_PIX_FMT_Y16_BE) { + + if (tpg->qual == TPG_QUAL_GRAY || + tpg->color_enc == TGP_COLOR_ENC_LUMA) { /* Rec. 709 Luma function */ /* (0.2126, 0.7152, 0.0722) * (255 * 256) */ r = g = b = (13879 * r + 46688 * g + 4713 * b) >> 16; @@ -867,8 +869,9 @@ static void precalculate_color(struct tpg_data *tpg, int k) b = (b - (16 << 4)) * 255 / 219; } - if (tpg->brightness != 128 || tpg->contrast != 128 || - tpg->saturation != 128 || tpg->hue) { + if ((tpg->brightness != 128 || tpg->contrast != 128 || +tpg->saturation != 128 || tpg->hue) && + tpg->color_enc != TGP_COLOR_ENC_LUMA) { /* Implement these operations */ int y, cb, cr; int tmp_cb, tmp_cr; @@ -894,6 +897,10 @@ static void precalculate_color(struct tpg_data *tpg, int k) return; } ycbcr_to_color(tpg, y, cb, cr, &r, &g, &b); + } else if ((tpg->brightness != 128 || tpg->contrast != 128) && + tpg->color_enc == TGP_COLOR_ENC_LUMA) { + r = (16 << 4) + ((r - (16 << 4)) * tpg->contrast) / 128; + r += (tpg->brightness << 4) - (128 << 4); } switch (tpg->color_enc) { @@ -944,6 +951,11 @@ static void precalculate_color(struct tpg_data *tpg, int k) tpg->colors[k][2] = cr; break; } + case TGP_COLOR_ENC_LUMA: + { + tpg->colors[k][0] = r >> 4; + break; + } case TGP_COLOR_ENC_RGB: { if (tpg->real_quantization == V4L2_QUANTIZATION_LIM_RANGE) { @@ -1985,6 +1997,8 @@ static const char *tpg_color_enc_str(enum tgp_color_enc return "HSV"; case TGP_COLOR_ENC_YUV: return "YCbCr"; + case TGP_COLOR_ENC_LUMA: + return "Luma"; case TGP_COLOR_ENC_RGB: default: return "RGB"; diff --git a/drivers/media/platform/vivid/vivid-vid-common.c b/drivers/media/platform/vivid/vivid-vid-common.c index 869e26ea7cf5..b78bca4c2f16 100644 --- a/drivers/media/platform/vivid/vivid-vid-common.c +++ b/drivers/media/platform/vivid/vivid-vid-common.c @@ -184,7 +184,7 @@ struct vivid_fmt vivid_formats[] = { .fourcc = V4L2_PIX_FMT_GREY, .vdownsampling = { 1 }, .bit_depth = { 8 }, - .color_enc = TGP_COLOR_ENC_YUV, + .color_enc = TGP_COLOR_ENC_LUMA, .planes = 1, .buffers = 1, }, @@ -192,7 +192,7 @@ struct vivid_fmt vivid_formats[] = { .fourcc = V4L2_PIX_FMT_Y16, .vdownsampling = { 1 }, .bit_depth = { 16 }, - .color_enc = TGP_COLOR_ENC_YUV, + .color_enc = TGP_COLOR_ENC_LUMA, .planes = 1, .buffers = 1, }, @@ -200,7 +200,7 @@ struct vivid_fmt vivid_formats[] = { .fourcc = V4L2_PIX_FMT_Y16_BE, .vdownsampling = { 1 }, .bit_depth = { 16 }, - .color_enc = TGP_COLOR_ENC_YUV, + .color_enc = TGP_COLOR_ENC_LUMA, .planes = 1, .buffers = 1, }, diff --git a/include/media/v4l2-tpg.h b/include/media/v4l2-tpg.h index 0f632ec619aa..d2487c12657d 100644 --- a/include/media/v4l2-tpg.h +++ b/include/media/v4l2-tpg.h @@ -91,6 +91,7 @@ enum tgp_color_enc { TGP_COLOR_ENC_RGB,
[PATCH v3 2/9] [media] Documentation: Add HSV format
Describe the HSV formats Signed-off-by: Ricardo Ribalda Delgado --- Documentation/media/uapi/v4l/hsv-formats.rst | 19 +++ Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst | 158 + Documentation/media/uapi/v4l/pixfmt.rst| 1 + Documentation/media/uapi/v4l/v4l2.rst | 5 + 4 files changed, 183 insertions(+) create mode 100644 Documentation/media/uapi/v4l/hsv-formats.rst create mode 100644 Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst diff --git a/Documentation/media/uapi/v4l/hsv-formats.rst b/Documentation/media/uapi/v4l/hsv-formats.rst new file mode 100644 index ..f0f2615eaa95 --- /dev/null +++ b/Documentation/media/uapi/v4l/hsv-formats.rst @@ -0,0 +1,19 @@ +.. -*- coding: utf-8; mode: rst -*- + +.. _hsv-formats: + +*** +HSV Formats +*** + +These formats store the color information of the image +in a geometrical representation. The colors are mapped into a +cylinder, where the angle is the HUE, the height is the VALUE +and the distance to the center is the SATURATION. This is a very +useful format for image segmentation algorithms. + + +.. toctree:: +:maxdepth: 1 + +pixfmt-packed-hsv diff --git a/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst b/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst new file mode 100644 index ..60ac821e309d --- /dev/null +++ b/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst @@ -0,0 +1,158 @@ +.. -*- coding: utf-8; mode: rst -*- + +.. _packed-hsv: + +** +Packed HSV formats +** + +*man Packed HSV formats(2)* + +Packed HSV formats + + +Description +=== + +The *hue* (h) is measured in degrees, one LSB represents two degrees. +The *saturation* (s) and the *value* (v) are measured in percentage of the +cylinder: 0 being the smallest value and 255 the maximum. + + +The values are packed in 24 or 32 bit formats. + + +.. flat-table:: Packed HSV Image Formats +:header-rows: 2 +:stub-columns: 0 + +- .. row 1 + + - Identifier + - Code + - + - :cspan:`7` Byte 0 in memory + - + - :cspan:`7` Byte 1 + - + - :cspan:`7` Byte 2 + - + - :cspan:`7` Byte 3 + +- .. row 2 + + - + - + - Bit + - 7 + - 6 + - 5 + - 4 + - 3 + - 2 + - 1 + - 0 + - + - 7 + - 6 + - 5 + - 4 + - 3 + - 2 + - 1 + - 0 + - + - 7 + - 6 + - 5 + - 4 + - 3 + - 2 + - 1 + - 0 + - + - 7 + - 6 + - 5 + - 4 + - 3 + - 2 + - 1 + - 0 + +- .. _V4L2-PIX-FMT-HSV32: + + - ``V4L2_PIX_FMT_HSV32`` + - 'HSV4' + - + - - + - - + - - + - - + - - + - - + - - + - - + - + - h\ :sub:`7` + - h\ :sub:`6` + - h\ :sub:`5` + - h\ :sub:`4` + - h\ :sub:`3` + - h\ :sub:`2` + - h\ :sub:`1` + - h\ :sub:`0` + - + - s\ :sub:`7` + - s\ :sub:`6` + - s\ :sub:`5` + - s\ :sub:`4` + - s\ :sub:`3` + - s\ :sub:`2` + - s\ :sub:`1` + - s\ :sub:`0` + - + - v\ :sub:`7` + - v\ :sub:`6` + - v\ :sub:`5` + - v\ :sub:`4` + - v\ :sub:`3` + - v\ :sub:`2` + - v\ :sub:`1` + - v\ :sub:`0` + +- .. _V4L2-PIX-FMT-HSV24: + + - ``V4L2_PIX_FMT_HSV24`` + - 'HSV3' + - + - h\ :sub:`7` + - h\ :sub:`6` + - h\ :sub:`5` + - h\ :sub:`4` + - h\ :sub:`3` + - h\ :sub:`2` + - h\ :sub:`1` + - h\ :sub:`0` + - + - s\ :sub:`7` + - s\ :sub:`6` + - s\ :sub:`5` + - s\ :sub:`4` + - s\ :sub:`3` + - s\ :sub:`2` + - s\ :sub:`1` + - s\ :sub:`0` + - + - v\ :sub:`7` + - v\ :sub:`6` + - v\ :sub:`5` + - v\ :sub:`4` + - v\ :sub:`3` + - v\ :sub:`2` + - v\ :sub:`1` + - v\ :sub:`0` + - + - + +Bit 7 is the most significant bit. diff --git a/Documentation/media/uapi/v4l/pixfmt.rst b/Documentation/media/uapi/v4l/pixfmt.rst index 81222a99f7ce..1d2270422345 100644 --- a/Documentation/media/uapi/v4l/pixfmt.rst +++ b/Documentation/media/uapi/v4l/pixfmt.rst @@ -29,6 +29,7 @@ see also :ref:`VIDIOC_G_FBUF `.) pixfmt-indexed pixfmt-rgb yuv-formats +hsv-formats depth-formats pixfmt-013 sdr-formats diff --git a/Documentation/media/uapi/v4l/v4l2.rst b/Documentation/media/uapi/v4l/v4l2.rst index c0859ebc88ee..6d23bc987f51 100644 --- a/Documentation/media/uapi/v4l/v4l2.rst +++ b/Documentation/media/uapi/v4l/v4l2.rst @@ -85,6 +85,11 @@ part can be used and distributed without restrictions. Revision History +:revisio
[PATCH v3 9/9] [media] vivid: Local optimization
Avoid duplicated data shifts when possible. Signed-off-by: Ricardo Ribalda Delgado --- drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c index 1c862465e335..2c23c458b1a6 100644 --- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c +++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c @@ -919,13 +919,14 @@ static void precalculate_color(struct tpg_data *tpg, int k) color_to_ycbcr(tpg, r, g, b, &y, &cb, &cr); if (tpg->real_quantization == V4L2_QUANTIZATION_LIM_RANGE) { - y = clamp(y, 16 << 4, 235 << 4); - cb = clamp(cb, 16 << 4, 240 << 4); - cr = clamp(cr, 16 << 4, 240 << 4); + y = clamp(y >> 4, 16, 235); + cb = clamp(cb >> 4, 16, 240); + cr = clamp(cr > 4, 16, 240); + } else { + y = clamp(y >> 4, 1, 254); + cb = clamp(cb >> 4, 1, 254); + cr = clamp(cr >> 4, 1, 254); } - y = clamp(y >> 4, 1, 254); - cb = clamp(cb >> 4, 1, 254); - cr = clamp(cr >> 4, 1, 254); switch (tpg->fourcc) { case V4L2_PIX_FMT_YUV444: y >>= 4; -- 2.8.1
[PATCH v3 8/9] [media] vivid: Fix YUV555 and YUV565 handling
precalculate_color() had a optimization that avoided duplicated conversion for YUV formats. This optimization did not take into consideration YUV444, YUV555, YUV565 or limited range quantization. This patch keeps the optimization, but fixes the wrong handling. Signed-off-by: Ricardo Ribalda Delgado --- drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 19 --- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c index e91bf3cbaab9..1c862465e335 100644 --- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c +++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c @@ -797,6 +797,8 @@ static void precalculate_color(struct tpg_data *tpg, int k) int r = tpg_colors[col].r; int g = tpg_colors[col].g; int b = tpg_colors[col].b; + int y, cb, cr; + bool ycbbr_valid = false; if (k == TPG_COLOR_TEXTBG) { col = tpg_get_textbg_color(tpg); @@ -873,7 +875,6 @@ static void precalculate_color(struct tpg_data *tpg, int k) tpg->saturation != 128 || tpg->hue) && tpg->color_enc != TGP_COLOR_ENC_LUMA) { /* Implement these operations */ - int y, cb, cr; int tmp_cb, tmp_cr; /* First convert to YCbCr */ @@ -890,13 +891,10 @@ static void precalculate_color(struct tpg_data *tpg, int k) cb = (128 << 4) + (tmp_cb * tpg->contrast * tpg->saturation) / (128 * 128); cr = (128 << 4) + (tmp_cr * tpg->contrast * tpg->saturation) / (128 * 128); - if (tpg->color_enc == TGP_COLOR_ENC_YUV) { - tpg->colors[k][0] = clamp(y >> 4, 1, 254); - tpg->colors[k][1] = clamp(cb >> 4, 1, 254); - tpg->colors[k][2] = clamp(cr >> 4, 1, 254); - return; - } - ycbcr_to_color(tpg, y, cb, cr, &r, &g, &b); + if (tpg->color_enc == TGP_COLOR_ENC_YUV) + ycbbr_valid = true; + else + ycbcr_to_color(tpg, y, cb, cr, &r, &g, &b); } else if ((tpg->brightness != 128 || tpg->contrast != 128) && tpg->color_enc == TGP_COLOR_ENC_LUMA) { r = (16 << 4) + ((r - (16 << 4)) * tpg->contrast) / 128; @@ -917,9 +915,8 @@ static void precalculate_color(struct tpg_data *tpg, int k) case TGP_COLOR_ENC_YUV: { /* Convert to YCbCr */ - int y, cb, cr; - - color_to_ycbcr(tpg, r, g, b, &y, &cb, &cr); + if (!ycbbr_valid) + color_to_ycbcr(tpg, r, g, b, &y, &cb, &cr); if (tpg->real_quantization == V4L2_QUANTIZATION_LIM_RANGE) { y = clamp(y, 16 << 4, 235 << 4); -- 2.8.1
Re: [PATCH -v4 2/2] printk: Add kernel parameter to control writes to /dev/kmsg
On 07/15/16 at 02:45pm, Borislav Petkov wrote: > On Fri, Jul 15, 2016 at 02:21:09PM +0800, Dave Young wrote: > > Sorry for jumping in late. But I just see this today and I really like the > > idea to add a switch to turn off the kmsg writing from userspace because > > I suffer from it also. > > Thanks, I'll send v5 soon. > > > I may missed the background, what is the reason for "ratelimit"? It sounds > > a little odd. I think use default "on" should be safe to keep same behavior > > as before. > > Yeah, so we want to ratelimit by default so that excessive logging > doesn't disturb system operation. People who want to see all that > blubber can do so with "on" so they should be taken care of too. Ratelimit the writing to kmsg sounds not a good way, the ratelimit use cases are for "callbacks" being mentioned in other thread. Basiclly specific printk from same line of a function will be suppressed, that means the supressed messages are somehow same error/message/warnings. But different writings to /dev/msg could be from different userspace software so that the messages being supressed could be different and the *ratelimit* is not fine-grained as *ratelimt* supposed to be. Mis-ratelimit cause critical userspace messages being lost, that is worse than use off as default. Suppose we turn off devkmsg by default distributions can still turn on it with sysctl and for us who do not want the flooding we can use printk.devkmsg=off in kernel cmdline to override it. Of course if we turn off it by default we can print a warning to alert user. BTW, for userspace messages maybe they should not go to same log buffer, maybe a separate log buffer for /dev/msg will be better. Thanks Dave
[PATCH v3 4/9] [media] vivid: code refactor for color encoding
Replace is_yuv with color_enc Which can be used by other color encodings such us HSV. This change should ease the review of the following patches. Signed-off-by: Ricardo Ribalda Delgado --- drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 49 +++ drivers/media/platform/vivid/vivid-core.h | 2 +- drivers/media/platform/vivid/vivid-vid-common.c | 52 - include/media/v4l2-tpg.h| 7 +++- 4 files changed, 66 insertions(+), 44 deletions(-) diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c index 3ec3cebe62b9..e8d2bf388597 100644 --- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c +++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c @@ -237,13 +237,13 @@ bool tpg_s_fourcc(struct tpg_data *tpg, u32 fourcc) case V4L2_PIX_FMT_GREY: case V4L2_PIX_FMT_Y16: case V4L2_PIX_FMT_Y16_BE: - tpg->is_yuv = false; + tpg->color_enc = TGP_COLOR_ENC_RGB; break; case V4L2_PIX_FMT_YUV444: case V4L2_PIX_FMT_YUV555: case V4L2_PIX_FMT_YUV565: case V4L2_PIX_FMT_YUV32: - tpg->is_yuv = true; + tpg->color_enc = TGP_COLOR_ENC_YUV; break; case V4L2_PIX_FMT_YUV420M: case V4L2_PIX_FMT_YVU420M: @@ -256,7 +256,7 @@ bool tpg_s_fourcc(struct tpg_data *tpg, u32 fourcc) tpg->hdownsampling[1] = 2; tpg->hdownsampling[2] = 2; tpg->planes = 3; - tpg->is_yuv = true; + tpg->color_enc = TGP_COLOR_ENC_YUV; break; case V4L2_PIX_FMT_YUV422M: case V4L2_PIX_FMT_YVU422M: @@ -268,7 +268,7 @@ bool tpg_s_fourcc(struct tpg_data *tpg, u32 fourcc) tpg->hdownsampling[1] = 2; tpg->hdownsampling[2] = 2; tpg->planes = 3; - tpg->is_yuv = true; + tpg->color_enc = TGP_COLOR_ENC_YUV; break; case V4L2_PIX_FMT_NV16M: case V4L2_PIX_FMT_NV61M: @@ -280,7 +280,7 @@ bool tpg_s_fourcc(struct tpg_data *tpg, u32 fourcc) tpg->hdownsampling[1] = 1; tpg->hmask[1] = ~1; tpg->planes = 2; - tpg->is_yuv = true; + tpg->color_enc = TGP_COLOR_ENC_YUV; break; case V4L2_PIX_FMT_NV12M: case V4L2_PIX_FMT_NV21M: @@ -292,7 +292,7 @@ bool tpg_s_fourcc(struct tpg_data *tpg, u32 fourcc) tpg->hdownsampling[1] = 1; tpg->hmask[1] = ~1; tpg->planes = 2; - tpg->is_yuv = true; + tpg->color_enc = TGP_COLOR_ENC_YUV; break; case V4L2_PIX_FMT_YUV444M: case V4L2_PIX_FMT_YVU444M: @@ -302,21 +302,21 @@ bool tpg_s_fourcc(struct tpg_data *tpg, u32 fourcc) tpg->vdownsampling[2] = 1; tpg->hdownsampling[1] = 1; tpg->hdownsampling[2] = 1; - tpg->is_yuv = true; + tpg->color_enc = TGP_COLOR_ENC_YUV; break; case V4L2_PIX_FMT_NV24: case V4L2_PIX_FMT_NV42: tpg->vdownsampling[1] = 1; tpg->hdownsampling[1] = 1; tpg->planes = 2; - tpg->is_yuv = true; + tpg->color_enc = TGP_COLOR_ENC_YUV; break; case V4L2_PIX_FMT_YUYV: case V4L2_PIX_FMT_UYVY: case V4L2_PIX_FMT_YVYU: case V4L2_PIX_FMT_VYUY: tpg->hmask[0] = ~1; - tpg->is_yuv = true; + tpg->color_enc = TGP_COLOR_ENC_YUV; break; default: return false; @@ -777,7 +777,8 @@ static void precalculate_color(struct tpg_data *tpg, int k) * Remember that r, g and b are still in the 0 - 0xff0 range. */ if (tpg->real_rgb_range == V4L2_DV_RGB_RANGE_LIMITED && - tpg->rgb_range == V4L2_DV_RGB_RANGE_FULL && !tpg->is_yuv) { + tpg->rgb_range == V4L2_DV_RGB_RANGE_FULL && + tpg->color_enc == TGP_COLOR_ENC_RGB) { /* * Convert from full range (which is what r, g and b are) * to limited range (which is the 'real' RGB range), which @@ -787,7 +788,9 @@ static void precalculate_color(struct tpg_data *tpg, int k) g = (g * 219) / 255 + (16 << 4); b = (b * 219) / 255 + (16 << 4); } else if (tpg->real_rgb_range != V4L2_DV_RGB_RANGE_LIMITED && - tpg->rgb_range == V4L2_DV_RGB_RANGE_LIMITED && !tpg->is_yuv) { + tpg->rgb_range == V4L2_DV_RGB_RANGE_LIMITED && + tpg->color_enc == TGP_COLOR_ENC_RGB) { + /* * Clamp r, g and b to the limited range and convert to full * range since that's what we deliver. @@ -820,7 +823,7 @@ static void precalculate_color
[PATCH v3 5/9] [media] vivid: Add support for HSV formats
This patch adds support for V4L2_PIX_FMT_HSV24 and V4L2_PIX_FMT_HSV32. Signed-off-by: Ricardo Ribalda Delgado --- drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 93 +++-- drivers/media/platform/vivid/vivid-vid-common.c | 14 include/media/v4l2-tpg.h| 1 + 3 files changed, 104 insertions(+), 4 deletions(-) diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c index e8d2bf388597..53e512f5a6b6 100644 --- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c +++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c @@ -318,6 +318,10 @@ bool tpg_s_fourcc(struct tpg_data *tpg, u32 fourcc) tpg->hmask[0] = ~1; tpg->color_enc = TGP_COLOR_ENC_YUV; break; + case V4L2_PIX_FMT_HSV24: + case V4L2_PIX_FMT_HSV32: + tpg->color_enc = TGP_COLOR_ENC_HSV; + break; default: return false; } @@ -351,6 +355,7 @@ bool tpg_s_fourcc(struct tpg_data *tpg, u32 fourcc) break; case V4L2_PIX_FMT_RGB24: case V4L2_PIX_FMT_BGR24: + case V4L2_PIX_FMT_HSV24: tpg->twopixelsize[0] = 2 * 3; break; case V4L2_PIX_FMT_BGR666: @@ -361,6 +366,7 @@ bool tpg_s_fourcc(struct tpg_data *tpg, u32 fourcc) case V4L2_PIX_FMT_ARGB32: case V4L2_PIX_FMT_ABGR32: case V4L2_PIX_FMT_YUV32: + case V4L2_PIX_FMT_HSV32: tpg->twopixelsize[0] = 2 * 4; break; case V4L2_PIX_FMT_NV12: @@ -490,6 +496,64 @@ static inline int linear_to_rec709(int v) return tpg_linear_to_rec709[v]; } +static void color_to_hsv(struct tpg_data *tpg, int r, int g, int b, + int *h, int *s, int *v) +{ + int max_rgb, min_rgb, diff_rgb; + int aux; + int third; + + r >>= 4; + g >>= 4; + b >>= 4; + + /* Value */ + max_rgb = max3(r, g, b); + *v = max_rgb; + if (!max_rgb) { + *h = 0; + *s = 0; + return; + } + + /* Saturation */ + min_rgb = min3(r, g, b); + diff_rgb = max_rgb - min_rgb; + aux = 255 * diff_rgb; + aux += max_rgb / 2; + aux /= max_rgb; + *s = aux; + if (!aux) { + *h = 0; + return; + } + + /* Hue */ + if (max_rgb == r) { + aux = g - b; + third = 0; + } else if (max_rgb == g) { + aux = b - r; + third = 60; + } else { + aux = r - g; + third = 120; + } + + aux *= 30; + aux += diff_rgb / 2; + aux /= diff_rgb; + aux += third; + + /* Clamp Hue */ + if (aux < 0) + aux += 180; + else if (aux > 180) + aux -= 180; + *h = aux; + +} + static void rgb2ycbcr(const int m[3][3], int r, int g, int b, int y_offset, int *y, int *cb, int *cr) { @@ -832,7 +896,19 @@ static void precalculate_color(struct tpg_data *tpg, int k) ycbcr_to_color(tpg, y, cb, cr, &r, &g, &b); } - if (tpg->color_enc == TGP_COLOR_ENC_YUV) { + switch (tpg->color_enc) { + case TGP_COLOR_ENC_HSV: + { + int h, s, v; + + color_to_hsv(tpg, r, g, b, &h, &s, &v); + tpg->colors[k][0] = h; + tpg->colors[k][1] = s; + tpg->colors[k][2] = v; + break; + } + case TGP_COLOR_ENC_YUV: + { /* Convert to YCbCr */ int y, cb, cr; @@ -866,7 +942,10 @@ static void precalculate_color(struct tpg_data *tpg, int k) tpg->colors[k][0] = y; tpg->colors[k][1] = cb; tpg->colors[k][2] = cr; - } else { + break; + } + case TGP_COLOR_ENC_RGB: + { if (tpg->real_quantization == V4L2_QUANTIZATION_LIM_RANGE) { r = (r * 219) / 255 + (16 << 4); g = (g * 219) / 255 + (16 << 4); @@ -916,6 +995,8 @@ static void precalculate_color(struct tpg_data *tpg, int k) tpg->colors[k][0] = r; tpg->colors[k][1] = g; tpg->colors[k][2] = b; + break; + } } } @@ -941,8 +1022,8 @@ static void gen_twopix(struct tpg_data *tpg, alpha = 0; if (color == TPG_COLOR_RANDOM) precalculate_color(tpg, color); - r_y = tpg->colors[color][0]; /* R or precalculated Y */ - g_u = tpg->colors[color][1]; /* G or precalculated U */ + r_y = tpg->colors[color][0]; /* R or precalculated Y, H */ + g_u = tpg->colors[color][1]; /* G or precalculated U, V */ b_v = tpg->colors[color][2]; /* B or precalculated V */ switch (tpg->fourcc) { @@ -1124,6 +1
Re: [PATCH] [media] ad9389b: Remove deprecated create_singlethread_workqueue
Hi, [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.7-rc7 next-20160715] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Bhaktipriya-Shridhar/ad9389b-Remove-deprecated-create_singlethread_workqueue/20160716-174501 base: git://linuxtv.org/media_tree.git master config: sparc64-allmodconfig (attached as .config) compiler: sparc64-linux-gnu-gcc (Debian 5.3.1-8) 5.3.1 20160205 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=sparc64 All warnings (new ones prefixed by >>): drivers/media/i2c/ad9389b.c: In function 'ad9389b_probe': >> drivers/media/i2c/ad9389b.c:1179:1: warning: label 'err_unreg' defined but >> not used [-Wunused-label] err_unreg: ^ vim +/err_unreg +1179 drivers/media/i2c/ad9389b.c 117a55b6 Hans Verkuil 2012-07-18 1163 if (state->edid_i2c_client == NULL) { 117a55b6 Hans Verkuil 2012-07-18 1164 v4l2_err(sd, "failed to register edid i2c client\n"); 6ec735df Wei Yongjun 2013-05-13 1165 err = -ENOMEM; 117a55b6 Hans Verkuil 2012-07-18 1166 goto err_entity; 117a55b6 Hans Verkuil 2012-07-18 1167 } 117a55b6 Hans Verkuil 2012-07-18 1168 117a55b6 Hans Verkuil 2012-07-18 1169 INIT_DELAYED_WORK(&state->edid_handler, ad9389b_edid_handler); 117a55b6 Hans Verkuil 2012-07-18 1170 state->dv_timings = dv1080p60; 117a55b6 Hans Verkuil 2012-07-18 1171 117a55b6 Hans Verkuil 2012-07-18 1172 ad9389b_init_setup(sd); 117a55b6 Hans Verkuil 2012-07-18 1173 ad9389b_set_isr(sd, true); 117a55b6 Hans Verkuil 2012-07-18 1174 117a55b6 Hans Verkuil 2012-07-18 1175 v4l2_info(sd, "%s found @ 0x%x (%s)\n", client->name, 117a55b6 Hans Verkuil 2012-07-18 1176client->addr << 1, client->adapter->name); 117a55b6 Hans Verkuil 2012-07-18 1177 return 0; 117a55b6 Hans Verkuil 2012-07-18 1178 117a55b6 Hans Verkuil 2012-07-18 @1179 err_unreg: 117a55b6 Hans Verkuil 2012-07-18 1180 i2c_unregister_device(state->edid_i2c_client); 117a55b6 Hans Verkuil 2012-07-18 1181 err_entity: 117a55b6 Hans Verkuil 2012-07-18 1182 media_entity_cleanup(&sd->entity); 117a55b6 Hans Verkuil 2012-07-18 1183 err_hdl: 117a55b6 Hans Verkuil 2012-07-18 1184 v4l2_ctrl_handler_free(&state->hdl); 117a55b6 Hans Verkuil 2012-07-18 1185 return err; 117a55b6 Hans Verkuil 2012-07-18 1186 } 117a55b6 Hans Verkuil 2012-07-18 1187 :: The code at line 1179 was first introduced by commit :: 117a55b69d36a19028d1c59a737ad1246a0a75ad [media] ad9389b: driver for the Analog Devices AD9389B video encoder :: TO: Hans Verkuil :: CC: Mauro Carvalho Chehab --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [PATCH v3 9/9] [media] vivid: Local optimization
Hi On Sat, Jul 16, 2016 at 12:41 PM, Ricardo Ribalda Delgado wrote: > - cr = clamp(cr, 16 << 4, 240 << 4); > + y = clamp(y >> 4, 16, 235); > + cb = clamp(cb >> 4, 16, 240); > + cr = clamp(cr > 4, 16, 240); This line is obviously wrong, sorry about that. I wait for some more comments and add it to v4. Regards!
Re: [PATCH net] net: nb8800: Fix SKB leak in nb8800_receive()
Florian Fainelli writes: > In case nb8800_receive() fails to allocate a fragment, we would leak the > SKB freshly allocated and just return, instead, free it. > > Reported-by: coverity (CID 1341750) > Signed-off-by: Florian Fainelli Acked-by: Mans Rullgard > --- > drivers/net/ethernet/aurora/nb8800.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ethernet/aurora/nb8800.c > b/drivers/net/ethernet/aurora/nb8800.c > index 08a23e6b60e9..1a3555d03a96 100644 > --- a/drivers/net/ethernet/aurora/nb8800.c > +++ b/drivers/net/ethernet/aurora/nb8800.c > @@ -259,6 +259,7 @@ static void nb8800_receive(struct net_device *dev, > unsigned int i, > if (err) { > netdev_err(dev, "rx buffer allocation failed\n"); > dev->stats.rx_dropped++; > + dev_kfree_skb(skb); > return; > } > > -- > 2.7.4 > -- Måns Rullgård
[PATCH v2] [media] ad9389b: Remove deprecated create_singlethread_workqueue
The workqueue work_queue is involved in EDID (Extended Display Identification Data) handling. It has a single work item(&state->edid_handler) and hence doesn't require ordering. It is not being used on a memory reclaim path. Hence, the singlethreaded workqueue has been replaced with the use of system_wq. &state->edid_handler is a self requeueing work item and it has been been sync cancelled in ad9389b_remove() to ensure that nothing is pending when the driver is disconnected. The unused label err_unreg has also been dropped. Signed-off-by: Bhaktipriya Shridhar --- Changes in v2: -Fixes kbuild warning: label 'err_unreg' defined but not used. drivers/media/i2c/ad9389b.c | 22 -- 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/drivers/media/i2c/ad9389b.c b/drivers/media/i2c/ad9389b.c index 0462f46..5fd2350 100644 --- a/drivers/media/i2c/ad9389b.c +++ b/drivers/media/i2c/ad9389b.c @@ -98,7 +98,6 @@ struct ad9389b_state { struct ad9389b_state_edid edid; /* Running counter of the number of detected EDIDs (for debugging) */ unsigned edid_detect_counter; - struct workqueue_struct *work_queue; struct delayed_work edid_handler; /* work entry */ }; @@ -843,8 +842,7 @@ static void ad9389b_edid_handler(struct work_struct *work) v4l2_dbg(1, debug, sd, "%s: edid read failed\n", __func__); ad9389b_s_power(sd, false); ad9389b_s_power(sd, true); - queue_delayed_work(state->work_queue, - &state->edid_handler, EDID_DELAY); + schedule_delayed_work(&state->edid_handler, EDID_DELAY); return; } } @@ -933,8 +931,7 @@ static void ad9389b_update_monitor_present_status(struct v4l2_subdev *sd) ad9389b_setup(sd); ad9389b_notify_monitor_detect(sd); state->edid.read_retries = EDID_MAX_RETRIES; - queue_delayed_work(state->work_queue, - &state->edid_handler, EDID_DELAY); + schedule_delayed_work(&state->edid_handler, EDID_DELAY); } else if (!(status & MASK_AD9389B_HPD_DETECT)) { v4l2_dbg(1, debug, sd, "%s: hotplug not detected\n", __func__); state->have_monitor = false; @@ -1065,8 +1062,7 @@ static bool ad9389b_check_edid_status(struct v4l2_subdev *sd) ad9389b_wr(sd, 0xc9, 0xf); ad9389b_wr(sd, 0xc4, state->edid.segments); state->edid.read_retries = EDID_MAX_RETRIES; - queue_delayed_work(state->work_queue, - &state->edid_handler, EDID_DELAY); + schedule_delayed_work(&state->edid_handler, EDID_DELAY); return false; } @@ -1170,13 +1166,6 @@ static int ad9389b_probe(struct i2c_client *client, const struct i2c_device_id * goto err_entity; } - state->work_queue = create_singlethread_workqueue(sd->name); - if (state->work_queue == NULL) { - v4l2_err(sd, "could not create workqueue\n"); - err = -ENOMEM; - goto err_unreg; - } - INIT_DELAYED_WORK(&state->edid_handler, ad9389b_edid_handler); state->dv_timings = dv1080p60; @@ -1187,8 +1176,6 @@ static int ad9389b_probe(struct i2c_client *client, const struct i2c_device_id * client->addr << 1, client->adapter->name); return 0; -err_unreg: - i2c_unregister_device(state->edid_i2c_client); err_entity: media_entity_cleanup(&sd->entity); err_hdl: @@ -1211,9 +1198,8 @@ static int ad9389b_remove(struct i2c_client *client) ad9389b_s_stream(sd, false); ad9389b_s_audio_stream(sd, false); ad9389b_init_setup(sd); - cancel_delayed_work(&state->edid_handler); + cancel_delayed_work_sync(&state->edid_handler); i2c_unregister_device(state->edid_i2c_client); - destroy_workqueue(state->work_queue); v4l2_device_unregister_subdev(sd); media_entity_cleanup(&sd->entity); v4l2_ctrl_handler_free(sd->ctrl_handler); -- 2.1.4
Re: [RFC][PATCH 0/7] Add HDMI audio support for HiKey
On Fri, Jul 15, 2016 at 08:38:47PM -0700, John Stultz wrote: > Yea. I'm not sure what the communities policy on Author/SoB lines in > the face of email address changes. Given that the signoff is all about DCO and hence licensing it should probably stay with Linaro. signature.asc Description: PGP signature
Re: [RFC][PATCH 5/7] Kconfig: Allow k3dma driver to be selected for more then HISI3xx platforms
On Fri, Jul 15, 2016 at 07:13:25PM -0700, John Stultz wrote: > This allows the k3dma driver to be selected on HiKey > config K3_DMA > tristate "Hisilicon K3 DMA support" > - depends on ARCH_HI3xxx > select DMA_ENGINE > select DMA_VIRTUAL_CHANNELS > help And *every* other platform ever. You probably want ARCH_HI3xxx || ARCH_HISI || COMPILE_TEST here. signature.asc Description: PGP signature
Re: [Nbd] [PATCH 2/2] nbd: Disallow ioctls on disconnected block device
On Sat, Jul 16, 2016 at 03:38:40PM +0530, Pranay Srivastava wrote: > Okay. So how about we include some negotiated key which goes in with every > request which the server could maintain for clients that can be checked while > resetting the connection with the same server? Wut? > So am I correct that this situation can > indeed happen or the server will throw an error back to client in case > the troubled > nbd-client is trying to reconnect to the original server but requests > are going to > another server? > > If yes to above query then what is the best effort we can do to avoid > such scenarios? Tell userspace not to do stupid things? This isn't a problem. The kernel assumes that whatever userspace does, once the connection is set up again everything's the way it was before. If that's not true, then userspace is to blame, not kernel space. Adding a "key" which we need to pass is going to make things wildly more complicated for no benefit. -- < ron> I mean, the main *practical* problem with C++, is there's like a dozen people in the world who think they really understand all of its rules, and pretty much all of them are just lying to themselves too. -- #debian-devel, OFTC, 2016-02-12
[PATCH v2] drm/radeon: Remove deprecated create_singlethread_workqueue
alloc_workqueue replaces deprecated create_singlethread_workqueue(). Each hardware CRTC has a single flip work queue. When a radeon_flip_work_func item is queued, it needs to be executed ASAP because even a slight delay may cause the flip to be delayed by one refresh cycle. Hence, a dedicated workqueue with WQ_HIGHPRI set, has been used here since a delay can cause the outcome to miss the refresh cycle. Since there are only a fixed number of work items, explicit concurrency limit is unnecessary here. Signed-off-by: Bhaktipriya Shridhar --- Changes in v2: -Used a dedicated work queue with WQ_HIGHPRI drivers/gpu/drm/radeon/radeon_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index 6a41b49..64b246e 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -711,7 +711,7 @@ static void radeon_crtc_init(struct drm_device *dev, int index) drm_mode_crtc_set_gamma_size(&radeon_crtc->base, 256); radeon_crtc->crtc_id = index; - radeon_crtc->flip_queue = create_singlethread_workqueue("radeon-crtc"); + radeon_crtc->flip_queue = alloc_workqueue("radeon-crtc", WQ_HIGHPRI, 0); rdev->mode_info.crtcs[index] = radeon_crtc; if (rdev->family >= CHIP_BONAIRE) { -- 2.1.4
Re: [RFC][PATCH 6/7] ASoC: hisilicon: Add hi6210 i2s audio driver for hdmi audio
On Fri, Jul 15, 2016 at 07:13:26PM -0700, John Stultz wrote: > sound/soc/Kconfig | 1 + > sound/soc/Makefile | 1 + > sound/soc/hisilicon/Kconfig| 5 + > sound/soc/hisilicon/Makefile | 2 + > sound/soc/hisilicon/hi6210-hdmi-card.c | 131 +++ > sound/soc/hisilicon/hi6210-i2s.c | 641 > + > sound/soc/hisilicon/hi6210-i2s.h | 276 ++ > 7 files changed, 1057 insertions(+) This is adding several different drivers so should be at least one patch per driver. Combining many different drivers into one patch makes it more difficult to review as the patch gets larger and more fatiguing and causes review problems in one driver to block others. Look at how existing drivers are added to the same subsystem. > +static int hdmi_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params) > +{ > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; > + int ret; > + > + ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S | > + SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS); > + if (ret) > + return ret; > + > + /* set i2s system clock */ > + ret = snd_soc_dai_set_sysclk(cpu_dai, 0, 24576000, SND_SOC_CLOCK_IN); > + if (ret) > + return ret; Neither of these depends at all on the parameters so they should be being set on init, not at hw_params time. In the case of the DAI format this should be done with dai_fmt in the dai_link. > + .cpu_dai_name = "f7118000.hi6210_i2s", Why is this not connected up in DT - as things stand the card has zero reuse potential. Though given that this has > + .codec_name = "0.hi6210_hdmi_card", If you've got a CODEC with hdmi_card in the name that suggests there's a serious abstraction problem going on somewhere. > +static void hi6210_bits(struct hi6210_i2s *i2s, u32 ofs, u32 reset, u32 set) > +{ > + u32 val = readl(i2s->base + ofs) & ~reset; > + > + writel(val | set, i2s->base + ofs); > +} This isn't the usual pattern for read/modify/write operations in the kernel and doesn't seem like it's going to be great for legibility - the reader has to remember which order clear and set are and the name doesn't give many clues. > + hi6210_bits(i2s, HII2S_DIG_FILTER_MODULE_CFG, > + HII2S_DIG_FILTER_MODULE_CFG__DACR_MIXER_IN2_MUTE | > + HII2S_DIG_FILTER_MODULE_CFG__DACL_MIXER_IN2_MUTE, > + 0 > + ); Coding style, the final ); is just weird. These register names make this look like there's some routing control going on here which should be being exposed to users. > + for (n = 0; n < i2s->clocks; n++) { > + ret = clk_prepare_enable(i2s->clk[n]); > + if (ret) > + return ret; > + } This won't unwind things on error. > + ret = clk_set_rate(i2s->clk[1], 49152000); > + if (ret) { > + dev_err(i2s->dev, "%s: setting 49.152MHz base rate failed %d\n", > + __func__, ret); > + return ret; > + } Magic array index there isn't great. > +static void hi6210_i2s_txctrl(struct snd_soc_dai *cpu_dai, int on) > +{ > + struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev); > + > + spin_lock(&i2s->lock); > + > + if (on) { > + /* enable S2 TX */ > + hi6210_bits(i2s, HII2S_I2S_CFG, 0, HII2S_I2S_CFG__S2_IF_TX_EN); > + } else > + /* disable S2 TX */ > + hi6210_bits(i2s, HII2S_I2S_CFG, HII2S_I2S_CFG__S2_IF_TX_EN, 0); Coding style, { } on both side or neither :( > +static int hi6210_i2s_set_sysclk(struct snd_soc_dai *cpu_dai, > + int clk_id, unsigned int freq, int dir) > +{ > + return 0; > +} Remove empty operations, if the operation can reasonably be ignored the frameworks will handle that well. > +static int hi6210_i2s_set_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) > +{ > + struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev); > + > + i2s->format = fmt; > + i2s->master = (i2s->format & SND_SOC_DAIFMT_MASTER_MASK) == > + SND_SOC_DAIFMT_CBS_CFS; We're validating this elsewhere rather than when the user sets it. > + switch (i2s->rate) { > + case 8000: > + u = HII2S_FS_RATE_8KHZ; > + break; > + case 16000: > + u = HII2S_FS_RATE_16KHZ; > + break; > + case 32000: > + u = HII2S_FS_RATE_32KHZ; > + break; > + case 48000: > + u = HII2S_FS_RATE_48KHZ; > + break; > + case 96000: > + u = HII2S_FS_RATE_96KHZ; > + break; > + case 192000: > + u = HII2S_FS_RATE_192KHZ; > + break; > + }; Should have validation on the default case. I'm not s
Re: [PATCH 7/7] dts: hi6220: Add k3-dma and i2s/hdmi audio support
On Fri, Jul 15, 2016 at 07:13:27PM -0700, John Stultz wrote: > Add entry for k3-dma driver and i2s/hdmi audio devices. > This enables HDMI audio output. These bindings appear to be undocumented. All new bindings require documentation. > + i2s0: hi6210_i2s { > + compatible = "hisilicon,hi6210-i2s"; > + reg = <0x0 0xf7118000 0x0 0x8000>, /* i2s unit */ > + <0x0 0xf703 0x0 0x400>, /* syscon */ > + <0x0 0xf7032000 0x0 0x400>; /* pmctrl */ Some of this looks like we should be using a system controller binding rather than listing the system controller directly here. The magic number indexing for the resources is also not good, we should be using reg-names to get name based lookups (I had been going to query that on the binding document when I failed to find it...). signature.asc Description: PGP signature
Re: [PATCH] lib/bitmap.c: enhance map pattern
On Sat, 2016-07-16 at 10:57 +0300, Noam Camus wrote: > From: Noam Camus > > Today there are platforms with many CPUs (up to 4K). > Trying to boot only part of the CPUs may result in too long string. > > For example lets take NPS platform that is part of arch/arc. > This platform have SMP system with 256 cores each with > 16 HW threads (SMT machine) where HW thread appears as CPU to the kernel. > In this example there is total of 4K CPUs. > When one tries to boot only part of the HW threads from each core the > string representing the map may be long... > For example if for sake of performance we decided to boot only first half > of HW threads of each core the map will look like: > 0-7,16-23,32,39,...,4080-4087 > > This patch introduce new format to accommodate with such use case. > I added an optional postfix to a range of CPUs which will choose > according to given modulo the desired range of reminders i.e.: > %modulo= This is a fairly awful syntax, and I question whether it belongs in this generic code. [...] > + * Optionally each range can be postfixed to denote that only parts of it > + * should be set. The parts are the range of reminders modulo some value. > + * i.e. range%mod=rem_range e.g. 0-1023%256=0-2 ==> > 0,1,256,257,512,513,768,769 [...] This example seems to be wrong - did you mean '=0-1'? Ben. -- Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse. signature.asc Description: This is a digitally signed message part
Re: [PATCH] [media] vb2: map dmabuf for planes on driver queue instead of vidioc_qbuf
On 15/07/16 17:26, Javier Martinez Canillas wrote: > The buffer planes' dma-buf are currently mapped when buffers are queued > from userspace but it's more appropriate to do the mapping when buffers > are queued in the driver since that's when the actual DMA operation are > going to happen. > > Suggested-by: Nicolas Dufresne > Signed-off-by: Javier Martinez Canillas > > --- > > Hello, > > A side effect of this change is that if the dmabuf map fails for some > reasons (i.e: a driver using the DMA contig memory allocator but CMA > not being enabled), the fail will no longer happen on VIDIOC_QBUF but > later (i.e: in VIDIOC_STREAMON). > > I don't know if that's an issue though but I think is worth mentioning. > > Best regards, > Javier > Just run this path on the ODROID using GStreamer and the vivid driver. It worked nicely. Tested-by: Luis de Bethencourt Thanks Javier, Luis
[patch v2] tools/vm/slabinfo: fix an unintentional printf
The curly braces are missing here so we print stuff unintentionally. Fixes: 9da4714a2d44 ('slub: slabinfo update for cmpxchg handling') Signed-off-by: Dan Carpenter --- v2: Fix typo in git hash diff --git a/tools/vm/slabinfo.c b/tools/vm/slabinfo.c index 7cf6e17..b9d34b3 100644 --- a/tools/vm/slabinfo.c +++ b/tools/vm/slabinfo.c @@ -510,10 +510,11 @@ static void slab_stats(struct slabinfo *s) s->alloc_node_mismatch, (s->alloc_node_mismatch * 100) / total); } - if (s->cmpxchg_double_fail || s->cmpxchg_double_cpu_fail) + if (s->cmpxchg_double_fail || s->cmpxchg_double_cpu_fail) { printf("\nCmpxchg_double Looping\n\n"); printf("Locked Cmpxchg Double redos %lu\nUnlocked Cmpxchg Double redos %lu\n", s->cmpxchg_double_fail, s->cmpxchg_double_cpu_fail); + } } static void report(struct slabinfo *s) -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 4/9] acpi/arm64: Add GTDT table parse driver
On Saturday, July 16, 2016 10:24:35 AM Fu Wei wrote: > Hi Rafeal, > > On 16 July 2016 at 05:22, Rafael J. Wysocki wrote: > > On Saturday, July 16, 2016 12:32:14 AM Fu Wei wrote: > >> Hi Rafael, > >> > >> On 15 July 2016 at 21:07, Rafael J. Wysocki wrote: > >> > On Friday, July 15, 2016 02:15:27 PM Rafael J. Wysocki wrote: > >> >> On Friday, July 15, 2016 03:32:35 PM Fu Wei wrote: > >> >> > Hi Rafael, > >> >> > > >> > > >> > [cut] > >> > > >> >> > > > >> >> > >> + return 0; > >> >> > >> + } > >> >> > >> + > >> >> > >> + if (!gtdt->platform_timer_count) { > >> >> > >> + pr_info("No Platform Timer.\n"); > >> >> > >> + return 0; > >> >> > >> + } > >> >> > >> + > >> >> > >> + acpi_gtdt_desc.platform_timer_start = (void *)gtdt + > >> >> > >> + > >> >> > >> gtdt->platform_timer_offset; > >> >> > >> + if (acpi_gtdt_desc.platform_timer_start < > >> >> > >> + (void *)table + sizeof(struct acpi_table_gtdt)) { > >> >> > >> + pr_err(FW_BUG "Platform Timer pointer error.\n"); > >> >> > > > >> >> > > Why pr_err()? > >> >> > > >> >> > if (true), that means the GTDT table has bugs. > >> >> > > >> >> > >> >> And that's not a very useful piece of information unless you're > >> >> debugging the > >> >> platform, is it? > >> > > >> > FWIW, I'm not a big fan of printing "your firmware is buggy" type of > >> > messages > >> > (especially at the "error" log level or higher) unless they can be > >> > clearly > >> > connected to a specific type of functional failure. > >> > > >> > So if you want to pring an error-level message, something like "I cannot > >> > do X > >> > because of the firmware bug Y" would be better IMO. > >> > >> So can I do this: > >> pr_err(FW_BUG "Can NOT init platform_timer pointer, because of the > >> GTDT table bug\n"); > >> > >> or pr_debug(FW_BUG "Can NOT init platform_timer_start, because of > >> platform_timer_offset bug in GTDT\n"); > >> > >> or just delete it? > >> > >> which one do you prefer? I think maybe should provide some clue for > >> users to fix the problem :-) > > > > And how exactly would they fix it then? > > > >> > >> any thought ? > > > > If you print variable or function names and the like, the message should be > > a debug one, because that's information that can only be understood by > > developers (some developers are users too, but they are a minority). > > > > If you want to report an error, say what is not working (or not available > > etc) and why (if you know the reason at the time the message is printed). > > Great thanks, I guess I got you point. > > maybe just a very simple message like: > pr_err(FW_BUG "Failed to init table: GTDT table is buggy.\n"); To understand this message one needs to know what "table" means here and what "GTDT" is. Also the prefix already will be something like "ACPI: GTDT:", so repeating part of it is not really useful IMO. Can you tell me please what's not going to work when that message is printed? Will the system boot at all then? If so, the functionality will be limited somehow I suppose. How is it going to be limited? > I will also check other pr_* , if I can update them OK, great! Thanks, Rafael
Re: [PATCH] usb: gadget: uvc: Fix return value in case of error
Hi Christophe, Thank you for the patch. On Saturday 16 Jul 2016 09:04:40 Christophe JAILLET wrote: > If this memory allocation fail, we will return 0, which means success. > Return -ENOMEM instead. > > Signed-off-by: Christophe JAILLET Reviewed-by: Laurent Pinchart > --- > drivers/usb/gadget/function/uvc_configfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/function/uvc_configfs.c > b/drivers/usb/gadget/function/uvc_configfs.c index 66753ba..31125a4 100644 > --- a/drivers/usb/gadget/function/uvc_configfs.c > +++ b/drivers/usb/gadget/function/uvc_configfs.c > @@ -2023,7 +2023,7 @@ static int uvcg_streaming_class_allow_link(struct > config_item *src, if (!data) { > kfree(*class_array); > *class_array = NULL; > - ret = PTR_ERR(data); > + ret = -ENOMEM; > goto unlock; > } > cl_arr = *class_array; -- Regards, Laurent Pinchart
Re: [PATCH v2 2/6] [media] Documentation: Add HSV format
Hi Hans, On Saturday 16 Jul 2016 10:19:29 Hans Verkuil wrote: > On 07/15/2016 08:11 PM, Laurent Pinchart wrote: > > On Friday 15 Jul 2016 18:13:15 Ricardo Ribalda Delgado wrote: > >> Describe the HSV formats > >> > >> Signed-off-by: Ricardo Ribalda Delgado > >> --- > >> > >> Documentation/media/uapi/v4l/hsv-formats.rst | 19 ++ > >> Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst | 253 +++ > >> Documentation/media/uapi/v4l/pixfmt.rst| 1 + > >> Documentation/media/uapi/v4l/v4l2.rst | 5 + > >> 4 files changed, 278 insertions(+) > >> create mode 100644 Documentation/media/uapi/v4l/hsv-formats.rst > >> create mode 100644 Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst > >> > >> diff --git a/Documentation/media/uapi/v4l/hsv-formats.rst > >> b/Documentation/media/uapi/v4l/hsv-formats.rst new file mode 100644 > >> index ..f0f2615eaa95 > >> --- /dev/null > >> +++ b/Documentation/media/uapi/v4l/hsv-formats.rst > >> @@ -0,0 +1,19 @@ > >> +.. -*- coding: utf-8; mode: rst -*- > >> + > >> +.. _hsv-formats: > >> + > >> +*** > >> +HSV Formats > >> +*** > >> + > >> +These formats store the color information of the image > >> +in a geometrical representation. The colors are mapped into a > >> +cylinder, where the angle is the HUE, the height is the VALUE > >> +and the distance to the center is the SATURATION. This is a very > >> +useful format for image segmentation algorithms. > >> + > >> + > >> +.. toctree:: > >> +:maxdepth: 1 > >> + > >> +pixfmt-packed-hsv > >> diff --git a/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst > >> b/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst new file mode 100644 > >> index ..b297aa4f7ba6 > >> --- /dev/null > >> +++ b/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst > >> @@ -0,0 +1,253 @@ > >> +.. -*- coding: utf-8; mode: rst -*- > >> + > >> +.. _packed-hsv: > >> + > >> +** > >> +Packed HSV formats > >> +** > >> + > >> +*man Packed HSV formats(2)* > >> + > >> +Packed HSV formats > >> + > >> + > >> +Description > >> +=== > >> + > >> +The HUE (h) is meassured in degrees, one LSB represents two degrees. > > > > Is this common ? I have a device that can handle HSV data, I need to check > > how it maps the hue values to binary, but I'm pretty sure they cover the > > full 0-255 range. We would then have to support the two formats. Separate > > 4CCs are an option, but reporting the range separately (possibly through > > the colorspace API) might be better. Any thought on that ? > > It's either a separate 4cc or we do something with the ycbcr_enc field > (reinterpreted as hsv_enc). I'm not sure, I would have to think some more > about that. I'm inclined to use the ycbcr_enc field, especially given that a similar usage could be useful for RGB as well (don't ask me what it's supposed to be used for, but I have hardware that support limiting the RGB values range to 16-235). -- Regards, Laurent Pinchart
Re: [Nbd] [PATCH 2/2] nbd: Disallow ioctls on disconnected block device
On Sat, Jul 16, 2016 at 4:56 PM, Wouter Verhelst wrote: > On Sat, Jul 16, 2016 at 03:38:40PM +0530, Pranay Srivastava wrote: >> Okay. So how about we include some negotiated key which goes in with every >> request which the server could maintain for clients that can be checked while >> resetting the connection with the same server? > > Wut? > >> So am I correct that this situation can >> indeed happen or the server will throw an error back to client in case >> the troubled >> nbd-client is trying to reconnect to the original server but requests >> are going to >> another server? >> >> If yes to above query then what is the best effort we can do to avoid >> such scenarios? > > Tell userspace not to do stupid things? > > This isn't a problem. The kernel assumes that whatever userspace does, > once the connection is set up again everything's the way it was before. > If that's not true, then userspace is to blame, not kernel space. > > Adding a "key" which we need to pass is going to make things wildly more > complicated for no benefit. Okay. So let things roll for timeout but stop for disconnect. > > -- > < ron> I mean, the main *practical* problem with C++, is there's like a dozen >people in the world who think they really understand all of its rules, >and pretty much all of them are just lying to themselves too. > -- #debian-devel, OFTC, 2016-02-12 -- ---P.K.S
Re: [PATCH v5] wlcore: spi: add wl18xx support
On Sun, Jul 10, 2016 at 08:32:40AM +, Reizer, Eyal wrote: > Add support for using with both wl12xx and wl18xx. > > - all wilink family needs special init command for entering wspi mode. > extra clock cycles should be sent after the spi init command while the > cs pin is high. > - Use inverted chip select for sending a dummy 4 bytes command that > completes the init stage. > > Signed-off-by: Eyal Reizer > --- > v1->v2:update device tree bindings configuration > v2->v3:revert from manual gpio manipulation. use inverted chip select instead > for sending the extra init cycle which, achieves the same hardware purpose. > update device tree bindings docucmentation accordingly > v3->v4: Remove redundant data form binding documentation and fix chip select > number mismatch in wl1271 example > v4->v5: Rebase on top of head of wireless-drivers-next Please add acks when reposting. Rob
Re: [PATCH V2 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings
On Mon, Jul 11, 2016 at 04:18:07PM +0800, Weiqing Kong wrote: > Add MT2701 compatible string. > > Signed-off-by: Weiqing Kong > --- > Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Acked-by: Rob Herring
Re: [PATCH] radix-tree: fix radix_tree_iter_retry() for tagged iterators.
On Fri, Jul 15, 2016 at 10:00 PM, Ross Zwisler wrote: > On Fri, Jul 15, 2016 at 11:52:58AM +0300, Andrey Ryabinin wrote: >> On 07/15/2016 01:25 AM, Ross Zwisler wrote: >> > On Thu, Jul 14, 2016 at 02:19:56PM +0300, Andrey Ryabinin wrote: >> >> radix_tree_iter_retry() resets slot to NULL, but it doesn't reset tags. >> >> Then NULL slot and non-zero iter.tags passed to radix_tree_next_slot() >> >> leading to crash: >> >> >> >> RIP: [< inline >] radix_tree_next_slot >> >> include/linux/radix-tree.h:473 >> >> [] find_get_pages_tag+0x334/0x930 mm/filemap.c:1452 >> >> >> >> Call Trace: >> >> [] pagevec_lookup_tag+0x3a/0x80 mm/swap.c:960 >> >> [] mpage_prepare_extent_to_map+0x321/0xa90 >> >> fs/ext4/inode.c:2516 >> >> [] ext4_writepages+0x10be/0x2b20 fs/ext4/inode.c:2736 >> >> [] do_writepages+0x97/0x100 mm/page-writeback.c:2364 >> >> [] __filemap_fdatawrite_range+0x248/0x2e0 >> >> mm/filemap.c:300 >> >> [] filemap_write_and_wait_range+0x121/0x1b0 >> >> mm/filemap.c:490 >> >> [] ext4_sync_file+0x34d/0xdb0 fs/ext4/fsync.c:115 >> >> [] vfs_fsync_range+0x10a/0x250 fs/sync.c:195 >> >> [< inline >] vfs_fsync fs/sync.c:209 >> >> [] do_fsync+0x42/0x70 fs/sync.c:219 >> >> [< inline >] SYSC_fdatasync fs/sync.c:232 >> >> [] SyS_fdatasync+0x19/0x20 fs/sync.c:230 >> >> [] entry_SYSCALL_64_fastpath+0x23/0xc1 >> >> arch/x86/entry/entry_64.S:207 >> >> >> >> We must reset iterator's tags to bail out from radix_tree_next_slot() and >> >> go to the slow-path in radix_tree_next_chunk(). >> > >> > This analysis doesn't make sense to me. In find_get_pages_tag(), when we >> > call >> > radix_tree_iter_retry(), this sets the local 'slot' variable to NULL, then >> > does a 'continue'. This will hop to the next iteration of the >> > radix_tree_for_each_tagged() loop, which will very check the exit >> > condition of >> > the for() loop: >> > >> > #define radix_tree_for_each_tagged(slot, root, iter, start, tag)\ >> > for (slot = radix_tree_iter_init(iter, start) ; \ >> > slot || (slot = radix_tree_next_chunk(root, iter, \ >> > RADIX_TREE_ITER_TAGGED | tag)) ; \ >> > slot = radix_tree_next_slot(slot, iter,\ >> > RADIX_TREE_ITER_TAGGED)) >> > >> > So, we'll run the >> > slot || (slot = radix_tree_next_chunk(root, iter, \ >> > RADIX_TREE_ITER_TAGGED | tag)) ; \ >> > >> > bit first. >> >> This is not the way how the for() loop works. slot = radix_tree_next_slot() >> executed first >> and only after that goes the condition statement. > > Right...*sigh*... Thanks for the sanity check. :) > >> > 'slot' is NULL, so we'll set it via radix_tree_next_chunk(). At >> > this point radix_tree_next_slot() hasn't been called. >> > >> > radix_tree_next_chunk() will set up the iter->index, iter->next_index and >> > iter->tags before it returns. The next iteration of the loop in >> > find_get_pages_tag() will use the non-NULL slot provided by >> > radix_tree_next_chunk(), and only after that iteration will we call >> > radix_tree_next_slot() again. By then iter->tags should be up to date. >> > >> > Do you have a test setup that reliably fails without this code but passes >> > when >> > you zero out iter->tags? >> > >> >> >> Yup, I run Dmitry's reproducer in a parallel loop: >> $ while true; do ./a.out & done >> >> Usually it takes just couple minutes maximum. > > Cool - I was able to get this to work on my system as well by upping the > thread count. > > In looking at this more, I agree that your patch fixes this particular bug, > but I think that ultimately we might want something more general. > > IIUC, the real issue is that we shouldn't be running through > radix_tree_next_slot() with a NULL 'slot' parameter. In the end I think it's > fine to zero out iter->tags in radix_tree_iter_retry(), but really we want to > guarantee that we just bail out of radix_tree_next_slot() if we have a NULL > 'slot'. > > I've run this patch in my test setup, and it fixes the reproducer provided by > Dmitry. I've also run xfstests against it with out any failures. > > --- 8< --- > From 533beefac12f61f467aeb72e2d2c46685247b9bc Mon Sep 17 00:00:00 2001 > From: Ross Zwisler > Date: Fri, 15 Jul 2016 12:46:38 -0600 > Subject: [PATCH] radix-tree: 'slot' can be NULL in radix_tree_next_slot() > > There are four cases I can see where we could end up with a NULL 'slot' in > radix_tree_next_slot() (there might be more): > > 1) radix_tree_iter_retry() via a non-tagged iteration like > radix_tree_for_each_slot(). In this case we currently aren't seeing a bug > because radix_tree_iter_retry() sets > > iter->next_index = iter->index; > > which means that in in the else case in radix_tree_next_slot(), 'count' is > zero, so we skip over the while() loop and effectively just return NULL > without ever dereferencing 'slot'. > > 2) rad
Re: [PATCH v1 4/6] drm/rockchip: dw_hdmi: add RK3399 HDMI support
On Mon, Jul 11, 2016 at 07:05:46PM +0800, Yakir Yang wrote: > RK3399 and RK3288 shared the same HDMI IP controller, only some light > difference with GRF configure. > > Signed-off-by: Yakir Yang > --- > .../devicetree/bindings/display/bridge/dw_hdmi.txt | 1 + > .../bindings/display/rockchip/dw_hdmi-rockchip.txt | 3 +- Acked-by: Rob Herring > drivers/gpu/drm/bridge/dw-hdmi.c | 2 +- > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c| 45 > ++ > include/drm/bridge/dw_hdmi.h | 6 +++ > 5 files changed, 48 insertions(+), 9 deletions(-)
Re: [PATCH v2 2/6] [media] Documentation: Add HSV format
On 07/16/2016 02:38 PM, Laurent Pinchart wrote: > Hi Hans, > > On Saturday 16 Jul 2016 10:19:29 Hans Verkuil wrote: >> On 07/15/2016 08:11 PM, Laurent Pinchart wrote: >>> On Friday 15 Jul 2016 18:13:15 Ricardo Ribalda Delgado wrote: Describe the HSV formats Signed-off-by: Ricardo Ribalda Delgado --- Documentation/media/uapi/v4l/hsv-formats.rst | 19 ++ Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst | 253 +++ Documentation/media/uapi/v4l/pixfmt.rst| 1 + Documentation/media/uapi/v4l/v4l2.rst | 5 + 4 files changed, 278 insertions(+) create mode 100644 Documentation/media/uapi/v4l/hsv-formats.rst create mode 100644 Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst diff --git a/Documentation/media/uapi/v4l/hsv-formats.rst b/Documentation/media/uapi/v4l/hsv-formats.rst new file mode 100644 index ..f0f2615eaa95 --- /dev/null +++ b/Documentation/media/uapi/v4l/hsv-formats.rst @@ -0,0 +1,19 @@ +.. -*- coding: utf-8; mode: rst -*- + +.. _hsv-formats: + +*** +HSV Formats +*** + +These formats store the color information of the image +in a geometrical representation. The colors are mapped into a +cylinder, where the angle is the HUE, the height is the VALUE +and the distance to the center is the SATURATION. This is a very +useful format for image segmentation algorithms. + + +.. toctree:: +:maxdepth: 1 + +pixfmt-packed-hsv diff --git a/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst b/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst new file mode 100644 index ..b297aa4f7ba6 --- /dev/null +++ b/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst @@ -0,0 +1,253 @@ +.. -*- coding: utf-8; mode: rst -*- + +.. _packed-hsv: + +** +Packed HSV formats +** + +*man Packed HSV formats(2)* + +Packed HSV formats + + +Description +=== + +The HUE (h) is meassured in degrees, one LSB represents two degrees. >>> >>> Is this common ? I have a device that can handle HSV data, I need to check >>> how it maps the hue values to binary, but I'm pretty sure they cover the >>> full 0-255 range. We would then have to support the two formats. Separate >>> 4CCs are an option, but reporting the range separately (possibly through >>> the colorspace API) might be better. Any thought on that ? >> >> It's either a separate 4cc or we do something with the ycbcr_enc field >> (reinterpreted as hsv_enc). I'm not sure, I would have to think some more >> about that. > > I'm inclined to use the ycbcr_enc field, especially given that a similar > usage > could be useful for RGB as well (don't ask me what it's supposed to be used > for, but I have hardware that support limiting the RGB values range to > 16-235). Limited vs full range quantization is handled by the quantization field. It's there already. Limited range RGB is needed for HDMI due to a brain-dead spec when dealing with certain kinds of TVs and configurations. Don't ask, it's horrible. Anyway, I am inclined to use ycbcr_enc as well. Regards, Hans
[GIT PULL] KVM fixes for 4.7-rc8
Linus, The following changes since commit a99cde438de0c4c0cecc1d1af1a55a75b10bfdef: Linux 4.7-rc6 (2016-07-03 23:01:00 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/virt/kvm/kvm.git tags/for-linus for you to fetch changes up to b244c9fc251e14a083a1cbf04bef10bd99303a76: KVM: VMX: handle PML full VMEXIT that occurs during event delivery (2016-07-16 15:27:40 +0200) This merge contains a SVM patch that was not fixing anything, its build fix, and their revert; KVM: SVM: fix trashing of MSR_TSC_AUX KVM: SVM: do not set MSR_TSC_AUX on 32-bit builds Revert "KVM: SVM: fix trashing of MSR_TSC_AUX" a patch to avoid a leak, its revert, and a hopefully correct handling of a case that was introduced with per-vm debugfs in this release; KVM: release anon file in failure path of vm creation Revert "KVM: release anon file in failure path of vm creation" KVM: don't use anon_inode_getfd() before possible failures two patches reordering code that should have been called between vcpu_load() and vcpu_put(); kvm: vmx: ensure VMCS is current while enabling PML KVM: nVMX: Fix memory corruption when using VMCS shadowing and two one-liners for stable. KVM: VMX: handle PML full VMEXIT that occurs during event delivery KVM: MTRR: fix kvm_mtrr_check_gfn_range_consistency page fault Al Viro (1): KVM: don't use anon_inode_getfd() before possible failures Alexis Dambricourt (1): KVM: MTRR: fix kvm_mtrr_check_gfn_range_consistency page fault Cao, Lei (1): KVM: VMX: handle PML full VMEXIT that occurs during event delivery Jim Mattson (1): KVM: nVMX: Fix memory corruption when using VMCS shadowing Liu Shuo (1): KVM: release anon file in failure path of vm creation Paolo Bonzini (3): KVM: SVM: fix trashing of MSR_TSC_AUX Revert "KVM: release anon file in failure path of vm creation" KVM: SVM: do not set MSR_TSC_AUX on 32-bit builds Peter Feiner (1): kvm: vmx: ensure VMCS is current while enabling PML Radim Krčmář (1): Revert "KVM: SVM: fix trashing of MSR_TSC_AUX" arch/x86/kvm/mtrr.c | 1 + arch/x86/kvm/vmx.c | 76 + virt/kvm/kvm_main.c | 15 +-- 3 files changed, 56 insertions(+), 36 deletions(-)
[PATCH] net/sched/sch_htb: clamp xstats tokens to fit into 32-bit int
In kernel HTB keeps tokens in signed 64-bit in nanoseconds. In netlink protocol these values are converted into pshed ticks (64ns for now) and truncated to 32-bit. In struct tc_htb_xstats fields "tokens" and "ctokens" are declared as unsigned 32-bit but they could be negative thus tool 'tc' prints them as signed. Big values loose higher bits and/or become negative. This patch clamps tokens in xstat into range from INT_MIN to INT_MAX. In this way it's easier to understand what's going on here. Signed-off-by: Konstantin Khlebnikov --- net/sched/sch_htb.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index 62f9d8100c6e..052f84d6cc23 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c @@ -1140,8 +1140,10 @@ htb_dump_class_stats(struct Qdisc *sch, unsigned long arg, struct gnet_dump *d) if (!cl->level && cl->un.leaf.q) qlen = cl->un.leaf.q->q.qlen; - cl->xstats.tokens = PSCHED_NS2TICKS(cl->tokens); - cl->xstats.ctokens = PSCHED_NS2TICKS(cl->ctokens); + cl->xstats.tokens = clamp_t(s64, PSCHED_NS2TICKS(cl->tokens), + INT_MIN, INT_MAX); + cl->xstats.ctokens = clamp_t(s64, PSCHED_NS2TICKS(cl->ctokens), +INT_MIN, INT_MAX); if (gnet_stats_copy_basic(d, NULL, &cl->bstats) < 0 || gnet_stats_copy_rate_est(d, NULL, &cl->rate_est) < 0 ||
Re: 4.1.28: memory leak introduced by "mm/swap.c: flush lru pvecs on compound page arrival"
Hi again, took lack of response to express reluctance examining vendor kernels. Therefore reproduced and can confirm memory leak on 4.1.28 vanilla x86. Identical symptoms. Regards, Jens From: Jens Rottmann Sent: Friday, July 15, 2016 21:27 To: Lukasz Odzioba; Sasha Levin Cc: sta...@vger.kernel.org; Michal Hocko; linux...@kvack.org; linux-kernel@vger.kernel.org Subject: 4.1.28: memory leak introduced by "mm/swap.c: flush lru pvecs on compound page arrival" Hi, 4.1.y stable commit c5ad33184354260be6d05de57e46a5498692f6d6 (Upstream commit 8f182270dfec432e93fae14f9208a6b9af01009f) "mm/swap.c: flush lru pvecs on compound page arrival" in 4.1.28 introduces a memory leak. Simply running while sleep 0.1; do clear; free; done shows mem continuously going down, eventually system panics with no killable processes left. Replacing "sleep" with "unxz -t some.xz" brings system down within minutes. Kmemleak did not report anything. Bisect ended at named commit, and reverting only this commit is indeed sufficient to fix the leak. Swap partition on/off makes no difference. My set-up: i.MX6 (ARM Cortex-A9) dual-core, 2 GB RAM. Kernel sources are from git.freescale.com i.e. heavily modified by Freescale for i.MX SoCs, kernel.org stable patches up to 4.1.28 manually added. I tried to reproduce with vanilla 4.1.28, but that wouldn't boot at all on my i.MX hardware, hangs immediately after "Starting kernel", sorry. However there is not a single difference between Freescale and vanilla in the whole mm/ subdirectory, so I don't think it's i.MX-specific. I didn't cross-check with an x86 system (yet). Regards, Jens
[PATCH v2 comment update] kernel/watchdog: use nmi registers snapshot in hardlockup handler
NMI handler doesn't call set_irq_regs(), it's set only by normal IRQ. Thus get_irq_regs() returns NULL or stale registers snapshot with IP/SP pointing to the code interrupted by IRQ which was interrupted by NMI. NULL isn't a problem: in this case watchdog calls dump_stack() and prints full stack trace including NMI. But if we're stuck in IRQ handler then NMI watchlog will print stack trace without IRQ part at all. This patch uses registers snapshot passed into NMI handler as arguments: these registers points exactly to the instruction interrupted by NMI. Signed-off-by: Konstantin Khlebnikov Fixes: 55537871ef66 ("kernel/watchdog.c: perform all-CPU backtrace in case of hard lockup") Cc: Stable # 4.4+ Cc: Jiri Kosina --- kernel/watchdog.c |1 - 1 file changed, 1 deletion(-) diff --git a/kernel/watchdog.c b/kernel/watchdog.c index 9acb29f280ec..6d1020c03d41 100644 --- a/kernel/watchdog.c +++ b/kernel/watchdog.c @@ -344,7 +344,6 @@ static void watchdog_overflow_callback(struct perf_event *event, */ if (is_hardlockup()) { int this_cpu = smp_processor_id(); - struct pt_regs *regs = get_irq_regs(); /* only print hardlockups once */ if (__this_cpu_read(hard_watchdog_warn) == true)
Re: [PATCH v2 2/6] [media] Documentation: Add HSV format
Hi Hans, On Saturday 16 Jul 2016 15:59:08 Hans Verkuil wrote: > On 07/16/2016 02:38 PM, Laurent Pinchart wrote: >> On Saturday 16 Jul 2016 10:19:29 Hans Verkuil wrote: >>> On 07/15/2016 08:11 PM, Laurent Pinchart wrote: On Friday 15 Jul 2016 18:13:15 Ricardo Ribalda Delgado wrote: > Describe the HSV formats > > Signed-off-by: Ricardo Ribalda Delgado > --- > > Documentation/media/uapi/v4l/hsv-formats.rst | 19 ++ > Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst | 253 > +++ > Documentation/media/uapi/v4l/pixfmt.rst| 1 + > Documentation/media/uapi/v4l/v4l2.rst | 5 + > 4 files changed, 278 insertions(+) > create mode 100644 Documentation/media/uapi/v4l/hsv-formats.rst > create mode 100644 Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst > > diff --git a/Documentation/media/uapi/v4l/hsv-formats.rst > b/Documentation/media/uapi/v4l/hsv-formats.rst new file mode 100644 > index ..f0f2615eaa95 > --- /dev/null > +++ b/Documentation/media/uapi/v4l/hsv-formats.rst > @@ -0,0 +1,19 @@ > +.. -*- coding: utf-8; mode: rst -*- > + > +.. _hsv-formats: > + > +*** > +HSV Formats > +*** > + > +These formats store the color information of the image > +in a geometrical representation. The colors are mapped into a > +cylinder, where the angle is the HUE, the height is the VALUE > +and the distance to the center is the SATURATION. This is a very > +useful format for image segmentation algorithms. > + > + > +.. toctree:: > +:maxdepth: 1 > + > +pixfmt-packed-hsv > diff --git a/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst > b/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst new file mode > 100644 > index ..b297aa4f7ba6 > --- /dev/null > +++ b/Documentation/media/uapi/v4l/pixfmt-packed-hsv.rst > @@ -0,0 +1,253 @@ > +.. -*- coding: utf-8; mode: rst -*- > + > +.. _packed-hsv: > + > +** > +Packed HSV formats > +** > + > +*man Packed HSV formats(2)* > + > +Packed HSV formats > + > + > +Description > +=== > + > +The HUE (h) is meassured in degrees, one LSB represents two degrees. Is this common ? I have a device that can handle HSV data, I need to check how it maps the hue values to binary, but I'm pretty sure they cover the full 0-255 range. We would then have to support the two formats. Separate 4CCs are an option, but reporting the range separately (possibly through the colorspace API) might be better. Any thought on that ? >>> >>> It's either a separate 4cc or we do something with the ycbcr_enc field >>> (reinterpreted as hsv_enc). I'm not sure, I would have to think some more >>> about that. >> >> I'm inclined to use the ycbcr_enc field, especially given that a similar >> usage could be useful for RGB as well (don't ask me what it's supposed to >> be used for, but I have hardware that support limiting the RGB values >> range to 16-235). > > Limited vs full range quantization is handled by the quantization field. > It's there already. Right. I wonder how we'll deal with that when someone will come up with more than one limited range quantizations, have you thought about it ? > Limited range RGB is needed for HDMI due to a brain-dead spec when dealing > with certain kinds of TVs and configurations. Don't ask, it's horrible. I'd still like to know about it for my personal information :-) > Anyway, I am inclined to use ycbcr_enc as well. I'm glad we agree. -- Regards, Laurent Pinchart
Re: [PATCH v2 2/6] [media] Documentation: Add HSV format
Hi On Sat, Jul 16, 2016 at 4:12 PM, Laurent Pinchart wrote: > I'd still like to know about it for my personal information :-) Maybe it is just a very cheap gamma. > >> Anyway, I am inclined to use ycbcr_enc as well. > > I'm glad we agree. > Are you thinking about something like this: diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index c7fb760386cf..3e613fba1b20 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -330,6 +330,16 @@ enum v4l2_ycbcr_encoding { V4L2_YCBCR_ENC_SMPTE240M = 8, }; +enum v4l2_hsv_encoding { + V4L2_HSV_ENC_180= 16, + V4L2_HSV_ENC_256= 17, +}; + +enum v4l2_rgb_encoding { + V4L2_RGB_ENC_FULL = 32, + V4L2_HSV_ENC_16_235 = 33, +}; + /* * Determine how YCBCR_ENC_DEFAULT should map to a proper Y'CbCr encoding. * This depends on the colorspace. @@ -455,7 +465,11 @@ struct v4l2_pix_format { __u32 colorspace; /* enum v4l2_colorspace */ __u32 priv; /* private data, depends on pixelformat */ __u32 flags; /* format flags (V4L2_PIX_FMT_FLAG_*) */ - __u32 ycbcr_enc; /* enum v4l2_ycbcr_encoding */ + union { + __u32 ycbcr_enc; /* enum v4l2_ycbcr_encoding */ + __u32 hsv_enc;/* enum v4l2_hsv_encoding */ + __u32 rgb_enc;/* enum v4l2_rgb_encoding */ + }; __u32 quantization; /* enum v4l2_quantization */ __u32 xfer_func; /* enum v4l2_xfer_func */ }; @@ -1988,7 +2002,11 @@ struct v4l2_pix_format_mplane { struct v4l2_plane_pix_formatplane_fmt[VIDEO_MAX_PLANES]; __u8num_planes; __u8flags; - __u8ycbcr_enc; + union { + __u8ycbcr_enc; /* enum v4l2_ycbcr_encoding */ + __u8hsv_enc;/* enum v4l2_hsv_encoding */ + __u8rgb_enc;/* enum v4l2_rgb_encoding */ + }; __u8quantization; __u8xfer_func; __u8reserved[7]; > -- Best regards! -- Ricardo Ribalda
[PATCH 0/8] drm/amdgpu: Fine-tuning for three function implementations
From: Markus Elfring Date: Sat, 16 Jul 2016 16:23:21 +0200 Further update suggestions were taken into account after patches were applied from static source code analysis. Markus Elfring (8): Delete an unnecessary check before drm_gem_object_unreference_unlocked() Delete unnecessary checks before the function call "kfree" One function call less in amdgpu_cgs_acpi_eval_object() after error detection Delete a variable in amdgpu_cgs_acpi_eval_object() Delete an unnecessary variable initialisation in amdgpu_cgs_acpi_eval_object() Change assignment for a variable in amdgpu_cgs_acpi_eval_object() Change assignment for a buffer variable in phm_dispatch_table() Delete an unnecessary variable initialisation in phm_dispatch_table() drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c| 28 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_display.c| 4 +--- .../gpu/drm/amd/powerplay/hwmgr/functiontables.c | 12 -- 3 files changed, 19 insertions(+), 25 deletions(-) -- 2.9.1
[PATCH 1/8] drm/amdgpu: Delete an unnecessary check before drm_gem_object_unreference_unlocked()
From: Markus Elfring Date: Sat, 16 Jul 2016 11:28:36 +0200 The drm_gem_object_unreference_unlocked() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index a6eecf6..2a07b15 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -516,9 +516,7 @@ static void amdgpu_user_framebuffer_destroy(struct drm_framebuffer *fb) { struct amdgpu_framebuffer *amdgpu_fb = to_amdgpu_framebuffer(fb); - if (amdgpu_fb->obj) { - drm_gem_object_unreference_unlocked(amdgpu_fb->obj); - } + drm_gem_object_unreference_unlocked(amdgpu_fb->obj); drm_framebuffer_cleanup(fb); kfree(amdgpu_fb); } -- 2.9.1
4.1.28 is broken due to "mm/swap.c: flush lru pvecs on compound page arrival"
Hi The patch c5ad33184354260be6d05de57e46a5498692f6d6 on the kernel v4.1.28 breaks the kernel. The kernel crashes when executing the boot scripts with "kernel panic: Out of memory and no killable processes...". The machine has 512MB ram and 1 core. Note that the upstream kernel 4.7-rc4 with this patch works, but when the patch is backported to the 4.1 branch, it makes the system unbootable. Mikulas
Re: 4.1.28: memory leak introduced by "mm/swap.c: flush lru pvecs on compound page arrival"
On Fri, Jul 15, 2016 at 09:27:55PM +0200, Jens Rottmann wrote: > Hi, > > 4.1.y stable commit c5ad33184354260be6d05de57e46a5498692f6d6 (Upstream > commit 8f182270dfec432e93fae14f9208a6b9af01009f) "mm/swap.c: flush lru > pvecs on compound page arrival" in 4.1.28 introduces a memory leak. > > Simply running > > while sleep 0.1; do clear; free; done > > shows mem continuously going down, eventually system panics with no > killable processes left. Using "unxz -t some.xz" instead of sleep brings > system down within minutes. > > Kmemleak did not report anything. Bisect ended at named commit, and > reverting only this commit is indeed sufficient to fix the leak. Swap > partition on/off makes no difference. > > My set-up: > i.MX6 (ARM Cortex-A9) dual-core, 2 GB RAM. Kernel sources are from > git.freescale.com i.e. heavily modified by Freescale for i.MX SoCs, > kernel.org stable patches up to 4.1.28 manually added. > > I tried to reproduce with vanilla 4.1.28, but that wouldn't boot at all > on my hardware, hangs immediately after "Starting kernel", sorry. > However there is not a single difference between Freescale and vanilla > in the whole mm/ subdirectory, so I don't think it's i.MX-specific. I > didn't cross-check with an x86 system (yet). I didn't have 4.1 stable tree in my local so just looked at git web and found __lru_cache_add has a bug. Please change static void __lru_cache_add(struct page *page) { struct pagevec *pvec = &get_cpu_var(lru_add_pvec); page_cache_get(page); if (!pagevec_space(pvec) || PageCompound(page)) <== __pagevec_lru_add(pvec); put_cpu_var(lru_add_pvec); } with static void __lru_cache_add(struct page *page) { struct pagevec *pvec = &get_cpu_var(lru_add_pvec); page_cache_get(page); if (!pagevec_add(pvec, page) || PageCompound(page)) <== __pagevec_lru_add(pvec); put_cpu_var(lru_add_pvec); }
[PATCH 2/8] drm/amdgpu/powerplay: Delete unnecessary checks before the function call "kfree"
From: Markus Elfring Date: Sat, 16 Jul 2016 12:38:12 +0200 The kfree() function tests whether its argument is NULL and then returns immediately. Thus the test around the calls is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 3 +-- drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c | 5 + 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c index cf6f49f..6f11bc1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c @@ -1053,8 +1053,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device, } error: - if (obj != NULL) - kfree(obj); + kfree(obj); kfree((void *)input.pointer); return result; } diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c b/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c index 7a705ce..024e22e 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/functiontables.c @@ -76,10 +76,7 @@ int phm_dispatch_table(struct pp_hwmgr *hwmgr, } result = phm_run_table(hwmgr, rt_table, input, output, temp_storage); - - if (NULL != temp_storage) - kfree(temp_storage); - + kfree(temp_storage); return result; } -- 2.9.1
[PATCH 3/8] drm/amdgpu: One function call less in amdgpu_cgs_acpi_eval_object() after error detection
From: Markus Elfring Date: Sat, 16 Jul 2016 13:43:44 +0200 The kfree() function was called in one case by the amdgpu_cgs_acpi_eval_object() function during error handling even if the passed variable "obj" contained a null pointer. * Adjust jump targets according to the Linux coding style convention. * Delete unnecessary initialisations for the variables "obj" and "params" then. Signed-off-by: Markus Elfring --- drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c index 6f11bc1..705bfa2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c @@ -903,8 +903,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device, acpi_handle handle; struct acpi_object_list input; struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; - union acpi_object *params = NULL; - union acpi_object *obj = NULL; + union acpi_object *params, *obj; uint8_t name[5] = {'\0'}; struct cgs_acpi_method_argument *argument = NULL; uint32_t i, count; @@ -996,7 +995,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device, if (ACPI_FAILURE(status)) { result = -EIO; - goto error; + goto free_input; } /* return the output info */ @@ -1006,7 +1005,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device, if ((obj->type != ACPI_TYPE_PACKAGE) || (obj->package.count != count)) { result = -EIO; - goto error; + goto free_obj; } params = obj->package.elements; } else @@ -1014,13 +1013,13 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device, if (params == NULL) { result = -EIO; - goto error; + goto free_obj; } for (i = 0; i < count; i++) { if (argument->type != params->type) { result = -EIO; - goto error; + goto free_obj; } switch (params->type) { case ACPI_TYPE_INTEGER: @@ -1030,7 +1029,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device, if ((params->string.length != argument->data_length) || (params->string.pointer == NULL)) { result = -EIO; - goto error; + goto free_obj; } strncpy(argument->pointer, params->string.pointer, @@ -1039,7 +1038,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device, case ACPI_TYPE_BUFFER: if (params->buffer.pointer == NULL) { result = -EIO; - goto error; + goto free_obj; } memcpy(argument->pointer, params->buffer.pointer, @@ -1052,8 +1051,9 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device, params++; } -error: +free_obj: kfree(obj); +free_input: kfree((void *)input.pointer); return result; } -- 2.9.1
Re: 4.1.28 is broken due to "mm/swap.c: flush lru pvecs on compound page arrival"
On Sat, Jul 16, 2016 at 10:46:17AM -0400, Mikulas Patocka wrote: > Hi > > The patch c5ad33184354260be6d05de57e46a5498692f6d6 on the kernel v4.1.28 > breaks the kernel. The kernel crashes when executing the boot scripts with > "kernel panic: Out of memory and no killable processes...". The machine > has 512MB ram and 1 core. > > Note that the upstream kernel 4.7-rc4 with this patch works, but when the > patch is backported to the 4.1 branch, it makes the system unbootable. It seems a bug was introduced at backport time, I think. Please, look at http://marc.info/?l=linux-mm&m=146868046305014&w=2 > > Mikulas
[PATCH 4/8] drm/amdgpu: Delete a variable in amdgpu_cgs_acpi_eval_object()
From: Markus Elfring Date: Sat, 16 Jul 2016 14:00:28 +0200 The local variable "func_no" was assigned a value at two places. But it was not read within this function. Thus delete it. Signed-off-by: Markus Elfring --- drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c index 705bfa2..f5de510 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c @@ -909,7 +909,6 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device, uint32_t i, count; acpi_status status; int result = 0; - uint32_t func_no = 0x; handle = ACPI_HANDLE(&adev->pdev->dev); if (!handle) @@ -926,7 +925,6 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device, if (info->pinput_argument == NULL) return -EINVAL; argument = info->pinput_argument; - func_no = argument->value; for (i = 0; i < info->input_count; i++) { if (((argument->type == ACPI_TYPE_STRING) || (argument->type == ACPI_TYPE_BUFFER)) && -- 2.9.1
[PATCH 5/8] drm/amdgpu: Delete an unnecessary variable initialisation in amdgpu_cgs_acpi_eval_object()
From: Markus Elfring Date: Sat, 16 Jul 2016 14:54:12 +0200 The variable "argument" will be set to an appropriate value a bit later. Thus omit the explicit initialisation at the beginning. Signed-off-by: Markus Elfring --- drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c index f5de510..47f2a43 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c @@ -905,7 +905,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device, struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; union acpi_object *params, *obj; uint8_t name[5] = {'\0'}; - struct cgs_acpi_method_argument *argument = NULL; + struct cgs_acpi_method_argument *argument; uint32_t i, count; acpi_status status; int result = 0; -- 2.9.1
[PATCH 5/8] drm/amdgpu: Delete an unnecessary variable initialisation in amdgpu_cgs_acpi_eval_object()
From: Markus Elfring Date: Sat, 16 Jul 2016 14:54:12 +0200 The variable "argument" will be set to an appropriate value a bit later. Thus omit the explicit initialisation at the beginning. Signed-off-by: Markus Elfring --- drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c index f5de510..47f2a43 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c @@ -905,7 +905,7 @@ static int amdgpu_cgs_acpi_eval_object(struct cgs_device *cgs_device, struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL }; union acpi_object *params, *obj; uint8_t name[5] = {'\0'}; - struct cgs_acpi_method_argument *argument = NULL; + struct cgs_acpi_method_argument *argument; uint32_t i, count; acpi_status status; int result = 0; -- 2.9.1
[PATCH v0 00/10] Convert Netgear WNR854T to devicetree
This is an attempt to resurrect the mainline WNR854T support (I had no luck getting a non-DT kernel to boot). First 2 patches are trivial, the next 3 add DT bindings for the SoC, then the old board file is swapped out for DT. The final 3 patches configure the PHY leds as Netgear intended. There's one major flaw; unicast traffic is never received on any port. Broadcast traffic is received however, and on the correct port. Thus an external machine can make an ARP request and get a response, for example. With a manually-entered ARP entry, the router can send pings out to a remote machine which responds, and the response is lost in the DSA switch. "ethtool -S" reports pings received on "in_unicast" but nothing makes it through the switch. This thread[0] seems very similar. I've run out of ideas here and can't find any switch datasheets to give me pointers so any suggestions greatly appreciated. I've not tried to move the PCI support into DT yet. Are there other branches that tackle this already? Patch 8 is the least-intrusive patch I could think of to find the PHY node, but I'm not sure it's the best approach. I tried to get dsa_slave_phy_connect() to wire up the OF node correctly[1], but doing this properly would require changes to the mdiobus api, any exposed methods register all nodes in an MDIO bus node, which doesn't exist in this case. Applied against v4.6 mainline, .config: http://jamie.lentin.co.uk/devices/netgear-wnr854t/wnr854t-support-v0a/kernel-config.txt Log output: http://jamie.lentin.co.uk/devices/netgear-wnr854t/wnr854t-support-v0a/u-boot.dmesg.txt The patchset is also available here: https://github.com/lentinj/linux/commits/wnr854t-support-v0a To use the supplied PCI Wifi card see the following patch: https://github.com/lentinj/linux/commit/d6b7b4695b7dcbf36d0663aea9cb4ecd65a1ada8 ...but this is probably a matter for another thread. Cheers, [0] http://thread.gmane.org/gmane.linux.network/120616/focus=121320 [1] https://github.com/lentinj/linux/commit/a4bceebb08ecdc25a4d49e23f2abad214868460e Jamie Lentin (10): arm: orion5x: Add required properties for orion-wdt to DT node arm: orion5x: Add documentation for SoC and board bindings arm: orion5x: Add clk support for mv88f5181 arm: orion5x: Generalise mv88f5181l pinctrl support for 88f5181 arm: orion5x: Add DT include for mv88f5181 arm: orion5x: Add DT-based support for Netgear WNR854T arm: orion5x: Remove old non-DT-based WNR854T support net: phy: Try looking for a phy-handle property to find the OF node net: phy: Re-attempt custom DT configuration after configuration arm: orion5x: Configure Netgear WNR854T network port LEDs .../bindings/arm/marvell/marvell,orion5x.txt | 25 +++ .../devicetree/bindings/clock/mvebu-core-clock.txt | 1 + .../bindings/pinctrl/marvell,orion-pinctrl.txt | 4 +- arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/orion5x-mv88f5181.dtsi | 35 +++ arch/arm/boot/dts/orion5x-netgear-wnr854t.dts | 245 + arch/arm/boot/dts/orion5x.dtsi | 3 +- arch/arm/mach-orion5x/Kconfig | 6 + arch/arm/mach-orion5x/Makefile | 2 +- arch/arm/mach-orion5x/board-wnr854t.c | 78 +++ arch/arm/mach-orion5x/wnr854t-setup.c | 185 drivers/clk/mvebu/orion.c | 70 ++ drivers/net/phy/marvell.c | 26 ++- drivers/pinctrl/mvebu/pinctrl-orion.c | 23 +- 14 files changed, 498 insertions(+), 206 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/marvell/marvell,orion5x.txt create mode 100644 arch/arm/boot/dts/orion5x-mv88f5181.dtsi create mode 100644 arch/arm/boot/dts/orion5x-netgear-wnr854t.dts create mode 100644 arch/arm/mach-orion5x/board-wnr854t.c delete mode 100644 arch/arm/mach-orion5x/wnr854t-setup.c -- 2.8.1
[PATCH v0 07/10] arm: orion5x: Remove old non-DT-based WNR854T support
Signed-off-by: Jamie Lentin --- arch/arm/mach-orion5x/Makefile| 1 - arch/arm/mach-orion5x/wnr854t-setup.c | 185 -- 2 files changed, 186 deletions(-) delete mode 100644 arch/arm/mach-orion5x/wnr854t-setup.c diff --git a/arch/arm/mach-orion5x/Makefile b/arch/arm/mach-orion5x/Makefile index 9dff2d3..a2c9ebc 100644 --- a/arch/arm/mach-orion5x/Makefile +++ b/arch/arm/mach-orion5x/Makefile @@ -14,7 +14,6 @@ obj-$(CONFIG_MACH_WRT350N_V2) += wrt350n-v2-setup.o obj-$(CONFIG_MACH_TS78XX) += ts78xx-setup.o obj-$(CONFIG_MACH_MV2120) += mv2120-setup.o obj-$(CONFIG_MACH_NET2BIG) += net2big-setup.o -obj-$(CONFIG_MACH_WNR854T) += wnr854t-setup.o obj-$(CONFIG_MACH_RD88F5181L_GE) += rd88f5181l-ge-setup.o obj-$(CONFIG_MACH_RD88F5181L_FXO) += rd88f5181l-fxo-setup.o obj-$(CONFIG_MACH_RD88F6183AP_GE) += rd88f6183ap-ge-setup.o diff --git a/arch/arm/mach-orion5x/wnr854t-setup.c b/arch/arm/mach-orion5x/wnr854t-setup.c deleted file mode 100644 index 4e1e5c8..000 --- a/arch/arm/mach-orion5x/wnr854t-setup.c +++ /dev/null @@ -1,185 +0,0 @@ -/* - * arch/arm/mach-orion5x/wnr854t-setup.c - * - * This file is licensed under the terms of the GNU General Public - * License version 2. This program is licensed "as is" without any - * warranty of any kind, whether express or implied. - */ -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include "orion5x.h" -#include "common.h" -#include "mpp.h" - -static unsigned int wnr854t_mpp_modes[] __initdata = { - MPP0_GPIO, /* Power LED green (0=on) */ - MPP1_GPIO, /* Reset Button (0=off) */ - MPP2_GPIO, /* Power LED blink (0=off) */ - MPP3_GPIO, /* WAN Status LED amber (0=off) */ - MPP4_GPIO, /* PCI int */ - MPP5_GPIO, /* ??? */ - MPP6_GPIO, /* ??? */ - MPP7_GPIO, /* ??? */ - MPP8_UNUSED,/* ??? */ - MPP9_GIGE, /* GE_RXERR */ - MPP10_UNUSED, /* ??? */ - MPP11_UNUSED, /* ??? */ - MPP12_GIGE, /* GE_TXD[4] */ - MPP13_GIGE, /* GE_TXD[5] */ - MPP14_GIGE, /* GE_TXD[6] */ - MPP15_GIGE, /* GE_TXD[7] */ - MPP16_GIGE, /* GE_RXD[4] */ - MPP17_GIGE, /* GE_RXD[5] */ - MPP18_GIGE, /* GE_RXD[6] */ - MPP19_GIGE, /* GE_RXD[7] */ - 0, -}; - -/* - * 8M NOR flash Device bus boot chip select - */ -#define WNR854T_NOR_BOOT_BASE 0xf400 -#define WNR854T_NOR_BOOT_SIZE SZ_8M - -static struct mtd_partition wnr854t_nor_flash_partitions[] = { - { - .name = "kernel", - .offset = 0x, - .size = 0x0010, - }, { - .name = "rootfs", - .offset = 0x0010, - .size = 0x0066, - }, { - .name = "uboot", - .offset = 0x0076, - .size = 0x0004, - }, -}; - -static struct physmap_flash_data wnr854t_nor_flash_data = { - .width = 2, - .parts = wnr854t_nor_flash_partitions, - .nr_parts = ARRAY_SIZE(wnr854t_nor_flash_partitions), -}; - -static struct resource wnr854t_nor_flash_resource = { - .flags = IORESOURCE_MEM, - .start = WNR854T_NOR_BOOT_BASE, - .end= WNR854T_NOR_BOOT_BASE + WNR854T_NOR_BOOT_SIZE - 1, -}; - -static struct platform_device wnr854t_nor_flash = { - .name = "physmap-flash", - .id = 0, - .dev= { - .platform_data = &wnr854t_nor_flash_data, - }, - .num_resources = 1, - .resource = &wnr854t_nor_flash_resource, -}; - -static struct mv643xx_eth_platform_data wnr854t_eth_data = { - .phy_addr = MV643XX_ETH_PHY_NONE, - .speed = SPEED_1000, - .duplex = DUPLEX_FULL, -}; - -static struct dsa_chip_data wnr854t_switch_chip_data = { - .port_names[0] = "lan3", - .port_names[1] = "lan4", - .port_names[2] = "wan", - .port_names[3] = "cpu", - .port_names[5] = "lan1", - .port_names[7] = "lan2", -}; - -static struct dsa_platform_data wnr854t_switch_plat_data = { - .nr_chips = 1, - .chip = &wnr854t_switch_chip_data, -}; - -static void __init wnr854t_init(void) -{ - /* -* Setup basic Orion functions. Need to be called early. -*/ - orion5x_init(); - - orion5x_mpp_conf(wnr854t_mpp_modes); - - /* -* Configure peripherals. -*/ - orion5x_eth_init(&wnr854t_eth_data); -
[PATCH v0 03/10] arm: orion5x: Add clk support for mv88f5181
Referring to values in the u-boot port, add support for the mv88f5181 Signed-off-by: Jamie Lentin --- .../devicetree/bindings/clock/mvebu-core-clock.txt | 1 + drivers/clk/mvebu/orion.c | 70 ++ 2 files changed, 71 insertions(+) diff --git a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt index 670c2af..eb985a6 100644 --- a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt +++ b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt @@ -52,6 +52,7 @@ Required properties: "marvell,dove-core-clock" - for Dove SoC core clocks "marvell,kirkwood-core-clock" - for Kirkwood SoC (except mv88f6180) "marvell,mv88f6180-core-clock" - for Kirkwood MV88f6180 SoC + "marvell,mv88f5181-core-clock" - for Orion MV88F5181 SoC "marvell,mv88f5182-core-clock" - for Orion MV88F5182 SoC "marvell,mv88f5281-core-clock" - for Orion MV88F5281 SoC "marvell,mv88f6183-core-clock" - for Orion MV88F6183 SoC diff --git a/drivers/clk/mvebu/orion.c b/drivers/clk/mvebu/orion.c index fd12956..a6e5bee 100644 --- a/drivers/clk/mvebu/orion.c +++ b/drivers/clk/mvebu/orion.c @@ -21,6 +21,76 @@ static const struct coreclk_ratio orion_coreclk_ratios[] __initconst = { }; /* + * Orion 5181 + */ + +#define SAR_MV88F5181_TCLK_FREQ 8 +#define SAR_MV88F5181_TCLK_FREQ_MASK 0x3 + +static u32 __init mv88f5181_get_tclk_freq(void __iomem *sar) +{ + u32 opt = (readl(sar) >> SAR_MV88F5181_TCLK_FREQ) & + SAR_MV88F5181_TCLK_FREQ_MASK; + if (opt == 0) + return 1; + else if (opt == 1) + return 15000; + else if (opt == 2) + return 16667; + else + return 0; +} + +#define SAR_MV88F5181_CPU_FREQ 4 +#define SAR_MV88F5181_CPU_FREQ_MASK 0xf + +static u32 __init mv88f5181_get_cpu_freq(void __iomem *sar) +{ + u32 opt = (readl(sar) >> SAR_MV88F5181_CPU_FREQ) & + SAR_MV88F5181_CPU_FREQ_MASK; + if (opt == 0) + return 3; + else if (opt == 1 || opt == 2) + return 4; + else if (opt == 3) + return 5; + else + return 0; +} + +static void __init mv88f5181_get_clk_ratio(void __iomem *sar, int id, + int *mult, int *div) +{ + u32 opt = (readl(sar) >> SAR_MV88F5181_CPU_FREQ) & + SAR_MV88F5181_CPU_FREQ_MASK; + if (opt == 0 || opt == 1) { + *mult = 1; + *div = 2; + } else if (opt == 2 || opt == 3) { + *mult = 1; + *div = 3; + } else { + *mult = 0; + *div = 1; + } +} + +static const struct coreclk_soc_desc mv88f5181_coreclks = { + .get_tclk_freq = mv88f5181_get_tclk_freq, + .get_cpu_freq = mv88f5181_get_cpu_freq, + .get_clk_ratio = mv88f5181_get_clk_ratio, + .ratios = orion_coreclk_ratios, + .num_ratios = ARRAY_SIZE(orion_coreclk_ratios), +}; + +static void __init mv88f5181_clk_init(struct device_node *np) +{ + return mvebu_coreclk_setup(np, &mv88f5181_coreclks); +} + +CLK_OF_DECLARE(mv88f5181_clk, "marvell,mv88f5181-core-clock", mv88f5181_clk_init); + +/* * Orion 5182 */ -- 2.8.1
[PATCH v0 09/10] net: phy: Re-attempt custom DT configuration after configuration
marvell,reg-init is generally used to apply a custom LED configuration on boot. However this is then blatted in m88e1121_config_aneg when the interface is brought up. Re-apply any custom configuration afterwards, to keep custom LED configuration. Signed-off-by: Jamie Lentin --- drivers/net/phy/marvell.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index c2ca347..b55e4e0 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -424,6 +424,10 @@ static int m88e1121_config_aneg(struct phy_device *phydev) phy_write(phydev, MII_MARVELL_PHY_PAGE, oldpage); err = genphy_config_aneg(phydev); + if (err < 0) + return err; + + err = marvell_of_reg_init(phydev); return err; } -- 2.8.1
[PATCH v0 02/10] arm: orion5x: Add documentation for SoC and board bindings
Copy the format for kirkwood/dove to orion5x Signed-off-by: Jamie Lentin --- .../bindings/arm/marvell/marvell,orion5x.txt | 23 ++ 1 file changed, 23 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/marvell/marvell,orion5x.txt diff --git a/Documentation/devicetree/bindings/arm/marvell/marvell,orion5x.txt b/Documentation/devicetree/bindings/arm/marvell/marvell,orion5x.txt new file mode 100644 index 000..a888011 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/marvell/marvell,orion5x.txt @@ -0,0 +1,23 @@ +Marvell Orion SoC Family Device Tree Bindings +- + +Boards with a SoC of the Marvell Orion family, eg 88f5181 + +* Required root node properties: +compatible: must contain "marvell,orion5x" + +In addition, the above compatible shall be extended with the specific +SoC. Currently known SoC compatibles are: + +"marvell,orion5x-88f5182" + +And in addition, the compatible shall be extended with the specific +board. Currently known boards are: + +"buffalo,lsgl" +"buffalo,lswsgl" +"buffalo,lswtgl" +"lacie,ethernet-disk-mini-v2" +"lacie,d2-network" +"marvell,rd-88f5182-nas" +"maxtor,shared-storage-2" -- 2.8.1