2018-10-23 18:56:50

by Oskari Lemmelä

[permalink] [raw]
Subject: [PATCH v5 00/11] AXP8x3 AC and battery power supply support

AXP813 AC power supply support with input current and
voltage limiting support.

AXP803 AC and battery power supply support.

Changes in v5:
* Return correct input current limit for values 0x6 and 0x7
* Add specific compatibles for AXP803
* Change commit messages
* Add Vasily Khoruzhick Pinebook DTS patch

Changes in v4:
* Change order of axp20x-gpio in axp20x.c
* Fix indentation and spaces to tabs in axp20x.c

Changes in v3:
* Reorder ac_power_supply DT node
* Rename axp20x_ac_power_set_property function
* Split mfd commit

Changes in v2:
* Reuse axp813 compatibles for axp803
* Refactor axp20x_ac_power.c

Oskari Lemmela (10):
dt-bindings: power: supply: axp20x: add AXP813 AC power DT binding
dt-bindings: power: supply: axp20x: add AXP803 power bindings
dt-bindings: gpio: gpio-axp209: add AXP803 GPIO bindings
dt-bindings: iio: adc: add AXP803 ADC bindings
ARM: dts: axp81x: add AC power supply subnode
arm64: dts: allwinner: axp803: add AC and battery power supplies
arm64: dts: allwinner: a64: sopine-baseboard: enable power supplies
power: supply: add AC power supply driver for AXP813
mfd: axp20x: Add AC power supply cell for AXP813
mfd: axp20x: Add supported cells for AXP803

Vasily Khoruzhick (1):
arm64: dts: allwinner: a64: pinebook: enable power supplies

.../devicetree/bindings/gpio/gpio-axp209.txt | 2 +
.../bindings/iio/adc/axp20x_adc.txt | 2 +
.../bindings/power/supply/axp20x_ac_power.txt | 4 +
.../bindings/power/supply/axp20x_battery.txt | 1 +
arch/arm/boot/dts/axp81x.dtsi | 5 +
arch/arm64/boot/dts/allwinner/axp803.dtsi | 33 +++++++
.../dts/allwinner/sun50i-a64-pinebook.dts | 8 ++
.../allwinner/sun50i-a64-sopine-baseboard.dts | 8 ++
drivers/mfd/axp20x.c | 20 ++++
drivers/power/supply/axp20x_ac_power.c | 94 +++++++++++++++++++
include/linux/mfd/axp20x.h | 1 +
11 files changed, 178 insertions(+)

--
2.17.1



2018-10-23 18:55:02

by Oskari Lemmelä

[permalink] [raw]
Subject: [PATCH v5 09/11] power: supply: add AC power supply driver for AXP813

AXP813 and AXP803 PMICs can control input current and minimum voltage.

Both of these values are configurable.

Signed-off-by: Oskari Lemmela <[email protected]>
Reviewed-by: Quentin Schulz <[email protected]>
---
drivers/power/supply/axp20x_ac_power.c | 94 ++++++++++++++++++++++++++
include/linux/mfd/axp20x.h | 1 +
2 files changed, 95 insertions(+)

diff --git a/drivers/power/supply/axp20x_ac_power.c b/drivers/power/supply/axp20x_ac_power.c
index 0771f951b11f..59b4c8d3b961 100644
--- a/drivers/power/supply/axp20x_ac_power.c
+++ b/drivers/power/supply/axp20x_ac_power.c
@@ -27,6 +27,16 @@
#define AXP20X_PWR_STATUS_ACIN_PRESENT BIT(7)
#define AXP20X_PWR_STATUS_ACIN_AVAIL BIT(6)

+#define AXP813_VHOLD_MASK GENMASK(5, 3)
+#define AXP813_VHOLD_UV_TO_BIT(x) ((((x) / 100000) - 40) << 3)
+#define AXP813_VHOLD_REG_TO_UV(x) \
+ (((((x) & AXP813_VHOLD_MASK) >> 3) + 40) * 100000)
+
+#define AXP813_CURR_LIMIT_MASK GENMASK(2, 0)
+#define AXP813_CURR_LIMIT_UA_TO_BIT(x) (((x) / 500000) - 3)
+#define AXP813_CURR_LIMIT_REG_TO_UA(x) \
+ ((((x) & AXP813_CURR_LIMIT_MASK) + 3) * 500000)
+
#define DRVNAME "axp20x-ac-power-supply"

struct axp20x_ac_power {
@@ -102,6 +112,57 @@ static int axp20x_ac_power_get_property(struct power_supply *psy,

return 0;

+ case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+ ret = regmap_read(power->regmap, AXP813_ACIN_PATH_CTRL, &reg);
+ if (ret)
+ return ret;
+
+ val->intval = AXP813_VHOLD_REG_TO_UV(reg);
+
+ return 0;
+
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ ret = regmap_read(power->regmap, AXP813_ACIN_PATH_CTRL, &reg);
+ if (ret)
+ return ret;
+
+ val->intval = AXP813_CURR_LIMIT_REG_TO_UA(reg);
+ /* AXP813 datasheet defines values 11x as 4000mA */
+ if (val->intval > 4000000)
+ val->intval = 4000000;
+
+ return 0;
+
+ default:
+ return -EINVAL;
+ }
+
+ return -EINVAL;
+}
+
+static int axp813_ac_power_set_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ struct axp20x_ac_power *power = power_supply_get_drvdata(psy);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+ if (val->intval < 4000000 || val->intval > 4700000)
+ return -EINVAL;
+
+ return regmap_update_bits(power->regmap, AXP813_ACIN_PATH_CTRL,
+ AXP813_VHOLD_MASK,
+ AXP813_VHOLD_UV_TO_BIT(val->intval));
+
+ case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+ if (val->intval < 1500000 || val->intval > 4000000)
+ return -EINVAL;
+
+ return regmap_update_bits(power->regmap, AXP813_ACIN_PATH_CTRL,
+ AXP813_CURR_LIMIT_MASK,
+ AXP813_CURR_LIMIT_UA_TO_BIT(val->intval));
+
default:
return -EINVAL;
}
@@ -109,6 +170,13 @@ static int axp20x_ac_power_get_property(struct power_supply *psy,
return -EINVAL;
}

+static int axp813_ac_power_prop_writeable(struct power_supply *psy,
+ enum power_supply_property psp)
+{
+ return psp == POWER_SUPPLY_PROP_VOLTAGE_MIN ||
+ psp == POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT;
+}
+
static enum power_supply_property axp20x_ac_power_properties[] = {
POWER_SUPPLY_PROP_HEALTH,
POWER_SUPPLY_PROP_PRESENT,
@@ -123,6 +191,14 @@ static enum power_supply_property axp22x_ac_power_properties[] = {
POWER_SUPPLY_PROP_ONLINE,
};

+static enum power_supply_property axp813_ac_power_properties[] = {
+ POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_VOLTAGE_MIN,
+ POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+};
+
static const struct power_supply_desc axp20x_ac_power_desc = {
.name = "axp20x-ac",
.type = POWER_SUPPLY_TYPE_MAINS,
@@ -139,6 +215,16 @@ static const struct power_supply_desc axp22x_ac_power_desc = {
.get_property = axp20x_ac_power_get_property,
};

+static const struct power_supply_desc axp813_ac_power_desc = {
+ .name = "axp813-ac",
+ .type = POWER_SUPPLY_TYPE_MAINS,
+ .properties = axp813_ac_power_properties,
+ .num_properties = ARRAY_SIZE(axp813_ac_power_properties),
+ .property_is_writeable = axp813_ac_power_prop_writeable,
+ .get_property = axp20x_ac_power_get_property,
+ .set_property = axp813_ac_power_set_property,
+};
+
struct axp_data {
const struct power_supply_desc *power_desc;
bool acin_adc;
@@ -154,6 +240,11 @@ static const struct axp_data axp22x_data = {
.acin_adc = false,
};

+static const struct axp_data axp813_data = {
+ .power_desc = &axp813_ac_power_desc,
+ .acin_adc = false,
+};
+
static int axp20x_ac_power_probe(struct platform_device *pdev)
{
struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
@@ -234,6 +325,9 @@ static const struct of_device_id axp20x_ac_power_match[] = {
}, {
.compatible = "x-powers,axp221-ac-power-supply",
.data = &axp22x_data,
+ }, {
+ .compatible = "x-powers,axp813-ac-power-supply",
+ .data = &axp813_data,
}, { /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, axp20x_ac_power_match);
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 517e60eecbcb..2302b620d238 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -266,6 +266,7 @@ enum axp20x_variants {
#define AXP288_RT_BATT_V_H 0xa0
#define AXP288_RT_BATT_V_L 0xa1

+#define AXP813_ACIN_PATH_CTRL 0x3a
#define AXP813_ADC_RATE 0x85

/* Fuel Gauge */
--
2.17.1


2018-10-23 18:55:08

by Oskari Lemmelä

[permalink] [raw]
Subject: [PATCH v5 11/11] mfd: axp20x: Add supported cells for AXP803

Parts of the AXP803 are compatible with their counterparts on the AXP813.
These include the GPIO, ADC, AC and battery power supplies.

Signed-off-by: Oskari Lemmela <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
Tested-by: Vasily Khoruzhick <[email protected]>
---
drivers/mfd/axp20x.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index dfc3cff1d08b..e415b967d38c 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -728,11 +728,26 @@ static const struct mfd_cell axp288_cells[] = {

static const struct mfd_cell axp803_cells[] = {
{
+ .name = "axp20x-gpio",
+ .of_compatible = "x-powers,axp813-gpio",
+ }, {
.name = "axp221-pek",
.num_resources = ARRAY_SIZE(axp803_pek_resources),
.resources = axp803_pek_resources,
},
{ .name = "axp20x-regulator" },
+ {
+ .name = "axp813-adc",
+ .of_compatible = "x-powers,axp813-adc",
+ }, {
+ .name = "axp20x-battery-power-supply",
+ .of_compatible = "x-powers,axp813-battery-power-supply",
+ }, {
+ .name = "axp20x-ac-power-supply",
+ .of_compatible = "x-powers,axp813-ac-power-supply",
+ .num_resources = ARRAY_SIZE(axp20x_ac_power_supply_resources),
+ .resources = axp20x_ac_power_supply_resources,
+ },
};

static const struct mfd_cell axp806_self_working_cells[] = {
--
2.17.1


2018-10-23 18:55:18

by Oskari Lemmelä

[permalink] [raw]
Subject: [PATCH v5 10/11] mfd: axp20x: Add AC power supply cell for AXP813

As axp20x-ac-power-supply now supports AXP813, add a cell for it.

Signed-off-by: Oskari Lemmela <[email protected]>
Reviewed-by: Quentin Schulz <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
Tested-by: Vasily Khoruzhick <[email protected]>
---
drivers/mfd/axp20x.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 0be511dd93d0..dfc3cff1d08b 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -778,6 +778,11 @@ static const struct mfd_cell axp813_cells[] = {
}, {
.name = "axp20x-battery-power-supply",
.of_compatible = "x-powers,axp813-battery-power-supply",
+ }, {
+ .name = "axp20x-ac-power-supply",
+ .of_compatible = "x-powers,axp813-ac-power-supply",
+ .num_resources = ARRAY_SIZE(axp20x_ac_power_supply_resources),
+ .resources = axp20x_ac_power_supply_resources,
},
};

--
2.17.1


2018-10-23 18:55:27

by Oskari Lemmelä

[permalink] [raw]
Subject: [PATCH v5 07/11] arm64: dts: allwinner: a64: sopine-baseboard: enable power supplies

AXP803 ACIN pins are routed from SOM to the DC jack on the baseboard.
AXP803 charger pins BATSENSE, LOADSENSE, N_BATDRV, LX_CHG, VIN_CHG
and IPSOUT are connected via PMOS driver to SOM VBAT pins. VBAT and
AXP803 TS pins are routed to the baseboard 3-pin battery connector.

Signed-off-by: Oskari Lemmela <[email protected]>
Reviewed-by: Quentin Schulz <[email protected]>
---
.../boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
index c21f2331add6..335cf2263d19 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
@@ -69,6 +69,14 @@
};
};

+&ac_power_supply {
+ status = "okay";
+};
+
+&battery_power_supply {
+ status = "okay";
+};
+
&ehci0 {
status = "okay";
};
--
2.17.1


2018-10-23 18:55:34

by Oskari Lemmelä

[permalink] [raw]
Subject: [PATCH v5 08/11] arm64: dts: allwinner: a64: pinebook: enable power supplies

From: Vasily Khoruzhick <[email protected]>

Pinebook has ACIN connector and 10000 mAh battery.

Signed-off-by: Vasily Khoruzhick <[email protected]>
---
arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
index 897e60cbe38d..4fabe22ece47 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinebook.dts
@@ -149,6 +149,14 @@

#include "axp803.dtsi"

+&ac_power_supply {
+ status = "okay";
+};
+
+&battery_power_supply {
+ status = "okay";
+};
+
&reg_aldo1 {
regulator-min-microvolt = <2800000>;
regulator-max-microvolt = <2800000>;
--
2.17.1


2018-10-23 18:55:41

by Oskari Lemmelä

[permalink] [raw]
Subject: [PATCH v5 03/11] dt-bindings: gpio: gpio-axp209: add AXP803 GPIO bindings

The AXP803 GPIO is compatible with AXP813 GPIO, but add
specific compatible for it.

Signed-off-by: Oskari Lemmela <[email protected]>
---
Documentation/devicetree/bindings/gpio/gpio-axp209.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
index fc42b2caa06d..5337a21d7bcf 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
@@ -11,6 +11,7 @@ This driver employs the per-pin muxing pattern.
Required properties:
- compatible: Should be one of:
- "x-powers,axp209-gpio"
+ - "x-powers,axp803-gpio"
- "x-powers,axp813-gpio"
- #gpio-cells: Should be two. The first cell is the pin number and the
second is the GPIO flags.
@@ -67,6 +68,7 @@ GPIO0 | gpio_in, gpio_out, ldo, adc
GPIO1 | gpio_in, gpio_out, ldo, adc
GPIO2 | gpio_in, gpio_out

+axp803
axp813
------
GPIO | Functions
--
2.17.1


2018-10-23 18:55:47

by Oskari Lemmelä

[permalink] [raw]
Subject: [PATCH v5 06/11] arm64: dts: allwinner: axp803: add AC and battery power supplies

Parts of the AXP803 are compatible with their counterparts on the AXP813.
Add DT nodes ADC, GPIO, AC and battery power supplies.

Signed-off-by: Oskari Lemmela <[email protected]>
Reviewed-by: Quentin Schulz <[email protected]>
Tested-by: Vasily Khoruzhick <[email protected]>
---
arch/arm64/boot/dts/allwinner/axp803.dtsi | 33 +++++++++++++++++++++++
1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi b/arch/arm64/boot/dts/allwinner/axp803.dtsi
index e5eae8bafc42..c3a618e1279a 100644
--- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
+++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
@@ -49,6 +49,39 @@
interrupt-controller;
#interrupt-cells = <1>;

+ ac_power_supply: ac-power-supply {
+ compatible = "x-powers,axp803-ac-power-supply",
+ "x-powers,axp813-ac-power-supply";
+ status = "disabled";
+ };
+
+ axp_adc: adc {
+ compatible = "x-powers,axp803-adc", "x-powers,axp813-adc";
+ #io-channel-cells = <1>;
+ };
+
+ axp_gpio: gpio {
+ compatible = "x-powers,axp803-gpio", "x-powers,axp813-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ gpio0_ldo: gpio0-ldo {
+ pins = "GPIO0";
+ function = "ldo";
+ };
+
+ gpio1_ldo: gpio1-ldo {
+ pins = "GPIO1";
+ function = "ldo";
+ };
+ };
+
+ battery_power_supply: battery-power-supply {
+ compatible = "x-powers,axp803-battery-power-supply",
+ "x-powers,axp813-battery-power-supply";
+ status = "disabled";
+ };
+
regulators {
/* Default work frequency for buck regulators */
x-powers,dcdc-freq = <3000>;
--
2.17.1


2018-10-23 18:56:16

by Oskari Lemmelä

[permalink] [raw]
Subject: [PATCH v5 01/11] dt-bindings: power: supply: axp20x: add AXP813 AC power DT binding

The AXP803/AXP813 AC power supply can limit input current and voltage.

Signed-off-by: Oskari Lemmela <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: Quentin Schulz <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
Reviewed-by: Sebastian Reichel <[email protected]>
Tested-by: Vasily Khoruzhick <[email protected]>
---
.../devicetree/bindings/power/supply/axp20x_ac_power.txt | 3 +++
1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
index 826e8a879121..7a1fb532abe5 100644
--- a/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
+++ b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
@@ -4,6 +4,7 @@ Required Properties:
- compatible: One of:
"x-powers,axp202-ac-power-supply"
"x-powers,axp221-ac-power-supply"
+ "x-powers,axp813-ac-power-supply"

This node is a subnode of the axp20x PMIC.

@@ -13,6 +14,8 @@ reading ADC channels from the AXP20X ADC.
The AXP22X is only able to tell if an AC power supply is present and
usable.

+AXP813/AXP803 are able to limit current and supply voltage
+
Example:

&axp209 {
--
2.17.1


2018-10-23 18:56:18

by Oskari Lemmelä

[permalink] [raw]
Subject: [PATCH v5 05/11] ARM: dts: axp81x: add AC power supply subnode

Add AC power supply subnode for AXP81X PMIC.

Signed-off-by: Oskari Lemmela <[email protected]>
Reviewed-by: Quentin Schulz <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
Tested-by: Vasily Khoruzhick <[email protected]>
---
arch/arm/boot/dts/axp81x.dtsi | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi
index 043c717dcef1..bd83962d3627 100644
--- a/arch/arm/boot/dts/axp81x.dtsi
+++ b/arch/arm/boot/dts/axp81x.dtsi
@@ -48,6 +48,11 @@
interrupt-controller;
#interrupt-cells = <1>;

+ ac_power_supply: ac-power-supply {
+ compatible = "x-powers,axp813-ac-power-supply";
+ status = "disabled";
+ };
+
axp_adc: adc {
compatible = "x-powers,axp813-adc";
#io-channel-cells = <1>;
--
2.17.1


2018-10-23 18:56:31

by Oskari Lemmelä

[permalink] [raw]
Subject: [PATCH v5 02/11] dt-bindings: power: supply: axp20x: add AXP803 power bindings

The AXP803 power supplies are compatible with AXP813, but
add specific compatibles for them.

Signed-off-by: Oskari Lemmela <[email protected]>
---
.../devicetree/bindings/power/supply/axp20x_ac_power.txt | 1 +
.../devicetree/bindings/power/supply/axp20x_battery.txt | 1 +
2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
index 7a1fb532abe5..acdeb4b8f4cc 100644
--- a/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
+++ b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
@@ -4,6 +4,7 @@ Required Properties:
- compatible: One of:
"x-powers,axp202-ac-power-supply"
"x-powers,axp221-ac-power-supply"
+ "x-powers,axp803-ac-power-supply"
"x-powers,axp813-ac-power-supply"

This node is a subnode of the axp20x PMIC.
diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
index 41916f69902c..780ebd7e3b84 100644
--- a/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
+++ b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
@@ -4,6 +4,7 @@ Required Properties:
- compatible, one of:
"x-powers,axp209-battery-power-supply"
"x-powers,axp221-battery-power-supply"
+ "x-powers,axp803-battery-power-supply"
"x-powers,axp813-battery-power-supply"

This node is a subnode of its respective PMIC DT node.
--
2.17.1


2018-10-23 18:56:50

by Oskari Lemmelä

[permalink] [raw]
Subject: [PATCH v5 04/11] dt-bindings: iio: adc: add AXP803 ADC bindings

The AXP803 ADC is compatible with AXP813 ADC, but add
specific compatible for it.

Signed-off-by: Oskari Lemmela <[email protected]>
---
Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt b/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt
index 7a6313913923..1dbd5e480acd 100644
--- a/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt
@@ -4,6 +4,7 @@ Required properties:
- compatible: should be one of:
- "x-powers,axp209-adc",
- "x-powers,axp221-adc",
+ - "x-powers,axp803-adc",
- "x-powers,axp813-adc",
- #io-channel-cells: should be 1,

@@ -39,6 +40,7 @@ AXP22x
2 | batt_chrg_i
3 | batt_dischrg_i

+AXP803
AXP813
------
0 | pmic_temp
--
2.17.1


2018-10-24 13:57:04

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v5 03/11] dt-bindings: gpio: gpio-axp209: add AXP803 GPIO bindings

On Tue, 23 Oct 2018 21:53:22 +0300, Oskari Lemmela wrote:
> The AXP803 GPIO is compatible with AXP813 GPIO, but add
> specific compatible for it.
>
> Signed-off-by: Oskari Lemmela <[email protected]>
> ---
> Documentation/devicetree/bindings/gpio/gpio-axp209.txt | 2 ++
> 1 file changed, 2 insertions(+)
>

Reviewed-by: Rob Herring <[email protected]>

2018-10-24 13:57:25

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v5 04/11] dt-bindings: iio: adc: add AXP803 ADC bindings

On Tue, 23 Oct 2018 21:53:23 +0300, Oskari Lemmela wrote:
> The AXP803 ADC is compatible with AXP813 ADC, but add
> specific compatible for it.
>
> Signed-off-by: Oskari Lemmela <[email protected]>
> ---
> Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt | 2 ++
> 1 file changed, 2 insertions(+)
>

Reviewed-by: Rob Herring <[email protected]>

2018-10-24 13:57:53

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v5 02/11] dt-bindings: power: supply: axp20x: add AXP803 power bindings

On Tue, Oct 23, 2018 at 09:53:21PM +0300, Oskari Lemmela wrote:
> The AXP803 power supplies are compatible with AXP813, but
> add specific compatibles for them.
>
> Signed-off-by: Oskari Lemmela <[email protected]>
> ---
> .../devicetree/bindings/power/supply/axp20x_ac_power.txt | 1 +
> .../devicetree/bindings/power/supply/axp20x_battery.txt | 1 +
> 2 files changed, 2 insertions(+)

Reviewed-by: Rob Herring <[email protected]>

2018-10-25 10:24:15

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v5 09/11] power: supply: add AC power supply driver for AXP813

On Tue, 23 Oct 2018, Oskari Lemmela wrote:

> AXP813 and AXP803 PMICs can control input current and minimum voltage.
>
> Both of these values are configurable.
>
> Signed-off-by: Oskari Lemmela <[email protected]>
> Reviewed-by: Quentin Schulz <[email protected]>
> ---
> drivers/power/supply/axp20x_ac_power.c | 94 ++++++++++++++++++++++++++

> include/linux/mfd/axp20x.h | 1 +

Acked-by: Lee Jones <[email protected]>

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2018-10-25 10:24:42

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v5 10/11] mfd: axp20x: Add AC power supply cell for AXP813

On Tue, 23 Oct 2018, Oskari Lemmela wrote:

> As axp20x-ac-power-supply now supports AXP813, add a cell for it.
>
> Signed-off-by: Oskari Lemmela <[email protected]>
> Reviewed-by: Quentin Schulz <[email protected]>
> Reviewed-by: Chen-Yu Tsai <[email protected]>
> Tested-by: Vasily Khoruzhick <[email protected]>
> ---
> drivers/mfd/axp20x.c | 5 +++++
> 1 file changed, 5 insertions(+)

For my own reference:
Acked-for-MFD-by: Lee Jones <[email protected]>

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2018-10-25 10:27:02

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v5 11/11] mfd: axp20x: Add supported cells for AXP803

On Tue, 23 Oct 2018, Oskari Lemmela wrote:

> Parts of the AXP803 are compatible with their counterparts on the AXP813.
> These include the GPIO, ADC, AC and battery power supplies.
>
> Signed-off-by: Oskari Lemmela <[email protected]>
> Reviewed-by: Chen-Yu Tsai <[email protected]>
> Tested-by: Vasily Khoruzhick <[email protected]>
> ---
> drivers/mfd/axp20x.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)

For my own reference:
Acked-for-MFD-by: Lee Jones <[email protected]>

--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2018-10-25 19:09:26

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v5 00/11] AXP8x3 AC and battery power supply support

Hi,

Patches 1, 2 & 9 look good to me and do not seem to have any
dependencies. I plan to merge them after the merge window
for 4.20 closes.

-- Sebastian

On Tue, Oct 23, 2018 at 09:53:19PM +0300, Oskari Lemmela wrote:
> AXP813 AC power supply support with input current and
> voltage limiting support.
>
> AXP803 AC and battery power supply support.
>
> Changes in v5:
> * Return correct input current limit for values 0x6 and 0x7
> * Add specific compatibles for AXP803
> * Change commit messages
> * Add Vasily Khoruzhick Pinebook DTS patch
>
> Changes in v4:
> * Change order of axp20x-gpio in axp20x.c
> * Fix indentation and spaces to tabs in axp20x.c
>
> Changes in v3:
> * Reorder ac_power_supply DT node
> * Rename axp20x_ac_power_set_property function
> * Split mfd commit
>
> Changes in v2:
> * Reuse axp813 compatibles for axp803
> * Refactor axp20x_ac_power.c
>
> Oskari Lemmela (10):
> dt-bindings: power: supply: axp20x: add AXP813 AC power DT binding
> dt-bindings: power: supply: axp20x: add AXP803 power bindings
> dt-bindings: gpio: gpio-axp209: add AXP803 GPIO bindings
> dt-bindings: iio: adc: add AXP803 ADC bindings
> ARM: dts: axp81x: add AC power supply subnode
> arm64: dts: allwinner: axp803: add AC and battery power supplies
> arm64: dts: allwinner: a64: sopine-baseboard: enable power supplies
> power: supply: add AC power supply driver for AXP813
> mfd: axp20x: Add AC power supply cell for AXP813
> mfd: axp20x: Add supported cells for AXP803
>
> Vasily Khoruzhick (1):
> arm64: dts: allwinner: a64: pinebook: enable power supplies
>
> .../devicetree/bindings/gpio/gpio-axp209.txt | 2 +
> .../bindings/iio/adc/axp20x_adc.txt | 2 +
> .../bindings/power/supply/axp20x_ac_power.txt | 4 +
> .../bindings/power/supply/axp20x_battery.txt | 1 +
> arch/arm/boot/dts/axp81x.dtsi | 5 +
> arch/arm64/boot/dts/allwinner/axp803.dtsi | 33 +++++++
> .../dts/allwinner/sun50i-a64-pinebook.dts | 8 ++
> .../allwinner/sun50i-a64-sopine-baseboard.dts | 8 ++
> drivers/mfd/axp20x.c | 20 ++++
> drivers/power/supply/axp20x_ac_power.c | 94 +++++++++++++++++++
> include/linux/mfd/axp20x.h | 1 +
> 11 files changed, 178 insertions(+)
>
> --
> 2.17.1
>


Attachments:
(No filename) (2.31 kB)
signature.asc (849.00 B)
Download all attachments

2018-10-28 15:43:34

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v5 04/11] dt-bindings: iio: adc: add AXP803 ADC bindings

On Wed, 24 Oct 2018 08:56:33 -0500
Rob Herring <[email protected]> wrote:

> On Tue, 23 Oct 2018 21:53:23 +0300, Oskari Lemmela wrote:
> > The AXP803 ADC is compatible with AXP813 ADC, but add
> > specific compatible for it.
> >
> > Signed-off-by: Oskari Lemmela <[email protected]>
> > ---
> > Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt | 2 ++
> > 1 file changed, 2 insertions(+)
> >
>
> Reviewed-by: Rob Herring <[email protected]>

This doesn't seem to have any dependencies with the other patches
so applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to ignore. However I am a little curious to know
why we would add the ID and then not use it (that I can see)?

Thanks,

Jonathan

2018-10-29 13:10:24

by Quentin Schulz

[permalink] [raw]
Subject: Re: [PATCH v5 04/11] dt-bindings: iio: adc: add AXP803 ADC bindings

Hi Jonathan,

On Sun, Oct 28, 2018 at 03:40:11PM +0000, Jonathan Cameron wrote:
> On Wed, 24 Oct 2018 08:56:33 -0500
> Rob Herring <[email protected]> wrote:
>
> > On Tue, 23 Oct 2018 21:53:23 +0300, Oskari Lemmela wrote:
> > > The AXP803 ADC is compatible with AXP813 ADC, but add
> > > specific compatible for it.
> > >
> > > Signed-off-by: Oskari Lemmela <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> >
> > Reviewed-by: Rob Herring <[email protected]>
>
> This doesn't seem to have any dependencies with the other patches
> so applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to ignore. However I am a little curious to know
> why we would add the ID and then not use it (that I can see)?
>

Sometimes with Allwinner (and X-Powers), two IPs seem identical until we
discover something that is slightly different. When this happens, we
have to add a compatible to differentiate both. However, we would also
need to change the Device Tree to change the compatible. We would need
to handle the driver behaviour for both Device Trees.

So better anticipate a possible difference so that we don't have to do
some hacks in the driver to handle the device correctly.

As always, Chen-Yu or Maxime may know better so I'm just stating what I
seem to recall.

Quentin


Attachments:
(No filename) (1.41 kB)
signature.asc (849.00 B)
Download all attachments

2018-10-31 02:34:58

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 04/11] dt-bindings: iio: adc: add AXP803 ADC bindings

On Mon, Oct 29, 2018 at 9:10 PM Quentin Schulz
<[email protected]> wrote:
>
> Hi Jonathan,
>
> On Sun, Oct 28, 2018 at 03:40:11PM +0000, Jonathan Cameron wrote:
> > On Wed, 24 Oct 2018 08:56:33 -0500
> > Rob Herring <[email protected]> wrote:
> >
> > > On Tue, 23 Oct 2018 21:53:23 +0300, Oskari Lemmela wrote:
> > > > The AXP803 ADC is compatible with AXP813 ADC, but add
> > > > specific compatible for it.
> > > >
> > > > Signed-off-by: Oskari Lemmela <[email protected]>
> > > > ---
> > > > Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > >
> > > Reviewed-by: Rob Herring <[email protected]>
> >
> > This doesn't seem to have any dependencies with the other patches
> > so applied to the togreg branch of iio.git and pushed out as testing
> > for the autobuilders to ignore. However I am a little curious to know
> > why we would add the ID and then not use it (that I can see)?
> >
>
> Sometimes with Allwinner (and X-Powers), two IPs seem identical until we
> discover something that is slightly different. When this happens, we
> have to add a compatible to differentiate both. However, we would also
> need to change the Device Tree to change the compatible. We would need
> to handle the driver behaviour for both Device Trees.
>
> So better anticipate a possible difference so that we don't have to do
> some hacks in the driver to handle the device correctly.
>
> As always, Chen-Yu or Maxime may know better so I'm just stating what I
> seem to recall.

With Allwinner stuff (X-Powers included), sometimes the documents are
incomplete or have errors. We tend to add a model-specific compatible
just in case things turn out not to be so compatible, unless someone
has triple-checked everything, documents and actual hardware included.

However we don't actually document these, so this patch isn't strictly
needed. (I suppose this might annoy the device tree binding maintainers.)

ChenYu

2018-10-31 02:42:29

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 00/11] AXP8x3 AC and battery power supply support

On Fri, Oct 26, 2018 at 3:08 AM Sebastian Reichel
<[email protected]> wrote:
>
> Hi,
>
> Patches 1, 2 & 9 look good to me and do not seem to have any
> dependencies. I plan to merge them after the merge window
> for 4.20 closes.

Patches 2, 3 & 4 aren't needed. They aren't in the correct format
for model-specific + fallback compatibles either.

ChenYu

>
> -- Sebastian
>
> On Tue, Oct 23, 2018 at 09:53:19PM +0300, Oskari Lemmela wrote:
> > AXP813 AC power supply support with input current and
> > voltage limiting support.
> >
> > AXP803 AC and battery power supply support.
> >
> > Changes in v5:
> > * Return correct input current limit for values 0x6 and 0x7
> > * Add specific compatibles for AXP803
> > * Change commit messages
> > * Add Vasily Khoruzhick Pinebook DTS patch
> >
> > Changes in v4:
> > * Change order of axp20x-gpio in axp20x.c
> > * Fix indentation and spaces to tabs in axp20x.c
> >
> > Changes in v3:
> > * Reorder ac_power_supply DT node
> > * Rename axp20x_ac_power_set_property function
> > * Split mfd commit
> >
> > Changes in v2:
> > * Reuse axp813 compatibles for axp803
> > * Refactor axp20x_ac_power.c
> >
> > Oskari Lemmela (10):
> > dt-bindings: power: supply: axp20x: add AXP813 AC power DT binding
> > dt-bindings: power: supply: axp20x: add AXP803 power bindings
> > dt-bindings: gpio: gpio-axp209: add AXP803 GPIO bindings
> > dt-bindings: iio: adc: add AXP803 ADC bindings
> > ARM: dts: axp81x: add AC power supply subnode
> > arm64: dts: allwinner: axp803: add AC and battery power supplies
> > arm64: dts: allwinner: a64: sopine-baseboard: enable power supplies
> > power: supply: add AC power supply driver for AXP813
> > mfd: axp20x: Add AC power supply cell for AXP813
> > mfd: axp20x: Add supported cells for AXP803
> >
> > Vasily Khoruzhick (1):
> > arm64: dts: allwinner: a64: pinebook: enable power supplies
> >
> > .../devicetree/bindings/gpio/gpio-axp209.txt | 2 +
> > .../bindings/iio/adc/axp20x_adc.txt | 2 +
> > .../bindings/power/supply/axp20x_ac_power.txt | 4 +
> > .../bindings/power/supply/axp20x_battery.txt | 1 +
> > arch/arm/boot/dts/axp81x.dtsi | 5 +
> > arch/arm64/boot/dts/allwinner/axp803.dtsi | 33 +++++++
> > .../dts/allwinner/sun50i-a64-pinebook.dts | 8 ++
> > .../allwinner/sun50i-a64-sopine-baseboard.dts | 8 ++
> > drivers/mfd/axp20x.c | 20 ++++
> > drivers/power/supply/axp20x_ac_power.c | 94 +++++++++++++++++++
> > include/linux/mfd/axp20x.h | 1 +
> > 11 files changed, 178 insertions(+)
> >
> > --
> > 2.17.1
> >

2018-10-31 02:57:29

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 02/11] dt-bindings: power: supply: axp20x: add AXP803 power bindings

On Wed, Oct 24, 2018 at 2:54 AM Oskari Lemmela <[email protected]> wrote:
>
> The AXP803 power supplies are compatible with AXP813, but
> add specific compatibles for them.
>
> Signed-off-by: Oskari Lemmela <[email protected]>
> ---
> .../devicetree/bindings/power/supply/axp20x_ac_power.txt | 1 +
> .../devicetree/bindings/power/supply/axp20x_battery.txt | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
> index 7a1fb532abe5..acdeb4b8f4cc 100644
> --- a/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
> +++ b/Documentation/devicetree/bindings/power/supply/axp20x_ac_power.txt
> @@ -4,6 +4,7 @@ Required Properties:
> - compatible: One of:
> "x-powers,axp202-ac-power-supply"
> "x-powers,axp221-ac-power-supply"
> + "x-powers,axp803-ac-power-supply"
> "x-powers,axp813-ac-power-supply"
>
> This node is a subnode of the axp20x PMIC.
> diff --git a/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
> index 41916f69902c..780ebd7e3b84 100644
> --- a/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
> +++ b/Documentation/devicetree/bindings/power/supply/axp20x_battery.txt
> @@ -4,6 +4,7 @@ Required Properties:
> - compatible, one of:
> "x-powers,axp209-battery-power-supply"
> "x-powers,axp221-battery-power-supply"
> + "x-powers,axp803-battery-power-supply"
> "x-powers,axp813-battery-power-supply"
>
> This node is a subnode of its respective PMIC DT node.

As mentioned in my reply to the cover letter, this patch isn't needed.

We ask people to add model-specific compatibles in the device tree
in case the hardware turns out not to be so compatible with the old
hardware we thought it was compatible with. With the model-specific
compatible in place, we have a way out. Without it, we'd have to ask
everyone to update their device trees, which annoys some people who'd
like to have some sort of backward compatibility.

If the model-specific + fallback compatibles combination was to be
listed, the line would include both compatibles. But we're not doing
that for Allwinner stuff. As mentioned it is there just in case. The
best outcome is we don't care and don't need to use them.

ChenYu

2018-10-31 02:57:49

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 09/11] power: supply: add AC power supply driver for AXP813

On Wed, Oct 24, 2018 at 2:54 AM Oskari Lemmela <[email protected]> wrote:
>
> AXP813 and AXP803 PMICs can control input current and minimum voltage.
>
> Both of these values are configurable.
>
> Signed-off-by: Oskari Lemmela <[email protected]>
> Reviewed-by: Quentin Schulz <[email protected]>

Reviewed-by: Chen-Yu Tsai <[email protected]>

2018-10-31 03:01:47

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 03/11] dt-bindings: gpio: gpio-axp209: add AXP803 GPIO bindings

On Wed, Oct 24, 2018 at 2:54 AM Oskari Lemmela <[email protected]> wrote:
>
> The AXP803 GPIO is compatible with AXP813 GPIO, but add
> specific compatible for it.
>
> Signed-off-by: Oskari Lemmela <[email protected]>
> ---
> Documentation/devicetree/bindings/gpio/gpio-axp209.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
> index fc42b2caa06d..5337a21d7bcf 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
> @@ -11,6 +11,7 @@ This driver employs the per-pin muxing pattern.
> Required properties:
> - compatible: Should be one of:
> - "x-powers,axp209-gpio"
> + - "x-powers,axp803-gpio"
> - "x-powers,axp813-gpio"
> - #gpio-cells: Should be two. The first cell is the pin number and the
> second is the GPIO flags.
> @@ -67,6 +68,7 @@ GPIO0 | gpio_in, gpio_out, ldo, adc
> GPIO1 | gpio_in, gpio_out, ldo, adc
> GPIO2 | gpio_in, gpio_out
>
> +axp803
> axp813
> ------
> GPIO | Functions
> --

As mentioned in my reply to the cover letter, this patch isn't needed.

We ask people to add model-specific compatibles in the device tree
in case the hardware turns out not to be so compatible with the old
hardware we thought it was compatible with. With the model-specific
compatible in place, we have a way out. Without it, we'd have to ask
everyone to update their device trees, which annoys some people who'd
like to have some sort of backward compatibility.

If the model-specific + fallback compatibles combination was to be
listed, the line would include both compatibles. But we're not doing
that for Allwinner stuff. As mentioned it is there just in case. The
best outcome is we don't care and don't need to use them.

ChenYu

2018-10-31 03:05:55

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 06/11] arm64: dts: allwinner: axp803: add AC and battery power supplies

On Wed, Oct 24, 2018 at 2:54 AM Oskari Lemmela <[email protected]> wrote:
>
> Parts of the AXP803 are compatible with their counterparts on the AXP813.
> Add DT nodes ADC, GPIO, AC and battery power supplies.
>
> Signed-off-by: Oskari Lemmela <[email protected]>
> Reviewed-by: Quentin Schulz <[email protected]>
> Tested-by: Vasily Khoruzhick <[email protected]>

Reviewed-by: Chen-Yu Tsai <[email protected]>

2018-10-31 03:06:36

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 07/11] arm64: dts: allwinner: a64: sopine-baseboard: enable power supplies

On Wed, Oct 24, 2018 at 2:54 AM Oskari Lemmela <[email protected]> wrote:
>
> AXP803 ACIN pins are routed from SOM to the DC jack on the baseboard.
> AXP803 charger pins BATSENSE, LOADSENSE, N_BATDRV, LX_CHG, VIN_CHG
> and IPSOUT are connected via PMOS driver to SOM VBAT pins. VBAT and
> AXP803 TS pins are routed to the baseboard 3-pin battery connector.
>
> Signed-off-by: Oskari Lemmela <[email protected]>
> Reviewed-by: Quentin Schulz <[email protected]>

Reviewed-by: Chen-Yu Tsai <[email protected]>

2018-10-31 03:08:23

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 08/11] arm64: dts: allwinner: a64: pinebook: enable power supplies

On Wed, Oct 24, 2018 at 2:54 AM Oskari Lemmela <[email protected]> wrote:
>
> From: Vasily Khoruzhick <[email protected]>
>
> Pinebook has ACIN connector and 10000 mAh battery.
>
> Signed-off-by: Vasily Khoruzhick <[email protected]>

You should have your own SoB following the author's when you resend other
people's patches. Otherwise,

Reviewed-by: Chen-Yu Tsai <[email protected]>

2018-11-03 10:22:36

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v5 04/11] dt-bindings: iio: adc: add AXP803 ADC bindings

On Wed, 31 Oct 2018 10:29:59 +0800
Chen-Yu Tsai <[email protected]> wrote:

> On Mon, Oct 29, 2018 at 9:10 PM Quentin Schulz
> <[email protected]> wrote:
> >
> > Hi Jonathan,
> >
> > On Sun, Oct 28, 2018 at 03:40:11PM +0000, Jonathan Cameron wrote:
> > > On Wed, 24 Oct 2018 08:56:33 -0500
> > > Rob Herring <[email protected]> wrote:
> > >
> > > > On Tue, 23 Oct 2018 21:53:23 +0300, Oskari Lemmela wrote:
> > > > > The AXP803 ADC is compatible with AXP813 ADC, but add
> > > > > specific compatible for it.
> > > > >
> > > > > Signed-off-by: Oskari Lemmela <[email protected]>
> > > > > ---
> > > > > Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > >
> > > > Reviewed-by: Rob Herring <[email protected]>
> > >
> > > This doesn't seem to have any dependencies with the other patches
> > > so applied to the togreg branch of iio.git and pushed out as testing
> > > for the autobuilders to ignore. However I am a little curious to know
> > > why we would add the ID and then not use it (that I can see)?
> > >
> >
> > Sometimes with Allwinner (and X-Powers), two IPs seem identical until we
> > discover something that is slightly different. When this happens, we
> > have to add a compatible to differentiate both. However, we would also
> > need to change the Device Tree to change the compatible. We would need
> > to handle the driver behaviour for both Device Trees.
> >
> > So better anticipate a possible difference so that we don't have to do
> > some hacks in the driver to handle the device correctly.
> >
> > As always, Chen-Yu or Maxime may know better so I'm just stating what I
> > seem to recall.
>
> With Allwinner stuff (X-Powers included), sometimes the documents are
> incomplete or have errors. We tend to add a model-specific compatible
> just in case things turn out not to be so compatible, unless someone
> has triple-checked everything, documents and actual hardware included.
>
> However we don't actually document these, so this patch isn't strictly
> needed. (I suppose this might annoy the device tree binding maintainers.)
>
> ChenYu
I don't think it does any harm so I'll leave it in place. Thanks for
the explanations.

Jonathan



2018-11-05 05:49:00

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH v5 04/11] dt-bindings: iio: adc: add AXP803 ADC bindings

On Sat, Nov 3, 2018 at 6:22 PM Jonathan Cameron
<[email protected]> wrote:
>
> On Wed, 31 Oct 2018 10:29:59 +0800
> Chen-Yu Tsai <[email protected]> wrote:
>
> > On Mon, Oct 29, 2018 at 9:10 PM Quentin Schulz
> > <[email protected]> wrote:
> > >
> > > Hi Jonathan,
> > >
> > > On Sun, Oct 28, 2018 at 03:40:11PM +0000, Jonathan Cameron wrote:
> > > > On Wed, 24 Oct 2018 08:56:33 -0500
> > > > Rob Herring <[email protected]> wrote:
> > > >
> > > > > On Tue, 23 Oct 2018 21:53:23 +0300, Oskari Lemmela wrote:
> > > > > > The AXP803 ADC is compatible with AXP813 ADC, but add
> > > > > > specific compatible for it.
> > > > > >
> > > > > > Signed-off-by: Oskari Lemmela <[email protected]>
> > > > > > ---
> > > > > > Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt | 2 ++
> > > > > > 1 file changed, 2 insertions(+)
> > > > > >
> > > > >
> > > > > Reviewed-by: Rob Herring <[email protected]>
> > > >
> > > > This doesn't seem to have any dependencies with the other patches
> > > > so applied to the togreg branch of iio.git and pushed out as testing
> > > > for the autobuilders to ignore. However I am a little curious to know
> > > > why we would add the ID and then not use it (that I can see)?
> > > >
> > >
> > > Sometimes with Allwinner (and X-Powers), two IPs seem identical until we
> > > discover something that is slightly different. When this happens, we
> > > have to add a compatible to differentiate both. However, we would also
> > > need to change the Device Tree to change the compatible. We would need
> > > to handle the driver behaviour for both Device Trees.
> > >
> > > So better anticipate a possible difference so that we don't have to do
> > > some hacks in the driver to handle the device correctly.
> > >
> > > As always, Chen-Yu or Maxime may know better so I'm just stating what I
> > > seem to recall.
> >
> > With Allwinner stuff (X-Powers included), sometimes the documents are
> > incomplete or have errors. We tend to add a model-specific compatible
> > just in case things turn out not to be so compatible, unless someone
> > has triple-checked everything, documents and actual hardware included.
> >
> > However we don't actually document these, so this patch isn't strictly
> > needed. (I suppose this might annoy the device tree binding maintainers.)
> >
> > ChenYu
> I don't think it does any harm so I'll leave it in place. Thanks for
> the explanations.

It might cause a bit of confusion though. We are not targetting these
compatibles in terms of driver support, and they should not be used
individually. IMO dropping this patch altogether is better. In the
device tree we would use the tuplet axp803 + axp813.

ChenYu

2018-11-11 15:31:00

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v5 04/11] dt-bindings: iio: adc: add AXP803 ADC bindings

On Mon, 5 Nov 2018 13:47:50 +0800
Chen-Yu Tsai <[email protected]> wrote:

> On Sat, Nov 3, 2018 at 6:22 PM Jonathan Cameron
> <[email protected]> wrote:
> >
> > On Wed, 31 Oct 2018 10:29:59 +0800
> > Chen-Yu Tsai <[email protected]> wrote:
> >
> > > On Mon, Oct 29, 2018 at 9:10 PM Quentin Schulz
> > > <[email protected]> wrote:
> > > >
> > > > Hi Jonathan,
> > > >
> > > > On Sun, Oct 28, 2018 at 03:40:11PM +0000, Jonathan Cameron wrote:
> > > > > On Wed, 24 Oct 2018 08:56:33 -0500
> > > > > Rob Herring <[email protected]> wrote:
> > > > >
> > > > > > On Tue, 23 Oct 2018 21:53:23 +0300, Oskari Lemmela wrote:
> > > > > > > The AXP803 ADC is compatible with AXP813 ADC, but add
> > > > > > > specific compatible for it.
> > > > > > >
> > > > > > > Signed-off-by: Oskari Lemmela <[email protected]>
> > > > > > > ---
> > > > > > > Documentation/devicetree/bindings/iio/adc/axp20x_adc.txt | 2 ++
> > > > > > > 1 file changed, 2 insertions(+)
> > > > > > >
> > > > > >
> > > > > > Reviewed-by: Rob Herring <[email protected]>
> > > > >
> > > > > This doesn't seem to have any dependencies with the other patches
> > > > > so applied to the togreg branch of iio.git and pushed out as testing
> > > > > for the autobuilders to ignore. However I am a little curious to know
> > > > > why we would add the ID and then not use it (that I can see)?
> > > > >
> > > >
> > > > Sometimes with Allwinner (and X-Powers), two IPs seem identical until we
> > > > discover something that is slightly different. When this happens, we
> > > > have to add a compatible to differentiate both. However, we would also
> > > > need to change the Device Tree to change the compatible. We would need
> > > > to handle the driver behaviour for both Device Trees.
> > > >
> > > > So better anticipate a possible difference so that we don't have to do
> > > > some hacks in the driver to handle the device correctly.
> > > >
> > > > As always, Chen-Yu or Maxime may know better so I'm just stating what I
> > > > seem to recall.
> > >
> > > With Allwinner stuff (X-Powers included), sometimes the documents are
> > > incomplete or have errors. We tend to add a model-specific compatible
> > > just in case things turn out not to be so compatible, unless someone
> > > has triple-checked everything, documents and actual hardware included.
> > >
> > > However we don't actually document these, so this patch isn't strictly
> > > needed. (I suppose this might annoy the device tree binding maintainers.)
> > >
> > > ChenYu
> > I don't think it does any harm so I'll leave it in place. Thanks for
> > the explanations.
>
> It might cause a bit of confusion though. We are not targetting these
> compatibles in terms of driver support, and they should not be used
> individually. IMO dropping this patch altogether is better. In the
> device tree we would use the tuplet axp803 + axp813.
>
OK. Dropped.

Thanks,

Jonathan

> ChenYu