| 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
| ||
|
Message-ID: <51D71337.6030409@wwwdotorg.org>
Date: Fri, 05 Jul 2013 12:40:55 -0600
From: Stephen Warren <swarren@...dotorg.org>
To: Christian Ruppert <christian.ruppert@...lis.com>
CC: Linus Walleij <linus.walleij@...aro.org>,
Patrice CHOTARD <patrice.chotard@...com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Grant Likely <grant.likely@...retlab.ca>,
Rob Herring <rob.herring@...xeda.com>,
Rob Landley <rob@...dley.net>,
Sascha Leuenberger <sascha.leuenberger@...lis.com>,
Pierrick Hascoet <pierrick.hascoet@...lis.com>,
devicetree-discuss@...ts.ozlabs.org, linux-doc@...r.kernel.org,
Alexandre Courbot <acourbot@...dia.com>
Subject: Re: [PATCH 2/4] pinmux: Add TB10x pinmux driver
On 07/05/2013 03:49 AM, Christian Ruppert wrote:
> On Wed, Jun 26, 2013 at 11:40:42AM -0600, Stephen Warren wrote:
>> On 06/26/2013 05:50 AM, Christian Ruppert wrote:
>>> On Wed, Jun 19, 2013 at 04:35:14PM -0600, Stephen Warren wrote:
>>>> On 06/18/2013 03:29 AM, Christian Ruppert wrote:
>> [...]
>>>>> +Example
>>>>> +-------
>>>>> +
>>>>> +iomux: iomux@...0601c {
>>>>> + compatible = "abilis,tb10x-iomux";
>>>>> + reg = <0xFF10601c 0x4>;
>>>>> + pctl_gpio_a: pctl-gpio-a {
>>>>> + pingrp = "gpioa_pins";
>>>>> + };
>>>>> + pctl_uart0: pctl-uart0 {
>>>>> + pingrp = "uart0_pins";
>>>>> + };
>>>>> +};
>>>>
>>>> The two nodes pctl-gpio-a and pctl-uart0 seem to be missing data. The
>>>> idea here is that you define nodes that says:
>>>>
>>>> * This node applies to these pin(s)/group(s).
>>>> * Select mux function X on those pins/groups and/or apply these pin
>>>> configuration options to those pins/groups.
>>>>
>>>> The examples above don't include any mux/config options, nor does the
>>>> binding say how to do specify them.
>>>>
>>>> The set of pin groups defined by this binding should correspond directly
>>>> to the set of pin groups that actually exist in HW. So, if you have 3
>>>> pin groups (A, B, C) in HW each of which has two mux functions (X, Y),
>>>> your DT binding should define just 3 pin groups (A, B, C), not 6 (A_X,
>>>> A_Y, B_X, B_Y, C_X, C_Y). In other words, the pin group name shouldn't
>>>> imply the mux function.
>>>
>>> Can we consider it as agreed now that this implementation is acceptable
>>> for the TB10x pin controller?
>>
>> There are two issues here:
>>
>> 1) What is a pin group:
>>
>> 1a) Must it solely represent a group of pins that actually exists in HW
>> (e.g. it's an RTL port, or a set of pins all controlled at once by a
>> single bit/field in a register)
>>
>> 1b) A SW-defined group of pins, simply because it's convenient to talk
>> about that set of pins at once, even though HW doesn't impose that those
>> pins are in a group in any way.
>>
>> Defining groups for either of those reasons is fine, although this is
>> the area where my preference and LinusW's differ.
>>
>> 2) Can groups represent just a set of pins, or can it also imply that a
>> particular mux function is selected on that group?
>>
>> I believe that both LinusW and I are in agreement that a group is simply
>> a list/set/group of pins. You select mux functions onto groups. A
>> groups's definition can't imply that a particular mux function is
>> selected onto it.
>>
>> If we don't follow this rule, then you end up with a combinatorial
>> explosion of groups; the cross-product of all possible groups of pins
>> vs. the mux function to select on them, rather than simply having a list
>> of groups of pins, which is a much smaller set/list.
>>
>> So, in the DT example above, I still believe that you need an extra
>> property that defines which mux function to select onto the specified
>> group. The group name can't imply this, so there needs to be some way of
>> specifying it.
>
> In your opinion, would something in the lines of
>
> pctl_spi1: pctl-spi1 {
> abilis,pingrp = "spi1";
So that defines a list of pins.
> abilis,ioport = <4>; /* spi1 is routed to port4 inside the
> pin controller */
I assume that defines the mux function value; the value that's
programmed into the HW register to select which HW module's signals are
routed out to the pins specified by abilis,pingrp.
> abilis,ioconf = <1>; /* spi1 is available in configuration 1
> of that port. */
But I don't understand what that is. ...
...
> In future, this could even be extended to allow several alternative
> configurations for a given function, e.g.
>
> pctl_spi3: pctl-spi3 {
> abilis,pingrp = "spi3";
> abilis,ioport = <6>;
> abilis,ioconf = <0 3>; /* spi3 is available in both
> configurations 0 and 3. Depending on
> what other functions are requested, the
> pinctrl driver can choose either of the
> two. */
... especially if you're talking about "spi3 being available in multiple
configurations". What's a configuration?
--
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/
Powered by blists - more mailing lists