RE: [PATCH v2] rtc/ds3232: fix ds3232 get a WARNING trace in resume function

[Wang Dongsheng]

Thanks, I will push v3 to fix them.

Regards,
-Dongsheng

 -Original Message-
 From: Alexandre Belloni [mailto:alexandre.bell...@free-electrons.com]
 Sent: Friday, August 21, 2015 7:22 AM
 To: Wang Dongsheng-B40534; Krzysztof Kozlowski
 Cc: a.zu...@towertech.it; rtc-li...@googlegroups.com; linux-
 ker...@vger.kernel.org
 Subject: Re: [PATCH v2] rtc/ds3232: fix ds3232 get a WARNING trace in resume
 function
 
 On 12/08/2015 at 17:14:13 +0800, Dongsheng Wang wrote :
  From: Wang Dongsheng dongsheng.w...@freescale.com
 
  If ds3232 work on some platform that is not implementation
  irq_set_wake, ds3232 will get a WARNING trace in resume.
  So fix ds3232-suspended state to false when irq_set_irq_wake return
  error.
 
  [ cut here ]
  WARNING: CPU: 0 PID: 729 at kernel/irq/manage.c:604
  irq_set_irq_wake+0x4b/0x8c()
  Unbalanced IRQ 201 wake disable
  Modules linked in:
  CPU: 0 PID: 729 Comm: sh Not tainted 3.12.19-rt30+ #25 [800107d9]
  (unwind_backtrace+0x1/0x88) from [8000e4ef]
  (show_stack+0xb/0xc)
  [8000e4ef] (show_stack+0xb/0xc) from [802b5fa9]
  (dump_stack+0x4d/0x60)
  [802b5fa9] (dump_stack+0x4d/0x60) from [800186dd]
  (warn_slowpath_common+0x45/0x64)
  [800186dd] (warn_slowpath_common+0x45/0x64) from [80018717]
  (warn_slowpath_fmt+0x1b/0x24)
  [80018717] (warn_slowpath_fmt+0x1b/0x24) from [8003a8d3]
  (irq_set_irq_wake+0x4b/0x8c)
  [8003a8d3] (irq_set_irq_wake+0x4b/0x8c) from [80204fcb]
  (ds3232_resume+0x2d/0x36)
  [80204fcb] (ds3232_resume+0x2d/0x36) from [801954c7]
  (dpm_run_callback.isra.13+0xb/0x28)
  [801954c7] (dpm_run_callback.isra.13+0xb/0x28) from [80195b1b]
  (device_resume+0x7b/0xa2)
  [80195b1b] (device_resume+0x7b/0xa2) from [80195f0f]
  (dpm_resume+0xbb/0x19c)
  [80195f0f] (dpm_resume+0xbb/0x19c) from [801960d9]
  (dpm_resume_end+0x9/0x12)
  [801960d9] (dpm_resume_end+0x9/0x12) from [80037e1d]
  (suspend_devices_and_enter+0x17d/0x1d0)
  [80037e1d] (suspend_devices_and_enter+0x17d/0x1d0) from [80037ee1]
  (pm_suspend+0x71/0x128)
  [80037ee1] (pm_suspend+0x71/0x128) from [80037449]
  (state_store+0x6d/0x80)
  [80037449] (state_store+0x6d/0x80) from [800af4d5]
  (sysfs_write_file+0x9f/0xde)
  [800af4d5] (sysfs_write_file+0x9f/0xde) from [8007a437]
  (vfs_write+0x7b/0x104)
  [8007a437] (vfs_write+0x7b/0x104) from [8007a7f7]
  (SyS_write+0x27/0x48)
  [8007a7f7] (SyS_write+0x27/0x48) from [8000c121]
  (ret_fast_syscall+0x1/0x44)
  ---[ end trace 640959d2e8de6ccc ]---
 
  Signed-off-by: Wang Dongsheng dongsheng.w...@freescale.com
  ---
  *v2*
  - Use dev_warn_once to instead of dev_info
 
 
 Applied, after fixing the kernel trace and the message like suggested by
 Krzysztof.
 
 --
 Alexandre Belloni, Free Electrons
 Embedded Linux, Kernel and Android engineering http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform

[Wang Dongsheng]

Hi Walleij and Russell,

I will drop this patch. Thanks for your review.

[PATCH v2 1/2] soc/fsl: add freescale dir for SOC specific drivers

But the 1/2 of the patches also need, because there has another patch(Freescale 
FPGA driver) need 1/2 patch.
Need I push the 1/2 patch with another patches(Freescale FPGA driver) or push 
1/2 standalone without another
patch? 

Regards,
-Dongsheng

 -Original Message-
 From: Linus Walleij [mailto:linus.wall...@linaro.org]
 Sent: Friday, August 14, 2015 6:07 PM
 To: Wang Dongsheng-B40534; John Stultz
 Cc: Alessandro Zummo; Alexandre Belloni; Shawn Guo; Nair, Sandeep; Hans de 
 Goede;
 Wang Huan-B18965; linux-arm-ker...@lists.infradead.org; linux-
 ker...@vger.kernel.org; rtc-li...@googlegroups.com
 Subject: Re: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform
 
 On Fri, Aug 14, 2015 at 5:12 AM, Wang Dongsheng
 dongsheng.w...@freescale.com wrote:
  On Wed, Aug 12, 2015 at 7:53 AM, Dongsheng Wang
  dongsheng.w...@freescale.com wrote:
 
   From: Wang Dongsheng dongsheng.w...@freescale.com
  
   Only Ftm0 can be used when system going to deep sleep. So this driver
   to support ftm0 as a wakeup source.
  
   Signed-off-by: Wang Dongsheng dongsheng.w...@freescale.com
   ---
   *V2*
   Change Copyright 2014 to 2015.
  (...)
   +config FTM_ALARM
   +   bool FTM alarm driver
   +   depends on SOC_LS1021A
   +   default n
   +   help
   + Say y here to enable FTM alarm support.  The FTM alarm provides
   + alarm functions for wakeup system from deep sleep.  There is 
   only
   + one FTM can be used in ALARM(FTM 0).
  (...)
   +static u32 time_to_cycle(unsigned long time)
   +static u32 cycle_to_time(u32 cycle)
   +static int ftm_set_alarm(u64 cycle)
   +static irqreturn_t ftm_alarm_interrupt(int irq, void *dev_id)
   +static ssize_t ftm_alarm_show(struct device *dev,
   + struct device_attribute *attr,
   + char *buf)
   +static ssize_t ftm_alarm_store(struct device *dev,
   +  struct device_attribute *attr,
   +  const char *buf, size_t count)
  (...)
   +static struct device_attribute ftm_alarm_attributes = __ATTR(ftm_alarm,
 0644,
   +   ftm_alarm_show, ftm_alarm_store);
 
  If you're gonna invent ABIs, document then in Documentation/ABI/testing/*.
 
  But I don't get it. Why is this driver not in drivers/rtc?
 
  It does a subset of what an RTC does. The ioctl()'s of an RTC
  can do what you want to do. And much much more.
 
  If it can't do all an RTC can do, surely the RTC subsystem
  can be augmented to host it anyway. It's way to close to
  an RTC to have it's own random sysfs driver like this.
 
  Unless I'm totally off, rewrite this to an RTC driver and post
  it to the RTC maintainers.
 
  FlexTimer is not a RTC device and not have any rtc deivce function. They
 belong to
  different devices, why we need to register this to RTC framework? I am
 confused about this.
 
  Now in freescale layerscape platform this driver is only for FlexTimer0, and
 not
  fit for each flextimer. Because only FlexTimer0 still turn-on when system in
 the Deep Sleep.
 
  If the alarm make you feel confused or mislead you think this is a RTC
 devices. I think
  I need to change the alarm to timer.
 
 I think it is an RTC, it is just that the hardware engineer
 designed it with a wakeup usecase in mind and did not call
 it an RTC. Wakeup is one of the things RTCs do.
 
 If you inspect a few drivers in drivers/rtc such as drivers/rtc/rtc-pl030.c
 you will find that they are just as crude as this alarm thing.
 
 It has a counter that counts cycles, it has a comparator
 and an alarm function. It is an on-chip RTC, just like PL030
 no matter what the datasheet or hardware engineer thinks it
 should be called, the Linux kernel calls this an RTC, and it
 has a subsystem for handling it, so we should use it and
 not invent random new stuff.
 
 If the hardware is really so strange that the counter can only
 be started if you also put an alarm at the same time (I doubt
 it, but OK if you say so) it is a subset of an RTC that can
 only be used for alarms but not timekeeping, but it should
 *still* live in drivers/rtc.
 
 Think for a moment on the huge effort that John Stultz put into
 integrating Android alarm timers with POSIX and the RTC
 subsystem and fixing it all from the smallest handset to
 the largest S360 supercomputer. The approach of a custom
 device just throws all of that out the window and reinvents the
 mechanism in userspace, forcing all standardized userspace to
 have special code to handle this special alarm with its
 special sysfs ABI.
 
 Check
 commit ff3ead96d17f47ee70c294a5cc2cce9b61e82f0f
 timers: Introduce in-kernel alarm-timer interface
 for example.
 
 Even if you persist on keeping it in its own magic driver
 like this, it should implement the alarm timer interface
 from linux

RE: [PATCH v2] rtc/ds3232: fix ds3232 get a WARNING trace in resume function

[Wang Dongsheng]

Hi Alexandre,

Could you apply this patch?

Regards,
-Dongsheng

 -Original Message-
 From: Dongsheng Wang [mailto:dongsheng.w...@freescale.com]
 Sent: Wednesday, August 12, 2015 5:14 PM
 To: alexandre.bell...@free-electrons.com
 Cc: a.zu...@towertech.it; rtc-li...@googlegroups.com; linux-
 ker...@vger.kernel.org; Wang Dongsheng-B40534
 Subject: [PATCH v2] rtc/ds3232: fix ds3232 get a WARNING trace in resume
 function
 
 From: Wang Dongsheng dongsheng.w...@freescale.com
 
 If ds3232 work on some platform that is not implementation
 irq_set_wake, ds3232 will get a WARNING trace in resume.
 So fix ds3232-suspended state to false when irq_set_irq_wake
 return error.
 
 [ cut here ]
 WARNING: CPU: 0 PID: 729 at kernel/irq/manage.c:604
 irq_set_irq_wake+0x4b/0x8c()
 Unbalanced IRQ 201 wake disable
 Modules linked in:
 CPU: 0 PID: 729 Comm: sh Not tainted 3.12.19-rt30+ #25
 [800107d9] (unwind_backtrace+0x1/0x88) from [8000e4ef]
 (show_stack+0xb/0xc)
 [8000e4ef] (show_stack+0xb/0xc) from [802b5fa9]
 (dump_stack+0x4d/0x60)
 [802b5fa9] (dump_stack+0x4d/0x60) from [800186dd]
 (warn_slowpath_common+0x45/0x64)
 [800186dd] (warn_slowpath_common+0x45/0x64) from [80018717]
 (warn_slowpath_fmt+0x1b/0x24)
 [80018717] (warn_slowpath_fmt+0x1b/0x24) from [8003a8d3]
 (irq_set_irq_wake+0x4b/0x8c)
 [8003a8d3] (irq_set_irq_wake+0x4b/0x8c) from [80204fcb]
 (ds3232_resume+0x2d/0x36)
 [80204fcb] (ds3232_resume+0x2d/0x36) from [801954c7]
 (dpm_run_callback.isra.13+0xb/0x28)
 [801954c7] (dpm_run_callback.isra.13+0xb/0x28) from [80195b1b]
 (device_resume+0x7b/0xa2)
 [80195b1b] (device_resume+0x7b/0xa2) from [80195f0f]
 (dpm_resume+0xbb/0x19c)
 [80195f0f] (dpm_resume+0xbb/0x19c) from [801960d9]
 (dpm_resume_end+0x9/0x12)
 [801960d9] (dpm_resume_end+0x9/0x12) from [80037e1d]
 (suspend_devices_and_enter+0x17d/0x1d0)
 [80037e1d] (suspend_devices_and_enter+0x17d/0x1d0) from [80037ee1]
 (pm_suspend+0x71/0x128)
 [80037ee1] (pm_suspend+0x71/0x128) from [80037449]
 (state_store+0x6d/0x80)
 [80037449] (state_store+0x6d/0x80) from [800af4d5]
 (sysfs_write_file+0x9f/0xde)
 [800af4d5] (sysfs_write_file+0x9f/0xde) from [8007a437]
 (vfs_write+0x7b/0x104)
 [8007a437] (vfs_write+0x7b/0x104) from [8007a7f7]
 (SyS_write+0x27/0x48)
 [8007a7f7] (SyS_write+0x27/0x48) from [8000c121]
 (ret_fast_syscall+0x1/0x44)
 ---[ end trace 640959d2e8de6ccc ]---
 
 Signed-off-by: Wang Dongsheng dongsheng.w...@freescale.com
 ---
 *v2*
 - Use dev_warn_once to instead of dev_info
 
 diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
 index 7e48e53..3554970 100644
 --- a/drivers/rtc/rtc-ds3232.c
 +++ b/drivers/rtc/rtc-ds3232.c
 @@ -463,7 +463,10 @@ static int ds3232_suspend(struct device *dev)
 
   if (device_can_wakeup(dev)) {
   ds3232-suspended = true;
 - irq_set_irq_wake(client-irq, 1);
 + if (irq_set_irq_wake(client-irq, 1)) {
 + dev_warn_once(dev, Can not set wakeup sources\n);
 + ds3232-suspended = false;
 + }
   }
 
   return 0;
 --
 2.1.0.27.g96db324

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v3 1/2] Documentation: DT: FTM: add FTM0 be used as alarm timer

[Wang Dongsheng]

Thank Yoder,

Fix them in next version.

Regards,
-Dongsheng

 -Original Message-
 From: Stuart Yoder [mailto:b08...@gmail.com]
 Sent: Friday, August 14, 2015 12:23 AM
 To: Wang Dongsheng-B40534
 Cc: shawn@linaro.org; devicet...@vger.kernel.org; linux-
 ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Wang Huan-B18965
 Subject: Re: [PATCH v3 1/2] Documentation: DT: FTM: add FTM0 be used as alarm
 timer
 
 On Tue, Aug 11, 2015 at 11:01 PM, Dongsheng Wang
 dongsheng.w...@freescale.com wrote:
  From: Wang Dongsheng dongsheng.w...@freescale.com
 
  In freescale layerscape platform there is only FTM0 can be used as
  alarm timer to wake up system. So add FTM0 description for devicetree
  document.
 
 Suggestion:
 
 In the Freescale Layerscape platform a flextimer module can be used
 as an alarm timer to wake up the system.  Define a binding for this
 alarm.
 
  Signed-off-by: Wang Dongsheng dongsheng.w...@freescale.com
  ---
  V3:
  Include this patch in V3.
 
  diff --git a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt
 b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt
  index aa8c402..380a0b3d 100644
  --- a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt
  +++ b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt
  @@ -1,5 +1,7 @@
   Freescale FlexTimer Module (FTM) Timer
 
  +* Default FTM Timer
  +
   Required properties:
 
   - compatible : should be fsl,ftm-timer
  @@ -29,3 +31,26 @@ ftm: ftm@400b8000 {
  clks VF610_CLK_FTM3_EXT_FIX_EN;
  big-endian;
   };
  +
  +* FTM Alarm Timer
  +  The default FTM device contains eight FlexTimer modules. FlexTimer1 is in
  +  on-domain(power is not switched off in deep sleep mode). Other seven
 FlexTimer
  +  modules(flextimer2/3/4/5/6/7/8) are in off-domain (power is switched off 
  in
  +  deep sleep mode).
 
 Suggestion:
 
   This binding describes a Flextimer (FTM) instance that can be used as an 
 alarm
   to wake up a system in deep sleep.  The flextimer module with alarm
   support is in a power domain that remains awake when the SoC is in
   deep sleep mode.
 
 (don't use flextimer1 in the binding...for all you know in the next SoC it
 will be flextimer7 that has this capability)
 
  +Required properties:
  +
  +- compatible : should be fsl,ftm-alarm.
  +- reg : should contain base address and length of FTM timer 0 register.
 
   should contain the address and size of the FTM alarm module registers
 
 (don't use the number 0)
 
  +- interrupts : Should contain FTM 0 interrupt.
 
describes the FTM alarm interrupt
 
  +- big-endian: One boolean property, the big endian mode will be in use if 
  it
 is
  +  present, or the little endian mode will be in use for all the device
 registers.
  +
  +Example:
  +ftm0: ftm0@29d {
  +   compatible = fsl,ftm-alarm;
  +   reg = 0x0 0x29d 0x0 0x1;
  +   interrupts = GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH;
  +   big-endian;
  +   status = disabled;
  +};
 
 Drop the number 0 from the names in your example.
 
 Thanks,
 Stuart
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform

[Wang Dongsheng]

 -Original Message-
 From: Linus Walleij [mailto:linus.wall...@linaro.org]
 Sent: Thursday, August 13, 2015 9:54 PM
 To: Wang Dongsheng-B40534; John Stultz; Alessandro Zummo; Alexandre Belloni
 Cc: Shawn Guo; Nair, Sandeep; Hans de Goede; Wang Huan-B18965; linux-arm-
 ker...@lists.infradead.org; linux-kernel@vger.kernel.org; rtc-
 li...@googlegroups.com
 Subject: Re: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform
 
 On Wed, Aug 12, 2015 at 7:53 AM, Dongsheng Wang
 dongsheng.w...@freescale.com wrote:
 
  From: Wang Dongsheng dongsheng.w...@freescale.com
 
  Only Ftm0 can be used when system going to deep sleep. So this driver
  to support ftm0 as a wakeup source.
 
  Signed-off-by: Wang Dongsheng dongsheng.w...@freescale.com
  ---
  *V2*
  Change Copyright 2014 to 2015.
 (...)
  +config FTM_ALARM
  +   bool FTM alarm driver
  +   depends on SOC_LS1021A
  +   default n
  +   help
  + Say y here to enable FTM alarm support.  The FTM alarm provides
  + alarm functions for wakeup system from deep sleep.  There is only
  + one FTM can be used in ALARM(FTM 0).
 (...)
  +static u32 time_to_cycle(unsigned long time)
  +static u32 cycle_to_time(u32 cycle)
  +static int ftm_set_alarm(u64 cycle)
  +static irqreturn_t ftm_alarm_interrupt(int irq, void *dev_id)
  +static ssize_t ftm_alarm_show(struct device *dev,
  + struct device_attribute *attr,
  + char *buf)
  +static ssize_t ftm_alarm_store(struct device *dev,
  +  struct device_attribute *attr,
  +  const char *buf, size_t count)
 (...)
  +static struct device_attribute ftm_alarm_attributes = __ATTR(ftm_alarm, 
  0644,
  +   ftm_alarm_show, ftm_alarm_store);
 
 If you're gonna invent ABIs, document then in Documentation/ABI/testing/*.
 
 But I don't get it. Why is this driver not in drivers/rtc?
 
 It does a subset of what an RTC does. The ioctl()'s of an RTC
 can do what you want to do. And much much more.
 
 If it can't do all an RTC can do, surely the RTC subsystem
 can be augmented to host it anyway. It's way to close to
 an RTC to have it's own random sysfs driver like this.
 
 Unless I'm totally off, rewrite this to an RTC driver and post
 it to the RTC maintainers.
 

FlexTimer is not a RTC device and not have any rtc deivce function. They belong 
to
different devices, why we need to register this to RTC framework? I am confused 
about this.

Now in freescale layerscape platform this driver is only for FlexTimer0, and not
fit for each flextimer. Because only FlexTimer0 still turn-on when system in 
the Deep Sleep.

If the alarm make you feel confused or mislead you think this is a RTC 
devices. I think
I need to change the alarm to timer.

Regards,
-Dongsheng

RE: [PATCH v2 1/2] soc/fsl: add freescale dir for SOC specific drivers

[Wang Dongsheng]

Hi Russell,

Thanks for your review. :)

 -Original Message-
 From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk]
 Sent: Wednesday, August 12, 2015 3:45 PM
 To: Wang Dongsheng-B40534
 Cc: shawn@linaro.org; Wang Huan-B18965; linus.wall...@linaro.org; linux-
 ker...@vger.kernel.org; sandee...@ti.com; hdego...@redhat.com; linux-arm-
 ker...@lists.infradead.org
 Subject: Re: [PATCH v2 1/2] soc/fsl: add freescale dir for SOC specific 
 drivers
 
 On Wed, Aug 12, 2015 at 01:53:26PM +0800, Dongsheng Wang wrote:
  diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig new
  file mode 100644 index 000..863d1ef
  --- /dev/null
  +++ b/drivers/soc/fsl/Kconfig
  @@ -0,0 +1,18 @@
  +#
  +# Freescale SOC drivers
  +#
  +menuconfig FSL_SOC_DRIVERS
  +   bool Freescale Soc Drivers
  +   default n
 
 No need for default n.

Thanks. Fix it in next version.

  +   help
  +   Say y here to enable Freescale LS1021A Soc Device Drivers support.
  +   The Soc Drivers provides the device driver that is a specific block
  +   or feature on LS1021A platform.
 
 Help text should be indented by two spaces as per almost every other help 
 text.
 

Thanks. My mistake...

  +
  +if LS1_SOC_DRIVERS
  +   source drivers/soc/fsl/ls1/Kconfig
  +endif
  diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile new
  file mode 100644 index 000..b4215dd
  --- /dev/null
  +++ b/drivers/soc/fsl/Makefile
  @@ -0,0 +1,6 @@
  +#
  +# Makefile for Freescale Soc specific device drivers.
  +#
  +
  +obj-$(CONFIG_LS1_SOC_DRIVERS) += ls1/
  +
  diff --git a/drivers/soc/fsl/ls1/Kconfig b/drivers/soc/fsl/ls1/Kconfig
  new file mode 100644 index 000..7556f44
  --- /dev/null
  +++ b/drivers/soc/fsl/ls1/Kconfig
  @@ -0,0 +1,3 @@
  +#
  +# LS-1 Soc drivers
  +#
 
 Doesn't this directory need a Makefile as well?
 

2/2 patch add a config option and Makefile for this.

Regards,
-Dongsheng
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v2 1/2] soc/fsl: add freescale dir for SOC specific drivers

[Wang Dongsheng]

 -Original Message-
 From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk]
 Sent: Wednesday, August 12, 2015 4:04 PM
 To: Wang Dongsheng-B40534
 Cc: shawn@linaro.org; Wang Huan-B18965; linus.wall...@linaro.org; linux-
 ker...@vger.kernel.org; sandee...@ti.com; hdego...@redhat.com; linux-arm-
 ker...@lists.infradead.org
 Subject: Re: [PATCH v2 1/2] soc/fsl: add freescale dir for SOC specific 
 drivers
 
 On Wed, Aug 12, 2015 at 08:01:32AM +, Wang Dongsheng wrote:
  Hi Russell,
 
  Thanks for your review. :)
 
   -Original Message-
   From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk]
   Sent: Wednesday, August 12, 2015 3:45 PM
   To: Wang Dongsheng-B40534
   Cc: shawn@linaro.org; Wang Huan-B18965;
   linus.wall...@linaro.org; linux- ker...@vger.kernel.org;
   sandee...@ti.com; hdego...@redhat.com; linux-arm-
   ker...@lists.infradead.org
   Subject: Re: [PATCH v2 1/2] soc/fsl: add freescale dir for SOC
   specific drivers
  
   On Wed, Aug 12, 2015 at 01:53:26PM +0800, Dongsheng Wang wrote:
diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig new
file mode 100644 index 000..863d1ef
--- /dev/null
+++ b/drivers/soc/fsl/Kconfig
@@ -0,0 +1,18 @@
+#
+# Freescale SOC drivers
+#
+menuconfig FSL_SOC_DRIVERS
+   bool Freescale Soc Drivers
+   default n
  
   No need for default n.
 
  Thanks. Fix it in next version.
 
+   help
+   Say y here to enable Freescale LS1021A Soc Device Drivers 
support.
+   The Soc Drivers provides the device driver that is a specific 
block
+   or feature on LS1021A platform.
  
   Help text should be indented by two spaces as per almost every other help
 text.
  
 
  Thanks. My mistake...
 
+
+if LS1_SOC_DRIVERS
+   source drivers/soc/fsl/ls1/Kconfig
+endif
diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
new file mode 100644 index 000..b4215dd
--- /dev/null
+++ b/drivers/soc/fsl/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for Freescale Soc specific device drivers.
+#
+
+obj-$(CONFIG_LS1_SOC_DRIVERS) += ls1/
+
diff --git a/drivers/soc/fsl/ls1/Kconfig
b/drivers/soc/fsl/ls1/Kconfig new file mode 100644 index
000..7556f44
--- /dev/null
+++ b/drivers/soc/fsl/ls1/Kconfig
@@ -0,0 +1,3 @@
+#
+# LS-1 Soc drivers
+#
  
   Doesn't this directory need a Makefile as well?
  
 
  2/2 patch add a config option and Makefile for this.
 
 If _just_ this patch is merged, it creates a build problem as
 CONFIG_LS1_SOC_DRIVERS can be enabled, which will cause the kbuild to decend
 into drivers/soc/fsl/ls1, where it will stop due to the missing build error.
 
 Please fix this by adding at least an empty Makefile to this directory.
 Do not rely on patch 2 being merged to fix this.
 

Um..Yes, miss it. Fix it in next version. :)

Regards,
-Dongsheng

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] rtc/ds3232: fix ds3232 get a WARNING trace in resume function

[Wang Dongsheng]

Hi Belloni,

I am not found this patch in your tree(rtc-fixes and rtc-next), Need I send v2 
patch? :)

Regards,
-Dongsheng

 -Original Message-
 From: Wang Dongsheng-B40534
 Sent: Wednesday, July 15, 2015 10:06 AM
 To: 'Alexandre Belloni'
 Cc: a.zu...@towertech.it; rtc-li...@googlegroups.com; linux-
 ker...@vger.kernel.org
 Subject: RE: [PATCH] rtc/ds3232: fix ds3232 get a WARNING trace in resume
 function
 
 Thanks Belloni. :)
 
 Regards,
 -Dongsheng
 
  -Original Message-
  From: Alexandre Belloni [mailto:alexandre.bell...@free-electrons.com]
  Sent: Wednesday, July 15, 2015 6:51 AM
  To: Wang Dongsheng-B40534
  Cc: a.zu...@towertech.it; rtc-li...@googlegroups.com; linux-
  ker...@vger.kernel.org
  Subject: Re: [PATCH] rtc/ds3232: fix ds3232 get a WARNING trace in
  resume function
 
  Hi,
 
  This seems ok, one small nitpick:
 
  On 07/07/2015 at 14:12:56 +0800, Dongsheng Wang wrote :
   From: Wang Dongsheng dongsheng.w...@freescale.com diff --git
   a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c index
   7e48e53..2081155 100644
   --- a/drivers/rtc/rtc-ds3232.c
   +++ b/drivers/rtc/rtc-ds3232.c
   @@ -463,7 +463,10 @@ static int ds3232_suspend(struct device *dev)
  
 if (device_can_wakeup(dev)) {
 ds3232-suspended = true;
   - irq_set_irq_wake(client-irq, 1);
   + if (irq_set_irq_wake(client-irq, 1)) {
   + dev_info(dev, Cannot serve as a wakeup source\n);
 
  I would use dev_warn_once or dev_info_once here to avoid spamming the
  log each time the machine is suspended.
 
  --
  Alexandre Belloni, Free Electrons
  Embedded Linux, Kernel and Android engineering
  http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v2] ARM: dts: ls1021a: add wakeup device ftm0 node for ls1021a

[Wang Dongsheng]

Hi Shawn,

 -Original Message-
 From: Shawn Guo [mailto:shawn...@kernel.org]
 Sent: Tuesday, July 28, 2015 9:50 AM
 To: Wang Huan-B18965
 Cc: shawn@linaro.org; linux-arm-ker...@lists.infradead.org; linux-
 ker...@vger.kernel.org; Wang Dongsheng-B40534; Wang Huan-B18965
 Subject: Re: [PATCH v2] ARM: dts: ls1021a: add wakeup device ftm0 node for
 ls1021a
 
 On Wed, Jul 15, 2015 at 03:40:11PM +0800, Alison Wang wrote:
  Add ftm0 node, cause of ftm0 can be set as a alarm before system going
  to deep sleep.
 
  Signed-off-by: Wang Dongsheng dongsheng.w...@freescale.com
  Signed-off-by: Alison Wang alison.w...@freescale.com
  ---
  Changes since v1:
  - Add my SoB.
  - Use ARM: as subject prefix.
 
   arch/arm/boot/dts/ls1021a-qds.dts | 4 
   arch/arm/boot/dts/ls1021a.dtsi| 8 
   2 files changed, 12 insertions(+)
 
  diff --git a/arch/arm/boot/dts/ls1021a-qds.dts
  b/arch/arm/boot/dts/ls1021a-qds.dts
  index 9c5e16b..f14731b 100644
  --- a/arch/arm/boot/dts/ls1021a-qds.dts
  +++ b/arch/arm/boot/dts/ls1021a-qds.dts
  @@ -75,6 +75,10 @@
  };
   };
 
  +ftm0 {
  +   status = okay;
  +};
  +
   i2c0 {
  status = okay;
 
  diff --git a/arch/arm/boot/dts/ls1021a.dtsi
  b/arch/arm/boot/dts/ls1021a.dtsi index c70bb27..7e9e122 100644
  --- a/arch/arm/boot/dts/ls1021a.dtsi
  +++ b/arch/arm/boot/dts/ls1021a.dtsi
  @@ -332,6 +332,14 @@
  status = disabled;
  };
 
  +   ftm0: ftm0@29d {
 
 Is the '0' in the node name is an instance number?  If so, please drop it from
 node name.

0 is a special number for FTM, because there is only ftm0 can be maked as a 
alarm timer.

Regards,
-Dongsheng

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] rtc/ds3232: fix ds3232 get a WARNING trace in resume function

[Wang Dongsheng]

Thanks Belloni. :)

Regards,
-Dongsheng

 -Original Message-
 From: Alexandre Belloni [mailto:alexandre.bell...@free-electrons.com]
 Sent: Wednesday, July 15, 2015 6:51 AM
 To: Wang Dongsheng-B40534
 Cc: a.zu...@towertech.it; rtc-li...@googlegroups.com; linux-
 ker...@vger.kernel.org
 Subject: Re: [PATCH] rtc/ds3232: fix ds3232 get a WARNING trace in resume
 function
 
 Hi,
 
 This seems ok, one small nitpick:
 
 On 07/07/2015 at 14:12:56 +0800, Dongsheng Wang wrote :
  From: Wang Dongsheng dongsheng.w...@freescale.com diff --git
  a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c index
  7e48e53..2081155 100644
  --- a/drivers/rtc/rtc-ds3232.c
  +++ b/drivers/rtc/rtc-ds3232.c
  @@ -463,7 +463,10 @@ static int ds3232_suspend(struct device *dev)
 
  if (device_can_wakeup(dev)) {
  ds3232-suspended = true;
  -   irq_set_irq_wake(client-irq, 1);
  +   if (irq_set_irq_wake(client-irq, 1)) {
  +   dev_info(dev, Cannot serve as a wakeup source\n);
 
 I would use dev_warn_once or dev_info_once here to avoid spamming the log each
 time the machine is suspended.
 
 --
 Alexandre Belloni, Free Electrons
 Embedded Linux, Kernel and Android engineering http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 04/17] powerpc: mpic: use IRQCHIP_SKIP_SET_WAKE instead of redundant mpic_irq_set_wake

[Wang Dongsheng]

> -Original Message-
> From: Wood Scott-B07421
> Sent: Wednesday, September 23, 2015 7:50 AM
> To: Sudeep Holla
> Cc: linux...@vger.kernel.org; linux-kernel@vger.kernel.org; Thomas Gleixner;
> Rafael J. Wysocki; Benjamin Herrenschmidt; Paul Mackerras; Michael Ellerman; 
> Jia
> Hongtao-B38951; Marc Zyngier; linuxppc-...@lists.ozlabs.org; Wang Dongsheng-
> B40534
> Subject: Re: [PATCH 04/17] powerpc: mpic: use IRQCHIP_SKIP_SET_WAKE instead of
> redundant mpic_irq_set_wake
> 
> On Mon, 2015-09-21 at 16:47 +0100, Sudeep Holla wrote:
> > mpic_irq_set_wake return -ENXIO for non FSL MPIC and sets IRQF_NO_SUSPEND
> > flag for FSL ones. enable_irq_wake already returns -ENXIO if irq_set_wak
> > is not implemented. Also there's no need to set the IRQF_NO_SUSPEND flag
> > as it doesn't guarantee wakeup for that interrupt.
> >

Non-freescale return -ENXIO, is there any issue? If non-freescale platform does
not support it, but IPs still use enable/disable_irq_wake, we should return a 
error number.

IRQCHIP_SKIP_SET_WAKE just skip this feature, this is not our expected.
If non-freescale platform need this flag to skip this feature, it should be add
in self platform.

@Scott:
If set this flag we cannot keep a irq as a wakeup source when system going to
SUSPEND or MEM.

irq_set_wake() means we can set this irq as a wake source.
IRQCHIP_SKIP_SET_WAKE is ignore irq_set_wake() feature.

Regards,
-Dongsheng

RE: [PATCH 04/17] powerpc: mpic: use IRQCHIP_SKIP_SET_WAKE instead of redundant mpic_irq_set_wake

[Wang Dongsheng]

> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel-
> ow...@vger.kernel.org] On Behalf Of Thomas Gleixner
> Sent: Wednesday, September 23, 2015 11:49 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; Sudeep Holla; linux...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Rafael J. Wysocki; Benjamin Herrenschmidt; Paul
> Mackerras; Michael Ellerman; Jia Hongtao-B38951; Marc Zyngier; linuxppc-
> d...@lists.ozlabs.org
> Subject: RE: [PATCH 04/17] powerpc: mpic: use IRQCHIP_SKIP_SET_WAKE instead of
> redundant mpic_irq_set_wake
> 
> On Wed, 23 Sep 2015, Wang Dongsheng wrote:
> > > On Mon, 2015-09-21 at 16:47 +0100, Sudeep Holla wrote:
> > > > mpic_irq_set_wake return -ENXIO for non FSL MPIC and sets 
> > > > IRQF_NO_SUSPEND
> > > > flag for FSL ones. enable_irq_wake already returns -ENXIO if irq_set_wak
> > > > is not implemented. Also there's no need to set the IRQF_NO_SUSPEND flag
> > > > as it doesn't guarantee wakeup for that interrupt.
> > > >
> >
> > Non-freescale return -ENXIO, is there any issue? If non-freescale
> > platform does not support it, but IPs still use
> > enable/disable_irq_wake, we should return a error number.
> 
> You can just set IRQCHIP_SKIP_SET_WAKE for FSL chips and not for the
> others.
> 
> > @Scott:
> > If set this flag we cannot keep a irq as a wakeup source when system going 
> > to
> > SUSPEND or MEM.
> >
> > irq_set_wake() means we can set this irq as a wake source.
> > IRQCHIP_SKIP_SET_WAKE is ignore irq_set_wake() feature.
> 
> Nonsense. IRQCHIP_SKIP_SET_WAKE merily tells the core not to bail on
> !chip->irq_set_wake(), but its still marking the interrupt as wakeup
> source and therefor not masking it on suspend.
> 

Sorry, I just check irq_set_irq_wake() code, right, IRQCHIP_SKIP_SET_WAKE also 
can
going to irqd_set to mask IRQD_WAKEUP_STATE.

Yes, this flag just skip the irq_set_wake() not this feature.

Regards,
-Dongsheng

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] phy: rcar: fix gen3-usb2 build error

[Wang Dongsheng]

of_usb_get_dr_mode_by_phy is implemented in usb/common/common.c.
So the PHY_RCAR_GEN3_USB2 must depend on USB_SUPPORT/USB_COMMON.

 LD  vmlinux.o
 MODPOST vmlinux.o
.../phy-rcar-gen3-usb2.o: In function `rcar_gen3_phy_usb2_probe':
...: undefined reference to `of_usb_get_dr_mode_by_phy'
make: *** [vmlinux] Error 1

Signed-off-by: Wang Dongsheng <dongsheng.w...@hxt-semitech.com>
---
 drivers/phy/renesas/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/phy/renesas/Kconfig b/drivers/phy/renesas/Kconfig
index cb09245..c845fac 100644
--- a/drivers/phy/renesas/Kconfig
+++ b/drivers/phy/renesas/Kconfig
@@ -12,7 +12,9 @@ config PHY_RCAR_GEN3_USB2
tristate "Renesas R-Car generation 3 USB 2.0 PHY driver"
depends on ARCH_RENESAS
depends on EXTCON
+   depends on USB_SUPPORT
select GENERIC_PHY
+   select USB_COMMON
help
  Support for USB 2.0 PHY found on Renesas R-Car generation 3 SoCs.
 
-- 
2.7.4

Re: Re: [PATCH v4] net: qcom/emac: extend DMA mask to 46bits

[Wang, Dongsheng]

Thanks.

Cheers,
-Dongsheng

On 2018/1/23 12:48:27, "Timur Tabi" <ti...@codeaurora.org> wrote:

>On 1/22/18 10:25 PM, Wang Dongsheng wrote:
>>Bit TPD3[31] is used as a timestamp bit if PTP is enabled, but
>>it's used as an address bit if PTP is disabled.  Since PTP isn't
>>supported by the driver, we can extend the DMA address to 46 bits.
>>
>>Signed-off-by: Wang Dongsheng<dongsheng.w...@hxt-semitech.com>
>
>Acked-by: Timur Tabi <ti...@codeaurora.org>
>
>--
>Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
>Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
>Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: Re: [PATCH v3] ACPI / tables: Add IORT to injectable table list

[Wang, Dongsheng]

Hey, Hanjun,

On 2018/2/2 19:54:24, "Hanjun Guo"  wrote:

>On 2018/2/2 18:25, Yang Shunyong wrote:
>>Loading IORT table from initrd is used to fix firmware IORT defects.
>
>I don't think this fix "firmware defects", it just for debug purpose,
>we will not use that for production purpose, right? I think above line
>can be removed.
>
I thinks the upgrade feature not only for debug. Here's an important
way to fix bugs that come from the firmware.

Documentation/acpi/initrd_table_override.txt


Cheers,
-Dongsheng

Re: [PATCH v3 1/2] net: ethernet: i40e: fix build error

[Wang, Dongsheng]

Hello all,

The i40e_ethtool_stats.h is just included by i40e/i40e_ethtool.c. So the
static doesn't make any affect. And Carolyn's team is working rebuild
i40e and i40evf.

Cheers,
Dongsheng

On 9/7/2018 7:19 PM, Wang, Dongsheng wrote:
> Remove "inline" from __i40e_add_stat_strings and move the function.
>
> In file included from
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c:9:0:
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function
> __i40e_add_stat_string:
> drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h:193:20: error:
> function __i40e_add_stat_strings can never be inlined because it uses
> variable argument lists
>  static inline void __i40e_add_stat_strings(u8 **p, const struct
>   i40e_stats stats[],
>
> Tested on: x86_64, make ARCH=i386
>
> Modules section .text:
> i40e: 00019380 <__i40e_add_stat_strings>:
> i40evf: 6b00 <__i40e_add_stat_strings>:
>
> Buildin section .text:
> i40e: c351ca60 <__i40e_add_stat_strings>:
> i40evf: c354f2c0 <__i40e_add_stat_strings>:
>
> Signed-off-by: Wang Dongsheng 
> ---
> V3: add static 
> V2: Move function
> ---
>  .../net/ethernet/intel/i40e/i40e_ethtool.c| 24 ++
>  .../ethernet/intel/i40e/i40e_ethtool_stats.h  | 25 ++-
>  2 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c 
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index d7d3974beca2..34121a72f2e7 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -1821,6 +1821,30 @@ static void i40e_get_ethtool_stats(struct net_device 
> *netdev,
> "ethtool stats count mismatch!");
>  }
>  
> +/**
> + * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> + * @p: ethtool supplied buffer
> + * @stats: stat definitions array
> + * @size: size of the stats array
> + *
> + * Format and copy the strings described by stats into the buffer pointed at
> + * by p.
> + **/
> +static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> + const unsigned int size, ...)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < size; i++) {
> + va_list args;
> +
> + va_start(args, size);
> + vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> + *p += ETH_GSTRING_LEN;
> + va_end(args);
> + }
> +}
> +
>  /**
>   * i40e_get_stat_strings - copy stat strings into supplied buffer
>   * @netdev: the netdev to collect strings for
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h 
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> index bba1cb0b658f..553b0d720839 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> @@ -181,29 +181,8 @@ i40e_add_queue_stats(u64 **data, struct i40e_ring *ring)
>   *data += size;
>  }
>  
> -/**
> - * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> - * @p: ethtool supplied buffer
> - * @stats: stat definitions array
> - * @size: size of the stats array
> - *
> - * Format and copy the strings described by stats into the buffer pointed at
> - * by p.
> - **/
> -static inline void __i40e_add_stat_strings(u8 **p, const struct i40e_stats 
> stats[],
> - const unsigned int size, ...)
> -{
> - unsigned int i;
> -
> - for (i = 0; i < size; i++) {
> - va_list args;
> -
> - va_start(args, size);
> - vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> - *p += ETH_GSTRING_LEN;
> - va_end(args);
> - }
> -}
> +static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> + const unsigned int size, ...);
>  
>  /**
>   * 40e_add_stat_strings - copy stat strings into ethtool buffer

[PATCH v4] net: qcom/emac: extend DMA mask to 46bits

[Wang Dongsheng]

Bit TPD3[31] is used as a timestamp bit if PTP is enabled, but
it's used as an address bit if PTP is disabled.  Since PTP isn't
supported by the driver, we can extend the DMA address to 46 bits.

Signed-off-by: Wang Dongsheng <dongsheng.w...@hxt-semitech.com>
---
v4:
- Changes: PATCH's description.
v3:
- Add: comments for TPD_BUFFER_ADDR_H_SET.
- Remove: "Dynamic fix TPD_BUFFER_ADDR_H_SET size."
v2:
- Add: Dynamic fix TPD_BUFFER_ADDR_H_SET size.
- Add: Comments for DMA MASK.
- Changes: PATCH subject.
- Modify: DMA MASK to 46bits.
---
 drivers/net/ethernet/qualcomm/emac/emac-mac.h | 3 ++-
 drivers/net/ethernet/qualcomm/emac/emac.c | 7 +--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.h 
b/drivers/net/ethernet/qualcomm/emac/emac-mac.h
index 5028fb4..4beedb8 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.h
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.h
@@ -114,8 +114,9 @@ struct emac_tpd {
 #define TPD_INSTC_SET(tpd, val)BITS_SET((tpd)->word[3], 17, 
17, val)
 /* High-14bit Buffer Address, So, the 64b-bit address is
  * {DESC_CTRL_11_TX_DATA_HIADDR[17:0],(register) BUFFER_ADDR_H, BUFFER_ADDR_L}
+ * Extend TPD_BUFFER_ADDR_H to [31, 18], because we never enable timestamping.
  */
-#define TPD_BUFFER_ADDR_H_SET(tpd, val)BITS_SET((tpd)->word[3], 18, 
30, val)
+#define TPD_BUFFER_ADDR_H_SET(tpd, val)BITS_SET((tpd)->word[3], 18, 
31, val)
 /* Format D. Word offset from the 1st byte of this packet to start to calculate
  * the custom checksum.
  */
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c 
b/drivers/net/ethernet/qualcomm/emac/emac.c
index 38c924bd..13235ba 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac.c
@@ -615,8 +615,11 @@ static int emac_probe(struct platform_device *pdev)
u32 reg;
int ret;
 
-   /* The TPD buffer address is limited to 45 bits. */
-   ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(45));
+   /* The TPD buffer address is limited to:
+* 1. PTP:  45bits. (Driver doesn't support yet.)
+* 2. NON-PTP:  46bits.
+*/
+   ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(46));
if (ret) {
dev_err(>dev, "could not set DMA mask\n");
return ret;
-- 
2.7.4

RE: Re: [RFC PATCH 2/2] ACPI/IORT: use swiotlb_dma_ops when smmu probe failed

[Wang, Dongsheng]

> -Original Message-
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: Thursday, April 05, 2018 2:57 AM
> To: Lorenzo Pieralisi <lorenzo.pieral...@arm.com>; Wang, Dongsheng
> <dongsheng.w...@hxt-semitech.com>
> Cc: r...@rjwysocki.net; gre...@linuxfoundation.org; hanjun@linaro.org;
> sudeep.ho...@arm.com; Zheng, Joey <yu.zh...@hxt-semitech.com>;
> linux-a...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [此邮件可能存在风险] Re: [RFC PATCH 2/2] ACPI/IORT: use
> swiotlb_dma_ops when smmu probe failed
> 
> On 04/04/18 17:01, Lorenzo Pieralisi wrote:
> > [+cc Robin]
> >
> > On Thu, Mar 29, 2018 at 03:01:00AM -0700, Wang Dongsheng wrote:
> >> If SMMU probe failed, master should use swiotlb as dma ops.
> >> SMMU may probe failed with specified environment, so there
> >> are not any iommu resources in iommu_device_list.
> >>
> >> The master will always get EPROBE_DEFER from really_probe
> >> (dma_configure) but in fact SMMU has probe failed. The issue
> >> causes all of masters failed to be driven.
> 
> Let's just take a step back - why is SMMU probe failing? That seems to
> be the primary issue here, because it implies that either your hardware,
> firmware or kernel is broken, any of which would make boot failure
> somewhat unsurprising anyway.
> 
It's actually not a hardware issue. This is my test case, just return
-EINVAL in arm_smmu_device_probe. The HW probe(arm_smmu_device_hw_probe)
is just part of SMMU driver probe and the failure may be caused by SW. So
I design this case, just make sure even if SMMU probe failed that cause by SW,
the MASTER also can work. _Because of our SMMU default mode is bypass._


> > I added Robin to pick his brain. An alternative would consist
> > in using a bus notifier to prevent deferred probing once the SMMU
> > driver probing failed but that seems backwards given that a major
> > reason to move to deferred probing was to remove the bus notifiers
> > dependency in the first place.
> >
> > It seems to me this is both an OF/ACPI issue - it is not an IORT
> > only problem.
> 
> Yes, this is just an instance of the general probe-deferral problem,
> e.g. once you have multiple dependencies it's possible to end up in a
> stalemate where everything including the IOMMU ends up on the deferred
> probe list with nothing to kick it and make progress again.
> 
> Furthermore it seems to me that the whole premise in this patch is
> flawed,
Ditto. :)


> since even genuine probe failure may well be transient - just
> because one attempt failed doesn't mean a later attempt can't succeed.
> Thus "the most recent probe attempt failed" cannot be considered a
> fundamentally different state from "no driver is currently bound".
> 
Agree, the genuine probe failure may well be transient. But there is
depend on SMMU probe(IOMMU instance) status. There are two situations:

1. MASTER probing, SMMU doesn't probe yet.
This case will match "the transient failure".
really_probe get an EPROBE_DEFER from IORT and the MASTER probe will be
delayed until SMMU probe successful.
2. MASTER probing, SMMU probe has failed.
really_probe will always get an EPROBE_DEFER from IORT, because kernel
has build in SMMU driver.(iort_iommu_driver_enabled) And the master
never cannot do probe.

The case 2 is I want to handle.

Cheers,
-Dongsheng

> Robin.
> 
> >
> > Lorenzo
> >
> >> Signed-off-by: Wang Dongsheng <dongsheng.w...@hxt-semitech.com>
> >> ---
> >>   drivers/acpi/arm64/iort.c | 39
> +--
> >>   1 file changed, 33 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> >> index e2f7bdd..a6f4c27 100644
> >> --- a/drivers/acpi/arm64/iort.c
> >> +++ b/drivers/acpi/arm64/iort.c
> >> @@ -774,17 +774,45 @@ static int arm_smmu_iort_xlate(struct device
> *dev, u32 streamid,
> >>return ret;
> >>   }
> >>
> >> -static inline bool iort_iommu_driver_enabled(u8 type)
> >> +static int iort_check_dev_dl_status(struct device *dev, void *data)
> >>   {
> >> +  struct fwnode_handle *fwnode = data;
> >> +
> >> +  if (dev->fwnode != fwnode)
> >> +  return 0;
> >> +
> >> +  if (dev->links.status == DL_DEV_PROBE_FAILED)
> >> +  return -ENODEV;
> >> +
> >> +  return -EPROBE_DEFER;
> >> +}
> >> +
> >> +static int iort_iommu_driver_enabled(u8 type, struct fwnode_handle
> *fwnode)
> >

RE: Re: [RFC PATCH 2/2] ACPI/IORT: use swiotlb_dma_ops when smmu probe failed

[Wang, Dongsheng]

> -Original Message-
> From: linux-acpi-ow...@vger.kernel.org
> [mailto:linux-acpi-ow...@vger.kernel.org] On Behalf Of Robin Murphy
> Sent: Monday, April 09, 2018 8:11 PM
> To: Wang, Dongsheng <dongsheng.w...@hxt-semitech.com>; Lorenzo Pieralisi
> <lorenzo.pieral...@arm.com>
> Cc: r...@rjwysocki.net; gre...@linuxfoundation.org; hanjun@linaro.org;
> sudeep.ho...@arm.com; Zheng, Joey <yu.zh...@hxt-semitech.com>;
> linux-a...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [此邮件可能存在风险] Re: [RFC PATCH 2/2] ACPI/IORT: use
> swiotlb_dma_ops when smmu probe failed
> 
> On 08/04/18 09:10, Wang, Dongsheng wrote:
> >
> >> -Original Message-
> >> From: Robin Murphy [mailto:robin.mur...@arm.com]
> >> Sent: Thursday, April 05, 2018 2:57 AM
> >> To: Lorenzo Pieralisi <lorenzo.pieral...@arm.com>; Wang, Dongsheng
> >> <dongsheng.w...@hxt-semitech.com>
> >> Cc: r...@rjwysocki.net; gre...@linuxfoundation.org;
> hanjun@linaro.org;
> >> sudeep.ho...@arm.com; Zheng, Joey <yu.zh...@hxt-semitech.com>;
> >> linux-a...@vger.kernel.org; linux-kernel@vger.kernel.org
> >> Subject: [此邮件可能存在风险] Re: [RFC PATCH 2/2] ACPI/IORT: use
> >> swiotlb_dma_ops when smmu probe failed
> >>
> >> On 04/04/18 17:01, Lorenzo Pieralisi wrote:
> >>> [+cc Robin]
> >>>
> >>> On Thu, Mar 29, 2018 at 03:01:00AM -0700, Wang Dongsheng wrote:
> >>>> If SMMU probe failed, master should use swiotlb as dma ops.
> >>>> SMMU may probe failed with specified environment, so there
> >>>> are not any iommu resources in iommu_device_list.
> >>>>
> >>>> The master will always get EPROBE_DEFER from really_probe
> >>>> (dma_configure) but in fact SMMU has probe failed. The issue
> >>>> causes all of masters failed to be driven.
> >>
> >> Let's just take a step back - why is SMMU probe failing? That seems to
> >> be the primary issue here, because it implies that either your hardware,
> >> firmware or kernel is broken, any of which would make boot failure
> >> somewhat unsurprising anyway.
> >>
> > It's actually not a hardware issue. This is my test case, just return
> > -EINVAL in arm_smmu_device_probe. The HW
> probe(arm_smmu_device_hw_probe)
> > is just part of SMMU driver probe and the failure may be caused by SW. So
> > I design this case, just make sure even if SMMU probe failed that cause by 
> > SW,
> > the MASTER also can work. _Because of our SMMU default mode is bypass._
> 
> I don't think it's particularly justifiable to make core API changes for
> the sake of contrived testcases. On real systems, the SMMU is a
> fundamental system component which is no more expected to fail probe
> than, say, the GIC, and as such if it *does* fail then further progress
> is on a best-effort basis at most.
Yes. Agree with you.


> Just because *your* system happens to work fine in this state doesn't make it 
> true for every SMMU
> implementation and integration that Linux may ever run on.
:(, yes, this is my mistake.


>   
> If you want the kernel to ignore an SMMU, either configure out the
> driver or don't describe that SMMU in firmware in the first place.
> 
> >>> I added Robin to pick his brain. An alternative would consist
> >>> in using a bus notifier to prevent deferred probing once the SMMU
> >>> driver probing failed but that seems backwards given that a major
> >>> reason to move to deferred probing was to remove the bus notifiers
> >>> dependency in the first place.
> >>>
> >>> It seems to me this is both an OF/ACPI issue - it is not an IORT
> >>> only problem.
> >>
> >> Yes, this is just an instance of the general probe-deferral problem,
> >> e.g. once you have multiple dependencies it's possible to end up in a
> >> stalemate where everything including the IOMMU ends up on the deferred
> >> probe list with nothing to kick it and make progress again.
> >>
> >> Furthermore it seems to me that the whole premise in this patch is
> >> flawed,
> > Ditto. :)
> >
> >
> >> since even genuine probe failure may well be transient - just
> >> because one attempt failed doesn't mean a later attempt can't succeed.
> >> Thus "the most recent probe attempt failed" cannot be considered a
> >> fundamentally different state from "no driver is currently bound".
> >>
> > Agree, the genuine probe failure may well be transi

[RFC PATCH 2/2] ACPI/IORT: use swiotlb_dma_ops when smmu probe failed

[Wang Dongsheng]

If SMMU probe failed, master should use swiotlb as dma ops.
SMMU may probe failed with specified environment, so there
are not any iommu resources in iommu_device_list.

The master will always get EPROBE_DEFER from really_probe
(dma_configure) but in fact SMMU has probe failed. The issue
causes all of masters failed to be driven.

Signed-off-by: Wang Dongsheng <dongsheng.w...@hxt-semitech.com>
---
 drivers/acpi/arm64/iort.c | 39 +--
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index e2f7bdd..a6f4c27 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -774,17 +774,45 @@ static int arm_smmu_iort_xlate(struct device *dev, u32 
streamid,
return ret;
 }
 
-static inline bool iort_iommu_driver_enabled(u8 type)
+static int iort_check_dev_dl_status(struct device *dev, void *data)
 {
+   struct fwnode_handle *fwnode = data;
+
+   if (dev->fwnode != fwnode)
+   return 0;
+
+   if (dev->links.status == DL_DEV_PROBE_FAILED)
+   return -ENODEV;
+
+   return -EPROBE_DEFER;
+}
+
+static int iort_iommu_driver_enabled(u8 type, struct fwnode_handle *fwnode)
+{
+   bool buildin;
+   int ret;
+
switch (type) {
case ACPI_IORT_NODE_SMMU_V3:
-   return IS_BUILTIN(CONFIG_ARM_SMMU_V3);
+   buildin = IS_BUILTIN(CONFIG_ARM_SMMU_V3);
+   break;
case ACPI_IORT_NODE_SMMU:
-   return IS_BUILTIN(CONFIG_ARM_SMMU);
+   buildin = IS_BUILTIN(CONFIG_ARM_SMMU);
+   break;
default:
pr_warn("IORT node type %u does not describe an SMMU\n", type);
-   return false;
+   buildin = false;
}
+
+   if (!buildin)
+   return -ENODEV;
+
+   ret = bus_for_each_dev(_bus_type, NULL, fwnode,
+  iort_check_dev_dl_status);
+   if (!ret)
+   return -EPROBE_DEFER;
+
+   return ret;
 }
 
 #ifdef CONFIG_IOMMU_API
@@ -919,8 +947,7 @@ static int iort_iommu_xlate(struct device *dev, struct 
acpi_iort_node *node,
 */
ops = iommu_ops_from_fwnode(iort_fwnode);
if (!ops)
-   return iort_iommu_driver_enabled(node->type) ?
-  -EPROBE_DEFER : -ENODEV;
+   return iort_iommu_driver_enabled(node->type, iort_fwnode);
 
return arm_smmu_iort_xlate(dev, streamid, iort_fwnode, ops);
 }
-- 
2.7.4

[RFC PATCH 0/2] driver core: add new dl device status DL_DEV_PROBE_FAILED

[Wang Dongsheng]

Recently we found the master of SMMU retries to probe endlessly.

[3.658956] pci :00:00.0: Retrying from deferred list
[3.658969] pci :00:00.0: Added to deferred list
[3.658987] pci 0004:00:00.0: Retrying from deferred list
[3.658994] pci 0004:00:00.0: Added to deferred list
[3.659003] pci 0005:00:00.0: Retrying from deferred list
[3.659010] pci 0005:00:00.0: Added to deferred list
[3.659019] pci 0004:01:00.0: Retrying from deferred list
[3.659029] pci 0004:01:00.0: Added to deferred list

The retrying should only happen when the IOMMU instance hasn't been
probed yet.
However, dma_configure() simply return -EPROBE_DEFER when failed to get
an IOMMU instance even if the IOMMU instance is failed to probe.

This patchset tries to fix the issue by distinguishing probe failures and
haven't-been-probed-yet state.

Wang Dongsheng (2):
  driver core: add new dl device status DL_DEV_PROBE_FAILED
  ACPI/IORT: use swiotlb_dma_ops when smmu probe failed

 Documentation/driver-api/device_link.rst |  2 +-
 drivers/acpi/arm64/iort.c| 39 +++-
 drivers/base/base.h  |  2 +-
 drivers/base/core.c  | 22 --
 drivers/base/dd.c|  2 +-
 include/linux/device.h   |  1 +
 6 files changed, 57 insertions(+), 11 deletions(-)

-- 
2.7.4

[RFC PATCH 1/2] driver core: add new dl device status DL_DEV_PROBE_FAILED

[Wang Dongsheng]

Currently the initialization state of device is DL_DEV_NO_DRIVER.
The problem is, after probe failure the state will also be set to
DL_DEV_NO_DRIVER as well. And the device is not linked, it has no
supplier or consumer. Thus adding a new state to distinguish
probe failure and not-probed-yet.

Signed-off-by: Wang Dongsheng <dongsheng.w...@hxt-semitech.com>
---
 Documentation/driver-api/device_link.rst |  2 +-
 drivers/base/base.h  |  2 +-
 drivers/base/core.c  | 22 --
 drivers/base/dd.c|  2 +-
 include/linux/device.h   |  1 +
 5 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/Documentation/driver-api/device_link.rst 
b/Documentation/driver-api/device_link.rst
index 70e328e..9054403 100644
--- a/Documentation/driver-api/device_link.rst
+++ b/Documentation/driver-api/device_link.rst
@@ -247,7 +247,7 @@ State machine
   :c:func:`device_links_unbind_consumers()`.)
 
 * If the probe fails, links to suppliers revert back to ``DL_STATE_AVAILABLE``.
-  (Call to :c:func:`device_links_no_driver()` from :c:func:`really_probe()`.)
+  (Call to :c:func:`device_links_probe_failed()` from 
:c:func:`really_probe()`.)
 
 * If the probe succeeds, links to suppliers progress to ``DL_STATE_ACTIVE``.
   (Call to :c:func:`device_links_driver_bound()` from 
:c:func:`driver_bound()`.)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index d800de6..f9931d9 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -158,6 +158,6 @@ extern void device_links_read_unlock(int idx);
 extern int device_links_check_suppliers(struct device *dev);
 extern void device_links_driver_bound(struct device *dev);
 extern void device_links_driver_cleanup(struct device *dev);
-extern void device_links_no_driver(struct device *dev);
+extern void device_links_probe_failed(struct device *dev);
 extern bool device_links_busy(struct device *dev);
 extern void device_links_unbind_consumers(struct device *dev);
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 5847364..31d4f68 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -452,10 +452,28 @@ static void __device_links_no_driver(struct device *dev)
dev->links.status = DL_DEV_NO_DRIVER;
 }
 
-void device_links_no_driver(struct device *dev)
+static void __device_links_probe_failed(struct device *dev)
+{
+   struct device_link *link, *ln;
+
+   list_for_each_entry_safe_reverse(link, ln, >links.suppliers,
+c_node) {
+   if (link->flags & DL_FLAG_STATELESS)
+   continue;
+
+   if (link->flags & DL_FLAG_AUTOREMOVE)
+   __device_link_del(link);
+   else if (link->status != DL_STATE_SUPPLIER_UNBIND)
+   WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
+   }
+
+   dev->links.status = DL_DEV_PROBE_FAILED;
+}
+
+void device_links_probe_failed(struct device *dev)
 {
device_links_write_lock();
-   __device_links_no_driver(dev);
+   __device_links_probe_failed(dev);
device_links_write_unlock();
 }
 
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index de6fd09..90d57e0 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -492,7 +492,7 @@ static int really_probe(struct device *dev, struct 
device_driver *drv)
blocking_notifier_call_chain(>bus->p->bus_notifier,
 BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
 pinctrl_bind_failed:
-   device_links_no_driver(dev);
+   device_links_probe_failed(dev);
devres_release_all(dev);
driver_sysfs_remove(dev);
dev->driver = NULL;
diff --git a/include/linux/device.h b/include/linux/device.h
index b093405..bf9630a 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -794,6 +794,7 @@ struct device_link {
 enum dl_dev_state {
DL_DEV_NO_DRIVER = 0,
DL_DEV_PROBING,
+   DL_DEV_PROBE_FAILED,
DL_DEV_DRIVER_BOUND,
DL_DEV_UNBINDING,
 };
-- 
2.7.4

Re: [RFC PATCH 1/2] driver core: add new dl device status DL_DEV_PROBE_FAILED

[Wang, Dongsheng]

On Thu, 2018-03-29 at 12:51 +0200, Rafael J. Wysocki wrote:
> On Thu, Mar 29, 2018 at 12:00 PM, Wang Dongsheng
> <dongsheng.w...@hxt-semitech.com> wrote:
> > Currently the initialization state of device is DL_DEV_NO_DRIVER.
> > The problem is, after probe failure the state will also be set to
> > DL_DEV_NO_DRIVER as well. And the device is not linked, it has no
> > supplier or consumer. Thus adding a new state to distinguish
> > probe failure and not-probed-yet.
> > 
> > Signed-off-by: Wang Dongsheng <dongsheng.w...@hxt-semitech.com>
> 
> I guess what you want is a cleanup after a failing probe, but after
> that the state really is "no driver" again, isn't it?
Yes, agree "no driver", device is never get a driver again.

But A depend on B successful probe. if B failed, A will never know:
1. B can't work.
2. B hasn't got a probe yet.

Like IOMMU. After SMMU successful probed, the driver add a resource
into "iommu_device_list". Master lookup the corresponding SMMU fwnode
from "iommu_device_list", after matched master will do probe. but if
the list is NULL master will get -EPROBE_DEFER, means SMMU device may
not probe yet, in fact SMMU may probe failed.

I try to use DL_DEV state to fix this issue, but NO_DRIVER does not
distinguish between the two cases.

Cheers,
-Dongsheng

> 
> > ---
> >  Documentation/driver-api/device_link.rst |  2 +-
> >  drivers/base/base.h  |  2 +-
> >  drivers/base/core.c  | 22
> > --
> >  drivers/base/dd.c|  2 +-
> >  include/linux/device.h   |  1 +
> >  5 files changed, 24 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/device_link.rst
> > b/Documentation/driver-api/device_link.rst
> > index 70e328e..9054403 100644
> > --- a/Documentation/driver-api/device_link.rst
> > +++ b/Documentation/driver-api/device_link.rst
> > @@ -247,7 +247,7 @@ State machine
> >:c:func:`device_links_unbind_consumers()`.)
> > 
> >  * If the probe fails, links to suppliers revert back to
> > ``DL_STATE_AVAILABLE``.
> > -  (Call to :c:func:`device_links_no_driver()` from
> > :c:func:`really_probe()`.)
> > +  (Call to :c:func:`device_links_probe_failed()` from
> > :c:func:`really_probe()`.)
> > 
> >  * If the probe succeeds, links to suppliers progress to
> > ``DL_STATE_ACTIVE``.
> >(Call to :c:func:`device_links_driver_bound()` from
> > :c:func:`driver_bound()`.)
> > diff --git a/drivers/base/base.h b/drivers/base/base.h
> > index d800de6..f9931d9 100644
> > --- a/drivers/base/base.h
> > +++ b/drivers/base/base.h
> > @@ -158,6 +158,6 @@ extern void device_links_read_unlock(int idx);
> >  extern int device_links_check_suppliers(struct device *dev);
> >  extern void device_links_driver_bound(struct device *dev);
> >  extern void device_links_driver_cleanup(struct device *dev);
> > -extern void device_links_no_driver(struct device *dev);
> > +extern void device_links_probe_failed(struct device *dev);
> >  extern bool device_links_busy(struct device *dev);
> >  extern void device_links_unbind_consumers(struct device *dev);
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 5847364..31d4f68 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -452,10 +452,28 @@ static void __device_links_no_driver(struct
> > device *dev)
> > dev->links.status = DL_DEV_NO_DRIVER;
> >  }
> > 
> > -void device_links_no_driver(struct device *dev)
> > +static void __device_links_probe_failed(struct device *dev)
> > +{
> > +   struct device_link *link, *ln;
> > +
> > +   list_for_each_entry_safe_reverse(link, ln, 
> > >links.suppliers,
> > +c_node) {
> > +   if (link->flags & DL_FLAG_STATELESS)
> > +   continue;
> > +
> > +   if (link->flags & DL_FLAG_AUTOREMOVE)
> > +   __device_link_del(link);
> > +   else if (link->status != DL_STATE_SUPPLIER_UNBIND)
> > +   WRITE_ONCE(link->status,
> > DL_STATE_AVAILABLE);
> > +   }
> > +
> > +   dev->links.status = DL_DEV_PROBE_FAILED;
> > +}
> > +
> > +void device_links_probe_failed(struct device *dev)
> >  {
> > device_links_write_lock();
> > -   __device_links_no_driver(dev);
> > +   __device_links_probe_failed(dev);
> > device_links_write_unlock();
>

[PATCH 1/1] ACPI / tables: add DSDT AmlCode new declaration name support

[Wang Dongsheng]

The new naming rule is added in acpica version 20180427.
So the dsdt aml code name changes from "AmlCode" to "dsdt_aml_code".

The patch that introduces naming rules is:
https://github.com/acpica/acpica/commit/f9a88a4c1cd020b6a5475d63b29626852a0b5f37

Tested:
ACPICA release version 20180427+.
ARM64: QCOM QDF2400
GCC: 4.8.5 20150623

Signed-off-by: Wang Dongsheng 
---
 drivers/acpi/Kconfig  |  2 +-
 drivers/acpi/tables.c | 10 --
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 9705fc986da9..15ab53a52fdc 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -336,7 +336,7 @@ config ACPI_CUSTOM_DSDT_FILE
  See Documentation/acpi/dsdt-override.txt
 
  Enter the full path name to the file which includes the AmlCode
- declaration.
+ or dsdt_aml_code declaration.
 
  If unsure, don't enter a file name.
 
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index a3d012b08fc5..297020bbaade 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -713,6 +713,9 @@ acpi_os_physical_table_override(struct acpi_table_header 
*existing_table,
  table_length);
 }
 
+static void *amlcode __attribute__ ((weakref("AmlCode")));
+static void *dsdt_amlcode __attribute__ ((weakref("dsdt_aml_code")));
+
 acpi_status
 acpi_os_table_override(struct acpi_table_header *existing_table,
   struct acpi_table_header **new_table)
@@ -723,8 +726,11 @@ acpi_os_table_override(struct acpi_table_header 
*existing_table,
*new_table = NULL;
 
 #ifdef CONFIG_ACPI_CUSTOM_DSDT
-   if (strncmp(existing_table->signature, "DSDT", 4) == 0)
-   *new_table = (struct acpi_table_header *)AmlCode;
+   if (!strncmp(existing_table->signature, "DSDT", 4)) {
+   *new_table = (struct acpi_table_header *)
+   if (!(*new_table))
+   *new_table = (struct acpi_table_header *)_amlcode;
+   }
 #endif
if (*new_table != NULL)
acpi_table_taint(existing_table);
-- 
2.18.0

Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC

[Wang, Dongsheng]

On 2018/11/30 5:22, Kees Cook wrote:
> On Tue, Nov 27, 2018 at 8:38 PM Wang, Dongsheng
>  wrote:
>> Hello Kees,
>>
>> On 2018/11/28 6:38, Kees Cook wrote:
>>> On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
>>>  wrote:
>>>> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
>>>> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
>>>> is not a real task on stack, it's only init_task on init_stack.
>>>>
>>>> Commit 0500871f21b2 ("Construct init thread stack in the linker script
>>>> rather than by union") added this macro and put task_strcut into
>>>> thread_union. This brings us the following possibilities:
>>>> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
>>>> - <-- thread_info & stack
>>>> NN | | --- <-- task
>>>>| ||   |
>>>> -  ---
>>>>
>>>> - <-- stack
>>>> NY | | --- <-- 
>>>> task(Including thread_info)
>>>>| ||   |
>>>> -  ---
>>>>
>>>> - <-- stack & task & 
>>>> thread_info
>>>> YN | |
>>>>| |
>>>> -
>>>>
>>>> - <-- stack & 
>>>> task(Including thread_info)
>>>> YY | |
>>>>| |
>>>> -
>>>> The kernel has handled the first two cases correctly.
>>>>
>>>> For the third case:
>>>> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
>>>> should never happen, because the task and thread_info will overlap. So
>>>> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>>>>
>>>> For the fourth case:
>>>> When task on stack, the end of stack should add a sizeof(task_struct) 
>>>> offset.
>>>>
>>>> This patch handled with the third and fourth case.
>>>>
>>>> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>>>>
>>>> Signed-off-by: Wang Dongsheng 
>>>> Signed-off-by: Shunyong Yang 
>>>> ---
>>>>  arch/Kconfig | 1 +
>>>>  include/linux/sched/task_stack.h | 5 -
>>>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/Kconfig b/arch/Kconfig
>>>> index e1e540ffa979..0a2c73e73195 100644
>>>> --- a/arch/Kconfig
>>>> +++ b/arch/Kconfig
>>>> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
>>>>  # Select if arch init_task must go in the __init_task_data section
>>>>  config ARCH_TASK_STRUCT_ON_STACK
>>>> bool
>>>> +   depends on THREAD_INFO_IN_TASK || IA64
>>> The "IA64" part shouldn't be needed since IA64 already selects it.
>>>
>>> Since it's selected, it also can't have a depends, IIUC.
>> Since the IA64 thread_info including task_struct, it doesn't need to
>> select THREAD_INFO_IN_TASK.
>> So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
>> THREAD_INFO.
> Okay.
>
>>>>  # Select if arch has its private alloc_task_struct() function
>>>>  config ARCH_TASK_STRUCT_ALLOCATOR
>>>> diff --git a/include/linux/sched/task_stack.h 
>>>> b/include/linux/sched/task_stack.h
>>>> index 6a841929073f..624c48defb9e 100644
>>>> --- a/include/linux/sched/task_stack.h
>>>> +++ b/include/linux/sched/task_stack.h
>>>> @@ -7,6 +7,7 @@
>>>>   */
>>>>
>>>>  #include 
>>>> +#include 
>>>>  #include 
>>>>
>>>>  #ifdef CONFIG_THREAD_INFO_IN_TASK
>>>> @@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct 
>>>> task_struct *task)

Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC

[Wang, Dongsheng]

On 2018/11/30 10:04, Wang, Dongsheng wrote:
> On 2018/11/30 5:22, Kees Cook wrote:
>> On Tue, Nov 27, 2018 at 8:38 PM Wang, Dongsheng
>>  wrote:
>>> Hello Kees,
>>>
>>> On 2018/11/28 6:38, Kees Cook wrote:
>>>> On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
>>>>  wrote:
>>>>> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
>>>>> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
>>>>> is not a real task on stack, it's only init_task on init_stack.
>>>>>
>>>>> Commit 0500871f21b2 ("Construct init thread stack in the linker script
>>>>> rather than by union") added this macro and put task_strcut into
>>>>> thread_union. This brings us the following possibilities:
>>>>> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
>>>>> - <-- thread_info & stack
>>>>> NN | | --- <-- task
>>>>>| ||   |
>>>>> -  ---
>>>>>
>>>>> - <-- stack
>>>>> NY | | --- <-- 
>>>>> task(Including thread_info)
>>>>>| ||   |
>>>>> -  ---
>>>>>
>>>>> - <-- stack & task & 
>>>>> thread_info
>>>>> YN | |
>>>>>| |
>>>>> -
>>>>>
>>>>> - <-- stack & 
>>>>> task(Including thread_info)
>>>>> YY | |
>>>>>| |
>>>>> -
>>>>> The kernel has handled the first two cases correctly.
>>>>>
>>>>> For the third case:
>>>>> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
>>>>> should never happen, because the task and thread_info will overlap. So
>>>>> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>>>>>
>>>>> For the fourth case:
>>>>> When task on stack, the end of stack should add a sizeof(task_struct) 
>>>>> offset.
>>>>>
>>>>> This patch handled with the third and fourth case.
>>>>>
>>>>> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>>>>>
>>>>> Signed-off-by: Wang Dongsheng 
>>>>> Signed-off-by: Shunyong Yang 
>>>>> ---
>>>>>  arch/Kconfig | 1 +
>>>>>  include/linux/sched/task_stack.h | 5 -
>>>>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/Kconfig b/arch/Kconfig
>>>>> index e1e540ffa979..0a2c73e73195 100644
>>>>> --- a/arch/Kconfig
>>>>> +++ b/arch/Kconfig
>>>>> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
>>>>>  # Select if arch init_task must go in the __init_task_data section
>>>>>  config ARCH_TASK_STRUCT_ON_STACK
>>>>> bool
>>>>> +   depends on THREAD_INFO_IN_TASK || IA64
>>>> The "IA64" part shouldn't be needed since IA64 already selects it.
>>>>
>>>> Since it's selected, it also can't have a depends, IIUC.
>>> Since the IA64 thread_info including task_struct, it doesn't need to
>>> select THREAD_INFO_IN_TASK.
>>> So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
>>> THREAD_INFO.
>> Okay.
>>
>>>>>  # Select if arch has its private alloc_task_struct() function
>>>>>  config ARCH_TASK_STRUCT_ALLOCATOR
>>>>> diff --git a/include/linux/sched/task_stack.h 
>>>>> b/include/linux/sched/task_stack.h
>>>>> index 6a841929073f..624c48defb9e 100644
>>>>> --- a/include/linux/sched/task_stack

Re: [PATCH 1/1] ACPI / tables: add DSDT AmlCode new declaration name support

[Wang, Dongsheng]

Hello Robert,

Do you have any comments about this patch?
Thanks.


Cheers
Dongsheng

On 2018/11/13 18:46, Wang, Dongsheng wrote:
> The new naming rule is added in acpica version 20180427.
> So the dsdt aml code name changes from "AmlCode" to "dsdt_aml_code".
>
> The patch that introduces naming rules is:
> https://github.com/acpica/acpica/commit/f9a88a4c1cd020b6a5475d63b29626852a0b5f37
>
> Tested:
> ACPICA release version 20180427+.
> ARM64: QCOM QDF2400
> GCC: 4.8.5 20150623
>
> Signed-off-by: Wang Dongsheng 
> ---
>  drivers/acpi/Kconfig  |  2 +-
>  drivers/acpi/tables.c | 10 --
>  2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 9705fc986da9..15ab53a52fdc 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -336,7 +336,7 @@ config ACPI_CUSTOM_DSDT_FILE
> See Documentation/acpi/dsdt-override.txt
>  
> Enter the full path name to the file which includes the AmlCode
> -   declaration.
> +   or dsdt_aml_code declaration.
>  
> If unsure, don't enter a file name.
>  
> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> index a3d012b08fc5..297020bbaade 100644
> --- a/drivers/acpi/tables.c
> +++ b/drivers/acpi/tables.c
> @@ -713,6 +713,9 @@ acpi_os_physical_table_override(struct acpi_table_header 
> *existing_table,
> table_length);
>  }
>  
> +static void *amlcode __attribute__ ((weakref("AmlCode")));
> +static void *dsdt_amlcode __attribute__ ((weakref("dsdt_aml_code")));
> +
>  acpi_status
>  acpi_os_table_override(struct acpi_table_header *existing_table,
>  struct acpi_table_header **new_table)
> @@ -723,8 +726,11 @@ acpi_os_table_override(struct acpi_table_header 
> *existing_table,
>   *new_table = NULL;
>  
>  #ifdef CONFIG_ACPI_CUSTOM_DSDT
> - if (strncmp(existing_table->signature, "DSDT", 4) == 0)
> - *new_table = (struct acpi_table_header *)AmlCode;
> + if (!strncmp(existing_table->signature, "DSDT", 4)) {
> + *new_table = (struct acpi_table_header *)
> + if (!(*new_table))
> + *new_table = (struct acpi_table_header *)_amlcode;
> + }
>  #endif
>   if (*new_table != NULL)
>   acpi_table_taint(existing_table);

[PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC

[Wang Dongsheng]

When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
is not a real task on stack, it's only init_task on init_stack.

Commit 0500871f21b2 ("Construct init thread stack in the linker script
rather than by union") added this macro and put task_strcut into
thread_union. This brings us the following possibilities:
TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
- <-- thread_info & stack
NN | | --- <-- task
   | ||   |
-  ---

- <-- stack
NY | | --- <-- task(Including 
thread_info)
   | ||   |
-  ---

- <-- stack & task & thread_info
YN | |
   | |
-

- <-- stack & task(Including 
thread_info)
YY | |
   | |
-
The kernel has handled the first two cases correctly.

For the third case:
TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
should never happen, because the task and thread_info will overlap. So
when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.

For the fourth case:
When task on stack, the end of stack should add a sizeof(task_struct) offset.

This patch handled with the third and fourth case.

Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")

Signed-off-by: Wang Dongsheng 
Signed-off-by: Shunyong Yang 
---
 arch/Kconfig | 1 +
 include/linux/sched/task_stack.h | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index e1e540ffa979..0a2c73e73195 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
 # Select if arch init_task must go in the __init_task_data section
 config ARCH_TASK_STRUCT_ON_STACK
bool
+   depends on THREAD_INFO_IN_TASK || IA64
 
 # Select if arch has its private alloc_task_struct() function
 config ARCH_TASK_STRUCT_ALLOCATOR
diff --git a/include/linux/sched/task_stack.h b/include/linux/sched/task_stack.h
index 6a841929073f..624c48defb9e 100644
--- a/include/linux/sched/task_stack.h
+++ b/include/linux/sched/task_stack.h
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 #include 
 
 #ifdef CONFIG_THREAD_INFO_IN_TASK
@@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct task_struct 
*task)
 
 static inline unsigned long *end_of_stack(const struct task_struct *task)
 {
-   return task->stack;
+   if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task != _task)
+   return task->stack;
+   return (unsigned long *)(task + 1);
 }
 
 #elif !defined(__HAVE_THREAD_FUNCTIONS)
-- 
2.19.1

Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC

[Wang, Dongsheng]

Hello Kees,

On 2018/11/28 6:38, Kees Cook wrote:
> On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
>  wrote:
>> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
>> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
>> is not a real task on stack, it's only init_task on init_stack.
>>
>> Commit 0500871f21b2 ("Construct init thread stack in the linker script
>> rather than by union") added this macro and put task_strcut into
>> thread_union. This brings us the following possibilities:
>> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
>> - <-- thread_info & stack
>> NN | | --- <-- task
>>| ||   |
>> -  ---
>>
>> - <-- stack
>> NY | | --- <-- 
>> task(Including thread_info)
>>| ||   |
>> -  ---
>>
>> - <-- stack & task & 
>> thread_info
>> YN | |
>>| |
>> -
>>
>> - <-- stack & task(Including 
>> thread_info)
>> YY | |
>>| |
>> -
>> The kernel has handled the first two cases correctly.
>>
>> For the third case:
>> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
>> should never happen, because the task and thread_info will overlap. So
>> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>>
>> For the fourth case:
>> When task on stack, the end of stack should add a sizeof(task_struct) offset.
>>
>> This patch handled with the third and fourth case.
>>
>> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>>
>> Signed-off-by: Wang Dongsheng 
>> Signed-off-by: Shunyong Yang 
>> ---
>>  arch/Kconfig | 1 +
>>  include/linux/sched/task_stack.h | 5 -
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index e1e540ffa979..0a2c73e73195 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
>>  # Select if arch init_task must go in the __init_task_data section
>>  config ARCH_TASK_STRUCT_ON_STACK
>> bool
>> +   depends on THREAD_INFO_IN_TASK || IA64
> The "IA64" part shouldn't be needed since IA64 already selects it.
>
> Since it's selected, it also can't have a depends, IIUC.

Since the IA64 thread_info including task_struct, it doesn't need to
select THREAD_INFO_IN_TASK.
So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
THREAD_INFO.

>>  # Select if arch has its private alloc_task_struct() function
>>  config ARCH_TASK_STRUCT_ALLOCATOR
>> diff --git a/include/linux/sched/task_stack.h 
>> b/include/linux/sched/task_stack.h
>> index 6a841929073f..624c48defb9e 100644
>> --- a/include/linux/sched/task_stack.h
>> +++ b/include/linux/sched/task_stack.h
>> @@ -7,6 +7,7 @@
>>   */
>>
>>  #include 
>> +#include 
>>  #include 
>>
>>  #ifdef CONFIG_THREAD_INFO_IN_TASK
>> @@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct 
>> task_struct *task)
>>
>>  static inline unsigned long *end_of_stack(const struct task_struct *task)
>>  {
>> -   return task->stack;
>> +   if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task != 
>> _task)
>> +   return task->stack;
>> +   return (unsigned long *)(task + 1);
>>  }
> This seems like a strange place for the change. It feels more like
> init_task has been defined incorrectly.

The init_task will put into init_stack when ARCH_TASK_STRUCT_ON_STACK is
selected.
include/asm-generic/vmlinux.lds.h:
#define INIT_TASK_DATA(align)\
. = ALIGN(align);\
__start_init_task = .;\
init_thread_union = .;\
init_stack = .;\
KEEP(*(.data..init_task))\
KEEP(*(.data..init_thread_info))\
. = __start_init_task + THREAD_SIZE;\
__end_init_task = .;

So we need end_of_stack to offset sizeof(task_struct).

Cheers,
Dongsheng

[RFC PATCH 2/2] ACPI/IORT: use swiotlb_dma_ops when smmu probe failed

[Wang Dongsheng]

If SMMU probe failed, master should use swiotlb as dma ops.
SMMU may probe failed with specified environment, so there
are not any iommu resources in iommu_device_list.

The master will always get EPROBE_DEFER from really_probe
(dma_configure) but in fact SMMU has probe failed. The issue
causes all of masters failed to be driven.

Signed-off-by: Wang Dongsheng 
---
 drivers/acpi/arm64/iort.c | 39 +--
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index e2f7bdd..a6f4c27 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -774,17 +774,45 @@ static int arm_smmu_iort_xlate(struct device *dev, u32 
streamid,
return ret;
 }
 
-static inline bool iort_iommu_driver_enabled(u8 type)
+static int iort_check_dev_dl_status(struct device *dev, void *data)
 {
+   struct fwnode_handle *fwnode = data;
+
+   if (dev->fwnode != fwnode)
+   return 0;
+
+   if (dev->links.status == DL_DEV_PROBE_FAILED)
+   return -ENODEV;
+
+   return -EPROBE_DEFER;
+}
+
+static int iort_iommu_driver_enabled(u8 type, struct fwnode_handle *fwnode)
+{
+   bool buildin;
+   int ret;
+
switch (type) {
case ACPI_IORT_NODE_SMMU_V3:
-   return IS_BUILTIN(CONFIG_ARM_SMMU_V3);
+   buildin = IS_BUILTIN(CONFIG_ARM_SMMU_V3);
+   break;
case ACPI_IORT_NODE_SMMU:
-   return IS_BUILTIN(CONFIG_ARM_SMMU);
+   buildin = IS_BUILTIN(CONFIG_ARM_SMMU);
+   break;
default:
pr_warn("IORT node type %u does not describe an SMMU\n", type);
-   return false;
+   buildin = false;
}
+
+   if (!buildin)
+   return -ENODEV;
+
+   ret = bus_for_each_dev(_bus_type, NULL, fwnode,
+  iort_check_dev_dl_status);
+   if (!ret)
+   return -EPROBE_DEFER;
+
+   return ret;
 }
 
 #ifdef CONFIG_IOMMU_API
@@ -919,8 +947,7 @@ static int iort_iommu_xlate(struct device *dev, struct 
acpi_iort_node *node,
 */
ops = iommu_ops_from_fwnode(iort_fwnode);
if (!ops)
-   return iort_iommu_driver_enabled(node->type) ?
-  -EPROBE_DEFER : -ENODEV;
+   return iort_iommu_driver_enabled(node->type, iort_fwnode);
 
return arm_smmu_iort_xlate(dev, streamid, iort_fwnode, ops);
 }
-- 
2.7.4

[RFC PATCH 1/2] driver core: add new dl device status DL_DEV_PROBE_FAILED

[Wang Dongsheng]

Currently the initialization state of device is DL_DEV_NO_DRIVER.
The problem is, after probe failure the state will also be set to
DL_DEV_NO_DRIVER as well. And the device is not linked, it has no
supplier or consumer. Thus adding a new state to distinguish
probe failure and not-probed-yet.

Signed-off-by: Wang Dongsheng 
---
 Documentation/driver-api/device_link.rst |  2 +-
 drivers/base/base.h  |  2 +-
 drivers/base/core.c  | 22 --
 drivers/base/dd.c|  2 +-
 include/linux/device.h   |  1 +
 5 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/Documentation/driver-api/device_link.rst 
b/Documentation/driver-api/device_link.rst
index 70e328e..9054403 100644
--- a/Documentation/driver-api/device_link.rst
+++ b/Documentation/driver-api/device_link.rst
@@ -247,7 +247,7 @@ State machine
   :c:func:`device_links_unbind_consumers()`.)
 
 * If the probe fails, links to suppliers revert back to ``DL_STATE_AVAILABLE``.
-  (Call to :c:func:`device_links_no_driver()` from :c:func:`really_probe()`.)
+  (Call to :c:func:`device_links_probe_failed()` from 
:c:func:`really_probe()`.)
 
 * If the probe succeeds, links to suppliers progress to ``DL_STATE_ACTIVE``.
   (Call to :c:func:`device_links_driver_bound()` from 
:c:func:`driver_bound()`.)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index d800de6..f9931d9 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -158,6 +158,6 @@ extern void device_links_read_unlock(int idx);
 extern int device_links_check_suppliers(struct device *dev);
 extern void device_links_driver_bound(struct device *dev);
 extern void device_links_driver_cleanup(struct device *dev);
-extern void device_links_no_driver(struct device *dev);
+extern void device_links_probe_failed(struct device *dev);
 extern bool device_links_busy(struct device *dev);
 extern void device_links_unbind_consumers(struct device *dev);
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 5847364..31d4f68 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -452,10 +452,28 @@ static void __device_links_no_driver(struct device *dev)
dev->links.status = DL_DEV_NO_DRIVER;
 }
 
-void device_links_no_driver(struct device *dev)
+static void __device_links_probe_failed(struct device *dev)
+{
+   struct device_link *link, *ln;
+
+   list_for_each_entry_safe_reverse(link, ln, >links.suppliers,
+c_node) {
+   if (link->flags & DL_FLAG_STATELESS)
+   continue;
+
+   if (link->flags & DL_FLAG_AUTOREMOVE)
+   __device_link_del(link);
+   else if (link->status != DL_STATE_SUPPLIER_UNBIND)
+   WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
+   }
+
+   dev->links.status = DL_DEV_PROBE_FAILED;
+}
+
+void device_links_probe_failed(struct device *dev)
 {
device_links_write_lock();
-   __device_links_no_driver(dev);
+   __device_links_probe_failed(dev);
device_links_write_unlock();
 }
 
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index de6fd09..90d57e0 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -492,7 +492,7 @@ static int really_probe(struct device *dev, struct 
device_driver *drv)
blocking_notifier_call_chain(>bus->p->bus_notifier,
 BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
 pinctrl_bind_failed:
-   device_links_no_driver(dev);
+   device_links_probe_failed(dev);
devres_release_all(dev);
driver_sysfs_remove(dev);
dev->driver = NULL;
diff --git a/include/linux/device.h b/include/linux/device.h
index b093405..bf9630a 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -794,6 +794,7 @@ struct device_link {
 enum dl_dev_state {
DL_DEV_NO_DRIVER = 0,
DL_DEV_PROBING,
+   DL_DEV_PROBE_FAILED,
DL_DEV_DRIVER_BOUND,
DL_DEV_UNBINDING,
 };
-- 
2.7.4

[RFC PATCH 0/2] driver core: add new dl device status DL_DEV_PROBE_FAILED

[Wang Dongsheng]

Recently we found the master of SMMU retries to probe endlessly.

[3.658956] pci :00:00.0: Retrying from deferred list
[3.658969] pci :00:00.0: Added to deferred list
[3.658987] pci 0004:00:00.0: Retrying from deferred list
[3.658994] pci 0004:00:00.0: Added to deferred list
[3.659003] pci 0005:00:00.0: Retrying from deferred list
[3.659010] pci 0005:00:00.0: Added to deferred list
[3.659019] pci 0004:01:00.0: Retrying from deferred list
[3.659029] pci 0004:01:00.0: Added to deferred list

The retrying should only happen when the IOMMU instance hasn't been
probed yet.
However, dma_configure() simply return -EPROBE_DEFER when failed to get
an IOMMU instance even if the IOMMU instance is failed to probe.

This patchset tries to fix the issue by distinguishing probe failures and
haven't-been-probed-yet state.

Wang Dongsheng (2):
  driver core: add new dl device status DL_DEV_PROBE_FAILED
  ACPI/IORT: use swiotlb_dma_ops when smmu probe failed

 Documentation/driver-api/device_link.rst |  2 +-
 drivers/acpi/arm64/iort.c| 39 +++-
 drivers/base/base.h  |  2 +-
 drivers/base/core.c  | 22 --
 drivers/base/dd.c|  2 +-
 include/linux/device.h   |  1 +
 6 files changed, 57 insertions(+), 11 deletions(-)

-- 
2.7.4

Re: [RFC PATCH 1/2] driver core: add new dl device status DL_DEV_PROBE_FAILED

[Wang, Dongsheng]

On Thu, 2018-03-29 at 12:51 +0200, Rafael J. Wysocki wrote:
> On Thu, Mar 29, 2018 at 12:00 PM, Wang Dongsheng
>  wrote:
> > Currently the initialization state of device is DL_DEV_NO_DRIVER.
> > The problem is, after probe failure the state will also be set to
> > DL_DEV_NO_DRIVER as well. And the device is not linked, it has no
> > supplier or consumer. Thus adding a new state to distinguish
> > probe failure and not-probed-yet.
> > 
> > Signed-off-by: Wang Dongsheng 
> 
> I guess what you want is a cleanup after a failing probe, but after
> that the state really is "no driver" again, isn't it?
Yes, agree "no driver", device is never get a driver again.

But A depend on B successful probe. if B failed, A will never know:
1. B can't work.
2. B hasn't got a probe yet.

Like IOMMU. After SMMU successful probed, the driver add a resource
into "iommu_device_list". Master lookup the corresponding SMMU fwnode
from "iommu_device_list", after matched master will do probe. but if
the list is NULL master will get -EPROBE_DEFER, means SMMU device may
not probe yet, in fact SMMU may probe failed.

I try to use DL_DEV state to fix this issue, but NO_DRIVER does not
distinguish between the two cases.

Cheers,
-Dongsheng

> 
> > ---
> >  Documentation/driver-api/device_link.rst |  2 +-
> >  drivers/base/base.h  |  2 +-
> >  drivers/base/core.c  | 22
> > --
> >  drivers/base/dd.c|  2 +-
> >  include/linux/device.h   |  1 +
> >  5 files changed, 24 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/driver-api/device_link.rst
> > b/Documentation/driver-api/device_link.rst
> > index 70e328e..9054403 100644
> > --- a/Documentation/driver-api/device_link.rst
> > +++ b/Documentation/driver-api/device_link.rst
> > @@ -247,7 +247,7 @@ State machine
> >:c:func:`device_links_unbind_consumers()`.)
> > 
> >  * If the probe fails, links to suppliers revert back to
> > ``DL_STATE_AVAILABLE``.
> > -  (Call to :c:func:`device_links_no_driver()` from
> > :c:func:`really_probe()`.)
> > +  (Call to :c:func:`device_links_probe_failed()` from
> > :c:func:`really_probe()`.)
> > 
> >  * If the probe succeeds, links to suppliers progress to
> > ``DL_STATE_ACTIVE``.
> >(Call to :c:func:`device_links_driver_bound()` from
> > :c:func:`driver_bound()`.)
> > diff --git a/drivers/base/base.h b/drivers/base/base.h
> > index d800de6..f9931d9 100644
> > --- a/drivers/base/base.h
> > +++ b/drivers/base/base.h
> > @@ -158,6 +158,6 @@ extern void device_links_read_unlock(int idx);
> >  extern int device_links_check_suppliers(struct device *dev);
> >  extern void device_links_driver_bound(struct device *dev);
> >  extern void device_links_driver_cleanup(struct device *dev);
> > -extern void device_links_no_driver(struct device *dev);
> > +extern void device_links_probe_failed(struct device *dev);
> >  extern bool device_links_busy(struct device *dev);
> >  extern void device_links_unbind_consumers(struct device *dev);
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 5847364..31d4f68 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -452,10 +452,28 @@ static void __device_links_no_driver(struct
> > device *dev)
> > dev->links.status = DL_DEV_NO_DRIVER;
> >  }
> > 
> > -void device_links_no_driver(struct device *dev)
> > +static void __device_links_probe_failed(struct device *dev)
> > +{
> > +   struct device_link *link, *ln;
> > +
> > +   list_for_each_entry_safe_reverse(link, ln, 
> > >links.suppliers,
> > +c_node) {
> > +   if (link->flags & DL_FLAG_STATELESS)
> > +   continue;
> > +
> > +   if (link->flags & DL_FLAG_AUTOREMOVE)
> > +   __device_link_del(link);
> > +   else if (link->status != DL_STATE_SUPPLIER_UNBIND)
> > +   WRITE_ONCE(link->status,
> > DL_STATE_AVAILABLE);
> > +   }
> > +
> > +   dev->links.status = DL_DEV_PROBE_FAILED;
> > +}
> > +
> > +void device_links_probe_failed(struct device *dev)
> >  {
> > device_links_write_lock();
> > -   __device_links_no_driver(dev);
> > +   __device_links_probe_failed(dev);
> > device_links_write_unlock();
> >  }
> > 
> > diff --git a/drivers/base/dd.c b/drivers/base

[PATCH] phy: rcar: fix gen3-usb2 build error

[Wang Dongsheng]

of_usb_get_dr_mode_by_phy is implemented in usb/common/common.c.
So the PHY_RCAR_GEN3_USB2 must depend on USB_SUPPORT/USB_COMMON.

 LD  vmlinux.o
 MODPOST vmlinux.o
.../phy-rcar-gen3-usb2.o: In function `rcar_gen3_phy_usb2_probe':
...: undefined reference to `of_usb_get_dr_mode_by_phy'
make: *** [vmlinux] Error 1

Signed-off-by: Wang Dongsheng 
---
 drivers/phy/renesas/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/phy/renesas/Kconfig b/drivers/phy/renesas/Kconfig
index cb09245..c845fac 100644
--- a/drivers/phy/renesas/Kconfig
+++ b/drivers/phy/renesas/Kconfig
@@ -12,7 +12,9 @@ config PHY_RCAR_GEN3_USB2
tristate "Renesas R-Car generation 3 USB 2.0 PHY driver"
depends on ARCH_RENESAS
depends on EXTCON
+   depends on USB_SUPPORT
select GENERIC_PHY
+   select USB_COMMON
help
  Support for USB 2.0 PHY found on Renesas R-Car generation 3 SoCs.
 
-- 
2.7.4

RE: Re: [RFC PATCH 2/2] ACPI/IORT: use swiotlb_dma_ops when smmu probe failed

[Wang, Dongsheng]

> -Original Message-
> From: linux-acpi-ow...@vger.kernel.org
> [mailto:linux-acpi-ow...@vger.kernel.org] On Behalf Of Robin Murphy
> Sent: Monday, April 09, 2018 8:11 PM
> To: Wang, Dongsheng ; Lorenzo Pieralisi
> 
> Cc: r...@rjwysocki.net; gre...@linuxfoundation.org; hanjun@linaro.org;
> sudeep.ho...@arm.com; Zheng, Joey ;
> linux-a...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [此邮件可能存在风险] Re: [RFC PATCH 2/2] ACPI/IORT: use
> swiotlb_dma_ops when smmu probe failed
> 
> On 08/04/18 09:10, Wang, Dongsheng wrote:
> >
> >> -Original Message-
> >> From: Robin Murphy [mailto:robin.mur...@arm.com]
> >> Sent: Thursday, April 05, 2018 2:57 AM
> >> To: Lorenzo Pieralisi ; Wang, Dongsheng
> >> 
> >> Cc: r...@rjwysocki.net; gre...@linuxfoundation.org;
> hanjun@linaro.org;
> >> sudeep.ho...@arm.com; Zheng, Joey ;
> >> linux-a...@vger.kernel.org; linux-kernel@vger.kernel.org
> >> Subject: [此邮件可能存在风险] Re: [RFC PATCH 2/2] ACPI/IORT: use
> >> swiotlb_dma_ops when smmu probe failed
> >>
> >> On 04/04/18 17:01, Lorenzo Pieralisi wrote:
> >>> [+cc Robin]
> >>>
> >>> On Thu, Mar 29, 2018 at 03:01:00AM -0700, Wang Dongsheng wrote:
> >>>> If SMMU probe failed, master should use swiotlb as dma ops.
> >>>> SMMU may probe failed with specified environment, so there
> >>>> are not any iommu resources in iommu_device_list.
> >>>>
> >>>> The master will always get EPROBE_DEFER from really_probe
> >>>> (dma_configure) but in fact SMMU has probe failed. The issue
> >>>> causes all of masters failed to be driven.
> >>
> >> Let's just take a step back - why is SMMU probe failing? That seems to
> >> be the primary issue here, because it implies that either your hardware,
> >> firmware or kernel is broken, any of which would make boot failure
> >> somewhat unsurprising anyway.
> >>
> > It's actually not a hardware issue. This is my test case, just return
> > -EINVAL in arm_smmu_device_probe. The HW
> probe(arm_smmu_device_hw_probe)
> > is just part of SMMU driver probe and the failure may be caused by SW. So
> > I design this case, just make sure even if SMMU probe failed that cause by 
> > SW,
> > the MASTER also can work. _Because of our SMMU default mode is bypass._
> 
> I don't think it's particularly justifiable to make core API changes for
> the sake of contrived testcases. On real systems, the SMMU is a
> fundamental system component which is no more expected to fail probe
> than, say, the GIC, and as such if it *does* fail then further progress
> is on a best-effort basis at most.
Yes. Agree with you.


> Just because *your* system happens to work fine in this state doesn't make it 
> true for every SMMU
> implementation and integration that Linux may ever run on.
:(, yes, this is my mistake.


>   
> If you want the kernel to ignore an SMMU, either configure out the
> driver or don't describe that SMMU in firmware in the first place.
> 
> >>> I added Robin to pick his brain. An alternative would consist
> >>> in using a bus notifier to prevent deferred probing once the SMMU
> >>> driver probing failed but that seems backwards given that a major
> >>> reason to move to deferred probing was to remove the bus notifiers
> >>> dependency in the first place.
> >>>
> >>> It seems to me this is both an OF/ACPI issue - it is not an IORT
> >>> only problem.
> >>
> >> Yes, this is just an instance of the general probe-deferral problem,
> >> e.g. once you have multiple dependencies it's possible to end up in a
> >> stalemate where everything including the IOMMU ends up on the deferred
> >> probe list with nothing to kick it and make progress again.
> >>
> >> Furthermore it seems to me that the whole premise in this patch is
> >> flawed,
> > Ditto. :)
> >
> >
> >> since even genuine probe failure may well be transient - just
> >> because one attempt failed doesn't mean a later attempt can't succeed.
> >> Thus "the most recent probe attempt failed" cannot be considered a
> >> fundamentally different state from "no driver is currently bound".
> >>
> > Agree, the genuine probe failure may well be transient. But there is
> > depend on SMMU probe(IOMMU instance) status. There are two situations:
> >
> > 1. MASTER probing, SMMU doesn't probe yet.
> > This case will match "the trans

RE: Re: [RFC PATCH 2/2] ACPI/IORT: use swiotlb_dma_ops when smmu probe failed

[Wang, Dongsheng]

> -Original Message-
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: Thursday, April 05, 2018 2:57 AM
> To: Lorenzo Pieralisi ; Wang, Dongsheng
> 
> Cc: r...@rjwysocki.net; gre...@linuxfoundation.org; hanjun@linaro.org;
> sudeep.ho...@arm.com; Zheng, Joey ;
> linux-a...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [此邮件可能存在风险] Re: [RFC PATCH 2/2] ACPI/IORT: use
> swiotlb_dma_ops when smmu probe failed
> 
> On 04/04/18 17:01, Lorenzo Pieralisi wrote:
> > [+cc Robin]
> >
> > On Thu, Mar 29, 2018 at 03:01:00AM -0700, Wang Dongsheng wrote:
> >> If SMMU probe failed, master should use swiotlb as dma ops.
> >> SMMU may probe failed with specified environment, so there
> >> are not any iommu resources in iommu_device_list.
> >>
> >> The master will always get EPROBE_DEFER from really_probe
> >> (dma_configure) but in fact SMMU has probe failed. The issue
> >> causes all of masters failed to be driven.
> 
> Let's just take a step back - why is SMMU probe failing? That seems to
> be the primary issue here, because it implies that either your hardware,
> firmware or kernel is broken, any of which would make boot failure
> somewhat unsurprising anyway.
> 
It's actually not a hardware issue. This is my test case, just return
-EINVAL in arm_smmu_device_probe. The HW probe(arm_smmu_device_hw_probe)
is just part of SMMU driver probe and the failure may be caused by SW. So
I design this case, just make sure even if SMMU probe failed that cause by SW,
the MASTER also can work. _Because of our SMMU default mode is bypass._


> > I added Robin to pick his brain. An alternative would consist
> > in using a bus notifier to prevent deferred probing once the SMMU
> > driver probing failed but that seems backwards given that a major
> > reason to move to deferred probing was to remove the bus notifiers
> > dependency in the first place.
> >
> > It seems to me this is both an OF/ACPI issue - it is not an IORT
> > only problem.
> 
> Yes, this is just an instance of the general probe-deferral problem,
> e.g. once you have multiple dependencies it's possible to end up in a
> stalemate where everything including the IOMMU ends up on the deferred
> probe list with nothing to kick it and make progress again.
> 
> Furthermore it seems to me that the whole premise in this patch is
> flawed,
Ditto. :)


> since even genuine probe failure may well be transient - just
> because one attempt failed doesn't mean a later attempt can't succeed.
> Thus "the most recent probe attempt failed" cannot be considered a
> fundamentally different state from "no driver is currently bound".
> 
Agree, the genuine probe failure may well be transient. But there is
depend on SMMU probe(IOMMU instance) status. There are two situations:

1. MASTER probing, SMMU doesn't probe yet.
This case will match "the transient failure".
really_probe get an EPROBE_DEFER from IORT and the MASTER probe will be
delayed until SMMU probe successful.
2. MASTER probing, SMMU probe has failed.
really_probe will always get an EPROBE_DEFER from IORT, because kernel
has build in SMMU driver.(iort_iommu_driver_enabled) And the master
never cannot do probe.

The case 2 is I want to handle.

Cheers,
-Dongsheng

> Robin.
> 
> >
> > Lorenzo
> >
> >> Signed-off-by: Wang Dongsheng 
> >> ---
> >>   drivers/acpi/arm64/iort.c | 39
> +--
> >>   1 file changed, 33 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> >> index e2f7bdd..a6f4c27 100644
> >> --- a/drivers/acpi/arm64/iort.c
> >> +++ b/drivers/acpi/arm64/iort.c
> >> @@ -774,17 +774,45 @@ static int arm_smmu_iort_xlate(struct device
> *dev, u32 streamid,
> >>return ret;
> >>   }
> >>
> >> -static inline bool iort_iommu_driver_enabled(u8 type)
> >> +static int iort_check_dev_dl_status(struct device *dev, void *data)
> >>   {
> >> +  struct fwnode_handle *fwnode = data;
> >> +
> >> +  if (dev->fwnode != fwnode)
> >> +  return 0;
> >> +
> >> +  if (dev->links.status == DL_DEV_PROBE_FAILED)
> >> +  return -ENODEV;
> >> +
> >> +  return -EPROBE_DEFER;
> >> +}
> >> +
> >> +static int iort_iommu_driver_enabled(u8 type, struct fwnode_handle
> *fwnode)
> >> +{
> >> +  bool buildin;
> >> +  int ret;
> >> +
> >>switch (type) {
> >>case ACPI_IORT_NODE_SMMU_V

Re: Re: [PATCH v3] ACPI / tables: Add IORT to injectable table list

[Wang, Dongsheng]

Hey, Hanjun,

On 2018/2/2 19:54:24, "Hanjun Guo"  wrote:

>On 2018/2/2 18:25, Yang Shunyong wrote:
>>Loading IORT table from initrd is used to fix firmware IORT defects.
>
>I don't think this fix "firmware defects", it just for debug purpose,
>we will not use that for production purpose, right? I think above line
>can be removed.
>
I thinks the upgrade feature not only for debug. Here's an important
way to fix bugs that come from the firmware.

Documentation/acpi/initrd_table_override.txt


Cheers,
-Dongsheng

[PATCH v4] net: qcom/emac: extend DMA mask to 46bits

[Wang Dongsheng]

Bit TPD3[31] is used as a timestamp bit if PTP is enabled, but
it's used as an address bit if PTP is disabled.  Since PTP isn't
supported by the driver, we can extend the DMA address to 46 bits.

Signed-off-by: Wang Dongsheng 
---
v4:
- Changes: PATCH's description.
v3:
- Add: comments for TPD_BUFFER_ADDR_H_SET.
- Remove: "Dynamic fix TPD_BUFFER_ADDR_H_SET size."
v2:
- Add: Dynamic fix TPD_BUFFER_ADDR_H_SET size.
- Add: Comments for DMA MASK.
- Changes: PATCH subject.
- Modify: DMA MASK to 46bits.
---
 drivers/net/ethernet/qualcomm/emac/emac-mac.h | 3 ++-
 drivers/net/ethernet/qualcomm/emac/emac.c | 7 +--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.h 
b/drivers/net/ethernet/qualcomm/emac/emac-mac.h
index 5028fb4..4beedb8 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.h
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.h
@@ -114,8 +114,9 @@ struct emac_tpd {
 #define TPD_INSTC_SET(tpd, val)BITS_SET((tpd)->word[3], 17, 
17, val)
 /* High-14bit Buffer Address, So, the 64b-bit address is
  * {DESC_CTRL_11_TX_DATA_HIADDR[17:0],(register) BUFFER_ADDR_H, BUFFER_ADDR_L}
+ * Extend TPD_BUFFER_ADDR_H to [31, 18], because we never enable timestamping.
  */
-#define TPD_BUFFER_ADDR_H_SET(tpd, val)BITS_SET((tpd)->word[3], 18, 
30, val)
+#define TPD_BUFFER_ADDR_H_SET(tpd, val)BITS_SET((tpd)->word[3], 18, 
31, val)
 /* Format D. Word offset from the 1st byte of this packet to start to calculate
  * the custom checksum.
  */
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c 
b/drivers/net/ethernet/qualcomm/emac/emac.c
index 38c924bd..13235ba 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac.c
@@ -615,8 +615,11 @@ static int emac_probe(struct platform_device *pdev)
u32 reg;
int ret;
 
-   /* The TPD buffer address is limited to 45 bits. */
-   ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(45));
+   /* The TPD buffer address is limited to:
+* 1. PTP:  45bits. (Driver doesn't support yet.)
+* 2. NON-PTP:  46bits.
+*/
+   ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(46));
if (ret) {
dev_err(>dev, "could not set DMA mask\n");
return ret;
-- 
2.7.4

Re: Re: [PATCH v4] net: qcom/emac: extend DMA mask to 46bits

[Wang, Dongsheng]

Thanks.

Cheers,
-Dongsheng

On 2018/1/23 12:48:27, "Timur Tabi"  wrote:

>On 1/22/18 10:25 PM, Wang Dongsheng wrote:
>>Bit TPD3[31] is used as a timestamp bit if PTP is enabled, but
>>it's used as an address bit if PTP is disabled.  Since PTP isn't
>>supported by the driver, we can extend the DMA address to 46 bits.
>>
>>Signed-off-by: Wang Dongsheng
>
>Acked-by: Timur Tabi 
>
>--
>Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
>Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
>Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC

[Wang Dongsheng]

When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
is not a real task on stack, it's only init_task on init_stack.

Commit 0500871f21b2 ("Construct init thread stack in the linker script
rather than by union") added this macro and put task_strcut into
thread_union. This brings us the following possibilities:
TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
- <-- thread_info & stack
NN | | --- <-- task
   | ||   |
-  ---

- <-- stack
NY | | --- <-- task(Including 
thread_info)
   | ||   |
-  ---

- <-- stack & task & thread_info
YN | |
   | |
-

- <-- stack & task(Including 
thread_info)
YY | |
   | |
-
The kernel has handled the first two cases correctly.

For the third case:
TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
should never happen, because the task and thread_info will overlap. So
when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.

For the fourth case:
When task on stack, the end of stack should add a sizeof(task_struct) offset.

This patch handled with the third and fourth case.

Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")

Signed-off-by: Wang Dongsheng 
Signed-off-by: Shunyong Yang 
---
 arch/Kconfig | 1 +
 include/linux/sched/task_stack.h | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index e1e540ffa979..0a2c73e73195 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
 # Select if arch init_task must go in the __init_task_data section
 config ARCH_TASK_STRUCT_ON_STACK
bool
+   depends on THREAD_INFO_IN_TASK || IA64
 
 # Select if arch has its private alloc_task_struct() function
 config ARCH_TASK_STRUCT_ALLOCATOR
diff --git a/include/linux/sched/task_stack.h b/include/linux/sched/task_stack.h
index 6a841929073f..624c48defb9e 100644
--- a/include/linux/sched/task_stack.h
+++ b/include/linux/sched/task_stack.h
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 #include 
 
 #ifdef CONFIG_THREAD_INFO_IN_TASK
@@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct task_struct 
*task)
 
 static inline unsigned long *end_of_stack(const struct task_struct *task)
 {
-   return task->stack;
+   if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task != _task)
+   return task->stack;
+   return (unsigned long *)(task + 1);
 }
 
 #elif !defined(__HAVE_THREAD_FUNCTIONS)
-- 
2.19.1

Re: [PATCH 1/1] ACPI / tables: add DSDT AmlCode new declaration name support

[Wang, Dongsheng]

Hello Robert,

Do you have any comments about this patch?
Thanks.


Cheers
Dongsheng

On 2018/11/13 18:46, Wang, Dongsheng wrote:
> The new naming rule is added in acpica version 20180427.
> So the dsdt aml code name changes from "AmlCode" to "dsdt_aml_code".
>
> The patch that introduces naming rules is:
> https://github.com/acpica/acpica/commit/f9a88a4c1cd020b6a5475d63b29626852a0b5f37
>
> Tested:
> ACPICA release version 20180427+.
> ARM64: QCOM QDF2400
> GCC: 4.8.5 20150623
>
> Signed-off-by: Wang Dongsheng 
> ---
>  drivers/acpi/Kconfig  |  2 +-
>  drivers/acpi/tables.c | 10 --
>  2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 9705fc986da9..15ab53a52fdc 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -336,7 +336,7 @@ config ACPI_CUSTOM_DSDT_FILE
> See Documentation/acpi/dsdt-override.txt
>  
> Enter the full path name to the file which includes the AmlCode
> -   declaration.
> +   or dsdt_aml_code declaration.
>  
> If unsure, don't enter a file name.
>  
> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> index a3d012b08fc5..297020bbaade 100644
> --- a/drivers/acpi/tables.c
> +++ b/drivers/acpi/tables.c
> @@ -713,6 +713,9 @@ acpi_os_physical_table_override(struct acpi_table_header 
> *existing_table,
> table_length);
>  }
>  
> +static void *amlcode __attribute__ ((weakref("AmlCode")));
> +static void *dsdt_amlcode __attribute__ ((weakref("dsdt_aml_code")));
> +
>  acpi_status
>  acpi_os_table_override(struct acpi_table_header *existing_table,
>  struct acpi_table_header **new_table)
> @@ -723,8 +726,11 @@ acpi_os_table_override(struct acpi_table_header 
> *existing_table,
>   *new_table = NULL;
>  
>  #ifdef CONFIG_ACPI_CUSTOM_DSDT
> - if (strncmp(existing_table->signature, "DSDT", 4) == 0)
> - *new_table = (struct acpi_table_header *)AmlCode;
> + if (!strncmp(existing_table->signature, "DSDT", 4)) {
> + *new_table = (struct acpi_table_header *)
> + if (!(*new_table))
> + *new_table = (struct acpi_table_header *)_amlcode;
> + }
>  #endif
>   if (*new_table != NULL)
>   acpi_table_taint(existing_table);

Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC

[Wang, Dongsheng]

Hello Kees,

On 2018/11/28 6:38, Kees Cook wrote:
> On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
>  wrote:
>> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
>> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
>> is not a real task on stack, it's only init_task on init_stack.
>>
>> Commit 0500871f21b2 ("Construct init thread stack in the linker script
>> rather than by union") added this macro and put task_strcut into
>> thread_union. This brings us the following possibilities:
>> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
>> - <-- thread_info & stack
>> NN | | --- <-- task
>>| ||   |
>> -  ---
>>
>> - <-- stack
>> NY | | --- <-- 
>> task(Including thread_info)
>>| ||   |
>> -  ---
>>
>> - <-- stack & task & 
>> thread_info
>> YN | |
>>| |
>> -
>>
>> - <-- stack & task(Including 
>> thread_info)
>> YY | |
>>| |
>> -
>> The kernel has handled the first two cases correctly.
>>
>> For the third case:
>> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
>> should never happen, because the task and thread_info will overlap. So
>> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>>
>> For the fourth case:
>> When task on stack, the end of stack should add a sizeof(task_struct) offset.
>>
>> This patch handled with the third and fourth case.
>>
>> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>>
>> Signed-off-by: Wang Dongsheng 
>> Signed-off-by: Shunyong Yang 
>> ---
>>  arch/Kconfig | 1 +
>>  include/linux/sched/task_stack.h | 5 -
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index e1e540ffa979..0a2c73e73195 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
>>  # Select if arch init_task must go in the __init_task_data section
>>  config ARCH_TASK_STRUCT_ON_STACK
>> bool
>> +   depends on THREAD_INFO_IN_TASK || IA64
> The "IA64" part shouldn't be needed since IA64 already selects it.
>
> Since it's selected, it also can't have a depends, IIUC.

Since the IA64 thread_info including task_struct, it doesn't need to
select THREAD_INFO_IN_TASK.
So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
THREAD_INFO.

>>  # Select if arch has its private alloc_task_struct() function
>>  config ARCH_TASK_STRUCT_ALLOCATOR
>> diff --git a/include/linux/sched/task_stack.h 
>> b/include/linux/sched/task_stack.h
>> index 6a841929073f..624c48defb9e 100644
>> --- a/include/linux/sched/task_stack.h
>> +++ b/include/linux/sched/task_stack.h
>> @@ -7,6 +7,7 @@
>>   */
>>
>>  #include 
>> +#include 
>>  #include 
>>
>>  #ifdef CONFIG_THREAD_INFO_IN_TASK
>> @@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct 
>> task_struct *task)
>>
>>  static inline unsigned long *end_of_stack(const struct task_struct *task)
>>  {
>> -   return task->stack;
>> +   if (!IS_ENABLED(CONFIG_ARCH_TASK_STRUCT_ON_STACK) || task != 
>> _task)
>> +   return task->stack;
>> +   return (unsigned long *)(task + 1);
>>  }
> This seems like a strange place for the change. It feels more like
> init_task has been defined incorrectly.

The init_task will put into init_stack when ARCH_TASK_STRUCT_ON_STACK is
selected.
include/asm-generic/vmlinux.lds.h:
#define INIT_TASK_DATA(align)\
. = ALIGN(align);\
__start_init_task = .;\
init_thread_union = .;\
init_stack = .;\
KEEP(*(.data..init_task))\
KEEP(*(.data..init_thread_info))\
. = __start_init_task + THREAD_SIZE;\
__end_init_task = .;

So we need end_of_stack to offset sizeof(task_struct).

Cheers,
Dongsheng

Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC

[Wang, Dongsheng]

On 2018/11/30 5:22, Kees Cook wrote:
> On Tue, Nov 27, 2018 at 8:38 PM Wang, Dongsheng
>  wrote:
>> Hello Kees,
>>
>> On 2018/11/28 6:38, Kees Cook wrote:
>>> On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
>>>  wrote:
>>>> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
>>>> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
>>>> is not a real task on stack, it's only init_task on init_stack.
>>>>
>>>> Commit 0500871f21b2 ("Construct init thread stack in the linker script
>>>> rather than by union") added this macro and put task_strcut into
>>>> thread_union. This brings us the following possibilities:
>>>> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
>>>> - <-- thread_info & stack
>>>> NN | | --- <-- task
>>>>| ||   |
>>>> -  ---
>>>>
>>>> - <-- stack
>>>> NY | | --- <-- 
>>>> task(Including thread_info)
>>>>| ||   |
>>>> -  ---
>>>>
>>>> - <-- stack & task & 
>>>> thread_info
>>>> YN | |
>>>>| |
>>>> -
>>>>
>>>> - <-- stack & 
>>>> task(Including thread_info)
>>>> YY | |
>>>>| |
>>>> -
>>>> The kernel has handled the first two cases correctly.
>>>>
>>>> For the third case:
>>>> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
>>>> should never happen, because the task and thread_info will overlap. So
>>>> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>>>>
>>>> For the fourth case:
>>>> When task on stack, the end of stack should add a sizeof(task_struct) 
>>>> offset.
>>>>
>>>> This patch handled with the third and fourth case.
>>>>
>>>> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>>>>
>>>> Signed-off-by: Wang Dongsheng 
>>>> Signed-off-by: Shunyong Yang 
>>>> ---
>>>>  arch/Kconfig | 1 +
>>>>  include/linux/sched/task_stack.h | 5 -
>>>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/Kconfig b/arch/Kconfig
>>>> index e1e540ffa979..0a2c73e73195 100644
>>>> --- a/arch/Kconfig
>>>> +++ b/arch/Kconfig
>>>> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
>>>>  # Select if arch init_task must go in the __init_task_data section
>>>>  config ARCH_TASK_STRUCT_ON_STACK
>>>> bool
>>>> +   depends on THREAD_INFO_IN_TASK || IA64
>>> The "IA64" part shouldn't be needed since IA64 already selects it.
>>>
>>> Since it's selected, it also can't have a depends, IIUC.
>> Since the IA64 thread_info including task_struct, it doesn't need to
>> select THREAD_INFO_IN_TASK.
>> So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
>> THREAD_INFO.
> Okay.
>
>>>>  # Select if arch has its private alloc_task_struct() function
>>>>  config ARCH_TASK_STRUCT_ALLOCATOR
>>>> diff --git a/include/linux/sched/task_stack.h 
>>>> b/include/linux/sched/task_stack.h
>>>> index 6a841929073f..624c48defb9e 100644
>>>> --- a/include/linux/sched/task_stack.h
>>>> +++ b/include/linux/sched/task_stack.h
>>>> @@ -7,6 +7,7 @@
>>>>   */
>>>>
>>>>  #include 
>>>> +#include 
>>>>  #include 
>>>>
>>>>  #ifdef CONFIG_THREAD_INFO_IN_TASK
>>>> @@ -25,7 +26,9 @@ static inline void *task_stack_page(const struct 
>>>> task_struct *task)

Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC

[Wang, Dongsheng]

On 2018/11/30 10:04, Wang, Dongsheng wrote:
> On 2018/11/30 5:22, Kees Cook wrote:
>> On Tue, Nov 27, 2018 at 8:38 PM Wang, Dongsheng
>>  wrote:
>>> Hello Kees,
>>>
>>> On 2018/11/28 6:38, Kees Cook wrote:
>>>> On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
>>>>  wrote:
>>>>> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
>>>>> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
>>>>> is not a real task on stack, it's only init_task on init_stack.
>>>>>
>>>>> Commit 0500871f21b2 ("Construct init thread stack in the linker script
>>>>> rather than by union") added this macro and put task_strcut into
>>>>> thread_union. This brings us the following possibilities:
>>>>> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
>>>>> - <-- thread_info & stack
>>>>> NN | | --- <-- task
>>>>>| ||   |
>>>>> -  ---
>>>>>
>>>>> - <-- stack
>>>>> NY | | --- <-- 
>>>>> task(Including thread_info)
>>>>>| ||   |
>>>>> -  ---
>>>>>
>>>>> - <-- stack & task & 
>>>>> thread_info
>>>>> YN | |
>>>>>| |
>>>>> -
>>>>>
>>>>> - <-- stack & 
>>>>> task(Including thread_info)
>>>>> YY | |
>>>>>| |
>>>>> -
>>>>> The kernel has handled the first two cases correctly.
>>>>>
>>>>> For the third case:
>>>>> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
>>>>> should never happen, because the task and thread_info will overlap. So
>>>>> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>>>>>
>>>>> For the fourth case:
>>>>> When task on stack, the end of stack should add a sizeof(task_struct) 
>>>>> offset.
>>>>>
>>>>> This patch handled with the third and fourth case.
>>>>>
>>>>> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>>>>>
>>>>> Signed-off-by: Wang Dongsheng 
>>>>> Signed-off-by: Shunyong Yang 
>>>>> ---
>>>>>  arch/Kconfig | 1 +
>>>>>  include/linux/sched/task_stack.h | 5 -
>>>>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/Kconfig b/arch/Kconfig
>>>>> index e1e540ffa979..0a2c73e73195 100644
>>>>> --- a/arch/Kconfig
>>>>> +++ b/arch/Kconfig
>>>>> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
>>>>>  # Select if arch init_task must go in the __init_task_data section
>>>>>  config ARCH_TASK_STRUCT_ON_STACK
>>>>> bool
>>>>> +   depends on THREAD_INFO_IN_TASK || IA64
>>>> The "IA64" part shouldn't be needed since IA64 already selects it.
>>>>
>>>> Since it's selected, it also can't have a depends, IIUC.
>>> Since the IA64 thread_info including task_struct, it doesn't need to
>>> select THREAD_INFO_IN_TASK.
>>> So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
>>> THREAD_INFO.
>> Okay.
>>
>>>>>  # Select if arch has its private alloc_task_struct() function
>>>>>  config ARCH_TASK_STRUCT_ALLOCATOR
>>>>> diff --git a/include/linux/sched/task_stack.h 
>>>>> b/include/linux/sched/task_stack.h
>>>>> index 6a841929073f..624c48defb9e 100644
>>>>> --- a/include/linux/sched/task_stack

[PATCH 1/1] ACPI / tables: add DSDT AmlCode new declaration name support

[Wang Dongsheng]

The new naming rule is added in acpica version 20180427.
So the dsdt aml code name changes from "AmlCode" to "dsdt_aml_code".

The patch that introduces naming rules is:
https://github.com/acpica/acpica/commit/f9a88a4c1cd020b6a5475d63b29626852a0b5f37

Tested:
ACPICA release version 20180427+.
ARM64: QCOM QDF2400
GCC: 4.8.5 20150623

Signed-off-by: Wang Dongsheng 
---
 drivers/acpi/Kconfig  |  2 +-
 drivers/acpi/tables.c | 10 --
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 9705fc986da9..15ab53a52fdc 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -336,7 +336,7 @@ config ACPI_CUSTOM_DSDT_FILE
  See Documentation/acpi/dsdt-override.txt
 
  Enter the full path name to the file which includes the AmlCode
- declaration.
+ or dsdt_aml_code declaration.
 
  If unsure, don't enter a file name.
 
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index a3d012b08fc5..297020bbaade 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -713,6 +713,9 @@ acpi_os_physical_table_override(struct acpi_table_header 
*existing_table,
  table_length);
 }
 
+static void *amlcode __attribute__ ((weakref("AmlCode")));
+static void *dsdt_amlcode __attribute__ ((weakref("dsdt_aml_code")));
+
 acpi_status
 acpi_os_table_override(struct acpi_table_header *existing_table,
   struct acpi_table_header **new_table)
@@ -723,8 +726,11 @@ acpi_os_table_override(struct acpi_table_header 
*existing_table,
*new_table = NULL;
 
 #ifdef CONFIG_ACPI_CUSTOM_DSDT
-   if (strncmp(existing_table->signature, "DSDT", 4) == 0)
-   *new_table = (struct acpi_table_header *)AmlCode;
+   if (!strncmp(existing_table->signature, "DSDT", 4)) {
+   *new_table = (struct acpi_table_header *)
+   if (!(*new_table))
+   *new_table = (struct acpi_table_header *)_amlcode;
+   }
 #endif
if (*new_table != NULL)
acpi_table_taint(existing_table);
-- 
2.18.0

Re: [PATCH 1/1] sched/headers: fix thread_info. is overwritten by STACK_END_MAGIC

[Wang, Dongsheng]

Hello all,

Any comments about this patch?

Cheers,
Dongsheng

On 2018/11/30 10:19, Wang, Dongsheng wrote:
> On 2018/11/30 10:04, Wang, Dongsheng wrote:
>> On 2018/11/30 5:22, Kees Cook wrote:
>>> On Tue, Nov 27, 2018 at 8:38 PM Wang, Dongsheng
>>>  wrote:
>>>> Hello Kees,
>>>>
>>>> On 2018/11/28 6:38, Kees Cook wrote:
>>>>> On Thu, Nov 22, 2018 at 11:54 PM, Wang Dongsheng
>>>>>  wrote:
>>>>>> When select ARCH_TASK_STRUCT_ON_STACK the first of thread_info variable
>>>>>> is overwritten by STACK_END_MAGIC. In fact, the ARCH_TASK_STRUCT_ON_STACK
>>>>>> is not a real task on stack, it's only init_task on init_stack.
>>>>>>
>>>>>> Commit 0500871f21b2 ("Construct init thread stack in the linker script
>>>>>> rather than by union") added this macro and put task_strcut into
>>>>>> thread_union. This brings us the following possibilities:
>>>>>> TASK_ON_STACKTHREAD_INFO_IN_TASKSTACK
>>>>>> - <-- thread_info & stack
>>>>>> NN | | --- <-- task
>>>>>>| ||   |
>>>>>> -  ---
>>>>>>
>>>>>> - <-- stack
>>>>>> NY | | --- <-- 
>>>>>> task(Including thread_info)
>>>>>>| ||   |
>>>>>> -  ---
>>>>>>
>>>>>> - <-- stack & task & 
>>>>>> thread_info
>>>>>> YN | |
>>>>>>| |
>>>>>> -
>>>>>>
>>>>>> - <-- stack & 
>>>>>> task(Including thread_info)
>>>>>> YY | |
>>>>>>| |
>>>>>>     -
>>>>>> The kernel has handled the first two cases correctly.
>>>>>>
>>>>>> For the third case:
>>>>>> TASK_ON_STACK: Y. THREAD_INFO_IN_TASK: N. this case
>>>>>> should never happen, because the task and thread_info will overlap. So
>>>>>> when TASK_ON_STACK is selected, THREAD_INFO_IN_TASK must be selected too.
>>>>>>
>>>>>> For the fourth case:
>>>>>> When task on stack, the end of stack should add a sizeof(task_struct) 
>>>>>> offset.
>>>>>>
>>>>>> This patch handled with the third and fourth case.
>>>>>>
>>>>>> Fixes: 0500871f21b2 ("Construct init thread stack in the linker ...")
>>>>>>
>>>>>> Signed-off-by: Wang Dongsheng 
>>>>>> Signed-off-by: Shunyong Yang 
>>>>>> ---
>>>>>>  arch/Kconfig | 1 +
>>>>>>  include/linux/sched/task_stack.h | 5 -
>>>>>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/arch/Kconfig b/arch/Kconfig
>>>>>> index e1e540ffa979..0a2c73e73195 100644
>>>>>> --- a/arch/Kconfig
>>>>>> +++ b/arch/Kconfig
>>>>>> @@ -251,6 +251,7 @@ config ARCH_HAS_SET_MEMORY
>>>>>>  # Select if arch init_task must go in the __init_task_data section
>>>>>>  config ARCH_TASK_STRUCT_ON_STACK
>>>>>> bool
>>>>>> +   depends on THREAD_INFO_IN_TASK || IA64
>>>>> The "IA64" part shouldn't be needed since IA64 already selects it.
>>>>>
>>>>> Since it's selected, it also can't have a depends, IIUC.
>>>> Since the IA64 thread_info including task_struct, it doesn't need to
>>>> select THREAD_INFO_IN_TASK.
>>>> So we need to allow IA64 select ARCH_TASK_STRUCT_ON_STACK without
>>>>

Re: [PATCH 1/1] ACPI / tables: add DSDT AmlCode new declaration name support

[Wang, Dongsheng]

Hello all,

Any comments about this patch?

Cheers
Dongsheng

On 2018/11/23 16:12, Wang, Dongsheng wrote:
> Hello Robert,
>
> Do you have any comments about this patch?
> Thanks.
>
>
> Cheers
> Dongsheng
>
> On 2018/11/13 18:46, Wang, Dongsheng wrote:
>> The new naming rule is added in acpica version 20180427.
>> So the dsdt aml code name changes from "AmlCode" to "dsdt_aml_code".
>>
>> The patch that introduces naming rules is:
>> https://github.com/acpica/acpica/commit/f9a88a4c1cd020b6a5475d63b29626852a0b5f37
>>
>> Tested:
>> ACPICA release version 20180427+.
>> ARM64: QCOM QDF2400
>> GCC: 4.8.5 20150623
>>
>> Signed-off-by: Wang Dongsheng 
>> ---
>>  drivers/acpi/Kconfig  |  2 +-
>>  drivers/acpi/tables.c | 10 --
>>  2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 9705fc986da9..15ab53a52fdc 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -336,7 +336,7 @@ config ACPI_CUSTOM_DSDT_FILE
>>See Documentation/acpi/dsdt-override.txt
>>  
>>Enter the full path name to the file which includes the AmlCode
>> -  declaration.
>> +  or dsdt_aml_code declaration.
>>  
>>If unsure, don't enter a file name.
>>  
>> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
>> index a3d012b08fc5..297020bbaade 100644
>> --- a/drivers/acpi/tables.c
>> +++ b/drivers/acpi/tables.c
>> @@ -713,6 +713,9 @@ acpi_os_physical_table_override(struct acpi_table_header 
>> *existing_table,
>>table_length);
>>  }
>>  
>> +static void *amlcode __attribute__ ((weakref("AmlCode")));
>> +static void *dsdt_amlcode __attribute__ ((weakref("dsdt_aml_code")));
>> +
>>  acpi_status
>>  acpi_os_table_override(struct acpi_table_header *existing_table,
>> struct acpi_table_header **new_table)
>> @@ -723,8 +726,11 @@ acpi_os_table_override(struct acpi_table_header 
>> *existing_table,
>>  *new_table = NULL;
>>  
>>  #ifdef CONFIG_ACPI_CUSTOM_DSDT
>> -if (strncmp(existing_table->signature, "DSDT", 4) == 0)
>> -*new_table = (struct acpi_table_header *)AmlCode;
>> +if (!strncmp(existing_table->signature, "DSDT", 4)) {
>> +*new_table = (struct acpi_table_header *)
>> +if (!(*new_table))
>> +*new_table = (struct acpi_table_header *)_amlcode;
>> +}
>>  #endif
>>  if (*new_table != NULL)
>>  acpi_table_taint(existing_table);
>
>

Re: [PATCH v6 09/11] mmc: sdhci-acpi: Make PCI dependency explicit

[Wang, Dongsheng]

On 2019/1/7 20:43, Adrian Hunter wrote:
> On 7/01/19 1:17 PM, Rafael J. Wysocki wrote:
>> On Sat, Jan 5, 2019 at 11:06 AM Sinan Kaya  wrote:
>>> After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
>>> CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
>>> satisfied implicitly through dependencies on CONFIG_ACPI have to be
>>> specified directly. This driver relies on IOSF_MBI and IOSF_MBI depends
>>> on PCI. For this reason, add a direct dependency to CONFIG_PCI here.
>>>
>>> Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI 
>>> set")
>>> Signed-off-by: Sinan Kaya 
>>> ---
>>>  drivers/mmc/host/Kconfig | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>>> index e26b8145efb3..1b9401fe94c0 100644
>>> --- a/drivers/mmc/host/Kconfig
>>> +++ b/drivers/mmc/host/Kconfig
>>> @@ -116,7 +116,7 @@ config MMC_RICOH_MMC
>>>
>>>  config MMC_SDHCI_ACPI
>>> tristate "SDHCI support for ACPI enumerated SDHCI controllers"
>>> -   depends on MMC_SDHCI && ACPI
>>> +   depends on MMC_SDHCI && ACPI && PCI
>>> select IOSF_MBI if X86
>>> help
>>>   This selects support for ACPI enumerated SDHCI controllers,
>>> --
>> Any objections against this one from anyone?
> + QCOM and AMD contributors
>
No issue with QCOM.

Cheers
Dongsheng

Re: [PATCH v3 1/2] net: ethernet: i40e: fix build error

[Wang, Dongsheng]

Hello all,

The i40e_ethtool_stats.h is just included by i40e/i40e_ethtool.c. So the
static doesn't make any affect. And Carolyn's team is working rebuild
i40e and i40evf.

Cheers,
Dongsheng

On 9/7/2018 7:19 PM, Wang, Dongsheng wrote:
> Remove "inline" from __i40e_add_stat_strings and move the function.
>
> In file included from
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c:9:0:
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function
> __i40e_add_stat_string:
> drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h:193:20: error:
> function __i40e_add_stat_strings can never be inlined because it uses
> variable argument lists
>  static inline void __i40e_add_stat_strings(u8 **p, const struct
>   i40e_stats stats[],
>
> Tested on: x86_64, make ARCH=i386
>
> Modules section .text:
> i40e: 00019380 <__i40e_add_stat_strings>:
> i40evf: 6b00 <__i40e_add_stat_strings>:
>
> Buildin section .text:
> i40e: c351ca60 <__i40e_add_stat_strings>:
> i40evf: c354f2c0 <__i40e_add_stat_strings>:
>
> Signed-off-by: Wang Dongsheng 
> ---
> V3: add static 
> V2: Move function
> ---
>  .../net/ethernet/intel/i40e/i40e_ethtool.c| 24 ++
>  .../ethernet/intel/i40e/i40e_ethtool_stats.h  | 25 ++-
>  2 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c 
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index d7d3974beca2..34121a72f2e7 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -1821,6 +1821,30 @@ static void i40e_get_ethtool_stats(struct net_device 
> *netdev,
> "ethtool stats count mismatch!");
>  }
>  
> +/**
> + * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> + * @p: ethtool supplied buffer
> + * @stats: stat definitions array
> + * @size: size of the stats array
> + *
> + * Format and copy the strings described by stats into the buffer pointed at
> + * by p.
> + **/
> +static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> + const unsigned int size, ...)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < size; i++) {
> + va_list args;
> +
> + va_start(args, size);
> + vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> + *p += ETH_GSTRING_LEN;
> + va_end(args);
> + }
> +}
> +
>  /**
>   * i40e_get_stat_strings - copy stat strings into supplied buffer
>   * @netdev: the netdev to collect strings for
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h 
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> index bba1cb0b658f..553b0d720839 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> @@ -181,29 +181,8 @@ i40e_add_queue_stats(u64 **data, struct i40e_ring *ring)
>   *data += size;
>  }
>  
> -/**
> - * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> - * @p: ethtool supplied buffer
> - * @stats: stat definitions array
> - * @size: size of the stats array
> - *
> - * Format and copy the strings described by stats into the buffer pointed at
> - * by p.
> - **/
> -static inline void __i40e_add_stat_strings(u8 **p, const struct i40e_stats 
> stats[],
> - const unsigned int size, ...)
> -{
> - unsigned int i;
> -
> - for (i = 0; i < size; i++) {
> - va_list args;
> -
> - va_start(args, size);
> - vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> - *p += ETH_GSTRING_LEN;
> - va_end(args);
> - }
> -}
> +static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> + const unsigned int size, ...);
>  
>  /**
>   * 40e_add_stat_strings - copy stat strings into ethtool buffer

RE: [PATCH v2] ARM: dts: ls1021a: add wakeup device ftm0 node for ls1021a

[Wang Dongsheng]

Hi Shawn,

> -Original Message-
> From: Shawn Guo [mailto:shawn...@kernel.org]
> Sent: Tuesday, July 28, 2015 9:50 AM
> To: Wang Huan-B18965
> Cc: shawn@linaro.org; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; Wang Dongsheng-B40534; Wang Huan-B18965
> Subject: Re: [PATCH v2] ARM: dts: ls1021a: add wakeup device ftm0 node for
> ls1021a
> 
> On Wed, Jul 15, 2015 at 03:40:11PM +0800, Alison Wang wrote:
> > Add ftm0 node, cause of ftm0 can be set as a alarm before system going
> > to deep sleep.
> >
> > Signed-off-by: Wang Dongsheng 
> > Signed-off-by: Alison Wang 
> > ---
> > Changes since v1:
> > - Add my SoB.
> > - Use "ARM:" as subject prefix.
> >
> >  arch/arm/boot/dts/ls1021a-qds.dts | 4 
> >  arch/arm/boot/dts/ls1021a.dtsi| 8 
> >  2 files changed, 12 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/ls1021a-qds.dts
> > b/arch/arm/boot/dts/ls1021a-qds.dts
> > index 9c5e16b..f14731b 100644
> > --- a/arch/arm/boot/dts/ls1021a-qds.dts
> > +++ b/arch/arm/boot/dts/ls1021a-qds.dts
> > @@ -75,6 +75,10 @@
> > };
> >  };
> >
> > + {
> > +   status = "okay";
> > +};
> > +
> >   {
> > status = "okay";
> >
> > diff --git a/arch/arm/boot/dts/ls1021a.dtsi
> > b/arch/arm/boot/dts/ls1021a.dtsi index c70bb27..7e9e122 100644
> > --- a/arch/arm/boot/dts/ls1021a.dtsi
> > +++ b/arch/arm/boot/dts/ls1021a.dtsi
> > @@ -332,6 +332,14 @@
> > status = "disabled";
> > };
> >
> > +   ftm0: ftm0@29d {
> 
> Is the '0' in the node name is an instance number?  If so, please drop it from
> node name.

"0" is a special number for FTM, because there is only ftm0 can be maked as a 
alarm timer.

Regards,
-Dongsheng

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] rtc/ds3232: fix ds3232 get a WARNING trace in resume function

[Wang Dongsheng]

Thanks Belloni. :)

Regards,
-Dongsheng

> -Original Message-
> From: Alexandre Belloni [mailto:alexandre.bell...@free-electrons.com]
> Sent: Wednesday, July 15, 2015 6:51 AM
> To: Wang Dongsheng-B40534
> Cc: a.zu...@towertech.it; rtc-li...@googlegroups.com; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH] rtc/ds3232: fix ds3232 get a WARNING trace in resume
> function
> 
> Hi,
> 
> This seems ok, one small nitpick:
> 
> On 07/07/2015 at 14:12:56 +0800, Dongsheng Wang wrote :
> > From: Wang Dongsheng  diff --git
> > a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c index
> > 7e48e53..2081155 100644
> > --- a/drivers/rtc/rtc-ds3232.c
> > +++ b/drivers/rtc/rtc-ds3232.c
> > @@ -463,7 +463,10 @@ static int ds3232_suspend(struct device *dev)
> >
> > if (device_can_wakeup(dev)) {
> > ds3232->suspended = true;
> > -   irq_set_irq_wake(client->irq, 1);
> > +   if (irq_set_irq_wake(client->irq, 1)) {
> > +   dev_info(dev, "Cannot serve as a wakeup source\n");
> 
> I would use dev_warn_once or dev_info_once here to avoid spamming the log each
> time the machine is suspended.
> 
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform

[Wang Dongsheng]

> -Original Message-
> From: Linus Walleij [mailto:linus.wall...@linaro.org]
> Sent: Thursday, August 13, 2015 9:54 PM
> To: Wang Dongsheng-B40534; John Stultz; Alessandro Zummo; Alexandre Belloni
> Cc: Shawn Guo; Nair, Sandeep; Hans de Goede; Wang Huan-B18965; linux-arm-
> ker...@lists.infradead.org; linux-kernel@vger.kernel.org; rtc-
> li...@googlegroups.com
> Subject: Re: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform
> 
> On Wed, Aug 12, 2015 at 7:53 AM, Dongsheng Wang
>  wrote:
> 
> > From: Wang Dongsheng 
> >
> > Only Ftm0 can be used when system going to deep sleep. So this driver
> > to support ftm0 as a wakeup source.
> >
> > Signed-off-by: Wang Dongsheng 
> > ---
> > *V2*
> > Change Copyright 2014 to 2015.
> (...)
> > +config FTM_ALARM
> > +   bool "FTM alarm driver"
> > +   depends on SOC_LS1021A
> > +   default n
> > +   help
> > + Say y here to enable FTM alarm support.  The FTM alarm provides
> > + alarm functions for wakeup system from deep sleep.  There is only
> > + one FTM can be used in ALARM(FTM 0).
> (...)
> > +static u32 time_to_cycle(unsigned long time)
> > +static u32 cycle_to_time(u32 cycle)
> > +static int ftm_set_alarm(u64 cycle)
> > +static irqreturn_t ftm_alarm_interrupt(int irq, void *dev_id)
> > +static ssize_t ftm_alarm_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +static ssize_t ftm_alarm_store(struct device *dev,
> > +  struct device_attribute *attr,
> > +  const char *buf, size_t count)
> (...)
> > +static struct device_attribute ftm_alarm_attributes = __ATTR(ftm_alarm, 
> > 0644,
> > +   ftm_alarm_show, ftm_alarm_store);
> 
> If you're gonna invent ABIs, document then in Documentation/ABI/testing/*.
> 
> But I don't get it. Why is this driver not in drivers/rtc?
> 
> It does a subset of what an RTC does. The ioctl()'s of an RTC
> can do what you want to do. And much much more.
> 
> If it can't do all an RTC can do, surely the RTC subsystem
> can be augmented to host it anyway. It's way to close to
> an RTC to have it's own random sysfs driver like this.
> 
> Unless I'm totally off, rewrite this to an RTC driver and post
> it to the RTC maintainers.
> 

FlexTimer is not a RTC device and not have any rtc deivce function. They belong 
to
different devices, why we need to register this to RTC framework? I am confused 
about this.

Now in freescale layerscape platform this driver is only for FlexTimer0, and not
fit for each flextimer. Because only FlexTimer0 still turn-on when system in 
the Deep Sleep.

If the "alarm" make you feel confused or mislead you think this is a RTC 
devices. I think
I need to change the "alarm" to "timer".

Regards,
-Dongsheng

RE: [PATCH v3 1/2] Documentation: DT: FTM: add FTM0 be used as alarm timer

[Wang Dongsheng]

Thank Yoder,

Fix them in next version.

Regards,
-Dongsheng

> -Original Message-
> From: Stuart Yoder [mailto:b08...@gmail.com]
> Sent: Friday, August 14, 2015 12:23 AM
> To: Wang Dongsheng-B40534
> Cc: shawn@linaro.org; devicet...@vger.kernel.org; linux-
> ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Wang Huan-B18965
> Subject: Re: [PATCH v3 1/2] Documentation: DT: FTM: add FTM0 be used as alarm
> timer
> 
> On Tue, Aug 11, 2015 at 11:01 PM, Dongsheng Wang
>  wrote:
> > From: Wang Dongsheng 
> >
> > In freescale layerscape platform there is only FTM0 can be used as
> > alarm timer to wake up system. So add FTM0 description for devicetree
> > document.
> 
> Suggestion:
> 
> In the Freescale Layerscape platform a flextimer module can be used
> as an alarm timer to wake up the system.  Define a binding for this
> alarm.
> 
> > Signed-off-by: Wang Dongsheng 
> > ---
> > V3:
> > Include this patch in V3.
> >
> > diff --git a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt
> b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt
> > index aa8c402..380a0b3d 100644
> > --- a/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt
> > +++ b/Documentation/devicetree/bindings/timer/fsl,ftm-timer.txt
> > @@ -1,5 +1,7 @@
> >  Freescale FlexTimer Module (FTM) Timer
> >
> > +* Default FTM Timer
> > +
> >  Required properties:
> >
> >  - compatible : should be "fsl,ftm-timer"
> > @@ -29,3 +31,26 @@ ftm: ftm@400b8000 {
> > < VF610_CLK_FTM3_EXT_FIX_EN>;
> > big-endian;
> >  };
> > +
> > +* FTM Alarm Timer
> > +  The default FTM device contains eight FlexTimer modules. FlexTimer1 is in
> > +  on-domain(power is not switched off in deep sleep mode). Other seven
> FlexTimer
> > +  modules(flextimer2/3/4/5/6/7/8) are in off-domain (power is switched off 
> > in
> > +  deep sleep mode).
> 
> Suggestion:
> 
>   This binding describes a Flextimer (FTM) instance that can be used as an 
> alarm
>   to wake up a system in deep sleep.  The flextimer module with alarm
>   support is in a power domain that remains awake when the SoC is in
>   deep sleep mode.
> 
> (don't use flextimer1 in the binding...for all you know in the next SoC it
> will be flextimer7 that has this capability)
> 
> > +Required properties:
> > +
> > +- compatible : should be "fsl,ftm-alarm".
> > +- reg : should contain base address and length of FTM timer 0 register.
> 
>   "should contain the address and size of the FTM alarm module registers"
> 
> (don't use the number 0)
> 
> > +- interrupts : Should contain FTM 0 interrupt.
> 
>"describes the FTM alarm interrupt"
> 
> > +- big-endian: One boolean property, the big endian mode will be in use if 
> > it
> is
> > +  present, or the little endian mode will be in use for all the device
> registers.
> > +
> > +Example:
> > +ftm0: ftm0@29d {
> > +   compatible = "fsl,ftm-alarm";
> > +   reg = <0x0 0x29d 0x0 0x1>;
> > +   interrupts = ;
> > +   big-endian;
> > +   status = "disabled";
> > +};
> 
> Drop the number 0 from the names in your example.
> 
> Thanks,
> Stuart
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH v2] rtc/ds3232: fix ds3232 get a WARNING trace in resume function

[Wang Dongsheng]

Hi Alexandre,

Could you apply this patch?

Regards,
-Dongsheng

> -Original Message-
> From: Dongsheng Wang [mailto:dongsheng.w...@freescale.com]
> Sent: Wednesday, August 12, 2015 5:14 PM
> To: alexandre.bell...@free-electrons.com
> Cc: a.zu...@towertech.it; rtc-li...@googlegroups.com; linux-
> ker...@vger.kernel.org; Wang Dongsheng-B40534
> Subject: [PATCH v2] rtc/ds3232: fix ds3232 get a WARNING trace in resume
> function
> 
> From: Wang Dongsheng 
> 
> If ds3232 work on some platform that is not implementation
> irq_set_wake, ds3232 will get a WARNING trace in resume.
> So fix ds3232->suspended state to false when irq_set_irq_wake
> return error.
> 
> [ cut here ]
> WARNING: CPU: 0 PID: 729 at kernel/irq/manage.c:604
> irq_set_irq_wake+0x4b/0x8c()
> Unbalanced IRQ 201 wake disable
> Modules linked in:
> CPU: 0 PID: 729 Comm: sh Not tainted 3.12.19-rt30+ #25
> [<800107d9>] (unwind_backtrace+0x1/0x88) from [<8000e4ef>]
> (show_stack+0xb/0xc)
> [<8000e4ef>] (show_stack+0xb/0xc) from [<802b5fa9>]
> (dump_stack+0x4d/0x60)
> [<802b5fa9>] (dump_stack+0x4d/0x60) from [<800186dd>]
> (warn_slowpath_common+0x45/0x64)
> [<800186dd>] (warn_slowpath_common+0x45/0x64) from [<80018717>]
> (warn_slowpath_fmt+0x1b/0x24)
> [<80018717>] (warn_slowpath_fmt+0x1b/0x24) from [<8003a8d3>]
> (irq_set_irq_wake+0x4b/0x8c)
> [<8003a8d3>] (irq_set_irq_wake+0x4b/0x8c) from [<80204fcb>]
> (ds3232_resume+0x2d/0x36)
> [<80204fcb>] (ds3232_resume+0x2d/0x36) from [<801954c7>]
> (dpm_run_callback.isra.13+0xb/0x28)
> [<801954c7>] (dpm_run_callback.isra.13+0xb/0x28) from [<80195b1b>]
> (device_resume+0x7b/0xa2)
> [<80195b1b>] (device_resume+0x7b/0xa2) from [<80195f0f>]
> (dpm_resume+0xbb/0x19c)
> [<80195f0f>] (dpm_resume+0xbb/0x19c) from [<801960d9>]
> (dpm_resume_end+0x9/0x12)
> [<801960d9>] (dpm_resume_end+0x9/0x12) from [<80037e1d>]
> (suspend_devices_and_enter+0x17d/0x1d0)
> [<80037e1d>] (suspend_devices_and_enter+0x17d/0x1d0) from [<80037ee1>]
> (pm_suspend+0x71/0x128)
> [<80037ee1>] (pm_suspend+0x71/0x128) from [<80037449>]
> (state_store+0x6d/0x80)
> [<80037449>] (state_store+0x6d/0x80) from [<800af4d5>]
> (sysfs_write_file+0x9f/0xde)
> [<800af4d5>] (sysfs_write_file+0x9f/0xde) from [<8007a437>]
> (vfs_write+0x7b/0x104)
> [<8007a437>] (vfs_write+0x7b/0x104) from [<8007a7f7>]
> (SyS_write+0x27/0x48)
> [<8007a7f7>] (SyS_write+0x27/0x48) from [<8000c121>]
> (ret_fast_syscall+0x1/0x44)
> ---[ end trace 640959d2e8de6ccc ]---
> 
> Signed-off-by: Wang Dongsheng 
> ---
> *v2*
> - Use dev_warn_once to instead of dev_info
> 
> diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
> index 7e48e53..3554970 100644
> --- a/drivers/rtc/rtc-ds3232.c
> +++ b/drivers/rtc/rtc-ds3232.c
> @@ -463,7 +463,10 @@ static int ds3232_suspend(struct device *dev)
> 
>   if (device_can_wakeup(dev)) {
>   ds3232->suspended = true;
> - irq_set_irq_wake(client->irq, 1);
> + if (irq_set_irq_wake(client->irq, 1)) {
> + dev_warn_once(dev, "Can not set wakeup sources\n");
> + ds3232->suspended = false;
> + }
>   }
> 
>   return 0;
> --
> 2.1.0.27.g96db324

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] rtc/ds3232: fix ds3232 get a WARNING trace in resume function

[Wang Dongsheng]

Hi Belloni,

I am not found this patch in your tree(rtc-fixes and rtc-next), Need I send v2 
patch? :)

Regards,
-Dongsheng

> -Original Message-
> From: Wang Dongsheng-B40534
> Sent: Wednesday, July 15, 2015 10:06 AM
> To: 'Alexandre Belloni'
> Cc: a.zu...@towertech.it; rtc-li...@googlegroups.com; linux-
> ker...@vger.kernel.org
> Subject: RE: [PATCH] rtc/ds3232: fix ds3232 get a WARNING trace in resume
> function
> 
> Thanks Belloni. :)
> 
> Regards,
> -Dongsheng
> 
> > -Original Message-
> > From: Alexandre Belloni [mailto:alexandre.bell...@free-electrons.com]
> > Sent: Wednesday, July 15, 2015 6:51 AM
> > To: Wang Dongsheng-B40534
> > Cc: a.zu...@towertech.it; rtc-li...@googlegroups.com; linux-
> > ker...@vger.kernel.org
> > Subject: Re: [PATCH] rtc/ds3232: fix ds3232 get a WARNING trace in
> > resume function
> >
> > Hi,
> >
> > This seems ok, one small nitpick:
> >
> > On 07/07/2015 at 14:12:56 +0800, Dongsheng Wang wrote :
> > > From: Wang Dongsheng  diff --git
> > > a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c index
> > > 7e48e53..2081155 100644
> > > --- a/drivers/rtc/rtc-ds3232.c
> > > +++ b/drivers/rtc/rtc-ds3232.c
> > > @@ -463,7 +463,10 @@ static int ds3232_suspend(struct device *dev)
> > >
> > >   if (device_can_wakeup(dev)) {
> > >   ds3232->suspended = true;
> > > - irq_set_irq_wake(client->irq, 1);
> > > + if (irq_set_irq_wake(client->irq, 1)) {
> > > + dev_info(dev, "Cannot serve as a wakeup source\n");
> >
> > I would use dev_warn_once or dev_info_once here to avoid spamming the
> > log each time the machine is suspended.
> >
> > --
> > Alexandre Belloni, Free Electrons
> > Embedded Linux, Kernel and Android engineering
> > http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v2 1/2] soc/fsl: add freescale dir for SOC specific drivers

[Wang Dongsheng]

Hi Russell,

Thanks for your review. :)

> -Original Message-
> From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk]
> Sent: Wednesday, August 12, 2015 3:45 PM
> To: Wang Dongsheng-B40534
> Cc: shawn@linaro.org; Wang Huan-B18965; linus.wall...@linaro.org; linux-
> ker...@vger.kernel.org; sandee...@ti.com; hdego...@redhat.com; linux-arm-
> ker...@lists.infradead.org
> Subject: Re: [PATCH v2 1/2] soc/fsl: add freescale dir for SOC specific 
> drivers
> 
> On Wed, Aug 12, 2015 at 01:53:26PM +0800, Dongsheng Wang wrote:
> > diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig new
> > file mode 100644 index 000..863d1ef
> > --- /dev/null
> > +++ b/drivers/soc/fsl/Kconfig
> > @@ -0,0 +1,18 @@
> > +#
> > +# Freescale SOC drivers
> > +#
> > +menuconfig FSL_SOC_DRIVERS
> > +   bool "Freescale Soc Drivers"
> > +   default n
> 
> No need for default n.

Thanks. Fix it in next version.

> > +   help
> > +   Say y here to enable Freescale LS1021A Soc Device Drivers support.
> > +   The Soc Drivers provides the device driver that is a specific block
> > +   or feature on LS1021A platform.
> 
> Help text should be indented by two spaces as per almost every other help 
> text.
> 

Thanks. My mistake...

> > +
> > +if LS1_SOC_DRIVERS
> > +   source "drivers/soc/fsl/ls1/Kconfig"
> > +endif
> > diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile new
> > file mode 100644 index 000..b4215dd
> > --- /dev/null
> > +++ b/drivers/soc/fsl/Makefile
> > @@ -0,0 +1,6 @@
> > +#
> > +# Makefile for Freescale Soc specific device drivers.
> > +#
> > +
> > +obj-$(CONFIG_LS1_SOC_DRIVERS) += ls1/
> > +
> > diff --git a/drivers/soc/fsl/ls1/Kconfig b/drivers/soc/fsl/ls1/Kconfig
> > new file mode 100644 index 000..7556f44
> > --- /dev/null
> > +++ b/drivers/soc/fsl/ls1/Kconfig
> > @@ -0,0 +1,3 @@
> > +#
> > +# LS-1 Soc drivers
> > +#
> 
> Doesn't this directory need a Makefile as well?
> 

2/2 patch add a config option and Makefile for this.

Regards,
-Dongsheng
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v2 1/2] soc/fsl: add freescale dir for SOC specific drivers

[Wang Dongsheng]

> -Original Message-
> From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk]
> Sent: Wednesday, August 12, 2015 4:04 PM
> To: Wang Dongsheng-B40534
> Cc: shawn@linaro.org; Wang Huan-B18965; linus.wall...@linaro.org; linux-
> ker...@vger.kernel.org; sandee...@ti.com; hdego...@redhat.com; linux-arm-
> ker...@lists.infradead.org
> Subject: Re: [PATCH v2 1/2] soc/fsl: add freescale dir for SOC specific 
> drivers
> 
> On Wed, Aug 12, 2015 at 08:01:32AM +, Wang Dongsheng wrote:
> > Hi Russell,
> >
> > Thanks for your review. :)
> >
> > > -Original Message-
> > > From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk]
> > > Sent: Wednesday, August 12, 2015 3:45 PM
> > > To: Wang Dongsheng-B40534
> > > Cc: shawn@linaro.org; Wang Huan-B18965;
> > > linus.wall...@linaro.org; linux- ker...@vger.kernel.org;
> > > sandee...@ti.com; hdego...@redhat.com; linux-arm-
> > > ker...@lists.infradead.org
> > > Subject: Re: [PATCH v2 1/2] soc/fsl: add freescale dir for SOC
> > > specific drivers
> > >
> > > On Wed, Aug 12, 2015 at 01:53:26PM +0800, Dongsheng Wang wrote:
> > > > diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig new
> > > > file mode 100644 index 000..863d1ef
> > > > --- /dev/null
> > > > +++ b/drivers/soc/fsl/Kconfig
> > > > @@ -0,0 +1,18 @@
> > > > +#
> > > > +# Freescale SOC drivers
> > > > +#
> > > > +menuconfig FSL_SOC_DRIVERS
> > > > +   bool "Freescale Soc Drivers"
> > > > +   default n
> > >
> > > No need for default n.
> >
> > Thanks. Fix it in next version.
> >
> > > > +   help
> > > > +   Say y here to enable Freescale LS1021A Soc Device Drivers 
> > > > support.
> > > > +   The Soc Drivers provides the device driver that is a specific 
> > > > block
> > > > +   or feature on LS1021A platform.
> > >
> > > Help text should be indented by two spaces as per almost every other help
> text.
> > >
> >
> > Thanks. My mistake...
> >
> > > > +
> > > > +if LS1_SOC_DRIVERS
> > > > +   source "drivers/soc/fsl/ls1/Kconfig"
> > > > +endif
> > > > diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
> > > > new file mode 100644 index 000..b4215dd
> > > > --- /dev/null
> > > > +++ b/drivers/soc/fsl/Makefile
> > > > @@ -0,0 +1,6 @@
> > > > +#
> > > > +# Makefile for Freescale Soc specific device drivers.
> > > > +#
> > > > +
> > > > +obj-$(CONFIG_LS1_SOC_DRIVERS) += ls1/
> > > > +
> > > > diff --git a/drivers/soc/fsl/ls1/Kconfig
> > > > b/drivers/soc/fsl/ls1/Kconfig new file mode 100644 index
> > > > 000..7556f44
> > > > --- /dev/null
> > > > +++ b/drivers/soc/fsl/ls1/Kconfig
> > > > @@ -0,0 +1,3 @@
> > > > +#
> > > > +# LS-1 Soc drivers
> > > > +#
> > >
> > > Doesn't this directory need a Makefile as well?
> > >
> >
> > 2/2 patch add a config option and Makefile for this.
> 
> If _just_ this patch is merged, it creates a build problem as
> CONFIG_LS1_SOC_DRIVERS can be enabled, which will cause the kbuild to decend
> into drivers/soc/fsl/ls1, where it will stop due to the missing build error.
> 
> Please fix this by adding at least an empty Makefile to this directory.
> Do not rely on patch 2 being merged to "fix" this.
> 

Um..Yes, miss it. Fix it in next version. :)

Regards,
-Dongsheng

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v2] rtc/ds3232: fix ds3232 get a WARNING trace in resume function

[Wang Dongsheng]

Thanks, I will push v3 to fix them.

Regards,
-Dongsheng

> -Original Message-
> From: Alexandre Belloni [mailto:alexandre.bell...@free-electrons.com]
> Sent: Friday, August 21, 2015 7:22 AM
> To: Wang Dongsheng-B40534; Krzysztof Kozlowski
> Cc: a.zu...@towertech.it; rtc-li...@googlegroups.com; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH v2] rtc/ds3232: fix ds3232 get a WARNING trace in resume
> function
> 
> On 12/08/2015 at 17:14:13 +0800, Dongsheng Wang wrote :
> > From: Wang Dongsheng 
> >
> > If ds3232 work on some platform that is not implementation
> > irq_set_wake, ds3232 will get a WARNING trace in resume.
> > So fix ds3232->suspended state to false when irq_set_irq_wake return
> > error.
> >
> > [ cut here ]
> > WARNING: CPU: 0 PID: 729 at kernel/irq/manage.c:604
> > irq_set_irq_wake+0x4b/0x8c()
> > Unbalanced IRQ 201 wake disable
> > Modules linked in:
> > CPU: 0 PID: 729 Comm: sh Not tainted 3.12.19-rt30+ #25 [<800107d9>]
> > (unwind_backtrace+0x1/0x88) from [<8000e4ef>]
> > (show_stack+0xb/0xc)
> > [<8000e4ef>] (show_stack+0xb/0xc) from [<802b5fa9>]
> > (dump_stack+0x4d/0x60)
> > [<802b5fa9>] (dump_stack+0x4d/0x60) from [<800186dd>]
> > (warn_slowpath_common+0x45/0x64)
> > [<800186dd>] (warn_slowpath_common+0x45/0x64) from [<80018717>]
> > (warn_slowpath_fmt+0x1b/0x24)
> > [<80018717>] (warn_slowpath_fmt+0x1b/0x24) from [<8003a8d3>]
> > (irq_set_irq_wake+0x4b/0x8c)
> > [<8003a8d3>] (irq_set_irq_wake+0x4b/0x8c) from [<80204fcb>]
> > (ds3232_resume+0x2d/0x36)
> > [<80204fcb>] (ds3232_resume+0x2d/0x36) from [<801954c7>]
> > (dpm_run_callback.isra.13+0xb/0x28)
> > [<801954c7>] (dpm_run_callback.isra.13+0xb/0x28) from [<80195b1b>]
> > (device_resume+0x7b/0xa2)
> > [<80195b1b>] (device_resume+0x7b/0xa2) from [<80195f0f>]
> > (dpm_resume+0xbb/0x19c)
> > [<80195f0f>] (dpm_resume+0xbb/0x19c) from [<801960d9>]
> > (dpm_resume_end+0x9/0x12)
> > [<801960d9>] (dpm_resume_end+0x9/0x12) from [<80037e1d>]
> > (suspend_devices_and_enter+0x17d/0x1d0)
> > [<80037e1d>] (suspend_devices_and_enter+0x17d/0x1d0) from [<80037ee1>]
> > (pm_suspend+0x71/0x128)
> > [<80037ee1>] (pm_suspend+0x71/0x128) from [<80037449>]
> > (state_store+0x6d/0x80)
> > [<80037449>] (state_store+0x6d/0x80) from [<800af4d5>]
> > (sysfs_write_file+0x9f/0xde)
> > [<800af4d5>] (sysfs_write_file+0x9f/0xde) from [<8007a437>]
> > (vfs_write+0x7b/0x104)
> > [<8007a437>] (vfs_write+0x7b/0x104) from [<8007a7f7>]
> > (SyS_write+0x27/0x48)
> > [<8007a7f7>] (SyS_write+0x27/0x48) from [<8000c121>]
> > (ret_fast_syscall+0x1/0x44)
> > ---[ end trace 640959d2e8de6ccc ]---
> >
> > Signed-off-by: Wang Dongsheng 
> > ---
> > *v2*
> > - Use dev_warn_once to instead of dev_info
> >
> 
> Applied, after fixing the kernel trace and the message like suggested by
> Krzysztof.
> 
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform

[Wang Dongsheng]

Hi Walleij and Russell,

I will drop this patch. Thanks for your review.

[PATCH v2 1/2] soc/fsl: add freescale dir for SOC specific drivers

But the 1/2 of the patches also need, because there has another patch(Freescale 
FPGA driver) need 1/2 patch.
Need I push the 1/2 patch with another patches(Freescale FPGA driver) or push 
1/2 standalone without another
patch? 

Regards,
-Dongsheng

> -Original Message-
> From: Linus Walleij [mailto:linus.wall...@linaro.org]
> Sent: Friday, August 14, 2015 6:07 PM
> To: Wang Dongsheng-B40534; John Stultz
> Cc: Alessandro Zummo; Alexandre Belloni; Shawn Guo; Nair, Sandeep; Hans de 
> Goede;
> Wang Huan-B18965; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; rtc-li...@googlegroups.com
> Subject: Re: [PATCH 2/2] soc/fsl: add ftm alarm driver for ls1021a platform
> 
> On Fri, Aug 14, 2015 at 5:12 AM, Wang Dongsheng
>  wrote:
> >> On Wed, Aug 12, 2015 at 7:53 AM, Dongsheng Wang
> >>  wrote:
> >>
> >> > From: Wang Dongsheng 
> >> >
> >> > Only Ftm0 can be used when system going to deep sleep. So this driver
> >> > to support ftm0 as a wakeup source.
> >> >
> >> > Signed-off-by: Wang Dongsheng 
> >> > ---
> >> > *V2*
> >> > Change Copyright 2014 to 2015.
> >> (...)
> >> > +config FTM_ALARM
> >> > +   bool "FTM alarm driver"
> >> > +   depends on SOC_LS1021A
> >> > +   default n
> >> > +   help
> >> > + Say y here to enable FTM alarm support.  The FTM alarm provides
> >> > + alarm functions for wakeup system from deep sleep.  There is 
> >> > only
> >> > + one FTM can be used in ALARM(FTM 0).
> >> (...)
> >> > +static u32 time_to_cycle(unsigned long time)
> >> > +static u32 cycle_to_time(u32 cycle)
> >> > +static int ftm_set_alarm(u64 cycle)
> >> > +static irqreturn_t ftm_alarm_interrupt(int irq, void *dev_id)
> >> > +static ssize_t ftm_alarm_show(struct device *dev,
> >> > + struct device_attribute *attr,
> >> > + char *buf)
> >> > +static ssize_t ftm_alarm_store(struct device *dev,
> >> > +  struct device_attribute *attr,
> >> > +  const char *buf, size_t count)
> >> (...)
> >> > +static struct device_attribute ftm_alarm_attributes = __ATTR(ftm_alarm,
> 0644,
> >> > +   ftm_alarm_show, ftm_alarm_store);
> >>
> >> If you're gonna invent ABIs, document then in Documentation/ABI/testing/*.
> >>
> >> But I don't get it. Why is this driver not in drivers/rtc?
> >>
> >> It does a subset of what an RTC does. The ioctl()'s of an RTC
> >> can do what you want to do. And much much more.
> >>
> >> If it can't do all an RTC can do, surely the RTC subsystem
> >> can be augmented to host it anyway. It's way to close to
> >> an RTC to have it's own random sysfs driver like this.
> >>
> >> Unless I'm totally off, rewrite this to an RTC driver and post
> >> it to the RTC maintainers.
> >
> > FlexTimer is not a RTC device and not have any rtc deivce function. They
> belong to
> > different devices, why we need to register this to RTC framework? I am
> confused about this.
> >
> > Now in freescale layerscape platform this driver is only for FlexTimer0, and
> not
> > fit for each flextimer. Because only FlexTimer0 still turn-on when system in
> the Deep Sleep.
> >
> > If the "alarm" make you feel confused or mislead you think this is a RTC
> devices. I think
> > I need to change the "alarm" to "timer".
> 
> I think it is an RTC, it is just that the hardware engineer
> designed it with a wakeup usecase in mind and did not call
> it an RTC. Wakeup is one of the things RTCs do.
> 
> If you inspect a few drivers in drivers/rtc such as drivers/rtc/rtc-pl030.c
> you will find that they are just as crude as this "alarm" thing.
> 
> It has a counter that counts cycles, it has a comparator
> and an alarm function. It is an on-chip RTC, just like PL030
> no matter what the datasheet or hardware engineer thinks it
> should be called, the Linux kernel calls this an RTC, and it
> has a subsystem for handling it, so we should use it and
> not invent random new stuff.
> 
> If the hardware is really so strange that the counter can only
> be started if you also put an alar

RE: [PATCH 04/17] powerpc: mpic: use IRQCHIP_SKIP_SET_WAKE instead of redundant mpic_irq_set_wake

[Wang Dongsheng]

> -Original Message-
> From: Wood Scott-B07421
> Sent: Wednesday, September 23, 2015 7:50 AM
> To: Sudeep Holla
> Cc: linux...@vger.kernel.org; linux-kernel@vger.kernel.org; Thomas Gleixner;
> Rafael J. Wysocki; Benjamin Herrenschmidt; Paul Mackerras; Michael Ellerman; 
> Jia
> Hongtao-B38951; Marc Zyngier; linuxppc-...@lists.ozlabs.org; Wang Dongsheng-
> B40534
> Subject: Re: [PATCH 04/17] powerpc: mpic: use IRQCHIP_SKIP_SET_WAKE instead of
> redundant mpic_irq_set_wake
> 
> On Mon, 2015-09-21 at 16:47 +0100, Sudeep Holla wrote:
> > mpic_irq_set_wake return -ENXIO for non FSL MPIC and sets IRQF_NO_SUSPEND
> > flag for FSL ones. enable_irq_wake already returns -ENXIO if irq_set_wak
> > is not implemented. Also there's no need to set the IRQF_NO_SUSPEND flag
> > as it doesn't guarantee wakeup for that interrupt.
> >

Non-freescale return -ENXIO, is there any issue? If non-freescale platform does
not support it, but IPs still use enable/disable_irq_wake, we should return a 
error number.

IRQCHIP_SKIP_SET_WAKE just skip this feature, this is not our expected.
If non-freescale platform need this flag to skip this feature, it should be add
in self platform.

@Scott:
If set this flag we cannot keep a irq as a wakeup source when system going to
SUSPEND or MEM.

irq_set_wake() means we can set this irq as a wake source.
IRQCHIP_SKIP_SET_WAKE is ignore irq_set_wake() feature.

Regards,
-Dongsheng

RE: [PATCH 04/17] powerpc: mpic: use IRQCHIP_SKIP_SET_WAKE instead of redundant mpic_irq_set_wake

[Wang Dongsheng]

> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel-
> ow...@vger.kernel.org] On Behalf Of Thomas Gleixner
> Sent: Wednesday, September 23, 2015 11:49 AM
> To: Wang Dongsheng-B40534
> Cc: Wood Scott-B07421; Sudeep Holla; linux...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Rafael J. Wysocki; Benjamin Herrenschmidt; Paul
> Mackerras; Michael Ellerman; Jia Hongtao-B38951; Marc Zyngier; linuxppc-
> d...@lists.ozlabs.org
> Subject: RE: [PATCH 04/17] powerpc: mpic: use IRQCHIP_SKIP_SET_WAKE instead of
> redundant mpic_irq_set_wake
> 
> On Wed, 23 Sep 2015, Wang Dongsheng wrote:
> > > On Mon, 2015-09-21 at 16:47 +0100, Sudeep Holla wrote:
> > > > mpic_irq_set_wake return -ENXIO for non FSL MPIC and sets 
> > > > IRQF_NO_SUSPEND
> > > > flag for FSL ones. enable_irq_wake already returns -ENXIO if irq_set_wak
> > > > is not implemented. Also there's no need to set the IRQF_NO_SUSPEND flag
> > > > as it doesn't guarantee wakeup for that interrupt.
> > > >
> >
> > Non-freescale return -ENXIO, is there any issue? If non-freescale
> > platform does not support it, but IPs still use
> > enable/disable_irq_wake, we should return a error number.
> 
> You can just set IRQCHIP_SKIP_SET_WAKE for FSL chips and not for the
> others.
> 
> > @Scott:
> > If set this flag we cannot keep a irq as a wakeup source when system going 
> > to
> > SUSPEND or MEM.
> >
> > irq_set_wake() means we can set this irq as a wake source.
> > IRQCHIP_SKIP_SET_WAKE is ignore irq_set_wake() feature.
> 
> Nonsense. IRQCHIP_SKIP_SET_WAKE merily tells the core not to bail on
> !chip->irq_set_wake(), but its still marking the interrupt as wakeup
> source and therefor not masking it on suspend.
> 

Sorry, I just check irq_set_irq_wake() code, right, IRQCHIP_SKIP_SET_WAKE also 
can
going to irqd_set to mask IRQD_WAKEUP_STATE.

Yes, this flag just skip the irq_set_wake() not this feature.

Regards,
-Dongsheng

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH V5 3/5] POWER/cpuidle: Generic IBM-POWER backend cpuidle driver.

[Wang Dongsheng-B40534]

 diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
 index 0e2cd5c..e805dcd 100644
 --- a/drivers/cpuidle/Kconfig
 +++ b/drivers/cpuidle/Kconfig

Maybe drivers/cpuidle/Kconfig.powerpc is better? Like arm.

 +obj-$(CONFIG_CPU_IDLE_IBM_POWER) += cpuidle-ibm-power.o
 diff --git a/drivers/cpuidle/cpuidle-ibm-power.c
 b/drivers/cpuidle/cpuidle-ibm-power.c
 new file mode 100644
 index 000..4ee5a94
 --- /dev/null
 +++ b/drivers/cpuidle/cpuidle-ibm-power.c
 @@ -0,0 +1,304 @@
 +/*
 + *  cpuidle-ibm-power - idle state cpuidle driver.
 + *  Adapted from drivers/idle/intel_idle.c and
 + *  drivers/acpi/processor_idle.c
 + *
 + */
 +
 +#include linux/kernel.h
 +#include linux/module.h
 +#include linux/init.h
 +#include linux/moduleparam.h
 +#include linux/cpuidle.h
 +#include linux/cpu.h
 +#include linux/notifier.h
 +
 +#include asm/paca.h
 +#include asm/reg.h
 +#include asm/machdep.h
 +#include asm/firmware.h
 +#include asm/runlatch.h
 +#include asm/plpar_wrappers.h
 +
 +struct cpuidle_driver power_idle_driver = {
 + .name = IBM-POWER-Idle,
 + .owner= THIS_MODULE,
 +};
 +
 +#define MAX_IDLE_STATE_COUNT 2
 +
 +static int max_idle_state = MAX_IDLE_STATE_COUNT - 1;

Again, do not use the macro.

 +static struct cpuidle_state *cpuidle_state_table;
 +
 +static inline void idle_loop_prolog(unsigned long *in_purr)
 +{
 + *in_purr = mfspr(SPRN_PURR);
 + /*
 +  * Indicate to the HV that we are idle. Now would be
 +  * a good time to find other work to dispatch.
 +  */
 + get_lppaca()-idle = 1;
 +}
 +
 +static inline void idle_loop_epilog(unsigned long in_purr)
 +{
 + get_lppaca()-wait_state_cycles += mfspr(SPRN_PURR) - in_purr;
 + get_lppaca()-idle = 0;
 +}
 +
 +static int snooze_loop(struct cpuidle_device *dev,
 + struct cpuidle_driver *drv,
 + int index)
 +{
 + unsigned long in_purr;
 +
 + idle_loop_prolog(in_purr);
 + local_irq_enable();

snooze_loop has already registered in cpuidle framework to handle snooze state.
where disable the irq? Why do enable here?

 +/*
 + * States for dedicated partition case.
 + */
 +static struct cpuidle_state dedicated_states[MAX_IDLE_STATE_COUNT] = {
 + { /* Snooze */
 + .name = snooze,
 + .desc = snooze,
 + .flags = CPUIDLE_FLAG_TIME_VALID,
 + .exit_latency = 0,
 + .target_residency = 0,
 + .enter = snooze_loop },
 + { /* CEDE */
 + .name = CEDE,
 + .desc = CEDE,
 + .flags = CPUIDLE_FLAG_TIME_VALID,
 + .exit_latency = 10,
 + .target_residency = 100,
 + .enter = dedicated_cede_loop },
 +};
 +
 +/*
 + * States for shared partition case.
 + */
 +static struct cpuidle_state shared_states[MAX_IDLE_STATE_COUNT] = {
 + { /* Shared Cede */
 + .name = Shared Cede,
 + .desc = Shared Cede,
 + .flags = CPUIDLE_FLAG_TIME_VALID,
 + .exit_latency = 0,
 + .target_residency = 0,
 + .enter = shared_cede_loop },
 +};
 +
 +static void __exit power_processor_idle_exit(void)
 +{
 +
 + unregister_cpu_notifier(setup_hotplug_notifier);

Remove a blank line.

 + cpuidle_unregister(power_idle_driver);
 + return;
 +}
 +
 +module_init(power_processor_idle_init);
 +module_exit(power_processor_idle_exit);
 +

Did you have tested the module? If not tested, please don't use the module.

 +MODULE_AUTHOR(Deepthi Dharwar deep...@linux.vnet.ibm.com);
 +MODULE_DESCRIPTION(Cpuidle driver for IBM POWER platforms);
 +MODULE_LICENSE(GPL);
 

RE: [PATCH V2 4/6] cpuidle/pseries: Move the pseries_idle backend driver to sysdev.

[Wang Dongsheng-B40534]

 -Original Message-
 From: Deepthi Dharwar [mailto:deep...@linux.vnet.ibm.com]
 Sent: Wednesday, July 31, 2013 10:59 AM
 To: b...@kernel.crashing.org; daniel.lezc...@linaro.org; linux-
 ker...@vger.kernel.org; mich...@ellerman.id.au;
 srivatsa.b...@linux.vnet.ibm.com; pre...@linux.vnet.ibm.com;
 sva...@linux.vnet.ibm.com; linuxppc-...@lists.ozlabs.org
 Cc: r...@sisk.pl; Wang Dongsheng-B40534; linux...@vger.kernel.org
 Subject: [PATCH V2 4/6] cpuidle/pseries: Move the pseries_idle backend
 driver to sysdev.
 
 Move pseries_idle backend driver code to arch/powerpc/sysdev
 so that the code can be used for a common driver for powernv
 and pseries. This removes a lot of code duplicacy.
 
Why not drivers/cpuidle/?

I think it should be move to drivers/cpuidle.

-dongsheng

 Signed-off-by: Deepthi Dharwar deep...@linux.vnet.ibm.com
 ---
  arch/powerpc/platforms/pseries/Kconfig  |9 -
  arch/powerpc/platforms/pseries/Makefile |1
  arch/powerpc/platforms/pseries/processor_idle.c |  384 -
 --
  arch/powerpc/sysdev/Kconfig |9 +
  arch/powerpc/sysdev/Makefile|1
  arch/powerpc/sysdev/processor_idle.c|  384
 +++
  6 files changed, 394 insertions(+), 394 deletions(-)
  delete mode 100644 arch/powerpc/platforms/pseries/processor_idle.c
  create mode 100644 arch/powerpc/sysdev/processor_idle.c
 
 diff --git a/arch/powerpc/platforms/pseries/Kconfig
 b/arch/powerpc/platforms/pseries/Kconfig
 index 62b4f80..bb59bb0 100644
 --- a/arch/powerpc/platforms/pseries/Kconfig
 +++ b/arch/powerpc/platforms/pseries/Kconfig
 @@ -119,12 +119,3 @@ config DTL
 which are accessible through a debugfs file.
 
 Say N if you are unsure.
 -
 -config PSERIES_IDLE
 - bool Cpuidle driver for pSeries platforms
 - depends on CPU_IDLE
 - depends on PPC_PSERIES
 - default y
 - help
 -   Select this option to enable processor idle state management
 -   through cpuidle subsystem.
 diff --git a/arch/powerpc/platforms/pseries/Makefile
 b/arch/powerpc/platforms/pseries/Makefile
 index 8ae0103..4b22379 100644
 --- a/arch/powerpc/platforms/pseries/Makefile
 +++ b/arch/powerpc/platforms/pseries/Makefile
 @@ -21,7 +21,6 @@ obj-$(CONFIG_HCALL_STATS)   += hvCall_inst.o
  obj-$(CONFIG_CMM)+= cmm.o
  obj-$(CONFIG_DTL)+= dtl.o
  obj-$(CONFIG_IO_EVENT_IRQ)   += io_event_irq.o
 -obj-$(CONFIG_PSERIES_IDLE)   += processor_idle.o
 
  ifeq ($(CONFIG_PPC_PSERIES),y)
  obj-$(CONFIG_SUSPEND)+= suspend.o
 diff --git a/arch/powerpc/platforms/pseries/processor_idle.c
 b/arch/powerpc/platforms/pseries/processor_idle.c
 deleted file mode 100644
 index 0d75a54..000
 --- a/arch/powerpc/platforms/pseries/processor_idle.c
 +++ /dev/null
 @@ -1,384 +0,0 @@
 -/*
 - *  processor_idle - idle state cpuidle driver.
 - *  Adapted from drivers/idle/intel_idle.c and
 - *  drivers/acpi/processor_idle.c
 - *
 - */
 -
 -#include linux/kernel.h
 -#include linux/module.h
 -#include linux/init.h
 -#include linux/moduleparam.h
 -#include linux/cpuidle.h
 -#include linux/cpu.h
 -#include linux/notifier.h
 -
 -#include asm/paca.h
 -#include asm/reg.h
 -#include asm/machdep.h
 -#include asm/firmware.h
 -#include asm/runlatch.h
 -#include asm/plpar_wrappers.h
 -
 -/* Snooze Delay, pseries_idle */
 -DECLARE_PER_CPU(long, smt_snooze_delay);
 -
 -struct cpuidle_driver pseries_idle_driver = {
 - .name = pseries_idle,
 - .owner= THIS_MODULE,
 -};
 -
 -#define MAX_IDLE_STATE_COUNT 2
 -
 -static int max_idle_state = MAX_IDLE_STATE_COUNT - 1;
 -static struct cpuidle_device __percpu *pseries_cpuidle_devices;
 -static struct cpuidle_state *cpuidle_state_table;
 -
 -static inline void idle_loop_prolog(unsigned long *in_purr)
 -{
 - *in_purr = mfspr(SPRN_PURR);
 - /*
 -  * Indicate to the HV that we are idle. Now would be
 -  * a good time to find other work to dispatch.
 -  */
 - get_lppaca()-idle = 1;
 -}
 -
 -static inline void idle_loop_epilog(unsigned long in_purr)
 -{
 - get_lppaca()-wait_state_cycles += mfspr(SPRN_PURR) - in_purr;
 - get_lppaca()-idle = 0;
 -}
 -
 -static int snooze_loop(struct cpuidle_device *dev,
 - struct cpuidle_driver *drv,
 - int index)
 -{
 - unsigned long in_purr;
 - int cpu = dev-cpu;
 -
 - idle_loop_prolog(in_purr);
 - local_irq_enable();
 - set_thread_flag(TIF_POLLING_NRFLAG);
 -
 - while ((!need_resched())  cpu_online(cpu)) {
 - ppc64_runlatch_off();
 - HMT_low();
 - HMT_very_low();
 - }
 -
 - HMT_medium();
 - clear_thread_flag(TIF_POLLING_NRFLAG);
 - smp_mb();
 -
 - idle_loop_epilog(in_purr);
 -
 - return index;
 -}
 -
 -static void check_and_cede_processor(void)
 -{
 - /*
 -  * Ensure our interrupt state is properly tracked,
 -  * also checks if no interrupt has

RE: [PATCH V2 5/6] cpuidle/powerpc: Backend-powerpc idle driver for powernv and pseries.

[Wang Dongsheng-B40534]

 
 -static int pseries_cpuidle_add_cpu_notifier(struct notifier_block *n,
 +static int powerpc_cpuidle_add_cpu_notifier(struct notifier_block *n,
   unsigned long action, void *hcpu)
  {
   int hotcpu = (unsigned long)hcpu;
   struct cpuidle_device *dev =
 - per_cpu_ptr(pseries_cpuidle_devices, hotcpu);
 + per_cpu_ptr(powerpc_cpuidle_devices, hotcpu);
 
   if (dev  cpuidle_get_driver()) {
   switch (action) {
 @@ -235,16 +270,16 @@ static int pseries_cpuidle_add_cpu_notifier(struct
 notifier_block *n,
  }
 
  static struct notifier_block setup_hotplug_notifier = {
 - .notifier_call = pseries_cpuidle_add_cpu_notifier,
 + .notifier_call = powerpc_cpuidle_add_cpu_notifier,
  };
 
I think Daniel means move the notifier to cpuidle framework, not just powerpc.

And should be remove all about *device*. If the notifier handle using device,
you can use cpuidle_devices.

- dongsheng

 -static int __init pseries_processor_idle_init(void)
 +static int __init powerpc_processor_idle_init(void)
  {
   int retval;
 
 - retval = pseries_idle_probe();
 + retval = powerpc_idle_probe();
   if (retval)
   return retval;
 
 - pseries_cpuidle_driver_init();
 - retval = cpuidle_register_driver(pseries_idle_driver);
 + powerpc_cpuidle_driver_init();
 + retval = cpuidle_register_driver(powerpc_idle_driver);
   if (retval) {
 - printk(KERN_DEBUG Registration of pseries driver failed.\n);
 + printk(KERN_DEBUG Registration of powerpc driver failed.\n);
   return retval;
   }
 
   update_smt_snooze_delay(-1, per_cpu(smt_snooze_delay, 0));
 
 - retval = pseries_idle_devices_init();
 + retval = powerpc_idle_devices_init();
Should be remove all *device*, using cpuidle_register.

- dongsheng

RE: [PATCH V2 4/6] cpuidle/pseries: Move the pseries_idle backend driver to sysdev.

[Wang Dongsheng-B40534]

Hi Preeti,

 -Original Message-
 From: Preeti U Murthy [mailto:pre...@linux.vnet.ibm.com]
 Sent: Wednesday, July 31, 2013 12:00 PM
 To: Wang Dongsheng-B40534
 Cc: Deepthi Dharwar; b...@kernel.crashing.org; daniel.lezc...@linaro.org;
 linux-kernel@vger.kernel.org; mich...@ellerman.id.au;
 srivatsa.b...@linux.vnet.ibm.com; sva...@linux.vnet.ibm.com; linuxppc-
 d...@lists.ozlabs.org; r...@sisk.pl; linux...@vger.kernel.org
 Subject: Re: [PATCH V2 4/6] cpuidle/pseries: Move the pseries_idle
 backend driver to sysdev.
 
 Hi Dongsheng,
 
 On 07/31/2013 08:52 AM, Wang Dongsheng-B40534 wrote:
 
 
  -Original Message-
  From: Deepthi Dharwar [mailto:deep...@linux.vnet.ibm.com]
  Sent: Wednesday, July 31, 2013 10:59 AM
  To: b...@kernel.crashing.org; daniel.lezc...@linaro.org; linux-
  ker...@vger.kernel.org; mich...@ellerman.id.au;
  srivatsa.b...@linux.vnet.ibm.com; pre...@linux.vnet.ibm.com;
  sva...@linux.vnet.ibm.com; linuxppc-...@lists.ozlabs.org
  Cc: r...@sisk.pl; Wang Dongsheng-B40534; linux...@vger.kernel.org
  Subject: [PATCH V2 4/6] cpuidle/pseries: Move the pseries_idle
  backend driver to sysdev.
 
  Move pseries_idle backend driver code to arch/powerpc/sysdev so that
  the code can be used for a common driver for powernv and pseries.
  This removes a lot of code duplicacy.
 
  Why not drivers/cpuidle/?
 
  I think it should be move to drivers/cpuidle.
 
 Please take a look at what the cpuidle under drivers has to provide.
 cpuidle has two parts to it. The front end and the back end. The front
 end constitutes the cpuidle governors, registering of arch specific
 cpuidle drivers, disabling and enabling of cpuidle feature. It is this
 front end code which is present under drivers/cpuidle.
 
 The arch specific cpuidle drivers which decide what needs to be done to
 enter a specific idle state chosen by the cpuidle governor is what
 constitutes the back end of cpuidle. This will not be in drivers/cpuidle
 but in an arch/ specific code.
 
 The cpuidle under drivers/cpuidle drives the idle power management, but
 the low level handling of the entry into idle states should be taken care
 of by the architecture.
 
 Your recent patch :
 cpuidle: add freescale e500 family porcessors idle support IMO should
 hook onto the backend cpuidle driver that this patchset provides.
 
Sorry, I don't think so, cpuidle framework has been already very common.
Here we just need to do state definition and handling. I wonder whether
we need this layer.

If your handle is platform dependent, it should be in arch/platform.

If it is only for some platforms and the operation of these platforms can be
multiplexed, Why cannot as a driver to put into driver/cpuidle?

If it a general driver, I think we can put some common operating to 
driver/cpuidle
and make the platform specific code to arch/powerpc/platform.

This patch include front end and back end, not just back end.

This patch include too many state of different platforms and handle function. 
This state
and handle that should belong to itself platforms. Not a general way. If 
Deepthi will do
a general powerpc cpuidle, I think, it's cannot just using the macro to 
distinguish
platform. the front end code maybe move to driver/cpuidle(drvier register) 
should be better,
make the Low Power State and what should be handle to arch/powerpc/platform/**, 
because different
platforms have different state of low power consumption, and the processing 
method.
The front end can provide some general methods to register into general powerpc 
cpuidle driver.

-dongsheng

RE: [PATCH V2 4/6] cpuidle/pseries: Move the pseries_idle backend driver to sysdev.

[Wang Dongsheng-B40534]

> -Original Message-
> From: Deepthi Dharwar [mailto:deep...@linux.vnet.ibm.com]
> Sent: Wednesday, July 31, 2013 10:59 AM
> To: b...@kernel.crashing.org; daniel.lezc...@linaro.org; linux-
> ker...@vger.kernel.org; mich...@ellerman.id.au;
> srivatsa.b...@linux.vnet.ibm.com; pre...@linux.vnet.ibm.com;
> sva...@linux.vnet.ibm.com; linuxppc-...@lists.ozlabs.org
> Cc: r...@sisk.pl; Wang Dongsheng-B40534; linux...@vger.kernel.org
> Subject: [PATCH V2 4/6] cpuidle/pseries: Move the pseries_idle backend
> driver to sysdev.
> 
> Move pseries_idle backend driver code to arch/powerpc/sysdev
> so that the code can be used for a common driver for powernv
> and pseries. This removes a lot of code duplicacy.
> 
Why not drivers/cpuidle/?

I think it should be move to drivers/cpuidle.

-dongsheng

> Signed-off-by: Deepthi Dharwar 
> ---
>  arch/powerpc/platforms/pseries/Kconfig  |9 -
>  arch/powerpc/platforms/pseries/Makefile |1
>  arch/powerpc/platforms/pseries/processor_idle.c |  384 -
> --
>  arch/powerpc/sysdev/Kconfig |9 +
>  arch/powerpc/sysdev/Makefile|1
>  arch/powerpc/sysdev/processor_idle.c|  384
> +++
>  6 files changed, 394 insertions(+), 394 deletions(-)
>  delete mode 100644 arch/powerpc/platforms/pseries/processor_idle.c
>  create mode 100644 arch/powerpc/sysdev/processor_idle.c
> 
> diff --git a/arch/powerpc/platforms/pseries/Kconfig
> b/arch/powerpc/platforms/pseries/Kconfig
> index 62b4f80..bb59bb0 100644
> --- a/arch/powerpc/platforms/pseries/Kconfig
> +++ b/arch/powerpc/platforms/pseries/Kconfig
> @@ -119,12 +119,3 @@ config DTL
> which are accessible through a debugfs file.
> 
> Say N if you are unsure.
> -
> -config PSERIES_IDLE
> - bool "Cpuidle driver for pSeries platforms"
> - depends on CPU_IDLE
> - depends on PPC_PSERIES
> - default y
> - help
> -   Select this option to enable processor idle state management
> -   through cpuidle subsystem.
> diff --git a/arch/powerpc/platforms/pseries/Makefile
> b/arch/powerpc/platforms/pseries/Makefile
> index 8ae0103..4b22379 100644
> --- a/arch/powerpc/platforms/pseries/Makefile
> +++ b/arch/powerpc/platforms/pseries/Makefile
> @@ -21,7 +21,6 @@ obj-$(CONFIG_HCALL_STATS)   += hvCall_inst.o
>  obj-$(CONFIG_CMM)+= cmm.o
>  obj-$(CONFIG_DTL)+= dtl.o
>  obj-$(CONFIG_IO_EVENT_IRQ)   += io_event_irq.o
> -obj-$(CONFIG_PSERIES_IDLE)   += processor_idle.o
> 
>  ifeq ($(CONFIG_PPC_PSERIES),y)
>  obj-$(CONFIG_SUSPEND)+= suspend.o
> diff --git a/arch/powerpc/platforms/pseries/processor_idle.c
> b/arch/powerpc/platforms/pseries/processor_idle.c
> deleted file mode 100644
> index 0d75a54..000
> --- a/arch/powerpc/platforms/pseries/processor_idle.c
> +++ /dev/null
> @@ -1,384 +0,0 @@
> -/*
> - *  processor_idle - idle state cpuidle driver.
> - *  Adapted from drivers/idle/intel_idle.c and
> - *  drivers/acpi/processor_idle.c
> - *
> - */
> -
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -/* Snooze Delay, pseries_idle */
> -DECLARE_PER_CPU(long, smt_snooze_delay);
> -
> -struct cpuidle_driver pseries_idle_driver = {
> - .name = "pseries_idle",
> - .owner= THIS_MODULE,
> -};
> -
> -#define MAX_IDLE_STATE_COUNT 2
> -
> -static int max_idle_state = MAX_IDLE_STATE_COUNT - 1;
> -static struct cpuidle_device __percpu *pseries_cpuidle_devices;
> -static struct cpuidle_state *cpuidle_state_table;
> -
> -static inline void idle_loop_prolog(unsigned long *in_purr)
> -{
> - *in_purr = mfspr(SPRN_PURR);
> - /*
> -  * Indicate to the HV that we are idle. Now would be
> -  * a good time to find other work to dispatch.
> -  */
> - get_lppaca()->idle = 1;
> -}
> -
> -static inline void idle_loop_epilog(unsigned long in_purr)
> -{
> - get_lppaca()->wait_state_cycles += mfspr(SPRN_PURR) - in_purr;
> - get_lppaca()->idle = 0;
> -}
> -
> -static int snooze_loop(struct cpuidle_device *dev,
> - struct cpuidle_driver *drv,
> - int index)
> -{
> - unsigned long in_purr;
> - int cpu = dev->cpu;
> -
> - idle_loop_prolog(_purr);
> - local_irq_enable();
> - set_thread_flag(TIF_POLLING_NRFLAG);
> -
> - while ((!need_resched()) && cpu_online(cpu)) {
> - ppc64_runlatch_off();
> -   

RE: [PATCH V2 5/6] cpuidle/powerpc: Backend-powerpc idle driver for powernv and pseries.

[Wang Dongsheng-B40534]

> 
> -static int pseries_cpuidle_add_cpu_notifier(struct notifier_block *n,
> +static int powerpc_cpuidle_add_cpu_notifier(struct notifier_block *n,
>   unsigned long action, void *hcpu)
>  {
>   int hotcpu = (unsigned long)hcpu;
>   struct cpuidle_device *dev =
> - per_cpu_ptr(pseries_cpuidle_devices, hotcpu);
> + per_cpu_ptr(powerpc_cpuidle_devices, hotcpu);
> 
>   if (dev && cpuidle_get_driver()) {
>   switch (action) {
> @@ -235,16 +270,16 @@ static int pseries_cpuidle_add_cpu_notifier(struct
> notifier_block *n,
>  }
> 
>  static struct notifier_block setup_hotplug_notifier = {
> - .notifier_call = pseries_cpuidle_add_cpu_notifier,
> + .notifier_call = powerpc_cpuidle_add_cpu_notifier,
>  };
> 
I think Daniel means move the notifier to cpuidle framework, not just powerpc.

And should be remove all about *device*. If the notifier handle using device,
you can use "cpuidle_devices".

- dongsheng

> -static int __init pseries_processor_idle_init(void)
> +static int __init powerpc_processor_idle_init(void)
>  {
>   int retval;
> 
> - retval = pseries_idle_probe();
> + retval = powerpc_idle_probe();
>   if (retval)
>   return retval;
> 
> - pseries_cpuidle_driver_init();
> - retval = cpuidle_register_driver(_idle_driver);
> + powerpc_cpuidle_driver_init();
> + retval = cpuidle_register_driver(_idle_driver);
>   if (retval) {
> - printk(KERN_DEBUG "Registration of pseries driver failed.\n");
> + printk(KERN_DEBUG "Registration of powerpc driver failed.\n");
>   return retval;
>   }
> 
>   update_smt_snooze_delay(-1, per_cpu(smt_snooze_delay, 0));
> 
> - retval = pseries_idle_devices_init();
> + retval = powerpc_idle_devices_init();
Should be remove all *device*, using cpuidle_register.

- dongsheng

RE: [PATCH V2 4/6] cpuidle/pseries: Move the pseries_idle backend driver to sysdev.

[Wang Dongsheng-B40534]

Hi Preeti,

> -Original Message-
> From: Preeti U Murthy [mailto:pre...@linux.vnet.ibm.com]
> Sent: Wednesday, July 31, 2013 12:00 PM
> To: Wang Dongsheng-B40534
> Cc: Deepthi Dharwar; b...@kernel.crashing.org; daniel.lezc...@linaro.org;
> linux-kernel@vger.kernel.org; mich...@ellerman.id.au;
> srivatsa.b...@linux.vnet.ibm.com; sva...@linux.vnet.ibm.com; linuxppc-
> d...@lists.ozlabs.org; r...@sisk.pl; linux...@vger.kernel.org
> Subject: Re: [PATCH V2 4/6] cpuidle/pseries: Move the pseries_idle
> backend driver to sysdev.
> 
> Hi Dongsheng,
> 
> On 07/31/2013 08:52 AM, Wang Dongsheng-B40534 wrote:
> >
> >
> >> -Original Message-
> >> From: Deepthi Dharwar [mailto:deep...@linux.vnet.ibm.com]
> >> Sent: Wednesday, July 31, 2013 10:59 AM
> >> To: b...@kernel.crashing.org; daniel.lezc...@linaro.org; linux-
> >> ker...@vger.kernel.org; mich...@ellerman.id.au;
> >> srivatsa.b...@linux.vnet.ibm.com; pre...@linux.vnet.ibm.com;
> >> sva...@linux.vnet.ibm.com; linuxppc-...@lists.ozlabs.org
> >> Cc: r...@sisk.pl; Wang Dongsheng-B40534; linux...@vger.kernel.org
> >> Subject: [PATCH V2 4/6] cpuidle/pseries: Move the pseries_idle
> >> backend driver to sysdev.
> >>
> >> Move pseries_idle backend driver code to arch/powerpc/sysdev so that
> >> the code can be used for a common driver for powernv and pseries.
> >> This removes a lot of code duplicacy.
> >>
> > Why not drivers/cpuidle/?
> >
> > I think it should be move to drivers/cpuidle.
> 
> Please take a look at what the cpuidle under drivers has to provide.
> cpuidle has two parts to it. The front end and the back end. The front
> end constitutes the cpuidle governors, registering of arch specific
> cpuidle drivers, disabling and enabling of cpuidle feature. It is this
> front end code which is present under drivers/cpuidle.
> 
> The arch specific cpuidle drivers which decide what needs to be done to
> enter a specific idle state chosen by the cpuidle governor is what
> constitutes the back end of cpuidle. This will not be in drivers/cpuidle
> but in an arch/ specific code.
> 
> The cpuidle under drivers/cpuidle drives the idle power management, but
> the low level handling of the entry into idle states should be taken care
> of by the architecture.
> 
> Your recent patch :
> cpuidle: add freescale e500 family porcessors idle support IMO should
> hook onto the backend cpuidle driver that this patchset provides.
> 
Sorry, I don't think so, cpuidle framework has been already very common.
Here we just need to do state definition and handling. I wonder whether
we need this layer.

If your handle is platform dependent, it should be in arch/platform.

If it is only for some platforms and the operation of these platforms can be
multiplexed, Why cannot as a driver to put into driver/cpuidle?

If it a general driver, I think we can put some common operating to 
driver/cpuidle
and make the platform specific code to arch/powerpc/platform.

This patch include front end and back end, not just back end.

This patch include too many state of different platforms and handle function. 
This state
and handle that should belong to itself platforms. Not a general way. If 
Deepthi will do
a general powerpc cpuidle, I think, it's cannot just using the macro to 
distinguish
platform. the front end code maybe move to driver/cpuidle(drvier register) 
should be better,
make the Low Power State and what should be handle to arch/powerpc/platform/**, 
because different
platforms have different state of low power consumption, and the processing 
method.
The front end can provide some general methods to register into general powerpc 
cpuidle driver.

-dongsheng

RE: [PATCH V5 3/5] POWER/cpuidle: Generic IBM-POWER backend cpuidle driver.

[Wang Dongsheng-B40534]

> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> index 0e2cd5c..e805dcd 100644
> --- a/drivers/cpuidle/Kconfig
> +++ b/drivers/cpuidle/Kconfig

Maybe drivers/cpuidle/Kconfig.powerpc is better? Like arm.

> +obj-$(CONFIG_CPU_IDLE_IBM_POWER) += cpuidle-ibm-power.o
> diff --git a/drivers/cpuidle/cpuidle-ibm-power.c
> b/drivers/cpuidle/cpuidle-ibm-power.c
> new file mode 100644
> index 000..4ee5a94
> --- /dev/null
> +++ b/drivers/cpuidle/cpuidle-ibm-power.c
> @@ -0,0 +1,304 @@
> +/*
> + *  cpuidle-ibm-power - idle state cpuidle driver.
> + *  Adapted from drivers/idle/intel_idle.c and
> + *  drivers/acpi/processor_idle.c
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct cpuidle_driver power_idle_driver = {
> + .name = "IBM-POWER-Idle",
> + .owner= THIS_MODULE,
> +};
> +
> +#define MAX_IDLE_STATE_COUNT 2
> +
> +static int max_idle_state = MAX_IDLE_STATE_COUNT - 1;

Again, do not use the macro.

> +static struct cpuidle_state *cpuidle_state_table;
> +
> +static inline void idle_loop_prolog(unsigned long *in_purr)
> +{
> + *in_purr = mfspr(SPRN_PURR);
> + /*
> +  * Indicate to the HV that we are idle. Now would be
> +  * a good time to find other work to dispatch.
> +  */
> + get_lppaca()->idle = 1;
> +}
> +
> +static inline void idle_loop_epilog(unsigned long in_purr)
> +{
> + get_lppaca()->wait_state_cycles += mfspr(SPRN_PURR) - in_purr;
> + get_lppaca()->idle = 0;
> +}
> +
> +static int snooze_loop(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv,
> + int index)
> +{
> + unsigned long in_purr;
> +
> + idle_loop_prolog(_purr);
> + local_irq_enable();

snooze_loop has already registered in cpuidle framework to handle snooze state.
where disable the irq? Why do "enable" here?

> +/*
> + * States for dedicated partition case.
> + */
> +static struct cpuidle_state dedicated_states[MAX_IDLE_STATE_COUNT] = {
> + { /* Snooze */
> + .name = "snooze",
> + .desc = "snooze",
> + .flags = CPUIDLE_FLAG_TIME_VALID,
> + .exit_latency = 0,
> + .target_residency = 0,
> + .enter = _loop },
> + { /* CEDE */
> + .name = "CEDE",
> + .desc = "CEDE",
> + .flags = CPUIDLE_FLAG_TIME_VALID,
> + .exit_latency = 10,
> + .target_residency = 100,
> + .enter = _cede_loop },
> +};
> +
> +/*
> + * States for shared partition case.
> + */
> +static struct cpuidle_state shared_states[MAX_IDLE_STATE_COUNT] = {
> + { /* Shared Cede */
> + .name = "Shared Cede",
> + .desc = "Shared Cede",
> + .flags = CPUIDLE_FLAG_TIME_VALID,
> + .exit_latency = 0,
> + .target_residency = 0,
> + .enter = _cede_loop },
> +};
> +
> +static void __exit power_processor_idle_exit(void)
> +{
> +
> + unregister_cpu_notifier(_hotplug_notifier);

Remove a blank line.

> + cpuidle_unregister(_idle_driver);
> + return;
> +}
> +
> +module_init(power_processor_idle_init);
> +module_exit(power_processor_idle_exit);
> +

Did you have tested the module? If not tested, please don't use the module.

> +MODULE_AUTHOR("Deepthi Dharwar ");
> +MODULE_DESCRIPTION("Cpuidle driver for IBM POWER platforms");
> +MODULE_LICENSE("GPL");
>