Re: [PATCH 3/3] mm, oom: introduce memory.oom.group
On Tue 31-07-18 18:14:48, Roman Gushchin wrote:
> On Tue, Jul 31, 2018 at 11:07:00AM +0200, Michal Hocko wrote:
> > On Mon 30-07-18 11:01:00, Roman Gushchin wrote:
> > > For some workloads an intervention from the OOM killer
> > > can be painful. Killing a random task can bring
> > > the workload into an inconsistent state.
> > >
> > > Historically, there are two common solutions for this
> > > problem:
> > > 1) enabling panic_on_oom,
> > > 2) using a userspace daemon to monitor OOMs and kill
> > >all outstanding processes.
> > >
> > > Both approaches have their downsides:
> > > rebooting on each OOM is an obvious waste of capacity,
> > > and handling all in userspace is tricky and requires
> > > a userspace agent, which will monitor all cgroups
> > > for OOMs.
> > >
> > > In most cases an in-kernel after-OOM cleaning-up
> > > mechanism can eliminate the necessity of enabling
> > > panic_on_oom. Also, it can simplify the cgroup
> > > management for userspace applications.
> > >
> > > This commit introduces a new knob for cgroup v2 memory
> > > controller: memory.oom.group. The knob determines
> > > whether the cgroup should be treated as a single
> > > unit by the OOM killer. If set, the cgroup and its
> > > descendants are killed together or not at all.
> >
> > I do not want to nit pick on wording but unit is not really a good
> > description. I would expect that to mean that the oom killer will
> > consider the unit also when selecting the task and that is not the case.
> > I would be more explicit about this being a single killable entity
> > because it forms an indivisible workload.
> >
> > You can reuse http://lkml.kernel.org/r/20180730080357.ga24...@dhcp22.suse.cz
> > if you want.
>
> Ok, I'll do my best to make it clearer.
>
> >
> > [...]
> > > +/**
> > > + * mem_cgroup_get_oom_group - get a memory cgroup to clean up after OOM
> > > + * @victim: task to be killed by the OOM killer
> > > + * @oom_domain: memcg in case of memcg OOM, NULL in case of system-wide
> > > OOM
> > > + *
> > > + * Returns a pointer to a memory cgroup, which has to be cleaned up
> > > + * by killing all belonging OOM-killable tasks.
> >
> > Caller has to call mem_cgroup_put on the returned non-null memcg.
>
> Added.
>
> >
> > > + */
> > > +struct mem_cgroup *mem_cgroup_get_oom_group(struct task_struct *victim,
> > > + struct mem_cgroup *oom_domain)
> > > +{
> > > + struct mem_cgroup *oom_group = NULL;
> > > + struct mem_cgroup *memcg;
> > > +
> > > + if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
> > > + return NULL;
> > > +
> > > + if (!oom_domain)
> > > + oom_domain = root_mem_cgroup;
> > > +
> > > + rcu_read_lock();
> > > +
> > > + memcg = mem_cgroup_from_task(victim);
> > > + if (!memcg || memcg == root_mem_cgroup)
> > > + goto out;
> >
> > When can we have memcg == NULL? victim should be always non-NULL.
> > Also why do you need to special case the root_mem_cgroup here. The loop
> > below should handle that just fine no?
>
> Idk, I prefer to keep an explicit root_mem_cgroup check,
> rather than traversing the tree and relying on an inability
> to set oom_group on the root.
I will not insist but this just makes the code harder to read.
[...]
> > > + if (oom_group) {
> >
> > we want a printk explaining that we are going to tear down the whole
> > oom_group here.
>
> Does this looks good?
> Or it's better to remove "memory." prefix?
>
> [ 52.835327] Out of memory: Kill process 1221 (allocate) score 241 or
> sacrifice child
> [ 52.836625] Killed process 1221 (allocate) total-vm:2257144kB,
> anon-rss:2009128kB, file-rss:4kB, shmem-rss:0kB
> [ 52.841431] Tasks in /A1 are going to be killed due to memory.oom.group set
Yes, looks good to me.
> [ 52.869439] Killed process 1217 (allocate) total-vm:2052344kB,
> anon-rss:1704036kB, file-rss:0kB, shmem-rss:0kB
> [ 52.875601] Killed process 1218 (allocate) total-vm:106668kB,
> anon-rss:24668kB, file-rss:0kB, shmem-rss:0kB
> [ 52.882914] Killed process 1219 (allocate) total-vm:106668kB,
> anon-rss:21528kB, file-rss:0kB, shmem-rss:0kB
> [ 52.891806] Killed process 1220 (allocate) total-vm:2257144kB,
> anon-rss:1984120kB, file-rss:4kB, shmem-rss:0kB
> [ 52.903770] Killed process 1221 (allocate) total-vm:2257144kB,
> anon-rss:2009128kB, file-rss:4kB, shmem-rss:0kB
> [ 52.905574] Killed process 1222 (allocate) total-vm:2257144kB,
> anon-rss:2063640kB, file-rss:0kB, shmem-rss:0kB
> [ 53.202153] oom_reaper: reaped process 1222 (allocate), now anon-rss:0kB,
> file-rss:0kB, shmem-rss:0kB
>
> >
> > > + mem_cgroup_scan_tasks(oom_group, oom_kill_memcg_member, NULL);
> > > + mem_cgroup_put(oom_group);
> > > + }
> > > }
> >
> > Other than that looks good to me. My concern that the previous
> > implementation was more consistent because we were comparing memcgs
> > still holds but if there is no way forward that direction this should be
> > acceptable as well.
> >
Re: [PATCH 3/3] mm, oom: introduce memory.oom.group
On Tue 31-07-18 18:14:48, Roman Gushchin wrote:
> On Tue, Jul 31, 2018 at 11:07:00AM +0200, Michal Hocko wrote:
> > On Mon 30-07-18 11:01:00, Roman Gushchin wrote:
> > > For some workloads an intervention from the OOM killer
> > > can be painful. Killing a random task can bring
> > > the workload into an inconsistent state.
> > >
> > > Historically, there are two common solutions for this
> > > problem:
> > > 1) enabling panic_on_oom,
> > > 2) using a userspace daemon to monitor OOMs and kill
> > >all outstanding processes.
> > >
> > > Both approaches have their downsides:
> > > rebooting on each OOM is an obvious waste of capacity,
> > > and handling all in userspace is tricky and requires
> > > a userspace agent, which will monitor all cgroups
> > > for OOMs.
> > >
> > > In most cases an in-kernel after-OOM cleaning-up
> > > mechanism can eliminate the necessity of enabling
> > > panic_on_oom. Also, it can simplify the cgroup
> > > management for userspace applications.
> > >
> > > This commit introduces a new knob for cgroup v2 memory
> > > controller: memory.oom.group. The knob determines
> > > whether the cgroup should be treated as a single
> > > unit by the OOM killer. If set, the cgroup and its
> > > descendants are killed together or not at all.
> >
> > I do not want to nit pick on wording but unit is not really a good
> > description. I would expect that to mean that the oom killer will
> > consider the unit also when selecting the task and that is not the case.
> > I would be more explicit about this being a single killable entity
> > because it forms an indivisible workload.
> >
> > You can reuse http://lkml.kernel.org/r/20180730080357.ga24...@dhcp22.suse.cz
> > if you want.
>
> Ok, I'll do my best to make it clearer.
>
> >
> > [...]
> > > +/**
> > > + * mem_cgroup_get_oom_group - get a memory cgroup to clean up after OOM
> > > + * @victim: task to be killed by the OOM killer
> > > + * @oom_domain: memcg in case of memcg OOM, NULL in case of system-wide
> > > OOM
> > > + *
> > > + * Returns a pointer to a memory cgroup, which has to be cleaned up
> > > + * by killing all belonging OOM-killable tasks.
> >
> > Caller has to call mem_cgroup_put on the returned non-null memcg.
>
> Added.
>
> >
> > > + */
> > > +struct mem_cgroup *mem_cgroup_get_oom_group(struct task_struct *victim,
> > > + struct mem_cgroup *oom_domain)
> > > +{
> > > + struct mem_cgroup *oom_group = NULL;
> > > + struct mem_cgroup *memcg;
> > > +
> > > + if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
> > > + return NULL;
> > > +
> > > + if (!oom_domain)
> > > + oom_domain = root_mem_cgroup;
> > > +
> > > + rcu_read_lock();
> > > +
> > > + memcg = mem_cgroup_from_task(victim);
> > > + if (!memcg || memcg == root_mem_cgroup)
> > > + goto out;
> >
> > When can we have memcg == NULL? victim should be always non-NULL.
> > Also why do you need to special case the root_mem_cgroup here. The loop
> > below should handle that just fine no?
>
> Idk, I prefer to keep an explicit root_mem_cgroup check,
> rather than traversing the tree and relying on an inability
> to set oom_group on the root.
I will not insist but this just makes the code harder to read.
[...]
> > > + if (oom_group) {
> >
> > we want a printk explaining that we are going to tear down the whole
> > oom_group here.
>
> Does this looks good?
> Or it's better to remove "memory." prefix?
>
> [ 52.835327] Out of memory: Kill process 1221 (allocate) score 241 or
> sacrifice child
> [ 52.836625] Killed process 1221 (allocate) total-vm:2257144kB,
> anon-rss:2009128kB, file-rss:4kB, shmem-rss:0kB
> [ 52.841431] Tasks in /A1 are going to be killed due to memory.oom.group set
Yes, looks good to me.
> [ 52.869439] Killed process 1217 (allocate) total-vm:2052344kB,
> anon-rss:1704036kB, file-rss:0kB, shmem-rss:0kB
> [ 52.875601] Killed process 1218 (allocate) total-vm:106668kB,
> anon-rss:24668kB, file-rss:0kB, shmem-rss:0kB
> [ 52.882914] Killed process 1219 (allocate) total-vm:106668kB,
> anon-rss:21528kB, file-rss:0kB, shmem-rss:0kB
> [ 52.891806] Killed process 1220 (allocate) total-vm:2257144kB,
> anon-rss:1984120kB, file-rss:4kB, shmem-rss:0kB
> [ 52.903770] Killed process 1221 (allocate) total-vm:2257144kB,
> anon-rss:2009128kB, file-rss:4kB, shmem-rss:0kB
> [ 52.905574] Killed process 1222 (allocate) total-vm:2257144kB,
> anon-rss:2063640kB, file-rss:0kB, shmem-rss:0kB
> [ 53.202153] oom_reaper: reaped process 1222 (allocate), now anon-rss:0kB,
> file-rss:0kB, shmem-rss:0kB
>
> >
> > > + mem_cgroup_scan_tasks(oom_group, oom_kill_memcg_member, NULL);
> > > + mem_cgroup_put(oom_group);
> > > + }
> > > }
> >
> > Other than that looks good to me. My concern that the previous
> > implementation was more consistent because we were comparing memcgs
> > still holds but if there is no way forward that direction this should be
> > acceptable as well.
> >
Re: [PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
Hi YueHaibing,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pinctrl/devel]
[also build test ERROR on v4.18-rc7 next-20180731]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/YueHaibing/pinctrl-berlin-fix-pctrl-functions-allocation-in-berlin_pinctrl_build_state/20180801-131813
base:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
drivers/pinctrl/berlin/berlin.c: In function 'berlin_pinctrl_build_state':
>> drivers/pinctrl/berlin/berlin.c:259:5: error: too few arguments to function
>> 'kfree'
kfree( );
^
In file included from drivers/pinctrl/berlin/berlin.c:20:
include/linux/slab.h:183:6: note: declared here
void kfree(const void *);
^
vim +/kfree +259 drivers/pinctrl/berlin/berlin.c
202
203 static int berlin_pinctrl_build_state(struct platform_device *pdev)
204 {
205 struct berlin_pinctrl *pctrl = platform_get_drvdata(pdev);
206 const struct berlin_desc_group *desc_group;
207 const struct berlin_desc_function *desc_function;
208 int i, max_functions = 0;
209
210 pctrl->nfunctions = 0;
211
212 for (i = 0; i < pctrl->desc->ngroups; i++) {
213 desc_group = pctrl->desc->groups + i;
214 /* compute the maxiumum number of functions a group can
have */
215 max_functions += 1 << (desc_group->bit_width + 1);
216 }
217
218 /* we will reallocate later */
219 pctrl->functions = kcalloc(max_functions,
220 sizeof(*pctrl->functions),
GFP_KERNEL);
221 if (!pctrl->functions)
222 return -ENOMEM;
223
224 /* register all functions */
225 for (i = 0; i < pctrl->desc->ngroups; i++) {
226 desc_group = pctrl->desc->groups + i;
227 desc_function = desc_group->functions;
228
229 while (desc_function->name) {
230 berlin_pinctrl_add_function(pctrl,
desc_function->name);
231 desc_function++;
232 }
233 }
234
235 pctrl->functions = krealloc(pctrl->functions,
236 pctrl->nfunctions *
sizeof(*pctrl->functions),
237 GFP_KERNEL);
238
239 /* map functions to theirs groups */
240 for (i = 0; i < pctrl->desc->ngroups; i++) {
241 desc_group = pctrl->desc->groups + i;
242 desc_function = desc_group->functions;
243
244 while (desc_function->name) {
245 struct berlin_pinctrl_function
246 *function = pctrl->functions;
247 const char **groups;
248 bool found = false;
249
250 while (function->name) {
251 if (!strcmp(desc_function->name,
function->name)) {
252 found = true;
253 break;
254 }
255 function++;
256 }
257
258 if (!found) {
> 259 kfree( );
260 return -EINVAL;
261 }
262
263 if (!function->groups) {
264 function->groups =
265 devm_kcalloc(>dev,
266 function->ngroups,
267 sizeof(char *),
268 GFP_KERNEL);
269
270 if (!function->groups) {
271 kfree(pctrl->functions);
272 return -ENOMEM;
273 }
274
Re: [PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
Hi YueHaibing,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pinctrl/devel]
[also build test ERROR on v4.18-rc7 next-20180731]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/YueHaibing/pinctrl-berlin-fix-pctrl-functions-allocation-in-berlin_pinctrl_build_state/20180801-131813
base:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
drivers/pinctrl/berlin/berlin.c: In function 'berlin_pinctrl_build_state':
>> drivers/pinctrl/berlin/berlin.c:259:5: error: too few arguments to function
>> 'kfree'
kfree( );
^
In file included from drivers/pinctrl/berlin/berlin.c:20:
include/linux/slab.h:183:6: note: declared here
void kfree(const void *);
^
vim +/kfree +259 drivers/pinctrl/berlin/berlin.c
202
203 static int berlin_pinctrl_build_state(struct platform_device *pdev)
204 {
205 struct berlin_pinctrl *pctrl = platform_get_drvdata(pdev);
206 const struct berlin_desc_group *desc_group;
207 const struct berlin_desc_function *desc_function;
208 int i, max_functions = 0;
209
210 pctrl->nfunctions = 0;
211
212 for (i = 0; i < pctrl->desc->ngroups; i++) {
213 desc_group = pctrl->desc->groups + i;
214 /* compute the maxiumum number of functions a group can
have */
215 max_functions += 1 << (desc_group->bit_width + 1);
216 }
217
218 /* we will reallocate later */
219 pctrl->functions = kcalloc(max_functions,
220 sizeof(*pctrl->functions),
GFP_KERNEL);
221 if (!pctrl->functions)
222 return -ENOMEM;
223
224 /* register all functions */
225 for (i = 0; i < pctrl->desc->ngroups; i++) {
226 desc_group = pctrl->desc->groups + i;
227 desc_function = desc_group->functions;
228
229 while (desc_function->name) {
230 berlin_pinctrl_add_function(pctrl,
desc_function->name);
231 desc_function++;
232 }
233 }
234
235 pctrl->functions = krealloc(pctrl->functions,
236 pctrl->nfunctions *
sizeof(*pctrl->functions),
237 GFP_KERNEL);
238
239 /* map functions to theirs groups */
240 for (i = 0; i < pctrl->desc->ngroups; i++) {
241 desc_group = pctrl->desc->groups + i;
242 desc_function = desc_group->functions;
243
244 while (desc_function->name) {
245 struct berlin_pinctrl_function
246 *function = pctrl->functions;
247 const char **groups;
248 bool found = false;
249
250 while (function->name) {
251 if (!strcmp(desc_function->name,
function->name)) {
252 found = true;
253 break;
254 }
255 function++;
256 }
257
258 if (!found) {
> 259 kfree( );
260 return -EINVAL;
261 }
262
263 if (!function->groups) {
264 function->groups =
265 devm_kcalloc(>dev,
266 function->ngroups,
267 sizeof(char *),
268 GFP_KERNEL);
269
270 if (!function->groups) {
271 kfree(pctrl->functions);
272 return -ENOMEM;
273 }
274
Re: [PATCH 1/3] mm: introduce mem_cgroup_put() helper
On Tue 31-07-18 07:58:00, Shakeel Butt wrote:
> On Tue, Jul 31, 2018 at 1:45 AM Michal Hocko wrote:
> >
> > On Mon 30-07-18 11:00:58, Roman Gushchin wrote:
> > > Introduce the mem_cgroup_put() helper, which helps to eliminate guarding
> > > memcg css release with "#ifdef CONFIG_MEMCG" in multiple places.
> >
> > Is there any reason for this to be a separate patch? I usually do not
> > like to add helpers without their users because this makes review
> > harder. This one is quite trivial to fit into Patch3 easilly.
> >
>
> The helper function introduced in this change is also used in the
> remote charging patches, so, I asked Roman to separate this change out
> and thus can be merged independently.
Ok, that was not clear from the description. Then this is ok
Acked-by: Michal Hocko
> Shakeel
>
> > > Link: http://lkml.kernel.org/r/20180623000600.5818-2-g...@fb.com
> > > Signed-off-by: Roman Gushchin
> > > Reviewed-by: Shakeel Butt
> > > Reviewed-by: Andrew Morton
> > > Cc: Shakeel Butt
> > > Cc: Johannes Weiner
> > > Cc: Michal Hocko
> > > Signed-off-by: Andrew Morton
> > > Signed-off-by: Stephen Rothwell
> > > ---
> > > include/linux/memcontrol.h | 9 +
> > > 1 file changed, 9 insertions(+)
> > >
> > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > > index 6c6fb116e925..e53e00cdbe3f 100644
> > > --- a/include/linux/memcontrol.h
> > > +++ b/include/linux/memcontrol.h
> > > @@ -375,6 +375,11 @@ struct mem_cgroup *mem_cgroup_from_css(struct
> > > cgroup_subsys_state *css){
> > > return css ? container_of(css, struct mem_cgroup, css) : NULL;
> > > }
> > >
> > > +static inline void mem_cgroup_put(struct mem_cgroup *memcg)
> > > +{
> > > + css_put(>css);
> > > +}
> > > +
> > > #define mem_cgroup_from_counter(counter, member) \
> > > container_of(counter, struct mem_cgroup, member)
> > >
> > > @@ -837,6 +842,10 @@ static inline bool task_in_mem_cgroup(struct
> > > task_struct *task,
> > > return true;
> > > }
> > >
> > > +static inline void mem_cgroup_put(struct mem_cgroup *memcg)
> > > +{
> > > +}
> > > +
> > > static inline struct mem_cgroup *
> > > mem_cgroup_iter(struct mem_cgroup *root,
> > > struct mem_cgroup *prev,
> > > --
> > > 2.14.4
> > >
> >
> > --
> > Michal Hocko
> > SUSE Labs
--
Michal Hocko
SUSE Labs
Re: [PATCH 1/3] mm: introduce mem_cgroup_put() helper
On Tue 31-07-18 07:58:00, Shakeel Butt wrote:
> On Tue, Jul 31, 2018 at 1:45 AM Michal Hocko wrote:
> >
> > On Mon 30-07-18 11:00:58, Roman Gushchin wrote:
> > > Introduce the mem_cgroup_put() helper, which helps to eliminate guarding
> > > memcg css release with "#ifdef CONFIG_MEMCG" in multiple places.
> >
> > Is there any reason for this to be a separate patch? I usually do not
> > like to add helpers without their users because this makes review
> > harder. This one is quite trivial to fit into Patch3 easilly.
> >
>
> The helper function introduced in this change is also used in the
> remote charging patches, so, I asked Roman to separate this change out
> and thus can be merged independently.
Ok, that was not clear from the description. Then this is ok
Acked-by: Michal Hocko
> Shakeel
>
> > > Link: http://lkml.kernel.org/r/20180623000600.5818-2-g...@fb.com
> > > Signed-off-by: Roman Gushchin
> > > Reviewed-by: Shakeel Butt
> > > Reviewed-by: Andrew Morton
> > > Cc: Shakeel Butt
> > > Cc: Johannes Weiner
> > > Cc: Michal Hocko
> > > Signed-off-by: Andrew Morton
> > > Signed-off-by: Stephen Rothwell
> > > ---
> > > include/linux/memcontrol.h | 9 +
> > > 1 file changed, 9 insertions(+)
> > >
> > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > > index 6c6fb116e925..e53e00cdbe3f 100644
> > > --- a/include/linux/memcontrol.h
> > > +++ b/include/linux/memcontrol.h
> > > @@ -375,6 +375,11 @@ struct mem_cgroup *mem_cgroup_from_css(struct
> > > cgroup_subsys_state *css){
> > > return css ? container_of(css, struct mem_cgroup, css) : NULL;
> > > }
> > >
> > > +static inline void mem_cgroup_put(struct mem_cgroup *memcg)
> > > +{
> > > + css_put(>css);
> > > +}
> > > +
> > > #define mem_cgroup_from_counter(counter, member) \
> > > container_of(counter, struct mem_cgroup, member)
> > >
> > > @@ -837,6 +842,10 @@ static inline bool task_in_mem_cgroup(struct
> > > task_struct *task,
> > > return true;
> > > }
> > >
> > > +static inline void mem_cgroup_put(struct mem_cgroup *memcg)
> > > +{
> > > +}
> > > +
> > > static inline struct mem_cgroup *
> > > mem_cgroup_iter(struct mem_cgroup *root,
> > > struct mem_cgroup *prev,
> > > --
> > > 2.14.4
> > >
> >
> > --
> > Michal Hocko
> > SUSE Labs
--
Michal Hocko
SUSE Labs
Re: [PATCH v2 2/3] test_overflow: Add shift overflow tests
On Tue, Jul 31, 2018 at 7:13 PM, Jason Gunthorpe wrote: > On Tue, Jul 31, 2018 at 05:00:38PM -0700, Kees Cook wrote: >> + /* Overflow: high bit falls off. */ >> + /* 10010110 */ >> + err |= TEST_ONE_SHIFT(150, 1, u8, 0, true); >> + /* 1000100010010110 */ >> + err |= TEST_ONE_SHIFT(34966, 1, u16, 0, true); >> + /* 110010001000100010010110 */ >> + err |= TEST_ONE_SHIFT(2215151766U, 1, u32, 0, true); >> + err |= TEST_ONE_SHIFT(2215151766U, 1, unsigned int, 0, true); >> + /* 101101000100110010001000100010010110 */ >> + err |= TEST_ONE_SHIFT(9372061470395238550ULL, 1, u64, 0, true); > > This same idea should be repeated with signed outputs and check both > overflow past the end (<<2) and overflow into the signed bit (<<1) > > /* Overflow, high bit falls into the sign bit or off the end */ > /* 01001011 */ > err |= TEST_ONE_SHIFT(75, 1, s8, 0, true); > err |= TEST_ONE_SHIFT(75, 2, s8, 0, true); > > And also general type mismatch overflow: > > err |= TEST_ONE_SHIFT(0x100, 0, u8, 0, true); > err |= TEST_ONE_SHIFT(0xFF, 0, s8, 0, true); Ah yes, thanks. I've added these for the next version now. -Kees -- Kees Cook Pixel Security
Re: [PATCH v2 2/3] test_overflow: Add shift overflow tests
On Tue, Jul 31, 2018 at 7:13 PM, Jason Gunthorpe wrote: > On Tue, Jul 31, 2018 at 05:00:38PM -0700, Kees Cook wrote: >> + /* Overflow: high bit falls off. */ >> + /* 10010110 */ >> + err |= TEST_ONE_SHIFT(150, 1, u8, 0, true); >> + /* 1000100010010110 */ >> + err |= TEST_ONE_SHIFT(34966, 1, u16, 0, true); >> + /* 110010001000100010010110 */ >> + err |= TEST_ONE_SHIFT(2215151766U, 1, u32, 0, true); >> + err |= TEST_ONE_SHIFT(2215151766U, 1, unsigned int, 0, true); >> + /* 101101000100110010001000100010010110 */ >> + err |= TEST_ONE_SHIFT(9372061470395238550ULL, 1, u64, 0, true); > > This same idea should be repeated with signed outputs and check both > overflow past the end (<<2) and overflow into the signed bit (<<1) > > /* Overflow, high bit falls into the sign bit or off the end */ > /* 01001011 */ > err |= TEST_ONE_SHIFT(75, 1, s8, 0, true); > err |= TEST_ONE_SHIFT(75, 2, s8, 0, true); > > And also general type mismatch overflow: > > err |= TEST_ONE_SHIFT(0x100, 0, u8, 0, true); > err |= TEST_ONE_SHIFT(0xFF, 0, s8, 0, true); Ah yes, thanks. I've added these for the next version now. -Kees -- Kees Cook Pixel Security
RE: [RFC PATCH 2/2] fpga manager: Adding FPGA Manager support for Xilinx zynqmp
Hi Alan Tull,
Thanks for the quick response.
Please find my Comments inline...
> -Original Message-
> From: Alan Tull [mailto:at...@kernel.org]
> Sent: Tuesday, July 31, 2018 8:52 PM
> To: Nava kishore Manne
> Cc: robh...@kernel.org; mark.rutl...@arm.com; Michal Simek
> ; Soren Brinkmann ;
> at...@opensource.altera.com; moritz.fisc...@ettus.com;
> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; Appana Durga Kedareswara Rao
> ; chinnikishore...@gmail.com; linux-
> f...@vger.kernel.org
> Subject: Re: [RFC PATCH 2/2] fpga manager: Adding FPGA Manager support for
> Xilinx zynqmp
>
> On Tue, Jul 31, 2018 at 8:08 AM, Nava kishore Manne
> wrote:
> >
> > +Alan Tull,
>
> + linux-fpga mailing list
>
> Hi Nava,
>
> Thanks for submitting.
>
> This should be on the linux-fpga mailing list. The
> linux/scripts/get_maintainer.pl script would tell you that.
>
I have Created this Patch On Master Branch there get_maintainer.pl Doesn't have
linux-fpga mailing list.
https://git.kernel.org/pub/scm/linux/kernel/git/atull/linux-fpga.git/
Please suggest a branch So will create a patch on top of it.
> Also, did you run checkpatch.pl on these? :) I encourage using the --strict
> parameter.
>
Will Fix in the next version.
> >
> >> -Original Message-
> >> From: Nava kishore Manne [mailto:nava.ma...@xilinx.com]
> >> Sent: Wednesday, August 1, 2018 3:35 PM
> >> To: robh...@kernel.org; mark.rutl...@arm.com; Michal Simek
> >> ; Soren Brinkmann ;
> >> at...@opensource.altera.com; moritz.fisc...@ettus.com; Nava kishore
> >> Manne ; devicet...@vger.kernel.org; linux-arm-
> >> ker...@lists.infradead.org; linux-kernel@vger.kernel.org; Appana
> >> Durga Kedareswara Rao ;
> >> chinnikishore...@gmail.com
> >> Subject: [RFC PATCH 2/2] fpga manager: Adding FPGA Manager support
> >> for Xilinx zynqmp
> >>
> >> This patch adds FPGA Manager support for the Xilinx ZynqMp chip.
> >>
> >> Signed-off-by: Nava kishore Manne
> >> ---
> >> drivers/fpga/Kconfig | 6 ++
> >> drivers/fpga/Makefile | 1 +
> >> drivers/fpga/zynqmp-fpga.c | 164
> >> +
> >> 3 files changed, 171 insertions(+)
> >> create mode 100644 drivers/fpga/zynqmp-fpga.c
> >>
> >> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index
> >> cd84934774cc..b84e3555b3e3 100644
> >> --- a/drivers/fpga/Kconfig
> >> +++ b/drivers/fpga/Kconfig
> >> @@ -26,6 +26,12 @@ config FPGA_MGR_ZYNQ_FPGA
> >> help
> >> FPGA manager driver support for Xilinx Zynq FPGAs.
> >>
> >> +config FPGA_MGR_ZYNQMP_FPGA
> >> + tristate "Xilinx Zynqmp FPGA"
> >> + depends on ARCH_ZYNQMP || COMPILE_TEST
> >> + help
> >> + FPGA manager driver support for Xilinx ZynqMp FPGAs.
> >> +
> >> endif # FPGA
> >>
> >> endmenu
> >> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile index
> >> 8d83fc6b1613..ef444512cb01 100644
> >> --- a/drivers/fpga/Makefile
> >> +++ b/drivers/fpga/Makefile
> >> @@ -8,3 +8,4 @@ obj-$(CONFIG_FPGA)+= fpga-mgr.o
> >> # FPGA Manager Drivers
> >> obj-$(CONFIG_FPGA_MGR_SOCFPGA) += socfpga.o
> >> obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA) += zynq-fpga.o
> >> +obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA) += zynqmp-fpga.o
> >> diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
> >> new file mode 100644 index ..e4172c3a6868
> >> --- /dev/null
> >> +++ b/drivers/fpga/zynqmp-fpga.c
> >> @@ -0,0 +1,164 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * Copyright (C) 2018 Xilinx, Inc.
> >> + *
>
> Please delete the unnecessary blank '*' line here.
>
Will Fix in the next version.
> >> + */
> >> +
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
>
> I don't see this firmware/xilinx folder. Is this dependent on other commits
> that
> were submitted? If so, please note that in the header.
>
This Driver have a dependency On firmware.h currently Jolly Shah is working On
it.
Once its got accepted will add the Required API's in the firmware.h
https://lkml.org/lkml/2018/7/17/1038
will fix this in the next version.
> >> +
> >> +/* Constant Definitions */
> >> +#define IXR_FPGA_DONE_MASK 0X0008U
> >> +#define IXR_FPGA_ENCRYPTION_EN 0x0008U
> >> +
> >> +/**
> >> + * struct zynqmp_fpga_priv - Private data structure
> >> + * @dev: Device data structure
> >> + * @flags: flags which is used to identify the bitfile type
> >> + */
> >> +struct zynqmp_fpga_priv {
> >> + struct device *dev;
> >> + u32 flags;
> >> +};
> >> +
> >> +static int zynqmp_fpga_ops_write_init(struct fpga_manager *mgr,
> >> + struct fpga_image_info *info,
> >> + const char *buf, size_t size) {
>
> checkpatch.pl would have complained about this:
> ERROR: open brace '{' following function definitions go on the next line
>
Will
RE: [RFC PATCH 2/2] fpga manager: Adding FPGA Manager support for Xilinx zynqmp
Hi Alan Tull,
Thanks for the quick response.
Please find my Comments inline...
> -Original Message-
> From: Alan Tull [mailto:at...@kernel.org]
> Sent: Tuesday, July 31, 2018 8:52 PM
> To: Nava kishore Manne
> Cc: robh...@kernel.org; mark.rutl...@arm.com; Michal Simek
> ; Soren Brinkmann ;
> at...@opensource.altera.com; moritz.fisc...@ettus.com;
> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; Appana Durga Kedareswara Rao
> ; chinnikishore...@gmail.com; linux-
> f...@vger.kernel.org
> Subject: Re: [RFC PATCH 2/2] fpga manager: Adding FPGA Manager support for
> Xilinx zynqmp
>
> On Tue, Jul 31, 2018 at 8:08 AM, Nava kishore Manne
> wrote:
> >
> > +Alan Tull,
>
> + linux-fpga mailing list
>
> Hi Nava,
>
> Thanks for submitting.
>
> This should be on the linux-fpga mailing list. The
> linux/scripts/get_maintainer.pl script would tell you that.
>
I have Created this Patch On Master Branch there get_maintainer.pl Doesn't have
linux-fpga mailing list.
https://git.kernel.org/pub/scm/linux/kernel/git/atull/linux-fpga.git/
Please suggest a branch So will create a patch on top of it.
> Also, did you run checkpatch.pl on these? :) I encourage using the --strict
> parameter.
>
Will Fix in the next version.
> >
> >> -Original Message-
> >> From: Nava kishore Manne [mailto:nava.ma...@xilinx.com]
> >> Sent: Wednesday, August 1, 2018 3:35 PM
> >> To: robh...@kernel.org; mark.rutl...@arm.com; Michal Simek
> >> ; Soren Brinkmann ;
> >> at...@opensource.altera.com; moritz.fisc...@ettus.com; Nava kishore
> >> Manne ; devicet...@vger.kernel.org; linux-arm-
> >> ker...@lists.infradead.org; linux-kernel@vger.kernel.org; Appana
> >> Durga Kedareswara Rao ;
> >> chinnikishore...@gmail.com
> >> Subject: [RFC PATCH 2/2] fpga manager: Adding FPGA Manager support
> >> for Xilinx zynqmp
> >>
> >> This patch adds FPGA Manager support for the Xilinx ZynqMp chip.
> >>
> >> Signed-off-by: Nava kishore Manne
> >> ---
> >> drivers/fpga/Kconfig | 6 ++
> >> drivers/fpga/Makefile | 1 +
> >> drivers/fpga/zynqmp-fpga.c | 164
> >> +
> >> 3 files changed, 171 insertions(+)
> >> create mode 100644 drivers/fpga/zynqmp-fpga.c
> >>
> >> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index
> >> cd84934774cc..b84e3555b3e3 100644
> >> --- a/drivers/fpga/Kconfig
> >> +++ b/drivers/fpga/Kconfig
> >> @@ -26,6 +26,12 @@ config FPGA_MGR_ZYNQ_FPGA
> >> help
> >> FPGA manager driver support for Xilinx Zynq FPGAs.
> >>
> >> +config FPGA_MGR_ZYNQMP_FPGA
> >> + tristate "Xilinx Zynqmp FPGA"
> >> + depends on ARCH_ZYNQMP || COMPILE_TEST
> >> + help
> >> + FPGA manager driver support for Xilinx ZynqMp FPGAs.
> >> +
> >> endif # FPGA
> >>
> >> endmenu
> >> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile index
> >> 8d83fc6b1613..ef444512cb01 100644
> >> --- a/drivers/fpga/Makefile
> >> +++ b/drivers/fpga/Makefile
> >> @@ -8,3 +8,4 @@ obj-$(CONFIG_FPGA)+= fpga-mgr.o
> >> # FPGA Manager Drivers
> >> obj-$(CONFIG_FPGA_MGR_SOCFPGA) += socfpga.o
> >> obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA) += zynq-fpga.o
> >> +obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA) += zynqmp-fpga.o
> >> diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
> >> new file mode 100644 index ..e4172c3a6868
> >> --- /dev/null
> >> +++ b/drivers/fpga/zynqmp-fpga.c
> >> @@ -0,0 +1,164 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * Copyright (C) 2018 Xilinx, Inc.
> >> + *
>
> Please delete the unnecessary blank '*' line here.
>
Will Fix in the next version.
> >> + */
> >> +
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
>
> I don't see this firmware/xilinx folder. Is this dependent on other commits
> that
> were submitted? If so, please note that in the header.
>
This Driver have a dependency On firmware.h currently Jolly Shah is working On
it.
Once its got accepted will add the Required API's in the firmware.h
https://lkml.org/lkml/2018/7/17/1038
will fix this in the next version.
> >> +
> >> +/* Constant Definitions */
> >> +#define IXR_FPGA_DONE_MASK 0X0008U
> >> +#define IXR_FPGA_ENCRYPTION_EN 0x0008U
> >> +
> >> +/**
> >> + * struct zynqmp_fpga_priv - Private data structure
> >> + * @dev: Device data structure
> >> + * @flags: flags which is used to identify the bitfile type
> >> + */
> >> +struct zynqmp_fpga_priv {
> >> + struct device *dev;
> >> + u32 flags;
> >> +};
> >> +
> >> +static int zynqmp_fpga_ops_write_init(struct fpga_manager *mgr,
> >> + struct fpga_image_info *info,
> >> + const char *buf, size_t size) {
>
> checkpatch.pl would have complained about this:
> ERROR: open brace '{' following function definitions go on the next line
>
Will
RE: [PATCH char-misc 1/1] Drivers: hv: vmbus: Make synic_initialized flag per-cpu
From: Vitaly Kuznetsov Sent: Tuesday, July 31, 2018 4:20
AM
>
> Reviewed-by: Vitaly Kuznetsov
Thanks for the review
>
> Alternatively, we can get rid of synic_initialized flag altogether:
> hv_synic_init() never fails in the first place but we can always
> implement something like:
>
> int hv_synic_is_initialized(void) {
> union hv_synic_scontrol sctrl;
>
> hv_get_synic_state(sctrl.as_uint64);
>
> return sctrl.enable;
> }
>
> as it doesn't seem that we need to check synic state on _other_ CPUs.
>
> --
> Vitaly
I was trying to decide if there are any arguments in favor of one
approach vs. the other: a per-cpu flag in memory or checking
the synic_control "enable" bit. Seems like a wash to me, in which
case I have a slight preference for the per-cpu flag in memory vs.
creating another function to return sctrl.enable. But I'm completely
open to reasons why checking sctrl.enable is better.
Michael
RE: [PATCH char-misc 1/1] Drivers: hv: vmbus: Make synic_initialized flag per-cpu
From: Vitaly Kuznetsov Sent: Tuesday, July 31, 2018 4:20
AM
>
> Reviewed-by: Vitaly Kuznetsov
Thanks for the review
>
> Alternatively, we can get rid of synic_initialized flag altogether:
> hv_synic_init() never fails in the first place but we can always
> implement something like:
>
> int hv_synic_is_initialized(void) {
> union hv_synic_scontrol sctrl;
>
> hv_get_synic_state(sctrl.as_uint64);
>
> return sctrl.enable;
> }
>
> as it doesn't seem that we need to check synic state on _other_ CPUs.
>
> --
> Vitaly
I was trying to decide if there are any arguments in favor of one
approach vs. the other: a per-cpu flag in memory or checking
the synic_control "enable" bit. Seems like a wash to me, in which
case I have a slight preference for the per-cpu flag in memory vs.
creating another function to return sctrl.enable. But I'm completely
open to reasons why checking sctrl.enable is better.
Michael
linux-next: please clean up the coresight tree
Hi Mathieu,
All but the last 9 commits in the coresight tree have been merged into
the char-misc tree as different commits (they were sent to Greg as a
patch series rather than a merge request). Can you please rebase your
tree into commit ccff2dfaceac ("coresight: tpiu: Fix disabling timeouts")
from the char-misc tree to avoid further conflicts (and duplication if
you ask Greg to merge your tree rather than take a patch series).
--
Cheers,
Stephen Rothwell
pgpzE9qr1DhLm.pgp
Description: OpenPGP digital signature
linux-next: please clean up the coresight tree
Hi Mathieu,
All but the last 9 commits in the coresight tree have been merged into
the char-misc tree as different commits (they were sent to Greg as a
patch series rather than a merge request). Can you please rebase your
tree into commit ccff2dfaceac ("coresight: tpiu: Fix disabling timeouts")
from the char-misc tree to avoid further conflicts (and duplication if
you ask Greg to merge your tree rather than take a patch series).
--
Cheers,
Stephen Rothwell
pgpzE9qr1DhLm.pgp
Description: OpenPGP digital signature
Ws: Trójdzielny za 4,65 dla linux-kernel@vger.kernel.org
Ws: Trójdzielny za 4,65 dla linux-kernel@vger.kernel.org
Re: [QUESTION] llist: Comment releasing 'must delete' restriction before traversing
Byungchul Park writes: > I think rcu list also works well. But I decided to use llist because > llist is simpler and has one less pointer. > > Just to be sure, let me explain my use case more: > >1. Introduced a global list where single linked list is sufficient. >2. All operations I need is to add items and traverse the list. >3. Ensure the operations always happen within irq-disabled section. >4. I'm considering CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG properly. >5. The list can be accessed by every CPU concurrently. > Can you provide more information about where is your use case? Is it a kernel driver? Then it is better to submit the patch together with its user. And I have the similar concern as Steven Rostedt. That is, if you are the only user forever, it's not necessary to change the common code. Best Regards, Huang, Ying
Re: [QUESTION] llist: Comment releasing 'must delete' restriction before traversing
Byungchul Park writes: > I think rcu list also works well. But I decided to use llist because > llist is simpler and has one less pointer. > > Just to be sure, let me explain my use case more: > >1. Introduced a global list where single linked list is sufficient. >2. All operations I need is to add items and traverse the list. >3. Ensure the operations always happen within irq-disabled section. >4. I'm considering CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG properly. >5. The list can be accessed by every CPU concurrently. > Can you provide more information about where is your use case? Is it a kernel driver? Then it is better to submit the patch together with its user. And I have the similar concern as Steven Rostedt. That is, if you are the only user forever, it's not necessary to change the common code. Best Regards, Huang, Ying
Re: Getting the instruction pointer on a per arch basis
On Tue, 31 Jul 2018 16:09:06 -0700
Nick Desaulniers wrote:
> + More maintainers and lists for visibility
>
> On Tue, Jul 31, 2018 at 3:32 PM Nick Desaulniers
> wrote:
> >
> > I'm currently looking into cleaning up the code duplication between
> > current_text_addr() and _THIS_IP_, virtually every implementation of
> > current_text_addr() and _THIS_IP_ itself are basically:
> >
> > #define _THIS_IP_ ({ __label__ _l; _l: &&_l; })
> >
> > For a few arch's, they have inline assembly instead (for
> > current_text_addr()). Examples:
> > * s390
> > * sh
> > * ia64
> > * x86 (um and 32b)
> > * c6x
> > * sparc
> >
> > I have a patch that cuts down on the duplication, but I don't
> > understand why the few arch specific implementations are necessary. I
> > could reduce the duplication further if it's ok to just use the
> > statement expression.
> >
> > Does anyone know why this is the case?
For s390 it is just that we did not know about the label trick when we
introduced the define. The inline has an advantage though, the code
generated with the label trick is using a LARL instruction which is
4 bytes, the inline assembly uses a BASR which is 2 bytes.
If I use the label method in current_text_addr() the size of vmlinux
increases by a small amount:
add/remove: 33/13 grow/shrink: 101/48 up/down: 11941/-8887 (3054)
This is acceptable though, I would not mind if _THIS_IP_ and
current_text_addr use a common definition using labels.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
Re: Getting the instruction pointer on a per arch basis
On Tue, 31 Jul 2018 16:09:06 -0700
Nick Desaulniers wrote:
> + More maintainers and lists for visibility
>
> On Tue, Jul 31, 2018 at 3:32 PM Nick Desaulniers
> wrote:
> >
> > I'm currently looking into cleaning up the code duplication between
> > current_text_addr() and _THIS_IP_, virtually every implementation of
> > current_text_addr() and _THIS_IP_ itself are basically:
> >
> > #define _THIS_IP_ ({ __label__ _l; _l: &&_l; })
> >
> > For a few arch's, they have inline assembly instead (for
> > current_text_addr()). Examples:
> > * s390
> > * sh
> > * ia64
> > * x86 (um and 32b)
> > * c6x
> > * sparc
> >
> > I have a patch that cuts down on the duplication, but I don't
> > understand why the few arch specific implementations are necessary. I
> > could reduce the duplication further if it's ok to just use the
> > statement expression.
> >
> > Does anyone know why this is the case?
For s390 it is just that we did not know about the label trick when we
introduced the define. The inline has an advantage though, the code
generated with the label trick is using a LARL instruction which is
4 bytes, the inline assembly uses a BASR which is 2 bytes.
If I use the label method in current_text_addr() the size of vmlinux
increases by a small amount:
add/remove: 33/13 grow/shrink: 101/48 up/down: 11941/-8887 (3054)
This is acceptable though, I would not mind if _THIS_IP_ and
current_text_addr use a common definition using labels.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
[PATCH v1 2/3] dt-bindings: i3c: Document Qualcomm GENI I3C master bindings
Signed-off-by: Mike Shettel
---
.../devicetree/bindings/i3c/qcom,geni-i3c.txt | 44 ++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i3c/qcom,geni-i3c.txt
diff --git a/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.txt
b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.txt
new file mode 100644
index 000..8b76fda
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.txt
@@ -0,0 +1,44 @@
+Bindings for Qualcomm Geni I3C master block
+===
+
+Required properties:
+
+- compatible: shall be "qcom,geni-i3c"
+- clocks: shall reference the se clock
+- clock-names: shall contain clock name corresponding to the serial engine
+- interrupts: the interrupt line connected to this I3C master
+- reg: I3C master registers
+
+Optional properties:
+
+- se-clock-frequency: Source serial clock frequency to use
+- dfs-index: Dynamic frequency scaling table index to use
+
+Mandatory properties defined by the generic binding (see
+Documentation/devicetree/bindings/i3c/i3c.txt for more details):
+
+- #address-cells: shall be set to 3
+- #size-cells: shall be set to 0
+
+Optional properties defined by the generic binding (see
+Documentation/devicetree/bindings/i3c/i3c.txt for more details):
+
+- i2c-scl-hz
+- i3c-scl-hz
+
+I3C device connected on the bus follow the generic description (see
+Documentation/devicetree/bindings/i3c/i3c.txt for more details).
+
+Example:
+
+ i3c-master@0d04 {
+ compatible = "qcom,geni-i3c";
+ clocks = <_gcc GCC_QUPV3_WRAP0_S0_CLK>;
+ clock-names = "se";
+ interrupts = ;
+ reg = <0x0d04 0x4000>;
+ #address-cells = <3>;
+ #size-cells = <0>;
+ i2c-scl-hz = <10>;
+ };
+
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v1 0/3] Add a driver for Qualcomm GENI I3C master IP
This patch series is a proposal for the I3C master driver for the Qualcomm GENI IP. This patch is to be applied on top of the I3C subsystem patchset v6 submitted by Boris Brezillon. Features supported: Regular CCC commands I3C private transfers I2C transfers Features not yet supported: Hot-join IBI Mike Shettel (3): i3c: master: Add a driver for Qualcomm GENI I3C master IP dt-bindings: i3c: Document Qualcomm GENI I3C master bindings MAINTAINERS: Add Qualcomm Generic Interface I3C driver maintainer .../devicetree/bindings/i3c/qcom,geni-i3c.txt | 44 + MAINTAINERS|8 + drivers/i3c/master/Kconfig | 13 + drivers/i3c/master/Makefile|1 + drivers/i3c/master/i3c-master-qcom-geni.c | 1243 5 files changed, 1309 insertions(+) create mode 100644 Documentation/devicetree/bindings/i3c/qcom,geni-i3c.txt create mode 100644 drivers/i3c/master/i3c-master-qcom-geni.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v1 3/3] MAINTAINERS: Add Qualcomm Generic Interface I3C driver maintainer
Signed-off-by: Mike Shettel --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 622327b..775829c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10943,6 +10943,14 @@ L: net...@vger.kernel.org S: Supported F: drivers/net/ethernet/qualcomm/emac/ +QUALCOMM GENERIC INTERFACE I3C DRIVER +M: Alok Chauhan +M: Mike Shettel +L: linux-...@vger.kernel.org +L: linux-arm-...@vger.kernel.org +S: Supported +F: drivers/i3c/master/i3c-master-qcom-geni.c + QUALCOMM HEXAGON ARCHITECTURE M: Richard Kuo L: linux-hexa...@vger.kernel.org -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v1 1/3] i3c: master: Add a driver for Qualcomm GENI I3C master IP
Signed-off-by: Mike Shettel
---
This patch is to be applied on top of the I3C subsystem patchset v6.
drivers/i3c/master/Kconfig| 13 +
drivers/i3c/master/Makefile |1 +
drivers/i3c/master/i3c-master-qcom-geni.c | 1243 +
3 files changed, 1257 insertions(+)
create mode 100644 drivers/i3c/master/i3c-master-qcom-geni.c
diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
index 56b9a18..18ca523 100644
--- a/drivers/i3c/master/Kconfig
+++ b/drivers/i3c/master/Kconfig
@@ -3,3 +3,16 @@ config CDNS_I3C_MASTER
depends on I3C
help
Enable this driver if you want to support Cadence I3C master block.
+
+config I3C_MASTER_QCOM_GENI
+ tristate "Qualcomm Technologies Inc GENI based I3C master"
+ depends on ARCH_QCOM
+depends on I3C
+ help
+ If you say yes to this option, support will be included for the
+ built-in I3C interface on the Qualcomm Technologies Inc SoCs.
+
+ This driver can also be built as a module. If so, the module
+ will be called i3c-master-qcom-geni
+
+
diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
index 4c4304a..5d084a9 100644
--- a/drivers/i3c/master/Makefile
+++ b/drivers/i3c/master/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_CDNS_I3C_MASTER) += i3c-master-cdns.o
+obj-$(CONFIG_I3C_MASTER_QCOM_GENI) += i3c-master-qcom-geni.o
diff --git a/drivers/i3c/master/i3c-master-qcom-geni.c
b/drivers/i3c/master/i3c-master-qcom-geni.c
new file mode 100644
index 000..dfbc257
--- /dev/null
+++ b/drivers/i3c/master/i3c-master-qcom-geni.c
@@ -0,0 +1,1243 @@
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define SE_I3C_SCL_HIGH 0x268
+#define SE_I3C_TX_TRANS_LEN 0x26C
+#define SE_I3C_RX_TRANS_LEN 0x270
+#define SE_I3C_DELAY_COUNTER0x274
+#define SE_I2C_SCL_COUNTERS 0x278
+#define SE_I3C_SCL_CYCLE0x27C
+#define SE_GENI_HW_IRQ_EN 0x920
+#define SE_GENI_HW_IRQ_IGNORE_ON_ACTIVE 0x924
+#define SE_GENI_HW_IRQ_CMD_PARAM_0 0x930
+
+/* SE_GENI_M_CLK_CFG field shifts */
+#define CLK_DEV_VALUE_SHFT 4
+#define SER_CLK_EN_SHFT 0
+
+/* SE_GENI_HW_IRQ_CMD_PARAM_0 field shifts */
+#define M_IBI_IRQ_PARAM_7E_SHFT 0
+#define M_IBI_IRQ_PARAM_STOP_STALL_SHFT 1
+
+/* SE_I2C_SCL_COUNTERS field shifts */
+#define I2C_SCL_HIGH_COUNTER_SHFT 20
+#define I2C_SCL_LOW_COUNTER_SHFT 10
+
+#define SE_I3C_ERR (M_CMD_OVERRUN_EN | M_ILLEGAL_CMD_EN | M_CMD_FAILURE_EN |\
+ M_CMD_ABORT_EN | M_GP_IRQ_0_EN | M_GP_IRQ_1_EN | M_GP_IRQ_2_EN | \
+ M_GP_IRQ_3_EN | M_GP_IRQ_4_EN)
+
+/* M_CMD OP codes for I2C/I3C */
+#define I3C_READ_IBI_HW 0
+#define I2C_WRITE1
+#define I2C_READ 2
+#define I2C_WRITE_READ 3
+#define I2C_ADDR_ONLY4
+#define I3C_INBAND_RESET 5
+#define I2C_BUS_CLEAR6
+#define I2C_STOP_ON_BUS 7
+#define I3C_HDR_DDR_EXIT 8
+#define I3C_PRIVATE_WRITE9
+#define I3C_PRIVATE_READ 10
+#define I3C_HDR_DDR_WRITE11
+#define I3C_HDR_DDR_READ 12
+#define I3C_DIRECT_CCC_ADDR_ONLY 13
+#define I3C_BCAST_CCC_ADDR_ONLY 14
+#define I3C_READ_IBI 15
+#define I3C_BCAST_CCC_WRITE 16
+#define I3C_DIRECT_CCC_WRITE 17
+#define I3C_DIRECT_CCC_READ 18
+/* M_CMD params for I3C */
+#define PRE_CMD_DELAY BIT(0)
+#define TIMESTAMP_BEFORE BIT(1)
+#define STOP_STRETCH BIT(2)
+#define TIMESTAMP_AFTERBIT(3)
+#define POST_COMMAND_DELAY BIT(4)
+#define IGNORE_ADD_NACKBIT(6)
+#define READ_FINISHED_WITH_ACK BIT(7)
+#define CONTINUOUS_MODE_DAABIT(8)
+#define SLV_ADDR_MSK GENMASK(15, 9)
+#define SLV_ADDR_SHFT 9
+#define CCC_HDR_CMD_MSKGENMASK(23, 16)
+#define CCC_HDR_CMD_SHFT 16
+#define IBI_NACK_TBL_CTRL BIT(24)
+#define USE_7E BIT(25)
+#define BYPASS_ADDR_PHASE BIT(26)
+
+enum geni_i3c_err_code {
+ RD_TERM,
+ NACK,
+ CRC_ERR,
+ BUS_PROTO,
+ NACK_7E,
+ NACK_IBI,
+ GENI_OVERRUN,
+ GENI_ILLEGAL_CMD,
+ GENI_ABORT_DONE,
+ GENI_TIMEOUT,
+};
+
+#define DM_I3C_CB_ERR ((BIT(NACK) | BIT(BUS_PROTO) | BIT(NACK_7E)) << 5)
+
+#define
[PATCH v1 2/3] dt-bindings: i3c: Document Qualcomm GENI I3C master bindings
Signed-off-by: Mike Shettel
---
.../devicetree/bindings/i3c/qcom,geni-i3c.txt | 44 ++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i3c/qcom,geni-i3c.txt
diff --git a/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.txt
b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.txt
new file mode 100644
index 000..8b76fda
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.txt
@@ -0,0 +1,44 @@
+Bindings for Qualcomm Geni I3C master block
+===
+
+Required properties:
+
+- compatible: shall be "qcom,geni-i3c"
+- clocks: shall reference the se clock
+- clock-names: shall contain clock name corresponding to the serial engine
+- interrupts: the interrupt line connected to this I3C master
+- reg: I3C master registers
+
+Optional properties:
+
+- se-clock-frequency: Source serial clock frequency to use
+- dfs-index: Dynamic frequency scaling table index to use
+
+Mandatory properties defined by the generic binding (see
+Documentation/devicetree/bindings/i3c/i3c.txt for more details):
+
+- #address-cells: shall be set to 3
+- #size-cells: shall be set to 0
+
+Optional properties defined by the generic binding (see
+Documentation/devicetree/bindings/i3c/i3c.txt for more details):
+
+- i2c-scl-hz
+- i3c-scl-hz
+
+I3C device connected on the bus follow the generic description (see
+Documentation/devicetree/bindings/i3c/i3c.txt for more details).
+
+Example:
+
+ i3c-master@0d04 {
+ compatible = "qcom,geni-i3c";
+ clocks = <_gcc GCC_QUPV3_WRAP0_S0_CLK>;
+ clock-names = "se";
+ interrupts = ;
+ reg = <0x0d04 0x4000>;
+ #address-cells = <3>;
+ #size-cells = <0>;
+ i2c-scl-hz = <10>;
+ };
+
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v1 0/3] Add a driver for Qualcomm GENI I3C master IP
This patch series is a proposal for the I3C master driver for the Qualcomm GENI IP. This patch is to be applied on top of the I3C subsystem patchset v6 submitted by Boris Brezillon. Features supported: Regular CCC commands I3C private transfers I2C transfers Features not yet supported: Hot-join IBI Mike Shettel (3): i3c: master: Add a driver for Qualcomm GENI I3C master IP dt-bindings: i3c: Document Qualcomm GENI I3C master bindings MAINTAINERS: Add Qualcomm Generic Interface I3C driver maintainer .../devicetree/bindings/i3c/qcom,geni-i3c.txt | 44 + MAINTAINERS|8 + drivers/i3c/master/Kconfig | 13 + drivers/i3c/master/Makefile|1 + drivers/i3c/master/i3c-master-qcom-geni.c | 1243 5 files changed, 1309 insertions(+) create mode 100644 Documentation/devicetree/bindings/i3c/qcom,geni-i3c.txt create mode 100644 drivers/i3c/master/i3c-master-qcom-geni.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v1 3/3] MAINTAINERS: Add Qualcomm Generic Interface I3C driver maintainer
Signed-off-by: Mike Shettel --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 622327b..775829c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10943,6 +10943,14 @@ L: net...@vger.kernel.org S: Supported F: drivers/net/ethernet/qualcomm/emac/ +QUALCOMM GENERIC INTERFACE I3C DRIVER +M: Alok Chauhan +M: Mike Shettel +L: linux-...@vger.kernel.org +L: linux-arm-...@vger.kernel.org +S: Supported +F: drivers/i3c/master/i3c-master-qcom-geni.c + QUALCOMM HEXAGON ARCHITECTURE M: Richard Kuo L: linux-hexa...@vger.kernel.org -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v1 1/3] i3c: master: Add a driver for Qualcomm GENI I3C master IP
Signed-off-by: Mike Shettel
---
This patch is to be applied on top of the I3C subsystem patchset v6.
drivers/i3c/master/Kconfig| 13 +
drivers/i3c/master/Makefile |1 +
drivers/i3c/master/i3c-master-qcom-geni.c | 1243 +
3 files changed, 1257 insertions(+)
create mode 100644 drivers/i3c/master/i3c-master-qcom-geni.c
diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
index 56b9a18..18ca523 100644
--- a/drivers/i3c/master/Kconfig
+++ b/drivers/i3c/master/Kconfig
@@ -3,3 +3,16 @@ config CDNS_I3C_MASTER
depends on I3C
help
Enable this driver if you want to support Cadence I3C master block.
+
+config I3C_MASTER_QCOM_GENI
+ tristate "Qualcomm Technologies Inc GENI based I3C master"
+ depends on ARCH_QCOM
+depends on I3C
+ help
+ If you say yes to this option, support will be included for the
+ built-in I3C interface on the Qualcomm Technologies Inc SoCs.
+
+ This driver can also be built as a module. If so, the module
+ will be called i3c-master-qcom-geni
+
+
diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
index 4c4304a..5d084a9 100644
--- a/drivers/i3c/master/Makefile
+++ b/drivers/i3c/master/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_CDNS_I3C_MASTER) += i3c-master-cdns.o
+obj-$(CONFIG_I3C_MASTER_QCOM_GENI) += i3c-master-qcom-geni.o
diff --git a/drivers/i3c/master/i3c-master-qcom-geni.c
b/drivers/i3c/master/i3c-master-qcom-geni.c
new file mode 100644
index 000..dfbc257
--- /dev/null
+++ b/drivers/i3c/master/i3c-master-qcom-geni.c
@@ -0,0 +1,1243 @@
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define SE_I3C_SCL_HIGH 0x268
+#define SE_I3C_TX_TRANS_LEN 0x26C
+#define SE_I3C_RX_TRANS_LEN 0x270
+#define SE_I3C_DELAY_COUNTER0x274
+#define SE_I2C_SCL_COUNTERS 0x278
+#define SE_I3C_SCL_CYCLE0x27C
+#define SE_GENI_HW_IRQ_EN 0x920
+#define SE_GENI_HW_IRQ_IGNORE_ON_ACTIVE 0x924
+#define SE_GENI_HW_IRQ_CMD_PARAM_0 0x930
+
+/* SE_GENI_M_CLK_CFG field shifts */
+#define CLK_DEV_VALUE_SHFT 4
+#define SER_CLK_EN_SHFT 0
+
+/* SE_GENI_HW_IRQ_CMD_PARAM_0 field shifts */
+#define M_IBI_IRQ_PARAM_7E_SHFT 0
+#define M_IBI_IRQ_PARAM_STOP_STALL_SHFT 1
+
+/* SE_I2C_SCL_COUNTERS field shifts */
+#define I2C_SCL_HIGH_COUNTER_SHFT 20
+#define I2C_SCL_LOW_COUNTER_SHFT 10
+
+#define SE_I3C_ERR (M_CMD_OVERRUN_EN | M_ILLEGAL_CMD_EN | M_CMD_FAILURE_EN |\
+ M_CMD_ABORT_EN | M_GP_IRQ_0_EN | M_GP_IRQ_1_EN | M_GP_IRQ_2_EN | \
+ M_GP_IRQ_3_EN | M_GP_IRQ_4_EN)
+
+/* M_CMD OP codes for I2C/I3C */
+#define I3C_READ_IBI_HW 0
+#define I2C_WRITE1
+#define I2C_READ 2
+#define I2C_WRITE_READ 3
+#define I2C_ADDR_ONLY4
+#define I3C_INBAND_RESET 5
+#define I2C_BUS_CLEAR6
+#define I2C_STOP_ON_BUS 7
+#define I3C_HDR_DDR_EXIT 8
+#define I3C_PRIVATE_WRITE9
+#define I3C_PRIVATE_READ 10
+#define I3C_HDR_DDR_WRITE11
+#define I3C_HDR_DDR_READ 12
+#define I3C_DIRECT_CCC_ADDR_ONLY 13
+#define I3C_BCAST_CCC_ADDR_ONLY 14
+#define I3C_READ_IBI 15
+#define I3C_BCAST_CCC_WRITE 16
+#define I3C_DIRECT_CCC_WRITE 17
+#define I3C_DIRECT_CCC_READ 18
+/* M_CMD params for I3C */
+#define PRE_CMD_DELAY BIT(0)
+#define TIMESTAMP_BEFORE BIT(1)
+#define STOP_STRETCH BIT(2)
+#define TIMESTAMP_AFTERBIT(3)
+#define POST_COMMAND_DELAY BIT(4)
+#define IGNORE_ADD_NACKBIT(6)
+#define READ_FINISHED_WITH_ACK BIT(7)
+#define CONTINUOUS_MODE_DAABIT(8)
+#define SLV_ADDR_MSK GENMASK(15, 9)
+#define SLV_ADDR_SHFT 9
+#define CCC_HDR_CMD_MSKGENMASK(23, 16)
+#define CCC_HDR_CMD_SHFT 16
+#define IBI_NACK_TBL_CTRL BIT(24)
+#define USE_7E BIT(25)
+#define BYPASS_ADDR_PHASE BIT(26)
+
+enum geni_i3c_err_code {
+ RD_TERM,
+ NACK,
+ CRC_ERR,
+ BUS_PROTO,
+ NACK_7E,
+ NACK_IBI,
+ GENI_OVERRUN,
+ GENI_ILLEGAL_CMD,
+ GENI_ABORT_DONE,
+ GENI_TIMEOUT,
+};
+
+#define DM_I3C_CB_ERR ((BIT(NACK) | BIT(BUS_PROTO) | BIT(NACK_7E)) << 5)
+
+#define
Re: bisected - arm64 kvm unit test failures
On Tue, 31 Jul 2018 19:28:49 +0100, Mike Galbraith wrote: Hi Mike, > > [1 ] > On Mon, 2018-07-30 at 18:24 +0200, Mike Galbraith wrote: > > On Sun, 2018-07-29 at 13:47 +0200, Mike Galbraith wrote: > > > FYI, per kvm unit tests, 4.16-rt definitely has more kvm issues. > > But it's not RT, or rather most of it isn't... > > > > huawei5:/abuild/mike/kvm-unit-tests # uname -r > > > 4.16.18-rt11-rt > > > huawei5:/abuild/mike/kvm-unit-tests # ./run_tests.sh > > > PASS selftest-setup (2 tests) > > > FAIL selftest-vectors-kernel > > > FAIL selftest-vectors-user > > > PASS selftest-smp (65 tests) > > > PASS pci-test (1 tests) > > > PASS pmu (3 tests) > > > FAIL gicv2-ipi > > > FAIL gicv3-ipi > > > FAIL gicv2-active > > > FAIL gicv3-active > > > PASS psci (4 tests) > > > FAIL timer > > > huawei5:/abuild/mike/kvm-unit-tests # > > > > > > 4.14-rt passes all tests. The above is with the kvm raw_spinlock_t > > > conversion patch applied, but the 4.12 based SLERT tree I cloned to > > > explore arm-land in the first place shows only one timer failure, and > > > has/needs it applied as well, which would seem to vindicate it. > > > > > > huawei5:/abuild/mike/kvm-unit-tests # uname -r > > > 4.12.14-0.gec0b559-rt > > > huawei5:/abuild/mike/kvm-unit-tests # ./run_tests.sh > > > PASS selftest-setup (2 tests) > > > PASS selftest-vectors-kernel (2 tests) > > > PASS selftest-vectors-user (2 tests) > > > PASS selftest-smp (65 tests) > > > PASS pci-test (1 tests) > > > PASS pmu (3 tests) > > > PASS gicv2-ipi (3 tests) > > > PASS gicv3-ipi (3 tests) > > > PASS gicv2-active (1 tests) > > > PASS gicv3-active (1 tests) > > > PASS psci (4 tests) > > > FAIL timer (8 tests, 1 unexpected failures) > > > > FWIW, this single timer failure wass inspired by something in the 4-15 > > merge window. > > As noted, the single timer failure is an RT issue of some sort, and > remains. The rest I bisected in @stable with the attached config, and > confirmed that revert fixes up 4.16-rt as well (modulo singleton). Is it something that is reproducible with the current mainline (non-RT)? > > a9c0e12ebee56ef06b7eccdbc73bab71d0018df8 is the first bad commit > commit a9c0e12ebee56ef06b7eccdbc73bab71d0018df8 > Author: Marc Zyngier > Date: Mon Oct 23 17:11:20 2017 +0100 > > KVM: arm/arm64: Only clean the dcache on translation fault > > The only case where we actually need to perform a dcache maintenance > is when we map the page for the first time, and subsequent permission > faults do not require cache maintenance. Let's make it conditional > on not being a permission fault (and thus a translation fault). > > Reviewed-by: Christoffer Dall > Signed-off-by: Marc Zyngier > Signed-off-by: Christoffer Dall Pretty worrying. What HW is that on? M. -- Jazz is not dead, it just smell funny.
Re: bisected - arm64 kvm unit test failures
On Tue, 31 Jul 2018 19:28:49 +0100, Mike Galbraith wrote: Hi Mike, > > [1 ] > On Mon, 2018-07-30 at 18:24 +0200, Mike Galbraith wrote: > > On Sun, 2018-07-29 at 13:47 +0200, Mike Galbraith wrote: > > > FYI, per kvm unit tests, 4.16-rt definitely has more kvm issues. > > But it's not RT, or rather most of it isn't... > > > > huawei5:/abuild/mike/kvm-unit-tests # uname -r > > > 4.16.18-rt11-rt > > > huawei5:/abuild/mike/kvm-unit-tests # ./run_tests.sh > > > PASS selftest-setup (2 tests) > > > FAIL selftest-vectors-kernel > > > FAIL selftest-vectors-user > > > PASS selftest-smp (65 tests) > > > PASS pci-test (1 tests) > > > PASS pmu (3 tests) > > > FAIL gicv2-ipi > > > FAIL gicv3-ipi > > > FAIL gicv2-active > > > FAIL gicv3-active > > > PASS psci (4 tests) > > > FAIL timer > > > huawei5:/abuild/mike/kvm-unit-tests # > > > > > > 4.14-rt passes all tests. The above is with the kvm raw_spinlock_t > > > conversion patch applied, but the 4.12 based SLERT tree I cloned to > > > explore arm-land in the first place shows only one timer failure, and > > > has/needs it applied as well, which would seem to vindicate it. > > > > > > huawei5:/abuild/mike/kvm-unit-tests # uname -r > > > 4.12.14-0.gec0b559-rt > > > huawei5:/abuild/mike/kvm-unit-tests # ./run_tests.sh > > > PASS selftest-setup (2 tests) > > > PASS selftest-vectors-kernel (2 tests) > > > PASS selftest-vectors-user (2 tests) > > > PASS selftest-smp (65 tests) > > > PASS pci-test (1 tests) > > > PASS pmu (3 tests) > > > PASS gicv2-ipi (3 tests) > > > PASS gicv3-ipi (3 tests) > > > PASS gicv2-active (1 tests) > > > PASS gicv3-active (1 tests) > > > PASS psci (4 tests) > > > FAIL timer (8 tests, 1 unexpected failures) > > > > FWIW, this single timer failure wass inspired by something in the 4-15 > > merge window. > > As noted, the single timer failure is an RT issue of some sort, and > remains. The rest I bisected in @stable with the attached config, and > confirmed that revert fixes up 4.16-rt as well (modulo singleton). Is it something that is reproducible with the current mainline (non-RT)? > > a9c0e12ebee56ef06b7eccdbc73bab71d0018df8 is the first bad commit > commit a9c0e12ebee56ef06b7eccdbc73bab71d0018df8 > Author: Marc Zyngier > Date: Mon Oct 23 17:11:20 2017 +0100 > > KVM: arm/arm64: Only clean the dcache on translation fault > > The only case where we actually need to perform a dcache maintenance > is when we map the page for the first time, and subsequent permission > faults do not require cache maintenance. Let's make it conditional > on not being a permission fault (and thus a translation fault). > > Reviewed-by: Christoffer Dall > Signed-off-by: Marc Zyngier > Signed-off-by: Christoffer Dall Pretty worrying. What HW is that on? M. -- Jazz is not dead, it just smell funny.
Re: [QUESTION] llist: Comment releasing 'must delete' restriction before traversing
On Tue, Jul 31, 2018 at 09:46:16AM -0400, Steven Rostedt wrote: > On Tue, 31 Jul 2018 18:38:09 +0900 > Byungchul Park wrote: > > > On Tue, Jul 31, 2018 at 10:52:57AM +0200, Peter Zijlstra wrote: > > > On Tue, Jul 31, 2018 at 09:58:36AM +0900, Byungchul Park wrote: > > > > In restrictive cases like only addtions happen but never deletion, can't > > > > we safely traverse a llist? I believe llist can be more useful if we can > > > > release the restriction. Can't we? > > > > > > Yes you can, but I'm not sure it makes much sense to confuse the > > > comments with this. > > > > > > This really ends up in the 'you had better know what you're doing' > > > category. > > > > Partially agree with you. But why not add more explanation? > > > > If you think the comment confuse us, then it's ok to keep it itself. > > > > It appears that you will only have your own users that will be doing > this, correct? How many use cases have no deletion? I wouldn't change > the generic comments on llist for something that's a one off and hardly > used. Thanks for the comment. OK. > -- Steve
Re: [QUESTION] llist: Comment releasing 'must delete' restriction before traversing
On Tue, Jul 31, 2018 at 07:30:52AM -0700, Paul E. McKenney wrote: > On Tue, Jul 31, 2018 at 06:29:50PM +0900, Byungchul Park wrote: > > On Mon, Jul 30, 2018 at 09:30:42PM -0700, Paul E. McKenney wrote: > > > On Tue, Jul 31, 2018 at 09:58:36AM +0900, Byungchul Park wrote: > > > > Hello folks, > > > > > > > > I'm careful in saying.. and curious about.. > > > > > > > > In restrictive cases like only addtions happen but never deletion, can't > > > > we safely traverse a llist? I believe llist can be more useful if we can > > > > release the restriction. Can't we? > > > > > > Yes, but please give a thought to the people looking at your code some > > > time down the line. If you are doing this, lots of comments, please. > > > > Yes, I will. Thank you for the comment. > > > > > Here are the approaches that I am aware of: > > > > > > 1.Normal RCU. Use list_add_rcu(), list_del_rcu(), and friends. > > > > > > 2.Things are added but never deleted. Use list_add_rcu() and > > > friends, but since you don't ever delete anything, you never > > > use list_del_rcu(), synchronize_rcu(), call_rcu(), and friends. > > > > I think rcu list also works well. But I decided to use llist because > > llist is simpler and has one less pointer. > > No. > > To see this, look at llist_for_each() below, which is absolutely -not- > able to reliably traverse lists while nodes are being inserted. Ah.. Not only 'node' but also 'pos->next' can cause a problem w/o rcu_dereference or similar.. I need consider another way. Or I might need memory barrier between READ_ONCE(head->first) and llist_for_each() to make sure safely to read 'pos->next' when I see a value of 'head->first'. > #define llist_for_each(pos, node) \ > for ((pos) = (node); pos; (pos) = (pos)->next) > > Now, you could introduce an llist_for_each_rcu() that used rcu_dereference > or similar (thus handling insertion, but that is not what your patches > currently do. Right. > > Just to be sure, let me explain my use case more: > > > >1. Introduced a global list where single linked list is sufficient. > >2. All operations I need is to add items and traverse the list. > >3. Ensure the operations always happen within irq-disabled section. > >4. I'm considering CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG properly. > >5. The list can be accessed by every CPU concurrently. > > > > Of course, I can use normal double list with a lock or rcu list. But I > > think it doesn't have to be protected by even rcu in that case. I wanted > > to use the simplest one all requiremnets are satisfied with and I > > thought it's llist. Thoughts? > > If you want lockless reader traversal, you need rcu_dereference(). Yes, I need that or similar anyway. Thanks a lot, Paul!
Re: [QUESTION] llist: Comment releasing 'must delete' restriction before traversing
On Tue, Jul 31, 2018 at 09:46:16AM -0400, Steven Rostedt wrote: > On Tue, 31 Jul 2018 18:38:09 +0900 > Byungchul Park wrote: > > > On Tue, Jul 31, 2018 at 10:52:57AM +0200, Peter Zijlstra wrote: > > > On Tue, Jul 31, 2018 at 09:58:36AM +0900, Byungchul Park wrote: > > > > In restrictive cases like only addtions happen but never deletion, can't > > > > we safely traverse a llist? I believe llist can be more useful if we can > > > > release the restriction. Can't we? > > > > > > Yes you can, but I'm not sure it makes much sense to confuse the > > > comments with this. > > > > > > This really ends up in the 'you had better know what you're doing' > > > category. > > > > Partially agree with you. But why not add more explanation? > > > > If you think the comment confuse us, then it's ok to keep it itself. > > > > It appears that you will only have your own users that will be doing > this, correct? How many use cases have no deletion? I wouldn't change > the generic comments on llist for something that's a one off and hardly > used. Thanks for the comment. OK. > -- Steve
Re: [QUESTION] llist: Comment releasing 'must delete' restriction before traversing
On Tue, Jul 31, 2018 at 07:30:52AM -0700, Paul E. McKenney wrote: > On Tue, Jul 31, 2018 at 06:29:50PM +0900, Byungchul Park wrote: > > On Mon, Jul 30, 2018 at 09:30:42PM -0700, Paul E. McKenney wrote: > > > On Tue, Jul 31, 2018 at 09:58:36AM +0900, Byungchul Park wrote: > > > > Hello folks, > > > > > > > > I'm careful in saying.. and curious about.. > > > > > > > > In restrictive cases like only addtions happen but never deletion, can't > > > > we safely traverse a llist? I believe llist can be more useful if we can > > > > release the restriction. Can't we? > > > > > > Yes, but please give a thought to the people looking at your code some > > > time down the line. If you are doing this, lots of comments, please. > > > > Yes, I will. Thank you for the comment. > > > > > Here are the approaches that I am aware of: > > > > > > 1.Normal RCU. Use list_add_rcu(), list_del_rcu(), and friends. > > > > > > 2.Things are added but never deleted. Use list_add_rcu() and > > > friends, but since you don't ever delete anything, you never > > > use list_del_rcu(), synchronize_rcu(), call_rcu(), and friends. > > > > I think rcu list also works well. But I decided to use llist because > > llist is simpler and has one less pointer. > > No. > > To see this, look at llist_for_each() below, which is absolutely -not- > able to reliably traverse lists while nodes are being inserted. Ah.. Not only 'node' but also 'pos->next' can cause a problem w/o rcu_dereference or similar.. I need consider another way. Or I might need memory barrier between READ_ONCE(head->first) and llist_for_each() to make sure safely to read 'pos->next' when I see a value of 'head->first'. > #define llist_for_each(pos, node) \ > for ((pos) = (node); pos; (pos) = (pos)->next) > > Now, you could introduce an llist_for_each_rcu() that used rcu_dereference > or similar (thus handling insertion, but that is not what your patches > currently do. Right. > > Just to be sure, let me explain my use case more: > > > >1. Introduced a global list where single linked list is sufficient. > >2. All operations I need is to add items and traverse the list. > >3. Ensure the operations always happen within irq-disabled section. > >4. I'm considering CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG properly. > >5. The list can be accessed by every CPU concurrently. > > > > Of course, I can use normal double list with a lock or rcu list. But I > > think it doesn't have to be protected by even rcu in that case. I wanted > > to use the simplest one all requiremnets are satisfied with and I > > thought it's llist. Thoughts? > > If you want lockless reader traversal, you need rcu_dereference(). Yes, I need that or similar anyway. Thanks a lot, Paul!
Re: [PATCH v3] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
On Wed, 1 Aug 2018 13:10:49 +0800 YueHaibing wrote:
> fixes following Smatch static check warning:
>
> drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state()
> warn: passing devm_ allocated variable to kfree. 'pctrl->functions'
>
> As we will be calling krealloc() on pointer 'pctrl->functions', which means
> kfree() will be called in there, devm_kzalloc() shouldn't be used with
> the allocation in the first place. Fix the warning by calling kcalloc()
> and managing the free procedure in error path on our own.
>
> Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for
> Marvell Berlin SoCs")
> Signed-off-by: YueHaibing
Reviewed-by: Jisheng Zhang
> ---
> v2: free pctrl->functions instead of function as Jisheng Zhang suggested
> v3: v2 I send a wrong patch,this is the correct patch.
> ---
> drivers/pinctrl/berlin/berlin.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c
> index d6d183e..b5903ff 100644
> --- a/drivers/pinctrl/berlin/berlin.c
> +++ b/drivers/pinctrl/berlin/berlin.c
> @@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct
> platform_device *pdev)
> }
>
> /* we will reallocate later */
> - pctrl->functions = devm_kcalloc(>dev,
> - max_functions,
> - sizeof(*pctrl->functions),
> - GFP_KERNEL);
> + pctrl->functions = kcalloc(max_functions,
> +sizeof(*pctrl->functions), GFP_KERNEL);
> if (!pctrl->functions)
> return -ENOMEM;
>
> @@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct
> platform_device *pdev)
> function++;
> }
>
> - if (!found)
> + if (!found) {
> + kfree(pctrl->functions);
> return -EINVAL;
> + }
>
> if (!function->groups) {
> function->groups =
> @@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct
> platform_device *pdev)
>sizeof(char *),
>GFP_KERNEL);
>
> - if (!function->groups)
> + if (!function->groups) {
> + kfree(pctrl->functions);
> return -ENOMEM;
> + }
> }
>
> groups = function->groups;
Re: [PATCH v3] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
On Wed, 1 Aug 2018 13:10:49 +0800 YueHaibing wrote:
> fixes following Smatch static check warning:
>
> drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state()
> warn: passing devm_ allocated variable to kfree. 'pctrl->functions'
>
> As we will be calling krealloc() on pointer 'pctrl->functions', which means
> kfree() will be called in there, devm_kzalloc() shouldn't be used with
> the allocation in the first place. Fix the warning by calling kcalloc()
> and managing the free procedure in error path on our own.
>
> Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for
> Marvell Berlin SoCs")
> Signed-off-by: YueHaibing
Reviewed-by: Jisheng Zhang
> ---
> v2: free pctrl->functions instead of function as Jisheng Zhang suggested
> v3: v2 I send a wrong patch,this is the correct patch.
> ---
> drivers/pinctrl/berlin/berlin.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c
> index d6d183e..b5903ff 100644
> --- a/drivers/pinctrl/berlin/berlin.c
> +++ b/drivers/pinctrl/berlin/berlin.c
> @@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct
> platform_device *pdev)
> }
>
> /* we will reallocate later */
> - pctrl->functions = devm_kcalloc(>dev,
> - max_functions,
> - sizeof(*pctrl->functions),
> - GFP_KERNEL);
> + pctrl->functions = kcalloc(max_functions,
> +sizeof(*pctrl->functions), GFP_KERNEL);
> if (!pctrl->functions)
> return -ENOMEM;
>
> @@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct
> platform_device *pdev)
> function++;
> }
>
> - if (!found)
> + if (!found) {
> + kfree(pctrl->functions);
> return -EINVAL;
> + }
>
> if (!function->groups) {
> function->groups =
> @@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct
> platform_device *pdev)
>sizeof(char *),
>GFP_KERNEL);
>
> - if (!function->groups)
> + if (!function->groups) {
> + kfree(pctrl->functions);
> return -ENOMEM;
> + }
> }
>
> groups = function->groups;
[PATCH v3] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
fixes following Smatch static check warning:
drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state()
warn: passing devm_ allocated variable to kfree. 'pctrl->functions'
As we will be calling krealloc() on pointer 'pctrl->functions', which means
kfree() will be called in there, devm_kzalloc() shouldn't be used with
the allocation in the first place. Fix the warning by calling kcalloc()
and managing the free procedure in error path on our own.
Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for Marvell
Berlin SoCs")
Signed-off-by: YueHaibing
---
v2: free pctrl->functions instead of function as Jisheng Zhang suggested
v3: v2 I send a wrong patch,this is the correct patch.
---
drivers/pinctrl/berlin/berlin.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c
index d6d183e..b5903ff 100644
--- a/drivers/pinctrl/berlin/berlin.c
+++ b/drivers/pinctrl/berlin/berlin.c
@@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct
platform_device *pdev)
}
/* we will reallocate later */
- pctrl->functions = devm_kcalloc(>dev,
- max_functions,
- sizeof(*pctrl->functions),
- GFP_KERNEL);
+ pctrl->functions = kcalloc(max_functions,
+ sizeof(*pctrl->functions), GFP_KERNEL);
if (!pctrl->functions)
return -ENOMEM;
@@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct
platform_device *pdev)
function++;
}
- if (!found)
+ if (!found) {
+ kfree(pctrl->functions);
return -EINVAL;
+ }
if (!function->groups) {
function->groups =
@@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct
platform_device *pdev)
sizeof(char *),
GFP_KERNEL);
- if (!function->groups)
+ if (!function->groups) {
+ kfree(pctrl->functions);
return -ENOMEM;
+ }
}
groups = function->groups;
--
2.7.0
[PATCH v3] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
fixes following Smatch static check warning:
drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state()
warn: passing devm_ allocated variable to kfree. 'pctrl->functions'
As we will be calling krealloc() on pointer 'pctrl->functions', which means
kfree() will be called in there, devm_kzalloc() shouldn't be used with
the allocation in the first place. Fix the warning by calling kcalloc()
and managing the free procedure in error path on our own.
Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for Marvell
Berlin SoCs")
Signed-off-by: YueHaibing
---
v2: free pctrl->functions instead of function as Jisheng Zhang suggested
v3: v2 I send a wrong patch,this is the correct patch.
---
drivers/pinctrl/berlin/berlin.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c
index d6d183e..b5903ff 100644
--- a/drivers/pinctrl/berlin/berlin.c
+++ b/drivers/pinctrl/berlin/berlin.c
@@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct
platform_device *pdev)
}
/* we will reallocate later */
- pctrl->functions = devm_kcalloc(>dev,
- max_functions,
- sizeof(*pctrl->functions),
- GFP_KERNEL);
+ pctrl->functions = kcalloc(max_functions,
+ sizeof(*pctrl->functions), GFP_KERNEL);
if (!pctrl->functions)
return -ENOMEM;
@@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct
platform_device *pdev)
function++;
}
- if (!found)
+ if (!found) {
+ kfree(pctrl->functions);
return -EINVAL;
+ }
if (!function->groups) {
function->groups =
@@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct
platform_device *pdev)
sizeof(char *),
GFP_KERNEL);
- if (!function->groups)
+ if (!function->groups) {
+ kfree(pctrl->functions);
return -ENOMEM;
+ }
}
groups = function->groups;
--
2.7.0
Re: [PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
Sorry, I send a wrong patch, pls ignore this.
On 2018/8/1 13:02, YueHaibing wrote:
> fixes following Smatch static check warning:
>
> drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state()
> warn: passing devm_ allocated variable to kfree. 'pctrl->functions'
>
> As we will be calling krealloc() on pointer 'pctrl->functions', which means
> kfree() will be called in there, devm_kzalloc() shouldn't be used with
> the allocation in the first place. Fix the warning by calling kcalloc()
> and managing the free procedure in error path on our own.
>
> Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for
> Marvell Berlin SoCs")
> Signed-off-by: YueHaibing
> ---
> v2: free pctrl->functions instead of function as Jisheng Zhang suggested
> ---
> drivers/pinctrl/berlin/berlin.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c
> index d6d183e..b5903ff 100644
> --- a/drivers/pinctrl/berlin/berlin.c
> +++ b/drivers/pinctrl/berlin/berlin.c
> @@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct
> platform_device *pdev)
> }
>
> /* we will reallocate later */
> - pctrl->functions = devm_kcalloc(>dev,
> - max_functions,
> - sizeof(*pctrl->functions),
> - GFP_KERNEL);
> + pctrl->functions = kcalloc(max_functions,
> +sizeof(*pctrl->functions), GFP_KERNEL);
> if (!pctrl->functions)
> return -ENOMEM;
>
> @@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct
> platform_device *pdev)
> function++;
> }
>
> - if (!found)
> + if (!found) {
> + kfree( );
> return -EINVAL;
> + }
>
> if (!function->groups) {
> function->groups =
> @@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct
> platform_device *pdev)
>sizeof(char *),
>GFP_KERNEL);
>
> - if (!function->groups)
> + if (!function->groups) {
> + kfree(pctrl->functions);
> return -ENOMEM;
> + }
> }
>
> groups = function->groups;
>
Re: [PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
Sorry, I send a wrong patch, pls ignore this.
On 2018/8/1 13:02, YueHaibing wrote:
> fixes following Smatch static check warning:
>
> drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state()
> warn: passing devm_ allocated variable to kfree. 'pctrl->functions'
>
> As we will be calling krealloc() on pointer 'pctrl->functions', which means
> kfree() will be called in there, devm_kzalloc() shouldn't be used with
> the allocation in the first place. Fix the warning by calling kcalloc()
> and managing the free procedure in error path on our own.
>
> Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for
> Marvell Berlin SoCs")
> Signed-off-by: YueHaibing
> ---
> v2: free pctrl->functions instead of function as Jisheng Zhang suggested
> ---
> drivers/pinctrl/berlin/berlin.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c
> index d6d183e..b5903ff 100644
> --- a/drivers/pinctrl/berlin/berlin.c
> +++ b/drivers/pinctrl/berlin/berlin.c
> @@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct
> platform_device *pdev)
> }
>
> /* we will reallocate later */
> - pctrl->functions = devm_kcalloc(>dev,
> - max_functions,
> - sizeof(*pctrl->functions),
> - GFP_KERNEL);
> + pctrl->functions = kcalloc(max_functions,
> +sizeof(*pctrl->functions), GFP_KERNEL);
> if (!pctrl->functions)
> return -ENOMEM;
>
> @@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct
> platform_device *pdev)
> function++;
> }
>
> - if (!found)
> + if (!found) {
> + kfree( );
> return -EINVAL;
> + }
>
> if (!function->groups) {
> function->groups =
> @@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct
> platform_device *pdev)
>sizeof(char *),
>GFP_KERNEL);
>
> - if (!function->groups)
> + if (!function->groups) {
> + kfree(pctrl->functions);
> return -ENOMEM;
> + }
> }
>
> groups = function->groups;
>
[PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
fixes following Smatch static check warning:
drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state()
warn: passing devm_ allocated variable to kfree. 'pctrl->functions'
As we will be calling krealloc() on pointer 'pctrl->functions', which means
kfree() will be called in there, devm_kzalloc() shouldn't be used with
the allocation in the first place. Fix the warning by calling kcalloc()
and managing the free procedure in error path on our own.
Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for Marvell
Berlin SoCs")
Signed-off-by: YueHaibing
---
v2: free pctrl->functions instead of function as Jisheng Zhang suggested
---
drivers/pinctrl/berlin/berlin.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c
index d6d183e..b5903ff 100644
--- a/drivers/pinctrl/berlin/berlin.c
+++ b/drivers/pinctrl/berlin/berlin.c
@@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct
platform_device *pdev)
}
/* we will reallocate later */
- pctrl->functions = devm_kcalloc(>dev,
- max_functions,
- sizeof(*pctrl->functions),
- GFP_KERNEL);
+ pctrl->functions = kcalloc(max_functions,
+ sizeof(*pctrl->functions), GFP_KERNEL);
if (!pctrl->functions)
return -ENOMEM;
@@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct
platform_device *pdev)
function++;
}
- if (!found)
+ if (!found) {
+ kfree( );
return -EINVAL;
+ }
if (!function->groups) {
function->groups =
@@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct
platform_device *pdev)
sizeof(char *),
GFP_KERNEL);
- if (!function->groups)
+ if (!function->groups) {
+ kfree(pctrl->functions);
return -ENOMEM;
+ }
}
groups = function->groups;
--
2.7.0
[PATCH v2] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
fixes following Smatch static check warning:
drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state()
warn: passing devm_ allocated variable to kfree. 'pctrl->functions'
As we will be calling krealloc() on pointer 'pctrl->functions', which means
kfree() will be called in there, devm_kzalloc() shouldn't be used with
the allocation in the first place. Fix the warning by calling kcalloc()
and managing the free procedure in error path on our own.
Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for Marvell
Berlin SoCs")
Signed-off-by: YueHaibing
---
v2: free pctrl->functions instead of function as Jisheng Zhang suggested
---
drivers/pinctrl/berlin/berlin.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c
index d6d183e..b5903ff 100644
--- a/drivers/pinctrl/berlin/berlin.c
+++ b/drivers/pinctrl/berlin/berlin.c
@@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct
platform_device *pdev)
}
/* we will reallocate later */
- pctrl->functions = devm_kcalloc(>dev,
- max_functions,
- sizeof(*pctrl->functions),
- GFP_KERNEL);
+ pctrl->functions = kcalloc(max_functions,
+ sizeof(*pctrl->functions), GFP_KERNEL);
if (!pctrl->functions)
return -ENOMEM;
@@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct
platform_device *pdev)
function++;
}
- if (!found)
+ if (!found) {
+ kfree( );
return -EINVAL;
+ }
if (!function->groups) {
function->groups =
@@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct
platform_device *pdev)
sizeof(char *),
GFP_KERNEL);
- if (!function->groups)
+ if (!function->groups) {
+ kfree(pctrl->functions);
return -ENOMEM;
+ }
}
groups = function->groups;
--
2.7.0
Re: How to secure erase PCI-E NVME SSD connected via USB3?
On Tue, Jul 31, 2018 at 7:07 PM, Ming Lei wrote: > On Sun, Jul 29, 2018 at 5:09 PM, Jeff Chua wrote: >> I'm testing the USB3-to-PCI-E NVME SSD. It's works using uas module, >> recognized it as /dev/sda. >> >> Since it's an USB device, the nvme-cli tools won't work, nor does >> hdparm, as it's a NVME SSD. >> >> So, how to secure-erase the NVME SSD connected via the JMS583 chip? > > You may try 'blkdiscard --secure' and see if you are luck. Interesting, will try that. Thanks for the pointer. Jeff.
Re: How to secure erase PCI-E NVME SSD connected via USB3?
On Tue, Jul 31, 2018 at 7:07 PM, Ming Lei wrote: > On Sun, Jul 29, 2018 at 5:09 PM, Jeff Chua wrote: >> I'm testing the USB3-to-PCI-E NVME SSD. It's works using uas module, >> recognized it as /dev/sda. >> >> Since it's an USB device, the nvme-cli tools won't work, nor does >> hdparm, as it's a NVME SSD. >> >> So, how to secure-erase the NVME SSD connected via the JMS583 chip? > > You may try 'blkdiscard --secure' and see if you are luck. Interesting, will try that. Thanks for the pointer. Jeff.
Re: [PATCH] perf vendor events arm64: Update ThunderX2 implementation defined pmu core events
Hi Arnaldo, On Tue, Jul 31, 2018 at 10:59 PM, Arnaldo Carvalho de Melo wrote: > Em Tue, Jul 31, 2018 at 08:40:51PM +0530, Ganapatrao Kulkarni escreveu: >> Hi Arnaldo, >> >> On Tue, Jul 31, 2018 at 7:58 PM, Arnaldo Carvalho de Melo >> wrote: >> > Em Tue, Jul 31, 2018 at 03:32:51PM +0530, Ganapatrao Kulkarni escreveu: >> >> Signed-off-by: Ganapatrao Kulkarni >> > >> > Can you please consider to provide an example of such counters being >> > used, i.e. with a simple C synthetic test that causes these events to >> > take place, then run it via 'perf stat' to show that indeed, they are >> > being programmed and read correctly? >> > >> > Ideally for all of them, but if that becomes too burdensome, for a few >> > of them? >> >> It may be tedious for all, certainly I will provide the test >> results/log for some of them(as many as possible). > > Right, we do try to test some of the events via 'perf test', for > instance: > > [root@jouet perf]# perf test openat > 2: Detect openat syscall event : Ok > 3: Detect openat syscall event on all cpus : Ok > 15: syscalls:sys_enter_openat event fields: Ok > [root@jouet perf]# we have not tried perf test, will look in to this test suite to keep it complaint on our hardware too! > > Things like setting up evsels for some events, then forking + calling a > syscall, then checking if that event appeared on the ring buffer, check > if the payload for the event, as read using the tracefs format fields > matches the parameters we passed in the syscall, etc. > > See tools/perf/tests/openat-syscall-tp-fields.c for that > "syscalls:sys_enter_openat event fields" specific source code. > > So doing some of these synthetic tests when updating the event files may > help us in the direction of having tests that run on those specific > hardwares (ThunderX2 in this case) everytime we run 'perf test', so that > we can detect failures sooner. > > I.e. first write a simple test for one of those events, use it as > documentation, at some point, as time permits, turn those into a 'perf > test' entry. All these events are implemented as per "ARMv8, The Performance Monitors Extension specification" [1]. Brief explanation of each of these events is already captured at tools/perf/pmu-events/arch/arm64/armv8-recommended.json [1] https://static.docs.arm.com/ddi0487/a/DDI0487A_j_armv8_arm.pdf?_ga=2.104377475.2065785066.1533095452-1490247355.1441251141 i have used ltp testcases as workload to test some of the events and log is below, root@SBR-26>ganapat>> perf stat -e unaligned_ld_spec,unaligned_st_spec,unaligned_ldst_spec,mem_access_rd,mem_access_wr,armv8_pmuv3_0/mem_access/ ltp/testcases/kernel/mem/mtest001 -p80 mtest01 0 TINFO : Total memory already used on system = 11849792 kbytes mtest01 0 TINFO : Total memory used needed to reach maximum = 214325040 kbytes mtest01 0 TINFO : Filling up 80% of ram which is 202475248 kbytes mtest01 1 TPASS : 202475248 kbytes allocated only. Performance counter stats for 'ltp/testcases/kernel/mem/mtest01/mtest01 -p80': 2,573 unaligned_ld_spec 3,976 unaligned_st_spec 6,549 unaligned_ldst_spec 1,525,489 mem_access_rd 1,549,531 mem_access_wr 3,075,020 armv8_pmuv3_0/mem_access/ 0.006368837 seconds time elapsed 0.0 seconds user 0.00639 seconds sys root@SBR-26>ganapat>> perf stat -e l1d_cache_refill_rd,l1d_cache_refill_wr,armv8_pmuv3_0/l1d_cache_refill/ ./ltp/testcases/kernel/mem/mtest01/mtest01 -p80 mtest01 0 TINFO : Total memory already used on system = 11851520 kbytes mtest01 0 TINFO : Total memory used needed to reach maximum = 214325040 kbytes mtest01 0 TINFO : Filling up 80% of ram which is 202473520 kbytes mtest01 1 TPASS : 202473520 kbytes allocated only. Performance counter stats for './ltp/testcases/kernel/mem/mtest01/mtest01 -p80': 257,128 l1d_cache_refill_rd 162,151 l1d_cache_refill_wr 419,279 armv8_pmuv3_0/l1d_cache_refill/ 0.006118645 seconds time elapsed 0.0 seconds user 0.006141000 seconds sys root@SBR-26>ganapat>> perf stat -e exc_svc ./ltp/testcases/kernel/syscalls/brk/brk01 tst_test.c:1015: INFO: Timeout per run is 0h 05m 00s brk01.c:67: PASS: brk() works fine Summary: passed 1 failed 0 skipped 0 warnings 0 Performance counter stats for './ltp/testcases/kernel/syscalls/brk/brk01': 100 exc_svc 0.000887222 seconds time elapsed 0.00095 seconds user 0.0 seconds sys root@SBR-26>ganapat>> > > Thanks, > > - Arnaldo > >> > >> > Thanks, >> > >> > - Arnaldo >> > >> >> --- >> >> .../arch/arm64/cavium/thunderx2/core-imp-def.json | 87 >> >> +- >> >> 1 file changed, 84 insertions(+), 3 deletions(-) >> >> >> >> diff --git >> >>
Re: [PATCH] perf vendor events arm64: Update ThunderX2 implementation defined pmu core events
Hi Arnaldo, On Tue, Jul 31, 2018 at 10:59 PM, Arnaldo Carvalho de Melo wrote: > Em Tue, Jul 31, 2018 at 08:40:51PM +0530, Ganapatrao Kulkarni escreveu: >> Hi Arnaldo, >> >> On Tue, Jul 31, 2018 at 7:58 PM, Arnaldo Carvalho de Melo >> wrote: >> > Em Tue, Jul 31, 2018 at 03:32:51PM +0530, Ganapatrao Kulkarni escreveu: >> >> Signed-off-by: Ganapatrao Kulkarni >> > >> > Can you please consider to provide an example of such counters being >> > used, i.e. with a simple C synthetic test that causes these events to >> > take place, then run it via 'perf stat' to show that indeed, they are >> > being programmed and read correctly? >> > >> > Ideally for all of them, but if that becomes too burdensome, for a few >> > of them? >> >> It may be tedious for all, certainly I will provide the test >> results/log for some of them(as many as possible). > > Right, we do try to test some of the events via 'perf test', for > instance: > > [root@jouet perf]# perf test openat > 2: Detect openat syscall event : Ok > 3: Detect openat syscall event on all cpus : Ok > 15: syscalls:sys_enter_openat event fields: Ok > [root@jouet perf]# we have not tried perf test, will look in to this test suite to keep it complaint on our hardware too! > > Things like setting up evsels for some events, then forking + calling a > syscall, then checking if that event appeared on the ring buffer, check > if the payload for the event, as read using the tracefs format fields > matches the parameters we passed in the syscall, etc. > > See tools/perf/tests/openat-syscall-tp-fields.c for that > "syscalls:sys_enter_openat event fields" specific source code. > > So doing some of these synthetic tests when updating the event files may > help us in the direction of having tests that run on those specific > hardwares (ThunderX2 in this case) everytime we run 'perf test', so that > we can detect failures sooner. > > I.e. first write a simple test for one of those events, use it as > documentation, at some point, as time permits, turn those into a 'perf > test' entry. All these events are implemented as per "ARMv8, The Performance Monitors Extension specification" [1]. Brief explanation of each of these events is already captured at tools/perf/pmu-events/arch/arm64/armv8-recommended.json [1] https://static.docs.arm.com/ddi0487/a/DDI0487A_j_armv8_arm.pdf?_ga=2.104377475.2065785066.1533095452-1490247355.1441251141 i have used ltp testcases as workload to test some of the events and log is below, root@SBR-26>ganapat>> perf stat -e unaligned_ld_spec,unaligned_st_spec,unaligned_ldst_spec,mem_access_rd,mem_access_wr,armv8_pmuv3_0/mem_access/ ltp/testcases/kernel/mem/mtest001 -p80 mtest01 0 TINFO : Total memory already used on system = 11849792 kbytes mtest01 0 TINFO : Total memory used needed to reach maximum = 214325040 kbytes mtest01 0 TINFO : Filling up 80% of ram which is 202475248 kbytes mtest01 1 TPASS : 202475248 kbytes allocated only. Performance counter stats for 'ltp/testcases/kernel/mem/mtest01/mtest01 -p80': 2,573 unaligned_ld_spec 3,976 unaligned_st_spec 6,549 unaligned_ldst_spec 1,525,489 mem_access_rd 1,549,531 mem_access_wr 3,075,020 armv8_pmuv3_0/mem_access/ 0.006368837 seconds time elapsed 0.0 seconds user 0.00639 seconds sys root@SBR-26>ganapat>> perf stat -e l1d_cache_refill_rd,l1d_cache_refill_wr,armv8_pmuv3_0/l1d_cache_refill/ ./ltp/testcases/kernel/mem/mtest01/mtest01 -p80 mtest01 0 TINFO : Total memory already used on system = 11851520 kbytes mtest01 0 TINFO : Total memory used needed to reach maximum = 214325040 kbytes mtest01 0 TINFO : Filling up 80% of ram which is 202473520 kbytes mtest01 1 TPASS : 202473520 kbytes allocated only. Performance counter stats for './ltp/testcases/kernel/mem/mtest01/mtest01 -p80': 257,128 l1d_cache_refill_rd 162,151 l1d_cache_refill_wr 419,279 armv8_pmuv3_0/l1d_cache_refill/ 0.006118645 seconds time elapsed 0.0 seconds user 0.006141000 seconds sys root@SBR-26>ganapat>> perf stat -e exc_svc ./ltp/testcases/kernel/syscalls/brk/brk01 tst_test.c:1015: INFO: Timeout per run is 0h 05m 00s brk01.c:67: PASS: brk() works fine Summary: passed 1 failed 0 skipped 0 warnings 0 Performance counter stats for './ltp/testcases/kernel/syscalls/brk/brk01': 100 exc_svc 0.000887222 seconds time elapsed 0.00095 seconds user 0.0 seconds sys root@SBR-26>ganapat>> > > Thanks, > > - Arnaldo > >> > >> > Thanks, >> > >> > - Arnaldo >> > >> >> --- >> >> .../arch/arm64/cavium/thunderx2/core-imp-def.json | 87 >> >> +- >> >> 1 file changed, 84 insertions(+), 3 deletions(-) >> >> >> >> diff --git >> >>
Re: [PATCH v7 5/6] i2c: Add Actions Semiconductor Owl family S900 I2C driver
Hi Wolfram,
On Tue, Jul 31, 2018 at 10:09:32PM +0200, Wolfram Sang wrote:
> Hi Manivannan,
>
> On Thu, Jul 26, 2018 at 09:16:02PM +0530, Manivannan Sadhasivam wrote:
> > Add Actions Semiconductor Owl family S900 I2C driver.
> >
> > Signed-off-by: Manivannan Sadhasivam
> > Acked-by: Peter Rosin
>
> Looks mostly good already. Thanks Peter for the initial review!
>
Thanks to Andy also :)
> > +static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
> > +{
> > + struct owl_i2c_dev *i2c_dev = _dev;
> > + struct i2c_msg *msg = i2c_dev->msg;
> > + unsigned long flags;
> > + unsigned int stat, fifostat;
> > +
> > + spin_lock_irqsave(_dev->lock, flags);
> > +
> > + fifostat = readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT);
> > + if (fifostat & OWL_I2C_FIFOSTAT_RNB) {
> > + dev_dbg(_dev->adap.dev, "received NACK from device\n");
> > + goto stop;
> > + }
> > +
> > + stat = readl(i2c_dev->base + OWL_I2C_REG_STAT);
> > + if (stat & OWL_I2C_STAT_BEB) {
> > + dev_dbg(_dev->adap.dev, "bus error\n");
> > + goto stop;
> > + }
>
> I wonder if you can't pass back the different errors to the upper
> layers? Like -ENXIO for NACK and -EIO for bus error? We have a
> convention for that and it seems your HW can support it. The different
> error codes would then maybe also make the debug outputs obsolete.
>
>
Sure, will catch the errors using an i2c_dev member and pass it to upper
layers in owl_i2c_master_xfer.
> > + /*
> > +* Here, -ENXIO will be returned if interrupt occurred but no
> > +* read or write happened. Else if msg_ptr equals to message length,
> > +* message count will be returned.
> > +*/
> > + ret = i2c_dev->msg_ptr == msg->len ? num : -ENXIO;
>
> I'd think this kinda unusual construct could go then as well by just
> returning the error code derived from the interrupt handler above.
>
Makes sense! It will become:
ret = i2c_dev->err < 0 ? i2c_dev->err : num;
Thanks,
Mani
> > +static const struct i2c_adapter_quirks owl_i2c_quirks = {
> > + .flags = I2C_AQ_COMB | I2C_AQ_COMB_WRITE_FIRST,
> > + .max_read_len = 240,
> > + .max_write_len = 240,
> > + .max_comb_1st_msg_len = 6,
> > + .max_comb_2nd_msg_len = 240,
> > +};
>
> Yay! Good use of the i2c_adapter_quirks struct :)
>
> Regards,
>
>Wolfram
>
Re: [PATCH v7 5/6] i2c: Add Actions Semiconductor Owl family S900 I2C driver
Hi Wolfram,
On Tue, Jul 31, 2018 at 10:09:32PM +0200, Wolfram Sang wrote:
> Hi Manivannan,
>
> On Thu, Jul 26, 2018 at 09:16:02PM +0530, Manivannan Sadhasivam wrote:
> > Add Actions Semiconductor Owl family S900 I2C driver.
> >
> > Signed-off-by: Manivannan Sadhasivam
> > Acked-by: Peter Rosin
>
> Looks mostly good already. Thanks Peter for the initial review!
>
Thanks to Andy also :)
> > +static irqreturn_t owl_i2c_interrupt(int irq, void *_dev)
> > +{
> > + struct owl_i2c_dev *i2c_dev = _dev;
> > + struct i2c_msg *msg = i2c_dev->msg;
> > + unsigned long flags;
> > + unsigned int stat, fifostat;
> > +
> > + spin_lock_irqsave(_dev->lock, flags);
> > +
> > + fifostat = readl(i2c_dev->base + OWL_I2C_REG_FIFOSTAT);
> > + if (fifostat & OWL_I2C_FIFOSTAT_RNB) {
> > + dev_dbg(_dev->adap.dev, "received NACK from device\n");
> > + goto stop;
> > + }
> > +
> > + stat = readl(i2c_dev->base + OWL_I2C_REG_STAT);
> > + if (stat & OWL_I2C_STAT_BEB) {
> > + dev_dbg(_dev->adap.dev, "bus error\n");
> > + goto stop;
> > + }
>
> I wonder if you can't pass back the different errors to the upper
> layers? Like -ENXIO for NACK and -EIO for bus error? We have a
> convention for that and it seems your HW can support it. The different
> error codes would then maybe also make the debug outputs obsolete.
>
>
Sure, will catch the errors using an i2c_dev member and pass it to upper
layers in owl_i2c_master_xfer.
> > + /*
> > +* Here, -ENXIO will be returned if interrupt occurred but no
> > +* read or write happened. Else if msg_ptr equals to message length,
> > +* message count will be returned.
> > +*/
> > + ret = i2c_dev->msg_ptr == msg->len ? num : -ENXIO;
>
> I'd think this kinda unusual construct could go then as well by just
> returning the error code derived from the interrupt handler above.
>
Makes sense! It will become:
ret = i2c_dev->err < 0 ? i2c_dev->err : num;
Thanks,
Mani
> > +static const struct i2c_adapter_quirks owl_i2c_quirks = {
> > + .flags = I2C_AQ_COMB | I2C_AQ_COMB_WRITE_FIRST,
> > + .max_read_len = 240,
> > + .max_write_len = 240,
> > + .max_comb_1st_msg_len = 6,
> > + .max_comb_2nd_msg_len = 240,
> > +};
>
> Yay! Good use of the i2c_adapter_quirks struct :)
>
> Regards,
>
>Wolfram
>
Re: [PATCH v4 4/4] ASoC: qcom: add sdm845 sound card support
On Wed, 01 Aug 2018 05:46:48 +0200,
kbuild test robot wrote:
>
> Hi Rohit,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on asoc/for-next]
> [also build test WARNING on next-20180731]
> [cannot apply to v4.18-rc7]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Rohit-kumar/Add-support-for-audio-on-SDM845-SoC/20180801-082203
> base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
> for-next
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
> >> sound/soc/qcom/sdm845.c:193:27: sparse: incorrect type in argument 2
> >> (different base types) @@expected unsigned int [unsigned] val @@
> >> got restricted snd_unsigned int [unsigned] val @@
>sound/soc/qcom/sdm845.c:193:27:expected unsigned int [unsigned] val
>sound/soc/qcom/sdm845.c:193:27:got restricted snd_pcm_format_t
> [usertype]
>
> vim +193 sound/soc/qcom/sdm845.c
>
>181
>182static int sdm845_be_hw_params_fixup(struct snd_soc_pcm_runtime
> *rtd,
>183struct snd_pcm_hw_params
> *params)
>184{
>185struct snd_interval *rate = hw_param_interval(params,
>186
> SNDRV_PCM_HW_PARAM_RATE);
>187struct snd_interval *channels =
> hw_param_interval(params,
>188
> SNDRV_PCM_HW_PARAM_CHANNELS);
>189struct snd_mask *fmt = hw_param_mask(params,
> SNDRV_PCM_HW_PARAM_FORMAT);
>190
>191rate->min = rate->max = DEFAULT_SAMPLE_RATE_48K;
>192channels->min = channels->max = 2;
> > 193snd_mask_set(fmt, SNDRV_PCM_FORMAT_S16_LE);
FYI, we have introduced a new helper, snd_mask_set_format(), just for
avoiding this sparse warning. It's already in Mark's for-next tree.
thanks,
Takashi
Re: [PATCH v4 4/4] ASoC: qcom: add sdm845 sound card support
On Wed, 01 Aug 2018 05:46:48 +0200,
kbuild test robot wrote:
>
> Hi Rohit,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on asoc/for-next]
> [also build test WARNING on next-20180731]
> [cannot apply to v4.18-rc7]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Rohit-kumar/Add-support-for-audio-on-SDM845-SoC/20180801-082203
> base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
> for-next
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
> >> sound/soc/qcom/sdm845.c:193:27: sparse: incorrect type in argument 2
> >> (different base types) @@expected unsigned int [unsigned] val @@
> >> got restricted snd_unsigned int [unsigned] val @@
>sound/soc/qcom/sdm845.c:193:27:expected unsigned int [unsigned] val
>sound/soc/qcom/sdm845.c:193:27:got restricted snd_pcm_format_t
> [usertype]
>
> vim +193 sound/soc/qcom/sdm845.c
>
>181
>182static int sdm845_be_hw_params_fixup(struct snd_soc_pcm_runtime
> *rtd,
>183struct snd_pcm_hw_params
> *params)
>184{
>185struct snd_interval *rate = hw_param_interval(params,
>186
> SNDRV_PCM_HW_PARAM_RATE);
>187struct snd_interval *channels =
> hw_param_interval(params,
>188
> SNDRV_PCM_HW_PARAM_CHANNELS);
>189struct snd_mask *fmt = hw_param_mask(params,
> SNDRV_PCM_HW_PARAM_FORMAT);
>190
>191rate->min = rate->max = DEFAULT_SAMPLE_RATE_48K;
>192channels->min = channels->max = 2;
> > 193snd_mask_set(fmt, SNDRV_PCM_FORMAT_S16_LE);
FYI, we have introduced a new helper, snd_mask_set_format(), just for
avoiding this sparse warning. It's already in Mark's for-next tree.
thanks,
Takashi
Re: [PATCH v2 1/3] overflow.h: Add arithmetic shift helper
On Tue, Jul 31, 2018 at 7:15 PM, Jason Gunthorpe wrote:
> On Tue, Jul 31, 2018 at 05:00:37PM -0700, Kees Cook wrote:
>> From: Jason Gunthorpe
>>
>> Add shift_overflow() helper to assist driver authors in ensuring that
>> shift operations don't cause overflows or other odd conditions.
>>
>> Signed-off-by: Jason Gunthorpe
>> Signed-off-by: Leon Romanovsky
>> [kees: tweaked comments and commit log, dropped unneeded assignment]
>> Signed-off-by: Kees Cook
>> ---
>> include/linux/overflow.h | 31 +++
>> 1 file changed, 31 insertions(+)
>>
>> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
>> index 8712ff70995f..69fc366ce865 100644
>> --- a/include/linux/overflow.h
>> +++ b/include/linux/overflow.h
>> @@ -202,6 +202,37 @@
>>
>> #endif /* COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW */
>>
>> +/** check_shift_overflow() - Calculate a left-shifted value and check
>> overflow
>> + *
>> + * @a: Value to be shifted
>> + * @b: How many bits left to shift
>
> The above @b should be @s
Ooops, copy-paste-o. :)
>
>> +#define check_shift_overflow(a, s, d) ({ \
>
> Should I run this series through the rdma tree?
I'd like to get Rasmus's Ack, but otherwise, yes, that'd be fine.
-Kees
--
Kees Cook
Pixel Security
Re: [PATCH v2 1/3] overflow.h: Add arithmetic shift helper
On Tue, Jul 31, 2018 at 7:15 PM, Jason Gunthorpe wrote:
> On Tue, Jul 31, 2018 at 05:00:37PM -0700, Kees Cook wrote:
>> From: Jason Gunthorpe
>>
>> Add shift_overflow() helper to assist driver authors in ensuring that
>> shift operations don't cause overflows or other odd conditions.
>>
>> Signed-off-by: Jason Gunthorpe
>> Signed-off-by: Leon Romanovsky
>> [kees: tweaked comments and commit log, dropped unneeded assignment]
>> Signed-off-by: Kees Cook
>> ---
>> include/linux/overflow.h | 31 +++
>> 1 file changed, 31 insertions(+)
>>
>> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
>> index 8712ff70995f..69fc366ce865 100644
>> --- a/include/linux/overflow.h
>> +++ b/include/linux/overflow.h
>> @@ -202,6 +202,37 @@
>>
>> #endif /* COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW */
>>
>> +/** check_shift_overflow() - Calculate a left-shifted value and check
>> overflow
>> + *
>> + * @a: Value to be shifted
>> + * @b: How many bits left to shift
>
> The above @b should be @s
Ooops, copy-paste-o. :)
>
>> +#define check_shift_overflow(a, s, d) ({ \
>
> Should I run this series through the rdma tree?
I'd like to get Rasmus's Ack, but otherwise, yes, that'd be fine.
-Kees
--
Kees Cook
Pixel Security
[PATCH] clk: meson-axg: pcie: drop the mpll3 clock parent
We found the PCIe driver doesn't really work with
the mpll3 clock which is actually reserved for debug,
So drop it from the mux list.
Fixes: 33b89db68236 ("clk: meson-axg: add clocks required by pcie driver")
Tested-by: Jianxin Qin
Signed-off-by: Yixun Lan
---
hi Jerome:
I'm sorry we found this during latest PCIe driver test.
I'm fine with either pull this as a fixup for 4.18 or
queued for next 4.19, since the PCIe driver is not merged yet,
just do as you feel what's fit best, thanks.
Yixun
---
drivers/clk/meson/axg.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c
index 2d458092884a..246c23df64a8 100644
--- a/drivers/clk/meson/axg.c
+++ b/drivers/clk/meson/axg.c
@@ -700,12 +700,14 @@ static struct clk_regmap axg_pcie_mux = {
.offset = HHI_PCIE_PLL_CNTL6,
.mask = 0x1,
.shift = 2,
+ /* skip the parent mpll3, reserved for debug */
+ .table = (u32[]){ 1 },
},
.hw.init = &(struct clk_init_data){
.name = "pcie_mux",
.ops = _regmap_mux_ops,
- .parent_names = (const char *[]){ "mpll3", "pcie_pll" },
- .num_parents = 2,
+ .parent_names = (const char *[]){ "pcie_pll" },
+ .num_parents = 1,
.flags = CLK_SET_RATE_PARENT,
},
};
--
2.18.0
[PATCH] clk: meson-axg: pcie: drop the mpll3 clock parent
We found the PCIe driver doesn't really work with
the mpll3 clock which is actually reserved for debug,
So drop it from the mux list.
Fixes: 33b89db68236 ("clk: meson-axg: add clocks required by pcie driver")
Tested-by: Jianxin Qin
Signed-off-by: Yixun Lan
---
hi Jerome:
I'm sorry we found this during latest PCIe driver test.
I'm fine with either pull this as a fixup for 4.18 or
queued for next 4.19, since the PCIe driver is not merged yet,
just do as you feel what's fit best, thanks.
Yixun
---
drivers/clk/meson/axg.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c
index 2d458092884a..246c23df64a8 100644
--- a/drivers/clk/meson/axg.c
+++ b/drivers/clk/meson/axg.c
@@ -700,12 +700,14 @@ static struct clk_regmap axg_pcie_mux = {
.offset = HHI_PCIE_PLL_CNTL6,
.mask = 0x1,
.shift = 2,
+ /* skip the parent mpll3, reserved for debug */
+ .table = (u32[]){ 1 },
},
.hw.init = &(struct clk_init_data){
.name = "pcie_mux",
.ops = _regmap_mux_ops,
- .parent_names = (const char *[]){ "mpll3", "pcie_pll" },
- .num_parents = 2,
+ .parent_names = (const char *[]){ "pcie_pll" },
+ .num_parents = 1,
.flags = CLK_SET_RATE_PARENT,
},
};
--
2.18.0
Re: [PATCH] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
On 2018/8/1 10:36, Jisheng Zhang wrote:
> Hi,
>
> On Tue, 31 Jul 2018 22:25:01 +0800 YueHaibing wrote:
>
>> fixes following Smatch static check warning:
>>
>> drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state()
>> warn: passing devm_ allocated variable to kfree. 'pctrl->functions'
>>
>> As we will be calling krealloc() on pointer 'pctrl->functions', which means
>> kfree() will be called in there, devm_kzalloc() shouldn't be used with
>> the allocation in the first place. Fix the warning by calling kcalloc()
>> and managing the free procedure in error path on our own.
>
> Good catch. Comments below.
>
>>
>> Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for
>> Marvell Berlin SoCs")
>> Signed-off-by: YueHaibing
>> ---
>> drivers/pinctrl/berlin/berlin.c | 14 --
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/pinctrl/berlin/berlin.c
>> b/drivers/pinctrl/berlin/berlin.c
>> index d6d183e..db2afb2 100644
>> --- a/drivers/pinctrl/berlin/berlin.c
>> +++ b/drivers/pinctrl/berlin/berlin.c
>> @@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct
>> platform_device *pdev)
>> }
>>
>> /* we will reallocate later */
>> -pctrl->functions = devm_kcalloc(>dev,
>> -max_functions,
>> -sizeof(*pctrl->functions),
>> -GFP_KERNEL);
>> +pctrl->functions = kcalloc(max_functions,
>> + sizeof(*pctrl->functions), GFP_KERNEL);
>> if (!pctrl->functions)
>> return -ENOMEM;
>>
>> @@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct
>> platform_device *pdev)
>> function++;
>> }
>>
>> -if (!found)
>> +if (!found) {
>> +kfree(function);
>
> is it enough to just free one function? I think we need to free functions.
Yep, should free 'pctrl->functions', Thanks!
Will send v2.
>
>> return -EINVAL;
>> +}
>>
>> if (!function->groups) {
>> function->groups =
>> @@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct
>> platform_device *pdev)
>> sizeof(char *),
>> GFP_KERNEL);
>>
>> -if (!function->groups)
>> +if (!function->groups) {
>> +kfree(function);
>
> ditto
>
>> return -ENOMEM;
>> +}
>> }
>>
>> groups = function->groups;
>
>
>
Re: [PATCH] pinctrl: berlin: fix 'pctrl->functions' allocation in berlin_pinctrl_build_state
On 2018/8/1 10:36, Jisheng Zhang wrote:
> Hi,
>
> On Tue, 31 Jul 2018 22:25:01 +0800 YueHaibing wrote:
>
>> fixes following Smatch static check warning:
>>
>> drivers/pinctrl/berlin/berlin.c:237 berlin_pinctrl_build_state()
>> warn: passing devm_ allocated variable to kfree. 'pctrl->functions'
>>
>> As we will be calling krealloc() on pointer 'pctrl->functions', which means
>> kfree() will be called in there, devm_kzalloc() shouldn't be used with
>> the allocation in the first place. Fix the warning by calling kcalloc()
>> and managing the free procedure in error path on our own.
>
> Good catch. Comments below.
>
>>
>> Fixes: 3de68d331c24 ("pinctrl: berlin: add the core pinctrl driver for
>> Marvell Berlin SoCs")
>> Signed-off-by: YueHaibing
>> ---
>> drivers/pinctrl/berlin/berlin.c | 14 --
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/pinctrl/berlin/berlin.c
>> b/drivers/pinctrl/berlin/berlin.c
>> index d6d183e..db2afb2 100644
>> --- a/drivers/pinctrl/berlin/berlin.c
>> +++ b/drivers/pinctrl/berlin/berlin.c
>> @@ -216,10 +216,8 @@ static int berlin_pinctrl_build_state(struct
>> platform_device *pdev)
>> }
>>
>> /* we will reallocate later */
>> -pctrl->functions = devm_kcalloc(>dev,
>> -max_functions,
>> -sizeof(*pctrl->functions),
>> -GFP_KERNEL);
>> +pctrl->functions = kcalloc(max_functions,
>> + sizeof(*pctrl->functions), GFP_KERNEL);
>> if (!pctrl->functions)
>> return -ENOMEM;
>>
>> @@ -257,8 +255,10 @@ static int berlin_pinctrl_build_state(struct
>> platform_device *pdev)
>> function++;
>> }
>>
>> -if (!found)
>> +if (!found) {
>> +kfree(function);
>
> is it enough to just free one function? I think we need to free functions.
Yep, should free 'pctrl->functions', Thanks!
Will send v2.
>
>> return -EINVAL;
>> +}
>>
>> if (!function->groups) {
>> function->groups =
>> @@ -267,8 +267,10 @@ static int berlin_pinctrl_build_state(struct
>> platform_device *pdev)
>> sizeof(char *),
>> GFP_KERNEL);
>>
>> -if (!function->groups)
>> +if (!function->groups) {
>> +kfree(function);
>
> ditto
>
>> return -ENOMEM;
>> +}
>> }
>>
>> groups = function->groups;
>
>
>
Re: [alsa-devel] [PATCH] ASoC: soc-pcm: Use delay set in pointer function
On 7/31/2018 8:10 PM, Mark Brown wrote: > On Tue, Jul 31, 2018 at 03:56:52PM +0200, Takashi Iwai wrote: >> Mark Brown wrote: > >>> Yes. I'm saying that if the CPU DAI thinks it can figure out the base >>> delay something is confused. > >> Then basically Akshu's patch does the correct thing, I suppose. > > I think so. Could perhaps do with a little clarification though. > Ok Mark, I will submit a v2 which makes it more clear on the intent. Thanks, Akshu
Re: [alsa-devel] [PATCH] ASoC: soc-pcm: Use delay set in pointer function
On 7/31/2018 8:10 PM, Mark Brown wrote: > On Tue, Jul 31, 2018 at 03:56:52PM +0200, Takashi Iwai wrote: >> Mark Brown wrote: > >>> Yes. I'm saying that if the CPU DAI thinks it can figure out the base >>> delay something is confused. > >> Then basically Akshu's patch does the correct thing, I suppose. > > I think so. Could perhaps do with a little clarification though. > Ok Mark, I will submit a v2 which makes it more clear on the intent. Thanks, Akshu
Re: [PATCH v4 4/4] ASoC: qcom: add sdm845 sound card support
Hi Rohit,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on asoc/for-next]
[also build test WARNING on next-20180731]
[cannot apply to v4.18-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Rohit-kumar/Add-support-for-audio-on-SDM845-SoC/20180801-082203
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
for-next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> sound/soc/qcom/sdm845.c:193:27: sparse: incorrect type in argument 2
>> (different base types) @@expected unsigned int [unsigned] val @@got
>> restricted snd_unsigned int [unsigned] val @@
sound/soc/qcom/sdm845.c:193:27:expected unsigned int [unsigned] val
sound/soc/qcom/sdm845.c:193:27:got restricted snd_pcm_format_t
[usertype]
vim +193 sound/soc/qcom/sdm845.c
181
182 static int sdm845_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
183 struct snd_pcm_hw_params *params)
184 {
185 struct snd_interval *rate = hw_param_interval(params,
186 SNDRV_PCM_HW_PARAM_RATE);
187 struct snd_interval *channels = hw_param_interval(params,
188 SNDRV_PCM_HW_PARAM_CHANNELS);
189 struct snd_mask *fmt = hw_param_mask(params,
SNDRV_PCM_HW_PARAM_FORMAT);
190
191 rate->min = rate->max = DEFAULT_SAMPLE_RATE_48K;
192 channels->min = channels->max = 2;
> 193 snd_mask_set(fmt, SNDRV_PCM_FORMAT_S16_LE);
194
195 return 0;
196 }
197
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH v4 4/4] ASoC: qcom: add sdm845 sound card support
Hi Rohit,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on asoc/for-next]
[also build test WARNING on next-20180731]
[cannot apply to v4.18-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Rohit-kumar/Add-support-for-audio-on-SDM845-SoC/20180801-082203
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
for-next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> sound/soc/qcom/sdm845.c:193:27: sparse: incorrect type in argument 2
>> (different base types) @@expected unsigned int [unsigned] val @@got
>> restricted snd_unsigned int [unsigned] val @@
sound/soc/qcom/sdm845.c:193:27:expected unsigned int [unsigned] val
sound/soc/qcom/sdm845.c:193:27:got restricted snd_pcm_format_t
[usertype]
vim +193 sound/soc/qcom/sdm845.c
181
182 static int sdm845_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
183 struct snd_pcm_hw_params *params)
184 {
185 struct snd_interval *rate = hw_param_interval(params,
186 SNDRV_PCM_HW_PARAM_RATE);
187 struct snd_interval *channels = hw_param_interval(params,
188 SNDRV_PCM_HW_PARAM_CHANNELS);
189 struct snd_mask *fmt = hw_param_mask(params,
SNDRV_PCM_HW_PARAM_FORMAT);
190
191 rate->min = rate->max = DEFAULT_SAMPLE_RATE_48K;
192 channels->min = channels->max = 2;
> 193 snd_mask_set(fmt, SNDRV_PCM_FORMAT_S16_LE);
194
195 return 0;
196 }
197
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[PATCH v2 09/10] reset: Add Actions Semi S700 SoC Reset Management Unit support
Add Reset Management Unit (RMU) support for Actions Semi S700 SoC
of the Owl family series. RMU belongs to the Owl SoCs system-controller
which also includes CMU (Clock Management Unit).
Signed-off-by: Manivannan Sadhasivam
---
drivers/reset/reset-owl.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/drivers/reset/reset-owl.c b/drivers/reset/reset-owl.c
index c4f07691fb36..2e761c64f81b 100644
--- a/drivers/reset/reset-owl.c
+++ b/drivers/reset/reset-owl.c
@@ -12,6 +12,7 @@
#include
#include
+#include
#define CMU_DEVRST0 0x00a8
#define CMU_DEVRST1 0x00ac
@@ -84,11 +85,42 @@ static const struct owl_reset_map s900_resets[] = {
[S900_RESET_I2C3] = { CMU_DEVRST1, BIT(19) },
};
+static const struct owl_reset_map s700_resets[] = {
+ [S700_RESET_DE] = { CMU_DEVRST0, BIT(0) },
+ [S700_RESET_LCD0]= { CMU_DEVRST0, BIT(1) },
+ [S700_RESET_DSI] = { CMU_DEVRST0, BIT(2) },
+ [S700_RESET_CSI] = { CMU_DEVRST0, BIT(13) },
+ [S700_RESET_SI] = { CMU_DEVRST0, BIT(14) },
+ [S700_RESET_I2C0]= { CMU_DEVRST1, BIT(0) },
+ [S700_RESET_I2C1]= { CMU_DEVRST1, BIT(1) },
+ [S700_RESET_I2C2]= { CMU_DEVRST1, BIT(2) },
+ [S700_RESET_I2C3]= { CMU_DEVRST1, BIT(3) },
+ [S700_RESET_SPI0]= { CMU_DEVRST1, BIT(4) },
+ [S700_RESET_SPI1]= { CMU_DEVRST1, BIT(5) },
+ [S700_RESET_SPI2]= { CMU_DEVRST1, BIT(6) },
+ [S700_RESET_SPI3]= { CMU_DEVRST1, BIT(7) },
+ [S700_RESET_UART0] = { CMU_DEVRST1, BIT(8) },
+ [S700_RESET_UART1] = { CMU_DEVRST1, BIT(9) },
+ [S700_RESET_UART2] = { CMU_DEVRST1, BIT(10) },
+ [S700_RESET_UART3] = { CMU_DEVRST1, BIT(11) },
+ [S700_RESET_UART4] = { CMU_DEVRST1, BIT(12) },
+ [S700_RESET_UART5] = { CMU_DEVRST1, BIT(13) },
+ [S700_RESET_UART6] = { CMU_DEVRST1, BIT(14) },
+ [S700_RESET_KEY] = { CMU_DEVRST1, BIT(24) },
+ [S700_RESET_GPIO]= { CMU_DEVRST1, BIT(25) },
+ [S700_RESET_AUDIO] = { CMU_DEVRST1, BIT(29) },
+};
+
static const struct owl_reset_hw s900_reset_hw = {
.resets = s900_resets,
.num_resets = ARRAY_SIZE(s900_resets),
};
+static const struct owl_reset_hw s700_reset_hw = {
+ .resets = s700_resets,
+ .num_resets = ARRAY_SIZE(s700_resets),
+};
+
static inline struct owl_reset *to_owl_reset(struct reset_controller_dev
*rcdev)
{
return container_of(rcdev, struct owl_reset, rcdev);
@@ -179,6 +211,7 @@ static int owl_reset_probe(struct platform_device *pdev)
static const struct of_device_id owl_reset_of_match[] = {
{ .compatible = "actions,s900-rmu", .data = _reset_hw },
+ { .compatible = "actions,s700-rmu", .data = _reset_hw },
{ /* sentinel */ }
};
--
2.17.1
[PATCH v2 09/10] reset: Add Actions Semi S700 SoC Reset Management Unit support
Add Reset Management Unit (RMU) support for Actions Semi S700 SoC
of the Owl family series. RMU belongs to the Owl SoCs system-controller
which also includes CMU (Clock Management Unit).
Signed-off-by: Manivannan Sadhasivam
---
drivers/reset/reset-owl.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/drivers/reset/reset-owl.c b/drivers/reset/reset-owl.c
index c4f07691fb36..2e761c64f81b 100644
--- a/drivers/reset/reset-owl.c
+++ b/drivers/reset/reset-owl.c
@@ -12,6 +12,7 @@
#include
#include
+#include
#define CMU_DEVRST0 0x00a8
#define CMU_DEVRST1 0x00ac
@@ -84,11 +85,42 @@ static const struct owl_reset_map s900_resets[] = {
[S900_RESET_I2C3] = { CMU_DEVRST1, BIT(19) },
};
+static const struct owl_reset_map s700_resets[] = {
+ [S700_RESET_DE] = { CMU_DEVRST0, BIT(0) },
+ [S700_RESET_LCD0]= { CMU_DEVRST0, BIT(1) },
+ [S700_RESET_DSI] = { CMU_DEVRST0, BIT(2) },
+ [S700_RESET_CSI] = { CMU_DEVRST0, BIT(13) },
+ [S700_RESET_SI] = { CMU_DEVRST0, BIT(14) },
+ [S700_RESET_I2C0]= { CMU_DEVRST1, BIT(0) },
+ [S700_RESET_I2C1]= { CMU_DEVRST1, BIT(1) },
+ [S700_RESET_I2C2]= { CMU_DEVRST1, BIT(2) },
+ [S700_RESET_I2C3]= { CMU_DEVRST1, BIT(3) },
+ [S700_RESET_SPI0]= { CMU_DEVRST1, BIT(4) },
+ [S700_RESET_SPI1]= { CMU_DEVRST1, BIT(5) },
+ [S700_RESET_SPI2]= { CMU_DEVRST1, BIT(6) },
+ [S700_RESET_SPI3]= { CMU_DEVRST1, BIT(7) },
+ [S700_RESET_UART0] = { CMU_DEVRST1, BIT(8) },
+ [S700_RESET_UART1] = { CMU_DEVRST1, BIT(9) },
+ [S700_RESET_UART2] = { CMU_DEVRST1, BIT(10) },
+ [S700_RESET_UART3] = { CMU_DEVRST1, BIT(11) },
+ [S700_RESET_UART4] = { CMU_DEVRST1, BIT(12) },
+ [S700_RESET_UART5] = { CMU_DEVRST1, BIT(13) },
+ [S700_RESET_UART6] = { CMU_DEVRST1, BIT(14) },
+ [S700_RESET_KEY] = { CMU_DEVRST1, BIT(24) },
+ [S700_RESET_GPIO]= { CMU_DEVRST1, BIT(25) },
+ [S700_RESET_AUDIO] = { CMU_DEVRST1, BIT(29) },
+};
+
static const struct owl_reset_hw s900_reset_hw = {
.resets = s900_resets,
.num_resets = ARRAY_SIZE(s900_resets),
};
+static const struct owl_reset_hw s700_reset_hw = {
+ .resets = s700_resets,
+ .num_resets = ARRAY_SIZE(s700_resets),
+};
+
static inline struct owl_reset *to_owl_reset(struct reset_controller_dev
*rcdev)
{
return container_of(rcdev, struct owl_reset, rcdev);
@@ -179,6 +211,7 @@ static int owl_reset_probe(struct platform_device *pdev)
static const struct of_device_id owl_reset_of_match[] = {
{ .compatible = "actions,s900-rmu", .data = _reset_hw },
+ { .compatible = "actions,s700-rmu", .data = _reset_hw },
{ /* sentinel */ }
};
--
2.17.1
[PATCH v2 10/10] MAINTAINERS: Add entry for Actions Semi Owl SoCs Reset Management Unit
Add entry for Actions Semi Reset Management Unit driver and its bindings under ARCH_ACTIONS. Currently only S700 and S900 SoCs of the Owl family are supported. Signed-off-by: Manivannan Sadhasivam --- MAINTAINERS | 4 1 file changed, 4 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9c292ef3c210..25934ae77ba6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1146,13 +1146,17 @@ F: arch/arm64/boot/dts/actions/ F: drivers/clk/actions/ F: drivers/clocksource/owl-* F: drivers/pinctrl/actions/* +F: drivers/reset/reset-owl.c F: drivers/soc/actions/ F: include/dt-bindings/power/owl-* +F: include/dt-bindings/reset/actions,s700-rmu.h +F: include/dt-bindings/reset/actions,s900-rmu.h F: include/linux/soc/actions/ F: Documentation/devicetree/bindings/arm/actions.txt F: Documentation/devicetree/bindings/clock/actions,s900-cmu.txt F: Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt F: Documentation/devicetree/bindings/power/actions,owl-sps.txt +F: Documentation/devicetree/bindings/reset/actions,owl-reset.txt F: Documentation/devicetree/bindings/timer/actions,owl-timer.txt ARM/ADS SPHERE MACHINE SUPPORT -- 2.17.1
[PATCH v2 10/10] MAINTAINERS: Add entry for Actions Semi Owl SoCs Reset Management Unit
Add entry for Actions Semi Reset Management Unit driver and its bindings under ARCH_ACTIONS. Currently only S700 and S900 SoCs of the Owl family are supported. Signed-off-by: Manivannan Sadhasivam --- MAINTAINERS | 4 1 file changed, 4 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9c292ef3c210..25934ae77ba6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1146,13 +1146,17 @@ F: arch/arm64/boot/dts/actions/ F: drivers/clk/actions/ F: drivers/clocksource/owl-* F: drivers/pinctrl/actions/* +F: drivers/reset/reset-owl.c F: drivers/soc/actions/ F: include/dt-bindings/power/owl-* +F: include/dt-bindings/reset/actions,s700-rmu.h +F: include/dt-bindings/reset/actions,s900-rmu.h F: include/linux/soc/actions/ F: Documentation/devicetree/bindings/arm/actions.txt F: Documentation/devicetree/bindings/clock/actions,s900-cmu.txt F: Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt F: Documentation/devicetree/bindings/power/actions,owl-sps.txt +F: Documentation/devicetree/bindings/reset/actions,owl-reset.txt F: Documentation/devicetree/bindings/timer/actions,owl-timer.txt ARM/ADS SPHERE MACHINE SUPPORT -- 2.17.1
[PATCH v2 08/10] reset: Add Actions Semi S900 SoC Reset Management Unit support
Add Reset Management Unit (RMU) support for Actions Semi S900 SoC
of the Owl family series. RMU belongs to the Owl SoCs system-controller
which also includes CMU (Clock Management Unit).
Signed-off-by: Manivannan Sadhasivam
---
drivers/reset/Kconfig | 6 ++
drivers/reset/Makefile| 1 +
drivers/reset/reset-owl.c | 192 ++
3 files changed, 199 insertions(+)
create mode 100644 drivers/reset/reset-owl.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index c0b292be1b72..90627430569b 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -73,6 +73,12 @@ config RESET_MESON
help
This enables the reset driver for Amlogic Meson SoCs.
+config RESET_OWL
+ bool "Actions Semi Owl SoCs Reset Driver" if COMPILE_TEST
+ default ARCH_ACTIONS
+ help
+ This enables the reset controller driver for Actions Semi Owl SoCs.
+
config RESET_OXNAS
bool
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index c1261dcfe9ad..fa655319cf17 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_RESET_IMX7) += reset-imx7.o
obj-$(CONFIG_RESET_LANTIQ) += reset-lantiq.o
obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
obj-$(CONFIG_RESET_MESON) += reset-meson.o
+obj-$(CONFIG_RESET_OWL) += reset-owl.o
obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
diff --git a/drivers/reset/reset-owl.c b/drivers/reset/reset-owl.c
new file mode 100644
index ..c4f07691fb36
--- /dev/null
+++ b/drivers/reset/reset-owl.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// Actions Semi Owl SoCs Reset Management Unit driver
+//
+// Copyright (c) 2018 Linaro Ltd.
+// Author: Manivannan Sadhasivam
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#define CMU_DEVRST0 0x00a8
+#define CMU_DEVRST1 0x00ac
+
+struct owl_reset_map {
+ u32 reg;
+ u32 bit;
+};
+
+struct owl_reset_hw {
+ const struct owl_reset_map *resets;
+ u32 num_resets;
+};
+
+struct owl_reset {
+ struct reset_controller_dev rcdev;
+ const struct owl_reset_hw *hw;
+ struct regmap *regmap;
+};
+
+static const struct owl_reset_map s900_resets[] = {
+ [S900_RESET_DMAC] = { CMU_DEVRST0, BIT(0) },
+ [S900_RESET_SRAMI] = { CMU_DEVRST0, BIT(1) },
+ [S900_RESET_DDR_CTL_PHY]= { CMU_DEVRST0, BIT(2) },
+ [S900_RESET_NANDC0] = { CMU_DEVRST0, BIT(3) },
+ [S900_RESET_SD0]= { CMU_DEVRST0, BIT(4) },
+ [S900_RESET_SD1]= { CMU_DEVRST0, BIT(5) },
+ [S900_RESET_PCM1] = { CMU_DEVRST0, BIT(6) },
+ [S900_RESET_DE] = { CMU_DEVRST0, BIT(7) },
+ [S900_RESET_LVDS] = { CMU_DEVRST0, BIT(8) },
+ [S900_RESET_SD2]= { CMU_DEVRST0, BIT(9) },
+ [S900_RESET_DSI]= { CMU_DEVRST0, BIT(10) },
+ [S900_RESET_CSI0] = { CMU_DEVRST0, BIT(11) },
+ [S900_RESET_BISP_AXI] = { CMU_DEVRST0, BIT(12) },
+ [S900_RESET_CSI1] = { CMU_DEVRST0, BIT(13) },
+ [S900_RESET_GPIO] = { CMU_DEVRST0, BIT(15) },
+ [S900_RESET_EDP]= { CMU_DEVRST0, BIT(16) },
+ [S900_RESET_AUDIO] = { CMU_DEVRST0, BIT(17) },
+ [S900_RESET_PCM0] = { CMU_DEVRST0, BIT(18) },
+ [S900_RESET_HDE]= { CMU_DEVRST0, BIT(21) },
+ [S900_RESET_GPU3D_PA] = { CMU_DEVRST0, BIT(22) },
+ [S900_RESET_IMX]= { CMU_DEVRST0, BIT(23) },
+ [S900_RESET_SE] = { CMU_DEVRST0, BIT(24) },
+ [S900_RESET_NANDC1] = { CMU_DEVRST0, BIT(25) },
+ [S900_RESET_SD3]= { CMU_DEVRST0, BIT(26) },
+ [S900_RESET_GIC]= { CMU_DEVRST0, BIT(27) },
+ [S900_RESET_GPU3D_PB] = { CMU_DEVRST0, BIT(28) },
+ [S900_RESET_DDR_CTL_PHY_AXI]= { CMU_DEVRST0, BIT(29) },
+ [S900_RESET_CMU_DDR]= { CMU_DEVRST0, BIT(30) },
+ [S900_RESET_DMM]= { CMU_DEVRST0, BIT(31) },
+ [S900_RESET_USB2HUB]= { CMU_DEVRST1, BIT(0) },
+ [S900_RESET_USB2HSIC] = { CMU_DEVRST1, BIT(1) },
+ [S900_RESET_HDMI] = { CMU_DEVRST1, BIT(2) },
+ [S900_RESET_HDCP2TX]= { CMU_DEVRST1, BIT(3) },
+ [S900_RESET_UART6] = { CMU_DEVRST1, BIT(4) },
+ [S900_RESET_UART0] = { CMU_DEVRST1, BIT(5) },
+ [S900_RESET_UART1] = { CMU_DEVRST1, BIT(6) },
+ [S900_RESET_UART2] = { CMU_DEVRST1, BIT(7) },
+ [S900_RESET_SPI0] = { CMU_DEVRST1, BIT(8) },
+ [S900_RESET_SPI1] = {
[PATCH v2 08/10] reset: Add Actions Semi S900 SoC Reset Management Unit support
Add Reset Management Unit (RMU) support for Actions Semi S900 SoC
of the Owl family series. RMU belongs to the Owl SoCs system-controller
which also includes CMU (Clock Management Unit).
Signed-off-by: Manivannan Sadhasivam
---
drivers/reset/Kconfig | 6 ++
drivers/reset/Makefile| 1 +
drivers/reset/reset-owl.c | 192 ++
3 files changed, 199 insertions(+)
create mode 100644 drivers/reset/reset-owl.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index c0b292be1b72..90627430569b 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -73,6 +73,12 @@ config RESET_MESON
help
This enables the reset driver for Amlogic Meson SoCs.
+config RESET_OWL
+ bool "Actions Semi Owl SoCs Reset Driver" if COMPILE_TEST
+ default ARCH_ACTIONS
+ help
+ This enables the reset controller driver for Actions Semi Owl SoCs.
+
config RESET_OXNAS
bool
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index c1261dcfe9ad..fa655319cf17 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_RESET_IMX7) += reset-imx7.o
obj-$(CONFIG_RESET_LANTIQ) += reset-lantiq.o
obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
obj-$(CONFIG_RESET_MESON) += reset-meson.o
+obj-$(CONFIG_RESET_OWL) += reset-owl.o
obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
diff --git a/drivers/reset/reset-owl.c b/drivers/reset/reset-owl.c
new file mode 100644
index ..c4f07691fb36
--- /dev/null
+++ b/drivers/reset/reset-owl.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// Actions Semi Owl SoCs Reset Management Unit driver
+//
+// Copyright (c) 2018 Linaro Ltd.
+// Author: Manivannan Sadhasivam
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#define CMU_DEVRST0 0x00a8
+#define CMU_DEVRST1 0x00ac
+
+struct owl_reset_map {
+ u32 reg;
+ u32 bit;
+};
+
+struct owl_reset_hw {
+ const struct owl_reset_map *resets;
+ u32 num_resets;
+};
+
+struct owl_reset {
+ struct reset_controller_dev rcdev;
+ const struct owl_reset_hw *hw;
+ struct regmap *regmap;
+};
+
+static const struct owl_reset_map s900_resets[] = {
+ [S900_RESET_DMAC] = { CMU_DEVRST0, BIT(0) },
+ [S900_RESET_SRAMI] = { CMU_DEVRST0, BIT(1) },
+ [S900_RESET_DDR_CTL_PHY]= { CMU_DEVRST0, BIT(2) },
+ [S900_RESET_NANDC0] = { CMU_DEVRST0, BIT(3) },
+ [S900_RESET_SD0]= { CMU_DEVRST0, BIT(4) },
+ [S900_RESET_SD1]= { CMU_DEVRST0, BIT(5) },
+ [S900_RESET_PCM1] = { CMU_DEVRST0, BIT(6) },
+ [S900_RESET_DE] = { CMU_DEVRST0, BIT(7) },
+ [S900_RESET_LVDS] = { CMU_DEVRST0, BIT(8) },
+ [S900_RESET_SD2]= { CMU_DEVRST0, BIT(9) },
+ [S900_RESET_DSI]= { CMU_DEVRST0, BIT(10) },
+ [S900_RESET_CSI0] = { CMU_DEVRST0, BIT(11) },
+ [S900_RESET_BISP_AXI] = { CMU_DEVRST0, BIT(12) },
+ [S900_RESET_CSI1] = { CMU_DEVRST0, BIT(13) },
+ [S900_RESET_GPIO] = { CMU_DEVRST0, BIT(15) },
+ [S900_RESET_EDP]= { CMU_DEVRST0, BIT(16) },
+ [S900_RESET_AUDIO] = { CMU_DEVRST0, BIT(17) },
+ [S900_RESET_PCM0] = { CMU_DEVRST0, BIT(18) },
+ [S900_RESET_HDE]= { CMU_DEVRST0, BIT(21) },
+ [S900_RESET_GPU3D_PA] = { CMU_DEVRST0, BIT(22) },
+ [S900_RESET_IMX]= { CMU_DEVRST0, BIT(23) },
+ [S900_RESET_SE] = { CMU_DEVRST0, BIT(24) },
+ [S900_RESET_NANDC1] = { CMU_DEVRST0, BIT(25) },
+ [S900_RESET_SD3]= { CMU_DEVRST0, BIT(26) },
+ [S900_RESET_GIC]= { CMU_DEVRST0, BIT(27) },
+ [S900_RESET_GPU3D_PB] = { CMU_DEVRST0, BIT(28) },
+ [S900_RESET_DDR_CTL_PHY_AXI]= { CMU_DEVRST0, BIT(29) },
+ [S900_RESET_CMU_DDR]= { CMU_DEVRST0, BIT(30) },
+ [S900_RESET_DMM]= { CMU_DEVRST0, BIT(31) },
+ [S900_RESET_USB2HUB]= { CMU_DEVRST1, BIT(0) },
+ [S900_RESET_USB2HSIC] = { CMU_DEVRST1, BIT(1) },
+ [S900_RESET_HDMI] = { CMU_DEVRST1, BIT(2) },
+ [S900_RESET_HDCP2TX]= { CMU_DEVRST1, BIT(3) },
+ [S900_RESET_UART6] = { CMU_DEVRST1, BIT(4) },
+ [S900_RESET_UART0] = { CMU_DEVRST1, BIT(5) },
+ [S900_RESET_UART1] = { CMU_DEVRST1, BIT(6) },
+ [S900_RESET_UART2] = { CMU_DEVRST1, BIT(7) },
+ [S900_RESET_SPI0] = { CMU_DEVRST1, BIT(8) },
+ [S900_RESET_SPI1] = {
[PATCH v2 06/10] arm64: dts: actions: Add RMU node for Actions Semi S900 SoC
Add Reset Controller Unit (RMU) node under the system-controller node
for Actions Semi S900 SoC. Also add the bindings constant header to
be used by clients.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm64/boot/dts/actions/s900.dtsi | 6 ++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/actions/s900.dtsi
b/arch/arm64/boot/dts/actions/s900.dtsi
index d239033f9599..a242d3193a6a 100644
--- a/arch/arm64/boot/dts/actions/s900.dtsi
+++ b/arch/arm64/boot/dts/actions/s900.dtsi
@@ -6,6 +6,7 @@
#include
#include
+#include
/ {
compatible = "actions,s900";
@@ -176,6 +177,11 @@
clocks = <>, <>;
#clock-cells = <1>;
};
+
+ rmu: reset-controller {
+ compatible = "actions,s900-rmu";
+ #reset-cells = <1>;
+ };
};
pinctrl: pinctrl@e01b {
--
2.17.1
[PATCH v2 05/10] dt-bindings: reset: Add Actions Semi S700 SoC RMU support
Add RMU (Reset Management Unit) support for the Actions Semi S700 SoC which is a part of the Actions Semi Owl family series. Signed-off-by: Manivannan Sadhasivam --- .../bindings/reset/actions,owl-reset.txt | 8 +++-- include/dt-bindings/reset/actions,s700-rmu.h | 34 +++ 2 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/reset/actions,s700-rmu.h diff --git a/Documentation/devicetree/bindings/reset/actions,owl-reset.txt b/Documentation/devicetree/bindings/reset/actions,owl-reset.txt index 38e2c7051d86..a29950cb2db0 100644 --- a/Documentation/devicetree/bindings/reset/actions,owl-reset.txt +++ b/Documentation/devicetree/bindings/reset/actions,owl-reset.txt @@ -7,12 +7,14 @@ controller binding usage. The RMU registers are part of the system-controller block on Owl SoCs. Required properties: -- compatible: Should be "actions,s900-rmu" +- compatible: Should be one of the following, + "actions,s900-rmu" + "actions,s700-rmu" - #reset-cells: Should be 1 All available resets are defined as preprocessor macros in corresponding -dt-bindings/reset/actions,s900-rmu.h header and can be used in device -tree sources. +dt-bindings/reset/actions,s900-rmu.h or actions,s700-rmu.h header and can +be used in device tree sources. Parent node should have the following properties : - compatible: "syscon", "simple-mfd" diff --git a/include/dt-bindings/reset/actions,s700-rmu.h b/include/dt-bindings/reset/actions,s700-rmu.h new file mode 100644 index ..8c5d4d1b8bd4 --- /dev/null +++ b/include/dt-bindings/reset/actions,s700-rmu.h @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: (GPL-2.0-or-later OR MIT) +// +// Device Tree binding constants for Actions Semi S700 Reset Management Unit +// +// Copyright (c) 2018 Linaro Ltd. + +#ifndef _DT_BINDINGS_ACTIONS_S700_RESET_H +#define _DT_BINDINGS_ACTIONS_S700_RESET_H + +#define S700_RESET_AUDIO 0 +#define S700_RESET_CSI 1 +#define S700_RESET_DE 2 +#define S700_RESET_DSI 3 +#define S700_RESET_GPIO4 +#define S700_RESET_I2C05 +#define S700_RESET_I2C16 +#define S700_RESET_I2C27 +#define S700_RESET_I2C38 +#define S700_RESET_KEY 9 +#define S700_RESET_LCD010 +#define S700_RESET_SI 11 +#define S700_RESET_SPI012 +#define S700_RESET_SPI113 +#define S700_RESET_SPI214 +#define S700_RESET_SPI315 +#define S700_RESET_UART0 16 +#define S700_RESET_UART1 17 +#define S700_RESET_UART2 18 +#define S700_RESET_UART3 19 +#define S700_RESET_UART4 20 +#define S700_RESET_UART5 21 +#define S700_RESET_UART6 22 + +#endif /* _DT_BINDINGS_ACTIONS_S700_RESET_H */ -- 2.17.1
[PATCH v2 07/10] arm64: dts: actions: Add RMU node for Actions Semi S700 SoC
Add Reset Controller Unit (RMU) node under the system-controller node
for Actions Semi S700 SoC. Also add the bindings constant header to
be used by clients.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm64/boot/dts/actions/s700.dtsi | 6 ++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/actions/s700.dtsi
b/arch/arm64/boot/dts/actions/s700.dtsi
index a57f54587164..8498579504c7 100644
--- a/arch/arm64/boot/dts/actions/s700.dtsi
+++ b/arch/arm64/boot/dts/actions/s700.dtsi
@@ -5,6 +5,7 @@
#include
#include
+#include
/ {
compatible = "actions,s700";
@@ -169,6 +170,11 @@
clocks = <>, <>;
#clock-cells = <1>;
};
+
+ rmu: reset-controller {
+ compatible = "actions,s700-rmu";
+ #reset-cells = <1>;
+ };
};
sps: power-controller@e01b0100 {
--
2.17.1
[PATCH v2 07/10] arm64: dts: actions: Add RMU node for Actions Semi S700 SoC
Add Reset Controller Unit (RMU) node under the system-controller node
for Actions Semi S700 SoC. Also add the bindings constant header to
be used by clients.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm64/boot/dts/actions/s700.dtsi | 6 ++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/actions/s700.dtsi
b/arch/arm64/boot/dts/actions/s700.dtsi
index a57f54587164..8498579504c7 100644
--- a/arch/arm64/boot/dts/actions/s700.dtsi
+++ b/arch/arm64/boot/dts/actions/s700.dtsi
@@ -5,6 +5,7 @@
#include
#include
+#include
/ {
compatible = "actions,s700";
@@ -169,6 +170,11 @@
clocks = <>, <>;
#clock-cells = <1>;
};
+
+ rmu: reset-controller {
+ compatible = "actions,s700-rmu";
+ #reset-cells = <1>;
+ };
};
sps: power-controller@e01b0100 {
--
2.17.1
[PATCH v2 06/10] arm64: dts: actions: Add RMU node for Actions Semi S900 SoC
Add Reset Controller Unit (RMU) node under the system-controller node
for Actions Semi S900 SoC. Also add the bindings constant header to
be used by clients.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm64/boot/dts/actions/s900.dtsi | 6 ++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/actions/s900.dtsi
b/arch/arm64/boot/dts/actions/s900.dtsi
index d239033f9599..a242d3193a6a 100644
--- a/arch/arm64/boot/dts/actions/s900.dtsi
+++ b/arch/arm64/boot/dts/actions/s900.dtsi
@@ -6,6 +6,7 @@
#include
#include
+#include
/ {
compatible = "actions,s900";
@@ -176,6 +177,11 @@
clocks = <>, <>;
#clock-cells = <1>;
};
+
+ rmu: reset-controller {
+ compatible = "actions,s900-rmu";
+ #reset-cells = <1>;
+ };
};
pinctrl: pinctrl@e01b {
--
2.17.1
[PATCH v2 05/10] dt-bindings: reset: Add Actions Semi S700 SoC RMU support
Add RMU (Reset Management Unit) support for the Actions Semi S700 SoC which is a part of the Actions Semi Owl family series. Signed-off-by: Manivannan Sadhasivam --- .../bindings/reset/actions,owl-reset.txt | 8 +++-- include/dt-bindings/reset/actions,s700-rmu.h | 34 +++ 2 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/reset/actions,s700-rmu.h diff --git a/Documentation/devicetree/bindings/reset/actions,owl-reset.txt b/Documentation/devicetree/bindings/reset/actions,owl-reset.txt index 38e2c7051d86..a29950cb2db0 100644 --- a/Documentation/devicetree/bindings/reset/actions,owl-reset.txt +++ b/Documentation/devicetree/bindings/reset/actions,owl-reset.txt @@ -7,12 +7,14 @@ controller binding usage. The RMU registers are part of the system-controller block on Owl SoCs. Required properties: -- compatible: Should be "actions,s900-rmu" +- compatible: Should be one of the following, + "actions,s900-rmu" + "actions,s700-rmu" - #reset-cells: Should be 1 All available resets are defined as preprocessor macros in corresponding -dt-bindings/reset/actions,s900-rmu.h header and can be used in device -tree sources. +dt-bindings/reset/actions,s900-rmu.h or actions,s700-rmu.h header and can +be used in device tree sources. Parent node should have the following properties : - compatible: "syscon", "simple-mfd" diff --git a/include/dt-bindings/reset/actions,s700-rmu.h b/include/dt-bindings/reset/actions,s700-rmu.h new file mode 100644 index ..8c5d4d1b8bd4 --- /dev/null +++ b/include/dt-bindings/reset/actions,s700-rmu.h @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: (GPL-2.0-or-later OR MIT) +// +// Device Tree binding constants for Actions Semi S700 Reset Management Unit +// +// Copyright (c) 2018 Linaro Ltd. + +#ifndef _DT_BINDINGS_ACTIONS_S700_RESET_H +#define _DT_BINDINGS_ACTIONS_S700_RESET_H + +#define S700_RESET_AUDIO 0 +#define S700_RESET_CSI 1 +#define S700_RESET_DE 2 +#define S700_RESET_DSI 3 +#define S700_RESET_GPIO4 +#define S700_RESET_I2C05 +#define S700_RESET_I2C16 +#define S700_RESET_I2C27 +#define S700_RESET_I2C38 +#define S700_RESET_KEY 9 +#define S700_RESET_LCD010 +#define S700_RESET_SI 11 +#define S700_RESET_SPI012 +#define S700_RESET_SPI113 +#define S700_RESET_SPI214 +#define S700_RESET_SPI315 +#define S700_RESET_UART0 16 +#define S700_RESET_UART1 17 +#define S700_RESET_UART2 18 +#define S700_RESET_UART3 19 +#define S700_RESET_UART4 20 +#define S700_RESET_UART5 21 +#define S700_RESET_UART6 22 + +#endif /* _DT_BINDINGS_ACTIONS_S700_RESET_H */ -- 2.17.1
[PATCH v2 04/10] dt-bindings: reset: Add Actions Semi S900 SoC RMU support
Add RMU (Reset Management Unit) support for the Actions Semi S900
SoC which is a part of the Actions Semi Owl family series.
Signed-off-by: Manivannan Sadhasivam
---
.../bindings/reset/actions,owl-reset.txt | 33 ++
include/dt-bindings/reset/actions,s900-rmu.h | 65 +++
2 files changed, 98 insertions(+)
create mode 100644
Documentation/devicetree/bindings/reset/actions,owl-reset.txt
create mode 100644 include/dt-bindings/reset/actions,s900-rmu.h
diff --git a/Documentation/devicetree/bindings/reset/actions,owl-reset.txt
b/Documentation/devicetree/bindings/reset/actions,owl-reset.txt
new file mode 100644
index ..38e2c7051d86
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/actions,owl-reset.txt
@@ -0,0 +1,33 @@
+Actions Semi Owl SoCs Reset Management Unit (RMU)
+=
+
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+The RMU registers are part of the system-controller block on Owl SoCs.
+
+Required properties:
+- compatible: Should be "actions,s900-rmu"
+- #reset-cells: Should be 1
+
+All available resets are defined as preprocessor macros in corresponding
+dt-bindings/reset/actions,s900-rmu.h header and can be used in device
+tree sources.
+
+Parent node should have the following properties :
+- compatible: "syscon", "simple-mfd"
+- reg: physical base address of the system controller and length of
+ memory mapped region.
+
+Example:
+
+sysctrl: system-controller@e016 {
+compatible = "syscon", "simple-mfd";
+reg = <0x0 0xe016 0x0 0x1000>;
+
+rmu: reset-controller {
+compatible = "actions,s900-rmu";
+#reset-cells = <1>;
+};
+};
+
diff --git a/include/dt-bindings/reset/actions,s900-rmu.h
b/include/dt-bindings/reset/actions,s900-rmu.h
new file mode 100644
index ..09e6dca46936
--- /dev/null
+++ b/include/dt-bindings/reset/actions,s900-rmu.h
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: (GPL-2.0-or-later OR MIT)
+//
+// Device Tree binding constants for Actions Semi S900 Reset Management Unit
+//
+// Copyright (c) 2018 Linaro Ltd.
+
+#ifndef _DT_BINDINGS_ACTIONS_S900_RESET_H
+#define _DT_BINDINGS_ACTIONS_S900_RESET_H
+
+#define S900_RESET_CHIPID 0
+#define S900_RESET_CPU_SCNT1
+#define S900_RESET_SRAMI 2
+#define S900_RESET_DDR_CTL_PHY 3
+#define S900_RESET_DMAC4
+#define S900_RESET_GPIO5
+#define S900_RESET_BISP_AXI6
+#define S900_RESET_CSI07
+#define S900_RESET_CSI18
+#define S900_RESET_DE 9
+#define S900_RESET_DSI 10
+#define S900_RESET_GPU3D_PA11
+#define S900_RESET_GPU3D_PB12
+#define S900_RESET_HDE 13
+#define S900_RESET_I2C014
+#define S900_RESET_I2C115
+#define S900_RESET_I2C216
+#define S900_RESET_I2C317
+#define S900_RESET_I2C418
+#define S900_RESET_I2C519
+#define S900_RESET_IMX 20
+#define S900_RESET_NANDC0 21
+#define S900_RESET_NANDC1 22
+#define S900_RESET_SD0 23
+#define S900_RESET_SD1 24
+#define S900_RESET_SD2 25
+#define S900_RESET_SD3 26
+#define S900_RESET_SPI027
+#define S900_RESET_SPI128
+#define S900_RESET_SPI229
+#define S900_RESET_SPI330
+#define S900_RESET_UART0 31
+#define S900_RESET_UART1 32
+#define S900_RESET_UART2 33
+#define S900_RESET_UART3 34
+#define S900_RESET_UART4 35
+#define S900_RESET_UART5 36
+#define S900_RESET_UART6 37
+#define S900_RESET_HDMI38
+#define S900_RESET_LVDS39
+#define S900_RESET_EDP 40
+#define S900_RESET_USB2HUB 41
+#define S900_RESET_USB2HSIC42
+#define S900_RESET_USB343
+#define S900_RESET_PCM144
+#define S900_RESET_AUDIO 45
+#define S900_RESET_PCM046
+#define S900_RESET_SE 47
+#define S900_RESET_GIC 48
+#define S900_RESET_DDR_CTL_PHY_AXI 49
+#define S900_RESET_CMU_DDR 50
+#define S900_RESET_DMM 51
+#define S900_RESET_HDCP2TX 52
+#define S900_RESET_ETHERNET53
+
+#endif /* _DT_BINDINGS_ACTIONS_S900_RESET_H */
--
2.17.1
[PATCH v2 04/10] dt-bindings: reset: Add Actions Semi S900 SoC RMU support
Add RMU (Reset Management Unit) support for the Actions Semi S900
SoC which is a part of the Actions Semi Owl family series.
Signed-off-by: Manivannan Sadhasivam
---
.../bindings/reset/actions,owl-reset.txt | 33 ++
include/dt-bindings/reset/actions,s900-rmu.h | 65 +++
2 files changed, 98 insertions(+)
create mode 100644
Documentation/devicetree/bindings/reset/actions,owl-reset.txt
create mode 100644 include/dt-bindings/reset/actions,s900-rmu.h
diff --git a/Documentation/devicetree/bindings/reset/actions,owl-reset.txt
b/Documentation/devicetree/bindings/reset/actions,owl-reset.txt
new file mode 100644
index ..38e2c7051d86
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/actions,owl-reset.txt
@@ -0,0 +1,33 @@
+Actions Semi Owl SoCs Reset Management Unit (RMU)
+=
+
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+The RMU registers are part of the system-controller block on Owl SoCs.
+
+Required properties:
+- compatible: Should be "actions,s900-rmu"
+- #reset-cells: Should be 1
+
+All available resets are defined as preprocessor macros in corresponding
+dt-bindings/reset/actions,s900-rmu.h header and can be used in device
+tree sources.
+
+Parent node should have the following properties :
+- compatible: "syscon", "simple-mfd"
+- reg: physical base address of the system controller and length of
+ memory mapped region.
+
+Example:
+
+sysctrl: system-controller@e016 {
+compatible = "syscon", "simple-mfd";
+reg = <0x0 0xe016 0x0 0x1000>;
+
+rmu: reset-controller {
+compatible = "actions,s900-rmu";
+#reset-cells = <1>;
+};
+};
+
diff --git a/include/dt-bindings/reset/actions,s900-rmu.h
b/include/dt-bindings/reset/actions,s900-rmu.h
new file mode 100644
index ..09e6dca46936
--- /dev/null
+++ b/include/dt-bindings/reset/actions,s900-rmu.h
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: (GPL-2.0-or-later OR MIT)
+//
+// Device Tree binding constants for Actions Semi S900 Reset Management Unit
+//
+// Copyright (c) 2018 Linaro Ltd.
+
+#ifndef _DT_BINDINGS_ACTIONS_S900_RESET_H
+#define _DT_BINDINGS_ACTIONS_S900_RESET_H
+
+#define S900_RESET_CHIPID 0
+#define S900_RESET_CPU_SCNT1
+#define S900_RESET_SRAMI 2
+#define S900_RESET_DDR_CTL_PHY 3
+#define S900_RESET_DMAC4
+#define S900_RESET_GPIO5
+#define S900_RESET_BISP_AXI6
+#define S900_RESET_CSI07
+#define S900_RESET_CSI18
+#define S900_RESET_DE 9
+#define S900_RESET_DSI 10
+#define S900_RESET_GPU3D_PA11
+#define S900_RESET_GPU3D_PB12
+#define S900_RESET_HDE 13
+#define S900_RESET_I2C014
+#define S900_RESET_I2C115
+#define S900_RESET_I2C216
+#define S900_RESET_I2C317
+#define S900_RESET_I2C418
+#define S900_RESET_I2C519
+#define S900_RESET_IMX 20
+#define S900_RESET_NANDC0 21
+#define S900_RESET_NANDC1 22
+#define S900_RESET_SD0 23
+#define S900_RESET_SD1 24
+#define S900_RESET_SD2 25
+#define S900_RESET_SD3 26
+#define S900_RESET_SPI027
+#define S900_RESET_SPI128
+#define S900_RESET_SPI229
+#define S900_RESET_SPI330
+#define S900_RESET_UART0 31
+#define S900_RESET_UART1 32
+#define S900_RESET_UART2 33
+#define S900_RESET_UART3 34
+#define S900_RESET_UART4 35
+#define S900_RESET_UART5 36
+#define S900_RESET_UART6 37
+#define S900_RESET_HDMI38
+#define S900_RESET_LVDS39
+#define S900_RESET_EDP 40
+#define S900_RESET_USB2HUB 41
+#define S900_RESET_USB2HSIC42
+#define S900_RESET_USB343
+#define S900_RESET_PCM144
+#define S900_RESET_AUDIO 45
+#define S900_RESET_PCM046
+#define S900_RESET_SE 47
+#define S900_RESET_GIC 48
+#define S900_RESET_DDR_CTL_PHY_AXI 49
+#define S900_RESET_CMU_DDR 50
+#define S900_RESET_DMM 51
+#define S900_RESET_HDCP2TX 52
+#define S900_RESET_ETHERNET53
+
+#endif /* _DT_BINDINGS_ACTIONS_S900_RESET_H */
--
2.17.1
[PATCH v2 03/10] clk: actions: Add syscon support for Actions Semi Owl SoCs
Since the clock and reset management units are sharing the same memory
map, convert the Owl common clock driver to support System Controller so
that the reset driver can reuse the same memory region.
Signed-off-by: Manivannan Sadhasivam
---
drivers/clk/actions/owl-common.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/clk/actions/owl-common.c b/drivers/clk/actions/owl-common.c
index 61c1071b5180..080f980b2ec4 100644
--- a/drivers/clk/actions/owl-common.c
+++ b/drivers/clk/actions/owl-common.c
@@ -8,6 +8,7 @@
// Copyright (c) 2018 Linaro Ltd.
// Author: Manivannan Sadhasivam
+#include
#include
#include
#include
@@ -15,14 +16,6 @@
#include "owl-common.h"
-static const struct regmap_config owl_regmap_config = {
- .reg_bits = 32,
- .reg_stride = 4,
- .val_bits = 32,
- .max_register = 0x00cc,
- .fast_io= true,
-};
-
static void owl_clk_set_regmap(const struct owl_clk_desc *desc,
struct regmap *regmap)
{
@@ -41,18 +34,11 @@ static void owl_clk_set_regmap(const struct owl_clk_desc
*desc,
int owl_clk_regmap_init(struct platform_device *pdev,
const struct owl_clk_desc *desc)
{
- void __iomem *base;
struct regmap *regmap;
- struct resource *res;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(>dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
- regmap = devm_regmap_init_mmio(>dev, base, _regmap_config);
+ regmap = syscon_node_to_regmap(of_get_parent(pdev->dev.of_node));
if (IS_ERR(regmap)) {
- pr_err("failed to init regmap\n");
+ dev_err(>dev, "failed to get regmap\n");
return PTR_ERR(regmap);
}
--
2.17.1
[PATCH v2 03/10] clk: actions: Add syscon support for Actions Semi Owl SoCs
Since the clock and reset management units are sharing the same memory
map, convert the Owl common clock driver to support System Controller so
that the reset driver can reuse the same memory region.
Signed-off-by: Manivannan Sadhasivam
---
drivers/clk/actions/owl-common.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/clk/actions/owl-common.c b/drivers/clk/actions/owl-common.c
index 61c1071b5180..080f980b2ec4 100644
--- a/drivers/clk/actions/owl-common.c
+++ b/drivers/clk/actions/owl-common.c
@@ -8,6 +8,7 @@
// Copyright (c) 2018 Linaro Ltd.
// Author: Manivannan Sadhasivam
+#include
#include
#include
#include
@@ -15,14 +16,6 @@
#include "owl-common.h"
-static const struct regmap_config owl_regmap_config = {
- .reg_bits = 32,
- .reg_stride = 4,
- .val_bits = 32,
- .max_register = 0x00cc,
- .fast_io= true,
-};
-
static void owl_clk_set_regmap(const struct owl_clk_desc *desc,
struct regmap *regmap)
{
@@ -41,18 +34,11 @@ static void owl_clk_set_regmap(const struct owl_clk_desc
*desc,
int owl_clk_regmap_init(struct platform_device *pdev,
const struct owl_clk_desc *desc)
{
- void __iomem *base;
struct regmap *regmap;
- struct resource *res;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(>dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
- regmap = devm_regmap_init_mmio(>dev, base, _regmap_config);
+ regmap = syscon_node_to_regmap(of_get_parent(pdev->dev.of_node));
if (IS_ERR(regmap)) {
- pr_err("failed to init regmap\n");
+ dev_err(>dev, "failed to get regmap\n");
return PTR_ERR(regmap);
}
--
2.17.1
[PATCH v2 01/10] dt-bindings: clock: Add syscon support to Actions Semi Owl SoCs
Since the clock and reset management units are sharing the same memory
map, document the clock bindings to support System Controller.
Signed-off-by: Manivannan Sadhasivam
---
.../bindings/clock/actions,owl-cmu.txt| 21 +--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
b/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
index d1e60d297387..649c95fc4582 100644
--- a/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
+++ b/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
@@ -9,8 +9,6 @@ Required Properties:
- compatible: should be one of the following,
"actions,s900-cmu"
"actions,s700-cmu"
-- reg: physical base address of the controller and length of memory mapped
- region.
- clocks: Reference to the parent clocks ("hosc", "losc")
- #clock-cells: should be 1.
@@ -21,6 +19,13 @@ All available clocks are defined as preprocessor macros in
corresponding
dt-bindings/clock/actions,s900-cmu.h or actions,s700-cmu.h header and can be
used in device tree sources.
+The CMU registers are part of the system-controller block on Owl SoCs.
+
+Parent node should have the following properties :
+- compatible: "syscon", "simple-mfd"
+- reg: physical base address of the controller and length of memory mapped
+ region.
+
External clocks:
The hosc clock used as input for the plls is generated outside the SoC. It is
@@ -31,11 +36,15 @@ Actions Semi S900 CMU also requires one more clock:
Example: Clock Management Unit node:
-cmu: clock-controller@e016 {
-compatible = "actions,s900-cmu";
+sysctrl: system-controller@e016 {
+compatible = "syscon", "simple-mfd";
reg = <0x0 0xe016 0x0 0x1000>;
-clocks = <>, <>;
-#clock-cells = <1>;
+
+cmu: clock-controller {
+compatible = "actions,s900-cmu";
+clocks = <>, <>;
+#clock-cells = <1>;
+};
};
Example: UART controller node that consumes clock generated by the clock
--
2.17.1
[PATCH v2 02/10] arm64: dts: actions: Convert Owl SoCs clock-controller nodes to syscon
Since clock and reset management units are sharing the same memory map,
Owl SoCs clock-controller nodes needs to be converted to syscon so that
the corresponding reset drivers can also reuse the same memory region.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm64/boot/dts/actions/s700.dtsi | 12
arch/arm64/boot/dts/actions/s900.dtsi | 12
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/actions/s700.dtsi
b/arch/arm64/boot/dts/actions/s700.dtsi
index 59d29e4ca404..a57f54587164 100644
--- a/arch/arm64/boot/dts/actions/s700.dtsi
+++ b/arch/arm64/boot/dts/actions/s700.dtsi
@@ -160,11 +160,15 @@
status = "disabled";
};
- cmu: clock-controller@e0168000 {
- compatible = "actions,s700-cmu";
+ sysctrl: system-controller@e0168000 {
+ compatible = "syscon", "simple-mfd";
reg = <0x0 0xe0168000 0x0 0x1000>;
- clocks = <>, <>;
- #clock-cells = <1>;
+
+ cmu: clock-controller {
+ compatible = "actions,s700-cmu";
+ clocks = <>, <>;
+ #clock-cells = <1>;
+ };
};
sps: power-controller@e01b0100 {
diff --git a/arch/arm64/boot/dts/actions/s900.dtsi
b/arch/arm64/boot/dts/actions/s900.dtsi
index aa3a49b0d646..d239033f9599 100644
--- a/arch/arm64/boot/dts/actions/s900.dtsi
+++ b/arch/arm64/boot/dts/actions/s900.dtsi
@@ -167,11 +167,15 @@
status = "disabled";
};
- cmu: clock-controller@e016 {
- compatible = "actions,s900-cmu";
+ sysctrl: system-controller@e016 {
+ compatible = "syscon", "simple-mfd";
reg = <0x0 0xe016 0x0 0x1000>;
- clocks = <>, <>;
- #clock-cells = <1>;
+
+ cmu: clock-controller {
+ compatible = "actions,s900-cmu";
+ clocks = <>, <>;
+ #clock-cells = <1>;
+ };
};
pinctrl: pinctrl@e01b {
--
2.17.1
[PATCH v2 01/10] dt-bindings: clock: Add syscon support to Actions Semi Owl SoCs
Since the clock and reset management units are sharing the same memory
map, document the clock bindings to support System Controller.
Signed-off-by: Manivannan Sadhasivam
---
.../bindings/clock/actions,owl-cmu.txt| 21 +--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
b/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
index d1e60d297387..649c95fc4582 100644
--- a/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
+++ b/Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
@@ -9,8 +9,6 @@ Required Properties:
- compatible: should be one of the following,
"actions,s900-cmu"
"actions,s700-cmu"
-- reg: physical base address of the controller and length of memory mapped
- region.
- clocks: Reference to the parent clocks ("hosc", "losc")
- #clock-cells: should be 1.
@@ -21,6 +19,13 @@ All available clocks are defined as preprocessor macros in
corresponding
dt-bindings/clock/actions,s900-cmu.h or actions,s700-cmu.h header and can be
used in device tree sources.
+The CMU registers are part of the system-controller block on Owl SoCs.
+
+Parent node should have the following properties :
+- compatible: "syscon", "simple-mfd"
+- reg: physical base address of the controller and length of memory mapped
+ region.
+
External clocks:
The hosc clock used as input for the plls is generated outside the SoC. It is
@@ -31,11 +36,15 @@ Actions Semi S900 CMU also requires one more clock:
Example: Clock Management Unit node:
-cmu: clock-controller@e016 {
-compatible = "actions,s900-cmu";
+sysctrl: system-controller@e016 {
+compatible = "syscon", "simple-mfd";
reg = <0x0 0xe016 0x0 0x1000>;
-clocks = <>, <>;
-#clock-cells = <1>;
+
+cmu: clock-controller {
+compatible = "actions,s900-cmu";
+clocks = <>, <>;
+#clock-cells = <1>;
+};
};
Example: UART controller node that consumes clock generated by the clock
--
2.17.1
[PATCH v2 02/10] arm64: dts: actions: Convert Owl SoCs clock-controller nodes to syscon
Since clock and reset management units are sharing the same memory map,
Owl SoCs clock-controller nodes needs to be converted to syscon so that
the corresponding reset drivers can also reuse the same memory region.
Signed-off-by: Manivannan Sadhasivam
---
arch/arm64/boot/dts/actions/s700.dtsi | 12
arch/arm64/boot/dts/actions/s900.dtsi | 12
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/actions/s700.dtsi
b/arch/arm64/boot/dts/actions/s700.dtsi
index 59d29e4ca404..a57f54587164 100644
--- a/arch/arm64/boot/dts/actions/s700.dtsi
+++ b/arch/arm64/boot/dts/actions/s700.dtsi
@@ -160,11 +160,15 @@
status = "disabled";
};
- cmu: clock-controller@e0168000 {
- compatible = "actions,s700-cmu";
+ sysctrl: system-controller@e0168000 {
+ compatible = "syscon", "simple-mfd";
reg = <0x0 0xe0168000 0x0 0x1000>;
- clocks = <>, <>;
- #clock-cells = <1>;
+
+ cmu: clock-controller {
+ compatible = "actions,s700-cmu";
+ clocks = <>, <>;
+ #clock-cells = <1>;
+ };
};
sps: power-controller@e01b0100 {
diff --git a/arch/arm64/boot/dts/actions/s900.dtsi
b/arch/arm64/boot/dts/actions/s900.dtsi
index aa3a49b0d646..d239033f9599 100644
--- a/arch/arm64/boot/dts/actions/s900.dtsi
+++ b/arch/arm64/boot/dts/actions/s900.dtsi
@@ -167,11 +167,15 @@
status = "disabled";
};
- cmu: clock-controller@e016 {
- compatible = "actions,s900-cmu";
+ sysctrl: system-controller@e016 {
+ compatible = "syscon", "simple-mfd";
reg = <0x0 0xe016 0x0 0x1000>;
- clocks = <>, <>;
- #clock-cells = <1>;
+
+ cmu: clock-controller {
+ compatible = "actions,s900-cmu";
+ clocks = <>, <>;
+ #clock-cells = <1>;
+ };
};
pinctrl: pinctrl@e01b {
--
2.17.1
[PATCH v2 00/10] Add Reset Controller support for Actions Semi Owl SoCs
This patchset adds Reset Controller (RMU) support for Actions Semi Owl SoCs, S900 and S700. For the Owl SoCs, RMU's registers has been integrated into the CMU memory map in hardware. Hence, in this patchset the CMU driver has been converted to syscon so that the same memory map can be resued by both CMU and RMU drivers. Finally, the support for RMU in S700 and S900 SoCs are added. This patch series depends on the recently posted S700 clk series: "[PATCH v7 0/5] Add clock driver for Actions S700 SoC". For the S700 clk series, driver and bindings patches are applied through the clk tree. But the DTS patches are not yet picked up by the platform maintainer, Andreas. Hence, Andreas is expected to pick the DTS patches in this series once reviewed by the maintainers along with S700 clk DTS patches. Note: I have only tested the S900 part, S700 is only compile tested. But there is no reason for it to fail. Thanks, Mani Changes in v2: * Converted the CMU and RMU drivers to syscon for a more cleaner approach * Declared the owl_reset_map structs to const * Used regmap_update_bits instead of a combined regmap_read and write * Removed unused headers in RMU drivers * Added MAINTAINERS entry for the RMU driver and bindings Manivannan Sadhasivam (10): dt-bindings: clock: Add syscon support to Actions Semi Owl SoCs arm64: dts: actions: Convert Owl SoCs clock-controller nodes to syscon clk: actions: Add syscon support for Actions Semi Owl SoCs dt-bindings: reset: Add Actions Semi S900 SoC RMU support dt-bindings: reset: Add Actions Semi S700 SoC RMU support arm64: dts: actions: Add RMU node for Actions Semi S900 SoC arm64: dts: actions: Add RMU node for Actions Semi S700 SoC reset: Add Actions Semi S900 SoC Reset Management Unit support reset: Add Actions Semi S700 SoC Reset Management Unit support MAINTAINERS: Add entry for Actions Semi Owl SoCs Reset Management Unit .../bindings/clock/actions,owl-cmu.txt| 21 +- .../bindings/reset/actions,owl-reset.txt | 35 +++ MAINTAINERS | 4 + arch/arm64/boot/dts/actions/s700.dtsi | 18 +- arch/arm64/boot/dts/actions/s900.dtsi | 18 +- drivers/clk/actions/owl-common.c | 20 +- drivers/reset/Kconfig | 6 + drivers/reset/Makefile| 1 + drivers/reset/reset-owl.c | 225 ++ include/dt-bindings/reset/actions,s700-rmu.h | 34 +++ include/dt-bindings/reset/actions,s900-rmu.h | 65 + 11 files changed, 416 insertions(+), 31 deletions(-) create mode 100644 Documentation/devicetree/bindings/reset/actions,owl-reset.txt create mode 100644 drivers/reset/reset-owl.c create mode 100644 include/dt-bindings/reset/actions,s700-rmu.h create mode 100644 include/dt-bindings/reset/actions,s900-rmu.h -- 2.17.1
[PATCH v2 00/10] Add Reset Controller support for Actions Semi Owl SoCs
This patchset adds Reset Controller (RMU) support for Actions Semi Owl SoCs, S900 and S700. For the Owl SoCs, RMU's registers has been integrated into the CMU memory map in hardware. Hence, in this patchset the CMU driver has been converted to syscon so that the same memory map can be resued by both CMU and RMU drivers. Finally, the support for RMU in S700 and S900 SoCs are added. This patch series depends on the recently posted S700 clk series: "[PATCH v7 0/5] Add clock driver for Actions S700 SoC". For the S700 clk series, driver and bindings patches are applied through the clk tree. But the DTS patches are not yet picked up by the platform maintainer, Andreas. Hence, Andreas is expected to pick the DTS patches in this series once reviewed by the maintainers along with S700 clk DTS patches. Note: I have only tested the S900 part, S700 is only compile tested. But there is no reason for it to fail. Thanks, Mani Changes in v2: * Converted the CMU and RMU drivers to syscon for a more cleaner approach * Declared the owl_reset_map structs to const * Used regmap_update_bits instead of a combined regmap_read and write * Removed unused headers in RMU drivers * Added MAINTAINERS entry for the RMU driver and bindings Manivannan Sadhasivam (10): dt-bindings: clock: Add syscon support to Actions Semi Owl SoCs arm64: dts: actions: Convert Owl SoCs clock-controller nodes to syscon clk: actions: Add syscon support for Actions Semi Owl SoCs dt-bindings: reset: Add Actions Semi S900 SoC RMU support dt-bindings: reset: Add Actions Semi S700 SoC RMU support arm64: dts: actions: Add RMU node for Actions Semi S900 SoC arm64: dts: actions: Add RMU node for Actions Semi S700 SoC reset: Add Actions Semi S900 SoC Reset Management Unit support reset: Add Actions Semi S700 SoC Reset Management Unit support MAINTAINERS: Add entry for Actions Semi Owl SoCs Reset Management Unit .../bindings/clock/actions,owl-cmu.txt| 21 +- .../bindings/reset/actions,owl-reset.txt | 35 +++ MAINTAINERS | 4 + arch/arm64/boot/dts/actions/s700.dtsi | 18 +- arch/arm64/boot/dts/actions/s900.dtsi | 18 +- drivers/clk/actions/owl-common.c | 20 +- drivers/reset/Kconfig | 6 + drivers/reset/Makefile| 1 + drivers/reset/reset-owl.c | 225 ++ include/dt-bindings/reset/actions,s700-rmu.h | 34 +++ include/dt-bindings/reset/actions,s900-rmu.h | 65 + 11 files changed, 416 insertions(+), 31 deletions(-) create mode 100644 Documentation/devicetree/bindings/reset/actions,owl-reset.txt create mode 100644 drivers/reset/reset-owl.c create mode 100644 include/dt-bindings/reset/actions,s700-rmu.h create mode 100644 include/dt-bindings/reset/actions,s900-rmu.h -- 2.17.1
Re: [PATCH 7/9] clk: actions: Add Actions Semi Owl SoCs Reset Management Unit support
Hi Philipp,
On Mon, Jul 30, 2018 at 12:21:52PM +0200, Philipp Zabel wrote:
> Hi Manivannan,
>
> Thank you for the patch, a few small comments below:
>
> On Sat, 2018-07-28 at 00:15 +0530, Manivannan Sadhasivam wrote:
> > Add Reset Management Unit (RMU) support for Actions Semi Owl SoCs.
> >
> > Signed-off-by: Manivannan Sadhasivam
> > ---
> > drivers/clk/actions/Kconfig | 1 +
> > drivers/clk/actions/Makefile | 1 +
> > drivers/clk/actions/owl-common.h | 2 +
> > drivers/clk/actions/owl-reset.c | 72
> > drivers/clk/actions/owl-reset.h | 32 ++
> > 5 files changed, 108 insertions(+)
> > create mode 100644 drivers/clk/actions/owl-reset.c
> > create mode 100644 drivers/clk/actions/owl-reset.h
> >
> > diff --git a/drivers/clk/actions/Kconfig b/drivers/clk/actions/Kconfig
> > index dc38c85a4833..04f0a6355726 100644
> > --- a/drivers/clk/actions/Kconfig
> > +++ b/drivers/clk/actions/Kconfig
> > @@ -2,6 +2,7 @@ config CLK_ACTIONS
> > bool "Clock driver for Actions Semi SoCs"
> > depends on ARCH_ACTIONS || COMPILE_TEST
> > select REGMAP_MMIO
> > + select RESET_CONTROLLER
> > default ARCH_ACTIONS
> >
> > if CLK_ACTIONS
> > diff --git a/drivers/clk/actions/Makefile b/drivers/clk/actions/Makefile
> > index 78c17d56f991..ccfdf9781cef 100644
> > --- a/drivers/clk/actions/Makefile
> > +++ b/drivers/clk/actions/Makefile
> > @@ -7,6 +7,7 @@ clk-owl-y += owl-divider.o
> > clk-owl-y += owl-factor.o
> > clk-owl-y += owl-composite.o
> > clk-owl-y += owl-pll.o
> > +clk-owl-y += owl-reset.o
> >
> > # SoC support
> > obj-$(CONFIG_CLK_OWL_S700) += owl-s700.o
> > diff --git a/drivers/clk/actions/owl-common.h
> > b/drivers/clk/actions/owl-common.h
> > index 56f01f7774aa..4dc7f286831f 100644
> > --- a/drivers/clk/actions/owl-common.h
> > +++ b/drivers/clk/actions/owl-common.h
> > @@ -26,6 +26,8 @@ struct owl_clk_desc {
> > struct owl_clk_common **clks;
> > unsigned long num_clks;
> > struct clk_hw_onecell_data *hw_clks;
> > + struct owl_reset_map*resets;
>
> Could this be:
> const struct owl_reset_map *resets;
> ?
>
Ack.
> > + unsigned long num_resets;
> > struct regmap *regmap;
> > };
> >
> > diff --git a/drivers/clk/actions/owl-reset.c
> > b/drivers/clk/actions/owl-reset.c
> > new file mode 100644
> > index ..91b161cc68de
> > --- /dev/null
> > +++ b/drivers/clk/actions/owl-reset.c
> > @@ -0,0 +1,72 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +//
> > +// Actions Semi Owl SoCs Reset Management Unit driver
> > +//
> > +// Copyright (c) 2018 Linaro Ltd.
> > +// Author: Manivannan Sadhasivam
> > +
> > +#include
> > +#include
>
> This seems unnecessary, since register access is done via regmap.
>
Will remove this header.
> > +#include
> > +#include
> > +
> > +#include "owl-reset.h"
> > +
> > +static int owl_reset_assert(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct owl_reset *reset = to_owl_reset(rcdev);
> > + const struct owl_reset_map *map = >reset_map[id];
> > + u32 reg;
> > +
> > + regmap_read(reset->regmap, map->reg, );
> > + regmap_write(reset->regmap, map->reg, reg & ~map->bit);
>
> This read-modify-write sequence needs locking against concurrent
> register access. Better use regmap_update_bits():
>
> return regmap_update_bits(reset->regmap, map->reg, map->bit, 0);
>
Ack.
> > +
> > + return 0;
> > +}
> > +
> > +static int owl_reset_deassert(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct owl_reset *reset = to_owl_reset(rcdev);
> > + const struct owl_reset_map *map = >reset_map[id];
> > + u32 reg;
> > +
> > + regmap_read(reset->regmap, map->reg, );
> > + regmap_write(reset->regmap, map->reg, reg | map->bit);
>
> Better:
>
> return regmap_update_bits(reset->regmap, map->reg, map->bit, map->bit);
>
Ack.
> > +
> > + return 0;
> > +}
> > +
> > +static int owl_reset_reset(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + owl_reset_assert(rcdev, id);
> > + udelay(1);
>
> Is the delay valid for all IP cores on all SoCs variants?
>
It is valid for S900 and S700 for now but I'm not sure about S500. Will
make sure while adding S500 support.
> > + owl_reset_deassert(rcdev, id);
> > +
> > + return 0;
> > +}
> > +
> > +static int owl_reset_status(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct owl_reset *reset = to_owl_reset(rcdev);
> > + const struct owl_reset_map *map = >reset_map[id];
> > + u32 reg;
> > +
> > + regmap_read(reset->regmap, map->reg, );
>
> If this could return an error, better report it.
>
Ack.
> > +
> > + /*
> > +
Re: [PATCH 7/9] clk: actions: Add Actions Semi Owl SoCs Reset Management Unit support
Hi Philipp,
On Mon, Jul 30, 2018 at 12:21:52PM +0200, Philipp Zabel wrote:
> Hi Manivannan,
>
> Thank you for the patch, a few small comments below:
>
> On Sat, 2018-07-28 at 00:15 +0530, Manivannan Sadhasivam wrote:
> > Add Reset Management Unit (RMU) support for Actions Semi Owl SoCs.
> >
> > Signed-off-by: Manivannan Sadhasivam
> > ---
> > drivers/clk/actions/Kconfig | 1 +
> > drivers/clk/actions/Makefile | 1 +
> > drivers/clk/actions/owl-common.h | 2 +
> > drivers/clk/actions/owl-reset.c | 72
> > drivers/clk/actions/owl-reset.h | 32 ++
> > 5 files changed, 108 insertions(+)
> > create mode 100644 drivers/clk/actions/owl-reset.c
> > create mode 100644 drivers/clk/actions/owl-reset.h
> >
> > diff --git a/drivers/clk/actions/Kconfig b/drivers/clk/actions/Kconfig
> > index dc38c85a4833..04f0a6355726 100644
> > --- a/drivers/clk/actions/Kconfig
> > +++ b/drivers/clk/actions/Kconfig
> > @@ -2,6 +2,7 @@ config CLK_ACTIONS
> > bool "Clock driver for Actions Semi SoCs"
> > depends on ARCH_ACTIONS || COMPILE_TEST
> > select REGMAP_MMIO
> > + select RESET_CONTROLLER
> > default ARCH_ACTIONS
> >
> > if CLK_ACTIONS
> > diff --git a/drivers/clk/actions/Makefile b/drivers/clk/actions/Makefile
> > index 78c17d56f991..ccfdf9781cef 100644
> > --- a/drivers/clk/actions/Makefile
> > +++ b/drivers/clk/actions/Makefile
> > @@ -7,6 +7,7 @@ clk-owl-y += owl-divider.o
> > clk-owl-y += owl-factor.o
> > clk-owl-y += owl-composite.o
> > clk-owl-y += owl-pll.o
> > +clk-owl-y += owl-reset.o
> >
> > # SoC support
> > obj-$(CONFIG_CLK_OWL_S700) += owl-s700.o
> > diff --git a/drivers/clk/actions/owl-common.h
> > b/drivers/clk/actions/owl-common.h
> > index 56f01f7774aa..4dc7f286831f 100644
> > --- a/drivers/clk/actions/owl-common.h
> > +++ b/drivers/clk/actions/owl-common.h
> > @@ -26,6 +26,8 @@ struct owl_clk_desc {
> > struct owl_clk_common **clks;
> > unsigned long num_clks;
> > struct clk_hw_onecell_data *hw_clks;
> > + struct owl_reset_map*resets;
>
> Could this be:
> const struct owl_reset_map *resets;
> ?
>
Ack.
> > + unsigned long num_resets;
> > struct regmap *regmap;
> > };
> >
> > diff --git a/drivers/clk/actions/owl-reset.c
> > b/drivers/clk/actions/owl-reset.c
> > new file mode 100644
> > index ..91b161cc68de
> > --- /dev/null
> > +++ b/drivers/clk/actions/owl-reset.c
> > @@ -0,0 +1,72 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +//
> > +// Actions Semi Owl SoCs Reset Management Unit driver
> > +//
> > +// Copyright (c) 2018 Linaro Ltd.
> > +// Author: Manivannan Sadhasivam
> > +
> > +#include
> > +#include
>
> This seems unnecessary, since register access is done via regmap.
>
Will remove this header.
> > +#include
> > +#include
> > +
> > +#include "owl-reset.h"
> > +
> > +static int owl_reset_assert(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct owl_reset *reset = to_owl_reset(rcdev);
> > + const struct owl_reset_map *map = >reset_map[id];
> > + u32 reg;
> > +
> > + regmap_read(reset->regmap, map->reg, );
> > + regmap_write(reset->regmap, map->reg, reg & ~map->bit);
>
> This read-modify-write sequence needs locking against concurrent
> register access. Better use regmap_update_bits():
>
> return regmap_update_bits(reset->regmap, map->reg, map->bit, 0);
>
Ack.
> > +
> > + return 0;
> > +}
> > +
> > +static int owl_reset_deassert(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct owl_reset *reset = to_owl_reset(rcdev);
> > + const struct owl_reset_map *map = >reset_map[id];
> > + u32 reg;
> > +
> > + regmap_read(reset->regmap, map->reg, );
> > + regmap_write(reset->regmap, map->reg, reg | map->bit);
>
> Better:
>
> return regmap_update_bits(reset->regmap, map->reg, map->bit, map->bit);
>
Ack.
> > +
> > + return 0;
> > +}
> > +
> > +static int owl_reset_reset(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + owl_reset_assert(rcdev, id);
> > + udelay(1);
>
> Is the delay valid for all IP cores on all SoCs variants?
>
It is valid for S900 and S700 for now but I'm not sure about S500. Will
make sure while adding S500 support.
> > + owl_reset_deassert(rcdev, id);
> > +
> > + return 0;
> > +}
> > +
> > +static int owl_reset_status(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct owl_reset *reset = to_owl_reset(rcdev);
> > + const struct owl_reset_map *map = >reset_map[id];
> > + u32 reg;
> > +
> > + regmap_read(reset->regmap, map->reg, );
>
> If this could return an error, better report it.
>
Ack.
> > +
> > + /*
> > +
Wichtige Mitteilung
Hallo, ich bin Herr Tayeb Souami, New Jersey, Vereinigte Staaten von Amerika, Sie haben eine Wohltätigkeitsspende in Höhe von € 2.000.000,00, ich habe die America Lotterie in Amerika im Wert von $ 315 Millionen gewonnen, und ich gebe einen Teil davon an fünf glückliche Menschen und wohltätige Häuser, um Armut aus dieser Welt zu beseitigen, bekämpfe ich die Armut. Kontaktieren Sie mich für weitere Informationen: wumt.claimserv...@gmail.com Das ist dein Spendencode: [DFC530342018] Antworten Sie mit dem Spendencode auf diese E-Mail: wumt.claimserv...@gmail.com Ich hoffe, Sie und Ihre Familie glücklich zu machen. Grüße Herr Tayeb Souami
Wichtige Mitteilung
Hallo, ich bin Herr Tayeb Souami, New Jersey, Vereinigte Staaten von Amerika, Sie haben eine Wohltätigkeitsspende in Höhe von € 2.000.000,00, ich habe die America Lotterie in Amerika im Wert von $ 315 Millionen gewonnen, und ich gebe einen Teil davon an fünf glückliche Menschen und wohltätige Häuser, um Armut aus dieser Welt zu beseitigen, bekämpfe ich die Armut. Kontaktieren Sie mich für weitere Informationen: wumt.claimserv...@gmail.com Das ist dein Spendencode: [DFC530342018] Antworten Sie mit dem Spendencode auf diese E-Mail: wumt.claimserv...@gmail.com Ich hoffe, Sie und Ihre Familie glücklich zu machen. Grüße Herr Tayeb Souami
[PATCH] fs/jffs2 : prevent gc to pick block which includes inode with I_NEW state
In JFFS2, the process of creating a new file is split into two parts.
First, create the inode, name it "Part A", then create the dirent,
named it "Part B", both need reserve space. In Part A, a inode with
I_NEW state has been written in the block, we name it "block_a".
The inode I_NEW state will be cleared after Part B finished.
After "Part A" finished, alloc_sem lock is released. Other waiting
process able to run and write data in "block_a" until no more space
in the block. Next, if reserve space trigger gc, then "block_a" is
possible been picked by gc to collect.
Then, there is an inode with state I_NEW in "block_a", and gc collect
will find that inode and wait on I_NEW clear. Because gc holds alloc_sem
lock, so Part B waiting alloc_sem forever. System enter a deadlock state.
The call trace is as follows:
-
[43850.832264] Call trace:
[43850.834700] [] __switch_to+0xcc/0xd8
[43850.839834] [] __schedule+0x180/0x468
[43850.845048] [] schedule+0x38/0xb8
[43850.849920] [] bit_wait+0x14/0x68
[43850.854787] [] __wait_on_bit+0xa4/0xe0
[43850.860093] [] out_of_line_wait_on_bit+0x60/0x68
[43850.866263] [] iget_locked+0xd8/0x1a8
[43850.871483] [] jffs2_iget+0x14/0x330
[43850.876610] [] jffs2_gc_fetch_inode+0x5c/0x148
[43850.882611] [] jffs2_garbage_collect_pass+0x678/0x810
[43850.889219] [] jffs2_reserve_space+0x170/0x338
[43850.895215] [] jffs2_do_unlink+0x5c/0x240
[43850.900782] [] jffs2_rename+0x108/0x298
[43850.906169] [] vfs_rename+0x840/0x858
[43850.911389] [] SyS_renameat2+0x430/0x4a0
[43850.916863] [] SyS_renameat+0x10/0x18
[43850.922083] [] __sys_trace_return+0x0/0x4
This patch add "block_a" into a prevent list, which prevent gc to pick
for collecting. After testing, it can effectively solve the deadlock
bug.
Reported-by: Yang Yi
Signed-off-by: Liu Song
Tested-by: Yang Yi
Reviewed-by: Jiang Biao
---
fs/jffs2/build.c | 1 +
fs/jffs2/dir.c | 22 +-
fs/jffs2/gc.c | 4
fs/jffs2/jffs2_fs_sb.h | 1 +
fs/jffs2/nodelist.h| 46 +-
fs/jffs2/write.c | 5 -
6 files changed, 76 insertions(+), 3 deletions(-)
diff --git a/fs/jffs2/build.c b/fs/jffs2/build.c
index b288c8a..bee6521 100644
--- a/fs/jffs2/build.c
+++ b/fs/jffs2/build.c
@@ -403,6 +403,7 @@ int jffs2_do_mount_fs(struct jffs2_sb_info *c)
INIT_LIST_HEAD(>free_list);
INIT_LIST_HEAD(>bad_list);
INIT_LIST_HEAD(>bad_used_list);
+ INIT_LIST_HEAD(>prevent_gc_list);
c->highest_ino = 1;
c->summary = NULL;
diff --git a/fs/jffs2/dir.c b/fs/jffs2/dir.c
index b2944f9..b6782f1 100644
--- a/fs/jffs2/dir.c
+++ b/fs/jffs2/dir.c
@@ -164,6 +164,7 @@ static int jffs2_create(struct inode *dir_i, struct dentry
*dentry,
struct jffs2_inode_info *f, *dir_f;
struct jffs2_sb_info *c;
struct inode *inode;
+ struct prevent_block pb;
int ret;
ri = jffs2_alloc_raw_inode();
@@ -197,7 +198,7 @@ static int jffs2_create(struct inode *dir_i, struct dentry
*dentry,
no chance of AB-BA deadlock involving its f->sem). */
mutex_unlock(>sem);
- ret = jffs2_do_create(c, dir_f, f, ri, >d_name);
+ ret = jffs2_do_create(c, dir_f, f, ri, >d_name, );
if (ret)
goto fail;
@@ -210,10 +211,12 @@ static int jffs2_create(struct inode *dir_i, struct
dentry *dentry,
f->inocache->pino_nlink, inode->i_mapping->nrpages);
d_instantiate_new(dentry, inode);
+ remove_prevent_gc_list(c, );
return 0;
fail:
iget_failed(inode);
+ remove_prevent_gc_list(c, );
jffs2_free_raw_inode(ri);
return ret;
}
@@ -285,6 +288,7 @@ static int jffs2_symlink (struct inode *dir_i, struct
dentry *dentry, const char
struct jffs2_raw_dirent *rd;
struct jffs2_full_dnode *fn;
struct jffs2_full_dirent *fd;
+ struct prevent_block pb;
int namelen;
uint32_t alloclen;
int ret, targetlen = strlen(target);
@@ -346,6 +350,8 @@ static int jffs2_symlink (struct inode *dir_i, struct
dentry *dentry, const char
goto fail;
}
+ add_prevent_gc_list(c, );
+
/* We use f->target field to store the target path. */
f->target = kmemdup(target, targetlen + 1, GFP_KERNEL);
if (!f->target) {
@@ -430,10 +436,12 @@ static int jffs2_symlink (struct inode *dir_i, struct
dentry *dentry, const char
jffs2_complete_reservation(c);
d_instantiate_new(dentry, inode);
+ remove_prevent_gc_list(c, );
return 0;
fail:
iget_failed(inode);
+ remove_prevent_gc_list(c, );
return ret;
}
@@ -447,6 +455,7 @@ static int jffs2_mkdir (struct inode *dir_i, struct dentry
*dentry, umode_t mode
struct jffs2_raw_dirent *rd;
struct jffs2_full_dnode *fn;
struct jffs2_full_dirent *fd;
+ struct prevent_block pb;
int
[PATCH] fs/jffs2 : prevent gc to pick block which includes inode with I_NEW state
In JFFS2, the process of creating a new file is split into two parts.
First, create the inode, name it "Part A", then create the dirent,
named it "Part B", both need reserve space. In Part A, a inode with
I_NEW state has been written in the block, we name it "block_a".
The inode I_NEW state will be cleared after Part B finished.
After "Part A" finished, alloc_sem lock is released. Other waiting
process able to run and write data in "block_a" until no more space
in the block. Next, if reserve space trigger gc, then "block_a" is
possible been picked by gc to collect.
Then, there is an inode with state I_NEW in "block_a", and gc collect
will find that inode and wait on I_NEW clear. Because gc holds alloc_sem
lock, so Part B waiting alloc_sem forever. System enter a deadlock state.
The call trace is as follows:
-
[43850.832264] Call trace:
[43850.834700] [] __switch_to+0xcc/0xd8
[43850.839834] [] __schedule+0x180/0x468
[43850.845048] [] schedule+0x38/0xb8
[43850.849920] [] bit_wait+0x14/0x68
[43850.854787] [] __wait_on_bit+0xa4/0xe0
[43850.860093] [] out_of_line_wait_on_bit+0x60/0x68
[43850.866263] [] iget_locked+0xd8/0x1a8
[43850.871483] [] jffs2_iget+0x14/0x330
[43850.876610] [] jffs2_gc_fetch_inode+0x5c/0x148
[43850.882611] [] jffs2_garbage_collect_pass+0x678/0x810
[43850.889219] [] jffs2_reserve_space+0x170/0x338
[43850.895215] [] jffs2_do_unlink+0x5c/0x240
[43850.900782] [] jffs2_rename+0x108/0x298
[43850.906169] [] vfs_rename+0x840/0x858
[43850.911389] [] SyS_renameat2+0x430/0x4a0
[43850.916863] [] SyS_renameat+0x10/0x18
[43850.922083] [] __sys_trace_return+0x0/0x4
This patch add "block_a" into a prevent list, which prevent gc to pick
for collecting. After testing, it can effectively solve the deadlock
bug.
Reported-by: Yang Yi
Signed-off-by: Liu Song
Tested-by: Yang Yi
Reviewed-by: Jiang Biao
---
fs/jffs2/build.c | 1 +
fs/jffs2/dir.c | 22 +-
fs/jffs2/gc.c | 4
fs/jffs2/jffs2_fs_sb.h | 1 +
fs/jffs2/nodelist.h| 46 +-
fs/jffs2/write.c | 5 -
6 files changed, 76 insertions(+), 3 deletions(-)
diff --git a/fs/jffs2/build.c b/fs/jffs2/build.c
index b288c8a..bee6521 100644
--- a/fs/jffs2/build.c
+++ b/fs/jffs2/build.c
@@ -403,6 +403,7 @@ int jffs2_do_mount_fs(struct jffs2_sb_info *c)
INIT_LIST_HEAD(>free_list);
INIT_LIST_HEAD(>bad_list);
INIT_LIST_HEAD(>bad_used_list);
+ INIT_LIST_HEAD(>prevent_gc_list);
c->highest_ino = 1;
c->summary = NULL;
diff --git a/fs/jffs2/dir.c b/fs/jffs2/dir.c
index b2944f9..b6782f1 100644
--- a/fs/jffs2/dir.c
+++ b/fs/jffs2/dir.c
@@ -164,6 +164,7 @@ static int jffs2_create(struct inode *dir_i, struct dentry
*dentry,
struct jffs2_inode_info *f, *dir_f;
struct jffs2_sb_info *c;
struct inode *inode;
+ struct prevent_block pb;
int ret;
ri = jffs2_alloc_raw_inode();
@@ -197,7 +198,7 @@ static int jffs2_create(struct inode *dir_i, struct dentry
*dentry,
no chance of AB-BA deadlock involving its f->sem). */
mutex_unlock(>sem);
- ret = jffs2_do_create(c, dir_f, f, ri, >d_name);
+ ret = jffs2_do_create(c, dir_f, f, ri, >d_name, );
if (ret)
goto fail;
@@ -210,10 +211,12 @@ static int jffs2_create(struct inode *dir_i, struct
dentry *dentry,
f->inocache->pino_nlink, inode->i_mapping->nrpages);
d_instantiate_new(dentry, inode);
+ remove_prevent_gc_list(c, );
return 0;
fail:
iget_failed(inode);
+ remove_prevent_gc_list(c, );
jffs2_free_raw_inode(ri);
return ret;
}
@@ -285,6 +288,7 @@ static int jffs2_symlink (struct inode *dir_i, struct
dentry *dentry, const char
struct jffs2_raw_dirent *rd;
struct jffs2_full_dnode *fn;
struct jffs2_full_dirent *fd;
+ struct prevent_block pb;
int namelen;
uint32_t alloclen;
int ret, targetlen = strlen(target);
@@ -346,6 +350,8 @@ static int jffs2_symlink (struct inode *dir_i, struct
dentry *dentry, const char
goto fail;
}
+ add_prevent_gc_list(c, );
+
/* We use f->target field to store the target path. */
f->target = kmemdup(target, targetlen + 1, GFP_KERNEL);
if (!f->target) {
@@ -430,10 +436,12 @@ static int jffs2_symlink (struct inode *dir_i, struct
dentry *dentry, const char
jffs2_complete_reservation(c);
d_instantiate_new(dentry, inode);
+ remove_prevent_gc_list(c, );
return 0;
fail:
iget_failed(inode);
+ remove_prevent_gc_list(c, );
return ret;
}
@@ -447,6 +455,7 @@ static int jffs2_mkdir (struct inode *dir_i, struct dentry
*dentry, umode_t mode
struct jffs2_raw_dirent *rd;
struct jffs2_full_dnode *fn;
struct jffs2_full_dirent *fd;
+ struct prevent_block pb;
int
[PATCH] staging:mt29f_spinand: MT29F2G failing as only 16-bit arguments and variables used for addressing.
For NAND flash chips with more than 1Gbit (e.g. MT29F2G) more than 16 bits
are necessary to address the correct page. The driver sets the address for
more than 16 bits, but it uses 16-bit arguments and variables (these are
page_id, block_id, row) to do address operations. Obviously, these
arguments and variables cannot deal with more than 16-bit address.
Signed-off-by: Jheng-Jhong Wu
---
drivers/staging/mt29f_spinand/mt29f_spinand.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c
b/drivers/staging/mt29f_spinand/mt29f_spinand.c
index 4484784..a0f4cbcb 100644
--- a/drivers/staging/mt29f_spinand/mt29f_spinand.c
+++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c
@@ -308,10 +308,10 @@ static int spinand_write_enable(struct spi_device
*spi_nand)
return spinand_cmd(spi_nand, );
}
-static int spinand_read_page_to_cache(struct spi_device *spi_nand, u16 page_id)
+static int spinand_read_page_to_cache(struct spi_device *spi_nand, u32 page_id)
{
struct spinand_cmd cmd = {0};
- u16 row;
+ u32 row;
row = page_id;
cmd.cmd = CMD_READ;
@@ -331,7 +331,7 @@ static int spinand_read_page_to_cache(struct spi_device
*spi_nand, u16 page_id)
* locations.
* No tRd delay.
*/
-static int spinand_read_from_cache(struct spi_device *spi_nand, u16 page_id,
+static int spinand_read_from_cache(struct spi_device *spi_nand, u32 page_id,
u16 byte_id, u16 len, u8 *rbuf)
{
struct spinand_cmd cmd = {0};
@@ -362,7 +362,7 @@ static int spinand_read_from_cache(struct spi_device
*spi_nand, u16 page_id,
* The read includes two commands to the Nand - 0x13 and 0x03 commands
* Poll to read status to wait for tRD time.
*/
-static int spinand_read_page(struct spi_device *spi_nand, u16 page_id,
+static int spinand_read_page(struct spi_device *spi_nand, u32 page_id,
u16 offset, u16 len, u8 *rbuf)
{
int ret;
@@ -430,7 +430,7 @@ static int spinand_read_page(struct spi_device *spi_nand,
u16 page_id,
* Since it is writing the data to cache, there is no tPROG time.
*/
static int spinand_program_data_to_cache(struct spi_device *spi_nand,
-u16 page_id, u16 byte_id,
+u32 page_id, u16 byte_id,
u16 len, u8 *wbuf)
{
struct spinand_cmd cmd = {0};
@@ -457,10 +457,10 @@ static int spinand_program_data_to_cache(struct
spi_device *spi_nand,
* the Nand array.
* Need to wait for tPROG time to finish the transaction.
*/
-static int spinand_program_execute(struct spi_device *spi_nand, u16 page_id)
+static int spinand_program_execute(struct spi_device *spi_nand, u32 page_id)
{
struct spinand_cmd cmd = {0};
- u16 row;
+ u32 row;
row = page_id;
cmd.cmd = CMD_PROG_PAGE_EXC;
@@ -486,7 +486,7 @@ static int spinand_program_execute(struct spi_device
*spi_nand, u16 page_id)
* Poll to wait for the tPROG time to finish the transaction.
*/
static int spinand_program_page(struct spi_device *spi_nand,
- u16 page_id, u16 offset, u16 len, u8 *buf)
+ u32 page_id, u16 offset, u16 len, u8 *buf)
{
int retval;
u8 status = 0;
@@ -573,10 +573,10 @@ static int spinand_program_page(struct spi_device
*spi_nand,
* one block--64 pages
* Need to wait for tERS.
*/
-static int spinand_erase_block_erase(struct spi_device *spi_nand, u16 block_id)
+static int spinand_erase_block_erase(struct spi_device *spi_nand, u32 block_id)
{
struct spinand_cmd cmd = {0};
- u16 row;
+ u32 row;
row = block_id;
cmd.cmd = CMD_ERASE_BLK;
@@ -599,7 +599,7 @@ static int spinand_erase_block_erase(struct spi_device
*spi_nand, u16 block_id)
* and then send the 0xd8 erase command
* Poll to wait for the tERS time to complete the tranaction.
*/
-static int spinand_erase_block(struct spi_device *spi_nand, u16 block_id)
+static int spinand_erase_block(struct spi_device *spi_nand, u32 block_id)
{
int retval;
u8 status = 0;
--
2.7.4
Re: [PATCH] sched/fair: sync expires_seq in distribute_cfs_runtime()
On 8/1/18 4:55 AM, Cong Wang wrote:
> On Tue, Jul 31, 2018 at 10:13 AM wrote:
>>
>> Xunlei Pang writes:
>>
>>> On 7/31/18 1:55 AM, Cong Wang wrote:
On Sun, Jul 29, 2018 at 10:29 PM Xunlei Pang
wrote:
>
> Hi Cong,
>
> On 7/28/18 8:24 AM, Cong Wang wrote:
>> Each time we sync cfs_rq->runtime_expires with cfs_b->runtime_expires,
>> we should sync its ->expires_seq too. However it is missing
>> for distribute_cfs_runtime(), especially the slack timer call path.
>
> I don't think it's a problem, as expires_seq will get synced in
> assign_cfs_rq_runtime().
Sure, but there is a small window during which they are not synced.
Why do you want to wait until the next assign_cfs_rq_runtime() when
you already know runtime_expires is synced?
Also, expire_cfs_rq_runtime() is called before assign_cfs_rq_runtime()
inside __account_cfs_rq_runtime(), which means the check of
cfs_rq->expires_seq is not accurate for unthrottling case if the clock
drift happens soon enough?
>>>
>>> expire_cfs_rq_runtime():
>>> if (cfs_rq->expires_seq == cfs_b->expires_seq) {
>>> /* extend local deadline, drift is bounded above by 2 ticks */
>>> cfs_rq->runtime_expires += TICK_NSEC;
>>> } else {
>>> /* global deadline is ahead, expiration has passed */
>>> cfs_rq->runtime_remaining = 0;
>>> }
>>>
>>> So if clock drift happens soon, then expires_seq decides the correct
>>> thing we should do: if cfs_b->expires_seq advanced, then clear the stale
>>> cfs_rq->runtime_remaining from the slack timer of the past period, then
>>> assign_cfs_rq_runtime() will refresh them afterwards, otherwise it is a
>>> real clock drift. I am still not getting where the race is?
>
> But expires_seq is supposed to be the same here, after
> distribute_cfs_runtime(), therefore runtime_remaining is not supposed
> to be cleared.
>
> Which part do I misunderstand? expires_seq should not be same here?
> Or you are saying a wrongly clear of runtime_remaning is fine?
>
Let's see the unthrottle cases.
1. for the periodic timer
distribute_cfs_runtime updates the throttled cfs_rq->runtime_expires to
be a new value, so expire_cfs_rq_runtime does nothing because of:
rq_clock(rq_of(cfs_rq)) - cfs_rq->runtime_expires < 0
Afterwards assign_cfs_rq_runtime() will sync its expires_seq.
2. for the slack timer
the two expires_seq should be the same, so if clock drift happens soon,
expire_cfs_rq_runtime regards it as true clock drift:
cfs_rq->runtime_expires += TICK_NSEC
If it happens that global expires_seq advances, it also doesn't matter,
expire_cfs_rq_runtime will clear the stale expire_cfs_rq_runtime as
expected.
>
>>
>> Nothing /important/ goes wrong because distribute_cfs_runtime only fills
>> runtime_remaining up to 1, not a real amount.
>
> No, runtime_remaining is updated right before expire_cfs_rq_runtime():
>
> static void __account_cfs_rq_runtime(struct cfs_rq *cfs_rq, u64 delta_exec)
> {
> /* dock delta_exec before expiring quota (as it could span periods) */
> cfs_rq->runtime_remaining -= delta_exec;
> expire_cfs_rq_runtime(cfs_rq);
>
> so almost certainly it can't be 1.
I think Ben means it firstly gets a distributtion of 1 to run after
unthrottling, soon it will have a negative runtime_remaining, and go
to assign_cfs_rq_runtime().
Thanks,
Xunlei
>
> Which means the following check could be passed:
>
> 4655 if (cfs_rq->runtime_remaining < 0)
> 4656 return;
>
> therefore we are reaching the clock drift logic code inside
> expire_cfs_rq_runtime()
> where expires_seq is supposed to be same as they should be sync'ed.
> Therefore without patch, we wrongly clear the runtime_remainng?
>
> Thanks.
>
[PATCH] staging:mt29f_spinand: MT29F2G failing as only 16-bit arguments and variables used for addressing.
For NAND flash chips with more than 1Gbit (e.g. MT29F2G) more than 16 bits
are necessary to address the correct page. The driver sets the address for
more than 16 bits, but it uses 16-bit arguments and variables (these are
page_id, block_id, row) to do address operations. Obviously, these
arguments and variables cannot deal with more than 16-bit address.
Signed-off-by: Jheng-Jhong Wu
---
drivers/staging/mt29f_spinand/mt29f_spinand.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c
b/drivers/staging/mt29f_spinand/mt29f_spinand.c
index 4484784..a0f4cbcb 100644
--- a/drivers/staging/mt29f_spinand/mt29f_spinand.c
+++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c
@@ -308,10 +308,10 @@ static int spinand_write_enable(struct spi_device
*spi_nand)
return spinand_cmd(spi_nand, );
}
-static int spinand_read_page_to_cache(struct spi_device *spi_nand, u16 page_id)
+static int spinand_read_page_to_cache(struct spi_device *spi_nand, u32 page_id)
{
struct spinand_cmd cmd = {0};
- u16 row;
+ u32 row;
row = page_id;
cmd.cmd = CMD_READ;
@@ -331,7 +331,7 @@ static int spinand_read_page_to_cache(struct spi_device
*spi_nand, u16 page_id)
* locations.
* No tRd delay.
*/
-static int spinand_read_from_cache(struct spi_device *spi_nand, u16 page_id,
+static int spinand_read_from_cache(struct spi_device *spi_nand, u32 page_id,
u16 byte_id, u16 len, u8 *rbuf)
{
struct spinand_cmd cmd = {0};
@@ -362,7 +362,7 @@ static int spinand_read_from_cache(struct spi_device
*spi_nand, u16 page_id,
* The read includes two commands to the Nand - 0x13 and 0x03 commands
* Poll to read status to wait for tRD time.
*/
-static int spinand_read_page(struct spi_device *spi_nand, u16 page_id,
+static int spinand_read_page(struct spi_device *spi_nand, u32 page_id,
u16 offset, u16 len, u8 *rbuf)
{
int ret;
@@ -430,7 +430,7 @@ static int spinand_read_page(struct spi_device *spi_nand,
u16 page_id,
* Since it is writing the data to cache, there is no tPROG time.
*/
static int spinand_program_data_to_cache(struct spi_device *spi_nand,
-u16 page_id, u16 byte_id,
+u32 page_id, u16 byte_id,
u16 len, u8 *wbuf)
{
struct spinand_cmd cmd = {0};
@@ -457,10 +457,10 @@ static int spinand_program_data_to_cache(struct
spi_device *spi_nand,
* the Nand array.
* Need to wait for tPROG time to finish the transaction.
*/
-static int spinand_program_execute(struct spi_device *spi_nand, u16 page_id)
+static int spinand_program_execute(struct spi_device *spi_nand, u32 page_id)
{
struct spinand_cmd cmd = {0};
- u16 row;
+ u32 row;
row = page_id;
cmd.cmd = CMD_PROG_PAGE_EXC;
@@ -486,7 +486,7 @@ static int spinand_program_execute(struct spi_device
*spi_nand, u16 page_id)
* Poll to wait for the tPROG time to finish the transaction.
*/
static int spinand_program_page(struct spi_device *spi_nand,
- u16 page_id, u16 offset, u16 len, u8 *buf)
+ u32 page_id, u16 offset, u16 len, u8 *buf)
{
int retval;
u8 status = 0;
@@ -573,10 +573,10 @@ static int spinand_program_page(struct spi_device
*spi_nand,
* one block--64 pages
* Need to wait for tERS.
*/
-static int spinand_erase_block_erase(struct spi_device *spi_nand, u16 block_id)
+static int spinand_erase_block_erase(struct spi_device *spi_nand, u32 block_id)
{
struct spinand_cmd cmd = {0};
- u16 row;
+ u32 row;
row = block_id;
cmd.cmd = CMD_ERASE_BLK;
@@ -599,7 +599,7 @@ static int spinand_erase_block_erase(struct spi_device
*spi_nand, u16 block_id)
* and then send the 0xd8 erase command
* Poll to wait for the tERS time to complete the tranaction.
*/
-static int spinand_erase_block(struct spi_device *spi_nand, u16 block_id)
+static int spinand_erase_block(struct spi_device *spi_nand, u32 block_id)
{
int retval;
u8 status = 0;
--
2.7.4
Re: [PATCH] sched/fair: sync expires_seq in distribute_cfs_runtime()
On 8/1/18 4:55 AM, Cong Wang wrote:
> On Tue, Jul 31, 2018 at 10:13 AM wrote:
>>
>> Xunlei Pang writes:
>>
>>> On 7/31/18 1:55 AM, Cong Wang wrote:
On Sun, Jul 29, 2018 at 10:29 PM Xunlei Pang
wrote:
>
> Hi Cong,
>
> On 7/28/18 8:24 AM, Cong Wang wrote:
>> Each time we sync cfs_rq->runtime_expires with cfs_b->runtime_expires,
>> we should sync its ->expires_seq too. However it is missing
>> for distribute_cfs_runtime(), especially the slack timer call path.
>
> I don't think it's a problem, as expires_seq will get synced in
> assign_cfs_rq_runtime().
Sure, but there is a small window during which they are not synced.
Why do you want to wait until the next assign_cfs_rq_runtime() when
you already know runtime_expires is synced?
Also, expire_cfs_rq_runtime() is called before assign_cfs_rq_runtime()
inside __account_cfs_rq_runtime(), which means the check of
cfs_rq->expires_seq is not accurate for unthrottling case if the clock
drift happens soon enough?
>>>
>>> expire_cfs_rq_runtime():
>>> if (cfs_rq->expires_seq == cfs_b->expires_seq) {
>>> /* extend local deadline, drift is bounded above by 2 ticks */
>>> cfs_rq->runtime_expires += TICK_NSEC;
>>> } else {
>>> /* global deadline is ahead, expiration has passed */
>>> cfs_rq->runtime_remaining = 0;
>>> }
>>>
>>> So if clock drift happens soon, then expires_seq decides the correct
>>> thing we should do: if cfs_b->expires_seq advanced, then clear the stale
>>> cfs_rq->runtime_remaining from the slack timer of the past period, then
>>> assign_cfs_rq_runtime() will refresh them afterwards, otherwise it is a
>>> real clock drift. I am still not getting where the race is?
>
> But expires_seq is supposed to be the same here, after
> distribute_cfs_runtime(), therefore runtime_remaining is not supposed
> to be cleared.
>
> Which part do I misunderstand? expires_seq should not be same here?
> Or you are saying a wrongly clear of runtime_remaning is fine?
>
Let's see the unthrottle cases.
1. for the periodic timer
distribute_cfs_runtime updates the throttled cfs_rq->runtime_expires to
be a new value, so expire_cfs_rq_runtime does nothing because of:
rq_clock(rq_of(cfs_rq)) - cfs_rq->runtime_expires < 0
Afterwards assign_cfs_rq_runtime() will sync its expires_seq.
2. for the slack timer
the two expires_seq should be the same, so if clock drift happens soon,
expire_cfs_rq_runtime regards it as true clock drift:
cfs_rq->runtime_expires += TICK_NSEC
If it happens that global expires_seq advances, it also doesn't matter,
expire_cfs_rq_runtime will clear the stale expire_cfs_rq_runtime as
expected.
>
>>
>> Nothing /important/ goes wrong because distribute_cfs_runtime only fills
>> runtime_remaining up to 1, not a real amount.
>
> No, runtime_remaining is updated right before expire_cfs_rq_runtime():
>
> static void __account_cfs_rq_runtime(struct cfs_rq *cfs_rq, u64 delta_exec)
> {
> /* dock delta_exec before expiring quota (as it could span periods) */
> cfs_rq->runtime_remaining -= delta_exec;
> expire_cfs_rq_runtime(cfs_rq);
>
> so almost certainly it can't be 1.
I think Ben means it firstly gets a distributtion of 1 to run after
unthrottling, soon it will have a negative runtime_remaining, and go
to assign_cfs_rq_runtime().
Thanks,
Xunlei
>
> Which means the following check could be passed:
>
> 4655 if (cfs_rq->runtime_remaining < 0)
> 4656 return;
>
> therefore we are reaching the clock drift logic code inside
> expire_cfs_rq_runtime()
> where expires_seq is supposed to be same as they should be sync'ed.
> Therefore without patch, we wrongly clear the runtime_remainng?
>
> Thanks.
>
Re: [PATCH 2/4] genirq: Provide NMI management for percpu_devid interrupts
On Tue, Jul 24, 2018 at 12:07:05PM +0100, Julien Thierry wrote:
> Add support for percpu_devid interrupts treated as NMIs.
>
> Percpu_devid NMIs need to be setup/torn down on each CPU they target.
>
> The same restrictions as for global NMIs still apply for percpu_devid NMIs.
>
> Signed-off-by: Julien Thierry
> Cc: Thomas Gleixner
> Cc: Peter Zijlstra
> Cc: Ingo Molnar
> Cc: Marc Zyngier
> ---
> include/linux/interrupt.h | 9 +++
> kernel/irq/manage.c | 149
> ++
> 2 files changed, 158 insertions(+)
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index 3b1a320..59d3877 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -168,10 +168,15 @@ struct irqaction {
> devname, percpu_dev_id);
> }
>
> +extern int __must_check
> +request_percpu_nmi(unsigned int irq, irq_handler_t handler,
> +const char *devname, void __percpu *dev);
> +
> extern const void *free_irq(unsigned int, void *);
> extern void free_percpu_irq(unsigned int, void __percpu *);
>
> extern const void *free_nmi(unsigned int irq, void *dev_id);
> +extern void free_percpu_nmi(unsigned int irq, void __percpu *percpu_dev_id);
>
> struct device;
>
> @@ -224,7 +229,11 @@ struct irqaction {
> extern void irq_wake_thread(unsigned int irq, void *dev_id);
>
> extern void disable_nmi_nosync(unsigned int irq);
> +extern void disable_percpu_nmi(unsigned int irq);
Shouldn't this be a disable_percpu_nmi_nosync() as disable_nmi_nosync()?
> extern void enable_nmi(unsigned int irq);
> +extern void enable_percpu_nmi(unsigned int irq, unsigned int type);
> +extern int ready_percpu_nmi(unsigned int irq);
> +extern void teardown_percpu_nmi(unsigned int irq);
>
> /* The following three functions are for the core kernel use only. */
> extern void suspend_device_irqs(void);
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 3b87143..ad41c4d 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -2162,6 +2162,11 @@ void enable_percpu_irq(unsigned int irq, unsigned int
> type)
> }
> EXPORT_SYMBOL_GPL(enable_percpu_irq);
>
> +void enable_percpu_nmi(unsigned int irq, unsigned int type)
> +{
> + enable_percpu_irq(irq, type);
> +}
> +
> /**
> * irq_percpu_is_enabled - Check whether the per cpu irq is enabled
> * @irq: Linux irq number to check for
> @@ -2201,6 +2206,11 @@ void disable_percpu_irq(unsigned int irq)
> }
> EXPORT_SYMBOL_GPL(disable_percpu_irq);
>
> +void disable_percpu_nmi(unsigned int irq)
> +{
> + disable_percpu_irq(irq);
> +}
> +
> /*
> * Internal function to unregister a percpu irqaction.
> */
> @@ -2232,6 +2242,8 @@ static struct irqaction *__free_percpu_irq(unsigned int
> irq, void __percpu *dev_
> /* Found it - now remove it from the list of entries: */
> desc->action = NULL;
>
> + desc->istate &= ~IRQS_NMI;
> +
> raw_spin_unlock_irqrestore(>lock, flags);
>
> unregister_handler_proc(irq, action);
> @@ -2285,6 +2297,19 @@ void free_percpu_irq(unsigned int irq, void __percpu
> *dev_id)
> }
> EXPORT_SYMBOL_GPL(free_percpu_irq);
>
> +void free_percpu_nmi(unsigned int irq, void __percpu *dev_id)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (!desc || !irq_settings_is_per_cpu_devid(desc))
> + return;
> +
> + if (WARN_ON(!(desc->istate & IRQS_NMI)))
> + return;
> +
> + kfree(__free_percpu_irq(irq, dev_id));
> +}
> +
> /**
> * setup_percpu_irq - setup a per-cpu interrupt
> * @irq: Interrupt line to setup
> @@ -2375,6 +2400,130 @@ int __request_percpu_irq(unsigned int irq,
> irq_handler_t handler,
> EXPORT_SYMBOL_GPL(__request_percpu_irq);
>
> /**
> + * request_percpu_nmi - allocate a percpu interrupt line for NMI delivery
> + * @irq: Interrupt line to allocate
> + * @handler: Function to be called when the IRQ occurs.
> + * @devname: An ascii name for the claiming device
> + * @dev_id: A percpu cookie passed back to the handler function
> + *
> + * This call allocates interrupt resources and enables the
> + * interrupt on the local CPU. If the interrupt is supposed to be
> + * enabled on other CPUs, it has to be done on each CPU using
> + * enable_percpu_nmi().
> + *
> + * Dev_id must be globally unique. It is a per-cpu variable, and
> + * the handler gets called with the interrupted CPU's instance of
> + * that variable.
> + *
> + * Interrupt lines requested for NMI delivering should have auto enabling
> + * setting disabled.
> + *
> + * If the interrupt line cannot be used to deliver NMIs, function
> + * will fail returning a negative value.
> + */
> +int request_percpu_nmi(unsigned int irq, irq_handler_t handler,
> +const char *name, void __percpu *dev_id)
> +{
> + struct irqaction *action;
> + struct irq_desc *desc;
> + unsigned long flags;
> +
Re: [PATCH 2/4] genirq: Provide NMI management for percpu_devid interrupts
On Tue, Jul 24, 2018 at 12:07:05PM +0100, Julien Thierry wrote:
> Add support for percpu_devid interrupts treated as NMIs.
>
> Percpu_devid NMIs need to be setup/torn down on each CPU they target.
>
> The same restrictions as for global NMIs still apply for percpu_devid NMIs.
>
> Signed-off-by: Julien Thierry
> Cc: Thomas Gleixner
> Cc: Peter Zijlstra
> Cc: Ingo Molnar
> Cc: Marc Zyngier
> ---
> include/linux/interrupt.h | 9 +++
> kernel/irq/manage.c | 149
> ++
> 2 files changed, 158 insertions(+)
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index 3b1a320..59d3877 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -168,10 +168,15 @@ struct irqaction {
> devname, percpu_dev_id);
> }
>
> +extern int __must_check
> +request_percpu_nmi(unsigned int irq, irq_handler_t handler,
> +const char *devname, void __percpu *dev);
> +
> extern const void *free_irq(unsigned int, void *);
> extern void free_percpu_irq(unsigned int, void __percpu *);
>
> extern const void *free_nmi(unsigned int irq, void *dev_id);
> +extern void free_percpu_nmi(unsigned int irq, void __percpu *percpu_dev_id);
>
> struct device;
>
> @@ -224,7 +229,11 @@ struct irqaction {
> extern void irq_wake_thread(unsigned int irq, void *dev_id);
>
> extern void disable_nmi_nosync(unsigned int irq);
> +extern void disable_percpu_nmi(unsigned int irq);
Shouldn't this be a disable_percpu_nmi_nosync() as disable_nmi_nosync()?
> extern void enable_nmi(unsigned int irq);
> +extern void enable_percpu_nmi(unsigned int irq, unsigned int type);
> +extern int ready_percpu_nmi(unsigned int irq);
> +extern void teardown_percpu_nmi(unsigned int irq);
>
> /* The following three functions are for the core kernel use only. */
> extern void suspend_device_irqs(void);
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 3b87143..ad41c4d 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -2162,6 +2162,11 @@ void enable_percpu_irq(unsigned int irq, unsigned int
> type)
> }
> EXPORT_SYMBOL_GPL(enable_percpu_irq);
>
> +void enable_percpu_nmi(unsigned int irq, unsigned int type)
> +{
> + enable_percpu_irq(irq, type);
> +}
> +
> /**
> * irq_percpu_is_enabled - Check whether the per cpu irq is enabled
> * @irq: Linux irq number to check for
> @@ -2201,6 +2206,11 @@ void disable_percpu_irq(unsigned int irq)
> }
> EXPORT_SYMBOL_GPL(disable_percpu_irq);
>
> +void disable_percpu_nmi(unsigned int irq)
> +{
> + disable_percpu_irq(irq);
> +}
> +
> /*
> * Internal function to unregister a percpu irqaction.
> */
> @@ -2232,6 +2242,8 @@ static struct irqaction *__free_percpu_irq(unsigned int
> irq, void __percpu *dev_
> /* Found it - now remove it from the list of entries: */
> desc->action = NULL;
>
> + desc->istate &= ~IRQS_NMI;
> +
> raw_spin_unlock_irqrestore(>lock, flags);
>
> unregister_handler_proc(irq, action);
> @@ -2285,6 +2297,19 @@ void free_percpu_irq(unsigned int irq, void __percpu
> *dev_id)
> }
> EXPORT_SYMBOL_GPL(free_percpu_irq);
>
> +void free_percpu_nmi(unsigned int irq, void __percpu *dev_id)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> +
> + if (!desc || !irq_settings_is_per_cpu_devid(desc))
> + return;
> +
> + if (WARN_ON(!(desc->istate & IRQS_NMI)))
> + return;
> +
> + kfree(__free_percpu_irq(irq, dev_id));
> +}
> +
> /**
> * setup_percpu_irq - setup a per-cpu interrupt
> * @irq: Interrupt line to setup
> @@ -2375,6 +2400,130 @@ int __request_percpu_irq(unsigned int irq,
> irq_handler_t handler,
> EXPORT_SYMBOL_GPL(__request_percpu_irq);
>
> /**
> + * request_percpu_nmi - allocate a percpu interrupt line for NMI delivery
> + * @irq: Interrupt line to allocate
> + * @handler: Function to be called when the IRQ occurs.
> + * @devname: An ascii name for the claiming device
> + * @dev_id: A percpu cookie passed back to the handler function
> + *
> + * This call allocates interrupt resources and enables the
> + * interrupt on the local CPU. If the interrupt is supposed to be
> + * enabled on other CPUs, it has to be done on each CPU using
> + * enable_percpu_nmi().
> + *
> + * Dev_id must be globally unique. It is a per-cpu variable, and
> + * the handler gets called with the interrupted CPU's instance of
> + * that variable.
> + *
> + * Interrupt lines requested for NMI delivering should have auto enabling
> + * setting disabled.
> + *
> + * If the interrupt line cannot be used to deliver NMIs, function
> + * will fail returning a negative value.
> + */
> +int request_percpu_nmi(unsigned int irq, irq_handler_t handler,
> +const char *name, void __percpu *dev_id)
> +{
> + struct irqaction *action;
> + struct irq_desc *desc;
> + unsigned long flags;
> +
Re: [PATCH 1/4] genirq: Provide basic NMI management for interrupt lines
On Tue, Jul 24, 2018 at 12:07:04PM +0100, Julien Thierry wrote:
Hi Julien,
Many thanks for your patchset and I apologize for the late reply. I tried
to use your patches on my own use case and I have the following
comments...
> Add functionality to allocate interrupt lines that will deliver IRQs
> as Non-Maskable Interrupts. These allocations are only successful if
> the irqchip provides the necessary support and allows NMI delivery for the
> interrupt line.
>
> Interrupt lines allocated for NMI delivery must be enabled/disabled through
> enable_nmi/disable_nmi_nosync to keep their state consistent.
>
> To treat a PERCPU IRQ as NMI, the interrupt must not be shared nor threaded,
> the irqchip directly managing the IRQ must be the root irqchip and the
> irqchip cannot be behind a slow bus.
>
> Signed-off-by: Julien Thierry
> Cc: Thomas Gleixner
> Cc: Peter Zijlstra
> Cc: Ingo Molnar
> Cc: Marc Zyngier
> ---
> include/linux/interrupt.h | 9 ++
> include/linux/irq.h | 7 ++
> kernel/irq/debugfs.c | 6 +-
> kernel/irq/internals.h| 1 +
> kernel/irq/manage.c | 228
> +-
> 5 files changed, 248 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index eeceac3..3b1a320 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -156,6 +156,10 @@ struct irqaction {
>unsigned long flags, const char *devname,
>void __percpu *percpu_dev_id);
>
> +extern int __must_check
> +request_nmi(unsigned int irq, irq_handler_t handler, unsigned long flags,
> + const char *name, void *dev);
> +
> static inline int __must_check
> request_percpu_irq(unsigned int irq, irq_handler_t handler,
> const char *devname, void __percpu *percpu_dev_id)
> @@ -167,6 +171,8 @@ struct irqaction {
> extern const void *free_irq(unsigned int, void *);
> extern void free_percpu_irq(unsigned int, void __percpu *);
>
> +extern const void *free_nmi(unsigned int irq, void *dev_id);
> +
> struct device;
>
> extern int __must_check
> @@ -217,6 +223,9 @@ struct irqaction {
> extern bool irq_percpu_is_enabled(unsigned int irq);
> extern void irq_wake_thread(unsigned int irq, void *dev_id);
>
> +extern void disable_nmi_nosync(unsigned int irq);
> +extern void enable_nmi(unsigned int irq);
> +
> /* The following three functions are for the core kernel use only. */
> extern void suspend_device_irqs(void);
> extern void resume_device_irqs(void);
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 201de12..0d71f63 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -441,6 +441,8 @@ static inline irq_hw_number_t irqd_to_hwirq(struct
> irq_data *d)
> * @irq_set_vcpu_affinity: optional to target a vCPU in a virtual machine
> * @ipi_send_single: send a single IPI to destination cpus
> * @ipi_send_mask: send an IPI to destination cpus in cpumask
> + * @irq_nmi_setup: function called from core code before enabling an NMI
> + * @irq_nmi_teardown:function called from core code after disabling
> an NMI
> * @flags: chip specific flags
> */
> struct irq_chip {
> @@ -489,6 +491,9 @@ struct irq_chip {
> void(*ipi_send_single)(struct irq_data *data, unsigned int
> cpu);
> void(*ipi_send_mask)(struct irq_data *data, const struct
> cpumask *dest);
>
> + int (*irq_nmi_setup)(struct irq_data *data);
> + void(*irq_nmi_teardown)(struct irq_data *data);
> +
> unsigned long flags;
> };
>
> @@ -504,6 +509,7 @@ struct irq_chip {
> * IRQCHIP_ONESHOT_SAFE: One shot does not require mask/unmask
> * IRQCHIP_EOI_THREADED: Chip requires eoi() on unmask in threaded mode
> * IRQCHIP_SUPPORTS_LEVEL_MSIChip can provide two doorbells for
> Level MSIs
> + * IRQCHIP_SUPPORTS_NMI: Chip can deliver NMIs, only for root irqchips
> */
> enum {
> IRQCHIP_SET_TYPE_MASKED = (1 << 0),
> @@ -514,6 +520,7 @@ enum {
> IRQCHIP_ONESHOT_SAFE= (1 << 5),
> IRQCHIP_EOI_THREADED= (1 << 6),
> IRQCHIP_SUPPORTS_LEVEL_MSI = (1 << 7),
> + IRQCHIP_SUPPORTS_NMI= (1 << 8),
> };
>
> #include
> diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c
> index 6f63613..59a04d2 100644
> --- a/kernel/irq/debugfs.c
> +++ b/kernel/irq/debugfs.c
> @@ -56,6 +56,7 @@ static void irq_debug_show_masks(struct seq_file *m, struct
> irq_desc *desc) { }
> BIT_MASK_DESCR(IRQCHIP_ONESHOT_SAFE),
> BIT_MASK_DESCR(IRQCHIP_EOI_THREADED),
> BIT_MASK_DESCR(IRQCHIP_SUPPORTS_LEVEL_MSI),
> + BIT_MASK_DESCR(IRQCHIP_SUPPORTS_NMI),
> };
>
> static void
> @@ -140,6 +141,7 @@ static void irq_debug_show_masks(struct seq_file *m,
> struct irq_desc *desc) { }
> BIT_MASK_DESCR(IRQS_WAITING),
> BIT_MASK_DESCR(IRQS_PENDING),
>
Re: [PATCH 1/4] genirq: Provide basic NMI management for interrupt lines
On Tue, Jul 24, 2018 at 12:07:04PM +0100, Julien Thierry wrote:
Hi Julien,
Many thanks for your patchset and I apologize for the late reply. I tried
to use your patches on my own use case and I have the following
comments...
> Add functionality to allocate interrupt lines that will deliver IRQs
> as Non-Maskable Interrupts. These allocations are only successful if
> the irqchip provides the necessary support and allows NMI delivery for the
> interrupt line.
>
> Interrupt lines allocated for NMI delivery must be enabled/disabled through
> enable_nmi/disable_nmi_nosync to keep their state consistent.
>
> To treat a PERCPU IRQ as NMI, the interrupt must not be shared nor threaded,
> the irqchip directly managing the IRQ must be the root irqchip and the
> irqchip cannot be behind a slow bus.
>
> Signed-off-by: Julien Thierry
> Cc: Thomas Gleixner
> Cc: Peter Zijlstra
> Cc: Ingo Molnar
> Cc: Marc Zyngier
> ---
> include/linux/interrupt.h | 9 ++
> include/linux/irq.h | 7 ++
> kernel/irq/debugfs.c | 6 +-
> kernel/irq/internals.h| 1 +
> kernel/irq/manage.c | 228
> +-
> 5 files changed, 248 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index eeceac3..3b1a320 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -156,6 +156,10 @@ struct irqaction {
>unsigned long flags, const char *devname,
>void __percpu *percpu_dev_id);
>
> +extern int __must_check
> +request_nmi(unsigned int irq, irq_handler_t handler, unsigned long flags,
> + const char *name, void *dev);
> +
> static inline int __must_check
> request_percpu_irq(unsigned int irq, irq_handler_t handler,
> const char *devname, void __percpu *percpu_dev_id)
> @@ -167,6 +171,8 @@ struct irqaction {
> extern const void *free_irq(unsigned int, void *);
> extern void free_percpu_irq(unsigned int, void __percpu *);
>
> +extern const void *free_nmi(unsigned int irq, void *dev_id);
> +
> struct device;
>
> extern int __must_check
> @@ -217,6 +223,9 @@ struct irqaction {
> extern bool irq_percpu_is_enabled(unsigned int irq);
> extern void irq_wake_thread(unsigned int irq, void *dev_id);
>
> +extern void disable_nmi_nosync(unsigned int irq);
> +extern void enable_nmi(unsigned int irq);
> +
> /* The following three functions are for the core kernel use only. */
> extern void suspend_device_irqs(void);
> extern void resume_device_irqs(void);
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 201de12..0d71f63 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -441,6 +441,8 @@ static inline irq_hw_number_t irqd_to_hwirq(struct
> irq_data *d)
> * @irq_set_vcpu_affinity: optional to target a vCPU in a virtual machine
> * @ipi_send_single: send a single IPI to destination cpus
> * @ipi_send_mask: send an IPI to destination cpus in cpumask
> + * @irq_nmi_setup: function called from core code before enabling an NMI
> + * @irq_nmi_teardown:function called from core code after disabling
> an NMI
> * @flags: chip specific flags
> */
> struct irq_chip {
> @@ -489,6 +491,9 @@ struct irq_chip {
> void(*ipi_send_single)(struct irq_data *data, unsigned int
> cpu);
> void(*ipi_send_mask)(struct irq_data *data, const struct
> cpumask *dest);
>
> + int (*irq_nmi_setup)(struct irq_data *data);
> + void(*irq_nmi_teardown)(struct irq_data *data);
> +
> unsigned long flags;
> };
>
> @@ -504,6 +509,7 @@ struct irq_chip {
> * IRQCHIP_ONESHOT_SAFE: One shot does not require mask/unmask
> * IRQCHIP_EOI_THREADED: Chip requires eoi() on unmask in threaded mode
> * IRQCHIP_SUPPORTS_LEVEL_MSIChip can provide two doorbells for
> Level MSIs
> + * IRQCHIP_SUPPORTS_NMI: Chip can deliver NMIs, only for root irqchips
> */
> enum {
> IRQCHIP_SET_TYPE_MASKED = (1 << 0),
> @@ -514,6 +520,7 @@ enum {
> IRQCHIP_ONESHOT_SAFE= (1 << 5),
> IRQCHIP_EOI_THREADED= (1 << 6),
> IRQCHIP_SUPPORTS_LEVEL_MSI = (1 << 7),
> + IRQCHIP_SUPPORTS_NMI= (1 << 8),
> };
>
> #include
> diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c
> index 6f63613..59a04d2 100644
> --- a/kernel/irq/debugfs.c
> +++ b/kernel/irq/debugfs.c
> @@ -56,6 +56,7 @@ static void irq_debug_show_masks(struct seq_file *m, struct
> irq_desc *desc) { }
> BIT_MASK_DESCR(IRQCHIP_ONESHOT_SAFE),
> BIT_MASK_DESCR(IRQCHIP_EOI_THREADED),
> BIT_MASK_DESCR(IRQCHIP_SUPPORTS_LEVEL_MSI),
> + BIT_MASK_DESCR(IRQCHIP_SUPPORTS_NMI),
> };
>
> static void
> @@ -140,6 +141,7 @@ static void irq_debug_show_masks(struct seq_file *m,
> struct irq_desc *desc) { }
> BIT_MASK_DESCR(IRQS_WAITING),
> BIT_MASK_DESCR(IRQS_PENDING),
>
Help..
I need your help which will be of mutual benefit, please reply for further details. Thank You De informatie verzonden in dit e-mailbericht is voor zover mogelijk in overeenstemming met de waarden van OSG Sevenwolden, vertrouwelijk en is uitsluitend bestemd voor de geadresseerde(n). Openbaarmaking, vermenigvuldiging, verspreiding en/of verstrekking van deze informatie aan derden is, behoudens voorafgaande toestemming van OSG Sevenwolden niet toegestaan. OSG Sevenwolden is op geen enkele wijze aansprakelijk voor eventuele fouten, omissies of andere onjuistheden in deze informatie of de gevolgen daarvan, en is - specifieke gevallen uitgezonderd - niet aan de inhoud van deze e-mail op enigerlei wijze gebonden. Indien bovenstaand e-mailbericht niet aan u is gericht, verzoeken wij u vriendelijk doch dringend het e-mailbericht te retourneren aan de verzender en het origineel en eventuele kopie?n te verwijderen en te vernietigen. Hoewel alle bijlagen voor verzending worden gescand kunnen wij niet garanderen dat deze virusvrij zijn.
Help..
I need your help which will be of mutual benefit, please reply for further details. Thank You De informatie verzonden in dit e-mailbericht is voor zover mogelijk in overeenstemming met de waarden van OSG Sevenwolden, vertrouwelijk en is uitsluitend bestemd voor de geadresseerde(n). Openbaarmaking, vermenigvuldiging, verspreiding en/of verstrekking van deze informatie aan derden is, behoudens voorafgaande toestemming van OSG Sevenwolden niet toegestaan. OSG Sevenwolden is op geen enkele wijze aansprakelijk voor eventuele fouten, omissies of andere onjuistheden in deze informatie of de gevolgen daarvan, en is - specifieke gevallen uitgezonderd - niet aan de inhoud van deze e-mail op enigerlei wijze gebonden. Indien bovenstaand e-mailbericht niet aan u is gericht, verzoeken wij u vriendelijk doch dringend het e-mailbericht te retourneren aan de verzender en het origineel en eventuele kopie?n te verwijderen en te vernietigen. Hoewel alle bijlagen voor verzending worden gescand kunnen wij niet garanderen dat deze virusvrij zijn.
Re: [PATCH] timers: fix offset calculation when the expires align with LVL_GRAN
On 2018/7/31 22:34, Thomas Gleixner wrote:
>
>
> On Tue, 31 Jul 2018, Thomas Gleixner wrote:
>
>> On Tue, 31 Jul 2018, Xu YiPing wrote:
>>> On 2018/7/30 19:03, Thomas Gleixner wrote:
__internal_add_timer(base, timer)
{
idx = calc_wheel_index(1, 1)
{
delta = 1 - 1; <- 0
if (0 < LVL_START(1))
idx = calc_index(1, 0)
{
expires = (1 + LVL_GRAN(0) - 1) >> LVL_SHIFT(0);
> expires = 0
>>>
>>> LVL_GRAN(0) = 1 and LVL_SHIFT(0) = 0
>>>
>>> after the calculation, expires = 1 ?
>>
>> Indeed. You're right. Math is hard... So the index would be 1 and still not
>> fulfil the below:
>
> Hmm, crap. Let me think about it some more. 34C is way too hot to think.
>
when the expire is aligned with LVL_GRAN(x), it could be triggered at
expire,
no need to wait another LVL_GRAN(x).
just a little improvement, useful when expire is small.
> Thanks,
>
> tglx
>
> .
>
Re: [PATCH] timers: fix offset calculation when the expires align with LVL_GRAN
On 2018/7/31 22:34, Thomas Gleixner wrote:
>
>
> On Tue, 31 Jul 2018, Thomas Gleixner wrote:
>
>> On Tue, 31 Jul 2018, Xu YiPing wrote:
>>> On 2018/7/30 19:03, Thomas Gleixner wrote:
__internal_add_timer(base, timer)
{
idx = calc_wheel_index(1, 1)
{
delta = 1 - 1; <- 0
if (0 < LVL_START(1))
idx = calc_index(1, 0)
{
expires = (1 + LVL_GRAN(0) - 1) >> LVL_SHIFT(0);
> expires = 0
>>>
>>> LVL_GRAN(0) = 1 and LVL_SHIFT(0) = 0
>>>
>>> after the calculation, expires = 1 ?
>>
>> Indeed. You're right. Math is hard... So the index would be 1 and still not
>> fulfil the below:
>
> Hmm, crap. Let me think about it some more. 34C is way too hot to think.
>
when the expire is aligned with LVL_GRAN(x), it could be triggered at
expire,
no need to wait another LVL_GRAN(x).
just a little improvement, useful when expire is small.
> Thanks,
>
> tglx
>
> .
>
