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@linaro.org
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 itsassociated 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.
- {}
+};
[...]