On 02/03/2018 at 08:22:22 -0800, David Daney wrote: > I am no longer able to test this patch as I lost access to the hardware. > Ah, too bad. I will take your patch as-is and then add a small patch to fixup the nvmem registration. > If someone else wants to take charge of the patch, that would be great. > > Sorry I couldn't drive this to completion, > David Daney > > On Wed, Feb 28, 2018 at 7:17 AM, Alexandre Belloni < > alexandre.belloni@xxxxxxxxxxxxxxxxxx> wrote: > > > 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 > > -- 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
