fs: Add model field to geometry and mtd_geometry_s

[xiaoxiang781216]

Summary

The model is very useful to track the device info. The upcoming patch will addd model info driver by driver.

Impact

New field

Testing

[gustavonihei]

I agree that the model information may be useful, but is it correct to add it to geometry? It will be a bit strange for it to be there while the struct is named geometry.

On the long run, I think the geometry struct could be refactored to be more than simply geometry details, and carry general low level details of the MTD.

So, if that's the idea, for compatibility purposes this refactor could be done in two phases:

1. Add all the MTD low-level info into the geometry structure (e.g. erase state, model, technology) -> compatibility maintained.
2. Rename the geometry struct and its interfaces (e.g. ioctl commands) -> breaking change.

[xiaoxiang781216]

I agree that the model information may be useful, but is it correct to add it to geometry?

Some exist fields is already not relate to geometry:):

  bool      geo_available;    /* true: The device is available */
  bool      geo_mediachanged; /* true: The media has changed since last query */
  bool      geo_writeenabled; /* true: It is okay to write to this device */

It will be a bit strange for it to be there while the struct is named geometry.

On the long run, I think the geometry struct could be refactored to be more than simply geometry details, and carry general low level details of the MTD.

So, if that's the idea, for compatibility purposes this refactor could be done in two phases:

1. Add all the MTD low-level info into the geometry structure (e.g. erase state, model, technology) -> compatibility maintained.

It's a reasonable approach.

2. Rename the geometry struct and its interfaces (e.g. ioctl commands) -> breaking change.

Could be too.

[Ouss4]

What's going to be stored in this field from the driver side? @xiaoxiang781216 do you have a usage for this? (just curious).

[xiaoxiang781216]

What's going to be stored in this field from the driver side? @xiaoxiang781216 do you have a usage for this? (just curious).

the flash part number, something like GD25WQ20E. The block/mtd driver could:

1. hard code a value if the driver just support a specific part number
2. build the value from the register value dynamically

[Ouss4]

What's going to be stored in this field from the driver side? @xiaoxiang781216 do you have a usage for this? (just curious).

the flash part number, something like GD25WQ20E. The block/mtd driver could:

1. hard code a value if the driver just support a specific part number
2. build the value from the register value dynamically

For the second point, this is what we do when reading the flash JEDEC ID and building the geometry. However, we are not keeping all the possible information we can get. I was thinking that this could be redundant with that, that sure have some overlap, but I don't think they are really redundant. What are the plans for other drivers like, ramtd, and mtdpartitions? Just hard code a value?

[xiaoxiang781216]

What's going to be stored in this field from the driver side? @xiaoxiang781216 do you have a usage for this? (just curious).

the flash part number, something like GD25WQ20E. The block/mtd driver could:

1. hard code a value if the driver just support a specific part number
2. build the value from the register value dynamically

For the second point, this is what we do when reading the flash JEDEC ID and building the geometry. However, we are not keeping all the possible information we can get. I was thinking that this could be redundant with that, that sure have some overlap, but I don't think they are really redundant.

What are the plans for other drivers like, ramtd, and mtdpartitions? Just hard code a value?

ramtd could add CONFIG_ or pass from rammtd_initialize. mtdpartitions should return the value from low layer real mtd device.

[xiaoxiang781216]

@gustavonihei I finally get the time to improve the patch as your suggestion, please take a look.

[xiaoxiang781216]

ping @gustavonihei

[gustavonihei]

@gustavonihei I finally get the time to improve the patch as your suggestion, please take a look.

Sorry for the delay, I didn't notice the first ping. So in this PR we'll be adding just the model information to both geometry structures, right? Is the remaining of the planning from https://github.com/apache/nuttx/pull/4320#issuecomment-896795388 going to implemented soon? Otherwise it is important to open an issue for that improvement to MTD's organization.

[xiaoxiang781216]

@gustavonihei I finally get the time to improve the patch as your suggestion, please take a look.

Sorry for the delay, I didn't notice the first ping. So in this PR we'll be adding just the model information to both geometry structures, right?

Yes, this PR is just for model.

Is the remaining of the planning from #4320 (comment) going to implemented soon?

move erase state to geometry and remove MTDIOC_ERASESTATE is the next step.

Otherwise it is important to open an issue for that improvement to MTD's organization.

Ok, let's me open an issue track it.

[xiaoxiang781216]

@gustavonihei here is the issue: https://github.com/apache/nuttx/issues/8376

[gustavonihei]

Is the remaining of the planning from #4320 (comment) going to implemented soon?

move erase state to geometry and remove MTDIOC_ERASESTATE is the next step.

Please, note that the NuttX port of MCUboot makes use of the MTDIOC_ERASESTATE: https://github.com/mcu-tools/mcuboot/blob/5047f032c93fd8957be6529f8f1ad7a3fe626057/boot/nuttx/src/flash_map_backend/flash_map_backend.c#L302-L303