This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: fatfs and io disk misc patches


Andrew Lunn wrote:

On Thu, Jul 01, 2004 at 11:40:09AM +0200, Savin Zlobec wrote:


Andrew Lunn wrote:



On Thu, Jun 24, 2004 at 11:14:11AM +0200, Savin Zlobec wrote:




+2004-06-24 Savin Zlobec <savin@elatec.si> +
+ * src/synthdisk.c:
+ Removed static keyword before DISK_CHANNEL macro (which
+ has changed).
+
2004-01-15 Nick Garnett <nickg@calivar.com>


* cdl/synthdisk.cdl:
Index: devs/disk/synth/current/src/synthdisk.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/disk/synth/current/src/synthdisk.c,v
retrieving revision 1.1
diff -u -w -r1.1 synthdisk.c
--- devs/disk/synth/current/src/synthdisk.c 19 Jan 2004 14:35:01 -0000 1.1
+++ devs/disk/synth/current/src/synthdisk.c 24 Jun 2004 08:38:44 -0000
@@ -123,7 +123,7 @@
filefd: -1, \
filename: CYGDAT_IO_DISK_ECOSYNTH_DISK##_number_##_FILENAME \
}; \
-static DISK_CHANNEL(synth_disk_channel##_number_, \
+DISK_CHANNEL(synth_disk_channel##_number_, \
synth_disk_funs, \
synth_disk_info##_number_, \
_mbr_supp_ \




I don't know if this is the correct fix. You polute the name space. I
don't think synth_disk##_number_ needs to be a global symbol, so
static is correct. The macro defines two variables and only the first
is being made static. Probably there needs to be two macros, one for
the cyg_disk_info_t variable and a second for the dock_channel.




With the new implementation (malloc-less) the DISK_CHANNEL macro
is composed of devtab entry array (partitions devtabs), disk_channel array
(partitions disk channels), cyg_disk_info_t (disk info shared by all disk channels)
and the master disk channel . I've made them all static, thats why I removed
the static keywork before the macro usage in synthdisk.c. I don't think that
having multiple macros is a meaningfull solution, since the declarations are
interdependent and by declaring then separatly one would not gain anything.
But I realize that this doesn't fit well into the current concept.



OK. So this is actually part of the malloc-less patch which you have
not sent yet. That explains the confusion. I will keep this in my mbox
until you send the complete patch.


I've send it (http://ecos.sourceware.org/ml/ecos-patches/2004-06/msg00037.html),
but I forgot do diff it with -5 (making it unclear), sorry about that.


Here it is again with removed CYGNUM_IO_DISK_PART_NUM define - I didn't
change it into an CDL option, but I've added an additional parameter to DISK_CHANNEL
macro, so the low level driver can specify the maximal number of supported partitions -
the decision of making it an CDL option is now left to the driver implementation.
This way one can have multiple drivers with different max partitions, which I think
is more flexible.


It looks like there are a few other name space polution problems as
well. There are some types without cyg_ prefixes. It would be nice to
clean this up.


I agree. I've found the block devtab entry synth_disk_io##_number_
in synthdisk.c which can be made static, can you point out the odhers.



I was talking about types, not variables:


disk_callbacks_t, disk_channel, disk_funs all in disk.h


I tried to follow the naming convensions used in io subsystem -
like the io_serial :  serial_callbacks_t, serial_channel, serial_funs, ...
or io_eth : eth_drv_sc, eth_drv_funs, eth_hwr_funs, ...

savin
Index: io/disk/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/disk/current/ChangeLog,v
retrieving revision 1.1
diff -u -5 -r1.1 ChangeLog
--- io/disk/current/ChangeLog	19 Jan 2004 14:35:02 -0000	1.1
+++ io/disk/current/ChangeLog	1 Jul 2004 10:47:37 -0000
@@ -1,5 +1,14 @@
+2004-07-01  Savin Zlobec  <savin@elatec.si> 
+
+ 	* src/disk.c:
+ 	* include/disk.h:
+ 	* include/diskio.h:
+ 	Use predefined arrays for partition devices and info 
+ 	radher than malloc. Extended DISK_CHANNEL macro to
+  	support defining maximum number of partitions.
+
 2004-01-15  Nick Garnett  <nickg@calivar.com>
 
 	* src/disk.c:
 	* include/disk.h: Removed block_pos arguments from
 	hardware driver read and write calls: it is not necessary.
Index: io/disk/current/include/disk.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/disk/current/include/disk.h,v
retrieving revision 1.1
diff -u -5 -r1.1 disk.h
--- io/disk/current/include/disk.h	19 Jan 2004 14:35:03 -0000	1.1
+++ io/disk/current/include/disk.h	1 Jul 2004 10:47:37 -0000
@@ -104,27 +104,38 @@
     disk_funs               *funs;
     disk_callbacks_t        *callbacks;
     void                    *dev_priv;    // device private data
     cyg_disk_info_t         *info;        // disk info 
     cyg_disk_partition_t    *partition;   // partition data 
+    struct cyg_devtab_entry *pdevs_dev;   // partition devs devtab ents 
+    disk_channel            *pdevs_chan;  // partition devs disk chans 
     cyg_bool                 mbr_support; // true if disk has MBR
     cyg_bool                 valid;       // true if device valid 
     cyg_bool                 init;        // true if initialized
 };
 
 // Initialization macro for disk channel
 #define DISK_CHANNEL(_l,                                              \
                      _funs,                                           \
                      _dev_priv,                                       \
-                     _mbr_supp)                                       \
-cyg_disk_info_t _l##_disk_info;                                       \
-disk_channel _l = {                                                   \
+                     _mbr_supp,                                       \
+                     _max_part_num)                                   \
+static struct cyg_devtab_entry _l##_part_dev[_max_part_num];          \
+static disk_channel            _l##_part_chan[_max_part_num];         \
+static cyg_disk_partition_t    _l##_part_tab[_max_part_num];          \
+static cyg_disk_info_t         _l##_disk_info = {                     \
+    _l##_part_tab,                                                    \
+    _max_part_num                                                     \
+};                                                                    \
+static disk_channel _l = {                                            \
     &(_funs),                                                         \
     &cyg_io_disk_callbacks,                                           \
     &(_dev_priv),                                                     \
     &(_l##_disk_info),                                                \
     NULL,                                                             \
+    _l##_part_dev,                                                    \
+    _l##_part_chan,                                                   \
     _mbr_supp,                                                        \
     false,                                                            \
     false                                                             \
 };
 
Index: io/disk/current/include/diskio.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/disk/current/include/diskio.h,v
retrieving revision 1.1
diff -u -5 -r1.1 diskio.h
--- io/disk/current/include/diskio.h	19 Jan 2004 14:35:03 -0000	1.1
+++ io/disk/current/include/diskio.h	1 Jul 2004 10:47:37 -0000
@@ -56,11 +56,11 @@
 #include <cyg/io/config_keys.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
-    
+
 typedef struct {
     char        serial[20+1];      // serial number
     char        firmware_rev[8+1]; // firmware revision
     char        model_num[40+1];   // model number 
     cyg_uint32  cylinders_num;     // number of cylinders         (CHS)
@@ -76,13 +76,13 @@
     cyg_uint32 end;     // last sector number
     cyg_uint32 size;    // size in sectors
 } cyg_disk_partition_t;
 
 typedef struct {
+    cyg_disk_partition_t *partitions;    // partition table
+    int                   partitions_num;// partition table size
     cyg_disk_identify_t   ident;         // identify data
-    cyg_disk_partition_t  partitions[4]; // partitions
-    cyg_addrword_t        devs[4];       // device instances
     cyg_uint32            block_size;    // block size
     cyg_uint32            blocks_num;    // number of blocks on disk
     cyg_bool              connected;     // true if device connected
 } cyg_disk_info_t;
 
Index: io/disk/current/src/disk.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/disk/current/src/disk.c,v
retrieving revision 1.1
diff -u -5 -r1.1 disk.c
--- io/disk/current/src/disk.c	19 Jan 2004 14:35:03 -0000	1.1
+++ io/disk/current/src/disk.c	1 Jul 2004 10:47:38 -0000
@@ -54,12 +54,10 @@
 #include <cyg/io/devtab.h>
 #include <cyg/io/disk.h>
 #include <cyg/infra/cyg_ass.h>      // assertion support
 #include <cyg/infra/diag.h>         // diagnostic output
 
-#include <stdlib.h>  // malloc 
-
 // ---------------------------------------------------------------------------
 
 //#define DEBUG 1
 
 #ifdef DEBUG
@@ -170,11 +168,11 @@
     READ_CHS(&data[1], c, h, s);
     CHS_TO_LBA(&info->ident, c, h, s, part->start);
 
     READ_CHS(&data[5], c, h, s);
     CHS_TO_LBA(&info->ident, c, h, s, part->end);
-
+    
     READ_DWORD(&data[12], part->size);
 }
 
 //
 // Read Master Boot Record (partitions)
@@ -186,22 +184,27 @@
     disk_funs       *funs = chan->funs;
     cyg_uint8 buf[512];
     Cyg_ErrNo res = ENOERR;
     int i;
  
-    for (i = 0; i < MBR_PART_NUM; i++)
+    for (i = 0; i < info->partitions_num; i++)
         info->partitions[i].type = 0x00;    
    
     res = (funs->read)(chan, (void *)buf, 512, 0);
     if (ENOERR != res)
         return res;
 
     if (MBR_SIG_BYTE0 == buf[MBR_SIG_ADDR+0] && MBR_SIG_BYTE1 == buf[MBR_SIG_ADDR+1])
     {
+        int npart;
+
         D(("disk MBR found\n")); 
  
-        for (i = 0; i < MBR_PART_NUM; i++)
+        npart = info->partitions_num < MBR_PART_NUM ? 
+            info->partitions_num : MBR_PART_NUM;
+
+        for (i = 0; i < npart; i++)
         {
             cyg_disk_partition_t *part = &info->partitions[i];
             
             read_partition(&buf[MBR_PART_ADDR+MBR_PART_SIZE*i], info, part); 
 #ifdef DEBUG
@@ -229,18 +232,14 @@
 
     if (!chan->init)
     {
         info->connected = false;
 
-        // clear devices array (one per partition)
-        // and partition data
-        for (i = 0; i < MBR_PART_NUM; i++)
-        {
-            info->devs[i] = (cyg_addrword_t) 0;
+        // clear partition data
+        for (i = 0; i < info->partitions_num; i++)
             info->partitions[i].type = 0x00;
-        }
-        
+
         chan->init = true;
     }
     return true;
 }
 
@@ -292,26 +291,15 @@
         return -EINVAL;
     
     info->connected = false;
     chan->valid     = false;
      
-    // clear partition data and invalidate 
-    // any allocated devices
-    for (i = 0; i < MBR_PART_NUM; i++)
+    // clear partition data and invalidate partition devices 
+    for (i = 0; i < info->partitions_num; i++)
     {
-        info->partitions[i].type = 0x00;
-
-        if (0 != info->devs[i])
-        {
-            struct cyg_devtab_entry *dtab;
-            disk_channel            *dchan;
-
-            dtab  = (struct cyg_devtab_entry *)info->devs[i];
-            dchan = (disk_channel *) dtab->priv;
-            
-            dchan->valid = false;
-        }
+        info->partitions[i].type  = 0x00;
+        chan->pdevs_chan[i].valid = false;
     }
 
     
     D(("disk disconnected\n")); 
 
@@ -327,14 +315,26 @@
     cyg_disk_info_t *info = chan->info;
     struct cyg_devtab_entry *new_tab;
     disk_channel            *new_chan;
     int dev_num;
     
-    if (!info->connected || name[0] < '0' || name[0] > '4' || '\0' != name[1])
+    if (!info->connected)
         return -EINVAL;
 
-    dev_num = name[0] - '0';
+    dev_num = 0;
+
+    while ('\0' != *name)
+    {
+        if (*name < '0' || *name > '9')
+            return -EINVAL;
+
+        dev_num = 10 * dev_num + (*name - '0');
+        name++;
+    }
+   
+    if (dev_num > info->partitions_num)
+        return -EINVAL;
 
     D(("disk lookup dev number = %d\n", dev_num)); 
 
     if (0 == dev_num)
     {
@@ -344,44 +344,22 @@
     if (0x00 == info->partitions[dev_num-1].type)
     {
         D(("disk NO partition for dev\n")); 
         return -EINVAL;
     }
-    
-    if (0 == info->devs[dev_num-1])
-    {
-        D(("disk creating new devtab entry\n")); 
 
-        // alloc mem for new device
-        new_tab = (struct cyg_devtab_entry *)
-            malloc(sizeof(struct cyg_devtab_entry));
-        if (NULL == new_tab)
-            return -ENOMEM;        
-
-        // alloc mem for new device private data
-        new_chan = (disk_channel *)malloc(sizeof(disk_channel));
-        if (NULL == new_chan)
-        {
-            free(new_tab);
-            return -ENOMEM;
-        }
-    }
-    else
-    {
-        new_tab  = (struct cyg_devtab_entry *) info->devs[dev_num-1]; 
-        new_chan = (disk_channel *) new_tab->priv;   
-    }
+    new_tab  = &chan->pdevs_dev[dev_num-1];
+    new_chan = &chan->pdevs_chan[dev_num-1];
     
     // copy device data from parent
     *new_tab  = **tab; 
     *new_chan = *chan;
 
     new_tab->priv = (void *)new_chan;
 
-    // set partition ptr and put this device into devices array    
+    // set partition ptr
     new_chan->partition = &info->partitions[dev_num-1];
-    chan->info->devs[dev_num-1] = (cyg_addrword_t) new_tab;
 
     // return device tab 
     *tab = new_tab;
         
     return ENOERR;
Index: devs/disk/synth/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/disk/synth/current/ChangeLog,v
retrieving revision 1.1
diff -u -5 -r1.1 ChangeLog
--- devs/disk/synth/current/ChangeLog	19 Jan 2004 14:35:01 -0000	1.1
+++ devs/disk/synth/current/ChangeLog	1 Jul 2004 10:47:38 -0000
@@ -1,5 +1,10 @@
+2004-07-01  Savin Zlobec  <savin@elatec.si> 
+
+        * src/synthdisk.c:
+        Updated to work with the new DISK_CHANNEL macro definition.
+
 2004-01-15  Nick Garnett  <nickg@calivar.com>
 
 	* cdl/synthdisk.cdl:
 	* src/synthdisk.c: 	
 	Added _FILENAME option for disk instances to map device to an
Index: devs/disk/synth/current/src/synthdisk.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/devs/disk/synth/current/src/synthdisk.c,v
retrieving revision 1.1
diff -u -5 -r1.1 synthdisk.c
--- devs/disk/synth/current/src/synthdisk.c	19 Jan 2004 14:35:01 -0000	1.1
+++ devs/disk/synth/current/src/synthdisk.c	1 Jul 2004 10:47:38 -0000
@@ -121,14 +121,15 @@
     sectors_num:   _spt_,                                                  \
     size:          CYGNUM_IO_DISK_ECOSYNTH_DISK##_number_##_SIZE,          \
     filefd:        -1,                                                     \
     filename:      CYGDAT_IO_DISK_ECOSYNTH_DISK##_number_##_FILENAME       \
 };                                                                         \
-static DISK_CHANNEL(synth_disk_channel##_number_,                          \
-                    synth_disk_funs,                                       \
-                    synth_disk_info##_number_,                             \
-                    _mbr_supp_                                             \
+DISK_CHANNEL(synth_disk_channel##_number_,                                 \
+             synth_disk_funs,                                              \
+             synth_disk_info##_number_,                                    \
+             _mbr_supp_,                                                   \
+             4                                                             \
 );                                                                         \
 BLOCK_DEVTAB_ENTRY(synth_disk_io##_number_,                                \
              CYGDAT_IO_DISK_ECOSYNTH_DISK##_number_##_NAME,                \
              0,                                                            \
              &cyg_io_disk_devio,                                           \

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]