Re: [PATCH] greybus: audio_manager: fix a missing check of ida_simple_get
On Thu, Mar 14, 2019 at 12:15 PM Kangjie Lu wrote:
>
> ida_simple_get could fail. The fix inserts a check for its
> return value.
>
> Signed-off-by: Kangjie Lu
> ---
> drivers/staging/greybus/audio_manager.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/staging/greybus/audio_manager.c
> b/drivers/staging/greybus/audio_manager.c
> index d44b070d8862..c2a4af4c1d06 100644
> --- a/drivers/staging/greybus/audio_manager.c
> +++ b/drivers/staging/greybus/audio_manager.c
> @@ -45,6 +45,9 @@ int gb_audio_manager_add(struct
> gb_audio_manager_module_descriptor *desc)
> int err;
>
> id = ida_simple_get(_id, 0, 0, GFP_KERNEL);
> + if (id < 0)
> + return id;
> +
> err = gb_audio_manager_module_create(, manager_kset,
> id, desc);
> if (err) {
Reviewed-by: Vaibhav Agarwal
--
./va
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
On Thu, 2019-03-14 at 15:07 +0100, Jean Delvare wrote: > My principle is that if a script > is present in the kernel tree then it can and should be maintained. If > it is deemed not worth the maintenance effort then it should be > deleted. I've suggested deleting Lindent in the past. https://lkml.org/lkml/2013/2/11/390 Also there is now the clang-format tool that does an OK job of reflowing source to something approximating the typical kernel style. See: Documentation/process/clang-format.rst ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] simple_lmk: Introduce Simple Low Memory Killer for Android
On Thu, Mar 14, 2019 at 11:16:41PM -0400, Steven Rostedt wrote: > How would you implement such a method in userspace? kill() doesn't take > any parameters but the pid of the process you want to send a signal to, > and the signal to send. This would require a new system call, and be > quite a bit of work. If you can solve this with an ebpf program, I > strongly suggest you do that instead. This can be done by introducing a new signal number that provides SIGKILL functionality while blocking (maybe SIGKILLBLOCK?). Thanks, Sultan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] simple_lmk: Introduce Simple Low Memory Killer for Android
On Thu, Mar 14, 2019 at 10:54:48PM -0400, Joel Fernandes wrote: > I'm not sure if that makes much semantic sense for how the signal handling is > supposed to work. Imagine a parent sends SIGKILL to its child, and then does > a wait(2). Because the SIGKILL blocks in your idea, then the wait cannot > execute, and because the wait cannot execute, the zombie task will not get > reaped and so the SIGKILL senders never gets unblocked and the whole thing > just gets locked up. No? I don't know it just feels incorrect. Block until the victim becomes a zombie instead. > Further, in your idea adding stuff to task_struct will simply bloat it - when > this task can easily be handled using eBPF without making any kernel changes. > Either by probing sched_process_free or sched_process_exit tracepoints. > Scheduler maintainers generally frown on adding stuff to task_struct > pointlessly there's a good reason since bloating it effects the performance > etc, and something like this would probably never be ifdef'd out behind a > CONFIG. Adding something to task_struct is just the easiest way to test things for experimentation. This can be avoided in my suggestion by passing the pointer to a completion via the relevant functions, and then completing it at the time the victim transitions to a zombie state. I understand it's possible to use eBPF for this, but it seems kind of messy since this functionality is something that I think others would want provided by the kernel (i.e., anyone using PSI to implement their own OOM killer daemon similar to LMKD). Thanks, Sultan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] simple_lmk: Introduce Simple Low Memory Killer for Android
On Thu, 14 Mar 2019 13:49:11 -0700 Sultan Alsawaf wrote: > Perhaps I'm missing something, but if you want to know when a process has died > after sending a SIGKILL to it, then why not just make the SIGKILL optionally > block until the process has died completely? It'd be rather trivial to just > store a pointer to an onstack completion inside the victim process' > task_struct, > and then complete it in free_task(). How would you implement such a method in userspace? kill() doesn't take any parameters but the pid of the process you want to send a signal to, and the signal to send. This would require a new system call, and be quite a bit of work. If you can solve this with an ebpf program, I strongly suggest you do that instead. -- Steve ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] simple_lmk: Introduce Simple Low Memory Killer for Android
On Thu, Mar 14, 2019 at 01:49:11PM -0700, Sultan Alsawaf wrote: > On Thu, Mar 14, 2019 at 10:47:17AM -0700, Joel Fernandes wrote: > > About the 100ms latency, I wonder whether it is that high because of > > the way Android's lmkd is observing that a process has died. There is > > a gap between when a process memory is freed and when it disappears > > from the process-table. Once a process is SIGKILLed, it becomes a > > zombie. Its memory is freed instantly during the SIGKILL delivery (I > > traced this so that's how I know), but until it is reaped by its > > parent thread, it will still exist in /proc/ . So if testing the > > existence of /proc/ is how Android is observing that the process > > died, then there can be a large latency where it takes a very long > > time for the parent to actually reap the child way after its memory > > was long freed. A quicker way to know if a process's memory is freed > > before it is reaped could be to read back /proc//maps in > > userspace of the victim , and that file will be empty for zombie > > processes. So then one does not need wait for the parent to reap it. I > > wonder how much of that 100ms you mentioned is actually the "Waiting > > while Parent is reaping the child", than "memory freeing time". So > > yeah for this second problem, the procfds work will help. > > > > By the way another approach that can provide a quick and asynchronous > > notification of when the process memory is freed, is to monitor > > sched_process_exit trace event using eBPF. You can tell eBPF the PID > > that you want to monitor before the SIGKILL. As soon as the process > > dies and its memory is freed, the eBPF program can send a notification > > to user space (using the perf_events polling infra). The > > sched_process_exit fires just after the mmput() happens so it is quite > > close to when the memory is reclaimed. This also doesn't need any > > kernel changes. I could come up with a prototype for this and > > benchmark it on Android, if you want. Just let me know. > > Perhaps I'm missing something, but if you want to know when a process has died > after sending a SIGKILL to it, then why not just make the SIGKILL optionally > block until the process has died completely? It'd be rather trivial to just > store a pointer to an onstack completion inside the victim process' > task_struct, > and then complete it in free_task(). I'm not sure if that makes much semantic sense for how the signal handling is supposed to work. Imagine a parent sends SIGKILL to its child, and then does a wait(2). Because the SIGKILL blocks in your idea, then the wait cannot execute, and because the wait cannot execute, the zombie task will not get reaped and so the SIGKILL senders never gets unblocked and the whole thing just gets locked up. No? I don't know it just feels incorrect. Further, in your idea adding stuff to task_struct will simply bloat it - when this task can easily be handled using eBPF without making any kernel changes. Either by probing sched_process_free or sched_process_exit tracepoints. Scheduler maintainers generally frown on adding stuff to task_struct pointlessly there's a good reason since bloating it effects the performance etc, and something like this would probably never be ifdef'd out behind a CONFIG. thanks, - Joel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] driver : staging : ion: optimization for decreasing memory fragmentaion
Hi All,
I would like to explain more for the purpose of this patch. During our
daily test on android system, we found that there is always showing
the memory fragmentation after some test cases(etc camera snapshot),
which lead to huge amount of order0 pages which other page blocks
remain none(200M order0 page blocks in a 1G RAM system). By analysing
the DDR dump file, we can find that it is the ION SYSTEM HEAP which
cut the whole memory into pieces. That is to say, Burst huge order0
allocation is alike a knife that cut higher order blocks into small
one and can not back to original buddy somehow.
On Thu, Mar 14, 2019 at 7:12 PM Zhaoyang Huang wrote:
>
> From: Zhaoyang Huang
>
> Two action for this patch:
> 1. set a batch size for system heap's shrinker, which can have it buffer
> reasonable page blocks in pool for future allocation.
> 2. reverse the order sequence when free page blocks, the purpose is also
> to have system heap keep as more big blocks as it can.
>
> By testing on an android system with 2G RAM, the changes with setting
> batch = 48MB can help reduce the fragmentation obviously and improve
> big block allocation speed for 15%.
>
> Signed-off-by: Zhaoyang Huang
> ---
> drivers/staging/android/ion/ion_heap.c| 12 +++-
> drivers/staging/android/ion/ion_system_heap.c | 2 +-
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion_heap.c
> b/drivers/staging/android/ion/ion_heap.c
> index 31db510..68aa970 100644
> --- a/drivers/staging/android/ion/ion_heap.c
> +++ b/drivers/staging/android/ion/ion_heap.c
> @@ -16,6 +16,8 @@
> #include
> #include "ion.h"
>
> +unsigned long ion_heap_batch;
> +
> void *ion_heap_map_kernel(struct ion_heap *heap,
> struct ion_buffer *buffer)
> {
> @@ -303,7 +305,15 @@ int ion_heap_init_shrinker(struct ion_heap *heap)
> heap->shrinker.count_objects = ion_heap_shrink_count;
> heap->shrinker.scan_objects = ion_heap_shrink_scan;
> heap->shrinker.seeks = DEFAULT_SEEKS;
> - heap->shrinker.batch = 0;
> + heap->shrinker.batch = ion_heap_batch;
>
> return register_shrinker(>shrinker);
> }
> +
> +static int __init ion_system_heap_batch_init(char *arg)
> +{
> +ion_heap_batch = memparse(arg, NULL);
> +
> + return 0;
> +}
> +early_param("ion_batch", ion_system_heap_batch_init);
> diff --git a/drivers/staging/android/ion/ion_system_heap.c
> b/drivers/staging/android/ion/ion_system_heap.c
> index 701eb9f..d249f8d 100644
> --- a/drivers/staging/android/ion/ion_system_heap.c
> +++ b/drivers/staging/android/ion/ion_system_heap.c
> @@ -182,7 +182,7 @@ static int ion_system_heap_shrink(struct ion_heap *heap,
> gfp_t gfp_mask,
> if (!nr_to_scan)
> only_scan = 1;
>
> - for (i = 0; i < NUM_ORDERS; i++) {
> + for (i = NUM_ORDERS - 1; i >= 0; i--) {
> pool = sys_heap->pools[i];
>
> if (only_scan) {
> --
> 1.9.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] simple_lmk: Introduce Simple Low Memory Killer for Android
On Thu, Mar 14, 2019 at 10:47:17AM -0700, Joel Fernandes wrote: > About the 100ms latency, I wonder whether it is that high because of > the way Android's lmkd is observing that a process has died. There is > a gap between when a process memory is freed and when it disappears > from the process-table. Once a process is SIGKILLed, it becomes a > zombie. Its memory is freed instantly during the SIGKILL delivery (I > traced this so that's how I know), but until it is reaped by its > parent thread, it will still exist in /proc/ . So if testing the > existence of /proc/ is how Android is observing that the process > died, then there can be a large latency where it takes a very long > time for the parent to actually reap the child way after its memory > was long freed. A quicker way to know if a process's memory is freed > before it is reaped could be to read back /proc//maps in > userspace of the victim , and that file will be empty for zombie > processes. So then one does not need wait for the parent to reap it. I > wonder how much of that 100ms you mentioned is actually the "Waiting > while Parent is reaping the child", than "memory freeing time". So > yeah for this second problem, the procfds work will help. > > By the way another approach that can provide a quick and asynchronous > notification of when the process memory is freed, is to monitor > sched_process_exit trace event using eBPF. You can tell eBPF the PID > that you want to monitor before the SIGKILL. As soon as the process > dies and its memory is freed, the eBPF program can send a notification > to user space (using the perf_events polling infra). The > sched_process_exit fires just after the mmput() happens so it is quite > close to when the memory is reclaimed. This also doesn't need any > kernel changes. I could come up with a prototype for this and > benchmark it on Android, if you want. Just let me know. Perhaps I'm missing something, but if you want to know when a process has died after sending a SIGKILL to it, then why not just make the SIGKILL optionally block until the process has died completely? It'd be rather trivial to just store a pointer to an onstack completion inside the victim process' task_struct, and then complete it in free_task(). Thanks, Sultan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC] simple_lmk: Introduce Simple Low Memory Killer for Android
Hi Tim, Thanks for the detailed and excellent write-up. It will serve as a good future reference for low memory killer requirements. I made some comments below on the "how to kill" part. On Tue, Mar 12, 2019 at 10:17 AM Tim Murray wrote: > > On Tue, Mar 12, 2019 at 9:37 AM Sultan Alsawaf wrote: > > > > On Tue, Mar 12, 2019 at 09:05:32AM +0100, Michal Hocko wrote: > > > The only way to control the OOM behavior pro-actively is to throttle > > > allocation speed. We have memcg high limit for that purpose. Along with > > > PSI, I can imagine a reasonably working user space early oom > > > notifications and reasonable acting upon that. > > > > The issue with pro-active memory management that prompted me to create this > > was > > poor memory utilization. All of the alternative means of reclaiming pages > > in the > > page allocator's slow path turn out to be very useful for maximizing memory > > utilization, which is something that we would have to forgo by relying on a > > purely pro-active solution. I have not had a chance to look at PSI yet, but > > unless a PSI-enabled solution allows allocations to reach the same point as > > when > > the OOM killer is invoked (which is contradictory to what it sets out to > > do), > > then it cannot take advantage of all of the alternative memory-reclaim means > > employed in the slowpath, and will result in killing a process before it is > > _really_ necessary. > > There are two essential parts of a lowmemorykiller implementation: > when to kill and how to kill. > > There are a million possible approaches to decide when to kill an > unimportant process. They usually trade off between the same two > failure modes depending on the workload. > > If you kill too aggressively, a transient spike that could be > imperceptibly absorbed by evicting some file pages or moving some > pages to ZRAM will result in killing processes, which then get started > up later and have a performance/battery cost. > > If you don't kill aggressively enough, you will encounter a workload > that thrashes the page cache, constantly evicting and reloading file > pages and moving things in and out of ZRAM, which makes the system > unusable when a process should have been killed instead. > > As far as I've seen, any methodology that uses single points in time > to decide when to kill without completely biasing toward one or the > other is susceptible to both. The minfree approach used by > lowmemorykiller/lmkd certainly is; it is both too aggressive for some > workloads and not aggressive enough for other workloads. My guess is > that simple LMK won't kill on transient spikes but will be extremely > susceptible to page cache thrashing. This is not an improvement; page > cache thrashing manifests as the entire system running very slowly. > > What you actually want from lowmemorykiller/lmkd on Android is to only > kill once it becomes clear that the system will continue to try to > reclaim memory to the extent that it could impact what the user > actually cares about. That means tracking how much time is spent in > reclaim/paging operations and the like, and that's exactly what PSI > does. lmkd has had support for using PSI as a replacement for > vmpressure for use as a wakeup trigger (to check current memory levels > against the minfree thresholds) since early February. It works fine; > unsurprisingly it's better than vmpressure at avoiding false wakeups. > > Longer term, there's a lot of work to be done in lmkd to turn PSI into > a kill trigger and remove minfree entirely. It's tricky (mainly > because of the "when to kill another process" problem discussed > later), but I believe it's feasible. > > How to kill is similarly messy. The latency of reclaiming memory post > SIGKILL can be severe (usually tens of milliseconds, occasionally > >100ms). The latency we see on Android usually isn't because those > threads are blocked in uninterruptible sleep, it's because times of > memory pressure are also usually times of significant CPU contention > and these are overwhelmingly CFS threads, some of which may be > assigned a very low priority. lmkd now sets priorities and resets > cpusets upon killing a process, and we have seen improved reclaim > latency because of this. oom reaper might be a good approach to avoid > this latency (I think some in-kernel lowmemorykiller implementations > rely on it), but we can't use it from userspace. Something for future > consideration. > This makes sense. If the process receiving the SIGKILL does not get CPU time, then the kernel may not be able to execute the unconditional signal handling paths in the context of the victim process to free the memory. I don't see how proc-fds approach will solve this though. Say you have process L (which is LMKd) which sends a SIGKILL to process V(which is a victim). Now L sends SIGKILL to V. Unless V executes the signal-handling code in kernel context and is scheduled at high enough priority to get CPU time, I don't think the
Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
On 14/03/2019 14:24, Stefan Roese wrote: > On 14.03.19 14:14, Matthias Brugger wrote: >> >> >> On 14/03/2019 12:37, Armando Miraglia wrote: >>> Absolutely! >> >> Please don't top post :) >> >>> >>> Cheers, >>> A. >>> >>> On Thu, Mar 14, 2019 at 12:36 PM Stefan Roese wrote: >> [...] Would it be possible for you to wait a bit with this minor cleanup? As I'm preparing a patch to move this driver out of staging right now. You can definitely follow-up with your cleanup, once this move is done. Otherwise the move might be delayed even more. >> >> Hm but shouldn't style issues be a criteria for not accepting a move out of >> staging? > > I would agree, if those style issues where non trivial. In the end we > are talking about one non-optimal identation now.> Fair enough, anyway I'm not the person to decide on that :) Regards, Matthias >> I think so. You could add Armandos patch in your series or rebase your >> series against Greg's tree, once he took the clean-up. Normally Greg is >> incredibly fast :) > > I should have included the history here to make this more clean. I've > started pulling this driver out of staging a few weeks ago: > > https://patchwork.kernel.org/patch/10790537/ > ... > > Here you find Greg's comment, that the style patches should be merged > first before the move out of staging. This is what I worked on after > this first patch series: > > https://patchwork.kernel.org/patch/10792455/ > ... > > Now these 9 style issue patches from me have been merged and I would > like to proceed with the driver move. > > Thanks, > Stefan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 19/38] vfs: Convert binderfs to fs_context
Signed-off-by: David Howells
Reviewed-by: Christian Brauner
cc: Greg Kroah-Hartman
cc: "Arve Hjønnevåg"
cc: Todd Kjos
cc: Martijn Coenen
cc: Joel Fernandes
cc: de...@driverdev.osuosl.org
---
drivers/android/binderfs.c | 173
1 file changed, 96 insertions(+), 77 deletions(-)
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index e773f45d19d9..0a16ecc9594f 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -18,7 +18,8 @@
#include
#include
#include
-#include
+#include
+#include
#include
#include
#include
@@ -48,22 +49,18 @@ static dev_t binderfs_dev;
static DEFINE_MUTEX(binderfs_minors_mutex);
static DEFINE_IDA(binderfs_minors);
-/**
- * binderfs_mount_opts - mount options for binderfs
- * @max: maximum number of allocatable binderfs binder devices
- */
-struct binderfs_mount_opts {
- int max;
-};
-
enum {
Opt_max,
- Opt_err
};
-static const match_table_t tokens = {
- { Opt_max, "max=%d" },
- { Opt_err, NULL }
+static const struct fs_parameter_spec binderfs_param_specs[] = {
+ fsparam_s32 ("max", Opt_max),
+ {}
+};
+
+static const struct fs_parameter_description binderfs_fs_parameters = {
+ .name = "binderfs",
+ .specs = binderfs_param_specs,
};
/**
@@ -75,7 +72,7 @@ static const match_table_t tokens = {
* created.
* @root_gid: gid that needs to be used when a new binder device is
* created.
- * @mount_opts: The mount options in use.
+ * @max: Maximum number of allocatable binderfs binder devices.
* @device_count: The current number of allocated binder devices.
*/
struct binderfs_info {
@@ -83,7 +80,7 @@ struct binderfs_info {
struct dentry *control_dentry;
kuid_t root_uid;
kgid_t root_gid;
- struct binderfs_mount_opts mount_opts;
+ int max;
int device_count;
};
@@ -138,7 +135,7 @@ static int binderfs_binder_device_create(struct inode
*ref_inode,
/* Reserve new minor number for the new device. */
mutex_lock(_minors_mutex);
- if (++info->device_count <= info->mount_opts.max)
+ if (++info->device_count <= info->max)
minor = ida_alloc_max(_minors,
use_reserve ? BINDERFS_MAX_MINOR :
BINDERFS_MAX_MINOR_CAPPED,
@@ -285,46 +282,36 @@ static void binderfs_evict_inode(struct inode *inode)
}
/**
- * binderfs_parse_mount_opts - parse binderfs mount options
- * @data: options to set (can be NULL in which case defaults are used)
+ * binderfs_parse_param - parse a binderfs mount option
+ * @fc: The context to be configured
+ * @param: The parameter to apply
*/
-static int binderfs_parse_mount_opts(char *data,
-struct binderfs_mount_opts *opts)
+static int binderfs_parse_param(struct fs_context *fc, struct fs_parameter
*param)
{
- char *p;
- opts->max = BINDERFS_MAX_MINOR;
-
- while ((p = strsep(, ",")) != NULL) {
- substring_t args[MAX_OPT_ARGS];
- int token;
- int max_devices;
-
- if (!*p)
- continue;
-
- token = match_token(p, tokens, args);
- switch (token) {
- case Opt_max:
- if (match_int([0], _devices) ||
- (max_devices < 0 ||
-(max_devices > BINDERFS_MAX_MINOR)))
- return -EINVAL;
-
- opts->max = max_devices;
- break;
- default:
- pr_err("Invalid mount options\n");
- return -EINVAL;
- }
+ struct fs_parse_result result;
+ struct binderfs_info *info = fc->s_fs_info;
+ int opt;
+
+ opt = fs_parse(fc, _fs_parameters, param, );
+ if (opt < 0)
+ return opt;
+
+ switch (opt) {
+ case Opt_max:
+ info->max = result.int_32;
+ break;
}
return 0;
}
-static int binderfs_remount(struct super_block *sb, int *flags, char *data)
+static int binderfs_reconfigure(struct fs_context *fc)
{
- struct binderfs_info *info = sb->s_fs_info;
- return binderfs_parse_mount_opts(data, >mount_opts);
+ struct binderfs_info *info = fc->root->d_sb->s_fs_info;
+ struct binderfs_info *cfg = fc->s_fs_info;
+
+ info->max = cfg->max;
+ return 0;
}
static int binderfs_show_mount_opts(struct seq_file *seq, struct dentry *root)
@@ -332,15 +319,14 @@ static int binderfs_show_mount_opts(struct seq_file *seq,
struct dentry *root)
struct binderfs_info *info;
info = root->d_sb->s_fs_info;
- if (info->mount_opts.max <= BINDERFS_MAX_MINOR)
-
Re: [PATCH] staging: vboxvideo: fix vbox_dumb_create fail logic
Hi,
On 14-03-19 16:21, Sidong Yang wrote:
In function vbox_dumb_create() of vbox_main.c, It calls vbox_gem_create()
for creating drm_gem_object. and it calls vbox_gem_handle_create() for handle.
If handle creation fails only, drm_gem_object should be released by calling
drm_gem_object_put_unlocked().
Nack.
On success drm_gem_handle_create does a drm_gem_object_get on success, since
we forgot about the gem_object afterwards (we only keep track of the
gem_handle with its embedded reference), we should release our
gem_object_reference on success, so that when the gem_handle gets free-ed
the gem_object also gets free-ed.
So we should release out reference on the gem_object in both the
success and failure paths and the old code is correct.
Also for drm-drivers, you should always Cc the dri-devel list.
Regards,
Hans
p.s.
In drm-misc-next vboxvideo has been moved out of staging, please base any
further vboxvideo patches on top of drm-misc-next.
Signed-off-by: Sidong Yang > ---
drivers/staging/vboxvideo/vbox_main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_main.c
b/drivers/staging/vboxvideo/vbox_main.c
index e1fb70a42d32..ca676ba37bb4 100644
--- a/drivers/staging/vboxvideo/vbox_main.c
+++ b/drivers/staging/vboxvideo/vbox_main.c
@@ -312,9 +312,10 @@ int vbox_dumb_create(struct drm_file *file,
return ret;
ret = drm_gem_handle_create(file, gobj, );
- drm_gem_object_put_unlocked(gobj);
- if (ret)
+ if (ret) {
+ drm_gem_object_put_unlocked(gobj);
return ret;
+ }
args->handle = handle;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vboxvideo: fix vbox_dumb_create fail logic
In function vbox_dumb_create() of vbox_main.c, It calls vbox_gem_create()
for creating drm_gem_object. and it calls vbox_gem_handle_create() for handle.
If handle creation fails only, drm_gem_object should be released by calling
drm_gem_object_put_unlocked().
Signed-off-by: Sidong Yang
---
drivers/staging/vboxvideo/vbox_main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_main.c
b/drivers/staging/vboxvideo/vbox_main.c
index e1fb70a42d32..ca676ba37bb4 100644
--- a/drivers/staging/vboxvideo/vbox_main.c
+++ b/drivers/staging/vboxvideo/vbox_main.c
@@ -312,9 +312,10 @@ int vbox_dumb_create(struct drm_file *file,
return ret;
ret = drm_gem_handle_create(file, gobj, );
- drm_gem_object_put_unlocked(gobj);
- if (ret)
+ if (ret) {
+ drm_gem_object_put_unlocked(gobj);
return ret;
+ }
args->handle = handle;
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
Hi Dan, On Thu, 14 Mar 2019 14:27:32 +0300, Dan Carpenter wrote: > On Thu, Mar 14, 2019 at 12:13:15PM +0100, Armando Miraglia wrote: > > Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl > > --fix? If one would like to contribute to fixing the tooling for linting > > which > > of the two would be the right target for such an effort? > > I've added Jean to the CC list because he worked on Lindent a few > years ago. I really think we should just delete Lindent. I haven't > used the checkpatch.pl --fix option but Joe Perches has good style. I merely fixed obvious but minor issues in Lindent as I stumbled upon them the one time in my life I used that script. That was years ago. I'm not using it on a regular basis. My principle is that if a script is present in the kernel tree then it can and should be maintained. If it is deemed not worth the maintenance effort then it should be deleted. I don't care either way. -- Jean Delvare SUSE L3 Support ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
On 14.03.19 14:14, Matthias Brugger wrote: On 14/03/2019 12:37, Armando Miraglia wrote: Absolutely! Please don't top post :) Cheers, A. On Thu, Mar 14, 2019 at 12:36 PM Stefan Roese wrote: [...] Would it be possible for you to wait a bit with this minor cleanup? As I'm preparing a patch to move this driver out of staging right now. You can definitely follow-up with your cleanup, once this move is done. Otherwise the move might be delayed even more. Hm but shouldn't style issues be a criteria for not accepting a move out of staging? I would agree, if those style issues where non trivial. In the end we are talking about one non-optimal identation now. I think so. You could add Armandos patch in your series or rebase your series against Greg's tree, once he took the clean-up. Normally Greg is incredibly fast :) I should have included the history here to make this more clean. I've started pulling this driver out of staging a few weeks ago: https://patchwork.kernel.org/patch/10790537/ ... Here you find Greg's comment, that the style patches should be merged first before the move out of staging. This is what I worked on after this first patch series: https://patchwork.kernel.org/patch/10792455/ ... Now these 9 style issue patches from me have been merged and I would like to proceed with the driver move. Thanks, Stefan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
MT7621 PCIe PHY
This series adds support for the PCIe PHY found in the Mediatek MT7621 SoC. This is the first attempt to get feedback of what is missing in this driver to be promoted from staging. There is also a 'mt7621-pci' driver which is the controller part which is still in staging and is a client of this phy. Both drivers have been tested together in a gnubee1 board. Thanks in advance for your time. Best regards, Sergio Paracuellos Sergio Paracuellos (2): phy: ralink: Add PHY driver for MT7621 PCIe PHY dt-bindings: phy: Add binding for Mediatek MT7621 PCIe PHY .../bindings/phy/mediatek,mt7621-pci-phy.txt | 54 +++ drivers/phy/ralink/Kconfig| 7 + drivers/phy/ralink/Makefile | 1 + drivers/phy/ralink/phy-mt7621-pci.c | 387 ++ 4 files changed, 449 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.txt create mode 100644 drivers/phy/ralink/phy-mt7621-pci.c -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] phy: ralink: Add PHY driver for MT7621 PCIe PHY
This patch adds a driver for the PCIe PHY of MT7621 SoC.
Signed-off-by: Sergio Paracuellos
---
drivers/phy/ralink/Kconfig | 7 +
drivers/phy/ralink/Makefile | 1 +
drivers/phy/ralink/phy-mt7621-pci.c | 387
3 files changed, 395 insertions(+)
create mode 100644 drivers/phy/ralink/phy-mt7621-pci.c
diff --git a/drivers/phy/ralink/Kconfig b/drivers/phy/ralink/Kconfig
index 14fd219535ef..87943a10f210 100644
--- a/drivers/phy/ralink/Kconfig
+++ b/drivers/phy/ralink/Kconfig
@@ -1,6 +1,13 @@
#
# PHY drivers for Ralink platforms.
#
+config PHY_MT7621_PCI
+ tristate "MediaTek MT7621 PCI PHY Driver"
+ depends on RALINK && OF
+ select GENERIC_PHY
+ help
+ Say 'Y' here to add support for MediaTek MT7621 PCI PHY driver,
+
config PHY_RALINK_USB
tristate "Ralink USB PHY driver"
depends on RALINK || COMPILE_TEST
diff --git a/drivers/phy/ralink/Makefile b/drivers/phy/ralink/Makefile
index 5c9e326e8757..2052d5649863 100644
--- a/drivers/phy/ralink/Makefile
+++ b/drivers/phy/ralink/Makefile
@@ -1 +1,2 @@
+obj-$(CONFIG_PHY_MT7621_PCI) += phy-mt7621-pci.o
obj-$(CONFIG_PHY_RALINK_USB) += phy-ralink-usb.o
diff --git a/drivers/phy/ralink/phy-mt7621-pci.c
b/drivers/phy/ralink/phy-mt7621-pci.c
new file mode 100644
index ..d3ca2f019112
--- /dev/null
+++ b/drivers/phy/ralink/phy-mt7621-pci.c
@@ -0,0 +1,387 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Mediatek MT7621 PCI PHY Driver
+ * Author: Sergio Paracuellos
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define RALINK_CLKCFG1 0x30
+#define CHIP_REV_MT7621_E2 0x0101
+
+#define PCIE_PORT_CLK_EN(x)BIT(24 + (x))
+
+#define RG_PE1_PIPE_REG0x02c
+#define RG_PE1_PIPE_RSTBIT(12)
+#define RG_PE1_PIPE_CMD_FRCBIT(4)
+
+#define RG_P0_TO_P1_WIDTH 0x100
+#define RG_PE1_H_LCDDS_REG 0x49c
+#define RG_PE1_H_LCDDS_PCW GENMASK(30, 0)
+#define RG_PE1_H_LCDDS_PCW_VAL(x) ((0x7fff & (x)) << 0)
+
+#define RG_PE1_FRC_H_XTAL_REG 0x400
+#define RG_PE1_FRC_H_XTAL_TYPE BIT(8)
+#define RG_PE1_H_XTAL_TYPE GENMASK(10, 9)
+#define RG_PE1_H_XTAL_TYPE_VAL(x) ((0x3 & (x)) << 9)
+
+#define RG_PE1_FRC_PHY_REG 0x000
+#define RG_PE1_FRC_PHY_EN BIT(4)
+#define RG_PE1_PHY_EN BIT(5)
+
+#define RG_PE1_H_PLL_REG 0x490
+#define RG_PE1_H_PLL_BCGENMASK(23, 22)
+#define RG_PE1_H_PLL_BC_VAL(x) ((0x3 & (x)) << 22)
+#define RG_PE1_H_PLL_BPGENMASK(21, 18)
+#define RG_PE1_H_PLL_BP_VAL(x) ((0xf & (x)) << 18)
+#define RG_PE1_H_PLL_IRGENMASK(15, 12)
+#define RG_PE1_H_PLL_IR_VAL(x) ((0xf & (x)) << 12)
+#define RG_PE1_H_PLL_ICGENMASK(11, 8)
+#define RG_PE1_H_PLL_IC_VAL(x) ((0xf & (x)) << 8)
+#define RG_PE1_H_PLL_PREDIV GENMASK(7, 6)
+#define RG_PE1_H_PLL_PREDIV_VAL(x) ((0x3 & (x)) << 6)
+#define RG_PE1_PLL_DIVEN GENMASK(3, 1)
+#define RG_PE1_PLL_DIVEN_VAL(x)((0x7 & (x)) << 1)
+
+#define RG_PE1_H_PLL_FBKSEL_REG0x4bc
+#define RG_PE1_H_PLL_FBKSEL GENMASK(5, 4)
+#define RG_PE1_H_PLL_FBKSEL_VAL(x) ((0x3 & (x)) << 4)
+
+#defineRG_PE1_H_LCDDS_SSC_PRD_REG 0x4a4
+#define RG_PE1_H_LCDDS_SSC_PRD GENMASK(15, 0)
+#define RG_PE1_H_LCDDS_SSC_PRD_VAL(x) ((0x & (x)) << 0)
+
+#define RG_PE1_H_LCDDS_SSC_DELTA_REG 0x4a8
+#define RG_PE1_H_LCDDS_SSC_DELTAGENMASK(11, 0)
+#define RG_PE1_H_LCDDS_SSC_DELTA_VAL(x) ((0xfff & (x)) << 0)
+#define RG_PE1_H_LCDDS_SSC_DELTA1 GENMASK(27, 16)
+#define RG_PE1_H_LCDDS_SSC_DELTA1_VAL(x) ((0xff & (x)) << 16)
+
+#define RG_PE1_LCDDS_CLK_PH_INV_REG0x4a0
+#define RG_PE1_LCDDS_CLK_PH_INVBIT(5)
+
+#define RG_PE1_H_PLL_BR_REG0x4ac
+#define RG_PE1_H_PLL_BRGENMASK(18, 16)
+#define RG_PE1_H_PLL_BR_VAL(x) ((0x7 & (x)) << 16)
+
+#defineRG_PE1_MSTCKDIV_REG 0x414
+#define RG_PE1_MSTCKDIVGENMASK(7, 6)
+#define RG_PE1_MSTCKDIV_VAL(x) ((0x3 & (x)) << 6)
+
+#define RG_PE1_FRC_MSTCKDIVBIT(5)
+
+/**
+ * struct mt7621_pci_phy_instance - Mt7621 Pcie PHY device
+ * @phy: pointer to the kernel PHY device
+ * @port_base: base register
+ * @index: internal ID to identify the Mt7621 PCIe PHY
+ */
+struct mt7621_pci_phy_instance {
+ struct phy *phy;
+ void __iomem *port_base;
+ u32 index;
+};
+
+/**
+ * struct mt7621_pci_phy - Mt7621 Pcie PHY core
+ * @dev: pointer to device
+ * @phys: pointer to Mt7621 PHY device
+ * @nphys: number of PHY devices for this core
+ */
+struct mt7621_pci_phy {
+ struct device *dev;
+ struct
[PATCH 2/2] dt-bindings: phy: Add binding for Mediatek MT7621 PCIe PHY
Add bindings to describe Mediatek MT7621 PCIe PHY.
Signed-off-by: Sergio Paracuellos
---
.../bindings/phy/mediatek,mt7621-pci-phy.txt | 54 +++
1 file changed, 54 insertions(+)
create mode 100644
Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.txt
diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.txt
b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.txt
new file mode 100644
index ..8addedbe815e
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.txt
@@ -0,0 +1,54 @@
+Mediatek Mt7621 PCIe PHY
+
+Required properties:
+- compatible: must be "mediatek,mt7621-pci-phy"
+- reg: base address and length of the PCIe PHY block
+- #address-cells: must be 1
+- #size-cells: must be 0
+
+Each PCIe PHY should be represented by a child node
+
+Required properties For the child node:
+- reg: the PHY ID
+0 - PCIe RC 0
+1 - PCIe RC 1
+- #phy-cells: must be 0
+
+Example:
+ pcie0_phy: pcie-phy@1e149000 {
+ compatible = "mediatek,mt7621-pci-phy";
+ reg = <0x1e149000 0x0700>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pcie0_port: pcie-phy@0 {
+ reg = <0>;
+ #phy-cells = <0>;
+ };
+
+ pcie1_port: pcie-phy@1 {
+ reg = <1>;
+ #phy-cells = <0>;
+ };
+ };
+
+ pcie1_phy: pcie-phy@1e14a000 {
+ compatible = "mediatek,mt7621-pci-phy";
+ reg = <0x1e14a000 0x0700>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pcie2_port: pcie-phy@0 {
+ reg = <0>;
+ #phy-cells = <0>;
+ };
+ };
+
+ /* users of the PCIe phy */
+
+ pcie: pcie@1e14 {
+ ...
+ ...
+ phys = <_port>, <_port>, <_port>;
+ phy-names = "pcie-phy0", "pcie-phy1", "pcie-phy2";
+ };
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
On 14/03/2019 12:37, Armando Miraglia wrote: > Absolutely! Please don't top post :) > > Cheers, > A. > > On Thu, Mar 14, 2019 at 12:36 PM Stefan Roese wrote: [...] >> >> Would it be possible for you to wait a bit with this minor cleanup? >> As I'm preparing a patch to move this driver out of staging right >> now. You can definitely follow-up with your cleanup, once this move >> is done. Otherwise the move might be delayed even more. >> Hm but shouldn't style issues be a criteria for not accepting a move out of staging? I think so. You could add Armandos patch in your series or rebase your series against Greg's tree, once he took the clean-up. Normally Greg is incredibly fast :) Regards, Matthias ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] spi: mt7621: Move SPI driver out of staging
This patch moves the MT7621 SPI driver, which is used on some Ralink /
MediaTek MT76xx MIPS SoC's, out of the staging directory. No changes to
the source code are done in this patch.
This driver version was tested successfully on an MT7688 based platform
with an SPI NOR on CS0 and an SPI NAND on CS1 without any issues (so
far).
This patch also documents the devicetree bindings for the MT7621 SPI
device driver.
Signed-off-by: Stefan Roese
Cc: Rob Herring
Cc: Mark Brown
Cc: Greg Kroah-Hartman
Cc: NeilBrown
Cc: Sankalp Negi
Cc: Chuanhong Guo
Cc: John Crispin
Cc: Armando Miraglia
---
.../devicetree/bindings/spi/spi-mt7621.txt| 26 ++
drivers/spi/Kconfig | 6 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-mt7621.c | 422 ++
drivers/staging/Kconfig | 2 -
drivers/staging/Makefile | 1 -
drivers/staging/mt7621-spi/Kconfig| 6 -
drivers/staging/mt7621-spi/Makefile | 1 -
drivers/staging/mt7621-spi/TODO | 5 -
drivers/staging/mt7621-spi/spi-mt7621.c | 422 --
10 files changed, 455 insertions(+), 437 deletions(-)
create mode 100644 Documentation/devicetree/bindings/spi/spi-mt7621.txt
create mode 100644 drivers/spi/spi-mt7621.c
delete mode 100644 drivers/staging/mt7621-spi/Kconfig
delete mode 100644 drivers/staging/mt7621-spi/Makefile
delete mode 100644 drivers/staging/mt7621-spi/TODO
delete mode 100644 drivers/staging/mt7621-spi/spi-mt7621.c
diff --git a/Documentation/devicetree/bindings/spi/spi-mt7621.txt
b/Documentation/devicetree/bindings/spi/spi-mt7621.txt
new file mode 100644
index ..d5baec0fa56e
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-mt7621.txt
@@ -0,0 +1,26 @@
+Binding for MTK SPI controller (MT7621 MIPS)
+
+Required properties:
+- compatible: Should be one of the following:
+ - "ralink,mt7621-spi": for mt7621/mt7628/mt7688 platforms
+- #address-cells: should be 1.
+- #size-cells: should be 0.
+- reg: Address and length of the register set for the device
+- resets: phandle to the reset controller asserting this device in
+ reset
+ See ../reset/reset.txt for details.
+
+Optional properties:
+- cs-gpios: see spi-bus.txt.
+
+Example:
+
+- SoC Specific Portion:
+spi0: spi@b00 {
+ compatible = "ralink,mt7621-spi";
+ reg = <0xb00 0x100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ resets = < 18>;
+ reset-names = "spi";
+};
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index f761655e2a36..202952f9da56 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -426,6 +426,12 @@ config SPI_MT65XX
say Y or M here.If you are not sure, say N.
SPI drivers for Mediatek MT65XX and MT81XX series ARM SoCs.
+config SPI_MT7621
+ tristate "MediaTek MT7621 SPI Controller"
+ depends on RALINK
+ help
+ This selects a driver for the MediaTek MT7621 SPI Controller.
+
config SPI_NPCM_PSPI
tristate "Nuvoton NPCM PSPI Controller"
depends on ARCH_NPCM || COMPILE_TEST
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index d8fc03c9faa2..369f29a8a6af 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_SPI_MPC512x_PSC) += spi-mpc512x-psc.o
obj-$(CONFIG_SPI_MPC52xx_PSC) += spi-mpc52xx-psc.o
obj-$(CONFIG_SPI_MPC52xx) += spi-mpc52xx.o
obj-$(CONFIG_SPI_MT65XX)+= spi-mt65xx.o
+obj-$(CONFIG_SPI_MT7621) += spi-mt7621.o
obj-$(CONFIG_SPI_MXIC) += spi-mxic.o
obj-$(CONFIG_SPI_MXS) += spi-mxs.o
obj-$(CONFIG_SPI_NPCM_PSPI)+= spi-npcm-pspi.o
diff --git a/drivers/spi/spi-mt7621.c b/drivers/spi/spi-mt7621.c
new file mode 100644
index ..b509f9fe3346
--- /dev/null
+++ b/drivers/spi/spi-mt7621.c
@@ -0,0 +1,422 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * spi-mt7621.c -- MediaTek MT7621 SPI controller driver
+ *
+ * Copyright (C) 2011 Sergiy
+ * Copyright (C) 2011-2013 Gabor Juhos
+ * Copyright (C) 2014-2015 Felix Fietkau
+ *
+ * Some parts are based on spi-orion.c:
+ * Author: Shadi Ammouri
+ * Copyright (C) 2007-2008 Marvell Ltd.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DRIVER_NAME"spi-mt7621"
+
+/* in usec */
+#define RALINK_SPI_WAIT_MAX_LOOP 2000
+
+/* SPISTAT register bit field */
+#define SPISTAT_BUSY BIT(0)
+
+#define MT7621_SPI_TRANS 0x00
+#define SPITRANS_BUSY BIT(16)
+
+#define MT7621_SPI_OPCODE 0x04
+#define MT7621_SPI_DATA0 0x08
+#define MT7621_SPI_DATA4 0x18
+#define SPI_CTL_TX_RX_CNT_MASK 0xff
+#define SPI_CTL_START BIT(8)
+
+#define MT7621_SPI_MASTER 0x28
+#define MASTER_MORE_BUFMODEBIT(2)
+#define MASTER_FULL_DUPLEX BIT(10)
+#define MASTER_RS_CLK_SEL
Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
Absolutely!
Cheers,
A.
On Thu, Mar 14, 2019 at 12:36 PM Stefan Roese wrote:
>
> Hi Armando,
>
> On 14.03.19 12:13, Armando Miraglia wrote:
> > My answers are in-line below. BTW bare with me as this is my attempt to get
> > my
> > feet wet in how to contribute to the linux kernel for my own pleasure and
> > interest :)
> >
> > On Wed, Mar 13, 2019 at 03:34:54PM +0300, Dan Carpenter wrote:
> >> On Wed, Mar 13, 2019 at 01:24:04PM +0100, Armando Miraglia wrote:
> >>> Running Lindent on the mt7621-spi.c file in drivers/staging I noticed
> >>> that the
> >>> file contained style issues. This change attempts to address such style
> >>> problems.
> >>>
> >>
> >> Don't run lindent. I think checkpatch.pl has a --fix option that might
> >> be better, but once the code is merged then our standard become much
> >> higher for follow up patches.
> >>
> >>> Signed-off-by: Armando Miraglia
> >>> ---
> >>> NOTE: resend this patch to include all mainteners listed by
> >>> get_mantainers.pl.
> >>> drivers/staging/mt7621-spi/spi-mt7621.c | 27 +
> >>> 1 file changed, 14 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c
> >>> b/drivers/staging/mt7621-spi/spi-mt7621.c
> >>> index b509f9fe3346..03d53845f8c5 100644
> >>> --- a/drivers/staging/mt7621-spi/spi-mt7621.c
> >>> +++ b/drivers/staging/mt7621-spi/spi-mt7621.c
> >>> @@ -52,14 +52,14 @@
> >>> #define MT7621_LSB_FIRST BIT(3)
> >>>
> >>> struct mt7621_spi {
> >>> - struct spi_master *master;
> >>> - void __iomem*base;
> >>> - unsigned intsys_freq;
> >>> - unsigned intspeed;
> >>> - struct clk *clk;
> >>> - int pending_write;
> >>> -
> >>> - struct mt7621_spi_ops *ops;
> >>> + struct spi_master *master;
> >>> + void __iomem *base;
> >>> + unsigned int sys_freq;
> >>> + unsigned int speed;
> >>> + struct clk *clk;
> >>> + int pending_write;
> >>> +
> >>> + struct mt7621_spi_ops *ops;
> >>
> >> The original is fine. I don't encourage people to do fancy indenting
> >> with their local variable declarations inside functions but for a struct
> >> the declarations aren't going to change a lot so people can get fancy
> >> if they want.
> >>
> > Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
> > --fix? If one would like to contribute to fixing the tooling for linting
> > which
> > of the two would be the right target for such an effort?
> >
> >> The problem with a local is if you need to add a new variable then you
> >> have to re-indent a bunch of unrelated lines or have one out of
> >> alignment line. Most people know this intuitively so they don't get
> >> fancy.
> >>
> >>> };
> >>>
> >>> static inline struct mt7621_spi *spidev_to_mt7621_spi(struct spi_device
> >>> *spi)
> >>> @@ -303,7 +303,7 @@ static int mt7621_spi_setup(struct spi_device *spi)
> >>> struct mt7621_spi *rs = spidev_to_mt7621_spi(spi);
> >>>
> >>> if ((spi->max_speed_hz == 0) ||
> >>> - (spi->max_speed_hz > (rs->sys_freq / 2)))
> >>> + (spi->max_speed_hz > (rs->sys_freq / 2)))
> >>
> >> Yeah. Lindent is correct here.
> >
> > Funny enough, this is something I adjusted manually :)
> >
> >>> spi->max_speed_hz = (rs->sys_freq / 2);
> >>>
> >>> if (spi->max_speed_hz < (rs->sys_freq / 4097)) {
> >>> @@ -316,9 +316,10 @@ static int mt7621_spi_setup(struct spi_device *spi)
> >>> }
> >>>
> >>> static const struct of_device_id mt7621_spi_match[] = {
> >>> - { .compatible = "ralink,mt7621-spi" },
> >>> + {.compatible = "ralink,mt7621-spi"},
> >>
> >> The original was better.
> >>
> >>> {},
> >>> };
> >>> +
> >>> MODULE_DEVICE_TABLE(of, mt7621_spi_match);
> >>
> >> No need for a blank. These are closely related.
> >
> > Ack.
> >
> >>>
> >>> static int mt7621_spi_probe(struct platform_device *pdev)
> >>> @@ -408,9 +409,9 @@ MODULE_ALIAS("platform:" DRIVER_NAME);
> >>>
> >>> static struct platform_driver mt7621_spi_driver = {
> >>> .driver = {
> >>> - .name = DRIVER_NAME,
> >>> - .of_match_table = mt7621_spi_match,
> >>> - },
> >>> + .name = DRIVER_NAME,
> >>> + .of_match_table = mt7621_spi_match,
> >>> + },
> >>
> >> The new indenting is very wrong.
> >
> > Ack. In fact, I was thinking this could be one target to fix the logic in
> > Lindent to do this appropriately.
> >
> > I have a process question here: to post a change for the only accepted
> > change I
> > have in this patch should I send out a new patch?
>
> Would it be possible for you to wait a bit with this minor cleanup?
> As I'm preparing a patch to move this driver out of staging right
> now. You can definitely follow-up with your cleanup, once this move
> is done. Otherwise the move might be delayed even more.
>
> Thanks,
> Stefan
___
devel mailing list
Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
Hi Armando,
On 14.03.19 12:13, Armando Miraglia wrote:
My answers are in-line below. BTW bare with me as this is my attempt to get my
feet wet in how to contribute to the linux kernel for my own pleasure and
interest :)
On Wed, Mar 13, 2019 at 03:34:54PM +0300, Dan Carpenter wrote:
On Wed, Mar 13, 2019 at 01:24:04PM +0100, Armando Miraglia wrote:
Running Lindent on the mt7621-spi.c file in drivers/staging I noticed that the
file contained style issues. This change attempts to address such style
problems.
Don't run lindent. I think checkpatch.pl has a --fix option that might
be better, but once the code is merged then our standard become much
higher for follow up patches.
Signed-off-by: Armando Miraglia
---
NOTE: resend this patch to include all mainteners listed by get_mantainers.pl.
drivers/staging/mt7621-spi/spi-mt7621.c | 27 +
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c
b/drivers/staging/mt7621-spi/spi-mt7621.c
index b509f9fe3346..03d53845f8c5 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -52,14 +52,14 @@
#define MT7621_LSB_FIRST BIT(3)
struct mt7621_spi {
- struct spi_master *master;
- void __iomem*base;
- unsigned intsys_freq;
- unsigned intspeed;
- struct clk *clk;
- int pending_write;
-
- struct mt7621_spi_ops *ops;
+ struct spi_master *master;
+ void __iomem *base;
+ unsigned int sys_freq;
+ unsigned int speed;
+ struct clk *clk;
+ int pending_write;
+
+ struct mt7621_spi_ops *ops;
The original is fine. I don't encourage people to do fancy indenting
with their local variable declarations inside functions but for a struct
the declarations aren't going to change a lot so people can get fancy
if they want.
Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
--fix? If one would like to contribute to fixing the tooling for linting which
of the two would be the right target for such an effort?
The problem with a local is if you need to add a new variable then you
have to re-indent a bunch of unrelated lines or have one out of
alignment line. Most people know this intuitively so they don't get
fancy.
};
static inline struct mt7621_spi *spidev_to_mt7621_spi(struct spi_device *spi)
@@ -303,7 +303,7 @@ static int mt7621_spi_setup(struct spi_device *spi)
struct mt7621_spi *rs = spidev_to_mt7621_spi(spi);
if ((spi->max_speed_hz == 0) ||
- (spi->max_speed_hz > (rs->sys_freq / 2)))
+ (spi->max_speed_hz > (rs->sys_freq / 2)))
Yeah. Lindent is correct here.
Funny enough, this is something I adjusted manually :)
spi->max_speed_hz = (rs->sys_freq / 2);
if (spi->max_speed_hz < (rs->sys_freq / 4097)) {
@@ -316,9 +316,10 @@ static int mt7621_spi_setup(struct spi_device *spi)
}
static const struct of_device_id mt7621_spi_match[] = {
- { .compatible = "ralink,mt7621-spi" },
+ {.compatible = "ralink,mt7621-spi"},
The original was better.
{},
};
+
MODULE_DEVICE_TABLE(of, mt7621_spi_match);
No need for a blank. These are closely related.
Ack.
static int mt7621_spi_probe(struct platform_device *pdev)
@@ -408,9 +409,9 @@ MODULE_ALIAS("platform:" DRIVER_NAME);
static struct platform_driver mt7621_spi_driver = {
.driver = {
- .name = DRIVER_NAME,
- .of_match_table = mt7621_spi_match,
- },
+ .name = DRIVER_NAME,
+ .of_match_table = mt7621_spi_match,
+ },
The new indenting is very wrong.
Ack. In fact, I was thinking this could be one target to fix the logic in
Lindent to do this appropriately.
I have a process question here: to post a change for the only accepted change I
have in this patch should I send out a new patch?
Would it be possible for you to wait a bit with this minor cleanup?
As I'm preparing a patch to move this driver out of staging right
now. You can definitely follow-up with your cleanup, once this move
is done. Otherwise the move might be delayed even more.
Thanks,
Stefan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
On Thu, Mar 14, 2019 at 12:13:15PM +0100, Armando Miraglia wrote: > My answers are in-line below. BTW bare with me as this is my attempt to get my > feet wet in how to contribute to the linux kernel for my own pleasure and > interest :) > No problem at all. > Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl > --fix? If one would like to contribute to fixing the tooling for linting which > of the two would be the right target for such an effort? > I've added Jean to the CC list because he worked on Lindent a few years ago. I really think we should just delete Lindent. I haven't used the checkpatch.pl --fix option but Joe Perches has good style. > > > static inline struct mt7621_spi *spidev_to_mt7621_spi(struct spi_device > > > *spi) > > > @@ -303,7 +303,7 @@ static int mt7621_spi_setup(struct spi_device *spi) > > > struct mt7621_spi *rs = spidev_to_mt7621_spi(spi); > > > > > > if ((spi->max_speed_hz == 0) || > > > - (spi->max_speed_hz > (rs->sys_freq / 2))) > > > + (spi->max_speed_hz > (rs->sys_freq / 2))) > > > > Yeah. Lindent is correct here. > > Funny enough, this is something I adjusted manually :) > :) Good. > I have a process question here: to post a change for the only accepted change > I > have in this patch should I send out a new patch? > Yeah. If you want. Google for how to send a v2 patch. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] spi: mediatek: Attempt to address style issues in spi-mt7621.c
My answers are in-line below. BTW bare with me as this is my attempt to get my
feet wet in how to contribute to the linux kernel for my own pleasure and
interest :)
On Wed, Mar 13, 2019 at 03:34:54PM +0300, Dan Carpenter wrote:
> On Wed, Mar 13, 2019 at 01:24:04PM +0100, Armando Miraglia wrote:
> > Running Lindent on the mt7621-spi.c file in drivers/staging I noticed that
> > the
> > file contained style issues. This change attempts to address such style
> > problems.
> >
>
> Don't run lindent. I think checkpatch.pl has a --fix option that might
> be better, but once the code is merged then our standard become much
> higher for follow up patches.
>
> > Signed-off-by: Armando Miraglia
> > ---
> > NOTE: resend this patch to include all mainteners listed by
> > get_mantainers.pl.
> > drivers/staging/mt7621-spi/spi-mt7621.c | 27 +
> > 1 file changed, 14 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c
> > b/drivers/staging/mt7621-spi/spi-mt7621.c
> > index b509f9fe3346..03d53845f8c5 100644
> > --- a/drivers/staging/mt7621-spi/spi-mt7621.c
> > +++ b/drivers/staging/mt7621-spi/spi-mt7621.c
> > @@ -52,14 +52,14 @@
> > #define MT7621_LSB_FIRST BIT(3)
> >
> > struct mt7621_spi {
> > - struct spi_master *master;
> > - void __iomem*base;
> > - unsigned intsys_freq;
> > - unsigned intspeed;
> > - struct clk *clk;
> > - int pending_write;
> > -
> > - struct mt7621_spi_ops *ops;
> > + struct spi_master *master;
> > + void __iomem *base;
> > + unsigned int sys_freq;
> > + unsigned int speed;
> > + struct clk *clk;
> > + int pending_write;
> > +
> > + struct mt7621_spi_ops *ops;
>
> The original is fine. I don't encourage people to do fancy indenting
> with their local variable declarations inside functions but for a struct
> the declarations aren't going to change a lot so people can get fancy
> if they want.
>
Is there an explicit intent to deprecate Lindent in favor of checkpatch.pl
--fix? If one would like to contribute to fixing the tooling for linting which
of the two would be the right target for such an effort?
> The problem with a local is if you need to add a new variable then you
> have to re-indent a bunch of unrelated lines or have one out of
> alignment line. Most people know this intuitively so they don't get
> fancy.
>
> > };
> >
> > static inline struct mt7621_spi *spidev_to_mt7621_spi(struct spi_device
> > *spi)
> > @@ -303,7 +303,7 @@ static int mt7621_spi_setup(struct spi_device *spi)
> > struct mt7621_spi *rs = spidev_to_mt7621_spi(spi);
> >
> > if ((spi->max_speed_hz == 0) ||
> > - (spi->max_speed_hz > (rs->sys_freq / 2)))
> > + (spi->max_speed_hz > (rs->sys_freq / 2)))
>
> Yeah. Lindent is correct here.
Funny enough, this is something I adjusted manually :)
> > spi->max_speed_hz = (rs->sys_freq / 2);
> >
> > if (spi->max_speed_hz < (rs->sys_freq / 4097)) {
> > @@ -316,9 +316,10 @@ static int mt7621_spi_setup(struct spi_device *spi)
> > }
> >
> > static const struct of_device_id mt7621_spi_match[] = {
> > - { .compatible = "ralink,mt7621-spi" },
> > + {.compatible = "ralink,mt7621-spi"},
>
> The original was better.
>
> > {},
> > };
> > +
> > MODULE_DEVICE_TABLE(of, mt7621_spi_match);
>
> No need for a blank. These are closely related.
Ack.
> >
> > static int mt7621_spi_probe(struct platform_device *pdev)
> > @@ -408,9 +409,9 @@ MODULE_ALIAS("platform:" DRIVER_NAME);
> >
> > static struct platform_driver mt7621_spi_driver = {
> > .driver = {
> > - .name = DRIVER_NAME,
> > - .of_match_table = mt7621_spi_match,
> > - },
> > + .name = DRIVER_NAME,
> > + .of_match_table = mt7621_spi_match,
> > + },
>
> The new indenting is very wrong.
Ack. In fact, I was thinking this could be one target to fix the logic in
Lindent to do this appropriately.
I have a process question here: to post a change for the only accepted change I
have in this patch should I send out a new patch?
Thanks for the help and the review!
Cheers,
A.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] driver : staging : ion: optimization for decreasing memory fragmentaion
From: Zhaoyang Huang
Two action for this patch:
1. set a batch size for system heap's shrinker, which can have it buffer
reasonable page blocks in pool for future allocation.
2. reverse the order sequence when free page blocks, the purpose is also
to have system heap keep as more big blocks as it can.
By testing on an android system with 2G RAM, the changes with setting
batch = 48MB can help reduce the fragmentation obviously and improve
big block allocation speed for 15%.
Signed-off-by: Zhaoyang Huang
---
drivers/staging/android/ion/ion_heap.c| 12 +++-
drivers/staging/android/ion/ion_system_heap.c | 2 +-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion_heap.c
b/drivers/staging/android/ion/ion_heap.c
index 31db510..68aa970 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -16,6 +16,8 @@
#include
#include "ion.h"
+unsigned long ion_heap_batch;
+
void *ion_heap_map_kernel(struct ion_heap *heap,
struct ion_buffer *buffer)
{
@@ -303,7 +305,15 @@ int ion_heap_init_shrinker(struct ion_heap *heap)
heap->shrinker.count_objects = ion_heap_shrink_count;
heap->shrinker.scan_objects = ion_heap_shrink_scan;
heap->shrinker.seeks = DEFAULT_SEEKS;
- heap->shrinker.batch = 0;
+ heap->shrinker.batch = ion_heap_batch;
return register_shrinker(>shrinker);
}
+
+static int __init ion_system_heap_batch_init(char *arg)
+{
+ion_heap_batch = memparse(arg, NULL);
+
+ return 0;
+}
+early_param("ion_batch", ion_system_heap_batch_init);
diff --git a/drivers/staging/android/ion/ion_system_heap.c
b/drivers/staging/android/ion/ion_system_heap.c
index 701eb9f..d249f8d 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -182,7 +182,7 @@ static int ion_system_heap_shrink(struct ion_heap *heap,
gfp_t gfp_mask,
if (!nr_to_scan)
only_scan = 1;
- for (i = 0; i < NUM_ORDERS; i++) {
+ for (i = NUM_ORDERS - 1; i >= 0; i--) {
pool = sys_heap->pools[i];
if (only_scan) {
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: imx7-mipi-csis: fix debugfs compilation
Hi Arnd,
Thanks for the patch.
On Wed 13 Mar 2019 at 21:17, Arnd Bergmann wrote:
When CONFIG_DEBUGFS is enabled, we get a warning about an
incorrect section annotation that can lead to undefined
behavior:
WARNING: vmlinux.o(.text+0xd3c7c4): Section mismatch in
reference from the function mipi_csis_probe() to the function
.init.text:mipi_csis_debugfs_init()
The function mipi_csis_probe() references
the function __init mipi_csis_debugfs_init().
This is often because mipi_csis_probe lacks a __init
annotation or the annotation of mipi_csis_debugfs_init is wrong.
The same function for an unknown reason has a different
version for !CONFIG_DEBUGFS, which does not have this problem,
but behaves the same way otherwise (it does nothing when debugfs
is disabled).
Consolidate the two versions, using the correct section from
one version, and the implementation from the other.
Signed-off-by: Arnd Bergmann
Reviewed-by: Rui Miguel Silva
---
Cheers,
Rui
---
drivers/staging/media/imx/imx7-mipi-csis.c | 16
++--
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c
b/drivers/staging/media/imx/imx7-mipi-csis.c
index 2ddcc42ab8ff..001ce369ec45 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -9,6 +9,7 @@
*/
#include
+#include
#include
#include
#include
@@ -889,8 +890,6 @@ static int mipi_csis_subdev_init(struct
v4l2_subdev *mipi_sd,
return ret;
}
-#ifdef CONFIG_DEBUG_FS
-#include
static int mipi_csis_dump_regs_show(struct seq_file *m, void
*private)
{
@@ -900,7 +899,7 @@ static int mipi_csis_dump_regs_show(struct
seq_file *m, void *private)
}
DEFINE_SHOW_ATTRIBUTE(mipi_csis_dump_regs);
-static int __init_or_module mipi_csis_debugfs_init(struct
csi_state *state)
+static int mipi_csis_debugfs_init(struct csi_state *state)
{
struct dentry *d;
@@ -934,17 +933,6 @@ static void mipi_csis_debugfs_exit(struct
csi_state *state)
debugfs_remove_recursive(state->debugfs_root);
}
-#else
-static int mipi_csis_debugfs_init(struct csi_state *state
__maybe_unused)
-{
- return 0;
-}
-
-static void mipi_csis_debugfs_exit(struct csi_state *state
__maybe_unused)
-{
-}
-#endif
-
static int mipi_csis_probe(struct platform_device *pdev)
{
struct device *dev = >dev;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [greybus-dev] [PATCH] greybus: audio_manager: fix a missing check of ida_simple_get
On 14-03-19, 01:45, Kangjie Lu wrote:
> ida_simple_get could fail. The fix inserts a check for its
> return value.
>
> Signed-off-by: Kangjie Lu
> ---
> drivers/staging/greybus/audio_manager.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/staging/greybus/audio_manager.c
> b/drivers/staging/greybus/audio_manager.c
> index d44b070d8862..c2a4af4c1d06 100644
> --- a/drivers/staging/greybus/audio_manager.c
> +++ b/drivers/staging/greybus/audio_manager.c
> @@ -45,6 +45,9 @@ int gb_audio_manager_add(struct
> gb_audio_manager_module_descriptor *desc)
> int err;
>
> id = ida_simple_get(_id, 0, 0, GFP_KERNEL);
> + if (id < 0)
> + return id;
> +
> err = gb_audio_manager_module_create(, manager_kset,
>id, desc);
> if (err) {
Acked-by: Viresh Kumar
--
viresh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723bs: do not use __constant_cpu_to_le16
A trivial patch.
cpu_to_le16() is capable enough to detect __builtin_constant_p()
and to use an appropriate compile time ___constant_swahbXX()
function.
So we can use cpu_to_le16() instead of __constant_cpu_to_le16().
Signed-off-by: Sergey Senozhatsky
---
drivers/staging/rtl8723bs/include/wifi.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8723bs/include/wifi.h
b/drivers/staging/rtl8723bs/include/wifi.h
index 559bf2606fb7..1e79e6f0c206 100644
--- a/drivers/staging/rtl8723bs/include/wifi.h
+++ b/drivers/staging/rtl8723bs/include/wifi.h
@@ -266,8 +266,8 @@ enum WIFI_REG_DOMAIN {
#define SetFrameType(pbuf, type) \
do {\
- *(unsigned short *)(pbuf) &= __constant_cpu_to_le16(~(BIT(3) |
BIT(2))); \
- *(unsigned short *)(pbuf) |= __constant_cpu_to_le16(type); \
+ *(unsigned short *)(pbuf) &= cpu_to_le16(~(BIT(3) | BIT(2))); \
+ *(unsigned short *)(pbuf) |= cpu_to_le16(type); \
} while (0)
#define GetFrameSubType(pbuf) (le16_to_cpu(*(__le16 *)(pbuf)) & (BIT(7) |\
--
2.21.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] greybus: audio_manager: fix a missing check of ida_simple_get
ida_simple_get could fail. The fix inserts a check for its
return value.
Signed-off-by: Kangjie Lu
---
drivers/staging/greybus/audio_manager.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/staging/greybus/audio_manager.c
b/drivers/staging/greybus/audio_manager.c
index d44b070d8862..c2a4af4c1d06 100644
--- a/drivers/staging/greybus/audio_manager.c
+++ b/drivers/staging/greybus/audio_manager.c
@@ -45,6 +45,9 @@ int gb_audio_manager_add(struct
gb_audio_manager_module_descriptor *desc)
int err;
id = ida_simple_get(_id, 0, 0, GFP_KERNEL);
+ if (id < 0)
+ return id;
+
err = gb_audio_manager_module_create(, manager_kset,
id, desc);
if (err) {
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
