This adds support for the 256-byte internal RAM. There are two windows
which can be used to access this RAM: 64 bytes at 0x40 (the "standard"
address space) and 128 bytes at 0x80 (the "alternate" address space). We
use the standard address space because it is also accessible over SPI
(if such a port is ever done). We are limited to 32-byte reads for SMBus
compatibility, so there's no advantage to using the alternate address
space.
There are some reserved bits in the EXTRAM register, and the datasheet
doesn't say what to do with them. I've opted to skip a read/modify/write
and just write the whole thing. If this driver is ever converted to
regmap, this would be a good place to use regmap_update_bits.
Signed-off-by: Sean Anderson <[email protected]>
---
Changes in v2:
- Fix building on non-arm platforms
drivers/rtc/rtc-abx80x.c | 87 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 87 insertions(+)
diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
index 9b0138d07232..e606bf126dc3 100644
--- a/drivers/rtc/rtc-abx80x.c
+++ b/drivers/rtc/rtc-abx80x.c
@@ -11,6 +11,7 @@
*/
#include <linux/bcd.h>
+#include <linux/bitfield.h>
#include <linux/i2c.h>
#include <linux/module.h>
#include <linux/of_device.h>
@@ -87,6 +88,16 @@
#define ABX8XX_TRICKLE_STANDARD_DIODE 0x8
#define ABX8XX_TRICKLE_SCHOTTKY_DIODE 0x4
+#define ABX8XX_REG_EXTRAM 0x3f
+#define ABX8XX_EXTRAM_XADS GENMASK(1, 0)
+
+#define ABX8XX_SRAM_BASE 0x40
+#define ABX8XX_SRAM_WIN_SIZE 0x40
+#define ABX8XX_RAM_SIZE 256
+
+#define NVMEM_ADDR_LOWER GENMASK(5, 0)
+#define NVMEM_ADDR_UPPER GENMASK(7, 6)
+
static u8 trickle_resistors[] = {0, 3, 6, 11};
enum abx80x_chip {AB0801, AB0803, AB0804, AB0805,
@@ -673,6 +684,78 @@ static int abx80x_setup_watchdog(struct abx80x_priv *priv)
}
#endif
+#ifdef CONFIG_NVMEM
+static int abx80x_nvmem_xfer(struct abx80x_priv *priv, unsigned int offset,
+ void *val, size_t bytes, bool write)
+{
+ int ret;
+
+ while (bytes) {
+ u8 extram, reg, len, lower, upper;
+
+ lower = FIELD_GET(NVMEM_ADDR_LOWER, offset);
+ upper = FIELD_GET(NVMEM_ADDR_UPPER, offset);
+ extram = FIELD_PREP(ABX8XX_EXTRAM_XADS, upper);
+ reg = ABX8XX_SRAM_BASE + lower;
+ len = min(lower + bytes, (size_t)ABX8XX_SRAM_WIN_SIZE) - lower;
+ len = min_t(u8, len, I2C_SMBUS_BLOCK_MAX);
+
+ ret = i2c_smbus_write_byte_data(priv->client, ABX8XX_REG_EXTRAM,
+ extram);
+ if (ret)
+ return ret;
+
+ if (write)
+ ret = i2c_smbus_write_i2c_block_data(priv->client, reg,
+ len, val);
+ else
+ ret = i2c_smbus_read_i2c_block_data(priv->client, reg,
+ len, val);
+ if (ret)
+ return ret;
+
+ offset += len;
+ val += len;
+ bytes -= len;
+ }
+
+ return 0;
+}
+
+static int abx80x_nvmem_read(void *priv, unsigned int offset, void *val,
+ size_t bytes)
+{
+ return abx80x_nvmem_xfer(priv, offset, val, bytes, false);
+}
+
+static int abx80x_nvmem_write(void *priv, unsigned int offset, void *val,
+ size_t bytes)
+{
+ return abx80x_nvmem_xfer(priv, offset, val, bytes, true);
+}
+
+static int abx80x_setup_nvmem(struct abx80x_priv *priv)
+{
+ struct device *dev = &priv->client->dev;
+ struct nvmem_config config = {
+ .dev = dev,
+ .type = NVMEM_TYPE_BATTERY_BACKED,
+ .reg_read = abx80x_nvmem_read,
+ .reg_write = abx80x_nvmem_write,
+ .size = ABX8XX_RAM_SIZE,
+ .priv = priv,
+ };
+
+ return PTR_ERR_OR_ZERO(devm_nvmem_register(&priv->client->dev,
+ &config));
+}
+#else
+static int abx80x_setup_nvmem(struct abx80x_priv *priv)
+{
+ return 0;
+}
+#endif
+
static int abx80x_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -824,6 +907,10 @@ static int abx80x_probe(struct i2c_client *client,
return err;
}
+ err = abx80x_setup_nvmem(priv);
+ if (err)
+ return err;
+
if (client->irq > 0) {
dev_info(&client->dev, "IRQ %d supplied\n", client->irq);
err = devm_request_threaded_irq(&client->dev, client->irq, NULL,
--
2.35.1.1320.gc452695387.dirty
On 12/13/22 10:18, Alexandre Belloni wrote:
> On 05/12/2022 10:19:18-0500, Sean Anderson wrote:
>> This adds support for the 256-byte internal RAM. There are two windows
>> which can be used to access this RAM: 64 bytes at 0x40 (the "standard"
>> address space) and 128 bytes at 0x80 (the "alternate" address space). We
>> use the standard address space because it is also accessible over SPI
>> (if such a port is ever done). We are limited to 32-byte reads for SMBus
>> compatibility, so there's no advantage to using the alternate address
>> space.
>>
>> There are some reserved bits in the EXTRAM register, and the datasheet
>> doesn't say what to do with them. I've opted to skip a read/modify/write
>> and just write the whole thing. If this driver is ever converted to
>> regmap, this would be a good place to use regmap_update_bits.
>>
>> Signed-off-by: Sean Anderson <[email protected]>
>> ---
>>
>> Changes in v2:
>> - Fix building on non-arm platforms
>>
>> drivers/rtc/rtc-abx80x.c | 87 ++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 87 insertions(+)
>>
>> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
>> index 9b0138d07232..e606bf126dc3 100644
>> --- a/drivers/rtc/rtc-abx80x.c
>> +++ b/drivers/rtc/rtc-abx80x.c
>> @@ -11,6 +11,7 @@
>> */
>>
>> #include <linux/bcd.h>
>> +#include <linux/bitfield.h>
>> #include <linux/i2c.h>
>> #include <linux/module.h>
>> #include <linux/of_device.h>
>> @@ -87,6 +88,16 @@
>> #define ABX8XX_TRICKLE_STANDARD_DIODE 0x8
>> #define ABX8XX_TRICKLE_SCHOTTKY_DIODE 0x4
>>
>> +#define ABX8XX_REG_EXTRAM 0x3f
>> +#define ABX8XX_EXTRAM_XADS GENMASK(1, 0)
>> +
>> +#define ABX8XX_SRAM_BASE 0x40
>> +#define ABX8XX_SRAM_WIN_SIZE 0x40
>> +#define ABX8XX_RAM_SIZE 256
>> +
>> +#define NVMEM_ADDR_LOWER GENMASK(5, 0)
>> +#define NVMEM_ADDR_UPPER GENMASK(7, 6)
>> +
>> static u8 trickle_resistors[] = {0, 3, 6, 11};
>>
>> enum abx80x_chip {AB0801, AB0803, AB0804, AB0805,
>> @@ -673,6 +684,78 @@ static int abx80x_setup_watchdog(struct abx80x_priv *priv)
>> }
>> #endif
>>
>> +#ifdef CONFIG_NVMEM
>> +static int abx80x_nvmem_xfer(struct abx80x_priv *priv, unsigned int offset,
>> + void *val, size_t bytes, bool write)
>> +{
>> + int ret;
>> +
>> + while (bytes) {
>> + u8 extram, reg, len, lower, upper;
>> +
>> + lower = FIELD_GET(NVMEM_ADDR_LOWER, offset);
>> + upper = FIELD_GET(NVMEM_ADDR_UPPER, offset);
>> + extram = FIELD_PREP(ABX8XX_EXTRAM_XADS, upper);
>> + reg = ABX8XX_SRAM_BASE + lower;
>> + len = min(lower + bytes, (size_t)ABX8XX_SRAM_WIN_SIZE) - lower;
>> + len = min_t(u8, len, I2C_SMBUS_BLOCK_MAX);
>> +
>> + ret = i2c_smbus_write_byte_data(priv->client, ABX8XX_REG_EXTRAM,
>> + extram);
>> + if (ret)
>> + return ret;
>> +
>> + if (write)
>> + ret = i2c_smbus_write_i2c_block_data(priv->client, reg,
>> + len, val);
>> + else
>> + ret = i2c_smbus_read_i2c_block_data(priv->client, reg,
>> + len, val);
>> + if (ret)
>> + return ret;
>> +
>> + offset += len;
>> + val += len;
>> + bytes -= len;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int abx80x_nvmem_read(void *priv, unsigned int offset, void *val,
>> + size_t bytes)
>> +{
>> + return abx80x_nvmem_xfer(priv, offset, val, bytes, false);
>> +}
>> +
>> +static int abx80x_nvmem_write(void *priv, unsigned int offset, void *val,
>> + size_t bytes)
>> +{
>> + return abx80x_nvmem_xfer(priv, offset, val, bytes, true);
>> +}
>> +
>> +static int abx80x_setup_nvmem(struct abx80x_priv *priv)
>> +{
>> + struct device *dev = &priv->client->dev;
>> + struct nvmem_config config = {
>> + .dev = dev,
>> + .type = NVMEM_TYPE_BATTERY_BACKED,
>> + .reg_read = abx80x_nvmem_read,
>> + .reg_write = abx80x_nvmem_write,
>> + .size = ABX8XX_RAM_SIZE,
>> + .priv = priv,
>> + };
>> +
>> + return PTR_ERR_OR_ZERO(devm_nvmem_register(&priv->client->dev,
>> + &config));
>
> Is there any reason why you are not using devm_rtc_nvmem_register ?
>
>
I didn't know it existed. To be honest, it doesn't seem to be doing much.
With that changed, is the rest of the patch OK?
--Sean
On 05/12/2022 10:19:18-0500, Sean Anderson wrote:
> This adds support for the 256-byte internal RAM. There are two windows
> which can be used to access this RAM: 64 bytes at 0x40 (the "standard"
> address space) and 128 bytes at 0x80 (the "alternate" address space). We
> use the standard address space because it is also accessible over SPI
> (if such a port is ever done). We are limited to 32-byte reads for SMBus
> compatibility, so there's no advantage to using the alternate address
> space.
>
> There are some reserved bits in the EXTRAM register, and the datasheet
> doesn't say what to do with them. I've opted to skip a read/modify/write
> and just write the whole thing. If this driver is ever converted to
> regmap, this would be a good place to use regmap_update_bits.
>
> Signed-off-by: Sean Anderson <[email protected]>
> ---
>
> Changes in v2:
> - Fix building on non-arm platforms
>
> drivers/rtc/rtc-abx80x.c | 87 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 87 insertions(+)
>
> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> index 9b0138d07232..e606bf126dc3 100644
> --- a/drivers/rtc/rtc-abx80x.c
> +++ b/drivers/rtc/rtc-abx80x.c
> @@ -11,6 +11,7 @@
> */
>
> #include <linux/bcd.h>
> +#include <linux/bitfield.h>
> #include <linux/i2c.h>
> #include <linux/module.h>
> #include <linux/of_device.h>
> @@ -87,6 +88,16 @@
> #define ABX8XX_TRICKLE_STANDARD_DIODE 0x8
> #define ABX8XX_TRICKLE_SCHOTTKY_DIODE 0x4
>
> +#define ABX8XX_REG_EXTRAM 0x3f
> +#define ABX8XX_EXTRAM_XADS GENMASK(1, 0)
> +
> +#define ABX8XX_SRAM_BASE 0x40
> +#define ABX8XX_SRAM_WIN_SIZE 0x40
> +#define ABX8XX_RAM_SIZE 256
> +
> +#define NVMEM_ADDR_LOWER GENMASK(5, 0)
> +#define NVMEM_ADDR_UPPER GENMASK(7, 6)
> +
> static u8 trickle_resistors[] = {0, 3, 6, 11};
>
> enum abx80x_chip {AB0801, AB0803, AB0804, AB0805,
> @@ -673,6 +684,78 @@ static int abx80x_setup_watchdog(struct abx80x_priv *priv)
> }
> #endif
>
> +#ifdef CONFIG_NVMEM
> +static int abx80x_nvmem_xfer(struct abx80x_priv *priv, unsigned int offset,
> + void *val, size_t bytes, bool write)
> +{
> + int ret;
> +
> + while (bytes) {
> + u8 extram, reg, len, lower, upper;
> +
> + lower = FIELD_GET(NVMEM_ADDR_LOWER, offset);
> + upper = FIELD_GET(NVMEM_ADDR_UPPER, offset);
> + extram = FIELD_PREP(ABX8XX_EXTRAM_XADS, upper);
> + reg = ABX8XX_SRAM_BASE + lower;
> + len = min(lower + bytes, (size_t)ABX8XX_SRAM_WIN_SIZE) - lower;
> + len = min_t(u8, len, I2C_SMBUS_BLOCK_MAX);
> +
> + ret = i2c_smbus_write_byte_data(priv->client, ABX8XX_REG_EXTRAM,
> + extram);
> + if (ret)
> + return ret;
> +
> + if (write)
> + ret = i2c_smbus_write_i2c_block_data(priv->client, reg,
> + len, val);
> + else
> + ret = i2c_smbus_read_i2c_block_data(priv->client, reg,
> + len, val);
> + if (ret)
> + return ret;
> +
> + offset += len;
> + val += len;
> + bytes -= len;
> + }
> +
> + return 0;
> +}
> +
> +static int abx80x_nvmem_read(void *priv, unsigned int offset, void *val,
> + size_t bytes)
> +{
> + return abx80x_nvmem_xfer(priv, offset, val, bytes, false);
> +}
> +
> +static int abx80x_nvmem_write(void *priv, unsigned int offset, void *val,
> + size_t bytes)
> +{
> + return abx80x_nvmem_xfer(priv, offset, val, bytes, true);
> +}
> +
> +static int abx80x_setup_nvmem(struct abx80x_priv *priv)
> +{
> + struct device *dev = &priv->client->dev;
> + struct nvmem_config config = {
> + .dev = dev,
> + .type = NVMEM_TYPE_BATTERY_BACKED,
> + .reg_read = abx80x_nvmem_read,
> + .reg_write = abx80x_nvmem_write,
> + .size = ABX8XX_RAM_SIZE,
> + .priv = priv,
> + };
> +
> + return PTR_ERR_OR_ZERO(devm_nvmem_register(&priv->client->dev,
> + &config));
Is there any reason why you are not using devm_rtc_nvmem_register ?
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On 13/12/2022 10:22:25-0500, Sean Anderson wrote:
> On 12/13/22 10:18, Alexandre Belloni wrote:
> > On 05/12/2022 10:19:18-0500, Sean Anderson wrote:
> >> This adds support for the 256-byte internal RAM. There are two windows
> >> which can be used to access this RAM: 64 bytes at 0x40 (the "standard"
> >> address space) and 128 bytes at 0x80 (the "alternate" address space). We
> >> use the standard address space because it is also accessible over SPI
> >> (if such a port is ever done). We are limited to 32-byte reads for SMBus
> >> compatibility, so there's no advantage to using the alternate address
> >> space.
> >>
> >> There are some reserved bits in the EXTRAM register, and the datasheet
> >> doesn't say what to do with them. I've opted to skip a read/modify/write
> >> and just write the whole thing. If this driver is ever converted to
> >> regmap, this would be a good place to use regmap_update_bits.
> >>
> >> Signed-off-by: Sean Anderson <[email protected]>
> >> ---
> >>
> >> Changes in v2:
> >> - Fix building on non-arm platforms
> >>
> >> drivers/rtc/rtc-abx80x.c | 87 ++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 87 insertions(+)
> >>
> >> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> >> index 9b0138d07232..e606bf126dc3 100644
> >> --- a/drivers/rtc/rtc-abx80x.c
> >> +++ b/drivers/rtc/rtc-abx80x.c
> >> @@ -11,6 +11,7 @@
> >> */
> >>
> >> #include <linux/bcd.h>
> >> +#include <linux/bitfield.h>
> >> #include <linux/i2c.h>
> >> #include <linux/module.h>
> >> #include <linux/of_device.h>
> >> @@ -87,6 +88,16 @@
> >> #define ABX8XX_TRICKLE_STANDARD_DIODE 0x8
> >> #define ABX8XX_TRICKLE_SCHOTTKY_DIODE 0x4
> >>
> >> +#define ABX8XX_REG_EXTRAM 0x3f
> >> +#define ABX8XX_EXTRAM_XADS GENMASK(1, 0)
> >> +
> >> +#define ABX8XX_SRAM_BASE 0x40
> >> +#define ABX8XX_SRAM_WIN_SIZE 0x40
> >> +#define ABX8XX_RAM_SIZE 256
> >> +
> >> +#define NVMEM_ADDR_LOWER GENMASK(5, 0)
> >> +#define NVMEM_ADDR_UPPER GENMASK(7, 6)
> >> +
> >> static u8 trickle_resistors[] = {0, 3, 6, 11};
> >>
> >> enum abx80x_chip {AB0801, AB0803, AB0804, AB0805,
> >> @@ -673,6 +684,78 @@ static int abx80x_setup_watchdog(struct abx80x_priv *priv)
> >> }
> >> #endif
> >>
> >> +#ifdef CONFIG_NVMEM
> >> +static int abx80x_nvmem_xfer(struct abx80x_priv *priv, unsigned int offset,
> >> + void *val, size_t bytes, bool write)
> >> +{
> >> + int ret;
> >> +
> >> + while (bytes) {
> >> + u8 extram, reg, len, lower, upper;
> >> +
> >> + lower = FIELD_GET(NVMEM_ADDR_LOWER, offset);
> >> + upper = FIELD_GET(NVMEM_ADDR_UPPER, offset);
> >> + extram = FIELD_PREP(ABX8XX_EXTRAM_XADS, upper);
> >> + reg = ABX8XX_SRAM_BASE + lower;
> >> + len = min(lower + bytes, (size_t)ABX8XX_SRAM_WIN_SIZE) - lower;
> >> + len = min_t(u8, len, I2C_SMBUS_BLOCK_MAX);
> >> +
> >> + ret = i2c_smbus_write_byte_data(priv->client, ABX8XX_REG_EXTRAM,
> >> + extram);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (write)
> >> + ret = i2c_smbus_write_i2c_block_data(priv->client, reg,
> >> + len, val);
> >> + else
> >> + ret = i2c_smbus_read_i2c_block_data(priv->client, reg,
> >> + len, val);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + offset += len;
> >> + val += len;
> >> + bytes -= len;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int abx80x_nvmem_read(void *priv, unsigned int offset, void *val,
> >> + size_t bytes)
> >> +{
> >> + return abx80x_nvmem_xfer(priv, offset, val, bytes, false);
> >> +}
> >> +
> >> +static int abx80x_nvmem_write(void *priv, unsigned int offset, void *val,
> >> + size_t bytes)
> >> +{
> >> + return abx80x_nvmem_xfer(priv, offset, val, bytes, true);
> >> +}
> >> +
> >> +static int abx80x_setup_nvmem(struct abx80x_priv *priv)
> >> +{
> >> + struct device *dev = &priv->client->dev;
> >> + struct nvmem_config config = {
> >> + .dev = dev,
> >> + .type = NVMEM_TYPE_BATTERY_BACKED,
> >> + .reg_read = abx80x_nvmem_read,
> >> + .reg_write = abx80x_nvmem_write,
> >> + .size = ABX8XX_RAM_SIZE,
> >> + .priv = priv,
> >> + };
> >> +
> >> + return PTR_ERR_OR_ZERO(devm_nvmem_register(&priv->client->dev,
> >> + &config));
> >
> > Is there any reason why you are not using devm_rtc_nvmem_register ?
> >
> >
>
> I didn't know it existed. To be honest, it doesn't seem to be doing much.
It properly sets the owner which is going to save you from race
conditions and headaches.
It also removes the need for the ifdefery.
> With that changed, is the rest of the patch OK?
>
find the use of FIELD_* a bit too much but this is fine, yes.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On 12/13/22 10:42, Alexandre Belloni wrote:
> On 13/12/2022 10:22:25-0500, Sean Anderson wrote:
>> On 12/13/22 10:18, Alexandre Belloni wrote:
>> > On 05/12/2022 10:19:18-0500, Sean Anderson wrote:
>> >> This adds support for the 256-byte internal RAM. There are two windows
>> >> which can be used to access this RAM: 64 bytes at 0x40 (the "standard"
>> >> address space) and 128 bytes at 0x80 (the "alternate" address space). We
>> >> use the standard address space because it is also accessible over SPI
>> >> (if such a port is ever done). We are limited to 32-byte reads for SMBus
>> >> compatibility, so there's no advantage to using the alternate address
>> >> space.
>> >>
>> >> There are some reserved bits in the EXTRAM register, and the datasheet
>> >> doesn't say what to do with them. I've opted to skip a read/modify/write
>> >> and just write the whole thing. If this driver is ever converted to
>> >> regmap, this would be a good place to use regmap_update_bits.
>> >>
>> >> Signed-off-by: Sean Anderson <[email protected]>
>> >> ---
>> >>
>> >> Changes in v2:
>> >> - Fix building on non-arm platforms
>> >>
>> >> drivers/rtc/rtc-abx80x.c | 87 ++++++++++++++++++++++++++++++++++++++++
>> >> 1 file changed, 87 insertions(+)
>> >>
>> >> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
>> >> index 9b0138d07232..e606bf126dc3 100644
>> >> --- a/drivers/rtc/rtc-abx80x.c
>> >> +++ b/drivers/rtc/rtc-abx80x.c
>> >> @@ -11,6 +11,7 @@
>> >> */
>> >>
>> >> #include <linux/bcd.h>
>> >> +#include <linux/bitfield.h>
>> >> #include <linux/i2c.h>
>> >> #include <linux/module.h>
>> >> #include <linux/of_device.h>
>> >> @@ -87,6 +88,16 @@
>> >> #define ABX8XX_TRICKLE_STANDARD_DIODE 0x8
>> >> #define ABX8XX_TRICKLE_SCHOTTKY_DIODE 0x4
>> >>
>> >> +#define ABX8XX_REG_EXTRAM 0x3f
>> >> +#define ABX8XX_EXTRAM_XADS GENMASK(1, 0)
>> >> +
>> >> +#define ABX8XX_SRAM_BASE 0x40
>> >> +#define ABX8XX_SRAM_WIN_SIZE 0x40
>> >> +#define ABX8XX_RAM_SIZE 256
>> >> +
>> >> +#define NVMEM_ADDR_LOWER GENMASK(5, 0)
>> >> +#define NVMEM_ADDR_UPPER GENMASK(7, 6)
>> >> +
>> >> static u8 trickle_resistors[] = {0, 3, 6, 11};
>> >>
>> >> enum abx80x_chip {AB0801, AB0803, AB0804, AB0805,
>> >> @@ -673,6 +684,78 @@ static int abx80x_setup_watchdog(struct abx80x_priv *priv)
>> >> }
>> >> #endif
>> >>
>> >> +#ifdef CONFIG_NVMEM
>> >> +static int abx80x_nvmem_xfer(struct abx80x_priv *priv, unsigned int offset,
>> >> + void *val, size_t bytes, bool write)
>> >> +{
>> >> + int ret;
>> >> +
>> >> + while (bytes) {
>> >> + u8 extram, reg, len, lower, upper;
>> >> +
>> >> + lower = FIELD_GET(NVMEM_ADDR_LOWER, offset);
>> >> + upper = FIELD_GET(NVMEM_ADDR_UPPER, offset);
>> >> + extram = FIELD_PREP(ABX8XX_EXTRAM_XADS, upper);
>> >> + reg = ABX8XX_SRAM_BASE + lower;
>> >> + len = min(lower + bytes, (size_t)ABX8XX_SRAM_WIN_SIZE) - lower;
>> >> + len = min_t(u8, len, I2C_SMBUS_BLOCK_MAX);
>> >> +
>> >> + ret = i2c_smbus_write_byte_data(priv->client, ABX8XX_REG_EXTRAM,
>> >> + extram);
>> >> + if (ret)
>> >> + return ret;
>> >> +
>> >> + if (write)
>> >> + ret = i2c_smbus_write_i2c_block_data(priv->client, reg,
>> >> + len, val);
>> >> + else
>> >> + ret = i2c_smbus_read_i2c_block_data(priv->client, reg,
>> >> + len, val);
>> >> + if (ret)
>> >> + return ret;
>> >> +
>> >> + offset += len;
>> >> + val += len;
>> >> + bytes -= len;
>> >> + }
>> >> +
>> >> + return 0;
>> >> +}
>> >> +
>> >> +static int abx80x_nvmem_read(void *priv, unsigned int offset, void *val,
>> >> + size_t bytes)
>> >> +{
>> >> + return abx80x_nvmem_xfer(priv, offset, val, bytes, false);
>> >> +}
>> >> +
>> >> +static int abx80x_nvmem_write(void *priv, unsigned int offset, void *val,
>> >> + size_t bytes)
>> >> +{
>> >> + return abx80x_nvmem_xfer(priv, offset, val, bytes, true);
>> >> +}
>> >> +
>> >> +static int abx80x_setup_nvmem(struct abx80x_priv *priv)
>> >> +{
>> >> + struct device *dev = &priv->client->dev;
>> >> + struct nvmem_config config = {
>> >> + .dev = dev,
>> >> + .type = NVMEM_TYPE_BATTERY_BACKED,
>> >> + .reg_read = abx80x_nvmem_read,
>> >> + .reg_write = abx80x_nvmem_write,
>> >> + .size = ABX8XX_RAM_SIZE,
>> >> + .priv = priv,
>> >> + };
>> >> +
>> >> + return PTR_ERR_OR_ZERO(devm_nvmem_register(&priv->client->dev,
>> >> + &config));
>> >
>> > Is there any reason why you are not using devm_rtc_nvmem_register ?
>> >
>> >
>>
>> I didn't know it existed. To be honest, it doesn't seem to be doing much.
>
> It properly sets the owner which is going to save you from race
> conditions and headaches.
nvmem_register has
nvmem->owner = config->owner;
if (!nvmem->owner && config->dev->driver)
nvmem->owner = config->dev->driver->owner;
which is going to get set to THIS_MODULE because we set dev. This is the
same thing that rtc->owner gets set to, so there's no difference.
> It also removes the need for the ifdefery.
This is just following the style of the watchdog code, which is
similarly ifdef'd out.
--Sean
>> With that changed, is the rest of the patch OK?
>>
>
> find the use of FIELD_* a bit too much but this is fine, yes.
>