[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4248: mtd: Return MTD Partition Information

[GitBox <gi...@apache.org>]

gustavonihei commented on a change in pull request #4248:
URL: https://github.com/apache/incubator-nuttx/pull/4248#discussion_r679892023



##########
File path: arch/arm/src/cxd56xx/cxd56_sfc.c
##########
@@ -200,6 +200,7 @@ static int cxd56_ioctl(FAR struct mtd_dev_s *dev, int cmd, unsigned long arg)
               geo->blocksize    = PAGE_SIZE;
               geo->erasesize    = SECTOR_SIZE;
               geo->neraseblocks = priv->density >> SECTOR_SHIFT;
+              geo->firstblock   = 0;

Review comment:
       > Think of what you see if you list partitions with fdisk or other partition tool. You would want, for example, the filesystem type if there is a file system in the partition. This comes from the magic number at the beginning of the partition (as in include/sys/statfs.h).
   > 
   > With fdisk -l you would see the device, an indication of the volume is bootable, start sector, end sector, size in setions, size in human-readable bytes, ID (https://www.win.tue.nl/~aeb/partitions/partition_types-1.html), and human readable type.
   
   > 
   > MTD makes this more complex, however, since MTD devices to do not have partition tables; mtd_partition.c is a software layer that imposes the partition layout on the device with nothing stored on the media. The good part of that design is that it is very light weight; the bad part is that only the firmware knows the partition layout of the FLASH memory. There is no way to move an MTD part from on platform to another while retaining the partition information. That should probably be made more standard someday.
   
   Ok, so for an MTD partition the proposed struct seems fit, maybe just an `endblock` field and change the `nblocks` to a `length` in bytes.
   
   > I believe that reading the partition table from a block device is a user application. Nothing special is required from the OS since the partition table can be directly read from the block device by a user-space application.
   
   Agreed. The information from a single partition (and the partition table, as well) can be retrieved both via IOCTL calls or via `procfs`. For the use case I am dealing with, an `ioctl` call seems more suitable. I believe the `procfs` approach fits best from NSH usage.
   
   So your idea is that we should have a generic `struct partition_info`, not specific to MTD. Basically that would be the previous `mtd_partition_info` with added info, something like:
   ```c
   struct partition_info_s
   {
     uint32_t        magic;        /* File system magic, 0 for RAW
                                    * (see <sys/statfs.h>) */
     size_t          length;       /* Partition size in bytes */
     size_t          numsectors;   /* Number of sectors in the partition */
     size_t          sectorsize;   /* Size in bytes of a single sector */
     off_t           startsector;  /* Offset to the first section/block of the
                                    * managed sub-region */
     off_t           endsector;    /* Offset to the last section/block of the
                                    * managed sub-region */
     FAR const char *parent;       /* Name of the parent node of the partition */
   };
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org