linux-kernel - Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver

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]

Message-ID: <20140131224704.GI2950@lukather>
Date:	Fri, 31 Jan 2014 23:47:04 +0100
From:	Maxime Ripard <maxime.ripard@...e-electrons.com>
To:	Mark Brown <broonie@...nel.org>
Cc:	Mike Turquette <mturquette@...aro.org>,
	Emilio Lopez <emilio@...pez.com.ar>,
	linux-sunxi@...glegroups.com, linux-spi@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, kevin.z.m.zh@...il.com,
	sunny@...winnertech.com, shuge@...winnertech.com,
	zhuzhenhua@...winnertech.com
Subject: Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller
 driver

On Fri, Jan 31, 2014 at 12:48:09PM +0000, Mark Brown wrote:
> On Fri, Jan 31, 2014 at 11:55:50AM +0100, Maxime Ripard wrote:
> 
> > +	master = devm_spi_alloc_master(&pdev->dev, sizeof(struct sun6i_spi));
> > +	if (!master) {
> > +		dev_err(&pdev->dev, "Unable to allocate SPI Master\n");
> > +		return -ENOMEM;
> > +	}
> 
> This now depends on your other series which as I said doesn't look like
> the best approach.

Indeed, I forgot to mention it in the cover letter. My bad.

> 
> > +	pm_runtime_enable(&pdev->dev);
> > +	if (!pm_runtime_enabled(&pdev->dev)) {
> > +		ret = sun6i_spi_runtime_resume(&pdev->dev);
> > +		if (ret) {
> > +			dev_err(&pdev->dev, "Couldn't resume the device\n");
> > +			return ret;
> > +		}
> > +	}
> 
> No, as discussed don't do this - notice how other drivers aren't written
> this way either.  Like I said leave the device powered on startup and
> then let it be idled by runtime PM.

Well, some SPI drivers are actually written like that (all the tegra
SPI drivers for example). It's not an excuse, but waking up the device
only to put it back in suspend right away seems kind of
inefficient. Plus, the pm_runtime_idle callback you suggested are
actually calling runtime_idle, while we want to call runtime_suspend.


-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)