Re: [PATCH] pinctrl: msm: Add support for MSM TLMM pinmux

[hanumant <hanumant@codeaurora.org>]

On 06/24/2013 05:18 AM, Linus Walleij wrote:

>> +
>> +  - qcom,gp-pull: Pull up/down configuration.
>> +  - qcom,gp-drv: Drive strength configuration.
>> +  - qcom,gp-dir: Pull up/down configuration in power down mode.
> 
> Rebase this to use the generic pin config mappings and parsing
> code that can be found in the "devel" branch of the pinctrl tree.
> 
>> +  The following pin configurations are properties are supported by SDC pins
>> +  - qcom,sdc1-clk-pull: Pull up/down configuration SDC1 clock pin.
>> +  - qcom,sdc1-clk-drv: Drive strength configuration for SDC1 clock pin.
>> +  - qcom,sdc1-cmd-pull: Pull up/down configuration for SDC1 command pin.
>> +  - qcom,sdc1-cmd-drv: Drive strength configuration for SDC1 command pin.
>> +  - qcom,sdc1-data-pull: Pull up/down configuration for SDC1 data pin.
>> +  - qcom,sdc1-data-drv: Drive strength configuration for SDC1 data pin.
>> +  - qcom,sdc2-clk-pull: Pull up/down configuration SDC2 clock pin.
>> +  - qcom,sdc2-clk-drv: Drive strength configuration for SDC2 clock pin.
>> +  - qcom,sdc2-cmd-pull: Pull up/down configuration for SDC2 command pin.
>> +  - qcom,sdc2-cmd-drv: Drive strength configuration for SDC2 command pin.
>> +  - qcom,sdc2-data-pull: Pull up/down configuration for SDC2 data pin.
>> +  - qcom,sdc2-data-drv: Drive strength configuration for SDC2 data pin.
> 
> I don't understand why each sdc thing needs its own definition
> for everything. Please use the generic pin config bindings, call the
> generic parser function and then reject if someone tries to config
> something that is not supported.
>

The register semantics of SDC1 clk, command, and data lines, pull up and
drive strength attributes differ from SDC2 clk, command and data lines.

In general the TLMM v3 has more pin types then just the general/multi
purpose(gp) and SDC pin types above.
There are some pin types on the TLMM, whose config attributes do not
fall under the cattegories supported by generic pin config.
These attributes in some cases happen to be protocol specific.
Hence I would prefer to go with a custom config pack and unpack
implementation rather then the generic one.

> 
>> +Example 4: Set up the default pin state for spi controller.
>> +
>> +       static inline int msm_spi_request_pins{struct msm_spi *dd)
>> +       {
>> +               /* ... */
>> +               dd->pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
>> +       }
> 
> Nowadays the device core will do this, so this is not any good
> advice. (See commit ab78029ecc347debbd737f06688d788bd9d60c1d)

I will modify this.

> 
> I will look closer at this when it uses generic pinconf and
> generic pinconf DT bindings, let's target v3.12.
> 
> Yours,
> Linus Walleij
> 


-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, hosted by The Linux Foundation
--