linux-kernel - RE: [PATCH 11/11] i2c: omap: enhance pinctrl support

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: <1BAFE6F6C881BF42822005164F1491C33EC2875B@DBDE04.ent.ti.com>
Date:	Wed, 5 Jun 2013 09:05:37 +0000
From:	"Hebbar, Gururaja" <gururaja.hebbar@...com>
To:	Kevin Hilman <khilman@...aro.org>
CC:	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	"davinci-linux-open-source@...ux.davincidsp.com" 
	<davinci-linux-open-source@...ux.davincidsp.com>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux@....linux.org.uk" <linux@....linux.org.uk>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Bedia, Vaibhav" <vaibhav.bedia@...com>,
	"Rajashekhara, Sudhakar" <sudhakar.raj@...com>,
	Tony Lindgren <tony@...mide.com>,
	Wolfram Sang <wsa@...-dreams.de>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>
Subject: RE: [PATCH 11/11] i2c: omap: enhance pinctrl support

On Fri, May 31, 2013 at 23:04:59, Kevin Hilman wrote:
> Hebbar Gururaja <gururaja.hebbar@...com> writes:
> 
> > Amend the I2C omap pin controller to optionally take a pin control
> > handle and set the state of the pins to:
> >
> > - "default" on boot, resume and before performing an i2c transfer
> > - "idle" after initial default, after resume default, and after each
> > i2c xfer
> > - "sleep" on suspend()
> >
> > By optionally putting the pins into sleep state in the suspend callback
> > we can accomplish two things.
> > - One is to minimize current leakage from pins and thus save power,
> > - second, we can prevent the IP from driving pins output in an
> > uncontrolled manner, which may happen if the power domain drops the
> > domain regulator.
> >
> > Note:
> > A .suspend & .resume callback is added which simply puts the pins to sleep
> > state upon suspend & are moved to default & idle state upon resume.
> >
> > If any of the above pin states are missing in dt, a warning message
> > about the missing state is displayed.
> > If certain pin-states are not available, to remove this warning message
> > pass respective state name with null phandler.
> >
> > (Changes based on i2c-nomadik.c)
> >
> > Signed-off-by: Hebbar Gururaja <gururaja.hebbar@...com>
> > Cc: Tony Lindgren <tony@...mide.com>
> > Cc: Wolfram Sang <wsa@...-dreams.de>
> > Cc: linux-omap@...r.kernel.org
> > Cc: linux-i2c@...r.kernel.org
> 
> [...]
> 
> > @@ -664,7 +673,13 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> >  
> >  out:
> >  	pm_runtime_mark_last_busy(dev->dev);
> > +
> >  	pm_runtime_put_autosuspend(dev->dev);
> > +	/* Optionally let pins go into idle state */
> > +	if (!IS_ERR(dev->pins_idle))
> > +		if (pinctrl_select_state(dev->pinctrl, dev->pins_idle))
> > +			dev_err(dev->dev, "could not set pins to idle state\n");
> 
> This is wrong.  You're changing the muxing before the device actually
> goes idle.  Anything you want to happen when the device actually idles
> for real has to be in runtime PM callbacks.
> 
> Looking below, I see it's already in the callbacks, so why is it here also?

Just to be double sure. Seems it is unwanted.

> 
> [...]
> 
> > @@ -1300,6 +1348,10 @@ static int omap_i2c_runtime_suspend(struct device *dev)
> >  		omap_i2c_read_reg(_dev, OMAP_I2C_STAT_REG);
> >  	}
> >  
> > +	if (!IS_ERR(_dev->pins_idle))
> > +		if (pinctrl_select_state(_dev->pinctrl, _dev->pins_idle))
> > +			dev_err(dev, "could not set pins to idle state\n");
> > +
> >  	return 0;
> >  }
> >  
> 
> Kevin
> 


Regards, 
Gururaja
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/