* Re: [PATCH v3 1/4] powerpc: Removing support for 'protected-sources'
From: Benjamin Herrenschmidt @ 2011-02-07 21:45 UTC (permalink / raw)
To: Meador Inge; +Cc: Hollis Blanchard, devicetree-discuss, linuxppc-dev
In-Reply-To: <4D5033C9.8030407@mentor.com>
> In my previous reply I said that "it is not so much as a need as it is a
> potential simplification." After further reflection, I don't think that
> is completely true. As we get into AMP systems with higher core counts,
> then implementing this functionality using the existing
> "protected-sources" implementation versus the new "pic-no-reset" work is
> going to be harder to maintain.
I'm not arguing that your approach isn't more suitable for AMP systems,
I just want to leave the existing protected-sources mechanism alone. I'm
not opposing adding a new, better, mechanism for newer platforms.
However, I'd name it differently. "pic-no-reset" doesn't carry enough
meaning in that case. What we want to point out here is that the PIC
has been pre-initialized.
Another option, which may be cleaner, is to stick to "no-reset" (no need
for pic- prefix) and make it do just that (prevent the reset), and then
use a positive variant of "protected-sources", call it
"allowed-sources". Maybe even make it a series of ranges. Then have the
MPIC only access these.
I think this is more robust as it would also prevent "accidental" use of
the wrong sources (bad device-tree, drivers that let you muck around
with irq numbers, etc...).
Cheers,
Ben.
> The reason being that *every* OS instance has to know about *every*
> other OSes interrupt sources, which is a little gross. You can see this
> happening already in "arch/powerpc/boot/dts/p2020rdb_camp_core0.dts" and
> "arch/powerpc/boot/dts/p2020rdb_camp_core1.dts":
>
> // p2020rdb_camp_core0.dts
> mpic: pic@40000 {
> ...
> // Sources used by the OS on core 1
> protected-sources = <
> 42 76 77 78 79 /* serial1 , dma2 */
> 29 30 34 26 /* enet0, pci1 */
> 0xe0 0xe1 0xe2 0xe3 /* msi */
> 0xe4 0xe5 0xe6 0xe7
> >;
> };
>
> // p2020rdb_camp_core1.dts
> mpic: pic@40000 {
> ...
> // Sources used by the OS on core 0
> protected-sources = <
> 17 18 43 42 59 47 /*ecm, mem, i2c, serial0, spi,gpio */
> 16 20 21 22 23 28 /* L2, dma1, USB */
> 03 35 36 40 31 32 33 /* mdio, enet1, enet2 */
> 72 45 58 25 /* sdhci, crypto , pci */
> >;
> };
>
> It is going to be a real pain to keep all of the lists up to date.
> Especially considering we already have sufficient information in the
> device tree to do this work. I do understand the concern of
> finding/testing the older systems. However, is the testing of those
> systems enough to keep out the proposed change and potentially lower
> maintenance in the future? Is the legacy system argument the only
> reason to keep this change out or are there other technical deficiencies?
>
> Also, in the proposed MPIC modifications there is a check for protected
> sources (it is treated as an alias for "pic-no-reset"; see PATCH 3 in
> the set) that should provide functionality equivalent to what systems
> using "protected-sources" already have. That check only looks for the
> presence of "protected-sources" and does not process the cells. Another
> option would be to leave in the protected sources implementation (but
> undocumented in the binding) and have the full "pic-no-reset" behavior
> there as well (and documented in the binding).
>
> If this has no chance of acceptance (?), then I will just re-submit the
> binding and implementation with "protected-sources" and the limited form
> of "pic-no-reset".
>
^ permalink raw reply
* Re: [PATCH v2 3/3] powerpc: Add IO event interrupt support
From: Tseng-Hui (Frank) Lin @ 2011-02-07 21:40 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: tsenglin, linuxppc-dev
In-Reply-To: <1297055437.14982.57.camel@pasglop>
On Mon, 2011-02-07 at 16:10 +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2011-01-05 at 16:44 -0600, Tseng-Hui (Frank) Lin wrote:
> > This patch adds support for handling IO Event interrupts which come
> > through at the /event-sources/ibm,io-events device tree node.
>
> .../...
>
> The previous patches regarding the error log sometimes call it error log
> and sometimes event log. Can you consolidate that ?
Sure. The top level of the RTAS error/event log is for both error and
event. Starting V6 extension, they should be all event.
>
> > +int pseries_ioei_register_handler(pseries_ioei_handler_t handler)
> > +{
>
> Shouldn't we have a void * to attach with each client so it can put
> its private data there and get them back in the handler ?
Although not used at this moment, this is a good idea. Will add private
data.
> We are -really- re-inventing interrupts and/or notifiers here, which
> I find a tad annoying..
Agree. This is ugly. We could have just used the regular interrupt
mechanism if the firmware didn't use shared rtas call and interrupt
to report multiple types of events.
>
> Cheers,
> Ben.
>
>
^ permalink raw reply
* Re: [PATCH 21/23] hvc_console: Fix race between hvc_close and hvc_remove
From: Anton Blanchard @ 2011-02-07 21:16 UTC (permalink / raw)
To: Willy Tarreau
Cc: Rusty Russell, Greg Kroah-Hartman, linux-kernel, stable,
linuxppc-dev, Amit Shah, stable-review, Alan Cox
In-Reply-To: <20110206232253.421321729@pcw.home.local>
Hi,
> From: Amit Shah <amit.shah@redhat.com>
>
> commit e74d098c66543d0731de62eb747ccd5b636a6f4c upstream.
>
> Alan pointed out a race in the code where hvc_remove is invoked. The
> recent virtio_console work is the first user of hvc_remove().
I faintly remember this bug caused boot issues and the following patch
must be applied to fix it.
Anton
--
commit 320718ee074acce5ffced6506cb51af1388942aa
Author: Anton Blanchard <anton@samba.org>
Date: Tue Apr 6 21:42:38 2010 +1000
hvc_console: Fix race between hvc_close and hvc_remove
I don't claim to understand the tty layer, but it seems like hvc_open and
hvc_close should be balanced in their kref reference counting.
Right now we get a kref every call to hvc_open:
if (hp->count++ > 0) {
tty_kref_get(tty); <----- here
spin_unlock_irqrestore(&hp->lock, flags);
hvc_kick();
return 0;
} /* else count == 0 */
tty->driver_data = hp;
hp->tty = tty_kref_get(tty); <------ or here if hp->count was 0
But hvc_close has:
tty_kref_get(tty);
if (--hp->count == 0) {
...
/* Put the ref obtained in hvc_open() */
tty_kref_put(tty);
...
}
tty_kref_put(tty);
Since the outside kref get/put balance we only do a single kref_put when
count reaches 0.
The patch below changes things to call tty_kref_put once for every
hvc_close call, and with that my machine boots fine.
Signed-off-by: Anton Blanchard <anton@samba.org>
Acked-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
diff --git a/drivers/char/hvc_console.c b/drivers/char/hvc_console.c
index d3890e8..35cca4c 100644
--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c
@@ -368,16 +368,12 @@ static void hvc_close(struct tty_struct *tty, struct file * filp)
hp = tty->driver_data;
spin_lock_irqsave(&hp->lock, flags);
- tty_kref_get(tty);
if (--hp->count == 0) {
/* We are done with the tty pointer now. */
hp->tty = NULL;
spin_unlock_irqrestore(&hp->lock, flags);
- /* Put the ref obtained in hvc_open() */
- tty_kref_put(tty);
-
if (hp->ops->notifier_del)
hp->ops->notifier_del(hp, hp->data);
^ permalink raw reply related
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Rafael J. Wysocki @ 2011-02-07 19:16 UTC (permalink / raw)
To: Mark Brown
Cc: Stephen Rothwell, linux-embedded, Len Brown, Dmitry Torokhov,
linux-kernel, Alan Stern, Geert Uytterhoeven, linux-pm, ppc-dev,
Andrew Morton
In-Reply-To: <20110207154953.GN10564@opensource.wolfsonmicro.com>
On Monday, February 07, 2011, Mark Brown wrote:
> On Mon, Feb 07, 2011 at 10:36:31AM -0500, Alan Stern wrote:
> > On Mon, 7 Feb 2011, Mark Brown wrote:
>
> > > I'd not be so sure - since it's a bool without an explicit default set
> > > Kconfig will default to disabling it and if anything enabling it is the
> > > option that requires special effort.
>
> > This may be a naive suggestion, but have you considered simply _asking_
> > the people who added those defconfigs?
>
> I'm rather hoping that they'll notice the mailing list thread or that
> someone else who knows what's going on with them does - as Geert pointed
> out there's a considerable number of defconfigs that have this turned
> off. It seems more sensible to get some idea if this seems sane to
> people in the general case before going trying to identify and contact
> so many individuals.
>
> If there are systems that really require disabling CONFIG_PM we probably
> need to add stuff to Kconfig to make sure it can't be enabled anyway;
> this shouldn't enable any new configurations.
Well, as I've just said, I don't like this change. I'd very much prefer it if
CONFIG_PM_OPS were renamed to CONFIG_PM.
Thanks,
Rafael
^ permalink raw reply
* RE: [PATCH V8 04/10] USB/ppc4xx: Add Synopsys DWC OTG HCD function
From: Tirumala Marri @ 2011-02-07 18:53 UTC (permalink / raw)
To: Alexander Gordeev
Cc: greg, linux-usb, linuxppc-dev, Fushen Chen, Mark Miesfeld
In-Reply-To: <20110126192743.646af6d2@desktopvm.lvknet>
}
dev_set_drvdata(_dev, dwc_otg_device);
hcd->regs = otg_dev->base;
+ hcd->rsrc_start = otg_dev->phys_addr;
+ hcd->rsrc_len = otg_dev->base_len;
hcd->self.otg_port = 1;
[Marri] Sure
^ permalink raw reply
* RE: [PATCH V8 03/10] USB/ppc4xx: Add Synopsys DWC OTG Core Interface Layer
From: Tirumala Marri @ 2011-02-07 18:53 UTC (permalink / raw)
To: Alexander Gordeev
Cc: greg, linux-usb, linuxppc-dev, Fushen Chen, Mark Miesfeld
In-Reply-To: <20110126191734.5dff9641@desktopvm.lvknet>
dwc_read_reg32 is used nowhere throughout the code. One of dwc_read32 and
dwc_read_reg32 should be removed IMO. There was once only dwc_read_reg32. In
version 5 of your patchset I believe. Why did you add another function?
AFAIK it is not correct to store pointers in u32 because they need 8 bytes
on 64-bit archs. So it was ok with the old dwc_read_reg32.
[Marri] If u32 is 8bytes isn't pointer type would be 8bytes as well. I had
change the API to avoid type castings to register addresses.
^ permalink raw reply
* RE: [PATCH V8 03/10] USB/ppc4xx: Add Synopsys DWC OTG CoreInterface Layer
From: Tirumala Marri @ 2011-02-07 18:45 UTC (permalink / raw)
To: David Laight; +Cc: linux-usb, linuxppc-dev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6D8AC2C@saturn3.aculab.com>
-----Original Message-----
From: linuxppc-dev-bounces+tmarri=amcc.com@lists.ozlabs.org
[mailto:linuxppc-dev-bounces+tmarri=amcc.com@lists.ozlabs.org] On Behalf
Of David Laight
Sent: Wednesday, January 26, 2011 8:35 AM
Cc: linux-usb@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
Subject: RE: [PATCH V8 03/10] USB/ppc4xx: Add Synopsys DWC OTG
CoreInterface Layer
> Also in_le32/out_le32/in_be32/out_be32 are
> architecture-specific AFAIK.
Isn't the whole patch architecture-specific ?
> I'd suggest using readl/writel for LE ops and
> __be32_to_cpu(__raw_readl(addr))/__raw_writel(__cpu_to_be32(b),addr)
> for BE ops.
Since the ppc doesn't have a byteswap instruction (and the LE
memory transfers might even be slow!) you really don't want to
be doing software byteswap.
Not to mention the horrific synchronistion instructions
that in_le32() and out_le32() actually contain.
I didn't find __raw_readl() when I was looking for asm
patterns that byteswapped memory transfers.
I did find st_le32() and ld_le32() in arch/powerpc/include/asm/swab.h
but that file is actually quite difficult to #include because
there is another swab.h that gets included instead.
[marri] Initial version of DWC-IP did not have feature to swap endianness.
Whereas next versions have it.
^ permalink raw reply
* Re: [PATCH v3 1/4] powerpc: Removing support for 'protected-sources'
From: Meador Inge @ 2011-02-07 18:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Hollis Blanchard, devicetree-discuss, linuxppc-dev
In-Reply-To: <1297035336.14982.14.camel@pasglop>
On 02/06/2011 05:35 PM, Benjamin Herrenschmidt wrote:
> On Fri, 2011-02-04 at 17:25 -0600, Meador Inge wrote:
>> In a recent thread [1,2,3] concerning device trees for AMP systems, the
>> question of whether we really need 'protected-sources' arose. The general
>> consensus was that a new boolean property 'pic-no-reset' (described in more
>> detail in a following patch) could be expanded to cover the use cases that
>> 'protected-sources' was covering.
>>
>> One concern that was raised was for legacy systems which already use the
>> 'protected-sources' property [4]. For legacy use cases, 'protected-sources'
>> will be treated as an alias of 'pic-no-reset'. The sources
>> encoded in the 'protected-sources' cells, however, will not be processed. This
>> legacy check is added in a later patch in the series.
>
> I'm a bit annoyed here. Why do we need to do that ? Those Cell machines
> are going to be real bastards to find and test with, and I don't really
> see the point...
In my previous reply I said that "it is not so much as a need as it is a
potential simplification." After further reflection, I don't think that
is completely true. As we get into AMP systems with higher core counts,
then implementing this functionality using the existing
"protected-sources" implementation versus the new "pic-no-reset" work is
going to be harder to maintain.
The reason being that *every* OS instance has to know about *every*
other OSes interrupt sources, which is a little gross. You can see this
happening already in "arch/powerpc/boot/dts/p2020rdb_camp_core0.dts" and
"arch/powerpc/boot/dts/p2020rdb_camp_core1.dts":
// p2020rdb_camp_core0.dts
mpic: pic@40000 {
...
// Sources used by the OS on core 1
protected-sources = <
42 76 77 78 79 /* serial1 , dma2 */
29 30 34 26 /* enet0, pci1 */
0xe0 0xe1 0xe2 0xe3 /* msi */
0xe4 0xe5 0xe6 0xe7
>;
};
// p2020rdb_camp_core1.dts
mpic: pic@40000 {
...
// Sources used by the OS on core 0
protected-sources = <
17 18 43 42 59 47 /*ecm, mem, i2c, serial0, spi,gpio */
16 20 21 22 23 28 /* L2, dma1, USB */
03 35 36 40 31 32 33 /* mdio, enet1, enet2 */
72 45 58 25 /* sdhci, crypto , pci */
>;
};
It is going to be a real pain to keep all of the lists up to date.
Especially considering we already have sufficient information in the
device tree to do this work. I do understand the concern of
finding/testing the older systems. However, is the testing of those
systems enough to keep out the proposed change and potentially lower
maintenance in the future? Is the legacy system argument the only
reason to keep this change out or are there other technical deficiencies?
Also, in the proposed MPIC modifications there is a check for protected
sources (it is treated as an alias for "pic-no-reset"; see PATCH 3 in
the set) that should provide functionality equivalent to what systems
using "protected-sources" already have. That check only looks for the
presence of "protected-sources" and does not process the cells. Another
option would be to leave in the protected sources implementation (but
undocumented in the binding) and have the full "pic-no-reset" behavior
there as well (and documented in the binding).
If this has no chance of acceptance (?), then I will just re-submit the
binding and implementation with "protected-sources" and the limited form
of "pic-no-reset".
--
Meador Inge | meador_inge AT mentor.com
Mentor Embedded | http://www.mentor.com/embedded-software
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Mark Brown @ 2011-02-07 15:49 UTC (permalink / raw)
To: Alan Stern
Cc: Stephen Rothwell, linux-embedded, Len Brown, Dmitry Torokhov,
linux-kernel, Rafael J. Wysocki, Geert Uytterhoeven, linux-pm,
ppc-dev, Andrew Morton
In-Reply-To: <Pine.LNX.4.44L0.1102071035320.1924-100000@iolanthe.rowland.org>
On Mon, Feb 07, 2011 at 10:36:31AM -0500, Alan Stern wrote:
> On Mon, 7 Feb 2011, Mark Brown wrote:
> > I'd not be so sure - since it's a bool without an explicit default set
> > Kconfig will default to disabling it and if anything enabling it is the
> > option that requires special effort.
> This may be a naive suggestion, but have you considered simply _asking_
> the people who added those defconfigs?
I'm rather hoping that they'll notice the mailing list thread or that
someone else who knows what's going on with them does - as Geert pointed
out there's a considerable number of defconfigs that have this turned
off. It seems more sensible to get some idea if this seems sane to
people in the general case before going trying to identify and contact
so many individuals.
If there are systems that really require disabling CONFIG_PM we probably
need to add stuff to Kconfig to make sure it can't be enabled anyway;
this shouldn't enable any new configurations.
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Alan Stern @ 2011-02-07 15:36 UTC (permalink / raw)
To: Mark Brown
Cc: Stephen Rothwell, linux-embedded, Len Brown, Dmitry Torokhov,
linux-kernel, Rafael J. Wysocki, Geert Uytterhoeven, linux-pm,
ppc-dev, Andrew Morton
In-Reply-To: <20110207152132.GM10564@opensource.wolfsonmicro.com>
On Mon, 7 Feb 2011, Mark Brown wrote:
> On Tue, Feb 08, 2011 at 02:19:16AM +1100, Stephen Rothwell wrote:
>
> > At least some of the powerpc defconfigs were added with CONFIG_PM
> > disabled. I assume that was on purpose (though it may not have been).
>
> I'd not be so sure - since it's a bool without an explicit default set
> Kconfig will default to disabling it and if anything enabling it is the
> option that requires special effort.
This may be a naive suggestion, but have you considered simply _asking_
the people who added those defconfigs?
Alan Stern
^ permalink raw reply
* Re: minimum guaranteed alignment of dma_alloc_coherent?
From: Timur Tabi @ 2011-02-07 15:13 UTC (permalink / raw)
To: Dan Malek; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <9E25355F-62B4-46A3-9991-FDE89FB6B862@digitaldans.com>
Dan Malek wrote:
> So, I did a little research. It appears in the case of PowerPC,
> the GFP_DMA can change the way the allocation operates.
> Since the coherent allocator works with pages of memory,
> in the case of a system with highmem, not using GFP_DMA
> could hand you a physical page out of the highmem pool.
I think that's true only if SWIOTLB is enabled. dma_direct_alloc_coherent()
does this:
/* ignore region specifiers */
flag &= ~(__GFP_HIGHMEM);
> This behavior is modified if you specify a restricted DMA
> mask for the device. In this case, dma_alloc_coherent
> will force GFP_DMA on your behalf (on PowerPC).
Isn't it required for all callers of dma_alloc_coherent to specify a mask (via
dma_set_mask) first?
--
Timur Tabi
Linux kernel developer at Freescale
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Mark Brown @ 2011-02-07 15:21 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Len Brown, linux-embedded, Dmitry Torokhov, linux-kernel,
Rafael J. Wysocki, Geert Uytterhoeven, Alan Stern, linux-pm,
ppc-dev, Andrew Morton
In-Reply-To: <20110208021916.68b33b37.sfr@canb.auug.org.au>
On Tue, Feb 08, 2011 at 02:19:16AM +1100, Stephen Rothwell wrote:
> At least some of the powerpc defconfigs were added with CONFIG_PM
> disabled. I assume that was on purpose (though it may not have been).
I'd not be so sure - since it's a bool without an explicit default set
Kconfig will default to disabling it and if anything enabling it is the
option that requires special effort.
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Stephen Rothwell @ 2011-02-07 15:19 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Len Brown, Dmitry Torokhov, linux-embedded, Mark Brown,
linux-kernel, Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev,
Andrew Morton
In-Reply-To: <20110208021045.699654ca.sfr@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 844 bytes --]
On Tue, 8 Feb 2011 02:10:45 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> On Mon, 7 Feb 2011 16:00:55 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > $ git grep "CONFIG_PM is not set"
> > 7cf3d73b4360e91b14326632ab1aeda4cb26308d^ -- arch/ | wc -l
> > 256
> > $
> >
> > 7cf3d73b4360e91b14326632ab1aeda4cb26308d is the commit that introduced
> > savedefconfig, so that's a safe revision with untrimmed defconfigs.
>
> Yeah, but we can't tell if CONFIG_PM is turned off on purpose in those
> defconfigs, or just off because noone explicitly turned it on.
At least some of the powerpc defconfigs were added with CONFIG_PM
disabled. I assume that was on purpose (though it may not have been).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Stephen Rothwell @ 2011-02-07 15:10 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Len Brown, Dmitry Torokhov, linux-embedded, Mark Brown,
linux-kernel, Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev,
Andrew Morton
In-Reply-To: <AANLkTikw7j5Q7CGZK=Xbcgubt9LYBGiXf01wQ4nbg8NR@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 605 bytes --]
Hi Geert,
On Mon, 7 Feb 2011 16:00:55 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> $ git grep "CONFIG_PM is not set"
> 7cf3d73b4360e91b14326632ab1aeda4cb26308d^ -- arch/ | wc -l
> 256
> $
>
> 7cf3d73b4360e91b14326632ab1aeda4cb26308d is the commit that introduced
> savedefconfig, so that's a safe revision with untrimmed defconfigs.
Yeah, but we can't tell if CONFIG_PM is turned off on purpose in those
defconfigs, or just off because noone explicitly turned it on.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Geert Uytterhoeven @ 2011-02-07 15:00 UTC (permalink / raw)
To: Mark Brown
Cc: Stephen Rothwell, linux-embedded, Len Brown, Dmitry Torokhov,
linux-kernel, Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev,
Andrew Morton
In-Reply-To: <20110207145031.GL10564@opensource.wolfsonmicro.com>
On Mon, Feb 7, 2011 at 15:50, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Feb 08, 2011 at 01:44:32AM +1100, Stephen Rothwell wrote:
>> On Mon, 7 Feb 2011 14:18:29 +0000 Mark Brown <broonie@opensource.wolfson=
micro.com> wrote:
>
>> > Do you mean that these systems require CONFIG_PM be turned off, or jus=
t
>> > that people tend not to turn it on? =C2=A0If the latter would you expe=
ct any
>> > ill effects from doing so?
>
>> I don't know the answer to either question without testing. =C2=A0All I =
am
>> saying is that currently the default for CONFIG_PM is "off" and you are
>> changing it to be "on" and there may not have been any testing done of
>> that in some situations. =C2=A0 We don't know where it was explicitly
>> turned off any more since we shrank our defconfig files (which was done
>> automatically) ... since it is off by default, it doesn't need to be
>> mentioned in a defconfig unless it needs to be turned on.
>
> My suspicion would be that it'll have been turned off by someone hitting
> return through a config upgrade rather than through deliberate effort.
> On the other hand if it is essential for some machines to have it
> disabled they probably want to have somethnig in Kconfig.
$ git grep "CONFIG_PM is not set"
7cf3d73b4360e91b14326632ab1aeda4cb26308d^ -- arch/ | wc -l
256
$
7cf3d73b4360e91b14326632ab1aeda4cb26308d is the commit that introduced
savedefconfig, so that's a safe revision with untrimmed defconfigs.
Gr{oetje,eeting}s,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org
In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0 =C2=A0=C2=A0 -- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Mark Brown @ 2011-02-07 14:50 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Len Brown, linux-embedded, Dmitry Torokhov, linux-kernel,
Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev, Andrew Morton
In-Reply-To: <20110208014432.2c9288e4.sfr@canb.auug.org.au>
On Tue, Feb 08, 2011 at 01:44:32AM +1100, Stephen Rothwell wrote:
> On Mon, 7 Feb 2011 14:18:29 +0000 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> > Do you mean that these systems require CONFIG_PM be turned off, or just
> > that people tend not to turn it on? If the latter would you expect any
> > ill effects from doing so?
> I don't know the answer to either question without testing. All I am
> saying is that currently the default for CONFIG_PM is "off" and you are
> changing it to be "on" and there may not have been any testing done of
> that in some situations. We don't know where it was explicitly
> turned off any more since we shrank our defconfig files (which was done
> automatically) ... since it is off by default, it doesn't need to be
> mentioned in a defconfig unless it needs to be turned on.
My suspicion would be that it'll have been turned off by someone hitting
return through a config upgrade rather than through deliberate effort.
On the other hand if it is essential for some machines to have it
disabled they probably want to have somethnig in Kconfig.
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Stephen Rothwell @ 2011-02-07 14:44 UTC (permalink / raw)
To: Mark Brown
Cc: Len Brown, linux-embedded, Dmitry Torokhov, linux-kernel,
Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev, Andrew Morton
In-Reply-To: <20110207141829.GK10564@opensource.wolfsonmicro.com>
[-- Attachment #1: Type: text/plain, Size: 1252 bytes --]
Hi Mark,
On Mon, 7 Feb 2011 14:18:29 +0000 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
>
> On Tue, Feb 08, 2011 at 01:13:24AM +1100, Stephen Rothwell wrote:
> > On Mon, 7 Feb 2011 12:22:15 +0000 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
>
> > > + bool
> > > + default y if !IA64_HP_SIM
>
> > Several powerpc configs have CONFIG_PM (implicitly) disabled (e.g. the
> > server configs), so this will unexpectedly turn it on for them.
>
> Do you mean that these systems require CONFIG_PM be turned off, or just
> that people tend not to turn it on? If the latter would you expect any
> ill effects from doing so?
I don't know the answer to either question without testing. All I am
saying is that currently the default for CONFIG_PM is "off" and you are
changing it to be "on" and there may not have been any testing done of
that in some situations. We don't know where it was explicitly
turned off any more since we shrank our defconfig files (which was done
automatically) ... since it is off by default, it doesn't need to be
mentioned in a defconfig unless it needs to be turned on.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Mark Brown @ 2011-02-07 14:18 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Len Brown, linux-embedded, Dmitry Torokhov, linux-kernel,
Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev, Andrew Morton
In-Reply-To: <20110208011324.d5371c4c.sfr@canb.auug.org.au>
On Tue, Feb 08, 2011 at 01:13:24AM +1100, Stephen Rothwell wrote:
> On Mon, 7 Feb 2011 12:22:15 +0000 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> > + bool
> > + default y if !IA64_HP_SIM
> Several powerpc configs have CONFIG_PM (implicitly) disabled (e.g. the
> server configs), so this will unexpectedly turn it on for them.
Do you mean that these systems require CONFIG_PM be turned off, or just
that people tend not to turn it on? If the latter would you expect any
ill effects from doing so?
^ permalink raw reply
* Re: [PATCH] PM: Hide CONFIG_PM from users
From: Stephen Rothwell @ 2011-02-07 14:13 UTC (permalink / raw)
To: Mark Brown
Cc: Len Brown, linux-embedded, Dmitry Torokhov, linux-kernel,
Rafael J. Wysocki, Alan Stern, linux-pm, ppc-dev, Andrew Morton
In-Reply-To: <1297081335-13631-1-git-send-email-broonie@opensource.wolfsonmicro.com>
[-- Attachment #1: Type: text/plain, Size: 1591 bytes --]
Hi Mark,
On Mon, 7 Feb 2011 12:22:15 +0000 Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
>
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 2657299..99e3c52 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -1,23 +1,6 @@
> config PM
> - bool "Power Management support"
> - depends on !IA64_HP_SIM
> - ---help---
> - "Power Management" means that parts of your computer are shut
> - off or put into a power conserving "sleep" mode if they are not
> - being used. There are two competing standards for doing this: APM
> - and ACPI. If you want to use either one, say Y here and then also
> - to the requisite support below.
> -
> - Power Management is most important for battery powered laptop
> - computers; if you have a laptop, check out the Linux Laptop home
> - page on the WWW at <http://www.linux-on-laptops.com/> or
> - Tuxmobil - Linux on Mobile Computers at <http://www.tuxmobil.org/>
> - and the Battery Powered Linux mini-HOWTO, available from
> - <http://www.tldp.org/docs.html#howto>.
> -
> - Note that, even if you say N here, Linux on the x86 architecture
> - will issue the hlt instruction if nothing is to be done, thereby
> - sending the processor to sleep and saving power.
> + bool
> + default y if !IA64_HP_SIM
Several powerpc configs have CONFIG_PM (implicitly) disabled (e.g. the
server configs), so this will unexpectedly turn it on for them.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH] sched: provide scheduler_ipi() callback in response to smp_send_reschedule()
From: Peter Zijlstra @ 2011-02-07 13:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-m32r-ja, linux-mips, linux-ia64, linux-sh, H. Peter Anvin,
Heiko Carstens, Paul Mackerras, Helge Deller, sparclinux,
Linux-Arch, linux-s390, Jesper Nilsson, Jeremy Fitzhardinge,
Russell King, Hirokazu Takata, x86, James E.J. Bottomley,
virtualization, Ingo Molnar, Matt Turner, Fenghua Yu,
Mike Frysinger, user-mode-linux-devel, Konrad Rzeszutek Wilk,
Jeff Dike, Chris Metcalf, xen-devel, Mikael Starvik, linux-m32r,
Ivan Kokshaysky, user-mode-linux-user, uclinux-dist-devel,
Thomas Gleixner, linux-arm-kernel, Richard Henderson, Tony Luck,
linux-parisc, linux-cris-kernel, linux-am33-list, linux-kernel,
Ralf Baechle, Kyle McMartin, Paul Mundt, linux-alpha,
Martin Schwidefsky, linux390, Koichi Yasutake, linuxppc-dev,
David S. Miller
In-Reply-To: <1297034792.14982.10.camel@pasglop>
On Mon, 2011-02-07 at 10:26 +1100, Benjamin Herrenschmidt wrote:
> You missed:
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 9813605..467d122 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -98,6 +98,7 @@ void smp_message_recv(int msg)
> break;
> case PPC_MSG_RESCHEDULE:
> /* we notice need_resched on exit */
> + scheduler_ipi();
> break;
> case PPC_MSG_CALL_FUNC_SINGLE:
> generic_smp_call_function_single_interrupt();
>
> Fold that in and add:
>
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Thanks Ben!
^ permalink raw reply
* Re: [PATCH V2 4/6] powerpc/44x: don't use tlbivax on AMP systems
From: David Gibson @ 2011-02-07 8:30 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: linuxppc-dev
In-Reply-To: <1296774957.14077.23.camel@shaggy-w500>
On Thu, Feb 03, 2011 at 05:15:57PM -0600, Dave Kleikamp wrote:
> On Thu, 2011-02-03 at 16:03 +1100, David Gibson wrote:
> > On Wed, Feb 02, 2011 at 05:53:59PM -0600, Dave Kleikamp wrote:
> > > On Thu, 2011-02-03 at 10:08 +1100, David Gibson wrote:
> > > > On Tue, Feb 01, 2011 at 12:48:44PM -0600, Dave Kleikamp wrote:
> > > > > Since other OS's may be running on the other cores don't use tlbivax
> > > >
> > > > [snip]
> > > > > +#ifdef CONFIG_44x
> > > > > +void __init early_init_mmu_44x(void)
> > > > > +{
> > > > > + unsigned long root = of_get_flat_dt_root();
> > > > > + if (of_flat_dt_is_compatible(root, "ibm,47x-AMP"))
> > > > > + amp = 1;
> > > > > +}
> > > > > +#endif /* CONFIG_44x */
> > > >
> > > > A test against a hardcoded compatible string seems a nasty way to do
> > > > this. Maybe we should define a new boolean property for the root
> > > > node.
> > >
> > > I'm not crazy about this string, but I needed something in the device
> > > tree to key off of. Freescale has something similar (i.e.
> > > MPC8572DS-CAMP), so I chose to follow their example. I'd be happy to
> > > replace it with a boolean property. Any objection to just using
> > > "amp"?
> >
> > Bit too short, I think. I'd suggest either spelling out
> > 'asymmetric-multiprocessor' or 'cooperative-partition' (a more
> > accurate term, IMO).
>
> I could be wrong, but I thought the A stands for Asynchronous, not
> Asymmetric. I thought Asymmetric means that different types of tasks
> run on the secondary processors, as on the Cell.
Yeah, I thought so too, but Freescale at least seem to use it this
way. 'Asynchronous' would make even less sense.
> Anyway, going with
> 'cooperative-partition' would avoid that confusion.
>
> Shaggy
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [PATCH V2 1/6] powerpc: Move udbg_early_init() after early_init_devtree()
From: David Gibson @ 2011-02-07 8:29 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: linuxppc-dev
In-Reply-To: <1296691225.12290.112.camel@shaggy-w500>
On Wed, Feb 02, 2011 at 06:00:25PM -0600, Dave Kleikamp wrote:
> On Thu, 2011-02-03 at 10:06 +1100, David Gibson wrote:
> > On Tue, Feb 01, 2011 at 12:48:41PM -0600, Dave Kleikamp wrote:
> > > so that it can use information from the device tree.
> >
> > Hrm. On the other hand this means that the early_init_devtree() code
> > can't benefit from hardcoded early debugging. Since you don't
> > actually appear to use devtree information in udbg_early_init() in the
> > latest series, I'd suggest dropping this patch.
>
> Patch 2 depends on early_init_devtree() being run. Until then, I don't
> know of a way to get at the bootargs.
Ah, yes. Drat.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply
* Re: [PATCH] powerpc: Fix call to flush_ptrace_hw_breakpoint()
From: Benjamin Herrenschmidt @ 2011-02-07 5:13 UTC (permalink / raw)
To: prasad; +Cc: linuxppc-dev, David Kleikamp
In-Reply-To: <20110207045230.GB4287@in.ibm.com>
On Mon, 2011-02-07 at 10:22 +0530, K.Prasad wrote:
> Okay! Another wrapper of "#ifndef CONFIG_HAVE_HW_BREAKPOINT" around
> the
> definition of 'set_debug_reg_defaults'.
Can you send a patch ?
> There's indeed too much sprinkling of #ifdefs in the code, but most of
> it would go away when the BookE code also uses the generic
> hw-breakpoint interfaces.
What's your status for those patches ?
> Given the advanced debug features that BookE supports, it's
> unfortunately not that straight-forward (needs additions to generic
> hw-breakpoint infrastructure).
Might want to move a lot of that code to a separate set of files maybe
and just call a single hook from the various process.c locations ?
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v2 3/3] powerpc: Add IO event interrupt support
From: Benjamin Herrenschmidt @ 2011-02-07 5:10 UTC (permalink / raw)
To: Tseng-Hui (Frank) Lin; +Cc: tsenglin, linuxppc-dev
In-Reply-To: <1294267483.24377.30.camel@flin.austin.ibm.com>
On Wed, 2011-01-05 at 16:44 -0600, Tseng-Hui (Frank) Lin wrote:
> This patch adds support for handling IO Event interrupts which come
> through at the /event-sources/ibm,io-events device tree node.
.../...
The previous patches regarding the error log sometimes call it error log
and sometimes event log. Can you consolidate that ?
> +int pseries_ioei_register_handler(pseries_ioei_handler_t handler)
> +{
Shouldn't we have a void * to attach with each client so it can put
its private data there and get them back in the handler ?
We are -really- re-inventing interrupts and/or notifiers here, which
I find a tad annoying..
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 2/6] nvram: Capture oops/panic reports in ibm, oops-log partition
From: Benjamin Herrenschmidt @ 2011-02-07 5:01 UTC (permalink / raw)
To: Jim Keniston; +Cc: linuxppc-dev
In-Reply-To: <20101114041521.9457.69485.stgit@localhost.localdomain>
On Sat, 2010-11-13 at 20:15 -0800, Jim Keniston wrote:
> Create the ibm,oops-log NVRAM partition, and capture the end of the printk
> buffer in it when there's an oops or panic. If we can't create the
> ibm,oops-log partition, capture the oops/panic report in ibm,rtas-log.
Won't the parsing tools choke on the oops log in the RTAS log
partition ?
Also, maybe remove the "ibm," prefix on the nvram partition, make it
"lnx," instead.
Then, if you move the rest of the code out of pseries, you can move that
out too, just make pseries platform code call the init code for it, that
way other platforms can re-use that as-is.
But that later part can wait a separate patch series, just make sure you
change the partition name now.
Cheers,
Ben.
> Signed-off-by: Jim Keniston <jkenisto@us.ibm.com>
> ---
>
> arch/powerpc/platforms/pseries/nvram.c | 89 ++++++++++++++++++++++++++++++++
> 1 files changed, 88 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
> index 43d5c52..6c88cda 100644
> --- a/arch/powerpc/platforms/pseries/nvram.c
> +++ b/arch/powerpc/platforms/pseries/nvram.c
> @@ -16,6 +16,8 @@
> #include <linux/errno.h>
> #include <linux/init.h>
> #include <linux/spinlock.h>
> +#include <linux/slab.h>
> +#include <linux/kmsg_dump.h>
> #include <asm/uaccess.h>
> #include <asm/nvram.h>
> #include <asm/rtas.h>
> @@ -50,11 +52,32 @@ static struct os_partition rtas_log_partition = {
> .index = -1
> };
>
> +static struct os_partition oops_log_partition = {
> + .name = "ibm,oops-log",
> + .req_size = 4000,
> + .min_size = 2000,
> + .index = -1
> +};
> +
> static const char *valid_os_partitions[] = {
> "ibm,rtas-log",
> + "ibm,oops-log",
> NULL
> };
>
> +static void oops_to_nvram(struct kmsg_dumper *dumper,
> + enum kmsg_dump_reason reason,
> + const char *old_msgs, unsigned long old_len,
> + const char *new_msgs, unsigned long new_len);
> +
> +static struct kmsg_dumper nvram_kmsg_dumper = {
> + .dump = oops_to_nvram
> +};
> +
> +/* We preallocate oops_buf during init to avoid kmalloc during oops/panic. */
> +static size_t oops_buf_sz;
> +static char *oops_buf;
> +
> static ssize_t pSeries_nvram_read(char *buf, size_t count, loff_t *index)
> {
> unsigned int i;
> @@ -337,9 +360,36 @@ static int __init pseries_nvram_init_os_partition(struct os_partition *part)
> return 0;
> }
>
> +static void __init nvram_init_oops_partition(int rtas_partition_exists)
> +{
> + int rc;
> +
> + rc = pseries_nvram_init_os_partition(&oops_log_partition);
> + if (rc != 0) {
> + if (!rtas_partition_exists)
> + return;
> + pr_notice("nvram: Using %s partition to log both"
> + " RTAS errors and oops/panic reports\n",
> + rtas_log_partition.name);
> + memcpy(&oops_log_partition, &rtas_log_partition,
> + sizeof(rtas_log_partition));
> + }
> + oops_buf_sz = oops_log_partition.size - sizeof(struct err_log_info);
> + oops_buf = kmalloc(oops_buf_sz, GFP_KERNEL);
> + rc = kmsg_dump_register(&nvram_kmsg_dumper);
> + if (rc != 0) {
> + pr_err("nvram: kmsg_dump_register() failed; returned %d\n", rc);
> + kfree(oops_buf);
> + return;
> + }
> +}
> +
> static int __init pseries_nvram_init_log_partitions(void)
> {
> - (void) pseries_nvram_init_os_partition(&rtas_log_partition);
> + int rc;
> +
> + rc = pseries_nvram_init_os_partition(&rtas_log_partition);
> + nvram_init_oops_partition(rc == 0);
> return 0;
> }
> machine_late_initcall(pseries, pseries_nvram_init_log_partitions);
> @@ -373,3 +423,40 @@ int __init pSeries_nvram_init(void)
>
> return 0;
> }
> +
> +/*
> + * Try to capture the last capture_len bytes of the printk buffer. Return
> + * the amount actually captured.
> + */
> +static size_t capture_last_msgs(const char *old_msgs, size_t old_len,
> + const char *new_msgs, size_t new_len,
> + char *captured, size_t capture_len)
> +{
> + if (new_len >= capture_len) {
> + memcpy(captured, new_msgs + (new_len - capture_len),
> + capture_len);
> + return capture_len;
> + } else {
> + /* Grab the end of old_msgs. */
> + size_t old_tail_len = min(old_len, capture_len - new_len);
> + memcpy(captured, old_msgs + (old_len - old_tail_len),
> + old_tail_len);
> + memcpy(captured + old_tail_len, new_msgs, new_len);
> + return old_tail_len + new_len;
> + }
> +}
> +
> +/* our kmsg_dump callback */
> +static void oops_to_nvram(struct kmsg_dumper *dumper,
> + enum kmsg_dump_reason reason,
> + const char *old_msgs, unsigned long old_len,
> + const char *new_msgs, unsigned long new_len)
> +{
> + static unsigned int oops_count = 0;
> + size_t text_len;
> +
> + text_len = capture_last_msgs(old_msgs, old_len, new_msgs, new_len,
> + oops_buf, oops_buf_sz);
> + (void) nvram_write_os_partition(&oops_log_partition, oops_buf,
> + (int) text_len, ERR_TYPE_KERNEL_PANIC, ++oops_count);
> +}
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox