I agree, are you planning to send the fixes for these?OK.Yep, thats much better indeed!
(C) looks reasonable because nvmem_config is pretty small.
(sizeof(struct nvmem_config) = 104 byte on 64bit systems)
I think (B) should be fixed as soon as possible
because new drivers often copy existing drivers.
I agree, there is some level of redundancy here, we should be able to get rid of it, as you said by removing the read_only flag from config which can be derived from r/w callbacks.
(Q2) Is nvmem_config::read_only necessary?
If .reg_write() callback is set, it is probably writable.
If .reg_write() is missing, it must be read-only.
I have no idea when nvmem_config::read_only is useful...
You can mark particular instance of provider as read-only which could be
specific to board.
reg_write callbacks can be implemented by provider driver, but read-only
flag would give the flexibility at board level.
Hmm, I did not get it.
Please help me to be clearer.
For each instance, the driver passes a different
nvmem_config to nvmem_register().
The driver should be able to do
config.reg_write = NULL;
to specify this instance is read-only.
Do we really need to specify both?
config.reg_write = NULL;
config.read_only = true;
I know nvmem_register() understands DT property "read-only".Yep this should work too.
This DT property is definitely useful for the
board-level and/or instance-granule flexibility.
But, I do not find a good example where
nvmem_config.read_only provides additional value.
For example, drivers/misc/eeprom/eeprom_93xx46.c
conditionally sets the read_only flag, like this:
edev->nvmem_config.read_only = pd->flags & EE_READONLY;
If nvmem had not supported .read_only flag,
the driver would probably have done like this:
if (!(pd->flags & EE_READONLY))
edev->nvmem_config.reg_write = eeprom_93xx46_write;
This should be fine.
Not sure if this is some thing mandatory! but its good to have kinda thing.
I 100% agree that all nvmem modules should have "nvmem-" prefixThis is mainly done for consistent module naming.
(Q3) The style of drivers/nvmem/Makefile
This Makefile looks ugly to me.
All nvmem drivers are just single file modules.
Why are they renamed when modules are created?
For the name-space reason for modules,
prefix "nvmem-" makes sense to me.
It is true that adding "nvmem-" prefix is redundant while
they are located in drivers/nvmem/ directory,
but renaming in the Makefile is even more annoying to me.
Having said that, we may not want to churn this.
I prefer to have nvmem- prefix for nvmem modules.
My question was, why .c files do not have the same file name as
the module name?
The more straight-forward way would be:
-- Best Regards Masahiro Yamada