| Message ID | 20230809191032.820271-3-james.ogletree@cirrus.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for CS40L50 | expand |
On 09/08/2023 21:10, James Ogletree wrote: > Introduce support for Cirrus Logic Device CS40L50: a > haptics driver with waveform memory DSP and closed-loop > algorithms. > > Signed-off-by: James Ogletree <james.ogletree@cirrus.com> ... > + > +static int cs40l50_cs_dsp_init(struct cs40l50_private *cs40l50) > +{ > + cs40l50->dsp.num = 1; > + cs40l50->dsp.type = WMFW_HALO; > + cs40l50->dsp.dev = cs40l50->dev; > + cs40l50->dsp.regmap = cs40l50->regmap; > + cs40l50->dsp.base = CS40L50_DSP1_CORE_BASE; > + cs40l50->dsp.base_sysinfo = CS40L50_DSP1_SYS_INFO_ID; > + cs40l50->dsp.mem = cs40l50_dsp_regions; > + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions); > + cs40l50->dsp.lock_regions = 0xFFFFFFFF; > + cs40l50->dsp.no_core_startstop = true; > + cs40l50->dsp.client_ops = &cs40l50_cs_dsp_client_ops; > + > + return cs_dsp_halo_init(&cs40l50->dsp); > +} > + > +int cs40l50_probe(struct cs40l50_private *cs40l50) > +{ > + int error, i, irq; > + u32 val; > + > + mutex_init(&cs40l50->lock); > + > + error = devm_regulator_bulk_get(cs40l50->dev, ARRAY_SIZE(cs40l50_supplies), > + cs40l50_supplies); > + if (error) > + return dev_err_probe(cs40l50->dev, error, "Failed to request supplies\n"); > + > + error = regulator_bulk_enable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies); > + if (error) > + return dev_err_probe(cs40l50->dev, error, "Failed to enable supplies\n"); > + > + cs40l50->reset_gpio = devm_gpiod_get_optional(cs40l50->dev, "reset", GPIOD_OUT_HIGH); None of the lines above or below seem to be wrapped according to Linux coding style (80). > + if (IS_ERR(cs40l50->reset_gpio)) { > + error = PTR_ERR(cs40l50->reset_gpio); > + goto err; Why do you disable IRQ now? I asked to test this. Your entire cleanup path is: 1. not tested. 2. buggy. 3. done differently than Linux style. Use proper cleanup calls and multiple goto labels. > + } > + > + usleep_range(CS40L50_MIN_RESET_PULSE_WIDTH_US, CS40L50_MIN_RESET_PULSE_WIDTH_US + 100); > + > + gpiod_set_value_cansleep(cs40l50->reset_gpio, 0); > + > + usleep_range(CS40L50_CP_READY_DELAY_US, CS40L50_CP_READY_DELAY_US + 1000); > + > + for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) { > + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_HALO_STATE, &val); > + if (!error && val < 0xFFFF && val >= CS40L50_HALO_STATE_BOOT_DONE) > + break; > + > + usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100); > + } > + > + if (i == CS40L50_DSP_TIMEOUT_COUNT) { > + dev_err(cs40l50->dev, "Firmware boot failed: %d, halo state = %#x\n", error, val); > + goto err; > + } > + > + cs40l50->vibe_workqueue = alloc_ordered_workqueue("cs40l50_workqueue", WQ_HIGHPRI); > + if (!cs40l50->vibe_workqueue) { > + error = -ENOMEM; > + goto err; > + } > + > + INIT_WORK(&cs40l50->vibe_start_work, cs40l50_vibe_start_worker); > + INIT_WORK(&cs40l50->vibe_stop_work, cs40l50_vibe_stop_worker); > + > + error = cs40l50_cs_dsp_init(cs40l50); > + if (error) > + goto err; > + > + error = cs40l50_input_init(cs40l50); > + if (error) > + goto err; > + > + error = cs40l50_patch_firmware(cs40l50); > + if (error) > + goto err; > + > + error = cs40l50_pseq_init(cs40l50); > + if (error) > + goto err; > + > + error = cs40l50_config_bst(cs40l50); > + if (error) > + goto err; > + > + error = devm_regmap_add_irq_chip(cs40l50->dev, cs40l50->regmap, cs40l50->irq, > + IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW, 0, &cs40l50_regmap_irq_chip, > + &cs40l50->irq_data); > + if (error) { > + dev_err(cs40l50->dev, "Failed to register IRQ chip: %d\n", error); > + goto err; > + } > + > + for (i = 0; i < ARRAY_SIZE(cs40l50_irqs); i++) { > + irq = regmap_irq_get_virq(cs40l50->irq_data, cs40l50_irqs[i].irq); > + if (irq < 0) { > + dev_err(cs40l50->dev, "Failed to get %s\n", cs40l50_irqs[i].name); > + error = irq; > + goto err; > + } > + > + error = devm_request_threaded_irq(cs40l50->dev, irq, NULL, cs40l50_irqs[i].handler, > + IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW, > + cs40l50_irqs[i].name, cs40l50); > + if (error) { > + dev_err(cs40l50->dev, "Failed to request IRQ %s: %d\n", > + cs40l50_irqs[i].name, error); > + goto err; > + } > + } > + > + return 0; > + > +err: > + cs40l50_remove(cs40l50); > + > + return error; > +} > +EXPORT_SYMBOL_GPL(cs40l50_probe); > + > +int cs40l50_remove(struct cs40l50_private *cs40l50) > +{ > + disable_irq(cs40l50->irq); > + > + if (cs40l50->dsp.booted) > + cs_dsp_power_down(&cs40l50->dsp); > + if (&cs40l50->dsp) > + cs_dsp_remove(&cs40l50->dsp); > + > + if (cs40l50->vibe_workqueue) { > + flush_workqueue(cs40l50->vibe_workqueue); > + destroy_workqueue(cs40l50->vibe_workqueue); > + } > + > + gpiod_set_value_cansleep(cs40l50->reset_gpio, 1); > + regulator_bulk_disable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(cs40l50_remove); > + > +MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver"); > +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>"); Nothing improved here. Don't send new version immediately after responding to my comment, but allow discussion. Best regards, Krzysztof
On Wed, Aug 09, 2023 at 07:10:28PM +0000, James Ogletree wrote: > Introduce support for Cirrus Logic Device CS40L50: a > haptics driver with waveform memory DSP and closed-loop > algorithms. > > Signed-off-by: James Ogletree <james.ogletree@cirrus.com> > --- > MAINTAINERS | 2 + > drivers/input/misc/Kconfig | 33 + > drivers/input/misc/Makefile | 3 + > drivers/input/misc/cs40l50-i2c.c | 67 ++ > drivers/input/misc/cs40l50-spi.c | 67 ++ > drivers/input/misc/cs40l50.c | 1008 ++++++++++++++++++++++++++++++ > include/linux/input/cs40l50.h | 321 ++++++++++ Is this part not going to support streaming at some point? Fine if not, but it seems likely to me and as such we should probably follow the l26 stuff and make this an MFD from the start, even if we haven't implemented the audio bits. > + > +static int cs40l50_pseq_write(struct cs40l50_private *cs40l50, u32 addr, u32 data) > +{ > + > +static int cs40l50_owt_upload(struct cs40l50_private *cs40l50, s16 *in_data, u32 in_data_nibbles) > +{ These pseq and OWT bits, could they be shared with l26? Definitely worth syncing with those guys, my assumption is the wavetable/pseq won't have changed much and it might be nice to factor these bits out into some library code that both drivers can use. > +err_free: > + if (is_new) > + error ? kfree(new_effect) : list_add(&new_effect->list, &cs40l50->effect_head); This is a bit of an excessive use for a ternary, just make it an if. > +static int cs40l50_erase_effect(struct input_dev *dev, int effect_id) > +{ > + int error = 0; > + struct cs40l50_private *cs40l50 = input_get_drvdata(dev); > + struct cs40l50_effect *effect, *effect_owt; > + > + mutex_lock(&cs40l50->lock); > + > + effect = cs40l50_find_effect(cs40l50, dev->ff->effects[effect_id].id); > + if (!effect) { > + error = -EINVAL; > + goto err_mutex; > + } > + > + if (effect->mapping != CS40L50_GPIO_MAPPING_INVALID) { > + error = cs40l50_dsp_write(cs40l50, effect->mapping, CS40L50_GPI_DISABLE); > + if (error) > + goto err_mutex; > + } > + > + if (effect->wvfrm_bank == CS40L50_WVFRM_BANK_OWT) { > + error = cs40l50_dsp_write(cs40l50, CS40L50_DSP_VIRTUAL1_MBOX_1, > + CS40L50_MBOX_CMD_OWT_DELETE | effect->trigger_index); > + if (error) > + goto err_mutex; > + > + list_for_each_entry(effect_owt, &cs40l50->effect_head, list) { > + if (effect_owt->wvfrm_bank == CS40L50_WVFRM_BANK_OWT && > + effect_owt->trigger_index > effect->trigger_index) > + effect_owt->trigger_index--; > + } > + } > + > + list_del(&effect->list); > + kfree(effect); > +err_mutex: > + mutex_unlock(&cs40l50->lock); > + > + return error; > +} I seem to remember add/erase needed to get pushed to the work queue too, not because the framework might call it in atomic context, but in order to ensure ordering with the start/stops. > +int cs40l50_probe(struct cs40l50_private *cs40l50) > +{ > + int error, i, irq; > + u32 val; > + > + mutex_init(&cs40l50->lock); > + > + error = devm_regulator_bulk_get(cs40l50->dev, ARRAY_SIZE(cs40l50_supplies), > + cs40l50_supplies); > + if (error) > + return dev_err_probe(cs40l50->dev, error, "Failed to request supplies\n"); > + > + error = regulator_bulk_enable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies); > + if (error) > + return dev_err_probe(cs40l50->dev, error, "Failed to enable supplies\n"); > + > + cs40l50->reset_gpio = devm_gpiod_get_optional(cs40l50->dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(cs40l50->reset_gpio)) { > + error = PTR_ERR(cs40l50->reset_gpio); > + goto err; > + } > + > + usleep_range(CS40L50_MIN_RESET_PULSE_WIDTH_US, CS40L50_MIN_RESET_PULSE_WIDTH_US + 100); > + > + gpiod_set_value_cansleep(cs40l50->reset_gpio, 0); > + > + usleep_range(CS40L50_CP_READY_DELAY_US, CS40L50_CP_READY_DELAY_US + 1000); > + > + for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) { > + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_HALO_STATE, &val); > + if (!error && val < 0xFFFF && val >= CS40L50_HALO_STATE_BOOT_DONE) > + break; > + > + usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100); > + } > + > + if (i == CS40L50_DSP_TIMEOUT_COUNT) { > + dev_err(cs40l50->dev, "Firmware boot failed: %d, halo state = %#x\n", error, val); > + goto err; > + } > + > + cs40l50->vibe_workqueue = alloc_ordered_workqueue("cs40l50_workqueue", WQ_HIGHPRI); > + if (!cs40l50->vibe_workqueue) { > + error = -ENOMEM; > + goto err; > + } > + > + INIT_WORK(&cs40l50->vibe_start_work, cs40l50_vibe_start_worker); > + INIT_WORK(&cs40l50->vibe_stop_work, cs40l50_vibe_stop_worker); > + > + error = cs40l50_cs_dsp_init(cs40l50); > + if (error) > + goto err; > + > + error = cs40l50_input_init(cs40l50); > + if (error) > + goto err; > + > + error = cs40l50_patch_firmware(cs40l50); > + if (error) > + goto err; > + Doing this from probe, with a sync firmware load, can be a little problematic as the firmware is often not available if the driver is builtin rather than a module. Fairly quick review as I am on holiday at the moment, but will check back next week. Thanks, Charles
Hi James, Great work as always; just some minor comments throughout. On Wed, Aug 09, 2023 at 07:10:28PM +0000, James Ogletree wrote: > Introduce support for Cirrus Logic Device CS40L50: a > haptics driver with waveform memory DSP and closed-loop > algorithms. From my extremely naive point of view, some of the code that follows bares resemblance to the recently reviewed L26. My assumption is that these devices are different enough in nature to warrant completely different drivers; is that accurate? One reason for asking is because the L26 driver included a cornucopia of power-management overhead, yet we see none of that here. Assuming both L50 and L26 are built around the same Halo DSP, why is there such a fundamental difference between the two in terms of power management? To that end, how does this driver handle hibernation? Is hibernation not supported, or do we simply defer to the DSP? In the case of the latter, why is L50 given this freedom but not L26? This is not a suggestion to change anything; it's just to help set the context and provide more background information. However, I'm curious whether you've considered consolidating both L26 and L50 into a single driver, especially given recent discussion around promoting L26 into an MFD with both input and codec children. Would L50 ever support audio-to -haptics over I2S? > > Signed-off-by: James Ogletree <james.ogletree@cirrus.com> > --- > MAINTAINERS | 2 + > drivers/input/misc/Kconfig | 33 + > drivers/input/misc/Makefile | 3 + > drivers/input/misc/cs40l50-i2c.c | 67 ++ > drivers/input/misc/cs40l50-spi.c | 67 ++ > drivers/input/misc/cs40l50.c | 1008 ++++++++++++++++++++++++++++++ > include/linux/input/cs40l50.h | 321 ++++++++++ > 7 files changed, 1501 insertions(+) > create mode 100644 drivers/input/misc/cs40l50-i2c.c > create mode 100644 drivers/input/misc/cs40l50-spi.c > create mode 100644 drivers/input/misc/cs40l50.c > create mode 100644 include/linux/input/cs40l50.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 2869dd2dbbcf..e82d03293383 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2088,6 +2088,8 @@ M: Ben Bright <ben.bright@cirrus.com> > L: patches@opensource.cirrus.com > S: Supported > F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml > +F: drivers/input/misc/cs40l* > +F: include/linux/input/cs40l* > > ARM/CLKDEV SUPPORT > M: Russell King <linux@armlinux.org.uk> > diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig > index c47eecc6f83b..c1c1572ec39c 100644 > --- a/drivers/input/misc/Kconfig > +++ b/drivers/input/misc/Kconfig > @@ -140,6 +140,39 @@ config INPUT_BMA150 > To compile this driver as a module, choose M here: the > module will be called bma150. > > +config INPUT_CS40L50 > + tristate "Cirrus Logic CS40L50 Haptic Driver support" > + select CS_DSP > + select REGMAP_IRQ > + help > + Say Y here to enable support for CS40L50 boosted haptic > + driver. > + > + To compile the driver as a module choose M here: the module > + will be called cs40l50. > + > +config INPUT_CS40L50_I2C > + tristate "Support I2C bus connection" > + depends on INPUT_CS40L50 && I2C > + select REGMAP_I2C > + help > + Say Y here if you have Cirrus Logic's CS40L50 connected > + to an I2C bus. > + > + To compile the driver as a module choose M here: the > + module will be called cs40l50_i2c. > + > +config INPUT_CS40L50_SPI > + tristate "Support SPI bus connection" > + depends on INPUT_CS40L50 && SPI > + select REGMAP_SPI > + help > + Say Y here if you have Cirrus Logic's CS40L50 connected > + to a SPI bus. > + > + To compile the driver as a module choose M here: the > + module will be called cs40l50_spi. > + > config INPUT_E3X0_BUTTON > tristate "NI Ettus Research USRP E3xx Button support." > default n > diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile > index 04296a4abe8e..77f34a33c364 100644 > --- a/drivers/input/misc/Makefile > +++ b/drivers/input/misc/Makefile > @@ -28,6 +28,9 @@ obj-$(CONFIG_INPUT_CMA3000) += cma3000_d0x.o > obj-$(CONFIG_INPUT_CMA3000_I2C) += cma3000_d0x_i2c.o > obj-$(CONFIG_INPUT_COBALT_BTNS) += cobalt_btns.o > obj-$(CONFIG_INPUT_CPCAP_PWRBUTTON) += cpcap-pwrbutton.o > +obj-$(CONFIG_INPUT_CS40L50) += cs40l50.o > +obj-$(CONFIG_INPUT_CS40L50_I2C) += cs40l50-i2c.o > +obj-$(CONFIG_INPUT_CS40L50_SPI) += cs40l50-spi.o > obj-$(CONFIG_INPUT_DA7280_HAPTICS) += da7280.o > obj-$(CONFIG_INPUT_DA9052_ONKEY) += da9052_onkey.o > obj-$(CONFIG_INPUT_DA9055_ONKEY) += da9055_onkey.o > diff --git a/drivers/input/misc/cs40l50-i2c.c b/drivers/input/misc/cs40l50-i2c.c > new file mode 100644 > index 000000000000..6ebe1b51990b > --- /dev/null > +++ b/drivers/input/misc/cs40l50-i2c.c > @@ -0,0 +1,67 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * CS40L50 Advanced Haptic Driver with Waveform Memory DSP > + * and Closed-Loop Algorithms Nit: the capitalization and general sentence structure here is hard to follow; maybe something like "... with DSP-based waveform memory and closed-loop algorithms" would make more sense? > + * > + * Copyright 2023 Cirrus Logic, Inc. > + * > + */ > +#include <linux/i2c.h> > +#include <linux/input/cs40l50.h> > + > +static const struct i2c_device_id cs40l50_id_i2c[] = { > + {"cs40l50", 0}, > + {} > +}; > +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 int cs40l50_i2c_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct cs40l50_private *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"); Nit: line breaks tend to be more readable when aligned against the parenthesis as follows: if (...) return dev_err_probe(..., ...); At least, it's much more common. > + > + cs40l50->dev = dev; > + cs40l50->irq = client->irq; > + > + return cs40l50_probe(cs40l50); > +} > + > +static void cs40l50_i2c_remove(struct i2c_client *client) > +{ > + struct cs40l50_private *cs40l50 = i2c_get_clientdata(client); > + > + cs40l50_remove(cs40l50); > +} > + > +static struct i2c_driver cs40l50_i2c_driver = { > + .driver = { > + .name = "cs40l50", > + .of_match_table = cs40l50_of_match, > + }, > + .id_table = cs40l50_id_i2c, > + .probe_new = cs40l50_i2c_probe, Mainline has moved back to .probe now; .probe_new is becoming depracated. > + .remove = cs40l50_i2c_remove, > +}; > + > +module_i2c_driver(cs40l50_i2c_driver); > + > +MODULE_DESCRIPTION("CS40L50 I2C Driver"); > +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/input/misc/cs40l50-spi.c b/drivers/input/misc/cs40l50-spi.c > new file mode 100644 > index 000000000000..5fcf3ba137f0 > --- /dev/null > +++ b/drivers/input/misc/cs40l50-spi.c > @@ -0,0 +1,67 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * CS40L50 Advanced Haptic Driver with Waveform Memory DSP > + * and Closed-Loop Algorithms > + * > + * Copyright 2023 Cirrus Logic, Inc. > + * > + */ > +#include <linux/spi/spi.h> > +#include <linux/input/cs40l50.h> > + > +static const struct spi_device_id cs40l50_id_spi[] = { > + {"cs40l50", 0}, > + {} > +}; > +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 int cs40l50_spi_probe(struct spi_device *spi) > +{ > + struct cs40l50_private *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_private *cs40l50 = spi_get_drvdata(spi); > + > + cs40l50_remove(cs40l50); > +} > + > +static struct spi_driver cs40l50_spi_driver = { > + .driver = { > + .name = "cs40l50", > + .of_match_table = cs40l50_of_match, > + }, > + .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@cirrus.com>"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/input/misc/cs40l50.c b/drivers/input/misc/cs40l50.c > new file mode 100644 > index 000000000000..9bb2881683c0 > --- /dev/null > +++ b/drivers/input/misc/cs40l50.c > @@ -0,0 +1,1008 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * CS40L50 Advanced Haptic Driver with Waveform Memory DSP > + * and Closed-Loop Algorithms > + * > + * Copyright 2023 Cirrus Logic, Inc. > + * > + */ > +#include <linux/firmware.h> > +#include <linux/i2c.h> Why to #include i2c.h here? This driver should be agnostic to the transport. > +#include <linux/interrupt.h> > +#include <linux/kernel.h> > +#include <linux/string.h> > +#include <linux/bitfield.h> > +#include <linux/input/cs40l50.h> Please sort these alphabetically. Are you sure you're using all of them? > + > +const struct regmap_config cs40l50_regmap = { > + .reg_bits = 32, > + .val_bits = 32, > + .reg_stride = 4, > + .reg_format_endian = REGMAP_ENDIAN_BIG, > + .val_format_endian = REGMAP_ENDIAN_BIG, > +}; > +EXPORT_SYMBOL_GPL(cs40l50_regmap); > + > +static struct regulator_bulk_data cs40l50_supplies[] = { > + { .supply = "vp" }, > + { .supply = "vio" }, > + { .supply = "va" }, > +}; > + > +static int cs40l50_dsp_write(struct cs40l50_private *cs40l50, u32 write_reg, u32 write_val) > +{ > + int error = 0, i; No need to initialize 'error' here. > + > + for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) { > + error = regmap_write(cs40l50->regmap, write_reg, write_val); > + if (!error) > + break; This seems more efficient: if (!error) return 0; ...then you can avoid the check for i == CS40L50_DSP_TIMEOUT_COUNT below. > + > + usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100); > + } > + > + if (i == CS40L50_DSP_TIMEOUT_COUNT) { > + dev_err(cs40l50->dev, "Error writing %#X to %#X: %d", write_val, write_reg, error); > + return error; > + } > + > + return 0; > +} > + > +static int cs40l50_dsp_read(struct cs40l50_private *cs40l50, u32 read_reg, u32 *read_val) > +{ > + int error = 0, i; ...and here. > + > + for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) { > + error = regmap_read(cs40l50->regmap, read_reg, read_val); > + if (!error) > + break; ...and here. > + > + usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100); > + } > + > + if (i == CS40L50_DSP_TIMEOUT_COUNT) { > + dev_err(cs40l50->dev, "Error reading %#X: %d", read_reg, error); > + return error; > + } > + > + return 0; > +} > + > +static int cs40l50_pseq_write(struct cs40l50_private *cs40l50, u32 addr, u32 data) > +{ > + u8 operation = 0; > + struct cs40l50_pseq_op *op_new, *op_end; > + int error; Nit: it is kind of personal preference, but "reverse Christmas tree" tends to be more common, i.e. move the struct definitions (longest) to the top. > + > + op_new = devm_kzalloc(cs40l50->dev, sizeof(*op_new), GFP_KERNEL); > + if (!op_new) > + return -ENOMEM; > + > + op_new->words[0] = (addr >> CS40L50_PSEQ_WRITE_FULL_UPPER_ADDR_SHIFT); > + op_new->words[1] = ((addr & CS40L50_PSEQ_WRITE_FULL_LOWER_ADDR_MASK) << > + CS40L50_PSEQ_WRITE_FULL_LOWER_ADDR_SHIFT) | > + (data >> CS40L50_PSEQ_WRITE_FULL_UPPER_DATA_SHIFT); Nit: consider aligning the line breaks past the equal sign for readability. > + op_new->words[2] = data & CS40L50_PSEQ_WRITE_FULL_LOWER_DATA_MASK; > + > + list_for_each_entry(op_end, &cs40l50->pseq_op_head, list) { > + operation = op_end->operation; > + if (operation == CS40L50_PSEQ_OP_END) > + break; > + } > + > + if (operation != CS40L50_PSEQ_OP_END) { > + error = -ENOENT; > + goto op_new_free; > + } > + > + if ((CS40L50_PSEQ_MAX_WORDS - op_end->offset) < ARRAY_SIZE(op_new->words)) { > + error = -ENOSPC; > + goto op_new_free; > + } > + > + op_new->offset = op_end->offset; > + op_end->offset += (ARRAY_SIZE(op_new->words) * sizeof(u32)); > + > + error = regmap_bulk_write(cs40l50->regmap, CS40L50_DSP1_POWER_ON_SEQ + op_new->offset, > + op_new->words, ARRAY_SIZE(op_new->words)); Nit: alignment > + if (error) > + goto op_new_free; > + > + error = cs40l50_dsp_write(cs40l50, CS40L50_DSP1_POWER_ON_SEQ + op_end->offset, > + op_end->words[0]); ...and here. > + if (error) > + goto op_new_free; > + > + list_add(&op_new->list, &cs40l50->pseq_op_head); > + > + return 0; > + > +op_new_free: > + devm_kfree(cs40l50->dev, op_new); > + > + return error; > +} > + > +static struct cs40l50_effect *cs40l50_find_effect(struct cs40l50_private *cs40l50, int id) > +{ > + struct cs40l50_effect *effect; > + > + if (!list_empty(&cs40l50->effect_head)) { > + list_for_each_entry(effect, &cs40l50->effect_head, list) { > + if (effect->id == id) > + return effect; > + } > + } Are you sure the check against list_empty is necessary? Won't list_for_each_entry() simply be a NOP if the list is empty? (genuine question) > + > + return NULL; > +} > + > +static int cs40l50_owt_upload(struct cs40l50_private *cs40l50, s16 *in_data, u32 in_data_nibbles) > +{ > + size_t in_data_bytes = 2 * in_data_nibbles; > + struct cs40l50_owt_header header; > + u32 wt_offset, wt_size_words; > + int error; > + u8 *data; > + > + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_OWT_NEXT, &wt_offset); > + if (error) > + return error; > + > + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_OWT_SIZE, &wt_size_words); > + if (error) > + return error; > + > + if ((wt_size_words * sizeof(u32)) < CS40L50_WT_HEADER_PWLE_SIZE + in_data_bytes) > + return -ENOSPC; > + > + data = kcalloc(CS40L50_WT_HEADER_PWLE_SIZE + in_data_bytes, sizeof(u8), GFP_KERNEL); > + if (IS_ERR_OR_NULL(data)) > + return -ENOMEM; > + > + header.wvfrm_type = in_data[0] == CS40L50_PCM_ID ? CS40L50_WT_TYPE_PCM : > + CS40L50_WT_TYPE_PWLE; Nit: this is a bit more readable: header.wvfrm_type = in_data[0] == CS40L50_PCM_ID ? CS40L50_WT_TYPE_PCM : CS40L50_WT_TYPE_PWLE; > + header.offset = CS40L50_WT_HEADER_OFFSET; > + header.data_size = in_data_bytes / sizeof(u32); > + > + memcpy(data, &header, sizeof(header)); > + memcpy(data + (CS40L50_WT_HEADER_OFFSET * sizeof(u32)), in_data, in_data_bytes); > + > + error = regmap_bulk_write(cs40l50->regmap, CS40L50_DSP1_OWT_BASE + > + (wt_offset * sizeof(u32)), data, > + CS40L50_WT_HEADER_PWLE_SIZE + in_data_bytes); > + if (error) > + goto err_free; > + > + error = cs40l50_dsp_write(cs40l50, CS40L50_DSP_VIRTUAL1_MBOX_1, CS40L50_MBOX_CMD_OWT_PUSH); > +err_free: > + kfree(data); We can avoid a goto and label for this simple case as follows: error = ... if (!error) error = ... kfree(...); return error; > + > + return error; > +} > + > +static int cs40l50_add_effect(struct input_dev *dev, struct ff_effect *effect, > + struct ff_effect *old) Nit: alignment > +{ > + struct cs40l50_private *cs40l50 = input_get_drvdata(dev); > + bool is_new = false; > + struct cs40l50_effect *new_effect; > + u32 nwaves, base_index, max_index; > + s16 *raw_custom_data; > + int error; > + > + if (effect->type != FF_PERIODIC || effect->u.periodic.waveform != FF_CUSTOM) { > + dev_err(cs40l50->dev, "Type (%#X) or waveform (%#X) unsupported\n", > + effect->type, effect->u.periodic.waveform); > + return -EINVAL; > + } > + > + raw_custom_data = kcalloc(effect->u.periodic.custom_len, sizeof(s16), GFP_KERNEL); > + if (IS_ERR_OR_NULL(raw_custom_data)) > + return -ENOMEM; > + > + error = copy_from_user(raw_custom_data, effect->u.periodic.custom_data, > + sizeof(s16) * effect->u.periodic.custom_len); > + if (error) > + goto out_free; > + > + mutex_lock(&cs40l50->lock); > + > + new_effect = cs40l50_find_effect(cs40l50, effect->id); > + if (!new_effect) { > + is_new = true; > + new_effect = kzalloc(sizeof(*new_effect), GFP_KERNEL); > + if (IS_ERR_OR_NULL(new_effect)) { > + error = -ENOMEM; > + goto err_mutex; > + } > + } > + > + if (effect->u.periodic.custom_len > CS40L50_CUSTOM_DATA_SIZE) { > + error = cs40l50_dsp_read(cs40l50, CS40L50_NUM_OF_OWT_WAVES, > + &new_effect->trigger_index); > + if (error) > + goto err_free; > + > + new_effect->wvfrm_bank = CS40L50_WVFRM_BANK_OWT; > + base_index = CS40L50_OWT_INDEX_START; > + max_index = CS40L50_OWT_INDEX_END; > + } else { > + new_effect->trigger_index = (raw_custom_data[1] & 0xffffu); > + new_effect->wvfrm_bank = (raw_custom_data[0] & 0xffffu); > + > + if (new_effect->wvfrm_bank == CS40L50_WVFRM_BANK_ROM) { > + base_index = CS40L50_ROM_BANK_INDEX_START; > + max_index = CS40L50_ROM_BANK_INDEX_END; > + } else if (new_effect->wvfrm_bank == CS40L50_WVFRM_BANK_RAM) { > + error = cs40l50_dsp_read(cs40l50, CS40L50_NUM_OF_WAVES, &nwaves); > + if (error) > + goto err_free; > + > + base_index = CS40L50_RAM_BANK_INDEX_START; > + max_index = base_index + nwaves - 1; > + } else { > + dev_err(cs40l50->dev, "Invalid waveform bank\n"); > + error = -EINVAL; > + goto err_free; > + } > + } > + > + new_effect->trigger_index += base_index; > + > + if (new_effect->trigger_index > max_index) { > + dev_err(cs40l50->dev, "Index %#X out of bounds\n", new_effect->trigger_index); > + error = -EINVAL; > + goto err_free; > + } > + > + if (effect->trigger.button) { > + new_effect->mapping = CS40L50_DSP1_GPIO_BASE + sizeof(u32) * > + FIELD_GET(CS40L50_BTN_NUM_MASK, effect->trigger.button) * 2 - > + FIELD_GET(CS40L50_BTN_EDGE_MASK, effect->trigger.button); > + > + error = cs40l50_dsp_write(cs40l50, new_effect->mapping, effect->trigger.button); > + if (error) > + goto err_free; > + } > + > + if (new_effect->wvfrm_bank == CS40L50_WVFRM_BANK_OWT) { > + error = cs40l50_owt_upload(cs40l50, raw_custom_data, > + effect->u.periodic.custom_len); > + if (error) > + goto err_free; > + } > + > +err_free: > + if (is_new) > + error ? kfree(new_effect) : list_add(&new_effect->list, &cs40l50->effect_head); Just wanted to echo Charles' point that this made my head spin a bit. Good to know it's not just me ;) > +err_mutex: > + mutex_unlock(&cs40l50->lock); > +out_free: > + kfree(raw_custom_data); > + > + return error ? -EFAULT : 0; > +} > + > + > +static void cs40l50_vibe_start_worker(struct work_struct *work) > +{ > + struct cs40l50_private *cs40l50 = > + container_of(work, struct cs40l50_private, vibe_start_work); > + struct cs40l50_effect *effect; > + > + mutex_lock(&cs40l50->lock); > + > + effect = cs40l50_find_effect(cs40l50, cs40l50->trigger_effect->id); > + if (effect) > + cs40l50_dsp_write(cs40l50, CS40L50_DSP_VIRTUAL1_MBOX_1, effect->trigger_index); > + > + mutex_unlock(&cs40l50->lock); > +} > + > +static void cs40l50_vibe_stop_worker(struct work_struct *work) > +{ > + struct cs40l50_private *cs40l50 = > + container_of(work, struct cs40l50_private, vibe_stop_work); > + > + mutex_lock(&cs40l50->lock); > + > + cs40l50_dsp_write(cs40l50, CS40L50_DSP_VIRTUAL1_MBOX_1, CS40L50_MBOX_CMD_STOP_PLAYBACK); > + > + mutex_unlock(&cs40l50->lock); > +} > + > +static int cs40l50_playback_effect(struct input_dev *dev, int effect_id, int val) > +{ > + struct cs40l50_private *cs40l50 = input_get_drvdata(dev); > + > + cs40l50->trigger_effect = &dev->ff->effects[effect_id]; > + > + if (val > 0) > + queue_work(cs40l50->vibe_workqueue, &cs40l50->vibe_start_work); > + else > + queue_work(cs40l50->vibe_workqueue, &cs40l50->vibe_stop_work); > + > + return 0; > +} > + > +static int cs40l50_erase_effect(struct input_dev *dev, int effect_id) > +{ > + int error = 0; > + struct cs40l50_private *cs40l50 = input_get_drvdata(dev); > + struct cs40l50_effect *effect, *effect_owt; > + > + mutex_lock(&cs40l50->lock); > + > + effect = cs40l50_find_effect(cs40l50, dev->ff->effects[effect_id].id); > + if (!effect) { > + error = -EINVAL; > + goto err_mutex; > + } > + > + if (effect->mapping != CS40L50_GPIO_MAPPING_INVALID) { > + error = cs40l50_dsp_write(cs40l50, effect->mapping, CS40L50_GPI_DISABLE); > + if (error) > + goto err_mutex; > + } > + > + if (effect->wvfrm_bank == CS40L50_WVFRM_BANK_OWT) { > + error = cs40l50_dsp_write(cs40l50, CS40L50_DSP_VIRTUAL1_MBOX_1, > + CS40L50_MBOX_CMD_OWT_DELETE | effect->trigger_index); Nit: alignment here and elsewhere. > + if (error) > + goto err_mutex; > + > + list_for_each_entry(effect_owt, &cs40l50->effect_head, list) { > + if (effect_owt->wvfrm_bank == CS40L50_WVFRM_BANK_OWT && > + effect_owt->trigger_index > effect->trigger_index) > + effect_owt->trigger_index--; > + } > + } > + > + list_del(&effect->list); > + kfree(effect); > +err_mutex: > + mutex_unlock(&cs40l50->lock); > + > + return error; > +} > + > +static int cs40l50_mailbox_read_next(struct cs40l50_private *cs40l50, u32 *val) > +{ > + u32 wt_ptr, rd_ptr; > + int error = 0; No need to initialize 'error' here. > + > + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_MBOX_QUEUE_WT, &wt_ptr); > + if (error) > + return error; > + > + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_MBOX_QUEUE_RD, &rd_ptr); > + if (error) > + return error; > + > + if (wt_ptr == rd_ptr) { > + *val = 0; > + return 0; > + } > + > + error = cs40l50_dsp_read(cs40l50, rd_ptr, val); > + if (error) > + return error; > + > + rd_ptr += sizeof(u32); > + if (rd_ptr > CS40L50_MBOX_QUEUE_END) > + rd_ptr = CS40L50_MBOX_QUEUE_BASE; > + > + return cs40l50_dsp_write(cs40l50, CS40L50_DSP1_MBOX_QUEUE_RD, rd_ptr); > +} > + > +static int cs40l50_handle_f0_est_done(struct cs40l50_private *cs40l50) > +{ > + int error; > + u32 f_zero; > + > + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_F0_ESTIMATION, &f_zero); > + if (error) > + return error; > + > + return cs40l50_dsp_write(cs40l50, CS40L50_DSP1_F0_STORED, f_zero); > +} > + > +static int cs40l50_handle_redc_est_done(struct cs40l50_private *cs40l50) > +{ > + int integer, fractional, error; > + u32 redc; > + > + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_RE_EST_STATUS, &redc); > + if (error) > + return error; > + > + error = cs40l50_dsp_write(cs40l50, CS40L50_DSP1_REDC_ESTIMATION, redc); > + if (error) > + return error; > + > + /* Convert from Q8.15 to (Q7.17 * 29/240) */ > + integer = (redc >> 15) & 0xFF; > + fractional = (redc & 0x7FFF) * 4; > + > + return cs40l50_dsp_write(cs40l50, CS40L50_DSP1_REDC_STORED, > + (((integer << 17) | fractional) * 29 / 240) & GENMASK(23, 0)); > +} > + > +static int cs40l50_error_release(struct cs40l50_private *cs40l50) > +{ > + int error; > + > + error = cs40l50_dsp_write(cs40l50, CS40L50_ERROR_RELEASE, CS40L50_ERROR_RELEASE_MASK); > + if (error) > + return error; > + > + return cs40l50_dsp_write(cs40l50, CS40L50_ERROR_RELEASE, 0); > +} > + > +static irqreturn_t cs40l50_handle_mbox_buffer(int irq, void *data) > +{ > + struct cs40l50_private *cs40l50 = data; > + int error = 0; > + u32 val; > + > + mutex_lock(&cs40l50->lock); > + > + while (!cs40l50_mailbox_read_next(cs40l50, &val)) { > + switch (val) { > + case 0: > + goto out_irq; > + case CS40L50_DSP_MBOX_HAPTIC_TRIGGER_GPIO: > + dev_dbg(cs40l50->dev, "Mailbox: TRIGGER_GPIO\n"); > + break; > + case CS40L50_DSP_MBOX_HAPTIC_TRIGGER_MBOX: > + dev_dbg(cs40l50->dev, "Mailbox: TRIGGER_MBOX\n"); > + break; > + case CS40L50_DSP_MBOX_HAPTIC_TRIGGER_I2S: > + dev_dbg(cs40l50->dev, "Mailbox: TRIGGER_I2S\n"); > + break; > + case CS40L50_DSP_MBOX_HAPTIC_COMPLETE_MBOX: > + dev_dbg(cs40l50->dev, "Mailbox: COMPLETE_MBOX\n"); > + break; > + case CS40L50_DSP_MBOX_HAPTIC_COMPLETE_GPIO: > + dev_dbg(cs40l50->dev, "Mailbox: COMPLETE_GPIO\n"); > + break; > + case CS40L50_DSP_MBOX_HAPTIC_COMPLETE_I2S: > + dev_dbg(cs40l50->dev, "Mailbox: COMPLETE_I2S\n"); > + break; > + case CS40L50_DSP_MBOX_F0_EST_START: > + dev_dbg(cs40l50->dev, "Mailbox: F0_EST_START\n"); > + break; > + case CS40L50_DSP_MBOX_F0_EST_DONE: > + dev_dbg(cs40l50->dev, "Mailbox: F0_EST_DONE\n"); > + error = cs40l50_handle_f0_est_done(cs40l50); > + if (error) > + goto out_irq; > + break; > + case CS40L50_DSP_MBOX_REDC_EST_START: > + dev_dbg(cs40l50->dev, "Mailbox: REDC_EST_START\n"); > + break; > + case CS40L50_DSP_MBOX_REDC_EST_DONE: > + dev_dbg(cs40l50->dev, "Mailbox: REDC_EST_DONE\n"); > + error = cs40l50_handle_redc_est_done(cs40l50); > + if (error) > + goto out_irq; > + break; > + case CS40L50_DSP_MBOX_LE_EST_START: > + dev_dbg(cs40l50->dev, "Mailbox: LE_EST_START\n"); > + break; > + case CS40L50_DSP_MBOX_LE_EST_DONE: > + dev_dbg(cs40l50->dev, "Mailbox: LE_EST_DONE\n"); > + break; > + case CS40L50_DSP_MBOX_AWAKE: > + dev_dbg(cs40l50->dev, "Mailbox: AWAKE\n"); > + break; > + case CS40L50_DSP_MBOX_INIT: > + dev_dbg(cs40l50->dev, "Mailbox: INIT\n"); > + break; > + case CS40L50_DSP_MBOX_ACK: > + dev_dbg(cs40l50->dev, "Mailbox: ACK\n"); > + break; > + case CS40L50_DSP_MBOX_ERR_EVENT_UNMAPPED: > + dev_err(cs40l50->dev, "Unmapped event\n"); > + break; > + case CS40L50_DSP_MBOX_ERR_EVENT_MODIFY: > + dev_err(cs40l50->dev, "Attempt to modify event index failed\n"); > + break; > + case CS40L50_DSP_MBOX_ERR_NULLPTR: > + dev_err(cs40l50->dev, "Null pointer\n"); > + break; > + case CS40L50_DSP_MBOX_ERR_BRAKING: > + dev_err(cs40l50->dev, "Braking not in progress\n"); > + break; > + case CS40L50_DSP_MBOX_ERR_INVAL_SRC: > + dev_err(cs40l50->dev, "Suspend/resume invalid source\n"); > + break; > + case CS40L50_DSP_MBOX_ERR_ENABLE_RANGE: > + dev_err(cs40l50->dev, "GPIO enable out of range\n"); > + break; > + case CS40L50_DSP_MBOX_ERR_GPIO_UNMAPPED: > + dev_err(cs40l50->dev, "GPIO not mapped\n"); > + break; > + case CS40L50_DSP_MBOX_ERR_ISR_RANGE: > + dev_err(cs40l50->dev, "GPIO ISR out of range\n"); > + break; > + case CS40L50_DSP_MBOX_PERMANENT_SHORT: > + dev_crit(cs40l50->dev, "Permanent short detected\n"); > + break; > + case CS40L50_DSP_MBOX_RUNTIME_SHORT: > + dev_err(cs40l50->dev, "Runtime short detected\n"); > + error = cs40l50_error_release(cs40l50); > + if (error) > + goto out_irq; > + break; > + default: > + dev_err(cs40l50->dev, "Mailbox value %#X not recognized\n", val); > + error = -EINVAL; > + goto out_irq; > + } > + } > + > + error = -EIO; > + > +out_irq: > + mutex_unlock(&cs40l50->lock); > + > + return IRQ_RETVAL(!error); > +} > + > + > +static irqreturn_t cs40l50_vddb_uv(int irq, void *data) > +{ > + struct cs40l50_private *cs40l50 = data; > + > + dev_err(cs40l50->dev, "VDD_B undervoltage error\n"); > + > + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); > +} > + > +static irqreturn_t cs40l50_boost_current_limit(int irq, void *data) > +{ > + struct cs40l50_private *cs40l50 = data; > + > + dev_err(cs40l50->dev, "Boost current limit engagement\n"); > + > + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); > +} > + > +static irqreturn_t cs40l50_boost_short(int irq, void *data) > +{ > + struct cs40l50_private *cs40l50 = data; > + > + dev_err(cs40l50->dev, "Boost inductor short error\n"); > + > + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); > +} > + > +static irqreturn_t cs40l50_boost_uv(int irq, void *data) > +{ > + struct cs40l50_private *cs40l50 = data; > + > + dev_err(cs40l50->dev, "Boost undervoltage error\n"); > + > + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); > +} > + > +static irqreturn_t cs40l50_overtemperature(int irq, void *data) > +{ > + struct cs40l50_private *cs40l50 = data; > + > + dev_err(cs40l50->dev, "Overtemperature error\n"); > + > + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); > +} > + > +static irqreturn_t cs40l50_amp_short(int irq, void *data) > +{ > + struct cs40l50_private *cs40l50 = data; > + > + dev_err(cs40l50->dev, "Amp short error\n"); > + > + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); > +} > + > +static irqreturn_t cs40l50_global_error(int irq, void *data) > +{ > + struct cs40l50_private *cs40l50 = data; > + > + dev_err(cs40l50->dev, "MSM chip error\n"); > + > + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); > +} > + > +static const struct cs40l50_irq cs40l50_irqs[] = { > + CS40L50_IRQ(CS40L50_AMP_SHORT_IRQ, "Amp short", cs40l50_amp_short), > + CS40L50_IRQ(CS40L50_VIRT2_MBOX_IRQ, "Firmware interrupt", cs40l50_handle_mbox_buffer), > + CS40L50_IRQ(CS40L50_TEMP_ERR_IRQ, "Overtemperature", cs40l50_overtemperature), > + CS40L50_IRQ(CS40L50_BST_UVP_IRQ, "Boost undervoltage", cs40l50_boost_uv), > + CS40L50_IRQ(CS40L50_BST_SHORT_IRQ, "Boost short", cs40l50_boost_short), > + CS40L50_IRQ(CS40L50_BST_ILIMIT_IRQ, "Boost current limit", cs40l50_boost_current_limit), > + CS40L50_IRQ(CS40L50_UVLO_VDDBATT_IRQ, "VDD_B undervoltage", cs40l50_vddb_uv), > + CS40L50_IRQ(CS40L50_GLOBAL_ERROR_IRQ, "Global error", cs40l50_global_error), > +}; > + > +static const struct regmap_irq cs40l50_reg_irqs[] = { > + CS40L50_REG_IRQ(IRQ1_INT_1, AMP_SHORT), > + CS40L50_REG_IRQ(IRQ1_INT_2, VIRT2_MBOX), > + CS40L50_REG_IRQ(IRQ1_INT_8, TEMP_ERR), > + CS40L50_REG_IRQ(IRQ1_INT_9, BST_UVP), > + CS40L50_REG_IRQ(IRQ1_INT_9, BST_SHORT), > + CS40L50_REG_IRQ(IRQ1_INT_9, BST_ILIMIT), > + CS40L50_REG_IRQ(IRQ1_INT_10, UVLO_VDDBATT), > + CS40L50_REG_IRQ(IRQ1_INT_18, GLOBAL_ERROR), > +}; > + > +static const struct regmap_irq_chip cs40l50_regmap_irq_chip = { > + .name = "cs40l50 IRQ1 Controller", > + .status_base = CS40L50_IRQ1_INT_1, > + .mask_base = CS40L50_IRQ1_MASK_1, > + .ack_base = CS40L50_IRQ1_INT_1, > + .num_regs = 19, > + .irqs = cs40l50_reg_irqs, > + .num_irqs = ARRAY_SIZE(cs40l50_reg_irqs), > + .runtime_pm = true, > +}; > + > +static const struct reg_sequence cs40l50_ext_bst_seq[] = { > + { CS40L50_BLOCK_ENABLES, CS40L50_MON_ENABLE }, > + { CS40L50_MON_VALUE_CTRL, CS40L50_VBST_VDD_AMP }, > + { CS40L50_TEST_KEY, CS40L50_KEY_SET_1 }, > + { CS40L50_TEST_KEY, CS40L50_KEY_SET_2 }, > + { CS40L50_TEST_VIS_SPARE, CS40L50_TEMP_SENSOR_3 }, > + { CS40L50_MON_ERROR_OVERIDE, CS40L50_TEMP_ERR_THLD }, > + { CS40L50_STATUS, CS40L50_TEMP_WARN_THLD }, > + { CS40L50_TEST_CONFIG, CS40L50_TEMP_FILT_DEBOUNCE_OFF }, > + { CS40L50_TEMP_WARN_CONFIG, CS40L50_TEMP_WARN_ATK_VOL }, > +}; > + > +static int cs40l50_config_bst(struct cs40l50_private *cs40l50) > +{ > + int error, i; > + > + if (device_property_present(cs40l50->dev, "cirrus,external-boost")) { > + error = regmap_multi_reg_write(cs40l50->regmap, cs40l50_ext_bst_seq, > + ARRAY_SIZE(cs40l50_ext_bst_seq)); > + if (error) > + return error; > + > + for (i = 0; i < ARRAY_SIZE(cs40l50_ext_bst_seq); i++) { > + error = cs40l50_pseq_write(cs40l50, cs40l50_ext_bst_seq[i].reg, > + cs40l50_ext_bst_seq[i].def); > + if (error) > + return error; > + } > + > + return 0; > + } > + > + /* Internal boost config */ This driver is conspicuously void of comments and documentation; why is this line mysterious enough to warrant one? > + return cs40l50_dsp_write(cs40l50, CS40L50_BST_LPMODE_SEL, CS40L50_DCM_LOW_POWER); > +} > + > +static int cs40l50_patch_firmware(struct cs40l50_private *cs40l50) > +{ > + const struct firmware *bin = NULL, *wmfw = NULL; > + int error = 0; > + > + if (request_firmware(&bin, "cs40l50.bin", cs40l50->dev)) > + dev_info(cs40l50->dev, "Could not request wavetable file: %d\n", error); > + > + if (request_firmware(&wmfw, "cs40l50.wmfw", cs40l50->dev)) > + dev_info(cs40l50->dev, "Could not request firmware patch file: %d\n", error); > + > + if (wmfw) { It is a much more common design pattern to bail if request_firmware() returns an error, than to proceed and check against the FW pointer remaining NULL. Is this done because cs_dsp_power_up() must be called, with or without a wmfw or coefficient file being available? > + error = cs40l50_dsp_write(cs40l50, CS40L50_CCM_CORE_CONTROL, > + CS40L50_DSP1_CLOCK_DISABLE); > + if (error) > + goto err_fw; > + > + error = cs40l50_dsp_write(cs40l50, CS40L50_RAM_INIT, > + CS40L50_DSP1_RAM_INIT_FLAG); > + if (error) > + goto err_fw; > + > + error = cs40l50_dsp_write(cs40l50, CS40L50_PWRMGT_CTL, > + CS40L50_DSP1_MEM_RDY_HW); > + if (error) > + goto err_fw; > + } > + > + mutex_lock(&cs40l50->lock); > + > + cs_dsp_power_up(&cs40l50->dsp, wmfw, "cs40l50.wmfw", bin, "cs40l50.bin", "cs40l50"); > + > + mutex_unlock(&cs40l50->lock); > + > + if (wmfw) > + error = cs40l50_dsp_write(cs40l50, CS40L50_CCM_CORE_CONTROL, > + CS40L50_DSP1_CLOCK_ENABLE); > + > +err_fw: > + release_firmware(wmfw); > + release_firmware(bin); > + > + return error; > +} > + > +static int cs40l50_pseq_init(struct cs40l50_private *cs40l50) > +{ > + struct cs40l50_pseq_op *pseq_op; > + int error, i, num_words; > + u8 operation; > + u32 *words; > + > + INIT_LIST_HEAD(&cs40l50->pseq_op_head); > + > + words = kcalloc(CS40L50_PSEQ_MAX_WORDS, sizeof(u32), GFP_KERNEL); > + if (IS_ERR_OR_NULL(words)) > + return -ENOMEM; Just curious what is the need to call IS_ERR_OR_NULL() here and throughout after allocation rather than just !foo ? > + > + error = regmap_bulk_read(cs40l50->regmap, CS40L50_DSP1_POWER_ON_SEQ, words, > + CS40L50_PSEQ_MAX_WORDS); > + if (error) > + goto err_free; > + > + for (i = 0; i < CS40L50_PSEQ_MAX_WORDS; i++) > + words[i] = be32_to_cpu(words[i]); > + > + for (i = 0; i < CS40L50_PSEQ_MAX_WORDS; i += num_words) { > + operation = (words[i] & CS40L50_PSEQ_OP_MASK) >> CS40L50_PSEQ_OP_SHIFT; > + > + switch (operation) { > + case CS40L50_PSEQ_OP_END: > + num_words = CS40L50_PSEQ_OP_END_WORDS; > + break; > + case CS40L50_PSEQ_OP_WRITE_ADDR8: > + case CS40L50_PSEQ_OP_WRITE_H16: > + case CS40L50_PSEQ_OP_WRITE_L16: > + num_words = CS40L50_PSEQ_OP_WRITE_X16_WORDS; > + break; > + case CS40L50_PSEQ_OP_WRITE_FULL: > + num_words = CS40L50_PSEQ_OP_WRITE_FULL_WORDS; > + break; > + default: > + dev_err(cs40l50->dev, "Invalid OP code 0x%02X\n", operation); > + error = -EINVAL; > + goto err_free; > + } > + > + pseq_op = devm_kzalloc(cs40l50->dev, sizeof(*pseq_op), GFP_KERNEL); > + if (!pseq_op) { > + error = -ENOMEM; > + goto err_free; > + } > + > + memcpy(pseq_op->words, &words[i], num_words * sizeof(u32)); > + pseq_op->size = num_words; > + pseq_op->offset = i * sizeof(u32); > + pseq_op->operation = operation; > + list_add(&pseq_op->list, &cs40l50->pseq_op_head); > + > + if (operation == CS40L50_PSEQ_OP_END) > + break; > + } > + > + if (operation != CS40L50_PSEQ_OP_END) { > + dev_err(cs40l50->dev, "PSEQ_END_OF_SCRIPT not found\n"); > + error = -ENOENT; > + } > + > +err_free: > + kfree(words); > + > + return error; > +} > + > +static int cs40l50_input_init(struct cs40l50_private *cs40l50) > +{ > + int error; > + u32 val; > + > + error = cs40l50_dsp_read(cs40l50, CS40L50_DEVID, &val); > + if (error) > + return error; > + > + if (val != CS40L50_DEVID_A) { > + dev_err(cs40l50->dev, "Invalid device ID: %#010X\n", val); > + return -EINVAL; > + } > + > + cs40l50->input->id.product = val & 0xFFFF; > + > + error = cs40l50_dsp_read(cs40l50, CS40L50_REVID, &val); > + if (error) > + return error; > + > + if (val != CS40L50_REVID_B0) { > + dev_err(cs40l50->dev, "Invalid device revision: %#04X\n", val); > + return -EINVAL; > + } > + > + cs40l50->input->id.version = val; > + > + cs40l50->input = devm_input_allocate_device(cs40l50->dev); > + if (IS_ERR_OR_NULL(cs40l50->input)) > + return cs40l50->input ? PTR_ERR(cs40l50->input) : -ENOMEM; > + > + cs40l50->input->name = "cs40l50_input"; > + input_set_drvdata(cs40l50->input, cs40l50); > + input_set_capability(cs40l50->input, EV_FF, FF_PERIODIC); > + input_set_capability(cs40l50->input, EV_FF, FF_CUSTOM); > + > + error = input_ff_create(cs40l50->input, FF_MAX_EFFECTS); > + if (error) > + return error; > + > + clear_bit(FF_RUMBLE, cs40l50->input->ffbit); If there is a HW limitation that prevents this capability, it would be handy to explain as much with a comment. > + > + cs40l50->input->ff->upload = cs40l50_add_effect; > + cs40l50->input->ff->playback = cs40l50_playback_effect; > + cs40l50->input->ff->erase = cs40l50_erase_effect; > + > + INIT_LIST_HEAD(&cs40l50->effect_head); > + > + return input_register_device(cs40l50->input); > +} > + > +static const struct cs_dsp_client_ops cs40l50_cs_dsp_client_ops; > + > +static const struct cs_dsp_region cs40l50_dsp_regions[] = { > + { .type = WMFW_HALO_PM_PACKED, .base = CS40L50_DSP1_PMEM_0 }, > + { .type = WMFW_HALO_XM_PACKED, .base = CS40L50_DSP1_XMEM_PACKED_0 }, > + { .type = WMFW_HALO_YM_PACKED, .base = CS40L50_DSP1_YMEM_PACKED_0 }, > + { .type = WMFW_ADSP2_XM, .base = CS40L50_DSP1_XMEM_UNPACKED24_0 }, > + { .type = WMFW_ADSP2_YM, .base = CS40L50_DSP1_YMEM_UNPACKED24_0 }, > +}; This is more readable as: [...], { .type = ..., .base = ..., }, [...], > + > +static int cs40l50_cs_dsp_init(struct cs40l50_private *cs40l50) > +{ > + cs40l50->dsp.num = 1; > + cs40l50->dsp.type = WMFW_HALO; > + cs40l50->dsp.dev = cs40l50->dev; > + cs40l50->dsp.regmap = cs40l50->regmap; > + cs40l50->dsp.base = CS40L50_DSP1_CORE_BASE; > + cs40l50->dsp.base_sysinfo = CS40L50_DSP1_SYS_INFO_ID; > + cs40l50->dsp.mem = cs40l50_dsp_regions; > + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions); > + cs40l50->dsp.lock_regions = 0xFFFFFFFF; > + cs40l50->dsp.no_core_startstop = true; > + cs40l50->dsp.client_ops = &cs40l50_cs_dsp_client_ops; > + > + return cs_dsp_halo_init(&cs40l50->dsp); > +} > + > +int cs40l50_probe(struct cs40l50_private *cs40l50) > +{ > + int error, i, irq; > + u32 val; > + > + mutex_init(&cs40l50->lock); > + > + error = devm_regulator_bulk_get(cs40l50->dev, ARRAY_SIZE(cs40l50_supplies), > + cs40l50_supplies); > + if (error) > + return dev_err_probe(cs40l50->dev, error, "Failed to request supplies\n"); > + > + error = regulator_bulk_enable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies); > + if (error) > + return dev_err_probe(cs40l50->dev, error, "Failed to enable supplies\n"); > + > + cs40l50->reset_gpio = devm_gpiod_get_optional(cs40l50->dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(cs40l50->reset_gpio)) { > + error = PTR_ERR(cs40l50->reset_gpio); > + goto err; > + } Krzysztof covered this already, but I just want to re-iterate that you are kind of swallowing your own head by calling cs40l50_remove() in your error path. This is a confusing and uncommon design pattern. You cannot have possibly booted the DSP or allocated work here, so why not just return? > + > + usleep_range(CS40L50_MIN_RESET_PULSE_WIDTH_US, CS40L50_MIN_RESET_PULSE_WIDTH_US + 100); > + > + gpiod_set_value_cansleep(cs40l50->reset_gpio, 0); > + > + usleep_range(CS40L50_CP_READY_DELAY_US, CS40L50_CP_READY_DELAY_US + 1000); > + > + for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) { > + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_HALO_STATE, &val); > + if (!error && val < 0xFFFF && val >= CS40L50_HALO_STATE_BOOT_DONE) > + break; > + > + usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100); > + } > + > + if (i == CS40L50_DSP_TIMEOUT_COUNT) { > + dev_err(cs40l50->dev, "Firmware boot failed: %d, halo state = %#x\n", error, val); > + goto err; > + } > + > + cs40l50->vibe_workqueue = alloc_ordered_workqueue("cs40l50_workqueue", WQ_HIGHPRI); > + if (!cs40l50->vibe_workqueue) { > + error = -ENOMEM; > + goto err; > + } > + > + INIT_WORK(&cs40l50->vibe_start_work, cs40l50_vibe_start_worker); > + INIT_WORK(&cs40l50->vibe_stop_work, cs40l50_vibe_stop_worker); > + > + error = cs40l50_cs_dsp_init(cs40l50); > + if (error) > + goto err; It seems your error path is complex because you intend to unwind the DSP in case of an error, in which case any teardown should be explicitly called from the bottom of probe() rather than calling remove() on your own. Opinions may differ here, but mine is that you need not go out of your way to power down the DSP if glaring errors are encountered during probe. If you cannot read HALO_STATE, do you really expect to be able to do any additional work anyway? Understanding that haptics bears the risk of a motor stuck on, perhaps it is sufficient to simply throw the device into reset if things go afoul? > + > + error = cs40l50_input_init(cs40l50); > + if (error) > + goto err; > + > + error = cs40l50_patch_firmware(cs40l50); > + if (error) > + goto err; I just want to re-iterate Charles' comment; this _will_not_ work across all customer platforms because you cannot guarantee the filesystem is ready here if the driver is built-in. In the best case you will get -ENOENT right away; in the worst case you will get a deadlock. If this device does not have nonvolatile memory and you must load FW from probe, use request_firmware_nowait() instead. > + > + error = cs40l50_pseq_init(cs40l50); > + if (error) > + goto err; > + > + error = cs40l50_config_bst(cs40l50); > + if (error) > + goto err; > + > + error = devm_regmap_add_irq_chip(cs40l50->dev, cs40l50->regmap, cs40l50->irq, > + IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW, 0, &cs40l50_regmap_irq_chip, > + &cs40l50->irq_data); We should not be mandating IRQ polarity in the driver. Sometimes the customer may use a GPIO expander with inverted polarity; we need to let dts govern the polarity. > + if (error) { > + dev_err(cs40l50->dev, "Failed to register IRQ chip: %d\n", error); > + goto err; > + } > + > + for (i = 0; i < ARRAY_SIZE(cs40l50_irqs); i++) { > + irq = regmap_irq_get_virq(cs40l50->irq_data, cs40l50_irqs[i].irq); > + if (irq < 0) { > + dev_err(cs40l50->dev, "Failed to get %s\n", cs40l50_irqs[i].name); > + error = irq; > + goto err; > + } > + > + error = devm_request_threaded_irq(cs40l50->dev, irq, NULL, cs40l50_irqs[i].handler, > + IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW, > + cs40l50_irqs[i].name, cs40l50); > + if (error) { > + dev_err(cs40l50->dev, "Failed to request IRQ %s: %d\n", > + cs40l50_irqs[i].name, error); > + goto err; > + } > + } > + > + return 0; > + > +err: > + cs40l50_remove(cs40l50); > + > + return error; > +} > +EXPORT_SYMBOL_GPL(cs40l50_probe); > + > +int cs40l50_remove(struct cs40l50_private *cs40l50) > +{ > + disable_irq(cs40l50->irq); > + > + if (cs40l50->dsp.booted) > + cs_dsp_power_down(&cs40l50->dsp); > + if (&cs40l50->dsp) > + cs_dsp_remove(&cs40l50->dsp); > + > + if (cs40l50->vibe_workqueue) { > + flush_workqueue(cs40l50->vibe_workqueue); > + destroy_workqueue(cs40l50->vibe_workqueue); > + } > + > + gpiod_set_value_cansleep(cs40l50->reset_gpio, 1); > + regulator_bulk_disable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(cs40l50_remove); > + > +MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver"); > +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>"); > +MODULE_LICENSE("GPL"); > diff --git a/include/linux/input/cs40l50.h b/include/linux/input/cs40l50.h > new file mode 100644 > index 000000000000..f2c8a101fa00 > --- /dev/null > +++ b/include/linux/input/cs40l50.h > @@ -0,0 +1,321 @@ > +/* SPDX-License-Identifier: GPL-2.0 > + * > + * CS40L50 Advanced Haptic Driver with Waveform Memory DSP > + * and Closed-Loop Algorithms > + * > + * Copyright 2023 Cirrus Logic, Inc. > + * > + */ > +#ifndef __CS40L50_H__ > +#define __CS40L50_H__ > + > +#include <linux/bitops.h> bitops.h is quite outdated; did you mean bits.h? > +#include <linux/gpio/consumer.h> > +#include <linux/input.h> > +#include <linux/regmap.h> > +#include <linux/interrupt.h> > +#include <linux/firmware/cirrus/cs_dsp.h> > +#include <linux/firmware/cirrus/wmfw.h> > + > +/* Boost */ > +#define CS40L50_TEST_KEY 0x40 > +#define CS40L50_BLOCK_ENABLES 0x2018 > +#define CS40L50_ERROR_RELEASE 0x2034 > +#define CS40L50_PWRMGT_CTL 0x2900 > +#define CS40L50_BST_LPMODE_SEL 0x3810 > +#define CS40L50_STATUS 0x4200 > +#define CS40L50_MON_ERROR_OVERIDE 0x4220 > +#define CS40L50_TEST_CONFIG 0x4240 > +#define CS40L50_MON_VALUE_CTRL 0x4404 > +#define CS40L50_TEST_VIS_SPARE 0x5C00 > +#define CS40L50_TEMP_WARN_CONFIG 0x6024 > +#define CS40L50_KEY_SET_1 0x55 > +#define CS40L50_KEY_SET_2 0xAA > +#define CS40L50_TEMP_SENSOR_3 0x400 > +#define CS40L50_DCM_LOW_POWER 0x1 > +#define CS40L50_MON_ENABLE 0x3201 > +#define CS40L50_VBST_VDD_AMP 0x1000000 > +#define CS40L50_TEMP_ERR_THLD 0x8000007D > +#define CS40L50_TEMP_WARN_THLD 0x8 > +#define CS40L50_TEMP_FILT_DEBOUNCE_OFF 0x510002B5 > +#define CS40L50_TEMP_WARN_ATK_VOL 0x522303 > + > +/* Interrupts */ > +#define CS40L50_IRQ1_INT_1 0xE010 > +#define CS40L50_IRQ1_INT_2 0xE014 > +#define CS40L50_IRQ1_INT_3 0xE018 > +#define CS40L50_IRQ1_INT_4 0xE01C > +#define CS40L50_IRQ1_INT_8 0xE02C > +#define CS40L50_IRQ1_INT_9 0xE030 > +#define CS40L50_IRQ1_INT_10 0xE034 > +#define CS40L50_IRQ1_INT_18 0xE054 > +#define CS40L50_IRQ1_INT_19 0xE058 > +#define CS40L50_IRQ1_MASK_1 0xE090 > +#define CS40L50_IRQ1_MASK_2 0xE094 > +#define CS40L50_IRQ1_MASK_3 0xE098 > +#define CS40L50_IRQ1_MASK_8 0xE0AC > +#define CS40L50_IRQ1_MASK_9 0xE0B0 > +#define CS40L50_IRQ1_MASK_10 0xE0B4 > +#define CS40L50_IRQ1_MASK_18 0xE0D4 > +#define CS40L50_IRQ1_MASK_19 0xE0D8 > +#define CS40L50_IRQ1_STS_1 0xE210 > +#define CS40L50_IRQ1_STS_2 0xE214 > +#define CS40L50_IRQ1_STS_3 0xE218 > +#define CS40L50_IRQ1_STS_8 0xE22C > +#define CS40L50_IRQ1_STS_9 0xE230 > +#define CS40L50_IRQ1_STS_10 0xE234 > +#define CS40L50_IRQ1_STS_18 0xE254 > +#define CS40L50_IRQ1_STS_19 0xE258 > +#define CS40L50_AMP_SHORT_MASK BIT(31) > +#define CS40L50_VIRT2_MBOX_MASK BIT(21) > +#define CS40L50_TEMP_ERR_MASK BIT(31) > +#define CS40L50_BST_UVP_MASK BIT(6) > +#define CS40L50_BST_SHORT_MASK BIT(7) > +#define CS40L50_BST_ILIMIT_MASK BIT(8) > +#define CS40L50_UVLO_VDDBATT_MASK BIT(16) > +#define CS40L50_GLOBAL_ERROR_MASK BIT(15) > +#define CS40L50_ERROR_RELEASE_MASK BIT(11) > +#define CS40L50_BOOT_DONE_MASK BIT(1) > +#define CS40L50_IRQ(_irq, _name, _hand) \ > + { \ > + .irq = _irq, \ > + .name = _name, \ > + .handler = _hand, \ > + } > +#define CS40L50_REG_IRQ(_reg, _irq) \ > + [CS40L50_ ## _irq ## _IRQ] = { \ > + .reg_offset = (CS40L50_ ## _reg) - CS40L50_IRQ1_INT_1, \ > + .mask = CS40L50_ ## _irq ## _MASK \ > + } > + > +/* Mailbox Inbound Commands */ > +#define CS40L50_RAM_BANK_INDEX_START 0x1000000 > +#define CS40L50_RAM_BANK_INDEX_END 0x100007F > +#define CS40L50_OWT_INDEX_START 0x1400000 > +#define CS40L50_OWT_INDEX_END 0x140007F > +#define CS40L50_ROM_BANK_INDEX_START 0x1800000 > +#define CS40L50_ROM_BANK_INDEX_END 0x180001A > +#define CS40L50_OTP_BUZZ 0x1800080 > +#define CS40L50_RAM_BUZZ_INDEX_START 0x1800081 > +#define CS40L50_RAM_BUZZ_INDEX_END 0x1800085 > +#define CS40L50_MBOX_CMD_HIBERNATE 0x2000001 > +#define CS40L50_MBOX_CMD_WAKEUP 0x2000002 > +#define CS40L50_MBOX_CMD_PREVENT_HIBERNATE 0x2000003 > +#define CS40L50_MBOX_CMD_ALLOW_HIBERNATION 0x2000004 > +#define CS40L50_MBOX_CMD_SHUTDOWN 0x2000005 > +#define CS40L50_MBOX_CMD_DURATION_REPORT 0x3000001 > +#define CS40L50_MBOX_CMD_START_I2S 0x3000002 > +#define CS40L50_MBOX_CMD_STOP_I2S 0x3000003 > +#define CS40L50_MBOX_CMD_HAPTICS_LOGGER_RESET 0x3000004 > +#define CS40L50_MBOX_CMD_OWT_PUSH 0x3000008 > +#define CS40L50_MBOX_CMD_OWT_RESET 0x3000009 > +#define CS40L50_MBOX_CMD_DVL_REINIT 0x300000A > +#define CS40L50_MBOX_CMD_STOP_PLAYBACK 0x5000000 > +#define CS40L50_MBOX_CMD_F0_EST 0x7000001 > +#define CS40L50_MBOX_CMD_REDC_EST 0x7000002 > +#define CS40L50_MBOX_CMD_LE_EST 0x7000004 > +#define CS40L50_MBOX_CMD_OWT_DELETE 0xD000000 > + > +/* Mailbox Outbound Commands */ > +#define CS40L50_MBOX_QUEUE_BASE 0x11004 > +#define CS40L50_MBOX_QUEUE_END 0x1101C > +#define CS40L50_DSP_VIRTUAL1_MBOX_1 0x11020 > +#define CS40L50_DSP1_MBOX_QUEUE_WT 0x28042C8 > +#define CS40L50_DSP1_MBOX_QUEUE_RD 0x28042CC > +#define CS40L50_DSP_MBOX_HAPTIC_COMPLETE_MBOX 0x1000000 > +#define CS40L50_DSP_MBOX_HAPTIC_COMPLETE_GPIO 0x1000001 > +#define CS40L50_DSP_MBOX_HAPTIC_COMPLETE_I2S 0x1000002 > +#define CS40L50_DSP_MBOX_HAPTIC_TRIGGER_MBOX 0x1000010 > +#define CS40L50_DSP_MBOX_HAPTIC_TRIGGER_GPIO 0x1000011 > +#define CS40L50_DSP_MBOX_HAPTIC_TRIGGER_I2S 0x1000012 > +#define CS40L50_DSP_MBOX_INIT 0x2000000 > +#define CS40L50_DSP_MBOX_AWAKE 0x2000002 > +#define CS40L50_DSP_MBOX_F0_EST_START 0x7000011 > +#define CS40L50_DSP_MBOX_F0_EST_DONE 0x7000021 > +#define CS40L50_DSP_MBOX_REDC_EST_START 0x7000012 > +#define CS40L50_DSP_MBOX_REDC_EST_DONE 0x7000022 > +#define CS40L50_DSP_MBOX_LE_EST_START 0x7000014 > +#define CS40L50_DSP_MBOX_LE_EST_DONE 0x7000024 > +#define CS40L50_DSP_MBOX_ACK 0xA000000 > +#define CS40L50_DSP_MBOX_PANIC 0xC000000 > +#define CS40L50_DSP_MBOX_WATERMARK 0xD000000 > +#define CS40L50_DSP_MBOX_ERR_EVENT_UNMAPPED 0xC0004B3 > +#define CS40L50_DSP_MBOX_ERR_EVENT_MODIFY 0xC0004B4 > +#define CS40L50_DSP_MBOX_ERR_NULLPTR 0xC0006A5 > +#define CS40L50_DSP_MBOX_ERR_BRAKING 0xC0006A8 > +#define CS40L50_DSP_MBOX_ERR_INVAL_SRC 0xC0006AC > +#define CS40L50_DSP_MBOX_ERR_ENABLE_RANGE 0xC000836 > +#define CS40L50_DSP_MBOX_ERR_GPIO_UNMAPPED 0xC000837 > +#define CS40L50_DSP_MBOX_ERR_ISR_RANGE 0xC000838 > +#define CS40L50_DSP_MBOX_PERMANENT_SHORT 0xC000C1C > +#define CS40L50_DSP_MBOX_RUNTIME_SHORT 0xC000C1D > + > +/* DSP */ > +#define CS40L50_DSP1_XMEM_PACKED_0 0x2000000 > +#define CS40L50_DSP1_XMEM_UNPACKED32_0 0x2400000 > +#define CS40L50_DSP1_SYS_INFO_ID 0x25E0000 > +#define CS40L50_DSP1_XMEM_UNPACKED24_0 0x2800000 > +#define CS40L50_NUM_OF_WAVES 0x280CB4C > +#define CS40L50_DSP1_CORE_BASE 0x2B80000 > +#define CS40L50_DSP1_SCRATCH1 0x2B805C0 > +#define CS40L50_DSP1_SCRATCH2 0x2B805C8 > +#define CS40L50_DSP1_SCRATCH3 0x2B805D0 > +#define CS40L50_DSP1_SCRATCH4 0x2B805D8 I didn't look too closely, but some of these #defines are unused. > +#define CS40L50_CCM_CORE_CONTROL 0x2BC1000 > +#define CS40L50_RAM_INIT 0x28021DC > +#define CS40L50_DSP1_YMEM_PACKED_0 0x2C00000 > +#define CS40L50_DSP1_YMEM_UNPACKED32_0 0x3000000 > +#define CS40L50_DSP1_YMEM_UNPACKED24_0 0x3400000 > +#define CS40L50_DSP1_PMEM_0 0x3800000 > +#define CS40L50_DSP1_PMEM_5114 0x3804FE8 > +#define CS40L50_DSP1_MEM_RDY_HW 0x2 > +#define CS40L50_DSP1_RAM_INIT_FLAG 0x1 > +#define CS40L50_DSP1_CLOCK_DISABLE 0x80 > +#define CS40L50_DSP1_CLOCK_ENABLE 0x281 > +#define CS40L50_DSP_POLL_US 1000 > +#define CS40L50_DSP_TIMEOUT_COUNT 100 > +#define CS40L50_MBOX_POLL_US 5000 > + > +/* Calibration */ > +#define CS40L50_DSP1_REDC_ESTIMATION 0x2802F7C > +#define CS40L50_DSP1_F0_ESTIMATION 0x2802F84 > +#define CS40L50_DSP1_DYNAMIC_F0_ENABLE 0x2802F8C > +#define CS40L50_DSP1_DYNAMIC_F0_THRESHOLD 0x2802F90 > +#define CS40L50_DSP1_F0_STORED 0x2805C00 > +#define CS40L50_DSP1_REDC_STORED 0x2805C04 > +#define CS40L50_DSP1_RE_EST_STATUS 0x3401B40 > +#define CS40L50_REDC_EST_DELAY_US 30000 > +#define CS40L50_F0_EST_DELAY_US 10000 > + > +/* OWT */ > +#define CS40L50_DSP1_OWT_SIZE 0x2805C38 > +#define CS40L50_DSP1_OWT_NEXT 0x2805C3C > +#define CS40L50_NUM_OF_OWT_WAVES 0x2805C40 > +#define CS40L50_DSP1_OWT_BASE 0x2805C34 > +#define CS40L50_MAX_PWLE_SECTIONS 126 > +#define CS40L50_CUSTOM_DATA_SIZE 2 > +#define CS40L50_WT_HEADER_PWLE_SIZE 12 > +#define CS40L50_WT_HEADER_DEFAULT_FLAGS 0x0000 > +#define CS40L50_WT_HEADER_OFFSET 3 > +#define CS40L50_WT_TYPE_PCM 8 > +#define CS40L50_WT_TYPE_PWLE 12 > +#define CS40L50_PCM_ID 0x0000 > + > +/* GPIO */ > +#define CS40L50_DSP1_GPIO_BASE 0x2804140 > +#define CS40L50_GPIO_MAPPING_INVALID 0 > +#define CS40L50_GPI_DISABLE 0x1FF > +#define CS40L50_BTN_NUM_MASK GENMASK(14, 12) > +#define CS40L50_BTN_EDGE_MASK BIT(15) > + > +/* State */ > +#define CS40L50_DSP1_HALO_STATE 0x28021E0 > +#define CS40L50_HALO_STATE_BOOT_DONE 2 > +#define CS40L50_MIN_RESET_PULSE_WIDTH_US 1100 > +#define CS40L50_CP_READY_DELAY_US 2200 > + > +/* Device */ > +#define CS40L50_DEVID 0x0 > +#define CS40L50_REVID 0x4 > +#define CS40L50_DEVID_A 0x40A50 > +#define CS40L50_REVID_B0 0xB0 > + > +/* Write sequencer */ > +#define CS40L50_DSP1_POWER_ON_SEQ 0x280C320 > +#define CS40L50_PSEQ_MAX_WORDS 200 > +#define CS40L50_PSEQ_OP_MASK GENMASK(23, 16) > +#define CS40L50_PSEQ_OP_SHIFT 16 > +#define CS40L50_PSEQ_OP_WRITE_ADDR8 0x02 > +#define CS40L50_PSEQ_OP_WRITE_L16 0x04 > +#define CS40L50_PSEQ_OP_WRITE_H16 0x05 > +#define CS40L50_PSEQ_OP_WRITE_FULL 0x00 > +#define CS40L50_PSEQ_OP_WRITE_FULL_WORDS 3 > +#define CS40L50_PSEQ_OP_WRITE_X16_WORDS 2 > +#define CS40L50_PSEQ_OP_END 0xFF > +#define CS40L50_PSEQ_OP_END_WORDS 1 > +#define CS40L50_PSEQ_WRITE_FULL_LOWER_ADDR_SHIFT 8 > +#define CS40L50_PSEQ_WRITE_FULL_UPPER_ADDR_SHIFT 16 > +#define CS40L50_PSEQ_WRITE_FULL_LOWER_ADDR_MASK GENMASK(15, 0) > +#define CS40L50_PSEQ_WRITE_FULL_UPPER_DATA_SHIFT 24 > +#define CS40L50_PSEQ_WRITE_FULL_LOWER_DATA_MASK GENMASK(23, 0) > + > +enum cs40l50_wvfrm_bank { > + CS40L50_WVFRM_BANK_RAM, > + CS40L50_WVFRM_BANK_ROM, > + CS40L50_WVFRM_BANK_OWT, > + CS40L50_WVFRM_BANK_BUZ, > + CS40L50_WVFRM_BANK_NUM, > +}; > + > +enum cs40l50_irq_list { > + CS40L50_GLOBAL_ERROR_IRQ, > + CS40L50_UVLO_VDDBATT_IRQ, > + CS40L50_BST_ILIMIT_IRQ, > + CS40L50_BST_SHORT_IRQ, > + CS40L50_BST_UVP_IRQ, > + CS40L50_TEMP_ERR_IRQ, > + CS40L50_VIRT2_MBOX_IRQ, > + CS40L50_AMP_SHORT_IRQ, > + CS40L50_NUM_IRQ > +}; > + > +union cs40l50_buzzgen { > + u32 words[3]; > + struct { > + u32 freq; > + u32 level; > + u32 duration; > + } params; > +}; > + > +struct cs40l50_owt_header { > + u32 wvfrm_type; > + u32 offset; > + u32 data_size; > +}; > + > +struct cs40l50_irq { > + int irq; > + const char *name; > + irqreturn_t (*handler)(int irq, void *data); > +}; > + > +struct cs40l50_effect { > + int id; > + u32 trigger_index; > + enum cs40l50_wvfrm_bank wvfrm_bank; > + u32 mapping; > + struct list_head list; > +}; > + > +struct cs40l50_pseq_op { > + u16 offset; > + u8 operation; > + u32 words[3]; > + u32 size; > + struct list_head list; > +}; > + > +struct cs40l50_private { > + struct device *dev; > + struct regmap *regmap; > + struct cs_dsp dsp; > + struct mutex lock; > + struct gpio_desc *reset_gpio; > + struct input_dev *input; > + struct list_head effect_head; > + struct ff_effect *trigger_effect; > + struct workqueue_struct *vibe_workqueue; > + struct work_struct vibe_start_work; > + struct work_struct vibe_stop_work; > + struct regmap_irq_chip_data *irq_data; > + struct list_head pseq_op_head; > + int irq; > +}; > + > +int cs40l50_probe(struct cs40l50_private *cs40l50); > +int cs40l50_remove(struct cs40l50_private *cs40l50); > + > +extern const struct regmap_config cs40l50_regmap; > + > +#endif /* __CS40L50_H__ */ > -- > 2.25.1 > Kind regards, Jeff LaBundy
> On Aug 10, 2023, at 1:17 AM, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 09/08/2023 21:10, James Ogletree wrote: > >> + >> +static int cs40l50_cs_dsp_init(struct cs40l50_private *cs40l50) >> +{ >> + cs40l50->dsp.num = 1; >> + cs40l50->dsp.type = WMFW_HALO; >> + cs40l50->dsp.dev = cs40l50->dev; >> + cs40l50->dsp.regmap = cs40l50->regmap; >> + cs40l50->dsp.base = CS40L50_DSP1_CORE_BASE; >> + cs40l50->dsp.base_sysinfo = CS40L50_DSP1_SYS_INFO_ID; >> + cs40l50->dsp.mem = cs40l50_dsp_regions; >> + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions); >> + cs40l50->dsp.lock_regions = 0xFFFFFFFF; >> + cs40l50->dsp.no_core_startstop = true; >> + cs40l50->dsp.client_ops = &cs40l50_cs_dsp_client_ops; >> + >> + return cs_dsp_halo_init(&cs40l50->dsp); >> +} >> + >> +int cs40l50_probe(struct cs40l50_private *cs40l50) >> +{ >> + int error, i, irq; >> + u32 val; >> + >> + mutex_init(&cs40l50->lock); >> + >> + error = devm_regulator_bulk_get(cs40l50->dev, ARRAY_SIZE(cs40l50_supplies), >> + cs40l50_supplies); >> + if (error) >> + return dev_err_probe(cs40l50->dev, error, "Failed to request supplies\n"); >> + >> + error = regulator_bulk_enable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies); >> + if (error) >> + return dev_err_probe(cs40l50->dev, error, "Failed to enable supplies\n"); >> + >> + cs40l50->reset_gpio = devm_gpiod_get_optional(cs40l50->dev, "reset", GPIOD_OUT_HIGH); > > None of the lines above or below seem to be wrapped according to Linux > coding style (80). This patch abides by the 100-column line length limit which checkpatch.pl enforces. However, I can see how some of the lines might be less jarring to the eyes if wrapped. That will be addressed in V4. > >> + if (IS_ERR(cs40l50->reset_gpio)) { >> + error = PTR_ERR(cs40l50->reset_gpio); >> + goto err; > > Why do you disable IRQ now? > > I asked to test this. Your entire cleanup path is: > 1. not tested. > 2. buggy. > 3. done differently than Linux style. Use proper cleanup calls and > multiple goto labels. Disabling IRQ there is a mistake. The probe error path will be tightened up and made to utilize multiple goto labels instead in V4. Regards, James Ogletree
On 15/08/2023 17:56, James Ogletree wrote: > > >> On Aug 10, 2023, at 1:17 AM, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: >> >> On 09/08/2023 21:10, James Ogletree wrote: >> >>> + >>> +static int cs40l50_cs_dsp_init(struct cs40l50_private *cs40l50) >>> +{ >>> + cs40l50->dsp.num = 1; >>> + cs40l50->dsp.type = WMFW_HALO; >>> + cs40l50->dsp.dev = cs40l50->dev; >>> + cs40l50->dsp.regmap = cs40l50->regmap; >>> + cs40l50->dsp.base = CS40L50_DSP1_CORE_BASE; >>> + cs40l50->dsp.base_sysinfo = CS40L50_DSP1_SYS_INFO_ID; >>> + cs40l50->dsp.mem = cs40l50_dsp_regions; >>> + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions); >>> + cs40l50->dsp.lock_regions = 0xFFFFFFFF; >>> + cs40l50->dsp.no_core_startstop = true; >>> + cs40l50->dsp.client_ops = &cs40l50_cs_dsp_client_ops; >>> + >>> + return cs_dsp_halo_init(&cs40l50->dsp); >>> +} >>> + >>> +int cs40l50_probe(struct cs40l50_private *cs40l50) >>> +{ >>> + int error, i, irq; >>> + u32 val; >>> + >>> + mutex_init(&cs40l50->lock); >>> + >>> + error = devm_regulator_bulk_get(cs40l50->dev, ARRAY_SIZE(cs40l50_supplies), >>> + cs40l50_supplies); >>> + if (error) >>> + return dev_err_probe(cs40l50->dev, error, "Failed to request supplies\n"); >>> + >>> + error = regulator_bulk_enable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies); >>> + if (error) >>> + return dev_err_probe(cs40l50->dev, error, "Failed to enable supplies\n"); >>> + >>> + cs40l50->reset_gpio = devm_gpiod_get_optional(cs40l50->dev, "reset", GPIOD_OUT_HIGH); >> >> None of the lines above or below seem to be wrapped according to Linux >> coding style (80). > > This patch abides by the 100-column line length limit which checkpatch.pl enforces. Checkpatch does not enforce 100, because it accepts also 80. It complains if something is longer than 100, but coding style is still the same. Format everything according to coding style and use 100 only if it increases readability. > However, I can see how some of the lines might be less jarring to the eyes if wrapped. > That will be addressed in V4. > Best regards, Krzysztof
> On Aug 10, 2023, at 5:30 AM, Charles Keepax <ckeepax@opensource.cirrus.com> wrote: > > On Wed, Aug 09, 2023 at 07:10:28PM +0000, James Ogletree wrote: >> + >> +static int cs40l50_pseq_write(struct cs40l50_private *cs40l50, u32 addr, u32 data) >> +{ >> + >> +static int cs40l50_owt_upload(struct cs40l50_private *cs40l50, s16 *in_data, u32 in_data_nibbles) >> +{ > > These pseq and OWT bits, could they be shared with l26? > Definitely worth syncing with those guys, my assumption is the > wavetable/pseq won't have changed much and it might be nice to > factor these bits out into some library code that both drivers > can use. The pseq code most certainly can, likely the OWT code, perhaps others. I assume it is acceptable to move some of these functions to a library in this patch set, even though this is the only driver utilizing them as far as mainline is concerned? In other words, we shouldn’t wait for one of L26 or L50 drivers to be accepted before splitting off the common code as part of the others’ patchset? I’m probably overcomplicating; just want to be sure on the process here. Everything else in your review will be fixed in V4. Thank you. Regards, James Ogletree
On Tue, Aug 15, 2023 at 10:33:00PM +0000, James Ogletree wrote: > > > > On Aug 10, 2023, at 5:30 AM, Charles Keepax <ckeepax@opensource.cirrus.com> wrote: > > > > On Wed, Aug 09, 2023 at 07:10:28PM +0000, James Ogletree wrote: > >> + > >> +static int cs40l50_pseq_write(struct cs40l50_private *cs40l50, u32 addr, u32 data) > >> +{ > >> + > >> +static int cs40l50_owt_upload(struct cs40l50_private *cs40l50, s16 *in_data, u32 in_data_nibbles) > >> +{ > > > > These pseq and OWT bits, could they be shared with l26? > > Definitely worth syncing with those guys, my assumption is the > > wavetable/pseq won't have changed much and it might be nice to > > factor these bits out into some library code that both drivers > > can use. > > The pseq code most certainly can, likely the OWT code, perhaps others. I assume it is > acceptable to move some of these functions to a library in this patch set, even though this is > the only driver utilizing them as far as mainline is concerned? In other words, we shouldn’t > wait for one of L26 or L50 drivers to be accepted before splitting off the common code as part > of the others’ patchset? I’m probably overcomplicating; just want to be sure on the process here. > > Everything else in your review will be fixed in V4. Thank you. > I think this makes sense to do now, just need to make sure the next series of L26 is synced up to it. Thanks, Charles
> On Aug 10, 2023, at 11:07 PM, Jeff LaBundy <jeff@labundy.com> wrote: > > On Wed, Aug 09, 2023 at 07:10:28PM +0000, James Ogletree wrote: >> Introduce support for Cirrus Logic Device CS40L50: a >> haptics driver with waveform memory DSP and closed-loop >> algorithms. > > From my extremely naive point of view, some of the code that follows > bares resemblance to the recently reviewed L26. My assumption is that > these devices are different enough in nature to warrant completely > different drivers; is that accurate? > > One reason for asking is because the L26 driver included a cornucopia > of power-management overhead, yet we see none of that here. Assuming > both L50 and L26 are built around the same Halo DSP, why is there such > a fundamental difference between the two in terms of power management? > > To that end, how does this driver handle hibernation? Is hibernation > not supported, or do we simply defer to the DSP? In the case of the > latter, why is L50 given this freedom but not L26? One key difference is that L50’s Halo Core DSP is self-booting; the firmware is burned in and no firmware download is required. On L26, firmware downloading is compulsory. This differentiates dealing with the DSP in the two drivers, because the L50 driver does not need to do a look up every time it reads or writes to a firmware control. The registers are all static. Minor reasons are that they have different power supply configurations that require different register settings, they have errata differences, and a different set of exposed features (L50 being much more simplistic). I think taken cumulatively these differences warrant separate drivers. Though, I will take Charles’ recommendation to factor out the similarities into a shared library that both L50 and L26 can use. Let me know whether you disagree on the above points or have followup questions. With respect to power management, I did not think that that there was any merit in itself in maintaining equality with L26’s approach, and I was inclined to accept your reasoning for using retry logic over the runtime PM facilities (not that the latter way is incorrect). Regarding the need for I2S streaming support, signs point to maybe. I will migrate this driver to MFD for V4. > >> + return cs40l50_dsp_write(cs40l50, CS40L50_BST_LPMODE_SEL, CS40L50_DCM_LOW_POWER); >> +} >> + >> +static int cs40l50_patch_firmware(struct cs40l50_private *cs40l50) >> +{ >> + const struct firmware *bin = NULL, *wmfw = NULL; >> + int error = 0; >> + >> + if (request_firmware(&bin, "cs40l50.bin", cs40l50->dev)) >> + dev_info(cs40l50->dev, "Could not request wavetable file: %d\n", error); >> + >> + if (request_firmware(&wmfw, "cs40l50.wmfw", cs40l50->dev)) >> + dev_info(cs40l50->dev, "Could not request firmware patch file: %d\n", error); >> + >> + if (wmfw) { > > It is a much more common design pattern to bail if request_firmware() returns > an error, than to proceed and check against the FW pointer remaining NULL. > > Is this done because cs_dsp_power_up() must be called, with or without a wmfw > or coefficient file being available? I don’t think that cs_dsp_power_up() must be called in the case of non-existent .wmfw and .bin files, so I will take your suggestion and optimize this function. I will also switch to asynchronous firmware requesting for V4. I will also incorporate the rest of your review comments not mentioned here in V4. Thank you for the thorough review. Regards, James Ogletree
Hi James, On Wed, Aug 16, 2023 at 09:02:26PM +0000, James Ogletree wrote: > > > > On Aug 10, 2023, at 11:07 PM, Jeff LaBundy <jeff@labundy.com> wrote: > > > > On Wed, Aug 09, 2023 at 07:10:28PM +0000, James Ogletree wrote: > >> Introduce support for Cirrus Logic Device CS40L50: a > >> haptics driver with waveform memory DSP and closed-loop > >> algorithms. > > > > From my extremely naive point of view, some of the code that follows > > bares resemblance to the recently reviewed L26. My assumption is that > > these devices are different enough in nature to warrant completely > > different drivers; is that accurate? > > > > One reason for asking is because the L26 driver included a cornucopia > > of power-management overhead, yet we see none of that here. Assuming > > both L50 and L26 are built around the same Halo DSP, why is there such > > a fundamental difference between the two in terms of power management? > > > > To that end, how does this driver handle hibernation? Is hibernation > > not supported, or do we simply defer to the DSP? In the case of the > > latter, why is L50 given this freedom but not L26? > > One key difference is that L50’s Halo Core DSP is self-booting; the firmware > is burned in and no firmware download is required. On L26, firmware > downloading is compulsory. This differentiates dealing with the DSP in the > two drivers, because the L50 driver does not need to do a look up every > time it reads or writes to a firmware control. The registers are all static. Interesting stuff; thanks for sharing that background information. > > Minor reasons are that they have different power supply configurations that > require different register settings, they have errata differences, and a different set > of exposed features (L50 being much more simplistic). I think taken cumulatively > these differences warrant separate drivers. Though, I will take Charles’ > recommendation to factor out the similarities into a shared library that both L50 > and L26 can use. > > Let me know whether you disagree on the above points or have followup > questions. Makes sense to me. > > With respect to power management, I did not think that that there was any merit > in itself in maintaining equality with L26’s approach, and I was inclined to accept > your reasoning for using retry logic over the runtime PM facilities (not that the > latter way is incorrect). > > Regarding the need for I2S streaming support, signs point to maybe. I will > migrate this driver to MFD for V4. MFD seems like the right (i.e. scalable) solution if A2H is on the roadmap. In early L25 days, very early adopters simply asked for a sysfs control exposed from the core (then LED !!) driver to enable/disable I2S streaming mode, but this got hairy in case customers needed to control BCLK/Fs ratio, bit depth, etc. during runtime. From my naive point of view, maybe the solution looks something like this: * drivers/mfd/cs40l50-i2c.c: I2C client * drivers/mfd/cs40l50-spi.c: SPI client * drivers/mfd/cs40l50-core.c: common tasks such as FW loading, OTP management, etc; perhaps L26 can leverage it as well * drivers/input/misc/cs40l50-vibra.c: FF-specific support (name as you like, I just picked what seems to be most common) * sound/soc/codecs/cs40l50-codec.c: codec-specific support In case I have misunderstood, please let me know. > > > > >> + return cs40l50_dsp_write(cs40l50, CS40L50_BST_LPMODE_SEL, CS40L50_DCM_LOW_POWER); > >> +} > >> + > >> +static int cs40l50_patch_firmware(struct cs40l50_private *cs40l50) > >> +{ > >> + const struct firmware *bin = NULL, *wmfw = NULL; > >> + int error = 0; > >> + > >> + if (request_firmware(&bin, "cs40l50.bin", cs40l50->dev)) > >> + dev_info(cs40l50->dev, "Could not request wavetable file: %d\n", error); > >> + > >> + if (request_firmware(&wmfw, "cs40l50.wmfw", cs40l50->dev)) > >> + dev_info(cs40l50->dev, "Could not request firmware patch file: %d\n", error); > >> + > >> + if (wmfw) { > > > > It is a much more common design pattern to bail if request_firmware() returns > > an error, than to proceed and check against the FW pointer remaining NULL. > > > > Is this done because cs_dsp_power_up() must be called, with or without a wmfw > > or coefficient file being available? > > I don’t think that cs_dsp_power_up() must be called in the case of non-existent .wmfw > and .bin files, so I will take your suggestion and optimize this function. I will also switch > to asynchronous firmware requesting for V4. Since L50 can work out of the box without FW, another option is to follow the approach used by some codec drivers and wait to load FW until the first request to stream I2S, and/or trigger an FF effect in this case. By that point, rootfs has been available for some time and request_firmware() will succeed. This is advantageous because even though request_firmware_nowait() can solve the deadlock problem for FW loaded at probe, it could still return -ENOENT right away on some platforms. The disadvantage is that the first effect would be very delayed due to the overhead of transferring several kB of FW over I2C for the first time. That's OK for audio applications, but not haptics where delay normally must be within single-digit ms. Ultimately, I think the right approach is to look for FW from probe as you have done, but use request_firmware_nowait(). However, I recommend structuring the driver such that rip-up would be minimal in case you had to retry FW loading from the first FF trigger, to support a customer whose rootfs is not available at probe. In such a case, the customer could work around the first-time latency problem by triggering a dummy effect early in their boot (e.g. init.rc). > > I will also incorporate the rest of your review comments not mentioned here in V4. > Thank you for the thorough review. Thank you for the productive discussion! > > Regards, > James Ogletree Kind regards, Jeff LaBundy
diff --git a/MAINTAINERS b/MAINTAINERS index 2869dd2dbbcf..e82d03293383 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2088,6 +2088,8 @@ M: Ben Bright <ben.bright@cirrus.com> L: patches@opensource.cirrus.com S: Supported F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml +F: drivers/input/misc/cs40l* +F: include/linux/input/cs40l* ARM/CLKDEV SUPPORT M: Russell King <linux@armlinux.org.uk> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig index c47eecc6f83b..c1c1572ec39c 100644 --- a/drivers/input/misc/Kconfig +++ b/drivers/input/misc/Kconfig @@ -140,6 +140,39 @@ config INPUT_BMA150 To compile this driver as a module, choose M here: the module will be called bma150. +config INPUT_CS40L50 + tristate "Cirrus Logic CS40L50 Haptic Driver support" + select CS_DSP + select REGMAP_IRQ + help + Say Y here to enable support for CS40L50 boosted haptic + driver. + + To compile the driver as a module choose M here: the module + will be called cs40l50. + +config INPUT_CS40L50_I2C + tristate "Support I2C bus connection" + depends on INPUT_CS40L50 && I2C + select REGMAP_I2C + help + Say Y here if you have Cirrus Logic's CS40L50 connected + to an I2C bus. + + To compile the driver as a module choose M here: the + module will be called cs40l50_i2c. + +config INPUT_CS40L50_SPI + tristate "Support SPI bus connection" + depends on INPUT_CS40L50 && SPI + select REGMAP_SPI + help + Say Y here if you have Cirrus Logic's CS40L50 connected + to a SPI bus. + + To compile the driver as a module choose M here: the + module will be called cs40l50_spi. + config INPUT_E3X0_BUTTON tristate "NI Ettus Research USRP E3xx Button support." default n diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile index 04296a4abe8e..77f34a33c364 100644 --- a/drivers/input/misc/Makefile +++ b/drivers/input/misc/Makefile @@ -28,6 +28,9 @@ obj-$(CONFIG_INPUT_CMA3000) += cma3000_d0x.o obj-$(CONFIG_INPUT_CMA3000_I2C) += cma3000_d0x_i2c.o obj-$(CONFIG_INPUT_COBALT_BTNS) += cobalt_btns.o obj-$(CONFIG_INPUT_CPCAP_PWRBUTTON) += cpcap-pwrbutton.o +obj-$(CONFIG_INPUT_CS40L50) += cs40l50.o +obj-$(CONFIG_INPUT_CS40L50_I2C) += cs40l50-i2c.o +obj-$(CONFIG_INPUT_CS40L50_SPI) += cs40l50-spi.o obj-$(CONFIG_INPUT_DA7280_HAPTICS) += da7280.o obj-$(CONFIG_INPUT_DA9052_ONKEY) += da9052_onkey.o obj-$(CONFIG_INPUT_DA9055_ONKEY) += da9055_onkey.o diff --git a/drivers/input/misc/cs40l50-i2c.c b/drivers/input/misc/cs40l50-i2c.c new file mode 100644 index 000000000000..6ebe1b51990b --- /dev/null +++ b/drivers/input/misc/cs40l50-i2c.c @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * CS40L50 Advanced Haptic Driver with Waveform Memory DSP + * and Closed-Loop Algorithms + * + * Copyright 2023 Cirrus Logic, Inc. + * + */ +#include <linux/i2c.h> +#include <linux/input/cs40l50.h> + +static const struct i2c_device_id cs40l50_id_i2c[] = { + {"cs40l50", 0}, + {} +}; +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 int cs40l50_i2c_probe(struct i2c_client *client) +{ + struct device *dev = &client->dev; + struct cs40l50_private *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_private *cs40l50 = i2c_get_clientdata(client); + + cs40l50_remove(cs40l50); +} + +static struct i2c_driver cs40l50_i2c_driver = { + .driver = { + .name = "cs40l50", + .of_match_table = cs40l50_of_match, + }, + .id_table = cs40l50_id_i2c, + .probe_new = 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@cirrus.com>"); +MODULE_LICENSE("GPL"); diff --git a/drivers/input/misc/cs40l50-spi.c b/drivers/input/misc/cs40l50-spi.c new file mode 100644 index 000000000000..5fcf3ba137f0 --- /dev/null +++ b/drivers/input/misc/cs40l50-spi.c @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * CS40L50 Advanced Haptic Driver with Waveform Memory DSP + * and Closed-Loop Algorithms + * + * Copyright 2023 Cirrus Logic, Inc. + * + */ +#include <linux/spi/spi.h> +#include <linux/input/cs40l50.h> + +static const struct spi_device_id cs40l50_id_spi[] = { + {"cs40l50", 0}, + {} +}; +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 int cs40l50_spi_probe(struct spi_device *spi) +{ + struct cs40l50_private *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_private *cs40l50 = spi_get_drvdata(spi); + + cs40l50_remove(cs40l50); +} + +static struct spi_driver cs40l50_spi_driver = { + .driver = { + .name = "cs40l50", + .of_match_table = cs40l50_of_match, + }, + .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@cirrus.com>"); +MODULE_LICENSE("GPL"); diff --git a/drivers/input/misc/cs40l50.c b/drivers/input/misc/cs40l50.c new file mode 100644 index 000000000000..9bb2881683c0 --- /dev/null +++ b/drivers/input/misc/cs40l50.c @@ -0,0 +1,1008 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * CS40L50 Advanced Haptic Driver with Waveform Memory DSP + * and Closed-Loop Algorithms + * + * Copyright 2023 Cirrus Logic, Inc. + * + */ +#include <linux/firmware.h> +#include <linux/i2c.h> +#include <linux/interrupt.h> +#include <linux/kernel.h> +#include <linux/string.h> +#include <linux/bitfield.h> +#include <linux/input/cs40l50.h> + +const struct regmap_config cs40l50_regmap = { + .reg_bits = 32, + .val_bits = 32, + .reg_stride = 4, + .reg_format_endian = REGMAP_ENDIAN_BIG, + .val_format_endian = REGMAP_ENDIAN_BIG, +}; +EXPORT_SYMBOL_GPL(cs40l50_regmap); + +static struct regulator_bulk_data cs40l50_supplies[] = { + { .supply = "vp" }, + { .supply = "vio" }, + { .supply = "va" }, +}; + +static int cs40l50_dsp_write(struct cs40l50_private *cs40l50, u32 write_reg, u32 write_val) +{ + int error = 0, i; + + for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) { + error = regmap_write(cs40l50->regmap, write_reg, write_val); + if (!error) + break; + + usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100); + } + + if (i == CS40L50_DSP_TIMEOUT_COUNT) { + dev_err(cs40l50->dev, "Error writing %#X to %#X: %d", write_val, write_reg, error); + return error; + } + + return 0; +} + +static int cs40l50_dsp_read(struct cs40l50_private *cs40l50, u32 read_reg, u32 *read_val) +{ + int error = 0, i; + + for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) { + error = regmap_read(cs40l50->regmap, read_reg, read_val); + if (!error) + break; + + usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100); + } + + if (i == CS40L50_DSP_TIMEOUT_COUNT) { + dev_err(cs40l50->dev, "Error reading %#X: %d", read_reg, error); + return error; + } + + return 0; +} + +static int cs40l50_pseq_write(struct cs40l50_private *cs40l50, u32 addr, u32 data) +{ + u8 operation = 0; + struct cs40l50_pseq_op *op_new, *op_end; + int error; + + op_new = devm_kzalloc(cs40l50->dev, sizeof(*op_new), GFP_KERNEL); + if (!op_new) + return -ENOMEM; + + op_new->words[0] = (addr >> CS40L50_PSEQ_WRITE_FULL_UPPER_ADDR_SHIFT); + op_new->words[1] = ((addr & CS40L50_PSEQ_WRITE_FULL_LOWER_ADDR_MASK) << + CS40L50_PSEQ_WRITE_FULL_LOWER_ADDR_SHIFT) | + (data >> CS40L50_PSEQ_WRITE_FULL_UPPER_DATA_SHIFT); + op_new->words[2] = data & CS40L50_PSEQ_WRITE_FULL_LOWER_DATA_MASK; + + list_for_each_entry(op_end, &cs40l50->pseq_op_head, list) { + operation = op_end->operation; + if (operation == CS40L50_PSEQ_OP_END) + break; + } + + if (operation != CS40L50_PSEQ_OP_END) { + error = -ENOENT; + goto op_new_free; + } + + if ((CS40L50_PSEQ_MAX_WORDS - op_end->offset) < ARRAY_SIZE(op_new->words)) { + error = -ENOSPC; + goto op_new_free; + } + + op_new->offset = op_end->offset; + op_end->offset += (ARRAY_SIZE(op_new->words) * sizeof(u32)); + + error = regmap_bulk_write(cs40l50->regmap, CS40L50_DSP1_POWER_ON_SEQ + op_new->offset, + op_new->words, ARRAY_SIZE(op_new->words)); + if (error) + goto op_new_free; + + error = cs40l50_dsp_write(cs40l50, CS40L50_DSP1_POWER_ON_SEQ + op_end->offset, + op_end->words[0]); + if (error) + goto op_new_free; + + list_add(&op_new->list, &cs40l50->pseq_op_head); + + return 0; + +op_new_free: + devm_kfree(cs40l50->dev, op_new); + + return error; +} + +static struct cs40l50_effect *cs40l50_find_effect(struct cs40l50_private *cs40l50, int id) +{ + struct cs40l50_effect *effect; + + if (!list_empty(&cs40l50->effect_head)) { + list_for_each_entry(effect, &cs40l50->effect_head, list) { + if (effect->id == id) + return effect; + } + } + + return NULL; +} + +static int cs40l50_owt_upload(struct cs40l50_private *cs40l50, s16 *in_data, u32 in_data_nibbles) +{ + size_t in_data_bytes = 2 * in_data_nibbles; + struct cs40l50_owt_header header; + u32 wt_offset, wt_size_words; + int error; + u8 *data; + + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_OWT_NEXT, &wt_offset); + if (error) + return error; + + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_OWT_SIZE, &wt_size_words); + if (error) + return error; + + if ((wt_size_words * sizeof(u32)) < CS40L50_WT_HEADER_PWLE_SIZE + in_data_bytes) + return -ENOSPC; + + data = kcalloc(CS40L50_WT_HEADER_PWLE_SIZE + in_data_bytes, sizeof(u8), GFP_KERNEL); + if (IS_ERR_OR_NULL(data)) + return -ENOMEM; + + header.wvfrm_type = in_data[0] == CS40L50_PCM_ID ? CS40L50_WT_TYPE_PCM : + CS40L50_WT_TYPE_PWLE; + header.offset = CS40L50_WT_HEADER_OFFSET; + header.data_size = in_data_bytes / sizeof(u32); + + memcpy(data, &header, sizeof(header)); + memcpy(data + (CS40L50_WT_HEADER_OFFSET * sizeof(u32)), in_data, in_data_bytes); + + error = regmap_bulk_write(cs40l50->regmap, CS40L50_DSP1_OWT_BASE + + (wt_offset * sizeof(u32)), data, + CS40L50_WT_HEADER_PWLE_SIZE + in_data_bytes); + if (error) + goto err_free; + + error = cs40l50_dsp_write(cs40l50, CS40L50_DSP_VIRTUAL1_MBOX_1, CS40L50_MBOX_CMD_OWT_PUSH); +err_free: + kfree(data); + + return error; +} + +static int cs40l50_add_effect(struct input_dev *dev, struct ff_effect *effect, + struct ff_effect *old) +{ + struct cs40l50_private *cs40l50 = input_get_drvdata(dev); + bool is_new = false; + struct cs40l50_effect *new_effect; + u32 nwaves, base_index, max_index; + s16 *raw_custom_data; + int error; + + if (effect->type != FF_PERIODIC || effect->u.periodic.waveform != FF_CUSTOM) { + dev_err(cs40l50->dev, "Type (%#X) or waveform (%#X) unsupported\n", + effect->type, effect->u.periodic.waveform); + return -EINVAL; + } + + raw_custom_data = kcalloc(effect->u.periodic.custom_len, sizeof(s16), GFP_KERNEL); + if (IS_ERR_OR_NULL(raw_custom_data)) + return -ENOMEM; + + error = copy_from_user(raw_custom_data, effect->u.periodic.custom_data, + sizeof(s16) * effect->u.periodic.custom_len); + if (error) + goto out_free; + + mutex_lock(&cs40l50->lock); + + new_effect = cs40l50_find_effect(cs40l50, effect->id); + if (!new_effect) { + is_new = true; + new_effect = kzalloc(sizeof(*new_effect), GFP_KERNEL); + if (IS_ERR_OR_NULL(new_effect)) { + error = -ENOMEM; + goto err_mutex; + } + } + + if (effect->u.periodic.custom_len > CS40L50_CUSTOM_DATA_SIZE) { + error = cs40l50_dsp_read(cs40l50, CS40L50_NUM_OF_OWT_WAVES, + &new_effect->trigger_index); + if (error) + goto err_free; + + new_effect->wvfrm_bank = CS40L50_WVFRM_BANK_OWT; + base_index = CS40L50_OWT_INDEX_START; + max_index = CS40L50_OWT_INDEX_END; + } else { + new_effect->trigger_index = (raw_custom_data[1] & 0xffffu); + new_effect->wvfrm_bank = (raw_custom_data[0] & 0xffffu); + + if (new_effect->wvfrm_bank == CS40L50_WVFRM_BANK_ROM) { + base_index = CS40L50_ROM_BANK_INDEX_START; + max_index = CS40L50_ROM_BANK_INDEX_END; + } else if (new_effect->wvfrm_bank == CS40L50_WVFRM_BANK_RAM) { + error = cs40l50_dsp_read(cs40l50, CS40L50_NUM_OF_WAVES, &nwaves); + if (error) + goto err_free; + + base_index = CS40L50_RAM_BANK_INDEX_START; + max_index = base_index + nwaves - 1; + } else { + dev_err(cs40l50->dev, "Invalid waveform bank\n"); + error = -EINVAL; + goto err_free; + } + } + + new_effect->trigger_index += base_index; + + if (new_effect->trigger_index > max_index) { + dev_err(cs40l50->dev, "Index %#X out of bounds\n", new_effect->trigger_index); + error = -EINVAL; + goto err_free; + } + + if (effect->trigger.button) { + new_effect->mapping = CS40L50_DSP1_GPIO_BASE + sizeof(u32) * + FIELD_GET(CS40L50_BTN_NUM_MASK, effect->trigger.button) * 2 - + FIELD_GET(CS40L50_BTN_EDGE_MASK, effect->trigger.button); + + error = cs40l50_dsp_write(cs40l50, new_effect->mapping, effect->trigger.button); + if (error) + goto err_free; + } + + if (new_effect->wvfrm_bank == CS40L50_WVFRM_BANK_OWT) { + error = cs40l50_owt_upload(cs40l50, raw_custom_data, + effect->u.periodic.custom_len); + if (error) + goto err_free; + } + +err_free: + if (is_new) + error ? kfree(new_effect) : list_add(&new_effect->list, &cs40l50->effect_head); +err_mutex: + mutex_unlock(&cs40l50->lock); +out_free: + kfree(raw_custom_data); + + return error ? -EFAULT : 0; +} + + +static void cs40l50_vibe_start_worker(struct work_struct *work) +{ + struct cs40l50_private *cs40l50 = + container_of(work, struct cs40l50_private, vibe_start_work); + struct cs40l50_effect *effect; + + mutex_lock(&cs40l50->lock); + + effect = cs40l50_find_effect(cs40l50, cs40l50->trigger_effect->id); + if (effect) + cs40l50_dsp_write(cs40l50, CS40L50_DSP_VIRTUAL1_MBOX_1, effect->trigger_index); + + mutex_unlock(&cs40l50->lock); +} + +static void cs40l50_vibe_stop_worker(struct work_struct *work) +{ + struct cs40l50_private *cs40l50 = + container_of(work, struct cs40l50_private, vibe_stop_work); + + mutex_lock(&cs40l50->lock); + + cs40l50_dsp_write(cs40l50, CS40L50_DSP_VIRTUAL1_MBOX_1, CS40L50_MBOX_CMD_STOP_PLAYBACK); + + mutex_unlock(&cs40l50->lock); +} + +static int cs40l50_playback_effect(struct input_dev *dev, int effect_id, int val) +{ + struct cs40l50_private *cs40l50 = input_get_drvdata(dev); + + cs40l50->trigger_effect = &dev->ff->effects[effect_id]; + + if (val > 0) + queue_work(cs40l50->vibe_workqueue, &cs40l50->vibe_start_work); + else + queue_work(cs40l50->vibe_workqueue, &cs40l50->vibe_stop_work); + + return 0; +} + +static int cs40l50_erase_effect(struct input_dev *dev, int effect_id) +{ + int error = 0; + struct cs40l50_private *cs40l50 = input_get_drvdata(dev); + struct cs40l50_effect *effect, *effect_owt; + + mutex_lock(&cs40l50->lock); + + effect = cs40l50_find_effect(cs40l50, dev->ff->effects[effect_id].id); + if (!effect) { + error = -EINVAL; + goto err_mutex; + } + + if (effect->mapping != CS40L50_GPIO_MAPPING_INVALID) { + error = cs40l50_dsp_write(cs40l50, effect->mapping, CS40L50_GPI_DISABLE); + if (error) + goto err_mutex; + } + + if (effect->wvfrm_bank == CS40L50_WVFRM_BANK_OWT) { + error = cs40l50_dsp_write(cs40l50, CS40L50_DSP_VIRTUAL1_MBOX_1, + CS40L50_MBOX_CMD_OWT_DELETE | effect->trigger_index); + if (error) + goto err_mutex; + + list_for_each_entry(effect_owt, &cs40l50->effect_head, list) { + if (effect_owt->wvfrm_bank == CS40L50_WVFRM_BANK_OWT && + effect_owt->trigger_index > effect->trigger_index) + effect_owt->trigger_index--; + } + } + + list_del(&effect->list); + kfree(effect); +err_mutex: + mutex_unlock(&cs40l50->lock); + + return error; +} + +static int cs40l50_mailbox_read_next(struct cs40l50_private *cs40l50, u32 *val) +{ + u32 wt_ptr, rd_ptr; + int error = 0; + + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_MBOX_QUEUE_WT, &wt_ptr); + if (error) + return error; + + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_MBOX_QUEUE_RD, &rd_ptr); + if (error) + return error; + + if (wt_ptr == rd_ptr) { + *val = 0; + return 0; + } + + error = cs40l50_dsp_read(cs40l50, rd_ptr, val); + if (error) + return error; + + rd_ptr += sizeof(u32); + if (rd_ptr > CS40L50_MBOX_QUEUE_END) + rd_ptr = CS40L50_MBOX_QUEUE_BASE; + + return cs40l50_dsp_write(cs40l50, CS40L50_DSP1_MBOX_QUEUE_RD, rd_ptr); +} + +static int cs40l50_handle_f0_est_done(struct cs40l50_private *cs40l50) +{ + int error; + u32 f_zero; + + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_F0_ESTIMATION, &f_zero); + if (error) + return error; + + return cs40l50_dsp_write(cs40l50, CS40L50_DSP1_F0_STORED, f_zero); +} + +static int cs40l50_handle_redc_est_done(struct cs40l50_private *cs40l50) +{ + int integer, fractional, error; + u32 redc; + + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_RE_EST_STATUS, &redc); + if (error) + return error; + + error = cs40l50_dsp_write(cs40l50, CS40L50_DSP1_REDC_ESTIMATION, redc); + if (error) + return error; + + /* Convert from Q8.15 to (Q7.17 * 29/240) */ + integer = (redc >> 15) & 0xFF; + fractional = (redc & 0x7FFF) * 4; + + return cs40l50_dsp_write(cs40l50, CS40L50_DSP1_REDC_STORED, + (((integer << 17) | fractional) * 29 / 240) & GENMASK(23, 0)); +} + +static int cs40l50_error_release(struct cs40l50_private *cs40l50) +{ + int error; + + error = cs40l50_dsp_write(cs40l50, CS40L50_ERROR_RELEASE, CS40L50_ERROR_RELEASE_MASK); + if (error) + return error; + + return cs40l50_dsp_write(cs40l50, CS40L50_ERROR_RELEASE, 0); +} + +static irqreturn_t cs40l50_handle_mbox_buffer(int irq, void *data) +{ + struct cs40l50_private *cs40l50 = data; + int error = 0; + u32 val; + + mutex_lock(&cs40l50->lock); + + while (!cs40l50_mailbox_read_next(cs40l50, &val)) { + switch (val) { + case 0: + goto out_irq; + case CS40L50_DSP_MBOX_HAPTIC_TRIGGER_GPIO: + dev_dbg(cs40l50->dev, "Mailbox: TRIGGER_GPIO\n"); + break; + case CS40L50_DSP_MBOX_HAPTIC_TRIGGER_MBOX: + dev_dbg(cs40l50->dev, "Mailbox: TRIGGER_MBOX\n"); + break; + case CS40L50_DSP_MBOX_HAPTIC_TRIGGER_I2S: + dev_dbg(cs40l50->dev, "Mailbox: TRIGGER_I2S\n"); + break; + case CS40L50_DSP_MBOX_HAPTIC_COMPLETE_MBOX: + dev_dbg(cs40l50->dev, "Mailbox: COMPLETE_MBOX\n"); + break; + case CS40L50_DSP_MBOX_HAPTIC_COMPLETE_GPIO: + dev_dbg(cs40l50->dev, "Mailbox: COMPLETE_GPIO\n"); + break; + case CS40L50_DSP_MBOX_HAPTIC_COMPLETE_I2S: + dev_dbg(cs40l50->dev, "Mailbox: COMPLETE_I2S\n"); + break; + case CS40L50_DSP_MBOX_F0_EST_START: + dev_dbg(cs40l50->dev, "Mailbox: F0_EST_START\n"); + break; + case CS40L50_DSP_MBOX_F0_EST_DONE: + dev_dbg(cs40l50->dev, "Mailbox: F0_EST_DONE\n"); + error = cs40l50_handle_f0_est_done(cs40l50); + if (error) + goto out_irq; + break; + case CS40L50_DSP_MBOX_REDC_EST_START: + dev_dbg(cs40l50->dev, "Mailbox: REDC_EST_START\n"); + break; + case CS40L50_DSP_MBOX_REDC_EST_DONE: + dev_dbg(cs40l50->dev, "Mailbox: REDC_EST_DONE\n"); + error = cs40l50_handle_redc_est_done(cs40l50); + if (error) + goto out_irq; + break; + case CS40L50_DSP_MBOX_LE_EST_START: + dev_dbg(cs40l50->dev, "Mailbox: LE_EST_START\n"); + break; + case CS40L50_DSP_MBOX_LE_EST_DONE: + dev_dbg(cs40l50->dev, "Mailbox: LE_EST_DONE\n"); + break; + case CS40L50_DSP_MBOX_AWAKE: + dev_dbg(cs40l50->dev, "Mailbox: AWAKE\n"); + break; + case CS40L50_DSP_MBOX_INIT: + dev_dbg(cs40l50->dev, "Mailbox: INIT\n"); + break; + case CS40L50_DSP_MBOX_ACK: + dev_dbg(cs40l50->dev, "Mailbox: ACK\n"); + break; + case CS40L50_DSP_MBOX_ERR_EVENT_UNMAPPED: + dev_err(cs40l50->dev, "Unmapped event\n"); + break; + case CS40L50_DSP_MBOX_ERR_EVENT_MODIFY: + dev_err(cs40l50->dev, "Attempt to modify event index failed\n"); + break; + case CS40L50_DSP_MBOX_ERR_NULLPTR: + dev_err(cs40l50->dev, "Null pointer\n"); + break; + case CS40L50_DSP_MBOX_ERR_BRAKING: + dev_err(cs40l50->dev, "Braking not in progress\n"); + break; + case CS40L50_DSP_MBOX_ERR_INVAL_SRC: + dev_err(cs40l50->dev, "Suspend/resume invalid source\n"); + break; + case CS40L50_DSP_MBOX_ERR_ENABLE_RANGE: + dev_err(cs40l50->dev, "GPIO enable out of range\n"); + break; + case CS40L50_DSP_MBOX_ERR_GPIO_UNMAPPED: + dev_err(cs40l50->dev, "GPIO not mapped\n"); + break; + case CS40L50_DSP_MBOX_ERR_ISR_RANGE: + dev_err(cs40l50->dev, "GPIO ISR out of range\n"); + break; + case CS40L50_DSP_MBOX_PERMANENT_SHORT: + dev_crit(cs40l50->dev, "Permanent short detected\n"); + break; + case CS40L50_DSP_MBOX_RUNTIME_SHORT: + dev_err(cs40l50->dev, "Runtime short detected\n"); + error = cs40l50_error_release(cs40l50); + if (error) + goto out_irq; + break; + default: + dev_err(cs40l50->dev, "Mailbox value %#X not recognized\n", val); + error = -EINVAL; + goto out_irq; + } + } + + error = -EIO; + +out_irq: + mutex_unlock(&cs40l50->lock); + + return IRQ_RETVAL(!error); +} + + +static irqreturn_t cs40l50_vddb_uv(int irq, void *data) +{ + struct cs40l50_private *cs40l50 = data; + + dev_err(cs40l50->dev, "VDD_B undervoltage error\n"); + + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); +} + +static irqreturn_t cs40l50_boost_current_limit(int irq, void *data) +{ + struct cs40l50_private *cs40l50 = data; + + dev_err(cs40l50->dev, "Boost current limit engagement\n"); + + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); +} + +static irqreturn_t cs40l50_boost_short(int irq, void *data) +{ + struct cs40l50_private *cs40l50 = data; + + dev_err(cs40l50->dev, "Boost inductor short error\n"); + + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); +} + +static irqreturn_t cs40l50_boost_uv(int irq, void *data) +{ + struct cs40l50_private *cs40l50 = data; + + dev_err(cs40l50->dev, "Boost undervoltage error\n"); + + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); +} + +static irqreturn_t cs40l50_overtemperature(int irq, void *data) +{ + struct cs40l50_private *cs40l50 = data; + + dev_err(cs40l50->dev, "Overtemperature error\n"); + + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); +} + +static irqreturn_t cs40l50_amp_short(int irq, void *data) +{ + struct cs40l50_private *cs40l50 = data; + + dev_err(cs40l50->dev, "Amp short error\n"); + + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); +} + +static irqreturn_t cs40l50_global_error(int irq, void *data) +{ + struct cs40l50_private *cs40l50 = data; + + dev_err(cs40l50->dev, "MSM chip error\n"); + + return IRQ_RETVAL(!cs40l50_error_release(cs40l50)); +} + +static const struct cs40l50_irq cs40l50_irqs[] = { + CS40L50_IRQ(CS40L50_AMP_SHORT_IRQ, "Amp short", cs40l50_amp_short), + CS40L50_IRQ(CS40L50_VIRT2_MBOX_IRQ, "Firmware interrupt", cs40l50_handle_mbox_buffer), + CS40L50_IRQ(CS40L50_TEMP_ERR_IRQ, "Overtemperature", cs40l50_overtemperature), + CS40L50_IRQ(CS40L50_BST_UVP_IRQ, "Boost undervoltage", cs40l50_boost_uv), + CS40L50_IRQ(CS40L50_BST_SHORT_IRQ, "Boost short", cs40l50_boost_short), + CS40L50_IRQ(CS40L50_BST_ILIMIT_IRQ, "Boost current limit", cs40l50_boost_current_limit), + CS40L50_IRQ(CS40L50_UVLO_VDDBATT_IRQ, "VDD_B undervoltage", cs40l50_vddb_uv), + CS40L50_IRQ(CS40L50_GLOBAL_ERROR_IRQ, "Global error", cs40l50_global_error), +}; + +static const struct regmap_irq cs40l50_reg_irqs[] = { + CS40L50_REG_IRQ(IRQ1_INT_1, AMP_SHORT), + CS40L50_REG_IRQ(IRQ1_INT_2, VIRT2_MBOX), + CS40L50_REG_IRQ(IRQ1_INT_8, TEMP_ERR), + CS40L50_REG_IRQ(IRQ1_INT_9, BST_UVP), + CS40L50_REG_IRQ(IRQ1_INT_9, BST_SHORT), + CS40L50_REG_IRQ(IRQ1_INT_9, BST_ILIMIT), + CS40L50_REG_IRQ(IRQ1_INT_10, UVLO_VDDBATT), + CS40L50_REG_IRQ(IRQ1_INT_18, GLOBAL_ERROR), +}; + +static const struct regmap_irq_chip cs40l50_regmap_irq_chip = { + .name = "cs40l50 IRQ1 Controller", + .status_base = CS40L50_IRQ1_INT_1, + .mask_base = CS40L50_IRQ1_MASK_1, + .ack_base = CS40L50_IRQ1_INT_1, + .num_regs = 19, + .irqs = cs40l50_reg_irqs, + .num_irqs = ARRAY_SIZE(cs40l50_reg_irqs), + .runtime_pm = true, +}; + +static const struct reg_sequence cs40l50_ext_bst_seq[] = { + { CS40L50_BLOCK_ENABLES, CS40L50_MON_ENABLE }, + { CS40L50_MON_VALUE_CTRL, CS40L50_VBST_VDD_AMP }, + { CS40L50_TEST_KEY, CS40L50_KEY_SET_1 }, + { CS40L50_TEST_KEY, CS40L50_KEY_SET_2 }, + { CS40L50_TEST_VIS_SPARE, CS40L50_TEMP_SENSOR_3 }, + { CS40L50_MON_ERROR_OVERIDE, CS40L50_TEMP_ERR_THLD }, + { CS40L50_STATUS, CS40L50_TEMP_WARN_THLD }, + { CS40L50_TEST_CONFIG, CS40L50_TEMP_FILT_DEBOUNCE_OFF }, + { CS40L50_TEMP_WARN_CONFIG, CS40L50_TEMP_WARN_ATK_VOL }, +}; + +static int cs40l50_config_bst(struct cs40l50_private *cs40l50) +{ + int error, i; + + if (device_property_present(cs40l50->dev, "cirrus,external-boost")) { + error = regmap_multi_reg_write(cs40l50->regmap, cs40l50_ext_bst_seq, + ARRAY_SIZE(cs40l50_ext_bst_seq)); + if (error) + return error; + + for (i = 0; i < ARRAY_SIZE(cs40l50_ext_bst_seq); i++) { + error = cs40l50_pseq_write(cs40l50, cs40l50_ext_bst_seq[i].reg, + cs40l50_ext_bst_seq[i].def); + if (error) + return error; + } + + return 0; + } + + /* Internal boost config */ + return cs40l50_dsp_write(cs40l50, CS40L50_BST_LPMODE_SEL, CS40L50_DCM_LOW_POWER); +} + +static int cs40l50_patch_firmware(struct cs40l50_private *cs40l50) +{ + const struct firmware *bin = NULL, *wmfw = NULL; + int error = 0; + + if (request_firmware(&bin, "cs40l50.bin", cs40l50->dev)) + dev_info(cs40l50->dev, "Could not request wavetable file: %d\n", error); + + if (request_firmware(&wmfw, "cs40l50.wmfw", cs40l50->dev)) + dev_info(cs40l50->dev, "Could not request firmware patch file: %d\n", error); + + if (wmfw) { + error = cs40l50_dsp_write(cs40l50, CS40L50_CCM_CORE_CONTROL, + CS40L50_DSP1_CLOCK_DISABLE); + if (error) + goto err_fw; + + error = cs40l50_dsp_write(cs40l50, CS40L50_RAM_INIT, + CS40L50_DSP1_RAM_INIT_FLAG); + if (error) + goto err_fw; + + error = cs40l50_dsp_write(cs40l50, CS40L50_PWRMGT_CTL, + CS40L50_DSP1_MEM_RDY_HW); + if (error) + goto err_fw; + } + + mutex_lock(&cs40l50->lock); + + cs_dsp_power_up(&cs40l50->dsp, wmfw, "cs40l50.wmfw", bin, "cs40l50.bin", "cs40l50"); + + mutex_unlock(&cs40l50->lock); + + if (wmfw) + error = cs40l50_dsp_write(cs40l50, CS40L50_CCM_CORE_CONTROL, + CS40L50_DSP1_CLOCK_ENABLE); + +err_fw: + release_firmware(wmfw); + release_firmware(bin); + + return error; +} + +static int cs40l50_pseq_init(struct cs40l50_private *cs40l50) +{ + struct cs40l50_pseq_op *pseq_op; + int error, i, num_words; + u8 operation; + u32 *words; + + INIT_LIST_HEAD(&cs40l50->pseq_op_head); + + words = kcalloc(CS40L50_PSEQ_MAX_WORDS, sizeof(u32), GFP_KERNEL); + if (IS_ERR_OR_NULL(words)) + return -ENOMEM; + + error = regmap_bulk_read(cs40l50->regmap, CS40L50_DSP1_POWER_ON_SEQ, words, + CS40L50_PSEQ_MAX_WORDS); + if (error) + goto err_free; + + for (i = 0; i < CS40L50_PSEQ_MAX_WORDS; i++) + words[i] = be32_to_cpu(words[i]); + + for (i = 0; i < CS40L50_PSEQ_MAX_WORDS; i += num_words) { + operation = (words[i] & CS40L50_PSEQ_OP_MASK) >> CS40L50_PSEQ_OP_SHIFT; + + switch (operation) { + case CS40L50_PSEQ_OP_END: + num_words = CS40L50_PSEQ_OP_END_WORDS; + break; + case CS40L50_PSEQ_OP_WRITE_ADDR8: + case CS40L50_PSEQ_OP_WRITE_H16: + case CS40L50_PSEQ_OP_WRITE_L16: + num_words = CS40L50_PSEQ_OP_WRITE_X16_WORDS; + break; + case CS40L50_PSEQ_OP_WRITE_FULL: + num_words = CS40L50_PSEQ_OP_WRITE_FULL_WORDS; + break; + default: + dev_err(cs40l50->dev, "Invalid OP code 0x%02X\n", operation); + error = -EINVAL; + goto err_free; + } + + pseq_op = devm_kzalloc(cs40l50->dev, sizeof(*pseq_op), GFP_KERNEL); + if (!pseq_op) { + error = -ENOMEM; + goto err_free; + } + + memcpy(pseq_op->words, &words[i], num_words * sizeof(u32)); + pseq_op->size = num_words; + pseq_op->offset = i * sizeof(u32); + pseq_op->operation = operation; + list_add(&pseq_op->list, &cs40l50->pseq_op_head); + + if (operation == CS40L50_PSEQ_OP_END) + break; + } + + if (operation != CS40L50_PSEQ_OP_END) { + dev_err(cs40l50->dev, "PSEQ_END_OF_SCRIPT not found\n"); + error = -ENOENT; + } + +err_free: + kfree(words); + + return error; +} + +static int cs40l50_input_init(struct cs40l50_private *cs40l50) +{ + int error; + u32 val; + + error = cs40l50_dsp_read(cs40l50, CS40L50_DEVID, &val); + if (error) + return error; + + if (val != CS40L50_DEVID_A) { + dev_err(cs40l50->dev, "Invalid device ID: %#010X\n", val); + return -EINVAL; + } + + cs40l50->input->id.product = val & 0xFFFF; + + error = cs40l50_dsp_read(cs40l50, CS40L50_REVID, &val); + if (error) + return error; + + if (val != CS40L50_REVID_B0) { + dev_err(cs40l50->dev, "Invalid device revision: %#04X\n", val); + return -EINVAL; + } + + cs40l50->input->id.version = val; + + cs40l50->input = devm_input_allocate_device(cs40l50->dev); + if (IS_ERR_OR_NULL(cs40l50->input)) + return cs40l50->input ? PTR_ERR(cs40l50->input) : -ENOMEM; + + cs40l50->input->name = "cs40l50_input"; + input_set_drvdata(cs40l50->input, cs40l50); + input_set_capability(cs40l50->input, EV_FF, FF_PERIODIC); + input_set_capability(cs40l50->input, EV_FF, FF_CUSTOM); + + error = input_ff_create(cs40l50->input, FF_MAX_EFFECTS); + if (error) + return error; + + clear_bit(FF_RUMBLE, cs40l50->input->ffbit); + + cs40l50->input->ff->upload = cs40l50_add_effect; + cs40l50->input->ff->playback = cs40l50_playback_effect; + cs40l50->input->ff->erase = cs40l50_erase_effect; + + INIT_LIST_HEAD(&cs40l50->effect_head); + + return input_register_device(cs40l50->input); +} + +static const struct cs_dsp_client_ops cs40l50_cs_dsp_client_ops; + +static const struct cs_dsp_region cs40l50_dsp_regions[] = { + { .type = WMFW_HALO_PM_PACKED, .base = CS40L50_DSP1_PMEM_0 }, + { .type = WMFW_HALO_XM_PACKED, .base = CS40L50_DSP1_XMEM_PACKED_0 }, + { .type = WMFW_HALO_YM_PACKED, .base = CS40L50_DSP1_YMEM_PACKED_0 }, + { .type = WMFW_ADSP2_XM, .base = CS40L50_DSP1_XMEM_UNPACKED24_0 }, + { .type = WMFW_ADSP2_YM, .base = CS40L50_DSP1_YMEM_UNPACKED24_0 }, +}; + +static int cs40l50_cs_dsp_init(struct cs40l50_private *cs40l50) +{ + cs40l50->dsp.num = 1; + cs40l50->dsp.type = WMFW_HALO; + cs40l50->dsp.dev = cs40l50->dev; + cs40l50->dsp.regmap = cs40l50->regmap; + cs40l50->dsp.base = CS40L50_DSP1_CORE_BASE; + cs40l50->dsp.base_sysinfo = CS40L50_DSP1_SYS_INFO_ID; + cs40l50->dsp.mem = cs40l50_dsp_regions; + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions); + cs40l50->dsp.lock_regions = 0xFFFFFFFF; + cs40l50->dsp.no_core_startstop = true; + cs40l50->dsp.client_ops = &cs40l50_cs_dsp_client_ops; + + return cs_dsp_halo_init(&cs40l50->dsp); +} + +int cs40l50_probe(struct cs40l50_private *cs40l50) +{ + int error, i, irq; + u32 val; + + mutex_init(&cs40l50->lock); + + error = devm_regulator_bulk_get(cs40l50->dev, ARRAY_SIZE(cs40l50_supplies), + cs40l50_supplies); + if (error) + return dev_err_probe(cs40l50->dev, error, "Failed to request supplies\n"); + + error = regulator_bulk_enable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies); + if (error) + return dev_err_probe(cs40l50->dev, error, "Failed to enable supplies\n"); + + cs40l50->reset_gpio = devm_gpiod_get_optional(cs40l50->dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(cs40l50->reset_gpio)) { + error = PTR_ERR(cs40l50->reset_gpio); + goto err; + } + + usleep_range(CS40L50_MIN_RESET_PULSE_WIDTH_US, CS40L50_MIN_RESET_PULSE_WIDTH_US + 100); + + gpiod_set_value_cansleep(cs40l50->reset_gpio, 0); + + usleep_range(CS40L50_CP_READY_DELAY_US, CS40L50_CP_READY_DELAY_US + 1000); + + for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) { + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_HALO_STATE, &val); + if (!error && val < 0xFFFF && val >= CS40L50_HALO_STATE_BOOT_DONE) + break; + + usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100); + } + + if (i == CS40L50_DSP_TIMEOUT_COUNT) { + dev_err(cs40l50->dev, "Firmware boot failed: %d, halo state = %#x\n", error, val); + goto err; + } + + cs40l50->vibe_workqueue = alloc_ordered_workqueue("cs40l50_workqueue", WQ_HIGHPRI); + if (!cs40l50->vibe_workqueue) { + error = -ENOMEM; + goto err; + } + + INIT_WORK(&cs40l50->vibe_start_work, cs40l50_vibe_start_worker); + INIT_WORK(&cs40l50->vibe_stop_work, cs40l50_vibe_stop_worker); + + error = cs40l50_cs_dsp_init(cs40l50); + if (error) + goto err; + + error = cs40l50_input_init(cs40l50); + if (error) + goto err; + + error = cs40l50_patch_firmware(cs40l50); + if (error) + goto err; + + error = cs40l50_pseq_init(cs40l50); + if (error) + goto err; + + error = cs40l50_config_bst(cs40l50); + if (error) + goto err; + + error = devm_regmap_add_irq_chip(cs40l50->dev, cs40l50->regmap, cs40l50->irq, + IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW, 0, &cs40l50_regmap_irq_chip, + &cs40l50->irq_data); + if (error) { + dev_err(cs40l50->dev, "Failed to register IRQ chip: %d\n", error); + goto err; + } + + for (i = 0; i < ARRAY_SIZE(cs40l50_irqs); i++) { + irq = regmap_irq_get_virq(cs40l50->irq_data, cs40l50_irqs[i].irq); + if (irq < 0) { + dev_err(cs40l50->dev, "Failed to get %s\n", cs40l50_irqs[i].name); + error = irq; + goto err; + } + + error = devm_request_threaded_irq(cs40l50->dev, irq, NULL, cs40l50_irqs[i].handler, + IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW, + cs40l50_irqs[i].name, cs40l50); + if (error) { + dev_err(cs40l50->dev, "Failed to request IRQ %s: %d\n", + cs40l50_irqs[i].name, error); + goto err; + } + } + + return 0; + +err: + cs40l50_remove(cs40l50); + + return error; +} +EXPORT_SYMBOL_GPL(cs40l50_probe); + +int cs40l50_remove(struct cs40l50_private *cs40l50) +{ + disable_irq(cs40l50->irq); + + if (cs40l50->dsp.booted) + cs_dsp_power_down(&cs40l50->dsp); + if (&cs40l50->dsp) + cs_dsp_remove(&cs40l50->dsp); + + if (cs40l50->vibe_workqueue) { + flush_workqueue(cs40l50->vibe_workqueue); + destroy_workqueue(cs40l50->vibe_workqueue); + } + + gpiod_set_value_cansleep(cs40l50->reset_gpio, 1); + regulator_bulk_disable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies); + + return 0; +} +EXPORT_SYMBOL_GPL(cs40l50_remove); + +MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver"); +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>"); +MODULE_LICENSE("GPL"); diff --git a/include/linux/input/cs40l50.h b/include/linux/input/cs40l50.h new file mode 100644 index 000000000000..f2c8a101fa00 --- /dev/null +++ b/include/linux/input/cs40l50.h @@ -0,0 +1,321 @@ +/* SPDX-License-Identifier: GPL-2.0 + * + * CS40L50 Advanced Haptic Driver with Waveform Memory DSP + * and Closed-Loop Algorithms + * + * Copyright 2023 Cirrus Logic, Inc. + * + */ +#ifndef __CS40L50_H__ +#define __CS40L50_H__ + +#include <linux/bitops.h> +#include <linux/gpio/consumer.h> +#include <linux/input.h> +#include <linux/regmap.h> +#include <linux/interrupt.h> +#include <linux/firmware/cirrus/cs_dsp.h> +#include <linux/firmware/cirrus/wmfw.h> + +/* Boost */ +#define CS40L50_TEST_KEY 0x40 +#define CS40L50_BLOCK_ENABLES 0x2018 +#define CS40L50_ERROR_RELEASE 0x2034 +#define CS40L50_PWRMGT_CTL 0x2900 +#define CS40L50_BST_LPMODE_SEL 0x3810 +#define CS40L50_STATUS 0x4200 +#define CS40L50_MON_ERROR_OVERIDE 0x4220 +#define CS40L50_TEST_CONFIG 0x4240 +#define CS40L50_MON_VALUE_CTRL 0x4404 +#define CS40L50_TEST_VIS_SPARE 0x5C00 +#define CS40L50_TEMP_WARN_CONFIG 0x6024 +#define CS40L50_KEY_SET_1 0x55 +#define CS40L50_KEY_SET_2 0xAA +#define CS40L50_TEMP_SENSOR_3 0x400 +#define CS40L50_DCM_LOW_POWER 0x1 +#define CS40L50_MON_ENABLE 0x3201 +#define CS40L50_VBST_VDD_AMP 0x1000000 +#define CS40L50_TEMP_ERR_THLD 0x8000007D +#define CS40L50_TEMP_WARN_THLD 0x8 +#define CS40L50_TEMP_FILT_DEBOUNCE_OFF 0x510002B5 +#define CS40L50_TEMP_WARN_ATK_VOL 0x522303 + +/* Interrupts */ +#define CS40L50_IRQ1_INT_1 0xE010 +#define CS40L50_IRQ1_INT_2 0xE014 +#define CS40L50_IRQ1_INT_3 0xE018 +#define CS40L50_IRQ1_INT_4 0xE01C +#define CS40L50_IRQ1_INT_8 0xE02C +#define CS40L50_IRQ1_INT_9 0xE030 +#define CS40L50_IRQ1_INT_10 0xE034 +#define CS40L50_IRQ1_INT_18 0xE054 +#define CS40L50_IRQ1_INT_19 0xE058 +#define CS40L50_IRQ1_MASK_1 0xE090 +#define CS40L50_IRQ1_MASK_2 0xE094 +#define CS40L50_IRQ1_MASK_3 0xE098 +#define CS40L50_IRQ1_MASK_8 0xE0AC +#define CS40L50_IRQ1_MASK_9 0xE0B0 +#define CS40L50_IRQ1_MASK_10 0xE0B4 +#define CS40L50_IRQ1_MASK_18 0xE0D4 +#define CS40L50_IRQ1_MASK_19 0xE0D8 +#define CS40L50_IRQ1_STS_1 0xE210 +#define CS40L50_IRQ1_STS_2 0xE214 +#define CS40L50_IRQ1_STS_3 0xE218 +#define CS40L50_IRQ1_STS_8 0xE22C +#define CS40L50_IRQ1_STS_9 0xE230 +#define CS40L50_IRQ1_STS_10 0xE234 +#define CS40L50_IRQ1_STS_18 0xE254 +#define CS40L50_IRQ1_STS_19 0xE258 +#define CS40L50_AMP_SHORT_MASK BIT(31) +#define CS40L50_VIRT2_MBOX_MASK BIT(21) +#define CS40L50_TEMP_ERR_MASK BIT(31) +#define CS40L50_BST_UVP_MASK BIT(6) +#define CS40L50_BST_SHORT_MASK BIT(7) +#define CS40L50_BST_ILIMIT_MASK BIT(8) +#define CS40L50_UVLO_VDDBATT_MASK BIT(16) +#define CS40L50_GLOBAL_ERROR_MASK BIT(15) +#define CS40L50_ERROR_RELEASE_MASK BIT(11) +#define CS40L50_BOOT_DONE_MASK BIT(1) +#define CS40L50_IRQ(_irq, _name, _hand) \ + { \ + .irq = _irq, \ + .name = _name, \ + .handler = _hand, \ + } +#define CS40L50_REG_IRQ(_reg, _irq) \ + [CS40L50_ ## _irq ## _IRQ] = { \ + .reg_offset = (CS40L50_ ## _reg) - CS40L50_IRQ1_INT_1, \ + .mask = CS40L50_ ## _irq ## _MASK \ + } + +/* Mailbox Inbound Commands */ +#define CS40L50_RAM_BANK_INDEX_START 0x1000000 +#define CS40L50_RAM_BANK_INDEX_END 0x100007F +#define CS40L50_OWT_INDEX_START 0x1400000 +#define CS40L50_OWT_INDEX_END 0x140007F +#define CS40L50_ROM_BANK_INDEX_START 0x1800000 +#define CS40L50_ROM_BANK_INDEX_END 0x180001A +#define CS40L50_OTP_BUZZ 0x1800080 +#define CS40L50_RAM_BUZZ_INDEX_START 0x1800081 +#define CS40L50_RAM_BUZZ_INDEX_END 0x1800085 +#define CS40L50_MBOX_CMD_HIBERNATE 0x2000001 +#define CS40L50_MBOX_CMD_WAKEUP 0x2000002 +#define CS40L50_MBOX_CMD_PREVENT_HIBERNATE 0x2000003 +#define CS40L50_MBOX_CMD_ALLOW_HIBERNATION 0x2000004 +#define CS40L50_MBOX_CMD_SHUTDOWN 0x2000005 +#define CS40L50_MBOX_CMD_DURATION_REPORT 0x3000001 +#define CS40L50_MBOX_CMD_START_I2S 0x3000002 +#define CS40L50_MBOX_CMD_STOP_I2S 0x3000003 +#define CS40L50_MBOX_CMD_HAPTICS_LOGGER_RESET 0x3000004 +#define CS40L50_MBOX_CMD_OWT_PUSH 0x3000008 +#define CS40L50_MBOX_CMD_OWT_RESET 0x3000009 +#define CS40L50_MBOX_CMD_DVL_REINIT 0x300000A +#define CS40L50_MBOX_CMD_STOP_PLAYBACK 0x5000000 +#define CS40L50_MBOX_CMD_F0_EST 0x7000001 +#define CS40L50_MBOX_CMD_REDC_EST 0x7000002 +#define CS40L50_MBOX_CMD_LE_EST 0x7000004 +#define CS40L50_MBOX_CMD_OWT_DELETE 0xD000000 + +/* Mailbox Outbound Commands */ +#define CS40L50_MBOX_QUEUE_BASE 0x11004 +#define CS40L50_MBOX_QUEUE_END 0x1101C +#define CS40L50_DSP_VIRTUAL1_MBOX_1 0x11020 +#define CS40L50_DSP1_MBOX_QUEUE_WT 0x28042C8 +#define CS40L50_DSP1_MBOX_QUEUE_RD 0x28042CC +#define CS40L50_DSP_MBOX_HAPTIC_COMPLETE_MBOX 0x1000000 +#define CS40L50_DSP_MBOX_HAPTIC_COMPLETE_GPIO 0x1000001 +#define CS40L50_DSP_MBOX_HAPTIC_COMPLETE_I2S 0x1000002 +#define CS40L50_DSP_MBOX_HAPTIC_TRIGGER_MBOX 0x1000010 +#define CS40L50_DSP_MBOX_HAPTIC_TRIGGER_GPIO 0x1000011 +#define CS40L50_DSP_MBOX_HAPTIC_TRIGGER_I2S 0x1000012 +#define CS40L50_DSP_MBOX_INIT 0x2000000 +#define CS40L50_DSP_MBOX_AWAKE 0x2000002 +#define CS40L50_DSP_MBOX_F0_EST_START 0x7000011 +#define CS40L50_DSP_MBOX_F0_EST_DONE 0x7000021 +#define CS40L50_DSP_MBOX_REDC_EST_START 0x7000012 +#define CS40L50_DSP_MBOX_REDC_EST_DONE 0x7000022 +#define CS40L50_DSP_MBOX_LE_EST_START 0x7000014 +#define CS40L50_DSP_MBOX_LE_EST_DONE 0x7000024 +#define CS40L50_DSP_MBOX_ACK 0xA000000 +#define CS40L50_DSP_MBOX_PANIC 0xC000000 +#define CS40L50_DSP_MBOX_WATERMARK 0xD000000 +#define CS40L50_DSP_MBOX_ERR_EVENT_UNMAPPED 0xC0004B3 +#define CS40L50_DSP_MBOX_ERR_EVENT_MODIFY 0xC0004B4 +#define CS40L50_DSP_MBOX_ERR_NULLPTR 0xC0006A5 +#define CS40L50_DSP_MBOX_ERR_BRAKING 0xC0006A8 +#define CS40L50_DSP_MBOX_ERR_INVAL_SRC 0xC0006AC +#define CS40L50_DSP_MBOX_ERR_ENABLE_RANGE 0xC000836 +#define CS40L50_DSP_MBOX_ERR_GPIO_UNMAPPED 0xC000837 +#define CS40L50_DSP_MBOX_ERR_ISR_RANGE 0xC000838 +#define CS40L50_DSP_MBOX_PERMANENT_SHORT 0xC000C1C +#define CS40L50_DSP_MBOX_RUNTIME_SHORT 0xC000C1D + +/* DSP */ +#define CS40L50_DSP1_XMEM_PACKED_0 0x2000000 +#define CS40L50_DSP1_XMEM_UNPACKED32_0 0x2400000 +#define CS40L50_DSP1_SYS_INFO_ID 0x25E0000 +#define CS40L50_DSP1_XMEM_UNPACKED24_0 0x2800000 +#define CS40L50_NUM_OF_WAVES 0x280CB4C +#define CS40L50_DSP1_CORE_BASE 0x2B80000 +#define CS40L50_DSP1_SCRATCH1 0x2B805C0 +#define CS40L50_DSP1_SCRATCH2 0x2B805C8 +#define CS40L50_DSP1_SCRATCH3 0x2B805D0 +#define CS40L50_DSP1_SCRATCH4 0x2B805D8 +#define CS40L50_CCM_CORE_CONTROL 0x2BC1000 +#define CS40L50_RAM_INIT 0x28021DC +#define CS40L50_DSP1_YMEM_PACKED_0 0x2C00000 +#define CS40L50_DSP1_YMEM_UNPACKED32_0 0x3000000 +#define CS40L50_DSP1_YMEM_UNPACKED24_0 0x3400000 +#define CS40L50_DSP1_PMEM_0 0x3800000 +#define CS40L50_DSP1_PMEM_5114 0x3804FE8 +#define CS40L50_DSP1_MEM_RDY_HW 0x2 +#define CS40L50_DSP1_RAM_INIT_FLAG 0x1 +#define CS40L50_DSP1_CLOCK_DISABLE 0x80 +#define CS40L50_DSP1_CLOCK_ENABLE 0x281 +#define CS40L50_DSP_POLL_US 1000 +#define CS40L50_DSP_TIMEOUT_COUNT 100 +#define CS40L50_MBOX_POLL_US 5000 + +/* Calibration */ +#define CS40L50_DSP1_REDC_ESTIMATION 0x2802F7C +#define CS40L50_DSP1_F0_ESTIMATION 0x2802F84 +#define CS40L50_DSP1_DYNAMIC_F0_ENABLE 0x2802F8C +#define CS40L50_DSP1_DYNAMIC_F0_THRESHOLD 0x2802F90 +#define CS40L50_DSP1_F0_STORED 0x2805C00 +#define CS40L50_DSP1_REDC_STORED 0x2805C04 +#define CS40L50_DSP1_RE_EST_STATUS 0x3401B40 +#define CS40L50_REDC_EST_DELAY_US 30000 +#define CS40L50_F0_EST_DELAY_US 10000 + +/* OWT */ +#define CS40L50_DSP1_OWT_SIZE 0x2805C38 +#define CS40L50_DSP1_OWT_NEXT 0x2805C3C +#define CS40L50_NUM_OF_OWT_WAVES 0x2805C40 +#define CS40L50_DSP1_OWT_BASE 0x2805C34 +#define CS40L50_MAX_PWLE_SECTIONS 126 +#define CS40L50_CUSTOM_DATA_SIZE 2 +#define CS40L50_WT_HEADER_PWLE_SIZE 12 +#define CS40L50_WT_HEADER_DEFAULT_FLAGS 0x0000 +#define CS40L50_WT_HEADER_OFFSET 3 +#define CS40L50_WT_TYPE_PCM 8 +#define CS40L50_WT_TYPE_PWLE 12 +#define CS40L50_PCM_ID 0x0000 + +/* GPIO */ +#define CS40L50_DSP1_GPIO_BASE 0x2804140 +#define CS40L50_GPIO_MAPPING_INVALID 0 +#define CS40L50_GPI_DISABLE 0x1FF +#define CS40L50_BTN_NUM_MASK GENMASK(14, 12) +#define CS40L50_BTN_EDGE_MASK BIT(15) + +/* State */ +#define CS40L50_DSP1_HALO_STATE 0x28021E0 +#define CS40L50_HALO_STATE_BOOT_DONE 2 +#define CS40L50_MIN_RESET_PULSE_WIDTH_US 1100 +#define CS40L50_CP_READY_DELAY_US 2200 + +/* Device */ +#define CS40L50_DEVID 0x0 +#define CS40L50_REVID 0x4 +#define CS40L50_DEVID_A 0x40A50 +#define CS40L50_REVID_B0 0xB0 + +/* Write sequencer */ +#define CS40L50_DSP1_POWER_ON_SEQ 0x280C320 +#define CS40L50_PSEQ_MAX_WORDS 200 +#define CS40L50_PSEQ_OP_MASK GENMASK(23, 16) +#define CS40L50_PSEQ_OP_SHIFT 16 +#define CS40L50_PSEQ_OP_WRITE_ADDR8 0x02 +#define CS40L50_PSEQ_OP_WRITE_L16 0x04 +#define CS40L50_PSEQ_OP_WRITE_H16 0x05 +#define CS40L50_PSEQ_OP_WRITE_FULL 0x00 +#define CS40L50_PSEQ_OP_WRITE_FULL_WORDS 3 +#define CS40L50_PSEQ_OP_WRITE_X16_WORDS 2 +#define CS40L50_PSEQ_OP_END 0xFF +#define CS40L50_PSEQ_OP_END_WORDS 1 +#define CS40L50_PSEQ_WRITE_FULL_LOWER_ADDR_SHIFT 8 +#define CS40L50_PSEQ_WRITE_FULL_UPPER_ADDR_SHIFT 16 +#define CS40L50_PSEQ_WRITE_FULL_LOWER_ADDR_MASK GENMASK(15, 0) +#define CS40L50_PSEQ_WRITE_FULL_UPPER_DATA_SHIFT 24 +#define CS40L50_PSEQ_WRITE_FULL_LOWER_DATA_MASK GENMASK(23, 0) + +enum cs40l50_wvfrm_bank { + CS40L50_WVFRM_BANK_RAM, + CS40L50_WVFRM_BANK_ROM, + CS40L50_WVFRM_BANK_OWT, + CS40L50_WVFRM_BANK_BUZ, + CS40L50_WVFRM_BANK_NUM, +}; + +enum cs40l50_irq_list { + CS40L50_GLOBAL_ERROR_IRQ, + CS40L50_UVLO_VDDBATT_IRQ, + CS40L50_BST_ILIMIT_IRQ, + CS40L50_BST_SHORT_IRQ, + CS40L50_BST_UVP_IRQ, + CS40L50_TEMP_ERR_IRQ, + CS40L50_VIRT2_MBOX_IRQ, + CS40L50_AMP_SHORT_IRQ, + CS40L50_NUM_IRQ +}; + +union cs40l50_buzzgen { + u32 words[3]; + struct { + u32 freq; + u32 level; + u32 duration; + } params; +}; + +struct cs40l50_owt_header { + u32 wvfrm_type; + u32 offset; + u32 data_size; +}; + +struct cs40l50_irq { + int irq; + const char *name; + irqreturn_t (*handler)(int irq, void *data); +}; + +struct cs40l50_effect { + int id; + u32 trigger_index; + enum cs40l50_wvfrm_bank wvfrm_bank; + u32 mapping; + struct list_head list; +}; + +struct cs40l50_pseq_op { + u16 offset; + u8 operation; + u32 words[3]; + u32 size; + struct list_head list; +}; + +struct cs40l50_private { + struct device *dev; + struct regmap *regmap; + struct cs_dsp dsp; + struct mutex lock; + struct gpio_desc *reset_gpio; + struct input_dev *input; + struct list_head effect_head; + struct ff_effect *trigger_effect; + struct workqueue_struct *vibe_workqueue; + struct work_struct vibe_start_work; + struct work_struct vibe_stop_work; + struct regmap_irq_chip_data *irq_data; + struct list_head pseq_op_head; + int irq; +}; + +int cs40l50_probe(struct cs40l50_private *cs40l50); +int cs40l50_remove(struct cs40l50_private *cs40l50); + +extern const struct regmap_config cs40l50_regmap; + +#endif /* __CS40L50_H__ */
Introduce support for Cirrus Logic Device CS40L50: a haptics driver with waveform memory DSP and closed-loop algorithms. Signed-off-by: James Ogletree <james.ogletree@cirrus.com> --- MAINTAINERS | 2 + drivers/input/misc/Kconfig | 33 + drivers/input/misc/Makefile | 3 + drivers/input/misc/cs40l50-i2c.c | 67 ++ drivers/input/misc/cs40l50-spi.c | 67 ++ drivers/input/misc/cs40l50.c | 1008 ++++++++++++++++++++++++++++++ include/linux/input/cs40l50.h | 321 ++++++++++ 7 files changed, 1501 insertions(+) create mode 100644 drivers/input/misc/cs40l50-i2c.c create mode 100644 drivers/input/misc/cs40l50-spi.c create mode 100644 drivers/input/misc/cs40l50.c create mode 100644 include/linux/input/cs40l50.h