| 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: <eb8c820c864717d6348cd11f16e6899c744a92ed.camel@linaro.org>
Date: Fri, 09 May 2025 07:45:11 +0100
From: André Draszik <andre.draszik@...aro.org>
To: Lee Jones <lee@...nel.org>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Linus Walleij
<linus.walleij@...aro.org>, Bartosz Golaszewski <brgl@...ev.pl>, Srinivas
Kandagatla <srini@...nel.org>, Kees Cook <kees@...nel.org>, "Gustavo A. R.
Silva" <gustavoars@...nel.org>, Peter Griffin <peter.griffin@...aro.org>,
Tudor Ambarus <tudor.ambarus@...aro.org>, Will McVicker
<willmcvicker@...gle.com>, kernel-team@...roid.com,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux-gpio@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v9 4/6] mfd: max77759: add Maxim MAX77759 core mfd driver
Hi Lee,
On Thu, 2025-05-08 at 15:02 +0100, Lee Jones wrote:
> On Wed, 30 Apr 2025, André Draszik wrote:
>
> > The Maxim MAX77759 is a companion PMIC for USB Type-C applications and
> > includes Battery Charger, Fuel Gauge, temperature sensors, USB Type-C
> > Port Controller (TCPC), NVMEM, and a GPIO expander.
> >
> > Fuel Gauge and TCPC have separate and independent I2C addresses,
> > register maps, and interrupt lines and are therefore excluded from the
> > MFD core device driver here.
> >
> > The GPIO and NVMEM interfaces are accessed via specific commands to the
> > built-in microprocessor. This driver implements an API that client
> > drivers can use for accessing those.
> >
> > Signed-off-by: André Draszik <andre.draszik@...aro.org>
> >
> > ---
> > v6: really use postinc
> >
> > v5:
> > * update all (I hope) of Lee's comments:
> > * file header C comment (not C++)
> > * drop references to 'MFD'
> > * extra indent register bit definitions
> > * make 'struct max77759' public
> > * drop comments that were used for visual separation only
> > * drop MAX77759_*_REG_LAST_REGISTER defines
> > * add comments to regmap ranges
> > * use longer lines
> > * sort local variable in reverse christmas tree order
> > * update comments in max77759_maxq_command()
> > * drop BUILD_BUG_ON()
> > * use dev_err() in max77759_maxq_command()
> > * reflow max77759_create_i2c_subdev() slightly and update error messages
> > * drop useless comment in max77759_add_chained_maxq()
> > * reflow max77759_probe()
> > * consistent upper-/lower-case in messages
> >
> > v4:
> > * add missing build_bug.h include
> > * update an irq chip comment
> > * fix a whitespace in register definitions
> >
> > v2:
> > * add kernel doc for max77759_maxq_command() and related structs
> > * fix an msec / usec typo
> > * add missing error handling of devm_mutex_init() (Christophe)
> > * align sentinel in max77759_of_id[] with max77759_i2c_id[]
> > (Christophe)
> > * some tidy-ups in max77759_maxq_command() (Christophe)
> >
> > max77759 Lee's updates
> > ---
> > MAINTAINERS | 2 +
> > drivers/mfd/Kconfig | 20 ++
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/max77759.c | 690 +++++++++++++++++++++++++++++++++++++++++++
> > include/linux/mfd/max77759.h | 165 +++++++++++
> > 5 files changed, 878 insertions(+)
>
> This looks okay to me now.
Thanks :-)
> I assume the other patches depend on this one?
Yes, that is correct.
>
> [...]
>
> > diff --git a/drivers/mfd/max77759.c b/drivers/mfd/max77759.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..15723ac3ef49771eafd5c2e9984abc550eec7aa1
> > --- /dev/null
> > +++ b/drivers/mfd/max77759.c
> > @@ -0,0 +1,690 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright 2020 Google Inc
> > + * Copyright 2025 Linaro Ltd.
> > + *
> > + * Core driver for Maxim MAX77759 companion PMIC for USB Type-C
> > + */
>
> [...]
>
> > +int max77759_maxq_command(struct max77759 *max77759,
> > + const struct max77759_maxq_command *cmd,
> > + struct max77759_maxq_response *rsp)
> > +{
> > + DEFINE_FLEX(struct max77759_maxq_response, _rsp, rsp, length, 1);
> > + struct device *dev = regmap_get_device(max77759->regmap_maxq);
> > + static const unsigned int timeout_ms = 200;
> > + int ret;
> > +
> > + if (cmd->length > MAX77759_MAXQ_OPCODE_MAXLENGTH)
> > + return -EINVAL;
> > +
> > + /*
> > + * As a convenience for API users when issuing simple commands, rsp is
> > + * allowed to be NULL. In that case we need a temporary here to write
> > + * the response to, as we need to verify that the command was indeed
> > + * completed correctly.
> > + */
> > + if (!rsp)
> > + rsp = _rsp;
> > +
> > + if (!rsp->length || rsp->length > MAX77759_MAXQ_OPCODE_MAXLENGTH)
> > + return -EINVAL;
> > +
> > + guard(mutex)(&max77759->maxq_lock);
> > +
> > + reinit_completion(&max77759->cmd_done);
> > +
> > + /*
> > + * MaxQ latches the message when the DATAOUT32 register is written. If
> > + * cmd->length is shorter we still need to write 0 to it.
> > + */
> > + ret = regmap_bulk_write(max77759->regmap_maxq,
> > + MAX77759_MAXQ_REG_AP_DATAOUT0, cmd->cmd,
> > + cmd->length);
> > + if (!ret && cmd->length < MAX77759_MAXQ_OPCODE_MAXLENGTH)
> > + ret = regmap_write(max77759->regmap_maxq,
> > + MAX77759_MAXQ_REG_AP_DATAOUT32, 0);
> > + if (ret) {
> > + dev_err(dev, "writing command failed: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + /* wait for response from MaxQ */
>
> If you have to respin this patch, please s/wait/Wait/.
>
> If not, please send a subsequent patch.
I guess I might as well send another version, given this series is still
waiting for Srini's ACK.
Cheers,
Andre'
Powered by blists - more mailing lists