diff mbox series

[U-Boot,v2] rockchip: rk3288-evb: dts: remove 'vmmc' from emmc node

Message ID 20181205022500.5878-1-kever.yang@rock-chips.com
State Superseded
Delegated to: Philipp Tomsich
Headers show
Series [U-Boot,v2] rockchip: rk3288-evb: dts: remove 'vmmc' from emmc node | expand

Commit Message

Kever Yang Dec. 5, 2018, 2:25 a.m. UTC
The U-Boot eMMC does not need to care about the power for Rockchip
SoC, because if the board is using eMMC, the power will default on
(for bootrom), and we do not do power management for it like kernel,
so the 'vmmc', 'vqmmc' is only useful for SD in U-Boot.

This make U-Boot can boot into kernel even if the pmic driver is
broken.

The rk3288-evb dts may be used in many boards using rockchip reference
schematic but with little change, so we hope it can be more robust to
boot into next stage.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

Changes in v2:
- only update for rk3288-evb, remove change for other boards.

 arch/arm/dts/rk3288-evb.dtsi | 2 --
 1 file changed, 2 deletions(-)

Comments

Philipp Tomsich Dec. 6, 2018, 1:50 p.m. UTC | #1
+Tom

> On 05.12.2018, at 03:25, Kever Yang <kever.yang@rock-chips.com> wrote:
> 
> The U-Boot eMMC does not need to care about the power for Rockchip
> SoC, because if the board is using eMMC, the power will default on
> (for bootrom), and we do not do power management for it like kernel,
> so the 'vmmc', 'vqmmc' is only useful for SD in U-Boot.
> 
> This make U-Boot can boot into kernel even if the pmic driver is
> broken.

If the PMIC driver is broken, we should fix the PMIC driver.
I would feel more comfortable w/o this statement.

> The rk3288-evb dts may be used in many boards using rockchip reference
> schematic but with little change, so we hope it can be more robust to
> boot into next stage.

Again, this is not how the DTS should be used.  I believe that Heiko, Fabio and
I had already highlighted this in comments to the earlier thread.

I am willing to move ahead with these and simply merge them, if either Tom or
Simon agree that they are fine with a DTS that describes one board and is then
used for incompatible (if they were compatible, this change wasn’t needed)
boards...

> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

> ---
> 
> Changes in v2:
> - only update for rk3288-evb, remove change for other boards.
> 
> arch/arm/dts/rk3288-evb.dtsi | 2 --
> 1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm/dts/rk3288-evb.dtsi b/arch/arm/dts/rk3288-evb.dtsi
> index ce75bd5d28..04902c0bd3 100644
> --- a/arch/arm/dts/rk3288-evb.dtsi
> +++ b/arch/arm/dts/rk3288-evb.dtsi
> @@ -150,8 +150,6 @@
> 	num-slots = <1>;
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&emmc_clk>, <&emmc_cmd>, <&emmc_pwr>, <&emmc_bus8>;
> -	vmmc-supply = <&vcc_io>;
> -	vqmmc-supply = <&vcc_flash>;
> 	status = "okay";
> };
> 
> -- 
> 2.18.0
>
Kever Yang Dec. 7, 2018, 1:39 a.m. UTC | #2
Hi Philipp,

On 12/06/2018 09:50 PM, Philipp Tomsich wrote:
> +Tom
>
>> On 05.12.2018, at 03:25, Kever Yang <kever.yang@rock-chips.com> wrote:
>>
>> The U-Boot eMMC does not need to care about the power for Rockchip
>> SoC, because if the board is using eMMC, the power will default on
>> (for bootrom), and we do not do power management for it like kernel,
>> so the 'vmmc', 'vqmmc' is only useful for SD in U-Boot.
>>
>> This make U-Boot can boot into kernel even if the pmic driver is
>> broken.
> If the PMIC driver is broken, we should fix the PMIC driver.
> I would feel more comfortable w/o this statement.
>
>> The rk3288-evb dts may be used in many boards using rockchip reference
>> schematic but with little change, so we hope it can be more robust to
>> boot into next stage.
> Again, this is not how the DTS should be used.  I believe that Heiko, Fabio and
> I had already highlighted this in comments to the earlier thread.

    Not sure if you have read my previous mail for answer all your comments,

I do agree DTS should represent the hardware, but please note that the DTS

is no kind of standard, and people always choose what they need and add

those part in there dts, but not always add all the property and
everyone use

the same model. I would say there are many boards does not have this
'vmmc-supply'

in there emmc node.

There are many dts properties only available in U-Boot, and many dts
properties

only available in kernel, and I'm very sure the properity 'vmmc-supply'
in rockchip evbs

is not need, the emmc can works very good with default pmic config. A
remove of

this property does not trying to describe something not belong to this
board, but only

ignore something that not important or even redundant.

Only eMMC driver is the one must work in a bootloader, and it should
boot into next

stage even all other drivers can not work.

Rockchip can not upstream all the board dts from different customers,
and I believe

there are much much more boards with different vendor's SoCs, we should
care a

little more about the robust of the bootloader.

Rockchip private U-Boot is trying to use one dts per SoC instead of one
dts per board,

and if the basic driver(timer, mmc) works, we can boot into kernel in
emmc, the pmic/clock is to

make it faster, and all other feature, like led, display, key, usb
gadget, are plus to the

basic function, and we always make basic function works.


Thanks,

- Kever

>
> I am willing to move ahead with these and simply merge them, if either Tom or
> Simon agree that they are fine with a DTS that describes one board and is then
> used for incompatible (if they were compatible, this change wasn’t needed)
> boards...
>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>
>> ---
>>
>> Changes in v2:
>> - only update for rk3288-evb, remove change for other boards.
>>
>> arch/arm/dts/rk3288-evb.dtsi | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/arch/arm/dts/rk3288-evb.dtsi b/arch/arm/dts/rk3288-evb.dtsi
>> index ce75bd5d28..04902c0bd3 100644
>> --- a/arch/arm/dts/rk3288-evb.dtsi
>> +++ b/arch/arm/dts/rk3288-evb.dtsi
>> @@ -150,8 +150,6 @@
>> 	num-slots = <1>;
>> 	pinctrl-names = "default";
>> 	pinctrl-0 = <&emmc_clk>, <&emmc_cmd>, <&emmc_pwr>, <&emmc_bus8>;
>> -	vmmc-supply = <&vcc_io>;
>> -	vqmmc-supply = <&vcc_flash>;
>> 	status = "okay";
>> };
>>
>> -- 
>> 2.18.0
>>
>
Philipp Tomsich Dec. 7, 2018, 1:24 p.m. UTC | #3
Kever,

> On 07.12.2018, at 02:39, Kever Yang <kever.yang@rock-chips.com> wrote:
> 
> Hi Philipp,
> 
> On 12/06/2018 09:50 PM, Philipp Tomsich wrote:
>> +Tom
>> 
>>> On 05.12.2018, at 03:25, Kever Yang <kever.yang@rock-chips.com> wrote:
>>> 
>>> The U-Boot eMMC does not need to care about the power for Rockchip
>>> SoC, because if the board is using eMMC, the power will default on
>>> (for bootrom), and we do not do power management for it like kernel,
>>> so the 'vmmc', 'vqmmc' is only useful for SD in U-Boot.
>>> 
>>> This make U-Boot can boot into kernel even if the pmic driver is
>>> broken.
>> If the PMIC driver is broken, we should fix the PMIC driver.
>> I would feel more comfortable w/o this statement.
>> 
>>> The rk3288-evb dts may be used in many boards using rockchip reference
>>> schematic but with little change, so we hope it can be more robust to
>>> boot into next stage.
>> Again, this is not how the DTS should be used.  I believe that Heiko, Fabio and
>> I had already highlighted this in comments to the earlier thread.
> 
>     Not sure if you have read my previous mail for answer all your comments,
> 
> I do agree DTS should represent the hardware, but please note that the DTS
> is no kind of standard, and people always choose what they need and add
> those part in there dts, but not always add all the property and
> everyone use the same model. I would say there are many boards does not have this
> 'vmmc-supply’ in there emmc node.

That is exactly the reason why I bumped the decision up the stairs (to Tom and/or
Simon): what you are saying makes sense to me (viewed through your eyes and 
from your specific usecase), but it directly contradicts how the DTS usage is intended.

In other words: Tom (as the top-level decision maker) or Simon (who owns the 
device-model and therefore will also have an opinion on DTS usage) should make
the final call.

Thanks,
Philipp.
Tom Rini Dec. 7, 2018, 2:13 p.m. UTC | #4
On Fri, Dec 07, 2018 at 02:24:22PM +0100, Philipp Tomsich wrote:
> Kever,
> 
> > On 07.12.2018, at 02:39, Kever Yang <kever.yang@rock-chips.com> wrote:
> > 
> > Hi Philipp,
> > 
> > On 12/06/2018 09:50 PM, Philipp Tomsich wrote:
> >> +Tom
> >> 
> >>> On 05.12.2018, at 03:25, Kever Yang <kever.yang@rock-chips.com> wrote:
> >>> 
> >>> The U-Boot eMMC does not need to care about the power for Rockchip
> >>> SoC, because if the board is using eMMC, the power will default on
> >>> (for bootrom), and we do not do power management for it like kernel,
> >>> so the 'vmmc', 'vqmmc' is only useful for SD in U-Boot.
> >>> 
> >>> This make U-Boot can boot into kernel even if the pmic driver is
> >>> broken.
> >> If the PMIC driver is broken, we should fix the PMIC driver.
> >> I would feel more comfortable w/o this statement.
> >> 
> >>> The rk3288-evb dts may be used in many boards using rockchip reference
> >>> schematic but with little change, so we hope it can be more robust to
> >>> boot into next stage.
> >> Again, this is not how the DTS should be used.  I believe that Heiko, Fabio and
> >> I had already highlighted this in comments to the earlier thread.
> > 
> >     Not sure if you have read my previous mail for answer all your comments,
> > 
> > I do agree DTS should represent the hardware, but please note that the DTS
> > is no kind of standard, and people always choose what they need and add
> > those part in there dts, but not always add all the property and
> > everyone use the same model. I would say there are many boards does not have this
> > 'vmmc-supply’ in there emmc node.
> 
> That is exactly the reason why I bumped the decision up the stairs (to Tom and/or
> Simon): what you are saying makes sense to me (viewed through your eyes and 
> from your specific usecase), but it directly contradicts how the DTS usage is intended.
> 
> In other words: Tom (as the top-level decision maker) or Simon (who owns the 
> device-model and therefore will also have an opinion on DTS usage) should make
> the final call.

My answer is that I would strongly suspect that over in linux "we have N
different close-enough boards using this one DTS" isn't acceptable.  You
make a dtsi and include it from the board and things that aren't common
don't go into the dtsi.  And yes, when starting off everyone (myself
included) copies the reference platform dts and then changes it as
needed, and sometimes misses a thing or two.  But no, I don't think we
want a wrong dts and I'm pretty sure the kernel really wouldn't want
wrong dts files and the general goal is that excluding the -u-boot.dtsi
files, ours are copies of the kernel.
Kever Yang Dec. 8, 2018, 4:27 a.m. UTC | #5
Hi Tom,


On 12/07/2018 10:13 PM, Tom Rini wrote:
> On Fri, Dec 07, 2018 at 02:24:22PM +0100, Philipp Tomsich wrote:
>> Kever,
>>
>>> On 07.12.2018, at 02:39, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>
>>> Hi Philipp,
>>>
>>> On 12/06/2018 09:50 PM, Philipp Tomsich wrote:
>>>> +Tom
>>>>
>>>>> On 05.12.2018, at 03:25, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>
>>>>> The U-Boot eMMC does not need to care about the power for Rockchip
>>>>> SoC, because if the board is using eMMC, the power will default on
>>>>> (for bootrom), and we do not do power management for it like kernel,
>>>>> so the 'vmmc', 'vqmmc' is only useful for SD in U-Boot.
>>>>>
>>>>> This make U-Boot can boot into kernel even if the pmic driver is
>>>>> broken.
>>>> If the PMIC driver is broken, we should fix the PMIC driver.
>>>> I would feel more comfortable w/o this statement.
>>>>
>>>>> The rk3288-evb dts may be used in many boards using rockchip reference
>>>>> schematic but with little change, so we hope it can be more robust to
>>>>> boot into next stage.
>>>> Again, this is not how the DTS should be used.  I believe that Heiko, Fabio and
>>>> I had already highlighted this in comments to the earlier thread.
>>>     Not sure if you have read my previous mail for answer all your comments,
>>>
>>> I do agree DTS should represent the hardware, but please note that the DTS
>>> is no kind of standard, and people always choose what they need and add
>>> those part in there dts, but not always add all the property and
>>> everyone use the same model. I would say there are many boards does not have this
>>> 'vmmc-supply’ in there emmc node.
>> That is exactly the reason why I bumped the decision up the stairs (to Tom and/or
>> Simon): what you are saying makes sense to me (viewed through your eyes and 
>> from your specific usecase), but it directly contradicts how the DTS usage is intended.
>>
>> In other words: Tom (as the top-level decision maker) or Simon (who owns the 
>> device-model and therefore will also have an opinion on DTS usage) should make
>> the final call.
> My answer is that I would strongly suspect that over in linux "we have N
> different close-enough boards using this one DTS" isn't acceptable.  You
> make a dtsi and include it from the board and things that aren't common
> don't go into the dtsi.  And yes, when starting off everyone (myself
> included) copies the reference platform dts and then changes it as
> needed, and sometimes misses a thing or two.  But no, I don't think we
> want a wrong dts and I'm pretty sure the kernel really wouldn't want
> wrong dts files and the general goal is that excluding the -u-boot.dtsi
> files, ours are copies of the kernel.
I don't think this is a "wrong dts" after my patch, these two nodes are
not mark
as required property in kernel, so many dts emmc node does not have it.
I check the latest kernel dtsi in arch/arm/boot/dts/rk3288-evb.dtsi [1],
the emmc node do not have 'vmmc' and 'vqmmc' while the SD node have, which
just like description in my commit message.

Well, I don't know why U-Boot project is so difficult to accept a
reasonable patch now, I don't
want to make you unhappy, but make 'every board must have its own dts'
in U-Boot to make
every developer to join U-Boot does not make sense to me. The kernel
need different
dts for different board because they need to use/control those different
feature, but U-Boot
is not the case, U-Boot should work if the storage driver works.

Thanks,
- Kever

[1]
https://github.com/torvalds/linux/blob/master/arch/arm/boot/dts/rk3288-evb.dtsi#L208
>
Tom Rini Dec. 10, 2018, 6:20 p.m. UTC | #6
On Sat, Dec 08, 2018 at 12:27:42PM +0800, Kever Yang wrote:
> Hi Tom,
> 
> 
> On 12/07/2018 10:13 PM, Tom Rini wrote:
> > On Fri, Dec 07, 2018 at 02:24:22PM +0100, Philipp Tomsich wrote:
> >> Kever,
> >>
> >>> On 07.12.2018, at 02:39, Kever Yang <kever.yang@rock-chips.com> wrote:
> >>>
> >>> Hi Philipp,
> >>>
> >>> On 12/06/2018 09:50 PM, Philipp Tomsich wrote:
> >>>> +Tom
> >>>>
> >>>>> On 05.12.2018, at 03:25, Kever Yang <kever.yang@rock-chips.com> wrote:
> >>>>>
> >>>>> The U-Boot eMMC does not need to care about the power for Rockchip
> >>>>> SoC, because if the board is using eMMC, the power will default on
> >>>>> (for bootrom), and we do not do power management for it like kernel,
> >>>>> so the 'vmmc', 'vqmmc' is only useful for SD in U-Boot.
> >>>>>
> >>>>> This make U-Boot can boot into kernel even if the pmic driver is
> >>>>> broken.
> >>>> If the PMIC driver is broken, we should fix the PMIC driver.
> >>>> I would feel more comfortable w/o this statement.
> >>>>
> >>>>> The rk3288-evb dts may be used in many boards using rockchip reference
> >>>>> schematic but with little change, so we hope it can be more robust to
> >>>>> boot into next stage.
> >>>> Again, this is not how the DTS should be used.  I believe that Heiko, Fabio and
> >>>> I had already highlighted this in comments to the earlier thread.
> >>>     Not sure if you have read my previous mail for answer all your comments,
> >>>
> >>> I do agree DTS should represent the hardware, but please note that the DTS
> >>> is no kind of standard, and people always choose what they need and add
> >>> those part in there dts, but not always add all the property and
> >>> everyone use the same model. I would say there are many boards does not have this
> >>> 'vmmc-supply’ in there emmc node.
> >> That is exactly the reason why I bumped the decision up the stairs (to Tom and/or
> >> Simon): what you are saying makes sense to me (viewed through your eyes and 
> >> from your specific usecase), but it directly contradicts how the DTS usage is intended.
> >>
> >> In other words: Tom (as the top-level decision maker) or Simon (who owns the 
> >> device-model and therefore will also have an opinion on DTS usage) should make
> >> the final call.
> > My answer is that I would strongly suspect that over in linux "we have N
> > different close-enough boards using this one DTS" isn't acceptable.  You
> > make a dtsi and include it from the board and things that aren't common
> > don't go into the dtsi.  And yes, when starting off everyone (myself
> > included) copies the reference platform dts and then changes it as
> > needed, and sometimes misses a thing or two.  But no, I don't think we
> > want a wrong dts and I'm pretty sure the kernel really wouldn't want
> > wrong dts files and the general goal is that excluding the -u-boot.dtsi
> > files, ours are copies of the kernel.
> I don't think this is a "wrong dts" after my patch, these two nodes are
> not mark
> as required property in kernel, so many dts emmc node does not have it.
> I check the latest kernel dtsi in arch/arm/boot/dts/rk3288-evb.dtsi [1],
> the emmc node do not have 'vmmc' and 'vqmmc' while the SD node have, which
> just like description in my commit message.

OK.  So this would fall into the category of "sync with upstream dts"
then, right?  That is what we want.

> Well, I don't know why U-Boot project is so difficult to accept a
> reasonable patch now, I don't
> want to make you unhappy, but make 'every board must have its own dts'
> in U-Boot to make
> every developer to join U-Boot does not make sense to me. The kernel
> need different
> dts for different board because they need to use/control those different
> feature, but U-Boot
> is not the case, U-Boot should work if the storage driver works.

Well, here's the thing.  If you want U-Boot to then load and pass the
correct DTB to the kernel, we need per-board tweaks to the code anyhow
to find and load that DTB (see the various "findfdt" environment scripts
for example).  If you want to rework things so that you have a "generic"
type board under U-Boot that's more clearly not tied to any specific
board but instead runs on many, that might help clear things up too.
Hope this helps!
Kever Yang Dec. 12, 2018, 9:05 a.m. UTC | #7
On 12/11/2018 02:20 AM, Tom Rini wrote:
> On Sat, Dec 08, 2018 at 12:27:42PM +0800, Kever Yang wrote:
>> Hi Tom,
>>
>>
>> On 12/07/2018 10:13 PM, Tom Rini wrote:
>>> On Fri, Dec 07, 2018 at 02:24:22PM +0100, Philipp Tomsich wrote:
>>>> Kever,
>>>>
>>>>> On 07.12.2018, at 02:39, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>
>>>>> Hi Philipp,
>>>>>
>>>>> On 12/06/2018 09:50 PM, Philipp Tomsich wrote:
>>>>>> +Tom
>>>>>>
>>>>>>> On 05.12.2018, at 03:25, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>>>
>>>>>>> The U-Boot eMMC does not need to care about the power for Rockchip
>>>>>>> SoC, because if the board is using eMMC, the power will default on
>>>>>>> (for bootrom), and we do not do power management for it like kernel,
>>>>>>> so the 'vmmc', 'vqmmc' is only useful for SD in U-Boot.
>>>>>>>
>>>>>>> This make U-Boot can boot into kernel even if the pmic driver is
>>>>>>> broken.
>>>>>> If the PMIC driver is broken, we should fix the PMIC driver.
>>>>>> I would feel more comfortable w/o this statement.
>>>>>>
>>>>>>> The rk3288-evb dts may be used in many boards using rockchip reference
>>>>>>> schematic but with little change, so we hope it can be more robust to
>>>>>>> boot into next stage.
>>>>>> Again, this is not how the DTS should be used.  I believe that Heiko, Fabio and
>>>>>> I had already highlighted this in comments to the earlier thread.
>>>>>     Not sure if you have read my previous mail for answer all your comments,
>>>>>
>>>>> I do agree DTS should represent the hardware, but please note that the DTS
>>>>> is no kind of standard, and people always choose what they need and add
>>>>> those part in there dts, but not always add all the property and
>>>>> everyone use the same model. I would say there are many boards does not have this
>>>>> 'vmmc-supply’ in there emmc node.
>>>> That is exactly the reason why I bumped the decision up the stairs (to Tom and/or
>>>> Simon): what you are saying makes sense to me (viewed through your eyes and 
>>>> from your specific usecase), but it directly contradicts how the DTS usage is intended.
>>>>
>>>> In other words: Tom (as the top-level decision maker) or Simon (who owns the 
>>>> device-model and therefore will also have an opinion on DTS usage) should make
>>>> the final call.
>>> My answer is that I would strongly suspect that over in linux "we have N
>>> different close-enough boards using this one DTS" isn't acceptable.  You
>>> make a dtsi and include it from the board and things that aren't common
>>> don't go into the dtsi.  And yes, when starting off everyone (myself
>>> included) copies the reference platform dts and then changes it as
>>> needed, and sometimes misses a thing or two.  But no, I don't think we
>>> want a wrong dts and I'm pretty sure the kernel really wouldn't want
>>> wrong dts files and the general goal is that excluding the -u-boot.dtsi
>>> files, ours are copies of the kernel.
>> I don't think this is a "wrong dts" after my patch, these two nodes are
>> not mark
>> as required property in kernel, so many dts emmc node does not have it.
>> I check the latest kernel dtsi in arch/arm/boot/dts/rk3288-evb.dtsi [1],
>> the emmc node do not have 'vmmc' and 'vqmmc' while the SD node have, which
>> just like description in my commit message.
> OK.  So this would fall into the category of "sync with upstream dts"
> then, right?  That is what we want.

OK, Got it, 'sync with upstream dts' is a good and acceptable commit
message
rather than the real reason why we need the patch.

Philipp,
    Do you need me to send out a patch with update the commit message?

Thanks,
- Kever
>> Well, I don't know why U-Boot project is so difficult to accept a
>> reasonable patch now, I don't
>> want to make you unhappy, but make 'every board must have its own dts'
>> in U-Boot to make
>> every developer to join U-Boot does not make sense to me. The kernel
>> need different
>> dts for different board because they need to use/control those different
>> feature, but U-Boot
>> is not the case, U-Boot should work if the storage driver works.
> Well, here's the thing.  If you want U-Boot to then load and pass the
> correct DTB to the kernel, we need per-board tweaks to the code anyhow
> to find and load that DTB (see the various "findfdt" environment scripts
> for example).  If you want to rework things so that you have a "generic"
> type board under U-Boot that's more clearly not tied to any specific
> board but instead runs on many, that might help clear things up too.
> Hope this helps!
>
Philipp Tomsich Dec. 12, 2018, 9:51 a.m. UTC | #8
> On 12.12.2018, at 10:05, Kever Yang <kever.yang@rock-chips.com> wrote:
> 
> 
> 
> On 12/11/2018 02:20 AM, Tom Rini wrote:
>> On Sat, Dec 08, 2018 at 12:27:42PM +0800, Kever Yang wrote:
>>> Hi Tom,
>>> 
>>> 
>>> On 12/07/2018 10:13 PM, Tom Rini wrote:
>>>> On Fri, Dec 07, 2018 at 02:24:22PM +0100, Philipp Tomsich wrote:
>>>>> Kever,
>>>>> 
>>>>>> On 07.12.2018, at 02:39, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>> 
>>>>>> Hi Philipp,
>>>>>> 
>>>>>> On 12/06/2018 09:50 PM, Philipp Tomsich wrote:
>>>>>>> +Tom
>>>>>>> 
>>>>>>>> On 05.12.2018, at 03:25, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>>>>> 
>>>>>>>> The U-Boot eMMC does not need to care about the power for Rockchip
>>>>>>>> SoC, because if the board is using eMMC, the power will default on
>>>>>>>> (for bootrom), and we do not do power management for it like kernel,
>>>>>>>> so the 'vmmc', 'vqmmc' is only useful for SD in U-Boot.
>>>>>>>> 
>>>>>>>> This make U-Boot can boot into kernel even if the pmic driver is
>>>>>>>> broken.
>>>>>>> If the PMIC driver is broken, we should fix the PMIC driver.
>>>>>>> I would feel more comfortable w/o this statement.
>>>>>>> 
>>>>>>>> The rk3288-evb dts may be used in many boards using rockchip reference
>>>>>>>> schematic but with little change, so we hope it can be more robust to
>>>>>>>> boot into next stage.
>>>>>>> Again, this is not how the DTS should be used.  I believe that Heiko, Fabio and
>>>>>>> I had already highlighted this in comments to the earlier thread.
>>>>>>    Not sure if you have read my previous mail for answer all your comments,
>>>>>> 
>>>>>> I do agree DTS should represent the hardware, but please note that the DTS
>>>>>> is no kind of standard, and people always choose what they need and add
>>>>>> those part in there dts, but not always add all the property and
>>>>>> everyone use the same model. I would say there are many boards does not have this
>>>>>> 'vmmc-supply’ in there emmc node.
>>>>> That is exactly the reason why I bumped the decision up the stairs (to Tom and/or
>>>>> Simon): what you are saying makes sense to me (viewed through your eyes and 
>>>>> from your specific usecase), but it directly contradicts how the DTS usage is intended.
>>>>> 
>>>>> In other words: Tom (as the top-level decision maker) or Simon (who owns the 
>>>>> device-model and therefore will also have an opinion on DTS usage) should make
>>>>> the final call.
>>>> My answer is that I would strongly suspect that over in linux "we have N
>>>> different close-enough boards using this one DTS" isn't acceptable.  You
>>>> make a dtsi and include it from the board and things that aren't common
>>>> don't go into the dtsi.  And yes, when starting off everyone (myself
>>>> included) copies the reference platform dts and then changes it as
>>>> needed, and sometimes misses a thing or two.  But no, I don't think we
>>>> want a wrong dts and I'm pretty sure the kernel really wouldn't want
>>>> wrong dts files and the general goal is that excluding the -u-boot.dtsi
>>>> files, ours are copies of the kernel.
>>> I don't think this is a "wrong dts" after my patch, these two nodes are
>>> not mark
>>> as required property in kernel, so many dts emmc node does not have it.
>>> I check the latest kernel dtsi in arch/arm/boot/dts/rk3288-evb.dtsi [1],
>>> the emmc node do not have 'vmmc' and 'vqmmc' while the SD node have, which
>>> just like description in my commit message.
>> OK.  So this would fall into the category of "sync with upstream dts"
>> then, right?  That is what we want.
> 
> OK, Got it, 'sync with upstream dts' is a good and acceptable commit
> message
> rather than the real reason why we need the patch.
> 
> Philipp,
>     Do you need me to send out a patch with update the commit message?

If you do it, you’ll save me work and the history on the list is 1:1 identical
with what I apply …

If you absolutely don’t have time, I’ll change the commit message as I process
this for my end-of-week pull-request.

Thanks,
Philipp.
diff mbox series

Patch

diff --git a/arch/arm/dts/rk3288-evb.dtsi b/arch/arm/dts/rk3288-evb.dtsi
index ce75bd5d28..04902c0bd3 100644
--- a/arch/arm/dts/rk3288-evb.dtsi
+++ b/arch/arm/dts/rk3288-evb.dtsi
@@ -150,8 +150,6 @@ 
 	num-slots = <1>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&emmc_clk>, <&emmc_cmd>, <&emmc_pwr>, <&emmc_bus8>;
-	vmmc-supply = <&vcc_io>;
-	vqmmc-supply = <&vcc_flash>;
 	status = "okay";
 };