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

Stephen Boyd
2024-12-04T20:05:17Z

Quoting Jerome Brunet (2024-12-04 09:19:50)
> On Tue 03 Dec 2024 at 12:15, Stephen Boyd <sboyd@kernel.org> wrote:
>
> > Quoting Jerome Brunet (2024-12-03 03:15:41)
> >> On Mon 02 Dec 2024 at 18:53, Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Is the half finished migration a problem for this cycle? I was intending
> > to send the revert later this week and try again next cycle.
>
> Not really, with the fix you applied. There is just code present in
> reset that should not be used in its current form. I'd prefer to
> sanitise the situation sooner rather than later.

Alright. Let's just sort it out in the next few weeks for the next merge
window then. Maybe you can just do it once then and get auxiliary bus
maintainers to ack the patch so you can merge the helper locally and use
it in the amlogic clk tree.

> >
> > Sure. You can make devm_meson_clk_rst_aux_register() use the same
> > signature as I proposed above so that it's a one line patch later. And
> > definitely drop the imply RESET_MESON and depends on REGMAP part. Maybe
> > you can put it in the clkc-utils file?
>
> Sure. Few things I'd like to clarify
>
> * I tend think like Arnd, platform data will be needed eventually. Not
> sure having 2 functions, one with the param, one without is really
> worth it. We could just pass NULL when it is not needed. It is not
> uncommon. Would it be acceptable ? (for the generic helper, the
> temporary solution does not need that for sure)

I'll defer to the maintainers there. I don't feel strongly.

>
> * You mean (meson-)clkc-utils.c ? I could but that would add dependency on
> the auxiliary_bus for clock controllers that don't need it. It is a
> minor problem really that I could just ignore.
> I'd rather keep this helper separate if possible.

Ok, no worries.

>
> * Why drop 'imply RESET_MESON_AUX' ? I would still like the
> COMMON_CLK_AXG_AUDIO to 'strongly suggest' RESET_MESON_AUX, with
> dependency problem sorted out.

Because eventually you'll lose this Kconfig. I guess you'll want to add
that to the driver Kconfig option to maintain "strongly suggested".