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

[Chris Brandt]

Hello Andy,

On Monday, May 08, 2017, Andy Shevchenko wrote:
> > 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?

For #1, IP_ENI and IP_ENO are controlled by PFC/PFCE/PFCAE.
Those basically equate to the pin function register (as in,
what IP block gets wired up to each pin.) However, it
seems that PFC/PFCE/PFCAE cannot enable both IP_ENI and
IP_ENO signals at the same time (the diagram doesn't really
show you that piece of info), hence they had to make an
'override' register can called it PBDC (bir-dir register)
to manually turn the input buffers on when needed.

Seems like a HW hack if you ask me.


> 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.

So your suggestion is to rename PIN_CONFIG_OUTPUT to
PIN_CONFIG_OUTPUT_INPUT_ENABLE?

I would say the description for @PIN_CONFIG_INPUT_ENABLE is probably
'technically' correct for our bi-dir needs, but I didn't like it
because it might confuse users making a DT file for their board (unless
of course they studied the hardware manual in detail to finally come
to the conclusion of the screwy PFC hardware).


> >> >  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?

The one issue I was trying to avoid by hiding things in the driver
with some type of look-up table was that this series of parts comes
in different subsets/packages and sometimes the functions comes out
on different port numbers. So now you need multiple look-up tables
and then also a way to signal what part/package you have so you use
the correct look-up table. It seemed easier (and safer) to just pass
the info in (if you needed it) in the Device Tree for the board.

Of these 'special' pins (Table 54.7):

For 16 of them, they truly can operate as input or output, so
the user needs to specify that direction they need in the DT.

For LVDS, Sound and WDT, those will be fixed, so they would be
the only ones you could do a table for, but as I mentioned, Sound
and WDT don't always come out in the same place so a lookup table
isn't so cut and dry.


> >> 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.

I see from your other email you sent to Jacopo, it looks like the link
I sent you only brought you to the shorter 'data sheet' version of the
hardware manual, not the full manual that includes 'Figure 54.1'.
Sorry about that.



Chris