From patchwork Fri Jun 7 23:13:48 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Heiko Stuebner X-Patchwork-Id: 2691581 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) by patchwork2.kernel.org (Postfix) with ESMTP id 13DCADFB78 for ; Fri, 7 Jun 2013 23:14:37 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Ul5rX-0002wB-NO; Fri, 07 Jun 2013 23:14:28 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Ul5rU-00020i-TM; Fri, 07 Jun 2013 23:14:24 +0000 Received: from gloria.sntech.de ([95.129.55.99]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Ul5rR-0001zq-Ib for linux-arm-kernel@lists.infradead.org; Fri, 07 Jun 2013 23:14:23 +0000 Received: from 146-52-32-150-dynip.superkabel.de ([146.52.32.150] helo=marty.localnet) by gloria.sntech.de with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.72) (envelope-from ) id 1Ul5qx-0000gU-3M; Sat, 08 Jun 2013 01:13:51 +0200 From: Heiko =?iso-8859-1?q?St=FCbner?= To: Linus Walleij Subject: Re: [PATCH v2 5/8] pinctrl: add pinctrl driver for Rockchip SoCs Date: Sat, 8 Jun 2013 01:13:48 +0200 User-Agent: KMail/1.13.7 (Linux/3.2.0-3-686-pae; KDE/4.8.4; i686; ; ) References: <201306062107.58875.heiko@sntech.de> <201306062111.40983.heiko@sntech.de> In-Reply-To: MIME-Version: 1.0 Message-Id: <201306080113.49186.heiko@sntech.de> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130607_191421_734971_2FF6D08E X-CRM114-Status: GOOD ( 32.63 ) X-Spam-Score: -2.4 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.4 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.5 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Thomas Petazzoni , Mike Turquette , Arnd Bergmann , Seungwon Jeon , "devicetree-discuss@lists.ozlabs.org" , "linux-mmc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Rob Herring , Jaehoon Chung , Olof Johansson , Grant Likely , Russell King , Chris Ball , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org Am Freitag, 7. Juni 2013, 14:53:51 schrieb Linus Walleij: > On Thu, Jun 6, 2013 at 9:11 PM, Heiko Stübner wrote: > > + for (i = 0, j = 0; i < size; i += 4, j++) { > > + unsigned long pinconf; > > + > > + num = be32_to_cpu(*list++); > > + bank = bank_num_to_bank(info, num); > > + if (IS_ERR(bank)) > > + return PTR_ERR(bank); > > + > > + pin = be32_to_cpu(*list++); > > + grp->pins[j] = bank->pin_base + pin; > > + grp->func[j] = be32_to_cpu(*list++); > > + > > + pinconf = be32_to_cpu(*list++); > > + switch(pinconf) { > > + case RK_PINCTRL_NONE: > > + bias = PIN_CONFIG_BIAS_DISABLE; > > + break; > > + case RK_PINCTRL_PULL_PIN_DEFAULT: > > + bias = PIN_CONFIG_BIAS_PULL_PIN_DEFAULT; > > + break; > > + case RK_PINCTRL_PULL_UP: > > + bias = PIN_CONFIG_BIAS_PULL_UP; > > + break; > > + case RK_PINCTRL_PULL_DOWN: > > + bias = PIN_CONFIG_BIAS_PULL_DOWN; > > + break; > > + } > > + > > + grp->configs[j] = pinconf_to_config_packed(bias, 0); > > + } > > I would like this to be added to > drivers/pinctrl/pinconf-generic.c > as a utility function, with the header in > drivers/pinctrl/pinconf.h > > Also in a separate patch. > > > +++ b/include/dt-bindings/pinctrl/rockchip.h > > (...) > > > +#define RK_PINCTRL_NONE 0 > > +#define RK_PINCTRL_PULL_PIN_DEFAULT (1 << 0) > > +#define RK_PINCTRL_PULL_UP (1 << 1) > > +#define RK_PINCTRL_PULL_DOWN (1 << 2) > > So I'd like these moved to /include/dt-bindings/pinctrl/pinconfig.h > and used as a separete include. Drop the RK_* prefix as it > will be universal and use a PINCONFIG_* prefix instead > of PINCTRL_*. > > I think that is the route we need to take. > > Bonus if you implement more config options from > pinconf-generic.h but otherwise me and others will have > to do it. Gah, damn me for always wanting to get bonus points ;-), I did play around a bit with the idea you described above and came up with the stuff a bit below. This is in no way meant to be finished, I also only was able to compile test it yet, but it looks like it might work when I test it tomorrow. So if possible I would just like to get a "looks halfway sane, continue" or "completely wrong track" please :-) : constants in dt-bindings/pinctrl/pinconfig.h: ----------------------------- #define PINCONFIG_NONE 0 #define PINCONFIG_BIAS_DISABLE (1 << 0) #define PINCONFIG_BIAS_HIGH_IMPEDANCE (1 << 1) #define PINCONFIG_BIAS_BUS_HOLD (1 << 2) #define PINCONFIG_BIAS_PULL_PIN_DEFAULT (1 << 3) #define PINCONFIG_BIAS_PULL_UP (1 << 4) #define PINCONFIG_BIAS_PULL_DOWN (1 << 5) #define PINCONFIG_DRIVE_PUSH_PULL (1 << 6) #define PINCONFIG_DRIVE_OPEN_DRAIN (1 << 7) #define PINCONFIG_DRIVE_OPEN_SOURCE (1 << 8) #define PINCONFIG_INPUT_SCHMITT_ENABLE (1 << 9) #define PINCONFIG_INPUT_SCHMITT_DISABLE (1 << 10) #define PINCONFIG_LOW_POWER_MODE (1 << 11) #define PINCONFIG_OUTPUT_LOW (1 << 12) #define PINCONFIG_OUTPUT_HIGH (1 << 13) and code including how the rockchip driver could look like: --------- diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c index 9a6812b..35c40be 100644 --- a/drivers/pinctrl/pinconf-generic.c +++ b/drivers/pinctrl/pinconf-generic.c @@ -139,3 +139,61 @@ void pinconf_generic_dump_config(struct pinctrl_dev *pctldev, } EXPORT_SYMBOL_GPL(pinconf_generic_dump_config); #endif + +/* + * Maps the devicetree config bits to actual pinconf values. + * The array indizes match the bits set in dt-bindings/pinctrl/pinconf.h + * and the array should contain an entry for each bit defined there + */ +static unsigned long conf_map[] = { + PIN_CONF_PACKED(PIN_CONFIG_BIAS_DISABLE, 0), + PIN_CONF_PACKED(PIN_CONFIG_BIAS_HIGH_IMPEDANCE, 0), + PIN_CONF_PACKED(PIN_CONFIG_BIAS_BUS_HOLD, 0), + PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_PIN_DEFAULT, 0), + PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_UP, 0), + PIN_CONF_PACKED(PIN_CONFIG_BIAS_PULL_DOWN, 0), + PIN_CONF_PACKED(PIN_CONFIG_DRIVE_PUSH_PULL, 0), + PIN_CONF_PACKED(PIN_CONFIG_DRIVE_OPEN_DRAIN, 0), + PIN_CONF_PACKED(PIN_CONFIG_DRIVE_OPEN_SOURCE, 0), + PIN_CONF_PACKED(PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1), + PIN_CONF_PACKED(PIN_CONFIG_INPUT_SCHMITT_ENABLE, 0), + PIN_CONF_PACKED(PIN_CONFIG_LOW_POWER_MODE, 0), + PIN_CONF_PACKED(PIN_CONFIG_OUTPUT, 0), + PIN_CONF_PACKED(PIN_CONFIG_OUTPUT, 1), +}; + +/* + * Parse a pinconf bitmap from a devicetree entry into individual pin configs. + * @pinconf: the bitmap containing config bits + * @configs: after the function returns contains a pointer to an array of + * pin configs + * @nconfigs: number of entries of configs + */ +int pinconf_generic_parse_dt_bitmap(unsigned long pinconf, + unsigned long *configs, + unsigned int *nconfigs) +{ + int bit; + int i; + + *nconfigs = hweight_long(pinconf); + configs = kzalloc(*nconfigs * sizeof(unsigned long), GFP_KERNEL); + + i = 0; + while (pinconf && i < *nconfigs) { + bit = __ffs(pinconf); + pinconf &= ~BIT(i); + + if (bit > ARRAY_SIZE(conf_map)) { + pr_err("%s: unknown bit %d\n", __func__, bit); + kfree(configs); + return -EINVAL; + } + + configs[i] = conf_map[bit]; + + i++; + } + + return 0; +} diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h index 92c7267..0fab53a 100644 --- a/drivers/pinctrl/pinconf.h +++ b/drivers/pinctrl/pinconf.h @@ -123,3 +123,9 @@ static inline void pinconf_generic_dump_config(struct pinctrl_dev *pctldev, return; } #endif + +#ifdef CONFIG_GENERIC_PINCONF +int pinconf_generic_parse_dt_bitmap(unsigned long pinconf, + unsigned long *configs, + unsigned int *nconfigs); +#endif diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c index b77e241..88398df 100644 --- a/drivers/pinctrl/pinctrl-rockchip.c +++ b/drivers/pinctrl/pinctrl-rockchip.c @@ -40,6 +40,7 @@ #include #include "core.h" +#include "pinconf.h" /* GPIO control registers */ #define GPIO_SWPORT_DR 0x00 @@ -639,7 +640,8 @@ static int rockchip_pinctrl_parse_groups(struct device_node *np, int num; int i, j; int pin; - int bias; + int ret; + int nconfigs; dev_dbg(info->dev, "group(%d): %s\n", index, np->name); @@ -683,22 +685,16 @@ static int rockchip_pinctrl_parse_groups(struct device_node *np, grp->func[j] = be32_to_cpu(*list++); pinconf = be32_to_cpu(*list++); - switch(pinconf) { - case RK_PINCTRL_NONE: - bias = PIN_CONFIG_BIAS_DISABLE; - break; - case RK_PINCTRL_PULL_PIN_DEFAULT: - bias = PIN_CONFIG_BIAS_PULL_PIN_DEFAULT; - break; - case RK_PINCTRL_PULL_UP: - bias = PIN_CONFIG_BIAS_PULL_UP; - break; - case RK_PINCTRL_PULL_DOWN: - bias = PIN_CONFIG_BIAS_PULL_DOWN; - break; - } - grp->configs[j] = pinconf_to_config_packed(bias, 0); + ret = pinconf_generic_parse_dt_bitmap(pinconf, + &grp->configs[j], &nconfigs); + if (ret) + return ret; + + if (nconfigs != 1) { + pr_err("unexpected number of config entries\n"); + return -EINVAL; + } } return 0;