Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTC: MAX31331: Support and documentation for RTC MAX31331 #2593

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

RTC: MAX31331: Support and documentation for RTC MAX31331 #2593

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d69eef
rtc: max31335: Add driver support for MAX31331
SwaroopPKADI Sep 6, 2024
6f413ca
dt-bindings: rtc: max31335: Add max31331 support
SwaroopPKADI Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 121 additions & 40 deletions drivers/rtc/rtc-max31335.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,31 +184,90 @@
#define MAX31335_RAM_SIZE 32
#define MAX31335_TIME_SIZE 0x07

/* MAX31331 Register Map */
#define MAX31331_RTC_CONFIG2 0x04

#define clk_hw_to_max31335(_hw) container_of(_hw, struct max31335_data, clkout)

/* Supported Maxim RTC */
enum max_rtc_ids {
ID_MAX31331,
ID_MAX31335,
MAX_RTC_ID_NR
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make the ids part of chip_desc

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The id is a part of max31335_data. chip_desc is being used for the register address mapping between multiple RTCs.

Would making it a part of chip_desc be more useful than keeping it out of it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doing the below?

max31335->id = max31335->chip - chip;

when the id is already a constant piece of information? Just make it part of that struct and be done with it :)


struct chip_desc {
u8 sec_reg;
u8 alarm1_sec_reg;

u8 int_en_reg;
u8 int_status_reg;

u8 ram_reg;
u8 ram_size;

u8 temp_reg;

u8 trickle_reg;

u8 clkout_reg;
};

struct max31335_data {
enum max_rtc_ids id;
struct regmap *regmap;
struct rtc_device *rtc;
struct clk_hw clkout;
struct clk *clkin;
const struct chip_desc *chip;
int irq;
};

static const int max31335_clkout_freq[] = { 1, 64, 1024, 32768 };

static const struct chip_desc chip[MAX_RTC_ID_NR] = {
[ID_MAX31331] = {
.int_en_reg = 0x01,
.int_status_reg = 0x00,
.sec_reg = 0x08,
.alarm1_sec_reg = 0x0F,
.ram_reg = 0x20,
.ram_size = 32,
.trickle_reg = 0x1B,
.clkout_reg = 0x04,
},
[ID_MAX31335] = {
.int_en_reg = 0x01,
.int_status_reg = 0x00,
.sec_reg = 0x0A,
.alarm1_sec_reg = 0x11,
.ram_reg = 0x40,
.ram_size = 32,
.temp_reg = 0x35,
.trickle_reg = 0x1D,
.clkout_reg = 0x06,
},
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just define two variables. The array is more cumbersome long term and it brings no added value...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few RTCs (MAX31334, MAX31328/29 etc) being developed. Keeping that in mind, I have put an array in place instead of 2 variables.


static const u16 max31335_trickle_resistors[] = {3000, 6000, 11000};

static bool max31335_volatile_reg(struct device *dev, unsigned int reg)
{
struct max31335_data *max31335 = dev_get_drvdata(dev);
const struct chip_desc *chip = max31335->chip;

/* time keeping registers */
if (reg >= MAX31335_SECONDS &&
reg < MAX31335_SECONDS + MAX31335_TIME_SIZE)
if (reg >= chip->sec_reg &&
reg < chip->sec_reg + MAX31335_TIME_SIZE)
return true;

/* interrupt status register */
if (reg == MAX31335_STATUS1)
if (reg == chip->int_status_reg)
return true;

/* temperature registers */
if (reg == MAX31335_TEMP_DATA_MSB || MAX31335_TEMP_DATA_LSB)
/* temperature registers if valid*/
if (chip->temp_reg &&
(reg == chip->temp_reg || reg == chip->temp_reg + 1))
return true;

return false;
Expand All @@ -227,7 +286,7 @@ static int max31335_read_time(struct device *dev, struct rtc_time *tm)
u8 date[7];
int ret;

ret = regmap_bulk_read(max31335->regmap, MAX31335_SECONDS, date,
ret = regmap_bulk_read(max31335->regmap, max31335->chip->sec_reg, date,
sizeof(date));
if (ret)
return ret;
Expand Down Expand Up @@ -262,7 +321,7 @@ static int max31335_set_time(struct device *dev, struct rtc_time *tm)
if (tm->tm_year >= 200)
date[5] |= FIELD_PREP(MAX31335_MONTH_CENTURY, 1);

return regmap_bulk_write(max31335->regmap, MAX31335_SECONDS, date,
return regmap_bulk_write(max31335->regmap, max31335->chip->sec_reg, date,
sizeof(date));
}

Expand All @@ -273,7 +332,7 @@ static int max31335_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
struct rtc_time time;
u8 regs[6];

ret = regmap_bulk_read(max31335->regmap, MAX31335_ALM1_SEC, regs,
ret = regmap_bulk_read(max31335->regmap, max31335->chip->alarm1_sec_reg, regs,
sizeof(regs));
if (ret)
return ret;
Expand All @@ -292,11 +351,11 @@ static int max31335_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
if (time.tm_year >= 200)
alrm->time.tm_year += 100;

ret = regmap_read(max31335->regmap, MAX31335_INT_EN1, &ctrl);
ret = regmap_read(max31335->regmap, max31335->chip->int_en_reg, &ctrl);
if (ret)
return ret;

ret = regmap_read(max31335->regmap, MAX31335_STATUS1, &status);
ret = regmap_read(max31335->regmap, max31335->chip->int_status_reg, &status);
if (ret)
return ret;

Expand All @@ -320,18 +379,18 @@ static int max31335_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
regs[4] = bin2bcd(alrm->time.tm_mon + 1);
regs[5] = bin2bcd(alrm->time.tm_year % 100);

ret = regmap_bulk_write(max31335->regmap, MAX31335_ALM1_SEC,
ret = regmap_bulk_write(max31335->regmap, max31335->chip->alarm1_sec_reg,
regs, sizeof(regs));
if (ret)
return ret;

reg = FIELD_PREP(MAX31335_INT_EN1_A1IE, alrm->enabled);
ret = regmap_update_bits(max31335->regmap, MAX31335_INT_EN1,
ret = regmap_update_bits(max31335->regmap, max31335->chip->int_en_reg,
MAX31335_INT_EN1_A1IE, reg);
if (ret)
return ret;

ret = regmap_update_bits(max31335->regmap, MAX31335_STATUS1,
ret = regmap_update_bits(max31335->regmap, max31335->chip->int_status_reg,
MAX31335_STATUS1_A1F, 0);

return 0;
Expand All @@ -341,7 +400,7 @@ static int max31335_alarm_irq_enable(struct device *dev, unsigned int enabled)
{
struct max31335_data *max31335 = dev_get_drvdata(dev);

return regmap_update_bits(max31335->regmap, MAX31335_INT_EN1,
return regmap_update_bits(max31335->regmap, max31335->chip->int_en_reg,
MAX31335_INT_EN1_A1IE, enabled);
}

Expand All @@ -353,12 +412,12 @@ static irqreturn_t max31335_handle_irq(int irq, void *dev_id)

mutex_lock(lock);

ret = regmap_read(max31335->regmap, MAX31335_STATUS1, &status);
ret = regmap_read(max31335->regmap, max31335->chip->int_status_reg, &status);
if (ret)
goto exit;

if (FIELD_GET(MAX31335_STATUS1_A1F, status)) {
ret = regmap_update_bits(max31335->regmap, MAX31335_STATUS1,
ret = regmap_update_bits(max31335->regmap, max31335->chip->int_status_reg,
MAX31335_STATUS1_A1F, 0);
if (ret)
goto exit;
Expand Down Expand Up @@ -414,7 +473,7 @@ static int max31335_trickle_charger_setup(struct device *dev,

i = i + trickle_cfg;

return regmap_write(max31335->regmap, MAX31335_TRICKLE_REG,
return regmap_write(max31335->regmap, max31335->chip->trickle_reg,
FIELD_PREP(MAX31335_TRICKLE_REG_TRICKLE, i) |
FIELD_PREP(MAX31335_TRICKLE_REG_EN_TRICKLE,
chargeable));
Expand All @@ -428,7 +487,7 @@ static unsigned long max31335_clkout_recalc_rate(struct clk_hw *hw,
unsigned int reg;
int ret;

ret = regmap_read(max31335->regmap, MAX31335_RTC_CONFIG2, &reg);
ret = regmap_read(max31335->regmap, max31335->chip->clkout_reg, &reg);
if (ret)
return 0;

Expand All @@ -454,29 +513,32 @@ static int max31335_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
struct max31335_data *max31335 = clk_hw_to_max31335(hw);
unsigned int freq_mask;
int index;
int ret;

index = find_closest(rate, max31335_clkout_freq,
ARRAY_SIZE(max31335_clkout_freq));
freq_mask = __roundup_pow_of_two(ARRAY_SIZE(max31335_clkout_freq)) - 1;

return regmap_update_bits(max31335->regmap, MAX31335_RTC_CONFIG2,
freq_mask, index);
ret = regmap_update_bits(max31335->regmap, max31335->chip->clkout_reg,
freq_mask, index);

return ret;
}

static int max31335_clkout_enable(struct clk_hw *hw)
{
struct max31335_data *max31335 = clk_hw_to_max31335(hw);

return regmap_set_bits(max31335->regmap, MAX31335_RTC_CONFIG2,
MAX31335_RTC_CONFIG2_ENCLKO);
return regmap_set_bits(max31335->regmap, max31335->chip->clkout_reg,
MAX31335_RTC_CONFIG2_ENCLKO);
}

static void max31335_clkout_disable(struct clk_hw *hw)
{
struct max31335_data *max31335 = clk_hw_to_max31335(hw);

regmap_clear_bits(max31335->regmap, MAX31335_RTC_CONFIG2,
MAX31335_RTC_CONFIG2_ENCLKO);
regmap_clear_bits(max31335->regmap, max31335->chip->clkout_reg,
MAX31335_RTC_CONFIG2_ENCLKO);
}

static int max31335_clkout_is_enabled(struct clk_hw *hw)
Expand All @@ -485,7 +547,7 @@ static int max31335_clkout_is_enabled(struct clk_hw *hw)
unsigned int reg;
int ret;

ret = regmap_read(max31335->regmap, MAX31335_RTC_CONFIG2, &reg);
ret = regmap_read(max31335->regmap, max31335->chip->clkout_reg, &reg);
if (ret)
return ret;

Expand All @@ -510,7 +572,7 @@ static int max31335_nvmem_reg_read(void *priv, unsigned int offset,
void *val, size_t bytes)
{
struct max31335_data *max31335 = priv;
unsigned int reg = MAX31335_TS0_SEC_1_128 + offset;
unsigned int reg = max31335->chip->ram_reg + offset;

return regmap_bulk_read(max31335->regmap, reg, val, bytes);
}
Expand All @@ -519,7 +581,7 @@ static int max31335_nvmem_reg_write(void *priv, unsigned int offset,
void *val, size_t bytes)
{
struct max31335_data *max31335 = priv;
unsigned int reg = MAX31335_TS0_SEC_1_128 + offset;
unsigned int reg = max31335->chip->ram_reg + offset;

return regmap_bulk_write(max31335->regmap, reg, val, bytes);
}
Expand All @@ -543,7 +605,7 @@ static int max31335_read_temp(struct device *dev, enum hwmon_sensor_types type,
if (type != hwmon_temp || attr != hwmon_temp_input)
return -EOPNOTSUPP;

ret = regmap_bulk_read(max31335->regmap, MAX31335_TEMP_DATA_MSB,
ret = regmap_bulk_read(max31335->regmap, max31335->chip->temp_reg,
reg, 2);
if (ret)
return ret;
Expand Down Expand Up @@ -586,9 +648,10 @@ static int max31335_clkout_register(struct device *dev)
struct max31335_data *max31335 = dev_get_drvdata(dev);
int ret;

if (!device_property_present(dev, "#clock-cells"))
return regmap_clear_bits(max31335->regmap, MAX31335_RTC_CONFIG2,
MAX31335_RTC_CONFIG2_ENCLKO);
if (!device_property_present(dev, "#clock-cells")) {
regmap_clear_bits(max31335->regmap, max31335->chip->clkout_reg,
MAX31335_RTC_CONFIG2_ENCLKO);
}

max31335->clkout.init = &max31335_clk_init;

Expand Down Expand Up @@ -617,6 +680,7 @@ static int max31335_probe(struct i2c_client *client,
#if IS_REACHABLE(HWMON)
struct device *hwmon;
#endif
const void *match;
int ret;

max31335 = devm_kzalloc(&client->dev, sizeof(*max31335), GFP_KERNEL);
Expand All @@ -629,6 +693,16 @@ static int max31335_probe(struct i2c_client *client,

i2c_set_clientdata(client, max31335);

match = device_get_match_data(&client->dev);
if (match)
max31335->chip = match;
else if (id)
max31335->chip = (struct chip_desc *)id->driver_data;
else
return -ENODEV;

max31335->id = max31335->chip - chip;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use i2c_get_match_data() and check for NULL (return error in that case)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't find a definition for i2c_get_match_data()

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be it's yet not supported in our tree. But it is upstream and that what you should use when upstreaming this driver

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still nok IMO. But if you really think it's ok like, I won't argue more :)


max31335->rtc = devm_rtc_allocate_device(&client->dev);
if (IS_ERR(max31335->rtc))
return PTR_ERR(max31335->rtc);
Expand All @@ -651,6 +725,8 @@ static int max31335_probe(struct i2c_client *client,
dev_warn(&client->dev,
"unable to request IRQ, alarm max31335 disabled\n");
client->irq = 0;
} else {
max31335->irq = client->irq;
}
}

Expand All @@ -664,13 +740,15 @@ static int max31335_probe(struct i2c_client *client,
"cannot register rtc nvmem\n");

#if IS_REACHABLE(HWMON)
hwmon = devm_hwmon_device_register_with_info(&client->dev, client->name,
max31335,
&max31335_chip_info,
NULL);
if (IS_ERR(hwmon))
return dev_err_probe(&client->dev, PTR_ERR(hwmon),
"cannot register hwmon device\n");
if (max313xx->chip->temp_reg) {
hwmon = devm_hwmon_device_register_with_info(&client->dev, client->name,
max31335,
&max31335_chip_info,
NULL);
if (IS_ERR(hwmon))
return dev_err_probe(&client->dev, PTR_ERR(hwmon),
"cannot register hwmon device\n");
}
#endif

ret = max31335_trickle_charger_setup(&client->dev, max31335);
Expand All @@ -681,14 +759,16 @@ static int max31335_probe(struct i2c_client *client,
}

static const struct i2c_device_id max31335_id[] = {
{ "max31335", 0 },
{ "max31331", (kernel_ulong_t)&chip[ID_MAX31331] },
{ "max31335", (kernel_ulong_t)&chip[ID_MAX31335] },
{ }
};

MODULE_DEVICE_TABLE(i2c, max31335_id);

static const struct of_device_id max31335_of_match[] = {
{ .compatible = "adi,max31335" },
{ .compatible = "adi,max31331", .data = &chip[ID_MAX31331] },
{ .compatible = "adi,max31335", .data = &chip[ID_MAX31335] },
{ }
};

Expand All @@ -705,5 +785,6 @@ static struct i2c_driver max31335_driver = {
module_i2c_driver(max31335_driver);

MODULE_AUTHOR("Antoniu Miclaus <[email protected]>");
MODULE_AUTHOR("Saket Kumar Purwar <[email protected]>");
MODULE_DESCRIPTION("MAX31335 RTC driver");
MODULE_LICENSE("GPL");