michallenc commented on PR #16642:
URL: https://github.com/apache/nuttx/pull/16642#issuecomment-3061900079
> > I replaced `ftl_initialize` and `bchdev_register` with
`register_mtddriver` call, but it seems all data are now written to a
pseudofile in RAM memory, not to the physical device. I did the same changes as
you in `boards/arm/samv7/common/src/sam_progmem.c`, so the registration process
should be fine (also tested on progmem, that has same issues). Maybe we should
use `register_partition_with_mtd` or `register_mtdpartition` here instead of
`register_mtddriver`?
>
> @michallenc It's my mistake. Yes, you're absolutely right—there is indeed
an issue here. From the context, the code is trying to create sub-partition
devices based on the main MTD device progmem_mtd. I will fix this on my end. By
the way, do you have this hardware board?
>
> @michallenc Wait, the issue might not be in the registration itself. I
noticed that `register_partition_with_mtd` ultimately calls `register_mtddriver
`internally. We don't need `register_partition_with_mtd` here—instead, we
should directly use `register_mtddriver` because `mtd_partition `is already
initialized in `boards/arm/samv7/common/src/sam_progmem.c`.
>
> @michallenc Could you give me some more information about this: _all data
are now written to a pseudofile in RAM memory, not to the physical device_
Sure, I am sorry, I should have specified this better. This is the file for
initialization of w25q nor flash. It's quite large, the important part is in
`sam_w25qxxxjv_init`. I did the same as you in
`boards/arm/samv7/common/src/sam_progmem.c`, replaced FTL/BCH init with one
call to MTD layer.
```C
#include <nuttx/config.h>
#include <stdio.h>
#include <stdbool.h>
#include <syslog.h>
#include <assert.h>
#include <debug.h>
#include <errno.h>
#include <nuttx/arch.h>
#include <nuttx/board.h>
#include <nuttx/drivers/drivers.h>
#include <nuttx/mtd/mtd.h>
#include <nuttx/spi/qspi.h>
#include <nuttx/net/usrsock.h>
#include <nuttx/fs/ioctl.h>
#include <nuttx/fs/smart.h>
#include <sys/ioctl.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#include "sam_qspi.h"
#include "sam_board.h"
#include "board_progmem.h" /* For struct mtd_partition_s definition */
#define QSPI_PORTNO 0
typedef enum
{
MTD_TYPE_BCH,
MTD_TYPE_SMARTFS,
} mtd_types;
static struct mtd_partition_s g_mtd_partition_table[] =
{
{
.size = CONFIG_BOARD_SLOT_SIZE, /* 1966 KiB, MCUboot partition */
.devpath = "/dev/ota1",
.type = MTD_TYPE_BCH,
},
{
.size = CONFIG_BOARD_SLOT_SIZE, /* 1966 KiB, MCUboot partition */
.devpath = "/dev/ota2",
.type = MTD_TYPE_BCH,
},
{
.size = 0x8000, /* 32KiB */
.devpath = "/etc",
.type = MTD_TYPE_SMARTFS,
},
{
.size = 0x800000, /* 8MiB */
.devpath = "/dev/mtdlog_maintenance",
.type = MTD_TYPE_BCH,
},
{
.size = 0x8000, /* 32 KiB */
.devpath = "/dev/mtdtest",
.type = MTD_TYPE_BCH,
},
{
.size = 0, /* Calculate the rest of the size */
.devpath = "/dev/mtdlog_trace",
.type = MTD_TYPE_BCH,
}
};
static const size_t g_mtd_partition_table_size =
nitems(g_mtd_partition_table);
#ifdef CONFIG_FS_SMARTFS
static int issmartfs(FAR const char *pathname)
{
struct smart_format_s fmt;
int fd;
int ret;
/* Find the inode of the block driver identified by 'source' */
fd = open(pathname, O_RDONLY);
if (fd < 0)
{
return ERROR;
}
/* Get the format information so we know the block have been formatted */
ret = ioctl(fd, BIOC_GETFORMAT, (unsigned long)&fmt);
if (ret < 0)
{
close(fd);
return ret;
}
if (!(fmt.flags & SMART_FMT_ISFORMATTED))
{
errno = EFTYPE;
ret = ERROR;
}
close(fd);
return ret;
}
static int mksmartfs(FAR const char *pathname, uint16_t sectorsize)
{
struct smart_format_s fmt;
struct smart_read_write_s request;
uint8_t type;
int fd;
int ret;
/* Find the inode of the block driver identified by 'pathname' */
fd = open(pathname, O_RDWR);
if (fd < 0)
{
ret = -ENOENT;
goto errout;
}
/* Perform a low-level SMART format */
ret = ioctl(fd, BIOC_LLFORMAT, sectorsize << 16);
if (ret != OK)
{
ret = -errno;
goto errout_with_driver;
}
/* Get the format information so we know how big the sectors are */
ret = ioctl(fd, BIOC_GETFORMAT, (unsigned long) &fmt);
if (ret != OK)
{
ret = -errno;
goto errout_with_driver;
}
/* Now write the filesystem to media. */
ret = ioctl(fd, BIOC_ALLOCSECT, SMARTFS_ROOT_DIR_SECTOR);
if (ret != SMARTFS_ROOT_DIR_SECTOR)
{
ret = -EIO;
goto errout_with_driver;
}
/* Mark this block as a directory entry */
type = SMARTFS_SECTOR_TYPE_DIR;
request.offset = 0;
request.count = 1;
request.buffer = &type;
request.logsector = SMARTFS_ROOT_DIR_SECTOR;
/* Issue a write to the sector, single byte */
ret = ioctl(fd, BIOC_WRITESECT, (unsigned long) &request);
if (ret != OK)
{
ret = -EIO;
goto errout_with_driver;
}
errout_with_driver:
/* Close the driver */
close(fd);
errout:
/* Return any reported errors */
if (ret < 0)
{
errno = -ret;
return ERROR;
}
return OK;
}
#endif /* CONFIG_FS_SMARTFS */
static struct mtd_dev_s
*sam_w25qxxxjv_alloc_mtdpart(struct mtd_dev_s *mtd,
struct mtd_geometry_s *geo,
struct mtd_partition_s
*part)
{
ASSERT((part->offset + part->size) <= geo->neraseblocks * geo->erasesize);
ASSERT((part->offset % geo->erasesize) == 0);
ASSERT((part->size % geo->erasesize) == 0);
size_t startblock = part->offset / geo->blocksize;
size_t blocks = part->size / geo->blocksize;
return mtd_partition(mtd, startblock, blocks);
}
int sam_w25qxxxjv_init(void)
{
int ret = OK;
#ifdef CONFIG_FS_SMARTFS
char partref[4];
char smartfs_path[30];
#endif
struct qspi_dev_s *qspi = sam_qspi_initialize(QSPI_PORTNO);
if (!qspi)
{
syslog(LOG_ERR, "ERROR: sam_qspi_initialize failed\n");
return ERROR;
}
/* Use the QSPI device instance to initialize the W25QxxJV device. */
struct mtd_dev_s *mtd = w25qxxxjv_initialize(qspi, true);
if (!mtd)
{
syslog(LOG_ERR, "ERROR: w25qxxxjv_initialize failed\n");
return ERROR;
}
/* Get geometry of W25QxxxJV */
struct mtd_geometry_s geometry;
ret = MTD_IOCTL(mtd, MTDIOC_GEOMETRY, (unsigned long)&geometry);
if (ret < 0)
{
syslog(LOG_ERR, "ERROR: w25qxxxjv ioctl failed: %s\n",
strerror(errno));
return ret;
}
size_t i;
size_t offset = 0;
for (i = 0; i < g_mtd_partition_table_size; i++)
{
struct mtd_partition_s *part = &g_mtd_partition_table[i];
part->offset = offset;
if (part->size == 0)
{
part->size = (geometry.neraseblocks * geometry.erasesize) - offset;
}
part->mtd = sam_w25qxxxjv_alloc_mtdpart(mtd, &geometry, part);
if (!part->mtd)
{
syslog(LOG_ERR, "ERROR: sam_w25qxxxjv_alloc_mtdpart failed\n");
return ERROR;
}
/* Use the FTL layer to wrap the MTD driver as a block driver */
if (part->type == MTD_TYPE_BCH)
{
ret = register_mtddriver(part->devpath, part->mtd, 0755, NULL);
if (ret < 0)
{
syslog(LOG_ERR, "ERROR: failed to register mtd %d", ret);
return ret;
}
// ret = ftl_initialize(W25QXXJV_MTD_MINOR + i, part->mtd);
// if (ret < 0)
// {
// syslog(LOG_ERR, "ERROR: Failed to initialize FTL layer:
%d\n",
// ret);
// return ret;
// }
//
// /* Now create a character device on the block device */
//
//#if defined(CONFIG_BCH)
// char blockdev[18];
// snprintf(blockdev, sizeof(blockdev), "/dev/mtdblock%d",
// W25QXXJV_MTD_MINOR + i);
// ret = bchdev_register(blockdev, part->devpath, false);
// if (ret < 0)
// {
// syslog(LOG_ERR, "ERROR: bchdev_register %s failed: %d\n",
// part->devpath, ret);
// return ret;
// }
//#endif /* defined(CONFIG_BCH) */
}
else if (part->type == MTD_TYPE_SMARTFS)
{
#if defined (CONFIG_FS_SMARTFS)
sprintf(partref, "p%d", i);
ret = smart_initialize(0, part->mtd, partref);
if (ret < 0)
{
syslog(LOG_ERR, "ERROR: smart_initialize %s failed: %d\n",
part->devpath, ret);
return ret;
}
sprintf(smartfs_path, "/dev/smart0%s", partref);
if (issmartfs(smartfs_path) != OK)
{
mksmartfs(smartfs_path, 0);
}
ret = nx_mount(smartfs_path, part->devpath, "smartfs", 0,
NULL);
if (ret < 0)
{
/* Mount failed. */
syslog(LOG_ERR, "ERROR: mount %s failed: %d\n", smartfs_path,
ret);
return ret;
}
#endif
}
else
{
syslog(LOG_ERR, "ERROR: Incorrect device type %d for %s.\n",
part->type, part->devpath);
}
offset += part->size;
}
return ret;
}
```
Then I have a simple application (see below) that writes to the flash.
```C
#include <nuttx/config.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdio.h>
#include <errno.h>
int main(int argc, FAR char *argv[])
{
int ret = 0;
int fd = open("/dev/mtdtest", O_RDWR | O_DIRECT);
if (fd < 0) {
printf("failed to open %d", errno);
return -1;
}
char buf[4] = {0x33, 0x22, 0x00, 0xab};
ret = write(fd, buf, sizeof buf);
if (ret < 0) {
printf("failed to write: %d\n", errno);
}
lseek(fd, 32, SEEK_SET);
ret = write(fd, buf, sizeof buf);
if (ret < 0) {
printf("failed to write: %d\n", errno);
}
buf[0] = 0xff;
ret = write(fd, buf, sizeof buf);
if (ret < 0) {
printf("failed to write: %d\n", errno);
}
lseek(fd, -4, SEEK_CUR);
buf[0] &= ~(1 << 7);
ret = write(fd, &buf[0], 1);
if (ret < 0) {
printf("failed to write: %d\n", errno);
}
close(fd);
return 0;
}
```
But instead of reading the data back with `hexdump`, I read just `0xff` and
some memory is consumed during each write. Thus the idea it writes to some
pseudofile instead of real flash.
```
brcg2> free
total used free maxused maxfree nused nfree name
323260 174268 148992 180816 144144 1493 17 Umem
brcg2> confirm_image
brcg2> hexdump /dev/mtdtest count=48
/dev/mtdtest at 00000000:
0000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
0010: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
0020: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
brcg2> free
total used free maxused maxfree nused nfree name
323260 175076 148184 180816 143880 1504 16 Umem
brcg2> confirm_image
brcg2> hexdump /dev/mtdtest count=48
/dev/mtdtest at 00000000:
0000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
0010: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
0020: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................
brcg2> free
total used free maxused maxfree nused nfree name
323260 175884 147376 180816 143424 1515 16 Umem
brcg2>
```
I saw the same behavior on `boards/arm/samv7/common/src/sam_progmem.c` so I
don't expect this is driver related. Maybe some configuration mistake on my
part? But I have `CONFIG_BCH` and `CONFIG_MTD` enabled and disabled buffers in
FTL layer.
--
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-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
