linux-kernel - Re: [PATCH v2] pinctrl: rockchip: add support for the rk3399

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: <1664345.86tvmnNB3Q@diego>
Date:	Wed, 27 Jan 2016 15:10:19 +0100
From:	Heiko Stübner <heiko@...ech.de>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	David Wu <wdc@...k-chips.com>, huangtao@...k-chips.com,
	Chris Zhong <zyw@...k-chips.com>,
	Eddie Cai <cf@...k-chips.com>, xjq@...k-chips.com,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	David Wu <david.wu@...k-chips.com>
Subject: Re: [PATCH v2] pinctrl: rockchip: add support for the rk3399

Am Mittwoch, 27. Januar 2016, 15:02:18 schrieb Linus Walleij:
> On Thu, Jan 7, 2016 at 8:41 AM, David Wu <wdc@...k-chips.com> wrote:
> > From: David Wu <david.wu@...k-chips.com>
> > 
> > The pinctrl of rk3399 is much different from other's,
> > especially the 3bits of drive strength.
> > 
> > Signed-off-by: David Wu <david.wu@...k-chips.com>
> > ---
> > change from v1:
> > - need spin_unlock_irqrestore for set drive default case
> 
> Heiko can you please look at this patch?

sorry, about that falling so far down my inbox. I'll try to look at it today 
or tomorrow.

But overall it looks good.

@David: if you wake up before I manage to review more in-depth, the
"reading register twice" (+writing counterpart)
+                       /* need to read regs twice */
+                       ret = regmap_read(regmap, reg, &temp);
+                       if (ret)
+                               return ret;

should definitly get more explanation on how that access scheme works. Seems to 
be some special register access method with the register returning different 
values each time? As I don't have a manual for the soc yet this is definitly 
curious for me :-)

> 
> > +       { .compatible = "rockchip,rk3399-pinctrl",
> > +               .data = (void *)&rk3399_pin_ctrl },
> 
> I guess this requires a small patch to some binding file.
> It's OK to fold that into this patch.

@David:
Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.txt