linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND v10 0/5] Add support for CS40L50
@ 2024-04-08 15:32 James Ogletree
  2024-04-08 15:32 ` [PATCH RESEND v10 1/5] firmware: cs_dsp: Add write sequence interface James Ogletree
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: James Ogletree @ 2024-04-08 15:32 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
	broonie, jeff
  Cc: patches, linux-sound, linux-input, devicetree, James Ogletree

Changes in v10:
- Minor refactoring and logical improvements all around
- Renamed and added supplies

Changes in v9:
- Fixed empty struct by utilizing cs_dsp's post_run callback
- Style fixes in MFD driver

Changes in v8:
- set_sysclk() -> set_bclk_ratio()
- Added ID table to codec driver
- Style improvements
- Fixed ordering of new write sequence operations

Changes in v7:
- Fixed sparse warning
- Moved write sequences to private data structure
- Logical and style improvements in write sequence interface

Changes in v6:
- Updated write sequencer interface to be control-name based
- Fixed a race condition and non-handling of repeats in playback callback
- Stylistic and logical improvements all around

Changes in v5:
- Added a codec sub-device to support I2S streaming
- Moved write sequencer code from cirrus_haptics to cs_dsp
- Reverted cirrus_haptics library; future Cirrus input
  drivers will export and utilize cs40l50_vibra functions
- Added more comments
- Many small stylistic and logical improvements

Changes in v4:
- Moved from Input to MFD
- Moved common Cirrus haptic functions to a library
- Incorporated runtime PM framework
- Many style improvements

Changes in v3:
- YAML formatting corrections
- Fixed typo in MAINTAINERS
- Used generic node name "haptic-driver"
- Fixed probe error code paths
- Switched to "sizeof(*)"
- Removed tree reference in MAINTAINERS

Changes in v2:
- Fixed checkpatch warnings

James Ogletree (5):
  firmware: cs_dsp: Add write sequence interface
  dt-bindings: input: cirrus,cs40l50: Add initial DT binding
  mfd: cs40l50: Add support for CS40L50 core driver
  Input: cs40l50 - Add support for the CS40L50 haptic driver
  ASoC: cs40l50: Support I2S streaming to CS40L50

 .../bindings/input/cirrus,cs40l50.yaml        |  68 +++
 MAINTAINERS                                   |  12 +
 drivers/firmware/cirrus/cs_dsp.c              | 278 +++++++++
 drivers/input/misc/Kconfig                    |  10 +
 drivers/input/misc/Makefile                   |   1 +
 drivers/input/misc/cs40l50-vibra.c            | 577 ++++++++++++++++++
 drivers/mfd/Kconfig                           |  30 +
 drivers/mfd/Makefile                          |   4 +
 drivers/mfd/cs40l50-core.c                    | 570 +++++++++++++++++
 drivers/mfd/cs40l50-i2c.c                     |  68 +++
 drivers/mfd/cs40l50-spi.c                     |  68 +++
 include/linux/firmware/cirrus/cs_dsp.h        |  27 +
 include/linux/mfd/cs40l50.h                   | 137 +++++
 sound/soc/codecs/Kconfig                      |  11 +
 sound/soc/codecs/Makefile                     |   2 +
 sound/soc/codecs/cs40l50-codec.c              | 308 ++++++++++
 16 files changed, 2171 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
 create mode 100644 drivers/input/misc/cs40l50-vibra.c
 create mode 100644 drivers/mfd/cs40l50-core.c
 create mode 100644 drivers/mfd/cs40l50-i2c.c
 create mode 100644 drivers/mfd/cs40l50-spi.c
 create mode 100644 include/linux/mfd/cs40l50.h
 create mode 100644 sound/soc/codecs/cs40l50-codec.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH RESEND v10 1/5] firmware: cs_dsp: Add write sequence interface
  2024-04-08 15:32 [PATCH RESEND v10 0/5] Add support for CS40L50 James Ogletree
@ 2024-04-08 15:32 ` James Ogletree
  2024-04-08 15:32 ` [PATCH RESEND v10 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding James Ogletree
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: James Ogletree @ 2024-04-08 15:32 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
	broonie, jeff
  Cc: patches, linux-sound, linux-input, devicetree, James Ogletree,
	Charles Keepax

A write sequence is a sequence of register addresses
and values executed by some Cirrus DSPs following
certain power state transitions.

Add support for Cirrus drivers to update or add to a
write sequence present in firmware.

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
 drivers/firmware/cirrus/cs_dsp.c       | 278 +++++++++++++++++++++++++
 include/linux/firmware/cirrus/cs_dsp.h |  27 +++
 2 files changed, 305 insertions(+)

diff --git a/drivers/firmware/cirrus/cs_dsp.c b/drivers/firmware/cirrus/cs_dsp.c
index 79d4254d1f9b..6d886ffea10f 100644
--- a/drivers/firmware/cirrus/cs_dsp.c
+++ b/drivers/firmware/cirrus/cs_dsp.c
@@ -275,6 +275,12 @@
 #define HALO_MPU_VIO_ERR_SRC_MASK           0x00007fff
 #define HALO_MPU_VIO_ERR_SRC_SHIFT                   0
 
+/*
+ * Write Sequence
+ */
+#define WSEQ_OP_MAX_WORDS	3
+#define WSEQ_END_OF_SCRIPT	0xFFFFFF
+
 struct cs_dsp_ops {
 	bool (*validate_version)(struct cs_dsp *dsp, unsigned int version);
 	unsigned int (*parse_sizes)(struct cs_dsp *dsp,
@@ -3339,6 +3345,278 @@ int cs_dsp_chunk_read(struct cs_dsp_chunk *ch, int nbits)
 }
 EXPORT_SYMBOL_NS_GPL(cs_dsp_chunk_read, FW_CS_DSP);
 
+
+struct cs_dsp_wseq_op {
+	struct list_head list;
+	u32 address;
+	u32 data;
+	u16 offset;
+	u8 operation;
+};
+
+static void cs_dsp_wseq_clear(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq)
+{
+	struct cs_dsp_wseq_op *op, *op_tmp;
+
+	list_for_each_entry_safe(op, op_tmp, &wseq->ops, list) {
+		list_del(&op->list);
+		devm_kfree(dsp->dev, op);
+	}
+}
+
+static int cs_dsp_populate_wseq(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq)
+{
+	struct cs_dsp_wseq_op *op = NULL;
+	struct cs_dsp_chunk chunk;
+	u8 *words;
+	int ret;
+
+	if (!wseq->ctl) {
+		cs_dsp_err(dsp, "No control for write sequence\n");
+		return -EINVAL;
+	}
+
+	words = kzalloc(wseq->ctl->len, GFP_KERNEL);
+	if (!words)
+		return -ENOMEM;
+
+	ret = cs_dsp_coeff_read_ctrl(wseq->ctl, 0, words, wseq->ctl->len);
+	if (ret) {
+		cs_dsp_err(dsp, "Failed to read %s: %d\n", wseq->ctl->subname, ret);
+		goto err_free;
+	}
+
+	INIT_LIST_HEAD(&wseq->ops);
+
+	chunk = cs_dsp_chunk(words, wseq->ctl->len);
+
+	while (!cs_dsp_chunk_end(&chunk)) {
+		op = devm_kzalloc(dsp->dev, sizeof(*op), GFP_KERNEL);
+		if (!op) {
+			ret = -ENOMEM;
+			goto err_free;
+		}
+
+		op->offset = cs_dsp_chunk_bytes(&chunk);
+		op->operation = cs_dsp_chunk_read(&chunk, 8);
+
+		switch (op->operation) {
+		case CS_DSP_WSEQ_END:
+			op->data = WSEQ_END_OF_SCRIPT;
+			break;
+		case CS_DSP_WSEQ_UNLOCK:
+			op->data = cs_dsp_chunk_read(&chunk, 16);
+			break;
+		case CS_DSP_WSEQ_ADDR8:
+			op->address = cs_dsp_chunk_read(&chunk, 8);
+			op->data = cs_dsp_chunk_read(&chunk, 32);
+			break;
+		case CS_DSP_WSEQ_H16:
+		case CS_DSP_WSEQ_L16:
+			op->address = cs_dsp_chunk_read(&chunk, 24);
+			op->data = cs_dsp_chunk_read(&chunk, 16);
+			break;
+		case CS_DSP_WSEQ_FULL:
+			op->address = cs_dsp_chunk_read(&chunk, 32);
+			op->data = cs_dsp_chunk_read(&chunk, 32);
+			break;
+		default:
+			ret = -EINVAL;
+			cs_dsp_err(dsp, "Unsupported op: %X\n", op->operation);
+			devm_kfree(dsp->dev, op);
+			goto err_free;
+		}
+
+		list_add_tail(&op->list, &wseq->ops);
+
+		if (op->operation == CS_DSP_WSEQ_END)
+			break;
+	}
+
+	if (op && op->operation != CS_DSP_WSEQ_END) {
+		cs_dsp_err(dsp, "%s missing end terminator\n", wseq->ctl->subname);
+		ret = -ENOENT;
+	}
+
+err_free:
+	kfree(words);
+
+	return ret;
+}
+
+/**
+ * cs_dsp_wseq_init() - Initialize write sequences contained within the loaded DSP firmware
+ * @dsp: Pointer to DSP structure
+ * @wseqs: List of write sequences to initialize
+ * @num_wseqs: Number of write sequences to initialize
+ *
+ * Return: Zero for success, a negative number on error.
+ */
+int cs_dsp_wseq_init(struct cs_dsp *dsp, struct cs_dsp_wseq *wseqs, unsigned int num_wseqs)
+{
+	int i, ret;
+
+	lockdep_assert_held(&dsp->pwr_lock);
+
+	for (i = 0; i < num_wseqs; i++) {
+		ret = cs_dsp_populate_wseq(dsp, &wseqs[i]);
+		if (ret) {
+			cs_dsp_wseq_clear(dsp, &wseqs[i]);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_init, FW_CS_DSP);
+
+static struct cs_dsp_wseq_op *cs_dsp_wseq_find_op(u32 addr, u8 op_code,
+						  struct list_head *wseq_ops)
+{
+	struct cs_dsp_wseq_op *op;
+
+	list_for_each_entry(op, wseq_ops, list) {
+		if (op->operation == op_code && op->address == addr)
+			return op;
+	}
+
+	return NULL;
+}
+
+/**
+ * cs_dsp_wseq_write() - Add or update an entry in a write sequence
+ * @dsp: Pointer to a DSP structure
+ * @wseq: Write sequence to write to
+ * @addr: Address of the register to be written to
+ * @data: Data to be written
+ * @op_code: The type of operation of the new entry
+ * @update: If true, searches for the first entry in the write sequence with
+ * the same address and op_code, and replaces it. If false, creates a new entry
+ * at the tail
+ *
+ * This function formats register address and value pairs into the format
+ * required for write sequence entries, and either updates or adds the
+ * new entry into the write sequence.
+ *
+ * If update is set to true and no matching entry is found, it will add a new entry.
+ *
+ * Return: Zero for success, a negative number on error.
+ */
+int cs_dsp_wseq_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
+		      u32 addr, u32 data, u8 op_code, bool update)
+{
+	struct cs_dsp_wseq_op *op_end, *op_new = NULL;
+	u32 words[WSEQ_OP_MAX_WORDS];
+	struct cs_dsp_chunk chunk;
+	int new_op_size, ret;
+
+	if (update)
+		op_new = cs_dsp_wseq_find_op(addr, op_code, &wseq->ops);
+
+	/* If entry to update is not found, treat it as a new operation */
+	if (!op_new) {
+		op_end = cs_dsp_wseq_find_op(0, CS_DSP_WSEQ_END, &wseq->ops);
+		if (!op_end) {
+			cs_dsp_err(dsp, "Missing terminator for %s\n", wseq->ctl->subname);
+			return -EINVAL;
+		}
+
+		op_new = devm_kzalloc(dsp->dev, sizeof(*op_new), GFP_KERNEL);
+		if (!op_new)
+			return -ENOMEM;
+
+		op_new->operation = op_code;
+		op_new->address = addr;
+		op_new->offset = op_end->offset;
+		update = false;
+	}
+
+	op_new->data = data;
+
+	chunk = cs_dsp_chunk(words, sizeof(words));
+	cs_dsp_chunk_write(&chunk, 8, op_new->operation);
+
+	switch (op_code) {
+	case CS_DSP_WSEQ_FULL:
+		cs_dsp_chunk_write(&chunk, 32, op_new->address);
+		cs_dsp_chunk_write(&chunk, 32, op_new->data);
+		break;
+	case CS_DSP_WSEQ_L16:
+	case CS_DSP_WSEQ_H16:
+		cs_dsp_chunk_write(&chunk, 24, op_new->address);
+		cs_dsp_chunk_write(&chunk, 16, op_new->data);
+		break;
+	default:
+		ret = -EINVAL;
+		cs_dsp_err(dsp, "Operation %X not supported\n", op_code);
+		goto op_new_free;
+	}
+
+	new_op_size = cs_dsp_chunk_bytes(&chunk);
+
+	if (!update) {
+		if (wseq->ctl->len - op_end->offset < new_op_size) {
+			cs_dsp_err(dsp, "Not enough memory in %s for entry\n", wseq->ctl->subname);
+			ret = -E2BIG;
+			goto op_new_free;
+		}
+
+		op_end->offset += new_op_size;
+
+		ret = cs_dsp_coeff_write_ctrl(wseq->ctl, op_end->offset / sizeof(u32),
+					      &op_end->data, sizeof(u32));
+		if (ret)
+			goto op_new_free;
+
+		list_add_tail(&op_new->list, &op_end->list);
+	}
+
+	ret = cs_dsp_coeff_write_ctrl(wseq->ctl, op_new->offset / sizeof(u32),
+				      words, new_op_size);
+	if (ret)
+		goto op_new_free;
+
+	return 0;
+
+op_new_free:
+	devm_kfree(dsp->dev, op_new);
+
+	return ret;
+}
+EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_write, FW_CS_DSP);
+
+/**
+ * cs_dsp_wseq_multi_write() - Add or update multiple entries in a write sequence
+ * @dsp: Pointer to a DSP structure
+ * @wseq: Write sequence to write to
+ * @reg_seq: List of address-data pairs
+ * @num_regs: Number of address-data pairs
+ * @op_code: The types of operations of the new entries
+ * @update: If true, searches for the first entry in the write sequence with
+ * the same address and op_code, and replaces it. If false, creates a new entry
+ * at the tail
+ *
+ * This function calls cs_dsp_wseq_write() for multiple address-data pairs.
+ *
+ * Return: Zero for success, a negative number on error.
+ */
+int cs_dsp_wseq_multi_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
+			    const struct reg_sequence *reg_seq, int num_regs,
+			    u8 op_code, bool update)
+{
+	int i, ret;
+
+	for (i = 0; i < num_regs; i++) {
+		ret = cs_dsp_wseq_write(dsp, wseq, reg_seq[i].reg,
+					reg_seq[i].def, op_code, update);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_multi_write, FW_CS_DSP);
+
 MODULE_DESCRIPTION("Cirrus Logic DSP Support");
 MODULE_AUTHOR("Simon Trimmer <simont@opensource.cirrus.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/include/linux/firmware/cirrus/cs_dsp.h b/include/linux/firmware/cirrus/cs_dsp.h
index 29cd11d5a3cf..4cef6fafa1d8 100644
--- a/include/linux/firmware/cirrus/cs_dsp.h
+++ b/include/linux/firmware/cirrus/cs_dsp.h
@@ -42,6 +42,16 @@
 #define CS_DSP_ACKED_CTL_MIN_VALUE           0
 #define CS_DSP_ACKED_CTL_MAX_VALUE           0xFFFFFF
 
+/*
+ * Write sequence operation codes
+ */
+#define CS_DSP_WSEQ_FULL	0x00
+#define CS_DSP_WSEQ_ADDR8	0x02
+#define CS_DSP_WSEQ_L16		0x04
+#define CS_DSP_WSEQ_H16		0x05
+#define CS_DSP_WSEQ_UNLOCK	0xFD
+#define CS_DSP_WSEQ_END		0xFF
+
 /**
  * struct cs_dsp_region - Describes a logical memory region in DSP address space
  * @type:	Memory region type
@@ -255,6 +265,23 @@ struct cs_dsp_alg_region *cs_dsp_find_alg_region(struct cs_dsp *dsp,
 
 const char *cs_dsp_mem_region_name(unsigned int type);
 
+/**
+ * struct cs_dsp_wseq - Describes a write sequence
+ * @ctl:	Write sequence cs_dsp control
+ * @ops:	Operations contained within
+ */
+struct cs_dsp_wseq {
+	struct cs_dsp_coeff_ctl *ctl;
+	struct list_head ops;
+};
+
+int cs_dsp_wseq_init(struct cs_dsp *dsp, struct cs_dsp_wseq *wseqs, unsigned int num_wseqs);
+int cs_dsp_wseq_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq, u32 addr, u32 data,
+		      u8 op_code, bool update);
+int cs_dsp_wseq_multi_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
+			    const struct reg_sequence *reg_seq, int num_regs,
+			    u8 op_code, bool update);
+
 /**
  * struct cs_dsp_chunk - Describes a buffer holding data formatted for the DSP
  * @data:	Pointer to underlying buffer memory
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH RESEND v10 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding
  2024-04-08 15:32 [PATCH RESEND v10 0/5] Add support for CS40L50 James Ogletree
  2024-04-08 15:32 ` [PATCH RESEND v10 1/5] firmware: cs_dsp: Add write sequence interface James Ogletree
@ 2024-04-08 15:32 ` James Ogletree
  2024-04-08 15:32 ` [PATCH RESEND v10 3/5] mfd: cs40l50: Add support for CS40L50 core driver James Ogletree
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: James Ogletree @ 2024-04-08 15:32 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
	broonie, jeff
  Cc: patches, linux-sound, linux-input, devicetree, James Ogletree,
	Krzysztof Kozlowski

CS40L50 is a haptic driver with waveform memory,
integrated DSP, and closed-loop algorithms.

Add a YAML DT binding document for this device.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
 .../bindings/input/cirrus,cs40l50.yaml        | 68 +++++++++++++++++++
 MAINTAINERS                                   |  8 +++
 2 files changed, 76 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml

diff --git a/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml b/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
new file mode 100644
index 000000000000..89bd06864bd4
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/cirrus,cs40l50.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus Logic CS40L50 Advanced Haptic Driver
+
+maintainers:
+  - James Ogletree <jogletre@opensource.cirrus.com>
+
+description:
+  CS40L50 is a haptic driver with waveform memory,
+  integrated DSP, and closed-loop algorithms.
+
+properties:
+  compatible:
+    enum:
+      - cirrus,cs40l50
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  vdd-a-supply:
+    description: Power supply for internal analog circuits.
+
+  vdd-p-supply:
+    description: Power supply for always-on circuits.
+
+  vdd-io-supply:
+    description: Power supply for digital input/output.
+
+  vdd-b-supply:
+    description: Power supply for the boost converter.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - reset-gpios
+  - vdd-io-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      haptic-driver@34 {
+        compatible = "cirrus,cs40l50";
+        reg = <0x34>;
+        interrupt-parent = <&gpio>;
+        interrupts = <113 IRQ_TYPE_LEVEL_LOW>;
+        reset-gpios = <&gpio 112 GPIO_ACTIVE_LOW>;
+        vdd-io-supply = <&vreg>;
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index dd5de540ec0b..862a18e3fc7b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4933,6 +4933,14 @@ F:	sound/pci/hda/cs*
 F:	sound/pci/hda/hda_cs_dsp_ctl.*
 F:	sound/soc/codecs/cs*
 
+CIRRUS LOGIC HAPTIC DRIVERS
+M:	James Ogletree <jogletre@opensource.cirrus.com>
+M:	Fred Treven <fred.treven@cirrus.com>
+M:	Ben Bright <ben.bright@cirrus.com>
+L:	patches@opensource.cirrus.com
+S:	Supported
+F:	Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
+
 CIRRUS LOGIC DSP FIRMWARE DRIVER
 M:	Simon Trimmer <simont@opensource.cirrus.com>
 M:	Charles Keepax <ckeepax@opensource.cirrus.com>
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH RESEND v10 3/5] mfd: cs40l50: Add support for CS40L50 core driver
  2024-04-08 15:32 [PATCH RESEND v10 0/5] Add support for CS40L50 James Ogletree
  2024-04-08 15:32 ` [PATCH RESEND v10 1/5] firmware: cs_dsp: Add write sequence interface James Ogletree
  2024-04-08 15:32 ` [PATCH RESEND v10 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding James Ogletree
@ 2024-04-08 15:32 ` James Ogletree
  2024-04-11 15:35   ` Lee Jones
  2024-04-08 15:32 ` [PATCH RESEND v10 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver James Ogletree
  2024-04-08 15:32 ` [PATCH RESEND v10 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50 James Ogletree
  4 siblings, 1 reply; 16+ messages in thread
From: James Ogletree @ 2024-04-08 15:32 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
	broonie, jeff
  Cc: patches, linux-sound, linux-input, devicetree, James Ogletree

Introduce support for Cirrus Logic Device CS40L50: a
haptic driver with waveform memory, integrated DSP,
and closed-loop algorithms.

The MFD component registers and initializes the device.

Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
 MAINTAINERS                 |   2 +
 drivers/mfd/Kconfig         |  30 ++
 drivers/mfd/Makefile        |   4 +
 drivers/mfd/cs40l50-core.c  | 570 ++++++++++++++++++++++++++++++++++++
 drivers/mfd/cs40l50-i2c.c   |  68 +++++
 drivers/mfd/cs40l50-spi.c   |  68 +++++
 include/linux/mfd/cs40l50.h | 137 +++++++++
 7 files changed, 879 insertions(+)
 create mode 100644 drivers/mfd/cs40l50-core.c
 create mode 100644 drivers/mfd/cs40l50-i2c.c
 create mode 100644 drivers/mfd/cs40l50-spi.c
 create mode 100644 include/linux/mfd/cs40l50.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 862a18e3fc7b..e804f3766cba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4940,6 +4940,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/mfd/cs40l*
+F:	include/linux/mfd/cs40l*
 
 CIRRUS LOGIC DSP FIRMWARE DRIVER
 M:	Simon Trimmer <simont@opensource.cirrus.com>
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 90ce58fd629e..6273c255f107 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2241,6 +2241,36 @@ config MCP_UCB1200_TS
 
 endmenu
 
+config MFD_CS40L50_CORE
+	tristate
+	select MFD_CORE
+	select FW_CS_DSP
+	select REGMAP_IRQ
+
+config MFD_CS40L50_I2C
+	tristate "Cirrus Logic CS40L50 (I2C)"
+	select REGMAP_I2C
+	select MFD_CS40L50_CORE
+	depends on I2C
+	help
+	  Select this to support the Cirrus Logic CS40L50 Haptic
+	  Driver over I2C.
+
+	  This driver can be built as a module. If built as a module it will be
+	  called "cs40l50-i2c".
+
+config MFD_CS40L50_SPI
+	tristate "Cirrus Logic CS40L50 (SPI)"
+	select REGMAP_SPI
+	select MFD_CS40L50_CORE
+	depends on SPI
+	help
+	  Select this to support the Cirrus Logic CS40L50 Haptic
+	  Driver over SPI.
+
+	  This driver can be built as a module. If built as a module it will be
+	  called "cs40l50-spi".
+
 config MFD_VEXPRESS_SYSREG
 	tristate "Versatile Express System Registers"
 	depends on VEXPRESS_CONFIG && GPIOLIB
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index c66f07edcd0e..a8d18ba155d0 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -88,6 +88,10 @@ obj-$(CONFIG_MFD_MADERA)	+= madera.o
 obj-$(CONFIG_MFD_MADERA_I2C)	+= madera-i2c.o
 obj-$(CONFIG_MFD_MADERA_SPI)	+= madera-spi.o
 
+obj-$(CONFIG_MFD_CS40L50_CORE)	+= cs40l50-core.o
+obj-$(CONFIG_MFD_CS40L50_I2C)	+= cs40l50-i2c.o
+obj-$(CONFIG_MFD_CS40L50_SPI)	+= cs40l50-spi.o
+
 obj-$(CONFIG_TPS6105X)		+= tps6105x.o
 obj-$(CONFIG_TPS65010)		+= tps65010.o
 obj-$(CONFIG_TPS6507X)		+= tps6507x.o
diff --git a/drivers/mfd/cs40l50-core.c b/drivers/mfd/cs40l50-core.c
new file mode 100644
index 000000000000..34d0b1cfc486
--- /dev/null
+++ b/drivers/mfd/cs40l50-core.c
@@ -0,0 +1,570 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2024 Cirrus Logic, Inc.
+ *
+ * Author: James Ogletree <james.ogletree@cirrus.com>
+ */
+
+#include <linux/firmware/cirrus/cs_dsp.h>
+#include <linux/firmware/cirrus/wmfw.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/cs40l50.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+
+static const struct mfd_cell cs40l50_devs[] = {
+	{ .name = "cs40l50-codec", },
+	{ .name = "cs40l50-vibra", },
+};
+
+const struct regmap_config cs40l50_regmap = {
+	.reg_bits =		32,
+	.reg_stride =		4,
+	.val_bits =		32,
+	.reg_format_endian =	REGMAP_ENDIAN_BIG,
+	.val_format_endian =	REGMAP_ENDIAN_BIG,
+};
+EXPORT_SYMBOL_GPL(cs40l50_regmap);
+
+static const char * const cs40l50_supplies[] = {
+	"vdd-io",
+};
+
+static const struct regmap_irq cs40l50_reg_irqs[] = {
+	REGMAP_IRQ_REG(CS40L50_DSP_QUEUE_IRQ, CS40L50_IRQ1_INT_2_OFFSET,
+		       CS40L50_DSP_QUEUE_MASK),
+	REGMAP_IRQ_REG(CS40L50_AMP_SHORT_IRQ, CS40L50_IRQ1_INT_1_OFFSET,
+		       CS40L50_AMP_SHORT_MASK),
+	REGMAP_IRQ_REG(CS40L50_TEMP_ERR_IRQ, CS40L50_IRQ1_INT_8_OFFSET,
+		       CS40L50_TEMP_ERR_MASK),
+	REGMAP_IRQ_REG(CS40L50_BST_UVP_IRQ, CS40L50_IRQ1_INT_9_OFFSET,
+		       CS40L50_BST_UVP_MASK),
+	REGMAP_IRQ_REG(CS40L50_BST_SHORT_IRQ, CS40L50_IRQ1_INT_9_OFFSET,
+		       CS40L50_BST_SHORT_MASK),
+	REGMAP_IRQ_REG(CS40L50_BST_ILIMIT_IRQ, CS40L50_IRQ1_INT_9_OFFSET,
+		       CS40L50_BST_ILIMIT_MASK),
+	REGMAP_IRQ_REG(CS40L50_UVLO_VDDBATT_IRQ, CS40L50_IRQ1_INT_10_OFFSET,
+		       CS40L50_UVLO_VDDBATT_MASK),
+	REGMAP_IRQ_REG(CS40L50_GLOBAL_ERROR_IRQ, CS40L50_IRQ1_INT_18_OFFSET,
+		       CS40L50_GLOBAL_ERROR_MASK),
+};
+
+static struct regmap_irq_chip cs40l50_irq_chip = {
+	.name =		"cs40l50",
+	.status_base =	CS40L50_IRQ1_INT_1,
+	.mask_base =	CS40L50_IRQ1_MASK_1,
+	.ack_base =	CS40L50_IRQ1_INT_1,
+	.num_regs =	22,
+	.irqs =		cs40l50_reg_irqs,
+	.num_irqs =	ARRAY_SIZE(cs40l50_reg_irqs),
+	.runtime_pm =	true,
+};
+
+int cs40l50_dsp_write(struct device *dev, struct regmap *regmap, u32 val)
+{
+	int i, ret;
+	u32 ack;
+
+	/* Device NAKs if hibernating, so optionally retry */
+	for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) {
+		ret = regmap_write(regmap, CS40L50_DSP_QUEUE, val);
+		if (!ret)
+			break;
+
+		usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100);
+	}
+
+	/* If write never occurred, don't bother polling for ACK */
+	if (i == CS40L50_DSP_TIMEOUT_COUNT) {
+		dev_err(dev, "Timed out writing %#X to DSP: %d\n", val, ret);
+		return ret;
+	}
+
+	ret = regmap_read_poll_timeout(regmap, CS40L50_DSP_QUEUE, ack, !ack,
+				       CS40L50_DSP_POLL_US,
+				       CS40L50_DSP_POLL_US * CS40L50_DSP_TIMEOUT_COUNT);
+	if (ret)
+		dev_err(dev, "DSP failed to ACK %#X: %d\n", val, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cs40l50_dsp_write);
+
+static const struct cs_dsp_region cs40l50_dsp_regions[] = {
+	{ .type = WMFW_HALO_PM_PACKED, .base = CS40L50_PMEM_0 },
+	{ .type = WMFW_HALO_XM_PACKED, .base = CS40L50_XMEM_PACKED_0 },
+	{ .type = WMFW_HALO_YM_PACKED, .base = CS40L50_YMEM_PACKED_0 },
+	{ .type = WMFW_ADSP2_XM, .base = CS40L50_XMEM_UNPACKED24_0 },
+	{ .type = WMFW_ADSP2_YM, .base = CS40L50_YMEM_UNPACKED24_0 },
+};
+
+static const struct reg_sequence cs40l50_internal_vamp_config[] = {
+	{ CS40L50_BST_LPMODE_SEL, CS40L50_DCM_LOW_POWER },
+	{ CS40L50_BLOCK_ENABLES2, CS40L50_OVERTEMP_WARN },
+};
+
+static const struct reg_sequence cs40l50_irq_mask_override[] = {
+	{ CS40L50_IRQ1_MASK_2, CS40L50_IRQ_MASK_2_OVERRIDE },
+	{ CS40L50_IRQ1_MASK_20, CS40L50_IRQ_MASK_20_OVERRIDE },
+};
+
+static int cs40l50_wseq_init(struct cs40l50 *cs40l50)
+{
+	struct cs_dsp *dsp = &cs40l50->dsp;
+
+	cs40l50->wseqs[CS40L50_STANDBY].ctl = cs_dsp_get_ctl(dsp, "STANDBY_SEQUENCE",
+							     WMFW_ADSP2_XM,
+							     CS40L50_PM_ALGO);
+	if (!cs40l50->wseqs[CS40L50_STANDBY].ctl) {
+		dev_err(cs40l50->dev, "Control not found for standby sequence\n");
+		return -ENOENT;
+	}
+
+	cs40l50->wseqs[CS40L50_ACTIVE].ctl = cs_dsp_get_ctl(dsp, "ACTIVE_SEQUENCE",
+							    WMFW_ADSP2_XM,
+							    CS40L50_PM_ALGO);
+	if (!cs40l50->wseqs[CS40L50_ACTIVE].ctl) {
+		dev_err(cs40l50->dev, "Control not found for active sequence\n");
+		return -ENOENT;
+	}
+
+	cs40l50->wseqs[CS40L50_PWR_ON].ctl = cs_dsp_get_ctl(dsp, "PM_PWR_ON_SEQ",
+							    WMFW_ADSP2_XM,
+							    CS40L50_PM_ALGO);
+	if (!cs40l50->wseqs[CS40L50_PWR_ON].ctl) {
+		dev_err(cs40l50->dev, "Control not found for power-on sequence\n");
+		return -ENOENT;
+	}
+
+	return cs_dsp_wseq_init(&cs40l50->dsp, cs40l50->wseqs, ARRAY_SIZE(cs40l50->wseqs));
+}
+
+static int cs40l50_dsp_config(struct cs40l50 *cs40l50)
+{
+	int ret;
+
+	/* Configure internal V_AMP supply */
+	ret = regmap_multi_reg_write(cs40l50->regmap, cs40l50_internal_vamp_config,
+				     ARRAY_SIZE(cs40l50_internal_vamp_config));
+	if (ret)
+		return ret;
+
+	ret = cs_dsp_wseq_multi_write(&cs40l50->dsp, &cs40l50->wseqs[CS40L50_PWR_ON],
+				      cs40l50_internal_vamp_config, CS_DSP_WSEQ_FULL,
+				      ARRAY_SIZE(cs40l50_internal_vamp_config), false);
+	if (ret)
+		return ret;
+
+	/* Override firmware defaults for IRQ masks */
+	ret = regmap_multi_reg_write(cs40l50->regmap, cs40l50_irq_mask_override,
+				     ARRAY_SIZE(cs40l50_irq_mask_override));
+	if (ret)
+		return ret;
+
+	return cs_dsp_wseq_multi_write(&cs40l50->dsp, &cs40l50->wseqs[CS40L50_PWR_ON],
+				       cs40l50_irq_mask_override, CS_DSP_WSEQ_FULL,
+				       ARRAY_SIZE(cs40l50_irq_mask_override), false);
+}
+
+static int cs40l50_dsp_post_run(struct cs_dsp *dsp)
+{
+	struct cs40l50 *cs40l50 = container_of(dsp, struct cs40l50, dsp);
+	int ret;
+
+	ret = cs40l50_wseq_init(cs40l50);
+	if (ret)
+		return ret;
+
+	ret = cs40l50_dsp_config(cs40l50);
+	if (ret) {
+		dev_err(cs40l50->dev, "Failed to configure DSP: %d\n", ret);
+		return ret;
+	}
+
+	ret = devm_mfd_add_devices(cs40l50->dev, PLATFORM_DEVID_NONE, cs40l50_devs,
+				   ARRAY_SIZE(cs40l50_devs), NULL, 0, NULL);
+	if (ret)
+		dev_err(cs40l50->dev, "Failed to add child devices: %d\n", ret);
+
+	return ret;
+}
+
+static const struct cs_dsp_client_ops client_ops = {
+	.post_run = cs40l50_dsp_post_run,
+};
+
+static void cs40l50_dsp_remove(void *data)
+{
+	cs_dsp_remove(data);
+}
+
+static int cs40l50_dsp_init(struct cs40l50 *cs40l50)
+{
+	int ret;
+
+	cs40l50->dsp.num = 1;
+	cs40l50->dsp.type = WMFW_HALO;
+	cs40l50->dsp.dev = cs40l50->dev;
+	cs40l50->dsp.regmap = cs40l50->regmap;
+	cs40l50->dsp.base = CS40L50_CORE_BASE;
+	cs40l50->dsp.base_sysinfo = CS40L50_SYS_INFO_ID;
+	cs40l50->dsp.mem = cs40l50_dsp_regions;
+	cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions);
+	cs40l50->dsp.no_core_startstop = true;
+	cs40l50->dsp.client_ops = &client_ops;
+
+	ret = cs_dsp_halo_init(&cs40l50->dsp);
+	if (ret)
+		return ret;
+
+	return devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_remove,
+					&cs40l50->dsp);
+}
+
+static int cs40l50_reset_dsp(struct cs40l50 *cs40l50)
+{
+	int ret;
+
+	mutex_lock(&cs40l50->lock);
+
+	if (cs40l50->dsp.running)
+		cs_dsp_stop(&cs40l50->dsp);
+
+	if (cs40l50->dsp.booted)
+		cs_dsp_power_down(&cs40l50->dsp);
+
+	ret = cs40l50_dsp_write(cs40l50->dev, cs40l50->regmap, CS40L50_SHUTDOWN);
+	if (ret)
+		goto err_mutex;
+
+	ret = cs_dsp_power_up(&cs40l50->dsp, cs40l50->fw, "cs40l50.wmfw",
+			      cs40l50->bin, "cs40l50.bin", "cs40l50");
+	if (ret)
+		goto err_mutex;
+
+	ret = cs40l50_dsp_write(cs40l50->dev, cs40l50->regmap, CS40L50_SYSTEM_RESET);
+	if (ret)
+		goto err_mutex;
+
+	ret = cs40l50_dsp_write(cs40l50->dev, cs40l50->regmap, CS40L50_PREVENT_HIBER);
+	if (ret)
+		goto err_mutex;
+
+	ret = cs_dsp_run(&cs40l50->dsp);
+err_mutex:
+	mutex_unlock(&cs40l50->lock);
+
+	return ret;
+}
+
+static void cs40l50_dsp_power_down(void *data)
+{
+	cs_dsp_power_down(data);
+}
+
+static void cs40l50_dsp_stop(void *data)
+{
+	cs_dsp_stop(data);
+}
+
+static void cs40l50_dsp_bringup(const struct firmware *bin, void *context)
+{
+	struct cs40l50 *cs40l50 = context;
+	u32 nwaves;
+	int ret;
+
+	/* Wavetable is optional; bringup DSP regardless */
+	cs40l50->bin = bin;
+
+	ret = cs40l50_reset_dsp(cs40l50);
+	if (ret) {
+		dev_err(cs40l50->dev, "Failed to reset DSP: %d\n", ret);
+		goto err_fw;
+	}
+
+	ret = regmap_read(cs40l50->regmap, CS40L50_NUM_WAVES, &nwaves);
+	if (ret)
+		goto err_fw;
+
+	dev_info(cs40l50->dev, "%u RAM effects loaded\n", nwaves);
+
+	/* Add teardown actions for first-time bringup */
+	ret = devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_power_down,
+				       &cs40l50->dsp);
+	if (ret) {
+		dev_err(cs40l50->dev, "Failed to add power down action: %d\n", ret);
+		goto err_fw;
+	}
+
+	ret = devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_stop, &cs40l50->dsp);
+	if (ret)
+		dev_err(cs40l50->dev, "Failed to add stop action: %d\n", ret);
+err_fw:
+	release_firmware(cs40l50->bin);
+	release_firmware(cs40l50->fw);
+}
+
+static void cs40l50_request_firmware(const struct firmware *fw, void *context)
+{
+	struct cs40l50 *cs40l50 = context;
+	int ret;
+
+	if (!fw) {
+		dev_err(cs40l50->dev, "No firmware file found\n");
+		return;
+	}
+
+	cs40l50->fw = fw;
+
+	ret = request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT, CS40L50_WT,
+				      cs40l50->dev, GFP_KERNEL, cs40l50,
+				      cs40l50_dsp_bringup);
+	if (ret) {
+		dev_err(cs40l50->dev, "Failed to request %s: %d\n", CS40L50_WT, ret);
+		release_firmware(cs40l50->fw);
+	}
+}
+
+struct cs40l50_irq {
+	const char *name;
+	int virq;
+};
+
+static struct cs40l50_irq cs40l50_irqs[] = {
+	{ "DSP", },
+	{ "Global", },
+	{ "Boost UVLO", },
+	{ "Boost current limit", },
+	{ "Boost short", },
+	{ "Boost undervolt", },
+	{ "Overtemp", },
+	{ "Amp short", },
+};
+
+static const struct reg_sequence cs40l50_err_rls[] = {
+	{ CS40L50_ERR_RLS, CS40L50_GLOBAL_ERR_RLS_SET },
+	{ CS40L50_ERR_RLS, CS40L50_GLOBAL_ERR_RLS_CLEAR },
+};
+
+static irqreturn_t cs40l50_hw_err(int irq, void *data)
+{
+	struct cs40l50 *cs40l50 = data;
+	int ret = 0, i;
+
+	mutex_lock(&cs40l50->lock);
+
+	/* Log hardware interrupt and execute error release sequence */
+	for (i = 1; i < ARRAY_SIZE(cs40l50_irqs); i++) {
+		if (cs40l50_irqs[i].virq == irq) {
+			dev_err(cs40l50->dev, "%s error\n", cs40l50_irqs[i].name);
+			ret = regmap_multi_reg_write(cs40l50->regmap, cs40l50_err_rls,
+						     ARRAY_SIZE(cs40l50_err_rls));
+			break;
+		}
+	}
+
+	mutex_unlock(&cs40l50->lock);
+	return IRQ_RETVAL(!ret);
+}
+
+static irqreturn_t cs40l50_dsp_queue(int irq, void *data)
+{
+	struct cs40l50 *cs40l50 = data;
+	u32 rd_ptr, val, wt_ptr;
+	int ret = 0;
+
+	mutex_lock(&cs40l50->lock);
+
+	/* Read from DSP queue, log, and update read pointer */
+	while (!ret) {
+		ret = regmap_read(cs40l50->regmap, CS40L50_DSP_QUEUE_WT, &wt_ptr);
+		if (ret)
+			break;
+
+		ret = regmap_read(cs40l50->regmap, CS40L50_DSP_QUEUE_RD, &rd_ptr);
+		if (ret)
+			break;
+
+		/* Check if queue is empty */
+		if (wt_ptr == rd_ptr)
+			break;
+
+		ret = regmap_read(cs40l50->regmap, rd_ptr, &val);
+		if (ret)
+			break;
+
+		dev_dbg(cs40l50->dev, "DSP payload: %#X", val);
+
+		rd_ptr += sizeof(u32);
+
+		if (rd_ptr > CS40L50_DSP_QUEUE_END)
+			rd_ptr = CS40L50_DSP_QUEUE_BASE;
+
+		ret = regmap_write(cs40l50->regmap, CS40L50_DSP_QUEUE_RD, rd_ptr);
+	}
+
+	mutex_unlock(&cs40l50->lock);
+
+	return IRQ_RETVAL(!ret);
+}
+
+static int cs40l50_irq_init(struct cs40l50 *cs40l50)
+{
+	int ret, i, virq;
+
+	ret = devm_regmap_add_irq_chip(cs40l50->dev, cs40l50->regmap, cs40l50->irq,
+				       IRQF_ONESHOT | IRQF_SHARED, 0,
+				       &cs40l50_irq_chip, &cs40l50->irq_data);
+	if (ret) {
+		dev_err(cs40l50->dev, "Failed adding IRQ chip\n");
+		return ret;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(cs40l50_irqs); i++) {
+		virq = regmap_irq_get_virq(cs40l50->irq_data, i);
+		if (virq < 0) {
+			dev_err(cs40l50->dev, "Failed getting virq for %s\n",
+				cs40l50_irqs[i].name);
+			return virq;
+		}
+
+		cs40l50_irqs[i].virq = virq;
+
+		/* Handle DSP and hardware interrupts separately */
+		ret = devm_request_threaded_irq(cs40l50->dev, virq, NULL,
+						i ? cs40l50_hw_err : cs40l50_dsp_queue,
+						IRQF_ONESHOT | IRQF_SHARED,
+						cs40l50_irqs[i].name, cs40l50);
+		if (ret) {
+			return dev_err_probe(cs40l50->dev, ret,
+					     "Failed requesting %s IRQ\n",
+					     cs40l50_irqs[i].name);
+		}
+	}
+
+	return 0;
+}
+
+static int cs40l50_get_model(struct cs40l50 *cs40l50)
+{
+	int ret;
+
+	ret = regmap_read(cs40l50->regmap, CS40L50_DEVID, &cs40l50->devid);
+	if (ret)
+		return ret;
+
+	if (cs40l50->devid != CS40L50_DEVID_A)
+		return -EINVAL;
+
+	ret = regmap_read(cs40l50->regmap, CS40L50_REVID, &cs40l50->revid);
+	if (ret)
+		return ret;
+
+	if (cs40l50->revid < CS40L50_REVID_B0)
+		return -EINVAL;
+
+	dev_dbg(cs40l50->dev, "Cirrus Logic CS40L50 rev. %02X\n", cs40l50->revid);
+
+	return 0;
+}
+
+static int cs40l50_pm_runtime_setup(struct device *dev)
+{
+	int ret;
+
+	pm_runtime_set_autosuspend_delay(dev, CS40L50_AUTOSUSPEND_MS);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_get_noresume(dev);
+	ret = pm_runtime_set_active(dev);
+	if (ret)
+		return ret;
+
+	return devm_pm_runtime_enable(dev);
+}
+
+int cs40l50_probe(struct cs40l50 *cs40l50)
+{
+	struct device *dev = cs40l50->dev;
+	int ret;
+
+	mutex_init(&cs40l50->lock);
+
+	cs40l50->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(cs40l50->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(cs40l50->reset_gpio),
+				     "Failed getting reset GPIO\n");
+
+	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(cs40l50_supplies),
+					     cs40l50_supplies);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed getting supplies\n");
+
+	/* Ensure minimum reset pulse width */
+	usleep_range(CS40L50_RESET_PULSE_US, CS40L50_RESET_PULSE_US + 100);
+
+	gpiod_set_value_cansleep(cs40l50->reset_gpio, 0);
+
+	/* Wait for control port to be ready */
+	usleep_range(CS40L50_CP_READY_US, CS40L50_CP_READY_US + 100);
+
+	ret = cs40l50_get_model(cs40l50);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to get part number\n");
+
+	ret = cs40l50_dsp_init(cs40l50);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize DSP\n");
+
+	ret = cs40l50_pm_runtime_setup(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize runtime PM\n");
+
+	ret = cs40l50_irq_init(cs40l50);
+	if (ret)
+		return ret;
+
+	ret = request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT, CS40L50_FW,
+				      dev, GFP_KERNEL, cs40l50, cs40l50_request_firmware);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to request %s\n", CS40L50_FW);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cs40l50_probe);
+
+int cs40l50_remove(struct cs40l50 *cs40l50)
+{
+	gpiod_set_value_cansleep(cs40l50->reset_gpio, 1);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cs40l50_remove);
+
+static int cs40l50_runtime_suspend(struct device *dev)
+{
+	struct cs40l50 *cs40l50 = dev_get_drvdata(dev);
+
+	return regmap_write(cs40l50->regmap, CS40L50_DSP_QUEUE, CS40L50_ALLOW_HIBER);
+}
+
+static int cs40l50_runtime_resume(struct device *dev)
+{
+	struct cs40l50 *cs40l50 = dev_get_drvdata(dev);
+
+	return cs40l50_dsp_write(dev, cs40l50->regmap, CS40L50_PREVENT_HIBER);
+}
+
+EXPORT_GPL_DEV_PM_OPS(cs40l50_pm_ops) = {
+	RUNTIME_PM_OPS(cs40l50_runtime_suspend, cs40l50_runtime_resume, NULL)
+};
+
+MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver");
+MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(FW_CS_DSP);
diff --git a/drivers/mfd/cs40l50-i2c.c b/drivers/mfd/cs40l50-i2c.c
new file mode 100644
index 000000000000..639be743d956
--- /dev/null
+++ b/drivers/mfd/cs40l50-i2c.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2024 Cirrus Logic, Inc.
+ *
+ * Author: James Ogletree <james.ogletree@cirrus.com>
+ */
+
+#include <linux/i2c.h>
+#include <linux/mfd/cs40l50.h>
+
+static int cs40l50_i2c_probe(struct i2c_client *i2c)
+{
+	struct cs40l50 *cs40l50;
+
+	cs40l50 = devm_kzalloc(&i2c->dev, sizeof(*cs40l50), GFP_KERNEL);
+	if (!cs40l50)
+		return -ENOMEM;
+
+	i2c_set_clientdata(i2c, cs40l50);
+
+	cs40l50->dev = &i2c->dev;
+	cs40l50->irq = i2c->irq;
+
+	cs40l50->regmap = devm_regmap_init_i2c(i2c, &cs40l50_regmap);
+	if (IS_ERR(cs40l50->regmap))
+		return dev_err_probe(cs40l50->dev, PTR_ERR(cs40l50->regmap),
+				     "Failed to initialize register map\n");
+
+	return cs40l50_probe(cs40l50);
+}
+
+static void cs40l50_i2c_remove(struct i2c_client *i2c)
+{
+	struct cs40l50 *cs40l50 = i2c_get_clientdata(i2c);
+
+	cs40l50_remove(cs40l50);
+}
+
+static const struct i2c_device_id cs40l50_id_i2c[] = {
+	{ "cs40l50" },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, cs40l50_id_i2c);
+
+static const struct of_device_id cs40l50_of_match[] = {
+	{ .compatible = "cirrus,cs40l50" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, cs40l50_of_match);
+
+static struct i2c_driver cs40l50_i2c_driver = {
+	.driver = {
+		.name = "cs40l50",
+		.of_match_table = cs40l50_of_match,
+		.pm = pm_ptr(&cs40l50_pm_ops),
+	},
+	.id_table = cs40l50_id_i2c,
+	.probe = cs40l50_i2c_probe,
+	.remove = cs40l50_i2c_remove,
+};
+module_i2c_driver(cs40l50_i2c_driver);
+
+MODULE_DESCRIPTION("CS40L50 I2C Driver");
+MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/cs40l50-spi.c b/drivers/mfd/cs40l50-spi.c
new file mode 100644
index 000000000000..53526b595a0d
--- /dev/null
+++ b/drivers/mfd/cs40l50-spi.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2024 Cirrus Logic, Inc.
+ *
+ * Author: James Ogletree <james.ogletree@cirrus.com>
+ */
+
+#include <linux/mfd/cs40l50.h>
+#include <linux/spi/spi.h>
+
+static int cs40l50_spi_probe(struct spi_device *spi)
+{
+	struct cs40l50 *cs40l50;
+
+	cs40l50 = devm_kzalloc(&spi->dev, sizeof(*cs40l50), GFP_KERNEL);
+	if (!cs40l50)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, cs40l50);
+
+	cs40l50->dev = &spi->dev;
+	cs40l50->irq = spi->irq;
+
+	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");
+
+	return cs40l50_probe(cs40l50);
+}
+
+static void cs40l50_spi_remove(struct spi_device *spi)
+{
+	struct cs40l50 *cs40l50 = spi_get_drvdata(spi);
+
+	cs40l50_remove(cs40l50);
+}
+
+static const struct spi_device_id cs40l50_id_spi[] = {
+	{ "cs40l50" },
+	{}
+};
+MODULE_DEVICE_TABLE(spi, cs40l50_id_spi);
+
+static const struct of_device_id cs40l50_of_match[] = {
+	{ .compatible = "cirrus,cs40l50" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, cs40l50_of_match);
+
+static struct spi_driver cs40l50_spi_driver = {
+	.driver = {
+		.name = "cs40l50",
+		.of_match_table = cs40l50_of_match,
+		.pm = pm_ptr(&cs40l50_pm_ops),
+	},
+	.id_table = cs40l50_id_spi,
+	.probe = cs40l50_spi_probe,
+	.remove = cs40l50_spi_remove,
+};
+module_spi_driver(cs40l50_spi_driver);
+
+MODULE_DESCRIPTION("CS40L50 SPI Driver");
+MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/cs40l50.h b/include/linux/mfd/cs40l50.h
new file mode 100644
index 000000000000..e5dc49860944
--- /dev/null
+++ b/include/linux/mfd/cs40l50.h
@@ -0,0 +1,137 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2024 Cirrus Logic, Inc.
+ *
+ * Author: James Ogletree <james.ogletree@cirrus.com>
+ */
+
+#ifndef __MFD_CS40L50_H__
+#define __MFD_CS40L50_H__
+
+#include <linux/firmware/cirrus/cs_dsp.h>
+#include <linux/gpio/consumer.h>
+#include <linux/pm.h>
+#include <linux/regmap.h>
+
+/* Power Supply Configuration */
+#define CS40L50_BLOCK_ENABLES2		0x201C
+#define CS40L50_ERR_RLS			0x2034
+#define CS40L50_BST_LPMODE_SEL		0x3810
+#define CS40L50_DCM_LOW_POWER		0x1
+#define CS40L50_OVERTEMP_WARN		0x4000010
+
+/* Interrupts */
+#define CS40L50_IRQ1_INT_1		0xE010
+#define CS40L50_IRQ1_BASE		CS40L50_IRQ1_INT_1
+#define CS40L50_IRQ1_INT_2		0xE014
+#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_MASK_1		0xE090
+#define CS40L50_IRQ1_MASK_2		0xE094
+#define CS40L50_IRQ1_MASK_20		0xE0DC
+#define CS40L50_IRQ1_INT_1_OFFSET	(CS40L50_IRQ1_INT_1 - CS40L50_IRQ1_BASE)
+#define CS40L50_IRQ1_INT_2_OFFSET	(CS40L50_IRQ1_INT_2 - CS40L50_IRQ1_BASE)
+#define CS40L50_IRQ1_INT_8_OFFSET	(CS40L50_IRQ1_INT_8 - CS40L50_IRQ1_BASE)
+#define CS40L50_IRQ1_INT_9_OFFSET	(CS40L50_IRQ1_INT_9 - CS40L50_IRQ1_BASE)
+#define CS40L50_IRQ1_INT_10_OFFSET	(CS40L50_IRQ1_INT_10 - CS40L50_IRQ1_BASE)
+#define CS40L50_IRQ1_INT_18_OFFSET	(CS40L50_IRQ1_INT_18 - CS40L50_IRQ1_BASE)
+#define CS40L50_IRQ_MASK_2_OVERRIDE	0xFFDF7FFF
+#define CS40L50_IRQ_MASK_20_OVERRIDE	0x15C01000
+#define CS40L50_AMP_SHORT_MASK		BIT(31)
+#define CS40L50_DSP_QUEUE_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(18)
+#define CS40L50_UVLO_VDDBATT_MASK	BIT(16)
+#define CS40L50_GLOBAL_ERROR_MASK	BIT(15)
+
+enum cs40l50_irq_list {
+	CS40L50_DSP_QUEUE_IRQ,
+	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_AMP_SHORT_IRQ,
+};
+
+/* DSP */
+#define CS40L50_XMEM_PACKED_0		0x2000000
+#define CS40L50_XMEM_UNPACKED24_0	0x2800000
+#define CS40L50_SYS_INFO_ID		0x25E0000
+#define CS40L50_DSP_QUEUE_WT		0x28042C8
+#define CS40L50_DSP_QUEUE_RD		0x28042CC
+#define CS40L50_NUM_WAVES		0x2805C18
+#define CS40L50_CORE_BASE		0x2B80000
+#define CS40L50_YMEM_PACKED_0		0x2C00000
+#define CS40L50_YMEM_UNPACKED24_0	0x3400000
+#define CS40L50_PMEM_0			0x3800000
+#define CS40L50_DSP_POLL_US		1000
+#define CS40L50_DSP_TIMEOUT_COUNT	100
+#define CS40L50_RESET_PULSE_US		2200
+#define CS40L50_CP_READY_US		3100
+#define CS40L50_AUTOSUSPEND_MS		2000
+#define CS40L50_PM_ALGO			0x9F206
+#define CS40L50_GLOBAL_ERR_RLS_SET	BIT(11)
+#define CS40L50_GLOBAL_ERR_RLS_CLEAR	0
+
+enum cs40l50_wseqs {
+	CS40L50_PWR_ON,
+	CS40L50_STANDBY,
+	CS40L50_ACTIVE,
+	CS40L50_NUM_WSEQS,
+};
+
+/* DSP Queue */
+#define CS40L50_DSP_QUEUE_BASE		0x11004
+#define CS40L50_DSP_QUEUE_END		0x1101C
+#define CS40L50_DSP_QUEUE		0x11020
+#define CS40L50_PREVENT_HIBER		0x2000003
+#define CS40L50_ALLOW_HIBER		0x2000004
+#define CS40L50_SHUTDOWN		0x2000005
+#define CS40L50_SYSTEM_RESET		0x2000007
+#define CS40L50_START_I2S		0x3000002
+#define CS40L50_OWT_PUSH		0x3000008
+#define CS40L50_STOP_PLAYBACK		0x5000000
+#define CS40L50_OWT_DELETE		0xD000000
+
+/* Firmware files */
+#define CS40L50_FW			"cs40l50.wmfw"
+#define CS40L50_WT			"cs40l50.bin"
+
+/* Device */
+#define CS40L50_DEVID			0x0
+#define CS40L50_REVID			0x4
+#define CS40L50_DEVID_A			0x40A50
+#define CS40L50_REVID_B0		0xB0
+
+struct cs40l50 {
+	struct device *dev;
+	struct regmap *regmap;
+	struct mutex lock;
+	struct cs_dsp dsp;
+	struct gpio_desc *reset_gpio;
+	struct regmap_irq_chip_data *irq_data;
+	const struct firmware *fw;
+	const struct firmware *bin;
+	struct cs_dsp_wseq wseqs[CS40L50_NUM_WSEQS];
+	int irq;
+	u32 devid;
+	u32 revid;
+};
+
+int cs40l50_dsp_write(struct device *dev, struct regmap *regmap, u32 val);
+int cs40l50_probe(struct cs40l50 *cs40l50);
+int cs40l50_remove(struct cs40l50 *cs40l50);
+
+extern const struct regmap_config cs40l50_regmap;
+extern const struct dev_pm_ops cs40l50_pm_ops;
+
+#endif /* __MFD_CS40L50_H__ */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH RESEND v10 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
  2024-04-08 15:32 [PATCH RESEND v10 0/5] Add support for CS40L50 James Ogletree
                   ` (2 preceding siblings ...)
  2024-04-08 15:32 ` [PATCH RESEND v10 3/5] mfd: cs40l50: Add support for CS40L50 core driver James Ogletree
@ 2024-04-08 15:32 ` James Ogletree
  2024-04-16 23:28   ` Dmitry Torokhov
  2024-04-08 15:32 ` [PATCH RESEND v10 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50 James Ogletree
  4 siblings, 1 reply; 16+ messages in thread
From: James Ogletree @ 2024-04-08 15:32 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
	broonie, jeff
  Cc: patches, linux-sound, linux-input, devicetree, James Ogletree

Introduce support for Cirrus Logic Device CS40L50: a
haptic driver with waveform memory, integrated DSP,
and closed-loop algorithms.

The input driver provides the interface for control of
haptic effects through the device.

Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
v10:
Minor concern that playback stop is still misused with respect to not
specifying an effect ID. The device can only play one effect at a
time, but setting max effects for the EVIOCGEFFECTS ioctl to 1 would
restrict the number of uploads to 1 as well.

 MAINTAINERS                        |   1 +
 drivers/input/misc/Kconfig         |  10 +
 drivers/input/misc/Makefile        |   1 +
 drivers/input/misc/cs40l50-vibra.c | 577 +++++++++++++++++++++++++++++
 4 files changed, 589 insertions(+)
 create mode 100644 drivers/input/misc/cs40l50-vibra.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e804f3766cba..49c2e6e57b09 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4940,6 +4940,7 @@ 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:	drivers/mfd/cs40l*
 F:	include/linux/mfd/cs40l*
 
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 6ba984d7f0b1..ee45dbb0636e 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -140,6 +140,16 @@ config INPUT_BMA150
 	  To compile this driver as a module, choose M here: the
 	  module will be called bma150.
 
+config INPUT_CS40L50_VIBRA
+	tristate "CS40L50 Haptic Driver support"
+	depends on MFD_CS40L50_CORE
+	help
+	  Say Y here to enable support for Cirrus Logic's CS40L50
+	  haptic driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cs40l50-vibra.
+
 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..88279de6d3d5 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -28,6 +28,7 @@ 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_VIBRA)	+= cs40l50-vibra.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-vibra.c b/drivers/input/misc/cs40l50-vibra.c
new file mode 100644
index 000000000000..5440cf224f59
--- /dev/null
+++ b/drivers/input/misc/cs40l50-vibra.c
@@ -0,0 +1,577 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2024 Cirrus Logic, Inc.
+ *
+ * Author: James Ogletree <james.ogletree@cirrus.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/input.h>
+#include <linux/mfd/cs40l50.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+/* Wavetables */
+#define CS40L50_RAM_INDEX_START		0x1000000
+#define CS40L50_RAM_INDEX_END		0x100007F
+#define CS40L50_RTH_INDEX_START		0x1400000
+#define CS40L50_RTH_INDEX_END		0x1400001
+#define CS40L50_ROM_INDEX_START		0x1800000
+#define CS40L50_ROM_INDEX_END		0x180001A
+#define CS40L50_TYPE_PCM		8
+#define CS40L50_TYPE_PWLE		12
+#define CS40L50_PCM_ID			0x0
+#define CS40L50_OWT_CUSTOM_DATA_SIZE	2
+#define CS40L50_CUSTOM_DATA_MASK	0xFFFFU
+
+/* DSP */
+#define CS40L50_GPIO_BASE		0x2804140
+#define CS40L50_OWT_BASE		0x2805C34
+#define CS40L50_OWT_SIZE		0x2805C38
+#define CS40L50_OWT_NEXT		0x2805C3C
+
+/* GPIO */
+#define CS40L50_GPIO_NUM_MASK		GENMASK(14, 12)
+#define CS40L50_GPIO_EDGE_MASK		BIT(15)
+#define CS40L50_GPIO_MAPPING_NONE	0
+#define CS40L50_GPIO_DISABLE		0x1FF
+
+enum cs40l50_bank_type {
+	CS40L50_WVFRM_BANK_RAM,
+	CS40L50_WVFRM_BANK_ROM,
+	CS40L50_WVFRM_BANK_OWT,
+	CS40L50_WVFRM_BANK_NUM,
+};
+
+/* Describes an area in DSP memory populated by effects */
+struct cs40l50_bank {
+	enum cs40l50_bank_type type;
+	u32 base_index;
+	u32 max_index;
+};
+
+struct cs40l50_effect {
+	enum cs40l50_bank_type type;
+	struct list_head list;
+	u32 gpio_reg;
+	u32 index;
+	int id;
+};
+
+/* Describes haptic interface of loaded DSP firmware */
+struct cs40l50_vibra_dsp {
+	struct cs40l50_bank *banks;
+	u32 gpio_base_reg;
+	u32 owt_offset_reg;
+	u32 owt_size_reg;
+	u32 owt_base_reg;
+	u32 push_owt_cmd;
+	u32 delete_owt_cmd;
+	u32 stop_cmd;
+	int (*write)(struct device *dev, struct regmap *regmap, u32 val);
+};
+
+/* Describes configuration and state of haptic operations */
+struct cs40l50_vibra {
+	struct device *dev;
+	struct regmap *regmap;
+	struct input_dev *input;
+	struct mutex lock;
+	struct workqueue_struct *vibe_wq;
+	struct list_head effect_head;
+	struct cs40l50_vibra_dsp dsp;
+};
+
+struct cs40l50_work {
+	struct cs40l50_vibra *vibra;
+	struct ff_effect *effect;
+	struct work_struct work;
+	s16 *custom_data;
+	int custom_len;
+	int count;
+	int error;
+};
+
+static struct cs40l50_bank cs40l50_banks[] = {
+	{
+		.type =		CS40L50_WVFRM_BANK_RAM,
+		.base_index =	CS40L50_RAM_INDEX_START,
+		.max_index =	CS40L50_RAM_INDEX_END,
+	},
+	{
+		.type =		CS40L50_WVFRM_BANK_ROM,
+		.base_index =	CS40L50_ROM_INDEX_START,
+		.max_index =	CS40L50_ROM_INDEX_END,
+	},
+	{
+		.type =		CS40L50_WVFRM_BANK_OWT,
+		.base_index =	CS40L50_RTH_INDEX_START,
+		.max_index =	CS40L50_RTH_INDEX_END,
+	},
+};
+
+static struct cs40l50_vibra_dsp cs40l50_dsp = {
+	.banks =		cs40l50_banks,
+	.gpio_base_reg =	CS40L50_GPIO_BASE,
+	.owt_base_reg =		CS40L50_OWT_BASE,
+	.owt_offset_reg =	CS40L50_OWT_NEXT,
+	.owt_size_reg =		CS40L50_OWT_SIZE,
+	.push_owt_cmd =		CS40L50_OWT_PUSH,
+	.delete_owt_cmd =	CS40L50_OWT_DELETE,
+	.stop_cmd =		CS40L50_STOP_PLAYBACK,
+	.write =		cs40l50_dsp_write,
+};
+
+static struct cs40l50_effect *cs40l50_find_effect(int id, struct list_head *effect_head)
+{
+	struct cs40l50_effect *effect;
+
+	list_for_each_entry(effect, effect_head, list)
+		if (effect->id == id)
+			return effect;
+
+	return NULL;
+}
+
+static int cs40l50_effect_bank_set(struct cs40l50_work *work_data,
+				   struct cs40l50_effect *effect)
+{
+	s16 bank_type = work_data->custom_data[0] & CS40L50_CUSTOM_DATA_MASK;
+
+	if (bank_type >= CS40L50_WVFRM_BANK_NUM) {
+		dev_err(work_data->vibra->dev, "Invalid bank (%d)\n", bank_type);
+		return -EINVAL;
+	}
+
+	if (work_data->custom_len > CS40L50_OWT_CUSTOM_DATA_SIZE)
+		effect->type = CS40L50_WVFRM_BANK_OWT;
+	else
+		effect->type = bank_type;
+
+	return 0;
+}
+
+static int cs40l50_effect_gpio_mapping_set(struct cs40l50_work *work_data,
+					   struct cs40l50_effect *effect)
+{
+	u16 gpio_num, gpio_edge, button = work_data->effect->trigger.button;
+	struct cs40l50_vibra *vibra = work_data->vibra;
+
+	if (button) {
+		gpio_num = FIELD_GET(CS40L50_GPIO_NUM_MASK, button);
+		gpio_edge = FIELD_GET(CS40L50_GPIO_EDGE_MASK, button);
+		effect->gpio_reg = vibra->dsp.gpio_base_reg + (gpio_num * 8) - gpio_edge;
+
+		return regmap_write(vibra->regmap, effect->gpio_reg, button);
+	}
+
+	effect->gpio_reg = CS40L50_GPIO_MAPPING_NONE;
+
+	return 0;
+}
+
+static int cs40l50_effect_index_set(struct cs40l50_work *work_data,
+				   struct cs40l50_effect *effect)
+{
+	struct cs40l50_vibra *vibra = work_data->vibra;
+	struct cs40l50_effect *owt_effect;
+	u32 base_index, max_index;
+
+	base_index = vibra->dsp.banks[effect->type].base_index;
+	max_index = vibra->dsp.banks[effect->type].max_index;
+
+	effect->index = base_index;
+
+	switch (effect->type) {
+	case CS40L50_WVFRM_BANK_OWT:
+		list_for_each_entry(owt_effect, &vibra->effect_head, list)
+			if (owt_effect->type == CS40L50_WVFRM_BANK_OWT)
+				effect->index++;
+		break;
+	case CS40L50_WVFRM_BANK_ROM:
+	case CS40L50_WVFRM_BANK_RAM:
+		effect->index += work_data->custom_data[1] & CS40L50_CUSTOM_DATA_MASK;
+		break;
+	default:
+		dev_err(vibra->dev, "Bank type %d not supported\n", effect->type);
+		return -EINVAL;
+	}
+
+	if (effect->index > max_index || effect->index < base_index) {
+		dev_err(vibra->dev, "Index out of bounds: %u\n", effect->index);
+		return -ENOSPC;
+	}
+
+	return 0;
+}
+
+/* Describes a header for an OWT effect */
+struct cs40l50_owt_header {
+	u32 type;
+	u32 data_words;
+	u32 offset;
+} __packed;
+
+static int cs40l50_upload_owt(struct cs40l50_work *work_data)
+{
+	u32 len = 2 * work_data->custom_len, wt_offset, wt_size;
+	struct cs40l50_vibra *vibra = work_data->vibra;
+	struct cs40l50_owt_header header;
+	u8 *out_data;
+	int error;
+
+	error = regmap_read(vibra->regmap, vibra->dsp.owt_size_reg, &wt_size);
+	if (error)
+		return error;
+
+	if ((wt_size * sizeof(u32)) < sizeof(header) + len) {
+		dev_err(vibra->dev, "No space in OWT bank for effect\n");
+		return -ENOSPC;
+	}
+
+	out_data = kzalloc(sizeof(header) + len, GFP_KERNEL);
+	if (!out_data)
+		return -ENOMEM;
+
+	header.type = work_data->custom_data[0] == CS40L50_PCM_ID ? CS40L50_TYPE_PCM :
+								    CS40L50_TYPE_PWLE;
+	header.offset = sizeof(header) / sizeof(u32);
+	header.data_words = len / sizeof(u32);
+
+	memcpy(out_data, &header, sizeof(header));
+	memcpy(out_data + sizeof(header), work_data->custom_data, len);
+
+	error = regmap_read(vibra->regmap, vibra->dsp.owt_offset_reg, &wt_offset);
+	if (error)
+		goto err_free;
+
+	error = regmap_bulk_write(vibra->regmap, vibra->dsp.owt_base_reg +
+				  (wt_offset * sizeof(u32)), out_data,
+				  sizeof(header) + len);
+	if (error)
+		goto err_free;
+
+	error = vibra->dsp.write(vibra->dev, vibra->regmap, vibra->dsp.push_owt_cmd);
+err_free:
+	kfree(out_data);
+
+	return error;
+}
+
+static void cs40l50_add_worker(struct work_struct *work)
+{
+	struct cs40l50_work *work_data = container_of(work, struct cs40l50_work, work);
+	struct cs40l50_vibra *vibra = work_data->vibra;
+	struct cs40l50_effect *effect;
+	bool is_new = false;
+	int error;
+
+	error = pm_runtime_resume_and_get(vibra->dev);
+	if (error)
+		goto err_exit;
+
+	mutex_lock(&vibra->lock);
+
+	/* Update effect if already present, otherwise create new effect */
+	effect = cs40l50_find_effect(work_data->effect->id, &vibra->effect_head);
+	if (!effect) {
+		effect = kzalloc(sizeof(*effect), GFP_KERNEL);
+		if (!effect) {
+			error = -ENOMEM;
+			goto err_mutex;
+		}
+
+		effect->id = work_data->effect->id;
+		is_new = true;
+	}
+
+	error = cs40l50_effect_bank_set(work_data, effect);
+	if (error)
+		goto err_free;
+
+	error = cs40l50_effect_index_set(work_data, effect);
+	if (error)
+		goto err_free;
+
+	error = cs40l50_effect_gpio_mapping_set(work_data, effect);
+	if (error)
+		goto err_free;
+
+	if (effect->type == CS40L50_WVFRM_BANK_OWT)
+		error = cs40l50_upload_owt(work_data);
+err_free:
+	if (is_new) {
+		if (error)
+			kfree(effect);
+		else
+			list_add(&effect->list, &vibra->effect_head);
+	}
+err_mutex:
+	mutex_unlock(&vibra->lock);
+
+	pm_runtime_mark_last_busy(vibra->dev);
+	pm_runtime_put_autosuspend(vibra->dev);
+err_exit:
+	work_data->error = error;
+}
+
+static int cs40l50_add(struct input_dev *dev, struct ff_effect *effect,
+		       struct ff_effect *old)
+{
+	struct ff_periodic_effect *periodic = &effect->u.periodic;
+	struct cs40l50_vibra *vibra = input_get_drvdata(dev);
+	u32 len = effect->u.periodic.custom_len;
+	struct cs40l50_work work_data;
+
+	if (effect->type != FF_PERIODIC || periodic->waveform != FF_CUSTOM) {
+		dev_err(vibra->dev, "Type (%#X) or waveform (%#X) unsupported\n",
+			effect->type, periodic->waveform);
+		return -EINVAL;
+	}
+
+	work_data.custom_data = kcalloc(len, sizeof(s16), GFP_KERNEL);
+	if (!work_data.custom_data)
+		return -ENOMEM;
+
+	if (copy_from_user(work_data.custom_data, effect->u.periodic.custom_data,
+			   sizeof(s16) * len)) {
+		work_data.error = -EFAULT;
+		goto err_free;
+	}
+
+	work_data.custom_len = len;
+	work_data.vibra = vibra;
+	work_data.effect = effect;
+	INIT_WORK(&work_data.work, cs40l50_add_worker);
+
+	/* Push to the workqueue to serialize with playbacks */
+	queue_work(vibra->vibe_wq, &work_data.work);
+	flush_work(&work_data.work);
+err_free:
+	kfree(work_data.custom_data);
+
+	return work_data.error;
+}
+
+static void cs40l50_start_worker(struct work_struct *work)
+{
+	struct cs40l50_work *work_data = container_of(work, struct cs40l50_work, work);
+	struct cs40l50_vibra *vibra = work_data->vibra;
+	struct cs40l50_effect *start_effect;
+
+	if (pm_runtime_resume_and_get(vibra->dev) < 0)
+		goto err_free;
+
+	mutex_lock(&vibra->lock);
+
+	start_effect = cs40l50_find_effect(work_data->effect->id, &vibra->effect_head);
+	if (start_effect) {
+		while (--work_data->count >= 0) {
+			vibra->dsp.write(vibra->dev, vibra->regmap, start_effect->index);
+			usleep_range(work_data->effect->replay.length,
+				     work_data->effect->replay.length + 100);
+		}
+	}
+
+	mutex_unlock(&vibra->lock);
+
+	if (!start_effect)
+		dev_err(vibra->dev, "Effect to play not found\n");
+
+	pm_runtime_mark_last_busy(vibra->dev);
+	pm_runtime_put_autosuspend(vibra->dev);
+err_free:
+	kfree(work_data);
+}
+
+static void cs40l50_stop_worker(struct work_struct *work)
+{
+	struct cs40l50_work *work_data = container_of(work, struct cs40l50_work, work);
+	struct cs40l50_vibra *vibra = work_data->vibra;
+
+	if (pm_runtime_resume_and_get(vibra->dev) < 0)
+		return;
+
+	mutex_lock(&vibra->lock);
+
+	vibra->dsp.write(vibra->dev, vibra->regmap, vibra->dsp.stop_cmd);
+
+	mutex_unlock(&vibra->lock);
+
+	pm_runtime_mark_last_busy(vibra->dev);
+	pm_runtime_put_autosuspend(vibra->dev);
+
+	kfree(work_data);
+}
+
+static int cs40l50_playback(struct input_dev *dev, int effect_id, int val)
+{
+	struct cs40l50_vibra *vibra = input_get_drvdata(dev);
+	struct cs40l50_work *work_data;
+
+	work_data = kzalloc(sizeof(*work_data), GFP_ATOMIC);
+	if (!work_data)
+		return -ENOMEM;
+
+	work_data->vibra = vibra;
+
+	if (val > 0) {
+		work_data->effect = &dev->ff->effects[effect_id];
+		work_data->count = val;
+		INIT_WORK(&work_data->work, cs40l50_start_worker);
+	} else {
+		/* Just stop the amplifier as device drives only one effect */
+		INIT_WORK(&work_data->work, cs40l50_stop_worker);
+	}
+
+	queue_work(vibra->vibe_wq, &work_data->work);
+
+	return 0;
+}
+
+static void cs40l50_erase_worker(struct work_struct *work)
+{
+	struct cs40l50_work *work_data = container_of(work, struct cs40l50_work, work);
+	struct cs40l50_effect *owt_effect, *erase_effect;
+	struct cs40l50_vibra *vibra = work_data->vibra;
+	int error;
+
+	error = pm_runtime_resume_and_get(vibra->dev);
+	if (error)
+		goto err_exit;
+
+	mutex_lock(&vibra->lock);
+
+	erase_effect = cs40l50_find_effect(work_data->effect->id, &vibra->effect_head);
+	if (!erase_effect) {
+		dev_err(vibra->dev, "Effect to erase not found\n");
+		error = -EINVAL;
+		goto err_mutex;
+	}
+
+	if (erase_effect->gpio_reg != CS40L50_GPIO_MAPPING_NONE) {
+		error = regmap_write(vibra->regmap, erase_effect->gpio_reg,
+				     CS40L50_GPIO_DISABLE);
+		if (error)
+			goto err_mutex;
+	}
+
+	if (erase_effect->type == CS40L50_WVFRM_BANK_OWT) {
+		error = vibra->dsp.write(vibra->dev, vibra->regmap,
+					 vibra->dsp.delete_owt_cmd |
+					 erase_effect->index);
+		if (error)
+			goto err_mutex;
+
+		list_for_each_entry(owt_effect, &vibra->effect_head, list)
+			if (owt_effect->type == CS40L50_WVFRM_BANK_OWT &&
+			    owt_effect->index > erase_effect->index)
+				owt_effect->index--;
+	}
+
+	list_del(&erase_effect->list);
+	kfree(erase_effect);
+err_mutex:
+	mutex_unlock(&vibra->lock);
+
+	pm_runtime_mark_last_busy(vibra->dev);
+	pm_runtime_put_autosuspend(vibra->dev);
+err_exit:
+	work_data->error = error;
+}
+
+static int cs40l50_erase(struct input_dev *dev, int effect_id)
+{
+	struct cs40l50_vibra *vibra = input_get_drvdata(dev);
+	struct cs40l50_work work_data;
+
+	work_data.vibra = vibra;
+	work_data.effect = &dev->ff->effects[effect_id];
+
+	INIT_WORK(&work_data.work, cs40l50_erase_worker);
+
+	/* Push to workqueue to serialize with playbacks */
+	queue_work(vibra->vibe_wq, &work_data.work);
+	flush_work(&work_data.work);
+
+	return work_data.error;
+}
+
+static void cs40l50_remove_wq(void *data)
+{
+	flush_workqueue(data);
+	destroy_workqueue(data);
+}
+
+static int cs40l50_vibra_probe(struct platform_device *pdev)
+{
+	struct cs40l50 *cs40l50 = dev_get_drvdata(pdev->dev.parent);
+	struct cs40l50_vibra *vibra;
+	int error;
+
+	vibra = devm_kzalloc(pdev->dev.parent, sizeof(*vibra), GFP_KERNEL);
+	if (!vibra)
+		return -ENOMEM;
+
+	mutex_init(&vibra->lock);
+
+	vibra->dev = cs40l50->dev;
+	vibra->regmap = cs40l50->regmap;
+	vibra->dsp = cs40l50_dsp;
+
+	vibra->input = devm_input_allocate_device(vibra->dev);
+	if (!vibra->input)
+		return -ENOMEM;
+
+	vibra->input->id.product = cs40l50->devid;
+	vibra->input->id.version = cs40l50->revid;
+	vibra->input->name = "cs40l50_vibra";
+
+	input_set_drvdata(vibra->input, vibra);
+	input_set_capability(vibra->input, EV_FF, FF_PERIODIC);
+	input_set_capability(vibra->input, EV_FF, FF_CUSTOM);
+
+	error = input_ff_create(vibra->input, FF_MAX_EFFECTS);
+	if (error) {
+		dev_err(vibra->dev, "Failed to create input device\n");
+		return error;
+	}
+
+	vibra->input->ff->upload = cs40l50_add;
+	vibra->input->ff->playback = cs40l50_playback;
+	vibra->input->ff->erase = cs40l50_erase;
+
+	INIT_LIST_HEAD(&vibra->effect_head);
+
+	vibra->vibe_wq = alloc_ordered_workqueue("vibe_wq", WQ_HIGHPRI);
+	if (!vibra->vibe_wq)
+		return -ENOMEM;
+
+	error = devm_add_action_or_reset(vibra->dev, cs40l50_remove_wq, vibra->vibe_wq);
+	if (error)
+		return error;
+
+	return input_register_device(vibra->input);
+}
+
+static const struct platform_device_id cs40l50_vibra_id_match[] = {
+	{ "cs40l50-vibra", },
+	{}
+};
+MODULE_DEVICE_TABLE(platform, cs40l50_vibra_id_match);
+
+static struct platform_driver cs40l50_vibra_driver = {
+	.probe		= cs40l50_vibra_probe,
+	.id_table	= cs40l50_vibra_id_match,
+	.driver		= {
+		.name	= "cs40l50-vibra",
+	},
+};
+module_platform_driver(cs40l50_vibra_driver);
+
+MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver");
+MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH RESEND v10 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50
  2024-04-08 15:32 [PATCH RESEND v10 0/5] Add support for CS40L50 James Ogletree
                   ` (3 preceding siblings ...)
  2024-04-08 15:32 ` [PATCH RESEND v10 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver James Ogletree
@ 2024-04-08 15:32 ` James Ogletree
  2024-05-29 19:24   ` James Ogletree
  2024-05-30 17:07   ` Rivera-Matos, Ricardo
  4 siblings, 2 replies; 16+ messages in thread
From: James Ogletree @ 2024-04-08 15:32 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
	broonie, jeff
  Cc: patches, linux-sound, linux-input, devicetree, James Ogletree,
	David Rhodes

Introduce support for Cirrus Logic Device CS40L50: a
haptic driver with waveform memory, integrated DSP,
and closed-loop algorithms.

The ASoC driver enables I2S streaming to the device.

Reviewed-by: David Rhodes <drhodes@opensource.cirrus.com>
Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
 MAINTAINERS                      |   1 +
 sound/soc/codecs/Kconfig         |  11 ++
 sound/soc/codecs/Makefile        |   2 +
 sound/soc/codecs/cs40l50-codec.c | 308 +++++++++++++++++++++++++++++++
 4 files changed, 322 insertions(+)
 create mode 100644 sound/soc/codecs/cs40l50-codec.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 49c2e6e57b09..62701b13f741 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4943,6 +4943,7 @@ F:	Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
 F:	drivers/input/misc/cs40l*
 F:	drivers/mfd/cs40l*
 F:	include/linux/mfd/cs40l*
+F:	sound/soc/codecs/cs40l*
 
 CIRRUS LOGIC DSP FIRMWARE DRIVER
 M:	Simon Trimmer <simont@opensource.cirrus.com>
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index f1e1dbc509f6..1a81bedfdbe3 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -73,6 +73,7 @@ config SND_SOC_ALL_CODECS
 	imply SND_SOC_CS35L56_I2C
 	imply SND_SOC_CS35L56_SPI
 	imply SND_SOC_CS35L56_SDW
+	imply SND_SOC_CS40L50
 	imply SND_SOC_CS42L42
 	imply SND_SOC_CS42L42_SDW
 	imply SND_SOC_CS42L43
@@ -800,6 +801,16 @@ config SND_SOC_CS35L56_SDW
 	help
 	  Enable support for Cirrus Logic CS35L56 boosted amplifier with SoundWire control
 
+config SND_SOC_CS40L50
+	tristate "Cirrus Logic CS40L50 CODEC"
+	depends on MFD_CS40L50_CORE
+	help
+	  This option enables support for I2S streaming to Cirrus Logic CS40L50.
+
+	  CS40L50 is a haptic driver with waveform memory, an integrated
+	  DSP, and closed-loop algorithms. If built as a module, it will be
+	  called snd-soc-cs40l50.
+
 config SND_SOC_CS42L42_CORE
 	tristate
 
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index a87e56938ce5..7e31f000774a 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -74,6 +74,7 @@ snd-soc-cs35l56-shared-objs := cs35l56-shared.o
 snd-soc-cs35l56-i2c-objs := cs35l56-i2c.o
 snd-soc-cs35l56-spi-objs := cs35l56-spi.o
 snd-soc-cs35l56-sdw-objs := cs35l56-sdw.o
+snd-soc-cs40l50-objs := cs40l50-codec.o
 snd-soc-cs42l42-objs := cs42l42.o
 snd-soc-cs42l42-i2c-objs := cs42l42-i2c.o
 snd-soc-cs42l42-sdw-objs := cs42l42-sdw.o
@@ -460,6 +461,7 @@ obj-$(CONFIG_SND_SOC_CS35L56_SHARED)	+= snd-soc-cs35l56-shared.o
 obj-$(CONFIG_SND_SOC_CS35L56_I2C)	+= snd-soc-cs35l56-i2c.o
 obj-$(CONFIG_SND_SOC_CS35L56_SPI)	+= snd-soc-cs35l56-spi.o
 obj-$(CONFIG_SND_SOC_CS35L56_SDW)	+= snd-soc-cs35l56-sdw.o
+obj-$(CONFIG_SND_SOC_CS40L50)		+= snd-soc-cs40l50.o
 obj-$(CONFIG_SND_SOC_CS42L42_CORE)	+= snd-soc-cs42l42.o
 obj-$(CONFIG_SND_SOC_CS42L42)	+= snd-soc-cs42l42-i2c.o
 obj-$(CONFIG_SND_SOC_CS42L42_SDW)	+= snd-soc-cs42l42-sdw.o
diff --git a/sound/soc/codecs/cs40l50-codec.c b/sound/soc/codecs/cs40l50-codec.c
new file mode 100644
index 000000000000..6d4a0970b219
--- /dev/null
+++ b/sound/soc/codecs/cs40l50-codec.c
@@ -0,0 +1,308 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// CS40L50 Advanced Haptic Driver with waveform memory,
+// integrated DSP, and closed-loop algorithms
+//
+// Copyright 2024 Cirrus Logic, Inc.
+//
+// Author: James Ogletree <james.ogletree@cirrus.com>
+
+#include <linux/bitfield.h>
+#include <linux/mfd/cs40l50.h>
+#include <linux/pm_runtime.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+#define CS40L50_REFCLK_INPUT		0x2C04
+#define CS40L50_ASP_CONTROL2		0x4808
+#define CS40L50_ASP_DATA_CONTROL5	0x4840
+
+/* PLL Config */
+#define CS40L50_PLL_REFCLK_BCLK		0x0
+#define CS40L50_PLL_REFCLK_MCLK		0x5
+#define CS40L50_PLL_REEFCLK_MCLK_CFG	0x00
+#define CS40L50_PLL_REFCLK_LOOP_MASK	BIT(11)
+#define CS40L50_PLL_REFCLK_OPEN_LOOP	1
+#define CS40L50_PLL_REFCLK_CLOSED_LOOP	0
+#define CS40L50_PLL_REFCLK_LOOP_SHIFT	11
+#define CS40L50_PLL_REFCLK_FREQ_MASK	GENMASK(10, 5)
+#define CS40L50_PLL_REFCLK_FREQ_SHIFT	5
+#define CS40L50_PLL_REFCLK_SEL_MASK	GENMASK(2, 0)
+#define CS40L50_BCLK_RATIO_DEFAULT	32
+
+/* ASP Config */
+#define CS40L50_ASP_RX_WIDTH_SHIFT	24
+#define CS40L50_ASP_RX_WIDTH_MASK	GENMASK(31, 24)
+#define CS40L50_ASP_RX_WL_MASK		GENMASK(5, 0)
+#define CS40L50_ASP_FSYNC_INV_MASK	BIT(2)
+#define CS40L50_ASP_BCLK_INV_MASK	BIT(6)
+#define CS40L50_ASP_FMT_MASK		GENMASK(10, 8)
+#define CS40L50_ASP_FMT_I2S		0x2
+
+struct cs40l50_pll_config {
+	unsigned int freq;
+	unsigned int cfg;
+};
+
+struct cs40l50_codec {
+	struct device *dev;
+	struct regmap *regmap;
+	unsigned int daifmt;
+	unsigned int bclk_ratio;
+	unsigned int rate;
+};
+
+static const struct cs40l50_pll_config cs40l50_pll_cfg[] = {
+	{ 32768, 0x00 },
+	{ 1536000, 0x1B },
+	{ 3072000, 0x21 },
+	{ 6144000, 0x28 },
+	{ 9600000, 0x30 },
+	{ 12288000, 0x33 },
+};
+
+static int cs40l50_get_clk_config(unsigned int freq, unsigned int *cfg)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(cs40l50_pll_cfg); i++) {
+		if (cs40l50_pll_cfg[i].freq == freq) {
+			*cfg = cs40l50_pll_cfg[i].cfg;
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int cs40l50_swap_ext_clk(struct cs40l50_codec *codec, unsigned int clk_src)
+{
+	unsigned int cfg;
+	int ret;
+
+	switch (clk_src) {
+	case CS40L50_PLL_REFCLK_BCLK:
+		ret = cs40l50_get_clk_config(codec->bclk_ratio * codec->rate, &cfg);
+		if (ret)
+			return ret;
+		break;
+	case CS40L50_PLL_REFCLK_MCLK:
+		cfg = CS40L50_PLL_REEFCLK_MCLK_CFG;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
+				 CS40L50_PLL_REFCLK_LOOP_MASK,
+				 CS40L50_PLL_REFCLK_OPEN_LOOP <<
+				 CS40L50_PLL_REFCLK_LOOP_SHIFT);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
+				 CS40L50_PLL_REFCLK_FREQ_MASK |
+				 CS40L50_PLL_REFCLK_SEL_MASK,
+				 (cfg << CS40L50_PLL_REFCLK_FREQ_SHIFT) | clk_src);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
+				  CS40L50_PLL_REFCLK_LOOP_MASK,
+				  CS40L50_PLL_REFCLK_CLOSED_LOOP <<
+				  CS40L50_PLL_REFCLK_LOOP_SHIFT);
+}
+
+static int cs40l50_clk_en(struct snd_soc_dapm_widget *w,
+			  struct snd_kcontrol *kcontrol,
+			  int event)
+{
+	struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
+	struct cs40l50_codec *codec = snd_soc_component_get_drvdata(comp);
+	int ret;
+
+	switch (event) {
+	case SND_SOC_DAPM_POST_PMU:
+		ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_STOP_PLAYBACK);
+		if (ret)
+			return ret;
+
+		ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_START_I2S);
+		if (ret)
+			return ret;
+
+		ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_BCLK);
+		if (ret)
+			return ret;
+		break;
+	case SND_SOC_DAPM_PRE_PMD:
+		ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_MCLK);
+		if (ret)
+			return ret;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_dapm_widget cs40l50_dapm_widgets[] = {
+	SND_SOC_DAPM_SUPPLY_S("ASP PLL", 0, SND_SOC_NOPM, 0, 0, cs40l50_clk_en,
+			      SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
+	SND_SOC_DAPM_AIF_IN("ASPRX1", NULL, 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_IN("ASPRX2", NULL, 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_OUTPUT("OUT"),
+};
+
+static const struct snd_soc_dapm_route cs40l50_dapm_routes[] = {
+	{ "ASP Playback", NULL, "ASP PLL" },
+	{ "ASPRX1", NULL, "ASP Playback" },
+	{ "ASPRX2", NULL, "ASP Playback" },
+
+	{ "OUT", NULL, "ASPRX1" },
+	{ "OUT", NULL, "ASPRX2" },
+};
+
+static int cs40l50_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
+{
+	struct cs40l50_codec *codec = snd_soc_component_get_drvdata(codec_dai->component);
+
+	if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBC_CFC)
+		return -EINVAL;
+
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		codec->daifmt = 0;
+		break;
+	case SND_SOC_DAIFMT_NB_IF:
+		codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		codec->daifmt = CS40L50_ASP_BCLK_INV_MASK;
+		break;
+	case SND_SOC_DAIFMT_IB_IF:
+		codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK | CS40L50_ASP_BCLK_INV_MASK;
+		break;
+	default:
+		dev_err(codec->dev, "Invalid clock invert\n");
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		codec->daifmt |= FIELD_PREP(CS40L50_ASP_FMT_MASK, CS40L50_ASP_FMT_I2S);
+		break;
+	default:
+		dev_err(codec->dev, "Unsupported DAI format\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int cs40l50_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *dai)
+{
+	struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component);
+	unsigned int asp_rx_wl = params_width(params);
+	int ret;
+
+	codec->rate = params_rate(params);
+
+	ret = regmap_update_bits(codec->regmap, CS40L50_ASP_DATA_CONTROL5,
+				 CS40L50_ASP_RX_WL_MASK, asp_rx_wl);
+	if (ret)
+		return ret;
+
+	codec->daifmt |= (asp_rx_wl << CS40L50_ASP_RX_WIDTH_SHIFT);
+
+	return regmap_update_bits(codec->regmap, CS40L50_ASP_CONTROL2,
+				  CS40L50_ASP_FSYNC_INV_MASK |
+				  CS40L50_ASP_BCLK_INV_MASK |
+				  CS40L50_ASP_FMT_MASK |
+				  CS40L50_ASP_RX_WIDTH_MASK, codec->daifmt);
+}
+
+static int cs40l50_set_dai_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio)
+{
+	struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component);
+
+	codec->bclk_ratio = ratio;
+
+	return 0;
+}
+
+static const struct snd_soc_dai_ops cs40l50_dai_ops = {
+	.set_fmt = cs40l50_set_dai_fmt,
+	.set_bclk_ratio = cs40l50_set_dai_bclk_ratio,
+	.hw_params = cs40l50_hw_params,
+};
+
+static struct snd_soc_dai_driver cs40l50_dai[] = {
+	{
+		.name = "cs40l50-pcm",
+		.id = 0,
+		.playback = {
+			.stream_name = "ASP Playback",
+			.channels_min = 1,
+			.channels_max = 2,
+			.rates = SNDRV_PCM_RATE_48000,
+			.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+		},
+		.ops = &cs40l50_dai_ops,
+	},
+};
+
+static int cs40l50_codec_probe(struct snd_soc_component *component)
+{
+	struct cs40l50_codec *codec = snd_soc_component_get_drvdata(component);
+
+	codec->bclk_ratio = CS40L50_BCLK_RATIO_DEFAULT;
+
+	return 0;
+}
+
+static const struct snd_soc_component_driver soc_codec_dev_cs40l50 = {
+	.probe = cs40l50_codec_probe,
+	.dapm_widgets = cs40l50_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(cs40l50_dapm_widgets),
+	.dapm_routes = cs40l50_dapm_routes,
+	.num_dapm_routes = ARRAY_SIZE(cs40l50_dapm_routes),
+};
+
+static int cs40l50_codec_driver_probe(struct platform_device *pdev)
+{
+	struct cs40l50 *cs40l50 = dev_get_drvdata(pdev->dev.parent);
+	struct cs40l50_codec *codec;
+
+	codec = devm_kzalloc(&pdev->dev, sizeof(*codec), GFP_KERNEL);
+	if (!codec)
+		return -ENOMEM;
+
+	codec->regmap = cs40l50->regmap;
+	codec->dev = &pdev->dev;
+
+	return devm_snd_soc_register_component(&pdev->dev, &soc_codec_dev_cs40l50,
+					       cs40l50_dai, ARRAY_SIZE(cs40l50_dai));
+}
+
+static const struct platform_device_id cs40l50_id[] = {
+	{ "cs40l50-codec", },
+	{}
+};
+MODULE_DEVICE_TABLE(platform, cs40l50_id);
+
+static struct platform_driver cs40l50_codec_driver = {
+	.probe = cs40l50_codec_driver_probe,
+	.id_table = cs40l50_id,
+	.driver = {
+		.name = "cs40l50-codec",
+	},
+};
+module_platform_driver(cs40l50_codec_driver);
+
+MODULE_DESCRIPTION("ASoC CS40L50 driver");
+MODULE_AUTHOR("James Ogletree <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND v10 3/5] mfd: cs40l50: Add support for CS40L50 core driver
  2024-04-08 15:32 ` [PATCH RESEND v10 3/5] mfd: cs40l50: Add support for CS40L50 core driver James Ogletree
@ 2024-04-11 15:35   ` Lee Jones
  0 siblings, 0 replies; 16+ messages in thread
From: Lee Jones @ 2024-04-11 15:35 UTC (permalink / raw)
  To: James Ogletree
  Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	broonie, jeff, patches, linux-sound, linux-input, devicetree

On Mon, 08 Apr 2024, James Ogletree wrote:

> Introduce support for Cirrus Logic Device CS40L50: a
> haptic driver with waveform memory, integrated DSP,
> and closed-loop algorithms.
> 
> The MFD component registers and initializes the device.
> 
> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
> ---
>  MAINTAINERS                 |   2 +
>  drivers/mfd/Kconfig         |  30 ++
>  drivers/mfd/Makefile        |   4 +
>  drivers/mfd/cs40l50-core.c  | 570 ++++++++++++++++++++++++++++++++++++
>  drivers/mfd/cs40l50-i2c.c   |  68 +++++
>  drivers/mfd/cs40l50-spi.c   |  68 +++++
>  include/linux/mfd/cs40l50.h | 137 +++++++++
>  7 files changed, 879 insertions(+)
>  create mode 100644 drivers/mfd/cs40l50-core.c
>  create mode 100644 drivers/mfd/cs40l50-i2c.c
>  create mode 100644 drivers/mfd/cs40l50-spi.c
>  create mode 100644 include/linux/mfd/cs40l50.h

Looks like this hasn't been updated since my last review.

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND v10 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
  2024-04-08 15:32 ` [PATCH RESEND v10 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver James Ogletree
@ 2024-04-16 23:28   ` Dmitry Torokhov
  2024-04-19 18:26     ` James Ogletree
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2024-04-16 23:28 UTC (permalink / raw)
  To: James Ogletree
  Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, lee, broonie, jeff,
	patches, linux-sound, linux-input, devicetree

Hi James,

On Mon, Apr 08, 2024 at 03:32:13PM +0000, James Ogletree wrote:
> Introduce support for Cirrus Logic Device CS40L50: a
> haptic driver with waveform memory, integrated DSP,
> and closed-loop algorithms.
> 
> The input driver provides the interface for control of
> haptic effects through the device.
> 
> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
> ---
> v10:
> Minor concern that playback stop is still misused with respect to not
> specifying an effect ID. The device can only play one effect at a
> time, but setting max effects for the EVIOCGEFFECTS ioctl to 1 would
> restrict the number of uploads to 1 as well.

Sorry for a long delay on my part.

Unfortunately this is not a minor concern, because it breaks the API
that we so far been presenting to userspace. EVIOCGEFFECTS ioctl which returns
input->ff->max_effects is described as "Report number of effects
playable at the same time".

I suggest you limit number of effects to 1 so that we can merge the
driver with such limitation, and then try to figure out how to expand
the API. We will probably have to split the notion of number of playable
effects vs number of uploadable effects, and only accept uploads of
effects that exceed number of playable effects if they all have the same
owner, so that different processes would not be able to affect each
other in case when number of simultaneously playable effects is smaller.

A few code comments below...

> 
>  MAINTAINERS                        |   1 +
>  drivers/input/misc/Kconfig         |  10 +
>  drivers/input/misc/Makefile        |   1 +
>  drivers/input/misc/cs40l50-vibra.c | 577 +++++++++++++++++++++++++++++
>  4 files changed, 589 insertions(+)
>  create mode 100644 drivers/input/misc/cs40l50-vibra.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e804f3766cba..49c2e6e57b09 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4940,6 +4940,7 @@ 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:	drivers/mfd/cs40l*
>  F:	include/linux/mfd/cs40l*
>  
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 6ba984d7f0b1..ee45dbb0636e 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -140,6 +140,16 @@ config INPUT_BMA150
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called bma150.
>  
> +config INPUT_CS40L50_VIBRA
> +	tristate "CS40L50 Haptic Driver support"
> +	depends on MFD_CS40L50_CORE
> +	help
> +	  Say Y here to enable support for Cirrus Logic's CS40L50
> +	  haptic driver.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called cs40l50-vibra.
> +
>  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..88279de6d3d5 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -28,6 +28,7 @@ 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_VIBRA)	+= cs40l50-vibra.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-vibra.c b/drivers/input/misc/cs40l50-vibra.c
> new file mode 100644
> index 000000000000..5440cf224f59
> --- /dev/null
> +++ b/drivers/input/misc/cs40l50-vibra.c
> @@ -0,0 +1,577 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CS40L50 Advanced Haptic Driver with waveform memory,
> + * integrated DSP, and closed-loop algorithms
> + *
> + * Copyright 2024 Cirrus Logic, Inc.
> + *
> + * Author: James Ogletree <james.ogletree@cirrus.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/input.h>
> +#include <linux/mfd/cs40l50.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +/* Wavetables */
> +#define CS40L50_RAM_INDEX_START		0x1000000
> +#define CS40L50_RAM_INDEX_END		0x100007F
> +#define CS40L50_RTH_INDEX_START		0x1400000
> +#define CS40L50_RTH_INDEX_END		0x1400001
> +#define CS40L50_ROM_INDEX_START		0x1800000
> +#define CS40L50_ROM_INDEX_END		0x180001A
> +#define CS40L50_TYPE_PCM		8
> +#define CS40L50_TYPE_PWLE		12
> +#define CS40L50_PCM_ID			0x0
> +#define CS40L50_OWT_CUSTOM_DATA_SIZE	2
> +#define CS40L50_CUSTOM_DATA_MASK	0xFFFFU
> +
> +/* DSP */
> +#define CS40L50_GPIO_BASE		0x2804140
> +#define CS40L50_OWT_BASE		0x2805C34
> +#define CS40L50_OWT_SIZE		0x2805C38
> +#define CS40L50_OWT_NEXT		0x2805C3C
> +
> +/* GPIO */
> +#define CS40L50_GPIO_NUM_MASK		GENMASK(14, 12)
> +#define CS40L50_GPIO_EDGE_MASK		BIT(15)
> +#define CS40L50_GPIO_MAPPING_NONE	0
> +#define CS40L50_GPIO_DISABLE		0x1FF
> +
> +enum cs40l50_bank_type {
> +	CS40L50_WVFRM_BANK_RAM,
> +	CS40L50_WVFRM_BANK_ROM,
> +	CS40L50_WVFRM_BANK_OWT,
> +	CS40L50_WVFRM_BANK_NUM,
> +};
> +
> +/* Describes an area in DSP memory populated by effects */
> +struct cs40l50_bank {
> +	enum cs40l50_bank_type type;
> +	u32 base_index;
> +	u32 max_index;

This looks like it is written to the device, so I think this needs
proper endianness annotation. Is there any concern about padding between
the type and base_index?

> +};
> +
> +struct cs40l50_effect {
> +	enum cs40l50_bank_type type;
> +	struct list_head list;
> +	u32 gpio_reg;
> +	u32 index;
> +	int id;
> +};
> +
> +/* Describes haptic interface of loaded DSP firmware */
> +struct cs40l50_vibra_dsp {
> +	struct cs40l50_bank *banks;
> +	u32 gpio_base_reg;
> +	u32 owt_offset_reg;
> +	u32 owt_size_reg;
> +	u32 owt_base_reg;
> +	u32 push_owt_cmd;
> +	u32 delete_owt_cmd;
> +	u32 stop_cmd;
> +	int (*write)(struct device *dev, struct regmap *regmap, u32 val);
> +};
> +
> +/* Describes configuration and state of haptic operations */
> +struct cs40l50_vibra {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct input_dev *input;
> +	struct mutex lock;
> +	struct workqueue_struct *vibe_wq;
> +	struct list_head effect_head;
> +	struct cs40l50_vibra_dsp dsp;
> +};
> +
> +struct cs40l50_work {
> +	struct cs40l50_vibra *vibra;
> +	struct ff_effect *effect;
> +	struct work_struct work;
> +	s16 *custom_data;
> +	int custom_len;
> +	int count;
> +	int error;
> +};
> +
> +static struct cs40l50_bank cs40l50_banks[] = {
> +	{
> +		.type =		CS40L50_WVFRM_BANK_RAM,
> +		.base_index =	CS40L50_RAM_INDEX_START,
> +		.max_index =	CS40L50_RAM_INDEX_END,
> +	},
> +	{
> +		.type =		CS40L50_WVFRM_BANK_ROM,
> +		.base_index =	CS40L50_ROM_INDEX_START,
> +		.max_index =	CS40L50_ROM_INDEX_END,
> +	},
> +	{
> +		.type =		CS40L50_WVFRM_BANK_OWT,
> +		.base_index =	CS40L50_RTH_INDEX_START,
> +		.max_index =	CS40L50_RTH_INDEX_END,
> +	},
> +};
> +
> +static struct cs40l50_vibra_dsp cs40l50_dsp = {
> +	.banks =		cs40l50_banks,
> +	.gpio_base_reg =	CS40L50_GPIO_BASE,
> +	.owt_base_reg =		CS40L50_OWT_BASE,
> +	.owt_offset_reg =	CS40L50_OWT_NEXT,
> +	.owt_size_reg =		CS40L50_OWT_SIZE,
> +	.push_owt_cmd =		CS40L50_OWT_PUSH,
> +	.delete_owt_cmd =	CS40L50_OWT_DELETE,
> +	.stop_cmd =		CS40L50_STOP_PLAYBACK,
> +	.write =		cs40l50_dsp_write,
> +};
> +
> +static struct cs40l50_effect *cs40l50_find_effect(int id, struct list_head *effect_head)
> +{
> +	struct cs40l50_effect *effect;
> +
> +	list_for_each_entry(effect, effect_head, list)
> +		if (effect->id == id)
> +			return effect;
> +
> +	return NULL;
> +}
> +
> +static int cs40l50_effect_bank_set(struct cs40l50_work *work_data,
> +				   struct cs40l50_effect *effect)
> +{
> +	s16 bank_type = work_data->custom_data[0] & CS40L50_CUSTOM_DATA_MASK;
> +
> +	if (bank_type >= CS40L50_WVFRM_BANK_NUM) {
> +		dev_err(work_data->vibra->dev, "Invalid bank (%d)\n", bank_type);
> +		return -EINVAL;
> +	}
> +
> +	if (work_data->custom_len > CS40L50_OWT_CUSTOM_DATA_SIZE)
> +		effect->type = CS40L50_WVFRM_BANK_OWT;
> +	else
> +		effect->type = bank_type;
> +
> +	return 0;
> +}
> +
> +static int cs40l50_effect_gpio_mapping_set(struct cs40l50_work *work_data,
> +					   struct cs40l50_effect *effect)
> +{
> +	u16 gpio_num, gpio_edge, button = work_data->effect->trigger.button;
> +	struct cs40l50_vibra *vibra = work_data->vibra;
> +
> +	if (button) {
> +		gpio_num = FIELD_GET(CS40L50_GPIO_NUM_MASK, button);
> +		gpio_edge = FIELD_GET(CS40L50_GPIO_EDGE_MASK, button);
> +		effect->gpio_reg = vibra->dsp.gpio_base_reg + (gpio_num * 8) - gpio_edge;
> +
> +		return regmap_write(vibra->regmap, effect->gpio_reg, button);
> +	}
> +
> +	effect->gpio_reg = CS40L50_GPIO_MAPPING_NONE;
> +
> +	return 0;
> +}
> +
> +static int cs40l50_effect_index_set(struct cs40l50_work *work_data,
> +				   struct cs40l50_effect *effect)
> +{
> +	struct cs40l50_vibra *vibra = work_data->vibra;
> +	struct cs40l50_effect *owt_effect;
> +	u32 base_index, max_index;
> +
> +	base_index = vibra->dsp.banks[effect->type].base_index;
> +	max_index = vibra->dsp.banks[effect->type].max_index;
> +
> +	effect->index = base_index;
> +
> +	switch (effect->type) {
> +	case CS40L50_WVFRM_BANK_OWT:
> +		list_for_each_entry(owt_effect, &vibra->effect_head, list)
> +			if (owt_effect->type == CS40L50_WVFRM_BANK_OWT)
> +				effect->index++;
> +		break;
> +	case CS40L50_WVFRM_BANK_ROM:
> +	case CS40L50_WVFRM_BANK_RAM:
> +		effect->index += work_data->custom_data[1] & CS40L50_CUSTOM_DATA_MASK;
> +		break;
> +	default:
> +		dev_err(vibra->dev, "Bank type %d not supported\n", effect->type);
> +		return -EINVAL;
> +	}
> +
> +	if (effect->index > max_index || effect->index < base_index) {
> +		dev_err(vibra->dev, "Index out of bounds: %u\n", effect->index);
> +		return -ENOSPC;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Describes a header for an OWT effect */
> +struct cs40l50_owt_header {
> +	u32 type;
> +	u32 data_words;
> +	u32 offset;
> +} __packed;
> +
> +static int cs40l50_upload_owt(struct cs40l50_work *work_data)
> +{
> +	u32 len = 2 * work_data->custom_len, wt_offset, wt_size;
> +	struct cs40l50_vibra *vibra = work_data->vibra;
> +	struct cs40l50_owt_header header;
> +	u8 *out_data;
> +	int error;
> +
> +	error = regmap_read(vibra->regmap, vibra->dsp.owt_size_reg, &wt_size);
> +	if (error)
> +		return error;
> +
> +	if ((wt_size * sizeof(u32)) < sizeof(header) + len) {
> +		dev_err(vibra->dev, "No space in OWT bank for effect\n");
> +		return -ENOSPC;
> +	}
> +
> +	out_data = kzalloc(sizeof(header) + len, GFP_KERNEL);

You can make this as

	u8 *out_data __free(kfree) = kzalloc(...);

and then you do not need to explicitly kfree() it.

Also, I wonder if you have to zero it and can't use kmalloc() given that
you copy over the entire thing.

> +	if (!out_data)
> +		return -ENOMEM;
> +
> +	header.type = work_data->custom_data[0] == CS40L50_PCM_ID ? CS40L50_TYPE_PCM :
> +								    CS40L50_TYPE_PWLE;
> +	header.offset = sizeof(header) / sizeof(u32);
> +	header.data_words = len / sizeof(u32);
> +
> +	memcpy(out_data, &header, sizeof(header));
> +	memcpy(out_data + sizeof(header), work_data->custom_data, len);
> +
> +	error = regmap_read(vibra->regmap, vibra->dsp.owt_offset_reg, &wt_offset);
> +	if (error)
> +		goto err_free;
> +
> +	error = regmap_bulk_write(vibra->regmap, vibra->dsp.owt_base_reg +
> +				  (wt_offset * sizeof(u32)), out_data,
> +				  sizeof(header) + len);
> +	if (error)
> +		goto err_free;
> +
> +	error = vibra->dsp.write(vibra->dev, vibra->regmap, vibra->dsp.push_owt_cmd);
> +err_free:
> +	kfree(out_data);
> +
> +	return error;
> +}
> +
> +static void cs40l50_add_worker(struct work_struct *work)
> +{
> +	struct cs40l50_work *work_data = container_of(work, struct cs40l50_work, work);
> +	struct cs40l50_vibra *vibra = work_data->vibra;
> +	struct cs40l50_effect *effect;
> +	bool is_new = false;
> +	int error;
> +
> +	error = pm_runtime_resume_and_get(vibra->dev);
> +	if (error)
> +		goto err_exit;
> +
> +	mutex_lock(&vibra->lock);
> +
> +	/* Update effect if already present, otherwise create new effect */
> +	effect = cs40l50_find_effect(work_data->effect->id, &vibra->effect_head);
> +	if (!effect) {
> +		effect = kzalloc(sizeof(*effect), GFP_KERNEL);
> +		if (!effect) {
> +			error = -ENOMEM;
> +			goto err_mutex;
> +		}
> +
> +		effect->id = work_data->effect->id;
> +		is_new = true;
> +	}
> +
> +	error = cs40l50_effect_bank_set(work_data, effect);
> +	if (error)
> +		goto err_free;
> +
> +	error = cs40l50_effect_index_set(work_data, effect);
> +	if (error)
> +		goto err_free;
> +
> +	error = cs40l50_effect_gpio_mapping_set(work_data, effect);
> +	if (error)
> +		goto err_free;
> +
> +	if (effect->type == CS40L50_WVFRM_BANK_OWT)
> +		error = cs40l50_upload_owt(work_data);
> +err_free:
> +	if (is_new) {
> +		if (error)
> +			kfree(effect);
> +		else
> +			list_add(&effect->list, &vibra->effect_head);
> +	}
> +err_mutex:
> +	mutex_unlock(&vibra->lock);
> +
> +	pm_runtime_mark_last_busy(vibra->dev);
> +	pm_runtime_put_autosuspend(vibra->dev);
> +err_exit:
> +	work_data->error = error;
> +}
> +
> +static int cs40l50_add(struct input_dev *dev, struct ff_effect *effect,
> +		       struct ff_effect *old)
> +{
> +	struct ff_periodic_effect *periodic = &effect->u.periodic;
> +	struct cs40l50_vibra *vibra = input_get_drvdata(dev);
> +	u32 len = effect->u.periodic.custom_len;
> +	struct cs40l50_work work_data;
> +
> +	if (effect->type != FF_PERIODIC || periodic->waveform != FF_CUSTOM) {
> +		dev_err(vibra->dev, "Type (%#X) or waveform (%#X) unsupported\n",
> +			effect->type, periodic->waveform);
> +		return -EINVAL;
> +	}
> +
> +	work_data.custom_data = kcalloc(len, sizeof(s16), GFP_KERNEL);
> +	if (!work_data.custom_data)
> +		return -ENOMEM;
> +
> +	if (copy_from_user(work_data.custom_data, effect->u.periodic.custom_data,
> +			   sizeof(s16) * len)) {
> +		work_data.error = -EFAULT;
> +		goto err_free;
> +	}

	work_data.custom_data = memdup_array_user(effect->u.periodic.custom_data,
						  len, sizeof(s16));
	if (IS_ERR(work_data.custom_data))
		return PTR_ERR(work_data.custom_data);

> +
> +	work_data.custom_len = len;
> +	work_data.vibra = vibra;
> +	work_data.effect = effect;
> +	INIT_WORK(&work_data.work, cs40l50_add_worker);
> +
> +	/* Push to the workqueue to serialize with playbacks */
> +	queue_work(vibra->vibe_wq, &work_data.work);
> +	flush_work(&work_data.work);
> +err_free:
> +	kfree(work_data.custom_data);
> +
> +	return work_data.error;
> +}
> +
> +static void cs40l50_start_worker(struct work_struct *work)
> +{
> +	struct cs40l50_work *work_data = container_of(work, struct cs40l50_work, work);
> +	struct cs40l50_vibra *vibra = work_data->vibra;
> +	struct cs40l50_effect *start_effect;
> +
> +	if (pm_runtime_resume_and_get(vibra->dev) < 0)
> +		goto err_free;
> +
> +	mutex_lock(&vibra->lock);
> +
> +	start_effect = cs40l50_find_effect(work_data->effect->id, &vibra->effect_head);
> +	if (start_effect) {
> +		while (--work_data->count >= 0) {
> +			vibra->dsp.write(vibra->dev, vibra->regmap, start_effect->index);
> +			usleep_range(work_data->effect->replay.length,
> +				     work_data->effect->replay.length + 100);

If (I am reading this right you are spinning here playing the effect. It
would be much better if you tracked outstanding number of replays for an
effect and had a work re-scheduled that would play an instance.
Similarly to what code in ff-memless.c is doing.

> +		}
> +	}
> +
> +	mutex_unlock(&vibra->lock);
> +
> +	if (!start_effect)
> +		dev_err(vibra->dev, "Effect to play not found\n");
> +
> +	pm_runtime_mark_last_busy(vibra->dev);
> +	pm_runtime_put_autosuspend(vibra->dev);
> +err_free:
> +	kfree(work_data);
> +}
> +
> +static void cs40l50_stop_worker(struct work_struct *work)
> +{
> +	struct cs40l50_work *work_data = container_of(work, struct cs40l50_work, work);
> +	struct cs40l50_vibra *vibra = work_data->vibra;
> +
> +	if (pm_runtime_resume_and_get(vibra->dev) < 0)
> +		return;
> +
> +	mutex_lock(&vibra->lock);
> +
> +	vibra->dsp.write(vibra->dev, vibra->regmap, vibra->dsp.stop_cmd);
> +
> +	mutex_unlock(&vibra->lock);
> +
> +	pm_runtime_mark_last_busy(vibra->dev);
> +	pm_runtime_put_autosuspend(vibra->dev);
> +
> +	kfree(work_data);
> +}
> +
> +static int cs40l50_playback(struct input_dev *dev, int effect_id, int val)
> +{
> +	struct cs40l50_vibra *vibra = input_get_drvdata(dev);
> +	struct cs40l50_work *work_data;
> +
> +	work_data = kzalloc(sizeof(*work_data), GFP_ATOMIC);
> +	if (!work_data)
> +		return -ENOMEM;
> +
> +	work_data->vibra = vibra;
> +
> +	if (val > 0) {
> +		work_data->effect = &dev->ff->effects[effect_id];
> +		work_data->count = val;
> +		INIT_WORK(&work_data->work, cs40l50_start_worker);
> +	} else {
> +		/* Just stop the amplifier as device drives only one effect */
> +		INIT_WORK(&work_data->work, cs40l50_stop_worker);
> +	}
> +
> +	queue_work(vibra->vibe_wq, &work_data->work);
> +
> +	return 0;
> +}
> +
> +static void cs40l50_erase_worker(struct work_struct *work)
> +{
> +	struct cs40l50_work *work_data = container_of(work, struct cs40l50_work, work);
> +	struct cs40l50_effect *owt_effect, *erase_effect;
> +	struct cs40l50_vibra *vibra = work_data->vibra;
> +	int error;
> +
> +	error = pm_runtime_resume_and_get(vibra->dev);
> +	if (error)
> +		goto err_exit;
> +
> +	mutex_lock(&vibra->lock);
> +
> +	erase_effect = cs40l50_find_effect(work_data->effect->id, &vibra->effect_head);
> +	if (!erase_effect) {
> +		dev_err(vibra->dev, "Effect to erase not found\n");
> +		error = -EINVAL;
> +		goto err_mutex;
> +	}
> +
> +	if (erase_effect->gpio_reg != CS40L50_GPIO_MAPPING_NONE) {
> +		error = regmap_write(vibra->regmap, erase_effect->gpio_reg,
> +				     CS40L50_GPIO_DISABLE);
> +		if (error)
> +			goto err_mutex;
> +	}
> +
> +	if (erase_effect->type == CS40L50_WVFRM_BANK_OWT) {
> +		error = vibra->dsp.write(vibra->dev, vibra->regmap,
> +					 vibra->dsp.delete_owt_cmd |
> +					 erase_effect->index);
> +		if (error)
> +			goto err_mutex;
> +
> +		list_for_each_entry(owt_effect, &vibra->effect_head, list)
> +			if (owt_effect->type == CS40L50_WVFRM_BANK_OWT &&
> +			    owt_effect->index > erase_effect->index)
> +				owt_effect->index--;
> +	}
> +
> +	list_del(&erase_effect->list);
> +	kfree(erase_effect);
> +err_mutex:
> +	mutex_unlock(&vibra->lock);
> +
> +	pm_runtime_mark_last_busy(vibra->dev);
> +	pm_runtime_put_autosuspend(vibra->dev);
> +err_exit:
> +	work_data->error = error;
> +}
> +
> +static int cs40l50_erase(struct input_dev *dev, int effect_id)
> +{
> +	struct cs40l50_vibra *vibra = input_get_drvdata(dev);
> +	struct cs40l50_work work_data;
> +
> +	work_data.vibra = vibra;
> +	work_data.effect = &dev->ff->effects[effect_id];
> +
> +	INIT_WORK(&work_data.work, cs40l50_erase_worker);
> +
> +	/* Push to workqueue to serialize with playbacks */
> +	queue_work(vibra->vibe_wq, &work_data.work);
> +	flush_work(&work_data.work);

You already take the lock when you play, upload or erase an effect. Why
do we need additional single-thread workqueue for this? Why do we need
additional ordering and synchronization?

> +
> +	return work_data.error;
> +}
> +
> +static void cs40l50_remove_wq(void *data)
> +{
> +	flush_workqueue(data);
> +	destroy_workqueue(data);
> +}
> +
> +static int cs40l50_vibra_probe(struct platform_device *pdev)
> +{
> +	struct cs40l50 *cs40l50 = dev_get_drvdata(pdev->dev.parent);
> +	struct cs40l50_vibra *vibra;
> +	int error;
> +
> +	vibra = devm_kzalloc(pdev->dev.parent, sizeof(*vibra), GFP_KERNEL);
> +	if (!vibra)
> +		return -ENOMEM;
> +
> +	mutex_init(&vibra->lock);
> +
> +	vibra->dev = cs40l50->dev;
> +	vibra->regmap = cs40l50->regmap;
> +	vibra->dsp = cs40l50_dsp;
> +
> +	vibra->input = devm_input_allocate_device(vibra->dev);
> +	if (!vibra->input)
> +		return -ENOMEM;
> +
> +	vibra->input->id.product = cs40l50->devid;
> +	vibra->input->id.version = cs40l50->revid;
> +	vibra->input->name = "cs40l50_vibra";
> +
> +	input_set_drvdata(vibra->input, vibra);
> +	input_set_capability(vibra->input, EV_FF, FF_PERIODIC);
> +	input_set_capability(vibra->input, EV_FF, FF_CUSTOM);
> +
> +	error = input_ff_create(vibra->input, FF_MAX_EFFECTS);
> +	if (error) {
> +		dev_err(vibra->dev, "Failed to create input device\n");
> +		return error;
> +	}
> +
> +	vibra->input->ff->upload = cs40l50_add;
> +	vibra->input->ff->playback = cs40l50_playback;
> +	vibra->input->ff->erase = cs40l50_erase;
> +
> +	INIT_LIST_HEAD(&vibra->effect_head);
> +
> +	vibra->vibe_wq = alloc_ordered_workqueue("vibe_wq", WQ_HIGHPRI);
> +	if (!vibra->vibe_wq)
> +		return -ENOMEM;
> +
> +	error = devm_add_action_or_reset(vibra->dev, cs40l50_remove_wq, vibra->vibe_wq);
> +	if (error)
> +		return error;
> +
> +	return input_register_device(vibra->input);

Please no this kind of short hands when there are multiple exists from a
function.

	error = input_register_device(vibra->input);
	if (error)
		return error;

	return 0;

This way it is much easier to move the code around when needed.
	

> +}
> +
> +static const struct platform_device_id cs40l50_vibra_id_match[] = {
> +	{ "cs40l50-vibra", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(platform, cs40l50_vibra_id_match);
> +
> +static struct platform_driver cs40l50_vibra_driver = {
> +	.probe		= cs40l50_vibra_probe,
> +	.id_table	= cs40l50_vibra_id_match,
> +	.driver		= {
> +		.name	= "cs40l50-vibra",
> +	},
> +};
> +module_platform_driver(cs40l50_vibra_driver);
> +
> +MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver");
> +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.25.1
> 

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND v10 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
  2024-04-16 23:28   ` Dmitry Torokhov
@ 2024-04-19 18:26     ` James Ogletree
  2024-05-03 15:25       ` James Ogletree
  0 siblings, 1 reply; 16+ messages in thread
From: James Ogletree @ 2024-04-19 18:26 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: James Ogletree, robh+dt@kernel.org, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Mark Brown, Jeff LaBundy,
	open list:CIRRUS LOGIC HAPTIC DRIVERS,
	open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
	devicetree@vger.kernel.org


Hi Dmitry,

> On Apr 16, 2024, at 6:28 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> On Mon, Apr 08, 2024 at 03:32:13PM +0000, James Ogletree wrote:
>> Introduce support for Cirrus Logic Device CS40L50: a
>> haptic driver with waveform memory, integrated DSP,
>> and closed-loop algorithms.
>> 
>> The input driver provides the interface for control of
>> haptic effects through the device.
>> 
>> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
>> ---
>> v10:
>> Minor concern that playback stop is still misused with respect to not
>> specifying an effect ID. The device can only play one effect at a
>> time, but setting max effects for the EVIOCGEFFECTS ioctl to 1 would
>> restrict the number of uploads to 1 as well.
> 
> Sorry for a long delay on my part.

Not to worry at all. I am very grateful for the thorough review.

> 
> Unfortunately this is not a minor concern, because it breaks the API
> that we so far been presenting to userspace. EVIOCGEFFECTS ioctl which returns
> input->ff->max_effects is described as "Report number of effects
> playable at the same time".
> 
> I suggest you limit number of effects to 1 so that we can merge the
> driver with such limitation, and then try to figure out how to expand
> the API. We will probably have to split the notion of number of playable
> effects vs number of uploadable effects, and only accept uploads of
> effects that exceed number of playable effects if they all have the same
> owner, so that different processes would not be able to affect each
> other in case when number of simultaneously playable effects is smaller.

I understand. I will do just that for this submission.

>> +/* Describes an area in DSP memory populated by effects */
>> +struct cs40l50_bank {
>> + enum cs40l50_bank_type type;
>> + u32 base_index;
>> + u32 max_index;
> 
> This looks like it is written to the device, so I think this needs
> proper endianness annotation. Is there any concern about padding between
> the type and base_index?

The structure as a whole is not written, so there is no concern about padding.
Only base_index is written, and the endianness conversion is handled by regmap.
“cs40l50_owt_header” would be an example of a struct which is written, and there
care is taken for padding, and there also regmap handles the endianness
conversion.

>> +static int cs40l50_upload_owt(struct cs40l50_work *work_data)
>> +{
>> + u32 len = 2 * work_data->custom_len, wt_offset, wt_size;
>> + struct cs40l50_vibra *vibra = work_data->vibra;
>> + struct cs40l50_owt_header header;
>> + u8 *out_data;
>> + int error;
>> +
>> + error = regmap_read(vibra->regmap, vibra->dsp.owt_size_reg, &wt_size);
>> + if (error)
>> + return error;
>> +
>> + if ((wt_size * sizeof(u32)) < sizeof(header) + len) {
>> + dev_err(vibra->dev, "No space in OWT bank for effect\n");
>> + return -ENOSPC;
>> + }
>> +
>> + out_data = kzalloc(sizeof(header) + len, GFP_KERNEL);
> 
> You can make this as
> 
> u8 *out_data __free(kfree) = kzalloc(...);
> 
> and then you do not need to explicitly kfree() it.
> 
> Also, I wonder if you have to zero it and can't use kmalloc() given that
> you copy over the entire thing.

Ack. I will declare it as:
u8 *out_data __free(kfree) = NULL;

And then initialize it in the same place with kmalloc().
This will allow me to keep reverse Christmas tree style.

>> +static int cs40l50_add(struct input_dev *dev, struct ff_effect *effect,
>> +        struct ff_effect *old)
>> +{
>> + struct ff_periodic_effect *periodic = &effect->u.periodic;
>> + struct cs40l50_vibra *vibra = input_get_drvdata(dev);
>> + u32 len = effect->u.periodic.custom_len;
>> + struct cs40l50_work work_data;
>> +
>> + if (effect->type != FF_PERIODIC || periodic->waveform != FF_CUSTOM) {
>> + dev_err(vibra->dev, "Type (%#X) or waveform (%#X) unsupported\n",
>> + effect->type, periodic->waveform);
>> + return -EINVAL;
>> + }
>> +
>> + work_data.custom_data = kcalloc(len, sizeof(s16), GFP_KERNEL);
>> + if (!work_data.custom_data)
>> + return -ENOMEM;
>> +
>> + if (copy_from_user(work_data.custom_data, effect->u.periodic.custom_data,
>> +    sizeof(s16) * len)) {
>> + work_data.error = -EFAULT;
>> + goto err_free;
>> + }
> 
> work_data.custom_data = memdup_array_user(effect->u.periodic.custom_data,
>   len, sizeof(s16));
> if (IS_ERR(work_data.custom_data))
> return PTR_ERR(work_data.custom_data);

Ack.

>> +static void cs40l50_start_worker(struct work_struct *work)
>> +{
>> + struct cs40l50_work *work_data = container_of(work, struct cs40l50_work, work);
>> + struct cs40l50_vibra *vibra = work_data->vibra;
>> + struct cs40l50_effect *start_effect;
>> +
>> + if (pm_runtime_resume_and_get(vibra->dev) < 0)
>> + goto err_free;
>> +
>> + mutex_lock(&vibra->lock);
>> +
>> + start_effect = cs40l50_find_effect(work_data->effect->id, &vibra->effect_head);
>> + if (start_effect) {
>> + while (--work_data->count >= 0) {
>> + vibra->dsp.write(vibra->dev, vibra->regmap, start_effect->index);
>> + usleep_range(work_data->effect->replay.length,
>> +      work_data->effect->replay.length + 100);
> 
> If (I am reading this right you are spinning here playing the effect. It
> would be much better if you tracked outstanding number of replays for an
> effect and had a work re-scheduled that would play an instance.
> Similarly to what code in ff-memless.c is doing.

I think you are reading it right.

My concern with the ff-memless.c method is that it seems to risk out of order
executions when adopted to our driver, because of the use of the workqueue.

If a playback is issued with a high enough repeat value, it seems very plausible
that another operation, e.g. erase, could arrive in the middle of the chain of
re-scheduling and disrupt the playbacks which have yet to be queued. Currently,
the driver handles all repeats in the same work item, so it is guaranteed to repeat
the effect for the specified number of times without being interrupted.

Please let me know what you think.


>> +static int cs40l50_erase(struct input_dev *dev, int effect_id)
>> +{
>> + struct cs40l50_vibra *vibra = input_get_drvdata(dev);
>> + struct cs40l50_work work_data;
>> +
>> + work_data.vibra = vibra;
>> + work_data.effect = &dev->ff->effects[effect_id];
>> +
>> + INIT_WORK(&work_data.work, cs40l50_erase_worker);
>> +
>> + /* Push to workqueue to serialize with playbacks */
>> + queue_work(vibra->vibe_wq, &work_data.work);
>> + flush_work(&work_data.work);
> 
> You already take the lock when you play, upload or erase an effect. Why
> do we need additional single-thread workqueue for this? Why do we need
> additional ordering and synchronization?

I think there is unnecessary serialization here, but I think it is the mutexes
which are excessive.

Without the workqueue I think there will be out of order operations. The only
reason the workqueue is necessary is because playback blocks (via
pm_runtime_resume_and_get), and so playback must defer work.

Now upload and erase are not called in atomic context. But without queueing
those executions, they could cut in front of any playbacks waiting in the
workqueue. This is the meaning of the comment "Push to workqueue to
serialize with playbacks”.

But if they are already serialized in this way, then I don’t see the need
for the locks. That’s my thinking, let me know if it is sound.

>> + return input_register_device(vibra->input);
> 
> Please no this kind of short hands when there are multiple exists from a
> function.
> 
> error = input_register_device(vibra->input);
> if (error)
> return error;
> 
> return 0;
> 
> This way it is much easier to move the code around when needed.

Ack.

Best,
James



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND v10 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
  2024-04-19 18:26     ` James Ogletree
@ 2024-05-03 15:25       ` James Ogletree
  2024-05-20 15:47         ` James Ogletree
  0 siblings, 1 reply; 16+ messages in thread
From: James Ogletree @ 2024-05-03 15:25 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: James Ogletree, robh+dt@kernel.org, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Mark Brown, Jeff LaBundy,
	open list:CIRRUS LOGIC HAPTIC DRIVERS,
	open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
	devicetree@vger.kernel.org

Hi Dmitry,

Trimming down my last reply to just the points that need your attention/ack.

I made some small edits for the sake of clarity.

> On Apr 16, 2024, at 6:28 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> On Mon, Apr 08, 2024 at 03:32:13PM +0000, James Ogletree wrote:
>> 
>> +/* Describes an area in DSP memory populated by effects */
>> +struct cs40l50_bank {
>> + enum cs40l50_bank_type type;
>> + u32 base_index;
>> + u32 max_index;
> 
> This looks like it is written to the device, so I think this needs
> proper endianness annotation. Is there any concern about padding between
> the type and base_index?

The structure itself is never written, so there is no concern about padding.
Only base_index is written, and the endianness conversion is handled by regmap.

>> +static void cs40l50_start_worker(struct work_struct *work)
>> +{
>> + struct cs40l50_work *work_data = container_of(work, struct cs40l50_work, work);
>> + struct cs40l50_vibra *vibra = work_data->vibra;
>> + struct cs40l50_effect *start_effect;
>> +
>> + if (pm_runtime_resume_and_get(vibra->dev) < 0)
>> + goto err_free;
>> +
>> + mutex_lock(&vibra->lock);
>> +
>> + start_effect = cs40l50_find_effect(work_data->effect->id, &vibra->effect_head);
>> + if (start_effect) {
>> + while (--work_data->count >= 0) {
>> + vibra->dsp.write(vibra->dev, vibra->regmap, start_effect->index);
>> + usleep_range(work_data->effect->replay.length,
>> +      work_data->effect->replay.length + 100);
> 
> If (I am reading this right you are spinning here playing the effect. It
> would be much better if you tracked outstanding number of replays for an
> effect and had a work re-scheduled that would play an instance.
> Similarly to what code in ff-memless.c is doing.

Yes, you are reading it right.

It appears that ff-memless.c handles repeats atomically, however for
reasons explained below, this driver must queue work for playback
executions.

This results in a possible scenario where if a playback is issued with a
high enough repeat value, an erase operation could arrive in the middle of
the chain of re-scheduling and disrupt the playbacks which have yet to be
queued. But with the current approach, the effect is guaranteed to repeat
any number of times without risk of being erased in the middle.

That’s my concern with adopting the re-scheduling approach for this
driver. Please let me know your thoughts.

>> +static int cs40l50_erase(struct input_dev *dev, int effect_id)
>> +{
>> + struct cs40l50_vibra *vibra = input_get_drvdata(dev);
>> + struct cs40l50_work work_data;
>> +
>> + work_data.vibra = vibra;
>> + work_data.effect = &dev->ff->effects[effect_id];
>> +
>> + INIT_WORK(&work_data.work, cs40l50_erase_worker);
>> +
>> + /* Push to workqueue to serialize with playbacks */
>> + queue_work(vibra->vibe_wq, &work_data.work);
>> + flush_work(&work_data.work);
> 
> You already take the lock when you play, upload or erase an effect. Why
> do we need additional single-thread workqueue for this? Why do we need
> additional ordering and synchronization?

The workqueue is necessary is because playback blocks (via
pm_runtime_resume_and_get), and so playback must defer work. Upload
and erase are not called in atomic context, but without queueing those
executions, they could cut in front of playbacks waiting in the
workqueue.

But as the callbacks are already serialized via the workqueue, then I do
think the locks are excessive. That’s my thinking, let me know if it is
sound.

Best,
James

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND v10 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
  2024-05-03 15:25       ` James Ogletree
@ 2024-05-20 15:47         ` James Ogletree
  0 siblings, 0 replies; 16+ messages in thread
From: James Ogletree @ 2024-05-20 15:47 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: James Ogletree, robh+dt@kernel.org, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Mark Brown, Jeff LaBundy,
	open list:CIRRUS LOGIC HAPTIC DRIVERS,
	open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
	devicetree@vger.kernel.org

Hi Dmitry,

Reaching out to get closure on these few comments before I send up the
next version.


> On May 3, 2024, at 10:25 AM, James Ogletree <James.Ogletree@cirrus.com> wrote:
> 
> Hi Dmitry,
> 
> Trimming down my last reply to just the points that need your attention/ack.
> 
> I made some small edits for the sake of clarity.
> 
>> On Apr 16, 2024, at 6:28 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>> 
>> On Mon, Apr 08, 2024 at 03:32:13PM +0000, James Ogletree wrote:
>>> 
>>> +/* Describes an area in DSP memory populated by effects */
>>> +struct cs40l50_bank {
>>> + enum cs40l50_bank_type type;
>>> + u32 base_index;
>>> + u32 max_index;
>> 
>> This looks like it is written to the device, so I think this needs
>> proper endianness annotation. Is there any concern about padding between
>> the type and base_index?
> 
> The structure itself is never written, so there is no concern about padding.
> Only base_index is written, and the endianness conversion is handled by regmap.
> 
>>> +static void cs40l50_start_worker(struct work_struct *work)
>>> +{
>>> + struct cs40l50_work *work_data = container_of(work, struct cs40l50_work, work);
>>> + struct cs40l50_vibra *vibra = work_data->vibra;
>>> + struct cs40l50_effect *start_effect;
>>> +
>>> + if (pm_runtime_resume_and_get(vibra->dev) < 0)
>>> + goto err_free;
>>> +
>>> + mutex_lock(&vibra->lock);
>>> +
>>> + start_effect = cs40l50_find_effect(work_data->effect->id, &vibra->effect_head);
>>> + if (start_effect) {
>>> + while (--work_data->count >= 0) {
>>> + vibra->dsp.write(vibra->dev, vibra->regmap, start_effect->index);
>>> + usleep_range(work_data->effect->replay.length,
>>> +      work_data->effect->replay.length + 100);
>> 
>> If (I am reading this right you are spinning here playing the effect. It
>> would be much better if you tracked outstanding number of replays for an
>> effect and had a work re-scheduled that would play an instance.
>> Similarly to what code in ff-memless.c is doing.
> 
> Yes, you are reading it right.
> 
> It appears that ff-memless.c handles repeats atomically, however for
> reasons explained below, this driver must queue work for playback
> executions.
> 
> This results in a possible scenario where if a playback is issued with a
> high enough repeat value, an erase operation could arrive in the middle of
> the chain of re-scheduling and disrupt the playbacks which have yet to be
> queued. But with the current approach, the effect is guaranteed to repeat
> any number of times without risk of being erased in the middle.
> 
> That’s my concern with adopting the re-scheduling approach for this
> driver. Please let me know your thoughts.
> 
>>> +static int cs40l50_erase(struct input_dev *dev, int effect_id)
>>> +{
>>> + struct cs40l50_vibra *vibra = input_get_drvdata(dev);
>>> + struct cs40l50_work work_data;
>>> +
>>> + work_data.vibra = vibra;
>>> + work_data.effect = &dev->ff->effects[effect_id];
>>> +
>>> + INIT_WORK(&work_data.work, cs40l50_erase_worker);
>>> +
>>> + /* Push to workqueue to serialize with playbacks */
>>> + queue_work(vibra->vibe_wq, &work_data.work);
>>> + flush_work(&work_data.work);
>> 
>> You already take the lock when you play, upload or erase an effect. Why
>> do we need additional single-thread workqueue for this? Why do we need
>> additional ordering and synchronization?
> 
> The workqueue is necessary is because playback blocks (via
> pm_runtime_resume_and_get), and so playback must defer work. Upload
> and erase are not called in atomic context, but without queueing those
> executions, they could cut in front of playbacks waiting in the
> workqueue.
> 
> But as the callbacks are already serialized via the workqueue, then I do
> think the locks are excessive. That’s my thinking, let me know if it is
> sound.
> 
> Best,
> James



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND v10 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50
  2024-04-08 15:32 ` [PATCH RESEND v10 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50 James Ogletree
@ 2024-05-29 19:24   ` James Ogletree
  2024-05-29 19:28     ` Mark Brown
  2024-05-29 22:03     ` James Ogletree
  2024-05-30 17:07   ` Rivera-Matos, Ricardo
  1 sibling, 2 replies; 16+ messages in thread
From: James Ogletree @ 2024-05-29 19:24 UTC (permalink / raw)
  To: James Ogletree, Mark Brown
  Cc: Dmitry Torokhov, robh+dt@kernel.org, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Jeff LaBundy,
	open list:CIRRUS LOGIC HAPTIC DRIVERS,
	open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	David Rhodes

HI Mark,

This file has not had any feedback yet for this version. For the sake of efficiency, I
would appreciate your feedback. That way I can make any needed changes
in the already-planned v11.

Best,
James


> On Apr 8, 2024, at 10:32 AM, James Ogletree <jogletre@opensource.cirrus.com> wrote:
> 
> Introduce support for Cirrus Logic Device CS40L50: a
> haptic driver with waveform memory, integrated DSP,
> and closed-loop algorithms.
> 
> The ASoC driver enables I2S streaming to the device.
> 
> Reviewed-by: David Rhodes <drhodes@opensource.cirrus.com>
> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
> ---
> MAINTAINERS                      |   1 +
> sound/soc/codecs/Kconfig         |  11 ++
> sound/soc/codecs/Makefile        |   2 +
> sound/soc/codecs/cs40l50-codec.c | 308 +++++++++++++++++++++++++++++++
> 4 files changed, 322 insertions(+)
> create mode 100644 sound/soc/codecs/cs40l50-codec.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 49c2e6e57b09..62701b13f741 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4943,6 +4943,7 @@ F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
> F: drivers/input/misc/cs40l*
> F: drivers/mfd/cs40l*
> F: include/linux/mfd/cs40l*
> +F: sound/soc/codecs/cs40l*
> 
> CIRRUS LOGIC DSP FIRMWARE DRIVER
> M: Simon Trimmer <simont@opensource.cirrus.com>
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index f1e1dbc509f6..1a81bedfdbe3 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -73,6 +73,7 @@ config SND_SOC_ALL_CODECS
> imply SND_SOC_CS35L56_I2C
> imply SND_SOC_CS35L56_SPI
> imply SND_SOC_CS35L56_SDW
> + imply SND_SOC_CS40L50
> imply SND_SOC_CS42L42
> imply SND_SOC_CS42L42_SDW
> imply SND_SOC_CS42L43
> @@ -800,6 +801,16 @@ config SND_SOC_CS35L56_SDW
> help
>  Enable support for Cirrus Logic CS35L56 boosted amplifier with SoundWire control
> 
> +config SND_SOC_CS40L50
> + tristate "Cirrus Logic CS40L50 CODEC"
> + depends on MFD_CS40L50_CORE
> + help
> +  This option enables support for I2S streaming to Cirrus Logic CS40L50.
> +
> +  CS40L50 is a haptic driver with waveform memory, an integrated
> +  DSP, and closed-loop algorithms. If built as a module, it will be
> +  called snd-soc-cs40l50.
> +
> config SND_SOC_CS42L42_CORE
> tristate
> 
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index a87e56938ce5..7e31f000774a 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -74,6 +74,7 @@ snd-soc-cs35l56-shared-objs := cs35l56-shared.o
> snd-soc-cs35l56-i2c-objs := cs35l56-i2c.o
> snd-soc-cs35l56-spi-objs := cs35l56-spi.o
> snd-soc-cs35l56-sdw-objs := cs35l56-sdw.o
> +snd-soc-cs40l50-objs := cs40l50-codec.o
> snd-soc-cs42l42-objs := cs42l42.o
> snd-soc-cs42l42-i2c-objs := cs42l42-i2c.o
> snd-soc-cs42l42-sdw-objs := cs42l42-sdw.o
> @@ -460,6 +461,7 @@ obj-$(CONFIG_SND_SOC_CS35L56_SHARED) += snd-soc-cs35l56-shared.o
> obj-$(CONFIG_SND_SOC_CS35L56_I2C) += snd-soc-cs35l56-i2c.o
> obj-$(CONFIG_SND_SOC_CS35L56_SPI) += snd-soc-cs35l56-spi.o
> obj-$(CONFIG_SND_SOC_CS35L56_SDW) += snd-soc-cs35l56-sdw.o
> +obj-$(CONFIG_SND_SOC_CS40L50) += snd-soc-cs40l50.o
> obj-$(CONFIG_SND_SOC_CS42L42_CORE) += snd-soc-cs42l42.o
> obj-$(CONFIG_SND_SOC_CS42L42) += snd-soc-cs42l42-i2c.o
> obj-$(CONFIG_SND_SOC_CS42L42_SDW) += snd-soc-cs42l42-sdw.o
> diff --git a/sound/soc/codecs/cs40l50-codec.c b/sound/soc/codecs/cs40l50-codec.c
> new file mode 100644
> index 000000000000..6d4a0970b219
> --- /dev/null
> +++ b/sound/soc/codecs/cs40l50-codec.c
> @@ -0,0 +1,308 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// CS40L50 Advanced Haptic Driver with waveform memory,
> +// integrated DSP, and closed-loop algorithms
> +//
> +// Copyright 2024 Cirrus Logic, Inc.
> +//
> +// Author: James Ogletree <james.ogletree@cirrus.com>
> +
> +#include <linux/bitfield.h>
> +#include <linux/mfd/cs40l50.h>
> +#include <linux/pm_runtime.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +
> +#define CS40L50_REFCLK_INPUT 0x2C04
> +#define CS40L50_ASP_CONTROL2 0x4808
> +#define CS40L50_ASP_DATA_CONTROL5 0x4840
> +
> +/* PLL Config */
> +#define CS40L50_PLL_REFCLK_BCLK 0x0
> +#define CS40L50_PLL_REFCLK_MCLK 0x5
> +#define CS40L50_PLL_REEFCLK_MCLK_CFG 0x00
> +#define CS40L50_PLL_REFCLK_LOOP_MASK BIT(11)
> +#define CS40L50_PLL_REFCLK_OPEN_LOOP 1
> +#define CS40L50_PLL_REFCLK_CLOSED_LOOP 0
> +#define CS40L50_PLL_REFCLK_LOOP_SHIFT 11
> +#define CS40L50_PLL_REFCLK_FREQ_MASK GENMASK(10, 5)
> +#define CS40L50_PLL_REFCLK_FREQ_SHIFT 5
> +#define CS40L50_PLL_REFCLK_SEL_MASK GENMASK(2, 0)
> +#define CS40L50_BCLK_RATIO_DEFAULT 32
> +
> +/* ASP Config */
> +#define CS40L50_ASP_RX_WIDTH_SHIFT 24
> +#define CS40L50_ASP_RX_WIDTH_MASK GENMASK(31, 24)
> +#define CS40L50_ASP_RX_WL_MASK GENMASK(5, 0)
> +#define CS40L50_ASP_FSYNC_INV_MASK BIT(2)
> +#define CS40L50_ASP_BCLK_INV_MASK BIT(6)
> +#define CS40L50_ASP_FMT_MASK GENMASK(10, 8)
> +#define CS40L50_ASP_FMT_I2S 0x2
> +
> +struct cs40l50_pll_config {
> + unsigned int freq;
> + unsigned int cfg;
> +};
> +
> +struct cs40l50_codec {
> + struct device *dev;
> + struct regmap *regmap;
> + unsigned int daifmt;
> + unsigned int bclk_ratio;
> + unsigned int rate;
> +};
> +
> +static const struct cs40l50_pll_config cs40l50_pll_cfg[] = {
> + { 32768, 0x00 },
> + { 1536000, 0x1B },
> + { 3072000, 0x21 },
> + { 6144000, 0x28 },
> + { 9600000, 0x30 },
> + { 12288000, 0x33 },
> +};
> +
> +static int cs40l50_get_clk_config(unsigned int freq, unsigned int *cfg)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(cs40l50_pll_cfg); i++) {
> + if (cs40l50_pll_cfg[i].freq == freq) {
> + *cfg = cs40l50_pll_cfg[i].cfg;
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int cs40l50_swap_ext_clk(struct cs40l50_codec *codec, unsigned int clk_src)
> +{
> + unsigned int cfg;
> + int ret;
> +
> + switch (clk_src) {
> + case CS40L50_PLL_REFCLK_BCLK:
> + ret = cs40l50_get_clk_config(codec->bclk_ratio * codec->rate, &cfg);
> + if (ret)
> + return ret;
> + break;
> + case CS40L50_PLL_REFCLK_MCLK:
> + cfg = CS40L50_PLL_REEFCLK_MCLK_CFG;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
> + CS40L50_PLL_REFCLK_LOOP_MASK,
> + CS40L50_PLL_REFCLK_OPEN_LOOP <<
> + CS40L50_PLL_REFCLK_LOOP_SHIFT);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
> + CS40L50_PLL_REFCLK_FREQ_MASK |
> + CS40L50_PLL_REFCLK_SEL_MASK,
> + (cfg << CS40L50_PLL_REFCLK_FREQ_SHIFT) | clk_src);
> + if (ret)
> + return ret;
> +
> + return regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
> +  CS40L50_PLL_REFCLK_LOOP_MASK,
> +  CS40L50_PLL_REFCLK_CLOSED_LOOP <<
> +  CS40L50_PLL_REFCLK_LOOP_SHIFT);
> +}
> +
> +static int cs40l50_clk_en(struct snd_soc_dapm_widget *w,
> +  struct snd_kcontrol *kcontrol,
> +  int event)
> +{
> + struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(comp);
> + int ret;
> +
> + switch (event) {
> + case SND_SOC_DAPM_POST_PMU:
> + ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_STOP_PLAYBACK);
> + if (ret)
> + return ret;
> +
> + ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_START_I2S);
> + if (ret)
> + return ret;
> +
> + ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_BCLK);
> + if (ret)
> + return ret;
> + break;
> + case SND_SOC_DAPM_PRE_PMD:
> + ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_MCLK);
> + if (ret)
> + return ret;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_dapm_widget cs40l50_dapm_widgets[] = {
> + SND_SOC_DAPM_SUPPLY_S("ASP PLL", 0, SND_SOC_NOPM, 0, 0, cs40l50_clk_en,
> +      SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
> + SND_SOC_DAPM_AIF_IN("ASPRX1", NULL, 0, SND_SOC_NOPM, 0, 0),
> + SND_SOC_DAPM_AIF_IN("ASPRX2", NULL, 0, SND_SOC_NOPM, 0, 0),
> + SND_SOC_DAPM_OUTPUT("OUT"),
> +};
> +
> +static const struct snd_soc_dapm_route cs40l50_dapm_routes[] = {
> + { "ASP Playback", NULL, "ASP PLL" },
> + { "ASPRX1", NULL, "ASP Playback" },
> + { "ASPRX2", NULL, "ASP Playback" },
> +
> + { "OUT", NULL, "ASPRX1" },
> + { "OUT", NULL, "ASPRX2" },
> +};
> +
> +static int cs40l50_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
> +{
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(codec_dai->component);
> +
> + if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBC_CFC)
> + return -EINVAL;
> +
> + switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> + case SND_SOC_DAIFMT_NB_NF:
> + codec->daifmt = 0;
> + break;
> + case SND_SOC_DAIFMT_NB_IF:
> + codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK;
> + break;
> + case SND_SOC_DAIFMT_IB_NF:
> + codec->daifmt = CS40L50_ASP_BCLK_INV_MASK;
> + break;
> + case SND_SOC_DAIFMT_IB_IF:
> + codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK | CS40L50_ASP_BCLK_INV_MASK;
> + break;
> + default:
> + dev_err(codec->dev, "Invalid clock invert\n");
> + return -EINVAL;
> + }
> +
> + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> + case SND_SOC_DAIFMT_I2S:
> + codec->daifmt |= FIELD_PREP(CS40L50_ASP_FMT_MASK, CS40L50_ASP_FMT_I2S);
> + break;
> + default:
> + dev_err(codec->dev, "Unsupported DAI format\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int cs40l50_hw_params(struct snd_pcm_substream *substream,
> +     struct snd_pcm_hw_params *params,
> +     struct snd_soc_dai *dai)
> +{
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component);
> + unsigned int asp_rx_wl = params_width(params);
> + int ret;
> +
> + codec->rate = params_rate(params);
> +
> + ret = regmap_update_bits(codec->regmap, CS40L50_ASP_DATA_CONTROL5,
> + CS40L50_ASP_RX_WL_MASK, asp_rx_wl);
> + if (ret)
> + return ret;
> +
> + codec->daifmt |= (asp_rx_wl << CS40L50_ASP_RX_WIDTH_SHIFT);
> +
> + return regmap_update_bits(codec->regmap, CS40L50_ASP_CONTROL2,
> +  CS40L50_ASP_FSYNC_INV_MASK |
> +  CS40L50_ASP_BCLK_INV_MASK |
> +  CS40L50_ASP_FMT_MASK |
> +  CS40L50_ASP_RX_WIDTH_MASK, codec->daifmt);
> +}
> +
> +static int cs40l50_set_dai_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio)
> +{
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component);
> +
> + codec->bclk_ratio = ratio;
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_dai_ops cs40l50_dai_ops = {
> + .set_fmt = cs40l50_set_dai_fmt,
> + .set_bclk_ratio = cs40l50_set_dai_bclk_ratio,
> + .hw_params = cs40l50_hw_params,
> +};
> +
> +static struct snd_soc_dai_driver cs40l50_dai[] = {
> + {
> + .name = "cs40l50-pcm",
> + .id = 0,
> + .playback = {
> + .stream_name = "ASP Playback",
> + .channels_min = 1,
> + .channels_max = 2,
> + .rates = SNDRV_PCM_RATE_48000,
> + .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
> + },
> + .ops = &cs40l50_dai_ops,
> + },
> +};
> +
> +static int cs40l50_codec_probe(struct snd_soc_component *component)
> +{
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(component);
> +
> + codec->bclk_ratio = CS40L50_BCLK_RATIO_DEFAULT;
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_component_driver soc_codec_dev_cs40l50 = {
> + .probe = cs40l50_codec_probe,
> + .dapm_widgets = cs40l50_dapm_widgets,
> + .num_dapm_widgets = ARRAY_SIZE(cs40l50_dapm_widgets),
> + .dapm_routes = cs40l50_dapm_routes,
> + .num_dapm_routes = ARRAY_SIZE(cs40l50_dapm_routes),
> +};
> +
> +static int cs40l50_codec_driver_probe(struct platform_device *pdev)
> +{
> + struct cs40l50 *cs40l50 = dev_get_drvdata(pdev->dev.parent);
> + struct cs40l50_codec *codec;
> +
> + codec = devm_kzalloc(&pdev->dev, sizeof(*codec), GFP_KERNEL);
> + if (!codec)
> + return -ENOMEM;
> +
> + codec->regmap = cs40l50->regmap;
> + codec->dev = &pdev->dev;
> +
> + return devm_snd_soc_register_component(&pdev->dev, &soc_codec_dev_cs40l50,
> +       cs40l50_dai, ARRAY_SIZE(cs40l50_dai));
> +}
> +
> +static const struct platform_device_id cs40l50_id[] = {
> + { "cs40l50-codec", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(platform, cs40l50_id);
> +
> +static struct platform_driver cs40l50_codec_driver = {
> + .probe = cs40l50_codec_driver_probe,
> + .id_table = cs40l50_id,
> + .driver = {
> + .name = "cs40l50-codec",
> + },
> +};
> +module_platform_driver(cs40l50_codec_driver);
> +
> +MODULE_DESCRIPTION("ASoC CS40L50 driver");
> +MODULE_AUTHOR("James Ogletree <james.ogletree@cirrus.com>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.25.1
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND v10 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50
  2024-05-29 19:24   ` James Ogletree
@ 2024-05-29 19:28     ` Mark Brown
  2024-05-29 22:03     ` James Ogletree
  1 sibling, 0 replies; 16+ messages in thread
From: Mark Brown @ 2024-05-29 19:28 UTC (permalink / raw)
  To: James Ogletree
  Cc: James Ogletree, Dmitry Torokhov, robh+dt@kernel.org,
	Krzysztof Kozlowski, Conor Dooley, Lee Jones, Jeff LaBundy,
	open list:CIRRUS LOGIC HAPTIC DRIVERS,
	open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	David Rhodes

[-- Attachment #1: Type: text/plain, Size: 1287 bytes --]

On Wed, May 29, 2024 at 07:24:08PM +0000, James Ogletree wrote:
> HI Mark,
> 
> This file has not had any feedback yet for this version. For the sake of efficiency, I
> would appreciate your feedback. That way I can make any needed changes
> in the already-planned v11.

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND v10 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50
  2024-05-29 19:24   ` James Ogletree
  2024-05-29 19:28     ` Mark Brown
@ 2024-05-29 22:03     ` James Ogletree
  2024-05-30 10:05       ` Mark Brown
  1 sibling, 1 reply; 16+ messages in thread
From: James Ogletree @ 2024-05-29 22:03 UTC (permalink / raw)
  To: James Ogletree, Mark Brown
  Cc: Dmitry Torokhov, robh+dt@kernel.org, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Jeff LaBundy,
	open list:CIRRUS LOGIC HAPTIC DRIVERS,
	open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	David Rhodes


> On Apr 8, 2024, at 10:32 AM, James Ogletree <jogletre@opensource.cirrus.com> wrote:
> 
> Introduce support for Cirrus Logic Device CS40L50: a
> haptic driver with waveform memory, integrated DSP,
> and closed-loop algorithms.
> 
> The ASoC driver enables I2S streaming to the device.
> 
> Reviewed-by: David Rhodes <drhodes@opensource.cirrus.com>
> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
> ---
> MAINTAINERS                      |   1 +
> sound/soc/codecs/Kconfig         |  11 ++
> sound/soc/codecs/Makefile        |   2 +
> sound/soc/codecs/cs40l50-codec.c | 308 +++++++++++++++++++++++++++++++
> 4 files changed, 322 insertions(+)
> create mode 100644 sound/soc/codecs/cs40l50-codec.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 49c2e6e57b09..62701b13f741 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4943,6 +4943,7 @@ F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
> F: drivers/input/misc/cs40l*
> F: drivers/mfd/cs40l*
> F: include/linux/mfd/cs40l*
> +F: sound/soc/codecs/cs40l*
> 
> CIRRUS LOGIC DSP FIRMWARE DRIVER
> M: Simon Trimmer <simont@opensource.cirrus.com>
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index f1e1dbc509f6..1a81bedfdbe3 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -73,6 +73,7 @@ config SND_SOC_ALL_CODECS
> imply SND_SOC_CS35L56_I2C
> imply SND_SOC_CS35L56_SPI
> imply SND_SOC_CS35L56_SDW
> + imply SND_SOC_CS40L50
> imply SND_SOC_CS42L42
> imply SND_SOC_CS42L42_SDW
> imply SND_SOC_CS42L43
> @@ -800,6 +801,16 @@ config SND_SOC_CS35L56_SDW
> help
> Enable support for Cirrus Logic CS35L56 boosted amplifier with SoundWire control
> 
> +config SND_SOC_CS40L50
> + tristate "Cirrus Logic CS40L50 CODEC"
> + depends on MFD_CS40L50_CORE
> + help
> +  This option enables support for I2S streaming to Cirrus Logic CS40L50.
> +
> +  CS40L50 is a haptic driver with waveform memory, an integrated
> +  DSP, and closed-loop algorithms. If built as a module, it will be
> +  called snd-soc-cs40l50.
> +
> config SND_SOC_CS42L42_CORE
> tristate
> 
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index a87e56938ce5..7e31f000774a 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -74,6 +74,7 @@ snd-soc-cs35l56-shared-objs := cs35l56-shared.o
> snd-soc-cs35l56-i2c-objs := cs35l56-i2c.o
> snd-soc-cs35l56-spi-objs := cs35l56-spi.o
> snd-soc-cs35l56-sdw-objs := cs35l56-sdw.o
> +snd-soc-cs40l50-objs := cs40l50-codec.o
> snd-soc-cs42l42-objs := cs42l42.o
> snd-soc-cs42l42-i2c-objs := cs42l42-i2c.o
> snd-soc-cs42l42-sdw-objs := cs42l42-sdw.o
> @@ -460,6 +461,7 @@ obj-$(CONFIG_SND_SOC_CS35L56_SHARED) += snd-soc-cs35l56-shared.o
> obj-$(CONFIG_SND_SOC_CS35L56_I2C) += snd-soc-cs35l56-i2c.o
> obj-$(CONFIG_SND_SOC_CS35L56_SPI) += snd-soc-cs35l56-spi.o
> obj-$(CONFIG_SND_SOC_CS35L56_SDW) += snd-soc-cs35l56-sdw.o
> +obj-$(CONFIG_SND_SOC_CS40L50) += snd-soc-cs40l50.o
> obj-$(CONFIG_SND_SOC_CS42L42_CORE) += snd-soc-cs42l42.o
> obj-$(CONFIG_SND_SOC_CS42L42) += snd-soc-cs42l42-i2c.o
> obj-$(CONFIG_SND_SOC_CS42L42_SDW) += snd-soc-cs42l42-sdw.o
> diff --git a/sound/soc/codecs/cs40l50-codec.c b/sound/soc/codecs/cs40l50-codec.c
> new file mode 100644
> index 000000000000..6d4a0970b219
> --- /dev/null
> +++ b/sound/soc/codecs/cs40l50-codec.c
> @@ -0,0 +1,308 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// CS40L50 Advanced Haptic Driver with waveform memory,
> +// integrated DSP, and closed-loop algorithms
> +//
> +// Copyright 2024 Cirrus Logic, Inc.
> +//
> +// Author: James Ogletree <james.ogletree@cirrus.com>
> +
> +#include <linux/bitfield.h>
> +#include <linux/mfd/cs40l50.h>
> +#include <linux/pm_runtime.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +
> +#define CS40L50_REFCLK_INPUT 0x2C04
> +#define CS40L50_ASP_CONTROL2 0x4808
> +#define CS40L50_ASP_DATA_CONTROL5 0x4840
> +
> +/* PLL Config */
> +#define CS40L50_PLL_REFCLK_BCLK 0x0
> +#define CS40L50_PLL_REFCLK_MCLK 0x5
> +#define CS40L50_PLL_REEFCLK_MCLK_CFG 0x00
> +#define CS40L50_PLL_REFCLK_LOOP_MASK BIT(11)
> +#define CS40L50_PLL_REFCLK_OPEN_LOOP 1
> +#define CS40L50_PLL_REFCLK_CLOSED_LOOP 0
> +#define CS40L50_PLL_REFCLK_LOOP_SHIFT 11
> +#define CS40L50_PLL_REFCLK_FREQ_MASK GENMASK(10, 5)
> +#define CS40L50_PLL_REFCLK_FREQ_SHIFT 5
> +#define CS40L50_PLL_REFCLK_SEL_MASK GENMASK(2, 0)
> +#define CS40L50_BCLK_RATIO_DEFAULT 32
> +
> +/* ASP Config */
> +#define CS40L50_ASP_RX_WIDTH_SHIFT 24
> +#define CS40L50_ASP_RX_WIDTH_MASK GENMASK(31, 24)
> +#define CS40L50_ASP_RX_WL_MASK GENMASK(5, 0)
> +#define CS40L50_ASP_FSYNC_INV_MASK BIT(2)
> +#define CS40L50_ASP_BCLK_INV_MASK BIT(6)
> +#define CS40L50_ASP_FMT_MASK GENMASK(10, 8)
> +#define CS40L50_ASP_FMT_I2S 0x2
> +
> +struct cs40l50_pll_config {
> + unsigned int freq;
> + unsigned int cfg;
> +};
> +
> +struct cs40l50_codec {
> + struct device *dev;
> + struct regmap *regmap;
> + unsigned int daifmt;
> + unsigned int bclk_ratio;
> + unsigned int rate;
> +};
> +
> +static const struct cs40l50_pll_config cs40l50_pll_cfg[] = {
> + { 32768, 0x00 },
> + { 1536000, 0x1B },
> + { 3072000, 0x21 },
> + { 6144000, 0x28 },
> + { 9600000, 0x30 },
> + { 12288000, 0x33 },
> +};
> +
> +static int cs40l50_get_clk_config(unsigned int freq, unsigned int *cfg)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(cs40l50_pll_cfg); i++) {
> + if (cs40l50_pll_cfg[i].freq == freq) {
> + *cfg = cs40l50_pll_cfg[i].cfg;
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int cs40l50_swap_ext_clk(struct cs40l50_codec *codec, unsigned int clk_src)
> +{
> + unsigned int cfg;
> + int ret;
> +
> + switch (clk_src) {
> + case CS40L50_PLL_REFCLK_BCLK:
> + ret = cs40l50_get_clk_config(codec->bclk_ratio * codec->rate, &cfg);
> + if (ret)
> + return ret;
> + break;
> + case CS40L50_PLL_REFCLK_MCLK:
> + cfg = CS40L50_PLL_REEFCLK_MCLK_CFG;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
> + CS40L50_PLL_REFCLK_LOOP_MASK,
> + CS40L50_PLL_REFCLK_OPEN_LOOP <<
> + CS40L50_PLL_REFCLK_LOOP_SHIFT);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
> + CS40L50_PLL_REFCLK_FREQ_MASK |
> + CS40L50_PLL_REFCLK_SEL_MASK,
> + (cfg << CS40L50_PLL_REFCLK_FREQ_SHIFT) | clk_src);
> + if (ret)
> + return ret;
> +
> + return regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
> +  CS40L50_PLL_REFCLK_LOOP_MASK,
> +  CS40L50_PLL_REFCLK_CLOSED_LOOP <<
> +  CS40L50_PLL_REFCLK_LOOP_SHIFT);
> +}
> +
> +static int cs40l50_clk_en(struct snd_soc_dapm_widget *w,
> +  struct snd_kcontrol *kcontrol,
> +  int event)
> +{
> + struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(comp);
> + int ret;
> +
> + switch (event) {
> + case SND_SOC_DAPM_POST_PMU:
> + ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_STOP_PLAYBACK);
> + if (ret)
> + return ret;
> +
> + ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_START_I2S);
> + if (ret)
> + return ret;
> +
> + ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_BCLK);
> + if (ret)
> + return ret;
> + break;
> + case SND_SOC_DAPM_PRE_PMD:
> + ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_MCLK);
> + if (ret)
> + return ret;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_dapm_widget cs40l50_dapm_widgets[] = {
> + SND_SOC_DAPM_SUPPLY_S("ASP PLL", 0, SND_SOC_NOPM, 0, 0, cs40l50_clk_en,
> +      SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
> + SND_SOC_DAPM_AIF_IN("ASPRX1", NULL, 0, SND_SOC_NOPM, 0, 0),
> + SND_SOC_DAPM_AIF_IN("ASPRX2", NULL, 0, SND_SOC_NOPM, 0, 0),
> + SND_SOC_DAPM_OUTPUT("OUT"),
> +};
> +
> +static const struct snd_soc_dapm_route cs40l50_dapm_routes[] = {
> + { "ASP Playback", NULL, "ASP PLL" },
> + { "ASPRX1", NULL, "ASP Playback" },
> + { "ASPRX2", NULL, "ASP Playback" },
> +
> + { "OUT", NULL, "ASPRX1" },
> + { "OUT", NULL, "ASPRX2" },
> +};
> +
> +static int cs40l50_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
> +{
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(codec_dai->component);
> +
> + if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBC_CFC)
> + return -EINVAL;
> +
> + switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> + case SND_SOC_DAIFMT_NB_NF:
> + codec->daifmt = 0;
> + break;
> + case SND_SOC_DAIFMT_NB_IF:
> + codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK;
> + break;
> + case SND_SOC_DAIFMT_IB_NF:
> + codec->daifmt = CS40L50_ASP_BCLK_INV_MASK;
> + break;
> + case SND_SOC_DAIFMT_IB_IF:
> + codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK | CS40L50_ASP_BCLK_INV_MASK;
> + break;
> + default:
> + dev_err(codec->dev, "Invalid clock invert\n");
> + return -EINVAL;
> + }
> +
> + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> + case SND_SOC_DAIFMT_I2S:
> + codec->daifmt |= FIELD_PREP(CS40L50_ASP_FMT_MASK, CS40L50_ASP_FMT_I2S);
> + break;
> + default:
> + dev_err(codec->dev, "Unsupported DAI format\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int cs40l50_hw_params(struct snd_pcm_substream *substream,
> +     struct snd_pcm_hw_params *params,
> +     struct snd_soc_dai *dai)
> +{
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component);
> + unsigned int asp_rx_wl = params_width(params);
> + int ret;
> +
> + codec->rate = params_rate(params);
> +
> + ret = regmap_update_bits(codec->regmap, CS40L50_ASP_DATA_CONTROL5,
> + CS40L50_ASP_RX_WL_MASK, asp_rx_wl);
> + if (ret)
> + return ret;
> +
> + codec->daifmt |= (asp_rx_wl << CS40L50_ASP_RX_WIDTH_SHIFT);
> +
> + return regmap_update_bits(codec->regmap, CS40L50_ASP_CONTROL2,
> +  CS40L50_ASP_FSYNC_INV_MASK |
> +  CS40L50_ASP_BCLK_INV_MASK |
> +  CS40L50_ASP_FMT_MASK |
> +  CS40L50_ASP_RX_WIDTH_MASK, codec->daifmt);
> +}
> +
> +static int cs40l50_set_dai_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio)
> +{
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component);
> +
> + codec->bclk_ratio = ratio;
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_dai_ops cs40l50_dai_ops = {
> + .set_fmt = cs40l50_set_dai_fmt,
> + .set_bclk_ratio = cs40l50_set_dai_bclk_ratio,
> + .hw_params = cs40l50_hw_params,
> +};
> +
> +static struct snd_soc_dai_driver cs40l50_dai[] = {
> + {
> + .name = "cs40l50-pcm",
> + .id = 0,
> + .playback = {
> + .stream_name = "ASP Playback",
> + .channels_min = 1,
> + .channels_max = 2,
> + .rates = SNDRV_PCM_RATE_48000,
> + .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
> + },
> + .ops = &cs40l50_dai_ops,
> + },
> +};
> +
> +static int cs40l50_codec_probe(struct snd_soc_component *component)
> +{
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(component);
> +
> + codec->bclk_ratio = CS40L50_BCLK_RATIO_DEFAULT;
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_component_driver soc_codec_dev_cs40l50 = {
> + .probe = cs40l50_codec_probe,
> + .dapm_widgets = cs40l50_dapm_widgets,
> + .num_dapm_widgets = ARRAY_SIZE(cs40l50_dapm_widgets),
> + .dapm_routes = cs40l50_dapm_routes,
> + .num_dapm_routes = ARRAY_SIZE(cs40l50_dapm_routes),
> +};
> +
> +static int cs40l50_codec_driver_probe(struct platform_device *pdev)
> +{
> + struct cs40l50 *cs40l50 = dev_get_drvdata(pdev->dev.parent);
> + struct cs40l50_codec *codec;
> +
> + codec = devm_kzalloc(&pdev->dev, sizeof(*codec), GFP_KERNEL);
> + if (!codec)
> + return -ENOMEM;
> +
> + codec->regmap = cs40l50->regmap;
> + codec->dev = &pdev->dev;
> +
> + return devm_snd_soc_register_component(&pdev->dev, &soc_codec_dev_cs40l50,
> +       cs40l50_dai, ARRAY_SIZE(cs40l50_dai));
> +}
> +
> +static const struct platform_device_id cs40l50_id[] = {
> + { "cs40l50-codec", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(platform, cs40l50_id);
> +
> +static struct platform_driver cs40l50_codec_driver = {
> + .probe = cs40l50_codec_driver_probe,
> + .id_table = cs40l50_id,
> + .driver = {
> + .name = "cs40l50-codec",
> + },
> +};
> +module_platform_driver(cs40l50_codec_driver);
> +
> +MODULE_DESCRIPTION("ASoC CS40L50 driver");
> +MODULE_AUTHOR("James Ogletree <james.ogletree@cirrus.com>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.25.1
> 

Bottom posting this time.

Hi Mark,

This file has not had any feedback yet for this version. For the sake of efficiency, I
would appreciate your review. That way I can make any needed changes
in the already-planned v11.

Best,
James



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND v10 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50
  2024-05-29 22:03     ` James Ogletree
@ 2024-05-30 10:05       ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2024-05-30 10:05 UTC (permalink / raw)
  To: James Ogletree
  Cc: James Ogletree, Dmitry Torokhov, robh+dt@kernel.org,
	Krzysztof Kozlowski, Conor Dooley, Lee Jones, Jeff LaBundy,
	open list:CIRRUS LOGIC HAPTIC DRIVERS,
	open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	David Rhodes

[-- Attachment #1: Type: text/plain, Size: 1681 bytes --]

On Wed, May 29, 2024 at 10:03:00PM +0000, James Ogletree wrote:

> This file has not had any feedback yet for this version. For the sake of efficiency, I
> would appreciate your review. That way I can make any needed changes
> in the already-planned v11.

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

Please don't ignore review comments, people are generally making them
for a reason and are likely to have the same concerns if issues remain
unaddressed.  Having to repeat the same comments can get repetitive and
make people question the value of time spent reviewing.  If you disagree
with the review comments that's fine but you need to reply and discuss
your concerns so that the reviewer can understand your decisions.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RESEND v10 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50
  2024-04-08 15:32 ` [PATCH RESEND v10 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50 James Ogletree
  2024-05-29 19:24   ` James Ogletree
@ 2024-05-30 17:07   ` Rivera-Matos, Ricardo
  1 sibling, 0 replies; 16+ messages in thread
From: Rivera-Matos, Ricardo @ 2024-05-30 17:07 UTC (permalink / raw)
  To: James Ogletree, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, lee, broonie, jeff
  Cc: patches, linux-sound, linux-input, devicetree, David Rhodes


On 4/8/24 10:32 AM, James Ogletree wrote:
> Introduce support for Cirrus Logic Device CS40L50: a
> haptic driver with waveform memory, integrated DSP,
> and closed-loop algorithms.
>
> The ASoC driver enables I2S streaming to the device.
>
> Reviewed-by: David Rhodes <drhodes@opensource.cirrus.com>
> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
> ---
>   MAINTAINERS                      |   1 +
>   sound/soc/codecs/Kconfig         |  11 ++
>   sound/soc/codecs/Makefile        |   2 +
>   sound/soc/codecs/cs40l50-codec.c | 308 +++++++++++++++++++++++++++++++
>   4 files changed, 322 insertions(+)
>   create mode 100644 sound/soc/codecs/cs40l50-codec.c
>
<cut>
> diff --git a/sound/soc/codecs/cs40l50-codec.c b/sound/soc/codecs/cs40l50-codec.c
> new file mode 100644
> index 000000000000..6d4a0970b219
> --- /dev/null
> +++ b/sound/soc/codecs/cs40l50-codec.c
> @@ -0,0 +1,308 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// CS40L50 Advanced Haptic Driver with waveform memory,
> +// integrated DSP, and closed-loop algorithms
> +//
> +// Copyright 2024 Cirrus Logic, Inc.
> +//
> +// Author: James Ogletree <james.ogletree@cirrus.com>
> +
> +#include <linux/bitfield.h>
> +#include <linux/mfd/cs40l50.h>
> +#include <linux/pm_runtime.h>
Is pm_runtime.h being used in the context of the codec driver? If not, 
you should drop it.
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +
<cut>
> +
> +static const struct cs40l50_pll_config cs40l50_pll_cfg[] = {
> +	{ 32768, 0x00 },
> +	{ 1536000, 0x1B },
> +	{ 3072000, 0x21 },
> +	{ 6144000, 0x28 },
> +	{ 9600000, 0x30 },
> +	{ 12288000, 0x33 },
> +};
> +
> +static int cs40l50_get_clk_config(unsigned int freq, unsigned int *cfg)
You could constify freq.
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(cs40l50_pll_cfg); i++) {
> +		if (cs40l50_pll_cfg[i].freq == freq) {
> +			*cfg = cs40l50_pll_cfg[i].cfg;
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int cs40l50_swap_ext_clk(struct cs40l50_codec *codec, unsigned int clk_src)
You could constify clk_src.
> +{
> +	unsigned int cfg;
> +	int ret;
> +
> +	switch (clk_src) {
> +	case CS40L50_PLL_REFCLK_BCLK:
> +		ret = cs40l50_get_clk_config(codec->bclk_ratio * codec->rate, &cfg);
> +		if (ret)
> +			return ret;
> +		break;
> +	case CS40L50_PLL_REFCLK_MCLK:
> +		cfg = CS40L50_PLL_REEFCLK_MCLK_CFG;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
> +				 CS40L50_PLL_REFCLK_LOOP_MASK,
> +				 CS40L50_PLL_REFCLK_OPEN_LOOP <<
> +				 CS40L50_PLL_REFCLK_LOOP_SHIFT);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
> +				 CS40L50_PLL_REFCLK_FREQ_MASK |
> +				 CS40L50_PLL_REFCLK_SEL_MASK,
> +				 (cfg << CS40L50_PLL_REFCLK_FREQ_SHIFT) | clk_src);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
> +				  CS40L50_PLL_REFCLK_LOOP_MASK,
> +				  CS40L50_PLL_REFCLK_CLOSED_LOOP <<
> +				  CS40L50_PLL_REFCLK_LOOP_SHIFT);
> +}
> +
<cut>
> +
> +MODULE_DESCRIPTION("ASoC CS40L50 driver");
> +MODULE_AUTHOR("James Ogletree <james.ogletree@cirrus.com>");
> +MODULE_LICENSE("GPL");

This gets my Reviewed-by pending these edits.

Ricardo


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-05-30 17:07 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-08 15:32 [PATCH RESEND v10 0/5] Add support for CS40L50 James Ogletree
2024-04-08 15:32 ` [PATCH RESEND v10 1/5] firmware: cs_dsp: Add write sequence interface James Ogletree
2024-04-08 15:32 ` [PATCH RESEND v10 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding James Ogletree
2024-04-08 15:32 ` [PATCH RESEND v10 3/5] mfd: cs40l50: Add support for CS40L50 core driver James Ogletree
2024-04-11 15:35   ` Lee Jones
2024-04-08 15:32 ` [PATCH RESEND v10 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver James Ogletree
2024-04-16 23:28   ` Dmitry Torokhov
2024-04-19 18:26     ` James Ogletree
2024-05-03 15:25       ` James Ogletree
2024-05-20 15:47         ` James Ogletree
2024-04-08 15:32 ` [PATCH RESEND v10 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50 James Ogletree
2024-05-29 19:24   ` James Ogletree
2024-05-29 19:28     ` Mark Brown
2024-05-29 22:03     ` James Ogletree
2024-05-30 10:05       ` Mark Brown
2024-05-30 17:07   ` Rivera-Matos, Ricardo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).