Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1476250

Re: [PATCH 4/6] clk: sunxi-ng: Add A33 CCU support

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

Hi Chen-Yu,

On Mon, Sep 05, 2016 at 02:34:21PM +0800, Chen-Yu Tsai wrote:
> > +static SUNXI_CCU_NKMP_WITH_GATE_LOCK(pll_cpux_clk, "pll-cpux",
> > +                                    "osc24M", 0x000,
> > +                                    8, 5,              /* N */
> > +                                    4, 2,              /* K */
> > +                                    0, 2,              /* M */
> > +                                    16, 2,             /* P */
> 
> Manual says there's no divider for P = 0x3.
> You'll need a table here.

Hmmm, Indeed. Or rather, a max value. We discussed that in the past, I
guess it's time to introduce it.

> > +                                    BIT(31),           /* gate */
> > +                                    BIT(28),           /* lock */
> > +                                    0);
> > +
> > +/*
> > + * The Audio PLL is supposed to have 4 outputs: 3 fixed factors from
> > + * the base (2x, 4x and 8x), and one variable divider (the one true
> > + * pll audio).
> > + *
> > + * We don't have any need for the variable divider for now, so we just
> > + * hardcode it to match with the clock names
> > + */
> > +#define SUN8I_A33_PLL_AUDIO_REG        0x008
> > +
> > +static SUNXI_CCU_NM_WITH_GATE_LOCK(pll_audio_base_clk, "pll-audio-base",
> > +                                  "osc24M", 0x008,
> > +                                  8, 7,                /* N */
> > +                                  0, 5,                /* M */
> > +                                  BIT(31),             /* gate */
> > +                                  BIT(28),             /* lock */
> > +                                  CLK_SET_RATE_UNGATE);
> 
> Are you sure about this? I suspect CLK_SET_RATE_GATE (clk must be
> gated before set rate) is what you want, given the non-DVFS
> nature of the PLL. Same for the other PLLs.

Yes, I'm sure about it. The lock, on the A33 at least, this has to be
confirmed on other SoCs, won't work if the clock is gated.

So, every time we call clk_set_rate, we will hit the WARN_ON because
of the lock timeout unless the clock runs.

> > +static SUNXI_CCU_GATE(bus_mipi_dsi_clk,        "bus-mipi-dsi", "ahb1",
> > +                     0x060, BIT(1), 0);
> 
> Nit: we know which bus these peripherals are on.
> Can we be more explicit with the names?

I wanted to be consistent with the datasheet, and would really prefer
to stick to it.

> > +static SUNXI_CCU_GATE(bus_ce_clk,      "bus-ce",       "ahb1",
> > +                     0x060, BIT(5), 0);
> 
> Nit: manual says Security System.
> 
> (Maybe I should change the name on sun6i to say Crypto Engine
>  if that's what we're going with?)

But I can't really use reject that argument there then :)

The rationale was that it's called CE in the H3 which was already
merged, but I'll change it.

> [...]
> 
> > +static SUNXI_CCU_GATE(usb_hsic_12M_clk,        "usb-hsic-12M", "osc24M",
> > +                     0x0cc, BIT(11), 0);
> 
> A TODO note saying we should have a fixed-factor-gate for this
> would be nice.

Hmmm? I'm not sure what you're saying.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/6] clk: sunxi-ng: Introduce support for A23 and A33 CCUs Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-09-01 16:20 +0200
  [PATCH 3/6] clk: sunxi-ng: Add N-class clocks support Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-09-01 16:20 +0200
    Re: [PATCH 3/6] clk: sunxi-ng: Add N-class clocks support Chen-Yu Tsai <wens@csie.org> - 2016-09-05 05:40 +0200
  [PATCH 1/6] clk: sunxi-ng: div: Add mux table macros Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-09-01 16:20 +0200
    Re: [PATCH 1/6] clk: sunxi-ng: div: Add mux table macros Chen-Yu Tsai <wens@csie.org> - 2016-09-02 09:30 +0200
  [PATCH 5/6] clk: sunxi-ng: Add A23 CCU Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-09-01 16:20 +0200
  [PATCH 4/6] clk: sunxi-ng: Add A33 CCU support Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-09-01 16:20 +0200
    Re: [PATCH 4/6] clk: sunxi-ng: Add A33 CCU support Chen-Yu Tsai <wens@csie.org> - 2016-09-05 08:40 +0200
      Re: [PATCH 4/6] clk: sunxi-ng: Add A33 CCU support Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-09-05 12:00 +0200
        Re: [PATCH 4/6] clk: sunxi-ng: Add A33 CCU support Chen-Yu Tsai <wens@csie.org> - 2016-09-05 12:30 +0200
  [PATCH 6/6] ARM: sun8i: Convert the A23 and A33 to the CCU Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-09-01 16:20 +0200

csiph-web