LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH -next] powerpc/pseries/memory-hotplug: Fix return value type of find_aa_index
From: Nathan Fontenot @ 2018-09-28 20:25 UTC (permalink / raw)
  To: YueHaibing, benh, paulus, mpe; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20180921103759.24976-1-yuehaibing@huawei.com>

On 09/21/2018 05:37 AM, YueHaibing wrote:
> find_aa_index will return -1 when dlpar_clone_property fails,
> its return value type should be int. Also the caller
> update_lmb_associativity_index should use a int variable to
> get it,then compared with 0.

The aa_index that we are handling here is defined as an unsigned value
in the PAPR so I'm a little hesitant in changing it to a signed value.
Also, changing the aa_index to be signed, we still assign it to the
u32 lmb->aa_index.

There are some other places where the aa_index is treated as a signed value
in finc_aa_index(). Perhaps the better solution is use an rc value to track
the validation of finding the aa_index instead of the aa_index value itself.

-Nathan 

> 
> Fixes: c05a5a40969e ("powerpc/pseries: Dynamic add entires to associativity lookup array")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  arch/powerpc/platforms/pseries/hotplug-memory.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index 9a15d39..6aad17c 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -101,13 +101,12 @@ static struct property *dlpar_clone_property(struct property *prop,
>  	return new_prop;
>  }
> 
> -static u32 find_aa_index(struct device_node *dr_node,
> +static int find_aa_index(struct device_node *dr_node,
>  			 struct property *ala_prop, const u32 *lmb_assoc)
>  {
>  	u32 *assoc_arrays;
> -	u32 aa_index;
>  	int aa_arrays, aa_array_entries, aa_array_sz;
> -	int i, index;
> +	int i, index, aa_index;
> 
>  	/*
>  	 * The ibm,associativity-lookup-arrays property is defined to be
> @@ -168,7 +167,7 @@ static int update_lmb_associativity_index(struct drmem_lmb *lmb)
>  	struct device_node *parent, *lmb_node, *dr_node;
>  	struct property *ala_prop;
>  	const u32 *lmb_assoc;
> -	u32 aa_index;
> +	int aa_index;
> 
>  	parent = of_find_node_by_path("/");
>  	if (!parent)
> 


^ permalink raw reply

* Re: [PATCH] powerpc/rtas: Fix a potential race between CPU-Offline & Migration
From: Nathan Fontenot @ 2018-09-28 20:36 UTC (permalink / raw)
  To: ego; +Cc: linuxppc-dev, Tyrel Datwyler
In-Reply-To: <20180928070244.GA6190@in.ibm.com>

On 09/28/2018 02:02 AM, Gautham R Shenoy wrote:
> Hi Nathan,
> 
> On Thu, Sep 27, 2018 at 12:31:34PM -0500, Nathan Fontenot wrote:
>> On 09/27/2018 11:51 AM, Gautham R. Shenoy wrote:
>>> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>>>
>>> Live Partition Migrations require all the present CPUs to execute the
>>> H_JOIN call, and hence rtas_ibm_suspend_me() onlines any offline CPUs
>>> before initiating the migration for this purpose.
>>>
>>> The commit 85a88cabad57
>>> ("powerpc/pseries: Disable CPU hotplug across migrations")
>>> disables any CPU-hotplug operations once all the offline CPUs are
>>> brought online to prevent any further state change. Once the
>>> CPU-Hotplug operation is disabled, the code assumes that all the CPUs
>>> are online.
>>>
>>> However, there is a minor window in rtas_ibm_suspend_me() between
>>> onlining the offline CPUs and disabling CPU-Hotplug when a concurrent
>>> CPU-offline operations initiated by the userspace can succeed thereby
>>> nullifying the the aformentioned assumption. In this unlikely case
>>> these offlined CPUs will not call H_JOIN, resulting in a system hang.
>>>
>>> Fix this by verifying that all the present CPUs are actually online
>>> after CPU-Hotplug has been disabled, failing which we return from
>>> rtas_ibm_suspend_me() with -EBUSY.
>>
>> Would we also want to havr the ability to re-try onlining all of the cpus
>> before failing the migration?
> 
> Given that we haven't been able to hit issue in practice after your
> fix to disable CPU Hotplug after migrations, it indicates that the
> race-window, if it is not merely a theoretical one, is extremely
> narrow. So, this current patch addresses the safety aspect, as in,
> should someone manage to exploit this narrow race-window, it ensures
> that the system doesn't go to a hang state.
> 
> Having the ability to retry onlining all the CPUs is only required for
> progress of LPM in this rarest of cases. We should add the code to
> retry onlining the CPUs if the consequence of failing an LPM is high,
> even in this rarest of case. Otherwise IMHO we should be ok not adding
> the additional code.

I believe you're correct. One small update to the patch below...

> 
>>
>> This would involve a bigger code change as the current code to online all
>> CPUs would work in its current form.
>>
>> -Nathan
>>
>>>
>>> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
>>> Cc: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
>>> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
>>> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
>>> ---
>>>  arch/powerpc/kernel/rtas.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
>>> index 2c7ed31..27f6fd3 100644
>>> --- a/arch/powerpc/kernel/rtas.c
>>> +++ b/arch/powerpc/kernel/rtas.c
>>> @@ -982,6 +982,16 @@ int rtas_ibm_suspend_me(u64 handle)
>>>  	}
>>>
>>>  	cpu_hotplug_disable();
>>> +
>>> +	/* Check if we raced with a CPU-Offline Operation */
>>> +	if (unlikely(!cpumask_equal(cpu_present_mask, cpu_online_mask))) {
>>> +		pr_err("%s: Raced against a concurrent CPU-Offline\n",
>>> +		       __func__);
>>> +		atomic_set(&data.error, -EBUSY);
>>> +		cpu_hotplug_enable();

Before returning, we return all CPUs that were offline prior to the migration
back to the offline state. We should be doing that here as well. This should
be as simple as adding a call to rtas_offline_cpus_mask() here.

-Nathan

>>> +		goto out;
>>> +	}
>>> +
>>>  	stop_topology_update();
>>>
>>>  	/* Call function on all CPUs.  One of us will make the
>>>


^ permalink raw reply

* Re: [PATCH v3 5/6] arm64: dts: add QorIQ LX2160A SoC support
From: Li Yang @ 2018-09-28 20:41 UTC (permalink / raw)
  To: vabhav.sharma
  Cc: Mark Rutland, Kate Stewart, yogeshnarayan.gaur,
	linux-kernel-owner, Catalin Marinas, Michael Turquette,
	Will Deacon, yamada.masahiro, Sriram Dash, linux-clk, udit.kumar,
	Russell King, Priyanka Jain, Viresh Kumar,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Arnd Bergmann, linux-pm, Scott Wood, Rob Herring, V.Sethi,
	nipun.gupta,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Ramneek Mehresh, sboyd, Greg Kroah-Hartman, Ying Zhang,
	Rafael J. Wysocki, lkml, Sudeep Holla, linuxppc-dev
In-Reply-To: <1537747741-6245-6-git-send-email-vabhav.sharma@nxp.com>

On Mon, Sep 24, 2018 at 7:47 AM Vabhav Sharma <vabhav.sharma@nxp.com> wrote:
>
> LX2160A SoC is based on Layerscape Chassis Generation 3.2 Architecture.
>
> LX2160A features an advanced 16 64-bit ARM v8 CortexA72 processor cores
> in 8 cluster, CCN508, GICv3,two 64-bit DDR4 memory controller, 8 I2C
> controllers, 3 dspi, 2 esdhc,2 USB 3.0, mmu 500, 3 SATA, 4 PL011 SBSA
> UARTs etc.
>
> Signed-off-by: Ramneek Mehresh <ramneek.mehresh@nxp.com>
> Signed-off-by: Zhang Ying-22455 <ying.zhang22455@nxp.com>
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> Signed-off-by: Priyanka Jain <priyanka.jain@nxp.com>
> Signed-off-by: Yogesh Gaur <yogeshnarayan.gaur@nxp.com>
> Signed-off-by: Sriram Dash <sriram.dash@nxp.com>
> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi | 693 +++++++++++++++++++++++++
>  1 file changed, 693 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> new file mode 100644
> index 0000000..46eea16
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> @@ -0,0 +1,693 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +//
> +// Device Tree Include file for Layerscape-LX2160A family SoC.
> +//
> +// Copyright 2018 NXP
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>

You included the header file, but you didn't use the MACROs in most of
the interrupts property below.  It is recommended to use them for
better readibity.

> +
> +/memreserve/ 0x80000000 0x00010000;
> +
> +/ {
> +       compatible = "fsl,lx2160a";
> +       interrupt-parent = <&gic>;
> +       #address-cells = <2>;
> +       #size-cells = <2>;
> +
> +       cpus {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               // 8 clusters having 2 Cortex-A72 cores each
> +               cpu@0 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x0>;
> +                       clocks = <&clockgen 1 0>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster0_l2>;

enable-method is a required property for this and cpu below.

> +               };
> +
> +               cpu@1 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x1>;
> +                       clocks = <&clockgen 1 0>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster0_l2>;
> +               };
> +
> +               cpu@100 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x100>;
> +                       clocks = <&clockgen 1 1>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster1_l2>;
> +               };
> +
> +               cpu@101 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x101>;
> +                       clocks = <&clockgen 1 1>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster1_l2>;
> +               };
> +
> +               cpu@200 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x200>;
> +                       clocks = <&clockgen 1 2>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster2_l2>;
> +               };
> +
> +               cpu@201 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x201>;
> +                       clocks = <&clockgen 1 2>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster2_l2>;
> +               };
> +
> +               cpu@300 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x300>;
> +                       clocks = <&clockgen 1 3>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster3_l2>;
> +               };
> +
> +               cpu@301 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x301>;
> +                       clocks = <&clockgen 1 3>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster3_l2>;
> +               };
> +
> +               cpu@400 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x400>;
> +                       clocks = <&clockgen 1 4>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster4_l2>;
> +               };
> +
> +               cpu@401 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x401>;
> +                       clocks = <&clockgen 1 4>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster4_l2>;
> +               };
> +
> +               cpu@500 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x500>;
> +                       clocks = <&clockgen 1 5>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster5_l2>;
> +               };
> +
> +               cpu@501 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x501>;
> +                       clocks = <&clockgen 1 5>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster5_l2>;
> +               };
> +
> +               cpu@600 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x600>;
> +                       clocks = <&clockgen 1 6>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster6_l2>;
> +               };
> +
> +               cpu@601 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x601>;
> +                       clocks = <&clockgen 1 6>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster6_l2>;
> +               };
> +
> +               cpu@700 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x700>;
> +                       clocks = <&clockgen 1 7>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster7_l2>;
> +               };
> +
> +               cpu@701 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72";
> +                       reg = <0x701>;
> +                       clocks = <&clockgen 1 7>;
> +                       d-cache-size = <0x8000>;
> +                       d-cache-line-size = <64>;
> +                       d-cache-sets = <128>;
> +                       i-cache-size = <0xC000>;
> +                       i-cache-line-size = <64>;
> +                       i-cache-sets = <192>;
> +                       next-level-cache = <&cluster7_l2>;
> +               };
> +
> +               cluster0_l2: l2-cache0 {
> +                       compatible = "cache";
> +                       cache-size = <0x100000>;
> +                       cache-line-size = <64>;
> +                       cache-sets = <1024>;
> +                       cache-level = <2>;
> +               };
> +
> +               cluster1_l2: l2-cache1 {
> +                       compatible = "cache";
> +                       cache-size = <0x100000>;
> +                       cache-line-size = <64>;
> +                       cache-sets = <1024>;
> +                       cache-level = <2>;
> +               };
> +
> +               cluster2_l2: l2-cache2 {
> +                       compatible = "cache";
> +                       cache-size = <0x100000>;
> +                       cache-line-size = <64>;
> +                       cache-sets = <1024>;
> +                       cache-level = <2>;
> +               };
> +
> +               cluster3_l2: l2-cache3 {
> +                       compatible = "cache";
> +                       cache-size = <0x100000>;
> +                       cache-line-size = <64>;
> +                       cache-sets = <1024>;
> +                       cache-level = <2>;
> +               };
> +
> +               cluster4_l2: l2-cache4 {
> +                       compatible = "cache";
> +                       cache-size = <0x100000>;
> +                       cache-line-size = <64>;
> +                       cache-sets = <1024>;
> +                       cache-level = <2>;
> +               };
> +
> +               cluster5_l2: l2-cache5 {
> +                       compatible = "cache";
> +                       cache-size = <0x100000>;
> +                       cache-line-size = <64>;
> +                       cache-sets = <1024>;
> +                       cache-level = <2>;
> +               };
> +
> +               cluster6_l2: l2-cache6 {
> +                       compatible = "cache";
> +                       cache-size = <0x100000>;
> +                       cache-line-size = <64>;
> +                       cache-sets = <1024>;
> +                       cache-level = <2>;
> +               };
> +
> +               cluster7_l2: l2-cache7 {
> +                       compatible = "cache";
> +                       cache-size = <0x100000>;
> +                       cache-line-size = <64>;
> +                       cache-sets = <1024>;
> +                       cache-level = <2>;
> +               };
> +       };
> +
> +       gic: interrupt-controller@6000000 {
> +               compatible = "arm,gic-v3";
> +               reg = <0x0 0x06000000 0 0x10000>, // GIC Dist
> +                       <0x0 0x06200000 0 0x200000>, // GICR (RD_base +
> +                                                    // SGI_base)
> +                       <0x0 0x0c0c0000 0 0x2000>, // GICC
> +                       <0x0 0x0c0d0000 0 0x1000>, // GICH
> +                       <0x0 0x0c0e0000 0 0x20000>; // GICV
> +               #interrupt-cells = <3>;
> +               #address-cells = <2>;
> +               #size-cells = <2>;
> +               ranges;
> +               interrupt-controller;
> +               interrupts = <1 9 0x4>;
> +
> +               its: gic-its@6020000 {
> +                       compatible = "arm,gic-v3-its";
> +                       msi-controller;
> +                       reg = <0x0 0x6020000 0 0x20000>;
> +               };
> +       };
> +
> +       rstcr: syscon@1e60000 {
> +               compatible = "syscon";
> +               reg = <0x0 0x1e60000 0x0 0x4>;
> +       };

This is no use if you don't have a syscon-reboot node pointing to it.

> +
> +       timer {
> +               compatible = "arm,armv8-timer";
> +               interrupts = <1 13 4>,
> +                            <1 14 4>,
> +                            <1 11 4>,
> +                            <1 10 4>;
> +       };
> +
> +       pmu {
> +               compatible = "arm,cortex-a72-pmu";
> +               interrupts = <1 7 0x8>; // PMU PPI, Level low type
> +       };
> +
> +       psci {
> +               compatible = "arm,psci-0.2";
> +               method = "smc";
> +       };
> +
> +       memory@80000000 {
> +               // DRAM space - 1, size : 2 GB DRAM
> +               device_type = "memory";
> +               reg = <0x00000000 0x80000000 0 0x80000000>;
> +       };
> +
> +       ddr1: memory-controller@1080000 {
> +               compatible = "fsl,qoriq-memory-controller";
> +               reg = <0x0 0x1080000 0x0 0x1000>;
> +               interrupts = <0 17 0x4>;
> +               little-endian;
> +       };
> +
> +       ddr2: memory-controller@1090000 {
> +               compatible = "fsl,qoriq-memory-controller";
> +               reg = <0x0 0x1090000 0x0 0x1000>;
> +               interrupts = <0 18 0x4>;
> +               little-endian;
> +       };
> +
> +       sysclk: sysclk {
> +               compatible = "fixed-clock";
> +               #clock-cells = <0>;
> +               clock-frequency = <100000000>;
> +               clock-output-names = "sysclk";
> +       };
> +
> +       soc {
> +               compatible = "simple-bus";
> +               #address-cells = <2>;
> +               #size-cells = <2>;
> +               ranges;
> +
> +               clockgen: clocking@1300000 {
> +                       compatible = "fsl,lx2160a-clockgen";

Also update the binding to include this new compatible.

> +                       reg = <0 0x1300000 0 0xa0000>;
> +                       #clock-cells = <2>;
> +                       clocks = <&sysclk>;
> +               };
> +
> +               crypto: crypto@8000000 {
> +                       compatible = "fsl,sec-v5.0", "fsl,sec-v4.0";
> +                       fsl,sec-era = <10>;
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +                       ranges = <0x0 0x00 0x8000000 0x100000>;
> +                       reg = <0x00 0x8000000 0x0 0x100000>;
> +                       interrupts = <GIC_SPI 139 IRQ_TYPE_LEVEL_HIGH>;
> +                       dma-coherent;
> +                       status = "disabled";
> +
> +                       sec_jr0: jr@10000 {
> +                               compatible = "fsl,sec-v5.0-job-ring",
> +                                            "fsl,sec-v4.0-job-ring";
> +                               reg        = <0x10000 0x10000>;
> +                               interrupts = <GIC_SPI 140 IRQ_TYPE_LEVEL_HIGH>;
> +                       };
> +
> +                       sec_jr1: jr@20000 {
> +                               compatible = "fsl,sec-v5.0-job-ring",
> +                                            "fsl,sec-v4.0-job-ring";
> +                               reg        = <0x20000 0x10000>;
> +                               interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>;
> +                       };
> +
> +                       sec_jr2: jr@30000 {
> +                               compatible = "fsl,sec-v5.0-job-ring",
> +                                            "fsl,sec-v4.0-job-ring";
> +                               reg        = <0x30000 0x10000>;
> +                               interrupts = <GIC_SPI 142 IRQ_TYPE_LEVEL_HIGH>;
> +                       };
> +
> +                       sec_jr3: jr@40000 {
> +                               compatible = "fsl,sec-v5.0-job-ring",
> +                                            "fsl,sec-v4.0-job-ring";
> +                               reg        = <0x40000 0x10000>;
> +                               interrupts = <GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>;
> +                       };
> +               };
> +
> +               dcfg: dcfg@1e00000 {
> +                       compatible = "fsl,lx2160a-dcfg", "syscon";

Update the binding to include this new compatible.

> +                       reg = <0x0 0x1e00000 0x0 0x10000>;
> +                       little-endian;
> +               };
> +
> +               gpio0: gpio@2300000 {
> +                       compatible = "fsl,qoriq-gpio";
> +                       reg = <0x0 0x2300000 0x0 0x10000>;
> +                       interrupts = <0 36 0x4>; // Level high type
> +                       gpio-controller;
> +                       little-endian;
> +                       #gpio-cells = <2>;
> +                       interrupt-controller;
> +                       #interrupt-cells = <2>;
> +               };
> +
> +               gpio1: gpio@2310000 {
> +                       compatible = "fsl,qoriq-gpio";
> +                       reg = <0x0 0x2310000 0x0 0x10000>;
> +                       interrupts = <0 36 0x4>; // Level high type
> +                       gpio-controller;
> +                       little-endian;
> +                       #gpio-cells = <2>;
> +                       interrupt-controller;
> +                       #interrupt-cells = <2>;
> +               };
> +
> +               gpio2: gpio@2320000 {
> +                       compatible = "fsl,qoriq-gpio";
> +                       reg = <0x0 0x2320000 0x0 0x10000>;
> +                       interrupts = <0 37 0x4>; // Level high type
> +                       gpio-controller;
> +                       little-endian;
> +                       #gpio-cells = <2>;
> +                       interrupt-controller;
> +                       #interrupt-cells = <2>;
> +               };
> +
> +               gpio3: gpio@2330000 {
> +                       compatible = "fsl,qoriq-gpio";
> +                       reg = <0x0 0x2330000 0x0 0x10000>;
> +                       interrupts = <0 37 0x4>; // Level high type
> +                       gpio-controller;
> +                       little-endian;
> +                       #gpio-cells = <2>;
> +                       interrupt-controller;
> +                       #interrupt-cells = <2>;
> +               };
> +
> +

One new line is enough.

> +               i2c0: i2c@2000000 {
> +                       compatible = "fsl,vf610-i2c";
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg = <0x0 0x2000000 0x0 0x10000>;
> +                       interrupts = <0 34 0x4>; // Level high type
> +                       clock-names = "i2c";
> +                       clocks = <&clockgen 4 7>;
> +                       fsl-scl-gpio = <&gpio2 15 0>;
> +                       status = "disabled";
> +               };
> +
> +               i2c1: i2c@2010000 {
> +                       compatible = "fsl,vf610-i2c";
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg = <0x0 0x2010000 0x0 0x10000>;
> +                       interrupts = <0 34 0x4>; // Level high type
> +                       clock-names = "i2c";
> +                       clocks = <&clockgen 4 7>;
> +                       status = "disabled";
> +               };
> +
> +               i2c2: i2c@2020000 {
> +                       compatible = "fsl,vf610-i2c";
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg = <0x0 0x2020000 0x0 0x10000>;
> +                       interrupts = <0 35 0x4>; // Level high type
> +                       clock-names = "i2c";
> +                       clocks = <&clockgen 4 7>;
> +                       status = "disabled";
> +               };
> +
> +               i2c3: i2c@2030000 {
> +                       compatible = "fsl,vf610-i2c";
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg = <0x0 0x2030000 0x0 0x10000>;
> +                       interrupts = <0 35 0x4>; // Level high type
> +                       clock-names = "i2c";
> +                       clocks = <&clockgen 4 7>;
> +                       status = "disabled";
> +               };
> +
> +               i2c4: i2c@2040000 {
> +                       compatible = "fsl,vf610-i2c";
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg = <0x0 0x2040000 0x0 0x10000>;
> +                       interrupts = <0 74 0x4>; // Level high type
> +                       clock-names = "i2c";
> +                       clocks = <&clockgen 4 7>;
> +                       fsl-scl-gpio = <&gpio2 16 0>;
> +                       status = "disabled";
> +               };
> +
> +               i2c5: i2c@2050000 {
> +                       compatible = "fsl,vf610-i2c";
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg = <0x0 0x2050000 0x0 0x10000>;
> +                       interrupts = <0 74 0x4>; // Level high type
> +                       clock-names = "i2c";
> +                       clocks = <&clockgen 4 7>;
> +                       status = "disabled";
> +               };
> +
> +               i2c6: i2c@2060000 {
> +                       compatible = "fsl,vf610-i2c";
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg = <0x0 0x2060000 0x0 0x10000>;
> +                       interrupts = <0 75 0x4>; // Level high type
> +                       clock-names = "i2c";
> +                       clocks = <&clockgen 4 7>;
> +                       status = "disabled";
> +               };
> +
> +               i2c7: i2c@2070000 {
> +                       compatible = "fsl,vf610-i2c";
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg = <0x0 0x2070000 0x0 0x10000>;
> +                       interrupts = <0 75 0x4>; // Level high type
> +                       clock-names = "i2c";
> +                       clocks = <&clockgen 4 7>;
> +                       status = "disabled";
> +               };
> +
> +               uart0: serial@21c0000 {
> +                       device_type = "serial";
> +                       compatible = "arm,pl011","arm,sbsa-uart";

From the sbsa_uart binding:
This UART uses a subset of the PL011 registers and consequently lives
in the PL011 driver. It's baudrate and other communication parameters
cannot be adjusted at runtime, so it lacks a clock specifier here.

So it is a more specific variant of pl011.  Put the sbsa-uart compatible first.

> +                       reg = <0x0 0x21c0000 0x0 0x1000>;
> +                       interrupts = <0 32 0x4>;        // Level high type
> +                       current-speed = <115200>;
> +                       status = "disabled";
> +               };
> +
> +               uart1: serial@21d0000 {
> +                       device_type = "serial";
> +                       compatible = "arm,pl011","arm,sbsa-uart";
> +                       reg = <0x0 0x21d0000 0x0 0x1000>;
> +                       interrupts = <0 33 0x4>;        // Level high type
> +                       current-speed = <115200>;
> +                       status = "disabled";
> +               };
> +
> +               uart2: serial@21e0000 {
> +                       device_type = "serial";
> +                       compatible = "arm,pl011","arm,sbsa-uart";
> +                       reg = <0x0 0x21e0000 0x0 0x1000>;
> +                       interrupts = <0 72 0x4>;        // Level high type
> +                       current-speed = <115200>;
> +                       status = "disabled";
> +               };
> +
> +               uart3: serial@21f0000 {
> +                       device_type = "serial";
> +                       compatible = "arm,pl011","arm,sbsa-uart";
> +                       reg = <0x0 0x21f0000 0x0 0x1000>;
> +                       interrupts = <0 73 0x4>;        // Level high type
> +                       current-speed = <115200>;
> +                       status = "disabled";
> +               };
> +
> +               smmu: iommu@5000000 {
> +                       compatible = "arm,mmu-500";
> +                       reg = <0 0x5000000 0 0x800000>;
> +                       #iommu-cells = <1>;
> +                       #global-interrupts = <14>;
> +                       interrupts = <0 13 4>, // global secure fault
> +                                    <0 14 4>, // combined secure interrupt
> +                                    <0 15 4>, // global non-secure fault
> +                                    <0 16 4>, // combined non-secure interrupt
> +                               // performance counter interrupts 0-9
> +                                    <0 211 4>, <0 212 4>,
> +                                    <0 213 4>, <0 214 4>,
> +                                    <0 215 4>, <0 216 4>,
> +                                    <0 217 4>, <0 218 4>,
> +                                    <0 219 4>, <0 220 4>,
> +                               // per context interrupt, 64 interrupts
> +                                    <0 146 4>, <0 147 4>,
> +                                    <0 148 4>, <0 149 4>,
> +                                    <0 150 4>, <0 151 4>,
> +                                    <0 152 4>, <0 153 4>,
> +                                    <0 154 4>, <0 155 4>,
> +                                    <0 156 4>, <0 157 4>,
> +                                    <0 158 4>, <0 159 4>,
> +                                    <0 160 4>, <0 161 4>,
> +                                    <0 162 4>, <0 163 4>,
> +                                    <0 164 4>, <0 165 4>,
> +                                    <0 166 4>, <0 167 4>,
> +                                    <0 168 4>, <0 169 4>,
> +                                    <0 170 4>, <0 171 4>,
> +                                    <0 172 4>, <0 173 4>,
> +                                    <0 174 4>, <0 175 4>,
> +                                    <0 176 4>, <0 177 4>,
> +                                    <0 178 4>, <0 179 4>,
> +                                    <0 180 4>, <0 181 4>,
> +                                    <0 182 4>, <0 183 4>,
> +                                    <0 184 4>, <0 185 4>,
> +                                    <0 186 4>, <0 187 4>,
> +                                    <0 188 4>, <0 189 4>,
> +                                    <0 190 4>, <0 191 4>,
> +                                    <0 192 4>, <0 193 4>,
> +                                    <0 194 4>, <0 195 4>,
> +                                    <0 196 4>, <0 197 4>,
> +                                    <0 198 4>, <0 199 4>,
> +                                    <0 200 4>, <0 201 4>,
> +                                    <0 202 4>, <0 203 4>,
> +                                    <0 204 4>, <0 205 4>,
> +                                    <0 206 4>, <0 207 4>,
> +                                    <0 208 4>, <0 209 4>;
> +                       dma-coherent;
> +               };
> +
> +               usb0: usb3@3100000 {
> +                       compatible = "snps,dwc3";
> +                       reg = <0x0 0x3100000 0x0 0x10000>;
> +                       interrupts = <0 80 0x4>; // Level high type
> +                       dr_mode = "host";
> +                       snps,quirk-frame-length-adjustment = <0x20>;
> +                       snps,dis_rxdet_inp3_quirk;
> +                       status = "disabled";
> +               };
> +
> +               usb1: usb3@3110000 {
> +                       compatible = "snps,dwc3";
> +                       reg = <0x0 0x3110000 0x0 0x10000>;
> +                       interrupts = <0 81 0x4>; // Level high type
> +                       dr_mode = "host";
> +                       snps,quirk-frame-length-adjustment = <0x20>;
> +                       snps,dis_rxdet_inp3_quirk;
> +                       status = "disabled";
> +               };
> +
> +               watchdog@23a0000 {
> +                       compatible = "arm,sbsa-gwdt";
> +                       reg = <0x0 0x23a0000 0 0x1000>,
> +                             <0x0 0x2390000 0 0x1000>;
> +                       interrupts = <0 59 4>;
> +                       timeout-sec = <30>;
> +               };
> +

No new line is needed here.

> +       };
> +};
> --
> 2.7.4
>

^ permalink raw reply

* Re: drivers binding to device node with multiple compatible strings
From: Li Yang @ 2018-09-28 21:00 UTC (permalink / raw)
  To: Rob Herring
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linuxppc-dev, lkml, frowand.list,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAL_JsqKRH8uEzSx6H6ZSRJF9Ecp4am1cNsxgQqf2OO5_aGR1zw@mail.gmail.com>

On Fri, Sep 28, 2018 at 3:07 PM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Thu, Sep 27, 2018 at 5:25 PM Li Yang <leoyang.li@nxp.com> wrote:
> >
> > Hi Rob and Grant,
> >
> > Various device tree specs are recommending to include all the
> > potential compatible strings in the device node, with the order from
> > most specific to most general.  But it looks like Linux kernel doesn't
> > provide a way to bind the device to the most specific driver, however,
> > the first registered compatible driver will be bound.
> >
> > As more and more generic drivers are added to the Linux kernel, they
> > are competing with the more specific vendor drivers and causes problem
> > when both are built into the kernel.  I'm wondering if there is a
> > generic solution (or in plan) to make the most specific driver bound
> > to the device.   Or we have to disable the more general driver or
> > remove the more general compatible string from the device tree?
>
> It's been a known limitation for a long time. However, in practice it
> doesn't seem to be a common problem. Perhaps folks just remove the
> less specific compatible from their DT (though that's not ideal). For
> most modern bindings, there's so many other resources beyond
> compatible (clocks, resets, pinctrl, etc.) that there are few generic
> drivers that can work.
>
> I guess if we want to fix this, we'd need to have weighted matching in
> the driver core and unbind drivers when we get a better match. Though
> it could get messy if the better driver probe fails. Then we've got to
> rebind to the original driver.

Probably we can populate the platform devices from device tree after
the device_init phase?  So that all built-in drivers are already
registered when the devices are created and we can try find the best
match in one go?  For more specific loadable modules we probably need
to unbind from the old driver and bind to the new one.  But I agree
with you that it could be messy.

>
> Do you have a specific case where you hit this?

Maybe not a new issue but "snps,dw-pcie" is competing with various
"fsl,<chip>-pcie" compatibles.  Also a specific PHY
"ethernet-phy-idAAAA.BBBB" with generic "ethernet-phy-ieee802.3-c45".

Regards,
Leo

^ permalink raw reply

* Re: drivers binding to device node with multiple compatible strings
From: Li Yang @ 2018-09-28 21:19 UTC (permalink / raw)
  To: Rob Herring
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linuxppc-dev, lkml, frowand.list,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CADRPPNQam-QUfqDMvu42KS+50VaHfGPhCZj-1jRHQ-Od0fbxuQ@mail.gmail.com>

On Fri, Sep 28, 2018 at 4:00 PM Li Yang <leoyang.li@nxp.com> wrote:
>
> On Fri, Sep 28, 2018 at 3:07 PM Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Thu, Sep 27, 2018 at 5:25 PM Li Yang <leoyang.li@nxp.com> wrote:
> > >
> > > Hi Rob and Grant,
> > >
> > > Various device tree specs are recommending to include all the
> > > potential compatible strings in the device node, with the order from
> > > most specific to most general.  But it looks like Linux kernel doesn't
> > > provide a way to bind the device to the most specific driver, however,
> > > the first registered compatible driver will be bound.
> > >
> > > As more and more generic drivers are added to the Linux kernel, they
> > > are competing with the more specific vendor drivers and causes problem
> > > when both are built into the kernel.  I'm wondering if there is a
> > > generic solution (or in plan) to make the most specific driver bound
> > > to the device.   Or we have to disable the more general driver or
> > > remove the more general compatible string from the device tree?
> >
> > It's been a known limitation for a long time. However, in practice it
> > doesn't seem to be a common problem. Perhaps folks just remove the
> > less specific compatible from their DT (though that's not ideal). For
> > most modern bindings, there's so many other resources beyond
> > compatible (clocks, resets, pinctrl, etc.) that there are few generic
> > drivers that can work.
> >
> > I guess if we want to fix this, we'd need to have weighted matching in
> > the driver core and unbind drivers when we get a better match. Though
> > it could get messy if the better driver probe fails. Then we've got to
> > rebind to the original driver.
>
> Probably we can populate the platform devices from device tree after
> the device_init phase?  So that all built-in drivers are already
> registered when the devices are created and we can try find the best
> match in one go?  For more specific loadable modules we probably need
> to unbind from the old driver and bind to the new one.  But I agree
> with you that it could be messy.
>
> >
> > Do you have a specific case where you hit this?
>
> Maybe not a new issue but "snps,dw-pcie" is competing with various
> "fsl,<chip>-pcie" compatibles.  Also a specific PHY
> "ethernet-phy-idAAAA.BBBB" with generic "ethernet-phy-ieee802.3-c45".

The ethernet-phy issue is not related to the general device binding
framework, it should be an issue with the of_mdio framework. But it is
still a inalignment with the device tree recommendation.

Regards,
Leo

^ permalink raw reply

* Re: [PATCH] tty: Convert to using %pOFn instead of device_node.name
From: Rob Herring @ 2018-09-28 22:09 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org, Greg Kroah-Hartman
  Cc: Jiri Slaby, Paul Mackerras, open list:SERIAL DRIVERS,
	linuxppc-dev
In-Reply-To: <20180828015252.28511-46-robh@kernel.org>

On Mon, Aug 27, 2018 at 8:55 PM Rob Herring <robh@kernel.org> wrote:
>
> In preparation to remove the node name pointer from struct device_node,
> convert printf users to use the %pOFn format specifier.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linux-serial@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/tty/ehv_bytechan.c                  | 12 ++++++------
>  drivers/tty/serial/cpm_uart/cpm_uart_core.c |  8 ++++----
>  drivers/tty/serial/pmac_zilog.c             |  4 ++--
>  3 files changed, 12 insertions(+), 12 deletions(-)

Hey Greg, Is this still in your queue? Maybe you've just been extra
busy lately. ;)

Rob
>
> diff --git a/drivers/tty/ehv_bytechan.c b/drivers/tty/ehv_bytechan.c
> index eea4049b5dcc..769e0a5d1dfc 100644
> --- a/drivers/tty/ehv_bytechan.c
> +++ b/drivers/tty/ehv_bytechan.c
> @@ -128,8 +128,8 @@ static int find_console_handle(void)
>          */
>         iprop = of_get_property(np, "hv-handle", NULL);
>         if (!iprop) {
> -               pr_err("ehv-bc: no 'hv-handle' property in %s node\n",
> -                      np->name);
> +               pr_err("ehv-bc: no 'hv-handle' property in %pOFn node\n",
> +                      np);
>                 return 0;
>         }
>         stdout_bc = be32_to_cpu(*iprop);
> @@ -661,8 +661,8 @@ static int ehv_bc_tty_probe(struct platform_device *pdev)
>
>         iprop = of_get_property(np, "hv-handle", NULL);
>         if (!iprop) {
> -               dev_err(&pdev->dev, "no 'hv-handle' property in %s node\n",
> -                       np->name);
> +               dev_err(&pdev->dev, "no 'hv-handle' property in %pOFn node\n",
> +                       np);
>                 return -ENODEV;
>         }
>
> @@ -682,8 +682,8 @@ static int ehv_bc_tty_probe(struct platform_device *pdev)
>         bc->rx_irq = irq_of_parse_and_map(np, 0);
>         bc->tx_irq = irq_of_parse_and_map(np, 1);
>         if ((bc->rx_irq == NO_IRQ) || (bc->tx_irq == NO_IRQ)) {
> -               dev_err(&pdev->dev, "no 'interrupts' property in %s node\n",
> -                       np->name);
> +               dev_err(&pdev->dev, "no 'interrupts' property in %pOFn node\n",
> +                       np);
>                 ret = -ENODEV;
>                 goto error;
>         }
> diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_core.c b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
> index 24a5f05e769b..ea7204d75022 100644
> --- a/drivers/tty/serial/cpm_uart/cpm_uart_core.c
> +++ b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
> @@ -1151,8 +1151,8 @@ static int cpm_uart_init_port(struct device_node *np,
>         if (!pinfo->clk) {
>                 data = of_get_property(np, "fsl,cpm-brg", &len);
>                 if (!data || len != 4) {
> -                       printk(KERN_ERR "CPM UART %s has no/invalid "
> -                                       "fsl,cpm-brg property.\n", np->name);
> +                       printk(KERN_ERR "CPM UART %pOFn has no/invalid "
> +                                       "fsl,cpm-brg property.\n", np);
>                         return -EINVAL;
>                 }
>                 pinfo->brg = *data;
> @@ -1160,8 +1160,8 @@ static int cpm_uart_init_port(struct device_node *np,
>
>         data = of_get_property(np, "fsl,cpm-command", &len);
>         if (!data || len != 4) {
> -               printk(KERN_ERR "CPM UART %s has no/invalid "
> -                               "fsl,cpm-command property.\n", np->name);
> +               printk(KERN_ERR "CPM UART %pOFn has no/invalid "
> +                               "fsl,cpm-command property.\n", np);
>                 return -EINVAL;
>         }
>         pinfo->command = *data;
> diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
> index 3d21790d961e..a4ec22d1f214 100644
> --- a/drivers/tty/serial/pmac_zilog.c
> +++ b/drivers/tty/serial/pmac_zilog.c
> @@ -1566,9 +1566,9 @@ static int pmz_attach(struct macio_dev *mdev, const struct of_device_id *match)
>          * to work around bugs in ancient Apple device-trees
>          */
>         if (macio_request_resources(uap->dev, "pmac_zilog"))
> -               printk(KERN_WARNING "%s: Failed to request resource"
> +               printk(KERN_WARNING "%pOFn: Failed to request resource"
>                        ", port still active\n",
> -                      uap->node->name);
> +                      uap->node);
>         else
>                 uap->flags |= PMACZILOG_FLAG_RSRC_REQUESTED;
>
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH] tty: Convert to using %pOFn instead of device_node.name
From: Rob Herring @ 2018-09-28 22:32 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org, Greg Kroah-Hartman
  Cc: Jiri Slaby, Paul Mackerras, open list:SERIAL DRIVERS,
	linuxppc-dev
In-Reply-To: <CAL_JsqJfAsv-MOgk2Ycs2VBj2vK0m29B_3FxwJV5xUjnMbH4sg@mail.gmail.com>

On Fri, Sep 28, 2018 at 5:09 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Aug 27, 2018 at 8:55 PM Rob Herring <robh@kernel.org> wrote:
> >
> > In preparation to remove the node name pointer from struct device_node,
> > convert printf users to use the %pOFn format specifier.
> >
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Jiri Slaby <jslaby@suse.com>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: linux-serial@vger.kernel.org
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> >  drivers/tty/ehv_bytechan.c                  | 12 ++++++------
> >  drivers/tty/serial/cpm_uart/cpm_uart_core.c |  8 ++++----
> >  drivers/tty/serial/pmac_zilog.c             |  4 ++--
> >  3 files changed, 12 insertions(+), 12 deletions(-)
>
> Hey Greg, Is this still in your queue? Maybe you've just been extra
> busy lately. ;)

NM. I see it's applied now. Sorry for the noise.

Rob

^ permalink raw reply

* [PATCH v2] i2c: Convert to using %pOFn instead of device_node.name
From: Rob Herring @ 2018-09-28 22:49 UTC (permalink / raw)
  To: linux-kernel; +Cc: Paul Mackerras, linux-i2c, linuxppc-dev, Peter Rosin

In preparation to remove the node name pointer from struct device_node,
convert printf users to use the %pOFn format specifier.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Peter Rosin <peda@axentia.se>
Cc: linux-i2c@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
v2:
- Remove initialization of parent

 drivers/i2c/busses/i2c-powermac.c | 17 +++++++++--------
 drivers/i2c/muxes/i2c-mux-gpmux.c |  4 ++--
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-powermac.c b/drivers/i2c/busses/i2c-powermac.c
index f2a2067525ef..f6f4ed8afc93 100644
--- a/drivers/i2c/busses/i2c-powermac.c
+++ b/drivers/i2c/busses/i2c-powermac.c
@@ -388,9 +388,8 @@ static void i2c_powermac_register_devices(struct i2c_adapter *adap,
 static int i2c_powermac_probe(struct platform_device *dev)
 {
 	struct pmac_i2c_bus *bus = dev_get_platdata(&dev->dev);
-	struct device_node *parent = NULL;
+	struct device_node *parent;
 	struct i2c_adapter *adapter;
-	const char *basename;
 	int rc;
 
 	if (bus == NULL)
@@ -407,23 +406,25 @@ static int i2c_powermac_probe(struct platform_device *dev)
 		parent = of_get_parent(pmac_i2c_get_controller(bus));
 		if (parent == NULL)
 			return -EINVAL;
-		basename = parent->name;
+		snprintf(adapter->name, sizeof(adapter->name), "%pOFn %d",
+			 parent,
+			 pmac_i2c_get_channel(bus));
+		of_node_put(parent);
 		break;
 	case pmac_i2c_bus_pmu:
-		basename = "pmu";
+		snprintf(adapter->name, sizeof(adapter->name), "pmu %d",
+			 pmac_i2c_get_channel(bus));
 		break;
 	case pmac_i2c_bus_smu:
 		/* This is not what we used to do but I'm fixing drivers at
 		 * the same time as this change
 		 */
-		basename = "smu";
+		snprintf(adapter->name, sizeof(adapter->name), "smu %d",
+			 pmac_i2c_get_channel(bus));
 		break;
 	default:
 		return -EINVAL;
 	}
-	snprintf(adapter->name, sizeof(adapter->name), "%s %d", basename,
-		 pmac_i2c_get_channel(bus));
-	of_node_put(parent);
 
 	platform_set_drvdata(dev, adapter);
 	adapter->algo = &i2c_powermac_algorithm;
diff --git a/drivers/i2c/muxes/i2c-mux-gpmux.c b/drivers/i2c/muxes/i2c-mux-gpmux.c
index 92cf5f48afe6..f60b670deff7 100644
--- a/drivers/i2c/muxes/i2c-mux-gpmux.c
+++ b/drivers/i2c/muxes/i2c-mux-gpmux.c
@@ -120,8 +120,8 @@ static int i2c_mux_probe(struct platform_device *pdev)
 
 		ret = of_property_read_u32(child, "reg", &chan);
 		if (ret < 0) {
-			dev_err(dev, "no reg property for node '%s'\n",
-				child->name);
+			dev_err(dev, "no reg property for node '%pOFn'\n",
+				child);
 			goto err_children;
 		}
 
-- 
2.17.1


^ permalink raw reply related

* Re: [GIT PULL] Please pull powerpc/linux.git powerpc-4.19-3 tag
From: Greg KH @ 2018-09-29  1:02 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: mikey, srikar, aik, linux-kernel, mwb, linuxppc-dev, bauerman
In-Reply-To: <87r2hdua01.fsf@concordia.ellerman.id.au>

On Fri, Sep 28, 2018 at 09:39:10PM +1000, Michael Ellerman wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
> 
> Hi Greg,
> 
> Please pull some more powerpc fixes for 4.19:
> 
> The following changes since commit 11da3a7f84f19c26da6f86af878298694ede0804:
> 
>   Linux 4.19-rc3 (2018-09-09 17:26:43 -0700)
> 
> are available in the git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-4.19-3

Now pulled, thanks.

greg k-h

^ permalink raw reply

* [PATCH 1/2] powerpc/time: Use clockevents_register_device(), fixing an issue with large decrementer
From: Anton Blanchard @ 2018-09-29  1:26 UTC (permalink / raw)
  To: benh, paulus, mpe, npiggin, mikey, oohall; +Cc: linuxppc-dev

We currently cap the decrementer clockevent at 4 seconds, even on systems
with large decrementer support. Fix this by converting the code to use
clockevents_register_device() which calculates the upper bound based on
the max_delta passed in.

Signed-off-by: Anton Blanchard <anton@samba.org>
---
 arch/powerpc/kernel/time.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 70f145e02487..6a1f0a084ca3 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -984,10 +984,10 @@ static void register_decrementer_clockevent(int cpu)
 	*dec = decrementer_clockevent;
 	dec->cpumask = cpumask_of(cpu);
 
+	clockevents_config_and_register(dec, ppc_tb_freq, 2, decrementer_max);
+
 	printk_once(KERN_DEBUG "clockevent: %s mult[%x] shift[%d] cpu[%d]\n",
 		    dec->name, dec->mult, dec->shift, cpu);
-
-	clockevents_register_device(dec);
 }
 
 static void enable_large_decrementer(void)
@@ -1035,18 +1035,7 @@ static void __init set_decrementer_max(void)
 
 static void __init init_decrementer_clockevent(void)
 {
-	int cpu = smp_processor_id();
-
-	clockevents_calc_mult_shift(&decrementer_clockevent, ppc_tb_freq, 4);
-
-	decrementer_clockevent.max_delta_ns =
-		clockevent_delta2ns(decrementer_max, &decrementer_clockevent);
-	decrementer_clockevent.max_delta_ticks = decrementer_max;
-	decrementer_clockevent.min_delta_ns =
-		clockevent_delta2ns(2, &decrementer_clockevent);
-	decrementer_clockevent.min_delta_ticks = 2;
-
-	register_decrementer_clockevent(cpu);
+	register_decrementer_clockevent(smp_processor_id());
 }
 
 void secondary_cpu_time_init(void)
-- 
2.17.1


^ permalink raw reply related

* [PATCH 2/2] powerpc/time: Only cap decrementer when watchdog is enabled
From: Anton Blanchard @ 2018-09-29  1:26 UTC (permalink / raw)
  To: benh, paulus, mpe, npiggin, mikey, oohall; +Cc: linuxppc-dev
In-Reply-To: <20180929012607.10204-1-anton@ozlabs.org>

If CONFIG_PPC_WATCHDOG is enabled, we always cap the decrementer to
0x7fffffff. As suggested by Nick, add a run time check of the watchdog
cpumask, so if it is disabled we use the large decrementer.

Signed-off-by: Anton Blanchard <anton@samba.org>
---
 arch/powerpc/kernel/time.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 6a1f0a084ca3..3372019f52bd 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -60,6 +60,7 @@
 #include <linux/rtc.h>
 #include <linux/sched/cputime.h>
 #include <linux/processor.h>
+#include <linux/nmi.h>
 #include <asm/trace.h>
 
 #include <asm/io.h>
@@ -575,7 +576,8 @@ void timer_interrupt(struct pt_regs *regs)
 	 * 31 bits, which is about 4 seconds on most systems, which gives
 	 * the watchdog a chance of catching timer interrupt hard lockups.
 	 */
-	if (IS_ENABLED(CONFIG_PPC_WATCHDOG))
+	if (IS_ENABLED(CONFIG_PPC_WATCHDOG) &&
+	    cpumask_test_cpu(smp_processor_id(), &watchdog_cpumask))
 		set_dec(0x7fffffff);
 	else
 		set_dec(decrementer_max);
-- 
2.17.1


^ permalink raw reply related

* New mail archive
From: Stephen Rothwell @ 2018-09-29  3:56 UTC (permalink / raw)
  To: ppc-dev

[-- Attachment #1: Type: text/plain, Size: 127 bytes --]

Hi all,

This mailing list is now also archived at
https://lore.kernel.org/linuxppc-dev/

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: New mail archive
From: Stephen Rothwell @ 2018-09-29  3:59 UTC (permalink / raw)
  To: ppc-dev
In-Reply-To: <20180929135628.783ca76e@canb.auug.org.au>

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

Hi all,

On Sat, 29 Sep 2018 13:56:28 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> This mailing list is now also archived at
> https://lore.kernel.org/linuxppc-dev/

A few of the earliest emails have been misdated in the archive (to today
or yesterday), sorry.

Also, this archive includes as much as we had of the linuxppc-embedded
archive as well.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/time: Only cap decrementer when watchdog is enabled
From: kbuild test robot @ 2018-09-29  4:11 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: mikey, npiggin, paulus, kbuild-all, oohall, linuxppc-dev
In-Reply-To: <20180929012607.10204-2-anton@ozlabs.org>

[-- Attachment #1: Type: text/plain, Size: 3257 bytes --]

Hi Anton,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.19-rc5 next-20180928]
[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/Anton-Blanchard/powerpc-time-Use-clockevents_register_device-fixing-an-issue-with-large-decrementer/20180929-093322
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-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/kernel/time.c: In function 'timer_interrupt':
>> arch/powerpc/kernel/time.c:580:44: error: 'watchdog_cpumask' undeclared (first use in this function); did you mean 'proc_watchdog_cpumask'?
         cpumask_test_cpu(smp_processor_id(), &watchdog_cpumask))
                                               ^~~~~~~~~~~~~~~~
                                               proc_watchdog_cpumask
   arch/powerpc/kernel/time.c:580:44: note: each undeclared identifier is reported only once for each function it appears in

vim +580 arch/powerpc/kernel/time.c

   549	
   550	/*
   551	 * timer_interrupt - gets called when the decrementer overflows,
   552	 * with interrupts disabled.
   553	 */
   554	void timer_interrupt(struct pt_regs *regs)
   555	{
   556		struct clock_event_device *evt = this_cpu_ptr(&decrementers);
   557		u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
   558		struct pt_regs *old_regs;
   559		u64 now;
   560	
   561		/* Some implementations of hotplug will get timer interrupts while
   562		 * offline, just ignore these and we also need to set
   563		 * decrementers_next_tb as MAX to make sure __check_irq_replay
   564		 * don't replay timer interrupt when return, otherwise we'll trap
   565		 * here infinitely :(
   566		 */
   567		if (unlikely(!cpu_online(smp_processor_id()))) {
   568			*next_tb = ~(u64)0;
   569			set_dec(decrementer_max);
   570			return;
   571		}
   572	
   573		/* Ensure a positive value is written to the decrementer, or else
   574		 * some CPUs will continue to take decrementer exceptions. When the
   575		 * PPC_WATCHDOG (decrementer based) is configured, keep this at most
   576		 * 31 bits, which is about 4 seconds on most systems, which gives
   577		 * the watchdog a chance of catching timer interrupt hard lockups.
   578		 */
   579		if (IS_ENABLED(CONFIG_PPC_WATCHDOG) &&
 > 580		    cpumask_test_cpu(smp_processor_id(), &watchdog_cpumask))
   581			set_dec(0x7fffffff);
   582		else
   583			set_dec(decrementer_max);
   584	
   585		/* Conditionally hard-enable interrupts now that the DEC has been
   586		 * bumped to its maximum value
   587		 */
   588		may_hard_irq_enable();
   589	
   590	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 5832 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/time: Only cap decrementer when watchdog is enabled
From: Nicholas Piggin @ 2018-09-29  5:16 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: mikey, paulus, oohall, linuxppc-dev
In-Reply-To: <20180929012607.10204-2-anton@ozlabs.org>

On Sat, 29 Sep 2018 11:26:07 +1000
Anton Blanchard <anton@ozlabs.org> wrote:

> If CONFIG_PPC_WATCHDOG is enabled, we always cap the decrementer to
> 0x7fffffff. As suggested by Nick, add a run time check of the watchdog
> cpumask, so if it is disabled we use the large decrementer.
> 
> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---

Thanks for tracking this down. It's a fix for my breakage

a7cba02deced ("powerpc: allow soft-NMI watchdog to cover timer
interrupts with large decrementers")

Taking another look... what I had expected here is the timer subsystem
would have stopped the decrementer device after it processed the timer
and found nothing left. And we should have set DEC to max at that time.

The above patch was really intended to only cover the timer interrupt
itself locking up. I wonder if we need to add

    .set_state_oneshot_stopped = decrementer_shutdown

In our decremementer clockevent device?

Thanks,
Nick

^ permalink raw reply

* Re: [v3,2/3] watchdog: mpc8xxx: provide boot status
From: Guenter Roeck @ 2018-09-29 18:18 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Mark Rutland, devicetree, linux-watchdog, linux-kernel,
	Rob Herring, Radu Rendec, Wim Van Sebroeck, linuxppc-dev
In-Reply-To: <207741bf9c994a869f574a93d4fb4152d4e169c9.1537163779.git.christophe.leroy@c-s.fr>

On Mon, Sep 17, 2018 at 06:22:50AM +0000, Christophe Leroy wrote:
> mpc8xxx watchdog driver supports the following platforms:
> - mpc8xx
> - mpc83xx
> - mpc86xx
> 
> Those three platforms have a 32 bits register which provides the
> reason of the last boot, including whether it was caused by the
> watchdog.
> 
> mpc8xx: Register RSR, bit SWRS (bit 3)
> mpc83xx: Register RSR, bit SWRS (bit 28)
> mpc86xx: Register RSTRSCR, bit WDT_RR (bit 11)
> 
> This patch maps the register as defined in the device tree and updates
> wdt.bootstatus based on the value of the watchdog related bit. Then
> the information can be retrieved via the WDIOC_GETBOOTSTATUS ioctl.
> 
> Hereunder is an example of devicetree for mpc8xx,
> the Reset Status Register being at offset 0x288:
> 
> 		WDT: watchdog@0 {
> 			compatible = "fsl,mpc823-wdt";
> 			reg = <0x0 0x10 0x288 0x4>;
> 		};
> 
> On the mpc83xx, RSR is at offset 0x910
> On the mpc86xx, RSTRSCR is at offset 0xe0094
> 
> Suggested-by: Radu Rendec <radu.rendec@gmail.com>
> Tested-by: Christophe Leroy <christophe.leroy@c-s.fr> # On mpc885
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/mpc8xxx_wdt.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/watchdog/mpc8xxx_wdt.c b/drivers/watchdog/mpc8xxx_wdt.c
> index 1dcf5f10cdd9..069072e6747d 100644
> --- a/drivers/watchdog/mpc8xxx_wdt.c
> +++ b/drivers/watchdog/mpc8xxx_wdt.c
> @@ -47,6 +47,7 @@ struct mpc8xxx_wdt {
>  struct mpc8xxx_wdt_type {
>  	int prescaler;
>  	bool hw_enabled;
> +	u32 rsr_mask;
>  };
>  
>  struct mpc8xxx_wdt_ddata {
> @@ -159,6 +160,24 @@ static int mpc8xxx_wdt_probe(struct platform_device *ofdev)
>  		return -ENODEV;
>  	}
>  
> +	res = platform_get_resource(ofdev, IORESOURCE_MEM, 1);
> +	if (res) {
> +		bool status;
> +		u32 __iomem *rsr = ioremap(res->start, resource_size(res));
> +
> +		if (!rsr)
> +			return -ENOMEM;
> +
> +		status = in_be32(rsr) & wdt_type->rsr_mask;
> +		ddata->wdd.bootstatus = status ? WDIOF_CARDRESET : 0;
> +		 /* clear reset status bits related to watchdog timer */
> +		out_be32(rsr, wdt_type->rsr_mask);
> +		iounmap(rsr);
> +
> +		dev_info(dev, "Last boot was %scaused by watchdog\n",
> +			 status ? "" : "not ");
> +	}
> +
>  	spin_lock_init(&ddata->lock);
>  
>  	ddata->wdd.info = &mpc8xxx_wdt_info,
> @@ -216,6 +235,7 @@ static const struct of_device_id mpc8xxx_wdt_match[] = {
>  		.compatible = "mpc83xx_wdt",
>  		.data = &(struct mpc8xxx_wdt_type) {
>  			.prescaler = 0x10000,
> +			.rsr_mask = BIT(3), /* RSR Bit SWRS */
>  		},
>  	},
>  	{
> @@ -223,6 +243,7 @@ static const struct of_device_id mpc8xxx_wdt_match[] = {
>  		.data = &(struct mpc8xxx_wdt_type) {
>  			.prescaler = 0x10000,
>  			.hw_enabled = true,
> +			.rsr_mask = BIT(20), /* RSTRSCR Bit WDT_RR */
>  		},
>  	},
>  	{
> @@ -230,6 +251,7 @@ static const struct of_device_id mpc8xxx_wdt_match[] = {
>  		.data = &(struct mpc8xxx_wdt_type) {
>  			.prescaler = 0x800,
>  			.hw_enabled = true,
> +			.rsr_mask = BIT(28), /* RSR Bit SWRS */
>  		},
>  	},
>  	{},

^ permalink raw reply

* Re: [v3,3/3] dt-bindings: watchdog: add mpc8xxx-wdt support
From: Guenter Roeck @ 2018-09-29 18:19 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Mark Rutland, devicetree, linux-watchdog, linux-kernel,
	Rob Herring, Radu Rendec, Wim Van Sebroeck, linuxppc-dev
In-Reply-To: <e3e0bcfd7b779b978d07e088a8bd667ea9ddf4aa.1537163780.git.christophe.leroy@c-s.fr>

On Mon, Sep 17, 2018 at 06:22:53AM +0000, Christophe Leroy wrote:
> Add description of DT bindings for mpc8xxx-wdt driver which
> handles the CPU watchdog timer on the mpc83xx, mpc86xx and mpc8xx.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Reviewed-by: Rob Herring <robh@kernel.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  .../devicetree/bindings/watchdog/mpc8xxx-wdt.txt   | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/mpc8xxx-wdt.txt
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/mpc8xxx-wdt.txt b/Documentation/devicetree/bindings/watchdog/mpc8xxx-wdt.txt
> new file mode 100644
> index 000000000000..e176face472a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/mpc8xxx-wdt.txt
> @@ -0,0 +1,25 @@
> +* Freescale mpc8xxx watchdog driver (For 83xx, 86xx and 8xx)
> +
> +Required properties:
> +- compatible: Shall contain one of the following:
> +	"mpc83xx_wdt" for an mpc83xx
> +	"fsl,mpc8610-wdt" for an mpc86xx
> +	"fsl,mpc823-wdt" for an mpc8xx
> +- reg: base physical address and length of the area hosting the
> +       watchdog registers.
> +		On the 83xx, "Watchdog Timer Registers" area:	<0x200 0x100>
> +		On the 86xx, "Watchdog Timer Registers" area:	<0xe4000 0x100>
> +		On the 8xx, "General System Interface Unit" area: <0x0 0x10>
> +
> +Optional properties:
> +- reg: additional physical address and length (4) of location of the
> +       Reset Status Register (called RSTRSCR on the mpc86xx)
> +       		On the 83xx, it is located at offset 0x910
> +       		On the 86xx, it is located at offset 0xe0094
> +       		On the 8xx, it is located at offset 0x288
> +
> +Example:
> +		WDT: watchdog@0 {
> +		    compatible = "fsl,mpc823-wdt";
> +		    reg = <0x0 0x10 0x288 0x4>;
> +		};

^ permalink raw reply

* Re: [PATCH 0/8] add generic builtin command line
From: dwalker @ 2018-09-29 18:17 UTC (permalink / raw)
  To: Maksym Kokhan
  Cc: linux-mips, linux-efi, Ruslan Bilovol, Will Deacon,
	Paul Mackerras, H. Peter Anvin, Frank Rowand, x86, Russell King,
	Ingo Molnar, Catalin Marinas, James Hogan, Daniel Walker,
	devicetree, Rob Herring, Borislav Petkov, Thomas Gleixner,
	Andrii Bordunov, Ard Biesheuvel, linux-kernel, Ralf Baechle,
	Paul Burton, Andrew Morton, linuxppc-dev
In-Reply-To: <1538067309-5711-1-git-send-email-maksym.kokhan@globallogic.com>

On Thu, Sep 27, 2018 at 07:55:08PM +0300, Maksym Kokhan wrote:
> Daniel Walker (7):
>   add generic builtin command line
>   drivers: of: ifdef out cmdline section
>   x86: convert to generic builtin command line
>   arm: convert to generic builtin command line
>   arm64: convert to generic builtin command line
>   mips: convert to generic builtin command line
>   powerpc: convert to generic builtin command line
> 

When I originally submitted these I had a very good conversion with Rob Herring
on the device tree changes. It seemed fairly clear that my approach in these
changes could be done better. It effected specifically arm64, but a lot of other
platforms use the device tree integrally. With arm64 you can reduce the changes
down to only Kconfig changes, and that would likely be the case for many of the
other architecture. I made patches to do this a while back, but have not had
time to test them and push them out.

In terms of mips I think there's a fair amount of work needed to pull out their
architecture specific mangling into something generic. Part of my motivation for
these was to take the architecture specific feature and open that up for all the
architecture. So it makes sense that the mips changes should become part of
that.

The only changes which have no comments are the generic changes, x86, and
powerpc. Those patches have been used at Cisco for years with no issues.
I added those changes into my -next tree for a round of testing. Assuming there
are no issues I can work out the merging with the architecture maintainers.
As for the other changes I think they can be done in time, as long as the
generic parts of upstream the rest can be worked on by any of the architecture
developers.

Daniel

^ permalink raw reply

* Looking for architecture papers
From: Raz @ 2018-09-29 21:24 UTC (permalink / raw)
  To: linuxppc-dev

Hello

I want to learn about powerpc architecture, mainly hypervisor and
partioning.  I download the books (1,2, and 3 ) but I feel it lacks
a lot of information. Are there other books ?

Kind regards

^ permalink raw reply

* Re: Looking for architecture papers
From: Segher Boessenkool @ 2018-09-29 22:13 UTC (permalink / raw)
  To: Raz; +Cc: linuxppc-dev
In-Reply-To: <CAPB=Z-rEoVWFFDBS_XhuOQyL+YxR8jScSAAQfO1X-n_Lbm=StA@mail.gmail.com>

Hi Raz,

On Sun, Sep 30, 2018 at 12:24:35AM +0300, Raz wrote:
> I want to learn about powerpc architecture, mainly hypervisor and
> partioning.  I download the books (1,2, and 3 ) but I feel it lacks
> a lot of information. Are there other books ?

I suspect you are looking at an ancient version of the architecture.
The current version is 3.0B, available here:
https://openpowerfoundation.org/?resource_lib=power-isa-version-3-0

Good luck and have fun,


Segher

^ permalink raw reply

* Re: [PATCH v3 1/2] powerpc/32: add stack protector support
From: kbuild test robot @ 2018-09-30  2:16 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linux-kernel, Paul Mackerras, kbuild-all, linuxppc-dev
In-Reply-To: <2b934594d21ade67a4092637cf9e6b7d2d131187.1537801613.git.christophe.leroy@c-s.fr>

[-- Attachment #1: Type: text/plain, Size: 1696 bytes --]

Hi Christophe,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.19-rc5 next-20180928]
[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/Christophe-Leroy/powerpc-32-add-stack-protector-support/20180926-123625
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-mpc86xx_basic_defconfig (attached as .config)
compiler: powerpc-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/powermac/bootx_init.o: In function `bootx_add_display_props.isra.1':
>> bootx_init.c:(.init.text+0x504): undefined reference to `__stack_chk_fail_local'
   arch/powerpc/platforms/powermac/bootx_init.o: In function `bootx_scan_dt_build_struct':
   bootx_init.c:(.init.text+0x820): undefined reference to `__stack_chk_fail_local'
   arch/powerpc/platforms/powermac/bootx_init.o: In function `bootx_init':
   bootx_init.c:(.init.text+0xbbc): undefined reference to `__stack_chk_fail_local'
   powerpc-linux-gnu-ld: .tmp_vmlinux1: hidden symbol `__stack_chk_fail_local' isn't defined
   powerpc-linux-gnu-ld: final link failed: Bad value

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 8414 bytes --]

^ permalink raw reply

* [PATCH 2/2] powerpc/64: Increase stack redzone for 64-bit kernel to 512 bytes
From: Bin Meng @ 2018-09-30  6:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	linuxppc-dev
In-Reply-To: <1538288720-14910-1-git-send-email-bmeng.cn@gmail.com>

commit 573ebfa6601f ("powerpc: Increase stack redzone for 64-bit
userspace to 512 bytes") only changes stack userspace redzone size.
We need increase the kernel one to 512 bytes too per ABIv2 spec.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 arch/powerpc/include/asm/ptrace.h | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 447cbd1..817be3f 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -30,16 +30,12 @@
 #ifdef __powerpc64__
 
 /*
- * Size of redzone that userspace is allowed to use below the stack
+ * Size of redzone that kernel/userspace is allowed to use below the stack
  * pointer.  This is 288 in the 64-bit big-endian ELF ABI, and 512 in
  * the new ELFv2 little-endian ABI, so we allow the larger amount.
- *
- * For kernel code we allow a 288-byte redzone, in order to conserve
- * kernel stack space; gcc currently only uses 288 bytes, and will
- * hopefully allow explicit control of the redzone size in future.
  */
 #define USER_REDZONE_SIZE	512
-#define KERNEL_REDZONE_SIZE	288
+#define KERNEL_REDZONE_SIZE	512
 
 #define STACK_FRAME_OVERHEAD	112	/* size of minimum stack frame */
 #define STACK_FRAME_LR_SAVE	2	/* Location of LR in stack frame */
-- 
2.7.4


^ permalink raw reply related

* [PATCH 1/2] powerpc/64: Remove duplicated -mabi=elfv2 for little endian targets
From: Bin Meng @ 2018-09-30  6:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	linuxppc-dev

The -mabi=elfv2 is currently specified twice in the makefile. Remove
the one that does not test compiler.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 arch/powerpc/Makefile | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 11a1acb..16b0695 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -93,7 +93,6 @@ ifdef CONFIG_PPC64
 cflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mabi=elfv1)
 cflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mcall-aixdesc)
 aflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mabi=elfv1)
-aflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= -mabi=elfv2
 endif
 
 ifneq ($(cc-name),clang)
-- 
2.7.4


^ permalink raw reply related

* Re: Looking for architecture papers
From: Raz @ 2018-09-30  7:05 UTC (permalink / raw)
  To: segher; +Cc: linuxppc-dev
In-Reply-To: <20180929221321.GX23155@gate.crashing.org>

First, thank you.
And another question, is there a development environment that
resembles Fixed Virtual Platform for ARM ( FVP ARM) ?





On Sun, Sep 30, 2018 at 1:13 AM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> Hi Raz,
>
> On Sun, Sep 30, 2018 at 12:24:35AM +0300, Raz wrote:
> > I want to learn about powerpc architecture, mainly hypervisor and
> > partioning.  I download the books (1,2, and 3 ) but I feel it lacks
> > a lot of information. Are there other books ?
>
> I suspect you are looking at an ancient version of the architecture.
> The current version is 3.0B, available here:
> https://openpowerfoundation.org/?resource_lib=power-isa-version-3-0
>
> Good luck and have fun,
>
>
> Segher

^ permalink raw reply

* Re: Looking for architecture papers
From: Segher Boessenkool @ 2018-09-30  8:56 UTC (permalink / raw)
  To: Raz; +Cc: linuxppc-dev
In-Reply-To: <CAPB=Z-ojoLwhNWhE6agindvF_41gQYdjZkvquGBF86FYju37vA@mail.gmail.com>

On Sun, Sep 30, 2018 at 10:05:01AM +0300, Raz wrote:
> First, thank you.
> And another question, is there a development environment that
> resembles Fixed Virtual Platform for ARM ( FVP ARM) ?

I don't know FVP, but that seems similar to systemsim?
https://www14.software.ibm.com/webapp/set2/sas/f/pwrfs/home.html
(p8 version)
https://www14.software.ibm.com/webapp/set2/sas/f/pwrfs/pwr9/home.html
(p9 version)


Segher

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox