Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v11 2/8] power: add power sequence library
From: Krzysztof Kozlowski @ 2017-01-07  8:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483596119-27508-3-git-send-email-peter.chen@nxp.com>

On Thu, Jan 05, 2017 at 02:01:53PM +0800, Peter Chen wrote:
> We have an well-known problem that the device needs to do some power
> sequence before it can be recognized by related host, the typical
> example like hard-wired mmc devices and usb devices.
> 
> This power sequence is hard to be described at device tree and handled by
> related host driver, so we have created a common power sequence
> library to cover this requirement. The core code has supplied
> some common helpers for host driver, and individual power sequence
> libraries handle kinds of power sequence for devices. The pwrseq
> librares always need to allocate extra instance for compatible
> string match.
> 
> pwrseq_generic is intended for general purpose of power sequence, which
> handles gpios and clocks currently, and can cover other controls in
> future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> if only one power sequence is needed, else call of_pwrseq_on_list
> /of_pwrseq_off_list instead (eg, USB hub driver).
> 
> For new power sequence library, it can add its compatible string
> to pwrseq_of_match_table, then the pwrseq core will match it with
> DT's, and choose this library at runtime.
> 
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> Tested-by Joshua Clayton <stillcompiling@gmail.com>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  MAINTAINERS                           |   9 +
>  drivers/power/Kconfig                 |   1 +
>  drivers/power/Makefile                |   1 +
>  drivers/power/pwrseq/Kconfig          |  20 ++
>  drivers/power/pwrseq/Makefile         |   2 +
>  drivers/power/pwrseq/core.c           | 335 ++++++++++++++++++++++++++++++++++
>  drivers/power/pwrseq/pwrseq_generic.c | 224 +++++++++++++++++++++++
>  include/linux/power/pwrseq.h          |  81 ++++++++
>  8 files changed, 673 insertions(+)
>  create mode 100644 drivers/power/pwrseq/Kconfig
>  create mode 100644 drivers/power/pwrseq/Makefile
>  create mode 100644 drivers/power/pwrseq/core.c
>  create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
>  create mode 100644 include/linux/power/pwrseq.h
> 

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
Tested on Odroid U3 (reset sequence for LAN9730):
Tested-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

^ permalink raw reply

* [PATCH v11 1/8] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library
From: Krzysztof Kozlowski @ 2017-01-07  8:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483596119-27508-2-git-send-email-peter.chen@nxp.com>

On Thu, Jan 05, 2017 at 02:01:52PM +0800, Peter Chen wrote:
> Add binding doc for generic power sequence library.
> 
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/power/pwrseq/pwrseq-generic.txt       | 48 ++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

^ permalink raw reply

* [PATCH 4/4] ARM: multi_v7_defconfig: Enable power sequence for Odroid U3
From: Krzysztof Kozlowski @ 2017-01-07  8:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170107085203.4431-1-krzk@kernel.org>

Odroid U3 needs a power sequence for lan9730, if it was enabled by
bootloader.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index b01a43851294..1750d99862b9 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -443,6 +443,7 @@ CONFIG_POWER_RESET_RMOBILE=y
 CONFIG_POWER_RESET_ST=y
 CONFIG_POWER_AVS=y
 CONFIG_ROCKCHIP_IODOMAIN=y
+CONFIG_PWRSEQ_GENERIC=y
 CONFIG_SENSORS_IIO_HWMON=y
 CONFIG_SENSORS_LM90=y
 CONFIG_SENSORS_LM95245=y
-- 
2.9.3

^ permalink raw reply related

* [PATCH 3/4] ARM: exynos_defconfig: Enable power sequence for Odroid U3
From: Krzysztof Kozlowski @ 2017-01-07  8:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170107085203.4431-1-krzk@kernel.org>

Odroid U3 needs a power sequence for lan9730, if it was enabled by
bootloader.  Enable also GPIO_SYSFS which is useful for playing with
GPIO during debug process.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm/configs/exynos_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
index 79c415c33f69..ad1a509c296a 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -99,6 +99,7 @@ CONFIG_SPI=y
 CONFIG_SPI_GPIO=y
 CONFIG_SPI_S3C64XX=y
 CONFIG_DEBUG_GPIO=y
+CONFIG_GPIO_SYSFS=y
 CONFIG_GPIO_WM8994=y
 CONFIG_POWER_SUPPLY=y
 CONFIG_BATTERY_SBS=y
@@ -108,6 +109,7 @@ CONFIG_CHARGER_MAX14577=y
 CONFIG_CHARGER_MAX77693=y
 CONFIG_CHARGER_MAX8997=y
 CONFIG_CHARGER_TPS65090=y
+CONFIG_PWRSEQ_GENERIC=y
 CONFIG_SENSORS_LM90=y
 CONFIG_SENSORS_NTC_THERMISTOR=y
 CONFIG_SENSORS_PWM_FAN=y
-- 
2.9.3

^ permalink raw reply related

* [PATCH 2/4] ARM: dts: exynos: Fix LAN9730 on Odroid U3 after tftpboot
From: Krzysztof Kozlowski @ 2017-01-07  8:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170107085203.4431-1-krzk@kernel.org>

The ethernet adapter LAN9730, after enabling in bootloader (e.g. for
tftpboot) requires reset during boot.  Otherwise it won't come up.

The schematics of Odroid U3 are detailed enough but after grabbing
knowledge also from other sources (like U-Boot) the overall design looks
like:
1. LAN9730 is connected to HSIC0 and USB3503 to HSIC1 of EHCI controller.
2. USB3503 comes with its own reset pin: gpx3-5.
3. Reset pin of LAN9730 is pulled up to 3.3 V so it cannot be used.
4. The supply of 3.3 V for LAN9730 is delivered from buck8.
5. Buck8 state is a logical OR of registry value (through I2C command)
   and ENB8 pin.  The ENB8, not described in schematics, is in fact
   gpa1-1.
6. Missing or wrongly timed reset of LAN9730 might result in missing of
   two devices: LAN9730 and USB3503. Without reset, LAN9730 will not
   come up, if it was enabled by bootloader.

To fix the issue use the generic power sequence driver and toggle the
ENB8 (buck8) pin.  Reset duration of 500 us was chosen by experiments
(shortest working time was 400 us).  This is an easiest way to fix the
long standing LAN9730 reset issue.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm/boot/dts/exynos4412-odroidu3.dts | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts
index 99634c54dca9..aef49007cba0 100644
--- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
@@ -84,10 +84,23 @@
 	regulator-max-microvolt = <2800000>;
 };
 
+&max77686 {
+	pinctrl-0 = <&max77686_irq &max77686_enb8>;
+};
+
 &mshc_0 {
 	vqmmc-supply = <&ldo22_reg>;
 };
 
+&pinctrl_0 {
+	max77686_enb8: max77686-enb8 {
+		samsung,pins = "gpa1-1";
+		samsung,pin-function = <EXYNOS_PIN_FUNC_OUTPUT>;
+		samsung,pin-pud = <EXYNOS_PIN_PULL_DOWN>;
+		samsung,pin-drv = <EXYNOS4_PIN_DRV_LV1>;
+	};
+};
+
 &pwm {
 	pinctrl-0 = <&pwm0_out>;
 	pinctrl-names = "default";
@@ -103,7 +116,15 @@
 
 &ehci {
 	port at 1 {
+		/* HSIC for LAN9730 */
 		status = "okay";
+		/* buck8 enable pin, use it for power sequence */
+		reset-gpios = <&gpa1 1 GPIO_ACTIVE_LOW>;
+		/*
+		 * Reset duration of 500 us was chosen experimentally.
+		 * Minimal working value was 400 us. Add some safe margin.
+		 */
+		reset-duration-us = <500>;
 	};
 	port at 2 {
 		status = "okay";
-- 
2.9.3

^ permalink raw reply related

* [PATCH 1/4] ARM: dts: exynos: Fix indentation of EHCI and OHCI ports
From: Krzysztof Kozlowski @ 2017-01-07  8:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170107085203.4431-1-krzk@kernel.org>

Replace spaces with tabs in EHCI and OHCI ports indentation.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm/boot/dts/exynos4.dtsi | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index c64737baa45e..3209c60389e2 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -370,19 +370,19 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 		port at 0 {
-		    reg = <0>;
-		    phys = <&exynos_usbphy 1>;
-		    status = "disabled";
+			reg = <0>;
+			phys = <&exynos_usbphy 1>;
+			status = "disabled";
 		};
 		port at 1 {
-		    reg = <1>;
-		    phys = <&exynos_usbphy 2>;
-		    status = "disabled";
+			reg = <1>;
+			phys = <&exynos_usbphy 2>;
+			status = "disabled";
 		};
 		port at 2 {
-		    reg = <2>;
-		    phys = <&exynos_usbphy 3>;
-		    status = "disabled";
+			reg = <2>;
+			phys = <&exynos_usbphy 3>;
+			status = "disabled";
 		};
 	};
 
@@ -396,9 +396,9 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 		port at 0 {
-		    reg = <0>;
-		    phys = <&exynos_usbphy 1>;
-		    status = "disabled";
+			reg = <0>;
+			phys = <&exynos_usbphy 1>;
+			status = "disabled";
 		};
 	};
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH 0/4] ARM: exynos: Fix Odroid U3 USB/LAN when TFTP booting (power sequence)
From: Krzysztof Kozlowski @ 2017-01-07  8:51 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

Thanks to Markus Reichl, I got an Odroid U3 to work with. Thanks to Peter
Chen, we got a power sequence generic library which solves my long
standing Odroid U3 problem - no LAN9730 if it was enabled by bootloader.

My previous attempts for this can be found here [0].

This patchset is based on Peter's v11 of power sequence [1].
Patchset is available also on my Github [2].

More detailed analysis is described in patch 2/4 ("ARM: dts: exynos: Fix
LAN9730 on Odroid U3 after tftpboot").


Best regards,
Krzysztof


[0] http://www.spinics.net/lists/linux-mmc/msg37386.html
[1] https://lwn.net/Articles/710736/
[2] https://github.com/krzk/linux/commits/for-next/odroid-u3-usb3503-pwrseq

Krzysztof Kozlowski (4):
  ARM: dts: exynos: Fix indentation of EHCI and OHCI ports
  ARM: dts: exynos: Fix LAN9730 on Odroid U3 after tftpboot
  ARM: exynos_defconfig: Enable power sequence for Odroid U3
  ARM: multi_v7_defconfig: Enable power sequence for Odroid U3

 arch/arm/boot/dts/exynos4.dtsi            | 24 ++++++++++++------------
 arch/arm/boot/dts/exynos4412-odroidu3.dts | 21 +++++++++++++++++++++
 arch/arm/configs/exynos_defconfig         |  2 ++
 arch/arm/configs/multi_v7_defconfig       |  1 +
 4 files changed, 36 insertions(+), 12 deletions(-)

-- 
2.9.3

^ permalink raw reply

* [PATCH 2/2] usb: host: ohci-exynos: Decrese node refcount on exynos_ehci_get_phy() error paths
From: Krzysztof Kozlowski @ 2017-01-07  8:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170107084141.3731-1-krzk@kernel.org>

Returning from for_each_available_child_of_node() loop requires cleaning
up node refcount.  Error paths lacked it so for example in case of
deferred probe, the refcount of phy node was left increased.

Fixes: 6d40500ac9b6 ("usb: ehci/ohci-exynos: Fix of_node_put() for child when getting PHYs")
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/usb/host/ohci-exynos.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 2cd105be7319..6865b919403f 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -66,10 +66,12 @@ static int exynos_ohci_get_phy(struct device *dev,
 		if (IS_ERR(phy)) {
 			ret = PTR_ERR(phy);
 			if (ret == -EPROBE_DEFER) {
+				of_node_put(child);
 				return ret;
 			} else if (ret != -ENOSYS && ret != -ENODEV) {
 				dev_err(dev,
 					"Error retrieving usb2 phy: %d\n", ret);
+				of_node_put(child);
 				return ret;
 			}
 		}
-- 
2.9.3

^ permalink raw reply related

* [PATCH 1/2] usb: host: ehci-exynos: Decrese node refcount on exynos_ehci_get_phy() error paths
From: Krzysztof Kozlowski @ 2017-01-07  8:41 UTC (permalink / raw)
  To: linux-arm-kernel

Returning from for_each_available_child_of_node() loop requires cleaning
up node refcount.  Error paths lacked it so for example in case of
deferred probe, the refcount of phy node was left increased.

Fixes: 6d40500ac9b6 ("usb: ehci/ohci-exynos: Fix of_node_put() for child when getting PHYs")
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/usb/host/ehci-exynos.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 42e5b66353ef..7a603f66a9bc 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -77,10 +77,12 @@ static int exynos_ehci_get_phy(struct device *dev,
 		if (IS_ERR(phy)) {
 			ret = PTR_ERR(phy);
 			if (ret == -EPROBE_DEFER) {
+				of_node_put(child);
 				return ret;
 			} else if (ret != -ENOSYS && ret != -ENODEV) {
 				dev_err(dev,
 					"Error retrieving usb2 phy: %d\n", ret);
+				of_node_put(child);
 				return ret;
 			}
 		}
-- 
2.9.3

^ permalink raw reply related

* [PATCH 1/5] ARM: dts: qcom: apq8064: Add missing scm clock
From: Bjorn Andersson @ 2017-01-07  7:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20170107030120.GC5710@hector.attlocal.net>

On Fri 06 Jan 19:01 PST 2017, Andy Gross wrote:

> On Fri, Jan 06, 2017 at 05:10:44PM -0800, John Stultz wrote:
> > On Wed, Dec 21, 2016 at 3:49 AM, Bjorn Andersson
> > <bjorn.andersson@linaro.org> wrote:
> > > As per the device tree binding the apq8064 scm node requires the core
> > > clock to be specified, so add this.
> > >
> > > Cc: John Stultz <john.stultz@linaro.org>
> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > ---
> > >  arch/arm/boot/dts/qcom-apq8064.dtsi | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> > > index 268bd470c865..78bf155a52f3 100644
> > > --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> > > +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> > > @@ -303,6 +303,9 @@
> > >         firmware {
> > >                 scm {
> > >                         compatible = "qcom,scm-apq8064";
> > > +
> > > +                       clocks = <&gcc CE3_CORE_CLK>;
> > > +                       clock-names = "core";
> > 
> > 
> > Tested-by: John Stultz <john.stultz@linaro.org>
> > 
> > I know Bjorn has a new version of this patch that uses the
> > RPM_DAYTONA_FABRIC_CLK value, but that one results in problems with
> > usb gadget functionality on my Nexus7.  This one seems to work ok
> > though.
> 
> Odd.  Is the usb gadget using the daytona but not getting a reference?  I wonder
> if this is related to not having the bus driver running the bus clk enablement
> and frequencies.
> 

The fact that we now reference the Daytona clock means that we're also
telling the RPM to disable it, so that might very well be the case.

Unfortunately I can't find any block diagram for 8064 to show what hangs
off the Daytona, so I'm not sure in what way USB should reference it.

Regards,
Bjorn

^ permalink raw reply

* [PATCH v7 4/4] arm64: arch timer: Add timer erratum property for Hip05-d02 and Hip06-d03
From: Ding Tianhong @ 2017-01-07  7:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483772858-10380-1-git-send-email-dingtianhong@huawei.com>

Enable workaround for hisilicon erratum 161601 on Hip05-d02 and Hip06-d03 board.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 arch/arm64/boot/dts/hisilicon/hip05.dtsi | 1 +
 arch/arm64/boot/dts/hisilicon/hip06.dtsi | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hip05.dtsi b/arch/arm64/boot/dts/hisilicon/hip05.dtsi
index 4b472a3..a8e9969 100644
--- a/arch/arm64/boot/dts/hisilicon/hip05.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hip05.dtsi
@@ -281,6 +281,7 @@
 			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
 			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
 			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+		hisilicon,erratum-161601;
 	};
 
 	pmu {
diff --git a/arch/arm64/boot/dts/hisilicon/hip06.dtsi b/arch/arm64/boot/dts/hisilicon/hip06.dtsi
index a049b64..344e0f0 100644
--- a/arch/arm64/boot/dts/hisilicon/hip06.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hip06.dtsi
@@ -260,6 +260,7 @@
 			     <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
 			     <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
 			     <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+		hisilicon,erratum-161601;
 	};
 
 	pmu {
-- 
1.9.0

^ permalink raw reply related

* [PATCH v7 3/4] arm64: arch_timer: Work around Erratum Hisilicon-161601
From: Ding Tianhong @ 2017-01-07  7:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483772858-10380-1-git-send-email-dingtianhong@huawei.com>

Erratum Hisilicon-161601 says that the ARM generic timer counter "has the
potential to contain an erroneous value when the timer value changes".
Accesses to TVAL (both read and write) are also affected due to the implicit counter
read.  Accesses to CVAL are not affected.

The workaround is to reread the system count registers until the value of the second
read is larger than the first one by less than 32, the system counter can be guaranteed
not to return wrong value twice by back-to-back read and the error value is always larger
than the correct one by 32. Writes to TVAL are replaced with an equivalent write to CVAL.

The workaround is enabled if the hisilicon,erratum-161601 property is found in
the timer node in the device tree. This can be overridden with the
clocksource.arm_arch_timer.hisilicon-161601 boot parameter, which allows KVM
users to enable the workaround until a mechanism is implemented to
automatically communicate this information.

Fix some description for fsl erratum a008585.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 Documentation/arm64/silicon-errata.txt |  1 +
 drivers/clocksource/Kconfig            | 12 ++++++++-
 drivers/clocksource/arm_arch_timer.c   | 49 ++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 405da11..1c1a95f 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -63,3 +63,4 @@ stable kernels.
 | Cavium         | ThunderX SMMUv2 | #27704          | N/A		       |
 |                |                 |                 |                         |
 | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585     |
+| Hisilicon      | Hip0{5,6,7}     | #161601         | HISILICON_ERRATUM_161601|
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 97f95f8..c0eabed 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -327,7 +327,7 @@ config ARM_ARCH_TIMER_EVTSTREAM
 
 config ARM_ARCH_TIMER_OOL_WORKAROUND
 	bool "Workaround for arm arch timer unstable counter"
-	depends on FSL_ERRATUM_A008585
+	depends on FSL_ERRATUM_A008585 || HISILICON_ERRATUM_161601
 	help
 	  This option would only be enabled by Freescale/NXP Erratum A-008585
 	  or something else chip has similar erratum.
@@ -343,6 +343,16 @@ config FSL_ERRATUM_A008585
 	  value").  The workaround will only be active if the
 	  fsl,erratum-a008585 property is found in the timer node.
 
+config HISILICON_ERRATUM_161601
+	bool "Workaround for Hisilicon Erratum 161601"
+	default y
+	select ARM_ARCH_TIMER_OOL_WORKAROUND
+	depends on ARM_ARCH_TIMER && ARM64
+	help
+	  This option enables a workaround for Hisilicon Erratum
+	  161601. The workaround will be active if the hisilicon,erratum-161601
+	  property is found in the timer node.
+
 config ARM_GLOBAL_TIMER
 	bool "Support for the ARM global timer" if COMPILE_TEST
 	select CLKSRC_OF if OF
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 2487c66..ef09e59f 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -131,6 +131,47 @@ static u64 notrace fsl_a008585_read_cntvct_el0(void)
 }
 #endif
 
+#ifdef CONFIG_HISILICON_ERRATUM_161601
+/*
+ * Verify whether the value of the second read is larger than the first by
+ * less than 32 is the only way to confirm the value is correct, so clear the
+ * lower 5 bits to check whether the difference is greater than 32 or not.
+ * Theoretically the erratum should not occur more than twice in succession
+ * when reading the system counter, but it is possible that some interrupts
+ * may lead to more than twice read errors, triggering the warning, so setting
+ * the number of retries far beyond the number of iterations the loop has been
+ * observed to take.
+ */
+#define __hisi_161601_read_reg(reg) ({				\
+	u64 _old, _new;						\
+	int _retries = 50;					\
+								\
+	do {							\
+		_old = read_sysreg(reg);			\
+		_new = read_sysreg(reg);			\
+		_retries--;					\
+	} while (unlikely((_new - _old) >> 5) && _retries);	\
+								\
+	WARN_ON_ONCE(!_retries);				\
+	_new;							\
+})
+
+static u32 notrace hisi_161601_read_cntp_tval_el0(void)
+{
+	return __hisi_161601_read_reg(cntp_tval_el0);
+}
+
+static u32 notrace hisi_161601_read_cntv_tval_el0(void)
+{
+	return __hisi_161601_read_reg(cntv_tval_el0);
+}
+
+static u64 notrace hisi_161601_read_cntvct_el0(void)
+{
+	return __hisi_161601_read_reg(cntvct_el0);
+}
+#endif
+
 #ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
 const struct arch_timer_erratum_workaround *timer_unstable_counter_workaround = NULL;
 EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
@@ -147,6 +188,14 @@ static u64 notrace fsl_a008585_read_cntvct_el0(void)
 		.read_cntvct_el0 = fsl_a008585_read_cntvct_el0,
 	},
 #endif
+#ifdef CONFIG_HISILICON_ERRATUM_161601
+	{
+		.id = "hisilicon,erratum-161601",
+		.read_cntp_tval_el0 = hisi_161601_read_cntp_tval_el0,
+		.read_cntv_tval_el0 = hisi_161601_read_cntv_tval_el0,
+		.read_cntvct_el0 = hisi_161601_read_cntvct_el0,
+	},
+#endif
 };
 #endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
-- 
1.9.0

^ permalink raw reply related

* [PATCH v7 2/4] arm64: arch_timer: Introduce a generic erratum handing mechanism for fsl-a008585
From: Ding Tianhong @ 2017-01-07  7:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483772858-10380-1-git-send-email-dingtianhong@huawei.com>

The workaround for hisilicon,161601 will check the return value of the system counter
by different way, in order to distinguish with the fsl-a008585 workaround, introduce
a new generic erratum handing mechanism for fsl-a008585 and rename some functions.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
 Documentation/admin-guide/kernel-parameters.txt |   9 --
 arch/arm64/include/asm/arch_timer.h             |  38 +++------
 drivers/clocksource/Kconfig                     |   8 ++
 drivers/clocksource/arm_arch_timer.c            | 105 ++++++++++++++----------
 4 files changed, 84 insertions(+), 76 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 21e2d88..76437ad 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -539,15 +539,6 @@
 			loops can be debugged more effectively on production
 			systems.
 
-	clocksource.arm_arch_timer.fsl-a008585=
-			[ARM64]
-			Format: <bool>
-			Enable/disable the workaround of Freescale/NXP
-			erratum A-008585.  This can be useful for KVM
-			guests, if the guest device tree doesn't show the
-			erratum.  If unspecified, the workaround is
-			enabled based on the device tree.
-
 	clearcpuid=BITNUM [X86]
 			Disable CPUID feature X for the kernel. See
 			arch/x86/include/asm/cpufeatures.h for the valid bit
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index eaa5bbe..b4b3400 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -29,41 +29,29 @@
 
 #include <clocksource/arm_arch_timer.h>
 
-#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585)
+#if IS_ENABLED(CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND)
 extern struct static_key_false arch_timer_read_ool_enabled;
-#define needs_fsl_a008585_workaround() \
+#define needs_unstable_timer_counter_workaround() \
 	static_branch_unlikely(&arch_timer_read_ool_enabled)
 #else
-#define needs_fsl_a008585_workaround()  false
+#define needs_unstable_timer_counter_workaround()  false
 #endif
 
-u32 __fsl_a008585_read_cntp_tval_el0(void);
-u32 __fsl_a008585_read_cntv_tval_el0(void);
-u64 __fsl_a008585_read_cntvct_el0(void);
 
-/*
- * The number of retries is an arbitrary value well beyond the highest number
- * of iterations the loop has been observed to take.
- */
-#define __fsl_a008585_read_reg(reg) ({			\
-	u64 _old, _new;					\
-	int _retries = 200;				\
-							\
-	do {						\
-		_old = read_sysreg(reg);		\
-		_new = read_sysreg(reg);		\
-		_retries--;				\
-	} while (unlikely(_old != _new) && _retries);	\
-							\
-	WARN_ON_ONCE(!_retries);			\
-	_new;						\
-})
+struct arch_timer_erratum_workaround {
+	const char *id;		/* Indicate the Erratum ID */
+	u32 (*read_cntp_tval_el0)(void);
+	u32 (*read_cntv_tval_el0)(void);
+	u64 (*read_cntvct_el0)(void);
+};
+
+extern const struct arch_timer_erratum_workaround *timer_unstable_counter_workaround;
 
 #define arch_timer_reg_read_stable(reg) 		\
 ({							\
 	u64 _val;					\
-	if (needs_fsl_a008585_workaround())		\
-		_val = __fsl_a008585_read_##reg();	\
+	if (needs_unstable_timer_counter_workaround())		\
+		_val = timer_unstable_counter_workaround->read_##reg();\
 	else						\
 		_val = read_sysreg(reg);		\
 	_val;						\
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 4866f7a..97f95f8 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -325,9 +325,17 @@ config ARM_ARCH_TIMER_EVTSTREAM
 	  This must be disabled for hardware validation purposes to detect any
 	  hardware anomalies of missing events.
 
+config ARM_ARCH_TIMER_OOL_WORKAROUND
+	bool "Workaround for arm arch timer unstable counter"
+	depends on FSL_ERRATUM_A008585
+	help
+	  This option would only be enabled by Freescale/NXP Erratum A-008585
+	  or something else chip has similar erratum.
+
 config FSL_ERRATUM_A008585
 	bool "Workaround for Freescale/NXP Erratum A-008585"
 	default y
+	select ARM_ARCH_TIMER_OOL_WORKAROUND
 	depends on ARM_ARCH_TIMER && ARM64
 	help
 	  This option enables a workaround for Freescale/NXP Erratum
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 02fef68..2487c66 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -96,41 +96,59 @@ static int __init early_evtstrm_cfg(char *buf)
  */
 
 #ifdef CONFIG_FSL_ERRATUM_A008585
-DEFINE_STATIC_KEY_FALSE(arch_timer_read_ool_enabled);
-EXPORT_SYMBOL_GPL(arch_timer_read_ool_enabled);
-
-static int fsl_a008585_enable = -1;
-
-static int __init early_fsl_a008585_cfg(char *buf)
-{
-	int ret;
-	bool val;
 
-	ret = strtobool(buf, &val);
-	if (ret)
-		return ret;
-
-	fsl_a008585_enable = val;
-	return 0;
-}
-early_param("clocksource.arm_arch_timer.fsl-a008585", early_fsl_a008585_cfg);
-
-u32 __fsl_a008585_read_cntp_tval_el0(void)
+/*
+ * The number of retries is an arbitrary value well beyond the highest number
+ * of iterations the loop has been observed to take.
+ */
+#define __fsl_a008585_read_reg(reg) ({			\
+	u64 _old, _new;					\
+	int _retries = 200;				\
+							\
+	do {						\
+		_old = read_sysreg(reg);		\
+		_new = read_sysreg(reg);		\
+		_retries--;				\
+	} while (unlikely(_old != _new) && _retries);	\
+							\
+	WARN_ON_ONCE(!_retries);			\
+	_new;						\
+})
+
+static u32 notrace fsl_a008585_read_cntp_tval_el0(void)
 {
 	return __fsl_a008585_read_reg(cntp_tval_el0);
 }
 
-u32 __fsl_a008585_read_cntv_tval_el0(void)
+static u32 notrace fsl_a008585_read_cntv_tval_el0(void)
 {
 	return __fsl_a008585_read_reg(cntv_tval_el0);
 }
 
-u64 __fsl_a008585_read_cntvct_el0(void)
+static u64 notrace fsl_a008585_read_cntvct_el0(void)
 {
 	return __fsl_a008585_read_reg(cntvct_el0);
 }
-EXPORT_SYMBOL(__fsl_a008585_read_cntvct_el0);
-#endif /* CONFIG_FSL_ERRATUM_A008585 */
+#endif
+
+#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
+const struct arch_timer_erratum_workaround *timer_unstable_counter_workaround = NULL;
+EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
+
+DEFINE_STATIC_KEY_FALSE(arch_timer_read_ool_enabled);
+EXPORT_SYMBOL_GPL(arch_timer_read_ool_enabled);
+
+static const struct arch_timer_erratum_workaround ool_workarounds[] = {
+#ifdef CONFIG_FSL_ERRATUM_A008585
+	{
+		.id = "fsl,erratum-a008585",
+		.read_cntp_tval_el0 = fsl_a008585_read_cntp_tval_el0,
+		.read_cntv_tval_el0 = fsl_a008585_read_cntv_tval_el0,
+		.read_cntvct_el0 = fsl_a008585_read_cntvct_el0,
+	},
+#endif
+};
+#endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
 static __always_inline
 void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val,
@@ -281,8 +299,8 @@ static __always_inline void set_next_event(const int access, unsigned long evt,
 	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
 }
 
-#ifdef CONFIG_FSL_ERRATUM_A008585
-static __always_inline void fsl_a008585_set_next_event(const int access,
+#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
+static __always_inline void erratum_set_next_event_generic(const int access,
 		unsigned long evt, struct clock_event_device *clk)
 {
 	unsigned long ctrl;
@@ -300,20 +318,20 @@ static __always_inline void fsl_a008585_set_next_event(const int access,
 	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
 }
 
-static int fsl_a008585_set_next_event_virt(unsigned long evt,
+static int erratum_set_next_event_virt(unsigned long evt,
 					   struct clock_event_device *clk)
 {
-	fsl_a008585_set_next_event(ARCH_TIMER_VIRT_ACCESS, evt, clk);
+	erratum_set_next_event_generic(ARCH_TIMER_VIRT_ACCESS, evt, clk);
 	return 0;
 }
 
-static int fsl_a008585_set_next_event_phys(unsigned long evt,
+static int erratum_set_next_event_phys(unsigned long evt,
 					   struct clock_event_device *clk)
 {
-	fsl_a008585_set_next_event(ARCH_TIMER_PHYS_ACCESS, evt, clk);
+	erratum_set_next_event_generic(ARCH_TIMER_PHYS_ACCESS, evt, clk);
 	return 0;
 }
-#endif /* CONFIG_FSL_ERRATUM_A008585 */
+#endif /* CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND */
 
 static int arch_timer_set_next_event_virt(unsigned long evt,
 					  struct clock_event_device *clk)
@@ -343,16 +361,16 @@ static int arch_timer_set_next_event_phys_mem(unsigned long evt,
 	return 0;
 }
 
-static void fsl_a008585_set_sne(struct clock_event_device *clk)
+static void erratum_workaround_set_sne(struct clock_event_device *clk)
 {
-#ifdef CONFIG_FSL_ERRATUM_A008585
+#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
 	if (!static_branch_unlikely(&arch_timer_read_ool_enabled))
 		return;
 
 	if (arch_timer_uses_ppi == VIRT_PPI)
-		clk->set_next_event = fsl_a008585_set_next_event_virt;
+		clk->set_next_event = erratum_set_next_event_virt;
 	else
-		clk->set_next_event = fsl_a008585_set_next_event_phys;
+		clk->set_next_event = erratum_set_next_event_phys;
 #endif
 }
 
@@ -385,7 +403,7 @@ static void __arch_timer_setup(unsigned type,
 			BUG();
 		}
 
-		fsl_a008585_set_sne(clk);
+		erratum_workaround_set_sne(clk);
 	} else {
 		clk->features |= CLOCK_EVT_FEAT_DYNIRQ;
 		clk->name = "arch_mem_timer";
@@ -605,7 +623,7 @@ static void __init arch_counter_register(unsigned type)
 
 		clocksource_counter.archdata.vdso_direct = true;
 
-#ifdef CONFIG_FSL_ERRATUM_A008585
+#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
 		/*
 		 * Don't use the vdso fastpath if errata require using
 		 * the out-of-line counter accessor.
@@ -893,12 +911,15 @@ static int __init arch_timer_of_init(struct device_node *np)
 
 	arch_timer_c3stop = !of_property_read_bool(np, "always-on");
 
-#ifdef CONFIG_FSL_ERRATUM_A008585
-	if (fsl_a008585_enable < 0)
-		fsl_a008585_enable = of_property_read_bool(np, "fsl,erratum-a008585");
-	if (fsl_a008585_enable) {
-		static_branch_enable(&arch_timer_read_ool_enabled);
-		pr_info("Enabling workaround for FSL erratum A-008585\n");
+#ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
+	for (i = 0; i < ARRAY_SIZE(ool_workarounds); i++) {
+		if (of_property_read_bool(np, ool_workarounds[i].id)) {
+			timer_unstable_counter_workaround = &ool_workarounds[i];
+			static_branch_enable(&arch_timer_read_ool_enabled);
+			pr_info("arch_timer: Enabling workaround for %s\n",
+				timer_unstable_counter_workaround->id);
+			break;
+		}
 	}
 #endif
 
-- 
1.9.0

^ permalink raw reply related

* [PATCH v7 1/4] arm64: arch_timer: Add device tree binding for hisilicon-161601 erratum
From: Ding Tianhong @ 2017-01-07  7:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483772858-10380-1-git-send-email-dingtianhong@huawei.com>

This erratum describes a bug in logic outside the core, so MIDR can't be
used to identify its presence, and reading an SoC-specific revision
register from common arch timer code would be awkward.  So, describe it
in the device tree.

Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
index ad440a2..935f142 100644
--- a/Documentation/devicetree/bindings/arm/arch_timer.txt
+++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
@@ -31,6 +31,14 @@ to deliver its interrupts via SPIs.
   This also affects writes to the tval register, due to the implicit
   counter read.
 
+- hisilicon,erratum-161601 : A boolean property. Indicates the presence of
+  erratum 161601, which says that reading the counter is unreliable unless
+  reading twice on the register and the value of the second read is larger
+  than the first by less than 32. If the verification is unsuccessful, then
+  discard the value of this read and repeat this procedure until the verification
+  is successful.  This also affects writes to the tval register, due to the
+  implicit counter read.
+
 ** Optional properties:
 
 - arm,cpu-registers-not-fw-configured : Firmware does not initialize
-- 
1.9.0

^ permalink raw reply related

* [PATCH v7 0/4] arm64: arch_timer: Add workaround for hisilicon-161601 erratum
From: Ding Tianhong @ 2017-01-07  7:07 UTC (permalink / raw)
  To: linux-arm-kernel

Erratum Hisilicon-161601 says that the ARM generic timer counter "has the
potential to contain an erroneous value when the timer value changes".
Accesses to TVAL (both read and write) are also affected due to the implicit counter
read.  Accesses to CVAL are not affected.

The workaround is to reread the system count registers until the value of the second
read is larger than the first one by less than 32, the system counter can be guaranteed
not to return wrong value twice by back-to-back read and the error value is always larger
than the correct one by 32. Writes to TVAL are replaced with an equivalent write to CVAL.

v2: Introducing a new generic erratum handling mechanism for fsl,a008585 and hisilicon,161601.
    Significant rework based on feedback, including seperate the fsl erratum a008585
    to another patch, update the erratum name and remove unwanted code.

v3: Introducing the erratum_workaround_set_sne generic function for fsl erratum a008585
    and make the #define __fsl_a008585_read_reg to be private to the .c file instead of
    being globally visible. After discussion with Marc and Will, a consensus decision was
    made to remove the commandline parameter for enabling fsl,erratum-a008585 erratum,
    and make some generic name more specific, export timer_unstable_counter_workaround
    for module access.
    
    Significant rework based on feedback, including fix some alignment problem, make the
    #define __hisi_161601_read_reg to be private to the .c file instead of being globally
    visible, add more accurate annotation and modify a bit of logical format to enable
    arch_timer_read_ool_enabled, remove the kernel commandline parameter
    clocksource.arm_arch_timer.hisilicon-161601.

    Introduce a generic aquick framework for erratum in ACPI mode.

v4: rename the quirk handler parameter to make it more generic, and
    avoid break loop when handling the quirk becasue it need to
    support multi quirks handler.

    update some data structures for acpi mode. 

v5: Adapt the new kernel-parameters.txt for latest kernel version.
    Set the retries of reread system counter to 50, because it is possible 
    that some interrupts may lead to more than twice read errors and break the loop,
    it will trigger the warning, so we set the number of retries far beyond the number of
    iterations the loop has been observed to take.

v6: The last 2 patches in the previous version about the ACPI mode will conflict witch Fuwei's
    GTDT patches, so remove the ACPI part and only support the DT base code for this patch set.

    We have trigger a bug when select the CONFIG_FUNCTION_GRAPH_TRACER and enable function_graph
    to /sys/kernel/debug/tracing/current_tracer, the system will stall into an endless loop, it looks
    like that the ftrace_graph_caller will be related to xxx.read_cntvct_el0 and read the system counter
    again, so mark the xxx.read_cntvct_el0 with notrace to fix the problem.

v7: Introduce a new general config symbol named CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND to enable the workaround
    for any chips which has similar arch timer erratum just like "fsl,erratum_a008585" and "hisilicon,erratum_161601",
    modify the struct arch_timer_erratum_workaround to be compatible different chip erratum more easily, and
    reconstruction some code base on the new config symbol and struct, thanks to Marc's suggestion. 

Ding Tianhong (4):
  arm64: arch_timer: Add device tree binding for hisilicon-161601
    erratum
  arm64: arch_timer: Introduce a generic erratum handing mechanism for
    fsl-a008585
  arm64: arch_timer: Work around Erratum Hisilicon-161601
  arm64: arch timer: Add timer erratum property for Hip05-d02 and
    Hip06-d03

 Documentation/admin-guide/kernel-parameters.txt    |   9 --
 Documentation/arm64/silicon-errata.txt             |   1 +
 .../devicetree/bindings/arm/arch_timer.txt         |   8 ++
 arch/arm64/boot/dts/hisilicon/hip05.dtsi           |   1 +
 arch/arm64/boot/dts/hisilicon/hip06.dtsi           |   1 +
 arch/arm64/include/asm/arch_timer.h                |  38 ++----
 drivers/clocksource/Kconfig                        |  18 +++
 drivers/clocksource/arm_arch_timer.c               | 150 +++++++++++++++------
 8 files changed, 152 insertions(+), 74 deletions(-)

-- 
1.9.0

^ permalink raw reply

* [PATCH v5 3/3] dmaengine: xilinx_dma: Fix race condition in the driver for multiple descriptor scenario
From: Kedareswara rao Appana @ 2017-01-07  6:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483771530-8545-1-git-send-email-appanad@xilinx.com>

When driver is handling AXI DMA SoftIP
When user submits multiple descriptors back to back on the S2MM(recv)
side with the current driver flow the last buffer descriptor next bd
points to a invalid location resulting the invalid data or errors in the
DMA engine.

This patch fixes this issue by creating a BD Chain during
channel allocation itself and use those BD's.

Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v5:
---> None.
Changes for v4:
---> None.
Changes for v3:
---> None.
Changes for v2:
---> None.

 drivers/dma/xilinx/xilinx_dma.c | 133 +++++++++++++++++++++++++---------------
 1 file changed, 83 insertions(+), 50 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 0e9c02e..af2159d 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -163,6 +163,7 @@
 #define XILINX_DMA_BD_SOP		BIT(27)
 #define XILINX_DMA_BD_EOP		BIT(26)
 #define XILINX_DMA_COALESCE_MAX		255
+#define XILINX_DMA_NUM_DESCS		255
 #define XILINX_DMA_NUM_APP_WORDS	5
 
 /* Multi-Channel DMA Descriptor offsets*/
@@ -310,6 +311,7 @@ struct xilinx_dma_tx_descriptor {
  * @pending_list: Descriptors waiting
  * @active_list: Descriptors ready to submit
  * @done_list: Complete descriptors
+ * @free_seg_list: Free descriptors
  * @common: DMA common channel
  * @desc_pool: Descriptors pool
  * @dev: The dma device
@@ -331,7 +333,9 @@ struct xilinx_dma_tx_descriptor {
  * @desc_submitcount: Descriptor h/w submitted count
  * @residue: Residue for AXI DMA
  * @seg_v: Statically allocated segments base
+ * @seg_p: Physical allocated segments base
  * @cyclic_seg_v: Statically allocated segment base for cyclic transfers
+ * @cyclic_seg_p: Physical allocated segments base for cyclic dma
  * @start_transfer: Differentiate b/w DMA IP's transfer
  */
 struct xilinx_dma_chan {
@@ -342,6 +346,7 @@ struct xilinx_dma_chan {
 	struct list_head pending_list;
 	struct list_head active_list;
 	struct list_head done_list;
+	struct list_head free_seg_list;
 	struct dma_chan common;
 	struct dma_pool *desc_pool;
 	struct device *dev;
@@ -363,7 +368,9 @@ struct xilinx_dma_chan {
 	u32 desc_submitcount;
 	u32 residue;
 	struct xilinx_axidma_tx_segment *seg_v;
+	dma_addr_t seg_p;
 	struct xilinx_axidma_tx_segment *cyclic_seg_v;
+	dma_addr_t cyclic_seg_p;
 	void (*start_transfer)(struct xilinx_dma_chan *chan);
 	u16 tdest;
 };
@@ -569,17 +576,31 @@ static struct xilinx_axidma_tx_segment *
 xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan)
 {
 	struct xilinx_axidma_tx_segment *segment;
-	dma_addr_t phys;
-
-	segment = dma_pool_zalloc(chan->desc_pool, GFP_ATOMIC, &phys);
-	if (!segment)
-		return NULL;
+	unsigned long flags;
 
-	segment->phys = phys;
+	spin_lock_irqsave(&chan->lock, flags);
+	if (!list_empty(&chan->free_seg_list)) {
+		segment = list_first_entry(&chan->free_seg_list,
+					   struct xilinx_axidma_tx_segment,
+					   node);
+		list_del(&segment->node);
+	}
+	spin_unlock_irqrestore(&chan->lock, flags);
 
 	return segment;
 }
 
+static void xilinx_dma_clean_hw_desc(struct xilinx_axidma_desc_hw *hw)
+{
+	u32 next_desc = hw->next_desc;
+	u32 next_desc_msb = hw->next_desc_msb;
+
+	memset(hw, 0, sizeof(struct xilinx_axidma_desc_hw));
+
+	hw->next_desc = next_desc;
+	hw->next_desc_msb = next_desc_msb;
+}
+
 /**
  * xilinx_dma_free_tx_segment - Free transaction segment
  * @chan: Driver specific DMA channel
@@ -588,7 +609,9 @@ xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan)
 static void xilinx_dma_free_tx_segment(struct xilinx_dma_chan *chan,
 				struct xilinx_axidma_tx_segment *segment)
 {
-	dma_pool_free(chan->desc_pool, segment, segment->phys);
+	xilinx_dma_clean_hw_desc(&segment->hw);
+
+	list_add_tail(&segment->node, &chan->free_seg_list);
 }
 
 /**
@@ -713,16 +736,26 @@ static void xilinx_dma_free_descriptors(struct xilinx_dma_chan *chan)
 static void xilinx_dma_free_chan_resources(struct dma_chan *dchan)
 {
 	struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
+	unsigned long flags;
 
 	dev_dbg(chan->dev, "Free all channel resources.\n");
 
 	xilinx_dma_free_descriptors(chan);
+
 	if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
-		xilinx_dma_free_tx_segment(chan, chan->cyclic_seg_v);
-		xilinx_dma_free_tx_segment(chan, chan->seg_v);
+		spin_lock_irqsave(&chan->lock, flags);
+		INIT_LIST_HEAD(&chan->free_seg_list);
+		spin_unlock_irqrestore(&chan->lock, flags);
+
+		/* Free Memory that is allocated for cyclic DMA Mode */
+		dma_free_coherent(chan->dev, sizeof(*chan->cyclic_seg_v),
+				  chan->cyclic_seg_v, chan->cyclic_seg_p);
+	}
+
+	if (chan->xdev->dma_config->dmatype != XDMA_TYPE_AXIDMA) {
+		dma_pool_destroy(chan->desc_pool);
+		chan->desc_pool = NULL;
 	}
-	dma_pool_destroy(chan->desc_pool);
-	chan->desc_pool = NULL;
 }
 
 /**
@@ -805,6 +838,7 @@ static void xilinx_dma_do_tasklet(unsigned long data)
 static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
 {
 	struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
+	int i;
 
 	/* Has this channel already been allocated? */
 	if (chan->desc_pool)
@@ -815,11 +849,30 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
 	 * for meeting Xilinx VDMA specification requirement.
 	 */
 	if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
-		chan->desc_pool = dma_pool_create("xilinx_dma_desc_pool",
-				   chan->dev,
-				   sizeof(struct xilinx_axidma_tx_segment),
-				   __alignof__(struct xilinx_axidma_tx_segment),
-				   0);
+		/* Allocate the buffer descriptors. */
+		chan->seg_v = dma_zalloc_coherent(chan->dev,
+						  sizeof(*chan->seg_v) *
+						  XILINX_DMA_NUM_DESCS,
+						  &chan->seg_p, GFP_KERNEL);
+		if (!chan->seg_v) {
+			dev_err(chan->dev,
+				"unable to allocate channel %d descriptors\n",
+				chan->id);
+			return -ENOMEM;
+		}
+
+		for (i = 0; i < XILINX_DMA_NUM_DESCS; i++) {
+			chan->seg_v[i].hw.next_desc =
+			lower_32_bits(chan->seg_p + sizeof(*chan->seg_v) *
+				((i + 1) % XILINX_DMA_NUM_DESCS));
+			chan->seg_v[i].hw.next_desc_msb =
+			upper_32_bits(chan->seg_p + sizeof(*chan->seg_v) *
+				((i + 1) % XILINX_DMA_NUM_DESCS));
+			chan->seg_v[i].phys = chan->seg_p +
+				sizeof(*chan->seg_v) * i;
+			list_add_tail(&chan->seg_v[i].node,
+				      &chan->free_seg_list);
+		}
 	} else if (chan->xdev->dma_config->dmatype == XDMA_TYPE_CDMA) {
 		chan->desc_pool = dma_pool_create("xilinx_cdma_desc_pool",
 				   chan->dev,
@@ -834,7 +887,8 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
 				     0);
 	}
 
-	if (!chan->desc_pool) {
+	if (!chan->desc_pool &&
+	    (chan->xdev->dma_config->dmatype != XDMA_TYPE_AXIDMA)) {
 		dev_err(chan->dev,
 			"unable to allocate channel %d descriptor pool\n",
 			chan->id);
@@ -843,22 +897,20 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
 
 	if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
 		/*
-		 * For AXI DMA case after submitting a pending_list, keep
-		 * an extra segment allocated so that the "next descriptor"
-		 * pointer on the tail descriptor always points to a
-		 * valid descriptor, even when paused after reaching taildesc.
-		 * This way, it is possible to issue additional
-		 * transfers without halting and restarting the channel.
-		 */
-		chan->seg_v = xilinx_axidma_alloc_tx_segment(chan);
-
-		/*
 		 * For cyclic DMA mode we need to program the tail Descriptor
 		 * register with a value which is not a part of the BD chain
 		 * so allocating a desc segment during channel allocation for
 		 * programming tail descriptor.
 		 */
-		chan->cyclic_seg_v = xilinx_axidma_alloc_tx_segment(chan);
+		chan->cyclic_seg_v = dma_zalloc_coherent(chan->dev,
+					sizeof(*chan->cyclic_seg_v),
+					&chan->cyclic_seg_p, GFP_KERNEL);
+		if (!chan->cyclic_seg_v) {
+			dev_err(chan->dev,
+				"unable to allocate desc segment for cyclic DMA\n");
+			return -ENOMEM;
+		}
+		chan->cyclic_seg_v->phys = chan->cyclic_seg_p;
 	}
 
 	dma_cookie_init(dchan);
@@ -1198,7 +1250,7 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan)
 static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
 {
 	struct xilinx_dma_tx_descriptor *head_desc, *tail_desc;
-	struct xilinx_axidma_tx_segment *tail_segment, *old_head, *new_head;
+	struct xilinx_axidma_tx_segment *tail_segment;
 	u32 reg;
 
 	if (chan->err)
@@ -1217,21 +1269,6 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
 	tail_segment = list_last_entry(&tail_desc->segments,
 				       struct xilinx_axidma_tx_segment, node);
 
-	if (chan->has_sg && !chan->xdev->mcdma) {
-		old_head = list_first_entry(&head_desc->segments,
-					struct xilinx_axidma_tx_segment, node);
-		new_head = chan->seg_v;
-		/* Copy Buffer Descriptor fields. */
-		new_head->hw = old_head->hw;
-
-		/* Swap and save new reserve */
-		list_replace_init(&old_head->node, &new_head->node);
-		chan->seg_v = old_head;
-
-		tail_segment->hw.next_desc = chan->seg_v->phys;
-		head_desc->async_tx.phys = new_head->phys;
-	}
-
 	reg = dma_ctrl_read(chan, XILINX_DMA_REG_DMACR);
 
 	if (chan->desc_pendingcount <= XILINX_DMA_COALESCE_MAX) {
@@ -1729,7 +1766,7 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
 {
 	struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
 	struct xilinx_dma_tx_descriptor *desc;
-	struct xilinx_axidma_tx_segment *segment = NULL, *prev = NULL;
+	struct xilinx_axidma_tx_segment *segment = NULL;
 	u32 *app_w = (u32 *)context;
 	struct scatterlist *sg;
 	size_t copy;
@@ -1780,10 +1817,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
 					       XILINX_DMA_NUM_APP_WORDS);
 			}
 
-			if (prev)
-				prev->hw.next_desc = segment->phys;
-
-			prev = segment;
 			sg_used += copy;
 
 			/*
@@ -1797,7 +1830,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
 	segment = list_first_entry(&desc->segments,
 				   struct xilinx_axidma_tx_segment, node);
 	desc->async_tx.phys = segment->phys;
-	prev->hw.next_desc = segment->phys;
 
 	/* For the last DMA_MEM_TO_DEV transfer, set EOP */
 	if (chan->direction == DMA_MEM_TO_DEV) {
@@ -2341,6 +2373,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
 	INIT_LIST_HEAD(&chan->pending_list);
 	INIT_LIST_HEAD(&chan->done_list);
 	INIT_LIST_HEAD(&chan->active_list);
+	INIT_LIST_HEAD(&chan->free_seg_list);
 
 	/* Retrieve the channel properties from the device tree */
 	has_dre = of_property_read_bool(node, "xlnx,include-dre");
-- 
2.1.2

^ permalink raw reply related

* [PATCH v5 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma
From: Kedareswara rao Appana @ 2017-01-07  6:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483771530-8545-1-git-send-email-appanad@xilinx.com>

When VDMA is configured for more than one frame in the h/w
for example h/w is configured for n number of frames and user
Submits n number of frames and triggered the DMA using issue_pending API.
In the current driver flow we are submitting one frame at a time
but we should submit all the n number of frames at one time as the h/w
Is configured for n number of frames.

This patch fixes this issue.

Reviewed-by: Jose Abreu <joabreu@synopsys.com>
Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v5:
---> Updated xlnx,fstore-config property to xlnx,fstore-enable
     and updated description as suggested by Rob.
Changes for v4:
---> Add Check for framestore configuration on Transmit case as well
     as suggested by Jose Abreu.
---> Modified the dev_dbg checks to dev_warn checks as suggested
     by Jose Abreu.
Changes for v3:
---> Added Checks for frame store configuration. If frame store
     Configuration is not present at the h/w level and user
     Submits less frames added debug prints in the driver as relevant.
Changes for v2:
---> Fixed race conditions in the driver as suggested by Jose Abreu
---> Fixed unnecessray if else checks in the vdma_start_transfer
     as suggested by Laurent Pinchart.

 .../devicetree/bindings/dma/xilinx/xilinx_dma.txt  |  2 +
 drivers/dma/xilinx/xilinx_dma.c                    | 78 +++++++++++++++-------
 2 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
index a2b8bfa..e951c09 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
@@ -66,6 +66,8 @@ Optional child node properties:
 Optional child node properties for VDMA:
 - xlnx,genlock-mode: Tells Genlock synchronization is
 	enabled/disabled in hardware.
+- xlnx,fstore-enable: boolean; if defined, it indicates that controller
+	supports frame store configuration.
 Optional child node properties for AXI DMA:
 -dma-channels: Number of dma channels in child node.
 
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index be7eb41..0e9c02e 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -322,6 +322,7 @@ struct xilinx_dma_tx_descriptor {
  * @genlock: Support genlock mode
  * @err: Channel has errors
  * @idle: Check for channel idle
+ * @has_fstoreen: Check for frame store configuration
  * @tasklet: Cleanup work after irq
  * @config: Device configuration info
  * @flush_on_fsync: Flush on Frame sync
@@ -353,6 +354,7 @@ struct xilinx_dma_chan {
 	bool genlock;
 	bool err;
 	bool idle;
+	bool has_fstoreen;
 	struct tasklet_struct tasklet;
 	struct xilinx_vdma_config config;
 	bool flush_on_fsync;
@@ -990,6 +992,27 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
 	if (list_empty(&chan->pending_list))
 		return;
 
+	/*
+	 * Note: When VDMA is built with default h/w configuration
+	 * User should submit frames upto H/W configured.
+	 * If users submits less than h/w configured
+	 * VDMA engine tries to write to a invalid location
+	 * Results undefined behaviour/memory corruption.
+	 *
+	 * If user would like to submit frames less than h/w capable
+	 * On S2MM side please enable debug info 13 at the h/w level
+	 * On MM2S side please enable debug info 6 at the h/w level
+	 * It will allows the frame buffers numbers to be modified at runtime.
+	 */
+	if (!chan->has_fstoreen &&
+	     chan->desc_pendingcount < chan->num_frms) {
+		dev_warn(chan->dev, "Frame Store Configuration is not enabled at the\n");
+		dev_warn(chan->dev, "H/w level enable Debug info 13 or 6 at the h/w level\n");
+		dev_warn(chan->dev, "OR Submit the frames upto h/w Capable\n\r");
+
+		return;
+	}
+
 	desc = list_first_entry(&chan->pending_list,
 				struct xilinx_dma_tx_descriptor, node);
 	tail_desc = list_last_entry(&chan->pending_list,
@@ -1052,25 +1075,38 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
 	if (chan->has_sg) {
 		dma_ctrl_write(chan, XILINX_DMA_REG_TAILDESC,
 				tail_segment->phys);
+		list_splice_tail_init(&chan->pending_list, &chan->active_list);
+		chan->desc_pendingcount = 0;
 	} else {
 		struct xilinx_vdma_tx_segment *segment, *last = NULL;
-		int i = 0;
+		int i = 0, j = 0;
 
 		if (chan->desc_submitcount < chan->num_frms)
 			i = chan->desc_submitcount;
 
-		list_for_each_entry(segment, &desc->segments, node) {
-			if (chan->ext_addr)
-				vdma_desc_write_64(chan,
-					XILINX_VDMA_REG_START_ADDRESS_64(i++),
-					segment->hw.buf_addr,
-					segment->hw.buf_addr_msb);
-			else
-				vdma_desc_write(chan,
-					XILINX_VDMA_REG_START_ADDRESS(i++),
-					segment->hw.buf_addr);
-
-			last = segment;
+		for (j = 0; j < chan->num_frms; ) {
+			list_for_each_entry(segment, &desc->segments, node) {
+				if (chan->ext_addr)
+					vdma_desc_write_64(chan,
+					  XILINX_VDMA_REG_START_ADDRESS_64(i++),
+					  segment->hw.buf_addr,
+					  segment->hw.buf_addr_msb);
+				else
+					vdma_desc_write(chan,
+					    XILINX_VDMA_REG_START_ADDRESS(i++),
+					    segment->hw.buf_addr);
+
+				last = segment;
+			}
+			list_del(&desc->node);
+			list_add_tail(&desc->node, &chan->active_list);
+			j++;
+			if (list_empty(&chan->pending_list) ||
+			    (i == chan->num_frms))
+				break;
+			desc = list_first_entry(&chan->pending_list,
+						struct xilinx_dma_tx_descriptor,
+						node);
 		}
 
 		if (!last)
@@ -1081,20 +1117,14 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
 		vdma_desc_write(chan, XILINX_DMA_REG_FRMDLY_STRIDE,
 				last->hw.stride);
 		vdma_desc_write(chan, XILINX_DMA_REG_VSIZE, last->hw.vsize);
-	}
 
-	chan->idle = false;
-	if (!chan->has_sg) {
-		list_del(&desc->node);
-		list_add_tail(&desc->node, &chan->active_list);
-		chan->desc_submitcount++;
-		chan->desc_pendingcount--;
+		chan->desc_submitcount += j;
+		chan->desc_pendingcount -= j;
 		if (chan->desc_submitcount == chan->num_frms)
 			chan->desc_submitcount = 0;
-	} else {
-		list_splice_tail_init(&chan->pending_list, &chan->active_list);
-		chan->desc_pendingcount = 0;
 	}
+
+	chan->idle = false;
 }
 
 /**
@@ -1342,6 +1372,7 @@ static int xilinx_dma_reset(struct xilinx_dma_chan *chan)
 
 	chan->err = false;
 	chan->idle = true;
+	chan->desc_submitcount = 0;
 
 	return err;
 }
@@ -2315,6 +2346,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
 	has_dre = of_property_read_bool(node, "xlnx,include-dre");
 
 	chan->genlock = of_property_read_bool(node, "xlnx,genlock-mode");
+	chan->has_fstoreen = of_property_read_bool(node, "xlnx,fstore-enable");
 
 	err = of_property_read_u32(node, "xlnx,datawidth", &value);
 	if (err) {
-- 
2.1.2

^ permalink raw reply related

* [PATCH v5 1/3] dmaengine: xilinx_dma: Check for channel idle state before submitting dma descriptor
From: Kedareswara rao Appana @ 2017-01-07  6:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483771530-8545-1-git-send-email-appanad@xilinx.com>

Add channel idle state to ensure that dma descriptor is not
submitted when VDMA engine is in progress.

Reviewed-by: Jose Abreu <joabreu@synopsys.com>
Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v5:
---> None.
Changes for v4:
---> None.
Changes for v3:
---> None.
Changes for v2:
---> Add idle check in the reset as suggested by Jose Abreu
---> Removed xilinx_dma_is_running/xilinx_dma_is_idle checks
    in the driver and used common idle checks across the driver
    as suggested by Laurent Pinchart.

 drivers/dma/xilinx/xilinx_dma.c | 56 +++++++++++++----------------------------
 1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 8288fe4..be7eb41 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -321,6 +321,7 @@ struct xilinx_dma_tx_descriptor {
  * @cyclic: Check for cyclic transfers.
  * @genlock: Support genlock mode
  * @err: Channel has errors
+ * @idle: Check for channel idle
  * @tasklet: Cleanup work after irq
  * @config: Device configuration info
  * @flush_on_fsync: Flush on Frame sync
@@ -351,6 +352,7 @@ struct xilinx_dma_chan {
 	bool cyclic;
 	bool genlock;
 	bool err;
+	bool idle;
 	struct tasklet_struct tasklet;
 	struct xilinx_vdma_config config;
 	bool flush_on_fsync;
@@ -920,32 +922,6 @@ static enum dma_status xilinx_dma_tx_status(struct dma_chan *dchan,
 }
 
 /**
- * xilinx_dma_is_running - Check if DMA channel is running
- * @chan: Driver specific DMA channel
- *
- * Return: '1' if running, '0' if not.
- */
-static bool xilinx_dma_is_running(struct xilinx_dma_chan *chan)
-{
-	return !(dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
-		 XILINX_DMA_DMASR_HALTED) &&
-		(dma_ctrl_read(chan, XILINX_DMA_REG_DMACR) &
-		 XILINX_DMA_DMACR_RUNSTOP);
-}
-
-/**
- * xilinx_dma_is_idle - Check if DMA channel is idle
- * @chan: Driver specific DMA channel
- *
- * Return: '1' if idle, '0' if not.
- */
-static bool xilinx_dma_is_idle(struct xilinx_dma_chan *chan)
-{
-	return dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
-		XILINX_DMA_DMASR_IDLE;
-}
-
-/**
  * xilinx_dma_halt - Halt DMA channel
  * @chan: Driver specific DMA channel
  */
@@ -966,6 +942,7 @@ static void xilinx_dma_halt(struct xilinx_dma_chan *chan)
 			chan, dma_ctrl_read(chan, XILINX_DMA_REG_DMASR));
 		chan->err = true;
 	}
+	chan->idle = true;
 }
 
 /**
@@ -1007,6 +984,9 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
 	if (chan->err)
 		return;
 
+	if (!chan->idle)
+		return;
+
 	if (list_empty(&chan->pending_list))
 		return;
 
@@ -1018,13 +998,6 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
 	tail_segment = list_last_entry(&tail_desc->segments,
 				       struct xilinx_vdma_tx_segment, node);
 
-	/* If it is SG mode and hardware is busy, cannot submit */
-	if (chan->has_sg && xilinx_dma_is_running(chan) &&
-	    !xilinx_dma_is_idle(chan)) {
-		dev_dbg(chan->dev, "DMA controller still busy\n");
-		return;
-	}
-
 	/*
 	 * If hardware is idle, then all descriptors on the running lists are
 	 * done, start new transfers
@@ -1110,6 +1083,7 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
 		vdma_desc_write(chan, XILINX_DMA_REG_VSIZE, last->hw.vsize);
 	}
 
+	chan->idle = false;
 	if (!chan->has_sg) {
 		list_del(&desc->node);
 		list_add_tail(&desc->node, &chan->active_list);
@@ -1136,6 +1110,9 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan)
 	if (chan->err)
 		return;
 
+	if (!chan->idle)
+		return;
+
 	if (list_empty(&chan->pending_list))
 		return;
 
@@ -1181,6 +1158,7 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan)
 
 	list_splice_tail_init(&chan->pending_list, &chan->active_list);
 	chan->desc_pendingcount = 0;
+	chan->idle = false;
 }
 
 /**
@@ -1196,15 +1174,11 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
 	if (chan->err)
 		return;
 
-	if (list_empty(&chan->pending_list))
+	if (!chan->idle)
 		return;
 
-	/* If it is SG mode and hardware is busy, cannot submit */
-	if (chan->has_sg && xilinx_dma_is_running(chan) &&
-	    !xilinx_dma_is_idle(chan)) {
-		dev_dbg(chan->dev, "DMA controller still busy\n");
+	if (list_empty(&chan->pending_list))
 		return;
-	}
 
 	head_desc = list_first_entry(&chan->pending_list,
 				     struct xilinx_dma_tx_descriptor, node);
@@ -1302,6 +1276,7 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
 
 	list_splice_tail_init(&chan->pending_list, &chan->active_list);
 	chan->desc_pendingcount = 0;
+	chan->idle = false;
 }
 
 /**
@@ -1366,6 +1341,7 @@ static int xilinx_dma_reset(struct xilinx_dma_chan *chan)
 	}
 
 	chan->err = false;
+	chan->idle = true;
 
 	return err;
 }
@@ -1447,6 +1423,7 @@ static irqreturn_t xilinx_dma_irq_handler(int irq, void *data)
 	if (status & XILINX_DMA_DMASR_FRM_CNT_IRQ) {
 		spin_lock(&chan->lock);
 		xilinx_dma_complete_descriptor(chan);
+		chan->idle = true;
 		chan->start_transfer(chan);
 		spin_unlock(&chan->lock);
 	}
@@ -2327,6 +2304,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
 	chan->has_sg = xdev->has_sg;
 	chan->desc_pendingcount = 0x0;
 	chan->ext_addr = xdev->ext_addr;
+	chan->idle = true;
 
 	spin_lock_init(&chan->lock);
 	INIT_LIST_HEAD(&chan->pending_list);
-- 
2.1.2

^ permalink raw reply related

* [PATCH v5 0/3] dmaengine: xilinx_dma: Bug fixes
From: Kedareswara rao Appana @ 2017-01-07  6:45 UTC (permalink / raw)
  To: linux-arm-kernel

This patch series fixes below bugs in DMA and VDMA IP's
---> Do not start VDMA until frame buffer is processed by the h/w Fix 
---> bug in Multi frame sotres handling in VDMA Fix issues w.r.to multi 
---> frame descriptors submit with AXI DMA S2MM(recv) Side.

Kedareswara rao Appana (3):
  dmaengine: xilinx_dma: Check for channel idle state before submitting
    dma descriptor
  dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in
    vdma
  dmaengine: xilinx_dma: Fix race condition in the driver for multiple
    descriptor scenario

 .../devicetree/bindings/dma/xilinx/xilinx_dma.txt  |   2 +
 drivers/dma/xilinx/xilinx_dma.c                    | 265 ++++++++++++---------
 2 files changed, 156 insertions(+), 111 deletions(-)

-- 
2.1.2

^ permalink raw reply

* [PATCH v1 3/3] thermal: zx2967: add thermal driver for ZTE's zx2967 family
From: Baoyou Xie @ 2017-01-07  5:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483767488-19778-1-git-send-email-baoyou.xie@linaro.org>

This patch adds thermal driver for ZTE's zx2967 family.

Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
 drivers/thermal/Kconfig          |   6 +
 drivers/thermal/Makefile         |   1 +
 drivers/thermal/zx2967_thermal.c | 241 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 248 insertions(+)
 create mode 100644 drivers/thermal/zx2967_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 18f2de6..0dd597e 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -445,3 +445,9 @@ config BCM2835_THERMAL
 	  Support for thermal sensors on Broadcom bcm2835 SoCs.
 
 endif
+
+config ZX2967_THERMAL
+	tristate "Thermal sensors on zx2967 SoC"
+	depends on ARCH_ZX
+	help
+	  Support for thermal sensors on ZTE zx2967 SoCs.
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 677c6d9..c00c05e 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -57,3 +57,4 @@ obj-$(CONFIG_HISI_THERMAL)     += hisi_thermal.o
 obj-$(CONFIG_MTK_THERMAL)	+= mtk_thermal.o
 obj-$(CONFIG_GENERIC_ADC_THERMAL)	+= thermal-generic-adc.o
 obj-$(CONFIG_BCM2835_THERMAL)	+= bcm2835_thermal.o
+obj-$(CONFIG_ZX2967_THERMAL)	+= zx2967_thermal.o
diff --git a/drivers/thermal/zx2967_thermal.c b/drivers/thermal/zx2967_thermal.c
new file mode 100644
index 0000000..1aef070
--- /dev/null
+++ b/drivers/thermal/zx2967_thermal.c
@@ -0,0 +1,241 @@
+/*
+ * ZTE's zx2967 family thermal sensor driver
+ *
+ * Copyright (C) 2017 ZTE Ltd.
+ *
+ * Author: Baoyou Xie <baoyou.xie@linaro.org>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+/* DCF Control Register */
+#define ZX2967_THERMAL_DCF		0x4
+
+/* Selection Register */
+#define ZX2967_THERMAL_SEL		0x8
+
+/* Control Register */
+#define ZX2967_THERMAL_CTRL		0x10
+
+#define ZX2967_THERMAL_ID_MASK		(0x18)
+
+struct zx2967_thermal_sensor {
+	struct zx2967_thermal_priv *priv;
+	struct thermal_zone_device *tzd;
+	int id;
+};
+
+#define NUM_SENSORS	1
+
+struct zx2967_thermal_priv {
+	struct zx2967_thermal_sensor	sensors[NUM_SENSORS];
+	struct mutex			lock;
+	struct clk			*clk_gate;
+	struct clk			*pclk;
+	void __iomem			*regs;
+	struct pinctrl			*pinmux_dvi0_d3;
+	struct pinctrl			*pinmux_dvi0_d4;
+	struct pinctrl			*pinmux_dvi0_d5;
+};
+
+static int zx2967_thermal_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct zx2967_thermal_priv *priv = platform_get_drvdata(pdev);
+
+	if (priv && priv->pclk)
+		clk_disable_unprepare(priv->pclk);
+
+	if (priv && priv->clk_gate)
+		clk_disable_unprepare(priv->clk_gate);
+	dev_info(dev, "suspended\n");
+
+	return 0;
+}
+
+static int zx2967_thermal_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct zx2967_thermal_priv *priv = platform_get_drvdata(pdev);
+	int error;
+
+	error = clk_prepare_enable(priv->clk_gate);
+	if (error)
+		return error;
+
+	error = clk_prepare_enable(priv->pclk);
+	if (error)
+		return error;
+
+	dev_info(dev, "resumed\n");
+
+	return 0;
+}
+
+static int zx2967_thermal_get_temp(void *data, int *temp)
+{
+	void __iomem *regs;
+	struct zx2967_thermal_sensor *sensor = data;
+	struct zx2967_thermal_priv *priv = sensor->priv;
+	unsigned long timeout = jiffies + msecs_to_jiffies(100);
+	u32 val, sel_id;
+
+	regs = priv->regs;
+	mutex_lock(&priv->lock);
+
+	writel_relaxed(0, regs);
+	writel_relaxed(2, regs + ZX2967_THERMAL_DCF);
+
+	val = readl_relaxed(regs + ZX2967_THERMAL_SEL);
+	val &= ~ZX2967_THERMAL_ID_MASK;
+	sel_id = sensor->id ? 8 : 0x10;
+	val |= sel_id;
+	writel_relaxed(val, regs + ZX2967_THERMAL_SEL);
+
+	usleep_range(100, 300);
+	while (!(readl_relaxed(regs + ZX2967_THERMAL_CTRL) & 0x1000)) {
+		if (time_after(jiffies, timeout)) {
+			pr_err("*** Thermal sensor %d data timeout\n",
+			      sensor->id);
+			mutex_unlock(&priv->lock);
+			return -EIO;
+		}
+	}
+
+	writel_relaxed(3, regs + ZX2967_THERMAL_DCF);
+	val = readl_relaxed(regs + ZX2967_THERMAL_CTRL) & 0xfff;
+	writel_relaxed(1, regs);
+
+	/** Calculate temperature */
+	*temp = DIV_ROUND_CLOSEST((val - 922) * 1000, 1951);
+
+	mutex_unlock(&priv->lock);
+
+	return 0;
+}
+
+static struct thermal_zone_of_device_ops zx2967_of_thermal_ops = {
+	.get_temp = zx2967_thermal_get_temp,
+};
+
+static int zx2967_thermal_probe(struct platform_device *pdev)
+{
+	struct zx2967_thermal_priv *priv;
+	struct resource *res;
+	int ret, i;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->regs))
+		return PTR_ERR(priv->regs);
+
+	priv->clk_gate = devm_clk_get(&pdev->dev, "tempsensor_gate");
+	if (IS_ERR(priv->clk_gate)) {
+		ret = PTR_ERR(priv->clk_gate);
+		dev_err(&pdev->dev, "failed to get clock gate: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(priv->clk_gate);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable converter clock: %d\n",
+			ret);
+		return ret;
+	}
+
+	priv->pclk = devm_clk_get(&pdev->dev, "tempsensor_pclk");
+	if (IS_ERR(priv->pclk)) {
+		ret = PTR_ERR(priv->pclk);
+		dev_err(&pdev->dev, "failed to get apb clock: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(priv->pclk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable converter clock: %d\n",
+			ret);
+		return ret;
+	}
+
+	mutex_init(&priv->lock);
+	for (i = 0; i < NUM_SENSORS; i++) {
+		struct zx2967_thermal_sensor *sensor = &priv->sensors[i];
+
+		sensor->priv = priv;
+		sensor->id = i;
+		sensor->tzd = thermal_zone_of_sensor_register(&pdev->dev,
+					i,
+					sensor,
+					&zx2967_of_thermal_ops);
+		if (IS_ERR(sensor->tzd)) {
+			ret = PTR_ERR(sensor->tzd);
+			dev_err(&pdev->dev, "failed to register sensor %d: %d\n",
+				i, ret);
+			goto remove_ts;
+		}
+	}
+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+
+remove_ts:
+	for (i--; i >= 0; i--)
+		thermal_zone_of_sensor_unregister(&pdev->dev,
+						  priv->sensors[i].tzd);
+
+	return ret;
+}
+
+static int zx2967_thermal_exit(struct platform_device *pdev)
+{
+	struct zx2967_thermal_priv *priv = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < NUM_SENSORS; i++) {
+		struct zx2967_thermal_sensor *sensor = &priv->sensors[i];
+
+		thermal_zone_of_sensor_unregister(&pdev->dev, sensor->tzd);
+	}
+	clk_disable_unprepare(priv->pclk);
+	clk_disable_unprepare(priv->clk_gate);
+
+	return 0;
+}
+
+static const struct of_device_id zx2967_thermal_id_table[] = {
+	{ .compatible = "zte,zx2967-thermal" },
+	{ .compatible = "zte,zx296718-thermal" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, zx2967_thermal_id_table);
+
+static SIMPLE_DEV_PM_OPS(zx2967_thermal_pm_ops,
+			 zx2967_thermal_suspend, zx2967_thermal_resume);
+
+static struct platform_driver zx2967_thermal_driver = {
+	.probe = zx2967_thermal_probe,
+	.remove = zx2967_thermal_exit,
+	.driver = {
+		.name = "zx2967_thermal",
+		.of_match_table = zx2967_thermal_id_table,
+		.pm = &zx2967_thermal_pm_ops,
+	},
+};
+module_platform_driver(zx2967_thermal_driver);
+
+MODULE_AUTHOR("Baoyou Xie <baoyou.xie@linaro.org>");
+MODULE_DESCRIPTION("ZTE zx2967 thermal driver");
+MODULE_LICENSE("GPL");
-- 
2.7.4

^ permalink raw reply related

* [PATCH v1 2/3] MAINTAINERS: add zx2967 thermal drivers to ARM ZTE architecture
From: Baoyou Xie @ 2017-01-07  5:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1483767488-19778-1-git-send-email-baoyou.xie@linaro.org>

Add the zx2967 thermal drivers as maintained by ARM ZTE
architecture maintainers, as they're parts of the core IP.

Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 64f04df..2593296 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1981,6 +1981,7 @@ S:	Maintained
 F:	arch/arm/mach-zx/
 F:	drivers/clk/zte/
 F:	drivers/soc/zte/
+F:	drivers/thermal/zx*
 F:	Documentation/devicetree/bindings/arm/zte.txt
 F:	Documentation/devicetree/bindings/clock/zx296702-clk.txt
 F:	Documentation/devicetree/bindings/soc/zte/
-- 
2.7.4

^ permalink raw reply related

* [PATCH v1 1/3] dt: bindings: add thermal device driver for zx2967
From: Baoyou Xie @ 2017-01-07  5:38 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds dt-binding documentation for zx2967 family thermal sensor.

Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
 .../devicetree/bindings/thermal/zx2967-thermal.txt | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/zx2967-thermal.txt

diff --git a/Documentation/devicetree/bindings/thermal/zx2967-thermal.txt b/Documentation/devicetree/bindings/thermal/zx2967-thermal.txt
new file mode 100644
index 0000000..2633964
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/zx2967-thermal.txt
@@ -0,0 +1,22 @@
+* ZTE zx2967 family Thermal
+
+Required Properties:
+- compatible: should be one of the following.
+    * zte,zx2967-thermal
+    * zte,zx296718-thermal
+- reg: physical base address of the controller and length of memory mapped
+    region.
+- clocks : Pairs of phandle and specifier referencing the controller's clocks.
+- clock-names: "tempsensor_gate" for the topcrm clock.
+	       "tempsensor_pclk" for the apb clock.
+- #thermal-sensor-cells: must be 0.
+
+Example:
+
+	tempsensor: tempsensor at 148a000 {
+		compatible = "zte,zx2967-thermal";
+		reg = <0x0148a000 0x20>;
+		clocks = <&topcrm TEMPSENSOR_GATE>, <&audiocrm AUDIO_TS_PCLK>;
+		clock-names = "tempsensor_gate", "tempsensor_pclk";
+		#thermal-sensor-cells = <0>;
+	};
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2] ARM: dts: qcom: apq8064: Add missing scm clock
From: John Stultz @ 2017-01-07  4:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161229120611.7948-1-bjorn.andersson@linaro.org>

On Thu, Dec 29, 2016 at 4:06 AM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> As per the device tree binding the apq8064 scm node requires the core
> clock to be specified, so add this.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v1:
> - Changed clock to Daytona Fabric
>
>  arch/arm/boot/dts/qcom-apq8064.dtsi | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> index 1dbe697b2e90..a27cc96ac069 100644
> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> @@ -4,6 +4,7 @@
>  #include <dt-bindings/clock/qcom,gcc-msm8960.h>
>  #include <dt-bindings/reset/qcom,gcc-msm8960.h>
>  #include <dt-bindings/clock/qcom,mmcc-msm8960.h>
> +#include <dt-bindings/clock/qcom,rpmcc.h>
>  #include <dt-bindings/soc/qcom,gsbi.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
> @@ -303,6 +304,9 @@
>         firmware {
>                 scm {
>                         compatible = "qcom,scm-apq8064";
> +
> +                       clocks = <&rpmcc RPM_DAYTONA_FABRIC_CLK>;
> +                       clock-names = "core";
>                 };
>         };

So using this on my nexus7, I see:

[   14.240169] ------------[ cut here ]------------
[   14.240230] WARNING: CPU: 0 PID: 0 at
drivers/usb/chipidea/udc.c:954 isr_setup_status_phase+0x98/0x9c
[   14.243872] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.10.0-rc2-00024-g4f53a60 #1774
[   14.252975] Hardware name: Generic DT based system
[   14.260810] [<c03113f0>] (unwind_backtrace) from [<c030d678>]
(show_stack+0x20/0x24)
[   14.265493] [<c030d678>] (show_stack) from [<c05cad80>]
(dump_stack+0x80/0x94)
[   14.273385] [<c05cad80>] (dump_stack) from [<c03207dc>] (__warn+0xf0/0x10c)
[   14.280416] [<c03207dc>] (__warn) from [<c03208c8>]
(warn_slowpath_null+0x30/0x38)
[   14.287269] [<c03208c8>] (warn_slowpath_null) from [<c07ce6b4>]
(isr_setup_status_phase+0x98/0x9c)
[   14.294913] [<c07ce6b4>] (isr_setup_status_phase) from [<c07cf11c>]
(udc_irq+0x9f0/0xd0c)
[   14.303856] [<c07cf11c>] (udc_irq) from [<c07ca294>] (ci_irq+0x64/0x118)
[   14.312103] [<c07ca294>] (ci_irq) from [<c03785c0>]
(__handle_irq_event_percpu+0x84/0x2b4)
[   14.318871] [<c03785c0>] (__handle_irq_event_percpu) from
[<c037881c>] (handle_irq_event_percpu+0x2c/0x68)
[   14.326945] [<c037881c>] (handle_irq_event_percpu) from
[<c03788a0>] (handle_irq_event+0x48/0x6c)
[   14.336581] [<c03788a0>] (handle_irq_event) from [<c037c620>]
(handle_fasteoi_irq+0xe0/0x1b0)
[   14.345522] [<c037c620>] (handle_fasteoi_irq) from [<c0377bb8>]
(generic_handle_irq+0x30/0x44)
[   14.354026] [<c0377bb8>] (generic_handle_irq) from [<c0377c58>]
(__handle_domain_irq+0x8c/0xfc)
[   14.362535] [<c0377c58>] (__handle_domain_irq) from [<c03014dc>]
(gic_handle_irq+0x58/0x9c)
[   14.371125] [<c03014dc>] (gic_handle_irq) from [<c030e28c>]
(__irq_svc+0x6c/0xa8)
[   14.379450] Exception stack(0xc1001ee8 to 0xc1001f30)
[   14.387102] 1ee0:                   00000001 00000000 00000000
c031b240 c1000000 c10050c0
[   14.392145] 1f00: c100506c c0f92ea8 c1001f58 00000000 00000000
c1001f44 c1001f48 c1001f38
[   14.400295] 1f20: c03097d4 c03097d8 60000113 ffffffff
[   14.408457] [<c030e28c>] (__irq_svc) from [<c03097d8>]
(arch_cpu_idle+0x48/0x4c)
[   14.413497] [<c03097d8>] (arch_cpu_idle) from [<c0b2debc>]
(default_idle_call+0x30/0x3c)
[   14.420959] [<c0b2debc>] (default_idle_call) from [<c036b390>]
(do_idle+0x17c/0x210)
[   14.429027] [<c036b390>] (do_idle) from [<c036b710>]
(cpu_startup_entry+0x28/0x2c)
[   14.436756] [<c036b710>] (cpu_startup_entry) from [<c0b2647c>]
(rest_init+0x94/0x98)
[   14.444130] [<c0b2647c>] (rest_init) from [<c0f00e08>]
(start_kernel+0x390/0x39c)
[   14.452022] ---[ end trace cc56495fca556bcb ]---


And then usb doesn't seem to work...

thanks
-john

^ permalink raw reply

* [PATCH 1/5] ARM: dts: qcom: apq8064: Add missing scm clock
From: Andy Gross @ 2017-01-07  3:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CALAqxLVGMzPmRBM5SPO=pQjuasf0f_aRJ+94FUHtidYYU7yhpA@mail.gmail.com>

On Fri, Jan 06, 2017 at 05:10:44PM -0800, John Stultz wrote:
> On Wed, Dec 21, 2016 at 3:49 AM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > As per the device tree binding the apq8064 scm node requires the core
> > clock to be specified, so add this.
> >
> > Cc: John Stultz <john.stultz@linaro.org>
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >  arch/arm/boot/dts/qcom-apq8064.dtsi | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> > index 268bd470c865..78bf155a52f3 100644
> > --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> > +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> > @@ -303,6 +303,9 @@
> >         firmware {
> >                 scm {
> >                         compatible = "qcom,scm-apq8064";
> > +
> > +                       clocks = <&gcc CE3_CORE_CLK>;
> > +                       clock-names = "core";
> 
> 
> Tested-by: John Stultz <john.stultz@linaro.org>
> 
> I know Bjorn has a new version of this patch that uses the
> RPM_DAYTONA_FABRIC_CLK value, but that one results in problems with
> usb gadget functionality on my Nexus7.  This one seems to work ok
> though.

Odd.  Is the usb gadget using the daytona but not getting a reference?  I wonder
if this is related to not having the bus driver running the bus clk enablement
and frequencies.


Andy

^ permalink raw reply

* [PATCH v6 3/4] arm64: arch_timer: Work around Erratum Hisilicon-161601
From: Ding Tianhong @ 2017-01-07  2:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e57fb582-5396-8a39-95c3-95e63e3f58e6@arm.com>



On 2017/1/6 22:49, Marc Zyngier wrote:
> On 05/01/17 05:31, Ding Tianhong wrote:
>> Erratum Hisilicon-161601 says that the ARM generic timer counter "has the
>> potential to contain an erroneous value when the timer value changes".
>> Accesses to TVAL (both read and write) are also affected due to the implicit counter
>> read.  Accesses to CVAL are not affected.
>>
>> The workaround is to reread the system count registers until the value of the second
>> read is larger than the first one by less than 32, the system counter can be guaranteed
>> not to return wrong value twice by back-to-back read and the error value is always larger
>> than the correct one by 32. Writes to TVAL are replaced with an equivalent write to CVAL.
>>
>> The workaround is enabled if the hisilicon,erratum-161601 property is found in
>> the timer node in the device tree. This can be overridden with the
>> clocksource.arm_arch_timer.hisilicon-161601 boot parameter, which allows KVM
>> users to enable the workaround until a mechanism is implemented to
>> automatically communicate this information.
>>
>> Fix some description for fsl erratum a008585.
>>
>> v2: Significant rework based on feedback, including seperate the fsl erratum a008585
>>     to another patch, update the erratum name and remove unwanted code.
>>
>> v3: Significant rework based on feedback, including fix some alignment problem, make the
>>     #define __hisi_161601_read_reg to be private to the .c file instead of being globally
>>     visible, add more accurate annotation and modify a bit of logical format to enable
>>     arch_timer_read_ool_enabled, remove the kernel commandline parameter
>>     clocksource.arm_arch_timer.hisilicon-161601.
>>
>> v5: Theoretically the erratum should not occur more than twice in succession when reading
>>     the system counter, but it is possible that some interrupts may lead to more than twice
>>     read errors, triggering the warning, so setting the number of retries to 50 which is far
>>     beyond the number of iterations the loop has been observed to take.
>>
>> v6: Mark the hisi_161601_read_xxx_el0 with notrace, if CONFIG_FUNCTION_GRAPH_TRACER selected,
>>     hisi_161601_read_xxx_el0 will be related to ftrace_graph_caller which will calling sched_clock
>>     to read system counter again, it will cause the system stall into an endless loop.
> 
> Please move the changelog to the cover letter, as I don't need
> any of this to end-up in the commit log.
> 

OK.


>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>>  Documentation/arm64/silicon-errata.txt |  1 +
>>  arch/arm64/include/asm/arch_timer.h    |  2 +-
>>  drivers/clocksource/Kconfig            |  9 +++++
>>  drivers/clocksource/arm_arch_timer.c   | 70 +++++++++++++++++++++++++++++++---
>>  4 files changed, 76 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
>> index 405da11..1c1a95f 100644
>> --- a/Documentation/arm64/silicon-errata.txt
>> +++ b/Documentation/arm64/silicon-errata.txt
>> @@ -63,3 +63,4 @@ stable kernels.
>>  | Cavium         | ThunderX SMMUv2 | #27704          | N/A		       |
>>  |                |                 |                 |                         |
>>  | Freescale/NXP  | LS2080A/LS1043A | A-008585        | FSL_ERRATUM_A008585     |
>> +| Hisilicon      | Hip0{5,6,7}     | #161601         | HISILICON_ERRATUM_161601|
>> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
>> index f882c7c..ebf4cde 100644
>> --- a/arch/arm64/include/asm/arch_timer.h
>> +++ b/arch/arm64/include/asm/arch_timer.h
>> @@ -29,7 +29,7 @@
>>  
>>  #include <clocksource/arm_arch_timer.h>
>>  
>> -#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585)
>> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>>  extern struct static_key_false arch_timer_read_ool_enabled;
>>  #define needs_unstable_timer_counter_workaround() \
>>  	static_branch_unlikely(&arch_timer_read_ool_enabled)
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index 4866f7a..162d820 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -335,6 +335,15 @@ config FSL_ERRATUM_A008585
>>  	  value").  The workaround will only be active if the
>>  	  fsl,erratum-a008585 property is found in the timer node.
>>  
>> +config HISILICON_ERRATUM_161601
>> +	bool "Workaround for Hisilicon Erratum 161601"
>> +	default y
>> +	depends on ARM_ARCH_TIMER && ARM64
>> +	help
>> +	  This option enables a workaround for Hisilicon Erratum
>> +	  161601. The workaround will be active if the hisilicon,erratum-161601
>> +	  property is found in the timer node.
>> +
>>  config ARM_GLOBAL_TIMER
>>  	bool "Support for the ARM global timer" if COMPILE_TEST
>>  	select CLKSRC_OF if OF
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
>> index f9097b6..078d38f 100644
>> --- a/drivers/clocksource/arm_arch_timer.c
>> +++ b/drivers/clocksource/arm_arch_timer.c
>> @@ -95,15 +95,18 @@ static int __init early_evtstrm_cfg(char *buf)
>>   * Architected system timer support.
>>   */
>>  
>> -#ifdef CONFIG_FSL_ERRATUM_A008585
>> +#if CONFIG_FSL_ERRATUM_A008585 || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
> 
> This line looks horrible. it should probably be IS_ENABLED(CONFIG_FSL).
> But more importantly, given that at least two independent SoC
> manufacturers have managed to screw their timers in a similar way,
> I'd expect that list to grow.
> 
> So please introduce a new config symbol (for example something like
> CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND) which gets selected by individual
> workarounds, and use that everywhere where you have both symbols.
> 

Fine, will introduce a new general config.

>>  struct arch_timer_erratum_workaround *timer_unstable_counter_workaround = NULL;
>>  EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
>>  
>>  #define        FSL_A008585	0x0001
>> +#define        HISILICON_161601	0x0002
>>  
>>  DEFINE_STATIC_KEY_FALSE(arch_timer_read_ool_enabled);
>>  EXPORT_SYMBOL_GPL(arch_timer_read_ool_enabled);
>> +#endif
>>  
>> +#ifdef CONFIG_FSL_ERRATUM_A008585
>>  /*
>>   * The number of retries is an arbitrary value well beyond the highest number
>>   * of iterations the loop has been observed to take.
>> @@ -145,6 +148,54 @@ static u64 notrace fsl_a008585_read_cntvct_el0(void)
>>  };
>>  #endif /* CONFIG_FSL_ERRATUM_A008585 */
>>  
>> +#ifdef CONFIG_HISILICON_ERRATUM_161601
>> +/*
>> + * Verify whether the value of the second read is larger than the first by
>> + * less than 32 is the only way to confirm the value is correct, so clear the
>> + * lower 5 bits to check whether the difference is greater than 32 or not.
>> + * Theoretically the erratum should not occur more than twice in succession
>> + * when reading the system counter, but it is possible that some interrupts
>> + * may lead to more than twice read errors, triggering the warning, so setting
>> + * the number of retries far beyond the number of iterations the loop has been
>> + * observed to take.
>> + */
>> +#define __hisi_161601_read_reg(reg) ({				\
>> +	u64 _old, _new;						\
>> +	int _retries = 50;					\
>> +								\
>> +	do {							\
>> +		_old = read_sysreg(reg);			\
>> +		_new = read_sysreg(reg);			\
>> +		_retries--;					\
>> +	} while (unlikely((_new - _old) >> 5) && _retries);	\
>> +								\
>> +	WARN_ON_ONCE(!_retries);				\
>> +	_new;							\
>> +})
>> +
>> +static u32 notrace hisi_161601_read_cntp_tval_el0(void)
>> +{
>> +	return __hisi_161601_read_reg(cntp_tval_el0);
>> +}
>> +
>> +static u32 notrace hisi_161601_read_cntv_tval_el0(void)
>> +{
>> +	return __hisi_161601_read_reg(cntv_tval_el0);
>> +}
>> +
>> +static u64 notrace hisi_161601_read_cntvct_el0(void)
>> +{
>> +	return __hisi_161601_read_reg(cntvct_el0);
>> +}
>> +
>> +static struct arch_timer_erratum_workaround arch_timer_hisi_161601 = {
>> +	.erratum = HISILICON_161601,
>> +	.read_cntp_tval_el0 = hisi_161601_read_cntp_tval_el0,
>> +	.read_cntv_tval_el0 = hisi_161601_read_cntv_tval_el0,
>> +	.read_cntvct_el0 = hisi_161601_read_cntvct_el0,
>> +};
>> +#endif /* CONFIG_HISILICON_ERRATUM_161601 */
>> +
>>  static __always_inline
>>  void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val,
>>  			  struct clock_event_device *clk)
>> @@ -294,7 +345,7 @@ static __always_inline void set_next_event(const int access, unsigned long evt,
>>  	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
>>  }
>>  
>> -#ifdef CONFIG_FSL_ERRATUM_A008585
>> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>>  static __always_inline void erratum_set_next_event_generic(const int access,
>>  		unsigned long evt, struct clock_event_device *clk)
>>  {
>> @@ -358,7 +409,7 @@ static int arch_timer_set_next_event_phys_mem(unsigned long evt,
>>  
>>  static void erratum_workaround_set_sne(struct clock_event_device *clk)
>>  {
>> -#ifdef CONFIG_FSL_ERRATUM_A008585
>> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>>  	if (!static_branch_unlikely(&arch_timer_read_ool_enabled))
>>  		return;
>>  
>> @@ -618,7 +669,7 @@ static void __init arch_counter_register(unsigned type)
>>  
>>  		clocksource_counter.archdata.vdso_direct = true;
>>  
>> -#ifdef CONFIG_FSL_ERRATUM_A008585
>> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>>  		/*
>>  		 * Don't use the vdso fastpath if errata require using
>>  		 * the out-of-line counter accessor.
>> @@ -909,10 +960,19 @@ static int __init arch_timer_of_init(struct device_node *np)
>>  #ifdef CONFIG_FSL_ERRATUM_A008585
>>  	if (!timer_unstable_counter_workaround && of_property_read_bool(np, "fsl,erratum-a008585"))
>>  		timer_unstable_counter_workaround = &arch_timer_fsl_a008585;
>> +#endif
>> +
>> +#ifdef CONFIG_HISILICON_ERRATUM_161601
>> +	if (!timer_unstable_counter_workaround && of_property_read_bool(np, "hisilicon,erratum-161601"))
>> +		timer_unstable_counter_workaround = &arch_timer_hisi_161601;
>> +#endif
>>  
>> +#if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
>>  	if (timer_unstable_counter_workaround) {
>>  		static_branch_enable(&arch_timer_read_ool_enabled);
>> -		pr_info("Enabling workaround for FSL erratum A-008585\n");
>> +		pr_info("Enabling workaround for %s\n",
>> +			timer_unstable_counter_workaround->erratum == FSL_A008585 ?
>> +			"FSL ERRATUM A-008585" : "HISILICON ERRATUM 161601");
>>  	}
>>  #endif
> 
> This looks extremely messy, and maybe it is time you properly refactor
> the whole thing so that we can scale. I came up with the following
> approach, which seems more manageable:
> 
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index ebf4cde..1f89d94 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -37,15 +37,14 @@ extern struct static_key_false arch_timer_read_ool_enabled;
>  #define needs_unstable_timer_counter_workaround()  false
>  #endif
>  
> -
>  struct arch_timer_erratum_workaround {
> -	int erratum;		/* Indicate the Erratum ID */
> +	const char *id;
>  	u32 (*read_cntp_tval_el0)(void);
>  	u32 (*read_cntv_tval_el0)(void);
>  	u64 (*read_cntvct_el0)(void);
>  };
>  
> -extern struct arch_timer_erratum_workaround *timer_unstable_counter_workaround;
> +extern const struct arch_timer_erratum_workaround *timer_unstable_counter_workaround;
>  
>  #define arch_timer_reg_read_stable(reg) 		\
>  ({							\
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index c069f1a..0dd80e6 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -96,12 +96,9 @@ early_param("clocksource.arm_arch_timer.evtstrm", early_evtstrm_cfg);
>   */
>  
>  #if CONFIG_FSL_ERRATUM_A008585 || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
> -struct arch_timer_erratum_workaround *timer_unstable_counter_workaround = NULL;
> +const struct arch_timer_erratum_workaround *timer_unstable_counter_workaround;
>  EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
>  
> -#define        FSL_A008585	0x0001
> -#define        HISILICON_161601	0x0002
> -
>  DEFINE_STATIC_KEY_FALSE(arch_timer_read_ool_enabled);
>  EXPORT_SYMBOL_GPL(arch_timer_read_ool_enabled);
>  #endif
> @@ -139,13 +136,6 @@ static u64 notrace fsl_a008585_read_cntvct_el0(void)
>  {
>  	return __fsl_a008585_read_reg(cntvct_el0);
>  }
> -
> -static struct arch_timer_erratum_workaround arch_timer_fsl_a008585 = {
> -	.erratum = FSL_A008585,
> -	.read_cntp_tval_el0 = fsl_a008585_read_cntp_tval_el0,
> -	.read_cntv_tval_el0 = fsl_a008585_read_cntv_tval_el0,
> -	.read_cntvct_el0 = fsl_a008585_read_cntvct_el0,
> -};
>  #endif /* CONFIG_FSL_ERRATUM_A008585 */
>  
>  #ifdef CONFIG_HISILICON_ERRATUM_161601
> @@ -187,13 +177,6 @@ static u64 notrace hisi_161601_read_cntvct_el0(void)
>  {
>  	return __hisi_161601_read_reg(cntvct_el0);
>  }
> -
> -static struct arch_timer_erratum_workaround arch_timer_hisi_161601 = {
> -	.erratum = HISILICON_161601,
> -	.read_cntp_tval_el0 = hisi_161601_read_cntp_tval_el0,
> -	.read_cntv_tval_el0 = hisi_161601_read_cntv_tval_el0,
> -	.read_cntvct_el0 = hisi_161601_read_cntvct_el0,
> -};
>  #endif /* CONFIG_HISILICON_ERRATUM_161601 */
>  
>  static __always_inline
> @@ -346,6 +329,25 @@ static __always_inline void set_next_event(const int access, unsigned long evt,
>  }
>  
>  #if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
> +static const struct arch_timer_erratum_workaround ool_workarounds[] = {
> +#ifdef CONFIG_FSL_ERRATUM_A008585
> +	{
> +		.id = "fsl,erratum-a008585",
> +		.read_cntp_tval_el0 = fsl_a008585_read_cntp_tval_el0,
> +		.read_cntv_tval_el0 = fsl_a008585_read_cntv_tval_el0,
> +		.read_cntvct_el0 = fsl_a008585_read_cntvct_el0,
> +	},
> +#endif
> +#ifdef CONFIG_HISILICON_ERRATUM_161601
> +	{
> +		.id = "hisilicon,erratum-161601",
> +		.read_cntp_tval_el0 = hisi_161601_read_cntp_tval_el0,
> +		.read_cntv_tval_el0 = hisi_161601_read_cntv_tval_el0,
> +		.read_cntvct_el0 = hisi_161601_read_cntvct_el0,
> +	},
> +#endif
> +};
> +
>  static __always_inline void erratum_set_next_event_generic(const int access,
>  		unsigned long evt, struct clock_event_device *clk)
>  {
> @@ -957,22 +959,15 @@ static int __init arch_timer_of_init(struct device_node *np)
>  
>  	arch_timer_c3stop = !of_property_read_bool(np, "always-on");
>  
> -#ifdef CONFIG_FSL_ERRATUM_A008585
> -	if (!timer_unstable_counter_workaround && of_property_read_bool(np, "fsl,erratum-a008585"))
> -		timer_unstable_counter_workaround = &arch_timer_fsl_a008585;
> -#endif
> -
> -#ifdef CONFIG_HISILICON_ERRATUM_161601
> -	if (!timer_unstable_counter_workaround && of_property_read_bool(np, "hisilicon,erratum-161601"))
> -		timer_unstable_counter_workaround = &arch_timer_hisi_161601;
> -#endif
> -
>  #if IS_ENABLED(CONFIG_FSL_ERRATUM_A008585) || IS_ENABLED(CONFIG_HISILICON_ERRATUM_161601)
> -	if (timer_unstable_counter_workaround) {
> -		static_branch_enable(&arch_timer_read_ool_enabled);
> -		pr_info("Enabling workaround for %s\n",
> -			timer_unstable_counter_workaround->erratum == FSL_A008585 ?
> -			"FSL ERRATUM A-008585" : "HISILICON ERRATUM 161601");
> +	for (i = 0; i < ARRAY_SIZE(ool_workarounds); i++) {
> +		if (of_property_read_bool(np, ool_workarounds[i].id)) {
> +			timer_unstable_counter_workaround = &ool_workarounds[i];
> +			static_branch_enable(&arch_timer_read_ool_enabled);
> +			pr_info("Enabling workaround %s\n",
> +				timer_unstable_counter_workaround->id);
> +			break;
> +		}
>  	}
>  #endif
>  
this changes looks good to me, I will test it and release a new version, thanks a lot.

Ding
> 
> Thoughts?
> 
> 	M.
> 

^ permalink raw reply


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