On 11/11/2019 11:18, Lee Jones wrote:
On Tue, 29 Oct 2019, Srinivas Kandagatla wrote:
Qualcomm WCD9340/WCD9341 Codec is a standalone Hi-Fi audio codec IC.
This codec has integrated SoundWire controller, pin controller and
interrupt controller.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
---
No changelog?
I have done that in cover letter.
If you prefer it here, I can add that in next version.
drivers/mfd/Kconfig | 12 +
drivers/mfd/Makefile | 1 +
drivers/mfd/wcd934x.c | 306 +++++++++++++++
include/linux/mfd/wcd934x/registers.h | 529 ++++++++++++++++++++++++++
include/linux/mfd/wcd934x/wcd934x.h | 31 ++
5 files changed, 879 insertions(+)
create mode 100644 drivers/mfd/wcd934x.c
create mode 100644 include/linux/mfd/wcd934x/registers.h
create mode 100644 include/linux/mfd/wcd934x/wcd934x.h
This driver reads much better now. Thanks for making the changes.
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index ae24d3ea68ea..9fe7e54b13bf 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1967,6 +1967,18 @@ config MFD_STMFX
additional drivers must be enabled in order to use the functionality
of the device.
+config MFD_WCD934X
+ tristate "Support for WCD9340/WCD9341 Codec"
+ depends on SLIMBUS
+ select REGMAP
+ select REGMAP_SLIMBUS
+ select REGMAP_IRQ
+ select MFD_CORE
+ help
+ Support for the Qualcomm WCD9340/WCD9341 Codec.
+ This driver provides common support wcd934x audio codec and its
+ associated Pin Controller, Soundwire Controller and Audio codec.
Your capitalisation of devices is all over the place in both your help
section and in the commit message. Either capitalise them all or none
of them. Personally I would prefer all, rather than none. What ever
you choose, please be consistent.
Same for "wcd934x", this should read "WCD934x" in all comments and the
help.
I agree, will fix it along with other Nits you suggested.
[...]
+static void wcd934x_slim_remove(struct slim_device *sdev)
+{
+ struct wcd934x_ddata *ddata = dev_get_drvdata(&sdev->dev);
+
+ regulator_bulk_disable(WCD934X_MAX_SUPPLY, ddata->supplies);
+ mfd_remove_devices(&sdev->dev);
+ kfree(ddata);
+}
+
+static const struct slim_device_id wcd934x_slim_id[] = {
+ { SLIM_MANF_ID_QCOM, SLIM_PROD_CODE_WCD9340, 0x1, 0x0 },
What do the last parameters mean? Might be better to define them.
This is Instance ID and Device ID of SLIMBus enumeration address.
+ {}
+};
[...]
