Re: [PATCH v2 1/4] clk: tegra: Move mipi-cal initialization into clk-tegra1[12]4.c

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2982 bytes --]

On Tue, Oct 07, 2014 at 01:07:29PM -0700, Sean Paul wrote:
> On Wed, Oct 1, 2014 at 11:38 AM, Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > On Fri, Sep 19, 2014 at 7:28 AM, Peter De Schrijver
> > <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >> On Thu, Sep 18, 2014 at 08:59:53PM +0200, Sean Paul wrote:
> >>> On Thu, Sep 18, 2014 at 7:26 AM, Peter De Schrijver
> >>> <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >>> > On Wed, Sep 10, 2014 at 06:17:44PM +0200, Stephen Warren wrote:
> >>> >> On 09/10/2014 08:52 AM, Sean Paul wrote:
> >>> >> > This patch moves the mipi-cal gate registration down into the SoC
> >>> >> > specific files to reflect the different in parent between them.
> >>> >> >
> >>> >> > Without this change, MIPI calibration will fail on K1 devices if
> >>> >> > the 72MHz clock is off.
> >>> >>
> >>> >> This isn't a problem with this patch per se, but I notice that what's
> >>> >> removed from clk-tegra-periph.c is data in a table, whereas open-coded
> >>> >> calls are added to clk-tegra*.c. It'd be nice if the open-coded calls in
> >>> >> clk-tegra*.c could be converted to a table (simply with SoC-specific
> >>> >> data) and processed by the same function. Peter, would that make sense?
> >>> >
> >>> > Maybe yes. I will look into this. For this patch, it would make more sense
> >>> > to keep the entry in clk-tegra-periph.c as it can be used by both Tegra124
> >>> > and Tegra132.
> >>> >
> >>>
> >>> How do you suggest we switch the parent based on 124 vs 132 if it
> >>> remains in clk-tegra-periph.c?
> >>>
> >>
> >> I would change the parent of mipi-cal in clk-tegra-periph.c to clk72mhz and
> >> use a different mechanism for Tegra114.
> >>
> >
> > Hi Peter,
> > Sorry for the delay, I'm just getting back to this now.
> >
> > I'm still unclear on how you propose we alter mipi-cal in tegra114.
> > AFAICT, we can't re-parent it since it's a gate clock. Can you please
> > be a little more specific (and forgive my ignorance wrt this driver,
> > it's still new to me)?
> >
> 
> Ping. Can I get some advice on this?

I don't see a way how we can make this work with a table that wouldn't
involve adding a lot more code. Given that we need to separate the table
based on SoC generation we need to add code to both Tegra114 and
Tegra124 to register a table of generation-specific clocks.

We do something like that already for Tegra114 using the
tegra_periph_clk_list table, but initializing the clocks is done using
an open-coded loop. Perhaps one possibility would be to implement a
generic function that takes a table of tegra_periph_init_data structs
and a count. Then we can add per-generation clocks to such a table and
only need to modify the code once.

But again, this would be adding much more code than this patch does, so
perhaps we can address it when we start needing more generation-specific
clocks.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]