Hi,
That's mostly good
On 22/02/2018 at 12:04:32 -0800, David Daney wrote:
> + priv->rtc->ops = &isl12026_rtc_ops;
> + priv->rtc->nvram_old_abi = false;
This allocation is not necessary and I would refer not having t so when
the ABI goes away, it is not necessary to change this driver.
> + ret = rtc_register_device(priv->rtc);
> + if (ret)
> + return ret;
> +
> + memset(&nvm_cfg, 0, sizeof(nvm_cfg));
> + nvm_cfg.name = "eeprom";
You probably need something more descriptive, usually the rtc model name
else, it is difficult (but not impossible) to find where this nvmem is
actually located when using /sys/bus/nvmem/devices
isl12026- would be a good choice.
> + nvm_cfg.read_only = false;
> + nvm_cfg.root_only = true;
any reason to have it root only?
> + nvm_cfg.base_dev = &client->dev;
> + nvm_cfg.priv = priv;
> + nvm_cfg.stride = 1;
> + nvm_cfg.word_size = 1;
> + nvm_cfg.size = 512;
> + nvm_cfg.reg_read = isl12026_nvm_read;
> + nvm_cfg.reg_write = isl12026_nvm_write;
> +
> + return rtc_nvmem_register(priv->rtc, &nvm_cfg);
The probe function must not fail after rtc_register_device has been
called so you must return 0 here.
It is not currently possible to call rtc_nvmem_register before
rtc_register_device unless we move the nvmem to the parent device:
diff --git a/drivers/rtc/nvmem.c b/drivers/rtc/nvmem.c
index 3a02357eb783..17ec4c8d0fad 100644
--- a/drivers/rtc/nvmem.c
+++ b/drivers/rtc/nvmem.c
@@ -91,7 +91,7 @@ int rtc_nvmem_register(struct rtc_device *rtc,
if (!nvmem_config)
return -ENODEV;
- nvmem_config->dev = &rtc->dev;
+ nvmem_config->dev = rtc->dev.parent;
nvmem_config->owner = rtc->owner;
rtc->nvmem = nvmem_register(nvmem_config);
if (IS_ERR_OR_NULL(rtc->nvmem))
I'll submit this patch for this cycle before there are many users of the
interface (converted drivers are still exposing the old ABI).
If you prefer, you can depend on it.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
