* [patch] MIN/MAX in ide-timing.h
@ 2004-07-08 18:18 Clemens Buchacher
2004-07-10 13:55 ` [Kernel-janitors] " Luiz Fernando N. Capitulino
2004-07-10 14:03 ` Vojtech Pavlik
0 siblings, 2 replies; 7+ messages in thread
From: Clemens Buchacher @ 2004-07-08 18:18 UTC (permalink / raw)
To: vojtech; +Cc: Linux-ide list, kernel janitors list
[-- Attachment #1: Type: text/plain, Size: 378 bytes --]
Hi,
I replaced the custom MIN/MAX macros with the type safe min/max macros
from linux/kernel.h.
The typecasts of the FIT macro parameters in amd74xx.c are a bit
asymmetric because min/max don't seem to return a value of the same type
as the parameters they are passed.
I do not have the hardware to test this. The patch is based on the
current bk kernel repository.
Clemens
[-- Attachment #2: minmax.stat --]
[-- Type: text/plain, Size: 216 bytes --]
drivers/ide/ide-timing.h | 25 ++++++++++++-------------
drivers/ide/pci/amd74xx.c | 14 +++++++-------
drivers/ide/pci/via82cxxx.c | 14 +++++++-------
3 files changed, 26 insertions(+), 27 deletions(-)
[-- Attachment #3: minmax.patch --]
[-- Type: text/plain, Size: 6631 bytes --]
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2004/07/04 03:24:26+02:00 drizzd@aon
# replaced custom MIN/MAX macros with min/max from kernel.h
#
# drivers/ide/pci/via82cxxx.c
# 2004/07/04 03:23:22+02:00 drizzd@aon +7 -7
# added typecasts to satisfy min/max
#
# drivers/ide/pci/amd74xx.c
# 2004/07/04 03:23:22+02:00 drizzd@aon +7 -7
# added typecasts to satisfy min/max
#
# drivers/ide/ide-timing.h
# 2004/07/04 03:23:22+02:00 drizzd@aon +12 -11
# using min/max from kernel.h now
#
# drivers/ide/ide-timing.h
# 2004/07/03 17:18:20+02:00 drizzd@aon +0 -2
# removed unused macros
#
diff -Nru a/drivers/ide/ide-timing.h b/drivers/ide/ide-timing.h
--- a/drivers/ide/ide-timing.h 2004-07-04 12:23:14 +02:00
+++ b/drivers/ide/ide-timing.h 2004-07-04 12:23:14 +02:00
@@ -27,6 +27,7 @@
* Vojtech Pavlik, Simunkova 1594, Prague 8, 182 00 Czech Republic
*/
+#include <linux/kernel.h>
#include <linux/hdreg.h>
#define XFER_PIO_5 0x0d
@@ -96,11 +97,9 @@
#define IDE_TIMING_UDMA 0x80
#define IDE_TIMING_ALL 0xff
-#define MIN(a,b) ((a)<(b)?(a):(b))
-#define MAX(a,b) ((a)>(b)?(a):(b))
-#define FIT(v,min,max) MAX(MIN(v,max),min)
-#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
-#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
+#define FIT(v,vmin,vmax) max(min(v,vmax),vmin)
+#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
+#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
#define XFER_MODE 0xf0
#define XFER_UDMA_133 0x48
@@ -188,14 +187,14 @@
static void ide_timing_merge(struct ide_timing *a, struct ide_timing *b, struct ide_timing *m, unsigned int what)
{
- if (what & IDE_TIMING_SETUP ) m->setup = MAX(a->setup, b->setup);
- if (what & IDE_TIMING_ACT8B ) m->act8b = MAX(a->act8b, b->act8b);
- if (what & IDE_TIMING_REC8B ) m->rec8b = MAX(a->rec8b, b->rec8b);
- if (what & IDE_TIMING_CYC8B ) m->cyc8b = MAX(a->cyc8b, b->cyc8b);
- if (what & IDE_TIMING_ACTIVE ) m->active = MAX(a->active, b->active);
- if (what & IDE_TIMING_RECOVER) m->recover = MAX(a->recover, b->recover);
- if (what & IDE_TIMING_CYCLE ) m->cycle = MAX(a->cycle, b->cycle);
- if (what & IDE_TIMING_UDMA ) m->udma = MAX(a->udma, b->udma);
+ if (what & IDE_TIMING_SETUP ) m->setup = max(a->setup, b->setup);
+ if (what & IDE_TIMING_ACT8B ) m->act8b = max(a->act8b, b->act8b);
+ if (what & IDE_TIMING_REC8B ) m->rec8b = max(a->rec8b, b->rec8b);
+ if (what & IDE_TIMING_CYC8B ) m->cyc8b = max(a->cyc8b, b->cyc8b);
+ if (what & IDE_TIMING_ACTIVE ) m->active = max(a->active, b->active);
+ if (what & IDE_TIMING_RECOVER) m->recover = max(a->recover, b->recover);
+ if (what & IDE_TIMING_CYCLE ) m->cycle = max(a->cycle, b->cycle);
+ if (what & IDE_TIMING_UDMA ) m->udma = max(a->udma, b->udma);
}
static struct ide_timing* ide_timing_find_mode(short speed)
diff -Nru a/drivers/ide/pci/amd74xx.c b/drivers/ide/pci/amd74xx.c
--- a/drivers/ide/pci/amd74xx.c 2004-07-04 12:23:14 +02:00
+++ b/drivers/ide/pci/amd74xx.c 2004-07-04 12:23:14 +02:00
@@ -205,20 +205,20 @@
unsigned char t;
pci_read_config_byte(dev, AMD_ADDRESS_SETUP, &t);
- t = (t & ~(3 << ((3 - dn) << 1))) | ((FIT(timing->setup, 1, 4) - 1) << ((3 - dn) << 1));
+ t = (t & ~(3 << ((3 - dn) << 1))) | ((FIT(timing->setup, 1, (short)4) - 1) << ((3 - dn) << 1));
pci_write_config_byte(dev, AMD_ADDRESS_SETUP, t);
pci_write_config_byte(dev, AMD_8BIT_TIMING + (1 - (dn >> 1)),
- ((FIT(timing->act8b, 1, 16) - 1) << 4) | (FIT(timing->rec8b, 1, 16) - 1));
+ ((FIT(timing->act8b, 1, (short)16) - 1) << 4) | (FIT(timing->rec8b, 1, (short)16) - 1));
pci_write_config_byte(dev, AMD_DRIVE_TIMING + (3 - dn),
- ((FIT(timing->active, 1, 16) - 1) << 4) | (FIT(timing->recover, 1, 16) - 1));
+ ((FIT(timing->active, 1, (short)16) - 1) << 4) | (FIT(timing->recover, 1, (short)16) - 1));
switch (amd_config->flags & AMD_UDMA) {
- case AMD_UDMA_33: t = timing->udma ? (0xc0 | (FIT(timing->udma, 2, 5) - 2)) : 0x03; break;
- case AMD_UDMA_66: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 2, 10)]) : 0x03; break;
- case AMD_UDMA_100: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 1, 10)]) : 0x03; break;
- case AMD_UDMA_133: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 1, 15)]) : 0x03; break;
+ case AMD_UDMA_33: t = timing->udma ? (0xc0 | (FIT(timing->udma, 2, (short)5) - 2)) : 0x03; break;
+ case AMD_UDMA_66: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 2, (short)10)]) : 0x03; break;
+ case AMD_UDMA_100: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 1, (short)10)]) : 0x03; break;
+ case AMD_UDMA_133: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 1, (short)15)]) : 0x03; break;
default: return;
}
diff -Nru a/drivers/ide/pci/via82cxxx.c b/drivers/ide/pci/via82cxxx.c
--- a/drivers/ide/pci/via82cxxx.c 2004-07-04 12:23:14 +02:00
+++ b/drivers/ide/pci/via82cxxx.c 2004-07-04 12:23:14 +02:00
@@ -291,21 +291,21 @@
if (~via_config->flags & VIA_BAD_AST) {
pci_read_config_byte(dev, VIA_ADDRESS_SETUP, &t);
- t = (t & ~(3 << ((3 - dn) << 1))) | ((FIT(timing->setup, 1, 4) - 1) << ((3 - dn) << 1));
+ t = (t & ~(3 << ((3 - dn) << 1))) | ((FIT(timing->setup, 1, (short)4) - 1) << ((3 - dn) << 1));
pci_write_config_byte(dev, VIA_ADDRESS_SETUP, t);
}
pci_write_config_byte(dev, VIA_8BIT_TIMING + (1 - (dn >> 1)),
- ((FIT(timing->act8b, 1, 16) - 1) << 4) | (FIT(timing->rec8b, 1, 16) - 1));
+ ((FIT(timing->act8b, 1, (short)16) - 1) << 4) | (FIT(timing->rec8b, 1, (short)16) - 1));
pci_write_config_byte(dev, VIA_DRIVE_TIMING + (3 - dn),
- ((FIT(timing->active, 1, 16) - 1) << 4) | (FIT(timing->recover, 1, 16) - 1));
+ ((FIT(timing->active, 1, (short)16) - 1) << 4) | (FIT(timing->recover, 1, (short)16) - 1));
switch (via_config->flags & VIA_UDMA) {
- case VIA_UDMA_33: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, 5) - 2)) : 0x03; break;
- case VIA_UDMA_66: t = timing->udma ? (0xe8 | (FIT(timing->udma, 2, 9) - 2)) : 0x0f; break;
- case VIA_UDMA_100: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, 9) - 2)) : 0x07; break;
- case VIA_UDMA_133: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, 9) - 2)) : 0x07; break;
+ case VIA_UDMA_33: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, (short)5) - 2)) : 0x03; break;
+ case VIA_UDMA_66: t = timing->udma ? (0xe8 | (FIT(timing->udma, 2, (short)9) - 2)) : 0x0f; break;
+ case VIA_UDMA_100: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, (short)9) - 2)) : 0x07; break;
+ case VIA_UDMA_133: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, (short)9) - 2)) : 0x07; break;
default: return;
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Kernel-janitors] [patch] MIN/MAX in ide-timing.h
2004-07-08 18:18 [patch] MIN/MAX in ide-timing.h Clemens Buchacher
@ 2004-07-10 13:55 ` Luiz Fernando N. Capitulino
2004-07-11 9:12 ` Clemens Buchacher
2004-07-10 14:03 ` Vojtech Pavlik
1 sibling, 1 reply; 7+ messages in thread
From: Luiz Fernando N. Capitulino @ 2004-07-10 13:55 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: vojtech, linux-ide, kernel-janitors, lcapitulino
Hello Clemens,
The patch seems right to me, but I have some comments:
Em Thu, 8 Jul 2004 20:18:43 +0200
Clemens Buchacher <drizzd@aon.at> screveu:
| # This is a BitKeeper generated diff -Nru style patch.
[...]
| -#define MIN(a,b) ((a)<(b)?(a):(b))
| -#define MAX(a,b) ((a)>(b)?(a):(b))
| -#define FIT(v,min,max) MAX(MIN(v,max),min)
| -#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
| -#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
| +#define FIT(v,vmin,vmax) max(min(v,vmax),vmin)
| +#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
| +#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
Did you changed something in ENOUGH and EZ ?
Maybe we can try to make it inlined functions, thus maybe is possible to
solve the warning problem.
PS: Try to send your patches in the body of the e-mail.
--
Luiz Fernando
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] MIN/MAX in ide-timing.h
2004-07-08 18:18 [patch] MIN/MAX in ide-timing.h Clemens Buchacher
2004-07-10 13:55 ` [Kernel-janitors] " Luiz Fernando N. Capitulino
@ 2004-07-10 14:03 ` Vojtech Pavlik
2004-07-10 22:43 ` Clemens Buchacher
1 sibling, 1 reply; 7+ messages in thread
From: Vojtech Pavlik @ 2004-07-10 14:03 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: Linux-ide list, kernel janitors list
On Thu, Jul 08, 2004 at 08:18:43PM +0200, Clemens Buchacher wrote:
> Hi,
>
> I replaced the custom MIN/MAX macros with the type safe min/max macros
> from linux/kernel.h.
>
> The typecasts of the FIT macro parameters in amd74xx.c are a bit
> asymmetric because min/max don't seem to return a value of the same type
> as the parameters they are passed.
How about if you used the "min_t" and "max_t" macros inside FIT? That
could help get rid of the casts completely (assuming the second and
third parameters to FIT are expected to be constants).
>
> I do not have the hardware to test this. The patch is based on the
> current bk kernel repository.
>
> Clemens
> drivers/ide/ide-timing.h | 25 ++++++++++++-------------
> drivers/ide/pci/amd74xx.c | 14 +++++++-------
> drivers/ide/pci/via82cxxx.c | 14 +++++++-------
> 3 files changed, 26 insertions(+), 27 deletions(-)
> # This is a BitKeeper generated diff -Nru style patch.
> #
> # ChangeSet
> # 2004/07/04 03:24:26+02:00 drizzd@aon
> # replaced custom MIN/MAX macros with min/max from kernel.h
> #
> # drivers/ide/pci/via82cxxx.c
> # 2004/07/04 03:23:22+02:00 drizzd@aon +7 -7
> # added typecasts to satisfy min/max
> #
> # drivers/ide/pci/amd74xx.c
> # 2004/07/04 03:23:22+02:00 drizzd@aon +7 -7
> # added typecasts to satisfy min/max
> #
> # drivers/ide/ide-timing.h
> # 2004/07/04 03:23:22+02:00 drizzd@aon +12 -11
> # using min/max from kernel.h now
> #
> # drivers/ide/ide-timing.h
> # 2004/07/03 17:18:20+02:00 drizzd@aon +0 -2
> # removed unused macros
> #
> diff -Nru a/drivers/ide/ide-timing.h b/drivers/ide/ide-timing.h
> --- a/drivers/ide/ide-timing.h 2004-07-04 12:23:14 +02:00
> +++ b/drivers/ide/ide-timing.h 2004-07-04 12:23:14 +02:00
> @@ -27,6 +27,7 @@
> * Vojtech Pavlik, Simunkova 1594, Prague 8, 182 00 Czech Republic
> */
>
> +#include <linux/kernel.h>
> #include <linux/hdreg.h>
>
> #define XFER_PIO_5 0x0d
> @@ -96,11 +97,9 @@
> #define IDE_TIMING_UDMA 0x80
> #define IDE_TIMING_ALL 0xff
>
> -#define MIN(a,b) ((a)<(b)?(a):(b))
> -#define MAX(a,b) ((a)>(b)?(a):(b))
> -#define FIT(v,min,max) MAX(MIN(v,max),min)
> -#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
> -#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
> +#define FIT(v,vmin,vmax) max(min(v,vmax),vmin)
> +#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
> +#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
>
> #define XFER_MODE 0xf0
> #define XFER_UDMA_133 0x48
> @@ -188,14 +187,14 @@
>
> static void ide_timing_merge(struct ide_timing *a, struct ide_timing *b, struct ide_timing *m, unsigned int what)
> {
> - if (what & IDE_TIMING_SETUP ) m->setup = MAX(a->setup, b->setup);
> - if (what & IDE_TIMING_ACT8B ) m->act8b = MAX(a->act8b, b->act8b);
> - if (what & IDE_TIMING_REC8B ) m->rec8b = MAX(a->rec8b, b->rec8b);
> - if (what & IDE_TIMING_CYC8B ) m->cyc8b = MAX(a->cyc8b, b->cyc8b);
> - if (what & IDE_TIMING_ACTIVE ) m->active = MAX(a->active, b->active);
> - if (what & IDE_TIMING_RECOVER) m->recover = MAX(a->recover, b->recover);
> - if (what & IDE_TIMING_CYCLE ) m->cycle = MAX(a->cycle, b->cycle);
> - if (what & IDE_TIMING_UDMA ) m->udma = MAX(a->udma, b->udma);
> + if (what & IDE_TIMING_SETUP ) m->setup = max(a->setup, b->setup);
> + if (what & IDE_TIMING_ACT8B ) m->act8b = max(a->act8b, b->act8b);
> + if (what & IDE_TIMING_REC8B ) m->rec8b = max(a->rec8b, b->rec8b);
> + if (what & IDE_TIMING_CYC8B ) m->cyc8b = max(a->cyc8b, b->cyc8b);
> + if (what & IDE_TIMING_ACTIVE ) m->active = max(a->active, b->active);
> + if (what & IDE_TIMING_RECOVER) m->recover = max(a->recover, b->recover);
> + if (what & IDE_TIMING_CYCLE ) m->cycle = max(a->cycle, b->cycle);
> + if (what & IDE_TIMING_UDMA ) m->udma = max(a->udma, b->udma);
> }
>
> static struct ide_timing* ide_timing_find_mode(short speed)
> diff -Nru a/drivers/ide/pci/amd74xx.c b/drivers/ide/pci/amd74xx.c
> --- a/drivers/ide/pci/amd74xx.c 2004-07-04 12:23:14 +02:00
> +++ b/drivers/ide/pci/amd74xx.c 2004-07-04 12:23:14 +02:00
> @@ -205,20 +205,20 @@
> unsigned char t;
>
> pci_read_config_byte(dev, AMD_ADDRESS_SETUP, &t);
> - t = (t & ~(3 << ((3 - dn) << 1))) | ((FIT(timing->setup, 1, 4) - 1) << ((3 - dn) << 1));
> + t = (t & ~(3 << ((3 - dn) << 1))) | ((FIT(timing->setup, 1, (short)4) - 1) << ((3 - dn) << 1));
> pci_write_config_byte(dev, AMD_ADDRESS_SETUP, t);
>
> pci_write_config_byte(dev, AMD_8BIT_TIMING + (1 - (dn >> 1)),
> - ((FIT(timing->act8b, 1, 16) - 1) << 4) | (FIT(timing->rec8b, 1, 16) - 1));
> + ((FIT(timing->act8b, 1, (short)16) - 1) << 4) | (FIT(timing->rec8b, 1, (short)16) - 1));
>
> pci_write_config_byte(dev, AMD_DRIVE_TIMING + (3 - dn),
> - ((FIT(timing->active, 1, 16) - 1) << 4) | (FIT(timing->recover, 1, 16) - 1));
> + ((FIT(timing->active, 1, (short)16) - 1) << 4) | (FIT(timing->recover, 1, (short)16) - 1));
>
> switch (amd_config->flags & AMD_UDMA) {
> - case AMD_UDMA_33: t = timing->udma ? (0xc0 | (FIT(timing->udma, 2, 5) - 2)) : 0x03; break;
> - case AMD_UDMA_66: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 2, 10)]) : 0x03; break;
> - case AMD_UDMA_100: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 1, 10)]) : 0x03; break;
> - case AMD_UDMA_133: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 1, 15)]) : 0x03; break;
> + case AMD_UDMA_33: t = timing->udma ? (0xc0 | (FIT(timing->udma, 2, (short)5) - 2)) : 0x03; break;
> + case AMD_UDMA_66: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 2, (short)10)]) : 0x03; break;
> + case AMD_UDMA_100: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 1, (short)10)]) : 0x03; break;
> + case AMD_UDMA_133: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 1, (short)15)]) : 0x03; break;
> default: return;
> }
>
> diff -Nru a/drivers/ide/pci/via82cxxx.c b/drivers/ide/pci/via82cxxx.c
> --- a/drivers/ide/pci/via82cxxx.c 2004-07-04 12:23:14 +02:00
> +++ b/drivers/ide/pci/via82cxxx.c 2004-07-04 12:23:14 +02:00
> @@ -291,21 +291,21 @@
>
> if (~via_config->flags & VIA_BAD_AST) {
> pci_read_config_byte(dev, VIA_ADDRESS_SETUP, &t);
> - t = (t & ~(3 << ((3 - dn) << 1))) | ((FIT(timing->setup, 1, 4) - 1) << ((3 - dn) << 1));
> + t = (t & ~(3 << ((3 - dn) << 1))) | ((FIT(timing->setup, 1, (short)4) - 1) << ((3 - dn) << 1));
> pci_write_config_byte(dev, VIA_ADDRESS_SETUP, t);
> }
>
> pci_write_config_byte(dev, VIA_8BIT_TIMING + (1 - (dn >> 1)),
> - ((FIT(timing->act8b, 1, 16) - 1) << 4) | (FIT(timing->rec8b, 1, 16) - 1));
> + ((FIT(timing->act8b, 1, (short)16) - 1) << 4) | (FIT(timing->rec8b, 1, (short)16) - 1));
>
> pci_write_config_byte(dev, VIA_DRIVE_TIMING + (3 - dn),
> - ((FIT(timing->active, 1, 16) - 1) << 4) | (FIT(timing->recover, 1, 16) - 1));
> + ((FIT(timing->active, 1, (short)16) - 1) << 4) | (FIT(timing->recover, 1, (short)16) - 1));
>
> switch (via_config->flags & VIA_UDMA) {
> - case VIA_UDMA_33: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, 5) - 2)) : 0x03; break;
> - case VIA_UDMA_66: t = timing->udma ? (0xe8 | (FIT(timing->udma, 2, 9) - 2)) : 0x0f; break;
> - case VIA_UDMA_100: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, 9) - 2)) : 0x07; break;
> - case VIA_UDMA_133: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, 9) - 2)) : 0x07; break;
> + case VIA_UDMA_33: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, (short)5) - 2)) : 0x03; break;
> + case VIA_UDMA_66: t = timing->udma ? (0xe8 | (FIT(timing->udma, 2, (short)9) - 2)) : 0x0f; break;
> + case VIA_UDMA_100: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, (short)9) - 2)) : 0x07; break;
> + case VIA_UDMA_133: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, (short)9) - 2)) : 0x07; break;
> default: return;
> }
>
--
Vojtech Pavlik
SuSE Labs, SuSE CR
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] MIN/MAX in ide-timing.h
2004-07-10 14:03 ` Vojtech Pavlik
@ 2004-07-10 22:43 ` Clemens Buchacher
2004-07-29 14:48 ` [Kernel-janitors] " Vojtech Pavlik
0 siblings, 1 reply; 7+ messages in thread
From: Clemens Buchacher @ 2004-07-10 22:43 UTC (permalink / raw)
To: Vojtech Pavlik; +Cc: Linux-ide list, kernel janitors list
On Sat, Jul 10, 2004 at 04:03:37PM +0200, Vojtech Pavlik wrote:
> How about if you used the "min_t" and "max_t" macros inside FIT? That
> could help get rid of the casts completely (assuming the second and
> third parameters to FIT are expected to be constants).
Yes, that's better. It reduces the necessary changes to a minimum.
ide-timing.h | 25 ++++++++++++-------------
1 files changed, 12 insertions(+), 13 deletions(-)
diff -Nur a/drivers/ide/ide-timing.h b/drivers/ide/ide-timing.h
--- a/drivers/ide/ide-timing.h 2004-07-11 00:27:38 +02:00
+++ b/drivers/ide/ide-timing.h 2004-07-11 00:27:01 +02:00
@@ -27,6 +27,7 @@
* Vojtech Pavlik, Simunkova 1594, Prague 8, 182 00 Czech Republic
*/
+#include <linux/kernel.h>
#include <linux/hdreg.h>
#define XFER_PIO_5 0x0d
@@ -96,11 +97,9 @@
#define IDE_TIMING_UDMA 0x80
#define IDE_TIMING_ALL 0xff
-#define MIN(a,b) ((a)<(b)?(a):(b))
-#define MAX(a,b) ((a)>(b)?(a):(b))
-#define FIT(v,min,max) MAX(MIN(v,max),min)
-#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
-#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
+#define FIT(v,vmin,vmax) max_t(short,min_t(short,v,vmax),vmin)
+#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
+#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
#define XFER_MODE 0xf0
#define XFER_UDMA_133 0x48
@@ -188,14 +187,14 @@
static void ide_timing_merge(struct ide_timing *a, struct ide_timing *b, struct ide_timing *m, unsigned int what)
{
- if (what & IDE_TIMING_SETUP ) m->setup = MAX(a->setup, b->setup);
- if (what & IDE_TIMING_ACT8B ) m->act8b = MAX(a->act8b, b->act8b);
- if (what & IDE_TIMING_REC8B ) m->rec8b = MAX(a->rec8b, b->rec8b);
- if (what & IDE_TIMING_CYC8B ) m->cyc8b = MAX(a->cyc8b, b->cyc8b);
- if (what & IDE_TIMING_ACTIVE ) m->active = MAX(a->active, b->active);
- if (what & IDE_TIMING_RECOVER) m->recover = MAX(a->recover, b->recover);
- if (what & IDE_TIMING_CYCLE ) m->cycle = MAX(a->cycle, b->cycle);
- if (what & IDE_TIMING_UDMA ) m->udma = MAX(a->udma, b->udma);
+ if (what & IDE_TIMING_SETUP ) m->setup = max(a->setup, b->setup);
+ if (what & IDE_TIMING_ACT8B ) m->act8b = max(a->act8b, b->act8b);
+ if (what & IDE_TIMING_REC8B ) m->rec8b = max(a->rec8b, b->rec8b);
+ if (what & IDE_TIMING_CYC8B ) m->cyc8b = max(a->cyc8b, b->cyc8b);
+ if (what & IDE_TIMING_ACTIVE ) m->active = max(a->active, b->active);
+ if (what & IDE_TIMING_RECOVER) m->recover = max(a->recover, b->recover);
+ if (what & IDE_TIMING_CYCLE ) m->cycle = max(a->cycle, b->cycle);
+ if (what & IDE_TIMING_UDMA ) m->udma = max(a->udma, b->udma);
}
static struct ide_timing* ide_timing_find_mode(short speed)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] MIN/MAX in ide-timing.h
2004-07-10 13:55 ` [Kernel-janitors] " Luiz Fernando N. Capitulino
@ 2004-07-11 9:12 ` Clemens Buchacher
2004-07-11 13:09 ` Luiz Fernando N. Capitulino
0 siblings, 1 reply; 7+ messages in thread
From: Clemens Buchacher @ 2004-07-11 9:12 UTC (permalink / raw)
To: Luiz Fernando N. Capitulino; +Cc: vojtech, linux-ide, kernel-janitors
On Sat, Jul 10, 2004 at 10:55:29AM -0300, Luiz Fernando N. Capitulino wrote:
> | -#define MIN(a,b) ((a)<(b)?(a):(b))
> | -#define MAX(a,b) ((a)>(b)?(a):(b))
> | -#define FIT(v,min,max) MAX(MIN(v,max),min)
> | -#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
> | -#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
> | +#define FIT(v,vmin,vmax) max(min(v,vmax),vmin)
> | +#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
> | +#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
>
> Did you changed something in ENOUGH and EZ ?
Yes, I added an additional tab to line up with the FIT definition.
> Maybe we can try to make it inlined functions, thus maybe is possible to
> solve the warning problem.
That's fine too.
ide-timing.h | 52 +++++++++++++++++++++++++++++++---------------------
pci/amd74xx.c | 14 +++++++-------
pci/via82cxxx.c | 14 +++++++-------
3 files changed, 45 insertions(+), 35 deletions(-)
diff -Nur a/drivers/ide/ide-timing.h b/drivers/ide/ide-timing.h
--- a/drivers/ide/ide-timing.h 2004-07-11 00:27:38 +02:00
+++ b/drivers/ide/ide-timing.h 2004-07-11 10:59:23 +02:00
@@ -27,6 +27,7 @@
* Vojtech Pavlik, Simunkova 1594, Prague 8, 182 00 Czech Republic
*/
+#include <linux/kernel.h>
#include <linux/hdreg.h>
#define XFER_PIO_5 0x0d
@@ -96,11 +97,20 @@
#define IDE_TIMING_UDMA 0x80
#define IDE_TIMING_ALL 0xff
-#define MIN(a,b) ((a)<(b)?(a):(b))
-#define MAX(a,b) ((a)>(b)?(a):(b))
-#define FIT(v,min,max) MAX(MIN(v,max),min)
-#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
-#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
+static inline short fit(short val, short min, short max)
+{
+ return max_t(short, min_t(short, val, max), min);
+}
+
+static inline short enough(short val, short unit)
+{
+ return (val-1)/(unit+1);
+}
+
+static inline short enough_zero(short val, short unit)
+{
+ return val ? enough(val, unit) : 0;
+}
#define XFER_MODE 0xf0
#define XFER_UDMA_133 0x48
@@ -176,26 +186,26 @@
static void ide_timing_quantize(struct ide_timing *t, struct ide_timing *q, int T, int UT)
{
- q->setup = EZ(t->setup * 1000, T);
- q->act8b = EZ(t->act8b * 1000, T);
- q->rec8b = EZ(t->rec8b * 1000, T);
- q->cyc8b = EZ(t->cyc8b * 1000, T);
- q->active = EZ(t->active * 1000, T);
- q->recover = EZ(t->recover * 1000, T);
- q->cycle = EZ(t->cycle * 1000, T);
- q->udma = EZ(t->udma * 1000, UT);
+ q->setup = enough_zero(t->setup * 1000, T);
+ q->act8b = enough_zero(t->act8b * 1000, T);
+ q->rec8b = enough_zero(t->rec8b * 1000, T);
+ q->cyc8b = enough_zero(t->cyc8b * 1000, T);
+ q->active = enough_zero(t->active * 1000, T);
+ q->recover = enough_zero(t->recover * 1000, T);
+ q->cycle = enough_zero(t->cycle * 1000, T);
+ q->udma = enough_zero(t->udma * 1000, UT);
}
static void ide_timing_merge(struct ide_timing *a, struct ide_timing *b, struct ide_timing *m, unsigned int what)
{
- if (what & IDE_TIMING_SETUP ) m->setup = MAX(a->setup, b->setup);
- if (what & IDE_TIMING_ACT8B ) m->act8b = MAX(a->act8b, b->act8b);
- if (what & IDE_TIMING_REC8B ) m->rec8b = MAX(a->rec8b, b->rec8b);
- if (what & IDE_TIMING_CYC8B ) m->cyc8b = MAX(a->cyc8b, b->cyc8b);
- if (what & IDE_TIMING_ACTIVE ) m->active = MAX(a->active, b->active);
- if (what & IDE_TIMING_RECOVER) m->recover = MAX(a->recover, b->recover);
- if (what & IDE_TIMING_CYCLE ) m->cycle = MAX(a->cycle, b->cycle);
- if (what & IDE_TIMING_UDMA ) m->udma = MAX(a->udma, b->udma);
+ if (what & IDE_TIMING_SETUP ) m->setup = max(a->setup, b->setup);
+ if (what & IDE_TIMING_ACT8B ) m->act8b = max(a->act8b, b->act8b);
+ if (what & IDE_TIMING_REC8B ) m->rec8b = max(a->rec8b, b->rec8b);
+ if (what & IDE_TIMING_CYC8B ) m->cyc8b = max(a->cyc8b, b->cyc8b);
+ if (what & IDE_TIMING_ACTIVE ) m->active = max(a->active, b->active);
+ if (what & IDE_TIMING_RECOVER) m->recover = max(a->recover, b->recover);
+ if (what & IDE_TIMING_CYCLE ) m->cycle = max(a->cycle, b->cycle);
+ if (what & IDE_TIMING_UDMA ) m->udma = max(a->udma, b->udma);
}
static struct ide_timing* ide_timing_find_mode(short speed)
diff -Nur a/drivers/ide/pci/amd74xx.c b/drivers/ide/pci/amd74xx.c
--- a/drivers/ide/pci/amd74xx.c 2004-07-11 00:27:38 +02:00
+++ b/drivers/ide/pci/amd74xx.c 2004-07-11 11:00:06 +02:00
@@ -205,20 +205,20 @@
unsigned char t;
pci_read_config_byte(dev, AMD_ADDRESS_SETUP, &t);
- t = (t & ~(3 << ((3 - dn) << 1))) | ((FIT(timing->setup, 1, 4) - 1) << ((3 - dn) << 1));
+ t = (t & ~(3 << ((3 - dn) << 1))) | ((fit(timing->setup, 1, 4) - 1) << ((3 - dn) << 1));
pci_write_config_byte(dev, AMD_ADDRESS_SETUP, t);
pci_write_config_byte(dev, AMD_8BIT_TIMING + (1 - (dn >> 1)),
- ((FIT(timing->act8b, 1, 16) - 1) << 4) | (FIT(timing->rec8b, 1, 16) - 1));
+ ((fit(timing->act8b, 1, 16) - 1) << 4) | (fit(timing->rec8b, 1, 16) - 1));
pci_write_config_byte(dev, AMD_DRIVE_TIMING + (3 - dn),
- ((FIT(timing->active, 1, 16) - 1) << 4) | (FIT(timing->recover, 1, 16) - 1));
+ ((fit(timing->active, 1, 16) - 1) << 4) | (fit(timing->recover, 1, 16) - 1));
switch (amd_config->flags & AMD_UDMA) {
- case AMD_UDMA_33: t = timing->udma ? (0xc0 | (FIT(timing->udma, 2, 5) - 2)) : 0x03; break;
- case AMD_UDMA_66: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 2, 10)]) : 0x03; break;
- case AMD_UDMA_100: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 1, 10)]) : 0x03; break;
- case AMD_UDMA_133: t = timing->udma ? (0xc0 | amd_cyc2udma[FIT(timing->udma, 1, 15)]) : 0x03; break;
+ case AMD_UDMA_33: t = timing->udma ? (0xc0 | (fit(timing->udma, 2, 5) - 2)) : 0x03; break;
+ case AMD_UDMA_66: t = timing->udma ? (0xc0 | amd_cyc2udma[fit(timing->udma, 2, 10)]) : 0x03; break;
+ case AMD_UDMA_100: t = timing->udma ? (0xc0 | amd_cyc2udma[fit(timing->udma, 1, 10)]) : 0x03; break;
+ case AMD_UDMA_133: t = timing->udma ? (0xc0 | amd_cyc2udma[fit(timing->udma, 1, 15)]) : 0x03; break;
default: return;
}
diff -Nur a/drivers/ide/pci/via82cxxx.c b/drivers/ide/pci/via82cxxx.c
--- a/drivers/ide/pci/via82cxxx.c 2004-07-11 00:27:38 +02:00
+++ b/drivers/ide/pci/via82cxxx.c 2004-07-11 10:59:51 +02:00
@@ -291,21 +291,21 @@
if (~via_config->flags & VIA_BAD_AST) {
pci_read_config_byte(dev, VIA_ADDRESS_SETUP, &t);
- t = (t & ~(3 << ((3 - dn) << 1))) | ((FIT(timing->setup, 1, 4) - 1) << ((3 - dn) << 1));
+ t = (t & ~(3 << ((3 - dn) << 1))) | ((fit(timing->setup, 1, 4) - 1) << ((3 - dn) << 1));
pci_write_config_byte(dev, VIA_ADDRESS_SETUP, t);
}
pci_write_config_byte(dev, VIA_8BIT_TIMING + (1 - (dn >> 1)),
- ((FIT(timing->act8b, 1, 16) - 1) << 4) | (FIT(timing->rec8b, 1, 16) - 1));
+ ((fit(timing->act8b, 1, 16) - 1) << 4) | (fit(timing->rec8b, 1, 16) - 1));
pci_write_config_byte(dev, VIA_DRIVE_TIMING + (3 - dn),
- ((FIT(timing->active, 1, 16) - 1) << 4) | (FIT(timing->recover, 1, 16) - 1));
+ ((fit(timing->active, 1, 16) - 1) << 4) | (fit(timing->recover, 1, 16) - 1));
switch (via_config->flags & VIA_UDMA) {
- case VIA_UDMA_33: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, 5) - 2)) : 0x03; break;
- case VIA_UDMA_66: t = timing->udma ? (0xe8 | (FIT(timing->udma, 2, 9) - 2)) : 0x0f; break;
- case VIA_UDMA_100: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, 9) - 2)) : 0x07; break;
- case VIA_UDMA_133: t = timing->udma ? (0xe0 | (FIT(timing->udma, 2, 9) - 2)) : 0x07; break;
+ case VIA_UDMA_33: t = timing->udma ? (0xe0 | (fit(timing->udma, 2, 5) - 2)) : 0x03; break;
+ case VIA_UDMA_66: t = timing->udma ? (0xe8 | (fit(timing->udma, 2, 9) - 2)) : 0x0f; break;
+ case VIA_UDMA_100: t = timing->udma ? (0xe0 | (fit(timing->udma, 2, 9) - 2)) : 0x07; break;
+ case VIA_UDMA_133: t = timing->udma ? (0xe0 | (fit(timing->udma, 2, 9) - 2)) : 0x07; break;
default: return;
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] MIN/MAX in ide-timing.h
2004-07-11 9:12 ` Clemens Buchacher
@ 2004-07-11 13:09 ` Luiz Fernando N. Capitulino
0 siblings, 0 replies; 7+ messages in thread
From: Luiz Fernando N. Capitulino @ 2004-07-11 13:09 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: vojtech, linux-ide, kernel-janitors
Em Sun, 11 Jul 2004 11:12:50 +0200
Clemens Buchacher <drizzd@aon.at> screveu:
| On Sat, Jul 10, 2004 at 10:55:29AM -0300, Luiz Fernando N. Capitulino wrote:
| > | -#define MIN(a,b) ((a)<(b)?(a):(b))
| > | -#define MAX(a,b) ((a)>(b)?(a):(b))
| > | -#define FIT(v,min,max) MAX(MIN(v,max),min)
| > | -#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
| > | -#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
| > | +#define FIT(v,vmin,vmax) max(min(v,vmax),vmin)
| > | +#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
| > | +#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
| >
| > Did you changed something in ENOUGH and EZ ?
|
| Yes, I added an additional tab to line up with the FIT definition.
|
| > Maybe we can try to make it inlined functions, thus maybe is possible to
| > solve the warning problem.
|
| That's fine too.
I just read the Vojtech's sugestion after I send this e-mail, I think
he's tip is very better (more simple, less changes).
Sorry for that.
--
Luiz Fernando
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Kernel-janitors] Re: [patch] MIN/MAX in ide-timing.h
2004-07-10 22:43 ` Clemens Buchacher
@ 2004-07-29 14:48 ` Vojtech Pavlik
0 siblings, 0 replies; 7+ messages in thread
From: Vojtech Pavlik @ 2004-07-29 14:48 UTC (permalink / raw)
To: Clemens Buchacher, B.Zolnierkiewicz; +Cc: Linux-ide list, kernel janitors list
[-- Attachment #1: Type: text/plain, Size: 2723 bytes --]
On Sun, Jul 11, 2004 at 12:43:29AM +0200, Clemens Buchacher wrote:
> On Sat, Jul 10, 2004 at 04:03:37PM +0200, Vojtech Pavlik wrote:
> > How about if you used the "min_t" and "max_t" macros inside FIT? That
> > could help get rid of the casts completely (assuming the second and
> > third parameters to FIT are expected to be constants).
>
> Yes, that's better. It reduces the necessary changes to a minimum.
Bartolomiej, please apply this patch. Thanks.
diff -Nur a/drivers/ide/ide-timing.h b/drivers/ide/ide-timing.h
--- a/drivers/ide/ide-timing.h 2004-07-11 00:27:38 +02:00
+++ b/drivers/ide/ide-timing.h 2004-07-11 00:27:01 +02:00
@@ -27,6 +27,7 @@
* Vojtech Pavlik, Simunkova 1594, Prague 8, 182 00 Czech Republic
*/
+#include <linux/kernel.h>
#include <linux/hdreg.h>
#define XFER_PIO_5 0x0d
@@ -96,11 +97,9 @@
#define IDE_TIMING_UDMA 0x80
#define IDE_TIMING_ALL 0xff
-#define MIN(a,b) ((a)<(b)?(a):(b))
-#define MAX(a,b) ((a)>(b)?(a):(b))
-#define FIT(v,min,max) MAX(MIN(v,max),min)
-#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
-#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
+#define FIT(v,vmin,vmax) max_t(short,min_t(short,v,vmax),vmin)
+#define ENOUGH(v,unit) (((v)-1)/(unit)+1)
+#define EZ(v,unit) ((v)?ENOUGH(v,unit):0)
#define XFER_MODE 0xf0
#define XFER_UDMA_133 0x48
@@ -188,14 +187,14 @@
static void ide_timing_merge(struct ide_timing *a, struct ide_timing *b, struct ide_timing *m, unsigned int what)
{
- if (what & IDE_TIMING_SETUP ) m->setup = MAX(a->setup, b->setup);
- if (what & IDE_TIMING_ACT8B ) m->act8b = MAX(a->act8b, b->act8b);
- if (what & IDE_TIMING_REC8B ) m->rec8b = MAX(a->rec8b, b->rec8b);
- if (what & IDE_TIMING_CYC8B ) m->cyc8b = MAX(a->cyc8b, b->cyc8b);
- if (what & IDE_TIMING_ACTIVE ) m->active = MAX(a->active, b->active);
- if (what & IDE_TIMING_RECOVER) m->recover = MAX(a->recover, b->recover);
- if (what & IDE_TIMING_CYCLE ) m->cycle = MAX(a->cycle, b->cycle);
- if (what & IDE_TIMING_UDMA ) m->udma = MAX(a->udma, b->udma);
+ if (what & IDE_TIMING_SETUP ) m->setup = max(a->setup, b->setup);
+ if (what & IDE_TIMING_ACT8B ) m->act8b = max(a->act8b, b->act8b);
+ if (what & IDE_TIMING_REC8B ) m->rec8b = max(a->rec8b, b->rec8b);
+ if (what & IDE_TIMING_CYC8B ) m->cyc8b = max(a->cyc8b, b->cyc8b);
+ if (what & IDE_TIMING_ACTIVE ) m->active = max(a->active, b->active);
+ if (what & IDE_TIMING_RECOVER) m->recover = max(a->recover, b->recover);
+ if (what & IDE_TIMING_CYCLE ) m->cycle = max(a->cycle, b->cycle);
+ if (what & IDE_TIMING_UDMA ) m->udma = max(a->udma, b->udma);
}
static struct ide_timing* ide_timing_find_mode(short speed)
--
Vojtech Pavlik
SuSE Labs, SuSE CR
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2004-07-29 14:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-07-08 18:18 [patch] MIN/MAX in ide-timing.h Clemens Buchacher
2004-07-10 13:55 ` [Kernel-janitors] " Luiz Fernando N. Capitulino
2004-07-11 9:12 ` Clemens Buchacher
2004-07-11 13:09 ` Luiz Fernando N. Capitulino
2004-07-10 14:03 ` Vojtech Pavlik
2004-07-10 22:43 ` Clemens Buchacher
2004-07-29 14:48 ` [Kernel-janitors] " Vojtech Pavlik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).