linux-kernel - Re: [PATCH v3 06/12] mfd: wm97xx-core: core support for wm97xx Codec

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8760emsvey.fsf@belgarion.home>
Date:   Thu, 20 Jul 2017 22:30:45 +0200
From:   Robert Jarzmik <robert.jarzmik@...e.fr>
To:     Charles Keepax <ckeepax@...nsource.cirrus.com>
Cc:     Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Lee Jones <lee.jones@...aro.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, Daniel Mack <daniel@...que.org>,
        Haojian Zhuang <haojian.zhuang@...il.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Charles Keepax <ckeepax@...nsource.wolfsonmicro.com>,
        <linux-kernel@...r.kernel.org>, <linux-input@...r.kernel.org>,
        <patches@...nsource.wolfsonmicro.com>,
        <alsa-devel@...a-project.org>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3 06/12] mfd: wm97xx-core: core support for wm97xx Codec

Charles Keepax <ckeepax@...nsource.cirrus.com> writes:

> On Fri, Jun 30, 2017 at 09:44:02PM +0200, Robert Jarzmik wrote:
>> The WM9705, WM9712 and WM9713 are highly integrated codecs, with an
>> audio codec, DAC and ADC, GPIO unit and a touchscreen interface.
...
>> +static const struct reg_default wm97xx_reg_defaults[] = {
>> +};
>
> Should we not have some defaults in here?
Most certainly, I'll copy-paste the ones from :
 - sound/soc/codecs/wm9705.c
 - sound/soc/codecs/wm9712.c
>> +static int wm9705_register(struct wm97xx_priv *wm97xx)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int wm9712_register(struct wm97xx_priv *wm97xx)
>> +{
>> +	return 0;
>> +}
>> +
>
> So are we only adding stubs for the 9705 and 9712? If so we
> should probably make that clear in the commit message at least.
Mmmh, I think I've been too lazy here.
I'll add the true functions, which will be on par with wm9713_register(), adding
2 sub-cells, the codec and the touchscreen.

...zip..
>> +	codec_pdata.ac97 = wm97xx->ac97;
>> +	codec_pdata.regmap = devm_regmap_init_ac97(wm97xx->ac97,
>> +						   &wm9713_regmap_config);
>> +	codec_pdata.batt_pdata = pdata->batt_pdata;
>> +	if (IS_ERR(codec_pdata.regmap))
>> +		return PTR_ERR(codec_pdata.regmap);
>> +
>> +	return devm_mfd_add_devices(wm97xx->dev, -1, cells,
>
> Should probably use the define PLATFORM_DEVID_NONE here.
Yeah sure.

> Other than those minor comments looks ok to me.
Thanks Charles, I'll include them for next iteration.

Cheers.

-- 
Robert