linux-kernel - RE: [PATCH RESEND] driver core: export device_is

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: <AM6PR04MB4966EFA169A7C29456ACD44680E00@AM6PR04MB4966.eurprd04.prod.outlook.com>
Date:   Thu, 19 Nov 2020 15:20:25 +0000
From:   Aisheng Dong <aisheng.dong@....com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC:     Saravana Kannan <saravanak@...gle.com>,
        Sudip Mukherjee <sudipm.mukherjee@...il.com>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Shawn Guo <shawnguo@...nel.org>,
        Stephen Boyd <sboyd@...nel.org>
Subject: RE: [PATCH RESEND] driver core: export device_is_bound() to fix build
 failure

> From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Sent: Thursday, November 19, 2020 10:26 PM
> 
> On Thu, Nov 19, 2020 at 02:09:42PM +0000, Aisheng Dong wrote:
> > > From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > > Sent: Thursday, November 19, 2020 9:10 PM
> > >
> > > On Thu, Nov 19, 2020 at 04:13:34AM +0000, Aisheng Dong wrote:
> > > > > Long story short, either
> > > > >
> > > > > * Don't care about the power domain in your clock driver.
> > > > >
> > > > > Or,
> > > > >
> > > > > * List the power domain in the clock controller's DT node and
> > > > > then use the normal APIs to get the power domain. And defer like
> > > > > any other driver if you can't get the power domain.
> > > > >
> > > >
> > > > Yes, I understand those are for normal cases. But our case is a
> > > > bit different and we don't want
> > > > imx_clk_scu() API return PROBE_DEFER which is unnecessary for a
> > > > hundred of
> > > clocks.
> > > > Even we want to defer probe, we prefer to defer in
> > > > imx_clk_scu_init() rather
> > > than in imx_clk_scu().
> > >
> > > What is wrong with PROBE_DEFER, that is what it is there for.
> >
> > Yes, we can use PROBE_DEFER, just not want to defer in
> > imx_clk_scu_init() when creating sub clock devices. Instead, we want
> > to defer at the beginning of clock driver probe which can save tens of defer
> probes due to the same reasons that PD driver is not ready.
> 
> There's nothing wrong with deferring that many times until your proper driver is
> loaded, what does it cost you to do so?

One problem is that current imx8qxp-clk driver allows sub clocks register optionally fail.
That's also the reason we don't want defer probe during register sub clocks.

static int imx8qxp_clk_probe(struct platform_device *pdev)
{
	clks[IMX_LSIO_PWM0_CLK]		= imx_clk_scu("pwm0_clk", IMX_SC_R_PWM_0, IMX_SC_PM_CLK_PER, clk_cells);
	clks[IMX_LSIO_PWM1_CLK]		= imx_clk_scu("pwm1_clk", IMX_SC_R_PWM_1, IMX_SC_PM_CLK_PER, clk_cells)
	....
	return of_clk_add_hw_provider(ccm_node, imx_scu_of_clk_src_get, imx_scu_clks);
}

> 
> > > > Maybe the things can be simplified as a simple requirement:
> > > > How users can make Driver A (CLK) to be probed after Driver B (PD)
> > > > without explicit firmware function dependency description (e.g.
> > > > phandle in
> > > DT)?
> > > >
> > > > As kernel core does not want to support it, then the left way may
> > > > be change scu-pd driver Inicall level or provide a private
> > > > callback to query the
> > > readiness.
> > >
> > > No, do not mess with that, as it totally breaks when things are built as a
> module.
> > >
> >
> > Can't this be addressed by proper module dependency? E.g clock module
> > depends on power domain module. Then clock driver can only be loaded after
> power domain.
> 
> Sure, if you can do that, make your modules load properly by symbol
> dependency and all should be fine.  Have you done that?

Still no. I planned to do that in another separate patch
The rough idea may be:
In PD driver, export an API like:
Bool imx_scu_pd_is_initilized(void);

In SCU clock driver:
If (!imx_scu_pd_is_initialized())
	return -EPROBE_DEFER;

BTW I've already sent out a patch to remove the calling of device_is_bound() in order to fix
the build break first.
https://patchwork.kernel.org/project/linux-clk/patch/20201119114302.26263-1-aisheng.dong@nxp.com/
It's okay to remove it first as DT patch using this code still not merged in mainline.

Regards
Aisheng

> 
> thanks,
> 
> greg k-h