All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Rob Herring <robherring2@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linaro Patches <patches@linaro.org>
Subject: Re: [PATCH 0/3] of: dts: enable memory@0 quirk for PPC32 only
Date: Sat, 19 Apr 2014 01:36:34 +0100	[thread overview]
Message-ID: <20140419003634.GI5904@bivouac.eciton.net> (raw)
In-Reply-To: <CAL_Jsq+L14rgYq5swC3H109DThiyDajmTJpO=y562jcE97K-fg@mail.gmail.com>

On Fri, Apr 18, 2014 at 04:28:17PM -0500, Rob Herring wrote:
> >> > Apart from the current code permitting recreating a 15+ year old
> >> > firmware bug into completely new platform ports?
> >>
> >> I would prefer to see a "WARN_ON(!IS_ENABLED(CONFIG_PPC32));" added here.
> >
> > In addition to, or instead of, the QUIRK ifdef?
> 
> Instead of because I don't see how you handle the ARM board
> compatibility with the ifdef. (And please, no ifdef for that board).

Umm, according to my memory as well as my sent mail folder, I cc:d you
on v2 of part 3. Could you have a look at that, please?

A WARN_ON would still mean this ancient workaround for a specific ppc32
platform remains enabled on ~10 architectures that don't use it.

> >> Really, I would like to see quirks like this fixed up out of line from
> >> the normal flow somewhat like how PCI quirks are handled. So in this
> >> example, we would just add the missing property to the dtb for the
> >> broken platform before doing the memory scan. That does then require
> >> libfdt which is something I'm working on.
> >
> > Getting rid of all this handling from generic code would clearly be
> > preferable. Is that code going in in the near future, or could we add
> > the quirk as a stopgap?
> 
> Some sort of quirk infrastructure is not going to happen soon. It's
> just an idea bouncing in my head ATM.

Mmm...

> > What would be the effect of the UEFI code adding all its memblocks,
> > minus the reserved areas, and then the DT code doing a memblock_add
> > of all RAM (including reserved areas)? Would memblock_reserve()s on
> > the protected regions suffice to prevent crazy stuff from happening?
> 
> So use UEFI to add the memory, but then add reserved areas with DT?

No, to add memory and reserved areas based on UEFI memory map.
And then add any memory@0/!type nodes as well, if they're left around.

> I'm not sure I follow, but even if I did I don't know memblock code
> well enough to say what it would do.

If we did end up with stray memory@0/!type nodes, we could initialise
memblock multiple times with overlapping but incompatible areas.
And I don't know if that would be a problem. Which makes me a little
bit nervous.

/
    Leif

WARNING: multiple messages have this Message-ID (diff)
From: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
To: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Linaro Patches <patches-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linuxppc-dev
	<linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Subject: Re: [PATCH 0/3] of: dts: enable memory@0 quirk for PPC32 only
Date: Sat, 19 Apr 2014 01:36:34 +0100	[thread overview]
Message-ID: <20140419003634.GI5904@bivouac.eciton.net> (raw)
In-Reply-To: <CAL_Jsq+L14rgYq5swC3H109DThiyDajmTJpO=y562jcE97K-fg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Apr 18, 2014 at 04:28:17PM -0500, Rob Herring wrote:
> >> > Apart from the current code permitting recreating a 15+ year old
> >> > firmware bug into completely new platform ports?
> >>
> >> I would prefer to see a "WARN_ON(!IS_ENABLED(CONFIG_PPC32));" added here.
> >
> > In addition to, or instead of, the QUIRK ifdef?
> 
> Instead of because I don't see how you handle the ARM board
> compatibility with the ifdef. (And please, no ifdef for that board).

Umm, according to my memory as well as my sent mail folder, I cc:d you
on v2 of part 3. Could you have a look at that, please?

A WARN_ON would still mean this ancient workaround for a specific ppc32
platform remains enabled on ~10 architectures that don't use it.

> >> Really, I would like to see quirks like this fixed up out of line from
> >> the normal flow somewhat like how PCI quirks are handled. So in this
> >> example, we would just add the missing property to the dtb for the
> >> broken platform before doing the memory scan. That does then require
> >> libfdt which is something I'm working on.
> >
> > Getting rid of all this handling from generic code would clearly be
> > preferable. Is that code going in in the near future, or could we add
> > the quirk as a stopgap?
> 
> Some sort of quirk infrastructure is not going to happen soon. It's
> just an idea bouncing in my head ATM.

Mmm...

> > What would be the effect of the UEFI code adding all its memblocks,
> > minus the reserved areas, and then the DT code doing a memblock_add
> > of all RAM (including reserved areas)? Would memblock_reserve()s on
> > the protected regions suffice to prevent crazy stuff from happening?
> 
> So use UEFI to add the memory, but then add reserved areas with DT?

No, to add memory and reserved areas based on UEFI memory map.
And then add any memory@0/!type nodes as well, if they're left around.

> I'm not sure I follow, but even if I did I don't know memblock code
> well enough to say what it would do.

If we did end up with stray memory@0/!type nodes, we could initialise
memblock multiple times with overlapping but incompatible areas.
And I don't know if that would be a problem. Which makes me a little
bit nervous.

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

WARNING: multiple messages have this Message-ID (diff)
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Rob Herring <robherring2@gmail.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linaro Patches <patches@linaro.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	Mark Rutland <mark.rutland@arm.com>
Subject: Re: [PATCH 0/3] of: dts: enable memory@0 quirk for PPC32 only
Date: Sat, 19 Apr 2014 01:36:34 +0100	[thread overview]
Message-ID: <20140419003634.GI5904@bivouac.eciton.net> (raw)
In-Reply-To: <CAL_Jsq+L14rgYq5swC3H109DThiyDajmTJpO=y562jcE97K-fg@mail.gmail.com>

On Fri, Apr 18, 2014 at 04:28:17PM -0500, Rob Herring wrote:
> >> > Apart from the current code permitting recreating a 15+ year old
> >> > firmware bug into completely new platform ports?
> >>
> >> I would prefer to see a "WARN_ON(!IS_ENABLED(CONFIG_PPC32));" added here.
> >
> > In addition to, or instead of, the QUIRK ifdef?
> 
> Instead of because I don't see how you handle the ARM board
> compatibility with the ifdef. (And please, no ifdef for that board).

Umm, according to my memory as well as my sent mail folder, I cc:d you
on v2 of part 3. Could you have a look at that, please?

A WARN_ON would still mean this ancient workaround for a specific ppc32
platform remains enabled on ~10 architectures that don't use it.

> >> Really, I would like to see quirks like this fixed up out of line from
> >> the normal flow somewhat like how PCI quirks are handled. So in this
> >> example, we would just add the missing property to the dtb for the
> >> broken platform before doing the memory scan. That does then require
> >> libfdt which is something I'm working on.
> >
> > Getting rid of all this handling from generic code would clearly be
> > preferable. Is that code going in in the near future, or could we add
> > the quirk as a stopgap?
> 
> Some sort of quirk infrastructure is not going to happen soon. It's
> just an idea bouncing in my head ATM.

Mmm...

> > What would be the effect of the UEFI code adding all its memblocks,
> > minus the reserved areas, and then the DT code doing a memblock_add
> > of all RAM (including reserved areas)? Would memblock_reserve()s on
> > the protected regions suffice to prevent crazy stuff from happening?
> 
> So use UEFI to add the memory, but then add reserved areas with DT?

No, to add memory and reserved areas based on UEFI memory map.
And then add any memory@0/!type nodes as well, if they're left around.

> I'm not sure I follow, but even if I did I don't know memblock code
> well enough to say what it would do.

If we did end up with stray memory@0/!type nodes, we could initialise
memblock multiple times with overlapping but incompatible areas.
And I don't know if that would be a problem. Which makes me a little
bit nervous.

/
    Leif

  reply	other threads:[~2014-04-19  0:33 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-17 17:41 [PATCH 0/3] of: dts: enable memory@0 quirk for PPC32 only Leif Lindholm
2014-04-17 17:41 ` Leif Lindholm
2014-04-17 17:41 ` Leif Lindholm
2014-04-17 17:41 ` [PATCH 1/3] arm: dts: add device_type="memory" for ste-ccu8540 Leif Lindholm
2014-04-17 17:41   ` Leif Lindholm
2014-04-22  7:39   ` Lee Jones
2014-04-22  7:39     ` Lee Jones
2014-04-22 13:09     ` Grant Likely
2014-04-22 13:09       ` Grant Likely
2014-04-22 13:26   ` Linus Walleij
2014-04-22 13:26     ` Linus Walleij
2014-05-15 14:50     ` Grant Likely
2014-05-15 14:50       ` Grant Likely
2014-05-15 14:50       ` Grant Likely
2014-04-17 17:42 ` [PATCH 2/3] mips: dts: add device_type="memory" where missing Leif Lindholm
2014-04-18 17:16   ` John Crispin
2014-04-18 17:16     ` John Crispin
2014-04-22 13:13   ` Grant Likely
2014-04-22 13:13     ` Grant Likely
2014-04-22 13:13     ` Grant Likely
2014-05-15 14:50     ` Grant Likely
2014-04-17 17:42 ` [PATCH 3/3] of: Handle memory@0 node on PPC32 only Leif Lindholm
2014-04-17 17:42   ` Leif Lindholm
2014-04-18  8:04   ` Geert Uytterhoeven
2014-04-18  8:04     ` Geert Uytterhoeven
2014-04-18  8:04     ` Geert Uytterhoeven
2014-04-18 12:59     ` Leif Lindholm
2014-04-18 12:59       ` Leif Lindholm
2014-04-18 12:59       ` Leif Lindholm
2014-04-18 13:11       ` Geert Uytterhoeven
2014-04-18 13:11         ` Geert Uytterhoeven
2014-04-21 12:56       ` Rob Herring
2014-04-21 12:56         ` Rob Herring
2014-04-21 12:56         ` Rob Herring
2014-04-23 10:35         ` Mark Rutland
2014-04-23 10:35           ` Mark Rutland
2014-04-23 10:35           ` Mark Rutland
2014-04-22 13:35       ` Grant Likely
2014-04-22 13:35         ` Grant Likely
2014-04-23 10:45         ` Mark Rutland
2014-04-23 10:45           ` Mark Rutland
2014-04-23 11:14           ` Geert Uytterhoeven
2014-04-23 11:14             ` Geert Uytterhoeven
2014-04-23 13:10           ` Grant Likely
2014-04-23 13:10             ` Grant Likely
2014-04-24  9:26             ` Leif Lindholm
2014-04-24  9:26               ` Leif Lindholm
2014-05-15 14:59               ` Grant Likely
2014-05-15 14:59                 ` Grant Likely
2014-04-18  0:43 ` [PATCH 0/3] of: dts: enable memory@0 quirk for " Rob Herring
2014-04-18  0:43   ` Rob Herring
2014-04-18  0:43   ` Rob Herring
2014-04-18 12:48   ` Leif Lindholm
2014-04-18 12:48     ` Leif Lindholm
2014-04-18 12:48     ` Leif Lindholm
2014-04-18 15:37     ` Rob Herring
2014-04-18 15:37       ` Rob Herring
2014-04-18 20:13       ` Leif Lindholm
2014-04-18 20:13         ` Leif Lindholm
2014-04-18 20:13         ` Leif Lindholm
2014-04-18 21:28         ` Rob Herring
2014-04-18 21:28           ` Rob Herring
2014-04-18 21:28           ` Rob Herring
2014-04-19  0:36           ` Leif Lindholm [this message]
2014-04-19  0:36             ` Leif Lindholm
2014-04-19  0:36             ` Leif Lindholm
2014-04-22 13:08       ` Grant Likely
2014-04-22 13:08         ` Grant Likely
2014-04-22 14:05         ` Leif Lindholm
2014-04-22 14:05           ` Leif Lindholm
2014-04-23 13:15           ` Grant Likely
2014-04-23 13:15             ` Grant Likely
2014-04-23 17:25             ` Leif Lindholm
2014-04-23 17:25               ` Leif Lindholm
2014-04-23 17:25               ` Leif Lindholm
2014-04-23 13:17           ` Grant Likely
2014-04-23 13:17             ` Grant Likely

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=20140419003634.GI5904@bivouac.eciton.net \
    --to=leif.lindholm@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mark.rutland@arm.com \
    --cc=patches@linaro.org \
    --cc=robherring2@gmail.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.