On 13.01.2021 13:36, [email protected] wrote:
>
>
> On 13.01.2021 13:09, Heiner Kallweit wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 13.01.2021 10:29, [email protected] wrote:
>>> Hi Heiner,
>>>
>>> On 08.01.2021 18:31, Heiner Kallweit wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> On 08.01.2021 16:45, Claudiu Beznea wrote:
>>>>> KSZ9131 is used in setups with SAMA7G5. SAMA7G5 supports a special
>>>>> power saving mode (backup mode) that cuts the power for almost all
>>>>> parts of the SoC. The rail powering the ethernet PHY is also cut off.
>>>>> When resuming, in case the PHY has been configured on probe with
>>>>> slew rate or DLL settings these needs to be restored thus call
>>>>> driver's config_init() on resume.
>>>>>
>>>> When would the SoC enter this backup mode?
>>>
>>> It could enter in this mode based on request for standby or suspend-to-mem:
>>> echo mem > /sys/power/state
>>> echo standby > /sys/power/state
>>>
>>> What I didn't mentioned previously is that the RAM remains in self-refresh
>>> while the rest of the SoC is powered down.
>>>
>>
>> This leaves the question which driver sets backup mode in the SoC.
>
> From Linux point of view the backup mode is a standard suspend-to-mem PM
> mode. The only difference is in SoC specific PM code
> (arch/arm/mach-at91/pm_suspend.S) where the SoC shutdown command is
> executed at the end and the fact that we save the address in RAM of
> cpu_resume() function in a powered memory. Then, the resume is done with
> the help of bootloader (it configures necessary clocks) and jump the
> execution to the previously saved address, resuming Linux.
>
>> Whatever/whoever wakes the SoC later would have to take care that basically
>> everything that was switched off is reconfigured (incl. calling phy_init_hw()).
>
> For this the bootloader should know the PHY settings passed via DT (skew
> settings or DLL settings). The bootloader runs from a little SRAM which, at
> the moment doesn't know to parse DT bindings and the DT parsing lib might
> be big enough that the final bootloader size will cross the SRAM size.
>
>> So it' more or less the same as waking up from hibernation. Therefore I think
>> the .restore of all subsystems would have to be executed, incl. .restore of
>> the MDIO bus.
>
> I see your point. I think it has been implemented like a standard
> suspend-to-mem PM mode because the RAM remains in self-refresh whereas in
> hibernation it is shut of (as far as I know).
>
>> Having said that I don't think that change belongs into the
>> PHY driver.
>> Just imagine tomorrow another PHY type is used in a SAMA7G5 setup.
>> Then you would have to do same change in another PHY driver.
>
> I understand this. At the moment the PM code for drivers in SAMA7G5 are
> saving/restoring in/from RAM the registers content in suspend/resume()
> functions of each drivers and I think it has been chosen like this as the
> RAM remains in self-refresh. Mapping this mode to hibernation will involve
> saving the content of RAM to a non-volatile support which is not wanted as
> this increases the suspend/resume time and it wasn't intended.
>
>>
>>
>>>> And would it suspend the
>>>> MDIO bus before cutting power to the PHY?
>>>
>>> SAMA7G5 embeds Cadence macb driver which has a integrated MDIO bus. Inside
>>> macb driver the bus is registered with of_mdiobus_register() or
>>> mdiobus_register() based on the PHY devices present in DT or not. On macb
>>> suspend()/resume() functions there are calls to
>>> phylink_stop()/phylink_start() before cutting/after enabling the power to
>>> the PHY.
>>>
>>>> I'm asking because in mdio_bus_phy_restore() we call phy_init_hw()
>>>> already (that calls the driver's config_init).
>>>
>>> As far as I can see from documentation the .restore API of dev_pm_ops is
>>> hibernation specific (please correct me if I'm wrong). On transitions to
>>> backup mode the suspend()/resume() PM APIs are called on the drivers.
>>>
I'm not a Linux PM expert, to me it seems your use case is somewhere in the
middle between s2r and hibernation. I *think* the assumption with s2r is
that one component shouldn't simply cut the power to another component,
and the kernel has no idea about it.
My personal point of view:
If a driver cuts power to another component in s2r, it should take care that
this component is properly re-initialized once power is back.
Otherwise I would miss to see why we need different callbacks resume and restore.
It may be worth to involve the following people/list:
HIBERNATION (aka Software Suspend, aka swsusp)
M: "Rafael J. Wysocki" <[email protected]>
M: Pavel Machek <[email protected]>
L: [email protected]
>>> Thank you,
>>> Claudiu Beznea
>>>
>>>>
>>>>> Signed-off-by: Claudiu Beznea <[email protected]>
>>>>> ---
>>>>> drivers/net/phy/micrel.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>>>>> index 3fe552675dd2..52d3a0480158 100644
>>>>> --- a/drivers/net/phy/micrel.c
>>>>> +++ b/drivers/net/phy/micrel.c
>>>>> @@ -1077,7 +1077,7 @@ static int kszphy_resume(struct phy_device *phydev)
>>>>> */
>>>>> usleep_range(1000, 2000);
>>>>>
>>>>> - ret = kszphy_config_reset(phydev);
>>>>> + ret = phydev->drv->config_init(phydev);
>>>>> if (ret)
>>>>> return ret;
>>>>>
>>>>>
On Wed, Jan 13, 2021 at 10:34:53PM +0100, Heiner Kallweit wrote:
> On 13.01.2021 13:36, [email protected] wrote:
> > On 13.01.2021 13:09, Heiner Kallweit wrote:
> >> On 13.01.2021 10:29, [email protected] wrote:
> >>> It could enter in this mode based on request for standby or suspend-to-mem:
> >>> echo mem > /sys/power/state
> >>> echo standby > /sys/power/state
This is a standard way to enter S2R - I've used it many times in the
past on platforms that support it.
> I'm not a Linux PM expert, to me it seems your use case is somewhere in the
> middle between s2r and hibernation. I *think* the assumption with s2r is
> that one component shouldn't simply cut the power to another component,
> and the kernel has no idea about it.
When entering S2R, power can (and probably will) be cut to all system
components, certainly all components that do not support wakeup. If
the system doesn't support WoL, then that will include the ethernet
PHY.
When resuming, the responsibility is of the kernel and each driver's
.resume function to ensure that the hardware state is restored. Only
each device driver that knows the device itself can restore the state
of that device. In the case of an ethernet PHY, that is phylib and
its associated PHY driver.
One has to be a tad careful with phylib and PHYs compared to their
MAC devices in terms of the resume order - it has not been unheard
of over the years for a MAC device to be resumed before its connected
PHY has been.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On 13.01.2021 23:01, Russell King - ARM Linux admin wrote:
> On Wed, Jan 13, 2021 at 10:34:53PM +0100, Heiner Kallweit wrote:
>> On 13.01.2021 13:36, [email protected] wrote:
>>> On 13.01.2021 13:09, Heiner Kallweit wrote:
>>>> On 13.01.2021 10:29, [email protected] wrote:
>>>>> It could enter in this mode based on request for standby or suspend-to-mem:
>>>>> echo mem > /sys/power/state
>>>>> echo standby > /sys/power/state
>
> This is a standard way to enter S2R - I've used it many times in the
> past on platforms that support it.
>
>> I'm not a Linux PM expert, to me it seems your use case is somewhere in the
>> middle between s2r and hibernation. I *think* the assumption with s2r is
>> that one component shouldn't simply cut the power to another component,
>> and the kernel has no idea about it.
>
> When entering S2R, power can (and probably will) be cut to all system
> components, certainly all components that do not support wakeup. If
> the system doesn't support WoL, then that will include the ethernet
> PHY.
>
I'm with you if we talk about a driver's suspend callback cutting power
to the component it controls, or at least putting it to a power-saving
state. However a driver shouldn't have to expect that during S2R somebody
else cuts the power. If this would be the case, then I think we wouldn't
need separate resume and restore pm callbacks in general.
> When resuming, the responsibility is of the kernel and each driver's
> .resume function to ensure that the hardware state is restored. Only
> each device driver that knows the device itself can restore the state
> of that device. In the case of an ethernet PHY, that is phylib and
> its associated PHY driver.
>
Also in phylib we have separate functions mdio_bus_phy_resume() and
mdio_bus_phy_restore(), with the first one not fully reconfiguring
the PHY.
> One has to be a tad careful with phylib and PHYs compared to their
> MAC devices in terms of the resume order - it has not been unheard
> of over the years for a MAC device to be resumed before its connected
> PHY has been.
>
Right.