linux-kernel - Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8c3827c4-1272-f004-52d9-7c79e93813c6@denx.de>
Date:   Thu, 4 May 2017 16:34:14 +0200
From:   Marek Vasut <marex@...x.de>
To:     Shawn Guo <shawnguo@...nel.org>
Cc:     Leonard Crestez <leonard.crestez@....com>,
        Peter Chen <Peter.Chen@....com>,
        Anson Huang <Anson.Huang@....com>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Sascha Hauer <kernel@...gutronix.de>,
        Fabio Estevam <fabio.estevam@....com>,
        Fabio Estevam <festevam@...il.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override

On 05/04/2017 03:41 PM, Shawn Guo wrote:
> On Thu, May 04, 2017 at 03:08:41PM +0200, Marek Vasut wrote:
>> On 05/04/2017 02:44 PM, Shawn Guo wrote:
>>> On Thu, May 04, 2017 at 12:06:11PM +0200, Marek Vasut wrote:
> 
> <snip>
> 
>>>> Mind you, my patch is not fixing any crash, it's correcting the
>>>> regulator binding and removing the OPP override which is at that
>>>> point useless.
>>>
>>> Heh, that's the primary reason why I prefer Leonard's patch, as his
>>> patch is fixing a critical crash issue, while yours is just removing
>>> some useless stuff and correcting something that doesn't show any real
>>> problem.
>>
>> Maybe you want to compare those two patches again, his patch fixing the
>> critical crash is removing the same "some useless stuff" as mine.
> 
> The difference is in commit message.  From your commit message, people
> do not see what real world user noticeable issue your patch is fixing.
> 
>> Plus
>> mine is actually making sure the regulators are configured correctly,
>> not just removing "some useless stuff" and hoping for the best that
>> bootloader will do the right thing.
> 
> Without this part, we do not get anything worse.  That said, it can be
> an improvement in another patch.
> 
>>>> Which part of the following commit message is unclear?
>>>>
>>>> "
>>>> The imx6sx-sdb has one power supply that drives both VDDARM_IN
>>>> and VDDSOC_IN, which is the sw1a regulator on the PFUZE PMIC.
>>>> Connect both inputs to the sw1a regulator on the PMIC and drop
>>>> the OPP hackery which is no longer needed as the power framework
>>>> will take care of the regulator configuration as needed.
>>>> "
>>>
>>> Something unclear in my opinion:
>>>
>>>  - The OPP hackery was never needed, as it's only needed for LDO bypass
>>>    mode which hasn't been supported by mainline kernel.  It's not 'no
>>>    longer needed as the power framework ...'.
>>
>> I don't know what this is about, it's not from my patch either ...
> 
> So I guess you do not understand how the OPP hackery was born and why it
> shouldn't be there for mainline kernel at all.

The OPP hackery is there to keep both regulators configured the same
way, since they share the same input voltage rail IMO. If you model the
power distribution correctly, the OPP hackery can be removed.

>>>  - What are the related changes in power framework?  It will be more
>>>    clear if we can have the particular commit mentioned here.
>>
>> Uh, I don't understand your question, there are no new changes in the
>> power framework. The DT for the SDB was always wrong, my patch fixes it.
> 
> Then 'no longer needed' in your commit message is really confusing.
> 
> Shawn
> 


-- 
Best regards,
Marek Vasut