Re: Dropping device tree pinmux nodes for GPIO usage (Was: [PATCH 3/5] arm64: dts: sun50i: add MMC nodes)

2017-01-17 Thread Maxime Ripard
On Sun, Jan 15, 2017 at 04:59:32PM +0100, Rask Ingemann Lambertsen wrote:
> On Tue, Jan 03, 2017 at 10:52:12AM +0800, Chen-Yu Tsai wrote:
> > On Tue, Jan 3, 2017 at 7:03 AM, Andre Przywara  
> > wrote:
> > > +
> > > +   mmc0_default_cd_pin: mmc0_cd_pin@0 {
> > > +   pins = "PF6";
> > > +   function = "gpio_in";
> > > +   bias-pull-up;
> > > +   };
> > 
> > We are starting to drop pinmux nodes for gpio usage.
> 
> How do we get the equivalent of bias-pull-up/down and drive-strength if we
> run across a pin that needs it?

Hmm, that's actually a very good question...

For those cases, leave the pinctrl node for now, we'll deal with that
in due time

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature


Dropping device tree pinmux nodes for GPIO usage (Was: [PATCH 3/5] arm64: dts: sun50i: add MMC nodes)

2017-01-15 Thread Rask Ingemann Lambertsen
On Tue, Jan 03, 2017 at 10:52:12AM +0800, Chen-Yu Tsai wrote:
> On Tue, Jan 3, 2017 at 7:03 AM, Andre Przywara  wrote:
> > +
> > +   mmc0_default_cd_pin: mmc0_cd_pin@0 {
> > +   pins = "PF6";
> > +   function = "gpio_in";
> > +   bias-pull-up;
> > +   };
> 
> We are starting to drop pinmux nodes for gpio usage.

How do we get the equivalent of bias-pull-up/down and drive-strength if we
run across a pin that needs it?

-- 
Rask Ingemann Lambertsen


Re: [PATCH 3/5] arm64: dts: sun50i: add MMC nodes

2017-01-05 Thread Maxime Ripard
On Mon, Jan 02, 2017 at 11:03:44PM +, Andre Przywara wrote:
> Signed-off-by: Andre Przywara 
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 67 
> +++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi 
> b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index e0dcab8..c680566 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -150,6 +150,32 @@
>   pins = "PB8", "PB9";
>   function = "uart0";
>   };
> +
> + mmc0_pins: mmc0@0 {
> + pins = "PF0", "PF1", "PF2", "PF3", "PF4", "PF5";
> + function = "mmc0";
> + drive-strength = <30>;
> + };
> +
> + mmc0_default_cd_pin: mmc0_cd_pin@0 {
> + pins = "PF6";
> + function = "gpio_in";
> + bias-pull-up;
> + };
> +
> + mmc1_pins: mmc1@0 {
> + pins = "PG0", "PG1", "PG2", "PG3", "PG4", "PG5";
> + function = "mmc1";
> + drive-strength = <30>;
> + };
> +
> + mmc2_pins: mmc2@0 {
> + pins = "PC1", "PC5", "PC6", "PC8", "PC9",
> +"PC10", "PC11", "PC12", "PC13", "PC14",
> +"PC15", "PC16";
> + function = "mmc2";
> + drive-strength = <30>;
> + };
>   };
>  
>   uart0: serial@1c28000 {
> @@ -240,6 +266,47 @@
>   #size-cells = <0>;
>   };
>  
> + mmc0: mmc@1c0f000 {
> + compatible = "allwinner,sun50i-a64-mmc",
> +  "allwinner,sun5i-a13-mmc";

That's not correct. There's a bunch of features that are broken
without A64-specific (or at least not present on the A13)
quirks. Those features include for example the SDIO aggregation that
are currently broken for all modes supported so far.

I was in holidays and send those patches tomorrow, but you were a bit
too quick :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature


Re: [PATCH 3/5] arm64: dts: sun50i: add MMC nodes

2017-01-03 Thread Chen-Yu Tsai
On Wed, Jan 4, 2017 at 8:22 AM, André Przywara  wrote:
> On 03/01/17 13:28, Chen-Yu Tsai wrote:
>> On Tue, Jan 3, 2017 at 6:48 PM, André Przywara  
>> wrote:
>>> On 03/01/17 02:52, Chen-Yu Tsai wrote:
>
> Hi Chen-Yu,
>
 On Tue, Jan 3, 2017 at 7:03 AM, Andre Przywara  
 wrote:

 A commit message explaining the mmc controllers would be nice.
>>>
>>> OK.
>>>
> Signed-off-by: Andre Przywara 
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 67 
> +++
>  1 file changed, 67 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi 
> b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index e0dcab8..c680566 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -150,6 +150,32 @@
> pins = "PB8", "PB9";
> function = "uart0";
> };
> +
> +   mmc0_pins: mmc0@0 {
> +   pins = "PF0", "PF1", "PF2", "PF3", "PF4", 
> "PF5";
> +   function = "mmc0";
> +   drive-strength = <30>;
> +   };
> +
> +   mmc0_default_cd_pin: mmc0_cd_pin@0 {
> +   pins = "PF6";
> +   function = "gpio_in";
> +   bias-pull-up;
> +   };

 We are starting to drop pinmux nodes for gpio usage.
>>>
>>> And replacing them with what?
>>> Or do you mean they go in the individual board .dts files?
>>> In this case I believe having a default pin defined here would help to
>>> define it in every .dts.
>>
>> Nope. I meant dropping them. Pinmux and gpio are orthogonal. One should not
>> need to specify a gpio pinmux to use it as a gpio. We added them because in
>> the past nothing was preventing someone from claiming an already muxed pin
>> as a gpio. On some platforms this is fine. For sunxi, this breaks the system,
>> as the gpio functions are muxed in.
>>
>> The idea moving forward is that these cases should be guarded in the driver.
>
> Ah, OK, you mean just referencing the pin like:
> cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
> in the board DT is enough? And the driver claiming it will (eventually?)
> prevent users from using them via the sysfs interface then?

cd-gpios = 

should be enough to block other devices from claiming a pinmux (pinctrl-N = ...)
on the same pin, or vice versa, whichever claimed it first in kernel space.

Same goes for a pin already claimed by a device through a pinmux. It should
not be claimable as a gpio through the sysfs interface.

>> Of course we would have to deal with existing dtbs, but lets not add any 
>> more.
>>
> +
> +   mmc1_pins: mmc1@0 {
> +   pins = "PG0", "PG1", "PG2", "PG3", "PG4", 
> "PG5";
> +   function = "mmc1";
> +   drive-strength = <30>;
> +   };
> +
> +   mmc2_pins: mmc2@0 {
> +   pins = "PC1", "PC5", "PC6", "PC8", "PC9",
> +  "PC10", "PC11", "PC12", "PC13", 
> "PC14",
> +  "PC15", "PC16";
> +   function = "mmc2";
> +   drive-strength = <30>;
> +   };

 Moreover I think you should split out the pinmux nodes to a separate patch.
>>>
>>> I can surely do, just wondering what's the rationale is behind that?
>>
>> More or less the "do one thing in one patch" rationale. Of course you can
>> claim these are the defaults used in the reference design and pretty much
>> every board out there. Then it makes sense to do them together. :)
>
> Mmmh, probably a misunderstanding, but:
> Those pins are the possible pins that expose the MMC interface. Those
> nodes here name them to allow easy and concise reference by a board DT
> (as in the following patch).
> And in fact in case of the A64 there are _no_ alternative muxes for the
> three MMC controllers: SDC0 is only on PF0-5, SDC1 on PG0-PG5 and SDC2
> on PC1-PC16.
> So it's not a board design question, apart from using a controller or not.
>
> That's why I rather keep them together with the MMC controller nodes.

Cool. Please mention this in the commit log, as in they are the only
possible choice.S ince they are the only choices, please drop the @0
in the node name.

And you might want to rename mmc2_pins to something like
"mmc2_8bit_pins: mmc2_8bit". Who knows, someone might consider using
it for an SD card or SDIO, and the remaining pins for something else.


> };
>

Re: [PATCH 3/5] arm64: dts: sun50i: add MMC nodes

2017-01-03 Thread André Przywara
On 03/01/17 13:28, Chen-Yu Tsai wrote:
> On Tue, Jan 3, 2017 at 6:48 PM, André Przywara  wrote:
>> On 03/01/17 02:52, Chen-Yu Tsai wrote:

Hi Chen-Yu,

>>> On Tue, Jan 3, 2017 at 7:03 AM, Andre Przywara  
>>> wrote:
>>>
>>> A commit message explaining the mmc controllers would be nice.
>>
>> OK.
>>
 Signed-off-by: Andre Przywara 
 ---
  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 67 
 +++
  1 file changed, 67 insertions(+)

 diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi 
 b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
 index e0dcab8..c680566 100644
 --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
 +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
 @@ -150,6 +150,32 @@
 pins = "PB8", "PB9";
 function = "uart0";
 };
 +
 +   mmc0_pins: mmc0@0 {
 +   pins = "PF0", "PF1", "PF2", "PF3", "PF4", 
 "PF5";
 +   function = "mmc0";
 +   drive-strength = <30>;
 +   };
 +
 +   mmc0_default_cd_pin: mmc0_cd_pin@0 {
 +   pins = "PF6";
 +   function = "gpio_in";
 +   bias-pull-up;
 +   };
>>>
>>> We are starting to drop pinmux nodes for gpio usage.
>>
>> And replacing them with what?
>> Or do you mean they go in the individual board .dts files?
>> In this case I believe having a default pin defined here would help to
>> define it in every .dts.
> 
> Nope. I meant dropping them. Pinmux and gpio are orthogonal. One should not
> need to specify a gpio pinmux to use it as a gpio. We added them because in
> the past nothing was preventing someone from claiming an already muxed pin
> as a gpio. On some platforms this is fine. For sunxi, this breaks the system,
> as the gpio functions are muxed in.
> 
> The idea moving forward is that these cases should be guarded in the driver.

Ah, OK, you mean just referencing the pin like:
cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
in the board DT is enough? And the driver claiming it will (eventually?)
prevent users from using them via the sysfs interface then?

> Of course we would have to deal with existing dtbs, but lets not add any more.
> 
 +
 +   mmc1_pins: mmc1@0 {
 +   pins = "PG0", "PG1", "PG2", "PG3", "PG4", 
 "PG5";
 +   function = "mmc1";
 +   drive-strength = <30>;
 +   };
 +
 +   mmc2_pins: mmc2@0 {
 +   pins = "PC1", "PC5", "PC6", "PC8", "PC9",
 +  "PC10", "PC11", "PC12", "PC13", 
 "PC14",
 +  "PC15", "PC16";
 +   function = "mmc2";
 +   drive-strength = <30>;
 +   };
>>>
>>> Moreover I think you should split out the pinmux nodes to a separate patch.
>>
>> I can surely do, just wondering what's the rationale is behind that?
> 
> More or less the "do one thing in one patch" rationale. Of course you can
> claim these are the defaults used in the reference design and pretty much
> every board out there. Then it makes sense to do them together. :)

Mmmh, probably a misunderstanding, but:
Those pins are the possible pins that expose the MMC interface. Those
nodes here name them to allow easy and concise reference by a board DT
(as in the following patch).
And in fact in case of the A64 there are _no_ alternative muxes for the
three MMC controllers: SDC0 is only on PF0-5, SDC1 on PG0-PG5 and SDC2
on PC1-PC16.
So it's not a board design question, apart from using a controller or not.

That's why I rather keep them together with the MMC controller nodes.

>>>
 };

 uart0: serial@1c28000 {
 @@ -240,6 +266,47 @@
 #size-cells = <0>;
 };

 +   mmc0: mmc@1c0f000 {
 +   compatible = "allwinner,sun50i-a64-mmc",
 +"allwinner,sun5i-a13-mmc";
>>>
>>> Given that sun5i doesn't support mmc delay timings, and the A64 has
>>> calibration and delay timings, I wouldn't call them compatible.
>>>
>>> Or are you claiming that for the A64 has a delay of 0 for the
>>> currently supported speeds, so the calibration doesn't really
>>> matter? If so this should be mentioned in the commit message.
>>
>> Yes, that's my observation: Driving it with sun5-a13-mmc just works.
>> This sun5i driver version does not (and will never) support higher
>> transfer modes

Re: [PATCH 3/5] arm64: dts: sun50i: add MMC nodes

2017-01-03 Thread Chen-Yu Tsai
On Tue, Jan 3, 2017 at 6:48 PM, André Przywara  wrote:
> On 03/01/17 02:52, Chen-Yu Tsai wrote:
>
> Hi,
>
>> On Tue, Jan 3, 2017 at 7:03 AM, Andre Przywara  
>> wrote:
>>
>> A commit message explaining the mmc controllers would be nice.
>
> OK.
>
>>> Signed-off-by: Andre Przywara 
>>> ---
>>>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 67 
>>> +++
>>>  1 file changed, 67 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi 
>>> b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>>> index e0dcab8..c680566 100644
>>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>>> @@ -150,6 +150,32 @@
>>> pins = "PB8", "PB9";
>>> function = "uart0";
>>> };
>>> +
>>> +   mmc0_pins: mmc0@0 {
>>> +   pins = "PF0", "PF1", "PF2", "PF3", "PF4", 
>>> "PF5";
>>> +   function = "mmc0";
>>> +   drive-strength = <30>;
>>> +   };
>>> +
>>> +   mmc0_default_cd_pin: mmc0_cd_pin@0 {
>>> +   pins = "PF6";
>>> +   function = "gpio_in";
>>> +   bias-pull-up;
>>> +   };
>>
>> We are starting to drop pinmux nodes for gpio usage.
>
> And replacing them with what?
> Or do you mean they go in the individual board .dts files?
> In this case I believe having a default pin defined here would help to
> define it in every .dts.

Nope. I meant dropping them. Pinmux and gpio are orthogonal. One should not
need to specify a gpio pinmux to use it as a gpio. We added them because in
the past nothing was preventing someone from claiming an already muxed pin
as a gpio. On some platforms this is fine. For sunxi, this breaks the system,
as the gpio functions are muxed in.

The idea moving forward is that these cases should be guarded in the driver.
Of course we would have to deal with existing dtbs, but lets not add any more.

>>> +
>>> +   mmc1_pins: mmc1@0 {
>>> +   pins = "PG0", "PG1", "PG2", "PG3", "PG4", 
>>> "PG5";
>>> +   function = "mmc1";
>>> +   drive-strength = <30>;
>>> +   };
>>> +
>>> +   mmc2_pins: mmc2@0 {
>>> +   pins = "PC1", "PC5", "PC6", "PC8", "PC9",
>>> +  "PC10", "PC11", "PC12", "PC13", 
>>> "PC14",
>>> +  "PC15", "PC16";
>>> +   function = "mmc2";
>>> +   drive-strength = <30>;
>>> +   };
>>
>> Moreover I think you should split out the pinmux nodes to a separate patch.
>
> I can surely do, just wondering what's the rationale is behind that?

More or less the "do one thing in one patch" rationale. Of course you can
claim these are the defaults used in the reference design and pretty much
every board out there. Then it makes sense to do them together. :)

>
>>
>>> };
>>>
>>> uart0: serial@1c28000 {
>>> @@ -240,6 +266,47 @@
>>> #size-cells = <0>;
>>> };
>>>
>>> +   mmc0: mmc@1c0f000 {
>>> +   compatible = "allwinner,sun50i-a64-mmc",
>>> +"allwinner,sun5i-a13-mmc";
>>
>> Given that sun5i doesn't support mmc delay timings, and the A64 has
>> calibration and delay timings, I wouldn't call them compatible.
>>
>> Or are you claiming that for the A64 has a delay of 0 for the
>> currently supported speeds, so the calibration doesn't really
>> matter? If so this should be mentioned in the commit message.
>
> Yes, that's my observation: Driving it with sun5-a13-mmc just works.
> This sun5i driver version does not (and will never) support higher
> transfer modes anyway, so for that subset they are compatible. This
> opens up the door to other operating systems not having a particular
> driver for the A64, for instance, also older Linux kernels.
> I know that sunxi doesn't use this compatible feature much, but IMHO we
> should really start thinking about the DT not just being Linux specific
> - or even being specific to a certain Linux version. And this case here
> is a good example: An A13 MMC driver can drive this device - if there is
> no better driver (an A64 one) available.

Cool. Please put this in the commit log. :)

>
>>
>>> +   reg = <0x01c0f000 0x1000>;
>>> +   clocks = <&ccu 31>, <&ccu 75>;
>>
>> The clock / reset index macros are in the tree now.
>> Please switch to them.
>
> The include file is in the tree, but it isn't used in the current HEAD
> (as in #included and the actual macros being used in the 

Re: [PATCH 3/5] arm64: dts: sun50i: add MMC nodes

2017-01-03 Thread André Przywara
On 03/01/17 02:52, Chen-Yu Tsai wrote:

Hi,

> On Tue, Jan 3, 2017 at 7:03 AM, Andre Przywara  wrote:
> 
> A commit message explaining the mmc controllers would be nice.

OK.

>> Signed-off-by: Andre Przywara 
>> ---
>>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 67 
>> +++
>>  1 file changed, 67 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi 
>> b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> index e0dcab8..c680566 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
>> @@ -150,6 +150,32 @@
>> pins = "PB8", "PB9";
>> function = "uart0";
>> };
>> +
>> +   mmc0_pins: mmc0@0 {
>> +   pins = "PF0", "PF1", "PF2", "PF3", "PF4", 
>> "PF5";
>> +   function = "mmc0";
>> +   drive-strength = <30>;
>> +   };
>> +
>> +   mmc0_default_cd_pin: mmc0_cd_pin@0 {
>> +   pins = "PF6";
>> +   function = "gpio_in";
>> +   bias-pull-up;
>> +   };
> 
> We are starting to drop pinmux nodes for gpio usage.

And replacing them with what?
Or do you mean they go in the individual board .dts files?
In this case I believe having a default pin defined here would help to
define it in every .dts.

>> +
>> +   mmc1_pins: mmc1@0 {
>> +   pins = "PG0", "PG1", "PG2", "PG3", "PG4", 
>> "PG5";
>> +   function = "mmc1";
>> +   drive-strength = <30>;
>> +   };
>> +
>> +   mmc2_pins: mmc2@0 {
>> +   pins = "PC1", "PC5", "PC6", "PC8", "PC9",
>> +  "PC10", "PC11", "PC12", "PC13", 
>> "PC14",
>> +  "PC15", "PC16";
>> +   function = "mmc2";
>> +   drive-strength = <30>;
>> +   };
> 
> Moreover I think you should split out the pinmux nodes to a separate patch.

I can surely do, just wondering what's the rationale is behind that?

> 
>> };
>>
>> uart0: serial@1c28000 {
>> @@ -240,6 +266,47 @@
>> #size-cells = <0>;
>> };
>>
>> +   mmc0: mmc@1c0f000 {
>> +   compatible = "allwinner,sun50i-a64-mmc",
>> +"allwinner,sun5i-a13-mmc";
> 
> Given that sun5i doesn't support mmc delay timings, and the A64 has
> calibration and delay timings, I wouldn't call them compatible.
> 
> Or are you claiming that for the A64 has a delay of 0 for the
> currently supported speeds, so the calibration doesn't really
> matter? If so this should be mentioned in the commit message.

Yes, that's my observation: Driving it with sun5-a13-mmc just works.
This sun5i driver version does not (and will never) support higher
transfer modes anyway, so for that subset they are compatible. This
opens up the door to other operating systems not having a particular
driver for the A64, for instance, also older Linux kernels.
I know that sunxi doesn't use this compatible feature much, but IMHO we
should really start thinking about the DT not just being Linux specific
- or even being specific to a certain Linux version. And this case here
is a good example: An A13 MMC driver can drive this device - if there is
no better driver (an A64 one) available.

> 
>> +   reg = <0x01c0f000 0x1000>;
>> +   clocks = <&ccu 31>, <&ccu 75>;
> 
> The clock / reset index macros are in the tree now.
> Please switch to them.

The include file is in the tree, but it isn't used in the current HEAD
(as in #included and the actual macros being used in the .dtsi).
So I was wondering if there is a patch pending for that?

Cheers,
Andre

>> +   clock-names = "ahb", "mmc";
>> +   resets = <&ccu 8>;
>> +   reset-names = "ahb";
>> +   interrupts = ;
>> +   status = "disabled";
>> +   #address-cells = <1>;
>> +   #size-cells = <0>;
>> +   };
>> +
>> +   mmc1: mmc@1c1 {
>> +   compatible = "allwinner,sun50i-a64-mmc",
>> +"allwinner,sun5i-a13-mmc";
>> +   reg = <0x01c1 0x1000>;
>> +   clocks = <&ccu 32>, <&ccu 76>;
>> +   clock-names = "ahb", "mmc";
>> +   resets = <&ccu 9>;
>> +   reset-names = "ahb";
>> +   interrupts = ;
>> +   

Re: [PATCH 3/5] arm64: dts: sun50i: add MMC nodes

2017-01-02 Thread Chen-Yu Tsai
Hi,

On Tue, Jan 3, 2017 at 7:03 AM, Andre Przywara  wrote:

A commit message explaining the mmc controllers would be nice.

> Signed-off-by: Andre Przywara 
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 67 
> +++
>  1 file changed, 67 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi 
> b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index e0dcab8..c680566 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -150,6 +150,32 @@
> pins = "PB8", "PB9";
> function = "uart0";
> };
> +
> +   mmc0_pins: mmc0@0 {
> +   pins = "PF0", "PF1", "PF2", "PF3", "PF4", 
> "PF5";
> +   function = "mmc0";
> +   drive-strength = <30>;
> +   };
> +
> +   mmc0_default_cd_pin: mmc0_cd_pin@0 {
> +   pins = "PF6";
> +   function = "gpio_in";
> +   bias-pull-up;
> +   };

We are starting to drop pinmux nodes for gpio usage.

> +
> +   mmc1_pins: mmc1@0 {
> +   pins = "PG0", "PG1", "PG2", "PG3", "PG4", 
> "PG5";
> +   function = "mmc1";
> +   drive-strength = <30>;
> +   };
> +
> +   mmc2_pins: mmc2@0 {
> +   pins = "PC1", "PC5", "PC6", "PC8", "PC9",
> +  "PC10", "PC11", "PC12", "PC13", "PC14",
> +  "PC15", "PC16";
> +   function = "mmc2";
> +   drive-strength = <30>;
> +   };

Moreover I think you should split out the pinmux nodes to a separate patch.

> };
>
> uart0: serial@1c28000 {
> @@ -240,6 +266,47 @@
> #size-cells = <0>;
> };
>
> +   mmc0: mmc@1c0f000 {
> +   compatible = "allwinner,sun50i-a64-mmc",
> +"allwinner,sun5i-a13-mmc";

Given that sun5i doesn't support mmc delay timings, and the A64 has
calibration and delay timings, I wouldn't call them compatible.

Or are you claiming that for the A64 has a delay of 0 for the
currently supported speeds, so the calibration doesn't really
matter? If so this should be mentioned in the commit message.

> +   reg = <0x01c0f000 0x1000>;
> +   clocks = <&ccu 31>, <&ccu 75>;

The clock / reset index macros are in the tree now.
Please switch to them.

ChenYu

> +   clock-names = "ahb", "mmc";
> +   resets = <&ccu 8>;
> +   reset-names = "ahb";
> +   interrupts = ;
> +   status = "disabled";
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   };
> +
> +   mmc1: mmc@1c1 {
> +   compatible = "allwinner,sun50i-a64-mmc",
> +"allwinner,sun5i-a13-mmc";
> +   reg = <0x01c1 0x1000>;
> +   clocks = <&ccu 32>, <&ccu 76>;
> +   clock-names = "ahb", "mmc";
> +   resets = <&ccu 9>;
> +   reset-names = "ahb";
> +   interrupts = ;
> +   status = "disabled";
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   };
> +
> +   mmc2: mmc@1c11000 {
> +   compatible = "allwinner,sun50i-a64-emmc";
> +   reg = <0x01c11000 0x1000>;
> +   clocks = <&ccu 33>, <&ccu 77>;
> +   clock-names = "ahb", "mmc";
> +   resets = <&ccu 10>;
> +   reset-names = "ahb";
> +   interrupts = ;
> +   status = "disabled";
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   };
> +
> gic: interrupt-controller@1c81000 {
> compatible = "arm,gic-400";
> reg = <0x01c81000 0x1000>,
> --
> 2.8.2
>


[PATCH 3/5] arm64: dts: sun50i: add MMC nodes

2017-01-02 Thread Andre Przywara
Signed-off-by: Andre Przywara 
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 67 +++
 1 file changed, 67 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi 
b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index e0dcab8..c680566 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -150,6 +150,32 @@
pins = "PB8", "PB9";
function = "uart0";
};
+
+   mmc0_pins: mmc0@0 {
+   pins = "PF0", "PF1", "PF2", "PF3", "PF4", "PF5";
+   function = "mmc0";
+   drive-strength = <30>;
+   };
+
+   mmc0_default_cd_pin: mmc0_cd_pin@0 {
+   pins = "PF6";
+   function = "gpio_in";
+   bias-pull-up;
+   };
+
+   mmc1_pins: mmc1@0 {
+   pins = "PG0", "PG1", "PG2", "PG3", "PG4", "PG5";
+   function = "mmc1";
+   drive-strength = <30>;
+   };
+
+   mmc2_pins: mmc2@0 {
+   pins = "PC1", "PC5", "PC6", "PC8", "PC9",
+  "PC10", "PC11", "PC12", "PC13", "PC14",
+  "PC15", "PC16";
+   function = "mmc2";
+   drive-strength = <30>;
+   };
};
 
uart0: serial@1c28000 {
@@ -240,6 +266,47 @@
#size-cells = <0>;
};
 
+   mmc0: mmc@1c0f000 {
+   compatible = "allwinner,sun50i-a64-mmc",
+"allwinner,sun5i-a13-mmc";
+   reg = <0x01c0f000 0x1000>;
+   clocks = <&ccu 31>, <&ccu 75>;
+   clock-names = "ahb", "mmc";
+   resets = <&ccu 8>;
+   reset-names = "ahb";
+   interrupts = ;
+   status = "disabled";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+
+   mmc1: mmc@1c1 {
+   compatible = "allwinner,sun50i-a64-mmc",
+"allwinner,sun5i-a13-mmc";
+   reg = <0x01c1 0x1000>;
+   clocks = <&ccu 32>, <&ccu 76>;
+   clock-names = "ahb", "mmc";
+   resets = <&ccu 9>;
+   reset-names = "ahb";
+   interrupts = ;
+   status = "disabled";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+
+   mmc2: mmc@1c11000 {
+   compatible = "allwinner,sun50i-a64-emmc";
+   reg = <0x01c11000 0x1000>;
+   clocks = <&ccu 33>, <&ccu 77>;
+   clock-names = "ahb", "mmc";
+   resets = <&ccu 10>;
+   reset-names = "ahb";
+   interrupts = ;
+   status = "disabled";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+
gic: interrupt-controller@1c81000 {
compatible = "arm,gic-400";
reg = <0x01c81000 0x1000>,
-- 
2.8.2