linux-kernel - Re: [PATCH v2] mux: mmio: 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] [day] [month] [year] [list]

Message-ID: <5d0a298f-559a-466b-b13a-7c6520406bb6@ti.com>
Date: Wed, 4 Dec 2024 08:52:48 -0600
From: Andrew Davis <afd@...com>
To: Thomas Richard <thomas.richard@...tlin.com>, Peter Rosin <peda@...ntia.se>
CC: <linux-kernel@...r.kernel.org>, <gregory.clement@...tlin.com>,
        <theo.lebrun@...tlin.com>, <thomas.petazzoni@...tlin.com>,
        <u-kumar1@...com>
Subject: Re: [PATCH v2] mux: mmio: Add suspend and resume support

On 12/4/24 8:14 AM, Thomas Richard wrote:
> On 12/4/24 14:53, Andrew Davis wrote:
>> On 9/11/24 3:53 AM, Thomas Richard wrote:
>>> The status of each mux is read during suspend and stored in the private
>>> memory of the mux_chip.
>>> Then the state is restored during the resume.
>>>
>>> Signed-off-by: Thomas Richard <thomas.richard@...tlin.com>
>>> ---
>>> In this second version, as discussed with Peter, everything is done in
>>> the
>>> mmio-mux driver.
>>> A mux_mmio_set() function was added, and used during suspend stage to get
>>> the status of the of the muxes.
>>> This status is stored in the private memory of the mux_chip.
>>> ---
>>> Changes in v2:
>>> - Remove all modifications done in the mux subsystem
>>> - Add a mux_mmio_set()
>>> - Read the status of muxes during suspend and store in the private memory
>>>     of the mux_chip.
>>
>> Do you need this private memory? Since this is already using regmap, why
>> not use the regmap cache, then you can restore all the values on resume
>> with a simple call to regcache_sync().
> The regmap can be set without cache.
> As you inherit the regmap, you cannot assume that the cache is enabled.
> 

Fair point, we don't really know much about the underlying registers.
One thing we don't know is if they are retained during suspend, so
saving and restoring might be unneeded in some cases. Maybe that isn't
something we can solve at this point, could be something that should
be done in the regmap framework instead..

For now, the rest of this patch seems good to me,

Reviewed-by: Andrew Davis <afd@...com>