On Thu, 14 Aug 2025, Nuno Sá via B4 Relay wrote: > From: Nuno Sá <nuno.sa@xxxxxxxxxx> > > The LTC4283 is a negative voltage hot swap controller that drives an > external N-channel MOSFET to allow a board to be safely inserted and > removed from a live backplane. > > Main usage is as an Hardware Monitoring device. However, it has up to 8 > pins that can be configured and used as GPIOs and hence, the device can > also be a GPIO controller (and so being added as MFD device). > > Signed-off-by: Nuno Sá <nuno.sa@xxxxxxxxxx> > --- > MAINTAINERS | 2 + > drivers/mfd/Kconfig | 11 ++++ > drivers/mfd/Makefile | 1 + > drivers/mfd/ltc4283.c | 140 ++++++++++++++++++++++++++++++++++++++++++++ > include/linux/mfd/ltc4283.h | 33 +++++++++++ > 5 files changed, 187 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 413bb77d5eebe2b51aa9c3af86e7cfd5ab142044..b5f4f1c41c64b738d57c1fb5552a60b4c6b9985c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14506,6 +14506,8 @@ L: linux-hwmon@xxxxxxxxxxxxxxx > L: linux-gpio@xxxxxxxxxxxxxxx > S: Supported > F: Documentation/devicetree/bindings/mfd/adi,ltc4283.yaml > +F: drivers/mfd/ltc4283.c > +F: include/linux/mfd/ltc4283.h > > LTC4286 HARDWARE MONITOR DRIVER > M: Delphine CC Chiu <Delphine_CC_Chiu@xxxxxxxxxx> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 425c5fba6cb1e7848dcea05bd77c729a71d48e2c..ec3e02d40fd17a0bba29e3157723055feedebd11 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -900,6 +900,17 @@ config MFD_MAX14577 > additional drivers must be enabled in order to use the functionality > of the device. > > +config MFD_LTC4283 > + tristate "LTC4283 Hot Swap Controller" > + depends on I2C > + select MFD_CORE > + select REGMAP_I2C > + help > + This enables support for the LTC4283 Negative Voltage Hot Swap > + Controller. 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_MAX77541 > tristate "Analog Devices MAX77541/77540 PMIC Support" > depends on I2C=y > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index f7bdedd5a66d16bf8ccee0da1236a441e6f085b0..4a3bcd30ab7e12aed4bc5e48294500c77cb61aa7 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -23,6 +23,7 @@ obj-$(CONFIG_MFD_EXYNOS_LPASS) += exynos-lpass.o > obj-$(CONFIG_MFD_GATEWORKS_GSC) += gateworks-gsc.o > obj-$(CONFIG_MFD_MACSMC) += macsmc.o > > +obj-$(CONFIG_MFD_LTC4283) += ltc4283.o > obj-$(CONFIG_MFD_TI_LP873X) += lp873x.o > obj-$(CONFIG_MFD_TI_LP87565) += lp87565.o > obj-$(CONFIG_MFD_TI_AM335X_TSCADC) += ti_am335x_tscadc.o > diff --git a/drivers/mfd/ltc4283.c b/drivers/mfd/ltc4283.c > new file mode 100644 > index 0000000000000000000000000000000000000000..a45c01278f884630984790c922c057c4f2db9c82 > --- /dev/null > +++ b/drivers/mfd/ltc4283.c > @@ -0,0 +1,140 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Analog Devices LTC4283 I2C Negative Voltage Hot Swap Controller > + * > + * Copyright 2025 Analog Devices Inc. > + */ > +#include <linux/bitmap.h> > +#include <linux/bitops.h> > +#include <linux/device.h> > +#include <linux/i2c.h> > +#include <linux/mfd/core.h> > +#include <linux/module.h> > +#include <linux/mod_devicetable.h> > +#include <linux/regmap.h> > +#include <linux/property.h> Alphabetical. > + > +#include <linux/mfd/ltc4283.h> > + > +static const struct mfd_cell ltc4283_cells[] = { > + MFD_CELL_OF("ltc4283-hwmon", NULL, NULL, 0, 0, "adi,ltc4283-hwmon"), > + MFD_CELL_OF("ltc4283-gpio", NULL, NULL, 0, 0, "adi,ltc4283-gpio"), > +}; > + > +static bool ltc4283_writable_reg(struct device *dev, unsigned int reg) > +{ > + switch (reg) { > + case 0x00 ... 0x03: Define these magic numbers. > + return false; > + case 0x3c: > + return false; > + case 0x86 ... 0x8f: > + return false; > + case 0x91 ... 0xa1: > + return false; > + case 0xa3: > + return false; > + case 0xac: > + return false; > + case 0xf1 ... 0xff: > + return false; > + default: > + return true; > + } > +} > + > +static const struct regmap_config ltc4283_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = 0xff, > + .writeable_reg = ltc4283_writable_reg, > +}; > + > +static int ltc4283_get_gpio_pins(struct i2c_client *client, u32 *n_cells) Why not do this in the GPIO driver? > +{ > + struct device *dev = &client->dev; > + u32 pins[LTC4283_GPIO_MAX], pin; > + unsigned long *gpio_mask; > + int n_pins, ret; > + > + /* > + * The device has up to 8 pins that can be configured either as GPIOS or > + * for monitoring purposes. Both gpio and hwmon devices need to have > + * this information in order to do proper validations and > + * configurations. Hence, this property needs to be in the top level > + * device. > + */ > + n_pins = device_property_count_u32(dev, "adi,gpio-pins"); > + if (n_pins < 0) > + return 0; > + if (n_pins >= LTC4283_GPIO_MAX) > + return dev_err_probe(dev, -EINVAL, "Too many GPIO pins specified (%d), max is %d\n", > + n_pins, LTC4283_GPIO_MAX); > + > + ret = device_property_read_u32_array(dev, "adi,gpio-pins", pins, n_pins); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to read GPIO pins\n"); > + > + gpio_mask = devm_bitmap_zalloc(dev, LTC4283_GPIO_MAX, GFP_KERNEL); > + if (!gpio_mask) > + return -ENOMEM; > + > + for (pin = 0; pin < n_pins; pin++) { > + if (pins[pin] >= LTC4283_GPIO_MAX) > + return dev_err_probe(dev, -EINVAL, > + "Invalid GPIO pin specified (%u), max is %d\n", > + pins[pin], LTC4283_GPIO_MAX); > + > + __set_bit(pins[pin], gpio_mask); > + } > + > + /* Add the GPIO cell */ > + *n_cells += 1; Just register it anyway and have the GPIO driver error out if it doesn't have the right cells / properties. > + i2c_set_clientdata(client, gpio_mask); > + > + return 0; > +} > + > +static int ltc4283_probe(struct i2c_client *client) > +{ > + u32 n_cells = ARRAY_SIZE(ltc4283_cells) - 1; > + struct regmap *regmap; > + int ret; > + > + regmap = devm_regmap_init_i2c(client, <c4283_regmap_config); > + if (IS_ERR(regmap)) > + return PTR_ERR(regmap); > + > + ret = ltc4283_get_gpio_pins(client, &n_cells); > + if (ret) > + return ret; > + > + return devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO, > + ltc4283_cells, n_cells, NULL, 0, NULL); > +} > + > +static const struct of_device_id ltc4283_of_match[] = { > + { .compatible = "adi,ltc4283" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ltc4283_of_match); > + > +static const struct i2c_device_id ltc4283_i2c_id[] = { > + { "ltc4283" }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, ltc4283_i2c_id); > + > +static struct i2c_driver ltc4283_driver = { > + .driver = { > + .name = "ltc4283", > + .of_match_table = ltc4283_of_match, > + }, > + .probe = ltc4283_probe, > + .id_table = ltc4283_i2c_id, > +}; > +module_i2c_driver(ltc4283_driver); > + > +MODULE_AUTHOR("Nuno Sá <nuno.sa@xxxxxxxxxx>"); > +MODULE_DESCRIPTION("LTC4283 MFD I2C driver"); It's not an MFD anything! And the communication method is meaningless. You can use "Core driver" instead. > +MODULE_LICENSE("GPL"); > diff --git a/include/linux/mfd/ltc4283.h b/include/linux/mfd/ltc4283.h > new file mode 100644 > index 0000000000000000000000000000000000000000..153594009c07b296ce5743e5e817e96464d81cb3 > --- /dev/null > +++ b/include/linux/mfd/ltc4283.h > @@ -0,0 +1,33 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Analog Devices LTC4283 I2C Negative Voltage Hot Swap Controller > + * > + * Copyright 2025 Analog Devices Inc. > + */ > + > +#ifndef __MFD_LTC4283_H_ > +#define __MFD_LTC4283_H_ > + > +#include <linux/bitops.h> > +#include <linux/bits.h> > + > +/* > + * We can have up to 8 gpios. 4 PGIOs and 4 ADIOs. PGIOs start at index 4 in the > + * gpios mask. > + */ > +#define LTC4283_PGIOX_START_NR 4 > + > +#define LTC4283_PGIO_CONFIG 0x10 > +#define LTC4283_PGIO_CFG_MASK(pin) \ > + GENMASK(((pin) - LTC4283_PGIOX_START_NR) * 2 + 1, (((pin) - LTC4283_PGIOX_START_NR) * 2)) What do all of these numbers mean? Can you define them? > +#define LTC4283_PGIO_CONFIG_2 0x11 > +#define LTC4283_ADC_MASK GENMASK(2, 0) > +#define LTC4283_PGIO_OUT_MASK(pin) BIT(4 + (pin)) > + > +#define LTC4283_GPIO_MAX 8 > + > +/* Non-constant mask variant of FIELD_GET() and FIELD_PREP() */ Why? > +#define field_get(_mask, _reg) (((_reg) & (_mask)) >> (ffs(_mask) - 1)) > +#define field_prep(_mask, _val) (((_val) << (ffs(_mask) - 1)) & (_mask)) > + > +#endif > > -- > 2.50.1 > > -- Lee Jones [李琼斯]