linux-kernel - Re: [PATCH v2 3/4] dt-bindings: Add pinctrl bindings for mt65xx/mt81xx.

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: <1412260889.4875.11.camel@pengutronix.de>
Date:	Thu, 02 Oct 2014 16:41:29 +0200
From:	Lucas Stach <l.stach@...gutronix.de>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	"Hongzhou.Yang" <srv_hongzhou.yang@...iatek.com>,
	Mark Rutland <mark.rutland@....com>,
	Ashwin Chaugule <ashwin.chaugule@...aro.org>,
	Vladimir Murzin <vladimir.murzin@....com>,
	Russell King <linux@....linux.org.uk>,
	srv_heupstream@...iatek.com, Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Hongzhou Yang <hongzhou.yang@...iatek.com>,
	Catalin Marinas <catalin.marinas@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Grant Likely <grant.likely@...aro.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Rob Herring <robh+dt@...nel.org>,
	Sascha Hauer <kernel@...gutronix.de>,
	Kumar Gala <galak@...eaurora.org>,
	Matthias Brugger <matthias.bgg@...il.com>,
	"Joe.C" <yingjoe.chen@...iatek.com>, dandan.he@...iatek.com,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 3/4] dt-bindings: Add pinctrl bindings for
 mt65xx/mt81xx.

Am Donnerstag, den 02.10.2014, 16:00 +0200 schrieb Linus Walleij:
> On Tue, Sep 23, 2014 at 5:39 AM, Hongzhou.Yang
> <srv_hongzhou.yang@...iatek.com> wrote:
> 
> > From: Hongzhou Yang <hongzhou.yang@...iatek.com>
> >
> > Add devicetree bindings for Mediatek SoC pinctrl driver.
> >
> > Signed-off-by: Hongzhou Yang <hongzhou.yang@...iatek.com>
> 
> I have worked on generic pin control bindings a bit because it
> is getting out of hand with all these custom bindings.
> 
> See:
> http://marc.info/?l=devicetree&m=141223584006648&w=2
> 
> Especially.
> 
> > +- mediatek,pinfunc: List of gpio number and function to mux.
> 
> A "GPIO number" and a "pin number" is not the same thing at all,
> this is very confusing. Those are two separate number spaces.
> This is likely about the pin numbers.
> 
> > +The mediatek,pinfunc can use defines directly,
> > +which are already defind in boot/dts/mt8135-pinfunc.h.
> > +
> > +Optional subnode-properties:
> > +- generic pin configuration option to use, bias-disable, bias-pull-down,
> > +  bias-pull,up, output-low and output-high are valid.
> > +  Example :
> > +       i2c0_pins_a {
> > +               mediatek,pinfunc = <MT8135_PIN_195_SDA1__FUNC_SDA1>;
> > +               bias-disable;
> > +       };
> 
> I don't like this approach at all.
> 
> I prefer that pins are put into groups named by strings, like "i2c0-pos0"
> inside the driver and then connected to function with a certain
> device-related name, such as "i2c0".
> 
So we should create artificial software groups where there are none in
hardware? This sounds really backward to me. Almost every new SoC out
there has the ability to mux every pin on it's own.

By defining artificial software groups in the driver we are pushing
constraints in the binding that don't really exist in hardware. So if
someone comes up with a pin usage that isn't covered by the existing
groups we need to change the binding. Experience from working with lots
of hardware engineers tells us that if something can be done (if there
are no constraints in HW) it will be done sooner or later.
If the hardware allows this much freedom we should also allow it in the
pinctrl binding.

> Then put the pin configuration (bias etc) in a separate node in the same
> state definition like that:
> 
> i2c0_pins_a {
>          function = "i2c0";
>          groups = "i2c0-pos0";
> };
> i2c0_pins_b {
>          bias-disable;
> };

The problem with that is that different pins might need different
configuration for the same muxed function. To properly reflect this we
would need to duplicate the pin definitions. One popular example is the
MMC interface where part of the pins need to have a pull-up, while
others don't. How would you reflect this with the DT description above?

Regards,
Lucas

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
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/