Re: [PATCH] ARM: dts: Add missing GPIO entries for sd_bus in

[Sachin Kamat <sachin.kamat@linaro.org>]

Hi Leela,

Thanks for the detailed explanation.

On 12 November 2013 17:20, Leela Krishna Amudala
<leelakrishna.a@gmail.com> wrote:
> Hi Sachin,
>
> On Tue, Nov 12, 2013 at 3:53 PM, Kukjin Kim <kgene@kernel.org> wrote:
>> Sachin Kamat wrote:
>>>
>>
>> Following is more clear?
>>
>> "ARM: dts: Add missing GPIO entries for sd_bus_width4 in exynos5420-pinctrl"
>>
>>> Adds missing GPIO entries for sd_bus nodes in exynos5420-pinctrl.
>>>
>
> This is not a missing stuff, I did this purposefully.
> Myself and Doug anderson discussed on this and the objective of doing this is to
> eliminate the gpio pin configuration multiple times.
>
> Please see the below explanation
>
> The current code in main line kernel shows like
>                sd0_bus1: sd0-bus-width1 {
>                        samsung,pins = "gpc0-3";
>                  };
>
>                sd0_bus4: sd0-bus-width4 {
>                        samsung,pins = "gpc0-4", "gpc0-5", "gpc0-6";
>                };
>
>                sd0_bus8: sd0-bus-width8 {
>                        samsung,pins = "gpc3-0", "gpc3-1", "gpc3-2", "gpc3-3";
>                };
>
> and lets mmc wants to use 8 bit width then node should be like below
>
> dwmmc0@12200000 {
>                 [snip]
>                 pinctrl-names = "default";
>                 pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus1 &sd0_bus4 &sd0_bus8>;
>
> Here it will configure all the 8 pins ("gpc0-3", "gpc0-4", "gpc0-5",
> "gpc0-6", "gpc3-0", "gpc3-1", "gpc3-2", "gpc3-3")
>
>                 slot@0 {
>                         reg = <0>;
>                         bus-width = <8>;
>                 };
>         };
>
> Similarly for using 1 bit width the property value should be like below
>                pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus1>;
>
> and if other mmc wants to use 4 bit width the property value should be
> like below
>                pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus1 &sd0_bus4>;
>
> But with the changes you made and if one mmc wants to use 1 bit width
> and other wants to use 4 bit width
> the pin "gpc0-3" will be configured twice which is wrong.

I think this is the case with current code too.

>From your above example, for 2 slots:
For 1 bit width: pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus1>;
"gpc0-3"

For 4 bit width: pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus1 &sd0_bus4>;
"gpc0-3", "gpc0-4", "gpc0-5", "gpc0-6"


> So, I strongly feel it is better to change the existing mmc node
> instead of changing the "sd0_bus" entries

I think this is already taken care of in the mmc nodes.
For slot with bus-width of 8,
pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus4 &sd0_bus8>;

Similarly for slot with bus width of 4,
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;

Do you still see the problem here?

-- 
With warm regards,
Sachin