From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755062AbbIHPHZ (ORCPT ); Tue, 8 Sep 2015 11:07:25 -0400 Received: from mail-pa0-f46.google.com ([209.85.220.46]:35711 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754671AbbIHPHS (ORCPT ); Tue, 8 Sep 2015 11:07:18 -0400 Subject: Re: [PATCH-v2 4/7] mmc: sdhci-pxav3: Add pinctl setting according to bus clock To: Linus Walleij References: <1441624721-15612-1-git-send-email-vaibhav.hiremath@linaro.org> <1441624721-15612-5-git-send-email-vaibhav.hiremath@linaro.org> Cc: "linux-mmc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "devicetree@vger.kernel.org" , Rob Herring , Ulf Hansson , Kevin Liu From: Vaibhav Hiremath Message-ID: <55EEF99F.1020904@linaro.org> Date: Tue, 8 Sep 2015 20:37:11 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 08 September 2015 08:12 PM, Linus Walleij wrote: > On Mon, Sep 7, 2015 at 1:18 PM, Vaibhav Hiremath > wrote: > >> Different bus clock may need different pin setting. >> For example, fast bus clock like 208Mhz need pin drive fast >> while slow bus clock prefer pin drive slow to guarantee >> signal quality. >> >> So this patch creates two states, >> - Default (slow/normal) pin state >> - And fast pin state for higher freq bus speed. >> >> And selection of pin state is done based on timing mode. >> >> Signed-off-by: Vaibhav Hiremath >> Signed-off-by: Kevin Liu > (...) >> + pxa->pinctrl = devm_pinctrl_get(dev); >> + if (!IS_ERR(pxa->pinctrl)) { >> + pxa->pins_default = pinctrl_lookup_state(pxa->pinctrl, "default"); >> + if (IS_ERR(pxa->pins_default)) >> + dev_err(dev, "could not get default pinstate\n"); >> + pxa->pins_fast = pinctrl_lookup_state(pxa->pinctrl, "fast"); >> + if (IS_ERR(pxa->pins_fast)) >> + dev_info(dev, "could not get fast pinstate\n"); >> + } > > This is exactly how I think it should be used from a pin control > point of view. > > If you depended on CONFIG_PM you could use > pinctrl_pm_select_default_state() but for this simple scenario > this is fine. > > Reviewed-by: Linus Walleij > From a pinctrl point of view. > Thanks for your review. Linus, I agree this is how it should be used. But I still have one small doubt on expectation from devm_pinctrl_get() function. If pinctrl properties are not populated in Devicetree node, then, shouldn't devm_pinctrl_get() return error ? I followed the code flow, and it seems even if pinctrl properties are not populated in DT node, the devm_pinctrl_get() returns valid pointer to "struct pinctrl", isn't this against the expectation of the call? Code flow - devm_pinctrl_get() ... --> creat_pinctrl() --> pinctrl_dt_to_map() ... pinctrl_dt_to_map() iterates for pinctrl-x (x = 0,1,...) and if it founds the entry then it parses the node. If it doesn't find any pinctrl property then also it returns 0. and subsequently rreturns handle to "struct pinctrl" for the device. Why is so? Thanks, Vaibhav