lkml.org 
[lkml]   [2006]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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:galak@kernel.crashing.org]
>> Sent: 2006年5月17日 21:28
>> To: Zang Roy-r61911
>> Cc: Paul Mackerras; linuxppc-dev list; Alexandre.Bounine@tundra.com; 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



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-05-18 17:29    [W:0.062 / U:5.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site