| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20230417150520.384d0747@donnerap.cambridge.arm.com>
Date: Mon, 17 Apr 2023 15:05:20 +0100
From: Andre Przywara <andre.przywara@....com>
To: Shengyu Qu <wiagn233@...look.com>
Cc: lee@...nel.org, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, wens@...e.org,
lgirdwood@...il.com, broonie@...nel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Martin Botka <martin.botka@...ainline.org>,
martin.botka1@...il.com
Subject: Re: [PATCH v1 2/4] mfd: axp20x: Add support for AXP15060 PMIC
On Fri, 7 Apr 2023 22:18:11 +0800
Shengyu Qu <wiagn233@...look.com> wrote:
Hi Shengyu,
thanks for doing the tedious work and sending this!
> The AXP15060 is a PMIC chip produced by X-Powers, and could be connected
> via an I2C bus.
>
> Describe the regmap and the MFD bits, along with the registers exposed
> via I2C. Eventually advertise the device using a new compatible string
> and add support for power off the system.
>
> The driver would disable PEK function if IRQ is not configured in device
> tree, since some boards (For example, Starfive Visionfive 2) didn't
> connect IRQ line of PMIC to SOC.
>
> GPIO function isn't enabled in this commit, since its configuration
> operation is different from any existing AXP PMICs and needs
> logic modification on existing driver. GPIO support might come in later
> patches.
>
> Signed-off-by: Shengyu Qu <wiagn233@...look.com>
> ---
> drivers/mfd/axp20x-i2c.c | 2 +
> drivers/mfd/axp20x.c | 90 ++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/axp20x.h | 85 +++++++++++++++++++++++++++++++++++
> 3 files changed, 177 insertions(+)
>
> diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
> index f49fbd307958..b4f5cb457117 100644
> --- a/drivers/mfd/axp20x-i2c.c
> +++ b/drivers/mfd/axp20x-i2c.c
> @@ -65,6 +65,7 @@ static const struct of_device_id axp20x_i2c_of_match[] = {
> { .compatible = "x-powers,axp223", .data = (void *)AXP223_ID },
> { .compatible = "x-powers,axp803", .data = (void *)AXP803_ID },
> { .compatible = "x-powers,axp806", .data = (void *)AXP806_ID },
> + { .compatible = "x-powers,axp15060", .data = (void *)AXP15060_ID },
> { },
> };
> MODULE_DEVICE_TABLE(of, axp20x_i2c_of_match);
> @@ -78,6 +79,7 @@ static const struct i2c_device_id axp20x_i2c_id[] = {
> { "axp223", 0 },
> { "axp803", 0 },
> { "axp806", 0 },
> + { "axp15060", 0 },
> { },
> };
> MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 01a6bbb6d266..42ec27a4dfc4 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -43,6 +43,7 @@ static const char * const axp20x_model_names[] = {
> "AXP806",
> "AXP809",
> "AXP813",
> + "AXP15060",
> };
>
> static const struct regmap_range axp152_writeable_ranges[] = {
> @@ -168,6 +169,31 @@ static const struct regmap_access_table axp806_volatile_table = {
> .n_yes_ranges = ARRAY_SIZE(axp806_volatile_ranges),
> };
>
> +static const struct regmap_range axp15060_writeable_ranges[] = {
> + regmap_reg_range(AXP15060_PWR_OUT_CTRL1, AXP15060_DCDC_MODE_CTRL2),
> + regmap_reg_range(AXP15060_OUTPUT_MONITOR_DISCHARGE, AXP15060_CPUSLDO_V_CTRL),
> + regmap_reg_range(AXP15060_PWR_WAKEUP_CTRL, AXP15060_PWR_DISABLE_DOWN_SEQ),
> + regmap_reg_range(AXP15060_PEK_KEY, AXP15060_PEK_KEY),
> + regmap_reg_range(AXP15060_IRQ1_EN, AXP15060_IRQ2_EN),
> + regmap_reg_range(AXP15060_IRQ1_STATE, AXP15060_IRQ2_STATE),
> +};
> +
> +static const struct regmap_range axp15060_volatile_ranges[] = {
> + regmap_reg_range(AXP15060_STARTUP_SRC, AXP15060_STARTUP_SRC),
> + regmap_reg_range(AXP15060_PWR_WAKEUP_CTRL, AXP15060_PWR_DISABLE_DOWN_SEQ),
> + regmap_reg_range(AXP15060_IRQ1_STATE, AXP15060_IRQ2_STATE),
> +};
> +
> +static const struct regmap_access_table axp15060_writeable_table = {
> + .yes_ranges = axp15060_writeable_ranges,
> + .n_yes_ranges = ARRAY_SIZE(axp15060_writeable_ranges),
> +};
> +
> +static const struct regmap_access_table axp15060_volatile_table = {
> + .yes_ranges = axp15060_volatile_ranges,
> + .n_yes_ranges = ARRAY_SIZE(axp15060_volatile_ranges),
> +};
> +
> static const struct resource axp152_pek_resources[] = {
> DEFINE_RES_IRQ_NAMED(AXP152_IRQ_PEK_RIS_EDGE, "PEK_DBR"),
> DEFINE_RES_IRQ_NAMED(AXP152_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
> @@ -236,6 +262,11 @@ static const struct resource axp809_pek_resources[] = {
> DEFINE_RES_IRQ_NAMED(AXP809_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
> };
>
> +static const struct resource axp15060_pek_resources[] = {
> + DEFINE_RES_IRQ_NAMED(AXP15060_IRQ_PEK_RIS_EDGE, "PEK_DBR"),
> + DEFINE_RES_IRQ_NAMED(AXP15060_IRQ_PEK_FAL_EDGE, "PEK_DBF"),
> +};
> +
> static const struct regmap_config axp152_regmap_config = {
> .reg_bits = 8,
> .val_bits = 8,
> @@ -281,6 +312,15 @@ static const struct regmap_config axp806_regmap_config = {
> .cache_type = REGCACHE_RBTREE,
> };
>
> +static const struct regmap_config axp15060_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .wr_table = &axp15060_writeable_table,
> + .volatile_table = &axp15060_volatile_table,
> + .max_register = AXP15060_IRQ2_STATE,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> #define INIT_REGMAP_IRQ(_variant, _irq, _off, _mask) \
> [_variant##_IRQ_##_irq] = { .reg_offset = (_off), .mask = BIT(_mask) }
>
> @@ -502,6 +542,23 @@ static const struct regmap_irq axp809_regmap_irqs[] = {
> INIT_REGMAP_IRQ(AXP809, GPIO0_INPUT, 4, 0),
> };
>
> +static const struct regmap_irq axp15060_regmap_irqs[] = {
> + INIT_REGMAP_IRQ(AXP15060, DIE_TEMP_HIGH_LV1, 0, 0),
> + INIT_REGMAP_IRQ(AXP15060, DIE_TEMP_HIGH_LV2, 0, 1),
> + INIT_REGMAP_IRQ(AXP15060, DCDC1_V_LOW, 0, 2),
> + INIT_REGMAP_IRQ(AXP15060, DCDC2_V_LOW, 0, 3),
> + INIT_REGMAP_IRQ(AXP15060, DCDC3_V_LOW, 0, 4),
> + INIT_REGMAP_IRQ(AXP15060, DCDC4_V_LOW, 0, 5),
> + INIT_REGMAP_IRQ(AXP15060, DCDC5_V_LOW, 0, 6),
> + INIT_REGMAP_IRQ(AXP15060, DCDC6_V_LOW, 0, 7),
> + INIT_REGMAP_IRQ(AXP15060, PEK_LONG, 1, 0),
> + INIT_REGMAP_IRQ(AXP15060, PEK_SHORT, 1, 1),
> + INIT_REGMAP_IRQ(AXP15060, GPIO1_INPUT, 1, 2),
> + INIT_REGMAP_IRQ(AXP15060, PEK_FAL_EDGE, 1, 3),
> + INIT_REGMAP_IRQ(AXP15060, PEK_RIS_EDGE, 1, 4),
> + INIT_REGMAP_IRQ(AXP15060, GPIO2_INPUT, 1, 5),
> +};
> +
> static const struct regmap_irq_chip axp152_regmap_irq_chip = {
> .name = "axp152_irq_chip",
> .status_base = AXP152_IRQ1_STATE,
> @@ -581,6 +638,17 @@ static const struct regmap_irq_chip axp809_regmap_irq_chip = {
> .num_regs = 5,
> };
>
> +static const struct regmap_irq_chip axp15060_regmap_irq_chip = {
> + .name = "axp15060",
> + .status_base = AXP15060_IRQ1_STATE,
> + .ack_base = AXP15060_IRQ1_STATE,
> + .unmask_base = AXP15060_IRQ1_EN,
> + .init_ack_masked = true,
> + .irqs = axp15060_regmap_irqs,
> + .num_irqs = ARRAY_SIZE(axp15060_regmap_irqs),
> + .num_regs = 2,
> +};
> +
> static const struct mfd_cell axp20x_cells[] = {
> {
> .name = "axp20x-gpio",
> @@ -825,6 +893,16 @@ static const struct mfd_cell axp813_cells[] = {
> },
> };
>
> +static const struct mfd_cell axp15060_cells[] = {
> + {
> + .name = "axp221-pek",
> + .num_resources = ARRAY_SIZE(axp15060_pek_resources),
> + .resources = axp15060_pek_resources,
> + }, {
> + .name = "axp20x-regulator",
> + },
As Lee mentioned the other day, there are the MFD_CELL_NAME and
MFD_CELL_RES macro to wrap those now.
I compared all the register offsets and bit names against the (AXP853T)
manual, they match.
> +};
> +
> static int axp20x_power_off(struct sys_off_data *data)
> {
> struct axp20x_dev *axp20x = data->cb_data;
> @@ -934,6 +1012,18 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
> */
> axp20x->regmap_irq_chip = &axp803_regmap_irq_chip;
> break;
> + case AXP15060_ID:
> + /* Don't register the power key part if there is no interrupt line. */
Mmh, I don't know if this is really needed. If the IRQ line is not
connected, it isn't really critical, it just wouldn't work, would it? I
wonder if the same problem applies to all the other PMICs?
I see that we handle it like this for the AXP806, but I wonder if this is
more a by-product of the slave mode requirement there.
> + if (axp20x->irq > 0) {
> + axp20x->nr_cells = ARRAY_SIZE(axp15060_cells);
> + axp20x->cells = axp15060_cells;
> + } else {
> + axp20x->nr_cells = ARRAY_SIZE(axp806_cells);
> + axp20x->cells = axp806_cells;
That looks a bit odd. Either we rename this alternative axp806_cells
definition somewhere above to something generic (axp_regulator_cells?), or
there should be a comment here explaining why all of a sudden an AXP15060
without an IRQ line is the same as the AXP806.
> + }
> + axp20x->regmap_cfg = &axp15060_regmap_config;
> + axp20x->regmap_irq_chip = &axp15060_regmap_irq_chip;
> + break;
> default:
> dev_err(dev, "unsupported AXP20X ID %lu\n", axp20x->variant);
> return -EINVAL;
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 2058194807bd..abc2bdc54bf5 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -21,6 +21,7 @@ enum axp20x_variants {
> AXP806_ID,
> AXP809_ID,
> AXP813_ID,
> + AXP15060_ID,
> NR_AXP20X_VARIANTS,
> };
>
> @@ -131,6 +132,39 @@ enum axp20x_variants {
> /* Other DCDC regulator control registers are the same as AXP803 */
> #define AXP813_DCDC7_V_OUT 0x26
>
> +#define AXP15060_STARTUP_SRC 0x00
> +#define AXP15060_PWR_OUT_CTRL1 0x10
> +#define AXP15060_PWR_OUT_CTRL2 0x11
> +#define AXP15060_PWR_OUT_CTRL3 0x12
> +#define AXP15060_DCDC1_V_CTRL 0x13
> +#define AXP15060_DCDC2_V_CTRL 0x14
> +#define AXP15060_DCDC3_V_CTRL 0x15
> +#define AXP15060_DCDC4_V_CTRL 0x16
> +#define AXP15060_DCDC5_V_CTRL 0x17
> +#define AXP15060_DCDC6_V_CTRL 0x18
> +#define AXP15060_ALDO1_V_CTRL 0x19
> +#define AXP15060_DCDC_MODE_CTRL1 0x1a
> +#define AXP15060_DCDC_MODE_CTRL2 0x1b
> +#define AXP15060_OUTPUT_MONITOR_DISCHARGE 0x1e
> +#define AXP15060_IRQ_PWROK_VOFF 0x1f
> +#define AXP15060_ALDO2_V_CTRL 0x20
> +#define AXP15060_ALDO3_V_CTRL 0x21
> +#define AXP15060_ALDO4_V_CTRL 0x22
> +#define AXP15060_ALDO5_V_CTRL 0x23
> +#define AXP15060_BLDO1_V_CTRL 0x24
> +#define AXP15060_BLDO2_V_CTRL 0x25
> +#define AXP15060_BLDO3_V_CTRL 0x26
> +#define AXP15060_BLDO4_V_CTRL 0x27
> +#define AXP15060_BLDO5_V_CTRL 0x28
> +#define AXP15060_CLDO1_V_CTRL 0x29
> +#define AXP15060_CLDO2_V_CTRL 0x2a
> +#define AXP15060_CLDO3_V_CTRL 0x2b
> +#define AXP15060_CLDO4_V_CTRL 0x2d
> +#define AXP15060_CPUSLDO_V_CTRL 0x2e
> +#define AXP15060_PWR_WAKEUP_CTRL 0x31
> +#define AXP15060_PWR_DISABLE_DOWN_SEQ 0x32
> +#define AXP15060_PEK_KEY 0x36
> +
> /* Interrupt */
> #define AXP152_IRQ1_EN 0x40
> #define AXP152_IRQ2_EN 0x41
> @@ -139,6 +173,11 @@ enum axp20x_variants {
> #define AXP152_IRQ2_STATE 0x49
> #define AXP152_IRQ3_STATE 0x4a
>
> +#define AXP15060_IRQ1_EN 0x40
> +#define AXP15060_IRQ2_EN 0x41
> +#define AXP15060_IRQ1_STATE 0x48
> +#define AXP15060_IRQ2_STATE 0x49
> +
> #define AXP20X_IRQ1_EN 0x40
> #define AXP20X_IRQ2_EN 0x41
> #define AXP20X_IRQ3_EN 0x42
> @@ -222,6 +261,8 @@ enum axp20x_variants {
> #define AXP22X_GPIO_STATE 0x94
> #define AXP22X_GPIO_PULL_DOWN 0x95
>
> +#define AXP15060_CLDO4_GPIO2_MODESET 0x2c
> +
Compared the register offsets against the manual, they match.
Cheers,
Andre
> /* Battery */
> #define AXP20X_CHRG_CC_31_24 0xb0
> #define AXP20X_CHRG_CC_23_16 0xb1
> @@ -419,6 +460,33 @@ enum {
> AXP813_REG_ID_MAX,
> };
>
> +enum {
> + AXP15060_DCDC1 = 0,
> + AXP15060_DCDC2,
> + AXP15060_DCDC3,
> + AXP15060_DCDC4,
> + AXP15060_DCDC5,
> + AXP15060_DCDC6,
> + AXP15060_ALDO1,
> + AXP15060_ALDO2,
> + AXP15060_ALDO3,
> + AXP15060_ALDO4,
> + AXP15060_ALDO5,
> + AXP15060_BLDO1,
> + AXP15060_BLDO2,
> + AXP15060_BLDO3,
> + AXP15060_BLDO4,
> + AXP15060_BLDO5,
> + AXP15060_CLDO1,
> + AXP15060_CLDO2,
> + AXP15060_CLDO3,
> + AXP15060_CLDO4,
> + AXP15060_CPUSLDO,
> + AXP15060_SW,
> + AXP15060_RTC_LDO,
> + AXP15060_REG_ID_MAX,
> +};
> +
> /* IRQs */
> enum {
> AXP152_IRQ_LDO0IN_CONNECT = 1,
> @@ -637,6 +705,23 @@ enum axp809_irqs {
> AXP809_IRQ_GPIO0_INPUT,
> };
>
> +enum axp15060_irqs {
> + AXP15060_IRQ_DIE_TEMP_HIGH_LV1 = 1,
> + AXP15060_IRQ_DIE_TEMP_HIGH_LV2,
> + AXP15060_IRQ_DCDC1_V_LOW,
> + AXP15060_IRQ_DCDC2_V_LOW,
> + AXP15060_IRQ_DCDC3_V_LOW,
> + AXP15060_IRQ_DCDC4_V_LOW,
> + AXP15060_IRQ_DCDC5_V_LOW,
> + AXP15060_IRQ_DCDC6_V_LOW,
> + AXP15060_IRQ_PEK_LONG,
> + AXP15060_IRQ_PEK_SHORT,
> + AXP15060_IRQ_GPIO1_INPUT,
> + AXP15060_IRQ_PEK_FAL_EDGE,
> + AXP15060_IRQ_PEK_RIS_EDGE,
> + AXP15060_IRQ_GPIO2_INPUT,
> +};
> +
> struct axp20x_dev {
> struct device *dev;
> int irq;
Powered by blists - more mailing lists