* Re: [PATCH] [POWERPC] Add AMCC Glacier 460GT eval board dts
From: Stefan Roese @ 2008-03-21 10:54 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
In-Reply-To: <1206090031.8420.67.camel@pasglop>
On Friday 21 March 2008, Benjamin Herrenschmidt wrote:
> On Wed, 2008-03-19 at 17:15 +0100, Stefan Roese wrote:
> > + cpu@0 {
> > + device_type = "cpu";
> > + model = "PowerPC,460GT";
> > + reg = <0>;
>
> I wonder if we should do something here to differenciate the SoC chip
> from the core. After all, all those 4xx mostly have the same core (there
> are 2 or 3 revisions of the core maybe ...) but they tend to have all
> different PVR which is a pain and won't scale...
>
> Maybe AMCC could do something in HW (splitting the PVR from whatever
> indicates what "chip" it is, and keeping the PVR purely for the core
> rev) but I'm wondering if we should also do something in the DTS..
> Stefan can you talk to your AMCC contacts about this ?
Yes, I'll do that. Not sure about the outcome though.
> As for the DTS, maybe a "compatible" property in the CPU might make some
> sense with a content along the lines of "ppc440x6" or whatever rev of
> the 440 core it is.
>
> What do you think ?
Good idea. I'll try to come up with a list for all existing 4xx SoC's and it's
core versions.
Best regards,
Stefan
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
=====================================================================
^ permalink raw reply
* Re: crash in init_ipic_sysfs on efika
From: Benjamin Herrenschmidt @ 2008-03-21 11:18 UTC (permalink / raw)
To: Matt Sealey; +Cc: linuxppc-dev, Olaf Hering
In-Reply-To: <47E24B69.2080607@genesi-usa.com>
On Thu, 2008-03-20 at 11:32 +0000, Matt Sealey wrote:
> subsys_initcall(init_ipic_sysfs); <-- this, in my eyes, is the culprit. If
> init_ipic() runs, init_ipic_sysfs should be called from that, not left for
> some further subsystem to blindly try and register sysfs nodes for devices
> which may not even be present..
>
> I'd love someone to explain to me how this works and why, and why it isn't
> braindead? :D
init_ipic() is called way too early to be able to manpipulate sysfs.
Ben.
^ permalink raw reply
* Re: DTS question
From: Segher Boessenkool @ 2008-03-21 11:31 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, linuxppc-dev, Sean MacLennan
In-Reply-To: <fa686aa40803202134p7db6a106n8487aa0040b5990c@mail.gmail.com>
>> I wondered about this. Since the AD from Analog Devices is built into
>> the part number, I didn't know if it was needed. And analog-devices
>> is
>> pretty long ;)
>>
>> But I am willing to put it in if it is necessary.
>
> Convention is to use the stock ticker symbol. If the company is
> private and has no stock ticker symbol, then the company name should
> be used.
The three forms are, in order of preference:
0NNNNNN with NNNNNN the (hexadecimal) OUI;
ABCDE (uppercase stock symbol);
abcde (any name that cannot be confused for the other two forms;
usually by making it lowercase).
The preference is in this order because OUI _is_ unique, stock ticker
will likely be unique (but not always!), and for random names there
is no guarantee or assurance at all.
I've never actually seen the OUI used, it's not very user-friendly ;-)
Segher
^ permalink raw reply
* Re: DTS question
From: Segher Boessenkool @ 2008-03-21 11:35 UTC (permalink / raw)
To: David Gibson; +Cc: Scott Wood, linuxppc-dev, Sean MacLennan
In-Reply-To: <20080321070512.GA29010@localhost.localdomain>
>>> Convention is to use the stock ticker symbol. If the company is
>>> private and has no stock ticker symbol, then the company name should
>>> be used.
>>
>> I didn't know that. ADI it is then.
>
> Well.. stock ticker is the new convention. IEEE1275 used IEEE
> assigned OUI strings (Organization Unique Identifiers). Often those
> are the same as the stock ticker, but not always.
Erm, an OUI is a 24-bit number. I think you're confusing something
here.
> Stock ticker is a good choice for new things, but for anything from a
> vendor which has existing 1275 bindings for its products, I think we
> should keep the original assigned OUI, even if it differs from the
> stock ticker.
Yes, when there is an existing binding, obviously you should use what
it says (unless that binding is *completely* broken). Compatibility
is good.
Note that a stock symbol needs to be written in uppercase; in lowercase,
it is just a random name that has no collision protection.
Segher
^ permalink raw reply
* Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart 16550.
From: Segher Boessenkool @ 2008-03-21 11:39 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, John Linn
In-Reply-To: <18403.32257.725539.470771@cargo.ozlabs.ibm.com>
>> Personally, I'm not fond of this approach. There is already some
>> traction to using the reg-shift property to specify spacing, and I
>> think it would be appropriate to also define a reg-offset property to
>> handle the +3 offset and then let the xilinx 16550 nodes use those.
>
> Why do we need a reg-offset property when we can just add the offset
> to the appropriate word(s) in the reg property?
Because if you do that, the "reg" property cannot describe the full
register block (it misses the first few bytes). Not a huge problem
in practice, sure.
Segher
^ permalink raw reply
* Re: [PATCH] [POWERPC] Add AMCC Glacier 460GT eval board dts
From: Segher Boessenkool @ 2008-03-21 11:46 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev, Stefan Roese
In-Reply-To: <1206090031.8420.67.camel@pasglop>
> As for the DTS, maybe a "compatible" property in the CPU might make
> some
> sense with a content along the lines of "ppc440x6" or whatever rev of
> the 440 core it is.
Good idea, but please _also_ put the exact name in there, first; so
something like
compatible = "AMCC,PowerPC,460GT", "AMCC,ppc440x6";
That way, you can still probe for specific versions where necessary.
This is completely in line with the "generic naming" recommended
practice; the PowerPC binding predates that RP, it really needs to
be updated.
Any reason we keep the device_type here, btw?
Segher
^ permalink raw reply
* Re: [PATCH] [POWERPC] Add AMCC Glacier 460GT eval board dts
From: Benjamin Herrenschmidt @ 2008-03-21 11:51 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, Stefan Roese
In-Reply-To: <011f73a2a2f33e075c20f0d0bb42d4a7@kernel.crashing.org>
On Fri, 2008-03-21 at 12:46 +0100, Segher Boessenkool wrote:
> > As for the DTS, maybe a "compatible" property in the CPU might make
> > some
> > sense with a content along the lines of "ppc440x6" or whatever rev of
> > the 440 core it is.
>
> Good idea, but please _also_ put the exact name in there, first; so
> something like
>
> compatible = "AMCC,PowerPC,460GT", "AMCC,ppc440x6";
>
> That way, you can still probe for specific versions where necessary.
> This is completely in line with the "generic naming" recommended
> practice; the PowerPC binding predates that RP, it really needs to
> be updated.
>
> Any reason we keep the device_type here, btw?
BTW, Should we sort compatible from the most specific to the most
specific or the other way around ?
Ben.
^ permalink raw reply
* Re: [PATCH] [POWERPC] Add AMCC Glacier 460GT eval board dts
From: Segher Boessenkool @ 2008-03-21 11:58 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev, Stefan Roese
In-Reply-To: <1206100302.8420.71.camel@pasglop>
> BTW, Should we sort compatible from the most specific to the most
> specific or the other way around ?
Most specific first.
Segher
^ permalink raw reply
* RE: [PATCH] ucc_geth: Add 8 bytes to max TX frame for VLANs
From: Li Yang @ 2008-03-21 12:20 UTC (permalink / raw)
To: Joakim Tjernlund, Netdev, Linuxppc-Embedded@Ozlabs.Org
In-Reply-To: <1205853088-30171-1-git-send-email-Joakim.Tjernlund@transmode.se>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org=20
> [mailto:netdev-owner@vger.kernel.org] On Behalf Of Joakim Tjernlund
> Sent: Tuesday, March 18, 2008 11:11 PM
> To: Netdev; Li Yang
> Cc: Joakim Tjernlund
> Subject: [PATCH] ucc_geth: Add 8 bytes to max TX frame for VLANs
>=20
> Creating a VLAN interface on top of ucc_geth adds 4 bytes to=20
> the frame and the HW controller is not prepared to TX a frame=20
> bigger than 1518 bytes which is 4 bytes too small for a full=20
> VLAN frame. Also add 4 extra bytes for future expansion.
IMO, VLAN and Jumbo packet support is not general case of Ethernet.
Could you make this change optional? Thanks.
- Leo
>=20
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
> drivers/net/ucc_geth.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>=20
> diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c=20
> index 348892b..038ec75 100644
> --- a/drivers/net/ucc_geth.c
> +++ b/drivers/net/ucc_geth.c
> @@ -114,10 +114,10 @@ static struct ucc_geth_info=20
> ugeth_primary_info =3D {
> .maxGroupAddrInHash =3D 4,
> .maxIndAddrInHash =3D 4,
> .prel =3D 7,
> - .maxFrameLength =3D 1518,
> + .maxFrameLength =3D 1518+8, /* Add 4 bytes for VLAN tags=20
> and 4 extra=20
> +bytes */
> .minFrameLength =3D 64,
> - .maxD1Length =3D 1520,
> - .maxD2Length =3D 1520,
> + .maxD1Length =3D 1520+8,
> + .maxD2Length =3D 1520+8,
> .vlantype =3D 0x8100,
> .ecamptr =3D ((uint32_t) NULL),
> .eventRegMask =3D UCCE_OTHER,
> --
> 1.5.4.3
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe=20
> netdev" in the body of a message to majordomo@vger.kernel.org=20
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>=20
^ permalink raw reply
* Re: [PATCH] [POWERPC] Add AMCC Glacier 460GT eval board dts
From: Josh Boyer @ 2008-03-21 12:24 UTC (permalink / raw)
To: Stefan Roese; +Cc: linuxppc-dev
In-Reply-To: <200803211154.34027.sr@denx.de>
On Fri, 21 Mar 2008 11:54:33 +0100
Stefan Roese <sr@denx.de> wrote:
> On Friday 21 March 2008, Benjamin Herrenschmidt wrote:
> > On Wed, 2008-03-19 at 17:15 +0100, Stefan Roese wrote:
> > > + cpu@0 {
> > > + device_type = "cpu";
> > > + model = "PowerPC,460GT";
> > > + reg = <0>;
> >
> > I wonder if we should do something here to differenciate the SoC chip
> > from the core. After all, all those 4xx mostly have the same core (there
> > are 2 or 3 revisions of the core maybe ...) but they tend to have all
> > different PVR which is a pain and won't scale...
> >
> > Maybe AMCC could do something in HW (splitting the PVR from whatever
> > indicates what "chip" it is, and keeping the PVR purely for the core
> > rev) but I'm wondering if we should also do something in the DTS..
> > Stefan can you talk to your AMCC contacts about this ?
>
> Yes, I'll do that. Not sure about the outcome though.
>
> > As for the DTS, maybe a "compatible" property in the CPU might make some
> > sense with a content along the lines of "ppc440x6" or whatever rev of
> > the 440 core it is.
> >
> > What do you think ?
>
> Good idea. I'll try to come up with a list for all existing 4xx SoC's and it's
> core versions.
I don't really care either way, but what does that buy us? Merely
documentation?
josh
^ permalink raw reply
* Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart 16550.
From: Sergei Shtylyov @ 2008-03-21 13:00 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, John Linn
In-Reply-To: <fa686aa40803201719i7bf02ba2qd259f1a36a24d943@mail.gmail.com>
Hello.
Grant Likely wrote:
> On Thu, Mar 20, 2008 at 8:43 AM, John Linn <john.linn@xilinx.com> wrote:
>>The Xilinx 16550 uart core is not a standard 16550, because it uses
>> word-based addressing rather than byte-based addressing. As a result,
>> it is not compatible with the open firmware 'ns16550' compatible
>> binding. This code introduces new bindings, which pass the correct
>> register base and regshift properties to the uart driver to enable
>> this core to be used. Doing this cleanly required some refactoring of
>> the existing code.
> Personally, I'm not fond of this approach. There is already some
> traction to using the reg-shift property to specify spacing, and I
> think it would be appropriate to also define a reg-offset property to
> handle the +3 offset and then let the xilinx 16550 nodes use those.
That's making things only worse than the mere "reg-shift" idea. I think
that both are totally wrong. Everything about the programming interface should
be said in the "compatible" and possibly "model" properties. of_serial driver
should recognize them and pass the necessary details to 8250.c. As for me, I'm
strongly against plaguing the device tree with the *Linux driver
implementation specifics* (despite I was trying this with MTD -- there it
seemed somewhat more grounded :-).
> More comments below.
>> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
>> Signed-off-by: John Linn <john.linn@xilinx.com>
>> diff --git a/drivers/serial/of_serial.c b/drivers/serial/of_serial.c
>> index 2efb892..910c94f 100644
>> --- a/drivers/serial/of_serial.c
>> +++ b/drivers/serial/of_serial.c
[...]
>> return 0;
>> }
>> @@ -81,8 +82,9 @@ static int __devinit of_platform_serial_probe(struct of_device *ofdev,
>> if (info == NULL)
>> return -ENOMEM;
>>
>> - port_type = (unsigned long)id->data;
>> - ret = of_platform_serial_setup(ofdev, port_type, &port);
>> + memcpy(info, id->data, sizeof(struct of_serial_info));
>> + port_type = info->type;
>> + ret = of_platform_serial_setup(ofdev, info, &port);
>> if (ret)
>> goto out;
>>
>> @@ -100,7 +102,6 @@ static int __devinit of_platform_serial_probe(struct of_device *ofdev,
>> if (ret < 0)
>> goto out;
>>
>> - info->type = port_type;
>> info->line = ret;
>> ofdev->dev.driver_data = info;
>> return 0;
>> @@ -128,15 +129,33 @@ static int of_platform_serial_remove(struct of_device *ofdev)
>> return 0;
>> }
>>
>> +static struct of_serial_info __devinitdata ns8250_info = { .type = PORT_8250 };
>> +static struct of_serial_info __devinitdata ns16450_info = { .type = PORT_16450 };
>> +static struct of_serial_info __devinitdata ns16550_info = { .type = PORT_16550 };
>> +static struct of_serial_info __devinitdata ns16750_info = { .type = PORT_16750 };
>> +static struct of_serial_info __devinitdata xilinx_16550_info = {
>> + .type = PORT_16550,
>> + .regshift = 2,
>> + .regoffset = 3,
I see that the data is already encoded in the driver itself, so I agree
with the patch.
>> +};
>> +static struct of_serial_info __devinitdata unknown_info = { .type = PORT_UNKNOWN };
> In support of my argument; the fact that you need a table of data says
> to me that this data should really be encoded in the device tree. :-)
Not at all.
> Cheers,
> g.
WBR, Sergei
^ permalink raw reply
* Re: crash in init_ipic_sysfs on efika
From: Matt Sealey @ 2008-03-21 13:14 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, Olaf Hering
In-Reply-To: <18403.31214.826941.838442@cargo.ozlabs.ibm.com>
Is the MPC5200B PSC-AC97 driver in there?
--
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations
Paul Mackerras wrote:
> Grant Likely writes:
>
>> Confirmed, this patch fixes the problem. Paulus or Kumar, can you
>> please pick it up for .25?
>
> Sure, will do. I thought about putting it in the last batch but I
> wanted an ack from you.
>
> Anyone else got any last-minute things for 2.6.25?
>
> Paul.
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH 6/8] ptrace: arch_ptrace -ENOSYS return
From: Christoph Hellwig @ 2008-03-21 14:07 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-arch, tony.luck, linux-ia64, linux-kernel, David Miller,
linuxppc-dev, Paul Mackerras, sparclinux, Andrew Morton,
Linus Torvalds, Ingo Molnar, Roland McGrath, Richard Henderson
In-Reply-To: <alpine.LFD.1.00.0803211441180.3781@apollo.tec.linutronix.de>
On Fri, Mar 21, 2008 at 02:50:01PM +0100, Thomas Gleixner wrote:
> What about adding a CONFIG_ARCH_HAS_PTRACE2, which is set by the archs
> which are converted. For those which are not you add a fallback
> implementation:
Bah. Folks, we're talking about adding a single new argument to a
single function implemented by our 24 architectures. This is a trivial
almost scriptable conversion.
^ permalink raw reply
* Re: [PATCH 6/8] ptrace: arch_ptrace -ENOSYS return
From: Sam Ravnborg @ 2008-03-21 14:10 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-arch, tony.luck, linux-ia64, linux-kernel, David Miller,
linuxppc-dev, Paul Mackerras, sparclinux, Andrew Morton,
Linus Torvalds, Ingo Molnar, Roland McGrath, Richard Henderson
In-Reply-To: <alpine.LFD.1.00.0803211441180.3781@apollo.tec.linutronix.de>
On Fri, Mar 21, 2008 at 02:50:01PM +0100, Thomas Gleixner wrote:
> On Thu, 20 Mar 2008, Roland McGrath wrote:
> > > Wouldn't it be nicer to just let "arch_ptrace()" return a flag saying
> > > whether it handled things or not?
> >
> > It would certainly be nicer. I would prefer:
> >
> > extern int arch_ptrace(struct task_struct *child, long request,
> > long addr, long data, long *retval);
> >
> > where it returns an error code or it returns 0 and *retval is the value
> > or it returns 1 and it didn't do anything.
> >
> > So this ugliness seemed like a better bet than waiting for 20 more
> > arch sign-offs before any of it could go in. You are certainly in a
> > position to just change the generic signature and make every arch do
> > the update (or fix your typos if you just tweak them all blind), and
> > let them grumble. I did not presume to do so.
>
> What about adding a CONFIG_ARCH_HAS_PTRACE2, which is set by the archs
> which are converted. For those which are not you add a fallback
> implementation:
HAVE_PTRACE2 or at least following the HAVE_* semnatic.
And then do:
config HAVE_PTRACE2
def_bool n
In some common file.
Then arch files can do:
config X86
...
+ select HAVE_PTRACE2
Sam
^ permalink raw reply
* Re: [PATCH 6/8] ptrace: arch_ptrace -ENOSYS return
From: Thomas Gleixner @ 2008-03-21 13:50 UTC (permalink / raw)
To: Roland McGrath
Cc: linux-arch, tony.luck, linux-ia64, linux-kernel, David Miller,
linuxppc-dev, Paul Mackerras, sparclinux, Andrew Morton,
Linus Torvalds, Ingo Molnar, Richard Henderson
In-Reply-To: <20080320081658.0B23826F995@magilla.localdomain>
On Thu, 20 Mar 2008, Roland McGrath wrote:
> > Wouldn't it be nicer to just let "arch_ptrace()" return a flag saying
> > whether it handled things or not?
>
> It would certainly be nicer. I would prefer:
>
> extern int arch_ptrace(struct task_struct *child, long request,
> long addr, long data, long *retval);
>
> where it returns an error code or it returns 0 and *retval is the value
> or it returns 1 and it didn't do anything.
>
> So this ugliness seemed like a better bet than waiting for 20 more
> arch sign-offs before any of it could go in. You are certainly in a
> position to just change the generic signature and make every arch do
> the update (or fix your typos if you just tweak them all blind), and
> let them grumble. I did not presume to do so.
What about adding a CONFIG_ARCH_HAS_PTRACE2, which is set by the archs
which are converted. For those which are not you add a fallback
implementation:
#ifndef CONFIG_ARCH_HAS_PTRACE2
static int arch_ptrace2(whatever your favourite interface)
{
ret = arch_ptrace();
return do_ugly_fixups(ret);
}
#endif
That way you introduce the new interface and convert one or two archs
initialy without breaking the other 22.
At the same time you mark arch_ptrace() deprecated so it will get the
attention of the arch maintainers pretty fast. Once all archs are
converted we can remove the config flag and the fallback quirk.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH 6/8] ptrace: arch_ptrace -ENOSYS return
From: Linus Torvalds @ 2008-03-21 14:55 UTC (permalink / raw)
To: Roland McGrath
Cc: linux-arch, tony.luck, linux-ia64, linux-kernel, David Miller,
linuxppc-dev, Thomas Gleixner, sparclinux, Paul Mackerras,
Andrew Morton, Ingo Molnar, Richard Henderson
In-Reply-To: <20080320081658.0B23826F995@magilla.localdomain>
On Thu, 20 Mar 2008, Roland McGrath wrote:
>
> The motivation is to get the arch function out of the code path for the
> machine-independent request handling. I want to be able to change the
> implementation later without touching the arch code again.
If there is no stronger motivation than that, then I don't want to have
this ugly and unnecessary complication.
I really don't see the advantage of doing
/* We can't handle it, let the generic code sort it out */
return -ENOSYS;
over a
/* We can't handle it, let the generic code sort it out */
return ptrace_request(child, request, addr, data);
in the arch-specific code, and I think the latter version is *much*
preferable if it avoids this whole unnecessary new abstraction crud and
odd testing in the generic part.
> The reason I took the approach I did instead is incrementalism.
> I can't change that signature without breaking about 22 arch builds.
Don't worry about the arch builds. If that's your main worry and reason
for this, it's not worth it. Yes, ptrace changes, and yes, we may have
arch issues, but no, it's not that big of a deal. Just break them.
Make sure x86[-64] works, and make sure that other architectures *can*
work (and explain it on linux-arch) when you have to fix something up, but
ptrace is a blip on the radar for people, it's not going to be a huge
issue.
> I'm only really prepared to thoroughly verify a change on 2 of those
> myself.
.. and you really aren't expected to. It's why we have architecture
people: if there is a good reason for the change (ie it's not just some
churn for churns sake), it's largely up to them to make sure the code
works.
Sure, you need to be able to explain the interface changes and answer
questions, but as mentioned, the ptrace code is not a big deal: it has
lots of tiny _details_, but it's not conceptually complex code.
> So this ugliness seemed like a better bet than waiting for 20 more
> arch sign-offs before any of it could go in.
Really, that has never been a major concern. I will _happily_ break the
odd architectures if I see that it's a matter of changing over some
detail (ie it's not some fundamentally hard issue for an arch maintaner to
fix up).
I think it's *wrong* to add a new an odd calling convention that is less
readable than what we have now, just to try to avoid something like this
that really isn't a big problem. It's not like we haven't broken
architectures over ptrace in the past, and it's also not like it's an area
that I think anybody has ever lost sleep over..
Linus
^ permalink raw reply
* Re: [PATCH 1/5] generic __remove_pages() support
From: Yasunori Goto @ 2008-03-21 15:25 UTC (permalink / raw)
To: Badari Pulavarty; +Cc: linuxppc-dev, Andrew Morton, paulus, lkml
In-Reply-To: <1204829734.7939.61.camel@dyn9047017100.beaverton.ibm.com>
Badari-san.
> Index: linux-2.6.25-rc2/mm/memory_hotplug.c
> ===================================================================
> --- linux-2.6.25-rc2.orig/mm/memory_hotplug.c 2008-02-27 12:58:17.000000000 -0800
> +++ linux-2.6.25-rc2/mm/memory_hotplug.c 2008-02-27 16:06:50.000000000 -0800
> @@ -102,6 +102,21 @@ static int __add_section(struct zone *zo
> return register_new_memory(__pfn_to_section(phys_start_pfn));
> }
>
> +static int __remove_section(struct zone *zone, struct mem_section *ms)
> +{
> + int ret = -EINVAL;
> +
> + if (!valid_section(ms))
> + return ret;
> +
> + ret = unregister_memory_section(ms);
> + if (ret)
> + return ret;
> +
> + sparse_remove_one_section(zone, ms);
> + return 0;
> +}
> +
> /*
> * Reasonably generic function for adding memory. It is
> * expected that archs that support memory hotplug will
> @@ -135,6 +150,35 @@ int __add_pages(struct zone *zone, unsig
> }
> EXPORT_SYMBOL_GPL(__add_pages);
>
> +int __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
> + unsigned long nr_pages)
> +{
> + unsigned long i, ret = 0;
> + int sections_to_remove;
> + unsigned long flags;
> + struct pglist_data *pgdat = zone->zone_pgdat;
> +
> + /*
> + * We can only remove entire sections
> + */
> + BUG_ON(phys_start_pfn & ~PAGE_SECTION_MASK);
> + BUG_ON(nr_pages % PAGES_PER_SECTION);
> +
> + release_mem_region(phys_start_pfn << PAGE_SHIFT, nr_pages * PAGE_SIZE);
> +
> + sections_to_remove = nr_pages / PAGES_PER_SECTION;
> + for (i = 0; i < sections_to_remove; i++) {
> + unsigned long pfn = phys_start_pfn + i*PAGES_PER_SECTION;
> + pgdat_resize_lock(pgdat, &flags);
> + ret = __remove_section(zone, __pfn_to_section(pfn));
> + pgdat_resize_unlock(pgdat, &flags);
> + if (ret)
> + break;
> + }
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(__remove_pages);
Here may be a bug.
__remove_section() is called with pgdat_resize_lock() which is
spin_lock_irqsave().
__remove_section()
|
+--> unregister_memory_section()
|
+--> remove_memory_block()
|
+--> unregister_memory()
|
+--> sysdev_unregister()
sysdev_unregister() calls mutex_lock().
It might sleep with irq disable, right?
I found BUG()'s messages by this.
Bye.
--
Yasunori Goto
^ permalink raw reply
* Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart 16550.
From: Segher Boessenkool @ 2008-03-21 15:37 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev, John Linn
In-Reply-To: <47E3B189.6060002@ru.mvista.com>
>> Personally, I'm not fond of this approach. There is already some
>> traction to using the reg-shift property to specify spacing, and I
>> think it would be appropriate to also define a reg-offset property to
>> handle the +3 offset and then let the xilinx 16550 nodes use those.
>
> That's making things only worse than the mere "reg-shift" idea. I
> think that both are totally wrong. Everything about the programming
> interface should be said in the "compatible" and possibly "model"
> properties.
No. In effect, you are saying here that no device binding should define
any binding-specific properties. This will just lead to combinatorial
explosion of "compatible" values.
That said, "reg-spacing"/"reg-shift"/"reg-offset" should *not* be
considered something generic; they are part of specific device
bindings. Of course it is nice if various bindings use the same
names for the same concepts, but that's an orthogonal issue.
> of_serial driver should recognize them and pass the necessary details
> to 8250.c. As for me, I'm strongly against plaguing the device tree
> with the *Linux driver implementation specifics*
"reg-*" has nothing to do with Linux device driver implementation
issues: it describes how a device is physically wired up!
> (despite I was trying this with MTD -- there it seemed somewhat more
> grounded :-).
Quite the opposite, but let's not rehash that discussion.
>> In support of my argument; the fact that you need a table of data says
>> to me that this data should really be encoded in the device tree. :-)
>
> Not at all.
Not _necessarily_. I agree with Grant here: for many of these devices
with byte-size registers, it is very common to find them with their
register banks wired up differently, and that is often the *only*
difference to the "normal" device. In this situation, it makes a lot
of sense to describe that difference with "reg-*" properties.
In some other situations, it is better to create a new binding for
the device.
Segher
^ permalink raw reply
* Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart 16550.
From: Sergei Shtylyov @ 2008-03-21 15:54 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, John Linn
In-Reply-To: <75a17dc1bd4e99a473ed679ccf9b210f@kernel.crashing.org>
Hello.
Segher Boessenkool wrote:
>>> Personally, I'm not fond of this approach. There is already some
>>> traction to using the reg-shift property to specify spacing, and I
>>> think it would be appropriate to also define a reg-offset property to
>>> handle the +3 offset and then let the xilinx 16550 nodes use those.
>> That's making things only worse than the mere "reg-shift" idea. I
>> think that both are totally wrong. Everything about the programming
>> interface should be said in the "compatible" and possibly "model"
>> properties.
> No. In effect, you are saying here that no device binding should define
> any binding-specific properties. This will just lead to combinatorial
> explosion of "compatible" values.
> That said, "reg-spacing"/"reg-shift"/"reg-offset" should *not* be
> considered something generic; they are part of specific device
> bindings. Of course it is nice if various bindings use the same
> names for the same concepts, but that's an orthogonal issue.
The proposed use clearly would treat them as generic, since in the context
of the Xilinx UART they're just not needed -- it's known beforehand and most
probably fixed how/where the registers are mapped. There's just no need for
such info in the device tree -- unless you're going to teach the *generic*
driver to handle this specific (and possibly others alike) kind of a device.
>> of_serial driver should recognize them and pass the necessary details
>> to 8250.c. As for me, I'm strongly against plaguing the device tree
>> with the *Linux driver implementation specifics*
> "reg-*" has nothing to do with Linux device driver implementation
> issues: it describes how a device is physically wired up!
Hm... wasn't that you who were telling that use of "range" properties
guarantees 1:1 correspondence of the upstream/downstream bus addresses (in
their LSB part of course -- meaning that the device registers 0..x are seen by
the CPU at addresses base+0..base+X?
>> (despite I was trying this with MTD -- there it seemed somewhat more
>> grounded :-).
> Quite the opposite, but let's not rehash that discussion.
I might be mixing with the February thread about this UART -- have
alsready forgotten about it.
>>> In support of my argument; the fact that you need a table of data says
>>> to me that this data should really be encoded in the device tree. :-)
>> Not at all.
> Not _necessarily_. I agree with Grant here: for many of these devices
> with byte-size registers, it is very common to find them with their
> register banks wired up differently, and that is often the *only*
> difference to the "normal" device. In this situation, it makes a lot
> of sense to describe that difference with "reg-*" properties.
Note that "compicated" mapping is not (necessarily) a property of the
device itself but generally a property of the chip select circuit, i.e.
external entity.
> In some other situations, it is better to create a new binding for
> the device.
> Segher
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 1/5] generic __remove_pages() support
From: Badari Pulavarty @ 2008-03-21 16:55 UTC (permalink / raw)
To: Yasunori Goto; +Cc: linuxppc-dev, Andrew Morton, paulus, lkml
In-Reply-To: <20080321235322.2B17.E1E9C6FF@jp.fujitsu.com>
On Sat, 2008-03-22 at 00:25 +0900, Yasunori Goto wrote:
> Badari-san.
>
> > Index: linux-2.6.25-rc2/mm/memory_hotplug.c
> > ===================================================================
> > --- linux-2.6.25-rc2.orig/mm/memory_hotplug.c 2008-02-27 12:58:17.000000000 -0800
> > +++ linux-2.6.25-rc2/mm/memory_hotplug.c 2008-02-27 16:06:50.000000000 -0800
> > @@ -102,6 +102,21 @@ static int __add_section(struct zone *zo
> > return register_new_memory(__pfn_to_section(phys_start_pfn));
> > }
> >
> > +static int __remove_section(struct zone *zone, struct mem_section *ms)
> > +{
> > + int ret = -EINVAL;
> > +
> > + if (!valid_section(ms))
> > + return ret;
> > +
> > + ret = unregister_memory_section(ms);
> > + if (ret)
> > + return ret;
> > +
> > + sparse_remove_one_section(zone, ms);
> > + return 0;
> > +}
> > +
> > /*
> > * Reasonably generic function for adding memory. It is
> > * expected that archs that support memory hotplug will
> > @@ -135,6 +150,35 @@ int __add_pages(struct zone *zone, unsig
> > }
> > EXPORT_SYMBOL_GPL(__add_pages);
> >
> > +int __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
> > + unsigned long nr_pages)
> > +{
> > + unsigned long i, ret = 0;
> > + int sections_to_remove;
> > + unsigned long flags;
> > + struct pglist_data *pgdat = zone->zone_pgdat;
> > +
> > + /*
> > + * We can only remove entire sections
> > + */
> > + BUG_ON(phys_start_pfn & ~PAGE_SECTION_MASK);
> > + BUG_ON(nr_pages % PAGES_PER_SECTION);
> > +
> > + release_mem_region(phys_start_pfn << PAGE_SHIFT, nr_pages * PAGE_SIZE);
> > +
> > + sections_to_remove = nr_pages / PAGES_PER_SECTION;
> > + for (i = 0; i < sections_to_remove; i++) {
> > + unsigned long pfn = phys_start_pfn + i*PAGES_PER_SECTION;
> > + pgdat_resize_lock(pgdat, &flags);
> > + ret = __remove_section(zone, __pfn_to_section(pfn));
> > + pgdat_resize_unlock(pgdat, &flags);
> > + if (ret)
> > + break;
> > + }
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(__remove_pages);
>
> Here may be a bug.
> __remove_section() is called with pgdat_resize_lock() which is
> spin_lock_irqsave().
>
> __remove_section()
> |
> +--> unregister_memory_section()
> |
> +--> remove_memory_block()
> |
> +--> unregister_memory()
> |
> +--> sysdev_unregister()
>
> sysdev_unregister() calls mutex_lock().
> It might sleep with irq disable, right?
> I found BUG()'s messages by this.
Hmm. I think you are right. Let me take a look
and see if we can move sysdev_unregister() outside
the lock.
Thanks for finding it.
Thanks,
Badari
^ permalink raw reply
* RE: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart16550.
From: Stephen Neuendorffer @ 2008-03-21 16:08 UTC (permalink / raw)
To: Segher Boessenkool, Paul Mackerras; +Cc: John Linn, linuxppc-dev
In-Reply-To: <82ce3dbb9ebdb1fa4fc7407ed3396411@kernel.crashing.org>
I thought about this, and reconsidered it again after I finally figured
out how to get the Power.org website to relenquish the ePAPR spec, which
(FWIW) has reg-shift as an optional property of the ns16550 binding.
However, I'm not enamoured of it, since I doubt it's really good to be
doing ioremaps and memory management on unaligned blocks. It seems more
logical to me to have the reg property define a range of aligned
addresses, and in this case, it happens that the driver never touches
some of those bytes.
In any event, the real point of the patch was to spark some discussion,
which we failed to get otherwise.. :) I think any of these proposals
are workable...
Steve
> -----Original Message-----
> From: =
linuxppc-dev-bounces+stephen.neuendorffer=3Dxilinx.com@ozlabs.org
[mailto:linuxppc-dev-
> bounces+stephen.neuendorffer=3Dxilinx.com@ozlabs.org] On Behalf Of
Segher Boessenkool
> Sent: Friday, March 21, 2008 4:40 AM
> To: Paul Mackerras
> Cc: linuxppc-dev@ozlabs.org; John Linn
> Subject: Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for
Xilinx uart16550.
>=20
> >> Personally, I'm not fond of this approach. There is already some
> >> traction to using the reg-shift property to specify spacing, and I
> >> think it would be appropriate to also define a reg-offset property
to
> >> handle the +3 offset and then let the xilinx 16550 nodes use those.
> >
> > Why do we need a reg-offset property when we can just add the offset
> > to the appropriate word(s) in the reg property?
>=20
> Because if you do that, the "reg" property cannot describe the full
> register block (it misses the first few bytes). Not a huge problem
> in practice, sure.
>=20
>=20
> Segher
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH 6/8] ptrace: arch_ptrace -ENOSYS return
From: Ingo Molnar @ 2008-03-21 15:07 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-arch, tony.luck, linux-ia64, linux-kernel, David Miller,
linuxppc-dev, Paul Mackerras, sparclinux, Andrew Morton,
Thomas Gleixner, Roland McGrath, Richard Henderson
In-Reply-To: <alpine.LFD.1.00.0803210744470.3020@woody.linux-foundation.org>
* Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > The reason I took the approach I did instead is incrementalism. I
> > can't change that signature without breaking about 22 arch builds.
>
> Don't worry about the arch builds. If that's your main worry and
> reason for this, it's not worth it. Yes, ptrace changes, and yes, we
> may have arch issues, but no, it's not that big of a deal. Just break
> them.
>
> Make sure x86[-64] works, and make sure that other architectures *can*
> work (and explain it on linux-arch) when you have to fix something up,
> but ptrace is a blip on the radar for people, it's not going to be a
> huge issue.
for a long time all the nice but intrusive utrace improvements from
Roland had this big adoption barrier. So Roland went around that and
with a lot of effort he made it optional, incremental and per arch, so
that we could try it on x86 and merge it upstream.
Now that we see how cleaner it is and that it actually was an almost
regression-free endevour on x86 (we had 2 regressions so far, both fixed
- which is an amazing feat for such wide changes IMO), i very much agree
that we should just do the "rest" of this in one big step.
it's a bit of a chicken & egg problem for such changes. If it breaks
architectures it gets dropped out of -mm - even though 90% of our
developers, 95% of our testers and 99% of our active users are using x86
only.
but in general, prototyping something on a single architecture in the
first step is OK - and maybe even the first merge is OK. But once it has
been proven on the most tested Linux platform that we have (and there's
no blatant x86-ism in it), there's no reason not to mandate it.
Ingo
^ permalink raw reply
* Re: [PATCH 1/2 v2] Driver for Freescale 8610 and 5121 DIU
From: Timur Tabi @ 2008-03-21 16:12 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-fbdev-devel, Peter Zijlstra, linux-kernel, linuxppc-dev,
York Sun
In-Reply-To: <20080320152708.23c6c734.akpm@linux-foundation.org>
Andrew Morton wrote:
>> +static struct diu_hw dr = {
>> + .mode = MFB_MODE1,
>> + .reg_lock = __SPIN_LOCK_UNLOCKED(old_style_spin_init),
>> +};
>
> I'm not clear on what's supposed to happen with __SPIN_LOCK_UNLOCKED(). I
> do know that its documentation is crap.
Yes, "__SPIN_LOCK_UNLOCKED(old_style_spin_init)" is wrong. We'll fix it.
> static struct diu_hw dr = {
> .mode = MFB_MODE1,
> - .reg_lock = __SPIN_LOCK_UNLOCKED(old_style_spin_init),
> + .reg_lock = __SPIN_LOCK_UNLOCKED(diu_hw.reg_lock),
> };
Yes, this is better. Did you already make this change when you applied it to
your -mm repo?
> GFP_DMA implies GFP_ATOMIC, but it's appropriate for documentation purposes.
So does that mean that "GFP_DMA | GFP_KERNEL" is always wrong? If so, this
combination is used a lot in the kernel today.
>> + if (virt) {
>> + *phys = virt_to_phys(virt);
>> + pr_debug("virt %p, phys=%llx\n", virt, (uint64_t) *phys);
>> + memset(virt, 0, size);
>
> Could have used __GFP_ZERO, I guess.
I had completely forgotten about __GFP_ZERO. Thanks.
>> + virt = (void *) rh_alloc(&diu_ops.diu_rh_info, size, "DIU");
>
> hm, I'd have expected checkpatch to whine about the space after the cast
> there. Whatever.
I thought a space after a cast is the right thing to do?
> please take a look, and please use checkpatch on all future patches.
Sorry, we forgot to run it again after our second version of the patch.
>> +static void free_irq_local(int irq)
>> +{
>> + struct diu *hw = dr.diu_reg;
>> +
>> + /* Disable all LCDC interrupt */
>> + out_be32(&(hw->int_mask), 0x1f);
>> +
>> + free_irq(irq, 0);
>> +}
>
> and the free_irq() will go splat?
Sorry, but I don't understand what's wrong with this code.
We'll make the other changes you've suggested and repost.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* RE: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart16550.
From: Stephen Neuendorffer @ 2008-03-21 16:14 UTC (permalink / raw)
To: Grant Likely, John Linn; +Cc: linuxppc-dev
In-Reply-To: <fa686aa40803201719i7bf02ba2qd259f1a36a24d943@mail.gmail.com>
> -----Original Message-----
> From: =
linuxppc-dev-bounces+stephen.neuendorffer=3Dxilinx.com@ozlabs.org
[mailto:linuxppc-dev-
> bounces+stephen.neuendorffer=3Dxilinx.com@ozlabs.org] On Behalf Of =
Grant
Likely
> Sent: Thursday, March 20, 2008 5:19 PM
> To: John Linn
> Cc: linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for
Xilinx uart16550.
>=20
> On Thu, Mar 20, 2008 at 8:43 AM, John Linn <john.linn@xilinx.com>
wrote:
> > The Xilinx 16550 uart core is not a standard 16550, because it uses
> > word-based addressing rather than byte-based addressing. As a
result,
> > it is not compatible with the open firmware 'ns16550' compatible
> > binding. This code introduces new bindings, which pass the correct
> > register base and regshift properties to the uart driver to enable
> > this core to be used. Doing this cleanly required some refactoring
of
> > the existing code.
>=20
> Personally, I'm not fond of this approach. There is already some
> traction to using the reg-shift property to specify spacing, and I
> think it would be appropriate to also define a reg-offset property to
> handle the +3 offset and then let the xilinx 16550 nodes use those.
Since the ePAPR ns16550 defines reg-shift, I don't think it makes sense
to have a separate binding for sparse16550, or for the xilinx16550
types. Personally, I like having reg-offset better than
adding three to the reg space, but I leave that up to someone who is
much more involved in ePAPR.
> > +static struct of_serial_info __devinitdata ns8250_info =3D { .type =
=3D
PORT_8250 };
> > +static struct of_serial_info __devinitdata ns16450_info =3D { =
.type
=3D PORT_16450 };
> > +static struct of_serial_info __devinitdata ns16550_info =3D { =
.type
=3D PORT_16550 };
> > +static struct of_serial_info __devinitdata ns16750_info =3D { =
.type
=3D PORT_16750 };
> > +static struct of_serial_info __devinitdata xilinx_16550_info =3D {
> > + .type =3D PORT_16550,
> > + .regshift =3D 2,
> > + .regoffset =3D 3,
> > +};
> > +static struct of_serial_info __devinitdata unknown_info =3D { =
.type
=3D PORT_UNKNOWN };
>=20
> In support of my argument; the fact that you need a table of data says
> to me that this data should really be encoded in the device tree. :-)
The table of data was always there to bind the UART types, I just added
some more info to it.
Steve
^ permalink raw reply
* Re: muram in device tree for mpc8250 in arch/powerpc
From: Scott Wood @ 2008-03-21 16:18 UTC (permalink / raw)
To: James Black; +Cc: linuxppc-embedded
In-Reply-To: <b77025b40803201602q68fce3e1j703502a270351179@mail.gmail.com>
On Thu, Mar 20, 2008 at 05:02:35PM -0600, James Black wrote:
> On Thu, Mar 20, 2008 at 12:20 PM, Scott Wood <scottwood@freescale.com> wrote:
> > James Black wrote:
> > > Zone PFN ranges:
> > > DMA 0 -> 16384
> > > Normal 16384 -> 16384
> > > Movable zone start PFN for each node
> > > early_node_map[1] active PFN ranges
> > > 0: 0 -> 16384
> > > Unable to handle kernel paging request for data at address 0xe001a000
> > > Faulting instruction address: 0xc00e1a6c
> >
> > What function is 0xc00e1a6c in?
> Drilled down into the source and find that mm/bootmem.c:line 232
>
> : find_next_zero_bit()
> may be the culprit. When I put printks in, the line moves around a bit.
Is it node_bootmem_map, eidx, or i that is bad? What is end_pfn? What is
calling alloc_bootmem?
> > You can't use all of 0x8000-0x8fff; there is device parameter RAM in
> > there. If you can figure out the portions that aren't in use, you can
> > use those, but I wouldn't bother unless you really need the extra muram.
>
> So, are you are saying that the muram node is giving the ram to the
> kernel and is no longer available for devices to use as buffer
> descriptors?
No.
> I imagined the opposite. I thought I was telling the
> kernel where the parameter ram and buffer descriptors live.
It's telling the kernel where available muram is, to be allocated by CPM
drivers for buffer descriptors.
> I am going to turn on the MCC2 device eventually and will need all of the
> dpram for buffer descriptors.
>
> The memory map in my mind right now is
> 0x0000 - 0x3FFF --> buffer descriptors for SCCx and MCC2.
> 0x8000 - 0x8FFF --> paramter ram for SCCx and MCC2, et al. (reserved)
> 0xB000 - 0xB8FF --> FCCx specific (reserved)
>
> In my case, I shouldn't define any of the dual port ram for the kernel?
You shouldn't define any of the reserved areas as general purpose muram.
-Scott
^ 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