* [PATCH v8 3/7] ARM: davinci: Add support for configuring DA8XX_REMOTEPROC
[not found] ` <1362782508-3166-4-git-send-email-rtivy@ti.com>
@ 2013-03-09 9:26 ` Sergei Shtylyov
2013-03-11 11:48 ` Sekhar Nori
1 sibling, 0 replies; 5+ messages in thread
From: Sergei Shtylyov @ 2013-03-09 9:26 UTC (permalink / raw)
To: linux-arm-kernel
Hello.
On 09-03-2013 2:41, Robert Tivy wrote:
> Also fix REMOTEPROC config to select FW_LOADER (instead of FW_CONFIG).
> Signed-off-by: Robert Tivy <rtivy@ti.com>
> ---
> 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
[...]
> @@ -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
> + default n
> + help
> + Say y here to support DA830/OMAPL137 & DA850/OMAPL138 remote
Naming nit: they're OMAP-L13x. Here and above.
WBR, Sergei
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v8 3/7] ARM: davinci: Add support for configuring DA8XX_REMOTEPROC
[not found] ` <1362782508-3166-4-git-send-email-rtivy@ti.com>
2013-03-09 9:26 ` [PATCH v8 3/7] ARM: davinci: Add support for configuring DA8XX_REMOTEPROC Sergei Shtylyov
@ 2013-03-11 11:48 ` Sekhar Nori
2013-03-11 15:11 ` Paul Bolle
2013-03-11 20:26 ` Tivy, Robert
1 sibling, 2 replies; 5+ messages in thread
From: Sekhar Nori @ 2013-03-11 11:48 UTC (permalink / raw)
To: linux-arm-kernel
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.
>
> Signed-off-by: Robert Tivy <rtivy@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.
And I think I mentioned this once before but we want to see
Kconfig/Makefile changes along with driver addition in the same patch.
> ---
> 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.
> + default n
No need of this I think, it should default to no already.
> + 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'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.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v8 3/7] ARM: davinci: Add support for configuring DA8XX_REMOTEPROC
2013-03-11 11:48 ` Sekhar Nori
@ 2013-03-11 15:11 ` Paul Bolle
2013-03-11 20:26 ` Tivy, Robert
1 sibling, 0 replies; 5+ messages in thread
From: Paul Bolle @ 2013-03-11 15:11 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 2013-03-11 at 17:18 +0530, Sekhar Nori wrote:
> On 3/9/2013 4:11 AM, Robert Tivy wrote:
> > 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
This fixes the typo introduced in the typo introduced in (one line)
commit e121aefa7d9f10eee5cf26ed47129237a05d940b ("remoteproc: fix
missing CONFIG_FW_LOADER configurations"). That seems a separate issue.
Can't it be submitted as a separate (one line) patch?
That would probably make it easier to get it into stable. Ohad has
already stated that this would be a good idea.
Paul Bolle
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v8 3/7] ARM: davinci: Add support for configuring DA8XX_REMOTEPROC
2013-03-11 11:48 ` Sekhar Nori
2013-03-11 15:11 ` Paul Bolle
@ 2013-03-11 20:26 ` Tivy, Robert
2013-03-12 1:49 ` [PATCH v8 3/7] ARM: davinci: Add support for configuringDA8XX_REMOTEPROC cstsai
1 sibling, 1 reply; 5+ messages in thread
From: Tivy, Robert @ 2013-03-11 20:26 UTC (permalink / raw)
To: linux-arm-kernel
> -----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@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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v8 3/7] ARM: davinci: Add support for configuringDA8XX_REMOTEPROC
2013-03-11 20:26 ` Tivy, Robert
@ 2013-03-12 1:49 ` cstsai
0 siblings, 0 replies; 5+ messages in thread
From: cstsai @ 2013-03-12 1:49 UTC (permalink / raw)
To: linux-arm-kernel
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@ti.com>
To: "Nori, Sekhar" <nsekhar@ti.com>
Cc: <ohad@wizery.com>; <davinci-linux-open-source@linux.davincidsp.com>;
<stable@vger.kernel.org>; <linux-arm-kernel@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@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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-03-12 1:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1362782508-3166-1-git-send-email-rtivy@ti.com>
[not found] ` <1362782508-3166-4-git-send-email-rtivy@ti.com>
2013-03-09 9:26 ` [PATCH v8 3/7] ARM: davinci: Add support for configuring DA8XX_REMOTEPROC Sergei Shtylyov
2013-03-11 11:48 ` Sekhar Nori
2013-03-11 15:11 ` Paul Bolle
2013-03-11 20:26 ` Tivy, Robert
2013-03-12 1:49 ` [PATCH v8 3/7] ARM: davinci: Add support for configuringDA8XX_REMOTEPROC cstsai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.