From: Chris Brandt <Chris.Brandt@renesas.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
CC: Linus Walleij <linus.walleij@linaro.org>,
        Jacopo Mondi <jacopo+renesas@jmondi.org>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>,
        "Russell King - ARM Linux" <linux@armlinux.org.uk>,
        Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
        "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
        devicetree <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v5 01/10] pinctrl: generic: Add bi-directional and
 output-enable
Thread-Topic: [PATCH v5 01/10] pinctrl: generic: Add bi-directional and
 output-enable
Thread-Index: AQHSvy8kW/T8jbNXV0G/jE8WsCEqd6HZTneAgAEnEICAACrhkIAAE36AgAAKEjCAACHsAIAAARSggAALXoCAAAXYkA==
Date: Fri, 28 Apr 2017 16:46:14 +0000
Message-ID: <SG2PR06MB1165288487474DE8F71752208A130@SG2PR06MB1165.apcprd06.prod.outlook.com>
References: <1493281194-5200-1-git-send-email-jacopo+renesas@jmondi.org>
 <1493281194-5200-2-git-send-email-jacopo+renesas@jmondi.org>
 <CAHp75Vcy1OE+9htFx9k3kQhGARQPT1C0gg3SEKZd7ULu_vkUMA@mail.gmail.com>
 <CACRpkdayW0qPXDcc_JofkwtX5UHhDPxiY_8deZZ7+hPNJkjgeQ@mail.gmail.com>
 <SG2PR06MB1165E7E651BE96CD7B7738B08A130@SG2PR06MB1165.apcprd06.prod.outlook.com>
 <CAHp75Vcp_eMWrjZRX-x3=Tt3JRXcL_wWG43MymF=oyLLP4EJwg@mail.gmail.com>
 <SG2PR06MB11658EC3A3A80C57E6F539AC8A130@SG2PR06MB1165.apcprd06.prod.outlook.com>
 <CAHp75VeLtOuty=iaxuCaH43rnBZDTT03iH7JyjDJBQhTD-EjzA@mail.gmail.com>
 <SG2PR06MB11658E6434EB343246ADC9F58A130@SG2PR06MB1165.apcprd06.prod.outlook.com>
 <CAHp75VfYPfm_GKPzvX7Go010fRi99t12ub7VUPubB6z3HnsFLg@mail.gmail.com>
In-Reply-To: <CAHp75VfYPfm_GKPzvX7Go010fRi99t12ub7VUPubB6z3HnsFLg@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Apr 2017 16:46:14.4635
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 53d82571-da19-47e4-9cb4-625a166a4a2a
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SG2PR06MB1166
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id v3SGl72a006539
Content-Length: 2113
Lines: 51

On Friday, April 28, 2017, Andy Shevchenko wrote:
> > We were using "input-enable" to signal when the pin function that we set
> also needs to be forcible set to input by the software (once again,
> because the HW is not smart enough to do it on its own), but is different
> than the bi-directional functionality (ie, a different register setting).
> 
> You are trying to introduce an abstraction, called BiDi, which is
> *not* a separate thing from a set of pin properties.

Note, I'm talking about 2 different issues we had:

1) Pins that need input and output buffers enabled during normal use. We created "bi-directional" for that.

2) For whatever reason, the HW manual points out that the PFC hardware can't really automatically set buffers enables correctly for some pin instances, and we have to manually assign the pin as input or output using another register. For that, we were using "input-enable" and "output-enable".


For #2:

> > Note that we added a enable-output for the same reason.
> > See RZ/A1H HW Manual section "Table 54.7 Alternative Functions that
> PIPCn.PIPCnm Bit Should be Set to 0"
> 
> Perhaps needs to be revisited as well.

Sorry, we didn't 'add' anything new. The property "output-enable", (ie, PIN_CONFIG_OUTPUT) already existed and describes what we are doing in the case for output.

But, we still have the issue that we have 2 cases that need the input enabled, but they are not the same situation, so we can't just use "input-enable" for both.


My only suggestion is (and I'm not sure this is possible in the driver):

"input-enable"  : case #2 where you need the pin to be forced as an input
"output-enable" : case #2 where you need the pin to be forced as an output
"input-enable" + "output-enable" : case #1 (replaces "bi-directional").

For example:

	i2c2_pins: i2c2 {
		pinmux = <RZA1_PINMUX(1, 4, 1)>, <RZA1_PINMUX(1, 5, 1)>;
		input-enable;
		output-enable;
	};

So in the SW driver, if we see both, that will signal to us what is going on and what to do about it (as in, set the bi-directional register and not the input direction register).


Thoughts?


Chris