On 05/15, Dong Aisheng wrote:
> +
> + clks[IMX7ULP_CLK_VIU] = imx_clk_gate("viu", "nic1_clk", base + 0xA0, 30);
> + clks[IMX7ULP_CLK_PCTLC] = imx_clk_gate("pctlc", "nic1_bus_clk", base + 0xB8, 30);
> + clks[IMX7ULP_CLK_PCTLD] = imx_clk_gate("pctld", "nic1_bus_clk", base + 0xBC, 30);
> + clks[IMX7ULP_CLK_PCTLE] = imx_clk_gate("pctle", "nic1_bus_clk", base + 0xc0, 30);
> + clks[IMX7ULP_CLK_PCTLF] = imx_clk_gate("pctlf", "nic1_bus_clk", base + 0xc4, 30);
> +
> + clks[IMX7ULP_CLK_GPU3D] = imx_clk_composite("gpu3d", periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, false, true, base + 0x140);
> + clks[IMX7ULP_CLK_GPU2D] = imx_clk_composite("gpu2d", periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, false, true, base + 0x144);
> +
> + imx_check_clocks(clks, ARRAY_SIZE(clks));
> +
> + clk_data.clks = clks;
> + clk_data.clk_num = ARRAY_SIZE(clks);
> + of_clk_add_provider(scg_node, of_clk_src_onecell_get, &clk_data);
Please use of_clk_add_hw_provider() instead, and the associated
clk_hw registration APIs.
> +
> + pr_info("i.MX7ULP clock tree init done.\n");
pr_debug?
> +}
> +
> +CLK_OF_DECLARE(imx7ulp, "fsl,imx7ulp-clock", imx7ulp_clocks_init);
>
Any reason why it can't be a platform driver? If not, please add
some comment explaining why.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Mon, Jun 19, 2017 at 07:01:19PM -0700, Stephen Boyd wrote:
> On 05/15, Dong Aisheng wrote:
> > +
> > + clks[IMX7ULP_CLK_VIU] = imx_clk_gate("viu", "nic1_clk", base + 0xA0, 30);
> > + clks[IMX7ULP_CLK_PCTLC] = imx_clk_gate("pctlc", "nic1_bus_clk", base + 0xB8, 30);
> > + clks[IMX7ULP_CLK_PCTLD] = imx_clk_gate("pctld", "nic1_bus_clk", base + 0xBC, 30);
> > + clks[IMX7ULP_CLK_PCTLE] = imx_clk_gate("pctle", "nic1_bus_clk", base + 0xc0, 30);
> > + clks[IMX7ULP_CLK_PCTLF] = imx_clk_gate("pctlf", "nic1_bus_clk", base + 0xc4, 30);
> > +
> > + clks[IMX7ULP_CLK_GPU3D] = imx_clk_composite("gpu3d", periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, false, true, base + 0x140);
> > + clks[IMX7ULP_CLK_GPU2D] = imx_clk_composite("gpu2d", periph_plat_sels, ARRAY_SIZE(periph_plat_sels), true, false, true, base + 0x144);
> > +
> > + imx_check_clocks(clks, ARRAY_SIZE(clks));
> > +
> > + clk_data.clks = clks;
> > + clk_data.clk_num = ARRAY_SIZE(clks);
> > + of_clk_add_provider(scg_node, of_clk_src_onecell_get, &clk_data);
>
> Please use of_clk_add_hw_provider() instead, and the associated
> clk_hw registration APIs.
>
Sure, will do it.
> > +
> > + pr_info("i.MX7ULP clock tree init done.\n");
>
> pr_debug?
>
Yes
> > +}
> > +
> > +CLK_OF_DECLARE(imx7ulp, "fsl,imx7ulp-clock", imx7ulp_clocks_init);
> >
>
> Any reason why it can't be a platform driver? If not, please add
> some comment explaining why.
>
Timer is using it at early stage. GIC seems not although standard
binding claim possible clock requirement.
Others still not sure.
What your suggestion?
Convert timer to platform driver and make clock as platform driver as well?
Regards
Dong Aisheng
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/20, Dong Aisheng wrote:
> On Mon, Jun 19, 2017 at 07:01:19PM -0700, Stephen Boyd wrote:
> >
> > Any reason why it can't be a platform driver? If not, please add
> > some comment explaining why.
> >
>
> Timer is using it at early stage. GIC seems not although standard
> binding claim possible clock requirement.
> Others still not sure.
>
> What your suggestion?
> Convert timer to platform driver and make clock as platform driver as well?
>
The timer can't be a platform driver because it would be too
late. The clock driver could register whatever clks are required
for the timer/GIC in a CLK_OF_DECLARE_DRIVER hook, and then leave
the rest to a platform driver. This way we get some of the device
driver framework in this code.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
> -----Original Message-----
> From: Stephen Boyd [mailto:[email protected]]
> Sent: Wednesday, June 21, 2017 4:42 AM
> To: Dong Aisheng
> Cc: A.s. Dong; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Anson Huang; Jacky Bai
> Subject: Re: [PATCH 9/9] clk: imx: add imx7ulp clk driver
>
> On 06/20, Dong Aisheng wrote:
> > On Mon, Jun 19, 2017 at 07:01:19PM -0700, Stephen Boyd wrote:
> > >
> > > Any reason why it can't be a platform driver? If not, please add
> > > some comment explaining why.
> > >
> >
> > Timer is using it at early stage. GIC seems not although standard
> > binding claim possible clock requirement.
> > Others still not sure.
> >
> > What your suggestion?
> > Convert timer to platform driver and make clock as platform driver as
> well?
> >
>
> The timer can't be a platform driver because it would be too late. The
> clock driver could register whatever clks are required for the timer/GIC
> in a CLK_OF_DECLARE_DRIVER hook, and then leave the rest to a platform
> driver. This way we get some of the device driver framework in this code.
>
Okay, I could try it. Thanks.
One thing is that TPM clock has a lot parents and parents having parents,
as well as PIT timer. So I may need enable more than half clocks in
CLK_OF_DECLARE_DRIVER hook.
BTW, What's benefit to convert into two parts of probe?
I'm not quite if I already get it all, can you help clarify it?
Regards
Dong Aisheng
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project
On 06/21, A.s. Dong wrote:
> > -----Original Message-----
> > From: Stephen Boyd [mailto:[email protected]]
> > Sent: Wednesday, June 21, 2017 4:42 AM
> > To: Dong Aisheng
> > Cc: A.s. Dong; [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; Anson Huang; Jacky Bai
> > Subject: Re: [PATCH 9/9] clk: imx: add imx7ulp clk driver
> >
> > On 06/20, Dong Aisheng wrote:
> > > On Mon, Jun 19, 2017 at 07:01:19PM -0700, Stephen Boyd wrote:
> > > >
> > > > Any reason why it can't be a platform driver? If not, please add
> > > > some comment explaining why.
> > > >
> > >
> > > Timer is using it at early stage. GIC seems not although standard
> > > binding claim possible clock requirement.
> > > Others still not sure.
> > >
> > > What your suggestion?
> > > Convert timer to platform driver and make clock as platform driver as
> > well?
> > >
> >
> > The timer can't be a platform driver because it would be too late. The
> > clock driver could register whatever clks are required for the timer/GIC
> > in a CLK_OF_DECLARE_DRIVER hook, and then leave the rest to a platform
> > driver. This way we get some of the device driver framework in this code.
> >
>
> Okay, I could try it. Thanks.
>
> One thing is that TPM clock has a lot parents and parents having parents,
> as well as PIT timer. So I may need enable more than half clocks in
> CLK_OF_DECLARE_DRIVER hook.
That's fine.
>
> BTW, What's benefit to convert into two parts of probe?
> I'm not quite if I already get it all, can you help clarify it?
>
The benefit is that we still get a platform driver and we can
associate a device pointer with the clock controller eventually.
Here's a reply I sent yesterday on the same topic:
Reasons (in no particular order):
1. We get a dev pointer to use with clk_hw_register()
2. We can handle probe defer if some resource is not available
3. Using device model gets us a hook into power management frameworks
like runtime PM and system PM for things like suspend and hibernate
4. It encourages a single DT node clk controller style binding
instead of a single node per clk style binding
5. We can use non-DT specific functions like devm_ioremap_resource() to map
registers and acquire other resources, leading to more portable and
generic code
6. We may be able to make the device driver a module, which will
make distros happy if we don't have to compile in all
these clk drivers to the resulting vmlinux (this one doesn't
apply here)
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
> -----Original Message-----
> From: Stephen Boyd [mailto:[email protected]]
> Sent: Saturday, July 01, 2017 8:35 AM
> To: A.s. Dong
> Cc: Dong Aisheng; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Anson Huang; Jacky Bai
> Subject: Re: [PATCH 9/9] clk: imx: add imx7ulp clk driver
>
> On 06/21, A.s. Dong wrote:
> > > -----Original Message-----
> > > From: Stephen Boyd [mailto:[email protected]]
> > > Sent: Wednesday, June 21, 2017 4:42 AM
> > > To: Dong Aisheng
> > > Cc: A.s. Dong; [email protected];
> > > [email protected]; [email protected];
> > > [email protected]; [email protected]; Anson Huang; Jacky Bai
> > > Subject: Re: [PATCH 9/9] clk: imx: add imx7ulp clk driver
> > >
> > > On 06/20, Dong Aisheng wrote:
> > > > On Mon, Jun 19, 2017 at 07:01:19PM -0700, Stephen Boyd wrote:
> > > > >
> > > > > Any reason why it can't be a platform driver? If not, please add
> > > > > some comment explaining why.
> > > > >
> > > >
> > > > Timer is using it at early stage. GIC seems not although standard
> > > > binding claim possible clock requirement.
> > > > Others still not sure.
> > > >
> > > > What your suggestion?
> > > > Convert timer to platform driver and make clock as platform driver
> > > > as
> > > well?
> > > >
> > >
> > > The timer can't be a platform driver because it would be too late.
> > > The clock driver could register whatever clks are required for the
> > > timer/GIC in a CLK_OF_DECLARE_DRIVER hook, and then leave the rest
> > > to a platform driver. This way we get some of the device driver
> framework in this code.
> > >
> >
> > Okay, I could try it. Thanks.
> >
> > One thing is that TPM clock has a lot parents and parents having
> > parents, as well as PIT timer. So I may need enable more than half
> > clocks in CLK_OF_DECLARE_DRIVER hook.
>
> That's fine.
>
> >
> > BTW, What's benefit to convert into two parts of probe?
> > I'm not quite if I already get it all, can you help clarify it?
> >
>
> The benefit is that we still get a platform driver and we can associate a
> device pointer with the clock controller eventually.
> Here's a reply I sent yesterday on the same topic:
>
> Reasons (in no particular order):
>
> 1. We get a dev pointer to use with clk_hw_register()
>
> 2. We can handle probe defer if some resource is not available
>
> 3. Using device model gets us a hook into power management frameworks
> like runtime PM and system PM for things like suspend and hibernate
>
> 4. It encourages a single DT node clk controller style binding
> instead of a single node per clk style binding
>
> 5. We can use non-DT specific functions like devm_ioremap_resource() to
> map
> registers and acquire other resources, leading to more portable and
> generic code
>
> 6. We may be able to make the device driver a module, which will
> make distros happy if we don't have to compile in all
> these clk drivers to the resulting vmlinux (this one doesn't
> apply here)
>
Very clear.
Thanks for the great explanation.
Regards
Dong Aisheng
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project