All of lore.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.com>
To: Paul Burton <paul.burton@imgtec.com>, linux-mips@linux-mips.org
Cc: Paul Burton <paul.burton@imgtec.com>,
	Andrzej Hajda <a.hajda@samsung.com>,
	Aaro Koskinen <aaro.koskinen@nokia.com>,
	Masahiro Yamada <yamada.masahiro@socionext.com>,
	Rob Herring <robh@kernel.org>,
	Alexander Sverdlin <alexander.sverdlin@gmail.com>,
	Peter Hurley <peter@hurleysoftware.com>,
	linux-kernel@vger.kernel.org,
	Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>,
	Jaedon Shin <jaedon.shin@gmail.com>,
	James Hogan <james.hogan@imgtec.com>,
	Jonas Gorski <jogo@openwrt.org>,
	Markos Chandras <markos.chandras@imgtec.com>,
	Ralf Baechle <ralf@linux-mips.org>
Subject: Re: [PATCH] MIPS: Fix early CM probing
Date: Tue, 02 Jan 2018 10:12:25 +1100	[thread overview]
Message-ID: <87fu7pfa1y.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <1454953591-19491-1-git-send-email-paul.burton@imgtec.com>

[-- Attachment #1: Type: text/plain, Size: 3874 bytes --]

On Mon, Feb 08 2016, Paul Burton wrote:

> Commit c014d164f21d ("MIPS: Add platform callback before initializing
> the L2 cache") added a platform_early_l2_init function in order to allow
> platforms to probe for the CM before L2 initialisation is performed, so
> that CM GCRs are available to mips_sc_probe.
>
> That commit actually fails to do anything useful, since it checks
> mips_cm_revision to determine whether it should call mips_cm_probe but
> the result of mips_cm_revision will always be 0 until mips_cm_probe has
> been called. Thus the "early" mips_cm_probe call never occurs.
>
> Fix this & drop the useless weak platform_early_l2_init function by
> simply calling mips_cm_probe from setup_arch. For platforms that don't
> select CONFIG_MIPS_CM this will be a no-op, and for those that do it
> removes the requirement for them to call mips_cm_probe manually
> (although doing so isn't harmful for now).
>
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>

Hi,
 this change breaks the "gnubee" open-hardware NAS board (gnubee.org),
 which is based on a mediatek mt7621 SOC (which admittedly needs a bunch
 of other patches to work at all).

 I forward ported the patches from a mostly-working 4.4-based release to
 upstream and it didn't even print anything to the console.   "git
 bisect" led me to this patch.
 Reverting the change to setup_arch() means my mainline-based kernel
 gets a lot further.

 Based on the description above, I thought that maybe I should just
 disabled CONFIG_MIPS_CM, but that is selected automatically by
 CONFIG_MIPS_GIC, and I suspect that I need that.

 Can you suggest anything I might try to isolate why mips_cm_probe()
 might be causing a problem on this hardware?

Thanks,
NeilBrown


> ---
>
>  arch/mips/kernel/setup.c         |  1 +
>  arch/mips/mm/sc-mips.c           | 10 ----------
>  arch/mips/mti-malta/malta-init.c |  8 --------
>  3 files changed, 1 insertion(+), 18 deletions(-)
>
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index 569a7d5..5fdaf8b 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -782,6 +782,7 @@ static inline void prefill_possible_map(void) {}
>  void __init setup_arch(char **cmdline_p)
>  {
>  	cpu_probe();
> +	mips_cm_probe();
>  	prom_init();
>  
>  	setup_early_fdc_console();
> diff --git a/arch/mips/mm/sc-mips.c b/arch/mips/mm/sc-mips.c
> index 3bd0597..2496475 100644
> --- a/arch/mips/mm/sc-mips.c
> +++ b/arch/mips/mm/sc-mips.c
> @@ -181,10 +181,6 @@ static int __init mips_sc_probe_cm3(void)
>  	return 1;
>  }
>  
> -void __weak platform_early_l2_init(void)
> -{
> -}
> -
>  static inline int __init mips_sc_probe(void)
>  {
>  	struct cpuinfo_mips *c = &current_cpu_data;
> @@ -194,12 +190,6 @@ static inline int __init mips_sc_probe(void)
>  	/* Mark as not present until probe completed */
>  	c->scache.flags |= MIPS_CACHE_NOT_PRESENT;
>  
> -	/*
> -	 * Do we need some platform specific probing before
> -	 * we configure L2?
> -	 */
> -	platform_early_l2_init();
> -
>  	if (mips_cm_revision() >= CM_REV_CM3)
>  		return mips_sc_probe_cm3();
>  
> diff --git a/arch/mips/mti-malta/malta-init.c b/arch/mips/mti-malta/malta-init.c
> index 571148c..dc2c521 100644
> --- a/arch/mips/mti-malta/malta-init.c
> +++ b/arch/mips/mti-malta/malta-init.c
> @@ -293,7 +293,6 @@ mips_pci_controller:
>  	console_config();
>  #endif
>  	/* Early detection of CMP support */
> -	mips_cm_probe();
>  	mips_cpc_probe();
>  
>  	if (!register_cps_smp_ops())
> @@ -304,10 +303,3 @@ mips_pci_controller:
>  		return;
>  	register_up_smp_ops();
>  }
> -
> -void platform_early_l2_init(void)
> -{
> -	/* L2 configuration lives in the CM3 */
> -	if (mips_cm_revision() >= CM_REV_CM3)
> -		mips_cm_probe();
> -}
> -- 
> 2.7.0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: NeilBrown <neilb@suse.com>
To: Paul Burton <paul.burton@imgtec.com>, linux-mips@linux-mips.org
Cc: Andrzej Hajda <a.hajda@samsung.com>,
	Aaro Koskinen <aaro.koskinen@nokia.com>,
	Masahiro Yamada <yamada.masahiro@socionext.com>,
	Rob Herring <robh@kernel.org>,
	Alexander Sverdlin <alexander.sverdlin@gmail.com>,
	Peter Hurley <peter@hurleysoftware.com>,
	linux-kernel@vger.kernel.org,
	Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>,
	Jaedon Shin <jaedon.shin@gmail.com>,
	James Hogan <james.hogan@imgtec.com>,
	Jonas Gorski <jogo@openwrt.org>,
	Markos Chandras <markos.chandras@imgtec.com>,
	Ralf Baechle <ralf@linux-mips.org>
Subject: Re: [PATCH] MIPS: Fix early CM probing
Date: Tue, 02 Jan 2018 10:12:25 +1100	[thread overview]
Message-ID: <87fu7pfa1y.fsf@notabene.neil.brown.name> (raw)
Message-ID: <20180101231225.Cy6TH4y7VaDJ3xKrPSU4tp6huOp63HNoC8jYu0mJUgs@z> (raw)
In-Reply-To: <1454953591-19491-1-git-send-email-paul.burton@imgtec.com>

[-- Attachment #1: Type: text/plain, Size: 3874 bytes --]

On Mon, Feb 08 2016, Paul Burton wrote:

> Commit c014d164f21d ("MIPS: Add platform callback before initializing
> the L2 cache") added a platform_early_l2_init function in order to allow
> platforms to probe for the CM before L2 initialisation is performed, so
> that CM GCRs are available to mips_sc_probe.
>
> That commit actually fails to do anything useful, since it checks
> mips_cm_revision to determine whether it should call mips_cm_probe but
> the result of mips_cm_revision will always be 0 until mips_cm_probe has
> been called. Thus the "early" mips_cm_probe call never occurs.
>
> Fix this & drop the useless weak platform_early_l2_init function by
> simply calling mips_cm_probe from setup_arch. For platforms that don't
> select CONFIG_MIPS_CM this will be a no-op, and for those that do it
> removes the requirement for them to call mips_cm_probe manually
> (although doing so isn't harmful for now).
>
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>

Hi,
 this change breaks the "gnubee" open-hardware NAS board (gnubee.org),
 which is based on a mediatek mt7621 SOC (which admittedly needs a bunch
 of other patches to work at all).

 I forward ported the patches from a mostly-working 4.4-based release to
 upstream and it didn't even print anything to the console.   "git
 bisect" led me to this patch.
 Reverting the change to setup_arch() means my mainline-based kernel
 gets a lot further.

 Based on the description above, I thought that maybe I should just
 disabled CONFIG_MIPS_CM, but that is selected automatically by
 CONFIG_MIPS_GIC, and I suspect that I need that.

 Can you suggest anything I might try to isolate why mips_cm_probe()
 might be causing a problem on this hardware?

Thanks,
NeilBrown


> ---
>
>  arch/mips/kernel/setup.c         |  1 +
>  arch/mips/mm/sc-mips.c           | 10 ----------
>  arch/mips/mti-malta/malta-init.c |  8 --------
>  3 files changed, 1 insertion(+), 18 deletions(-)
>
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index 569a7d5..5fdaf8b 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -782,6 +782,7 @@ static inline void prefill_possible_map(void) {}
>  void __init setup_arch(char **cmdline_p)
>  {
>  	cpu_probe();
> +	mips_cm_probe();
>  	prom_init();
>  
>  	setup_early_fdc_console();
> diff --git a/arch/mips/mm/sc-mips.c b/arch/mips/mm/sc-mips.c
> index 3bd0597..2496475 100644
> --- a/arch/mips/mm/sc-mips.c
> +++ b/arch/mips/mm/sc-mips.c
> @@ -181,10 +181,6 @@ static int __init mips_sc_probe_cm3(void)
>  	return 1;
>  }
>  
> -void __weak platform_early_l2_init(void)
> -{
> -}
> -
>  static inline int __init mips_sc_probe(void)
>  {
>  	struct cpuinfo_mips *c = &current_cpu_data;
> @@ -194,12 +190,6 @@ static inline int __init mips_sc_probe(void)
>  	/* Mark as not present until probe completed */
>  	c->scache.flags |= MIPS_CACHE_NOT_PRESENT;
>  
> -	/*
> -	 * Do we need some platform specific probing before
> -	 * we configure L2?
> -	 */
> -	platform_early_l2_init();
> -
>  	if (mips_cm_revision() >= CM_REV_CM3)
>  		return mips_sc_probe_cm3();
>  
> diff --git a/arch/mips/mti-malta/malta-init.c b/arch/mips/mti-malta/malta-init.c
> index 571148c..dc2c521 100644
> --- a/arch/mips/mti-malta/malta-init.c
> +++ b/arch/mips/mti-malta/malta-init.c
> @@ -293,7 +293,6 @@ mips_pci_controller:
>  	console_config();
>  #endif
>  	/* Early detection of CMP support */
> -	mips_cm_probe();
>  	mips_cpc_probe();
>  
>  	if (!register_cps_smp_ops())
> @@ -304,10 +303,3 @@ mips_pci_controller:
>  		return;
>  	register_up_smp_ops();
>  }
> -
> -void platform_early_l2_init(void)
> -{
> -	/* L2 configuration lives in the CM3 */
> -	if (mips_cm_revision() >= CM_REV_CM3)
> -		mips_cm_probe();
> -}
> -- 
> 2.7.0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

  parent reply	other threads:[~2018-01-01 23:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-08 17:46 [PATCH] MIPS: Fix early CM probing Paul Burton
2016-02-08 17:46 ` Paul Burton
2016-02-09 15:46 ` Alexander Sverdlin
2018-01-01 23:12 ` NeilBrown [this message]
2018-01-01 23:12   ` NeilBrown
2018-01-01 23:56 ` NeilBrown

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=87fu7pfa1y.fsf@notabene.neil.brown.name \
    --to=neilb@suse.com \
    --cc=Leonid.Yegoshin@imgtec.com \
    --cc=a.hajda@samsung.com \
    --cc=aaro.koskinen@nokia.com \
    --cc=alexander.sverdlin@gmail.com \
    --cc=jaedon.shin@gmail.com \
    --cc=james.hogan@imgtec.com \
    --cc=jogo@openwrt.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=markos.chandras@imgtec.com \
    --cc=paul.burton@imgtec.com \
    --cc=peter@hurleysoftware.com \
    --cc=ralf@linux-mips.org \
    --cc=robh@kernel.org \
    --cc=yamada.masahiro@socionext.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.