2006-05-18 04:03:56

by Zang Roy-r61911

[permalink] [raw]
Subject: RE: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform


-----Original Message-----
From: Kumar Gala [mailto:[email protected]]
Sent: 2006年5月17日 21:28
To: Zang Roy-r61911
Cc: Paul Mackerras; linuxppc-dev list; [email protected]; Yang Xin-Xin-r48390
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerpc pl atform



On May 17, 2006, at 5:14 AM, Zang Roy-r61911 wrote:

> Fix Marvell SATA driver bugs on PowerPC platform:
> SATA device can't work for the problem on little-endian mode.
> U-Boot can't find SATA device after kernel reboots.
>
> Signed-off-by: Hongjun cheng <[email protected]>
> Signed-off-by: Roy Zang <[email protected]>
>
>> From nobody Mon Sep 17 00:00:00 2001
> From: roy zang <[email protected]>
> Date: Tue May 16 15:25:23 2006 +0800
> Subject: [PATCH] Fix bugs on powerpc platform for mv sata driver

This needs to go to Jeff Garzik as SATA driver maintainer.

- kumar

>
> drivers/scsi/sata_mv.c | 10 +++++++++-
> 1 files changed, 9 insertions(+), 1 deletions(-)
>
> d82ac19d259f8487a31105eaf844a93cbd9008e8
> diff --git a/drivers/scsi/sata_mv.c b/drivers/scsi/sata_mv.c
> index d5fdcb9..4166422 100644
> --- a/drivers/scsi/sata_mv.c
> +++ b/drivers/scsi/sata_mv.c
> @@ -1032,6 +1032,9 @@ static inline void mv_crqb_pack_cmd(u16
> {
> *cmdw = data | (addr << CRQB_CMD_ADDR_SHIFT) | CRQB_CMD_CS |
> (last ? CRQB_CMD_LAST : 0);
> +#ifdef CONFIG_PPC
> + *cmdw = cpu_to_le16(*cmdw);
> +#endif
> }
>
> /**
> @@ -1567,13 +1570,18 @@ static void mv5_read_preamp(struct mv_ho
> static void mv5_enable_leds(struct mv_host_priv *hpriv, void
> __iomem *mmio)
> {
> u32 tmp;
> -
> +#ifndef CONFIG_PPC
> writel(0, mmio + MV_GPIO_PORT_CTL);
> +#endif
>
> /* FIXME: handle MV_HP_ERRATA_50XXB2 errata */
>
> tmp = readl(mmio + MV_PCI_EXP_ROM_BAR_CTL);
> +#ifdef CONFIG_PPC
> + tmp &= ~(1 << 0);
> +#else
> tmp |= ~(1 << 0);
> +#endif
> writel(tmp, mmio + MV_PCI_EXP_ROM_BAR_CTL);
> }
>
> --
> 1.3.0
> _______________________________________________
> Linuxppc-dev mailing list
> [email protected]
> https://ozlabs.org/mailman/listinfo/linuxppc-dev


2006-05-18 07:02:34

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: RE: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

On Thu, 2006-05-18 at 12:03 +0800, Zang Roy-r61911 wrote:
> -----Original Message-----
> From: Kumar Gala [mailto:[email protected]]
> Sent: 2006年5月17日 21:28
> To: Zang Roy-r61911
> Cc: Paul Mackerras; linuxppc-dev list; [email protected]; Yang Xin-Xin-r48390
> Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerpc pl atform

Copying here the comments I already made so Jeff gets them...

> @@ -1032,6 +1032,9 @@ static inline void mv_crqb_pack_cmd(u16
> {
> *cmdw = data | (addr << CRQB_CMD_ADDR_SHIFT) | CRQB_CMD_CS |
> (last ? CRQB_CMD_LAST : 0);
> +#ifdef CONFIG_PPC
> + *cmdw = cpu_to_le16(*cmdw);
> +#endif
> }

Why an ifdef here ? The cpu_to_le16 should probably be unconditional.
And even if for some weird reason you wanted to ifdef it, why PPC ? and
what about other BE architectures ?

> /**
> @@ -1567,13 +1570,18 @@ static void mv5_read_preamp(struct mv_ho
> static void mv5_enable_leds(struct mv_host_priv *hpriv, void __iomem
*mmio)
> {
> u32 tmp;
> -
> +#ifndef CONFIG_PPC
> writel(0, mmio + MV_GPIO_PORT_CTL);
> +#endif

You'll have to do better here too... I don't wee why when compiled on
PPC, this driver should "magically" not clear those bits... At the very
least, you should test the machine type if you want to do something
specific to your platform, but first, you'll have to convince Jeff why
this change has to be done in the first place and if there is a better
way to handle it.

> /* FIXME: handle MV_HP_ERRATA_50XXB2 errata */
>
> tmp = readl(mmio + MV_PCI_EXP_ROM_BAR_CTL);
> +#ifdef CONFIG_PPC
> + tmp &= ~(1 << 0);
> +#else
> tmp |= ~(1 << 0);
> +#endif
> writel(tmp, mmio + MV_PCI_EXP_ROM_BAR_CTL);
> }

Looks to me like the initial code was bogus, thus the #ifdef shouldn't
be necessary neither, and even if it was, an ifdef CONFIG_PPC would be
the wrong approach for what I think should be ovious enough reasons...

Ben.


2006-05-18 11:01:55

by Ric Wheeler

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

Mark Lord has been actively working on the sata_mv driver recently,

ric


Zang Roy-r61911 wrote:

>-----Original Message-----
>From: Kumar Gala [mailto:[email protected]]
>Sent: 2006年5月17日 21:28
>To: Zang Roy-r61911
>Cc: Paul Mackerras; linuxppc-dev list; [email protected]; Yang Xin-Xin-r48390
>Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerpc pl atform
>
>
>
>On May 17, 2006, at 5:14 AM, Zang Roy-r61911 wrote:
>
>
>
>>Fix Marvell SATA driver bugs on PowerPC platform:
>>SATA device can't work for the problem on little-endian mode.
>>U-Boot can't find SATA device after kernel reboots.
>>
>>Signed-off-by: Hongjun cheng <[email protected]>
>>Signed-off-by: Roy Zang <[email protected]>
>>
>>
>>
>>>From nobody Mon Sep 17 00:00:00 2001
>>>
>>>
>>From: roy zang <[email protected]>
>>Date: Tue May 16 15:25:23 2006 +0800
>>Subject: [PATCH] Fix bugs on powerpc platform for mv sata driver
>>
>>
>
>This needs to go to Jeff Garzik as SATA driver maintainer.
>
>- kumar
>
>
>
>> drivers/scsi/sata_mv.c | 10 +++++++++-
>> 1 files changed, 9 insertions(+), 1 deletions(-)
>>
>>d82ac19d259f8487a31105eaf844a93cbd9008e8
>>diff --git a/drivers/scsi/sata_mv.c b/drivers/scsi/sata_mv.c
>>index d5fdcb9..4166422 100644
>>--- a/drivers/scsi/sata_mv.c
>>+++ b/drivers/scsi/sata_mv.c
>>@@ -1032,6 +1032,9 @@ static inline void mv_crqb_pack_cmd(u16
>> {
>> *cmdw = data | (addr << CRQB_CMD_ADDR_SHIFT) | CRQB_CMD_CS |
>> (last ? CRQB_CMD_LAST : 0);
>>+#ifdef CONFIG_PPC
>>+ *cmdw = cpu_to_le16(*cmdw);
>>+#endif
>> }
>>
>> /**
>>@@ -1567,13 +1570,18 @@ static void mv5_read_preamp(struct mv_ho
>> static void mv5_enable_leds(struct mv_host_priv *hpriv, void
>>__iomem *mmio)
>> {
>> u32 tmp;
>>-
>>+#ifndef CONFIG_PPC
>> writel(0, mmio + MV_GPIO_PORT_CTL);
>>+#endif
>>
>> /* FIXME: handle MV_HP_ERRATA_50XXB2 errata */
>>
>> tmp = readl(mmio + MV_PCI_EXP_ROM_BAR_CTL);
>>+#ifdef CONFIG_PPC
>>+ tmp &= ~(1 << 0);
>>+#else
>> tmp |= ~(1 << 0);
>>+#endif
>> writel(tmp, mmio + MV_PCI_EXP_ROM_BAR_CTL);
>> }
>>
>>--
>>1.3.0
>>_______________________________________________
>>Linuxppc-dev mailing list
>>[email protected]
>>https://ozlabs.org/mailman/listinfo/linuxppc-dev
>>
>>
>-
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/
>
>

2006-05-18 15:26:24

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

Benjamin Herrenschmidt wrote:
> On Thu, 2006-05-18 at 12:03 +0800, Zang Roy-r61911 wrote:
>> -----Original Message-----
>> From: Kumar Gala [mailto:[email protected]]
>> Sent: 2006年5月17日 21:28
>> To: Zang Roy-r61911
>> Cc: Paul Mackerras; linuxppc-dev list; [email protected]; Yang Xin-Xin-r48390
>> Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerpc pl atform
>
> Copying here the comments I already made so Jeff gets them...
>
>> @@ -1032,6 +1032,9 @@ static inline void mv_crqb_pack_cmd(u16
>> {
>> *cmdw = data | (addr << CRQB_CMD_ADDR_SHIFT) | CRQB_CMD_CS |
>> (last ? CRQB_CMD_LAST : 0);
>> +#ifdef CONFIG_PPC
>> + *cmdw = cpu_to_le16(*cmdw);
>> +#endif
>> }
>
> Why an ifdef here ? The cpu_to_le16 should probably be unconditional.
> And even if for some weird reason you wanted to ifdef it, why PPC ? and
> what about other BE architectures ?
>
>> /**
>> @@ -1567,13 +1570,18 @@ static void mv5_read_preamp(struct mv_ho
>> static void mv5_enable_leds(struct mv_host_priv *hpriv, void __iomem
> *mmio)
>> {
>> u32 tmp;
>> -
>> +#ifndef CONFIG_PPC
>> writel(0, mmio + MV_GPIO_PORT_CTL);
>> +#endif
>
> You'll have to do better here too... I don't wee why when compiled on
> PPC, this driver should "magically" not clear those bits... At the very
> least, you should test the machine type if you want to do something
> specific to your platform, but first, you'll have to convince Jeff why
> this change has to be done in the first place and if there is a better
> way to handle it.

Correct... it does seem some bugs were found, but #ifdef powerpc is
certainly out of the question. We want the driver to work without
ifdefs on all platforms.

Jeff



2006-05-18 20:50:50

by Mark Lord

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

Jeff Garzik wrote:
> Benjamin Herrenschmidt wrote:
>> On Thu, 2006-05-18 at 12:03 +0800, Zang Roy-r61911 wrote:
..
>>> @@ -1567,13 +1570,18 @@ static void mv5_read_preamp(struct mv_ho
>>> static void mv5_enable_leds(struct mv_host_priv *hpriv, void __iomem
>> *mmio)
>>> {
>>> u32 tmp;
>>> -
>>> +#ifndef CONFIG_PPC
>>> writel(0, mmio + MV_GPIO_PORT_CTL);
>>> +#endif
>>
>> You'll have to do better here too... I don't wee why when compiled on
>> PPC, this driver should "magically" not clear those bits... At the very
>> least, you should test the machine type if you want to do something
>> specific to your platform, but first, you'll have to convince Jeff why
>> this change has to be done in the first place and if there is a better
>> way to handle it.
>
> Correct... it does seem some bugs were found, but #ifdef powerpc is
> certainly out of the question. We want the driver to work without
> ifdefs on all platforms.

Yup. I have a powerpc platform here with PCI-X, and a PCI-X Marvell card
to try in it. So I'll pick up these changes and try to integrate them a
little more nicely in my internal updated driver, and then pass it on to Jeff.

Cheers

2006-05-18 21:07:42

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

Mark Lord wrote:
> Jeff Garzik wrote:
>> Benjamin Herrenschmidt wrote:
>>> On Thu, 2006-05-18 at 12:03 +0800, Zang Roy-r61911 wrote:
> ..
>>>> @@ -1567,13 +1570,18 @@ static void mv5_read_preamp(struct mv_ho
>>>> static void mv5_enable_leds(struct mv_host_priv *hpriv, void __iomem
>>> *mmio)
>>>> {
>>>> u32 tmp;
>>>> -
>>>> +#ifndef CONFIG_PPC
>>>> writel(0, mmio + MV_GPIO_PORT_CTL);
>>>> +#endif
>>>
>>> You'll have to do better here too... I don't wee why when compiled on
>>> PPC, this driver should "magically" not clear those bits... At the very
>>> least, you should test the machine type if you want to do something
>>> specific to your platform, but first, you'll have to convince Jeff why
>>> this change has to be done in the first place and if there is a better
>>> way to handle it.
>>
>> Correct... it does seem some bugs were found, but #ifdef powerpc is
>> certainly out of the question. We want the driver to work without
>> ifdefs on all platforms.
>
> Yup. I have a powerpc platform here with PCI-X, and a PCI-X Marvell card
> to try in it. So I'll pick up these changes and try to integrate them a
> little more nicely in my internal updated driver, and then pass it on to

When will this happen? I'm getting worried that useful stuff is
disappearing into the Mark Lord ether, with no changes appearing on the
other side. Haven't seen patches from you in weeks.

I'm happy that you have an updated internal driver, but that helps no
one else.

Jeff



2006-05-18 21:37:08

by Mark Lord

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

Jeff Garzik wrote:
>
> I'm happy that you have an updated internal driver, but that helps no
> one else.

Yikes! Mine own words, echoed back at me!

I'll organize some patches for you Friday.

Cheers

2006-05-26 08:44:35

by Sven Luther

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

On Thu, May 18, 2006 at 04:50:46PM -0400, Mark Lord wrote:
> Jeff Garzik wrote:
> > Benjamin Herrenschmidt wrote:
> >> On Thu, 2006-05-18 at 12:03 +0800, Zang Roy-r61911 wrote:
> ..
> >>> @@ -1567,13 +1570,18 @@ static void mv5_read_preamp(struct mv_ho
> >>> static void mv5_enable_leds(struct mv_host_priv *hpriv, void __iomem
> >> *mmio)
> >>> {
> >>> u32 tmp;
> >>> -
> >>> +#ifndef CONFIG_PPC
> >>> writel(0, mmio + MV_GPIO_PORT_CTL);
> >>> +#endif
> >>
> >> You'll have to do better here too... I don't wee why when compiled on
> >> PPC, this driver should "magically" not clear those bits... At the very
> >> least, you should test the machine type if you want to do something
> >> specific to your platform, but first, you'll have to convince Jeff why
> >> this change has to be done in the first place and if there is a better
> >> way to handle it.
> >
> > Correct... it does seem some bugs were found, but #ifdef powerpc is
> > certainly out of the question. We want the driver to work without
> > ifdefs on all platforms.
>
> Yup. I have a powerpc platform here with PCI-X, and a PCI-X Marvell card
> to try in it. So I'll pick up these changes and try to integrate them a
> little more nicely in my internal updated driver, and then pass it on to Jeff.

Hi, ...

I am trying to use a Marvell 88SX5081 based card here in my pegasos machine,
and i never got it working with the libata driver, even with the patches Zang
provided (and 2.6.16 though, maybe i should update to a newer version). The
marvell provided driver worked though at some time.

Would it be possible to have access to your work, in order to not duplicate
effort or something ?

Friendly,

Sven Luther

2006-05-26 08:56:13

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

Sven Luther wrote:
> On Thu, May 18, 2006 at 04:50:46PM -0400, Mark Lord wrote:
>> Jeff Garzik wrote:
>>> Benjamin Herrenschmidt wrote:
>>>> On Thu, 2006-05-18 at 12:03 +0800, Zang Roy-r61911 wrote:
>> ..
>>>>> @@ -1567,13 +1570,18 @@ static void mv5_read_preamp(struct mv_ho
>>>>> static void mv5_enable_leds(struct mv_host_priv *hpriv, void __iomem
>>>> *mmio)
>>>>> {
>>>>> u32 tmp;
>>>>> -
>>>>> +#ifndef CONFIG_PPC
>>>>> writel(0, mmio + MV_GPIO_PORT_CTL);
>>>>> +#endif
>>>> You'll have to do better here too... I don't wee why when compiled on
>>>> PPC, this driver should "magically" not clear those bits... At the very
>>>> least, you should test the machine type if you want to do something
>>>> specific to your platform, but first, you'll have to convince Jeff why
>>>> this change has to be done in the first place and if there is a better
>>>> way to handle it.
>>> Correct... it does seem some bugs were found, but #ifdef powerpc is
>>> certainly out of the question. We want the driver to work without
>>> ifdefs on all platforms.
>> Yup. I have a powerpc platform here with PCI-X, and a PCI-X Marvell card
>> to try in it. So I'll pick up these changes and try to integrate them a
>> little more nicely in my internal updated driver, and then pass it on to Jeff.
>
> Hi, ...
>
> I am trying to use a Marvell 88SX5081 based card here in my pegasos machine,
> and i never got it working with the libata driver, even with the patches Zang
> provided (and 2.6.16 though, maybe i should update to a newer version). The
> marvell provided driver worked though at some time.
>
> Would it be possible to have access to your work, in order to not duplicate
> effort or something ?

Do you have any debug output (see top of libata.h)?

Jeff



2006-05-26 09:04:05

by Sven Luther

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

On Fri, May 26, 2006 at 04:56:00AM -0400, Jeff Garzik wrote:
> Sven Luther wrote:
> >On Thu, May 18, 2006 at 04:50:46PM -0400, Mark Lord wrote:
> >>Jeff Garzik wrote:
> >>>Benjamin Herrenschmidt wrote:
> >>>>On Thu, 2006-05-18 at 12:03 +0800, Zang Roy-r61911 wrote:
> >>..
> >>>>>@@ -1567,13 +1570,18 @@ static void mv5_read_preamp(struct mv_ho
> >>>>> static void mv5_enable_leds(struct mv_host_priv *hpriv, void __iomem
> >>>>*mmio)
> >>>>> {
> >>>>> u32 tmp;
> >>>>>-
> >>>>>+#ifndef CONFIG_PPC
> >>>>> writel(0, mmio + MV_GPIO_PORT_CTL);
> >>>>>+#endif
> >>>>You'll have to do better here too... I don't wee why when compiled on
> >>>>PPC, this driver should "magically" not clear those bits... At the very
> >>>>least, you should test the machine type if you want to do something
> >>>>specific to your platform, but first, you'll have to convince Jeff why
> >>>>this change has to be done in the first place and if there is a better
> >>>>way to handle it.
> >>>Correct... it does seem some bugs were found, but #ifdef powerpc is
> >>>certainly out of the question. We want the driver to work without
> >>>ifdefs on all platforms.
> >>Yup. I have a powerpc platform here with PCI-X, and a PCI-X Marvell card
> >>to try in it. So I'll pick up these changes and try to integrate them a
> >>little more nicely in my internal updated driver, and then pass it on to
> >>Jeff.
> >
> >Hi, ...
> >
> >I am trying to use a Marvell 88SX5081 based card here in my pegasos
> >machine,
> >and i never got it working with the libata driver, even with the patches
> >Zang
> >provided (and 2.6.16 though, maybe i should update to a newer version). The
> >marvell provided driver worked though at some time.
> >
> >Would it be possible to have access to your work, in order to not duplicate
> >effort or something ?
>
> Do you have any debug output (see top of libata.h)?

Ah, not yet, but i can provide such.

Right now, i see the drive (with Zang's patch), but any attempt to access it
get me :

ata1: status=0x50 {DriveReady SeekComplete }
ata1: error=0x01 {AddrMarkNotFound }
sata_mv: PCI ERROR; PCI IRQ cause=0x00000400
sda: Current: sensekey: No Sense
Additional sense: No Additional sense information

The disk is a hitachi 80GB sata one.

I will compile a kernel with debug output and provide more info in a bit.

Friendly,

Sven Luther

2006-05-26 11:41:27

by Mark Lord

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

Sven Luther wrote:
> I'm trying to use a Marvell 88SX5081 based card here in my pegasos machine,
> and i never got it working with the libata driver, even with the patches Zang
> provided (and 2.6.16 though, maybe i should update to a newer version). The
> marvell provided driver worked though at some time.
>
> Would it be possible to have access to your work, in order to not duplicate
> effort or something ?

All of the relevant bits of "my work" are now in Jeff's upstream libata tree,
from the recently posted sata_mv patches.

After struggling with bad SDRAM earlier this week, I now have Ubuntu installed
and running reliably on my G3 box here. Next step is to upgrade the kernel
to something modern, and add in the latest sata_mv.

After that, I'll try my own 6081 Marvell card in it, and hopefully see the
same failures as you.. hopefully!

Cheers

2006-05-26 11:47:58

by Sven Luther

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

On Fri, May 26, 2006 at 07:41:24AM -0400, Mark Lord wrote:
> Sven Luther wrote:
> >I'm trying to use a Marvell 88SX5081 based card here in my pegasos machine,
> >and i never got it working with the libata driver, even with the patches
> >Zang
> >provided (and 2.6.16 though, maybe i should update to a newer version). The
> >marvell provided driver worked though at some time.
> >
> >Would it be possible to have access to your work, in order to not duplicate
> >effort or something ?
>
> All of the relevant bits of "my work" are now in Jeff's upstream libata
> tree,
> from the recently posted sata_mv patches.

Ok. can i use this tree with a 2.6.16 base ?

> After struggling with bad SDRAM earlier this week, I now have Ubuntu
> installed
> and running reliably on my G3 box here. Next step is to upgrade the kernel
> to something modern, and add in the latest sata_mv.

Ok.

> After that, I'll try my own 6081 Marvell card in it, and hopefully see the
> same failures as you.. hopefully!

I tried getting a 6081 based low profile card, but they don't seem to sell in
europe.

Friendly,

Sven Luther

2006-05-26 13:19:38

by Mark Lord

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

Sven Luther wrote:
> On Fri, May 26, 2006 at 07:41:24AM -0400, Mark Lord wrote:
>> All of the relevant bits of "my work" are now in Jeff's upstream libata
>> tree,
>> from the recently posted sata_mv patches.
>
> Ok. can i use this tree with a 2.6.16 base ?

Not as-is. Here (attached) is a patch for 2.6.16.17+ that updates
the sata_mv driver to the latest source. Completely untested,
but it does compile.

I will hopefully test it later today, but in the meanwhile, have a go at it.

Cheers


Attachments:
sata_mv-2.6.16_backport.patch (22.14 kB)

2006-05-26 14:20:51

by Sven Luther

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

On Fri, May 26, 2006 at 09:19:33AM -0400, Mark Lord wrote:
> Sven Luther wrote:
> >On Fri, May 26, 2006 at 07:41:24AM -0400, Mark Lord wrote:
> >>All of the relevant bits of "my work" are now in Jeff's upstream libata
> >>tree,
> >>from the recently posted sata_mv patches.
> >
> >Ok. can i use this tree with a 2.6.16 base ?
>
> Not as-is. Here (attached) is a patch for 2.6.16.17+ that updates
> the sata_mv driver to the latest source. Completely untested,
> but it does compile.
>
> I will hopefully test it later today, but in the meanwhile, have a go at it.

And here is attached my dmesg output. The last bit of mv_host_intr was when i
tried to access the partition table of the disk with parted.

Friendly,

Sven Luther


Attachments:
(No filename) (733.00 B)
dmesg-2.6.16-mv_sata.gz (3.91 kB)
Download all attachments

2006-05-26 15:47:14

by Mark Lord

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

Sven Luther wrote:
> On Fri, May 26, 2006 at 09:19:33AM -0400, Mark Lord wrote:
>> Sven Luther wrote:
>>> Ok. can i use this tree with a 2.6.16 base ?
>> Not as-is. Here (attached) is a patch for 2.6.16.17+ that updates
>> the sata_mv driver to the latest source. Completely untested,
>> but it does compile.
>>
>> I will hopefully test it later today, but in the meanwhile, have a go at it.
>
> And here is attached my dmesg output. The last bit of mv_host_intr was when i
> tried to access the partition table of the disk with parted.

I don't see anything particularly bad in that dmesg output,
apart from all of the debug output --> did you enable that,
or was it "on" by default?

It finds one SATA drive, with no *known* partition table format.

Can you access the disk? Eg. hexdump -C /dev/sda

Meanwhile, I just booted 2.6.17-rc5-git1 (latest kernel.org) on my Mac G3
box here, and sata_mv seems to be behaving for me (thus far).

Cheers

2006-05-26 16:07:12

by Sven Luther

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

On Fri, May 26, 2006 at 11:47:11AM -0400, Mark Lord wrote:
> Sven Luther wrote:
> >On Fri, May 26, 2006 at 09:19:33AM -0400, Mark Lord wrote:
> >>Sven Luther wrote:
> >>>Ok. can i use this tree with a 2.6.16 base ?
> >>Not as-is. Here (attached) is a patch for 2.6.16.17+ that updates
> >>the sata_mv driver to the latest source. Completely untested,
> >>but it does compile.
> >>
> >>I will hopefully test it later today, but in the meanwhile, have a go at
> >>it.
> >
> >And here is attached my dmesg output. The last bit of mv_host_intr was
> >when i
> >tried to access the partition table of the disk with parted.
>
> I don't see anything particularly bad in that dmesg output,
> apart from all of the debug output --> did you enable that,
> or was it "on" by default?

Ah, yes, i enabled it on Jeff's suggestion.

> It finds one SATA drive, with no *known* partition table format.

Well, it is a blank disk indeed, so this is no suprise.

> Can you access the disk? Eg. hexdump -C /dev/sda

Trying to partition the disk with parted yielded the last set of debug
messages, and a :

Error: Unable to open /dev/sda - unrecognized disk label.

The same when trying to write a partition table to it, and naturally, there is
no partition afterward.

hexdump yielded only the debugmessages and nothing else.

> Meanwhile, I just booted 2.6.17-rc5-git1 (latest kernel.org) on my Mac G3
> box here, and sata_mv seems to be behaving for me (thus far).

Mmm, this is a G3, while i have a G4. The G4 does some I/O reordering, which
we don't have with a G3, so this may be the cause.

Do you have a mac version of the board, with a forth/OF bios in it ? Or a
normal PC card, which is thus uninitialized ?

Friendly,

Sven Luther

2006-05-26 16:20:32

by Sven Luther

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

On Fri, May 26, 2006 at 06:01:56PM +0200, Sven Luther wrote:
> > Can you access the disk? Eg. hexdump -C /dev/sda
>
> Trying to partition the disk with parted yielded the last set of debug
> messages, and a :

Interesting. Using dd, i am able to write some stuff to disk :

dd if=/dev/sda of=/tmp/blah count=1 -> hexdump /tmp/blah yields only 0s.
dd if=/dev/random of=/dev/sda count=4 (count = 4, because /dev/random outputs blocks of 128 bytes)
dd if=/dev/sda of=/tmp/blah count=1 -> hexdump /tmp/blah yield random non-0 content

Trying to write past the first block, seems to freeze the disk or someting, at
least dd doesn't come back without me ctr-c'ing it.

Friendly,

Sven Luther


2006-05-26 16:21:39

by Mark Lord

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

Sven Luther wrote:
> On Fri, May 26, 2006 at 11:47:11AM -0400, Mark Lord wrote:
>
>> Meanwhile, I just booted 2.6.17-rc5-git1 (latest kernel.org) on my Mac G3
>> box here, and sata_mv seems to be behaving for me (thus far).
>
> Mmm, this is a G3, while i have a G4. The G4 does some I/O reordering, which
> we don't have with a G3, so this may be the cause.

MMmm.. is your G4 using a 64-bit kernel, or a 32-bit kernel?
The driver is a complete unknown (for me, at least) on 64-bit systems,
as I don't have one (of any processor type) here yet.

> Do you have a mac version of the board, with a forth/OF bios in it ? Or a
> normal PC card, which is thus uninitialized ?

The board claims to be "mac" compatible, so it probably has an OF bios on it,
but I have not verified this yet.

Cheers

2006-05-26 16:31:21

by Sven Luther

[permalink] [raw]
Subject: Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerp c pl atform

On Fri, May 26, 2006 at 12:21:36PM -0400, Mark Lord wrote:
> Sven Luther wrote:
> >On Fri, May 26, 2006 at 11:47:11AM -0400, Mark Lord wrote:
> >
> >>Meanwhile, I just booted 2.6.17-rc5-git1 (latest kernel.org) on my Mac G3
> >>box here, and sata_mv seems to be behaving for me (thus far).
> >
> >Mmm, this is a G3, while i have a G4. The G4 does some I/O reordering,
> >which
> >we don't have with a G3, so this may be the cause.
>
> MMmm.. is your G4 using a 64-bit kernel, or a 32-bit kernel?

Ah, no, the G4 is a 32bit processor.

> The driver is a complete unknown (for me, at least) on 64-bit systems,
> as I don't have one (of any processor type) here yet.

I have an Xserve G5, which is a 64bit powerpc machine, and i could try it in
it. I need to get the fans to a usable level first though, as it sounds like
an airplan, and is sitting right beside my desk.

> >Do you have a mac version of the board, with a forth/OF bios in it ? Or a
> >normal PC card, which is thus uninitialized ?
>
> The board claims to be "mac" compatible, so it probably has an OF bios on
> it, but I have not verified this yet.

Yes, probably. This could mean that the board gets initialized for you, but
not for me.

Friendly,

Sven Luther