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
| ||
|
Date: Mon, 19 Apr 2021 01:46:46 +0000 From: "Liao, Bard" <bard.liao@...el.com> To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>, Vinod Koul <vkoul@...nel.org>, Bard Liao <yung-chuan.liao@...ux.intel.com> CC: "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>, "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "hui.wang@...onical.com" <hui.wang@...onical.com>, "srinivas.kandagatla@...aro.org" <srinivas.kandagatla@...aro.org>, "Kale, Sanyog R" <sanyog.r.kale@...el.com>, "rander.wang@...ux.intel.com" <rander.wang@...ux.intel.com> Subject: RE: [PATCH] soundwire: add slave device to linked list after device_register() > -----Original Message----- > From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com> > Sent: Wednesday, March 24, 2021 2:31 AM > To: Vinod Koul <vkoul@...nel.org>; Bard Liao <yung- > chuan.liao@...ux.intel.com> > Cc: alsa-devel@...a-project.org; gregkh@...uxfoundation.org; linux- > kernel@...r.kernel.org; hui.wang@...onical.com; > srinivas.kandagatla@...aro.org; Kale, Sanyog R <sanyog.r.kale@...el.com>; > rander.wang@...ux.intel.com; Liao, Bard <bard.liao@...el.com> > Subject: Re: [PATCH] soundwire: add slave device to linked list after > device_register() > > Hi Vinod, > > >> We currently add the slave device to a linked list before > >> device_register(), and then remove it if device_register() fails. > >> > >> It's not clear why this sequence was necessary, this patch moves the > >> linked list management to after the device_register(). > > > > Maybe add a comment :-) > > > > The reason here is that before calling device_register() we need to be > > ready and completely initialized. device_register is absolutely the > > last step in the flow, always. > > > > The probe of the device can happen and before you get a chance to add > > to list, many number of things can happen.. So adding after is not a > > very good idea :) > > Can you describe that 'many number of things' in the SoundWire context? > > While you are correct in general on the use of device_register(), in this specific > case the device registration and the possible Slave driver probe if there's a > match doesn't seem to use this linked list. > > This sdw_slave_add() routine is called while parsing ACPI/DT tables and at this > point the bus isn't functional. This sequence only deals with device registration > and driver probe, the actual activation and enumeration happen much later. > What does matter is that by the time all ACPI/DT devices have been scanned all > initialization is complete. The last part of the flow is not the device_register() at > the individual peripheral level. > > Even for the Qualcomm case, the steps are different: > > ret = sdw_bus_master_add(&ctrl->bus, dev, dev->fwnode); > if (ret) { > dev_err(dev, "Failed to register Soundwire controller (%d)\n", > ret); > goto err_clk; > } > > qcom_swrm_init(ctrl); <<< that's where the bus is functional > > Note that we are not going to lay on the tracks for this, this sequence was > tagged by static analysis tools who don't understand that > put_device() actually frees memory by way of the .release callback, but that led > us to ask ourselves whether this sequence was actually necessary. Hi Vinod, Do you have any comment or objection after Pierre's explanation? Regards, Bard
Powered by blists - more mailing lists