* Re: [PATCH 09/11] hwmon: Drop obsolete JZ4740 driver
From: Guenter Roeck @ 2019-07-25 22:16 UTC (permalink / raw)
To: Paul Cercueil
Cc: Ralf Baechle, Paul Burton, James Hogan, Rob Herring, Mark Rutland,
Vinod Koul, Jean Delvare, Lee Jones, Miquel Raynal,
Richard Weinberger, Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Liam Girdwood, Mark Brown, od, devicetree, linux-mips,
linux-kernel, dmaengine, linux-hwmon, linux-mtd, linux-pm,
dri-devel
In-Reply-To: <20190725220215.460-10-paul@crapouillou.net>
On Thu, Jul 25, 2019 at 06:02:13PM -0400, Paul Cercueil wrote:
> The JZ4740 boards now use the iio-hwmon driver.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Tested-by: Artur Rojek <contact@artur-rojek.eu>
Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/hwmon/Kconfig | 10 ---
> drivers/hwmon/Makefile | 1 -
> drivers/hwmon/jz4740-hwmon.c | 135 -----------------------------------
> 3 files changed, 146 deletions(-)
> delete mode 100644 drivers/hwmon/jz4740-hwmon.c
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 650dd71f9724..2199ac1d0ba7 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -660,16 +660,6 @@ config SENSORS_IT87
> This driver can also be built as a module. If so, the module
> will be called it87.
>
> -config SENSORS_JZ4740
> - tristate "Ingenic JZ4740 SoC ADC driver"
> - depends on MACH_JZ4740 && MFD_JZ4740_ADC
> - help
> - If you say yes here you get support for reading adc values from the ADCIN
> - pin on Ingenic JZ4740 SoC based boards.
> -
> - This driver can also be built as a module. If so, the module will be
> - called jz4740-hwmon.
> -
> config SENSORS_JC42
> tristate "JEDEC JC42.4 compliant memory module temperature sensors"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 8db472ea04f0..1e82e912a5c4 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -85,7 +85,6 @@ obj-$(CONFIG_SENSORS_INA2XX) += ina2xx.o
> obj-$(CONFIG_SENSORS_INA3221) += ina3221.o
> obj-$(CONFIG_SENSORS_IT87) += it87.o
> obj-$(CONFIG_SENSORS_JC42) += jc42.o
> -obj-$(CONFIG_SENSORS_JZ4740) += jz4740-hwmon.o
> obj-$(CONFIG_SENSORS_K8TEMP) += k8temp.o
> obj-$(CONFIG_SENSORS_K10TEMP) += k10temp.o
> obj-$(CONFIG_SENSORS_LINEAGE) += lineage-pem.o
> diff --git a/drivers/hwmon/jz4740-hwmon.c b/drivers/hwmon/jz4740-hwmon.c
> deleted file mode 100644
> index bec5befd1d8b..000000000000
> --- a/drivers/hwmon/jz4740-hwmon.c
> +++ /dev/null
> @@ -1,135 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-or-later
> -/*
> - * Copyright (C) 2009-2010, Lars-Peter Clausen <lars@metafoo.de>
> - * JZ4740 SoC HWMON driver
> - */
> -
> -#include <linux/err.h>
> -#include <linux/interrupt.h>
> -#include <linux/kernel.h>
> -#include <linux/module.h>
> -#include <linux/mutex.h>
> -#include <linux/platform_device.h>
> -#include <linux/slab.h>
> -#include <linux/io.h>
> -
> -#include <linux/completion.h>
> -#include <linux/mfd/core.h>
> -
> -#include <linux/hwmon.h>
> -
> -struct jz4740_hwmon {
> - void __iomem *base;
> - int irq;
> - const struct mfd_cell *cell;
> - struct platform_device *pdev;
> - struct completion read_completion;
> - struct mutex lock;
> -};
> -
> -static irqreturn_t jz4740_hwmon_irq(int irq, void *data)
> -{
> - struct jz4740_hwmon *hwmon = data;
> -
> - complete(&hwmon->read_completion);
> - return IRQ_HANDLED;
> -}
> -
> -static ssize_t in0_input_show(struct device *dev,
> - struct device_attribute *dev_attr, char *buf)
> -{
> - struct jz4740_hwmon *hwmon = dev_get_drvdata(dev);
> - struct platform_device *pdev = hwmon->pdev;
> - struct completion *completion = &hwmon->read_completion;
> - long t;
> - unsigned long val;
> - int ret;
> -
> - mutex_lock(&hwmon->lock);
> -
> - reinit_completion(completion);
> -
> - enable_irq(hwmon->irq);
> - hwmon->cell->enable(pdev);
> -
> - t = wait_for_completion_interruptible_timeout(completion, HZ);
> -
> - if (t > 0) {
> - val = readw(hwmon->base) & 0xfff;
> - val = (val * 3300) >> 12;
> - ret = sprintf(buf, "%lu\n", val);
> - } else {
> - ret = t ? t : -ETIMEDOUT;
> - }
> -
> - hwmon->cell->disable(pdev);
> - disable_irq(hwmon->irq);
> -
> - mutex_unlock(&hwmon->lock);
> -
> - return ret;
> -}
> -
> -static DEVICE_ATTR_RO(in0_input);
> -
> -static struct attribute *jz4740_attrs[] = {
> - &dev_attr_in0_input.attr,
> - NULL
> -};
> -
> -ATTRIBUTE_GROUPS(jz4740);
> -
> -static int jz4740_hwmon_probe(struct platform_device *pdev)
> -{
> - int ret;
> - struct device *dev = &pdev->dev;
> - struct jz4740_hwmon *hwmon;
> - struct device *hwmon_dev;
> -
> - hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL);
> - if (!hwmon)
> - return -ENOMEM;
> -
> - hwmon->cell = mfd_get_cell(pdev);
> -
> - hwmon->irq = platform_get_irq(pdev, 0);
> - if (hwmon->irq < 0) {
> - dev_err(&pdev->dev, "Failed to get platform irq: %d\n",
> - hwmon->irq);
> - return hwmon->irq;
> - }
> -
> - hwmon->base = devm_platform_ioremap_resource(pdev, 0);
> - if (IS_ERR(hwmon->base))
> - return PTR_ERR(hwmon->base);
> -
> - hwmon->pdev = pdev;
> - init_completion(&hwmon->read_completion);
> - mutex_init(&hwmon->lock);
> -
> - ret = devm_request_irq(dev, hwmon->irq, jz4740_hwmon_irq, 0,
> - pdev->name, hwmon);
> - if (ret) {
> - dev_err(&pdev->dev, "Failed to request irq: %d\n", ret);
> - return ret;
> - }
> - disable_irq(hwmon->irq);
> -
> - hwmon_dev = devm_hwmon_device_register_with_groups(dev, "jz4740", hwmon,
> - jz4740_groups);
> - return PTR_ERR_OR_ZERO(hwmon_dev);
> -}
> -
> -static struct platform_driver jz4740_hwmon_driver = {
> - .probe = jz4740_hwmon_probe,
> - .driver = {
> - .name = "jz4740-hwmon",
> - },
> -};
> -
> -module_platform_driver(jz4740_hwmon_driver);
> -
> -MODULE_DESCRIPTION("JZ4740 SoC HWMON driver");
> -MODULE_AUTHOR("Lars-Peter Clausen <lars@metafoo.de>");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:jz4740-hwmon");
> --
> 2.21.0.593.g511ec345e18
>
^ permalink raw reply
* [PATCH 00/15] thermal: qcom: tsens: Add interrupt support
From: Amit Kucheria @ 2019-07-25 22:18 UTC (permalink / raw)
To: linux-kernel, linux-arm-msm, bjorn.andersson, edubezval,
andy.gross, Andy Gross, Daniel Lezcano, Mark Rutland, Rob Herring,
Zhang Rui
Cc: marc.w.gonzalez, masneyb, devicetree, linux-pm
Add interrupt support to TSENS. The first 6 patches are general fixes and
cleanups to the driver before interrupt support is introduced.
This series has been developed against qcs404 and sdm845 and then tested on
msm8916. Testing on msm8998 and msm8974 would be appreciated since I don't
have hardware handy. Further, I plan to test on msm8996 and also submit to
kernelci.
I'm sending this out for more review to get help with testing.
Amit Kucheria (15):
drivers: thermal: tsens: Get rid of id field in tsens_sensor
drivers: thermal: tsens: Simplify code flow in tsens_probe
drivers: thermal: tsens: Add __func__ identifier to debug statements
drivers: thermal: tsens: Add debugfs support
arm: dts: msm8974: thermal: Add thermal zones for each sensor
arm64: dts: msm8916: thermal: Fixup HW ids for cpu sensors
dt: thermal: tsens: Document interrupt support in tsens driver
arm64: dts: sdm845: thermal: Add interrupt support
arm64: dts: msm8996: thermal: Add interrupt support
arm64: dts: msm8998: thermal: Add interrupt support
arm64: dts: qcs404: thermal: Add interrupt support
arm64: dts: msm8974: thermal: Add interrupt support
arm64: dts: msm8916: thermal: Add interrupt support
drivers: thermal: tsens: Create function to return sign-extended
temperature
drivers: thermal: tsens: Add interrupt support
.../bindings/thermal/qcom-tsens.txt | 5 +
arch/arm/boot/dts/qcom-msm8974.dtsi | 108 +++-
arch/arm64/boot/dts/qcom/msm8916.dtsi | 26 +-
arch/arm64/boot/dts/qcom/msm8996.dtsi | 60 +-
arch/arm64/boot/dts/qcom/msm8998.dtsi | 82 +--
arch/arm64/boot/dts/qcom/qcs404.dtsi | 42 +-
arch/arm64/boot/dts/qcom/sdm845.dtsi | 88 +--
drivers/thermal/qcom/tsens-8960.c | 4 +-
drivers/thermal/qcom/tsens-common.c | 610 +++++++++++++++++-
drivers/thermal/qcom/tsens-v0_1.c | 11 +
drivers/thermal/qcom/tsens-v1.c | 29 +
drivers/thermal/qcom/tsens-v2.c | 18 +
drivers/thermal/qcom/tsens.c | 52 +-
drivers/thermal/qcom/tsens.h | 285 +++++++-
14 files changed, 1214 insertions(+), 206 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH 05/15] arm: dts: msm8974: thermal: Add thermal zones for each sensor
From: Amit Kucheria @ 2019-07-25 22:18 UTC (permalink / raw)
To: linux-kernel, linux-arm-msm, bjorn.andersson, edubezval,
andy.gross, Andy Gross, Daniel Lezcano, Mark Rutland, Rob Herring,
Zhang Rui
Cc: masneyb, devicetree
In-Reply-To: <cover.1564091601.git.amit.kucheria@linaro.org>
msm8974 has 11 sensors connected to a single TSENS IP. Define a thermal
zone for each of those sensors to expose the temperature of each zone.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
Cc: masneyb@onstation.org
arch/arm/boot/dts/qcom-msm8974.dtsi | 90 +++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 369e58f64145..d32f639505f1 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -217,6 +217,96 @@
};
};
};
+
+ q6-dsp-thermal {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 1>;
+
+ trips {
+ q6_dsp_alert0: trip-point0 {
+ temperature = <90000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+ };
+ };
+
+ modemtx-thermal {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 2>;
+
+ trips {
+ modemtx_alert0: trip-point0 {
+ temperature = <90000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+ };
+ };
+
+ video-thermal {
+ polling-delay-passive = <0>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 3>;
+
+ trips {
+ video_alert0: trip-point0 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+ };
+ };
+
+ wlan-thermal {
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
+
+ thermal-sensors = <&tsens 4>;
+
+ trips {
+ wlan_alert0: trip-point0 {
+ temperature = <105000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+ };
+ };
+
+ gpu-thermal-top {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 9>;
+
+ trips {
+ gpu1_alert0: trip-point0 {
+ temperature = <90000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+ };
+ };
+
+ gpu-thermal-bottom {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 10>;
+
+ trips {
+ gpu2_alert0: trip-point0 {
+ temperature = <90000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+ };
+ };
};
cpu-pmu {
--
2.17.1
^ permalink raw reply related
* [PATCH 06/15] arm64: dts: msm8916: thermal: Fixup HW ids for cpu sensors
From: Amit Kucheria @ 2019-07-25 22:18 UTC (permalink / raw)
To: linux-kernel, linux-arm-msm, bjorn.andersson, edubezval,
andy.gross, Andy Gross, Daniel Lezcano, Mark Rutland, Rob Herring,
Zhang Rui
Cc: devicetree
In-Reply-To: <cover.1564091601.git.amit.kucheria@linaro.org>
msm8916 uses sensors 0, 1, 2, 4 and 5. Sensor 3 is NOT used. Fixup the
device tree so that the correct sensor ID is used and as a result we can
actually check the temperature for the cpu2_3 sensor.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
arch/arm64/boot/dts/qcom/msm8916.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 5ea9fb8f2f87..8686e101905c 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -179,7 +179,7 @@
polling-delay-passive = <250>;
polling-delay = <1000>;
- thermal-sensors = <&tsens 4>;
+ thermal-sensors = <&tsens 5>;
trips {
cpu0_1_alert0: trip-point@0 {
@@ -209,7 +209,7 @@
polling-delay-passive = <250>;
polling-delay = <1000>;
- thermal-sensors = <&tsens 3>;
+ thermal-sensors = <&tsens 4>;
trips {
cpu2_3_alert0: trip-point@0 {
--
2.17.1
^ permalink raw reply related
* [PATCH 07/15] dt: thermal: tsens: Document interrupt support in tsens driver
From: Amit Kucheria @ 2019-07-25 22:18 UTC (permalink / raw)
To: linux-kernel, linux-arm-msm, bjorn.andersson, edubezval,
andy.gross, Andy Gross, Daniel Lezcano, Mark Rutland, Rob Herring,
Zhang Rui
Cc: linux-pm, devicetree
In-Reply-To: <cover.1564091601.git.amit.kucheria@linaro.org>
Define two new required properties to define interrupts and
interrupt-names for tsens.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
index 673cc1831ee9..3d3dd5dc6d36 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
@@ -22,6 +22,8 @@ Required properties:
- #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
- #qcom,sensors: Number of sensors in tsens block
+- interrupts: Interrupts generated from Always-On subsystem (AOSS)
+- interrupt-names: The name of the interrupt e.g. "tsens0", "tsens1"
- Refer to Documentation/devicetree/bindings/nvmem/nvmem.txt to know how to specify
nvmem cells
@@ -40,6 +42,9 @@ tsens0: thermal-sensor@c263000 {
reg = <0xc263000 0x1ff>, /* TM */
<0xc222000 0x1ff>; /* SROT */
#qcom,sensors = <13>;
+ interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "tsens0";
+
#thermal-sensor-cells = <1>;
};
--
2.17.1
^ permalink raw reply related
* [PATCH 08/15] arm64: dts: sdm845: thermal: Add interrupt support
From: Amit Kucheria @ 2019-07-25 22:18 UTC (permalink / raw)
To: linux-kernel, linux-arm-msm, bjorn.andersson, edubezval,
andy.gross, Andy Gross, Daniel Lezcano, Mark Rutland, Rob Herring,
Zhang Rui
Cc: devicetree
In-Reply-To: <cover.1564091601.git.amit.kucheria@linaro.org>
Register upper-lower interrupts for each of the two tsens controllers.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 88 +++++++++++++++-------------
1 file changed, 46 insertions(+), 42 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 4babff5f19b5..43c06e54b69d 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2386,6 +2386,8 @@
reg = <0 0x0c263000 0 0x1ff>, /* TM */
<0 0x0c222000 0 0x1ff>; /* SROT */
#qcom,sensors = <13>;
+ interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "tsens0";
#thermal-sensor-cells = <1>;
};
@@ -2394,6 +2396,8 @@
reg = <0 0x0c265000 0 0x1ff>, /* TM */
<0 0x0c223000 0 0x1ff>; /* SROT */
#qcom,sensors = <8>;
+ interrupts = <GIC_SPI 507 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "tsens1";
#thermal-sensor-cells = <1>;
};
@@ -2712,8 +2716,8 @@
thermal-zones {
cpu0-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 1>;
@@ -2756,8 +2760,8 @@
};
cpu1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 2>;
@@ -2800,8 +2804,8 @@
};
cpu2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 3>;
@@ -2844,8 +2848,8 @@
};
cpu3-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 4>;
@@ -2888,8 +2892,8 @@
};
cpu4-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 7>;
@@ -2932,8 +2936,8 @@
};
cpu5-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 8>;
@@ -2976,8 +2980,8 @@
};
cpu6-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 9>;
@@ -3020,8 +3024,8 @@
};
cpu7-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 10>;
@@ -3064,8 +3068,8 @@
};
aoss0-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 0>;
@@ -3079,8 +3083,8 @@
};
cluster0-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 5>;
@@ -3099,8 +3103,8 @@
};
cluster1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 6>;
@@ -3119,8 +3123,8 @@
};
gpu-thermal-top {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 11>;
@@ -3134,8 +3138,8 @@
};
gpu-thermal-bottom {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 12>;
@@ -3149,8 +3153,8 @@
};
aoss1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 0>;
@@ -3164,8 +3168,8 @@
};
q6-modem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 1>;
@@ -3179,8 +3183,8 @@
};
mem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 2>;
@@ -3194,8 +3198,8 @@
};
wlan-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 3>;
@@ -3209,8 +3213,8 @@
};
q6-hvx-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 4>;
@@ -3224,8 +3228,8 @@
};
camera-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 5>;
@@ -3239,8 +3243,8 @@
};
video-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 6>;
@@ -3254,8 +3258,8 @@
};
modem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 7>;
--
2.17.1
^ permalink raw reply related
* [PATCH 09/15] arm64: dts: msm8996: thermal: Add interrupt support
From: Amit Kucheria @ 2019-07-25 22:18 UTC (permalink / raw)
To: linux-kernel, linux-arm-msm, bjorn.andersson, edubezval,
andy.gross, Andy Gross, Daniel Lezcano, Mark Rutland, Rob Herring,
Zhang Rui
Cc: devicetree
In-Reply-To: <cover.1564091601.git.amit.kucheria@linaro.org>
Register upper-lower interrupts for each of the two tsens controllers.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
arch/arm64/boot/dts/qcom/msm8996.dtsi | 60 ++++++++++++++-------------
1 file changed, 32 insertions(+), 28 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 96c0a481f454..7325eba42d19 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -175,8 +175,8 @@
thermal-zones {
cpu0-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 3>;
@@ -196,8 +196,8 @@
};
cpu1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 5>;
@@ -217,8 +217,8 @@
};
cpu2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 8>;
@@ -238,8 +238,8 @@
};
cpu3-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 10>;
@@ -259,8 +259,8 @@
};
gpu-thermal-top {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 6>;
@@ -274,8 +274,8 @@
};
gpu-thermal-bottom {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 7>;
@@ -289,8 +289,8 @@
};
m4m-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 1>;
@@ -304,8 +304,8 @@
};
l3-or-venus-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 2>;
@@ -319,8 +319,8 @@
};
cluster0-l2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 7>;
@@ -334,8 +334,8 @@
};
cluster1-l2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 12>;
@@ -349,8 +349,8 @@
};
camera-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 1>;
@@ -364,8 +364,8 @@
};
q6-dsp-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 2>;
@@ -379,8 +379,8 @@
};
mem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 3>;
@@ -394,8 +394,8 @@
};
modemtx-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 4>;
@@ -591,6 +591,8 @@
reg = <0x4a9000 0x1000>, /* TM */
<0x4a8000 0x1000>; /* SROT */
#qcom,sensors = <13>;
+ interrupts = <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "tsens0";
#thermal-sensor-cells = <1>;
};
@@ -599,6 +601,8 @@
reg = <0x4ad000 0x1000>, /* TM */
<0x4ac000 0x1000>; /* SROT */
#qcom,sensors = <8>;
+ interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "tsens1";
#thermal-sensor-cells = <1>;
};
--
2.17.1
^ permalink raw reply related
* [PATCH 10/15] arm64: dts: msm8998: thermal: Add interrupt support
From: Amit Kucheria @ 2019-07-25 22:18 UTC (permalink / raw)
To: linux-kernel, linux-arm-msm, bjorn.andersson, edubezval,
andy.gross, Andy Gross, Daniel Lezcano, Mark Rutland, Rob Herring,
Zhang Rui
Cc: marc.w.gonzalez, devicetree
In-Reply-To: <cover.1564091601.git.amit.kucheria@linaro.org>
Register upper-lower interrupts for each of the two tsens controllers.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
Cc: marc.w.gonzalez@free.fr
arch/arm64/boot/dts/qcom/msm8998.dtsi | 82 ++++++++++++++-------------
1 file changed, 42 insertions(+), 40 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index c13ed7aeb1e0..f9abd652a544 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -440,8 +440,8 @@
thermal-zones {
cpu0-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 1>;
@@ -461,8 +461,8 @@
};
cpu1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 2>;
@@ -482,8 +482,8 @@
};
cpu2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 3>;
@@ -503,8 +503,8 @@
};
cpu3-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 4>;
@@ -524,8 +524,8 @@
};
cpu4-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 7>;
@@ -545,8 +545,8 @@
};
cpu5-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 8>;
@@ -566,8 +566,8 @@
};
cpu6-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 9>;
@@ -587,8 +587,8 @@
};
cpu7-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 10>;
@@ -608,8 +608,8 @@
};
gpu-thermal-bottom {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 12>;
@@ -623,8 +623,8 @@
};
gpu-thermal-top {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 13>;
@@ -638,8 +638,8 @@
};
clust0-mhm-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 5>;
@@ -653,8 +653,8 @@
};
clust1-mhm-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 6>;
@@ -668,8 +668,8 @@
};
cluster1-l2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens0 11>;
@@ -683,8 +683,8 @@
};
modem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 1>;
@@ -698,8 +698,8 @@
};
mem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 2>;
@@ -713,8 +713,8 @@
};
wlan-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 3>;
@@ -728,8 +728,8 @@
};
q6-dsp-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 4>;
@@ -743,8 +743,8 @@
};
camera-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 5>;
@@ -758,8 +758,8 @@
};
multimedia-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens1 6>;
@@ -845,8 +845,9 @@
compatible = "qcom,msm8998-tsens", "qcom,tsens-v2";
reg = <0x10ab000 0x1000>, /* TM */
<0x10aa000 0x1000>; /* SROT */
-
#qcom,sensors = <14>;
+ interrupts = <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "tsens0";
#thermal-sensor-cells = <1>;
};
@@ -854,8 +855,9 @@
compatible = "qcom,msm8998-tsens", "qcom,tsens-v2";
reg = <0x10ae000 0x1000>, /* TM */
<0x10ad000 0x1000>; /* SROT */
-
#qcom,sensors = <8>;
+ interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "tsens1";
#thermal-sensor-cells = <1>;
};
--
2.17.1
^ permalink raw reply related
* [PATCH 11/15] arm64: dts: qcs404: thermal: Add interrupt support
From: Amit Kucheria @ 2019-07-25 22:18 UTC (permalink / raw)
To: linux-kernel, linux-arm-msm, bjorn.andersson, edubezval,
andy.gross, Andy Gross, Daniel Lezcano, Mark Rutland, Rob Herring,
Zhang Rui
Cc: devicetree
In-Reply-To: <cover.1564091601.git.amit.kucheria@linaro.org>
Register upper-lower interrupt for the tsens controller.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
arch/arm64/boot/dts/qcom/qcs404.dtsi | 42 +++++++++++++++-------------
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index 3d0789775009..0c715f9cc011 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -280,6 +280,8 @@
nvmem-cells = <&tsens_caldata>;
nvmem-cell-names = "calib";
#qcom,sensors = <10>;
+ interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "tsens";
#thermal-sensor-cells = <1>;
};
@@ -1071,8 +1073,8 @@
thermal-zones {
aoss-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 0>;
@@ -1086,8 +1088,8 @@
};
q6-hvx-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 1>;
@@ -1101,8 +1103,8 @@
};
lpass-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 2>;
@@ -1116,8 +1118,8 @@
};
wlan-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 3>;
@@ -1131,8 +1133,8 @@
};
cluster-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 4>;
@@ -1165,8 +1167,8 @@
};
cpu0-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 5>;
@@ -1199,8 +1201,8 @@
};
cpu1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 6>;
@@ -1233,8 +1235,8 @@
};
cpu2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 7>;
@@ -1267,8 +1269,8 @@
};
cpu3-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 8>;
@@ -1301,8 +1303,8 @@
};
gpu-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 9>;
--
2.17.1
^ permalink raw reply related
* [PATCH 12/15] arm64: dts: msm8974: thermal: Add interrupt support
From: Amit Kucheria @ 2019-07-25 22:18 UTC (permalink / raw)
To: linux-kernel, linux-arm-msm, bjorn.andersson, edubezval,
andy.gross, Andy Gross, Daniel Lezcano, Mark Rutland, Rob Herring,
Zhang Rui
Cc: masneyb, devicetree
In-Reply-To: <cover.1564091601.git.amit.kucheria@linaro.org>
Register upper-lower interrupt for the tsens controller.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
Cc: masneyb@onstation.org
arch/arm/boot/dts/qcom-msm8974.dtsi | 36 +++++++++++++++--------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index d32f639505f1..d10d47d20ab8 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -139,8 +139,8 @@
thermal-zones {
cpu-thermal0 {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 5>;
@@ -159,8 +159,8 @@
};
cpu-thermal1 {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 6>;
@@ -179,8 +179,8 @@
};
cpu-thermal2 {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 7>;
@@ -199,8 +199,8 @@
};
cpu-thermal3 {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 8>;
@@ -219,8 +219,8 @@
};
q6-dsp-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 1>;
@@ -234,8 +234,8 @@
};
modemtx-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 2>;
@@ -250,7 +250,7 @@
video-thermal {
polling-delay-passive = <0>;
- polling-delay = <1000>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 3>;
@@ -279,8 +279,8 @@
};
gpu-thermal-top {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 9>;
@@ -294,8 +294,8 @@
};
gpu-thermal-bottom {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 10>;
@@ -531,6 +531,8 @@
nvmem-cells = <&tsens_calib>, <&tsens_backup>;
nvmem-cell-names = "calib", "calib_backup";
#qcom,sensors = <11>;
+ interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "tsens";
#thermal-sensor-cells = <1>;
};
--
2.17.1
^ permalink raw reply related
* [PATCH 13/15] arm64: dts: msm8916: thermal: Add interrupt support
From: Amit Kucheria @ 2019-07-25 22:18 UTC (permalink / raw)
To: linux-kernel, linux-arm-msm, bjorn.andersson, edubezval,
andy.gross, Andy Gross, Daniel Lezcano, Mark Rutland, Rob Herring,
Zhang Rui
Cc: devicetree
In-Reply-To: <cover.1564091601.git.amit.kucheria@linaro.org>
Register upper-lower interrupt for the tsens controller.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
arch/arm64/boot/dts/qcom/msm8916.dtsi | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 8686e101905c..400045a100ca 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -176,8 +176,8 @@
thermal-zones {
cpu0_1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 5>;
@@ -206,8 +206,8 @@
};
cpu2_3-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 4>;
@@ -236,8 +236,8 @@
};
gpu-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 2>;
@@ -256,8 +256,8 @@
};
camera-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 1>;
@@ -271,8 +271,8 @@
};
modem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
thermal-sensors = <&tsens 0>;
@@ -816,6 +816,8 @@
nvmem-cells = <&tsens_caldata>, <&tsens_calsel>;
nvmem-cell-names = "calib", "calib_sel";
#qcom,sensors = <5>;
+ interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "tsens";
#thermal-sensor-cells = <1>;
};
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] ARM: dts: add device tree for Mecer Xtreme Mini S6
From: Heiko Stuebner @ 2019-07-25 22:19 UTC (permalink / raw)
To: Justin Swartz
Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
linux-arm-kernel, linux-rockchip
In-Reply-To: <20190616204746.21001-1-justin.swartz@risingedge.co.za>
Hi Justin,
Am Sonntag, 16. Juni 2019, 22:47:45 CEST schrieb Justin Swartz:
> The Mecer Xtreme Mini S6 features a Rockchip RK3229 SoC,
> 1GB DDR3 RAM, 8GB eMMC, MicroSD port, 10/100Mbps Ethernet,
> Realtek 8723BS WLAN module, 2 x USB 2.0 ports, HDMI output,
> and S/PDIF output.
>
> Signed-off-by: Justin Swartz <justin.swartz@risingedge.co.za>
> ---
please add an entry to Documentation/devicetree/bindings/arm/rockchip.yaml
for your board and if necessary also a vendor-prefix to
Documentation/devicetree/bindings/vendor-prefixes.(yaml?)
See below.
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/rk3229-xms6.dts | 286 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 287 insertions(+)
> create mode 100644 arch/arm/boot/dts/rk3229-xms6.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index dab2914fa293..6fbd7c304f62 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -902,6 +902,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += \
> rk3188-radxarock.dtb \
> rk3228-evb.dtb \
> rk3229-evb.dtb \
> + rk3229-xms6.dtb \
> rk3288-evb-act8846.dtb \
> rk3288-evb-rk808.dtb \
> rk3288-fennec.dtb \
> diff --git a/arch/arm/boot/dts/rk3229-xms6.dts b/arch/arm/boot/dts/rk3229-xms6.dts
> new file mode 100644
> index 000000000000..9b666fa66292
> --- /dev/null
> +++ b/arch/arm/boot/dts/rk3229-xms6.dts
> @@ -0,0 +1,286 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/input/input.h>
> +#include "rk3229.dtsi"
> +
> +/ {
> + model = "Rockchip RK3229 (Mecer Xtreme Mini S6)";
> + compatible = "rockchip,rk3229-xms6", "rockchip,rk3229";
mode = "Mecer Xtreme Mini S6";
compatible = "mecer,xms6", "rockchip,rk3229";
(and as written above, add a vendor-prefix for mecer)
...
> +&cpu0 {
> + clock-frequency = <1464000000>;
not sure I understand the reasoning here.
There seems to be a regulator defined, so the cpu cores should
have operating points defined to allow them to switch between
different frequencies as needed.
> + cpu-supply = <&vdd_arm>;
> +};
> +
> +&cpu1 {
> + clock-frequency = <1464000000>;
> + cpu-supply = <&vdd_arm>;
> +};
> +
> +&cpu2 {
> + clock-frequency = <1464000000>;
> + cpu-supply = <&vdd_arm>;
> +};
> +
> +&cpu3 {
> + clock-frequency = <1464000000>;
> + cpu-supply = <&vdd_arm>;
> +};
> +
> +&vop {
please sort the &node-references alphabetically.
Heiko
^ permalink raw reply
* Re: [PATCH v4, 05/33] dt-bindings: mediatek: add RDMA1 description for mt8183 display
From: Rob Herring @ 2019-07-25 22:23 UTC (permalink / raw)
To: CK Hu
Cc: yongqiang.niu, Philipp Zabel, Matthias Brugger, David Airlie,
Daniel Vetter, Mark Rutland, dri-devel, devicetree,
linux-kernel@vger.kernel.org,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/Mediatek SoC support
In-Reply-To: <1564024819.2621.4.camel@mtksdaap41>
On Wed, Jul 24, 2019 at 9:20 PM CK Hu <ck.hu@mediatek.com> wrote:
>
> Hi, Rob:
>
> On Wed, 2019-07-24 at 14:16 -0600, Rob Herring wrote:
> > On Tue, Jul 09, 2019 at 06:33:45AM +0800, yongqiang.niu@mediatek.com wrote:
> > > From: Yongqiang Niu <yongqiang.niu@mediatek.com>
> > >
> > > This patch add RDMA1 description for mt8183 display
> > >
> > > Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
> > > ---
> > > Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
> > > index afd3c90..bb9274a 100644
> > > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
> > > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,disp.txt
> > > @@ -30,6 +30,7 @@ Required properties (all function blocks):
> > > "mediatek,<chip>-disp-ovl" - overlay (4 layers, blending, csc)
> > > "mediatek,<chip>-disp-ovl-2l" - overlay (2 layers, blending, csc)
> > > "mediatek,<chip>-disp-rdma" - read DMA / line buffer
> > > + "mediatek,<chip>-disp-rdma1" - function is same with RDMA, fifo size is different
> >
> > This can't be determined by which chip it is? IOW, a chip may have both
> > rdma and rdma1?
>
> In MT8183, there are two different rdma. The difference is the fifo size
> in each one. I've a question: is it better to have two compatible string
> for each one, or just one compatible string for both but with a property
> to set fifo size?
If that's the only diff, then a property for fifo size is fine. We
just don't want to be adding a new property for each new difference.
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: irqchip: Add PRUSS interrupt controller bindings
From: Rob Herring @ 2019-07-25 22:27 UTC (permalink / raw)
To: Suman Anna
Cc: Marc Zyngier, Thomas Gleixner, Jason Cooper, Tony Lindgren,
Andrew F. Davis, Roger Quadros, Lokesh Vutla, Grygorii Strashko,
Sekhar Nori, David Lechner, Murali Karicheri, devicetree,
linux-omap,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-kernel@vger.kernel.org
In-Reply-To: <6871c381-9fc6-f6be-6386-f183fcc5546a@ti.com>
On Wed, Jul 24, 2019 at 1:42 PM Suman Anna <s-anna@ti.com> wrote:
>
> On 7/24/19 11:34 AM, Rob Herring wrote:
> > On Sun, 7 Jul 2019 22:52:38 -0500, Suman Anna wrote:
> >> The Programmable Real-Time Unit Subsystem (PRUSS) contains an interrupt
> >> controller (INTC) that can handle various system input events and post
> >> interrupts back to the device-level initiators. The INTC can support
> >> upto 64 input events on most SoCs with individual control configuration
> >> and hardware prioritization. These events are mapped onto 10 interrupt
> >> lines through two levels of many-to-one mapping support. Different
> >> interrupt lines are routed to the individual PRU cores or to the
> >> host CPU or to other PRUSS instances.
> >>
> >> The K3 AM65x and J721E SoCs have the next generation of the PRU-ICSS IP,
> >> commonly called ICSSG. The ICSSG interrupt controller on K3 SoCs provide
> >> a higher number of host interrupts (20 vs 10) and can handle an increased
> >> number of input events (160 vs 64) from various SoC interrupt sources.
> >>
> >> Add the bindings document for these interrupt controllers on all the
> >> applicable SoCs. It covers the OMAP architecture SoCs - AM33xx, AM437x
> >> and AM57xx; the Keystone 2 architecture based 66AK2G SoC; the Davinci
> >> architecture based OMAPL138 SoCs, and the K3 architecture based AM65x
> >> and J721E SoCs.
> >>
> >> Signed-off-by: Suman Anna <s-anna@ti.com>
> >> Signed-off-by: Andrew F. Davis <afd@ti.com>
> >> Signed-off-by: Roger Quadros <rogerq@ti.com>
> >> ---
> >> Prior version: https://patchwork.kernel.org/patch/10795771/
> >>
> >> .../interrupt-controller/ti,pruss-intc.txt | 92 +++++++++++++++++++
> >> 1 file changed, 92 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.txt
> >>
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
> >
>
> Thanks Rob. I am going to submit a v2 with some minor reword changes
> based on couple of comments, but no addition or removal of properties.
> Should I be retaining your Reviewed-by for v2?
Yes.
^ permalink raw reply
* Re: [PATCH 2/2] DTS: ARM: gta04: introduce legacy spi-cs-high to make display work again
From: Rob Herring @ 2019-07-25 22:42 UTC (permalink / raw)
To: H. Nikolaus Schaller
Cc: Linus Walleij, Mark Brown, Mark Rutland, Benoît Cousson,
Tony Lindgren, Discussions about the Letux Kernel, linux-spi,
devicetree, Linux Kernel Mailing List, Linux-OMAP, stable
In-Reply-To: <2EA06398-E45B-481B-9A26-4DD2E043BF9C@goldelico.com>
On Thu, Jul 25, 2019 at 12:23 AM H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> Hi Rob,
>
> > Am 24.07.2019 um 21:42 schrieb Rob Herring <robh@kernel.org>:
> >
> > On Mon, Jul 08, 2019 at 04:46:05PM +0200, H. Nikolaus Schaller wrote:
> >> commit 6953c57ab172 "gpio: of: Handle SPI chipselect legacy bindings"
> >>
> >> did introduce logic to centrally handle the legacy spi-cs-high property
> >> in combination with cs-gpios. This assumes that the polarity
> >> of the CS has to be inverted if spi-cs-high is missing, even
> >> and especially if non-legacy GPIO_ACTIVE_HIGH is specified.
> >>
> >> The DTS for the GTA04 was orginally introduced under the assumption
> >> that there is no need for spi-cs-high if the gpio is defined with
> >> proper polarity GPIO_ACTIVE_HIGH.
> >
> > Given that spi-cs-high is called legacy, that would imply that DT's
> > should not have to use spi-cs-high.
>
> Yes.
>
> >
> >> This was not a problem until gpiolib changed the interpretation of
> >> GPIO_ACTIVE_HIGH and missing spi-cs-high.
> >
> > Then we should fix gpiolib...
>
> I tried to convince Linus that this is the right way but he convinced
> me that a fix that handles all cases does not exist.
>
> There seem to be embedded devices with older DTB (potentially in ROM)
> which provide a plain 0 value for a gpios definition. And either with
> or without spi-cs-high.
>
> Since "0" is the same as "GPIO_ACTIVE_HIGH", the absence of
> spi-cs-high was and must be interpreted as active low for these
> devices. This leads to the inversion logic in code.
>
> AFAIR it boils down to the question if gpiolib and the bindings
> should still support such legacy devices with out-of tree DTB,
> but force in-tree DTS to add the legacy spi-cs-high property.
>
> Or if we should fix the 2 or 3 cases of in-tree legacy cases
> and potentially break out-of tree DTBs.
If it is small number of platforms, then the kernel could handle those
cases explicitly as needed.
> IMHO it is more general to keep the out-of-tree DTBs working
> and "fix" what we can control (in-tree DTS).
If we do this, then we need to not call spi-cs-high legacy because
we're stuck with it forever.
Rob
^ permalink raw reply
* Re: [PATCH v5 2/4] dt-bindings: thermal: nvme: Add binding documentation
From: Rob Herring @ 2019-07-25 22:46 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-nvme, open list:THERMAL, open list:OPEN FIRMWARE AND...,
Zhang Rui, Eduardo Valentin, Daniel Lezcano, Keith Busch,
Jens Axboe, Christoph Hellwig, Sagi Grimberg, Minwoo Im,
Kenneth Heitke, Chaitanya Kulkarni
In-Reply-To: <CAC5umyh5d7Ya-Ou8BZmPfpXKT+WkMCWnRKkCw8xhe9upNKqVbg@mail.gmail.com>
On Thu, Jul 25, 2019 at 8:24 AM Akinobu Mita <akinobu.mita@gmail.com> wrote:
>
> 2019年7月23日(火) 7:16 Rob Herring <robh@kernel.org>:
> >
> > On Mon, Jul 01, 2019 at 11:12:32PM +0900, Akinobu Mita wrote:
> > > Add thermal binding documentation for NVMe temperature sensor.
> > >
> > > Cc: Rob Herring <robh@kernel.org>
> > > Cc: Zhang Rui <rui.zhang@intel.com>
> > > Cc: Eduardo Valentin <edubezval@gmail.com>
> > > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > Cc: Keith Busch <kbusch@kernel.org>
> > > Cc: Jens Axboe <axboe@fb.com>
> > > Cc: Christoph Hellwig <hch@lst.de>
> > > Cc: Sagi Grimberg <sagi@grimberg.me>
> > > Cc: Minwoo Im <minwoo.im.dev@gmail.com>
> > > Cc: Kenneth Heitke <kenneth.heitke@intel.com>
> > > Cc: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
> > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > > ---
> > > * v5
> > > - New patch
> > >
> > > Documentation/devicetree/bindings/thermal/nvme.txt | 56 ++++++++++++++++++++++
> > > 1 file changed, 56 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/thermal/nvme.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/thermal/nvme.txt b/Documentation/devicetree/bindings/thermal/nvme.txt
> > > new file mode 100644
> > > index 0000000..60b90de
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/thermal/nvme.txt
> > > @@ -0,0 +1,56 @@
> > > +Binding for NVMe temperature sensor
> > > +
> > > +An NVMe controller reports up to nine temperature values in the SMART / Health
> > > +log.
> > > +
> > > +Required properties:
> > > +- reg: A five-cell address encoded as (phys.hi phys.mid phys.lo size.hi
> > > + size.lo). phys.hi should contain the device's BDF (Bus/Device/Function)
> > > + as 0b00000000 bbbbbbbb dddddfff 00000000. The other cells should be zero.
> > > + See also Documentation/devicetree/bindings/pci/pci.txt
> > > +
> > > +- #thermal-sensor-cells: Must be 1. See ./thermal.txt for a description.
> > > + In the thermal-sensors property, the sensor ID 0 for composite temperature,
> > > + 1 through 8 for NVMe temperature sensor N.
> > > +
> > > +Example:
> > > +
> > > +&pcie0 {
> > > + ...
> > > + nvme: nvme@0,0 {
> > > + reg = <0x0000 0 0 0 0>;
> > > + #address-cells = <3>;
> > > + #size-cells = <2>;
> > > +
> > > + nvmetemp: nvmetemp {
> > > + reg = <0x0000 0 0 0 0>; /* DEVFN = 0x00 (0:0) */
> >
> > I'm not sure this is really valid PCI addressing as the parent has the
> > same address.
> >
> > > + #thermal-sensor-cells = <1>;
> >
> > Can't you just put this in the parent? Is this really a separate
> > addressable device from the parent?
>
> How about this?
>
> &pcie0 {
> ...
> pci-bridge@0 {
> reg = <0x00000 0 0 0 0>;
> #address-cells = <3>;
> #size-cells = <2>;
>
> nvme: nvme@0,0 {
> reg = <0x10000 0 0 0 0>;
> #thermal-sensor-cells = <1>;
> };
> };
> };
>
> and
>
> &thermal_zones {
> ...
> thermal-sensors = <&nvme 0>;
> };
>
> I tested this with the RockPro64 and edited
> arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts.
>
> $ lspci
> 00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 0100
> 01:00.0 Non-Volatile memory controller: Micron/Crucial Technology
> Device 2263 (rev 03)
>
> $ lspci -tv
> -[0000:00]---00.0-[01]----00.0 Micron/Crucial Technology Device 2263
Looks better to me.
Rob
^ permalink raw reply
* Re: [PATCH 3/3] remoteproc: ingenic: Added remoteproc driver
From: Paul Cercueil @ 2019-07-25 22:50 UTC (permalink / raw)
To: Ohad Ben-Cohen, Bjorn Andersson, Rob Herring, Mark Rutland
Cc: linux-remoteproc, devicetree, linux-kernel, od
In-Reply-To: <20190722023140.14701-3-paul@crapouillou.net>
Hi,
Le dim. 21 juil. 2019 à 22:31, Paul Cercueil <paul@crapouillou.net> a
écrit :
> This driver is used to boot, communicate with and load firmwares to
> the
> MIPS co-processor found in the VPU hardware of the JZ47xx SoCs from
> Ingenic.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> drivers/remoteproc/Kconfig | 8 +
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/ingenic_rproc.c | 302
> +++++++++++++++++++++++++++++
> 3 files changed, 311 insertions(+)
> create mode 100644 drivers/remoteproc/ingenic_rproc.c
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 28ed306982f7..a0be40e2098d 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -214,6 +214,14 @@ config STM32_RPROC
>
> This can be either built-in or a loadable module.
>
> +config INGENIC_RPROC
> + tristate "Ingenic JZ47xx VPU remoteproc support"
> + depends on MIPS || COMPILE_TEST
> + help
> + Say y or m here to support the VPU in the JZ47xx SoCs from
> Ingenic.
> + This can be either built-in or a loadable module.
> + If unsure say N.
> +
> endif # REMOTEPROC
>
> endmenu
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 00f09e658cb3..6eb0137abbc7 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -10,6 +10,7 @@ remoteproc-y += remoteproc_sysfs.o
> remoteproc-y += remoteproc_virtio.o
> remoteproc-y += remoteproc_elf_loader.o
> obj-$(CONFIG_IMX_REMOTEPROC) += imx_rproc.o
> +obj-$(CONFIG_INGENIC_RPROC) += ingenic_rproc.o
> obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
> obj-$(CONFIG_WKUP_M3_RPROC) += wkup_m3_rproc.o
> obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
> diff --git a/drivers/remoteproc/ingenic_rproc.c
> b/drivers/remoteproc/ingenic_rproc.c
> new file mode 100644
> index 000000000000..a4963158bdd3
> --- /dev/null
> +++ b/drivers/remoteproc/ingenic_rproc.c
> @@ -0,0 +1,302 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Ingenic JZ47xx remoteproc driver
> + * Copyright 2019, Paul Cercueil <paul@crapouillou.net>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/remoteproc.h>
> +
> +#include "remoteproc_internal.h"
> +
> +#define REG_AUX_CTRL 0x0
> +#define REG_AUX_MSG_ACK 0x10
> +#define REG_AUX_MSG 0x14
> +#define REG_CORE_MSG_ACK 0x18
> +#define REG_CORE_MSG 0x1C
> +
> +#define AUX_CTRL_SLEEP BIT(31)
> +#define AUX_CTRL_MSG_IRQ_EN BIT(3)
> +#define AUX_CTRL_NMI_RESETS BIT(2)
> +#define AUX_CTRL_NMI BIT(1)
> +#define AUX_CTRL_SW_RESET BIT(0)
> +
> +struct vpu_mem_map {
> + const char *name;
> + unsigned int da;
> + bool direct_io;
> +};
> +
> +struct vpu_mem_info {
> + const struct vpu_mem_map *map;
> + unsigned long len;
> + void __iomem *base;
> +};
> +
> +static const struct vpu_mem_map vpu_mem_map[] = {
> + { "tcsm0", 0x132b0000, true },
> + { "tcsm1", 0xf4000000 },
> + { "sram", 0x132f0000 },
> +};
> +
> +/* Device data */
> +struct vpu {
> + int irq;
> + struct clk *vpu_clk;
> + struct clk *aux_clk;
> + void __iomem *aux_base;
> + struct vpu_mem_info mem_info[ARRAY_SIZE(vpu_mem_map)];
> + struct device *dev;
> +};
> +
> +static int ingenic_rproc_prepare(struct rproc *rproc)
> +{
> + struct vpu *vpu = rproc->priv;
> + int ret;
> +
> + ret = clk_prepare_enable(vpu->vpu_clk);
> + if (ret) {
> + dev_err(vpu->dev, "Unable to start VPU clock: %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(vpu->aux_clk);
> + if (ret) {
> + dev_err(vpu->dev, "Unable to start AUX clock: %d\n", ret);
> + goto err_disable_vpu_clk;
> + }
> +
> + return 0;
> +
> +err_disable_vpu_clk:
> + clk_disable_unprepare(vpu->vpu_clk);
> + return ret;
> +}
> +
> +static void ingenic_rproc_unprepare(struct rproc *rproc)
> +{
> + struct vpu *vpu = rproc->priv;
> +
> + clk_disable_unprepare(vpu->aux_clk);
> + clk_disable_unprepare(vpu->vpu_clk);
> +}
> +
> +static int ingenic_rproc_start(struct rproc *rproc)
> +{
> + struct vpu *vpu = rproc->priv;
> + u32 ctrl;
> +
> + enable_irq(vpu->irq);
> +
> + /* Reset the AUX and enable message IRQ */
> + ctrl = AUX_CTRL_NMI_RESETS | AUX_CTRL_NMI | AUX_CTRL_MSG_IRQ_EN;
> + writel(ctrl, vpu->aux_base + REG_AUX_CTRL);
> +
> + return 0;
> +}
> +
> +static int ingenic_rproc_stop(struct rproc *rproc)
> +{
> + struct vpu *vpu = rproc->priv;
> +
> + /* Keep AUX in reset mode */
> + writel(AUX_CTRL_SW_RESET, vpu->aux_base + REG_AUX_CTRL);
> +
> + disable_irq_nosync(vpu->irq);
> +
> + return 0;
> +}
> +
> +static void ingenic_rproc_kick(struct rproc *rproc, int vqid)
> +{
> + struct vpu *vpu = rproc->priv;
> +
> + writel(vqid, vpu->aux_base + REG_CORE_MSG);
> +}
> +
> +static void *ingenic_rproc_da_to_va(struct rproc *rproc, u64 da, int
> len)
> +{
> + struct vpu *vpu = rproc->priv;
> + void __iomem *va = NULL;
> + unsigned int i;
> +
> + if (len <= 0)
> + return NULL;
> +
> + for (i = 0; i < ARRAY_SIZE(vpu_mem_map); i++) {
> + const struct vpu_mem_info *info = &vpu->mem_info[i];
> + const struct vpu_mem_map *map = info->map;
> +
> + if (da >= map->da && (da + len) < (map->da + info->len)) {
> + va = info->base + (da - map->da);
> + break;
> + }
> + }
> +
> + return (__force void *)va;
> +}
> +
> +static struct rproc_ops ingenic_rproc_ops = {
> + .prepare = ingenic_rproc_prepare,
> + .unprepare = ingenic_rproc_unprepare,
> + .start = ingenic_rproc_start,
> + .stop = ingenic_rproc_stop,
> + .kick = ingenic_rproc_kick,
> + .da_to_va = ingenic_rproc_da_to_va,
> +};
> +
> +static irqreturn_t vpu_interrupt(int irq, void *data)
> +{
> + struct rproc *rproc = data;
> + struct vpu *vpu = rproc->priv;
> + u32 vring;
> +
> + vring = readl(vpu->aux_base + REG_AUX_MSG);
> +
> + /* Ack the interrupt */
> + writel(0, vpu->aux_base + REG_AUX_MSG_ACK);
> +
> + return rproc_vq_interrupt(rproc, vring);
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id ingenic_rproc_of_matches[] = {
> + { .compatible = "ingenic,jz4770-vpu-rproc", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, ingenic_rproc_of_matches);
> +#endif
> +
> +static void ingenic_rproc_free(void *rproc)
> +{
> + rproc_free(rproc);
> +}
> +
> +static void ingenic_rproc_unregister(void *rproc)
> +{
> + rproc_del(rproc);
> + rproc_shutdown(rproc);
> +}
> +
> +static int ingenic_rproc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *mem;
> + struct rproc *rproc;
> + struct vpu *vpu;
> + unsigned int i;
> + int ret;
> +
> + rproc = rproc_alloc(dev, "ingenic-vpu",
> + &ingenic_rproc_ops, NULL, sizeof(*vpu));
> + if (!rproc)
> + return -ENOMEM;
> +
> + ret = devm_add_action_or_reset(dev, ingenic_rproc_free, rproc);
> + if (ret) {
> + dev_err(dev, "Unable to add action");
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, rproc);
> + vpu = rproc->priv;
> + vpu->dev = &pdev->dev;
> +
> + mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "aux");
> + vpu->aux_base = devm_ioremap_resource(dev, mem);
> + if (IS_ERR(vpu->aux_base)) {
> + dev_err(dev, "Failed to ioremap");
> + return PTR_ERR(vpu->aux_base);
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(vpu_mem_map); i++) {
> + mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + vpu_mem_map[i].name);
> +
> + if (vpu_mem_map[i].direct_io) {
> + /*
> + * Handle shared memories that cannot be iomapped.
> + * They can be read or written directly through their
> + * physical address.
> + */
I found there's actually a mirror of that shared memory that can be
iomapped.
I'll send a V2 and remove that workaround.
Thanks,
-Paul
> + if (!devm_request_mem_region(dev, mem->start,
> + resource_size(mem),
> + dev_name(dev))) {
> + dev_err(dev, "Unable to request memory region");
> + return -EBUSY;
> + }
> +
> + vpu->mem_info[i].base = (void __iomem *)mem->start;
> + } else {
> + vpu->mem_info[i].base = devm_ioremap_resource(dev, mem);
> + if (IS_ERR(vpu->mem_info[i].base)) {
> + ret = PTR_ERR(vpu->mem_info[i].base);
> + dev_err(dev, "Failed to ioremap");
> + return ret;
> + }
> + }
> +
> + vpu->mem_info[i].len = resource_size(mem);
> + vpu->mem_info[i].map = &vpu_mem_map[i];
> + }
> +
> + vpu->vpu_clk = devm_clk_get(dev, "vpu");
> + if (IS_ERR(vpu->vpu_clk)) {
> + dev_err(dev, "Failed to get VPU clock");
> + return PTR_ERR(vpu->vpu_clk);
> + }
> +
> + vpu->aux_clk = devm_clk_get(dev, "aux");
> + if (IS_ERR(vpu->aux_clk)) {
> + dev_err(dev, "Failed to get AUX clock");
> + return PTR_ERR(vpu->aux_clk);
> + }
> +
> + vpu->irq = platform_get_irq(pdev, 0);
> + if (vpu->irq < 0) {
> + dev_err(dev, "Failed to get platform IRQ");
> + return vpu->irq;
> + }
> +
> + ret = devm_request_irq(dev, vpu->irq, vpu_interrupt, 0, "VPU",
> rproc);
> + if (ret < 0) {
> + dev_err(dev, "Failed to request IRQ");
> + return ret;
> + }
> +
> + disable_irq_nosync(vpu->irq);
> +
> + ret = rproc_add(rproc);
> + if (ret) {
> + dev_err(dev, "Failed to register remote processor");
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(dev, ingenic_rproc_unregister,
> rproc);
> + if (ret) {
> + dev_err(dev, "Unable to add action");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static struct platform_driver ingenic_rproc_driver = {
> + .probe = ingenic_rproc_probe,
> + .driver = {
> + .name = "ingenic-vpu",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(ingenic_rproc_of_matches),
> + },
> +};
> +module_platform_driver(ingenic_rproc_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
> +MODULE_DESCRIPTION("Ingenic JZ47xx Remote Processor control driver");
> --
> 2.21.0.593.g511ec345e18
>
^ permalink raw reply
* Re: [PATCH 0/4] new driver for TI eQEP
From: David Lechner @ 2019-07-25 22:52 UTC (permalink / raw)
To: William Breathitt Gray
Cc: linux-iio, linux-omap, devicetree, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Thierry Reding, linux-kernel,
linux-pwm
In-Reply-To: <20190725124037.GA4802@icarus>
On 7/25/19 7:40 AM, William Breathitt Gray wrote:
> On Mon, Jul 22, 2019 at 10:45:34AM -0500, David Lechner wrote:
>> This series adds device tree bindings and a new counter driver for the Texas
>> Instruments Enhanced Quadrature Encoder Pulse (eQEP).
>>
>> As mentioned in one of the commit messages, to start with, the driver only
>> supports reading the current counter value and setting the min/max values.
>> Other features can be added on an as-needed basis.
>>
>> The only other feature I am interested in is adding is getting time data in
>> order to calculate the rotational speed of a motor. However, there probably
>> needs to be a higher level discussion of how this can fit into the counter
>> subsystem in general first.
>
> I believe exposing some sort of time data has merit. Quadrature counter
> devices in particular are commonly used for position tracking of
> automation systems, and such systems would benefit from velocity/speed
> information. So let's try to introduce that sort of functionality in this
> driver if possible.
>
> First, let's discuss your specific use case and requirements, and hopefully we
> can generalize it enough to be of use for future drivers. From your description,
> it sounds like you're attaching some sort of rotary encoder to the eQEP device.
> Is that correct? What sort of time data are you hoping to use; does the eQEP
> device provide a clock value, or would you be grabbing a timestamp from the
> system?
My use case is robotics using LEGO MINDSTORMS. More specifically, I am using
motors that have a cheap optical rotary encoder (plastic wheel and infrared
LED/detectors) that give 360 counts per 1 rotation of the motor shaft. One count
is defined as the rising edge or falling edge of the A signal. We are looking at
anywhere from 0 to around 2000 counts per second. We use the speed as feedback in
a control algorithm to drive the motor at a constant speed. The control loop
updates on the order of 1 to 10 ms.
Because the encoder resolution and speeds are relatively low, we are currently
logging a timestamp for each count. If no count occurs for 50ms, then we log the
same count again with a new timestamp (otherwise we would never see 0 speed). To
get the actual speed, we find the first timestamp > 20 ms before the current
timestamp then compute the speed as the change in position divided by the change
in time between these two samples. This give a fairly accurate speed across most
of the range, but does get a bit noisy once we get below 100 counts per second.
It also means that we need a ring buffer that holds about 50 samples.
The timestamp itself comes from the eQEP, not the system. There are latching
registers to ensure that the timestamp read is from exactly the moment when
the count register was read.
> I'm not sure yet if it would make sense to expose rotational speed directly as
> an attribute. If we were to expose just the count value and timestamp since the
> last read, that should be enough for a user to compute the delta and derive
> speed. I'll think more about this since some devices may simplify that case if
> the hardware is able to compute the speed for us.
>
I agree that it probably doesn't make sense to expect drivers to compute the
speed. There isn't really a general way to do that works for an arbitrary
speed. For example at high speeds, it is better to just look at the change
in counts over a fixed interval rather than triggering a timestamp based on
a certain number of counts.
I also don't think having a timestamp sysfs attribute would be very useful.
To make it work at all, I think it would have to be implemented such that
it returns the timestamp for the count that was most recently read via sysfs.
And it would require 4 syscalls (2 seeks and 2 reads) to get a single count/
timestamp pair in a control loop. On a 300MHz ARM9 processor, this is not
a negligible amount of time.
I noticed that several of the other counter drivers also register an IIO
device. So this got me thinking that perhaps the counter subsystem should
just be for configuring a counter device an then the IIO subsystem should
be used for triggers and ring buffers.
For the general case a counter device could have two possible triggers.
One that triggers an interrupt after X counts and another that triggers
with a period of T nanoseconds (or microseconds). Both triggers would add
a count/timestamp pair to an IIO ring buffer.
To fully reproduce our current methodology the first trigger would actually
need two configurable settings, the count X that triggers every X counts and
a watchdog time setting (using terminology from eQEP docs) that will also
trigger if and only if the count does not change before the time has elapsed.
Note, this is different from the other proposed time based trigger which
would cause a trigger interrupt at a fixed period regardless of whether
the count changed or not.
---
Thinking more generally though, I think what I would propose is adding a new
component to the existing list of Count, Signal and Synapse. The new component
could be called Event. Event would be more general than the trigger conditions
I have just discussed. In addition to those two, it could be any event
generated by the hardware, such as an error condition or a change in direction.
Drivers could register an arbitrary number of events for each Count, so we
would have /sys/bus/counter/devices/counterX/eventY/*. There should be a few
standard attributes, like "name" and "enable". Configurable events would need
ext attributes to allow configuration.
However, I see that there are already preset and error_noise "events" for
count objects, so maybe we don't do the eventY thing and keep it flatter (or
is the counter subsystem still considered in "staging" where breaking ABI
changes could be made?).
When thinking about what events would actually do when enabled though, it
seems like we should be using IIO events and triggers (we have found reading
sysfs attributes to be insufficient performance-wise). It seems like unnecessary
work to reproduce all of this in the counter subsystem. Which makes me wonder if
it would be better to have counter devices just be a different device type (i.e.
different struct device_type for dev->type) in the IIO subsystem instead of
creating a completely new subsystem.
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: crypto: Add DT bindings documentation for amlogic-crypto
From: Rob Herring @ 2019-07-25 22:55 UTC (permalink / raw)
To: Corentin Labbe
Cc: David Miller, Herbert Xu, Kevin Hilman, Mark Rutland, devicetree,
linux-amlogic,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
linux-kernel@vger.kernel.org, baylibre-upstreaming
In-Reply-To: <1564083776-20540-2-git-send-email-clabbe@baylibre.com>
On Thu, Jul 25, 2019 at 1:43 PM Corentin Labbe <clabbe@baylibre.com> wrote:
>
> This patch adds documentation for Device-Tree bindings for the
> Amlogic GXL cryptographic offloader driver.
>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
> .../bindings/crypto/amlogic-gxl-crypto.yaml | 45 +++++++++++++++++++
Follow the compatible string for the filename: amlogic,gxl-crypto.yaml
> 1 file changed, 45 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/crypto/amlogic-gxl-crypto.yaml
>
> diff --git a/Documentation/devicetree/bindings/crypto/amlogic-gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic-gxl-crypto.yaml
> new file mode 100644
> index 000000000000..41265e57c00b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/amlogic-gxl-crypto.yaml
> @@ -0,0 +1,45 @@
> +# SPDX-License-Identifier: GPL-2.0
Dual (GPL-2.0 OR BSD-2-Clause) is preferred for new bindings. Not a
requirement though.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/crypto/amlogic-gxl-crypto.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Amlogic GXL Cryptographic Offloader
> +
> +maintainers:
> + - Corentin Labbe <clabbe@baylibre.com>
> +
> +properties:
> + compatible:
> + oneOf:
Don't need 'oneOf' when there is only 1.
> + - const: amlogic,gxl-crypto
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + const: blkmv
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> +
> +examples:
> + - |
> + crypto: crypto@c883e000 {
> + compatible = "amlogic,gxl-crypto";
> + reg = <0x0 0xc883e000 0x0 0x36>;
This should throw errors because the default size on examples are 1
cell. But validating the examples with the schema only just landed in
5.3-rc1.
> + interrupts = <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>;
This doesn't match the schema.
> + clocks = <&clkc CLKID_BLKMV>;
> + clock-names = "blkmv";
> + };
> --
> 2.21.0
>
^ permalink raw reply
* Re: [PATCH] Revert "ARM: dts: rockchip: add startup delay to rk3288-veyron panel-regulators"
From: Doug Anderson @ 2019-07-25 23:35 UTC (permalink / raw)
To: Heiko Stuebner
Cc: Enric Balletbo i Serra, Matthias Kaehlcke, devicetree, LKML,
open list:ARM/Rockchip SoC..., Rob Herring, Mark Rutland,
Linux ARM
In-Reply-To: <3386344.sHu1S4gNag@phil>
Hi,
On Thu, Jul 25, 2019 at 2:33 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> Am Mittwoch, 3. Juli 2019, 06:54:58 CEST schrieb Doug Anderson:
> > Hi,
> >
> > On Thu, Jun 20, 2019 at 1:31 PM Doug Anderson <dianders@chromium.org> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, Jun 20, 2019 at 11:21 AM Douglas Anderson <dianders@chromium.org> wrote:
> > > >
> > > > This reverts commit 1f45e8c6d0161f044d679f242fe7514e2625af4a.
> > > >
> > > > This 100 ms mystery delay is not on downstream kernels and no longer
> > > > seems needed on upstream kernels either [1]. Presumably something in the
> > > > meantime has made things better. A few possibilities for patches that
> > > > have landed in the meantime that could have made this better are
> > > > commit 3157694d8c7f ("pwm-backlight: Add support for PWM delays
> > > > proprieties."), commit 5fb5caee92ba ("pwm-backlight: Enable/disable
> > > > the PWM before/after LCD enable toggle."), and commit 6d5922dd0d60
> > > > ("ARM: dts: rockchip: set PWM delay backlight settings for Veyron")
> > > >
> > > > Let's revert and get our 100 ms back.
> > > >
> > > > [1] https://lkml.kernel.org/r/2226970.BAPq4liE1j@diego
> > > >
> > > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > > ---
> > > >
> > > > arch/arm/boot/dts/rk3288-veyron-jaq.dts | 1 -
> > > > arch/arm/boot/dts/rk3288-veyron-jerry.dts | 1 -
> > > > arch/arm/boot/dts/rk3288-veyron-minnie.dts | 1 -
> > > > arch/arm/boot/dts/rk3288-veyron-speedy.dts | 1 -
> > > > 4 files changed, 4 deletions(-)
> > >
> > > Maybe wait before applying. I've been running reboot tests now with
> > > this patch applied (among others) and with enough reboots I managed to
> > > see:
> > >
> > > [ 5.682418] rockchip-dp ff970000.dp: eDP link training failed (-5)
> > >
> > > I'll see if I can confirm that it's this patch and why things are
> > > different compared to downstream.
> >
> > OK, I finally got back to this and confirmed:
> >
> > 1. The above error is actually somewhat harmless. The eDP failure
> > will be retried automatically despite the scary message. Specifically
> > see the loop in analogix_dp_bridge_enable(). I confirmed that after
> > seeing the error the screen came up just fine (I looked at the screen
> > in two actual instances but I believe it's pretty much always fine).
> >
> > 2. I haven't seen any evidence that the eDP link training happens any
> > more often with this revert in place. Specifically, I see the same
> > message in the logs (at what appears to be the same rate) with or
> > without this revert.
> >
> > 3. Probably the link-training failures here are the same ones we
> > debugged for PSR for rk3399-gru-kevin that we fixed by making the eDP
> > PCLK rate exactly 24 MHz. See <https://crrev.com/c/433393> for
> > details. On rk3399-gru-kevin it was super important to resolve the
> > root cause of these errors because we had PSR (which meant we were
> > constantly taking to the eDP controller). On rk3288-veyron devices
> > with no PSR the retry should be a fine solution and it doesn't seem
> > like a good idea to fully rejigger our clock plan to fix the root
> > cause.
> >
> >
> > NOTE: I saw _one_ case on rk3288-veyron-minnie where the screen looked
> > wonky at bootup and I saw the eDP link training error in the logs.
> > That's what originally made me cautious. I haven't been able to
> > reproduce this, but presumably I just got super unlucky in that one
> > case. I've left devices rebooting all day at work and haven't seen
> > the wonky screen since then.
> >
> >
> > Summary: I think this revert is just fine.
>
> it looks like by picking Matthias' cleanups of the veyron displays
> first I broke this patch. I guess we just need to remove the
> startup-delay-us = <100000>;
> from the panel_regulator in the new rk3288-veyron-edp.dtsi ?
Oops, I only checked Matthias's change against the current status of
your for-next tree and forgot about this change. Yes, the
startup-delay should be removed there. Do you want to resolve that
when applying the patch or would you prefer a resend?
-Doug
^ permalink raw reply
* Re: [PATCH 02/11] MIPS: qi_lb60: Migrate to devicetree
From: Paul Burton @ 2019-07-25 23:47 UTC (permalink / raw)
To: Paul Cercueil
Cc: Ralf Baechle, James Hogan, Rob Herring, Mark Rutland, Vinod Koul,
Jean Delvare, Guenter Roeck, Lee Jones, Miquel Raynal,
Richard Weinberger, Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Liam Girdwood, Mark Brown, od@zcrc.me, devicetree@vger.kernel.org,
linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org,
"dmaengine@vger.kernel.org" <dmaengin>
In-Reply-To: <20190725220215.460-3-paul@crapouillou.net>
Hi Paul,
On Thu, Jul 25, 2019 at 06:02:06PM -0400, Paul Cercueil wrote:
> Move all the platform data to devicetree.
Nice! :)
> The only bit dropped is the PWM beeper, which requires the PWM driver
> to be updated. I figured it's okay to remove it here since it's really
> a non-critical device, and it'll be re-introduced soon enough.
OK, I can see that being a price worth paying. Though it's possible to
include the binding at least for that in this series I'd be even
happier. Actually I see we already have
Documentation/devicetree/bindings/pwm/ingenic,jz47xx-pwm.txt
in mainline - what needs to change with it?
> + spi {
> + compatible = "spi-gpio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sck-gpios = <&gpc 23 GPIO_ACTIVE_HIGH>;
> + mosi-gpios = <&gpc 22 GPIO_ACTIVE_HIGH>;
> + cs-gpios = <&gpc 21 GPIO_ACTIVE_LOW>;
> + num-chipselects = <1>;
> +
> + spi@0 {
> + compatible = "ili8960";
Should this be "ilitek,ili8960"?
Is there a binding & driver for this submitted somewhere? If not then do
we need this at all? It doesn't look like the existing platform data
would actually lead to a driver being loaded so I'm wondering if we can
just drop this until such a driver (or at least a documented DT binding)
exists.
Thanks,
Paul
^ permalink raw reply
* Re: [PATCH 02/11] MIPS: qi_lb60: Migrate to devicetree
From: Paul Cercueil @ 2019-07-26 0:09 UTC (permalink / raw)
To: Paul Burton
Cc: Ralf Baechle, James Hogan, Rob Herring, Mark Rutland, Vinod Koul,
Jean Delvare, Guenter Roeck, Lee Jones, Miquel Raynal,
Richard Weinberger, Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Liam Girdwood, Mark Brown, od, devicetree, linux-mips,
linux-kernel, dmaengine, linux-hwmon, linux-mtd, linux-pm,
dri-devel
In-Reply-To: <20190725234735.h7qmtt26qpkjw3n6@pburton-laptop>
Le jeu. 25 juil. 2019 à 19:47, Paul Burton <paul.burton@mips.com> a
écrit :
> Hi Paul,
>
> On Thu, Jul 25, 2019 at 06:02:06PM -0400, Paul Cercueil wrote:
>> Move all the platform data to devicetree.
>
> Nice! :)
>
>> The only bit dropped is the PWM beeper, which requires the PWM
>> driver
>> to be updated. I figured it's okay to remove it here since it's
>> really
>> a non-critical device, and it'll be re-introduced soon enough.
>
> OK, I can see that being a price worth paying. Though it's possible to
> include the binding at least for that in this series I'd be even
> happier. Actually I see we already have
>
> Documentation/devicetree/bindings/pwm/ingenic,jz47xx-pwm.txt
>
> in mainline - what needs to change with it?
The PWM driver will be updated to use the TCU clocks and the regmap
provided
by the TCU driver. The PWM node will be a sub-node of the TCU one.
Additionally, there is this[1] ongoing discussion about PWM which makes
me uneasy about how to write the binding. So I'd rather not rush it,
because once the devicetree is written, it's ABI.
[1]: https://lkml.org/lkml/2019/5/22/607
>> + spi {
>> + compatible = "spi-gpio";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + sck-gpios = <&gpc 23 GPIO_ACTIVE_HIGH>;
>> + mosi-gpios = <&gpc 22 GPIO_ACTIVE_HIGH>;
>> + cs-gpios = <&gpc 21 GPIO_ACTIVE_LOW>;
>> + num-chipselects = <1>;
>> +
>> + spi@0 {
>> + compatible = "ili8960";
>
> Should this be "ilitek,ili8960"?
>
> Is there a binding & driver for this submitted somewhere? If not then
> do
> we need this at all? It doesn't look like the existing platform data
> would actually lead to a driver being loaded so I'm wondering if we
> can
> just drop this until such a driver (or at least a documented DT
> binding)
> exists.
I can drop it. There is no driver for it, and I'm not even sure the LB60
has a ILI8960 in the first place.
> Thanks,
> Paul
^ permalink raw reply
* Re: [PATCH v2 2/2] mmc: Add support for the ASPEED SD controller
From: Andrew Jeffery @ 2019-07-26 0:52 UTC (permalink / raw)
To: Adrian Hunter, linux-mmc
Cc: mark.rutland, devicetree, Ulf Hansson, linux-aspeed, Ryan Chen,
linux-kernel, Rob Herring, Joel Stanley, linux-arm-kernel
In-Reply-To: <d6f7fdf2-07ed-354a-ca29-f3175623679c@intel.com>
On Thu, 25 Jul 2019, at 22:49, Adrian Hunter wrote:
> On 12/07/19 6:32 AM, Andrew Jeffery wrote:
> > Add a minimal driver for ASPEED's SD controller, which exposes two
> > SDHCIs.
> >
> > The ASPEED design implements a common register set for the SDHCIs, and
> > moves some of the standard configuration elements out to this common
> > area (e.g. 8-bit mode, and card detect configuration which is not
> > currently supported).
> >
> > The SD controller has a dedicated hardware interrupt that is shared
> > between the slots. The common register set exposes information on which
> > slot triggered the interrupt; early revisions of the patch introduced an
> > irqchip for the register, but reality is it doesn't behave as an
> > irqchip, and the result fits awkwardly into the irqchip APIs. Instead
> > I've taken the simple approach of using the IRQ as a shared IRQ with
> > some minor performance impact for the second slot.
> >
> > Ryan was the original author of the patch - I've taken his work and
> > massaged it to drop the irqchip support and rework the devicetree
> > integration. The driver has been smoke tested under qemu against a
> > minimal SD controller model and lightly tested on an ast2500-evb.
> >
> > Signed-off-by: Ryan Chen <ryanchen.aspeed@gmail.com>
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>
> Looks fine. Few minor comments below.
>
> > ---
> > In v2:
> >
> > * Drop unnecesasry MODULE_DEVICE_TABLE()
> > * Rename sd-controller compatible
> > * Add IBM copyright
> > * Drop unnecesary data assignment in of match table entries
> > * Derive the slot from the SDHCI offset
> >
> > drivers/mmc/host/Kconfig | 12 ++
> > drivers/mmc/host/Makefile | 1 +
> > drivers/mmc/host/sdhci-of-aspeed.c | 326 +++++++++++++++++++++++++++++
> > 3 files changed, 339 insertions(+)
> > create mode 100644 drivers/mmc/host/sdhci-of-aspeed.c
> >
> > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> > index 931770f17087..2bb5e1264b3d 100644
> > --- a/drivers/mmc/host/Kconfig
> > +++ b/drivers/mmc/host/Kconfig
> > @@ -154,6 +154,18 @@ config MMC_SDHCI_OF_ARASAN
> >
> > If unsure, say N.
> >
> > +config MMC_SDHCI_OF_ASPEED
> > + tristate "SDHCI OF support for the ASPEED SDHCI controller"
> > + depends on MMC_SDHCI_PLTFM
> > + depends on OF
> > + help
> > + This selects the ASPEED Secure Digital Host Controller Interface.
> > +
> > + If you have a controller with this interface, say Y or M here. You
> > + also need to enable an appropriate bus interface.
> > +
> > + If unsure, say N.
> > +
> > config MMC_SDHCI_OF_AT91
> > tristate "SDHCI OF support for the Atmel SDMMC controller"
> > depends on MMC_SDHCI_PLTFM
> > diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> > index 73578718f119..390ee162fe71 100644
> > --- a/drivers/mmc/host/Makefile
> > +++ b/drivers/mmc/host/Makefile
> > @@ -84,6 +84,7 @@ obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX) += sdhci-esdhc-imx.o
> > obj-$(CONFIG_MMC_SDHCI_DOVE) += sdhci-dove.o
> > obj-$(CONFIG_MMC_SDHCI_TEGRA) += sdhci-tegra.o
> > obj-$(CONFIG_MMC_SDHCI_OF_ARASAN) += sdhci-of-arasan.o
> > +obj-$(CONFIG_MMC_SDHCI_OF_ASPEED) += sdhci-of-aspeed.o
> > obj-$(CONFIG_MMC_SDHCI_OF_AT91) += sdhci-of-at91.o
> > obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o
> > obj-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o
> > diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
> > new file mode 100644
> > index 000000000000..9528e43c257d
> > --- /dev/null
> > +++ b/drivers/mmc/host/sdhci-of-aspeed.c
> > @@ -0,0 +1,326 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/* Copyright (C) 2019 ASPEED Technology Inc. */
> > +/* Copyright (C) 2019 IBM Corp. */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/io.h>
> > +#include <linux/mmc/host.h>
> > +#include <linux/module.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/spinlock.h>
> > +
> > +#include "sdhci-pltfm.h"
> > +
> > +#define ASPEED_SDC_INFO 0x00
> > +#define ASPEED_SDC_S1MMC8 BIT(25)
> > +#define ASPEED_SDC_S0MMC8 BIT(24)
> > +
> > +struct aspeed_sdc {
> > + struct clk *clk;
> > + struct resource *res;
> > +
> > + spinlock_t lock;
> > + void __iomem *regs;
> > +};
> > +
> > +struct aspeed_sdhci {
> > + struct aspeed_sdc *parent;
> > + u32 width_mask;
> > +};
> > +
> > +static void aspeed_sdc_bus_width(struct aspeed_sdc *sdc,
> > + struct aspeed_sdhci *sdhci, bool bus8)
>
> The function name threw me at first. I suggest:
>
> static void aspeed_sdhci_set_clr_8_bit_mode(struct aspeed_sdhci *aspeed_sdhci,
> bool bus8)
I wasn't real happy with the name I picked, so was hoping to some degree to be
bike-shedded on it. I'll fix it up.
> {
> struct aspeed_sdc *aspeed_sdc = aspeed_sdhci->parent;
>
> > +{
> > + u32 info;
> > +
> > + /* Set/clear 8 bit mode */
> > + spin_lock(&sdc->lock);
> > + info = readl(sdc->regs + ASPEED_SDC_INFO);
> > + if (bus8)
> > + info |= sdhci->width_mask;
> > + else
> > + info &= ~sdhci->width_mask;
> > + writel(info, sdc->regs + ASPEED_SDC_INFO);
> > + spin_unlock(&sdc->lock);
> > +}
> > +
> > +static void aspeed_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
> > +{
> > + unsigned long timeout;
> > + int div;
> > + u16 clk;
> > +
> > + if (clock == host->clock)
> > + return;
> > +
> > + sdhci_writew(host, 0, SDHCI_CLOCK_CONTROL);
> > +
> > + if (clock == 0)
> > + goto out;
> > +
> > + for (div = 1; div < 256; div *= 2) {
> > + if ((host->max_clk / div) <= clock)
> > + break;
> > + }
> > + div >>= 1;
> > +
> > + clk = div << SDHCI_DIVIDER_SHIFT;
>
> Could call sdhci_enable_clk() here.
I'll look into it.
>
> > + clk |= SDHCI_CLOCK_INT_EN;
> > + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> > +
> > + /* Wait max 20 ms */
> > + timeout = 20;
> > + while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> > + & SDHCI_CLOCK_INT_STABLE)) {
> > + if (timeout == 0) {
> > + pr_err("%s: Internal clock never stabilised.\n",
> > + mmc_hostname(host->mmc));
> > + return;
> > + }
> > + timeout--;
> > + mdelay(1);
> > + }
> > +
> > + clk |= SDHCI_CLOCK_CARD_EN;
> > + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> > +
> > +out:
> > + host->clock = clock;
> > +}
> > +
> > +static void aspeed_sdhci_set_bus_width(struct sdhci_host *host, int width)
> > +{
> > + struct sdhci_pltfm_host *pltfm_priv;
> > + struct aspeed_sdhci *aspeed_sdhci;
> > + struct aspeed_sdc *aspeed_sdc;
> > + u8 ctrl;
> > +
> > + pltfm_priv = sdhci_priv(host);
> > + aspeed_sdhci = sdhci_pltfm_priv(pltfm_priv);
> > + aspeed_sdc = aspeed_sdhci->parent;
> > +
> > + /* Set/clear 8-bit mode */
> > + aspeed_sdc_bus_width(aspeed_sdc, aspeed_sdhci,
> > + width == MMC_BUS_WIDTH_8);
> > +
> > + /* Set/clear 1 or 4 bit mode */
> > + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> > + if (width == MMC_BUS_WIDTH_4)
> > + ctrl |= SDHCI_CTRL_4BITBUS;
> > + else
> > + ctrl &= ~SDHCI_CTRL_4BITBUS;
> > + sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> > +}
> > +
> > +static const struct sdhci_ops aspeed_sdhci_ops = {
> > + .set_clock = aspeed_sdhci_set_clock,
> > + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> > + .set_bus_width = aspeed_sdhci_set_bus_width,
> > + .get_timeout_clock = sdhci_pltfm_clk_get_max_clock,
> > + .reset = sdhci_reset,
> > + .set_uhs_signaling = sdhci_set_uhs_signaling,
> > +};
> > +
> > +static const struct sdhci_pltfm_data aspeed_sdc_pdata = {
>
> Up to you, but this is for the aspeed_sdhci driver, so I would
> have expected it to be called aspeed_sdhci_pdata
Uh, yeah. Good catch.
>
> > + .ops = &aspeed_sdhci_ops,
> > + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> > + .quirks2 = SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
>
> You don't use sdhci_set_clock() or sdhci_calc_clk(), so it doesn't
> look like SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN is needed.
I did look at that and was a bit suspicious about it, good to have my suspicion
confirmed.
>
> > +};
> > +
> > +static inline int aspeed_sdhci_calculate_slot(struct aspeed_sdhci *dev,
> > + struct resource *res)
> > +{
> > + resource_size_t delta;
> > +
> > + if (!res || resource_type(res) != IORESOURCE_MEM)
> > + return -EINVAL;
> > +
> > + if (res->start < dev->parent->res->start)
> > + return -EINVAL;
> > +
> > + delta = res->start - dev->parent->res->start;
> > + if (delta & (0x100 - 1))
> > + return -EINVAL;
> > +
> > + return (delta / 0x100) - 1;
> > +}
> > +
> > +static int aspeed_sdhci_probe(struct platform_device *pdev)
> > +{
> > + struct sdhci_pltfm_host *pltfm_host;
> > + struct aspeed_sdhci *dev;
> > + struct sdhci_host *host;
> > + struct resource *res;
> > + int slot;
> > + int ret;
> > +
> > + host = sdhci_pltfm_init(pdev, &aspeed_sdc_pdata, sizeof(*dev));
> > + if (IS_ERR(host))
> > + return PTR_ERR(host);
> > +
> > + pltfm_host = sdhci_priv(host);
> > + dev = sdhci_pltfm_priv(pltfm_host);
> > + dev->parent = dev_get_drvdata(pdev->dev.parent);
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + slot = aspeed_sdhci_calculate_slot(dev, res);
> > + if (slot < 0)
> > + return slot;
> > + dev_info(&pdev->dev, "Configuring for slot %d\n", slot);
> > + dev->width_mask = !slot ? ASPEED_SDC_S0MMC8 : ASPEED_SDC_S1MMC8;
>
> That implies that you only support 2 slots which begs the question why
> you don't validate slot.
I'm not sure what you mean here, but I'll dig into it.
>
> > +
> > + sdhci_get_of_property(pdev);
> > +
> > + pltfm_host->clk = devm_clk_get(&pdev->dev, NULL);
> > + if (IS_ERR(pltfm_host->clk))
> > + return PTR_ERR(pltfm_host->clk);
> > +
> > + ret = clk_prepare_enable(pltfm_host->clk);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Unable to enable SDIO clock\n");
> > + goto err_pltfm_free;
> > + }
> > +
> > + ret = mmc_of_parse(host->mmc);
> > + if (ret)
> > + goto err_sdhci_add;
> > +
> > + ret = sdhci_add_host(host);
> > + if (ret)
> > + goto err_sdhci_add;
> > +
> > + return 0;
> > +
> > +err_sdhci_add:
> > + clk_disable_unprepare(pltfm_host->clk);
> > +err_pltfm_free:
> > + sdhci_pltfm_free(pdev);
> > + return ret;
> > +}
> > +
> > +static int aspeed_sdhci_remove(struct platform_device *pdev)
> > +{
> > + struct sdhci_pltfm_host *pltfm_host;
> > + struct sdhci_host *host;
> > + int dead;
> > +
> > + host = platform_get_drvdata(pdev);
> > + pltfm_host = sdhci_priv(host);
> > +
> > + dead = readl(host->ioaddr + SDHCI_INT_STATUS) == 0xffffffff;
>
> 'dead' only makes sense for PCI. Just set it to zero.
Ack.
Thanks for the feedback, I'll send a v3.
Andrew
>
> > +
> > + sdhci_remove_host(host, dead);
> > +
> > + clk_disable_unprepare(pltfm_host->clk);
> > +
> > + sdhci_pltfm_free(pdev);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id aspeed_sdhci_of_match[] = {
> > + { .compatible = "aspeed,ast2400-sdhci", },
> > + { .compatible = "aspeed,ast2500-sdhci", },
> > + { }
> > +};
> > +
> > +static struct platform_driver aspeed_sdhci_driver = {
> > + .driver = {
> > + .name = "sdhci-aspeed",
> > + .of_match_table = aspeed_sdhci_of_match,
> > + },
> > + .probe = aspeed_sdhci_probe,
> > + .remove = aspeed_sdhci_remove,
> > +};
> > +
> > +module_platform_driver(aspeed_sdhci_driver);
> > +
> > +static int aspeed_sdc_probe(struct platform_device *pdev)
> > +
> > +{
> > + struct device_node *parent, *child;
> > + struct aspeed_sdc *sdc;
> > + int ret;
> > +
> > + sdc = devm_kzalloc(&pdev->dev, sizeof(*sdc), GFP_KERNEL);
> > + if (!sdc)
> > + return -ENOMEM;
> > +
> > + spin_lock_init(&sdc->lock);
> > +
> > + sdc->clk = devm_clk_get(&pdev->dev, NULL);
> > + if (IS_ERR(sdc->clk))
> > + return PTR_ERR(sdc->clk);
> > +
> > + ret = clk_prepare_enable(sdc->clk);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Unable to enable SDCLK\n");
> > + return ret;
> > + }
> > +
> > + sdc->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + sdc->regs = devm_ioremap_resource(&pdev->dev, sdc->res);
> > + if (IS_ERR(sdc->regs)) {
> > + ret = PTR_ERR(sdc->regs);
> > + goto err_clk;
> > + }
> > +
> > + dev_set_drvdata(&pdev->dev, sdc);
> > +
> > + parent = pdev->dev.of_node;
> > + for_each_available_child_of_node(parent, child) {
> > + struct platform_device *cpdev;
> > +
> > + cpdev = of_platform_device_create(child, NULL, &pdev->dev);
> > + if (IS_ERR(cpdev)) {
> > + of_node_put(child);
> > + ret = PTR_ERR(pdev);
> > + goto err_clk;
> > + }
> > + }
> > +
> > + return 0;
> > +
> > +err_clk:
> > + clk_disable_unprepare(sdc->clk);
> > + return ret;
> > +}
> > +
> > +static int aspeed_sdc_remove(struct platform_device *pdev)
> > +{
> > + struct aspeed_sdc *sdc = dev_get_drvdata(&pdev->dev);
> > +
> > + clk_disable_unprepare(sdc->clk);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id aspeed_sdc_of_match[] = {
> > + { .compatible = "aspeed,ast2400-sd-controller", },
> > + { .compatible = "aspeed,ast2500-sd-controller", },
> > + { }
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, aspeed_sdc_of_match);
> > +
> > +static struct platform_driver aspeed_sdc_driver = {
> > + .driver = {
> > + .name = "sd-controller-aspeed",
> > + .pm = &sdhci_pltfm_pmops,
> > + .of_match_table = aspeed_sdc_of_match,
> > + },
> > + .probe = aspeed_sdc_probe,
> > + .remove = aspeed_sdc_remove,
> > +};
> > +
> > +module_platform_driver(aspeed_sdc_driver);
> > +
> > +MODULE_DESCRIPTION("Driver for the ASPEED SD/SDIO/SDHCI Controllers");
> > +MODULE_AUTHOR("Ryan Chen <ryan_chen@aspeedtech.com>");
> > +MODULE_AUTHOR("Andrew Jeffery <andrew@aj.id.au>");
> > +MODULE_LICENSE("GPL v2");
> >
>
>
^ permalink raw reply
* Re: [PATCH v3 3/5] devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
From: Chanwoo Choi @ 2019-07-26 1:02 UTC (permalink / raw)
To: Kamil Konieczny, cwchoi00
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Krzysztof Kozlowski,
Kukjin Kim, Kyungmin Park, Mark Rutland, MyungJoo Ham,
Nishanth Menon, Rob Herring, Stephen Boyd, Viresh Kumar,
devicetree, linux-arm-kernel, linux-kernel, Linux PM list,
linux-samsung-soc
In-Reply-To: <4f62d397-0f62-f81d-8b76-b73c6fbee93d@partner.samsung.com>
Hi,
On 19. 7. 26. 오전 12:15, Kamil Konieczny wrote:
> Hi,
>
> On 25.07.2019 16:53, Chanwoo Choi wrote:
>> 2019년 7월 25일 (목) 오후 11:19, Kamil Konieczny
>> <k.konieczny@partner.samsung.com>님이 작성:
>>>
>>> Hi Chanwoo,
>>>
>>> On 25.07.2019 12:17, Chanwoo Choi wrote:
>>>> Hi Kamil,
>>>>
>>>> Looks good to me. But, I have some comment. Please check them.
>>>
>>> Thank you for review, please see answers below.
>>>
>>>> After this patch, exynos_bus_target is perfectly same with
>>>> exynos_bus_passive_target. The exynos_bus_passive_target() could be removed.
>>>
>>> Ok, I will make it in separate patch to simplify review process.
>>
>> I think you can just modify this patch without any separate patch.
>
> Do you want me to send v5 with patch 5 squashed in patch 3 ?
Yes. In result, it made two functions same by this patch
So, I think that can combine them without separate patch.
>
>>>> On 19. 7. 20. 오전 12:05, k.konieczny@partner.samsung.com wrote:
>>>>> Reuse opp core code for setting bus clock and voltage. As a side
>>>>> effect this allow useage of coupled regulators feature (required
>>>>
>>>> s/useage/usage ?
>>>
>>> Good catch, I will change it.
>>>
>>>>> for boards using Exynos5422/5800 SoCs) because dev_pm_opp_set_rate()
>>>>> uses regulator_set_voltage_triplet() for setting regulator voltage
>>>>> while the old code used regulator_set_voltage_tol() with fixed
>>>>> tolerance. This patch also removes no longer needed parsing of DT
>>>>> property "exynos,voltage-tolerance" (no Exynos devfreq DT node uses
>>>>> it).
>>>>>
>>>>> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
>>>>> ---
>>>>> drivers/devfreq/exynos-bus.c | 143 +++++++++--------------------------
>>>>> 1 file changed, 37 insertions(+), 106 deletions(-)
>>>>>
>>>>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>>>>> index f391044aa39d..c2147b0912a0 100644
>>>>> --- a/drivers/devfreq/exynos-bus.c
>>>>> +++ b/drivers/devfreq/exynos-bus.c
>>>>> @@ -25,7 +25,6 @@
>>>>> #include <linux/slab.h>
>>>>>
>>>>> #define DEFAULT_SATURATION_RATIO 40
>>>>> -#define DEFAULT_VOLTAGE_TOLERANCE 2
>>>>>
>>>>> struct exynos_bus {
>>>>> struct device *dev;
>>>>> @@ -37,9 +36,9 @@ struct exynos_bus {
>>>>>
>>>>> unsigned long curr_freq;
>>>>>
>>>>> - struct regulator *regulator;
>>>>> + struct opp_table *opp_table;
>>>>> +
>>>>> struct clk *clk;
>>>>> - unsigned int voltage_tolerance;
>>>>> unsigned int ratio;
>>>>> };
>>>>>
>>>>> @@ -99,56 +98,23 @@ static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 flags)
>>>>> {
>>>>> struct exynos_bus *bus = dev_get_drvdata(dev);
>>>>> struct dev_pm_opp *new_opp;
>>>>> - unsigned long old_freq, new_freq, new_volt, tol;
>>>>> int ret = 0;
>>>>>
>>>>> - /* Get new opp-bus instance according to new bus clock */
>>>>> + /* Get correct frequency for bus. */
>>>>> new_opp = devfreq_recommended_opp(dev, freq, flags);
>>>>> if (IS_ERR(new_opp)) {
>>>>> dev_err(dev, "failed to get recommended opp instance\n");
>>>>> return PTR_ERR(new_opp);
>>>>> }
>>>>>
>>>>> - new_freq = dev_pm_opp_get_freq(new_opp);
>>>>> - new_volt = dev_pm_opp_get_voltage(new_opp);
>>>>> dev_pm_opp_put(new_opp);
>>>>>
>>>>> - old_freq = bus->curr_freq;
>>>>> -
>>>>> - if (old_freq == new_freq)
>>>>> - return 0;
>>>>> - tol = new_volt * bus->voltage_tolerance / 100;
>>>>> -
>>>>> /* Change voltage and frequency according to new OPP level */
>>>>> mutex_lock(&bus->lock);
>>>>> + ret = dev_pm_opp_set_rate(dev, *freq);
>>>>> + if (!ret)
>>>>> + bus->curr_freq = *freq;
>>>>>
>>>>> - if (old_freq < new_freq) {
>>>>> - ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
>>>>> - if (ret < 0) {
>>>>> - dev_err(bus->dev, "failed to set voltage\n");
>>>>> - goto out;
>>>>> - }
>>>>> - }
>>>>> -
>>>>> - ret = clk_set_rate(bus->clk, new_freq);
>>>>> - if (ret < 0) {
>>>>> - dev_err(dev, "failed to change clock of bus\n");
>>>>> - clk_set_rate(bus->clk, old_freq);
>>>>> - goto out;
>>>>> - }
>>>>> -
>>>>> - if (old_freq > new_freq) {
>>>>> - ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
>>>>> - if (ret < 0) {
>>>>> - dev_err(bus->dev, "failed to set voltage\n");
>>>>> - goto out;
>>>>> - }
>>>>> - }
>>>>> - bus->curr_freq = new_freq;
>>>>> -
>>>>> - dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
>>>>> - old_freq, new_freq, clk_get_rate(bus->clk));
>>>>> -out:
>>>>> mutex_unlock(&bus->lock);
>>>>>
>>>>> return ret;
>>>>> @@ -195,8 +161,8 @@ static void exynos_bus_exit(struct device *dev)
>>>>> dev_warn(dev, "failed to disable the devfreq-event devices\n");
>>>>>
>>>>> clk_disable_unprepare(bus->clk);
>>>>> - if (bus->regulator)
>>>>> - regulator_disable(bus->regulator);
>>>>> + if (bus->opp_table)
>>>>> + dev_pm_opp_put_regulators(bus->opp_table);
>>>>>
>>>>> dev_pm_opp_of_remove_table(dev);
>>>>> }
>>>>> @@ -209,39 +175,23 @@ static int exynos_bus_passive_target(struct device *dev, unsigned long *freq,
>>>>> {
>>>>> struct exynos_bus *bus = dev_get_drvdata(dev);
>>>>> struct dev_pm_opp *new_opp;
>>>>> - unsigned long old_freq, new_freq;
>>>>> - int ret = 0;
>>>>> + int ret;
>>>>>
>>>>> - /* Get new opp-bus instance according to new bus clock */
>>>>> + /* Get correct frequency for bus. */
>>>>> new_opp = devfreq_recommended_opp(dev, freq, flags);
>>>>> if (IS_ERR(new_opp)) {
>>>>> dev_err(dev, "failed to get recommended opp instance\n");
>>>>> return PTR_ERR(new_opp);
>>>>> }
>>>>>
>>>>> - new_freq = dev_pm_opp_get_freq(new_opp);
>>>>> dev_pm_opp_put(new_opp);
>>>>>
>>>>> - old_freq = bus->curr_freq;
>>>>> -
>>>>> - if (old_freq == new_freq)
>>>>> - return 0;
>>>>> -
>>>>> /* Change the frequency according to new OPP level */
>>>>> mutex_lock(&bus->lock);
>>>>> + ret = dev_pm_opp_set_rate(dev, *freq);
>>>>> + if (!ret)
>>>>> + bus->curr_freq = *freq;
>>>>>
>>>>> - ret = clk_set_rate(bus->clk, new_freq);
>>>>> - if (ret < 0) {
>>>>> - dev_err(dev, "failed to set the clock of bus\n");
>>>>> - goto out;
>>>>> - }
>>>>> -
>>>>> - *freq = new_freq;
>>>>> - bus->curr_freq = new_freq;
>>>>> -
>>>>> - dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
>>>>> - old_freq, new_freq, clk_get_rate(bus->clk));
>>>>> -out:
>>>>> mutex_unlock(&bus->lock);
>>>>>
>>>>> return ret;
>>>>> @@ -259,20 +209,9 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>>>>> struct exynos_bus *bus)
>>>>> {
>>>>> struct device *dev = bus->dev;
>>>>> - int i, ret, count, size;
>>>>> -
>>>>> - /* Get the regulator to provide each bus with the power */
>>>>> - bus->regulator = devm_regulator_get(dev, "vdd");
>>>>> - if (IS_ERR(bus->regulator)) {
>>>>> - dev_err(dev, "failed to get VDD regulator\n");
>>>>> - return PTR_ERR(bus->regulator);
>>>>> - }
>>>>> -
>>>>> - ret = regulator_enable(bus->regulator);
>>>>> - if (ret < 0) {
>>>>> - dev_err(dev, "failed to enable VDD regulator\n");
>>>>> - return ret;
>>>>> - }
>>>>> + struct opp_table *opp_table;
>>>>> + const char *vdd = "vdd";
>>>>> + int i, count, size;
>>>>>
>>>>> /*
>>>>> * Get the devfreq-event devices to get the current utilization of
>>>>> @@ -281,26 +220,29 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>>>>> count = devfreq_event_get_edev_count(dev);
>>>>> if (count < 0) {
>>>>> dev_err(dev, "failed to get the count of devfreq-event dev\n");
>>>>> - ret = count;
>>>>> - goto err_regulator;
>>>>> + return count;
>>>>> }
>>>>> - bus->edev_count = count;
>>>>>
>>>>> + bus->edev_count = count;
>>>>> size = sizeof(*bus->edev) * count;
>>>>> bus->edev = devm_kzalloc(dev, size, GFP_KERNEL);
>>>>> - if (!bus->edev) {
>>>>> - ret = -ENOMEM;
>>>>> - goto err_regulator;
>>>>> - }
>>>>> + if (!bus->edev)
>>>>> + return -ENOMEM;
>>>>>
>>>>> for (i = 0; i < count; i++) {
>>>>> bus->edev[i] = devfreq_event_get_edev_by_phandle(dev, i);
>>>>> - if (IS_ERR(bus->edev[i])) {
>>>>> - ret = -EPROBE_DEFER;
>>>>> - goto err_regulator;
>>>>> - }
>>>>> + if (IS_ERR(bus->edev[i]))
>>>>> + return -EPROBE_DEFER;
>>>>> + }
>>>>> +
>>>>> + opp_table = dev_pm_opp_set_regulators(dev, &vdd, 1);
>>>>> + if (IS_ERR(opp_table)) {
>>>>> + i = PTR_ERR(opp_table);
>>>>> + dev_err(dev, "failed to set regulators %d\n", i);
>>>>> + return i;
>>>>
>>>> Maybe, you just used the 'i' defined variable instead of adding
>>>> new variable. But, I think that you better to add new variable
>>>> like 'err' for the readability. Or, jut use the 'PTR_ERR(opp_table)'
>>>> directly without any additional variable.
>>>
>>> I will remove not related changes, so here I will reuse 'ret' variable.
>>>
>>>>> }
>>>>>
>>>>> + bus->opp_table = opp_table;
>>>>
>>>> Add blank line.
>>>
>>> OK
>>>
>>>>> /*
>>>>> * Optionally, Get the saturation ratio according to Exynos SoC
>>>>> * When measuring the utilization of each AXI bus with devfreq-event
>>>>> @@ -314,16 +256,7 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>>>>> if (of_property_read_u32(np, "exynos,saturation-ratio", &bus->ratio))
>>>>> bus->ratio = DEFAULT_SATURATION_RATIO;
>>>>>
>>>>> - if (of_property_read_u32(np, "exynos,voltage-tolerance",
>>>>> - &bus->voltage_tolerance))
>>>>> - bus->voltage_tolerance = DEFAULT_VOLTAGE_TOLERANCE;
>>>>> -
>>>>> return 0;
>>>>> -
>>>>> -err_regulator:
>>>>> - regulator_disable(bus->regulator);
>>>>> -
>>>>> - return ret;
>>>>> }
>>>>>
>>>>> static int exynos_bus_parse_of(struct exynos_bus *bus)
>>>>> @@ -414,12 +347,8 @@ static int exynos_bus_probe(struct platform_device *pdev)
>>>>>
>>>>> /* Parse the device-tree to get the resource information */
>>>>> ret = exynos_bus_parse_of(bus);
>>>>> - if (ret < 0) {
>>>>> - if (!passive)
>>>>> - regulator_disable(bus->regulator);
>>>>> -
>>>>> - return ret;
>>>>> - }
>>>>> + if (ret < 0)
>>>>> + goto err_reg;
>>>>>
>>>>> if (passive)
>>>>> goto passive;
>>>>> @@ -512,10 +441,12 @@ static int exynos_bus_probe(struct platform_device *pdev)
>>>>>
>>>>> err:
>>>>> clk_disable_unprepare(bus->clk);
>>>>> - if (!passive)
>>>>> - regulator_disable(bus->regulator);
>>>>> -
>>>>> dev_pm_opp_of_remove_table(dev);
>>>>
>>>> This function removes the 'opp_table'. But, the below code
>>>> uses the 'opp_table' variable by dev_pm_opp_put_regulators().
>>>>
>>>> To disable the regulator, you have to call dev_pm_opp_of_remove_table(dev)
>>>> after dev_pm_opp_put_regulators(bus->opp_table).
>>>
>>> Regulators should be set before dev_pm_opp_add_table(), so exit sequence
>>> should be in reverse order,
>>>
>>> init order:
>>>
>>> exynos_bus_parent_parse_of()
>>> dev_pm_opp_set_regulators()
>>> exynos_bus_parse_of()
>>> clk_prepare_enable()
>>> dev_pm_opp_of_add_table()
>>>
>>> exit or error order:
>>>
>>> dev_pm_opp_of_remove_table()
>>> clk_disable_unprepare()
>>> if (bus->opp_table)
>>> dev_pm_opp_put_regulators(bus->opp_table);
>>
>> dev_pm_opp_of_remove_table() have to be called at the end of exit sequence
>> after disabling clock and put regulators. Because dev_pm_opp_of_remove_table()
>> frees the 'opp_table' pointer of device.
>>
>> clk_disable_unprepare()
>> if (bus->opp_table)
>> dev_pm_opp_put_regulators(bus->opp_table);
>> dev_pm_opp_of_remove_table()
>
> The table is reference counted so it will be freed after count go to zero.
You're right. I checked it with OPP code.
>
>>> I will send v4 soon.
>>>
>>>>> +err_reg:
>>>>> + if (bus->opp_table) {
>>>>> + dev_pm_opp_put_regulators(bus->opp_table);
>>>>> + bus->opp_table = NULL;
>>>>> + }
>>>>>
>>>>> return ret;
>>>>> }
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply
* RE: [PATCH v2 2/2] dt-bindings: usb: renesas_gen3: Rename bindings documentation file
From: Yoshihiro Shimoda @ 2019-07-26 1:22 UTC (permalink / raw)
To: Greg Kroah-Hartman, Simon Horman
Cc: Geert Uytterhoeven, Kuninori Morimoto, Magnus Damm, USB list,
Linux-Renesas, Rob Herring, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>, Niklas Söderlund
In-Reply-To: <20190725090946.GA3311@kroah.com>
Hi Greg,
> From: Greg Kroah-Hartman, Sent: Thursday, July 25, 2019 6:10 PM
>
> On Thu, Jul 11, 2019 at 10:03:03AM +0200, Simon Horman wrote:
> > On Wed, Jul 03, 2019 at 02:28:51PM +0200, Geert Uytterhoeven wrote:
> > > Hi Simon,
> > >
> > > On Wed, Jul 3, 2019 at 10:35 AM Simon Horman <horms+renesas@verge.net.au> wrote:
> > > > For consistency with the naming of (most) other documentation files for DT
> > > > bindings for Renesas IP blocks rename the Renesas USB3.0 peripheral
> > > > documentation file from renesas-gen3.txt to renesas,usb3-peri.txt
> > > >
> > > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > > Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > > Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > >
> > > > ---
> > > > v2
> > > > * Accumulate review tags
> > > > * Use renesas,usb3-peri.txt as new filename as suggested by Shimoda-san
> > >
> > > Unfortunately the previous version has already made it into usb-next
> > > 23c46801d14cb647 dt-bindings: usb: renesas_gen3: Rename bindings
> > > documentation file
> >
> > Ok, I guess we should go with that version.
>
> So can you resend this series based on 5.3-rc1 so I know what to apply?
Since your usb-testing branch already has it which is merged from Felipe's usb-next branch,
I don't think Simon has to resend this series.
https://www.spinics.net/lists/linux-usb/msg182103.html
Best regards,
Yoshihiro Shimoda
> thanks,
>
> greg k-h
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox