This patchset is a trial for DVFS support for Allwinner H3 SoC,
considering two kinds of adjustable regulators used on H3 boards:
SY8106A I2C-controlled regulator and SY8113B regulator (controllable
by GPIO with some special designs on the board).
PATCH 1 and PATCH 2 are for the SY8106A regulator, then PATCH 3 and
PATCH 4 are for the r_i2c bus, which is used by boards with SY8106A
to communicate with the regulator.
PATCH 5 and 6 are CCU fixes for H3 DVFS.
PATCH 7 is adding new compatible strings to the cpufreq-dt driver.
PATCH 8 adds the opp-v2 table for H3 SoC (without OPPs higher than
1.008GHz).
PATCH 9 and 10 finally adds DVFS support for two boards, Orange Pi
Zero (SY8113B) and Orange Pi PC (SY8106A).
Tested on these boards mentioned.
PATCH 9 and 10 finally adds DVFS support for two boards, Orange Pi
Zero (SY8113B) and Orange Pi PC (SY8106A).
Tested on these boards mentioned.
Chen-Yu Tsai (1):
clk: sunxi-ng: h3: gate then ungate PLL CPU clk after rate change
Icenowy Zheng (4):
clk: sunxi-ng: allow set parent clock (PLL_CPUX) for CPUX clock on H3
cpufreq: dt: Add support for some new Allwinner SoCs
ARM: sun8i: h3: add operating-points-v2 table for CPU
ARM: sun8i: h2+: add SY8113B regulator used by Orange Pi Zero board
Ondrej Jirman (5):
dt-bindings: add binding for the SY8160A voltage regulator
regulator: add support for SY8106A regulator
ARM: sunxi: h3/h5: Add r_i2c pinmux node
ARM: sunxi: h3/h5: Add r_i2c I2C controller
ARM: dts: sun8i: Add SY8106A regulator to Orange Pi PC
.../bindings/regulator/sy8106a-regulator.txt | 21 +++
arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts | 21 +++
arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 19 +++
arch/arm/boot/dts/sun8i-h3.dtsi | 38 ++++-
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 19 +++
drivers/clk/sunxi-ng/ccu-sun8i-h3.c | 13 +-
drivers/cpufreq/cpufreq-dt-platdev.c | 6 +
drivers/regulator/Kconfig | 8 +-
drivers/regulator/Makefile | 2 +-
drivers/regulator/sy8106a-regulator.c | 163 +++++++++++++++++++++
10 files changed, 306 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt
create mode 100644 drivers/regulator/sy8106a-regulator.c
--
2.13.0
From: Ondrej Jirman <[email protected]>
SY8106A is an I2C-controlled adjustable voltage regulator made by
Silergy Corp.
Add its device tree binding.
Signed-off-by: Ondrej Jirman <[email protected]>
[Icenowy: Change commit message]
Signed-off-by: Icenowy Zheng <[email protected]>
---
.../bindings/regulator/sy8106a-regulator.txt | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt
diff --git a/Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt b/Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt
new file mode 100644
index 000000000000..1e623a34b1cb
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt
@@ -0,0 +1,21 @@
+SY8106A Voltage regulator
+
+Required properties:
+- compatible: Must be "silergy,sy8106a"
+- reg: I2C slave address - must be <0x65>
+
+Any property defined as part of the core regulator binding, defined in
+regulator.txt, can also be used.
+
+Example:
+
+ sy8106a {
+ compatible = "silergy,sy8106a";
+ reg = <0x65>;
+ regulator-name = "sy8106a-vdd";
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <1400000>;
+ regulator-ramp-delay = <200>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
--
2.13.0
From: Ondrej Jirman <[email protected]>
SY8106A is an I2C attached single output regulator made by Silergy Corp,
which is used on several Allwinner H3/H5 SBCs to control the power
supply of the ARM cores.
Add a driver for it.
Signed-off-by: Ondrej Jirman <[email protected]>
[Icenowy: Change commit message]
Signed-off-by: Icenowy Zheng <[email protected]>
---
drivers/regulator/Kconfig | 8 +-
drivers/regulator/Makefile | 2 +-
drivers/regulator/sy8106a-regulator.c | 163 ++++++++++++++++++++++++++++++++++
3 files changed, 171 insertions(+), 2 deletions(-)
create mode 100644 drivers/regulator/sy8106a-regulator.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 99b9362331b5..1efa73e18d07 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -764,6 +764,13 @@ config REGULATOR_STW481X_VMMC
This driver supports the internal VMMC regulator in the STw481x
PMIC chips.
+config REGULATOR_SY8106A
+ tristate "Silergy SY8106A regulator"
+ depends on I2C && (OF || COMPILE_TEST)
+ select REGMAP_I2C
+ help
+ This driver supports SY8106A single output regulator.
+
config REGULATOR_TPS51632
tristate "TI TPS51632 Power Regulator"
depends on I2C
@@ -938,4 +945,3 @@ config REGULATOR_WM8994
WM8994 CODEC.
endif
-
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 95b1e86ae692..f5120252f86a 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -95,6 +95,7 @@ obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
obj-$(CONFIG_REGULATOR_SKY81452) += sky81452-regulator.o
obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
+obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
obj-$(CONFIG_REGULATOR_TI_ABB) += ti-abb-regulator.o
obj-$(CONFIG_REGULATOR_TPS6105X) += tps6105x-regulator.o
obj-$(CONFIG_REGULATOR_TPS62360) += tps62360-regulator.o
@@ -120,5 +121,4 @@ obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
-
ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/sy8106a-regulator.c b/drivers/regulator/sy8106a-regulator.c
new file mode 100644
index 000000000000..1df889f68b3d
--- /dev/null
+++ b/drivers/regulator/sy8106a-regulator.c
@@ -0,0 +1,163 @@
+/*
+ * sy8106a-regulator.c - Regulator device driver for SY8106A
+ *
+ * Copyright (C) 2016 Ondřej Jirman <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Library General Public License for more details.
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+#define SY8106A_REG_VOUT1_SEL 0x01
+#define SY8106A_REG_VOUT_COM 0x02
+#define SY8106A_REG_VOUT1_SEL_MASK 0x7f
+#define SY8106A_DISABLE_REG BIT(0)
+#define SY8106A_GO_BIT BIT(7)
+
+struct sy8106a {
+ struct regulator_dev *rdev;
+ struct regmap *regmap;
+};
+
+static const struct regmap_config sy8106a_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
+static int sy8106a_set_voltage_sel(struct regulator_dev *rdev, unsigned int sel)
+{
+ /* We use our set_voltage_sel in order to avoid unnecessary I2C
+ * chatter, because the regulator_get_voltage_sel_regmap using
+ * apply_bit would perform 4 unnecessary transfers instead of one,
+ * increasing the chance of error.
+ */
+ return regmap_write(rdev->regmap, rdev->desc->vsel_reg,
+ sel | SY8106A_GO_BIT);
+}
+
+static const struct regulator_ops sy8106a_ops = {
+ .is_enabled = regulator_is_enabled_regmap,
+ .set_voltage_sel = sy8106a_set_voltage_sel,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .list_voltage = regulator_list_voltage_linear,
+};
+
+/* Default limits measured in millivolts and milliamps */
+#define SY8106A_MIN_MV 680
+#define SY8106A_MAX_MV 1950
+#define SY8106A_STEP_MV 10
+
+static const struct regulator_desc sy8106a_reg = {
+ .name = "SY8106A",
+ .id = 0,
+ .ops = &sy8106a_ops,
+ .type = REGULATOR_VOLTAGE,
+ .n_voltages = ((SY8106A_MAX_MV - SY8106A_MIN_MV) / SY8106A_STEP_MV) + 1,
+ .min_uV = (SY8106A_MIN_MV * 1000),
+ .uV_step = (SY8106A_STEP_MV * 1000),
+ .vsel_reg = SY8106A_REG_VOUT1_SEL,
+ .vsel_mask = SY8106A_REG_VOUT1_SEL_MASK,
+ .enable_reg = SY8106A_REG_VOUT_COM,
+ .enable_mask = SY8106A_DISABLE_REG,
+ .disable_val = SY8106A_DISABLE_REG,
+ .enable_is_inverted = 1,
+ .owner = THIS_MODULE,
+};
+
+/*
+ * I2C driver interface functions
+ */
+static int sy8106a_i2c_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ struct sy8106a *chip;
+ struct device *dev = &i2c->dev;
+ struct regulator_dev *rdev = NULL;
+ struct regulator_config config = { };
+ unsigned int selector;
+ int error;
+
+ chip = devm_kzalloc(&i2c->dev, sizeof(struct sy8106a), GFP_KERNEL);
+ if (!chip)
+ return -ENOMEM;
+
+ chip->regmap = devm_regmap_init_i2c(i2c, &sy8106a_regmap_config);
+ if (IS_ERR(chip->regmap)) {
+ error = PTR_ERR(chip->regmap);
+ dev_err(&i2c->dev, "Failed to allocate register map: %d\n",
+ error);
+ return error;
+ }
+
+ config.dev = &i2c->dev;
+ config.regmap = chip->regmap;
+ config.driver_data = chip;
+
+ config.of_node = dev->of_node;
+ config.init_data = of_get_regulator_init_data(dev, dev->of_node,
+ &sy8106a_reg);
+
+ if (!config.init_data)
+ return -ENOMEM;
+
+ /* Probe regulator */
+ error = regmap_read(chip->regmap, SY8106A_REG_VOUT1_SEL, &selector);
+ if (error) {
+ dev_err(&i2c->dev, "Failed to read voltage at probe time: %d\n", error);
+ return error;
+ }
+
+ rdev = devm_regulator_register(&i2c->dev, &sy8106a_reg, &config);
+ if (IS_ERR(rdev)) {
+ error = PTR_ERR(rdev);
+ dev_err(&i2c->dev, "Failed to register SY8106A regulator: %d\n", error);
+ return error;
+ }
+
+ chip->rdev = rdev;
+
+ i2c_set_clientdata(i2c, chip);
+
+ return 0;
+}
+
+static const struct of_device_id sy8106a_i2c_of_match[] = {
+ { .compatible = "silergy,sy8106a" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, sy8106a_i2c_of_match);
+
+static const struct i2c_device_id sy8106a_i2c_id[] = {
+ { "sy8106a", 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(i2c, sy8106a_i2c_id);
+
+static struct i2c_driver sy8106a_regulator_driver = {
+ .driver = {
+ .name = "sy8106a",
+ .of_match_table = of_match_ptr(sy8106a_i2c_of_match),
+ },
+ .probe = sy8106a_i2c_probe,
+ .id_table = sy8106a_i2c_id,
+};
+
+module_i2c_driver(sy8106a_regulator_driver);
+
+MODULE_AUTHOR("Ondřej Jirman <[email protected]>");
+MODULE_DESCRIPTION("Regulator device driver for Silergy SY8106A");
+MODULE_LICENSE("GPL v2");
--
2.13.0
From: Ondrej Jirman <[email protected]>
H3/H5 SoCs contain an I2C controller optionally available
on the PL0 and PL1 pins. This patch adds pinmux configuration
for this controller.
Signed-off-by: Ondrej Jirman <[email protected]>
[Icenowy: change commit message and node name]
Signed-off-by: Icenowy Zheng <[email protected]>
---
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 6f2162608006..b240099bc865 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -639,6 +639,11 @@
pins = "PL11";
function = "s_cir_rx";
};
+
+ r_i2c_pins: r-i2c {
+ pins = "PL0", "PL1";
+ function = "s_twi";
+ };
};
};
};
--
2.13.0
From: Ondrej Jirman <[email protected]>
Allwinner H3/H5 SoCs have an I2C controller at PL GPIO bank.
Add support for it in the device tree.
Signed-off-by: Ondrej Jirman <[email protected]>
[Icenowy: Change to use r_ccu and change pinmux node name]
Signed-off-by: Icenowy Zheng <[email protected]>
---
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index b240099bc865..65643c4710a3 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -624,6 +624,20 @@
status = "disabled";
};
+ r_i2c: i2c@01f02400 {
+ compatible = "allwinner,sun6i-a31-i2c";
+ reg = <0x01f02400 0x400>;
+ interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&r_i2c_pins>;
+ clocks = <&r_ccu CLK_APB0_I2C>;
+ clock-frequency = <100000>;
+ resets = <&r_ccu RST_APB0_I2C>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
r_pio: pinctrl@01f02c00 {
compatible = "allwinner,sun8i-h3-r-pinctrl";
reg = <0x01f02c00 0x400>;
--
2.13.0
From: Chen-Yu Tsai <[email protected]>
This patch utilizes the new PLL clk notifier to gate then ungate the
PLL CPU clock after rate changes. This should prevent any system hangs
resulting from cpufreq changes to the clk.
Reported-by: Ondrej Jirman <[email protected]>
Signed-off-by: Chen-Yu Tsai <[email protected]>
Tested-by: Icenowy Zheng <[email protected]>
---
drivers/clk/sunxi-ng/ccu-sun8i-h3.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-h3.c b/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
index 62e4f0d2b2fc..406d0aac9fd6 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
@@ -1103,6 +1103,13 @@ static const struct sunxi_ccu_desc sun50i_h5_ccu_desc = {
.num_resets = ARRAY_SIZE(sun50i_h5_ccu_resets),
};
+static struct ccu_pll_nb sun8i_h3_pll_cpu_nb = {
+ .common = &pll_cpux_clk.common,
+ /* copy from pll_cpux_clk */
+ .enable = BIT(31),
+ .lock = BIT(28),
+};
+
static struct ccu_mux_nb sun8i_h3_cpu_nb = {
.common = &cpux_clk.common,
.cm = &cpux_clk.mux,
@@ -1130,6 +1137,10 @@ static void __init sunxi_h3_h5_ccu_init(struct device_node *node,
sunxi_ccu_probe(node, reg, desc);
+ /* Gate then ungate PLL CPU after any rate changes */
+ ccu_pll_notifier_register(&sun8i_h3_pll_cpu_nb);
+
+ /* Reparent CPU during PLL CPU rate changes */
ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
&sun8i_h3_cpu_nb);
}
--
2.13.0
The CPUX clock, which is the main clock of the ARM core on Allwinner H3,
can be adjusted by changing the frequency of the PLL_CPUX clock.
Allowing setting parent clock for the CPUX clock, thus the PLL_CPUX
clock can be adjusted when adjusting the CPUX clock.
Signed-off-by: Icenowy Zheng <[email protected]>
---
drivers/clk/sunxi-ng/ccu-sun8i-h3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-h3.c b/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
index 406d0aac9fd6..4cdbc88f2783 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-h3.c
@@ -135,7 +135,7 @@ static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK(pll_de_clk, "pll-de",
static const char * const cpux_parents[] = { "osc32k", "osc24M",
"pll-cpux" , "pll-cpux" };
static SUNXI_CCU_MUX(cpux_clk, "cpux", cpux_parents,
- 0x050, 16, 2, CLK_IS_CRITICAL);
+ 0x050, 16, 2, CLK_IS_CRITICAL | CLK_SET_RATE_PARENT);
static SUNXI_CCU_M(axi_clk, "axi", "cpux", 0x050, 0, 2, 0);
--
2.13.0
Some new Allwinner SoCs get supported in the kernel after the
compatibles are added to cpufreq-dt-platdev driver.
Add their compatible strings in the cpufreq-dt-platdev driver.
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Viresh Kumar <[email protected]>
Signed-off-by: Icenowy Zheng <[email protected]>
---
drivers/cpufreq/cpufreq-dt-platdev.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index 2eb40d46d357..c3851453e84a 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -24,7 +24,11 @@ static const struct of_device_id machines[] __initconst = {
{ .compatible = "allwinner,sun8i-a23", },
{ .compatible = "allwinner,sun8i-a33", },
{ .compatible = "allwinner,sun8i-a83t", },
+ { .compatible = "allwinner,sun8i-h2-plus", },
{ .compatible = "allwinner,sun8i-h3", },
+ { .compatible = "allwinner,sun8i-v3s", },
+ { .compatible = "allwinner,sun50i-a64", },
+ { .compatible = "allwinner,sun50i-h5", },
{ .compatible = "apm,xgene-shadowcat", },
@@ -43,6 +47,8 @@ static const struct of_device_id machines[] __initconst = {
{ .compatible = "marvell,pxa250", },
{ .compatible = "marvell,pxa270", },
+ { .compatible = "nextthing,gr8", },
+
{ .compatible = "samsung,exynos3250", },
{ .compatible = "samsung,exynos4210", },
{ .compatible = "samsung,exynos4212", },
--
2.13.0
The CPU on Allwinner H3 can do dynamic frequency scaling.
Add a DVFS table based on the one tweaked by Armbian developers, which
are proven to work stably on BSP kernels.
Frequencies higher than 1008MHz are temporarily dropped in the table, as
they may lead to over voltage on boards without proper regulator
settings or over temperature on boards with proper regulator settings.
They will be added back once regulator settings are ready and thermal
sensor driver is merged.
In order to satisfy all different regulators (SY8106A which is 50mV per
level, SY8113B which have two states: 1.1V and 1.3V, and some board with
non-tweakable regulators), all the OPPs are defined with a range which has
the target value as the minimum allowed value, and 1.3V (the highest
VDD-CPUX voltage suggested by the datasheet) as the maximum allowed value.
It's proven to work well with a board with SY8113B.
Signed-off-by: Icenowy Zheng <[email protected]>
---
arch/arm/boot/dts/sun8i-h3.dtsi | 38 +++++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index b36f9f423c39..a0cee17fe44b 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -43,32 +43,68 @@
#include "sunxi-h3-h5.dtsi"
/ {
+ cpu0_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp@480000000 {
+ opp-hz = /bits/ 64 <480000000>;
+ opp-microvolt = <980000 980000 1300000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+
+ opp@648000000 {
+ opp-hz = /bits/ 64 <816000000>;
+ opp-microvolt = <1020000 1020000 1300000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+
+ opp@912000000 {
+ opp-hz = /bits/ 64 <960000000>;
+ opp-microvolt = <1080000 1080000 1300000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+
+ opp@1008000000 {
+ opp-hz = /bits/ 64 <1008000000>;
+ opp-microvolt = <1140000 1140000 1300000>;
+ clock-latency-ns = <244144>; /* 8 32k periods */
+ };
+ };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
- cpu@0 {
+ cpu0: cpu@0 {
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <0>;
+ clocks = <&ccu CLK_CPUX>;
+ clock-names = "cpu";
+ operating-points-v2 = <&cpu0_opp_table>;
+ #cooling-cells = <0x2>;
};
cpu@1 {
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <1>;
+ operating-points-v2 = <&cpu0_opp_table>;
};
cpu@2 {
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <2>;
+ operating-points-v2 = <&cpu0_opp_table>;
};
cpu@3 {
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <3>;
+ operating-points-v2 = <&cpu0_opp_table>;
};
};
--
2.13.0
Orange Pi Zero board has a SY8113B regulator, which is controlled via
GPIO and capable of outputing 1.1V when the PL6 GPIO is set to output 0
or 1.3V when the PL6 GPIO is set to input or output 1, and the output is
the power supply of the ARM cores in H2+ SoC.
Add the device tree node of this regulator and set the cpu's cpu-supply
property to it.
Signed-off-by: Icenowy Zheng <[email protected]>
---
arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
index 6713d0f2b3f4..28b4433bd7f7 100644
--- a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
+++ b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
@@ -94,6 +94,27 @@
reset-gpios = <&r_pio 0 7 GPIO_ACTIVE_LOW>;
post-power-on-delay-ms = <200>;
};
+
+ reg_sy8113b: gpio-regulator {
+ compatible = "regulator-gpio";
+
+ regulator-name = "vdd-cpux";
+ regulator-type = "voltage";
+ regulator-boot-on;
+ regulator-always-on;
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1300000>;
+ regulator-ramp-delay = <50>; /* 4ms */
+
+ gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
+ gpios-states = <0x1>;
+ states = <1100000 0x0
+ 1300000 0x1>;
+ };
+};
+
+&cpu0 {
+ cpu-supply = <®_sy8113b>;
};
&ehci0 {
--
2.13.0
From: Ondrej Jirman <[email protected]>
Add SY8106A regulator to r_i2c bus and enable the r_i2c bus on
Orange Pi PC, then set the power supply of the ARM cores to this
regulator, in order to enable DVFS.
Signed-off-by: Ondrej Jirman <[email protected]>
[Icenowy: Enable DVFS in this patch, slight changes and change commit
message]
Signed-off-by: Icenowy Zheng <[email protected]>
---
arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
index 998b60f8d295..d855f8b6254e 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
@@ -98,6 +98,10 @@
status = "okay";
};
+&cpu0 {
+ cpu-supply = <®_sy8106a>;
+};
+
&ehci0 {
status = "okay";
};
@@ -160,6 +164,21 @@
};
};
+&r_i2c {
+ status = "okay";
+
+ reg_sy8106a: regulator@65 {
+ compatible = "silergy,sy8106a";
+ reg = <0x65>;
+ regulator-name = "vdd-cpux";
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <1400000>;
+ regulator-ramp-delay = <200>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+};
+
&r_pio {
leds_r_opc: led_pins@0 {
pins = "PL10";
--
2.13.0
On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
> From: Ondrej Jirman <[email protected]>
>
> SY8106A is an I2C attached single output regulator made by Silergy Corp,
> which is used on several Allwinner H3/H5 SBCs to control the power
> supply of the ARM cores.
>
> Add a driver for it.
>
> Signed-off-by: Ondrej Jirman <[email protected]>
> [Icenowy: Change commit message]
> Signed-off-by: Icenowy Zheng <[email protected]>
> ---
> drivers/regulator/Kconfig | 8 +-
> drivers/regulator/Makefile | 2 +-
> drivers/regulator/sy8106a-regulator.c | 163 ++++++++++++++++++++++++++++++++++
> 3 files changed, 171 insertions(+), 2 deletions(-)
> create mode 100644 drivers/regulator/sy8106a-regulator.c
>
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index 99b9362331b5..1efa73e18d07 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -764,6 +764,13 @@ config REGULATOR_STW481X_VMMC
> This driver supports the internal VMMC regulator in the STw481x
> PMIC chips.
>
> +config REGULATOR_SY8106A
> + tristate "Silergy SY8106A regulator"
> + depends on I2C && (OF || COMPILE_TEST)
> + select REGMAP_I2C
> + help
> + This driver supports SY8106A single output regulator.
> +
> config REGULATOR_TPS51632
> tristate "TI TPS51632 Power Regulator"
> depends on I2C
> @@ -938,4 +945,3 @@ config REGULATOR_WM8994
> WM8994 CODEC.
>
> endif
> -
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 95b1e86ae692..f5120252f86a 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -95,6 +95,7 @@ obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
> obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
> obj-$(CONFIG_REGULATOR_SKY81452) += sky81452-regulator.o
> obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
> +obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
> obj-$(CONFIG_REGULATOR_TI_ABB) += ti-abb-regulator.o
> obj-$(CONFIG_REGULATOR_TPS6105X) += tps6105x-regulator.o
> obj-$(CONFIG_REGULATOR_TPS62360) += tps62360-regulator.o
> @@ -120,5 +121,4 @@ obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
> obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
> obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
>
> -
> ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
> diff --git a/drivers/regulator/sy8106a-regulator.c b/drivers/regulator/sy8106a-regulator.c
> new file mode 100644
> index 000000000000..1df889f68b3d
> --- /dev/null
> +++ b/drivers/regulator/sy8106a-regulator.c
> @@ -0,0 +1,163 @@
> +/*
> + * sy8106a-regulator.c - Regulator device driver for SY8106A
> + *
> + * Copyright (C) 2016 Ondřej Jirman <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Library General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Library General Public License for more details.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/of_regulator.h>
> +
> +#define SY8106A_REG_VOUT1_SEL 0x01
> +#define SY8106A_REG_VOUT_COM 0x02
> +#define SY8106A_REG_VOUT1_SEL_MASK 0x7f
> +#define SY8106A_DISABLE_REG BIT(0)
> +#define SY8106A_GO_BIT BIT(7)
> +
> +struct sy8106a {
> + struct regulator_dev *rdev;
> + struct regmap *regmap;
> +};
> +
> +static const struct regmap_config sy8106a_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> +static int sy8106a_set_voltage_sel(struct regulator_dev *rdev, unsigned int sel)
> +{
> + /* We use our set_voltage_sel in order to avoid unnecessary I2C
> + * chatter, because the regulator_get_voltage_sel_regmap using
> + * apply_bit would perform 4 unnecessary transfers instead of one,
> + * increasing the chance of error.
> + */
> + return regmap_write(rdev->regmap, rdev->desc->vsel_reg,
> + sel | SY8106A_GO_BIT);
There should also be an explanation of what the "go bit" does.
In this case it enables control of the voltage over I2C.
> +}
> +
> +static const struct regulator_ops sy8106a_ops = {
> + .is_enabled = regulator_is_enabled_regmap,
> + .set_voltage_sel = sy8106a_set_voltage_sel,
> + .set_voltage_time_sel = regulator_set_voltage_time_sel,
> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
> + .list_voltage = regulator_list_voltage_linear,
No enable/disable ops?
> +};
> +
> +/* Default limits measured in millivolts and milliamps */
> +#define SY8106A_MIN_MV 680
> +#define SY8106A_MAX_MV 1950
> +#define SY8106A_STEP_MV 10
> +
> +static const struct regulator_desc sy8106a_reg = {
> + .name = "SY8106A",
> + .id = 0,
> + .ops = &sy8106a_ops,
> + .type = REGULATOR_VOLTAGE,
> + .n_voltages = ((SY8106A_MAX_MV - SY8106A_MIN_MV) / SY8106A_STEP_MV) + 1,
> + .min_uV = (SY8106A_MIN_MV * 1000),
> + .uV_step = (SY8106A_STEP_MV * 1000),
> + .vsel_reg = SY8106A_REG_VOUT1_SEL,
> + .vsel_mask = SY8106A_REG_VOUT1_SEL_MASK,
> + .enable_reg = SY8106A_REG_VOUT_COM,
> + .enable_mask = SY8106A_DISABLE_REG,
> + .disable_val = SY8106A_DISABLE_REG,
> + .enable_is_inverted = 1,
As the regulator core helpers currently are, there is no way to disable
this regulator. regulator_disable_regmap looks like:
if (rdev->desc->enable_is_inverted) {
val = rdev->desc->enable_val;
if (!val)
val = rdev->desc->enable_mask;
} else {
val = rdev->desc->disable_val;
}
return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
rdev->desc->enable_mask, val);
Note the fallback to enable_mask if enable_val is not set. The enable
helper has a similar structure. This regulator uses an enable value
of 0, and disable value of 1, it is impossible to get it right. As
it is now, it doesn't really make sense to me.
The code was introduced in ca5d1b3524b4 ("regulator: helpers: Modify
helpers enabling multi-bit control"). I wonder if it was an unintended
consequence of the change? (original author CC-ed)
> + .owner = THIS_MODULE,
> +};
> +
> +/*
> + * I2C driver interface functions
> + */
> +static int sy8106a_i2c_probe(struct i2c_client *i2c,
> + const struct i2c_device_id *id)
> +{
> + struct sy8106a *chip;
> + struct device *dev = &i2c->dev;
> + struct regulator_dev *rdev = NULL;
> + struct regulator_config config = { };
> + unsigned int selector;
> + int error;
> +
> + chip = devm_kzalloc(&i2c->dev, sizeof(struct sy8106a), GFP_KERNEL);
> + if (!chip)
> + return -ENOMEM;
> +
> + chip->regmap = devm_regmap_init_i2c(i2c, &sy8106a_regmap_config);
> + if (IS_ERR(chip->regmap)) {
> + error = PTR_ERR(chip->regmap);
> + dev_err(&i2c->dev, "Failed to allocate register map: %d\n",
> + error);
> + return error;
> + }
> +
> + config.dev = &i2c->dev;
> + config.regmap = chip->regmap;
> + config.driver_data = chip;
> +
> + config.of_node = dev->of_node;
> + config.init_data = of_get_regulator_init_data(dev, dev->of_node,
> + &sy8106a_reg);
> +
> + if (!config.init_data)
> + return -ENOMEM;
> +
> + /* Probe regulator */
> + error = regmap_read(chip->regmap, SY8106A_REG_VOUT1_SEL, &selector);
> + if (error) {
> + dev_err(&i2c->dev, "Failed to read voltage at probe time: %d\n", error);
> + return error;
> + }
> +
> + rdev = devm_regulator_register(&i2c->dev, &sy8106a_reg, &config);
> + if (IS_ERR(rdev)) {
> + error = PTR_ERR(rdev);
> + dev_err(&i2c->dev, "Failed to register SY8106A regulator: %d\n", error);
> + return error;
> + }
> +
> + chip->rdev = rdev;
> +
> + i2c_set_clientdata(i2c, chip);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sy8106a_i2c_of_match[] = {
> + { .compatible = "silergy,sy8106a" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, sy8106a_i2c_of_match);
> +
> +static const struct i2c_device_id sy8106a_i2c_id[] = {
> + { "sy8106a", 0 },
> + { },
> +};
> +MODULE_DEVICE_TABLE(i2c, sy8106a_i2c_id);
> +
> +static struct i2c_driver sy8106a_regulator_driver = {
> + .driver = {
> + .name = "sy8106a",
> + .of_match_table = of_match_ptr(sy8106a_i2c_of_match),
> + },
> + .probe = sy8106a_i2c_probe,
> + .id_table = sy8106a_i2c_id,
> +};
> +
> +module_i2c_driver(sy8106a_regulator_driver);
> +
> +MODULE_AUTHOR("Ondřej Jirman <[email protected]>");
> +MODULE_DESCRIPTION("Regulator device driver for Silergy SY8106A");
> +MODULE_LICENSE("GPL v2");
This should be "GPL", to match what the license at the very top says.
Regards
ChenYu
> --
> 2.13.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
> From: Ondrej Jirman <[email protected]>
>
> SY8106A is an I2C-controlled adjustable voltage regulator made by
> Silergy Corp.
>
> Add its device tree binding.
>
> Signed-off-by: Ondrej Jirman <[email protected]>
> [Icenowy: Change commit message]
> Signed-off-by: Icenowy Zheng <[email protected]>
> ---
> .../bindings/regulator/sy8106a-regulator.txt | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt
>
> diff --git a/Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt b/Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt
> new file mode 100644
> index 000000000000..1e623a34b1cb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt
> @@ -0,0 +1,21 @@
> +SY8106A Voltage regulator
> +
> +Required properties:
> +- compatible: Must be "silergy,sy8106a"
> +- reg: I2C slave address - must be <0x65>
> +
> +Any property defined as part of the core regulator binding, defined in
> +regulator.txt, can also be used.
Nit: use ./regulator.txt to show that this is a path, and not just a
file name. This should help other people find the file quickly.
Otherwise,
Reviewed-by: Chen-Yu Tsai <[email protected]>
> +
> +Example:
> +
> + sy8106a {
> + compatible = "silergy,sy8106a";
> + reg = <0x65>;
> + regulator-name = "sy8106a-vdd";
> + regulator-min-microvolt = <1000000>;
> + regulator-max-microvolt = <1400000>;
> + regulator-ramp-delay = <200>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> --
> 2.13.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
> From: Ondrej Jirman <[email protected]>
>
> H3/H5 SoCs contain an I2C controller optionally available
> on the PL0 and PL1 pins. This patch adds pinmux configuration
> for this controller.
>
> Signed-off-by: Ondrej Jirman <[email protected]>
> [Icenowy: change commit message and node name]
> Signed-off-by: Icenowy Zheng <[email protected]>
> ---
> arch/arm/boot/dts/sunxi-h3-h5.dtsi | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> index 6f2162608006..b240099bc865 100644
> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> @@ -639,6 +639,11 @@
> pins = "PL11";
> function = "s_cir_rx";
> };
> +
> + r_i2c_pins: r-i2c {
> + pins = "PL0", "PL1";
> + function = "s_twi";
Can we fix the pinctrl driver to use standard names first? :(
ChenYu
> + };
> };
> };
> };
> --
> 2.13.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
在 2017-07-24 11:07，Chen-Yu Tsai 写道：
> On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
>> From: Ondrej Jirman <[email protected]>
>>
>> H3/H5 SoCs contain an I2C controller optionally available
>> on the PL0 and PL1 pins. This patch adds pinmux configuration
>> for this controller.
>>
>> Signed-off-by: Ondrej Jirman <[email protected]>
>> [Icenowy: change commit message and node name]
>> Signed-off-by: Icenowy Zheng <[email protected]>
>> ---
>> arch/arm/boot/dts/sunxi-h3-h5.dtsi | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> index 6f2162608006..b240099bc865 100644
>> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> @@ -639,6 +639,11 @@
>> pins = "PL11";
>> function = "s_cir_rx";
>> };
>> +
>> + r_i2c_pins: r-i2c {
>> + pins = "PL0", "PL1";
>> + function = "s_twi";
>
> Can we fix the pinctrl driver to use standard names first? :(
Oh good problem.
Let me fix it.
>
> ChenYu
>
>> + };
>> };
>> };
>> };
>> --
>> 2.13.0
>>
>> --
>> You received this message because you are subscribed to the Google
>> Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send
>> an email to [email protected].
>> For more options, visit https://groups.google.com/d/optout.
On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
> From: Ondrej Jirman <[email protected]>
>
> Allwinner H3/H5 SoCs have an I2C controller at PL GPIO bank.
>
> Add support for it in the device tree.
>
> Signed-off-by: Ondrej Jirman <[email protected]>
> [Icenowy: Change to use r_ccu and change pinmux node name]
> Signed-off-by: Icenowy Zheng <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
> The CPUX clock, which is the main clock of the ARM core on Allwinner H3,
> can be adjusted by changing the frequency of the PLL_CPUX clock.
>
> Allowing setting parent clock for the CPUX clock, thus the PLL_CPUX
> clock can be adjusted when adjusting the CPUX clock.
>
> Signed-off-by: Icenowy Zheng <[email protected]>
Fixes: 0577e4853bfb ("clk: sunxi-ng: Add H3 clocks")
Reviewed-by: Chen-Yu Tsai <[email protected]>
在 2017-07-24 11:03，Chen-Yu Tsai 写道：
> On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
>> From: Ondrej Jirman <[email protected]>
>>
>> SY8106A is an I2C attached single output regulator made by Silergy
>> Corp,
>> which is used on several Allwinner H3/H5 SBCs to control the power
>> supply of the ARM cores.
>>
>> Add a driver for it.
>>
>> Signed-off-by: Ondrej Jirman <[email protected]>
>> [Icenowy: Change commit message]
>> Signed-off-by: Icenowy Zheng <[email protected]>
>> ---
>> drivers/regulator/Kconfig | 8 +-
>> drivers/regulator/Makefile | 2 +-
>> drivers/regulator/sy8106a-regulator.c | 163
>> ++++++++++++++++++++++++++++++++++
>> 3 files changed, 171 insertions(+), 2 deletions(-)
>> create mode 100644 drivers/regulator/sy8106a-regulator.c
>>
>> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
>> index 99b9362331b5..1efa73e18d07 100644
>> --- a/drivers/regulator/Kconfig
>> +++ b/drivers/regulator/Kconfig
>> @@ -764,6 +764,13 @@ config REGULATOR_STW481X_VMMC
>> This driver supports the internal VMMC regulator in the
>> STw481x
>> PMIC chips.
>>
>> +config REGULATOR_SY8106A
>> + tristate "Silergy SY8106A regulator"
>> + depends on I2C && (OF || COMPILE_TEST)
>> + select REGMAP_I2C
>> + help
>> + This driver supports SY8106A single output regulator.
>> +
>> config REGULATOR_TPS51632
>> tristate "TI TPS51632 Power Regulator"
>> depends on I2C
>> @@ -938,4 +945,3 @@ config REGULATOR_WM8994
>> WM8994 CODEC.
>>
>> endif
>> -
>> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
>> index 95b1e86ae692..f5120252f86a 100644
>> --- a/drivers/regulator/Makefile
>> +++ b/drivers/regulator/Makefile
>> @@ -95,6 +95,7 @@ obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
>> obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
>> obj-$(CONFIG_REGULATOR_SKY81452) += sky81452-regulator.o
>> obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
>> +obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
>> obj-$(CONFIG_REGULATOR_TI_ABB) += ti-abb-regulator.o
>> obj-$(CONFIG_REGULATOR_TPS6105X) += tps6105x-regulator.o
>> obj-$(CONFIG_REGULATOR_TPS62360) += tps62360-regulator.o
>> @@ -120,5 +121,4 @@ obj-$(CONFIG_REGULATOR_WM8350) +=
>> wm8350-regulator.o
>> obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
>> obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
>>
>> -
>> ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
>> diff --git a/drivers/regulator/sy8106a-regulator.c
>> b/drivers/regulator/sy8106a-regulator.c
>> new file mode 100644
>> index 000000000000..1df889f68b3d
>> --- /dev/null
>> +++ b/drivers/regulator/sy8106a-regulator.c
>> @@ -0,0 +1,163 @@
>> +/*
>> + * sy8106a-regulator.c - Regulator device driver for SY8106A
>> + *
>> + * Copyright (C) 2016 Ondřej Jirman <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Library General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * Library General Public License for more details.
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +#include <linux/regulator/driver.h>
>> +#include <linux/regulator/of_regulator.h>
>> +
>> +#define SY8106A_REG_VOUT1_SEL 0x01
>> +#define SY8106A_REG_VOUT_COM 0x02
>> +#define SY8106A_REG_VOUT1_SEL_MASK 0x7f
>> +#define SY8106A_DISABLE_REG BIT(0)
>> +#define SY8106A_GO_BIT BIT(7)
>> +
>> +struct sy8106a {
>> + struct regulator_dev *rdev;
>> + struct regmap *regmap;
>> +};
>> +
>> +static const struct regmap_config sy8106a_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> +};
>> +
>> +static int sy8106a_set_voltage_sel(struct regulator_dev *rdev,
>> unsigned int sel)
>> +{
>> + /* We use our set_voltage_sel in order to avoid unnecessary
>> I2C
>> + * chatter, because the regulator_get_voltage_sel_regmap using
>> + * apply_bit would perform 4 unnecessary transfers instead of
>> one,
>> + * increasing the chance of error.
>> + */
>> + return regmap_write(rdev->regmap, rdev->desc->vsel_reg,
>> + sel | SY8106A_GO_BIT);
>
> There should also be an explanation of what the "go bit" does.
> In this case it enables control of the voltage over I2C.
>
>> +}
>> +
>> +static const struct regulator_ops sy8106a_ops = {
>> + .is_enabled = regulator_is_enabled_regmap,
>> + .set_voltage_sel = sy8106a_set_voltage_sel,
>> + .set_voltage_time_sel = regulator_set_voltage_time_sel,
>> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
>> + .list_voltage = regulator_list_voltage_linear,
>
> No enable/disable ops?
>
>> +};
>> +
>> +/* Default limits measured in millivolts and milliamps */
>> +#define SY8106A_MIN_MV 680
>> +#define SY8106A_MAX_MV 1950
>> +#define SY8106A_STEP_MV 10
>> +
>> +static const struct regulator_desc sy8106a_reg = {
>> + .name = "SY8106A",
>> + .id = 0,
>> + .ops = &sy8106a_ops,
>> + .type = REGULATOR_VOLTAGE,
>> + .n_voltages = ((SY8106A_MAX_MV - SY8106A_MIN_MV) /
>> SY8106A_STEP_MV) + 1,
>> + .min_uV = (SY8106A_MIN_MV * 1000),
>> + .uV_step = (SY8106A_STEP_MV * 1000),
>> + .vsel_reg = SY8106A_REG_VOUT1_SEL,
>> + .vsel_mask = SY8106A_REG_VOUT1_SEL_MASK,
>> + .enable_reg = SY8106A_REG_VOUT_COM,
>> + .enable_mask = SY8106A_DISABLE_REG,
>> + .disable_val = SY8106A_DISABLE_REG,
>> + .enable_is_inverted = 1,
>
> As the regulator core helpers currently are, there is no way to disable
> this regulator. regulator_disable_regmap looks like:
By reading the current code, I think removing the disable_val variable
will
just make it work -- when enabling, the enable_reg will be set to
disable_val
(as enable_is_inverted is 1), and disable_val is defaultly 0; when
disabling,
the enable_reg will be set to enable_mask (fallbacked from enable_val,
which
is also the default value 0), and the regulator successfully get
disabled.
>
> if (rdev->desc->enable_is_inverted) {
> val = rdev->desc->enable_val;
> if (!val)
> val = rdev->desc->enable_mask;
> } else {
> val = rdev->desc->disable_val;
> }
>
> return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
> rdev->desc->enable_mask, val);
>
> Note the fallback to enable_mask if enable_val is not set. The enable
> helper has a similar structure. This regulator uses an enable value
> of 0, and disable value of 1, it is impossible to get it right. As
> it is now, it doesn't really make sense to me.
>
> The code was introduced in ca5d1b3524b4 ("regulator: helpers: Modify
> helpers enabling multi-bit control"). I wonder if it was an unintended
> consequence of the change? (original author CC-ed)
>
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +/*
>> + * I2C driver interface functions
>> + */
>> +static int sy8106a_i2c_probe(struct i2c_client *i2c,
>> + const struct i2c_device_id *id)
>> +{
>> + struct sy8106a *chip;
>> + struct device *dev = &i2c->dev;
>> + struct regulator_dev *rdev = NULL;
>> + struct regulator_config config = { };
>> + unsigned int selector;
>> + int error;
>> +
>> + chip = devm_kzalloc(&i2c->dev, sizeof(struct sy8106a),
>> GFP_KERNEL);
>> + if (!chip)
>> + return -ENOMEM;
>> +
>> + chip->regmap = devm_regmap_init_i2c(i2c,
>> &sy8106a_regmap_config);
>> + if (IS_ERR(chip->regmap)) {
>> + error = PTR_ERR(chip->regmap);
>> + dev_err(&i2c->dev, "Failed to allocate register map:
>> %d\n",
>> + error);
>> + return error;
>> + }
>> +
>> + config.dev = &i2c->dev;
>> + config.regmap = chip->regmap;
>> + config.driver_data = chip;
>> +
>> + config.of_node = dev->of_node;
>> + config.init_data = of_get_regulator_init_data(dev,
>> dev->of_node,
>> + &sy8106a_reg);
>> +
>> + if (!config.init_data)
>> + return -ENOMEM;
>> +
>> + /* Probe regulator */
>> + error = regmap_read(chip->regmap, SY8106A_REG_VOUT1_SEL,
>> &selector);
>> + if (error) {
>> + dev_err(&i2c->dev, "Failed to read voltage at probe
>> time: %d\n", error);
>> + return error;
>> + }
>> +
>> + rdev = devm_regulator_register(&i2c->dev, &sy8106a_reg,
>> &config);
>> + if (IS_ERR(rdev)) {
>> + error = PTR_ERR(rdev);
>> + dev_err(&i2c->dev, "Failed to register SY8106A
>> regulator: %d\n", error);
>> + return error;
>> + }
>> +
>> + chip->rdev = rdev;
>> +
>> + i2c_set_clientdata(i2c, chip);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id sy8106a_i2c_of_match[] = {
>> + { .compatible = "silergy,sy8106a" },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, sy8106a_i2c_of_match);
>> +
>> +static const struct i2c_device_id sy8106a_i2c_id[] = {
>> + { "sy8106a", 0 },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(i2c, sy8106a_i2c_id);
>> +
>> +static struct i2c_driver sy8106a_regulator_driver = {
>> + .driver = {
>> + .name = "sy8106a",
>> + .of_match_table = of_match_ptr(sy8106a_i2c_of_match),
>> + },
>> + .probe = sy8106a_i2c_probe,
>> + .id_table = sy8106a_i2c_id,
>> +};
>> +
>> +module_i2c_driver(sy8106a_regulator_driver);
>> +
>> +MODULE_AUTHOR("Ondřej Jirman <[email protected]>");
>> +MODULE_DESCRIPTION("Regulator device driver for Silergy SY8106A");
>> +MODULE_LICENSE("GPL v2");
>
> This should be "GPL", to match what the license at the very top says.
>
> Regards
> ChenYu
>
>> --
>> 2.13.0
>>
>> --
>> You received this message because you are subscribed to the Google
>> Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send
>> an email to [email protected].
>> For more options, visit https://groups.google.com/d/optout.
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Jul 24, 2017 at 11:18 AM, <[email protected]> wrote:
> 在 2017-07-24 11:03，Chen-Yu Tsai 写道：
>>
>> On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
>>>
>>> From: Ondrej Jirman <[email protected]>
>>>
>>> SY8106A is an I2C attached single output regulator made by Silergy Corp,
>>> which is used on several Allwinner H3/H5 SBCs to control the power
>>> supply of the ARM cores.
>>>
>>> Add a driver for it.
>>>
>>> Signed-off-by: Ondrej Jirman <[email protected]>
>>> [Icenowy: Change commit message]
>>> Signed-off-by: Icenowy Zheng <[email protected]>
>>> ---
>>> drivers/regulator/Kconfig | 8 +-
>>> drivers/regulator/Makefile | 2 +-
>>> drivers/regulator/sy8106a-regulator.c | 163
>>> ++++++++++++++++++++++++++++++++++
>>> 3 files changed, 171 insertions(+), 2 deletions(-)
>>> create mode 100644 drivers/regulator/sy8106a-regulator.c
>>>
>>> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
>>> index 99b9362331b5..1efa73e18d07 100644
>>> --- a/drivers/regulator/Kconfig
>>> +++ b/drivers/regulator/Kconfig
>>> @@ -764,6 +764,13 @@ config REGULATOR_STW481X_VMMC
>>> This driver supports the internal VMMC regulator in the STw481x
>>> PMIC chips.
>>>
>>> +config REGULATOR_SY8106A
>>> + tristate "Silergy SY8106A regulator"
>>> + depends on I2C && (OF || COMPILE_TEST)
>>> + select REGMAP_I2C
>>> + help
>>> + This driver supports SY8106A single output regulator.
>>> +
>>> config REGULATOR_TPS51632
>>> tristate "TI TPS51632 Power Regulator"
>>> depends on I2C
>>> @@ -938,4 +945,3 @@ config REGULATOR_WM8994
>>> WM8994 CODEC.
>>>
>>> endif
>>> -
>>> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
>>> index 95b1e86ae692..f5120252f86a 100644
>>> --- a/drivers/regulator/Makefile
>>> +++ b/drivers/regulator/Makefile
>>> @@ -95,6 +95,7 @@ obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
>>> obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
>>> obj-$(CONFIG_REGULATOR_SKY81452) += sky81452-regulator.o
>>> obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
>>> +obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
>>> obj-$(CONFIG_REGULATOR_TI_ABB) += ti-abb-regulator.o
>>> obj-$(CONFIG_REGULATOR_TPS6105X) += tps6105x-regulator.o
>>> obj-$(CONFIG_REGULATOR_TPS62360) += tps62360-regulator.o
>>> @@ -120,5 +121,4 @@ obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
>>> obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
>>> obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
>>>
>>> -
>>> ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
>>> diff --git a/drivers/regulator/sy8106a-regulator.c
>>> b/drivers/regulator/sy8106a-regulator.c
>>> new file mode 100644
>>> index 000000000000..1df889f68b3d
>>> --- /dev/null
>>> +++ b/drivers/regulator/sy8106a-regulator.c
>>> @@ -0,0 +1,163 @@
>>> +/*
>>> + * sy8106a-regulator.c - Regulator device driver for SY8106A
>>> + *
>>> + * Copyright (C) 2016 Ondřej Jirman <[email protected]>
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Library General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2 of the License, or (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>> + * Library General Public License for more details.
>>> + */
>>> +
>>> +#include <linux/err.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/module.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/regulator/driver.h>
>>> +#include <linux/regulator/of_regulator.h>
>>> +
>>> +#define SY8106A_REG_VOUT1_SEL 0x01
>>> +#define SY8106A_REG_VOUT_COM 0x02
>>> +#define SY8106A_REG_VOUT1_SEL_MASK 0x7f
>>> +#define SY8106A_DISABLE_REG BIT(0)
>>> +#define SY8106A_GO_BIT BIT(7)
>>> +
>>> +struct sy8106a {
>>> + struct regulator_dev *rdev;
>>> + struct regmap *regmap;
>>> +};
>>> +
>>> +static const struct regmap_config sy8106a_regmap_config = {
>>> + .reg_bits = 8,
>>> + .val_bits = 8,
>>> +};
>>> +
>>> +static int sy8106a_set_voltage_sel(struct regulator_dev *rdev, unsigned
>>> int sel)
>>> +{
>>> + /* We use our set_voltage_sel in order to avoid unnecessary I2C
>>> + * chatter, because the regulator_get_voltage_sel_regmap using
>>> + * apply_bit would perform 4 unnecessary transfers instead of
>>> one,
>>> + * increasing the chance of error.
>>> + */
>>> + return regmap_write(rdev->regmap, rdev->desc->vsel_reg,
>>> + sel | SY8106A_GO_BIT);
>>
>>
>> There should also be an explanation of what the "go bit" does.
>> In this case it enables control of the voltage over I2C.
>>
>>> +}
>>> +
>>> +static const struct regulator_ops sy8106a_ops = {
>>> + .is_enabled = regulator_is_enabled_regmap,
>>> + .set_voltage_sel = sy8106a_set_voltage_sel,
>>> + .set_voltage_time_sel = regulator_set_voltage_time_sel,
>>> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
>>> + .list_voltage = regulator_list_voltage_linear,
>>
>>
>> No enable/disable ops?
>>
>>> +};
>>> +
>>> +/* Default limits measured in millivolts and milliamps */
>>> +#define SY8106A_MIN_MV 680
>>> +#define SY8106A_MAX_MV 1950
>>> +#define SY8106A_STEP_MV 10
>>> +
>>> +static const struct regulator_desc sy8106a_reg = {
>>> + .name = "SY8106A",
>>> + .id = 0,
>>> + .ops = &sy8106a_ops,
>>> + .type = REGULATOR_VOLTAGE,
>>> + .n_voltages = ((SY8106A_MAX_MV - SY8106A_MIN_MV) /
>>> SY8106A_STEP_MV) + 1,
>>> + .min_uV = (SY8106A_MIN_MV * 1000),
>>> + .uV_step = (SY8106A_STEP_MV * 1000),
>>> + .vsel_reg = SY8106A_REG_VOUT1_SEL,
>>> + .vsel_mask = SY8106A_REG_VOUT1_SEL_MASK,
>>> + .enable_reg = SY8106A_REG_VOUT_COM,
>>> + .enable_mask = SY8106A_DISABLE_REG,
>>> + .disable_val = SY8106A_DISABLE_REG,
>>> + .enable_is_inverted = 1,
>>
>>
>> As the regulator core helpers currently are, there is no way to disable
>> this regulator. regulator_disable_regmap looks like:
>
>
> By reading the current code, I think removing the disable_val variable will
> just make it work -- when enabling, the enable_reg will be set to disable_val
> (as enable_is_inverted is 1), and disable_val is defaultly 0; when disabling,
> the enable_reg will be set to enable_mask (fallbacked from enable_val, which
> is also the default value 0), and the regulator successfully get disabled.
(Gmail is wrapping the lines :( )
enable_mask is set to SY8106A_DISABLE_REG, which equals disable_val here,
which is not 0. This is correct, as it is the bit mask passed to
regmap_update_bits. So no, it does not work.
It seems quite a lot of drivers depend on this fallback behavior though.
ChenYu
>
>>
>> if (rdev->desc->enable_is_inverted) {
>> val = rdev->desc->enable_val;
>> if (!val)
>> val = rdev->desc->enable_mask;
>> } else {
>> val = rdev->desc->disable_val;
>> }
>>
>> return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
>> rdev->desc->enable_mask, val);
>>
>> Note the fallback to enable_mask if enable_val is not set. The enable
>> helper has a similar structure. This regulator uses an enable value
>> of 0, and disable value of 1, it is impossible to get it right. As
>> it is now, it doesn't really make sense to me.
>>
>> The code was introduced in ca5d1b3524b4 ("regulator: helpers: Modify
>> helpers enabling multi-bit control"). I wonder if it was an unintended
>> consequence of the change? (original author CC-ed)
>>
>>> + .owner = THIS_MODULE,
>>> +};
>>> +
>>> +/*
>>> + * I2C driver interface functions
>>> + */
>>> +static int sy8106a_i2c_probe(struct i2c_client *i2c,
>>> + const struct i2c_device_id *id)
>>> +{
>>> + struct sy8106a *chip;
>>> + struct device *dev = &i2c->dev;
>>> + struct regulator_dev *rdev = NULL;
>>> + struct regulator_config config = { };
>>> + unsigned int selector;
>>> + int error;
>>> +
>>> + chip = devm_kzalloc(&i2c->dev, sizeof(struct sy8106a),
>>> GFP_KERNEL);
>>> + if (!chip)
>>> + return -ENOMEM;
>>> +
>>> + chip->regmap = devm_regmap_init_i2c(i2c, &sy8106a_regmap_config);
>>> + if (IS_ERR(chip->regmap)) {
>>> + error = PTR_ERR(chip->regmap);
>>> + dev_err(&i2c->dev, "Failed to allocate register map:
>>> %d\n",
>>> + error);
>>> + return error;
>>> + }
>>> +
>>> + config.dev = &i2c->dev;
>>> + config.regmap = chip->regmap;
>>> + config.driver_data = chip;
>>> +
>>> + config.of_node = dev->of_node;
>>> + config.init_data = of_get_regulator_init_data(dev, dev->of_node,
>>> + &sy8106a_reg);
>>> +
>>> + if (!config.init_data)
>>> + return -ENOMEM;
>>> +
>>> + /* Probe regulator */
>>> + error = regmap_read(chip->regmap, SY8106A_REG_VOUT1_SEL,
>>> &selector);
>>> + if (error) {
>>> + dev_err(&i2c->dev, "Failed to read voltage at probe time:
>>> %d\n", error);
>>> + return error;
>>> + }
>>> +
>>> + rdev = devm_regulator_register(&i2c->dev, &sy8106a_reg, &config);
>>> + if (IS_ERR(rdev)) {
>>> + error = PTR_ERR(rdev);
>>> + dev_err(&i2c->dev, "Failed to register SY8106A regulator:
>>> %d\n", error);
>>> + return error;
>>> + }
>>> +
>>> + chip->rdev = rdev;
>>> +
>>> + i2c_set_clientdata(i2c, chip);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct of_device_id sy8106a_i2c_of_match[] = {
>>> + { .compatible = "silergy,sy8106a" },
>>> + { },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, sy8106a_i2c_of_match);
>>> +
>>> +static const struct i2c_device_id sy8106a_i2c_id[] = {
>>> + { "sy8106a", 0 },
>>> + { },
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, sy8106a_i2c_id);
>>> +
>>> +static struct i2c_driver sy8106a_regulator_driver = {
>>> + .driver = {
>>> + .name = "sy8106a",
>>> + .of_match_table = of_match_ptr(sy8106a_i2c_of_match),
>>> + },
>>> + .probe = sy8106a_i2c_probe,
>>> + .id_table = sy8106a_i2c_id,
>>> +};
>>> +
>>> +module_i2c_driver(sy8106a_regulator_driver);
>>> +
>>> +MODULE_AUTHOR("Ondřej Jirman <[email protected]>");
>>> +MODULE_DESCRIPTION("Regulator device driver for Silergy SY8106A");
>>> +MODULE_LICENSE("GPL v2");
>>
>>
>> This should be "GPL", to match what the license at the very top says.
>>
>> Regards
>> ChenYu
>>
>>> --
>>> 2.13.0
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups
>>> "linux-sunxi" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an
>>> email to [email protected].
>>> For more options, visit https://groups.google.com/d/optout.
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
在 2017-07-24 11:33，Chen-Yu Tsai 写道：
> On Mon, Jul 24, 2017 at 11:18 AM, <[email protected]> wrote:
>> 在 2017-07-24 11:03，Chen-Yu Tsai 写道：
>>>
>>> On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]>
>>> wrote:
>>>>
>>>> From: Ondrej Jirman <[email protected]>
>>>>
>>>> SY8106A is an I2C attached single output regulator made by Silergy
>>>> Corp,
>>>> which is used on several Allwinner H3/H5 SBCs to control the power
>>>> supply of the ARM cores.
>>>>
>>>> Add a driver for it.
>>>>
>>>> Signed-off-by: Ondrej Jirman <[email protected]>
>>>> [Icenowy: Change commit message]
>>>> Signed-off-by: Icenowy Zheng <[email protected]>
>>>> ---
>>>> drivers/regulator/Kconfig | 8 +-
>>>> drivers/regulator/Makefile | 2 +-
>>>> drivers/regulator/sy8106a-regulator.c | 163
>>>> ++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 171 insertions(+), 2 deletions(-)
>>>> create mode 100644 drivers/regulator/sy8106a-regulator.c
>>>>
>>>> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
>>>> index 99b9362331b5..1efa73e18d07 100644
>>>> --- a/drivers/regulator/Kconfig
>>>> +++ b/drivers/regulator/Kconfig
>>>> @@ -764,6 +764,13 @@ config REGULATOR_STW481X_VMMC
>>>> This driver supports the internal VMMC regulator in the
>>>> STw481x
>>>> PMIC chips.
>>>>
>>>> +config REGULATOR_SY8106A
>>>> + tristate "Silergy SY8106A regulator"
>>>> + depends on I2C && (OF || COMPILE_TEST)
>>>> + select REGMAP_I2C
>>>> + help
>>>> + This driver supports SY8106A single output regulator.
>>>> +
>>>> config REGULATOR_TPS51632
>>>> tristate "TI TPS51632 Power Regulator"
>>>> depends on I2C
>>>> @@ -938,4 +945,3 @@ config REGULATOR_WM8994
>>>> WM8994 CODEC.
>>>>
>>>> endif
>>>> -
>>>> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
>>>> index 95b1e86ae692..f5120252f86a 100644
>>>> --- a/drivers/regulator/Makefile
>>>> +++ b/drivers/regulator/Makefile
>>>> @@ -95,6 +95,7 @@ obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
>>>> obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
>>>> obj-$(CONFIG_REGULATOR_SKY81452) += sky81452-regulator.o
>>>> obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
>>>> +obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
>>>> obj-$(CONFIG_REGULATOR_TI_ABB) += ti-abb-regulator.o
>>>> obj-$(CONFIG_REGULATOR_TPS6105X) += tps6105x-regulator.o
>>>> obj-$(CONFIG_REGULATOR_TPS62360) += tps62360-regulator.o
>>>> @@ -120,5 +121,4 @@ obj-$(CONFIG_REGULATOR_WM8350) +=
>>>> wm8350-regulator.o
>>>> obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
>>>> obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
>>>>
>>>> -
>>>> ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
>>>> diff --git a/drivers/regulator/sy8106a-regulator.c
>>>> b/drivers/regulator/sy8106a-regulator.c
>>>> new file mode 100644
>>>> index 000000000000..1df889f68b3d
>>>> --- /dev/null
>>>> +++ b/drivers/regulator/sy8106a-regulator.c
>>>> @@ -0,0 +1,163 @@
>>>> +/*
>>>> + * sy8106a-regulator.c - Regulator device driver for SY8106A
>>>> + *
>>>> + * Copyright (C) 2016 Ondřej Jirman <[email protected]>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU Library General Public
>>>> + * License as published by the Free Software Foundation; either
>>>> + * version 2 of the License, or (at your option) any later version.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> GNU
>>>> + * Library General Public License for more details.
>>>> + */
>>>> +
>>>> +#include <linux/err.h>
>>>> +#include <linux/i2c.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/regmap.h>
>>>> +#include <linux/regulator/driver.h>
>>>> +#include <linux/regulator/of_regulator.h>
>>>> +
>>>> +#define SY8106A_REG_VOUT1_SEL 0x01
>>>> +#define SY8106A_REG_VOUT_COM 0x02
>>>> +#define SY8106A_REG_VOUT1_SEL_MASK 0x7f
>>>> +#define SY8106A_DISABLE_REG BIT(0)
>>>> +#define SY8106A_GO_BIT BIT(7)
>>>> +
>>>> +struct sy8106a {
>>>> + struct regulator_dev *rdev;
>>>> + struct regmap *regmap;
>>>> +};
>>>> +
>>>> +static const struct regmap_config sy8106a_regmap_config = {
>>>> + .reg_bits = 8,
>>>> + .val_bits = 8,
>>>> +};
>>>> +
>>>> +static int sy8106a_set_voltage_sel(struct regulator_dev *rdev,
>>>> unsigned
>>>> int sel)
>>>> +{
>>>> + /* We use our set_voltage_sel in order to avoid unnecessary
>>>> I2C
>>>> + * chatter, because the regulator_get_voltage_sel_regmap
>>>> using
>>>> + * apply_bit would perform 4 unnecessary transfers instead
>>>> of
>>>> one,
>>>> + * increasing the chance of error.
>>>> + */
>>>> + return regmap_write(rdev->regmap, rdev->desc->vsel_reg,
>>>> + sel | SY8106A_GO_BIT);
>>>
>>>
>>> There should also be an explanation of what the "go bit" does.
>>> In this case it enables control of the voltage over I2C.
>>>
>>>> +}
>>>> +
>>>> +static const struct regulator_ops sy8106a_ops = {
>>>> + .is_enabled = regulator_is_enabled_regmap,
>>>> + .set_voltage_sel = sy8106a_set_voltage_sel,
>>>> + .set_voltage_time_sel = regulator_set_voltage_time_sel,
>>>> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
>>>> + .list_voltage = regulator_list_voltage_linear,
>>>
>>>
>>> No enable/disable ops?
>>>
>>>> +};
>>>> +
>>>> +/* Default limits measured in millivolts and milliamps */
>>>> +#define SY8106A_MIN_MV 680
>>>> +#define SY8106A_MAX_MV 1950
>>>> +#define SY8106A_STEP_MV 10
>>>> +
>>>> +static const struct regulator_desc sy8106a_reg = {
>>>> + .name = "SY8106A",
>>>> + .id = 0,
>>>> + .ops = &sy8106a_ops,
>>>> + .type = REGULATOR_VOLTAGE,
>>>> + .n_voltages = ((SY8106A_MAX_MV - SY8106A_MIN_MV) /
>>>> SY8106A_STEP_MV) + 1,
>>>> + .min_uV = (SY8106A_MIN_MV * 1000),
>>>> + .uV_step = (SY8106A_STEP_MV * 1000),
>>>> + .vsel_reg = SY8106A_REG_VOUT1_SEL,
>>>> + .vsel_mask = SY8106A_REG_VOUT1_SEL_MASK,
>>>> + .enable_reg = SY8106A_REG_VOUT_COM,
>>>> + .enable_mask = SY8106A_DISABLE_REG,
>>>> + .disable_val = SY8106A_DISABLE_REG,
>>>> + .enable_is_inverted = 1,
>>>
>>>
>>> As the regulator core helpers currently are, there is no way to
>>> disable
>>> this regulator. regulator_disable_regmap looks like:
>>
>>
>> By reading the current code, I think removing the disable_val variable
>> will
>> just make it work -- when enabling, the enable_reg will be set to
>> disable_val
>> (as enable_is_inverted is 1), and disable_val is defaultly 0; when
>> disabling,
>> the enable_reg will be set to enable_mask (fallbacked from enable_val,
>> which
>> is also the default value 0), and the regulator successfully get
>> disabled.
>
> (Gmail is wrapping the lines :( )
>
> enable_mask is set to SY8106A_DISABLE_REG, which equals disable_val
> here,
> which is not 0. This is correct, as it is the bit mask passed to
> regmap_update_bits. So no, it does not work.
From my personal understanding of the code, the behavior of
enable_is_inverted
is that switch the roles of enable_val and disable_val, so if
enable_is_inverted is set, the regulator_disable_regmap function will
write
enable_val (if it's 0, fallback to enable_mask) to the register, and
regulator_enable_regmap will write disable_val.
Maybe this design is used to ensure enable_val (then fallback to
enable_mask)
is not 0.
>
> It seems quite a lot of drivers depend on this fallback behavior
> though.
>
> ChenYu
>
>>
>>>
>>> if (rdev->desc->enable_is_inverted) {
>>> val = rdev->desc->enable_val;
>>> if (!val)
>>> val = rdev->desc->enable_mask;
>>> } else {
>>> val = rdev->desc->disable_val;
>>> }
>>>
>>> return regmap_update_bits(rdev->regmap,
>>> rdev->desc->enable_reg,
>>> rdev->desc->enable_mask, val);
>>>
>>> Note the fallback to enable_mask if enable_val is not set. The enable
>>> helper has a similar structure. This regulator uses an enable value
>>> of 0, and disable value of 1, it is impossible to get it right. As
>>> it is now, it doesn't really make sense to me.
>>>
>>> The code was introduced in ca5d1b3524b4 ("regulator: helpers: Modify
>>> helpers enabling multi-bit control"). I wonder if it was an
>>> unintended
>>> consequence of the change? (original author CC-ed)
>>>
>>>> + .owner = THIS_MODULE,
>>>> +};
>>>> +
>>>> +/*
>>>> + * I2C driver interface functions
>>>> + */
>>>> +static int sy8106a_i2c_probe(struct i2c_client *i2c,
>>>> + const struct i2c_device_id *id)
>>>> +{
>>>> + struct sy8106a *chip;
>>>> + struct device *dev = &i2c->dev;
>>>> + struct regulator_dev *rdev = NULL;
>>>> + struct regulator_config config = { };
>>>> + unsigned int selector;
>>>> + int error;
>>>> +
>>>> + chip = devm_kzalloc(&i2c->dev, sizeof(struct sy8106a),
>>>> GFP_KERNEL);
>>>> + if (!chip)
>>>> + return -ENOMEM;
>>>> +
>>>> + chip->regmap = devm_regmap_init_i2c(i2c,
>>>> &sy8106a_regmap_config);
>>>> + if (IS_ERR(chip->regmap)) {
>>>> + error = PTR_ERR(chip->regmap);
>>>> + dev_err(&i2c->dev, "Failed to allocate register map:
>>>> %d\n",
>>>> + error);
>>>> + return error;
>>>> + }
>>>> +
>>>> + config.dev = &i2c->dev;
>>>> + config.regmap = chip->regmap;
>>>> + config.driver_data = chip;
>>>> +
>>>> + config.of_node = dev->of_node;
>>>> + config.init_data = of_get_regulator_init_data(dev,
>>>> dev->of_node,
>>>> + &sy8106a_reg);
>>>> +
>>>> + if (!config.init_data)
>>>> + return -ENOMEM;
>>>> +
>>>> + /* Probe regulator */
>>>> + error = regmap_read(chip->regmap, SY8106A_REG_VOUT1_SEL,
>>>> &selector);
>>>> + if (error) {
>>>> + dev_err(&i2c->dev, "Failed to read voltage at probe
>>>> time:
>>>> %d\n", error);
>>>> + return error;
>>>> + }
>>>> +
>>>> + rdev = devm_regulator_register(&i2c->dev, &sy8106a_reg,
>>>> &config);
>>>> + if (IS_ERR(rdev)) {
>>>> + error = PTR_ERR(rdev);
>>>> + dev_err(&i2c->dev, "Failed to register SY8106A
>>>> regulator:
>>>> %d\n", error);
>>>> + return error;
>>>> + }
>>>> +
>>>> + chip->rdev = rdev;
>>>> +
>>>> + i2c_set_clientdata(i2c, chip);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static const struct of_device_id sy8106a_i2c_of_match[] = {
>>>> + { .compatible = "silergy,sy8106a" },
>>>> + { },
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, sy8106a_i2c_of_match);
>>>> +
>>>> +static const struct i2c_device_id sy8106a_i2c_id[] = {
>>>> + { "sy8106a", 0 },
>>>> + { },
>>>> +};
>>>> +MODULE_DEVICE_TABLE(i2c, sy8106a_i2c_id);
>>>> +
>>>> +static struct i2c_driver sy8106a_regulator_driver = {
>>>> + .driver = {
>>>> + .name = "sy8106a",
>>>> + .of_match_table =
>>>> of_match_ptr(sy8106a_i2c_of_match),
>>>> + },
>>>> + .probe = sy8106a_i2c_probe,
>>>> + .id_table = sy8106a_i2c_id,
>>>> +};
>>>> +
>>>> +module_i2c_driver(sy8106a_regulator_driver);
>>>> +
>>>> +MODULE_AUTHOR("Ondřej Jirman <[email protected]>");
>>>> +MODULE_DESCRIPTION("Regulator device driver for Silergy SY8106A");
>>>> +MODULE_LICENSE("GPL v2");
>>>
>>>
>>> This should be "GPL", to match what the license at the very top says.
>>>
>>> Regards
>>> ChenYu
>>>
>>>> --
>>>> 2.13.0
>>>>
>>>> --
>>>> You received this message because you are subscribed to the Google
>>>> Groups
>>>> "linux-sunxi" group.
>>>> To unsubscribe from this group and stop receiving emails from it,
>>>> send an
>>>> email to [email protected].
>>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> [email protected]
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>>
>> --
>> You received this message because you are subscribed to the Google
>> Groups
>> "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send
>> an
>> email to [email protected].
>> For more options, visit https://groups.google.com/d/optout.
On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
> Orange Pi Zero board has a SY8113B regulator, which is controlled via
> GPIO and capable of outputing 1.1V when the PL6 GPIO is set to output 0
> or 1.3V when the PL6 GPIO is set to input or output 1, and the output is
> the power supply of the ARM cores in H2+ SoC.
>
> Add the device tree node of this regulator and set the cpu's cpu-supply
> property to it.
>
> Signed-off-by: Icenowy Zheng <[email protected]>
> ---
> arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
> index 6713d0f2b3f4..28b4433bd7f7 100644
> --- a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
> +++ b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
> @@ -94,6 +94,27 @@
> reset-gpios = <&r_pio 0 7 GPIO_ACTIVE_LOW>;
> post-power-on-delay-ms = <200>;
> };
> +
> + reg_sy8113b: gpio-regulator {
> + compatible = "regulator-gpio";
> +
No need for the newline.
> + regulator-name = "vdd-cpux";
> + regulator-type = "voltage";
> + regulator-boot-on;
> + regulator-always-on;
> + regulator-min-microvolt = <1100000>;
> + regulator-max-microvolt = <1300000>;
> + regulator-ramp-delay = <50>; /* 4ms */
You should also add "enable-active-high". The driver might not
honor the GPIO_ACTIVE_HIGH flag. AFAIK this was a problem with
the old GPIO API.
ChenYu
> +
> + gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
> + gpios-states = <0x1>;
> + states = <1100000 0x0
> + 1300000 0x1>;
> + };
> +};
> +
> +&cpu0 {
> + cpu-supply = <®_sy8113b>;
> };
>
> &ehci0 {
> --
> 2.13.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
On Mon, Jul 24, 2017 at 11:38 AM, <[email protected]> wrote:
> 在 2017-07-24 11:33，Chen-Yu Tsai 写道：
>>
>> On Mon, Jul 24, 2017 at 11:18 AM, <[email protected]> wrote:
>>>
>>> 在 2017-07-24 11:03，Chen-Yu Tsai 写道：
>>>>
>>>>
>>>> On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
>>>>>
>>>>>
>>>>> From: Ondrej Jirman <[email protected]>
>>>>>
>>>>> SY8106A is an I2C attached single output regulator made by Silergy
>>>>> Corp,
>>>>> which is used on several Allwinner H3/H5 SBCs to control the power
>>>>> supply of the ARM cores.
>>>>>
>>>>> Add a driver for it.
>>>>>
>>>>> Signed-off-by: Ondrej Jirman <[email protected]>
>>>>> [Icenowy: Change commit message]
>>>>> Signed-off-by: Icenowy Zheng <[email protected]>
>>>>> ---
>>>>> drivers/regulator/Kconfig | 8 +-
>>>>> drivers/regulator/Makefile | 2 +-
>>>>> drivers/regulator/sy8106a-regulator.c | 163
>>>>> ++++++++++++++++++++++++++++++++++
>>>>> 3 files changed, 171 insertions(+), 2 deletions(-)
>>>>> create mode 100644 drivers/regulator/sy8106a-regulator.c
>>>>>
>>>>> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
>>>>> index 99b9362331b5..1efa73e18d07 100644
>>>>> --- a/drivers/regulator/Kconfig
>>>>> +++ b/drivers/regulator/Kconfig
>>>>> @@ -764,6 +764,13 @@ config REGULATOR_STW481X_VMMC
>>>>> This driver supports the internal VMMC regulator in the
>>>>> STw481x
>>>>> PMIC chips.
>>>>>
>>>>> +config REGULATOR_SY8106A
>>>>> + tristate "Silergy SY8106A regulator"
>>>>> + depends on I2C && (OF || COMPILE_TEST)
>>>>> + select REGMAP_I2C
>>>>> + help
>>>>> + This driver supports SY8106A single output regulator.
>>>>> +
>>>>> config REGULATOR_TPS51632
>>>>> tristate "TI TPS51632 Power Regulator"
>>>>> depends on I2C
>>>>> @@ -938,4 +945,3 @@ config REGULATOR_WM8994
>>>>> WM8994 CODEC.
>>>>>
>>>>> endif
>>>>> -
>>>>> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
>>>>> index 95b1e86ae692..f5120252f86a 100644
>>>>> --- a/drivers/regulator/Makefile
>>>>> +++ b/drivers/regulator/Makefile
>>>>> @@ -95,6 +95,7 @@ obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
>>>>> obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
>>>>> obj-$(CONFIG_REGULATOR_SKY81452) += sky81452-regulator.o
>>>>> obj-$(CONFIG_REGULATOR_STW481X_VMMC) += stw481x-vmmc.o
>>>>> +obj-$(CONFIG_REGULATOR_SY8106A) += sy8106a-regulator.o
>>>>> obj-$(CONFIG_REGULATOR_TI_ABB) += ti-abb-regulator.o
>>>>> obj-$(CONFIG_REGULATOR_TPS6105X) += tps6105x-regulator.o
>>>>> obj-$(CONFIG_REGULATOR_TPS62360) += tps62360-regulator.o
>>>>> @@ -120,5 +121,4 @@ obj-$(CONFIG_REGULATOR_WM8350) +=
>>>>> wm8350-regulator.o
>>>>> obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
>>>>> obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
>>>>>
>>>>> -
>>>>> ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
>>>>> diff --git a/drivers/regulator/sy8106a-regulator.c
>>>>> b/drivers/regulator/sy8106a-regulator.c
>>>>> new file mode 100644
>>>>> index 000000000000..1df889f68b3d
>>>>> --- /dev/null
>>>>> +++ b/drivers/regulator/sy8106a-regulator.c
>>>>> @@ -0,0 +1,163 @@
>>>>> +/*
>>>>> + * sy8106a-regulator.c - Regulator device driver for SY8106A
>>>>> + *
>>>>> + * Copyright (C) 2016 Ondřej Jirman <[email protected]>
>>>>> + *
>>>>> + * This program is free software; you can redistribute it and/or
>>>>> + * modify it under the terms of the GNU Library General Public
>>>>> + * License as published by the Free Software Foundation; either
>>>>> + * version 2 of the License, or (at your option) any later version.
>>>>> + *
>>>>> + * This program is distributed in the hope that it will be useful,
>>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>>>> + * Library General Public License for more details.
>>>>> + */
>>>>> +
>>>>> +#include <linux/err.h>
>>>>> +#include <linux/i2c.h>
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/regmap.h>
>>>>> +#include <linux/regulator/driver.h>
>>>>> +#include <linux/regulator/of_regulator.h>
>>>>> +
>>>>> +#define SY8106A_REG_VOUT1_SEL 0x01
>>>>> +#define SY8106A_REG_VOUT_COM 0x02
>>>>> +#define SY8106A_REG_VOUT1_SEL_MASK 0x7f
>>>>> +#define SY8106A_DISABLE_REG BIT(0)
>>>>> +#define SY8106A_GO_BIT BIT(7)
>>>>> +
>>>>> +struct sy8106a {
>>>>> + struct regulator_dev *rdev;
>>>>> + struct regmap *regmap;
>>>>> +};
>>>>> +
>>>>> +static const struct regmap_config sy8106a_regmap_config = {
>>>>> + .reg_bits = 8,
>>>>> + .val_bits = 8,
>>>>> +};
>>>>> +
>>>>> +static int sy8106a_set_voltage_sel(struct regulator_dev *rdev,
>>>>> unsigned
>>>>> int sel)
>>>>> +{
>>>>> + /* We use our set_voltage_sel in order to avoid unnecessary I2C
>>>>> + * chatter, because the regulator_get_voltage_sel_regmap using
>>>>> + * apply_bit would perform 4 unnecessary transfers instead of
>>>>> one,
>>>>> + * increasing the chance of error.
>>>>> + */
>>>>> + return regmap_write(rdev->regmap, rdev->desc->vsel_reg,
>>>>> + sel | SY8106A_GO_BIT);
>>>>
>>>>
>>>>
>>>> There should also be an explanation of what the "go bit" does.
>>>> In this case it enables control of the voltage over I2C.
>>>>
>>>>> +}
>>>>> +
>>>>> +static const struct regulator_ops sy8106a_ops = {
>>>>> + .is_enabled = regulator_is_enabled_regmap,
>>>>> + .set_voltage_sel = sy8106a_set_voltage_sel,
>>>>> + .set_voltage_time_sel = regulator_set_voltage_time_sel,
>>>>> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
>>>>> + .list_voltage = regulator_list_voltage_linear,
>>>>
>>>>
>>>>
>>>> No enable/disable ops?
>>>>
>>>>> +};
>>>>> +
>>>>> +/* Default limits measured in millivolts and milliamps */
>>>>> +#define SY8106A_MIN_MV 680
>>>>> +#define SY8106A_MAX_MV 1950
>>>>> +#define SY8106A_STEP_MV 10
>>>>> +
>>>>> +static const struct regulator_desc sy8106a_reg = {
>>>>> + .name = "SY8106A",
>>>>> + .id = 0,
>>>>> + .ops = &sy8106a_ops,
>>>>> + .type = REGULATOR_VOLTAGE,
>>>>> + .n_voltages = ((SY8106A_MAX_MV - SY8106A_MIN_MV) /
>>>>> SY8106A_STEP_MV) + 1,
>>>>> + .min_uV = (SY8106A_MIN_MV * 1000),
>>>>> + .uV_step = (SY8106A_STEP_MV * 1000),
>>>>> + .vsel_reg = SY8106A_REG_VOUT1_SEL,
>>>>> + .vsel_mask = SY8106A_REG_VOUT1_SEL_MASK,
>>>>> + .enable_reg = SY8106A_REG_VOUT_COM,
>>>>> + .enable_mask = SY8106A_DISABLE_REG,
>>>>> + .disable_val = SY8106A_DISABLE_REG,
>>>>> + .enable_is_inverted = 1,
>>>>
>>>>
>>>>
>>>> As the regulator core helpers currently are, there is no way to disable
>>>> this regulator. regulator_disable_regmap looks like:
>>>
>>>
>>>
>>> By reading the current code, I think removing the disable_val variable
>>> will
>>> just make it work -- when enabling, the enable_reg will be set to
>>> disable_val
>>> (as enable_is_inverted is 1), and disable_val is defaultly 0; when
>>> disabling,
>>> the enable_reg will be set to enable_mask (fallbacked from enable_val,
>>> which
>>> is also the default value 0), and the regulator successfully get
>>> disabled.
>>
>>
>> (Gmail is wrapping the lines :( )
>>
>> enable_mask is set to SY8106A_DISABLE_REG, which equals disable_val here,
>> which is not 0. This is correct, as it is the bit mask passed to
>> regmap_update_bits. So no, it does not work.
>
>
> From my personal understanding of the code, the behavior of
> enable_is_inverted
> is that switch the roles of enable_val and disable_val, so if
> enable_is_inverted is set, the regulator_disable_regmap function will write
> enable_val (if it's 0, fallback to enable_mask) to the register, and
> regulator_enable_regmap will write disable_val.
>
> Maybe this design is used to ensure enable_val (then fallback to
> enable_mask)
> is not 0.
I think the changes needed to the core are probably too invasive, i.e.
affect too many other drivers, to be worth doing. Especially since we
won't really be using enable/disable much, as the regulator supplies
the CPU cores.
For now, lets just remove the enable/disable related fields, and leave
a TODO note citing the possibly required changes to the core instead.
You haven't assigned the callbacks anyway.
ChenYu
>
>
>>
>> It seems quite a lot of drivers depend on this fallback behavior though.
>>
>> ChenYu
>>
>>>
>>>>
>>>> if (rdev->desc->enable_is_inverted) {
>>>> val = rdev->desc->enable_val;
>>>> if (!val)
>>>> val = rdev->desc->enable_mask;
>>>> } else {
>>>> val = rdev->desc->disable_val;
>>>> }
>>>>
>>>> return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
>>>> rdev->desc->enable_mask, val);
>>>>
>>>> Note the fallback to enable_mask if enable_val is not set. The enable
>>>> helper has a similar structure. This regulator uses an enable value
>>>> of 0, and disable value of 1, it is impossible to get it right. As
>>>> it is now, it doesn't really make sense to me.
>>>>
>>>> The code was introduced in ca5d1b3524b4 ("regulator: helpers: Modify
>>>> helpers enabling multi-bit control"). I wonder if it was an unintended
>>>> consequence of the change? (original author CC-ed)
>>>>
>>>>> + .owner = THIS_MODULE,
>>>>> +};
>>>>> +
>>>>> +/*
>>>>> + * I2C driver interface functions
>>>>> + */
>>>>> +static int sy8106a_i2c_probe(struct i2c_client *i2c,
>>>>> + const struct i2c_device_id *id)
>>>>> +{
>>>>> + struct sy8106a *chip;
>>>>> + struct device *dev = &i2c->dev;
>>>>> + struct regulator_dev *rdev = NULL;
>>>>> + struct regulator_config config = { };
>>>>> + unsigned int selector;
>>>>> + int error;
>>>>> +
>>>>> + chip = devm_kzalloc(&i2c->dev, sizeof(struct sy8106a),
>>>>> GFP_KERNEL);
>>>>> + if (!chip)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + chip->regmap = devm_regmap_init_i2c(i2c,
>>>>> &sy8106a_regmap_config);
>>>>> + if (IS_ERR(chip->regmap)) {
>>>>> + error = PTR_ERR(chip->regmap);
>>>>> + dev_err(&i2c->dev, "Failed to allocate register map:
>>>>> %d\n",
>>>>> + error);
>>>>> + return error;
>>>>> + }
>>>>> +
>>>>> + config.dev = &i2c->dev;
>>>>> + config.regmap = chip->regmap;
>>>>> + config.driver_data = chip;
>>>>> +
>>>>> + config.of_node = dev->of_node;
>>>>> + config.init_data = of_get_regulator_init_data(dev,
>>>>> dev->of_node,
>>>>> + &sy8106a_reg);
>>>>> +
>>>>> + if (!config.init_data)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + /* Probe regulator */
>>>>> + error = regmap_read(chip->regmap, SY8106A_REG_VOUT1_SEL,
>>>>> &selector);
>>>>> + if (error) {
>>>>> + dev_err(&i2c->dev, "Failed to read voltage at probe
>>>>> time:
>>>>> %d\n", error);
>>>>> + return error;
>>>>> + }
>>>>> +
>>>>> + rdev = devm_regulator_register(&i2c->dev, &sy8106a_reg,
>>>>> &config);
>>>>> + if (IS_ERR(rdev)) {
>>>>> + error = PTR_ERR(rdev);
>>>>> + dev_err(&i2c->dev, "Failed to register SY8106A
>>>>> regulator:
>>>>> %d\n", error);
>>>>> + return error;
>>>>> + }
>>>>> +
>>>>> + chip->rdev = rdev;
>>>>> +
>>>>> + i2c_set_clientdata(i2c, chip);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static const struct of_device_id sy8106a_i2c_of_match[] = {
>>>>> + { .compatible = "silergy,sy8106a" },
>>>>> + { },
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, sy8106a_i2c_of_match);
>>>>> +
>>>>> +static const struct i2c_device_id sy8106a_i2c_id[] = {
>>>>> + { "sy8106a", 0 },
>>>>> + { },
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(i2c, sy8106a_i2c_id);
>>>>> +
>>>>> +static struct i2c_driver sy8106a_regulator_driver = {
>>>>> + .driver = {
>>>>> + .name = "sy8106a",
>>>>> + .of_match_table = of_match_ptr(sy8106a_i2c_of_match),
>>>>> + },
>>>>> + .probe = sy8106a_i2c_probe,
>>>>> + .id_table = sy8106a_i2c_id,
>>>>> +};
>>>>> +
>>>>> +module_i2c_driver(sy8106a_regulator_driver);
>>>>> +
>>>>> +MODULE_AUTHOR("Ondřej Jirman <[email protected]>");
>>>>> +MODULE_DESCRIPTION("Regulator device driver for Silergy SY8106A");
>>>>> +MODULE_LICENSE("GPL v2");
>>>>
>>>>
>>>>
>>>> This should be "GPL", to match what the license at the very top says.
>>>>
>>>> Regards
>>>> ChenYu
>>>>
>>>>> --
>>>>> 2.13.0
>>>>>
>>>>> --
>>>>> You received this message because you are subscribed to the Google
>>>>> Groups
>>>>> "linux-sunxi" group.
>>>>> To unsubscribe from this group and stop receiving emails from it, send
>>>>> an
>>>>> email to [email protected].
>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-arm-kernel mailing list
>>>> [email protected]
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>
>>>
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups
>>> "linux-sunxi" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an
>>> email to [email protected].
>>> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
On 23-07-17, 18:27, Icenowy Zheng wrote:
> Some new Allwinner SoCs get supported in the kernel after the
> compatibles are added to cpufreq-dt-platdev driver.
>
> Add their compatible strings in the cpufreq-dt-platdev driver.
>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Viresh Kumar <[email protected]>
> Signed-off-by: Icenowy Zheng <[email protected]>
> ---
> drivers/cpufreq/cpufreq-dt-platdev.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> index 2eb40d46d357..c3851453e84a 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -24,7 +24,11 @@ static const struct of_device_id machines[] __initconst = {
> { .compatible = "allwinner,sun8i-a23", },
> { .compatible = "allwinner,sun8i-a33", },
> { .compatible = "allwinner,sun8i-a83t", },
> + { .compatible = "allwinner,sun8i-h2-plus", },
> { .compatible = "allwinner,sun8i-h3", },
> + { .compatible = "allwinner,sun8i-v3s", },
> + { .compatible = "allwinner,sun50i-a64", },
> + { .compatible = "allwinner,sun50i-h5", },
>
> { .compatible = "apm,xgene-shadowcat", },
>
> @@ -43,6 +47,8 @@ static const struct of_device_id machines[] __initconst = {
> { .compatible = "marvell,pxa250", },
> { .compatible = "marvell,pxa270", },
>
> + { .compatible = "nextthing,gr8", },
> +
> { .compatible = "samsung,exynos3250", },
> { .compatible = "samsung,exynos4210", },
> { .compatible = "samsung,exynos4212", },
Acked-by: Viresh Kumar <[email protected]>
--
viresh
On Monday, July 24, 2017 02:23:49 PM Viresh Kumar wrote:
> On 23-07-17, 18:27, Icenowy Zheng wrote:
> > Some new Allwinner SoCs get supported in the kernel after the
> > compatibles are added to cpufreq-dt-platdev driver.
> >
> > Add their compatible strings in the cpufreq-dt-platdev driver.
> >
> > Cc: "Rafael J. Wysocki" <[email protected]>
> > Cc: Viresh Kumar <[email protected]>
> > Signed-off-by: Icenowy Zheng <[email protected]>
> > ---
> > drivers/cpufreq/cpufreq-dt-platdev.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> > index 2eb40d46d357..c3851453e84a 100644
> > --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> > @@ -24,7 +24,11 @@ static const struct of_device_id machines[] __initconst = {
> > { .compatible = "allwinner,sun8i-a23", },
> > { .compatible = "allwinner,sun8i-a33", },
> > { .compatible = "allwinner,sun8i-a83t", },
> > + { .compatible = "allwinner,sun8i-h2-plus", },
> > { .compatible = "allwinner,sun8i-h3", },
> > + { .compatible = "allwinner,sun8i-v3s", },
> > + { .compatible = "allwinner,sun50i-a64", },
> > + { .compatible = "allwinner,sun50i-h5", },
> >
> > { .compatible = "apm,xgene-shadowcat", },
> >
> > @@ -43,6 +47,8 @@ static const struct of_device_id machines[] __initconst = {
> > { .compatible = "marvell,pxa250", },
> > { .compatible = "marvell,pxa270", },
> >
> > + { .compatible = "nextthing,gr8", },
> > +
> > { .compatible = "samsung,exynos3250", },
> > { .compatible = "samsung,exynos4210", },
> > { .compatible = "samsung,exynos4212", },
>
> Acked-by: Viresh Kumar <[email protected]>
>
>
OK
I'm assuming this will go in via arm-soc along with the rest of the series.
Thanks,
Rafael
On Mon, Jul 24, 2017 at 11:03:24AM +0800, Chen-Yu Tsai wrote:
> On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
> > From: Ondrej Jirman <[email protected]>
> >
> > SY8106A is an I2C attached single output regulator made by Silergy Corp,
> > which is used on several Allwinner H3/H5 SBCs to control the power
> > supply of the ARM cores.
> >
> > Add a driver for it.
Please delete unneeded context from mails when replying. Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.
On 07/23, Icenowy Zheng wrote:
> The CPUX clock, which is the main clock of the ARM core on Allwinner H3,
> can be adjusted by changing the frequency of the PLL_CPUX clock.
>
> Allowing setting parent clock for the CPUX clock, thus the PLL_CPUX
> clock can be adjusted when adjusting the CPUX clock.
>
> Signed-off-by: Icenowy Zheng <[email protected]>
> ---
Acked-by: Stephen Boyd <[email protected]>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 07/23, Icenowy Zheng wrote:
> From: Chen-Yu Tsai <[email protected]>
>
> This patch utilizes the new PLL clk notifier to gate then ungate the
> PLL CPU clock after rate changes. This should prevent any system hangs
> resulting from cpufreq changes to the clk.
>
> Reported-by: Ondrej Jirman <[email protected]>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> Tested-by: Icenowy Zheng <[email protected]>
> ---
Acked-by: Stephen Boyd <[email protected]>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
> From: Ondrej Jirman <[email protected]>
>
> Add SY8106A regulator to r_i2c bus and enable the r_i2c bus on
> Orange Pi PC, then set the power supply of the ARM cores to this
> regulator, in order to enable DVFS.
>
> Signed-off-by: Ondrej Jirman <[email protected]>
> [Icenowy: Enable DVFS in this patch, slight changes and change commit
> message]
> Signed-off-by: Icenowy Zheng <[email protected]>
> ---
> arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
> index 998b60f8d295..d855f8b6254e 100644
> --- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
> +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
> @@ -98,6 +98,10 @@
> status = "okay";
> };
>
> +&cpu0 {
> + cpu-supply = <®_sy8106a>;
> +};
> +
> &ehci0 {
> status = "okay";
> };
> @@ -160,6 +164,21 @@
> };
> };
>
> +&r_i2c {
> + status = "okay";
> +
> + reg_sy8106a: regulator@65 {
> + compatible = "silergy,sy8106a";
> + reg = <0x65>;
> + regulator-name = "vdd-cpux";
> + regulator-min-microvolt = <1000000>;
According to the H3 datasheet, the minimum voltage is 1.1V, not 1V.
Otherwse
> + regulator-max-microvolt = <1400000>;
> + regulator-ramp-delay = <200>;
Is this an actual constraint of the SoC? Or is it a characteristic
of the regulator? If it is the latter, it belongs in the driver.
AFAIK the regulator supports varying the ramp delay (slew rate).
ChenYu
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +};
> +
> &r_pio {
> leds_r_opc: led_pins@0 {
> pins = "PL10";
> --
> 2.13.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
> From: Chen-Yu Tsai <[email protected]>
>
> This patch utilizes the new PLL clk notifier to gate then ungate the
> PLL CPU clock after rate changes. This should prevent any system hangs
> resulting from cpufreq changes to the clk.
>
> Reported-by: Ondrej Jirman <[email protected]>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> Tested-by: Icenowy Zheng <[email protected]>
This is missing
Fixes: 0577e4853bfb ("clk: sunxi-ng: Add H3 clocks")
ChenYu
于 2017年7月26日 GMT+08:00 下午3:08:06, Chen-Yu Tsai <[email protected]> 写到:
>On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
>> From: Ondrej Jirman <[email protected]>
>>
>> Add SY8106A regulator to r_i2c bus and enable the r_i2c bus on
>> Orange Pi PC, then set the power supply of the ARM cores to this
>> regulator, in order to enable DVFS.
>>
>> Signed-off-by: Ondrej Jirman <[email protected]>
>> [Icenowy: Enable DVFS in this patch, slight changes and change commit
>> message]
>> Signed-off-by: Icenowy Zheng <[email protected]>
>> ---
>> arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>> index 998b60f8d295..d855f8b6254e 100644
>> --- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>> +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>> @@ -98,6 +98,10 @@
>> status = "okay";
>> };
>>
>> +&cpu0 {
>> + cpu-supply = <®_sy8106a>;
>> +};
>> +
>> &ehci0 {
>> status = "okay";
>> };
>> @@ -160,6 +164,21 @@
>> };
>> };
>>
>> +&r_i2c {
>> + status = "okay";
>> +
>> + reg_sy8106a: regulator@65 {
>> + compatible = "silergy,sy8106a";
>> + reg = <0x65>;
>> + regulator-name = "vdd-cpux";
>> + regulator-min-microvolt = <1000000>;
>
>According to the H3 datasheet, the minimum voltage is 1.1V, not 1V.
But the Armbian OPP table for H3 contains several
OPP under 1.1V...
>
>Otherwse
>
>> + regulator-max-microvolt = <1400000>;
>> + regulator-ramp-delay = <200>;
>
>Is this an actual constraint of the SoC? Or is it a characteristic
>of the regulator? If it is the latter, it belongs in the driver.
>AFAIK the regulator supports varying the ramp delay (slew rate).
>
>ChenYu
>
>> + regulator-boot-on;
>> + regulator-always-on;
>> + };
>> +};
>> +
>> &r_pio {
>> leds_r_opc: led_pins@0 {
>> pins = "PL10";
>> --
>> 2.13.0
>>
>> --
>> You received this message because you are subscribed to the Google
>Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it,
>send an email to [email protected].
>> For more options, visit https://groups.google.com/d/optout.
On Wed, Jul 26, 2017 at 3:16 PM, Icenowy Zheng <[email protected]> wrote:
>
>
> 于 2017年7月26日 GMT+08:00 下午3:08:06, Chen-Yu Tsai <[email protected]> 写到:
>>On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
>>> From: Ondrej Jirman <[email protected]>
>>>
>>> Add SY8106A regulator to r_i2c bus and enable the r_i2c bus on
>>> Orange Pi PC, then set the power supply of the ARM cores to this
>>> regulator, in order to enable DVFS.
>>>
>>> Signed-off-by: Ondrej Jirman <[email protected]>
>>> [Icenowy: Enable DVFS in this patch, slight changes and change commit
>>> message]
>>> Signed-off-by: Icenowy Zheng <[email protected]>
>>> ---
>>> arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 19 +++++++++++++++++++
>>> 1 file changed, 19 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>>b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>>> index 998b60f8d295..d855f8b6254e 100644
>>> --- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>>> +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>>> @@ -98,6 +98,10 @@
>>> status = "okay";
>>> };
>>>
>>> +&cpu0 {
>>> + cpu-supply = <®_sy8106a>;
>>> +};
>>> +
>>> &ehci0 {
>>> status = "okay";
>>> };
>>> @@ -160,6 +164,21 @@
>>> };
>>> };
>>>
>>> +&r_i2c {
>>> + status = "okay";
>>> +
>>> + reg_sy8106a: regulator@65 {
>>> + compatible = "silergy,sy8106a";
>>> + reg = <0x65>;
>>> + regulator-name = "vdd-cpux";
>>> + regulator-min-microvolt = <1000000>;
>>
>>According to the H3 datasheet, the minimum voltage is 1.1V, not 1V.
>
> But the Armbian OPP table for H3 contains several
> OPP under 1.1V...
Can you provide a link?
If Armbian users have actually field tested this (a big if),
then I would like to see some evidence of the SoC running
stably at those OPPs with those lower voltages under full load.
Even then you should still leave a note describing why we allow
voltages below the recommended range.
ChenYu
>
>>
>>Otherwse
>>
>>> + regulator-max-microvolt = <1400000>;
>>> + regulator-ramp-delay = <200>;
>>
>>Is this an actual constraint of the SoC? Or is it a characteristic
>>of the regulator? If it is the latter, it belongs in the driver.
>>AFAIK the regulator supports varying the ramp delay (slew rate).
>>
>>ChenYu
>>
>>> + regulator-boot-on;
>>> + regulator-always-on;
>>> + };
>>> +};
>>> +
>>> &r_pio {
>>> leds_r_opc: led_pins@0 {
>>> pins = "PL10";
>>> --
>>> 2.13.0
>>>
在 2017-07-26 15:30，Chen-Yu Tsai 写道：
> On Wed, Jul 26, 2017 at 3:16 PM, Icenowy Zheng <[email protected]> wrote:
>>
>>
>> 于 2017年7月26日 GMT+08:00 下午3:08:06, Chen-Yu Tsai <[email protected]> 写到:
>>> On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]>
>>> wrote:
>>>> From: Ondrej Jirman <[email protected]>
>>>>
>>>> Add SY8106A regulator to r_i2c bus and enable the r_i2c bus on
>>>> Orange Pi PC, then set the power supply of the ARM cores to this
>>>> regulator, in order to enable DVFS.
>>>>
>>>> Signed-off-by: Ondrej Jirman <[email protected]>
>>>> [Icenowy: Enable DVFS in this patch, slight changes and change
>>>> commit
>>>> message]
>>>> Signed-off-by: Icenowy Zheng <[email protected]>
>>>> ---
>>>> arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 19 +++++++++++++++++++
>>>> 1 file changed, 19 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>>> b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>>>> index 998b60f8d295..d855f8b6254e 100644
>>>> --- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>>>> +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
>>>> @@ -98,6 +98,10 @@
>>>> status = "okay";
>>>> };
>>>>
>>>> +&cpu0 {
>>>> + cpu-supply = <®_sy8106a>;
>>>> +};
>>>> +
>>>> &ehci0 {
>>>> status = "okay";
>>>> };
>>>> @@ -160,6 +164,21 @@
>>>> };
>>>> };
>>>>
>>>> +&r_i2c {
>>>> + status = "okay";
>>>> +
>>>> + reg_sy8106a: regulator@65 {
>>>> + compatible = "silergy,sy8106a";
>>>> + reg = <0x65>;
>>>> + regulator-name = "vdd-cpux";
>>>> + regulator-min-microvolt = <1000000>;
>>>
>>> According to the H3 datasheet, the minimum voltage is 1.1V, not 1V.
>>
>> But the Armbian OPP table for H3 contains several
>> OPP under 1.1V...
>
> Can you provide a link?
>
> If Armbian users have actually field tested this (a big if),
> then I would like to see some evidence of the SoC running
> stably at those OPPs with those lower voltages under full load.
See [1].
[1]
https://github.com/armbian/build/blob/master/config/fex/orangepipc.fex#L736
>
> Even then you should still leave a note describing why we allow
> voltages below the recommended range.
>
> ChenYu
>
>>
>>>
>>> Otherwse
>>>
>>>> + regulator-max-microvolt = <1400000>;
>>>> + regulator-ramp-delay = <200>;
>>>
>>> Is this an actual constraint of the SoC? Or is it a characteristic
>>> of the regulator? If it is the latter, it belongs in the driver.
>>> AFAIK the regulator supports varying the ramp delay (slew rate).
I don't know...
Maybe I should ask Ondrej?
>>>
>>> ChenYu
>>>
>>>> + regulator-boot-on;
>>>> + regulator-always-on;
>>>> + };
>>>> +};
>>>> +
>>>> &r_pio {
>>>> leds_r_opc: led_pins@0 {
>>>> pins = "PL10";
>>>> --
>>>> 2.13.0
>>>>
Hi,
[email protected] píše v St 26. 07. 2017 v 15:36 +0800:
>
> > > >
> > > > Otherwse
> > > >
> > > > > + regulator-max-microvolt = <1400000>;
> > > > > + regulator-ramp-delay = <200>;
> > > >
> > > > Is this an actual constraint of the SoC? Or is it a characteristic
> > > > of the regulator? If it is the latter, it belongs in the driver.
> > > > AFAIK the regulator supports varying the ramp delay (slew rate).
>
> I don't know...
>
> Maybe I should ask Ondrej?
It is probably neither.
It is used to calculate a delay inserted by the kernel between setting
a new target voltage over I2C and changing the frequency of the CPU.
The actual delay is calculated by the difference between previous and
the new voltage.
I don't remember seeing anything in the datasheet of the regulator.
This is just some low value that works.
It would probably be dependent on the capacitance on the output of the
regulator, actual load (which varies), etc. So it is a board specific
value. One could measure it with an oscilloscope if there's a need to
optimize this.
regards,
o.
> > > >
> > > > ChenYu
> > > >
> > > > > + regulator-boot-on;
> > > > > + regulator-always-on;
> > > > > + };
> > > > > +};
> > > > > +
> > > > > &r_pio {
> > > > > leds_r_opc: led_pins@0 {
> > > > > pins = "PL10";
> > > > > --
> > > > > 2.13.0
> > > > >
Hi,
On Wed, Jul 26, 2017 at 12:23:48PM +0200, Ondřej Jirman wrote:
> Hi,
>
> [email protected] píše v St 26. 07. 2017 v 15:36 +0800:
> >
> > > > >
> > > > > Otherwse
> > > > >
> > > > > > + regulator-max-microvolt = <1400000>;
> > > > > > + regulator-ramp-delay = <200>;
> > > > >
> > > > > Is this an actual constraint of the SoC? Or is it a characteristic
> > > > > of the regulator? If it is the latter, it belongs in the driver.
> > > > > AFAIK the regulator supports varying the ramp delay (slew rate).
> >
> > I don't know...
> >
> > Maybe I should ask Ondrej?
>
> It is probably neither.
>
> It is used to calculate a delay inserted by the kernel between setting
> a new target voltage over I2C and changing the frequency of the CPU.
> The actual delay is calculated by the difference between previous and
> the new voltage.
>
> I don't remember seeing anything in the datasheet of the regulator.
> This is just some low value that works.
>
> It would probably be dependent on the capacitance on the output of the
> regulator, actual load (which varies), etc. So it is a board specific
> value. One could measure it with an oscilloscope if there's a need to
> optimize this.
If this is a reasonable default, then this should be in the
driver. You can't expect anyone to properly calculate a ramp delay and
have access to both a scope and the CPU power lines.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
在 2017-07-26 19:44，Maxime Ripard 写道：
> Hi,
>
> On Wed, Jul 26, 2017 at 12:23:48PM +0200, Ondřej Jirman wrote:
>> Hi,
>>
>> [email protected] píše v St 26. 07. 2017 v 15:36 +0800:
>> >
>> > > > >
>> > > > > Otherwse
>> > > > >
>> > > > > > + regulator-max-microvolt = <1400000>;
>> > > > > > + regulator-ramp-delay = <200>;
>> > > > >
>> > > > > Is this an actual constraint of the SoC? Or is it a characteristic
>> > > > > of the regulator? If it is the latter, it belongs in the driver.
>> > > > > AFAIK the regulator supports varying the ramp delay (slew rate).
>> >
>> > I don't know...
>> >
>> > Maybe I should ask Ondrej?
>>
>> It is probably neither.
>>
>> It is used to calculate a delay inserted by the kernel between setting
>> a new target voltage over I2C and changing the frequency of the CPU.
>> The actual delay is calculated by the difference between previous and
>> the new voltage.
>>
>> I don't remember seeing anything in the datasheet of the regulator.
>> This is just some low value that works.
>>
>> It would probably be dependent on the capacitance on the output of the
>> regulator, actual load (which varies), etc. So it is a board specific
>> value. One could measure it with an oscilloscope if there's a need to
>> optimize this.
>
> If this is a reasonable default, then this should be in the
> driver. You can't expect anyone to properly calculate a ramp delay and
> have access to both a scope and the CPU power lines.
It seems that in regulator_desc structure a default value of ramp delay
can be set, and the ones specified in dt can override it.
So just add .ramp_delay = 200 in the driver's regulator_desc part?
Should a comment be added that explains it's only an experienced value
on Allwinner H3/H5 boards VDD-CPUX usage?
>
> Maxime
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Maxime Ripard píše v St 26. 07. 2017 v 13:44 +0200:
> Hi,
>
> On Wed, Jul 26, 2017 at 12:23:48PM +0200, Ondřej Jirman wrote:
> > Hi,
> >
> > [email protected] píše v St 26. 07. 2017 v 15:36 +0800:
> > >
> > > > > >
> > > > > > Otherwse
> > > > > >
> > > > > > > + regulator-max-microvolt = <1400000>;
> > > > > > > + regulator-ramp-delay = <200>;
> > > > > >
> > > > > > Is this an actual constraint of the SoC? Or is it a characteristic
> > > > > > of the regulator? If it is the latter, it belongs in the driver.
> > > > > > AFAIK the regulator supports varying the ramp delay (slew rate).
> > >
> > > I don't know...
> > >
> > > Maybe I should ask Ondrej?
> >
> > It is probably neither.
> >
> > It is used to calculate a delay inserted by the kernel between setting
> > a new target voltage over I2C and changing the frequency of the CPU.
> > The actual delay is calculated by the difference between previous and
> > the new voltage.
> >
> > I don't remember seeing anything in the datasheet of the regulator.
> > This is just some low value that works.
> >
> > It would probably be dependent on the capacitance on the output of the
> > regulator, actual load (which varies), etc. So it is a board specific
> > value. One could measure it with an oscilloscope if there's a need to
> > optimize this.
>
> If this is a reasonable default, then this should be in the
> driver. You can't expect anyone to properly calculate a ramp delay and
> have access to both a scope and the CPU power lines.
It translates to 1ms per 0.2V which is highly conservative. The real
times will be in 1-10us range. So I guess this could be a default in
the driver.
regards,
o.
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
>
On Sun, Jul 23, 2017 at 06:27:40PM +0800, Icenowy Zheng wrote:
> From: Ondrej Jirman <[email protected]>
>
> SY8106A is an I2C-controlled adjustable voltage regulator made by
> Silergy Corp.
>
> Add its device tree binding.
>
> Signed-off-by: Ondrej Jirman <[email protected]>
> [Icenowy: Change commit message]
> Signed-off-by: Icenowy Zheng <[email protected]>
> ---
> .../bindings/regulator/sy8106a-regulator.txt | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/sy8106a-regulator.txt
Acked-by: Rob Herring <[email protected]>
On Wed, Jul 26, 2017 at 3:14 PM, Chen-Yu Tsai <[email protected]> wrote:
> On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
>> From: Chen-Yu Tsai <[email protected]>
>>
>> This patch utilizes the new PLL clk notifier to gate then ungate the
>> PLL CPU clock after rate changes. This should prevent any system hangs
>> resulting from cpufreq changes to the clk.
>>
>> Reported-by: Ondrej Jirman <[email protected]>
>> Signed-off-by: Chen-Yu Tsai <[email protected]>
>> Tested-by: Icenowy Zheng <[email protected]>
>
> This is missing
>
> Fixes: 0577e4853bfb ("clk: sunxi-ng: Add H3 clocks")
Applied for 4.14 with the Fixes tag and Stephen's ack.
On Mon, Jul 24, 2017 at 11:10 AM, Chen-Yu Tsai <[email protected]> wrote:
> On Sun, Jul 23, 2017 at 6:27 PM, Icenowy Zheng <[email protected]> wrote:
>> The CPUX clock, which is the main clock of the ARM core on Allwinner H3,
>> can be adjusted by changing the frequency of the PLL_CPUX clock.
>>
>> Allowing setting parent clock for the CPUX clock, thus the PLL_CPUX
>> clock can be adjusted when adjusting the CPUX clock.
>>
>> Signed-off-by: Icenowy Zheng <[email protected]>
>
> Fixes: 0577e4853bfb ("clk: sunxi-ng: Add H3 clocks")
> Reviewed-by: Chen-Yu Tsai <[email protected]>
Applied for 4.14 with the Fixes tag and Stephen's ack.
ChenYu
On Mon, Jul 24, 2017 at 7:46 PM, Rafael J. Wysocki <[email protected]> wrote:
> On Monday, July 24, 2017 02:23:49 PM Viresh Kumar wrote:
>> On 23-07-17, 18:27, Icenowy Zheng wrote:
>> > Some new Allwinner SoCs get supported in the kernel after the
>> > compatibles are added to cpufreq-dt-platdev driver.
>> >
>> > Add their compatible strings in the cpufreq-dt-platdev driver.
>> >
>> > Cc: "Rafael J. Wysocki" <[email protected]>
>> > Cc: Viresh Kumar <[email protected]>
>> > Signed-off-by: Icenowy Zheng <[email protected]>
>> > ---
>> > drivers/cpufreq/cpufreq-dt-platdev.c | 6 ++++++
>> > 1 file changed, 6 insertions(+)
>> >
>> > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
>> > index 2eb40d46d357..c3851453e84a 100644
>> > --- a/drivers/cpufreq/cpufreq-dt-platdev.c
>> > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
>> > @@ -24,7 +24,11 @@ static const struct of_device_id machines[] __initconst = {
>> > { .compatible = "allwinner,sun8i-a23", },
>> > { .compatible = "allwinner,sun8i-a33", },
>> > { .compatible = "allwinner,sun8i-a83t", },
>> > + { .compatible = "allwinner,sun8i-h2-plus", },
>> > { .compatible = "allwinner,sun8i-h3", },
>> > + { .compatible = "allwinner,sun8i-v3s", },
>> > + { .compatible = "allwinner,sun50i-a64", },
>> > + { .compatible = "allwinner,sun50i-h5", },
>> >
>> > { .compatible = "apm,xgene-shadowcat", },
>> >
>> > @@ -43,6 +47,8 @@ static const struct of_device_id machines[] __initconst = {
>> > { .compatible = "marvell,pxa250", },
>> > { .compatible = "marvell,pxa270", },
>> >
>> > + { .compatible = "nextthing,gr8", },
>> > +
>> > { .compatible = "samsung,exynos3250", },
>> > { .compatible = "samsung,exynos4210", },
>> > { .compatible = "samsung,exynos4212", },
>>
>> Acked-by: Viresh Kumar <[email protected]>
>>
>>
>
> OK
>
> I'm assuming this will go in via arm-soc along with the rest of the series.
The rest of the series, except for the clk patches I've already taken,
doesn't seem to be going anywhere just yet. Is there a cpufreq or pm tree
this can be merged through?
Thanks
ChenYu
On Tuesday, August 15, 2017 7:42:19 AM CEST Chen-Yu Tsai wrote:
> On Mon, Jul 24, 2017 at 7:46 PM, Rafael J. Wysocki <[email protected]> wrote:
> > On Monday, July 24, 2017 02:23:49 PM Viresh Kumar wrote:
> >> On 23-07-17, 18:27, Icenowy Zheng wrote:
> >> > Some new Allwinner SoCs get supported in the kernel after the
> >> > compatibles are added to cpufreq-dt-platdev driver.
> >> >
> >> > Add their compatible strings in the cpufreq-dt-platdev driver.
> >> >
> >> > Cc: "Rafael J. Wysocki" <[email protected]>
> >> > Cc: Viresh Kumar <[email protected]>
> >> > Signed-off-by: Icenowy Zheng <[email protected]>
> >> > ---
> >> > drivers/cpufreq/cpufreq-dt-platdev.c | 6 ++++++
> >> > 1 file changed, 6 insertions(+)
> >> >
> >> > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> >> > index 2eb40d46d357..c3851453e84a 100644
> >> > --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> >> > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> >> > @@ -24,7 +24,11 @@ static const struct of_device_id machines[] __initconst = {
> >> > { .compatible = "allwinner,sun8i-a23", },
> >> > { .compatible = "allwinner,sun8i-a33", },
> >> > { .compatible = "allwinner,sun8i-a83t", },
> >> > + { .compatible = "allwinner,sun8i-h2-plus", },
> >> > { .compatible = "allwinner,sun8i-h3", },
> >> > + { .compatible = "allwinner,sun8i-v3s", },
> >> > + { .compatible = "allwinner,sun50i-a64", },
> >> > + { .compatible = "allwinner,sun50i-h5", },
> >> >
> >> > { .compatible = "apm,xgene-shadowcat", },
> >> >
> >> > @@ -43,6 +47,8 @@ static const struct of_device_id machines[] __initconst = {
> >> > { .compatible = "marvell,pxa250", },
> >> > { .compatible = "marvell,pxa270", },
> >> >
> >> > + { .compatible = "nextthing,gr8", },
> >> > +
> >> > { .compatible = "samsung,exynos3250", },
> >> > { .compatible = "samsung,exynos4210", },
> >> > { .compatible = "samsung,exynos4212", },
> >>
> >> Acked-by: Viresh Kumar <[email protected]>
> >>
> >>
> >
> > OK
> >
> > I'm assuming this will go in via arm-soc along with the rest of the series.
>
> The rest of the series, except for the clk patches I've already taken,
> doesn't seem to be going anywhere just yet. Is there a cpufreq or pm tree
> this can be merged through?
You mean this particular patch or the whole series?
If the former, then yes, there is.
Thanks,
Rafael
在 2017-08-15 20:25，Rafael J. Wysocki 写道：
> On Tuesday, August 15, 2017 7:42:19 AM CEST Chen-Yu Tsai wrote:
>> On Mon, Jul 24, 2017 at 7:46 PM, Rafael J. Wysocki <[email protected]>
>> wrote:
>> > On Monday, July 24, 2017 02:23:49 PM Viresh Kumar wrote:
>> >> On 23-07-17, 18:27, Icenowy Zheng wrote:
>> >> > Some new Allwinner SoCs get supported in the kernel after the
>> >> > compatibles are added to cpufreq-dt-platdev driver.
>> >> >
>> >> > Add their compatible strings in the cpufreq-dt-platdev driver.
>> >> >
>> >> > Cc: "Rafael J. Wysocki" <[email protected]>
>> >> > Cc: Viresh Kumar <[email protected]>
>> >> > Signed-off-by: Icenowy Zheng <[email protected]>
>> >> > ---
>> >> > drivers/cpufreq/cpufreq-dt-platdev.c | 6 ++++++
>> >> > 1 file changed, 6 insertions(+)
>> >> >
>> >> > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
>> >> > index 2eb40d46d357..c3851453e84a 100644
>> >> > --- a/drivers/cpufreq/cpufreq-dt-platdev.c
>> >> > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
>> >> > @@ -24,7 +24,11 @@ static const struct of_device_id machines[] __initconst = {
>> >> > { .compatible = "allwinner,sun8i-a23", },
>> >> > { .compatible = "allwinner,sun8i-a33", },
>> >> > { .compatible = "allwinner,sun8i-a83t", },
>> >> > + { .compatible = "allwinner,sun8i-h2-plus", },
>> >> > { .compatible = "allwinner,sun8i-h3", },
>> >> > + { .compatible = "allwinner,sun8i-v3s", },
>> >> > + { .compatible = "allwinner,sun50i-a64", },
>> >> > + { .compatible = "allwinner,sun50i-h5", },
>> >> >
>> >> > { .compatible = "apm,xgene-shadowcat", },
>> >> >
>> >> > @@ -43,6 +47,8 @@ static const struct of_device_id machines[] __initconst = {
>> >> > { .compatible = "marvell,pxa250", },
>> >> > { .compatible = "marvell,pxa270", },
>> >> >
>> >> > + { .compatible = "nextthing,gr8", },
>> >> > +
>> >> > { .compatible = "samsung,exynos3250", },
>> >> > { .compatible = "samsung,exynos4210", },
>> >> > { .compatible = "samsung,exynos4212", },
>> >>
>> >> Acked-by: Viresh Kumar <[email protected]>
>> >>
>> >>
>> >
>> > OK
>> >
>> > I'm assuming this will go in via arm-soc along with the rest of the series.
>>
>> The rest of the series, except for the clk patches I've already taken,
>> doesn't seem to be going anywhere just yet. Is there a cpufreq or pm
>> tree
>> this can be merged through?
>
> You mean this particular patch or the whole series?
>
> If the former, then yes, there is.
I think he means the particular patch.
Other patches in this series still need some review, however, this patch
is standalone and can be taken independently.
>
> Thanks,
> Rafael
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Aug 15, 2017 at 8:25 PM, Rafael J. Wysocki <[email protected]> wrote:
> On Tuesday, August 15, 2017 7:42:19 AM CEST Chen-Yu Tsai wrote:
>> On Mon, Jul 24, 2017 at 7:46 PM, Rafael J. Wysocki <[email protected]> wrote:
>> > On Monday, July 24, 2017 02:23:49 PM Viresh Kumar wrote:
>> >> On 23-07-17, 18:27, Icenowy Zheng wrote:
>> >> > Some new Allwinner SoCs get supported in the kernel after the
>> >> > compatibles are added to cpufreq-dt-platdev driver.
>> >> >
>> >> > Add their compatible strings in the cpufreq-dt-platdev driver.
>> >> >
>> >> > Cc: "Rafael J. Wysocki" <[email protected]>
>> >> > Cc: Viresh Kumar <[email protected]>
>> >> > Signed-off-by: Icenowy Zheng <[email protected]>
>> >> > ---
>> >> > drivers/cpufreq/cpufreq-dt-platdev.c | 6 ++++++
>> >> > 1 file changed, 6 insertions(+)
>> >> >
>> >> > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
>> >> > index 2eb40d46d357..c3851453e84a 100644
>> >> > --- a/drivers/cpufreq/cpufreq-dt-platdev.c
>> >> > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
>> >> > @@ -24,7 +24,11 @@ static const struct of_device_id machines[] __initconst = {
>> >> > { .compatible = "allwinner,sun8i-a23", },
>> >> > { .compatible = "allwinner,sun8i-a33", },
>> >> > { .compatible = "allwinner,sun8i-a83t", },
>> >> > + { .compatible = "allwinner,sun8i-h2-plus", },
>> >> > { .compatible = "allwinner,sun8i-h3", },
>> >> > + { .compatible = "allwinner,sun8i-v3s", },
>> >> > + { .compatible = "allwinner,sun50i-a64", },
>> >> > + { .compatible = "allwinner,sun50i-h5", },
>> >> >
>> >> > { .compatible = "apm,xgene-shadowcat", },
>> >> >
>> >> > @@ -43,6 +47,8 @@ static const struct of_device_id machines[] __initconst = {
>> >> > { .compatible = "marvell,pxa250", },
>> >> > { .compatible = "marvell,pxa270", },
>> >> >
>> >> > + { .compatible = "nextthing,gr8", },
>> >> > +
>> >> > { .compatible = "samsung,exynos3250", },
>> >> > { .compatible = "samsung,exynos4210", },
>> >> > { .compatible = "samsung,exynos4212", },
>> >>
>> >> Acked-by: Viresh Kumar <[email protected]>
>> >>
>> >>
>> >
>> > OK
>> >
>> > I'm assuming this will go in via arm-soc along with the rest of the series.
>>
>> The rest of the series, except for the clk patches I've already taken,
>> doesn't seem to be going anywhere just yet. Is there a cpufreq or pm tree
>> this can be merged through?
>
> You mean this particular patch or the whole series?
>
> If the former, then yes, there is.
Just this patch. As I mentioned the rest of the series still needs some
work, and likely won't make it in this cycle. If you could take this
through the tree it would normally go through, avoiding any conflicts,
that'd be great.
ChenYu
On Tue, Aug 15, 2017 at 8:39 PM, Chen-Yu Tsai <[email protected]> wrote:
> On Tue, Aug 15, 2017 at 8:25 PM, Rafael J. Wysocki <[email protected]> wrote:
>> On Tuesday, August 15, 2017 7:42:19 AM CEST Chen-Yu Tsai wrote:
>>> On Mon, Jul 24, 2017 at 7:46 PM, Rafael J. Wysocki <[email protected]> wrote:
>>> > On Monday, July 24, 2017 02:23:49 PM Viresh Kumar wrote:
>>> >> On 23-07-17, 18:27, Icenowy Zheng wrote:
>>> >> > Some new Allwinner SoCs get supported in the kernel after the
>>> >> > compatibles are added to cpufreq-dt-platdev driver.
>>> >> >
>>> >> > Add their compatible strings in the cpufreq-dt-platdev driver.
>>> >> >
>>> >> > Cc: "Rafael J. Wysocki" <[email protected]>
>>> >> > Cc: Viresh Kumar <[email protected]>
>>> >> > Signed-off-by: Icenowy Zheng <[email protected]>
>>> >> > ---
>>> >> > drivers/cpufreq/cpufreq-dt-platdev.c | 6 ++++++
>>> >> > 1 file changed, 6 insertions(+)
>>> >> >
>>> >> > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
>>> >> > index 2eb40d46d357..c3851453e84a 100644
>>> >> > --- a/drivers/cpufreq/cpufreq-dt-platdev.c
>>> >> > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
>>> >> > @@ -24,7 +24,11 @@ static const struct of_device_id machines[] __initconst = {
>>> >> > { .compatible = "allwinner,sun8i-a23", },
>>> >> > { .compatible = "allwinner,sun8i-a33", },
>>> >> > { .compatible = "allwinner,sun8i-a83t", },
>>> >> > + { .compatible = "allwinner,sun8i-h2-plus", },
>>> >> > { .compatible = "allwinner,sun8i-h3", },
>>> >> > + { .compatible = "allwinner,sun8i-v3s", },
>>> >> > + { .compatible = "allwinner,sun50i-a64", },
>>> >> > + { .compatible = "allwinner,sun50i-h5", },
>>> >> >
>>> >> > { .compatible = "apm,xgene-shadowcat", },
>>> >> >
>>> >> > @@ -43,6 +47,8 @@ static const struct of_device_id machines[] __initconst = {
>>> >> > { .compatible = "marvell,pxa250", },
>>> >> > { .compatible = "marvell,pxa270", },
>>> >> >
>>> >> > + { .compatible = "nextthing,gr8", },
>>> >> > +
>>> >> > { .compatible = "samsung,exynos3250", },
>>> >> > { .compatible = "samsung,exynos4210", },
>>> >> > { .compatible = "samsung,exynos4212", },
>>> >>
>>> >> Acked-by: Viresh Kumar <[email protected]>
>>> >>
>>> >>
>>> >
>>> > OK
>>> >
>>> > I'm assuming this will go in via arm-soc along with the rest of the series.
>>>
>>> The rest of the series, except for the clk patches I've already taken,
>>> doesn't seem to be going anywhere just yet. Is there a cpufreq or pm tree
>>> this can be merged through?
>>
>> You mean this particular patch or the whole series?
>>
>> If the former, then yes, there is.
>
> Just this patch. As I mentioned the rest of the series still needs some
> work, and likely won't make it in this cycle. If you could take this
> through the tree it would normally go through, avoiding any conflicts,
> that'd be great.
This patch has been obsoleted by Viresh's "cpufreq: dt-platdev:
Automatically create cpufreq device with OPP v2" patch.
ChenYu