All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Walker <dwalker@fifo99.com>
To: Stephen Boyd <sboyd@codeaurora.org>
Cc: David Brown <davidb@codeaurora.org>,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Saravana Kannan <skannan@codeaurora.org>
Subject: Re: [PATCH 2/3] msm: Peripheral Image Loader (PIL) driver
Date: Wed, 16 Mar 2011 03:20:19 -0700	[thread overview]
Message-ID: <1300270820.13755.14.camel@desktop> (raw)
In-Reply-To: <1299732274-10742-3-git-send-email-sboyd@codeaurora.org>

On Wed, 2011-03-09 at 20:44 -0800, Stephen Boyd wrote:
> On 8660, the modem, dsp, and sensors peripherals require their
> firmware to be loaded into memory before they can be properly
> taken out of reset.
> 
> Drivers are expected to call pil_get() when they wish to load a
> peripheral. This will initiate multiple firmware_request()s for
> the metadata and image blobs for a peripheral. Once the image has
> been loaded into memory, it is validated and brought out of reset
> via the peripheral reset driver.

Why can't this be part of the generic firmware request API ?

> Change-Id: I041139464bbd3b646b82370ab540f40b0ac9af6b

Can't have Change-Id's ..

> Reviewed-by: Saravana Kannan <skannan@codeaurora.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  arch/arm/mach-msm/Kconfig                          |   13 +
>  arch/arm/mach-msm/Makefile                         |    2 +
>  arch/arm/mach-msm/include/mach/peripheral-loader.h |   23 +
>  arch/arm/mach-msm/peripheral-loader.c              |  402
> +++++++++++++++
>  arch/arm/mach-msm/peripheral-loader.h              |   38 ++
>  arch/arm/mach-msm/peripheral-reset.c               |  528
> ++++++++++++++++++++
>  6 files changed, 1006 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-msm/include/mach/peripheral-loader.h
>  create mode 100644 arch/arm/mach-msm/peripheral-loader.c
>  create mode 100644 arch/arm/mach-msm/peripheral-loader.h
>  create mode 100644 arch/arm/mach-msm/peripheral-reset.c
> 
> diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
> index 997c5bd..25b73b0 100644
> --- a/arch/arm/mach-msm/Kconfig
> +++ b/arch/arm/mach-msm/Kconfig
> @@ -210,4 +210,17 @@ config IOMMU_API
>  
>  config MSM_SCM
>         bool
> +
> +config MSM_PIL
> +       bool "Peripheral image loading (PIL)"
> +       select FW_LOADER
> +       select MSM_SCM
> +       depends on ARCH_MSM8X60
> +       help
> +         Some peripherals need to be loaded into memory before they
> can be
> +         brought out of reset.
> +
> +         Say yes to support these devices.
> +
> +

You shouldn't be adding anything like this to the Kconfig. To me if you
add stuff like this it's a big red flag.

I didn't review the rest sign it might be wasted effort on my part..

Daniel



WARNING: multiple messages have this Message-ID (diff)
From: dwalker@fifo99.com (Daniel Walker)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/3] msm: Peripheral Image Loader (PIL) driver
Date: Wed, 16 Mar 2011 03:20:19 -0700	[thread overview]
Message-ID: <1300270820.13755.14.camel@desktop> (raw)
In-Reply-To: <1299732274-10742-3-git-send-email-sboyd@codeaurora.org>

On Wed, 2011-03-09 at 20:44 -0800, Stephen Boyd wrote:
> On 8660, the modem, dsp, and sensors peripherals require their
> firmware to be loaded into memory before they can be properly
> taken out of reset.
> 
> Drivers are expected to call pil_get() when they wish to load a
> peripheral. This will initiate multiple firmware_request()s for
> the metadata and image blobs for a peripheral. Once the image has
> been loaded into memory, it is validated and brought out of reset
> via the peripheral reset driver.

Why can't this be part of the generic firmware request API ?

> Change-Id: I041139464bbd3b646b82370ab540f40b0ac9af6b

Can't have Change-Id's ..

> Reviewed-by: Saravana Kannan <skannan@codeaurora.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  arch/arm/mach-msm/Kconfig                          |   13 +
>  arch/arm/mach-msm/Makefile                         |    2 +
>  arch/arm/mach-msm/include/mach/peripheral-loader.h |   23 +
>  arch/arm/mach-msm/peripheral-loader.c              |  402
> +++++++++++++++
>  arch/arm/mach-msm/peripheral-loader.h              |   38 ++
>  arch/arm/mach-msm/peripheral-reset.c               |  528
> ++++++++++++++++++++
>  6 files changed, 1006 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-msm/include/mach/peripheral-loader.h
>  create mode 100644 arch/arm/mach-msm/peripheral-loader.c
>  create mode 100644 arch/arm/mach-msm/peripheral-loader.h
>  create mode 100644 arch/arm/mach-msm/peripheral-reset.c
> 
> diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
> index 997c5bd..25b73b0 100644
> --- a/arch/arm/mach-msm/Kconfig
> +++ b/arch/arm/mach-msm/Kconfig
> @@ -210,4 +210,17 @@ config IOMMU_API
>  
>  config MSM_SCM
>         bool
> +
> +config MSM_PIL
> +       bool "Peripheral image loading (PIL)"
> +       select FW_LOADER
> +       select MSM_SCM
> +       depends on ARCH_MSM8X60
> +       help
> +         Some peripherals need to be loaded into memory before they
> can be
> +         brought out of reset.
> +
> +         Say yes to support these devices.
> +
> +

You shouldn't be adding anything like this to the Kconfig. To me if you
add stuff like this it's a big red flag.

I didn't review the rest sign it might be wasted effort on my part..

Daniel

  reply	other threads:[~2011-03-16 10:20 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-10  4:44 [PATCH 0/3] Peripheral image loader (PIL) Stephen Boyd
2011-03-10  4:44 ` Stephen Boyd
2011-03-10  4:44 ` [PATCH 1/3] msm: 8660: Add clock control regions to iomap Stephen Boyd
2011-03-10  4:44   ` Stephen Boyd
2011-03-10 15:40   ` Sergei Shtylyov
2011-03-10 15:40     ` Sergei Shtylyov
2011-03-10 20:12     ` David Brown
2011-03-10 20:12       ` David Brown
2011-03-10 20:19     ` Stephen Boyd
2011-03-10 20:19       ` Stephen Boyd
2011-03-10  4:44 ` [PATCH 2/3] msm: Peripheral Image Loader (PIL) driver Stephen Boyd
2011-03-10  4:44   ` Stephen Boyd
2011-03-16 10:20   ` Daniel Walker [this message]
2011-03-16 10:20     ` Daniel Walker
2011-03-16 18:40     ` David Brown
2011-03-16 18:40       ` David Brown
2011-03-16 18:48       ` Saravana Kannan
2011-03-16 18:48         ` Saravana Kannan
2011-03-23 12:27         ` Daniel Walker
2011-03-23 12:27           ` Daniel Walker
2011-03-23 12:37       ` Daniel Walker
2011-03-23 12:37         ` Daniel Walker
2011-03-16 21:12     ` Bryan Huntsman
2011-03-16 21:12       ` Bryan Huntsman
2011-03-16 21:12       ` Bryan Huntsman
2011-03-10  4:44 ` [PATCH 3/3] msm: documentation: Add pil documentation Stephen Boyd
2011-03-10  4:44   ` Stephen Boyd

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1300270820.13755.14.camel@desktop \
    --to=dwalker@fifo99.com \
    --cc=davidb@codeaurora.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sboyd@codeaurora.org \
    --cc=skannan@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.