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] [thread-next>] [day] [month] [year] [list]

Message-Id: <306b0b30-5a32-4c7c-86b4-57d50e2307e8@app.fastmail.com>
Date: Thu, 28 Nov 2024 15:11:56 +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 14:33, Jerome Brunet wrote:
> On Wed 27 Nov 2024 at 22:23, "Arnd Bergmann" <arnd@...db.de> wrote:
>> On Wed, Nov 27, 2024, at 21:56, Jerome Brunet wrote:
>>> On Wed 27 Nov 2024 at 20:30, "Arnd Bergmann" <arnd@...db.de> wrote:
>>>>
>>>> It looks like RESET_MESON_AUX is a user-visible symbol,
>>>> so you can simply ask users to turn it on, and add it to
>>>> the defconfig.
>>>
>>> That would work yes but It's really something a user should not be
>>> concerned with. I can follow-up with another change to remove the user
>>> visibilty of RESET_MESON_AUX. It is always going to be something
>>> requested by another driver.
>>
>> But that's true for all reset drivers, each one of them is
>> only useful because it's going to be used by another driver,
>> same for clk, pinctrl, regulator, ...
>>
>
> All clk, pinctrl or regulator are used by other driver yes, this one as
> well, sure.
>
> What special about this on is that it is an auxiliary bus driver.
> It is directly instantiated by another driver. That's where
> it differs. The axg-audio clock driver instantiate the auxiliary reset,
> it does not use it, which is why it makes sense for it to select the
> driver.

Can you explain the logic behind this design? It seems that the
entire problem here is the split into more drivers than it
should be. It's common for clk drivers to also act as a
reset driver, and my impression here is that you were trying
too hard to split out the reset functionality into file
in drivers/reset/ rather than to have it in drivers/clk/.

Could you perhaps move the contents of
drivers/reset/amlogic/reset-meson-aux.c into
drivers/clk/meson/axg-audio.c, and change the exported
symbol to a static function? This would still require
a dependency on the exported meson_reset_toggle_ops,
but that feels like a more natural interface here,
since it's just a library module.

     Arnd