[riot-notifications] [RIOT-OS/RIOT] pkg/fatfs: add vfs integration for FatFs (#7104)

Vincent Dupont notifications at github.com
Wed Dec 20 12:46:40 CET 2017


vincent-d requested changes on this pull request.

I didn't have time to review thoroughly nor testing, but I found some minor code style issues.

> @@ -668,6 +668,11 @@ ifneq (,$(filter ccn-lite,$(USEPKG)))
   USEMODULE += ccn-lite-utils
 endif
 
+ifneq (,$(filter fatfs_vfs,$(USEMODULE)))
+    USEPKG += fatfs

Indent should be 2 spaces

>  
-ifneq (,$(filter fatfs_diskio_sdcard_spi,$(USEMODULE)))
-  DIRS += $(RIOTBASE)/pkg/fatfs/fatfs_diskio/sdcard_spi
+ifneq (,$(filter fatfs_vfs,$(USEMODULE)))
+    DIRS += $(RIOTBASE)/pkg/fatfs/fatfs_vfs

Indent 2 spaces

> + * @return             RES_ERROR if data wasn't read completely
+ * @return             RES_PARERR if prdv is invalid or has no mtd-driver set
+ */
+DRESULT disk_read(BYTE pdrv, BYTE *buff, DWORD sector, UINT count)
+{
+    DEBUG("disk_read: %d, %lu, %d\n", pdrv, sector, count);
+    if ( (pdrv >= _VOLUMES) || (fatfs_mtd_devs[pdrv]->driver == NULL) ) {
+        return RES_PARERR;
+    }
+
+    int res = mtd_read(fatfs_mtd_devs[pdrv], buff,
+                       sector * fatfs_mtd_devs[pdrv]->page_size,
+                       count * fatfs_mtd_devs[pdrv]->page_size);
+
+    if (res >= 0) {
+        uint32_t r_sect = ((unsigned)res)/fatfs_mtd_devs[pdrv]->page_size;

code style: spaces around '/'

> + * @return             RES_PARERR if prdv is invalid or has no mtd-driver set
+ */
+DRESULT disk_read(BYTE pdrv, BYTE *buff, DWORD sector, UINT count)
+{
+    DEBUG("disk_read: %d, %lu, %d\n", pdrv, sector, count);
+    if ( (pdrv >= _VOLUMES) || (fatfs_mtd_devs[pdrv]->driver == NULL) ) {
+        return RES_PARERR;
+    }
+
+    int res = mtd_read(fatfs_mtd_devs[pdrv], buff,
+                       sector * fatfs_mtd_devs[pdrv]->page_size,
+                       count * fatfs_mtd_devs[pdrv]->page_size);
+
+    if (res >= 0) {
+        uint32_t r_sect = ((unsigned)res)/fatfs_mtd_devs[pdrv]->page_size;
+        return ( (r_sect == count) ? RES_OK : RES_ERROR);

extra space between the two '('

> +
+    /* erase memory before writing to it */
+    int res = mtd_erase(fatfs_mtd_devs[pdrv],
+                        sector * fatfs_mtd_devs[pdrv]->page_size,
+                        count * fatfs_mtd_devs[pdrv]->page_size);
+
+    if (res != 0) {
+        return RES_ERROR; /* erase failed! */
+    }
+
+    res = mtd_write(fatfs_mtd_devs[pdrv], buff,
+                    sector * fatfs_mtd_devs[pdrv]->page_size,
+                    count * fatfs_mtd_devs[pdrv]->page_size);
+
+    if (res >= 0) {
+        uint32_t w_sect = ((unsigned)res)/fatfs_mtd_devs[pdrv]->page_size;

spaces around '/'

> +/**
+ * @brief              writes sectors to disk
+ *
+ * @param[in]  pdrv    Physical drive nmuber to identify the drive
+ * @param[in]  buff    Data to be written
+ * @param[in]  sector  Start sector in LBA
+ * @param[in]  count   Number of sectors to write
+ *
+ * @return             RES_OK if no error occurred
+ * @return             RES_ERROR if data wasn't written completely
+ * @return             RES_PARERR if prdv is invalid or has no mtd-driver set
+ */
+DRESULT disk_write(BYTE pdrv, const BYTE *buff, DWORD sector, UINT count)
+{
+    DEBUG("disk_write: %d, %lu, %d\n", pdrv, sector, count);
+    if ( (pdrv >= _VOLUMES) || (fatfs_mtd_devs[pdrv]->driver == NULL) ) {

extra space

> +/**
+ * @brief              reads sectors from disk
+ *
+ * @param[in]  pdrv    drive number to identify the drive
+ * @param[out] buff    Data buffer to store read data
+ * @param[in]  sector  Start sector in LBA
+ * @param[in]  count   Number of sectors to read
+ *
+ * @return             RES_OK if no error occurred
+ * @return             RES_ERROR if data wasn't read completely
+ * @return             RES_PARERR if prdv is invalid or has no mtd-driver set
+ */
+DRESULT disk_read(BYTE pdrv, BYTE *buff, DWORD sector, UINT count)
+{
+    DEBUG("disk_read: %d, %lu, %d\n", pdrv, sector, count);
+    if ( (pdrv >= _VOLUMES) || (fatfs_mtd_devs[pdrv]->driver == NULL) ) {

extra space

> + * @param[in]      pdrv    Physical drive nmuber (0.._VOLUMES-1)
+ * @param[in out]  cmd     Control code
+ * @param[in]      sector  Buffer to send/receive control data
+ *
+ * @return                 RES_OK if no error occurred
+ * @return                 RES_PARERR if cmd is unknown or
+ *                         prdv is either invalid or has no mtd-driver set
+ */
+DRESULT disk_ioctl(BYTE pdrv, BYTE cmd, void *buff)
+{
+    if ( (pdrv >= _VOLUMES) || (fatfs_mtd_devs[pdrv]->driver == NULL) ) {
+        return RES_PARERR;
+    }
+
+    switch (cmd) {
+        #if (_FS_READONLY == 0)

I think preprocessor statements should no be indented, here and in some other places

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/7104#pullrequestreview-84736355
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20171220/59484c5a/attachment.html>


More information about the notifications mailing list