lkml.org 
[lkml]   [2019]   [Nov]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 02/11] mfd: wcd934x: add support to wcd9340/wcd9341 codec
From
Date


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 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.


>
>> + {}
>> +};
>
> [...]
>

\
 
 \ /
  Last update: 2019-11-11 13:49    [W:0.120 / U:0.504 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site