Changes in v3:
* Update pinctrl driver to address Andy Shevchenko's review comments
from v1, and fix a few other issues.
* Add gpio-ranges property and example snippet to gpio DT bindings.
* Update commit message of patch 01/16 to point out that all register
addresses are obtained using sub_irq_reg().
* Document ccc_table in axp20x_battery. Also update commit message to
note a small fix that is part of that patch.
* Drop axp20x_adc consolidation patch in favor of using separate adc_raw
functions. It's a minor code size optimization that may not be worth
the effort due to implementation complexity.
* Use the FIELD_GET macro in axp20x_adc to further clarify intent.
* Fix a typo in the regulator driver where an AXP20X regulator ID was
mistakenly used instead of an AXP192 regulator ID. Also carry over
an Acked-by: tag from v1. Hope that's okay.
* Accumulate Acked-by: tags from v1 on DT patches.
* Accumulate Acked-by: tags from v2.
Note that regmap maintainer Mark Brown has said the first two patches to
regmap-irq aren't suitable for inclusion into the kernel in their current
state. I'm including them for v3 so the series remains testable.
Changes in v2:
* Do a little cleanup of axp20x_adc suggested by Jonathan Cameron
* Consolidate ADC read functions in axp20x_adc
* Drop the axp192's read_label callback in axp20x_adc
* Clean up the axp192-gpio dt bindings
* Rewrite a problematic bit of code in axp20x_usb_power reported
by kernel test robot
* Support AXP192 in axp20x_battery
* Split up regmap-irq changes to two separate patches
Cover letter from v1:
Hi all,
This patch series adds support for the X-Powers AXP192 PMIC to the
AXP20x driver framework.
The first patch is a small change to regmap-irq to support the AXP192's
unusual IRQ register layout. It isn't possible to include all of the
IRQ registers in one regmap-irq chip without this.
The rest of the changes are pretty straightforward, I think the only
notable parts are the axp20x_adc driver where there seems to be some
opportunities for code reuse (the axp192 is nearly a duplicate of the
axp20x) and the addition of a new pinctrl driver for the axp192, since
the axp20x pinctrl driver was not very easy to adapt.
Aidan MacDonald (16):
regmap-irq: Use sub_irq_reg() to calculate unmask register address
regmap-irq: Add get_irq_reg to support unusual register layouts
dt-bindings: mfd: add bindings for AXP192 MFD device
dt-bindings: iio: adc: axp209: Add AXP192 compatible
dt-bindings: power: supply: axp20x: Add AXP192 compatible
dt-bindings: gpio: Add AXP192 GPIO bindings
dt-bindings: power: axp20x-battery: Add AXP192 compatible
mfd: axp20x: Add support for AXP192
regulator: axp20x: Add support for AXP192
iio: adc: axp20x_adc: Minor code cleanups
iio: adc: axp20x_adc: Add support for AXP192
power: supply: axp20x_usb_power: Add support for AXP192
pinctrl: Add AXP192 pin control driver
power: axp20x_battery: Add constant charge current table
power: axp20x_battery: Support battery status without fuel gauge
power: axp20x_battery: Add support for AXP192
.../bindings/gpio/x-powers,axp192-gpio.yaml | 68 +++
.../bindings/iio/adc/x-powers,axp209-adc.yaml | 18 +
.../bindings/mfd/x-powers,axp152.yaml | 1 +
.../x-powers,axp20x-battery-power-supply.yaml | 1 +
.../x-powers,axp20x-usb-power-supply.yaml | 1 +
drivers/base/regmap/regmap-irq.c | 19 +-
drivers/iio/adc/axp20x_adc.c | 359 +++++++++--
drivers/mfd/axp20x-i2c.c | 2 +
drivers/mfd/axp20x.c | 153 +++++
drivers/pinctrl/Kconfig | 14 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-axp192.c | 562 ++++++++++++++++++
drivers/power/supply/axp20x_battery.c | 143 ++++-
drivers/power/supply/axp20x_usb_power.c | 80 ++-
drivers/regulator/axp20x-regulator.c | 101 +++-
include/linux/mfd/axp20x.h | 84 +++
include/linux/regmap.h | 5 +
17 files changed, 1529 insertions(+), 83 deletions(-)
create mode 100644 Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
create mode 100644 drivers/pinctrl/pinctrl-axp192.c
--
2.35.1
The AXP192's USB power supply is similar to the AXP202 but it has
different USB current limits.
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Aidan MacDonald <[email protected]>
---
.../bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml b/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml
index 0c371b55c9e1..e800b3b97f0d 100644
--- a/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml
+++ b/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml
@@ -22,6 +22,7 @@ properties:
compatible:
oneOf:
- enum:
+ - x-powers,axp192-usb-power-supply
- x-powers,axp202-usb-power-supply
- x-powers,axp221-usb-power-supply
- x-powers,axp223-usb-power-supply
--
2.35.1
Add a new callback, get_irq_reg, for regmap IRQ chips, to support devices
with unusual register layouts. This is required in the rare cases where
the offset of an IRQ register is not constant with respect to the base
register. This is probably best illustrated with an example (taken from
the AXP192 PMIC):
mask status
IRQ0 0x40 0x44
IRQ1 0x41 0x45
IRQ2 0x42 0x46
IRQ3 0x43 0x47
IRQ4 0x4a 0x4d
If we set mask_base = 0x40 and status_base = 0x44, the offsets of each
register relative to the base are:
mask status
IRQ0 0 0
IRQ1 1 1
IRQ2 2 2
IRQ3 3 3
IRQ4 10 9
The existing mapping mechanisms can't include IRQ4 in the same irqchip
as IRQ0-3 because the offset of IRQ4's register depends on which type
of register we're asking for, ie. which base register is used.
The get_irq_reg callback allows drivers to specify an arbitrary mapping
of (base register, register index) pairs to register addresses, instead
of the default linear mapping "base_register + register_index". This
allows unusual layouts, like the one above, to be handled using a single
regmap IRQ chip.
The drawback is that when get_irq_reg is used, it's impossible to use
bulk reads for status registers even if some of them are contiguous,
because the mapping is opaque to regmap-irq. This should be acceptable
for the case of a few infrequently-polled status registers.
Signed-off-by: Aidan MacDonald <[email protected]>
---
drivers/base/regmap/regmap-irq.c | 6 ++++--
include/linux/regmap.h | 5 +++++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 4a2e1f6aa94d..e50437b18284 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -55,7 +55,9 @@ static int sub_irq_reg(struct regmap_irq_chip_data *data,
unsigned int offset;
int reg = 0;
- if (!chip->sub_reg_offsets || !chip->not_fixed_stride) {
+ if (chip->get_irq_reg) {
+ reg = chip->get_irq_reg(base_reg, i);
+ } else if (!chip->sub_reg_offsets || !chip->not_fixed_stride) {
/* Assume linear mapping */
reg = base_reg + (i * map->reg_stride * data->irq_reg_stride);
} else {
@@ -479,7 +481,7 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
}
} else if (!map->use_single_read && map->reg_stride == 1 &&
- data->irq_reg_stride == 1) {
+ data->irq_reg_stride == 1 && !chip->get_irq_reg) {
u8 *buf8 = data->status_reg_buf;
u16 *buf16 = data->status_reg_buf;
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 8952fa3d0d59..4828021ab9e8 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -1495,6 +1495,10 @@ struct regmap_irq_sub_irq_map {
* after handling the interrupts in regmap_irq_handler().
* @set_type_virt: Driver specific callback to extend regmap_irq_set_type()
* and configure virt regs.
+ * @get_irq_reg: Callback to map a register index in range [0, num_regs[
+ * to a register, relative to a specific base register. This
+ * is mainly useful for devices where the register offsets
+ * change depending on the base register.
* @irq_drv_data: Driver specific IRQ data which is passed as parameter when
* driver specific pre/post interrupt handler is called.
*
@@ -1545,6 +1549,7 @@ struct regmap_irq_chip {
int (*handle_post_irq)(void *irq_drv_data);
int (*set_type_virt)(unsigned int **buf, unsigned int type,
unsigned long hwirq, int reg);
+ int (*get_irq_reg)(unsigned int base_reg, int i);
void *irq_drv_data;
};
--
2.35.1
The AXP192 is another X-Powers PMIC similar to the existing ones.
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Aidan MacDonald <[email protected]>
---
Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
index 3a53bae611bc..33c9b1b3cc04 100644
--- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
+++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
@@ -84,6 +84,7 @@ properties:
oneOf:
- enum:
- x-powers,axp152
+ - x-powers,axp192
- x-powers,axp202
- x-powers,axp209
- x-powers,axp221
--
2.35.1
The AXP192 is mostly the same as the AXP202 but has a different
current limit.
Acked-by: Sebastian Reichel <[email protected]>
Signed-off-by: Aidan MacDonald <[email protected]>
---
drivers/power/supply/axp20x_usb_power.c | 80 +++++++++++++++++++++----
1 file changed, 69 insertions(+), 11 deletions(-)
diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
index a1e6d1d44808..03145374ae72 100644
--- a/drivers/power/supply/axp20x_usb_power.c
+++ b/drivers/power/supply/axp20x_usb_power.c
@@ -48,6 +48,9 @@
#define AXP813_VBUS_CLIMIT_2000mA 2
#define AXP813_VBUS_CLIMIT_2500mA 3
+#define AXP192_VBUS_CLIMIT_EN BIT(1)
+#define AXP192_VBUS_CLIMIT_100mA BIT(0)
+
#define AXP20X_ADC_EN1_VBUS_CURR BIT(2)
#define AXP20X_ADC_EN1_VBUS_VOLT BIT(3)
@@ -121,6 +124,24 @@ static void axp20x_usb_power_poll_vbus(struct work_struct *work)
mod_delayed_work(system_power_efficient_wq, &power->vbus_detect, DEBOUNCE_TIME);
}
+static int axp192_get_current_max(struct axp20x_usb_power *power, int *val)
+{
+ unsigned int v;
+ int ret = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v);
+
+ if (ret)
+ return ret;
+
+ if (!(v & AXP192_VBUS_CLIMIT_EN))
+ *val = -1;
+ else if (v & AXP192_VBUS_CLIMIT_100mA)
+ *val = 100000;
+ else
+ *val = 500000;
+
+ return 0;
+}
+
static int axp20x_get_current_max(struct axp20x_usb_power *power, int *val)
{
unsigned int v;
@@ -179,7 +200,7 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
enum power_supply_property psp, union power_supply_propval *val)
{
struct axp20x_usb_power *power = power_supply_get_drvdata(psy);
- unsigned int input, v;
+ unsigned int input, v, reg;
int ret;
switch (psp) {
@@ -215,6 +236,8 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
case POWER_SUPPLY_PROP_CURRENT_MAX:
if (power->axp20x_id == AXP813_ID)
return axp813_get_current_max(power, &val->intval);
+ else if (power->axp20x_id == AXP192_ID)
+ return axp192_get_current_max(power, &val->intval);
return axp20x_get_current_max(power, &val->intval);
case POWER_SUPPLY_PROP_CURRENT_NOW:
if (IS_ENABLED(CONFIG_AXP20X_ADC)) {
@@ -256,16 +279,20 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
val->intval = POWER_SUPPLY_HEALTH_GOOD;
- if (power->axp20x_id == AXP202_ID) {
- ret = regmap_read(power->regmap,
- AXP20X_USB_OTG_STATUS, &v);
- if (ret)
- return ret;
+ if (power->axp20x_id == AXP192_ID)
+ reg = AXP192_USB_OTG_STATUS;
+ else if (power->axp20x_id == AXP202_ID)
+ reg = AXP20X_USB_OTG_STATUS;
+ else
+ /* Other chips do not have an OTG status register */
+ break;
- if (!(v & AXP20X_USB_STATUS_VBUS_VALID))
- val->intval =
- POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
- }
+ ret = regmap_read(power->regmap, reg, &v);
+ if (ret)
+ return ret;
+
+ if (!(v & AXP20X_USB_STATUS_VBUS_VALID))
+ val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
break;
case POWER_SUPPLY_PROP_PRESENT:
val->intval = !!(input & AXP20X_PWR_STATUS_VBUS_PRESENT);
@@ -316,6 +343,24 @@ static int axp20x_usb_power_set_voltage_min(struct axp20x_usb_power *power,
return -EINVAL;
}
+static int axp192_usb_power_set_current_max(struct axp20x_usb_power *power,
+ int intval)
+{
+ int val = AXP192_VBUS_CLIMIT_EN;
+ const int mask = AXP192_VBUS_CLIMIT_EN | AXP192_VBUS_CLIMIT_100mA;
+
+ switch (intval) {
+ case 100000:
+ val |= AXP192_VBUS_CLIMIT_100mA;
+ fallthrough;
+ case 500000:
+ return regmap_update_bits(power->regmap,
+ AXP20X_VBUS_IPSOUT_MGMT, mask, val);
+ default:
+ return -EINVAL;
+ }
+}
+
static int axp813_usb_power_set_current_max(struct axp20x_usb_power *power,
int intval)
{
@@ -383,6 +428,9 @@ static int axp20x_usb_power_set_property(struct power_supply *psy,
if (power->axp20x_id == AXP813_ID)
return axp813_usb_power_set_current_max(power,
val->intval);
+ else if (power->axp20x_id == AXP192_ID)
+ return axp192_usb_power_set_current_max(power,
+ val->intval);
return axp20x_usb_power_set_current_max(power, val->intval);
default:
@@ -468,6 +516,13 @@ struct axp_data {
enum axp20x_variants axp20x_id;
};
+static const struct axp_data axp192_data = {
+ .power_desc = &axp20x_usb_power_desc,
+ .irq_names = axp20x_irq_names,
+ .num_irq_names = ARRAY_SIZE(axp20x_irq_names),
+ .axp20x_id = AXP192_ID,
+};
+
static const struct axp_data axp202_data = {
.power_desc = &axp20x_usb_power_desc,
.irq_names = axp20x_irq_names,
@@ -600,7 +655,7 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
if (ret)
return ret;
- if (power->axp20x_id == AXP202_ID) {
+ if (power->axp20x_id == AXP192_ID || power->axp20x_id == AXP202_ID) {
/* Enable vbus valid checking */
ret = regmap_update_bits(power->regmap, AXP20X_VBUS_MON,
AXP20X_VBUS_MON_VBUS_VALID,
@@ -659,6 +714,9 @@ static int axp20x_usb_power_probe(struct platform_device *pdev)
static const struct of_device_id axp20x_usb_power_match[] = {
{
+ .compatible = "x-powers,axp192-usb-power-supply",
+ .data = &axp192_data,
+ }, {
.compatible = "x-powers,axp202-usb-power-supply",
.data = &axp202_data,
}, {
--
2.35.1
The AXP192 is identical to the AXP20x, except for the addition of
two more GPIO ADC channels.
Signed-off-by: Aidan MacDonald <[email protected]>
---
drivers/iio/adc/axp20x_adc.c | 298 +++++++++++++++++++++++++++++++++--
1 file changed, 289 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
index 041511280e1e..929aa14f9cf3 100644
--- a/drivers/iio/adc/axp20x_adc.c
+++ b/drivers/iio/adc/axp20x_adc.c
@@ -22,11 +22,19 @@
#include <linux/iio/machine.h>
#include <linux/mfd/axp20x.h>
+#define AXP192_ADC_EN1_MASK GENMASK(7, 0)
+#define AXP192_ADC_EN2_MASK (GENMASK(3, 0) | BIT(7))
+
#define AXP20X_ADC_EN1_MASK GENMASK(7, 0)
#define AXP20X_ADC_EN2_MASK (GENMASK(3, 2) | BIT(7))
#define AXP22X_ADC_EN1_MASK (GENMASK(7, 5) | BIT(0))
+#define AXP192_GPIO30_IN_RANGE_GPIO0 BIT(0)
+#define AXP192_GPIO30_IN_RANGE_GPIO1 BIT(1)
+#define AXP192_GPIO30_IN_RANGE_GPIO2 BIT(2)
+#define AXP192_GPIO30_IN_RANGE_GPIO3 BIT(3)
+
#define AXP20X_GPIO10_IN_RANGE_GPIO0 BIT(0)
#define AXP20X_GPIO10_IN_RANGE_GPIO1 BIT(1)
@@ -71,6 +79,25 @@ struct axp20x_adc_iio {
const struct axp_data *data;
};
+enum axp192_adc_channel_v {
+ AXP192_ACIN_V = 0,
+ AXP192_VBUS_V,
+ AXP192_TS_IN,
+ AXP192_GPIO0_V,
+ AXP192_GPIO1_V,
+ AXP192_GPIO2_V,
+ AXP192_GPIO3_V,
+ AXP192_IPSOUT_V,
+ AXP192_BATT_V,
+};
+
+enum axp192_adc_channel_i {
+ AXP192_ACIN_I = 0,
+ AXP192_VBUS_I,
+ AXP192_BATT_CHRG_I,
+ AXP192_BATT_DISCHRG_I,
+};
+
enum axp20x_adc_channel_v {
AXP20X_ACIN_V = 0,
AXP20X_VBUS_V,
@@ -158,6 +185,43 @@ static struct iio_map axp22x_maps[] = {
* The only exception is for the battery. batt_v will be in_voltage6_raw and
* charge current in_current6_raw and discharge current will be in_current7_raw.
*/
+static const struct iio_chan_spec axp192_adc_channels[] = {
+ AXP20X_ADC_CHANNEL(AXP192_ACIN_V, "acin_v", IIO_VOLTAGE,
+ AXP20X_ACIN_V_ADC_H),
+ AXP20X_ADC_CHANNEL(AXP192_ACIN_I, "acin_i", IIO_CURRENT,
+ AXP20X_ACIN_I_ADC_H),
+ AXP20X_ADC_CHANNEL(AXP192_VBUS_V, "vbus_v", IIO_VOLTAGE,
+ AXP20X_VBUS_V_ADC_H),
+ AXP20X_ADC_CHANNEL(AXP192_VBUS_I, "vbus_i", IIO_CURRENT,
+ AXP20X_VBUS_I_ADC_H),
+ {
+ .type = IIO_TEMP,
+ .address = AXP20X_TEMP_ADC_H,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_OFFSET),
+ .datasheet_name = "pmic_temp",
+ },
+ AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO0_V, "gpio0_v", IIO_VOLTAGE,
+ AXP20X_GPIO0_V_ADC_H),
+ AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO1_V, "gpio1_v", IIO_VOLTAGE,
+ AXP20X_GPIO1_V_ADC_H),
+ AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO2_V, "gpio2_v", IIO_VOLTAGE,
+ AXP192_GPIO2_V_ADC_H),
+ AXP20X_ADC_CHANNEL_OFFSET(AXP192_GPIO3_V, "gpio3_v", IIO_VOLTAGE,
+ AXP192_GPIO3_V_ADC_H),
+ AXP20X_ADC_CHANNEL(AXP192_IPSOUT_V, "ipsout_v", IIO_VOLTAGE,
+ AXP20X_IPSOUT_V_HIGH_H),
+ AXP20X_ADC_CHANNEL(AXP192_BATT_V, "batt_v", IIO_VOLTAGE,
+ AXP20X_BATT_V_H),
+ AXP20X_ADC_CHANNEL(AXP192_BATT_CHRG_I, "batt_chrg_i", IIO_CURRENT,
+ AXP20X_BATT_CHRG_I_H),
+ AXP20X_ADC_CHANNEL(AXP192_BATT_DISCHRG_I, "batt_dischrg_i", IIO_CURRENT,
+ AXP20X_BATT_DISCHRG_I_H),
+ AXP20X_ADC_CHANNEL(AXP192_TS_IN, "ts_v", IIO_VOLTAGE,
+ AXP20X_TS_IN_H),
+};
+
static const struct iio_chan_spec axp20x_adc_channels[] = {
AXP20X_ADC_CHANNEL(AXP20X_ACIN_V, "acin_v", IIO_VOLTAGE,
AXP20X_ACIN_V_ADC_H),
@@ -231,6 +295,27 @@ static const struct iio_chan_spec axp813_adc_channels[] = {
AXP288_TS_ADC_H),
};
+static int axp192_adc_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val)
+{
+ struct axp20x_adc_iio *info = iio_priv(indio_dev);
+ int ret, size;
+
+ if (chan->type == IIO_CURRENT &&
+ (chan->channel == AXP192_BATT_CHRG_I ||
+ chan->channel == AXP192_BATT_DISCHRG_I))
+ size = 13;
+ else
+ size = 12;
+
+ ret = axp20x_read_variable_width(info->regmap, chan->address, size);
+ if (ret < 0)
+ return ret;
+
+ *val = ret;
+ return IIO_VAL_INT;
+}
+
static int axp20x_adc_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val)
{
@@ -283,6 +368,44 @@ static int axp813_adc_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
}
+static int axp192_adc_scale_voltage(int channel, int *val, int *val2)
+{
+ switch (channel) {
+ case AXP192_ACIN_V:
+ case AXP192_VBUS_V:
+ *val = 1;
+ *val2 = 700000;
+ return IIO_VAL_INT_PLUS_MICRO;
+
+ case AXP192_GPIO0_V:
+ case AXP192_GPIO1_V:
+ case AXP192_GPIO2_V:
+ case AXP192_GPIO3_V:
+ *val = 0;
+ *val2 = 500000;
+ return IIO_VAL_INT_PLUS_MICRO;
+
+ case AXP192_BATT_V:
+ *val = 1;
+ *val2 = 100000;
+ return IIO_VAL_INT_PLUS_MICRO;
+
+ case AXP192_IPSOUT_V:
+ *val = 1;
+ *val2 = 400000;
+ return IIO_VAL_INT_PLUS_MICRO;
+
+ case AXP192_TS_IN:
+ /* 0.8 mV per LSB */
+ *val = 0;
+ *val2 = 800000;
+ return IIO_VAL_INT_PLUS_MICRO;
+
+ default:
+ return -EINVAL;
+ }
+}
+
static int axp20x_adc_scale_voltage(int channel, int *val, int *val2)
{
switch (channel) {
@@ -386,6 +509,29 @@ static int axp20x_adc_scale_current(int channel, int *val, int *val2)
}
}
+static int axp192_adc_scale(struct iio_chan_spec const *chan, int *val,
+ int *val2)
+{
+ switch (chan->type) {
+ case IIO_VOLTAGE:
+ return axp192_adc_scale_voltage(chan->channel, val, val2);
+
+ case IIO_CURRENT:
+ /*
+ * AXP192 current channels are identical to the AXP20x,
+ * therefore we can re-use the scaling function.
+ */
+ return axp20x_adc_scale_current(chan->channel, val, val2);
+
+ case IIO_TEMP:
+ *val = 100;
+ return IIO_VAL_INT;
+
+ default:
+ return -EINVAL;
+ }
+}
+
static int axp20x_adc_scale(struct iio_chan_spec const *chan, int *val,
int *val2)
{
@@ -445,6 +591,42 @@ static int axp813_adc_scale(struct iio_chan_spec const *chan, int *val,
}
}
+static int axp192_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
+ int *val)
+{
+ struct axp20x_adc_iio *info = iio_priv(indio_dev);
+ unsigned int regval;
+ int ret;
+
+ ret = regmap_read(info->regmap, AXP192_GPIO30_IN_RANGE, ®val);
+ if (ret < 0)
+ return ret;
+
+ switch (channel) {
+ case AXP192_GPIO0_V:
+ regval = FIELD_GET(AXP192_GPIO30_IN_RANGE_GPIO0, regval);
+ break;
+
+ case AXP192_GPIO1_V:
+ regval = FIELD_GET(AXP192_GPIO30_IN_RANGE_GPIO1, regval);
+ break;
+
+ case AXP192_GPIO2_V:
+ regval = FIELD_GET(AXP192_GPIO30_IN_RANGE_GPIO2, regval);
+ break;
+
+ case AXP192_GPIO3_V:
+ regval = FIELD_GET(AXP192_GPIO30_IN_RANGE_GPIO3, regval);
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ *val = regval ? 700000 : 0;
+ return IIO_VAL_INT;
+}
+
static int axp20x_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
int *val)
{
@@ -473,6 +655,22 @@ static int axp20x_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
return IIO_VAL_INT;
}
+static int axp192_adc_offset(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val)
+{
+ switch (chan->type) {
+ case IIO_VOLTAGE:
+ return axp192_adc_offset_voltage(indio_dev, chan->channel, val);
+
+ case IIO_TEMP:
+ *val = -1447;
+ return IIO_VAL_INT;
+
+ default:
+ return -EINVAL;
+ }
+}
+
static int axp20x_adc_offset(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val)
{
@@ -489,6 +687,25 @@ static int axp20x_adc_offset(struct iio_dev *indio_dev,
}
}
+static int axp192_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_OFFSET:
+ return axp192_adc_offset(indio_dev, chan, val);
+
+ case IIO_CHAN_INFO_SCALE:
+ return axp192_adc_scale(chan, val, val2);
+
+ case IIO_CHAN_INFO_RAW:
+ return axp192_adc_raw(indio_dev, chan, val);
+
+ default:
+ return -EINVAL;
+ }
+}
+
static int axp20x_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val,
int *val2, long mask)
@@ -549,6 +766,53 @@ static int axp813_read_raw(struct iio_dev *indio_dev,
}
}
+static int axp192_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int val, int val2,
+ long mask)
+{
+ struct axp20x_adc_iio *info = iio_priv(indio_dev);
+ unsigned int regmask, regval;
+
+ /*
+ * The AXP192 PMIC allows the user to choose between 0V and 0.7V offsets
+ * for (independently) GPIO0-3 when in ADC mode.
+ */
+ if (mask != IIO_CHAN_INFO_OFFSET)
+ return -EINVAL;
+
+ if (val != 0 && val != 700000)
+ return -EINVAL;
+
+ regval = val ? 1 : 0;
+
+ switch (chan->channel) {
+ case AXP192_GPIO0_V:
+ regmask = AXP192_GPIO30_IN_RANGE_GPIO0;
+ regval = FIELD_PREP(AXP192_GPIO30_IN_RANGE_GPIO0, regval);
+ break;
+
+ case AXP192_GPIO1_V:
+ regmask = AXP192_GPIO30_IN_RANGE_GPIO1;
+ regval = FIELD_PREP(AXP192_GPIO30_IN_RANGE_GPIO1, regval);
+ break;
+
+ case AXP192_GPIO2_V:
+ regmask = AXP192_GPIO30_IN_RANGE_GPIO2;
+ regval = FIELD_PREP(AXP192_GPIO30_IN_RANGE_GPIO2, regval);
+ break;
+
+ case AXP192_GPIO3_V:
+ regmask = AXP192_GPIO30_IN_RANGE_GPIO3;
+ regval = FIELD_PREP(AXP192_GPIO30_IN_RANGE_GPIO3, regval);
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return regmap_update_bits(info->regmap, AXP192_GPIO30_IN_RANGE, regmask, regval);
+}
+
static int axp20x_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int val, int val2,
long mask)
@@ -586,6 +850,11 @@ static int axp20x_write_raw(struct iio_dev *indio_dev,
return regmap_update_bits(info->regmap, AXP20X_GPIO10_IN_RANGE, regmask, regval);
}
+static const struct iio_info axp192_adc_iio_info = {
+ .read_raw = axp192_read_raw,
+ .write_raw = axp192_write_raw,
+};
+
static const struct iio_info axp20x_adc_iio_info = {
.read_raw = axp20x_read_raw,
.write_raw = axp20x_write_raw,
@@ -625,19 +894,29 @@ struct axp_data {
int num_channels;
struct iio_chan_spec const *channels;
unsigned long adc_en1_mask;
+ unsigned long adc_en2_mask;
int (*adc_rate)(struct axp20x_adc_iio *info,
int rate);
- bool adc_en2;
struct iio_map *maps;
};
+static const struct axp_data axp192_data = {
+ .iio_info = &axp192_adc_iio_info,
+ .num_channels = ARRAY_SIZE(axp192_adc_channels),
+ .channels = axp192_adc_channels,
+ .adc_en1_mask = AXP192_ADC_EN1_MASK,
+ .adc_en2_mask = AXP192_ADC_EN2_MASK,
+ .adc_rate = axp20x_adc_rate,
+ .maps = axp20x_maps,
+};
+
static const struct axp_data axp20x_data = {
.iio_info = &axp20x_adc_iio_info,
.num_channels = ARRAY_SIZE(axp20x_adc_channels),
.channels = axp20x_adc_channels,
.adc_en1_mask = AXP20X_ADC_EN1_MASK,
+ .adc_en2_mask = AXP20X_ADC_EN2_MASK,
.adc_rate = axp20x_adc_rate,
- .adc_en2 = true,
.maps = axp20x_maps,
};
@@ -647,7 +926,6 @@ static const struct axp_data axp22x_data = {
.channels = axp22x_adc_channels,
.adc_en1_mask = AXP22X_ADC_EN1_MASK,
.adc_rate = axp22x_adc_rate,
- .adc_en2 = false,
.maps = axp22x_maps,
};
@@ -657,11 +935,11 @@ static const struct axp_data axp813_data = {
.channels = axp813_adc_channels,
.adc_en1_mask = AXP22X_ADC_EN1_MASK,
.adc_rate = axp813_adc_rate,
- .adc_en2 = false,
.maps = axp22x_maps,
};
static const struct of_device_id axp20x_adc_of_match[] = {
+ { .compatible = "x-powers,axp192-adc", .data = (void *)&axp192_data, },
{ .compatible = "x-powers,axp209-adc", .data = (void *)&axp20x_data, },
{ .compatible = "x-powers,axp221-adc", .data = (void *)&axp22x_data, },
{ .compatible = "x-powers,axp813-adc", .data = (void *)&axp813_data, },
@@ -670,6 +948,7 @@ static const struct of_device_id axp20x_adc_of_match[] = {
MODULE_DEVICE_TABLE(of, axp20x_adc_of_match);
static const struct platform_device_id axp20x_adc_id_match[] = {
+ { .name = "axp192-adc", .driver_data = (kernel_ulong_t)&axp192_data, },
{ .name = "axp20x-adc", .driver_data = (kernel_ulong_t)&axp20x_data, },
{ .name = "axp22x-adc", .driver_data = (kernel_ulong_t)&axp22x_data, },
{ .name = "axp813-adc", .driver_data = (kernel_ulong_t)&axp813_data, },
@@ -715,10 +994,11 @@ static int axp20x_probe(struct platform_device *pdev)
/* Enable the ADCs on IP */
regmap_write(info->regmap, AXP20X_ADC_EN1, info->data->adc_en1_mask);
- if (info->data->adc_en2)
- /* Enable GPIO0/1 and internal temperature ADCs */
+ if (info->data->adc_en2_mask)
+ /* Enable GPIO and internal temperature ADCs */
regmap_update_bits(info->regmap, AXP20X_ADC_EN2,
- AXP20X_ADC_EN2_MASK, AXP20X_ADC_EN2_MASK);
+ info->data->adc_en2_mask,
+ info->data->adc_en2_mask);
/* Configure ADCs rate */
info->data->adc_rate(info, 100);
@@ -743,7 +1023,7 @@ static int axp20x_probe(struct platform_device *pdev)
fail_map:
regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
- if (info->data->adc_en2)
+ if (info->data->adc_en2_mask)
regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
return ret;
@@ -759,7 +1039,7 @@ static int axp20x_remove(struct platform_device *pdev)
regmap_write(info->regmap, AXP20X_ADC_EN1, 0);
- if (info->data->adc_en2)
+ if (info->data->adc_en2_mask)
regmap_write(info->regmap, AXP20X_ADC_EN2, 0);
return 0;
--
2.35.1
The AXP192 PMIC is different enough from the PMICs supported by
the AXP20x GPIO driver to warrant a separate driver. The AXP192
driver also supports interrupts and pinconf settings.
Signed-off-by: Aidan MacDonald <[email protected]>
---
.../bindings/gpio/x-powers,axp192-gpio.yaml | 68 +++++++++++++++++++
1 file changed, 68 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
diff --git a/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
new file mode 100644
index 000000000000..1e368cac9d3a
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/gpio/x-powers,axp192-gpio.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: X-Powers AXP192 GPIO Device Tree Bindings
+
+maintainers:
+ - Chen-Yu Tsai <[email protected]>
+
+properties:
+ compatible:
+ const: x-powers,axp192-gpio
+
+ gpio-controller: true
+
+ "#gpio-cells":
+ const: 2
+ description: >
+ The first cell is the pin number and the second is the GPIO flags.
+
+ gpio-ranges:
+ maxItems: 1
+
+patternProperties:
+ "-pins$":
+ $ref: /schemas/pinctrl/pinmux-node.yaml#
+
+ properties:
+ pins:
+ items:
+ enum:
+ - GPIO0
+ - GPIO1
+ - GPIO2
+ - GPIO3
+ - GPIO4
+ - N_RSTO
+
+ function:
+ enum:
+ - output
+ - input
+ - ldo
+ - pwm
+ - adc
+ - low_output
+ - floating
+ - ext_chg_ctl
+ - ldo_status
+
+required:
+ - compatible
+ - "#gpio-cells"
+ - gpio-controller
+ - gpio-ranges
+
+additionalProperties: false
+
+examples:
+ - |
+ pinctrl0: gpio@0 {
+ compatible = "x-powers,axp192-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&pinctrl0 0 0 6>;
+ };
--
2.35.1
Add a table-based lookup method for constant charge current,
which is necessary when the setting cannot be represented as
a linear range.
This also replaces the hard-coded 300 mA default ccc setting
if the DT-specified value is unsupported; the minimum value
for the device is now set exactly instead of relying on the
value being rounded down to a supported value.
Signed-off-by: Aidan MacDonald <[email protected]>
---
drivers/power/supply/axp20x_battery.c | 60 +++++++++++++++++++++------
1 file changed, 48 insertions(+), 12 deletions(-)
diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
index 9106077c0dbb..ce22c0a92878 100644
--- a/drivers/power/supply/axp20x_battery.c
+++ b/drivers/power/supply/axp20x_battery.c
@@ -61,6 +61,14 @@ struct axp20x_batt_ps;
struct axp_data {
int ccc_scale;
int ccc_offset;
+ /*
+ * Lookup table for constant charge current, if provided this is used
+ * instead of ccc_scale/ccc_offset.
+ *
+ * The table is indexed by the field AXP20X_CHRG_CTRL1_TGT_CURR so it
+ * must have AXP20X_CHRG_CTRL1_TGT_CURR+1 elements.
+ */
+ const int *ccc_table;
bool has_fg_valid;
int (*get_max_voltage)(struct axp20x_batt_ps *batt, int *val);
int (*set_max_voltage)(struct axp20x_batt_ps *batt, int val);
@@ -176,7 +184,10 @@ static int axp20x_get_constant_charge_current(struct axp20x_batt_ps *axp,
*val &= AXP20X_CHRG_CTRL1_TGT_CURR;
- *val = *val * axp->data->ccc_scale + axp->data->ccc_offset;
+ if (axp->data->ccc_table)
+ *val = axp->data->ccc_table[*val];
+ else
+ *val = *val * axp->data->ccc_scale + axp->data->ccc_offset;
return 0;
}
@@ -389,16 +400,36 @@ static int axp20x_battery_set_max_voltage(struct axp20x_batt_ps *axp20x_batt,
AXP20X_CHRG_CTRL1_TGT_VOLT, val);
}
+static int axp20x_get_constant_charge_current_sel(struct axp20x_batt_ps *axp_batt,
+ int charge_current)
+{
+ int i;
+
+ if (axp_batt->data->ccc_table) {
+ for (i = AXP20X_CHRG_CTRL1_TGT_CURR; i >= 0; --i) {
+ if (axp_batt->data->ccc_table[i] <= charge_current)
+ return i;
+ }
+
+ return -EINVAL;
+ }
+
+ i = (charge_current - axp_batt->data->ccc_offset) / axp_batt->data->ccc_scale;
+
+ if (i > AXP20X_CHRG_CTRL1_TGT_CURR || i < 0)
+ return -EINVAL;
+
+ return i;
+}
+
static int axp20x_set_constant_charge_current(struct axp20x_batt_ps *axp_batt,
int charge_current)
{
if (charge_current > axp_batt->max_ccc)
return -EINVAL;
- charge_current = (charge_current - axp_batt->data->ccc_offset) /
- axp_batt->data->ccc_scale;
-
- if (charge_current > AXP20X_CHRG_CTRL1_TGT_CURR || charge_current < 0)
+ charge_current = axp20x_get_constant_charge_current_sel(axp_batt, charge_current);
+ if (charge_current < 0)
return -EINVAL;
return regmap_update_bits(axp_batt->regmap, AXP20X_CHRG_CTRL1,
@@ -410,14 +441,14 @@ static int axp20x_set_max_constant_charge_current(struct axp20x_batt_ps *axp,
{
bool lower_max = false;
- charge_current = (charge_current - axp->data->ccc_offset) /
- axp->data->ccc_scale;
-
- if (charge_current > AXP20X_CHRG_CTRL1_TGT_CURR || charge_current < 0)
+ charge_current = axp20x_get_constant_charge_current_sel(axp, charge_current);
+ if (charge_current < 0)
return -EINVAL;
- charge_current = charge_current * axp->data->ccc_scale +
- axp->data->ccc_offset;
+ if (axp->data->ccc_table)
+ charge_current = axp->data->ccc_table[charge_current];
+ else
+ charge_current = charge_current * axp->data->ccc_scale + axp->data->ccc_offset;
if (charge_current > axp->max_ccc)
dev_warn(axp->dev,
@@ -629,7 +660,12 @@ static int axp20x_power_probe(struct platform_device *pdev)
ccc)) {
dev_err(&pdev->dev,
"couldn't set constant charge current from DT: fallback to minimum value\n");
- ccc = 300000;
+
+ if (axp20x_batt->data->ccc_table)
+ ccc = axp20x_batt->data->ccc_table[0];
+ else
+ ccc = axp20x_batt->data->ccc_offset;
+
axp20x_batt->max_ccc = ccc;
axp20x_set_constant_charge_current(axp20x_batt, ccc);
}
--
2.35.1
Add support for the AXP192 PMIC.
Acked-by: Mark Brown <[email protected]>
Signed-off-by: Aidan MacDonald <[email protected]>
---
drivers/regulator/axp20x-regulator.c | 101 ++++++++++++++++++++++++---
1 file changed, 92 insertions(+), 9 deletions(-)
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index d260c442b788..2c59f398f6dd 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -27,6 +27,29 @@
#include <linux/regulator/machine.h>
#include <linux/regulator/of_regulator.h>
+#define AXP192_GPIO0_FUNC_MASK GENMASK(2, 0)
+
+#define AXP192_IO_ENABLED 0x02
+#define AXP192_IO_DISABLED 0x06
+
+#define AXP192_WORKMODE_DCDC1_MASK BIT_MASK(3)
+#define AXP192_WORKMODE_DCDC2_MASK BIT_MASK(2)
+#define AXP192_WORKMODE_DCDC3_MASK BIT_MASK(1)
+
+#define AXP192_DCDC1_V_OUT_MASK GENMASK(6, 0)
+#define AXP192_DCDC2_V_OUT_MASK GENMASK(5, 0)
+#define AXP192_DCDC3_V_OUT_MASK GENMASK(6, 0)
+#define AXP192_LDO2_V_OUT_MASK GENMASK(7, 4)
+#define AXP192_LDO3_V_OUT_MASK GENMASK(3, 0)
+#define AXP192_LDO_IO0_V_OUT_MASK GENMASK(7, 4)
+
+#define AXP192_PWR_OUT_EXTEN_MASK BIT_MASK(6)
+#define AXP192_PWR_OUT_DCDC2_MASK BIT_MASK(4)
+#define AXP192_PWR_OUT_LDO3_MASK BIT_MASK(3)
+#define AXP192_PWR_OUT_LDO2_MASK BIT_MASK(2)
+#define AXP192_PWR_OUT_DCDC3_MASK BIT_MASK(1)
+#define AXP192_PWR_OUT_DCDC1_MASK BIT_MASK(0)
+
#define AXP20X_GPIO0_FUNC_MASK GENMASK(3, 0)
#define AXP20X_GPIO1_FUNC_MASK GENMASK(3, 0)
@@ -375,25 +398,32 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
switch (axp20x->variant) {
case AXP209_ID:
- if (id == AXP20X_DCDC2) {
+ if (id == AXP20X_LDO3) {
slew_rates = axp209_dcdc2_ldo3_slew_rates;
rate_count = ARRAY_SIZE(axp209_dcdc2_ldo3_slew_rates);
reg = AXP20X_DCDC2_LDO3_V_RAMP;
- mask = AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_RATE_MASK |
- AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN_MASK;
+ mask = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE_MASK |
+ AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN_MASK;
enable = (ramp > 0) ?
- AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN : 0;
+ AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN : 0;
break;
}
- if (id == AXP20X_LDO3) {
+ fallthrough;
+
+ case AXP192_ID:
+ /*
+ * AXP192 and AXP209 share the same DCDC2 ramp configuration
+ */
+ if ((axp20x->variant == AXP209_ID && id == AXP20X_DCDC2) ||
+ (axp20x->variant == AXP192_ID && id == AXP192_DCDC2)) {
slew_rates = axp209_dcdc2_ldo3_slew_rates;
rate_count = ARRAY_SIZE(axp209_dcdc2_ldo3_slew_rates);
reg = AXP20X_DCDC2_LDO3_V_RAMP;
- mask = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE_MASK |
- AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN_MASK;
+ mask = AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_RATE_MASK |
+ AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN_MASK;
enable = (ramp > 0) ?
- AXP20X_DCDC2_LDO3_V_RAMP_LDO3_EN : 0;
+ AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_EN : 0;
break;
}
@@ -401,6 +431,7 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
break;
fallthrough;
+
default:
/* Not supported for this regulator */
return -ENOTSUPP;
@@ -415,7 +446,8 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
if (ramp > slew_rates[i])
break;
- if (id == AXP20X_DCDC2)
+ if ((axp20x->variant == AXP209_ID && id == AXP20X_DCDC2) ||
+ (axp20x->variant == AXP192_ID && id == AXP192_DCDC2))
cfg = AXP20X_DCDC2_LDO3_V_RAMP_DCDC2_RATE(i);
else
cfg = AXP20X_DCDC2_LDO3_V_RAMP_LDO3_RATE(i);
@@ -511,6 +543,29 @@ static const struct regulator_ops axp20x_ops_sw = {
.is_enabled = regulator_is_enabled_regmap,
};
+static const struct regulator_desc axp192_regulators[] = {
+ AXP_DESC(AXP192, DCDC1, "dcdc1", "vin1", 700, 3500, 25,
+ AXP192_DCDC1_V_OUT, AXP192_DCDC1_V_OUT_MASK,
+ AXP192_PWR_OUT_CTRL, AXP192_PWR_OUT_DCDC1_MASK),
+ AXP_DESC(AXP192, DCDC2, "dcdc2", "vin2", 700, 2275, 25,
+ AXP192_DCDC2_V_OUT, AXP192_DCDC2_V_OUT_MASK,
+ AXP192_PWR_OUT_CTRL, AXP192_PWR_OUT_DCDC2_MASK),
+ AXP_DESC(AXP192, DCDC3, "dcdc3", "vin3", 700, 3500, 25,
+ AXP192_DCDC3_V_OUT, AXP192_DCDC3_V_OUT_MASK,
+ AXP192_PWR_OUT_CTRL, AXP192_PWR_OUT_DCDC3_MASK),
+ AXP_DESC_FIXED(AXP192, LDO1, "ldo1", "acin", 1250),
+ AXP_DESC(AXP192, LDO2, "ldo2", "ldoin", 1800, 3300, 100,
+ AXP192_LDO2_3_V_OUT, AXP192_LDO2_V_OUT_MASK,
+ AXP192_PWR_OUT_CTRL, AXP192_PWR_OUT_LDO2_MASK),
+ AXP_DESC(AXP192, LDO3, "ldo3", "ldoin", 1800, 3300, 100,
+ AXP192_LDO2_3_V_OUT, AXP192_LDO3_V_OUT_MASK,
+ AXP192_PWR_OUT_CTRL, AXP192_PWR_OUT_LDO3_MASK),
+ AXP_DESC_IO(AXP192, LDO_IO0, "ldo_io0", "ips", 700, 3300, 100,
+ AXP192_LDO_IO0_V_OUT, AXP192_LDO_IO0_V_OUT_MASK,
+ AXP192_GPIO0_CTRL, AXP192_GPIO0_FUNC_MASK,
+ AXP192_IO_ENABLED, AXP192_IO_DISABLED),
+};
+
static const struct linear_range axp20x_ldo4_ranges[] = {
REGULATOR_LINEAR_RANGE(1250000,
AXP20X_LDO4_V_OUT_1250mV_START,
@@ -1008,6 +1063,12 @@ static int axp20x_set_dcdc_freq(struct platform_device *pdev, u32 dcdcfreq)
u32 min, max, def, step;
switch (axp20x->variant) {
+ case AXP192_ID:
+ min = 900;
+ max = 2025;
+ def = 1500;
+ step = 75;
+ break;
case AXP202_ID:
case AXP209_ID:
min = 750;
@@ -1100,6 +1161,24 @@ static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 work
unsigned int mask;
switch (axp20x->variant) {
+ case AXP192_ID:
+ switch (id) {
+ case AXP192_DCDC1:
+ mask = AXP192_WORKMODE_DCDC1_MASK;
+ break;
+ case AXP192_DCDC2:
+ mask = AXP192_WORKMODE_DCDC2_MASK;
+ break;
+ case AXP192_DCDC3:
+ mask = AXP192_WORKMODE_DCDC3_MASK;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ workmode <<= ffs(mask) - 1;
+ break;
+
case AXP202_ID:
case AXP209_ID:
if ((id != AXP20X_DCDC2) && (id != AXP20X_DCDC3))
@@ -1220,6 +1299,10 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
bool drivevbus = false;
switch (axp20x->variant) {
+ case AXP192_ID:
+ regulators = axp192_regulators;
+ nregulators = AXP192_REG_ID_MAX;
+ break;
case AXP202_ID:
case AXP209_ID:
regulators = axp20x_regulators;
--
2.35.1
The AXP192 PMIC's GPIO registers are much different from the GPIO
registers of the AXP20x and AXP813 PMICs supported by the existing
pinctrl-axp209 driver. It makes more sense to add a new driver for
the AXP192, rather than add support in the existing axp20x driver.
The pinctrl-axp192 driver is considerably more flexible in terms of
register layout and should be able to support other X-Powers PMICs.
Interrupts and pull down resistor configuration are supported too.
Signed-off-by: Aidan MacDonald <[email protected]>
---
drivers/pinctrl/Kconfig | 14 +
drivers/pinctrl/Makefile | 1 +
drivers/pinctrl/pinctrl-axp192.c | 562 +++++++++++++++++++++++++++++++
3 files changed, 577 insertions(+)
create mode 100644 drivers/pinctrl/pinctrl-axp192.c
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index f52960d2dfbe..a71e35de333d 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -113,6 +113,20 @@ config PINCTRL_AT91PIO4
Say Y here to enable the at91 pinctrl/gpio driver for Atmel PIO4
controller available on sama5d2 SoC.
+config PINCTRL_AXP192
+ tristate "X-Powers AXP192 PMIC pinctrl and GPIO Support"
+ depends on MFD_AXP20X
+ depends on OF
+ select PINMUX
+ select GENERIC_PINCONF
+ select GPIOLIB
+ help
+ AXP PMICs provides multiple GPIOs that can be muxed for different
+ functions. This driver bundles a pinctrl driver to select the function
+ muxing and a GPIO driver to handle the GPIO when the GPIO function is
+ selected.
+ Say Y to enable pinctrl and GPIO support for the AXP192 PMIC.
+
config PINCTRL_AXP209
tristate "X-Powers AXP209 PMIC pinctrl and GPIO Support"
depends on MFD_AXP20X
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index e76f5cdc64b0..9d2b6420c5dd 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_PINCTRL_ARTPEC6) += pinctrl-artpec6.o
obj-$(CONFIG_PINCTRL_AS3722) += pinctrl-as3722.o
obj-$(CONFIG_PINCTRL_AT91) += pinctrl-at91.o
obj-$(CONFIG_PINCTRL_AT91PIO4) += pinctrl-at91-pio4.o
+obj-$(CONFIG_PINCTRL_AXP192) += pinctrl-axp192.o
obj-$(CONFIG_PINCTRL_AXP209) += pinctrl-axp209.o
obj-$(CONFIG_PINCTRL_BM1880) += pinctrl-bm1880.o
obj-$(CONFIG_PINCTRL_DA850_PUPD) += pinctrl-da850-pupd.o
diff --git a/drivers/pinctrl/pinctrl-axp192.c b/drivers/pinctrl/pinctrl-axp192.c
new file mode 100644
index 000000000000..6a920258662d
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-axp192.c
@@ -0,0 +1,562 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AXP192 pinctrl and GPIO driver
+ *
+ * Copyright (C) 2022 Aidan MacDonald <[email protected]>
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/limits.h>
+#include <linux/mfd/axp20x.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+enum {
+ AXP192_FUNC_OUTPUT = 0,
+ AXP192_FUNC_INPUT,
+ AXP192_FUNC_LDO,
+ AXP192_FUNC_PWM,
+ AXP192_FUNC_ADC,
+ AXP192_FUNC_LOW_OUTPUT,
+ AXP192_FUNC_FLOATING,
+ AXP192_FUNC_EXT_CHG_CTL,
+ AXP192_FUNC_LDO_STATUS,
+ AXP192_FUNCS_NB,
+};
+
+struct axp192_pctl_function {
+ const char *name;
+ /* Mux value written to the control register to select the function (-1 if unsupported) */
+ const u8 *muxvals;
+ const char * const *groups;
+ unsigned int ngroups;
+};
+
+struct axp192_pctl_reg_info {
+ u8 reg;
+ u8 mask;
+};
+
+struct axp192_pctl_desc {
+ unsigned int npins;
+ const struct pinctrl_pin_desc *pins;
+ /* Description of the function control register for each pin */
+ const struct axp192_pctl_reg_info *ctrl_regs;
+ /* Description of the output signal register for each pin */
+ const struct axp192_pctl_reg_info *out_regs;
+ /* Description of the input signal register for each pin */
+ const struct axp192_pctl_reg_info *in_regs;
+ /* Description of the pull down resistor config register for each pin */
+ const struct axp192_pctl_reg_info *pull_down_regs;
+
+ unsigned int nfunctions;
+ const struct axp192_pctl_function *functions;
+};
+
+static const struct pinctrl_pin_desc axp192_pins[] = {
+ PINCTRL_PIN(0, "GPIO0"),
+ PINCTRL_PIN(1, "GPIO1"),
+ PINCTRL_PIN(2, "GPIO2"),
+ PINCTRL_PIN(3, "GPIO3"),
+ PINCTRL_PIN(4, "GPIO4"),
+ PINCTRL_PIN(5, "N_RSTO"),
+};
+
+static const char * const axp192_io_groups[] = { "GPIO0", "GPIO1", "GPIO2",
+ "GPIO3", "GPIO4", "N_RSTO" };
+static const char * const axp192_ldo_groups[] = { "GPIO0" };
+static const char * const axp192_pwm_groups[] = { "GPIO1", "GPIO2" };
+static const char * const axp192_adc_groups[] = { "GPIO0", "GPIO1", "GPIO2", "GPIO3" };
+static const char * const axp192_extended_io_groups[] = { "GPIO0", "GPIO1", "GPIO2" };
+static const char * const axp192_ext_chg_ctl_groups[] = { "GPIO3", "GPIO4" };
+static const char * const axp192_ldo_status_groups[] = { "N_RSTO" };
+
+static const u8 axp192_output_muxvals[] = { 0, 0, 0, 1, 1, 2 };
+static const u8 axp192_input_muxvals[] = { 1, 1, 1, 2, 2, 3 };
+static const u8 axp192_ldo_muxvals[] = { 2, -1, -1, -1, -1, -1 };
+static const u8 axp192_pwm_muxvals[] = { -1, 2, 2, -1, -1, -1 };
+static const u8 axp192_adc_muxvals[] = { 4, 4, 4, 3, -1, -1 };
+static const u8 axp192_low_output_muxvals[] = { 5, 5, 5, -1, -1, -1 };
+static const u8 axp192_floating_muxvals[] = { 6, 6, 6, -1, -1, -1 };
+static const u8 axp192_ext_chg_ctl_muxvals[] = { -1, -1, -1, 0, 0, -1 };
+static const u8 axp192_ldo_status_muxvals[] = { -1, -1, -1, -1, -1, 0 };
+
+static const struct axp192_pctl_function axp192_functions[AXP192_FUNCS_NB] = {
+ [AXP192_FUNC_OUTPUT] = {
+ .name = "output",
+ .muxvals = axp192_output_muxvals,
+ .groups = axp192_io_groups,
+ .ngroups = ARRAY_SIZE(axp192_io_groups),
+ },
+ [AXP192_FUNC_INPUT] = {
+ .name = "input",
+ .muxvals = axp192_input_muxvals,
+ .groups = axp192_io_groups,
+ .ngroups = ARRAY_SIZE(axp192_io_groups),
+ },
+ [AXP192_FUNC_LDO] = {
+ .name = "ldo",
+ .muxvals = axp192_ldo_muxvals,
+ .groups = axp192_ldo_groups,
+ .ngroups = ARRAY_SIZE(axp192_ldo_groups),
+ },
+ [AXP192_FUNC_PWM] = {
+ .name = "pwm",
+ .muxvals = axp192_pwm_muxvals,
+ .groups = axp192_pwm_groups,
+ .ngroups = ARRAY_SIZE(axp192_pwm_groups),
+ },
+ [AXP192_FUNC_ADC] = {
+ .name = "adc",
+ .muxvals = axp192_adc_muxvals,
+ .groups = axp192_adc_groups,
+ .ngroups = ARRAY_SIZE(axp192_adc_groups),
+ },
+ [AXP192_FUNC_LOW_OUTPUT] = {
+ .name = "low_output",
+ .muxvals = axp192_low_output_muxvals,
+ .groups = axp192_extended_io_groups,
+ .ngroups = ARRAY_SIZE(axp192_extended_io_groups),
+ },
+ [AXP192_FUNC_FLOATING] = {
+ .name = "floating",
+ .muxvals = axp192_floating_muxvals,
+ .groups = axp192_extended_io_groups,
+ .ngroups = ARRAY_SIZE(axp192_extended_io_groups),
+ },
+ [AXP192_FUNC_EXT_CHG_CTL] = {
+ .name = "ext_chg_ctl",
+ .muxvals = axp192_ext_chg_ctl_muxvals,
+ .groups = axp192_ext_chg_ctl_groups,
+ .ngroups = ARRAY_SIZE(axp192_ext_chg_ctl_groups),
+ },
+ [AXP192_FUNC_LDO_STATUS] = {
+ .name = "ldo_status",
+ .muxvals = axp192_ldo_status_muxvals,
+ .groups = axp192_ldo_groups,
+ .ngroups = ARRAY_SIZE(axp192_ldo_status_groups),
+ },
+};
+
+static const struct axp192_pctl_reg_info axp192_pin_ctrl_regs[] = {
+ { .reg = AXP192_GPIO0_CTRL, .mask = GENMASK(2, 0) },
+ { .reg = AXP192_GPIO1_CTRL, .mask = GENMASK(2, 0) },
+ { .reg = AXP192_GPIO2_CTRL, .mask = GENMASK(2, 0) },
+ { .reg = AXP192_GPIO4_3_CTRL, .mask = GENMASK(1, 0) },
+ { .reg = AXP192_GPIO4_3_CTRL, .mask = GENMASK(3, 2) },
+ { .reg = AXP192_N_RSTO_CTRL, .mask = GENMASK(7, 6) },
+};
+
+static const struct axp192_pctl_reg_info axp192_pin_in_regs[] = {
+ { .reg = AXP192_GPIO2_0_STATE, .mask = BIT(4) },
+ { .reg = AXP192_GPIO2_0_STATE, .mask = BIT(5) },
+ { .reg = AXP192_GPIO2_0_STATE, .mask = BIT(6) },
+ { .reg = AXP192_GPIO4_3_STATE, .mask = BIT(4) },
+ { .reg = AXP192_GPIO4_3_STATE, .mask = BIT(5) },
+ { .reg = AXP192_N_RSTO_CTRL, .mask = BIT(4) },
+};
+
+static const struct axp192_pctl_reg_info axp192_pin_out_regs[] = {
+ { .reg = AXP192_GPIO2_0_STATE, .mask = BIT(0) },
+ { .reg = AXP192_GPIO2_0_STATE, .mask = BIT(1) },
+ { .reg = AXP192_GPIO2_0_STATE, .mask = BIT(2) },
+ { .reg = AXP192_GPIO4_3_STATE, .mask = BIT(0) },
+ { .reg = AXP192_GPIO4_3_STATE, .mask = BIT(1) },
+ { .reg = AXP192_N_RSTO_CTRL, .mask = BIT(5) },
+};
+
+static const struct axp192_pctl_reg_info axp192_pull_down_regs[] = {
+ { .reg = AXP192_GPIO2_0_PULL, .mask = BIT(0) },
+ { .reg = AXP192_GPIO2_0_PULL, .mask = BIT(1) },
+ { .reg = AXP192_GPIO2_0_PULL, .mask = BIT(2) },
+ { .reg = 0, .mask = 0 /* unsupported */ },
+ { .reg = 0, .mask = 0 /* unsupported */ },
+ { .reg = 0, .mask = 0 /* unsupported */ },
+};
+
+static const struct axp192_pctl_desc axp192_data = {
+ .npins = ARRAY_SIZE(axp192_pins),
+ .pins = axp192_pins,
+ .ctrl_regs = axp192_pin_ctrl_regs,
+ .out_regs = axp192_pin_out_regs,
+ .in_regs = axp192_pin_in_regs,
+ .pull_down_regs = axp192_pull_down_regs,
+
+ .nfunctions = ARRAY_SIZE(axp192_functions),
+ .functions = axp192_functions,
+};
+
+
+struct axp192_pctl {
+ struct gpio_chip chip;
+ struct regmap *regmap;
+ struct regmap_irq_chip_data *regmap_irqc;
+ struct pinctrl_dev *pctl_dev;
+ struct device *dev;
+ const struct axp192_pctl_desc *desc;
+ int *irqs;
+};
+
+static int axp192_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct axp192_pctl *pctl = gpiochip_get_data(chip);
+ const struct axp192_pctl_reg_info *reginfo = &pctl->desc->in_regs[offset];
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(pctl->regmap, reginfo->reg, &val);
+ if (ret)
+ return ret;
+
+ return !!(val & reginfo->mask);
+}
+
+static int axp192_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+ struct axp192_pctl *pctl = gpiochip_get_data(chip);
+ const struct axp192_pctl_reg_info *reginfo = &pctl->desc->ctrl_regs[offset];
+ const u8 *input_muxvals = pctl->desc->functions[AXP192_FUNC_INPUT].muxvals;
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(pctl->regmap, reginfo->reg, &val);
+ if (ret)
+ return ret;
+
+ if ((val & reginfo->mask) == (input_muxvals[offset] << (ffs(reginfo->mask) - 1)))
+ return GPIO_LINE_DIRECTION_IN;
+
+ return GPIO_LINE_DIRECTION_OUT;
+}
+
+static void axp192_gpio_set(struct gpio_chip *chip, unsigned int offset, int value)
+{
+ struct axp192_pctl *pctl = gpiochip_get_data(chip);
+ const struct axp192_pctl_reg_info *reginfo = &pctl->desc->out_regs[offset];
+
+ regmap_update_bits(pctl->regmap, reginfo->reg, reginfo->mask, value ? reginfo->mask : 0);
+}
+
+static int axp192_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+ return pinctrl_gpio_direction_input(chip->base + offset);
+}
+
+static int axp192_gpio_direction_output(struct gpio_chip *chip, unsigned int offset, int value)
+{
+ chip->set(chip, offset, value);
+ return 0;
+}
+
+static int axp192_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
+{
+ struct axp192_pctl *pctl = gpiochip_get_data(chip);
+
+ /* GPIO IRQs are optional */
+ if (!pctl->irqs[offset])
+ return 0;
+
+ return regmap_irq_get_virq(pctl->regmap_irqc, pctl->irqs[offset]);
+}
+
+static int axp192_pinconf_get_pull_down(struct pinctrl_dev *pctldev, unsigned int pin)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ const struct axp192_pctl_reg_info *reginfo = &pctl->desc->pull_down_regs[pin];
+ unsigned int val;
+ int ret;
+
+ if (!reginfo->mask)
+ return -ENOTSUPP;
+
+ ret = regmap_read(pctl->regmap, reginfo->reg, &val);
+ if (ret)
+ return ret;
+
+ return !!(val & reginfo->mask);
+}
+
+static int axp192_pinconf_set_pull_down(struct pinctrl_dev *pctldev, unsigned int pin, int value)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ const struct axp192_pctl_reg_info *reginfo = &pctl->desc->pull_down_regs[pin];
+
+ if (!reginfo->mask)
+ return -ENOTSUPP;
+
+ return regmap_update_bits(pctl->regmap, reginfo->reg, reginfo->mask,
+ value ? reginfo->mask : 0);
+}
+
+static int axp192_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin, unsigned long *config)
+{
+ enum pin_config_param param = pinconf_to_config_param(*config);
+ unsigned int arg = 1;
+ int ret;
+
+ switch (param) {
+ case PIN_CONFIG_BIAS_DISABLE:
+ ret = axp192_pinconf_get_pull_down(pctldev, pin);
+ if (ret < 0)
+ return ret;
+ else if (ret != 0)
+ return -EINVAL;
+ break;
+
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ ret = axp192_pinconf_get_pull_down(pctldev, pin);
+ if (ret < 0)
+ return ret;
+ else if (ret == 0)
+ return -EINVAL;
+ break;
+
+ default:
+ return -ENOTSUPP;
+ }
+
+ *config = pinconf_to_config_packed(param, arg);
+ return 0;
+}
+
+static int axp192_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
+ unsigned long *configs, unsigned int num_configs)
+{
+ int ret;
+ unsigned int cfg;
+
+ for (cfg = 0; cfg < num_configs; ++cfg) {
+ switch (pinconf_to_config_param(configs[cfg])) {
+ case PIN_CONFIG_BIAS_DISABLE:
+ ret = axp192_pinconf_set_pull_down(pctldev, pin, 0);
+ if (ret)
+ return ret;
+ break;
+
+ case PIN_CONFIG_BIAS_PULL_DOWN:
+ ret = axp192_pinconf_set_pull_down(pctldev, pin, 1);
+ if (ret)
+ return ret;
+ break;
+
+ default:
+ return -ENOTSUPP;
+ }
+ }
+
+ return 0;
+}
+
+static const struct pinconf_ops axp192_conf_ops = {
+ .is_generic = true,
+ .pin_config_get = axp192_pinconf_get,
+ .pin_config_set = axp192_pinconf_set,
+ .pin_config_group_get = axp192_pinconf_get,
+ .pin_config_group_set = axp192_pinconf_set,
+};
+
+static int axp192_pmx_set(struct pinctrl_dev *pctldev, unsigned int offset, u8 config)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ const struct axp192_pctl_reg_info *reginfo = &pctl->desc->ctrl_regs[offset];
+ unsigned int regval = config << (ffs(reginfo->mask) - 1);
+
+ return regmap_update_bits(pctl->regmap, reginfo->reg, reginfo->mask, regval);
+}
+
+static int axp192_pmx_func_cnt(struct pinctrl_dev *pctldev)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->desc->nfunctions;
+}
+
+static const char *axp192_pmx_func_name(struct pinctrl_dev *pctldev, unsigned int selector)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->desc->functions[selector].name;
+}
+
+static int axp192_pmx_func_groups(struct pinctrl_dev *pctldev, unsigned int selector,
+ const char * const **groups, unsigned int *num_groups)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ *groups = pctl->desc->functions[selector].groups;
+ *num_groups = pctl->desc->functions[selector].ngroups;
+
+ return 0;
+}
+
+static int axp192_pmx_set_mux(struct pinctrl_dev *pctldev,
+ unsigned int function, unsigned int group)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ const u8 *muxvals = pctl->desc->functions[function].muxvals;
+
+ if (muxvals[group] == U8_MAX)
+ return -EINVAL;
+
+ /*
+ * Switching to LDO or PWM function will enable LDO/PWM output, so it's
+ * better to ignore these requests and let the regulator or PWM drivers
+ * handle muxing to avoid interfering with them.
+ */
+ if (function == AXP192_FUNC_LDO || function == AXP192_FUNC_PWM)
+ return 0;
+
+ return axp192_pmx_set(pctldev, group, muxvals[group]);
+}
+
+static int axp192_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range *range,
+ unsigned int offset, bool input)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+ const u8 *muxvals = input ? pctl->desc->functions[AXP192_FUNC_INPUT].muxvals
+ : pctl->desc->functions[AXP192_FUNC_OUTPUT].muxvals;
+
+ return axp192_pmx_set(pctldev, offset, muxvals[offset]);
+}
+
+static const struct pinmux_ops axp192_pmx_ops = {
+ .get_functions_count = axp192_pmx_func_cnt,
+ .get_function_name = axp192_pmx_func_name,
+ .get_function_groups = axp192_pmx_func_groups,
+ .set_mux = axp192_pmx_set_mux,
+ .gpio_set_direction = axp192_pmx_gpio_set_direction,
+ .strict = true,
+};
+
+static int axp192_groups_cnt(struct pinctrl_dev *pctldev)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->desc->npins;
+}
+
+static const char *axp192_group_name(struct pinctrl_dev *pctldev, unsigned int selector)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ return pctl->desc->pins[selector].name;
+}
+
+static int axp192_group_pins(struct pinctrl_dev *pctldev, unsigned int selector,
+ const unsigned int **pins, unsigned int *num_pins)
+{
+ struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
+
+ *pins = &pctl->desc->pins[selector].number;
+ *num_pins = 1;
+
+ return 0;
+}
+
+static const struct pinctrl_ops axp192_pctrl_ops = {
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_group,
+ .dt_free_map = pinconf_generic_dt_free_map,
+ .get_groups_count = axp192_groups_cnt,
+ .get_group_name = axp192_group_name,
+ .get_group_pins = axp192_group_pins,
+};
+
+static int axp192_pctl_probe(struct platform_device *pdev)
+{
+ struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
+ struct axp192_pctl *pctl;
+ struct pinctrl_desc *pctrl_desc;
+ int ret, i;
+
+ pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL);
+ if (!pctl)
+ return -ENOMEM;
+
+ pctl->desc = device_get_match_data(&pdev->dev);
+ pctl->regmap = axp20x->regmap;
+ pctl->regmap_irqc = axp20x->regmap_irqc;
+ pctl->dev = &pdev->dev;
+
+ pctl->chip.base = -1;
+ pctl->chip.can_sleep = true;
+ pctl->chip.request = gpiochip_generic_request;
+ pctl->chip.free = gpiochip_generic_free;
+ pctl->chip.parent = &pdev->dev;
+ pctl->chip.label = dev_name(&pdev->dev);
+ pctl->chip.owner = THIS_MODULE;
+ pctl->chip.get = axp192_gpio_get;
+ pctl->chip.get_direction = axp192_gpio_get_direction;
+ pctl->chip.set = axp192_gpio_set;
+ pctl->chip.direction_input = axp192_gpio_direction_input;
+ pctl->chip.direction_output = axp192_gpio_direction_output;
+ pctl->chip.to_irq = axp192_gpio_to_irq;
+ pctl->chip.ngpio = pctl->desc->npins;
+
+ pctl->irqs = devm_kcalloc(&pdev->dev, pctl->desc->npins, sizeof(int), GFP_KERNEL);
+ if (!pctl->irqs)
+ return -ENOMEM;
+
+ for (i = 0; i < pctl->desc->npins; ++i) {
+ ret = platform_get_irq_byname_optional(pdev, pctl->desc->pins[i].name);
+ if (ret > 0)
+ pctl->irqs[i] = ret;
+ }
+
+ platform_set_drvdata(pdev, pctl);
+
+ pctrl_desc = devm_kzalloc(&pdev->dev, sizeof(*pctrl_desc), GFP_KERNEL);
+ if (!pctrl_desc)
+ return -ENOMEM;
+
+ pctrl_desc->name = dev_name(&pdev->dev);
+ pctrl_desc->owner = THIS_MODULE;
+ pctrl_desc->pins = pctl->desc->pins;
+ pctrl_desc->npins = pctl->desc->npins;
+ pctrl_desc->pctlops = &axp192_pctrl_ops;
+ pctrl_desc->pmxops = &axp192_pmx_ops;
+ pctrl_desc->confops = &axp192_conf_ops;
+
+ pctl->pctl_dev = devm_pinctrl_register(&pdev->dev, pctrl_desc, pctl);
+ if (IS_ERR(pctl->pctl_dev))
+ dev_err_probe(&pdev->dev, PTR_ERR(pctl->pctl_dev),
+ "couldn't register pinctrl driver\n");
+
+ ret = devm_gpiochip_add_data(&pdev->dev, &pctl->chip, pctl);
+ if (ret)
+ dev_err_probe(&pdev->dev, ret, "Failed to register GPIO chip\n");
+
+ return 0;
+}
+
+static const struct of_device_id axp192_pctl_match[] = {
+ { .compatible = "x-powers,axp192-gpio", .data = &axp192_data, },
+ { }
+};
+MODULE_DEVICE_TABLE(of, axp192_pctl_match);
+
+static struct platform_driver axp192_pctl_driver = {
+ .probe = axp192_pctl_probe,
+ .driver = {
+ .name = "axp192-gpio",
+ .of_match_table = axp192_pctl_match,
+ },
+};
+module_platform_driver(axp192_pctl_driver);
+
+MODULE_AUTHOR("Aidan MacDonald <[email protected]>");
+MODULE_DESCRIPTION("AXP192 PMIC pinctrl and GPIO driver");
+MODULE_LICENSE("GPL");
--
2.35.1
The AXP192 has a battery charger similar to other X-Powers PMICs,
but unlike the other supported devices, it does not have a fuel
gauge and can't report battery capacity directly.
Acked-by: Sebastian Reichel <[email protected]>
Signed-off-by: Aidan MacDonald <[email protected]>
---
drivers/power/supply/axp20x_battery.c | 49 +++++++++++++++++++++++++--
1 file changed, 46 insertions(+), 3 deletions(-)
diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
index f2c57fd1329b..cd5d6905b2d2 100644
--- a/drivers/power/supply/axp20x_battery.c
+++ b/drivers/power/supply/axp20x_battery.c
@@ -545,6 +545,19 @@ static int axp20x_battery_set_prop(struct power_supply *psy,
}
}
+static enum power_supply_property axp192_battery_props[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CURRENT_NOW,
+ POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
+ POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
+ POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
+ POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+};
+
static enum power_supply_property axp20x_battery_props[] = {
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_ONLINE,
@@ -569,6 +582,16 @@ static int axp20x_battery_prop_writeable(struct power_supply *psy,
psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX;
}
+static const struct power_supply_desc axp192_batt_ps_desc = {
+ .name = "axp192-battery",
+ .type = POWER_SUPPLY_TYPE_BATTERY,
+ .properties = axp192_battery_props,
+ .num_properties = ARRAY_SIZE(axp192_battery_props),
+ .property_is_writeable = axp20x_battery_prop_writeable,
+ .get_property = axp20x_battery_get_prop,
+ .set_property = axp20x_battery_set_prop,
+};
+
static const struct power_supply_desc axp20x_batt_ps_desc = {
.name = "axp20x-battery",
.type = POWER_SUPPLY_TYPE_BATTERY,
@@ -579,6 +602,19 @@ static const struct power_supply_desc axp20x_batt_ps_desc = {
.set_property = axp20x_battery_set_prop,
};
+static const int axp192_ccc_table[AXP20X_CHRG_CTRL1_TGT_CURR+1] = {
+ 100000, 190000, 280000, 360000,
+ 450000, 550000, 630000, 700000,
+ 780000, 880000, 960000, 1000000,
+ 1080000, 1160000, 1240000, 1320000,
+};
+
+static const struct axp_data axp192_data = {
+ .ccc_table = axp192_ccc_table,
+ .get_max_voltage = axp20x_battery_get_max_voltage,
+ .set_max_voltage = axp20x_battery_set_max_voltage,
+};
+
static const struct axp_data axp209_data = {
.ccc_scale = 100000,
.ccc_offset = 300000,
@@ -607,6 +643,9 @@ static const struct axp_data axp813_data = {
static const struct of_device_id axp20x_battery_ps_id[] = {
{
+ .compatible = "x-powers,axp192-battery-power-supply",
+ .data = (void *)&axp192_data,
+ }, {
.compatible = "x-powers,axp209-battery-power-supply",
.data = (void *)&axp209_data,
}, {
@@ -624,6 +663,7 @@ static int axp20x_power_probe(struct platform_device *pdev)
struct axp20x_batt_ps *axp20x_batt;
struct power_supply_config psy_cfg = {};
struct power_supply_battery_info *info;
+ const struct power_supply_desc *ps_desc;
struct device *dev = &pdev->dev;
if (!of_device_is_available(pdev->dev.of_node))
@@ -667,9 +707,12 @@ static int axp20x_power_probe(struct platform_device *pdev)
axp20x_batt->data = (struct axp_data *)of_device_get_match_data(dev);
- axp20x_batt->batt = devm_power_supply_register(&pdev->dev,
- &axp20x_batt_ps_desc,
- &psy_cfg);
+ if (!axp20x_batt->data->has_fg)
+ ps_desc = &axp192_batt_ps_desc;
+ else
+ ps_desc = &axp20x_batt_ps_desc;
+
+ axp20x_batt->batt = devm_power_supply_register(&pdev->dev, ps_desc, &psy_cfg);
if (IS_ERR(axp20x_batt->batt)) {
dev_err(&pdev->dev, "failed to register power supply: %ld\n",
PTR_ERR(axp20x_batt->batt));
--
2.35.1
The AXP192 is identical to the AXP20x, except for two additional
GPIO ADC channels.
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Aidan MacDonald <[email protected]>
---
.../bindings/iio/adc/x-powers,axp209-adc.yaml | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml b/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml
index d6d3d8590171..1a68e650ac7d 100644
--- a/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml
@@ -14,6 +14,23 @@ description: |
Device is a child of an axp209 multifunction device
ADC channels and their indexes per variant:
+ AXP192
+ ------
+ 0 | acin_v
+ 1 | acin_i
+ 2 | vbus_v
+ 3 | vbus_i
+ 4 | pmic_temp
+ 5 | gpio0_v
+ 6 | gpio1_v
+ 7 | gpio2_v
+ 8 | gpio3_v
+ 9 | ipsout_v
+ 10 | batt_v
+ 11 | batt_chrg_i
+ 12 | batt_dischrg_i
+ 13 | ts_v
+
AXP209
------
0 | acin_v
@@ -50,6 +67,7 @@ description: |
properties:
compatible:
oneOf:
+ - const: x-powers,axp192-adc
- const: x-powers,axp209-adc
- const: x-powers,axp221-adc
- const: x-powers,axp813-adc
--
2.35.1
Add a "has_fg" flag to indicate that the chip has a fuel gauge.
Report battery full status on chips with no fuel gauge using the
battery voltage.
Acked-by: Sebastian Reichel <[email protected]>
Signed-off-by: Aidan MacDonald <[email protected]>
---
drivers/power/supply/axp20x_battery.c | 34 ++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/power/supply/axp20x_battery.c b/drivers/power/supply/axp20x_battery.c
index ce22c0a92878..f2c57fd1329b 100644
--- a/drivers/power/supply/axp20x_battery.c
+++ b/drivers/power/supply/axp20x_battery.c
@@ -69,6 +69,7 @@ struct axp_data {
* must have AXP20X_CHRG_CTRL1_TGT_CURR+1 elements.
*/
const int *ccc_table;
+ bool has_fg;
bool has_fg_valid;
int (*get_max_voltage)(struct axp20x_batt_ps *batt, int *val);
int (*set_max_voltage)(struct axp20x_batt_ps *batt, int val);
@@ -197,7 +198,7 @@ static int axp20x_battery_get_prop(struct power_supply *psy,
union power_supply_propval *val)
{
struct axp20x_batt_ps *axp20x_batt = power_supply_get_drvdata(psy);
- int ret = 0, reg, val1;
+ int ret = 0, reg, val1, val2;
switch (psp) {
case POWER_SUPPLY_PROP_PRESENT:
@@ -231,6 +232,34 @@ static int axp20x_battery_get_prop(struct power_supply *psy,
return 0;
}
+ /*
+ * If the chip does not have a fuel gauge, we check battery full status
+ * using voltage instead.
+ */
+ if (!axp20x_batt->data->has_fg) {
+ ret = axp20x_batt->data->get_max_voltage(axp20x_batt, &val1);
+ if (ret)
+ return ret;
+
+ ret = iio_read_channel_processed(axp20x_batt->batt_v, &val2);
+ if (ret)
+ return ret;
+
+ /* IIO subsystem reports voltage in mV but we need uV */
+ val2 *= 1000;
+
+ /*
+ * According to the AXP192 datasheet, charging will restart if
+ * the battery voltage drops below V_rch = V_tgt - 0.1 V, so we
+ * report the battery is full if its voltage is at least V_rch.
+ */
+ if (val2 >= val1 - 100000)
+ val->intval = POWER_SUPPLY_STATUS_FULL;
+ else
+ val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ break;
+ }
+
ret = regmap_read(axp20x_batt->regmap, AXP20X_FG_RES, &val1);
if (ret)
return ret;
@@ -553,6 +582,7 @@ static const struct power_supply_desc axp20x_batt_ps_desc = {
static const struct axp_data axp209_data = {
.ccc_scale = 100000,
.ccc_offset = 300000,
+ .has_fg = true,
.get_max_voltage = axp20x_battery_get_max_voltage,
.set_max_voltage = axp20x_battery_set_max_voltage,
};
@@ -560,6 +590,7 @@ static const struct axp_data axp209_data = {
static const struct axp_data axp221_data = {
.ccc_scale = 150000,
.ccc_offset = 300000,
+ .has_fg = true,
.has_fg_valid = true,
.get_max_voltage = axp22x_battery_get_max_voltage,
.set_max_voltage = axp22x_battery_set_max_voltage,
@@ -568,6 +599,7 @@ static const struct axp_data axp221_data = {
static const struct axp_data axp813_data = {
.ccc_scale = 200000,
.ccc_offset = 200000,
+ .has_fg = true,
.has_fg_valid = true,
.get_max_voltage = axp813_battery_get_max_voltage,
.set_max_voltage = axp20x_battery_set_max_voltage,
--
2.35.1
The AXP192's battery charger is similar to the others supported by
the axp20x_battery driver.
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Aidan MacDonald <[email protected]>
---
.../power/supply/x-powers,axp20x-battery-power-supply.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-battery-power-supply.yaml b/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-battery-power-supply.yaml
index d055428ae39f..b7347683a07e 100644
--- a/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-battery-power-supply.yaml
+++ b/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-battery-power-supply.yaml
@@ -20,6 +20,7 @@ allOf:
properties:
compatible:
oneOf:
+ - const: x-powers,axp192-battery-power-supply
- const: x-powers,axp202-battery-power-supply
- const: x-powers,axp209-battery-power-supply
- const: x-powers,axp221-battery-power-supply
--
2.35.1
The AXP192 PMIC is similar to the AXP202/AXP209, but with different
regulators, additional GPIOs, and a different IRQ register layout.
Signed-off-by: Aidan MacDonald <[email protected]>
---
drivers/mfd/axp20x-i2c.c | 2 +
drivers/mfd/axp20x.c | 153 +++++++++++++++++++++++++++++++++++++
include/linux/mfd/axp20x.h | 84 ++++++++++++++++++++
3 files changed, 239 insertions(+)
diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
index 00ab48018d8d..9ada58fad77f 100644
--- a/drivers/mfd/axp20x-i2c.c
+++ b/drivers/mfd/axp20x-i2c.c
@@ -62,6 +62,7 @@ static int axp20x_i2c_remove(struct i2c_client *i2c)
#ifdef CONFIG_OF
static const struct of_device_id axp20x_i2c_of_match[] = {
{ .compatible = "x-powers,axp152", .data = (void *)AXP152_ID },
+ { .compatible = "x-powers,axp192", .data = (void *)AXP192_ID },
{ .compatible = "x-powers,axp202", .data = (void *)AXP202_ID },
{ .compatible = "x-powers,axp209", .data = (void *)AXP209_ID },
{ .compatible = "x-powers,axp221", .data = (void *)AXP221_ID },
@@ -75,6 +76,7 @@ MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
static const struct i2c_device_id axp20x_i2c_id[] = {
{ "axp152", 0 },
+ { "axp192", 0 },
{ "axp202", 0 },
{ "axp209", 0 },
{ "axp221", 0 },
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 8161a5dc68e8..1011e668589a 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -34,6 +34,7 @@
static const char * const axp20x_model_names[] = {
"AXP152",
+ "AXP192",
"AXP202",
"AXP209",
"AXP221",
@@ -92,6 +93,35 @@ static const struct regmap_access_table axp20x_volatile_table = {
.n_yes_ranges = ARRAY_SIZE(axp20x_volatile_ranges),
};
+static const struct regmap_range axp192_writeable_ranges[] = {
+ regmap_reg_range(AXP192_DATACACHE(0), AXP192_DATACACHE(5)),
+ regmap_reg_range(AXP192_PWR_OUT_CTRL, AXP192_IRQ5_STATE),
+ regmap_reg_range(AXP20X_DCDC_MODE, AXP192_N_RSTO_CTRL),
+ regmap_reg_range(AXP20X_CC_CTRL, AXP20X_CC_CTRL),
+};
+
+static const struct regmap_range axp192_volatile_ranges[] = {
+ regmap_reg_range(AXP20X_PWR_INPUT_STATUS, AXP192_USB_OTG_STATUS),
+ regmap_reg_range(AXP192_IRQ1_STATE, AXP192_IRQ4_STATE),
+ regmap_reg_range(AXP192_IRQ5_STATE, AXP192_IRQ5_STATE),
+ regmap_reg_range(AXP20X_ACIN_V_ADC_H, AXP20X_IPSOUT_V_HIGH_L),
+ regmap_reg_range(AXP20X_TIMER_CTRL, AXP20X_TIMER_CTRL),
+ regmap_reg_range(AXP192_GPIO2_0_STATE, AXP192_GPIO2_0_STATE),
+ regmap_reg_range(AXP192_GPIO4_3_STATE, AXP192_GPIO4_3_STATE),
+ regmap_reg_range(AXP192_N_RSTO_CTRL, AXP192_N_RSTO_CTRL),
+ regmap_reg_range(AXP20X_CHRG_CC_31_24, AXP20X_CC_CTRL),
+};
+
+static const struct regmap_access_table axp192_writeable_table = {
+ .yes_ranges = axp192_writeable_ranges,
+ .n_yes_ranges = ARRAY_SIZE(axp192_writeable_ranges),
+};
+
+static const struct regmap_access_table axp192_volatile_table = {
+ .yes_ranges = axp192_volatile_ranges,
+ .n_yes_ranges = ARRAY_SIZE(axp192_volatile_ranges),
+};
+
/* AXP22x ranges are shared with the AXP809, as they cover the same range */
static const struct regmap_range axp22x_writeable_ranges[] = {
regmap_reg_range(AXP20X_DATACACHE(0), AXP20X_IRQ5_STATE),
@@ -173,6 +203,25 @@ static const struct resource axp152_pek_resources[] = {
DEFINE_RES_IRQ_NAMED(AXP152_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
};
+static const struct resource axp192_gpio_resources[] = {
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_GPIO0_INPUT, "GPIO0"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_GPIO1_INPUT, "GPIO1"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_GPIO2_INPUT, "GPIO2"),
+};
+
+static const struct resource axp192_ac_power_supply_resources[] = {
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_ACIN_PLUGIN, "ACIN_PLUGIN"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_ACIN_REMOVAL, "ACIN_REMOVAL"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_ACIN_OVER_V, "ACIN_OVER_V"),
+};
+
+static const struct resource axp192_usb_power_supply_resources[] = {
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_PLUGIN, "VBUS_PLUGIN"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_REMOVAL, "VBUS_REMOVAL"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_VALID, "VBUS_VALID"),
+ DEFINE_RES_IRQ_NAMED(AXP192_IRQ_VBUS_NOT_VALID, "VBUS_NOT_VALID"),
+};
+
static const struct resource axp20x_ac_power_supply_resources[] = {
DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_ACIN_PLUGIN, "ACIN_PLUGIN"),
DEFINE_RES_IRQ_NAMED(AXP20X_IRQ_ACIN_REMOVAL, "ACIN_REMOVAL"),
@@ -245,6 +294,15 @@ static const struct regmap_config axp152_regmap_config = {
.cache_type = REGCACHE_RBTREE,
};
+static const struct regmap_config axp192_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .wr_table = &axp192_writeable_table,
+ .volatile_table = &axp192_volatile_table,
+ .max_register = AXP20X_CC_CTRL,
+ .cache_type = REGCACHE_RBTREE,
+};
+
static const struct regmap_config axp20x_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
@@ -304,6 +362,55 @@ static const struct regmap_irq axp152_regmap_irqs[] = {
INIT_REGMAP_IRQ(AXP152, GPIO0_INPUT, 2, 0),
};
+static const struct regmap_irq axp192_regmap_irqs[] = {
+ INIT_REGMAP_IRQ(AXP192, ACIN_OVER_V, 0, 7),
+ INIT_REGMAP_IRQ(AXP192, ACIN_PLUGIN, 0, 6),
+ INIT_REGMAP_IRQ(AXP192, ACIN_REMOVAL, 0, 5),
+ INIT_REGMAP_IRQ(AXP192, VBUS_OVER_V, 0, 4),
+ INIT_REGMAP_IRQ(AXP192, VBUS_PLUGIN, 0, 3),
+ INIT_REGMAP_IRQ(AXP192, VBUS_REMOVAL, 0, 2),
+ INIT_REGMAP_IRQ(AXP192, VBUS_V_LOW, 0, 1),
+ INIT_REGMAP_IRQ(AXP192, BATT_PLUGIN, 1, 7),
+ INIT_REGMAP_IRQ(AXP192, BATT_REMOVAL, 1, 6),
+ INIT_REGMAP_IRQ(AXP192, BATT_ENT_ACT_MODE, 1, 5),
+ INIT_REGMAP_IRQ(AXP192, BATT_EXIT_ACT_MODE, 1, 4),
+ INIT_REGMAP_IRQ(AXP192, CHARG, 1, 3),
+ INIT_REGMAP_IRQ(AXP192, CHARG_DONE, 1, 2),
+ INIT_REGMAP_IRQ(AXP192, BATT_TEMP_HIGH, 1, 1),
+ INIT_REGMAP_IRQ(AXP192, BATT_TEMP_LOW, 1, 0),
+ INIT_REGMAP_IRQ(AXP192, DIE_TEMP_HIGH, 2, 7),
+ INIT_REGMAP_IRQ(AXP192, CHARG_I_LOW, 2, 6),
+ INIT_REGMAP_IRQ(AXP192, DCDC1_V_LONG, 2, 5),
+ INIT_REGMAP_IRQ(AXP192, DCDC2_V_LONG, 2, 4),
+ INIT_REGMAP_IRQ(AXP192, DCDC3_V_LONG, 2, 3),
+ INIT_REGMAP_IRQ(AXP192, PEK_SHORT, 2, 1),
+ INIT_REGMAP_IRQ(AXP192, PEK_LONG, 2, 0),
+ INIT_REGMAP_IRQ(AXP192, N_OE_PWR_ON, 3, 7),
+ INIT_REGMAP_IRQ(AXP192, N_OE_PWR_OFF, 3, 6),
+ INIT_REGMAP_IRQ(AXP192, VBUS_VALID, 3, 5),
+ INIT_REGMAP_IRQ(AXP192, VBUS_NOT_VALID, 3, 4),
+ INIT_REGMAP_IRQ(AXP192, VBUS_SESS_VALID, 3, 3),
+ INIT_REGMAP_IRQ(AXP192, VBUS_SESS_END, 3, 2),
+ INIT_REGMAP_IRQ(AXP192, LOW_PWR_LVL, 3, 0),
+ INIT_REGMAP_IRQ(AXP192, TIMER, 4, 7),
+ INIT_REGMAP_IRQ(AXP192, GPIO2_INPUT, 4, 2),
+ INIT_REGMAP_IRQ(AXP192, GPIO1_INPUT, 4, 1),
+ INIT_REGMAP_IRQ(AXP192, GPIO0_INPUT, 4, 0),
+};
+
+static int axp192_get_irq_reg(unsigned int base_reg, int i)
+{
+ /* linear mapping for IRQ1 to IRQ4 */
+ if (i < 4)
+ return base_reg + i;
+
+ /* handle IRQ5 separately */
+ if (base_reg == AXP192_IRQ1_EN)
+ return AXP192_IRQ5_EN;
+ else
+ return AXP192_IRQ5_STATE;
+}
+
static const struct regmap_irq axp20x_regmap_irqs[] = {
INIT_REGMAP_IRQ(AXP20X, ACIN_OVER_V, 0, 7),
INIT_REGMAP_IRQ(AXP20X, ACIN_PLUGIN, 0, 6),
@@ -514,6 +621,19 @@ static const struct regmap_irq_chip axp152_regmap_irq_chip = {
.num_regs = 3,
};
+static const struct regmap_irq_chip axp192_regmap_irq_chip = {
+ .name = "axp192_irq_chip",
+ .status_base = AXP192_IRQ1_STATE,
+ .ack_base = AXP192_IRQ1_STATE,
+ .mask_base = AXP192_IRQ1_EN,
+ .mask_invert = true,
+ .init_ack_masked = true,
+ .irqs = axp192_regmap_irqs,
+ .num_irqs = ARRAY_SIZE(axp192_regmap_irqs),
+ .num_regs = 5,
+ .get_irq_reg = axp192_get_irq_reg,
+};
+
static const struct regmap_irq_chip axp20x_regmap_irq_chip = {
.name = "axp20x_irq_chip",
.status_base = AXP20X_IRQ1_STATE,
@@ -588,6 +708,33 @@ static const struct regmap_irq_chip axp809_regmap_irq_chip = {
.num_regs = 5,
};
+static const struct mfd_cell axp192_cells[] = {
+ {
+ .name = "axp192-gpio",
+ .of_compatible = "x-powers,axp192-gpio",
+ .num_resources = ARRAY_SIZE(axp192_gpio_resources),
+ .resources = axp192_gpio_resources,
+ }, {
+ .name = "axp20x-regulator",
+ }, {
+ .name = "axp192-adc",
+ .of_compatible = "x-powers,axp192-adc",
+ }, {
+ .name = "axp20x-battery-power-supply",
+ .of_compatible = "x-powers,axp192-battery-power-supply",
+ }, {
+ .name = "axp20x-ac-power-supply",
+ .of_compatible = "x-powers,axp202-ac-power-supply",
+ .num_resources = ARRAY_SIZE(axp192_ac_power_supply_resources),
+ .resources = axp192_ac_power_supply_resources,
+ }, {
+ .name = "axp20x-usb-power-supply",
+ .of_compatible = "x-powers,axp192-usb-power-supply",
+ .num_resources = ARRAY_SIZE(axp192_usb_power_supply_resources),
+ .resources = axp192_usb_power_supply_resources,
+ }
+};
+
static const struct mfd_cell axp20x_cells[] = {
{
.name = "axp20x-gpio",
@@ -865,6 +1012,12 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
axp20x->regmap_cfg = &axp152_regmap_config;
axp20x->regmap_irq_chip = &axp152_regmap_irq_chip;
break;
+ case AXP192_ID:
+ axp20x->nr_cells = ARRAY_SIZE(axp192_cells);
+ axp20x->cells = axp192_cells;
+ axp20x->regmap_cfg = &axp192_regmap_config;
+ axp20x->regmap_irq_chip = &axp192_regmap_irq_chip;
+ break;
case AXP202_ID:
case AXP209_ID:
axp20x->nr_cells = ARRAY_SIZE(axp20x_cells);
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 9ab0e2fca7ea..18546e124919 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -12,6 +12,7 @@
enum axp20x_variants {
AXP152_ID = 0,
+ AXP192_ID,
AXP202_ID,
AXP209_ID,
AXP221_ID,
@@ -24,6 +25,7 @@ enum axp20x_variants {
NR_AXP20X_VARIANTS,
};
+#define AXP192_DATACACHE(m) (0x06 + (m))
#define AXP20X_DATACACHE(m) (0x04 + (m))
/* Power supply */
@@ -45,6 +47,13 @@ enum axp20x_variants {
#define AXP152_DCDC_FREQ 0x37
#define AXP152_DCDC_MODE 0x80
+#define AXP192_USB_OTG_STATUS 0x04
+#define AXP192_PWR_OUT_CTRL 0x12
+#define AXP192_DCDC2_V_OUT 0x23
+#define AXP192_DCDC1_V_OUT 0x26
+#define AXP192_DCDC3_V_OUT 0x27
+#define AXP192_LDO2_3_V_OUT 0x28
+
#define AXP20X_PWR_INPUT_STATUS 0x00
#define AXP20X_PWR_OP_MODE 0x01
#define AXP20X_USB_OTG_STATUS 0x02
@@ -139,6 +148,17 @@ enum axp20x_variants {
#define AXP152_IRQ2_STATE 0x49
#define AXP152_IRQ3_STATE 0x4a
+#define AXP192_IRQ1_EN 0x40
+#define AXP192_IRQ2_EN 0x41
+#define AXP192_IRQ3_EN 0x42
+#define AXP192_IRQ4_EN 0x43
+#define AXP192_IRQ1_STATE 0x44
+#define AXP192_IRQ2_STATE 0x45
+#define AXP192_IRQ3_STATE 0x46
+#define AXP192_IRQ4_STATE 0x47
+#define AXP192_IRQ5_EN 0x4a
+#define AXP192_IRQ5_STATE 0x4d
+
#define AXP20X_IRQ1_EN 0x40
#define AXP20X_IRQ2_EN 0x41
#define AXP20X_IRQ3_EN 0x42
@@ -153,6 +173,11 @@ enum axp20x_variants {
#define AXP20X_IRQ6_STATE 0x4d
/* ADC */
+#define AXP192_GPIO2_V_ADC_H 0x68
+#define AXP192_GPIO2_V_ADC_L 0x69
+#define AXP192_GPIO3_V_ADC_H 0x6a
+#define AXP192_GPIO3_V_ADC_L 0x6b
+
#define AXP20X_ACIN_V_ADC_H 0x56
#define AXP20X_ACIN_V_ADC_L 0x57
#define AXP20X_ACIN_I_ADC_H 0x58
@@ -182,6 +207,8 @@ enum axp20x_variants {
#define AXP20X_IPSOUT_V_HIGH_L 0x7f
/* Power supply */
+#define AXP192_GPIO30_IN_RANGE 0x85
+
#define AXP20X_DCDC_MODE 0x80
#define AXP20X_ADC_EN1 0x82
#define AXP20X_ADC_EN2 0x83
@@ -210,6 +237,16 @@ enum axp20x_variants {
#define AXP152_PWM1_FREQ_Y 0x9c
#define AXP152_PWM1_DUTY_CYCLE 0x9d
+#define AXP192_GPIO0_CTRL 0x90
+#define AXP192_LDO_IO0_V_OUT 0x91
+#define AXP192_GPIO1_CTRL 0x92
+#define AXP192_GPIO2_CTRL 0x93
+#define AXP192_GPIO2_0_STATE 0x94
+#define AXP192_GPIO4_3_CTRL 0x95
+#define AXP192_GPIO4_3_STATE 0x96
+#define AXP192_GPIO2_0_PULL 0x97
+#define AXP192_N_RSTO_CTRL 0x9e
+
#define AXP20X_GPIO0_CTRL 0x90
#define AXP20X_LDO5_V_OUT 0x91
#define AXP20X_GPIO1_CTRL 0x92
@@ -287,6 +324,17 @@ enum axp20x_variants {
#define AXP288_FG_TUNE5 0xed
/* Regulators IDs */
+enum {
+ AXP192_DCDC1 = 0,
+ AXP192_DCDC2,
+ AXP192_DCDC3,
+ AXP192_LDO1,
+ AXP192_LDO2,
+ AXP192_LDO3,
+ AXP192_LDO_IO0,
+ AXP192_REG_ID_MAX,
+};
+
enum {
AXP20X_LDO1 = 0,
AXP20X_LDO2,
@@ -440,6 +488,42 @@ enum {
AXP152_IRQ_GPIO0_INPUT,
};
+enum axp192_irqs {
+ AXP192_IRQ_ACIN_OVER_V = 1,
+ AXP192_IRQ_ACIN_PLUGIN,
+ AXP192_IRQ_ACIN_REMOVAL,
+ AXP192_IRQ_VBUS_OVER_V,
+ AXP192_IRQ_VBUS_PLUGIN,
+ AXP192_IRQ_VBUS_REMOVAL,
+ AXP192_IRQ_VBUS_V_LOW,
+ AXP192_IRQ_BATT_PLUGIN,
+ AXP192_IRQ_BATT_REMOVAL,
+ AXP192_IRQ_BATT_ENT_ACT_MODE,
+ AXP192_IRQ_BATT_EXIT_ACT_MODE,
+ AXP192_IRQ_CHARG,
+ AXP192_IRQ_CHARG_DONE,
+ AXP192_IRQ_BATT_TEMP_HIGH,
+ AXP192_IRQ_BATT_TEMP_LOW,
+ AXP192_IRQ_DIE_TEMP_HIGH,
+ AXP192_IRQ_CHARG_I_LOW,
+ AXP192_IRQ_DCDC1_V_LONG,
+ AXP192_IRQ_DCDC2_V_LONG,
+ AXP192_IRQ_DCDC3_V_LONG,
+ AXP192_IRQ_PEK_SHORT = 22,
+ AXP192_IRQ_PEK_LONG,
+ AXP192_IRQ_N_OE_PWR_ON,
+ AXP192_IRQ_N_OE_PWR_OFF,
+ AXP192_IRQ_VBUS_VALID,
+ AXP192_IRQ_VBUS_NOT_VALID,
+ AXP192_IRQ_VBUS_SESS_VALID,
+ AXP192_IRQ_VBUS_SESS_END,
+ AXP192_IRQ_LOW_PWR_LVL = 31,
+ AXP192_IRQ_TIMER,
+ AXP192_IRQ_GPIO2_INPUT = 37,
+ AXP192_IRQ_GPIO1_INPUT,
+ AXP192_IRQ_GPIO0_INPUT,
+};
+
enum {
AXP20X_IRQ_ACIN_OVER_V = 1,
AXP20X_IRQ_ACIN_PLUGIN,
--
2.35.1
The code may be clearer if parameters are not re-purposed to hold
temporary results like register values, so introduce local variables
as necessary to avoid that. Also, use the common FIELD_PREP macro
instead of a hand-rolled version.
Suggested-by: Jonathan Cameron <[email protected]>
Signed-off-by: Aidan MacDonald <[email protected]>
---
drivers/iio/adc/axp20x_adc.c | 61 +++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
index 53bf7d4899d2..041511280e1e 100644
--- a/drivers/iio/adc/axp20x_adc.c
+++ b/drivers/iio/adc/axp20x_adc.c
@@ -15,6 +15,7 @@
#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/thermal.h>
+#include <linux/bitfield.h>
#include <linux/iio/iio.h>
#include <linux/iio/driver.h>
@@ -22,20 +23,20 @@
#include <linux/mfd/axp20x.h>
#define AXP20X_ADC_EN1_MASK GENMASK(7, 0)
-
#define AXP20X_ADC_EN2_MASK (GENMASK(3, 2) | BIT(7))
+
#define AXP22X_ADC_EN1_MASK (GENMASK(7, 5) | BIT(0))
#define AXP20X_GPIO10_IN_RANGE_GPIO0 BIT(0)
#define AXP20X_GPIO10_IN_RANGE_GPIO1 BIT(1)
-#define AXP20X_GPIO10_IN_RANGE_GPIO0_VAL(x) ((x) & BIT(0))
-#define AXP20X_GPIO10_IN_RANGE_GPIO1_VAL(x) (((x) & BIT(0)) << 1)
#define AXP20X_ADC_RATE_MASK GENMASK(7, 6)
-#define AXP813_V_I_ADC_RATE_MASK GENMASK(5, 4)
-#define AXP813_ADC_RATE_MASK (AXP20X_ADC_RATE_MASK | AXP813_V_I_ADC_RATE_MASK)
#define AXP20X_ADC_RATE_HZ(x) ((ilog2((x) / 25) << 6) & AXP20X_ADC_RATE_MASK)
+
#define AXP22X_ADC_RATE_HZ(x) ((ilog2((x) / 100) << 6) & AXP20X_ADC_RATE_MASK)
+
+#define AXP813_V_I_ADC_RATE_MASK GENMASK(5, 4)
+#define AXP813_ADC_RATE_MASK (AXP20X_ADC_RATE_MASK | AXP813_V_I_ADC_RATE_MASK)
#define AXP813_TS_GPIO0_ADC_RATE_HZ(x) AXP20X_ADC_RATE_HZ(x)
#define AXP813_V_I_ADC_RATE_HZ(x) ((ilog2((x) / 100) << 4) & AXP813_V_I_ADC_RATE_MASK)
#define AXP813_ADC_RATE_HZ(x) (AXP20X_ADC_RATE_HZ(x) | AXP813_V_I_ADC_RATE_HZ(x))
@@ -234,7 +235,7 @@ static int axp20x_adc_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val)
{
struct axp20x_adc_iio *info = iio_priv(indio_dev);
- int size = 12;
+ int ret, size;
/*
* N.B.: Unlike the Chinese datasheets tell, the charging current is
@@ -246,10 +247,11 @@ static int axp20x_adc_raw(struct iio_dev *indio_dev,
else
size = 12;
- *val = axp20x_read_variable_width(info->regmap, chan->address, size);
- if (*val < 0)
- return *val;
+ ret = axp20x_read_variable_width(info->regmap, chan->address, size);
+ if (ret < 0)
+ return ret;
+ *val = ret;
return IIO_VAL_INT;
}
@@ -257,11 +259,13 @@ static int axp22x_adc_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val)
{
struct axp20x_adc_iio *info = iio_priv(indio_dev);
+ int ret;
- *val = axp20x_read_variable_width(info->regmap, chan->address, 12);
- if (*val < 0)
- return *val;
+ ret = axp20x_read_variable_width(info->regmap, chan->address, 12);
+ if (ret < 0)
+ return ret;
+ *val = ret;
return IIO_VAL_INT;
}
@@ -269,11 +273,13 @@ static int axp813_adc_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val)
{
struct axp20x_adc_iio *info = iio_priv(indio_dev);
+ int ret;
- *val = axp20x_read_variable_width(info->regmap, chan->address, 12);
- if (*val < 0)
- return *val;
+ ret = axp20x_read_variable_width(info->regmap, chan->address, 12);
+ if (ret < 0)
+ return ret;
+ *val = ret;
return IIO_VAL_INT;
}
@@ -443,27 +449,27 @@ static int axp20x_adc_offset_voltage(struct iio_dev *indio_dev, int channel,
int *val)
{
struct axp20x_adc_iio *info = iio_priv(indio_dev);
+ unsigned int regval;
int ret;
- ret = regmap_read(info->regmap, AXP20X_GPIO10_IN_RANGE, val);
+ ret = regmap_read(info->regmap, AXP20X_GPIO10_IN_RANGE, ®val);
if (ret < 0)
return ret;
switch (channel) {
case AXP20X_GPIO0_V:
- *val &= AXP20X_GPIO10_IN_RANGE_GPIO0;
+ regval = FIELD_GET(AXP20X_GPIO10_IN_RANGE_GPIO0, regval);
break;
case AXP20X_GPIO1_V:
- *val &= AXP20X_GPIO10_IN_RANGE_GPIO1;
+ regval = FIELD_GET(AXP20X_GPIO10_IN_RANGE_GPIO1, regval);
break;
default:
return -EINVAL;
}
- *val = *val ? 700000 : 0;
-
+ *val = regval ? 700000 : 0;
return IIO_VAL_INT;
}
@@ -548,7 +554,7 @@ static int axp20x_write_raw(struct iio_dev *indio_dev,
long mask)
{
struct axp20x_adc_iio *info = iio_priv(indio_dev);
- unsigned int reg, regval;
+ unsigned int regmask, regval;
/*
* The AXP20X PMIC allows the user to choose between 0V and 0.7V offsets
@@ -560,25 +566,24 @@ static int axp20x_write_raw(struct iio_dev *indio_dev,
if (val != 0 && val != 700000)
return -EINVAL;
- val = val ? 1 : 0;
+ regval = val ? 1 : 0;
switch (chan->channel) {
case AXP20X_GPIO0_V:
- reg = AXP20X_GPIO10_IN_RANGE_GPIO0;
- regval = AXP20X_GPIO10_IN_RANGE_GPIO0_VAL(val);
+ regmask = AXP20X_GPIO10_IN_RANGE_GPIO0;
+ regval = FIELD_PREP(AXP20X_GPIO10_IN_RANGE_GPIO0, regval);
break;
case AXP20X_GPIO1_V:
- reg = AXP20X_GPIO10_IN_RANGE_GPIO1;
- regval = AXP20X_GPIO10_IN_RANGE_GPIO1_VAL(val);
+ regmask = AXP20X_GPIO10_IN_RANGE_GPIO1;
+ regval = FIELD_PREP(AXP20X_GPIO10_IN_RANGE_GPIO1, regval);
break;
default:
return -EINVAL;
}
- return regmap_update_bits(info->regmap, AXP20X_GPIO10_IN_RANGE, reg,
- regval);
+ return regmap_update_bits(info->regmap, AXP20X_GPIO10_IN_RANGE, regmask, regval);
}
static const struct iio_info axp20x_adc_iio_info = {
--
2.35.1
On 6/18/22 14:40, Aidan MacDonald wrote:
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index f52960d2dfbe..a71e35de333d 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -113,6 +113,20 @@ config PINCTRL_AT91PIO4
> Say Y here to enable the at91 pinctrl/gpio driver for Atmel PIO4
> controller available on sama5d2 SoC.
>
> +config PINCTRL_AXP192
> + tristate "X-Powers AXP192 PMIC pinctrl and GPIO Support"
> + depends on MFD_AXP20X
> + depends on OF
> + select PINMUX
> + select GENERIC_PINCONF
> + select GPIOLIB
> + help
> + AXP PMICs provides multiple GPIOs that can be muxed for different
provide
> + functions. This driver bundles a pinctrl driver to select the function
> + muxing and a GPIO driver to handle the GPIO when the GPIO function is
> + selected.
> + Say Y to enable pinctrl and GPIO support for the AXP192 PMIC.
--
~Randy
On Sun, Jun 19, 2022 at 5:40 AM Aidan MacDonald
<[email protected]> wrote:
>
> The AXP192 is another X-Powers PMIC similar to the existing ones.
>
> Acked-by: Rob Herring <[email protected]>
> Signed-off-by: Aidan MacDonald <[email protected]>
Acked-by: Chen-Yu Tsai <[email protected]>
On Sun, Jun 19, 2022 at 5:40 AM Aidan MacDonald
<[email protected]> wrote:
>
> The AXP192's USB power supply is similar to the AXP202 but it has
> different USB current limits.
Should also mention the different register offset for VBUS status.
ChenYu
>
> Acked-by: Rob Herring <[email protected]>
> Signed-off-by: Aidan MacDonald <[email protected]>
> ---
> .../bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml b/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml
> index 0c371b55c9e1..e800b3b97f0d 100644
> --- a/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml
> +++ b/Documentation/devicetree/bindings/power/supply/x-powers,axp20x-usb-power-supply.yaml
> @@ -22,6 +22,7 @@ properties:
> compatible:
> oneOf:
> - enum:
> + - x-powers,axp192-usb-power-supply
> - x-powers,axp202-usb-power-supply
> - x-powers,axp221-usb-power-supply
> - x-powers,axp223-usb-power-supply
> --
> 2.35.1
>
On Sun, Jun 19, 2022 at 5:40 AM Aidan MacDonald
<[email protected]> wrote:
>
> The AXP192 is identical to the AXP20x, except for two additional
> GPIO ADC channels.
>
> Acked-by: Rob Herring <[email protected]>
> Signed-off-by: Aidan MacDonald <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
On Sun, Jun 19, 2022 at 5:40 AM Aidan MacDonald
<[email protected]> wrote:
>
> The AXP192 is mostly the same as the AXP202 but has a different
> current limit.
>
> Acked-by: Sebastian Reichel <[email protected]>
> Signed-off-by: Aidan MacDonald <[email protected]>
> ---
> drivers/power/supply/axp20x_usb_power.c | 80 +++++++++++++++++++++----
> 1 file changed, 69 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c
> index a1e6d1d44808..03145374ae72 100644
> --- a/drivers/power/supply/axp20x_usb_power.c
> +++ b/drivers/power/supply/axp20x_usb_power.c
> @@ -48,6 +48,9 @@
> #define AXP813_VBUS_CLIMIT_2000mA 2
> #define AXP813_VBUS_CLIMIT_2500mA 3
>
> +#define AXP192_VBUS_CLIMIT_EN BIT(1)
> +#define AXP192_VBUS_CLIMIT_100mA BIT(0)
> +
> #define AXP20X_ADC_EN1_VBUS_CURR BIT(2)
> #define AXP20X_ADC_EN1_VBUS_VOLT BIT(3)
>
> @@ -121,6 +124,24 @@ static void axp20x_usb_power_poll_vbus(struct work_struct *work)
> mod_delayed_work(system_power_efficient_wq, &power->vbus_detect, DEBOUNCE_TIME);
> }
>
> +static int axp192_get_current_max(struct axp20x_usb_power *power, int *val)
> +{
> + unsigned int v;
> + int ret = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v);
> +
> + if (ret)
> + return ret;
> +
> + if (!(v & AXP192_VBUS_CLIMIT_EN))
> + *val = -1;
> + else if (v & AXP192_VBUS_CLIMIT_100mA)
> + *val = 100000;
> + else
> + *val = 500000;
> +
> + return 0;
> +}
> +
> static int axp20x_get_current_max(struct axp20x_usb_power *power, int *val)
> {
> unsigned int v;
> @@ -179,7 +200,7 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
> enum power_supply_property psp, union power_supply_propval *val)
> {
> struct axp20x_usb_power *power = power_supply_get_drvdata(psy);
> - unsigned int input, v;
> + unsigned int input, v, reg;
> int ret;
>
> switch (psp) {
> @@ -215,6 +236,8 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
> case POWER_SUPPLY_PROP_CURRENT_MAX:
> if (power->axp20x_id == AXP813_ID)
> return axp813_get_current_max(power, &val->intval);
> + else if (power->axp20x_id == AXP192_ID)
> + return axp192_get_current_max(power, &val->intval);
> return axp20x_get_current_max(power, &val->intval);
> case POWER_SUPPLY_PROP_CURRENT_NOW:
> if (IS_ENABLED(CONFIG_AXP20X_ADC)) {
> @@ -256,16 +279,20 @@ static int axp20x_usb_power_get_property(struct power_supply *psy,
>
> val->intval = POWER_SUPPLY_HEALTH_GOOD;
>
> - if (power->axp20x_id == AXP202_ID) {
> - ret = regmap_read(power->regmap,
> - AXP20X_USB_OTG_STATUS, &v);
> - if (ret)
> - return ret;
> + if (power->axp20x_id == AXP192_ID)
> + reg = AXP192_USB_OTG_STATUS;
> + else if (power->axp20x_id == AXP202_ID)
> + reg = AXP20X_USB_OTG_STATUS;
> + else
> + /* Other chips do not have an OTG status register */
> + break;
Nit: put the comment on the same line as the break, trailing it.
Otherwise,
Reviewed-by: Chen-Yu Tsai <[email protected]>
On Sun, Jun 19, 2022 at 5:40 AM Aidan MacDonald
<[email protected]> wrote:
>
> The code may be clearer if parameters are not re-purposed to hold
> temporary results like register values, so introduce local variables
> as necessary to avoid that. Also, use the common FIELD_PREP macro
> instead of a hand-rolled version.
>
> Suggested-by: Jonathan Cameron <[email protected]>
> Signed-off-by: Aidan MacDonald <[email protected]>
> ---
> drivers/iio/adc/axp20x_adc.c | 61 +++++++++++++++++++-----------------
> 1 file changed, 33 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> index 53bf7d4899d2..041511280e1e 100644
> --- a/drivers/iio/adc/axp20x_adc.c
> +++ b/drivers/iio/adc/axp20x_adc.c
> @@ -15,6 +15,7 @@
> #include <linux/property.h>
> #include <linux/regmap.h>
> #include <linux/thermal.h>
> +#include <linux/bitfield.h>
Sort this group of headers alphabetically please.
>
> #include <linux/iio/iio.h>
> #include <linux/iio/driver.h>
> @@ -22,20 +23,20 @@
> #include <linux/mfd/axp20x.h>
>
> #define AXP20X_ADC_EN1_MASK GENMASK(7, 0)
> -
> #define AXP20X_ADC_EN2_MASK (GENMASK(3, 2) | BIT(7))
> +
> #define AXP22X_ADC_EN1_MASK (GENMASK(7, 5) | BIT(0))
>
> #define AXP20X_GPIO10_IN_RANGE_GPIO0 BIT(0)
> #define AXP20X_GPIO10_IN_RANGE_GPIO1 BIT(1)
> -#define AXP20X_GPIO10_IN_RANGE_GPIO0_VAL(x) ((x) & BIT(0))
> -#define AXP20X_GPIO10_IN_RANGE_GPIO1_VAL(x) (((x) & BIT(0)) << 1)
>
> #define AXP20X_ADC_RATE_MASK GENMASK(7, 6)
> -#define AXP813_V_I_ADC_RATE_MASK GENMASK(5, 4)
> -#define AXP813_ADC_RATE_MASK (AXP20X_ADC_RATE_MASK | AXP813_V_I_ADC_RATE_MASK)
> #define AXP20X_ADC_RATE_HZ(x) ((ilog2((x) / 25) << 6) & AXP20X_ADC_RATE_MASK)
> +
> #define AXP22X_ADC_RATE_HZ(x) ((ilog2((x) / 100) << 6) & AXP20X_ADC_RATE_MASK)
> +
> +#define AXP813_V_I_ADC_RATE_MASK GENMASK(5, 4)
> +#define AXP813_ADC_RATE_MASK (AXP20X_ADC_RATE_MASK | AXP813_V_I_ADC_RATE_MASK)
Please also mention "grouping macros based on chip type" in the commit log.
Otherwise,
Reviewed-by: Chen-Yu Tsai <[email protected]>
On Sat, Jun 18, 2022 at 11:40 PM Aidan MacDonald
<[email protected]> wrote:
>
> The AXP192 PMIC is similar to the AXP202/AXP209, but with different
> regulators, additional GPIOs, and a different IRQ register layout.
...
> +static int axp192_get_irq_reg(unsigned int base_reg, int i)
> +{
> + /* linear mapping for IRQ1 to IRQ4 */
> + if (i < 4)
> + return base_reg + i;
> +
> + /* handle IRQ5 separately */
> + if (base_reg == AXP192_IRQ1_EN)
> + return AXP192_IRQ5_EN;
> + else
Redundant 'else'.
> + return AXP192_IRQ5_STATE;
> +}
...
> +enum {
> + AXP192_DCDC1 = 0,
> + AXP192_DCDC2,
> + AXP192_DCDC3,
> + AXP192_LDO1,
> + AXP192_LDO2,
> + AXP192_LDO3,
> + AXP192_LDO_IO0,
> + AXP192_REG_ID_MAX,
Comma is not needed for a terminator.
> +};
--
With Best Regards,
Andy Shevchenko
On Sat, Jun 18, 2022 at 11:40 PM Aidan MacDonald
<[email protected]> wrote:
>
> Add support for the AXP192 PMIC.
...
> @@ -401,6 +431,7 @@ static int axp20x_set_ramp_delay(struct regulator_dev *rdev, int ramp)
> break;
>
> fallthrough;
> +
> default:
> /* Not supported for this regulator */
> return -ENOTSUPP;
Stray change?
--
With Best Regards,
Andy Shevchenko
On Sat, Jun 18, 2022 at 11:40 PM Aidan MacDonald
<[email protected]> wrote:
>
> The AXP192 is identical to the AXP20x, except for the addition of
> two more GPIO ADC channels.
...
> +static int axp192_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val, int val2,
> + long mask)
> +{
> + struct axp20x_adc_iio *info = iio_priv(indio_dev);
> + unsigned int regmask, regval;
> +
> + /*
> + * The AXP192 PMIC allows the user to choose between 0V and 0.7V offsets
> + * for (independently) GPIO0-3 when in ADC mode.
> + */
> + if (mask != IIO_CHAN_INFO_OFFSET)
> + return -EINVAL;
> +
> + if (val != 0 && val != 700000)
> + return -EINVAL;
> + regval = val ? 1 : 0;
> +
As per comment against the previous patch use !!val directly?
> + switch (chan->channel) {
> + case AXP192_GPIO0_V:
> + regmask = AXP192_GPIO30_IN_RANGE_GPIO0;
> + regval = FIELD_PREP(AXP192_GPIO30_IN_RANGE_GPIO0, regval);
> + break;
> +
> + case AXP192_GPIO1_V:
> + regmask = AXP192_GPIO30_IN_RANGE_GPIO1;
> + regval = FIELD_PREP(AXP192_GPIO30_IN_RANGE_GPIO1, regval);
> + break;
> +
> + case AXP192_GPIO2_V:
> + regmask = AXP192_GPIO30_IN_RANGE_GPIO2;
> + regval = FIELD_PREP(AXP192_GPIO30_IN_RANGE_GPIO2, regval);
> + break;
> +
> + case AXP192_GPIO3_V:
> + regmask = AXP192_GPIO30_IN_RANGE_GPIO3;
> + regval = FIELD_PREP(AXP192_GPIO30_IN_RANGE_GPIO3, regval);
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return regmap_update_bits(info->regmap, AXP192_GPIO30_IN_RANGE, regmask, regval);
> +}
--
With Best Regards,
Andy Shevchenko
On Sun, 19 Jun 2022 11:56:56 +0800
Chen-Yu Tsai <[email protected]> wrote:
> On Sun, Jun 19, 2022 at 5:40 AM Aidan MacDonald
> <[email protected]> wrote:
> >
> > The code may be clearer if parameters are not re-purposed to hold
> > temporary results like register values, so introduce local variables
> > as necessary to avoid that. Also, use the common FIELD_PREP macro
> > instead of a hand-rolled version.
> >
> > Suggested-by: Jonathan Cameron <[email protected]>
> > Signed-off-by: Aidan MacDonald <[email protected]>
> > ---
> > drivers/iio/adc/axp20x_adc.c | 61 +++++++++++++++++++-----------------
> > 1 file changed, 33 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/iio/adc/axp20x_adc.c b/drivers/iio/adc/axp20x_adc.c
> > index 53bf7d4899d2..041511280e1e 100644
> > --- a/drivers/iio/adc/axp20x_adc.c
> > +++ b/drivers/iio/adc/axp20x_adc.c
> > @@ -15,6 +15,7 @@
> > #include <linux/property.h>
> > #include <linux/regmap.h>
> > #include <linux/thermal.h>
> > +#include <linux/bitfield.h>
>
> Sort this group of headers alphabetically please.
>
> >
> > #include <linux/iio/iio.h>
> > #include <linux/iio/driver.h>
> > @@ -22,20 +23,20 @@
> > #include <linux/mfd/axp20x.h>
> >
> > #define AXP20X_ADC_EN1_MASK GENMASK(7, 0)
> > -
> > #define AXP20X_ADC_EN2_MASK (GENMASK(3, 2) | BIT(7))
> > +
> > #define AXP22X_ADC_EN1_MASK (GENMASK(7, 5) | BIT(0))
> >
> > #define AXP20X_GPIO10_IN_RANGE_GPIO0 BIT(0)
> > #define AXP20X_GPIO10_IN_RANGE_GPIO1 BIT(1)
> > -#define AXP20X_GPIO10_IN_RANGE_GPIO0_VAL(x) ((x) & BIT(0))
> > -#define AXP20X_GPIO10_IN_RANGE_GPIO1_VAL(x) (((x) & BIT(0)) << 1)
> >
> > #define AXP20X_ADC_RATE_MASK GENMASK(7, 6)
> > -#define AXP813_V_I_ADC_RATE_MASK GENMASK(5, 4)
> > -#define AXP813_ADC_RATE_MASK (AXP20X_ADC_RATE_MASK | AXP813_V_I_ADC_RATE_MASK)
> > #define AXP20X_ADC_RATE_HZ(x) ((ilog2((x) / 25) << 6) & AXP20X_ADC_RATE_MASK)
> > +
> > #define AXP22X_ADC_RATE_HZ(x) ((ilog2((x) / 100) << 6) & AXP20X_ADC_RATE_MASK)
> > +
> > +#define AXP813_V_I_ADC_RATE_MASK GENMASK(5, 4)
> > +#define AXP813_ADC_RATE_MASK (AXP20X_ADC_RATE_MASK | AXP813_V_I_ADC_RATE_MASK)
>
> Please also mention "grouping macros based on chip type" in the commit log.
>
> Otherwise,
>
> Reviewed-by: Chen-Yu Tsai <[email protected]>
With Chen-Yu's suggested changes,
Reviewed-by: Jonathan Cameron <[email protected]>
Thanks,
Jonathan
On Sat, Jun 18, 2022 at 11:40 PM Aidan MacDonald
<[email protected]> wrote:
>
> The code may be clearer if parameters are not re-purposed to hold
> temporary results like register values, so introduce local variables
> as necessary to avoid that. Also, use the common FIELD_PREP macro
FIELD_PREP()
> instead of a hand-rolled version.
...
> #include <linux/property.h>
> #include <linux/regmap.h>
> #include <linux/thermal.h>
> +#include <linux/bitfield.h>
Keep it sorted?
...
> - val = val ? 1 : 0;
> + regval = val ? 1 : 0;
>
I think you may drop these two lines (including blank line) and...
> switch (chan->channel) {
> case AXP20X_GPIO0_V:
> - reg = AXP20X_GPIO10_IN_RANGE_GPIO0;
> - regval = AXP20X_GPIO10_IN_RANGE_GPIO0_VAL(val);
> + regmask = AXP20X_GPIO10_IN_RANGE_GPIO0;
> + regval = FIELD_PREP(AXP20X_GPIO10_IN_RANGE_GPIO0, regval);
...use !!val as an argument here...
> break;
>
> case AXP20X_GPIO1_V:
> - reg = AXP20X_GPIO10_IN_RANGE_GPIO1;
> - regval = AXP20X_GPIO10_IN_RANGE_GPIO1_VAL(val);
> + regmask = AXP20X_GPIO10_IN_RANGE_GPIO1;
> + regval = FIELD_PREP(AXP20X_GPIO10_IN_RANGE_GPIO1, regval);
...and here.
> break;
>
> default:
> return -EINVAL;
> }
--
With Best Regards,
Andy Shevchenko
On Sat, Jun 18, 2022 at 11:40 PM Aidan MacDonald
<[email protected]> wrote:
>
> The AXP192 is mostly the same as the AXP202 but has a different
> current limit.
...
> + int ret = regmap_read(power->regmap, AXP20X_VBUS_IPSOUT_MGMT, &v);
> +
> + if (ret)
> + return ret;
Please, split assignment, so it will be like
int ret;
ret = ...
if (ret)
...
> +static int axp192_usb_power_set_current_max(struct axp20x_usb_power *power,
> + int intval)
> +{
> + int val = AXP192_VBUS_CLIMIT_EN;
> + const int mask = AXP192_VBUS_CLIMIT_EN | AXP192_VBUS_CLIMIT_100mA;
> +
> + switch (intval) {
> + case 100000:
> + val |= AXP192_VBUS_CLIMIT_100mA;
> + fallthrough;
It's harder to part and error prone, can't you simply call a
regmap_update_bits with different arguments?
With this suggestion consider the defaults like these:
const int mask = AXP192_VBUS_CLIMIT_EN | AXP192_VBUS_CLIMIT_100mA;
int val = mask;
Then use val & ~AXP192_VBUS_CLIMIT_100mA in the below case.
> + case 500000:
> + return regmap_update_bits(power->regmap,
> + AXP20X_VBUS_IPSOUT_MGMT, mask, val);
> + default:
> + return -EINVAL;
> + }
> +}
--
With Best Regards,
Andy Shevchenko
On Sat, 18 Jun 2022 22:40:04 +0100
Aidan MacDonald <[email protected]> wrote:
> The AXP192 is identical to the AXP20x, except for the addition of
> two more GPIO ADC channels.
>
> Signed-off-by: Aidan MacDonald <[email protected]>
Hi Aidan,
Looking at this again, I'd have a slight preference for doing the switch to
adc_en2_mask as a precursor patch.
Still, not a big thing (and I should have raised it earlier!) so up to you.
Either way
Reviewed-by: Jonathan Cameron <[email protected]>
Thanks,
Jonathan
On Sun, 19 Jun 2022 00:43:07 +0200
Andy Shevchenko <[email protected]> wrote:
> On Saturday, June 18, 2022, Aidan MacDonald <[email protected]>
> wrote:
>
> > Changes in v3:
> >
> > * Update pinctrl driver to address Andy Shevchenko's review comments
> > from v1, and fix a few other
>
>
> I believe I gave more comments than just against pin control driver. Even
> though, some comments are still not addressed in the series, including pin
> control. Am I mistaken?
Hi Andy,
Maybe, it's a question of clarity/misunderstanding? You had some 'global' comments
at the end of the pinctrl review. Perhaps not clear enough you meant
they should apply to the rest of the patch series (and more generally to
the driver being modified I think).
Just guessing!
Jonathan
>
>
> > * Add gpio-ranges property and example snippet to gpio DT bindings.
> > * Update commit message of patch 01/16 to point out that all register
> > addresses are obtained using sub_irq_reg().
> > * Document ccc_table in axp20x_battery. Also update commit message to
> > note a small fix that is part of that patch.
> > * Drop axp20x_adc consolidation patch in favor of using separate adc_raw
> > functions. It's a minor code size optimization that may not be worth
> > the effort due to implementation complexity.
> > * Use the FIELD_GET macro in axp20x_adc to further clarify intent.
> > * Fix a typo in the regulator driver where an AXP20X regulator ID was
> > mistakenly used instead of an AXP192 regulator ID. Also carry over
> > an Acked-by: tag from v1. Hope that's okay.
> > * Accumulate Acked-by: tags from v1 on DT patches.
> > * Accumulate Acked-by: tags from v2.
> >
> > Note that regmap maintainer Mark Brown has said the first two patches to
> > regmap-irq aren't suitable for inclusion into the kernel in their current
> > state. I'm including them for v3 so the series remains testable.
> >
> > Changes in v2:
> >
> > * Do a little cleanup of axp20x_adc suggested by Jonathan Cameron
> > * Consolidate ADC read functions in axp20x_adc
> > * Drop the axp192's read_label callback in axp20x_adc
> > * Clean up the axp192-gpio dt bindings
> > * Rewrite a problematic bit of code in axp20x_usb_power reported
> > by kernel test robot
> > * Support AXP192 in axp20x_battery
> > * Split up regmap-irq changes to two separate patches
> >
> > Cover letter from v1:
> >
> > Hi all,
> >
> > This patch series adds support for the X-Powers AXP192 PMIC to the
> > AXP20x driver framework.
> >
> > The first patch is a small change to regmap-irq to support the AXP192's
> > unusual IRQ register layout. It isn't possible to include all of the
> > IRQ registers in one regmap-irq chip without this.
> >
> > The rest of the changes are pretty straightforward, I think the only
> > notable parts are the axp20x_adc driver where there seems to be some
> > opportunities for code reuse (the axp192 is nearly a duplicate of the
> > axp20x) and the addition of a new pinctrl driver for the axp192, since
> > the axp20x pinctrl driver was not very easy to adapt.
> >
> > Aidan MacDonald (16):
> > regmap-irq: Use sub_irq_reg() to calculate unmask register address
> > regmap-irq: Add get_irq_reg to support unusual register layouts
> > dt-bindings: mfd: add bindings for AXP192 MFD device
> > dt-bindings: iio: adc: axp209: Add AXP192 compatible
> > dt-bindings: power: supply: axp20x: Add AXP192 compatible
> > dt-bindings: gpio: Add AXP192 GPIO bindings
> > dt-bindings: power: axp20x-battery: Add AXP192 compatible
> > mfd: axp20x: Add support for AXP192
> > regulator: axp20x: Add support for AXP192
> > iio: adc: axp20x_adc: Minor code cleanups
> > iio: adc: axp20x_adc: Add support for AXP192
> > power: supply: axp20x_usb_power: Add support for AXP192
> > pinctrl: Add AXP192 pin control driver
> > power: axp20x_battery: Add constant charge current table
> > power: axp20x_battery: Support battery status without fuel gauge
> > power: axp20x_battery: Add support for AXP192
> >
> > .../bindings/gpio/x-powers,axp192-gpio.yaml | 68 +++
> > .../bindings/iio/adc/x-powers,axp209-adc.yaml | 18 +
> > .../bindings/mfd/x-powers,axp152.yaml | 1 +
> > .../x-powers,axp20x-battery-power-supply.yaml | 1 +
> > .../x-powers,axp20x-usb-power-supply.yaml | 1 +
> > drivers/base/regmap/regmap-irq.c | 19 +-
> > drivers/iio/adc/axp20x_adc.c | 359 +++++++++--
> > drivers/mfd/axp20x-i2c.c | 2 +
> > drivers/mfd/axp20x.c | 153 +++++
> > drivers/pinctrl/Kconfig | 14 +
> > drivers/pinctrl/Makefile | 1 +
> > drivers/pinctrl/pinctrl-axp192.c | 562 ++++++++++++++++++
> > drivers/power/supply/axp20x_battery.c | 143 ++++-
> > drivers/power/supply/axp20x_usb_power.c | 80 ++-
> > drivers/regulator/axp20x-regulator.c | 101 +++-
> > include/linux/mfd/axp20x.h | 84 +++
> > include/linux/regmap.h | 5 +
> > 17 files changed, 1529 insertions(+), 83 deletions(-)
> > create mode 100644 Documentation/devicetree/
> > bindings/gpio/x-powers,axp192-gpio.yaml
> > create mode 100644 drivers/pinctrl/pinctrl-axp192.c
> >
> > --
> > 2.35.1
> >
> >
>
On Sun, Jun 19, 2022 at 1:20 PM Andy Shevchenko
<[email protected]> wrote:
> On Sat, Jun 18, 2022 at 11:40 PM Aidan MacDonald
> <[email protected]> wrote:
Hit 'Send' accidentally, here is the rest of the review, including
previous comments.
...
> > +config PINCTRL_AXP192
> > + tristate "X-Powers AXP192 PMIC pinctrl and GPIO Support"
> > + depends on MFD_AXP20X
>
>
> > + depends on OF
>
> Why?
>
> > + select PINMUX
> > + select GENERIC_PINCONF
> > + select GPIOLIB
>
> ...
>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
>
> Why?
Perhaps you missed mod_devicetable.h.
> ...
>
> > +struct axp192_pctl_function {
> > + const char *name;
> > + /* Mux value written to the control register to select the function (-1 if unsupported) */
>
> Comment is misleading. -1 can't be a value of unsigned type.
>
> > + const u8 *muxvals;
> > + const char * const *groups;
> > + unsigned int ngroups;
> > +};
>
> ...
>
> > +struct axp192_pctl_desc {
> > + unsigned int npins;
> > + const struct pinctrl_pin_desc *pins;
> > + /* Description of the function control register for each pin */
> > + const struct axp192_pctl_reg_info *ctrl_regs;
> > + /* Description of the output signal register for each pin */
> > + const struct axp192_pctl_reg_info *out_regs;
> > + /* Description of the input signal register for each pin */
> > + const struct axp192_pctl_reg_info *in_regs;
> > + /* Description of the pull down resistor config register for each pin */
>
> Can you just convert these comments to a kernel-doc?
>
> > + const struct axp192_pctl_reg_info *pull_down_regs;
> > +
> > + unsigned int nfunctions;
> > + const struct axp192_pctl_function *functions;
> > +};
>
> ...
>
> > +
> > +
>
> One blank line is enough.
>
> ...
>
> > + switch (param) {
> > + case PIN_CONFIG_BIAS_DISABLE:
> > + ret = axp192_pinconf_get_pull_down(pctldev, pin);
> > + if (ret < 0)
> > + return ret;
>
> > + else if (ret != 0)
>
> 1. Redundant 'else'
> 2. if (ret > 0)
>
> > + return -EINVAL;
> > + break;
> > +
> > + case PIN_CONFIG_BIAS_PULL_DOWN:
> > + ret = axp192_pinconf_get_pull_down(pctldev, pin);
> > + if (ret < 0)
> > + return ret;
> > + else if (ret == 0)
>
> Ditto.
>
> Looking at this I would rather expect the function to return something
> defined, than 0, non-0.
>
> > + return -EINVAL;
> > + break;
>
> > + default:
> > + return -ENOTSUPP;
> > + }
>
> ...
>
> > + for (cfg = 0; cfg < num_configs; ++cfg) {
>
> cfg++ will work the same way and easier to read.
...
You may make some lines shorter by introducing here
struct device *dev = &pdev->dev;
> > + struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
dev->parent
and so on...
...
> > + pctl->pctl_dev = devm_pinctrl_register(&pdev->dev, pctrl_desc, pctl);
> > + if (IS_ERR(pctl->pctl_dev))
> > + dev_err_probe(&pdev->dev, PTR_ERR(pctl->pctl_dev),
> > + "couldn't register pinctrl driver\n");
With the above it probably fits one line.
--
With Best Regards,
Andy Shevchenko
On Sun, Jun 19, 2022 at 1:08 PM Jonathan Cameron <[email protected]> wrote:
> On Sun, 19 Jun 2022 00:43:07 +0200
> Andy Shevchenko <[email protected]> wrote:
> > On Saturday, June 18, 2022, Aidan MacDonald <[email protected]>
> > wrote:
> >
> > > Changes in v3:
> > >
> > > * Update pinctrl driver to address Andy Shevchenko's review comments
> > > from v1, and fix a few other
> >
> > I believe I gave more comments than just against pin control driver. Even
> > though, some comments are still not addressed in the series, including pin
> > control. Am I mistaken?
>
> Hi Andy,
>
> Maybe, it's a question of clarity/misunderstanding? You had some 'global' comments
> at the end of the pinctrl review. Perhaps not clear enough you meant
> they should apply to the rest of the patch series (and more generally to
> the driver being modified I think).
Yeah, I think that is.
I don't remember if we have somewhere a documentation on how to
respond to the review comments, in which the point of addressing
comment everywhere in the series, and not only in the place(s) where
it was given.
--
With Best Regards,
Andy Shevchenko
On 18/06/2022 23:39, Aidan MacDonald wrote:
> The AXP192 PMIC is different enough from the PMICs supported by
> the AXP20x GPIO driver to warrant a separate driver. The AXP192
> driver also supports interrupts and pinconf settings.
>
> Signed-off-by: Aidan MacDonald <[email protected]>
> ---
> .../bindings/gpio/x-powers,axp192-gpio.yaml | 68 +++++++++++++++++++
> 1 file changed, 68 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
>
> diff --git a/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
> new file mode 100644
> index 000000000000..1e368cac9d3a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/gpio/x-powers,axp192-gpio.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: X-Powers AXP192 GPIO Device Tree Bindings
> +
> +maintainers:
> + - Chen-Yu Tsai <[email protected]>
> +
> +properties:
> + compatible:
> + const: x-powers,axp192-gpio
> +
> + gpio-controller: true
> +
> + "#gpio-cells":
> + const: 2
> + description: >
> + The first cell is the pin number and the second is the GPIO flags.
> +
> + gpio-ranges:
> + maxItems: 1
> +
> +patternProperties:
> + "-pins$":
> + $ref: /schemas/pinctrl/pinmux-node.yaml#
> +
> + properties:
> + pins:
> + items:
> + enum:
> + - GPIO0
> + - GPIO1
> + - GPIO2
> + - GPIO3
> + - GPIO4
> + - N_RSTO
> +
> + function:
> + enum:
> + - output
> + - input
> + - ldo
> + - pwm
> + - adc
> + - low_output
> + - floating
> + - ext_chg_ctl
> + - ldo_status
> +
> +required:
> + - compatible
> + - "#gpio-cells"
> + - gpio-controller
> + - gpio-ranges
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + pinctrl0: gpio@0 {
Unit address without reg, that's not a valid DTS.
> + compatible = "x-powers,axp192-gpio";
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl0 0 0 6>;
> + };
Best regards,
Krzysztof
On Sat, Jun 18, 2022 at 11:40 PM Aidan MacDonald
<[email protected]> wrote:
>
> The AXP192 PMIC's GPIO registers are much different from the GPIO
> registers of the AXP20x and AXP813 PMICs supported by the existing
> pinctrl-axp209 driver. It makes more sense to add a new driver for
> the AXP192, rather than add support in the existing axp20x driver.
>
> The pinctrl-axp192 driver is considerably more flexible in terms of
> register layout and should be able to support other X-Powers PMICs.
> Interrupts and pull down resistor configuration are supported too.
...
> +config PINCTRL_AXP192
> + tristate "X-Powers AXP192 PMIC pinctrl and GPIO Support"
> + depends on MFD_AXP20X
> + depends on OF
Why?
> + select PINMUX
> + select GENERIC_PINCONF
> + select GPIOLIB
...
> +#include <linux/of.h>
> +#include <linux/of_device.h>
Why?
...
> +struct axp192_pctl_function {
> + const char *name;
> + /* Mux value written to the control register to select the function (-1 if unsupported) */
Comment is misleading. -1 can't be a value of unsigned type.
> + const u8 *muxvals;
> + const char * const *groups;
> + unsigned int ngroups;
> +};
...
> +struct axp192_pctl_desc {
> + unsigned int npins;
> + const struct pinctrl_pin_desc *pins;
> + /* Description of the function control register for each pin */
> + const struct axp192_pctl_reg_info *ctrl_regs;
> + /* Description of the output signal register for each pin */
> + const struct axp192_pctl_reg_info *out_regs;
> + /* Description of the input signal register for each pin */
> + const struct axp192_pctl_reg_info *in_regs;
> + /* Description of the pull down resistor config register for each pin */
Can you just convert these comments to a kernel-doc?
> + const struct axp192_pctl_reg_info *pull_down_regs;
> +
> + unsigned int nfunctions;
> + const struct axp192_pctl_function *functions;
> +};
...
> +
> +
One blank line is enough.
...
> + switch (param) {
> + case PIN_CONFIG_BIAS_DISABLE:
> + ret = axp192_pinconf_get_pull_down(pctldev, pin);
> + if (ret < 0)
> + return ret;
> + else if (ret != 0)
1. Redundant 'else'
2. if (ret > 0)
> + return -EINVAL;
> + break;
> +
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> + ret = axp192_pinconf_get_pull_down(pctldev, pin);
> + if (ret < 0)
> + return ret;
> + else if (ret == 0)
Ditto.
Looking at this I would rather expect the function to return something
defined, than 0, non-0.
> + return -EINVAL;
> + break;
> + default:
> + return -ENOTSUPP;
> + }
...
> + for (cfg = 0; cfg < num_configs; ++cfg) {
cfg++ will work the same way and easier to read.
> + switch (pinconf_to_config_param(configs[cfg])) {
> + case PIN_CONFIG_BIAS_DISABLE:
> + ret = axp192_pinconf_set_pull_down(pctldev, pin, 0);
> + if (ret)
> + return ret;
> + break;
> +
> + case PIN_CONFIG_BIAS_PULL_DOWN:
> + ret = axp192_pinconf_set_pull_down(pctldev, pin, 1);
> + if (ret)
> + return ret;
> + break;
> +
> + default:
> + return -ENOTSUPP;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static const struct pinconf_ops axp192_conf_ops = {
> + .is_generic = true,
> + .pin_config_get = axp192_pinconf_get,
> + .pin_config_set = axp192_pinconf_set,
> + .pin_config_group_get = axp192_pinconf_get,
> + .pin_config_group_set = axp192_pinconf_set,
> +};
> +
> +static int axp192_pmx_set(struct pinctrl_dev *pctldev, unsigned int offset, u8 config)
> +{
> + struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
> + const struct axp192_pctl_reg_info *reginfo = &pctl->desc->ctrl_regs[offset];
> + unsigned int regval = config << (ffs(reginfo->mask) - 1);
> +
> + return regmap_update_bits(pctl->regmap, reginfo->reg, reginfo->mask, regval);
> +}
> +
> +static int axp192_pmx_func_cnt(struct pinctrl_dev *pctldev)
> +{
> + struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
> +
> + return pctl->desc->nfunctions;
> +}
> +
> +static const char *axp192_pmx_func_name(struct pinctrl_dev *pctldev, unsigned int selector)
> +{
> + struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
> +
> + return pctl->desc->functions[selector].name;
> +}
> +
> +static int axp192_pmx_func_groups(struct pinctrl_dev *pctldev, unsigned int selector,
> + const char * const **groups, unsigned int *num_groups)
> +{
> + struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
> +
> + *groups = pctl->desc->functions[selector].groups;
> + *num_groups = pctl->desc->functions[selector].ngroups;
> +
> + return 0;
> +}
> +
> +static int axp192_pmx_set_mux(struct pinctrl_dev *pctldev,
> + unsigned int function, unsigned int group)
> +{
> + struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
> + const u8 *muxvals = pctl->desc->functions[function].muxvals;
> +
> + if (muxvals[group] == U8_MAX)
> + return -EINVAL;
> +
> + /*
> + * Switching to LDO or PWM function will enable LDO/PWM output, so it's
> + * better to ignore these requests and let the regulator or PWM drivers
> + * handle muxing to avoid interfering with them.
> + */
> + if (function == AXP192_FUNC_LDO || function == AXP192_FUNC_PWM)
> + return 0;
> +
> + return axp192_pmx_set(pctldev, group, muxvals[group]);
> +}
> +
> +static int axp192_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
> + struct pinctrl_gpio_range *range,
> + unsigned int offset, bool input)
> +{
> + struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
> + const u8 *muxvals = input ? pctl->desc->functions[AXP192_FUNC_INPUT].muxvals
> + : pctl->desc->functions[AXP192_FUNC_OUTPUT].muxvals;
> +
> + return axp192_pmx_set(pctldev, offset, muxvals[offset]);
> +}
> +
> +static const struct pinmux_ops axp192_pmx_ops = {
> + .get_functions_count = axp192_pmx_func_cnt,
> + .get_function_name = axp192_pmx_func_name,
> + .get_function_groups = axp192_pmx_func_groups,
> + .set_mux = axp192_pmx_set_mux,
> + .gpio_set_direction = axp192_pmx_gpio_set_direction,
> + .strict = true,
> +};
> +
> +static int axp192_groups_cnt(struct pinctrl_dev *pctldev)
> +{
> + struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
> +
> + return pctl->desc->npins;
> +}
> +
> +static const char *axp192_group_name(struct pinctrl_dev *pctldev, unsigned int selector)
> +{
> + struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
> +
> + return pctl->desc->pins[selector].name;
> +}
> +
> +static int axp192_group_pins(struct pinctrl_dev *pctldev, unsigned int selector,
> + const unsigned int **pins, unsigned int *num_pins)
> +{
> + struct axp192_pctl *pctl = pinctrl_dev_get_drvdata(pctldev);
> +
> + *pins = &pctl->desc->pins[selector].number;
> + *num_pins = 1;
> +
> + return 0;
> +}
> +
> +static const struct pinctrl_ops axp192_pctrl_ops = {
> + .dt_node_to_map = pinconf_generic_dt_node_to_map_group,
> + .dt_free_map = pinconf_generic_dt_free_map,
> + .get_groups_count = axp192_groups_cnt,
> + .get_group_name = axp192_group_name,
> + .get_group_pins = axp192_group_pins,
> +};
> +
> +static int axp192_pctl_probe(struct platform_device *pdev)
> +{
> + struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> + struct axp192_pctl *pctl;
> + struct pinctrl_desc *pctrl_desc;
> + int ret, i;
> +
> + pctl = devm_kzalloc(&pdev->dev, sizeof(*pctl), GFP_KERNEL);
> + if (!pctl)
> + return -ENOMEM;
> +
> + pctl->desc = device_get_match_data(&pdev->dev);
> + pctl->regmap = axp20x->regmap;
> + pctl->regmap_irqc = axp20x->regmap_irqc;
> + pctl->dev = &pdev->dev;
> +
> + pctl->chip.base = -1;
> + pctl->chip.can_sleep = true;
> + pctl->chip.request = gpiochip_generic_request;
> + pctl->chip.free = gpiochip_generic_free;
> + pctl->chip.parent = &pdev->dev;
> + pctl->chip.label = dev_name(&pdev->dev);
> + pctl->chip.owner = THIS_MODULE;
> + pctl->chip.get = axp192_gpio_get;
> + pctl->chip.get_direction = axp192_gpio_get_direction;
> + pctl->chip.set = axp192_gpio_set;
> + pctl->chip.direction_input = axp192_gpio_direction_input;
> + pctl->chip.direction_output = axp192_gpio_direction_output;
> + pctl->chip.to_irq = axp192_gpio_to_irq;
> + pctl->chip.ngpio = pctl->desc->npins;
> +
> + pctl->irqs = devm_kcalloc(&pdev->dev, pctl->desc->npins, sizeof(int), GFP_KERNEL);
> + if (!pctl->irqs)
> + return -ENOMEM;
> +
> + for (i = 0; i < pctl->desc->npins; ++i) {
> + ret = platform_get_irq_byname_optional(pdev, pctl->desc->pins[i].name);
> + if (ret > 0)
> + pctl->irqs[i] = ret;
> + }
> +
> + platform_set_drvdata(pdev, pctl);
> +
> + pctrl_desc = devm_kzalloc(&pdev->dev, sizeof(*pctrl_desc), GFP_KERNEL);
> + if (!pctrl_desc)
> + return -ENOMEM;
> +
> + pctrl_desc->name = dev_name(&pdev->dev);
> + pctrl_desc->owner = THIS_MODULE;
> + pctrl_desc->pins = pctl->desc->pins;
> + pctrl_desc->npins = pctl->desc->npins;
> + pctrl_desc->pctlops = &axp192_pctrl_ops;
> + pctrl_desc->pmxops = &axp192_pmx_ops;
> + pctrl_desc->confops = &axp192_conf_ops;
> +
> + pctl->pctl_dev = devm_pinctrl_register(&pdev->dev, pctrl_desc, pctl);
> + if (IS_ERR(pctl->pctl_dev))
> + dev_err_probe(&pdev->dev, PTR_ERR(pctl->pctl_dev),
> + "couldn't register pinctrl driver\n");
> +
> + ret = devm_gpiochip_add_data(&pdev->dev, &pctl->chip, pctl);
> + if (ret)
> + dev_err_probe(&pdev->dev, ret, "Failed to register GPIO chip\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id axp192_pctl_match[] = {
> + { .compatible = "x-powers,axp192-gpio", .data = &axp192_data, },
> + { }
> +};
--
With Best Regards,
Andy Shevchenko
On Sat, Jun 18, 2022 at 11:40 PM Aidan MacDonald
<[email protected]> wrote:
>
> Add a table-based lookup method for constant charge current,
> which is necessary when the setting cannot be represented as
> a linear range.
>
> This also replaces the hard-coded 300 mA default ccc setting
> if the DT-specified value is unsupported; the minimum value
> for the device is now set exactly instead of relying on the
> value being rounded down to a supported value.
...
> +static int axp20x_get_constant_charge_current_sel(struct axp20x_batt_ps *axp_batt,
> + int charge_current)
> +{
> + int i;
> +
> + if (axp_batt->data->ccc_table) {
> + for (i = AXP20X_CHRG_CTRL1_TGT_CURR; i >= 0; --i) {
i-- should give the same result.
> + if (axp_batt->data->ccc_table[i] <= charge_current)
> + return i;
> + }
> +
> + return -EINVAL;
> + }
> + i = (charge_current - axp_batt->data->ccc_offset) / axp_batt->data->ccc_scale;
> +
No need to have a blank line here.
> + if (i > AXP20X_CHRG_CTRL1_TGT_CURR || i < 0)
> + return -EINVAL;
> +
> + return i;
> +}
--
With Best Regards,
Andy Shevchenko
Andy Shevchenko <[email protected]> writes:
> On Sun, Jun 19, 2022 at 1:08 PM Jonathan Cameron <[email protected]> wrote:
>> On Sun, 19 Jun 2022 00:43:07 +0200
>> Andy Shevchenko <[email protected]> wrote:
>> > On Saturday, June 18, 2022, Aidan MacDonald <[email protected]>
>> > wrote:
>> >
>> > > Changes in v3:
>> > >
>> > > * Update pinctrl driver to address Andy Shevchenko's review comments
>> > > from v1, and fix a few other
>> >
>> > I believe I gave more comments than just against pin control driver. Even
>> > though, some comments are still not addressed in the series, including pin
>> > control. Am I mistaken?
>>
>> Hi Andy,
>>
>> Maybe, it's a question of clarity/misunderstanding? You had some 'global' comments
>> at the end of the pinctrl review. Perhaps not clear enough you meant
>> they should apply to the rest of the patch series (and more generally to
>> the driver being modified I think).
>
> Yeah, I think that is.
> I don't remember if we have somewhere a documentation on how to
> respond to the review comments, in which the point of addressing
> comment everywhere in the series, and not only in the place(s) where
> it was given.
That's exactly it, I was only looking at the pinctrl patch since that's
the one you replied to, and didn't think to check the other patches for
similar cases even though that's obvious in retrospect. Sorry for any
confusion.
Jonathan Cameron <[email protected]> writes:
> On Sat, 18 Jun 2022 22:40:04 +0100
> Aidan MacDonald <[email protected]> wrote:
>
>> The AXP192 is identical to the AXP20x, except for the addition of
>> two more GPIO ADC channels.
>>
>> Signed-off-by: Aidan MacDonald <[email protected]>
> Hi Aidan,
>
> Looking at this again, I'd have a slight preference for doing the switch to
> adc_en2_mask as a precursor patch.
I might as well, it's a bit cleaner that way.
>
> Still, not a big thing (and I should have raised it earlier!) so up to you.
> Either way
>
> Reviewed-by: Jonathan Cameron <[email protected]>
>
> Thanks,
>
> Jonathan
On Sat, 18 Jun 2022 22:39:59 +0100, Aidan MacDonald wrote:
> The AXP192 PMIC is different enough from the PMICs supported by
> the AXP20x GPIO driver to warrant a separate driver. The AXP192
> driver also supports interrupts and pinconf settings.
>
> Signed-off-by: Aidan MacDonald <[email protected]>
> ---
> .../bindings/gpio/x-powers,axp192-gpio.yaml | 68 +++++++++++++++++++
> 1 file changed, 68 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/gpio/x-powers,axp192-gpio.example.dts:18.26-23.11: Warning (unit_address_vs_reg): /example-0/gpio@0: node has a unit name, but no reg or ranges property
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
Hi,
As Linus suggested you could have a look at devm_gpio_regmap_register()
with a custom xlate() callback and some improvements. But I've never
worked with pinctrl so I might be wrong. See below.
> +static int axp192_gpio_get(struct gpio_chip *chip, unsigned int
> offset)
> +{
> + struct axp192_pctl *pctl = gpiochip_get_data(chip);
> + const struct axp192_pctl_reg_info *reginfo =
> &pctl->desc->in_regs[offset];
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(pctl->regmap, reginfo->reg, &val);
> + if (ret)
> + return ret;
> +
> + return !!(val & reginfo->mask);
> +}
This should work.
> +
> +static int axp192_gpio_get_direction(struct gpio_chip *chip, unsigned
> int offset)
> +{
> + struct axp192_pctl *pctl = gpiochip_get_data(chip);
> + const struct axp192_pctl_reg_info *reginfo =
> &pctl->desc->ctrl_regs[offset];
> + const u8 *input_muxvals =
> pctl->desc->functions[AXP192_FUNC_INPUT].muxvals;
> + unsigned int val;
> + int ret;
> +
> + ret = regmap_read(pctl->regmap, reginfo->reg, &val);
> + if (ret)
> + return ret;
> +
> + if ((val & reginfo->mask) == (input_muxvals[offset] <<
> (ffs(reginfo->mask) - 1)))
> + return GPIO_LINE_DIRECTION_IN;
This isn't supported (yet) in gpio-regmap...
> +
> + return GPIO_LINE_DIRECTION_OUT;
> +}
> +
> +static void axp192_gpio_set(struct gpio_chip *chip, unsigned int
> offset, int value)
> +{
> + struct axp192_pctl *pctl = gpiochip_get_data(chip);
> + const struct axp192_pctl_reg_info *reginfo =
> &pctl->desc->out_regs[offset];
> +
> + regmap_update_bits(pctl->regmap, reginfo->reg, reginfo->mask, value
> ? reginfo->mask : 0);
> +}
> +
> +static int axp192_gpio_direction_input(struct gpio_chip *chip,
> unsigned int offset)
> +{
> + return pinctrl_gpio_direction_input(chip->base + offset);
> +}
..as well as this.
> +
> +static int axp192_gpio_direction_output(struct gpio_chip *chip,
> unsigned int offset, int value)
> +{
> + chip->set(chip, offset, value);
Why don't you call pinctrl_gpio_direction_output() here?
I *think* what is needed for gpio-regmap to support this is:
- support values and masks for the direction, for now, we
only support single bits.
- support the pinctrl_gpio_direction_{input,output} calls
-michael
Michael Walle <[email protected]> writes:
> Hi,
>
> As Linus suggested you could have a look at devm_gpio_regmap_register()
> with a custom xlate() callback and some improvements. But I've never
> worked with pinctrl so I might be wrong. See below.
>
>> +static int axp192_gpio_get(struct gpio_chip *chip, unsigned int offset)
>> +{
>> + struct axp192_pctl *pctl = gpiochip_get_data(chip);
>> + const struct axp192_pctl_reg_info *reginfo = &pctl->desc->in_regs[offset];
>> + unsigned int val;
>> + int ret;
>> +
>> + ret = regmap_read(pctl->regmap, reginfo->reg, &val);
>> + if (ret)
>> + return ret;
>> +
>> + return !!(val & reginfo->mask);
>> +}
>
> This should work.
>
>> +
>> +static int axp192_gpio_get_direction(struct gpio_chip *chip, unsigned
>> int offset)
>> +{
>> + struct axp192_pctl *pctl = gpiochip_get_data(chip);
>> + const struct axp192_pctl_reg_info *reginfo =
>> &pctl->desc->ctrl_regs[offset];
>> + const u8 *input_muxvals = pctl->desc->functions[AXP192_FUNC_INPUT].muxvals;
>> + unsigned int val;
>> + int ret;
>> +
>> + ret = regmap_read(pctl->regmap, reginfo->reg, &val);
>> + if (ret)
>> + return ret;
>> +
>> + if ((val & reginfo->mask) == (input_muxvals[offset] <<
>> (ffs(reginfo->mask) - 1)))
>> + return GPIO_LINE_DIRECTION_IN;
>
> This isn't supported (yet) in gpio-regmap...
>
>> +
>> + return GPIO_LINE_DIRECTION_OUT;
>> +}
>> +
>> +static void axp192_gpio_set(struct gpio_chip *chip, unsigned int
>> offset, int value)
>> +{
>> + struct axp192_pctl *pctl = gpiochip_get_data(chip);
>> + const struct axp192_pctl_reg_info *reginfo = &pctl->desc->out_regs[offset];
>> +
>> + regmap_update_bits(pctl->regmap, reginfo->reg, reginfo->mask, value
>> ? reginfo->mask : 0);
>> +}
>> +
>> +static int axp192_gpio_direction_input(struct gpio_chip *chip,
>> unsigned int offset)
>> +{
>> + return pinctrl_gpio_direction_input(chip->base + offset);
>> +}
>
> ..as well as this.
>
>> +
>> +static int axp192_gpio_direction_output(struct gpio_chip *chip,
>> unsigned int offset, int value)
>> +{
>> + chip->set(chip, offset, value);
>
> Why don't you call pinctrl_gpio_direction_output() here?
Probably because I copied this from pinctrl-axp209. I'll fix it in
the next version.
>
>
> I *think* what is needed for gpio-regmap to support this is:
> - support values and masks for the direction, for now, we
> only support single bits.
> - support the pinctrl_gpio_direction_{input,output} calls
>
> -michael
That sounds about right, thanks for taking a look.
Am 2022-06-27 15:12, schrieb Aidan MacDonald:
>> I *think* what is needed for gpio-regmap to support this is:
>> - support values and masks for the direction, for now, we
>> only support single bits.
>> - support the pinctrl_gpio_direction_{input,output} calls
>>
>> -michael
>
> That sounds about right, thanks for taking a look.
I thought you were trying to add these to gpio-regmap? Unless
I'm missing something, that should be easy enough.
-michael
Michael Walle <[email protected]> writes:
> Am 2022-06-27 15:12, schrieb Aidan MacDonald:
>
>>> I *think* what is needed for gpio-regmap to support this is:
>>> - support values and masks for the direction, for now, we
>>> only support single bits.
>>> - support the pinctrl_gpio_direction_{input,output} calls
>>> -michael
>> That sounds about right, thanks for taking a look.
>
> I thought you were trying to add these to gpio-regmap? Unless
> I'm missing something, that should be easy enough.
>
> -michael
Yeah, I can send patches this weekend. One detail I missed was the
to_irq() part, so I'm going to add a hook for that too.
The alternative is using hierarchical IRQ domains, but that seems
like a lot of added complication when to_irq() works just as well.