| 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: <5c901a6c831775a04924880cc9f783814f75b6aa.camel@linaro.org>
Date: Mon, 24 Nov 2025 06:21:00 +0000
From: André Draszik <andre.draszik@...aro.org>
To: amitsd@...gle.com, Sebastian Reichel <sre@...nel.org>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Lee Jones <lee@...nel.org>, Greg Kroah-Hartman
<gregkh@...uxfoundation.org>, Badhri Jagan Sridharan <badhri@...gle.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>, Peter Griffin
<peter.griffin@...aro.org>, Tudor Ambarus <tudor.ambarus@...aro.org>, Alim
Akhtar <alim.akhtar@...sung.com>
Cc: linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
devicetree@...r.kernel.org, linux-usb@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org, RD
Babiera <rdbabiera@...gle.com>, Kyle Tso <kyletso@...gle.com>
Subject: Re: [PATCH 4/6] mfd: max77759: modify irq configs
Hi Amit,
Thanks for your patches to enable the charger!
> From: Amit Sunil Dhamne <amitsd@...gle.com>
>
> Define specific bit-level masks for charger's registers and modify the
> irq mask for charger irq_chip. Also, configure the max77759 interrupt
> lines as active low to all interrupt registrations to ensure the
> interrupt controllers are configured with the correct trigger type.
>
> Signed-off-by: Amit Sunil Dhamne <amitsd@...gle.com>
> ---
> drivers/mfd/max77759.c | 24 +++++++++++++++++-------
> include/linux/mfd/max77759.h | 9 +++++++++
> 2 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mfd/max77759.c b/drivers/mfd/max77759.c
> index 6cf6306c4a3b..5fe22884f362 100644
> --- a/drivers/mfd/max77759.c
> +++ b/drivers/mfd/max77759.c
> @@ -256,8 +256,17 @@ static const struct regmap_irq max77759_topsys_irqs[] = {
> };
>
> static const struct regmap_irq max77759_chgr_irqs[] = {
> - REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0, GENMASK(7, 0)),
> - REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1, GENMASK(7, 0)),
> + REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0,
> + MAX77759_CHGR_REG_CHG_INT_AICL |
> + MAX77759_CHGR_REG_CHG_INT_CHGIN |
> + MAX77759_CHGR_REG_CHG_INT_CHG |
> + MAX77759_CHGR_REG_CHG_INT_INLIM),
> + REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1,
> + MAX77759_CHGR_REG_CHG_INT2_BAT_OILO |
> + MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC |
> + MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV |
> + MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO |
> + MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE),
> };
>
> static const struct regmap_irq_chip max77759_pmic_irq_chip = {
> @@ -486,8 +495,8 @@ static int max77759_add_chained_irq_chip(struct device *dev,
> "failed to get parent vIRQ(%d) for chip %s\n",
> pirq, chip->name);
>
> - ret = devm_regmap_add_irq_chip(dev, regmap, irq,
> - IRQF_ONESHOT | IRQF_SHARED, 0, chip,
> + ret = devm_regmap_add_irq_chip(dev, regmap, irq, IRQF_ONESHOT |
> + IRQF_SHARED | IRQF_TRIGGER_LOW, 0, chip,
> data);
Please correct me if I'm wrong, but I don't think this makes sense for a
chained IRQ in this case. What problem does this change fix?
> if (ret)
> return dev_err_probe(dev, ret, "failed to add %s IRQ chip\n",
> @@ -519,8 +528,9 @@ static int max77759_add_chained_maxq(struct i2c_client *client,
>
> ret = devm_request_threaded_irq(&client->dev, apcmdres_irq,
> NULL, apcmdres_irq_handler,
> - IRQF_ONESHOT | IRQF_SHARED,
> - dev_name(&client->dev), max77759);
> + IRQF_ONESHOT | IRQF_SHARED |
> + IRQF_TRIGGER_LOW, dev_name(&client->dev),
> + max77759);
dito.
> if (ret)
> return dev_err_probe(&client->dev, ret,
> "MAX77759_MAXQ_INT_APCMDRESI failed\n");
> @@ -633,7 +643,7 @@ static int max77759_probe(struct i2c_client *client)
> return dev_err_probe(&client->dev, -EINVAL,
> "invalid IRQ: %d\n", client->irq);
>
> - irq_flags = IRQF_ONESHOT | IRQF_SHARED;
> + irq_flags = IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW;
I don't believe IRQF_TRIGGER_LOW should be added here, as this is board-specific.
The polarity is meant to be set via DT (and the only current user of this driver
does so).
> irq_flags |= irqd_get_trigger_type(irq_data);
That's what gets us the config from DT.
>
> ret = devm_regmap_add_irq_chip(&client->dev, max77759->regmap_top,
> diff --git a/include/linux/mfd/max77759.h b/include/linux/mfd/max77759.h
> index c6face34e385..0ef29a48deec 100644
> --- a/include/linux/mfd/max77759.h
> +++ b/include/linux/mfd/max77759.h
> @@ -62,7 +62,16 @@
> #define MAX77759_CHGR_REG_CHG_INT 0xb0
> #define MAX77759_CHGR_REG_CHG_INT2 0xb1
> #define MAX77759_CHGR_REG_CHG_INT_MASK 0xb2
> +#define MAX77759_CHGR_REG_CHG_INT_AICL BIT(7)
> +#define MAX77759_CHGR_REG_CHG_INT_CHGIN BIT(6)
> +#define MAX77759_CHGR_REG_CHG_INT_CHG BIT(4)
> +#define MAX77759_CHGR_REG_CHG_INT_INLIM BIT(2)
> #define MAX77759_CHGR_REG_CHG_INT2_MASK 0xb3
> +#define MAX77759_CHGR_REG_CHG_INT2_BAT_OILO BIT(4)
> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC BIT(3)
> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV BIT(2)
> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO BIT(1)
> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE BIT(0)
Even if wireless out of scope, it'd still be nice to add macros for
the remaining bits to make this complete and avoid having to update
these again in case wireless support gets added in the future.
Also, would be nice to keep existing style and indent the bits from
the registers (see existing bit definitions in this file a few lines
further up).
Finally, can you add the bits below the respective register (0xb0 / 0xb1)
please, to keep suffix meaningful, and to follow existing style for cases
like this (see MAX77759_MAXQ_REG_UIC_INT1).
Cheers,
Andre'
Powered by blists - more mailing lists