Re: [PATCH] clk: amlogic: axg-audio: select RESET_MESON_AUX

["Arnd Bergmann" <arnd@arndb.de>]

On Thu, Nov 28, 2024, at 16:53, Jerome Brunet wrote:
> On Thu 28 Nov 2024 at 16:34, "Arnd Bergmann" <arnd@arndb.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

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic