Pin control mappings for DT

Pin control mappings for DT

[Linus Walleij]

Greetings,

can we have some initial idea about DT support for pin control
devices and how these will look? Since pin control seems to be
a cruicial piece for people to get board data out of their kernels
we'd better have a common understanding.

The way I've understood it, we will need three pieces of
DT data:

1) Per-driver info, includes the pin controller base, its pins, their
 (optional) names and their specific presets like bias etc.

2) Board pin group definitions, saying "these three pins
  belong to the group "i2c0-pins"

3) Board function mapping definitions saying "the pin group
  'i2c0-pins' is to be used with device i2c0" which can map
  several groups to a single device.

(1) should be read out by each pin controller driver and
(2)+(3) by the pin controller, specifically  pinmux core.

I'm tentatively also thinking about:

(4) Pin group state map - as in "group 'i2c0-pins' have
  the states 'active', 'sleeping' and 'idle' "

Since this last piece needs to be connected to say
bias settings etc it's currently unclear to me how that
would work. But (1) thro (3) seem pretty solid.

Does this agree with others' idea about this?

(Feaing this thread will run on for a while, DT issues seem
pretty discussable.)

Yours,
Linus Walleij

Re: Pin control mappings for DT

[Tony Lindgren]

Hi,

* Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [111128 23:07]:
> Greetings,
> 
> can we have some initial idea about DT support for pin control
> devices and how these will look? Since pin control seems to be
> a cruicial piece for people to get board data out of their kernels
> we'd better have a common understanding.

Below is what I currently have for the pinmux-simple.c driver
I'm working on. That's easy to change though so please comment.
 
> The way I've understood it, we will need three pieces of
> DT data:
> 
> 1) Per-driver info, includes the pin controller base, its pins, their
>  (optional) names and their specific presets like bias etc.

	pinmux@4a100000 {
		compatible = "pinmux-simple";
		reg = <0x4a100000 0x194>;
		#address-cells = <1>;
		#size-cells = <0>;
		pinmux,access-width-16bit = <1>;

		pinmux-gpmc_ad0@4a100040 {
			reg = <0x4a100040>;
			pinmux,functions =
				"gpmc_ad0", "sdmmc2_dat0", "", "",
				"", "", "", "";
		};

		pinmux-uart3_rx_irrx@4a100144 {
			reg = <0x4a100144>;
			pinmux,functions =
				"uart3_rx_irrx", "dmtimer8_pwm_evt", "", "gpio_143",
				"", "", "", "safe_mode";
		};

		...
	};

 
> 2) Board pin group definitions, saying "these three pins
>   belong to the group "i2c0-pins"

These should be there for each driver, just like the interrupts:

	uart3: serial@48020000 {
		compatible = "ti,8250";
		reg = <0x48020000 0x100>;
		reg-shift = <2>;
		interrupts = < 106 >;
		pins = "uart3_rx_irrx.uart3_rx_irrx",
			"uart3_tx_irtx.uart3_tx_irtx";
	};

Note that for now the format for now is pin_register.function.
I guess we could have it set up with something like:

		pins = <&uart3_rx_irrx 7 0>;

But it easily becomes impossible for people to get it right.
Anybody got better suggestions here?

Also I don't have anything for setting the driver specific
default values yet.
 
> 3) Board function mapping definitions saying "the pin group
>   'i2c0-pins' is to be used with device i2c0" which can map
>   several groups to a single device.

I don't think is needed? The #2 above allows mapping them
automatically. However, the mapping needs to be fixed for the
pinctrl fwk to make this work with both static data DT driver
instances.
 
> (1) should be read out by each pin controller driver and
> (2)+(3) by the pin controller, specifically  pinmux core.

Yeah it makes sense to have the common code map the DT entries
to devices.
 
> I'm tentatively also thinking about:
> 
> (4) Pin group state map - as in "group 'i2c0-pins' have
>   the states 'active', 'sleeping' and 'idle' "
> 
> Since this last piece needs to be connected to say
> bias settings etc it's currently unclear to me how that
> would work. But (1) thro (3) seem pretty solid.

Setting the pin states seems to be unhandled in the pinctrl
fwk currently, right?

In many cases it's only the pin using device driver that knows
how the pins should be set for PM and pulls, so these could
be set by the default values in DT for each driver.

Then some pins need to be dynamically re-muxed for wake-up
events etc, so I guess adding generic 'wakeup' and 'disabled'
modes might do the trick as long as those won't touch the
pull values configured by the driver. For example, 'wakeup' can
be used data lines, such as serial rx line. In that case the
serial rx line would be temporarily muxed into a GPIO input
function with wake-up flags set for the pin.

Regards, 

Tony

RE: Pin control mappings for DT

[Stephen Warren]

Tony Lindgren wrote at Tuesday, November 29, 2011 10:43 AM:
> * Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [111128 23:07]:
> > Greetings,
> >
> > can we have some initial idea about DT support for pin control
> > devices and how these will look? Since pin control seems to be
> > a cruicial piece for people to get board data out of their kernels
> > we'd better have a common understanding.
...
> > 2) Board pin group definitions, saying "these three pins
> >   belong to the group "i2c0-pins"
> 
> These should be there for each driver, just like the interrupts:
> 
> 	uart3: serial@48020000 {
> 		compatible = "ti,8250";
> 		reg = <0x48020000 0x100>;
> 		reg-shift = <2>;
> 		interrupts = < 106 >;
> 		pins = "uart3_rx_irrx.uart3_rx_irrx",
> 			"uart3_tx_irtx.uart3_tx_irtx";
> 	};

I don't agree here. I think that pinmux data is more strongly related
to the pinmux controller HW than the HW blocks that have their signals
affected by the pinmux.

I'd rather see a centralized pinmux table listing all the pinmux settings
that the board should use. Either the table should include references to
other nodes that use each setting, or the other nodes should refer to the
table in the pinmux node and this indicate which settings they should use.

Ignoring this objection, if the pinmux details are kept in each device's
own node, the binding needs to be a lot more complex than just listing the
set of pins that are used; the following need to be covered:

* Alternative configurations e.g. for muxing a single I2C controller on
to multiple sets of I2C pins to represent a bus mux.

* Alternative configurations for active/sleep(suspend) etc.

* Pin configuration in addition to mux function selection.

It seems like it'd be easier to represent that all in a/some table(s) in
the pinmux node. I think the current pinctrl mapping table would be a good
model to start out with. But, we'd have to expand that to cover pin config
as well as mux, or have separate tables in DT for each (which would be
harder to keep in sync)

-- 
nvpublic

Re: Pin control mappings for DT

[Tony Lindgren]

* Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> [111129 09:27]:
> Tony Lindgren wrote at Tuesday, November 29, 2011 10:43 AM:
> > * Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [111128 23:07]:
> > > Greetings,
> > >
> > > can we have some initial idea about DT support for pin control
> > > devices and how these will look? Since pin control seems to be
> > > a cruicial piece for people to get board data out of their kernels
> > > we'd better have a common understanding.
> ...
> > > 2) Board pin group definitions, saying "these three pins
> > >   belong to the group "i2c0-pins"
> > 
> > These should be there for each driver, just like the interrupts:
> > 
> > 	uart3: serial@48020000 {
> > 		compatible = "ti,8250";
> > 		reg = <0x48020000 0x100>;
> > 		reg-shift = <2>;
> > 		interrupts = < 106 >;
> > 		pins = "uart3_rx_irrx.uart3_rx_irrx",
> > 			"uart3_tx_irtx.uart3_tx_irtx";
> > 	};
> 
> I don't agree here. I think that pinmux data is more strongly related
> to the pinmux controller HW than the HW blocks that have their signals
> affected by the pinmux.

I don't think so. The pinmux controller needs to know the available functions.
But the functions selected for each driver are board/machine specific.

We already have many alternative pins on the same package at least for
omaps. It's up to the board designed to select which pins to use.
And the pinmux controller has no way of knowing the routing. And mapping
all the combinations in the pinmux driver is just not scalable.

> I'd rather see a centralized pinmux table listing all the pinmux settings
> that the board should use. Either the table should include references to
> other nodes that use each setting, or the other nodes should refer to the
> table in the pinmux node and this indicate which settings they should use.

Are you talking about static tables in the kernel? We already have something
like this, and it just does not scale. We end up with huge amounts of static
data in the kernel that needs to be patched constantly for drivers and PM..

The goal is to boot the same kernel on various SoC/board/machine variants,
we don't want to edit a board/machine specific table in the kernel for
each variant. Or maybe you have something else in mind that I don't
understand?
 
> Ignoring this objection, if the pinmux details are kept in each device's
> own node, the binding needs to be a lot more complex than just listing the
> set of pins that are used; the following need to be covered:
> 
> * Alternative configurations e.g. for muxing a single I2C controller on
> to multiple sets of I2C pins to represent a bus mux.

Hmm, then you just list the functions in the I2C controller DT entry?
The above example already passes two pins.
 
> * Alternative configurations for active/sleep(suspend) etc.

This is too completely board specific. For example, if you have an external
pull on the I2C data line, the internal pull needs to be disabled. And then
the driver may need to change some of these settings for PM etc.
 
> * Pin configuration in addition to mux function selection.

Yes this is open.. It seems that we could set the board specific default
value from DT for the pins used by the driver. Then we could potentially
have generic configuration flags that the driver can set for "idle",
"wakeup" "off" like Linus suggested?
 
> It seems like it'd be easier to represent that all in a/some table(s) in
> the pinmux node. I think the current pinctrl mapping table would be a good
> model to start out with. But, we'd have to expand that to cover pin config
> as well as mux, or have separate tables in DT for each (which would be
> harder to keep in sync)

The current pinctrl mapping is totally messed up in it's current form I'm
sorry to say in a polite way :)

It currently assumes one set of static map data being passed from the
board/machine. So you can't mix static data and dynamic data coming
from DT. And it does not allow freeing the mappings if you have a loadable
pinmux module.. Things get even more messed up when trying to support
multiple driver instances..

To sort this out the mappings need to be tied to pinctrl/pinmux driver
instances to start with. That should be done before relying on it further
and adding more features to it. Anyways, on the positive side, it seems
that eventually most mappings can be dynamically generated with DT.

But I don't see why we should not be able to support static mappings
like you suggest, and dynamic mappings I suggest the same time. It
just requires some changes to the pinctrl fwk, right?

Oh, one more thing that needs to be considered is that the mappings are not
static. Consider popular development boards like beagle. Depending how
you choose to use the available pins, you need to mux them for an
extension board, for example. You don't really want to edit the mux mappings
and recompile the kernel just to support the same board with different
add on boards. Just changing the DT entries should be enough to map
the extra pins to an extra I2C controller or LCD or whatever.

Cheers,

Tony

Re: Pin control mappings for DT

[Linus Walleij]

On Tue, Nov 29, 2011 at 8:59 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:

> The current pinctrl mapping is totally messed up in it's current form I'm
> sorry to say in a polite way :)

Hehe :-)

> It currently assumes one set of static map data being passed from the
> board/machine. So you can't mix static data and dynamic data coming
> from DT.

For the global list of maps in pinctrl I sent a patch today which
augments this so board files can add several incremental sets of
maps.

So it can now grow dynamically but not shrink dynamically (hey,
50% done!)

> And it does not allow freeing the mappings if you have a loadable
> pinmux module.. Things get even more messed up when trying to support
> multiple driver instances..
>
> To sort this out the mappings need to be tied to pinctrl/pinmux driver
> instances to start with.

It *is* tied to the pinctrl/pinmux driver instance since each mapping entry
has:

 * @ctrl_dev: the pin control device to be used by this mapping, may be NULL
 *      if you provide .ctrl_dev_name instead (this is more common)
 * @ctrl_dev_name: the name of the device controlling this specific mapping,
 *      the name must be the same as in your struct device*, may be NULL if
 *      you provide .ctrl_dev instead

        struct device *ctrl_dev;
        const char *ctrl_dev_name;

If what you propose is to do away with these two fields and instead
store just the rest of the mapping in a per-pinctrl list passed in
from platform data, as is done with regulators, I see what you mean,
that's maybe more elegant. The current solution is on par with
clkdev.

> That should be done before relying on it further
> and adding more features to it. Anyways, on the positive side, it seems
> that eventually most mappings can be dynamically generated with DT.

Yes, DT will likely make things better.

> Oh, one more thing that needs to be considered is that the mappings are not
> static. Consider popular development boards like beagle. Depending how
> you choose to use the available pins, you need to mux them for an
> extension board, for example. You don't really want to edit the mux mappings
> and recompile the kernel just to support the same board with different
> add on boards. Just changing the DT entries should be enough to map
> the extra pins to an extra I2C controller or LCD or whatever.

Once we have DT support for the mappings this should
come for free I guess?

Linus

Re: Pin control mappings for DT

[Tony Lindgren]

* Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [111130 06:39]:
> On Tue, Nov 29, 2011 at 8:59 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> 
> > The current pinctrl mapping is totally messed up in it's current form I'm
> > sorry to say in a polite way :)
> 
> Hehe :-)

Heh I knew you would not be offended by that ;)
 
> > It currently assumes one set of static map data being passed from the
> > board/machine. So you can't mix static data and dynamic data coming
> > from DT.
> 
> For the global list of maps in pinctrl I sent a patch today which
> augments this so board files can add several incremental sets of
> maps.
> 
> So it can now grow dynamically but not shrink dynamically (hey,
> 50% done!)

Great, sounds like that makes multiple pinmux driver instances to work,
unloading the pinmux driver of course will still not work..
 
> > And it does not allow freeing the mappings if you have a loadable
> > pinmux module.. Things get even more messed up when trying to support
> > multiple driver instances..
> >
> > To sort this out the mappings need to be tied to pinctrl/pinmux driver
> > instances to start with.
> 
> It *is* tied to the pinctrl/pinmux driver instance since each mapping entry
> has:
> 
>  * @ctrl_dev: the pin control device to be used by this mapping, may be NULL
>  *      if you provide .ctrl_dev_name instead (this is more common)
>  * @ctrl_dev_name: the name of the device controlling this specific mapping,
>  *      the name must be the same as in your struct device*, may be NULL if
>  *      you provide .ctrl_dev instead
> 
>         struct device *ctrl_dev;
>         const char *ctrl_dev_name;
> 
> If what you propose is to do away with these two fields and instead
> store just the rest of the mapping in a per-pinctrl list passed in
> from platform data, as is done with regulators, I see what you mean,
> that's maybe more elegant. The current solution is on par with
> clkdev.

Yes right, they should be per-pinctrl as otherwise we can't free the
mapping when unloading a pinmux driver. Basically we need a way to do
pinmux_free_mappings(struct pinmux_map *map) so it only frees per-pinctrl
map.

Maybe the static map should be copied over to make it per-pinctrl during
init based on the ctrl_dev_name?
 
> > That should be done before relying on it further
> > and adding more features to it. Anyways, on the positive side, it seems
> > that eventually most mappings can be dynamically generated with DT.
> 
> Yes, DT will likely make things better.
> 
> > Oh, one more thing that needs to be considered is that the mappings are not
> > static. Consider popular development boards like beagle. Depending how
> > you choose to use the available pins, you need to mux them for an
> > extension board, for example. You don't really want to edit the mux mappings
> > and recompile the kernel just to support the same board with different
> > add on boards. Just changing the DT entries should be enough to map
> > the extra pins to an extra I2C controller or LCD or whatever.
> 
> Once we have DT support for the mappings this should
> come for free I guess?

Yes that's correct. Then the board just needs to have the add-on board
defined in DT with it's pins and it will get probed if the drivers
are there.

Further, we should be able to also support new package variations with
existing kernels just by modifying the DT data. The only difference
between package versions is which signals are available and to which
pins they are being routed.

Just to give some idea of the scope of problem it solves, we already
have static data for 9 packages in arch/arm/mach-omap2/mux*4*.c with
omap3 three alone having 4 packages.. Even with that being incremental
data that's marked __initdata, it's still not a scalable way of doing
things as there will be more and more packages.

Regards,

Tony

RE: Pin control mappings for DT

[Stephen Warren]

Tony Lindgren wrote at Wednesday, November 30, 2011 9:54 AM:
> * Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [111130 06:39]:
> > On Tue, Nov 29, 2011 at 8:59 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
...
> Just to give some idea of the scope of problem it solves, we already
> have static data for 9 packages in arch/arm/mach-omap2/mux*4*.c with
> omap3 three alone having 4 packages.. Even with that being incremental
> data that's marked __initdata, it's still not a scalable way of doing
> things as there will be more and more packages.

Are these purely packaging options for the same silicon, such that any
pin ID in the pinmux controller always represents the same pad on the
die, it's just that some may not actually be bonded out to actual pins
or balls on the package (or some mux functions might select controllers
that are "not present" in the die)? Or, is this the same IP block used
in multiple SoCs, where the use of each pin ID in the pinmux controller,
and the meaning of each mux function value for each pin ID, are actually
hooked up to different logical functions within the SoC?

For the former case (pure bonding options), the pinctrl documentation
recommends having the pinctrl driver expose the die pads, rather than
the package pins/balls. That way, there's a pinctrl single data set that
works across all the n package variations. The only issue here is that
when constructing the pinctrl mapping table, you need to only actually
use pins/functions that are valid for the particular SoC you know you're
using, but that's true anyway irrespective of whether the pinctrl driver
actually defines the unusable options or not.

Tegra20 is in the same situation; there are at least 2 packaging options
for it. The Tegra20 pinctrl driver has been written with 1 dataset
representing what's on the die, rather than 2 datasets fully representing
which subsets actually make it out to the package.

-- 
nvpublic

Re: Pin control mappings for DT

[Tony Lindgren]

* Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> [111130 09:10]:
> Tony Lindgren wrote at Wednesday, November 30, 2011 9:54 AM:
> > * Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [111130 06:39]:
> > > On Tue, Nov 29, 2011 at 8:59 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> ...
> > Just to give some idea of the scope of problem it solves, we already
> > have static data for 9 packages in arch/arm/mach-omap2/mux*4*.c with
> > omap3 three alone having 4 packages.. Even with that being incremental
> > data that's marked __initdata, it's still not a scalable way of doing
> > things as there will be more and more packages.
> 
> Are these purely packaging options for the same silicon, such that any
> pin ID in the pinmux controller always represents the same pad on the
> die, it's just that some may not actually be bonded out to actual pins
> or balls on the package (or some mux functions might select controllers
> that are "not present" in the die)? Or, is this the same IP block used
> in multiple SoCs, where the use of each pin ID in the pinmux controller,
> and the meaning of each mux function value for each pin ID, are actually
> hooked up to different logical functions within the SoC?

It's the same IP block used across omap2/3/4 where the register can be
hooked to a different logical function within the SoC. So the differences
can be quite big.
 
> For the former case (pure bonding options), the pinctrl documentation
> recommends having the pinctrl driver expose the die pads, rather than
> the package pins/balls. That way, there's a pinctrl single data set that
> works across all the n package variations. The only issue here is that
> when constructing the pinctrl mapping table, you need to only actually
> use pins/functions that are valid for the particular SoC you know you're
> using, but that's true anyway irrespective of whether the pinctrl driver
> actually defines the unusable options or not.

Yes we are using the die internal signal names. So for example, omap3
has the following die pad options for uart3 rx:

mux_register.mux_function	balls depending on the package
uart3_rx_irrx.uart3_rx_irrx	h24, b24 or h20
dss_data4.uart3_rx_irrx		ad23, ad21 or ag24
dss_data8.uart3_rx_irrx		h24, e24 or f27
hsusb0_data1.uart3_rx_irrx	y20, t23 or u28

So for selecting the function, the ball name does not matter.

> Tegra20 is in the same situation; there are at least 2 packaging options
> for it. The Tegra20 pinctrl driver has been written with 1 dataset
> representing what's on the die, rather than 2 datasets fully representing
> which subsets actually make it out to the package.

Yes we have currently 4 completely different supersets, then the rest 5
are diffs to these, usually subsets, but can have new functions added too.

Regards,

Tony

Re: Pin control mappings for DT

[Linus Walleij]

On Wed, Nov 30, 2011 at 5:54 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> * Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [111130 06:39]:

>> For the global list of maps in pinctrl I sent a patch today which
>> augments this so board files can add several incremental sets of
>> maps.
>>
>> So it can now grow dynamically but not shrink dynamically (hey,
>> 50% done!)
>
> Great, sounds like that makes multiple pinmux driver instances to work,
> unloading the pinmux driver of course will still not work..

Oh that already works, it's just that you have a single static mapping
table for all the controllers, and it doesn't get free:ed up when
loading and unloading pinmux drivers, the mappings get enabled
when you load them and disabled when you unload them, simply.

Of course you have to use a string like "pinctrl.0" or "pinctrl.1"
(or a symbolic .init_name) to refer to it in the map since your
struct device * is not static, but the string works just fine.

So the situation is the same as with say, clkdev and multiple
clock drivers.

But: how do we deal with clients still using a pinmux say if a
device driver has issued pmx = pinmux_get() on it, when you
remove the device underneath? (We're not doing anything
about that today.)

>> If what you propose is to do away with these two fields and instead
>> store just the rest of the mapping in a per-pinctrl list passed in
>> from platform data, as is done with regulators, I see what you mean,
>> that's maybe more elegant. The current solution is on par with
>> clkdev.
>
> Yes right, they should be per-pinctrl as otherwise we can't free the
> mapping when unloading a pinmux driver. Basically we need a way to do
> pinmux_free_mappings(struct pinmux_map *map) so it only frees per-pinctrl
> map.

It's true you cannot free the mappings but for example the pinmux
hogs will be free:ed when you unload the driver.

> Maybe the static map should be copied over to make it per-pinctrl during
> init based on the ctrl_dev_name?

Maybe I'm misunderstanding something. This is currently inside
pinmux.c:

/* List of pinmux hogs */
static DEFINE_MUTEX(pinmux_hoglist_mutex);
static LIST_HEAD(pinmux_hoglist);

/* Global pinmux maps */
static struct pinmux_map *pinmux_maps;
static unsigned pinmux_maps_num;

When  a pinmux device is loaded, it attempts to match
pinmux mappings from the pinmux_map onto itself and
set up any hogs, using pinmux_get() then these are
added to the pinmux_hoglist in the function
pinmux_hog_maps(); (Hogs are optional, it is orthogonal
to use them directly from board code or devices with
pinmux_get())

After this, board code or devices can look up
pinmuxes with pinmux_get() and this will traverse
the map to find matching pinmux devices.

When a device is unloaded the hogs are likewise removed
from the list in pinmux_unhog_maps() with
pinmux_put() calls.

That the mapping is for all devices doesn't stop you from
loading and unloading pinmux drivers as much as you
want, we already handle that (well there may be bugs in the
code since it's not deployed on any system adding/removing
controllers, but the idea sure is there...)  if you're only
using hogs and no pinmux_get() from elsewhere on it,
it will even work in practice :-)

Thanks,
Linus Walleij

Re: Pin control mappings for DT

[Tony Lindgren]

* Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [111201 04:15]:
> On Wed, Nov 30, 2011 at 5:54 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> > * Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [111130 06:39]:
> 
> >> For the global list of maps in pinctrl I sent a patch today which
> >> augments this so board files can add several incremental sets of
> >> maps.
> >>
> >> So it can now grow dynamically but not shrink dynamically (hey,
> >> 50% done!)
> >
> > Great, sounds like that makes multiple pinmux driver instances to work,
> > unloading the pinmux driver of course will still not work..
> 
> Oh that already works, it's just that you have a single static mapping
> table for all the controllers, and it doesn't get free:ed up when
> loading and unloading pinmux drivers, the mappings get enabled
> when you load them and disabled when you unload them, simply.

Yes that's the problem I'm having. If the mapping exists, then we
should not add to it.

But how does the second pinmux driver instance know if it's OK
to add to the mapping or not? The mapping exists, but it might be
also from the previous time you loaded the pinmux driver..

I really think we should free the (non-static) mapping on unregister
so we can use standard Linux development methods for the pinmux drivers.
 
> Of course you have to use a string like "pinctrl.0" or "pinctrl.1"
> (or a symbolic .init_name) to refer to it in the map since your
> struct device * is not static, but the string works just fine.
> 
> So the situation is the same as with say, clkdev and multiple
> clock drivers.
> 
> But: how do we deal with clients still using a pinmux say if a
> device driver has issued pmx = pinmux_get() on it, when you
> remove the device underneath? (We're not doing anything
> about that today.)

Heh, I'd assume it's mostly the people developing pinmux drivers
who need to rmmod and insmod :)

Maybe we should just print warnings in this case, some people
actually prefer to set up the pins once during the init and then
not touch them at all afterwards.
 
> >> If what you propose is to do away with these two fields and instead
> >> store just the rest of the mapping in a per-pinctrl list passed in
> >> from platform data, as is done with regulators, I see what you mean,
> >> that's maybe more elegant. The current solution is on par with
> >> clkdev.
> >
> > Yes right, they should be per-pinctrl as otherwise we can't free the
> > mapping when unloading a pinmux driver. Basically we need a way to do
> > pinmux_free_mappings(struct pinmux_map *map) so it only frees per-pinctrl
> > map.
> 
> It's true you cannot free the mappings but for example the pinmux
> hogs will be free:ed when you unload the driver.
> 
> > Maybe the static map should be copied over to make it per-pinctrl during
> > init based on the ctrl_dev_name?
> 
> Maybe I'm misunderstanding something. This is currently inside
> pinmux.c:
> 
> /* List of pinmux hogs */
> static DEFINE_MUTEX(pinmux_hoglist_mutex);
> static LIST_HEAD(pinmux_hoglist);
> 
> /* Global pinmux maps */
> static struct pinmux_map *pinmux_maps;
> static unsigned pinmux_maps_num;
> 
> When  a pinmux device is loaded, it attempts to match
> pinmux mappings from the pinmux_map onto itself and
> set up any hogs, using pinmux_get() then these are
> added to the pinmux_hoglist in the function
> pinmux_hog_maps(); (Hogs are optional, it is orthogonal
> to use them directly from board code or devices with
> pinmux_get())
> 
> After this, board code or devices can look up
> pinmuxes with pinmux_get() and this will traverse
> the map to find matching pinmux devices.
> 
> When a device is unloaded the hogs are likewise removed
> from the list in pinmux_unhog_maps() with
> pinmux_put() calls.
> 
> That the mapping is for all devices doesn't stop you from
> loading and unloading pinmux drivers as much as you
> want, we already handle that (well there may be bugs in the
> code since it's not deployed on any system adding/removing
> controllers, but the idea sure is there...)  if you're only
> using hogs and no pinmux_get() from elsewhere on it,
> it will even work in practice :-)

Well if there are dynamic maps, we still need to release
the maps also.

How about we pass the static map in platform_data to the pinmux
driver in question, then have the pinmux driver set it up with
pinctrl fwk as per-pinctrl map.

This would remove the need for the global map. This would
also allow making the pinctrl fwk into loadable modules
in some cases.

Regards,

Tony

Re: Pin control mappings for DT

[Linus Walleij]

On Thu, Dec 1, 2011 at 11:22 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:

> But how does the second pinmux driver instance know if it's OK
> to add to the mapping or not? The mapping exists, but it might be
> also from the previous time you loaded the pinmux driver..

The mappings are not added from pinmux drivers, atleast not
the in-tree drivers. Mappings are board data.

The existing U300 driver only defines it's pins and groups
(and these are indeed per-device)

The map is defined by arch/arm/mach-u300/core.c
and registered when that MACHINE_TYPE is registered.

So that's why I say it's like the clkdev maps.

>> That the mapping is for all devices doesn't stop you from
>> loading and unloading pinmux drivers as much as you
>> want, we already handle that (well there may be bugs in the
>> code since it's not deployed on any system adding/removing
>> controllers, but the idea sure is there...)  if you're only
>> using hogs and no pinmux_get() from elsewhere on it,
>> it will even work in practice :-)
>
> Well if there are dynamic maps, we still need to release
> the maps also.

Hm I think I'm not grasping the concept of dynamic maps
simply.

The current maps are dynamic in the sense that they are
registered by the board files, and can also be (with
the recent patches) additively added, say if the SoC
has one big chunk of common mappings and smaller
chunks of per-board mappings (implemented for PXA).

> How about we pass the static map in platform_data to the pinmux
> driver in question, then have the pinmux driver set it up with
> pinctrl fwk as per-pinctrl map.

That would be a refactoring making it more like how
regulators pass their consumer lists, but it's not
fixing something that is broken IMO.

> This would remove the need for the global map. This would
> also allow making the pinctrl fwk into loadable modules
> in some cases.

The drivers are loadable today, but why should the pinctrl
*framework* be a loadable module? Regulators and
clocks are bool I don't see why the pinctrl core should
be any different?

Yours,
Linus Walleij

Re: Pin control mappings for DT

[Tony Lindgren]

* Linus Walleij <linus.walleij@linaro.org> [111202 01:56]:
> On Thu, Dec 1, 2011 at 11:22 PM, Tony Lindgren <tony@atomide.com> wrote:
> 
> > But how does the second pinmux driver instance know if it's OK
> > to add to the mapping or not? The mapping exists, but it might be
> > also from the previous time you loaded the pinmux driver..
> 
> The mappings are not added from pinmux drivers, atleast not
> the in-tree drivers. Mappings are board data.
> 
> The existing U300 driver only defines it's pins and groups
> (and these are indeed per-device)
> 
> The map is defined by arch/arm/mach-u300/core.c
> and registered when that MACHINE_TYPE is registered.
> 
> So that's why I say it's like the clkdev maps.

Right, but there may not be board data necessarily. With device
tree there really should not be any need for static maps. All the
necessary information can be pulled from device tree to dynamically
build the maps.
 
> >> That the mapping is for all devices doesn't stop you from
> >> loading and unloading pinmux drivers as much as you
> >> want, we already handle that (well there may be bugs in the
> >> code since it's not deployed on any system adding/removing
> >> controllers, but the idea sure is there...)  if you're only
> >> using hogs and no pinmux_get() from elsewhere on it,
> >> it will even work in practice :-)
> >
> > Well if there are dynamic maps, we still need to release
> > the maps also.
> 
> Hm I think I'm not grasping the concept of dynamic maps
> simply.
> 
> The current maps are dynamic in the sense that they are
> registered by the board files, and can also be (with
> the recent patches) additively added, say if the SoC
> has one big chunk of common mappings and smaller
> chunks of per-board mappings (implemented for PXA).

Sure, sounds like that's working fine as long as you have board
files and maps passed from board files.
 
> > How about we pass the static map in platform_data to the pinmux
> > driver in question, then have the pinmux driver set it up with
> > pinctrl fwk as per-pinctrl map.
> 
> That would be a refactoring making it more like how
> regulators pass their consumer lists, but it's not
> fixing something that is broken IMO.

Yeah I understand it's not broken right now with maps
passed from board files.

I guess I'll try to fix it up at some point to make it
behave also for device tree.
 
> > This would remove the need for the global map. This would
> > also allow making the pinctrl fwk into loadable modules
> > in some cases.
> 
> The drivers are loadable today, but why should the pinctrl
> *framework* be a loadable module? Regulators and
> clocks are bool I don't see why the pinctrl core should
> be any different?

Heh just because we could.. to make development easier? :)

I think currently only pinmux_register_mappings would block
that.

Regards,

Tony
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

RE: Pin control mappings for DT

[Stephen Warren]

Tony Lindgren wrote at Tuesday, November 29, 2011 1:00 PM:
> * Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> [111129 09:27]:
> > Tony Lindgren wrote at Tuesday, November 29, 2011 10:43 AM:
> > > * Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [111128 23:07]:
> > > > Greetings,
> > > >
> > > > can we have some initial idea about DT support for pin control
> > > > devices and how these will look? Since pin control seems to be
> > > > a cruicial piece for people to get board data out of their kernels
> > > > we'd better have a common understanding.
> > ...
> > > > 2) Board pin group definitions, saying "these three pins
> > > >   belong to the group "i2c0-pins"
> > >
> > > These should be there for each driver, just like the interrupts:
> > >
> > > 	uart3: serial@48020000 {
> > > 		compatible = "ti,8250";
> > > 		reg = <0x48020000 0x100>;
> > > 		reg-shift = <2>;
> > > 		interrupts = < 106 >;
> > > 		pins = "uart3_rx_irrx.uart3_rx_irrx",
> > > 			"uart3_tx_irtx.uart3_tx_irtx";
> > > 	};
> >
> > I don't agree here. I think that pinmux data is more strongly related
> > to the pinmux controller HW than the HW blocks that have their signals
> > affected by the pinmux.
> 
> I don't think so. The pinmux controller needs to know the available functions.
> But the functions selected for each driver are board/machine specific.

That's certainly true.

However, in my opinion just because the actual usage of the pinmux is
board-/machine-specific, that doesn't determine whether the description
of that usage has to be stored in the DT node of the devices that are
affected by the pinmux, or in some centralized table.

> We already have many alternative pins on the same package at least for
> omaps. It's up to the board designed to select which pins to use.
> And the pinmux controller has no way of knowing the routing. And mapping
> all the combinations in the pinmux driver is just not scalable.
>
> > I'd rather see a centralized pinmux table listing all the pinmux settings
> > that the board should use. Either the table should include references to
> > other nodes that use each setting, or the other nodes should refer to the
> > table in the pinmux node and this indicate which settings they should use.
> 
> Are you talking about static tables in the kernel? We already have something
> like this, and it just does not scale. We end up with huge amounts of static
> data in the kernel that needs to be patched constantly for drivers and PM..
> 
> The goal is to boot the same kernel on various SoC/board/machine variants,
> we don't want to edit a board/machine specific table in the kernel for
> each variant. Or maybe you have something else in mind that I don't
> understand?

Yes, I think we're talking about slightly different things.

There are two components to the data:

1) The definition of what the pinmux HW can do.

There shouldn't be any combinations here.

This data should simply list for each pin what the valid mux options are.
(or for each pin group if mux registers control n pins at a time as on
Tegra)

This is the equivalent of the data that the pinctrl driver provides to
the pinctrl core in Linux.

If a logical function (8-bit MMC) requires 10 pins to be individually
configured, then that isn't represented at all in this dataset. This is
why I say "no combinations".

Yes, this can be large in itself, but it's linear in the number of pins,
groups, and functions, not exponential in the number of possible
combinations of those pins/groups/functions.

If n packages use the same pinmux HW, but end up routing different subsets
of die pads to actual package pins/balls, I think this dataset should
represent the die, not the package options. (See my other email) This
reduces the number of similar but different datasets (perhaps that's what
you mean when you talked about combinations?)

This data set is what I'd expect some pinctrl drivers to encode internally
using static tables, and perhaps some will parse this out of DT, most
likely using a custom DT binding per pinctrl HW design; perhaps with
consistent/similar base design across SoCs, but unlikely to be parse by
any core pinctrl code.

I mainly say that the DT binding is custom here because the information
needed by the pinctrl driver is pretty custom per SoC. Basic stuff like
a list of pins, functions may be common, but there will likely be many
additional fields for each pin/group/function/... needed by the
individual pinctrl driver.

2) The board-specific configuration for the pinmux.

This only represents the specific usage for a board; it'll say e.g. that
for SD/MMC controller 2, you need pins A=SD2_CMD, F=SD2_CLK, G..N=SD2_DAT.

This should limit the size of the data; there's no need to represent e.g.
all the possible sets of pins that a particular SD/MMC controller could be
mux'd to here, since presumably only a single option is actually valid on
the board.

This is the equivalent of the mapping table handled by the pinctrl core
on Linux.

> > Ignoring this objection, if the pinmux details are kept in each device's
> > own node, the binding needs to be a lot more complex than just listing the
> > set of pins that are used; the following need to be covered:
> >
> > * Alternative configurations e.g. for muxing a single I2C controller on
> > to multiple sets of I2C pins to represent a bus mux.
> 
> Hmm, then you just list the functions in the I2C controller DT entry?
> The above example already passes two pins.

I suppose with a sufficiently flexible mapping, we could distribute the
data that becomes the pinctrl mapping table across each device's own
node.

But when the pinctrl core starts up, it probably won't want to enumerate
the whole device tree looking for nodes that happen to have properties
that look like this distributed pinmux binding. I guess we'd need to
lazily parse each device's DT node when it first calls into pinmux_get().

I wonder what node we'd put the "system hog" entries into. I guess the
pinctrl core can parse the pinctrl device's own node for the same
properties whenever a controller is registered.

-- 
nvpublic

Re: Pin control mappings for DT

[Tony Lindgren]

* Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> [111130 09:41]:
> Tony Lindgren wrote at Tuesday, November 29, 2011 1:00 PM:
> > * Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> [111129 09:27]:
> > 
> > I don't think so. The pinmux controller needs to know the available functions.
> > But the functions selected for each driver are board/machine specific.
> 
> That's certainly true.
> 
> However, in my opinion just because the actual usage of the pinmux is
> board-/machine-specific, that doesn't determine whether the description
> of that usage has to be stored in the DT node of the devices that are
> affected by the pinmux, or in some centralized table.

Yes well both should work. I guess as long as the serial driver can
do pinmux_get(dev, NULL) the driver stays arch independent.
 
> There are two components to the data:
> 
> 1) The definition of what the pinmux HW can do.
> 
> There shouldn't be any combinations here.

But there are..
 
> This data should simply list for each pin what the valid mux options are.
> (or for each pin group if mux registers control n pins at a time as on
> Tegra)

..and we already have four completely different function tables and five
subsets/incremental changes for the same IP block for omap2/3/4.
 
> This is the equivalent of the data that the pinctrl driver provides to
> the pinctrl core in Linux.
> 
> If a logical function (8-bit MMC) requires 10 pins to be individually
> configured, then that isn't represented at all in this dataset. This is
> why I say "no combinations".

Right, but that's still nine data sets we already have :)
 
> Yes, this can be large in itself, but it's linear in the number of pins,
> groups, and functions, not exponential in the number of possible
> combinations of those pins/groups/functions.

Right, but we already have about 6000 LOC for that data right now that
we need to build in for that:

$ wc -l arch/arm/mach-omap2/mux*4*.[ch]
   690 arch/arm/mach-omap2/mux2420.c
   282 arch/arm/mach-omap2/mux2420.h
   793 arch/arm/mach-omap2/mux2430.c
   370 arch/arm/mach-omap2/mux2430.h
  2061 arch/arm/mach-omap2/mux34xx.c
   398 arch/arm/mach-omap2/mux34xx.h
  1356 arch/arm/mach-omap2/mux44xx.c
   298 arch/arm/mach-omap2/mux44xx.h
  6248 total
 
> If n packages use the same pinmux HW, but end up routing different subsets
> of die pads to actual package pins/balls, I think this dataset should
> represent the die, not the package options. (See my other email) This
> reduces the number of similar but different datasets (perhaps that's what
> you mean when you talked about combinations?)

Yes that's 4 the for suspersets we already have.
 
> This data set is what I'd expect some pinctrl drivers to encode internally
> using static tables, and perhaps some will parse this out of DT, most
> likely using a custom DT binding per pinctrl HW design; perhaps with
> consistent/similar base design across SoCs, but unlikely to be parse by
> any core pinctrl code.

I agree both should work, have it in the driver for simple cases,
or parse all or some of it from DT.
 
> I mainly say that the DT binding is custom here because the information
> needed by the pinctrl driver is pretty custom per SoC. Basic stuff like
> a list of pins, functions may be common, but there will likely be many
> additional fields for each pin/group/function/... needed by the
> individual pinctrl driver.

Yeah it can be totally different. But it seems in many cases it can
be abstracted to one 8 or 16 bit register per pin that toggle the various
functions and configure the pin direction, pull and wake-up capabilities.
So I'm trying to do a generic DT based to cover those case.
 
> 2) The board-specific configuration for the pinmux.
> 
> This only represents the specific usage for a board; it'll say e.g. that
> for SD/MMC controller 2, you need pins A=SD2_CMD, F=SD2_CLK, G..N=SD2_DAT.
> 
> This should limit the size of the data; there's no need to represent e.g.
> all the possible sets of pins that a particular SD/MMC controller could be
> mux'd to here, since presumably only a single option is actually valid on
> the board.
> 
> This is the equivalent of the mapping table handled by the pinctrl core
> on Linux.

Yes and this too should be possible to pass from the board/machine code,
or from device tree.
 
> I suppose with a sufficiently flexible mapping, we could distribute the
> data that becomes the pinctrl mapping table across each device's own
> node.
> 
> But when the pinctrl core starts up, it probably won't want to enumerate
> the whole device tree looking for nodes that happen to have properties
> that look like this distributed pinmux binding. I guess we'd need to
> lazily parse each device's DT node when it first calls into pinmux_get().

Well that just requires one pass over the tree to get the pins and the
pin using device entry, so it's not that bad actually.
 
> I wonder what node we'd put the "system hog" entries into. I guess the
> pinctrl core can parse the pinctrl device's own node for the same
> properties whenever a controller is registered.

Yes no idea what would be the best place to put these.. The unused
pins should be disabled for PM to avoid floating pins. For the clock
framework we do a late_initcall that disables the unused clocks, maybe
something similar could be done eventually for unused pins when a
SoC specific PM driver gets initialized.

Regards,

Tony

RE: Pin control mappings for DT

[Stephen Warren]

Tony Lindgren wrote at Wednesday, November 30, 2011 12:34 PM:
> * Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> [111130 09:41]:
> > Tony Lindgren wrote at Tuesday, November 29, 2011 1:00 PM:
> > > * Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> [111129 09:27]:
> > >
> > > I don't think so. The pinmux controller needs to know the available functions.
> > > But the functions selected for each driver are board/machine specific.
> >
> > That's certainly true.
> >
> > However, in my opinion just because the actual usage of the pinmux is
> > board-/machine-specific, that doesn't determine whether the description
> > of that usage has to be stored in the DT node of the devices that are
> > affected by the pinmux, or in some centralized table.
> 
> Yes well both should work. I guess as long as the serial driver can
> do pinmux_get(dev, NULL) the driver stays arch independent.
> 
> > There are two components to the data:
> >
> > 1) The definition of what the pinmux HW can do.
> >
> > There shouldn't be any combinations here.
> 
> But there are..

As I was trying to point out, I think what you're talking about as
"combinations" is something totally different to what I was talking
about.

I was talking about pins combining to create busses, something which can
get pretty exponential within a single SoC.

I think you're talking about different SoCs having different sets of
pins, groups, functions, etc.

For a given SoC (die + package option), there is a single table of all
known pins, groups, and functions. No combinations.

Now it may well be that you have a family of SoCs that have many
similarities between their sets if pins/groups/functions (or subsets
thereof) and I guess that's why you were talking about combinations?
But, IIRC, that's unrelated to the points I was making.

> > This data should simply list for each pin what the valid mux options are.
> > (or for each pin group if mux registers control n pins at a time as on
> > Tegra)
> 
> ..and we already have four completely different function tables and five
> subsets/incremental changes for the same IP block for omap2/3/4.

Well, sure, you have much different HW.

But the pinctrl core should only receive the 1 specific complete set of
pins/groups/functions for the particular SoC that the code is running
on.

Anything to do with constructing that list or representing it in device
tree is something implementation detail for the individual pinctrl driver.

If you have the tables in code, you can presumably merge them together
while implementing the pinctrl subsystem's callbacks. If you have the
tables in device tree, you can presumably merge them together when running
dtc, using /include/ statements from the SoC *.dtsi file.

-- 
nvpublic

Re: Pin control mappings for DT

[Tony Lindgren]

* Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> [111201 12:45]:
> Tony Lindgren wrote at Wednesday, November 30, 2011 12:34 PM:
> > * Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> [111130 09:41]:
> > > Tony Lindgren wrote at Tuesday, November 29, 2011 1:00 PM:
> > > > * Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> [111129 09:27]:
> > > >
> > > > I don't think so. The pinmux controller needs to know the available functions.
> > > > But the functions selected for each driver are board/machine specific.
> > >
> > > That's certainly true.
> > >
> > > However, in my opinion just because the actual usage of the pinmux is
> > > board-/machine-specific, that doesn't determine whether the description
> > > of that usage has to be stored in the DT node of the devices that are
> > > affected by the pinmux, or in some centralized table.
> > 
> > Yes well both should work. I guess as long as the serial driver can
> > do pinmux_get(dev, NULL) the driver stays arch independent.
> > 
> > > There are two components to the data:
> > >
> > > 1) The definition of what the pinmux HW can do.
> > >
> > > There shouldn't be any combinations here.
> > 
> > But there are..
> 
> As I was trying to point out, I think what you're talking about as
> "combinations" is something totally different to what I was talking
> about.
> 
> I was talking about pins combining to create busses, something which can
> get pretty exponential within a single SoC.

Ah right, yes that's correct. I'm not even considering statically
grouping pins because of the infinite number of options :)
 
> I think you're talking about different SoCs having different sets of
> pins, groups, functions, etc.
> 
> For a given SoC (die + package option), there is a single table of all
> known pins, groups, and functions. No combinations.

That's correct. And we already have about 6000 LOC of that for omaps
without any grouped pins to create extra permutations.
 
> Now it may well be that you have a family of SoCs that have many
> similarities between their sets if pins/groups/functions (or subsets
> thereof) and I guess that's why you were talking about combinations?
> But, IIRC, that's unrelated to the points I was making.
> 
> > > This data should simply list for each pin what the valid mux options are.
> > > (or for each pin group if mux registers control n pins at a time as on
> > > Tegra)
> > 
> > ..and we already have four completely different function tables and five
> > subsets/incremental changes for the same IP block for omap2/3/4.
> 
> Well, sure, you have much different HW.
> 
> But the pinctrl core should only receive the 1 specific complete set of
> pins/groups/functions for the particular SoC that the code is running
> on.

Yes I completely agree.
 
> Anything to do with constructing that list or representing it in device
> tree is something implementation detail for the individual pinctrl driver.
> 
> If you have the tables in code, you can presumably merge them together
> while implementing the pinctrl subsystem's callbacks. If you have the
> tables in device tree, you can presumably merge them together when running
> dtc, using /include/ statements from the SoC *.dtsi file.

That's correct too. The real question is where do we place the data
that is just growing all the time..

Regards,

Tony

RE: Pin control mappings for DT

[Stephen Warren]

Linus Walleij wrote at Tuesday, November 29, 2011 12:42 AM:
> can we have some initial idea about DT support for pin control
> devices and how these will look? Since pin control seems to be
> a cruicial piece for people to get board data out of their kernels
> we'd better have a common understanding.
> 
> The way I've understood it, we will need three pieces of
> DT data:
> 
> 1) Per-driver info, includes the pin controller base, its pins, their
>  (optional) names and their specific presets like bias etc.

I'm still planning on putting this all into the driver source code; I
don't really see any advantage of putting this almost static data into
DT just to parse it back out into the same tables that could be written
straight into the driver anyway. (only almost static rather than static
since we'll need new tables for each SoC)

> 2) Board pin group definitions, saying "these three pins
>   belong to the group "i2c0-pins"

I think this is part of (1), if present in the DT at all.

This is HW description data, rather than higher level data being added
for the purpose of more abstract pinctrl reasons. As such, it belongs in
the pinmux HW's node.

I don't think we should force this data to be in DT at all if the driver
includes the tables itself.

There's more to a pin group than just the list of pins it includes; on
Tegra20 at least, all muxing/configuration/... is at the pin group level
rather than the pin level, so each pin group needs register address,
some representation of which configuration options are valid for it, etc.
As such, any core DT binding specified would cover a limited subset of
the data that needs to be parsed.

That said, we could standardize on the way to list the set of pins in
each group, and even provide helper functions to enumerate the list of
groups and parse the pin list, so that drivers that wish to include this
information in their bindings can use it. But I still think the driver
should be responsible for obtaining the data however it sees fit, and
passing that to the pinctrl core using the existing APIs.

> 3) Board function mapping definitions saying "the pin group
>   'i2c0-pins' is to be used with device i2c0" which can map
>   several groups to a single device.

Yes, I see this as the domain of the core pinctrl DT bindings.

> (1) should be read out by each pin controller driver and
> (2)+(3) by the pin controller, specifically  pinmux core.
...

-- 
nvpublic

Re: Pin control mappings for DT

[Linus Walleij]

On Tue, Nov 29, 2011 at 6:55 PM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Linus Walleij wrote at Tuesday, November 29, 2011 12:42 AM:

>> 1) Per-driver info, includes the pin controller base, its pins, their
>>  (optional) names and their specific presets like bias etc.
>
> I'm still planning on putting this all into the driver source code; I
> don't really see any advantage of putting this almost static data into
> DT just to parse it back out into the same tables that could be written
> straight into the driver anyway. (only almost static rather than static
> since we'll need new tables for each SoC)

I think OMAP already have a few different SoCs using the same
pinmux HW block, so for them it makes more sense.

Maybe when you have your Tegra 7 chips using the same
controller as Tegra { 3, 4, 5, 6 } with yet another pin list
it will make sense :-)

Besides - doing it one way does not exclude the other.

If the tables gets large I will have Torvalds on my tail for
putting nonsensical lists into the kernel that'd better been
kept outside it.

Yours,
Linus Walleij

Re: Pin control mappings for DT

[Tony Lindgren]

* Linus Walleij <linus.walleij@linaro.org> [111130 05:00]:
> On Tue, Nov 29, 2011 at 6:55 PM, Stephen Warren <swarren@nvidia.com> wrote:
> > Linus Walleij wrote at Tuesday, November 29, 2011 12:42 AM:
> 
> >> 1) Per-driver info, includes the pin controller base, its pins, their
> >>  (optional) names and their specific presets like bias etc.
> >
> > I'm still planning on putting this all into the driver source code; I
> > don't really see any advantage of putting this almost static data into
> > DT just to parse it back out into the same tables that could be written
> > straight into the driver anyway. (only almost static rather than static
> > since we'll need new tables for each SoC)
> 
> I think OMAP already have a few different SoCs using the same
> pinmux HW block, so for them it makes more sense.
> 
> Maybe when you have your Tegra 7 chips using the same
> controller as Tegra { 3, 4, 5, 6 } with yet another pin list
> it will make sense :-)
> 
> Besides - doing it one way does not exclude the other.

Yeah we should be pretty easily able to support any combination
of the following data sources:

- Static maps passed from board/machine
- Mux data being loaded dynamically from loadable pinmux modules
- Mux data and maps being loaded dynamically from DT
 
> If the tables gets large I will have Torvalds on my tail for
> putting nonsensical lists into the kernel that'd better been
> kept outside it.

Yeah the problem is that data will just grow and grow because
packages change with shrinkage and need to support non-POP and
POP chips. So no matter where you stuff this data, the amount
of if will just grow. Passing the mux data from DT has the
advantage of only passing only one single instance of hopefully
correct data.

So probably some combination of static data, loading data from
loadable modules, and passing some from DT will be the optimal
solution.

Regards,

Tony
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss