lkml.org 
[lkml]   [2019]   [Apr]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/1] rtc: ds3232: get SRAM access using NVMEM Framework
From
Date
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

\
 
 \ /
  Last update: 2019-04-08 10:51    [W:0.059 / U:6.584 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site