Re: [RESEND PATCH 02/10] ide-floppy: move ide-floppy struct and macro defs into its own header. While at it

2008-01-05 Thread Bartlomiej Zolnierkiewicz 

Hi,

On Saturday 05 January 2008, Borislav Petkov wrote:
> On Fri, Jan 04, 2008 at 11:49:09PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > 
> > Hi,
> Hi Bart,
> 
> > Hmm, contrary to ide-cd.c case there doesn't seem to be a need currently for
> > moving code out of ide-floppy.c (and this patch series doesn't change that).
> ?, you mean this patch series _does_ change that, meaning it moves the struct

I mean that even with this patch series applied there is no need to for header
file since ide-floppy will still be the only user of it.

> defs into a header file. Do i get this correctly that we don't need the header
> file and the struct defs should remain in the .c file?

Yes.

> > Besides it would be better to just remove some structs like it has been done
> > with i.e. struct atapi_capabilities_page in ide-cd.c case [1] because:
> 
> [...]
> 
> > typedefs are evil (exceptions are rare) and should die :)
> > 
> 
> i'm redoing them right now against ide-2.6.git and will post when ready. By 
> the

ide-2.6.git is for syncing with Linus, the development tree is kept in quilt
tree and is merged by Andrew to -mm.  Please get the quilt patch series from:

http://www.kernel.org/pub/linux/kernel/people/bart/pata-2.6/

or use the latest 2.6.24-rc6-mm1 kernel (should have most of IDE patches).

> way, i have done some more cleanups in the meantime. Should i include them 
> into
> this series or send them later?

Include! :)

Thanks,
Bart

PS I still have to comment on patches #5/6/8/9.
--
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/

Re: [RESEND PATCH 02/10] ide-floppy: move ide-floppy struct and macro defs into its own header. While at it

2008-01-05 Thread Borislav Petkov 
On Fri, Jan 04, 2008 at 11:49:09PM +0100, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
Hi Bart,

> Hmm, contrary to ide-cd.c case there doesn't seem to be a need currently for
> moving code out of ide-floppy.c (and this patch series doesn't change that).
?, you mean this patch series _does_ change that, meaning it moves the struct
defs into a header file. Do i get this correctly that we don't need the header
file and the struct defs should remain in the .c file?

> Besides it would be better to just remove some structs like it has been done
> with i.e. struct atapi_capabilities_page in ide-cd.c case [1] because:

[...]

> typedefs are evil (exceptions are rare) and should die :)
> 

i'm redoing them right now against ide-2.6.git and will post when ready. By the
way, i have done some more cleanups in the meantime. Should i include them into
this series or send them later?

-- 
Regards/Gruß,
Boris.
--
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/

Re: [RESEND PATCH 02/10] ide-floppy: move ide-floppy struct and macro defs into its own header. While at it

2008-01-05 Thread Bartlomiej Zolnierkiewicz 

Hi,

On Saturday 05 January 2008, Borislav Petkov wrote:
 On Fri, Jan 04, 2008 at 11:49:09PM +0100, Bartlomiej Zolnierkiewicz wrote:
  
  Hi,
 Hi Bart,
 
  Hmm, contrary to ide-cd.c case there doesn't seem to be a need currently for
  moving code out of ide-floppy.c (and this patch series doesn't change that).
 ?, you mean this patch series _does_ change that, meaning it moves the struct

I mean that even with this patch series applied there is no need to for header
file since ide-floppy will still be the only user of it.

 defs into a header file. Do i get this correctly that we don't need the header
 file and the struct defs should remain in the .c file?

Yes.

  Besides it would be better to just remove some structs like it has been done
  with i.e. struct atapi_capabilities_page in ide-cd.c case [1] because:
 
 [...]
 
  typedefs are evil (exceptions are rare) and should die :)
  
 
 i'm redoing them right now against ide-2.6.git and will post when ready. By 
 the

ide-2.6.git is for syncing with Linus, the development tree is kept in quilt
tree and is merged by Andrew to -mm.  Please get the quilt patch series from:

http://www.kernel.org/pub/linux/kernel/people/bart/pata-2.6/

or use the latest 2.6.24-rc6-mm1 kernel (should have most of IDE patches).

 way, i have done some more cleanups in the meantime. Should i include them 
 into
 this series or send them later?

Include! :)

Thanks,
Bart

PS I still have to comment on patches #5/6/8/9.
--
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/

Re: [RESEND PATCH 02/10] ide-floppy: move ide-floppy struct and macro defs into its own header. While at it

2008-01-04 Thread Bartlomiej Zolnierkiewicz 

Hi,

Hmm, contrary to ide-cd.c case there doesn't seem to be a need currently for
moving code out of ide-floppy.c (and this patch series doesn't change that).

Besides it would be better to just remove some structs like it has been done
with i.e. struct atapi_capabilities_page in ide-cd.c case [1] because:

* it is easier to audit/debug the code if you don't have to look at typedefs
  all the time just to see which bytes/bits the code is really accessing

* not using typedefs will make the source code significantly smaller

* documentation is publically available

* these structs are not memory mapped

[1] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg242747.html

On Thursday 03 January 2008, Borislav Petkov wrote:
> - do a white-space cleanup
> - remove old crufty code untouched since at least 2005

Please try to not mix things like moving code around and doing actual
changes because it makes patch review difficult (i.e. it took me quite a
while to find "old crufty code" that has been removed)...

Sometimes having more "trivial" patches is better...

> - shorten lines longer than 80ish columns
> - shorten some LAAARGE typenames.

typedefs are evil (exceptions are rare) and should die :)

> There should be no functional changes resulting from this patch.
> 
> Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]>
> ---
>  drivers/ide/ide-floppy.c |  763 
> --
>  drivers/ide/ide-floppy.h |  382 +++
>  2 files changed, 581 insertions(+), 564 deletions(-)
> 
> diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
> index 785fbde..52d09c1 100644
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c

[...]

> -} idefloppy_capabilities_page_t;

[...]

> -} idefloppy_flexible_disk_page_t;

[...]

> -} idefloppy_capacity_descriptor_t;

as mentioned earlier it would be best to just remove these structs/typedefs

[ respective structures are described in SFF-8070i spec, it is useful to audit
  the final code against the spec to catch potential coding mistakes early ]

[...]

> -#if 0
> -/*
> - *   Special requests for our block device strategy routine.
> - */
> -#define  IDEFLOPPY_FIRST_RQ  90
> -
> -/*
> - *   IDEFLOPPY_PC_RQ is used to queue a packet command in the request queue.
> - */
> -#define  IDEFLOPPY_PC_RQ 90
> -
> -#define IDEFLOPPY_LAST_RQ90
> -
> -/*
> - *   A macro which can be used to check if a given request command
> - *   originated in the driver or in the buffer cache layer.
> - */
> -#define IDEFLOPPY_RQ_CMD(cmd)((cmd >= IDEFLOPPY_FIRST_RQ) && (cmd <= 
> IDEFLOPPY_LAST_RQ))
> -
> -#endif

Well, it was already removed, the patch is in IDE tree (and thus in -mm).

http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.24-rc6/2.6.24-rc6-mm1/broken-out/ide-mm-ide-floppy-remove-dead-code.patch

If possible please base your patches against IDE tree or the latest -mm patch.

[...]

> -} idefloppy_inquiry_result_t;

[...]

> -} idefloppy_request_sense_result_t;

[...]

> -} idefloppy_mode_parameter_header_t;

more structs/typedefs heading for /dev/null

[...]

> @@ -1689,9 +1304,12 @@ static int idefloppy_identify_device (ide_drive_t 
> *drive,struct hd_driveid *id)
>   printk(KERN_INFO "Write buffer size(?): %d bytes\n",id->buf_size*512);
>   printk(KERN_INFO "DMA: %s",id->capability & 0x01 ? "Yes\n":"No\n");
>   printk(KERN_INFO "LBA: %s",id->capability & 0x02 ? "Yes\n":"No\n");
> - printk(KERN_INFO "IORDY can be disabled: %s",id->capability & 0x04 ? 
> "Yes\n":"No\n");
> - printk(KERN_INFO "IORDY supported: %s",id->capability & 0x08 ? 
> "Yes\n":"Unknown\n");
> - printk(KERN_INFO "ATAPI overlap supported: %s",id->capability & 0x20 ? 
> "Yes\n":"No\n");
> + printk(KERN_INFO "IORDY can be disabled: %s",
> + id->capability & 0x04 ? "Yes\n":"No\n");
> + printk(KERN_INFO "IORDY supported: %s",
> + id->capability & 0x08 ? "Yes\n":"Unknown\n");
> + printk(KERN_INFO "ATAPI overlap supported: %s",
> + id->capability & 0x20 ? "Yes\n":"No\n");
>   printk(KERN_INFO "PIO Cycle Timing Category: %d\n",id->tPIO);
>   printk(KERN_INFO "DMA Cycle Timing Category: %d\n",id->tDMA);
>   printk(KERN_INFO "Single Word DMA supported modes:\n");
> @@ -1713,12 +1331,16 @@ static int idefloppy_identify_device (ide_drive_t 
> *drive,struct hd_driveid *id)
>   sprintf(buffer, "Not supported");
>   else
>   sprintf(buffer, "%d ns",id->eide_dma_min);
> - printk(KERN_INFO "Minimum Multi-word DMA cycle per word: %s\n", 
> buffer);
> + printk(KERN_INFO "Minimum Multi-word DMA cycle per word: %s\n",
> + buffer);
> +
>   if (id->eide_dma_time == 0)
>   sprintf(buffer, "Not supported");
>   else
>   sprintf(buffer, "%d

[RESEND PATCH 02/10] ide-floppy: move ide-floppy struct and macro defs into its own header. While at it

2008-01-03 Thread Borislav Petkov 
- do a white-space cleanup
- remove old crufty code untouched since at least 2005
- shorten lines longer than 80ish columns
- shorten some LAAARGE typenames.

There should be no functional changes resulting from this patch.

Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]>
---
 drivers/ide/ide-floppy.c |  763 --
 drivers/ide/ide-floppy.h |  382 +++
 2 files changed, 581 insertions(+), 564 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 785fbde..52d09c1 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -44,418 +44,7 @@
 #include 
 #include 
 
-/*
- * The following are used to debug the driver.
- */
-#define IDEFLOPPY_DEBUG_LOG0
-#define IDEFLOPPY_DEBUG_INFO   0
-#define IDEFLOPPY_DEBUG_BUGS   1
-
-/* #define IDEFLOPPY_DEBUG(fmt, args...) printk(KERN_INFO fmt, ## args) */
-#define IDEFLOPPY_DEBUG( fmt, args... )
-
-#if IDEFLOPPY_DEBUG_LOG
-#define debug_log printk
-#else
-#define debug_log(fmt, args... ) do {} while(0)
-#endif
-
-
-/*
- * Some drives require a longer irq timeout.
- */
-#define IDEFLOPPY_WAIT_CMD (5 * WAIT_CMD)
-
-/*
- * After each failed packet command we issue a request sense command
- * and retry the packet command IDEFLOPPY_MAX_PC_RETRIES times.
- */
-#define IDEFLOPPY_MAX_PC_RETRIES   3
-
-/*
- * With each packet command, we allocate a buffer of
- * IDEFLOPPY_PC_BUFFER_SIZE bytes.
- */
-#define IDEFLOPPY_PC_BUFFER_SIZE   256
-
-/*
- * In various places in the driver, we need to allocate storage
- * for packet commands and requests, which will remain valid while
- * we leave the driver to wait for an interrupt or a timeout event.
- */
-#define IDEFLOPPY_PC_STACK (10 + IDEFLOPPY_MAX_PC_RETRIES)
-
-/*
- * Our view of a packet command.
- */
-typedef struct idefloppy_packet_command_s {
-   u8 c[12];   /* Actual packet bytes */
-   int retries;/* On each retry, we increment 
retries */
-   int error;  /* Error code */
-   int request_transfer;   /* Bytes to transfer */
-   int actually_transferred;   /* Bytes actually transferred */
-   int buffer_size;/* Size of our data buffer */
-   int b_count;/* Missing/Available data on 
the current buffer */
-   struct request *rq; /* The corresponding request */
-   u8 *buffer; /* Data buffer */
-   u8 *current_position;   /* Pointer into the above 
buffer */
-   void (*callback) (ide_drive_t *);   /* Called when this packet 
command is completed */
-   u8 pc_buffer[IDEFLOPPY_PC_BUFFER_SIZE]; /* Temporary buffer */
-   unsigned long flags;/* Status/Action bit flags: 
long for set_bit */
-} idefloppy_pc_t;
-
-/*
- * Packet command flag bits.
- */
-#definePC_ABORT0   /* Set when an error is 
considered normal - We won't retry */
-#define PC_DMA_RECOMMENDED 2   /* 1 when we prefer to use DMA 
if possible */
-#definePC_DMA_IN_PROGRESS  3   /* 1 while DMA in 
progress */
-#definePC_DMA_ERROR4   /* 1 when encountered 
problem during DMA */
-#definePC_WRITING  5   /* Data direction */
-
-#definePC_SUPPRESS_ERROR   6   /* Suppress error 
reporting */
-
-/*
- * Removable Block Access Capabilities Page
- */
-typedef struct {
-#if defined(__LITTLE_ENDIAN_BITFIELD)
-   unsignedpage_code   :6; /* Page code - Should be 0x1b */
-   unsignedreserved1_6 :1; /* Reserved */
-   unsignedps  :1; /* Should be 0 */
-#elif defined(__BIG_ENDIAN_BITFIELD)
-   unsignedps  :1; /* Should be 0 */
-   unsignedreserved1_6 :1; /* Reserved */
-   unsignedpage_code   :6; /* Page code - Should be 0x1b */
-#else
-#error "Bitfield endianness not defined! Check your byteorder.h"
-#endif
-   u8  page_length;/* Page Length - Should be 0xa 
*/
-#if defined(__LITTLE_ENDIAN_BITFIELD)
-   unsignedreserved2   :6;
-   unsignedsrfp:1; /* Supports reporting progress 
of format */
-   unsignedsflp:1; /* System floppy type device */
-   unsignedtlun:3; /* Total logical units 
supported by the device */
-   unsignedreserved3   :3;
-   unsignedsml :1; /* Single / Multiple lun 
supported */
-   unsignedncd :1; /* Non cd optical device */
-#elif defined(__BIG_ENDIAN_BITFIELD)
-

[RESEND PATCH 02/10] ide-floppy: move ide-floppy struct and macro defs into its own header. While at it

2008-01-03 Thread Borislav Petkov 
- do a white-space cleanup
- remove old crufty code untouched since at least 2005
- shorten lines longer than 80ish columns
- shorten some LAAARGE typenames.

There should be no functional changes resulting from this patch.

Signed-off-by: Borislav Petkov [EMAIL PROTECTED]
---
 drivers/ide/ide-floppy.c |  763 --
 drivers/ide/ide-floppy.h |  382 +++
 2 files changed, 581 insertions(+), 564 deletions(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 785fbde..52d09c1 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -44,418 +44,7 @@
 #include asm/io.h
 #include asm/unaligned.h
 
-/*
- * The following are used to debug the driver.
- */
-#define IDEFLOPPY_DEBUG_LOG0
-#define IDEFLOPPY_DEBUG_INFO   0
-#define IDEFLOPPY_DEBUG_BUGS   1
-
-/* #define IDEFLOPPY_DEBUG(fmt, args...) printk(KERN_INFO fmt, ## args) */
-#define IDEFLOPPY_DEBUG( fmt, args... )
-
-#if IDEFLOPPY_DEBUG_LOG
-#define debug_log printk
-#else
-#define debug_log(fmt, args... ) do {} while(0)
-#endif
-
-
-/*
- * Some drives require a longer irq timeout.
- */
-#define IDEFLOPPY_WAIT_CMD (5 * WAIT_CMD)
-
-/*
- * After each failed packet command we issue a request sense command
- * and retry the packet command IDEFLOPPY_MAX_PC_RETRIES times.
- */
-#define IDEFLOPPY_MAX_PC_RETRIES   3
-
-/*
- * With each packet command, we allocate a buffer of
- * IDEFLOPPY_PC_BUFFER_SIZE bytes.
- */
-#define IDEFLOPPY_PC_BUFFER_SIZE   256
-
-/*
- * In various places in the driver, we need to allocate storage
- * for packet commands and requests, which will remain valid while
- * we leave the driver to wait for an interrupt or a timeout event.
- */
-#define IDEFLOPPY_PC_STACK (10 + IDEFLOPPY_MAX_PC_RETRIES)
-
-/*
- * Our view of a packet command.
- */
-typedef struct idefloppy_packet_command_s {
-   u8 c[12];   /* Actual packet bytes */
-   int retries;/* On each retry, we increment 
retries */
-   int error;  /* Error code */
-   int request_transfer;   /* Bytes to transfer */
-   int actually_transferred;   /* Bytes actually transferred */
-   int buffer_size;/* Size of our data buffer */
-   int b_count;/* Missing/Available data on 
the current buffer */
-   struct request *rq; /* The corresponding request */
-   u8 *buffer; /* Data buffer */
-   u8 *current_position;   /* Pointer into the above 
buffer */
-   void (*callback) (ide_drive_t *);   /* Called when this packet 
command is completed */
-   u8 pc_buffer[IDEFLOPPY_PC_BUFFER_SIZE]; /* Temporary buffer */
-   unsigned long flags;/* Status/Action bit flags: 
long for set_bit */
-} idefloppy_pc_t;
-
-/*
- * Packet command flag bits.
- */
-#definePC_ABORT0   /* Set when an error is 
considered normal - We won't retry */
-#define PC_DMA_RECOMMENDED 2   /* 1 when we prefer to use DMA 
if possible */
-#definePC_DMA_IN_PROGRESS  3   /* 1 while DMA in 
progress */
-#definePC_DMA_ERROR4   /* 1 when encountered 
problem during DMA */
-#definePC_WRITING  5   /* Data direction */
-
-#definePC_SUPPRESS_ERROR   6   /* Suppress error 
reporting */
-
-/*
- * Removable Block Access Capabilities Page
- */
-typedef struct {
-#if defined(__LITTLE_ENDIAN_BITFIELD)
-   unsignedpage_code   :6; /* Page code - Should be 0x1b */
-   unsignedreserved1_6 :1; /* Reserved */
-   unsignedps  :1; /* Should be 0 */
-#elif defined(__BIG_ENDIAN_BITFIELD)
-   unsignedps  :1; /* Should be 0 */
-   unsignedreserved1_6 :1; /* Reserved */
-   unsignedpage_code   :6; /* Page code - Should be 0x1b */
-#else
-#error Bitfield endianness not defined! Check your byteorder.h
-#endif
-   u8  page_length;/* Page Length - Should be 0xa 
*/
-#if defined(__LITTLE_ENDIAN_BITFIELD)
-   unsignedreserved2   :6;
-   unsignedsrfp:1; /* Supports reporting progress 
of format */
-   unsignedsflp:1; /* System floppy type device */
-   unsignedtlun:3; /* Total logical units 
supported by the device */
-   unsignedreserved3   :3;
-   unsignedsml :1; /* Single / Multiple lun 
supported */
-   unsignedncd :1; /* Non cd optical device */
-#elif