[I know Stephen is on vacation, just stacking up some nice reading for when he gets back and for others to enjoy]
On Fri, Sep 2, 2011 at 6:33 PM, Stephen Warren <swar...@nvidia.com> wrote: > Linus Walleij wrote at Friday, September 02, 2011 6:59 AM: >> >> diff --git a/include/linux/pinctrl/pinmux.h >> >> b/include/linux/pinctrl/pinmux.h >> > >> >> +/** >> >> + * struct pinmux_ops - pinmux operations, to be implemented by pin >> >> controller >> >> + * drivers that support pinmuxing >> >> + * @request: called by the core to see if a certain pin can be made >> >> available >> >> + * available for muxing. This is called by the core to acquire the pins >> >> + * before selecting any actual mux setting across a function. The >> >> driver >> >> + * is allowed to answer "no" by returning a negative error code >> >> + * @free: the reverse function of the request() callback, frees a pin >> >> after >> >> + * being requested >> > >> > Shouldn't request/free be for a pingroup, not a pin, since that's what >> > functions are assigned to? Also, the function should be passed, since >> > some restrictions these functions might need to check for might depend >> > on what the pin/group will be used for. >> >> This is not for checking anything on function or group level. >> It's exclusively for things that need to be on a per-pin level, so any >> pin can deny being selected for muxing at any time. >> >> So what you're saying is that you need a function to check >> also on group level as part of the pinmux_get() sematics? >> >> We can add that and have two optional checks: @request_pin() >> on pin level and say @request_function_and_group() on the >> group+function level, would that work for your scenarios? > > Well, that's a move in the right direction, but not quite everything I'd > like. > > The basic issue is that a single logical function (I2C controller 0) can > be mux'd out onto more than one pingroup. However, the HW requires that > at /any given time/ it only be mux'd out onto a single pingroup. > > To fully enforce this, the request() function needs to know what the > complete state will be after the entire (set of) mapping entries is > processed. I'm not quite following this, but I'm pretty sure that this verification can be bolted onto the subsystem later with apropriate callbacks. It looks like you're after a state holder to avoid pinmux_get() to activate the same muxing in two different places, it's then a safety mechanism agains supplying bad mappings and doing pinmux_get() on stuff before doing pinmux_put() on conflicting mappings. > When the core can only process 1 mapping entry at a time, this is trivial, > since there's only 1 change relative to current HW to process in request(). You will have this in v7 :-) I was aware that this would cause new semantic issues, but since I think only Tegra will use that feature as of now, we can postpone this group denial stuff until there is a driver for it I think, it shouldn't be too hard to add, and you can still get the driver to a working state, it's just that it'll be possible to do some nasty things with it if you give it weird mapping tables and calls. > The only way I can see this being implementable is: > > a) Add request_start() and request_end() functions, so the driver can > copy HW state to SW state in request_start(), and modify this cached > state in each request() call, and hence has access to all state changes > to-date in each request() call. Then, request_end() to finalize the > whole set of changes. > > Or: > > b) request() passes an array of changes at once, instead of many calls > each requesting a single .change > > This is certainly a tough problem. The current Tegra pinmux driver doesn't > actually make any attempt to enforce this, so perhaps it's not worth > worrying about; we just assume people write sensible mapping tables. Ok we are at that stage with the subsystem now so let's wait with that until we have a driver we can fix. >> > When the core is modified to support applying n entries in the mapping >> > table for each pinmux_get() call, how will request/free be aware of the >> > partial pending state? >> >> That is like answering how code I haven't yet written will >> look like... The easiest answer is to implement it I think, >> then you can check if it looks sane. > > :-) As it is implemented now it backs off and free all pins if there is an error on any group when doing pinmux_get(). Yours, Linus Walleij _______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
