Re: [PATCHv3 2/3] optee: use uuid for sysfs driver entry
On Mon, 25 May 2020 at 15:47, Greg KH wrote: > > On Mon, May 25, 2020 at 02:52:34PM +0300, Maxim Uvarov wrote: > > Optee device names for sysfs needed to be unique > > and it's better if they will mean something. UUID for name > > looks like good solution: > > /sys/bus/tee/devices/optee-clnt- > > Can you document that in Documentation/ABI/ ? > yes, sure if we agree to go with uuid. > And why UUID? Those are usually huge, is that easier than just a unique > number? > UUID here is connected to Trusted Application (TA) in a secure world. If you need to 'find' sysfs entry for the corresponding driver becomes very easy. Also UUID here are not really huge, like: /sys/bus/tee/devices/optee-clnt-71d950bc-c9d4-c442-82cb-343fb7f37896 /sys/bus/tee/devices/optee-clnt-ba3ac5b6-6996-6846-a7f2-205629d00f86 I think that is better then optee-clnt-0, optee-clnt-1.. which can be reordered on each boot and does not carry any information. And on module unload there will be missing numbers. Regards, Maxim. > thanks, > > greg k-h
Re: [PATCH] media: vsp1: dl: Fix NULL pointer dereference on unbind
Hi Kieran, On Mon, May 25, 2020 at 02:19:02PM +0100, Kieran Bingham wrote: > Hi Eugeniu, > > Yeouch. Looks like I really missed a trick there! Not a big deal. The good part is that it can be proactively fixed and shared across the community. > > We should probably update the $SUBJECT to match what is performed in the > patch, which is perhaps more like: > > "media: vsp1: dl: Store VSP reference when creating cmd pools" To be honest, I am not a big fan of WHAT summary lines. Rather, I prefer the WHY summary lines (and I think everyone should). > > On 23/05/2020 09:13, Eugeniu Rosca wrote: > > And then we can explain here: > > In commit f3b98e3c4d2e16 ("media: vsp1: Provide support for extended > command pools"), the vsp pointer used for referencing the VSP1 device > structure from a command pool during vsp1_dl_ext_cmd_pool_destroy() was > not populated. > > Correctly assign the pointer to prevent the following > null-pointer-dereference when removing the device: That sounds good and I can push this improved description as v2. > > Fixes: f3b98e3c4d2e16 ("media: vsp1: Provide support for extended command > > pools") > > Cc: sta...@vger.kernel.org # v4.19+ > > Signed-off-by: Eugeniu Rosca > > Reviewed-by: Kieran Bingham > > > --- > > > > How about adding a new unit test perfoming unbind/rebind to > > http://git.ideasonboard.com/renesas/vsp-tests.git, to avoid > > such issues in future? > > Yes, now I wish I had done so back at 4.19! I hope this wasn't too > painful to diagnose and fix, and thank you for being so thorough in your > report! > > > > Locally, below command has been used to identify the problem: > > > > for f in $(find /sys/bus/platform/devices/ -name "*vsp*" -o -name "*fdp*"); > > do \ > > b=$(basename $f); \ > > echo $b > $f/driver/unbind; \ > > done > > > > I've created a test to add to vsp-tests, which I'll post next, thank you > for the suggestion. > > Before your patch is applied, I experience the same crash you have seen, > and after your patch - I can successfully unbind/bind all of the VSP1 > instances. > > So I think you can have this too: > > Tested-by: Kieran Bingham Awesome. Thanks! -- Best regards, Eugeniu Rosca
Re: [PATCH] can: mcp251x: convert to half-duplex SPI
On Mon, May 25, 2020 at 03:12:05PM +0200, Marc Kleine-Budde wrote: > On 5/25/20 2:57 PM, Mark Brown wrote: > > On Mon, May 25, 2020 at 02:41:31PM +0200, Marc Kleine-Budde wrote: > >> On 5/25/20 1:31 PM, Mark Brown wrote: > >> The core could merge several half duplex transfers (until there's as > >> cs_change) > >> into a single full duplex transfer. > > Yes, that is what I am suggesting. > Where in the SPI stack do you see such a "merge" function? One point to > clarify > is when and where to allocate and free the memory for the contiguous full > duplex > buffers. My first thought would be about the same point as we're rewriting to handle MUST_TX and MUST_RX in map_msg() which does similar allocations and deallocations to insert dummy data for controllers that need them. > >> I think spi_write_then_read() can be extended to generate one full duplex > >> transfer instead on two half duplex ones it does a memcpy() anyways. > > This has the same problem as doing it in any other driver code - it > > causes a needless incompatibility with three wire and single duplex > > devices. > What about the note "portable code should never use this for more than 32 > bytes" > in spi_write_then_read()? The CAN driver in question may read more than 32 > bytes > of data. I think that comment is actually not valid any more - we used to use a fixed statically allocated buffer in write_then_read() but added the option to fall back onto allocating one dynamically if another user was running or the transfer was too big. signature.asc Description: PGP signature
Re: [PATCH v2 2/7] radix-tree: Use local_lock for protection
* Matthew Wilcox wrote: > On Mon, May 25, 2020 at 08:29:54AM +0200, Ingo Molnar wrote: > > > +void radix_tree_preload_end(void) > > > +{ > > > + local_unlock(_tree_preloads.lock); > > > +} > > > +EXPORT_SYMBOL(radix_tree_preload_end); > > > > Since upstream we are still mapping the local_lock primitives to > > preempt_disable()/preempt_enable(), I believe these uninlining changes > > should not be done > > in this patch, i.e. idr_preload_end() and radix_tree_preload_end() should > > stay inline. > > But radix_tree_preloads is static, and I wouldn't be terribly happy to > see that exported to modules. Well, it seems a bit silly to make radix_tree_preload_end() a standalone function, on most distro kernels that don't have CONFIG_PREEMPT=y, preempt_enable() is a NOP: 2bf0 : 2bf0: c3 retq I.e. we'd be introducing a separate function call for no good reason. Thanks, Ingo
Re: [PATCH 1/5] seccomp: Add find_notification helper
On Sun, May 24, 2020 at 04:39:38PM -0700, Sargun Dhillon wrote: > This adds a helper which can iterate through a seccomp_filter to > find a notification matching an ID. It removes several replicated > chunks of code. > > Signed-off-by: Sargun Dhillon > Cc: Matt Denton > Cc: Kees Cook , > Cc: Jann Horn , > Cc: Robert Sesek , > Cc: Chris Palmer > Cc: Christian Brauner > Cc: Tycho Andersen > --- > kernel/seccomp.c | 38 +- > 1 file changed, 21 insertions(+), 17 deletions(-) > > diff --git a/kernel/seccomp.c b/kernel/seccomp.c > index 55a6184f5990..f6ce94b7a167 100644 > --- a/kernel/seccomp.c > +++ b/kernel/seccomp.c > @@ -1021,10 +1021,25 @@ static int seccomp_notify_release(struct inode > *inode, struct file *file) > return 0; > } > > +/* must be called with notif_lock held */ > +static inline struct seccomp_knotif * > +find_notification(struct seccomp_filter *filter, u64 id) > +{ > + struct seccomp_knotif *cur; > + > + list_for_each_entry(cur, >notif->notifications, list) { > + if (cur->id == id) > + return cur; > + } > + > + return NULL; > +} > + > + > static long seccomp_notify_recv(struct seccomp_filter *filter, > void __user *buf) > { > - struct seccomp_knotif *knotif = NULL, *cur; > + struct seccomp_knotif *knotif, *cur; > struct seccomp_notif unotif; > ssize_t ret; > > @@ -1078,14 +1093,8 @@ static long seccomp_notify_recv(struct seccomp_filter > *filter, >* may have died when we released the lock, so we need to make >* sure it's still around. >*/ > - knotif = NULL; > mutex_lock(>notify_lock); > - list_for_each_entry(cur, >notif->notifications, list) { > - if (cur->id == unotif.id) { > - knotif = cur; > - break; > - } > - } > + knotif = find_notification(filter, unotif.id); > > if (knotif) { > knotif->state = SECCOMP_NOTIFY_INIT; > @@ -1150,7 +1159,7 @@ static long seccomp_notify_send(struct seccomp_filter > *filter, > static long seccomp_notify_id_valid(struct seccomp_filter *filter, > void __user *buf) > { > - struct seccomp_knotif *knotif = NULL; > + struct seccomp_knotif *knotif; > u64 id; > long ret; > > @@ -1162,15 +1171,10 @@ static long seccomp_notify_id_valid(struct > seccomp_filter *filter, > return ret; > > ret = -ENOENT; > - list_for_each_entry(knotif, >notif->notifications, list) { > - if (knotif->id == id) { > - if (knotif->state == SECCOMP_NOTIFY_SENT) > - ret = 0; > - goto out; > - } > - } > + knotif = find_notification(filter, id); > + if (knotif && knotif->state == SECCOMP_NOTIFY_SENT) > + ret = 0; Coul be a little nicer to have this be: if (knotif && knotif->state == SECCOMP_NOTIFY_SENT) ret = 0; else ret = -ENOENT; or, if you want to keep the assignment out of the lock: ret = -ENOENT; ret = mutex_lock_interruptible(>notify_lock); if (ret < 0) return ret; knotif = find_notification(filter, id); if (knotif && knotif->state == SECCOMP_NOTIFY_SENT) ret = 0; otherwise looks like a good cleanup to me.
Re: [PATCH v3 10/16] gpio: add a reusable generic gpio_chip using regmap
On Mon, May 25, 2020 at 02:59:36PM +0200, Linus Walleij wrote: > On Mon, May 25, 2020 at 12:20 PM Michael Walle wrote: > > > If you like I could submit this patch on its own. But then there > > wouldn't be a user for it. > > I'm pretty much fine with that, we do merge code that has no > users if we anticipate they will be around the corner. I remember we discussed with Pierre to use it for his ASoC work. Pierre, does it sound useful for you? -- With Best Regards, Andy Shevchenko
Re: [PATCH] iommu: Don't take group reference in iommu_alloc_default_domain()
Hi Joerg, On 2020-05-25 18:31, Joerg Roedel wrote: From: Joerg Roedel The iommu_alloc_default_domain() function takes a reference to an IOMMU group without releasing it. This causes the group to never be released, with undefined side effects. The function has only one call-site, which takes a group reference on its own, so to fix this leak, do not take another reference in iommu_alloc_default_domain() and pass the group as a function parameter instead. Reference: https://lore.kernel.org/lkml/20200522130145.30067-1-saiprakash.ran...@codeaurora.org/ Reported-by: Sai Prakash Ranjan Cc: Sai Prakash Ranjan Fixes: 6e1aa2049154 ("iommu: Move default domain allocation to iommu_probe_device()") Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 21 + 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 374b34fd6fac..bf20674769e0 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -80,7 +80,8 @@ static bool iommu_cmd_line_dma_api(void) return !!(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API); } -static int iommu_alloc_default_domain(struct device *dev); +static int iommu_alloc_default_domain(struct iommu_group *group, + struct device *dev); static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus, unsigned type); static int __iommu_attach_device(struct iommu_domain *domain, @@ -251,17 +252,17 @@ int iommu_probe_device(struct device *dev) if (ret) goto err_out; + group = iommu_group_get(dev); + if (!group) + goto err_release; + /* * Try to allocate a default domain - needs support from the * IOMMU driver. There are still some drivers which don't * support default domains, so the return value is not yet * checked. */ - iommu_alloc_default_domain(dev); - - group = iommu_group_get(dev); - if (!group) - goto err_release; + iommu_alloc_default_domain(group, dev); if (group->default_domain) ret = __iommu_attach_device(group->default_domain, dev); @@ -1478,15 +1479,11 @@ static int iommu_group_alloc_default_domain(struct bus_type *bus, return 0; } -static int iommu_alloc_default_domain(struct device *dev) +static int iommu_alloc_default_domain(struct iommu_group *group, + struct device *dev) { - struct iommu_group *group; unsigned int type; - group = iommu_group_get(dev); - if (!group) - return -ENODEV; - if (group->default_domain) return 0; Tested this out and it works for me. Tested-by: Sai Prakash Ranjan Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [RFC/RFT][PATCH] cpufreq: intel_pstate: Work in passive mode with HWP enabled
On Mon, May 25, 2020 at 3:39 AM Francisco Jerez wrote: > > "Rafael J. Wysocki" writes: > > > From: Rafael J. Wysocki > > > > Allow intel_pstate to work in the passive mode with HWP enabled and > > make it translate the target frequency supplied by the cpufreq > > governor in use into an EPP value to be written to the HWP request > > MSR (high frequencies are mapped to low EPP values that mean more > > performance-oriented HWP operation) as a hint for the HWP algorithm > > in the processor, so as to prevent it and the CPU scheduler from > > working against each other at least when the schedutil governor is > > in use. > > > > Signed-off-by: Rafael J. Wysocki > > --- > > > > This is a prototype not intended for production use (based on linux-next). > > > > Please test it if you can (on HWP systems, of course) and let me know the > > results. > > > > The INTEL_CPUFREQ_TRANSITION_DELAY_HWP value has been guessed and it very > > well > > may turn out to be either too high or too low for the general use, which is > > one > > reason why getting as much testing coverage as possible is key here. > > > > If you can play with different INTEL_CPUFREQ_TRANSITION_DELAY_HWP values, > > please do so and let me know the conclusions. > > > > Cheers, > > Rafael > > > > --- > > drivers/cpufreq/intel_pstate.c | 169 > > +++-- > > 1 file changed, 131 insertions(+), 38 deletions(-) > > > > Index: linux-pm/drivers/cpufreq/intel_pstate.c > > === > > --- linux-pm.orig/drivers/cpufreq/intel_pstate.c > > +++ linux-pm/drivers/cpufreq/intel_pstate.c > > @@ -36,6 +36,7 @@ > > #define INTEL_PSTATE_SAMPLING_INTERVAL (10 * NSEC_PER_MSEC) > > > > #define INTEL_CPUFREQ_TRANSITION_LATENCY 2 > > +#define INTEL_CPUFREQ_TRANSITION_DELAY_HWP 5000 > > #define INTEL_CPUFREQ_TRANSITION_DELAY 500 > > > > #ifdef CONFIG_ACPI > > @@ -95,6 +96,8 @@ static inline int32_t percent_ext_fp(int > > return div_ext_fp(percent, 100); > > } > > > > +#define HWP_EPP_TO_BYTE(x) (((u64)x >> 24) & 0xFF) > > + > > /** > > * struct sample - Store performance sample > > * @core_avg_perf: Ratio of APERF/MPERF which is the actual average > > @@ -2175,7 +2178,10 @@ static int intel_pstate_verify_policy(st > > > > static void intel_cpufreq_stop_cpu(struct cpufreq_policy *policy) > > { > > - intel_pstate_set_min_pstate(all_cpu_data[policy->cpu]); > > + if (hwp_active) > > + intel_pstate_hwp_force_min_perf(policy->cpu); > > + else > > + intel_pstate_set_min_pstate(all_cpu_data[policy->cpu]); > > } > > > > static void intel_pstate_stop_cpu(struct cpufreq_policy *policy) > > @@ -2183,12 +2189,10 @@ static void intel_pstate_stop_cpu(struct > > pr_debug("CPU %d exiting\n", policy->cpu); > > > > intel_pstate_clear_update_util_hook(policy->cpu); > > - if (hwp_active) { > > + if (hwp_active) > > intel_pstate_hwp_save_state(policy); > > - intel_pstate_hwp_force_min_perf(policy->cpu); > > - } else { > > - intel_cpufreq_stop_cpu(policy); > > - } > > + > > + intel_cpufreq_stop_cpu(policy); > > } > > > > static int intel_pstate_cpu_exit(struct cpufreq_policy *policy) > > @@ -2296,7 +2300,8 @@ static int intel_cpufreq_verify_policy(s > > #define INTEL_PSTATE_TRACE_TARGET 10 > > #define INTEL_PSTATE_TRACE_FAST_SWITCH 90 > > > > -static void intel_cpufreq_trace(struct cpudata *cpu, unsigned int > > trace_type, int old_pstate) > > +static void intel_cpufreq_trace(struct cpudata *cpu, unsigned int > > trace_type, > > + int from, int to) > > { > > struct sample *sample; > > > > @@ -2309,8 +2314,8 @@ static void intel_cpufreq_trace(struct c > > sample = >sample; > > trace_pstate_sample(trace_type, > > 0, > > - old_pstate, > > - cpu->pstate.current_pstate, > > + from, > > + to, > > sample->mperf, > > sample->aperf, > > sample->tsc, > > @@ -2318,40 +2323,110 @@ static void intel_cpufreq_trace(struct c > > fp_toint(cpu->iowait_boost * 100)); > > } > > > > -static int intel_cpufreq_target(struct cpufreq_policy *policy, > > - unsigned int target_freq, > > - unsigned int relation) > > +static void intel_cpufreq_update_hwp_request(struct cpudata *cpu, u8 > > new_epp) > > { > > - struct cpudata *cpu = all_cpu_data[policy->cpu]; > > - struct cpufreq_freqs freqs; > > - int target_pstate, old_pstate; > > + u64 value, prev; > > > > - update_turbo_state(); > > + prev = READ_ONCE(cpu->hwp_req_cached); > > + value = prev; > > > > - freqs.old = policy->cur; > > - freqs.new = target_freq; > > + /* > > + * The entire MSR needs to be updated in order to update the EPP
[PATCH v6 0/3] Add Caninos Loucos Labrador CoM and Base Board Device Tree
Thanks Andreas, Mani and Rob for your time reviewing it. Changes since v5: (Suggested by Andreas Färber) - Put caninos,labrador-v2 as const one level down Changes since v4: (Suggested by Rob Herring) - Fix issues with yaml indentation Matheus Castello (3): dt-bindings: Add vendor prefix for Caninos Loucos dt-bindings: arm: actions: Document Caninos Loucos Labrador ARM: dts: Add Caninos Loucos Labrador .../devicetree/bindings/arm/actions.yaml | 5 +++ .../devicetree/bindings/vendor-prefixes.yaml | 2 ++ arch/arm/boot/dts/Makefile| 1 + .../arm/boot/dts/owl-s500-labrador-base-m.dts | 34 +++ arch/arm/boot/dts/owl-s500-labrador-v2.dtsi | 22 5 files changed, 64 insertions(+) create mode 100644 arch/arm/boot/dts/owl-s500-labrador-base-m.dts create mode 100644 arch/arm/boot/dts/owl-s500-labrador-v2.dtsi -- 2.26.2
Re: Endless soft-lockups for compiling workload since next-20200519
On Thu, May 21, 2020 at 02:41:14PM +0200, Frederic Weisbecker wrote: > On Thu, May 21, 2020 at 01:00:27PM +0200, Peter Zijlstra wrote: > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > > index 01f94cf52783..b6d8a7b991f0 100644 > > --- a/kernel/sched/fair.c > > +++ b/kernel/sched/fair.c > > @@ -10033,7 +10033,7 @@ static void kick_ilb(unsigned int flags) > > * is idle. And the softirq performing nohz idle load balance > > * will be run before returning from the IPI. > > */ > > - smp_call_function_single_async(ilb_cpu, _rq(ilb_cpu)->nohz_csd); > > + smp_call_function_single_async(ilb_cpu, _rq()->nohz_csd); > > My fear here is that if a previous call from the the same CPU but to another > target is still pending, the new one will be spuriously ignored. > Urgh, indeed! > But I believe we can still keep the remote csd if nohz_flags() are > strictly only set before the IPI and strictly only cleared from it. > > And I still don't understand why trigger_load_balance() raise the > softirq without setting the current CPU as ilb. run_rebalance_domains() > thus ignores it most of the time in the end or it spuriously clear the > nohz_flags set by an IPI sender. Or there is something I misunderstood > there. That is because it is simple and didn't matter before. Whoever got there first go to run the ilb whenever the flag was set. But now we have this race due to having to serialize access to the csd. We want the IPI to clear the flag, but then the softirq no longer knows it was supposed to do ILB. How's this then? --- include/linux/sched.h | 4 kernel/sched/core.c | 41 + kernel/sched/fair.c | 15 +++ kernel/sched/sched.h | 2 +- 4 files changed, 25 insertions(+), 37 deletions(-) diff --git a/include/linux/sched.h b/include/linux/sched.h index f38d62c4632c..136ee400b568 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -696,6 +696,10 @@ struct task_struct { struct uclamp_seuclamp[UCLAMP_CNT]; #endif +#ifdef CONFIG_SMP + call_single_data_t wake_csd; +#endif + #ifdef CONFIG_PREEMPT_NOTIFIERS /* List of struct preempt_notifier: */ struct hlist_head preempt_notifiers; diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 5b286469e26e..90484b988b65 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -637,41 +637,25 @@ void wake_up_nohz_cpu(int cpu) wake_up_idle_cpu(cpu); } -static inline bool got_nohz_idle_kick(void) +static void nohz_csd_func(void *info) { - int cpu = smp_processor_id(); - - if (!(atomic_read(nohz_flags(cpu)) & NOHZ_KICK_MASK)) - return false; - - if (idle_cpu(cpu) && !need_resched()) - return true; + struct rq *rq = info; + int cpu = cpu_of(rq); + WARN_ON(!(atomic_read(nohz_flags(cpu)) & NOHZ_KICK_MASK)); /* -* We can't run Idle Load Balance on this CPU for this time so we -* cancel it and clear NOHZ_BALANCE_KICK +* Release the rq::nohz_csd. */ + smp_mb__before_atomic(); atomic_andnot(NOHZ_KICK_MASK, nohz_flags(cpu)); - return false; -} - -static void nohz_csd_func(void *info) -{ - struct rq *rq = info; - if (got_nohz_idle_kick()) { - rq->idle_balance = 1; + rq->idle_balance = idle_cpu(cpu); + if (rq->idle_balance && !need_resched()) { + rq->nohz_idle_balance = 1; raise_softirq_irqoff(SCHED_SOFTIRQ); } } -#else /* CONFIG_NO_HZ_COMMON */ - -static inline bool got_nohz_idle_kick(void) -{ - return false; -} - #endif /* CONFIG_NO_HZ_COMMON */ #ifdef CONFIG_NO_HZ_FULL @@ -2320,7 +2304,7 @@ static void ttwu_queue_remote(struct task_struct *p, int cpu, int wake_flags) if (llist_add(>wake_entry, >wake_list)) { if (!set_nr_if_polling(rq->idle)) - smp_call_function_single_async(cpu, >wake_csd); + smp_call_function_single_async(cpu, >wake_csd); else trace_sched_wake_idle_without_ipi(cpu); } @@ -2921,6 +2905,9 @@ int sched_fork(unsigned long clone_flags, struct task_struct *p) #endif #if defined(CONFIG_SMP) p->on_cpu = 0; + p->wake_csd = (struct __call_single_data) { + .func = wake_csd_func, + }; #endif init_task_preempt_count(p); #ifdef CONFIG_SMP @@ -6723,8 +6710,6 @@ void __init sched_init(void) rq->avg_idle = 2*sysctl_sched_migration_cost; rq->max_idle_balance_cost = sysctl_sched_migration_cost; - rq_csd_init(rq, >wake_csd, wake_csd_func); - INIT_LIST_HEAD(>cfs_tasks); rq_attach_root(rq, _root_domain); diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 01f94cf52783..93525549a023 100644 --- a/kernel/sched/fair.c
Re: [PATCH] media: vsp1: dl: Fix NULL pointer dereference on unbind
Hi Eugeniu, Yeouch. Looks like I really missed a trick there! We should probably update the $SUBJECT to match what is performed in the patch, which is perhaps more like: "media: vsp1: dl: Store VSP reference when creating cmd pools" On 23/05/2020 09:13, Eugeniu Rosca wrote: And then we can explain here: In commit f3b98e3c4d2e16 ("media: vsp1: Provide support for extended command pools"), the vsp pointer used for referencing the VSP1 device structure from a command pool during vsp1_dl_ext_cmd_pool_destroy() was not populated. Correctly assign the pointer to prevent the following null-pointer-dereference when removing the device: > v4.19 commit f3b98e3c4d2e16 ("media: vsp1: Provide support for extended > command pools") introduced below issue [*], consistently reproduced. > > In order to fix it, inspire from the sibling/predecessor v4.18-rc1 > commit 5de0473982aab2 ("media: vsp1: Provide a body pool"), which saves > the vsp1 instance address in vsp1_dl_body_pool_create(). > > [*] h3ulcb-kf #> > echo fea28000.vsp > /sys/bus/platform/devices/fea28000.vsp/driver/unbind > Unable to handle kernel NULL pointer dereference at virtual address > 0028 > Mem abort info: >ESR = 0x9606 >EC = 0x25: DABT (current EL), IL = 32 bits >SET = 0, FnV = 0 >EA = 0, S1PTW = 0 > Data abort info: >ISV = 0, ISS = 0x0006 >CM = 0, WnR = 0 > user pgtable: 4k pages, 48-bit VAs, pgdp=0007318be000 > [0028] pgd=0007333a1003, pud=0007333a6003, > pmd= > Internal error: Oops: 9606 [#1] PREEMPT SMP > Modules linked in: > CPU: 1 PID: 486 Comm: sh Not tainted > 5.7.0-rc6-arm64-renesas-00118-ge644645abf47 #185 > Hardware name: Renesas H3ULCB Kingfisher board based on r8a77951 (DT) > pstate: 4005 (nZcv daif -PAN -UAO) > pc : vsp1_dlm_destroy+0xe4/0x11c > lr : vsp1_dlm_destroy+0xc8/0x11c > sp : 800012963b60 > x29: 800012963b60 x28: 0006f83fc440 > x27: x26: 0006f5e13e80 > x25: 0006f5e13ed0 x24: 0006f5e13ed0 > x23: 0006f5e13ed0 x22: dead0122 > x21: 0006f5e3a080 x20: 0006f5df2938 > x19: 0006f5df2980 x18: 0003 > x17: x16: 0016 > x15: 0003 x14: 000393c0 > x13: 800011a5ec18 x12: 800011d8d000 > x11: 0006f83fcc68 x10: 800011a53d70 > x9 : 8000111f3000 x8 : > x7 : 00210d00 x6 : > x5 : 800010872e60 x4 : 0004 > x3 : 78068000 x2 : 800012781000 > x1 : 2c00 x0 : > Call trace: > vsp1_dlm_destroy+0xe4/0x11c > vsp1_wpf_destroy+0x10/0x20 > vsp1_entity_destroy+0x24/0x4c > vsp1_destroy_entities+0x54/0x130 > vsp1_remove+0x1c/0x40 > platform_drv_remove+0x28/0x50 > __device_release_driver+0x178/0x220 > device_driver_detach+0x44/0xc0 > unbind_store+0xe0/0x104 > drv_attr_store+0x20/0x30 > sysfs_kf_write+0x48/0x70 > kernfs_fop_write+0x148/0x230 > __vfs_write+0x18/0x40 > vfs_write+0xdc/0x1c4 > ksys_write+0x68/0xf0 > __arm64_sys_write+0x18/0x20 > el0_svc_common.constprop.0+0x70/0x170 > do_el0_svc+0x20/0x80 > el0_sync_handler+0x134/0x1b0 > el0_sync+0x140/0x180 > Code: b4c2 f9403a60 d2800084 a9400663 (f9401400) > ---[ end trace 3875369841fb288a ]--- > > Fixes: f3b98e3c4d2e16 ("media: vsp1: Provide support for extended command > pools") > Cc: sta...@vger.kernel.org # v4.19+ > Signed-off-by: Eugeniu Rosca Reviewed-by: Kieran Bingham > --- > > How about adding a new unit test perfoming unbind/rebind to > http://git.ideasonboard.com/renesas/vsp-tests.git, to avoid > such issues in future? Yes, now I wish I had done so back at 4.19! I hope this wasn't too painful to diagnose and fix, and thank you for being so thorough in your report! > Locally, below command has been used to identify the problem: > > for f in $(find /sys/bus/platform/devices/ -name "*vsp*" -o -name "*fdp*"); > do \ > b=$(basename $f); \ > echo $b > $f/driver/unbind; \ > done > I've created a test to add to vsp-tests, which I'll post next, thank you for the suggestion. Before your patch is applied, I experience the same crash you have seen, and after your patch - I can successfully unbind/bind all of the VSP1 instances. So I think you can have this too: Tested-by: Kieran Bingham > --- > drivers/media/platform/vsp1/vsp1_dl.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > b/drivers/media/platform/vsp1/vsp1_dl.c > index d7b43037e500..e07b135613eb 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.c > +++ b/drivers/media/platform/vsp1/vsp1_dl.c > @@ -431,6 +431,8 @@ vsp1_dl_cmd_pool_create(struct vsp1_device *vsp1, enum > vsp1_extcmd_type type, > if (!pool) > return NULL; > > + pool->vsp1 = vsp1; > + > spin_lock_init(>lock); > INIT_LIST_HEAD(>free); > >
Re: [PATCH 1/3 RESEND] sched: Remove __rcu annotation from cred pointer
On 2020-05-24 13:41, Amol Grover wrote: > On Thu, Apr 02, 2020 at 11:26:38AM +0530, Amol Grover wrote: > > task_struct::cred (subjective credentials) is *always* used > > task-synchronously, hence, does not require RCU semantics. > > > > task_struct::real_cred (objective credentials) can be used in > > RCU context and its __rcu annotation is retained. > > > > However, task_struct::cred and task_struct::real_cred *may* > > point to the same object, hence, the object pointed to by > > task_struct::cred *may* have RCU delayed freeing. > > > > Suggested-by: Jann Horn > > Co-developed-by: Joel Fernandes (Google) > > Signed-off-by: Joel Fernandes (Google) > > Signed-off-by: Amol Grover > > Hello everyone, > > Could you please go through patches 1/3 and 2/3 and if deemed OK, give > your acks. I sent the original patch in beginning of February (~4 months > back) and resent the patches again in beginning of April due to lack of > traffic. Paul Moore was kind enough to ack twice - the 3/3 and its > resend patch. However these 2 patches still remain. I'd really > appreciate if someone reviewed them. I asked on April 3 which upstream tree you expect this patchset to go through and I did not see a reply. Do you have a specific target or is the large addressee list assuming someone else is taking this set? All we have seen is that it is not intended to go through the audit tree. > Thanks > Amol - RGB -- Richard Guy Briggs Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635
Re: MRP netlink interface
On Mon, May 25, 2020 at 01:14:35PM +, Horatiu Vultur wrote: > The 05/25/2020 13:26, Nikolay Aleksandrov wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > > content is safe > > > > On 25/05/2020 13:03, Michal Kubecek wrote: > > > On Mon, May 25, 2020 at 11:28:27AM +, Horatiu Vultur wrote: > > > [...] > > >> My first approach was to extend the 'struct br_mrp_instance' with a > > >> field that > > >> contains the priority of the node. But this breaks the backwards > > >> compatibility, > > >> and then every time when I need to change something, I will break the > > >> backwards > > >> compatibility. Is this a way to go forward? > > > > > > No, I would rather say it's an example showing why passing data > > > structures as binary data via netlink is a bad idea. I definitely > > > wouldn't advice this approach for any new interface. One of the > > > strengths of netlink is the ability to use structured and extensible > > > messages. > > > > > >> Another approach is to restructure MRP netlink interface. What I was > > >> thinking to > > >> keep the current attributes (IFLA_BRIDGE_MRP_INSTANCE, > > >> IFLA_BRIDGE_MRP_PORT_STATE,...) but they will be nested attributes and > > >> each of > > >> this attribute to contain the fields of the structures they represents. > > >> For example: > > >> [IFLA_AF_SPEC] = { > > >> [IFLA_BRIDGE_FLAGS] > > >> [IFLA_BRIDGE_MRP] > > >> [IFLA_BRIDGE_MRP_INSTANCE] > > >> [IFLA_BRIDGE_MRP_INSTANCE_RING_ID] > > >> [IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX] > > >> [IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX] > > >> [IFLA_BRIDGE_MRP_RING_ROLE] > > >> [IFLA_BRIDGE_MRP_RING_ROLE_RING_ID] > > >> [IFLA_BRIDGE_MRP_RING_ROLE_ROLE] > > >> ... > > >> } > > >> And then I can parse each field separately and then fill up the structure > > >> (br_mrp_instance, br_mrp_port_role, ...) which will be used forward. > > >> Then when this needs to be extended with the priority it would have the > > >> following format: > > >> [IFLA_AF_SPEC] = { > > >> [IFLA_BRIDGE_FLAGS] > > >> [IFLA_BRIDGE_MRP] > > >> [IFLA_BRIDGE_MRP_INSTANCE] > > >> [IFLA_BRIDGE_MRP_INSTANCE_RING_ID] > > >> [IFLA_BRIDGE_MRP_INSTANCE_P_IFINDEX] > > >> [IFLA_BRIDGE_MRP_INSTANCE_S_IFINDEX] > > >> [IFLA_BRIDGE_MRP_INSTANCE_PRIO] > > >> [IFLA_BRIDGE_MRP_RING_ROLE] > > >> [IFLA_BRIDGE_MRP_RING_ROLE_RING_ID] > > >> [IFLA_BRIDGE_MRP_RING_ROLE_ROLE] > > >> ... > > >> } > > >> And also the br_mrp_instance will have a field called prio. > > >> So now, if the userspace is not updated to have support for setting the > > >> prio > > >> then the kernel will use a default value. Then if the userspace contains > > >> a field > > >> that the kernel doesn't know about, then it would just ignore it. > > >> So in this way every time when the netlink interface will be extended it > > >> would > > >> be backwards compatible. > > > > > > Silently ignoring unrecognized attributes in userspace requests is what > > > most kernel netlink based interfaces have been doing traditionally but > > > it's not really a good idea. Essentially it ties your hands so that you > > > can only add new attributes which can be silently ignored without doing > > > any harm, otherwise you risk that kernel will do something different > > > than userspace asked and userspace does not even have a way to find out > > > if the feature is supported or not. (IIRC there are even some places > > > where ignoring an attribute changes the nature of the request but it is > > > still ignored by older kernels.) > > > > > > That's why there have been an effort, mostly by Johannes Berg, to > > > introduce and promote strict checking for new netlink interfaces and new > > > attributes in existing netlink attributes. If you don't have strict > > > checking for unknown attributes enabled yet, there isn't much that can > > > be done for already released kernels but I would suggest to enable it as > > > soon as possible. > > > > > > Michal > > Thanks for the detail explanation. Currently this is in net-next so I > would try to change it. > Can you point me to some code that is using this strict checking for > netlink attributes? Just to have a better understanding of it. AFAICS you are using nla_parse_nested() in br_mrp_parse() so that the validation should be strict, including rejection of unknown attributes. See the comments at nla_parse() and nla_parse_deprecated() and enum netlink_validation in include/net/netlink.h for details. Michal > > +1, we don't have strict checking for the bridge main af spec attributes, > > but > > you could add that for new nested interfaces that need to be parsed like the > > above
Re: KVM broken after suspend in most recent kernels.
On 25/5/20 7:46 pm, Maxim Levitsky wrote: On Sun, 2020-05-24 at 18:43 +0800, Brad Campbell wrote: On 24/5/20 12:50 pm, Brad Campbell wrote: G'day all. Machine is a Macbook Pro Retina ~ 2014. Kernels are always vanilla kernel and compiled on the machine. No additional patches. vendor_id: GenuineIntel cpu family: 6 model: 69 model name: Intel(R) Core(TM) i5-4278U CPU @ 2.60GHz stepping: 1 microcode: 0x25 cpu MHz: 2795.034 cache size: 3072 KB physical id: 0 siblings: 4 core id: 1 cpu cores: 2 apicid: 3 initial apicid: 3 fpu: yes fpu_exception: yes cpuid level: 13 wp: yes flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm cpuid_fault epb invpcid_single ssbd ibrs ibpb stibp tpr_shadow vnmi flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid xsaveopt dtherm ida arat pln pts md_clear flush_l1d vmx flags: vnmi preemption_timer invvpid ept_x_only ept_ad ept_1gb flexpriority tsc_offset vtpr mtf vapic ept vpid unrestricted_guest ple bugs: cpu_meltdown spectre_v1 spectre_v2 spec_store_bypass l1tf mds swapgs itlb_multihit bogomips: 5199.87 clflush size: 64 cache_alignment: 64 address sizes: 39 bits physical, 48 bits virtual KVM worked fine in kernels somewhere prior to 5.4-5.5. KVM works fine in later kernels up to and including 5.7.0-rc6 after a clean boot. It does not work after a suspend. I can't actually bisect this because there is a bug in earlier kernels that breaks the suspend method used which requires manual patching to work around. This is using qemu version 5.0.0, but also happens with 4.2.0. In kernels earlier than 5.7 it results in either an immediate hard lock, or a GPF that results in progressive system freeze until a hard reboot is required (won't flush to disk so no logs get recorded and I have no serial or netconsole ability). In 5.7-rc6 it results in the following trace and thankfully no further issues (so I can get the logs and report it). I can and will perform any required testing and debugging, but this machine suspends with pm-utils s2both, and that is broken between about 5.4 & 5.6 due to swapfile locking issues, which makes actual bisection very, very difficult as it *requires* a suspend/resume to trigger the bug. [ 227.715173] [ cut here ] [ 227.715176] VMXON faulted, MSR_IA32_FEAT_CTL (0x3a) = 0x4 [ 227.715194] WARNING: CPU: 0 PID: 5502 at arch/x86/kvm/vmx/vmx.c:2239 hardware_enable+0x167/0x180 [kvm_intel] [ 227.715195] Modules linked in: brcmfmac xhci_pci xhci_hcd cmac bnep iptable_nat xt_MASQUERADE nf_nat nf_conntrack nf_defrag_ipv4 ip_tables x_tables nfsd bridge stp llc appletouch brcmutil snd_hda_codec_hdmi sha256_ssse3 snd_hda_codec_cirrus snd_hda_codec_generic sha256_generic libsha256 x86_pkg_temp_thermal coretemp btusb kvm_intel btrtl kvm btbcm btintel irqbypass bluetooth cfg80211 snd_hda_intel ecdh_generic ecc snd_intel_dspcfg bcm5974 rfkill snd_hda_codec snd_hwdep snd_hda_core snd_pcm_oss snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi i915 snd_seq snd_seq_device snd_timer i2c_algo_bit iosf_mbi drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops snd drm intel_gtt agpgart evdev apple_bl video soundcore hid_apple usb_storage hid_generic usbhid hid dm_crypt dm_mod i2c_i801 i2c_core sg usbcore usb_common [last unloaded: xhci_hcd] [ 227.715221] CPU: 0 PID: 5502 Comm: qemu Not tainted 5.7.0-rc6+ #15 [ 227.715222] Hardware name: Apple Inc. MacBookPro11,1/Mac-189A3D4F975D5FFC, BIOS 159.0.0.0.0 02/05/2020 [ 227.715225] RIP: 0010:hardware_enable+0x167/0x180 [kvm_intel] [ 227.715227] Code: 01 00 01 b9 3a 00 00 00 0f 32 31 c9 48 c1 e2 20 be ef be ad de 48 c7 c7 68 fd bb c0 48 09 c2 85 c9 48 0f 44 f2 e8 43 78 4f dc <0f> 0b eb 8a 48 8b 15 ce 89 06 dd e9 c7 fe ff ff 66 0f 1f 84 00 00 [ 227.715228] RSP: 0018:97091d873df8 EFLAGS: 00010092 [ 227.715229] RAX: 002d RBX: 0046 RCX: 0007 [ 227.715230] RDX: 0007 RSI: 0082 RDI: 97091f2187a0 [ 227.715231] RBP: 97091d873e10 R08: 0008 R09: 0495 [ 227.715232] R10: 0010 R11: 97091d873c6d R12: [ 227.715233] R13: 0286 R14: b5d08015e010 R15: [ 227.715234] FS: 7f1468fd33c0() GS:97091f20() knlGS: [ 227.715235] CS: 0010 DS: ES: CR0: 80050033 [ 227.715236] CR2: 563b54c7201d CR3: 00043f43f001 CR4: 001626f0 [
Re: [PATCH v2 08/12] i2c: designware: Introduce platform drivers glue layer interface
Hi On 5/21/20 5:37 AM, Serge Semin wrote: On Wed, May 20, 2020 at 03:46:11PM +0300, Jarkko Nikula wrote: Hi On 5/10/20 12:50 PM, Serge Semin wrote: Seeing the DW I2C platform driver is getting overcomplicated with a lot of vendor-specific configs let's introduce a glue-layer interface so new platforms which equipped with Synopsys Designware APB I2C IP-core would be able to handle their peculiarities in the dedicated objects. Comment to this patch and patches 9/12 and 12/12: Currently i2c-designware-platdrv.c is about 500 lines of code so I don't think it's too overcomplicated. But I feel we have already too many Kconfig options and source modules for i2c-designware and obviously would like to push back a little from adding more. I don't think i2c-designware-platdrv.c becomes yet too complicated if Baikal related code is added there, perhaps under #ifdef CONFIG_OF like MSCC Ocelot code is currently. Well, it's up to you to decide, what solution is more suitable for you to maintain. My idea of detaching the MSCC and Baikal-T1 code to the dedicated source files was to eventually move the whole i2c-designware-* set of files into a dedicated directory drivers/i2c/buses/dw as it's done for some others Synopsys DesignWare controllers: drivers/pci/controller/dwc/, drivers/usb/dwc2, drivers/usb/dwc3, drivers/net/ethernet/synopsys/ . If you think, that it's too early for Dw I2C code to live in a dedicated directory, fine with me. I can merge the MSCC and Baikal-T1 code back into the i2c-designware-platdrv.c . So what's your final word in this matter? I think sub directory decision under each subsystem is more subsystem rather than vendor/driver specific. Good point anyway. For this patchset I'd like more if changes are done to i2c-designware-platdrv.c since it's not too complicated yet :-) If it starts to look too messy in the future then it's time split I think. Jarkko
Re: [GIT PULL] EFI changes for v5.8
* Ard Biesheuvel wrote: > Ingo, Thomsd, Boris, > > Please pull the changes below. Note that I did not incorporate the GOT > handling changes for the x86 decompressor - Arvind has some changes on > top that might just as well go in at the same time, and they are not > really EFI changes anyway. > > The following changes since commit 4da0b2b7e67524cc206067865666899bc02e1cb0: > > efi/libstub: Re-enable command line initrd loading for x86 (2020-04-25 > 12:26:32 +0200) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git > tags/efi-changes-for-v5.8 > > for you to fetch changes up to 9241dfe7f2772fc73c82eb950afb1c795d2c012c: > > efi/x86: Drop the special GDT for the EFI thunk (2020-05-24 00:25:15 +0200) > > Cc: Ingo Molnar , > Cc: Borislav Petkov > Cc: Thomas Gleixner > Cc: Arvind Sankar > > > More EFI changes for v5.8: > - Rename pr_efi/pr_efi_err to efi_info/efi_err, and use them consistently > - Simplify and unify initrd loading > - Parse the builtin command line on x86 (if provided) > - Implement printk() support, including support for wide character strings > - Some fixes for issues introduced by the first batch of v5.8 changes > - Fix a missing prototypes warning > - Simplify GDT handling in early mixed mode thunking code > - Some other minor fixes and cleanups > 24 files changed, 1211 insertions(+), 313 deletions(-) Pulled, thanks a lot Ard! Will push it out after a bit of testing. There was one conflict, in drivers/firmware/efi/libstub/efistub.h, with a straightforward resolution. Ingo
Re: [PATCH v13 2/2] media: v4l: xilinx: Add Xilinx MIPI CSI-2 Rx Subsystem driver
Hi Vishal, thanks. I have only a few minor nitpicking comments. On 12/05/20 17:19, Vishal Sagar wrote: > The Xilinx MIPI CSI-2 Rx Subsystem soft IP is used to capture images > from MIPI CSI-2 camera sensors and output AXI4-Stream video data ready > for image processing. Please refer to PG232 for details. > > The CSI2 Rx controller filters out all packets except for the packets > with data type fixed in hardware. RAW8 packets are always allowed to > pass through. > > It is also used to setup and handle interrupts and enable the core. It > logs all the events in respective counters between streaming on and off. > > The driver supports only the video format bridge enabled configuration. > Some data types like YUV 422 10bpc, RAW16, RAW20 are supported when the > CSI v2.0 feature is enabled in design. When the VCX feature is enabled, > the maximum number of virtual channels becomes 16 from 4. > > Signed-off-by: Vishal Sagar > Reviewed-by: Hyun Kwon > Reviewed-by: Laurent Pinchart [...] > +static int xcsi2rxss_start_stream(struct xcsi2rxss_state *state) > +{ > + int ret = 0; > + > + /* enable core */ > + xcsi2rxss_set(state, XCSI_CCR_OFFSET, XCSI_CCR_ENABLE); > + > + ret = xcsi2rxss_soft_reset(state); > + if (ret < 0) { 'if (ret)' is enough, it's a classic nonzero-on-error return value. > +/** > + * xcsi2rxss_irq_handler - Interrupt handler for CSI-2 > + * @irq: IRQ number > + * @data: Pointer to device state > + * > + * In the interrupt handler, a list of event counters are updated for > + * corresponding interrupts. This is useful to get status / debug. > + * > + * Return: IRQ_HANDLED after handling interrupts > + */ > +static irqreturn_t xcsi2rxss_irq_handler(int irq, void *data) > +{ > + struct xcsi2rxss_state *state = (struct xcsi2rxss_state *)data; > + struct device *dev = state->dev; > + u32 status; > + > + status = xcsi2rxss_read(state, XCSI_ISR_OFFSET) & XCSI_ISR_ALLINTR_MASK; > + xcsi2rxss_write(state, XCSI_ISR_OFFSET, status); > + > + /* Received a short packet */ > + if (status & XCSI_ISR_SPFIFONE) { > + u32 count = 0; > + > + /* > + * Drain generic short packet FIFO by reading max 31 > + * (fifo depth) short packets from fifo or till fifo is empty. > + */ > + for (count = 0; count < XCSI_SPKT_FIFO_DEPTH; ++count) { > + u32 spfifostat, spkt; > + > + spkt = xcsi2rxss_read(state, XCSI_SPKTR_OFFSET); > + dev_dbg(dev, "Short packet = 0x%08x\n", spkt); > + spfifostat = xcsi2rxss_read(state, XCSI_ISR_OFFSET); > + spfifostat &= XCSI_ISR_SPFIFONE; > + if (!spfifostat) > + break; > + xcsi2rxss_write(state, XCSI_ISR_OFFSET, spfifostat); > + } > + } > + > + /* Short packet FIFO overflow */ > + if (status & XCSI_ISR_SPFIFOF) > + dev_dbg_ratelimited(dev, "Short packet FIFO overflowed\n"); > + > + /* > + * Stream line buffer full > + * This means there is a backpressure from downstream IP > + */ > + if (status & XCSI_ISR_SLBF) { > + dev_alert_ratelimited(dev, "Stream Line Buffer Full!\n"); > + > + /* disable interrupts */ > + xcsi2rxss_clr(state, XCSI_IER_OFFSET, XCSI_IER_INTR_MASK); > + xcsi2rxss_clr(state, XCSI_GIER_OFFSET, XCSI_GIER_GIE); > + > + /* disable core */ > + xcsi2rxss_clr(state, XCSI_CCR_OFFSET, XCSI_CCR_ENABLE); > + state->streaming = false; > + > + /* > + * The IP needs to be hard reset before it can be used now. > + * This will be done in streamoff. > + */ > + > + /* > + * TODO: Notify the whole pipeline with v4l2_subdev_notify() to > + * inform userspace. > + */ > + } > + > + /* Increment event counters */ > + if (status & XCSI_ISR_ALLINTR_MASK) { > + unsigned int i; > + > + for (i = 0; i < XCSI_NUM_EVENTS; i++) { > + if (!(status & xcsi2rxss_events[i].mask)) > + continue; > + state->events[i]++; > + dev_dbg_ratelimited(dev, "%s: %u\n", > + xcsi2rxss_events[i].name, > + state->events[i]); > + } > + > + if (status & XCSI_ISR_VCXFE && state->en_vcx) { > + u32 vcxstatus; > + > + vcxstatus = xcsi2rxss_read(state, XCSI_VCXR_OFFSET); > + vcxstatus &= XCSI_VCXR_VCERR; > + for (i = 0; i < XCSI_VCX_NUM_EVENTS; i++) { > + if (!(vcxstatus & (1 << i))) You can use BIT(i) instead of (1 << i). > +/** > + * xcsi2rxss_set_format - This is
Re: [PATCH] can: mcp251x: convert to half-duplex SPI
On 5/25/20 2:57 PM, Mark Brown wrote: > On Mon, May 25, 2020 at 02:41:31PM +0200, Marc Kleine-Budde wrote: >> On 5/25/20 1:31 PM, Mark Brown wrote: > >>> This isn't something that every individual driver should be doing, such >>> rewriting should happen in the core so that everything sees the benefit. > >> The core could merge several half duplex transfers (until there's as >> cs_change) >> into a single full duplex transfer. > > Yes, that is what I am suggesting. Where in the SPI stack do you see such a "merge" function? One point to clarify is when and where to allocate and free the memory for the contiguous full duplex buffers. >> I think it's not easy to detect and reliable to split a full duplex transfer >> into half duplex ones. How can you tell, if the controller is supposed to tx >> 0x0 >> or actually receive. > > I don't understand how that could possibly work or why it would make > sense? ACK, I was just thinking loud about options. >> I think spi_write_then_read() can be extended to generate one full duplex >> transfer instead on two half duplex ones it does a memcpy() anyways. > > This has the same problem as doing it in any other driver code - it > causes a needless incompatibility with three wire and single duplex > devices. What about the note "portable code should never use this for more than 32 bytes" in spi_write_then_read()? The CAN driver in question may read more than 32 bytes of data. >> To get a feeling for the use cases, this is what I do in the regmap read >> function of a (not yet mainlined) CAN SPI driver. > > Like I say it's probably better if code like this gets pushed into the > SPI core where we've got more information about what the controller can > do and there's more win from doing the tuning since more devices and > systems can take advantage of it. ACK Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH] sctp: check assoc before SCTP_ADDR_{MADE_PRIM,ADDED} event
On Mon, May 25, 2020 at 04:42:16PM +0800, Xin Long wrote: > On Sat, May 23, 2020 at 8:04 PM Jonas Falkevik > wrote: > > > > On Tue, May 19, 2020 at 10:42 PM Marcelo Ricardo Leitner > > wrote: > > > > > > On Fri, May 15, 2020 at 10:30:29AM +0200, Jonas Falkevik wrote: > > > > On Wed, May 13, 2020 at 11:32 PM Marcelo Ricardo Leitner > > > > wrote: > > > > > > > > > > On Wed, May 13, 2020 at 10:11:05PM +0200, Jonas Falkevik wrote: > > > > > > On Wed, May 13, 2020 at 6:01 PM Marcelo Ricardo Leitner > > > > > > wrote: > > > > > > > > > > > > > > On Wed, May 13, 2020 at 04:52:16PM +0200, Jonas Falkevik wrote: > > > > > > > > Do not generate SCTP_ADDR_{MADE_PRIM,ADDED} events for > > > > > > > > SCTP_FUTURE_ASSOC assocs. > > > > > > > > > > > > > > How did you get them? > > > > > > > > > > > > > > > > > > > I think one case is when receiving INIT chunk in > > > > > > sctp_sf_do_5_1B_init(). > > > > > > Here a closed association is created, sctp_make_temp_assoc(). > > > > > > Which is later used when calling sctp_process_init(). > > > > > > In sctp_process_init() one of the first things are to call > > > > > > sctp_assoc_add_peer() > > > > > > on the closed / temp assoc. > > > > > > > > > > > > sctp_assoc_add_peer() are generating the SCTP_ADDR_ADDED event on > > > > > > the socket > > > > > > for the potentially new association. > > > > > > > > > > I see, thanks. The SCTP_FUTURE_ASSOC means something different. It is > > > > > for setting/getting socket options that will be used for new asocs. In > > > > > this case, it is just a coincidence that asoc_id is not set (but > > > > > initialized to 0) and SCTP_FUTURE_ASSOC is also 0. > > > > > > > > yes, you are right, I overlooked that. > > > > > > > > > Moreso, if I didn't > > > > > miss anything, it would block valid events, such as those from > > > > > sctp_sf_do_5_1D_ce > > > > >sctp_process_init > > > > > because sctp_process_init will only call sctp_assoc_set_id() by its > > > > > end. > > > > > > > > Do we want these events at this stage? > > > > Since the association is a newly established one, have the peer address > > > > changed? > > > > Should we enqueue these messages with sm commands instead? > > > > And drop them if we don't have state SCTP_STATE_ESTABLISHED? > > > > > > > > > > > > > > I can't see a good reason for generating any event on temp assocs. So > > > > > I'm thinking the checks on this patch should be on whether the asoc is > > > > > a temporary one instead. WDYT? > > > > > > > > > > > > > Agree, we shouldn't rely on coincidence. > > > > Either check temp instead or the above mentioned state? > > > > > > > > > Then, considering the socket is locked, both code points should be > > > > > allocating the IDR earlier. It's expensive, yes (point being, it could > > > > > be avoided in case of other failures), but it should be generating > > > > > events with the right assoc id. Are you interested in pursuing this > > > > > fix as well? > > > > > > > > Sure. > > > > > > > > If we check temp status instead, we would need to allocate IDR earlier, > > > > as you mention. So that we send the notification with correct assoc id. > > > > > > > > But shouldn't the SCTP_COMM_UP, for a newly established association, be > > > > the > > > > first notification event sent? > > > > The SCTP_COMM_UP notification is enqueued later in sctp_sf_do_5_1D_ce(). > > > > > > The RFC doesn't mention any specific ordering for them, but it would > > > make sense. Reading the FreeBSD code now (which I consider a reference > > > implementation), it doesn't raise these notifications from > > > INIT_ACK/COOKIE_ECHO at all. The only trigger for SCTP_ADDR_ADDED > > > event is ASCONF ADD command itself. So these are extra in Linux, and > > > I'm afraid we got to stick with them. > > > > > > Considering the error handling it already has, looks like the > > > reordering is feasible and welcomed. I'm thinking the temp check and > > > reordering is the best way forward here. > > > > > > Thoughts? Neil? Xin? The assoc_id change might be considered an UAPI > > > breakage. > > > > Some order is mentioned in RFC 6458 Chapter 6.1.1. > > > > SCTP_COMM_UP: A new association is now ready, and data may be > > exchanged with this peer. When an association has been > > established successfully, this notification should be the > > first one. Oh, nice finding. > If this is true, as SCTP_COMM_UP event is always followed by state changed > to ESTABLISHED. So I'm thinking to NOT make addr events by checking the > state: > > @@ -343,6 +343,9 @@ void sctp_ulpevent_nofity_peer_addr_change(struct > sctp_transport *transport, > struct sockaddr_storage addr; > struct sctp_ulpevent *event; > > + if (asoc->state < SCTP_STATE_ESTABLISHED) > + return; > + > memset(, 0, sizeof(struct sockaddr_storage)); > memcpy(, >ipaddr, > transport->af_specific->sockaddr_len); With the above said, yep. Thanks. > >
[PATCH] iio: dac: ad5592r: remove usage of iio_priv_to_dev() helper
This was partially removed when the mlock cleanup was done. Only one more call is left in the ad5592r_alloc_channels() function. This one is simple. We just need to pass the iio_dev object and get the state via iio_priv(). Signed-off-by: Alexandru Ardelean --- drivers/iio/dac/ad5592r-base.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c index 410e90e5f75f..272c97bb841c 100644 --- a/drivers/iio/dac/ad5592r-base.c +++ b/drivers/iio/dac/ad5592r-base.c @@ -508,11 +508,11 @@ static void ad5592r_setup_channel(struct iio_dev *iio_dev, chan->ext_info = ad5592r_ext_info; } -static int ad5592r_alloc_channels(struct ad5592r_state *st) +static int ad5592r_alloc_channels(struct iio_dev *iio_dev) { + struct ad5592r_state *st = iio_priv(iio_dev); unsigned i, curr_channel = 0, num_channels = st->num_channels; - struct iio_dev *iio_dev = iio_priv_to_dev(st); struct iio_chan_spec *channels; struct fwnode_handle *child; u32 reg, tmp; @@ -636,7 +636,7 @@ int ad5592r_probe(struct device *dev, const char *name, if (ret) goto error_disable_reg; - ret = ad5592r_alloc_channels(st); + ret = ad5592r_alloc_channels(iio_dev); if (ret) goto error_disable_reg; -- 2.25.1
Re: [PATCH] iommu: Fix group refcount in iommu_alloc_default_domain()
Hi Joerg, On 2020-05-25 18:32, Joerg Roedel wrote: Hi, On Fri, May 22, 2020 at 06:31:45PM +0530, Sai Prakash Ranjan wrote: diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index a4c2f122eb8b..05f7b77c432f 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1491,6 +1491,7 @@ static int iommu_alloc_default_domain(struct device *dev) { struct iommu_group *group; unsigned int type; + int ret; group = iommu_group_get(dev); if (!group) @@ -1501,7 +1502,11 @@ static int iommu_alloc_default_domain(struct device *dev) type = iommu_get_def_domain_type(dev); - return iommu_group_alloc_default_domain(dev->bus, group, type); + ret = iommu_group_alloc_default_domain(dev->bus, group, type); + + iommu_group_put(group); + + return ret; } /** Thanks for the report and the fix. I think it is better to fix this by not taking a group reference in iommu_alloc_default_domain() at all and pass group as a parameter. Please see the patch I just sent out. Thanks for the patch, it looks like the right thing to do. Testing it out now. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCH v2 2/2] phy: Remove CONFIG_ARCH_* check for related subdir in Makefile
If CONFIG_ARCH_ROCKCHIP is not set but COMPILE_TEST is set, the file in the subdir rockchip can not be built due to CONFIG_ARCH_ROCKCHIP check in drivers/phy/Makefile. Since the related configs in drivers/phy/rockchip/Kconfig depend on ARCH_ROCKCHIP, so remove CONFIG_ARCH_ROCKCHIP check for subdir rockchip in drivers/phy/Makefile. The other CONFIG_ARCH_* about allwinner, amlogic, mediatek, renesas and tegra have the same situation, so remove them too. Signed-off-by: Tiezhu Yang --- v2: - Remove all the CONFIG_ARCH_* check for related subdir in Makefile - Modify the patch subject and update commit message drivers/phy/Makefile | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 310c149..16e2622 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -8,24 +8,24 @@ obj-$(CONFIG_GENERIC_PHY_MIPI_DPHY) += phy-core-mipi-dphy.o obj-$(CONFIG_PHY_LPC18XX_USB_OTG) += phy-lpc18xx-usb-otg.o obj-$(CONFIG_PHY_XGENE)+= phy-xgene.o obj-$(CONFIG_PHY_PISTACHIO_USB)+= phy-pistachio-usb.o -obj-$(CONFIG_ARCH_SUNXI) += allwinner/ -obj-$(CONFIG_ARCH_MESON) += amlogic/ -obj-$(CONFIG_ARCH_MEDIATEK)+= mediatek/ -obj-$(CONFIG_ARCH_RENESAS) += renesas/ -obj-$(CONFIG_ARCH_ROCKCHIP)+= rockchip/ -obj-$(CONFIG_ARCH_TEGRA) += tegra/ -obj-y += broadcom/\ +obj-y += allwinner/ \ + amlogic/ \ + broadcom/\ cadence/ \ freescale/ \ hisilicon/ \ intel/ \ lantiq/ \ marvell/ \ + mediatek/\ motorola/\ mscc/\ qualcomm/\ ralink/ \ + renesas/ \ + rockchip/\ samsung/ \ socionext/ \ st/ \ + tegra/ \ ti/ -- 2.1.0
[PATCH v2 1/2] phy: rockchip: Fix return value of inno_dsidphy_probe()
When call function devm_platform_ioremap_resource(), we should use IS_ERR() to check the return value and return PTR_ERR() if failed. Fixes: b7535a3bc0ba ("phy/rockchip: Add support for Innosilicon MIPI/LVDS/TTL PHY") Signed-off-by: Tiezhu Yang Reviewed-by: Heiko Stuebner --- v2: - No changes, just add Reviewed-by tag drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c index a7c6c94..8af8c6c 100644 --- a/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c +++ b/drivers/phy/rockchip/phy-rockchip-inno-dsidphy.c @@ -607,8 +607,8 @@ static int inno_dsidphy_probe(struct platform_device *pdev) platform_set_drvdata(pdev, inno); inno->phy_base = devm_platform_ioremap_resource(pdev, 0); - if (!inno->phy_base) - return -ENOMEM; + if (IS_ERR(inno->phy_base)) + return PTR_ERR(inno->phy_base); inno->ref_clk = devm_clk_get(dev, "ref"); if (IS_ERR(inno->ref_clk)) { -- 2.1.0
Re: [PATCH] arm: dts: am33xx-bone-common: add gpio-line-names
On Mon, May 25, 2020 at 2:07 PM Drew Fustini wrote: > On Mon, May 25, 2020 at 11:23:17AM +0200, Linus Walleij wrote: > > On Thu, May 21, 2020 at 12:02 AM Drew Fustini wrote: > > > > > I've posted a v2 which I hope improves the intent of the line names. [0] > > > > > > I'm happy to integrate any feedback and create a v3 - especially if it > > > is prefered for me to list the specific peripherial signals instead of > > > an abstract term like "[ethernet]" or "[emmc]". This is for lines that > > > can not be used because they are not routed to the expansion headers. > > > > > > [0] https://lore.kernel.org/linux-omap/20200520214757.GA362547@x1/T/#u > > > > This looks good to me. FWIW > > Acked-by: Linus Walleij > > > > Yours, > > Linus Walleij > > Linus - > > I have posted a newer patch that targets am335x-beagleblack.dts [0] > instead of am335x-bone-common.dtsi as Grygorii Strashko pointed out > that these line names are not applicable to all BeagleBone models. > > The gpio line naming scheme is the same, is it ok to add your Ack? Yes FWIW Acked-by. Linus Walleij
Re: [PATCH 05/16] bcache: use bio_{start,end}_io_acct
On 2020/5/25 19:30, Christoph Hellwig wrote: > Switch bcache to use the nicer bio accounting helpers, and call the > routines where we also sample the start time to give coherent accounting > results. > > Signed-off-by: Christoph Hellwig Acked-by: Coly Li Coly Li > --- > drivers/md/bcache/request.c | 18 -- > 1 file changed, 4 insertions(+), 14 deletions(-) > > diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c > index 77d1a26975174..22b483527176b 100644 > --- a/drivers/md/bcache/request.c > +++ b/drivers/md/bcache/request.c > @@ -668,9 +668,7 @@ static void backing_request_endio(struct bio *bio) > static void bio_complete(struct search *s) > { > if (s->orig_bio) { > - generic_end_io_acct(s->d->disk->queue, bio_op(s->orig_bio), > - >d->disk->part0, s->start_time); > - > + bio_end_io_acct(s->orig_bio, s->start_time); > trace_bcache_request_end(s->d, s->orig_bio); > s->orig_bio->bi_status = s->iop.status; > bio_endio(s->orig_bio); > @@ -730,7 +728,7 @@ static inline struct search *search_alloc(struct bio *bio, > s->recoverable = 1; > s->write= op_is_write(bio_op(bio)); > s->read_dirty_data = 0; > - s->start_time = jiffies; > + s->start_time = bio_start_io_acct(bio); > > s->iop.c= d->c; > s->iop.bio = NULL; > @@ -1082,8 +1080,7 @@ static void detached_dev_end_io(struct bio *bio) > bio->bi_end_io = ddip->bi_end_io; > bio->bi_private = ddip->bi_private; > > - generic_end_io_acct(ddip->d->disk->queue, bio_op(bio), > - >d->disk->part0, ddip->start_time); > + bio_end_io_acct(bio, ddip->start_time); > > if (bio->bi_status) { > struct cached_dev *dc = container_of(ddip->d, > @@ -1108,7 +1105,7 @@ static void detached_dev_do_request(struct > bcache_device *d, struct bio *bio) >*/ > ddip = kzalloc(sizeof(struct detached_dev_io_private), GFP_NOIO); > ddip->d = d; > - ddip->start_time = jiffies; > + ddip->start_time = bio_start_io_acct(bio); > ddip->bi_end_io = bio->bi_end_io; > ddip->bi_private = bio->bi_private; > bio->bi_end_io = detached_dev_end_io; > @@ -1190,11 +1187,6 @@ blk_qc_t cached_dev_make_request(struct request_queue > *q, struct bio *bio) > } > } > > - generic_start_io_acct(q, > - bio_op(bio), > - bio_sectors(bio), > - >disk->part0); > - > bio_set_dev(bio, dc->bdev); > bio->bi_iter.bi_sector += dc->sb.data_offset; > > @@ -1311,8 +1303,6 @@ blk_qc_t flash_dev_make_request(struct request_queue > *q, struct bio *bio) > return BLK_QC_T_NONE; > } > > - generic_start_io_acct(q, bio_op(bio), bio_sectors(bio), > >disk->part0); > - > s = search_alloc(bio, d); > cl = >cl; > bio = >bio.bio; >
Re: [RFC PATCH v2 0/3] Prefer working VT console over SPCR and device-tree chosen stdout-path
On Fri 2020-05-15 22:27:02, Alper Nebi Yasak wrote: > On 13/05/2020 17:37, Petr Mladek wrote: > > On Thu 2020-04-30 19:14:34, Alper Nebi Yasak wrote: > I think things run roughly in the following order (from what I can > decipher from kernel messages) and I think it matches your explanations: > > |ACPI SPCR| dt chosen stdout-path | > +=+=+ > | acpi_parse_spcr() | | > | -> add_preferred_console(uart0) | | > |(if not on x86) | | > +-+-+ > |console_setup()| > |-> add_preferred_console(tty0) | > | (if console=tty0) | > +-+-+ > |register_console(vt) | > +-+-+ > | | of_console_check() | > | | -> add_preferred_console(uart2) | > | |(if no console arg) | > +-+-+ > |register_console(serial) | > +-+-+ I was first a bit confused by the above table. The order looks fine but I was not sure about the indentation. I think that some more details are needed to get the picture and context. I see the following order in start_kernel(): 1. Add spcr consoles: by acpi_parse_spcr() called from setup_arch(). 2. Add and register early consoles: by parse_early_param() 3. Add normal consoles from command line: by parse_args() 4. Register tty console: by vty_init() called via long chain from fs_initcall(chr_dev_init). It seems to be init call in 5th round, see include/linux/init.h 5. Register other (serial) consoles are most likely registered from device_initcall() in 6th round, see include/linux/init.h. The consoles defined by the device tree are not added directly. Instead, the probe() callbacks checks whether such console is selected in device tree by of_console_check() called from uart_add_one_port(). > > My suggestion is: > > > > + Fix SPCR setting or device tree of your device when the defaults > > are not as expected. > > Maybe I can get QEMU's SPCR use conditional on the existence a > framebuffer, and get distributions to remove stdout-path from certain > device-trees; but that would disable the serial console completely > (instead of having it enabled where tty0 is still preferred). I am afraid that this is a problem with many defaults. They might be good enough for many people but others would want something else. It might be acceptable to add consoles. But it might be a problem to remove consoles or change the currently preferred one. The only exception would be when most people are annoyed with the current default. But this need to be discussed with people familiar with the given architecture or device. > > + Use command line to force your value when the defaults are not > > as expected and you could not change them. > > This works; but I'd have to know the machine's serial configuration in > advance to put it in the cmdline as "console= console=tty0", or > lose the serial console as in the above. (A "console=dt" like that > "console=spcr" patch you linked to would be useful here if it existed.) The generic parameters: console=tty, console=serial, console=dt, console=spcr looks fine to me. IMHO, the only problem might be when a particular serial console drive is not able to guess reasonable defaults for the baud rate, etc. Best Regards, Petr
Re: [PATCH] RDMA/core: fix missing release in add_port.
On Mon, May 25, 2020 at 01:06:56AM -0500, wu000...@umn.edu wrote: > From: Qiushi Wu > > In function add_port(), pointer p is not released in error paths. > Fix this issue by adding a kfree(p) into the end of error path. > > Signed-off-by: Qiushi Wu > drivers/infiniband/core/sysfs.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c > index 087682e6969e..04a003378dfc 100644 > +++ b/drivers/infiniband/core/sysfs.c > @@ -1202,6 +1202,7 @@ static int add_port(struct ib_core_device *coredev, int > port_num) > > err_put: > kobject_put(>kobj); > + kfree(p); > return ret; > } Er, no, kobject_put does the kfree There is a bug here, it is calling kfree() after kobject_init_and_add fails, which is wrong, it should be goto err_put Jason
Re: [PATCH] iommu: Fix group refcount in iommu_alloc_default_domain()
Hi, On Fri, May 22, 2020 at 06:31:45PM +0530, Sai Prakash Ranjan wrote: > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index a4c2f122eb8b..05f7b77c432f 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -1491,6 +1491,7 @@ static int iommu_alloc_default_domain(struct device > *dev) > { > struct iommu_group *group; > unsigned int type; > + int ret; > > group = iommu_group_get(dev); > if (!group) > @@ -1501,7 +1502,11 @@ static int iommu_alloc_default_domain(struct device > *dev) > > type = iommu_get_def_domain_type(dev); > > - return iommu_group_alloc_default_domain(dev->bus, group, type); > + ret = iommu_group_alloc_default_domain(dev->bus, group, type); > + > + iommu_group_put(group); > + > + return ret; > } > > /** Thanks for the report and the fix. I think it is better to fix this by not taking a group reference in iommu_alloc_default_domain() at all and pass group as a parameter. Please see the patch I just sent out. Regards, Joerg
[PATCH] serial: 8250: probe all 16550A variants by default
From: Vladimir Oltean On NXP T1040, the UART is typically detected as 16550A_FSL64. After said patch, it gets detected as plain 16550A and the Linux console is completely garbled and missing characters. So clearly, introducing the SERIAL_8250_16550A_VARIANTS config option has broken many existing users because it has changed the default behavior. Restore that by adding a 'default y' to this option. Users who care about 20 ms shorter boot time can always disable it, but stop wasting many debugging hours for people who don't care all that much. Fixes: dc56ecb81a0a ("serial: 8250: Support disabling mdelay-filled probes of 16550A variants") Signed-off-by: Vladimir Oltean --- drivers/tty/serial/8250/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig index af0688156dd0..89c7ecb55619 100644 --- a/drivers/tty/serial/8250/Kconfig +++ b/drivers/tty/serial/8250/Kconfig @@ -63,6 +63,7 @@ config SERIAL_8250_PNP config SERIAL_8250_16550A_VARIANTS bool "Support for variants of the 16550A serial port" depends on SERIAL_8250 + default y help The 8250 driver can probe for many variants of the venerable 16550A serial port. Doing so takes additional time at boot. -- 2.25.1
Re: [PATCH 2/2] usb: serial: xr_serial: Add gpiochip support
On Mon, May 25, 2020 at 1:12 PM Greg KH wrote: > > I remember I even referred to this myself, but I've been waning a bit > > on it recently, because it turns out that userspace/users aren't very > > good at parsing sysfs for topology. > > Which is why they could use libudev :) Yet they insist on using things like Busybox' mdev (e.g. OpenWrt) or Android ... or is Android using libudev now? I'd be delighted if they did. Yours, Linus Walleij
[PATCH] iommu: Don't take group reference in iommu_alloc_default_domain()
From: Joerg Roedel The iommu_alloc_default_domain() function takes a reference to an IOMMU group without releasing it. This causes the group to never be released, with undefined side effects. The function has only one call-site, which takes a group reference on its own, so to fix this leak, do not take another reference in iommu_alloc_default_domain() and pass the group as a function parameter instead. Reference: https://lore.kernel.org/lkml/20200522130145.30067-1-saiprakash.ran...@codeaurora.org/ Reported-by: Sai Prakash Ranjan Cc: Sai Prakash Ranjan Fixes: 6e1aa2049154 ("iommu: Move default domain allocation to iommu_probe_device()") Signed-off-by: Joerg Roedel --- drivers/iommu/iommu.c | 21 + 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 374b34fd6fac..bf20674769e0 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -80,7 +80,8 @@ static bool iommu_cmd_line_dma_api(void) return !!(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API); } -static int iommu_alloc_default_domain(struct device *dev); +static int iommu_alloc_default_domain(struct iommu_group *group, + struct device *dev); static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus, unsigned type); static int __iommu_attach_device(struct iommu_domain *domain, @@ -251,17 +252,17 @@ int iommu_probe_device(struct device *dev) if (ret) goto err_out; + group = iommu_group_get(dev); + if (!group) + goto err_release; + /* * Try to allocate a default domain - needs support from the * IOMMU driver. There are still some drivers which don't * support default domains, so the return value is not yet * checked. */ - iommu_alloc_default_domain(dev); - - group = iommu_group_get(dev); - if (!group) - goto err_release; + iommu_alloc_default_domain(group, dev); if (group->default_domain) ret = __iommu_attach_device(group->default_domain, dev); @@ -1478,15 +1479,11 @@ static int iommu_group_alloc_default_domain(struct bus_type *bus, return 0; } -static int iommu_alloc_default_domain(struct device *dev) +static int iommu_alloc_default_domain(struct iommu_group *group, + struct device *dev) { - struct iommu_group *group; unsigned int type; - group = iommu_group_get(dev); - if (!group) - return -ENODEV; - if (group->default_domain) return 0; -- 2.26.2
[PATCH] [v2] media: coda: Fix runtime PM imbalance in coda_probe
When coda_firmware_request() returns an error code, a pairing runtime PM usage counter decrement is needed to keep the counter balanced. Signed-off-by: Dinghao Liu --- Changelog: v2: - Remove changes to coda_remove(), which is incorrect. --- drivers/media/platform/coda/coda-common.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c index d0d093dd8f7c..6e246c7a56c3 100644 --- a/drivers/media/platform/coda/coda-common.c +++ b/drivers/media/platform/coda/coda-common.c @@ -3119,6 +3119,8 @@ static int coda_probe(struct platform_device *pdev) return 0; err_alloc_workqueue: + pm_runtime_disable(>dev); + pm_runtime_put_noidle(>dev); destroy_workqueue(dev->workqueue); err_v4l2_register: v4l2_device_unregister(>v4l2_dev); -- 2.17.1
Re: [PATCH v2 07/12] i2c: designware: Move Baytrail sem config to the platform if-clause
On 5/21/20 5:22 AM, Serge Semin wrote: On Wed, May 20, 2020 at 03:16:14PM +0300, Jarkko Nikula wrote: On 5/10/20 12:50 PM, Serge Semin wrote: Currently Intel Baytrail I2C semaphore is a feature of the DW APB I2C platform driver. It's a bit confusing to see it's config in the menu at some separated place with no reference to the platform code. Lets move the config definition under the if-I2C_DESIGNWARE_PLATFORM clause. By doing so the config menu will display the feature right below the DW I2C platform driver item and will indent it to the right so signifying its belonging. Signed-off-by: Serge Semin Cc: Alexey Malahov Cc: Thomas Bogendoerfer Cc: Paul Burton Cc: Ralf Baechle Cc: Andy Shevchenko Cc: Mika Westerberg Cc: Wolfram Sang Cc: Rob Herring Cc: Frank Rowand Cc: linux-m...@vger.kernel.org Cc: devicet...@vger.kernel.org --- drivers/i2c/busses/Kconfig | 30 +- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 368aa64e9266..ed6927c4c540 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -530,8 +530,8 @@ config I2C_DESIGNWARE_CORE config I2C_DESIGNWARE_PLATFORM tristate "Synopsys DesignWare Platform" - select I2C_DESIGNWARE_CORE depends on (ACPI && COMMON_CLK) || !ACPI + select I2C_DESIGNWARE_CORE help If you say yes to this option, support will be included for the Synopsys DesignWare I2C adapter. @@ -539,6 +539,22 @@ config I2C_DESIGNWARE_PLATFORM This driver can also be built as a module. If so, the module will be called i2c-designware-platform. +if I2C_DESIGNWARE_PLATFORM + +config I2C_DESIGNWARE_BAYTRAIL + bool "Intel Baytrail I2C semaphore support" + depends on ACPI + depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \ + (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y) + help + This driver enables managed host access to the PMIC I2C bus on select + Intel BayTrail platforms using the X-Powers AXP288 PMIC. It allows + the host to request uninterrupted access to the PMIC's I2C bus from + the platform firmware controlling it. You should say Y if running on + a BayTrail system using the AXP288. + +endif # I2C_DESIGNWARE_PLATFORM + Is the added "if I2C_DESIGNWARE_PLATFORM" needed here? Should the "depends on" be enough? The idea was to add if-endif clause here for features possibly added sometime in future. But using normal "depends on I2C_DESIGNWARE_PLATFORM" shall make the config depicted as an indented sub-config as well. Would you like me to remove the if-clause and use the depends on operator instead? Yes, please remove it from this patch. Keeps this patch simpler and if some future feature needs it then that patch(set) is the right place to add it. Jarkko
Re: [PATCH v5 2/3] dt-bindings: arm: actions: Document Caninos Loucos Labrador
Hi Andreas, Em 5/25/20 7:41 AM, Andreas Färber escreveu: Hi, Am 25.05.20 um 03:30 schrieb Matheus Castello: Update the documentation to add the Caninos Loucos Labrador. Labrador project consists of a computer on module based on the Actions Semi S500 processor and the Labrador base board. Signed-off-by: Matheus Castello Acked-by: Rob Herring --- Documentation/devicetree/bindings/arm/actions.yaml | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/actions.yaml b/Documentation/devicetree/bindings/arm/actions.yaml index ace3fdaa8396..2187e1c5bc73 100644 --- a/Documentation/devicetree/bindings/arm/actions.yaml +++ b/Documentation/devicetree/bindings/arm/actions.yaml @@ -19,6 +19,11 @@ properties: - allo,sparky # Allo.com Sparky - cubietech,cubieboard6 # Cubietech CubieBoard6 - const: actions,s500 + - items: + - enum: + - caninos,labrador-v2 # Labrador Core v2 + - caninos,labrador-base-m # Labrador Base Board M v1 This enum still strikes me as wrong, it means either-or. (Was planning to look into it myself, but no time yet...) caninos,labrador-v2 should be a const one level down: board, SoM, SoC from most specific to most generic. Compare Guitar below. I got it, I agree make sense, I will send the v6. + - const: actions,s500 - items: - enum: - lemaker,guitar-bb-rev-b # LeMaker Guitar Base Board rev. B Regards, Andreas Best Regards, Matheus Castello
Re: [PATCH v3 10/16] gpio: add a reusable generic gpio_chip using regmap
On Mon, May 25, 2020 at 12:20 PM Michael Walle wrote: > If you like I could submit this patch on its own. But then there > wouldn't be a user for it. I'm pretty much fine with that, we do merge code that has no users if we anticipate they will be around the corner. Yours, Linus Walleij
[PATCH v2 4/4] pwm: jz4740: Add support for the JZ4725B
The PWM hardware in the JZ4725B works the same as in the JZ4740, but has only six channels available. Signed-off-by: Paul Cercueil --- Notes: v2: Simply return -EINVAL if we can't get match data drivers/pwm/pwm-jz4740.c | 24 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c index fe06ca8ce30f..5830ac2bdf6a 100644 --- a/drivers/pwm/pwm-jz4740.c +++ b/drivers/pwm/pwm-jz4740.c @@ -20,7 +20,9 @@ #include #include -#define NUM_PWM 8 +struct soc_info { + unsigned int num_pwms; +}; struct jz4740_pwm_chip { struct pwm_chip chip; @@ -36,7 +38,7 @@ static bool jz4740_pwm_can_use_chn(struct jz4740_pwm_chip *jz, unsigned int channel) { /* Enable all TCU channels for PWM use by default except channels 0/1 */ - u32 pwm_channels_mask = GENMASK(NUM_PWM - 1, 2); + u32 pwm_channels_mask = GENMASK(jz->chip.npwm - 1, 2); device_property_read_u32(jz->chip.dev->parent, "ingenic,pwm-channels-mask", @@ -226,6 +228,11 @@ static int jz4740_pwm_probe(struct platform_device *pdev) { struct device *dev = >dev; struct jz4740_pwm_chip *jz4740; + const struct soc_info *info; + + info = device_get_match_data(dev); + if (!info) + return -EINVAL; jz4740 = devm_kzalloc(dev, sizeof(*jz4740), GFP_KERNEL); if (!jz4740) @@ -239,7 +246,7 @@ static int jz4740_pwm_probe(struct platform_device *pdev) jz4740->chip.dev = dev; jz4740->chip.ops = _pwm_ops; - jz4740->chip.npwm = NUM_PWM; + jz4740->chip.npwm = info->num_pwms; jz4740->chip.base = -1; jz4740->chip.of_xlate = of_pwm_xlate_with_flags; jz4740->chip.of_pwm_n_cells = 3; @@ -256,9 +263,18 @@ static int jz4740_pwm_remove(struct platform_device *pdev) return pwmchip_remove(>chip); } +static const struct soc_info __maybe_unused jz4740_soc_info = { + .num_pwms = 8, +}; + +static const struct soc_info __maybe_unused jz4725b_soc_info = { + .num_pwms = 6, +}; + #ifdef CONFIG_OF static const struct of_device_id jz4740_pwm_dt_ids[] = { - { .compatible = "ingenic,jz4740-pwm", }, + { .compatible = "ingenic,jz4740-pwm", .data = _soc_info }, + { .compatible = "ingenic,jz4725b-pwm", .data = _soc_info }, {}, }; MODULE_DEVICE_TABLE(of, jz4740_pwm_dt_ids); -- 2.26.2
Re: [PATCH] can: mcp251x: convert to half-duplex SPI
On Mon, May 25, 2020 at 02:41:31PM +0200, Marc Kleine-Budde wrote: > On 5/25/20 1:31 PM, Mark Brown wrote: > > This isn't something that every individual driver should be doing, such > > rewriting should happen in the core so that everything sees the benefit. > The core could merge several half duplex transfers (until there's as > cs_change) > into a single full duplex transfer. Yes, that is what I am suggesting. > I think it's not easy to detect and reliable to split a full duplex transfer > into half duplex ones. How can you tell, if the controller is supposed to tx > 0x0 > or actually receive. I don't understand how that could possibly work or why it would make sense? > I think spi_write_then_read() can be extended to generate one full duplex > transfer instead on two half duplex ones it does a memcpy() anyways. This has the same problem as doing it in any other driver code - it causes a needless incompatibility with three wire and single duplex devices. > To get a feeling for the use cases, this is what I do in the regmap read > function of a (not yet mainlined) CAN SPI driver. Like I say it's probably better if code like this gets pushed into the SPI core where we've got more information about what the controller can do and there's more win from doing the tuning since more devices and systems can take advantage of it. signature.asc Description: PGP signature
[PATCH v2 1/4] pwm: jz4740: Drop dependency on MACH_INGENIC
Depending on MACH_INGENIC prevent us from creating a generic kernel that works on more than one MIPS board. Instead, we just depend on MIPS being set. Signed-off-by: Paul Cercueil Acked-by: Uwe Kleine-König --- Notes: v2: No change drivers/pwm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index eebbc917ac97..7814e5b2cad7 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -234,7 +234,7 @@ config PWM_IMX_TPM config PWM_JZ4740 tristate "Ingenic JZ47xx PWM support" - depends on MACH_INGENIC + depends on MIPS depends on COMMON_CLK select MFD_SYSCON help -- 2.26.2
[PATCH v2 2/4] pwm: jz4740: Enhance precision in calculation of duty cycle
Calculating the hardware value for the duty from the hardware value of the period resulted in a precision loss versus calculating it from the clock rate directly. (Also remove a cast that doesn't really need to be here) Signed-off-by: Paul Cercueil --- Notes: v2: New patch. I don't consider this a fix but an enhancement, since the old behaviour was in place since the driver was born in ~2010, so no Fixes tag. drivers/pwm/pwm-jz4740.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c index 3cd5c054ad9a..4fe9d99ac9a9 100644 --- a/drivers/pwm/pwm-jz4740.c +++ b/drivers/pwm/pwm-jz4740.c @@ -158,11 +158,11 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, /* Calculate period value */ tmp = (unsigned long long)rate * state->period; do_div(tmp, NSEC_PER_SEC); - period = (unsigned long)tmp; + period = tmp; /* Calculate duty value */ - tmp = (unsigned long long)period * state->duty_cycle; - do_div(tmp, state->period); + tmp = (unsigned long long)rate * state->duty_cycle; + do_div(tmp, NSEC_PER_SEC); duty = period - tmp; if (duty >= period) -- 2.26.2
[PATCH v2 3/4] pwm: jz4740: Make PWM start with the active part
The PWM in Ingenic SoCs starts in inactive state until the internal timer reaches the duty value, then becomes active until the timer reaches the period value. In theory, we should then use (period - duty) as the real duty value, as a high duty value would otherwise result in the PWM pin being inactive most of the time. This is the reason why the duty value was inverted in the driver until now, but it still had the problem that it would not start with the active part. To address this remaining issue, the common trick is to invert the duty, and invert the polarity when the PWM is enabled. Since the duty was already inverted, and we invert it again, we now program the hardware for the requested duty, and simply invert the polarity when the PWM is enabled. Signed-off-by: Paul Cercueil --- Notes: v2: Add documentation about why we invert the polarity, and improve commit message drivers/pwm/pwm-jz4740.c | 25 - 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c index 4fe9d99ac9a9..fe06ca8ce30f 100644 --- a/drivers/pwm/pwm-jz4740.c +++ b/drivers/pwm/pwm-jz4740.c @@ -6,7 +6,6 @@ * Limitations: * - The .apply callback doesn't complete the currently running period before * reconfiguring the hardware. - * - Each period starts with the inactive part. */ #include @@ -163,7 +162,7 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, /* Calculate duty value */ tmp = (unsigned long long)rate * state->duty_cycle; do_div(tmp, NSEC_PER_SEC); - duty = period - tmp; + duty = tmp; if (duty >= period) duty = period - 1; @@ -189,18 +188,26 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm, regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm), TCU_TCSR_PWM_SD, TCU_TCSR_PWM_SD); - /* Set polarity */ - switch (state->polarity) { - case PWM_POLARITY_NORMAL: + /* +* Set polarity. +* +* The PWM starts in inactive state until the internal timer reaches the +* duty value, then becomes active until the timer reaches the period +* value. In theory, we should then use (period - duty) as the real duty +* value, as a high duty value would otherwise result in the PWM pin +* being inactive most of the time. +* +* Here, we don't do that, and instead invert the polarity of the PWM +* when it is active. This trick makes the PWM start with its active +* state instead of its inactive state. +*/ + if ((state->polarity == PWM_POLARITY_NORMAL) ^ state->enabled) regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm), TCU_TCSR_PWM_INITL_HIGH, 0); - break; - case PWM_POLARITY_INVERSED: + else regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm), TCU_TCSR_PWM_INITL_HIGH, TCU_TCSR_PWM_INITL_HIGH); - break; - } if (state->enabled) jz4740_pwm_enable(chip, pwm); -- 2.26.2
Re: [PATCH] RDMA/core: Complete exception handling in add_port()
> In function add_port(), pointer p is not released in error paths. 1. I would prefer to describe that an ib_port data structure was not released in some error cases. How relevant can its size be here? > Fix this issue by adding a kfree(p) into the end of error path. 2. I suggest to improve also this change description. 3. I find an other subject more appropriate. … > +++ b/drivers/infiniband/core/sysfs.c > @@ -1202,6 +1202,7 @@ static int add_port(struct ib_core_device *coredev, > int port_num) > > err_put: > kobject_put(>kobj); > + kfree(p); > return ret; > } 4. I recommend to add also the label “free_port” before the missed function call. Another source code place should be accordingly adjusted then. ret = kobject_init_and_add(>kobj, _type, coredev->ports_kobj, "%d", port_num); - if (ret) { - kfree(p); - return ret; - } + if (ret) + goto free_port; 5. Will a similar adjustment be needed for the data structure member “gid_attr_group” according to the desired complete exception handling? Regards, Markus
[PATCH v3 5/6] mm: tlb: Provide flush_*_tlb_range wrappers
This patch provides flush_{pte|pmd|pud|p4d}_tlb_range() in generic code, which are expressed through the mmu_gather APIs. These interface set tlb->cleared_* and finally call tlb_flush(), so we can do the tlb invalidation according to the information in struct mmu_gather. Signed-off-by: Zhenyu Ye --- include/asm-generic/pgtable.h | 12 ++-- mm/pgtable-generic.c | 22 ++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h index 329b8c8ca703..8c92122ded9b 100644 --- a/include/asm-generic/pgtable.h +++ b/include/asm-generic/pgtable.h @@ -1161,11 +1161,19 @@ static inline int pmd_free_pte_page(pmd_t *pmd, unsigned long addr) * invalidate the entire TLB which is not desitable. * e.g. see arch/arc: flush_pmd_tlb_range */ -#define flush_pmd_tlb_range(vma, addr, end)flush_tlb_range(vma, addr, end) -#define flush_pud_tlb_range(vma, addr, end)flush_tlb_range(vma, addr, end) +extern void flush_pte_tlb_range(struct vm_area_struct *vma, + unsigned long addr, unsigned long end); +extern void flush_pmd_tlb_range(struct vm_area_struct *vma, + unsigned long addr, unsigned long end); +extern void flush_pud_tlb_range(struct vm_area_struct *vma, + unsigned long addr, unsigned long end); +extern void flush_p4d_tlb_range(struct vm_area_struct *vma, + unsigned long addr, unsigned long end); #else +#define flush_pte_tlb_range(vma, addr, end)BUILD_BUG() #define flush_pmd_tlb_range(vma, addr, end)BUILD_BUG() #define flush_pud_tlb_range(vma, addr, end)BUILD_BUG() +#define flush_p4d_tlb_range(vma, addr, end)BUILD_BUG() #endif #endif diff --git a/mm/pgtable-generic.c b/mm/pgtable-generic.c index 3d7c01e76efc..3eff199d3507 100644 --- a/mm/pgtable-generic.c +++ b/mm/pgtable-generic.c @@ -101,6 +101,28 @@ pte_t ptep_clear_flush(struct vm_area_struct *vma, unsigned long address, #ifdef CONFIG_TRANSPARENT_HUGEPAGE +#ifndef __HAVE_ARCH_FLUSH_PMD_TLB_RANGE + +#define FLUSH_Pxx_TLB_RANGE(_pxx) \ +void flush_##_pxx##_tlb_range(struct vm_area_struct *vma, \ + unsigned long addr, unsigned long end)\ +{ \ + struct mmu_gather tlb; \ + \ + tlb_gather_mmu(, vma->vm_mm, addr, end);\ + tlb_start_vma(, vma); \ + tlb_flush_##_pxx##_range(, addr, end - addr); \ + tlb_end_vma(, vma); \ + tlb_finish_mmu(, addr, end);\ +} + +FLUSH_Pxx_TLB_RANGE(pte) +FLUSH_Pxx_TLB_RANGE(pmd) +FLUSH_Pxx_TLB_RANGE(pud) +FLUSH_Pxx_TLB_RANGE(p4d) + +#endif /* __HAVE_ARCH_FLUSH_PMD_TLB_RANGE */ + #ifndef __HAVE_ARCH_PMDP_SET_ACCESS_FLAGS int pmdp_set_access_flags(struct vm_area_struct *vma, unsigned long address, pmd_t *pmdp, -- 2.19.1
[PATCH v3 2/6] arm64: Add level-hinted TLB invalidation helper
From: Marc Zyngier Add a level-hinted TLB invalidation helper that only gets used if ARMv8.4-TTL gets detected. Signed-off-by: Marc Zyngier Signed-off-by: Zhenyu Ye Reviewed-by: Catalin Marinas --- arch/arm64/include/asm/tlbflush.h | 29 + 1 file changed, 29 insertions(+) diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h index bc3949064725..8adbd6fd8489 100644 --- a/arch/arm64/include/asm/tlbflush.h +++ b/arch/arm64/include/asm/tlbflush.h @@ -10,6 +10,7 @@ #ifndef __ASSEMBLY__ +#include #include #include #include @@ -59,6 +60,34 @@ __ta; \ }) +#define TLBI_TTL_MASK GENMASK_ULL(47, 44) + +#define __tlbi_level(op, addr, level) do { \ + u64 arg = addr; \ + \ + if (cpus_have_const_cap(ARM64_HAS_ARMv8_4_TTL) && \ + level) {\ + u64 ttl = level;\ + \ + switch (PAGE_SIZE) {\ + case SZ_4K: \ + ttl |= 1 << 2; \ + break; \ + case SZ_16K:\ + ttl |= 2 << 2; \ + break; \ + case SZ_64K:\ + ttl |= 3 << 2; \ + break; \ + } \ + \ + arg &= ~TLBI_TTL_MASK; \ + arg |= FIELD_PREP(TLBI_TTL_MASK, ttl); \ + } \ + \ + __tlbi(op, arg); \ +} while (0) + /* * TLB Invalidation * -- 2.19.1
[PATCH v3 0/6] arm64: tlb: add support for TTL feature
In order to reduce the cost of TLB invalidation, ARMv8.4 provides the TTL field in TLBI instruction. The TTL field indicates the level of page table walk holding the leaf entry for the address being invalidated. This series provide support for this feature. When ARMv8.4-TTL is implemented, the operand for TLBIs looks like below: * +--+---+--+ * | ASID | TTL |BADDR | * +--+---+--+ * |63 48|47 44|43 0| This version updates some codes implementation according to Peter's suggestion, and adds some commit msg. See patches for details, Thanks. -- ChangeList: v3: minor changes: reduce the indentation levels of __tlbi_level(). v2: rebase series on Linux 5.7-rc1 and simplify the code implementation. v1: add support for TTL feature in arm64. Marc Zyngier (2): arm64: Detect the ARMv8.4 TTL feature arm64: Add level-hinted TLB invalidation helper Peter Zijlstra (Intel) (1): tlb: mmu_gather: add tlb_flush_*_range APIs Zhenyu Ye (3): arm64: Add tlbi_user_level TLB invalidation helper mm: tlb: Provide flush_*_tlb_range wrappers arm64: tlb: Set the TTL field in flush_tlb_range arch/arm64/include/asm/cpucaps.h | 3 +- arch/arm64/include/asm/sysreg.h | 1 + arch/arm64/include/asm/tlb.h | 29 +++- arch/arm64/include/asm/tlbflush.h | 49 +++ arch/arm64/kernel/cpufeature.c| 11 +++ include/asm-generic/pgtable.h | 12 +-- include/asm-generic/tlb.h | 55 ++- mm/pgtable-generic.c | 22 + 8 files changed, 157 insertions(+), 25 deletions(-) -- 2.19.1
[PATCH v3 1/6] arm64: Detect the ARMv8.4 TTL feature
From: Marc Zyngier In order to reduce the cost of TLB invalidation, the ARMv8.4 TTL feature allows TLBs to be issued with a level allowing for quicker invalidation. Let's detect the feature for now. Further patches will implement its actual usage. Signed-off-by: Marc Zyngier Signed-off-by: Zhenyu Ye Reviewed-by: Catalin Marinas --- arch/arm64/include/asm/cpucaps.h | 3 ++- arch/arm64/include/asm/sysreg.h | 1 + arch/arm64/kernel/cpufeature.c | 11 +++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h index 8eb5a088ae65..cabb0c49a1d1 100644 --- a/arch/arm64/include/asm/cpucaps.h +++ b/arch/arm64/include/asm/cpucaps.h @@ -61,7 +61,8 @@ #define ARM64_HAS_AMU_EXTN 51 #define ARM64_HAS_ADDRESS_AUTH 52 #define ARM64_HAS_GENERIC_AUTH 53 +#define ARM64_HAS_ARMv8_4_TTL 54 -#define ARM64_NCAPS54 +#define ARM64_NCAPS55 #endif /* __ASM_CPUCAPS_H */ diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index c4ac0ac25a00..477d84ba1056 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -725,6 +725,7 @@ /* id_aa64mmfr2 */ #define ID_AA64MMFR2_E0PD_SHIFT60 +#define ID_AA64MMFR2_TTL_SHIFT 48 #define ID_AA64MMFR2_FWB_SHIFT 40 #define ID_AA64MMFR2_AT_SHIFT 32 #define ID_AA64MMFR2_LVA_SHIFT 16 diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 9fac745aa7bb..d993dc6dc7d5 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -244,6 +244,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = { static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = { ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_E0PD_SHIFT, 4, 0), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_TTL_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_FWB_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_AT_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_LVA_SHIFT, 4, 0), @@ -1622,6 +1623,16 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .matches = has_cpuid_feature, .cpu_enable = cpu_has_fwb, }, + { + .desc = "ARMv8.4 Translation Table Level", + .type = ARM64_CPUCAP_SYSTEM_FEATURE, + .capability = ARM64_HAS_ARMv8_4_TTL, + .sys_reg = SYS_ID_AA64MMFR2_EL1, + .sign = FTR_UNSIGNED, + .field_pos = ID_AA64MMFR2_TTL_SHIFT, + .min_field_value = 1, + .matches = has_cpuid_feature, + }, #ifdef CONFIG_ARM64_HW_AFDBM { /* -- 2.19.1
[PATCH v3 4/6] tlb: mmu_gather: add tlb_flush_*_range APIs
From: "Peter Zijlstra (Intel)" tlb_flush_{pte|pmd|pud|p4d}_range() adjust the tlb->start and tlb->end, then set corresponding cleared_*. Signed-off-by: Peter Zijlstra (Intel) Signed-off-by: Zhenyu Ye Acked-by: Catalin Marinas --- include/asm-generic/tlb.h | 55 --- 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h index 3f1649a8cf55..ef75ec86f865 100644 --- a/include/asm-generic/tlb.h +++ b/include/asm-generic/tlb.h @@ -512,6 +512,38 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vm } #endif +/* + * tlb_flush_{pte|pmd|pud|p4d}_range() adjust the tlb->start and tlb->end, + * and set corresponding cleared_*. + */ +static inline void tlb_flush_pte_range(struct mmu_gather *tlb, +unsigned long address, unsigned long size) +{ + __tlb_adjust_range(tlb, address, size); + tlb->cleared_ptes = 1; +} + +static inline void tlb_flush_pmd_range(struct mmu_gather *tlb, +unsigned long address, unsigned long size) +{ + __tlb_adjust_range(tlb, address, size); + tlb->cleared_pmds = 1; +} + +static inline void tlb_flush_pud_range(struct mmu_gather *tlb, +unsigned long address, unsigned long size) +{ + __tlb_adjust_range(tlb, address, size); + tlb->cleared_puds = 1; +} + +static inline void tlb_flush_p4d_range(struct mmu_gather *tlb, +unsigned long address, unsigned long size) +{ + __tlb_adjust_range(tlb, address, size); + tlb->cleared_p4ds = 1; +} + #ifndef __tlb_remove_tlb_entry #define __tlb_remove_tlb_entry(tlb, ptep, address) do { } while (0) #endif @@ -525,19 +557,17 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vm */ #define tlb_remove_tlb_entry(tlb, ptep, address) \ do {\ - __tlb_adjust_range(tlb, address, PAGE_SIZE);\ - tlb->cleared_ptes = 1; \ + tlb_flush_pte_range(tlb, address, PAGE_SIZE); \ __tlb_remove_tlb_entry(tlb, ptep, address); \ } while (0) #define tlb_remove_huge_tlb_entry(h, tlb, ptep, address) \ do {\ unsigned long _sz = huge_page_size(h); \ - __tlb_adjust_range(tlb, address, _sz); \ if (_sz == PMD_SIZE)\ - tlb->cleared_pmds = 1; \ + tlb_flush_pmd_range(tlb, address, _sz); \ else if (_sz == PUD_SIZE) \ - tlb->cleared_puds = 1; \ + tlb_flush_pud_range(tlb, address, _sz); \ __tlb_remove_tlb_entry(tlb, ptep, address); \ } while (0) @@ -551,8 +581,7 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vm #define tlb_remove_pmd_tlb_entry(tlb, pmdp, address) \ do {\ - __tlb_adjust_range(tlb, address, HPAGE_PMD_SIZE); \ - tlb->cleared_pmds = 1; \ + tlb_flush_pmd_range(tlb, address, HPAGE_PMD_SIZE); \ __tlb_remove_pmd_tlb_entry(tlb, pmdp, address); \ } while (0) @@ -566,8 +595,7 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vm #define tlb_remove_pud_tlb_entry(tlb, pudp, address) \ do {\ - __tlb_adjust_range(tlb, address, HPAGE_PUD_SIZE); \ - tlb->cleared_puds = 1; \ + tlb_flush_pud_range(tlb, address, HPAGE_PUD_SIZE); \ __tlb_remove_pud_tlb_entry(tlb, pudp, address); \ } while (0) @@ -592,9 +620,8 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vm #ifndef pte_free_tlb #define pte_free_tlb(tlb, ptep, address) \ do {\ - __tlb_adjust_range(tlb, address, PAGE_SIZE);\ + tlb_flush_pmd_range(tlb, address, PAGE_SIZE); \ tlb->freed_tables = 1; \ - tlb->cleared_pmds = 1; \ __pte_free_tlb(tlb, ptep, address); \ } while (0) #endif @@ -602,9 +629,8 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vm #ifndef pmd_free_tlb #define pmd_free_tlb(tlb, pmdp, address)
[PATCH v3 3/6] arm64: Add tlbi_user_level TLB invalidation helper
Add a level-hinted parameter to __tlbi_user, which only gets used if ARMv8.4-TTL gets detected. ARMv8.4-TTL provides the TTL field in tlbi instruction to indicate the level of translation table walk holding the leaf entry for the address that is being invalidated. This patch set the default level value to 0. Signed-off-by: Zhenyu Ye --- arch/arm64/include/asm/tlbflush.h | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h index 8adbd6fd8489..969dcf88e2a9 100644 --- a/arch/arm64/include/asm/tlbflush.h +++ b/arch/arm64/include/asm/tlbflush.h @@ -88,6 +88,12 @@ __tlbi(op, arg); \ } while (0) +#define __tlbi_user_level(op, arg, level) do { \ + if (arm64_kernel_unmapped_at_el0()) \ + __tlbi_level(op, (arg | USER_ASID_FLAG), level);\ +} while (0) + + /* * TLB Invalidation * @@ -230,11 +236,11 @@ static inline void __flush_tlb_range(struct vm_area_struct *vma, dsb(ishst); for (addr = start; addr < end; addr += stride) { if (last_level) { - __tlbi(vale1is, addr); - __tlbi_user(vale1is, addr); + __tlbi_level(vale1is, addr, 0); + __tlbi_user_level(vale1is, addr, 0); } else { - __tlbi(vae1is, addr); - __tlbi_user(vae1is, addr); + __tlbi_level(vae1is, addr, 0); + __tlbi_user_level(vae1is, addr, 0); } } dsb(ish); -- 2.19.1
[PATCH v3 6/6] arm64: tlb: Set the TTL field in flush_tlb_range
This patch uses the cleared_* in struct mmu_gather to set the TTL field in flush_tlb_range(). Signed-off-by: Zhenyu Ye --- arch/arm64/include/asm/tlb.h | 29 - arch/arm64/include/asm/tlbflush.h | 14 -- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/arch/arm64/include/asm/tlb.h b/arch/arm64/include/asm/tlb.h index b76df828e6b7..61c97d3b58c7 100644 --- a/arch/arm64/include/asm/tlb.h +++ b/arch/arm64/include/asm/tlb.h @@ -21,11 +21,37 @@ static void tlb_flush(struct mmu_gather *tlb); #include +/* + * get the tlbi levels in arm64. Default value is 0 if more than one + * of cleared_* is set or neither is set. + * Arm64 doesn't support p4ds now. + */ +static inline int tlb_get_level(struct mmu_gather *tlb) +{ + if (tlb->cleared_ptes && !(tlb->cleared_pmds || + tlb->cleared_puds || + tlb->cleared_p4ds)) + return 3; + + if (tlb->cleared_pmds && !(tlb->cleared_ptes || + tlb->cleared_puds || + tlb->cleared_p4ds)) + return 2; + + if (tlb->cleared_puds && !(tlb->cleared_ptes || + tlb->cleared_pmds || + tlb->cleared_p4ds)) + return 1; + + return 0; +} + static inline void tlb_flush(struct mmu_gather *tlb) { struct vm_area_struct vma = TLB_FLUSH_VMA(tlb->mm, 0); bool last_level = !tlb->freed_tables; unsigned long stride = tlb_get_unmap_size(tlb); + int tlb_level = tlb_get_level(tlb); /* * If we're tearing down the address space then we only care about @@ -38,7 +64,8 @@ static inline void tlb_flush(struct mmu_gather *tlb) return; } - __flush_tlb_range(, tlb->start, tlb->end, stride, last_level); + __flush_tlb_range(, tlb->start, tlb->end, stride, + last_level, tlb_level); } static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t pte, diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h index 969dcf88e2a9..ba2f6b544cb7 100644 --- a/arch/arm64/include/asm/tlbflush.h +++ b/arch/arm64/include/asm/tlbflush.h @@ -214,7 +214,8 @@ static inline void flush_tlb_page(struct vm_area_struct *vma, static inline void __flush_tlb_range(struct vm_area_struct *vma, unsigned long start, unsigned long end, -unsigned long stride, bool last_level) +unsigned long stride, bool last_level, +int tlb_level) { unsigned long asid = ASID(vma->vm_mm); unsigned long addr; @@ -236,11 +237,11 @@ static inline void __flush_tlb_range(struct vm_area_struct *vma, dsb(ishst); for (addr = start; addr < end; addr += stride) { if (last_level) { - __tlbi_level(vale1is, addr, 0); - __tlbi_user_level(vale1is, addr, 0); + __tlbi_level(vale1is, addr, tlb_level); + __tlbi_user_level(vale1is, addr, tlb_level); } else { - __tlbi_level(vae1is, addr, 0); - __tlbi_user_level(vae1is, addr, 0); + __tlbi_level(vae1is, addr, tlb_level); + __tlbi_user_level(vae1is, addr, tlb_level); } } dsb(ish); @@ -252,8 +253,9 @@ static inline void flush_tlb_range(struct vm_area_struct *vma, /* * We cannot use leaf-only invalidation here, since we may be invalidating * table entries as part of collapsing hugepages or moving page tables. +* Set the tlb_level to 0 because we can not get enough information here. */ - __flush_tlb_range(vma, start, end, PAGE_SIZE, false); + __flush_tlb_range(vma, start, end, PAGE_SIZE, false, 0); } static inline void flush_tlb_kernel_range(unsigned long start, unsigned long end) -- 2.19.1
Re: Re: [PATCH] media: coda: Fix runtime PM imbalance in coda_probe
> Hi Dinghao, > > thank you for the patch! The first part is fine, but I think the second > part is not necessary, see below: > > On Sat, May 23, 2020 at 06:03:32PM +0800, Dinghao Liu wrote: > > When coda_firmware_request() returns an error code, > > a pairing runtime PM usage counter decrement is needed > > to keep the counter balanced. > > > > Also, the caller expects coda_probe() to increase PM > > usage counter, there should be a refcount decrement > > in coda_remove() to keep the counter balanced. > > coda_probe() increments the usage counter only until coda_fw_callback() > decrements it again. Where is the imbalance? > You are right, I missed coda_firmware_request() before and thank you for your correction! I will fix this in the next edition of patch. Regards, Dinghao > > Signed-off-by: Dinghao Liu > > --- > > drivers/media/platform/coda/coda-common.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/media/platform/coda/coda-common.c > > b/drivers/media/platform/coda/coda-common.c > > index d0d093dd8f7c..550e9a1266da 100644 > > --- a/drivers/media/platform/coda/coda-common.c > > +++ b/drivers/media/platform/coda/coda-common.c > > @@ -3119,6 +3119,8 @@ static int coda_probe(struct platform_device *pdev) > > return 0; > > > > err_alloc_workqueue: > > + pm_runtime_disable(>dev); > > + pm_runtime_put_noidle(>dev); > > These seem right, they balance out the pm_runtime_enable() > and pm_runtime_get_noresume() right before the error. > > > destroy_workqueue(dev->workqueue); > > err_v4l2_register: > > v4l2_device_unregister(>v4l2_dev); > > @@ -3136,6 +3138,7 @@ static int coda_remove(struct platform_device *pdev) > > } > > if (dev->m2m_dev) > > v4l2_m2m_release(dev->m2m_dev); > > + pm_runtime_put_noidle(>dev); > > I think this is incorrect. There is one pm_runtime_get_noresume() in > coda_probe(), and one pm_runtime_put_sync() in coda_fw_callback(). > By the time coda_remove() is called, balance is already restored. > > regards > Philipp
linux-next: Tree for May 25
Hi all, Changes since 20200522: New trees: uniphier and uniphier-fixes My fixes tree contains: cd2b06ec45d6 ("device_cgroup: Fix RCU list debugging warning") The mips tree gained conflicts against Linus' tree. The nand tree gained a build failure for which I applied a patch. The drm tree gained conflicts against Linus' tree. The drm-msm tree still had its build failure so I applied a patch. The block tree lost its build failure but gained a semantic conflict against the btrfs tree. The tip tree lost its build problems. The rcu tree lost its build problems. The notifications tree gained a conflict and a semantic conflict against the vfs tree. The akpm-current tree gained conflicts against the btrfs and tip trees. The akpm tree gained a build failure for which I applied a patch. Non-merge commits (relative to Linus' tree): 11444 12290 files changed, 693336 insertions(+), 193947 deletions(-) I have created today's linux-next tree at git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (patches at http://www.kernel.org/pub/linux/kernel/next/ ). If you are tracking the linux-next tree using git, you should not use "git pull" to do so as that will try to merge the new linux-next release with the old one. You should use "git fetch" and checkout or reset to the new master. You can see which trees have been included by looking in the Next/Trees file in the source. There are also quilt-import.log and merge.log files in the Next directory. Between each merge, the tree was built with a ppc64_defconfig for powerpc, an allmodconfig for x86_64, a multi_v7_defconfig for arm and a native build of tools/perf. After the final fixups (if any), I do an x86_64 modules_install followed by builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc and sparc64 defconfig and htmldocs. And finally, a simple boot test of the powerpc pseries_le_defconfig kernel in qemu (with and without kvm enabled). Below is a summary of the state of the merge. I am currently merging 324 trees (counting Linus' and 82 trees of bug fix patches pending for the current merge release). Stats about the size of the tree over time can be seen at http://neuling.org/linux-next-size.html . Status of my local build tests will be at http://kisskb.ellerman.id.au/linux-next . If maintainers want to give advice about cross compilers/configs that work, we are always open to add more builds. Thanks to Randy Dunlap for doing many randconfig builds. And to Paul Gortmaker for triage and bug fixes. -- Cheers, Stephen Rothwell $ git checkout master $ git reset --hard stable Merging origin/master (98790bbac4db Merge tag 'efi-urgent-2020-05-24' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip) Merging fixes/master (cd2b06ec45d6 device_cgroup: Fix RCU list debugging warning) Merging kbuild-current/fixes (6a8b55ed4056 Linux 5.7-rc3) Merging arc-current/for-curr (7915502377c5 ARC: show_regs: avoid extra line of output) Merging arm-current/fixes (3866f217aaa8 ARM: 8977/1: ptrace: Fix mask for thumb breakpoint hook) Merging arm-soc-fixes/arm/fixes (ccffeae7afa4 Merge branch 'v5.7-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/matthias.bgg/linux into arm/fixes) Merging uniphier-fixes/fixes (0e698dfa2822 Linux 5.7-rc4) Merging arm64-fixes/for-next/fixes (8cfb347ad0cf arm64: Add get_user() type annotation on the !access_ok() path) Merging m68k-current/for-linus (86cded5fc525 m68k: defconfig: Update defconfigs for v5.6-rc4) Merging powerpc-fixes/fixes (8659a0e0efdd powerpc/64s: Disable STRICT_KERNEL_RWX) Merging s390-fixes/fixes (4c1cbcbd6c56 s390/kaslr: add support for R_390_JMP_SLOT relocation type) Merging sparc/master (fcdf818d239e Merge branch 'sparc-scnprintf') Merging fscrypt-current/for-stable (2b4eae95c736 fscrypt: don't evict dirty inodes after removing key) Merging net/master (98790bbac4db Merge tag 'efi-urgent-2020-05-24' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip) Merging bpf/master (d04322a0da1e Merge tag 'rxrpc-fixes-20200523-v2' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs) Merging ipsec/master (3ffb93ba326f esp4: improve xfrm4_beet_gso_segment() to be more readable) Merging netfilter/master (5a730153984d net: sun: fix missing release regions in cas_init_one().) Merging ipvs/master (0141317611ab Merge branch 'hns3-fixes') Merging wireless-drivers/master (f92f26f2ed2c iwlwifi: pcie: handle QuZ configs with killer NICs as well) Merging mac80211/master (4a3de90b1184 mac80211: sta_info: Add lockdep condition for RCU list usage) Merging rdma-fixes/for-rc (189277f3814c RDMA/mlx5: Fix NULL pointer dereference in destroy_prefetch_work) Merging sound-current/for-linus (259eb8247531 ALSA: hda/realtek - Add more fixup entries for Clevo machines) Merging sound-asoc-fixes/for-linus (d51683021891 Merge
Re: [PATCHv3 2/3] optee: use uuid for sysfs driver entry
On Mon, May 25, 2020 at 02:52:34PM +0300, Maxim Uvarov wrote: > Optee device names for sysfs needed to be unique > and it's better if they will mean something. UUID for name > looks like good solution: > /sys/bus/tee/devices/optee-clnt- Can you document that in Documentation/ABI/ ? And why UUID? Those are usually huge, is that easier than just a unique number? thanks, greg k-h
Re: [PATCH] i2c: core: fix NULL pointer dereference in suspend/resume callbacks
Hi Tomasz On 25.05.2020 14:28, Tomasz Figa wrote: > On Fri, May 22, 2020 at 1:15 PM Marek Szyprowski > wrote: >> On 22.05.2020 12:13, Marek Szyprowski wrote: >>> Commit 6fe12cdbcfe3 ("i2c: core: support bus regulator controlling in >>> adapter") added generic suspend and resume functions for i2c devices. >>> Those functions unconditionally access an i2c_client structure assigned >>> to the given i2c device. However, there exist i2c devices in the system >>> without a valid i2c_client. Add the needed check before accessing the >>> i2c_client. >> Just one more comment. The devices without i2c_client structure are the >> i2c 'devices' associated with the respective i2c bus. They are visible >> in /sys: >> >> ls -l /sys/bus/i2c/devices/i2c-* >> >> I wonder if this patch has been ever tested with system suspend/resume, >> as those devices are always available in the system... > Sorry for the trouble and thanks a lot for the fix. We'll make sure to > do more thorough testing, including suspend/resume before relanding > this change. > > Since the patch was reverted, can we squash your fix with the next > revision together with your Co-developed-by and Signed-off-by tags? Sure, no problem. The fix is trivial. Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] can: mcp251x: convert to half-duplex SPI
On 5/25/20 1:31 PM, Mark Brown wrote: >>> Should I be submitting this patch with logic that only does >>> half-duplex if the spi controller doesn't support it (if >>> (spi->controller->flags & SPI_CONTROLLER_HALF_DUPLEX)) or is it >>> acceptable to simply make the driver half-duplex like this for all >>> cases? > >> Please make half duplex transfers depending on SPI_CONTROLLER_HALF_DUPLEX as >> most drivers have a considerable overhead at the end of a transfer. > >> Most of them wait for a transfer complete interrupt. Which might take longer >> than the actual SPI transfer. Splitting one full duplex read-register >> transfer >> (which is a write followed by a read) into two half duplex transfers would >> kill >> performance on full duplex capable controllers. > > This isn't something that every individual driver should be doing, such > rewriting should happen in the core so that everything sees the benefit. The core could merge several half duplex transfers (until there's as cs_change) into a single full duplex transfer. I think it's not easy to detect and reliable to split a full duplex transfer into half duplex ones. How can you tell, if the controller is supposed to tx 0x0 or actually receive. I think spi_write_then_read() can be extended to generate one full duplex transfer instead on two half duplex ones it does a memcpy() anyways. To get a feeling for the use cases, this is what I do in the regmap read function of a (not yet mainlined) CAN SPI driver. > static int > mcp25xxfd_regmap_nocrc_read(void *context, > const void *reg, size_t reg_len, > void *val_buf, size_t val_len) > { > struct spi_device *spi = context; > struct mcp25xxfd_priv *priv = spi_get_drvdata(spi); > struct mcp25xxfd_map_buf_nocrc *buf_rx = priv->map_buf_nocrc_rx; > struct mcp25xxfd_map_buf_nocrc *buf_tx = priv->map_buf_nocrc_tx; > struct spi_transfer xfer[2] = { }; > struct spi_message msg; > int err; > > spi_message_init(); > spi_message_add_tail([0], ); > > if (priv->devtype_data.quirks & MCP25XXFD_QUIRK_HALF_DUPLEX) { > xfer[0].tx_buf = reg; > xfer[0].len = sizeof(buf_tx->cmd); > > xfer[1].rx_buf = val_buf; > xfer[1].len = val_len; > spi_message_add_tail([1], ); > } else { > xfer[0].tx_buf = buf_tx; > xfer[0].rx_buf = buf_rx; > xfer[0].len = sizeof(buf_tx->cmd) + val_len; > memcpy(_tx->cmd, reg, sizeof(buf_tx->cmd)); > }; > > err = spi_sync(spi, ); > if (err) > return err; > > if (!(priv->devtype_data.quirks & MCP25XXFD_QUIRK_HALF_DUPLEX)) > memcpy(val_buf, buf_rx->data, val_len); > > return 0; > } The tradeoff here is two transfers vs. the the memcpy(). As CAN frames are quite small the memcpy() is usually faster. Even on the rpi, where the driver is optimized for small transfers. regards Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH] media: bdisp: Fix runtime PM imbalance on error
Hi, Looks good to me. Reviewed-by: Fabien Dessenne BR Fabien On 21/05/2020 12:00 pm, Dinghao Liu wrote: > pm_runtime_get_sync() increments the runtime PM usage counter even > when it returns an error code. Thus a pairing decrement is needed on > the error handling path to keep the counter balanced. > > Signed-off-by: Dinghao Liu > --- > drivers/media/platform/sti/bdisp/bdisp-v4l2.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c > b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c > index af2d5eb782ce..e1d150584bdc 100644 > --- a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c > +++ b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c > @@ -1371,7 +1371,7 @@ static int bdisp_probe(struct platform_device *pdev) > ret = pm_runtime_get_sync(dev); > if (ret < 0) { > dev_err(dev, "failed to set PM\n"); > - goto err_dbg; > + goto err_pm; > } > > /* Filters */ > @@ -1399,7 +1399,6 @@ static int bdisp_probe(struct platform_device *pdev) > bdisp_hw_free_filters(bdisp->dev); > err_pm: > pm_runtime_put(dev); > -err_dbg: > bdisp_debugfs_remove(bdisp); > err_v4l2: > v4l2_device_unregister(>v4l2_dev);
linux-next: build warning after merge of the net-next tree
Hi all, After merging the net-next tree, today's linux-next build (sparc64 defconfig) produced this warning: drivers/net/ethernet/intel/e1000e/netdev.c:137:13: warning: 'e1000e_check_me' defined but not used [-Wunused-function] static bool e1000e_check_me(u16 device_id) ^~~ Introduced by commit e086ba2fccda ("e1000e: disable s0ix entry and exit flows for ME systems") CONFIG_PM_SLEEP is not set for this build. -- Cheers, Stephen Rothwell pgpelGr_k_XkL.pgp Description: OpenPGP digital signature
Re: [PATCH V4 00/17] arm64/cpufeature: Introduce ID_PFR2, ID_DFR1, ID_MMFR5 and other changes
On 05/21/2020 08:49 PM, Will Deacon wrote: > On Tue, 19 May 2020 15:10:37 +0530, Anshuman Khandual wrote: >> This series is primarily motivated from an adhoc list from Mark Rutland >> during our previous ID_ISAR6 discussion [1]. The current proposal also >> accommodates some more suggestions from Will and Suzuki. >> >> This series adds missing 32 bit system registers (ID_PFR2, ID_DFR1 and >> ID_MMFR5), adds missing features bits on all existing system registers >> (32 and 64 bit) and some other miscellaneous changes. While here it also >> includes a patch which does macro replacement for various open bits shift >> encodings for various CPU ID registers. There is a slight re-order of the >> patches here as compared to the previous version (V1). >> >> [...] > > Applied to arm64 (for-next/cpufeature), thanks! > > [01/17] arm64/cpufeature: Add explicit ftr_id_isar0[] for ID_ISAR0 register > https://git.kernel.org/arm64/c/2a5bc6c47bc3 > [02/17] arm64/cpufeature: Drop TraceFilt feature exposure from ID_DFR0 > register > https://git.kernel.org/arm64/c/1ed1b90a0594 > [03/17] arm64/cpufeature: Make doublelock a signed feature in ID_AA64DFR0 > https://git.kernel.org/arm64/c/e965bcb06256 > [04/17] arm64/cpufeature: Introduce ID_PFR2 CPU register > https://git.kernel.org/arm64/c/16824085a7dd > [05/17] arm64/cpufeature: Introduce ID_DFR1 CPU register > https://git.kernel.org/arm64/c/dd35ec070457 > [06/17] arm64/cpufeature: Introduce ID_MMFR5 CPU register > https://git.kernel.org/arm64/c/152accf8476f > [07/17] arm64/cpufeature: Add remaining feature bits in ID_PFR0 register > https://git.kernel.org/arm64/c/0ae43a99fe91 > [08/17] arm64/cpufeature: Add remaining feature bits in ID_MMFR4 register > https://git.kernel.org/arm64/c/fcd6535322cc > [09/17] arm64/cpufeature: Add remaining feature bits in ID_AA64ISAR0 register > https://git.kernel.org/arm64/c/7cd51a5a84d1 > [10/17] arm64/cpufeature: Add remaining feature bits in ID_AA64PFR0 register > https://git.kernel.org/arm64/c/011e5f5bf529 > [11/17] arm64/cpufeature: Add remaining feature bits in ID_AA64PFR1 register > https://git.kernel.org/arm64/c/14e270fa5c4c > [12/17] arm64/cpufeature: Add remaining feature bits in ID_AA64MMFR0 register > (no commit info) > [13/17] arm64/cpufeature: Add remaining feature bits in ID_AA64MMFR1 register > (no commit info) > [14/17] arm64/cpufeature: Add remaining feature bits in ID_AA64MMFR2 register > (no commit info) > [15/17] arm64/cpufeature: Add remaining feature bits in ID_AA64DFR0 register > (no commit info) > [16/17] arm64/cpufeature: Replace all open bits shift encodings with macros > (no commit info) > [17/17] arm64/cpuinfo: Add ID_MMFR4_EL1 into the cpuinfo_arm64 context > https://git.kernel.org/arm64/c/858b8a8039d0 > > Note that Suzuki had comments on 12-16, so assume you'll respin those (I fixed > up the trivial comments on earlier patches myself). [PATCH 15/17] might need some more investigation and rework. Hence planning to defer that for later and respin the remaining patches (12, 13, 14, 16) for now. - Anshuman
Re: [PATCH] media: coda: Fix runtime PM imbalance in coda_probe
Hi Dinghao, thank you for the patch! The first part is fine, but I think the second part is not necessary, see below: On Sat, May 23, 2020 at 06:03:32PM +0800, Dinghao Liu wrote: > When coda_firmware_request() returns an error code, > a pairing runtime PM usage counter decrement is needed > to keep the counter balanced. > > Also, the caller expects coda_probe() to increase PM > usage counter, there should be a refcount decrement > in coda_remove() to keep the counter balanced. coda_probe() increments the usage counter only until coda_fw_callback() decrements it again. Where is the imbalance? > Signed-off-by: Dinghao Liu > --- > drivers/media/platform/coda/coda-common.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/media/platform/coda/coda-common.c > b/drivers/media/platform/coda/coda-common.c > index d0d093dd8f7c..550e9a1266da 100644 > --- a/drivers/media/platform/coda/coda-common.c > +++ b/drivers/media/platform/coda/coda-common.c > @@ -3119,6 +3119,8 @@ static int coda_probe(struct platform_device *pdev) > return 0; > > err_alloc_workqueue: > + pm_runtime_disable(>dev); > + pm_runtime_put_noidle(>dev); These seem right, they balance out the pm_runtime_enable() and pm_runtime_get_noresume() right before the error. > destroy_workqueue(dev->workqueue); > err_v4l2_register: > v4l2_device_unregister(>v4l2_dev); > @@ -3136,6 +3138,7 @@ static int coda_remove(struct platform_device *pdev) > } > if (dev->m2m_dev) > v4l2_m2m_release(dev->m2m_dev); > + pm_runtime_put_noidle(>dev); I think this is incorrect. There is one pm_runtime_get_noresume() in coda_probe(), and one pm_runtime_put_sync() in coda_fw_callback(). By the time coda_remove() is called, balance is already restored. regards Philipp
Re: [PATCH] FS: BTRFS: ulist.c: Fixed a brace coding style and space before tab
On Fri, May 22, 2020 at 05:29:02PM -0400, Ethan Edwards wrote: > From: Ethan Edwards > Fixed coding style No thanks. Long answer: https://btrfs.wiki.kernel.org/index.php/Developer%27s_FAQ#How_not_to_start
Re: [PATCH 2/2] phy: Remove CONFIG_ARCH_ROCKCHIP check for subdir rockchip
On 05/25/2020 06:48 PM, Heiko Stübner wrote: Am Montag, 25. Mai 2020, 06:08:59 CEST schrieb Tiezhu Yang: If CONFIG_ARCH_ROCKCHIP is not set but COMPILE_TEST is set, the file in the subdir rockchip can not be built due to CONFIG_ARCH_ROCKCHIP check in drivers/phy/Makefile. Since the related configs in drivers/phy/rockchip/Kconfig depend on ARCH_ROCKCHIP, so remove CONFIG_ARCH_ROCKCHIP check for subdir rockchip in drivers/phy/Makefile. Signed-off-by: Tiezhu Yang wouldn't this make more sense to do for all subdirs? - allwinner: also has arch_sunxi || compile_test in its Kconfig - amlogic: same - mediatek: same - renesas: same - tega: same So I think the right way would be to drop all the obj-$(CONFIG_ARCH_...) options and group the separate directories into the generic subdir listing below them. Hi Heiko, Thanks for your suggestions. I will check it and then send v2. Thanks, Tiezhu Yang Heiko --- drivers/phy/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 310c149..e5b4f58 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -12,7 +12,7 @@ obj-$(CONFIG_ARCH_SUNXI) += allwinner/ obj-$(CONFIG_ARCH_MESON) += amlogic/ obj-$(CONFIG_ARCH_MEDIATEK) += mediatek/ obj-$(CONFIG_ARCH_RENESAS)+= renesas/ -obj-$(CONFIG_ARCH_ROCKCHIP)+= rockchip/ +obj-y += rockchip/ obj-$(CONFIG_ARCH_TEGRA) += tegra/ obj-y += broadcom/\ cadence/ \
Re: [RFC 00/14] perf tests: Check on subtest for user specified test
On Mon, May 25, 2020 at 02:06:07PM +0200, Michael Petlan wrote: > On Mon, 25 May 2020, Jiri Olsa wrote: > > hi, > > changes for using metric result in another metric seem > > to change lot of core metric code, so it's better we > > have some more tests before we do that. > > > > Sending as RFC as it's still alive and you guys might > > have some other idea of how to do this. > > > > Also available in here: > > git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git > > perf/fixes > > > > jirka > > > Hi! > Last commit from perf/fixes branch I see there is: > > commit 0445062df28fef1002302aa419af65fa80513dd4 (HEAD -> perf/fixes, > origin/perf/fixes) > Author: Jiri Olsa > Date: Fri Dec 6 00:10:13 2019 +0100 > > Different branch? ugh.. sorry it's perf/metric_test thanks, jirka
Re: block I/O accounting improvements
On 25/05/2020 14.29, Christoph Hellwig wrote: Hi Jens, they series contains various improvement for block I/O accounting. The first bunch of patches switch the bio based drivers to better accounting helpers compared to the current mess. The end contains a fix and various performanc improvements. Most of this comes from a series Konstantin sent a few weeks ago, rebased on changes that landed in your tree since and my change to always use the percpu version of the disk stats. Thanks for picking this up. One note about possible further improvement in reply to first patch. Reviewed-by: Konstantin Khlebnikov
Re: [PATCH] i2c: core: fix NULL pointer dereference in suspend/resume callbacks
Hi Marek, On Fri, May 22, 2020 at 1:15 PM Marek Szyprowski wrote: > > Hi All, > > On 22.05.2020 12:13, Marek Szyprowski wrote: > > Commit 6fe12cdbcfe3 ("i2c: core: support bus regulator controlling in > > adapter") added generic suspend and resume functions for i2c devices. > > Those functions unconditionally access an i2c_client structure assigned > > to the given i2c device. However, there exist i2c devices in the system > > without a valid i2c_client. Add the needed check before accessing the > > i2c_client. > > Just one more comment. The devices without i2c_client structure are the > i2c 'devices' associated with the respective i2c bus. They are visible > in /sys: > > ls -l /sys/bus/i2c/devices/i2c-* > > I wonder if this patch has been ever tested with system suspend/resume, > as those devices are always available in the system... Sorry for the trouble and thanks a lot for the fix. We'll make sure to do more thorough testing, including suspend/resume before relanding this change. Since the patch was reverted, can we squash your fix with the next revision together with your Co-developed-by and Signed-off-by tags? Best regards, Tomasz
Re: [PATCH 01/16] block: add disk/bio-based accounting helpers
On 25/05/2020 14.29, Christoph Hellwig wrote: Add two new helpers to simplify I/O accounting for bio based drivers. Currently these drivers use the generic_start_io_acct and generic_end_io_acct helpers which have very cumbersome calling conventions, don't actually return the time they started accounting, and try to deal with accounting for partitions, which can't happen for bio based drivers. The new helpers will be used to subsequently replace uses of the old helpers. The main function is the bio based wrappes in blkdev.h, but for zram which wants to account rw_page based I/O lower level routines are provided as well. Signed-off-by: Christoph Hellwig --- block/blk-core.c | 34 ++ include/linux/blkdev.h | 26 ++ 2 files changed, 60 insertions(+) diff --git a/block/blk-core.c b/block/blk-core.c index 77e57c2e8d602..8973104f88d90 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1432,6 +1432,40 @@ void blk_account_io_start(struct request *rq, bool new_io) part_stat_unlock(); } +unsigned long disk_start_io_acct(struct gendisk *disk, unsigned int sectors, + unsigned int op) +{ + struct hd_struct *part = >part0; + const int sgrp = op_stat_group(op); + unsigned long now = READ_ONCE(jiffies); + + part_stat_lock(); + update_io_ticks(part, now, false); + part_stat_inc(part, ios[sgrp]); + part_stat_add(part, sectors[sgrp], sectors); + part_stat_local_inc(part, in_flight[op_is_write(op)]); + part_stat_unlock(); + + return now; +} +EXPORT_SYMBOL(disk_start_io_acct); + +void disk_end_io_acct(struct gendisk *disk, unsigned int op, + unsigned long start_time) +{ + struct hd_struct *part = >part0; + const int sgrp = op_stat_group(op); + unsigned long now = READ_ONCE(jiffies); + unsigned long duration = now - start_time; I think it would be better to leave this jiffies legacy nonsense in callers and pass here request duration in nanoseconds. So rewriting them to nanoseconds later wouldn't touch generic code. + + part_stat_lock(); + update_io_ticks(part, now, true); + part_stat_add(part, nsecs[sgrp], jiffies_to_nsecs(duration)); + part_stat_local_dec(part, in_flight[op_is_write(op)]); + part_stat_unlock(); +} +EXPORT_SYMBOL(disk_end_io_acct); + /* * Steal bios from a request and add them to a bio list. * The request must not have been partially completed before. diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 7d10f4e632325..76d01a8a13b80 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -1892,4 +1892,30 @@ static inline void blk_wake_io_task(struct task_struct *waiter) wake_up_process(waiter); } +unsigned long disk_start_io_acct(struct gendisk *disk, unsigned int sectors, + unsigned int op); +void disk_end_io_acct(struct gendisk *disk, unsigned int op, + unsigned long start_time); + +/** + * bio_start_io_acct - start I/O accounting for bio based drivers + * @bio: bio to start account for + * + * Returns the start time that should be passed back to bio_end_io_acct(). + */ +static inline unsigned long bio_start_io_acct(struct bio *bio) +{ + return disk_start_io_acct(bio->bi_disk, bio_sectors(bio), bio_op(bio)); +} + +/** + * bio_end_io_acct - end I/O accounting for bio based drivers + * @bio: bio to end account for + * @start: start time returned by bio_start_io_acct() + */ +static inline void bio_end_io_acct(struct bio *bio, unsigned long start_time) +{ + return disk_end_io_acct(bio->bi_disk, bio_op(bio), start_time); +} + #endif
[PATCH v2] e1000: use generic power management
compile-tested only With legacy PM hooks, it was the responsibility of a driver to manage PCI states and also the device's power state. The generic approach is to let PCI core handle the work. e1000_suspend() calls __e1000_shutdown() to perform intermediate tasks. __e1000_shutdown() modifies the value of "wake" (device should be wakeup enabled or not), responsible for controlling the flow of legacy PM. Since, PCI core has no idea about the value of "wake", new code for generic PM may produce unexpected results. Thus, use "device_set_wakeup_enable()" to wakeup-enable the device accordingly. Signed-off-by: Vaibhav Gupta --- drivers/net/ethernet/intel/e1000/e1000_main.c | 49 +-- 1 file changed, 13 insertions(+), 36 deletions(-) diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c index 0d51cbc88028..011509709b3f 100644 --- a/drivers/net/ethernet/intel/e1000/e1000_main.c +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c @@ -151,10 +151,8 @@ static int e1000_vlan_rx_kill_vid(struct net_device *netdev, __be16 proto, u16 vid); static void e1000_restore_vlan(struct e1000_adapter *adapter); -#ifdef CONFIG_PM -static int e1000_suspend(struct pci_dev *pdev, pm_message_t state); -static int e1000_resume(struct pci_dev *pdev); -#endif +static int __maybe_unused e1000_suspend(struct device *dev); +static int __maybe_unused e1000_resume(struct device *dev); static void e1000_shutdown(struct pci_dev *pdev); #ifdef CONFIG_NET_POLL_CONTROLLER @@ -179,16 +177,16 @@ static const struct pci_error_handlers e1000_err_handler = { .resume = e1000_io_resume, }; +static SIMPLE_DEV_PM_OPS(e1000_pm_ops, e1000_suspend, e1000_resume); + static struct pci_driver e1000_driver = { .name = e1000_driver_name, .id_table = e1000_pci_tbl, .probe= e1000_probe, .remove = e1000_remove, -#ifdef CONFIG_PM - /* Power Management Hooks */ - .suspend = e1000_suspend, - .resume = e1000_resume, -#endif + .driver = { + .pm = _pm_ops, + }, .shutdown = e1000_shutdown, .err_handler = _err_handler }; @@ -5048,9 +5046,6 @@ static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake) struct e1000_hw *hw = >hw; u32 ctrl, ctrl_ext, rctl, status; u32 wufc = adapter->wol; -#ifdef CONFIG_PM - int retval = 0; -#endif netif_device_detach(netdev); @@ -5064,12 +5059,6 @@ static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake) e1000_down(adapter); } -#ifdef CONFIG_PM - retval = pci_save_state(pdev); - if (retval) - return retval; -#endif - status = er32(STATUS); if (status & E1000_STATUS_LU) wufc &= ~E1000_WUFC_LNKC; @@ -5130,37 +5119,26 @@ static int __e1000_shutdown(struct pci_dev *pdev, bool *enable_wake) return 0; } -#ifdef CONFIG_PM -static int e1000_suspend(struct pci_dev *pdev, pm_message_t state) +static int __maybe_unused e1000_suspend(struct device *dev) { int retval; + struct pci_dev *pdev = to_pci_dev(dev); bool wake; retval = __e1000_shutdown(pdev, ); - if (retval) - return retval; - - if (wake) { - pci_prepare_to_sleep(pdev); - } else { - pci_wake_from_d3(pdev, false); - pci_set_power_state(pdev, PCI_D3hot); - } + device_set_wakeup_enable(dev, wake); - return 0; + return retval; } -static int e1000_resume(struct pci_dev *pdev) +static int __maybe_unused e1000_resume(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); struct net_device *netdev = pci_get_drvdata(pdev); struct e1000_adapter *adapter = netdev_priv(netdev); struct e1000_hw *hw = >hw; u32 err; - pci_set_power_state(pdev, PCI_D0); - pci_restore_state(pdev); - pci_save_state(pdev); - if (adapter->need_ioport) err = pci_enable_device(pdev); else @@ -5197,7 +5175,6 @@ static int e1000_resume(struct pci_dev *pdev) return 0; } -#endif static void e1000_shutdown(struct pci_dev *pdev) { -- 2.26.2
Re: [PATCH v3 3/3] vfio-pci: Invalidate mmaps and block MMIO access on disabled memory
On Sat, May 23, 2020 at 07:52:57PM -0400, Peter Xu wrote: > For what I understand now, IMHO we should still need all those handlings of > FAULT_FLAG_RETRY_NOWAIT like in the initial version. E.g., IIUC KVM gup will > try with FOLL_NOWAIT when async is allowed, before the complete slow path. > I'm > not sure what would be the side effect of that if fault() blocked it. E.g., > the caller could be in an atomic context. AFAICT FAULT_FLAG_RETRY_NOWAIT only impacts what happens when VM_FAULT_RETRY is returned, which this doesn't do? It is not a generic 'do not sleep' Do you know different? Jason
Re: [PATCH v5 6/8] drm/panel: Add ilitek ili9341 panel driver
On Mon, May 25, 2020 at 5:46 AM wrote: > From: dillon min > > This driver combine tiny/ili9341.c mipi_dbi_interface driver > with mipi_dpi_interface driver, can support ili9341 with serial > mode or parallel rgb interface mode by register configuration. > > Changes since V3: > > accoding to Linus Walleij's suggestion. > 1 add more comments to driver. > 2 reduce magic number usage in the driver. > 3 move panel configuration from common place to system configuration. > 4 reuse MIPI_DCS_* as more as possible. > > Signed-off-by: dillon min This looks good to me! Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH v2 1/4] gpio: gpiolib: Allow GPIO IRQs to lazy disable
On 25/05/2020 13:55, Linus Walleij wrote: > On Sat, May 23, 2020 at 7:11 PM Maulik Shah wrote: > >> With 'commit 461c1a7d4733 ("gpiolib: override irq_enable/disable")' gpiolib >> overrides irqchip's irq_enable and irq_disable callbacks. If irq_disable >> callback is implemented then genirq takes unlazy path to disable irq. >> >> Underlying irqchip may not want to implement irq_disable callback to lazy >> disable irq when client drivers invokes disable_irq(). By overriding >> irq_disable callback, gpiolib ends up always unlazy disabling IRQ. >> >> Allow gpiolib to lazy disable IRQs by overriding irq_disable callback only >> if irqchip implemented irq_disable. In cases where irq_disable is not >> implemented irq_mask is overridden. Similarly override irq_enable callback >> only if irqchip implemented irq_enable otherwise irq_unmask is overridden. >> >> Fixes: 461c1a7d47 (gpiolib: override irq_enable/disable) >> Signed-off-by: Maulik Shah > > I definitely want Hans Verkuils test and review on this, since it > is a usecase that he is really dependent on. Maulik, since I am no longer subscribed to linux-gpio, can you mail the series to me? I have two use-cases, but I can only test one (I don't have access to the SBC I need to test the other use-case for the next few months). Once I have the whole series I'll try to test the first use-case and at least look into the code if this series could affect the second use-case. Regards, Hans > > Also the irqchip people preferredly. > > But it does seem to mop up my mistakes and fix this up properly! > > So with some testing I'll be happy to merge it, even this one > patch separately if Hans can verify that it works. > > Yours, > Linus Walleij >
Re: [PATCH v1] iommu/tegra-smmu: Add missing locks around mapping operations
On Mon, May 25, 2020 at 01:51:50PM +0300, Dmitry Osipenko wrote: > 25.05.2020 11:35, Thierry Reding пишет: > > On Sun, May 24, 2020 at 09:37:55PM +0300, Dmitry Osipenko wrote: > >> The mapping operations of the Tegra SMMU driver are subjected to a race > >> condition issues because SMMU Address Space isn't allocated and freed > >> atomically, while it should be. This patch makes the mapping operations > >> atomic, it fixes an accidentally released Host1x Address Space problem > >> which happens while running multiple graphics tests in parallel on > >> Tegra30, i.e. by having multiple threads racing with each other in the > >> Host1x's submission and completion code paths, performing IOVA mappings > >> and unmappings in parallel. > >> > >> Cc: > >> Signed-off-by: Dmitry Osipenko > >> --- > >> drivers/iommu/tegra-smmu.c | 43 +- > >> 1 file changed, 38 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c > >> index 7426b7666e2b..4f956a797838 100644 > >> --- a/drivers/iommu/tegra-smmu.c > >> +++ b/drivers/iommu/tegra-smmu.c > >> @@ -12,6 +12,7 @@ > >> #include > >> #include > >> #include > >> +#include > >> #include > >> > >> #include > >> @@ -49,6 +50,7 @@ struct tegra_smmu_as { > >>struct iommu_domain domain; > >>struct tegra_smmu *smmu; > >>unsigned int use_count; > >> + spinlock_t lock; > >>u32 *count; > >>struct page **pts; > >>struct page *pd; > >> @@ -308,6 +310,8 @@ static struct iommu_domain > >> *tegra_smmu_domain_alloc(unsigned type) > >>return NULL; > >>} > >> > >> + spin_lock_init(>lock); > >> + > >>/* setup aperture */ > >>as->domain.geometry.aperture_start = 0; > >>as->domain.geometry.aperture_end = 0x; > >> @@ -578,7 +582,7 @@ static u32 *as_get_pte(struct tegra_smmu_as *as, > >> dma_addr_t iova, > >>struct page *page; > >>dma_addr_t dma; > >> > >> - page = alloc_page(GFP_KERNEL | __GFP_DMA | __GFP_ZERO); > >> + page = alloc_page(GFP_ATOMIC | __GFP_DMA | __GFP_ZERO); > > > > I'm not sure this is a good idea. My recollection is that GFP_ATOMIC > > will allocate from a special reserved region of memory, which may be > > easily exhausted. > > So far I haven't noticed any problems. Will be great if you could > provide more details about the pool size and how this exhaustion problem > could be reproduced in practice. I can't exactly pinpoint where that pool is created nor what its size is, but just from looking at: Documentation/core-api/memory-allocation.rst and searching for GFP_ATOMIC it says: * If you think that accessing memory reserves is justified and the kernel will be stressed unless allocation succeeds, you may use ``GFP_ATOMIC``. That doesn't sound like the case that we're running into here. It sounds to me like GFP_ATOMIC should really only be used in cases where interrupts are being processed or where allocations really need to be successful to ensure the system keeps operational (i.e. during handling of severe errors, ...). Failure to allocate a page directory is hardly very critical. > > Is there any reason why we need the spinlock? Can't we use a mutex > > instead? > > This is what other IOMMU drivers do. I guess mutex might be too > expensive, it may create a noticeable contention which you don't want to > have in a case of a GPU submission code path. I see indeed that many other drivers seem to heavily make use of GFP_ATOMIC. But that doesn't necessarily mean that it's the right thing to do on Tegra (or for the other drivers for that matter). Do we have a good way to find out how bad exactly the contention would be when using a mutex? > I also suspect that drivers of other platforms are using IOMMU API in > interrupt context, although today this is not needed for Tegra. > > >>if (!page) > >>return NULL; > >> > >> @@ -655,8 +659,9 @@ static void tegra_smmu_set_pte(struct tegra_smmu_as > >> *as, unsigned long iova, > >>smmu_flush(smmu); > >> } > >> > >> -static int tegra_smmu_map(struct iommu_domain *domain, unsigned long iova, > >> -phys_addr_t paddr, size_t size, int prot, gfp_t gfp) > >> +static int > >> +tegra_smmu_map_locked(struct iommu_domain *domain, unsigned long iova, > >> +phys_addr_t paddr, size_t size, int prot, gfp_t gfp) > > > > I think it's more typical to use the _unlocked suffix for functions that > > don't take a lock themselves. > > Personally I can't feel the difference. Both variants are good to me. I > can replace the literal postfix with a __tegra_smmu prefix, similarly to > what we have in the GART driver, to avoid bikeshedding. In my opinion a function name is most useful when it describes what the function does. To me, an _unlocked suffix indicates that the function itself doesn't lock, and therefore it needs to be called with some
Re: [PATCH v5 4/8] dt-bindings: display: panel: Add ilitek ili9341 panel bindings
On Mon, May 25, 2020 at 5:46 AM wrote: > From: dillon min > > Add documentation for "ilitek,ili9341" panel. > > Signed-off-by: dillon min This looks good to me! Reviewed-by: Linus Walleij Yours, Linus Walleij
[PATCH v6 3/5] mtd: rawnand: Add write_oob hook in nand_chip_ops
From: Bean Huo Break the function nand_write_oob() into two functions, and one of them is named nand_write_oob_nand(), which will be assigned to new added hook write_oob by default. The hook write_oob will be overwritten in the NAND vendor lower-level driver if needed. Signed-off-by: Miquel Raynal Signed-off-by: Bean Huo --- drivers/mtd/nand/raw/internals.h | 3 ++- drivers/mtd/nand/raw/nand_base.c | 9 + include/linux/mtd/rawnand.h | 3 +++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h index 03866b0aadea..94d300a207ac 100644 --- a/drivers/mtd/nand/raw/internals.h +++ b/drivers/mtd/nand/raw/internals.h @@ -99,7 +99,8 @@ int nand_read_param_page_op(struct nand_chip *chip, u8 page, void *buf, void nand_decode_ext_id(struct nand_chip *chip); void panic_nand_wait(struct nand_chip *chip, unsigned long timeo); void sanitize_string(uint8_t *s, size_t len); - +int nand_write_oob_nand(struct nand_chip *chip, loff_t to, +struct mtd_oob_ops *ops); static inline bool nand_has_exec_op(struct nand_chip *chip) { if (!chip->controller || !chip->controller->ops || diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 3de53c42fb74..ab39bb33e688 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -4318,6 +4318,13 @@ static int nand_write_oob(struct mtd_info *mtd, loff_t to, struct mtd_oob_ops *ops) { struct nand_chip *chip = mtd_to_nand(mtd); + + return chip->ops.write_oob(chip, to, ops); +} + +int nand_write_oob_nand(struct nand_chip *chip, loff_t to, + struct mtd_oob_ops *ops) +{ int ret; ops->retlen = 0; @@ -4624,6 +4631,8 @@ static void nand_set_defaults(struct nand_chip *chip) if (!chip->buf_align) chip->buf_align = 1; + + chip->ops.write_oob = nand_write_oob_nand; } /* Sanitize ONFI strings so we can safely print them */ diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index 59150f729cf0..ae1cc60260a7 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -1037,6 +1037,7 @@ struct nand_legacy { * setting the read-retry mode. Mostly needed for MLC NAND. * @pre_erase: [FLASHSPECIFIC] prepare a physical erase block * @post_erase:[FLASHSPECIFIC] physical block erase post + * @write_oob: [REPLACEABLE] Raw NAND write operation */ struct nand_chip_ops { int (*suspend)(struct nand_chip *chip); @@ -1046,6 +1047,8 @@ struct nand_chip_ops { int (*setup_read_retry)(struct nand_chip *chip, int retry_mode); int (*pre_erase)(struct nand_chip *chip, u32 eraseblock); int (*post_erase)(struct nand_chip *chip, u32 eraseblock); + int (*write_oob)(struct nand_chip *chip, loff_t to, +struct mtd_oob_ops *ops); }; /** -- 2.17.1
[PATCH v6 1/5] mtd: rawnand: group all NAND specific ops into new nand_chip_ops
From: Bean Huo This patch is to create a new structure nand_chip_ops, and take all NAND specific functions out from nand_chip and put them in this new structure. Signed-off-by: Bean Huo --- drivers/mtd/nand/raw/nand_base.c | 20 +- drivers/mtd/nand/raw/nand_hynix.c| 2 +- drivers/mtd/nand/raw/nand_macronix.c | 10 - drivers/mtd/nand/raw/nand_micron.c | 2 +- include/linux/mtd/rawnand.h | 31 +--- 5 files changed, 36 insertions(+), 29 deletions(-) diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 6a6a0a36b3fd..b86f641f6d74 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -3285,10 +3285,10 @@ static int nand_setup_read_retry(struct nand_chip *chip, int retry_mode) if (retry_mode >= chip->read_retries) return -EINVAL; - if (!chip->setup_read_retry) + if (!chip->ops.setup_read_retry) return -EOPNOTSUPP; - return chip->setup_read_retry(chip, retry_mode); + return chip->ops.setup_read_retry(chip, retry_mode); } static void nand_wait_readrdy(struct nand_chip *chip) @@ -4532,8 +4532,8 @@ static int nand_suspend(struct mtd_info *mtd) int ret = 0; mutex_lock(>lock); - if (chip->suspend) - ret = chip->suspend(chip); + if (chip->ops.suspend) + ret = chip->ops.suspend(chip); if (!ret) chip->suspended = 1; mutex_unlock(>lock); @@ -4551,8 +4551,8 @@ static void nand_resume(struct mtd_info *mtd) mutex_lock(>lock); if (chip->suspended) { - if (chip->resume) - chip->resume(chip); + if (chip->ops.resume) + chip->ops.resume(chip); chip->suspended = 0; } else { pr_err("%s called for a chip which is not in suspended state\n", @@ -4581,10 +4581,10 @@ static int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len) { struct nand_chip *chip = mtd_to_nand(mtd); - if (!chip->lock_area) + if (!chip->ops.lock_area) return -ENOTSUPP; - return chip->lock_area(chip, ofs, len); + return chip->ops.lock_area(chip, ofs, len); } /** @@ -4597,10 +4597,10 @@ static int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len) { struct nand_chip *chip = mtd_to_nand(mtd); - if (!chip->unlock_area) + if (!chip->ops.unlock_area) return -ENOTSUPP; - return chip->unlock_area(chip, ofs, len); + return chip->ops.unlock_area(chip, ofs, len); } /* Set default functions */ diff --git a/drivers/mtd/nand/raw/nand_hynix.c b/drivers/mtd/nand/raw/nand_hynix.c index 7caedaa5b9e5..7d1be53f27f3 100644 --- a/drivers/mtd/nand/raw/nand_hynix.c +++ b/drivers/mtd/nand/raw/nand_hynix.c @@ -337,7 +337,7 @@ static int hynix_mlc_1xnm_rr_init(struct nand_chip *chip, rr->nregs = nregs; rr->regs = hynix_1xnm_mlc_read_retry_regs; hynix->read_retry = rr; - chip->setup_read_retry = hynix_nand_setup_read_retry; + chip->ops.setup_read_retry = hynix_nand_setup_read_retry; chip->read_retries = nmodes; out: diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c index 09c254c97b5c..1472f925f386 100644 --- a/drivers/mtd/nand/raw/nand_macronix.c +++ b/drivers/mtd/nand/raw/nand_macronix.c @@ -130,7 +130,7 @@ static void macronix_nand_onfi_init(struct nand_chip *chip) return; chip->read_retries = MACRONIX_NUM_READ_RETRY_MODES; - chip->setup_read_retry = macronix_nand_setup_read_retry; + chip->ops.setup_read_retry = macronix_nand_setup_read_retry; if (p->supports_set_get_features) { bitmap_set(p->set_feature_list, @@ -242,8 +242,8 @@ static void macronix_nand_block_protection_support(struct nand_chip *chip) bitmap_set(chip->parameters.set_feature_list, ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1); - chip->lock_area = mxic_nand_lock; - chip->unlock_area = mxic_nand_unlock; + chip->ops.lock_area = mxic_nand_lock; + chip->ops.unlock_area = mxic_nand_unlock; } static int nand_power_down_op(struct nand_chip *chip) @@ -312,8 +312,8 @@ static void macronix_nand_deep_power_down_support(struct nand_chip *chip) if (i < 0) return; - chip->suspend = mxic_nand_suspend; - chip->resume = mxic_nand_resume; + chip->ops.suspend = mxic_nand_suspend; + chip->ops.resume = mxic_nand_resume; } static int macronix_nand_init(struct nand_chip *chip) diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c index 3589b4fce0d4..4385092a9325 100644 --- a/drivers/mtd/nand/raw/nand_micron.c +++ b/drivers/mtd/nand/raw/nand_micron.c @@ -84,7 +84,7 @@ static int micron_nand_onfi_init(struct
[PATCH v6 0/5] Micron SLC NAND filling block
From: Bean Huo Hi, On planar 2D Micron NAND devices when a block erase command is issued, occasionally even though a block erase operation completes and returns a pass status, the flash block may not be completely erased. Subsequent operations to this block on very rare cases can result in subtle failures or corruption. These extremely rare cases should nevertheless be considered. This patchset is to address this potential issue. After submission of patch V1 [1] and V2 [2], we stopped its update since we get stuck in the solution on how to avoid the power-loss issue in case power-cut hits the block filling. In the v1 and v2, to avoid this issue, we always damaged page0, page1, this's based on the hypothesis that NAND FS is UBIFS. This FS-specifical code is unacceptable in the MTD layer. Also, it cannot cover all NAND based file system. Based on the current discussion, seems that re-write all first 15 page from page0 is a satisfactory solution. Meanwhile, I borrowed one idea from Miquel Raynal patchset [3], in which keeps a recode of programmed pages, base on it, for most of the cases, we don't need to read every page to see if current erasing block is a partially programmed block. Changelog: v5 - v6: 1. Fix a misleading-indentation in patch 5/5 (Reported-by: kbuild test robot ) 2. Rebase patch to git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next v4 - v5: 1. Add Miquel Raynal Authorship and SoB in 4/5 and 5/5 (Miquel Raynal) 2. Change commit message in 5/5. (Steve deRosier) 3. delete unused variable max_bitflips in 4/5 v3 - v4: 1. In the patch 4/5, change to directly use ecc.strength to judge the page is a empty page or not, rather than max_bitflips < mtd->bitflip_threshold 2. In the patch 5/5, for the powerloss case, from the next time boot up, lots of page will be programmed from >page15 address, if still using first_p as GENMASK() bitmask starting position, writtenp will be always 0. fix it by changing its bitmask starting at bit position 0. v2 - v3: 1. Rebase patch to the latest MTD git tree 2. Add a record that keeps tracking the programmed pages in the first 16 pages 3. Change from program odd pages, damage page 0 and page 1, to program all first 15 pages 4. Address issues which exist in the V2. v1 - v2: 1. Rebased V1 to latest Linux kernel. 2. Add erase preparation function pointer in nand_manufacturer_ops. [1] https://www.spinics.net/lists/linux-mtd/msg04112.html [2] https://www.spinics.net/lists/linux-mtd/msg04450.html [3] https://www.spinics.net/lists/linux-mtd/msg13083.html Bean Huo (3): mtd: rawnand: group all NAND specific ops into new nand_chip_ops mtd: rawnand: Add {pre,post}_erase hooks in nand_chip_ops mtd: rawnand: Introduce a new function nand_check_is_erased_page() Miquel Raynal (2): mtd: rawnand: Add write_oob hook in nand_chip_ops mtd: rawnand: micron: Micron SLC NAND filling block drivers/mtd/nand/raw/internals.h | 3 +- drivers/mtd/nand/raw/nand_base.c | 87 ++ drivers/mtd/nand/raw/nand_hynix.c| 2 +- drivers/mtd/nand/raw/nand_macronix.c | 10 +-- drivers/mtd/nand/raw/nand_micron.c | 104 ++- include/linux/mtd/rawnand.h | 40 +++ 6 files changed, 211 insertions(+), 35 deletions(-) -- 2.17.1
[PATCH v6 2/5] mtd: rawnand: Add {pre, post}_erase hooks in nand_chip_ops
From: Bean Huo Add {pre,post}_erase hooks in the structure nand_chip_ops: pre_erase will be called before a block is physically erased. post_erase will be called after a block is erased. Signed-off-by: Bean Huo --- drivers/mtd/nand/raw/nand_base.c | 18 +- include/linux/mtd/rawnand.h | 16 ++-- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index b86f641f6d74..3de53c42fb74 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -4369,7 +4369,7 @@ static int nand_erase(struct mtd_info *mtd, struct erase_info *instr) int nand_erase_nand(struct nand_chip *chip, struct erase_info *instr, int allowbbt) { - int page, pages_per_block, ret, chipnr; + int page, pages_per_block, ret, chipnr, eb; loff_t len; pr_debug("%s: start = 0x%012llx, len = %llu\n", @@ -4423,16 +4423,24 @@ int nand_erase_nand(struct nand_chip *chip, struct erase_info *instr, (page + pages_per_block)) chip->pagecache.page = -1; - ret = nand_erase_op(chip, (page & chip->pagemask) >> - (chip->phys_erase_shift - chip->page_shift)); + eb = (page & chip->pagemask) >> + (chip->phys_erase_shift - chip->page_shift); + + if (chip->ops.pre_erase) + chip->ops.pre_erase(chip, eb); + + ret = nand_erase_op(chip, eb); if (ret) { - pr_debug("%s: failed erase, page 0x%08x\n", - __func__, page); + pr_debug("%s: failed erase block %d, page 0x%08x\n", + __func__, eb, page); instr->fail_addr = ((loff_t)page << chip->page_shift); goto erase_exit; } + if (chip->ops.post_erase) + chip->ops.post_erase(chip, eb); + /* Increment page address and decrement length */ len -= (1ULL << chip->phys_erase_shift); page += pages_per_block; diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index 0c73e9a81e3a..59150f729cf0 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -1029,12 +1029,14 @@ struct nand_legacy { /** * struct nand_chip_ops - NAND Chip specific operations - * @suspend: [REPLACEABLE] specific NAND device suspend operation - * @resume:[REPLACEABLE] specific NAND device resume operation - * @lock_area: [REPLACEABLE] specific NAND chip lock operation - * @unlock_area: [REPLACEABLE] specific NAND chip unlock operation - * @setup_read_retry: [FLASHSPECIFIC] flash (vendor) specific function for - * setting the read-retry mode. Mostly needed for MLC NAND. + * @suspend: [REPLACEABLE] specific NAND device suspend operation + * @resume:[REPLACEABLE] specific NAND device resume operation + * @lock_area: [REPLACEABLE] specific NAND chip lock operation + * @unlock_area: [REPLACEABLE] specific NAND chip unlock operation + * @setup_read_retry: [FLASHSPECIFIC] flash (vendor) specific function for + * setting the read-retry mode. Mostly needed for MLC NAND. + * @pre_erase: [FLASHSPECIFIC] prepare a physical erase block + * @post_erase:[FLASHSPECIFIC] physical block erase post */ struct nand_chip_ops { int (*suspend)(struct nand_chip *chip); @@ -1042,6 +1044,8 @@ struct nand_chip_ops { int (*lock_area)(struct nand_chip *chip, loff_t ofs, u64 len); int (*unlock_area)(struct nand_chip *chip, loff_t ofs, u64 len); int (*setup_read_retry)(struct nand_chip *chip, int retry_mode); + int (*pre_erase)(struct nand_chip *chip, u32 eraseblock); + int (*post_erase)(struct nand_chip *chip, u32 eraseblock); }; /** -- 2.17.1
[PATCH v6 5/5] mtd: rawnand: micron: Micron SLC NAND filling block
From: Bean Huo On planar 2D Micron NAND devices when a block erase command is issued, occasionally even though a block erase operation completes and returns a pass status, the flash block may not be completely erased. Subsequent operations to this block on very rare cases can result in subtle failures or corruption. These extremely rare cases should nevertheless be considered. These rare occurrences have been observed on partially written blocks. To avoid this rare occurrence, we should make sure that at least 15 pages have been programmed to a block before it is erased. In case we find that less than 15 pages have been programmed, we will rewrite first 15 pages of block. Signed-off-by: Miquel Raynal Signed-off-by: Bean Huo --- drivers/mtd/nand/raw/nand_micron.c | 102 + 1 file changed, 102 insertions(+) diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c index 4385092a9325..85aa17ddd3fc 100644 --- a/drivers/mtd/nand/raw/nand_micron.c +++ b/drivers/mtd/nand/raw/nand_micron.c @@ -36,6 +36,9 @@ #define NAND_ECC_STATUS_1_3_CORRECTED BIT(4) #define NAND_ECC_STATUS_7_8_CORRECTED (BIT(4) | BIT(3)) +#define MICRON_SHALLOW_ERASE_MIN_PAGE 15 +#define MICRON_PAGE_MASK_TRIGGER GENMASK(MICRON_SHALLOW_ERASE_MIN_PAGE, 0) + struct nand_onfi_vendor_micron { u8 two_plane_read; u8 read_cache; @@ -64,6 +67,7 @@ struct micron_on_die_ecc { struct micron_nand { struct micron_on_die_ecc ecc; + u16 *writtenp; }; static int micron_nand_setup_read_retry(struct nand_chip *chip, int retry_mode) @@ -472,6 +476,93 @@ static int micron_supports_on_die_ecc(struct nand_chip *chip) return MICRON_ON_DIE_SUPPORTED; } +static int micron_nand_pre_erase(struct nand_chip *chip, u32 eraseblock) +{ + struct micron_nand *micron = nand_get_manufacturer_data(chip); + struct mtd_info *mtd = nand_to_mtd(chip); + u8 last_page = MICRON_SHALLOW_ERASE_MIN_PAGE - 1; + u32 page; + u8 *data_buf; + int ret, i; + + data_buf = nand_get_data_buf(chip); + WARN_ON(!data_buf); + + if (likely(micron->writtenp[eraseblock] & BIT(last_page))) + return 0; + + page = eraseblock << (chip->phys_erase_shift - chip->page_shift); + + if (unlikely(micron->writtenp[eraseblock] == 0)) { + ret = nand_read_page_raw(chip, data_buf, 1, page + last_page); + if (ret) + return ret; /* Read error */ + ret = nand_check_is_erased_page(chip, data_buf, true); + if (!ret) + return 0; + } + + memset(data_buf, 0x00, mtd->writesize); + + for (i = 0; i < MICRON_SHALLOW_ERASE_MIN_PAGE; i++) { + ret = nand_write_page_raw(chip, data_buf, false, page + i); + if (ret) + return ret; + } + + return 0; +} + +static int micron_nand_post_erase(struct nand_chip *chip, u32 eraseblock) +{ + struct micron_nand *micron = nand_get_manufacturer_data(chip); + + if (!micron) + return -EINVAL; + + micron->writtenp[eraseblock] = 0; + + return 0; +} + +static int micron_nand_write_oob(struct nand_chip *chip, loff_t to, +struct mtd_oob_ops *ops) +{ + struct micron_nand *micron = nand_get_manufacturer_data(chip); + u32 eb_sz = nanddev_eraseblock_size(>base); + u32 p_sz = nanddev_page_size(>base); + u32 ppeb = nanddev_pages_per_eraseblock(>base); + u32 nb_p_tot = ops->len / p_sz; + u32 first_eb = DIV_ROUND_DOWN_ULL(to, eb_sz); + u32 first_p = DIV_ROUND_UP_ULL(to - (first_eb * eb_sz), p_sz); + u32 nb_eb = DIV_ROUND_UP_ULL(first_p + nb_p_tot, ppeb); + u32 remaining_p, eb, nb_p; + int ret; + + ret = nand_write_oob_nand(chip, to, ops); + + if (ret || ops->len != ops->retlen) + return ret; + + /* Mark the last pages of the first erase block to write */ + nb_p = min(nb_p_tot, ppeb - first_p); + micron->writtenp[first_eb] |= GENMASK(first_p + nb_p, 0) & + MICRON_PAGE_MASK_TRIGGER; + remaining_p = nb_p_tot - nb_p; + + /* Mark all the pages of all "in-the-middle" erase blocks */ + for (eb = first_eb + 1; eb < first_eb + nb_eb - 1; eb++) { + micron->writtenp[eb] |= MICRON_PAGE_MASK_TRIGGER; + remaining_p -= ppeb; + } + + /* Mark the first pages of the last erase block to write */ + if (remaining_p) + micron->writtenp[eb] |= GENMASK(remaining_p - 1, 0) & + MICRON_PAGE_MASK_TRIGGER; + return 0; +} + static int micron_nand_init(struct nand_chip *chip) { struct mtd_info *mtd = nand_to_mtd(chip); @@ -558,6 +649,17 @@ static int micron_nand_init(struct nand_chip *chip) } } + if
[PATCH v6 4/5] mtd: rawnand: Introduce a new function nand_check_is_erased_page()
From: Bean Huo Add a new function nand_check_is_erased_page() in nand_base.c, which is used to check whether one programmable page is empty or already programmed. Signed-off-by: Bean Huo --- drivers/mtd/nand/raw/nand_base.c | 40 include/linux/mtd/rawnand.h | 2 ++ 2 files changed, 42 insertions(+) diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index ab39bb33e688..05ee32106af9 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -2697,6 +2697,46 @@ int nand_check_erased_ecc_chunk(void *data, int datalen, } EXPORT_SYMBOL(nand_check_erased_ecc_chunk); +/** + * nand_check_is_erased_page - check if this page is a empty page + * @chip: nand chip info structure + * @page_data: data buffer containing the data in the page being checked + * @oob: indicate if chip->oob_poi points to oob date of the page + * + * Returns true if this is an un-programmed page, false otherwise. + */ +int nand_check_is_erased_page(struct nand_chip *chip, u8 *page_data, bool oob) +{ + struct mtd_info *mtd = nand_to_mtd(chip); + int ret, i; + u8 *databuf, *eccbuf = NULL; + struct mtd_oob_region oobregion; + int datasize, eccbytes = 0; + + databuf = page_data; + datasize = chip->ecc.size; + + if (oob) { + mtd_ooblayout_ecc(mtd, 0, ); + eccbuf = chip->oob_poi + oobregion.offset; + eccbytes = chip->ecc.bytes; + } + + for (i = 0; i < chip->ecc.steps; i++) { + ret = nand_check_erased_ecc_chunk(databuf, datasize, + eccbuf, eccbytes, + NULL, 0, chip->ecc.strength); + if (ret < 0) + return false; + + databuf += chip->ecc.size; + eccbuf += chip->ecc.bytes; + } + + return true; +} +EXPORT_SYMBOL(nand_check_is_erased_page); + /** * nand_read_page_raw_notsupp - dummy read raw page function * @chip: nand chip info structure diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index ae1cc60260a7..44c2715691bb 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -1339,6 +1339,8 @@ int nand_check_erased_ecc_chunk(void *data, int datalen, void *extraoob, int extraooblen, int threshold); +int nand_check_is_erased_page(struct nand_chip *chip, u8 *page_data, bool oob); + int nand_ecc_choose_conf(struct nand_chip *chip, const struct nand_ecc_caps *caps, int oobavail); -- 2.17.1
Re: [PATCH] gpiolib: add GPIO_SET_DEBOUNCE_IOCTL
On Mon, May 25, 2020 at 4:22 AM Kent Gibson wrote: > You mention timers for the gpio pins that cannot provide debounce, > so I'm confused. Could you clarify which strategy you have in mind? My idea is that the callback gpiod_set_debounce() for in-kernel consumers should more or less always return success. Either the hardware does the debounce, or gpiolib sets up a timer. > I've also had a quick look at the possibility of providing the software > debounce for in-kernel consumers. That is where I think it should start. > Are you anticipating new API for > that? e.g. allowing consumers to request gpio events? Cos, gpio_keys > grabs the irq itself - and that would bypass the software debouncer, > or even conflict with it. It may be hard or impossible. I suppose gpiolib would have to steal or intercept the interrupt by using e.g. IRQF_SHARED and then just return IRQ_HANDLED on the first IRQ so the underlying irq handler does not get called. After the timer times out it needs to retrigger the IRQ. So the consuming driver would se a "debounced and ready" IRQ so when it gets this IRQ it knows for sure there is no bounciness on it because gpiolib already took care of that. The above is in no way trivial, but it follows the design pattern of "narrow and deep" APIs. Failure is an option! Sorry if I push too complex ideas. Yours, Linus Walleij
linux-next: build failure after merge of the akpm tree
Hi all, After merging the akpm tree, today's linux-next build (x86_64 allmodconfig) failed like this: In file included from include/linux/kernel.h:14, from mm/gup.c:2: mm/gup.c: In function 'internal_get_user_pages_fast': mm/gup.c:2732:33: error: 'struct mm_struct' has no member named 'mmap_sem'; did you mean 'mmap_base'? 2732 | might_lock_read(>mm->mmap_sem); | ^~~~ Caused by commit 64fe66e8a95e ("mmap locking API: rename mmap_sem to mmap_lock") fron the akpm tree interacting with commit b1fc8b5ddb4e ("mm/gup: might_lock_read(mmap_sem) in get_user_pages_fast()") from the akpm-current tree. I added the following patch for today. From: Stephen Rothwell Date: Mon, 25 May 2020 22:11:51 +1000 Subject: [PATCH] mm/gup: update for mmap_sem rename Signed-off-by: Stephen Rothwell --- mm/gup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/gup.c b/mm/gup.c index 8977e5fe9843..f4bca3de0b4b 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -2729,7 +2729,7 @@ static int internal_get_user_pages_fast(unsigned long start, int nr_pages, return -EINVAL; if (!(gup_flags & FOLL_FAST_ONLY)) - might_lock_read(>mm->mmap_sem); + might_lock_read(>mm->mmap_lock); start = untagged_addr(start) & PAGE_MASK; addr = start; -- 2.26.2 -- Cheers, Stephen Rothwell pgp8IDlGwruSN.pgp Description: OpenPGP digital signature
Re: [Tee-dev] [PATCHv3 2/3] optee: use uuid for sysfs driver entry
On 5/25/20 1:52 PM, Maxim Uvarov wrote: > Optee device names for sysfs needed to be unique s/Optee/OP-TEE/ s/needed/need/ > and it's better if they will mean something. UUID for name > looks like good solution: > /sys/bus/tee/devices/optee-clnt- How about mentioning it is the UUID of the Trusted Application on the TEE side? > > Signed-off-by: Maxim Uvarov > --- > drivers/tee/optee/device.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Thanks, -- Jerome
Re: [PATCH] iio: magnetometer: ak8974: Fix runtime PM imbalance on error
On Sun, May 24, 2020 at 4:51 AM Dinghao Liu wrote: > When devm_regmap_init_i2c() returns an error code, a pairing > runtime PM usage counter decrement is needed to keep the > counter balanced. For error paths after ak8974_set_power(), > ak8974_detect() and ak8974_reset(), things are the same. > > However, When iio_triggered_buffer_setup() returns an error > code, we don't need such a decrement because there is already > one before this call. Things are the same for other error paths > after it. > > Signed-off-by: Dinghao Liu > ak8974->map = devm_regmap_init_i2c(i2c, _regmap_config); > if (IS_ERR(ak8974->map)) { > dev_err(>dev, "failed to allocate register map\n"); > + pm_runtime_put_noidle(>dev); > + pm_runtime_disable(>dev); > return PTR_ERR(ak8974->map); This is correct. > ret = ak8974_set_power(ak8974, AK8974_PWR_ON); > if (ret) { > dev_err(>dev, "could not power on\n"); > + pm_runtime_put_noidle(>dev); > + pm_runtime_disable(>dev); > goto power_off; What about just changing this to goto disable_pm; > ret = ak8974_detect(ak8974); > if (ret) { > dev_err(>dev, "neither AK8974 nor AMI30x found\n"); > + pm_runtime_put_noidle(>dev); > + pm_runtime_disable(>dev); > goto power_off; goto disable_pm; > @@ -786,6 +792,8 @@ static int ak8974_probe(struct i2c_client *i2c, > ret = ak8974_reset(ak8974); > if (ret) { > dev_err(>dev, "AK8974 reset failed\n"); > + pm_runtime_put_noidle(>dev); > + pm_runtime_disable(>dev); goto disable_pm; > disable_pm: > - pm_runtime_put_noidle(>dev); > pm_runtime_disable(>dev); > ak8974_set_power(ak8974, AK8974_PWR_OFF); Keep the top pm_runtime_put_noidle(). The ak8974_set_power() call is fine, the power on call does not need to happen in balance. Sure it will attempt to write a register but so will the power on call. Yours, Linus Walleij
Re: [PATCH] arm: dts: am33xx-bone-common: add gpio-line-names
On Mon, May 25, 2020 at 11:23:17AM +0200, Linus Walleij wrote: > On Thu, May 21, 2020 at 12:02 AM Drew Fustini wrote: > > > I've posted a v2 which I hope improves the intent of the line names. [0] > > > > I'm happy to integrate any feedback and create a v3 - especially if it > > is prefered for me to list the specific peripherial signals instead of > > an abstract term like "[ethernet]" or "[emmc]". This is for lines that > > can not be used because they are not routed to the expansion headers. > > > > [0] https://lore.kernel.org/linux-omap/20200520214757.GA362547@x1/T/#u > > This looks good to me. FWIW > Acked-by: Linus Walleij > > Yours, > Linus Walleij Linus - I have posted a newer patch that targets am335x-beagleblack.dts [0] instead of am335x-bone-common.dtsi as Grygorii Strashko pointed out that these line names are not applicable to all BeagleBone models. The gpio line naming scheme is the same, is it ok to add your Ack? thanks, drew [0] https://lore.kernel.org/linux-omap/20200521200926.GC429020@x1/
Re: [RFC 00/14] perf tests: Check on subtest for user specified test
On Mon, 25 May 2020, Jiri Olsa wrote: > hi, > changes for using metric result in another metric seem > to change lot of core metric code, so it's better we > have some more tests before we do that. > > Sending as RFC as it's still alive and you guys might > have some other idea of how to do this. > > Also available in here: > git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git > perf/fixes > > jirka > Hi! Last commit from perf/fixes branch I see there is: commit 0445062df28fef1002302aa419af65fa80513dd4 (HEAD -> perf/fixes, origin/perf/fixes) Author: Jiri Olsa Date: Fri Dec 6 00:10:13 2019 +0100 Different branch? > > --- > Jiri Olsa (14): > perf tests: Check on subtest for user specified test > perf tools: Do not pass avg to generic_metric > perf tools: Add struct parse_events_state pointer to scanner > perf tools: Add fake pmu support > perf tools: Add parse_events_fake interface > perf tests: Add another pmu-events tests > perf tools: Factor out parse_groups function > perf tools: Add metricgroup__parse_groups_test function > perf tools: Add fake_pmu to parse_events function > perf tools: Add map to parse_events function > perf tools: Factor out prepare_metric function > perf tools: Add test_generic_metric function > perf tests: Add parse metric test for ipc metric > perf tests: Add parse metric test for frontend metric > > tools/perf/tests/Build | 1 + > tools/perf/tests/builtin-test.c | 38 ++-- > tools/perf/tests/parse-metric.c | 163 > +++ > tools/perf/tests/pmu-events.c | 120 > +++ > tools/perf/tests/tests.h| 1 + > tools/perf/util/metricgroup.c | 53 ++- > tools/perf/util/metricgroup.h | 9 +++ > tools/perf/util/parse-events.c | 73 > ++--- > tools/perf/util/parse-events.h | 6 - > tools/perf/util/parse-events.l | 16 +++- > tools/perf/util/parse-events.y | 37 +-- > tools/perf/util/stat-shadow.c | 77 > > tools/perf/util/stat.h | 3 +++ > 13 files changed, 521 insertions(+), 76 deletions(-) > create mode 100644 tools/perf/tests/parse-metric.c >
Re: [PATCH] ASoC: img: Complete exception handling in img_i2s_in_probe()
> Function "pm_runtime_get_sync()" is not handled by "pm_runtime_put()" > if "PTR_ERR(rst) == -EPROBE_DEFER". Fix this issue by adding > "pm_runtime_put()" into this error path. * I suggest to improve this change description. * An other subject can be more appropriate for the final commit. Regards, Markus
Re: [PATCH 0/3] STMFX power related fixes
Hi, Gentle reminder regarding this series sent one month ago. Regards, Amelie On 4/22/20 11:08 AM, Amelie Delaunay wrote: With suspend/resume tests on STM32MP157C-EV1 board, on which STMFX is used by several devices, some errors could occurred: -6 when trying to restore STMFX registers, spurious interrupts after disabling supply... This patchset fixes all these issues and cleans IRQ init error path. Amelie Delaunay (3): mfd: stmfx: reset chip on resume as supply was disabled mfd: stmfx: fix stmfx_irq_init error path mfd: stmfx: disable irq in suspend to avoid spurious interrupt drivers/mfd/stmfx.c | 22 -- include/linux/mfd/stmfx.h | 1 + 2 files changed, 21 insertions(+), 2 deletions(-)
Re: [RESEND PATCH v7 4/5] ndctl/papr_scm, uapi: Add support for PAPR nvdimm specific methods
Hi Ira, Mpe and Aneesh, Vaibhav Jain writes: > Michael Ellerman writes: > >> Ira Weiny writes: >>> On Wed, May 20, 2020 at 12:30:57AM +0530, Vaibhav Jain wrote: Introduce support for Papr nvDimm Specific Methods (PDSM) in papr_scm modules and add the command family to the white list of NVDIMM command sets. Also advertise support for ND_CMD_CALL for the dimm command mask and implement necessary scaffolding in the module to handle ND_CMD_CALL ioctl and PDSM requests that we receive. >> ... + * + * Payload Version: + * + * A 'payload_version' field is present in PDSM header that indicates a specific + * version of the structure present in PDSM Payload for a given PDSM command. + * This provides backward compatibility in case the PDSM Payload structure + * evolves and different structures are supported by 'papr_scm' and 'libndctl'. + * + * When sending a PDSM Payload to 'papr_scm', 'libndctl' should send the version + * of the payload struct it supports via 'payload_version' field. The 'papr_scm' + * module when servicing the PDSM envelope checks the 'payload_version' and then + * uses 'payload struct version' == MIN('payload_version field', + * 'max payload-struct-version supported by papr_scm') to service the PDSM. + * After servicing the PDSM, 'papr_scm' put the negotiated version of payload + * struct in returned 'payload_version' field. >>> >>> FWIW many people believe using a size rather than version is more >>> sustainable. >>> It is expected that new payload structures are larger (more features) than >>> the >>> previous payload structure. >>> >>> I can't find references at the moment through. >> >> I think clone_args is a good modern example: >> >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/sched.h#n88 >> >> cheers > > Thank Ira and Mpe for pointing this out. I looked into how clone3 sycall > handles clone_args and few differences came out: > > * Unlike clone_args that are always transferred in one direction from > user-space to kernel, payload contents of pdsms are transferred in both > directions. Having a single version number makes it easier for > user-space and kernel to determine what data will be exchanged. > > * For PDSMs, the version number is negotiated between libndctl and > kernel. For example in case kernel only supports an older version of > a structure, its free to send a lower version number back to > libndctl. Such negotiations doesnt happen with clone3 syscall. If you are ok with the explaination above please let me know. I will quickly spin off a v8 addressing your review comments. Thanks, -- Cheers ~ Vaibhav
Re: [PATCH 5/5] crypto: stm32/crc: protect from concurrent accesses
On Mon, 25 May 2020 at 13:49, Nicolas TOROMANOFF wrote: > > > -Original Message- > > From: Ard Biesheuvel > > Sent: Monday, May 25, 2020 11:07 AM > > To: Nicolas TOROMANOFF ; Eric Biggers > > > > On Mon, 25 May 2020 at 11:01, Nicolas TOROMANOFF > > wrote: > > > > > > > -Original Message- > > > > From: Ard Biesheuvel > > > > Sent: Monday, May 25, 2020 9:46 AM > > > > To: Nicolas TOROMANOFF > > > > Subject: Re: [PATCH 5/5] crypto: stm32/crc: protect from concurrent > > > > accesses > > > > > > > > On Mon, 25 May 2020 at 09:24, Nicolas TOROMANOFF > > > > wrote: > > > > > > > > > > Hello, > > > > > > > > > > > -Original Message- > > > > > > From: Ard Biesheuvel > > > > > > Sent: Friday, May 22, 2020 6:12 PM> On Tue, 12 May 2020 at > > > > > > 16:13, Nicolas Toromanoff wrote: > > > > > > > > > > > > > > Protect STM32 CRC device from concurrent accesses. > > > > > > > > > > > > > > As we create a spinlocked section that increase with buffer > > > > > > > size, we provide a module parameter to release the pressure by > > > > > > > splitting critical section in chunks. > > > > > > > > > > > > > > Size of each chunk is defined in burst_size module parameter. > > > > > > > By default burst_size=0, i.e. don't split incoming buffer. > > > > > > > > > > > > > > Signed-off-by: Nicolas Toromanoff > > > > > > > > > > > > Would you mind explaining the usage model here? It looks like > > > > > > you are sharing a CRC hardware accelerator with a synchronous > > > > > > interface between different users by using spinlocks? You are > > > > > > aware that this will tie up the waiting CPUs completely during > > > > > > this time, right? So it would be much better to use a mutex > > > > > > here. Or perhaps it would make more sense to fall back to a s/w > > > > > > based CRC routine if the h/w is tied up > > > > working for another task? > > > > > > > > > > I know mutex are more acceptable here, but shash _update() and > > > > > _init() may be call from any context, and so I cannot take a mutex. > > > > > And to protect my concurrent HW access I only though about spinlock. > > > > > Due to possible constraint on CPUs, I add a burst_size option to > > > > > force slitting long buffer into smaller one, and so decrease time we > > > > > take > > the lock. > > > > > But I didn't though to fallback to software CRC. > > > > > > > > > > I'll do a patch on top. > > > > > In in the burst_update() function I'll use a > > > > > spin_trylock_irqsave() and use > > > > software CRC32 if HW is already in use. > > > > > > > > > > > > > Right. I didn't even notice that you were keeping interrupts > > > > disabled the whole time when using the h/w block. That means that > > > > any serious use of this h/w block will make IRQ latency go through the > > > > roof. > > > > > > > > I recommend that you go back to the drawing board on this driver, > > > > rather than papering over the issues with a spin_trylock(). Perhaps > > > > it would be better to model it as a ahash (even though the h/w block > > > > itself is synchronous) and use a kthread to feed in the data. > > > > > > I thought when I updated the driver to move to a ahash interface, but > > > the main usage of crc32 is the ext4 fs, that calls the shash API. > > > Commit 877b5691f27a ("crypto: shash - remove shash_desc::flags") > > > removed possibility to sleep in shash callback. (before this commit > > > and with MAY_SLEEP option set, using a mutex may have been fine). > > > > > > > According to that commit's log, sleeping is never fine for shash(), since > > it uses > > kmap_atomic() when iterating over the scatterlist. > > Today, we could avoid using kmap_atomic() in shash_ashash_*() APIs (the > ones that Walks through the scatterlist) by using the > crypto_ahash_walk_first() function to initialize the shash_ahash walker > (note that this function is never call in current kernel source [to remove > ?]). > Then shash_ahash_*() functions will call ahash_*() function that use kmap() > if (walk->flags & CRYPTO_ALG_ASYNC) [flag set by crypto_ahash_walk_first()] > The last kmap_atomic() will be in the shash_ahash_digest() call in the > optimize branch (that should be replaced by the no atomic one) > > I didn't investigate more this way, because I didn't check the drawback of > using kmap() instead of kmap_atomic(), I wanted to avoid modifying behavior > of other drivers and using a function never use elsewhere in kernel scarred > me ;-). > If these updates correct visible bugs in the ahash usage of the stm32-crc32 > code [no more "sleep while atomic" traces even with mutex in tests], > Documentation states that shash API could be called from any context, > I cannot add mutex in them. > > > > By now the solution I see is to use the spin_trylock_irqsave(), > > > fallback to software crc *AND* capping burst_size to ensure the locked > > section stay reasonable. > > > > > > Does this seems acceptable ? > > > > > > > If the reason for disabling interrupts is to
Re: [PATCH 2/6] arm64: dts: qcom: sm8250: add apps_smmu node
On 2020-05-25 17:23, Jonathan Marek wrote: On 5/25/20 7:40 AM, Sai Prakash Ranjan wrote: On 2020-05-25 16:57, Jonathan Marek wrote: On 5/25/20 7:17 AM, Sai Prakash Ranjan wrote: Hi, On 2020-05-25 16:38, Jonathan Marek wrote: On 5/25/20 6:54 AM, Sai Prakash Ranjan wrote: On 2020-05-25 15:39, Jonathan Marek wrote: Hi, On 5/25/20 5:42 AM, Sai Prakash Ranjan wrote: Hi Jonathan, On 2020-05-24 08:08, Jonathan Marek wrote: Add the apps_smmu node for sm8250. Note that adding the iommus field for UFS is required because initializing the iommu removes the bypass mapping that created by the bootloader. This statement doesn't seem right, you can just say since the bypass is disabled by default now, we need to add this property to enable translation and avoid global faults. If I use this patch [1] then the UFS iommu property isn't needed. In downstream, the identity (bypass?) stream mapping is inherited from the bootloader, and UFS is used without any iommu property. Setting ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n doesn't make it work on its own (without the UFS iommu property), so there's more to it than just "bypass is disabled by default now". https://patchwork.kernel.org/patch/11310757/ "iommus" property is not about inheriting stream mapping from bootloader, it is used to enable SMMU address translation for the corresponding master when specified. So when you have disabled bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y or via cmdline "arm-smmu.disable_bypass=1" and iommus property with SID and mask is not specified, then it will result in SMMU global faults. Downstream has bypass enabled(ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n),so you won't see any global faults if you do not have iommus property. Patch in your link is for display because of the usecase for splash screen on android and some other devices where the bootloader will configure SMMU, it has not yet merged and not likely to get merged in the current state. So yes "there is *not* much more to it than bypass is disabled by default now" and you have to specify "iommus" for the master devices or you should enable bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or arm-smmu.disable_bypass=n Try without the patch in the link and without iommus for UFS and ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y and you will see. -Sai I know that the "iommus" property is not about inheriting stream mapping. Probing the iommu removes the stream mapping created by the bootloader, the iommus property is added so that new mappings are created to replace what was removed. You seem to be under the impression that the SM8150/SM8250 bootloader does not configure SMMU. It does, for both UFS and SDHC, just like it does for display/splash screen on some devices. It could be that bootloader does configure SMMU for UFS and SDHC, but the upstream SMMU driver doesnt allow to inherit stream mapping from the bootloader yet, so adding iommus property based on the assumption that it is inherited seems wrong. I never said adding the iommus property is for inheriting stream mapping. I mentioned inheriting to say UFS works without the iommus property on downstream (it inherits a identity/bypass mapping). Your commit description says "adding the iommus field for UFS is required because initializing the iommu removes the bypass mapping that created by the bootloader". So here it would mean like iommus property for UFS is not for enabling address translation by SMMU for UFS but to avoid removing mappings created by the bootloader which is not exactly what iommus property is for. I guess the commit message is ambiguous, that's not what I meant. Is "Now that the kernel initializes the iommu, the bypass mappings set by the bootloader are cleared. Adding the iommus property is required so that new mappings are created for UFS." better? Yes looks good. With either value of ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT, it will not work without the iommus property. I'm pretty sure that if you have ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n and without iommus, it should work. It doesn't work, with either ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y. Ok since you are very sure about this, I will try with your patches on SM8150 MTP tomorrow since I do not have access to one now. Also just to make sure, please remove all the extra SMMU patches you have in your tree which are not yet merged or from downstream kernel. FYI, I have a branch [1] integrating patches for upstream. All the patches up to 34fff8a519cc075933 ("arm64: dts: qcom: add sm8250 GPU nodes") have been submitted (and the patches after that are unnecessary for testing on sm8150). [1] https://github.com/flto/linux/commits/sm8x50-upstream Thanks, I will test this out. -Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH v2 1/4] gpio: gpiolib: Allow GPIO IRQs to lazy disable
On Sat, May 23, 2020 at 7:11 PM Maulik Shah wrote: > With 'commit 461c1a7d4733 ("gpiolib: override irq_enable/disable")' gpiolib > overrides irqchip's irq_enable and irq_disable callbacks. If irq_disable > callback is implemented then genirq takes unlazy path to disable irq. > > Underlying irqchip may not want to implement irq_disable callback to lazy > disable irq when client drivers invokes disable_irq(). By overriding > irq_disable callback, gpiolib ends up always unlazy disabling IRQ. > > Allow gpiolib to lazy disable IRQs by overriding irq_disable callback only > if irqchip implemented irq_disable. In cases where irq_disable is not > implemented irq_mask is overridden. Similarly override irq_enable callback > only if irqchip implemented irq_enable otherwise irq_unmask is overridden. > > Fixes: 461c1a7d47 (gpiolib: override irq_enable/disable) > Signed-off-by: Maulik Shah I definitely want Hans Verkuils test and review on this, since it is a usecase that he is really dependent on. Also the irqchip people preferredly. But it does seem to mop up my mistakes and fix this up properly! So with some testing I'll be happy to merge it, even this one patch separately if Hans can verify that it works. Yours, Linus Walleij
Re: linux-next: Fixes tag needs some work in the iommu tree
On Wed, May 20, 2020 at 04:36:31AM +1000, Stephen Rothwell wrote: > Maybe you meant > > Fixes: b0d1f8741b81 ("iommu/vt-d: Add nested translation helper function") > Fixes: 56722a4398a3 ("iommu/vt-d: Add bind guest PASID support") Updated, thanks.
[PATCH v2 3/3] iio: remove iio_triggered_buffer_postenable()/iio_triggered_buffer_predisable()
From: Lars-Peter Clausen This patch should be squashed into the first one, as the first one is breaking the build (intentionally) to make the IIO core files easier to review. Signed-off-by: Lars-Peter Clausen Signed-off-by: Alexandru Ardelean --- drivers/iio/accel/adxl372.c | 20 +++ drivers/iio/accel/bmc150-accel-core.c | 4 +-- drivers/iio/accel/kxcjk-1013.c | 2 -- drivers/iio/accel/kxsd9.c | 2 -- drivers/iio/accel/st_accel_buffer.c | 22 +++- drivers/iio/accel/stk8312.c | 2 -- drivers/iio/accel/stk8ba50.c| 2 -- drivers/iio/adc/ad7266.c| 2 -- drivers/iio/adc/ad7606.c| 3 +- drivers/iio/adc/ad7766.c| 2 -- drivers/iio/adc/ad7768-1.c | 8 + drivers/iio/adc/ad7887.c| 2 -- drivers/iio/adc/ad_sigma_delta.c| 5 --- drivers/iio/adc/dln2-adc.c | 12 +-- drivers/iio/adc/mxs-lradc-adc.c | 2 -- drivers/iio/adc/stm32-adc.c | 36 +++ drivers/iio/adc/stm32-dfsdm-adc.c | 39 +++-- drivers/iio/adc/ti-adc084s021.c | 2 -- drivers/iio/adc/ti-ads1015.c| 2 -- drivers/iio/adc/vf610_adc.c | 7 +--- drivers/iio/adc/xilinx-xadc-core.c | 2 -- drivers/iio/chemical/atlas-sensor.c | 6 +--- drivers/iio/dummy/iio_simple_dummy_buffer.c | 14 drivers/iio/gyro/bmg160_core.c | 2 -- drivers/iio/gyro/mpu3050-core.c | 2 -- drivers/iio/gyro/st_gyro_buffer.c | 21 +++ drivers/iio/humidity/hdc100x.c | 12 +-- drivers/iio/humidity/hts221_buffer.c| 2 -- drivers/iio/light/gp2ap020a00f.c| 10 -- drivers/iio/light/isl29125.c| 20 ++- drivers/iio/light/rpr0521.c | 2 -- drivers/iio/light/si1145.c | 2 -- drivers/iio/light/st_uvis25_core.c | 2 -- drivers/iio/light/tcs3414.c | 20 ++- drivers/iio/light/vcnl4000.c| 35 -- drivers/iio/magnetometer/bmc150_magn.c | 2 -- drivers/iio/magnetometer/rm3100-core.c | 2 -- drivers/iio/magnetometer/st_magn_buffer.c | 26 ++ drivers/iio/potentiostat/lmp91000.c | 13 ++- drivers/iio/pressure/st_pressure_buffer.c | 26 ++ drivers/iio/pressure/zpa2326.c | 27 ++ drivers/iio/proximity/sx9310.c | 2 -- drivers/iio/proximity/sx9500.c | 9 - 43 files changed, 58 insertions(+), 377 deletions(-) diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c index 60daf04ce188..53697ca09a22 100644 --- a/drivers/iio/accel/adxl372.c +++ b/drivers/iio/accel/adxl372.c @@ -795,13 +795,9 @@ static int adxl372_buffer_postenable(struct iio_dev *indio_dev) unsigned int mask; int i, ret; - ret = iio_triggered_buffer_postenable(indio_dev); - if (ret < 0) - return ret; - ret = adxl372_set_interrupts(st, ADXL372_INT1_MAP_FIFO_FULL_MSK, 0); if (ret < 0) - goto err; + return ret; mask = *indio_dev->active_scan_mask; @@ -810,10 +806,8 @@ static int adxl372_buffer_postenable(struct iio_dev *indio_dev) break; } - if (i == ARRAY_SIZE(adxl372_axis_lookup_table)) { - ret = -EINVAL; - goto err; - } + if (i == ARRAY_SIZE(adxl372_axis_lookup_table)) + return -EINVAL; st->fifo_format = adxl372_axis_lookup_table[i].fifo_format; st->fifo_set_size = bitmap_weight(indio_dev->active_scan_mask, @@ -833,14 +827,10 @@ static int adxl372_buffer_postenable(struct iio_dev *indio_dev) if (ret < 0) { st->fifo_mode = ADXL372_FIFO_BYPASSED; adxl372_set_interrupts(st, 0, 0); - goto err; + return ret; } return 0; - -err: - iio_triggered_buffer_predisable(indio_dev); - return ret; } static int adxl372_buffer_predisable(struct iio_dev *indio_dev) @@ -851,7 +841,7 @@ static int adxl372_buffer_predisable(struct iio_dev *indio_dev) st->fifo_mode = ADXL372_FIFO_BYPASSED; adxl372_configure_fifo(st); - return iio_triggered_buffer_predisable(indio_dev); + return 0; } static const struct iio_buffer_setup_ops adxl372_buffer_ops = { diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c index 121b4e89f038..a2ff7033202d 100644 --- a/drivers/iio/accel/bmc150-accel-core.c +++ b/drivers/iio/accel/bmc150-accel-core.c @@ -1411,7 +1411,7 @@ static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev) int ret = 0; if
Re: linux-next: build failure after merge of the nand tree
Hi Stephen, Stephen Rothwell wrote on Mon, 25 May 2020 20:45:35 +1000: > Hi all, > > After merging the nand tree, today's linux-next build (powerpc > allyesconfig) failed like this: > > drivers/mtd/nand/raw/pasemi_nand.c: In function 'pasemi_nand_probe': > drivers/mtd/nand/raw/pasemi_nand.c:157:1: warning: label 'out_cleanup' > defined but not used [-Wunused-label] > 157 | out_cleanup: > | ^~~ > drivers/mtd/nand/raw/pasemi_nand.c:149:3: error: label 'out_cleanup_nand' > used but not defined > 149 | goto out_cleanup_nand; > | ^~~~ > > Caused by commit > > d6a2207d79e3 ("mtd: rawnand: pasemi: Fix the probe error path") > > I have applied the following patch for today. > > From: Stephen Rothwell > Date: Mon, 25 May 2020 20:41:22 +1000 > Subject: [PATCH] mtd: rawnand: pasemi: fix up label spelling > > Signed-off-by: Stephen Rothwell > --- > drivers/mtd/nand/raw/pasemi_nand.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/raw/pasemi_nand.c > b/drivers/mtd/nand/raw/pasemi_nand.c > index 37570f0c3a36..d8eca8c3fdcd 100644 > --- a/drivers/mtd/nand/raw/pasemi_nand.c > +++ b/drivers/mtd/nand/raw/pasemi_nand.c > @@ -154,7 +154,7 @@ static int pasemi_nand_probe(struct platform_device > *ofdev) > > return 0; > > -out_cleanup: > + out_cleanup_nand: > nand_cleanup(chip); > out_lpc: > release_region(lpcctl, 4); Damn it, I always screw up with !COMPILE_TEST drivers... That's the right fix, I updated nand/next for tomorrow. Thanks a lot, Miquèl
[PATCH v2 1/3] iio: Move attach/detach of the poll func to the core
From: Lars-Peter Clausen All devices using a triggered buffer need to attach and detach the trigger to the device in order to properly work. Instead of doing this in each and every driver by hand move this into the core. At this point in time, all drivers should have been resolved to attach/detach the poll-function in the same order. Signed-off-by: Lars-Peter Clausen Signed-off-by: Alexandru Ardelean --- Changelog v1 -> v2: - drivers/iio/accel/st_accel_buffer.c - remove err2 var and return directly - drivers/iio/gyro/st_gyro_buffer.c - return earlier if st_sensors_set_enable() failed - drivers/iio/light/vcnl4000.c - return directly on error in vcnl4010_buffer_postenable() & vcnl4010_buffer_predisable() - drivers/iio/pressure/zpa2326.c - print error message on error paths only .../buffer/industrialio-triggered-buffer.c| 10 + drivers/iio/iio_core_trigger.h| 17 ++ drivers/iio/industrialio-buffer.c | 13 +++ drivers/iio/industrialio-trigger.c| 22 --- include/linux/iio/trigger_consumer.h | 7 -- 5 files changed, 35 insertions(+), 34 deletions(-) diff --git a/drivers/iio/buffer/industrialio-triggered-buffer.c b/drivers/iio/buffer/industrialio-triggered-buffer.c index e8046c1ecd6b..6c20a83f887e 100644 --- a/drivers/iio/buffer/industrialio-triggered-buffer.c +++ b/drivers/iio/buffer/industrialio-triggered-buffer.c @@ -13,11 +13,6 @@ #include #include -static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = { - .postenable = _triggered_buffer_postenable, - .predisable = _triggered_buffer_predisable, -}; - /** * iio_triggered_buffer_setup() - Setup triggered buffer and pollfunc * @indio_dev: IIO device structure @@ -67,10 +62,7 @@ int iio_triggered_buffer_setup(struct iio_dev *indio_dev, } /* Ring buffer functions - here trigger setup related */ - if (setup_ops) - indio_dev->setup_ops = setup_ops; - else - indio_dev->setup_ops = _triggered_buffer_setup_ops; + indio_dev->setup_ops = setup_ops; /* Flag that polled ring buffering is possible */ indio_dev->modes |= INDIO_BUFFER_TRIGGERED; diff --git a/drivers/iio/iio_core_trigger.h b/drivers/iio/iio_core_trigger.h index e59fe2f36bbb..9d1a92cc6480 100644 --- a/drivers/iio/iio_core_trigger.h +++ b/drivers/iio/iio_core_trigger.h @@ -18,6 +18,12 @@ void iio_device_register_trigger_consumer(struct iio_dev *indio_dev); **/ void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev); + +int iio_trigger_attach_poll_func(struct iio_trigger *trig, +struct iio_poll_func *pf); +int iio_trigger_detach_poll_func(struct iio_trigger *trig, +struct iio_poll_func *pf); + #else /** @@ -37,4 +43,15 @@ static void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev) { } +static inline int iio_trigger_attach_poll_func(struct iio_trigger *trig, + struct iio_poll_func *pf) +{ + return 0; +} +static inline int iio_trigger_detach_poll_func(struct iio_trigger *trig, + struct iio_poll_func *pf) +{ + return 0; +} + #endif /* CONFIG_TRIGGER_CONSUMER */ diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c index 9fa238c0a7d4..329dd4d6757a 100644 --- a/drivers/iio/industrialio-buffer.c +++ b/drivers/iio/industrialio-buffer.c @@ -20,6 +20,7 @@ #include #include "iio_core.h" +#include "iio_core_trigger.h" #include #include #include @@ -972,6 +973,13 @@ static int iio_enable_buffers(struct iio_dev *indio_dev, } } + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) { + ret = iio_trigger_attach_poll_func(indio_dev->trig, + indio_dev->pollfunc); + if (ret) + goto err_disable_buffers; + } + return 0; err_disable_buffers: @@ -998,6 +1006,11 @@ static int iio_disable_buffers(struct iio_dev *indio_dev) if (list_empty(_dev->buffer_list)) return 0; + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) { + iio_trigger_detach_poll_func(indio_dev->trig, +indio_dev->pollfunc); + } + /* * If things go wrong at some step in disable we still need to continue * to perform the other steps, otherwise we leave the device in a diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c index 53d1931f6be8..6f16357fd732 100644 --- a/drivers/iio/industrialio-trigger.c +++ b/drivers/iio/industrialio-trigger.c @@ -239,8 +239,8 @@ static void iio_trigger_put_irq(struct iio_trigger *trig, int irq) * the relevant function is in there may be the
[PATCH v2 2/3] iio: adc: at91-sama5d2_adc: remove predisable/postenable hooks
This should be squashed into the first patch, but it's the more peculiar of the changes. I am not sure whether this is correct. The touchscreen channels shouldn't be enabled by the IIO framework. So, we may need a different way to handle those if needed. Signed-off-by: Alexandru Ardelean --- drivers/iio/adc/at91-sama5d2_adc.c | 18 -- 1 file changed, 18 deletions(-) diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c index 9abbbdcc7420..f71071096392 100644 --- a/drivers/iio/adc/at91-sama5d2_adc.c +++ b/drivers/iio/adc/at91-sama5d2_adc.c @@ -937,14 +937,6 @@ static int at91_adc_buffer_preenable(struct iio_dev *indio_dev) return 0; } -static int at91_adc_buffer_postenable(struct iio_dev *indio_dev) -{ - if (at91_adc_current_chan_is_touch(indio_dev)) - return 0; - - return iio_triggered_buffer_postenable(indio_dev); -} - static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev) { struct at91_adc_state *st = iio_priv(indio_dev); @@ -995,19 +987,9 @@ static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev) return 0; } -static int at91_adc_buffer_predisable(struct iio_dev *indio_dev) -{ - if (at91_adc_current_chan_is_touch(indio_dev)) - return 0; - - return iio_triggered_buffer_predisable(indio_dev); -} - static const struct iio_buffer_setup_ops at91_buffer_setup_ops = { .preenable = _adc_buffer_preenable, .postdisable = _adc_buffer_postdisable, - .postenable = _adc_buffer_postenable, - .predisable = _adc_buffer_predisable, }; static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *indio, -- 2.25.1
[PATCHv3 1/3] optee: do drivers initialization before and after tee-supplicant run
Some drivers (like ftpm) can operate only after tee-supplicant runs becase of tee-supplicant provides things like storage services. This patch splits probe of non tee-supplicant dependable drivers to early stage, and after tee-supplicant run probe other drivers. Signed-off-by: Maxim Uvarov Suggested-by: Sumit Garg Suggested-by: Arnd Bergmann --- drivers/tee/optee/core.c | 28 +--- drivers/tee/optee/device.c| 17 +++-- drivers/tee/optee/optee_private.h | 10 +- 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c index 99698b8a3a74..d059e3ac491c 100644 --- a/drivers/tee/optee/core.c +++ b/drivers/tee/optee/core.c @@ -17,6 +17,7 @@ #include #include #include +#include #include "optee_private.h" #include "optee_smc.h" #include "shm_pool.h" @@ -218,6 +219,15 @@ static void optee_get_version(struct tee_device *teedev, *vers = v; } +static void optee_bus_scan(struct work_struct *work) +{ + int rc; + + rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP); + if (rc) + pr_err("optee_enumerate_devices failed %d\n", rc); +} + static int optee_open(struct tee_context *ctx) { struct optee_context_data *ctxdata; @@ -241,8 +251,18 @@ static int optee_open(struct tee_context *ctx) kfree(ctxdata); return -EBUSY; } - } + if (!optee->scan_bus_done) { + INIT_WORK(>scan_bus_work, optee_bus_scan); + optee->scan_bus_wq = create_workqueue("optee_bus_scan"); + if (!optee->scan_bus_wq) { + pr_err("optee: couldn't create workqueue\n"); + return -ECHILD; + } + queue_work(optee->scan_bus_wq, >scan_bus_work); + optee->scan_bus_done = true; + } + } mutex_init(>mutex); INIT_LIST_HEAD(>sess_list); @@ -296,8 +316,10 @@ static void optee_release(struct tee_context *ctx) ctx->data = NULL; - if (teedev == optee->supp_teedev) + if (teedev == optee->supp_teedev) { + destroy_workqueue(optee->scan_bus_wq); optee_supp_release(>supp); + } } static const struct tee_driver_ops optee_ops = { @@ -675,7 +697,7 @@ static int optee_probe(struct platform_device *pdev) platform_set_drvdata(pdev, optee); - rc = optee_enumerate_devices(); + rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES); if (rc) { optee_remove(pdev); return rc; diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index e3a148521ec1..d4931dad07aa 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -21,7 +21,6 @@ * TEE_ERROR_BAD_PARAMETERS - Incorrect input param * TEE_ERROR_SHORT_BUFFER - Output buffer size less than required */ -#define PTA_CMD_GET_DEVICES0x0 static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) { @@ -32,7 +31,8 @@ static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) } static int get_devices(struct tee_context *ctx, u32 session, - struct tee_shm *device_shm, u32 *shm_size) + struct tee_shm *device_shm, u32 *shm_size, + u32 func) { int ret = 0; struct tee_ioctl_invoke_arg inv_arg; @@ -42,7 +42,7 @@ static int get_devices(struct tee_context *ctx, u32 session, memset(, 0, sizeof(param)); /* Invoke PTA_CMD_GET_DEVICES function */ - inv_arg.func = PTA_CMD_GET_DEVICES; + inv_arg.func = func; inv_arg.session = session; inv_arg.num_params = 4; @@ -87,7 +87,7 @@ static int optee_register_device(const uuid_t *device_uuid, u32 device_id) return rc; } -int optee_enumerate_devices(void) +static int __optee_enumerate_devices(u32 func) { const uuid_t pta_uuid = UUID_INIT(0x7011a688, 0xddde, 0x4053, @@ -118,7 +118,7 @@ int optee_enumerate_devices(void) goto out_ctx; } - rc = get_devices(ctx, sess_arg.session, NULL, _size); + rc = get_devices(ctx, sess_arg.session, NULL, _size, func); if (rc < 0 || !shm_size) goto out_sess; @@ -130,7 +130,7 @@ int optee_enumerate_devices(void) goto out_sess; } - rc = get_devices(ctx, sess_arg.session, device_shm, _size); + rc = get_devices(ctx, sess_arg.session, device_shm, _size, func); if (rc < 0) goto out_shm; @@ -158,3 +158,8 @@ int optee_enumerate_devices(void) return rc; } + +int optee_enumerate_devices(u32 func) +{ + return __optee_enumerate_devices(func); +} diff --git
[PATCHv3 2/3] optee: use uuid for sysfs driver entry
Optee device names for sysfs needed to be unique and it's better if they will mean something. UUID for name looks like good solution: /sys/bus/tee/devices/optee-clnt- Signed-off-by: Maxim Uvarov --- drivers/tee/optee/device.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/tee/optee/device.c b/drivers/tee/optee/device.c index d4931dad07aa..aab917605e74 100644 --- a/drivers/tee/optee/device.c +++ b/drivers/tee/optee/device.c @@ -65,7 +65,7 @@ static int get_devices(struct tee_context *ctx, u32 session, return 0; } -static int optee_register_device(const uuid_t *device_uuid, u32 device_id) +static int optee_register_device(const uuid_t *device_uuid) { struct tee_client_device *optee_device = NULL; int rc; @@ -75,7 +75,7 @@ static int optee_register_device(const uuid_t *device_uuid, u32 device_id) return -ENOMEM; optee_device->dev.bus = _bus_type; - dev_set_name(_device->dev, "optee-clnt%u", device_id); + dev_set_name(_device->dev, "optee-clnt-%pUl", device_uuid); uuid_copy(_device->id.uuid, device_uuid); rc = device_register(_device->dev); @@ -144,7 +144,7 @@ static int __optee_enumerate_devices(u32 func) num_devices = shm_size / sizeof(uuid_t); for (idx = 0; idx < num_devices; idx++) { - rc = optee_register_device(_uuid[idx], idx); + rc = optee_register_device(_uuid[idx]); if (rc) goto out_shm; } -- 2.17.1
Re: [PATCH -next] iommu/vt-d: fix a GCC warning
On Thu, May 21, 2020 at 05:50:30PM -0400, Qian Cai wrote: > The commit 6ee1b77ba3ac ("iommu/vt-d: Add svm/sva invalidate function") > introduced a GCC warning, > > drivers/iommu/intel-iommu.c:5330:1: warning: 'static' is not at beginning of > declaration [-Wold-style-declaration] > const static int > ^ > > Signed-off-by: Qian Cai > --- > drivers/iommu/intel-iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied, thanks.
[PATCHv3 3/3] tpm_ftpm_tee: register driver on TEE bus
Register driver on the TEE bus. The :module tee registers bus, and module optee calls optee_enumerate_devices() to scan all devices on the bus. Trusted Application for this driver can be Early TA's (can be compiled into optee-os). In that case it will be on OPTEE bus before linux booting. Also optee-suplicant application is needed to be loaded between OPTEE module and ftpm module to maintain functionality for fTPM driver. Signed-off-by: Maxim Uvarov Suggested-by: Sumit Garg Suggested-by: Arnd Bergmann --- drivers/char/tpm/tpm_ftpm_tee.c | 70 - 1 file changed, 60 insertions(+), 10 deletions(-) diff --git a/drivers/char/tpm/tpm_ftpm_tee.c b/drivers/char/tpm/tpm_ftpm_tee.c index 22bf553ccf9d..28da638360d8 100644 --- a/drivers/char/tpm/tpm_ftpm_tee.c +++ b/drivers/char/tpm/tpm_ftpm_tee.c @@ -214,11 +214,10 @@ static int ftpm_tee_match(struct tee_ioctl_version_data *ver, const void *data) * Return: * On success, 0. On failure, -errno. */ -static int ftpm_tee_probe(struct platform_device *pdev) +static int ftpm_tee_probe(struct device *dev) { int rc; struct tpm_chip *chip; - struct device *dev = >dev; struct ftpm_tee_private *pvt_data = NULL; struct tee_ioctl_open_session_arg sess_arg; @@ -297,6 +296,13 @@ static int ftpm_tee_probe(struct platform_device *pdev) return rc; } +static int ftpm_plat_tee_probe(struct platform_device *pdev) +{ + struct device *dev = >dev; + + return ftpm_tee_probe(dev); +} + /** * ftpm_tee_remove() - remove the TPM device * @pdev: the platform_device description. @@ -304,9 +310,9 @@ static int ftpm_tee_probe(struct platform_device *pdev) * Return: * 0 always. */ -static int ftpm_tee_remove(struct platform_device *pdev) +static int ftpm_tee_remove(struct device *dev) { - struct ftpm_tee_private *pvt_data = dev_get_drvdata(>dev); + struct ftpm_tee_private *pvt_data = dev_get_drvdata(dev); /* Release the chip */ tpm_chip_unregister(pvt_data->chip); @@ -328,11 +334,18 @@ static int ftpm_tee_remove(struct platform_device *pdev) return 0; } +static int ftpm_plat_tee_remove(struct platform_device *pdev) +{ + struct device *dev = >dev; + + return ftpm_tee_remove(dev); +} + /** * ftpm_tee_shutdown() - shutdown the TPM device * @pdev: the platform_device description. */ -static void ftpm_tee_shutdown(struct platform_device *pdev) +static void ftpm_plat_tee_shutdown(struct platform_device *pdev) { struct ftpm_tee_private *pvt_data = dev_get_drvdata(>dev); @@ -347,17 +360,54 @@ static const struct of_device_id of_ftpm_tee_ids[] = { }; MODULE_DEVICE_TABLE(of, of_ftpm_tee_ids); -static struct platform_driver ftpm_tee_driver = { +static struct platform_driver ftpm_tee_plat_driver = { .driver = { .name = "ftpm-tee", .of_match_table = of_match_ptr(of_ftpm_tee_ids), }, - .probe = ftpm_tee_probe, - .remove = ftpm_tee_remove, - .shutdown = ftpm_tee_shutdown, + .shutdown = ftpm_plat_tee_shutdown, + .probe = ftpm_plat_tee_probe, + .remove = ftpm_plat_tee_remove, +}; + +/* UUID of the fTPM TA */ +static const struct tee_client_device_id optee_ftpm_id_table[] = { + {UUID_INIT(0xbc50d971, 0xd4c9, 0x42c4, + 0x82, 0xcb, 0x34, 0x3f, 0xb7, 0xf3, 0x78, 0x96)}, + {} }; -module_platform_driver(ftpm_tee_driver); +MODULE_DEVICE_TABLE(tee, optee_ftpm_id_table); + +static struct tee_client_driver ftpm_tee_driver = { + .id_table = optee_ftpm_id_table, + .driver = { + .name = "optee-ftpm", + .bus= _bus_type, + .probe = ftpm_tee_probe, + .remove = ftpm_tee_remove, + }, +}; + +static int __init ftpm_mod_init(void) +{ + int rc; + + rc = platform_driver_register(_tee_plat_driver); + if (rc) + return rc; + + return driver_register(_tee_driver.driver); +} + +static void __exit ftpm_mod_exit(void) +{ + platform_driver_unregister(_tee_plat_driver); + driver_unregister(_tee_driver.driver); +} + +module_init(ftpm_mod_init); +module_exit(ftpm_mod_exit); MODULE_AUTHOR("Thirupathaiah Annapureddy "); MODULE_DESCRIPTION("TPM Driver for fTPM TA in TEE"); -- 2.17.1
Re: [PATCH 2/6] arm64: dts: qcom: sm8250: add apps_smmu node
On 5/25/20 7:40 AM, Sai Prakash Ranjan wrote: On 2020-05-25 16:57, Jonathan Marek wrote: On 5/25/20 7:17 AM, Sai Prakash Ranjan wrote: Hi, On 2020-05-25 16:38, Jonathan Marek wrote: On 5/25/20 6:54 AM, Sai Prakash Ranjan wrote: On 2020-05-25 15:39, Jonathan Marek wrote: Hi, On 5/25/20 5:42 AM, Sai Prakash Ranjan wrote: Hi Jonathan, On 2020-05-24 08:08, Jonathan Marek wrote: Add the apps_smmu node for sm8250. Note that adding the iommus field for UFS is required because initializing the iommu removes the bypass mapping that created by the bootloader. This statement doesn't seem right, you can just say since the bypass is disabled by default now, we need to add this property to enable translation and avoid global faults. If I use this patch [1] then the UFS iommu property isn't needed. In downstream, the identity (bypass?) stream mapping is inherited from the bootloader, and UFS is used without any iommu property. Setting ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n doesn't make it work on its own (without the UFS iommu property), so there's more to it than just "bypass is disabled by default now". https://patchwork.kernel.org/patch/11310757/ "iommus" property is not about inheriting stream mapping from bootloader, it is used to enable SMMU address translation for the corresponding master when specified. So when you have disabled bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y or via cmdline "arm-smmu.disable_bypass=1" and iommus property with SID and mask is not specified, then it will result in SMMU global faults. Downstream has bypass enabled(ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n),so you won't see any global faults if you do not have iommus property. Patch in your link is for display because of the usecase for splash screen on android and some other devices where the bootloader will configure SMMU, it has not yet merged and not likely to get merged in the current state. So yes "there is *not* much more to it than bypass is disabled by default now" and you have to specify "iommus" for the master devices or you should enable bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or arm-smmu.disable_bypass=n Try without the patch in the link and without iommus for UFS and ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y and you will see. -Sai I know that the "iommus" property is not about inheriting stream mapping. Probing the iommu removes the stream mapping created by the bootloader, the iommus property is added so that new mappings are created to replace what was removed. You seem to be under the impression that the SM8150/SM8250 bootloader does not configure SMMU. It does, for both UFS and SDHC, just like it does for display/splash screen on some devices. It could be that bootloader does configure SMMU for UFS and SDHC, but the upstream SMMU driver doesnt allow to inherit stream mapping from the bootloader yet, so adding iommus property based on the assumption that it is inherited seems wrong. I never said adding the iommus property is for inheriting stream mapping. I mentioned inheriting to say UFS works without the iommus property on downstream (it inherits a identity/bypass mapping). Your commit description says "adding the iommus field for UFS is required because initializing the iommu removes the bypass mapping that created by the bootloader". So here it would mean like iommus property for UFS is not for enabling address translation by SMMU for UFS but to avoid removing mappings created by the bootloader which is not exactly what iommus property is for. I guess the commit message is ambiguous, that's not what I meant. Is "Now that the kernel initializes the iommu, the bypass mappings set by the bootloader are cleared. Adding the iommus property is required so that new mappings are created for UFS." better? With either value of ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT, it will not work without the iommus property. I'm pretty sure that if you have ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n and without iommus, it should work. It doesn't work, with either ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y. Ok since you are very sure about this, I will try with your patches on SM8150 MTP tomorrow since I do not have access to one now. Also just to make sure, please remove all the extra SMMU patches you have in your tree which are not yet merged or from downstream kernel. FYI, I have a branch [1] integrating patches for upstream. All the patches up to 34fff8a519cc075933 ("arm64: dts: qcom: add sm8250 GPU nodes") have been submitted (and the patches after that are unnecessary for testing on sm8150). [1] https://github.com/flto/linux/commits/sm8x50-upstream -Sai
[PATCHv3 0/3] optee: register drivers on optee bus
v3: - support tee-suppicant restart (Jens Wiklander) - description and comments ( Jarkko Sakkinen) - do not name optee drivers by index in sysfs (Sumit Garg) v2: - write TEE with capital letters. - declare __optee_enumerate_device() as static. Hello, This patchset fixes issues with probing() tee, optee and optee driver if they were compiled into kernel, built as modules or any mixed combination. These changes require optee-os changes which already were merged. Main corresponding commits are: https://github.com/OP-TEE/optee_os/commit/9389d8030ef198c9d7b8ab7ea8e877e0ace3369d https://github.com/OP-TEE/optee_os/commit/bc5921cdab538c8ae48422f5ffd600f1cbdd95b2 optee_enumerate_devices() which discovers Trusted Applications on tee bus is split up on 2 changes. Do probe of drivers which do not require userspace support of tee-supplicant and stage two to run drivers with support of tee-supplicant only after tee supplicant run. Best regards, Maxim. Maxim Uvarov (3): optee: do drivers initialization before and after tee-supplicant run optee: use uuid for sysfs driver entry tpm_ftpm_tee: register driver on TEE bus drivers/char/tpm/tpm_ftpm_tee.c | 70 ++- drivers/tee/optee/core.c | 28 +++-- drivers/tee/optee/device.c| 23 ++ drivers/tee/optee/optee_private.h | 10 - 4 files changed, 108 insertions(+), 23 deletions(-) -- 2.17.1
Re: next/master bisection: baseline.login on panda
Hi Guillaume, On Wed, May 20, 2020 at 10:54:44AM +0100, Guillaume Tucker wrote: > Please see the bisection report below about a boot failure. > > Reports aren't automatically sent to the public while we're > trialing new bisection features on kernelci.org but this one > looks valid. > > Unfortunately there isn't anything in the kernel log, it's > probably crashing very early on. The bisection was run on > omap4-panda, and there seems to be the same issue on > omap3-beagle-xm as it's also failing to boot. The issue is likely a deadlock fixed by: https://lore.kernel.org/r/20200519132824.15163-1-j...@8bytes.org The patch is already in the iommu-tree and should show up in linux-next soon. Regards, Joerg
[PATCH 3/4] exfat: add boot region verification
Add Boot-Regions verification specified in exFAT specification. Note that the checksum type is strongly related to the raw structure, so the'u32 'type is used to clarify the number of bits. Signed-off-by: Tetsuhiro Kohada --- fs/exfat/exfat_fs.h | 1 + fs/exfat/misc.c | 14 + fs/exfat/super.c| 50 + 3 files changed, 65 insertions(+) diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h index b0e5b9afc56c..15817281b3c8 100644 --- a/fs/exfat/exfat_fs.h +++ b/fs/exfat/exfat_fs.h @@ -517,6 +517,7 @@ void exfat_set_entry_time(struct exfat_sb_info *sbi, struct timespec64 *ts, u8 *tz, __le16 *time, __le16 *date, u8 *time_cs); unsigned short exfat_calc_chksum_2byte(void *data, int len, unsigned short chksum, int type); +u32 exfat_calc_chksum32(void *data, int len, u32 chksum, int type); void exfat_update_bh(struct super_block *sb, struct buffer_head *bh, int sync); void exfat_chain_set(struct exfat_chain *ec, unsigned int dir, unsigned int size, unsigned char flags); diff --git a/fs/exfat/misc.c b/fs/exfat/misc.c index ab7f88b1f6d3..b82d2dd5bd7c 100644 --- a/fs/exfat/misc.c +++ b/fs/exfat/misc.c @@ -151,6 +151,20 @@ unsigned short exfat_calc_chksum_2byte(void *data, int len, return chksum; } +u32 exfat_calc_chksum32(void *data, int len, u32 chksum, int type) +{ + int i; + u8 *c = (u8 *)data; + + for (i = 0; i < len; i++, c++) { + if (unlikely(type == CS_BOOT_SECTOR && +(i == 106 || i == 107 || i == 112))) + continue; + chksum = ((chksum << 31) | (chksum >> 1)) + *c; + } + return chksum; +} + void exfat_update_bh(struct super_block *sb, struct buffer_head *bh, int sync) { set_bit(EXFAT_SB_DIRTY, _SB(sb)->s_state); diff --git a/fs/exfat/super.c b/fs/exfat/super.c index 95909b4d5e75..42b3bd3df020 100644 --- a/fs/exfat/super.c +++ b/fs/exfat/super.c @@ -486,6 +486,50 @@ static int exfat_read_boot_sector(struct super_block *sb) return 0; } +static int exfat_verify_boot_region(struct super_block *sb) +{ + struct buffer_head *bh = NULL; + u32 chksum = 0, *p_sig, *p_chksum; + int sn, i; + + /* read boot sector sub-regions */ + for (sn = 0; sn < 11; sn++) { + bh = sb_bread(sb, sn); + if (!bh) + return -EIO; + + if (sn != 0 && sn <= 8) { + /* extended boot sector sub-regions */ + p_sig = (u32 *)>b_data[sb->s_blocksize - 4]; + if (le32_to_cpu(*p_sig) != EXBOOT_SIGNATURE) { + exfat_err(sb, "no exboot-signature"); + brelse(bh); + return -EINVAL; + } + } + + chksum = exfat_calc_chksum32(bh->b_data, sb->s_blocksize, + chksum, sn ? CS_DEFAULT : CS_BOOT_SECTOR); + brelse(bh); + } + + /* boot checksum sub-regions */ + bh = sb_bread(sb, sn); + if (!bh) + return -EIO; + + for (i = 0; i < sb->s_blocksize; i += sizeof(u32)) { + p_chksum = (u32 *)>b_data[i]; + if (le32_to_cpu(*p_chksum) != chksum) { + exfat_err(sb, "mismatch checksum"); + brelse(bh); + return -EINVAL; + } + } + brelse(bh); + return 0; +} + /* mount the file system volume */ static int __exfat_fill_super(struct super_block *sb) { @@ -498,6 +542,12 @@ static int __exfat_fill_super(struct super_block *sb) goto free_bh; } + ret = exfat_verify_boot_region(sb); + if (ret) { + exfat_err(sb, "invalid boot region"); + goto free_bh; + } + ret = exfat_create_upcase_table(sb); if (ret) { exfat_err(sb, "failed to load upcase table"); -- 2.25.1