Hi, Alan.
I update the patch merge with 2.4.19-rc2-ac2 for fix minor things
with pdc202xx.c, patch-2.4.19rc1-ac series and patch-2.4.19rc2-ac
series has some different with pdc202xx.c, I don't know who rewrite
this code...Why pdc202xx_ratefilter() down UDMA mode with
tune_chipset function?
Now we are the maintainer of pdc202xx controllers. We know the
ASIC detail spec. So please let us modify and maintain.
We can provide best support for world wide Promise users.
Alan, please merge this patch in your series. thanks.
Marcelo, Maybe you can help us to merge with 2.4.19rc3?
Thanks in advance
--
Hank Yang
Promise Technology, Inc.
I test this patch, and have compile problem with one } on ide.c
Now i test on my computer.
giro
On Fri, 19 Jul 2002, Giro wrote:
>
> I test this patch, and have compile problem with one } on ide.c
Sorry error on line 918
and more errors:
drivers/ide/idedriver.o: In function `ide_error':
drivers/ide/idedriver.o(.text+0x3b37): undefined reference to
`pdc202xx_marvell_idle'
i use 2.4.18 + patch-2.4.19-rc2 + patch-2.4.19-rc2-ac2 +
promise-patch-2.4.19-rc2-ac2
i supous it is correct?
Giro
>
>
giro
>
>
>
>
> -
> 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/
>
*******************************************************************************
David Gironella Casademont
Estudiant Informatica de Sistemes - Universitat de Girona
Administrador d'Hades [email protected]
Secretari Associaci? d'Estudiants d'Inform?tica de Girona AEIGI
AEIGi Web - http://www.aeigi.org
Giro Web - http://hades.udg.es/~giro
*******************************************************************************
Greetings,
support <[email protected]> :
[...]
> Now we are the maintainer of pdc202xx controllers. We know the
> ASIC detail spec. So please let us modify and maintain.
> We can provide best support for world wide Promise users.
+#define set_2regs(a, b) \
+ OUT_BYTE((a + adj), indexreg); \
+ OUT_BYTE(b, datareg);
+
+#define set_reg_and_wait(value, reg, delay) \
+ OUT_BYTE(value, reg); \
+ mdelay(delay);
Hmmm...
[...]
+ if (speed == XFER_UDMA_2)
+ set_2regs(thold, (IN_BYTE(datareg) & 0x7f));
Oops
[...]
+ if (new_chip) {
+ if (id->capability & 4) /* IORDY_EN & PREFETCH_EN */
+ set_2regs(iordy, (IN_BYTE(datareg) | 0x03));
+ }
Oops
--
Ueimor
support <[email protected]> :
[...]
> We don't occur Oops you said, Please check your patch rule again.
There's a side effect with your macro. Let's expand it:
+#define set_2regs(a, b) \
+ OUT_BYTE((a + adj), indexreg); \
+ OUT_BYTE(b, datareg);
[...]
+ if (speed == XFER_UDMA_2)
+ set_2regs(thold, (IN_BYTE(datareg) & 0x7f));
if (speed == XFER_UDMA_2)
OUT_BYTE((thold + adj), indexreg);
OUT_BYTE((IN_BYTE(datareg) & 0x7f), datareg);
Do you see the problem ?
--
Ueimor
Giro,
Yes, It's 2.4.18 + patch-2.4.19-rc2 + patch-2.4.19-rc2-ac2 +
promise-patch-2.4.19-rc2-ac2.
Sorry, In ide.c (line-918) lack a '{' sign.c
> drivers/ide/idedriver.o(.text+0x3b37): undefined reference to
> `pdc202xx_marvell_idle'
Sorry, You must not enable CONFIG_BLK_DEV_PDC202XX
Or append follows will be okay.
ide.c (line-172)
#ifdef CONFIG_BLK_DEV_PDC202XX
extern int pdc202xx_marvell_idle (ide_drive_t *);/* needed below -- Promise
*/
#endif
ide.c (line-918)
if (GET_STAT() & (BUSY_STAT|DRQ_STAT)) {
#ifdef CONFIG_BLK_DEV_PDC202XX
/* Give a breath for Idle Immediate by Promise */
if (HWIF(drive)->pci_devid.vid == PCI_VENDOR_ID_PROMISE)
pdc202xx_marvell_idle(drive);
else
#endif
OUT_BYTE(WIN_IDLEIMMEDIATE, IDE_COMMAND_REG); /* force an abort */
}
Alan or Marcelo, Would you please help us to update above issue? Thanks.
Francois,
We don't occur Oops you said, Please check your patch rule again.
--
Promise Technology, Inc
We think there is no problems, Acturally it is
if (speed == XFER_UDMA_2) {
OUT_BYTE((thold + adj), indexreg);
OUT_BYTE((IN_BYTE(datareg) & 0x7f), datareg);
}
So,
if (speed == XFER_UDMA_2)
set_2regs(thold, (IN_BYTE(datareg) & 0x7f));
--
Promise Technology, Inc
support <[email protected]> :
> We think there is no problems, Acturally it is
>
> if (speed == XFER_UDMA_2) {
> OUT_BYTE((thold + adj), indexreg);
> OUT_BYTE((IN_BYTE(datareg) & 0x7f), datareg);
> }
>
> So,
> if (speed == XFER_UDMA_2)
> set_2regs(thold, (IN_BYTE(datareg) & 0x7f));
set_2regs() is a macro. Macro have side effects.
Before the change, assuming speed != XFER_UDMA_2
if (speed == XFER_UDMA_2) {
OUT_BYTE((thold + adj), indexreg); <- not executed
OUT_BYTE((IN_BYTE(datareg) & 0x7f), datareg); <- not executed
}
-> the block isn't executed
After the change, the code is:
if (speed == XFER_UDMA_2)
OUT_BYTE((thold + adj), indexreg); <- not executed
OUT_BYTE((IN_BYTE(datareg) & 0x7f), datareg); <- executed, damn it !
-> only the first statement of the macro is executed.
Put a pair of {} after the "if", a "do {...} while (0)" in the macro
declaration. Please, please, think about it a few minutes.
--
Ueimor
On Tue, 23 Jul 2002 09:19:15 +0200, Francois Romieu wrote:
>support <[email protected]> :
>> We think there is no problems, Acturally it is
>>
>> if (speed == XFER_UDMA_2) {
>> OUT_BYTE((thold + adj), indexreg);
>> OUT_BYTE((IN_BYTE(datareg) & 0x7f), datareg);
>> }
>>
>> So,
>> if (speed == XFER_UDMA_2)
>> set_2regs(thold, (IN_BYTE(datareg) & 0x7f));
The problem is a common one for complex statement-like macros.
You have a macro M consisting of (in this case) two statements
S1 and S2: "#define M S1; S2". Now consider what happens when
M is used in non-block context, i.e. not as a top-level
statement between { and } but rather in e.g. the true branch
of an if-statement:
if (condition)
M;
which after preprocessing becomes
if (condition)
S1; S2;
However, indentation doesn't matter, only grouping does, so this
USE of the macro really is
if (condition)
S1;
S2;
Now do you see? The macro body was broken up, and the second statement
is now executed unconditionally.
The traditional approach is to write the body of a complex macro as a
do { ... } while(0) statement (i.e. #define M do { S1; S2; } while(0))
since this turns the macro body into a single unbreakable statement
which is safe to use in any context where a statement may occur.
Simply wrapping the macro body with a pair of braces { } doesn't work
in all contexts; the do{...}while(0) idiom does.
/Mikael
On Tue, Jul 23, 2002 at 09:19:15AM +0200, Francois Romieu wrote:
> support <[email protected]> :
> > We think there is no problems,
>
> After the change, the code is:
>
> if (speed == XFER_UDMA_2)
> OUT_BYTE((thold + adj), indexreg); <- not executed
> OUT_BYTE((IN_BYTE(datareg) & 0x7f), datareg); <- executed, damn it !
I have one more question. Is it really immaterial on the line above in
which order 'datareg' occurences are used? Regardless of what
'OUT_BYTE' is now or may become in the future and how these macro are
used? It looks to me that we are trying to read some byte from
'datareg', clear a bit in it and write it back but looks can be
deceptive. It may turn out that this does or does not work on a
particular compiler whim.
Maybe it is ok now but beeing explicit in a macro definition does
not really cost anything and may save some future grief.
Michal