From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751170AbeAVRF6 (ORCPT ); Mon, 22 Jan 2018 12:05:58 -0500 Received: from esa1.microchip.iphmx.com ([68.232.147.91]:35964 "EHLO esa1.microchip.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751091AbeAVRF4 (ORCPT ); Mon, 22 Jan 2018 12:05:56 -0500 X-IronPort-AV: E=Sophos;i="5.46,397,1511852400"; d="scan'208";a="11248474" Date: Mon, 22 Jan 2018 18:05:45 +0100 From: Ludovic Desroches To: Linus Walleij CC: Ludovic Desroches , Linux ARM , , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Nicolas Ferre , Alexandre Belloni , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] pinctrl: at91-pio4: add support for drive-strength property Message-ID: <20180122170545.GD3055@rfolt0960.corp.atmel.com> Mail-Followup-To: Linus Walleij , Linux ARM , linux-gpio@vger.kernel.org, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Nicolas Ferre , Alexandre Belloni , "linux-kernel@vger.kernel.org" References: <20180118160208.16461-1-ludovic.desroches@microchip.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jan 22, 2018 at 09:37:38AM +0100, Linus Walleij wrote: > On Thu, Jan 18, 2018 at 5:02 PM, Ludovic Desroches > wrote: > > > Add support for the drive-strength property. Usually its value is > > expressed in mA. Since the numeric value depends on VDDIOP voltage, > > the controller uses low, medium and high to define the drive-strengh. > > Aha I see. That's complex. It certainly results in a certain mA drive > strength in the end, but what you're saying is that this is not usually > what we configure. > The PIO Controller Interface is: Drive Strength: 0 Low drive 1 Low drive 2 Medium drive 3 High drive In the Electrical Characteristics, you have: GPIO 1.8V Low: max 1 mA GPIO 1.8V Medium: max 10 mA GPIO 1.8V High: max 18 mA GPIO 3.3V Low: max 2 mA GPIO 3.3V Medium: max 20 mA GPIO 3.3V High: max 32 mA > > The PIO controller accepts two values for the low drive: 0 or 1. Most > > of the time, we don't care about the drive strength, there is no need > > to change it, so 0 is considered as the default value. > > Do you mean default value as in "whatever the hardware was set > up as at boot time"? Yes > > > The low-drive > > value won't be advertised through pinconf-pins file excepted if it > > except? Yes > > > has been set explicitly in the device tree ie if its value is > > different from 0. > > > > Signed-off-by: Ludovic Desroches > > OK I think I get it. > > > Optional properties: > > - GENERIC_PINCONFIG: generic pinconfig options to use, bias-disable, > > -bias-pull-down, bias-pull-up, drive-open-drain, input-schmitt-enable, > > -input-debounce, output-low, output-high. > > +bias-pull-down, bias-pull-up, drive-open-drain, drive-strength, > > +input-schmitt-enable, input-debounce, output-low, output-high. > (...) > > + drive-strength = ; > > So you say you support this argument and it will be something like > > include/dt-bindings/pinctrl/at91.h:#define ATMEL_PIO_DRVSTR_LO 1 > include/dt-bindings/pinctrl/at91.h:#define ATMEL_PIO_DRVSTR_ME 2 > include/dt-bindings/pinctrl/at91.h:#define ATMEL_PIO_DRVSTR_HI 3 > > But the definition if generic drive strength is actually in mA. > > I think it is OK to deviate from stating it in mA, but you should > write this in the DT bindings so people do not get confused. > Ok > > @@ -814,6 +834,19 @@ static void atmel_conf_pin_config_dbg_show(struct pinctrl_dev *pctldev, > > seq_printf(s, "%s ", "open-drain"); > > if (conf & ATMEL_PIO_SCHMITT_MASK) > > seq_printf(s, "%s ", "schmitt"); > > + if (conf & ATMEL_PIO_DRVSTR_MASK) { > > + switch ((conf & ATMEL_PIO_DRVSTR_MASK) >> ATMEL_PIO_DRVSTR_OFFSET) { > > + case ATMEL_PIO_DRVSTR_LO: > > + seq_printf(s, "%s ", "low-drive"); > > + break; > > + case ATMEL_PIO_DRVSTR_ME: > > + seq_printf(s, "%s ", "medium-drive"); > > + break; > > + case ATMEL_PIO_DRVSTR_HI: > > + seq_printf(s, "%s ", "high-drive"); > > + break; > > But here you know what VDDIOP is, right? > No, I have no way to retrieve the power for the several VDDIOP power rails. > So in debugfs you could actually print the real drive strength > in mA, or both "medium-drive, %u mA"? > > Yours, > Linus Walleij Regards Ludovic