Re: [PATCH] 2.4.19-rc2-ac2 pdc202xx.c update

Re: [PATCH] 2.4.19-rc2-ac2 pdc202xx.c update

[support]

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

Re: [PATCH] 2.4.19-rc2-ac2 pdc202xx.c update

[Francois Romieu]

support <support@promise.com.tw> :
[...]
> 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

Re: [PATCH] 2.4.19-rc2-ac2 pdc202xx.c update

[support]

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

Re: [PATCH] 2.4.19-rc2-ac2 pdc202xx.c update

[Francois Romieu]

support <support@promise.com.tw> :
> 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

Re: [PATCH] 2.4.19-rc2-ac2 pdc202xx.c update

[Michal Jaegermann]

On Tue, Jul 23, 2002 at 09:19:15AM +0200, Francois Romieu wrote:
> support <support@promise.com.tw> :
> > 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

Re: [PATCH] 2.4.19-rc2-ac2 pdc202xx.c update

[Mikael Pettersson]

On Tue, 23 Jul 2002 09:19:15 +0200, Francois Romieu wrote:
>support <support@promise.com.tw> :
>> 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

[PATCH] 2.4.19-rc2-ac2 pdc202xx.c update

[support]

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

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.




[-- Attachment #2: promise-patch-2.4.19-rc2-ac2.gz --]
[-- Type: application/x-gzip, Size: 9697 bytes --]

Re: [PATCH] 2.4.19-rc2-ac2 pdc202xx.c update

[Giro]

I test this patch, and have compile problem with one } on ide.c

Now i test on my computer.

giro

Re: [PATCH] 2.4.19-rc2-ac2 pdc202xx.c update

[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 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/
>

*******************************************************************************
  David Gironella Casademont
  Estudiant Informatica de Sistemes - Universitat de Girona
  Administrador d'Hades root@hades.udg.es
  Secretari Associació d'Estudiants d'Informàtica de Girona AEIGI
  AEIGi Web - http://www.aeigi.org
  Giro Web - http://hades.udg.es/~giro
*******************************************************************************