linux-kernel - Re: [PATCH 13/13] net: ravb: Add runtime PM 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: <ec9b6604-839f-d31f-59c6-72367804d06a@omp.ru>
Date:   Mon, 27 Nov 2023 22:01:16 +0300
From:   Sergey Shtylyov <s.shtylyov@....ru>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        claudiu beznea <claudiu.beznea@...on.dev>
CC:     <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
        <pabeni@...hat.com>, <p.zabel@...gutronix.de>,
        <yoshihiro.shimoda.uh@...esas.com>,
        <wsa+renesas@...g-engineering.com>, <biju.das.jz@...renesas.com>,
        <prabhakar.mahadev-lad.rj@...renesas.com>,
        <sergei.shtylyov@...entembedded.com>,
        <mitsuhiro.kimura.kc@...esas.com>, <masaru.nagai.vx@...esas.com>,
        <netdev@...r.kernel.org>, <linux-renesas-soc@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH 13/13] net: ravb: Add runtime PM support

On 11/27/23 5:05 PM, Geert Uytterhoeven wrote:

[...]
>>>>>> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
>>>>>
>>>>>> RZ/G3S supports enabling/disabling clocks for its modules (including
>>>>>> Ethernet module). For this commit adds runtime PM support which
>>>>>> relies on PM domain to enable/disable Ethernet clocks.
>>>>>
>>>>>    That's not exactly something new in RZ/G3S. The ravb driver has unconditional
>>>>> RPM calls already in the probe() and remove() methods...
>>>>> And the sh_eth driver
>>>>> has RPM support since 2009...
>>>>>
>>>>>> At the end of probe ravb_pm_runtime_put() is called which will turn
>>>>>
>>>>>    I'd suggest a shorter name, like ravb_rpm_put() but (looking at this function)
>>>>>> off the Ethernet clocks (if no other request arrives at the driver).
>>>>>> After that if the interface is brought up (though ravb_open()) then
>>>>>> the clocks remain enabled until interface is brought down (operation
>>>>>> done though ravb_close()).
>>>>>>
>>>>>> If any request arrives to the driver while the interface is down the
>>>>>> clocks are enabled to serve the request and then disabled.
>>>>>>
>>>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>

[...]

> Sergey: does sh_eth.c really reinitialize the hardware completely after
> pm_runtime_get_sync()?

   Well, even with the original Magnus' commit that added the RPM support (bcd5149ded6b2edbf3732fa1483600a716b1cba6) it wasn't so -- sh_eth_open()
indeed seemed to re-init everything (but not TSU!) but sh_eth_get_stats()
surely didn't (the RPM calls there have been removed since); other RPM
"wrappers" have been added to the driver methods since -- which also
don't init anything... thus the comment in sh_eth_runtime_nop(() seems
to be wrong from the very start...

[...]

> Gr{oetje,eeting}s,
> 
>                         Geert

MBR, Sergey