All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Cousson, Benoit" <b-cousson@ti.com>
To: Paul Walmsley <paul@pwsan.com>
Cc: linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	"Péter Ujfalusi" <peter.ujfalusi@ti.com>
Subject: Re: [PATCHv2 12/12] ARM: OMAP4: hwmod data: do not enable or reset the McPDM during kernel init
Date: Mon, 11 Jun 2012 11:54:23 +0200	[thread overview]
Message-ID: <4FD5C04F.2040509@ti.com> (raw)
In-Reply-To: <20120611004626.20034.42995.stgit@dusk>

On 6/11/2012 2:46 AM, Paul Walmsley wrote:
> Resolve this kernel boot message:
>
> omap_hwmod: mcpdm: cannot be enabled for reset (3)
>
> It appears that the McPDM on OMAP4 can only receive its functional
> clock from an off-chip source.  This source is not guaranteed to be
> present on the board, and when present, it is controlled by I2C.  This
> would introduce a board dependency to the early hwmod code which it
> was not designed to handle.  Also, neither the driver for this
> off-chip clock provider nor the I2C code is available early in boot
> when the hwmod code is attempting to enable and reset IP blocks.  This
> effectively makes it impossible to enable and reset this device during
> hwmod init.
>
> At its core, this patch is a workaround for an OMAP hardware problem.
> It should be possible to configure the OMAP to provide any IP block's
> functional clock from an on-chip source.  (This is true for almost
> every IP block on the chip.  As far as I know, McPDM is the only
> exception.)  If the kernel cannot reset and configure IP blocks, it
> cannot guarantee a sane SoC state.  Relying on an optional off-chip
> clock also creates a board dependency which is beyond the scope of the
> early hwmod code.

And I guess, that's a good reason as well to let the driver managing the 
reset during the probe for such IPs.

> This patch works around the issue by marking the McPDM hwmod record
> with the HWMOD_EXT_OPT_MAIN_CLK flag.  This prevents the hwmod
> code from touching the device early during boot.
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Péter Ujfalusi <peter.ujfalusi@ti.com>
> Cc: Benoît Cousson <b-cousson@ti.com>
> ---
>   arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> index 3613054..86fc513 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> @@ -2091,6 +2091,18 @@ static struct omap_hwmod omap44xx_mcpdm_hwmod = {
>   	.name		= "mcpdm",
>   	.class		= &omap44xx_mcpdm_hwmod_class,
>   	.clkdm_name	= "abe_clkdm",
> +	/*
> +	 * It's suspected that the McPDM requires an off-chip main
> +	 * functional clock, controlled via I2C.

Nit: Is it not suspected, it is a fact. The clock tree clearly indicate 
that the mcpdm fclk is generated from the pad_clks.
The IP is a custom link for the Audio IC control / data. using the Audio 
IC as a source clock is standard. Since that IP is done only for that 
purpose, there is no optional clock path from the PRCM like it is the 
case for the McASP / McBSP.

>  This IP block is
> +	 * currently reset very early during boot, before I2C is
> +	 * available, so it doesn't seem that we have any choice in
> +	 * the kernel other than to avoid resetting it.  XXX This is
> +	 * really a hardware issue workaround: every IP block should
> +	 * be able to source its main functional clock from either
> +	 * on-chip or off-chip sources.  McPDM seems to be the only
> +	 * current exception.

I do not think this is the right place to put some complaint about the 
HW :-).
I guess we should just describe the facts.

> +	 */
> +	.flags		= HWMOD_EXT_OPT_MAIN_CLK,

Could you please update the hints for this IP in the autogen scripts?
I added a comment attribute to add a custom comment on top of the flags 
entry.
The latest branch is "omap5430_for_3.6".

Thanks,
Benoit


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: b-cousson@ti.com (Cousson, Benoit)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2 12/12] ARM: OMAP4: hwmod data: do not enable or reset the McPDM during kernel init
Date: Mon, 11 Jun 2012 11:54:23 +0200	[thread overview]
Message-ID: <4FD5C04F.2040509@ti.com> (raw)
In-Reply-To: <20120611004626.20034.42995.stgit@dusk>

On 6/11/2012 2:46 AM, Paul Walmsley wrote:
> Resolve this kernel boot message:
>
> omap_hwmod: mcpdm: cannot be enabled for reset (3)
>
> It appears that the McPDM on OMAP4 can only receive its functional
> clock from an off-chip source.  This source is not guaranteed to be
> present on the board, and when present, it is controlled by I2C.  This
> would introduce a board dependency to the early hwmod code which it
> was not designed to handle.  Also, neither the driver for this
> off-chip clock provider nor the I2C code is available early in boot
> when the hwmod code is attempting to enable and reset IP blocks.  This
> effectively makes it impossible to enable and reset this device during
> hwmod init.
>
> At its core, this patch is a workaround for an OMAP hardware problem.
> It should be possible to configure the OMAP to provide any IP block's
> functional clock from an on-chip source.  (This is true for almost
> every IP block on the chip.  As far as I know, McPDM is the only
> exception.)  If the kernel cannot reset and configure IP blocks, it
> cannot guarantee a sane SoC state.  Relying on an optional off-chip
> clock also creates a board dependency which is beyond the scope of the
> early hwmod code.

And I guess, that's a good reason as well to let the driver managing the 
reset during the probe for such IPs.

> This patch works around the issue by marking the McPDM hwmod record
> with the HWMOD_EXT_OPT_MAIN_CLK flag.  This prevents the hwmod
> code from touching the device early during boot.
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: P?ter Ujfalusi <peter.ujfalusi@ti.com>
> Cc: Beno?t Cousson <b-cousson@ti.com>
> ---
>   arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> index 3613054..86fc513 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> @@ -2091,6 +2091,18 @@ static struct omap_hwmod omap44xx_mcpdm_hwmod = {
>   	.name		= "mcpdm",
>   	.class		= &omap44xx_mcpdm_hwmod_class,
>   	.clkdm_name	= "abe_clkdm",
> +	/*
> +	 * It's suspected that the McPDM requires an off-chip main
> +	 * functional clock, controlled via I2C.

Nit: Is it not suspected, it is a fact. The clock tree clearly indicate 
that the mcpdm fclk is generated from the pad_clks.
The IP is a custom link for the Audio IC control / data. using the Audio 
IC as a source clock is standard. Since that IP is done only for that 
purpose, there is no optional clock path from the PRCM like it is the 
case for the McASP / McBSP.

>  This IP block is
> +	 * currently reset very early during boot, before I2C is
> +	 * available, so it doesn't seem that we have any choice in
> +	 * the kernel other than to avoid resetting it.  XXX This is
> +	 * really a hardware issue workaround: every IP block should
> +	 * be able to source its main functional clock from either
> +	 * on-chip or off-chip sources.  McPDM seems to be the only
> +	 * current exception.

I do not think this is the right place to put some complaint about the 
HW :-).
I guess we should just describe the facts.

> +	 */
> +	.flags		= HWMOD_EXT_OPT_MAIN_CLK,

Could you please update the hints for this IP in the autogen scripts?
I added a comment attribute to add a custom comment on top of the flags 
entry.
The latest branch is "omap5430_for_3.6".

Thanks,
Benoit

  reply	other threads:[~2012-06-11  9:54 UTC|newest]

Thread overview: 106+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-11  0:45 [PATCHv2 00/12] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc Paul Walmsley
2012-06-11  0:45 ` Paul Walmsley
2012-06-11  0:45 ` [PATCHv2 01/12] ARM: OMAP: PM: Lock clocks list while generating summary Paul Walmsley
2012-06-11  0:45   ` Paul Walmsley
2012-06-11  0:45 ` [PATCHv2 02/12] ARM: OMAP2+: hwmod code/data: fix 32K sync timer Paul Walmsley
2012-06-11  0:45   ` Paul Walmsley
2012-06-14 16:47   ` Cousson, Benoit
2012-06-14 16:47     ` Cousson, Benoit
2012-06-14 18:04     ` Paul Walmsley
2012-06-14 18:04       ` Paul Walmsley
2012-06-14 20:13       ` Cousson, Benoit
2012-06-14 20:13         ` Cousson, Benoit
2012-06-15  0:18         ` Paul Walmsley
2012-06-15  0:18           ` Paul Walmsley
2012-06-15 13:28           ` Cousson, Benoit
2012-06-15 13:28             ` Cousson, Benoit
2012-07-04 12:48             ` Paul Walmsley
2012-07-04 12:48               ` Paul Walmsley
2012-07-04 12:53               ` Paul Walmsley
2012-07-04 12:53                 ` Paul Walmsley
2012-07-04 14:27                 ` Kevin Hilman
2012-07-04 14:27                   ` Kevin Hilman
2012-07-04 14:53                   ` Paul Walmsley
2012-07-04 14:53                     ` Paul Walmsley
2012-07-04 16:14                   ` Benoit Cousson
2012-07-04 16:14                     ` Benoit Cousson
2012-07-04 16:41                     ` Tero Kristo
2012-07-04 16:41                       ` Tero Kristo
2012-07-04 16:43                       ` Benoit Cousson
2012-07-04 16:43                         ` Benoit Cousson
2012-07-04 19:02                         ` Paul Walmsley
2012-07-04 19:02                           ` Paul Walmsley
2012-07-04 16:57                 ` Benoit Cousson
2012-07-04 16:57                   ` Benoit Cousson
2012-07-04 18:59                   ` Paul Walmsley
2012-07-04 18:59                     ` Paul Walmsley
2012-07-05 22:06                     ` Kevin Hilman
2012-07-05 22:06                       ` Kevin Hilman
2012-07-04 16:01               ` Benoit Cousson
2012-07-04 16:01                 ` Benoit Cousson
2012-07-04 19:05                 ` Paul Walmsley
2012-07-04 19:05                   ` Paul Walmsley
2012-06-11  0:46 ` [PATCHv2 03/12] ARM: OMAP4+: hwmod: fix issue causing IPs not going back to Smart-Standby Paul Walmsley
2012-06-11  0:46   ` Paul Walmsley
2012-06-11  0:46 ` [PATCHv2 04/12] ARM: OMAP4: hwmod data: fix 32k sync timer idle modes Paul Walmsley
2012-06-11  0:46   ` Paul Walmsley
2012-06-11  9:31   ` Cousson, Benoit
2012-06-11  9:31     ` Cousson, Benoit
2012-06-13 17:22     ` Paul Walmsley
2012-06-13 17:22       ` Paul Walmsley
2012-06-11  0:46 ` [PATCHv2 05/12] ARM: OMAP2+: hwmod: add setup_preprogram hook Paul Walmsley
2012-06-11  0:46   ` Paul Walmsley
2012-06-11  0:46 ` [PATCHv2 06/12] ARM: OMAP2+: hwmod: add flag to prevent hwmod code from touching IP block during init Paul Walmsley
2012-06-11  0:46   ` Paul Walmsley
2012-06-11  0:46 ` [PATCHv2 07/12] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup Paul Walmsley
2012-06-11  0:46   ` Paul Walmsley
2012-06-11  6:29   ` Tony Lindgren
2012-06-11  6:29     ` Tony Lindgren
2012-06-14  9:49     ` Paul Walmsley
2012-06-14  9:49       ` Paul Walmsley
2012-06-14  9:59       ` Tony Lindgren
2012-06-14  9:59         ` Tony Lindgren
2012-06-11  0:46 ` [PATCHv2 08/12] ARM: OMAP4: hwmod data: add SL2IF hardreset line Paul Walmsley
2012-06-11  0:46   ` Paul Walmsley
2012-06-14 12:55   ` Cousson, Benoit
2012-06-14 12:55     ` Cousson, Benoit
2012-06-14 17:09     ` Paul Walmsley
2012-06-14 17:09       ` Paul Walmsley
2012-06-14 21:07       ` Cousson, Benoit
2012-06-14 21:07         ` Cousson, Benoit
2012-06-14 23:02         ` Paul Walmsley
2012-06-14 23:02           ` Paul Walmsley
2012-06-15  9:11           ` Cousson, Benoit
2012-06-15  9:11             ` Cousson, Benoit
2012-06-11  0:46 ` [PATCHv2 09/12] ARM: OMAP2+: usb_host_fs: add custom setup_preprogram for usb_host_fs (fsusb) Paul Walmsley
2012-06-11  0:46   ` Paul Walmsley
2012-06-11  6:34   ` Tony Lindgren
2012-06-11  6:34     ` Tony Lindgren
2012-06-11  7:13     ` Felipe Balbi
2012-06-11  7:13       ` Felipe Balbi
2012-06-11  7:41       ` Tony Lindgren
2012-06-11  7:41         ` Tony Lindgren
2012-06-11  7:48         ` Felipe Balbi
2012-06-11  7:48           ` Felipe Balbi
2012-06-11  8:03           ` Tony Lindgren
2012-06-11  8:03             ` Tony Lindgren
2012-06-11  9:12             ` Felipe Balbi
2012-06-11  9:12               ` Felipe Balbi
2012-06-11  0:46 ` [PATCHv2 10/12] ARM: OMAP2+: CM: increase the module disable timeout Paul Walmsley
2012-06-11  0:46   ` Paul Walmsley
2012-06-11  0:46 ` [PATCHv2 11/12] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks Paul Walmsley
2012-06-11  0:46   ` Paul Walmsley
2012-06-11 16:28   ` Cousson, Benoit
2012-06-11 16:28     ` Cousson, Benoit
2012-06-11 16:59     ` Paul Walmsley
2012-06-11 16:59       ` Paul Walmsley
2012-06-11 20:15       ` Cousson, Benoit
2012-06-11 20:15         ` Cousson, Benoit
2012-06-11  0:46 ` [PATCHv2 12/12] ARM: OMAP4: hwmod data: do not enable or reset the McPDM during kernel init Paul Walmsley
2012-06-11  0:46   ` Paul Walmsley
2012-06-11  9:54   ` Cousson, Benoit [this message]
2012-06-11  9:54     ` Cousson, Benoit
2012-10-30  4:05     ` Paul Walmsley
2012-10-30  4:05       ` Paul Walmsley
2012-10-30  7:41       ` Péter Ujfalusi
2012-10-30  7:41         ` Péter Ujfalusi

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=4FD5C04F.2040509@ti.com \
    --to=b-cousson@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=paul@pwsan.com \
    --cc=peter.ujfalusi@ti.com \
    /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.