All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
To: "Michał Mirosław" <mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH 3/9] arm: cache-l2x0: share l2x0_base
Date: Thu, 5 Oct 2017 17:48:32 +0100	[thread overview]
Message-ID: <20171005164831.GG5235@leverpostej> (raw)
In-Reply-To: <eb104cfdf44ef9bf7b4f271555e1496d2bf781bf.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>

On Thu, Jul 20, 2017 at 02:29:23AM +0200, Michał Mirosław wrote:
> Share l2x0_base between cache-l2x0 and pmu drivers.  They are duplicates.

This is not quite true, and this change introduces bugs.

The l2x0 pmu driver only sets its copy of the base for an l2x0 variant
it recognises:

> -void __init l2x0_pmu_register(void __iomem *base, u32 part)
> +void __init l2x0_pmu_register(u32 part)
>  {
>  	/*
>  	 * Determine whether we support the PMU, and choose the name for sysfs.
> @@ -503,8 +502,7 @@ void __init l2x0_pmu_register(void __iomem *base, u32 part)
>  	 * supported by this driver.
>  	 *
>  	 * We must defer registering the PMU until the perf subsystem is up and
> -	 * running, so just stash the name and base, and leave that to another
> -	 * initcall.
> +	 * running, so just stash the name, and leave that to another initcall.
>  	 */
>  	switch (part & L2X0_CACHE_ID_PART_MASK) {
>  	case L2X0_CACHE_ID_PART_L220:
> @@ -516,8 +514,6 @@ void __init l2x0_pmu_register(void __iomem *base, u32 part)
>  	default:
>  		return;
>  	}
> -
> -	l2x0_base = base;

... note that we may return *before* setting l2x0_base.

This (deliberately) prevents us from erroneously registering a PMU for
l2x0 variants for which we don't know how to drive any IMPLEMENTATION
DEFINED PMU functionality.

This cahnge breaks that, and will result in l2x0_pmu_init() trying to
register a PMU with a NULL name.

So NAK to this change.

Thanks,
Mark.

WARNING: multiple messages have this Message-ID (diff)
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/9] arm: cache-l2x0: share l2x0_base
Date: Thu, 5 Oct 2017 17:48:32 +0100	[thread overview]
Message-ID: <20171005164831.GG5235@leverpostej> (raw)
In-Reply-To: <eb104cfdf44ef9bf7b4f271555e1496d2bf781bf.1500510157.git.mirq-linux@rere.qmqm.pl>

On Thu, Jul 20, 2017 at 02:29:23AM +0200, Micha? Miros?aw wrote:
> Share l2x0_base between cache-l2x0 and pmu drivers.  They are duplicates.

This is not quite true, and this change introduces bugs.

The l2x0 pmu driver only sets its copy of the base for an l2x0 variant
it recognises:

> -void __init l2x0_pmu_register(void __iomem *base, u32 part)
> +void __init l2x0_pmu_register(u32 part)
>  {
>  	/*
>  	 * Determine whether we support the PMU, and choose the name for sysfs.
> @@ -503,8 +502,7 @@ void __init l2x0_pmu_register(void __iomem *base, u32 part)
>  	 * supported by this driver.
>  	 *
>  	 * We must defer registering the PMU until the perf subsystem is up and
> -	 * running, so just stash the name and base, and leave that to another
> -	 * initcall.
> +	 * running, so just stash the name, and leave that to another initcall.
>  	 */
>  	switch (part & L2X0_CACHE_ID_PART_MASK) {
>  	case L2X0_CACHE_ID_PART_L220:
> @@ -516,8 +514,6 @@ void __init l2x0_pmu_register(void __iomem *base, u32 part)
>  	default:
>  		return;
>  	}
> -
> -	l2x0_base = base;

... note that we may return *before* setting l2x0_base.

This (deliberately) prevents us from erroneously registering a PMU for
l2x0 variants for which we don't know how to drive any IMPLEMENTATION
DEFINED PMU functionality.

This cahnge breaks that, and will result in l2x0_pmu_init() trying to
register a PMU with a NULL name.

So NAK to this change.

Thanks,
Mark.

  parent reply	other threads:[~2017-10-05 16:48 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-20  0:29 [PATCH 0/9] Bringing Asus TF300T support to mainline Michał Mirosław
2017-07-20  0:29 ` Michał Mirosław
     [not found] ` <cover.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
2017-07-20  0:29   ` [PATCH 2/9] arm: cache-l2x0: remove duplicate warning Michał Mirosław
2017-07-20  0:29     ` Michał Mirosław
2017-07-20  0:29   ` [PATCH 3/9] arm: cache-l2x0: share l2x0_base Michał Mirosław
2017-07-20  0:29     ` Michał Mirosław
     [not found]     ` <eb104cfdf44ef9bf7b4f271555e1496d2bf781bf.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
2017-10-05 16:48       ` Mark Rutland [this message]
2017-10-05 16:48         ` Mark Rutland
2017-12-19 23:10         ` Russell King - ARM Linux
2017-12-19 23:10           ` Russell King - ARM Linux
2017-07-20  0:29   ` [PATCH 1/9] ARM: enable secure platform-only erratas Michał Mirosław
2017-07-20  0:29     ` Michał Mirosław
     [not found]     ` <77ce738c15b992a92bee3a18e5468342fb2dc5ab.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
2017-10-05 15:53       ` Dmitry Osipenko
2017-10-05 15:53         ` Dmitry Osipenko
2017-10-05 18:16       ` Dmitry Osipenko
2017-10-05 18:16         ` Dmitry Osipenko
     [not found]         ` <bdff72b9-8ebd-a426-b27b-fe055d45cfb1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-12-19 23:28           ` Russell King - ARM Linux
2017-12-19 23:28             ` Russell King - ARM Linux
     [not found]             ` <20171219232810.GI10595-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>
2017-12-20 12:49               ` Dmitry Osipenko
2017-12-20 12:49                 ` Dmitry Osipenko
2017-07-20  0:29   ` [PATCH 5/9] ARM: trusted_foundations: announce firmware version Michał Mirosław
2017-07-20  0:29     ` Michał Mirosław
2017-07-20  0:29   ` [PATCH 4/9] ARM: trusted_foundations: enable L2x0 cache via firmware_ops Michał Mirosław
2017-07-20  0:29     ` Michał Mirosław
     [not found]     ` <1d12ea86cca40749731a594afc165830c0b2463d.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
2017-10-05 16:24       ` Dmitry Osipenko
2017-10-05 16:24         ` Dmitry Osipenko
2017-12-19 18:56       ` Dmitry Osipenko
2017-12-19 18:56         ` Dmitry Osipenko
2017-07-20  0:29   ` [PATCH 8/9] ARM: tegra: avoid touching Secure registers in reset handler Michał Mirosław
2017-07-20  0:29     ` Michał Mirosław
2017-07-20  0:29   ` [PATCH 6/9] ARM: init: update secondary_data register documentation Michał Mirosław
2017-07-20  0:29     ` Michał Mirosław
     [not found]     ` <a6e5e735ebae32a433e501188a86082efc8b0b52.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
2017-12-19 23:16       ` Russell King - ARM Linux
2017-12-19 23:16         ` Russell King - ARM Linux
2017-07-20  0:29   ` [PATCH 7/9] ARM: tegra: enable cache via TF Michał Mirosław
2017-07-20  0:29     ` Michał Mirosław
     [not found]     ` <6a164b2270a3e996c083e94bf5b1e27028c1135e.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
2017-12-19 19:07       ` Dmitry Osipenko
2017-12-19 19:07         ` Dmitry Osipenko
2017-12-19 23:21       ` Russell King - ARM Linux
2017-12-19 23:21         ` Russell King - ARM Linux
2017-07-20  0:29   ` [PATCH 9/9] ARM: tegra: fix sleeping while atomic in CPU idle Michał Mirosław
2017-07-20  0:29     ` Michał Mirosław
     [not found]     ` <0a0600cdbcc9a71134105043c3e2ace84bab7c5a.1500510157.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
2017-07-20 12:45       ` Jon Hunter
2017-07-20 12:45         ` Jon Hunter
     [not found]         ` <378bf911-147e-f800-2de4-591e160528f0-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2017-07-20 16:28           ` Michał Mirosław
2017-07-20 16:28             ` Michał Mirosław
     [not found]             ` <20170720162852.rh565t5as2tbe6np-cHozx32mtrEEUmgCuDUIdw@public.gmane.org>
2017-07-21  8:15               ` Jon Hunter
2017-07-21  8:15                 ` Jon Hunter
2017-07-20  7:48   ` [PATCH 0/9] Bringing Asus TF300T support to mainline Mikko Perttunen
2017-07-20  7:48     ` Mikko Perttunen
     [not found]     ` <a4149270-561a-4190-4c4e-c164bd91446e-/1wQRMveznE@public.gmane.org>
2017-07-20 15:07       ` Michał Mirosław
2017-07-20 15:07         ` Michał Mirosław
2017-10-04 21:25 ` Michał Mirosław
2017-10-04 21:25   ` Michał Mirosław
     [not found]   ` <20171004212534.uosfz6757bbds2c5-cHozx32mtrEEUmgCuDUIdw@public.gmane.org>
2017-10-05 15:52     ` Dmitry Osipenko
2017-10-05 15:52       ` Dmitry Osipenko
     [not found]       ` <dcbbc541-60c1-8af5-25ea-927629447a55-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-10-05 16:07         ` Dmitry Osipenko
2017-10-05 16:07           ` Dmitry Osipenko
  -- strict thread matches above, loose matches on Subject: below --
2017-07-20  0:14 Michał Mirosław
     [not found] ` <cover.1500509346.git.mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.org>
2017-07-20  0:14   ` [PATCH 3/9] arm: cache-l2x0: share l2x0_base Michał Mirosław

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=20171005164831.GG5235@leverpostej \
    --to=mark.rutland-5wv7dgnigg8@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw@public.gmane.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.