Hi Andy,

On Mon, May 08, 2017 at 08:47:17PM +0300, Andy Shevchenko wrote:

On Mon, May 8, 2017 at 8:25 PM, jmondi [off-list ref] wrote:

quoted

Andy,

On Mon, May 08, 2017 at 07:08:32PM +0300, Andy Shevchenko wrote:

quoted

On Mon, May 8, 2017 at 7:01 PM, jmondi [off-list ref] wrote:

quoted

On Sun, May 07, 2017 at 09:52:49AM +0200, Linus Walleij wrote:

quoted

On Fri, Apr 28, 2017 at 4:53 PM, Andy Shevchenko
[off-list ref] wrote:

quoted

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.

quoted

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?

I really don't know what to add to what Chris said in his 2 previous
replies to the same question, and I don't know if I can even get this
information as the most detailed drawing I can provide is what you
have seen already at page 2696 Fig. 54.1 of the following document.

https://www.renesas.com/en-us/doc/products/mpumcu/doc/rz/r01uh0403ej0300_rz_a1h.pdf?key=ccbb2d79446f1cbd015031061140507c

I didn't see before this document. (I had downloaded what Chris
referred to, which has less than 1200 pages).

The figure you pointed to is really nice and explains it, thank you.

Oh sorry, I thought you had seen this already :)

So, BiDi in this hardware is just helper to enable Input
simultaneously when you enable output.

This makes me wonder what prevents you to do this in two steps in software?
So, basically in terms of pin control framework you define this pin
configuration as

1. PIN_CONFIG_INPUT_ENABLE:
2. PIN_CONFIG_OUTPUT:

(or wise versa)

That could be doable, as when we're collecting generic pin
configuration to apply to the pin I can simply check if both of them
are enabled.

That would feel un-natural in dts anyway, for someone that is not that
into the pin controller sub system details.
If I would have to do something like  this, not knowing all the
reasonable pre-conditions we've been discussing about

pins {
    pinmux = < .. >;
    input-enable;
    output-high; /* or output-low, we can ignore the argument here */
}

In place of

pins {
   pinmux = < .. >;
   renesas,bi-directional;
}

And the hardware manual speaks of "bi-directional" everywhere, I would
be wondering what those guys implementing this were thinking :)

quoted

From my perspective these flags are configurations internal to the pin
controller hardware used to enable/disable input buffers when a pin needs to
perform in both direction.

quoted

The level of detail I can provide on this is the logical diagram we have pointed
you to already.

As I assume you are trying to get this answer from us in order to
avoid duplicating things in pin controller sub-system, and I
understand this, but my question here was "can we have those flags as part
of the pinmux property argument list, as that property description
seems to allow us to do that, instead of making them generic pin
configuration properties and upset other developers?"

I guess Linus is better than me could answer to this.

quoted

Anyway, I still fail to see why those configuration flags, only
affecting the way the pin controller hardware enables/disables
its internal buffers and its internal operations have to be
described in term of their externally visible electrically characteristics.

quoted

Second question, what makes it differ to what already exists?

To me, what already exists are pin configuration properties generic to
the whole pin controller subsystem, and I understand you don't want to
see duplication there.

At the same time, to me, those flags are settings the pin controller
wants to have specified by software to overcome its hw design flaws,
and are intended to configure its internal buffers in a way it cannot
do by itself for some very specific operation modes (they are listed
in the hw reference manual, it's not something you can chose to
configure or not, if you want a pin working in i2c mode, you HAVE to
pass those flags to pin controller).

So, when you configuring pinmux to use group of pins to be i2c, what
does prevent you to apply those settings implicitly?

Chris already gave some valid reasons why it would be hard to do this
considering the different part numbers this driver may handle, but I
would also like to add that I have counted > 100 cases where
bi-directional flag has to be applied just in the first 5 IO ports (on a
total of 13).

As there are RZ systems out there running with just < 9MB of SRAM,
adding a static table (or several, considering the different part numbers)
with at least 300 entries, is a considerable waste :(

For SWIO it would be easier, there are just 16 cases, all of them
listed in the hardware reference manual as Chris said.

Thanks
   j

--
With Best Regards,
Andy Shevchenko