Re: [PATCH v3] powerpc/64s: reimplement book3s idle code in C

2018-08-30 Thread Nicholas Piggin
On Fri, 31 Aug 2018 12:38:19 +0800
kbuild test robot  wrote:

> Hi Nicholas,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on powerpc/next]
> [also build test ERROR on v4.19-rc1 next-20180830]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-64s-reimplement-book3s-idle-code-in-C/20180829-014912
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
> config: powerpc-skiroot_defconfig (attached as .config)
> compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=7.2.0 make.cross ARCH=powerpc 
> 
> All errors (new ones prefixed by >>):
> 
> >> arch/powerpc/platforms/powernv/idle.c:773:22: error: 'power9_offline_stop' 
> >> defined but not used [-Werror=unused-function]  
> static unsigned long power9_offline_stop(unsigned long psscr)
>  ^~~
> >> arch/powerpc/platforms/powernv/idle.c:486:22: error: 'power7_offline' 
> >> defined but not used [-Werror=unused-function]  
> static unsigned long power7_offline(void)
>  ^~
>cc1: all warnings being treated as errors
> 
> vim +/power9_offline_stop +773 arch/powerpc/platforms/powernv/idle.c
> 
>772
>  > 773static unsigned long power9_offline_stop(unsigned long psscr)  
>774{
>775unsigned long srr1;
>776

Ahh, no hotplug. Good bot.

Thanks,
Nick


Re: v4.17 regression: PowerMac G3 won't boot, was Re: [PATCH v5 1/3] of: cache phandle nodes to reduce cost of of_find_node_by_phandle()

2018-08-30 Thread Benjamin Herrenschmidt
On Thu, 2018-08-30 at 21:36 -0700, Frank Rowand wrote:
> > No idea whether that's relevant; I haven't done any further investigation. 
> > Complete dmesg output is attached. Please let me know if there's any more 
> > information you need to help find the bug.
> > 
> > Thanks.
> 
> I don't have any useful answers yet, but I am following the thread and have
> also quickly scanned the two commits for any obvious cause.  I will look
> into this some more, but have a few other tasks that I need to complete
> first.
> 
> A long shot, but something to consider, is that I failed to cover the
> cases of dynamic devicetree updates (removing nodes that contain a
> phandle) in ways other than overlays.  Michael Ellerman has reported
> such a problem for powerpc/mobility with of_detach_node().  A patch to
> fix that is one of the tasks I need to complete.

The only thing I can think of is booting via the BootX bootloader on
those ancient macs results in a DT with no phandles. I didn't see an
obvious reason why that would cause that patch to break though.

Cheers,
Ben.



Re: v4.17 regression: PowerMac G3 won't boot, was Re: [PATCH v5 1/3] of: cache phandle nodes to reduce cost of of_find_node_by_phandle()

2018-08-30 Thread Benjamin Herrenschmidt
On Fri, 2018-08-31 at 14:35 +1000, Benjamin Herrenschmidt wrote:
> 
> > If I force output with "-f", the resulting file has no occurrences 
> > of "phandle".
> 
> Are you booting with BootX or Open Firmware ?

Assuming you are using BootX (or miBoot), can you try this patch ?

--- a/arch/powerpc/platforms/powermac/bootx_init.c
+++ b/arch/powerpc/platforms/powermac/bootx_init.c
@@ -37,6 +37,7 @@ static unsigned long __initdata bootx_dt_strend;
 static unsigned long __initdata bootx_node_chosen;
 static boot_infos_t * __initdata bootx_info;
 static char __initdata bootx_disp_path[256];
+static int __initdata bootx_phandle;
 
 /* Is boot-info compatible ? */
 #define BOOT_INFO_IS_COMPATIBLE(bi) \
@@ -258,6 +259,8 @@ static void __init bootx_scan_dt_build_strings(unsigned 
long base,
namep = pp->name ? (char *)(base + pp->name) : NULL;
if (namep == NULL || strcmp(namep, "name") == 0)
goto next;
+   if (!strcmp(namep, "phandle") || !strcmp(namep, 
"linux,phandle"))
+   bootx_phandle = -1;
/* get/create string entry */
soff = bootx_dt_find_string(namep);
if (soff == 0)
@@ -310,6 +313,7 @@ static void __init bootx_scan_dt_build_struct(unsigned long 
base,
*mem_end = _ALIGN_UP((unsigned long)lp + 1, 4);
 
/* get and store all properties */
+   has_phandle = false;
while (*ppp) {
struct bootx_dt_prop *pp =
(struct bootx_dt_prop *)(base + *ppp);
@@ -330,6 +334,12 @@ static void __init bootx_scan_dt_build_struct(unsigned 
long base,
ppp = >next;
}
 
+   /* add a phandle */
+   if (bootx_phandle > 0) {
+   bootx_dt_add_prop("phandle", _phandle, 4, mem_end);
+   bootx_phandle++;
+   }
+
if (node == bootx_node_chosen) {
bootx_add_chosen_props(base, mem_end);
if (bootx_info->dispDeviceRegEntryOffset == 0)
@@ -385,6 +395,8 @@ static unsigned long __init bootx_flatten_dt(unsigned long 
start)
bootx_dt_add_string("linux,bootx-height", _end);
bootx_dt_add_string("linux,bootx-linebytes", _end);
bootx_dt_add_string("linux,bootx-addr", _end);
+   if (bootx_phandle > 0)
+   bootx_dt_add_string("phandle", _end);
/* Wrap up strings */
hdr->off_dt_strings = bootx_dt_strbase - mem_start;
hdr->dt_strings_size = bootx_dt_strend - bootx_dt_strbase;
@@ -482,6 +494,7 @@ void __init bootx_init(unsigned long r3, unsigned long r4)
bootx_dt_strbase = bootx_dt_strend = 0;
bootx_node_chosen = 0;
bootx_disp_path[0] = 0;
+   bootx_phandle = 1;
 
if (!BOOT_INFO_IS_V2_COMPATIBLE(bi))
bi->logicalDisplayBase = bi->dispDeviceBase;



Re: [PATCH v3] powerpc/64s: reimplement book3s idle code in C

2018-08-30 Thread kbuild test robot
Hi Nicholas,

I love your patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.19-rc1 next-20180830]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-64s-reimplement-book3s-idle-code-in-C/20180829-014912
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-skiroot_defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

>> arch/powerpc/platforms/powernv/idle.c:773:22: error: 'power9_offline_stop' 
>> defined but not used [-Werror=unused-function]
static unsigned long power9_offline_stop(unsigned long psscr)
 ^~~
>> arch/powerpc/platforms/powernv/idle.c:486:22: error: 'power7_offline' 
>> defined but not used [-Werror=unused-function]
static unsigned long power7_offline(void)
 ^~
   cc1: all warnings being treated as errors

vim +/power9_offline_stop +773 arch/powerpc/platforms/powernv/idle.c

   772  
 > 773  static unsigned long power9_offline_stop(unsigned long psscr)
   774  {
   775  unsigned long srr1;
   776  

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


.config.gz
Description: application/gzip


Re: v4.17 regression: PowerMac G3 won't boot, was Re: [PATCH v5 1/3] of: cache phandle nodes to reduce cost of of_find_node_by_phandle()

2018-08-30 Thread Frank Rowand
Hi Finn,

On 08/29/18 17:44, Finn Thain wrote:
> Hi Frank,
> 
> Linux v4.17 and later will no longer boot on a G3 PowerMac. The boot hangs 
> very early, before any video driver loads.
> 
> Stan and I were able to bisect the regression between v4.16 and v4.17 and 
> arrived at commit 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of 
> of_find_node_by_phandle()").
> 
> I don't see any obvious bug in 0b3ce78e90fc or b9952b5218ad. But if you 
> revert these from v4.18 (which is also affected) that certainly resolves 
> the issue.
> 
> I did see this in the kernel messages:
> 
> Duplicate name in PowerPC,750, renamed to "l2-cache#1"
> Duplicate name in mac-io, renamed to "ide#1"
> Duplicate name in ide#1, renamed to "atapi-disk#1"
> Duplicate name in multifunc-device, renamed to "pci1799,1#1"
> 
> No idea whether that's relevant; I haven't done any further investigation. 
> Complete dmesg output is attached. Please let me know if there's any more 
> information you need to help find the bug.
> 
> Thanks.

I don't have any useful answers yet, but I am following the thread and have
also quickly scanned the two commits for any obvious cause.  I will look
into this some more, but have a few other tasks that I need to complete
first.

A long shot, but something to consider, is that I failed to cover the
cases of dynamic devicetree updates (removing nodes that contain a
phandle) in ways other than overlays.  Michael Ellerman has reported
such a problem for powerpc/mobility with of_detach_node().  A patch to
fix that is one of the tasks I need to complete.

-Frank



Re: v4.17 regression: PowerMac G3 won't boot, was Re: [PATCH v5 1/3] of: cache phandle nodes to reduce cost of of_find_node_by_phandle()

2018-08-30 Thread Benjamin Herrenschmidt
On Thu, 2018-08-30 at 20:39 -0600, Mac User wrote:
> On 8/29/18 7:05 PM, Rob Herring wrote:
> 
> > On Wed, Aug 29, 2018 at 7:44 PM Finn Thain  
> > wrote:
> > > Hi Frank,
> > > 
> > > Linux v4.17 and later will no longer boot on a G3 PowerMac. The boot hangs
> > > very early, before any video driver loads.
> > > 
> > > Stan and I were able to bisect the regression between v4.16 and v4.17 and
> > > arrived at commit 0b3ce78e90fc ("of: cache phandle nodes to reduce cost of
> > > of_find_node_by_phandle()").
> > > 
> > > I don't see any obvious bug in 0b3ce78e90fc or b9952b5218ad. But if you
> > > revert these from v4.18 (which is also affected) that certainly resolves
> > > the issue.
> > 
> > Perhaps a bad assumption on phandle values causing a problem. Can you
> > provide a dump of all the phandle or linux,phandle values from
> > /proc/device-tree.
> > 
> > Rob
> 
> Rob,
> 
> As suggested by Finn, I installed device-tree-compiler and
> powerpc-ibm-utils.
>  
> Running "dtc -I fs -H both /sys/firmware/devicetree/base"
> resulted in the following errors:
> 
> DTC: fs->dts on file "/sys/firmware/devicetree/base"
> ERROR (name_properties): "name" property in
> /pci/multifunc-device/pci1799,1#1 is incorrect ("pci1799,1" instead of
> base node name)
> ERROR (name_properties): "name" property in /pci/mac-io/ide#1 is
> incorrect ("ide" instead of base node name)
> ERROR (name_properties): "name" property in
> /pci/mac-io/ide#1/atapi-disk#1 is incorrect ("atapi-disk" instead of
> base node name)
> ERROR (name_properties): "name" property in /cpus/PowerPC,750/l2-cache#1
> is incorrect ("l2-cache" instead of base node name)
> ERROR: Input tree has errors, aborting (use -f to force output)
> 
> If I force output with "-f", the resulting file has no occurrences 
> of "phandle".

Are you booting with BootX or Open Firmware ?

> Running "lsprop /proc/device-tree | grep -i phandle" results in no 
> output.
> 
> Please let me know if there's some other way to get information that 
> would be helpful.
> 
> thanks
> 
> -Stan



Re: [PATCH kernel 1/4] KVM: PPC: Validate all tces before updating tables

2018-08-30 Thread Alexey Kardashevskiy



On 30/08/2018 14:01, David Gibson wrote:
> On Thu, Aug 30, 2018 at 01:16:44PM +1000, Alexey Kardashevskiy wrote:
>> The KVM TCE handlers are written in a way so they fail when either
>> something went horribly wrong or the userspace did some obvious mistake
>> such as passing a misaligned address.
>>
>> We are going to enhance the TCE checker to fail on attempts to map bigger
>> IOMMU page than the underlying pinned memory so let's valitate TCE
>> beforehand.
>>
>> This should cause no behavioral change.
>>
>> Signed-off-by: Alexey Kardashevskiy 
> 
> Reviewed-by: David Gibson 
> 
> With one misgiving..
> 
>> ---
>>  arch/powerpc/kvm/book3s_64_vio.c| 8 
>>  arch/powerpc/kvm/book3s_64_vio_hv.c | 4 
>>  2 files changed, 12 insertions(+)
>>
>> diff --git a/arch/powerpc/kvm/book3s_64_vio.c 
>> b/arch/powerpc/kvm/book3s_64_vio.c
>> index 9a3f264..0fef22b 100644
>> --- a/arch/powerpc/kvm/book3s_64_vio.c
>> +++ b/arch/powerpc/kvm/book3s_64_vio.c
>> @@ -599,6 +599,14 @@ long kvmppc_h_put_tce_indirect(struct kvm_vcpu *vcpu,
>>  ret = kvmppc_tce_validate(stt, tce);
>>  if (ret != H_SUCCESS)
>>  goto unlock_exit;
>> +}
>> +
>> +for (i = 0; i < npages; ++i) {
>> +if (get_user(tce, tces + i)) {
> 
> This looks unsafe, because we validate, then regrab the TCE from
> userspace which could have been changed by another thread.
> 
> But it actually is safe, because the relevant checks will be
> re-executed in the following code.  If userspace tries to change this
> dodgily it will result in a messier failure mode but won't threaten
> the host.


I have put this to 3/4 for this get_user() while it should have been here:
+   /*
+* This get_user() may produce a different result than few
+* lines in the validation loop above but we translate it
+* again little later anyway and if that fails, we simply stop
+* and return error as it is likely the userspace shooting
+* itself in a foot.
+*/

Might repost, testing that THP+realmode patch


> 
> Long term, I think we would be better off copying everything into
> kernel space then doing the validation just once.  But the difference
> should only become apparent with a malicious or badly broken guest,
> and in the meantime this series addresses a real problem.
> 
> So, I think we should go ahead with it despite that imperfection.
> 
> 
>> +ret = H_TOO_HARD;
>> +goto unlock_exit;
>> +}
>> +tce = be64_to_cpu(tce);
>>  
>>  if (kvmppc_gpa_to_ua(vcpu->kvm,
>>  tce & ~(TCE_PCI_READ | TCE_PCI_WRITE),
>> diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c 
>> b/arch/powerpc/kvm/book3s_64_vio_hv.c
>> index 506a4d4..7ab6f3f 100644
>> --- a/arch/powerpc/kvm/book3s_64_vio_hv.c
>> +++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
>> @@ -501,6 +501,10 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
>>  ret = kvmppc_tce_validate(stt, tce);
>>  if (ret != H_SUCCESS)
>>  goto unlock_exit;
>> +}
>> +
>> +for (i = 0; i < npages; ++i) {
>> +unsigned long tce = be64_to_cpu(((u64 *)tces)[i]);
>>  
>>  ua = 0;
>>  if (kvmppc_gpa_to_ua(vcpu->kvm,
> 

-- 
Alexey


[PATCH 1/3] soc: fsl: add Platform PM driver QorIQ platforms

2018-08-30 Thread Ran Wang
This driver is to provide a independent framework for PM service
provider and consumer to configure system level wake up feature. For
example, RCPM driver could register a callback function on this
platform first, and Flex timer driver who want to enable timer wake
up feature, will call generic API provided by this platform driver,
and then it will trigger RCPM driver to do it. The benefit is to
isolate the user and service, such as flex timer driver will not have
to know the implement details of wakeup function it require. Besides,
it is also easy for service side to upgrade its logic when design is
changed and remain user side unchanged.

Signed-off-by: Ran Wang 
---
 drivers/soc/fsl/Kconfig   |   14 +
 drivers/soc/fsl/Makefile  |1 +
 drivers/soc/fsl/plat_pm.c |  144 +
 include/soc/fsl/plat_pm.h |   22 +++
 4 files changed, 181 insertions(+), 0 deletions(-)
 create mode 100644 drivers/soc/fsl/plat_pm.c
 create mode 100644 include/soc/fsl/plat_pm.h

diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
index 7a9fb9b..6517412 100644
--- a/drivers/soc/fsl/Kconfig
+++ b/drivers/soc/fsl/Kconfig
@@ -16,3 +16,17 @@ config FSL_GUTS
  Initially only reading SVR and registering soc device are supported.
  Other guts accesses, such as reading RCW, should eventually be moved
  into this driver as well.
+
+config FSL_PLAT_PM
+   bool "Freescale platform PM framework"
+   help
+ This driver is to provide a independent framework for PM service
+ provider and consumer to configure system level wake up feature. For
+ example, RCPM driver could register a callback function on this
+ platform first, and Flex timer driver who want to enable timer wake
+ up feature, will call generic API provided by this platform driver,
+ and then it will trigger RCPM driver to do it. The benefit is to
+ isolate the user and service, such as  flex timer driver will not
+ have to know the implement details of wakeup function it require.
+ Besides, it is also easy for service side to upgrade its logic when
+ design changed and remain user side unchanged.
diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
index 44b3beb..8f9db23 100644
--- a/drivers/soc/fsl/Makefile
+++ b/drivers/soc/fsl/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_FSL_DPAA) += qbman/
 obj-$(CONFIG_QUICC_ENGINE) += qe/
 obj-$(CONFIG_CPM)  += qe/
 obj-$(CONFIG_FSL_GUTS) += guts.o
+obj-$(CONFIG_FSL_PLAT_PM)  += plat_pm.o
diff --git a/drivers/soc/fsl/plat_pm.c b/drivers/soc/fsl/plat_pm.c
new file mode 100644
index 000..19ea14e
--- /dev/null
+++ b/drivers/soc/fsl/plat_pm.c
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// plat_pm.c - Freescale platform PM framework
+//
+// Copyright 2018 NXP
+//
+// Author: Ran Wang ,
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+struct plat_pm_t {
+   struct list_head node;
+   fsl_plat_pm_handle handle;
+   void *handle_priv;
+   spinlock_t  lock;
+};
+
+static struct plat_pm_t plat_pm;
+
+// register_fsl_platform_wakeup_source - Register callback function to plat_pm
+// @handle: Pointer to handle PM feature requirement
+// @handle_priv: Handler specific data struct
+//
+// Return 0 on success other negative errno
+int register_fsl_platform_wakeup_source(fsl_plat_pm_handle handle,
+   void *handle_priv)
+{
+   struct plat_pm_t *p;
+   unsigned long   flags;
+
+   if (!handle) {
+   pr_err("FSL plat_pm: Handler invalid, reject\n");
+   return -EINVAL;
+   }
+
+   p = kmalloc(sizeof(*p), GFP_KERNEL);
+   if (!p)
+   return -ENOMEM;
+
+   p->handle = handle;
+   p->handle_priv = handle_priv;
+
+   spin_lock_irqsave(_pm.lock, flags);
+   list_add_tail(>node, _pm.node);
+   spin_unlock_irqrestore(_pm.lock, flags);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(register_fsl_platform_wakeup_source);
+
+// Deregister_fsl_platform_wakeup_source - deregister callback function
+// @handle_priv: Handler specific data struct
+//
+// Return 0 on success other negative errno
+int deregister_fsl_platform_wakeup_source(void *handle_priv)
+{
+   struct plat_pm_t *p, *tmp;
+   unsigned long   flags;
+
+   spin_lock_irqsave(_pm.lock, flags);
+   list_for_each_entry_safe(p, tmp, _pm.node, node) {
+   if (p->handle_priv == handle_priv) {
+   list_del(>node);
+   kfree(p);
+   }
+   }
+   spin_unlock_irqrestore(_pm.lock, flags);
+   return 0;
+}
+EXPORT_SYMBOL_GPL(deregister_fsl_platform_wakeup_source);
+
+// fsl_platform_wakeup_config - Configure wakeup source by calling handlers
+// @dev: pointer to user's device struct
+// @flag: to tell enable or disable wakeup source
+//
+// 

[PATCH 3/3] soc: fsl: add RCPM driver

2018-08-30 Thread Ran Wang
The NXP's QorIQ Processors based on ARM Core have RCPM module (Run
Control and Power Management), which performs all device-level
tasks associated with power management such as wakeup source control.

This driver depends on FSL platform PM driver framework which help to
isolate user and PM service provider (such as RCPM driver).

Signed-off-by: Chenhui Zhao 
Signed-off-by: Ying Zhang 
Signed-off-by: Ran Wang 
---
 drivers/soc/fsl/Kconfig   |6 ++
 drivers/soc/fsl/Makefile  |1 +
 drivers/soc/fsl/ls-rcpm.c |  153 +
 3 files changed, 160 insertions(+), 0 deletions(-)
 create mode 100644 drivers/soc/fsl/ls-rcpm.c

diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
index 6517412..882330d 100644
--- a/drivers/soc/fsl/Kconfig
+++ b/drivers/soc/fsl/Kconfig
@@ -30,3 +30,9 @@ config FSL_PLAT_PM
  have to know the implement details of wakeup function it require.
  Besides, it is also easy for service side to upgrade its logic when
  design changed and remain user side unchanged.
+
+config LS_RCPM
+   bool "Freescale RCPM support"
+   depends on (FSL_PLAT_PM)
+   help
+ This feature is to enable specified wakeup source for system sleep.
diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
index 8f9db23..43ff71a 100644
--- a/drivers/soc/fsl/Makefile
+++ b/drivers/soc/fsl/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_QUICC_ENGINE)  += qe/
 obj-$(CONFIG_CPM)  += qe/
 obj-$(CONFIG_FSL_GUTS) += guts.o
 obj-$(CONFIG_FSL_PLAT_PM)  += plat_pm.o
+obj-$(CONFIG_LS_RCPM)  += ls-rcpm.o
diff --git a/drivers/soc/fsl/ls-rcpm.c b/drivers/soc/fsl/ls-rcpm.c
new file mode 100644
index 000..b0feb88
--- /dev/null
+++ b/drivers/soc/fsl/ls-rcpm.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// plat_pm.c - Freescale Layerscape RCPM driver
+//
+// Copyright 2018 NXP
+//
+// Author: Ran Wang ,
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MAX_COMPATIBLE_NUM 10
+
+struct rcpm_t {
+   struct device *dev;
+   void __iomem *ippdexpcr_addr;
+   bool big_endian;/* Big/Little endian of RCPM module */
+};
+
+// rcpm_handle - Configure RCPM reg according to wake up source request
+// @user_dev: pointer to user's device struct
+// @flag: to enable(true) or disable(false) wakeup source
+// @handle_priv: pointer to struct rcpm_t instance
+//
+// Return 0 on success other negative errno
+static int rcpm_handle(struct device *user_dev, bool flag, void *handle_priv)
+{
+   struct rcpm_t *rcpm;
+   bool big_endian;
+   const char  *dev_compatible_array[MAX_COMPATIBLE_NUM];
+   void __iomem *ippdexpcr_addr;
+   u32 ippdexpcr;
+   u32 set_bit;
+   int ret, num, i;
+
+   rcpm = handle_priv;
+   big_endian = rcpm->big_endian;
+   ippdexpcr_addr = rcpm->ippdexpcr_addr;
+
+   num = device_property_read_string_array(user_dev, "compatible",
+   dev_compatible_array, MAX_COMPATIBLE_NUM);
+   if (num < 0)
+   return num;
+
+   for (i = 0; i < num; i++) {
+   if (!device_property_present(rcpm->dev,
+   dev_compatible_array[i]))
+   continue;
+   else {
+   ret = device_property_read_u32(rcpm->dev,
+   dev_compatible_array[i], _bit);
+   if (ret)
+   return ret;
+
+   if (!device_property_present(rcpm->dev,
+   dev_compatible_array[i]))
+   return -ENODEV;
+   else {
+   ret = device_property_read_u32(rcpm->dev,
+   dev_compatible_array[i], 
_bit);
+   if (ret)
+   return ret;
+
+   if (big_endian)
+   ippdexpcr = ioread32be(ippdexpcr_addr);
+   else
+   ippdexpcr = ioread32(ippdexpcr_addr);
+
+   if (flag)
+   ippdexpcr |= set_bit;
+   else
+   ippdexpcr &= ~set_bit;
+
+   if (big_endian) {
+   iowrite32be(ippdexpcr, ippdexpcr_addr);
+   ippdexpcr = ioread32be(ippdexpcr_addr);
+   } else
+   iowrite32(ippdexpcr, ippdexpcr_addr);
+
+   return 0;
+   }
+   }
+   }
+
+   return -ENODEV;
+}
+
+static int ls_rcpm_probe(struct platform_device *pdev)

[PATCH 2/3] Documentation: dt: binding: fsl: update property description for RCPM

2018-08-30 Thread Ran Wang
Add property 'big-endian' and supportted IP's configuration info.
Remove property 'fsl,#rcpm-wakeup-cell'.

Signed-off-by: Ran Wang 
---
 Documentation/devicetree/bindings/soc/fsl/rcpm.txt |   42 ++-
 1 files changed, 13 insertions(+), 29 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt 
b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
index e284e4e..7fc630a 100644
--- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
+++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
@@ -5,8 +5,6 @@ and power management.
 
 Required properites:
   - reg : Offset and length of the register set of the RCPM block.
-  - fsl,#rcpm-wakeup-cells : The number of IPPDEXPCR register cells in the
-   fsl,rcpm-wakeup property.
   - compatible : Must contain a chip-specific RCPM block compatible string
and (if applicable) may contain a chassis-version RCPM compatible
string. Chip-specific strings are of the form "fsl,-rcpm",
@@ -27,37 +25,23 @@ Chassis Version Example Chips
 ------
 1.0p4080, p5020, p5040, p2041, p3041
 2.0t4240, b4860, b4420
-2.1t1040, ls1021
+2.1t1040, ls1021, ls1012
+
+Optional properties:
+ - big-endian : Indicate RCPM registers is big-endian. A RCPM node
+   that doesn't have this property will be regarded as little-endian.
+ -  : This string
+   is referred by RCPM driver to judge if the consumer (such as flex timer)
+   is able to be regards as wakeup source or not, such as 'fsl,ls1012a-ftm'.
+   Further, this property will carry the bit mask info to control
+   coresponding wake up source.
 
 Example:
 The RCPM node for T4240:
rcpm: global-utilities@e2000 {
compatible = "fsl,t4240-rcpm", "fsl,qoriq-rcpm-2.0";
reg = <0xe2000 0x1000>;
-   fsl,#rcpm-wakeup-cells = <2>;
-   };
-
-* Freescale RCPM Wakeup Source Device Tree Bindings

-Required fsl,rcpm-wakeup property should be added to a device node if the 
device
-can be used as a wakeup source.
-
-  - fsl,rcpm-wakeup: Consists of a phandle to the rcpm node and the IPPDEXPCR
-   register cells. The number of IPPDEXPCR register cells is defined in
-   "fsl,#rcpm-wakeup-cells" in the rcpm node. The first register cell is
-   the bit mask that should be set in IPPDEXPCR0, and the second register
-   cell is for IPPDEXPCR1, and so on.
-
-   Note: IPPDEXPCR(IP Powerdown Exception Control Register) provides a
-   mechanism for keeping certain blocks awake during STANDBY and MEM, in
-   order to use them as wake-up sources.
-
-Example:
-   lpuart0: serial@295 {
-   compatible = "fsl,ls1021a-lpuart";
-   reg = <0x0 0x295 0x0 0x1000>;
-   interrupts = ;
-   clocks = <>;
-   clock-names = "ipg";
-   fsl,rcpm-wakeup = < 0x0 0x4000>;
+   big-endian;
+   fsl,ls1012a-ftm = <0x2>;
+   fsl,pfe = <0xf020>;
};
-- 
1.7.1



[PATCH v4 2/2] powerpc/pseries:Remove unneeded uses of dlpar work queue

2018-08-30 Thread Nathan Fontenot
There are three instances in which dlpar hotplug events are invoked;
handling a hotplug interrupt (in a kvm guest), handling a dlpar
request through sysfs, and updating LMB affinity when handling a
PRRN event. Only in the case of handling a hotplug interrupt do we
have to put the work on a workqueue, the other cases can handle the
dlpar request directly.

This patch exports the handle_dlpar_errorlog() function so that
dlpar hotplug events can be handled directly and updates the two
instances mentioned above to use the direct invocation.

Signed-off-by: Nathan Fontenot 
---
 arch/powerpc/platforms/pseries/dlpar.c|   37 +++--
 arch/powerpc/platforms/pseries/mobility.c |   18 +-
 arch/powerpc/platforms/pseries/pseries.h  |5 ++--
 arch/powerpc/platforms/pseries/ras.c  |2 +-
 4 files changed, 19 insertions(+), 43 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index a0b20c03f078..052c4f2ba0a0 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -32,8 +32,6 @@ static struct workqueue_struct *pseries_hp_wq;
 struct pseries_hp_work {
struct work_struct work;
struct pseries_hp_errorlog *errlog;
-   struct completion *hp_completion;
-   int *rc;
 };
 
 struct cc_workarea {
@@ -329,7 +327,7 @@ int dlpar_release_drc(u32 drc_index)
return 0;
 }
 
-static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
+int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
 {
int rc;
 
@@ -371,20 +369,13 @@ static void pseries_hp_work_fn(struct work_struct *work)
struct pseries_hp_work *hp_work =
container_of(work, struct pseries_hp_work, work);
 
-   if (hp_work->rc)
-   *(hp_work->rc) = handle_dlpar_errorlog(hp_work->errlog);
-   else
-   handle_dlpar_errorlog(hp_work->errlog);
-
-   if (hp_work->hp_completion)
-   complete(hp_work->hp_completion);
+   handle_dlpar_errorlog(hp_work->errlog);
 
kfree(hp_work->errlog);
kfree((void *)work);
 }
 
-void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
-struct completion *hotplug_done, int *rc)
+void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog)
 {
struct pseries_hp_work *work;
struct pseries_hp_errorlog *hp_errlog_copy;
@@ -397,13 +388,9 @@ void queue_hotplug_event(struct pseries_hp_errorlog 
*hp_errlog,
if (work) {
INIT_WORK((struct work_struct *)work, pseries_hp_work_fn);
work->errlog = hp_errlog_copy;
-   work->hp_completion = hotplug_done;
-   work->rc = rc;
queue_work(pseries_hp_wq, (struct work_struct *)work);
} else {
-   *rc = -ENOMEM;
kfree(hp_errlog_copy);
-   complete(hotplug_done);
}
 }
 
@@ -521,18 +508,15 @@ static int dlpar_parse_id_type(char **cmd, struct 
pseries_hp_errorlog *hp_elog)
 static ssize_t dlpar_store(struct class *class, struct class_attribute *attr,
   const char *buf, size_t count)
 {
-   struct pseries_hp_errorlog *hp_elog;
-   struct completion hotplug_done;
+   struct pseries_hp_errorlog hp_elog;
char *argbuf;
char *args;
int rc;
 
args = argbuf = kstrdup(buf, GFP_KERNEL);
-   hp_elog = kzalloc(sizeof(*hp_elog), GFP_KERNEL);
-   if (!hp_elog || !argbuf) {
+   if (!argbuf) {
pr_info("Could not allocate resources for DLPAR operation\n");
kfree(argbuf);
-   kfree(hp_elog);
return -ENOMEM;
}
 
@@ -540,25 +524,22 @@ static ssize_t dlpar_store(struct class *class, struct 
class_attribute *attr,
 * Parse out the request from the user, this will be in the form:
 *
 */
-   rc = dlpar_parse_resource(, hp_elog);
+   rc = dlpar_parse_resource(, _elog);
if (rc)
goto dlpar_store_out;
 
-   rc = dlpar_parse_action(, hp_elog);
+   rc = dlpar_parse_action(, _elog);
if (rc)
goto dlpar_store_out;
 
-   rc = dlpar_parse_id_type(, hp_elog);
+   rc = dlpar_parse_id_type(, _elog);
if (rc)
goto dlpar_store_out;
 
-   init_completion(_done);
-   queue_hotplug_event(hp_elog, _done, );
-   wait_for_completion(_done);
+   rc = handle_dlpar_errorlog(_elog);
 
 dlpar_store_out:
kfree(argbuf);
-   kfree(hp_elog);
 
if (rc)
pr_err("Could not handle DLPAR request \"%s\"\n", buf);
diff --git a/arch/powerpc/platforms/pseries/mobility.c 
b/arch/powerpc/platforms/pseries/mobility.c
index f0e30dc94988..6f27d00505cf 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -242,7 +242,7 @@ static int 

[PATCH v4 1/2] powerpc/pseries: Remove prrn_work workqueue

2018-08-30 Thread Nathan Fontenot
When a PRRN event is received we are already running in a worker
thread. Instead of spawning off another worker thread on the prrn_work
workqueue to handle the PRRN event we can just call the PRRN handler
routine directly.

With this update we can also pass the scope variable for the PRRN
event directly to the handler instead of it being a global variable.

Signed-off-by: John Allen 
Signed-off-by: Haren Myneni 
---
v4:
  - Remove prrn_work workqueue as suggested by Michael Ellerman
  - Make the PRRN event scope passed in as opposed to a global, suggested
by Michael Ellerman
v3:
  -Scrap the mutex as it only replicates existing workqueue behavior.
v2:
  -Unlock prrn_lock when PRRN operations are complete, not after handler is
   scheduled.
  -Remove call to flush_work, the previous broken method of serializing
   PRRN events.
---
 arch/powerpc/kernel/rtasd.c |   17 +++--
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c
index 44d66c33d59d..23b88b923f06 100644
--- a/arch/powerpc/kernel/rtasd.c
+++ b/arch/powerpc/kernel/rtasd.c
@@ -274,27 +274,16 @@ void pSeries_log_error(char *buf, unsigned int err_type, 
int fatal)
 }
 
 #ifdef CONFIG_PPC_PSERIES
-static s32 prrn_update_scope;
-
-static void prrn_work_fn(struct work_struct *work)
+static void handle_prrn_event(s32 scope)
 {
/*
 * For PRRN, we must pass the negative of the scope value in
 * the RTAS event.
 */
-   pseries_devicetree_update(-prrn_update_scope);
+   pseries_devicetree_update(-scope);
numa_update_cpu_topology(false);
 }
 
-static DECLARE_WORK(prrn_work, prrn_work_fn);
-
-static void prrn_schedule_update(u32 scope)
-{
-   flush_work(_work);
-   prrn_update_scope = scope;
-   schedule_work(_work);
-}
-
 static void handle_rtas_event(const struct rtas_error_log *log)
 {
if (rtas_error_type(log) != RTAS_TYPE_PRRN || !prrn_is_enabled())
@@ -303,7 +292,7 @@ static void handle_rtas_event(const struct rtas_error_log 
*log)
/* For PRRN Events the extended log length is used to denote
 * the scope for calling rtas update-nodes.
 */
-   prrn_schedule_update(rtas_error_extended_log_length(log));
+   handle_prrn_event(rtas_error_extended_log_length(log));
 }
 
 #else



[PATCH v4 0/2] powerpc/pseries: Improve serialization of PRRN events

2018-08-30 Thread Nathan Fontenot
Stress testing has uncovered issues with handling continuously queued PRRN
events. Running PRRN events in this way can seriously load the system given
the sheer volume of dlpar being handled. This patchset ensures that PRRN
events are handled more synchronously. It also updates dlpar invocation
so that it can be done directly instead of waiting on a workqueue.

-Nathan
---
v4:
  -Update patch 1/2 to remove prrn workqueue
  -Replace patch 2/2 to allow for direct dlpar invocation
v3:
  -Scrap the PRRN mutex as it only replicates existing workqueue behavior.
v2:
  -Unlock prrn_lock when PRRN operations are complete, not after handler is
   scheduled.
  -Remove call to flush_work, the previous broken method of serializing
   PRRN events.

Nathan Fontenot (2):
  powerpc/pseries: Remove prrn_work workqueue
  powerpc/pseries:Remove unneeded uses of dlpar work queue


 arch/powerpc/kernel/rtasd.c   |   17 ++---
 arch/powerpc/platforms/pseries/dlpar.c|   37 +++--
 arch/powerpc/platforms/pseries/mobility.c |   18 +-
 arch/powerpc/platforms/pseries/pseries.h  |5 ++--
 arch/powerpc/platforms/pseries/ras.c  |2 +-
 5 files changed, 22 insertions(+), 57 deletions(-)



Re: [PATCH RFCv2 6/6] memory-hotplug.txt: Add some details about locking internals

2018-08-30 Thread Pasha Tatashin

Reviewed-by: Pavel Tatashin 

On 8/21/18 6:44 AM, David Hildenbrand wrote:
> Let's document the magic a bit, especially why device_hotplug_lock is
> required when adding/removing memory and how it all play together with
> requests to online/offline memory from user space.
> 

Re: [PATCH RFCv2 5/6] powerpc/powernv: hold device_hotplug_lock in memtrace_offline_pages()

2018-08-30 Thread Pasha Tatashin
Reviewed-by: Pavel Tatashin 

On 8/21/18 6:44 AM, David Hildenbrand wrote:
> Let's perform all checking + offlining + removing under
> device_hotplug_lock, so nobody can mess with these devices via
> sysfs concurrently.
> 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: Michael Ellerman 
> Cc: Rashmica Gupta 
> Cc: Balbir Singh 
> Cc: Michael Neuling 
> Signed-off-by: David Hildenbrand 
> ---
>  arch/powerpc/platforms/powernv/memtrace.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/memtrace.c 
> b/arch/powerpc/platforms/powernv/memtrace.c
> index ef7181d4fe68..473e59842ec5 100644
> --- a/arch/powerpc/platforms/powernv/memtrace.c
> +++ b/arch/powerpc/platforms/powernv/memtrace.c
> @@ -74,9 +74,13 @@ static bool memtrace_offline_pages(u32 nid, u64 start_pfn, 
> u64 nr_pages)
>  {
>   u64 end_pfn = start_pfn + nr_pages - 1;
>  
> + lock_device_hotplug();
> +
>   if (walk_memory_range(start_pfn, end_pfn, NULL,
> - check_memblock_online))
> + check_memblock_online)) {
> + unlock_device_hotplug();
>   return false;
> + }
>  
>   walk_memory_range(start_pfn, end_pfn, (void *)MEM_GOING_OFFLINE,
> change_memblock_state);
> @@ -84,14 +88,16 @@ static bool memtrace_offline_pages(u32 nid, u64 
> start_pfn, u64 nr_pages)
>   if (offline_pages(start_pfn, nr_pages)) {
>   walk_memory_range(start_pfn, end_pfn, (void *)MEM_ONLINE,
> change_memblock_state);
> + unlock_device_hotplug();
>   return false;
>   }
>  
>   walk_memory_range(start_pfn, end_pfn, (void *)MEM_OFFLINE,
> change_memblock_state);
>  
> - remove_memory(nid, start_pfn << PAGE_SHIFT, nr_pages << PAGE_SHIFT);
> + __remove_memory(nid, start_pfn << PAGE_SHIFT, nr_pages << PAGE_SHIFT);
>  
> + unlock_device_hotplug();
>   return true;
>  }
>  
> 

Re: [PATCH RFCv2 4/6] powerpc/powernv: hold device_hotplug_lock when calling device_online()

2018-08-30 Thread Pasha Tatashin
Reviewed-by: Pavel Tatashin 

On 8/21/18 6:44 AM, David Hildenbrand wrote:
> device_online() should be called with device_hotplug_lock() held.
> 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: Michael Ellerman 
> Cc: Rashmica Gupta 
> Cc: Balbir Singh 
> Cc: Michael Neuling 
> Signed-off-by: David Hildenbrand 
> ---
>  arch/powerpc/platforms/powernv/memtrace.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/powernv/memtrace.c 
> b/arch/powerpc/platforms/powernv/memtrace.c
> index 8f1cd4f3bfd5..ef7181d4fe68 100644
> --- a/arch/powerpc/platforms/powernv/memtrace.c
> +++ b/arch/powerpc/platforms/powernv/memtrace.c
> @@ -229,9 +229,11 @@ static int memtrace_online(void)
>* we need to online the memory ourselves.
>*/
>   if (!memhp_auto_online) {
> + lock_device_hotplug();
>   walk_memory_range(PFN_DOWN(ent->start),
> PFN_UP(ent->start + ent->size - 1),
> NULL, online_mem_block);
> + unlock_device_hotplug();
>   }
>  
>   /*
> 

Re: [PATCH RFCv2 3/6] mm/memory_hotplug: fix online/offline_pages called w.o. mem_hotplug_lock

2018-08-30 Thread Pasha Tatashin
On 8/21/18 6:44 AM, David Hildenbrand wrote:
> There seem to be some problems as result of 30467e0b3be ("mm, hotplug:
> fix concurrent memory hot-add deadlock"), which tried to fix a possible
> lock inversion reported and discussed in [1] due to the two locks
>   a) device_lock()
>   b) mem_hotplug_lock
> 
> While add_memory() first takes b), followed by a) during
> bus_probe_device(), onlining of memory from user space first took b),
> followed by a), exposing a possible deadlock.
> 
> In [1], and it was decided to not make use of device_hotplug_lock, but
> rather to enforce a locking order.
> 
> The problems I spotted related to this:
> 
> 1. Memory block device attributes: While .state first calls
>mem_hotplug_begin() and the calls device_online() - which takes
>device_lock() - .online does no longer call mem_hotplug_begin(), so
>effectively calls online_pages() without mem_hotplug_lock.
> 
> 2. device_online() should be called under device_hotplug_lock, however
>onlining memory during add_memory() does not take care of that.
> 
> In addition, I think there is also something wrong about the locking in
> 
> 3. arch/powerpc/platforms/powernv/memtrace.c calls offline_pages()
>without locks. This was introduced after 30467e0b3be. And skimming over
>the code, I assume it could need some more care in regards to locking
>(e.g. device_online() called without device_hotplug_lock - but I'll
>not touch that for now).
> 
> Now that we hold the device_hotplug_lock when
> - adding memory (e.g. via add_memory()/add_memory_resource())
> - removing memory (e.g. via remove_memory())
> - device_online()/device_offline()
> 
> We can move mem_hotplug_lock usage back into
> online_pages()/offline_pages().
> 
> Why is mem_hotplug_lock still needed? Essentially to make
> get_online_mems()/put_online_mems() be very fast (relying on
> device_hotplug_lock would be very slow), and to serialize against
> addition of memory that does not create memory block devices (hmm).
> 
> [1] http://driverdev.linuxdriverproject.org/pipermail/ driverdev-devel/
> 2015-February/065324.html
> 
> This patch is partly based on a patch by Vitaly Kuznetsov.

Reviewed-by: Pavel Tatashin 

Re: [PATCH RFCv2 2/6] mm/memory_hotplug: make add_memory() take the device_hotplug_lock

2018-08-30 Thread Pasha Tatashin
On 8/21/18 6:44 AM, David Hildenbrand wrote:
> add_memory() currently does not take the device_hotplug_lock, however
> is aleady called under the lock from
>   arch/powerpc/platforms/pseries/hotplug-memory.c
>   drivers/acpi/acpi_memhotplug.c
> to synchronize against CPU hot-remove and similar.
> 
> In general, we should hold the device_hotplug_lock when adding memory
> to synchronize against online/offline request (e.g. from user space) -
> which already resulted in lock inversions due to device_lock() and
> mem_hotplug_lock - see 30467e0b3be ("mm, hotplug: fix concurrent memory
> hot-add deadlock"). add_memory()/add_memory_resource() will create memory
> block devices, so this really feels like the right thing to do.
> 
> Holding the device_hotplug_lock makes sure that a memory block device
> can really only be accessed (e.g. via .online/.state) from user space,
> once the memory has been fully added to the system.
> 
> The lock is not held yet in
>   drivers/xen/balloon.c
>   arch/powerpc/platforms/powernv/memtrace.c
>   drivers/s390/char/sclp_cmd.c
>   drivers/hv/hv_balloon.c
> So, let's either use the locked variants or take the lock.
> 
> Don't export add_memory_resource(), as it once was exported to be used
> by XEN, which is never built as a module. If somebody requires it, we
> also have to export a locked variant (as device_hotplug_lock is never
> exported).

Reviewed-by: Pavel Tatashin 

Re: [PATCH RFCv2 1/6] mm/memory_hotplug: make remove_memory() take the device_hotplug_lock

2018-08-30 Thread Pasha Tatashin
> +
> +void __ref remove_memory(int nid, u64 start, u64 size)

Remove __ref, otherwise looks good:

Reviewed-by: Pavel Tatashin 

> +{
> + lock_device_hotplug();
> + __remove_memory(nid, start, size);
> + unlock_device_hotplug();
> +}
>  EXPORT_SYMBOL_GPL(remove_memory);
>  #endif /* CONFIG_MEMORY_HOTREMOVE */
> 

Re: [PATCH RFCv2 0/6] mm: online/offline_pages called w.o. mem_hotplug_lock

2018-08-30 Thread Pasha Tatashin


On 8/30/18 8:31 AM, David Hildenbrand wrote:
> On 21.08.2018 12:44, David Hildenbrand wrote:
>> This is the same approach as in the first RFC, but this time without
>> exporting device_hotplug_lock (requested by Greg) and with some more
>> details and documentation regarding locking. Tested only on x86 so far.
>>
> 
> I'll be on vacation for two weeks starting on Saturday. If there are no
> comments I'll send this as !RFC once I return.
>
I am studying this series, will send my comments later today.

Pavel

Re: [PATCH v1] mm: relax deferred struct page requirements

2018-08-30 Thread Pasha Tatashin
Hi Jiri,

I believe this bug is fixed with this change:

d39f8fb4b7776dcb09ec3bf7a321547083078ee3
mm: make DEFERRED_STRUCT_PAGE_INIT explicitly depend on SPARSEMEM

I am not able to reproduce this problem on x86-32.

Pavel

On 8/30/18 10:35 AM, Pavel Tatashin wrote:
> Thank you Jiri, I am studying it.
> 
> Pavel
> 
> On 8/24/18 3:44 AM, Jiri Slaby wrote:
>> pasha.tatas...@oracle.com -> pavel.tatas...@microsoft.com
>>
>> due to
>>  550 5.1.1 Unknown recipient address.
>>
>>
>> On 08/24/2018, 09:32 AM, Jiri Slaby wrote:
>>> On 06/19/2018, 09:56 PM, Pavel Tatashin wrote:
 On Tue, Jun 19, 2018 at 9:50 AM Pavel Tatashin
  wrote:
>
> On Sat, Jun 16, 2018 at 4:04 AM Jiri Slaby  wrote:
>>
>> On 11/21/2017, 08:24 AM, Michal Hocko wrote:
>>> On Thu 16-11-17 20:46:01, Pavel Tatashin wrote:
 There is no need to have ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT,
 as all the page initialization code is in common code.

 Also, there is no need to depend on MEMORY_HOTPLUG, as initialization 
 code
 does not really use hotplug memory functionality. So, we can remove 
 this
 requirement as well.

 This patch allows to use deferred struct page initialization on all
 platforms with memblock allocator.

 Tested on x86, arm64, and sparc. Also, verified that code compiles on
 PPC with CONFIG_MEMORY_HOTPLUG disabled.
>>>
>>> There is slight risk that we will encounter corner cases on some
>>> architectures with weird memory layout/topology
>>
>> Which x86_32-pae seems to be. Many bad page state errors are emitted
>> during boot when this patch is applied:
>
> Hi Jiri,
>
> Thank you for reporting this bug.
>
> Because 32-bit systems are limited in the maximum amount of physical
> memory, they don't need deferred struct pages. So, we can add depends
> on 64BIT to DEFERRED_STRUCT_PAGE_INIT in mm/Kconfig.
>
> However, before we do this, I want to try reproducing this problem and
> root cause it, as it might expose a general problem that is not 32-bit
> specific.

 Hi Jiri,

 Could you please attach your config and full qemu arguments that you
 used to reproduce this bug.
>>>
>>> Hi,
>>>
>>> I seem I never replied. Attaching .config and the qemu cmdline:
>>> $ qemu-kvm -m 2000 -hda /dev/null -kernel bzImage
>>>
>>> "-m 2000" is important to reproduce.
>>>
>>> If I disable CONFIG_DEFERRED_STRUCT_PAGE_INIT (which the patch allowed
>>> to enable), the error goes away, of course.
>>>
>>> thanks,
>>>
>>
>>

Re: [PATCH v1] mm: relax deferred struct page requirements

2018-08-30 Thread Pasha Tatashin
Thank you Jiri, I am studying it.

Pavel

On 8/24/18 3:44 AM, Jiri Slaby wrote:
> pasha.tatas...@oracle.com -> pavel.tatas...@microsoft.com
> 
> due to
>  550 5.1.1 Unknown recipient address.
> 
> 
> On 08/24/2018, 09:32 AM, Jiri Slaby wrote:
>> On 06/19/2018, 09:56 PM, Pavel Tatashin wrote:
>>> On Tue, Jun 19, 2018 at 9:50 AM Pavel Tatashin
>>>  wrote:

 On Sat, Jun 16, 2018 at 4:04 AM Jiri Slaby  wrote:
>
> On 11/21/2017, 08:24 AM, Michal Hocko wrote:
>> On Thu 16-11-17 20:46:01, Pavel Tatashin wrote:
>>> There is no need to have ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT,
>>> as all the page initialization code is in common code.
>>>
>>> Also, there is no need to depend on MEMORY_HOTPLUG, as initialization 
>>> code
>>> does not really use hotplug memory functionality. So, we can remove this
>>> requirement as well.
>>>
>>> This patch allows to use deferred struct page initialization on all
>>> platforms with memblock allocator.
>>>
>>> Tested on x86, arm64, and sparc. Also, verified that code compiles on
>>> PPC with CONFIG_MEMORY_HOTPLUG disabled.
>>
>> There is slight risk that we will encounter corner cases on some
>> architectures with weird memory layout/topology
>
> Which x86_32-pae seems to be. Many bad page state errors are emitted
> during boot when this patch is applied:

 Hi Jiri,

 Thank you for reporting this bug.

 Because 32-bit systems are limited in the maximum amount of physical
 memory, they don't need deferred struct pages. So, we can add depends
 on 64BIT to DEFERRED_STRUCT_PAGE_INIT in mm/Kconfig.

 However, before we do this, I want to try reproducing this problem and
 root cause it, as it might expose a general problem that is not 32-bit
 specific.
>>>
>>> Hi Jiri,
>>>
>>> Could you please attach your config and full qemu arguments that you
>>> used to reproduce this bug.
>>
>> Hi,
>>
>> I seem I never replied. Attaching .config and the qemu cmdline:
>> $ qemu-kvm -m 2000 -hda /dev/null -kernel bzImage
>>
>> "-m 2000" is important to reproduce.
>>
>> If I disable CONFIG_DEFERRED_STRUCT_PAGE_INIT (which the patch allowed
>> to enable), the error goes away, of course.
>>
>> thanks,
>>
> 
> 

[PATCH AUTOSEL 4.14 57/67] powerpc/platforms/85xx: fix t1042rdb_diu.c build errors & warning

2018-08-30 Thread Sasha Levin
From: Randy Dunlap 

[ Upstream commit f5daf77a55ef0e695cc90c440ed6503073ac5e07 ]

Fix build errors and warnings in t1042rdb_diu.c by adding header files
and MODULE_LICENSE().

../arch/powerpc/platforms/85xx/t1042rdb_diu.c:152:1: warning: data definition 
has no type or storage class
 early_initcall(t1042rdb_diu_init);
../arch/powerpc/platforms/85xx/t1042rdb_diu.c:152:1: error: type defaults to 
'int' in declaration of 'early_initcall' [-Werror=implicit-int]
../arch/powerpc/platforms/85xx/t1042rdb_diu.c:152:1: warning: parameter names 
(without types) in function declaration

and
WARNING: modpost: missing MODULE_LICENSE() in 
arch/powerpc/platforms/85xx/t1042rdb_diu.o

Signed-off-by: Randy Dunlap 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Scott Wood 
Cc: Kumar Gala 
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Michael Ellerman 
Signed-off-by: Sasha Levin 
---
 arch/powerpc/platforms/85xx/t1042rdb_diu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/platforms/85xx/t1042rdb_diu.c 
b/arch/powerpc/platforms/85xx/t1042rdb_diu.c
index 58fa3d319f1c..dac36ba82fea 100644
--- a/arch/powerpc/platforms/85xx/t1042rdb_diu.c
+++ b/arch/powerpc/platforms/85xx/t1042rdb_diu.c
@@ -9,8 +9,10 @@
  * option) any later version.
  */
 
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -150,3 +152,5 @@ static int __init t1042rdb_diu_init(void)
 }
 
 early_initcall(t1042rdb_diu_init);
+
+MODULE_LICENSE("GPL");
-- 
2.17.1


[PATCH AUTOSEL 4.18 095/113] powerpc/platforms/85xx: fix t1042rdb_diu.c build errors & warning

2018-08-30 Thread Sasha Levin
From: Randy Dunlap 

[ Upstream commit f5daf77a55ef0e695cc90c440ed6503073ac5e07 ]

Fix build errors and warnings in t1042rdb_diu.c by adding header files
and MODULE_LICENSE().

../arch/powerpc/platforms/85xx/t1042rdb_diu.c:152:1: warning: data definition 
has no type or storage class
 early_initcall(t1042rdb_diu_init);
../arch/powerpc/platforms/85xx/t1042rdb_diu.c:152:1: error: type defaults to 
'int' in declaration of 'early_initcall' [-Werror=implicit-int]
../arch/powerpc/platforms/85xx/t1042rdb_diu.c:152:1: warning: parameter names 
(without types) in function declaration

and
WARNING: modpost: missing MODULE_LICENSE() in 
arch/powerpc/platforms/85xx/t1042rdb_diu.o

Signed-off-by: Randy Dunlap 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Scott Wood 
Cc: Kumar Gala 
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Michael Ellerman 
Signed-off-by: Sasha Levin 
---
 arch/powerpc/platforms/85xx/t1042rdb_diu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/platforms/85xx/t1042rdb_diu.c 
b/arch/powerpc/platforms/85xx/t1042rdb_diu.c
index 58fa3d319f1c..dac36ba82fea 100644
--- a/arch/powerpc/platforms/85xx/t1042rdb_diu.c
+++ b/arch/powerpc/platforms/85xx/t1042rdb_diu.c
@@ -9,8 +9,10 @@
  * option) any later version.
  */
 
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -150,3 +152,5 @@ static int __init t1042rdb_diu_init(void)
 }
 
 early_initcall(t1042rdb_diu_init);
+
+MODULE_LICENSE("GPL");
-- 
2.17.1


Re: [PATCH 3/5] drivers: clk-qoriq: Add clockgen support for lx2160a

2018-08-30 Thread Scott Wood
On Thu, 2018-08-30 at 12:39 -0500, Scott Wood wrote:
> On Thu, 2018-08-30 at 07:36 +, Vabhav Sharma wrote:
> > > -Original Message-
> > > From: linux-kernel-ow...@vger.kernel.org  > > ow...@vger.kernel.org> On Behalf Of Scott Wood
> > > Sent: Wednesday, August 29, 2018 5:49 AM
> > > To: Vabhav Sharma ; linux-
> > > ker...@vger.kernel.org; devicet...@vger.kernel.org; robh...@kernel.org;
> > > mark.rutl...@arm.com; linuxppc-dev@lists.ozlabs.org; linux-arm-
> > > ker...@lists.infradead.org; mturque...@baylibre.com; sb...@kernel.org;
> > > r...@rjwysocki.net; viresh.ku...@linaro.org; linux-...@vger.kernel.org;
> > > linux...@vger.kernel.org; linux-kernel-ow...@vger.kernel.org;
> > > catalin.mari...@arm.com; will.dea...@arm.com;
> > > gre...@linuxfoundation.org; a...@arndb.de;
> > > kstew...@linuxfoundation.org; yamada.masah...@socionext.com
> > > Cc: Yogesh Narayan Gaur ; Andy Tang
> > > ; Udit Kumar ;
> > > li...@armlinux.org.uk; Varun Sethi 
> > > Subject: Re: [PATCH 3/5] drivers: clk-qoriq: Add clockgen support for
> > > lx2160a
> > > 
> > > On Mon, 2018-08-20 at 12:17 +0530, Vabhav Sharma wrote:
> > > > From: Yogesh Gaur 
> > > > 
> > > > Add clockgen support for lx2160a.
> > > > Added entry for compat 'fsl,lx2160a-clockgen'.
> > > > As LX2160A is 16 core, so modified value for NUM_CMUX
> > > > 
> > > > Signed-off-by: Tang Yuantian 
> > > > Signed-off-by: Yogesh Gaur 
> > > > Signed-off-by: Vabhav Sharma 
> > > > ---
> > > >  drivers/clk/clk-qoriq.c | 14 +-
> > > >  drivers/cpufreq/qoriq-cpufreq.c |  1 +
> > > >  2 files changed, 14 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/clk/clk-qoriq.c b/drivers/clk/clk-qoriq.c index
> > > > 3a1812f..fc6e308 100644
> > > > --- a/drivers/clk/clk-qoriq.c
> > > > +++ b/drivers/clk/clk-qoriq.c
> > > > @@ -60,7 +60,7 @@ struct clockgen_muxinfo {  };
> > > > 
> > > >  #define NUM_HWACCEL5
> > > > -#define NUM_CMUX   8
> > > > +#define NUM_CMUX   16
> > > > 
> > > >  struct clockgen;
> > > > 
> > > > @@ -570,6 +570,17 @@ static const struct clockgen_chipinfo chipinfo[]
> > > > =
> > > > {
> > > > .flags = CG_VER3 | CG_LITTLE_ENDIAN,
> > > > },
> > > > {
> > > > +   .compat = "fsl,lx2160a-clockgen",
> > > > +   .cmux_groups = {
> > > > +   _cmux_cga12, _cmux_cgb
> > > > +   },
> > > > +   .cmux_to_group = {
> > > > +   0, 0, 0, 0, 1, 1, 1, 1, -1
> > > > +   },
> > > > +   .pll_mask = 0x37,
> > > > +   .flags = CG_VER3 | CG_LITTLE_ENDIAN,
> > > > +   },
> > > 
> > > Why are you increasing NUM_CMUX beyond 8 for a chip that only has 8
> > > entries in cmux_to_group?
> > 
> > Configuration is 16 cores,8 cluster with 2 cores in each cluster
> 
> So?  This is about cmuxes, not cores.  You're increasing the array without
> ever using the new size.

Oh, and you also broke p4080 which has 8 cmuxes but no -1 terminator, because
the array was of length 8.  Probably the array should be changed to NUM_CMUX+1
so every array can be -1 terminated.

-Scott



Re: [PATCH 3/5] drivers: clk-qoriq: Add clockgen support for lx2160a

2018-08-30 Thread Scott Wood
On Thu, 2018-08-30 at 07:36 +, Vabhav Sharma wrote:
> > -Original Message-
> > From: linux-kernel-ow...@vger.kernel.org  > ow...@vger.kernel.org> On Behalf Of Scott Wood
> > Sent: Wednesday, August 29, 2018 5:49 AM
> > To: Vabhav Sharma ; linux-
> > ker...@vger.kernel.org; devicet...@vger.kernel.org; robh...@kernel.org;
> > mark.rutl...@arm.com; linuxppc-dev@lists.ozlabs.org; linux-arm-
> > ker...@lists.infradead.org; mturque...@baylibre.com; sb...@kernel.org;
> > r...@rjwysocki.net; viresh.ku...@linaro.org; linux-...@vger.kernel.org;
> > linux...@vger.kernel.org; linux-kernel-ow...@vger.kernel.org;
> > catalin.mari...@arm.com; will.dea...@arm.com;
> > gre...@linuxfoundation.org; a...@arndb.de;
> > kstew...@linuxfoundation.org; yamada.masah...@socionext.com
> > Cc: Yogesh Narayan Gaur ; Andy Tang
> > ; Udit Kumar ;
> > li...@armlinux.org.uk; Varun Sethi 
> > Subject: Re: [PATCH 3/5] drivers: clk-qoriq: Add clockgen support for
> > lx2160a
> > 
> > On Mon, 2018-08-20 at 12:17 +0530, Vabhav Sharma wrote:
> > > From: Yogesh Gaur 
> > > 
> > > Add clockgen support for lx2160a.
> > > Added entry for compat 'fsl,lx2160a-clockgen'.
> > > As LX2160A is 16 core, so modified value for NUM_CMUX
> > > 
> > > Signed-off-by: Tang Yuantian 
> > > Signed-off-by: Yogesh Gaur 
> > > Signed-off-by: Vabhav Sharma 
> > > ---
> > >  drivers/clk/clk-qoriq.c | 14 +-
> > >  drivers/cpufreq/qoriq-cpufreq.c |  1 +
> > >  2 files changed, 14 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/clk/clk-qoriq.c b/drivers/clk/clk-qoriq.c index
> > > 3a1812f..fc6e308 100644
> > > --- a/drivers/clk/clk-qoriq.c
> > > +++ b/drivers/clk/clk-qoriq.c
> > > @@ -60,7 +60,7 @@ struct clockgen_muxinfo {  };
> > > 
> > >  #define NUM_HWACCEL  5
> > > -#define NUM_CMUX 8
> > > +#define NUM_CMUX 16
> > > 
> > >  struct clockgen;
> > > 
> > > @@ -570,6 +570,17 @@ static const struct clockgen_chipinfo chipinfo[] =
> > > {
> > >   .flags = CG_VER3 | CG_LITTLE_ENDIAN,
> > >   },
> > >   {
> > > + .compat = "fsl,lx2160a-clockgen",
> > > + .cmux_groups = {
> > > + _cmux_cga12, _cmux_cgb
> > > + },
> > > + .cmux_to_group = {
> > > + 0, 0, 0, 0, 1, 1, 1, 1, -1
> > > + },
> > > + .pll_mask = 0x37,
> > > + .flags = CG_VER3 | CG_LITTLE_ENDIAN,
> > > + },
> > 
> > Why are you increasing NUM_CMUX beyond 8 for a chip that only has 8
> > entries in cmux_to_group?
> 
> Configuration is 16 cores,8 cluster with 2 cores in each cluster

So?  This is about cmuxes, not cores.  You're increasing the array without
ever using the new size.

-Scott



Re: [PATCH] fsl: remove redundant pointer 'priv'

2018-08-30 Thread Li Yang
On Wed, Aug 29, 2018 at 4:29 AM Colin King  wrote:
>
> From: Colin Ian King 
>
> Pointer 'priv' is being assigned but is never used hence it is
> redundant and can be removed.
>
> Cleans up clang warning:
> variable 'priv' set but not used [-Wunused-but-set-variable]
>
> Signed-off-by: Colin Ian King 

Merged for next after updated the title prefix.

Thanks!
> ---
>  drivers/soc/fsl/dpio/dpio-driver.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/soc/fsl/dpio/dpio-driver.c 
> b/drivers/soc/fsl/dpio/dpio-driver.c
> index b60b77bfaffa..e58fcc9096e8 100644
> --- a/drivers/soc/fsl/dpio/dpio-driver.c
> +++ b/drivers/soc/fsl/dpio/dpio-driver.c
> @@ -50,13 +50,10 @@ static void unregister_dpio_irq_handlers(struct 
> fsl_mc_device *dpio_dev)
>
>  static int register_dpio_irq_handlers(struct fsl_mc_device *dpio_dev, int 
> cpu)
>  {
> -   struct dpio_priv *priv;
> int error;
> struct fsl_mc_device_irq *irq;
> cpumask_t mask;
>
> -   priv = dev_get_drvdata(_dev->dev);
> -
> irq = dpio_dev->irqs[0];
> error = devm_request_irq(_dev->dev,
>  irq->msi_desc->irq,
> --
> 2.17.1
>


Re: [PATCH] soc: fsl/qe: Use of_get_child_by_name helper

2018-08-30 Thread Li Yang
On Wed, Aug 29, 2018 at 9:49 PM Qiang Zhao  wrote:
>
> From: Rob Herring 
> date: 2018/8/30 4:04
>
> > To: Qiang Zhao 
> > Cc: linux-ker...@vger.kernel.org; Leo Li ;
> > linuxppc-dev@lists.ozlabs.org; linux-arm-ker...@lists.infradead.org
> > Subject: [PATCH] soc: fsl/qe: Use of_get_child_by_name helper
> >
> > Use the of_get_child_by_name() helper instead of open coding searching for 
> > the
> > 'firmware' child node. This removes directly accessing the name pointer as 
> > well.
> >
> > Cc: Qiang Zhao 
> > Cc: Li Yang 
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: linux-arm-ker...@lists.infradead.org
> > Signed-off-by: Rob Herring 

Merged for next.  Thanks.

Regards,
Leo


Re: [PATCH RFCv2 0/6] mm: online/offline_pages called w.o. mem_hotplug_lock

2018-08-30 Thread David Hildenbrand
On 21.08.2018 12:44, David Hildenbrand wrote:
> This is the same approach as in the first RFC, but this time without
> exporting device_hotplug_lock (requested by Greg) and with some more
> details and documentation regarding locking. Tested only on x86 so far.
> 

I'll be on vacation for two weeks starting on Saturday. If there are no
comments I'll send this as !RFC once I return.

Thanks!

> --
> 
> Reading through the code and studying how mem_hotplug_lock is to be used,
> I noticed that there are two places where we can end up calling
> device_online()/device_offline() - online_pages()/offline_pages() without
> the mem_hotplug_lock. And there are other places where we call
> device_online()/device_offline() without the device_hotplug_lock.
> 
> While e.g.
>   echo "online" > /sys/devices/system/memory/memory9/state
> is fine, e.g.
>   echo 1 > /sys/devices/system/memory/memory9/online
> Will not take the mem_hotplug_lock. However the device_lock() and
> device_hotplug_lock.
> 
> E.g. via memory_probe_store(), we can end up calling
> add_memory()->online_pages() without the device_hotplug_lock. So we can
> have concurrent callers in online_pages(). We e.g. touch in online_pages()
> basically unprotected zone->present_pages then.
> 
> Looks like there is a longer history to that (see Patch #2 for details),
> and fixing it to work the way it was intended is not really possible. We
> would e.g. have to take the mem_hotplug_lock in device/base/core.c, which
> sounds wrong.
> 
> Summary: We had a lock inversion on mem_hotplug_lock and device_lock().
> More details can be found in patch 3 and patch 6.
> 
> I propose the general rules (documentation added in patch 6):
> 
> 1. add_memory/add_memory_resource() must only be called with
>device_hotplug_lock.
> 2. remove_memory() must only be called with device_hotplug_lock. This is
>already documented and holds for all callers.
> 3. device_online()/device_offline() must only be called with
>device_hotplug_lock. This is already documented and true for now in core
>code. Other callers (related to memory hotplug) have to be fixed up.
> 4. mem_hotplug_lock is taken inside of add_memory/remove_memory/
>online_pages/offline_pages.
> 
> To me, this looks way cleaner than what we have right now (and easier to
> verify). And looking at the documentation of remove_memory, using
> lock_device_hotplug also for add_memory() feels natural.
> 
> 
> RFC -> RFCv2:
> - Don't export device_hotplug_lock, provide proper remove_memory/add_memory
>   wrappers.
> - Split up the patches a bit.
> - Try to improve powernv memtrace locking
> - Add some documentation for locking that matches my knowledge
> 
> David Hildenbrand (6):
>   mm/memory_hotplug: make remove_memory() take the device_hotplug_lock
>   mm/memory_hotplug: make add_memory() take the device_hotplug_lock
>   mm/memory_hotplug: fix online/offline_pages called w.o.
> mem_hotplug_lock
>   powerpc/powernv: hold device_hotplug_lock when calling device_online()
>   powerpc/powernv: hold device_hotplug_lock in memtrace_offline_pages()
>   memory-hotplug.txt: Add some details about locking internals
> 
>  Documentation/memory-hotplug.txt  | 39 +++-
>  arch/powerpc/platforms/powernv/memtrace.c | 14 +++--
>  .../platforms/pseries/hotplug-memory.c|  8 +--
>  drivers/acpi/acpi_memhotplug.c|  4 +-
>  drivers/base/memory.c | 22 +++
>  drivers/xen/balloon.c |  3 +
>  include/linux/memory_hotplug.h|  4 +-
>  mm/memory_hotplug.c   | 59 +++
>  8 files changed, 115 insertions(+), 38 deletions(-)
> 


-- 

Thanks,

David / dhildenb


[PATCH 1/2] usb: gadget: fsl_udc_core: check allocation return value and cleanup on failure

2018-08-30 Thread Nicholas Mc Guire
The allocation with fsl_alloc_request() and kmalloc() were unchecked
fixed this up with a NULL check and appropriate cleanup.

Additionally udc->ep_qh_size was reset to 0 on failure of allocation.
Similar udc->phy_mode is initially 0 (as udc_controller was
allocated with kzalloc in fsl_udc_probe()) so reset it to 0 as well
so that this function is side-effect free on failure. Not clear if
this is necessary or sensible as fsl_udc_release() probably can not
be called if fsl_udc_probe() failed - but it should not hurt.

Signed-off-by: Nicholas Mc Guire 
Fixes: b504882da5 ("USB: add Freescale high-speed USB SOC device controller 
driver")
---

Problem located with experimental coccinelle script

Patch was compile tested with: imx_v6_v7_defconfig (implies USB_FSL_USB2=y)
(with a large number of sparse warnings not related to the proposed change
 and one smatch warning)

Patch is against 4.19-rc1 (localversion-next is next-20180830)

 drivers/usb/gadget/udc/fsl_udc_core.c | 30 ++
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c 
b/drivers/usb/gadget/udc/fsl_udc_core.c
index be59309..e637afb 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2247,8 +2247,10 @@ static int struct_udc_setup(struct fsl_udc *udc,
udc->phy_mode = pdata->phy_mode;
 
udc->eps = kcalloc(udc->max_ep, sizeof(struct fsl_ep), GFP_KERNEL);
-   if (!udc->eps)
-   return -1;
+   if (!udc->eps) {
+   ERR("kmalloc udc endpoint status failed\n");
+   goto eps_alloc_failed;
+   }
 
/* initialized QHs, take care of alignment */
size = udc->max_ep * sizeof(struct ep_queue_head);
@@ -2262,8 +2264,7 @@ static int struct_udc_setup(struct fsl_udc *udc,
>ep_qh_dma, GFP_KERNEL);
if (!udc->ep_qh) {
ERR("malloc QHs for udc failed\n");
-   kfree(udc->eps);
-   return -1;
+   goto ep_queue_alloc_failed;
}
 
udc->ep_qh_size = size;
@@ -2272,8 +2273,17 @@ static int struct_udc_setup(struct fsl_udc *udc,
/* FIXME: fsl_alloc_request() ignores ep argument */
udc->status_req = container_of(fsl_alloc_request(NULL, GFP_KERNEL),
struct fsl_req, req);
+   if (!udc->status_req) {
+   ERR("kzalloc for udc status request failed\n");
+   goto udc_status_alloc_failed;
+   }
+
/* allocate a small amount of memory to get valid address */
udc->status_req->req.buf = kmalloc(8, GFP_KERNEL);
+   if (!udc->status_req->req.buf) {
+   ERR("kzalloc for udc request buffer failed\n");
+   goto udc_req_buf_alloc_failed;
+   }
 
udc->resume_state = USB_STATE_NOTATTACHED;
udc->usb_state = USB_STATE_POWERED;
@@ -2281,6 +2291,18 @@ static int struct_udc_setup(struct fsl_udc *udc,
udc->remote_wakeup = 0; /* default to 0 on reset */
 
return 0;
+
+udc_req_buf_alloc_failed:
+   kfree(udc->status_req);
+udc_status_alloc_failed:
+   kfree(udc->ep_qh);
+   udc->ep_qh_size = 0;
+ep_queue_alloc_failed:
+   kfree(udc->eps);
+eps_alloc_failed:
+   udc->phy_mode = 0;
+   return -1;
+
 }
 
 /*
-- 
2.1.4



[PATCH 2/2] usb: gadget: fsl_udc_core: fixup struct_udc_setup documentation

2018-08-30 Thread Nicholas Mc Guire
The original implementation from commit b504882da539
("USB: add Freescale high-speed USB SOC device controller driver")
returned NULL on failure and an allocated + initialized struct fsl_udc on
success. The current code introduced in commit 4365831dadfe
("USB: fsl_usb2_udc: Get max ep number from DCCPARAMS register") only
provides partial initialization as well as returning 0 on success and -1
on failures. The function documentation is updated accordingly.

Signed-off-by: Nicholas Mc Guire 
Fixes: 4365831dadfe ("USB: fsl_usb2_udc: Get max ep number from DCCPARAMS 
register")
---

Problem located during code-review

Patch is against 4.19-rc1 (localversion-next is next-20180830)

 drivers/usb/gadget/udc/fsl_udc_core.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c 
b/drivers/usb/gadget/udc/fsl_udc_core.c
index e637afb..680b46e 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2234,8 +2234,10 @@ static void fsl_udc_release(struct device *dev)
Internal structure setup functions
 ***/
 /*--
- * init resource for globle controller
- * Return the udc handle on success or NULL on failure
+ * init resource for global controller called by fsl_udc_probe()
+ * On success the udc handle is initialized, on failure it is
+ * unchanged (reset).
+ * Return 0 on success and -1 on allocation failure
  --*/
 static int struct_udc_setup(struct fsl_udc *udc,
struct platform_device *pdev)
-- 
2.1.4



Re: [PATCH 0/9] PCI hotplug diet

2018-08-30 Thread Andy Shevchenko
On Sun, Aug 19, 2018 at 5:35 PM Lukas Wunner  wrote:
>
> Here's PCI hotplug material I've prepared for v4.20:
>
> Patches [1/9] to [8/9] reduce the code by 488 lines.  There should be no
> loss of functionality and no new features, just less and simpler code.
>
> Patch [9/9] collects TODOs for hotplug drivers to spur contributions.
>
>
> I've assumed that my patch "PCI: pciehp: Differentiate between surprise
> and safe removal" (submitted July 31) gets merged before this series:
> https://patchwork.ozlabs.org/patch/951386/
>
> If that patch is not accepted or other pciehp-related patches are merged
> before this series, I'll have to rebase patch [4/9] ("PCI: pciehp: Unify
> controller and slot structs").  No big deal, just a heads-up.
>

Acked-by: Andy Shevchenko 

for PDx86 bits

> Thanks,
>
> Lukas
>
>
> Lukas Wunner (9):
>   PCI: Simplify disconnected marking
>   PCI: pciehp: Drop unnecessary includes
>   PCI: pciehp: Drop hotplug_slot_ops wrappers
>   PCI: pciehp: Unify controller and slot structs
>   PCI: pciehp: Reshuffle controller struct for clarity
>   PCI: hotplug: Constify hotplug_slot_ops
>   PCI: hotplug: Drop hotplug_slot_info
>   PCI: hotplug: Embed hotplug_slot
>   PCI: hotplug: Document TODOs
>
>  arch/powerpc/include/asm/pnv-pci.h  |   2 +-
>  drivers/pci/hotplug/TODO|  74 +++
>  drivers/pci/hotplug/acpiphp.h   |  10 +-
>  drivers/pci/hotplug/acpiphp_core.c  |  36 +---
>  drivers/pci/hotplug/acpiphp_ibm.c   |   2 +-
>  drivers/pci/hotplug/cpci_hotplug.h  |  11 +-
>  drivers/pci/hotplug/cpci_hotplug_core.c | 105 +++---
>  drivers/pci/hotplug/cpci_hotplug_pci.c  |   6 +-
>  drivers/pci/hotplug/cpqphp.h|   9 +-
>  drivers/pci/hotplug/cpqphp_core.c   |  59 ++
>  drivers/pci/hotplug/cpqphp_ctrl.c   |  31 +--
>  drivers/pci/hotplug/ibmphp.h|   9 +-
>  drivers/pci/hotplug/ibmphp_core.c   | 121 ---
>  drivers/pci/hotplug/ibmphp_ebda.c   |  70 +--
>  drivers/pci/hotplug/pci_hotplug_core.c  |  53 ++---
>  drivers/pci/hotplug/pciehp.h| 138 ++---
>  drivers/pci/hotplug/pciehp_core.c   | 127 +++-
>  drivers/pci/hotplug/pciehp_ctrl.c   | 255 +++-
>  drivers/pci/hotplug/pciehp_hpc.c| 127 
>  drivers/pci/hotplug/pciehp_pci.c|  24 +--
>  drivers/pci/hotplug/pnv_php.c   |  35 ++--
>  drivers/pci/hotplug/rpaphp.h|  10 +-
>  drivers/pci/hotplug/rpaphp_core.c   |  20 +-
>  drivers/pci/hotplug/rpaphp_pci.c|  11 +-
>  drivers/pci/hotplug/rpaphp_slot.c   |  22 +-
>  drivers/pci/hotplug/s390_pci_hpc.c  |  44 ++--
>  drivers/pci/hotplug/sgi_hotplug.c   |  63 +++---
>  drivers/pci/hotplug/shpchp.h|   8 +-
>  drivers/pci/hotplug/shpchp_core.c   |  48 ++---
>  drivers/pci/hotplug/shpchp_ctrl.c   |  21 +-
>  drivers/pci/pci.c   |   4 +-
>  drivers/pci/pcie/err.c  |   8 +-
>  drivers/pci/slot.c  |   2 +-
>  drivers/platform/x86/asus-wmi.c |  39 +---
>  drivers/platform/x86/eeepc-laptop.c |  43 ++--
>  include/linux/pci_hotplug.h |  43 +---
>  36 files changed, 638 insertions(+), 1052 deletions(-)
>  create mode 100644 drivers/pci/hotplug/TODO
>
> --
> 2.18.0
>


-- 
With Best Regards,
Andy Shevchenko


RE: [PATCH 3/5] drivers: clk-qoriq: Add clockgen support for lx2160a

2018-08-30 Thread Vabhav Sharma


> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org  ow...@vger.kernel.org> On Behalf Of Scott Wood
> Sent: Wednesday, August 29, 2018 5:49 AM
> To: Vabhav Sharma ; linux-
> ker...@vger.kernel.org; devicet...@vger.kernel.org; robh...@kernel.org;
> mark.rutl...@arm.com; linuxppc-dev@lists.ozlabs.org; linux-arm-
> ker...@lists.infradead.org; mturque...@baylibre.com; sb...@kernel.org;
> r...@rjwysocki.net; viresh.ku...@linaro.org; linux-...@vger.kernel.org;
> linux...@vger.kernel.org; linux-kernel-ow...@vger.kernel.org;
> catalin.mari...@arm.com; will.dea...@arm.com;
> gre...@linuxfoundation.org; a...@arndb.de;
> kstew...@linuxfoundation.org; yamada.masah...@socionext.com
> Cc: Yogesh Narayan Gaur ; Andy Tang
> ; Udit Kumar ;
> li...@armlinux.org.uk; Varun Sethi 
> Subject: Re: [PATCH 3/5] drivers: clk-qoriq: Add clockgen support for lx2160a
> 
> On Mon, 2018-08-20 at 12:17 +0530, Vabhav Sharma wrote:
> > From: Yogesh Gaur 
> >
> > Add clockgen support for lx2160a.
> > Added entry for compat 'fsl,lx2160a-clockgen'.
> > As LX2160A is 16 core, so modified value for NUM_CMUX
> >
> > Signed-off-by: Tang Yuantian 
> > Signed-off-by: Yogesh Gaur 
> > Signed-off-by: Vabhav Sharma 
> > ---
> >  drivers/clk/clk-qoriq.c | 14 +-
> >  drivers/cpufreq/qoriq-cpufreq.c |  1 +
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/clk-qoriq.c b/drivers/clk/clk-qoriq.c index
> > 3a1812f..fc6e308 100644
> > --- a/drivers/clk/clk-qoriq.c
> > +++ b/drivers/clk/clk-qoriq.c
> > @@ -60,7 +60,7 @@ struct clockgen_muxinfo {  };
> >
> >  #define NUM_HWACCEL5
> > -#define NUM_CMUX   8
> > +#define NUM_CMUX   16
> >
> >  struct clockgen;
> >
> > @@ -570,6 +570,17 @@ static const struct clockgen_chipinfo chipinfo[] = {
> > .flags = CG_VER3 | CG_LITTLE_ENDIAN,
> > },
> > {
> > +   .compat = "fsl,lx2160a-clockgen",
> > +   .cmux_groups = {
> > +   _cmux_cga12, _cmux_cgb
> > +   },
> > +   .cmux_to_group = {
> > +   0, 0, 0, 0, 1, 1, 1, 1, -1
> > +   },
> > +   .pll_mask = 0x37,
> > +   .flags = CG_VER3 | CG_LITTLE_ENDIAN,
> > +   },
> 
> Why are you increasing NUM_CMUX beyond 8 for a chip that only has 8
> entries in cmux_to_group?
Configuration is 16 cores,8 cluster with 2 cores in each cluster
> 
> -Scott