[PATCH v8 3/7] ARM: davinci: Add support for configuringDA8XX_REMOTEPROC

cstsai cstsai at itri.org.tw
Mon Mar 11 21:49:51 EDT 2013


Dear Sir,

Please don't send update emails to me

Whenever I need I can check from website

Thanks & Regards,
cstsai

----- Original Message ----- 
From: "Tivy, Robert" <rtivy at ti.com>
To: "Nori, Sekhar" <nsekhar at ti.com>
Cc: <ohad at wizery.com>; <davinci-linux-open-source at linux.davincidsp.com>; 
<stable at vger.kernel.org>; <linux-arm-kernel at lists.infradead.org>
Sent: Tuesday, March 12, 2013 4:26 AM
Subject: RE: [PATCH v8 3/7] ARM: davinci: Add support for 
configuringDA8XX_REMOTEPROC


> -----Original Message-----
> From: Nori, Sekhar
> Sent: Monday, March 11, 2013 4:49 AM
>
> On 3/9/2013 4:11 AM, Robert Tivy wrote:
> > Also fix REMOTEPROC config to select FW_LOADER (instead of
> FW_CONFIG).
>
> It is preferable to have the patch description make sense standalone
> even when read without the subject line.

Will fix.

>
> >
> > Signed-off-by: Robert Tivy <rtivy at ti.com>
>
> Please follow the subject line conventions of the subsystem the patch
> is
> touching. This is not an ARM patch so the "ARM: davinci: " prefix in
> subject is surely wrong.

So even though it is a driver for just DA8XX, it should not get the "ARM: 
davinci:" prefix?

Is that mandated by the fact that it's in a general "driver" subdirectory?

>
> And I think I mentioned this once before but we want to see
> Kconfig/Makefile changes along with driver addition in the same patch.

Yeah, I was confusing Ohad's feedback about the "fix" to Kconfig being in a 
separate patch.  I will therefore put the "fix" (FW_CONFIG -> FW_LOADER) 
into its own patch, and group the driver's Kconfig additions with the driver 
and its Makefile.

>
> > ---
> >  drivers/remoteproc/Kconfig |   26 +++++++++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> > index 96ce101..21d04f1 100644
> > --- a/drivers/remoteproc/Kconfig
> > +++ b/drivers/remoteproc/Kconfig
> > @@ -5,7 +5,7 @@ config REMOTEPROC
> >  tristate
> >  depends on EXPERIMENTAL
> >  depends on HAS_DMA
> > - select FW_CONFIG
> > + select FW_LOADER
> >  select VIRTIO
> >
> >  config OMAP_REMOTEPROC
> > @@ -41,4 +41,28 @@ config STE_MODEM_RPROC
> >    This can be either built-in or a loadable module.
> >    If unsure say N.
> >
> > +config DA8XX_REMOTEPROC
> > + tristate "DA830/OMAPL137 & DA850/OMAPL138 remoteproc support
> (EXPERIMENTAL)"
> > + depends on ARCH_DAVINCI_DA8XX
> > + select REMOTEPROC
> > + select RPMSG
> > + select CMA
>
> Its nice to keep the selects sorted. It helps avoid duplicates.

Will fix.

>
> > + default n
>
> No need of this I think, it should default to no already.

OK.

>
> > + help
> > +   Say y here to support DA830/OMAPL137 & DA850/OMAPL138 remote
> > +   processors via the remote processor framework.
> > +
> > +   You want to say y here in order to enable AMP
> > +   use-cases to run on your platform (multimedia codecs are
> > +   offloaded to remote DSP processors using this framework).
> > +
> > +   This module controls the name of the firmware file that gets
> > +   loaded on the DSP.  This file must reside in the /lib/firmware
> > +   directory.  It can be specified via the module parameter
> > +   da8xx_fw_name=<filename>, and if not specified will default to
> > +   "rproc-dsp-fw".
>
> I hope running modinfo gives these details as well.

It will in the next patch :)

>
> > +
> > +   It's safe to say n here if you're not interested in multimedia
> > +   offloading or just want a bare minimum kernel.
>
> I think this is misleading a bit since just selecting 'n' here will not
> result in a "bare minimum kernel" - better to just avoid mention of it.

Agreed, will drop.

>
> Thanks,
> Sekhar

Thanks & Regards,

- Rob

_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source at linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source 

====================================================================
This email may contain confidential information. Please do not use or disclose it in any way and delete it if you are not the intended recipient.




More information about the linux-arm-kernel mailing list