On Tue, Feb 21, 2023 at 01:39:13PM +0300, Okan Sahin wrote:
> MFD driver for MAX77541/MAX77540 to enable its sub
> devices.
>
> The MAX77541 is a multi-function devices. It includes
> buck converter and ADC.
>
> The MAX77540 is a high-efficiency buck converter
> with two 3A switching phases.
>
> They have same regmap except for ADC part of MAX77541.
Extra space in the Subject.
...
> +#include <linux/of_device.h>
Why?
...
> +static const struct regmap_config max77541_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
Do you need lock of regmap?
> +};
...
> +static const struct mfd_cell max77540_devs[] = {
> + MFD_CELL_OF("max77540-regulator", NULL, NULL, 0, 0,
> + NULL),
Perfectly one line.
> +};
> +static const struct mfd_cell max77541_devs[] = {
> + MFD_CELL_OF("max77541-regulator", NULL, NULL, 0, 0,
> + NULL),
> + MFD_CELL_OF("max77541-adc", NULL, NULL, 0, 0,
> + NULL),
Ditto.
> +};
...
> + if (max77541->chip->id == MAX77541) {
> + ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
> + IRQF_ONESHOT | IRQF_SHARED, 0,
> + &max77541_adc_irq_chip,
> + &max77541->irq_adc);
> + if (ret)
> + return ret;
> + }
> + return ret;
return 0;
...
> +static const struct i2c_device_id max77541_i2c_id[];
What for?
...
> + if (dev->of_node)
> + max77541->chip = of_device_get_match_data(dev);
> + else
> + max77541->chip = (struct chip_info *)
> + i2c_match_id(max77541_i2c_id,
> + client)->driver_data;
Oh. Please use
const struct i2c_device_id *id = i2c_client_get_device_id(client);
...
max77541->chip = device_get_match_data(dev); // needs property.h
if (!max77541->chip)
max77541->chip = (struct chip_info *)id->driver_data;
> + if (!max77541->chip)
> + return -EINVAL;
...
> +#ifndef __MAX77541_MFD_H__
> +#define __MAX77541_MFD_H__
Can we go towards consistency in this?
Seems to me the most used patter so far is
#ifndef __LINUX_MFD_MAX77541_H
On Tue, 21 Feb 2023, Andy Shevchenko wrote:
> On Tue, Feb 21, 2023 at 01:39:13PM +0300, Okan Sahin wrote:
> > MFD driver for MAX77541/MAX77540 to enable its sub
> > devices.
> >
> > The MAX77541 is a multi-function devices. It includes
> > buck converter and ADC.
> >
> > The MAX77540 is a high-efficiency buck converter
> > with two 3A switching phases.
> >
> > They have same regmap except for ADC part of MAX77541.
>
> Extra space in the Subject.
>
> ...
>
> > +#include <linux/of_device.h>
>
> Why?
>
> ...
>
> > +static const struct regmap_config max77541_regmap_config = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
>
> Do you need lock of regmap?
>
> > +};
>
> ...
>
> > +static const struct mfd_cell max77540_devs[] = {
>
> > + MFD_CELL_OF("max77540-regulator", NULL, NULL, 0, 0,
> > + NULL),
>
> Perfectly one line.
>
> > +};
>
> > +static const struct mfd_cell max77541_devs[] = {
> > + MFD_CELL_OF("max77541-regulator", NULL, NULL, 0, 0,
> > + NULL),
> > + MFD_CELL_OF("max77541-adc", NULL, NULL, 0, 0,
> > + NULL),
>
> Ditto.
>
> > +};
>
> ...
>
> > + if (max77541->chip->id == MAX77541) {
> > + ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
> > + IRQF_ONESHOT | IRQF_SHARED, 0,
> > + &max77541_adc_irq_chip,
> > + &max77541->irq_adc);
> > + if (ret)
> > + return ret;
> > + }
>
> > + return ret;
>
> return 0;
>
> ...
>
> > +static const struct i2c_device_id max77541_i2c_id[];
>
> What for?
>
> ...
>
> > + if (dev->of_node)
> > + max77541->chip = of_device_get_match_data(dev);
> > + else
> > + max77541->chip = (struct chip_info *)
> > + i2c_match_id(max77541_i2c_id,
> > + client)->driver_data;
>
> Oh. Please use
>
> const struct i2c_device_id *id = i2c_client_get_device_id(client);
> ...
> max77541->chip = device_get_match_data(dev); // needs property.h
> if (!max77541->chip)
> max77541->chip = (struct chip_info *)id->driver_data;
>
> > + if (!max77541->chip)
> > + return -EINVAL;
>
> ...
>
> > +#ifndef __MAX77541_MFD_H__
> > +#define __MAX77541_MFD_H__
>
> Can we go towards consistency in this?
> Seems to me the most used patter so far is
>
> #ifndef __LINUX_MFD_MAX77541_H
Drop the LINUX_ part please.
@@ -791,6 +791,19 @@ config MFD_MAX14577
additional drivers must be enabled in order to use the functionality
of the device.
+config MFD_MAX77541
+ tristate "Analog Devices MAX77541/77540 PMIC Support"
+ depends on I2C=y
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ help
+ Say yes here to add support for Analog Devices
+ MAX77541 and MAX77540 Power Management ICs.This
+ driver provides common support for accessing the
+ device;additional drivers must be enabled in order
+ to use the functionality of the device.
+
config MFD_MAX77620
bool "Maxim Semiconductor MAX77620 and MAX20024 PMIC Support"
depends on I2C=y
@@ -161,6 +161,7 @@ obj-$(CONFIG_MFD_DA9063) += da9063.o
obj-$(CONFIG_MFD_DA9150) += da9150-core.o
obj-$(CONFIG_MFD_MAX14577) += max14577.o
+obj-$(CONFIG_MFD_MAX77541) += max77541.o
obj-$(CONFIG_MFD_MAX77620) += max77620.o
obj-$(CONFIG_MFD_MAX77650) += max77650.o
obj-$(CONFIG_MFD_MAX77686) += max77686.o
new file mode 100644
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 Analog Devices, Inc.
+ * Driver for the MAX77540 and MAX77541
+ */
+
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max77541.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+
+static const struct regmap_config max77541_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
+static const struct regmap_irq max77541_src_irqs[] = {
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_INT_SRC_TOPSYS),
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_INT_SRC_BUCK),
+};
+
+static const struct regmap_irq_chip max77541_src_irq_chip = {
+ .name = "max77541-src",
+ .status_base = MAX77541_REG_INT_SRC,
+ .mask_base = MAX77541_REG_INT_SRC,
+ .num_regs = 1,
+ .irqs = max77541_src_irqs,
+ .num_irqs = ARRAY_SIZE(max77541_src_irqs),
+};
+
+static const struct regmap_irq max77541_topsys_irqs[] = {
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_TOPSYS_INT_TJ_120C),
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_TOPSYS_INT_TJ_140C),
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_TOPSYS_INT_TSHDN),
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_TOPSYS_INT_UVLO),
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_TOPSYS_INT_ALT_SWO),
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_TOPSYS_INT_EXT_FREQ_DET),
+};
+
+static const struct regmap_irq_chip max77541_topsys_irq_chip = {
+ .name = "max77541-topsys",
+ .status_base = MAX77541_REG_TOPSYS_INT,
+ .mask_base = MAX77541_REG_TOPSYS_INT_M,
+ .num_regs = 1,
+ .irqs = max77541_topsys_irqs,
+ .num_irqs = ARRAY_SIZE(max77541_topsys_irqs),
+};
+
+static const struct regmap_irq max77541_buck_irqs[] = {
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_BUCK_INT_M1_POK_FLT),
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_BUCK_INT_M2_POK_FLT),
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_BUCK_INT_M1_SCFLT),
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_BUCK_INT_M2_SCFLT),
+};
+
+static const struct regmap_irq_chip max77541_buck_irq_chip = {
+ .name = "max77541-buck",
+ .status_base = MAX77541_REG_BUCK_INT,
+ .mask_base = MAX77541_REG_BUCK_INT_M,
+ .num_regs = 1,
+ .irqs = max77541_buck_irqs,
+ .num_irqs = ARRAY_SIZE(max77541_buck_irqs),
+};
+
+static const struct regmap_irq max77541_adc_irqs[] = {
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_ADC_INT_CH1_I),
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_ADC_INT_CH2_I),
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_ADC_INT_CH3_I),
+ MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_ADC_INT_CH6_I),
+};
+
+static const struct regmap_irq_chip max77541_adc_irq_chip = {
+ .name = "max77541-adc",
+ .status_base = MAX77541_REG_ADC_INT,
+ .mask_base = MAX77541_REG_ADC_MSK,
+ .num_regs = 1,
+ .irqs = max77541_adc_irqs,
+ .num_irqs = ARRAY_SIZE(max77541_adc_irqs),
+};
+
+static const struct mfd_cell max77540_devs[] = {
+ MFD_CELL_OF("max77540-regulator", NULL, NULL, 0, 0,
+ NULL),
+};
+
+static const struct mfd_cell max77541_devs[] = {
+ MFD_CELL_OF("max77541-regulator", NULL, NULL, 0, 0,
+ NULL),
+ MFD_CELL_OF("max77541-adc", NULL, NULL, 0, 0,
+ NULL),
+};
+
+static const struct chip_info chip[] = {
+ [MAX77540] = {
+ .id = MAX77540,
+ .n_devs = ARRAY_SIZE(max77540_devs),
+ .devs = max77540_devs,
+ },
+ [MAX77541] = {
+ .id = MAX77541,
+ .n_devs = ARRAY_SIZE(max77541_devs),
+ .devs = max77541_devs,
+ },
+};
+
+static int max77541_pmic_irq_init(struct device *dev)
+{
+ struct max77541 *max77541 = dev_get_drvdata(dev);
+ int irq = max77541->i2c->irq;
+ int ret;
+
+ ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
+ IRQF_ONESHOT | IRQF_SHARED, 0,
+ &max77541_src_irq_chip,
+ &max77541->irq_data);
+ if (ret)
+ return ret;
+
+ ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
+ IRQF_ONESHOT | IRQF_SHARED, 0,
+ &max77541_topsys_irq_chip,
+ &max77541->irq_topsys);
+ if (ret)
+ return ret;
+
+ ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
+ IRQF_ONESHOT | IRQF_SHARED, 0,
+ &max77541_buck_irq_chip,
+ &max77541->irq_buck);
+ if (ret)
+ return ret;
+
+ if (max77541->chip->id == MAX77541) {
+ ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
+ IRQF_ONESHOT | IRQF_SHARED, 0,
+ &max77541_adc_irq_chip,
+ &max77541->irq_adc);
+ if (ret)
+ return ret;
+ }
+
+ return ret;
+}
+
+static int max77541_pmic_setup(struct device *dev)
+{
+ struct max77541 *max77541 = dev_get_drvdata(dev);
+ unsigned int val;
+ int ret;
+
+ ret = max77541_pmic_irq_init(dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to initialize IRQ\n");
+
+ ret = regmap_read(max77541->regmap, MAX77541_REG_INT_SRC, &val);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(max77541->regmap, MAX77541_REG_TOPSYS_INT, &val);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(max77541->regmap, MAX77541_REG_BUCK_INT, &val);
+ if (ret)
+ return ret;
+
+ ret = device_init_wakeup(dev, true);
+ if (ret)
+ return dev_err_probe(dev, ret, "Unable to init wakeup\n");
+
+ return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
+ max77541->chip->devs,
+ max77541->chip->n_devs,
+ NULL, 0, NULL);
+}
+
+static const struct i2c_device_id max77541_i2c_id[];
+
+static int max77541_i2c_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct max77541 *max77541;
+
+ max77541 = devm_kzalloc(dev, sizeof(*max77541), GFP_KERNEL);
+ if (!max77541)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, max77541);
+ max77541->i2c = client;
+
+ if (dev->of_node)
+ max77541->chip = of_device_get_match_data(dev);
+ else
+ max77541->chip = (struct chip_info *)
+ i2c_match_id(max77541_i2c_id,
+ client)->driver_data;
+ if (!max77541->chip)
+ return -EINVAL;
+
+ max77541->regmap = devm_regmap_init_i2c(client,
+ &max77541_regmap_config);
+ if (IS_ERR(max77541->regmap))
+ return dev_err_probe(dev, PTR_ERR(max77541->regmap),
+ "Failed to allocate register map\n");
+
+ return max77541_pmic_setup(dev);
+}
+
+static const struct of_device_id max77541_of_id[] = {
+ {
+ .compatible = "adi,max77540",
+ .data = &chip[MAX77540],
+ },
+ {
+ .compatible = "adi,max77541",
+ .data = &chip[MAX77541],
+ },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max77541_of_id);
+
+static const struct i2c_device_id max77541_i2c_id[] = {
+ { "max77540", (kernel_ulong_t)&chip[MAX77540] },
+ { "max77541", (kernel_ulong_t)&chip[MAX77541] },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, max77541_i2c_id);
+
+static struct i2c_driver max77541_i2c_driver = {
+ .driver = {
+ .name = "max77541",
+ .of_match_table = max77541_of_id,
+ },
+ .probe_new = max77541_i2c_probe,
+ .id_table = max77541_i2c_id,
+};
+module_i2c_driver(max77541_i2c_driver);
+
+MODULE_DESCRIPTION("MAX7740/MAX7741 MFD Driver");
+MODULE_AUTHOR("Okan Sahin <okan.sahin@analog.com>");
+MODULE_LICENSE("GPL");
new file mode 100644
@@ -0,0 +1,102 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __MAX77541_MFD_H__
+#define __MAX77541_MFD_H__
+
+#include <linux/bits.h>
+#include <linux/types.h>
+
+/* REGISTERS */
+
+/* GLOBAL CONFIG1 */
+#define MAX77541_REG_INT_SRC 0x00
+#define MAX77541_REG_INT_SRC_M 0x01
+#define MAX77541_REG_TOPSYS_INT 0x02
+#define MAX77541_REG_TOPSYS_INT_M 0x03
+
+#define MAX77541_REG_EN_CTRL 0x0B
+
+/* BUCK CONFIG */
+#define MAX77541_REG_BUCK_INT 0x20
+#define MAX77541_REG_BUCK_INT_M 0x21
+
+#define MAX77541_REG_M1_VOUT 0x23
+#define MAX77541_REG_M1_CFG1 0x25
+
+#define MAX77541_REG_M2_VOUT 0x33
+#define MAX77541_REG_M2_CFG1 0x35
+
+/* INTERRUPT MASKS*/
+#define MAX77541_REG_INT_SRC_MASK 0x00
+#define MAX77541_REG_TOPSYS_INT_MASK 0x00
+#define MAX77541_REG_BUCK_INT_MASK 0x00
+
+/*BITS OF REGISTERS*/
+#define MAX77541_BIT_INT_SRC_TOPSYS BIT(0)
+#define MAX77541_BIT_INT_SRC_BUCK BIT(1)
+
+#define MAX77541_BIT_TOPSYS_INT_TJ_120C BIT(0)
+#define MAX77541_BIT_TOPSYS_INT_TJ_140C BIT(1)
+#define MAX77541_BIT_TOPSYS_INT_TSHDN BIT(2)
+#define MAX77541_BIT_TOPSYS_INT_UVLO BIT(3)
+#define MAX77541_BIT_TOPSYS_INT_ALT_SWO BIT(4)
+#define MAX77541_BIT_TOPSYS_INT_EXT_FREQ_DET BIT(5)
+
+#define MAX77541_BIT_BUCK_INT_M1_POK_FLT BIT(0)
+#define MAX77541_BIT_BUCK_INT_M2_POK_FLT BIT(1)
+#define MAX77541_BIT_BUCK_INT_M1_SCFLT BIT(4)
+#define MAX77541_BIT_BUCK_INT_M2_SCFLT BIT(5)
+
+#define MAX77541_BIT_M1_EN BIT(0)
+#define MAX77541_BIT_M2_EN BIT(1)
+
+#define MAX77541_BITS_MX_VOUT GENMASK(7, 0)
+#define MAX77541_BITS_MX_CFG1_RNG GENMASK(7, 6)
+
+/* ADC */
+#define MAX77541_REG_ADC_INT 0x70
+#define MAX77541_REG_ADC_MSK 0x71
+
+#define MAX77541_REG_ADC_DATA_CH1 0x72
+#define MAX77541_REG_ADC_DATA_CH2 0x73
+#define MAX77541_REG_ADC_DATA_CH3 0x74
+#define MAX77541_REG_ADC_DATA_CH6 0x77
+
+#define MAX77541_BIT_ADC_INT_CH1_I BIT(0)
+#define MAX77541_BIT_ADC_INT_CH2_I BIT(1)
+#define MAX77541_BIT_ADC_INT_CH3_I BIT(2)
+#define MAX77541_BIT_ADC_INT_CH6_I BIT(5)
+
+#define MAX77541_MAX_REGULATORS 2
+
+#define MAX77541_REGMAP_IRQ_REG(_mask) \
+ { .mask = (_mask) }
+
+enum max7754x_ids {
+ MAX77540,
+ MAX77541,
+};
+
+struct chip_info {
+ enum max7754x_ids id;
+ int n_devs;
+ const struct mfd_cell *devs;
+};
+
+struct regmap;
+struct regmap_irq_chip_data;
+struct i2c_client;
+
+struct max77541 {
+ const struct chip_info *chip;
+
+ struct regmap_irq_chip_data *irq_data;
+ struct regmap_irq_chip_data *irq_buck;
+ struct regmap_irq_chip_data *irq_topsys;
+ struct regmap_irq_chip_data *irq_adc;
+
+ struct i2c_client *i2c;
+ struct regmap *regmap;
+};
+
+#endif /* __MAX77541_MFD_H__ */