* [PATCH 1/2] dt-bindings: amazon: add Amazon Annapurna Labs Alpine support
From: Ronen Krupnik @ 2019-07-28 19:51 UTC (permalink / raw)
To: robh+dt, mark.rutland
Cc: devicetree, linux-kernel, barakw, dwmw, benh, jonnyc, talel,
hhhawa, hanochu, Ronen Krupnik
In-Reply-To: <20190728195135.12661-1-ronenk@amazon.com>
This patch adds DT bindings info for Amazon Annapurna Labs Alpine SOC
and related reference boards.
Signed-off-by: Ronen Krupnik <ronenk@amazon.com>
---
.../devicetree/bindings/arm/amazon,alpine.txt | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/amazon,alpine.txt
diff --git a/Documentation/devicetree/bindings/arm/amazon,alpine.txt b/Documentation/devicetree/bindings/arm/amazon,alpine.txt
new file mode 100644
index 000000000000..58817208421b
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/amazon,alpine.txt
@@ -0,0 +1,23 @@
+Amazon Annapurna Labs Alpine v3 Platform Device Tree Bindings
+---------------------------------------------------------------
+
+Platforms based on Amazon Annapurna Labs Alpine SoC architecture
+shall follow the following scheme:
+
+SoCs
+----
+
+Each device tree root node must specify which exact SoC in alpine
+architecture it uses, using one of the following compatible values:
+
+- alpine v3
+ compatible = "amazon,alpine-v3";
+
+Boards
+------
+
+Each device tree must specify which one or more of the following
+board-specific compatible values:
+
+- alpine-v3 Evaluation Platform (EVP)
+ compatible = "amazon,alpine-v3-evp";
--
2.21.0
^ permalink raw reply related
* [PATCH 2/2] arm64: dts: amazon: add Amazon Annapurna Labs Alpine v3 support
From: Ronen Krupnik @ 2019-07-28 19:51 UTC (permalink / raw)
To: robh+dt, mark.rutland
Cc: devicetree, linux-kernel, barakw, dwmw, benh, jonnyc, talel,
hhhawa, hanochu, Ronen Krupnik
In-Reply-To: <20190728195135.12661-1-ronenk@amazon.com>
This patch adds the initial support for the Amazon Annapurna Labs Alpine v3
Soc and Evaluation Platform (EVP).
Signed-off-by: Ronen Krupnik <ronenk@amazon.com>
---
arch/arm64/boot/dts/Makefile | 1 +
arch/arm64/boot/dts/amazon/Makefile | 1 +
arch/arm64/boot/dts/amazon/alpine-v3-evp.dts | 23 ++
arch/arm64/boot/dts/amazon/alpine-v3.dtsi | 371 +++++++++++++++++++
4 files changed, 396 insertions(+)
create mode 100644 arch/arm64/boot/dts/amazon/Makefile
create mode 100644 arch/arm64/boot/dts/amazon/alpine-v3-evp.dts
create mode 100644 arch/arm64/boot/dts/amazon/alpine-v3.dtsi
diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
index 4690364d584b..25b750f147d2 100644
--- a/arch/arm64/boot/dts/Makefile
+++ b/arch/arm64/boot/dts/Makefile
@@ -3,6 +3,7 @@ subdir-y += actions
subdir-y += al
subdir-y += allwinner
subdir-y += altera
+subdir-y += amazon
subdir-y += amd
subdir-y += amlogic
subdir-y += apm
diff --git a/arch/arm64/boot/dts/amazon/Makefile b/arch/arm64/boot/dts/amazon/Makefile
new file mode 100644
index 000000000000..d71ac87172eb
--- /dev/null
+++ b/arch/arm64/boot/dts/amazon/Makefile
@@ -0,0 +1 @@
+dtb-$(CONFIG_ARCH_ALPINE) += alpine-v3-evp.dtb
diff --git a/arch/arm64/boot/dts/amazon/alpine-v3-evp.dts b/arch/arm64/boot/dts/amazon/alpine-v3-evp.dts
new file mode 100644
index 000000000000..542e3d419904
--- /dev/null
+++ b/arch/arm64/boot/dts/amazon/alpine-v3-evp.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ */
+
+#include "alpine-v3.dtsi"
+
+/ {
+ model = "Amazon Alpine v3 Evaluation Platform (EVP)";
+ compatible = "amazon,alpine-v3-evp", "amazon,alpine-v3";
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+};
+
+&uart0 {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/amazon/alpine-v3.dtsi b/arch/arm64/boot/dts/amazon/alpine-v3.dtsi
new file mode 100644
index 000000000000..dd91d86ec486
--- /dev/null
+++ b/arch/arm64/boot/dts/amazon/alpine-v3.dtsi
@@ -0,0 +1,371 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019, Amazon.com, Inc. or its affiliates. All Rights Reserved
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+ model = "Amazon Alpine v3";
+ compatible = "amazon,alpine-v3";
+
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x0>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster0_l2>;
+ };
+
+ cpu@1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x1>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster0_l2>;
+ };
+
+ cpu@2 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x2>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster0_l2>;
+ };
+
+ cpu@3 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x3>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster0_l2>;
+ };
+
+ cpu@100 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x100>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster1_l2>;
+ };
+
+ cpu@101 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x101>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster1_l2>;
+ };
+
+ cpu@102 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x102>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster1_l2>;
+ };
+
+ cpu@103 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x103>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster1_l2>;
+ };
+
+ cpu@200 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x200>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster2_l2>;
+ };
+
+ cpu@201 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x201>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster2_l2>;
+ };
+
+ cpu@202 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x202>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster2_l2>;
+ };
+
+ cpu@203 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x203>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster2_l2>;
+ };
+
+ cpu@300 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x300>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster3_l2>;
+ };
+
+ cpu@301 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x301>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster3_l2>;
+ };
+
+ cpu@302 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x302>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster3_l2>;
+ };
+
+ cpu@303 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a72";
+ reg = <0x303>;
+ enable-method = "psci";
+ d-cache-size = <0x8000>;
+ d-cache-line-size = <64>;
+ d-cache-sets = <256>;
+ i-cache-size = <0xc000>;
+ i-cache-line-size = <64>;
+ i-cache-sets = <256>;
+ next-level-cache = <&cluster3_l2>;
+ };
+
+ cluster0_l2: cache@0 {
+ compatible = "cache";
+ cache-size = <0x200000>;
+ cache-line-size = <64>;
+ cache-sets = <2048>;
+ cache-level = <2>;
+ };
+
+ cluster1_l2: cache@100 {
+ compatible = "cache";
+ cache-size = <0x200000>;
+ cache-line-size = <64>;
+ cache-sets = <2048>;
+ cache-level = <2>;
+ };
+
+ cluster2_l2: cache@200 {
+ compatible = "cache";
+ cache-size = <0x200000>;
+ cache-line-size = <64>;
+ cache-sets = <2048>;
+ cache-level = <2>;
+ };
+
+ cluster3_l2: cache@300 {
+ compatible = "cache";
+ cache-size = <0x200000>;
+ cache-line-size = <64>;
+ cache-sets = <2048>;
+ cache-level = <2>;
+ };
+
+ };
+
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ secmon@0 {
+ reg = <0x0 0x0 0x0 0x100000>;
+ no-map;
+ };
+ };
+
+ psci {
+ compatible = "arm,psci-0.2";
+ method = "smc";
+ };
+
+ soc {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ interrupt-parent = <&gic>;
+ ranges;
+
+ arch-timer {
+ compatible = "arm,armv8-timer";
+ interrupts = <GIC_PPI 0xd IRQ_TYPE_LEVEL_LOW
+ GIC_PPI 0xe IRQ_TYPE_LEVEL_LOW
+ GIC_PPI 0xb IRQ_TYPE_LEVEL_LOW
+ GIC_PPI 0xa IRQ_TYPE_LEVEL_LOW>;
+ };
+
+ gic: interrupt-controller@f0000000 {
+ compatible = "arm,gic-v3";
+ #interrupt-cells = <3>;
+ #size-cells = <0>;
+ #address-cells = <0>;
+ interrupt-controller;
+ reg = <0x0 0xf0800000 0 0x10000>,
+ <0x0 0xf0a00000 0 0x200000>,
+ <0x0 0xf0000000 0 0x2000>,
+ <0x0 0xf0010000 0 0x1000>,
+ <0x0 0xf0020000 0 0x2000>;
+ interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ msix: msix@fbe00000 {
+ compatible = "al,alpine-msix";
+ reg = <0x0 0xfbe00000 0x0 0x100000>;
+ interrupt-controller;
+ msi-controller;
+ al,msi-base-spi = <160>;
+ al,msi-num-spis = <800>;
+ interrupt-parent = <&gic>;
+ };
+
+ pmu {
+ compatible = "arm,armv8-pmuv3";
+ interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ uart0: serial@fd883000 {
+ compatible = "ns16550a";
+ device_type = "serial";
+ reg = <0x0 0xfd883000 0x0 0x1000>;
+ clock-frequency = <0>; /* filled by uboot */
+ interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+ reg-shift = <2>;
+ reg-io-width = <4>;
+ };
+
+ pci@fbd00000 {
+ compatible = "pci-host-ecam-generic";
+ device_type = "pci";
+ #size-cells = <2>;
+ #address-cells = <3>;
+ #interrupt-cells = <1>;
+ reg = <0x0 0xfbd00000 0x0 0x100000>;
+ interrupt-map-mask = <0xf800 0 0 7>;
+ /* 8 x legacy interrupts for SATA only */
+ interrupt-map = <0x4000 0 0 1 &gic 0 57 IRQ_TYPE_LEVEL_HIGH>,
+ <0x4800 0 0 1 &gic 0 58 IRQ_TYPE_LEVEL_HIGH>,
+ <0x5000 0 0 1 &gic 0 59 IRQ_TYPE_LEVEL_HIGH>,
+ <0x5800 0 0 1 &gic 0 60 IRQ_TYPE_LEVEL_HIGH>,
+ <0x6000 0 0 1 &gic 0 61 IRQ_TYPE_LEVEL_HIGH>,
+ <0x6800 0 0 1 &gic 0 62 IRQ_TYPE_LEVEL_HIGH>,
+ <0x7000 0 0 1 &gic 0 63 IRQ_TYPE_LEVEL_HIGH>,
+ <0x7800 0 0 1 &gic 0 64 IRQ_TYPE_LEVEL_HIGH>;
+ ranges = <0x02000000 0x0 0xfe000000 0x0 0xfe000000 0x0 0x1000000>;
+ bus-range = <0x00 0x00>;
+ };
+ };
+};
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v9 0/8] EDAC drivers for Armada XP L2 and DDR
From: Chris Packham @ 2019-07-28 20:33 UTC (permalink / raw)
To: james.morse@arm.com
Cc: linux-kernel@vger.kernel.org, robh+dt@kernel.org,
linux@armlinux.org.uk, devicetree@vger.kernel.org,
mchehab@kernel.org, mark.rutland@arm.com, jlu@pengutronix.de,
bp@alien8.de, linux-arm-kernel@lists.infradead.org,
patches@armlinux.org.uk, linux-edac@vger.kernel.org
In-Reply-To: <d1dfe8ec-66e8-e2c8-5421-a18d7e7fc8fc@arm.com>
On Fri, 2019-07-26 at 15:53 +0100, James Morse wrote:
> Hi Chris,
>
> On 12/07/2019 04:48, Chris Packham wrote:
> >
> > I still seem to be struggling to get this on anyone's radar.
> Whose radar does it need to cross?
>
That's a good question. The last solid guidance I had was that this
series was going in via the ARM tree. But as you say below that was two
years ago.
I only realised recently that the ARM core seems to have a different
workflow to other kernel subsystems.
>
> >
> > The Reviews/Acks have been given so this should be good to go in
> > via the ARM
> > tree as planned.
> >
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2017-August/5
> > 25561.html
> For your v8 I took this to mean this series was done!
>
> If nothing has changed with Boris and Russell's decision (it was two
> years ago....),
> details of the patch system are here:
>
> https://lore.kernel.org/linux-arm-kernel/20190624142346.pxljv3m4npatd
> iyk@shell.armlinux.org.uk/
>
Thanks for the link. I would have never found that. I did try to follow
what I could find on https://www.armlinux.org.uk/developer/ but a lot
of that seems to be out of date.
I did manage to submit v9 to the ARM patch tracker but naturally I
messed it up the first time https://www.armlinux.org.uk/developer/patch
es/viewpatch.php?id=8877/1 and I'm not sure I got the second attempt
right either https://www.armlinux.org.uk/developer/patches/viewpatch.ph
p?id=8885/1
> Thanks,
>
> James
^ permalink raw reply
* Re: [PATCH v9 8/8] EDAC: armada_xp: Add support for more SoCs
From: Chris Packham @ 2019-07-28 20:52 UTC (permalink / raw)
To: james.morse@arm.com
Cc: linux-kernel@vger.kernel.org, robh+dt@kernel.org,
linux@armlinux.org.uk, devicetree@vger.kernel.org,
mchehab@kernel.org, mark.rutland@arm.com, jlu@pengutronix.de,
bp@alien8.de, linux-arm-kernel@lists.infradead.org,
patches@armlinux.org.uk, linux-edac@vger.kernel.org
In-Reply-To: <128016c1-380f-70c4-3a89-2d3b0edf9f88@arm.com>
On Fri, 2019-07-26 at 15:51 +0100, James Morse wrote:
> Hi Chris,
>
> On 12/07/2019 04:49, Chris Packham wrote:
> >
> > The Armada 38x and other integrated SoCs use a reduced pin count so
> > the
> > width of the SDRAM interface is smaller than the Armada XP SoCs.
> > This
> > means that the definition of "full" and "half" width is reduced
> > from
> > 64/32 to 32/16.
> >
> > diff --git a/drivers/edac/armada_xp_edac.c
> > b/drivers/edac/armada_xp_edac.c
> > index 3759a4fbbdee..7f227bdcbc84 100644
> > --- a/drivers/edac/armada_xp_edac.c
> > +++ b/drivers/edac/armada_xp_edac.c
> > @@ -332,6 +332,11 @@ static int axp_mc_probe(struct platform_device
> > *pdev)
> >
> > axp_mc_read_config(mci);
> >
> > + /* These SoCs have a reduced width bus */
> > + if (of_machine_is_compatible("marvell,armada380") ||
> > + of_machine_is_compatible("marvell,armadaxp-98dx3236"))
> > + drvdata->width /= 2;
> So the hardware's SDRAM_CONFIG_BUS_WIDTH value is wrong? Yuck.
>
The maximum width differs between Armada-XP (64-bit) and Armada-38x
(32-bit). There is still strapping to control half-width vs full-width.
> Is it too late for the DTs on these two systems to provide a DT
> version of the 'bus_width'
> to override the hardware's mis-advertised value?
In an earlier iteration I did have a DT property as you suggest. The
problem is that something like "bus-width = <32>" is ambiguous. On
Armada-XP this means the strapping is for half-width but on Armada-38x
you'd need to strap to full-width. That's why we settled on the mode
interpreting the strapping against SoC[1].
[1] https://lore.kernel.org/linux-arm-kernel/1502444067.1333.7.camel@pe
ngutronix.de/
>
> This way you don't need to grow this list.
>
> Acked-by: James Morse <james.morse@arm.com>
>
>
> Thanks,
>
> James
^ permalink raw reply
* Re: [PATCH 2/2] ARM: dts: rockchip: Add missing unit address to memory node on rk3288-veyron
From: Heiko Stuebner @ 2019-07-28 20:55 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-arm-kernel, linux-rockchip, devicetree, linux-kernel
In-Reply-To: <CAJKOXPcHB9969bqw+aqRh1HYHKDazhhpKy_+uKKcA=5Gkg6+EA@mail.gmail.com>
Am Sonntag, 28. Juli 2019, 13:38:43 CEST schrieb Krzysztof Kozlowski:
> On Sat, 27 Jul 2019 at 17:33, Heiko Stuebner <heiko@sntech.de> wrote:
> >
> > Hi Krzysztof,
> >
> > Am Samstag, 27. Juli 2019, 16:27:36 CEST schrieb Krzysztof Kozlowski:
> > > Fix DTC warning:
> > >
> > > arch/arm/boot/dts/rk3288-veyron.dtsi:21.9-24.4:
> > > Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name
> >
> > please see the comment directly above the memory node on why that needs
> > to stay that way. So no, we'll keep the veyron memory node as is.
>
> Damn it, I missed it.
no worries :-)
^ permalink raw reply
* [PATCH 0/2] at91: add support for Arietta G25
From: Uwe Kleine-König @ 2019-07-28 21:04 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Nicolas Ferre, Alexandre Belloni,
Ludovic Desroches
Cc: devicetree, info, linux-arm-kernel
Hello,
I now took the time to work on mainline support for the Arietta G25.
It boots fine on v5.3-rc1 apart from the issue I reported on the Linux
ARM list yesterday[1].
Best regards
Uwe
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2019-July/669167.html
Uwe Kleine-König (2):
dts: add vendor prefix "acme" for "Acme Systems srl"
ARM: dts: at91: add support for Arietta G25
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
arch/arm/boot/dts/at91sam9g25-arietta.dts | 86 +++++++++++++++++++
2 files changed, 88 insertions(+)
create mode 100644 arch/arm/boot/dts/at91sam9g25-arietta.dts
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 1/2] dts: add vendor prefix "acme" for "Acme Systems srl"
From: Uwe Kleine-König @ 2019-07-28 21:04 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Nicolas Ferre, Alexandre Belloni,
Ludovic Desroches
Cc: devicetree, info, linux-arm-kernel
In-Reply-To: <20190728210403.2730-1-uwe@kleine-koenig.org>
Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 6992bbbbffab..85965e3446bf 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -27,6 +27,8 @@ patternProperties:
description: Abilis Systems
"^abracon,.*":
description: Abracon Corporation
+ "^acme,.*":
+ description: Acme Systems srl
"^actions,.*":
description: Actions Semiconductor Co., Ltd.
"^active-semi,.*":
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 2/2] ARM: dts: at91: add support for Arietta G25
From: Uwe Kleine-König @ 2019-07-28 21:04 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Nicolas Ferre, Alexandre Belloni,
Ludovic Desroches
Cc: devicetree, info, linux-arm-kernel
In-Reply-To: <20190728210403.2730-1-uwe@kleine-koenig.org>
The Arietta G25 is a SBC powered by a AT91SAMG25 running at 400 MHz.
See https://www.acmesystems.it/arietta for more details.
Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
arch/arm/boot/dts/at91sam9g25-arietta.dts | 86 +++++++++++++++++++++++
1 file changed, 86 insertions(+)
create mode 100644 arch/arm/boot/dts/at91sam9g25-arietta.dts
diff --git a/arch/arm/boot/dts/at91sam9g25-arietta.dts b/arch/arm/boot/dts/at91sam9g25-arietta.dts
new file mode 100644
index 000000000000..6c20e02f0ea9
--- /dev/null
+++ b/arch/arm/boot/dts/at91sam9g25-arietta.dts
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Arietta - System On Module
+ * https://www.acmesystems.it/arietta
+ */
+
+/dts-v1/;
+#include "at91sam9g25.dtsi"
+
+/ {
+ model = "Acme Systems Arietta G25";
+ compatible = "acme,ariettag25", "atmel,at91sam9x5", "atmel,at91sam9";
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory {
+ reg = <0x20000000 0x8000000>;
+ };
+
+ clocks {
+ slow_xtal {
+ clock-frequency = <32768>;
+ };
+
+ main_xtal {
+ clock-frequency = <12000000>;
+ };
+ };
+
+ ahb {
+ apb {
+ rtc@fffffeb0 {
+ status = "okay";
+ };
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ arietta_led {
+ label = "arietta_led";
+ gpios = <&pioB 8 GPIO_ACTIVE_HIGH>;
+ linux,default-trigger = "heartbeat";
+ };
+ };
+};
+
+&dbgu {
+ status = "okay";
+};
+
+&mmc0 {
+ pinctrl-0 = <
+ &pinctrl_mmc0_slot0_clk_cmd_dat0
+ &pinctrl_mmc0_slot0_dat1_3>;
+ status = "okay";
+
+ slot@0 {
+ reg = <0>;
+ bus-width = <4>;
+ };
+};
+
+&usart0 {
+ status ="okay";
+};
+
+&usart1 {
+ status ="okay";
+};
+
+&usb0 {
+ status = "okay";
+ num-ports = <3>;
+};
+
+&usb1 {
+ status = "okay";
+};
+
+&usb2 {
+ status = "okay";
+};
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation
From: Morten Borup Petersen @ 2019-07-28 21:27 UTC (permalink / raw)
To: Jassi Brar, Tushar Khandelwal
Cc: Linux Kernel Mailing List, tushar.2nov@gmail.com, nd@arm.com,
Morten Borup Petersen, Rob Herring, Mark Rutland, Devicetree List
In-Reply-To: <CABb+yY04vW-i35N6P57KSKgmMAYkrA2CDyUvA-bLCZMxiZaocw@mail.gmail.com>
On 7/21/19 11:58 PM, Jassi Brar wrote:
> On Wed, Jul 17, 2019 at 2:26 PM Tushar Khandelwal
> <tushar.khandelwal@arm.com> wrote:
>
>> diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
>> new file mode 100644
>> index 000000000000..3a05593414bc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
>> @@ -0,0 +1,108 @@
>> +Arm MHUv2 Mailbox Driver
>> +========================
>> +
>> +The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
>> +between 1 and 124 channel windows to provide unidirectional communication with
>> +remote processor(s).
>> +
>> +Given the unidirectional nature of the device, an MHUv2 mailbox may only be
>> +written to or read from. If a pair of MHU devices is implemented between two
>> +processing elements to provide bidirectional communication, these must be
>> +specified as two separate mailboxes.
>> +
>> +A device tree node for an Arm MHUv2 device must specify either a receiver frame
>> +or a sender frame, indicating which end of the unidirectional MHU device which
>> +the device node entry describes.
>> +
>> +An MHU device must be specified with a transport protocol. The transport
>> +protocol of an MHU device determines the method of data transmission as well as
>> +the number of provided mailboxes.
>> +Following are the possible transport protocol types:
>> +- Single-word: An MHU device implements as many mailboxes as it
>> + provides channel windows. Data is transmitted through
>> + the MHU registers.
>> +- Multi-word: An MHU device implements a single mailbox. All channel windows
>> + will be used during transmission. Data is transmitted through
>> + the MHU registers.
>> +- Doorbell: An MHU device implements as many mailboxes as there are flag
>> + bits available in its channel windows. Optionally, data may
>> + be transmitted through a shared memory region, wherein the MHU
>> + is used strictly as an interrupt generation mechanism.
>> +
>> +Mailbox Device Node:
>> +====================
>> +
>> +Required properties:
>> +--------------------
>> +- compatible: Shall be "arm,mhuv2" & "arm,primecell"
>> +- reg: Contains the mailbox register address range (base
>> + address and length)
>> +- #mbox-cells Shall be 1 - the index of the channel needed.
>> +- mhu-frame Frame type of the device.
>> + Shall be either "sender" or "receiver"
>> +- mhu-protocol Transport protocol of the device. Shall be one of the
>> + following: "single-word", "multi-word", "doorbell"
>> +
>> +Required properties (receiver frame):
>> +-------------------------------------
>> +- interrupts: Contains the interrupt information corresponding to the
>> + combined interrupt of the receiver frame
>> +
>> +Example:
>> +--------
>> +
>> + mbox_mw_tx: mhu@10000000 {
>> + compatible = "arm,mhuv2","arm,primecell";
>> + reg = <0x10000000 0x1000>;
>> + clocks = <&refclk100mhz>;
>> + clock-names = "apb_pclk";
>> + #mbox-cells = <1>;
>> + mhu-protocol = "multi-word";
>> + mhu-frame = "sender";
>> + };
>> +
>> + mbox_sw_tx: mhu@10000000 {
>> + compatible = "arm,mhuv2","arm,primecell";
>> + reg = <0x11000000 0x1000>;
>> + clocks = <&refclk100mhz>;
>> + clock-names = "apb_pclk";
>> + #mbox-cells = <1>;
>> + mhu-protocol = "single-word";
>> + mhu-frame = "sender";
>> + };
>> +
>> + mbox_db_rx: mhu@10000000 {
>> + compatible = "arm,mhuv2","arm,primecell";
>> + reg = <0x12000000 0x1000>;
>> + clocks = <&refclk100mhz>;
>> + clock-names = "apb_pclk";
>> + #mbox-cells = <1>;
>> + interrupts = <0 45 4>;
>> + interrupt-names = "mhu_rx";
>> + mhu-protocol = "doorbell";
>> + mhu-frame = "receiver";
>> + };
>> +
>> + mhu_client: scb@2e000000 {
>> + compatible = "fujitsu,mb86s70-scb-1.0";
>> + reg = <0 0x2e000000 0x4000>;
>> + mboxes =
>> + // For multi-word frames, client may only instantiate a single
>> + // mailbox for a mailbox controller
>> + <&mbox_mw_tx 0>,
>> +
>> + // For single-word frames, client may instantiate as many
>> + // mailboxes as there are channel windows in the MHU
>> + <&mbox_sw_tx 0>,
>> + <&mbox_sw_tx 1>,
>> + <&mbox_sw_tx 2>,
>> + <&mbox_sw_tx 3>,
>> +
>> + // For doorbell frames, client may instantiate as many mailboxes
>> + // as there are bits available in the combined number of channel
>> + // windows ((channel windows * 32) mailboxes)
>> + <mbox_db_rx 0>,
>> + <mbox_db_rx 1>,
>> + ...
>> + <mbox_db_rx 17>;
>> + };
>
> If the mhuv2 instance implements, say, 3 channel windows between
> sender (linux) and receiver (firmware), and Linux runs two protocols
> each requiring 1 and 2-word sized messages respectively. The hardware
> supports that by assigning windows [0] and [1,2] to each protocol.
> However, I don't think the driver can support that. Or does it?
>
Correct, this version of the driver does not support mixing-and-matching
protocols for an MHU device.
Given the current use-cases for the driver, we do not currently see a
need for this functionality. However, as you mention, the hardware does
not restrict this and it would be possible to add in a future version.
> Also I see problem with implementing "protocol modes" in the
> controller driver - 'mhu-protocol' property should go away. And
> 'mhu-frame' is unncessary - presence of interrupt property could imply
> 'receiver', otherwise 'sender'.
>
> Cheers!
>
I agree that the 'mhu-frame' property can be removed and the frame type
can be deduced from whether an interrupt property is present.
In regards to 'mhu-protocol', i still see value in having it as a device
tree property. As mentioned above, mixing protocols within an MHU is not
currently supported. We decided to specify the protocol for an MHU in
the device tree, given that the protocol influences how many mboxes it
is allowed for a mailbox client to register with a controller,
Thanks,
Morten
^ permalink raw reply
* Re: [PATCH 1/4] mailbox: arm_mhuv2: add device tree binding documentation
From: Morten Borup Petersen @ 2019-07-28 21:28 UTC (permalink / raw)
To: Jassi Brar, Tushar Khandelwal
Cc: Linux Kernel Mailing List, tushar.2nov@gmail.com, nd@arm.com,
Morten Borup Petersen, Rob Herring, Mark Rutland, Devicetree List
In-Reply-To: <CABb+yY1SeHTgZQNAHJW+dZG=khah5c5igtKy+MrjADnZF29Aow@mail.gmail.com>
On 7/25/19 7:49 AM, Jassi Brar wrote:
> On Sun, Jul 21, 2019 at 4:58 PM Jassi Brar <jassisinghbrar@gmail.com> wrote:
>>
>> On Wed, Jul 17, 2019 at 2:26 PM Tushar Khandelwal
>> <tushar.khandelwal@arm.com> wrote:
>>
>>> diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
>>> new file mode 100644
>>> index 000000000000..3a05593414bc
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv2.txt
>>> @@ -0,0 +1,108 @@
>>> +Arm MHUv2 Mailbox Driver
>>> +========================
>>> +
>>> +The Arm Message-Handling-Unit (MHU) Version 2 is a mailbox controller that has
>>> +between 1 and 124 channel windows to provide unidirectional communication with
>>> +remote processor(s).
>>> +
>>> +Given the unidirectional nature of the device, an MHUv2 mailbox may only be
>>> +written to or read from. If a pair of MHU devices is implemented between two
>>> +processing elements to provide bidirectional communication, these must be
>>> +specified as two separate mailboxes.
>>> +
>>> +A device tree node for an Arm MHUv2 device must specify either a receiver frame
>>> +or a sender frame, indicating which end of the unidirectional MHU device which
>>> +the device node entry describes.
>>> +
>>> +An MHU device must be specified with a transport protocol. The transport
>>> +protocol of an MHU device determines the method of data transmission as well as
>>> +the number of provided mailboxes.
>>> +Following are the possible transport protocol types:
>>> +- Single-word: An MHU device implements as many mailboxes as it
>>> + provides channel windows. Data is transmitted through
>>> + the MHU registers.
>>> +- Multi-word: An MHU device implements a single mailbox. All channel windows
>>> + will be used during transmission. Data is transmitted through
>>> + the MHU registers.
>>> +- Doorbell: An MHU device implements as many mailboxes as there are flag
>>> + bits available in its channel windows. Optionally, data may
>>> + be transmitted through a shared memory region, wherein the MHU
>>> + is used strictly as an interrupt generation mechanism.
>>> +
>>> +Mailbox Device Node:
>>> +====================
>>> +
>>> +Required properties:
>>> +--------------------
>>> +- compatible: Shall be "arm,mhuv2" & "arm,primecell"
>>> +- reg: Contains the mailbox register address range (base
>>> + address and length)
>>> +- #mbox-cells Shall be 1 - the index of the channel needed.
>>> +- mhu-frame Frame type of the device.
>>> + Shall be either "sender" or "receiver"
>>> +- mhu-protocol Transport protocol of the device. Shall be one of the
>>> + following: "single-word", "multi-word", "doorbell"
>>> +
>>> +Required properties (receiver frame):
>>> +-------------------------------------
>>> +- interrupts: Contains the interrupt information corresponding to the
>>> + combined interrupt of the receiver frame
>>> +
>>> +Example:
>>> +--------
>>> +
>>> + mbox_mw_tx: mhu@10000000 {
>>> + compatible = "arm,mhuv2","arm,primecell";
>>> + reg = <0x10000000 0x1000>;
>>> + clocks = <&refclk100mhz>;
>>> + clock-names = "apb_pclk";
>>> + #mbox-cells = <1>;
>>> + mhu-protocol = "multi-word";
>>> + mhu-frame = "sender";
>>> + };
>>> +
>>> + mbox_sw_tx: mhu@10000000 {
>>> + compatible = "arm,mhuv2","arm,primecell";
>>> + reg = <0x11000000 0x1000>;
>>> + clocks = <&refclk100mhz>;
>>> + clock-names = "apb_pclk";
>>> + #mbox-cells = <1>;
>>> + mhu-protocol = "single-word";
>>> + mhu-frame = "sender";
>>> + };
>>> +
>>> + mbox_db_rx: mhu@10000000 {
>>> + compatible = "arm,mhuv2","arm,primecell";
>>> + reg = <0x12000000 0x1000>;
>>> + clocks = <&refclk100mhz>;
>>> + clock-names = "apb_pclk";
>>> + #mbox-cells = <1>;
>>> + interrupts = <0 45 4>;
>>> + interrupt-names = "mhu_rx";
>>> + mhu-protocol = "doorbell";
>>> + mhu-frame = "receiver";
>>> + };
>>> +
>>> + mhu_client: scb@2e000000 {
>>> + compatible = "fujitsu,mb86s70-scb-1.0";
>>> + reg = <0 0x2e000000 0x4000>;
>>> + mboxes =
>>> + // For multi-word frames, client may only instantiate a single
>>> + // mailbox for a mailbox controller
>>> + <&mbox_mw_tx 0>,
>>> +
>>> + // For single-word frames, client may instantiate as many
>>> + // mailboxes as there are channel windows in the MHU
>>> + <&mbox_sw_tx 0>,
>>> + <&mbox_sw_tx 1>,
>>> + <&mbox_sw_tx 2>,
>>> + <&mbox_sw_tx 3>,
>>> +
>>> + // For doorbell frames, client may instantiate as many mailboxes
>>> + // as there are bits available in the combined number of channel
>>> + // windows ((channel windows * 32) mailboxes)
>>> + <mbox_db_rx 0>,
>>> + <mbox_db_rx 1>,
>>> + ...
>>> + <mbox_db_rx 17>;
>>> + };
>>
>> If the mhuv2 instance implements, say, 3 channel windows between
>> sender (linux) and receiver (firmware), and Linux runs two protocols
>> each requiring 1 and 2-word sized messages respectively. The hardware
>> supports that by assigning windows [0] and [1,2] to each protocol.
>> However, I don't think the driver can support that. Or does it?
>>
> Thinking about it, IMO, the mbox-cell should carry a 128 (4x32) bit
> mask specifying the set of windows (corresponding to the bits set in
> the mask) associated with the channel.
> And the controller driver should see any channel as associated with
> variable number of windows 'N', where N is [0,124]
>
> mhu_client1: proto1@2e000000 {
> .....
> mboxes = <&mbox 0x0 0x0 0x0 0x1>
> }
>
> mhu_client2: proto2@2f000000 {
> .....
> mboxes = <&mbox 0x0 0x0 0x0 0x6>
> }
>
> Cheers!
>
As mentioned in the response to your initial comment, the driver does
not currently support mixing protocols.
If mixing protocols is to be supported in the future, then this seems
like a suitable way of specifying which channels are associated with
which mailboxes (especially for mixing single- and multi-word modes).
However, there still is an issue in that both single-word and doorbell
requires only 1 channel window - and as such, the transport protocol
cannot be deduced from merely the number of masked channel windows.
Furthermore, for doorbell, a mbox may be registered for _each_ available
bit within a channel window (further complicating things if we were to
include mixing protocols in this initial driver version), making
assigning channel windows to mailboxes semantically different from when
assigning to single- or multi-word.
Thanks,
Morten
^ permalink raw reply
* Re: [PATCH v2 2/3] scsi: ufs: Allow resetting the UFS device
From: Linus Walleij @ 2019-07-28 22:09 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Rob Herring, Mark Rutland, Alim Akhtar, Avri Altman, Pedro Sousa,
James E.J. Bottomley, Martin K. Petersen, Andy Gross, Evan Green,
MSM, open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org, linux-scsi
In-Reply-To: <20190606010249.3538-3-bjorn.andersson@linaro.org>
On Thu, Jun 6, 2019 at 3:02 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> Acquire the device-reset GPIO and toggle this to reset the UFS device
> during initialization and host reset.
>
> Based on downstream support implemented by Subhash Jadavani
> <subhashj@codeaurora.org>.
>
> Tested-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
I agree with Rob that just "reset-gpios" looks better.
Otherwise:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH RFC 2/7] pinctrl: sh-pfc: remove incomplete flag "cfg->type"
From: Linus Walleij @ 2019-07-28 23:02 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Geert Uytterhoeven, thierry.reding@gmail.com, Rob Herring,
Mark Rutland, open list:GPIO SUBSYSTEM, linux-pwm,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux-Renesas
In-Reply-To: <1562576868-8124-3-git-send-email-yoshihiro.shimoda.uh@renesas.com>
On Mon, Jul 8, 2019 at 11:08 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> The old commit c58d9c1b26e3 ("sh-pfc: Implement generic pinconf
> support") broke the cfg->type flag to PINMUX_TYPE_FUNCTION because
> sh_pfc_pinconf_set() didn't call sh_pfc_reconfig_pin().
> Now if we fix the cfg->type condition, it gets worse because:
> - Some drivers might be deferred so that .set_mux() will be called
> multiple times.
> - In such the case, the sh-pfc driver returns -EBUSY even if
> the group is the same, and then that driver fails to probe.
>
> Since the pinctrl subsystem already has such conditions according
> to @set_mux and @gpio_request_enable, this patch just remove
> the incomplete flag from sh-pfc/pinctrl.c.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
This looks like it should have a Fixes: tag as well.
Geert will decide what to do with this.
Can all the pinctrl patches be applied independently of the other
changes so Geert can apply and send me those patches in his pull
requests?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 0/6] pinctrl: aspeed: Add AST2600 pinmux support
From: Linus Walleij @ 2019-07-28 23:14 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
johnny_huang, linux-aspeed, ryanchen.aspeed,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
Rob Herring, Joel Stanley, Linux ARM
In-Reply-To: <20190711041942.23202-1-andrew@aj.id.au>
On Thu, Jul 11, 2019 at 6:19 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> This series adds pinmux support for the AST2600. Some more rework was required
> on top of the previous cleanup series, but this rework was focussed on
> supporting features of the AST2600 pinmux rather than fixing issues with the
> existing infrastructure for the ASPEED drivers. Due to the dependences it's
> based on top of pinctrl/devel, so should avoid any more SPDX issues.
>
> ASPEED have been testing the patches on hardware, so even for an initial pass
> there's some confidence in the implementation.
I'm unsure if I need to wait for the DT bindings to be fixed on this
series?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [v5 1/2] dt-bindings: gpio: aspeed: Add SGPIO support
From: Linus Walleij @ 2019-07-28 23:34 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Hongwei Zhang, open list:GPIO SUBSYSTEM, Joel Stanley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-aspeed, Bartosz Golaszewski, Rob Herring, Mark Rutland,
linux-kernel@vger.kernel.org, Linux ARM
In-Reply-To: <ef9d9c17-6e2d-4a4e-ac44-f8da4bb3b8eb@www.fastmail.com>
On Mon, Jul 22, 2019 at 3:42 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> If the clock driver owns the control register, it also needs to know how
> many GPIOs we want to emit on the bus. This seems like an awkward
> configuration parameter for a clock driver.
>
> To avoid the weird parameter we could protect the control register
> with a lock shared between the clock driver and the SGPIO driver. This
> way the SGPIO driver could have the ngpios parameter, and request
> the clock after its written the ngpios value to the control register. A
> regmap would be useful here to avoid the resource clash and it also
> provides the required lock.
Nah. Too complicated.
What about using the clock API locally (in the singleton driver,
much as it is today) though, to give the right abstraction?
See
drivers/gpu/drm/pl111/pl111_display.c
pl111_init_clock_divider() for an example of a local
clock.
> However, you're also going down the path of splitting the driver such
> that there's one instance per bank. With this approach we need to
> solve two problems: Accounting for the total number of GPIOs,
I don't see that as a big problem since each driver instance will
handle 8 GPIOs and don't need to know how many the
other instances have and whether they exist or not.
> and
> only enabling the bus after the last bank has had its driver probed.
That is a bigger problem and a good reason to stick with
some complex driver like this.
> The accounting might be handled by accumulating the number of
> GPIOs in each bank in the control register itself, e.g. the driver
> implementation does something like:
>
> spin_lock(...)
> ctrl = ioread32(...)
> ngpios = FIELD_GET(ASPEED_SGPIO_CTRL_NGPIOS, ctrl);
> ngpios += 8;
> ctrl &= ~ASPEED_SGPIO_CTRL_NGPIOS;
> ctrl |= FIELD_PREP(ASPEED_SGPIO_CTRL_NGPIOS, ngpios);
> iowrite32(ctrl, ...);
> spin_unlock(...);
But why. The gpio_chip only knows the ngpios for its own instance.
It has no business knowing about how many gpios are on the
other chips or not. If this is split across several instances this should
not be accounted that is the point.
> This works on cold boot of the device when the ngpios field is set to
> zero due to reset, however will fail on subsequent warm reboots if
> the GPIO IP block is protected from reset by the SoC's watchdog
> configuration: the field will not be zeroed in this case, and the
> value of the field is represented by `NR_BOOTS * NR_GPIOS`,
> which is incorrect. To do this correctly I guess we would need some
> other global state held in the driver implementation (zeroed when
> the kernel is loaded), and write the incremented value to the control
> register on each probe() invocation.
This is answered about I guess.
> However, that aside, we can't simply enable the bus in the clock
> enable callback if enable is called per-bank, as it is called once on
> the first request with further requests simply refcounted as you
> mentioned. This is exactly the behaviour we can't tolerate with the
> bus: it must only be enabled after the last GPIO bank is registered,
> when we know the total number of GPIOs to emit.
So the bus needs to know the total number of GPIOs or
everything breaks, and that is the blocker for this
divide-and-conquer approach.
Why does the bus need to know the total number of GPIOs?
(Maybe the answer is elsewhere in the thread...)
I guess I will accept it if it is really this complex in the
hardware.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 0/6] pinctrl: aspeed: Add AST2600 pinmux support
From: Andrew Jeffery @ 2019-07-28 23:41 UTC (permalink / raw)
To: Linus Walleij
Cc: open list:GPIO SUBSYSTEM, Rob Herring, Mark Rutland, Joel Stanley,
Ryan Chen, Johnny Huang, linux-aspeed,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, linux-kernel@vger.kernel.org
In-Reply-To: <CACRpkdb4pEdPHbo=3+fJXe9WG8K7A2_xVMtKWCJCfEawDO5wBw@mail.gmail.com>
On Mon, 29 Jul 2019, at 08:44, Linus Walleij wrote:
> On Thu, Jul 11, 2019 at 6:19 AM Andrew Jeffery <andrew@aj.id.au> wrote:
>
> > This series adds pinmux support for the AST2600. Some more rework was required
> > on top of the previous cleanup series, but this rework was focussed on
> > supporting features of the AST2600 pinmux rather than fixing issues with the
> > existing infrastructure for the ASPEED drivers. Due to the dependences it's
> > based on top of pinctrl/devel, so should avoid any more SPDX issues.
> >
> > ASPEED have been testing the patches on hardware, so even for an initial pass
> > there's some confidence in the implementation.
>
> I'm unsure if I need to wait for the DT bindings to be fixed on this
> series?
Yeah, I need to, sorry for the delay. Been distracted by other stuff.
Will send a v2 shortly.
Andrew
^ permalink raw reply
* Re: [v5 1/2] dt-bindings: gpio: aspeed: Add SGPIO support
From: Andrew Jeffery @ 2019-07-29 0:19 UTC (permalink / raw)
To: Linus Walleij
Cc: Hongwei Zhang, open list:GPIO SUBSYSTEM, Joel Stanley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-aspeed, Bartosz Golaszewski, Rob Herring, Mark Rutland,
linux-kernel@vger.kernel.org, Linux ARM
In-Reply-To: <CACRpkdZxsF9gQj0VicVLsPKXg6rKA1mLwbywmazOf0w8PLnOfA@mail.gmail.com>
On Mon, 29 Jul 2019, at 09:04, Linus Walleij wrote:
> On Mon, Jul 22, 2019 at 3:42 AM Andrew Jeffery <andrew@aj.id.au> wrote:
>
> > If the clock driver owns the control register, it also needs to know how
> > many GPIOs we want to emit on the bus. This seems like an awkward
> > configuration parameter for a clock driver.
> >
> > To avoid the weird parameter we could protect the control register
> > with a lock shared between the clock driver and the SGPIO driver. This
> > way the SGPIO driver could have the ngpios parameter, and request
> > the clock after its written the ngpios value to the control register. A
> > regmap would be useful here to avoid the resource clash and it also
> > provides the required lock.
>
> Nah. Too complicated.
>
> What about using the clock API locally (in the singleton driver,
> much as it is today) though, to give the right abstraction?
>
> See
> drivers/gpu/drm/pl111/pl111_display.c
> pl111_init_clock_divider() for an example of a local
> clock.
Thanks, I'll take a look at that.
>
> > However, that aside, we can't simply enable the bus in the clock
> > enable callback if enable is called per-bank, as it is called once on
> > the first request with further requests simply refcounted as you
> > mentioned. This is exactly the behaviour we can't tolerate with the
> > bus: it must only be enabled after the last GPIO bank is registered,
> > when we know the total number of GPIOs to emit.
>
> So the bus needs to know the total number of GPIOs or
> everything breaks, and that is the blocker for this
> divide-and-conquer approach.
>
> Why does the bus need to know the total number of GPIOs?
>
> (Maybe the answer is elsewhere in the thread...)
I didn't answer it explicitly, my apologies.
The behaviour is to periodically emit the state of all enabled GPIOs
(i.e. the ngpios value), one per bus clock cycle. There's no explicit
addressing scheme, the protocol encodes the value for a given GPIO
by its position in the data stream relative to a pulse on the "load data"
(LD) line, whose envelope covers the clock cycle for the last GPIO in
the sequence. Similar to SPI the bus has both out and in lines, which
cater to output/input GPIOs.
A rough timing diagram for a 16-GPIO configuration looks like what
I've pasted here:
https://gist.github.com/amboar/c9543af1957854474b8c05ab357f0675
Hope that helps.
Andrew
^ permalink raw reply
* Re: [RFC,v3 6/9] media: platform: Add Mediatek ISP P1 V4L2 functions
From: Jungo Lin @ 2019-07-29 1:18 UTC (permalink / raw)
To: Tomasz Figa
Cc: devicetree, Sean Cheng (鄭昇弘),
Mauro Carvalho Chehab, Rynn Wu (吳育恩),
srv_heupstream, Rob Herring, Ryan Yu (余孟修),
Frankie Chiu (邱文凱), Hans Verkuil,
Matthias Brugger, Sj Huang,
moderated list:ARM/Mediatek SoC support, Laurent Pinchart,
ddavenport, Frederic Chen (陳俊元), list
In-Reply-To: <CAAFQd5A8zW9s8cewmHnr9HFmrkxDnEqjrTiwLF2m8sKp0619hA@mail.gmail.com>
Hi, Tomasz:
On Fri, 2019-07-26 at 14:49 +0900, Tomasz Figa wrote:
> On Wed, Jul 24, 2019 at 1:31 PM Jungo Lin <jungo.lin@mediatek.com> wrote:
> >
> > Hi, Tomasz:
> >
> > On Tue, 2019-07-23 at 19:21 +0900, Tomasz Figa wrote:
> > > Hi Jungo,
> > >
> > > On Thu, Jul 18, 2019 at 1:39 PM Jungo Lin <jungo.lin@mediatek.com> wrote:
> > > >
> > > > Hi, Tomasz:
> > > >
> > > > On Wed, 2019-07-10 at 18:54 +0900, Tomasz Figa wrote:
> > > > > Hi Jungo,
> > > > >
> > > > > On Tue, Jun 11, 2019 at 11:53:41AM +0800, Jungo Lin wrote:
> > > [snip]
> > > > > > +static void mtk_cam_req_try_isp_queue(struct mtk_cam_dev *cam_dev,
> > > > > > + struct media_request *new_req)
> > > > > > +{
> > > > > > + struct mtk_cam_dev_request *req, *req_safe, *cam_dev_req;
> > > > > > + struct device *dev = &cam_dev->pdev->dev;
> > > > > > +
> > > > > > + dev_dbg(dev, "%s new req:%d", __func__, !new_req);
> > > > > > +
> > > > > > + if (!cam_dev->streaming) {
> > > > > > + cam_dev_req = mtk_cam_req_to_dev_req(new_req);
> > > > > > + spin_lock(&cam_dev->req_lock);
> > > > > > + list_add_tail(&cam_dev_req->list, &cam_dev->req_list);
> > > > > > + spin_unlock(&cam_dev->req_lock);
> > > > > > + dev_dbg(dev, "%s: stream off, no ISP enqueue\n", __func__);
> > > > > > + return;
> > > > > > + }
> > > > > > +
> > > > > > + /* Normal enqueue flow */
> > > > > > + if (new_req) {
> > > > > > + mtk_isp_req_enqueue(dev, new_req);
> > > > > > + return;
> > > > > > + }
> > > > > > +
> > > > > > + /* Flush all media requests wehen first stream on */
> > > > > > + list_for_each_entry_safe(req, req_safe, &cam_dev->req_list, list) {
> > > > > > + list_del(&req->list);
> > > > > > + mtk_isp_req_enqueue(dev, &req->req);
> > > > > > + }
> > > > > > +}
> > > > >
> > > > > This will have to be redone, as per the other suggestions, but generally one
> > > > > would have a function that tries to queue as much as possible from a list to
> > > > > the hardware and another function that adds a request to the list and calls
> > > > > the first function.
> > > > >
> > > >
> > > > We revised this function as below.
> > > > First to check the en-queue conditions:
> > > > a. stream on
> > > > b. The composer buffers in SCP are 3, so we only could has 3 jobs
> > > > at the same time.
> > > >
> > > >
> > > > Second, try to en-queue the frames in the pending job if possible and
> > > > move them into running job list if possible.
> > > >
> > > > The request has been inserted into pending job in mtk_cam_req_validate
> > > > which is used to validate media_request.
> > >
> > > Thanks for replying to each of the comments, that's very helpful.
> > > Snipped out the parts that I agreed with.
> > >
> > > Please note that req_validate is not supposed to change any driver
> > > state. It's only supposed to validate the request. req_queue is the
> > > right callback to insert the request into some internal driver
> > > bookkeeping structures.
> > >
> >
> > Yes, in req_validate function, we don't change any driver state.
> > Below is the function's implementation.
> >
> > a. Call vb2_request_validate(req) to verify media request.
> > b. Update the buffer internal structure buffer.
> > c. Insert the request into pending_job_list to prepare en-queue.
> >
>
> Adding to a list is changing driver state. The callback must not
> modify anything else than the request itself.
>
> Queuing to driver's list should happen in req_queue instead.
>
> [snip]
Ok, got your point. We will move these implementation to .req_queue.
static const struct media_device_ops mtk_cam_media_ops = {
.link_notify = v4l2_pipeline_link_notify,
.req_alloc = mtk_cam_req_alloc,
.req_free = mtk_cam_req_free,
.req_validate = vb2_request_validate,
.req_queue = mtk_cam_req_queue,
};
static void mtk_cam_req_queue(struct media_request *req)
{
struct mtk_cam_dev_request *cam_req = mtk_cam_req_to_dev_req(req);
struct mtk_cam_dev *cam = container_of(req->mdev, struct mtk_cam_dev,
media_dev);
atomic_set(&cam_req->buf_count, vb2_request_buffer_cnt(req));
/* add to pending job list */
spin_lock_irq(&cam->pending_job_lock);
list_add_tail(&cam_req->list, &cam->pending_job_list);
spin_unlock_irq(&cam->pending_job_lock);
vb2_request_queue(req);
}
> > > >
> > > > void mtk_cam_dev_req_try_queue(struct mtk_cam_dev *cam_dev)
> > > > {
> > > > struct mtk_cam_dev_request *req, *req_prev;
> > > > struct list_head enqueue_job_list;
> > > > int buffer_cnt = atomic_read(&cam_dev->running_job_count);
> > > > unsigned long flags;
> > > >
> > > > if (!cam_dev->streaming ||
> > > > buffer_cnt >= MTK_ISP_MAX_RUNNING_JOBS) {
> > >
> > > Do we have a guarantee that cam_dev->running_job_count doesn't
> > > decrement between the atomic_read() above and this line?
> > >
> >
> > Ok, we will use cam->pending_job_lock to protect
> > cam_dev->running_job_count access. Below is the revised version.
> >
> > void mtk_cam_dev_req_try_queue(struct mtk_cam_dev *cam)
> > {
> > struct mtk_cam_dev_request *req, *req_prev;
> > unsigned long flags;
> >
> > if (!cam->streaming) {
> > dev_dbg(cam->dev, "stream is off\n");
> > return;
> > }
> >
> > spin_lock_irqsave(&cam->pending_job_lock, flags);
> > if (atomic_read(&cam->running_job_count) >= MTK_ISP_MAX_RUNNING_JOBS) {
>
> If we use a spin_lock to protect the counter, perhaps we don't need
> the atomic type anymore?
>
Ok, we will remove atomic type usage.
> > dev_dbg(cam->dev, "jobs are full\n");
> > spin_unlock_irqrestore(&cam->pending_job_lock, flags);
> > return;
> > }
> > list_for_each_entry_safe(req, req_prev, &cam->pending_job_list, list) {
>
> Could we instead check the counter here and break if it's >=
> MTK_ISP_MAX_RUNNING_JOBS?
> Then we could increment it here too to simplify the code.
>
Thanks for your advice.
We simplified this function as below:
void mtk_cam_dev_req_try_queue(struct mtk_cam_dev *cam)
{
struct mtk_cam_dev_request *req, *req_prev;
unsigned long flags;
if (!cam->streaming) {
dev_dbg(cam->dev, "stream is off\n");
return;
}
spin_lock_irq(&cam->pending_job_lock);
spin_lock_irqsave(&cam->running_job_lock, flags);
list_for_each_entry_safe(req, req_prev, &cam->pending_job_list, list) {
if (cam->running_job_count >= MTK_ISP_MAX_RUNNING_JOBS) {
dev_dbg(cam->dev, "jobs are full\n");
break;
}
cam->running_job_count++;
list_del(&req->list);
list_add_tail(&req->list, &cam->running_job_list);
mtk_isp_req_enqueue(cam, req);
}
spin_unlock_irqrestore(&cam->running_job_lock, flags);
spin_unlock_irq(&cam->pending_job_lock);
}
> > list_del(&req->list);
> > spin_lock_irqsave(&cam->running_job_lock, flags);
> > list_add_tail(&req->list, &cam->running_job_list);
> > mtk_isp_req_enqueue(cam, req);
> > spin_unlock_irqrestore(&cam->running_job_lock, flags);
> > if (atomic_inc_return(&cam->running_job_count) >=
> > MTK_ISP_MAX_RUNNING_JOBS)
> > break;
>
> With the above suggestion, this if block would go away.
>
> [snip]
Ditto.
> > > > mtk_isp_req_enqueue(cam_dev, req);
> > > > }
> > > > }
> > > >
> > > [snip]
> > > > > > + stride = DIV_ROUND_UP(stride * pixel_byte, 8);
> > > > > > +
> > > > > > + if (pix_fmt == V4L2_PIX_FMT_MTISP_F10)
> > > > > > + stride = ALIGN(stride, 4);
> > > > >
> > > > > Is it expected that only the F10 format needs this alignment?
> > > > >
> > > >
> > > > yes, if the pixel bits of image format is 10, the byte alignment of bpl
> > > > should be 4. Otherwise, it is 8. We will revise this and add more
> > > > comments.
> > >
> > > That means that the B10 format also needs the extra alignment, as
> > > opposed to what the original code did, right?
> > >
> >
> > Sorry for short code snippet.
> > This alignment checking is only applied to F10, no B10.
> > If you like to check the full function, you could check this in this
> > link[1].
> >
> > static void cal_image_pix_mp(struct mtk_cam_dev *cam, unsigned int
> > node_id,
> > struct v4l2_pix_format_mplane *mp)
> > {
> > unsigned int bpl, ppl;
> > unsigned int pixel_bits = get_pixel_bits(mp->pixelformat);
> > unsigned int width = mp->width;
> >
> > if (node_id == MTK_CAM_P1_MAIN_STREAM_OUT) {
> > /* bayer encoding format & 2 bytes alignment */
> > bpl = ALIGN(DIV_ROUND_UP(width * pixel_bits, 8), 2);
> > } else if (node_id == MTK_CAM_P1_PACKED_BIN_OUT) {
> > /*
> > * The FULL-G encoding format
> > * 1 G component per pixel
> > * 1 R component per 4 pixel
> > * 1 B component per 4 pixel
> > * Total 4G/1R/1B in 4 pixel (pixel per line:ppl)
> > */
> > ppl = DIV_ROUND_UP(width * 6, 4);
> > bpl = DIV_ROUND_UP(ppl * pixel_bits, 8);
> >
> > /* 4 bytes alignment for 10 bit & others are 8 bytes */
> > if (pixel_bits == 10)
> > bpl = ALIGN(bpl, 4);
> > else
> > bpl = ALIGN(bpl, 8);
> > }
> >
> > [1]
> > https://crrev.com/c/1712885/2/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam.c#303
> >
>
> Got it, thanks!
>
> [snip]
> > > > > > +
> > > > > > +static struct v4l2_subdev *
> > > > > > +mtk_cam_cio_get_active_sensor(struct mtk_cam_dev *cam_dev)
> > > > > > +{
> > > > > > + struct media_device *mdev = cam_dev->seninf->entity.graph_obj.mdev;
> > > > > > + struct media_entity *entity;
> > > > > > + struct device *dev = &cam_dev->pdev->dev;
> > > > > > + struct v4l2_subdev *sensor;
> > > > >
> > > > > This variable would be unitialized if there is no streaming sensor. Was
> > > > > there no compiler warning generated for this?
> > > > >
> > > >
> > > > No, there is no compiler warning.
> > > > But, we will assign sensor to NULL to avoid unnecessary compiler warning
> > > > with different compiler options.
> > > >
> > >
> > > Thanks. It would be useful if you could check why the compiler you're
> > > using doesn't show a warning here. We might be missing other
> > > uninitialized variables.
> > >
> >
> > We will feedback to your project team to check the possible reason about
> > compiler warning issue.
> >
>
> Do you mean that it was the Clang toolchain used on Chromium OS (e.g.
> emerge chromeos-kernel-4_19)?
> [snip]
Yes, I checked this comment in the Chromium OS build environment.
But, I think I have made the mistake here. I need to check the build
status in the Mediatek's kernel upstream environment. I will pay
attention in next path set upstream.
> > > > > > +
> > > > > > + dev_dbg(dev, "%s: node:%d fd:%d idx:%d\n",
> > > > > > + __func__,
> > > > > > + node->id,
> > > > > > + buf->vbb.request_fd,
> > > > > > + buf->vbb.vb2_buf.index);
> > > > > > +
> > > > > > + /* For request buffers en-queue, handled in mtk_cam_req_try_queue */
> > > > > > + if (vb->vb2_queue->uses_requests)
> > > > > > + return;
> > > > >
> > > > > I'd suggest removing non-request support from this driver. Even if we end up
> > > > > with a need to provide compatibility for non-request mode, then it should be
> > > > > built on top of the requests mode, so that the driver itself doesn't have to
> > > > > deal with two modes.
> > > > >
> > > >
> > > > The purpose of non-request function in this driver is needed by
> > > > our camera middle-ware design. It needs 3A statistics buffers before
> > > > image buffers en-queue. So we need to en-queue 3A statistics with
> > > > non-request mode in this driver. After MW got the 3A statistics data, it
> > > > will en-queue the images, tuning buffer and other meta buffers with
> > > > request mode. Based on this requirement, do you have any suggestion?
> > > > For upstream driver, should we only consider request mode?
> > > >
> > >
> > > Where does that requirement come from? Why the timing of queuing of
> > > the buffers to the driver is important?
> > >
> > > [snip]
> >
> > Basically, this requirement comes from our internal camera
> > middle-ware/3A hal in user space. Since this is not generic requirement,
> > we will follow your original suggestion to keep the request mode only
> > and remove other non-request design in other files. For upstream driver,
> > it should support request mode only.
> >
>
> Note that Chromium OS will use the "upstream driver" and we don't want
> to diverge, so please make the userspace also use only requests. I
> don't see a reason why there would be any need to submit any buffers
> outside of a request.
>
> [snip]
Ok, I have raised your concern to our colleagues and let him to discuss
with you in another communication channel.
> > > > > > +static void mtk_cam_vb2_buf_request_complete(struct vb2_buffer *vb)
> > > > > > +{
> > > > > > + struct mtk_cam_dev *dev = vb2_get_drv_priv(vb->vb2_queue);
> > > > > > +
> > > > > > + v4l2_ctrl_request_complete(vb->req_obj.req,
> > > > > > + dev->v4l2_dev.ctrl_handler);
> > > > >
> > > > > This would end up being called multiple times, once for each video node.
> > > > > Instead, this should be called explicitly by the driver when it completed
> > > > > the request - perhaps in the frame completion handler?
> > > > >
> > > > > With that, we probably wouldn't even need this callback.
> > > > >
> > > >
> > > > First, if we don't implement this callback function, we will receive
> > > > kernel warning as below.
> > > >
> > > > https://elixir.bootlin.com/linux/latest/source/drivers/media/common/videobuf2/videobuf2-v4l2.c#L420
> > > >
> > > > Second, this function is only be called in __vb2_queue_cancel function.
> > > > Moreover, we will remove cam_dev->v4l2_dev.ctrl_handler in next patch.
> > > > So could we just implement dummy empty function?
> > > >
> > > > * @buf_request_complete: a buffer that was never queued to the driver
> > > > but is
> > > > * associated with a queued request was canceled.
> > > > * The driver will have to mark associated objects in the
> > > > * request as completed; required if requests are
> > > > * supported.
> > > >
> > >
> > > Good catch, thanks.
> > >
> > > Sounds like we may indeed need to implement this callback. In
> > > particular, we may need to remove the request that the buffer was
> > > associated with from the driver queue and return the other buffers
> > > associated to it with an error state. This should be similar to
> > > handling a request failure.
> > > [snip]
> >
> > Before calling this callback function, the VB2's stop_streaming has been
> > called. Normally, we will return the buffers belonged to this vb2 queu
> > with error state. On other hand, only if the state of request is
> > MEDIA_REQUEST_STATE_QUEUED, the buf_request_complete will be called in
> > __vb2_queue_cancel function. It hints this media request has been
> > validated and inserted into our driver's pending_job_list or
> > running_job_list. So we will call mtk_cam_dev_req_cleanup() remove these
> > requests from driver's list when streaming is off. Since we have no
> > v4l2_ctrl, do we need to do the above things which is already handled in
> > mtk_cam_vb2_stop_streaming function? Maybe is this callback function
> > only designed for v4l2_ctrl_request_complete usage?
>
> Are you sure that this callback can be only called after
> stop_streaming? Also wouldn't that be after stop_streaming only on 1
> queue? The other queues could still remain streaming, but we still
> have to return corresponding buffers I believe.
>
> Hans, could you clarify what exactly this callback is supposed to do?
>
Ok, we will look forward Hans' comments on this.
> >
> > static void mtk_cam_dev_req_cleanup(struct mtk_cam_dev *cam)
> > {
> > struct mtk_cam_dev_request *req, *req_prev;
> > unsigned long flags;
> >
> > dev_dbg(cam->dev, "%s\n", __func__);
> >
> > spin_lock_irqsave(&cam->pending_job_lock, flags);
> > list_for_each_entry_safe(req, req_prev, &cam->pending_job_list, list)
> > list_del(&req->list);
> > spin_unlock_irqrestore(&cam->pending_job_lock, flags);
> >
> > spin_lock_irqsave(&cam->running_job_lock, flags);
> > list_for_each_entry_safe(req, req_prev, &cam->running_job_list, list)
> > list_del(&req->list);
> > spin_unlock_irqrestore(&cam->running_job_lock, flags);
> > }
> >
> > static void mtk_cam_vb2_stop_streaming(struct vb2_queue *vq)
> > {
> > struct mtk_cam_dev *cam = vb2_get_drv_priv(vq);
> > struct mtk_cam_video_device *node = mtk_cam_vbq_to_vdev(vq);
> > struct device *dev = cam->dev;
> >
> > dev_dbg(dev, "%s node:%d count info:%d", __func__,
> > node->id, atomic_read(&cam->stream_count));
> >
> > mutex_lock(&cam->op_lock);
> > if (atomic_read(&cam->stream_count) == cam->enabled_count)
> > if (v4l2_subdev_call(&cam->subdev, video, s_stream, 0))
> > dev_err(dev, "failed to stop streaming\n");
> >
> > mtk_cam_vb2_return_all_buffers(cam, node, VB2_BUF_STATE_ERROR);
> >
> > /* Check the first node to stream-off */
> > if (!atomic_dec_and_test(&cam->stream_count)) {
> > mutex_unlock(&cam->op_lock);
> > return;
> > }
> > mutex_unlock(&cam->op_lock);
> >
> > mtk_cam_dev_req_cleanup(cam);
> > media_pipeline_stop(&node->vdev.entity);
> > }
>
> [keeping the context for Hans]
>
> Best regards,
> Tomasz
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Best regards,
Jungo
^ permalink raw reply
* Re: [RFC PATCH 08/11] arm: dts: exynos: Add parents and #interconnect-cells to Exynos4412
From: Chanwoo Choi @ 2019-07-29 1:20 UTC (permalink / raw)
To: Marek Szyprowski, cwchoi00, Artur Świgoń
Cc: devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel,
Linux PM list, dri-devel, Krzysztof Kozlowski, MyungJoo Ham,
inki.dae, Seung-Woo Kim, georgi.djakov
In-Reply-To: <cc4c48c3-06a5-c58f-20de-0aa18ae8667e@samsung.com>
Hi,
On 19. 7. 26. 오후 9:02, Marek Szyprowski wrote:
> Hi
>
> On 2019-07-25 15:13, Chanwoo Choi wrote:
>> 2019년 7월 24일 (수) 오전 8:07, Artur Świgoń <a.swigon@partner.samsung.com>님이 작성:
>>> This patch adds two fields tp the Exynos4412 DTS:
>>> - parent: to declare connections between nodes that are not in a
>>> parent-child relation in devfreq;
>>> - #interconnect-cells: required by the interconnect framework.
>>>
>>> Please note that #interconnect-cells is always zero and node IDs are not
>>> hardcoded anywhere.
>>>
>>> Signed-off-by: Artur Świgoń <a.swigon@partner.samsung.com>
>>> ---
>>> arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 1 +
>>> arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++
>>> 2 files changed, 10 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
>>> index ea55f377d17c..bdd61ae86103 100644
>>> --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
>>> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
>>> @@ -106,6 +106,7 @@
>>> &bus_leftbus {
>>> devfreq-events = <&ppmu_leftbus_3>, <&ppmu_rightbus_3>;
>>> vdd-supply = <&buck3_reg>;
>>> + parent = <&bus_dmc>;
>> It is wrong. 'bus_leftbus' has not any h/w dependency of 'bus_dmc'
>> and 'bus_leftbus' is not child of 'bus_dmc'.
>>
>> Even it there are some PMQoS requirement between them,
>> it it not proper to tie both 'bus_leftbus' and 'bus_dmc'.
>
> There is strict dependency between them. DMC bus running at frequency
> lower than left (or right) bus really doesn't make much sense, because
> it will limit the left bus performance. This dependency should be
> modeled somehow.
I misunderstood new 'parent' prototype as the existing 'devfreq' property.
I didn't understand why use the 'devfreq' property because 'bus_dmc' and
'bus_leftbus' don't share the power line. Please ignore my previous comment.
Basically, I agree that it is necessary to support the QoS requirement
between buses or if possible, between bus and gpu.
To support the interconnect between bus_dmc, bus_leftbus and bus_display,
it used the either 'devfreq' or 'parent' properties to connect them.
In order to catch the meaning of 'devfreq' and 'parent' properties,
If possible, it would be separate the usage role of between 'devfreq'
or 'parent' properties. Because it is possible to connect the 'buses'
with only using 'parent' property if all buses in the path uses
the devfreq governors except for 'passive' governor.
- If 'devfreq' property is used between buses,
it indicates that there are requirement of shading of power line.
- If 'parent' property is used between buses,
it indicates that there are requirement of interconnect connection.
>
>>> status = "okay";
>>> };
>>>
>>> diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
>>> index d20db2dfe8e2..a70a671acacd 100644
>>> --- a/arch/arm/boot/dts/exynos4412.dtsi
>>> +++ b/arch/arm/boot/dts/exynos4412.dtsi
>>> @@ -390,6 +390,7 @@
>>> clocks = <&clock CLK_DIV_DMC>;
>>> clock-names = "bus";
>>> operating-points-v2 = <&bus_dmc_opp_table>;
>>> + #interconnect-cells = <0>;
>>> status = "disabled";
>>> };
>>>
>>> @@ -398,6 +399,7 @@
>>> clocks = <&clock CLK_DIV_ACP>;
>>> clock-names = "bus";
>>> operating-points-v2 = <&bus_acp_opp_table>;
>>> + #interconnect-cells = <0>;
>>> status = "disabled";
>>> };
>>>
>>> @@ -406,6 +408,7 @@
>>> clocks = <&clock CLK_DIV_C2C>;
>>> clock-names = "bus";
>>> operating-points-v2 = <&bus_dmc_opp_table>;
>>> + #interconnect-cells = <0>;
>>> status = "disabled";
>>> };
>>>
>>> @@ -459,6 +462,7 @@
>>> clocks = <&clock CLK_DIV_GDL>;
>>> clock-names = "bus";
>>> operating-points-v2 = <&bus_leftbus_opp_table>;
>>> + #interconnect-cells = <0>;
>>> status = "disabled";
>>> };
>>>
>>> @@ -467,6 +471,7 @@
>>> clocks = <&clock CLK_DIV_GDR>;
>>> clock-names = "bus";
>>> operating-points-v2 = <&bus_leftbus_opp_table>;
>>> + #interconnect-cells = <0>;
>>> status = "disabled";
>>> };
>>>
>>> @@ -475,6 +480,7 @@
>>> clocks = <&clock CLK_ACLK160>;
>>> clock-names = "bus";
>>> operating-points-v2 = <&bus_display_opp_table>;
>>> + #interconnect-cells = <0>;
>>> status = "disabled";
>>> };
>>>
>>> @@ -483,6 +489,7 @@
>>> clocks = <&clock CLK_ACLK133>;
>>> clock-names = "bus";
>>> operating-points-v2 = <&bus_fsys_opp_table>;
>>> + #interconnect-cells = <0>;
>>> status = "disabled";
>>> };
>>>
>>> @@ -491,6 +498,7 @@
>>> clocks = <&clock CLK_ACLK100>;
>>> clock-names = "bus";
>>> operating-points-v2 = <&bus_peri_opp_table>;
>>> + #interconnect-cells = <0>;
>>> status = "disabled";
>>> };
>>>
>>> @@ -499,6 +507,7 @@
>>> clocks = <&clock CLK_SCLK_MFC>;
>>> clock-names = "bus";
>>> operating-points-v2 = <&bus_leftbus_opp_table>;
>>> + #interconnect-cells = <0>;
>>> status = "disabled";
>>> };
>>>
>>> --
>>> 2.17.1
>>>
>>
> Best regards
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply
* RE: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT
From: Anson Huang @ 2019-07-29 1:29 UTC (permalink / raw)
To: Abel Vesa, Daniel Baluta
Cc: rui.zhang@intel.com, edubezval@gmail.com,
daniel.lezcano@linaro.org, Rob Herring, Mark Rutland, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Michael Turquette, Stephen Boyd, Lucas Stach, Andrey Smirnov,
Angus Ainslie (Purism), Carlo Caione, Guido Günther,
Leonard Crestez, linux-pm@vger.kernel.org
In-Reply-To: <20190727161736.4dkfqgwftre67v56@fsr-ub1664-175>
Hi, Abel/Daniel
> On 19-07-27 09:33:10, Daniel Baluta wrote:
> > On Sat, Jul 27, 2019 at 9:19 AM Anson Huang <anson.huang@nxp.com>
> wrote:
> > >
> > > Hi, Daniel
> > >
> > > > Subject: Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag
> > > > for IMX8MQ_CLK_TMU_ROOT
> > > >
> > > > Hi all,
> > > >
> > > > latest linux-next hangs at boot.
> > > >
> > > > commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD ->
> master, tag:
> > > > next-20190726, origin/master, origin/HEAD)
> > > > Author: Stephen Rothwell <sfr@canb.auug.org.au>
> > > > Date: Fri Jul 26 15:18:02 2019 +1000
> > > >
> > > > Add linux-next specific files for 20190726
> > > >
> > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > >
> > > >
> > > > I know this is crazy but reverting commit:
> > > >
> > > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef
> (refs/bisect/bad)
> > > > Author: Anson Huang <Anson.Huang@nxp.com>
> > > > Date: Fri Jul 5 12:56:11 2019 +0800
> > > >
> > > > clk: imx8mq: Remove CLK_IS_CRITICAL flag for
> > > > IMX8MQ_CLK_TMU_ROOT
> > > >
> > > > IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the
> driver
> > > > should manage this clock, so no need to have CLK_IS_CRITICAL flag
> > > > set.
> > > >
> > > >
> > > >
> > > > makes the boot work again.
> > > >
> > > > Any idea?
> > >
> > > I just found if disabling SDMA1, then kernel can boot up, it does
> > > NOT make sense TMU clock is related to SDMA1, I will check with design
> and get back to you soon.
> > >
> >
> > Hi Anson,
> >
> > Applying Abel's patch:
> >
> > commit 8816c47db6a82f55bb4d64f62fd9dd3af680f0e4 (HEAD -> master)
> > Author: Abel Vesa <abel.vesa@nxp.com>
> > Date: Tue Jun 25 12:01:56 2019 +0300
> >
> > clk: imx8mq: Mark AHB clock as critical
> >
> > Keep the AHB clock always on since there is no driver to control it and
> > all the other clocks that use it as parent rely on it being always enabled.
> >
> >
> >
> > The kernel boots up again.
> >
> > It make some sense. I don't understand though why having
> > IMX8MQ_CLK_TMU_ROOT as critical also "unhangs" the kernel.
> >
>
> OK, so this is how it works.
>
> By removing the critical flag from TMU, the AHB doesn't stay always on.
> With my patch the AHB is marked as critical and therefore stays on.
>
> The sdma1_clk has as parent the ipg_root which in turn has as parent the
> ahb clock. And I think what happens is some read from the sdma registers
> hangs because, for whatever reason, enabling the sdma1_clk doesn't
> propagate to enable the ahb clock. I might be wrong though.
>
I can explain why Abel's patch can fix this issue, the AHB clock is a MUST
always ON for system bus, while it does NOT have CLK_IS_CRITICAL flag set for now,
when SDMA1 is probed, it will enable its clock, and the clk path is sdma1_clk->ipg_root->ahb,
after SDMA1 probed done, it will disable its clock since no one use it, and it will trigger
the ahb clock to be OFF, as its usecount is added by 1 when probe and decreased by 1 after
SDMA1 probe done, so usecount=0 will trigger AHB clock to be OFF.
So I think the best solution should be applying Abel's patch as you mentioned upper, the TMU
clock patch is NOT the root cause, it just triggers this issue accidently☹
But I saw Abel's AHB patch is still under discussion, so I think we need to speed it up and make
kernel boot up work for development. AHB/IPG are always critical for i.MX SoCs.
Thanks,
Anson.
^ permalink raw reply
* Re: [RFC PATCH 09/11] devfreq: exynos-bus: Add interconnect functionality to exynos-bus
From: Chanwoo Choi @ 2019-07-29 1:52 UTC (permalink / raw)
To: Artur Świgoń, devicetree, linux-arm-kernel,
linux-samsung-soc, linux-kernel, linux-pm, dri-devel
Cc: krzk, myungjoo.ham, inki.dae, sw0312.kim, georgi.djakov,
m.szyprowski
In-Reply-To: <20190723122016.30279-10-a.swigon@partner.samsung.com>
Hi,
On 19. 7. 23. 오후 9:20, Artur Świgoń wrote:
> This patch adds interconnect functionality to the exynos-bus devfreq
> driver.
>
> The SoC topology is a graph (or, more specifically, a tree) and most of its
> edges are taken from the devfreq parent-child hierarchy (cf.
> Documentation/devicetree/bindings/devfreq/exynos-bus.txt). The previous
> patch adds missing edges to the DT (under the name 'parent'). Due to
> unspecified relative probing order, -EPROBE_DEFER may be propagated to
> guarantee that a child is probed before its parent.
>
> Each bus is now an interconnect provider and an interconnect node as well
> (cf. Documentation/interconnect/interconnect.rst), i.e. every bus registers
> itself as a node. Node IDs are not hardcoded but rather assigned at
> runtime, in probing order (subject to the above-mentioned exception
> regarding relative order). This approach allows for using this driver with
> various Exynos SoCs.
>
> The devfreq target() callback provided by exynos-bus now selects either the
> frequency calculated by the devfreq governor or the frequency requested via
> the interconnect API for the given node, whichever is higher.
Basically, I agree to support the QoS requirement between devices.
But, I think that need to consider the multiple cases.
1. When changing the devfreq governor by user,
For example of the connection between bus_dmc/leftbus/display on patch8,
there are possible multiple cases with various devfreq governor
which is changed on the runtime by user through sysfs interface.
If users changes the devfreq governor as following:
Before,
- bus_dmc (simple_ondemand, available frequency 100/200/300/400 MHz)
--> bus_leftbus(simple_ondemand, available frequency 100/200/300/400 MHz)
----> bus_display(passive)
After changed governor of bus_dmc,
if the min_freq by interconnect requirement is 400Mhz,
- bus_dmc (powersave) : min_freq and max_freq and cur_freq is 100MHz
--> bus_leftbus(simple_ondemand) : cur_freq is 400Mhz
----> bus_display(passive)
The final frequency is 400MHz of bus_dmc
even if the min_freq/max_freq/cur_freq is 100MHz.
It cannot show the correct min_freq/max_freq through
devfreq sysfs interface.
2. When disabling the some frequency by devfreq-thermal throttling,
This patch checks the min_freq of interconnect requirement
in the exynos_bus_target() and exynos_bus_passive_target().
Also, it cannot show the correct min_freq/max_freq through
devfreq sysfs interface.
For example of bus_dmc bus,
- The available frequencies are 100MHz, 200MHz, 300MHz, 400MHz
- Disable 400MHz by devfreq-thermal throttling
- min_freq is 100MHz
- max_freq is 300MHz
- min_freq of interconnect is 400MHz
In result, the final frequency is 400MHz by exynos_bus_target()
There are no problem for working. But, the user cannot know
reason why cur_freq is 400MHz even if max_freq is 300MHz.
Basically, update_devfreq() considers the all constraints
of min_freq/max_freq to decide the proper target frequency.
3.
I think that the exynos_bus_passive_target() is used for devfreq device
using 'passive' governor. The frequency already depends on the parent device.
If already the parent devfreq device like bus_leftbus consider
the minimum frequency of QoS requirement like interconnect,
it is not necessary. The next frequency of devfreq device
with 'passive' governor, it will apply the QoS requirement
without any additional code.
>
> Please note that it is not an error when CONFIG_INTERCONNECT is 'n', in
> which case all interconnect API functions are no-op.
>
> Signed-off-by: Artur Świgoń <a.swigon@partner.samsung.com>
> ---
> drivers/devfreq/exynos-bus.c | 145 +++++++++++++++++++++++++++++++++++
> 1 file changed, 145 insertions(+)
>
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index 412511ca7703..12fb7c84ae50 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -14,6 +14,7 @@
> #include <linux/devfreq-event.h>
> #include <linux/device.h>
> #include <linux/export.h>
> +#include <linux/interconnect-provider.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/pm_opp.h>
> @@ -23,6 +24,8 @@
> #define DEFAULT_SATURATION_RATIO 40
> #define DEFAULT_VOLTAGE_TOLERANCE 2
>
> +#define icc_units_to_hz(x) ((x) * 1000UL / 8)
> +
> struct exynos_bus {
> struct device *dev;
>
> @@ -31,12 +34,17 @@ struct exynos_bus {
> unsigned int edev_count;
> struct mutex lock;
>
> + unsigned long min_freq;
> unsigned long curr_freq;
>
> struct regulator *regulator;
> struct clk *clk;
> unsigned int voltage_tolerance;
> unsigned int ratio;
> +
> + /* One provider per bus, one node per provider */
> + struct icc_provider provider;
> + struct icc_node *node;
> };
>
> /*
> @@ -61,6 +69,13 @@ exynos_bus_ops_edev(enable_edev);
> exynos_bus_ops_edev(disable_edev);
> exynos_bus_ops_edev(set_event);
>
> +static int exynos_bus_next_id(void)
> +{
> + static int exynos_bus_node_id;
> +
> + return exynos_bus_node_id++;
> +}
> +
> static int exynos_bus_get_event(struct exynos_bus *bus,
> struct devfreq_event_data *edata)
> {
> @@ -98,6 +113,8 @@ static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 flags)
> unsigned long old_freq, new_freq, new_volt, tol;
> int ret = 0;
>
> + *freq = max(*freq, bus->min_freq);
> +
> /* Get new opp-bus instance according to new bus clock */
> new_opp = devfreq_recommended_opp(dev, freq, flags);
> if (IS_ERR(new_opp)) {
> @@ -208,6 +225,8 @@ static int exynos_bus_passive_target(struct device *dev, unsigned long *freq,
> unsigned long old_freq, new_freq;
> int ret = 0;
>
> + *freq = max(*freq, bus->min_freq);
> +
> /* Get new opp-bus instance according to new bus clock */
> new_opp = devfreq_recommended_opp(dev, freq, flags);
> if (IS_ERR(new_opp)) {
> @@ -251,6 +270,35 @@ static void exynos_bus_passive_exit(struct device *dev)
> clk_disable_unprepare(bus->clk);
> }
>
> +static int exynos_bus_icc_set(struct icc_node *src, struct icc_node *dst)
> +{
> + struct exynos_bus *src_bus = src->data, *dst_bus = dst->data;
> +
> + src_bus->min_freq = icc_units_to_hz(src->peak_bw);
> + dst_bus->min_freq = icc_units_to_hz(dst->peak_bw);
> +
> + return 0;
> +}
> +
> +static int exynos_bus_icc_aggregate(struct icc_node *node, u32 avg_bw,
> + u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
> +{
> + *agg_peak = *agg_avg = peak_bw;
> +
> + return 0;
> +}
> +
> +static struct icc_node *exynos_bus_icc_xlate(struct of_phandle_args *spec,
> + void *data)
> +{
> + struct exynos_bus *bus = data;
> +
> + if (spec->np != bus->dev->of_node)
> + return ERR_PTR(-EINVAL);
> +
> + return bus->node;
> +}
> +
> static int exynos_bus_parent_parse_of(struct device_node *np,
> struct exynos_bus *bus)
> {
> @@ -469,6 +517,95 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus,
> return ret;
> }
>
> +static int exynos_bus_icc_connect(struct exynos_bus *bus)
> +{
> + struct device_node *np = bus->dev->of_node;
> + struct devfreq *parent_devfreq;
> + struct icc_node *parent_node = NULL;
> + struct of_phandle_args args;
> + int ret = 0;
> +
> + parent_devfreq = devfreq_get_devfreq_by_phandle(bus->dev, 0);
> + if (!IS_ERR(parent_devfreq)) {
> + struct exynos_bus *parent_bus;
> +
> + parent_bus = dev_get_drvdata(parent_devfreq->dev.parent);
> + parent_node = parent_bus->node;
> + } else {
> + /* Look for parent in DT */
> + int num = of_count_phandle_with_args(np, "parent",
> + "#interconnect-cells");
> + if (num != 1)
> + goto out;
> +
> + ret = of_parse_phandle_with_args(np, "parent",
> + "#interconnect-cells",
> + 0, &args);
> + if (ret < 0)
> + goto out;
> +
> + of_node_put(args.np);
> +
> + parent_node = of_icc_get_from_provider(&args);
> + if (IS_ERR(parent_node)) {
> + /* May be -EPROBE_DEFER */
> + ret = PTR_ERR(parent_node);
> + goto out;
> + }
> + }
> +
> + ret = icc_link_create(bus->node, parent_node->id);
> +
> +out:
> + return ret;
> +}
> +
> +static int exynos_bus_icc_init(struct exynos_bus *bus)
> +{
> + struct device *dev = bus->dev;
> + struct icc_provider *provider = &bus->provider;
> + struct icc_node *node;
> + int id, ret;
> +
> + /* Initialize the interconnect provider */
> + provider->set = exynos_bus_icc_set;
> + provider->aggregate = exynos_bus_icc_aggregate;
> + provider->xlate = exynos_bus_icc_xlate;
> + provider->dev = dev;
> + provider->data = bus;
> +
> + ret = icc_provider_add(provider);
> + if (ret < 0)
> + goto out;
> +
> + id = exynos_bus_next_id();
> + node = icc_node_create(id);
> + if (IS_ERR(node)) {
> + ret = PTR_ERR(node);
> + goto err_node;
> + }
> +
> + bus->node = node;
> + node->name = dev->of_node->name;
> + node->data = bus;
> + icc_node_add(node, provider);
> +
> + ret = exynos_bus_icc_connect(bus);
> + if (ret < 0)
> + goto err_connect;
> +
> +out:
> + return ret;
> +
> +err_connect:
> + icc_node_del(node);
> + icc_node_destroy(id);
> +err_node:
> + icc_provider_del(provider);
> +
> + return ret;
> +}
> +
> static int exynos_bus_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -517,6 +654,14 @@ static int exynos_bus_probe(struct platform_device *pdev)
> goto err;
> }
>
> + /*
> + * Initialize interconnect provider. A return value of -ENOTSUPP means
> + * that CONFIG_INTERCONNECT is disabled.
> + */
> + ret = exynos_bus_icc_init(bus);
> + if (ret < 0 && ret != -ENOTSUPP)
> + goto err;
> +
> max_state = bus->devfreq->profile->max_state;
> min_freq = (bus->devfreq->profile->freq_table[0] / 1000);
> max_freq = (bus->devfreq->profile->freq_table[max_state - 1] / 1000);
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply
* Re: [PATCH 6/8] PM / OPP: Support adjusting OPP voltages at runtime
From: Roger Lu @ 2019-07-29 3:39 UTC (permalink / raw)
To: Viresh Kumar, Stephen Boyd
Cc: Mark Rutland, Nishanth Menon,
Andrew-sh Cheng (鄭式勳), srv_heupstream,
linux-pm@vger.kernel.org, Stephen Boyd, Rafael J. Wysocki,
linux-kernel@vger.kernel.org, Rob Herring, yt.lee, Chanwoo Choi,
Kyungmin Park, MyungJoo Ham, linux-mediatek@lists.infradead.org,
linux-arm-kernel@lists.infradead.org, Matthias Brugger,
Fan Chen (陳凡)
In-Reply-To: <20190520044704.unftq6q5vy73z5bo@vireshk-i7>
Dear Stephen Boyd,
This patch is derived from [1]. Please kindly shares the suggestion to
us. Thanks very much.
[1]: https://lore.kernel.org/patchwork/patch/599279/
Dear Viresh,
I followed _opp_set_availability() coding style to refine
dev_pm_opp_adjust_voltage() from this patch. Is this refinement suitable
for OPP core? Thanks a lot.
On Mon, 2019-05-20 at 12:47 +0800, Viresh Kumar wrote:
> On 16-05-19, 17:08, Andrew-sh.Cheng wrote:
> > From: Stephen Boyd <sboyd@codeaurora.org>
> >
> > On some SoCs the Adaptive Voltage Scaling (AVS) technique is
> > employed to optimize the operating voltage of a device. At a
> > given frequency, the hardware monitors dynamic factors and either
> > makes a suggestion for how much to adjust a voltage for the
> > current frequency, or it automatically adjusts the voltage
> > without software intervention. Add an API to the OPP library for
> > the former case, so that AVS type devices can update the voltages
> > for an OPP when the hardware determines the voltage should
> > change. The assumption is that drivers like CPUfreq or devfreq
> > will register for the OPP notifiers and adjust the voltage
> > according to suggestions that AVS makes.
> >
> > This patch is devired from [1] submitted by Stephen.
> > [1] https://lore.kernel.org/patchwork/patch/599279/
> >
> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> > Signed-off-by: Roger Lu <roger.lu@mediatek.com>
> > ---
> > drivers/opp/core.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/pm_opp.h | 11 +++++++
> > 2 files changed, 89 insertions(+)
>
> This is an rcu implementation which got removed long back from OPP core. Please
> align this with the latest changes.
>
/**
* dev_pm_opp_adjust_voltage() - helper to change the voltage of an OPP
* @dev: device for which we do this operation
* @freq: OPP frequency to adjust voltage of
* @u_volt: new OPP voltage
*
* Return: -EINVAL for bad pointers, -ENOMEM if no memory available for
the
* copy operation, returns 0 if no modifcation was done OR modification
was
* successful.
*/
int dev_pm_opp_adjust_voltage(struct device *dev, unsigned long freq,
unsigned long u_volt)
{
struct opp_table *opp_table;
struct dev_pm_opp *tmp_opp, *opp = ERR_PTR(-ENODEV);
int r = 0;
/* Find the opp_table */
opp_table = _find_opp_table(dev);
if (IS_ERR(opp_table)) {
r = PTR_ERR(opp_table);
dev_warn(dev, "%s: Device OPP not found (%d)\n", __func__, r);
return r;
}
mutex_lock(&opp_table->lock);
/* Do we have the frequency? */
list_for_each_entry(tmp_opp, &opp_table->opp_list, node) {
if (tmp_opp->rate == freq) {
opp = tmp_opp;
break;
}
}
if (IS_ERR(opp)) {
r = PTR_ERR(opp);
goto adjust_unlock;
}
/* Is update really needed? */
if (opp->supplies->u_volt == u_volt)
goto adjust_unlock;
opp->supplies->u_volt = u_volt;
dev_pm_opp_get(opp);
mutex_unlock(&opp_table->lock);
/* Notify the voltage change of the OPP */
blocking_notifier_call_chain(&opp_table->head,
OPP_EVENT_ADJUST_VOLTAGE,
opp);
dev_pm_opp_put(opp);
goto adjust_put_table;
adjust_unlock:
mutex_unlock(&opp_table->lock);
adjust_put_table:
dev_pm_opp_put_opp_table(opp_table);
return r;
}
Sincerely,
Roger Lu.
^ permalink raw reply
* [PATCH 0/4] net: phy: Add AST2600 MDIO support
From: Andrew Jeffery @ 2019-07-29 4:39 UTC (permalink / raw)
To: netdev
Cc: Andrew Jeffery, davem, robh+dt, mark.rutland, joel, andrew,
f.fainelli, hkallweit1, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel
Hello,
This series adds support for the MDIO controllers found in the AST2600. In the
AST2500 and earlier the MDIO controller was embedded in the MAC; this has now
been separated out and the register interface rearranged (again).
Please review!
Andrew
Andrew Jeffery (4):
dt-bindings: net: Add aspeed,ast2600-mdio binding
net: phy: Add mdio-aspeed
net: ftgmac100: Add support for DT phy-handle property
net: ftgmac100: Select ASPEED MDIO driver for the AST2600
.../bindings/net/aspeed,ast2600-mdio.yaml | 61 +++++++
drivers/net/ethernet/faraday/Kconfig | 1 +
drivers/net/ethernet/faraday/ftgmac100.c | 37 +++-
drivers/net/phy/Kconfig | 13 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/mdio-aspeed.c | 159 ++++++++++++++++++
6 files changed, 268 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
create mode 100644 drivers/net/phy/mdio-aspeed.c
--
2.20.1
^ permalink raw reply
* [PATCH 1/4] dt-bindings: net: Add aspeed,ast2600-mdio binding
From: Andrew Jeffery @ 2019-07-29 4:39 UTC (permalink / raw)
To: netdev
Cc: Andrew Jeffery, davem, robh+dt, mark.rutland, joel, andrew,
f.fainelli, hkallweit1, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel
In-Reply-To: <20190729043926.32679-1-andrew@aj.id.au>
The AST2600 splits out the MDIO bus controller from the MAC into its own
IP block and rearranges the register layout. Add a new binding to
describe the new hardware.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
.../bindings/net/aspeed,ast2600-mdio.yaml | 61 +++++++++++++++++++
1 file changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
diff --git a/Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml b/Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
new file mode 100644
index 000000000000..fa86f6438473
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/aspeed,ast2600-mdio.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/aspeed,ast2600-mdio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ASPEED AST2600 MDIO Controller
+
+maintainers:
+ - Andrew Jeffery <andrew@aj.id.au>
+
+description: |+
+ The ASPEED AST2600 MDIO controller is the third iteration of ASPEED's MDIO
+ bus register interface, this time also separating out the controller from the
+ MAC.
+
+properties:
+ compatible:
+ const: aspeed,ast2600-mdio
+ reg:
+ maxItems: 1
+ description: The register range of the MDIO controller instance
+ "#address-cells":
+ const: 1
+ "#size-cells":
+ const: 0
+
+patternProperties:
+ "^ethernet-phy@[a-f0-9]$":
+ properties:
+ reg:
+ description:
+ The MDIO bus index of the PHY
+ compatible:
+ enum:
+ - ethernet-phy-ieee802.3-c22
+ - ethernet-phy-ieee802.3-c45
+ required:
+ - reg
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+
+examples:
+ - |
+ mdio0: mdio@1e650000 {
+ compatible = "aspeed,ast2600-mdio";
+ reg = <0x1e650000 0x8>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ status = "okay";
+
+ ethphy0: ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <0>;
+ };
+ };
--
2.20.1
^ permalink raw reply related
* [PATCH 2/4] net: phy: Add mdio-aspeed
From: Andrew Jeffery @ 2019-07-29 4:39 UTC (permalink / raw)
To: netdev
Cc: Andrew Jeffery, davem, robh+dt, mark.rutland, joel, andrew,
f.fainelli, hkallweit1, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel
In-Reply-To: <20190729043926.32679-1-andrew@aj.id.au>
The AST2600 design separates the MDIO controllers from the MAC, which is
where they were placed in the AST2400 and AST2500. Further, the register
interface is reworked again, so now we have three possible different
interface implementations, however this driver only supports the
interface provided by the AST2600. The AST2400 and AST2500 will continue
to be supported by the MDIO support embedded in the FTGMAC100 driver.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/net/phy/Kconfig | 13 +++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/mdio-aspeed.c | 159 ++++++++++++++++++++++++++++++++++
3 files changed, 173 insertions(+)
create mode 100644 drivers/net/phy/mdio-aspeed.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 20f14c5fbb7e..206d8650ee7f 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -21,6 +21,19 @@ config MDIO_BUS
if MDIO_BUS
+config MDIO_ASPEED
+ tristate "ASPEED MDIO bus controller"
+ depends on ARCH_ASPEED || COMPILE_TEST
+ depends on OF_MDIO && HAS_IOMEM
+ help
+ This module provides a driver for the independent MDIO bus
+ controllers found in the ASPEED AST2600 SoC. This is a driver for the
+ third revision of the ASPEED MDIO register interface - the first two
+ revisions are the "old" and "new" interfaces found in the AST2400 and
+ AST2500, embedded in the MAC. For legacy reasons, FTGMAC100 driver
+ continues to drive the embedded MDIO controller for the AST2400 and
+ AST2500 SoCs, so say N if AST2600 support is not required.
+
config MDIO_BCM_IPROC
tristate "Broadcom iProc MDIO bus controller"
depends on ARCH_BCM_IPROC || COMPILE_TEST
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 839acb292c38..ba07c27e4208 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -22,6 +22,7 @@ libphy-$(CONFIG_LED_TRIGGER_PHY) += phy_led_triggers.o
obj-$(CONFIG_PHYLINK) += phylink.o
obj-$(CONFIG_PHYLIB) += libphy.o
+obj-$(CONFIG_MDIO_ASPEED) += mdio-aspeed.o
obj-$(CONFIG_MDIO_BCM_IPROC) += mdio-bcm-iproc.o
obj-$(CONFIG_MDIO_BCM_UNIMAC) += mdio-bcm-unimac.o
obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o
diff --git a/drivers/net/phy/mdio-aspeed.c b/drivers/net/phy/mdio-aspeed.c
new file mode 100644
index 000000000000..71496a9ff54a
--- /dev/null
+++ b/drivers/net/phy/mdio-aspeed.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright (C) 2019 IBM Corp. */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/mdio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+#include <linux/platform_device.h>
+
+#define DRV_NAME "mdio-aspeed"
+
+#define ASPEED_MDIO_CTRL 0x0
+#define ASPEED_MDIO_CTRL_FIRE BIT(31)
+#define ASPEED_MDIO_CTRL_ST BIT(28)
+#define ASPEED_MDIO_CTRL_ST_C45 0
+#define ASPEED_MDIO_CTRL_ST_C22 1
+#define ASPEED_MDIO_CTRL_OP GENMASK(27, 26)
+#define MDIO_C22_OP_WRITE 0b01
+#define MDIO_C22_OP_READ 0b10
+#define ASPEED_MDIO_CTRL_PHYAD GENMASK(25, 21)
+#define ASPEED_MDIO_CTRL_REGAD GENMASK(20, 16)
+#define ASPEED_MDIO_CTRL_MIIWDATA GENMASK(15, 0)
+
+#define ASPEED_MDIO_DATA 0x4
+#define ASPEED_MDIO_DATA_MDC_THRES GENMASK(31, 24)
+#define ASPEED_MDIO_DATA_MDIO_EDGE BIT(23)
+#define ASPEED_MDIO_DATA_MDIO_LATCH GENMASK(22, 20)
+#define ASPEED_MDIO_DATA_IDLE BIT(16)
+#define ASPEED_MDIO_DATA_MIIRDATA GENMASK(15, 0)
+
+#define ASPEED_MDIO_RETRIES 10
+
+struct aspeed_mdio {
+ void __iomem *base;
+};
+
+static int aspeed_mdio_read(struct mii_bus *bus, int addr, int regnum)
+{
+ struct aspeed_mdio *ctx = bus->priv;
+ u32 ctrl;
+ int i;
+
+ dev_dbg(&bus->dev, "%s: addr: %d, regnum: %d\n", __func__, addr,
+ regnum);
+
+ /* Just clause 22 for the moment */
+ ctrl = ASPEED_MDIO_CTRL_FIRE
+ | FIELD_PREP(ASPEED_MDIO_CTRL_ST, ASPEED_MDIO_CTRL_ST_C22)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_OP, MDIO_C22_OP_READ)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_PHYAD, addr)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_REGAD, regnum);
+
+ iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL);
+
+ for (i = 0; i < ASPEED_MDIO_RETRIES; i++) {
+ u32 data;
+
+ data = ioread32(ctx->base + ASPEED_MDIO_DATA);
+ if (data & ASPEED_MDIO_DATA_IDLE)
+ return FIELD_GET(ASPEED_MDIO_DATA_MIIRDATA, data);
+
+ udelay(100);
+ }
+
+ dev_err(&bus->dev, "MDIO read failed\n");
+ return -EIO;
+}
+
+static int aspeed_mdio_write(struct mii_bus *bus, int addr, int regnum, u16 val)
+{
+ struct aspeed_mdio *ctx = bus->priv;
+ u32 ctrl;
+ int i;
+
+ dev_dbg(&bus->dev, "%s: addr: %d, regnum: %d, val: 0x%x\n",
+ __func__, addr, regnum, val);
+
+ /* Just clause 22 for the moment */
+ ctrl = ASPEED_MDIO_CTRL_FIRE
+ | FIELD_PREP(ASPEED_MDIO_CTRL_ST, ASPEED_MDIO_CTRL_ST_C22)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_OP, MDIO_C22_OP_WRITE)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_PHYAD, addr)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_REGAD, regnum)
+ | FIELD_PREP(ASPEED_MDIO_CTRL_MIIWDATA, val);
+
+ iowrite32(ctrl, ctx->base + ASPEED_MDIO_CTRL);
+
+ for (i = 0; i < ASPEED_MDIO_RETRIES; i++) {
+ ctrl = ioread32(ctx->base + ASPEED_MDIO_CTRL);
+ if (!(ctrl & ASPEED_MDIO_CTRL_FIRE))
+ return 0;
+
+ udelay(100);
+ }
+
+ dev_err(&bus->dev, "MDIO write failed\n");
+ return -EIO;
+}
+
+static int aspeed_mdio_probe(struct platform_device *pdev)
+{
+ struct aspeed_mdio *ctx;
+ struct mii_bus *bus;
+ int rc;
+
+ bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*ctx));
+ if (!bus)
+ return -ENOMEM;
+
+ ctx = bus->priv;
+ ctx->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(ctx->base))
+ return PTR_ERR(ctx->base);
+
+ bus->name = DRV_NAME;
+ snprintf(bus->id, MII_BUS_ID_SIZE, "%s%d", pdev->name, pdev->id);
+ bus->parent = &pdev->dev;
+ bus->read = aspeed_mdio_read;
+ bus->write = aspeed_mdio_write;
+
+ rc = of_mdiobus_register(bus, pdev->dev.of_node);
+ if (rc) {
+ dev_err(&pdev->dev, "Cannot register MDIO bus!\n");
+ return rc;
+ }
+
+ platform_set_drvdata(pdev, bus);
+
+ return 0;
+}
+
+static int aspeed_mdio_remove(struct platform_device *pdev)
+{
+ mdiobus_unregister(platform_get_drvdata(pdev));
+
+ return 0;
+}
+
+static const struct of_device_id aspeed_mdio_of_match[] = {
+ { .compatible = "aspeed,ast2600-mdio", },
+ { },
+};
+
+static struct platform_driver aspeed_mdio_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .of_match_table = aspeed_mdio_of_match,
+ },
+ .probe = aspeed_mdio_probe,
+ .remove = aspeed_mdio_remove,
+};
+
+module_platform_driver(aspeed_mdio_driver);
+
+MODULE_AUTHOR("Andrew Jeffery <andrew@aj.id.au>");
+MODULE_LICENSE("GPL");
--
2.20.1
^ permalink raw reply related
* [PATCH 3/4] net: ftgmac100: Add support for DT phy-handle property
From: Andrew Jeffery @ 2019-07-29 4:39 UTC (permalink / raw)
To: netdev
Cc: Andrew Jeffery, davem, robh+dt, mark.rutland, joel, andrew,
f.fainelli, hkallweit1, devicetree, linux-arm-kernel,
linux-aspeed, linux-kernel
In-Reply-To: <20190729043926.32679-1-andrew@aj.id.au>
phy-handle is necessary for the AST2600 which separates the MDIO
controllers from the MAC.
I've tried to minimise the intrusion of supporting the AST2600 to the
FTGMAC100 by leaving in place the existing MDIO support for the embedded
MDIO interface. The AST2400 and AST2500 continue to be supported this
way, as it avoids breaking/reworking existing devicetrees.
The AST2600 support by contrast requires the presence of the phy-handle
property in the MAC devicetree node to specify the appropriate PHY to
associate with the MAC. In the event that someone wants to specify the
MDIO bus topology under the MAC node on an AST2400 or AST2500, the
current auto-probe approach is done conditional on the absence of an
"mdio" child node of the MAC.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/net/ethernet/faraday/ftgmac100.c | 37 +++++++++++++++++++++---
1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 030fed65393e..563384b788ab 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -17,6 +17,7 @@
#include <linux/module.h>
#include <linux/netdevice.h>
#include <linux/of.h>
+#include <linux/of_mdio.h>
#include <linux/phy.h>
#include <linux/platform_device.h>
#include <linux/property.h>
@@ -1619,8 +1620,13 @@ static int ftgmac100_setup_mdio(struct net_device *netdev)
if (!priv->mii_bus)
return -EIO;
- if (priv->is_aspeed) {
- /* This driver supports the old MDIO interface */
+ if (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
+ of_device_is_compatible(np, "aspeed,ast2500-mac")) {
+ /* The AST2600 has a separate MDIO controller */
+
+ /* For the AST2400 and AST2500 this driver only supports the
+ * old MDIO interface
+ */
reg = ioread32(priv->base + FTGMAC100_OFFSET_REVR);
reg &= ~FTGMAC100_REVR_NEW_MDIO_INTERFACE;
iowrite32(reg, priv->base + FTGMAC100_OFFSET_REVR);
@@ -1797,7 +1803,8 @@ static int ftgmac100_probe(struct platform_device *pdev)
np = pdev->dev.of_node;
if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
- of_device_is_compatible(np, "aspeed,ast2500-mac"))) {
+ of_device_is_compatible(np, "aspeed,ast2500-mac") ||
+ of_device_is_compatible(np, "aspeed,ast2600-mac"))) {
priv->rxdes0_edorr_mask = BIT(30);
priv->txdes0_edotr_mask = BIT(30);
priv->is_aspeed = true;
@@ -1817,7 +1824,29 @@ static int ftgmac100_probe(struct platform_device *pdev)
priv->ndev = ncsi_register_dev(netdev, ftgmac100_ncsi_handler);
if (!priv->ndev)
goto err_ncsi_dev;
- } else {
+ } else if (np && of_get_property(np, "phy-handle", NULL)) {
+ struct phy_device *phy;
+
+ phy = of_phy_get_and_connect(priv->netdev, np,
+ &ftgmac100_adjust_link);
+ if (!phy) {
+ dev_err(&pdev->dev, "Failed to connect to phy\n");
+ goto err_setup_mdio;
+ }
+
+ /* Indicate that we support PAUSE frames (see comment in
+ * Documentation/networking/phy.txt)
+ */
+ phy_support_asym_pause(phy);
+
+ /* Display what we found */
+ phy_attached_info(phy);
+ } else if (np && !of_get_child_by_name(np, "mdio")) {
+ /* Support legacy ASPEED devicetree descriptions that decribe a
+ * MAC with an embedded MDIO controller but have no "mdio"
+ * child node. Automatically scan the MDIO bus for available
+ * PHYs.
+ */
priv->use_ncsi = false;
err = ftgmac100_setup_mdio(netdev);
if (err)
--
2.20.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox