linux-kernel - Re: [PATCH v2 1/6] pinctrl: baytrail: Add pin control data structures

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: <20160330114243.GT2099@lahna.fi.intel.com>
Date:	Wed, 30 Mar 2016 14:42:43 +0300
From:	Mika Westerberg <mika.westerberg@...ux.intel.com>
To:	Cristina Ciocan <cristina.ciocan@...el.com>
Cc:	mathias.nyman@...ux.intel.com, linux-gpio@...r.kernel.org,
	linux-kernel@...r.kernel.org, irina.tirdea@...el.com,
	octavian.purdila@...el.com
Subject: Re: [PATCH v2 1/6] pinctrl: baytrail: Add pin control data structures

On Wed, Mar 30, 2016 at 02:33:39PM +0300, Cristina Ciocan wrote:
> On 30.03.2016 14:15, Mika Westerberg wrote:
> > On Mon, Mar 28, 2016 at 04:29:35PM +0300, Cristina Ciocan wrote:
> >> +/* SCORE pins */
> >> +static const struct pinctrl_pin_desc byt_score_pins[] = {
> >> +	PINCTRL_PIN(0, "SATA_GP[0]"),		/* GPIOC_0 */
> >> +	PINCTRL_PIN(1, "SATA_GP[1]"),		/* GPIOC_1 */
> > 
> > Maybe we should call these "SATA_GP0" and "SATA_GP1" like we do in other
> > Intel pinctrl drivers?
> 
> The names are directly taken form the public datasheet found at:
> http://www.intel.com/content/www/us/en/embedded/products/bay-trail/atom-e3800-family-datasheet.html,
> section 10.3, Ball Name and Function by Location.
> 
> I kept those names, even though they are not always pretty, so that teh
> pins can be easily identified if someone searches them in the datasheet
> for extra information.

Well, I think it is not too hard to find out that SATA_GP0 matches
SATA_GP[0] in the datasheet ;-)

> > Also I don't think /* GPIOC_1 */ is really useful comment as that can be
> > derived already from the pin number.
> 
> The issue here is that pins are not referenced by the same name in the
> datasheet. In the above mentioned section (10.3), south core pins are
> GPIO_S0_SC[<pin_number>], whereas in the GPIO section (39) they are
> referenced as GPIOC_<pin_number>. I added the comments for the same
> reasoning as above, easy search for datasheet-driver pin matching.

And the <pin_number> in both cases is the same.

> If this is not an issue, I can change both names and comments.

I'm fine either way. Just wanted to mention because other Intel pinctrl
drivers use slightly different naming.