linux-kernel - Re: [PATCH v2 02/11] mfd: wcd934x: add support to wcd9340/wcd9341 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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 21 Oct 2019 14:19:27 +0100
From:   Lee Jones <lee.jones@...aro.org>
To:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Cc:     robh@...nel.org, broonie@...nel.org, linus.walleij@...aro.org,
        vinod.koul@...aro.org, alsa-devel@...a-project.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        spapothi@...eaurora.org, bgoswami@...eaurora.org,
        linux-gpio@...r.kernel.org
Subject: Re: [PATCH v2 02/11] mfd: wcd934x: add support to wcd9340/wcd9341
 codec

On Mon, 21 Oct 2019, Srinivas Kandagatla wrote:

> 
> 
> On 21/10/2019 12:45, Lee Jones wrote:
> > On Mon, 21 Oct 2019, Srinivas Kandagatla wrote:
> > 
> > > Thanks Lee for taking time to review.
> > > 
> > > I agree with most of the style related comments, will fix them in next
> > > version. For others I have replied it inline.
> > 
> > [...]
> > 
> > > > > +static int wcd934x_slim_status(struct slim_device *sdev,
> > > > > +			       enum slim_device_status status)
> > > > > +{
> > > > > +	struct device *dev = &sdev->dev;
> > > > > +	struct wcd934x_data *wcd;
> > > > > +	int ret;
> > > > 
> > > > This is semantically odd!  Why are you doing most of the
> > > > initialisation and bring-up in 'status' and not 'probe'.  Seems
> > > > broken to me.
> > > 
> > > SLIMBus device will not be in a state to communicate before enumeration (at
> > > probe), so all the device initialization is done in status callback where it
> > > is ready for communication.
> > 
> > Why do we need the device to be up *before* calling probe?
> > 
> 
> To Initialize the device.
> And SLIMbus device registers access can not be done before enumeration.

I'm not sure why you need any hardware to be enabled before calling
.probe(). That is the purpose of .probe(). The only way in which I can
see this being a requirement is if information located on the device
is required in order to do device-driver matching.

In which case, how was the .status() function matched?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog