[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250131171436.00002583@huawei.com>
Date: Fri, 31 Jan 2025 17:14:36 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Matti Vaittinen <mazziesaccount@...il.com>
CC: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>, Lee Jones
<lee@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Jonathan Cameron
<jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>, Linus Walleij
<linus.walleij@...aro.org>, Nuno Sa <nuno.sa@...log.com>, David Lechner
<dlechner@...libre.com>, Dumitru Ceclan <mitrutzceclan@...il.com>, "Trevor
Gamblin" <tgamblin@...libre.com>, Matteo Martelli
<matteomartelli3@...il.com>, AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-iio@...r.kernel.org>,
<linux-gpio@...r.kernel.org>
Subject: Re: [RFC PATCH 2/5] mfd: Add ROHM BD79124 ADC/GPO
On Fri, 31 Jan 2025 15:37:06 +0200
Matti Vaittinen <mazziesaccount@...il.com> wrote:
> Add core driver for the ROHM BD79124 ADC / GPO.
>
> The core driver launches the sub-drivers for the pinmux/GPO and for the
> IIO ADC. It also provides the regmap, and forwards the IRQ resource to
> the ADC.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@...il.com>
As per response in cover letter. This is a common device combination and so
far I don't think we ever bothered with an MFD. Lots of ADCs provide
GPIO chips as well so I'd just squash it into the ADC driver.
> ---
> drivers/mfd/Kconfig | 12 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/rohm-bd79124.c | 165 +++++++++++++++++++++++++++++++
> include/linux/mfd/rohm-bd79124.h | 32 ++++++
> 4 files changed, 210 insertions(+)
> create mode 100644 drivers/mfd/rohm-bd79124.c
> create mode 100644 include/linux/mfd/rohm-bd79124.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index ae23b317a64e..f024256fb180 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2113,6 +2113,18 @@ config MFD_ROHM_BD71828
> also a single-cell linear charger, a Coulomb counter, a real-time
> clock (RTC), GPIOs and a 32.768 kHz clock gate.
>
> +config MFD_ROHM_BD79124
> + tristate "Rohm BD79124 core driver"
> + depends on I2C
> + select REGMAP_I2C
> + help
> + Say yes here to build support for the ROHM BD79124 ADC core. The
> + ROHM BD79124 is a 12-bit, 8-channel, SAR ADC. The ADC supports
> + also an automatic measurement mode, with an alarm interrupt for
> + out-of-window measurements. The window is configurable for each
> + channel. The ADC inputs can optionally be used as general purpose
> + outputs.
> +
> config MFD_ROHM_BD957XMUF
> tristate "ROHM BD9576MUF and BD9573MUF Power Management ICs"
> depends on I2C=y
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e057d6d6faef..c7d64e933a7d 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -270,6 +270,7 @@ obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o
> obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
> obj-$(CONFIG_MFD_ROHM_BD71828) += rohm-bd71828.o
> obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
> +obj-$(CONFIG_MFD_ROHM_BD79124) += rohm-bd79124.o
> obj-$(CONFIG_MFD_ROHM_BD957XMUF) += rohm-bd9576.o
> obj-$(CONFIG_MFD_ROHM_BD96801) += rohm-bd96801.o
> obj-$(CONFIG_MFD_STMFX) += stmfx.o
> diff --git a/drivers/mfd/rohm-bd79124.c b/drivers/mfd/rohm-bd79124.c
> new file mode 100644
> index 000000000000..c35ab0e03b0b
> --- /dev/null
> +++ b/drivers/mfd/rohm-bd79124.c
> @@ -0,0 +1,165 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (C) 2025 ROHM Semiconductors
> +//
> +// ROHM BD79124 ADC / GPO driver
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
mod_devicetable.h
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/rohm-bd79124.h>
> +
> +static struct resource adc_alert;
What if we have two of these?
> +
> +enum {
> + CELL_PINMUX,
> + CELL_ADC,
> +};
> +
> +static struct mfd_cell bd79124_cells[] = {
> + [CELL_PINMUX] = { .name = "bd79124-pinmux", },
> + [CELL_ADC] = { .name = "bd79124-adc", },
> +};
> +
> +/* Read-only regs */
> +static const struct regmap_range bd79124_ro_ranges[] = {
> + {
> + .range_min = BD79124_REG_EVENT_FLAG,
> + .range_max = BD79124_REG_EVENT_FLAG,
> + }, {
> + .range_min = BD79124_REG_RECENT_CH0_LSB,
> + .range_max = BD79124_REG_RECENT_CH7_MSB,
> + },
> +};
> +
> +static const struct regmap_access_table bd79124_ro_regs = {
> + .no_ranges = &bd79124_ro_ranges[0],
> + .n_no_ranges = ARRAY_SIZE(bd79124_ro_ranges),
> +};
> +
> +static const struct regmap_range bd79124_volatile_ranges[] = {
> + {
> + .range_min = BD79124_REG_RECENT_CH0_LSB,
> + .range_max = BD79124_REG_RECENT_CH7_MSB,
> + }, {
> + .range_min = BD79124_REG_EVENT_FLAG,
> + .range_max = BD79124_REG_EVENT_FLAG,
> + }, {
> + .range_min = BD79124_REG_EVENT_FLAG_HI,
> + .range_max = BD79124_REG_EVENT_FLAG_HI,
> + }, {
> + .range_min = BD79124_REG_EVENT_FLAG_LO,
> + .range_max = BD79124_REG_EVENT_FLAG_LO,
> + }, {
> + .range_min = BD79124_REG_SYSTEM_STATUS,
> + .range_max = BD79124_REG_SYSTEM_STATUS,
> + },
> +};
> +
> +static const struct regmap_access_table bd79124_volatile_regs = {
> + .yes_ranges = &bd79124_volatile_ranges[0],
> + .n_yes_ranges = ARRAY_SIZE(bd79124_volatile_ranges),
> +};
> +
> +static const struct regmap_range bd79124_precious_ranges[] = {
> + {
> + .range_min = BD79124_REG_EVENT_FLAG_HI,
> + .range_max = BD79124_REG_EVENT_FLAG_HI,
> + }, {
> + .range_min = BD79124_REG_EVENT_FLAG_LO,
> + .range_max = BD79124_REG_EVENT_FLAG_LO,
> + },
> +};
> +
> +static const struct regmap_access_table bd79124_precious_regs = {
> + .yes_ranges = &bd79124_precious_ranges[0],
> + .n_yes_ranges = ARRAY_SIZE(bd79124_precious_ranges),
> +};
> +
> +static const struct regmap_config bd79124_regmap = {
> + .reg_bits = 16,
> + .val_bits = 8,
> + .read_flag_mask = BD79124_I2C_MULTI_READ,
> + .write_flag_mask = BD79124_I2C_MULTI_WRITE,
> + .max_register = BD79124_REG_MAX,
> + .cache_type = REGCACHE_MAPLE,
> + .volatile_table = &bd79124_volatile_regs,
> + .wr_table = &bd79124_ro_regs,
> + .precious_table = &bd79124_precious_regs,
> +};
> +
> +static int bd79124_probe(struct i2c_client *i2c)
> +{
> + int ret;
> + struct regmap *map;
> + struct device *dev = &i2c->dev;
> + int *adc_vref;
Wrap that in a structure. It's just a bit too odd to have just
one integer!
> +
> + adc_vref = devm_kzalloc(dev, sizeof(*adc_vref), GFP_KERNEL);
> + if (!adc_vref)
> + return -ENOMEM;
> +
> + /*
> + * Better to enable regulators here so we don't need to worry about the
> + * order of sub-device instantiation. We also need to deliver the
> + * reference voltage value to the ADC driver. This is done via
> + * the MFD driver's drvdata.
> + */
> + *adc_vref = devm_regulator_get_enable_read_voltage(dev, "vdd");
> + if (*adc_vref < 0)
> + return dev_err_probe(dev, ret, "Failed to get the Vdd\n");
> +
> + dev_set_drvdata(dev, adc_vref);
> +
> + ret = devm_regulator_get_enable(dev, "iovdd");
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Failed to enable I/O voltage\n");
> +
> + map = devm_regmap_init_i2c(i2c, &bd79124_regmap);
> + if (IS_ERR(map))
> + return dev_err_probe(dev, PTR_ERR(map),
> + "Failed to initialize Regmap\n");
> +
> + if (i2c->irq) {
> + adc_alert = DEFINE_RES_IRQ_NAMED(i2c->irq, "thresh-alert");
> + bd79124_cells[CELL_ADC].resources = &adc_alert;
> + bd79124_cells[CELL_ADC].num_resources = 1;
> + }
> +
> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, bd79124_cells,
> + ARRAY_SIZE(bd79124_cells), NULL, 0, NULL);
> + if (ret)
> + dev_err_probe(dev, ret, "Failed to create subdevices\n");
return dev_err_probe();
Then return 0 in other path.
> +
> + return ret;
> +}
>
> diff --git a/include/linux/mfd/rohm-bd79124.h b/include/linux/mfd/rohm-bd79124.h
> new file mode 100644
> index 000000000000..505faeb6f135
> --- /dev/null
> +++ b/include/linux/mfd/rohm-bd79124.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright 2021 ROHM Semiconductors.
No update on that date?
> + *
> + * Author: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
> + */
> +
> +#ifndef _MFD_BD79124_H
> +#define _MFD_BD79124_H
> +
> +#define BD79124_I2C_MULTI_READ 0x30
> +#define BD79124_I2C_MULTI_WRITE 0x28
> +#define BD79124_REG_MAX 0xaf
> +
> +#define BD79124_REG_SYSTEM_STATUS 0x0
Give it two digits. 0x00 for ever so slight readability advantage.
> +#define BD79124_REG_GEN_CFG 0x01
> +#define BD79124_REG_OPMODE_CFG 0x04
> +#define BD79124_REG_PINCFG 0x05
> +#define BD79124_REG_GPO_VAL 0x06
> +#define BD79124_REG_SEQUENCE_CFG 0x10
> +#define BD79124_REG_MANUAL_CHANNELS 0x11
> +#define BD79124_REG_AUTO_CHANNELS 0x12
> +#define BD79124_REG_ALERT_CH_SEL 0x14
> +#define BD79124_REG_EVENT_FLAG 0x18
> +#define BD79124_REG_EVENT_FLAG_HI 0x1a
> +#define BD79124_REG_EVENT_FLAG_LO 0x1c
> +#define BD79124_REG_HYSTERESIS_CH0 0x20
> +#define BD79124_REG_EVENTCOUNT_CH0 0x22
> +#define BD79124_REG_RECENT_CH0_LSB 0xa0
> +#define BD79124_REG_RECENT_CH7_MSB 0xaf
> +
> +#endif
Powered by blists - more mailing lists