* Re: time accounting problem (powerpc only?)
From: Johannes Berg @ 2007-11-27 13:42 UTC (permalink / raw)
To: Tony Breeds; +Cc: linuxppc-dev list, Linux Kernel list
In-Reply-To: <20071127050031.GC24243@bakeyournoodle.com>
[-- Attachment #1: Type: text/plain, Size: 547 bytes --]
On Tue, 2007-11-27 at 16:00 +1100, Tony Breeds wrote:
> On Mon, Nov 26, 2007 at 05:23:13PM +0100, Johannes Berg wrote:
> > Contrary to what I claimed later in the thread, my 64-bit powerpc box
> > (quad-core G5) doesn't suffer from this problem.
> >
> > Does anybody have any idea? I don't even know how to debug it further.
>
> I'll see if I can grab an appropriate machine tomorrow and have a look at
> it. I think it's just an accounting bug, which is probably my fault :)
Thanks. Let me know if you need any info.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: 85xx software reset problems from paulus.git
From: Dale Farnsworth @ 2007-11-27 13:50 UTC (permalink / raw)
To: linuxppc-embedded
In-Reply-To: <2F57AC6E-EE1B-4B8D-8AB5-AAF5B2DC4C06@kernel.crashing.org>
Kumar Gala wrote:
> >> Yes. The symptoms in 2.6.24RC2 are that during a kernel panic or when
> >> calling 'reboot' in the shell, it just hangs. Using the same dts and
> >> resets in 2.6.23.1 reboots fine. I don't have a cds reference, but
> >> someone who does should be able to confirm whether the issue exists
> >> or
> >> not by just attempting to reboot via bash.
>
> Can someone do a git-bisect and find the patch that breaks things?
I'll see if I can reproduce it on my 8548cds. If so, I'll then git-bisect.
-Dale
^ permalink raw reply
* Re: [PATCH 2/3] [POWERPC] fsl_soc: add support for gianfar for fixed-link property
From: Anton Vorontsov @ 2007-11-27 13:59 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: netdev, linux-kernel, Jeff Garzik, linuxppc-dev
In-Reply-To: <1196169432.9816.16.camel@gentoo-jocke.transmode.se>
On Tue, Nov 27, 2007 at 02:17:11PM +0100, Joakim Tjernlund wrote:
>
> On Tue, 2007-11-27 at 14:39 +0300, Anton Vorontsov wrote:
> > On Mon, Nov 26, 2007 at 04:04:08PM +0100, Joakim Tjernlund wrote:
> > > On Mon, 2007-11-26 at 17:29 +0300, Vitaly Bordug wrote:
> > > > fixed-link says: register new "Fixed/emulated PHY", i.e. PHY that
> > > > not connected to the real MDIO bus.
> > > >
> > > > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> > > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > > >
> > > > ---
> > > >
> > > > Documentation/powerpc/booting-without-of.txt | 3 +
> > > > arch/powerpc/sysdev/fsl_soc.c | 56 ++++++++++++++++++--------
> > > > 2 files changed, 42 insertions(+), 17 deletions(-)
> > > >
> > > >
> > > > diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> > > > index e9a3cb1..cf25070 100644
> > > > --- a/Documentation/powerpc/booting-without-of.txt
> > > > +++ b/Documentation/powerpc/booting-without-of.txt
> > > > @@ -1254,6 +1254,9 @@ platforms are moved over to use the flattened-device-tree model.
> > > > services interrupts for this device.
> > > > - phy-handle : The phandle for the PHY connected to this ethernet
> > > > controller.
> > > > + - fixed-link : <a b c> where a is emulated phy id - choose any,
> > > > + but unique to the all specified fixed-links, b is duplex - 0 half,
> > > > + 1 full, c is link speed - d#10/d#100/d#1000.
> > >
> > > Good work!
> > > May I suggest adding a "d" to <a b c> where d is flow control - 0 no, 1 yes
> >
> > Well, I see no reference of the "flow" neither in the include/linux/mii.h
> > nor in the drivers/net/phy/*. :-/ Thus today there is no such register
> > bit we can emulate?..
>
> Well, as good as I can recall, flow control(pause) is something that the
> PHY negotiates, just like FDX/HDX and should be dealt with in the
> adjust_link callback but not many do currently.
>
> If you seach for pause in phy_device.c you will find it.
Ah, pause. Sure, we can emulate that.
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH 2/3] [POWERPC] fsl_soc: add support for gianfar for fixed-link property
From: Joakim Tjernlund @ 2007-11-27 14:01 UTC (permalink / raw)
To: avorontsov; +Cc: netdev, linux-kernel, Jeff Garzik, linuxppc-dev
In-Reply-To: <20071127135952.GA1483@localhost.localdomain>
On Tue, 2007-11-27 at 16:59 +0300, Anton Vorontsov wrote:
> On Tue, Nov 27, 2007 at 02:17:11PM +0100, Joakim Tjernlund wrote:
> >
> > On Tue, 2007-11-27 at 14:39 +0300, Anton Vorontsov wrote:
> > > On Mon, Nov 26, 2007 at 04:04:08PM +0100, Joakim Tjernlund wrote:
> > > > On Mon, 2007-11-26 at 17:29 +0300, Vitaly Bordug wrote:
> > > > > fixed-link says: register new "Fixed/emulated PHY", i.e. PHY that
> > > > > not connected to the real MDIO bus.
> > > > >
> > > > > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
> > > > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > > > >
> > > > > ---
> > > > >
> > > > > Documentation/powerpc/booting-without-of.txt | 3 +
> > > > > arch/powerpc/sysdev/fsl_soc.c | 56 ++++++++++++++++++--------
> > > > > 2 files changed, 42 insertions(+), 17 deletions(-)
> > > > >
> > > > >
> > > > > diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> > > > > index e9a3cb1..cf25070 100644
> > > > > --- a/Documentation/powerpc/booting-without-of.txt
> > > > > +++ b/Documentation/powerpc/booting-without-of.txt
> > > > > @@ -1254,6 +1254,9 @@ platforms are moved over to use the flattened-device-tree model.
> > > > > services interrupts for this device.
> > > > > - phy-handle : The phandle for the PHY connected to this ethernet
> > > > > controller.
> > > > > + - fixed-link : <a b c> where a is emulated phy id - choose any,
> > > > > + but unique to the all specified fixed-links, b is duplex - 0 half,
> > > > > + 1 full, c is link speed - d#10/d#100/d#1000.
> > > >
> > > > Good work!
> > > > May I suggest adding a "d" to <a b c> where d is flow control - 0 no, 1 yes
> > >
> > > Well, I see no reference of the "flow" neither in the include/linux/mii.h
> > > nor in the drivers/net/phy/*. :-/ Thus today there is no such register
> > > bit we can emulate?..
> >
> > Well, as good as I can recall, flow control(pause) is something that the
> > PHY negotiates, just like FDX/HDX and should be dealt with in the
> > adjust_link callback but not many do currently.
> >
> > If you seach for pause in phy_device.c you will find it.
>
> Ah, pause. Sure, we can emulate that.
Good, looking into phy_device there seems to be 2 pauses:
ADVERTISED_Pause and ADVERTISE_PAUSE_ASYM which
maps to phydev->pause and phydev->asym_pause
so "d" should reflect that or a "e" should be added
for the asym pause.
Jocke
Jocke
^ permalink raw reply
* RE: [PATCH v2] [POWERPC] Optimize counting distinct entries in the relocation sections
From: Medve Emilian @ 2007-11-27 14:54 UTC (permalink / raw)
To: paulus, rusty, ntl, sfr, behlendorf1, galak, linuxppc-dev,
linuxppc-embedded
In-Reply-To: <1194971044-28840-1-git-send-email-Emilian.Medve@Freescale.com>
Hi Paul,
I'm just wondering what are your latest thoughts about this patch
(http://patchwork.ozlabs.org/linuxppc/patch?id=3D14707).
Cheers,
Emil.
> -----Original Message-----
> From: Medve Emilian=20
> Sent: Tuesday, November 13, 2007 10:24 AM
> To: paulus@samba.org; rusty@rustcorp.com.au; ntl@pobox.com;=20
> sfr@canb.auug.org.au; behlendorf1@llnl.gov;=20
> galak@kernel.crashing.org; linuxppc-dev@ozlabs.org;=20
> linuxppc-embedded@ozlabs.org
> Cc: Medve Emilian
> Subject: [PATCH v2] [POWERPC] Optimize counting distinct=20
> entries in the relocation sections
>=20
> When a module has relocation sections with tens of thousands=20
> worth of entries,
> counting the distinct/unique entries only (i.e. no=20
> duplicates) at load time can
> take tens of seconds and up to minutes. The sore point is the=20
> count_relocs()
> function which is called as part of the architecture specific=20
> module loading
> processing path:
>=20
> -> load_module() generic
> -> module_frob_arch_sections() arch specific
> -> get_plt_size() 32-bit
> -> get_stubs_size() 64-bit
> -> count_relocs()
>=20
> (Not sure why the relocation tables could contain lots of=20
> duplicates and why
> they are not trimmed at compile time by the linker. In some=20
> test cases, out of
> 35K relocation entries only 1.5K were distinct/unique)
>=20
> The previous counting algorithm was having O(n^2) complexity.=20
> Basically two
> solutions were proposed on the e-mail list: a hash based=20
> approach and a sort
> based approach
>=20
> The hash based approach is the fastest (O(n)) but the has it=20
> needs additional
> memory and for certain corner cases it could take lots of=20
> memory due to the
> degeneration of the hash. One such proposal was submitted here:
>=20
> http://ozlabs.org/pipermail/linuxppc-dev/2007-June/037641.html
>=20
> In this proposal, the symbol + addendum are hashed to=20
> generate a key and a
> pointer to the relocation entry will be stored in it. The=20
> hash is implemented as
> a linked list of memory pages with PAGE_SIZE /=20
> sizeof(Elfxx_Rela *) entries. In
> case of collisions in all the existing pages, a new page is=20
> added to the list to
> accommodate the new distinct relocation entry
>=20
> For 32-bit PowerPCs with 4K pages, a page can accommodate 1K=20
> worth of pointers
> to relocation entries. In the 35K entries scenario, as=20
> much/little of six (6)
> pages could be allocated using 24K of extra memory during the=20
> module load
>=20
> The sort based approach is slower (O(n * log n + n)) but if=20
> the sorting is done
> "in place" it doesn't need additional memory. A proposal was=20
> submitted here:
>=20
> http://ozlabs.org/pipermail/linuxppc-dev/2007-November/045854.html
> =
(http://patchwork.ozlabs.org/linuxppc/patch?filter=3Ddefault&id=3D14573)
>=20
> In this proposal an array of pointers to the relocation=20
> entries is built and
> then is sorted using the kernel sort() utility function. This=20
> is basically a heap
> sort algorithm with a stable O(n * log n) complexity. With=20
> this counting the
> distinct/unique entries is just linear (O(n)) complexity. The=20
> problem is the
> extra memory needed in this proposal, which in the 35K=20
> relocation entries test
> case it can be as much as 140K (for 32-bit PowerPCs; double=20
> for 64-bit). This is
> much more then the memory needed by the hash based approach described
> above/earlier but it doesn't hide potential degenerative corner cases
>=20
> The current patch is a happy compromise between the two=20
> proposals above:
> O(n + n * log n) performance with no additional memory=20
> requirements due to
> sorting in place the relocation table itself
>=20
> Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
> ---
>=20
> This patch only tries to address counting the distinct=20
> R_PPC_REL24 entries for
> the purpose of sizing the PLT. This operation was/can be=20
> detected by the proper
> kernel logic as a soft lockup for large relocation tables
>=20
> A related optimization (that could cause rewriting the this=20
> patch) is to
> optimize the PLT search from do_plt_call() but that doesn't=20
> seem to be a
> problem right now
>=20
> The errors below are false positives due to the fact that=20
> Elfxx_Rela are
> falsely assumed to be variables/operands instead of types:
>=20
> linux-2.6> scripts/checkpatch.pl=20
> 0001-POWERPC-Optimize-counting-distinct-entries-in-the.patch=20
> ERROR: need consistent spacing around '*' (ctx:WxV)
> #116: FILE: arch/powerpc/kernel/module_32.c:78:
> + const Elf32_Rela *x, *y;
> ^
>=20
> ERROR: need consistent spacing around '*' (ctx:WxV)
> #228: FILE: arch/powerpc/kernel/module_64.c:122:
> + const Elf64_Rela *x, *y;
> ^
>=20
> total: 2 errors, 0 warnings, 218 lines checked
> Your patch has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>=20
> arch/powerpc/kernel/module_32.c | 77=20
> ++++++++++++++++++++++++++++++-------
> arch/powerpc/kernel/module_64.c | 81=20
> ++++++++++++++++++++++++++++++--------
> 2 files changed, 127 insertions(+), 31 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/module_32.c=20
> b/arch/powerpc/kernel/module_32.c
> index 07a89a3..eab3138 100644
> --- a/arch/powerpc/kernel/module_32.c
> +++ b/arch/powerpc/kernel/module_32.c
> @@ -24,6 +24,7 @@
> #include <linux/kernel.h>
> #include <linux/cache.h>
> #include <linux/bug.h>
> +#include <linux/sort.h>
> =20
> #include "setup.h"
> =20
> @@ -54,22 +55,60 @@ void module_free(struct module *mod, void=20
> *module_region)
> addend) */
> static unsigned int count_relocs(const Elf32_Rela *rela,=20
> unsigned int num)
> {
> - unsigned int i, j, ret =3D 0;
> -
> - /* Sure, this is order(n^2), but it's usually short, and not
> - time critical */
> - for (i =3D 0; i < num; i++) {
> - for (j =3D 0; j < i; j++) {
> - /* If this addend appeared before, it's
> - already been counted */
> - if (ELF32_R_SYM(rela[i].r_info)
> - =3D=3D ELF32_R_SYM(rela[j].r_info)
> - && rela[i].r_addend =3D=3D rela[j].r_addend)
> - break;
> + unsigned int i, r_info, r_addend, _count_relocs;
> +
> + _count_relocs =3D 0;
> + r_info =3D 0;
> + r_addend =3D 0;
> + for (i =3D 0; i < num; i++)
> + /* Only count 24-bit relocs, others don't need stubs */
> + if (ELF32_R_TYPE(rela[i].r_info) =3D=3D R_PPC_REL24 &&
> + (r_info !=3D ELF32_R_SYM(rela[i].r_info) ||
> + r_addend !=3D rela[i].r_addend)) {
> + _count_relocs++;
> + r_info =3D ELF32_R_SYM(rela[i].r_info);
> + r_addend =3D rela[i].r_addend;
> }
> - if (j =3D=3D i) ret++;
> +
> + return _count_relocs;
> +}
> +
> +static int relacmp(const void *_x, const void *_y)
> +{
> + const Elf32_Rela *x, *y;
> +
> + y =3D (Elf32_Rela *)_x;
> + x =3D (Elf32_Rela *)_y;
> +
> + /* Compare the entire r_info (as opposed to=20
> ELF32_R_SYM(r_info) only) to
> + * make the comparison cheaper/faster. It won't affect=20
> the sorting or
> + * the counting algorithms' performance
> + */
> + if (x->r_info < y->r_info)
> + return -1;
> + else if (x->r_info > y->r_info)
> + return 1;
> + else if (x->r_addend < y->r_addend)
> + return -1;
> + else if (x->r_addend > y->r_addend)
> + return 1;
> + else
> + return 0;
> +}
> +
> +static void relaswap(void *_x, void *_y, int size)
> +{
> + uint32_t *x, *y, tmp;
> + int i;
> +
> + y =3D (uint32_t *)_x;
> + x =3D (uint32_t *)_y;
> +
> + for (i =3D 0; i < sizeof(Elf32_Rela) / sizeof(uint32_t); i++) {
> + tmp =3D x[i];
> + x[i] =3D y[i];
> + y[i] =3D tmp;
> }
> - return ret;
> }
> =20
> /* Get the potential trampolines size required of the init and
> @@ -100,6 +139,16 @@ static unsigned long get_plt_size(const=20
> Elf32_Ehdr *hdr,
> DEBUGP("Ptr: %p. Number: %u\n",
> (void *)hdr + sechdrs[i].sh_offset,
> sechdrs[i].sh_size / sizeof(Elf32_Rela));
> +
> + /* Sort the relocation information=20
> based on a symbol and
> + * addend key. This is a stable O(n*log=20
> n) complexity
> + * alogrithm but it will reduce the=20
> complexity of
> + * count_relocs() to linear complexity O(n)
> + */
> + sort((void *)hdr + sechdrs[i].sh_offset,
> + sechdrs[i].sh_size / sizeof(Elf32_Rela),
> + sizeof(Elf32_Rela), relacmp, relaswap);
> +
> ret +=3D count_relocs((void *)hdr
> + sechdrs[i].sh_offset,
> sechdrs[i].sh_size
> diff --git a/arch/powerpc/kernel/module_64.c=20
> b/arch/powerpc/kernel/module_64.c
> index 75c7c4f..3a82b02 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -24,6 +24,7 @@
> #include <asm/module.h>
> #include <asm/uaccess.h>
> #include <asm/firmware.h>
> +#include <linux/sort.h>
> =20
> #include "setup.h"
> =20
> @@ -81,25 +82,23 @@ static struct ppc64_stub_entry ppc64_stub =3D
> different addend) */
> static unsigned int count_relocs(const Elf64_Rela *rela,=20
> unsigned int num)
> {
> - unsigned int i, j, ret =3D 0;
> + unsigned int i, r_info, r_addend, _count_relocs;
> =20
> /* FIXME: Only count external ones --RR */
> - /* Sure, this is order(n^2), but it's usually short, and not
> - time critical */
> - for (i =3D 0; i < num; i++) {
> + _count_relocs =3D 0;
> + r_info =3D 0;
> + r_addend =3D 0;
> + for (i =3D 0; i < num; i++)
> /* Only count 24-bit relocs, others don't need stubs */
> - if (ELF64_R_TYPE(rela[i].r_info) !=3D R_PPC_REL24)
> - continue;
> - for (j =3D 0; j < i; j++) {
> - /* If this addend appeared before, it's
> - already been counted */
> - if (rela[i].r_info =3D=3D rela[j].r_info
> - && rela[i].r_addend =3D=3D rela[j].r_addend)
> - break;
> + if (ELF64_R_TYPE(rela[i].r_info) =3D=3D R_PPC_REL24 &&
> + (r_info !=3D ELF64_R_SYM(rela[i].r_info) ||
> + r_addend !=3D rela[i].r_addend)) {
> + _count_relocs++;
> + r_info =3D ELF64_R_SYM(rela[i].r_info);
> + r_addend =3D rela[i].r_addend;
> }
> - if (j =3D=3D i) ret++;
> - }
> - return ret;
> +
> + return _count_relocs;
> }
> =20
> void *module_alloc(unsigned long size)
> @@ -118,6 +117,44 @@ void module_free(struct module *mod,=20
> void *module_region)
> table entries. */
> }
> =20
> +static int relacmp(const void *_x, const void *_y)
> +{
> + const Elf64_Rela *x, *y;
> +
> + y =3D (Elf64_Rela *)_x;
> + x =3D (Elf64_Rela *)_y;
> +
> + /* Compare the entire r_info (as opposed to=20
> ELF64_R_SYM(r_info) only) to
> + * make the comparison cheaper/faster. It won't affect=20
> the sorting or
> + * the counting algorithms' performance
> + */
> + if (x->r_info < y->r_info)
> + return -1;
> + else if (x->r_info > y->r_info)
> + return 1;
> + else if (x->r_addend < y->r_addend)
> + return -1;
> + else if (x->r_addend > y->r_addend)
> + return 1;
> + else
> + return 0;
> +}
> +
> +static void relaswap(void *_x, void *_y, int size)
> +{
> + uint64_t *x, *y, tmp;
> + int i;
> +
> + y =3D (uint64_t *)_x;
> + x =3D (uint64_t *)_y;
> +
> + for (i =3D 0; i < sizeof(Elf64_Rela) / sizeof(uint64_t); i++) {
> + tmp =3D x[i];
> + x[i] =3D y[i];
> + y[i] =3D tmp;
> + }
> +}
> +
> /* Get size of potential trampolines required. */
> static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
> const Elf64_Shdr *sechdrs)
> @@ -133,6 +170,16 @@ static unsigned long=20
> get_stubs_size(const Elf64_Ehdr *hdr,
> DEBUGP("Ptr: %p. Number: %lu\n",
> (void *)sechdrs[i].sh_addr,
> sechdrs[i].sh_size / sizeof(Elf64_Rela));
> +
> + /* Sort the relocation information=20
> based on a symbol and
> + * addend key. This is a stable O(n*log=20
> n) complexity
> + * alogrithm but it will reduce the=20
> complexity of
> + * count_relocs() to linear complexity O(n)
> + */
> + sort((void *)sechdrs[i].sh_addr,
> + sechdrs[i].sh_size / sizeof(Elf64_Rela),
> + sizeof(Elf64_Rela), relacmp, relaswap);
> +
> relocs +=3D count_relocs((void=20
> *)sechdrs[i].sh_addr,
> sechdrs[i].sh_size
> / sizeof(Elf64_Rela));
> @@ -343,7 +390,7 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
> /* Simply set it */
> *(u32 *)location =3D value;
> break;
> - =09
> +
> case R_PPC64_ADDR64:
> /* Simply set it */
> *(unsigned long *)location =3D value;
> @@ -399,7 +446,7 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
> }
> =20
> /* Only replace bits 2 through 26 */
> - *(uint32_t *)location=20
> + *(uint32_t *)location
> =3D (*(uint32_t *)location & ~0x03fffffc)
> | (value & 0x03fffffc);
> break;
> --=20
> 1.5.3.GIT
^ permalink raw reply
* Re: 85xx software reset problems from paulus.git
From: Dale Farnsworth @ 2007-11-27 14:54 UTC (permalink / raw)
To: linuxppc-embedded
In-Reply-To: <20071127135051.32227.qmail@farnsworth.org>
I wrote:
> Kumar Gala wrote:
> > >> Yes. The symptoms in 2.6.24RC2 are that during a kernel panic or when
> > >> calling 'reboot' in the shell, it just hangs. Using the same dts and
> > >> resets in 2.6.23.1 reboots fine. I don't have a cds reference, but
> > >> someone who does should be able to confirm whether the issue exists
> > >> or
> > >> not by just attempting to reboot via bash.
> >
> > Can someone do a git-bisect and find the patch that breaks things?
>
> I'll see if I can reproduce it on my 8548cds. If so, I'll then git-bisect.
I tried this with the current powerpc.git tree on my mpc8548cds, and
the reboot command works for me. The system rebooted just fine.
-Dale
^ permalink raw reply
* Re: [PATCH] [POWERPC] Emulate isel (Integer Select) instruction
From: Geert Uytterhoeven @ 2007-11-27 14:56 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <Pine.LNX.4.62.0711211407260.12720@pademelon.sonytel.be>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 10848 bytes --]
On Wed, 21 Nov 2007, Geert Uytterhoeven wrote:
> On Tue, 20 Nov 2007, Kumar Gala wrote:
> > On Nov 20, 2007, at 11:54 AM, Scott Wood wrote:
> > > On Mon, Nov 19, 2007 at 09:36:57PM -0600, Kumar Gala wrote:
> > >> isel (Integer Select) is a new user space instruction in the
> > >> PowerISA 2.04 spec. Not all processors implement it so lets emulate
> > >> to ensure code built with isel will run everywhere.
> > >
> > > Given that the instruction is meant to be a performance enhancement,
> > > we should probably warn the first few times it's emulated, so the user
> > > knows they should change their toolchain setup if possible.
> >
> > The same is true of mcrxr, popcntb, and possibly string ld/st.
> >
> > Feel free to submit a patch that warns about their usage.
>
> Something like this?
New version below.
Do we want it in sysfs? Or should we use debugfs instead?
Subject: powerpc: Keep track of emulated instructions
From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
powerpc: Keep track of emulated instructions
Counters for the various classes of emulated instructions are available under
/sys/devices/system/cpu/cpu*/emulated/.
Optionally, rate-limited warnings can be printed to the console when
instructions are emulated.
Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
arch/powerpc/Kconfig.debug | 10 ++++++
arch/powerpc/kernel/align.c | 17 ++++++++--
arch/powerpc/kernel/sysfs.c | 64 ++++++++++++++++++++++++++++++++++++++++-
arch/powerpc/kernel/traps.c | 17 +++++++++-
include/asm-powerpc/emulator.h | 60 ++++++++++++++++++++++++++++++++++++++
5 files changed, 161 insertions(+), 7 deletions(-)
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -284,4 +284,14 @@ config PPC_EARLY_DEBUG_CPM_ADDR
platform probing is done, all platforms selected must
share the same address.
+config DEBUG_WARN_EMULATED
+ bool "Warn if emulated instructions are used"
+ depends on DEBUG_KERNEL && SYSFS
+ help
+ This option will cause messages to be printed if an instruction is
+ emulated.
+ Counters for emulated instruction usages are always available under
+ /sys/devices/system/cpu/cpu*/emulated/, irrespective of the state
+ of this option.
+
endmenu
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -24,6 +24,7 @@
#include <asm/system.h>
#include <asm/cache.h>
#include <asm/cputable.h>
+#include <asm/emulator.h>
struct aligninfo {
unsigned char len;
@@ -696,8 +697,10 @@ int fix_alignment(struct pt_regs *regs)
areg = dsisr & 0x1f; /* register to update */
#ifdef CONFIG_SPE
- if ((instr >> 26) == 0x4)
+ if ((instr >> 26) == 0x4) {
+ WARN_EMULATE(spe);
return emulate_spe(regs, reg, instr);
+ }
#endif
instr = (dsisr >> 10) & 0x7f;
@@ -731,17 +734,21 @@ int fix_alignment(struct pt_regs *regs)
/* A size of 0 indicates an instruction we don't support, with
* the exception of DCBZ which is handled as a special case here
*/
- if (instr == DCBZ)
+ if (instr == DCBZ) {
+ WARN_EMULATE(dcbz);
return emulate_dcbz(regs, addr);
+ }
if (unlikely(nb == 0))
return 0;
/* Load/Store Multiple instructions are handled in their own
* function
*/
- if (flags & M)
+ if (flags & M) {
+ WARN_EMULATE(multiple);
return emulate_multiple(regs, addr, reg, nb,
flags, instr, swiz);
+ }
/* Verify the address of the operand */
if (unlikely(user_mode(regs) &&
@@ -758,8 +765,10 @@ int fix_alignment(struct pt_regs *regs)
}
/* Special case for 16-byte FP loads and stores */
- if (nb == 16)
+ if (nb == 16) {
+ WARN_EMULATE(fp_pair);
return emulate_fp_pair(regs, addr, reg, flags);
+ }
/* If we are loading, get the data from user space, else
* get it from register values
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -19,6 +19,7 @@
#include <asm/lppaca.h>
#include <asm/machdep.h>
#include <asm/smp.h>
+#include <asm/emulator.h>
static DEFINE_PER_CPU(struct cpu, cpu_devices);
@@ -291,12 +292,68 @@ static struct sysdev_attribute pa6t_attr
};
+#define SYSFS_EMULATED_SETUP(type) \
+DEFINE_PER_CPU(atomic_long_t, emulated_ ## type); \
+static ssize_t show_emulated_ ## type (struct sys_device *dev, \
+ char *buf) \
+{ \
+ struct cpu *cpu = container_of(dev, struct cpu, sysdev); \
+ \
+ return sprintf(buf, "%lu\n", \
+ atomic_long_read(&per_cpu(emulated_ ## type, \
+ cpu->sysdev.id))); \
+} \
+ \
+static struct sysdev_attribute emulated_ ## type ## _attr = { \
+ .attr = { .name = #type, .mode = 0400 }, \
+ .show = show_emulated_ ## type, \
+};
+
+SYSFS_EMULATED_SETUP(dcba);
+SYSFS_EMULATED_SETUP(dcbz);
+SYSFS_EMULATED_SETUP(fp_pair);
+SYSFS_EMULATED_SETUP(mcrxr);
+SYSFS_EMULATED_SETUP(mfpvr);
+SYSFS_EMULATED_SETUP(multiple);
+SYSFS_EMULATED_SETUP(popcntb);
+SYSFS_EMULATED_SETUP(spe);
+SYSFS_EMULATED_SETUP(string);
+#ifdef CONFIG_MATH_EMULATION
+SYSFS_EMULATED_SETUP(math);
+#elif defined(CONFIG_8XX_MINIMAL_FPEMU)
+SYSFS_EMULATED_SETUP(8xx);
+#endif
+
+static struct attribute *emulated_attrs[] = {
+ &emulated_dcba_attr.attr,
+ &emulated_dcbz_attr.attr,
+ &emulated_fp_pair_attr.attr,
+ &emulated_mcrxr_attr.attr,
+ &emulated_mfpvr_attr.attr,
+ &emulated_multiple_attr.attr,
+ &emulated_popcntb_attr.attr,
+ &emulated_spe_attr.attr,
+ &emulated_string_attr.attr,
+#ifdef CONFIG_MATH_EMULATION
+ &emulated_math_attr.attr,
+#elif defined(CONFIG_8XX_MINIMAL_FPEMU)
+ &emulated_8xx_attr.attr,
+#endif
+ NULL
+};
+
+static struct attribute_group emulated_attr_group = {
+ .attrs = emulated_attrs,
+ .name = "emulated"
+};
+
+
static void register_cpu_online(unsigned int cpu)
{
struct cpu *c = &per_cpu(cpu_devices, cpu);
struct sys_device *s = &c->sysdev;
struct sysdev_attribute *attrs, *pmc_attrs;
- int i, nattrs;
+ int i, nattrs, res;
if (!firmware_has_feature(FW_FEATURE_ISERIES) &&
cpu_has_feature(CPU_FTR_SMT))
@@ -339,6 +396,11 @@ static void register_cpu_online(unsigned
if (cpu_has_feature(CPU_FTR_DSCR))
sysdev_create_file(s, &attr_dscr);
+
+ res = sysfs_create_group(&s->kobj, &emulated_attr_group);
+ if (res)
+ pr_warning("Cannot create emulated sysfs group for cpu %u\n",
+ cpu);
}
#ifdef CONFIG_HOTPLUG_CPU
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -53,6 +53,7 @@
#include <asm/processor.h>
#endif
#include <asm/kexec.h>
+#include <asm/emulator.h>
#ifdef CONFIG_DEBUGGER
int (*__debugger)(struct pt_regs *regs);
@@ -721,31 +722,38 @@ static int emulate_instruction(struct pt
/* Emulate the mfspr rD, PVR. */
if ((instword & INST_MFSPR_PVR_MASK) == INST_MFSPR_PVR) {
+ WARN_EMULATE(mfpvr);
rd = (instword >> 21) & 0x1f;
regs->gpr[rd] = mfspr(SPRN_PVR);
return 0;
}
/* Emulating the dcba insn is just a no-op. */
- if ((instword & INST_DCBA_MASK) == INST_DCBA)
+ if ((instword & INST_DCBA_MASK) == INST_DCBA) {
+ WARN_EMULATE(dcba);
return 0;
+ }
/* Emulate the mcrxr insn. */
if ((instword & INST_MCRXR_MASK) == INST_MCRXR) {
int shift = (instword >> 21) & 0x1c;
unsigned long msk = 0xf0000000UL >> shift;
+ WARN_EMULATE(mcrxr);
regs->ccr = (regs->ccr & ~msk) | ((regs->xer >> shift) & msk);
regs->xer &= ~0xf0000000UL;
return 0;
}
/* Emulate load/store string insn. */
- if ((instword & INST_STRING_GEN_MASK) == INST_STRING)
+ if ((instword & INST_STRING_GEN_MASK) == INST_STRING) {
+ WARN_EMULATE(string);
return emulate_string_inst(regs, instword);
+ }
/* Emulate the popcntb (Population Count Bytes) instruction. */
if ((instword & INST_POPCNTB_MASK) == INST_POPCNTB) {
+ WARN_EMULATE(popcntb);
return emulate_popcntb_inst(regs, instword);
}
@@ -929,6 +937,8 @@ void SoftwareEmulation(struct pt_regs *r
#ifdef CONFIG_MATH_EMULATION
errcode = do_mathemu(regs);
+ if (errcode >= 0)
+ WARN_EMULATE(math);
switch (errcode) {
case 0:
@@ -950,6 +960,9 @@ void SoftwareEmulation(struct pt_regs *r
#elif defined(CONFIG_8XX_MINIMAL_FPEMU)
errcode = Soft_emulate_8xx(regs);
+ if (errcode >= 0)
+ WARN_EMULATE(8xx);
+
switch (errcode) {
case 0:
emulate_single_step(regs);
--- /dev/null
+++ b/include/asm-powerpc/emulator.h
@@ -0,0 +1,60 @@
+/*
+ * Copyright 2007 Sony Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef _ASM_POWERPC_EMULATOR_H
+#define _ASM_POWERPC_EMULATOR_H
+
+#include <linux/kernel.h>
+#include <linux/percpu.h>
+
+#include <asm/atomic.h>
+
+DECLARE_PER_CPU(atomic_long_t, emulated_dcba);
+DECLARE_PER_CPU(atomic_long_t, emulated_dcbz);
+DECLARE_PER_CPU(atomic_long_t, emulated_fp_pair);
+DECLARE_PER_CPU(atomic_long_t, emulated_mcrxr);
+DECLARE_PER_CPU(atomic_long_t, emulated_mfpvr);
+DECLARE_PER_CPU(atomic_long_t, emulated_multiple);
+DECLARE_PER_CPU(atomic_long_t, emulated_popcntb);
+DECLARE_PER_CPU(atomic_long_t, emulated_spe);
+DECLARE_PER_CPU(atomic_long_t, emulated_string);
+#ifdef CONFIG_MATH_EMULATION
+DECLARE_PER_CPU(atomic_long_t, emulated_math);
+#elif defined(CONFIG_8XX_MINIMAL_FPEMU)
+DECLARE_PER_CPU(atomic_long_t, emulated_8xx);
+#endif
+
+#ifdef CONFIG_DEBUG_WARN_EMULATED
+static inline void do_warn_emulate(const char *type)
+{
+ if (printk_ratelimit())
+ pr_warning("%s used emulated %s instruction\n", current->comm,
+ type);
+}
+#else
+static inline void do_warn_emulate(const char *type) {}
+#endif
+
+#define WARN_EMULATE(type) \
+ do { \
+ atomic_long_inc(&per_cpu(emulated_ ## type, \
+ raw_smp_processor_id())); \
+ do_warn_emulate(#type); \
+ } while (0)
+
+
+#endif /* _ASM_POWERPC_EMULATOR_H */
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply
* Re: [PATCH] [POWERPC] Emulate isel (Integer Select) instruction
From: Olof Johansson @ 2007-11-27 15:23 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: linuxppc-dev
In-Reply-To: <Pine.LNX.4.62.0711271555520.5847@pademelon.sonytel.be>
On Tue, Nov 27, 2007 at 03:56:53PM +0100, Geert Uytterhoeven wrote:
> On Wed, 21 Nov 2007, Geert Uytterhoeven wrote:
> > On Tue, 20 Nov 2007, Kumar Gala wrote:
> > > On Nov 20, 2007, at 11:54 AM, Scott Wood wrote:
> > > > On Mon, Nov 19, 2007 at 09:36:57PM -0600, Kumar Gala wrote:
> > > >> isel (Integer Select) is a new user space instruction in the
> > > >> PowerISA 2.04 spec. Not all processors implement it so lets emulate
> > > >> to ensure code built with isel will run everywhere.
> > > >
> > > > Given that the instruction is meant to be a performance enhancement,
> > > > we should probably warn the first few times it's emulated, so the user
> > > > knows they should change their toolchain setup if possible.
> > >
> > > The same is true of mcrxr, popcntb, and possibly string ld/st.
> > >
> > > Feel free to submit a patch that warns about their usage.
> >
> > Something like this?
>
> New version below.
>
> Do we want it in sysfs? Or should we use debugfs instead?
I like this, and I'd say it's useful to have in sysfs. Few production
systems enable debugfs, and this is something that could be useful to
have access to there.
> Subject: powerpc: Keep track of emulated instructions
>
> From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
>
> powerpc: Keep track of emulated instructions
>
> Counters for the various classes of emulated instructions are available under
> /sys/devices/system/cpu/cpu*/emulated/.
> Optionally, rate-limited warnings can be printed to the console when
> instructions are emulated.
>
> Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> ---
> arch/powerpc/Kconfig.debug | 10 ++++++
> arch/powerpc/kernel/align.c | 17 ++++++++--
> arch/powerpc/kernel/sysfs.c | 64 ++++++++++++++++++++++++++++++++++++++++-
> arch/powerpc/kernel/traps.c | 17 +++++++++-
> include/asm-powerpc/emulator.h | 60 ++++++++++++++++++++++++++++++++++++++
This name stood out as being a bit too generic, emulator could mean
system support for running under some sort of emulator as well. How
about emulated_ops.h?
> +config DEBUG_WARN_EMULATED
> + bool "Warn if emulated instructions are used"
> + depends on DEBUG_KERNEL && SYSFS
> + help
> + This option will cause messages to be printed if an instruction is
> + emulated.
> + Counters for emulated instruction usages are always available under
> + /sys/devices/system/cpu/cpu*/emulated/, irrespective of the state
> + of this option.
How about making it a sysctl instead, so it can be flipped at runtime
(but default off)?
-Olof
^ permalink raw reply
* [PATCH 0/3] OF-platform PATA driver
From: Anton Vorontsov @ 2007-11-27 15:37 UTC (permalink / raw)
To: linuxppc-dev, linux-ide; +Cc: Paul Mundt, Jeff Garzik, Arnd Bergmann
Hi all,
Here is the second spin of the OF-platform PATA driver and
related patches.
Changes since RFC:
- nuked drivers/ata/pata_platform.h;
- powerpc bits: proper localbus node added.
Thanks for the previous review! This time I'm collecting acks,
don't be shy to give 'em generously. ;-)
Good luck,
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
^ permalink raw reply
* [PATCH 1/3] [libata] pata_platform: make probe and remove functions device type neutral
From: Anton Vorontsov @ 2007-11-27 15:39 UTC (permalink / raw)
To: linuxppc-dev, linux-ide
In-Reply-To: <20071127153708.GA12490@localhost.localdomain>
Split pata_platform_{probe,remove} into two pieces:
1. pata_platform_{probe,remove} -- platform_device-dependant bits;
2. __ptata_platform_{probe,remove} -- device type neutral bits.
This is done to not duplicate code for the OF-platform driver.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/ata/pata_platform.c | 139 ++++++++++++++++++++++++-----------------
include/linux/pata_platform.h | 8 +++
2 files changed, 90 insertions(+), 57 deletions(-)
diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c
index ac03a90..6436c38 100644
--- a/drivers/ata/pata_platform.c
+++ b/drivers/ata/pata_platform.c
@@ -93,14 +93,9 @@ static struct ata_port_operations pata_platform_port_ops = {
};
static void pata_platform_setup_port(struct ata_ioports *ioaddr,
- struct pata_platform_info *info)
+ unsigned int shift)
{
- unsigned int shift = 0;
-
/* Fixup the port shift for platforms that need it */
- if (info && info->ioport_shift)
- shift = info->ioport_shift;
-
ioaddr->data_addr = ioaddr->cmd_addr + (ATA_REG_DATA << shift);
ioaddr->error_addr = ioaddr->cmd_addr + (ATA_REG_ERR << shift);
ioaddr->feature_addr = ioaddr->cmd_addr + (ATA_REG_FEATURE << shift);
@@ -114,8 +109,12 @@ static void pata_platform_setup_port(struct ata_ioports *ioaddr,
}
/**
- * pata_platform_probe - attach a platform interface
- * @pdev: platform device
+ * __pata_platform_probe - attach a platform interface
+ * @dev: device
+ * @io_res: Resource representing I/O base
+ * @ctl_res: Resource representing CTL base
+ * @irq_res: Resource representing IRQ and its flags
+ * @ioport_shift: I/O port shift
*
* Register a platform bus IDE interface. Such interfaces are PIO and we
* assume do not support IRQ sharing.
@@ -135,42 +134,17 @@ static void pata_platform_setup_port(struct ata_ioports *ioaddr,
*
* If no IRQ resource is present, PIO polling mode is used instead.
*/
-static int __devinit pata_platform_probe(struct platform_device *pdev)
+int __devinit __pata_platform_probe(struct device *dev,
+ struct resource *io_res,
+ struct resource *ctl_res,
+ struct resource *irq_res,
+ unsigned int ioport_shift)
{
- struct resource *io_res, *ctl_res;
struct ata_host *host;
struct ata_port *ap;
- struct pata_platform_info *pp_info;
unsigned int mmio;
- int irq;
-
- /*
- * Simple resource validation ..
- */
- if ((pdev->num_resources != 3) && (pdev->num_resources != 2)) {
- dev_err(&pdev->dev, "invalid number of resources\n");
- return -EINVAL;
- }
-
- /*
- * Get the I/O base first
- */
- io_res = platform_get_resource(pdev, IORESOURCE_IO, 0);
- if (io_res == NULL) {
- io_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (unlikely(io_res == NULL))
- return -EINVAL;
- }
-
- /*
- * Then the CTL base
- */
- ctl_res = platform_get_resource(pdev, IORESOURCE_IO, 1);
- if (ctl_res == NULL) {
- ctl_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- if (unlikely(ctl_res == NULL))
- return -EINVAL;
- }
+ int irq = 0;
+ int irq_flags = 0;
/*
* Check for MMIO
@@ -181,14 +155,15 @@ static int __devinit pata_platform_probe(struct platform_device *pdev)
/*
* And the IRQ
*/
- irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- irq = 0; /* no irq */
+ if (irq_res && irq_res->start > 0) {
+ irq = irq_res->start;
+ irq_flags = irq_res->flags;
+ }
/*
* Now that that's out of the way, wire up the port..
*/
- host = ata_host_alloc(&pdev->dev, 1);
+ host = ata_host_alloc(dev, 1);
if (!host)
return -ENOMEM;
ap = host->ports[0];
@@ -209,25 +184,24 @@ static int __devinit pata_platform_probe(struct platform_device *pdev)
* Handle the MMIO case
*/
if (mmio) {
- ap->ioaddr.cmd_addr = devm_ioremap(&pdev->dev, io_res->start,
+ ap->ioaddr.cmd_addr = devm_ioremap(dev, io_res->start,
io_res->end - io_res->start + 1);
- ap->ioaddr.ctl_addr = devm_ioremap(&pdev->dev, ctl_res->start,
+ ap->ioaddr.ctl_addr = devm_ioremap(dev, ctl_res->start,
ctl_res->end - ctl_res->start + 1);
} else {
- ap->ioaddr.cmd_addr = devm_ioport_map(&pdev->dev, io_res->start,
+ ap->ioaddr.cmd_addr = devm_ioport_map(dev, io_res->start,
io_res->end - io_res->start + 1);
- ap->ioaddr.ctl_addr = devm_ioport_map(&pdev->dev, ctl_res->start,
+ ap->ioaddr.ctl_addr = devm_ioport_map(dev, ctl_res->start,
ctl_res->end - ctl_res->start + 1);
}
if (!ap->ioaddr.cmd_addr || !ap->ioaddr.ctl_addr) {
- dev_err(&pdev->dev, "failed to map IO/CTL base\n");
+ dev_err(dev, "failed to map IO/CTL base\n");
return -ENOMEM;
}
ap->ioaddr.altstatus_addr = ap->ioaddr.ctl_addr;
- pp_info = pdev->dev.platform_data;
- pata_platform_setup_port(&ap->ioaddr, pp_info);
+ pata_platform_setup_port(&ap->ioaddr, ioport_shift);
ata_port_desc(ap, "%s cmd 0x%llx ctl 0x%llx", mmio ? "mmio" : "ioport",
(unsigned long long)io_res->start,
@@ -235,26 +209,77 @@ static int __devinit pata_platform_probe(struct platform_device *pdev)
/* activate */
return ata_host_activate(host, irq, irq ? ata_interrupt : NULL,
- pp_info ? pp_info->irq_flags : 0,
- &pata_platform_sht);
+ irq_flags, &pata_platform_sht);
}
+EXPORT_SYMBOL_GPL(__pata_platform_probe);
/**
- * pata_platform_remove - unplug a platform interface
- * @pdev: platform device
+ * __pata_platform_remove - unplug a platform interface
+ * @dev: device
*
* A platform bus ATA device has been unplugged. Perform the needed
* cleanup. Also called on module unload for any active devices.
*/
-static int __devexit pata_platform_remove(struct platform_device *pdev)
+int __devexit __pata_platform_remove(struct device *dev)
{
- struct device *dev = &pdev->dev;
struct ata_host *host = dev_get_drvdata(dev);
ata_host_detach(host);
return 0;
}
+EXPORT_SYMBOL_GPL(__pata_platform_remove);
+
+static int __devinit pata_platform_probe(struct platform_device *pdev)
+{
+ struct resource *io_res;
+ struct resource *ctl_res;
+ struct resource *irq_res;
+ struct pata_platform_info *pp_info = pdev->dev.platform_data;
+
+ /*
+ * Simple resource validation ..
+ */
+ if ((pdev->num_resources != 3) && (pdev->num_resources != 2)) {
+ dev_err(&pdev->dev, "invalid number of resources\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Get the I/O base first
+ */
+ io_res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ if (io_res == NULL) {
+ io_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (unlikely(io_res == NULL))
+ return -EINVAL;
+ }
+
+ /*
+ * Then the CTL base
+ */
+ ctl_res = platform_get_resource(pdev, IORESOURCE_IO, 1);
+ if (ctl_res == NULL) {
+ ctl_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ if (unlikely(ctl_res == NULL))
+ return -EINVAL;
+ }
+
+ /*
+ * And the IRQ
+ */
+ irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ if (irq_res)
+ irq_res->flags = pp_info ? pp_info->irq_flags : 0;
+
+ return __pata_platform_probe(&pdev->dev, io_res, ctl_res, irq_res,
+ pp_info ? pp_info->ioport_shift : 0);
+}
+
+static int __devexit pata_platform_remove(struct platform_device *pdev)
+{
+ return __pata_platform_remove(&pdev->dev);
+}
static struct platform_driver pata_platform_driver = {
.probe = pata_platform_probe,
diff --git a/include/linux/pata_platform.h b/include/linux/pata_platform.h
index 5799e8d..0b11f6b 100644
--- a/include/linux/pata_platform.h
+++ b/include/linux/pata_platform.h
@@ -15,4 +15,12 @@ struct pata_platform_info {
unsigned int irq_flags;
};
+extern int __devinit __pata_platform_probe(struct device *dev,
+ struct resource *io_res,
+ struct resource *ctl_res,
+ struct resource *irq_res,
+ unsigned int ioport_shift);
+
+extern int __devexit __pata_platform_remove(struct device *dev);
+
#endif /* __LINUX_PATA_PLATFORM_H */
--
1.5.2.2
^ permalink raw reply related
* [PATCH 2/3] [libata] pata_of_platform: OF-Platform PATA device driver
From: Anton Vorontsov @ 2007-11-27 15:39 UTC (permalink / raw)
To: linuxppc-dev, linux-ide
In-Reply-To: <20071127153708.GA12490@localhost.localdomain>
This driver nicely wraps around pata_platform library functions,
and provides OF platform bus bindings to the PATA devices.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
drivers/ata/Kconfig | 10 +++++
drivers/ata/Makefile | 1 +
drivers/ata/pata_of_platform.c | 86 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 97 insertions(+), 0 deletions(-)
create mode 100644 drivers/ata/pata_of_platform.c
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index ba63619..5a492fa 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -614,6 +614,16 @@ config PATA_PLATFORM
If unsure, say N.
+config PATA_OF_PLATFORM
+ tristate "OpenFirmware platform device PATA support"
+ depends on PATA_PLATFORM && PPC_OF
+ help
+ This option enables support for generic directly connected ATA
+ devices commonly found on embedded systems with OpenFirmware
+ bindings.
+
+ If unsure, say N.
+
config PATA_ICSIDE
tristate "Acorn ICS PATA support"
depends on ARM && ARCH_ACORN
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index b13feb2..ebcee64 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_PATA_IXP4XX_CF) += pata_ixp4xx_cf.o
obj-$(CONFIG_PATA_SCC) += pata_scc.o
obj-$(CONFIG_PATA_BF54X) += pata_bf54x.o
obj-$(CONFIG_PATA_PLATFORM) += pata_platform.o
+obj-$(CONFIG_PATA_OF_PLATFORM) += pata_of_platform.o
obj-$(CONFIG_PATA_ICSIDE) += pata_icside.o
# Should be last but two libata driver
obj-$(CONFIG_PATA_ACPI) += pata_acpi.o
diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c
new file mode 100644
index 0000000..e6c769c
--- /dev/null
+++ b/drivers/ata/pata_of_platform.c
@@ -0,0 +1,86 @@
+/*
+ * OF-platform PATA driver
+ *
+ * Copyright (c) 2007 MontaVista Software, Inc.
+ * Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License (Version 2) as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/pata_platform.h>
+
+static int __devinit pata_of_platform_probe(struct of_device *ofdev,
+ const struct of_device_id *match)
+{
+ int ret;
+ struct device_node *dn = ofdev->node;
+ struct resource io_res;
+ struct resource ctl_res;
+ struct resource irq_res;
+ unsigned int ioport_shift = 0;
+ uint32_t *prop;
+
+ ret = of_address_to_resource(dn, 0, &io_res);
+ if (ret) {
+ dev_err(&ofdev->dev, "can't get IO address from "
+ "device tree\n");
+ return -EINVAL;
+ }
+
+ ret = of_address_to_resource(dn, 1, &ctl_res);
+ if (ret) {
+ dev_err(&ofdev->dev, "can't get CTL address from "
+ "device tree\n");
+ return -EINVAL;
+ }
+
+ ret = of_irq_to_resource(dn, 0, &irq_res);
+ if (ret == NO_IRQ)
+ irq_res.start = irq_res.end = -1;
+ else
+ irq_res.flags = 0;
+
+ prop = (uint32_t *)of_get_property(dn, "ioport-shift", NULL);
+ if (prop)
+ ioport_shift = *prop;
+
+ return __pata_platform_probe(&ofdev->dev, &io_res, &ctl_res, &irq_res,
+ ioport_shift);
+}
+
+static int __devexit pata_of_platform_remove(struct of_device *ofdev)
+{
+ return __pata_platform_remove(&ofdev->dev);
+}
+
+static struct of_device_id pata_of_platform_match[] = {
+ { .compatible = "pata-platform", },
+};
+
+static struct of_platform_driver pata_of_platform_driver = {
+ .name = "pata_of_platform",
+ .match_table = pata_of_platform_match,
+ .probe = pata_of_platform_probe,
+ .remove = __devexit_p(pata_of_platform_remove),
+};
+
+static int __init pata_of_platform_init(void)
+{
+ return of_register_platform_driver(&pata_of_platform_driver);
+}
+module_init(pata_of_platform_init);
+
+static void __exit pata_of_platform_exit(void)
+{
+ of_unregister_platform_driver(&pata_of_platform_driver);
+}
+module_exit(pata_of_platform_exit);
+
+MODULE_DESCRIPTION("OF-platform PATA driver");
+MODULE_AUTHOR("Anton Vorontsov <avorontsov@ru.mvista.com>");
+MODULE_LICENSE("GPL");
--
1.5.2.2
^ permalink raw reply related
* [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes
From: Anton Vorontsov @ 2007-11-27 15:39 UTC (permalink / raw)
To: linuxppc-dev, linux-ide
In-Reply-To: <20071127153708.GA12490@localhost.localdomain>
This patch adds localbus and pata nodes to use CF IDE interface
on MPC8349E-mITX boards.
Patch also adds code to probe localbus.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/powerpc/boot/dts/mpc8349emitx.dts | 17 ++++++++++++++++-
arch/powerpc/platforms/83xx/mpc834x_itx.c | 17 +++++++++++++++++
2 files changed, 33 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts b/arch/powerpc/boot/dts/mpc8349emitx.dts
index 5072f6d..7a97068 100644
--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -249,6 +249,21 @@
device_type = "pci";
};
+ localbus@e0005000 {
+ #address-cells = <2>;
+ #size-cells = <1>;
+ compatible = "fsl,mpc8349emitx-localbus",
+ "fsl,mpc8349e-localbus",
+ "fsl,pq2pro-localbus";
+ reg = <e0005000 d8>;
+ ranges = <3 0 f0000000 210>;
-
+ pata@3,0 {
+ compatible = "fsl,mpc8349emitx-pata", "pata-platform";
+ reg = <3 0 10 3 20c 4>;
+ ioport-shift = <1>;
+ interrupts = <17 8>;
+ interrupt-parent = <&ipic>;
+ };
+ };
};
diff --git a/arch/powerpc/platforms/83xx/mpc834x_itx.c b/arch/powerpc/platforms/83xx/mpc834x_itx.c
index aa76819..ea5f176 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_itx.c
+++ b/arch/powerpc/platforms/83xx/mpc834x_itx.c
@@ -23,6 +23,7 @@
#include <linux/delay.h>
#include <linux/seq_file.h>
#include <linux/root_dev.h>
+#include <linux/of_platform.h>
#include <asm/system.h>
#include <asm/atomic.h>
@@ -37,6 +38,22 @@
#include "mpc83xx.h"
+static struct of_device_id mpc834x_itx_ids[] = {
+ { .name = "localbus", },
+ {},
+};
+
+static int __init mpc834x_itx_declare_of_platform_devices(void)
+{
+ if (!machine_is(mpc834x_itx))
+ return 0;
+
+ of_platform_bus_probe(NULL, mpc834x_itx_ids, NULL);
+
+ return 0;
+}
+device_initcall(mpc834x_itx_declare_of_platform_devices);
+
/* ************************************************************************
*
* Setup the architecture
--
1.5.2.2
^ permalink raw reply related
* Re: [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes
From: Sergei Shtylyov @ 2007-11-27 15:49 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linuxppc-dev, linux-ide
In-Reply-To: <20071127153940.GC14183@localhost.localdomain>
Hello.
Anton Vorontsov wrote:
> This patch adds localbus and pata nodes to use CF IDE interface
> on MPC8349E-mITX boards.
> Patch also adds code to probe localbus.
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> arch/powerpc/boot/dts/mpc8349emitx.dts | 17 ++++++++++++++++-
> arch/powerpc/platforms/83xx/mpc834x_itx.c | 17 +++++++++++++++++
> 2 files changed, 33 insertions(+), 1 deletions(-)
> diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts b/arch/powerpc/boot/dts/mpc8349emitx.dts
> index 5072f6d..7a97068 100644
> --- a/arch/powerpc/boot/dts/mpc8349emitx.dts
> +++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
> @@ -249,6 +249,21 @@
> device_type = "pci";
> };
>
> + localbus@e0005000 {
> + #address-cells = <2>;
> + #size-cells = <1>;
> + compatible = "fsl,mpc8349emitx-localbus",
Board compatible bus?
> + "fsl,mpc8349e-localbus",
> + "fsl,pq2pro-localbus";
> + reg = <e0005000 d8>;
> + ranges = <3 0 f0000000 210>;
>
> -
> + pata@3,0 {
> + compatible = "fsl,mpc8349emitx-pata", "pata-platform";
> + reg = <3 0 10 3 20c 4>;
> + ioport-shift = <1>;
Bleh... that shift again. And this is surely not a good name for a
property (where's I/O ports in your case?) -- why not call it "reg-shift"
(well, I'd call it "reg-size" or "reg-stride" myself :-)?
MBR, Sergei
^ permalink raw reply
* How to notify userspace of custom bus master/slave access mode changes ?
From: Laurent Pinchart @ 2007-11-27 15:51 UTC (permalink / raw)
To: linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]
Hi everybody,
I'm a bit unsure about how to notify userspace of changes to access mode to a
custom bus. This is more an architecture issue than an implementation issue.
My system has an ISA-like cold-pluggable extension bus. The bus controller
sits on the CPU board which is plugged along with slave devices into a
passive backplane.
To add CPU redundancy support to the system, I implemented master and slave
modes for the CPU board. The slave monitors the master and can disconnect it
from the bus when a failure is detected.
The bus drivers haven't been developed with hotplug in mind, but I eventually
managed to fix them. The kernel is now able to scan the bus when a CPU board
switches from slave to master mode, adding peripherals as they are detected,
and to remove all peripherals when the CPU board switches from master to
slave.
I now have to notify userspace applications of master <-> slave mode changes.
I thought about using kobject netlink notifications, but the kernel only
generates events for individual peripherals (when they are added or removed).
The bus driver registers a peripheral for the bus controller at boot time. I
thought about registering the peripheral only when the system switches from
slave to master, and deregistering it when it switches back to slave mode,
but that's not very clean as the bus controller is always attached to the
CPU, regardless of its bus access mode. Beside, I need to access the bus
controller to detect master <-> slave transitions, so this would be a bit
hackish.
Is there a kobject netlink event I could use that I haven't thought about ? Am
I missing a pseudo-peripheral in my implementation ? Is there a preferred way
to notify userspace of such events ?
Thanks for any help you can provide (and thanks for having read this mail
throughout :-)).
Best regards,
--
Laurent Pinchart
CSE Semaphore Belgium
Chaussée de Bruxelles, 732A
B-1410 Waterloo
Belgium
T +32 (2) 387 42 59
F +32 (2) 387 42 75
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes
From: Anton Vorontsov @ 2007-11-27 16:41 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev, linux-ide
In-Reply-To: <474C3C79.5060908@ru.mvista.com>
On Tue, Nov 27, 2007 at 06:49:13PM +0300, Sergei Shtylyov wrote:
> Hello.
Hi Sergei,
> Anton Vorontsov wrote:
>
> >This patch adds localbus and pata nodes to use CF IDE interface
> >on MPC8349E-mITX boards.
>
> >Patch also adds code to probe localbus.
>
> >Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> >---
> > arch/powerpc/boot/dts/mpc8349emitx.dts | 17 ++++++++++++++++-
> > arch/powerpc/platforms/83xx/mpc834x_itx.c | 17 +++++++++++++++++
> > 2 files changed, 33 insertions(+), 1 deletions(-)
>
> >diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts
> >b/arch/powerpc/boot/dts/mpc8349emitx.dts
> >index 5072f6d..7a97068 100644
> >--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
> >+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
> >@@ -249,6 +249,21 @@
> > device_type = "pci";
> > };
> >
> >+ localbus@e0005000 {
> >+ #address-cells = <2>;
> >+ #size-cells = <1>;
> >+ compatible = "fsl,mpc8349emitx-localbus",
>
> Board compatible bus?
This is what Documentation/powerpc/booting-without-of.txt suggests
for localbuses. I'm following.
> >+ "fsl,mpc8349e-localbus",
> >+ "fsl,pq2pro-localbus";
> >+ reg = <e0005000 d8>;
> >+ ranges = <3 0 f0000000 210>;
> >
> >-
> >+ pata@3,0 {
> >+ compatible = "fsl,mpc8349emitx-pata",
> >"pata-platform";
> >+ reg = <3 0 10 3 20c 4>;
> >+ ioport-shift = <1>;
>
> Bleh... that shift again. And this is surely not a good name for a
> property (where's I/O ports in your case?) -- why not call it "reg-shift"
> (well, I'd call it "reg-size" or "reg-stride" myself :-)?
1. "shift" because pata_platform using that name. I don't see any
reason to contrive indirections. ioport-shift is what the whole
Linux kernel using nowadays, and ioport-shift dts property
anyway Linux-specific.
I'm just following todays' conventions.
If you feel really bad about that, I think better to fix that in
the source of the badness -- pata_platform. It's easy, I can do
that. Would you ack patch that converts whole pata_platform and
users? Would Paul ack it?
Still, is there any hardware that needs not power of 2 stride?
2. "ioport" because shift^Wstride ;-) applies only to the io range
(yes, it's obvious, but worth open-wording, no?).
And btw, I can get rid of ioport-shift at all. And do fixups in
the pata_of_platform driver via .compatible matching. But I don't
want: it feels bad to list every needs-to-fixup board in the common
driver. It also feels not so great creating something like
pata-platform-stride-{1,2,4,...} compatible stuff. Heh.
Thanks,
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: The question about the high memory support on MPC8360?
From: Scott Wood @ 2007-11-27 17:02 UTC (permalink / raw)
To: vijay baskar; +Cc: 郭劲, linuxppc-embedded
In-Reply-To: <474B9CC0.2000803@gdatech.co.in>
vijay baskar wrote:
> The kernel maps the last 1 GB of the virtual address space one to one
> to the physical memory.
No, it maps 768MB of RAM in this manner.
> This is called the kernel space. After the one
> to one mapping is done for the available physical memory, the
> remaining virtual addresses are used for vmalloc and ioremap.
And highmem mappings.
> The kernel also allows hardcoded mapping
> of IO regions into its virtual address space through the
> io_block_mapping interface.
Not in current arch/powerpc kernels.
> Many boards use the block IO mapping to
> map the CCSRBAR/IMMR into the kernel address space, such that the
> physical address and the virutal address is the same. Virtual
> addresses beyond these hardcoded mappings cannot be used by
> vmalloc/ioremap.
And this is why.
> Now as more and more memory is added to the system the addresses
> available for vmalloc and ioremap gets reduced, and memory allocations
> start to fail, due to the lack of availability of virtual addresses.
How so? The size of lowmem is constant once you reach the threshold, as
is the size of the highmem mapping area.
What *does* start to fail eventually, if you have a *lot* of highmem, is
that you run out of lowmem for pagetables and such.
-Scott
^ permalink raw reply
* Re: [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes
From: Sergei Shtylyov @ 2007-11-27 16:46 UTC (permalink / raw)
To: avorontsov; +Cc: linuxppc-dev, linux-ide
In-Reply-To: <20071127164101.GA14790@localhost.localdomain>
Anton Vorontsov wrote:
>>>This patch adds localbus and pata nodes to use CF IDE interface
>>>on MPC8349E-mITX boards.
>>>Patch also adds code to probe localbus.
>>>Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>>>---
>>>arch/powerpc/boot/dts/mpc8349emitx.dts | 17 ++++++++++++++++-
>>>arch/powerpc/platforms/83xx/mpc834x_itx.c | 17 +++++++++++++++++
>>>2 files changed, 33 insertions(+), 1 deletions(-)
>>>diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts
>>>b/arch/powerpc/boot/dts/mpc8349emitx.dts
>>>index 5072f6d..7a97068 100644
>>>--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
>>>+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
>>>@@ -249,6 +249,21 @@
>>> device_type = "pci";
>>> };
>>>
>>>+ localbus@e0005000 {
>>>+ #address-cells = <2>;
>>>+ #size-cells = <1>;
>>>+ compatible = "fsl,mpc8349emitx-localbus",
>>
>> Board compatible bus?
> This is what Documentation/powerpc/booting-without-of.txt suggests
> for localbuses. I'm following.
Hm...
>>>+ "fsl,mpc8349e-localbus",
>>>+ "fsl,pq2pro-localbus";
>>>+ reg = <e0005000 d8>;
>>>+ ranges = <3 0 f0000000 210>;
>>>
>>>-
>>>+ pata@3,0 {
>>>+ compatible = "fsl,mpc8349emitx-pata",
>>>"pata-platform";
>>>+ reg = <3 0 10 3 20c 4>;
>>>+ ioport-shift = <1>;
>> Bleh... that shift again. And this is surely not a good name for a
>>property (where's I/O ports in your case?) -- why not call it "reg-shift"
>>(well, I'd call it "reg-size" or "reg-stride" myself :-)?
> 1. "shift" because pata_platform using that name. I don't see any
> reason to contrive indirections. ioport-shift is what the whole
> Linux kernel using nowadays, and ioport-shift dts property
> anyway Linux-specific.
It's just a bad name. There's not even I/O ports in this case (and
moreover, the *real* I/O mapped device would always have a shift of 0, I bet
-- larger strides are for memory mapped devices).
> I'm just following todays' conventions.
> If you feel really bad about that, I think better to fix that in
> the source of the badness -- pata_platform. It's easy, I can do
I only feel really bad about the "ioport" part, I can live with "shift"
part. :-)
> that. Would you ack patch that converts whole pata_platform and
> users? Would Paul ack it?
I don't understand -- why the property name should duplicate pata_platform
field name? :-O
> Still, is there any hardware that needs not power of 2 stride?
Not really -- "size" just seems better, aesthetically. :-)
> 2. "ioport" because shift^Wstride ;-) applies only to the io range
> (yes, it's obvious, but worth open-wording, no?).
Contrarywise, to memory range.
> And btw, I can get rid of ioport-shift at all. And do fixups in
> the pata_of_platform driver via .compatible matching. But I don't
> want: it feels bad to list every needs-to-fixup board in the common
> driver. It also feels not so great creating something like
> pata-platform-stride-{1,2,4,...} compatible stuff. Heh.
I didn't propose neither of that. :-)
All I want is that "ioport-*" be renamed.
> Thanks,
MBR, Sergei
^ permalink raw reply
* Re: [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes
From: Anton Vorontsov @ 2007-11-27 17:27 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev, linux-ide
In-Reply-To: <474C49D5.2000206@ru.mvista.com>
On Tue, Nov 27, 2007 at 07:46:13PM +0300, Sergei Shtylyov wrote:
[...]
> >>>+ ioport-shift = <1>;
>
> >> Bleh... that shift again. And this is surely not a good name for a
> >>property (where's I/O ports in your case?) -- why not call it "reg-shift"
> >>(well, I'd call it "reg-size" or "reg-stride" myself :-)?
>
> >1. "shift" because pata_platform using that name. I don't see any
> > reason to contrive indirections. ioport-shift is what the whole
> > Linux kernel using nowadays, and ioport-shift dts property
> > anyway Linux-specific.
>
> It's just a bad name. There's not even I/O ports in this case (and
> moreover, the *real* I/O mapped device would always have a shift of 0, I
> bet -- larger strides are for memory mapped devices).
>
> > I'm just following todays' conventions.
>
> > If you feel really bad about that, I think better to fix that in
> > the source of the badness -- pata_platform. It's easy, I can do
>
> I only feel really bad about the "ioport" part, I can live with "shift"
> part. :-)
>
> > that. Would you ack patch that converts whole pata_platform and
> > users? Would Paul ack it?
>
> I don't understand -- why the property name should duplicate
> pata_platform field name? :-O
Because:
> >1. [...] I don't see any reason to contrive indirections.
That is, different names for single thing is worse than single
bogus name.
> Not really -- "size" just seems better, aesthetically. :-)
reg-size will look confusing. Is it ata registers' size? No,
can't be. So, what is it? It's stride/shift because of bus, on
which ata resides.
> >And btw, I can get rid of ioport-shift at all. And do fixups in
> >the pata_of_platform driver via .compatible matching. But I don't
> >want: it feels bad to list every needs-to-fixup board in the common
> >driver. It also feels not so great creating something like
> >pata-platform-stride-{1,2,4,...} compatible stuff. Heh.
>
> I didn't propose neither of that. :-)
Yup, that was "by the way"...
> All I want is that "ioport-*" be renamed.
I give up.
The final name is..? I can think out wrong one, so you'd better
convoy me on that way. ;-) reg-shift sounds okay? Or reg-stride
better? No size, please.
Thanks,
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes
From: Sergei Shtylyov @ 2007-11-27 17:25 UTC (permalink / raw)
To: avorontsov; +Cc: linuxppc-dev, linux-ide
In-Reply-To: <20071127172758.GA15429@localhost.localdomain>
Anton Vorontsov wrote:
>> All I want is that "ioport-*" be renamed.
> I give up.
Don't. :-)
> The final name is..? I can think out wrong one, so you'd better
> convoy me on that way. ;-) reg-shift sounds okay?
Yes.
> Thanks,
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes
From: Anton Vorontsov @ 2007-11-27 17:34 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev, linux-ide
In-Reply-To: <474C49D5.2000206@ru.mvista.com>
[ Had cut too much in the previous reply ]
On Tue, Nov 27, 2007 at 07:46:13PM +0300, Sergei Shtylyov wrote:
[...]
> >2. "ioport" because shift^Wstride ;-) applies only to the io range
> > (yes, it's obvious, but worth open-wording, no?).
>
> Contrarywise, to memory range.
By io range I meant "I/O base", in contrast to "CTL base".
There is no need to apply shifting for CTL. That's why ioport-*
appeared in the first place.
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes
From: Sergei Shtylyov @ 2007-11-27 17:34 UTC (permalink / raw)
To: avorontsov; +Cc: linuxppc-dev, linux-ide
In-Reply-To: <20071127173456.GA15693@localhost.localdomain>
Anton Vorontsov wrote:
>>>2. "ioport" because shift^Wstride ;-) applies only to the io range
>>> (yes, it's obvious, but worth open-wording, no?).
>> Contrarywise, to memory range.
> By io range I meant "I/O base", in contrast to "CTL base".
> There is no need to apply shifting for CTL. That's why ioport-*
> appeared in the first place.
So, a matter of wrong terminology then. The thing that you meant by "I/O"
is actually called "command block".
MBR, Sergei
^ permalink raw reply
* Re: [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes
From: Anton Vorontsov @ 2007-11-27 17:48 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linuxppc-dev, linux-ide
In-Reply-To: <474C5513.6010801@ru.mvista.com>
On Tue, Nov 27, 2007 at 08:34:11PM +0300, Sergei Shtylyov wrote:
> Anton Vorontsov wrote:
>
> >>>2. "ioport" because shift^Wstride ;-) applies only to the io range
> >>> (yes, it's obvious, but worth open-wording, no?).
>
> >> Contrarywise, to memory range.
>
> >By io range I meant "I/O base", in contrast to "CTL base".
>
> >There is no need to apply shifting for CTL. That's why ioport-*
> >appeared in the first place.
>
> So, a matter of wrong terminology then. The thing that you meant by
> "I/O" is actually called "command block".
Yes. And IO is the second name. It's used widespread in the
drivers/ide/.
Now you understand why I'm so reluctant to hanging up different
labels on the single thing? ;-)
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
^ permalink raw reply
* Re: 85xx software reset problems from paulus.git
From: Kumar Gala @ 2007-11-27 16:26 UTC (permalink / raw)
To: Dale Farnsworth; +Cc: linuxppc-embedded
In-Reply-To: <20071127145424.8925.qmail@farnsworth.org>
On Nov 27, 2007, at 8:54 AM, Dale Farnsworth wrote:
> I wrote:
>> Kumar Gala wrote:
>>>>> Yes. The symptoms in 2.6.24RC2 are that during a kernel panic or
>>>>> when
>>>>> calling 'reboot' in the shell, it just hangs. Using the same dts
>>>>> and
>>>>> resets in 2.6.23.1 reboots fine. I don't have a cds reference, but
>>>>> someone who does should be able to confirm whether the issue
>>>>> exists
>>>>> or
>>>>> not by just attempting to reboot via bash.
>>>
>>> Can someone do a git-bisect and find the patch that breaks things?
>>
>> I'll see if I can reproduce it on my 8548cds. If so, I'll then git-
>> bisect.
>
> I tried this with the current powerpc.git tree on my mpc8548cds, and
> the reboot command works for me. The system rebooted just fine.
I'm wondering if there is an issue rebooting if we are in an oops.
- k
^ permalink raw reply
* Re: [PATCH 3/3] [POWERPC] MPC8349E-mITX: introduce localbus and pata nodes
From: Sergei Shtylyov @ 2007-11-27 18:07 UTC (permalink / raw)
To: avorontsov; +Cc: linuxppc-dev, linux-ide
In-Reply-To: <20071127174826.GA15980@localhost.localdomain>
Anton Vorontsov wrote:
>>>>>2. "ioport" because shift^Wstride ;-) applies only to the io range
>>>>>(yes, it's obvious, but worth open-wording, no?).
>>>> Contrarywise, to memory range.
>>>By io range I meant "I/O base", in contrast to "CTL base".
>>>There is no need to apply shifting for CTL. That's why ioport-*
>>>appeared in the first place.
>> So, a matter of wrong terminology then. The thing that you meant by
>> "I/O" is actually called "command block".
> Yes. And IO is the second name.
I'd say the first place in drivers/ide belongs to the historic name
"taskfile". The "command block" which is as ATA standard calls it, is hardly used.
> It's used widespread in the drivers/ide/.
Don't remember seeing it.
> Now you understand why I'm so reluctant to hanging up different
> labels on the single thing? ;-)
:-)
MBR, Sergei
^ permalink raw reply
* Re: Linuxppc-embedded Digest, Vol 39, Issue 48
From: fabio777 @ 2007-11-27 18:59 UTC (permalink / raw)
To: linuxppc-embedded, bwarren
In-Reply-To: <mailman.3149.1196114204.3099.linuxppc-embedded@ozlabs.org>
Thanks Ben,
Here it is
static struct fsl_spi_platform_data k_platform_data = {
.initial_spmode = 0,
.bus_num = 1,
.max_chipselect = 1,
/* board specific information */
.activate_cs = k_cs_activate,
.deactivate_cs = k_cs_deactivate,
.sysclk = 266,
};
static struct spi_board_info spi_board_info[] __initdata = { {
.modalias = "kplus",
.platform_data = &k_platform_data,
.max_speed_hz = 120000,
.bus_num = 1,
.chip_select = 0,
},
};
struct platform_device k_plus = {
.name = "kplus",
.id = 1,
.dev = {
.platform_data = &k_platform_data,
},
};
platform_device_register(&k_plus);
spi_register_board_info(spi_board_info, ARRAY_SIZE(spi_board_info))
and then calls spi_register_driver(&k_driver);
I can't get the into the *probe functions.
Thanks
>
> fabio777 wrote:
>
>> Has anyone been using this driver ?
>>
>>
> I use it, on ARCH=powerpc, though.
>
>> For legacy reasons I need to keep the ppc=arch however I haven't found
>> out how to get this driver probed at start-up even basing my init on
>> Lublock.
>>
>>
>>
> The driver's expecting a platform device with name "mpc83xx_spi" to be
> registered in board init code. If you post your init code I may be able
> to help.
>
> regards,
> Ben
>
>
^ 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