LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Bugfix to commit 4f9a58d75bfe82ab2b8ba5b8506dfb190a267834
From: Grant Likely @ 2007-10-22 22:38 UTC (permalink / raw)
  To: Olaf Hering, linux-kernel, Linus Torvalds, Paul Mackerras,
	linuxppc-dev

From: Grant Likely <grant.likely@secretlab.ca>

Commit 4f9a58d75bfe82ab2b8ba5b8506dfb190a267834 changes the size of
AT_VECTOR_SIZE from hard coded '44' to a calculation based on the value
of AT_VECTOR_SIZE_ARCH and AT_VECTOR_SIZE_BASE.  The change works for
arch/powerpc, but it breaks arch/ppc because the needed
AT_VECTOR_SIZE_ARCH is not present in include/asm-ppc/system.h and a
default value of 0 is used instead.  This results in AT_VECTOR_SIZE
being too small and it causes a kernel crash on loading init.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

I think this bug fix needs to go in ASAP.  I cannot boot my Virtex ppc405
platform without it.

Olaf, do I have the correct solution here?

Thanks,
g.

 include/asm-ppc/system.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/asm-ppc/system.h b/include/asm-ppc/system.h
index cc45780..51df94c 100644
--- a/include/asm-ppc/system.h
+++ b/include/asm-ppc/system.h
@@ -33,6 +33,7 @@
 
 #define set_mb(var, value)	do { var = value; mb(); } while (0)
 
+#define AT_VECTOR_SIZE_ARCH 6 /* entries in ARCH_DLINFO */
 #ifdef CONFIG_SMP
 #define smp_mb()	mb()
 #define smp_rmb()	rmb()

^ permalink raw reply related

* Please pull powerpc.git master branch
From: Paul Mackerras @ 2007-10-23  0:08 UTC (permalink / raw)
  To: torvalds; +Cc: linuxppc-dev

Linus,

Please do

git pull \
git://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git master

to get five more commits that fix some bugs for systems using the
Freescale embedded 52xx processors and enable them to restart
properly.

Thanks,
Paul.

 .../powerpc/mpc52xx-device-tree-bindings.txt       |    4 +
 arch/powerpc/boot/dts/lite5200.dts                 |   26 +++----
 arch/powerpc/boot/dts/lite5200b.dts                |   26 +++----
 arch/powerpc/platforms/52xx/lite5200.c             |    4 +
 arch/powerpc/platforms/52xx/mpc52xx_common.c       |   71 +++++++++++++++++++-
 arch/powerpc/sysdev/bestcomm/bestcomm.c            |    9 ++-
 drivers/watchdog/mpc5200_wdt.c                     |    3 +
 include/asm-powerpc/mpc52xx.h                      |    9 +++
 8 files changed, 109 insertions(+), 43 deletions(-)

Grant Likely (1):
      [POWERPC] bestcomm: Restrict bus prefetch bugfix to original mpc5200 silicon.

Marian Balakowicz (4):
      [POWERPC] Add mpc52xx_find_and_map_path(), refactor utility functions
      [POWERPC] Update device tree binding for mpc5200 gpt
      [POWERPC] Add restart support for mpc52xx based platforms
      [POWERPC] Enable restart support for lite5200 board

^ permalink raw reply

* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
From: Linas Vepstas @ 2007-10-23  0:13 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci,
	David Miller
In-Reply-To: <1193088267.6745.108.camel@pasglop>

On Tue, Oct 23, 2007 at 07:24:27AM +1000, Benjamin Herrenschmidt wrote:
> 
> On Mon, 2007-10-22 at 13:13 -0500, Linas Vepstas wrote:
> > On Mon, Oct 22, 2007 at 11:49:24AM +1000, Michael Ellerman wrote:
> > > 
> > > On pseries there's a chance it will work for PCI error recovery, but if
> > > so it's just lucky that firmware has left everything configured the same
> > > way. 
> > 
> > ? The papr is quite clear that i is up to the OS to restore the msi
> > state after an eeh error.
> 
> Via direct config space access or via firmware change-msi calls ?

Direct config space access. It says that the OS is supposed to read the
MSI config (after its been set up), save it, and restore it, (via direct
config space writes) if the device is ever reset.

> I don't know why you keep talking about powerpc laptops here ... 

Well, there are Apple laptops, right?  Aren't those the "powermac" 
platform?  Now, I don't know if they support MSI, but if they do,
I get the impression that they might not restore msi state correctly,
after being put into hardware suspend.  But perhaps I'm mistaken;
I was simply grepping for various msi-related functions in various
arch subdirectories, comparing x86 to other arches, and noticed 
that code that would restore msi state seems to be missing for
most arches and most powerpc platforms.

--linas

^ permalink raw reply

* Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
From: David Miller @ 2007-10-23  0:23 UTC (permalink / raw)
  To: linas; +Cc: netdev, mcarlson, linuxppc-dev, mchan, linux-pci
In-Reply-To: <20071022195451.GE4280@austin.ibm.com>

From: linas@austin.ibm.com (Linas Vepstas)
Date: Mon, 22 Oct 2007 14:54:52 -0500

> As discussed in the other thread, I'll try to set up a patch
> for an arch callback for restoring msi state.

Thank you.

^ permalink raw reply

* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
From: Benjamin Herrenschmidt @ 2007-10-23  0:29 UTC (permalink / raw)
  To: Linas Vepstas
  Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci,
	David Miller
In-Reply-To: <20071023001307.GF4280@austin.ibm.com>


> > I don't know why you keep talking about powerpc laptops here ... 
> 
> Well, there are Apple laptops, right?  Aren't those the "powermac" 
> platform?  Now, I don't know if they support MSI, but if they do,
> I get the impression that they might not restore msi state correctly,
> after being put into hardware suspend.  But perhaps I'm mistaken;
> I was simply grepping for various msi-related functions in various
> arch subdirectories, comparing x86 to other arches, and noticed 
> that code that would restore msi state seems to be missing for
> most arches and most powerpc platforms.

Ah ok, i see. Well, platforms that use write_msi_msg() shouldn't need
anything special right ? So only pSeries is an issue here....

PowerBooks don't indeed have MSI support, though G5's do and some people
have been toying around with suspend/resume on them (hibernation only at
that stage) but it doesn't matter at this stage. We are specifically
talking about pSeries which is the "special" case here.

Ben.

^ permalink raw reply

* Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
From: Benjamin Herrenschmidt @ 2007-10-23  0:32 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, mcarlson, linuxppc-dev, mchan, linux-pci
In-Reply-To: <20071022.172332.112621497.davem@davemloft.net>


On Mon, 2007-10-22 at 17:23 -0700, David Miller wrote:
> From: linas@austin.ibm.com (Linas Vepstas)
> Date: Mon, 22 Oct 2007 14:54:52 -0500
> 
> > As discussed in the other thread, I'll try to set up a patch
> > for an arch callback for restoring msi state.

>From what it looks like at this stage, pSeries might need to
differenciate restoring MSI state after a device reset (PCI error
recovery) from restoring MSI state after suspend/resume (if we ever
implement that one).

The former apparently require manual saving & restoring of the config
space bits. (Linas, do you have a pointer to the bit of PAPR spec that
specifies that we need to save & restore the MSI message ourselves ?)

For the later (suspend/resume), that will definitely not work, or at
least, will not be enough, especially with something like suspend to
disk, where we'll need to have the firmware reconfigure the MSIs for us
(to make sure, among others, that the interrupt controllers are properly
configured for MSIs etc...).

Ben.

^ permalink raw reply

* Re: [PATCH 4/4] DTC: Begin the path to sane literals and expressions.
From: David Gibson @ 2007-10-23  0:33 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: linuxppc-dev
In-Reply-To: <E1IjwWf-0000dJ-45@jdl.com>

On Mon, Oct 22, 2007 at 07:36:57AM -0500, Jon Loeliger wrote:
> So, like, the other day Segher Boessenkool mumbled:
> > > Property names have been limited to start with
> > > characters from the set [a-zA-Z,._#?].  That is, the
> > > digits and the expression symbols have been removed.
> > 
> > This cannot work; many property names start with a digit,
> > for example.
> 
> Are any of those property names in use in any
> of our DTS files or b-w-o.txt?  Not really, no.
> In fact, with this lexical change, all of our
> DTS files still produce byte-identical results.
> 
> I really think this is one of those areas where
> we may need to stray from the other guideline.
> 
> Is there a compelling reason somewhere?  Really?

We may have deprecated te '64-bit' and '32-64-bridge' properties in
cpu nodes for the flattened tree, but it already exists in a great
number of Apple and IBM trees.  It would be poor form for dtc to choke
on these trees.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH 3/4] DTC: Appease the printf() format $Gods with a correct type.
From: David Gibson @ 2007-10-23  0:26 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, Jon Loeliger
In-Reply-To: <20071022115511.0b902044@weaponx.rchland.ibm.com>

On Mon, Oct 22, 2007 at 11:55:11AM -0500, Josh Boyer wrote:
> On Mon, 22 Oct 2007 11:50:52 -0500
> Jon Loeliger <jdl@jdl.com> wrote:
> 
> > So, like, the other day David Gibson mumbled:
> > > On Sat, Oct 20, 2007 at 10:51:29AM +0200, Andreas Schwab wrote:
> > > > David Gibson <david@gibson.dropbear.id.au> writes:
> > > > 
> > > > > What compiler/platform is this?  I can't off the top of my head think
> > > > > of one where size_t shouldn't be promoted to int automatically.
> > > > 
> > > > Only types narrower than int are subject to integer promotion.
> > > 
> > > Duh, yes.  Sorry.
> > 
> > So, Hmm.  You want the patch as I wrote it applied?
> > Or do you want to use %z in the format instead?  It does
> > seem like there is precedent to do that (in the kernel):
> > 
> >     % grep -r '%z' arch/
> >     arch/alpha/boot/tools/mkbb.c:   fprintf(stderr, "expected %zd, got %d\n", sizeof(bootblock), nread);
> >     arch/alpha/boot/tools/mkbb.c:   fprintf(stderr, "expected %zd, got %d\n", sizeof(bootblock), nread);
> >     arch/m68k/ifpsp060/src/itest.S: mulu.l          (0x00.w,%a3,%za4.l*8),%d2:%d3
> >     arch/m68k/ifpsp060/src/itest.S: mulu.l          (-0x10.w,%za3,%a4.l*1),%d2:%d3
> >     arch/m68k/ifpsp060/src/itest.S: mulu.l          (ea_77_mem+0x00.w,%pc,%za4.l*8),%d2:%d3
> >     ...
> >     arch/m68k/ifpsp060/src/itest.S: mulu.l          ([EASTORE.l,%zpc,%zd4.l*1]),%d2:%d3
> >     arch/m68k/ifpsp060/src/itest.S: mulu.l          ([EASTORE.l,%pc],%zd4.l*8,0x20.l),%d2:%d3
> >     arch/m68k/ifpsp060/src/itest.S: mulu.l          ([EASTORE.l,%zpc],%d4.l*8),%d2:%d3
> >     arch/um/drivers/cow_user.c:             /* Below, %zd is for a size_t value */
> >     arch/um/drivers/cow_user.c:                        "limited to %zd characters\n", backing_file,
> >     arch/x86/kernel/pci-nommu_64.c:                     "nommu_%s: overflow %Lx+%zu of device mask %Lx\n",
> > 
> > Opinions?
> 
> %z is fairly common now-a-days.  The kernel janitors tend to LART
> people for not using it rather quickly.

Yes, I think %z is preferable.  It used to be a glibc (et al)
extension, but I think it's now specified in C99 (we should double
check that).

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH] DTC: Minor grammar rule shuffle.
From: David Gibson @ 2007-10-23  0:35 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: linuxppc-dev
In-Reply-To: <E1Ik5JJ-0000Cp-KT@jdl.com>

On Mon, Oct 22, 2007 at 04:59:45PM -0500, Jon Loeliger wrote:
> I like to see the basis cases established early in
> the rule sets, so place  "empty" reduction first.
> Purely cosmetic.
> 
> Signed-off-by: Jon Loeliger <jdl@freescale.com>

As with the change of { } formatting, I don't really care much either
way on this one.  So

Acked-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH] DTC: Remove an unneeded %token definition.
From: David Gibson @ 2007-10-23  0:37 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: linuxppc-dev
In-Reply-To: <E1Ik5K6-0000EE-DL@jdl.com>

On Mon, Oct 22, 2007 at 05:00:34PM -0500, Jon Loeliger wrote:
> Signed-off-by: Jon Loeliger <jdl@freescale.com>

Oops.  Can't even remeber what that one was ever there for.

Acked-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH] DTC: Rewrite the propdata and propdataprefix rules.
From: David Gibson @ 2007-10-23  0:38 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: linuxppc-dev
In-Reply-To: <E1Ik5uH-0000bV-6C@jdl.com>

On Mon, Oct 22, 2007 at 05:37:57PM -0500, Jon Loeliger wrote:
> So, like, the other day Jon Loeliger mumbled:
> > 
> > After staring at these two rules for a bit, and not quite
> > groking them, I rewrote them into something I think is a
> > bit clearer.  With any luck, it's the same language still.
> > 
> > Signed-off-by: Jon Loeliger <jdl@freescale.com>
> > ---
> > 
> > You be the judge too!
> 
> Yeah, so, I botched this one.  It's NOT the same
> language, so don't apply it or think about it at all...

Yeah, I remember finding the whole propdataprefix confusing myself.
But the form was suggested by someone (forgotten whom now), and I
never did figure out a better way of doing it.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH 4/4] DTC: Begin the path to sane literals and expressions.
From: Segher Boessenkool @ 2007-10-23  0:57 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev, Jon Loeliger
In-Reply-To: <20071023003309.GE31839@localhost.localdomain>

>>>> Property names have been limited to start with
>>>> characters from the set [a-zA-Z,._#?].  That is, the
>>>> digits and the expression symbols have been removed.
>>>
>>> This cannot work; many property names start with a digit,
>>> for example.
>>
>> Are any of those property names in use in any
>> of our DTS files or b-w-o.txt?  Not really, no.
>> In fact, with this lexical change, all of our
>> DTS files still produce byte-identical results.
>>
>> I really think this is one of those areas where
>> we may need to stray from the other guideline.
>>
>> Is there a compelling reason somewhere?  Really?
>
> We may have deprecated te '64-bit' and '32-64-bridge' properties in
> cpu nodes for the flattened tree, but it already exists in a great
> number of Apple and IBM trees.

Huh?  "64-bit" isn't deprecated I hope, just the "32-bit" thing
(that never was defined) is.

> It would be poor form for dtc to choke on these trees.

Yah.  Small incompatibilities add up :-(


Segher

^ permalink raw reply

* Re: [PATCH 3/4] DTC: Appease the printf() format $Gods with a correct type.
From: Segher Boessenkool @ 2007-10-23  1:03 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev, Jon Loeliger
In-Reply-To: <20071023002631.GB31839@localhost.localdomain>

> Yes, I think %z is preferable.  It used to be a glibc (et al)
> extension, but I think it's now specified in C99 (we should double
> check that).

It is.


Segher

^ permalink raw reply

* Audio codec device tree entries
From: Jon Smirl @ 2007-10-23  1:59 UTC (permalink / raw)
  To: PowerPC dev list

Is this what the device tree entries should look like?

First example is ac97 audio:

ac97@2000 {           // PSC1
      device_type = "sound";
      compatible = "mpc5200b-psc-ac97\0mpc5200-psc-ac97";
      cell-index = <0>;
      reg = <2000 100>;
      interrupts = <2 1 0>;
      interrupt-parent = <&mpc5200_pic>;
      codec-handle = <&codec0>
};

pseudo-sound@0 { // use to trigger loading platform specific fabric driver
      device_type = "pseudo-sound"
};

codec0:codec@0 {
      device_type = "codec"
      compatible = "stac9766\0ac97"
};

Second is i2s/i2c connected:
How do I link this to the i2c bus?

i2s@2000 {           // PSC1
      device_type = "sound";
      compatible = "mpc5200b-psc-i2s\0mpc5200-psc-i2s";
      cell-index = <0>;
      reg = <2000 100>;
      interrupts = <2 1 0>;
      interrupt-parent = <&mpc5200_pic>;
      codec-handle = <&codec0>
};

i2c@3d00 {
      device_type = "i2c";
      compatible = "mpc5200b-i2c\0mpc5200-i2c\0fsl-i2c";
      cell-index = <0>;
      reg = <3d00 40>;
      interrupts = <2 f 0>;
      interrupt-parent = <&mpc5200_pic>;
      fsl5200-clocking;
};

pseudo-sound@0 { // use to trigger loading platform specific fabric driver
      device_type = "pseudo-sound"
};

codec0:codec@0 {
      device_type = "codec"
      compatible = "tas5508"
};





-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* Re: [PATCH] Allow sysfs_remove_group() to be called on non-added groups
From: Michael Ellerman @ 2007-10-23  2:02 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <096fae99bd9797c5484ed8f4b87b82e58737cdc0.1189657514.git.michael@ellerman.id.au>

On 9/13/07, Michael Ellerman <michael@ellerman.id.au> wrote:
> It would be nice to be able to do:
>
> for_each_thing(thing) {
>         error = sysfs_create_group(&thing->kobj, attrs);
>         if (error) {
>                 for_each_thing(thing)
>                         sysfs_remove_group(&thing->kobj, attrs);
>                 return error;
>         }
> }
>
> But there's a BUG_ON() in sysfs_remove_group() which hits if the attributes
> were never added.
>
> As discussed here ...
> http://ozlabs.org/pipermail/cbe-oss-dev/2007-July/002774.html
>
> .. we should just return in that case instead of BUG'ing.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
>  fs/sysfs/group.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
> index f318b73..a256775 100644
> --- a/fs/sysfs/group.c
> +++ b/fs/sysfs/group.c
> @@ -73,7 +73,8 @@ void sysfs_remove_group(struct kobject * kobj,
>
>         if (grp->name) {
>                 sd = sysfs_get_dirent(dir_sd, grp->name);
> -               BUG_ON(!sd);
> +               if (!sd)
> +                       return;
>         } else
>                 sd = sysfs_get(dir_sd);
>
>

Hi Greg,

I didn't see this in your series, any objections? AFAICT it still applies.

cheers

^ permalink raw reply

* Re: [PATCH] DTC:  Remove the need for the GLR Parser.
From: David Gibson @ 2007-10-23  2:54 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: linuxppc-dev
In-Reply-To: <E1Ik4aw-0007ud-Lu@jdl.com>

On Mon, Oct 22, 2007 at 04:13:54PM -0500, Jon Loeliger wrote:
> Previously, there were a few shift/reduce and reduce/reduce
> errors in the grammar that were being handled by the not-so-popular
> GLR Parser technique.

I haven't actually heard anyone whinge about glr-parser...

> Flip a right-recursive stack-abusing rule into a left-recursive
> stack-friendly rule and clear up three messes in one shot: No more
> conflicts, no need for the GLR parser, and friendlier stackness.

Ouch.  I'm feeling a bit stupid now, I really thought our conflicts
were somewhere else.  Specifically I thought the problem was that we
needed to look ahead more tokens that we were able to differentiate
between property and subnode definitions, i.e. between:
	label propname =
and
	label propname {

Except... I'm almost certain the conflicts first appeared when I added
labels, and I can't see how that would affect this.  Well, colour me
baffled.

Especially since the comments and content of commit
4102d840d993e7cce7d5c5aea8ef696dc81236fc (second commit in the entire
history!) appear to back up my memory of this.  I used to have a
lookahead hack in the lexer to remove the conflict.

But this patch certainly seems to make the conflicts go away, so I'm
confused.


Well, regardless of that, I have a few concerns.

First, a trivial one: I remember leaving this as a right-recursion,
despite the stack-nastiness, because that way the properties end up in
the same order as in the source.  I think that behaviour is worth
preserving, but of course we can do it with left-recursion by changing
chain_property() to add to the end of the list instead of the
beginning.  Also, if we're going to avoid right-recursion here, we
should do so for the 'subnodes' productions as well, which is
completely analogous.

More significantly, I don't know that we want to burn our bridges with
glr-parser.  glr-parser is a beautiful algorithm which means we can
use essentially whatever form of grammar is the easiest to work with
without having to fiddle about to ensure it's LALR(1).  This could
still be useful if we encounter some less easily finessable grammar
construct in future.

And even without glr-parser, I'm still uncomfortable with the
lexer<->parser execution ordering issues with the current
/dts-version/ proposal.  It may now be true that the order is
guaranteed to be correct, but it's still not exactly obvious.

It seems to me that the version change introduces a lexical change to
the input format, and should therefore be handled at the lexical
level.  And I think there are other potential advantages to parsing
the version identifier as a token, rather than as an integer (such as
being able to define entirely different grammars for different
versions, if we have to).

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: Audio codec device tree entries
From: David Gibson @ 2007-10-23  2:57 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list
In-Reply-To: <9e4733910710221859q6ea54810nba58907d5ddd966d@mail.gmail.com>

On Mon, Oct 22, 2007 at 09:59:00PM -0400, Jon Smirl wrote:
> Is this what the device tree entries should look like?
> 
> First example is ac97 audio:
> 
> ac97@2000 {           // PSC1
>       device_type = "sound";

Kill all these device_type values.

>       compatible = "mpc5200b-psc-ac97\0mpc5200-psc-ac97";

dtc now supports the more readable:
	compatible = "mpc5200b-psc-ac97", "mpc5200-psc-ac97";
Use it.

>       cell-index = <0>;

cell-index should only be present if the device is within a
system-on-chip *and* that device number is used to program some global
register somewhere.

>       reg = <2000 100>;
>       interrupts = <2 1 0>;
>       interrupt-parent = <&mpc5200_pic>;
>       codec-handle = <&codec0>
> };
> 
> pseudo-sound@0 { // use to trigger loading platform specific fabric driver
>       device_type = "pseudo-sound"
> };

This looks completely bogus.  The device tree should represent actual
hardware.

> codec0:codec@0 {

Space after the : please.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: libfdt: Make fdt_string() return a const pointer
From: David Gibson @ 2007-10-23  3:21 UTC (permalink / raw)
  To: Jon Loeliger; +Cc: linuxppc-dev
In-Reply-To: <E1Ik0Hf-0002ws-2r@jdl.com>

On Mon, Oct 22, 2007 at 11:37:43AM -0500, Jon Loeliger wrote:
> So, like, the other day David Gibson mumbled:
> > Currently, fdt_string() returns a (non-const) char *, despite taking a
> > const void *fdt.  This is inconsistent with all the other read-only
> > functions which all return const pointers into the blob.
> > 
> > This patch fixes that.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> Applied.

But apparently not pushed out to the public tree...

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [PATCH] Allow sysfs_remove_group() to be called on non-added groups
From: Greg KH @ 2007-10-23  3:28 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <dc1166600710221902t1383ff88q421a17b1ff1382cd@mail.gmail.com>

On Tue, Oct 23, 2007 at 12:02:39PM +1000, Michael Ellerman wrote:
> On 9/13/07, Michael Ellerman <michael@ellerman.id.au> wrote:
> > It would be nice to be able to do:
> >
> > for_each_thing(thing) {
> >         error = sysfs_create_group(&thing->kobj, attrs);
> >         if (error) {
> >                 for_each_thing(thing)
> >                         sysfs_remove_group(&thing->kobj, attrs);
> >                 return error;
> >         }
> > }
> >
> > But there's a BUG_ON() in sysfs_remove_group() which hits if the attributes
> > were never added.
> >
> > As discussed here ...
> > http://ozlabs.org/pipermail/cbe-oss-dev/2007-July/002774.html
> >
> > .. we should just return in that case instead of BUG'ing.
> >
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> > ---
> >  fs/sysfs/group.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
> > index f318b73..a256775 100644
> > --- a/fs/sysfs/group.c
> > +++ b/fs/sysfs/group.c
> > @@ -73,7 +73,8 @@ void sysfs_remove_group(struct kobject * kobj,
> >
> >         if (grp->name) {
> >                 sd = sysfs_get_dirent(dir_sd, grp->name);
> > -               BUG_ON(!sd);
> > +               if (!sd)
> > +                       return;
> >         } else
> >                 sd = sysfs_get(dir_sd);
> >
> >
> 
> Hi Greg,
> 
> I didn't see this in your series, any objections? AFAICT it still applies.

Hm, I think I just missed it, care to forward it to me "clean" again?

thanks,

greg k-h

^ permalink raw reply

* Re: Device trees and audio codecs
From: Grant Likely @ 2007-10-23  3:24 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list
In-Reply-To: <9e4733910710211729k2f2a065cm56670d1b0f90a1af@mail.gmail.com>

On 10/21/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> I have received conflicting opinions as to whether a codec hooked to
> an ac97 bus should get a chip specific codec entry in the device tree.
> Without the codec specific entry only generic ac97 features can be
> used. The Efika has a STA9766. Looking at the data sheet for the chip
> I see that it implements some proprietary functions in addition to the
> standard ones.

You definitely want the codec node on the control bus.  This is
analogous to how Ethernet PHYs are handled.  The PHY nodes are
children of an MDIO node (because that's the control path).  The
ethernet MAC node contains a phandle to the PHY.

In I2S/I2C terms, the CODEC ~= MAC, I2C ~= MDIO and I2S ~= MAC.

In AC97 terms this analogy doesn't work because AC97 doesn't separate
the control and data interfaces.  An AC97 codec is simply a child of
an AC97 controller.

For the MPC5200; the device tree should reflect the required mode.  If
the PSC needs to be in AC97 mode, then the device tree should say
compatible = "fsl,mpc5200b-psc-ac97".  If the PSC needs to be in I2C
mode, then it should be compatible = "fsl,mpc5200b-psc-i2s".  For the
5200 PSCs, the device node not only reflects the hardware (a PSC
core), but also reflects the schematic design (it is wired either as
an I2S bus or an AC97 bus).

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* [IRQ]: Fix synchronize_irq races with IRQ handler
From: Herbert Xu @ 2007-10-23  3:26 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Linux Kernel Mailing List, linuxppc-dev, Thomas Gleixner, akpm,
	Linus Torvalds, Ingo Molnar
In-Reply-To: <1193001005.6745.34.camel@pasglop>

On Mon, Oct 22, 2007 at 07:10:05AM +1000, Benjamin Herrenschmidt wrote:
>
> Hrm... not on yet. Herbert, care to resend, looks like it fell down the
> wrong hole in Linus mailbox :-)

Thanks for the reminder Ben.  Here it is again:

[IRQ]: Fix synchronize_irq races with IRQ handler

As it is some callers of synchronize_irq rely on memory barriers
to provide synchronisation against the IRQ handlers.  For example,
the tg3 driver does

	tp->irq_sync = 1;
	smp_mb();
	synchronize_irq();

and then in the IRQ handler:

	if (!tp->irq_sync)
		netif_rx_schedule(dev, &tp->napi);

Unfortunately memory barriers only work well when they come in
pairs.  Because we don't actually have memory barriers on the
IRQ path, the memory barrier before the synchronize_irq() doesn't
actually protect us.

In particular, synchronize_irq() may return followed by the
result of netif_rx_schedule being made visible.

This patch (mostly written by Linus) fixes this by using spin
locks instead of memory barries on the synchronize_irq() path.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 80eab7a..1f31422 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -29,12 +29,28 @@
 void synchronize_irq(unsigned int irq)
 {
 	struct irq_desc *desc = irq_desc + irq;
+	unsigned int status;
 
 	if (irq >= NR_IRQS)
 		return;
 
-	while (desc->status & IRQ_INPROGRESS)
-		cpu_relax();
+	do {
+		unsigned long flags;
+
+		/*
+		 * Wait until we're out of the critical section.  This might
+		 * give the wrong answer due to the lack of memory barriers.
+		 */
+		while (desc->status & IRQ_INPROGRESS)
+			cpu_relax();
+
+		/* Ok, that indicated we're done: double-check carefully. */
+		spin_lock_irqsave(&desc->lock, flags);
+		status = desc->status;
+		spin_unlock_irqrestore(&desc->lock, flags);
+
+		/* Oops, that failed? */
+	} while (status & IRQ_INPROGRESS);
 }
 EXPORT_SYMBOL(synchronize_irq);
 

^ permalink raw reply related

* RE: [PATCH] [POWERPC] ucc_geth: Eliminate compile warnings
From: Li Yang-r58472 @ 2007-10-23  3:38 UTC (permalink / raw)
  To: Medve Emilian-EMMEDVE1, David Miller; +Cc: netdev, jgarzik, linuxppc-dev
In-Reply-To: <598D5675D34BE349929AF5EDE9B03E2701685446@az33exm24.fsl.freescale.net>

> -----Original Message-----
> From: Medve Emilian-EMMEDVE1=20
> Sent: Monday, October 22, 2007 9:48 PM
> To: David Miller
> Cc: jgarzik@pobox.com; Li Yang-r58472;=20
> netdev@vger.kernel.org; linuxppc-dev@ozlabs.org
> Subject: RE: [PATCH] [POWERPC] ucc_geth: Eliminate compile warnings
>=20
> Hello David,
>=20
>=20
> > No piece of code in the kernel should live in a vacuum.
> >=20
> > In order to improve overall code quality, every piece of=20
> driver code=20
> > should avoid assuming things about pointer sizes and things of this=20
> > nature.
>=20
> I'm afraid we might be talking about orthogonal issues here.=20
> I actively agree that all code (not only kernel) should be=20
> written up to the coding/quality standards you mention above,=20
> but I see a difference between fixing a warning and making a=20
> driver portable (to 64-bit PowerPCs, to other platforms,=20
> etc.). If there is a kernel todo list somewhere lets add to=20
> it the task to make the ucc_geth more portable.
>=20
> > Then the driver can get enabled into the build on every=20
> platform, and=20
> > therefore nobody will break the build of this driver again since it=20
> > will get hit by "allmodconfig"
> > et al. builds even on platforms other than the one it is meant for.
> >=20
> > This hack fix is not acceptable, really.
>=20
> Are you suggesting we leave those warnings there until=20
> somebody decides to fix all the portability issues of this=20
> driver? My patch is a small and insignificant improvement and=20
> not the revolution you're asking for, but is an small=20
> improvement today (I dislike warnings) vs. an improbable big=20
> one in the future.

I'd say we can not use our way of doing things while working with the
community.  The community has to consider the kernel as a whole and thus
has its own virtue.  The warning has been there for some time.  It stays
as an indicator that we have something to do to improve the portability.
I will work on a patch to fix this portability issue.

- Leo

^ permalink raw reply

* Re: Audio codec device tree entries
From: Grant Likely @ 2007-10-23  3:57 UTC (permalink / raw)
  To: Jon Smirl; +Cc: PowerPC dev list
In-Reply-To: <9e4733910710221859q6ea54810nba58907d5ddd966d@mail.gmail.com>

On 10/22/07, Jon Smirl <jonsmirl@gmail.com> wrote:
> Is this what the device tree entries should look like?
>
> First example is ac97 audio:
>
> ac97@2000 {           // PSC1
>       device_type = "sound";
>       compatible = "mpc5200b-psc-ac97\0mpc5200-psc-ac97";

This format is so, like. 3 months ago.  :-)

dtc now supports a comma separated format:  property = "string1",
"string2", "string3"

Embedded nulls no longer needed.

>       cell-index = <0>;
>       reg = <2000 100>;
>       interrupts = <2 1 0>;
>       interrupt-parent = <&mpc5200_pic>;
>       codec-handle = <&codec0>
> };

Isn't this the ac97 bus node?  Can't the ac97 codecs simply be
children of this bus?

>
> pseudo-sound@0 { // use to trigger loading platform specific fabric driver
>       device_type = "pseudo-sound"
> };

I don't think this is a good idea.  The device tree is for describing
your hardware; so the layout should reflect that.  Make the platform
code do the right thing with the real nodes.

>
> codec0:codec@0 {
>       device_type = "codec"
>       compatible = "stac9766\0ac97"
> };
>
> Second is i2s/i2c connected:
> How do I link this to the i2c bus?
>
> i2s@2000 {           // PSC1
>       device_type = "sound";
>       compatible = "mpc5200b-psc-i2s\0mpc5200-psc-i2s";
>       cell-index = <0>;
>       reg = <2000 100>;
>       interrupts = <2 1 0>;
>       interrupt-parent = <&mpc5200_pic>;
>       codec-handle = <&codec0>
> };
>
> i2c@3d00 {
>       device_type = "i2c";
>       compatible = "mpc5200b-i2c\0mpc5200-i2c\0fsl-i2c";
>       cell-index = <0>;
>       reg = <3d00 40>;
>       interrupts = <2 f 0>;
>       interrupt-parent = <&mpc5200_pic>;
>       fsl5200-clocking;
> };
>
> pseudo-sound@0 { // use to trigger loading platform specific fabric driver
>       device_type = "pseudo-sound"
> };
>
> codec0:codec@0 {
>       device_type = "codec"
>       compatible = "tas5508"
> };

I think this is what you want:

ac97@2000 {           // PSC1
      compatible = "fsl,mpc5200b-psc-ac97","fsl,mpc5200-psc-ac97";
      cell-index = <0>;
      reg = <2000 100>;
      #address-cells = <1>;
      #size-cells = <0>;
      interrupts = <2 1 0>;
      interrupt-parent = <&mpc5200_pic>;
      // The following codec stuff could be a little off; It has been
a while since I've looked
      // at ac97 codec interfacing, but if I remember correctly you
can have multiple
      // codecs on an ac97 bus; an audio codec and a modem codec.  The following
      // reflects that understanding...
      ac97-codec@0 {
            // This could be the audio codec
            reg = <0>;
            compatible = "stac9766","ac97-audio"
      };
      ac97-codec@1 {
            // This could be the modem codec
            reg = <1>;
            compatible = "super-sexy-modem-codec","ac97-modem"
      };
};


// Now here's a big example for i2c.
// I've shown 3 audio interfaces; 2 i2s busses and 1 i2c controller.
// This may not be realistic on a 5200, but it is a possible scenario
and this shows
// that it can be handled elegantly.
i2s@2000 {           // PSC1
      compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
      cell-index = <0>;
      reg = <2000 100>;
      interrupts = <2 1 0>;
      interrupt-parent = <&mpc5200_pic>;

      // There are 2 codecs on this i2s bus; either only one at a time
can be used, or
      // (if both the i2s controller and codecs support it) both at
the same time if audio
      // stream interleaving is supported.
      codec-handle = <&codec0 &codec2>;
};

i2s@2200 {           // PSC2
      compatible = "fsl,mpc5200b-psc-i2s","fsl,mpc5200-psc-i2s";
      cell-index = <0>;
      reg = <2200 100>;
      interrupts = <2 2 0>;
      interrupt-parent = <&mpc5200_pic>;
      codec-handle = <&codec1>;
};

i2c@3d00 {
      compatible = "fsl,mpc5200b-i2c", "fsl,mpc5200-i2c", "fsl-i2c";
      #address-cells = <1>;
      #size-cells = <0>;
      cell-index = <0>;
      reg = <3d00 40>;
      interrupts = <2 f 0>;
      interrupt-parent = <&mpc5200_pic>;
      fsl5200-clocking;

      codec0: i2s-codec@0 {
            compatible = "<mfg>,tas5508";
            reg = <0>;
      };
      codec1: i2s-codec@1 {
            compatible = "<mfg>,tas5508";
            reg = <1>;
      };
      codec2: i2s-codec@2 {
            compatible = "<mfg>,tas5508";
            reg = <2>;
      };
};

So, this scheme describes the hardware, but it does not describe 2 of
your questions:

1. How does the device tree describe the myriad of possible circuits
that an audio codec can get dropped into, and
2. How do the drivers get probed

For question #1, I think the answer is simple... it doesn't try.  :-)

As was described in the previous thread, trying to describe the
circuit is very complex and figuring out what software would do with
such a description is even worse.  Instead, I propose the following:
- as much as possible, make the board firmware and the platform setup
code configure the GPIOs, port_config, and other 'fixed' configuration
- make the driver code look at either the board model/compatible
properties or add a board-unique value to the codec's compatible list.
 Either way the driver can apply board specific tweaks to its
behavious.  (I think the better solution is to add a value to the
front of the codec's compatible list because the same circuit can be
used on a different board and it can then claim compatibility with the
older board for just that part of the circuit).

(Segher, what are your thoughts on my above suggestion?)

As for question #2, I think you make the i2s driver probe on the i2s
bus node and the ac97 driver probe on the ac97 bus node.  In both
cases, the driver can find the link to the codec; determine what kind
of system it is running on, and instantiate the appropriate ASoC
fabric driver.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

^ permalink raw reply

* Problems with allyesconfig kernel build
From: Stephen Rothwell @ 2007-10-23  4:02 UTC (permalink / raw)
  To: ppc-dev; +Cc: Andrew Morton, amodra

[-- Attachment #1: Type: text/plain, Size: 2837 bytes --]

This was first noted with the -mm tree, but has now migrated into Linus'
tree.  An allyesconfig build dies in the link stage like this:

/usr/bin/ld: arch/powerpc/kernel/head_64.o(.text+0x80c8): sibling call optimization to `.text.init.refok' does not allow automatic multiple TOCs; recompile with -mminimal-toc or -fno-optimize-sibling-calls, or make `.text.init.refok' extern
/usr/bin/ld: arch/powerpc/kernel/head_64.o(.text+0x8160): sibling call optimization to `.text.init.refok' does not allow automatic multiple TOCs; recompile with -mminimal-toc or -fno-optimize-sibling-calls, or make `.text.init.refok' extern
/usr/bin/ld: arch/powerpc/kernel/head_64.o(.text+0x81c4): sibling call optimization to `.text.init.refok' does not allow automatic multiple TOCs; recompile with -mminimal-toc or -fno-optimize-sibling-calls, or make `.text.init.refok' extern
/usr/bin/ld: final link failed: Bad value

We already compile with -mminimal-toc and adding
-fno-optimize-sibling-call did not help.

Intuiting the obvious, I changed all the _INIT_STATIC and _INIT_GLOBAL
uses in head_64.S back to _STATIC and _GLOBAL (which just moves the code
from .text.init.refok to .text).  Now the linker segfaults instead.  :-)

/bin/sh: line 1:  5260 Segmentation fault      ld -m elf64ppc -Bstatic --emit-relocs --build-id -o .tmp_vmlinux1 -T arch/powerpc/kernel/vmlinux.lds arch/powerpc/kernel/head_64.o arch/powerpc/kernel/entry_64.o arch/powerpc/kernel/fpu.o init/built-in.o --start-group usr/built-in.o arch/powerpc/kernel/built-in.o arch/powerpc/mm/built-in.o arch/powerpc/lib/built-in.o arch/powerpc/sysdev/built-in.o arch/powerpc/platforms/built-in.o arch/powerpc/xmon/built-in.o kernel/built-in.o mm/built-in.o fs/built-in.o ipc/built-in.o security/built-in.o crypto/built-in.o block/built-in.o lib/lib.a lib/built-in.o drivers/built-in.o sound/built-in.o arch/powerpc/oprofile/built-in.o net/built-in.o --end-group
make[1]: *** [.tmp_vmlinux1] Error 139

$ ld --version
GNU ld (GNU Binutils for Debian) 2.18

I take this as an improvement :-)

We link .text.init.refok immediately after .text, but with
allyesconfig, .text ends up very large.

The --emit-relocs is a product of something else I am working on.  So I
took that out and now get a whole lot more messages like:

/usr/bin/ld: net/built-in.o(.fixup+0x4): sibling call optimization to `.text' does not allow automatic multiple TOCs; recompile with -mminimal-toc or -fno-optimize-sibling-calls, or make `.text' extern
/usr/bin/ld: net/built-in.o(.fixup+0xc): sibling call optimization to `.text' does not allow automatic multiple TOCs; recompile with -mminimal-toc or -fno-optimize-sibling-calls, or make `.text' extern

Anyone have any ideas?
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* powerpc: Fix fallout from sg_page() changes
From: Olof Johansson @ 2007-10-23  4:09 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linuxppc-dev, mingo, torvalds, linux-kernel
In-Reply-To: <1193076664-13652-10-git-send-email-jens.axboe@oracle.com>

Fix fallout from 18dabf473e15850c0dbc8ff13ac1e2806d542c15:

In file included from include/linux/dma-mapping.h:52,
                 from drivers/base/dma-mapping.c:10:
include/asm/dma-mapping.h: In function 'dma_map_sg':
include/asm/dma-mapping.h:288: error: 'struct scatterlist' has no member named 'page'
include/asm/dma-mapping.h:288: error: 'struct scatterlist' has no member named 'page'
include/asm/dma-mapping.h:288: error: 'struct scatterlist' has no member named 'page'
include/asm/dma-mapping.h:289: error: 'struct scatterlist' has no member named 'page'
include/asm/dma-mapping.h:290: error: 'struct scatterlist' has no member named 'page'
include/asm/dma-mapping.h: In function 'dma_sync_sg_for_cpu':
include/asm/dma-mapping.h:331: error: 'struct scatterlist' has no member named 'page'

drivers/scsi/ps3rom.c: In function 'fetch_to_dev_buffer':
drivers/scsi/ps3rom.c:150: error: 'struct scatterlist' has no member named 'page'



Signed-off-by: Olof Johansson <olof@lixom.net>

diff --git a/include/asm-powerpc/dma-mapping.h b/include/asm-powerpc/dma-mapping.h
index 65be95d..ff52013 100644
--- a/include/asm-powerpc/dma-mapping.h
+++ b/include/asm-powerpc/dma-mapping.h
@@ -285,9 +285,9 @@ dma_map_sg(struct device *dev, struct scatterlist *sgl, int nents,
 	BUG_ON(direction == DMA_NONE);
 
 	for_each_sg(sgl, sg, nents, i) {
-		BUG_ON(!sg->page);
-		__dma_sync_page(sg->page, sg->offset, sg->length, direction);
-		sg->dma_address = page_to_bus(sg->page) + sg->offset;
+		BUG_ON(!sg_page(sg));
+		__dma_sync_page(sg_page(sg), sg->offset, sg->length, direction);
+		sg->dma_address = page_to_bus(sg_page(sg)) + sg->offset;
 	}
 
 	return nents;
@@ -328,7 +328,7 @@ static inline void dma_sync_sg_for_cpu(struct device *dev,
 	BUG_ON(direction == DMA_NONE);
 
 	for_each_sg(sgl, sg, nents, i)
-		__dma_sync_page(sg->page, sg->offset, sg->length, direction);
+		__dma_sync_page(sg_page(sg), sg->offset, sg->length, direction);
 }
 
 static inline void dma_sync_sg_for_device(struct device *dev,
@@ -341,7 +341,7 @@ static inline void dma_sync_sg_for_device(struct device *dev,
 	BUG_ON(direction == DMA_NONE);
 
 	for_each_sg(sgl, sg, nents, i)
-		__dma_sync_page(sg->page, sg->offset, sg->length, direction);
+		__dma_sync_page(sg_page(sg), sg->offset, sg->length, direction);
 }
 
 static inline int dma_mapping_error(dma_addr_t dma_addr)
diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
index 03f19b8..17b4a7c 100644
--- a/drivers/scsi/ps3rom.c
+++ b/drivers/scsi/ps3rom.c
@@ -147,7 +147,7 @@ static int fetch_to_dev_buffer(struct scsi_cmnd *cmd, void *buf)
 
 	req_len = fin = 0;
 	scsi_for_each_sg(cmd, sgpnt, scsi_sg_count(cmd), k) {
-		kaddr = kmap_atomic(sg_page(sgpnt->page), KM_IRQ0);
+		kaddr = kmap_atomic(sg_page(sgpnt), KM_IRQ0);
 		len = sgpnt->length;
 		if ((req_len + len) > buflen) {
 			len = buflen - req_len;

^ permalink raw reply related


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