Re: [PATCH v5 01/10] pinctrl: generic: Add bi-directional and output-enable

From: Andy Shevchenko
Date: Mon May 08 2017 - 14:27:00 EST


On Mon, May 8, 2017 at 8:02 PM, Chris Brandt <Chris.Brandt@xxxxxxxxxxx> wrote:
> On Monday, May 08, 2017, Andy Shevchenko wrote:
>> On Mon, May 8, 2017 at 7:01 PM, jmondi <jacopo@xxxxxxxxxx> wrote:
>> > On Sun, May 07, 2017 at 09:52:49AM +0200, Linus Walleij wrote:
>> >> On Fri, Apr 28, 2017 at 4:53 PM, Andy Shevchenko
>> >> <andy.shevchenko@xxxxxxxxx> wrote:
>> >>
>> >> > Linus, for me it looks like better to revert that change, until we
>> >> > will have clear picture why existing configuration parameters can't
>> >> > work.
>> >>
>> >> Yeah I'll revert the binding for fixes.
>>
>> > As it seems we won't be able to proceed with the currently proposed
>> solution,
>> > would that be acceptable now that we use the "pinmux" property to add
>> > flags as BIDIR
>>
>> Can you explain what does this *electrically* mean?
>
> Bi-Directional:
>
> For any pin that needs to drive (send) or sense (receive) signals, the pin
> mux controller can only enable 1 direction of buffers (in this case, it
> only the output buffers). Therefore the appropriate bit in the
> 'bi-directional' register is set in order to enable the signal path in both
> directions (ie, enable the input buffers).

So, I see this way how it can be enabled:
1. IP_ENI + IP_ENO internally defines BiDi when PMC and PIPC bits are set
2. BiDi bit is set and BUFOE is set

Now the question is what the real use case for 2?

If we find one we need to rename and fix a description of the pin
control configuration property.

like:
@PIN_CONFIG_OUTPUT_INPUT_ENABLE: this will configure the pin as an output.
...
Note: As long as it's enabled the pin's input enabled as well and vice versa.

>> > and SWIO_[INPUT|OUTPUT] directly there?
>
> In the hardware manual, there is a list of pin functions that if you want
> to use, you cannot use the stand pinmux register method that you use for
> all the other pins. Instead, you are instructed to do a different
> procedure. If course electrically, input/output buffers are still turned
> on/off or whatever, but the root reason of why you need to do this
> differently for these specific pin/function is not known.
>
> The "SWIO_" portion of the naming comes from the hardware manual which
> refers to this as "Software I/O Control Alternative Mode" (which in my
> opinion means the HW guys couldn't get the pin directions/buffers to be set
> automatically for some reason, so they decided it's the SW guys problem now
> for those pins)

Okay, these are related to pin muxing explicitly.
So, you have 10 functions overall?
What prevents you describe them accordingly and hide this
implementation detail in the driver?

>> Second question, what makes it differ to what already exists?
>
> We have 3 different flags (properties) that need to be specified for some
> pins in some circumstances.
> At first, we just tried to pass this additional information in when
> defining what function the pin should be just for those pins whose
> direction (ie, buffers) would not be set up automatically.
> However, this was rejected and we were told to pick from the existing set
> generic properties.
> But, 3 different generic pinctrl properties that fit what we needed didn't
> exist. So, we used the existing "input-enable" and "output-enable", but
> then created "bi-directional".

Yes, that figure helped me a lot to understand.

--
With Best Regards,
Andy Shevchenko