Messages in this thread | | | Subject | Re: [PATCH 1/1] rtc: ds3232: get SRAM access using NVMEM Framework | From | Nandor Han <> | Date | Mon, 8 Apr 2019 11:51:15 +0300 |
| |
On 4/6/19 12:44 AM, Alexandre Belloni wrote: > Hi, > > On 05/04/2019 11:14:35+0000, Han Nandor wrote:
<snip>
>> ` >> # hexdump -n 10 -C /sys/bus/nvmem/devices/ds3232_sram0/nvmem >> 00000000 74 65 73 74 69 6e 67 0a 00 00 |testing...| >> 0000000a >> ` >> > > Thanks for that nice description! >
Glad that you like it. :)
>> drivers/rtc/rtc-ds3232.c | 41 ++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 39 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c >> index 7184e5145f12..fe615aedaa9a 100644 >> --- a/drivers/rtc/rtc-ds3232.c >> +++ b/drivers/rtc/rtc-ds3232.c >> @@ -24,6 +24,8 @@ >> #include <linux/regmap.h> >> #include <linux/hwmon.h> >> >> +#include "rtc-core.h" >> + > > The drivers should not have to include that header, see > fd5cd21d995e67f87b3eb4adf938be85fe83ef4b (from 4.16). >
Ahh...right. I did the implementation on 4.14 and it was needed there. I will change the implementation but I'll not be able to fully test it.
<snip>
>> struct ds3232 { >> struct device *dev; >> struct regmap *regmap; >> int irq; >> struct rtc_device *rtc; >> + struct nvmem_config nvmem_cfg; >> > > You don't actually need to keep that structure for the whole life of the > device as it is copied as soon as it is registered. I usually prefer to > have it on the stack. >
Very true. Done
<snip>
>> + ds3232->nvmem_cfg.stride = 1; >> + ds3232->nvmem_cfg.size = DS3232_REG_SRAM_SIZE; >> + ds3232->nvmem_cfg.word_size = 1; >> + ds3232->nvmem_cfg.reg_read = ds3232_nvmem_read; >> + ds3232->nvmem_cfg.reg_write = ds3232_nvmem_write; >> + ds3232->nvmem_cfg.priv = ds3232; > > Please also set the type (battery backed). >
Done.
<snip>
>> >> + ds3232->rtc->dev.of_node = dev->of_node; > > Please don't mess with rtc->dev. >
In my use-case I do use the device tree to declare a nvmem cell in this RTC node, which is used by another driver. Without this change finding the cell (using `devm_nvmem_cell_get` function) was failing because of missing `of_node` (again this was on 4.14).
I can do a bit of digging to see if something was changed, but I will really appreciate your advice, because I'm not be able to test the fix on linux-next.
>> + ds3232->rtc->nvmem_config = &ds3232->nvmem_cfg; >> + rtc_nvmem_register(ds3232->rtc); > > This part will not compile on v5.1-rc1. >
Sorry. I will fix this, and I will at least see that it builds on linux-next.
Thanks for review, Nandor
| |