Re: [PATCH 2/2 v4] pinctrl: introduce generic pin config

[Linus Walleij]

On Thu, Dec 1, 2011 at 6:16 PM, Stephen Warren <swarren@xxxxxxxxxx> wrote:
> Linus Walleij wrote at Thursday, December 01, 2011 3:12 AM:

>> That will get messy when if I refactor this stuff, add new enums
>> and whatever.
>
> I don't understand what'd be difficult about that.
>
> New standardized enums could be added with values without the top bit
> set. No existing driver would need modification, since their switch(param)
> would not have a case for that new value, and would just return an error.

That would be mixing binary #defines and enums in an unholy
manner, that sounds bad to me.

If you want to do things like that I should replace the current

enum pin_config_param {
        PIN_CONFIG_BIAS_DISABLE,
        PIN_CONFIG_BIAS_HIGH_IMPEDANCE,
       ...
}

With

#define PIN_CONFIG_BIAS_DISABLE 0x00000001
#define PIN_CONFIG_BIAS_HIGH_IMPEDANCE 0x00000002
...

Else it seems like a bit wicked mix-up. So is this what we
should do?

I am reluctant since it mirrors the problem in the GPIO
numberspace where you have no clue what, say GPIO
164 is on a multi-platform binary. It depends on which
platform it booted on. The same will be true for such
enums/defines above the predefined range, totally
depending on the system at hand.

>> Like in the generic debugfs dump function:
>>
>>       if (!ops->is_generic)
>>               return;
>>
>> If I take this out, the generic debugfs code will be used for
>> everything.
>
> I think that'd be fine; the generic code would do all the debug prints
> for the standardized enums, then the core would call into the pinctrl
> driver to perform any additional debug prints for any driver-defined
> custom parameters.

I think Marks point earlier was that he wanted the possibility
to cut out *all* the generic stuff and have only custom config
enumerators for a certain pin controller.

>> And then the generic sematics which you didn't like in the
>> previous patch:
>>
>> if (ret == -EINVAL || ret == -ENOTSUPP)
>>
>> Need to go back in, else the generic debugfs stuff won't
>> work.
>
> That'd be fine, provided the loop only checked the standardized parameters,
> or only enabled that special error-checking case for standardized parameters.

Same comment as above. This won't cut it for all-custom
pin controllers, they will get unwanted semantics pushed at
them.

Thanks,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/