linux-kernel - Re: [PATCH v2 02/14] soundwire: Add SoundWire bus type

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:   Fri, 17 Nov 2017 09:22:37 +0530
From:   Vinod Koul <vinod.koul@...el.com>
To:     Mark Brown <broonie@...nel.org>
Cc:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        ALSA <alsa-devel@...a-project.org>, Takashi <tiwai@...e.de>,
        Pierre <pierre-louis.bossart@...ux.intel.com>,
        Sanyog Kale <sanyog.r.kale@...el.com>,
        Shreyas NC <shreyas.nc@...el.com>, patches.audio@...el.com,
        alan@...ux.intel.com,
        Charles Keepax <ckeepax@...nsource.cirrus.com>,
        Sagar Dharia <sdharia@...eaurora.org>, plai@...eaurora.org,
        Sudheer Papothi <spapothi@...eaurora.org>
Subject: Re: [PATCH v2 02/14] soundwire: Add SoundWire bus type

On Thu, Nov 16, 2017 at 05:24:37PM +0000, Mark Brown wrote:
> On Thu, Nov 16, 2017 at 10:32:21PM +0530, Vinod Koul wrote:
> > On Thu, Nov 16, 2017 at 04:05:17PM +0000, Srinivas Kandagatla wrote:
> 
> > > >+static const struct sdw_device_id *
> > > >+sdw_get_device_id(struct sdw_slave *slave, struct sdw_driver *drv)
> > > Indentation looks Odd here,
> 
> > not really, the return type in preceding line is a perfect way to define
> > things...
> 
> Yeah, that's fairly normal - just ugly long.  You could also do it by
> splitting the arguments over two lines.

IMO that makes it look even worse :(

static const struct sdw_device_id *sdw_get_device_id(
					struct sdw_slave *slave,
					struct sdw_driver *drv)

current one seemed lesser evil :)

> 
> > > >+	ret = dev_pm_domain_attach(dev, false);
> > > >+	if (ret) {
> 
> > > I think we discussed this in v1, but erring out here means that all the
> > > devices need to have pm domain  attached, which might not be true all the
> > > time.
> 
> > oh yes, sorry i missed this one. will fix
> 
> Looking at this pattern here I'm wondering if this should be folded into
> the domain code, everything attaching a domain is going to need to take
> care of this at the minute.

Yes, but then only busses call this, handful of users and some like platform
core have bit different handling, so it may not work for them

-- 
~Vinod