linux-kernel - Re: [PATCH] clk: amlogic: axg-audio: select RESET_MESON

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: <4206bf5d-a11e-4d0d-86b7-50c922e41119@app.fastmail.com>
Date: Thu, 28 Nov 2024 18:21:19 +0100
From: "Arnd Bergmann" <arnd@...db.de>
To: "Jerome Brunet" <jbrunet@...libre.com>
Cc: "Neil Armstrong" <neil.armstrong@...aro.org>,
 "Michael Turquette" <mturquette@...libre.com>,
 "Stephen Boyd" <sboyd@...nel.org>, "Kevin Hilman" <khilman@...libre.com>,
 "Martin Blumenstingl" <martin.blumenstingl@...glemail.com>,
 linux-amlogic@...ts.infradead.org, linux-clk@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 "Mark Brown" <broonie@...nel.org>
Subject: Re: [PATCH] clk: amlogic: axg-audio: select RESET_MESON_AUX

On Thu, Nov 28, 2024, at 16:53, Jerome Brunet wrote:
> On Thu 28 Nov 2024 at 16:34, "Arnd Bergmann" <arnd@...db.de> wrote:
>> On Thu, Nov 28, 2024, at 16:06, Jerome Brunet wrote:
>>> We are deviating a bit from the initial regression reported by Mark.
>>> Is Ok with you to proceed with that fix and then continue this discussion
>>> ?
>>
>> I really don't want to see those stray 'select' statements
>> in there, as that leave very little incentive for anyone to
>> fix it properly.
>>
>> It sounds like Stephen gave you bad advice for how it should
>> be structured, so my best suggestion would be to move the
>> the problem (and the reset driver) back into his subsystem
>> and leave only a simple 'select RESET_CONTROLLER'.
>
> Okay, though I don't really understand why that select is okay and not
> the the proposed one. There is apparently a subtility I'm missing I'd
> like to avoid repeating that.

The thing with 'select' is that it really has to be used very
selectively. The 'select RESET_CONTROLLER' is fine as an
exception because there are already tons of clk drivers
that do this consistently so they can register themselves
as a reset controller.

A driver selecting a driver from another subsystem is pretty
much always a mistake. A single one may not cause much harm,
but the problems are frequent enough that we need to have
fewer of them rather than more.

>> From the message you cited, I think Stephen had the right
>> intentions ("so that the clk and reset drivers are decoupled"),
>> but the end result did not actually do what he intended
>> even if you did what he asked for.
>>
>> Stephen, can you please take a look here and see if you
>> have a better idea for either decoupling the two drivers
>> enough to avoid the link time dependency, or to reintegrate
>> the reset controller code into the clk driver and avoid
>> the complexity?
>
> If I may,
>
> * short term fix: revert both your fix and the initial clock
>   change that needed fixing. That will essentially bring back the reset
>   implementation in clock.
>
> * after that: remove the creation part from driver/reset and bring back
>   something similar to what was proposed in the initial RFC for the
>   creation and finally switch the clock driver back to it.
>   That should provide the proper decoupling your are requesting I think.

Works for me as well, though Mark's suggestion would be simpler.

     Arnd