Re: [PATCH v11 09/11] clk: realtek: Add RTD1625-CRT clock controller driver

[Brian Masney <bmasney@redhat.com>]

On Mon, Aug 03, 2026 at 09:56:01AM +0000, Yu-Chun Lin [林祐君] wrote:
> > On Fri, Jul 31, 2026 at 08:17:44AM +0000, Yu-Chun Lin [林祐君] wrote:
> > > > On Tue, Jul 28, 2026 at 10:28:04PM +0800, Yu-Chun Lin wrote:
> > > > > From: Cheng-Yu Lee <cylee12@realtek.com>
> > > > >
> > >
> > > (...)
> > >
> > > > > +     { /* sentinel */ }
> > > > > +};
> > > > > +MODULE_DEVICE_TABLE(of, rtd1625_crt_match);
> > > > > +
> > > > > +static struct platform_driver rtd1625_crt_driver = {
> > > > > +     .probe = rtd1625_crt_probe,
> > > > > +     .driver = {
> > > > > +             .name = "rtk-rtd1625-crt-clk",
> > > > > +             .of_match_table = rtd1625_crt_match,
> > > > > +             .suppress_bind_attrs = true,
> > > >
> > > > This driver is listed as a tristate in the Kconfig but then you have
> > > > this. Is the intention of this driver to be built in only?
> > > >
> > >
> > > I will change the Kconfig options for this driver, as well as the
> > > related config, from tristate to bool.
> > 
> > Keep it as a tristate if you are going to use module_platform_driver().
> > You can also drop suppress_bind_attrs unless you have a specific need why.
> > 
> > Brian
> 
> You are right. Keeping it tristate is beneficial, especially for aligning
> with the Android GKI design.
> 
> To properly support module unloading and driver unbinding, I will add
> the .remove callback and drop suppress_bind_attrs.

If you are able to use all devm_*() APIs, then you shouldn't need a
remove callback.

Brian


> 
> Thank you very much for the thorough review and suggestions!
> 
> Best regards,
> Yu Chun