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] [thread-next>] [day] [month] [year] [list]
Date: Mon, 29 Jan 2024 05:03:48 +0000
From: "Ding, Shenghao" <shenghao-ding@...com>
To: Mark Brown <broonie@...nel.org>
CC: "conor+dt@...nel.org" <conor+dt@...nel.org>,
        "krzysztof.kozlowski@...aro.org" <krzysztof.kozlowski@...aro.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>,
        "Lu,
 Kevin" <kevin-lu@...com>, "Xu, Baojun" <baojun.xu@...com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "P O, Vijeth"
	<v-po@...com>,
        "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "perex@...ex.cz"
	<perex@...ex.cz>,
        "pierre-louis.bossart@...ux.intel.com"
	<pierre-louis.bossart@...ux.intel.com>,
        "13916275206@....com"
	<13916275206@....com>,
        "Chawla, Mohit" <mohit.chawla@...com>,
        "linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "liam.r.girdwood@...el.com" <liam.r.girdwood@...el.com>,
        "soyer@....hu"
	<soyer@....hu>, "Huang, Jonathan" <jkhuang3@...com>,
        "tiwai@...e.de"
	<tiwai@...e.de>, "Djuandi, Peter" <pdjuandi@...com>,
        "McPherson, Jeff"
	<j-mcpherson@...com>,
        "Navada Kanyana, Mukund" <navada@...com>
Subject: RE: [EXTERNAL] Re: [PATCH v2 1/4] ASoc: PCM6240: Create PCM6240
 Family driver code



> -----Original Message-----
> From: Mark Brown <broonie@...nel.org>
> Sent: Friday, January 26, 2024 10:33 PM
> To: Ding, Shenghao <shenghao-ding@...com>
> Cc: conor+dt@...nel.org; krzysztof.kozlowski@...aro.org;
> robh+dt@...nel.org; andriy.shevchenko@...ux.intel.com; Lu, Kevin <kevin-
> lu@...com>; Xu, Baojun <baojun.xu@...com>; devicetree@...r.kernel.org; P
> O, Vijeth <v-po@...com>; lgirdwood@...il.com; perex@...ex.cz; pierre-
> louis.bossart@...ux.intel.com; 13916275206@....com; Chawla, Mohit
> <mohit.chawla@...com>; linux-sound@...r.kernel.org; linux-
> kernel@...r.kernel.org; liam.r.girdwood@...el.com; soyer@....hu; Huang,
> Jonathan <jkhuang3@...com>; tiwai@...e.de; Djuandi, Peter
> <pdjuandi@...com>; McPherson, Jeff <j-mcpherson@...com>; Navada
> Kanyana, Mukund <navada@...com>
> Subject: [EXTERNAL] Re: [PATCH v2 1/4] ASoc: PCM6240: Create PCM6240
> Family driver code
> 
> On Fri, Jan 26, 2024 at 11:58:51AM +0800, Shenghao Ding wrote:
> 
> This looks mostly good - I've got a few comments that are mainly stylistic or
> otherwise very minor, there's one issue with validation of profile IDs that
> does look like it's important to fix though.
.............................
> 
> > +	val = (val >> shift) & mask;
> > +	val = (val > max) ? max : val;
> > +	val = mc->invert ? max - val : val;
> > +	ucontrol->value.integer.value[0] = val;
> 
> There's the FIELD_GET() macro (and FIELD_SET() for writing values) - the core
> predates them and hence doesn't use them, we might want to update some
> time.
Hi, Mark. FIELD_GET seemed not suitable in this, because mask in not the const. 
it will cause compile error.
> 
> > +static int pcmdevice_codec_probe(struct snd_soc_component *codec) {
> 
> > +	ret = request_firmware_nowait(THIS_MODULE,
> FW_ACTION_UEVENT,
> > +		pcm_dev->regbin_name, pcm_dev->dev, GFP_KERNEL,
> pcm_dev,
> > +		pcmdev_regbin_ready);
> > +	if (ret) {
> > +		dev_err(pcm_dev->dev, "load %s error = %d\n",
> > +			pcm_dev->regbin_name, ret);
> > +		goto out;
> > +	}
> 
> It might be better to request the firmware in the I2C probe rather than in the
> ASoC level probe, that way there's more time for the firmware to be loaded
> before we actually need it.  That does mean you can't register the controls
> immediately though so it may be more trouble than it's worth.

I once put request_firmware_nowait in i2c_probe, but it sometimes returned 
error in some platforms. So my customer suggest that it would be moved into 
the codec_probe. It seemed filesystem is not completely ready in some 
platform during calling the i2c_probe.
> 
> Similarly for the reset, if we reset as early as possible that seems better.

As to reset, it is also from my customers' suggestion. they found the issue that 
i2c access error in i2c_probe in some platform. So they put it into codec_probe.


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ