* [PATCH] cpufreq: Add Kryo CPU scaling driver
From: Ilia Lin @ 2018-05-19 11:35 UTC (permalink / raw)
To: mturquette, sboyd, robh, mark.rutland, viresh.kumar, nm,
lgirdwood, broonie, andy.gross, david.brown, catalin.marinas,
will.deacon, rjw, linux-clk
Cc: devicetree, linux-kernel, linux-pm, linux-arm-msm, linux-soc,
linux-arm-kernel, rnayak, ilialin, amit.kucheria,
nicolas.dechesne, celster, tfinkel
In-Reply-To: <1526555955-29960-11-git-send-email-ilialin@codeaurora.org>
In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
the CPU frequency subset and voltage value of each OPP varies
based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables
defines the voltage and frequency value based on the msm-id in SMEM
and speedbin blown in the efuse combination.
The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
to provide the OPP framework with required information.
This is used to determine the voltage and frequency value for each OPP of
operating-points-v2 table when it is parsed by the OPP framework.
Signed-off-by: Ilia Lin <ilialin@codeaurora.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/Kconfig.arm | 10 +++
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/cpufreq-dt-platdev.c | 3 +
drivers/cpufreq/qcom-cpufreq-kryo.c | 164 +++++++++++++++++++++++++++++++++++
4 files changed, 178 insertions(+)
create mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index de55c7d..0bfd40e 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -124,6 +124,16 @@ config ARM_OMAP2PLUS_CPUFREQ
depends on ARCH_OMAP2PLUS
default ARCH_OMAP2PLUS
+config ARM_QCOM_CPUFREQ_KRYO
+ bool "Qualcomm Kryo based CPUFreq"
+ depends on QCOM_QFPROM
+ depends on QCOM_SMEM
+ select PM_OPP
+ help
+ This adds the CPUFreq driver for Qualcomm Kryo SoC based boards.
+
+ If in doubt, say N.
+
config ARM_S3C_CPUFREQ
bool
help
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 8d24ade..fb4a2ec 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7) += mvebu-cpufreq.o
obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o
obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o
obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO) += qcom-cpufreq-kryo.o
obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o
obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o
obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index 3b585e4..77d6ab8 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -118,6 +118,9 @@
{ .compatible = "nvidia,tegra124", },
+ { .compatible = "qcom,apq8096", },
+ { .compatible = "qcom,msm8996", },
+
{ .compatible = "st,stih407", },
{ .compatible = "st,stih410", },
diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-kryo.c
new file mode 100644
index 0000000..ae2d1b9
--- /dev/null
+++ b/drivers/cpufreq/qcom-cpufreq-kryo.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+/*
+ * In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
+ * the CPU frequency subset and voltage value of each OPP varies
+ * based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables
+ * defines the voltage and frequency value based on the msm-id in SMEM
+ * and speedbin blown in the efuse combination.
+ * The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
+ * to provide the OPP framework with required information.
+ * This is used to determine the voltage and frequency value for each OPP of
+ * operating-points-v2 table when it is parsed by the OPP framework.
+ */
+
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/smem.h>
+
+#define MSM_ID_SMEM 137
+#define SILVER_LEAD 0
+#define GOLD_LEAD 2
+
+enum _msm_id {
+ MSM8996V3 = 0xF6ul,
+ APQ8096V3 = 0x123ul,
+ MSM8996SG = 0x131ul,
+ APQ8096SG = 0x138ul,
+};
+
+enum _msm8996_version {
+ MSM8996_V3,
+ MSM8996_SG,
+ NUM_OF_MSM8996_VERSIONS,
+};
+
+static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void)
+{
+ size_t len;
+ u32 *msm_id;
+ enum _msm8996_version version;
+
+ msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY, MSM_ID_SMEM, &len);
+ /* The first 4 bytes are format, next to them is the actual msm-id */
+ msm_id++;
+
+ switch ((enum _msm_id)*msm_id) {
+ case MSM8996V3:
+ case APQ8096V3:
+ version = MSM8996_V3;
+ break;
+ case MSM8996SG:
+ case APQ8096SG:
+ version = MSM8996_SG;
+ break;
+ default:
+ version = NUM_OF_MSM8996_VERSIONS;
+ }
+
+ return version;
+}
+
+static int __init qcom_cpufreq_kryo_driver_init(void)
+{
+ struct device *cpu_dev_silver, *cpu_dev_gold;
+ struct opp_table *opp_silver, *opp_gold;
+ enum _msm8996_version msm8996_version;
+ struct nvmem_cell *speedbin_nvmem;
+ struct platform_device *pdev;
+ struct device_node *np;
+ u8 *speedbin;
+ u32 versions;
+ size_t len;
+ int ret;
+
+ cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+ if (IS_ERR_OR_NULL(cpu_dev_silver))
+ return PTR_ERR(cpu_dev_silver);
+
+ cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+ if (IS_ERR_OR_NULL(cpu_dev_gold))
+ return PTR_ERR(cpu_dev_gold);
+
+ msm8996_version = qcom_cpufreq_kryo_get_msm_id();
+ if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
+ dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
+ return -ENODEV;
+ }
+
+ np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
+ if (IS_ERR_OR_NULL(np))
+ return PTR_ERR(np);
+
+ if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
+ ret = -ENOENT;
+ goto free_np;
+ }
+
+ speedbin_nvmem = of_nvmem_cell_get(np, NULL);
+ if (IS_ERR(speedbin_nvmem)) {
+ ret = PTR_ERR(speedbin_nvmem);
+ dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n", ret);
+ goto free_np;
+ }
+
+ speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+ nvmem_cell_put(speedbin_nvmem);
+
+ switch (msm8996_version) {
+ case MSM8996_V3:
+ versions = 1 << (unsigned int)(*speedbin);
+ break;
+ case MSM8996_SG:
+ versions = 1 << ((unsigned int)(*speedbin) + 4);
+ break;
+ default:
+ BUG();
+ break;
+ }
+
+ opp_silver = dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);
+ if (IS_ERR(opp_silver)) {
+ dev_err(cpu_dev_silver, "Failed to set supported hardware\n");
+ ret = PTR_ERR(opp_silver);
+ goto free_np;
+ }
+
+ opp_gold = dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);
+ if (IS_ERR(opp_gold)) {
+ dev_err(cpu_dev_gold, "Failed to set supported hardware\n");
+ ret = PTR_ERR(opp_gold);
+ goto free_opp_silver;
+ }
+
+ pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+ if (!IS_ERR_OR_NULL(pdev))
+ return 0;
+
+ ret = PTR_ERR(pdev);
+ dev_err(cpu_dev_silver, "Failed to register platform device\n");
+ dev_pm_opp_put_supported_hw(opp_gold);
+
+free_opp_silver:
+ dev_pm_opp_put_supported_hw(opp_silver);
+
+free_np:
+ of_node_put(np);
+
+ return ret;
+}
+late_initcall(qcom_cpufreq_kryo_driver_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
^ permalink raw reply related
* RE: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver
From: ilialin @ 2018-05-19 11:09 UTC (permalink / raw)
To: 'Viresh Kumar'
Cc: mturquette, sboyd, robh, mark.rutland, nm, lgirdwood, broonie,
andy.gross, david.brown, catalin.marinas, will.deacon, rjw,
linux-clk, devicetree, linux-kernel, linux-pm, linux-arm-msm,
linux-soc, linux-arm-kernel, rnayak, amit.kucheria,
nicolas.dechesne, celster, tfinkel
In-Reply-To: <20180518014538.duphof6enscpm5vp@vireshk-i7>
commit 4abe2cd7176a43c77e9a462e4f6ec51aa7552e73
Author: Ilia Lin <ilialin@codeaurora.org>
Date: Thu May 17 13:55:12 2018 +0300
cpufreq: Add Kryo CPU scaling driver
In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
the CPU frequency subset and voltage value of each OPP varies
based on the silicon variant in use. Qualcomm Process Voltage Scaling
Tables
defines the voltage and frequency value based on the msm-id in SMEM
and speedbin blown in the efuse combination.
The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the
SoC
to provide the OPP framework with required information.
This is used to determine the voltage and frequency value for each OPP
of
operating-points-v2 table when it is parsed by the OPP framework.
Signed-off-by: Ilia Lin <ilialin@codeaurora.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index de55c7d..0bfd40e 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -124,6 +124,16 @@ config ARM_OMAP2PLUS_CPUFREQ
depends on ARCH_OMAP2PLUS
default ARCH_OMAP2PLUS
+config ARM_QCOM_CPUFREQ_KRYO
+ bool "Qualcomm Kryo based CPUFreq"
+ depends on QCOM_QFPROM
+ depends on QCOM_SMEM
+ select PM_OPP
+ help
+ This adds the CPUFreq driver for Qualcomm Kryo SoC based boards.
+
+ If in doubt, say N.
+
config ARM_S3C_CPUFREQ
bool
help
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 8d24ade..fb4a2ec 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_MACH_MVEBU_V7) += mvebu-cpufreq.o
obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o
obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o
obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO) += qcom-cpufreq-kryo.o
obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o
obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o
obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c
b/drivers/cpufreq/cpufreq-dt-platdev.c
index 3b585e4..77d6ab8 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -118,6 +118,9 @@
{ .compatible = "nvidia,tegra124", },
+ { .compatible = "qcom,apq8096", },
+ { .compatible = "qcom,msm8996", },
+
{ .compatible = "st,stih407", },
{ .compatible = "st,stih410", },
diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c
b/drivers/cpufreq/qcom-cpufreq-kryo.c
new file mode 100644
index 0000000..b024b23
--- /dev/null
+++ b/drivers/cpufreq/qcom-cpufreq-kryo.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+/*
+ * In Certain QCOM SoCs like apq8096 and msm8996 that have KRYO processors,
+ * the CPU frequency subset and voltage value of each OPP varies
+ * based on the silicon variant in use. Qualcomm Process Voltage Scaling
Tables
+ * defines the voltage and frequency value based on the msm-id in SMEM
+ * and speedbin blown in the efuse combination.
+ * The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the
SoC
+ * to provide the OPP framework with required information.
+ * This is used to determine the voltage and frequency value for each OPP
of
+ * operating-points-v2 table when it is parsed by the OPP framework.
+ */
+
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/smem.h>
+
+#define MSM_ID_SMEM 137
+#define SILVER_LEAD 0
+#define GOLD_LEAD 2
+
+enum _msm_id {
+ MSM8996V3 = 0xF6ul,
+ APQ8096V3 = 0x123ul,
+ MSM8996SG = 0x131ul,
+ APQ8096SG = 0x138ul,
+};
+
+enum _msm8996_version {
+ MSM8996_V3,
+ MSM8996_SG,
+ NUM_OF_MSM8996_VERSIONS,
+};
+
+static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void)
+{
+ size_t len;
+ u32 *msm_id;
+ enum _msm8996_version version;
+
+ msm_id = qcom_smem_get(QCOM_SMEM_HOST_ANY, MSM_ID_SMEM, &len);
+ /* The first 4 bytes are format, next to them is the actual msm-id
*/
+ msm_id++;
+
+ switch ((enum _msm_id)*msm_id) {
+ case MSM8996V3:
+ case APQ8096V3:
+ version = MSM8996_V3;
+ break;
+ case MSM8996SG:
+ case APQ8096SG:
+ version = MSM8996_SG;
+ break;
+ default:
+ version = NUM_OF_MSM8996_VERSIONS;
+ }
+
+ return version;
+}
+
+static int __init qcom_cpufreq_kryo_driver_init(void)
+{
+ struct device *cpu_dev_silver, *cpu_dev_gold;
+ struct opp_table *opp_silver, *opp_gold;
+ enum _msm8996_version msm8996_version;
+ struct nvmem_cell *speedbin_nvmem;
+ struct platform_device *pdev;
+ struct device_node *np;
+ u8 *speedbin;
+ u32 versions;
+ size_t len;
+ int ret;
+
+ cpu_dev_silver = get_cpu_device(SILVER_LEAD);
+ if (IS_ERR_OR_NULL(cpu_dev_silver))
+ return PTR_ERR(cpu_dev_silver);
+
+ cpu_dev_gold = get_cpu_device(SILVER_LEAD);
+ if (IS_ERR_OR_NULL(cpu_dev_gold))
+ return PTR_ERR(cpu_dev_gold);
+
+ msm8996_version = qcom_cpufreq_kryo_get_msm_id();
+ if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
+ dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
+ return -ENODEV;
+ }
+
+ np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
+ if (IS_ERR_OR_NULL(np))
+ return PTR_ERR(np);
+
+ if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
+ ret = -ENOENT;
+ goto free_np;
+ }
+
+ speedbin_nvmem = of_nvmem_cell_get(np, NULL);
+ if (IS_ERR(speedbin_nvmem)) {
+ ret = PTR_ERR(speedbin_nvmem);
+ dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n",
ret);
+ goto free_np;
+ }
+
+ speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+ nvmem_cell_put(speedbin_nvmem);
+
+ switch (msm8996_version) {
+ case MSM8996_V3:
+ versions = 1 << (unsigned int)(*speedbin);
+ break;
+ case MSM8996_SG:
+ versions = 1 << ((unsigned int)(*speedbin) + 4);
+ break;
+ default:
+ BUG();
+ break;
+ }
+
+ opp_silver =
dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);
+ if (IS_ERR_OR_NULL(opp_silver)) {
+ dev_err(cpu_dev_silver, "Failed to set supported
hardware\n");
+ ret = PTR_ERR(opp_silver);
+ goto free_np;
+ }
+
+ opp_gold = dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);
+ if (IS_ERR_OR_NULL(opp_gold)) {
+ dev_err(cpu_dev_gold, "Failed to set supported hardware\n");
+ ret = PTR_ERR(opp_gold);
+ goto free_opp_silver;
+ }
+
+ pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+ if (!IS_ERR_OR_NULL(pdev))
+ return 0;
+
+ ret = PTR_ERR(pdev);
+ dev_err(cpu_dev_silver, "Failed to register platform device\n");
+ dev_pm_opp_put_supported_hw(opp_gold);
+
+free_opp_silver:
+ dev_pm_opp_put_supported_hw(opp_silver);
+
+free_np:
+ of_node_put(np);
+
+ return ret;
+}
+late_initcall(qcom_cpufreq_kryo_driver_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Kryo CPUfreq driver");
+MODULE_LICENSE("GPL v2");
> -----Original Message-----
> From: Viresh Kumar <viresh.kumar@linaro.org>
> Sent: Friday, May 18, 2018 04:46
> To: Ilia Lin <ilialin@codeaurora.org>
> Cc: mturquette@baylibre.com; sboyd@kernel.org; robh@kernel.org;
> mark.rutland@arm.com; nm@ti.com; lgirdwood@gmail.com;
> broonie@kernel.org; andy.gross@linaro.org; david.brown@linaro.org;
> catalin.marinas@arm.com; will.deacon@arm.com; rjw@rjwysocki.net; linux-
> clk@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-pm@vger.kernel.org; linux-arm-
> msm@vger.kernel.org; linux-soc@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; rnayak@codeaurora.org;
> amit.kucheria@linaro.org; nicolas.dechesne@linaro.org;
> celster@codeaurora.org; tfinkel@codeaurora.org
> Subject: Re: [PATCH v8 10/15] cpufreq: Add Kryo CPU scaling driver
>
> On 17-05-18, 14:19, Ilia Lin wrote:
> > +static int __init qcom_cpufreq_kryo_driver_init(void)
> > +{
> > + size_t len;
> > + int ret = 0;
> > + u32 versions;
> > + enum _msm8996_version msm8996_version;
> > + u8 *speedbin;
> > + struct device *cpu_dev_silver, *cpu_dev_gold;
> > + struct device_node *np;
> > + struct nvmem_cell *speedbin_nvmem;
> > + struct platform_device *pdev;
> > + struct opp_table *opp_silver = NULL;
> > + struct opp_table *opp_gold = NULL;
>
> No need to initialize them and you may want to arrange all above in
> decreasing order of their length.
>
> > +
> > + cpu_dev_silver = get_cpu_device(SILVER_LEAD);
> > + if (IS_ERR_OR_NULL(cpu_dev_silver))
> > + return PTR_ERR(cpu_dev_silver);
> > +
> > + cpu_dev_gold = get_cpu_device(SILVER_LEAD);
> > + if (IS_ERR_OR_NULL(cpu_dev_gold))
> > + return PTR_ERR(cpu_dev_gold);
> > +
> > + msm8996_version = qcom_cpufreq_kryo_get_msm_id();
> > + if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
> > + dev_err(cpu_dev_silver, "Not Snapdragon 820/821!");
> > + return -ENODEV;
> > + }
> > +
> > + np = dev_pm_opp_of_get_opp_desc_node(cpu_dev_silver);
> > + if (IS_ERR_OR_NULL(np))
> > + return PTR_ERR(np);
> > +
> > + if (!of_device_is_compatible(np, "operating-points-v2-kryo-cpu")) {
> > + ret = -ENOENT;
> > + goto free_np;
> > + }
> > +
> > + speedbin_nvmem = of_nvmem_cell_get(np, NULL);
> > + if (IS_ERR(speedbin_nvmem)) {
> > + ret = PTR_ERR(speedbin_nvmem);
> > + dev_err(cpu_dev_silver, "Could not get nvmem cell: %d\n",
> ret);
> > + goto free_np;
> > + }
> > +
> > + speedbin = nvmem_cell_read(speedbin_nvmem, &len);
> > + nvmem_cell_put(speedbin_nvmem);
> > +
> > + switch (msm8996_version) {
> > + case MSM8996_V3:
> > + versions = 1 << (unsigned int)(*speedbin);
> > + break;
> > + case MSM8996_SG:
> > + versions = 1 << ((unsigned int)(*speedbin) + 4);
> > + break;
> > + default:
> > + BUG();
> > + break;
> > + }
> > +
> > + opp_silver =
> dev_pm_opp_set_supported_hw(cpu_dev_silver,&versions,1);
> > + if (IS_ERR_OR_NULL(opp_silver)) {
>
> This API doesn't return NULL and so IS_ERR() would be sufficient.
>
> > + dev_err(cpu_dev_silver, "Failed to set supported
> hardware\n");
> > + ret = PTR_ERR(opp_silver);
> > + goto free_np;
> > + }
> > +
> > + opp_gold =
> dev_pm_opp_set_supported_hw(cpu_dev_gold,&versions,1);
> > + if (IS_ERR_OR_NULL(opp_gold)) {
>
> same here.
>
> > + dev_err(cpu_dev_gold, "Failed to set supported
> hardware\n");
> > + ret = PTR_ERR(opp_gold);
> > + goto free_opp_silver;
> > + }
> > +
> > + pdev = platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
> > + if (!IS_ERR_OR_NULL(pdev))
> > + goto out;
>
> Simply return from here and remove the useless label out.
>
> > +
> > + ret = PTR_ERR(pdev);
> > + dev_err(cpu_dev_silver, "Failed to register platform device\n");
> > + dev_pm_opp_put_supported_hw(opp_gold);
> > +
> > +free_opp_silver:
> > + dev_pm_opp_put_supported_hw(opp_silver);
> > +
> > +free_np:
> > + of_node_put(np);
> > +
> > +out:
> > + return ret;
> > +}
> > +late_initcall(qcom_cpufreq_kryo_driver_init);
>
> Please resend only this patch now or just paste the new code in a mail
here
> so that I can review it quickly and then you can resend the final version.
Most
> of the patches aren't changing anyway.
>
> --
> viresh
^ permalink raw reply related
* Re: [PATCH v3 RESEND] display: panel: Add AUO g070vvn01 display support (800x480)
From: Lukasz Majewski @ 2018-05-19 11:06 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Mark Rutland, David Airlie, dri-devel, devicetree,
Rob Herring, linux-kernel
In-Reply-To: <20180514190849.18723-1-lukma@denx.de>
[-- Attachment #1: Type: text/plain, Size: 3647 bytes --]
Hi Thierry,
> This commit adds support for AUO's 7.0" display.
>
Thierry, shall I perform some more work on this code, or is it
eligible for applying to your tree?
Best regards,
Łukasz
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
> ---
> Changes for v3:
> - Remove not used 'bus-format-override = "rgb565";' property
>
> Changes for v2:
> - Add *.txt suffix to the auo,g070wn01 file
> ---
> .../bindings/display/panel/auo,g070vvn01.txt | 29
> ++++++++++++++++++++
> drivers/gpu/drm/panel/panel-simple.c | 31
> ++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode
> 100644
> Documentation/devicetree/bindings/display/panel/auo,g070vvn01.txt
>
> diff --git
> a/Documentation/devicetree/bindings/display/panel/auo,g070vvn01.txt
> b/Documentation/devicetree/bindings/display/panel/auo,g070vvn01.txt
> new file mode 100644 index 000000000000..49e4105378f6 --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/panel/auo,g070vvn01.txt
> @@ -0,0 +1,29 @@ +AU Optronics Corporation 7.0" FHD (800 x 480) TFT
> LCD panel +
> +Required properties:
> +- compatible: should be "auo,g070vvn01"
> +- backlight: phandle of the backlight device attached to the panel
> +- power-supply: single regulator to provide the supply voltage
> +
> +Required nodes:
> +- port: Parallel port mapping to connect this display
> +
> +This panel needs single power supply voltage. Its backlight is
> conntrolled +via PWM signal.
> +
> +Example:
> +--------
> +
> +Example device-tree definition when connected to iMX6Q based board
> +
> + lcd_panel: lcd-panel {
> + compatible = "auo,g070vvn01";
> + backlight = <&backlight_lcd>;
> + power-supply = <®_display>;
> +
> + port {
> + lcd_panel_in: endpoint {
> + remote-endpoint = <&lcd_display_out>;
> + };
> + };
> + };
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c index
> cbf1ab404ee7..d9984bdb5bb5 100644 ---
> a/drivers/gpu/drm/panel/panel-simple.c +++
> b/drivers/gpu/drm/panel/panel-simple.c @@ -581,6 +581,34 @@ static
> const struct panel_desc auo_b133htn01 = { },
> };
>
> +static const struct display_timing auo_g070vvn01_timings = {
> + .pixelclock = { 33300000, 34209000, 45000000 },
> + .hactive = { 800, 800, 800 },
> + .hfront_porch = { 20, 40, 200 },
> + .hback_porch = { 87, 40, 1 },
> + .hsync_len = { 1, 48, 87 },
> + .vactive = { 480, 480, 480 },
> + .vfront_porch = { 5, 13, 200 },
> + .vback_porch = { 31, 31, 29 },
> + .vsync_len = { 1, 1, 3 },
> +};
> +
> +static const struct panel_desc auo_g070vvn01 = {
> + .timings = &auo_g070vvn01_timings,
> + .num_timings = 1,
> + .bpc = 8,
> + .size = {
> + .width = 152,
> + .height = 91,
> + },
> + .delay = {
> + .prepare = 200,
> + .enable = 50,
> + .disable = 50,
> + .unprepare = 1000,
> + },
> +};
> +
> static const struct drm_display_mode auo_g104sn02_mode = {
> .clock = 40000,
> .hdisplay = 800,
> @@ -2095,6 +2123,9 @@ static const struct of_device_id
> platform_of_match[] = { .compatible = "auo,b133xtn01",
> .data = &auo_b133xtn01,
> }, {
> + .compatible = "auo,g070vvn01",
> + .data = &auo_g070vvn01,
> + }, {
> .compatible = "auo,g104sn02",
> .data = &auo_g104sn02,
> }, {
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND] display: panel: Add KOE tx14d24vm1bpa display support (320x240)
From: Lukasz Majewski @ 2018-05-19 11:05 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Mark Rutland, David Airlie, Rob Herring, dri-devel,
devicetree, linux-kernel
In-Reply-To: <20180509154338.2841-1-lukma@denx.de>
[-- Attachment #1: Type: text/plain, Size: 3701 bytes --]
Hi Thierry,
> This commit adds support for KOE's 5.7" display.
>
Thierry, shall I perform some more work on this code, or is it
eligible for applying to your tree?
Best regards,
Łukasz
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> ---
> .../bindings/display/panel/koe,tx14d24vm1bpa.txt | 42
> ++++++++++++++++++++++
> drivers/gpu/drm/panel/panel-simple.c | 26
> ++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644
> Documentation/devicetree/bindings/display/panel/koe,tx14d24vm1bpa.txt
>
> diff --git
> a/Documentation/devicetree/bindings/display/panel/koe,tx14d24vm1bpa.txt
> b/Documentation/devicetree/bindings/display/panel/koe,tx14d24vm1bpa.txt
> new file mode 100644 index 000000000000..be7ac666807b --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/panel/koe,tx14d24vm1bpa.txt
> @@ -0,0 +1,42 @@ +Kaohsiung Opto-Electronics Inc. 5.7" QVGA (320 x
> 240) TFT LCD panel +
> +Required properties:
> +- compatible: should be "koe,tx14d24vm1bpa"
> +- backlight: phandle of the backlight device attached to the panel
> +- power-supply: single regulator to provide the supply voltage
> +
> +Required nodes:
> +- port: Parallel port mapping to connect this display
> +
> +This panel needs single power supply voltage. Its backlight is
> conntrolled +via PWM signal.
> +
> +Example:
> +--------
> +
> +Example device-tree definition when connected to iMX53 based board
> +
> + lcd_panel: lcd-panel {
> + compatible = "koe,tx14d24vm1bpa";
> + backlight = <&backlight_lcd>;
> + power-supply = <®_3v3>;
> +
> + port {
> + lcd_panel_in: endpoint {
> + remote-endpoint = <&lcd_display_out>;
> + };
> + };
> + };
> +
> +Then one needs to extend the dispX node:
> +
> + lcd_display: disp1 {
> +
> + port@1 {
> + reg = <1>;
> +
> + lcd_display_out: endpoint {
> + remote-endpoint = <&lcd_panel_in>;
> + };
> + };
> + };
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c index
> d9984bdb5bb5..103b43ce7dee 100644 ---
> a/drivers/gpu/drm/panel/panel-simple.c +++
> b/drivers/gpu/drm/panel/panel-simple.c @@ -1268,6 +1268,29 @@ static
> const struct panel_desc innolux_zj070na_01p = { },
> };
>
> +static const struct display_timing koe_tx14d24vm1bpa_timing = {
> + .pixelclock = { 5580000, 5850000, 6200000 },
> + .hactive = { 320, 320, 320 },
> + .hfront_porch = { 30, 30, 30 },
> + .hback_porch = { 30, 30, 30 },
> + .hsync_len = { 1, 5, 17 },
> + .vactive = { 240, 240, 240 },
> + .vfront_porch = { 6, 6, 6 },
> + .vback_porch = { 5, 5, 5 },
> + .vsync_len = { 1, 2, 11 },
> + .flags = DISPLAY_FLAGS_DE_HIGH,
> +};
> +
> +static const struct panel_desc koe_tx14d24vm1bpa = {
> + .timings = &koe_tx14d24vm1bpa_timing,
> + .num_timings = 1,
> + .bpc = 6,
> + .size = {
> + .width = 115,
> + .height = 86,
> + },
> +};
> +
> static const struct display_timing koe_tx31d200vm0baa_timing = {
> .pixelclock = { 39600000, 43200000, 48000000 },
> .hactive = { 1280, 1280, 1280 },
> @@ -2204,6 +2227,9 @@ static const struct of_device_id
> platform_of_match[] = { .compatible = "innolux,zj070na-01p",
> .data = &innolux_zj070na_01p,
> }, {
> + .compatible = "koe,tx14d24vm1bpa",
> + .data = &koe_tx14d24vm1bpa,
> + }, {
> .compatible = "koe,tx31d200vm0baa",
> .data = &koe_tx31d200vm0baa,
> }, {
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 4/5] pinctrl: actions: Add gpio support for Actions S900 SoC
From: Manivannan Sadhasivam @ 2018-05-19 10:11 UTC (permalink / raw)
To: Christian Lamparter
Cc: linus.walleij, robh+dt, afaerber, liuwei, mp-cs, 96boards,
devicetree, andy.shevchenko, daniel.thompson, amit.kucheria,
linux-arm-kernel, linux-gpio, linux-kernel, hzhang, bdong,
manivannanece23
In-Reply-To: <12834501.5lBvNSZm06@debian64>
Hi Christian,
On Sat, May 19, 2018 at 11:18:53AM +0200, Christian Lamparter wrote:
> On Friday, May 18, 2018 4:30:55 AM CEST Manivannan Sadhasivam wrote:
> > Add gpio support to pinctrl driver for Actions Semi S900 SoC.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> > [...]
> > +static int owl_gpio_init(struct owl_pinctrl *pctrl)
> > +{
> > + struct gpio_chip *chip;
> > + int ret;
> > +
> > + chip = &pctrl->chip;
> > + chip->base = -1;
> > + chip->ngpio = pctrl->soc->ngpios;
> > + chip->label = dev_name(pctrl->dev);
> > + chip->parent = pctrl->dev;
> > + chip->owner = THIS_MODULE;
> > + chip->of_node = pctrl->dev->of_node;
> > +
> > + ret = gpiochip_add_data(&pctrl->chip, pctrl);
> > + if (ret) {
> > + dev_err(pctrl->dev, "failed to register gpiochip\n");
> > + return ret;
> > + }
> > +
> > + ret = gpiochip_add_pin_range(&pctrl->chip, dev_name(pctrl->dev),
> > + 0, 0, chip->ngpio);
> > + if (ret) {
> > + dev_err(pctrl->dev, "failed to add pin range\n");
> > + gpiochip_remove(&pctrl->chip);
> > + return ret;
> > + }
> > +
> gpiochip_add_pin_range()? That's not going to work with gpio-hogs.
>
Hmmm. Just looked into the gpio-hog mechanism and the patch you have
implemented for MSM driver. I agree with you on replacing
gpiochip_add_pin_range() with gpio-ranges property. But I'm curious
whether we should document it somewhere or not (probably in [1]).
Anyway I will send the v2 incorporating your suggestion.
Thanks,
Mani
[1] Documentation/devicetree/bindings/gpio/gpio.txt
> But, you can easily test this. Just add a gpio-hog [0]
> ( Section 2. gpio-controller nodes) into the Devicetree's
> pinctrl node.
>
> something like: (No idea if GPIO1 is already used, but any free
> gpio will do)
> | [...]
> | pinctrl@e01b0000 {
> | compatible = "actions,s900-pinctrl";
> | reg = <0x0 0xe01b0000 0x0 0x1000>;
> | clocks = <&cmu CLK_GPIO>;
> | gpio-controller;
> | #gpio-cells = <2>;
> |
> | line_b {
> | gpio-hog;
> | gpios = <1 GPIO_ACTIVE_HIGH>;
> | output-low;
> | line-name = "foo-bar-gpio";
> | };
> | };
>
> The pinctrl probe will fail. You can fix this by
> replacing the gpiochip_add_pin_range() and use
> the gpio-ranges [0] property to define the range.
>
> [0] <https://www.kernel.org/doc/Documentation/devicetree/bindings/gpio/gpio.txt>
>
>
>
^ permalink raw reply
* Re: [PATCH v3 4/4] arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and Makefile
From: kbuild test robot @ 2018-05-19 10:07 UTC (permalink / raw)
Cc: kbuild-all, Matthias Brugger, Rob Herring, Mark Rutland,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Greg Kroah-Hartman,
devicetree, srv_heupstream, linux-kernel, linux-serial,
linux-mediatek, linux-arm-kernel, yingjoe.chen, erin.lo,
mars.cheng, Ben Ho
In-Reply-To: <1526538126-51497-5-git-send-email-erin.lo@mediatek.com>
[-- Attachment #1: Type: text/plain, Size: 1108 bytes --]
Hi Ben,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Erin-Lo/Add-basic-support-for-Mediatek-MT8183-SoC/20180519-160349
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm64-alldefconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
>> Error: arch/arm64/boot/dts/mediatek/mt8183.dtsi:137.9-10 syntax error
FATAL ERROR: Unable to parse input tree
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 8591 bytes --]
^ permalink raw reply
* Re: [PATCH 4/5] pinctrl: actions: Add gpio support for Actions S900 SoC
From: Christian Lamparter @ 2018-05-19 9:18 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: linus.walleij, robh+dt, afaerber, liuwei, mp-cs, 96boards,
devicetree, andy.shevchenko, daniel.thompson, amit.kucheria,
linux-arm-kernel, linux-gpio, linux-kernel, hzhang, bdong,
manivannanece23
In-Reply-To: <20180518023056.7869-5-manivannan.sadhasivam@linaro.org>
On Friday, May 18, 2018 4:30:55 AM CEST Manivannan Sadhasivam wrote:
> Add gpio support to pinctrl driver for Actions Semi S900 SoC.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
> [...]
> +static int owl_gpio_init(struct owl_pinctrl *pctrl)
> +{
> + struct gpio_chip *chip;
> + int ret;
> +
> + chip = &pctrl->chip;
> + chip->base = -1;
> + chip->ngpio = pctrl->soc->ngpios;
> + chip->label = dev_name(pctrl->dev);
> + chip->parent = pctrl->dev;
> + chip->owner = THIS_MODULE;
> + chip->of_node = pctrl->dev->of_node;
> +
> + ret = gpiochip_add_data(&pctrl->chip, pctrl);
> + if (ret) {
> + dev_err(pctrl->dev, "failed to register gpiochip\n");
> + return ret;
> + }
> +
> + ret = gpiochip_add_pin_range(&pctrl->chip, dev_name(pctrl->dev),
> + 0, 0, chip->ngpio);
> + if (ret) {
> + dev_err(pctrl->dev, "failed to add pin range\n");
> + gpiochip_remove(&pctrl->chip);
> + return ret;
> + }
> +
gpiochip_add_pin_range()? That's not going to work with gpio-hogs.
But, you can easily test this. Just add a gpio-hog [0]
( Section 2. gpio-controller nodes) into the Devicetree's
pinctrl node.
something like: (No idea if GPIO1 is already used, but any free
gpio will do)
| [...]
| pinctrl@e01b0000 {
| compatible = "actions,s900-pinctrl";
| reg = <0x0 0xe01b0000 0x0 0x1000>;
| clocks = <&cmu CLK_GPIO>;
| gpio-controller;
| #gpio-cells = <2>;
|
| line_b {
| gpio-hog;
| gpios = <1 GPIO_ACTIVE_HIGH>;
| output-low;
| line-name = "foo-bar-gpio";
| };
| };
The pinctrl probe will fail. You can fix this by
replacing the gpiochip_add_pin_range() and use
the gpio-ranges [0] property to define the range.
[0] <https://www.kernel.org/doc/Documentation/devicetree/bindings/gpio/gpio.txt>
^ permalink raw reply
* Re: [PATCH v2 12/26] drm/sun4i: Add support for multiple DW HDMI PHY clock parents
From: Jernej Škrabec @ 2018-05-19 7:11 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jagan Teki, Chen-Yu Tsai, Icenowy Zheng, Rob Herring,
Mark Rutland, Catalin Marinas, Will Deacon, David Airlie,
dri-devel, Michael Turquette, Stephen Boyd, linux-clk,
Michael Trimarchi, linux-arm-kernel, devicetree, linux-kernel,
linux-sunxi
In-Reply-To: <20180518152651.lfymc3kj7npj5tww@flea>
Hi,
Dne petek, 18. maj 2018 ob 17:26:51 CEST je Maxime Ripard napisal(a):
> On Fri, May 18, 2018 at 04:46:41PM +0200, Jernej Škrabec wrote:
> > > And this is a bit sloppy, since if phy_clk_num == 3, you won't try to
> > > lookup pll-2 either.
> >
> > It is highly unlikely this will be higher than 2, at least for this HDMI
> > PHY, since it has only 1 bit reserved for parent selection. But since I
> > have to fix it, I'll add ">= 2"
>
> If we're only going to have two parents at most, ever, why don't we
> had just a single other boolean. This would be less intrusive, and we
> wouldn't have to check for those corner cases.
It seems that usage of "bool" data type in structures is not wanted anymore
according to checkpatch and this: https://lkml.org/lkml/2017/11/21/384
I guess I'll use "unsigned int" as recommended by Linus and named it
"has_second_parent" to be unambigous that it's boolean in reality.
Best regards,
Jernej
>
> > BTW, I'll resend fixed version of this patch for my R40 HDMI series, since
> > there is nothing to hold it back, unlike for this.
>
> Awesome, thanks!
> Maxime
>
> --
> Maxime Ripard, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com
^ permalink raw reply
* Re: [PATCH v3 2/6] mfd: at91-usart: added mfd driver for usart
From: Alexandre Belloni @ 2018-05-19 7:08 UTC (permalink / raw)
To: Rob Herring
Cc: Radu Pirea, devicetree, linux-serial, linux-kernel,
linux-arm-kernel, linux-spi, mark.rutland, lee.jones, gregkh,
jslaby, richard.genoud, nicolas.ferre, broonie
In-Reply-To: <20180518221949.GA13443@rob-hp-laptop>
On 18/05/2018 17:19:49-0500, Rob Herring wrote:
> On Fri, May 11, 2018 at 01:38:18PM +0300, Radu Pirea wrote:
> > This mfd driver is just a wrapper over atmel_serial driver and
> > spi-at91-usart driver. Selection of one of the drivers is based on a
> > property from device tree. If the property is not specified, the default
> > driver is atmel_serial.
> >
> > Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> > ---
> > drivers/mfd/Kconfig | 10 ++++
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/at91-usart.c | 75 ++++++++++++++++++++++++++++
> > include/dt-bindings/mfd/at91-usart.h | 17 +++++++
> > 4 files changed, 103 insertions(+)
> > create mode 100644 drivers/mfd/at91-usart.c
> > create mode 100644 include/dt-bindings/mfd/at91-usart.h
> >
>
> > +#ifndef __DT_BINDINGS_AT91_USART_H__
> > +#define __DT_BINDINGS_AT91_USART_H__
> > +
> > +#define AT91_USART_MODE_SERIAL 1
> > +#define AT91_USART_MODE_SPI 2
>
> Won't this require a DT update for serial mode to add the mode property?
> That breaks compatibility.
>
If the mode property is not present, it defaults to serial to keep
compatibility.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [v0 2/2] cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver
From: kbuild test robot @ 2018-05-19 1:24 UTC (permalink / raw)
Cc: kbuild-all, Rafael J. Wysocki, Viresh Kumar, linux-kernel,
linux-pm, Stephen Boyd, Rajendra Nayak, Amit Nischal, devicetree,
amit.kucheria, Taniya Das
In-Reply-To: <1526549401-25666-3-git-send-email-tdas@codeaurora.org>
[-- Attachment #1: Type: text/plain, Size: 10763 bytes --]
Hi Taniya,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Taniya-Das/Add-support-for-QCOM-cpufreq-FW-driver/20180519-050902
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
drivers/cpufreq/qcom-cpufreq-fw.c: In function 'qcom_cpufreq_fw_cpu_init':
>> drivers/cpufreq/qcom-cpufreq-fw.c:77:8: error: 'struct cpufreq_policy' has no member named 'table'
policy->table = c->table;
^~
drivers/cpufreq/qcom-cpufreq-fw.c: In function 'qcom_cpu_resources_init':
>> drivers/cpufreq/qcom-cpufreq-fw.c:187:37: error: passing argument 1 of 'platform_get_resource_byname' from incompatible pointer type [-Werror=incompatible-pointer-types]
res = platform_get_resource_byname(dev, IORESOURCE_MEM, "en_base");
^~~
In file included from include/linux/of_device.h:6:0,
from include/linux/of_platform.h:12,
from drivers/cpufreq/qcom-cpufreq-fw.c:11:
include/linux/platform_device.h:56:25: note: expected 'struct platform_device *' but argument is of type 'struct device *'
extern struct resource *platform_get_resource_byname(struct platform_device *,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/cpufreq/qcom-cpufreq-fw.c:187:6: error: incompatible types when assigning to type 'struct resource' from type 'struct resource *'
res = platform_get_resource_byname(dev, IORESOURCE_MEM, "en_base");
^
>> drivers/cpufreq/qcom-cpufreq-fw.c:188:6: error: wrong type argument to unary exclamation mark
if (!res) {
^
>> drivers/cpufreq/qcom-cpufreq-fw.c:193:33: error: invalid type argument of '->' (have 'struct resource')
en_base = devm_ioremap(dev, res->start, resource_size(res));
^~
>> drivers/cpufreq/qcom-cpufreq-fw.c:193:56: error: incompatible type for argument 1 of 'resource_size'
en_base = devm_ioremap(dev, res->start, resource_size(res));
^~~
In file included from include/linux/of_address.h:4:0,
from drivers/cpufreq/qcom-cpufreq-fw.c:10:
include/linux/ioport.h:196:31: note: expected 'const struct resource *' but argument is of type 'struct resource'
static inline resource_size_t resource_size(const struct resource *res)
^~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +77 drivers/cpufreq/qcom-cpufreq-fw.c
> 11 #include <linux/of_platform.h>
12
13 #define INIT_RATE 300000000UL
14 #define XO_RATE 19200000UL
15 #define LUT_MAX_ENTRIES 40U
16 #define CORE_COUNT_VAL(val) ((val & GENMASK(18, 16)) >> 16)
17 #define LUT_ROW_SIZE 32
18
19 struct cpufreq_qcom {
20 struct cpufreq_frequency_table *table;
21 struct device *dev;
22 void __iomem *perf_base;
23 void __iomem *lut_base;
24 cpumask_t related_cpus;
25 unsigned int max_cores;
26 };
27
28 static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
29
30 static int
31 qcom_cpufreq_fw_target_index(struct cpufreq_policy *policy, unsigned int index)
32 {
33 struct cpufreq_qcom *c = policy->driver_data;
34
35 if (index >= LUT_MAX_ENTRIES) {
36 dev_err(c->dev,
37 "Passing an index (%u) that's greater than max (%d)\n",
38 index, LUT_MAX_ENTRIES - 1);
39 return -EINVAL;
40 }
41
42 writel_relaxed(index, c->perf_base);
43
44 /* Make sure the write goes through before proceeding */
45 mb();
46 return 0;
47 }
48
49 static unsigned int qcom_cpufreq_fw_get(unsigned int cpu)
50 {
51 struct cpufreq_qcom *c;
52 unsigned int index;
53
54 c = qcom_freq_domain_map[cpu];
55 if (!c)
56 return -ENODEV;
57
58 index = readl_relaxed(c->perf_base);
59 index = min(index, LUT_MAX_ENTRIES - 1);
60
61 return c->table[index].frequency;
62 }
63
64 static int qcom_cpufreq_fw_cpu_init(struct cpufreq_policy *policy)
65 {
66 struct cpufreq_qcom *c;
67 int ret;
68
69 c = qcom_freq_domain_map[policy->cpu];
70 if (!c) {
71 pr_err("No scaling support for CPU%d\n", policy->cpu);
72 return -ENODEV;
73 }
74
75 cpumask_copy(policy->cpus, &c->related_cpus);
76
> 77 policy->table = c->table;
78 policy->driver_data = c;
79
80 return ret;
81 }
82
83 static struct freq_attr *qcom_cpufreq_fw_attr[] = {
84 &cpufreq_freq_attr_scaling_available_freqs,
85 &cpufreq_freq_attr_scaling_boost_freqs,
86 NULL
87 };
88
89 static struct cpufreq_driver cpufreq_qcom_fw_driver = {
90 .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
91 CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
92 .verify = cpufreq_generic_frequency_table_verify,
93 .target_index = qcom_cpufreq_fw_target_index,
94 .get = qcom_cpufreq_fw_get,
95 .init = qcom_cpufreq_fw_cpu_init,
96 .name = "qcom-cpufreq-fw",
97 .attr = qcom_cpufreq_fw_attr,
98 .boost_enabled = true,
99 };
100
101 static int qcom_read_lut(struct platform_device *pdev,
102 struct cpufreq_qcom *c)
103 {
104 struct device *dev = &pdev->dev;
105 u32 data, src, lval, i, core_count, prev_cc = 0;
106
107 c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
108 sizeof(*c->table), GFP_KERNEL);
109 if (!c->table)
110 return -ENOMEM;
111
112 for (i = 0; i < LUT_MAX_ENTRIES; i++) {
113 data = readl_relaxed(c->lut_base + i * LUT_ROW_SIZE);
114 src = ((data & GENMASK(31, 30)) >> 30);
115 lval = (data & GENMASK(7, 0));
116 core_count = CORE_COUNT_VAL(data);
117
118 if (!src)
119 c->table[i].frequency = INIT_RATE / 1000;
120 else
121 c->table[i].frequency = XO_RATE * lval / 1000;
122
123 c->table[i].driver_data = c->table[i].frequency;
124
125 dev_dbg(dev, "index=%d freq=%d, core_count %d\n",
126 i, c->table[i].frequency, core_count);
127
128 if (core_count != c->max_cores)
129 c->table[i].frequency = CPUFREQ_ENTRY_INVALID;
130
131 /*
132 * Two of the same frequencies with the same core counts means
133 * end of table.
134 */
135 if (i > 0 && c->table[i - 1].driver_data ==
136 c->table[i].driver_data
137 && prev_cc == core_count) {
138 struct cpufreq_frequency_table *prev = &c->table[i - 1];
139
140 if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
141 prev->flags = CPUFREQ_BOOST_FREQ;
142 prev->frequency = prev->driver_data;
143 }
144
145 break;
146 }
147 prev_cc = core_count;
148 }
149 c->table[i].frequency = CPUFREQ_TABLE_END;
150
151 return 0;
152 }
153
154 static int qcom_get_related_cpus(struct device_node *np, struct cpumask *m)
155 {
156 struct device_node *dev_phandle;
157 struct device *cpu_dev;
158 int cpu, i = 0, ret = -ENOENT;
159
160 dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);
161 while (dev_phandle) {
162 for_each_possible_cpu(cpu) {
163 cpu_dev = get_cpu_device(cpu);
164 if (cpu_dev && cpu_dev->of_node == dev_phandle) {
165 cpumask_set_cpu(cpu, m);
166 ret = 0;
167 break;
168 }
169 }
170 dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);
171 }
172
173 return ret;
174 }
175
176 static int qcom_cpu_resources_init(struct platform_device *pdev,
177 struct device_node *np)
178 {
179 struct cpufreq_qcom *c;
180 struct resource res;
181 struct device *dev = &pdev->dev;
182 void __iomem *en_base;
183 int cpu, index = 0, ret;
184
185 c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
186
> 187 res = platform_get_resource_byname(dev, IORESOURCE_MEM, "en_base");
> 188 if (!res) {
189 dev_err(dev, "Enable base not defined for %s\n", np->name);
190 return ret;
191 }
192
> 193 en_base = devm_ioremap(dev, res->start, resource_size(res));
194 if (!en_base) {
195 dev_err(dev, "Unable to map %s en-base\n", np->name);
196 return -ENOMEM;
197 }
198
199 /* FW should be enabled state to proceed */
200 if (!(readl_relaxed(en_base) & 0x1)) {
201 dev_err(dev, "%s firmware not enabled\n", np->name);
202 return -ENODEV;
203 }
204
205 devm_iounmap(&pdev->dev, en_base);
206
207 index = of_property_match_string(np, "reg-names", "perf_base");
208 if (index < 0)
209 return index;
210
211 if (of_address_to_resource(np, index, &res))
212 return -ENOMEM;
213
214 c->perf_base = devm_ioremap(dev, res.start, resource_size(&res));
215 if (!c->perf_base) {
216 dev_err(dev, "Unable to map %s perf-base\n", np->name);
217 return -ENOMEM;
218 }
219
220 index = of_property_match_string(np, "reg-names", "lut_base");
221 if (index < 0)
222 return index;
223
224 if (of_address_to_resource(np, index, &res))
225 return -ENOMEM;
226
227 c->lut_base = devm_ioremap(dev, res.start, resource_size(&res));
228 if (!c->lut_base) {
229 dev_err(dev, "Unable to map %s lut-base\n", np->name);
230 return -ENOMEM;
231 }
232
233 ret = qcom_get_related_cpus(np, &c->related_cpus);
234 if (ret) {
235 dev_err(dev, "%s failed to get core phandles\n", np->name);
236 return ret;
237 }
238
239 c->max_cores = cpumask_weight(&c->related_cpus);
240
241 ret = qcom_read_lut(pdev, c);
242 if (ret) {
243 dev_err(dev, "%s failed to read LUT\n", np->name);
244 return ret;
245 }
246
247 for_each_cpu(cpu, &c->related_cpus)
248 qcom_freq_domain_map[cpu] = c;
249
250 return 0;
251 }
252
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 65216 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: David Collins @ 2018-05-19 0:46 UTC (permalink / raw)
To: Doug Anderson
Cc: Mark Brown, Liam Girdwood, Rob Herring, Mark Rutland,
linux-arm-msm, Linux ARM, devicetree, LKML, Rajendra Nayak,
Stephen Boyd
In-Reply-To: <CAD=FV=U-QOQekUHj=ck57YJbTFdFjCAWr4t2A92x+fCNf+yV9A@mail.gmail.com>
On 05/17/2018 06:01 PM, Doug Anderson wrote:
> On Thu, May 17, 2018 at 5:16 PM, David Collins <collinsd@codeaurora.org> wrote:
>> On 05/17/2018 02:22 PM, Doug Anderson wrote:
>>> On Fri, May 11, 2018 at 7:28 PM, David Collins <collinsd@codeaurora.org> wrote:
>>>> +- qcom,regulator-initial-microvolt
>>>> + Usage: optional; VRM regulators only
>>>> + Value type: <u32>
>>>> + Definition: Specifies the initial voltage in microvolts to request for a
>>>> + VRM regulator.
>>>
>>> Now that Mark has landed the patch adding support for the
>>> -ENOTRECOVERABLE error code from get_voltage() / get_voltage_sel(), do
>>> we still need the qcom,regulator-initial-microvolt property?
>>
>> Yes, this is still needed. The -ENOTRECOVERABLE patch ensures that
>> qcom-rpmh-regulator devices can be registered even if
>> qcom,regulator-initial-microvolt is not specified. However, that will
>> result in the regulators being configured for the minimum voltage
>> supported in the DT specified min/max range. The
>> qcom,regulator-initial-microvolt property allows us to set a specific
>> voltage that is larger than the min constraint.
>
> Ah, OK. In the device tree fragment I saw the initial was always
> equal to the min, so I wasn't sure if this was really needed in
> practice. I presume it would only be important if a voltage was left
> high by the bootloader for some peripheral that needs to continue to
> function (and use the existing higher voltage) until a real device
> claims it. For all other voltages, it should be fine if it's set to
> the min until a real device claims it. Do you have real examples of
> devices like this in boards using sdm845?
Something to keep in mind about the downstream rpmh-regulator driver is
that it caches the initial voltages specified in device tree and only
sends them after a consumer driver makes a regulator framework call. This
saves time during boot and ensures that requests are not made for
regulators that no Linux consumer cares about.
It is generally not safe to request all regulators to be set to the
minimum allowed voltage. Special care will be needed with the upstream
qcom-rpmh-regulator driver to avoid disrupting the boot up state of
regulators that are needed by other subsystems. Therefore, I would like
to keep the initial voltage feature supported.
>>>> +- regulator-initial-mode
>>>> + Usage: optional; VRM regulators only
>>>> + Value type: <u32>
>>>> + Definition: Specifies the initial mode to request for a VRM regulator.
>>>> + Supported values are RPMH_REGULATOR_MODE_* which are defined
>>>> + in [1] (i.e. 0 to 3). This property may be specified even
>>>> + if the regulator-allow-set-load property is not specified.
>>>
>>> Every time I read the above I wonder why you're documenting a standard
>>> regulator regulator property in your bindings. ...then I realize it's
>>> because you're doing it because you want to explicitly document what
>>> the valid modes are. I wonder if it makes sense to just put a
>>> reference somewhere else in this document to go look at the header
>>> file where these are all nicely documented.
>>
>> Isn't that what the [1] in the above snippet is currently doing. Further
>> down in qcom,rpmh-regulator.txt is this line:
>>
>> +[1] include/dt-bindings/regulator/qcom,rpmh-regulator.h
>
> Right, but I want to move it so it doesn't look like you're defining a
> property that's already defined in the common bindings. AKA get rid
> of the "regulator-initial-mode" property description. Then add above
> Examples:
>
> ========================
> Regulator Modes
> ========================
>
> RPMh regulators are designed to work with the standard regulator mode
> bindings, using properties like "regulator-initial-mode". See
> include/dt-bindings/regulator/qcom,rpmh-regulator.h for information on
> the modes relevant to RPMh regulators.
>
> Some RPMh regulators (BOB regulators only) also support bypass using
> the standard "regulator-allow-bypass" binding.
>
>
> ...feel fee to reword, but basically the idea is to document it but
> not make it look like you're defining a novel property.
Ok, I'll try rewording the mode explanation and move it into another
section of the binding doc.
>>> Speaking of documenting things like that, it might be worth finding
>>> somewhere in this doc to mention that the "bob" regulator on PMI8998
>>> can support "regulator-allow-bypass". That tidbit got lost when we
>>> moved to the standard regulator bindings for bypass.
>>
>> I suppose that I could add something like this:
>>
>> +- regulator-allow-bypass
>> + Usage: optional; BOB type VRM regulators only
>> + Value type: <empty>
>> + Definition: See [2] for details.
>> ...
>> +[2]: Documentation/devicetree/bindings/regulator.txt
>>
>> However, I don't want the patch to get NACKed because it is defining a
>> property that is already defined in the common regulator.txt file.
>
> See above for my suggestion.
Ok.
>>>> +- qcom,allowed-drms-modes
>>>> + Usage: required if regulator-allow-set-load is specified;
>>>> + VRM regulators only
>>>> + Value type: <prop-encoded-array>
>>>> + Definition: A list of integers specifying the PMIC regulator modes which
>>>> + can be configured at runtime based upon consumer load needs.
>>>> + Supported values are RPMH_REGULATOR_MODE_* which are defined
>>>> + in [1] (i.e. 0 to 3).
>>>
>>> Why is this still here? You moved it to the core regulator framework,
>>> right? It's still in your examples too. Shouldn't this be removed?
>>> It looks like the driver still needs this and it needs to be an exact
>>> duplicate of the common binding. That doesn't seem right...
>>
>> The qcom,allowed-drms-modes property supports a different feature than the
>> regulator-allowed-modes property accepted in [2]. The latter specifies
>> the modes that may be used at all (e.g. in regulator_set_mode() calls) and
>> it lists the mode values in an unordered fashion.
>>
>> qcom,allowed-drms-modes defines a specific subset of the possible allowed
>> modes that should be set based on DRMS (e.g. in regulator_set_load()
>> calls). Its values are listed in a specific order and must match 1-to-1
>> with qcom,drms-mode-max-microamps entries.
>>
>> It would probably be good to change the name of the property from
>> qcom,allowed-drms-modes to qcom,regulator-drms-modes.
>
> Ah, I see. It's unfortunate that now we need to effectively list all
> modes twice. Have you seen real-life examples where these sets of
> modes need to be different, or is this just theoretical? If not can
> we start with one property (that controls both things) and if we
> really see that we need to specify different sets of modes for the two
> cases we can add a separate property? ...actually, even if you do
> have real-life examples of where these need to be different, if 90% of
> the time they are the same it would still be nice to just have one
> property apply to both cases.
I plan to keep qcom,regulator-drms-modes (and
qcom,drms-mode-max-microamps) around as a property specifically handled
for qcom-rpmh-regulator. It serves a purpose that is distinct from that
of the generic regulator-allowed-modes. Without it, there will not be a
way to utilize regulator_set_load() to configure the regulator modes.
>>>> +- qcom,drms-mode-max-microamps
>>>> + Usage: required if regulator-allow-set-load is specified;
>>>> + VRM regulators only
>>>> + Value type: <prop-encoded-array>
>>>> + Definition: A list of integers specifying the maximum allowed load
>>>> + current in microamps for each of the modes listed in
>>>> + qcom,allowed-drms-modes (matched 1-to-1 in order). Elements
>>>> + must be specified in order from lowest to highest value.
>>>
>>> Any reason this can't go into the regulator core? You'd basically
>>> just take the existing concept of rpmh_regulator_vrm_set_load() and
>>> put it in the core.
>>
>> This could be implemented in the core via new constraint elements parsed
>> in of_regulator and a helper function to specify in regulator_ops.
>> However, I'm not sure about the wide-spread applicability of this feature.
>> I'd prefer to leave it in the driver unless Mark would like me to add it
>> into the core.
>
> You're already using pre-existing APIs around specifying the current
> and having the regulator core call you to map the total current into a
> mode. That implies that this is applicable to others. Adding this
> tiny amount of code to the core makes the pre-existing APIs generally
> useful.
I don't see the benefit of making struct regulation_constraints more
complicated with DRMS mode and current arrays that would only every be
used by the qcom-rpmh-regulator driver. Other regulator drivers are able
to hard code this information in the driver code using get_optimum_mode()
callbacks.
As a side note, changing qcom-rpmh-regulator to use a get_optimum_mode()
callback instead of a set_load() callback would probably be a good idea too.
>>>> +- qcom,headroom-microvolt
>>>> + Usage: optional; VRM regulators only
>>>> + Value type: <u32>
>>>> + Definition: Specifies the headroom voltage in microvolts to request for
>>>> + a VRM regulator. RPMh hardware automatically ensures that
>>>> + the parent of this regulator outputs a voltage high enough
>>>> + to satisfy the requested headroom. Supported values are
>>>> + 0 to 511000.
>>>
>>> I'm curious: is this a voted-for value, or a global value?
>>>
>>> Said another way: the whole point of RPMh is that there may be more
>>> than one processor that needs the same rails, right? So the AP might
>>> request 1.1 V for a rail and the modem might request 1.3 V. RPMh
>>> would decide to pick the higher of those two (1.3 V), but if the modem
>>> said it no longer needs the rail it will drop down to 1.1 V.
>>>
>>> ...and as an example of why the headroom needs to be in hardware, if
>>> the source voltage was normally 1.4 V and the headroom was 200 mV then
>>> the hardware would need to know to bump up the source voltage to 1.5V
>>> during the period of of time that the modem wants the rail at 1.3V.
>>>
>>> So my question is: do the AP and modem in the above situation
>>> separately vote for headroom? How is it aggregated? ...or is it a
>>> global value and this sets the headroom for all clients of RPMh? It
>>> would be interesting to document this as it might help with figuring
>>> out how this value should be set.
>>
>> The headroom voltage voting is supported in hardware per-regulator and
>> per-master (AP, modem, etc). The headroom voltage and output voltage are
>> each aggregated (using max) per-regulator across masters. If the
>> aggregated enable state for a regulator is on, then the aggregated output
>> voltage and headroom voltage are added together and applied as a min
>> constraint on the parent's output voltage (if there is a parent).
>
> Ah, interesting. I'm not 100% convinced that the RPMh API is at the
> right abstraction level here. I guess you increase the headroom
> voltage if you expect a lot of current and need the regulator to still
> give a clean signal? If you truly wanted to aggregate then if both
> the modem and AP wanted to draw a lot of current they would both need
> to increase the headroom and then the headroom should maybe not be the
> max but something slightly more (you wouldn't want to add, but ...)
>
> Since it's just a max, in theory it seems like you get 99% of the way
> there by just using the Linux APIs to deal with dropout voltage. If
> Linux was managing it in software then if it needed to account for
> extra headroom it would just increase the supply voltage. That should
> play just fine with the modem (which might be using the hardware
> headroom feature) since it will be making its own completely separate
> requests and they should be aggregated OK.
>
> In another thread you said you'd be OK dropping the headroom voltage
> since it wasn't needed on SDM845. Maybe we should do that? ...and if
> someone later needs to account for a larger dropout they can figure
> out how to hookup the standard linux min_dropout_uV?
I will remove qcom,headroom-microvolt.
Take care,
David
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: Re: [PATCH v4 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet
From: Brüns, Stefan @ 2018-05-19 0:26 UTC (permalink / raw)
To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
Paul Kocialkowski
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
Rob Herring, Mark Rutland, Chen-Yu Tsai, Thierry Reding,
David Airlie
In-Reply-To: <20180518071436.3vvyllghrdxptsxg@flea>
On Freitag, 18. Mai 2018 09:14:36 CEST Maxime Ripard wrote:
> On Mon, May 14, 2018 at 10:36:08PM +0200, Paul Kocialkowski wrote:
> > > > + backlight: backlight {
> > > > + compatible = "pwm-backlight";
> > > > + pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
> > > > + brightness-levels = < 0 1 1 1 1 2 2 2
> > > > + 2 3 3 3 3 4 4 4
> > > > + 5 5 5 6 6 6 7 7
> > > > + 8 8 8 9 9 9 10 10
> > > > + 10 11 11 12 12 12 13 13
> > > > + 14 14 14 15 15 16 16 17
> > > > + 17 17 18 18 19 19 20 20
> > > > + 21 21 21 22 22 23 23 24
> > > > + 24 25 25 26 26 27 27 28
> > > > + 28 29 30 30 31 31 32 32
> > > > + 33 33 34 35 35 36 36 37
> > > > + 38 38 39 39 40 41 41 42
> > > > + 43 43 44 44 45 46 47 47
> > > > + 48 49 49 50 51 51 52 53
> > > > + 54 54 55 56 57 57 58 59
> > > > + 60 61 61 62 63 64 65 65
> > > > + 66 67 68 69 70 71 71 72
> > > > + 73 74 75 76 77 78 79 80
> > > > + 81 82 83 84 85 86 87 88
> > > > + 89 90 91 92 93 94 95 96
> > > > + 97 98 99 101 102 103 104 105
> > > > + 106 108 109 110 111 112 114 115
> > > > + 116 117 119 120 121 123 124 125
> > > > + 127 128 129 131 132 133 135 136
> > > > + 138 139 141 142 144 145 147 148
> > > > + 150 151 153 154 156 157 159 161
> > > > + 162 164 166 167 169 171 173 174
> > > > + 176 178 180 181 183 185 187 189
> > > > + 191 192 194 196 198 200 202 204
> > > > + 206 208 210 212 214 216 219 221
> > > > + 223 225 227 229 232 234 236 238
> > > > + 241 242 244 246 248 250 253 255>;
> > >
> > > You kind of overdid it here :)
> > >
> > > What I meant to say before was that if you have 10 elements (and you
> > > really should have something in that magnitude) each step should
> > > increase the perceived brightness by 10%.
> >
> > Mhh I think 10 elements would fall too short to really depict the curve
> > with appropriate precision. Given the usual size for brightness cursors
> > in e.g. gnome-shell, it feels like a bigger number would be more
> > appropriate. Let's make it to 100 with values from 0 to 255!
> >
> > > In this particular case, I really think having something close to <0 4
> > > 8 16 32 64 128 255> would be enough.
> > >
> > > And in general, that kind of odd looking table without any more
> > > context is just screaming for a comment :)
> >
> > Noted, I will explain the idea, but probably without the exact formula
> > that's really a nasty hack written down on a piece of paper sitting in
> > my garbage at this point.
>
> So no one will ever be able to understand where this sequence comes
> from (yourself-in-two-years included). That sounds like a pretty bad
> idea.
>
> Maxime
The following formula yields practically the same table:
out = ceil(255 * (0.245 * in/255 + 0.755 * pow(in/255, 2.6) ))
Maximum error: 4, maximum relative error: 0.33
Kind regards,
Stefan
^ permalink raw reply
* Re: [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: Rob Herring @ 2018-05-18 22:24 UTC (permalink / raw)
To: David Collins
Cc: Doug Anderson, Mark Brown, Liam Girdwood, Mark Rutland,
linux-arm-msm, Linux ARM, devicetree, LKML, Rajendra Nayak,
Stephen Boyd
In-Reply-To: <869aad59-1cc5-28ef-1fb5-4ef846696c40@codeaurora.org>
On Thu, May 17, 2018 at 05:16:13PM -0700, David Collins wrote:
> On 05/17/2018 02:22 PM, Doug Anderson wrote:
> > On Fri, May 11, 2018 at 7:28 PM, David Collins <collinsd@codeaurora.org> wrote:
> >> +- qcom,regulator-initial-microvolt
> >> + Usage: optional; VRM regulators only
> >> + Value type: <u32>
> >> + Definition: Specifies the initial voltage in microvolts to request for a
> >> + VRM regulator.
> >
> > Now that Mark has landed the patch adding support for the
> > -ENOTRECOVERABLE error code from get_voltage() / get_voltage_sel(), do
> > we still need the qcom,regulator-initial-microvolt property?
>
> Yes, this is still needed. The -ENOTRECOVERABLE patch ensures that
> qcom-rpmh-regulator devices can be registered even if
> qcom,regulator-initial-microvolt is not specified. However, that will
> result in the regulators being configured for the minimum voltage
> supported in the DT specified min/max range. The
> qcom,regulator-initial-microvolt property allows us to set a specific
> voltage that is larger than the min constraint.
>
> > If this is really still needed, can it be moved to the regulator core?
>
> I'm not opposed to the idea, but I think that Mark is [1]:
>
> >> Do you have a preference for qcom,regulator-initial-microvolt vs a generic
> >> framework supported regulator-initial-microvolt property for configuring a
> >> specific voltage at registration time? We'll need to have support for one
> >> or the other in order for the qcom_rpmh-regulator driver to be functional.
> >
> > This is basically specific to Qualcomm, I can't off hand think of any
> > other devices with similar issues.
>
>
> >> +- regulator-initial-mode
> >> + Usage: optional; VRM regulators only
> >> + Value type: <u32>
> >> + Definition: Specifies the initial mode to request for a VRM regulator.
> >> + Supported values are RPMH_REGULATOR_MODE_* which are defined
> >> + in [1] (i.e. 0 to 3). This property may be specified even
> >> + if the regulator-allow-set-load property is not specified.
> >
> > Every time I read the above I wonder why you're documenting a standard
> > regulator regulator property in your bindings. ...then I realize it's
> > because you're doing it because you want to explicitly document what
> > the valid modes are. I wonder if it makes sense to just put a
> > reference somewhere else in this document to go look at the header
> > file where these are all nicely documented.
>
> Isn't that what the [1] in the above snippet is currently doing. Further
> down in qcom,rpmh-regulator.txt is this line:
>
> +[1] include/dt-bindings/regulator/qcom,rpmh-regulator.h
>
>
> > Speaking of documenting things like that, it might be worth finding
> > somewhere in this doc to mention that the "bob" regulator on PMI8998
> > can support "regulator-allow-bypass". That tidbit got lost when we
> > moved to the standard regulator bindings for bypass.
>
> I suppose that I could add something like this:
>
> +- regulator-allow-bypass
> + Usage: optional; BOB type VRM regulators only
> + Value type: <empty>
> + Definition: See [2] for details.
> ...
> +[2]: Documentation/devicetree/bindings/regulator.txt
>
> However, I don't want the patch to get NACKed because it is defining a
> property that is already defined in the common regulator.txt file.
If all constraints are defined in the common doc, just "see
regulator.txt" is fine. You just need to say what properties this
binding uses.
Rob
^ permalink raw reply
* Re: [PATCH v3 2/6] mfd: at91-usart: added mfd driver for usart
From: Rob Herring @ 2018-05-18 22:19 UTC (permalink / raw)
To: Radu Pirea
Cc: devicetree, linux-serial, linux-kernel, linux-arm-kernel,
linux-spi, mark.rutland, lee.jones, gregkh, jslaby,
richard.genoud, alexandre.belloni, nicolas.ferre, broonie
In-Reply-To: <20180511103822.31698-3-radu.pirea@microchip.com>
On Fri, May 11, 2018 at 01:38:18PM +0300, Radu Pirea wrote:
> This mfd driver is just a wrapper over atmel_serial driver and
> spi-at91-usart driver. Selection of one of the drivers is based on a
> property from device tree. If the property is not specified, the default
> driver is atmel_serial.
>
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
> drivers/mfd/Kconfig | 10 ++++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/at91-usart.c | 75 ++++++++++++++++++++++++++++
> include/dt-bindings/mfd/at91-usart.h | 17 +++++++
> 4 files changed, 103 insertions(+)
> create mode 100644 drivers/mfd/at91-usart.c
> create mode 100644 include/dt-bindings/mfd/at91-usart.h
>
> +#ifndef __DT_BINDINGS_AT91_USART_H__
> +#define __DT_BINDINGS_AT91_USART_H__
> +
> +#define AT91_USART_MODE_SERIAL 1
> +#define AT91_USART_MODE_SPI 2
Won't this require a DT update for serial mode to add the mode property?
That breaks compatibility.
Rob
^ permalink raw reply
* Re: [PATCH v4 06/12] media: dt-bindings: add bindings for i.MX7 media driver
From: Sakari Ailus @ 2018-05-18 22:13 UTC (permalink / raw)
To: Rui Miguel Silva
Cc: devel, devicetree, Greg Kroah-Hartman, Ryan Harkin, Rob Herring,
Philipp Zabel, Steve Longerbeam, Fabio Estevam, mchehab,
Shawn Guo, linux-media
In-Reply-To: <m3tvr5xt9t.fsf@linaro.org>
On Fri, May 18, 2018 at 09:27:58AM +0100, Rui Miguel Silva wrote:
> > > +endpoint node
> > > +-------------
> > > +
> > > +- data-lanes : (required) an array specifying active physical
> > > MIPI-CSI2
> > > + data input lanes and their mapping to logical lanes; the
> > > + array's content is unused, only its length is meaningful;
Btw. do note that you may get a warning due to this from the CSI-2 bus
property parsing code if the lane numbers are wrong.
> > > +
> > > +- fsl,csis-hs-settle : (optional) differential receiver (HS-RX)
> > > settle time;
> >
> > Could you calculate this, as other drivers do? It probably changes
> > depending on the device runtime configuration.
>
> The only reference to possible values to this parameter is given by
> table in [0], can you point me out the formula for imx7 in the
> documentation?
I don't know imx7 but the other CSI-2 drivers need no such system specific
configuration.
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply
* Re: [PATCH v5 06/12] media: dt-bindings: add bindings for i.MX7 media driver
From: Sakari Ailus @ 2018-05-18 22:05 UTC (permalink / raw)
To: Rui Miguel Silva
Cc: devel, devicetree, Greg Kroah-Hartman, Ryan Harkin, Rob Herring,
Philipp Zabel, Steve Longerbeam, Fabio Estevam, mchehab,
Shawn Guo, linux-clk, linux-media
In-Reply-To: <20180518092806.3829-7-rui.silva@linaro.org>
Hi Rui,
On Fri, May 18, 2018 at 10:28:00AM +0100, Rui Miguel Silva wrote:
> Add bindings documentation for i.MX7 media drivers.
>
> Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
> ---
> .../devicetree/bindings/media/imx7.txt | 125 ++++++++++++++++++
> 1 file changed, 125 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/imx7.txt
>
> diff --git a/Documentation/devicetree/bindings/media/imx7.txt b/Documentation/devicetree/bindings/media/imx7.txt
> new file mode 100644
> index 000000000000..a26372630377
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/imx7.txt
> @@ -0,0 +1,125 @@
> +Freescale i.MX7 Media Video Device
> +==================================
> +
> +mipi_csi2 node
> +--------------
> +
> +This is the device node for the MIPI CSI-2 receiver core in i.MX7 SoC. It is
> +compatible with previous version of Samsung D-phy.
> +
> +Required properties:
> +
> +- compatible : "fsl,imx7-mipi-csi2";
> +- reg : base address and length of the register set for the device;
> +- interrupts : should contain MIPI CSIS interrupt;
> +- clocks : list of clock specifiers, see
> + Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
> +- clock-names : must contain "pclk", "wrap" and "phy" entries, matching
> + entries in the clock property;
> +- power-domains : a phandle to the power domain, see
> + Documentation/devicetree/bindings/power/power_domain.txt for details.
> +- reset-names : should include following entry "mrst";
> +- resets : a list of phandle, should contain reset entry of
> + reset-names;
> +- phy-supply : from the generic phy bindings, a phandle to a regulator that
> + provides power to MIPI CSIS core;
> +- bus-width : maximum number of data lanes supported (SoC specific);
> +
> +Optional properties:
> +
> +- clock-frequency : The IP's main (system bus) clock frequency in Hz, default
> + value when this property is not specified is 166 MHz;
> +
> +port node
> +---------
> +
> +- reg : (required) can take the values 0 or 1, where 0 is the
> + related sink port and port 1 should be the source one;
I don't have a datasheet --- does it discuss the hardware block's
interfaces in these terms?
> +
> +endpoint node
> +-------------
> +
> +- data-lanes : (required) an array specifying active physical MIPI-CSI2
> + data input lanes and their mapping to logical lanes; the
> + array's content is unused, only its length is meaningful;
> +
> +- fsl,csis-hs-settle : (optional) differential receiver (HS-RX) settle time;
> +
> +example:
> +
> + mipi_csi: mipi-csi@30750000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + compatible = "fsl,imx7-mipi-csi2";
> + reg = <0x30750000 0x10000>;
> + interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks IMX7D_IPG_ROOT_CLK>,
> + <&clks IMX7D_MIPI_CSI_ROOT_CLK>,
> + <&clks IMX7D_MIPI_DPHY_ROOT_CLK>;
> + clock-names = "pclk", "wrap", "phy";
> + clock-names = "mipi", "phy";
> + clock-frequency = <166000000>;
> + power-domains = <&pgc_mipi_phy>;
> + phy-supply = <®_1p0d>;
> + resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
> + reset-names = "mrst";
> + bus-width = <4>;
> + fsl,csis-hs-settle = <3>;
> + fsl,csis-clk-settle = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + mipi_from_sensor: endpoint {
> + remote-endpoint = <&ov2680_to_mipi>;
> + data-lanes = <1>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + mipi_vc0_to_csi_mux: endpoint {
> + remote-endpoint = <&csi_mux_from_mipi_vc0>;
> + };
> + };
> + };
> +
> +
> +csi node
> +--------
> +
> +This is device node for the CMOS Sensor Interface (CSI) which enables the chip
> +to connect directly to external CMOS image sensors.
> +
> +Required properties:
> +
> +- compatible : "fsl,imx7-csi";
> +- reg : base address and length of the register set for the device;
> +- interrupts : should contain CSI interrupt;
> +- clocks : list of clock specifiers, see
> + Documentation/devicetree/bindings/clock/clock-bindings.txt for details;
> +- clock-names : must contain "axi", "mclk" and "dcic" entries, matching
> + entries in the clock property;
> +
> +example:
> +
> + csi: csi@30710000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + compatible = "fsl,imx7-csi";
> + reg = <0x30710000 0x10000>;
> + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks IMX7D_CLK_DUMMY>,
> + <&clks IMX7D_CSI_MCLK_ROOT_CLK>,
> + <&clks IMX7D_CLK_DUMMY>;
> + clock-names = "axi", "mclk", "dcic";
> +
> + port {
> + csi_from_csi_mux: endpoint {
> + remote-endpoint = <&csi_mux_to_csi>;
> + };
> + };
> + };
> --
> 2.17.0
>
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply
* Re: [PATCH v5 08/12] ARM: dts: imx7s: add multiplexer controls
From: Rui Miguel Silva @ 2018-05-18 21:54 UTC (permalink / raw)
To: Rob Herring
Cc: devel, devicetree, sakari.ailus, Greg Kroah-Hartman, Ryan Harkin,
Rui Miguel Silva, Philipp Zabel, Steve Longerbeam, Fabio Estevam,
mchehab, Shawn Guo, linux-clk, linux-media
In-Reply-To: <20180518165146.GC21131@rob-hp-laptop>
Hi Rob,
On Fri 18 May 2018 at 16:51, Rob Herring wrote:
> On Fri, May 18, 2018 at 10:28:02AM +0100, Rui Miguel Silva
> wrote:
>> The IOMUXC General Purpose Register has bitfield to control
>> video bus
>> multiplexer to control the CSI input between the MIPI-CSI2 and
>> parallel
>> interface. Add that register and mask.
>>
>> Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
>> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
>> ---
>> arch/arm/boot/dts/imx7s.dtsi | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/imx7s.dtsi
>> b/arch/arm/boot/dts/imx7s.dtsi
>> index 67450ad89940..3590dab529f9 100644
>> --- a/arch/arm/boot/dts/imx7s.dtsi
>> +++ b/arch/arm/boot/dts/imx7s.dtsi
>> @@ -520,8 +520,14 @@
>>
>> gpr: iomuxc-gpr@30340000 {
>> compatible =
>> "fsl,imx7d-iomuxc-gpr",
>> - "fsl,imx6q-iomuxc-gpr",
>> "syscon";
>> + "fsl,imx6q-iomuxc-gpr",
>> "syscon", "simple-mfd";
>> reg = <0x30340000 0x10000>;
>> +
>> + mux: mux-controller {
>> + compatible = "mmio-mux";
>> + #mux-control-cells = <1>;
>> + mux-reg-masks = <0x14
>> 0x00000010>;
>
> If 1 bit control, then #mux-control-cells can be 0.
Ack.
>
>> + };
>> };
>>
>> ocotp: ocotp-ctrl@30350000 {
>> --
>> 2.17.0
>>
^ permalink raw reply
* Re: [PATCH v5 07/12] ARM: dts: imx7s: add mipi phy power domain
From: Rui Miguel Silva @ 2018-05-18 21:54 UTC (permalink / raw)
To: Rob Herring
Cc: devel, devicetree, sakari.ailus, Greg Kroah-Hartman, Ryan Harkin,
Rui Miguel Silva, Philipp Zabel, Steve Longerbeam, Fabio Estevam,
mchehab, Shawn Guo, linux-clk, linux-media
In-Reply-To: <20180518165022.GB21131@rob-hp-laptop>
Hi Rob,
On Fri 18 May 2018 at 16:50, Rob Herring wrote:
> On Fri, May 18, 2018 at 10:28:01AM +0100, Rui Miguel Silva
> wrote:
>> Add power domain index 0 related with mipi-phy to imx7s.
>>
>> Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
>> ---
>> arch/arm/boot/dts/imx7s.dtsi | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx7s.dtsi
>> b/arch/arm/boot/dts/imx7s.dtsi
>> index 4d42335c0dee..67450ad89940 100644
>> --- a/arch/arm/boot/dts/imx7s.dtsi
>> +++ b/arch/arm/boot/dts/imx7s.dtsi
>> @@ -636,6 +636,12 @@
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> + pgc_mipi_phy:
>> pgc-power-domain@0 {
>
> power-domain@0
I can change it, but...
>
>> +
>> #power-domain-cells = <0>;
>> + reg = <0>;
>> + power-supply =
>> <®_1p0d>;
>> + };
>> +
>> pgc_pcie_phy:
>> pgc-power-domain@1 {
>
> ditto.
This was not introduced by my patch, it is already there and I was
trying to be coherent with the existing naming.
but let me know if you like me to change it also.
---
Cheers,
Rui
^ permalink raw reply
* Re: [PATCH v5 06/12] media: dt-bindings: add bindings for i.MX7 media driver
From: Rui Miguel Silva @ 2018-05-18 21:52 UTC (permalink / raw)
To: Rob Herring
Cc: devel, devicetree, sakari.ailus, Greg Kroah-Hartman, Ryan Harkin,
Rui Miguel Silva, Philipp Zabel, Steve Longerbeam, Fabio Estevam,
mchehab, Shawn Guo, linux-clk, linux-media
In-Reply-To: <20180518164920.GA21131@rob-hp-laptop>
Hi Rob,
Thanks for your review, this and the other ones.
On Fri 18 May 2018 at 16:49, Rob Herring wrote:
> On Fri, May 18, 2018 at 10:28:00AM +0100, Rui Miguel Silva
> wrote:
>> Add bindings documentation for i.MX7 media drivers.
>>
>> Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
>> ---
>> .../devicetree/bindings/media/imx7.txt | 125
>> ++++++++++++++++++
>> 1 file changed, 125 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/media/imx7.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/imx7.txt
>> b/Documentation/devicetree/bindings/media/imx7.txt
>> new file mode 100644
>> index 000000000000..a26372630377
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/imx7.txt
>> @@ -0,0 +1,125 @@
>> +Freescale i.MX7 Media Video Device
>> +==================================
>> +
>> +mipi_csi2 node
>> +--------------
>> +
>> +This is the device node for the MIPI CSI-2 receiver core in
>> i.MX7 SoC. It is
>> +compatible with previous version of Samsung D-phy.
>> +
>> +Required properties:
>> +
>> +- compatible : "fsl,imx7-mipi-csi2";
>> +- reg : base address and length of the register set
>> for the device;
>> +- interrupts : should contain MIPI CSIS interrupt;
>> +- clocks : list of clock specifiers, see
>> +
>> Documentation/devicetree/bindings/clock/clock-bindings.txt for
>> details;
>> +- clock-names : must contain "pclk", "wrap" and "phy"
>> entries, matching
>> + entries in the clock property;
>> +- power-domains : a phandle to the power domain, see
>> +
>> Documentation/devicetree/bindings/power/power_domain.txt for
>> details.
>> +- reset-names : should include following entry "mrst";
>> +- resets : a list of phandle, should contain reset
>> entry of
>> + reset-names;
>> +- phy-supply : from the generic phy bindings, a phandle to
>> a regulator that
>> + provides power to MIPI CSIS core;
>> +- bus-width : maximum number of data lanes supported (SoC
>> specific);
>
> This property is for parallel buses. Either just rely on
> data-lanes in
> the graph or this should be implied by the compatible.
Ack, will remove it.
>
>> +
>> +Optional properties:
>> +
>> +- clock-frequency : The IP's main (system bus) clock frequency
>> in Hz, default
>> + value when this property is not specified is
>> 166 MHz;
>> +
>> +port node
>> +---------
>> +
>> +- reg : (required) can take the values 0 or 1,
>> where 0 is the
>> + related sink port and port 1 should be
>> the source one;
>> +
>> +endpoint node
>> +-------------
>> +
>> +- data-lanes : (required) an array specifying active
>> physical MIPI-CSI2
>> + data input lanes and their mapping to logical
>> lanes; the
>> + array's content is unused, only its length is
>> meaningful;
>> +
>> +- fsl,csis-hs-settle : (optional) differential receiver
>> (HS-RX) settle time;
>
> units and unit-suffix?
The information about this parameter is not much [0], but I think
it
does not have units, it is the ratio between the Ths-settle and
the
rx-clock period +/- constant.
>
>> +
>> +example:
>> +
>> + mipi_csi: mipi-csi@30750000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + compatible = "fsl,imx7-mipi-csi2";
>> + reg = <0x30750000 0x10000>;
>> + interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&clks IMX7D_IPG_ROOT_CLK>,
>> + <&clks
>> IMX7D_MIPI_CSI_ROOT_CLK>,
>> + <&clks
>> IMX7D_MIPI_DPHY_ROOT_CLK>;
>
>> + clock-names = "pclk", "wrap", "phy";
>> + clock-names = "mipi", "phy";
>
> Err...
Yeah, thanks for notice it.
>
>> + clock-frequency = <166000000>;
>> + power-domains = <&pgc_mipi_phy>;
>> + phy-supply = <®_1p0d>;
>> + resets = <&src IMX7_RESET_MIPI_PHY_MRST>;
>> + reset-names = "mrst";
>> + bus-width = <4>;
>> + fsl,csis-hs-settle = <3>;
>> + fsl,csis-clk-settle = <0>;
>
> Not documented.
It is the other way around, it should not be here in the example
since
its value is always zero. [0] and I remove it from code and dts.
>
>> +
>> + port@0 {
>> + reg = <0>;
>> +
>> + mipi_from_sensor: endpoint {
>> + remote-endpoint =
>> <&ov2680_to_mipi>;
>> + data-lanes = <1>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> +
>> + mipi_vc0_to_csi_mux: endpoint {
>> + remote-endpoint =
>> <&csi_mux_from_mipi_vc0>;
>> + };
>> + };
>> + };
>> +
>> +
>> +csi node
>> +--------
>
> I'd prefer these split into 2 files.
Sure, will do it.
>
>> +
>> +This is device node for the CMOS Sensor Interface (CSI) which
>> enables the chip
>> +to connect directly to external CMOS image sensors.
>> +
>> +Required properties:
>> +
>> +- compatible : "fsl,imx7-csi";
>> +- reg : base address and length of the register set
>> for the device;
>> +- interrupts : should contain CSI interrupt;
>> +- clocks : list of clock specifiers, see
>> +
>> Documentation/devicetree/bindings/clock/clock-bindings.txt for
>> details;
>> +- clock-names : must contain "axi", "mclk" and "dcic"
>> entries, matching
>> + entries in the clock property;
>
> Need the OF graph details.
Ack, will add it in next version.
---
Cheers,
Rui
[0]: https://community.nxp.com/thread/463777
>
>> +
>> +example:
>> +
>> + csi: csi@30710000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + compatible = "fsl,imx7-csi";
>> + reg = <0x30710000 0x10000>;
>> + interrupts = <GIC_SPI 7
>> IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&clks IMX7D_CLK_DUMMY>,
>> + <&clks
>> IMX7D_CSI_MCLK_ROOT_CLK>,
>> + <&clks
>> IMX7D_CLK_DUMMY>;
>> + clock-names = "axi", "mclk", "dcic";
>> +
>> + port {
>> + csi_from_csi_mux: endpoint {
>> + remote-endpoint =
>> <&csi_mux_to_csi>;
>> + };
>> + };
>> + };
>> --
>> 2.17.0
>>
^ permalink raw reply
* Re: [PATCH v4 5/5] remoteproc: qcom: Always assert and deassert reset signals in SDM845
From: Bjorn Andersson @ 2018-05-18 21:47 UTC (permalink / raw)
To: Sibi Sankar
Cc: p.zabel, robh+dt, linux-remoteproc, linux-kernel, devicetree,
georgi.djakov, jassisinghbrar, ohad, mark.rutland, kyan,
sricharan, akdwived, linux-arm-msm, tsoni
In-Reply-To: <20180425150843.26657-6-sibis@codeaurora.org>
On Wed 25 Apr 08:08 PDT 2018, Sibi Sankar wrote:
> SDM845 brings a new reset signal ALT_RESET which is a part of the MSS
> subsystem hence requires some of the active clks to be enabled before
> assert/deassert
>
> Reset the modem if the BOOT FSM does timeout
>
> Reset assert/deassert sequence vary across SoCs adding reset, adding
> start/stop helper functions to handle SoC specific reset sequences
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> drivers/remoteproc/qcom_q6v5_pil.c | 81 ++++++++++++++++++++++++++++--
> 1 file changed, 76 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
[..]
> @@ -349,6 +356,32 @@ static int q6v5_load(struct rproc *rproc, const struct firmware *fw)
> return 0;
> }
>
> +static int q6v5_reset_assert(struct q6v5 *qproc)
> +{
> + return reset_control_assert(qproc->mss_restart);
> +}
> +
> +static int q6v5_reset_deassert(struct q6v5 *qproc)
> +{
> + return reset_control_deassert(qproc->mss_restart);
> +}
> +
> +static int q6v5_alt_reset_assert(struct q6v5 *qproc)
> +{
> + return reset_control_reset(qproc->mss_restart);
> +}
> +
> +static int q6v5_alt_reset_deassert(struct q6v5 *qproc)
> +{
> + /* Ensure alt reset is written before restart reg */
> + writel(1, qproc->rmb_base + RMB_MBA_ALT_RESET);
> +
> + reset_control_reset(qproc->mss_restart);
> +
> + writel(0, qproc->rmb_base + RMB_MBA_ALT_RESET);
> + return 0;
> +}
> +
Rather than having these four functions and scattering jumps to some
function pointer in the code I think it will be shorter and cleaner to
just have the q6v5_reset_{asert,deassert}() functions and in there check
if has_alt_reset and take appropriate action.
> static int q6v5_rmb_pbl_wait(struct q6v5 *qproc, int ms)
> {
> unsigned long timeout;
> @@ -424,6 +457,8 @@ static int q6v5proc_reset(struct q6v5 *qproc)
> val, (val & BIT(0)) != 0, 10, BOOT_FSM_TIMEOUT);
> if (ret) {
> dev_err(qproc->dev, "Boot FSM failed to complete.\n");
> + /* Reset the modem so that boot FSM is in reset state */
> + qproc->reset_deassert(qproc);
A thing like this typically should go into it's own patch, to keep a
clear record of why it was changed, but as this is simply amending the
previous patch it indicates that that one wasn't complete.
So if you reorder the two patches you can just put this directly into
the sdm845 patch, making it "complete".
(This also means that I want to merge the handover vs ready interrupt
patch before that one, so please include it in the next revision of the
series).
> return ret;
> }
>
> @@ -792,12 +827,20 @@ static int q6v5_start(struct rproc *rproc)
> dev_err(qproc->dev, "failed to enable supplies\n");
> goto disable_proxy_clk;
> }
> - ret = reset_control_deassert(qproc->mss_restart);
> +
> + ret = q6v5_clk_enable(qproc->dev, qproc->reset_clks,
> + qproc->reset_clk_count);
Remind me, why can't you always enable the active clock before
deasserting reset? That way we wouldn't have to split out the iface
clock handling to be just slightly longer than the active clocks.
> if (ret) {
> - dev_err(qproc->dev, "failed to deassert mss restart\n");
> + dev_err(qproc->dev, "failed to enable reset clocks\n");
> goto disable_vdd;
> }
>
> + ret = qproc->reset_deassert(qproc);
> + if (ret) {
> + dev_err(qproc->dev, "failed to deassert mss restart\n");
> + goto disable_reset_clks;
> + }
> +
[..]
> @@ -1335,8 +1399,11 @@ static const struct rproc_hexagon_res sdm845_mss = {
> "prng",
> NULL
> },
> - .active_clk_names = (char*[]){
> + .reset_clk_names = (char*[]){
> "iface",
> + NULL
> + },
> + .active_clk_names = (char*[]){
Again, if you reorder your patches to first add the support for
alt_reset and then introduce sdm845 you don't need to modify the
previous patch directly to make it work.
> "bus",
> "mem",
> "gpll0_mss",
Regards,
Bjorn
^ permalink raw reply
* Re: [PATCH 4/5] pinctrl: actions: Add gpio support for Actions S900 SoC
From: Andy Shevchenko @ 2018-05-18 21:43 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Linus Walleij, Rob Herring, Andreas Färber, 刘炜,
mp-cs, 96boards, devicetree, Daniel Thompson, amit.kucheria,
linux-arm Mailing List, open list:GPIO SUBSYSTEM,
Linux Kernel Mailing List, hzhang, bdong, Mani Sadhasivam
In-Reply-To: <20180518023056.7869-5-manivannan.sadhasivam@linaro.org>
On Fri, May 18, 2018 at 5:30 AM, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
> Add gpio support to pinctrl driver for Actions Semi S900 SoC.
>
LGTM,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
> drivers/pinctrl/actions/Kconfig | 1 +
> drivers/pinctrl/actions/pinctrl-owl.c | 206 +++++++++++++++++++++++++++++++++
> drivers/pinctrl/actions/pinctrl-owl.h | 20 ++++
> drivers/pinctrl/actions/pinctrl-s900.c | 29 ++++-
> 4 files changed, 255 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/actions/Kconfig b/drivers/pinctrl/actions/Kconfig
> index ede97cdbbc12..490927b4ea76 100644
> --- a/drivers/pinctrl/actions/Kconfig
> +++ b/drivers/pinctrl/actions/Kconfig
> @@ -4,6 +4,7 @@ config PINCTRL_OWL
> select PINMUX
> select PINCONF
> select GENERIC_PINCONF
> + select GPIOLIB
> help
> Say Y here to enable Actions Semi OWL pinctrl driver
>
> diff --git a/drivers/pinctrl/actions/pinctrl-owl.c b/drivers/pinctrl/actions/pinctrl-owl.c
> index ee090697b1e9..4942e34c8b76 100644
> --- a/drivers/pinctrl/actions/pinctrl-owl.c
> +++ b/drivers/pinctrl/actions/pinctrl-owl.c
> @@ -11,6 +11,7 @@
>
> #include <linux/clk.h>
> #include <linux/err.h>
> +#include <linux/gpio/driver.h>
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/of.h>
> @@ -31,6 +32,7 @@
> * struct owl_pinctrl - pinctrl state of the device
> * @dev: device handle
> * @pctrldev: pinctrl handle
> + * @chip: gpio chip
> * @lock: spinlock to protect registers
> * @soc: reference to soc_data
> * @base: pinctrl register base address
> @@ -38,6 +40,7 @@
> struct owl_pinctrl {
> struct device *dev;
> struct pinctrl_dev *pctrldev;
> + struct gpio_chip chip;
> raw_spinlock_t lock;
> struct clk *clk;
> const struct owl_pinctrl_soc_data *soc;
> @@ -536,6 +539,198 @@ static struct pinctrl_desc owl_pinctrl_desc = {
> .owner = THIS_MODULE,
> };
>
> +static const struct owl_gpio_port *
> +owl_gpio_get_port(struct owl_pinctrl *pctrl, unsigned int *pin)
> +{
> + unsigned int start = 0, i;
> +
> + for (i = 0; i < pctrl->soc->nports; i++) {
> + const struct owl_gpio_port *port = &pctrl->soc->ports[i];
> +
> + if (*pin >= start && *pin < start + port->pins) {
> + *pin -= start;
> + return port;
> + }
> +
> + start += port->pins;
> + }
> +
> + return NULL;
> +}
> +
> +static void owl_gpio_update_reg(void __iomem *base, unsigned int pin, int flag)
> +{
> + u32 val;
> +
> + val = readl_relaxed(base);
> +
> + if (flag)
> + val |= BIT(pin);
> + else
> + val &= ~BIT(pin);
> +
> + writel_relaxed(val, base);
> +}
> +
> +static int owl_gpio_request(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct owl_pinctrl *pctrl = gpiochip_get_data(chip);
> + const struct owl_gpio_port *port;
> + void __iomem *gpio_base;
> + unsigned long flags;
> +
> + port = owl_gpio_get_port(pctrl, &offset);
> + if (WARN_ON(port == NULL))
> + return -ENODEV;
> +
> + gpio_base = pctrl->base + port->offset;
> +
> + /*
> + * GPIOs have higher priority over other modules, so either setting
> + * them as OUT or IN is sufficient
> + */
> + raw_spin_lock_irqsave(&pctrl->lock, flags);
> + owl_gpio_update_reg(gpio_base + port->outen, offset, true);
> + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> + return 0;
> +}
> +
> +static void owl_gpio_free(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct owl_pinctrl *pctrl = gpiochip_get_data(chip);
> + const struct owl_gpio_port *port;
> + void __iomem *gpio_base;
> + unsigned long flags;
> +
> + port = owl_gpio_get_port(pctrl, &offset);
> + if (WARN_ON(port == NULL))
> + return;
> +
> + gpio_base = pctrl->base + port->offset;
> +
> + raw_spin_lock_irqsave(&pctrl->lock, flags);
> + /* disable gpio output */
> + owl_gpio_update_reg(gpio_base + port->outen, offset, false);
> +
> + /* disable gpio input */
> + owl_gpio_update_reg(gpio_base + port->inen, offset, false);
> + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +}
> +
> +static int owl_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct owl_pinctrl *pctrl = gpiochip_get_data(chip);
> + const struct owl_gpio_port *port;
> + void __iomem *gpio_base;
> + unsigned long flags;
> + u32 val;
> +
> + port = owl_gpio_get_port(pctrl, &offset);
> + if (WARN_ON(port == NULL))
> + return -ENODEV;
> +
> + gpio_base = pctrl->base + port->offset;
> +
> + raw_spin_lock_irqsave(&pctrl->lock, flags);
> + val = readl_relaxed(gpio_base + port->dat);
> + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> + return !!(val & BIT(offset));
> +}
> +
> +static void owl_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
> +{
> + struct owl_pinctrl *pctrl = gpiochip_get_data(chip);
> + const struct owl_gpio_port *port;
> + void __iomem *gpio_base;
> + unsigned long flags;
> +
> + port = owl_gpio_get_port(pctrl, &offset);
> + if (WARN_ON(port == NULL))
> + return;
> +
> + gpio_base = pctrl->base + port->offset;
> +
> + raw_spin_lock_irqsave(&pctrl->lock, flags);
> + owl_gpio_update_reg(gpio_base + port->dat, offset, value);
> + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +}
> +
> +static int owl_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
> +{
> + struct owl_pinctrl *pctrl = gpiochip_get_data(chip);
> + const struct owl_gpio_port *port;
> + void __iomem *gpio_base;
> + unsigned long flags;
> +
> + port = owl_gpio_get_port(pctrl, &offset);
> + if (WARN_ON(port == NULL))
> + return -ENODEV;
> +
> + gpio_base = pctrl->base + port->offset;
> +
> + raw_spin_lock_irqsave(&pctrl->lock, flags);
> + owl_gpio_update_reg(gpio_base + port->outen, offset, false);
> + owl_gpio_update_reg(gpio_base + port->inen, offset, true);
> + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> + return 0;
> +}
> +
> +static int owl_gpio_direction_output(struct gpio_chip *chip,
> + unsigned int offset, int value)
> +{
> + struct owl_pinctrl *pctrl = gpiochip_get_data(chip);
> + const struct owl_gpio_port *port;
> + void __iomem *gpio_base;
> + unsigned long flags;
> +
> + port = owl_gpio_get_port(pctrl, &offset);
> + if (WARN_ON(port == NULL))
> + return -ENODEV;
> +
> + gpio_base = pctrl->base + port->offset;
> +
> + raw_spin_lock_irqsave(&pctrl->lock, flags);
> + owl_gpio_update_reg(gpio_base + port->inen, offset, false);
> + owl_gpio_update_reg(gpio_base + port->outen, offset, true);
> + owl_gpio_update_reg(gpio_base + port->dat, offset, value);
> + raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> + return 0;
> +}
> +
> +static int owl_gpio_init(struct owl_pinctrl *pctrl)
> +{
> + struct gpio_chip *chip;
> + int ret;
> +
> + chip = &pctrl->chip;
> + chip->base = -1;
> + chip->ngpio = pctrl->soc->ngpios;
> + chip->label = dev_name(pctrl->dev);
> + chip->parent = pctrl->dev;
> + chip->owner = THIS_MODULE;
> + chip->of_node = pctrl->dev->of_node;
> +
> + ret = gpiochip_add_data(&pctrl->chip, pctrl);
> + if (ret) {
> + dev_err(pctrl->dev, "failed to register gpiochip\n");
> + return ret;
> + }
> +
> + ret = gpiochip_add_pin_range(&pctrl->chip, dev_name(pctrl->dev),
> + 0, 0, chip->ngpio);
> + if (ret) {
> + dev_err(pctrl->dev, "failed to add pin range\n");
> + gpiochip_remove(&pctrl->chip);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> int owl_pinctrl_probe(struct platform_device *pdev,
> struct owl_pinctrl_soc_data *soc_data)
> {
> @@ -571,6 +766,13 @@ int owl_pinctrl_probe(struct platform_device *pdev,
> owl_pinctrl_desc.pins = soc_data->pins;
> owl_pinctrl_desc.npins = soc_data->npins;
>
> + pctrl->chip.direction_input = owl_gpio_direction_input;
> + pctrl->chip.direction_output = owl_gpio_direction_output;
> + pctrl->chip.get = owl_gpio_get;
> + pctrl->chip.set = owl_gpio_set;
> + pctrl->chip.request = owl_gpio_request;
> + pctrl->chip.free = owl_gpio_free;
> +
> pctrl->soc = soc_data;
> pctrl->dev = &pdev->dev;
>
> @@ -581,6 +783,10 @@ int owl_pinctrl_probe(struct platform_device *pdev,
> return PTR_ERR(pctrl->pctrldev);
> }
>
> + ret = owl_gpio_init(pctrl);
> + if (ret)
> + return ret;
> +
> platform_set_drvdata(pdev, pctrl);
>
> return 0;
> diff --git a/drivers/pinctrl/actions/pinctrl-owl.h b/drivers/pinctrl/actions/pinctrl-owl.h
> index 448f81a6db3b..74342378937c 100644
> --- a/drivers/pinctrl/actions/pinctrl-owl.h
> +++ b/drivers/pinctrl/actions/pinctrl-owl.h
> @@ -114,6 +114,22 @@ struct owl_pinmux_func {
> unsigned int ngroups;
> };
>
> +/**
> + * struct owl_gpio_port - Actions GPIO port info
> + * @offset: offset of the GPIO port.
> + * @pins: number of pins belongs to the GPIO port.
> + * @outen: offset of the output enable register.
> + * @inen: offset of the input enable register.
> + * @dat: offset of the data register.
> + */
> +struct owl_gpio_port {
> + unsigned int offset;
> + unsigned int pins;
> + unsigned int outen;
> + unsigned int inen;
> + unsigned int dat;
> +};
> +
> /**
> * struct owl_pinctrl_soc_data - Actions pin controller driver configuration
> * @pins: array describing all pins of the pin controller.
> @@ -124,6 +140,8 @@ struct owl_pinmux_func {
> * @ngroups: number of entries in @groups.
> * @padinfo: array describing the pad info of this SoC.
> * @ngpios: number of pingroups the driver should expose as GPIOs.
> + * @port: array describing all GPIO ports of this SoC.
> + * @nports: number of GPIO ports in this SoC.
> */
> struct owl_pinctrl_soc_data {
> const struct pinctrl_pin_desc *pins;
> @@ -134,6 +152,8 @@ struct owl_pinctrl_soc_data {
> unsigned int ngroups;
> const struct owl_padinfo *padinfo;
> unsigned int ngpios;
> + const struct owl_gpio_port *ports;
> + unsigned int nports;
> };
>
> int owl_pinctrl_probe(struct platform_device *pdev,
> diff --git a/drivers/pinctrl/actions/pinctrl-s900.c b/drivers/pinctrl/actions/pinctrl-s900.c
> index 08d93f8fc086..5503c7945764 100644
> --- a/drivers/pinctrl/actions/pinctrl-s900.c
> +++ b/drivers/pinctrl/actions/pinctrl-s900.c
> @@ -33,6 +33,13 @@
> #define PAD_SR1 (0x0274)
> #define PAD_SR2 (0x0278)
>
> +#define OWL_GPIO_PORT_A 0
> +#define OWL_GPIO_PORT_B 1
> +#define OWL_GPIO_PORT_C 2
> +#define OWL_GPIO_PORT_D 3
> +#define OWL_GPIO_PORT_E 4
> +#define OWL_GPIO_PORT_F 5
> +
> #define _GPIOA(offset) (offset)
> #define _GPIOB(offset) (32 + (offset))
> #define _GPIOC(offset) (64 + (offset))
> @@ -1814,6 +1821,24 @@ static struct owl_padinfo s900_padinfo[NUM_PADS] = {
> [SGPIO3] = PAD_INFO_PULLCTL_ST(SGPIO3)
> };
>
> +#define OWL_GPIO_PORT(port, base, count, _outen, _inen, _dat) \
> + [OWL_GPIO_PORT_##port] = { \
> + .offset = base, \
> + .pins = count, \
> + .outen = _outen, \
> + .inen = _inen, \
> + .dat = _dat, \
> + }
> +
> +static const struct owl_gpio_port s900_gpio_ports[] = {
> + OWL_GPIO_PORT(A, 0x0000, 32, 0x0, 0x4, 0x8),
> + OWL_GPIO_PORT(B, 0x000C, 32, 0x0, 0x4, 0x8),
> + OWL_GPIO_PORT(C, 0x0018, 12, 0x0, 0x4, 0x8),
> + OWL_GPIO_PORT(D, 0x0024, 30, 0x0, 0x4, 0x8),
> + OWL_GPIO_PORT(E, 0x0030, 32, 0x0, 0x4, 0x8),
> + OWL_GPIO_PORT(F, 0x00F0, 8, 0x0, 0x4, 0x8)
> +};
> +
> static struct owl_pinctrl_soc_data s900_pinctrl_data = {
> .padinfo = s900_padinfo,
> .pins = (const struct pinctrl_pin_desc *)s900_pads,
> @@ -1822,7 +1847,9 @@ static struct owl_pinctrl_soc_data s900_pinctrl_data = {
> .nfunctions = ARRAY_SIZE(s900_functions),
> .groups = s900_groups,
> .ngroups = ARRAY_SIZE(s900_groups),
> - .ngpios = NUM_GPIOS
> + .ngpios = NUM_GPIOS,
> + .ports = s900_gpio_ports,
> + .nports = ARRAY_SIZE(s900_gpio_ports)
> };
>
> static int s900_pinctrl_probe(struct platform_device *pdev)
> --
> 2.14.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 4/6] dt-bindings: add binding for at91-usart in spi mode
From: Rob Herring @ 2018-05-18 21:40 UTC (permalink / raw)
To: Radu Pirea
Cc: devicetree, linux-serial, linux-kernel, linux-arm-kernel,
linux-spi, mark.rutland, lee.jones, gregkh, jslaby,
richard.genoud, alexandre.belloni, nicolas.ferre, broonie
In-Reply-To: <20180511103822.31698-5-radu.pirea@microchip.com>
On Fri, May 11, 2018 at 01:38:20PM +0300, Radu Pirea wrote:
> These are bindings for at91-usart IP in spi spi mode. There is no support for
s/spi spi/SPI/
> internal chip select. Only kind of chip selects available are gpio chip
GPIO
> selects.
>
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
> .../bindings/spi/microchip,at91-usart-spi.txt | 28 +++++++++++++++++++
> 1 file changed, 28 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
>
> diff --git a/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
> new file mode 100644
> index 000000000000..b68a3bec4121
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
So now we have 2 copies of the same thing that varies by 2 properties?
Please make this one doc.
> @@ -0,0 +1,28 @@
> +* Universal Synchronous Asynchronous Receiver/Transmitter (USART) in SPI mode
> +
> +Required properties:
> +- #size-cells : Must be <0>
> +- #address-cells : Must be <1>
> +- compatible: Should be "atmel,at91rm9200-usart" or "atmel,at91sam9260-usart"
> +- reg: Should contain registers location and length
> +- interrupts: Should contain interrupt
> +- clocks: phandles to input clocks.
> +- clock-names: tuple listing input clock names.
> + Required elements: "usart"
> +- cs-gpios: chipselects (internal cs not supported)
> +- at91,usart-mode: AT91_USART_MODE_SPI (found in dt-bindings/mfd/at91-usart.h)
at91 is not a vendor.
> +
> +Example:
> + #include <dt-bindings/mfd/at91-usart.h>
> +
> + spi0: spi@f001c000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "atmel,at91rm9200-usart", "atmel,at91sam9260-usart";
> + at91,usart-mode = <AT91_USART_MODE_SPI>;
> + reg = <0xf001c000 0x100>;
> + interrupts = <12 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&usart0_clk>;
> + clock-names = "usart";
> + cs-gpios = <&pioB 3 0>;
> + };
> --
> 2.17.0
>
^ permalink raw reply
* Re: [PATCH 2/2] slimbus: ngd: Add qcom SLIMBus NGD driver
From: kbuild test robot @ 2018-05-18 21:39 UTC (permalink / raw)
Cc: mark.rutland, devicetree, alsa-devel, girishm, bgoswami, gregkh,
broonie, linux-kernel, robh+dt, kramasub, kbuild-all,
linux-arm-msm, Srinivas Kandagatla, sdharia
In-Reply-To: <20180516165118.16551-3-srinivas.kandagatla@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 6270 bytes --]
Hi Srinivas,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Srinivas-Kandagatla/slimbus-ngd-dt-bindings-Add-slim-ngd-dt-bindings/20180518-193916
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All error/warnings (new ones prefixed by >>):
drivers/slimbus/qcom-ngd-ctrl.c: In function 'qcom_slim_ngd_get_laddr':
>> drivers/slimbus/qcom-ngd-ctrl.c:862:8: error: implicit declaration of function 'slim_prepare_txn'; did you mean 'slab_prepare_cpu'? [-Werror=implicit-function-declaration]
ret = slim_prepare_txn(sctrl, &txn, &done, true);
^~~~~~~~~~~~~~~~
slab_prepare_cpu
drivers/slimbus/qcom-ngd-ctrl.c: In function 'qcom_slim_ngd_notify_slaves':
>> drivers/slimbus/qcom-ngd-ctrl.c:969:11: error: implicit declaration of function 'of_slim_get_device'; did you mean 'slim_get_device'? [-Werror=implicit-function-declaration]
sbdev = of_slim_get_device(&ctrl->ctrl, node);
^~~~~~~~~~~~~~~~~~
slim_get_device
>> drivers/slimbus/qcom-ngd-ctrl.c:969:9: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
sbdev = of_slim_get_device(&ctrl->ctrl, node);
^
cc1: some warnings being treated as errors
vim +862 drivers/slimbus/qcom-ngd-ctrl.c
839
840 static int qcom_slim_ngd_get_laddr(struct slim_controller *sctrl,
841 struct slim_eaddr *ea, u8 *laddr)
842 {
843 DECLARE_COMPLETION_ONSTACK(done);
844 struct slim_val_inf msg = {0};
845 struct slim_msg_txn txn;
846 u8 wbuf[10] = {0};
847 u8 rbuf[10] = {0};
848 int ret;
849
850 txn.mt = SLIM_MSG_MT_DEST_REFERRED_USER;
851 txn.dt = SLIM_MSG_DEST_LOGICALADDR;
852 txn.la = SLIM_LA_MGR;
853 txn.ec = 0;
854
855 txn.mc = SLIM_USR_MC_ADDR_QUERY;
856 txn.rl = 11;
857 txn.msg = &msg;
858 txn.msg->num_bytes = 7;
859 txn.msg->wbuf = wbuf;
860 txn.msg->rbuf = rbuf;
861
> 862 ret = slim_prepare_txn(sctrl, &txn, &done, true);
863 if (ret)
864 return ret;
865
866 wbuf[0] = (u8)txn.tid;
867 memcpy(&wbuf[1], ea, sizeof(*ea));
868 ret = slim_do_transfer(sctrl, &txn);
869
870 *laddr = rbuf[6];
871
872 return ret;
873 }
874
875 static int qcom_slim_ngd_exit_dma(struct qcom_slim_ngd_ctrl *ctrl)
876 {
877 if (ctrl->dma_rx_channel)
878 dma_release_channel(ctrl->dma_rx_channel);
879
880 if (ctrl->dma_tx_channel)
881 dma_release_channel(ctrl->dma_tx_channel);
882
883 ctrl->dma_tx_channel = ctrl->dma_rx_channel = NULL;
884
885 return 0;
886 }
887
888 static void qcom_slim_ngd_setup(struct qcom_slim_ngd_ctrl *ctrl)
889 {
890 u32 cfg = readl_relaxed(ctrl->base +
891 NGD_BASE(ctrl->id, ctrl->ver));
892
893 if (ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN)
894 qcom_slim_ngd_init_dma(ctrl);
895
896 /* By default enable message queues */
897 cfg |= NGD_CFG_RX_MSGQ_EN;
898 cfg |= NGD_CFG_TX_MSGQ_EN;
899
900 /* Enable NGD if it's not already enabled*/
901 if (!(cfg & NGD_CFG_ENABLE))
902 cfg |= NGD_CFG_ENABLE;
903
904 writel_relaxed(cfg, ctrl->base + NGD_BASE(ctrl->id, ctrl->ver));
905 }
906
907 static int qcom_slim_ngd_power_up(struct qcom_slim_ngd_ctrl *ctrl)
908 {
909 enum qcom_slim_ngd_state cur_state = ctrl->state;
910 void __iomem *ngd;
911 u32 laddr, rx_msgq;
912 int timeout, ret = 0;
913
914 if (ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN) {
915 timeout = wait_for_completion_timeout(&ctrl->qmi.qmi_comp, HZ);
916 if (!timeout)
917 return -EREMOTEIO;
918 }
919
920 if (ctrl->state == QCOM_SLIM_NGD_CTRL_ASLEEP ||
921 ctrl->state == QCOM_SLIM_NGD_CTRL_DOWN) {
922 ret = qcom_slim_qmi_power_request(ctrl, true);
923 if (ret) {
924 dev_err(ctrl->dev, "SLIM QMI power request failed:%d\n",
925 ret);
926 return ret;
927 }
928 }
929
930 ctrl->ver = readl_relaxed(ctrl->base);
931 /* Version info in 16 MSbits */
932 ctrl->ver >>= 16;
933 ngd = ctrl->base + NGD_BASE(ctrl->id, ctrl->ver);
934 laddr = readl_relaxed(ngd + NGD_STATUS);
935 if (laddr & NGD_LADDR) {
936 /*
937 * external MDM restart case where ADSP itself was active framer
938 * For example, modem restarted when playback was active
939 */
940 if (cur_state == QCOM_SLIM_NGD_CTRL_AWAKE) {
941 dev_info(ctrl->dev, "Subsys restart: ADSP active framer\n");
942 return 0;
943 }
944 return 0;
945 }
946
947 writel_relaxed(DEF_NGD_INT_MASK, ctrl->base + NGD_INT_EN +
948 NGD_BASE(ctrl->id, ctrl->ver));
949 rx_msgq = readl_relaxed(ngd + NGD_RX_MSGQ_CFG);
950
951 writel_relaxed(rx_msgq|SLIM_RX_MSGQ_TIMEOUT_VAL, ngd + NGD_RX_MSGQ_CFG);
952 qcom_slim_ngd_setup(ctrl);
953
954 timeout = wait_for_completion_timeout(&ctrl->reconf, HZ);
955 if (!timeout) {
956 dev_err(ctrl->dev, "capability exchange timed-out\n");
957 return -ETIMEDOUT;
958 }
959
960 return 0;
961 }
962
963 static void qcom_slim_ngd_notify_slaves(struct qcom_slim_ngd_ctrl *ctrl)
964 {
965 struct slim_device *sbdev;
966 struct device_node *node;
967
968 for_each_child_of_node(ctrl->dev->of_node, node) {
> 969 sbdev = of_slim_get_device(&ctrl->ctrl, node);
970 if (!sbdev)
971 continue;
972
973 if (slim_get_logical_addr(sbdev))
974 dev_err(ctrl->dev, "Failed to get logical address\n");
975 }
976 }
977
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 65207 bytes --]
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH v3 6/8] MIPS: jz4780: dts: Fix watchdog node
From: Rob Herring @ 2018-05-18 21:31 UTC (permalink / raw)
To: Paul Cercueil
Cc: Guenter Roeck, Mark Rutland, Ralf Baechle, James Hogan,
Wim Van Sebroeck, Mathieu Malaterre, linux-watchdog, devicetree,
linux-kernel, linux-mips
In-Reply-To: <20180510184751.13416-6-paul@crapouillou.net>
On Thu, May 10, 2018 at 08:47:49PM +0200, Paul Cercueil wrote:
> - The previous node requested a memory area of 0x100 bytes, while the
> driver only manipulates four registers present in the first 0x10 bytes.
>
> - The driver requests for the "rtc" clock, but the previous node did not
> provide any.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Reviewed-by: Mathieu Malaterre <malat@debian.org>
> Acked-by: James Hogan <jhogan@kernel.org>
> ---
> Documentation/devicetree/bindings/watchdog/ingenic,jz4740-wdt.txt | 7 ++++++-
> arch/mips/boot/dts/ingenic/jz4780.dtsi | 5 ++++-
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> v2: No change
> v3: Also fix documentation
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 4/5] remoteproc: qcom: Add support for mss remoteproc on SDM845
From: Bjorn Andersson @ 2018-05-18 21:31 UTC (permalink / raw)
To: Sibi Sankar
Cc: p.zabel, robh+dt, linux-remoteproc, linux-kernel, devicetree,
georgi.djakov, jassisinghbrar, ohad, mark.rutland, kyan,
sricharan, akdwived, linux-arm-msm, tsoni
In-Reply-To: <20180425150843.26657-5-sibis@codeaurora.org>
On Wed 25 Apr 08:08 PDT 2018, Sibi Sankar wrote:
> From SDM845, the Q6SS reset sequence on software side has been
> simplified with the introduction of boot FSM which assists in
> bringing the Q6 out of reset
>
> Add GLINK subdevice to allow definition of GLINK edge as a
> child of modem-pil
>
Please split this in two patches; one adding sdm845 and one adding the
glink subdev. You can squash in the addition of the compatible in the dt
binding into the sdm845 code patch, you wish as well.
Apart from that this looks good!
Regards,
Bjorn
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> drivers/remoteproc/qcom_q6v5_pil.c | 65 +++++++++++++++++++++++++++++-
> 1 file changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
> index 7e2d04d4f2f0..4d9504e8bf8e 100644
> --- a/drivers/remoteproc/qcom_q6v5_pil.c
> +++ b/drivers/remoteproc/qcom_q6v5_pil.c
> @@ -57,6 +57,8 @@
> #define RMB_PMI_META_DATA_REG 0x10
> #define RMB_PMI_CODE_START_REG 0x14
> #define RMB_PMI_CODE_LENGTH_REG 0x18
> +#define RMB_MBA_MSS_STATUS 0x40
> +#define RMB_MBA_ALT_RESET 0x44
>
> #define RMB_CMD_META_DATA_READY 0x1
> #define RMB_CMD_LOAD_READY 0x2
> @@ -104,6 +106,13 @@
> #define QDSP6SS_XO_CBCR 0x0038
> #define QDSP6SS_ACC_OVERRIDE_VAL 0x20
>
> +/* QDSP6v65 parameters */
> +#define QDSP6SS_SLEEP 0x3C
> +#define QDSP6SS_BOOT_CORE_START 0x400
> +#define QDSP6SS_BOOT_CMD 0x404
> +#define SLEEP_CHECK_MAX_LOOPS 200
> +#define BOOT_FSM_TIMEOUT 10000
> +
> struct reg_info {
> struct regulator *reg;
> int uV;
> @@ -170,6 +179,7 @@ struct q6v5 {
> void *mpss_region;
> size_t mpss_size;
>
> + struct qcom_rproc_glink glink_subdev;
> struct qcom_rproc_subdev smd_subdev;
> struct qcom_rproc_ssr ssr_subdev;
> struct qcom_sysmon *sysmon;
> @@ -184,6 +194,7 @@ enum {
> MSS_MSM8916,
> MSS_MSM8974,
> MSS_MSM8996,
> + MSS_SDM845,
> };
>
> static int q6v5_regulator_init(struct device *dev, struct reg_info *regs,
> @@ -390,8 +401,35 @@ static int q6v5proc_reset(struct q6v5 *qproc)
> int ret;
> int i;
>
> + if (qproc->version == MSS_SDM845) {
>
> - if (qproc->version == MSS_MSM8996) {
> + val = readl(qproc->reg_base + QDSP6SS_SLEEP);
> + val |= 0x1;
> + writel(val, qproc->reg_base + QDSP6SS_SLEEP);
> +
> + ret = readl_poll_timeout(qproc->reg_base + QDSP6SS_SLEEP,
> + val, !(val & BIT(31)), 1,
> + SLEEP_CHECK_MAX_LOOPS);
> + if (ret) {
> + dev_err(qproc->dev, "QDSP6SS Sleep clock timed out\n");
> + return -ETIMEDOUT;
> + }
> +
> + /* De-assert QDSP6 stop core */
> + writel(1, qproc->reg_base + QDSP6SS_BOOT_CORE_START);
> + /* Trigger boot FSM */
> + writel(1, qproc->reg_base + QDSP6SS_BOOT_CMD);
> +
> + ret = readl_poll_timeout(qproc->rmb_base + RMB_MBA_MSS_STATUS,
> + val, (val & BIT(0)) != 0, 10, BOOT_FSM_TIMEOUT);
> + if (ret) {
> + dev_err(qproc->dev, "Boot FSM failed to complete.\n");
> + return ret;
> + }
> +
> + goto pbl_wait;
> +
> + } else if (qproc->version == MSS_MSM8996) {
> /* Override the ACC value if required */
> writel(QDSP6SS_ACC_OVERRIDE_VAL,
> qproc->reg_base + QDSP6SS_STRAP_ACC);
> @@ -499,6 +537,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
> val &= ~Q6SS_STOP_CORE;
> writel(val, qproc->reg_base + QDSP6SS_RESET_REG);
>
> +pbl_wait:
> /* Wait for PBL status */
> ret = q6v5_rmb_pbl_wait(qproc, 1000);
> if (ret == -ETIMEDOUT) {
> @@ -1256,6 +1295,7 @@ static int q6v5_probe(struct platform_device *pdev)
> }
> qproc->mpss_perm = BIT(QCOM_SCM_VMID_HLOS);
> qproc->mba_perm = BIT(QCOM_SCM_VMID_HLOS);
> + qcom_add_glink_subdev(rproc, &qproc->glink_subdev);
> qcom_add_smd_subdev(rproc, &qproc->smd_subdev);
> qcom_add_ssr_subdev(rproc, &qproc->ssr_subdev, "mpss");
> qproc->sysmon = qcom_add_sysmon_subdev(rproc, "modem", 0x12);
> @@ -1279,6 +1319,7 @@ static int q6v5_remove(struct platform_device *pdev)
> rproc_del(qproc->rproc);
>
> qcom_remove_sysmon_subdev(qproc->sysmon);
> + qcom_remove_glink_subdev(qproc->rproc, &qproc->glink_subdev);
> qcom_remove_smd_subdev(qproc->rproc, &qproc->smd_subdev);
> qcom_remove_ssr_subdev(qproc->rproc, &qproc->ssr_subdev);
> rproc_free(qproc->rproc);
> @@ -1286,6 +1327,27 @@ static int q6v5_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct rproc_hexagon_res sdm845_mss = {
> + .hexagon_mba_image = "mba.mbn",
> + .proxy_clk_names = (char*[]){
> + "xo",
> + "axis2",
> + "prng",
> + NULL
> + },
> + .active_clk_names = (char*[]){
> + "iface",
> + "bus",
> + "mem",
> + "gpll0_mss",
> + "snoc_axi",
> + "mnoc_axi",
> + NULL
> + },
> + .need_mem_protection = true,
> + .version = MSS_SDM845,
> +};
> +
> static const struct rproc_hexagon_res msm8996_mss = {
> .hexagon_mba_image = "mba.mbn",
> .proxy_clk_names = (char*[]){
> @@ -1379,6 +1441,7 @@ static const struct of_device_id q6v5_of_match[] = {
> { .compatible = "qcom,msm8916-mss-pil", .data = &msm8916_mss},
> { .compatible = "qcom,msm8974-mss-pil", .data = &msm8974_mss},
> { .compatible = "qcom,msm8996-mss-pil", .data = &msm8996_mss},
> + { .compatible = "qcom,sdm845-mss-pil", .data = &sdm845_mss},
> { },
> };
> MODULE_DEVICE_TABLE(of, q6v5_of_match);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
^ 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