Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 2/3] arm64: dts: allwinner: a64: Add PWM controllers
From: Maxime Ripard @ 2018-06-06 19:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180606051702.6478-3-anarsoul@gmail.com>

On Tue, Jun 05, 2018 at 10:17:01PM -0700, Vasily Khoruzhick wrote:
> From: Andre Przywara <andre.przywara@arm.com>
> 
> The Allwinner A64 SoC features two PWM controllers, which are fully
> compatible to the one used in the A13 and H3 chips.
> 
> Add the nodes for the devices (one for the "normal" PWM, the other for
> the one in the CPUS domain) and the pins their outputs are connected to.
> 
> On the A64 the "normal" PWM is muxed together with one of the MDIO pins
> used to communicate with the Ethernet PHY, so it won't be usable on many
> boards. But the Pinebook laptop uses this pin for controlling the LCD
> backlight.
> 
> On Pine64 the CPUS PWM pin however is routed to the "RPi2" header,
> at the same location as the PWM pin on the RaspberryPi.
> 
> Tested on Pinebook and Teres-I
> 
> [vasily: fixed comment message as requested by Stefan Bruens, added default
>          muxing options to pwm and r_pwm nodes]
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> Tested-by: Harald Geyer <harald@ccbib.org>

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* [PATCH v4 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver
From: Rob Herring @ 2018-06-06 19:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <bbbd287c-437e-d7bd-d40f-6bb914d96860@gmail.com>

On Wed, Jun 6, 2018 at 2:30 PM, Frank Rowand <frowand.list@gmail.com> wrote:
> Hi Michel,
>
> On 06/05/18 23:36, Michel Pollet wrote:
>> Hi Frank,
>>
>> On 05 June 2018 18:34, Frank wrote:
>>> On 06/05/18 04:28, Michel Pollet wrote:
>>>> The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
>>>> it requires a special enable method to get it started.
>>>>
>>>> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>>>> ---
>>>>  arch/arm/mach-shmobile/Makefile        |  1 +
>>>>  arch/arm/mach-shmobile/smp-r9a06g032.c | 79
>>>> ++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 80 insertions(+)
>>>>  create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c
>>>>
>>>> diff --git a/arch/arm/mach-shmobile/Makefile
>>>> b/arch/arm/mach-shmobile/Makefile index 1939f52..d7fc98f 100644
>>>> --- a/arch/arm/mach-shmobile/Makefile
>>>> +++ b/arch/arm/mach-shmobile/Makefile
>>>> @@ -34,6 +34,7 @@ smp-$(CONFIG_ARCH_SH73A0)+= smp-sh73a0.o
>>> headsmp-scu.o platsmp-scu.o
>>>>  smp-$(CONFIG_ARCH_R8A7779)+= smp-r8a7779.o headsmp-scu.o
>>> platsmp-scu.o
>>>>  smp-$(CONFIG_ARCH_R8A7790)+= smp-r8a7790.o
>>>>  smp-$(CONFIG_ARCH_R8A7791)+= smp-r8a7791.o
>>>> +smp-$(CONFIG_ARCH_R9A06G032)+= smp-r9a06g032.o
>>>>  smp-$(CONFIG_ARCH_EMEV2)+= smp-emev2.o headsmp-scu.o
>>> platsmp-scu.o
>>>>
>>>>  # PM objects
>>>> diff --git a/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>> b/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>> new file mode 100644
>>>> index 0000000..cd40e6e
>>>> --- /dev/null
>>>> +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>> @@ -0,0 +1,79 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * R9A06G032 Second CA7 enabler.
>>>> + *
>>>> + * Copyright (C) 2018 Renesas Electronics Europe Limited
>>>> + *
>>>> + * Michel Pollet <michel.pollet@bp.renesas.com>,
>>> <buserror@gmail.com>
>>>> + * Derived from action,s500-smp
>>>> + */
>>>> +
>>>> +#include <linux/io.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/of_address.h>
>>>> +#include <linux/smp.h>
>>>> +
>>>> +/*
>>>> + * The second CPU is parked in ROM at boot time. It requires waking
>>>> +it after
>>>> + * writing an address into the BOOTADDR register of sysctrl.
>>>> + *
>>>> + * So the default value of the "cpu-release-addr" corresponds to
>>> BOOTADDR...
>>>> + *
>>>> + * *However* the BOOTADDR register is not available when the kernel
>>>> + * starts in NONSEC mode.
>>>> + *
>>>> + * So for NONSEC mode, the bootloader re-parks the second CPU into a
>>>> +pen
>>>> + * in SRAM, and changes the "cpu-release-addr" of linux's DT to a
>>>> +SRAM address,
>>>> + * which is not restricted.
>>>
>>> The binding document for cpu-release-addr does not have a definition for 32
>>> bit arm.  The existing definition is only 64 bit arm.  Please add the definition
>>> for 32 bit arm to patch 1.
>>
>> Hmmm I do find a definition in
>> Documentation/devicetree/bindings/arm/cpus.txt -- just under where I
>> added my 'enable-method' -- And it is already used as 32 bits in at least
>> arch/arm/boot/dts/stih407-family.dtsi.
>
> From cpus.txt:
>
>         - cpu-release-addr
>                 Usage: required for systems that have an "enable-method"
>                        property value of "spin-table".
>                 Value type: <prop-encoded-array>
>                 Definition:
>                         # On ARM v8 64-bit systems must be a two cell
>                           property identifying a 64-bit zero-initialised
>                           memory location.
>
> The definition specifies a two cell property for 64-bit systems.
>
> Please add to the definition that cpu-release-addr is a one cell property
> for 32-bit systems.

Actually, this is all already documented in the DT spec and it is
always 2 cells[1]. We should perhaps just remove whatever is
duplicated from the spec.

Rob

[1]
   ``cpu-release-addr``   | SD  | ``<u64>``        The
cpu-release-addr property is required for
                                                   cpu nodes that have
an enable-method property
                                                   value of
``"spin-table"``. The value specifies the
                                                   physical address of
a spin table entry that
                                                   releases a
secondary CPU from its spin loop.

^ permalink raw reply

* [PATCH v2 3/3] arm64: dts: allwinner: add support for Pinebook
From: Maxime Ripard @ 2018-06-06 19:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180606051702.6478-4-anarsoul@gmail.com>

On Tue, Jun 05, 2018 at 10:17:02PM -0700, Vasily Khoruzhick wrote:
> From: Icenowy Zheng <icenowy@aosc.xyz>
> 
> Pinebook is a A64-based laptop produced by Pine64, with the following
> peripherals:
> 
> USB:
> - Two external USB ports (one is directly connected to A64's OTG
> controller, the other is under a internal hub connected to the host-only
> controller.)
> - USB HID keyboard and touchpad connected to the internal hub.
> - USB UVC camera connected to the internal hub.
> 
> Power-related:
> - A DC IN jack connected to AXP803's DCIN pin.
> - A Li-Polymer battery connected to AXP803's battery pins.
> 
> Storage:
> - An eMMC by Foresee on the main board (in the product revision of the
> main board it's designed to be switchable).
> - An external MicroSD card slot.
> 
> Display:
> - An eDP LCD panel (1366x768) connected via an ANX6345 RGB-eDP bridge.
> - A mini HDMI port.
> 
> Misc:
> - A Hall sensor designed to detect the status of lid, connected to GPIO PL12.
> - A headphone jack connected to the SoC's internal codec.
> - A debug UART port muxed with headphone jack.
> 
> This commit adds basical support for it.
> 
> [vasily: squashed several commits into one, added simplefb node, added usbphy
> 	 to ehci0 and ohci0 nodes and other cosmetic changes to dts]
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>

I've updated Icenowy's domain and applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* [PATCH v4 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver
From: Florian Fainelli @ 2018-06-06 19:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <bbbd287c-437e-d7bd-d40f-6bb914d96860@gmail.com>

On 06/06/2018 12:30 PM, Frank Rowand wrote:
> Hi Michel,
> 
> On 06/05/18 23:36, Michel Pollet wrote:
>> Hi Frank,
>>
>> On 05 June 2018 18:34, Frank wrote:
>>> On 06/05/18 04:28, Michel Pollet wrote:
>>>> The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
>>>> it requires a special enable method to get it started.
>>>>
>>>> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>>>> ---
>>>>  arch/arm/mach-shmobile/Makefile        |  1 +
>>>>  arch/arm/mach-shmobile/smp-r9a06g032.c | 79
>>>> ++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 80 insertions(+)
>>>>  create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c
>>>>
>>>> diff --git a/arch/arm/mach-shmobile/Makefile
>>>> b/arch/arm/mach-shmobile/Makefile index 1939f52..d7fc98f 100644
>>>> --- a/arch/arm/mach-shmobile/Makefile
>>>> +++ b/arch/arm/mach-shmobile/Makefile
>>>> @@ -34,6 +34,7 @@ smp-$(CONFIG_ARCH_SH73A0)+= smp-sh73a0.o
>>> headsmp-scu.o platsmp-scu.o
>>>>  smp-$(CONFIG_ARCH_R8A7779)+= smp-r8a7779.o headsmp-scu.o
>>> platsmp-scu.o
>>>>  smp-$(CONFIG_ARCH_R8A7790)+= smp-r8a7790.o
>>>>  smp-$(CONFIG_ARCH_R8A7791)+= smp-r8a7791.o
>>>> +smp-$(CONFIG_ARCH_R9A06G032)+= smp-r9a06g032.o
>>>>  smp-$(CONFIG_ARCH_EMEV2)+= smp-emev2.o headsmp-scu.o
>>> platsmp-scu.o
>>>>
>>>>  # PM objects
>>>> diff --git a/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>> b/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>> new file mode 100644
>>>> index 0000000..cd40e6e
>>>> --- /dev/null
>>>> +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>> @@ -0,0 +1,79 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * R9A06G032 Second CA7 enabler.
>>>> + *
>>>> + * Copyright (C) 2018 Renesas Electronics Europe Limited
>>>> + *
>>>> + * Michel Pollet <michel.pollet@bp.renesas.com>,
>>> <buserror@gmail.com>
>>>> + * Derived from action,s500-smp
>>>> + */
>>>> +
>>>> +#include <linux/io.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/of_address.h>
>>>> +#include <linux/smp.h>
>>>> +
>>>> +/*
>>>> + * The second CPU is parked in ROM at boot time. It requires waking
>>>> +it after
>>>> + * writing an address into the BOOTADDR register of sysctrl.
>>>> + *
>>>> + * So the default value of the "cpu-release-addr" corresponds to
>>> BOOTADDR...
>>>> + *
>>>> + * *However* the BOOTADDR register is not available when the kernel
>>>> + * starts in NONSEC mode.
>>>> + *
>>>> + * So for NONSEC mode, the bootloader re-parks the second CPU into a
>>>> +pen
>>>> + * in SRAM, and changes the "cpu-release-addr" of linux's DT to a
>>>> +SRAM address,
>>>> + * which is not restricted.
>>>
>>> The binding document for cpu-release-addr does not have a definition for 32
>>> bit arm.  The existing definition is only 64 bit arm.  Please add the definition
>>> for 32 bit arm to patch 1.
>>
>> Hmmm I do find a definition in
>> Documentation/devicetree/bindings/arm/cpus.txt -- just under where I
>> added my 'enable-method' -- And it is already used as 32 bits in at least
>> arch/arm/boot/dts/stih407-family.dtsi.
> 
> From cpus.txt:
> 
>         - cpu-release-addr
>                 Usage: required for systems that have an "enable-method"
>                        property value of "spin-table".
>                 Value type: <prop-encoded-array>
>                 Definition:
>                         # On ARM v8 64-bit systems must be a two cell
>                           property identifying a 64-bit zero-initialised
>                           memory location.
> 
> The definition specifies a two cell property for 64-bit systems.
> 
> Please add to the definition that cpu-release-addr is a one cell property
> for 32-bit systems.

Or maybe phrase it such that the number of cells encoded in
cpu-release-addr must exactly match the CPU node's #address-cells size?
-- 
Florian

^ permalink raw reply

* [PATCH v4 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver
From: Geert Uytterhoeven @ 2018-06-06 19:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAL_JsqJCkP_c_wKGRc7Qzkiw8sZLRf6MGz-WgVsLjnQfqK8r6Q@mail.gmail.com>

Hi Rob,

On Wed, Jun 6, 2018 at 9:35 PM, Rob Herring <robh+dt@kernel.org> wrote:
> On Wed, Jun 6, 2018 at 2:30 PM, Frank Rowand <frowand.list@gmail.com> wrote:
>> On 06/05/18 23:36, Michel Pollet wrote:
>>> On 05 June 2018 18:34, Frank wrote:
>>>> On 06/05/18 04:28, Michel Pollet wrote:
>>>>> The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
>>>>> it requires a special enable method to get it started.
>>>>>
>>>>> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>

>>>>> --- /dev/null
>>>>> +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c

>>>>> +/*
>>>>> + * The second CPU is parked in ROM at boot time. It requires waking
>>>>> +it after
>>>>> + * writing an address into the BOOTADDR register of sysctrl.
>>>>> + *
>>>>> + * So the default value of the "cpu-release-addr" corresponds to
>>>> BOOTADDR...
>>>>> + *
>>>>> + * *However* the BOOTADDR register is not available when the kernel
>>>>> + * starts in NONSEC mode.
>>>>> + *
>>>>> + * So for NONSEC mode, the bootloader re-parks the second CPU into a
>>>>> +pen
>>>>> + * in SRAM, and changes the "cpu-release-addr" of linux's DT to a
>>>>> +SRAM address,
>>>>> + * which is not restricted.
>>>>
>>>> The binding document for cpu-release-addr does not have a definition for 32
>>>> bit arm.  The existing definition is only 64 bit arm.  Please add the definition
>>>> for 32 bit arm to patch 1.
>>>
>>> Hmmm I do find a definition in
>>> Documentation/devicetree/bindings/arm/cpus.txt -- just under where I
>>> added my 'enable-method' -- And it is already used as 32 bits in at least
>>> arch/arm/boot/dts/stih407-family.dtsi.
>>
>> From cpus.txt:
>>
>>         - cpu-release-addr
>>                 Usage: required for systems that have an "enable-method"
>>                        property value of "spin-table".
>>                 Value type: <prop-encoded-array>
>>                 Definition:
>>                         # On ARM v8 64-bit systems must be a two cell
>>                           property identifying a 64-bit zero-initialised
>>                           memory location.
>>
>> The definition specifies a two cell property for 64-bit systems.
>>
>> Please add to the definition that cpu-release-addr is a one cell property
>> for 32-bit systems.
>
> Actually, this is all already documented in the DT spec and it is
> always 2 cells[1]. We should perhaps just remove whatever is
> duplicated from the spec.
>
> Rob
>
> [1]
>    ``cpu-release-addr``   | SD  | ``<u64>``        The
> cpu-release-addr property is required for
>                                                    cpu nodes that have
> an enable-method property
>                                                    value of
> ``"spin-table"``. The value specifies the
>                                                    physical address of
> a spin table entry that
>                                                    releases a
> secondary CPU from its spin loop.

Interesting. But why is this <u64>, and not just following #address-cells?
Due to the ePAPR-spec being 64-bit Power System-centric?

There's also "initial-mapped-area", which must use 64-bit values for effective
and physical addresses, according to ePAPR.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH v4 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver
From: Geert Uytterhoeven @ 2018-06-06 19:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <7e49c265-e332-29c5-5d91-4b5d5da6cb37@gmail.com>

Hi Florian,

On Wed, Jun 6, 2018 at 9:37 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 06/06/2018 12:30 PM, Frank Rowand wrote:
>> On 06/05/18 23:36, Michel Pollet wrote:
>>> On 05 June 2018 18:34, Frank wrote:
>>>> On 06/05/18 04:28, Michel Pollet wrote:
>>>>> The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
>>>>> it requires a special enable method to get it started.
>>>>>
>>>>> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>

>>>>> --- /dev/null
>>>>> +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c

>>>>> +/*
>>>>> + * The second CPU is parked in ROM at boot time. It requires waking
>>>>> +it after
>>>>> + * writing an address into the BOOTADDR register of sysctrl.
>>>>> + *
>>>>> + * So the default value of the "cpu-release-addr" corresponds to
>>>> BOOTADDR...
>>>>> + *
>>>>> + * *However* the BOOTADDR register is not available when the kernel
>>>>> + * starts in NONSEC mode.
>>>>> + *
>>>>> + * So for NONSEC mode, the bootloader re-parks the second CPU into a
>>>>> +pen
>>>>> + * in SRAM, and changes the "cpu-release-addr" of linux's DT to a
>>>>> +SRAM address,
>>>>> + * which is not restricted.
>>>>
>>>> The binding document for cpu-release-addr does not have a definition for 32
>>>> bit arm.  The existing definition is only 64 bit arm.  Please add the definition
>>>> for 32 bit arm to patch 1.
>>>
>>> Hmmm I do find a definition in
>>> Documentation/devicetree/bindings/arm/cpus.txt -- just under where I
>>> added my 'enable-method' -- And it is already used as 32 bits in at least
>>> arch/arm/boot/dts/stih407-family.dtsi.
>>
>> From cpus.txt:
>>
>>         - cpu-release-addr
>>                 Usage: required for systems that have an "enable-method"
>>                        property value of "spin-table".
>>                 Value type: <prop-encoded-array>
>>                 Definition:
>>                         # On ARM v8 64-bit systems must be a two cell
>>                           property identifying a 64-bit zero-initialised
>>                           memory location.
>>
>> The definition specifies a two cell property for 64-bit systems.
>>
>> Please add to the definition that cpu-release-addr is a one cell property
>> for 32-bit systems.
>
> Or maybe phrase it such that the number of cells encoded in
> cpu-release-addr must exactly match the CPU node's #address-cells size?

The CPU node's #address-cells size is unrelated.
You need the #address-cells value from the SoC bus (typically the root
node, not considering heterogeneous systems with multiple CPUs ;-).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH v2 3/3] arm64: dts: allwinner: add support for Pinebook
From: Vasily Khoruzhick @ 2018-06-06 20:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180606193701.seerxyy6g3nx3flk@flea>

On Wed, Jun 6, 2018 at 12:37 PM, Maxime Ripard
<maxime.ripard@bootlin.com> wrote:
>
> I've updated Icenowy's domain and applied, thanks!
> Maxime

Thanks!

^ permalink raw reply

* [PATCH v4 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver
From: Rob Herring @ 2018-06-06 20:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMuHMdXv2UXTx_ttCqytH2TePoJ-Pw4gJ-PaSOmUL969ac1BMw@mail.gmail.com>

On Wed, Jun 6, 2018 at 2:42 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Rob,
>
> On Wed, Jun 6, 2018 at 9:35 PM, Rob Herring <robh+dt@kernel.org> wrote:
>> On Wed, Jun 6, 2018 at 2:30 PM, Frank Rowand <frowand.list@gmail.com> wrote:
>>> On 06/05/18 23:36, Michel Pollet wrote:
>>>> On 05 June 2018 18:34, Frank wrote:
>>>>> On 06/05/18 04:28, Michel Pollet wrote:
>>>>>> The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
>>>>>> it requires a special enable method to get it started.
>>>>>>
>>>>>> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>
>>>>>> --- /dev/null
>>>>>> +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
>
>>>>>> +/*
>>>>>> + * The second CPU is parked in ROM at boot time. It requires waking
>>>>>> +it after
>>>>>> + * writing an address into the BOOTADDR register of sysctrl.
>>>>>> + *
>>>>>> + * So the default value of the "cpu-release-addr" corresponds to
>>>>> BOOTADDR...
>>>>>> + *
>>>>>> + * *However* the BOOTADDR register is not available when the kernel
>>>>>> + * starts in NONSEC mode.
>>>>>> + *
>>>>>> + * So for NONSEC mode, the bootloader re-parks the second CPU into a
>>>>>> +pen
>>>>>> + * in SRAM, and changes the "cpu-release-addr" of linux's DT to a
>>>>>> +SRAM address,
>>>>>> + * which is not restricted.
>>>>>
>>>>> The binding document for cpu-release-addr does not have a definition for 32
>>>>> bit arm.  The existing definition is only 64 bit arm.  Please add the definition
>>>>> for 32 bit arm to patch 1.
>>>>
>>>> Hmmm I do find a definition in
>>>> Documentation/devicetree/bindings/arm/cpus.txt -- just under where I
>>>> added my 'enable-method' -- And it is already used as 32 bits in at least
>>>> arch/arm/boot/dts/stih407-family.dtsi.
>>>
>>> From cpus.txt:
>>>
>>>         - cpu-release-addr
>>>                 Usage: required for systems that have an "enable-method"
>>>                        property value of "spin-table".
>>>                 Value type: <prop-encoded-array>
>>>                 Definition:
>>>                         # On ARM v8 64-bit systems must be a two cell
>>>                           property identifying a 64-bit zero-initialised
>>>                           memory location.
>>>
>>> The definition specifies a two cell property for 64-bit systems.
>>>
>>> Please add to the definition that cpu-release-addr is a one cell property
>>> for 32-bit systems.
>>
>> Actually, this is all already documented in the DT spec and it is
>> always 2 cells[1]. We should perhaps just remove whatever is
>> duplicated from the spec.
>>
>> Rob
>>
>> [1]
>>    ``cpu-release-addr``   | SD  | ``<u64>``        The
>> cpu-release-addr property is required for
>>                                                    cpu nodes that have
>> an enable-method property
>>                                                    value of
>> ``"spin-table"``. The value specifies the
>>                                                    physical address of
>> a spin table entry that
>>                                                    releases a
>> secondary CPU from its spin loop.
>
> Interesting. But why is this <u64>, and not just following #address-cells?

As you said in your other email, it's not the same.

> Due to the ePAPR-spec being 64-bit Power System-centric?

Other than #address-cells already having another defined purpose in
/cpus, my guess is 64-bit works for either and 32-bit SMP systems
didn't predate 64-bit (for the ePAPR author's perspective).

> There's also "initial-mapped-area", which must use 64-bit values for effective
> and physical addresses, according to ePAPR.

I would have thought that followed #{size,address}-cells being
/memory. Though, I guess the bootloader fills this in and it is much
easier to work with fixed sizes.

Rob

^ permalink raw reply

* [PATCH 1/2] arm64: avoid alloc memory on offline node
From: Bjorn Helgaas @ 2018-06-06 20:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180606154516.GL6631@arm.com>

[+cc akpm, linux-mm, linux-pci]

On Wed, Jun 6, 2018 at 10:44 AM Will Deacon <will.deacon@arm.com> wrote:
>
> On Thu, May 31, 2018 at 08:14:38PM +0800, Xie XiuQi wrote:
> > A numa system may return node which is not online.
> > For example, a numa node:
> > 1) without memory
> > 2) NR_CPUS is very small, and the cpus on the node are not brought up
> >
> > In this situation, we use NUMA_NO_NODE to avoid oops.
> >
> > [   25.732905] Unable to handle kernel NULL pointer dereference at virtual address 00001988
> > [   25.740982] Mem abort info:
> > [   25.743762]   ESR = 0x96000005
> > [   25.746803]   Exception class = DABT (current EL), IL = 32 bits
> > [   25.752711]   SET = 0, FnV = 0
> > [   25.755751]   EA = 0, S1PTW = 0
> > [   25.758878] Data abort info:
> > [   25.761745]   ISV = 0, ISS = 0x00000005
> > [   25.765568]   CM = 0, WnR = 0
> > [   25.768521] [0000000000001988] user address but active_mm is swapper
> > [   25.774861] Internal error: Oops: 96000005 [#1] SMP
> > [   25.779724] Modules linked in:
> > [   25.782768] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.17.0-rc6-mpam+ #115
> > [   25.789714] Hardware name: Huawei D06/D06, BIOS Hisilicon D06 EC UEFI Nemo 2.0 RC0 - B305 05/28/2018
> > [   25.798831] pstate: 80c00009 (Nzcv daif +PAN +UAO)
> > [   25.803612] pc : __alloc_pages_nodemask+0xf0/0xe70
> > [   25.808389] lr : __alloc_pages_nodemask+0x184/0xe70
> > [   25.813252] sp : ffff00000996f660
> > [   25.816553] x29: ffff00000996f660 x28: 0000000000000000
> > [   25.821852] x27: 00000000014012c0 x26: 0000000000000000
> > [   25.827150] x25: 0000000000000003 x24: ffff000008099eac
> > [   25.832449] x23: 0000000000400000 x22: 0000000000000000
> > [   25.837747] x21: 0000000000000001 x20: 0000000000000000
> > [   25.843045] x19: 0000000000400000 x18: 0000000000010e00
> > [   25.848343] x17: 000000000437f790 x16: 0000000000000020
> > [   25.853641] x15: 0000000000000000 x14: 6549435020524541
> > [   25.858939] x13: 20454d502067756c x12: 0000000000000000
> > [   25.864237] x11: ffff00000996f6f0 x10: 0000000000000006
> > [   25.869536] x9 : 00000000000012a4 x8 : ffff8023c000ff90
> > [   25.874834] x7 : 0000000000000000 x6 : ffff000008d73c08
> > [   25.880132] x5 : 0000000000000000 x4 : 0000000000000081
> > [   25.885430] x3 : 0000000000000000 x2 : 0000000000000000
> > [   25.890728] x1 : 0000000000000001 x0 : 0000000000001980
> > [   25.896027] Process swapper/0 (pid: 1, stack limit = 0x        (ptrval))
> > [   25.902712] Call trace:
> > [   25.905146]  __alloc_pages_nodemask+0xf0/0xe70
> > [   25.909577]  allocate_slab+0x94/0x590
> > [   25.913225]  new_slab+0x68/0xc8
> > [   25.916353]  ___slab_alloc+0x444/0x4f8
> > [   25.920088]  __slab_alloc+0x50/0x68
> > [   25.923562]  kmem_cache_alloc_node_trace+0xe8/0x230
> > [   25.928426]  pci_acpi_scan_root+0x94/0x278
> > [   25.932510]  acpi_pci_root_add+0x228/0x4b0
> > [   25.936593]  acpi_bus_attach+0x10c/0x218
> > [   25.940501]  acpi_bus_attach+0xac/0x218
> > [   25.944323]  acpi_bus_attach+0xac/0x218
> > [   25.948144]  acpi_bus_scan+0x5c/0xc0
> > [   25.951708]  acpi_scan_init+0xf8/0x254
> > [   25.955443]  acpi_init+0x310/0x37c
> > [   25.958831]  do_one_initcall+0x54/0x208
> > [   25.962653]  kernel_init_freeable+0x244/0x340
> > [   25.966999]  kernel_init+0x18/0x118
> > [   25.970474]  ret_from_fork+0x10/0x1c
> > [   25.974036] Code: 7100047f 321902a4 1a950095 b5000602 (b9400803)
> > [   25.980162] ---[ end trace 64f0893eb21ec283 ]---
> > [   25.984765] Kernel panic - not syncing: Fatal exception
> >
> > Signed-off-by: Xie XiuQi <xiexiuqi@huawei.com>
> > Tested-by: Huiqiang Wang <wanghuiqiang@huawei.com>
> > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > Cc: Tomasz Nowicki <Tomasz.Nowicki@caviumnetworks.com>
> > Cc: Xishi Qiu <qiuxishi@huawei.com>
> > ---
> >  arch/arm64/kernel/pci.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > index 0e2ea1c..e17cc45 100644
> > --- a/arch/arm64/kernel/pci.c
> > +++ b/arch/arm64/kernel/pci.c
> > @@ -170,6 +170,9 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
> >       struct pci_bus *bus, *child;
> >       struct acpi_pci_root_ops *root_ops;
> >
> > +     if (node != NUMA_NO_NODE && !node_online(node))
> > +             node = NUMA_NO_NODE;
> > +
>
> This really feels like a bodge, but it does appear to be what other
> architectures do, so:
>
> Acked-by: Will Deacon <will.deacon@arm.com>

I agree, this doesn't feel like something we should be avoiding in the
caller of kzalloc_node().

I would not expect kzalloc_node() to return memory that's offline, no
matter what node we told it to allocate from.  I could imagine it
returning failure, or returning memory from a node that *is* online,
but returning a pointer to offline memory seems broken.

Are we putting memory that's offline in the free list?  I don't know
where to look to figure this out.

Bjorn

^ permalink raw reply

* [PATCH v4 05/14] coresight: get/put module in coresight_build/release_path
From: Kim Phillips @ 2018-06-06 20:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <a457594c-b391-23e1-6ab5-d115073cac5a@arm.com>

On Wed, 6 Jun 2018 10:46:36 +0100
Suzuki K Poulose <suzuki.poulose@arm.com> wrote:

> On 06/06/2018 09:24 AM, Greg Kroah-Hartman wrote:
> > On Tue, Jun 05, 2018 at 04:07:01PM -0500, Kim Phillips wrote:
> >> Increment the refcnt for driver modules in current use by calling
> >> module_get in coresight_build_path and module_put in release_path.
> >>
> >> This prevents driver modules from being unloaded when they are in use,
> >> either in sysfs or perf mode.
> > 
> > Why does it matter?  Shouldn't you be allowed to remove any module at
> > any point in time, much like a networking driver?
> > 
> > 
> >>
> >> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> >> Cc: Leo Yan <leo.yan@linaro.org>
> >> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> >> Cc: Randy Dunlap <rdunlap@infradead.org>
> >> Cc: Suzuki K Poulose <Suzuki.Poulose@arm.com>
> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> Cc: Russell King <linux@armlinux.org.uk>
> >> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
> >> ---
> >>   drivers/hwtracing/coresight/coresight.c | 9 +++++++++
> >>   1 file changed, 9 insertions(+)
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
> >> index 338f1719641c..1c941351f1d1 100644
> >> --- a/drivers/hwtracing/coresight/coresight.c
> >> +++ b/drivers/hwtracing/coresight/coresight.c
> >> @@ -465,6 +465,12 @@ static int _coresight_build_path(struct coresight_device *csdev,
> >>   
> >>   	node->csdev = csdev;
> >>   	list_add(&node->link, path);
> >> +
> >> +	if (!try_module_get(csdev->dev.parent->driver->owner)) {
> > 
> > What is to keep parent->driver from going away right here?  What keeps
> > parent around?  This feels very fragile to me, I don't see any locking
> > anywhere around this code path to try to keep things in place.
> 
> You're right. We do have coresight_mutex, which is held across the build 
> path and the csdev is removed when a device is unregistered. However, I
> see that we don't hold the mutex while removing the connections from
> coresight_unregister(). Holding the mutex should protect us from the
> csdev being removed, while we build the path.

OK, I'll add this for the next version:

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index f96258de1e9b..da702507a55c 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1040,8 +1040,12 @@ EXPORT_SYMBOL_GPL(coresight_register);
 
 void coresight_unregister(struct coresight_device *csdev)
 {
+       mutex_lock(&coresight_mutex);
+
        /* Remove references of that device in the topology */
        coresight_remove_conns(csdev);
        device_unregister(&csdev->dev);
+
+       mutex_unlock(&coresight_mutex);
 }
 EXPORT_SYMBOL_GPL(coresight_unregister);

> And while we are at this, I also realised that we hold references to the
> parent devices for each connection (via bus_find_device() from 
> of_coresight_get_endpoint_device()), while parsing the platform data, 
> which is never released.

Would this fix that?:

diff --git a/drivers/hwtracing/coresight/of_coresight.c b/drivers/hwtracing/coresight/of_coresight.c
index a33a92ebe74b..a43ab078c85e 100644
--- a/drivers/hwtracing/coresight/of_coresight.c
+++ b/drivers/hwtracing/coresight/of_coresight.c
@@ -181,6 +181,8 @@ of_get_coresight_platform_data(struct device *dev,
                        pdata->child_names[i] = dev_name(rdev);
                        pdata->child_ports[i] = rendpoint.id;
 
+                       put_device(rdev);
+
                        i++;
                } while (ep);
        }

Thanks,

Kim

^ permalink raw reply related

* [clk:clk-bcm-stingray 1/2] drivers/clk/bcm/clk-sr.c:217:3: error: 'BCM_SR_LCPLL0_SATA_REF_CLK' undeclared here (not in a function); did you mean 'BCM_SR_LCPLL0_SATA_REFN_CLK'?
From: Ray Jui @ 2018-06-06 21:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <201806021742.e6HPCQOb%fengguang.wu@intel.com>

Hi Stephen,

I'd like to confirm this kbuild error is caused by changing of the defines
in the header.

That is, kbuild test should pass with 2/2 patch.

If my understanding is incorrect and this requires any further action,
please let me know.

Thanks,

Ray

-----Original Message-----
From: kbuild test robot [mailto:lkp at intel.com]
Sent: Saturday, June 2, 2018 2:02 AM
To: Pramod Kumar
Cc: kbuild-all at 01.org; linux-clk at vger.kernel.org;
linux-arm-kernel at lists.infradead.org; Stephen Boyd; Ray Jui
Subject: [clk:clk-bcm-stingray 1/2] drivers/clk/bcm/clk-sr.c:217:3: error:
'BCM_SR_LCPLL0_SATA_REF_CLK' undeclared here (not in a function); did you
mean 'BCM_SR_LCPLL0_SATA_REFN_CLK'?

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
clk-bcm-stingray
head:   5afa881c6635427e68c73861a2c22d8cac00b984
commit: 48bf9a522c14449cc7c214c6062668ac54e4e88f [1/2] dt-bindings: clk:
Update Stingray binding doc
config: sh-allmodconfig (attached as .config)
compiler: sh4-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
        git checkout 48bf9a522c14449cc7c214c6062668ac54e4e88f
        # save the attached .config to linux build tree
        make.cross ARCH=sh

Note: the clk/clk-bcm-stingray HEAD
5afa881c6635427e68c73861a2c22d8cac00b984 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/clk/bcm/clk-sr.c:59:3: error: 'BCM_SR_GENPLL0_SATA_CLK'
undeclared here (not in a function); did you mean
'BCM_SR_GENPLL0_SCR_CLK'?
     [BCM_SR_GENPLL0_SATA_CLK] = {
      ^~~~~~~~~~~~~~~~~~~~~~~
      BCM_SR_GENPLL0_SCR_CLK
   drivers/clk/bcm/clk-sr.c:59:3: error: array index in initializer not of
integer type
   drivers/clk/bcm/clk-sr.c:59:3: note: (near initialization for
'sr_genpll0_clk')
   drivers/clk/bcm/clk-sr.c:184:3: error: 'BCM_SR_GENPLL5_FS_CLK'
undeclared here (not in a function); did you mean
'BCM_SR_GENPLL2_FS4_CLK'?
     [BCM_SR_GENPLL5_FS_CLK] = {
      ^~~~~~~~~~~~~~~~~~~~~
      BCM_SR_GENPLL2_FS4_CLK
   drivers/clk/bcm/clk-sr.c:184:3: error: array index in initializer not
of integer type
   drivers/clk/bcm/clk-sr.c:184:3: note: (near initialization for
'sr_genpll5_clk')
   drivers/clk/bcm/clk-sr.c:185:14: warning: initialization makes integer
from pointer without a cast [-Wint-conversion]
      .channel = BCM_SR_GENPLL5_FS_CLK,
                 ^~~~~~~~~~~~~~~~~~~~~
   drivers/clk/bcm/clk-sr.c:185:14: note: (near initialization for
'sr_genpll5_clk[0].channel')
   drivers/clk/bcm/clk-sr.c:185:14: error: initializer element is not
constant
   drivers/clk/bcm/clk-sr.c:185:14: note: (near initialization for
'sr_genpll5_clk[0].channel')
   drivers/clk/bcm/clk-sr.c:190:3: error: 'BCM_SR_GENPLL5_SPU_CLK'
undeclared here (not in a function); did you mean 'BCM_SR_GENPLL5_FS_CLK'?
     [BCM_SR_GENPLL5_SPU_CLK] = {
      ^~~~~~~~~~~~~~~~~~~~~~
      BCM_SR_GENPLL5_FS_CLK
   drivers/clk/bcm/clk-sr.c:190:3: error: array index in initializer not
of integer type
   drivers/clk/bcm/clk-sr.c:190:3: note: (near initialization for
'sr_genpll5_clk')
   drivers/clk/bcm/clk-sr.c:191:14: warning: initialization makes integer
from pointer without a cast [-Wint-conversion]
      .channel = BCM_SR_GENPLL5_SPU_CLK,
                 ^~~~~~~~~~~~~~~~~~~~~~
   drivers/clk/bcm/clk-sr.c:191:14: note: (near initialization for
'sr_genpll5_clk[1].channel')
   drivers/clk/bcm/clk-sr.c:191:14: error: initializer element is not
constant
   drivers/clk/bcm/clk-sr.c:191:14: note: (near initialization for
'sr_genpll5_clk[1].channel')
>> drivers/clk/bcm/clk-sr.c:217:3: error: 'BCM_SR_LCPLL0_SATA_REF_CLK'
undeclared here (not in a function); did you mean
'BCM_SR_LCPLL0_SATA_REFN_CLK'?
     [BCM_SR_LCPLL0_SATA_REF_CLK] = {
      ^~~~~~~~~~~~~~~~~~~~~~~~~~
      BCM_SR_LCPLL0_SATA_REFN_CLK
   drivers/clk/bcm/clk-sr.c:217:3: error: array index in initializer not
of integer type
   drivers/clk/bcm/clk-sr.c:217:3: note: (near initialization for
'sr_lcpll0_clk')
   drivers/clk/bcm/clk-sr.c:218:14: warning: initialization makes integer
from pointer without a cast [-Wint-conversion]
      .channel = BCM_SR_LCPLL0_SATA_REF_CLK,
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/clk/bcm/clk-sr.c:218:14: note: (near initialization for
'sr_lcpll0_clk[0].channel')
   drivers/clk/bcm/clk-sr.c:218:14: error: initializer element is not
constant
   drivers/clk/bcm/clk-sr.c:218:14: note: (near initialization for
'sr_lcpll0_clk[0].channel')
   drivers/clk/bcm/clk-sr.c:223:3: error: 'BCM_SR_LCPLL0_USB_REF_CLK'
undeclared here (not in a function); did you mean
'BCM_SR_LCPLL1_USB_REF_CLK'?
     [BCM_SR_LCPLL0_USB_REF_CLK] = {
      ^~~~~~~~~~~~~~~~~~~~~~~~~
      BCM_SR_LCPLL1_USB_REF_CLK
   drivers/clk/bcm/clk-sr.c:223:3: error: array index in initializer not
of integer type
   drivers/clk/bcm/clk-sr.c:223:3: note: (near initialization for
'sr_lcpll0_clk')
   drivers/clk/bcm/clk-sr.c:224:14: warning: initialization makes integer
from pointer without a cast [-Wint-conversion]
      .channel = BCM_SR_LCPLL0_USB_REF_CLK,
                 ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/clk/bcm/clk-sr.c:224:14: note: (near initialization for
'sr_lcpll0_clk[1].channel')
   drivers/clk/bcm/clk-sr.c:224:14: error: initializer element is not
constant
   drivers/clk/bcm/clk-sr.c:224:14: note: (near initialization for
'sr_lcpll0_clk[1].channel')
>> drivers/clk/bcm/clk-sr.c:229:3: error: 'BCM_SR_LCPLL0_SATA_REFPN_CLK'
undeclared here (not in a function); did you mean
'BCM_SR_LCPLL0_SATA_REFN_CLK'?
     [BCM_SR_LCPLL0_SATA_REFPN_CLK] = {
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      BCM_SR_LCPLL0_SATA_REFN_CLK
   drivers/clk/bcm/clk-sr.c:229:3: error: array index in initializer not
of integer type
   drivers/clk/bcm/clk-sr.c:229:3: note: (near initialization for
'sr_lcpll0_clk')
   drivers/clk/bcm/clk-sr.c:230:14: warning: initialization makes integer
from pointer without a cast [-Wint-conversion]
      .channel = BCM_SR_LCPLL0_SATA_REFPN_CLK,
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/clk/bcm/clk-sr.c:230:14: note: (near initialization for
'sr_lcpll0_clk[2].channel')
   drivers/clk/bcm/clk-sr.c:230:14: error: initializer element is not
constant
   drivers/clk/bcm/clk-sr.c:230:14: note: (near initialization for
'sr_lcpll0_clk[2].channel')

vim +217 drivers/clk/bcm/clk-sr.c

654cdd32 Sandeep Tripathy 2017-06-06  182
654cdd32 Sandeep Tripathy 2017-06-06  183  static const struct
iproc_clk_ctrl sr_genpll5_clk[] = {
654cdd32 Sandeep Tripathy 2017-06-06  184  	[BCM_SR_GENPLL5_FS_CLK] =
{
654cdd32 Sandeep Tripathy 2017-06-06  185  		.channel =
BCM_SR_GENPLL5_FS_CLK,
654cdd32 Sandeep Tripathy 2017-06-06  186  		.flags =
IPROC_CLK_AON,
654cdd32 Sandeep Tripathy 2017-06-06  187  		.enable =
ENABLE_VAL(0x4, 6, 0, 12),
654cdd32 Sandeep Tripathy 2017-06-06  188  		.mdiv =
REG_VAL(0x18, 0, 9),
654cdd32 Sandeep Tripathy 2017-06-06  189  	},
654cdd32 Sandeep Tripathy 2017-06-06 @190  	[BCM_SR_GENPLL5_SPU_CLK] =
{
654cdd32 Sandeep Tripathy 2017-06-06  191  		.channel =
BCM_SR_GENPLL5_SPU_CLK,
654cdd32 Sandeep Tripathy 2017-06-06  192  		.flags =
IPROC_CLK_AON,
654cdd32 Sandeep Tripathy 2017-06-06  193  		.enable =
ENABLE_VAL(0x4, 6, 0, 12),
654cdd32 Sandeep Tripathy 2017-06-06  194  		.mdiv =
REG_VAL(0x18, 10, 9),
654cdd32 Sandeep Tripathy 2017-06-06  195  	},
654cdd32 Sandeep Tripathy 2017-06-06  196  };
654cdd32 Sandeep Tripathy 2017-06-06  197
654cdd32 Sandeep Tripathy 2017-06-06  198  static int
sr_genpll5_clk_init(struct platform_device *pdev)
654cdd32 Sandeep Tripathy 2017-06-06  199  {
654cdd32 Sandeep Tripathy 2017-06-06  200
iproc_pll_clk_setup(pdev->dev.of_node,
654cdd32 Sandeep Tripathy 2017-06-06  201
&sr_genpll5, NULL, 0, sr_genpll5_clk,
654cdd32 Sandeep Tripathy 2017-06-06  202
ARRAY_SIZE(sr_genpll5_clk));
654cdd32 Sandeep Tripathy 2017-06-06  203  	return 0;
654cdd32 Sandeep Tripathy 2017-06-06  204  }
654cdd32 Sandeep Tripathy 2017-06-06  205
654cdd32 Sandeep Tripathy 2017-06-06  206  static const struct
iproc_pll_ctrl sr_lcpll0 = {
654cdd32 Sandeep Tripathy 2017-06-06  207  	.flags = IPROC_CLK_AON |
IPROC_CLK_PLL_NEEDS_SW_CFG,
654cdd32 Sandeep Tripathy 2017-06-06  208  	.aon = AON_VAL(0x0, 2, 19,
18),
654cdd32 Sandeep Tripathy 2017-06-06  209  	.reset = RESET_VAL(0x0,
31, 30),
654cdd32 Sandeep Tripathy 2017-06-06  210  	.sw_ctrl =
SW_CTRL_VAL(0x4, 31),
654cdd32 Sandeep Tripathy 2017-06-06  211  	.ndiv_int = REG_VAL(0x4,
16, 10),
654cdd32 Sandeep Tripathy 2017-06-06  212  	.pdiv = REG_VAL(0x4, 26,
4),
654cdd32 Sandeep Tripathy 2017-06-06  213  	.status = REG_VAL(0x38,
12, 1),
654cdd32 Sandeep Tripathy 2017-06-06  214  };
654cdd32 Sandeep Tripathy 2017-06-06  215
654cdd32 Sandeep Tripathy 2017-06-06  216  static const struct
iproc_clk_ctrl sr_lcpll0_clk[] = {
654cdd32 Sandeep Tripathy 2017-06-06 @217
[BCM_SR_LCPLL0_SATA_REF_CLK] = {
654cdd32 Sandeep Tripathy 2017-06-06  218  		.channel =
BCM_SR_LCPLL0_SATA_REF_CLK,
654cdd32 Sandeep Tripathy 2017-06-06  219  		.flags =
IPROC_CLK_AON,
654cdd32 Sandeep Tripathy 2017-06-06  220  		.enable =
ENABLE_VAL(0x0, 7, 1, 13),
654cdd32 Sandeep Tripathy 2017-06-06  221  		.mdiv =
REG_VAL(0x14, 0, 9),
654cdd32 Sandeep Tripathy 2017-06-06  222  	},
654cdd32 Sandeep Tripathy 2017-06-06 @223
[BCM_SR_LCPLL0_USB_REF_CLK] = {
654cdd32 Sandeep Tripathy 2017-06-06  224  		.channel =
BCM_SR_LCPLL0_USB_REF_CLK,
654cdd32 Sandeep Tripathy 2017-06-06  225  		.flags =
IPROC_CLK_AON,
654cdd32 Sandeep Tripathy 2017-06-06  226  		.enable =
ENABLE_VAL(0x0, 8, 2, 14),
654cdd32 Sandeep Tripathy 2017-06-06  227  		.mdiv =
REG_VAL(0x14, 10, 9),
654cdd32 Sandeep Tripathy 2017-06-06  228  	},
654cdd32 Sandeep Tripathy 2017-06-06 @229
[BCM_SR_LCPLL0_SATA_REFPN_CLK] = {
654cdd32 Sandeep Tripathy 2017-06-06  230  		.channel =
BCM_SR_LCPLL0_SATA_REFPN_CLK,
654cdd32 Sandeep Tripathy 2017-06-06  231  		.flags =
IPROC_CLK_AON,
654cdd32 Sandeep Tripathy 2017-06-06  232  		.enable =
ENABLE_VAL(0x0, 9, 3, 15),
654cdd32 Sandeep Tripathy 2017-06-06  233  		.mdiv =
REG_VAL(0x14, 20, 9),
654cdd32 Sandeep Tripathy 2017-06-06  234  	},
654cdd32 Sandeep Tripathy 2017-06-06  235  };
654cdd32 Sandeep Tripathy 2017-06-06  236

:::::: The code at line 217 was first introduced by commit
:::::: 654cdd3229cd1d3f2bfb9df57baf88ba382a52be clk: bcm: Add clocks for
Stingray SOC

:::::: TO: Sandeep Tripathy <sandeep.tripathy@broadcom.com>
:::::: CC: Stephen Boyd <sboyd@codeaurora.org>

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

^ permalink raw reply

* [PATCH] perf: xgene: Fix IOB SLOW PMU parser error
From: Hoan Tran @ 2018-06-06 21:06 UTC (permalink / raw)
  To: linux-arm-kernel

This patch fixes the below parser error of the IOB SLOW PMU.

        # perf stat -a -e iob-slow0/cycle-count/ sleep 1
        evenf syntax error: 'iob-slow0/cycle-count/'
                                 \___ parser error

It replaces the "-" character by "_" character inside the PMU name.

Signed-off-by: Hoan Tran <hoan.tran@amperecomputing.com>
---
 drivers/perf/xgene_pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index 6bdb1da..0e31f13 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -1463,7 +1463,7 @@ static char *xgene_pmu_dev_name(struct device *dev, u32 type, int id)
        case PMU_TYPE_IOB:
                return devm_kasprintf(dev, GFP_KERNEL, "iob%d", id);
        case PMU_TYPE_IOB_SLOW:
-               return devm_kasprintf(dev, GFP_KERNEL, "iob-slow%d", id);
+               return devm_kasprintf(dev, GFP_KERNEL, "iob_slow%d", id);
        case PMU_TYPE_MCB:
                return devm_kasprintf(dev, GFP_KERNEL, "mcb%d", id);
        case PMU_TYPE_MC:
--
2.7.4

CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is for the sole use of the intended recipient(s) and contains information that is confidential and proprietary to Ampere Computing or its subsidiaries. It is to be used solely for the purpose of furthering the parties' business relationship. Any review, copying, or distribution of this email (or any attachments thereto) is strictly prohibited. If you are not the intended recipient, please contact the sender immediately and permanently delete the original and any copies of this email and any attachments thereto.

^ permalink raw reply related

* [PATCH v4 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver
From: Frank Rowand @ 2018-06-06 21:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAL_JsqJCkP_c_wKGRc7Qzkiw8sZLRf6MGz-WgVsLjnQfqK8r6Q@mail.gmail.com>

On 06/06/18 12:35, Rob Herring wrote:
> On Wed, Jun 6, 2018 at 2:30 PM, Frank Rowand <frowand.list@gmail.com> wrote:
>> Hi Michel,
>>
>> On 06/05/18 23:36, Michel Pollet wrote:
>>> Hi Frank,
>>>
>>> On 05 June 2018 18:34, Frank wrote:
>>>> On 06/05/18 04:28, Michel Pollet wrote:
>>>>> The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
>>>>> it requires a special enable method to get it started.
>>>>>
>>>>> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>>>>> ---
>>>>>  arch/arm/mach-shmobile/Makefile        |  1 +
>>>>>  arch/arm/mach-shmobile/smp-r9a06g032.c | 79
>>>>> ++++++++++++++++++++++++++++++++++
>>>>>  2 files changed, 80 insertions(+)
>>>>>  create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c
>>>>>
>>>>> diff --git a/arch/arm/mach-shmobile/Makefile
>>>>> b/arch/arm/mach-shmobile/Makefile index 1939f52..d7fc98f 100644
>>>>> --- a/arch/arm/mach-shmobile/Makefile
>>>>> +++ b/arch/arm/mach-shmobile/Makefile
>>>>> @@ -34,6 +34,7 @@ smp-$(CONFIG_ARCH_SH73A0)+= smp-sh73a0.o
>>>> headsmp-scu.o platsmp-scu.o
>>>>>  smp-$(CONFIG_ARCH_R8A7779)+= smp-r8a7779.o headsmp-scu.o
>>>> platsmp-scu.o
>>>>>  smp-$(CONFIG_ARCH_R8A7790)+= smp-r8a7790.o
>>>>>  smp-$(CONFIG_ARCH_R8A7791)+= smp-r8a7791.o
>>>>> +smp-$(CONFIG_ARCH_R9A06G032)+= smp-r9a06g032.o
>>>>>  smp-$(CONFIG_ARCH_EMEV2)+= smp-emev2.o headsmp-scu.o
>>>> platsmp-scu.o
>>>>>
>>>>>  # PM objects
>>>>> diff --git a/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>>> b/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>>> new file mode 100644
>>>>> index 0000000..cd40e6e
>>>>> --- /dev/null
>>>>> +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>>> @@ -0,0 +1,79 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>> +/*
>>>>> + * R9A06G032 Second CA7 enabler.
>>>>> + *
>>>>> + * Copyright (C) 2018 Renesas Electronics Europe Limited
>>>>> + *
>>>>> + * Michel Pollet <michel.pollet@bp.renesas.com>,
>>>> <buserror@gmail.com>
>>>>> + * Derived from action,s500-smp
>>>>> + */
>>>>> +
>>>>> +#include <linux/io.h>
>>>>> +#include <linux/of.h>
>>>>> +#include <linux/of_address.h>
>>>>> +#include <linux/smp.h>
>>>>> +
>>>>> +/*
>>>>> + * The second CPU is parked in ROM at boot time. It requires waking
>>>>> +it after
>>>>> + * writing an address into the BOOTADDR register of sysctrl.
>>>>> + *
>>>>> + * So the default value of the "cpu-release-addr" corresponds to
>>>> BOOTADDR...
>>>>> + *
>>>>> + * *However* the BOOTADDR register is not available when the kernel
>>>>> + * starts in NONSEC mode.
>>>>> + *
>>>>> + * So for NONSEC mode, the bootloader re-parks the second CPU into a
>>>>> +pen
>>>>> + * in SRAM, and changes the "cpu-release-addr" of linux's DT to a
>>>>> +SRAM address,
>>>>> + * which is not restricted.
>>>>
>>>> The binding document for cpu-release-addr does not have a definition for 32
>>>> bit arm.  The existing definition is only 64 bit arm.  Please add the definition
>>>> for 32 bit arm to patch 1.
>>>
>>> Hmmm I do find a definition in
>>> Documentation/devicetree/bindings/arm/cpus.txt -- just under where I
>>> added my 'enable-method' -- And it is already used as 32 bits in at least
>>> arch/arm/boot/dts/stih407-family.dtsi.
>>
>> From cpus.txt:
>>
>>         - cpu-release-addr
>>                 Usage: required for systems that have an "enable-method"
>>                        property value of "spin-table".
>>                 Value type: <prop-encoded-array>
>>                 Definition:
>>                         # On ARM v8 64-bit systems must be a two cell
>>                           property identifying a 64-bit zero-initialised
>>                           memory location.
>>
>> The definition specifies a two cell property for 64-bit systems.
>>
>> Please add to the definition that cpu-release-addr is a one cell property
>> for 32-bit systems.
> 
> Actually, this is all already documented in the DT spec and it is
> always 2 cells[1]. We should perhaps just remove whatever is
> duplicated from the spec.

Thanks for noting that.  I jumped to the (incorrect) conclusion that the
property should be one cell based on the code and the .dtsi.

There are about 4 more emails following this in the thread that discuss
what size cpu-release-addr should be.  Whatever the end result is (one
cell or two or based on some #XXX-calls value), it needs to be documented
consistently in the binding and in the DT spec (or preferably only in the
DT spec), and if it is a two cell property for this system then
smp-r9a06g032.c and r9a06g032.dtsi need to be adjusted to reflect that.

-Frank

> 
> Rob
> 
> [1]
>    ``cpu-release-addr``   | SD  | ``<u64>``        The
> cpu-release-addr property is required for
>                                                    cpu nodes that have
> an enable-method property
>                                                    value of
> ``"spin-table"``. The value specifies the
>                                                    physical address of
> a spin table entry that
>                                                    releases a
> secondary CPU from its spin loop.
> 

^ permalink raw reply

* [PATCH v4 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver
From: Frank Rowand @ 2018-06-06 21:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <OSBPR01MB2054E3A1E383495F3534B551D2650@OSBPR01MB2054.jpnprd01.prod.outlook.com>

On 06/05/18 23:36, Michel Pollet wrote:
> Hi Frank,
> 
> On 05 June 2018 18:34, Frank wrote:
>> On 06/05/18 04:28, Michel Pollet wrote:
>>> The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
>>> it requires a special enable method to get it started.
>>>
>>> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>>> ---
>>>  arch/arm/mach-shmobile/Makefile        |  1 +
>>>  arch/arm/mach-shmobile/smp-r9a06g032.c | 79
>>> ++++++++++++++++++++++++++++++++++
>>>  2 files changed, 80 insertions(+)
>>>  create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c
>>>
>>> diff --git a/arch/arm/mach-shmobile/Makefile
>>> b/arch/arm/mach-shmobile/Makefile index 1939f52..d7fc98f 100644
>>> --- a/arch/arm/mach-shmobile/Makefile
>>> +++ b/arch/arm/mach-shmobile/Makefile
>>> @@ -34,6 +34,7 @@ smp-$(CONFIG_ARCH_SH73A0)+= smp-sh73a0.o
>> headsmp-scu.o platsmp-scu.o
>>>  smp-$(CONFIG_ARCH_R8A7779)+= smp-r8a7779.o headsmp-scu.o
>> platsmp-scu.o
>>>  smp-$(CONFIG_ARCH_R8A7790)+= smp-r8a7790.o
>>>  smp-$(CONFIG_ARCH_R8A7791)+= smp-r8a7791.o
>>> +smp-$(CONFIG_ARCH_R9A06G032)+= smp-r9a06g032.o
>>>  smp-$(CONFIG_ARCH_EMEV2)+= smp-emev2.o headsmp-scu.o
>> platsmp-scu.o
>>>
>>>  # PM objects
>>> diff --git a/arch/arm/mach-shmobile/smp-r9a06g032.c
>>> b/arch/arm/mach-shmobile/smp-r9a06g032.c
>>> new file mode 100644
>>> index 0000000..cd40e6e
>>> --- /dev/null
>>> +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
>>> @@ -0,0 +1,79 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * R9A06G032 Second CA7 enabler.
>>> + *
>>> + * Copyright (C) 2018 Renesas Electronics Europe Limited
>>> + *
>>> + * Michel Pollet <michel.pollet@bp.renesas.com>,
>> <buserror@gmail.com>
>>> + * Derived from action,s500-smp
>>> + */
>>> +
>>> +#include <linux/io.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/smp.h>
>>> +
>>> +/*
>>> + * The second CPU is parked in ROM at boot time. It requires waking
>>> +it after
>>> + * writing an address into the BOOTADDR register of sysctrl.
>>> + *
>>> + * So the default value of the "cpu-release-addr" corresponds to
>> BOOTADDR...
>>> + *
>>> + * *However* the BOOTADDR register is not available when the kernel
>>> + * starts in NONSEC mode.
>>> + *
>>> + * So for NONSEC mode, the bootloader re-parks the second CPU into a
>>> +pen
>>> + * in SRAM, and changes the "cpu-release-addr" of linux's DT to a
>>> +SRAM address,
>>> + * which is not restricted.
>>
>> The binding document for cpu-release-addr does not have a definition for 32
>> bit arm.  The existing definition is only 64 bit arm.  Please add the definition
>> for 32 bit arm to patch 1.
> 
> Hmmm I do find a definition in
> Documentation/devicetree/bindings/arm/cpus.txt -- just under where I
> added my 'enable-method' -- And it is already used as 32 bits in at least
> arch/arm/boot/dts/stih407-family.dtsi.

If the correct answer is for cpu-release-addr to be 64 bits in certain
cases (that discussion is ongoing further downthread) then one approach
to maintain compatibility _and_ to fix the devicetree source files is
to change the source code that currently gets cpu-release-addr as a
32 bit object to check the size of the property and get it as either
a 32 bit or 64 bit object, based on the actual size of the property
in the device tree and then change the value in the devicetree source
files to be two cells.  BUT this does not consider the bootloader
complication.  arch/arm/boot/dts/axm5516-cpus.dtsi has a note
"// Fixed by the boot loader", so the boot loader also has to be
modified to be able to handle the possibility that the property
could be either 32 bits or 64 bits.  I don't know how to maintain
compatibility with the boot loader since we can't force it to
change synchronously with changes in the kernel.

You can consider this comment to be a drive-by observation.  I think
Rob and Geert and people like that are likely to be more helpful with
what to actually do, and you can treat my comment more as pointing out
the issue than as providing the perfect solution.

-Frnak


> 
> What do you want me to add to this exactly? Do you want me to just
> change "required for systems that have an "enable-method" property
> value of "spin-table" to also specify renesas,r9a06g032 ?
> 
> Thanks!
> Michel
> 
>>
>> -Frank
>>
>>
>>> + */
>>> +
>>> +static void __iomem *cpu_bootaddr;
>>> +
>>> +static DEFINE_SPINLOCK(cpu_lock);
>>> +
>>> +static int r9a06g032_smp_boot_secondary(unsigned int cpu, struct
>>> +task_struct *idle) {
>>> +if (!cpu_bootaddr)
>>> +return -ENODEV;
>>> +
>>> +spin_lock(&cpu_lock);
>>> +
>>> +writel(__pa_symbol(secondary_startup), cpu_bootaddr);
>>> +arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>>> +
>>> +spin_unlock(&cpu_lock);
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +static void __init r9a06g032_smp_prepare_cpus(unsigned int max_cpus)
>>> +{
>>> +struct device_node *dn;
>>> +int ret;
>>> +u32 bootaddr;
>>> +
>>> +dn = of_get_cpu_node(1, NULL);
>>> +if (!dn) {
>>> +pr_err("CPU#1: missing device tree node\n");
>>> +return;
>>> +}
>>> +/*
>>> + * Determine the address from which the CPU is polling.
>>> + * The bootloader *does* change this property
>>> + */
>>> +ret = of_property_read_u32(dn, "cpu-release-addr", &bootaddr);
>>> +of_node_put(dn);
>>> +if (ret) {
>>> +pr_err("CPU#1: invalid cpu-release-addr property\n");
>>> +return;
>>> +}
>>> +pr_info("CPU#1: cpu-release-addr %08x\n", bootaddr);
>>> +
>>> +cpu_bootaddr = ioremap(bootaddr, sizeof(bootaddr)); }
>>> +
>>> +static const struct smp_operations r9a06g032_smp_ops __initconst = {
>>> +.smp_prepare_cpus = r9a06g032_smp_prepare_cpus,
>>> +.smp_boot_secondary = r9a06g032_smp_boot_secondary, };
>>> +CPU_METHOD_OF_DECLARE(r9a06g032_smp, "renesas,r9a06g032-smp",
>>> +&r9a06g032_smp_ops);
>>>
> 
> 
> 
> 
> Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
> 

^ permalink raw reply

* [PATCH v4 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver
From: Frank Rowand @ 2018-06-06 21:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <9cef7124-3020-5741-f3a2-6925a6c8f0f3@gmail.com>

On 06/06/18 14:48, Frank Rowand wrote:
> On 06/05/18 23:36, Michel Pollet wrote:
>> Hi Frank,
>>
>> On 05 June 2018 18:34, Frank wrote:
>>> On 06/05/18 04:28, Michel Pollet wrote:
>>>> The Renesas R9A06G032 second CA7 is parked in a ROM pen at boot time,
>>>> it requires a special enable method to get it started.
>>>>
>>>> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
>>>> ---
>>>>  arch/arm/mach-shmobile/Makefile        |  1 +
>>>>  arch/arm/mach-shmobile/smp-r9a06g032.c | 79
>>>> ++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 80 insertions(+)
>>>>  create mode 100644 arch/arm/mach-shmobile/smp-r9a06g032.c
>>>>
>>>> diff --git a/arch/arm/mach-shmobile/Makefile
>>>> b/arch/arm/mach-shmobile/Makefile index 1939f52..d7fc98f 100644
>>>> --- a/arch/arm/mach-shmobile/Makefile
>>>> +++ b/arch/arm/mach-shmobile/Makefile
>>>> @@ -34,6 +34,7 @@ smp-$(CONFIG_ARCH_SH73A0)+= smp-sh73a0.o
>>> headsmp-scu.o platsmp-scu.o
>>>>  smp-$(CONFIG_ARCH_R8A7779)+= smp-r8a7779.o headsmp-scu.o
>>> platsmp-scu.o
>>>>  smp-$(CONFIG_ARCH_R8A7790)+= smp-r8a7790.o
>>>>  smp-$(CONFIG_ARCH_R8A7791)+= smp-r8a7791.o
>>>> +smp-$(CONFIG_ARCH_R9A06G032)+= smp-r9a06g032.o
>>>>  smp-$(CONFIG_ARCH_EMEV2)+= smp-emev2.o headsmp-scu.o
>>> platsmp-scu.o
>>>>
>>>>  # PM objects
>>>> diff --git a/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>> b/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>> new file mode 100644
>>>> index 0000000..cd40e6e
>>>> --- /dev/null
>>>> +++ b/arch/arm/mach-shmobile/smp-r9a06g032.c
>>>> @@ -0,0 +1,79 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * R9A06G032 Second CA7 enabler.
>>>> + *
>>>> + * Copyright (C) 2018 Renesas Electronics Europe Limited
>>>> + *
>>>> + * Michel Pollet <michel.pollet@bp.renesas.com>,
>>> <buserror@gmail.com>
>>>> + * Derived from action,s500-smp
>>>> + */
>>>> +
>>>> +#include <linux/io.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/of_address.h>
>>>> +#include <linux/smp.h>
>>>> +
>>>> +/*
>>>> + * The second CPU is parked in ROM at boot time. It requires waking
>>>> +it after
>>>> + * writing an address into the BOOTADDR register of sysctrl.
>>>> + *
>>>> + * So the default value of the "cpu-release-addr" corresponds to
>>> BOOTADDR...
>>>> + *
>>>> + * *However* the BOOTADDR register is not available when the kernel
>>>> + * starts in NONSEC mode.
>>>> + *
>>>> + * So for NONSEC mode, the bootloader re-parks the second CPU into a
>>>> +pen
>>>> + * in SRAM, and changes the "cpu-release-addr" of linux's DT to a
>>>> +SRAM address,
>>>> + * which is not restricted.
>>>
>>> The binding document for cpu-release-addr does not have a definition for 32
>>> bit arm.  The existing definition is only 64 bit arm.  Please add the definition
>>> for 32 bit arm to patch 1.
>>
>> Hmmm I do find a definition in
>> Documentation/devicetree/bindings/arm/cpus.txt -- just under where I
>> added my 'enable-method' -- And it is already used as 32 bits in at least
>> arch/arm/boot/dts/stih407-family.dtsi.
> 
> If the correct answer is for cpu-release-addr to be 64 bits in certain
> cases (that discussion is ongoing further downthread) then one approach
> to maintain compatibility _and_ to fix the devicetree source files is
> to change the source code that currently gets cpu-release-addr as a
> 32 bit object to check the size of the property and get it as either
> a 32 bit or 64 bit object, based on the actual size of the property
> in the device tree and then change the value in the devicetree source
> files to be two cells.  BUT this does not consider the bootloader
> complication.  arch/arm/boot/dts/axm5516-cpus.dtsi has a note
> "// Fixed by the boot loader", so the boot loader also has to be
> modified to be able to handle the possibility that the property
> could be either 32 bits or 64 bits.  I don't know how to maintain
> compatibility with the boot loader since we can't force it to
> change synchronously with changes in the kernel.
> 
> You can consider this comment to be a drive-by observation.  I think
> Rob and Geert and people like that are likely to be more helpful with
> what to actually do, and you can treat my comment more as pointing out
> the issue than as providing the perfect solution.

Darn it, hit <send> too quickly.

I meant to mention that there are several devicetree source files that
have a single cell value for cpu-release-addr, and thus potentially
face the same situation, depending on what the final decision is on
the proper size for cpu-release-addr. As of v4.17, a git grep shows
one cell values in:

  arch/arm/boot/dts/axm5516-cpus.dtsi
  arch/arm/boot/dts/stih407-family.dtsi
  arch/arm/boot/dts/stih418.dtsi

-Frank

> -Frnak
> 
> 
>>
>> What do you want me to add to this exactly? Do you want me to just
>> change "required for systems that have an "enable-method" property
>> value of "spin-table" to also specify renesas,r9a06g032 ?
>>
>> Thanks!
>> Michel
>>
>>>
>>> -Frank
>>>
>>>
>>>> + */
>>>> +
>>>> +static void __iomem *cpu_bootaddr;
>>>> +
>>>> +static DEFINE_SPINLOCK(cpu_lock);
>>>> +
>>>> +static int r9a06g032_smp_boot_secondary(unsigned int cpu, struct
>>>> +task_struct *idle) {
>>>> +if (!cpu_bootaddr)
>>>> +return -ENODEV;
>>>> +
>>>> +spin_lock(&cpu_lock);
>>>> +
>>>> +writel(__pa_symbol(secondary_startup), cpu_bootaddr);
>>>> +arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>>>> +
>>>> +spin_unlock(&cpu_lock);
>>>> +
>>>> +return 0;
>>>> +}
>>>> +
>>>> +static void __init r9a06g032_smp_prepare_cpus(unsigned int max_cpus)
>>>> +{
>>>> +struct device_node *dn;
>>>> +int ret;
>>>> +u32 bootaddr;
>>>> +
>>>> +dn = of_get_cpu_node(1, NULL);
>>>> +if (!dn) {
>>>> +pr_err("CPU#1: missing device tree node\n");
>>>> +return;
>>>> +}
>>>> +/*
>>>> + * Determine the address from which the CPU is polling.
>>>> + * The bootloader *does* change this property
>>>> + */
>>>> +ret = of_property_read_u32(dn, "cpu-release-addr", &bootaddr);
>>>> +of_node_put(dn);
>>>> +if (ret) {
>>>> +pr_err("CPU#1: invalid cpu-release-addr property\n");
>>>> +return;
>>>> +}
>>>> +pr_info("CPU#1: cpu-release-addr %08x\n", bootaddr);
>>>> +
>>>> +cpu_bootaddr = ioremap(bootaddr, sizeof(bootaddr)); }
>>>> +
>>>> +static const struct smp_operations r9a06g032_smp_ops __initconst = {
>>>> +.smp_prepare_cpus = r9a06g032_smp_prepare_cpus,
>>>> +.smp_boot_secondary = r9a06g032_smp_boot_secondary, };
>>>> +CPU_METHOD_OF_DECLARE(r9a06g032_smp, "renesas,r9a06g032-smp",
>>>> +&r9a06g032_smp_ops);
>>>>
>>
>>
>>
>>
>> Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
>>
> 
> 

^ permalink raw reply

* [PATCH 06/15] drm/sun4i: tcon: Add support for tcon-top
From: Jernej Škrabec @ 2018-06-06 22:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180604162357.rige4rcsiowdl725@flea>

Dne ponedeljek, 04. junij 2018 ob 18:23:57 CEST je Maxime Ripard napisal(a):
> On Mon, Jun 04, 2018 at 05:09:56PM +0200, Jernej ?krabec wrote:
> > Dne ponedeljek, 04. junij 2018 ob 13:50:34 CEST je Maxime Ripard 
napisal(a):
> > > On Fri, Jun 01, 2018 at 09:19:43AM -0700, Chen-Yu Tsai wrote:
> > > > On Fri, Jun 1, 2018 at 8:29 AM, Maxime Ripard
> > > > <maxime.ripard@bootlin.com>
> > 
> > wrote:
> > > > > On Thu, May 31, 2018 at 07:54:08PM +0200, Jernej ?krabec wrote:
> > > > >> Dne ?etrtek, 31. maj 2018 ob 11:21:33 CEST je Maxime Ripard 
napisal(a):
> > > > >> > On Thu, May 24, 2018 at 03:01:09PM -0700, Chen-Yu Tsai wrote:
> > > > >> > > >> > > + if (tcon->quirks->needs_tcon_top) {
> > > > >> > > >> > > +         struct device_node *np;
> > > > >> > > >> > > +
> > > > >> > > >> > > +         np = of_parse_phandle(dev->of_node,
> > > > >> > > >> > > "allwinner,tcon-top",
> > > > >> > > >> > > 0);
> > > > >> > > >> > > +         if (np) {
> > > > >> > > >> > > +                 struct platform_device *pdev;
> > > > >> > > >> > > +
> > > > >> > > >> > > +                 pdev = of_find_device_by_node(np);
> > > > >> > > >> > > +                 if (pdev)
> > > > >> > > >> > > +                         tcon->tcon_top =
> > > > >> > > >> > > platform_get_drvdata(pdev);
> > > > >> > > >> > > +                 of_node_put(np);
> > > > >> > > >> > > +
> > > > >> > > >> > > +                 if (!tcon->tcon_top)
> > > > >> > > >> > > +                         return -EPROBE_DEFER;
> > > > >> > > >> > > +         }
> > > > >> > > >> > > + }
> > > > >> > > >> > > +
> > > > >> > > >> > 
> > > > >> > > >> > I might have missed it, but I've not seen the bindings
> > > > >> > > >> > additions for
> > > > >> > > >> > that property. This shouldn't really be done that way
> > > > >> > > >> > anyway,
> > > > >> > > >> > instead
> > > > >> > > >> > of using a direct phandle, you should be using the
> > > > >> > > >> > of-graph,
> > > > >> > > >> > with the
> > > > >> > > >> > TCON-top sitting where it belongs in the flow of data.
> > > > >> > > >> 
> > > > >> > > >> Just to answer to the first question, it did describe it in
> > > > >> > > >> "[PATCH
> > > > >> > > >> 07/15] dt- bindings: display: sun4i-drm: Add R40 HDMI
> > > > >> > > >> pipeline".
> > > > >> > > >> 
> > > > >> > > >> As why I designed it that way - HW representation could be
> > > > >> > > >> described
> > > > >> > > >> that way> >>
> > > > >> > > >> 
> > > > >> > > >> (ASCII art makes sense when fixed width font is used to view
> > 
> > it):
> > > > >> > > >>                             / LCD0/LVDS0
> > > > >> > > >>                 
> > > > >> > > >>                 / TCON-LCD0
> > > > >> > > >>                 
> > > > >> > > >>                 |           \ MIPI DSI
> > > > >> > > >> 
> > > > >> > > >> mixer0          |
> > > > >> > > >> 
> > > > >> > > >>        \        / TCON-LCD1 - LCD1/LVDS1
> > > > >> > > >>        
> > > > >> > > >>         TCON-TOP
> > > > >> > > >>        
> > > > >> > > >>        /        \ TCON-TV0 - TVE0/RGB
> > > > >> > > >> 
> > > > >> > > >> mixer1          |          \
> > > > >> > > >> 
> > > > >> > > >>                 |           TCON-TOP - HDMI
> > > > >> > > >>                 |          
> > > > >> > > >>                 |          /
> > > > >> > > >>                 
> > > > >> > > >>                 \ TCON-TV1 - TVE1/RGB
> > > > >> > > >> 
> > > > >> > > >> This is a bit simplified, since there is also TVE-TOP, which
> > > > >> > > >> is
> > > > >> > > >> responsible
> > > > >> > > >> for sharing 4 DACs between both TVE encoders. You can have
> > > > >> > > >> two
> > > > >> > > >> TV outs
> > > > >> > > >> (PAL/ NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa.
> > > > >> > > >> It
> > > > >> > > >> even
> > > > >> > > >> seems that you can arbitrarly choose which DAC is
> > > > >> > > >> responsible
> > > > >> > > >> for
> > > > >> > > >> which signal, so there is a ton of possible end
> > > > >> > > >> combinations,
> > > > >> > > >> but I'm
> > > > >> > > >> not 100% sure.
> > > > >> > > >> 
> > > > >> > > >> Even though I wrote TCON-TOP twice, this is same unit in HW.
> > > > >> > > >> R40
> > > > >> > > >> manual
> > > > >> > > >> suggest more possibilities, although some of them seem
> > > > >> > > >> wrong,
> > > > >> > > >> like RGB
> > > > >> > > >> feeding from LCD TCON. That is confirmed to be wrong when
> > > > >> > > >> checking BSP
> > > > >> > > >> code.
> > > > >> > > >> 
> > > > >> > > >> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0,
> > > > >> > > >> TVE1 and
> > > > >> > > >> LCD1 for pin muxing, although I'm not sure why is that
> > > > >> > > >> needed at
> > > > >> > > >> all,
> > > > >> > > >> since according to R40 datasheet, TVE0 and TVE1 pins are
> > > > >> > > >> dedicated and
> > > > >> > > >> not on PORT D and PORT H, respectively, as TCON-TOP
> > > > >> > > >> documentation
> > > > >> > > >> suggest. However, HSYNC and PSYNC lines might be shared
> > > > >> > > >> between
> > > > >> > > >> TVE
> > > > >> > > >> (when it works in RGB mode) and LCD. But that is just my
> > > > >> > > >> guess
> > > > >> > > >> since
> > > > >> > > >> I'm not really familiar with RGB and LCD interfaces.
> > > > >> > > >> 
> > > > >> > > >> I'm really not sure what would be the best representation in
> > > > >> > > >> OF-graph.
> > > > >> > > >> Can you suggest one?
> > > > >> > > > 
> > > > >> > > > Rob might disagree on this one, but I don't see anything
> > > > >> > > > wrong
> > > > >> > > > with
> > > > >> > > > having loops in the graph. If the TCON-TOP can be both the
> > > > >> > > > input
> > > > >> > > > and
> > > > >> > > > output of the TCONs, then so be it, and have it described
> > > > >> > > > that
> > > > >> > > > way in
> > > > >> > > > the graph.
> > > > >> > > > 
> > > > >> > > > The code is already able to filter out nodes that have
> > > > >> > > > already
> > > > >> > > > been
> > > > >> > > > added to the list of devices we need to wait for in the
> > > > >> > > > component
> > > > >> > > > framework, so that should work as well.
> > > > >> > > > 
> > > > >> > > > And we'd need to describe TVE-TOP as well, even though we
> > > > >> > > > don't
> > > > >> > > > have a
> > > > >> > > > driver for it yet. That will simplify the backward
> > > > >> > > > compatibility
> > > > >> > > > later
> > > > >> > > > on.
> > > > >> > > 
> > > > >> > > I'm getting the feeling that TCON-TOP / TVE-TOP is the glue
> > > > >> > > layer
> > > > >> > > that
> > > > >> > > binds everything together, and provides signal routing, kind of
> > > > >> > > like
> > > > >> > > DE-TOP on A64. So the signal mux controls that were originally
> > > > >> > > found
> > > > >> > > in TCON0 and TVE0 were moved out.
> > > > >> > > 
> > > > >> > > The driver needs to know about that, but the graph about
> > > > >> > > doesn't
> > > > >> > > make
> > > > >> > > much sense directly. Without looking at the manual, I
> > > > >> > > understand it
> > > > >> > > to
> > > > >> > > likely be one mux between the mixers and TCONs, and one between
> > > > >> > > the
> > > > >> > > TCON-TVs and HDMI. Would it make more sense to just have the
> > > > >> > > graph
> > > > >> > > connections between the muxed components, and remove TCON-TOP
> > > > >> > > from
> > > > >> > > it, like we had in the past? A phandle could be used to
> > > > >> > > reference
> > > > >> > > the TCON-TOP for mux controls, in addition to the clocks and
> > > > >> > > resets.
> > > > >> > > 
> > > > >> > > For TVE, we would need something to represent each of the
> > > > >> > > output
> > > > >> > > pins,
> > > > >> > > so the device tree can actually describe what kind of signal,
> > > > >> > > be it
> > > > >> > > each component of RGB/YUV or composite video, is wanted on each
> > > > >> > > pin,
> > > > >> > > if any. This is also needed on the A20 for the Cubietruck, so
> > > > >> > > we
> > > > >> > > can
> > > > >> > > describe which pins are tied to the VGA connector, and which
> > > > >> > > one
> > > > >> > > does
> > > > >> > > R, G, or B.
> > > > >> > 
> > > > >> > I guess we'll see how the DT maintainers feel about this, but my
> > > > >> > impression is that the OF graph should model the flow of data
> > > > >> > between
> > > > >> > the devices. If there's a mux somewhere, then the data is
> > > > >> > definitely
> > > > >> > going through it, and as such it should be part of the graph.
> > > > >> 
> > > > >> I concur, but I spent few days thinking how to represent this
> > > > >> sanely in
> > > > >> graph, but I didn't find any good solution. I'll represent here my
> > > > >> idea and please tell your opinion before I start implementing it.
> > > > >> 
> > > > >> First, let me be clear that mixer0 and mixer1 don't have same
> > > > >> capabilities
> > > > >> (different number of planes, mixer0 supports writeback, mixer1 does
> > > > >> not,
> > > > >> etc.). Thus, it does matter which mixer is connected to which
> > > > >> TCON/encoder.
> > > > >> mixer0 is meant to be connected to main display and mixer1 to
> > > > >> auxiliary. That obviously depends on end system.
> > > > >> 
> > > > >> So, TCON TOP has 3 muxes, which have to be represented in graph.
> > > > >> Two of
> > > > >> them are for mixer/TCON relationship (each of them 1 input and 4
> > > > >> possible outputs) and one for TV TCON/HDMI pair selection (2
> > > > >> possible
> > > > >> inputs, 1 output).
> > > > >> 
> > > > >> According to current practice in sun4i-drm driver, graph has to
> > > > >> have
> > > > >> port 0, representing input and port 1, representing output. This
> > > > >> would
> > > > >> mean that graph looks something like that:
> > > > >> 
> > > > >> tcon_top: tcon-top at 1c70000 {
> > > > >> 
> > > > >>       compatible = "allwinner,sun8i-r40-tcon-top";
> > > > >>       ...
> > > > >>       ports {
> > > > >>       
> > > > >>               #address-cells = <1>;
> > > > >>               #size-cells = <0>;
> > > > >>               
> > > > >>               tcon_top_in: port at 0 {
> > > > >>               
> > > > >>                       #address-cells = <1>;
> > > > >>                       #size-cells = <0>;
> > > > >>                       reg = <0>;
> > > > >>                       
> > > > >>                       tcon_top_in_mixer0: endpoint at 0 {
> > > > >>                       
> > > > >>                               reg = <0>;
> > > > >>                               remote-endpoint =
> > > > >>                               <&mixer0_out_tcon_top>;
> > > > >>                       
> > > > >>                       };
> > > > >>                       
> > > > >>                       tcon_top_in_mixer1: endpoint at 1 {
> > > > >>                       
> > > > >>                               reg = <1>;
> > > > >>                               remote-endpoint =
> > > > >>                               <&mixer1_out_tcon_top>;
> > > > >>                       
> > > > >>                       };
> > > > >>                       
> > > > >>                       tcon_top_in_tcon_tv: endpoint at 2 {
> > > > >>                       
> > > > >>                               reg = <2>;
> > > > >>                               // here is HDMI input connection,
> > > > >>                               part of
> > > > >>                               board DTS
> > > > >>                               remote-endpoint = <board specific
> > > > >>                               phandle
> > > > >>                               to TV TCON output>;
> > > > >>                       
> > > > >>                       };
> > > > >>               
> > > > >>               };
> > > > >>               
> > > > >>               tcon_top_out: port at 1 {
> > > > >>               
> > > > >>                       #address-cells = <1>;
> > > > >>                       #size-cells = <0>;
> > > > >>                       reg = <1>;
> > > > >>                       
> > > > >>                       tcon_top_out_tcon0: endpoint at 0 {
> > > > >>                       
> > > > >>                               reg = <0>;
> > > > >>                               // here is mixer0 output connection,
> > > > >>                               part
> > > > >>                               of board DTS
> > > > >>                               remote-endpoint = <board specific
> > > > >>                               phandle
> > > > >>                               to TCON>;
> > > > >>                       
> > > > >>                       };
> > > > >>                       
> > > > >>                       tcon_top_out_tcon1: endpoint at 1 {
> > > > >>                       
> > > > >>                               reg = <1>;
> > > > >>                               // here is mixer1 output connection,
> > > > >>                               part
> > > > >>                               of board DTS
> > > > >>                               remote-endpoint = <board specific
> > > > >>                               phandle
> > > > >>                               to TCON>;
> > > > >>                       
> > > > >>                       };
> > > > >>                       
> > > > >>                       tcon_top_out_hdmi: endpoint at 2 {
> > > > >>                       
> > > > >>                               reg = <2>;
> > > > >>                               remote-endpoint =
> > > > >>                               <&hdmi_in_tcon_top>;
> > > > >>                       
> > > > >>                       };
> > > > >>               
> > > > >>               };
> > > > >>       
> > > > >>       };
> > > > >> 
> > > > >> };
> > > > > 
> > > > > IIRC, each port is supposed to be one route for the data, so we
> > > > > would
> > > > > have multiple ports, one for the mixers in input and for the tcon in
> > > > > output, and one for the TCON in input and for the HDMI/TV in
> > > > > output. Rob might correct me here.
> > 
> > Ok, that seems more clean approach. I'll have to extend graph traversing
> > algorithm in sun4i_drv.c, but that's no problem.
> > 
> > Just to be clear, you have in mind 3 pairs of ports (0/1 for mixer0 mux,
> > 2/3 for mixer1 and 4/5 for HDMI input), right? That way each mux is
> > represented with one pair of ports, even numbered for input and odd
> > numbered for output.
> Yep, unless Rob feels otherwise.

I found an issue with this concept.

HDMI driver (sun8i_dw_hdmi.c) uses drm_of_find_possible_crtcs() to find 
connected crtcs (TCONs) to HDMI. This function assumes that crtc and encoder 
are directly connected through of_graph, but that is not the case with TCON 
TOP HDMI mux anymore. 
I could give TCON TOP node as an input to this function, but that won't work, 
since TCON TOP can have connections to other crtcs, not only that of HDMI and 
they will also be picked up by drm_of_find_possible_crtcs().

Any suggestion how to solve this nicely? I think creating my own version of 
drm_of_find_possible_crtcs() which considers that case is one way, but not 
very nice solution. Alternatively, we can fix possible_crtcs to BIT(0), since 
it always has only one input. This is done in meson_dw_hdmi.c for example. 

Best regards,
Jernej

^ permalink raw reply

* [PATCH v6 2/2] regulator: add QCOM RPMh regulator driver
From: Doug Anderson @ 2018-06-06 22:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <6d7abf248493cf81d62eb17ecb5030783aa85f72.1528138319.git.collinsd@codeaurora.org>

Hi,

On Mon, Jun 4, 2018 at 12:15 PM, David Collins <collinsd@codeaurora.org> wrote:
> Add the QCOM RPMh regulator driver to manage PMIC regulators
> which are controlled via RPMh on some Qualcomm Technologies, Inc.
> SoCs.  RPMh is a hardware block which contains several
> accelerators which are used to manage various hardware resources
> that are shared between the processors of the SoC.  The final
> hardware state of a regulator is determined within RPMh by
> performing max aggregation of the requests made by all of the
> processors.
>
> Add support for PMIC regulator control via the voltage regulator
> manager (VRM) and oscillator buffer (XOB) RPMh accelerators.
> VRM supports manipulation of enable state, voltage, and mode.
> XOB supports manipulation of enable state.
>
> Signed-off-by: David Collins <collinsd@codeaurora.org>
> ---
>  drivers/regulator/Kconfig               |   9 +
>  drivers/regulator/Makefile              |   1 +
>  drivers/regulator/qcom-rpmh-regulator.c | 767 ++++++++++++++++++++++++++++++++
>  3 files changed, 777 insertions(+)

Assuming Mark is OK with this, it looks good to me now.  My previous
feedback is resolved and I'm OK with the hardcoded current loads in
the driver for now until we come up with something better.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

NOTE: Presumably this can't actually land anywhere :( until the RPMh
patchset lands somewhere and that's still sitting in limbo waiting for
Qualcomm to spin the patches.  Presumably once RPMh lands someone will
need to put it somewhere that can be pulled into the relevant trees so
we don't need to wait for a whole merge window...


-Doug

^ permalink raw reply

* [PATCH v6 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Doug Anderson @ 2018-06-06 22:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <38cb1906c96504f74517018ca4e0aed63bb77356.1528138319.git.collinsd@codeaurora.org>

Hi,

On Mon, Jun 4, 2018 at 12:15 PM, David Collins <collinsd@codeaurora.org> wrote:
> Introduce bindings for RPMh regulator devices found on some
> Qualcomm Technlogies, Inc. SoCs.  These devices allow a given
> processor within the SoC to make PMIC regulator requests which
> are aggregated within the RPMh hardware block along with requests
> from other processors in the SoC to determine the final PMIC
> regulator hardware state.
>
> Signed-off-by: David Collins <collinsd@codeaurora.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/regulator/qcom,rpmh-regulator.txt     | 160 +++++++++++++++++++++
>  .../dt-bindings/regulator/qcom,rpmh-regulator.h    |  36 +++++
>  2 files changed, 196 insertions(+)

Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply

* [PATCH V6] arm64: alternative:flush cache with unpatched code
From: Rohit Khanna @ 2018-06-06 23:29 UTC (permalink / raw)
  To: linux-arm-kernel

In the current implementation,  __apply_alternatives patches
flush_icache_range and then executes it without invalidating the icache.
Thus, icache can contain some of the old instructions for
flush_icache_range. This can cause unpredictable behavior as during
execution we can get a mix of old and new instructions for
flush_icache_range.

This patch modifies __apply_alternatives so that it uses non hot-patched
__flush_icache_all after applying all the alternatives.

Signed-off-by: Rohit Khanna <rokhanna@nvidia.com>
---
 arch/arm64/kernel/alternative.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
index 5c4bce4ac381..047139f570ac 100644
--- a/arch/arm64/kernel/alternative.c
+++ b/arch/arm64/kernel/alternative.c
@@ -154,10 +154,8 @@ static void __apply_alternatives(void *alt_region, bool use_linear_alias)
 			alt_cb  = ALT_REPL_PTR(alt);
 
 		alt_cb(alt, origptr, updptr, nr_inst);
-
-		flush_icache_range((uintptr_t)origptr,
-				   (uintptr_t)(origptr + nr_inst));
 	}
+	__flush_icache_all();
 }
 
 /*
-- 
2.1.4

^ permalink raw reply related

* [PATCH v4 1/6] Documentation: DT: Consolidate SP805 binding docs
From: Ray Jui @ 2018-06-06 23:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CABGGisxEvWDy16SKE2RG-vhUHBZHSCCHyfwJsPY16B1fQS-=cA@mail.gmail.com>



On 6/6/2018 9:33 AM, Rob Herring wrote:
> On Wed, Jun 6, 2018 at 11:19 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 06/05/2018 12:41 PM, Rob Herring wrote:
>>>
>>> On Mon, May 28, 2018 at 11:01:32AM -0700, Ray Jui wrote:
>>>>
>>>> Consolidate two SP805 binding documents "arm,sp805.txt" and
>>>> "sp805-wdt.txt" into "arm,sp805.txt" that matches the naming of the
>>>> desired compatible string to be used
>>>>
>>>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>>>> ---
>>>>    .../devicetree/bindings/watchdog/arm,sp805.txt     | 27
>>>> ++++++++++++++-----
>>>>    .../devicetree/bindings/watchdog/sp805-wdt.txt     | 31
>>>> ----------------------
>>>>    2 files changed, 20 insertions(+), 38 deletions(-)
>>>>    delete mode 100644
>>>> Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>
>>>
>>> Would be good to get a ACK from FSL/NXP person on this. It looks to me
>>> like the driver fetches the wrong clock as it gets the first one and the
>>> driver really wants 'wdog_clk'. In any case, their dts files should be
>>> updated.
>>>
>>
>> This is really confusing, since he deleted file lists apb_pclk first.
>> Does the watchdog driver need apb_pclk or wdog_clk ? That isn't clear to me.
>> arch/arm64/boot/dts/hisilicon/hi3660.dtsi only provides apb_pclk, or at
>> least
>> it says so.
> 
> Note that that clock source is 32KHz. That is obviously a mistake
> because no one clocks their bus/register interface at 32KHz. Someone
> just filled in something that happened to work.
> 
>> The fsl dts files all have apb_pclk first.
> 
> It's all kind of a mess, but fortunately one we should be able to clean-up.
> 

It is indeed a mess. Note the SP805 driver only derive one clock from 
DT, and that's not done based on name. As a result, the first clock 
defined in DT will be fetched and the rate calculation will be carried 
out based on that clock rate.

I assumed the clock entries and their names defined in the binding 
document are just placeholders, at least for the 2nd clock.

Based on how the current driver is, the first clock needs to be the 
WDOGCLK for things to work properly.

According to the SP805 TRM, APB clock is the PCLK, that drives the bus 
for register access.

The relationship between WDOGCLK and PCLK is defined as:

- the rising edges of WDOGCLK must be synchronous and
balanced with a rising edge of PCLK

- the WDOGCLK frequency cannot be greater than the PCLK
frequency

> The compatible string changes too, but AMBA bus devices don't actually
> use the compatible string as they use the ID registers to match. I
> suppose some other OS could do things differently. Worth the risk to
> clean-up IMO.
> 
>>
>> Either case, why are two clocks asked for in the first place ? Are there
>> situations where the second clock is actually used/useful ?
> 
> For clocks, the bus needs "apb_pclk" and the driver just gets the
> first clock. The driver is obviously going to want the functional
> clock that determines the counter rate. That should
> 
> Primecell peripherals are about the only ones that have clear specs
> WRT clock inputs. Yet we've still managed to screw them up. There are
> 2 clocks in the spec, so the DT has (or should have) 2 clocks.
> 
> Rob
> 

Let me know how you guys want to proceed with this?

Thanks,

Ray

^ permalink raw reply

* [PATCH v4 0/6] Enhance support for the SP805 WDT
From: Ray Jui @ 2018-06-06 23:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e3f657f1-bd7e-cadb-9cfe-dee6cf8c1668@gmail.com>

Hi Florian,

On 6/6/2018 12:29 PM, Florian Fainelli wrote:
> On 05/28/2018 11:01 AM, Ray Jui wrote:
>> This patch series enhances the support for the SP805 watchdog timer.
>> First of all, 'timeout-sec' devicetree property is added. In addition,
>> support is also added to allow the driver to reset the watchdog if it
>> has been detected that watchdot has been started in the bootloader. In
>> this case, the driver will initiate the ping service from the kernel
>> watchdog subsystem, before a user mode daemon takes over. This series
>> also enables SP805 in the default ARM64 defconfig
>>
>> This patch series is based off v4.17-rc5 and is available on GIHUB:
>> repo: https://github.com/Broadcom/arm64-linux.git
>> branch: sp805-wdt-v4
>>
>> Changes since v3:
>>   - Improve description of 'timeout-sec' in the binding document, per
>> recommendation from Guenter Roeck
>>
>> Changes since v2:
>>   - Fix indent and format to make them consistent within arm,sp805.txt
>>
>> Changes since v1:
>>   - Consolidate two duplicated SP805 binding documents into one
>>   - Slight change of the wdt_is_running implementation per discussion
>>
>> Ray Jui (6):
>>    Documentation: DT: Consolidate SP805 binding docs
>>    Documentation: DT: Add optional 'timeout-sec' property for sp805
>>    watchdog: sp805: add 'timeout-sec' DT property support
>>    watchdog: sp805: set WDOG_HW_RUNNING when appropriate
>>    arm64: dt: set initial SR watchdog timeout to 60 seconds
>>    arm64: defconfig: add CONFIG_ARM_SP805_WATCHDOG
> 
> I can take the last two patches and Guenter would take the first 4 or
> would you want to proceed differently?
> 

It looks like we still need to figure out how to proceed based on 
discussions with Rob and Guenter on 1/6?

Thanks,

Ray

^ permalink raw reply

* [PATCH v4 1/7] interconnect: Add generic on-chip interconnect API
From: Evan Green @ 2018-06-07  1:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <c98d2ee2-c4fa-eb35-e6ae-f6d9c91ffe6c@linaro.org>

On Wed, Jun 6, 2018 at 11:09 AM Georgi Djakov <georgi.djakov@linaro.org> wrote:
>
> Hi Evan,
>
> On 06/06/2018 05:59 PM, Georgi Djakov wrote:
> >>> +
> >>> +/**
> >>> + * icc_node_create() - create a node
> >>> + * @id: node id
> >>> + *
> >>> + * Return: icc_node pointer on success, or ERR_PTR() on error
> >>> + */
> >>> +struct icc_node *icc_node_create(int id)
> >>> +{
> >>> +       struct icc_node *node;
> >>> +
> >>> +       /* check if node already exists */
> >>> +       node = node_find(id);
> >>> +       if (node)
> >>> +               return node;
> >>
> >> This is probably going to do more harm than good once icc_node_delete comes
> >> in, since it almost certainly indicates a programmer error or ID collision,
> >> and will likely result in a double free. We should probably fail with
> >> EEXIST instead.
> >
> > In the current approach we create the nodes one by one, and the linked
> > nodes are created when they are referenced. The other way around would
> > be to create first all the nodes and then populate the links to avoid
> > the "chicken and egg" problem.
> >
>
> Just to elaborate a bit more on that: We can't actually register all the
> nodes in advance, as we might have multiple interconnect providers
> probing in different order. Each provider may have nodes linking to
> nodes belonging to other providers (not probed yet). That's why we
> create the nodes on the first reference and then, when the actual
> provider driver is probed, the rest of the node data is filled.
>

Ah ok, the extra explanation helped a lot. This makes sense to me. Thanks.
-Evan

^ permalink raw reply

* [PATCH v3 2/5] gpio: syscon: rockchip: add GPIO_MUTE support for rk3328
From: Levin @ 2018-06-07  1:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180605195854.GA16394@rob-hp-laptop>

Rob Herring <robh@kernel.org> writes:

> On Sat, Jun 02, 2018 at 04:40:09PM +0800, Levin Du wrote:
>> 
>> Rob Herring <robh+dt@kernel.org> writes:
>> 
>> > On Thu, May 31, 2018 at 9:05 PM, Levin <djw@t-chip.com.cn> wrote:
>> > > Hi Rob,
>> > > 
>> > > 
>> > > On 2018-05-31 10:45 PM, Rob Herring wrote:
>> > > > 
>> > > > On Wed, May 30, 2018 at 10:27 PM,  <djw@t-chip.com.cn> wrote:
>> > > > > 
>> > > > > From: Levin Du <djw@t-chip.com.cn>
>> > > > > 
>> > > > > In Rockchip RK3328, the output only GPIO_MUTE pin,
>> > > > > originally for codec
>> > > > > mute control, can also be used for general purpose. It is
>> > > > > manipulated by
>> > > > > the GRF_SOC_CON10 register.
>> > > > > 
>> > > > > Signed-off-by: Levin Du <djw@t-chip.com.cn>
>> > > > > 
>> > > > > ---
>> > > > > 
>> > > > > Changes in v3:
>> > > > > - Change from general gpio-syscon to specific
>> > > > > rk3328-gpio-mute
>> > > > > 
>> > > > > Changes in v2:
>> > > > > - Rename gpio_syscon10 to gpio_mute in doc
>> > > > > 
>> > > > > Changes in v1:
>> > > > > - Refactured for general gpio-syscon usage for Rockchip SoCs.
>> > > > > - Add doc rockchip,gpio-syscon.txt
>> > > > > 
>> > > > >   .../bindings/gpio/rockchip,rk3328-gpio-mute.txt    | 28
>> > > > > +++++++++++++++++++
>> > > > >   drivers/gpio/gpio-syscon.c                         | 31
>> > > > > ++++++++++++++++++++++
>> > > > >   2 files changed, 59 insertions(+)
>> > > > >   create mode 100644
>> > > > > Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt
>> > > > > 
>> > > > > diff --git
>> > > > > a/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt
>> > > > > b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt
>> > > > > new file mode 100644
>> > > > > index 0000000..10bc632
>> > > > > --- /dev/null
>> > > > > +++
>> > > > > b/Documentation/devicetree/bindings/gpio/rockchip,rk3328-gpio-mute.txt
>> > > > > @@ -0,0 +1,28 @@
>> > > > > +Rockchip RK3328 GPIO controller dedicated for the GPIO_MUTE
>> > > > > pin.
>> > > > > +
>> > > > > +In Rockchip RK3328, the output only GPIO_MUTE pin,
>> > > > > originally for codec
>> > > > > mute
>> > > > > +control, can also be used for general purpose. It is
>> > > > > manipulated by the
>> > > > > +GRF_SOC_CON10 register.
>> > > > > +
>> > > > > +Required properties:
>> > > > > +- compatible: Should contain "rockchip,rk3328-gpio-mute".
>> > > > > +- gpio-controller: Marks the device node as a gpio
>> > > > > controller.
>> > > > > +- #gpio-cells: Should be 2. The first cell is the pin
>> > > > > number and
>> > > > > +  the second cell is used to specify the gpio polarity:
>> > > > > +    0 = Active high,
>> > > > > +    1 = Active low.
>> > > > > +
>> > > > > +Example:
>> > > > > +
>> > > > > +       grf: syscon at ff100000 {
>> > > > > +               compatible = "rockchip,rk3328-grf", "syscon",
>> > > > > "simple-mfd";
>> > > > > +
>> > > > > +               gpio_mute: gpio-mute {
>> > > > 
>> > > > Node names should be generic:
>> > > > 
>> > > > gpio {
>> > > > 
>> > > > This also means you can't add another GPIO node in the future
>> > > > and
>> > > > you'll have to live with "rockchip,rk3328-gpio-mute" covering
>> > > > more
>> > > > than 1 GPIO if you do need to add more GPIOs.
>> > > 
>> > > 
>> > > As the first line describes, this GPIO controller is dedicated for
>> > > the
>> > > GPIO_MUTE pin.
>> > > There's only one GPIO pin in the GRF_SOC_CON10 register. Therefore
>> > > the
>> > > gpio_mute
>> > > name is proper IMHO.
>> > 
>> > It's how many GPIOs in the GRF, not this register. What I'm saying is
>> > when you come along later to add another GPIO in the GRF, you had
>> > better just add it to this same node. I'm not going to accept another
>> > GPIO controller node within the GRF. You have the cells to support
>> > more than 1, so it would only be a driver change. The compatible
>> > string would then not be ideally named at that point. But compatible
>> > strings are just unique identifiers, so it doesn't really matter what
>> > the string is.
>> > 
>> 
>> I'll try my best to introduce the situation here. The GRF, GPIO0~GPIO3
>> are register blocks in the RK3328 Soc. The GPIO0~GPIO3 contain registers
>> for GPIO operations like reading/writing data, setting direction,
>> interruption etc, which corresponds to the GPIO banks (gpio0~gpio3)
>> defined in rk3328.dtsi:
>
> I'm only talking about GRF functions, not "regular" GPIOs.
>
>> 	pinctrl: pinctrl {
>> 		compatible = "rockchip,rk3328-pinctrl";
>> 		rockchip,grf = <&grf>;
>> 		#address-cells = <2>;
>> 		#size-cells = <2>;
>> 		ranges;
>> 
>> 		gpio0: gpio0 at ff210000 {
>> 			compatible = "rockchip,gpio-bank";
>> 			reg = <0x0 0xff210000 0x0 0x100>;
>> 			interrupts = <GIC_SPI 51 			IRQ_TYPE_LEVEL_HIGH>;
>> 			clocks = <&cru PCLK_GPIO0>;
>> 
>> 			gpio-controller;
>> 			#gpio-cells = <2>;
>> 
>> 			interrupt-controller;
>> 			#interrupt-cells = <2>;
>> 		};
>> 
>> 		gpio1: gpio1 at ff220000 {
>>                //...
>> 		};
>> 
>> 		gpio2: gpio2 at ff230000 {
>>                //...
>> 		};
>> 
>> 		gpio3: gpio3 at ff240000 {
>>                //...
>> 		};
>>         }
>> 
>> However, these general GPIO pins has multiplexed functions and their
>> pull up/down and driving strength can also be configured. These settings
>> are manipulated by the GRF registers in pinctrl driver. Quoted from the
>> TRM, the GRF has the following function:
>> 
>> - IOMUX control
>> - Control the state of GPIO in power-down mode
>> - GPIO PAD pull down and pull up control
>> - Used for common system control
>> - Used to record the system state
>> 
>> Therefore the functions of the GRF are messy and scattered in different
>> nodes. The so-called GPIO_MUTE does not belong to GPIO0~GPIO3. It is
>> manipulated by the GRF_SOC_CON10 register in the GRF block.
>> 
>> > I'm being told both "this is the only GPIO" and "the GRF has too many
>> > different functions for us to tell you what they all are". So which is
>> > it?
>> > 
>> > Rob
>> 
>> They are both true, but lack of context. See the above description.
>
> What I meant was "only GPIO in GRF registers"...
>
> Rob

I check the TRM and schematic once again. In GRF resters, there are also
HDMI GPIOs, which are already covered by the HDMI driver. Aside from
those, MUTE_GPIO is the only GPIO.

Levin

^ permalink raw reply

* [PATCH v1 0/4] clk: rockchip: support clock controller for px30 SoC
From: Elaine Zhang @ 2018-06-07  3:06 UTC (permalink / raw)
  To: linux-arm-kernel

Elaine Zhang (4):
  dt-bindings: add bindings for px30 clock controller
  clk: rockchip: add dt-binding header for px30
  clk: rockchip: add support for half divider
  clk: rockchip: add clock controller for px30

 .../bindings/clock/rockchip,px30-cru.txt           |   67 ++
 drivers/clk/rockchip/Makefile                      |    2 +
 drivers/clk/rockchip/clk-half-divider.c            |  235 +++++
 drivers/clk/rockchip/clk-px30.c                    | 1080 ++++++++++++++++++++
 drivers/clk/rockchip/clk.c                         |   10 +
 drivers/clk/rockchip/clk.h                         |   86 +-
 include/dt-bindings/clock/px30-cru.h               |  402 ++++++++
 7 files changed, 1881 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt
 create mode 100644 drivers/clk/rockchip/clk-half-divider.c
 create mode 100644 drivers/clk/rockchip/clk-px30.c
 create mode 100644 include/dt-bindings/clock/px30-cru.h

-- 
1.9.1

^ permalink raw reply

* [PATCH v1 1/4] dt-bindings: add bindings for px30 clock controller
From: Elaine Zhang @ 2018-06-07  3:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1528340786-462-1-git-send-email-zhangqing@rock-chips.com>

Add devicetree bindings for Rockchip cru which found on
Rockchip SoCs.

Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
 .../bindings/clock/rockchip,px30-cru.txt           | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt

diff --git a/Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt b/Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt
new file mode 100644
index 000000000000..af5a45b680d0
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/rockchip,px30-cru.txt
@@ -0,0 +1,67 @@
+* Rockchip PX30 Clock and Reset Unit
+
+The PX30 clock controller generates and supplies clock to various
+controllers within the SoC and also implements a reset controller for SoC
+peripherals.
+
+Required Properties:
+
+- compatible: PMU for CRU should be "rockchip,px30-pmu-cru"
+- compatible: CRU should be "rockchip,px30-cru"
+- reg: physical base address of the controller and length of memory mapped
+  region.
+- #clock-cells: should be 1.
+- #reset-cells: should be 1.
+
+Optional Properties:
+
+- rockchip,grf: phandle to the syscon managing the "general register files"
+  If missing, pll rates are not changeable, due to the missing pll lock status.
+
+Each clock is assigned an identifier and client nodes can use this identifier
+to specify the clock which they consume. All available clocks are defined as
+preprocessor macros in the dt-bindings/clock/px30-cru.h headers and can be
+used in device tree sources. Similar macros exist for the reset sources in
+these files.
+
+External clocks:
+
+There are several clocks that are generated outside the SoC. It is expected
+that they are defined using standard clock bindings with following
+clock-output-names:
+ - "xin24m" - crystal input - required,
+ - "xin32k" - rtc clock - optional,
+ - "i2sx_clkin" - external I2S clock - optional,
+ - "gmac_clkin" - external GMAC clock - optional
+
+Example: Clock controller node:
+
+	pmucru: pmu-clock-controller at ff2bc000 {
+		compatible = "rockchip,px30-pmucru";
+		reg = <0x0 0xff2bc000 0x0 0x1000>;
+		#clock-cells = <1>;
+		#reset-cells = <1>;
+	};
+
+	cru: clock-controller at ff2b0000 {
+		compatible = "rockchip,px30-cru";
+		reg = <0x0 0xff2b0000 0x0 0x1000>;
+		rockchip,grf = <&grf>;
+		#clock-cells = <1>;
+		#reset-cells = <1>;
+	};
+
+Example: UART controller node that consumes the clock generated by the clock
+  controller:
+
+	uart0: serial at ff030000 {
+		compatible = "rockchip,px30-uart", "snps,dw-apb-uart";
+		reg = <0x0 0xff030000 0x0 0x100>;
+		interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&pmucru SCLK_UART0_PMU>, <&pmucru PCLK_UART0_PMU>;
+		clock-names = "baudclk", "apb_pclk";
+		reg-shift = <2>;
+		reg-io-width = <4>;
+		status = "disabled";
+	};
+
-- 
1.9.1

^ permalink raw reply related


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