Re: [PATCH v8 06/11] mfd: max77650: new core mfd driver

From: Lee Jones
Date: Thu Apr 04 2019 - 03:15:51 EST

Next message: Mukesh Ojha: "[PATCH 0/5] Input: Use devm_platform_ioremap_resource()"
Previous message: Michal Hocko: "Re: [PATCH] mm:workingset use real time to judge activity of the file page"
In reply to: Bartosz Golaszewski: "[PATCH v8 06/11] mfd: max77650: new core mfd driver"
Next in thread: Bartosz Golaszewski: "Re: [PATCH v8 06/11] mfd: max77650: new core mfd driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, 03 Apr 2019, Bartosz Golaszewski wrote:

> From: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
>
> Add the core mfd driver for max77650 PMIC. We define five sub-devices
> for which the drivers will be added in subsequent patches.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
> ---
> drivers/mfd/Kconfig | 14 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/max77650.c | 234 +++++++++++++++++++++++++++++++++++
> include/linux/mfd/max77650.h | 59 +++++++++
> 4 files changed, 308 insertions(+)
> create mode 100644 drivers/mfd/max77650.c
> create mode 100644 include/linux/mfd/max77650.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 0ce2d8dfc5f1..ade04e124aa0 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -733,6 +733,20 @@ config MFD_MAX77620
> provides common support for accessing the device; additional drivers
> must be enabled in order to use the functionality of the device.
>
> +config MFD_MAX77650
> + tristate "Maxim MAX77650/77651 PMIC Support"
> + depends on I2C
> + depends on OF || COMPILE_TEST
> + select MFD_CORE
> + select REGMAP_I2C
> + help
> + Say Y here to add support for Maxim Semiconductor MAX77650 and
> + MAX77651 Power Management ICs. This is the core multifunction
> + driver for interacting with the device. The module name is
> + 'max77650'. Additional drivers can be enabled in order to use
> + the following functionalities of the device: GPIO, regulator,
> + charger, LED, onkey.
> +
> config MFD_MAX77686
> tristate "Maxim Semiconductor MAX77686/802 PMIC Support"
> depends on I2C
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index b4569ed7f3f3..5727d099c16f 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -155,6 +155,7 @@ obj-$(CONFIG_MFD_DA9150) += da9150-core.o
>
> obj-$(CONFIG_MFD_MAX14577) += max14577.o
> obj-$(CONFIG_MFD_MAX77620) += max77620.o
> +obj-$(CONFIG_MFD_MAX77650) += max77650.o
> obj-$(CONFIG_MFD_MAX77686) += max77686.o
> obj-$(CONFIG_MFD_MAX77693) += max77693.o
> obj-$(CONFIG_MFD_MAX77843) += max77843.o
> diff --git a/drivers/mfd/max77650.c b/drivers/mfd/max77650.c
> new file mode 100644
> index 000000000000..7a6c0a5cf602
> --- /dev/null
> +++ b/drivers/mfd/max77650.c
> @@ -0,0 +1,234 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (C) 2018 BayLibre SAS
> +// Author: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
> +//
> +// Core MFD driver for MAXIM 77650/77651 charger/power-supply.
> +// Programming manual: https://pdfserv.maximintegrated.com/en/an/AN6428.pdf
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/max77650.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +
> +#define MAX77650_INT_GPI_F_MSK BIT(0)
> +#define MAX77650_INT_GPI_R_MSK BIT(1)
> +#define MAX77650_INT_GPI_MSK \
> + (MAX77650_INT_GPI_F_MSK | MAX77650_INT_GPI_R_MSK)
> +#define MAX77650_INT_nEN_F_MSK BIT(2)
> +#define MAX77650_INT_nEN_R_MSK BIT(3)
> +#define MAX77650_INT_TJAL1_R_MSK BIT(4)
> +#define MAX77650_INT_TJAL2_R_MSK BIT(5)
> +#define MAX77650_INT_DOD_R_MSK BIT(6)
> +
> +#define MAX77650_INT_THM_MSK BIT(0)
> +#define MAX77650_INT_CHG_MSK BIT(1)
> +#define MAX77650_INT_CHGIN_MSK BIT(2)
> +#define MAX77650_INT_TJ_REG_MSK BIT(3)
> +#define MAX77650_INT_CHGIN_CTRL_MSK BIT(4)
> +#define MAX77650_INT_SYS_CTRL_MSK BIT(5)
> +#define MAX77650_INT_SYS_CNFG_MSK BIT(6)
> +
> +#define MAX77650_INT_GLBL_OFFSET 0
> +#define MAX77650_INT_CHG_OFFSET 1
> +
> +#define MAX77650_SBIA_LPM_MASK BIT(5)
> +#define MAX77650_SBIA_LPM_DISABLED 0x00
> +
> +enum {
> + MAX77650_INT_GPI,
> + MAX77650_INT_nEN_F,
> + MAX77650_INT_nEN_R,
> + MAX77650_INT_TJAL1_R,
> + MAX77650_INT_TJAL2_R,
> + MAX77650_INT_DOD_R,
> + MAX77650_INT_THM,
> + MAX77650_INT_CHG,
> + MAX77650_INT_CHGIN,
> + MAX77650_INT_TJ_REG,
> + MAX77650_INT_CHGIN_CTRL,
> + MAX77650_INT_SYS_CTRL,
> + MAX77650_INT_SYS_CNFG,
> +};
> +
> +static const struct resource max77650_charger_resources[] = {
> + DEFINE_RES_IRQ_NAMED(MAX77650_INT_CHG, "CHG"),
> + DEFINE_RES_IRQ_NAMED(MAX77650_INT_CHGIN, "CHGIN"),
> +};
> +
> +static const struct resource max77650_gpio_resources[] = {
> + DEFINE_RES_IRQ_NAMED(MAX77650_INT_GPI, "GPI"),
> +};
> +
> +static const struct resource max77650_onkey_resources[] = {
> + DEFINE_RES_IRQ_NAMED(MAX77650_INT_nEN_F, "nEN_F"),
> + DEFINE_RES_IRQ_NAMED(MAX77650_INT_nEN_R, "nEN_R"),
> +};
> +
> +static const struct mfd_cell max77650_cells[] = {
> + {
> + .name = "max77650-regulator",
> + .of_compatible = "maxim,max77650-regulator",

> + },
> + {

Nit: Place these all on the same line, like:

}, {

> + .name = "max77650-charger",
> + .of_compatible = "maxim,max77650-charger",
> + .resources = max77650_charger_resources,
> + .num_resources = ARRAY_SIZE(max77650_charger_resources),
> + },
> + {
> + .name = "max77650-gpio",
> + .of_compatible = "maxim,max77650-gpio",
> + .resources = max77650_gpio_resources,
> + .num_resources = ARRAY_SIZE(max77650_gpio_resources),
> + },
> + {
> + .name = "max77650-led",
> + .of_compatible = "maxim,max77650-led",
> + },
> + {
> + .name = "max77650-onkey",
> + .of_compatible = "maxim,max77650-onkey",
> + .resources = max77650_onkey_resources,
> + .num_resources = ARRAY_SIZE(max77650_onkey_resources),
> + },
> +};
> +
> +static const struct regmap_irq max77650_irqs[] = {
> + [MAX77650_INT_GPI] = {
> + .reg_offset = MAX77650_INT_GLBL_OFFSET,
> + .mask = MAX77650_INT_GPI_MSK,
> + .type = {
> + .type_falling_val = MAX77650_INT_GPI_F_MSK,
> + .type_rising_val = MAX77650_INT_GPI_R_MSK,
> + .types_supported = IRQ_TYPE_EDGE_BOTH,

Not really a fan of these silly double/triple tabs everywhere. I like
a straight line as much as anyone, but I'd probably remove them from
here.

> + },
> + },
> + REGMAP_IRQ_REG(MAX77650_INT_nEN_F,
> + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_nEN_F_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_nEN_R,
> + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_nEN_R_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_TJAL1_R,
> + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_TJAL1_R_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_TJAL2_R,
> + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_TJAL2_R_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_DOD_R,
> + MAX77650_INT_GLBL_OFFSET, MAX77650_INT_DOD_R_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_THM,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_THM_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_CHG,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_CHG_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_CHGIN,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_CHGIN_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_TJ_REG,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_TJ_REG_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_CHGIN_CTRL,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_CHGIN_CTRL_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_SYS_CTRL,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_SYS_CTRL_MSK),
> + REGMAP_IRQ_REG(MAX77650_INT_SYS_CNFG,
> + MAX77650_INT_CHG_OFFSET, MAX77650_INT_SYS_CNFG_MSK),
> +};
> +
> +static const struct regmap_irq_chip max77650_irq_chip = {
> + .name = "max77650-irq",
> + .irqs = max77650_irqs,
> + .num_irqs = ARRAY_SIZE(max77650_irqs),
> + .num_regs = 2,
> + .status_base = MAX77650_REG_INT_GLBL,
> + .mask_base = MAX77650_REG_INTM_GLBL,
> + .type_in_mask = true,
> + .type_invert = true,
> + .init_ack_masked = true,
> + .clear_on_unmask = true,
> +};
> +
> +static const struct regmap_config max77650_regmap_config = {
> + .name = "max77650",
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> +static int max77650_i2c_probe(struct i2c_client *i2c)
> +{
> + struct regmap_irq_chip_data *irq_data;
> + struct device *dev = &i2c->dev;
> + struct irq_domain *domain;
> + struct regmap *map;
> + unsigned int val;
> + int rv;
> +
> + map = devm_regmap_init_i2c(i2c, &max77650_regmap_config);
> + if (IS_ERR(map)) {
> + dev_err(dev, "unable to initialize i2c regmap\n");

"Unable to initialise I2C Regmap"

> + return PTR_ERR(map);
> + }
> +
> + rv = regmap_read(map, MAX77650_REG_CID, &val);
> + if (rv) {
> + dev_err(dev, "unable to read the CID from device\n");

"Unable to read Chip ID"

> + return rv;
> + }
> +
> + switch (MAX77650_CID_BITS(val)) {
> + case MAX77650_CID_77650A:
> + case MAX77650_CID_77650C:
> + case MAX77650_CID_77651A:
> + case MAX77650_CID_77651B:
> + break;
> + default:

So you're exiting silently here. What is the user going to think?

"Chip not supported - ID: %d"

> + return -ENODEV;
> + }
> +
> + /*
> + * This IC has a low-power mode which reduces the quiescent current
> + * consumption to ~5.6uA but is only suitable for systems consuming
> + * less than ~2mA. Since this is not likely the case even on
> + * linux-based wearables - keep the chip in normal power mode.
> + */
> + rv = regmap_update_bits(map,
> + MAX77650_REG_CNFG_GLBL,
> + MAX77650_SBIA_LPM_MASK,
> + MAX77650_SBIA_LPM_DISABLED);
> + if (rv) {
> + dev_err(dev, "unable to change the power mode\n");

"Unable ..."

> + return rv;
> + }
> +
> + rv = devm_regmap_add_irq_chip(dev, map, i2c->irq,
> + IRQF_ONESHOT | IRQF_SHARED, 0,
> + &max77650_irq_chip, &irq_data);
> + if (rv) {
> + dev_err(dev, "unable to add the regmap irq chip\n");

"Unable to add Regmap IRQ chip"

> + return rv;
> + }
> +
> + domain = regmap_irq_get_domain(irq_data);

No error checking?

> + return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> + max77650_cells, ARRAY_SIZE(max77650_cells),
> + NULL, 0, domain);
> +}
> +
> +static const struct of_device_id max77650_of_match[] = {
> + { .compatible = "maxim,max77650" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, max77650_of_match);
> +
> +static struct i2c_driver max77650_i2c_driver = {
> + .driver = {
> + .name = "max77650",
> + .of_match_table = of_match_ptr(max77650_of_match),
> + },
> + .probe_new = max77650_i2c_probe,

I wrote this years ago. Hasn't it gone away yet?

> +};
> +module_i2c_driver(max77650_i2c_driver);
> +
> +MODULE_DESCRIPTION("MAXIM 77650/77651 multi-function core driver");
> +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/max77650.h b/include/linux/mfd/max77650.h
> new file mode 100644
> index 000000000000..c809e211a8cd
> --- /dev/null
> +++ b/include/linux/mfd/max77650.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2018 BayLibre SAS
> + * Author: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
> + *
> + * Common definitions for MAXIM 77650/77651 charger/power-supply.
> + */
> +
> +#ifndef MAX77650_H
> +#define MAX77650_H
> +
> +#include <linux/bits.h>
> +
> +#define MAX77650_REG_INT_GLBL 0x00
> +#define MAX77650_REG_INT_CHG 0x01
> +#define MAX77650_REG_STAT_CHG_A 0x02
> +#define MAX77650_REG_STAT_CHG_B 0x03
> +#define MAX77650_REG_ERCFLAG 0x04
> +#define MAX77650_REG_STAT_GLBL 0x05
> +#define MAX77650_REG_INTM_GLBL 0x06
> +#define MAX77650_REG_INTM_CHG 0x07
> +#define MAX77650_REG_CNFG_GLBL 0x10
> +#define MAX77650_REG_CID 0x11
> +#define MAX77650_REG_CNFG_GPIO 0x12
> +#define MAX77650_REG_CNFG_CHG_A 0x18
> +#define MAX77650_REG_CNFG_CHG_B 0x19
> +#define MAX77650_REG_CNFG_CHG_C 0x1a
> +#define MAX77650_REG_CNFG_CHG_D 0x1b
> +#define MAX77650_REG_CNFG_CHG_E 0x1c
> +#define MAX77650_REG_CNFG_CHG_F 0x1d
> +#define MAX77650_REG_CNFG_CHG_G 0x1e
> +#define MAX77650_REG_CNFG_CHG_H 0x1f
> +#define MAX77650_REG_CNFG_CHG_I 0x20
> +#define MAX77650_REG_CNFG_SBB_TOP 0x28
> +#define MAX77650_REG_CNFG_SBB0_A 0x29
> +#define MAX77650_REG_CNFG_SBB0_B 0x2a
> +#define MAX77650_REG_CNFG_SBB1_A 0x2b
> +#define MAX77650_REG_CNFG_SBB1_B 0x2c
> +#define MAX77650_REG_CNFG_SBB2_A 0x2d
> +#define MAX77650_REG_CNFG_SBB2_B 0x2e
> +#define MAX77650_REG_CNFG_LDO_A 0x38
> +#define MAX77650_REG_CNFG_LDO_B 0x39
> +#define MAX77650_REG_CNFG_LED0_A 0x40
> +#define MAX77650_REG_CNFG_LED1_A 0x41
> +#define MAX77650_REG_CNFG_LED2_A 0x42
> +#define MAX77650_REG_CNFG_LED0_B 0x43
> +#define MAX77650_REG_CNFG_LED1_B 0x44
> +#define MAX77650_REG_CNFG_LED2_B 0x45
> +#define MAX77650_REG_CNFG_LED_TOP 0x46
> +
> +#define MAX77650_CID_MASK GENMASK(3, 0)
> +#define MAX77650_CID_BITS(_reg) (_reg & MAX77650_CID_MASK)
> +
> +#define MAX77650_CID_77650A 0x03
> +#define MAX77650_CID_77650C 0x0a
> +#define MAX77650_CID_77651A 0x06
> +#define MAX77650_CID_77651B 0x08
> +
> +#endif /* MAX77650_H */

--
Lee Jones [æçæ]
Linaro Services Technical Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Next message: Mukesh Ojha: "[PATCH 0/5] Input: Use devm_platform_ioremap_resource()"
Previous message: Michal Hocko: "Re: [PATCH] mm:workingset use real time to judge activity of the file page"
In reply to: Bartosz Golaszewski: "[PATCH v8 06/11] mfd: max77650: new core mfd driver"
Next in thread: Bartosz Golaszewski: "Re: [PATCH v8 06/11] mfd: max77650: new core mfd driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]