LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* 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 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 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 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 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 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: 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: 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: 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 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: [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] 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: 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] [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: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 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: 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: 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: 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: [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

* [PATCH] [POWERPC] Provide access to arch/powerpc include path on ppc64
From: Kumar Gala @ 2008-03-21  9:32 UTC (permalink / raw)
  To: linuxppc-dev

There does not appear to be any reason that we shouldn't just have
-Iarch/$(ARCH) on both ppc32 and ppc64 builds.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/Makefile |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index ab5cfe8..43ee815 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -71,13 +71,11 @@ endif

 LDFLAGS_vmlinux	:= -Bstatic

-CPPFLAGS-$(CONFIG_PPC32) := -Iarch/$(ARCH)
-AFLAGS-$(CONFIG_PPC32)	:= -Iarch/$(ARCH)
 CFLAGS-$(CONFIG_PPC64)	:= -mminimal-toc -mtraceback=none  -mcall-aixdesc
-CFLAGS-$(CONFIG_PPC32)	:= -Iarch/$(ARCH) -ffixed-r2 -mmultiple
-KBUILD_CPPFLAGS	+= $(CPPFLAGS-y)
-KBUILD_AFLAGS	+= $(AFLAGS-y)
-KBUILD_CFLAGS	+= -msoft-float -pipe $(CFLAGS-y)
+CFLAGS-$(CONFIG_PPC32)	:= -ffixed-r2 -mmultiple
+KBUILD_CPPFLAGS	+= -Iarch/$(ARCH)
+KBUILD_AFLAGS	+= -Iarch/$(ARCH)
+KBUILD_CFLAGS	+= -msoft-float -pipe -Iarch/$(ARCH) $(CFLAGS-y)
 CPP		= $(CC) -E $(KBUILD_CFLAGS)

 CHECKFLAGS	+= -m$(CONFIG_WORD_SIZE) -D__powerpc__ -D__powerpc$(CONFIG_WORD_SIZE)__
-- 
1.5.4.1

^ permalink raw reply related

* RFC - use of _stext vs PAGE_OFFSET?
From: Kumar Gala @ 2008-03-21  9:27 UTC (permalink / raw)
  To: linuxppc-dev

Is there any reason not use use _stext?

- k

diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index cd870a8..b0989ca 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -277,7 +277,7 @@ void __init setup_arch(char **cmdline_p)
 	if (ppc_md.panic)
 		setup_panic();

-	init_mm.start_code = PAGE_OFFSET;
+	init_mm.start_code = (unsigned long)_stext;
 	init_mm.end_code = (unsigned long) _etext;
 	init_mm.end_data = (unsigned long) _edata;
 	init_mm.brk = klimit;

^ permalink raw reply related

* Re: crash in init_ipic_sysfs on efika
From: Kumar Gala @ 2008-03-21  9:18 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: linuxppc-dev, Olaf Hering
In-Reply-To: <18403.31214.826941.838442@cargo.ozlabs.ibm.com>

On Fri, 21 Mar 2008, 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?

since you are going to send something to linus let me update the fsl
defconfigs and send you a pull request.

- k

^ permalink raw reply

* Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart 16550.
From: Paul Mackerras @ 2008-03-21  9:21 UTC (permalink / raw)
  To: Grant Likely; +Cc: linuxppc-dev, John Linn
In-Reply-To: <fa686aa40803201719i7bf02ba2qd259f1a36a24d943@mail.gmail.com>

Grant Likely writes:

> 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?

Paul.

^ permalink raw reply

* Re: [BUG]2.6.25-rc6:Unable to handle kernel paging request
From: Paul Mackerras @ 2008-03-21  9:07 UTC (permalink / raw)
  To: skumar; +Cc: Poornima Nayak, linuxppc-dev, akpm, linux-kernel
In-Reply-To: <20080320184216.GB32259@in.ibm.com>

Sudhir Kumar writes:

> > I suggest you turn off CONFIG_SENSORS_F71805F.
> It was a new feature so I turned it on while compiling.
> I have tried by turning off CONFIG_SENSORS_F71805F but the bug
> is still present.

Do you mean that you still saw a crash in f71805f_find?  If so, then
you have some problem with the way you rebuilt the kernel, or you
didn't boot the kernel you thought you did, or possibly that
CONFIG_SENSORS_F71805F is getting turned back on by a select statement
somewhere in some Kconfig file.

If you really turned CONFIG_SENSORS_F71805F off then there would be no
f71805f_find function in the kernel.

Paul.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox