On 01/10, Jacky Bai wrote:
>
> >
> > > +#include <linux/types.h>
> > > +
> > > +#include "clk.h"
> > > +
> > > +#define CCM_ANALOG_PLL_BYPASS (0x1 << 16)
> > > +#define BM_CCM_CCDR_MMDC_CH0_MASK (0x2 << 16)
> > > +#define CCDR 0x4
> > > +#define xPLL_CLR(offset) (offset + 0x8)
> > > +
> > > +static const char *pll_bypass_src_sels[] = { "osc", "dummy", };
> >
> > All these should be const char * const unless something is wrong.
>
> If changed to 'const char * const', it vill has argument type mismatch error, as imx_clk_* wrapper function
> has argument type 'const char *'.
Hmm that's unfortunate.
>
> >
> > > +static const char *pll1_bypass_sels[] = { "pll1", "pll1_bypass_src",
> > > +}; static const char *pll2_bypass_sels[] = { "pll2",
> > > +"pll2_bypass_src", }; static const char *pll3_bypass_sels[] = {
> > > +"pll3", "pll3_bypass_src", }; static const char *pll4_bypass_sels[] =
> > > +{ "pll4", "pll4_bypass_src", }; static const char *pll5_bypass_sels[]
> > > += { "pll5", "pll5_bypass_src", }; static const char
> > > +*pll6_bypass_sels[] = { "pll6", "pll6_bypass_src", };
> > [...]
> > > + clks[IMX6SLL_CLK_USDHC3] = imx_clk_gate2("usdhc3",
> > "usdhc3_podf", base + 0x80, 6);
> > > +
> > > + /* mask handshake of mmdc */
> > > + writel_relaxed(BM_CCM_CCDR_MMDC_CH0_MASK, base + CCDR);
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(clks); i++)
> > > + if (IS_ERR(clks[i]))
> > > + pr_err("i.MX6SLL clk %d: register failed with %ld\n", i,
> > > +PTR_ERR(clks[i]));
> > > +
> > > + clk_data.clks = clks;
> > > + clk_data.clk_num = ARRAY_SIZE(clks);
> > > + of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
> > > +
> > > + /* set perclk to from OSC */
> > > + clk_set_parent(clks[IMX6SLL_CLK_PERCLK_SEL],
> > clks[IMX6SLL_CLK_OSC]);
> >
> > Can this be done with assigned-clocks in DT?
>
> Ok, I will move it to assigned-clocks in DT.
>
> >
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
> > > + clk_prepare_enable(clks[clks_init_on[i]]);
> >
> > Critical clocks?
>
> Yes, these clocks must be always on.
>
> >
> > > +
> > > + if (IS_ENABLED(CONFIG_USB_MXS_PHY)) {
> > > + clk_prepare_enable(clks[IMX6SLL_CLK_USBPHY1_GATE]);
> > > + clk_prepare_enable(clks[IMX6SLL_CLK_USBPHY2_GATE]);
> >
> > The phy driver can't enable these?
>
> The reason why we enable these two clks here is in below commit
> commit a5120e89e7e187a91852896f586876c7a2030804
> Author: Peter Chen <peter.chen@xxxxxxxxxxxxx>
> Date: Fri Jan 18 10:38:05 2013 +0800
> ARM i.MX6: change mxs usbphy clock usage
>
So can we mark these clks with CLK_IS_CRITICAL flag then instead?
Or are they disabled out of the bootloader?
> >
> > > + }
> > > +
> > > + /* Lower the AHB clock rate before changing the clock source. */
> > > + clk_set_rate(clks[IMX6SLL_CLK_AHB], 99000000);
> > > +
> > > + /* Change periph_pre clock to pll2_bus to adjust AXI rate to 264MHz */
> > > + clk_set_parent(clks[IMX6SLL_CLK_PERIPH_CLK2_SEL],
> > clks[IMX6SLL_CLK_PLL3_USB_OTG]);
> > > + clk_set_parent(clks[IMX6SLL_CLK_PERIPH],
> > clks[IMX6SLL_CLK_PERIPH_CLK2]);
> > > + clk_set_parent(clks[IMX6SLL_CLK_PERIPH_PRE],
> > clks[IMX6SLL_CLK_PLL2_BUS]);
> > > + clk_set_parent(clks[IMX6SLL_CLK_PERIPH],
> > > +clks[IMX6SLL_CLK_PERIPH_PRE]);
> > > +
> > > + clk_set_rate(clks[IMX6SLL_CLK_AHB], 132000000);
> >
> > assigned-clocks for rates now? Or perhaps we shouldn't be exposing these as
> > clks if they have some sort of complicated rate sequence switch that we can't
> > guarantee with the clk_ops we have today.
>
> These clks will be used by some peripherals, so we need to expose these clocks.
> And the above parent and rate swith sequence is not very easy to be handled in assigned-clocks,
> So we leave it in this place.
>
How do we guarantee that the rate switch doesn't happen later on,
requiring this coordinated sequence of clk operations?
--
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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
