> -----Original Message----- > From: Chemparathy, Cyril > Sent: Monday, December 03, 2012 12:13 PM > To: Nori, Sekhar > Cc: Tivy, Robert; [email protected]; > [email protected] > Subject: Re: [PATCH v3 5/6] ARM: davinci: remoteproc board support for > OMAP-L138 DSP > > On 12/03/2012 09:41 AM, Sekhar Nori wrote: > > Hi Rob, > > > > On 12/1/2012 7:41 AM, Tivy, Robert wrote: > >> Hi Sekhar, > >> > >>> -----Original Message----- > >>> From: Nori, Sekhar > >>> Sent: Friday, November 30, 2012 2:51 AM > >>> To: Tivy, Robert > >>> Cc: [email protected]; linux-arm- > >>> [email protected]; Ring, Chris; Grosen, Mark > >>> Subject: Re: [PATCH v3 5/6] ARM: davinci: remoteproc board support > >>> for > >>> OMAP-L138 DSP > >>> > >>> Hi Rob, > >>> > >>> On 11/29/2012 7:08 AM, Tivy, Robert wrote: > >>>> Hi Sekhar, > >>>> > >>>> Please see comments and noob questions below... > >>>> > >>>>> -----Original Message----- > >>>>> From: Nori, Sekhar > >>>>> Sent: Tuesday, November 20, 2012 4:27 AM > >>>>> To: Tivy, Robert > >>>>> Cc: [email protected]; linux-arm- > >>>>> [email protected]; Ring, Chris; Grosen, Mark > >>>>> Subject: Re: [PATCH v3 5/6] ARM: davinci: remoteproc board > support > >>>>> for > >>>>> OMAP-L138 DSP > >>>>> > >>>>> On 11/14/2012 6:03 AM, Robert Tivy wrote: > >>>>>> Requires CMA services. > >>>>>> > >>>>>> New 'clk_reset()' API added so that the DSP's reset state can be > >>>>>> toggled separately from its enable/disable operation. > >>>>> > >>>>> But we cannot add a private clk_ API without it being defined in > >>>>> include/linux/clk.h. So, this requires wider alignment. > >>>>> > >>>>> And I am not sure clock API should be extended to support reset > >>> since > >>>>> "resetting a clock" does not make a lot of sense. On DaVinci, > >>>>> clock gating and reset are handled by the same module, but that > >>>>> need not always be the case. > >>>>> > >>>>> What you need is a way to reset an IP. I do not know of an > >>>>> existing framework to do this; likely a new API needs to be > >>>>> created. I am guessing this never existed so far because most of > >>>>> the IPs can be reset internally (by writing a bit within its own > >>>>> register space). I guess DSP is different since its actually a > >>>>> co-processor and may not have such a bit. > >>>>> > >>>>> How about simulating a reset by making the DSP jump to its reset > >>>>> address. While I am sure its not quite the same as resetting the > >>>>> DSP using PSC, may be it could be used while you wait for > >>>>> consensus around handling module reset in kernel? > >>>> > >>>> Since the ARM can't write the DSP's program counter I suspect such > >>>> a > >>> temporary solution is not possible. > >>> > >>> Okay. I think the way forward on this is to start a separate thread > >>> including the ARM list, LKML and the remoteproc folks to see if it > >>> makes sense to create a reset API in kernel. You can describe the > >>> usecase you have. > >> > >> Instead of fighting that fight, I thought of another way. The > da8xx_dsp platform_device has private data registered in its device > struct. This private data can contain a function pointer for a "DSP > reset" function, and davinci_remoteproc.c can call the registered > function. Does that sound OK? > > > > Passing function pointers from platform code will not work when > > converting to device tree (DT). DA850 will gain DT boot with v3.8 and > > there is work ongoing on converting drivers to be DT-aware. Adding a > > new driver which is DT-incompatible will not be right. Hence, I will > > not recommend this now. > > > > Not sure if this solves your problem, but you could add a new clock > type > (PSC_LRESET?) as a child under the PSC clock node for the DSP. > Something like: > > | > +-- PSC x (DSP0 clock) > | | > | +-- PSC-LRESET x (DSP0 reset control) > | > +-- PSC y (DSP1 clock) > | | > | +-- PSC-LRESET y (DSP1 reset control) > | > +-- PSC z (DSP2 clock) > | | > | +-- PSC-LRESET z (DSP2 reset control) > > > The idea here being that if you enable the PSC clocks, you enable the > clock gates, but leave the DSPs in reset. On the other hand, if you > enable the reset clock, the code implicitly enables the gate (via > parent), and takes the DSP out of reset as well. > > This is not quite the cleanest way to do it, considering that reset > lines have no business in the clock tree, but then, this is probably > the simplest way to fit in. Thoughts? > > BTW, we have the same situation on Keystone. > > Thanks > -- Cyril.
Cyril, Thankyou for the good suggestion. In trying it out to see if it would work I ran into an issue where the clk_register() function failed when called for the new child clock, complaining that its parent has no rate. This is true, since the parent was previously just a leaf and handled by the PSC (and the parent's parent is a CLK_PLL clock). So, I would need to add code to arch/arm/mach-davinci/clock.c that prevents rate operations from being performed on a clock of this PSC_LRESET type (as well as code that allows for no "rate" or "parent->rate" to be associated with it, instead of failing). I can modify clock.c to allow for this new clock type in this way, unless you have a better suggestion (I'm not too thrilled about changing clock.c, seems I should be able to live within its framework). Regards, - Rob _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
