On Wed, 21 Feb 2024, James Ogletree wrote: > Introduce support for Cirrus Logic Device CS40L50: a > haptic driver with waveform memory, integrated DSP, > and closed-loop algorithms. > > The MFD component registers and initializes the device. > > Signed-off-by: James Ogletree <jogletre@xxxxxxxxxxxxxxxxxxxxx> > --- > MAINTAINERS | 2 + > drivers/mfd/Kconfig | 30 ++ > drivers/mfd/Makefile | 4 + > drivers/mfd/cs40l50-core.c | 531 ++++++++++++++++++++++++++++++++++++ > drivers/mfd/cs40l50-i2c.c | 69 +++++ > drivers/mfd/cs40l50-spi.c | 69 +++++ > include/linux/mfd/cs40l50.h | 142 ++++++++++ > 7 files changed, 847 insertions(+) > create mode 100644 drivers/mfd/cs40l50-core.c > create mode 100644 drivers/mfd/cs40l50-i2c.c > create mode 100644 drivers/mfd/cs40l50-spi.c > create mode 100644 include/linux/mfd/cs40l50.h Mostly fine, just a few nits. Assumingly this needs to go in via one tree (usually MFD). I can't do so until the other maintainers have provided Acks. [...] > +static struct regmap_irq_chip cs40l50_irq_chip = { > + .name = "CS40L50 IRQ Controller", > + > + .status_base = CS40L50_IRQ1_INT_1, > + .mask_base = CS40L50_IRQ1_MASK_1, > + .ack_base = CS40L50_IRQ1_INT_1, > + .num_regs = 22, > + > + .irqs = cs40l50_reg_irqs, > + .num_irqs = ARRAY_SIZE(cs40l50_reg_irqs), > + > + .runtime_pm = true, > +}; > + > +int cs40l50_dsp_write(struct device *dev, struct regmap *regmap, u32 val) > +{ > + int err, i; > + u32 ack; > + > + /* Device NAKs if exiting hibernation, so optionally retry here */ > + for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) { > + err = regmap_write(regmap, CS40L50_DSP_QUEUE, val); > + if (!err) > + break; > + > + usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100); > + } > + > + /* If we never wrote, don't bother checking for ACK */ "If the write never took place, no need to check for the ACK" > + if (i == CS40L50_DSP_TIMEOUT_COUNT) { > + dev_err(dev, "Timed out writing %#X to DSP: %d\n", val, err); > + return err; > + } > + > + err = regmap_read_poll_timeout(regmap, CS40L50_DSP_QUEUE, ack, !ack, > + CS40L50_DSP_POLL_US, > + CS40L50_DSP_POLL_US * CS40L50_DSP_TIMEOUT_COUNT); > + if (err) > + dev_err(dev, "DSP did not ack %#X: %d\n", val, err); > + > + return err; > +} > +EXPORT_SYMBOL_GPL(cs40l50_dsp_write); > + > +static const struct cs_dsp_region cs40l50_dsp_regions[] = { > + { .type = WMFW_HALO_PM_PACKED, .base = CS40L50_PMEM_0 }, > + { .type = WMFW_HALO_XM_PACKED, .base = CS40L50_XMEM_PACKED_0 }, > + { .type = WMFW_HALO_YM_PACKED, .base = CS40L50_YMEM_PACKED_0 }, > + { .type = WMFW_ADSP2_XM, .base = CS40L50_XMEM_UNPACKED24_0 }, > + { .type = WMFW_ADSP2_YM, .base = CS40L50_YMEM_UNPACKED24_0 }, > +}; > + > +static void cs40l50_dsp_remove(void *data) > +{ > + cs_dsp_remove((struct cs_dsp *)data); Is the cast required? Where is this function? > +} > + > +static const struct cs_dsp_client_ops cs40l50_client_ops; What's this for? Where is it used? In general, I'm not a fan of empty struct definitions like this. > +static int cs40l50_dsp_init(struct cs40l50 *cs40l50) > +{ > + int err; > + > + cs40l50->dsp.num = 1; > + cs40l50->dsp.type = WMFW_HALO; > + cs40l50->dsp.dev = cs40l50->dev; > + cs40l50->dsp.regmap = cs40l50->regmap; > + cs40l50->dsp.base = CS40L50_CORE_BASE; > + cs40l50->dsp.base_sysinfo = CS40L50_SYS_INFO_ID; > + cs40l50->dsp.mem = cs40l50_dsp_regions; > + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions); > + cs40l50->dsp.no_core_startstop = true; > + cs40l50->dsp.client_ops = &cs40l50_client_ops; > + > + err = cs_dsp_halo_init(&cs40l50->dsp); > + if (err) > + return err; > + > + return devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_remove, > + &cs40l50->dsp); > +} [...] > +++ b/drivers/mfd/cs40l50-i2c.c > @@ -0,0 +1,69 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * CS40L50 Advanced Haptic Driver with waveform memory, > + * integrated DSP, and closed-loop algorithms > + * > + * Copyright 2024 Cirrus Logic, Inc. > + * > + * Author: James Ogletree <james.ogletree@xxxxxxxxxx> > + */ > + > +#include <linux/i2c.h> > +#include <linux/mfd/cs40l50.h> > + > +static int cs40l50_i2c_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct cs40l50 *cs40l50; > + > + cs40l50 = devm_kzalloc(dev, sizeof(*cs40l50), GFP_KERNEL); > + if (!cs40l50) > + return -ENOMEM; > + > + i2c_set_clientdata(client, cs40l50); > + > + cs40l50->regmap = devm_regmap_init_i2c(client, &cs40l50_regmap); > + if (IS_ERR(cs40l50->regmap)) > + return dev_err_probe(cs40l50->dev, PTR_ERR(cs40l50->regmap), > + "Failed to initialize register map\n"); > + > + cs40l50->dev = dev; > + cs40l50->irq = client->irq; > + > + return cs40l50_probe(cs40l50); > +} > + > +static void cs40l50_i2c_remove(struct i2c_client *client) > +{ > + struct cs40l50 *cs40l50 = i2c_get_clientdata(client); > + > + cs40l50_remove(cs40l50); > +} > + > +static const struct i2c_device_id cs40l50_id_i2c[] = { > + {"cs40l50"}, Spaces either side of the "s > + {} > +}; > +MODULE_DEVICE_TABLE(i2c, cs40l50_id_i2c); > + > +static const struct of_device_id cs40l50_of_match[] = { > + { .compatible = "cirrus,cs40l50" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, cs40l50_of_match); > + > +static struct i2c_driver cs40l50_i2c_driver = { > + .driver = { > + .name = "cs40l50", > + .of_match_table = cs40l50_of_match, > + .pm = pm_ptr(&cs40l50_pm_ops), > + }, > + .id_table = cs40l50_id_i2c, > + .probe = cs40l50_i2c_probe, > + .remove = cs40l50_i2c_remove, > +}; > +module_i2c_driver(cs40l50_i2c_driver); > + > +MODULE_DESCRIPTION("CS40L50 I2C Driver"); > +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@xxxxxxxxxx>"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/mfd/cs40l50-spi.c b/drivers/mfd/cs40l50-spi.c > new file mode 100644 > index 000000000000..9e18bb74eae0 > --- /dev/null > +++ b/drivers/mfd/cs40l50-spi.c > @@ -0,0 +1,69 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * CS40L50 Advanced Haptic Driver with waveform memory, > + * integrated DSP, and closed-loop algorithms > + * > + * Copyright 2024 Cirrus Logic, Inc. > + * > + * Author: James Ogletree <james.ogletree@xxxxxxxxxx> > + */ > + > +#include <linux/mfd/cs40l50.h> > +#include <linux/spi/spi.h> > + > +static int cs40l50_spi_probe(struct spi_device *spi) > +{ > + struct cs40l50 *cs40l50; > + struct device *dev = &spi->dev; > + > + cs40l50 = devm_kzalloc(dev, sizeof(*cs40l50), GFP_KERNEL); > + if (!cs40l50) > + return -ENOMEM; > + > + spi_set_drvdata(spi, cs40l50); > + > + cs40l50->regmap = devm_regmap_init_spi(spi, &cs40l50_regmap); > + if (IS_ERR(cs40l50->regmap)) > + return dev_err_probe(cs40l50->dev, PTR_ERR(cs40l50->regmap), > + "Failed to initialize register map\n"); > + > + cs40l50->dev = dev; > + cs40l50->irq = spi->irq; > + > + return cs40l50_probe(cs40l50); > +} > + > +static void cs40l50_spi_remove(struct spi_device *spi) > +{ > + struct cs40l50 *cs40l50 = spi_get_drvdata(spi); > + > + cs40l50_remove(cs40l50); > +} > + > +static const struct spi_device_id cs40l50_id_spi[] = { > + {"cs40l50"}, As above. > + {} > +}; > +MODULE_DEVICE_TABLE(spi, cs40l50_id_spi); > + > +static const struct of_device_id cs40l50_of_match[] = { > + { .compatible = "cirrus,cs40l50" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, cs40l50_of_match); > + > +static struct spi_driver cs40l50_spi_driver = { > + .driver = { > + .name = "cs40l50", > + .of_match_table = cs40l50_of_match, > + .pm = pm_ptr(&cs40l50_pm_ops), > + }, > + .id_table = cs40l50_id_spi, > + .probe = cs40l50_spi_probe, > + .remove = cs40l50_spi_remove, > +}; > +module_spi_driver(cs40l50_spi_driver); > + > +MODULE_DESCRIPTION("CS40L50 SPI Driver"); > +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@xxxxxxxxxx>"); > +MODULE_LICENSE("GPL"); > diff --git a/include/linux/mfd/cs40l50.h b/include/linux/mfd/cs40l50.h > new file mode 100644 > index 000000000000..d855784a88a9 > --- /dev/null > +++ b/include/linux/mfd/cs40l50.h > @@ -0,0 +1,142 @@ > +/* SPDX-License-Identifier: GPL-2.0 > + * > + * CS40L50 Advanced Haptic Driver with waveform memory, > + * integrated DSP, and closed-loop algorithms > + * > + * Copyright 2024 Cirrus Logic, Inc. > + * > + * Author: James Ogletree <james.ogletree@xxxxxxxxxx> > + */ > + > +#ifndef __CS40L50_H__ > +#define __CS40L50_H__ MFD_ And at the bottom of this file. [...] > +int cs40l50_dsp_write(struct device *dev, struct regmap *regmap, u32 val); > +int cs40l50_probe(struct cs40l50 *cs40l50); > +int cs40l50_remove(struct cs40l50 *cs40l50); > + > +extern const struct regmap_config cs40l50_regmap; > +extern const struct dev_pm_ops cs40l50_pm_ops; > + > +#endif /* __CS40L50_H__ */ Here. -- Lee Jones [李琼斯]