Re: [PATCH v6 0/5] clk: Add Aspeed clock driver

[Joel Stanley]

Hello clk maintainers,

Has anyone had a chance to review these patches?

On Tue, Nov 28, 2017 at 5:49 PM, Joel Stanley  wrote:
> This driver supports the ast2500, ast2400 (and derivative) BMC SoCs from
> Aspeed.
>
> This is v6. See patches for detailed changelogs.
>
> v6: Added reviewed-bys
> v5: Address review from Andrew
> v4: Address review from Andrew and Stephen.
> v3: Address review from Andrew and has seen more testing on hardware
> v2: split the driver out into a series of patches to make them easier to
> review.
>
> All of the important clocks are supported, with most non-essential ones
> also implemented where information is available. I am working with
> Aspeed to clear up some of the missing information, including the
> missing parent-sibling relationships.
>
> We need to know the rate of the apb clock in order to correctly program
> the clocksource driver, so the apb and it's parents are created in the
> CLK_OF_DECLARE_DRIVER callback.
>
> The rest of the clocks are created at normal driver probe time. I
> followed the Gemini driver's lead with using the regmap where I could,
> but also having a pointer to the base address for use with the common
> clock callbacks.
>
> The driver borrows from the clk_gate common clock infrastructure, but modifies
> it in order to support the clock gate and reset pair that most of the clocks
> have. This pair must be reset-ungated-released, with appropriate delays,
> according to the datasheet.
>
> The first patch introduces the core clock registration parts, and describes
> the clocks. The second creates the core clocks, giving the system enough to
> boot (but without uart). Next come the non-core clocks, and finally the reset
> controller that is used for the few cocks that don't have a gate to go with 
> their
> reset pair.
>
> Please review!
>
> Cheers,
>
> Joel
>
> Joel Stanley (5):
>   clk: Add clock driver for ASPEED BMC SoCs
>   clk: aspeed: Register core clocks
>   clk: aspeed: Add platform driver and register PLLs
>   clk: aspeed: Register gated clocks
>   clk: aspeed: Add reset controller
>
>  drivers/clk/Kconfig  |  12 +
>  drivers/clk/Makefile |   1 +
>  drivers/clk/clk-aspeed.c | 657 
> +++
>  include/dt-bindings/clock/aspeed-clock.h |  54 +++
>  4 files changed, 724 insertions(+)
>  create mode 100644 drivers/clk/clk-aspeed.c
>  create mode 100644 include/dt-bindings/clock/aspeed-clock.h
>
> --
> 2.14.1
>

Re: [PATCH v6 0/5] clk: Add Aspeed clock driver

[Joel Stanley]

Hello clk maintainers,

Has anyone had a chance to review these patches?

On Tue, Nov 28, 2017 at 5:49 PM, Joel Stanley  wrote:
> This driver supports the ast2500, ast2400 (and derivative) BMC SoCs from
> Aspeed.
>
> This is v6. See patches for detailed changelogs.
>
> v6: Added reviewed-bys
> v5: Address review from Andrew
> v4: Address review from Andrew and Stephen.
> v3: Address review from Andrew and has seen more testing on hardware
> v2: split the driver out into a series of patches to make them easier to
> review.
>
> All of the important clocks are supported, with most non-essential ones
> also implemented where information is available. I am working with
> Aspeed to clear up some of the missing information, including the
> missing parent-sibling relationships.
>
> We need to know the rate of the apb clock in order to correctly program
> the clocksource driver, so the apb and it's parents are created in the
> CLK_OF_DECLARE_DRIVER callback.
>
> The rest of the clocks are created at normal driver probe time. I
> followed the Gemini driver's lead with using the regmap where I could,
> but also having a pointer to the base address for use with the common
> clock callbacks.
>
> The driver borrows from the clk_gate common clock infrastructure, but modifies
> it in order to support the clock gate and reset pair that most of the clocks
> have. This pair must be reset-ungated-released, with appropriate delays,
> according to the datasheet.
>
> The first patch introduces the core clock registration parts, and describes
> the clocks. The second creates the core clocks, giving the system enough to
> boot (but without uart). Next come the non-core clocks, and finally the reset
> controller that is used for the few cocks that don't have a gate to go with 
> their
> reset pair.
>
> Please review!
>
> Cheers,
>
> Joel
>
> Joel Stanley (5):
>   clk: Add clock driver for ASPEED BMC SoCs
>   clk: aspeed: Register core clocks
>   clk: aspeed: Add platform driver and register PLLs
>   clk: aspeed: Register gated clocks
>   clk: aspeed: Add reset controller
>
>  drivers/clk/Kconfig  |  12 +
>  drivers/clk/Makefile |   1 +
>  drivers/clk/clk-aspeed.c | 657 
> +++
>  include/dt-bindings/clock/aspeed-clock.h |  54 +++
>  4 files changed, 724 insertions(+)
>  create mode 100644 drivers/clk/clk-aspeed.c
>  create mode 100644 include/dt-bindings/clock/aspeed-clock.h
>
> --
> 2.14.1
>

Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

['Greg KH']

On Wed, Dec 06, 2017 at 06:01:37AM +, Dhaval Rajeshbhai Shah wrote:
> Hi Greg k-h,
> 
> Thanks a lot for the review.
> 
> Replies inline.

As they should be, perhaps you need a better email client :)

> 
> > +config XILINX_VCU
> > +   tristate "Xilinx VCU Init"
> > +   default n
> 
> That's always the default, no need for this.
> [Dhaval ] : I will remove that. 

This style of replying is very odd, please use the normal format in the
future.

> > +   help
> > +  Driver for the Xilinx VCU Init based on the logicoreIP.
> 
> You need a lot more help text here to explain what this driver is, what it is 
> for, and who would need it.
> [Dhaval ] : I will provide more help text to provide more help on driver. 
> 
> Also, why is this a misc driver?
> [Dhaval ] :  this driver is for the logicoreIP which is created to support 
> the Processing system and Programmable logic isolation and to provide the 
> clock related information. So this is not a VCU driver and but just a 
> intermediate driver which supports logicoreIP. That's why no subsystem for 
> this.

Then you need to explain this a lot better, posting a random driver
for submission that is expected to be used by another one isn't ok.
Post the whole patch series please, we do not add infrastructure to the
kernel that is not used right then.

thanks,

greg k-h

Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

['Greg KH']

On Wed, Dec 06, 2017 at 06:01:37AM +, Dhaval Rajeshbhai Shah wrote:
> Hi Greg k-h,
> 
> Thanks a lot for the review.
> 
> Replies inline.

As they should be, perhaps you need a better email client :)

> 
> > +config XILINX_VCU
> > +   tristate "Xilinx VCU Init"
> > +   default n
> 
> That's always the default, no need for this.
> [Dhaval ] : I will remove that. 

This style of replying is very odd, please use the normal format in the
future.

> > +   help
> > +  Driver for the Xilinx VCU Init based on the logicoreIP.
> 
> You need a lot more help text here to explain what this driver is, what it is 
> for, and who would need it.
> [Dhaval ] : I will provide more help text to provide more help on driver. 
> 
> Also, why is this a misc driver?
> [Dhaval ] :  this driver is for the logicoreIP which is created to support 
> the Processing system and Programmable logic isolation and to provide the 
> clock related information. So this is not a VCU driver and but just a 
> intermediate driver which supports logicoreIP. That's why no subsystem for 
> this.

Then you need to explain this a lot better, posting a random driver
for submission that is expected to be used by another one isn't ok.
Post the whole patch series please, we do not add infrastructure to the
kernel that is not used right then.

thanks,

greg k-h

Re: [PATCH V2] clk: fix a panic error caused by accessing NULL pointer

[Chunyan Zhang]

On 6 December 2017 at 07:28, Stephen Boyd  wrote:
> On 11/21, Chunyan Zhang wrote:
>> From: Cai Li 
>>
>> In some cases the clock parent would be set NULL when doing re-parent,
>> it will cause a NULL pointer accessing if clk_set trace event is
>> enabled.
>>
>> This patch sets the parent as "none" if the input parameter is NULL.
>>
>> Fixes: dfc202ead312 (clk: Add tracepoints for hardware operations)
>> Signed-off-by: Cai Li 
>> Signed-off-by: Chunyan Zhang 
>> ---
>
> Applied to clk-fixes

Thanks!

Chunyan

>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: [PATCH V2] clk: fix a panic error caused by accessing NULL pointer

[Chunyan Zhang]

On 6 December 2017 at 07:28, Stephen Boyd  wrote:
> On 11/21, Chunyan Zhang wrote:
>> From: Cai Li 
>>
>> In some cases the clock parent would be set NULL when doing re-parent,
>> it will cause a NULL pointer accessing if clk_set trace event is
>> enabled.
>>
>> This patch sets the parent as "none" if the input parameter is NULL.
>>
>> Fixes: dfc202ead312 (clk: Add tracepoints for hardware operations)
>> Signed-off-by: Cai Li 
>> Signed-off-by: Chunyan Zhang 
>> ---
>
> Applied to clk-fixes

Thanks!

Chunyan

>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

[David Rientjes]

On Wed, 6 Dec 2017, Tetsuo Handa wrote:

> > > One way to solve the issue is to have two mm flags: one to indicate the 
> > > mm 
> > > is entering unmap_vmas(): set the flag, do down_write(>mmap_sem); 
> > > up_write(>mmap_sem), then unmap_vmas().  The oom reaper needs this 
> > > flag clear, not MMF_OOM_SKIP, while holding down_read(>mmap_sem) to 
> > > be 
> > > allowed to call unmap_page_range().  The oom killer will still defer 
> > > selecting this victim for MMF_OOM_SKIP after unmap_vmas() returns.
> > > 
> > > The result of that change would be that we do not oom reap from any mm 
> > > entering unmap_vmas(): we let unmap_vmas() do the work itself and avoid 
> > > racing with it.
> > > 
> > 
> > I think we need something like the following?
> 
> This patch does not work. __oom_reap_task_mm() can find MMF_REAPING and
> return true and sets MMF_OOM_SKIP before exit_mmap() calls down_write().
> 

Ah, you're talking about oom_reap_task() setting MMF_OOM_SKIP prematurely 
and allowing for additional oom kills?  I see your point, but I was mainly 
focused on preventing the panic as the first order of business.  We could 
certainly fix oom_reap_task() to not set MMF_OOM_SKIP itself and rather 
leave that to exit_mmap() if it finds MMF_REAPING if your concern matches 
my understanding.

> Also, I don't know what exit_mmap() is doing but I think that there is a
> possibility that the OOM reaper tries to reclaim mlocked pages as soon as
> exit_mmap() cleared VM_LOCKED flag by calling munlock_vma_pages_all().
> 
>   if (mm->locked_vm) {
>   vma = mm->mmap;
>   while (vma) {
>   if (vma->vm_flags & VM_LOCKED)
>   munlock_vma_pages_all(vma);
>   vma = vma->vm_next;
>   }
>   }
> 

Yes, that looks possible as well, although the problem I have reported can 
happen with or without mlock.  Did you find this by code inspection or 
have you experienced runtime problems with it?

I think this argues to do MMF_REAPING-style behavior at the beginning of 
exit_mmap() and avoid reaping all together once we have reached that 
point.  There are no more users of the mm and we are in the process of 
tearing it down, I'm not sure that the oom reaper should be in the 
business with trying to interfere with that.  Or are there actual bug 
reports where an oom victim gets wedged while in exit_mmap() prior to 
releasing its memory?

I know this conflicts with your patches in -mm to remove the oom mutex, 
but I think we should make sure we can prevent crashes before cleaning it 
up.

(I also noticed that the mm_has_notifiers() check is missing a 
trace_skip_task_reaping(tsk->pid))
---
diff --git a/include/linux/sched/coredump.h b/include/linux/sched/coredump.h
--- a/include/linux/sched/coredump.h
+++ b/include/linux/sched/coredump.h
@@ -70,6 +70,7 @@ static inline int get_dumpable(struct mm_struct *mm)
 #define MMF_UNSTABLE   22  /* mm is unstable for copy_from_user */
 #define MMF_HUGE_ZERO_PAGE 23  /* mm has ever used the global huge 
zero page */
 #define MMF_DISABLE_THP24  /* disable THP for all VMAs */
+#define MMF_REAPING25  /* mm is undergoing reaping */
 #define MMF_DISABLE_THP_MASK   (1 << MMF_DISABLE_THP)
 
 #define MMF_INIT_MASK  (MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
diff --git a/mm/mmap.c b/mm/mmap.c
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2996,6 +2996,23 @@ void exit_mmap(struct mm_struct *mm)
 
/* mm's last user has gone, and its about to be pulled down */
mmu_notifier_release(mm);
+   set_bit(MMF_REAPING, >flags);
+   if (unlikely(tsk_is_oom_victim(current))) {
+   /*
+* Wait for oom_reap_task() to stop working on this
+* mm. Because MMF_REAPING is already set before
+* calling down_read(), oom_reap_task() will not run
+* on this "mm" post up_write().
+*
+* tsk_is_oom_victim() cannot be set from under us
+* either because current->mm is already set to NULL
+* under task_lock before calling mmput and oom_mm is
+* set not NULL by the OOM killer only if current->mm
+* is found not NULL while holding the task_lock.
+*/
+   down_write(>mmap_sem);
+   up_write(>mmap_sem);
+   }
 
if (mm->locked_vm) {
vma = mm->mmap;
@@ -3018,26 +3035,9 @@ void exit_mmap(struct mm_struct *mm)
/* update_hiwater_rss(mm) here? but nobody should be looking */
/* Use -1 here to ensure all VMAs in the mm are unmapped */
unmap_vmas(, vma, 0, -1);
-
-   set_bit(MMF_OOM_SKIP, >flags);
-   if (unlikely(tsk_is_oom_victim(current))) {
-   /*
-* Wait for oom_reap_task() to stop working on this
-* mm. Because MMF_OOM_SKIP is already set before

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

[David Rientjes]

On Wed, 6 Dec 2017, Tetsuo Handa wrote:

> > > One way to solve the issue is to have two mm flags: one to indicate the 
> > > mm 
> > > is entering unmap_vmas(): set the flag, do down_write(>mmap_sem); 
> > > up_write(>mmap_sem), then unmap_vmas().  The oom reaper needs this 
> > > flag clear, not MMF_OOM_SKIP, while holding down_read(>mmap_sem) to 
> > > be 
> > > allowed to call unmap_page_range().  The oom killer will still defer 
> > > selecting this victim for MMF_OOM_SKIP after unmap_vmas() returns.
> > > 
> > > The result of that change would be that we do not oom reap from any mm 
> > > entering unmap_vmas(): we let unmap_vmas() do the work itself and avoid 
> > > racing with it.
> > > 
> > 
> > I think we need something like the following?
> 
> This patch does not work. __oom_reap_task_mm() can find MMF_REAPING and
> return true and sets MMF_OOM_SKIP before exit_mmap() calls down_write().
> 

Ah, you're talking about oom_reap_task() setting MMF_OOM_SKIP prematurely 
and allowing for additional oom kills?  I see your point, but I was mainly 
focused on preventing the panic as the first order of business.  We could 
certainly fix oom_reap_task() to not set MMF_OOM_SKIP itself and rather 
leave that to exit_mmap() if it finds MMF_REAPING if your concern matches 
my understanding.

> Also, I don't know what exit_mmap() is doing but I think that there is a
> possibility that the OOM reaper tries to reclaim mlocked pages as soon as
> exit_mmap() cleared VM_LOCKED flag by calling munlock_vma_pages_all().
> 
>   if (mm->locked_vm) {
>   vma = mm->mmap;
>   while (vma) {
>   if (vma->vm_flags & VM_LOCKED)
>   munlock_vma_pages_all(vma);
>   vma = vma->vm_next;
>   }
>   }
> 

Yes, that looks possible as well, although the problem I have reported can 
happen with or without mlock.  Did you find this by code inspection or 
have you experienced runtime problems with it?

I think this argues to do MMF_REAPING-style behavior at the beginning of 
exit_mmap() and avoid reaping all together once we have reached that 
point.  There are no more users of the mm and we are in the process of 
tearing it down, I'm not sure that the oom reaper should be in the 
business with trying to interfere with that.  Or are there actual bug 
reports where an oom victim gets wedged while in exit_mmap() prior to 
releasing its memory?

I know this conflicts with your patches in -mm to remove the oom mutex, 
but I think we should make sure we can prevent crashes before cleaning it 
up.

(I also noticed that the mm_has_notifiers() check is missing a 
trace_skip_task_reaping(tsk->pid))
---
diff --git a/include/linux/sched/coredump.h b/include/linux/sched/coredump.h
--- a/include/linux/sched/coredump.h
+++ b/include/linux/sched/coredump.h
@@ -70,6 +70,7 @@ static inline int get_dumpable(struct mm_struct *mm)
 #define MMF_UNSTABLE   22  /* mm is unstable for copy_from_user */
 #define MMF_HUGE_ZERO_PAGE 23  /* mm has ever used the global huge 
zero page */
 #define MMF_DISABLE_THP24  /* disable THP for all VMAs */
+#define MMF_REAPING25  /* mm is undergoing reaping */
 #define MMF_DISABLE_THP_MASK   (1 << MMF_DISABLE_THP)
 
 #define MMF_INIT_MASK  (MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
diff --git a/mm/mmap.c b/mm/mmap.c
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2996,6 +2996,23 @@ void exit_mmap(struct mm_struct *mm)
 
/* mm's last user has gone, and its about to be pulled down */
mmu_notifier_release(mm);
+   set_bit(MMF_REAPING, >flags);
+   if (unlikely(tsk_is_oom_victim(current))) {
+   /*
+* Wait for oom_reap_task() to stop working on this
+* mm. Because MMF_REAPING is already set before
+* calling down_read(), oom_reap_task() will not run
+* on this "mm" post up_write().
+*
+* tsk_is_oom_victim() cannot be set from under us
+* either because current->mm is already set to NULL
+* under task_lock before calling mmput and oom_mm is
+* set not NULL by the OOM killer only if current->mm
+* is found not NULL while holding the task_lock.
+*/
+   down_write(>mmap_sem);
+   up_write(>mmap_sem);
+   }
 
if (mm->locked_vm) {
vma = mm->mmap;
@@ -3018,26 +3035,9 @@ void exit_mmap(struct mm_struct *mm)
/* update_hiwater_rss(mm) here? but nobody should be looking */
/* Use -1 here to ensure all VMAs in the mm are unmapped */
unmap_vmas(, vma, 0, -1);
-
-   set_bit(MMF_OOM_SKIP, >flags);
-   if (unlikely(tsk_is_oom_victim(current))) {
-   /*
-* Wait for oom_reap_task() to stop working on this
-* mm. Because MMF_OOM_SKIP is already set before

Re: [PATCH 4.4 02/96] s390/runtime instrumention: fix possible memory corruption

[Greg Kroah-Hartman]

On Tue, Dec 05, 2017 at 07:15:34PM +0100, Heiko Carstens wrote:
> On Tue, Dec 05, 2017 at 06:08:47PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Dec 05, 2017 at 05:02:32PM +, Ben Hutchings wrote:
> > > On Tue, 2017-11-28 at 11:22 +0100, Greg Kroah-Hartman wrote:
> > > > 4.4-stable review patch.  If anyone has any objections, please let me 
> > > > know.
> > > > 
> > > > --
> > > > 
> > > > From: Heiko Carstens 
> > > > 
> > > > commit d6e646ad7cfa7034d280459b2b2546288f247144 upstream.
> > > [...]
> > > > --- a/arch/s390/kernel/runtime_instr.c
> > > > +++ b/arch/s390/kernel/runtime_instr.c
> > > > @@ -47,11 +47,13 @@ void exit_thread_runtime_instr(void)
> > > >  {
> > > >     struct task_struct *task = current;
> > > >  
> > > > +   preempt_disable();
> > > >     if (!task->thread.ri_cb)
> > > >     return;
> > > 
> > > This return path now leaves preemption disabled.  This seems to have
> > > been fixed upstream by commit 8d9047f8b967 "s390/runtime
> > > instrumentation: simplify task exit handling".
> > 
> > "simplify" doesn't seem to imply "fixes a bug" :)
> 
> Indeed ;) That where two subsequent patches, but incorrectly split by me...
> 
> > Heiko, should I also queue this patch up?
> 
> Yes, please.

It doesn't apply to 4.9-stable or 4.4-stable, can you provide a working
backport?

thanks,

greg k-h

Re: [PATCH 4.4 02/96] s390/runtime instrumention: fix possible memory corruption

[Greg Kroah-Hartman]

On Tue, Dec 05, 2017 at 07:15:34PM +0100, Heiko Carstens wrote:
> On Tue, Dec 05, 2017 at 06:08:47PM +0100, Greg Kroah-Hartman wrote:
> > On Tue, Dec 05, 2017 at 05:02:32PM +, Ben Hutchings wrote:
> > > On Tue, 2017-11-28 at 11:22 +0100, Greg Kroah-Hartman wrote:
> > > > 4.4-stable review patch.  If anyone has any objections, please let me 
> > > > know.
> > > > 
> > > > --
> > > > 
> > > > From: Heiko Carstens 
> > > > 
> > > > commit d6e646ad7cfa7034d280459b2b2546288f247144 upstream.
> > > [...]
> > > > --- a/arch/s390/kernel/runtime_instr.c
> > > > +++ b/arch/s390/kernel/runtime_instr.c
> > > > @@ -47,11 +47,13 @@ void exit_thread_runtime_instr(void)
> > > >  {
> > > >     struct task_struct *task = current;
> > > >  
> > > > +   preempt_disable();
> > > >     if (!task->thread.ri_cb)
> > > >     return;
> > > 
> > > This return path now leaves preemption disabled.  This seems to have
> > > been fixed upstream by commit 8d9047f8b967 "s390/runtime
> > > instrumentation: simplify task exit handling".
> > 
> > "simplify" doesn't seem to imply "fixes a bug" :)
> 
> Indeed ;) That where two subsequent patches, but incorrectly split by me...
> 
> > Heiko, should I also queue this patch up?
> 
> Yes, please.

It doesn't apply to 4.9-stable or 4.4-stable, can you provide a working
backport?

thanks,

greg k-h

Re: [PATCH] clk: fix a panic error caused by accessing NULL pointer

[Chunyan Zhang]

On 6 December 2017 at 07:28, Stephen Boyd  wrote:
> On 11/21, Chunyan Zhang wrote:
>> On 21 November 2017 at 16:57, Chunyan Zhang  wrote:
>> > On 21 November 2017 at 03:12, Stephen Boyd  wrote:
>> >> On 11/20, Chunyan Zhang wrote:
>> >>> From: Cai Li 
>> >>>
>> >>> In some cases the clock parent would be set NULL when doing re-parent,
>> >>> it will cause a NULL pointer accessing if clk_set trace event is enabled,
>> >>> since the trace event function would not check the input parameter.
>> >>>
>> >>> Signed-off-by: Cai Li 
>> >>> Signed-off-by: Chunyan Zhang 
>> >>
>> >> Fixes: tag?
>> >>
>> >>> ---
>> >>>  drivers/clk/clk.c | 9 -
>> >>>  1 file changed, 4 insertions(+), 5 deletions(-)
>> >>>
>> >>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> >>> index c8d83ac..64efaf0 100644
>> >>> --- a/drivers/clk/clk.c
>> >>> +++ b/drivers/clk/clk.c
>> >>> @@ -1242,13 +1242,12 @@ static int __clk_set_parent(struct clk_core 
>> >>> *core, struct clk_core *parent,
>> >>>
>> >>>   old_parent = __clk_set_parent_before(core, parent);
>> >>>
>> >>> - trace_clk_set_parent(core, parent);
>> >>> -
>> >>>   /* change clock input source */
>> >>> - if (parent && core->ops->set_parent)
>> >>> + if (parent && core->ops->set_parent) {
>> >>> + trace_clk_set_parent(core, parent);
>> >>>   ret = core->ops->set_parent(core->hw, p_index);
>> >>> -
>> >>> - trace_clk_set_parent_complete(core, parent);
>> >>> + trace_clk_set_parent_complete(core, parent);
>> >>> + }
>> >>
>> >> Is the problem that parent may be NULL and the tracepoint
>> >> dereferences it?
>> >
>> > Yes, I think that probably is uncommon usage though, it revealed that
>> > the tracepoint could be stronger :)
>>
>> The reason we need to set the parent as NULL is to disable the clk for
>> the purpose of saving power.
>
> Do you have drivers calling set_parent with NULL to save power?
> Seems sort of odd. Why not do something when disabling the clk in

It's a mux clk in the same address area with a device, when powerring
off the device module, the registers of this mux are reset with
default value by hardware.  When the device resumed afterward, the mux
clock needs to recover its previous clk parent, but the clk core would
find it is setting a same clk parent, the function
clk_core_set_parent() [1] will return directly without writing the
exact register to set the parent again.

So setting the clk parent with NULL is just a workaround, we will
appreciate if you have other better solution for us.

Thanks,
Chunyan

[1] 
http://elixir.free-electrons.com/linux/v4.15-rc1/source/drivers/clk/clk.c#L1886

> clk_disable() path instead? Either way, I'll apply the patch to
> clk-fixes.
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: [PATCH] clk: fix a panic error caused by accessing NULL pointer

[Chunyan Zhang]

On 6 December 2017 at 07:28, Stephen Boyd  wrote:
> On 11/21, Chunyan Zhang wrote:
>> On 21 November 2017 at 16:57, Chunyan Zhang  wrote:
>> > On 21 November 2017 at 03:12, Stephen Boyd  wrote:
>> >> On 11/20, Chunyan Zhang wrote:
>> >>> From: Cai Li 
>> >>>
>> >>> In some cases the clock parent would be set NULL when doing re-parent,
>> >>> it will cause a NULL pointer accessing if clk_set trace event is enabled,
>> >>> since the trace event function would not check the input parameter.
>> >>>
>> >>> Signed-off-by: Cai Li 
>> >>> Signed-off-by: Chunyan Zhang 
>> >>
>> >> Fixes: tag?
>> >>
>> >>> ---
>> >>>  drivers/clk/clk.c | 9 -
>> >>>  1 file changed, 4 insertions(+), 5 deletions(-)
>> >>>
>> >>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> >>> index c8d83ac..64efaf0 100644
>> >>> --- a/drivers/clk/clk.c
>> >>> +++ b/drivers/clk/clk.c
>> >>> @@ -1242,13 +1242,12 @@ static int __clk_set_parent(struct clk_core 
>> >>> *core, struct clk_core *parent,
>> >>>
>> >>>   old_parent = __clk_set_parent_before(core, parent);
>> >>>
>> >>> - trace_clk_set_parent(core, parent);
>> >>> -
>> >>>   /* change clock input source */
>> >>> - if (parent && core->ops->set_parent)
>> >>> + if (parent && core->ops->set_parent) {
>> >>> + trace_clk_set_parent(core, parent);
>> >>>   ret = core->ops->set_parent(core->hw, p_index);
>> >>> -
>> >>> - trace_clk_set_parent_complete(core, parent);
>> >>> + trace_clk_set_parent_complete(core, parent);
>> >>> + }
>> >>
>> >> Is the problem that parent may be NULL and the tracepoint
>> >> dereferences it?
>> >
>> > Yes, I think that probably is uncommon usage though, it revealed that
>> > the tracepoint could be stronger :)
>>
>> The reason we need to set the parent as NULL is to disable the clk for
>> the purpose of saving power.
>
> Do you have drivers calling set_parent with NULL to save power?
> Seems sort of odd. Why not do something when disabling the clk in

It's a mux clk in the same address area with a device, when powerring
off the device module, the registers of this mux are reset with
default value by hardware.  When the device resumed afterward, the mux
clock needs to recover its previous clk parent, but the clk core would
find it is setting a same clk parent, the function
clk_core_set_parent() [1] will return directly without writing the
exact register to set the parent again.

So setting the clk parent with NULL is just a workaround, we will
appreciate if you have other better solution for us.

Thanks,
Chunyan

[1] 
http://elixir.free-electrons.com/linux/v4.15-rc1/source/drivers/clk/clk.c#L1886

> clk_disable() path instead? Either way, I'll apply the patch to
> clk-fixes.
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Florian Weimer]

On 12/06/2017 08:33 AM, John Hubbard wrote:

In that case, maybe:

 MAP_EXACT

? ...because that's the characteristic behavior.


Is that true?  mmap still silently rounding up the length to the page 
size, I assume, so even that name is misleading.


Thanks,
Florian

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Florian Weimer]

On 12/06/2017 08:33 AM, John Hubbard wrote:

In that case, maybe:

 MAP_EXACT

? ...because that's the characteristic behavior.


Is that true?  mmap still silently rounding up the length to the page 
size, I assume, so even that name is misleading.


Thanks,
Florian

Re: [PATCH] doc: convert printk-formats.txt to rst

[Joe Perches]

On Wed, 2017-12-06 at 08:11 +0100, Markus Heiser wrote:
> > Am 06.12.2017 um 02:45 schrieb Tobin C. Harding :
> > Documentation/printk-formats.txt is a candidate for conversion to
> > ReStructuredText format. Some effort has already been made to do this
> > conversion even thought the suffix is currently .txt
[]
> just a question .. might it be better we stay with ASCII table
> in cases like this. I guess this table won't changed often.
> The flat-table directive is good for big and therefore frequently 
> changed tables where a small precise diff reduce the patch.
> But flat-table is also hard to read in plain text. Its a balancing
> and thats my opinion, lets hear what other say ...

I think the proposed conversion is unreadable
and the original table quite clear.

Re: [PATCH] doc: convert printk-formats.txt to rst

[Joe Perches]

On Wed, 2017-12-06 at 08:11 +0100, Markus Heiser wrote:
> > Am 06.12.2017 um 02:45 schrieb Tobin C. Harding :
> > Documentation/printk-formats.txt is a candidate for conversion to
> > ReStructuredText format. Some effort has already been made to do this
> > conversion even thought the suffix is currently .txt
[]
> just a question .. might it be better we stay with ASCII table
> in cases like this. I guess this table won't changed often.
> The flat-table directive is good for big and therefore frequently 
> changed tables where a small precise diff reduce the patch.
> But flat-table is also hard to read in plain text. Its a balancing
> and thats my opinion, lets hear what other say ...

I think the proposed conversion is unreadable
and the original table quite clear.

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[John Hubbard]

On 12/05/2017 11:03 PM, Matthew Wilcox wrote:
> On Tue, Dec 05, 2017 at 08:54:35PM -0800, Matthew Wilcox wrote:
>> On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
>>> Cyril Hrubis  writes:
>>>
 Hi!
>> MAP_FIXED_UNIQUE
>> MAP_FIXED_ONCE
>> MAP_FIXED_FRESH
>
> Well, I can open a poll for the best name, but none of those you are
> proposing sound much better to me. Yeah, naming sucks...

 Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
 would probably be a best fit.
>>>
>>> Yeah that could work.
>>>
>>> I prefer "no clobber" as I just suggested, because the existing
>>> MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
>>> one - which you or another thread may be using - and clobbers it with
>>> the new one.
>>
>> It's longer than MAP_FIXED_WEAK :-P
>>
>> You'd have to be pretty darn strong to clobber an existing mapping.
> 
> I think we're thinking about this all wrong.  We shouldn't document it as
> "This is a variant of MAP_FIXED".  We should document it as "Here's an
> alternative to MAP_FIXED".
> 
> So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
> we could add a new paragraph saying "at most one of MAP_FIXED or
> MAP_REQUIRED" and "any of the following values".
> 
> Now, we should implement MAP_REQUIRED as having each architecture
> define _MAP_NOT_A_HINT, and then #define MAP_REQUIRED (MAP_FIXED |
> _MAP_NOT_A_HINT), but that's not information to confuse users with.
> 
> Also, that lets us add a third option at some point that is Yet Another
> Way to interpret the 'addr' argument, by having MAP_FIXED clear and
> _MAP_NOT_A_HINT set.
> 
> I'm not set on MAP_REQUIRED.  I came up with some awful names
> (MAP_TODDLER, MAP_TANTRUM, MAP_ULTIMATUM, MAP_BOSS, MAP_PROGRAM_MANAGER,
> etc).  But I think we should drop FIXED from the middle of the name.
> 

In that case, maybe:

MAP_EXACT

? ...because that's the characteristic behavior. It doesn't clobber, but
you don't need to say that in the name, now that we're not including
_FIXED_ in the middle.

thanks,
John Hubbard
NVIDIA

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[John Hubbard]

On 12/05/2017 11:03 PM, Matthew Wilcox wrote:
> On Tue, Dec 05, 2017 at 08:54:35PM -0800, Matthew Wilcox wrote:
>> On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
>>> Cyril Hrubis  writes:
>>>
 Hi!
>> MAP_FIXED_UNIQUE
>> MAP_FIXED_ONCE
>> MAP_FIXED_FRESH
>
> Well, I can open a poll for the best name, but none of those you are
> proposing sound much better to me. Yeah, naming sucks...

 Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
 would probably be a best fit.
>>>
>>> Yeah that could work.
>>>
>>> I prefer "no clobber" as I just suggested, because the existing
>>> MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
>>> one - which you or another thread may be using - and clobbers it with
>>> the new one.
>>
>> It's longer than MAP_FIXED_WEAK :-P
>>
>> You'd have to be pretty darn strong to clobber an existing mapping.
> 
> I think we're thinking about this all wrong.  We shouldn't document it as
> "This is a variant of MAP_FIXED".  We should document it as "Here's an
> alternative to MAP_FIXED".
> 
> So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
> we could add a new paragraph saying "at most one of MAP_FIXED or
> MAP_REQUIRED" and "any of the following values".
> 
> Now, we should implement MAP_REQUIRED as having each architecture
> define _MAP_NOT_A_HINT, and then #define MAP_REQUIRED (MAP_FIXED |
> _MAP_NOT_A_HINT), but that's not information to confuse users with.
> 
> Also, that lets us add a third option at some point that is Yet Another
> Way to interpret the 'addr' argument, by having MAP_FIXED clear and
> _MAP_NOT_A_HINT set.
> 
> I'm not set on MAP_REQUIRED.  I came up with some awful names
> (MAP_TODDLER, MAP_TANTRUM, MAP_ULTIMATUM, MAP_BOSS, MAP_PROGRAM_MANAGER,
> etc).  But I think we should drop FIXED from the middle of the name.
> 

In that case, maybe:

MAP_EXACT

? ...because that's the characteristic behavior. It doesn't clobber, but
you don't need to say that in the name, now that we're not including
_FIXED_ in the middle.

thanks,
John Hubbard
NVIDIA

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Rasmus Villemoes]

On 2017-12-06 05:50, Michael Ellerman wrote:
> Michal Hocko  writes:
> 
>> On Wed 29-11-17 14:25:36, Kees Cook wrote:
>> It is safe in a sense it doesn't perform any address space dangerous
>> operations. mmap is _inherently_ about the address space so the context
>> should be kind of clear.
> 
> So now you have to define what "dangerous" means.
> 
>>> MAP_FIXED_UNIQUE
>>> MAP_FIXED_ONCE
>>> MAP_FIXED_FRESH
>>
>> Well, I can open a poll for the best name, but none of those you are
>> proposing sound much better to me. Yeah, naming sucks...

I also don't like the _SAFE name - MAP_FIXED in itself isn't unsafe [1],
but I do agree that having a way to avoid clobbering (parts of) an
existing mapping is quite useful. Since we're bikeshedding names, how
about MAP_FIXED_EXCL, in analogy with the O_ flag.

[1] I like the analogy between MAP_FIXED and dup2 made in
.

Rasmus

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Rasmus Villemoes]

On 2017-12-06 05:50, Michael Ellerman wrote:
> Michal Hocko  writes:
> 
>> On Wed 29-11-17 14:25:36, Kees Cook wrote:
>> It is safe in a sense it doesn't perform any address space dangerous
>> operations. mmap is _inherently_ about the address space so the context
>> should be kind of clear.
> 
> So now you have to define what "dangerous" means.
> 
>>> MAP_FIXED_UNIQUE
>>> MAP_FIXED_ONCE
>>> MAP_FIXED_FRESH
>>
>> Well, I can open a poll for the best name, but none of those you are
>> proposing sound much better to me. Yeah, naming sucks...

I also don't like the _SAFE name - MAP_FIXED in itself isn't unsafe [1],
but I do agree that having a way to avoid clobbering (parts of) an
existing mapping is quite useful. Since we're bikeshedding names, how
about MAP_FIXED_EXCL, in analogy with the O_ flag.

[1] I like the analogy between MAP_FIXED and dup2 made in
.

Rasmus

[PATCH v6 3/6] kernel/reboot.c: export pm_power_off_prepare

[Oleksij Rempel]

Export pm_power_off_prepare. It is needed to implement power off on
Freescale/NXP iMX6 based boards with external power management
integrated circuit (PMIC).

Signed-off-by: Oleksij Rempel 
---
 kernel/reboot.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/reboot.c b/kernel/reboot.c
index bd30a973fe94..a6903bf772c7 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -49,6 +49,7 @@ int reboot_force;
  */
 
 void (*pm_power_off_prepare)(void);
+EXPORT_SYMBOL(pm_power_off_prepare);
 
 /**
  * emergency_restart - reboot the system
-- 
2.11.0

[PATCH v6 3/6] kernel/reboot.c: export pm_power_off_prepare

[Oleksij Rempel]

Export pm_power_off_prepare. It is needed to implement power off on
Freescale/NXP iMX6 based boards with external power management
integrated circuit (PMIC).

Signed-off-by: Oleksij Rempel 
---
 kernel/reboot.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/reboot.c b/kernel/reboot.c
index bd30a973fe94..a6903bf772c7 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -49,6 +49,7 @@ int reboot_force;
  */
 
 void (*pm_power_off_prepare)(void);
+EXPORT_SYMBOL(pm_power_off_prepare);
 
 /**
  * emergency_restart - reboot the system
-- 
2.11.0

[PATCH RESEND v6 0/6] provide power off support for iMX6 with external PMIC

[Oleksij Rempel]

2017.12.06:
Adding Linus. Probably there is no maintainer for this patch set.
No changes are made, tested on v4.15-rc1.

2017.10.27:
Last version of this patch set was send at 20 Jun 2017, this is a rebase against
kernel v4.14-rc6. Probably this set got lost. If I forgot to address some 
comments,
please point me.

This patch series is providing power off support for Freescale/NXP iMX6 based
boards with external power management integrated circuit (PMIC).

changes:
v6:
 - rename imx6_pm_poweroff to imx6_pm_stby_poweroff
 - fix "MPIC_STBY_REQ" typo in the comment.

v5:
 - remove useless includes from pm-imx6.c patch
 - add Acked-by to "regulator: pfuze100: add fsl,pmic-stby-poweroff property"
   patch

v4:
 - update comment in "regulator: pfuze100: add fsl,pmic-stby-poweroff ..."
   patch
 - add Acked-by to "ARM: imx6q: provide documentation for new ..."
   patch

v3:
 - set pm_power_off_prepare = NULL on .remove.
 - documentation and spelling fixes.
 - use %pf instead of lookup_symbol_name.

Oleksij Rempel (6):
  ARM: imx6q: provide documentation for new fsl,pmic-stby-poweroff
property
  ARM: imx6: register pm_power_off handler if "fsl,pmic-stby-poweroff"
is set
  kernel/reboot.c: export pm_power_off_prepare
  regulator: pfuze100: add fsl,pmic-stby-poweroff property
  regulator: pfuze100-regulator: provide pm_power_off_prepare handler
  ARM: dts: imx6: RIoTboard provide standby on power off option

 .../devicetree/bindings/clock/imx6q-clock.txt  |  8 ++
 .../devicetree/bindings/regulator/pfuze100.txt |  7 ++
 arch/arm/boot/dts/imx6dl-riotboard.dts |  5 ++
 arch/arm/mach-imx/pm-imx6.c| 25 ++
 drivers/regulator/pfuze100-regulator.c | 92 ++
 kernel/reboot.c|  1 +
 6 files changed, 138 insertions(+)

-- 
2.11.0

[PATCH v6 2/6] ARM: imx6: register pm_power_off handler if "fsl,pmic-stby-poweroff" is set

[Oleksij Rempel]

One of the Freescale recommended sequences for power off with external
PMIC is the following:
...
3.  SoC is programming PMIC for power off when standby is asserted.
4.  In CCM STOP mode, Standby is asserted, PMIC gates SoC supplies.

See:
http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM.pdf
page 5083

This patch implements step 4. of this sequence.

Signed-off-by: Oleksij Rempel 
---
 arch/arm/mach-imx/pm-imx6.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
index ecdf071653d4..24689260a2a5 100644
--- a/arch/arm/mach-imx/pm-imx6.c
+++ b/arch/arm/mach-imx/pm-imx6.c
@@ -604,6 +604,28 @@ static void __init imx6_pm_common_init(const struct 
imx6_pm_socdata
   IMX6Q_GPR1_GINT);
 }
 
+static void imx6_pm_stby_poweroff(void)
+{
+   imx6_set_lpm(STOP_POWER_OFF);
+   cpu_suspend(0, imx6q_suspend_finish);
+
+   mdelay(1000);
+
+   pr_emerg("Unable to poweroff system\n");
+}
+
+static int imx6_pm_stby_poweroff_probe(void)
+{
+   if (pm_power_off) {
+   pr_warn("%s: pm_power_off already claimed  %p %pf!\n",
+   __func__, pm_power_off, pm_power_off);
+   return -EBUSY;
+   }
+
+   pm_power_off = imx6_pm_stby_poweroff;
+   return 0;
+}
+
 void __init imx6_pm_ccm_init(const char *ccm_compat)
 {
struct device_node *np;
@@ -620,6 +642,9 @@ void __init imx6_pm_ccm_init(const char *ccm_compat)
val = readl_relaxed(ccm_base + CLPCR);
val &= ~BM_CLPCR_LPM;
writel_relaxed(val, ccm_base + CLPCR);
+
+   if (of_property_read_bool(np, "fsl,pmic-stby-poweroff"))
+   imx6_pm_stby_poweroff_probe();
 }
 
 void __init imx6q_pm_init(void)
-- 
2.11.0

[PATCH v6 2/6] ARM: imx6: register pm_power_off handler if "fsl,pmic-stby-poweroff" is set

[Oleksij Rempel]

One of the Freescale recommended sequences for power off with external
PMIC is the following:
...
3.  SoC is programming PMIC for power off when standby is asserted.
4.  In CCM STOP mode, Standby is asserted, PMIC gates SoC supplies.

See:
http://www.nxp.com/assets/documents/data/en/reference-manuals/IMX6DQRM.pdf
page 5083

This patch implements step 4. of this sequence.

Signed-off-by: Oleksij Rempel 
---
 arch/arm/mach-imx/pm-imx6.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c
index ecdf071653d4..24689260a2a5 100644
--- a/arch/arm/mach-imx/pm-imx6.c
+++ b/arch/arm/mach-imx/pm-imx6.c
@@ -604,6 +604,28 @@ static void __init imx6_pm_common_init(const struct 
imx6_pm_socdata
   IMX6Q_GPR1_GINT);
 }
 
+static void imx6_pm_stby_poweroff(void)
+{
+   imx6_set_lpm(STOP_POWER_OFF);
+   cpu_suspend(0, imx6q_suspend_finish);
+
+   mdelay(1000);
+
+   pr_emerg("Unable to poweroff system\n");
+}
+
+static int imx6_pm_stby_poweroff_probe(void)
+{
+   if (pm_power_off) {
+   pr_warn("%s: pm_power_off already claimed  %p %pf!\n",
+   __func__, pm_power_off, pm_power_off);
+   return -EBUSY;
+   }
+
+   pm_power_off = imx6_pm_stby_poweroff;
+   return 0;
+}
+
 void __init imx6_pm_ccm_init(const char *ccm_compat)
 {
struct device_node *np;
@@ -620,6 +642,9 @@ void __init imx6_pm_ccm_init(const char *ccm_compat)
val = readl_relaxed(ccm_base + CLPCR);
val &= ~BM_CLPCR_LPM;
writel_relaxed(val, ccm_base + CLPCR);
+
+   if (of_property_read_bool(np, "fsl,pmic-stby-poweroff"))
+   imx6_pm_stby_poweroff_probe();
 }
 
 void __init imx6q_pm_init(void)
-- 
2.11.0

[PATCH RESEND v6 0/6] provide power off support for iMX6 with external PMIC

[Oleksij Rempel]

2017.12.06:
Adding Linus. Probably there is no maintainer for this patch set.
No changes are made, tested on v4.15-rc1.

2017.10.27:
Last version of this patch set was send at 20 Jun 2017, this is a rebase against
kernel v4.14-rc6. Probably this set got lost. If I forgot to address some 
comments,
please point me.

This patch series is providing power off support for Freescale/NXP iMX6 based
boards with external power management integrated circuit (PMIC).

changes:
v6:
 - rename imx6_pm_poweroff to imx6_pm_stby_poweroff
 - fix "MPIC_STBY_REQ" typo in the comment.

v5:
 - remove useless includes from pm-imx6.c patch
 - add Acked-by to "regulator: pfuze100: add fsl,pmic-stby-poweroff property"
   patch

v4:
 - update comment in "regulator: pfuze100: add fsl,pmic-stby-poweroff ..."
   patch
 - add Acked-by to "ARM: imx6q: provide documentation for new ..."
   patch

v3:
 - set pm_power_off_prepare = NULL on .remove.
 - documentation and spelling fixes.
 - use %pf instead of lookup_symbol_name.

Oleksij Rempel (6):
  ARM: imx6q: provide documentation for new fsl,pmic-stby-poweroff
property
  ARM: imx6: register pm_power_off handler if "fsl,pmic-stby-poweroff"
is set
  kernel/reboot.c: export pm_power_off_prepare
  regulator: pfuze100: add fsl,pmic-stby-poweroff property
  regulator: pfuze100-regulator: provide pm_power_off_prepare handler
  ARM: dts: imx6: RIoTboard provide standby on power off option

 .../devicetree/bindings/clock/imx6q-clock.txt  |  8 ++
 .../devicetree/bindings/regulator/pfuze100.txt |  7 ++
 arch/arm/boot/dts/imx6dl-riotboard.dts |  5 ++
 arch/arm/mach-imx/pm-imx6.c| 25 ++
 drivers/regulator/pfuze100-regulator.c | 92 ++
 kernel/reboot.c|  1 +
 6 files changed, 138 insertions(+)

-- 
2.11.0

[PATCH v6 5/6] regulator: pfuze100-regulator: provide pm_power_off_prepare handler

[Oleksij Rempel]

On some boards the SoC can use one pin "PMIC_STBY_REQ" to notify th PMIC
about state changes. In this case internal state of PMIC must be
preconfigured for upcomming state change.
It works fine with the current regulator framework, except with the
power-off case.

This patch is providing an optional pm_power_off_prepare handler
which will configure standby state of the PMIC to disable all power lines.

In my power consumption test on RIoTBoard, I got the following results:
power off without this patch:   320 mA
power off with this patch:  2   mA
suspend to ram: 40  mA

Signed-off-by: Oleksij Rempel 
---
 drivers/regulator/pfuze100-regulator.c | 92 ++
 1 file changed, 92 insertions(+)

diff --git a/drivers/regulator/pfuze100-regulator.c 
b/drivers/regulator/pfuze100-regulator.c
index 63922a2167e5..f6c276ed91d8 100644
--- a/drivers/regulator/pfuze100-regulator.c
+++ b/drivers/regulator/pfuze100-regulator.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define PFUZE_NUMREGS  128
@@ -42,11 +43,17 @@
 
 #define PFUZE100_COINVOL   0x1a
 #define PFUZE100_SW1ABVOL  0x20
+#define PFUZE100_SW1ABMODE 0x23
 #define PFUZE100_SW1CVOL   0x2e
+#define PFUZE100_SW1CMODE  0x31
 #define PFUZE100_SW2VOL0x35
+#define PFUZE100_SW2MODE   0x38
 #define PFUZE100_SW3AVOL   0x3c
+#define PFUZE100_SW3AMODE  0x3f
 #define PFUZE100_SW3BVOL   0x43
+#define PFUZE100_SW3BMODE  0x46
 #define PFUZE100_SW4VOL0x4a
+#define PFUZE100_SW4MODE   0x4d
 #define PFUZE100_SWBSTCON1 0x66
 #define PFUZE100_VREFDDRCON0x6a
 #define PFUZE100_VSNVSVOL  0x6b
@@ -57,6 +64,13 @@
 #define PFUZE100_VGEN5VOL  0x70
 #define PFUZE100_VGEN6VOL  0x71
 
+#define PFUZE100_SWxMODE_MASK  0xf
+#define PFUZE100_SWxMODE_APS_APS   0x8
+#define PFUZE100_SWxMODE_APS_OFF   0x4
+
+#define PFUZE100_VGENxLPWR BIT(6)
+#define PFUZE100_VGENxSTBY BIT(5)
+
 enum chips { PFUZE100, PFUZE200, PFUZE3000 = 3 };
 
 struct pfuze_regulator {
@@ -489,6 +503,69 @@ static inline struct device_node *match_of_node(int index)
 }
 #endif
 
+static struct pfuze_chip *syspm_pfuze_chip;
+
+static void pfuze_power_off_prepare(void)
+{
+   dev_info(syspm_pfuze_chip->dev, "Configure standy mode for power off");
+
+   /* Switch from default mode: APS/APS to APS/Off */
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW1ABMODE,
+  PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW1CMODE,
+  PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW2MODE,
+  PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW3AMODE,
+  PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW3BMODE,
+  PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW4MODE,
+  PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN1VOL,
+  PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+  PFUZE100_VGENxSTBY);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN2VOL,
+  PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+  PFUZE100_VGENxSTBY);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN3VOL,
+  PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+  PFUZE100_VGENxSTBY);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN4VOL,
+  PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+  PFUZE100_VGENxSTBY);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN5VOL,
+  PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+  PFUZE100_VGENxSTBY);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN6VOL,
+  PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+  PFUZE100_VGENxSTBY);
+}
+
+static int pfuze_power_off_prepare_init(struct pfuze_chip *pfuze_chip)
+{
+   if (pfuze_chip->chip_id != PFUZE100) {
+   dev_warn(pfuze_chip->dev, "Requested pm_power_off_prepare 
handler for not supported chip\n");
+   return -ENODEV;
+   }
+
+   if (pm_power_off_prepare) {
+   dev_warn(pfuze_chip->dev, "pm_power_off_prepare is already 
registered.\n");
+   return -EBUSY;
+   }
+
+   if (syspm_pfuze_chip) {
+   dev_warn(pfuze_chip->dev, 

[PATCH v6 4/6] regulator: pfuze100: add fsl,pmic-stby-poweroff property

[Oleksij Rempel]

Document the new optional "fsl,pmic-stby-poweroff" property.

Signed-off-by: Oleksij Rempel 
Acked-by: Rob Herring 
---
 Documentation/devicetree/bindings/regulator/pfuze100.txt | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/pfuze100.txt 
b/Documentation/devicetree/bindings/regulator/pfuze100.txt
index 444c47831a40..197f1a52e960 100644
--- a/Documentation/devicetree/bindings/regulator/pfuze100.txt
+++ b/Documentation/devicetree/bindings/regulator/pfuze100.txt
@@ -4,6 +4,13 @@ Required properties:
 - compatible: "fsl,pfuze100", "fsl,pfuze200", "fsl,pfuze3000"
 - reg: I2C slave address
 
+Optional properties:
+- fsl,pmic-stby-poweroff: if present, configure the PMIC to shutdown all
+  power rails when PMIC_STBY_REQ line is asserted during the power off 
sequence.
+  Use this option if the SoC should be powered off by external power
+  management IC (PMIC) on PMIC_STBY_REQ signal.
+  As opposite to PMIC_STBY_REQ boards can implement PMIC_ON_REQ signal.
+
 Required child node:
 - regulators: This is the list of child nodes that specify the regulator
   initialization data for defined regulators. Please refer to below doc
-- 
2.11.0

[PATCH v6 1/6] ARM: imx6q: provide documentation for new fsl,pmic-stby-poweroff property

[Oleksij Rempel]

Signed-off-by: Oleksij Rempel 
Acked-by: Rob Herring 
---
 Documentation/devicetree/bindings/clock/imx6q-clock.txt | 8 
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt 
b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
index aa0a4d423ef5..9fed9dfe4a23 100644
--- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
+++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
@@ -6,6 +6,14 @@ Required properties:
 - interrupts: Should contain CCM interrupt
 - #clock-cells: Should be <1>
 
+Optional properties:
+- fsl,pmic-stby-poweroff: Configure CCM to assert PMIC_STBY_REQ signal
+  on power off.
+  Use this property if the SoC should be powered off by external power
+  management IC (PMIC) triggered via PMIC_STBY_REQ signal.
+  Boards that are designed to initiate poweroff on PMIC_ON_REQ signal should
+  be using "syscon-poweroff" driver instead.
+
 The clock consumer should specify the desired clock by having the clock
 ID in its "clocks" phandle cell.  See include/dt-bindings/clock/imx6qdl-clock.h
 for the full list of i.MX6 Quad and DualLite clock IDs.
-- 
2.11.0

[PATCH v6 1/6] ARM: imx6q: provide documentation for new fsl,pmic-stby-poweroff property

[Oleksij Rempel]

Signed-off-by: Oleksij Rempel 
Acked-by: Rob Herring 
---
 Documentation/devicetree/bindings/clock/imx6q-clock.txt | 8 
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt 
b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
index aa0a4d423ef5..9fed9dfe4a23 100644
--- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
+++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
@@ -6,6 +6,14 @@ Required properties:
 - interrupts: Should contain CCM interrupt
 - #clock-cells: Should be <1>
 
+Optional properties:
+- fsl,pmic-stby-poweroff: Configure CCM to assert PMIC_STBY_REQ signal
+  on power off.
+  Use this property if the SoC should be powered off by external power
+  management IC (PMIC) triggered via PMIC_STBY_REQ signal.
+  Boards that are designed to initiate poweroff on PMIC_ON_REQ signal should
+  be using "syscon-poweroff" driver instead.
+
 The clock consumer should specify the desired clock by having the clock
 ID in its "clocks" phandle cell.  See include/dt-bindings/clock/imx6qdl-clock.h
 for the full list of i.MX6 Quad and DualLite clock IDs.
-- 
2.11.0

[PATCH v6 5/6] regulator: pfuze100-regulator: provide pm_power_off_prepare handler

[Oleksij Rempel]

On some boards the SoC can use one pin "PMIC_STBY_REQ" to notify th PMIC
about state changes. In this case internal state of PMIC must be
preconfigured for upcomming state change.
It works fine with the current regulator framework, except with the
power-off case.

This patch is providing an optional pm_power_off_prepare handler
which will configure standby state of the PMIC to disable all power lines.

In my power consumption test on RIoTBoard, I got the following results:
power off without this patch:   320 mA
power off with this patch:  2   mA
suspend to ram: 40  mA

Signed-off-by: Oleksij Rempel 
---
 drivers/regulator/pfuze100-regulator.c | 92 ++
 1 file changed, 92 insertions(+)

diff --git a/drivers/regulator/pfuze100-regulator.c 
b/drivers/regulator/pfuze100-regulator.c
index 63922a2167e5..f6c276ed91d8 100644
--- a/drivers/regulator/pfuze100-regulator.c
+++ b/drivers/regulator/pfuze100-regulator.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define PFUZE_NUMREGS  128
@@ -42,11 +43,17 @@
 
 #define PFUZE100_COINVOL   0x1a
 #define PFUZE100_SW1ABVOL  0x20
+#define PFUZE100_SW1ABMODE 0x23
 #define PFUZE100_SW1CVOL   0x2e
+#define PFUZE100_SW1CMODE  0x31
 #define PFUZE100_SW2VOL0x35
+#define PFUZE100_SW2MODE   0x38
 #define PFUZE100_SW3AVOL   0x3c
+#define PFUZE100_SW3AMODE  0x3f
 #define PFUZE100_SW3BVOL   0x43
+#define PFUZE100_SW3BMODE  0x46
 #define PFUZE100_SW4VOL0x4a
+#define PFUZE100_SW4MODE   0x4d
 #define PFUZE100_SWBSTCON1 0x66
 #define PFUZE100_VREFDDRCON0x6a
 #define PFUZE100_VSNVSVOL  0x6b
@@ -57,6 +64,13 @@
 #define PFUZE100_VGEN5VOL  0x70
 #define PFUZE100_VGEN6VOL  0x71
 
+#define PFUZE100_SWxMODE_MASK  0xf
+#define PFUZE100_SWxMODE_APS_APS   0x8
+#define PFUZE100_SWxMODE_APS_OFF   0x4
+
+#define PFUZE100_VGENxLPWR BIT(6)
+#define PFUZE100_VGENxSTBY BIT(5)
+
 enum chips { PFUZE100, PFUZE200, PFUZE3000 = 3 };
 
 struct pfuze_regulator {
@@ -489,6 +503,69 @@ static inline struct device_node *match_of_node(int index)
 }
 #endif
 
+static struct pfuze_chip *syspm_pfuze_chip;
+
+static void pfuze_power_off_prepare(void)
+{
+   dev_info(syspm_pfuze_chip->dev, "Configure standy mode for power off");
+
+   /* Switch from default mode: APS/APS to APS/Off */
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW1ABMODE,
+  PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW1CMODE,
+  PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW2MODE,
+  PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW3AMODE,
+  PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW3BMODE,
+  PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_SW4MODE,
+  PFUZE100_SWxMODE_MASK, PFUZE100_SWxMODE_APS_OFF);
+
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN1VOL,
+  PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+  PFUZE100_VGENxSTBY);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN2VOL,
+  PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+  PFUZE100_VGENxSTBY);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN3VOL,
+  PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+  PFUZE100_VGENxSTBY);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN4VOL,
+  PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+  PFUZE100_VGENxSTBY);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN5VOL,
+  PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+  PFUZE100_VGENxSTBY);
+   regmap_update_bits(syspm_pfuze_chip->regmap, PFUZE100_VGEN6VOL,
+  PFUZE100_VGENxLPWR | PFUZE100_VGENxSTBY,
+  PFUZE100_VGENxSTBY);
+}
+
+static int pfuze_power_off_prepare_init(struct pfuze_chip *pfuze_chip)
+{
+   if (pfuze_chip->chip_id != PFUZE100) {
+   dev_warn(pfuze_chip->dev, "Requested pm_power_off_prepare 
handler for not supported chip\n");
+   return -ENODEV;
+   }
+
+   if (pm_power_off_prepare) {
+   dev_warn(pfuze_chip->dev, "pm_power_off_prepare is already 
registered.\n");
+   return -EBUSY;
+   }
+
+   if (syspm_pfuze_chip) {
+   dev_warn(pfuze_chip->dev, "syspm_pfuze_chip is already 

[PATCH v6 4/6] regulator: pfuze100: add fsl,pmic-stby-poweroff property

[Oleksij Rempel]

Document the new optional "fsl,pmic-stby-poweroff" property.

Signed-off-by: Oleksij Rempel 
Acked-by: Rob Herring 
---
 Documentation/devicetree/bindings/regulator/pfuze100.txt | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/pfuze100.txt 
b/Documentation/devicetree/bindings/regulator/pfuze100.txt
index 444c47831a40..197f1a52e960 100644
--- a/Documentation/devicetree/bindings/regulator/pfuze100.txt
+++ b/Documentation/devicetree/bindings/regulator/pfuze100.txt
@@ -4,6 +4,13 @@ Required properties:
 - compatible: "fsl,pfuze100", "fsl,pfuze200", "fsl,pfuze3000"
 - reg: I2C slave address
 
+Optional properties:
+- fsl,pmic-stby-poweroff: if present, configure the PMIC to shutdown all
+  power rails when PMIC_STBY_REQ line is asserted during the power off 
sequence.
+  Use this option if the SoC should be powered off by external power
+  management IC (PMIC) on PMIC_STBY_REQ signal.
+  As opposite to PMIC_STBY_REQ boards can implement PMIC_ON_REQ signal.
+
 Required child node:
 - regulators: This is the list of child nodes that specify the regulator
   initialization data for defined regulators. Please refer to below doc
-- 
2.11.0

[PATCH v6 6/6] ARM: dts: imx6: RIoTboard provide standby on power off option

[Oleksij Rempel]

This board, as well as some other boards with i.MX6 and a PMIC, uses a
"PMIC_STBY_REQ" line to notify the PMIC about a state change.
The PMIC is programmed for a specific state change before triggering the
line.
In this case, PMIC_STBY_REQ can be used for stand by, sleep
and power off modes.

Signed-off-by: Oleksij Rempel 
---
 arch/arm/boot/dts/imx6dl-riotboard.dts | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/imx6dl-riotboard.dts 
b/arch/arm/boot/dts/imx6dl-riotboard.dts
index 275c6c05219d..574f6b261ecd 100644
--- a/arch/arm/boot/dts/imx6dl-riotboard.dts
+++ b/arch/arm/boot/dts/imx6dl-riotboard.dts
@@ -90,6 +90,10 @@
status = "okay";
 };
 
+ {
+   fsl,pmic-stby-poweroff;
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_enet>;
@@ -170,6 +174,7 @@
reg = <0x08>;
interrupt-parent = <>;
interrupts = <16 8>;
+   fsl,pmic-stby-poweroff;
 
regulators {
reg_vddcore: sw1ab {/* 
VDDARM_IN */
-- 
2.11.0

[PATCH v6 6/6] ARM: dts: imx6: RIoTboard provide standby on power off option

[Oleksij Rempel]

This board, as well as some other boards with i.MX6 and a PMIC, uses a
"PMIC_STBY_REQ" line to notify the PMIC about a state change.
The PMIC is programmed for a specific state change before triggering the
line.
In this case, PMIC_STBY_REQ can be used for stand by, sleep
and power off modes.

Signed-off-by: Oleksij Rempel 
---
 arch/arm/boot/dts/imx6dl-riotboard.dts | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/imx6dl-riotboard.dts 
b/arch/arm/boot/dts/imx6dl-riotboard.dts
index 275c6c05219d..574f6b261ecd 100644
--- a/arch/arm/boot/dts/imx6dl-riotboard.dts
+++ b/arch/arm/boot/dts/imx6dl-riotboard.dts
@@ -90,6 +90,10 @@
status = "okay";
 };
 
+ {
+   fsl,pmic-stby-poweroff;
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_enet>;
@@ -170,6 +174,7 @@
reg = <0x08>;
interrupt-parent = <>;
interrupts = <16 8>;
+   fsl,pmic-stby-poweroff;
 
regulators {
reg_vddcore: sw1ab {/* 
VDDARM_IN */
-- 
2.11.0

Re: [PATCH] doc: convert printk-formats.txt to rst

[Markus Heiser]

> Am 06.12.2017 um 02:45 schrieb Tobin C. Harding :
> 
> Documentation/printk-formats.txt is a candidate for conversion to
> ReStructuredText format. Some effort has already been made to do this
> conversion even thought the suffix is currently .txt
> 
> Changes required to complete conversion
> 
> - Add double backticks where needed.
> - Add entry to Documentation/index.rst
> - Use flat-table instead of ASCII table.

[...]

> +=
> +How to Get ``printk`` Format Specifiers Right
> +=
> 
> :Author: Randy Dunlap 
> :Author: Andrew Murray 
> @@ -8,56 +8,91 @@ How to get printk format specifiers right
> Integer types
> =
> 
> -::
> +For printing integer types, we have the following format specifiers:
> + 
> +   .. flat-table:: 
> +  :widths: 2 2
> +
> +  * - **Type**
> + - **Specifier**
> +
> +  * - ``int``
> +- ``%d`` or ``%x``
> +
> +  * - ``unsigned int``
> + - ``%u`` or ``%x``
> +
> +  * - ``long``
> + - ``%ld`` or ``%lx``
> +
> +  * - ``unsigned long``
> + - ``%lu`` or ``%lx``
> +
> +  * - ``long long``
> + - ``%lld`` or ``%llx``
> 
> - If variable is of Type, use printk format specifier:
> - 
> - int %d or %x
> - unsigned int%u or %x
> - long%ld or %lx
> - unsigned long   %lu or %lx
> - long long   %lld or %llx
> - unsigned long long  %llu or %llx
> - size_t  %zu or %zx
> - ssize_t %zd or %zx
> - s32 %d or %x
> - u32 %u or %x
> - s64 %lld or %llx
> - u64 %llu or %llx
> -
> -If  is dependent on a config option for its size (e.g., ``sector_t``,
> +  * - ``unsigned long long``
> + - ``%llu`` or ``%llx``
> +
> +  * - ``size_t``
> + - ``%zu`` or ``%zx``
> +
> +  * - ``ssize_t``
> + - ``%zd`` or ``%zx``
> +
> +  * - ``s32``
> + - ``%d`` or ``%x``
> +
> +  * - ``u32``
> + - ``%u`` or ``%x``
> +
> +  * - ``s64``
> + - ``%lld`` or ``%llx``
> +
> +  * - ``u64``
> + - ``%llu`` or ``%llx``
> +
> +

Thanks!

just a question .. might it be better we stay with ASCII table
in cases like this. I guess this table won't changed often.
The flat-table directive is good for big and therefore frequently 
changed tables where a small precise diff reduce the patch.
But flat-table is also hard to read in plain text. Its a balancing
and thats my opinion, lets hear what other say ...

-- Markus --

Re: [PATCH] doc: convert printk-formats.txt to rst

[Markus Heiser]

> Am 06.12.2017 um 02:45 schrieb Tobin C. Harding :
> 
> Documentation/printk-formats.txt is a candidate for conversion to
> ReStructuredText format. Some effort has already been made to do this
> conversion even thought the suffix is currently .txt
> 
> Changes required to complete conversion
> 
> - Add double backticks where needed.
> - Add entry to Documentation/index.rst
> - Use flat-table instead of ASCII table.

[...]

> +=
> +How to Get ``printk`` Format Specifiers Right
> +=
> 
> :Author: Randy Dunlap 
> :Author: Andrew Murray 
> @@ -8,56 +8,91 @@ How to get printk format specifiers right
> Integer types
> =
> 
> -::
> +For printing integer types, we have the following format specifiers:
> + 
> +   .. flat-table:: 
> +  :widths: 2 2
> +
> +  * - **Type**
> + - **Specifier**
> +
> +  * - ``int``
> +- ``%d`` or ``%x``
> +
> +  * - ``unsigned int``
> + - ``%u`` or ``%x``
> +
> +  * - ``long``
> + - ``%ld`` or ``%lx``
> +
> +  * - ``unsigned long``
> + - ``%lu`` or ``%lx``
> +
> +  * - ``long long``
> + - ``%lld`` or ``%llx``
> 
> - If variable is of Type, use printk format specifier:
> - 
> - int %d or %x
> - unsigned int%u or %x
> - long%ld or %lx
> - unsigned long   %lu or %lx
> - long long   %lld or %llx
> - unsigned long long  %llu or %llx
> - size_t  %zu or %zx
> - ssize_t %zd or %zx
> - s32 %d or %x
> - u32 %u or %x
> - s64 %lld or %llx
> - u64 %llu or %llx
> -
> -If  is dependent on a config option for its size (e.g., ``sector_t``,
> +  * - ``unsigned long long``
> + - ``%llu`` or ``%llx``
> +
> +  * - ``size_t``
> + - ``%zu`` or ``%zx``
> +
> +  * - ``ssize_t``
> + - ``%zd`` or ``%zx``
> +
> +  * - ``s32``
> + - ``%d`` or ``%x``
> +
> +  * - ``u32``
> + - ``%u`` or ``%x``
> +
> +  * - ``s64``
> + - ``%lld`` or ``%llx``
> +
> +  * - ``u64``
> + - ``%llu`` or ``%llx``
> +
> +

Thanks!

just a question .. might it be better we stay with ASCII table
in cases like this. I guess this table won't changed often.
The flat-table directive is good for big and therefore frequently 
changed tables where a small precise diff reduce the patch.
But flat-table is also hard to read in plain text. Its a balancing
and thats my opinion, lets hear what other say ...

-- Markus --

Re: [PATCH v4 1/3] drm/bridge/synopsys: dsi: stop clobbering drvdata

[Nickey Yang]

Hi Brian,


On 2017年12月06日 02:56, Brian Norris wrote:

Hi Nickey,

On Tue, Dec 05, 2017 at 05:14:11PM +0800, Nickey Yang wrote:

On 2017年12月01日 18:07, Philippe CORNU wrote:

On 12/01/2017 10:11 AM, Nickey Yang wrote:

On 2017年12月01日 16:32, Philippe CORNU wrote:

I am sorry to say that but you can not add my "Acked-by" to this patch
because this code is different from the "original" one from Brian (which
got my "Acked-by").

I'm sorry I didn't think much about it, Thank you for correcting me.

Sometimes it is not an issue because differences are not important but
in this particular case, the code is really different: you have remove
platform_set_drvdata() & platform_get_drvdata() in the stm part.

Could you please go back to the original code or propose me an updated
version of this code.

Could you help update new version of this code(stm part) and then test on
stm platform?

I think you can simply goes back to the original version from Brian (see
the discussion thread in https://patchwork.kernel.org/patch/10078493/)
unless you have specific/good reasons for modifying the code as you did.

mmm,I'm sorry, I feel a little puzzled. Do you means we should abandon
Brian's patch (https://patchwork.kernel.org/patch/10078493/)?
I think we need to adjust stm part because  dw_mipi_dsi_stm.c calls
bridge's drivers if we want merge Brian's patch.

It's really simple. Your code is different from the patch I sent, and in
a way that Philippe did not like. I'll highlight it again below:


diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c 
b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
index e5b6310..80f9950 100644
--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
@@ -66,6 +66,7 @@ enum dsi_color {
struct dw_mipi_dsi_stm {
void __iomem *base;
struct clk *pllref_clk;
+   struct dw_mipi_dsi *dmd;
};
static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
@@ -318,10 +319,11 @@ static int dw_mipi_dsi_stm_probe(struct platform_device 
*pdev)
dw_mipi_dsi_stm_plat_data.base = dsi->base;
dw_mipi_dsi_stm_plat_data.priv_data = dsi;
-   ret = dw_mipi_dsi_probe(pdev, _mipi_dsi_stm_plat_data);
-   if (ret) {
+   dsi->dmd = dw_mipi_dsi_probe(pdev, _mipi_dsi_stm_plat_data);
+   if (IS_ERR(dsi->dmd)) {
DRM_ERROR("Failed to initialize mipi dsi host\n");
clk_disable_unprepare(dsi->pllref_clk);
+   return PTR_ERR(dsi->dmd);
}
return ret;
@@ -332,7 +334,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device 
*pdev)
struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
clk_disable_unprepare(dsi->pllref_clk);
-   dw_mipi_dsi_remove(pdev);
+   dw_mipi_dsi_remove(dsi->dmd);
return 0;
}
  
Above is your diff for dw_mipi_dsi-stm.c. Particularly, notice that

remove() is directly referencing the static dw_mipi_dsi_stm_plat_data
struct.

If you look back at my patch [1] you'll see that you're missing hunks
like this:


Thank you for pointing out my mistake.
I will fix this in next version.

Nickey.

  static int dw_mipi_dsi_stm_remove(struct platform_device *pdev)
  {
-   struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
+   struct dw_mipi_dsi_stm *dsi = platform_get_drvdata(pdev);
  
  	clk_disable_unprepare(dsi->pllref_clk);

[...]

Brian

[1] https://patchwork.kernel.org/patch/10078493/

Re: [PATCH v4 1/3] drm/bridge/synopsys: dsi: stop clobbering drvdata

[Nickey Yang]

Hi Brian,


On 2017年12月06日 02:56, Brian Norris wrote:

Hi Nickey,

On Tue, Dec 05, 2017 at 05:14:11PM +0800, Nickey Yang wrote:

On 2017年12月01日 18:07, Philippe CORNU wrote:

On 12/01/2017 10:11 AM, Nickey Yang wrote:

On 2017年12月01日 16:32, Philippe CORNU wrote:

I am sorry to say that but you can not add my "Acked-by" to this patch
because this code is different from the "original" one from Brian (which
got my "Acked-by").

I'm sorry I didn't think much about it, Thank you for correcting me.

Sometimes it is not an issue because differences are not important but
in this particular case, the code is really different: you have remove
platform_set_drvdata() & platform_get_drvdata() in the stm part.

Could you please go back to the original code or propose me an updated
version of this code.

Could you help update new version of this code(stm part) and then test on
stm platform?

I think you can simply goes back to the original version from Brian (see
the discussion thread in https://patchwork.kernel.org/patch/10078493/)
unless you have specific/good reasons for modifying the code as you did.

mmm,I'm sorry, I feel a little puzzled. Do you means we should abandon
Brian's patch (https://patchwork.kernel.org/patch/10078493/)?
I think we need to adjust stm part because  dw_mipi_dsi_stm.c calls
bridge's drivers if we want merge Brian's patch.

It's really simple. Your code is different from the patch I sent, and in
a way that Philippe did not like. I'll highlight it again below:


diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c 
b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
index e5b6310..80f9950 100644
--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
@@ -66,6 +66,7 @@ enum dsi_color {
struct dw_mipi_dsi_stm {
void __iomem *base;
struct clk *pllref_clk;
+   struct dw_mipi_dsi *dmd;
};
static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
@@ -318,10 +319,11 @@ static int dw_mipi_dsi_stm_probe(struct platform_device 
*pdev)
dw_mipi_dsi_stm_plat_data.base = dsi->base;
dw_mipi_dsi_stm_plat_data.priv_data = dsi;
-   ret = dw_mipi_dsi_probe(pdev, _mipi_dsi_stm_plat_data);
-   if (ret) {
+   dsi->dmd = dw_mipi_dsi_probe(pdev, _mipi_dsi_stm_plat_data);
+   if (IS_ERR(dsi->dmd)) {
DRM_ERROR("Failed to initialize mipi dsi host\n");
clk_disable_unprepare(dsi->pllref_clk);
+   return PTR_ERR(dsi->dmd);
}
return ret;
@@ -332,7 +334,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device 
*pdev)
struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
clk_disable_unprepare(dsi->pllref_clk);
-   dw_mipi_dsi_remove(pdev);
+   dw_mipi_dsi_remove(dsi->dmd);
return 0;
}
  
Above is your diff for dw_mipi_dsi-stm.c. Particularly, notice that

remove() is directly referencing the static dw_mipi_dsi_stm_plat_data
struct.

If you look back at my patch [1] you'll see that you're missing hunks
like this:


Thank you for pointing out my mistake.
I will fix this in next version.

Nickey.

  static int dw_mipi_dsi_stm_remove(struct platform_device *pdev)
  {
-   struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
+   struct dw_mipi_dsi_stm *dsi = platform_get_drvdata(pdev);
  
  	clk_disable_unprepare(dsi->pllref_clk);

[...]

Brian

[1] https://patchwork.kernel.org/patch/10078493/

[PATCH] sched/wait: fix add_wait_queue() behavior change

[Omar Sandoval]

From: Omar Sandoval 

Commit 50816c48997a ("sched/wait: Standardize internal naming of
wait-queue entries") changed the behavior of add_wait_queue() from
inserting the wait entry at the head of the wait queue to the tail of
the wait queue. That commit was a cleanup and didn't mention any
functional changes so it was likely unintentional. This change in
behavior theoretically breaks wait queues which mix exclusive and
non-exclusive waiters, as non-exclusive waiters will not be woken up if
they are queued behind enough exclusive waiters.

Signed-off-by: Omar Sandoval 
---
Since Ingo hasn't replied, here's a proper patch.

 kernel/sched/wait.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index 98feab7933c7..929ecb7d6b78 100644
--- a/kernel/sched/wait.c
+++ b/kernel/sched/wait.c
@@ -27,7 +27,7 @@ void add_wait_queue(struct wait_queue_head *wq_head, struct 
wait_queue_entry *wq
 
wq_entry->flags &= ~WQ_FLAG_EXCLUSIVE;
spin_lock_irqsave(_head->lock, flags);
-   __add_wait_queue_entry_tail(wq_head, wq_entry);
+   __add_wait_queue(wq_head, wq_entry);
spin_unlock_irqrestore(_head->lock, flags);
 }
 EXPORT_SYMBOL(add_wait_queue);
-- 
2.15.1

[PATCH] sched/wait: fix add_wait_queue() behavior change

[Omar Sandoval]

From: Omar Sandoval 

Commit 50816c48997a ("sched/wait: Standardize internal naming of
wait-queue entries") changed the behavior of add_wait_queue() from
inserting the wait entry at the head of the wait queue to the tail of
the wait queue. That commit was a cleanup and didn't mention any
functional changes so it was likely unintentional. This change in
behavior theoretically breaks wait queues which mix exclusive and
non-exclusive waiters, as non-exclusive waiters will not be woken up if
they are queued behind enough exclusive waiters.

Signed-off-by: Omar Sandoval 
---
Since Ingo hasn't replied, here's a proper patch.

 kernel/sched/wait.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index 98feab7933c7..929ecb7d6b78 100644
--- a/kernel/sched/wait.c
+++ b/kernel/sched/wait.c
@@ -27,7 +27,7 @@ void add_wait_queue(struct wait_queue_head *wq_head, struct 
wait_queue_entry *wq
 
wq_entry->flags &= ~WQ_FLAG_EXCLUSIVE;
spin_lock_irqsave(_head->lock, flags);
-   __add_wait_queue_entry_tail(wq_head, wq_entry);
+   __add_wait_queue(wq_head, wq_entry);
spin_unlock_irqrestore(_head->lock, flags);
 }
 EXPORT_SYMBOL(add_wait_queue);
-- 
2.15.1

Re: rsi_91x: Failed to read status register on failed authentication

[Amitkumar Karwar]

On Tue, Dec 5, 2017 at 9:41 PM, Alexey Brodkin
 wrote:
> Hi Amit,
>
> I'm seeing quite a strange behavior of RedPine module.
> It connects perfectly fine to one of access points but fails
> to connect to another.
>
> Moreover after that failure RSI driver starts to flood me with
> messages saying:
> ->8
> rsi_91x: rsi_sdio_check_buffer_status: Failed to read status register
> ->8
>
> Below you may find details of my 2 experiments.
> Note I use vanilla Linux kernel v4.14.4
>
> Any ideas what could be wrong?
>

Could you enable driver debug zones and share dmesg log for analysis?

echo 0x > /sys/kernel/debug/phy0/debug_zone

Regards,
Amitkumar

Re: rsi_91x: Failed to read status register on failed authentication

[Amitkumar Karwar]

On Tue, Dec 5, 2017 at 9:41 PM, Alexey Brodkin
 wrote:
> Hi Amit,
>
> I'm seeing quite a strange behavior of RedPine module.
> It connects perfectly fine to one of access points but fails
> to connect to another.
>
> Moreover after that failure RSI driver starts to flood me with
> messages saying:
> ->8
> rsi_91x: rsi_sdio_check_buffer_status: Failed to read status register
> ->8
>
> Below you may find details of my 2 experiments.
> Note I use vanilla Linux kernel v4.14.4
>
> Any ideas what could be wrong?
>

Could you enable driver debug zones and share dmesg log for analysis?

echo 0x > /sys/kernel/debug/phy0/debug_zone

Regards,
Amitkumar

RE: [Bug fix] octeon-i2c driver updates

[Zhang, Sean C. (NSB - CN/Hangzhou)]

Hi David,

In fact, in current octeon_i2c_probe(), there has the bus reset unconditionally,
It is reset by octeon_i2c_init_lowlevel() unconditionally. 
In my patch, I replace it by octeon_i2c_recovery(), which will be executed
under three conditions.

+   /*
+* Try to recover bus in three conditions: TWSI core status
+* not IDLE, SDA stuck low or TWSI_CTL_IFLG not cleared
+*/
+   if ((octeon_i2c_stat_read(i2c) != STAT_IDLE) ||
+   !(octeon_i2c_read_int(i2c) & TWSI_INT_SDA_OVR) ||
+   (octeon_i2c_ctl_read(i2c) & TWSI_CTL_IFLG)) {
+   result = octeon_i2c_recovery(i2c);
+   if (result) {
+   dev_err(i2c->dev, "octeon i2c recovery failed\n");
+   goto  out;
+   }
+   }
+

Attached improved patch and C file for your review. Thanks in advance.

BR,
Sean Zhang

-Original Message-
From: David Daney [mailto:dda...@caviumnetworks.com] 
Sent: Wednesday, December 06, 2017 12:43 AM
To: Zhang, Sean C. (NSB - CN/Hangzhou) ; Jan 
Glauber ; david.da...@cavium.com
Cc: w...@the-dreams.de; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [Bug fix] octeon-i2c driver updates

On 12/04/2017 10:44 PM, Zhang, Sean C. (NSB - CN/Hangzhou) wrote:
> Hi Jan,
> 
> Thanks for your comments, I get your point for the second point (retry of 
> START after recovery).
> 
> Hi David,
> For the issue as the first one, would you give your further comments? Thanks 
> in advance.
> 
> I have an environment with CN6780 (TWSI core has property: compatible 
> = "cavium,octeon-3860-twsi"), And encounter below problem:
> During i2c-octeon driver probing, this TWSI core original status is 
> 0x20 (this may induced by uboot), And octeon_i2c_init_lowlevel() 
> function in octeon_i2c_probe() is not enough to recover the I2C bus, 
> If go without full recovery of octeon_i2c_recovery(), the following 
> octeon_i2c_hlc_write(), octeon_i2c_hlc_read(), octeon_i2c_hlc_comp_read() and 
> octeon_i2c_hlc_comp_write() will goes error, because these functions has no 
> bus recovery step.
> While after replace octeon_i2c_init_lowlevel() with 
> octeon_i2c_recovery() in octeon_i2c_probe(), the problem has gone.
> 
> Once more, this octeon_i2c_recovery() can also recover dead lock (I2C 
> slave device stuck low SCL) issue, so I think use octeon_i2c_recovery() 
> instead will be stronger.

I don't want to do a bus reset unconditionally, as it is currently working well 
on many systems.

Perhaps you could add a module parameter to enable the recovery mode on probe 
as an option.  Would that work or be acceptable?

Thanks,
David Daney



> 
> BR,
> Sean Zhang
> 
> 
> -Original Message-
> From: Jan Glauber [mailto:jan.glau...@caviumnetworks.com]
> Sent: Friday, December 01, 2017 6:07 PM
> To: Zhang, Sean C. (NSB - CN/Hangzhou) 
> Cc: david.da...@cavium.com; w...@the-dreams.de; 
> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [Bug fix] octeon-i2c driver updates
> 
> Hi Sean,
> 
> as you try to solve two different issues I suggest that you create one 
> patch per issue.
> 
> For the second point (retry of START after recovery) I would still 
> like to hear Wolfram's opinion. I would assume that any i2c user 
> should be well aware of -EAGAIN, so I wonder if it is worth the 
> additional complexity of the retry logic.
> 
> Also, the first issue changes Octeon MIPS which I'm not able to test, 
> so David needs to be involved here.
> 
> thanks,
> Jan
> 
> On Thu, Nov 30, 2017 at 01:56:09AM +, Zhang, Sean C. (NSB - CN/Hangzhou) 
> wrote:
>> Hi Jan,
>>
>> Any other comments for this patch?
>>
>> BR,
>> Sean Zhang
>>
>> -Original Message-
>> From: Zhang, Sean C. (NSB - CN/Hangzhou)
>> Sent: Monday, November 27, 2017 4:38 PM
>> To: 'Jan Glauber' 
>> Cc: david.da...@cavium.com; w...@the-dreams.de; 
>> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: RE: [Bug fix] octeon-i2c driver updates
>>
>> Hi Jan,
>>
>> There are two points in this patch.
>>
>> Point 1. As you see, replaced octeon_i2c_init_lowlevel() by recover bus 
>> status if TWSI controller is not IDLE.
>> Please take a scenario like this: when system soft reset without I2C 
>> slave reset, maybe make this I2C bus dead lock occurred (I2C slave 
>> device stuck low SCL) in chance. Then during system goes up and I2C 
>> slave device creating process, if this I2C slave device has a register with 
>> less than 8 bytes to read, but I2C bus was still stuck low SCL by last 
>> system reset, then the read will failed and this I2C slave device cannot be 
>> created.
>> If bus recovered before the reading process, this failure can be fixed.
>>
>> Function flow explanation shown as below:
>>
>> a. System reset without I2C slave device reset --make SCL stuck low 
>> by I2C slave device 

RE: [Bug fix] octeon-i2c driver updates

[Zhang, Sean C. (NSB - CN/Hangzhou)]

Hi David,

In fact, in current octeon_i2c_probe(), there has the bus reset unconditionally,
It is reset by octeon_i2c_init_lowlevel() unconditionally. 
In my patch, I replace it by octeon_i2c_recovery(), which will be executed
under three conditions.

+   /*
+* Try to recover bus in three conditions: TWSI core status
+* not IDLE, SDA stuck low or TWSI_CTL_IFLG not cleared
+*/
+   if ((octeon_i2c_stat_read(i2c) != STAT_IDLE) ||
+   !(octeon_i2c_read_int(i2c) & TWSI_INT_SDA_OVR) ||
+   (octeon_i2c_ctl_read(i2c) & TWSI_CTL_IFLG)) {
+   result = octeon_i2c_recovery(i2c);
+   if (result) {
+   dev_err(i2c->dev, "octeon i2c recovery failed\n");
+   goto  out;
+   }
+   }
+

Attached improved patch and C file for your review. Thanks in advance.

BR,
Sean Zhang

-Original Message-
From: David Daney [mailto:dda...@caviumnetworks.com] 
Sent: Wednesday, December 06, 2017 12:43 AM
To: Zhang, Sean C. (NSB - CN/Hangzhou) ; Jan 
Glauber ; david.da...@cavium.com
Cc: w...@the-dreams.de; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [Bug fix] octeon-i2c driver updates

On 12/04/2017 10:44 PM, Zhang, Sean C. (NSB - CN/Hangzhou) wrote:
> Hi Jan,
> 
> Thanks for your comments, I get your point for the second point (retry of 
> START after recovery).
> 
> Hi David,
> For the issue as the first one, would you give your further comments? Thanks 
> in advance.
> 
> I have an environment with CN6780 (TWSI core has property: compatible 
> = "cavium,octeon-3860-twsi"), And encounter below problem:
> During i2c-octeon driver probing, this TWSI core original status is 
> 0x20 (this may induced by uboot), And octeon_i2c_init_lowlevel() 
> function in octeon_i2c_probe() is not enough to recover the I2C bus, 
> If go without full recovery of octeon_i2c_recovery(), the following 
> octeon_i2c_hlc_write(), octeon_i2c_hlc_read(), octeon_i2c_hlc_comp_read() and 
> octeon_i2c_hlc_comp_write() will goes error, because these functions has no 
> bus recovery step.
> While after replace octeon_i2c_init_lowlevel() with 
> octeon_i2c_recovery() in octeon_i2c_probe(), the problem has gone.
> 
> Once more, this octeon_i2c_recovery() can also recover dead lock (I2C 
> slave device stuck low SCL) issue, so I think use octeon_i2c_recovery() 
> instead will be stronger.

I don't want to do a bus reset unconditionally, as it is currently working well 
on many systems.

Perhaps you could add a module parameter to enable the recovery mode on probe 
as an option.  Would that work or be acceptable?

Thanks,
David Daney



> 
> BR,
> Sean Zhang
> 
> 
> -Original Message-
> From: Jan Glauber [mailto:jan.glau...@caviumnetworks.com]
> Sent: Friday, December 01, 2017 6:07 PM
> To: Zhang, Sean C. (NSB - CN/Hangzhou) 
> Cc: david.da...@cavium.com; w...@the-dreams.de; 
> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [Bug fix] octeon-i2c driver updates
> 
> Hi Sean,
> 
> as you try to solve two different issues I suggest that you create one 
> patch per issue.
> 
> For the second point (retry of START after recovery) I would still 
> like to hear Wolfram's opinion. I would assume that any i2c user 
> should be well aware of -EAGAIN, so I wonder if it is worth the 
> additional complexity of the retry logic.
> 
> Also, the first issue changes Octeon MIPS which I'm not able to test, 
> so David needs to be involved here.
> 
> thanks,
> Jan
> 
> On Thu, Nov 30, 2017 at 01:56:09AM +, Zhang, Sean C. (NSB - CN/Hangzhou) 
> wrote:
>> Hi Jan,
>>
>> Any other comments for this patch?
>>
>> BR,
>> Sean Zhang
>>
>> -Original Message-
>> From: Zhang, Sean C. (NSB - CN/Hangzhou)
>> Sent: Monday, November 27, 2017 4:38 PM
>> To: 'Jan Glauber' 
>> Cc: david.da...@cavium.com; w...@the-dreams.de; 
>> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: RE: [Bug fix] octeon-i2c driver updates
>>
>> Hi Jan,
>>
>> There are two points in this patch.
>>
>> Point 1. As you see, replaced octeon_i2c_init_lowlevel() by recover bus 
>> status if TWSI controller is not IDLE.
>> Please take a scenario like this: when system soft reset without I2C 
>> slave reset, maybe make this I2C bus dead lock occurred (I2C slave 
>> device stuck low SCL) in chance. Then during system goes up and I2C 
>> slave device creating process, if this I2C slave device has a register with 
>> less than 8 bytes to read, but I2C bus was still stuck low SCL by last 
>> system reset, then the read will failed and this I2C slave device cannot be 
>> created.
>> If bus recovered before the reading process, this failure can be fixed.
>>
>> Function flow explanation shown as below:
>>
>> a. System reset without I2C slave device reset --make SCL stuck low 
>> by I2C slave device ..
>> b. octeon_i2c_probe（）
>> -- octeon_i2c_init_lowlevel  //reset TWSI core, but SCL still stuck low by.
>> ..
>>
>> 

[PATCH] Staging: rtl8192u: Fix no spaces around '+'

[Akash Kumar]

Added spaces around '+'. Warning found using checkpatch.pl
---
 drivers/staging/rtl8192u/ieee80211/dot11d.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/dot11d.c 
b/drivers/staging/rtl8192u/ieee80211/dot11d.c
index 64b13a5..ba284bf 100644
--- a/drivers/staging/rtl8192u/ieee80211/dot11d.c
+++ b/drivers/staging/rtl8192u/ieee80211/dot11d.c
@@ -11,7 +11,7 @@ void Dot11d_Init(struct ieee80211_device *ieee)
 
pDot11dInfo->State = DOT11D_STATE_NONE;
pDot11dInfo->CountryIeLen = 0;
-   memset(pDot11dInfo->channel_map, 0, MAX_CHANNEL_NUMBER+1);
+   memset(pDot11dInfo->channel_map, 0, MAX_CHANNEL_NUMBER + 1);
memset(pDot11dInfo->MaxTxPwrDbmList, 0xFF, MAX_CHANNEL_NUMBER+1);
RESET_CIE_WATCHDOG(ieee);
 
-- 
2.7.4

[PATCH] Staging: rtl8192u: Fix no spaces around '+'

[Akash Kumar]

Added spaces around '+'. Warning found using checkpatch.pl
---
 drivers/staging/rtl8192u/ieee80211/dot11d.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/dot11d.c 
b/drivers/staging/rtl8192u/ieee80211/dot11d.c
index 64b13a5..ba284bf 100644
--- a/drivers/staging/rtl8192u/ieee80211/dot11d.c
+++ b/drivers/staging/rtl8192u/ieee80211/dot11d.c
@@ -11,7 +11,7 @@ void Dot11d_Init(struct ieee80211_device *ieee)
 
pDot11dInfo->State = DOT11D_STATE_NONE;
pDot11dInfo->CountryIeLen = 0;
-   memset(pDot11dInfo->channel_map, 0, MAX_CHANNEL_NUMBER+1);
+   memset(pDot11dInfo->channel_map, 0, MAX_CHANNEL_NUMBER + 1);
memset(pDot11dInfo->MaxTxPwrDbmList, 0xFF, MAX_CHANNEL_NUMBER+1);
RESET_CIE_WATCHDOG(ieee);
 
-- 
2.7.4

[PATCH] MIPS: Octeon: Fix logging messages with spurious periods after newlines

[Joe Perches]

Using a period after a newline causes bad output.

Signed-off-by: Joe Perches 
---
 arch/mips/cavium-octeon/octeon-irq.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/mips/cavium-octeon/octeon-irq.c 
b/arch/mips/cavium-octeon/octeon-irq.c
index 5b3a3f6a9ad3..b993d9f2c9b9 100644
--- a/arch/mips/cavium-octeon/octeon-irq.c
+++ b/arch/mips/cavium-octeon/octeon-irq.c
@@ -2271,7 +2271,7 @@ static int __init octeon_irq_init_cib(struct device_node 
*ciu_node,
 
parent_irq = irq_of_parse_and_map(ciu_node, 0);
if (!parent_irq) {
-   pr_err("ERROR: Couldn't acquire parent_irq for %s\n.",
+   pr_err("ERROR: Couldn't acquire parent_irq for %s\n",
ciu_node->name);
return -EINVAL;
}
@@ -2281,7 +2281,7 @@ static int __init octeon_irq_init_cib(struct device_node 
*ciu_node,
 
addr = of_get_address(ciu_node, 0, NULL, NULL);
if (!addr) {
-   pr_err("ERROR: Couldn't acquire reg(0) %s\n.", ciu_node->name);
+   pr_err("ERROR: Couldn't acquire reg(0) %s\n", ciu_node->name);
return -EINVAL;
}
host_data->raw_reg = (u64)phys_to_virt(
@@ -2289,7 +2289,7 @@ static int __init octeon_irq_init_cib(struct device_node 
*ciu_node,
 
addr = of_get_address(ciu_node, 1, NULL, NULL);
if (!addr) {
-   pr_err("ERROR: Couldn't acquire reg(1) %s\n.", ciu_node->name);
+   pr_err("ERROR: Couldn't acquire reg(1) %s\n", ciu_node->name);
return -EINVAL;
}
host_data->en_reg = (u64)phys_to_virt(
@@ -2297,7 +2297,7 @@ static int __init octeon_irq_init_cib(struct device_node 
*ciu_node,
 
r = of_property_read_u32(ciu_node, "cavium,max-bits", );
if (r) {
-   pr_err("ERROR: Couldn't read cavium,max-bits from %s\n.",
+   pr_err("ERROR: Couldn't read cavium,max-bits from %s\n",
ciu_node->name);
return r;
}
@@ -2307,7 +2307,7 @@ static int __init octeon_irq_init_cib(struct device_node 
*ciu_node,
   _irq_domain_cib_ops,
   host_data);
if (!cib_domain) {
-   pr_err("ERROR: Couldn't irq_domain_add_linear()\n.");
+   pr_err("ERROR: Couldn't irq_domain_add_linear()\n");
return -ENOMEM;
}
 
-- 
2.15.0

[PATCH] MIPS: Octeon: Fix logging messages with spurious periods after newlines

[Joe Perches]

Using a period after a newline causes bad output.

Signed-off-by: Joe Perches 
---
 arch/mips/cavium-octeon/octeon-irq.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/mips/cavium-octeon/octeon-irq.c 
b/arch/mips/cavium-octeon/octeon-irq.c
index 5b3a3f6a9ad3..b993d9f2c9b9 100644
--- a/arch/mips/cavium-octeon/octeon-irq.c
+++ b/arch/mips/cavium-octeon/octeon-irq.c
@@ -2271,7 +2271,7 @@ static int __init octeon_irq_init_cib(struct device_node 
*ciu_node,
 
parent_irq = irq_of_parse_and_map(ciu_node, 0);
if (!parent_irq) {
-   pr_err("ERROR: Couldn't acquire parent_irq for %s\n.",
+   pr_err("ERROR: Couldn't acquire parent_irq for %s\n",
ciu_node->name);
return -EINVAL;
}
@@ -2281,7 +2281,7 @@ static int __init octeon_irq_init_cib(struct device_node 
*ciu_node,
 
addr = of_get_address(ciu_node, 0, NULL, NULL);
if (!addr) {
-   pr_err("ERROR: Couldn't acquire reg(0) %s\n.", ciu_node->name);
+   pr_err("ERROR: Couldn't acquire reg(0) %s\n", ciu_node->name);
return -EINVAL;
}
host_data->raw_reg = (u64)phys_to_virt(
@@ -2289,7 +2289,7 @@ static int __init octeon_irq_init_cib(struct device_node 
*ciu_node,
 
addr = of_get_address(ciu_node, 1, NULL, NULL);
if (!addr) {
-   pr_err("ERROR: Couldn't acquire reg(1) %s\n.", ciu_node->name);
+   pr_err("ERROR: Couldn't acquire reg(1) %s\n", ciu_node->name);
return -EINVAL;
}
host_data->en_reg = (u64)phys_to_virt(
@@ -2297,7 +2297,7 @@ static int __init octeon_irq_init_cib(struct device_node 
*ciu_node,
 
r = of_property_read_u32(ciu_node, "cavium,max-bits", );
if (r) {
-   pr_err("ERROR: Couldn't read cavium,max-bits from %s\n.",
+   pr_err("ERROR: Couldn't read cavium,max-bits from %s\n",
ciu_node->name);
return r;
}
@@ -2307,7 +2307,7 @@ static int __init octeon_irq_init_cib(struct device_node 
*ciu_node,
   _irq_domain_cib_ops,
   host_data);
if (!cib_domain) {
-   pr_err("ERROR: Couldn't irq_domain_add_linear()\n.");
+   pr_err("ERROR: Couldn't irq_domain_add_linear()\n");
return -ENOMEM;
}
 
-- 
2.15.0

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Matthew Wilcox]

On Tue, Dec 05, 2017 at 08:54:35PM -0800, Matthew Wilcox wrote:
> On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
> > Cyril Hrubis  writes:
> > 
> > > Hi!
> > >> > MAP_FIXED_UNIQUE
> > >> > MAP_FIXED_ONCE
> > >> > MAP_FIXED_FRESH
> > >> 
> > >> Well, I can open a poll for the best name, but none of those you are
> > >> proposing sound much better to me. Yeah, naming sucks...
> > >
> > > Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
> > > would probably be a best fit.
> > 
> > Yeah that could work.
> > 
> > I prefer "no clobber" as I just suggested, because the existing
> > MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
> > one - which you or another thread may be using - and clobbers it with
> > the new one.
> 
> It's longer than MAP_FIXED_WEAK :-P
> 
> You'd have to be pretty darn strong to clobber an existing mapping.

I think we're thinking about this all wrong.  We shouldn't document it as
"This is a variant of MAP_FIXED".  We should document it as "Here's an
alternative to MAP_FIXED".

So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
we could add a new paragraph saying "at most one of MAP_FIXED or
MAP_REQUIRED" and "any of the following values".

Now, we should implement MAP_REQUIRED as having each architecture
define _MAP_NOT_A_HINT, and then #define MAP_REQUIRED (MAP_FIXED |
_MAP_NOT_A_HINT), but that's not information to confuse users with.

Also, that lets us add a third option at some point that is Yet Another
Way to interpret the 'addr' argument, by having MAP_FIXED clear and
_MAP_NOT_A_HINT set.

I'm not set on MAP_REQUIRED.  I came up with some awful names
(MAP_TODDLER, MAP_TANTRUM, MAP_ULTIMATUM, MAP_BOSS, MAP_PROGRAM_MANAGER,
etc).  But I think we should drop FIXED from the middle of the name.

Re: [PATCH 0/2] mm: introduce MAP_FIXED_SAFE

[Matthew Wilcox]

On Tue, Dec 05, 2017 at 08:54:35PM -0800, Matthew Wilcox wrote:
> On Wed, Dec 06, 2017 at 03:51:44PM +1100, Michael Ellerman wrote:
> > Cyril Hrubis  writes:
> > 
> > > Hi!
> > >> > MAP_FIXED_UNIQUE
> > >> > MAP_FIXED_ONCE
> > >> > MAP_FIXED_FRESH
> > >> 
> > >> Well, I can open a poll for the best name, but none of those you are
> > >> proposing sound much better to me. Yeah, naming sucks...
> > >
> > > Given that MAP_FIXED replaces the previous mapping MAP_FIXED_NOREPLACE
> > > would probably be a best fit.
> > 
> > Yeah that could work.
> > 
> > I prefer "no clobber" as I just suggested, because the existing
> > MAP_FIXED doesn't politely "replace" a mapping, it destroys the current
> > one - which you or another thread may be using - and clobbers it with
> > the new one.
> 
> It's longer than MAP_FIXED_WEAK :-P
> 
> You'd have to be pretty darn strong to clobber an existing mapping.

I think we're thinking about this all wrong.  We shouldn't document it as
"This is a variant of MAP_FIXED".  We should document it as "Here's an
alternative to MAP_FIXED".

So, just like we currently say "exactly one of MAP_SHARED or MAP_PRIVATE",
we could add a new paragraph saying "at most one of MAP_FIXED or
MAP_REQUIRED" and "any of the following values".

Now, we should implement MAP_REQUIRED as having each architecture
define _MAP_NOT_A_HINT, and then #define MAP_REQUIRED (MAP_FIXED |
_MAP_NOT_A_HINT), but that's not information to confuse users with.

Also, that lets us add a third option at some point that is Yet Another
Way to interpret the 'addr' argument, by having MAP_FIXED clear and
_MAP_NOT_A_HINT set.

I'm not set on MAP_REQUIRED.  I came up with some awful names
(MAP_TODDLER, MAP_TANTRUM, MAP_ULTIMATUM, MAP_BOSS, MAP_PROGRAM_MANAGER,
etc).  But I think we should drop FIXED from the middle of the name.

Re: [PATCH] ARM: dts: introduce the sama5d2 ptc ek board

[Ludovic Desroches]

On Tue, Dec 05, 2017 at 07:01:41PM +0200, Baruch Siach wrote:
> Hi Ludovic,
> 
> On Tue, Dec 05, 2017 at 03:23:12PM +0100, Ludovic Desroches wrote:
> > Add the official SAMA5D2 Peripheral Touch Controller Evaluation
> > Kit board.
> > 
> > Signed-off-by: Ludovic Desroches 
> > ---
> 
> [...]
> 
> > +   memory {
> > +   reg = <0x2000 0x8>;
> > +   };
> 
> The size value is clearly wrong; you surely don't run on 512KB RAM. You most 
> likely rely on the bootloader to fix the size value. Since sama5d2.dtsi has a 
> memory node already with the same address, you can just drop it from here.
> 

Thanks, fixed in v2.

Ludovic

> baruch
> 
> -- 
>  http://baruch.siach.name/blog/  ~. .~   Tk Open Systems
> =}ooO--U--Ooo{=
>- bar...@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

Re: [PATCH] ARM: dts: introduce the sama5d2 ptc ek board

[Ludovic Desroches]

On Tue, Dec 05, 2017 at 07:01:41PM +0200, Baruch Siach wrote:
> Hi Ludovic,
> 
> On Tue, Dec 05, 2017 at 03:23:12PM +0100, Ludovic Desroches wrote:
> > Add the official SAMA5D2 Peripheral Touch Controller Evaluation
> > Kit board.
> > 
> > Signed-off-by: Ludovic Desroches 
> > ---
> 
> [...]
> 
> > +   memory {
> > +   reg = <0x2000 0x8>;
> > +   };
> 
> The size value is clearly wrong; you surely don't run on 512KB RAM. You most 
> likely rely on the bootloader to fix the size value. Since sama5d2.dtsi has a 
> memory node already with the same address, you can just drop it from here.
> 

Thanks, fixed in v2.

Ludovic

> baruch
> 
> -- 
>  http://baruch.siach.name/blog/  ~. .~   Tk Open Systems
> =}ooO--U--Ooo{=
>- bar...@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

[PATCH V2] ARM: dts: introduce the sama5d2 ptc ek board

[Ludovic Desroches]

Add the official SAMA5D2 Peripheral Touch Controller Evaluation
Kit board.

Signed-off-by: Ludovic Desroches 
---

Changes:
- v2:
  - remove memory node
  - use SPDX-License-Identifier

arch/arm/boot/dts/Makefile|   1 +
 arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts | 402 ++
 2 files changed, 403 insertions(+)
 create mode 100644 arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 043d7c720d0c..ed60582eb1da 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -48,6 +48,7 @@ dtb-$(CONFIG_SOC_AT91SAM9) += \
 dtb-$(CONFIG_SOC_SAM_V7) += \
at91-kizbox2.dtb \
at91-sama5d27_som1_ek.dtb \
+   at91-sama5d2_ptc_ek.dtb \
at91-sama5d2_xplained.dtb \
at91-sama5d3_xplained.dtb \
at91-tse850-3.dtb \
diff --git a/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts 
b/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts
new file mode 100644
index ..89ad23a7f92d
--- /dev/null
+++ b/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts
@@ -0,0 +1,402 @@
+/*
+ * at91-sama5d2_ptc_ek.dts - Device Tree file for SAMA5D2 PTC EK board
+ *
+ *  Copyright (C) 2017 Microchip/Atmel,
+ *   2017 Wenyou Yang 
+ *   2017 Ludovic Desroches 
+ *
+ * SPDX-License-Identifier: (GPL-2.0+ OR X11)
+ */
+/dts-v1/;
+#include "sama5d2.dtsi"
+#include "sama5d2-pinfunc.h"
+#include 
+#include 
+
+/ {
+   model = "Atmel SAMA5D2 PTC EK";
+   compatible = "atmel,sama5d2-ptc_ek", "atmel,sama5d2", "atmel,sama5";
+
+   aliases {
+   serial0 = 
+   i2c0= 
+   i2c1= 
+   i2c2= 
+   };
+
+   chosen {
+   stdout-path = "serial0:115200n8";
+   };
+
+   clocks {
+   slow_xtal {
+   clock-frequency = <32768>;
+   };
+
+   main_xtal {
+   clock-frequency = <2400>;
+   };
+   };
+
+   ahb {
+   usb0: gadget@30 {
+   atmel,vbus-gpio = < PIN_PA27 GPIO_ACTIVE_HIGH>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_usba_vbus>;
+   status = "okay";
+   };
+
+   usb1: ohci@40 {
+   num-ports = <3>;
+   atmel,vbus-gpio = <0
+   PIN_PB12 GPIO_ACTIVE_HIGH
+  0
+ >;
+   pinctrl-names = "default";
+   pinctrl-0 = <_usb_default>;
+   status = "okay";
+   };
+
+   usb2: ehci@50 {
+   status = "okay";
+   };
+
+   ebi: ebi@1000 {
+   pinctrl-names = "default";
+   pinctrl-0 = <_nand_default>;
+   status = "okay"; /* conflicts with sdmmc1 and qspi0 */
+
+   nand_controller: nand-controller {
+   status = "okay";
+
+   nand@3 {
+   reg = <0x3 0x0 0x2>;
+   atmel,rb = <0>;
+   nand-bus-width = <8>;
+   nand-ecc-mode = "hw";
+   nand-on-flash-bbt;
+   label = "atmel_nand";
+
+   partitions {
+   compatible = "fixed-partitions";
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   at91bootstrap@0 {
+   label = "bootstrap";
+   reg = <0x0 0x4>;
+   };
+
+   bootloader@4 {
+   label = "bootloader";
+   reg = <0x4 0xc>;
+   };
+
+   bootloaderenv@0x10 {
+   label = "bootloader 
env";
+   reg = <0x10 
0x4>;
+   };
+
+   bootloaderenvred@0x14 {
+   label = "bootloader env 
redundant";
+  

[PATCH V2] ARM: dts: introduce the sama5d2 ptc ek board

[Ludovic Desroches]

Add the official SAMA5D2 Peripheral Touch Controller Evaluation
Kit board.

Signed-off-by: Ludovic Desroches 
---

Changes:
- v2:
  - remove memory node
  - use SPDX-License-Identifier

arch/arm/boot/dts/Makefile|   1 +
 arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts | 402 ++
 2 files changed, 403 insertions(+)
 create mode 100644 arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 043d7c720d0c..ed60582eb1da 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -48,6 +48,7 @@ dtb-$(CONFIG_SOC_AT91SAM9) += \
 dtb-$(CONFIG_SOC_SAM_V7) += \
at91-kizbox2.dtb \
at91-sama5d27_som1_ek.dtb \
+   at91-sama5d2_ptc_ek.dtb \
at91-sama5d2_xplained.dtb \
at91-sama5d3_xplained.dtb \
at91-tse850-3.dtb \
diff --git a/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts 
b/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts
new file mode 100644
index ..89ad23a7f92d
--- /dev/null
+++ b/arch/arm/boot/dts/at91-sama5d2_ptc_ek.dts
@@ -0,0 +1,402 @@
+/*
+ * at91-sama5d2_ptc_ek.dts - Device Tree file for SAMA5D2 PTC EK board
+ *
+ *  Copyright (C) 2017 Microchip/Atmel,
+ *   2017 Wenyou Yang 
+ *   2017 Ludovic Desroches 
+ *
+ * SPDX-License-Identifier: (GPL-2.0+ OR X11)
+ */
+/dts-v1/;
+#include "sama5d2.dtsi"
+#include "sama5d2-pinfunc.h"
+#include 
+#include 
+
+/ {
+   model = "Atmel SAMA5D2 PTC EK";
+   compatible = "atmel,sama5d2-ptc_ek", "atmel,sama5d2", "atmel,sama5";
+
+   aliases {
+   serial0 = 
+   i2c0= 
+   i2c1= 
+   i2c2= 
+   };
+
+   chosen {
+   stdout-path = "serial0:115200n8";
+   };
+
+   clocks {
+   slow_xtal {
+   clock-frequency = <32768>;
+   };
+
+   main_xtal {
+   clock-frequency = <2400>;
+   };
+   };
+
+   ahb {
+   usb0: gadget@30 {
+   atmel,vbus-gpio = < PIN_PA27 GPIO_ACTIVE_HIGH>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_usba_vbus>;
+   status = "okay";
+   };
+
+   usb1: ohci@40 {
+   num-ports = <3>;
+   atmel,vbus-gpio = <0
+   PIN_PB12 GPIO_ACTIVE_HIGH
+  0
+ >;
+   pinctrl-names = "default";
+   pinctrl-0 = <_usb_default>;
+   status = "okay";
+   };
+
+   usb2: ehci@50 {
+   status = "okay";
+   };
+
+   ebi: ebi@1000 {
+   pinctrl-names = "default";
+   pinctrl-0 = <_nand_default>;
+   status = "okay"; /* conflicts with sdmmc1 and qspi0 */
+
+   nand_controller: nand-controller {
+   status = "okay";
+
+   nand@3 {
+   reg = <0x3 0x0 0x2>;
+   atmel,rb = <0>;
+   nand-bus-width = <8>;
+   nand-ecc-mode = "hw";
+   nand-on-flash-bbt;
+   label = "atmel_nand";
+
+   partitions {
+   compatible = "fixed-partitions";
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   at91bootstrap@0 {
+   label = "bootstrap";
+   reg = <0x0 0x4>;
+   };
+
+   bootloader@4 {
+   label = "bootloader";
+   reg = <0x4 0xc>;
+   };
+
+   bootloaderenv@0x10 {
+   label = "bootloader 
env";
+   reg = <0x10 
0x4>;
+   };
+
+   bootloaderenvred@0x14 {
+   label = "bootloader env 
redundant";
+   reg = <0x14 
0x4>;
+

Re: [PATCH] usb: dwc2: add optional usb ecc reset bit

[John Youn]

On 11/01/2017 08:35 AM, Dinh Nguyen wrote:
> The dwc2 USB controller in Stratix10 has an additional ECC reset bit that
> needs to get de-asserted in order for the controller to work properly.
>
> Signed-off-by: Dinh Nguyen 
> ---
>  drivers/usb/dwc2/core.h |  1 +
>  drivers/usb/dwc2/platform.c | 10 ++
>  2 files changed, 11 insertions(+)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 8367d4f9..a4b5f4e 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -920,6 +920,7 @@ struct dwc2_hsotg {
>   int irq;
>   struct clk *clk;
>   struct reset_control *reset;
> + struct reset_control *reset_ecc;
>
>   unsigned int queuing_high_bandwidth:1;
>   unsigned int srp_success:1;
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index daf0d37..d466e03 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -220,6 +220,15 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg 
> *hsotg)
>
>   reset_control_deassert(hsotg->reset);
>
> + hsotg->reset_ecc = devm_reset_control_get_optional(hsotg->dev, 
> "dwc2-ecc");
> + if (IS_ERR(hsotg->reset_ecc)) {
> + ret = PTR_ERR(hsotg->reset_ecc);
> + dev_err(hsotg->dev, "error getting reset control for ecc %d\n", 
> ret);
> + return ret;
> + }
> +
> + reset_control_deassert(hsotg->reset_ecc);
> +
>   /* Set default UTMI width */
>   hsotg->phyif = GUSBCFG_PHYIF16;
>
> @@ -318,6 +327,7 @@ static int dwc2_driver_remove(struct platform_device *dev)
>   dwc2_lowlevel_hw_disable(hsotg);
>
>   reset_control_assert(hsotg->reset);
> + reset_control_assert(hsotg->reset_ecc);
>
>   return 0;
>  }
>

Acked-by: John Youn 

John

Re: [PATCH] usb: dwc2: add optional usb ecc reset bit

[John Youn]

On 11/01/2017 08:35 AM, Dinh Nguyen wrote:
> The dwc2 USB controller in Stratix10 has an additional ECC reset bit that
> needs to get de-asserted in order for the controller to work properly.
>
> Signed-off-by: Dinh Nguyen 
> ---
>  drivers/usb/dwc2/core.h |  1 +
>  drivers/usb/dwc2/platform.c | 10 ++
>  2 files changed, 11 insertions(+)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 8367d4f9..a4b5f4e 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -920,6 +920,7 @@ struct dwc2_hsotg {
>   int irq;
>   struct clk *clk;
>   struct reset_control *reset;
> + struct reset_control *reset_ecc;
>
>   unsigned int queuing_high_bandwidth:1;
>   unsigned int srp_success:1;
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index daf0d37..d466e03 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -220,6 +220,15 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg 
> *hsotg)
>
>   reset_control_deassert(hsotg->reset);
>
> + hsotg->reset_ecc = devm_reset_control_get_optional(hsotg->dev, 
> "dwc2-ecc");
> + if (IS_ERR(hsotg->reset_ecc)) {
> + ret = PTR_ERR(hsotg->reset_ecc);
> + dev_err(hsotg->dev, "error getting reset control for ecc %d\n", 
> ret);
> + return ret;
> + }
> +
> + reset_control_deassert(hsotg->reset_ecc);
> +
>   /* Set default UTMI width */
>   hsotg->phyif = GUSBCFG_PHYIF16;
>
> @@ -318,6 +327,7 @@ static int dwc2_driver_remove(struct platform_device *dev)
>   dwc2_lowlevel_hw_disable(hsotg);
>
>   reset_control_assert(hsotg->reset);
> + reset_control_assert(hsotg->reset_ecc);
>
>   return 0;
>  }
>

Acked-by: John Youn 

John

A kernel warning will be triggered,during copy a big file into zstd compressed btrfs filesystem

[guoxuenan]

Hi all,

I have found a kernel warning of btrfs ,during run my  shell script in 
qemu qemu virtual machine.


Linux mainline version Linux 4.15-rc2 (ae64f9bd)


The script try to copy a big file(15G) into btrfs filesystem,when I 
login virtual machine,then run the script,A kernel warning will be 
triggered  within about ten file copies.

=
#!/bin/sh
function btrfs_test()
{
umount /mnt/vdb
mkfs.btrfs -f /dev/vdb
mount -o compress=$1 /dev/vdb /mnt/vdb
cp ../data.tar  /mnt/vdb
sleep 10
btrfs filesystem sync /mnt/vdb
}

function loop_test()
{

for k in $( seq 1 100 )
do
btrfs_test zstd
done
}
loop_test
==

Kernl Warning content:

[  291.809047] 
[  291.809981] WARNING: possible recursive locking detected
[  291.810913] 4.15.0-rc2-327.58.59.16.x86_64.debug+ #8 Not tainted
[  291.811908] 
[  291.812792] khugepaged/65 is trying to acquire lock:
[  291.813614]  (fs_reclaim){+.+.}, at: [<9263bd20>] 
fs_reclaim_acquire+0x12/0x40

[  291.814972]
[  291.814972] but task is already holding lock:
[  291.815912]  (fs_reclaim){+.+.}, at: [<9263bd20>] 
fs_reclaim_acquire+0x12/0x40

[  291.817263]
[  291.817263] other info that might help us debug this:
[  291.818355]  Possible unsafe locking scenario:
[  291.818355]
[  291.819348]CPU0
[  291.819743]
[  291.820121]   lock(fs_reclaim);
[  291.820689]   lock(fs_reclaim);
[  291.821223]
[  291.821223]  *** DEADLOCK ***
[  291.821223]
[  291.84]  May be due to missing lock nesting notation
[  291.84]
[  291.823411] 1 lock held by khugepaged/65:
[  291.824048]  #0:  (fs_reclaim){+.+.}, at: [<9263bd20>] 
fs_reclaim_acquire+0x12/0x40

[  291.825480]
[  291.825480] stack backtrace:
[  291.826155] CPU: 4 PID: 65 Comm: khugepaged Not tainted 
4.15.0-rc2-327.58.59.16.x86_64.debug+ #8
[  291.827696] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
BIOS 1.9.3-1.fc25 04/01/2014

[  291.829111] Call Trace:
[  291.829560]  dump_stack+0x7c/0xbf
[  291.830072]  __lock_acquire+0x8d6/0x1220
[  291.830837]  lock_acquire+0xc9/0x220
[  291.831425]  ? fs_reclaim_acquire+0x12/0x40
[  291.832110]  ? alloc_extent_state+0x21/0x1c0
[  291.832824]  fs_reclaim_acquire+0x35/0x40
[  291.833476]  ? fs_reclaim_acquire+0x12/0x40
[  291.834114]  kmem_cache_alloc+0x29/0x320
[  291.834777]  alloc_extent_state+0x21/0x1c0
[  291.835390]  __clear_extent_bit+0x2a5/0x3b0
[  291.836079]  try_release_extent_mapping+0x17c/0x200
[  291.836868]  __btrfs_releasepage+0x30/0x90
[  291.837519]  shrink_page_list+0x80d/0xf20
[  291.838158]  shrink_inactive_list+0x250/0x710
[  291.838875]  ? rcu_read_lock_sched_held+0x9b/0xb0
[  291.839666]  shrink_node_memcg+0x358/0x790
[  291.840282]  ? mem_cgroup_iter+0x98/0x530
[  291.840872]  shrink_node+0xe5/0x310
[  291.841387]  do_try_to_free_pages+0xe8/0x390
[  291.842013]  try_to_free_pages+0x146/0x3d0
[  291.842620]  __alloc_pages_slowpath+0x3d1/0xce1
[  291.843290]  __alloc_pages_nodemask+0x411/0x450
[  291.843989]  khugepaged_alloc_page+0x39/0x80
[  291.844626]  collapse_huge_page+0x78/0x9a0
[  291.845381]  ? khugepaged_scan_mm_slot+0x932/0xfa0
[  291.846113]  khugepaged_scan_mm_slot+0x953/0xfa0
[  291.846838]  ? khugepaged+0x130/0x5a0
[  291.847388]  khugepaged+0x2e0/0x5a0
[  291.848019]  ? remove_wait_queue+0x60/0x60
[  291.848678]  kthread+0x141/0x180
[  291.849239]  ? khugepaged_scan_mm_slot+0xfa0/0xfa0
[  291.849938]  ? kthread_stop+0x300/0x300
[  291.850514]  ret_from_fork+0x24/0x30

A kernel warning will be triggered,during copy a big file into zstd compressed btrfs filesystem

[guoxuenan]

Hi all,

I have found a kernel warning of btrfs ,during run my  shell script in 
qemu qemu virtual machine.


Linux mainline version Linux 4.15-rc2 (ae64f9bd)


The script try to copy a big file(15G) into btrfs filesystem,when I 
login virtual machine,then run the script,A kernel warning will be 
triggered  within about ten file copies.

=
#!/bin/sh
function btrfs_test()
{
umount /mnt/vdb
mkfs.btrfs -f /dev/vdb
mount -o compress=$1 /dev/vdb /mnt/vdb
cp ../data.tar  /mnt/vdb
sleep 10
btrfs filesystem sync /mnt/vdb
}

function loop_test()
{

for k in $( seq 1 100 )
do
btrfs_test zstd
done
}
loop_test
==

Kernl Warning content:

[  291.809047] 
[  291.809981] WARNING: possible recursive locking detected
[  291.810913] 4.15.0-rc2-327.58.59.16.x86_64.debug+ #8 Not tainted
[  291.811908] 
[  291.812792] khugepaged/65 is trying to acquire lock:
[  291.813614]  (fs_reclaim){+.+.}, at: [<9263bd20>] 
fs_reclaim_acquire+0x12/0x40

[  291.814972]
[  291.814972] but task is already holding lock:
[  291.815912]  (fs_reclaim){+.+.}, at: [<9263bd20>] 
fs_reclaim_acquire+0x12/0x40

[  291.817263]
[  291.817263] other info that might help us debug this:
[  291.818355]  Possible unsafe locking scenario:
[  291.818355]
[  291.819348]CPU0
[  291.819743]
[  291.820121]   lock(fs_reclaim);
[  291.820689]   lock(fs_reclaim);
[  291.821223]
[  291.821223]  *** DEADLOCK ***
[  291.821223]
[  291.84]  May be due to missing lock nesting notation
[  291.84]
[  291.823411] 1 lock held by khugepaged/65:
[  291.824048]  #0:  (fs_reclaim){+.+.}, at: [<9263bd20>] 
fs_reclaim_acquire+0x12/0x40

[  291.825480]
[  291.825480] stack backtrace:
[  291.826155] CPU: 4 PID: 65 Comm: khugepaged Not tainted 
4.15.0-rc2-327.58.59.16.x86_64.debug+ #8
[  291.827696] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), 
BIOS 1.9.3-1.fc25 04/01/2014

[  291.829111] Call Trace:
[  291.829560]  dump_stack+0x7c/0xbf
[  291.830072]  __lock_acquire+0x8d6/0x1220
[  291.830837]  lock_acquire+0xc9/0x220
[  291.831425]  ? fs_reclaim_acquire+0x12/0x40
[  291.832110]  ? alloc_extent_state+0x21/0x1c0
[  291.832824]  fs_reclaim_acquire+0x35/0x40
[  291.833476]  ? fs_reclaim_acquire+0x12/0x40
[  291.834114]  kmem_cache_alloc+0x29/0x320
[  291.834777]  alloc_extent_state+0x21/0x1c0
[  291.835390]  __clear_extent_bit+0x2a5/0x3b0
[  291.836079]  try_release_extent_mapping+0x17c/0x200
[  291.836868]  __btrfs_releasepage+0x30/0x90
[  291.837519]  shrink_page_list+0x80d/0xf20
[  291.838158]  shrink_inactive_list+0x250/0x710
[  291.838875]  ? rcu_read_lock_sched_held+0x9b/0xb0
[  291.839666]  shrink_node_memcg+0x358/0x790
[  291.840282]  ? mem_cgroup_iter+0x98/0x530
[  291.840872]  shrink_node+0xe5/0x310
[  291.841387]  do_try_to_free_pages+0xe8/0x390
[  291.842013]  try_to_free_pages+0x146/0x3d0
[  291.842620]  __alloc_pages_slowpath+0x3d1/0xce1
[  291.843290]  __alloc_pages_nodemask+0x411/0x450
[  291.843989]  khugepaged_alloc_page+0x39/0x80
[  291.844626]  collapse_huge_page+0x78/0x9a0
[  291.845381]  ? khugepaged_scan_mm_slot+0x932/0xfa0
[  291.846113]  khugepaged_scan_mm_slot+0x953/0xfa0
[  291.846838]  ? khugepaged+0x130/0x5a0
[  291.847388]  khugepaged+0x2e0/0x5a0
[  291.848019]  ? remove_wait_queue+0x60/0x60
[  291.848678]  kthread+0x141/0x180
[  291.849239]  ? khugepaged_scan_mm_slot+0xfa0/0xfa0
[  291.849938]  ? kthread_stop+0x300/0x300
[  291.850514]  ret_from_fork+0x24/0x30

Re: [PATCH 4.14 00/95] 4.14.4-stable review

[Greg Kroah-Hartman]

On Wed, Dec 06, 2017 at 07:49:49AM +0100, Greg Kroah-Hartman wrote:
> Don't fall down the trap of running code for the sake of running code
> (i.e. like that web site that starts with a P) that doesn't actually
> test anything that actually matters.

I forgot to add:

Especially when there are already so many tests out there that _do_
matter that are not getting run on the stable kernels today.

Re: [PATCH 4.14 00/95] 4.14.4-stable review

[Greg Kroah-Hartman]

On Wed, Dec 06, 2017 at 07:49:49AM +0100, Greg Kroah-Hartman wrote:
> Don't fall down the trap of running code for the sake of running code
> (i.e. like that web site that starts with a P) that doesn't actually
> test anything that actually matters.

I forgot to add:

Especially when there are already so many tests out there that _do_
matter that are not getting run on the stable kernels today.

Re: [PATCH 4.14 00/95] 4.14.4-stable review

[Greg Kroah-Hartman]

On Tue, Dec 05, 2017 at 03:45:07PM -0600, Tom Gall wrote:
> 
> 
> > On Dec 5, 2017, at 12:24 AM, Greg Kroah-Hartman 
> >  wrote:
> > 
> > On Mon, Dec 04, 2017 at 03:12:45PM -0600, Tom Gall wrote:
> >> 
> >> 
> >>> On Dec 4, 2017, at 9:59 AM, Greg Kroah-Hartman 
> >>>  wrote:
> >>> 
> >>> This is the start of the stable review cycle for the 4.14.4 release.
> >>> There are 95 patches in this series, all will be posted as a response
> >>> to this one.  If anyone has any issues with these being applied, please
> >>> let me know.
> >>> 
> >>> Responses should be made by Wed Dec  6 16:00:27 UTC 2017.
> >>> Anything received after that time might be too late.
> >>> 
> >>> The whole patch series can be found in one patch at:
> >>>   kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.14.4-rc1.gz
> >>> or in the git tree and branch at:
> >>> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> >>> linux-4.14.y
> >>> and the diffstat can be found below.
> >>> 
> >>> thanks,
> >>> 
> >>> greg k-h
> >>> 
> >> 
> >> Compiled, booted and ran the following package unit tests without 
> >> regressions on x86_64
> >> 
> >> boringssl : 
> >>   go test target:0/0/5764/5764/5764 PASS
> >>   ssl_test : 10 pass
> >>   crypto_test : 28 pass
> >> e2fsprogs:
> >>   make check : 340 pass
> >> sqlite
> >>   make test : 143914 pass
> >> drm
> >>   make check : 15 pass
> >>   modetest, drmdevice : pass
> >> alsa-lib
> >>   make check : 2 pass
> >> bluez
> >>   make check : 25 pass
> >> libusb
> >>   stress : 4 pass
> > 
> > How do the above tests stress the kernel?
> 
> Depends entirely on the package in question.
> 
> Sure, of completely no surprise a lot of package unit tests don’t really 
> do much that’s particularly interesting save to the package itself.

Then why run those tests?  Like sqlite, what kernel functionality does
that exercise that ltp does not?

> There are sometimes an interesting subset that drives some amount of work in 
> kernel. 
> That’s the useful stuff.

Is that true with the above list?  If so, why are those types of tests
not part of any kernel test suite that I have seen before?

> Take bluez, and it’s use of CONFIG_CRYPTO_USER_API.  

Nice, does that cover things that is not in LTP?  Should those tests be
added to LTP?

> >  Aren't they just
> > verifications that the source code in the package is correct?
> 
> So if there’s some useful subset, that’s what I’m looking for.
> 
> > I guess it proves something, but have you ever seen the above regress in
> > _any_ kernel release?
> 
> Past regressions make for a good test. 

You are testing past regressions of the userspace code, not the kernel
here.  Why do I care about that?  :)

Don't fall down the trap of running code for the sake of running code
(i.e. like that web site that starts with a P) that doesn't actually
test anything that actually matters.

thanks,

greg k-h

Re: [PATCH 4.14 00/95] 4.14.4-stable review

[Greg Kroah-Hartman]

On Tue, Dec 05, 2017 at 03:45:07PM -0600, Tom Gall wrote:
> 
> 
> > On Dec 5, 2017, at 12:24 AM, Greg Kroah-Hartman 
> >  wrote:
> > 
> > On Mon, Dec 04, 2017 at 03:12:45PM -0600, Tom Gall wrote:
> >> 
> >> 
> >>> On Dec 4, 2017, at 9:59 AM, Greg Kroah-Hartman 
> >>>  wrote:
> >>> 
> >>> This is the start of the stable review cycle for the 4.14.4 release.
> >>> There are 95 patches in this series, all will be posted as a response
> >>> to this one.  If anyone has any issues with these being applied, please
> >>> let me know.
> >>> 
> >>> Responses should be made by Wed Dec  6 16:00:27 UTC 2017.
> >>> Anything received after that time might be too late.
> >>> 
> >>> The whole patch series can be found in one patch at:
> >>>   kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.14.4-rc1.gz
> >>> or in the git tree and branch at:
> >>> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> >>> linux-4.14.y
> >>> and the diffstat can be found below.
> >>> 
> >>> thanks,
> >>> 
> >>> greg k-h
> >>> 
> >> 
> >> Compiled, booted and ran the following package unit tests without 
> >> regressions on x86_64
> >> 
> >> boringssl : 
> >>   go test target:0/0/5764/5764/5764 PASS
> >>   ssl_test : 10 pass
> >>   crypto_test : 28 pass
> >> e2fsprogs:
> >>   make check : 340 pass
> >> sqlite
> >>   make test : 143914 pass
> >> drm
> >>   make check : 15 pass
> >>   modetest, drmdevice : pass
> >> alsa-lib
> >>   make check : 2 pass
> >> bluez
> >>   make check : 25 pass
> >> libusb
> >>   stress : 4 pass
> > 
> > How do the above tests stress the kernel?
> 
> Depends entirely on the package in question.
> 
> Sure, of completely no surprise a lot of package unit tests don’t really 
> do much that’s particularly interesting save to the package itself.

Then why run those tests?  Like sqlite, what kernel functionality does
that exercise that ltp does not?

> There are sometimes an interesting subset that drives some amount of work in 
> kernel. 
> That’s the useful stuff.

Is that true with the above list?  If so, why are those types of tests
not part of any kernel test suite that I have seen before?

> Take bluez, and it’s use of CONFIG_CRYPTO_USER_API.  

Nice, does that cover things that is not in LTP?  Should those tests be
added to LTP?

> >  Aren't they just
> > verifications that the source code in the package is correct?
> 
> So if there’s some useful subset, that’s what I’m looking for.
> 
> > I guess it proves something, but have you ever seen the above regress in
> > _any_ kernel release?
> 
> Past regressions make for a good test. 

You are testing past regressions of the userspace code, not the kernel
here.  Why do I care about that?  :)

Don't fall down the trap of running code for the sake of running code
(i.e. like that web site that starts with a P) that doesn't actually
test anything that actually matters.

thanks,

greg k-h

Re: [PATCH] bfa: fix access to bfad_im_port_s

[James Bottomley]

On Tue, 2017-11-28 at 16:26 +0100, Johannes Thumshirn wrote:
> Commit 'cd21c605b2cf ("scsi: fc: provide fc_bsg_to_shost() helper")'
> changed access to bfa's 'struct bfad_im_port_s' by using shost_priv()
> instead of shost->hostdata[0].
> 
> This lead to crashes like in the following back-trace:
> 
> task: 880046375300 ti: 8800a2ef8000 task.ti: 8800a2ef8000
> RIP: e030:[]  []
> bfa_fcport_get_attr+0x82/0x260 [bfa]
> RSP: e02b:8800a2efba10  EFLAGS: 00010046
> RAX: 575f415441536432 RBX: 8800a2efba28 RCX: 
> RDX:  RSI: 8800a2efba28 RDI: 880004dc31d8
> RBP: 880004dc31d8 R08:  R09: 0001
> R10: 88011fadc468 R11: 0001 R12: 880004dc31f0
> R13: 0200 R14: 880004dc61d0 R15: 880004947a10
> FS:  7feb1e489700() GS:88011fac()
> knlGS:
> CS:  e033 DS:  ES:  CR0: 8005003b
> CR2: 7ffe14e46c10 CR3: 957b8000 CR4: 0660
> Stack:
>  88001d4da000 880004dc31c0 a048a9df 81e56380
>     
> [] bfad_iocmd_ioc_get_info+0x4f/0x220 [bfa]
> [] bfad_iocmd_handler+0xa00/0xd40 [bfa]
> [] bfad_im_bsg_request+0xee/0x1b0 [bfa]
> [] fc_bsg_dispatch+0x10b/0x1b0 [scsi_transport_fc]
> [] bsg_request_fn+0x11d/0x1c0
> [] __blk_run_queue+0x2f/0x40
> [] blk_execute_rq_nowait+0xa8/0x160
> [] blk_execute_rq+0x77/0x120
> [] bsg_ioctl+0x1b6/0x200
> [] do_vfs_ioctl+0x2cd/0x4a0
> [] SyS_ioctl+0x74/0x80
> [] entry_SYSCALL_64_fastpath+0x12/0x6d
> 
> Fixes: cd21c605b2cf ("scsi: fc: provide fc_bsg_to_shost() helper")
> Signed-off-by: Johannes Thumshirn 
> Cc: Michal Koutný 
> ---
>  drivers/scsi/bfa/bfad_bsg.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/bfa/bfad_bsg.c
> b/drivers/scsi/bfa/bfad_bsg.c
> index 72ca2a2e08e2..09ef68c8225f 100644
> --- a/drivers/scsi/bfa/bfad_bsg.c
> +++ b/drivers/scsi/bfa/bfad_bsg.c
> @@ -3135,7 +3135,8 @@ bfad_im_bsg_vendor_request(struct bsg_job *job)
>   struct fc_bsg_request *bsg_request = job->request;
>   struct fc_bsg_reply *bsg_reply = job->reply;
>   uint32_t vendor_cmd = bsg_request-
> >rqst_data.h_vendor.vendor_cmd[0];
> - struct bfad_im_port_s *im_port =
> shost_priv(fc_bsg_to_shost(job));
> + struct Scsi_Host *shost = fc_bsg_to_shost(job);
> + struct bfad_im_port_s *im_port = shost->hostdata[0];
>   struct bfad_s *bfad = im_port->bfad;
>   void *payload_kbuf;
>   int rc = -EINVAL;
> @@ -3350,7 +3351,8 @@ int
>  bfad_im_bsg_els_ct_request(struct bsg_job *job)
>  {
>   struct bfa_bsg_data *bsg_data;
> - struct bfad_im_port_s *im_port =
> shost_priv(fc_bsg_to_shost(job));
> + struct Scsi_Host *shost = fc_bsg_to_shost(job);
> + struct bfad_im_port_s *im_port = shost->hostdata[0];
>   struct bfad_s *bfad = im_port->bfad;
>   bfa_bsg_fcpt_t *bsg_fcpt;
>   struct bfad_fcxp*drv_fcxp;

OK, so we had a linux next failure with this:

After merging the scsi tree, today's linux-next build (x86_64
allmodconfig) produced these warnings:

drivers/scsi/bfa/bfad_bsg.c: In function 'bfad_im_bsg_vendor_request':
drivers/scsi/bfa/bfad_bsg.c:3137:35: warning: initialization makes pointer 
from integer without a cast [-Wint-conversion]
  struct bfad_im_port_s *im_port = shost->hostdata[0];
   ^
drivers/scsi/bfa/bfad_bsg.c: In function 'bfad_im_bsg_els_ct_request':
drivers/scsi/bfa/bfad_bsg.c:3353:35: warning: initialization makes pointer 
from integer without a cast [-Wint-conversion]
  struct bfad_im_port_s *im_port = shost->hostdata[0];
   ^

Introduced by commit

  45349821ab3a ("scsi: bfa: fix access to bfad_im_port_s")

-- 
Cheers,
Stephen Rothwell

The reason is you should be using shost_priv(shost) not shost->hostdata[0].

James

Re: [PATCH] bfa: fix access to bfad_im_port_s

[James Bottomley]

On Tue, 2017-11-28 at 16:26 +0100, Johannes Thumshirn wrote:
> Commit 'cd21c605b2cf ("scsi: fc: provide fc_bsg_to_shost() helper")'
> changed access to bfa's 'struct bfad_im_port_s' by using shost_priv()
> instead of shost->hostdata[0].
> 
> This lead to crashes like in the following back-trace:
> 
> task: 880046375300 ti: 8800a2ef8000 task.ti: 8800a2ef8000
> RIP: e030:[]  []
> bfa_fcport_get_attr+0x82/0x260 [bfa]
> RSP: e02b:8800a2efba10  EFLAGS: 00010046
> RAX: 575f415441536432 RBX: 8800a2efba28 RCX: 
> RDX:  RSI: 8800a2efba28 RDI: 880004dc31d8
> RBP: 880004dc31d8 R08:  R09: 0001
> R10: 88011fadc468 R11: 0001 R12: 880004dc31f0
> R13: 0200 R14: 880004dc61d0 R15: 880004947a10
> FS:  7feb1e489700() GS:88011fac()
> knlGS:
> CS:  e033 DS:  ES:  CR0: 8005003b
> CR2: 7ffe14e46c10 CR3: 957b8000 CR4: 0660
> Stack:
>  88001d4da000 880004dc31c0 a048a9df 81e56380
>     
> [] bfad_iocmd_ioc_get_info+0x4f/0x220 [bfa]
> [] bfad_iocmd_handler+0xa00/0xd40 [bfa]
> [] bfad_im_bsg_request+0xee/0x1b0 [bfa]
> [] fc_bsg_dispatch+0x10b/0x1b0 [scsi_transport_fc]
> [] bsg_request_fn+0x11d/0x1c0
> [] __blk_run_queue+0x2f/0x40
> [] blk_execute_rq_nowait+0xa8/0x160
> [] blk_execute_rq+0x77/0x120
> [] bsg_ioctl+0x1b6/0x200
> [] do_vfs_ioctl+0x2cd/0x4a0
> [] SyS_ioctl+0x74/0x80
> [] entry_SYSCALL_64_fastpath+0x12/0x6d
> 
> Fixes: cd21c605b2cf ("scsi: fc: provide fc_bsg_to_shost() helper")
> Signed-off-by: Johannes Thumshirn 
> Cc: Michal Koutný 
> ---
>  drivers/scsi/bfa/bfad_bsg.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/bfa/bfad_bsg.c
> b/drivers/scsi/bfa/bfad_bsg.c
> index 72ca2a2e08e2..09ef68c8225f 100644
> --- a/drivers/scsi/bfa/bfad_bsg.c
> +++ b/drivers/scsi/bfa/bfad_bsg.c
> @@ -3135,7 +3135,8 @@ bfad_im_bsg_vendor_request(struct bsg_job *job)
>   struct fc_bsg_request *bsg_request = job->request;
>   struct fc_bsg_reply *bsg_reply = job->reply;
>   uint32_t vendor_cmd = bsg_request-
> >rqst_data.h_vendor.vendor_cmd[0];
> - struct bfad_im_port_s *im_port =
> shost_priv(fc_bsg_to_shost(job));
> + struct Scsi_Host *shost = fc_bsg_to_shost(job);
> + struct bfad_im_port_s *im_port = shost->hostdata[0];
>   struct bfad_s *bfad = im_port->bfad;
>   void *payload_kbuf;
>   int rc = -EINVAL;
> @@ -3350,7 +3351,8 @@ int
>  bfad_im_bsg_els_ct_request(struct bsg_job *job)
>  {
>   struct bfa_bsg_data *bsg_data;
> - struct bfad_im_port_s *im_port =
> shost_priv(fc_bsg_to_shost(job));
> + struct Scsi_Host *shost = fc_bsg_to_shost(job);
> + struct bfad_im_port_s *im_port = shost->hostdata[0];
>   struct bfad_s *bfad = im_port->bfad;
>   bfa_bsg_fcpt_t *bsg_fcpt;
>   struct bfad_fcxp*drv_fcxp;

OK, so we had a linux next failure with this:

After merging the scsi tree, today's linux-next build (x86_64
allmodconfig) produced these warnings:

drivers/scsi/bfa/bfad_bsg.c: In function 'bfad_im_bsg_vendor_request':
drivers/scsi/bfa/bfad_bsg.c:3137:35: warning: initialization makes pointer 
from integer without a cast [-Wint-conversion]
  struct bfad_im_port_s *im_port = shost->hostdata[0];
   ^
drivers/scsi/bfa/bfad_bsg.c: In function 'bfad_im_bsg_els_ct_request':
drivers/scsi/bfa/bfad_bsg.c:3353:35: warning: initialization makes pointer 
from integer without a cast [-Wint-conversion]
  struct bfad_im_port_s *im_port = shost->hostdata[0];
   ^

Introduced by commit

  45349821ab3a ("scsi: bfa: fix access to bfad_im_port_s")

-- 
Cheers,
Stephen Rothwell

The reason is you should be using shost_priv(shost) not shost->hostdata[0].

James

[PATCH] usb: xhci: fix TDS for MTK xHCI1.1

[Chunfeng Yun]

For MTK's xHCI 1.0 or latter, TD size is the number of max
packet sized packets remaining in the TD, not including
this TRB (following spec).

For MTK's xHCI 0.96 and older, TD size is the number of max
packet sized packets remaining in the TD, including this TRB
(not following spec).

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/host/xhci-ring.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c239c68..0619869 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3108,7 +3108,7 @@ static u32 xhci_td_remainder(struct xhci_hcd *xhci, int 
transferred,
 {
u32 maxp, total_packet_count;
 
-   /* MTK xHCI is mostly 0.97 but contains some features from 1.0 */
+   /* MTK xHCI 0.96 contains some features from 1.0 */
if (xhci->hci_version < 0x100 && !(xhci->quirks & XHCI_MTK_HOST))
return ((td_total_len - transferred) >> 10);
 
@@ -3117,8 +3117,8 @@ static u32 xhci_td_remainder(struct xhci_hcd *xhci, int 
transferred,
trb_buff_len == td_total_len)
return 0;
 
-   /* for MTK xHCI, TD size doesn't include this TRB */
-   if (xhci->quirks & XHCI_MTK_HOST)
+   /* for MTK xHCI 0.96, TD size include this TRB, but not in 1.x */
+   if ((xhci->quirks & XHCI_MTK_HOST) && (xhci->hci_version < 0x100))
trb_buff_len = 0;
 
maxp = usb_endpoint_maxp(>ep->desc);
-- 
1.9.1

[PATCH] usb: xhci: fix TDS for MTK xHCI1.1

[Chunfeng Yun]

For MTK's xHCI 1.0 or latter, TD size is the number of max
packet sized packets remaining in the TD, not including
this TRB (following spec).

For MTK's xHCI 0.96 and older, TD size is the number of max
packet sized packets remaining in the TD, including this TRB
(not following spec).

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/host/xhci-ring.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c239c68..0619869 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3108,7 +3108,7 @@ static u32 xhci_td_remainder(struct xhci_hcd *xhci, int 
transferred,
 {
u32 maxp, total_packet_count;
 
-   /* MTK xHCI is mostly 0.97 but contains some features from 1.0 */
+   /* MTK xHCI 0.96 contains some features from 1.0 */
if (xhci->hci_version < 0x100 && !(xhci->quirks & XHCI_MTK_HOST))
return ((td_total_len - transferred) >> 10);
 
@@ -3117,8 +3117,8 @@ static u32 xhci_td_remainder(struct xhci_hcd *xhci, int 
transferred,
trb_buff_len == td_total_len)
return 0;
 
-   /* for MTK xHCI, TD size doesn't include this TRB */
-   if (xhci->quirks & XHCI_MTK_HOST)
+   /* for MTK xHCI 0.96, TD size include this TRB, but not in 1.x */
+   if ((xhci->quirks & XHCI_MTK_HOST) && (xhci->hci_version < 0x100))
trb_buff_len = 0;
 
maxp = usb_endpoint_maxp(>ep->desc);
-- 
1.9.1

[PATCH] xen-netback: Fix logging message with spurious period after newline

[Joe Perches]

Using a period after a newline causes bad output.

Signed-off-by: Joe Perches 
---
 drivers/net/xen-netback/interface.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/xen-netback/interface.c 
b/drivers/net/xen-netback/interface.c
index d6dff347f896..78ebe494fef0 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -186,7 +186,7 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct 
net_device *dev)
/* Obtain the queue to be used to transmit this packet */
index = skb_get_queue_mapping(skb);
if (index >= num_queues) {
-   pr_warn_ratelimited("Invalid queue %hu for packet on interface 
%s\n.",
+   pr_warn_ratelimited("Invalid queue %hu for packet on interface 
%s\n",
index, vif->dev->name);
index %= num_queues;
}
-- 
2.15.0

[PATCH] xen-netback: Fix logging message with spurious period after newline

[Joe Perches]

Using a period after a newline causes bad output.

Signed-off-by: Joe Perches 
---
 drivers/net/xen-netback/interface.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/xen-netback/interface.c 
b/drivers/net/xen-netback/interface.c
index d6dff347f896..78ebe494fef0 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -186,7 +186,7 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct 
net_device *dev)
/* Obtain the queue to be used to transmit this packet */
index = skb_get_queue_mapping(skb);
if (index >= num_queues) {
-   pr_warn_ratelimited("Invalid queue %hu for packet on interface 
%s\n.",
+   pr_warn_ratelimited("Invalid queue %hu for packet on interface 
%s\n",
index, vif->dev->name);
index %= num_queues;
}
-- 
2.15.0

Re: [PATCH] usb: core: Fix logging messages with spurious periods after newlines

[Joe Perches]

On Wed, 2017-12-06 at 07:27 +0100, Greg Kroah-Hartman wrote:
> On Tue, Dec 05, 2017 at 10:22:05PM -0800, Joe Perches wrote:
> > Using a period after a newline causes bad output.
> 
> Nice catch, how did you find that?

$ git grep '\\n\."'

Re: [PATCH] usb: core: Fix logging messages with spurious periods after newlines

[Joe Perches]

On Wed, 2017-12-06 at 07:27 +0100, Greg Kroah-Hartman wrote:
> On Tue, Dec 05, 2017 at 10:22:05PM -0800, Joe Perches wrote:
> > Using a period after a newline causes bad output.
> 
> Nice catch, how did you find that?

$ git grep '\\n\."'

Re: [PATCH] efi: move some sysfs files to be read-only by root

[Greg Kroah-Hartman]

On Wed, Dec 06, 2017 at 07:50:41AM +1100, Tobin C. Harding wrote:
> On Tue, Dec 05, 2017 at 11:13:43AM +0100, Greg Kroah-Hartman wrote:
> > Thanks to the scripts/leaking_addresses.pl script, it was found that
> > some EFI values should not be readable by non-root users.
> > 
> > So make them root-only, and to do that, add a __ATTR_RO_MODE() macro to
> > make this easier, and use it in other places at the same time.
> > 
> > Reported-by: Linus Torvalds 
> > Tested-by: Dave Young 
> > Cc: Matt Fleming 
> > Cc: Ard Biesheuvel 
> > Cc: stable 
> > Signed-off-by: Greg Kroah-Hartman 
> > 
> > ---
> >  drivers/firmware/efi/efi.c |3 +--
> >  drivers/firmware/efi/esrt.c|   15 ++-
> >  drivers/firmware/efi/runtime-map.c |   10 +-
> >  include/linux/sysfs.h  |5 +
> >  4 files changed, 17 insertions(+), 16 deletions(-)
> > 
> > --- a/drivers/firmware/efi/efi.c
> > +++ b/drivers/firmware/efi/efi.c
> > @@ -143,8 +143,7 @@ static ssize_t systab_show(struct kobjec
> > return str - buf;
> >  }
> 
> Greg, do you add the CC's here in the commit log for a technical reason?
> Is it so that future investigation that leads to this commit can see who
> to involve in any further discussion?

They came from the output of scripts/get_maintainer.pl on who I should
be sending the patch to, and who should hopefully review it.

> As an example, for the patch that added the %p hashing should I have
> CC'd Jason A. Donenfeld since he was the brains behind the SipHash
> stuff and gave loads of suggestions/direction?

If you want to.  It's also a good way for me to track who the patch gets
sent to when doing multiple versions of a patch series.  git send-email
picks those up and sends the patch to them as well, making it easier on
the developer instead of having to remember a long --cc= list of
addresses.

thanks,

greg k-h

Re: [PATCH] efi: move some sysfs files to be read-only by root

[Greg Kroah-Hartman]

On Wed, Dec 06, 2017 at 07:50:41AM +1100, Tobin C. Harding wrote:
> On Tue, Dec 05, 2017 at 11:13:43AM +0100, Greg Kroah-Hartman wrote:
> > Thanks to the scripts/leaking_addresses.pl script, it was found that
> > some EFI values should not be readable by non-root users.
> > 
> > So make them root-only, and to do that, add a __ATTR_RO_MODE() macro to
> > make this easier, and use it in other places at the same time.
> > 
> > Reported-by: Linus Torvalds 
> > Tested-by: Dave Young 
> > Cc: Matt Fleming 
> > Cc: Ard Biesheuvel 
> > Cc: stable 
> > Signed-off-by: Greg Kroah-Hartman 
> > 
> > ---
> >  drivers/firmware/efi/efi.c |3 +--
> >  drivers/firmware/efi/esrt.c|   15 ++-
> >  drivers/firmware/efi/runtime-map.c |   10 +-
> >  include/linux/sysfs.h  |5 +
> >  4 files changed, 17 insertions(+), 16 deletions(-)
> > 
> > --- a/drivers/firmware/efi/efi.c
> > +++ b/drivers/firmware/efi/efi.c
> > @@ -143,8 +143,7 @@ static ssize_t systab_show(struct kobjec
> > return str - buf;
> >  }
> 
> Greg, do you add the CC's here in the commit log for a technical reason?
> Is it so that future investigation that leads to this commit can see who
> to involve in any further discussion?

They came from the output of scripts/get_maintainer.pl on who I should
be sending the patch to, and who should hopefully review it.

> As an example, for the patch that added the %p hashing should I have
> CC'd Jason A. Donenfeld since he was the brains behind the SipHash
> stuff and gave loads of suggestions/direction?

If you want to.  It's also a good way for me to track who the patch gets
sent to when doing multiple versions of a patch series.  git send-email
picks those up and sends the patch to them as well, making it easier on
the developer instead of having to remember a long --cc= list of
addresses.

thanks,

greg k-h

Re: [PATCH] driver-core: platform: Avoid to return IRQ 0 in platform_get_irq()

[Arvind Yadav]

Hi Dmitry,


On Tuesday 05 December 2017 11:50 PM, Dmitry Torokhov wrote:

On Tue, Dec 05, 2017 at 11:41:52PM +0530, Arvind Yadav wrote:

Function platform_get_irq() can return 0. Which means NO_IRQ.
So this change will not allow to return 0.

This change is help to use platform_get_irq() without this,

val = platform_get_irq();
if (val <= 0)
ret = val ? val : -ENODEV;

Signed-off-by: Arvind Yadav 

You need to audit the drivers and make sure you are not breaking them
with this. I believe at least i2c designware controller on certain
boards has DEFINE_RES_IRQ(0) which will be broken by this patch.

Yes, you are right. Few driver is using  DEFINE_RES_IRQ(0).
Example:
drivers/mfd/intel-lpss.c
static const struct resource intel_lpss_dev_resources[] = {
DEFINE_RES_MEM_NAMED(LPSS_DEV_OFFSET, LPSS_DEV_SIZE, "lpss_dev"),
DEFINE_RES_MEM_NAMED(LPSS_PRIV_OFFSET, LPSS_PRIV_SIZE, 
"lpss_priv"),

DEFINE_RES_IRQ(0),
};
There's a bunch of platforms in the kernel that still use IRQ0. They can 
suffer because

of this change. I should have mark this patch for testing.



---
  drivers/base/platform.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index c203fb9..7b3079c 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -127,7 +127,7 @@ int platform_get_irq(struct platform_device *dev, unsigned 
int num)
irqd_set_trigger_type(irqd, r->flags & IORESOURCE_BITS);
}
  
-	return r ? r->start : -ENXIO;

+   return r && r->start ? r->start : -ENXIO;
  #endif
  }
  EXPORT_SYMBOL_GPL(platform_get_irq);
--
2.7.4


Thanks.


Thanks,

Re: [PATCH] driver-core: platform: Avoid to return IRQ 0 in platform_get_irq()

[Arvind Yadav]

Hi Dmitry,


On Tuesday 05 December 2017 11:50 PM, Dmitry Torokhov wrote:

On Tue, Dec 05, 2017 at 11:41:52PM +0530, Arvind Yadav wrote:

Function platform_get_irq() can return 0. Which means NO_IRQ.
So this change will not allow to return 0.

This change is help to use platform_get_irq() without this,

val = platform_get_irq();
if (val <= 0)
ret = val ? val : -ENODEV;

Signed-off-by: Arvind Yadav 

You need to audit the drivers and make sure you are not breaking them
with this. I believe at least i2c designware controller on certain
boards has DEFINE_RES_IRQ(0) which will be broken by this patch.

Yes, you are right. Few driver is using  DEFINE_RES_IRQ(0).
Example:
drivers/mfd/intel-lpss.c
static const struct resource intel_lpss_dev_resources[] = {
DEFINE_RES_MEM_NAMED(LPSS_DEV_OFFSET, LPSS_DEV_SIZE, "lpss_dev"),
DEFINE_RES_MEM_NAMED(LPSS_PRIV_OFFSET, LPSS_PRIV_SIZE, 
"lpss_priv"),

DEFINE_RES_IRQ(0),
};
There's a bunch of platforms in the kernel that still use IRQ0. They can 
suffer because

of this change. I should have mark this patch for testing.



---
  drivers/base/platform.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index c203fb9..7b3079c 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -127,7 +127,7 @@ int platform_get_irq(struct platform_device *dev, unsigned 
int num)
irqd_set_trigger_type(irqd, r->flags & IORESOURCE_BITS);
}
  
-	return r ? r->start : -ENXIO;

+   return r && r->start ? r->start : -ENXIO;
  #endif
  }
  EXPORT_SYMBOL_GPL(platform_get_irq);
--
2.7.4


Thanks.


Thanks,

Re: [PATCH v6 2/2] staging: ion: create one device entry per heap

[Greg KH]

On Tue, Dec 05, 2017 at 03:01:42PM -0800, Laura Abbott wrote:
> On 12/02/2017 07:53 AM, Greg KH wrote:
> > > This is one of the item in the TODO list before been able to unstage ION
> > > which is my real need.
> > Why does it matter where in the tree this code is?  Don't go adding new
> > things to it that are not needed.  Who needs this?  What userspace code
> > wants this type of multiple ion devices?
> > 
> 
> Requirements came in from several places to split /dev/ion -> /dev/ion0
> and /dev/ion1 so that security policy (i.e. selinux) could be used to
> protect access to certain heaps. I wanted the ABI to be settled before
> trying to move out of staging, hence the line in the TODO list about
> doing the split.

Ok, but we should have some way of testing it works, right?  :)

Re: [PATCH v6 2/2] staging: ion: create one device entry per heap

[Greg KH]

On Tue, Dec 05, 2017 at 03:01:42PM -0800, Laura Abbott wrote:
> On 12/02/2017 07:53 AM, Greg KH wrote:
> > > This is one of the item in the TODO list before been able to unstage ION
> > > which is my real need.
> > Why does it matter where in the tree this code is?  Don't go adding new
> > things to it that are not needed.  Who needs this?  What userspace code
> > wants this type of multiple ion devices?
> > 
> 
> Requirements came in from several places to split /dev/ion -> /dev/ion0
> and /dev/ion1 so that security policy (i.e. selinux) could be used to
> protect access to certain heaps. I wanted the ABI to be settled before
> trying to move out of staging, hence the line in the TODO list about
> doing the split.

Ok, but we should have some way of testing it works, right?  :)

Re: [PATCH] usb: core: Fix logging messages with spurious periods after newlines

[Greg Kroah-Hartman]

On Tue, Dec 05, 2017 at 10:22:05PM -0800, Joe Perches wrote:
> Using a period after a newline causes bad output.

Nice catch, how did you find that?

thanks,

greg k-h

Re: [PATCH] usb: core: Fix logging messages with spurious periods after newlines

[Greg Kroah-Hartman]

On Tue, Dec 05, 2017 at 10:22:05PM -0800, Joe Perches wrote:
> Using a period after a newline causes bad output.

Nice catch, how did you find that?

thanks,

greg k-h

Re: In kernel power management domain_pm created for async schedules

[gre...@linuxfoundation.org]

On Wed, Dec 06, 2017 at 05:38:31AM +, Vikas Bansal wrote:
> If there is a driver in system which starts creating async schedules just 
> after resume (Same as our case, in which we faced issue). Then 
> async_synchronize_full API in PM cores starts waiting for completion of async 
> schedules created by that driver (Even though those are in a domain). Because 
> of this kernel resume time is increased (We faces the same issue) and whole 
> system is delayed. 
> This problem can be solved by creating is domain for async schedules in PM 
> core (As we solved in our case). Below patch is for solving this problem.

Please properly line-wrap your changelog text, _AND_ put it in the
changelog itself. Right now you have a patch:

> >From b8ea152eed6eef3b53275e7dd240a4d2124e9d4d Mon Sep 17 00:00:00 2001
> From: Anuj Gupta 
> Date: Tue, 5 Dec 2017 21:34:49 -0800
> Subject: [PATCH] Added domain_pm to PM core
> 
> 
> Signed-off-by: Vikas Bansal 

With no changelog text at all.

Please look at any of the hundreds of patches on the mailing list as an
example of how to properly format this.

Also, you didn't address my review comment I made last time, why not?

thanks,

greg k-h

Re: In kernel power management domain_pm created for async schedules

[gre...@linuxfoundation.org]

On Wed, Dec 06, 2017 at 05:38:31AM +, Vikas Bansal wrote:
> If there is a driver in system which starts creating async schedules just 
> after resume (Same as our case, in which we faced issue). Then 
> async_synchronize_full API in PM cores starts waiting for completion of async 
> schedules created by that driver (Even though those are in a domain). Because 
> of this kernel resume time is increased (We faces the same issue) and whole 
> system is delayed. 
> This problem can be solved by creating is domain for async schedules in PM 
> core (As we solved in our case). Below patch is for solving this problem.

Please properly line-wrap your changelog text, _AND_ put it in the
changelog itself. Right now you have a patch:

> >From b8ea152eed6eef3b53275e7dd240a4d2124e9d4d Mon Sep 17 00:00:00 2001
> From: Anuj Gupta 
> Date: Tue, 5 Dec 2017 21:34:49 -0800
> Subject: [PATCH] Added domain_pm to PM core
> 
> 
> Signed-off-by: Vikas Bansal 

With no changelog text at all.

Please look at any of the hundreds of patches on the mailing list as an
example of how to properly format this.

Also, you didn't address my review comment I made last time, why not?

thanks,

greg k-h

[PATCH] usb: core: Fix logging messages with spurious periods after newlines

[Joe Perches]

Using a period after a newline causes bad output.

Miscellanea:

o Coalesce formats too

Signed-off-by: Joe Perches 
---
 drivers/usb/core/driver.c  |  8 
 drivers/usb/core/hub.c | 17 +++--
 drivers/usb/core/message.c |  6 +++---
 3 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 64262a9a8829..d8d7377b5fb8 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -342,8 +342,8 @@ static int usb_probe_interface(struct device *dev)
if (driver->disable_hub_initiated_lpm) {
lpm_disable_error = usb_unlocked_disable_lpm(udev);
if (lpm_disable_error) {
-   dev_err(>dev, "%s Failed to disable LPM for 
driver %s\n.",
-   __func__, driver->name);
+   dev_err(>dev, "%s Failed to disable LPM for 
driver %s\n",
+   __func__, driver->name);
error = lpm_disable_error;
goto err;
}
@@ -537,8 +537,8 @@ int usb_driver_claim_interface(struct usb_driver *driver,
if (driver->disable_hub_initiated_lpm) {
lpm_disable_error = usb_unlocked_disable_lpm(udev);
if (lpm_disable_error) {
-   dev_err(>dev, "%s Failed to disable LPM for 
driver %s\n.",
-   __func__, driver->name);
+   dev_err(>dev, "%s Failed to disable LPM for 
driver %s\n",
+   __func__, driver->name);
return -ENOMEM;
}
}
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index cf7bbcb9a63c..b95855076f43 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1049,12 +1049,10 @@ static void hub_activate(struct usb_hub *hub, enum 
hub_activation_type type)
ret = hcd->driver->update_hub_device(hcd, hdev,
>tt, GFP_NOIO);
if (ret < 0) {
-   dev_err(hub->intfdev, "Host not "
-   "accepting hub info "
-   "update.\n");
-   dev_err(hub->intfdev, "LS/FS devices "
-   "and hubs may not work "
-   "under this hub\n.");
+   dev_err(hub->intfdev,
+   "Host not accepting hub info 
update\n");
+   dev_err(hub->intfdev,
+   "LS/FS devices and hubs may not 
work under this hub\n");
}
}
hub_power_on(hub, true);
@@ -3157,7 +3155,7 @@ int usb_port_suspend(struct usb_device *udev, 
pm_message_t msg)
usb_set_usb2_hardware_lpm(udev, 0);
 
if (usb_disable_ltm(udev)) {
-   dev_err(>dev, "Failed to disable LTM before suspend\n.");
+   dev_err(>dev, "Failed to disable LTM before suspend\n");
status = -ENOMEM;
if (PMSG_IS_AUTO(msg))
goto err_ltm;
@@ -5484,13 +5482,12 @@ static int usb_reset_and_verify_device(struct 
usb_device *udev)
 */
ret = usb_unlocked_disable_lpm(udev);
if (ret) {
-   dev_err(>dev, "%s Failed to disable LPM\n.", __func__);
+   dev_err(>dev, "%s Failed to disable LPM\n", __func__);
goto re_enumerate_no_bos;
}
ret = usb_disable_ltm(udev);
if (ret) {
-   dev_err(>dev, "%s Failed to disable LTM\n.",
-   __func__);
+   dev_err(>dev, "%s Failed to disable LTM\n", __func__);
goto re_enumerate_no_bos;
}
 
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index f836bae1e485..dac4adeec213 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1356,7 +1356,7 @@ int usb_set_interface(struct usb_device *dev, int 
interface, int alternate)
 * so that the xHCI driver can recalculate the U1/U2 timeouts.
 */
if (usb_disable_lpm(dev)) {
-   dev_err(>dev, "%s Failed to disable LPM\n.", __func__);
+   dev_err(>dev, "%s Failed to disable LPM\n", __func__);
mutex_unlock(hcd->bandwidth_mutex);
return -ENOMEM;
}
@@ -1500,7 +1500,7 @@ int usb_reset_configuration(struct usb_device *dev)
 * that the xHCI driver can recalculate the U1/U2 timeouts.
 */
if (usb_disable_lpm(dev)) {
-   dev_err(>dev, "%s Failed to 

[PATCH] usb: core: Fix logging messages with spurious periods after newlines

[Joe Perches]

Using a period after a newline causes bad output.

Miscellanea:

o Coalesce formats too

Signed-off-by: Joe Perches 
---
 drivers/usb/core/driver.c  |  8 
 drivers/usb/core/hub.c | 17 +++--
 drivers/usb/core/message.c |  6 +++---
 3 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 64262a9a8829..d8d7377b5fb8 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -342,8 +342,8 @@ static int usb_probe_interface(struct device *dev)
if (driver->disable_hub_initiated_lpm) {
lpm_disable_error = usb_unlocked_disable_lpm(udev);
if (lpm_disable_error) {
-   dev_err(>dev, "%s Failed to disable LPM for 
driver %s\n.",
-   __func__, driver->name);
+   dev_err(>dev, "%s Failed to disable LPM for 
driver %s\n",
+   __func__, driver->name);
error = lpm_disable_error;
goto err;
}
@@ -537,8 +537,8 @@ int usb_driver_claim_interface(struct usb_driver *driver,
if (driver->disable_hub_initiated_lpm) {
lpm_disable_error = usb_unlocked_disable_lpm(udev);
if (lpm_disable_error) {
-   dev_err(>dev, "%s Failed to disable LPM for 
driver %s\n.",
-   __func__, driver->name);
+   dev_err(>dev, "%s Failed to disable LPM for 
driver %s\n",
+   __func__, driver->name);
return -ENOMEM;
}
}
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index cf7bbcb9a63c..b95855076f43 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1049,12 +1049,10 @@ static void hub_activate(struct usb_hub *hub, enum 
hub_activation_type type)
ret = hcd->driver->update_hub_device(hcd, hdev,
>tt, GFP_NOIO);
if (ret < 0) {
-   dev_err(hub->intfdev, "Host not "
-   "accepting hub info "
-   "update.\n");
-   dev_err(hub->intfdev, "LS/FS devices "
-   "and hubs may not work "
-   "under this hub\n.");
+   dev_err(hub->intfdev,
+   "Host not accepting hub info 
update\n");
+   dev_err(hub->intfdev,
+   "LS/FS devices and hubs may not 
work under this hub\n");
}
}
hub_power_on(hub, true);
@@ -3157,7 +3155,7 @@ int usb_port_suspend(struct usb_device *udev, 
pm_message_t msg)
usb_set_usb2_hardware_lpm(udev, 0);
 
if (usb_disable_ltm(udev)) {
-   dev_err(>dev, "Failed to disable LTM before suspend\n.");
+   dev_err(>dev, "Failed to disable LTM before suspend\n");
status = -ENOMEM;
if (PMSG_IS_AUTO(msg))
goto err_ltm;
@@ -5484,13 +5482,12 @@ static int usb_reset_and_verify_device(struct 
usb_device *udev)
 */
ret = usb_unlocked_disable_lpm(udev);
if (ret) {
-   dev_err(>dev, "%s Failed to disable LPM\n.", __func__);
+   dev_err(>dev, "%s Failed to disable LPM\n", __func__);
goto re_enumerate_no_bos;
}
ret = usb_disable_ltm(udev);
if (ret) {
-   dev_err(>dev, "%s Failed to disable LTM\n.",
-   __func__);
+   dev_err(>dev, "%s Failed to disable LTM\n", __func__);
goto re_enumerate_no_bos;
}
 
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index f836bae1e485..dac4adeec213 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1356,7 +1356,7 @@ int usb_set_interface(struct usb_device *dev, int 
interface, int alternate)
 * so that the xHCI driver can recalculate the U1/U2 timeouts.
 */
if (usb_disable_lpm(dev)) {
-   dev_err(>dev, "%s Failed to disable LPM\n.", __func__);
+   dev_err(>dev, "%s Failed to disable LPM\n", __func__);
mutex_unlock(hcd->bandwidth_mutex);
return -ENOMEM;
}
@@ -1500,7 +1500,7 @@ int usb_reset_configuration(struct usb_device *dev)
 * that the xHCI driver can recalculate the U1/U2 timeouts.
 */
if (usb_disable_lpm(dev)) {
-   dev_err(>dev, "%s Failed to disable LPM\n.", 

Re: [PATCH] usb: dwc2: Fix TxFIFOn sizes and total TxFIFO size issues

[John Youn]

On 11/30/2017 12:16 AM, Minas Harutyunyan wrote:
> In host mode reading from DPTXSIZn returning invalid value in
> dwc2_check_param_tx_fifo_sizes function.
>
> In total TxFIFO size calculations unnecessarily reducing by ep_info.
> hw->total_fifo_size can be fully allocated for FIFO's.
>
> Added num_dev_in_eps member in dwc2_hw_params structure to save number
> of IN EPs.
>
> Added g_tx_fifo_size array in dwc2_hw_params structure to store power
> on reset values of DPTXSIZn registers in forced device mode.
>
> Updated dwc2_hsotg_tx_fifo_count() function to get TxFIFO count from
> num_dev_in_eps.
>
> Updated dwc2_get_dev_hwparams() function to store DPTXFSIZn in
> g_tx_fifo_size array.
>
> dwc2_get_host/dev_hwparams() functions call moved after num_dev_in_eps
> set from hwcfg4.
>
> Modified dwc2_check_param_tx_fifo_sizes() function to check TxFIFOn
> sizes based on g_tx_fifo_size array.
>
> Removed ep_info subtraction during calculation of tx_addr_max in
> dwc2_hsotg_tx_fifo_total_depth() function. Also removed
> dwc2_hsotg_ep_info_size() function as no more need.
>
> Signed-off-by: Gevorg Sahakyan 
> Signed-off-by: Minas Harutyunyan 
> ---
>  drivers/usb/dwc2/core.h   |  4 
>  drivers/usb/dwc2/gadget.c | 42 ++
>  drivers/usb/dwc2/params.c | 29 +++--
>  3 files changed, 25 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 8367d4f985c1..686e9b5527dd 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -532,6 +532,7 @@ struct dwc2_core_params {
>   *   2 - Internal DMA
>   * @power_optimized Are power optimizations enabled?
>   * @num_dev_ep  Number of device endpoints available
> + * @num_dev_in_eps  Number of device IN endpoints available
>   * @num_dev_perio_in_ep Number of device periodic IN endpoints
>   *  available
>   * @dev_token_q_depth   Device Mode IN Token Sequence Learning Queue
> @@ -560,6 +561,7 @@ struct dwc2_core_params {
>   *   2 - 8 or 16 bits
>   * @snpsid: Value from SNPSID register
>   * @dev_ep_dirs:Direction of device endpoints (GHWCFG1)
> + * @g_tx_fifo_size[] Power-on values of TxFIFO sizes
>   */
>  struct dwc2_hw_params {
>   unsigned op_mode:3;
> @@ -581,12 +583,14 @@ struct dwc2_hw_params {
>   unsigned fs_phy_type:2;
>   unsigned i2c_enable:1;
>   unsigned num_dev_ep:4;
> + unsigned num_dev_in_eps : 4;
>   unsigned num_dev_perio_in_ep:4;
>   unsigned total_fifo_size:16;
>   unsigned power_optimized:1;
>   unsigned utmi_phy_data_width:2;
>   u32 snpsid;
>   u32 dev_ep_dirs;
> + u32 g_tx_fifo_size[MAX_EPS_CHANNELS];
>  };
>
>  /* Size of control and EP0 buffers */
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 0d8e09ccb59c..55950baae437 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -198,55 +198,18 @@ int dwc2_hsotg_tx_fifo_count(struct dwc2_hsotg *hsotg)
>  {
>   if (hsotg->hw_params.en_multiple_tx_fifo)
>   /* In dedicated FIFO mode we need count of IN EPs */
> - return (dwc2_readl(hsotg->regs + GHWCFG4)  &
> - GHWCFG4_NUM_IN_EPS_MASK) >> GHWCFG4_NUM_IN_EPS_SHIFT;
> + return hsotg->hw_params.num_dev_in_eps;
>   else
>   /* In shared FIFO mode we need count of Periodic IN EPs */
>   return hsotg->hw_params.num_dev_perio_in_ep;
>  }
>
> -/**
> - * dwc2_hsotg_ep_info_size - return Endpoint Info Control block size in 
> DWORDs
> - */
> -static int dwc2_hsotg_ep_info_size(struct dwc2_hsotg *hsotg)
> -{
> - int val = 0;
> - int i;
> - u32 ep_dirs;
> -
> - /*
> -  * Don't need additional space for ep info control registers in
> -  * slave mode.
> -  */
> - if (!using_dma(hsotg)) {
> - dev_dbg(hsotg->dev, "Buffer DMA ep info size 0\n");
> - return 0;
> - }
> -
> - /*
> -  * Buffer DMA mode - 1 location per endpoit
> -  * Descriptor DMA mode - 4 locations per endpoint
> -  */
> - ep_dirs = hsotg->hw_params.dev_ep_dirs;
> -
> - for (i = 0; i <= hsotg->hw_params.num_dev_ep; i++) {
> - val += ep_dirs & 3 ? 1 : 2;
> - ep_dirs >>= 2;
> - }
> -
> - if (using_desc_dma(hsotg))
> - val = val * 4;
> -
> - return val;
> -}
> -
>  /**
>   * dwc2_hsotg_tx_fifo_total_depth - return total FIFO depth available for
>   * device mode TX FIFOs
>   */
>  int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg)
>  {
> - int ep_info_size;
>   int addr;
>   int tx_addr_max;
>   u32 np_tx_fifo_size;
> @@ -255,8 +218,7 @@ int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg 
> *hsotg)
>   hsotg->params.g_np_tx_fifo_size);
>
>   /* Get Endpoint Info 

Re: [PATCH] usb: dwc2: Fix TxFIFOn sizes and total TxFIFO size issues

[John Youn]

On 11/30/2017 12:16 AM, Minas Harutyunyan wrote:
> In host mode reading from DPTXSIZn returning invalid value in
> dwc2_check_param_tx_fifo_sizes function.
>
> In total TxFIFO size calculations unnecessarily reducing by ep_info.
> hw->total_fifo_size can be fully allocated for FIFO's.
>
> Added num_dev_in_eps member in dwc2_hw_params structure to save number
> of IN EPs.
>
> Added g_tx_fifo_size array in dwc2_hw_params structure to store power
> on reset values of DPTXSIZn registers in forced device mode.
>
> Updated dwc2_hsotg_tx_fifo_count() function to get TxFIFO count from
> num_dev_in_eps.
>
> Updated dwc2_get_dev_hwparams() function to store DPTXFSIZn in
> g_tx_fifo_size array.
>
> dwc2_get_host/dev_hwparams() functions call moved after num_dev_in_eps
> set from hwcfg4.
>
> Modified dwc2_check_param_tx_fifo_sizes() function to check TxFIFOn
> sizes based on g_tx_fifo_size array.
>
> Removed ep_info subtraction during calculation of tx_addr_max in
> dwc2_hsotg_tx_fifo_total_depth() function. Also removed
> dwc2_hsotg_ep_info_size() function as no more need.
>
> Signed-off-by: Gevorg Sahakyan 
> Signed-off-by: Minas Harutyunyan 
> ---
>  drivers/usb/dwc2/core.h   |  4 
>  drivers/usb/dwc2/gadget.c | 42 ++
>  drivers/usb/dwc2/params.c | 29 +++--
>  3 files changed, 25 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 8367d4f985c1..686e9b5527dd 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -532,6 +532,7 @@ struct dwc2_core_params {
>   *   2 - Internal DMA
>   * @power_optimized Are power optimizations enabled?
>   * @num_dev_ep  Number of device endpoints available
> + * @num_dev_in_eps  Number of device IN endpoints available
>   * @num_dev_perio_in_ep Number of device periodic IN endpoints
>   *  available
>   * @dev_token_q_depth   Device Mode IN Token Sequence Learning Queue
> @@ -560,6 +561,7 @@ struct dwc2_core_params {
>   *   2 - 8 or 16 bits
>   * @snpsid: Value from SNPSID register
>   * @dev_ep_dirs:Direction of device endpoints (GHWCFG1)
> + * @g_tx_fifo_size[] Power-on values of TxFIFO sizes
>   */
>  struct dwc2_hw_params {
>   unsigned op_mode:3;
> @@ -581,12 +583,14 @@ struct dwc2_hw_params {
>   unsigned fs_phy_type:2;
>   unsigned i2c_enable:1;
>   unsigned num_dev_ep:4;
> + unsigned num_dev_in_eps : 4;
>   unsigned num_dev_perio_in_ep:4;
>   unsigned total_fifo_size:16;
>   unsigned power_optimized:1;
>   unsigned utmi_phy_data_width:2;
>   u32 snpsid;
>   u32 dev_ep_dirs;
> + u32 g_tx_fifo_size[MAX_EPS_CHANNELS];
>  };
>
>  /* Size of control and EP0 buffers */
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 0d8e09ccb59c..55950baae437 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -198,55 +198,18 @@ int dwc2_hsotg_tx_fifo_count(struct dwc2_hsotg *hsotg)
>  {
>   if (hsotg->hw_params.en_multiple_tx_fifo)
>   /* In dedicated FIFO mode we need count of IN EPs */
> - return (dwc2_readl(hsotg->regs + GHWCFG4)  &
> - GHWCFG4_NUM_IN_EPS_MASK) >> GHWCFG4_NUM_IN_EPS_SHIFT;
> + return hsotg->hw_params.num_dev_in_eps;
>   else
>   /* In shared FIFO mode we need count of Periodic IN EPs */
>   return hsotg->hw_params.num_dev_perio_in_ep;
>  }
>
> -/**
> - * dwc2_hsotg_ep_info_size - return Endpoint Info Control block size in 
> DWORDs
> - */
> -static int dwc2_hsotg_ep_info_size(struct dwc2_hsotg *hsotg)
> -{
> - int val = 0;
> - int i;
> - u32 ep_dirs;
> -
> - /*
> -  * Don't need additional space for ep info control registers in
> -  * slave mode.
> -  */
> - if (!using_dma(hsotg)) {
> - dev_dbg(hsotg->dev, "Buffer DMA ep info size 0\n");
> - return 0;
> - }
> -
> - /*
> -  * Buffer DMA mode - 1 location per endpoit
> -  * Descriptor DMA mode - 4 locations per endpoint
> -  */
> - ep_dirs = hsotg->hw_params.dev_ep_dirs;
> -
> - for (i = 0; i <= hsotg->hw_params.num_dev_ep; i++) {
> - val += ep_dirs & 3 ? 1 : 2;
> - ep_dirs >>= 2;
> - }
> -
> - if (using_desc_dma(hsotg))
> - val = val * 4;
> -
> - return val;
> -}
> -
>  /**
>   * dwc2_hsotg_tx_fifo_total_depth - return total FIFO depth available for
>   * device mode TX FIFOs
>   */
>  int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg)
>  {
> - int ep_info_size;
>   int addr;
>   int tx_addr_max;
>   u32 np_tx_fifo_size;
> @@ -255,8 +218,7 @@ int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg 
> *hsotg)
>   hsotg->params.g_np_tx_fifo_size);
>
>   /* Get Endpoint Info Control block size in DWORDs. */
> - 

drivers/nvme/target/fcloop.c:1080:9: warning: 'tport' may be used uninitialized in this function

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   328b4ed93b69a6f2083d52f31a240a09e5de386a
commit: fddc9923c6d41de9fe7b1f323a3cece53e046c88 nvme-fcloop: fix port deletes 
and callbacks
date:   2 months ago
config: i386-randconfig-h1-12061225 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
git checkout fddc9923c6d41de9fe7b1f323a3cece53e046c88
# save the attached .config to linux build tree
make ARCH=i386 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/nvme/target/fcloop.c: In function 'fcloop_delete_target_port':
>> drivers/nvme/target/fcloop.c:1080:9: warning: 'tport' may be used 
>> uninitialized in this function [-Wmaybe-uninitialized]
 return nvmet_fc_unregister_targetport(tport->targetport);
^
   drivers/nvme/target/fcloop.c:1088:23: note: 'tport' was declared here
 struct fcloop_tport *tport;
  ^

vim +/tport +1080 drivers/nvme/target/fcloop.c

  1073  
  1074  static int
  1075  __targetport_unreg(struct fcloop_nport *nport, struct fcloop_tport 
*tport)
  1076  {
  1077  if (!tport)
  1078  return -EALREADY;
  1079  
> 1080  return nvmet_fc_unregister_targetport(tport->targetport);
  1081  }
  1082  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

drivers/nvme/target/fcloop.c:1080:9: warning: 'tport' may be used uninitialized in this function

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   328b4ed93b69a6f2083d52f31a240a09e5de386a
commit: fddc9923c6d41de9fe7b1f323a3cece53e046c88 nvme-fcloop: fix port deletes 
and callbacks
date:   2 months ago
config: i386-randconfig-h1-12061225 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
git checkout fddc9923c6d41de9fe7b1f323a3cece53e046c88
# save the attached .config to linux build tree
make ARCH=i386 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/nvme/target/fcloop.c: In function 'fcloop_delete_target_port':
>> drivers/nvme/target/fcloop.c:1080:9: warning: 'tport' may be used 
>> uninitialized in this function [-Wmaybe-uninitialized]
 return nvmet_fc_unregister_targetport(tport->targetport);
^
   drivers/nvme/target/fcloop.c:1088:23: note: 'tport' was declared here
 struct fcloop_tport *tport;
  ^

vim +/tport +1080 drivers/nvme/target/fcloop.c

  1073  
  1074  static int
  1075  __targetport_unreg(struct fcloop_nport *nport, struct fcloop_tport 
*tport)
  1076  {
  1077  if (!tport)
  1078  return -EALREADY;
  1079  
> 1080  return nvmet_fc_unregister_targetport(tport->targetport);
  1081  }
  1082  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v3 1/2] usb: dwc2: host: Don't retry NAKed transactions right away

[John Youn]

On 12/05/2017 08:18 AM, Stefan Wahren wrote:
> Hi Felipe,
> Hi John,
>
>
> Am 30.10.2017 um 18:08 schrieb Douglas Anderson:
>> On rk3288-veyron devices on Chrome OS it was found that plugging in an
>> Arduino-based USB device could cause the system to lockup, especially
>> if the CPU Frequency was at one of the slower operating points (like
>> 100 MHz / 200 MHz).
>>
>> Upon tracing, I found that the following was happening:
>> * The USB device (full speed) was connected to a high speed hub and
>>then to the rk3288.  Thus, we were dealing with split transactions,
>>which is all handled in software on dwc2.
>> * Userspace was initiating a BULK IN transfer
>> * When we sent the SSPLIT (to start the split transaction), we got an
>>ACK.  Good.  Then we issued the CSPLIT.
>> * When we sent the CSPLIT, we got back a NAK.  We immediately (from
>>the interrupt handler) started to retry and sent another SSPLIT.
>> * The device kept NAKing our CSPLIT, so we kept ping-ponging between
>>sending a SSPLIT and a CSPLIT, each time sending from the interrupt
>>handler.
>> * The handling of the interrupts was (because of the low CPU speed and
>>the inefficiency of the dwc2 interrupt handler) was actually taking
>>_longer_ than it took the other side to send the ACK/NAK.  Thus we
>>were _always_ in the USB interrupt routine.
>> * The fact that USB interrupts were always going off was preventing
>>other things from happening in the system.  This included preventing
>>the system from being able to transition to a higher CPU frequency.
>>
>> As I understand it, there is no requirement to retry super quickly
>> after a NAK, we just have to retry sometime in the future.  Thus one
>> solution to the above is to just add a delay between getting a NAK and
>> retrying the transmission.  If this delay is sufficiently long to get
>> out of the interrupt routine then the rest of the system will be able
>> to make forward progress.  Even a 25 us delay would probably be
>> enough, but we'll be extra conservative and try to delay 1 ms (the
>> exact amount depends on HZ and the accuracy of the jiffy and how close
>> the current jiffy is to ticking, but could be as much as 20 ms or as
>> little as 1 ms).
>>
>> Presumably adding a delay like this could impact the USB throughput,
>> so we only add the delay with repeated NAKs.
>>
>> NOTE: Upon further testing of a pl2303 serial adapter, I found that
>> this fix may help with problems there.  Specifically I found that the
>> pl2303 serial adapters tend to respond with a NAK when they have
>> nothing to say and thus we end with this same sequence.
>>
>> Signed-off-by: Douglas Anderson 
>> Cc: sta...@vger.kernel.org
>> Reviewed-by: Julius Werner 
>> Tested-by: Stefan Wahren 
>> ---
>>
>> Changes in v3:
>> - Add tested-by for Stefan Wahren
>> - Sent to Felipe Balbi as candiate to land this.
>> - Add Cc for stable (it's always been broken so go as far is as easy)
>>
>> Changes in v2:
>> - Address 
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__crosreview.com_737520=DwICaQ=DPL6_X_6JkXFx7AXWqB0tg=U3o8uKoKhWme5_V9D-eeCkB11BFwt4KvWztBgdE9ZpA=Y_xpJ6Ks0XAK5_bQgmeQEvgKThZtPBQJ3cejNCGfEvM=olyPwyYvn_072esVwYxrCduKOKKJPUgc1YHX-CNhM1s=
>>  feedback
>>
>
> does it need a resend?
>

You can add my acked-by:

Acked-by: John Youn 

Regards,
John

Re: [PATCH v3 1/2] usb: dwc2: host: Don't retry NAKed transactions right away

[John Youn]

On 12/05/2017 08:18 AM, Stefan Wahren wrote:
> Hi Felipe,
> Hi John,
>
>
> Am 30.10.2017 um 18:08 schrieb Douglas Anderson:
>> On rk3288-veyron devices on Chrome OS it was found that plugging in an
>> Arduino-based USB device could cause the system to lockup, especially
>> if the CPU Frequency was at one of the slower operating points (like
>> 100 MHz / 200 MHz).
>>
>> Upon tracing, I found that the following was happening:
>> * The USB device (full speed) was connected to a high speed hub and
>>then to the rk3288.  Thus, we were dealing with split transactions,
>>which is all handled in software on dwc2.
>> * Userspace was initiating a BULK IN transfer
>> * When we sent the SSPLIT (to start the split transaction), we got an
>>ACK.  Good.  Then we issued the CSPLIT.
>> * When we sent the CSPLIT, we got back a NAK.  We immediately (from
>>the interrupt handler) started to retry and sent another SSPLIT.
>> * The device kept NAKing our CSPLIT, so we kept ping-ponging between
>>sending a SSPLIT and a CSPLIT, each time sending from the interrupt
>>handler.
>> * The handling of the interrupts was (because of the low CPU speed and
>>the inefficiency of the dwc2 interrupt handler) was actually taking
>>_longer_ than it took the other side to send the ACK/NAK.  Thus we
>>were _always_ in the USB interrupt routine.
>> * The fact that USB interrupts were always going off was preventing
>>other things from happening in the system.  This included preventing
>>the system from being able to transition to a higher CPU frequency.
>>
>> As I understand it, there is no requirement to retry super quickly
>> after a NAK, we just have to retry sometime in the future.  Thus one
>> solution to the above is to just add a delay between getting a NAK and
>> retrying the transmission.  If this delay is sufficiently long to get
>> out of the interrupt routine then the rest of the system will be able
>> to make forward progress.  Even a 25 us delay would probably be
>> enough, but we'll be extra conservative and try to delay 1 ms (the
>> exact amount depends on HZ and the accuracy of the jiffy and how close
>> the current jiffy is to ticking, but could be as much as 20 ms or as
>> little as 1 ms).
>>
>> Presumably adding a delay like this could impact the USB throughput,
>> so we only add the delay with repeated NAKs.
>>
>> NOTE: Upon further testing of a pl2303 serial adapter, I found that
>> this fix may help with problems there.  Specifically I found that the
>> pl2303 serial adapters tend to respond with a NAK when they have
>> nothing to say and thus we end with this same sequence.
>>
>> Signed-off-by: Douglas Anderson 
>> Cc: sta...@vger.kernel.org
>> Reviewed-by: Julius Werner 
>> Tested-by: Stefan Wahren 
>> ---
>>
>> Changes in v3:
>> - Add tested-by for Stefan Wahren
>> - Sent to Felipe Balbi as candiate to land this.
>> - Add Cc for stable (it's always been broken so go as far is as easy)
>>
>> Changes in v2:
>> - Address 
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__crosreview.com_737520=DwICaQ=DPL6_X_6JkXFx7AXWqB0tg=U3o8uKoKhWme5_V9D-eeCkB11BFwt4KvWztBgdE9ZpA=Y_xpJ6Ks0XAK5_bQgmeQEvgKThZtPBQJ3cejNCGfEvM=olyPwyYvn_072esVwYxrCduKOKKJPUgc1YHX-CNhM1s=
>>  feedback
>>
>
> does it need a resend?
>

You can add my acked-by:

Acked-by: John Youn 

Regards,
John

[PATCH v2] drm: rcar-du: calculate DPLLCR to be more small jitter

[Kuninori Morimoto]

From: Kuninori Morimoto 

In general, PLL has VCO (= Voltage controlled oscillator),
one of the very important electronic feature called as "jitter"
is related to this VCO.
In academic generalism, VCO should be maximum to be more small jitter.
In high frequency clock, jitter will be large impact.
Thus, selecting Hi VCO is general theory.

   fin fvcofout  fclkout
in --> [1/M] --> |PD| -> [LPF] -> [VCO] -> [1/P] -+-> [1/FDPLL] -> out
 +-> |  | |
 ||
 +-[1/N]<-+

fclkout = fvco / P / FDPLL -- (1)

In PD, it will loop until fin/M = fvco/P/N

fvco = fin * P *  N / M -- (2)

(1) + (2) indicates, fclkout = fin * N / M / FDPLL
In this device, N = (n + 1), M = (m + 1), P = 2, thus

fclkout = fin * (n + 1) / (m + 1) / FDPLL

This is the datasheet formula.
One note here is that it should be 2000 < fvco < 4096MHz
To be smaller jitter, fvco should be maximum,
in other words, N as large as possible, M as small as possible driver
should select. Here, basically M=1.
This patch do it.

Reported-by: HIROSHI INOSE 
Signed-off-by: Kuninori Morimoto 
---
v1 -> v2

 - tidyup typo on git-log "fout" -> "fclkout"
 - tidyup for loop terminate condition 40 -> 38 for n

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 36 --
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index b492063..57479c9 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -125,8 +125,40 @@ static void rcar_du_dpll_divider(struct rcar_du_crtc 
*rcrtc,
unsigned int m;
unsigned int n;
 
-   for (n = 39; n < 120; n++) {
-   for (m = 0; m < 4; m++) {
+   /*
+*   fin fvcofout   fclkout
+* in --> [1/M] --> |PD| -> [LPF] -> [VCO] -> [1/P] -+-> [1/FDPLL] -> 
out
+*  +-> |  | |
+*  ||
+*  +-[1/N]<-+
+*
+*  fclkout = fvco / P / FDPLL -- (1)
+*
+* fin/M = fvco/P/N
+*
+*  fvco = fin * P *  N / M -- (2)
+*
+* (1) + (2) indicates
+*
+*  fclkout = fin * N / M / FDPLL
+*
+* NOTES
+*  N = (n + 1), M = (m + 1), P = 2
+*  2000 < fvco < 4096Mhz
+*  Basically M=1
+*
+* To be small jitter,
+* N : as large as possible
+* M : as small as possible
+*/
+   for (m = 0; m < 4; m++) {
+   for (n = 119; n > 38; n--) {
+   unsigned long long fvco = input * 2 * (n + 1) / (m + 1);
+
+   if ((fvco < 2000) ||
+   (fvco > 409600ll))
+   continue;
+
for (fdpll = 1; fdpll < 32; fdpll++) {
unsigned long output;
 
-- 
1.9.1

[PATCH v2] drm: rcar-du: calculate DPLLCR to be more small jitter

[Kuninori Morimoto]

From: Kuninori Morimoto 

In general, PLL has VCO (= Voltage controlled oscillator),
one of the very important electronic feature called as "jitter"
is related to this VCO.
In academic generalism, VCO should be maximum to be more small jitter.
In high frequency clock, jitter will be large impact.
Thus, selecting Hi VCO is general theory.

   fin fvcofout  fclkout
in --> [1/M] --> |PD| -> [LPF] -> [VCO] -> [1/P] -+-> [1/FDPLL] -> out
 +-> |  | |
 ||
 +-[1/N]<-+

fclkout = fvco / P / FDPLL -- (1)

In PD, it will loop until fin/M = fvco/P/N

fvco = fin * P *  N / M -- (2)

(1) + (2) indicates, fclkout = fin * N / M / FDPLL
In this device, N = (n + 1), M = (m + 1), P = 2, thus

fclkout = fin * (n + 1) / (m + 1) / FDPLL

This is the datasheet formula.
One note here is that it should be 2000 < fvco < 4096MHz
To be smaller jitter, fvco should be maximum,
in other words, N as large as possible, M as small as possible driver
should select. Here, basically M=1.
This patch do it.

Reported-by: HIROSHI INOSE 
Signed-off-by: Kuninori Morimoto 
---
v1 -> v2

 - tidyup typo on git-log "fout" -> "fclkout"
 - tidyup for loop terminate condition 40 -> 38 for n

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 36 --
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index b492063..57479c9 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -125,8 +125,40 @@ static void rcar_du_dpll_divider(struct rcar_du_crtc 
*rcrtc,
unsigned int m;
unsigned int n;
 
-   for (n = 39; n < 120; n++) {
-   for (m = 0; m < 4; m++) {
+   /*
+*   fin fvcofout   fclkout
+* in --> [1/M] --> |PD| -> [LPF] -> [VCO] -> [1/P] -+-> [1/FDPLL] -> 
out
+*  +-> |  | |
+*  ||
+*  +-[1/N]<-+
+*
+*  fclkout = fvco / P / FDPLL -- (1)
+*
+* fin/M = fvco/P/N
+*
+*  fvco = fin * P *  N / M -- (2)
+*
+* (1) + (2) indicates
+*
+*  fclkout = fin * N / M / FDPLL
+*
+* NOTES
+*  N = (n + 1), M = (m + 1), P = 2
+*  2000 < fvco < 4096Mhz
+*  Basically M=1
+*
+* To be small jitter,
+* N : as large as possible
+* M : as small as possible
+*/
+   for (m = 0; m < 4; m++) {
+   for (n = 119; n > 38; n--) {
+   unsigned long long fvco = input * 2 * (n + 1) / (m + 1);
+
+   if ((fvco < 2000) ||
+   (fvco > 409600ll))
+   continue;
+
for (fdpll = 1; fdpll < 32; fdpll++) {
unsigned long output;
 
-- 
1.9.1

Re: [PATCH 5/8] ASoC: uniphier: add support for UniPhier AIO driver

[Katsuhiro Suzuki]

Hello,

> -Original Message-
> From: Mark Brown [mailto:broo...@kernel.org]
> Sent: Tuesday, December 5, 2017 9:15 PM
> To: Suzuki, Katsuhiro/鈴木 勝博 
> Cc: alsa-de...@alsa-project.org; Rob Herring ;
> devicet...@vger.kernel.org; Yamada, Masahiro/山田 真弘
> ; Masami Hiramatsu
> ; Jassi Brar ;
> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 5/8] ASoC: uniphier: add support for UniPhier AIO driver
> 
> On Tue, Dec 05, 2017 at 01:48:39PM +0900, Katsuhiro Suzuki wrote:
> 
> Please fix your mail client to word wrap within paragraphs at something
> substantially less than 80 columns.  Doing this makes your messages much
> easier to read and reply to.
> 

Thank you, I set it.


> > > Is there a mux in the SoC here?
> 
> > Sorry for confusing, It's not mux.
> 
> > uniphier_srcport_reset() resets HW SRC (sampling rate converter) block.
> > Audio data out ports of UniPhier audio system have HW SRC.
> 
> Is the SRC just a single block sitting between the DMA and the external
> audio port or is there more going on?  Some of the other code made me
> think the hardware was more flexible than this (all the writing to
> registers with names like RXSEL for example).
> 

This hardware has 2 types of HW SRC. First type sit before audio port and
cannot change routing. I use it in this driver. Second type is like a loopback,
but this block is not used in this driver to simplify the first version.

Type 1:
  Mem -> DMA -> SRC -> Out Port -> External pin
Type 2:
  Mem -> DMA -> SRC -> Out Port -> In Port -> DMA -> Mem


> > > > +#endif /* CONFIG_SND_SOC_UNIPHIER_LD11 */
> 
> > > Why is there an ifdef here?  There's no other conditional code in here,
> > > it seems pointless.
> 
> > This config is used to support or not LD11 SoC.
> > aio-ld11.c is not build and 'uniphier_aio_ldxx_spec' is undefined if this
config is
> disabled.
> >
> > aio-ld11.c defines SoC dependent resources (port, HW ring buffer, DMA ch,
etc.)
> > and fixed settings.
> > I know it's better to move such information into device-tree, but register
areas of
> > UniPhier's audio system is very strange and interleaved. It's hard to split
each
> nodes...
> 
> I'd expect this code to be structured more like a library - have a
> driver that handles the specific IPs then have it call into a shared
> block of code that does the generic bits.  Though in this case the
> device specific bit looks like a couple of tiny data tables so I'm not
> sure it's worth making it conditional or separate at all.
> 

Sorry... I agree your opinion, but I can't imagine the detail.

I think my driver has structure as follows (ex. startup):
  DAI: uniphier_aio_startup()@aio-core.c
  Lib: uniphier_aio_init()@aio-regctrl.c
  SoC specific: uniphier_aio_ld11_spec@aio-ld11.c

Am I wrong? Would you mean split the functions in aio-regctl.[ch] to other
kernel module? I wonder if you could tell me the example from existing
drivers. I'll try to fix my driver like as it.


> > > This looks awfully like compressed audio support...  should there be
> > > integration with the compressed audio API/
> 
> > Thanks, I'll try it. Is there Documentation in
> sound/designes/compress-offload.rst?
> > And best sample is... Intel's driver?
> 
> Yes.
> 
> > (Summary)
> > I think I should fix as follows:
> 
> >   - Split DMA, DAI patches from large one
> >   - Validate parameters in hw_params
> >   - Add description about HW SRC (or fix bad name 'srcport')
> >   - Add comments about uniphier_aiodma_irq()
> >   - Expose clocking and audio routing to userspace, or at the very
> > least machine driver configuration
> >   - Support compress-audio API for S/PDIF
> 
> > and post V2.
> 
> At least.  I do think we need to get to the bottom of how flexible the
> hardware is first though.

Yes, indeed. This hardware is more flexible and complex, but now I (and our
company) don't use it. Of course, I don't want to hide some features of this
hardware from ALSA people. I should try to upstream all features in the future,
I think.


Regards,
--
Katsuhiro Suzuki

Re: [PATCH 5/8] ASoC: uniphier: add support for UniPhier AIO driver

[Katsuhiro Suzuki]

Hello,

> -Original Message-
> From: Mark Brown [mailto:broo...@kernel.org]
> Sent: Tuesday, December 5, 2017 9:15 PM
> To: Suzuki, Katsuhiro/鈴木 勝博 
> Cc: alsa-de...@alsa-project.org; Rob Herring ;
> devicet...@vger.kernel.org; Yamada, Masahiro/山田 真弘
> ; Masami Hiramatsu
> ; Jassi Brar ;
> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 5/8] ASoC: uniphier: add support for UniPhier AIO driver
> 
> On Tue, Dec 05, 2017 at 01:48:39PM +0900, Katsuhiro Suzuki wrote:
> 
> Please fix your mail client to word wrap within paragraphs at something
> substantially less than 80 columns.  Doing this makes your messages much
> easier to read and reply to.
> 

Thank you, I set it.


> > > Is there a mux in the SoC here?
> 
> > Sorry for confusing, It's not mux.
> 
> > uniphier_srcport_reset() resets HW SRC (sampling rate converter) block.
> > Audio data out ports of UniPhier audio system have HW SRC.
> 
> Is the SRC just a single block sitting between the DMA and the external
> audio port or is there more going on?  Some of the other code made me
> think the hardware was more flexible than this (all the writing to
> registers with names like RXSEL for example).
> 

This hardware has 2 types of HW SRC. First type sit before audio port and
cannot change routing. I use it in this driver. Second type is like a loopback,
but this block is not used in this driver to simplify the first version.

Type 1:
  Mem -> DMA -> SRC -> Out Port -> External pin
Type 2:
  Mem -> DMA -> SRC -> Out Port -> In Port -> DMA -> Mem


> > > > +#endif /* CONFIG_SND_SOC_UNIPHIER_LD11 */
> 
> > > Why is there an ifdef here?  There's no other conditional code in here,
> > > it seems pointless.
> 
> > This config is used to support or not LD11 SoC.
> > aio-ld11.c is not build and 'uniphier_aio_ldxx_spec' is undefined if this
config is
> disabled.
> >
> > aio-ld11.c defines SoC dependent resources (port, HW ring buffer, DMA ch,
etc.)
> > and fixed settings.
> > I know it's better to move such information into device-tree, but register
areas of
> > UniPhier's audio system is very strange and interleaved. It's hard to split
each
> nodes...
> 
> I'd expect this code to be structured more like a library - have a
> driver that handles the specific IPs then have it call into a shared
> block of code that does the generic bits.  Though in this case the
> device specific bit looks like a couple of tiny data tables so I'm not
> sure it's worth making it conditional or separate at all.
> 

Sorry... I agree your opinion, but I can't imagine the detail.

I think my driver has structure as follows (ex. startup):
  DAI: uniphier_aio_startup()@aio-core.c
  Lib: uniphier_aio_init()@aio-regctrl.c
  SoC specific: uniphier_aio_ld11_spec@aio-ld11.c

Am I wrong? Would you mean split the functions in aio-regctl.[ch] to other
kernel module? I wonder if you could tell me the example from existing
drivers. I'll try to fix my driver like as it.


> > > This looks awfully like compressed audio support...  should there be
> > > integration with the compressed audio API/
> 
> > Thanks, I'll try it. Is there Documentation in
> sound/designes/compress-offload.rst?
> > And best sample is... Intel's driver?
> 
> Yes.
> 
> > (Summary)
> > I think I should fix as follows:
> 
> >   - Split DMA, DAI patches from large one
> >   - Validate parameters in hw_params
> >   - Add description about HW SRC (or fix bad name 'srcport')
> >   - Add comments about uniphier_aiodma_irq()
> >   - Expose clocking and audio routing to userspace, or at the very
> > least machine driver configuration
> >   - Support compress-audio API for S/PDIF
> 
> > and post V2.
> 
> At least.  I do think we need to get to the bottom of how flexible the
> hardware is first though.

Yes, indeed. This hardware is more flexible and complex, but now I (and our
company) don't use it. Of course, I don't want to hide some features of this
hardware from ALSA people. I should try to upstream all features in the future,
I think.


Regards,
--
Katsuhiro Suzuki

Re: Sending 802.1Q packets using AF_PACKET socket on filtered bridge forwards with wrong MAC addresses

[Toshiaki Makita]

Hi,
(CC: Vlad)

On 2017/11/30 7:01, Brandon Carpenter wrote:
> I narrowed the search to a memmove() called from
> skb_reorder_vlan_header() in net/core/skbuff.c.
> 
>> memmove(skb->data - ETH_HLEN, skb->data - skb->mac_len - VLAN_HLEN,
>>2 * ETH_ALEN);
> 
> Calling skb_reset_mac_len() after skb_reset_mac_header() before
> calling br_allowed_ingress() in net/bridge/br_device.c fixes the
> problem.
> 
> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> index af5b8c87f590..e10131e2f68f 100644
> --- a/net/bridge/br_device.c
> +++ b/net/bridge/br_device.c
> @@ -58,6 +58,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct
> net_device *dev)
> BR_INPUT_SKB_CB(skb)->brdev = dev;
> 
> skb_reset_mac_header(skb);
> +   skb_reset_mac_len(skb);
> eth = eth_hdr(skb);
> skb_pull(skb, ETH_HLEN);

Thanks for debugging this problem.
It seems this has been broken since a6e18ff11170 ("vlan: Fix untag
operations of stacked vlans with REORDER_HEADER off").

Unfortunately this does not always work correctly, since in tx path
drivers assume network header to be set to L3 protocol header offset.
Packet socket (packet_snd()) determines network header by
dev_hard_header which is ETH_HLEN in bridge devices, so this works for
packet socket, but with vlan devices on top of bridge device with
tx-vlan hwaccel disabled we get ETH_HLEN + VLAN_HLEN or longer by mac_len.

Since mac_len can be arbitrarily long if we stack vlan devices on bridge
devices, and since we want to untag the outermost tag, using mac_len to
untag in tx path is probably no longer correct.

I'll think deeper about how to fix it.

> I'll put together an official patch  and submit it. Should I use
> another email account? Are my emails being ignored because of that
> stupid disclaimer my employer attaches to my messages (outside my
> control)?
> 
> Brandon
> 

-- 
Toshiaki Makita

Re: Sending 802.1Q packets using AF_PACKET socket on filtered bridge forwards with wrong MAC addresses

[Toshiaki Makita]

Hi,
(CC: Vlad)

On 2017/11/30 7:01, Brandon Carpenter wrote:
> I narrowed the search to a memmove() called from
> skb_reorder_vlan_header() in net/core/skbuff.c.
> 
>> memmove(skb->data - ETH_HLEN, skb->data - skb->mac_len - VLAN_HLEN,
>>2 * ETH_ALEN);
> 
> Calling skb_reset_mac_len() after skb_reset_mac_header() before
> calling br_allowed_ingress() in net/bridge/br_device.c fixes the
> problem.
> 
> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> index af5b8c87f590..e10131e2f68f 100644
> --- a/net/bridge/br_device.c
> +++ b/net/bridge/br_device.c
> @@ -58,6 +58,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct
> net_device *dev)
> BR_INPUT_SKB_CB(skb)->brdev = dev;
> 
> skb_reset_mac_header(skb);
> +   skb_reset_mac_len(skb);
> eth = eth_hdr(skb);
> skb_pull(skb, ETH_HLEN);

Thanks for debugging this problem.
It seems this has been broken since a6e18ff11170 ("vlan: Fix untag
operations of stacked vlans with REORDER_HEADER off").

Unfortunately this does not always work correctly, since in tx path
drivers assume network header to be set to L3 protocol header offset.
Packet socket (packet_snd()) determines network header by
dev_hard_header which is ETH_HLEN in bridge devices, so this works for
packet socket, but with vlan devices on top of bridge device with
tx-vlan hwaccel disabled we get ETH_HLEN + VLAN_HLEN or longer by mac_len.

Since mac_len can be arbitrarily long if we stack vlan devices on bridge
devices, and since we want to untag the outermost tag, using mac_len to
untag in tx path is probably no longer correct.

I'll think deeper about how to fix it.

> I'll put together an official patch  and submit it. Should I use
> another email account? Are my emails being ignored because of that
> stupid disclaimer my employer attaches to my messages (outside my
> control)?
> 
> Brandon
> 

-- 
Toshiaki Makita

Re: [PATCH] drm: rcar-du: calculate DPLLCR to be more small jitter

[Kuninori Morimoto]

Hi

I noticed 1 typo, 1 bug on this patch.
I will post v2 patch

> From: Kuninori Morimoto 
> 
> In general, PLL has VCO (= Voltage controlled oscillator),
> one of the very important electronic feature called as "jitter"
> is related to this VCO.
> In academic generalism, VCO should be maximum to be more small jitter.
> In high frequency clock, jitter will be large impact.
> Thus, selecting Hi VCO is general theory.
> 
>fin fvcofout  fclkout
> in --> [1/M] --> |PD| -> [LPF] -> [VCO] -> [1/P] -+-> [1/FDPLL] -> out
>  +-> |  | |
>  ||
>  +-[1/N]<-+
> 
>   fclkout = fvco / P / FDPLL -- (1)
> 
> In PD, it will loop until fin/M = fvco/P/N
> 
>   fvco = fin * P *  N / M -- (2)
> 
> (1) + (2) indicates, fclkout = fin * N / M / FDPLL
> In this device, N = (n + 1), M = (m + 1), P = 2, thus
> 
>   fout = fin * (n + 1) / (m + 1) / FDPLL
> 
> This is the datasheet formula.
> One note here is that it should be 2000 < fvco < 4096MHz
> To be smaller jitter, fvco should be maximum,
> in other words, N as large as possible, M as small as possible driver
> should select. Here, basically M=1.
> This patch do it.
> 
> Reported-by: HIROSHI INOSE 
> Signed-off-by: Kuninori Morimoto 
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 36 
> --
>  1 file changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index b492063..45540fe 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -125,8 +125,40 @@ static void rcar_du_dpll_divider(struct rcar_du_crtc 
> *rcrtc,
>   unsigned int m;
>   unsigned int n;
>  
> - for (n = 39; n < 120; n++) {
> - for (m = 0; m < 4; m++) {
> + /*
> +  *   fin fvcofout   fclkout
> +  * in --> [1/M] --> |PD| -> [LPF] -> [VCO] -> [1/P] -+-> [1/FDPLL] -> 
> out
> +  *  +-> |  | |
> +  *  ||
> +  *  +-[1/N]<-+
> +  *
> +  *  fclkout = fvco / P / FDPLL -- (1)
> +  *
> +  * fin/M = fvco/P/N
> +  *
> +  *  fvco = fin * P *  N / M -- (2)
> +  *
> +  * (1) + (2) indicates
> +  *
> +  *  fclkout = fin * N / M / FDPLL
> +  *
> +  * NOTES
> +  *  N = (n + 1), M = (m + 1), P = 2
> +  *  2000 < fvco < 4096Mhz
> +  *  Basically M=1
> +  *
> +  * To be small jitter,
> +  * N : as large as possible
> +  * M : as small as possible
> +  */
> + for (m = 0; m < 4; m++) {
> + for (n = 119; n > 40; n--) {
> + unsigned long long fvco = input * 2 * (n + 1) / (m + 1);
> +
> + if ((fvco < 2000) ||
> + (fvco > 409600ll))
> + continue;
> +
>   for (fdpll = 1; fdpll < 32; fdpll++) {
>   unsigned long output;
>  
> -- 
> 1.9.1
> 

Re: [PATCH] drm: rcar-du: calculate DPLLCR to be more small jitter

[Kuninori Morimoto]

Hi

I noticed 1 typo, 1 bug on this patch.
I will post v2 patch

> From: Kuninori Morimoto 
> 
> In general, PLL has VCO (= Voltage controlled oscillator),
> one of the very important electronic feature called as "jitter"
> is related to this VCO.
> In academic generalism, VCO should be maximum to be more small jitter.
> In high frequency clock, jitter will be large impact.
> Thus, selecting Hi VCO is general theory.
> 
>fin fvcofout  fclkout
> in --> [1/M] --> |PD| -> [LPF] -> [VCO] -> [1/P] -+-> [1/FDPLL] -> out
>  +-> |  | |
>  ||
>  +-[1/N]<-+
> 
>   fclkout = fvco / P / FDPLL -- (1)
> 
> In PD, it will loop until fin/M = fvco/P/N
> 
>   fvco = fin * P *  N / M -- (2)
> 
> (1) + (2) indicates, fclkout = fin * N / M / FDPLL
> In this device, N = (n + 1), M = (m + 1), P = 2, thus
> 
>   fout = fin * (n + 1) / (m + 1) / FDPLL
> 
> This is the datasheet formula.
> One note here is that it should be 2000 < fvco < 4096MHz
> To be smaller jitter, fvco should be maximum,
> in other words, N as large as possible, M as small as possible driver
> should select. Here, basically M=1.
> This patch do it.
> 
> Reported-by: HIROSHI INOSE 
> Signed-off-by: Kuninori Morimoto 
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 36 
> --
>  1 file changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index b492063..45540fe 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -125,8 +125,40 @@ static void rcar_du_dpll_divider(struct rcar_du_crtc 
> *rcrtc,
>   unsigned int m;
>   unsigned int n;
>  
> - for (n = 39; n < 120; n++) {
> - for (m = 0; m < 4; m++) {
> + /*
> +  *   fin fvcofout   fclkout
> +  * in --> [1/M] --> |PD| -> [LPF] -> [VCO] -> [1/P] -+-> [1/FDPLL] -> 
> out
> +  *  +-> |  | |
> +  *  ||
> +  *  +-[1/N]<-+
> +  *
> +  *  fclkout = fvco / P / FDPLL -- (1)
> +  *
> +  * fin/M = fvco/P/N
> +  *
> +  *  fvco = fin * P *  N / M -- (2)
> +  *
> +  * (1) + (2) indicates
> +  *
> +  *  fclkout = fin * N / M / FDPLL
> +  *
> +  * NOTES
> +  *  N = (n + 1), M = (m + 1), P = 2
> +  *  2000 < fvco < 4096Mhz
> +  *  Basically M=1
> +  *
> +  * To be small jitter,
> +  * N : as large as possible
> +  * M : as small as possible
> +  */
> + for (m = 0; m < 4; m++) {
> + for (n = 119; n > 40; n--) {
> + unsigned long long fvco = input * 2 * (n + 1) / (m + 1);
> +
> + if ((fvco < 2000) ||
> + (fvco > 409600ll))
> + continue;
> +
>   for (fdpll = 1; fdpll < 32; fdpll++) {
>   unsigned long output;
>  
> -- 
> 1.9.1
> 

RE: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Dhaval Rajeshbhai Shah]

Hi Greg k-h,

Thanks a lot for the review.

Replies inline.

-Original Message-
From: Greg KH [mailto:gre...@linuxfoundation.org] 
Sent: Tuesday, December 05, 2017 5:30 AM
To: Dhaval Rajeshbhai Shah 
Cc: a...@arndb.de; linux-kernel@vger.kernel.org; michal.si...@xilinx.com; Hyun 
Kwon ; Dhaval Rajeshbhai Shah 
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP 
init driver

On Tue, Dec 05, 2017 at 03:43:32AM -0800, Dhaval Shah wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design 
> created. This driver will provide the api which can be used by the 
> encoder and decoder driver to get the configured value.

Your subject has a lot of [] in it, none of that is needed except the [PATCH] 
one :)
[Dhaval ] : I will take care from next version. 

> 
> Signed-off-by: Dhaval Shah 
> ---
>  drivers/misc/Kconfig|   6 +
>  drivers/misc/Makefile   |   1 +
>  drivers/misc/xlnx_vcu.c | 664 
> 
>  include/misc/xlnx_vcu.h |  18 ++
>  4 files changed, 689 insertions(+)
>  create mode 100644 drivers/misc/xlnx_vcu.c  create mode 100644 
> include/misc/xlnx_vcu.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 
> f1a5c23..3b7c796 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -496,6 +496,12 @@ config PCI_ENDPOINT_TEST
> Enable this configuration option to enable the host side test 
> driver
> for PCI Endpoint.
>  
> +config XILINX_VCU
> +   tristate "Xilinx VCU Init"
> +   default n

That's always the default, no need for this.
[Dhaval ] : I will remove that. 

> +   help
> +  Driver for the Xilinx VCU Init based on the logicoreIP.

You need a lot more help text here to explain what this driver is, what it is 
for, and who would need it.
[Dhaval ] : I will provide more help text to provide more help on driver. 

Also, why is this a misc driver?
[Dhaval ] :  this driver is for the logicoreIP which is created to support the 
Processing system and Programmable logic isolation and to provide the clock 
related information. So this is not a VCU driver and but just a intermediate 
driver which supports logicoreIP. That's why no subsystem for this.

> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,664 @@
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts   Dhaval Shah 
> + *
> + * SPDX-License-Identifier: GPL-2.0

That line goes at the top of the file with // in front of it.
[Dhaval ] : I will update that SPDX license as you said. 


> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 

Why do you need a .h file for a single driver?
[Dhaval ] : There are few APIs and structure which are provided in the .h file 
which will be used by the other driver on the Xilinx based system. I have 
exported few API as well because of this reason. Those API will share the 
information from the logicoreIP register set.

> +/**
> + * xvcu_get_color_depth - read the color depth register
> + * @xvcu:Pointer to the xvcu_device structure
> + *
> + * Return:   Returns 32bit value
> + *
> + */
> +u32 xvcu_get_color_depth(struct xvcu_device *xvcu) {
> + return xvcu_read(xvcu->logicore_reg_ba, VCU_ENC_COLOR_DEPTH); } 
> +EXPORT_SYMBOL_GPL(xvcu_get_color_depth);

Why is your driver exporting symbols that no one uses?

This feels very odd...
[Dhaval ] :  There are few information from the logicoreIp register set which 
needs to share with other driver. That's why those API are exported to use by 
other driver to get those information based on the requirement.

greg k-h

Regards,
Dhaval

RE: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Dhaval Rajeshbhai Shah]

Hi Greg k-h,

Thanks a lot for the review.

Replies inline.

-Original Message-
From: Greg KH [mailto:gre...@linuxfoundation.org] 
Sent: Tuesday, December 05, 2017 5:30 AM
To: Dhaval Rajeshbhai Shah 
Cc: a...@arndb.de; linux-kernel@vger.kernel.org; michal.si...@xilinx.com; Hyun 
Kwon ; Dhaval Rajeshbhai Shah 
Subject: Re: [PATCH] [linux][master][v1] misc: Add Xilinx ZYNQMP VCU logicoreIP 
init driver

On Tue, Dec 05, 2017 at 03:43:32AM -0800, Dhaval Shah wrote:
> Xilinx ZYNQMP VCU Init driver is based on the new LogiCoreIP design 
> created. This driver will provide the api which can be used by the 
> encoder and decoder driver to get the configured value.

Your subject has a lot of [] in it, none of that is needed except the [PATCH] 
one :)
[Dhaval ] : I will take care from next version. 

> 
> Signed-off-by: Dhaval Shah 
> ---
>  drivers/misc/Kconfig|   6 +
>  drivers/misc/Makefile   |   1 +
>  drivers/misc/xlnx_vcu.c | 664 
> 
>  include/misc/xlnx_vcu.h |  18 ++
>  4 files changed, 689 insertions(+)
>  create mode 100644 drivers/misc/xlnx_vcu.c  create mode 100644 
> include/misc/xlnx_vcu.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 
> f1a5c23..3b7c796 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -496,6 +496,12 @@ config PCI_ENDPOINT_TEST
> Enable this configuration option to enable the host side test 
> driver
> for PCI Endpoint.
>  
> +config XILINX_VCU
> +   tristate "Xilinx VCU Init"
> +   default n

That's always the default, no need for this.
[Dhaval ] : I will remove that. 

> +   help
> +  Driver for the Xilinx VCU Init based on the logicoreIP.

You need a lot more help text here to explain what this driver is, what it is 
for, and who would need it.
[Dhaval ] : I will provide more help text to provide more help on driver. 

Also, why is this a misc driver?
[Dhaval ] :  this driver is for the logicoreIP which is created to support the 
Processing system and Programmable logic isolation and to provide the clock 
related information. So this is not a VCU driver and but just a intermediate 
driver which supports logicoreIP. That's why no subsystem for this.

> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,664 @@
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts   Dhaval Shah 
> + *
> + * SPDX-License-Identifier: GPL-2.0

That line goes at the top of the file with // in front of it.
[Dhaval ] : I will update that SPDX license as you said. 


> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 

Why do you need a .h file for a single driver?
[Dhaval ] : There are few APIs and structure which are provided in the .h file 
which will be used by the other driver on the Xilinx based system. I have 
exported few API as well because of this reason. Those API will share the 
information from the logicoreIP register set.

> +/**
> + * xvcu_get_color_depth - read the color depth register
> + * @xvcu:Pointer to the xvcu_device structure
> + *
> + * Return:   Returns 32bit value
> + *
> + */
> +u32 xvcu_get_color_depth(struct xvcu_device *xvcu) {
> + return xvcu_read(xvcu->logicore_reg_ba, VCU_ENC_COLOR_DEPTH); } 
> +EXPORT_SYMBOL_GPL(xvcu_get_color_depth);

Why is your driver exporting symbols that no one uses?

This feels very odd...
[Dhaval ] :  There are few information from the logicoreIp register set which 
needs to share with other driver. That's why those API are exported to use by 
other driver to get those information based on the requirement.

greg k-h

Regards,
Dhaval

[PATCH PTI] Fixup "x86/entry/64: Use a per-CPU trampoline stack for IDT entries"

[Andy Lutomirski]

Add to commit message:

Using a trampoline stack would obnoxious for Xen PV because Xen PV
enters entry_SYSCALL_64_after_hwframe on the stack indicated by sp0.
This could be fixed, but I think it's nice to ensure the entry code
can still work without a trampoline stack.  So this patch doesn't
use the entry trampoline stack on Xen.

The regs != eregs check in sync_regs is optional, but I think it's
better than copying the whole set of regs over itself.

Signed-off-by: Andy Lutomirski 
---

Boris, this is lightly tested and should fix the problem you're seeing.

 arch/x86/include/asm/switch_to.h | 3 +++
 arch/x86/kernel/traps.c  | 9 +
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/switch_to.h b/arch/x86/include/asm/switch_to.h
index fab453ad2460..cbc71e73bd32 100644
--- a/arch/x86/include/asm/switch_to.h
+++ b/arch/x86/include/asm/switch_to.h
@@ -93,6 +93,9 @@ static inline void update_sp0(struct task_struct *task)
/* On x86_64, sp0 always points to the entry trampoline stack, which is 
constant: */
 #ifdef CONFIG_X86_32
load_sp0(task->thread.sp0);
+#else
+   if (static_cpu_has(X86_FEATURE_XENPV))
+   load_sp0(task_top_of_stack(task));
 #endif
 }
 
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 40cc3dc5967a..ee9ca0ad4388 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -619,14 +619,15 @@ NOKPROBE_SYMBOL(do_int3);
 
 #ifdef CONFIG_X86_64
 /*
- * Help handler running on IST stack to switch off the IST stack if the
- * interrupted code was in user mode. The actual stack switch is done in
- * entry_64.S
+ * Help handler running on a per-cpu (IST or entry trampoline) stack
+ * to switch to the normal thread stack if the interrupted code was in
+ * user mode. The actual stack switch is done in entry_64.S
  */
 asmlinkage __visible notrace struct pt_regs *sync_regs(struct pt_regs *eregs)
 {
struct pt_regs *regs = (struct pt_regs 
*)this_cpu_read(cpu_current_top_of_stack) - 1;
-   *regs = *eregs;
+   if (regs != eregs)
+   *regs = *eregs;
return regs;
 }
 NOKPROBE_SYMBOL(sync_regs);
-- 
2.13.6

[PATCH PTI] Fixup "x86/entry/64: Use a per-CPU trampoline stack for IDT entries"

[Andy Lutomirski]

Add to commit message:

Using a trampoline stack would obnoxious for Xen PV because Xen PV
enters entry_SYSCALL_64_after_hwframe on the stack indicated by sp0.
This could be fixed, but I think it's nice to ensure the entry code
can still work without a trampoline stack.  So this patch doesn't
use the entry trampoline stack on Xen.

The regs != eregs check in sync_regs is optional, but I think it's
better than copying the whole set of regs over itself.

Signed-off-by: Andy Lutomirski 
---

Boris, this is lightly tested and should fix the problem you're seeing.

 arch/x86/include/asm/switch_to.h | 3 +++
 arch/x86/kernel/traps.c  | 9 +
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/switch_to.h b/arch/x86/include/asm/switch_to.h
index fab453ad2460..cbc71e73bd32 100644
--- a/arch/x86/include/asm/switch_to.h
+++ b/arch/x86/include/asm/switch_to.h
@@ -93,6 +93,9 @@ static inline void update_sp0(struct task_struct *task)
/* On x86_64, sp0 always points to the entry trampoline stack, which is 
constant: */
 #ifdef CONFIG_X86_32
load_sp0(task->thread.sp0);
+#else
+   if (static_cpu_has(X86_FEATURE_XENPV))
+   load_sp0(task_top_of_stack(task));
 #endif
 }
 
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 40cc3dc5967a..ee9ca0ad4388 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -619,14 +619,15 @@ NOKPROBE_SYMBOL(do_int3);
 
 #ifdef CONFIG_X86_64
 /*
- * Help handler running on IST stack to switch off the IST stack if the
- * interrupted code was in user mode. The actual stack switch is done in
- * entry_64.S
+ * Help handler running on a per-cpu (IST or entry trampoline) stack
+ * to switch to the normal thread stack if the interrupted code was in
+ * user mode. The actual stack switch is done in entry_64.S
  */
 asmlinkage __visible notrace struct pt_regs *sync_regs(struct pt_regs *eregs)
 {
struct pt_regs *regs = (struct pt_regs 
*)this_cpu_read(cpu_current_top_of_stack) - 1;
-   *regs = *eregs;
+   if (regs != eregs)
+   *regs = *eregs;
return regs;
 }
 NOKPROBE_SYMBOL(sync_regs);
-- 
2.13.6