diff mbox series

[2/2] arm64: dts: renesas: salvator: Switch eMMC bus to 1V8

Message ID 20181027163410.7417-2-marek.vasut+renesas@gmail.com (mailing list archive)
State Superseded
Delegated to: Simon Horman
Headers show
Series [1/2] arm64: dts: renesas: ulcb: Switch eMMC bus to 1V8 | expand

Commit Message

Marek Vasut Oct. 27, 2018, 4:34 p.m. UTC
The eMMC card has two supplies, VCC and VCCQ. The VCC supplies the NAND
array and the VCCQ supplies the bus. On this particular board, the VCC is
connected to 3.3V rail, while the VCCQ is connected to 1.8V rail. Adjust
the pinmux to match the bus, which is always operating in 1.8V mode.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Simon Horman <horms+renesas@verge.net.au>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: linux-renesas-soc@vger.kernel.org
---
 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Wolfram Sang Oct. 28, 2018, 9:34 p.m. UTC | #1
Hi Marek,

On Sat, Oct 27, 2018 at 06:34:10PM +0200, Marek Vasut wrote:
> The eMMC card has two supplies, VCC and VCCQ. The VCC supplies the NAND
> array and the VCCQ supplies the bus. On this particular board, the VCC is
> connected to 3.3V rail, while the VCCQ is connected to 1.8V rail. Adjust
> the pinmux to match the bus, which is always operating in 1.8V mode.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Thanks for this!

I think Olof (and thus, Simon ;)) will be happy if those two patches are
merged.

Other than that, I think we should remove sdhi2_pins_uhs then because it
is the same as sdhi2_pins. And then use later "pinctrl-1 =
<&sdhi2_pins>;". So, basically the same phandles for both pinctrls. We
can re-add the second one when we need it.

> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Simon Horman <horms+renesas@verge.net.au>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Cc: linux-renesas-soc@vger.kernel.org
> ---
>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> index 7d3d866a0063..d9a309b28fcf 100644
> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> @@ -602,7 +602,7 @@
>  	sdhi2_pins: sd2 {
>  		groups = "sdhi2_data8", "sdhi2_ctrl", "sdhi2_ds";
>  		function = "sdhi2";
> -		power-source = <3300>;
> +		power-source = <1800>;
>  	};
>  
>  	sdhi2_pins_uhs: sd2_uhs {
> -- 
> 2.17.1
>
Marek Vasut Oct. 29, 2018, 8:01 a.m. UTC | #2
On 10/28/2018 10:34 PM, Wolfram Sang wrote:
> Hi Marek,

Hi,

> On Sat, Oct 27, 2018 at 06:34:10PM +0200, Marek Vasut wrote:
>> The eMMC card has two supplies, VCC and VCCQ. The VCC supplies the NAND
>> array and the VCCQ supplies the bus. On this particular board, the VCC is
>> connected to 3.3V rail, while the VCCQ is connected to 1.8V rail. Adjust
>> the pinmux to match the bus, which is always operating in 1.8V mode.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> 
> Thanks for this!
> 
> I think Olof (and thus, Simon ;)) will be happy if those two patches are
> merged.

Fine by me.

> Other than that, I think we should remove sdhi2_pins_uhs then because it
> is the same as sdhi2_pins. And then use later "pinctrl-1 =
> <&sdhi2_pins>;". So, basically the same phandles for both pinctrls. We
> can re-add the second one when we need it.

I wonder if removing the sdhi2_pins_uhs is what we want to do, given
that we might need to adjust TDSEL or pull resistor configurations for
the HS200/HS400 modes in the future.

Thoughts ?

>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>> Cc: Simon Horman <horms+renesas@verge.net.au>
>> Cc: Wolfram Sang <wsa@the-dreams.de>
>> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>> Cc: linux-renesas-soc@vger.kernel.org
>> ---
>>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>> index 7d3d866a0063..d9a309b28fcf 100644
>> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
>> @@ -602,7 +602,7 @@
>>  	sdhi2_pins: sd2 {
>>  		groups = "sdhi2_data8", "sdhi2_ctrl", "sdhi2_ds";
>>  		function = "sdhi2";
>> -		power-source = <3300>;
>> +		power-source = <1800>;
>>  	};
>>  
>>  	sdhi2_pins_uhs: sd2_uhs {
>> -- 
>> 2.17.1
>>
Wolfram Sang Oct. 29, 2018, 8:57 a.m. UTC | #3
> > <&sdhi2_pins>;". So, basically the same phandles for both pinctrls. We
> > can re-add the second one when we need it.
> 
> I wonder if removing the sdhi2_pins_uhs is what we want to do, given
> that we might need to adjust TDSEL or pull resistor configurations for
> the HS200/HS400 modes in the future.

Well, quoting myself "We can re-add the second one when we need it". It
is possible but a tad unlikely. That's my take on it but it is
ultimately up to Simon, of course.
Simon Horman Oct. 31, 2018, 12:46 p.m. UTC | #4
On Mon, Oct 29, 2018 at 08:57:21AM +0000, Wolfram Sang wrote:
> 
> > > <&sdhi2_pins>;". So, basically the same phandles for both pinctrls. We
> > > can re-add the second one when we need it.
> > 
> > I wonder if removing the sdhi2_pins_uhs is what we want to do, given
> > that we might need to adjust TDSEL or pull resistor configurations for
> > the HS200/HS400 modes in the future.
> 
> Well, quoting myself "We can re-add the second one when we need it". It
> is possible but a tad unlikely. That's my take on it but it is
> ultimately up to Simon, of course.

I agree we can add stuff later if we need it.
But can we discuss this in the context of describing the hardware?
Marek Vasut Oct. 31, 2018, 1:03 p.m. UTC | #5
On 10/31/2018 01:46 PM, Simon Horman wrote:
> On Mon, Oct 29, 2018 at 08:57:21AM +0000, Wolfram Sang wrote:
>>
>>>> <&sdhi2_pins>;". So, basically the same phandles for both pinctrls. We
>>>> can re-add the second one when we need it.
>>>
>>> I wonder if removing the sdhi2_pins_uhs is what we want to do, given
>>> that we might need to adjust TDSEL or pull resistor configurations for
>>> the HS200/HS400 modes in the future.
>>
>> Well, quoting myself "We can re-add the second one when we need it". It
>> is possible but a tad unlikely. That's my take on it but it is
>> ultimately up to Simon, of course.
> 
> I agree we can add stuff later if we need it.
> But can we discuss this in the context of describing the hardware?

I'm not sure I understand this remark. Anyway, I'll be spinning a V2 of
the patchset, which will be a single patch.
Wolfram Sang Oct. 31, 2018, 1:14 p.m. UTC | #6
> But can we discuss this in the context of describing the hardware?

The SDHI node needs two kinds of pinmux settings, one for normal speeds
and one for highspeeds. They might differ in supplied voltage, i.e. 3v3
and 1v8. This eMMC always works with 1v8, so both settings needed by the
SDHI node are the same currently. That might change if we add new
features to pinmux nodes like TDSEL/capacity support. My guess is that
this is not so likely to happen so I suggested using just one node.

But it has a taste of bike-shedding to it :)
Marek Vasut Nov. 4, 2018, 8:38 p.m. UTC | #7
On 10/31/2018 02:14 PM, Wolfram Sang wrote:
> 
>> But can we discuss this in the context of describing the hardware?
> 
> The SDHI node needs two kinds of pinmux settings, one for normal speeds
> and one for highspeeds. They might differ in supplied voltage, i.e. 3v3
> and 1v8. This eMMC always works with 1v8, so both settings needed by the
> SDHI node are the same currently. That might change if we add new
> features to pinmux nodes like TDSEL/capacity support. My guess is that
> this is not so likely to happen so I suggested using just one node.
> 
> But it has a taste of bike-shedding to it :)

V2 is out, including the deduplication.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index 7d3d866a0063..d9a309b28fcf 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -602,7 +602,7 @@ 
 	sdhi2_pins: sd2 {
 		groups = "sdhi2_data8", "sdhi2_ctrl", "sdhi2_ds";
 		function = "sdhi2";
-		power-source = <3300>;
+		power-source = <1800>;
 	};
 
 	sdhi2_pins_uhs: sd2_uhs {