linux-kernel - Re: [PATCH V3 02/17] pinctrl: tegra: add suspend and resume support

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: <cff9b6a2-dc33-d03b-9945-799b158deb07@nvidia.com>
Date:   Tue, 18 Jun 2019 10:34:59 -0700
From:   Sowjanya Komatineni <skomatineni@...dia.com>
To:     Stephen Warren <swarren@...dotorg.org>,
        Dmitry Osipenko <digetx@...il.com>
CC:     <thierry.reding@...il.com>, <jonathanh@...dia.com>,
        <tglx@...utronix.de>, <jason@...edaemon.net>,
        <marc.zyngier@....com>, <linus.walleij@...aro.org>,
        <stefan@...er.ch>, <mark.rutland@....com>,
        <pdeschrijver@...dia.com>, <pgaikwad@...dia.com>,
        <sboyd@...nel.org>, <linux-clk@...r.kernel.org>,
        <linux-gpio@...r.kernel.org>, <jckuo@...dia.com>,
        <josephl@...dia.com>, <talho@...dia.com>,
        <linux-tegra@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <mperttunen@...dia.com>, <spatra@...dia.com>, <robh+dt@...nel.org>,
        <devicetree@...r.kernel.org>
Subject: Re: [PATCH V3 02/17] pinctrl: tegra: add suspend and resume support


On 6/18/19 9:50 AM, Sowjanya Komatineni wrote:
>
> On 6/18/19 8:41 AM, Stephen Warren wrote:
>> On 6/18/19 3:30 AM, Dmitry Osipenko wrote:
>>> 18.06.2019 12:22, Dmitry Osipenko пишет:
>>>> 18.06.2019 10:46, Sowjanya Komatineni пишет:
>>>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>>>> and registers them to syscore so the pinmux settings are restored
>>>>> before the devices resume.
>>>>>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@...dia.com>
>>>>> ---
>>>>>   drivers/pinctrl/tegra/pinctrl-tegra.c    | 62 
>>>>> ++++++++++++++++++++++++++++++++
>>>>>   drivers/pinctrl/tegra/pinctrl-tegra.h    |  5 +++
>>>>>   drivers/pinctrl/tegra/pinctrl-tegra114.c |  1 +
>>>>>   drivers/pinctrl/tegra/pinctrl-tegra124.c |  1 +
>>>>>   drivers/pinctrl/tegra/pinctrl-tegra20.c  |  1 +
>>>>>   drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
>>>>>   drivers/pinctrl/tegra/pinctrl-tegra30.c  |  1 +
>>>>>   7 files changed, 84 insertions(+)
>>>>>
>>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c 
>>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>> index 34596b246578..ceced30d8bd1 100644
>>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>> @@ -20,11 +20,16 @@
>>>>>   #include <linux/pinctrl/pinmux.h>
>>>>>   #include <linux/pinctrl/pinconf.h>
>>>>>   #include <linux/slab.h>
>>>>> +#include <linux/syscore_ops.h>
>>>>>     #include "../core.h"
>>>>>   #include "../pinctrl-utils.h"
>>>>>   #include "pinctrl-tegra.h"
>>>>>   +#define EMMC2_PAD_CFGPADCTRL_0            0x1c8
>>>>> +#define EMMC4_PAD_CFGPADCTRL_0            0x1e0
>>>>> +#define EMMC_DPD_PARKING            (0x1fff << 14)
>>>>> +
>>>>>   static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 
>>>>> reg)
>>>>>   {
>>>>>       return readl(pmx->regs[bank] + reg);
>>>>> @@ -619,6 +624,48 @@ static void 
>>>>> tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>>>               pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
>>>>>           }
>>>>>       }
>>>>> +
>>>>> +    if (pmx->soc->has_park_padcfg) {
>>>>> +        val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>> +        pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>> +
>>>>> +        val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>> +        val &= ~EMMC_DPD_PARKING;
>>>>> +        pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>> +    }
>>>>> +}
>>>>
>>>> Is there any reason why parked_bit can't be changed to 
>>>> parked_bitmask like I was
>>>> asking in a comment to v2?
>>>>
>>>> I suppose that it's more preferable to keep pinctrl-tegra.c 
>>>> platform-agnostic for
>>>> consistency when possible, hence adding platform specifics here 
>>>> should be discouraged.
>>>> And then the parked_bitmask will also result in a proper hardware 
>>>> description in the code.
>>>>
>>>
>>> I'm now also vaguely recalling that Stephen Warren had some kind of 
>>> a "code generator"
>>> for the pinctrl drivers. So I guess all those tables were 
>>> auto-generated initially.
>>>
>>> Stephen, maybe you could adjust the generator to take into account 
>>> the bitmask (of
>>> course if that's a part of the generated code) and then re-gen it 
>>> all for Sowjanya?
>>
>> https://github.com/NVIDIA/tegra-pinmux-scripts holds the scripts that 
>> generate tegra-pinctrlNNN.c. See soc-to-kernel-pinctrl-driver.py. 
>> IIRC, tegra-pinctrl.c (the core file) isn't auto-generated. Sowjanya 
>> is welcome to send a patch to that repo if the code needs to be updated.
>
>
> Hi Dmitry,
>
> Just want to be clear on my understanding of your request.
>
> "change parked_bit to parked_bitmask" are you requested to change 
> parked_bit of PINGROUP and DRV_PINGROUP to use bitmask value rather 
> than bit position inorder to have parked bit configuration for EMMC 
> PADs as well to happen by masking rather than checking for existence 
> of parked_bit?
>
> Trying to understand the reason/benefit for changing parked_bit to 
> parked_bitmask.
Also, Park bits in CFGPAD registers are not common for all CFGPAD 
registers. Park bits are available only for EMMC and also those bits are 
used for something else on other CFGPAD registers so bitmask can't be 
common and this also need an update to DRV_PINGROUP macro args just only 
to handle EMMC parked_bitmask. So not sure of the benefit in using 
bitmask rather than parked_bit
>
> thanks
>
> Sowjanya
>