[PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerpc pl atform

[PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerpc pl atform

[Zang Roy-r61911]

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	<hong-jun.chen@reescale.com>
Signed-off-by: Roy Zang		<tie-fei.zang@freescale.com>

>From nobody Mon Sep 17 00:00:00 2001
From: roy zang <tie-fei.zang@freescale.com>
Date: Tue May 16 15:25:23 2006 +0800
Subject: [PATCH] Fix bugs on powerpc platform for mv sata driver

---

 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

Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerpc pl atform

[Kumar Gala]

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	<hong-jun.chen@reescale.com>
> Signed-off-by: Roy Zang		<tie-fei.zang@freescale.com>
>
>> From nobody Mon Sep 17 00:00:00 2001
> From: roy zang <tie-fei.zang@freescale.com>
> 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
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH/2.6.17-rc4 10/10] bugs fix for marvell SATA on powerpc pl atform

[Benjamin Herrenschmidt]

On Wed, 2006-05-17 at 18:14 +0800, 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.

As Kumar says -> Jeff is the maintainer. But I do have a couple of
comments still...

> @@ -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.