Sorry, I haven't had a chance to read any of the pincrl emails from
Friday onwards. However, I thought a bit more about this, and decided
to propose someting much simpler:
Thoughts:
* Defining all the pins, groups, functions, ... takes a lot of space,
whether it's in static data in pinctrl drivers or in the device tree.
The lists must also be stored in RAM at runtime.
* It's been very difficult to come up with a generic description of all
pin controller's capabilities. This is true even irrespective of device
tree; think pin config where we've agonized over whether we can create
a standardized list of pin config properties, or need to allow each
pinctrl driver to define its own set of properties, etc.
* The only real use of the lists is for debugfs. Drivers shouldn't expect
to directly request specific pinctrl settings, since that would encode
knowledge of an individual SoC's pin controller. This should be
abstracted from drivers.
* The data in debugfs could easily be replaced by a raw register dump
coupled with a SoC-specific script to print out what each register
means.
My proposal below is to radically simplify the pinctrl subsystem, and
make it little more than a system to execute a list of arbitrary register
writes.
Advantages:
* No need to define any kind of data model for pinctrl.
* No need to store any list of pins/groups/function/parameters/... either
in static data, in device tree, or at run-time.
* No need to store the the selected board-specific settings anywhere either.
* Completely generic; can handle anything that exists now or in the future.
* Handles pin mux and pin config identically.
* Extremely simple to implement and understand; probably just a few K
of code and no data. I assume this will be very appealing to those
interested in using the binding within U-Boot too.
* The lists of register writes can just as easily be provided by board
files as device tree, so operation should be identical.
Disadvantages:
* Creating the list of register values/masks may be a little painful. If
we move forward with this, we'd want to strongly push dtc towards
providing named constants, expressesion, macros, etc. Those dtc features
would be very useful anyway.
* Lack of high-level semantic meaning to the data; but I assert there's
little benefit to having that anyway.
To solve this, write a script to parse each pinctrl driver's regcache
file and print out all the register meanings.
* This scheme wouldn't represent which device "owns" which pins/groups,
nor be able to validate that different devices' pinmux settings don't
conflict.
I don't think this is a huge issue though, since that's mainly error-
checking. Any problems should be just as obvious during testing whether
they cause the HW to fail to operate correctly, or whether they cause
pinctrl to throw an error. Equally, there are many error conditions
that pinctrl core wasn't checking for anyway.
Precedent:
* There was previous discussion on using this exact technique for pinmux
at a previous Linaro Connect:
http://www.spinics.net/lists/linux-tegra/msg01943.html
* There is an existing binding that works very similarly, for the Mavell
PHY:
http://www.gossamer-threads.com/lists/linux/kernel/1305624
drivers/net/phy/marvell.c
Explicit mention was made here that a simple list of register writes
avoiding the complexity of representing a huge number of combinations
in the device tree.
tegra20.dtsi:
pinmux: pinmux@70000000 {
compatible = "nvidia,tegra20-pinmux";
reg = <0x70000014 0x10> /* Tri-state registers */
<0x70000080 0x20> /* Mux registers */
<0x700000a0 0x14> /* Pull-up/down registers */
<0x70000868 0xa8>; /* Pad control registers */
pinmux_i2c1_spdio_active: pinmux-i2c1-spdio-active {
reg-init =
/* Set pingroup SPDO to function I2C1 */
<
1 /* Register "bank" in controller */
0x8c /* Register offset */
0x000000c0 /* Write bitmask, 0 for whole register */
0x00000080 /* Write value */
>
/* Set pingroup SPDI to function I2C1 */
<
1 /* Register "bank" in controller */
0x8c /* Register offset */
0x00000300 /* Write bitmask, 0 for whole register */
0x00000200 /* Write value */
>;
};
pinmux_i2c1_spdio_suspend: pinmux-i2c1-spdio-suspend {
...
};
};
tegra-seaboard.dts:
i2c@7000c000 {
/*
* These are phandles to nodes that containthe reg-init list. Can
* we have phandles to properties instead? Then we wouldn't need
* all those nodes which each just contains 1 property.
*/
pinctrl = <&pinmux_i2c1_spdio_active &pinmux_i2c1_spdio_suspend>;
pinctrl-names = "active", "suspend";
};
In order to support programming more than one pin controller at once, we
could include the phandle of the pin controller in the "reg-init" property:
somewhere-other-than-under-a-pin-controller {
pinmux-foo {
reg-init =
<
&tegra_pmx /* Pin controller */
2 /* Number of entries for this controller */
>
<
1 /* Register "bank" in controller */
0x8c /* Register offset */
0x000000c0 /* Write bitmask, 0 for whole register */
0x00000080 /* Write value */
>
<
1 /* Register "bank" in controller */
0x8c /* Register offset */
0x00000300 /* Write bitmask, 0 for whole register */
0x00000200 /* Write value */
>
<
&other_pmx /* Pin controller */
1 /* Number of entries for this controller */
>
<
0 /* Register "bank" in controller */
0x04 /* Register offset */
0x000000ff /* Write bitmask, 0 for whole register */
0x0000005a /* Write value */
>;
}
};
When parsing a reg-init property, we know if it contains phandles by:
* If node containing property is a child of a pin controller, the property
doesn't containt pin controller phandles.
* Otherwise, it does.
What are people's thoughts. Thinking about all the issues involved, this
is certainly the approach I like most right now.
--
nvpublic
lol, great, and it may be more generic by define register update routines.
some boards may need different pad settings in suspend.
some quirks or work around for hw issues may be out kernel this way.
richard
Hi Stephen,
On 4 February 2012 21:31, Stephen Warren <[email protected]> wrote:
> Sorry, I haven't had a chance to read any of the pincrl emails from
> Friday onwards. However, I thought a bit more about this, and decided
> to propose someting much simpler:
>
> Thoughts:
>
> * Defining all the pins, groups, functions, ... takes a lot of space,
> ?whether it's in static data in pinctrl drivers or in the device tree.
> ?The lists must also be stored in RAM at runtime.
>
> * It's been very difficult to come up with a generic description of all
> ?pin controller's capabilities. This is true even irrespective of device
> ?tree; think pin config where we've agonized over whether we can create
> ?a standardized list of pin config properties, or need to allow each
> ?pinctrl driver to define its own set of properties, etc.
>
> * The only real use of the lists is for debugfs. Drivers shouldn't expect
> ?to directly request specific pinctrl settings, since that would encode
> ?knowledge of an individual SoC's pin controller. This should be
> ?abstracted from drivers.
>
> * The data in debugfs could easily be replaced by a raw register dump
> ?coupled with a SoC-specific script to print out what each register
> ?means.
>
> My proposal below is to radically simplify the pinctrl subsystem, and
> make it little more than a system to execute a list of arbitrary register
> writes.
Thanks for your work on pinctrl bindings proposal.
With this new approach, how much of the pinctrl susbystem is used in
device tree mode. Last time I had proposed something similar to this
(http://marc.info/?l=linux-kernel&m=132697880016289&w=2), you were not
happy about that approach since the pinctrl subsystem is largely
under-utilized (http://marc.info/?l=linux-kernel&m=132735884622374&w=2),
expect for providing a interface which individual
pinctrl/pinmux/pinconfig drivers will use to implement a function that
programs the hardware.
There need not be pinmux/pinctrl/pinconfig bindings that are designed
for the linux pinctrl subsystem. DT allows specifying the
pinconfig/pinmux properties in a simple way, which was not possible in
non-dt case and hence the pinctrl subsystem. Other OS'es which might
not have a pinctrl subsystem, similar to what linux has, should also
find the pinctrl bindings useful.
Regards,
Thomas.
>
> Advantages:
>
> * No need to define any kind of data model for pinctrl.
>
> * No need to store any list of pins/groups/function/parameters/... either
> ?in static data, in device tree, or at run-time.
>
> * No need to store the the selected board-specific settings anywhere either.
>
> * Completely generic; can handle anything that exists now or in the future.
>
> * Handles pin mux and pin config identically.
>
> * Extremely simple to implement and understand; probably just a few K
> ?of code and no data. I assume this will be very appealing to those
> ?interested in using the binding within U-Boot too.
>
> * The lists of register writes can just as easily be provided by board
> ?files as device tree, so operation should be identical.
>
> Disadvantages:
>
> * Creating the list of register values/masks may be a little painful. If
> ?we move forward with this, we'd want to strongly push dtc towards
> ?providing named constants, expressesion, macros, etc. Those dtc features
> ?would be very useful anyway.
>
> * Lack of high-level semantic meaning to the data; but I assert there's
> ?little benefit to having that anyway.
>
> ?To solve this, write a script to parse each pinctrl driver's regcache
> ?file and print out all the register meanings.
>
> * This scheme wouldn't represent which device "owns" which pins/groups,
> ?nor be able to validate that different devices' pinmux settings don't
> ?conflict.
>
> ?I don't think this is a huge issue though, since that's mainly error-
> ?checking. Any problems should be just as obvious during testing whether
> ?they cause the HW to fail to operate correctly, or whether they cause
> ?pinctrl to throw an error. Equally, there are many error conditions
> ?that pinctrl core wasn't checking for anyway.
>
> Precedent:
>
> * There was previous discussion on using this exact technique for pinmux
> ?at a previous Linaro Connect:
>
> ?http://www.spinics.net/lists/linux-tegra/msg01943.html
>
> * There is an existing binding that works very similarly, for the Mavell
> ?PHY:
>
> ?http://www.gossamer-threads.com/lists/linux/kernel/1305624
> ?drivers/net/phy/marvell.c
>
> ?Explicit mention was made here that a simple list of register writes
> ?avoiding the complexity of representing a huge number of combinations
> ?in the device tree.
>
> tegra20.dtsi:
>
> pinmux: pinmux@70000000 {
> ? ? ? ?compatible = "nvidia,tegra20-pinmux";
> ? ? ? ?reg = <0x70000014 0x10> ?/* Tri-state registers */
> ? ? ? ? ? ? ?<0x70000080 0x20> ?/* Mux registers */
> ? ? ? ? ? ? ?<0x700000a0 0x14> ?/* Pull-up/down registers */
> ? ? ? ? ? ? ?<0x70000868 0xa8>; /* Pad control registers */
>
> ? ? ? ?pinmux_i2c1_spdio_active: pinmux-i2c1-spdio-active {
> ? ? ? ? ? ? ? ?reg-init =
> ? ? ? ? ? ? ? ? ? ?/* Set pingroup SPDO to function I2C1 */
> ? ? ? ? ? ? ? ? ? ?<
> ? ? ? ? ? ? ? ? ? ? ? ?1 ? ? ? ? ?/* Register "bank" in controller */
> ? ? ? ? ? ? ? ? ? ? ? ?0x8c ? ? ? /* Register offset */
> ? ? ? ? ? ? ? ? ? ? ? ?0x000000c0 /* Write bitmask, 0 for whole register */
> ? ? ? ? ? ? ? ? ? ? ? ?0x00000080 /* Write value */
> ? ? ? ? ? ? ? ? ? ?>
> ? ? ? ? ? ? ? ? ? ?/* Set pingroup SPDI to function I2C1 */
> ? ? ? ? ? ? ? ? ? ?<
> ? ? ? ? ? ? ? ? ? ? ? ?1 ? ? ? ? ?/* Register "bank" in controller */
> ? ? ? ? ? ? ? ? ? ? ? ?0x8c ? ? ? /* Register offset */
> ? ? ? ? ? ? ? ? ? ? ? ?0x00000300 /* Write bitmask, 0 for whole register */
> ? ? ? ? ? ? ? ? ? ? ? ?0x00000200 /* Write value */
> ? ? ? ? ? ? ? ? ? ?>;
> ? ? ? ?};
> ? ? ? ?pinmux_i2c1_spdio_suspend: pinmux-i2c1-spdio-suspend {
> ? ? ? ? ? ? ? ?...
> ? ? ? ?};
> };
>
> tegra-seaboard.dts:
>
> i2c@7000c000 {
> ? ? ? ?/*
> ? ? ? ? * These are phandles to nodes that containthe reg-init list. Can
> ? ? ? ? * we have phandles to properties instead? Then we wouldn't need
> ? ? ? ? * all those nodes which each just contains 1 property.
> ? ? ? ? */
> ? ? ? ?pinctrl = <&pinmux_i2c1_spdio_active &pinmux_i2c1_spdio_suspend>;
> ? ? ? ?pinctrl-names = "active", "suspend";
> };
>
> In order to support programming more than one pin controller at once, we
> could include the phandle of the pin controller in the "reg-init" property:
>
> somewhere-other-than-under-a-pin-controller {
> ? ? ? ?pinmux-foo {
> ? ? ? ? ? ? ? ?reg-init =
> ? ? ? ? ? ? ? ? ? ?<
> ? ? ? ? ? ? ? ? ? ? ? ?&tegra_pmx /* Pin controller */
> ? ? ? ? ? ? ? ? ? ? ? ?2 ? ? ? ? ?/* Number of entries for this controller */
> ? ? ? ? ? ? ? ? ? ?>
> ? ? ? ? ? ? ? ? ? ?<
> ? ? ? ? ? ? ? ? ? ? ? ?1 ? ? ? ? ?/* Register "bank" in controller */
> ? ? ? ? ? ? ? ? ? ? ? ?0x8c ? ? ? /* Register offset */
> ? ? ? ? ? ? ? ? ? ? ? ?0x000000c0 /* Write bitmask, 0 for whole register */
> ? ? ? ? ? ? ? ? ? ? ? ?0x00000080 /* Write value */
> ? ? ? ? ? ? ? ? ? ?>
> ? ? ? ? ? ? ? ? ? ?<
> ? ? ? ? ? ? ? ? ? ? ? ?1 ? ? ? ? ?/* Register "bank" in controller */
> ? ? ? ? ? ? ? ? ? ? ? ?0x8c ? ? ? /* Register offset */
> ? ? ? ? ? ? ? ? ? ? ? ?0x00000300 /* Write bitmask, 0 for whole register */
> ? ? ? ? ? ? ? ? ? ? ? ?0x00000200 /* Write value */
> ? ? ? ? ? ? ? ? ? ?>
> ? ? ? ? ? ? ? ? ? ?<
> ? ? ? ? ? ? ? ? ? ? ? ?&other_pmx /* Pin controller */
> ? ? ? ? ? ? ? ? ? ? ? ?1 ? ? ? ? ?/* Number of entries for this controller */
> ? ? ? ? ? ? ? ? ? ?>
> ? ? ? ? ? ? ? ? ? ?<
> ? ? ? ? ? ? ? ? ? ? ? ?0 ? ? ? ? ?/* Register "bank" in controller */
> ? ? ? ? ? ? ? ? ? ? ? ?0x04 ? ? ? /* Register offset */
> ? ? ? ? ? ? ? ? ? ? ? ?0x000000ff /* Write bitmask, 0 for whole register */
> ? ? ? ? ? ? ? ? ? ? ? ?0x0000005a /* Write value */
> ? ? ? ? ? ? ? ? ? ?>;
> ? ? ? ?}
> };
>
> When parsing a reg-init property, we know if it contains phandles by:
> * If node containing property is a child of a pin controller, the property
> ?doesn't containt pin controller phandles.
> * Otherwise, it does.
>
> What are people's thoughts. Thinking about all the issues involved, this
> is certainly the approach I like most right now.
>
> --
> nvpublic
>
On Sun, Feb 5, 2012 at 6:31 AM, Stephen Warren <[email protected]> wrote:
> Sorry, I haven't had a chance to read any of the pincrl emails from
> Friday onwards. However, I thought a bit more about this, and decided
> to propose someting much simpler:
I think this approach is quite interesting, but since it removes more
or less all semantic meaning of the pinctrl system, I don't see why
it would be a pinctrl rewrite at all or called pinctrl, rather it'd be
something new and separate, since it could (if I understand
correctly) very well be used to configure things that are totally
unrelated to pin control, could it not?
It is indeed described as "little more than a system to execute a
list of arbitrary register writes".
For example it could be readily used to set up the config registers
of parameters in an external memory controller - these are also a lot
of magic writes into various registers, nobody really knows what they
actually do, timings for access and whatnot.
Incidentally external memory controllers is another things that
is usually magically reconfigured at idle/sleep and on the way
back up.
Can we defined it then as controlled register processing at
determined points, some during boot, some at runtime, and
maybe tied to certain devices at certain points in time?
A controlled set of register read/writes and maybe also conditionals
(if that bit is 1, do this, else do that, plus a loop command to wait
for a flag or similar) are known as a "jam tables" and usually used
in BIOSes to do a compact machine initialization. I learned this term
in Bunnie Huang's "Hacking the Xbox, where he describes finding a
jam table interpreter in the Xbox ROM.
So if I understand the proposal correctly it would fit nice as
drivers/jamtable or so, and could do that kind of control of whatever
hardware you want to control with opaque register accesses,
then reflect it as raw register contents in debugfs etc.
If it turns out everyone is happy with that and we can move other
drivers over to it we can just scrap pinctrl, or have it only for those
systems that want a semantic representation.
While I would probably mourn the death of sematics I also see
that if the goal is to get huge static data sets out of the kernel,
something like this may be the best way to get there.
Thoughts?
Linus Walleij
On 02/05/2012 07:07 PM, Thomas Abraham wrote:
> Hi Stephen,
>
> On 4 February 2012 21:31, Stephen Warren <[email protected]> wrote:
>> Sorry, I haven't had a chance to read any of the pincrl emails from
>> Friday onwards. However, I thought a bit more about this, and decided
>> to propose someting much simpler:
>>
>> Thoughts:
>>
>> * Defining all the pins, groups, functions, ... takes a lot of space,
>> whether it's in static data in pinctrl drivers or in the device tree.
>> The lists must also be stored in RAM at runtime.
>>
>> * It's been very difficult to come up with a generic description of all
>> pin controller's capabilities. This is true even irrespective of device
>> tree; think pin config where we've agonized over whether we can create
>> a standardized list of pin config properties, or need to allow each
>> pinctrl driver to define its own set of properties, etc.
>>
>> * The only real use of the lists is for debugfs. Drivers shouldn't expect
>> to directly request specific pinctrl settings, since that would encode
>> knowledge of an individual SoC's pin controller. This should be
>> abstracted from drivers.
>>
>> * The data in debugfs could easily be replaced by a raw register dump
>> coupled with a SoC-specific script to print out what each register
>> means.
>>
>> My proposal below is to radically simplify the pinctrl subsystem, and
>> make it little more than a system to execute a list of arbitrary register
>> writes.
>
> Thanks for your work on pinctrl bindings proposal.
>
> With this new approach, how much of the pinctrl susbystem is used in
> device tree mode.
Probably not a lot of the actual implementation. I'd assume that the
APIs called by device drivers would remain constant, or roughly so, in
order to still provide a simple interface for drivers.
> Last time I had proposed something similar to this
> (http://marc.info/?l=linux-kernel&m=132697880016289&w=2), you were not
> happy about that approach since the pinctrl subsystem is largely
> under-utilized (http://marc.info/?l=linux-kernel&m=132735884622374&w=2),
> expect for providing a interface which individual
> pinctrl/pinmux/pinconfig drivers will use to implement a function that
> programs the hardware.
That's true.
AS LinusW says in his later reply, losing the semantic representation in
the pinctrl system is a pity, which is the main reasoning behind my
previous response. However, I'm beginning to lean towards the simplicity
of something like a list of register writes trumping the lack of semantics.
> There need not be pinmux/pinctrl/pinconfig bindings that are designed
> for the linux pinctrl subsystem. DT allows specifying the
> pinconfig/pinmux properties in a simple way, which was not possible in
> non-dt case and hence the pinctrl subsystem. Other OS'es which might
> not have a pinctrl subsystem, similar to what linux has, should also
> find the pinctrl bindings useful.
True.
I think that pinctrl in a non-DT system probably could do something like
this DT binding proposal though, so we need not have DT/non-DT work
differently.
--
nvpublic
On 02/05/2012 08:20 PM, Linus Walleij wrote:
> On Sun, Feb 5, 2012 at 6:31 AM, Stephen Warren <[email protected]> wrote:
>
>> Sorry, I haven't had a chance to read any of the pincrl emails from
>> Friday onwards. However, I thought a bit more about this, and decided
>> to propose someting much simpler:
>
> I think this approach is quite interesting, but since it removes more
> or less all semantic meaning of the pinctrl system, I don't see why
> it would be a pinctrl rewrite at all or called pinctrl, rather it'd be
> something new and separate, since it could (if I understand
> correctly) very well be used to configure things that are totally
> unrelated to pin control, could it not?
I guess we could indeed use this technique for almost anything.
I think the main argument to call it pinctrl/pinmux still is to provide
some named API and reason for drivers to invoke when they wanted to make
use of the feature.
In other words, it's pretty easy to see why and when a driver would
invoke a pin control API to configure the HW surrounding the HW module
that driver controls. If we don't call that pinctrl, what else might we
call it? That said, I'm sure we can come up with some reasonable more
general-purpose name.
...
> Can we defined it then as controlled register processing at
> determined points, some during boot, some at runtime, and
> maybe tied to certain devices at certain points in time?
Certainly, yes.
> A controlled set of register read/writes and maybe also conditionals
> (if that bit is 1, do this, else do that, plus a loop command to wait
> for a flag or similar) are known as a "jam tables" and usually used
> in BIOSes to do a compact machine initialization. I learned this term
> in Bunnie Huang's "Hacking the Xbox, where he describes finding a
> jam table interpreter in the Xbox ROM.
I think anything beyond a simple linear list of register writes would
get a /lot/ of pushback. See for example Grant's comments in one of the
links I referenced:
http://www.gossamer-threads.com/lists/linux/kernel/1305624
about delays in the PHY initialization sequence being "right out".
I can imagine the data including flags like 8/16/32/64-bit register
accesses, or read-modify-write vs. just write (i.e. do we need to
include a mask or not) being reasonable, but any state, looping, delay,
conditionals etc. being nak'd. Otherwise, we'd be inventing a byte code
interpreter instead of just a list of register writes. If we /did/ want
to go that route (and I suspect there's no need...) then I think we'd
want to re-use some existing byte code definition rather than creating a
new one.
...
> While I would probably mourn the death of sematics I also see
> that if the goal is to get huge static data sets out of the kernel,
> something like this may be the best way to get there.
Yes, the loss of semantics also doesn't entirely appeal to me. However,
I wonder if the other advantages don't outweigh that.
--
nvpublic
On Mon, Feb 6, 2012 at 6:53 AM, Stephen Warren <[email protected]> wrote:
> I think the main argument to call it pinctrl/pinmux still is to provide
> some named API and reason for drivers to invoke when they wanted to make
> use of the feature.
>
> In other words, it's pretty easy to see why and when a driver would
> invoke a pin control API to configure the HW surrounding the HW module
> that driver controls. If we don't call that pinctrl, what else might we
> call it? That said, I'm sure we can come up with some reasonable more
> general-purpose name.
I feel pinctrl would be misleading and overly specific. Maybe something
like "hwstates" or "initvectors" or similar is more to the point. Or
just "soc-bios" :-)
An elegant way of doing it would be to hide the current pinctrl calls
behind the new API, say
hws = hardware_state_get(dev);
hardware_state_enable(hws);
hardware_state_disable(hws);
hardware_state_put(hws);
Then these can boil down to simple register read/writes or divert to
pinctrl or pinconfig.
>> A controlled set of register read/writes and maybe also conditionals
>> (...)
>
> I think anything beyond a simple linear list of register writes would
> get a /lot/ of pushback. See for example Grant's comments in one of the
> links I referenced:
OK. Keeping it simple is best then I guess.
> I can imagine the data including flags like 8/16/32/64-bit register
> accesses, or read-modify-write vs. just write (i.e. do we need to
> include a mask or not) being reasonable, but any state, looping, delay,
> conditionals etc. being nak'd.
Sure.
I have this mux on the AB8500 on the Ux500 that is on I2C.
So this off-chip device can mux its pins between GPIO and some
other functions. So I'd need something that can provide a
read/write function handle or so rather than plain register writes.
Or is this concept only for memory-mapped stuff?
>> While I would probably mourn the death of sematics I also see
>> that if the goal is to get huge static data sets out of the kernel,
>> something like this may be the best way to get there.
>
> Yes, the loss of semantics also doesn't entirely appeal to me. However,
> I wonder if the other advantages don't outweigh that.
I will certainly finalize the pinctrl subsystem as-is, adding the
pin configurations states as the last major piece. If for nothing
else it provides some understanding of the problem space.
I think we should keep both for the time being and consider the
alternative approach when patches appear. So if/when someone
creates a new subsystem like this, drivers can move over to it on a
per-driver basis. If there are zero drivers left in pinctrl it can be
deleted.
Yours,
Linus Walleij
* Stephen Warren <[email protected]> [120204 21:01]:
> Sorry, I haven't had a chance to read any of the pincrl emails from
> Friday onwards. However, I thought a bit more about this, and decided
> to propose someting much simpler:
>
> Thoughts:
>
> * Defining all the pins, groups, functions, ... takes a lot of space,
> whether it's in static data in pinctrl drivers or in the device tree.
> The lists must also be stored in RAM at runtime.
>
> * It's been very difficult to come up with a generic description of all
> pin controller's capabilities. This is true even irrespective of device
> tree; think pin config where we've agonized over whether we can create
> a standardized list of pin config properties, or need to allow each
> pinctrl driver to define its own set of properties, etc.
>
> * The only real use of the lists is for debugfs. Drivers shouldn't expect
> to directly request specific pinctrl settings, since that would encode
> knowledge of an individual SoC's pin controller. This should be
> abstracted from drivers.
>
> * The data in debugfs could easily be replaced by a raw register dump
> coupled with a SoC-specific script to print out what each register
> means.
My conclusions are pretty much the same: The data is only needed in
the driver for debugging. And the register names and values are best
translated into readable format using debugfs and userspace tools.
I pretty much did this with the pinctrl-simple.c I posted few days
ago, except no userspace debugging support yet in pinctrl framework
naturally.
This all seems to fit into the current pinctrl framework OK.
I think we just need to make string names optional data, and
structure things in debugfs to just display register physical
addresses rather than string names for userspace tools.
FYI, I'm currently working around the names by using the hex
register physical address as the pin name.
Also, the alternative pin modes bindings still needs to be
discussed, so maybe we all can talk about that at Linaro Connect
on Tuesday at some point.
Regards,
Tony
* Linus Walleij <[email protected]> [120206 08:58]:
> On Mon, Feb 6, 2012 at 6:53 AM, Stephen Warren <[email protected]> wrote:
>
> I will certainly finalize the pinctrl subsystem as-is, adding the
> pin configurations states as the last major piece. If for nothing
> else it provides some understanding of the problem space.
>
> I think we should keep both for the time being and consider the
> alternative approach when patches appear. So if/when someone
> creates a new subsystem like this, drivers can move over to it on a
> per-driver basis. If there are zero drivers left in pinctrl it can be
> deleted.
Yes it seems that we can easily do both. So far the only
change needed for pinctrl drivers containing no data is that
we should make the string names optional and structure debugfs
around the physical register addresses instead. I'm basically
just setting the mux register physcal address as the pin name
for now to work around this.
Regards,
Tony
I like the general approach of simplifying the pinctrl thing, as the
previous approach did not appear to be converging.
One possible name would be "gpconfig" - for general purpose configuration.
The register access model in the strawman proposal is probably too
simple. 32-bit memory mapped registers are certainly the most common
subcase on ARM, but there are many other cases that occur in practice:
* Registers that must be accessed with 8, 16, or 64-bit cycles.
* Registers that have side effects on read, so read-mask-write must be
avoided
* Registers accessed via an index/data cycle pair, thus having locking
requirements
* Registers that must be read after being written, or otherwise
requiring some sort of memory-ordering enforcement.
* Time delays between pairs of writes
* PCI configuration registers, which often have some combination of the
above
* Registers behind serial buses like I2C
Both Open Firmware and ACPI have addressed this general problem. In
addition to a numeric identifier for the register, you need to specify
the access semantics. It's difficult to finitely enumerate all possible
cases, but you can get to 99.9% with a modest number of access models,
and then add new models as needed.
On Mon, Feb 6, 2012 at 8:05 PM, Mitch Bradley <[email protected]> wrote:
> I like the general approach of simplifying the pinctrl thing, as the
> previous approach did not appear to be converging.
pinctrl as such is upstream, widely ACKed and quite converged I'd say.
But the Device Tree bindings and general path to get data out of the drivers
and board files are not (yet) converging...
> Both Open Firmware and ACPI have addressed this general problem. ?In
> addition to a numeric identifier for the register, you need to specify the
> access semantics. ?It's difficult to finitely enumerate all possible cases,
> but you can get to 99.9% with a modest number of access models, and then add
> new models as needed.
This is interesting. So are you referring to a piece of Open Firmware
that is not in the Device Tree? Since this is all about device tree that
comes from OF, we might be reinventing the wheel.
Can you give some pointers?
Indeed it seems related to ACPI or some BIOS stuff on
non-embedded systems.
Yours,
Linus Walleij
On Sun, Feb 05, 2012 at 09:53:38PM -0800, Stephen Warren wrote:
> On 02/05/2012 08:20 PM, Linus Walleij wrote:
> > A controlled set of register read/writes and maybe also conditionals
> > (if that bit is 1, do this, else do that, plus a loop command to wait
> > for a flag or similar) are known as a "jam tables" and usually used
> > in BIOSes to do a compact machine initialization. I learned this term
> > in Bunnie Huang's "Hacking the Xbox, where he describes finding a
> > jam table interpreter in the Xbox ROM.
FWIW I just added a subset of this functionality (called "patches" for
want of a better name) to regmap, just a simple list of register writes
that get blasted in htere. The intent is somewhat different, though -
it's there for dumping undocumented or partially documented register
write sequences from vendors into devices since there's a common pattern
of doing that when bringing things out of reset, more like what Grant
seems to have been talking about in that thread.
On Mon, Feb 6, 2012 at 8:03 PM, Tony Lindgren <[email protected]> wrote:
> So far the only
> change needed for pinctrl drivers containing no data is that
> we should make the string names optional and structure debugfs
> around the physical register addresses instead. I'm basically
> just setting the mux register physcal address as the pin name
> for now to work around this.
OK please make a patch to do it really optional in the core if
you find the time, it seems like a good change anyway, because
it will make it possible to reduce some current pin name lists
quite easily.
If you need to change the layout of debugfs just do it.
I actually had something like unnamed pins in the early patches
to register a bunch of anonymous pins ranges, so why not bring
it back in.
Thanks,
Linus Walleij
* Linus Walleij <[email protected]> [120206 11:25]:
> On Mon, Feb 6, 2012 at 8:03 PM, Tony Lindgren <[email protected]> wrote:
>
> > So far the only
> > change needed for pinctrl drivers containing no data is that
> > we should make the string names optional and structure debugfs
> > around the physical register addresses instead. I'm basically
> > just setting the mux register physcal address as the pin name
> > for now to work around this.
>
> OK please make a patch to do it really optional in the core if
> you find the time, it seems like a good change anyway, because
> it will make it possible to reduce some current pin name lists
> quite easily.
OK, will take a look at that.
> If you need to change the layout of debugfs just do it.
>
> I actually had something like unnamed pins in the early patches
> to register a bunch of anonymous pins ranges, so why not bring
> it back in.
Yeah it seems that the mux registers should be listed, it might
require a little bit of thinking for cases where one register
controls multiple pins. So maybe we need just a new entry for
mux registers?
Regards,
Tony
On 2/6/2012 9:26 AM, Linus Walleij wrote:
> On Mon, Feb 6, 2012 at 8:05 PM, Mitch Bradley<[email protected]> wrote:
>
>> I like the general approach of simplifying the pinctrl thing, as the
>> previous approach did not appear to be converging.
>
> pinctrl as such is upstream, widely ACKed and quite converged I'd say.
>
> But the Device Tree bindings and general path to get data out of the drivers
> and board files are not (yet) converging...
>
>> Both Open Firmware and ACPI have addressed this general problem. In
>> addition to a numeric identifier for the register, you need to specify the
>> access semantics. It's difficult to finitely enumerate all possible cases,
>> but you can get to 99.9% with a modest number of access models, and then add
>> new models as needed.
>
> This is interesting. So are you referring to a piece of Open Firmware
> that is not in the Device Tree? Since this is all about device tree that
> comes from OF, we might be reinventing the wheel.
Open Firmware in general is a complete firmware system, with features
for system initialization, bootloading, diagnostics (for factory, field
service, and end users), system maintenance functions like OS upgrades,
and debugging tools for hardware, the firmware itself, and the OS. It
exports a set of callbacks that OF-aware OSs (e.g. Solaris) can use to
perform demand-loading of driver modules, even for core drivers like the
filesystem root device. It is fully programmable, including an
interpreter, incremental compiler, decompiler, assembler, disassembler,
and both source-level and assembly language debugging. Modern
implementations include complete network stacks and support for numerous
filesystems.
Its device tree serves not only to communicate information to the OS,
but also as the framework for OF's driver model. In addition to the
descriptive properties, device nodes contain executable methods that
implement the device drivers that support the feature set listed above.
The device drivers can either be hard-compiled or demand-loaded from
"option ROMs" on plug-in cards. They are encoded using "FCode", a
processor and platform neutral byte-coded representation of Forth source
code.
The register access generalization that I was talking about is part of
that driver model. Each bus driver implements a set of register access
methods that apply to its children. The child drivers invoke those
methods from the parent, without having to know any platform-specific
details.
The register access model is based on executable methods rather than on
descriptive data in properties. In many cases, those methods use info
in the properties, including "reg", "ranges" and "#address-cells", but
the methods can also encode semantic requirements that are difficult to
describe as pure data.
To fully solve all the problems that eventually arise, you end up
needing a programming language, which is why Open Firmware was built
from the ground up around a programming language core.
Of course, it's usually possible to solve a very large subset of
problems with a sufficiently-elaborate data description.
> Can you give some pointers?
The definitive core document is IEEE 1275-1994, but that document is 18
years old and is not really the best way to learn about Open Firmware.
(The status of that document with respect to IEEE is "withdrawn". What
that really means is that we (the group that developed it) got fed up
with the IEEE process and declined to spend the time, money, and effort
necessary to comply with IEEE's public voting process for
"reaffirmation" every N years.)
Here are some pointers to information about modern implementations and
usage:
http://wiki.laptop.org/go/Open_Firmware
http://wiki.laptop.org/go/Forth_Lessons
http://www.openfirmware.info
One Laptop Per Child uses Open Firmware on both x86 and ARM systems.
>
> Indeed it seems related to ACPI or some BIOS stuff on
> non-embedded systems.
>
> Yours,
> Linus Walleij
>
>
On Mon, Feb 6, 2012 at 10:04 PM, Tony Lindgren <[email protected]> wrote:
>> I actually had something like unnamed pins in the early patches
>> to register a bunch of anonymous pins ranges, so why not bring
>> it back in.
>
> Yeah it seems that the mux registers should be listed, it might
> require a little bit of thinking for cases where one register
> controls multiple pins. So maybe we need just a new entry for
> mux registers?
I'm not sure if I'm following completely, if this is inside the devicetree-based
driver file, would it work to just add a struct dentry * to the
pinctrl_desc where you put a per-driver file?
Or maybe add extern void pinctrl_add_debugfs(struct dentry *) that adds
a new file to the existing per-driver directory through the core and then
have this add that file?
Or did you mean that the core.c should be register-aware?
Yours,
Linus Walleij
* Linus Walleij <[email protected]> [120206 14:44]:
> On Mon, Feb 6, 2012 at 10:04 PM, Tony Lindgren <[email protected]> wrote:
>
> >> I actually had something like unnamed pins in the early patches
> >> to register a bunch of anonymous pins ranges, so why not bring
> >> it back in.
> >
> > Yeah it seems that the mux registers should be listed, it might
> > require a little bit of thinking for cases where one register
> > controls multiple pins. So maybe we need just a new entry for
> > mux registers?
>
> I'm not sure if I'm following completely, if this is inside the devicetree-based
> driver file, would it work to just add a struct dentry * to the
> pinctrl_desc where you put a per-driver file?
I was thinking generic debufs entries for all drivers.
> Or maybe add extern void pinctrl_add_debugfs(struct dentry *) that adds
> a new file to the existing per-driver directory through the core and then
> have this add that file?
Sounds like you've thought it further than me already :)
Maybe that's the way to go to solve the one register for
multiple pins issue.
> Or did you mean that the core.c should be register-aware?
I was just thinking string name ignoring core.c, so that would
be the pinctrl_add_debugfs() option then. Do you see problems
with this approach?
Regards,
Tony
On Tue, Feb 7, 2012 at 12:57 AM, Tony Lindgren <[email protected]> wrote:
>> I'm not sure if I'm following completely, if this is inside the devicetree-based
>> driver file, would it work to just add a struct dentry * to the
>> pinctrl_desc where you put a per-driver file?
>
> I was thinking generic debufs entries for all drivers.
OK.
>> Or maybe add extern void pinctrl_add_debugfs(struct dentry *) that adds
>> a new file to the existing per-driver directory through the core and then
>> have this add that file?
>
> Sounds like you've thought it further than me already :)
>
> Maybe that's the way to go to solve the one register for
> multiple pins issue.
>
>> Or did you mean that the core.c should be register-aware?
>
> I was just thinking string name ignoring core.c, so that would
> be the pinctrl_add_debugfs() option then. Do you see problems
> with this approach?
No probs, something like that you call any number of times
after registering the core device should work fine I guess.
Yours,
Linus Walleij
On 02/06/2012 11:03 AM, Tony Lindgren wrote:
> * Linus Walleij <[email protected]> [120206 08:58]:
>> On Mon, Feb 6, 2012 at 6:53 AM, Stephen Warren <[email protected]> wrote:
>>
>> I will certainly finalize the pinctrl subsystem as-is, adding the
>> pin configurations states as the last major piece. If for nothing
>> else it provides some understanding of the problem space.
>>
>> I think we should keep both for the time being and consider the
>> alternative approach when patches appear. So if/when someone
>> creates a new subsystem like this, drivers can move over to it on a
>> per-driver basis. If there are zero drivers left in pinctrl it can be
>> deleted.
>
> Yes it seems that we can easily do both. So far the only
> change needed for pinctrl drivers containing no data is that
> we should make the string names optional and structure debugfs
> around the physical register addresses instead. I'm basically
> just setting the mux register physcal address as the pin name
> for now to work around this.
I was thinking that since there was just a plain list of register
writes, there wouldn't be any concept of pins, groups, functions, etc.
at all. As such, it wouldn't really fit into pinctrl as-is; it'd need to
be either something separate, or pinctrl to change substantially more
than just allowing unnamed pins, wouldn't it?
--
nvpublic
On 02/06/2012 11:05 AM, Mitch Bradley wrote:
> I like the general approach of simplifying the pinctrl thing, as the
> previous approach did not appear to be converging.
>
> One possible name would be "gpconfig" - for general purpose configuration.
Sounds reasonable
> The register access model in the strawman proposal is probably too
> simple. 32-bit memory mapped registers are certainly the most common
> subcase on ARM, but there are many other cases that occur in practice:
>
> * Registers that must be accessed with 8, 16, or 64-bit cycles.
> * Registers that have side effects on read, so read-mask-write must be
> avoided
> * Registers accessed via an index/data cycle pair, thus having locking
> requirements
> * Registers that must be read after being written, or otherwise
> requiring some sort of memory-ordering enforcement.
> * Time delays between pairs of writes
> * PCI configuration registers, which often have some combination of the
> above
> * Registers behind serial buses like I2C
>
> Both Open Firmware and ACPI have addressed this general problem. In
> addition to a numeric identifier for the register, you need to specify
> the access semantics. It's difficult to finitely enumerate all possible
> cases, but you can get to 99.9% with a modest number of access models,
> and then add new models as needed.
My thinking was that each driver that permitted execution of these
register write sequences on its registers would provide an extremely
simple driver to support this. Perhaps just one "op" function that
implemented each write, and took care of details such as timing delays
for the IO, indexed addressing, using I2C instead of memory-mapped,
perhaps register access width, etc.
Some of the items in the list above could be solved by explicit flags in
the data instead (register access width comes to mind for memory-mapped
devices at least).
Do you have any direct pointers to how OF and ACPI solved this kind of
thing; it'd be good background.
--
nvpublic
On 2/6/2012 7:33 PM, Stephen Warren wrote:
> On 02/06/2012 11:05 AM, Mitch Bradley wrote:
>> I like the general approach of simplifying the pinctrl thing, as the
>> previous approach did not appear to be converging.
>>
>> One possible name would be "gpconfig" - for general purpose configuration.
>
> Sounds reasonable
>
>> The register access model in the strawman proposal is probably too
>> simple. 32-bit memory mapped registers are certainly the most common
>> subcase on ARM, but there are many other cases that occur in practice:
>>
>> * Registers that must be accessed with 8, 16, or 64-bit cycles.
>> * Registers that have side effects on read, so read-mask-write must be
>> avoided
>> * Registers accessed via an index/data cycle pair, thus having locking
>> requirements
>> * Registers that must be read after being written, or otherwise
>> requiring some sort of memory-ordering enforcement.
>> * Time delays between pairs of writes
>> * PCI configuration registers, which often have some combination of the
>> above
>> * Registers behind serial buses like I2C
>>
>> Both Open Firmware and ACPI have addressed this general problem. In
>> addition to a numeric identifier for the register, you need to specify
>> the access semantics. It's difficult to finitely enumerate all possible
>> cases, but you can get to 99.9% with a modest number of access models,
>> and then add new models as needed.
>
> My thinking was that each driver that permitted execution of these
> register write sequences on its registers would provide an extremely
> simple driver to support this. Perhaps just one "op" function that
> implemented each write, and took care of details such as timing delays
> for the IO, indexed addressing, using I2C instead of memory-mapped,
> perhaps register access width, etc.
>
> Some of the items in the list above could be solved by explicit flags in
> the data instead (register access width comes to mind for memory-mapped
> devices at least).
>
> Do you have any direct pointers to how OF and ACPI solved this kind of
> thing; it'd be good background.
In ACPI, you declare an "OperationRegion" with a given "region space"
(an enumeration from SystemMemory, SystemIO, PCIConfig, SMBus,
EmbeddedControl, CMOS, PCIBARTarget) - ACPI spec section 17.5.89.
Inside that operation region, you declare various fields, further
specifying the subordinate access type from a region-dependent list.
Those access types include ByteAcc, WordAcc, DWordAcc, QWordAcc, and
BufferAcc. The field declaration further specifies whether or not a
mutex must be acquired, and a rule for what to do about unmodified bits
within the larger object containing the field. ACPI Spec 17.5.44
In OFW, each parent bus has a set of methods for performing
memory-mapped I/O operations on its addressable regions. The child
devices invoke a "map-in" method in the parent to acquire an address
token, then pass that token to access methods in the parent node. For
the common case, there are a set of fetch and store methods for the
usual sizes (8, 16, 32), and for more complex cases, the parent can define
whatever methods make sense for its needs. The methods are implemented
by byte code associated with the parent driver.