Hi Guenter, On Thu, Jul 13, 2017 at 08:57:52PM -0700, Guenter Roeck wrote: > On 07/13/2017 12:54 PM, Moritz Fischer wrote: > > From: Moritz Fischer <moritz.fischer@xxxxxxxxx> > > > > Add support for the Maxim/Dallas DS1374 RTC/WDT with trickle charger. > > The device can either be configured as simple RTC, as simple RTC with > > Alarm (IRQ) as well as simple RTC with watchdog timer. > > > > Break up the old monolithic driver in drivers/rtc/rtc-ds1374.c into: > > - rtc part in drivers/rtc/rtc-ds1374.c > > - watchdog part under drivers/watchdog/ds1374-wdt.c > > - mfd part drivers/mfd/ds1374.c > > > > The MFD part takes care of trickle charging and mode selection, > > since the usage modes of a) RTC + Alarm or b) RTC + WDT > > are mutually exclusive. > > > > Signed-off-by: Moritz Fischer <mdf@xxxxxxxxxx> > > [ Only reviewing watchdog part ] > > > --- > > drivers/mfd/Kconfig | 10 + > > drivers/mfd/Makefile | 1 + > > drivers/mfd/ds1374.c | 260 ++++++++++++++++ > > drivers/rtc/rtc-ds1374.c | 639 ++++++++++----------------------------- > > drivers/watchdog/Kconfig | 10 + > > drivers/watchdog/Makefile | 1 + > > drivers/watchdog/ds1374-wdt.c | 208 +++++++++++++ > > include/dt-bindings/mfd/ds1374.h | 17 ++ > > include/linux/mfd/ds1374.h | 59 ++++ > > 9 files changed, 722 insertions(+), 483 deletions(-) > > create mode 100644 drivers/mfd/ds1374.c > > create mode 100644 drivers/watchdog/ds1374-wdt.c > > create mode 100644 include/dt-bindings/mfd/ds1374.h > > create mode 100644 include/linux/mfd/ds1374.h > > > > If possible, it might be better to split the patch into multiple parts, one per subsystem. I'll have to think about that some more ;-) > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > index 3eb5c93..2dfef3c 100644 > > --- a/drivers/mfd/Kconfig > > +++ b/drivers/mfd/Kconfig > > @@ -203,6 +203,16 @@ config MFD_CROS_EC_SPI > > response time cannot be guaranteed, we support ignoring > > 'pre-amble' bytes before the response actually starts. > > +config MFD_DS1374 > > + tristate "Dallas/Maxim DS1374 RTC/WDT/ALARM (I2C)" > > + select MFD_CORE > > + depends on I2C > > + depends on REGMAP_I2C > > + > > + ---help--- > > + This driver supports the Dallas Maxim DS1374 multi function chip. > > + The chip combines an RTC, trickle charger, Watchdog or Alarm. > > + > > config MFD_ASIC3 > > bool "Compaq ASIC3" > > depends on GPIOLIB && ARM > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > > index c16bf1e..b5cfcf4 100644 > > --- a/drivers/mfd/Makefile > > +++ b/drivers/mfd/Makefile > > @@ -15,6 +15,7 @@ cros_ec_core-$(CONFIG_ACPI) += cros_ec_acpi_gpe.o > > obj-$(CONFIG_MFD_CROS_EC) += cros_ec_core.o > > obj-$(CONFIG_MFD_CROS_EC_I2C) += cros_ec_i2c.o > > obj-$(CONFIG_MFD_CROS_EC_SPI) += cros_ec_spi.o > > +obj-$(CONFIG_MFD_DS1374) += ds1374.o > > obj-$(CONFIG_MFD_EXYNOS_LPASS) += exynos-lpass.o > > rtsx_pci-objs := rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o > > diff --git a/drivers/mfd/ds1374.c b/drivers/mfd/ds1374.c > > new file mode 100644 > > index 0000000..a0cfa1b > > --- /dev/null > > +++ b/drivers/mfd/ds1374.c > > @@ -0,0 +1,260 @@ > > +/* > > + * Copyright (c) 2017, National Instruments Corp. > > + * > > + * Dallas/Maxim DS1374 Multi Function Device Driver > > + * > > + * The trickle charger code was taken more ore less 1:1 from > > + * drivers/rtc/rtc-1390.c > > + * > > + * SPDX-License-Identifier: GPL-2.0 > > + */ > > + > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/interrupt.h> > > +#include <linux/i2c.h> > > +#include <linux/slab.h> > > +#include <linux/pm.h> > > +#include <linux/regmap.h> > > +#include <linux/mfd/core.h> > > +#include <linux/mfd/ds1374.h> > > + > > +#define DS1374_TRICKLE_CHARGER_ENABLE 0xa0 > > +#define DS1374_TRICKLE_CHARGER_ENABLE_MASK 0xe0 > > + > > +#define DS1374_TRICKLE_CHARGER_250_OHM 0x01 > > +#define DS1374_TRICKLE_CHARGER_2K_OHM 0x02 > > +#define DS1374_TRICKLE_CHARGER_4K_OHM 0x03 > > +#define DS1374_TRICKLE_CHARGER_ROUT_MASK 0x03 > > + > > +#define DS1374_TRICKLE_CHARGER_NO_DIODE 0x04 > > +#define DS1374_TRICKLE_CHARGER_DIODE 0x08 > > +#define DS1374_TRICKLE_CHARGER_DIODE_MASK 0xc > > + > > +static const struct regmap_range volatile_ranges[] = { > > + regmap_reg_range(DS1374_REG_TOD0, DS1374_REG_WDALM2), > > + regmap_reg_range(DS1374_REG_SR, DS1374_REG_SR), > > +}; > > + > > +static const struct regmap_access_table ds1374_volatile_table = { > > + .yes_ranges = volatile_ranges, > > + .n_yes_ranges = ARRAY_SIZE(volatile_ranges), > > +}; > > + > > +static struct regmap_config ds1374_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > + .max_register = DS1374_REG_TCR, > > + .volatile_table = &ds1374_volatile_table, > > + .cache_type = REGCACHE_RBTREE, > > +}; > > + > > +static struct mfd_cell ds1374_wdt_cell = { > > + .name = "ds1374-wdt", > > +}; > > + > > +static struct mfd_cell ds1374_rtc_cell = { > > + .name = "ds1374-rtc", > > +}; > > + > > +static int ds1374_add_device(struct ds1374 *chip, > > + struct mfd_cell *cell) > > +{ > > + cell->platform_data = chip; > > + cell->pdata_size = sizeof(*chip); > > + > > + return mfd_add_devices(&chip->client->dev, PLATFORM_DEVID_AUTO, > > + cell, 1, NULL, 0, NULL); > > +} > > + > > +static int ds1374_trickle_of_init(struct ds1374 *ds1374) > > +{ > > + u32 ohms = 0; > > + u8 value; > > + struct i2c_client *client = ds1374->client; > > + > > + if (of_property_read_u32(client->dev.of_node, "trickle-resistor-ohms", > > + &ohms)) > > + return 0; > > + > > + /* Enable charger */ > > + value = DS1374_TRICKLE_CHARGER_ENABLE; > > + if (of_property_read_bool(client->dev.of_node, "trickle-diode-disable")) > > + value |= DS1374_TRICKLE_CHARGER_NO_DIODE; > > + else > > + value |= DS1374_TRICKLE_CHARGER_DIODE; > > + > > + /* Resistor select */ > > + switch (ohms) { > > + case 250: > > + value |= DS1374_TRICKLE_CHARGER_250_OHM; > > + break; > > + case 2000: > > + value |= DS1374_TRICKLE_CHARGER_2K_OHM; > > + break; > > + case 4000: > > + value |= DS1374_TRICKLE_CHARGER_4K_OHM; > > + break; > > + default: > > + dev_warn(&client->dev, > > + "Unsupported ohm value %02ux in dt\n", ohms); > > + return -EINVAL; > > + } > > + dev_dbg(&client->dev, "Trickle charge value is 0x%02x\n", value); > > + > > + return regmap_write(ds1374->regmap, DS1374_REG_TCR, value); > > +} > > + > > +int ds1374_read_bulk(struct ds1374 *ds1374, u32 *time, int reg, int nbytes) > > +{ > > + u8 buf[4]; > > + int ret; > > + int i; > > + > > + if (WARN_ON(nbytes > 4)) > > + return -EINVAL; > > + > > + ret = regmap_bulk_read(ds1374->regmap, reg, buf, nbytes); > > + if (ret) { > > + dev_err(&ds1374->client->dev, > > + "Failed to bulkread n = %d at R%d\n", > > + nbytes, reg); > > + return ret; > > + } > > + > > + for (i = nbytes - 1, *time = 0; i >= 0; i--) > > + *time = (*time << 8) | buf[i]; > > + > > I think those functions should go away; the calling code can use standard > endianness conversion functions and call regmap_bulk_{read,write} directly. Sounds good. I just ported over the code that had been there, but cleaning up is always a good idea :) > > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(ds1374_read_bulk); > > + > > +int ds1374_write_bulk(struct ds1374 *ds1374, u32 time, int reg, int nbytes) > > +{ > > + u8 buf[4]; > > + int i; > > + > > + if (nbytes > 4) { > > + WARN_ON(1); > > + return -EINVAL; > > + } > > + > > + for (i = 0; i < nbytes; i++) { > > + buf[i] = time & 0xff; > > + time >>= 8; > > + } > > + > > + return regmap_bulk_write(ds1374->regmap, reg, buf, nbytes); > > +} > > +EXPORT_SYMBOL_GPL(ds1374_write_bulk); > > + > > +static int ds1374_probe(struct i2c_client *client, > > + const struct i2c_device_id *id) > > +{ > > + struct ds1374 *ds1374; > > + u32 mode; > > + int err; > > + > > + ds1374 = devm_kzalloc(&client->dev, sizeof(struct ds1374), GFP_KERNEL); > > + if (!ds1374) > > + return -ENOMEM; > > + > > + ds1374->regmap = devm_regmap_init_i2c(client, &ds1374_regmap_config); > > + if (IS_ERR(ds1374->regmap)) > > + return PTR_ERR(ds1374->regmap); > > + > > + if (IS_ENABLED(CONFIG_OF) && client->dev.of_node) { > > + err = of_property_read_u32(client->dev.of_node, > > + "dallas,mode", &mode); > > + if (err < 0) { > > + dev_err(&client->dev, "missing dallas,mode property\n"); > > + return -EINVAL; > > + } > > + > > + ds1374->remapped_reset > > + = of_property_read_bool(client->dev.of_node, > > + "dallas,remap-reset"); > > + > > + ds1374->mode = (enum ds1374_mode)mode; > > + } else if (IS_ENABLED(CONFIG_RTC_DRV_DS1374_WDT)) { > > + ds1374->mode = DS1374_MODE_RTC_WDT; > > + } else { > > + ds1374->mode = DS1374_MODE_RTC_ALM; > > + } > > + > > + ds1374->client = client; > > + ds1374->irq = client->irq; > > + i2c_set_clientdata(client, ds1374); > > + > > + /* check if we're supposed to trickle charge */ > > + err = ds1374_trickle_of_init(ds1374); > > + if (err) { > > + dev_err(&client->dev, "Failed to init trickle charger!\n"); > > + return err; > > + } > > + > > + /* we always have a rtc */ > > + err = ds1374_add_device(ds1374, &ds1374_rtc_cell); > > + if (err) > > + return err; > > + > > + /* we might have a watchdog if configured that way */ > > + if (ds1374->mode == DS1374_MODE_RTC_WDT) > > + return ds1374_add_device(ds1374, &ds1374_wdt_cell); > > + > > + return err; > > +} > > + > > +static const struct i2c_device_id ds1374_id[] = { > > + { "ds1374", 0 }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(i2c, ds1374_id); > > + > > +#ifdef CONFIG_OF > > +static const struct of_device_id ds1374_of_match[] = { > > + { .compatible = "dallas,ds1374" }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(of, ds1374_of_match); > > +#endif > > + > > +#ifdef CONFIG_PM_SLEEP > > +static int ds1374_suspend(struct device *dev) > > +{ > > + return 0; > > +} > > + > > +static int ds1374_resume(struct device *dev) > > +{ > > + return 0; > > +} > > +#endif > > + > > +static SIMPLE_DEV_PM_OPS(ds1374_pm, ds1374_suspend, ds1374_resume); > > + > > +static struct i2c_driver ds1374_driver = { > > + .driver = { > > + .name = "ds1374", > > + .of_match_table = of_match_ptr(ds1374_of_match), > > + .pm = &ds1374_pm, > > + }, > > + .probe = ds1374_probe, > > + .id_table = ds1374_id, > > +}; > > + > > +static int __init ds1374_init(void) > > +{ > > + return i2c_add_driver(&ds1374_driver); > > +} > > +subsys_initcall(ds1374_init); > > + > > +static void __exit ds1374_exit(void) > > +{ > > + i2c_del_driver(&ds1374_driver); > > +} > > +module_exit(ds1374_exit); > > + > > +MODULE_AUTHOR("Moritz Fischer <mdf@xxxxxxxxxx>"); > > +MODULE_DESCRIPTION("Maxim/Dallas DS1374 MFD Driver"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/rtc/rtc-ds1374.c b/drivers/rtc/rtc-ds1374.c > > index 52429f0..29ce1a9 100644 > > --- a/drivers/rtc/rtc-ds1374.c > > +++ b/drivers/rtc/rtc-ds1374.c > > @@ -1,75 +1,38 @@ > > /* > > - * RTC client/driver for the Maxim/Dallas DS1374 Real-Time Clock over I2C > > + * RTC driver for the Maxim/Dallas DS1374 Real-Time Clock via MFD > > * > > * Based on code by Randy Vinson <rvinson@xxxxxxxxxx>, > > * which was based on the m41t00.c by Mark Greer <mgreer@xxxxxxxxxx>. > > * > > + * Copyright (C) 2017 National Instruments Corp > > * Copyright (C) 2014 Rose Technology > > * Copyright (C) 2006-2007 Freescale Semiconductor > > * > > + * SPDX-License-Identifier: GPL-2.0 > > + * > > * 2005 (c) MontaVista Software, Inc. This file is licensed under > > * the terms of the GNU General Public License version 2. This program > > * is licensed "as is" without any warranty of any kind, whether express > > * or implied. > > */ > > -/* > > - * It would be more efficient to use i2c msgs/i2c_transfer directly but, as > > - * recommened in .../Documentation/i2c/writing-clients section > > - * "Sending and receiving", using SMBus level communication is preferred. > > - */ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > #include <linux/kernel.h> > > #include <linux/module.h> > > #include <linux/interrupt.h> > > -#include <linux/i2c.h> > > #include <linux/rtc.h> > > #include <linux/bcd.h> > > #include <linux/workqueue.h> > > #include <linux/slab.h> > > #include <linux/pm.h> > > -#ifdef CONFIG_RTC_DRV_DS1374_WDT > > -#include <linux/fs.h> > > -#include <linux/ioctl.h> > > -#include <linux/miscdevice.h> > > -#include <linux/reboot.h> > > -#include <linux/watchdog.h> > > -#endif > > - > > -#define DS1374_REG_TOD0 0x00 /* Time of Day */ > > -#define DS1374_REG_TOD1 0x01 > > -#define DS1374_REG_TOD2 0x02 > > -#define DS1374_REG_TOD3 0x03 > > -#define DS1374_REG_WDALM0 0x04 /* Watchdog/Alarm */ > > -#define DS1374_REG_WDALM1 0x05 > > -#define DS1374_REG_WDALM2 0x06 > > -#define DS1374_REG_CR 0x07 /* Control */ > > -#define DS1374_REG_CR_AIE 0x01 /* Alarm Int. Enable */ > > -#define DS1374_REG_CR_WDALM 0x20 /* 1=Watchdog, 0=Alarm */ > > -#define DS1374_REG_CR_WACE 0x40 /* WD/Alarm counter enable */ > > -#define DS1374_REG_SR 0x08 /* Status */ > > -#define DS1374_REG_SR_OSF 0x80 /* Oscillator Stop Flag */ > > -#define DS1374_REG_SR_AF 0x01 /* Alarm Flag */ > > -#define DS1374_REG_TCR 0x09 /* Trickle Charge */ > > - > > -static const struct i2c_device_id ds1374_id[] = { > > - { "ds1374", 0 }, > > - { } > > -}; > > -MODULE_DEVICE_TABLE(i2c, ds1374_id); > > +#include <linux/regmap.h> > > +#include <linux/mfd/ds1374.h> > > +#include <linux/platform_device.h> > > -#ifdef CONFIG_OF > > -static const struct of_device_id ds1374_of_match[] = { > > - { .compatible = "dallas,ds1374" }, > > - { } > > -}; > > -MODULE_DEVICE_TABLE(of, ds1374_of_match); > > -#endif > > - > > -struct ds1374 { > > - struct i2c_client *client; > > +struct ds1374_rtc { > > struct rtc_device *rtc; > > + struct ds1374 *chip; > > struct work_struct work; > > /* The mutex protects alarm operations, and prevents a race > > @@ -80,89 +43,44 @@ struct ds1374 { > > int exiting; > > }; > > -static struct i2c_driver ds1374_driver; > > - > > -static int ds1374_read_rtc(struct i2c_client *client, u32 *time, > > - int reg, int nbytes) > > -{ > > - u8 buf[4]; > > - int ret; > > - int i; > > - > > - if (WARN_ON(nbytes > 4)) > > - return -EINVAL; > > - > > - ret = i2c_smbus_read_i2c_block_data(client, reg, nbytes, buf); > > - > > - if (ret < 0) > > - return ret; > > - if (ret < nbytes) > > - return -EIO; > > - > > - for (i = nbytes - 1, *time = 0; i >= 0; i--) > > - *time = (*time << 8) | buf[i]; > > - > > - return 0; > > -} > > - > > -static int ds1374_write_rtc(struct i2c_client *client, u32 time, > > - int reg, int nbytes) > > -{ > > - u8 buf[4]; > > - int i; > > - > > - if (nbytes > 4) { > > - WARN_ON(1); > > - return -EINVAL; > > - } > > - > > - for (i = 0; i < nbytes; i++) { > > - buf[i] = time & 0xff; > > - time >>= 8; > > - } > > - > > - return i2c_smbus_write_i2c_block_data(client, reg, nbytes, buf); > > -} > > - > > -static int ds1374_check_rtc_status(struct i2c_client *client) > > +static int ds1374_check_rtc_status(struct ds1374_rtc *ds1374) > > { > > int ret = 0; > > - int control, stat; > > + unsigned int control, stat; > > - stat = i2c_smbus_read_byte_data(client, DS1374_REG_SR); > > - if (stat < 0) > > + ret = regmap_read(ds1374->chip->regmap, DS1374_REG_SR, &stat); > > + if (ret) > > return stat; > > if (stat & DS1374_REG_SR_OSF) > > - dev_warn(&client->dev, > > + dev_warn(&ds1374->chip->client->dev, > > "oscillator discontinuity flagged, time unreliable\n"); > > - stat &= ~(DS1374_REG_SR_OSF | DS1374_REG_SR_AF); > > - > > - ret = i2c_smbus_write_byte_data(client, DS1374_REG_SR, stat); > > - if (ret < 0) > > + ret = regmap_update_bits(ds1374->chip->regmap, DS1374_REG_SR, > > + DS1374_REG_SR_OSF | DS1374_REG_SR_AF, 0); > > + if (ret) > > return ret; > > /* If the alarm is pending, clear it before requesting > > * the interrupt, so an interrupt event isn't reported > > * before everything is initialized. > > */ > > - > > - control = i2c_smbus_read_byte_data(client, DS1374_REG_CR); > > - if (control < 0) > > - return control; > > + ret = regmap_read(ds1374->chip->regmap, DS1374_REG_CR, &control); > > + if (ret) > > + return ret; > > control &= ~(DS1374_REG_CR_WACE | DS1374_REG_CR_AIE); > > - return i2c_smbus_write_byte_data(client, DS1374_REG_CR, control); > > + return regmap_write(ds1374->chip->regmap, DS1374_REG_CR, control); > > } > > static int ds1374_read_time(struct device *dev, struct rtc_time *time) > > { > > - struct i2c_client *client = to_i2c_client(dev); > > + struct platform_device *pdev = to_platform_device(dev); > > + struct ds1374_rtc *ds1374_rtc = platform_get_drvdata(pdev); > > u32 itime; > > int ret; > > - ret = ds1374_read_rtc(client, &itime, DS1374_REG_TOD0, 4); > > + ret = ds1374_read_bulk(ds1374_rtc->chip, &itime, DS1374_REG_TOD0, 4); > > if (!ret) > > rtc_time_to_tm(itime, time); > > @@ -171,44 +89,47 @@ static int ds1374_read_time(struct device *dev, struct rtc_time *time) > > static int ds1374_set_time(struct device *dev, struct rtc_time *time) > > { > > - struct i2c_client *client = to_i2c_client(dev); > > + struct platform_device *pdev = to_platform_device(dev); > > + struct ds1374_rtc *ds1374_rtc = platform_get_drvdata(pdev); > > unsigned long itime; > > rtc_tm_to_time(time, &itime); > > - return ds1374_write_rtc(client, itime, DS1374_REG_TOD0, 4); > > + return ds1374_write_bulk(ds1374_rtc->chip, itime, DS1374_REG_TOD0, 4); > > } > > -#ifndef CONFIG_RTC_DRV_DS1374_WDT > > /* The ds1374 has a decrementer for an alarm, rather than a comparator. > > * If the time of day is changed, then the alarm will need to be > > * reset. > > */ > > static int ds1374_read_alarm(struct device *dev, struct rtc_wkalrm *alarm) > > { > > - struct i2c_client *client = to_i2c_client(dev); > > - struct ds1374 *ds1374 = i2c_get_clientdata(client); > > + struct platform_device *pdev = to_platform_device(dev); > > + struct ds1374_rtc *ds1374_rtc = platform_get_drvdata(pdev); > > + struct ds1374 *ds1374 = ds1374_rtc->chip; > > + > > u32 now, cur_alarm; > > - int cr, sr; > > + unsigned int cr, sr; > > int ret = 0; > > - if (client->irq <= 0) > > + if (ds1374->irq <= 0) > > return -EINVAL; > > - mutex_lock(&ds1374->mutex); > > + mutex_lock(&ds1374_rtc->mutex); > > - cr = ret = i2c_smbus_read_byte_data(client, DS1374_REG_CR); > > + ret = regmap_read(ds1374->regmap, DS1374_REG_CR, &cr); > > if (ret < 0) > > goto out; > > - sr = ret = i2c_smbus_read_byte_data(client, DS1374_REG_SR); > > + ret = regmap_read(ds1374->regmap, DS1374_REG_SR, &sr); > > if (ret < 0) > > goto out; > > - ret = ds1374_read_rtc(client, &now, DS1374_REG_TOD0, 4); > > + ret = ds1374_read_bulk(ds1374_rtc->chip, &now, DS1374_REG_TOD0, 4); > > if (ret) > > goto out; > > - ret = ds1374_read_rtc(client, &cur_alarm, DS1374_REG_WDALM0, 3); > > + ret = ds1374_read_bulk(ds1374_rtc->chip, &cur_alarm, > > + DS1374_REG_WDALM0, 3); > > if (ret) > > goto out; > > @@ -217,20 +138,21 @@ static int ds1374_read_alarm(struct device *dev, struct rtc_wkalrm *alarm) > > alarm->pending = !!(sr & DS1374_REG_SR_AF); > > out: > > - mutex_unlock(&ds1374->mutex); > > + mutex_unlock(&ds1374_rtc->mutex); > > return ret; > > } > > static int ds1374_set_alarm(struct device *dev, struct rtc_wkalrm *alarm) > > { > > - struct i2c_client *client = to_i2c_client(dev); > > - struct ds1374 *ds1374 = i2c_get_clientdata(client); > > + struct platform_device *pdev = to_platform_device(dev); > > + struct ds1374_rtc *ds1374_rtc = platform_get_drvdata(pdev); > > + struct ds1374 *ds1374 = ds1374_rtc->chip; > > + > > struct rtc_time now; > > unsigned long new_alarm, itime; > > - int cr; > > int ret = 0; > > - if (client->irq <= 0) > > + if (ds1374->irq <= 0) > > return -EINVAL; > > ret = ds1374_read_time(dev, &now); > > @@ -251,468 +173,219 @@ static int ds1374_set_alarm(struct device *dev, struct rtc_wkalrm *alarm) > > else > > new_alarm -= itime; > > - mutex_lock(&ds1374->mutex); > > - > > - ret = cr = i2c_smbus_read_byte_data(client, DS1374_REG_CR); > > - if (ret < 0) > > - goto out; > > + mutex_lock(&ds1374_rtc->mutex); > > /* Disable any existing alarm before setting the new one > > - * (or lack thereof). */ > > - cr &= ~DS1374_REG_CR_WACE; > > - > > - ret = i2c_smbus_write_byte_data(client, DS1374_REG_CR, cr); > > - if (ret < 0) > > - goto out; > > + * (or lack thereof). > > + */ > > + ret = regmap_update_bits(ds1374->regmap, DS1374_REG_CR, > > + DS1374_REG_CR_WACE, 0); > > - ret = ds1374_write_rtc(client, new_alarm, DS1374_REG_WDALM0, 3); > > + ret = ds1374_write_bulk(ds1374_rtc->chip, new_alarm, > > + DS1374_REG_WDALM0, 3); > > if (ret) > > goto out; > > if (alarm->enabled) { > > - cr |= DS1374_REG_CR_WACE | DS1374_REG_CR_AIE; > > - cr &= ~DS1374_REG_CR_WDALM; > > - > > - ret = i2c_smbus_write_byte_data(client, DS1374_REG_CR, cr); > > + ret = regmap_update_bits(ds1374->regmap, DS1374_REG_CR, > > + DS1374_REG_CR_WACE | DS1374_REG_CR_AIE > > + | DS1374_REG_CR_WDALM, > > + DS1374_REG_CR_WACE > > + | DS1374_REG_CR_AIE); > > } > > out: > > - mutex_unlock(&ds1374->mutex); > > + mutex_unlock(&ds1374_rtc->mutex); > > return ret; > > } > > -#endif > > static irqreturn_t ds1374_irq(int irq, void *dev_id) > > { > > - struct i2c_client *client = dev_id; > > - struct ds1374 *ds1374 = i2c_get_clientdata(client); > > + struct ds1374_rtc *ds1374_rtc = dev_id; > > disable_irq_nosync(irq); > > - schedule_work(&ds1374->work); > > + schedule_work(&ds1374_rtc->work); > > return IRQ_HANDLED; > > } > > static void ds1374_work(struct work_struct *work) > > { > > - struct ds1374 *ds1374 = container_of(work, struct ds1374, work); > > - struct i2c_client *client = ds1374->client; > > - int stat, control; > > + struct ds1374_rtc *ds1374_rtc = container_of(work, struct ds1374_rtc, > > + work); > > + unsigned int stat; > > + int ret; > > - mutex_lock(&ds1374->mutex); > > + mutex_lock(&ds1374_rtc->mutex); > > - stat = i2c_smbus_read_byte_data(client, DS1374_REG_SR); > > - if (stat < 0) > > + ret = regmap_read(ds1374_rtc->chip->regmap, DS1374_REG_SR, &stat); > > + if (ret) > > goto unlock; > > if (stat & DS1374_REG_SR_AF) { > > - stat &= ~DS1374_REG_SR_AF; > > - i2c_smbus_write_byte_data(client, DS1374_REG_SR, stat); > > - > > - control = i2c_smbus_read_byte_data(client, DS1374_REG_CR); > > - if (control < 0) > > + regmap_update_bits(ds1374_rtc->chip->regmap, DS1374_REG_SR, > > + DS1374_REG_SR_AF, 0); > > + > > + ret = regmap_update_bits(ds1374_rtc->chip->regmap, > > + DS1374_REG_CR, DS1374_REG_CR_WACE > > + | DS1374_REG_CR_AIE, > > + 0); > > + if (ret) > > goto out; > > - control &= ~(DS1374_REG_CR_WACE | DS1374_REG_CR_AIE); > > - i2c_smbus_write_byte_data(client, DS1374_REG_CR, control); > > - > > - rtc_update_irq(ds1374->rtc, 1, RTC_AF | RTC_IRQF); > > + rtc_update_irq(ds1374_rtc->rtc, 1, RTC_AF | RTC_IRQF); > > } > > out: > > - if (!ds1374->exiting) > > - enable_irq(client->irq); > > + if (!ds1374_rtc->exiting) > > + enable_irq(ds1374_rtc->chip->irq); > > unlock: > > - mutex_unlock(&ds1374->mutex); > > + mutex_unlock(&ds1374_rtc->mutex); > > } > > -#ifndef CONFIG_RTC_DRV_DS1374_WDT > > static int ds1374_alarm_irq_enable(struct device *dev, unsigned int enabled) > > { > > - struct i2c_client *client = to_i2c_client(dev); > > - struct ds1374 *ds1374 = i2c_get_clientdata(client); > > + struct platform_device *pdev = to_platform_device(dev); > > + struct ds1374_rtc *ds1374 = platform_get_drvdata(pdev); > > + unsigned int cr; > > int ret; > > mutex_lock(&ds1374->mutex); > > - ret = i2c_smbus_read_byte_data(client, DS1374_REG_CR); > > + ret = regmap_read(ds1374->chip->regmap, DS1374_REG_CR, &cr); > > if (ret < 0) > > goto out; > > - if (enabled) { > > - ret |= DS1374_REG_CR_WACE | DS1374_REG_CR_AIE; > > - ret &= ~DS1374_REG_CR_WDALM; > > - } else { > > - ret &= ~DS1374_REG_CR_WACE; > > - } > > - ret = i2c_smbus_write_byte_data(client, DS1374_REG_CR, ret); > > - > > + if (enabled) > > + regmap_update_bits(ds1374->chip->regmap, DS1374_REG_CR, > > + DS1374_REG_CR_WACE | DS1374_REG_CR_AIE | > > + DS1374_REG_CR_WDALM, DS1374_REG_CR_WACE | > > + DS1374_REG_CR_AIE); > > + else > > + regmap_update_bits(ds1374->chip->regmap, DS1374_REG_CR, > > + DS1374_REG_CR_WACE, 0); > > out: > > mutex_unlock(&ds1374->mutex); > > return ret; > > } > > -#endif > > -static const struct rtc_class_ops ds1374_rtc_ops = { > > +static const struct rtc_class_ops ds1374_rtc_alm_ops = { > > .read_time = ds1374_read_time, > > .set_time = ds1374_set_time, > > -#ifndef CONFIG_RTC_DRV_DS1374_WDT > > .read_alarm = ds1374_read_alarm, > > .set_alarm = ds1374_set_alarm, > > .alarm_irq_enable = ds1374_alarm_irq_enable, > > -#endif > > }; > > -#ifdef CONFIG_RTC_DRV_DS1374_WDT > > -/* > > - ***************************************************************************** > > - * > > - * Watchdog Driver > > - * > > - ***************************************************************************** > > - */ > > -static struct i2c_client *save_client; > > -/* Default margin */ > > -#define WD_TIMO 131762 > > - > > -#define DRV_NAME "DS1374 Watchdog" > > - > > -static int wdt_margin = WD_TIMO; > > -static unsigned long wdt_is_open; > > -module_param(wdt_margin, int, 0); > > -MODULE_PARM_DESC(wdt_margin, "Watchdog timeout in seconds (default 32s)"); > > - > > -static const struct watchdog_info ds1374_wdt_info = { > > - .identity = "DS1374 WTD", > > - .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | > > - WDIOF_MAGICCLOSE, > > -}; > > - > > -static int ds1374_wdt_settimeout(unsigned int timeout) > > -{ > > - int ret = -ENOIOCTLCMD; > > - int cr; > > - > > - ret = cr = i2c_smbus_read_byte_data(save_client, DS1374_REG_CR); > > - if (ret < 0) > > - goto out; > > - > > - /* Disable any existing watchdog/alarm before setting the new one */ > > - cr &= ~DS1374_REG_CR_WACE; > > - > > - ret = i2c_smbus_write_byte_data(save_client, DS1374_REG_CR, cr); > > - if (ret < 0) > > - goto out; > > - > > - /* Set new watchdog time */ > > - ret = ds1374_write_rtc(save_client, timeout, DS1374_REG_WDALM0, 3); > > - if (ret) { > > - pr_info("couldn't set new watchdog time\n"); > > - goto out; > > - } > > - > > - /* Enable watchdog timer */ > > - cr |= DS1374_REG_CR_WACE | DS1374_REG_CR_WDALM; > > - cr &= ~DS1374_REG_CR_AIE; > > - > > - ret = i2c_smbus_write_byte_data(save_client, DS1374_REG_CR, cr); > > - if (ret < 0) > > - goto out; > > - > > - return 0; > > -out: > > - return ret; > > -} > > - > > - > > -/* > > - * Reload the watchdog timer. (ie, pat the watchdog) > > - */ > > -static void ds1374_wdt_ping(void) > > -{ > > - u32 val; > > - int ret = 0; > > - > > - ret = ds1374_read_rtc(save_client, &val, DS1374_REG_WDALM0, 3); > > - if (ret) > > - pr_info("WD TICK FAIL!!!!!!!!!! %i\n", ret); > > -} > > - > > -static void ds1374_wdt_disable(void) > > -{ > > - int ret = -ENOIOCTLCMD; > > - int cr; > > - > > - cr = i2c_smbus_read_byte_data(save_client, DS1374_REG_CR); > > - /* Disable watchdog timer */ > > - cr &= ~DS1374_REG_CR_WACE; > > - > > - ret = i2c_smbus_write_byte_data(save_client, DS1374_REG_CR, cr); > > -} > > - > > -/* > > - * Watchdog device is opened, and watchdog starts running. > > - */ > > -static int ds1374_wdt_open(struct inode *inode, struct file *file) > > -{ > > - struct ds1374 *ds1374 = i2c_get_clientdata(save_client); > > - > > - if (MINOR(inode->i_rdev) == WATCHDOG_MINOR) { > > - mutex_lock(&ds1374->mutex); > > - if (test_and_set_bit(0, &wdt_is_open)) { > > - mutex_unlock(&ds1374->mutex); > > - return -EBUSY; > > - } > > - /* > > - * Activate > > - */ > > - wdt_is_open = 1; > > - mutex_unlock(&ds1374->mutex); > > - return nonseekable_open(inode, file); > > - } > > - return -ENODEV; > > -} > > - > > -/* > > - * Close the watchdog device. > > - */ > > -static int ds1374_wdt_release(struct inode *inode, struct file *file) > > -{ > > - if (MINOR(inode->i_rdev) == WATCHDOG_MINOR) > > - clear_bit(0, &wdt_is_open); > > - > > - return 0; > > -} > > - > > -/* > > - * Pat the watchdog whenever device is written to. > > - */ > > -static ssize_t ds1374_wdt_write(struct file *file, const char __user *data, > > - size_t len, loff_t *ppos) > > -{ > > - if (len) { > > - ds1374_wdt_ping(); > > - return 1; > > - } > > - return 0; > > -} > > - > > -static ssize_t ds1374_wdt_read(struct file *file, char __user *data, > > - size_t len, loff_t *ppos) > > -{ > > - return 0; > > -} > > - > > -/* > > - * Handle commands from user-space. > > - */ > > -static long ds1374_wdt_ioctl(struct file *file, unsigned int cmd, > > - unsigned long arg) > > -{ > > - int new_margin, options; > > - > > - switch (cmd) { > > - case WDIOC_GETSUPPORT: > > - return copy_to_user((struct watchdog_info __user *)arg, > > - &ds1374_wdt_info, sizeof(ds1374_wdt_info)) ? -EFAULT : 0; > > - > > - case WDIOC_GETSTATUS: > > - case WDIOC_GETBOOTSTATUS: > > - return put_user(0, (int __user *)arg); > > - case WDIOC_KEEPALIVE: > > - ds1374_wdt_ping(); > > - return 0; > > - case WDIOC_SETTIMEOUT: > > - if (get_user(new_margin, (int __user *)arg)) > > - return -EFAULT; > > - > > - if (new_margin < 1 || new_margin > 16777216) > > - return -EINVAL; > > - > > - wdt_margin = new_margin; > > - ds1374_wdt_settimeout(new_margin); > > - ds1374_wdt_ping(); > > - /* fallthrough */ > > - case WDIOC_GETTIMEOUT: > > - return put_user(wdt_margin, (int __user *)arg); > > - case WDIOC_SETOPTIONS: > > - if (copy_from_user(&options, (int __user *)arg, sizeof(int))) > > - return -EFAULT; > > - > > - if (options & WDIOS_DISABLECARD) { > > - pr_info("disable watchdog\n"); > > - ds1374_wdt_disable(); > > - } > > - > > - if (options & WDIOS_ENABLECARD) { > > - pr_info("enable watchdog\n"); > > - ds1374_wdt_settimeout(wdt_margin); > > - ds1374_wdt_ping(); > > - } > > - > > - return -EINVAL; > > - } > > - return -ENOTTY; > > -} > > - > > -static long ds1374_wdt_unlocked_ioctl(struct file *file, unsigned int cmd, > > - unsigned long arg) > > -{ > > - int ret; > > - struct ds1374 *ds1374 = i2c_get_clientdata(save_client); > > - > > - mutex_lock(&ds1374->mutex); > > - ret = ds1374_wdt_ioctl(file, cmd, arg); > > - mutex_unlock(&ds1374->mutex); > > - > > - return ret; > > -} > > - > > -static int ds1374_wdt_notify_sys(struct notifier_block *this, > > - unsigned long code, void *unused) > > -{ > > - if (code == SYS_DOWN || code == SYS_HALT) > > - /* Disable Watchdog */ > > - ds1374_wdt_disable(); > > - return NOTIFY_DONE; > > -} > > - > > -static const struct file_operations ds1374_wdt_fops = { > > - .owner = THIS_MODULE, > > - .read = ds1374_wdt_read, > > - .unlocked_ioctl = ds1374_wdt_unlocked_ioctl, > > - .write = ds1374_wdt_write, > > - .open = ds1374_wdt_open, > > - .release = ds1374_wdt_release, > > - .llseek = no_llseek, > > -}; > > - > > -static struct miscdevice ds1374_miscdev = { > > - .minor = WATCHDOG_MINOR, > > - .name = "watchdog", > > - .fops = &ds1374_wdt_fops, > > -}; > > - > > -static struct notifier_block ds1374_wdt_notifier = { > > - .notifier_call = ds1374_wdt_notify_sys, > > +static const struct rtc_class_ops ds1374_rtc_ops = { > > + .read_time = ds1374_read_time, > > + .set_time = ds1374_set_time, > > }; > > -#endif /*CONFIG_RTC_DRV_DS1374_WDT*/ > > -/* > > - ***************************************************************************** > > - * > > - * Driver Interface > > - * > > - ***************************************************************************** > > - */ > > -static int ds1374_probe(struct i2c_client *client, > > - const struct i2c_device_id *id) > > +static int ds1374_rtc_probe(struct platform_device *pdev) > > { > > - struct ds1374 *ds1374; > > + struct device *dev = &pdev->dev; > > + struct ds1374 *ds1374 = dev_get_drvdata(dev->parent); > > + struct ds1374_rtc *ds1374_rtc; > > int ret; > > - ds1374 = devm_kzalloc(&client->dev, sizeof(struct ds1374), GFP_KERNEL); > > - if (!ds1374) > > + ds1374_rtc = devm_kzalloc(dev, sizeof(*ds1374_rtc), GFP_KERNEL); > > + if (!ds1374_rtc) > > return -ENOMEM; > > + ds1374_rtc->chip = ds1374; > > - ds1374->client = client; > > - i2c_set_clientdata(client, ds1374); > > + platform_set_drvdata(pdev, ds1374_rtc); > > - INIT_WORK(&ds1374->work, ds1374_work); > > - mutex_init(&ds1374->mutex); > > + INIT_WORK(&ds1374_rtc->work, ds1374_work); > > + mutex_init(&ds1374_rtc->mutex); > > - ret = ds1374_check_rtc_status(client); > > - if (ret) > > + ret = ds1374_check_rtc_status(ds1374_rtc); > > + if (ret) { > > + dev_err(dev, "Failed to check rtc status\n"); > > return ret; > > + } > > - if (client->irq > 0) { > > - ret = devm_request_irq(&client->dev, client->irq, ds1374_irq, 0, > > - "ds1374", client); > > + /* if the mfd device indicates is configured to run with ALM > > + * try to get the IRQ > > + */ > > + if (ds1374->mode == DS1374_MODE_RTC_ALM && ds1374->irq > 0) { > > + ret = devm_request_irq(dev, ds1374->irq, > > + ds1374_irq, 0, "ds1374", ds1374_rtc); > > if (ret) { > > - dev_err(&client->dev, "unable to request IRQ\n"); > > + dev_err(dev, "unable to request IRQ\n"); > > return ret; > > } > > - device_set_wakeup_capable(&client->dev, 1); > > + device_set_wakeup_capable(dev, 1); > > + ds1374_rtc->rtc = devm_rtc_device_register(dev, > > + "ds1374-rtc", > > + &ds1374_rtc_alm_ops, > > + THIS_MODULE); > > + } else { > > + ds1374_rtc->rtc = devm_rtc_device_register(dev, "ds1374-rtc", > > + &ds1374_rtc_ops, > > + THIS_MODULE); > > } > > - ds1374->rtc = devm_rtc_device_register(&client->dev, client->name, > > - &ds1374_rtc_ops, THIS_MODULE); > > - if (IS_ERR(ds1374->rtc)) { > > - dev_err(&client->dev, "unable to register the class device\n"); > > - return PTR_ERR(ds1374->rtc); > > + if (IS_ERR(ds1374_rtc->rtc)) { > > + dev_err(dev, "unable to register the class device\n"); > > + return PTR_ERR(ds1374_rtc->rtc); > > } > > - > > -#ifdef CONFIG_RTC_DRV_DS1374_WDT > > - save_client = client; > > - ret = misc_register(&ds1374_miscdev); > > - if (ret) > > - return ret; > > - ret = register_reboot_notifier(&ds1374_wdt_notifier); > > - if (ret) { > > - misc_deregister(&ds1374_miscdev); > > - return ret; > > - } > > - ds1374_wdt_settimeout(131072); > > -#endif > > - > > return 0; > > } > > -static int ds1374_remove(struct i2c_client *client) > > +static int ds1374_rtc_remove(struct platform_device *pdev) > > { > > - struct ds1374 *ds1374 = i2c_get_clientdata(client); > > -#ifdef CONFIG_RTC_DRV_DS1374_WDT > > - misc_deregister(&ds1374_miscdev); > > - ds1374_miscdev.parent = NULL; > > - unregister_reboot_notifier(&ds1374_wdt_notifier); > > -#endif > > + struct ds1374_rtc *ds1374_rtc = platform_get_drvdata(pdev); > > - if (client->irq > 0) { > > - mutex_lock(&ds1374->mutex); > > - ds1374->exiting = 1; > > - mutex_unlock(&ds1374->mutex); > > + if (ds1374_rtc->chip->irq > 0) { > > + mutex_lock(&ds1374_rtc->mutex); > > + ds1374_rtc->exiting = 1; > > + mutex_unlock(&ds1374_rtc->mutex); > > - devm_free_irq(&client->dev, client->irq, client); > > - cancel_work_sync(&ds1374->work); > > + devm_free_irq(&pdev->dev, ds1374_rtc->chip->irq, > > + ds1374_rtc); > > + cancel_work_sync(&ds1374_rtc->work); > > } > > return 0; > > } > > #ifdef CONFIG_PM_SLEEP > > -static int ds1374_suspend(struct device *dev) > > +static int ds1374_rtc_suspend(struct device *dev) > > { > > - struct i2c_client *client = to_i2c_client(dev); > > + struct platform_device *pdev = to_platform_device(dev); > > + struct ds1374_rtc *ds1374_rtc = platform_get_drvdata(pdev); > > - if (client->irq > 0 && device_may_wakeup(&client->dev)) > > - enable_irq_wake(client->irq); > > + if (ds1374_rtc->chip->irq > 0 && device_may_wakeup(&pdev->dev)) > > + enable_irq_wake(ds1374_rtc->chip->irq); > > return 0; > > } > > -static int ds1374_resume(struct device *dev) > > +static int ds1374_rtc_resume(struct device *dev) > > { > > - struct i2c_client *client = to_i2c_client(dev); > > + struct platform_device *pdev = to_platform_device(dev); > > + struct ds1374_rtc *ds1374_rtc = platform_get_drvdata(pdev); > > - if (client->irq > 0 && device_may_wakeup(&client->dev)) > > - disable_irq_wake(client->irq); > > + if (ds1374_rtc->chip->irq > 0 && device_may_wakeup(&pdev->dev)) > > + disable_irq_wake(ds1374_rtc->chip->irq); > > return 0; > > } > > #endif > > -static SIMPLE_DEV_PM_OPS(ds1374_pm, ds1374_suspend, ds1374_resume); > > +static SIMPLE_DEV_PM_OPS(ds1374_rtc_pm, ds1374_rtc_suspend, ds1374_rtc_resume); > > -static struct i2c_driver ds1374_driver = { > > +static struct platform_driver ds1374_rtc_driver = { > > .driver = { > > - .name = "rtc-ds1374", > > - .pm = &ds1374_pm, > > + .name = "ds1374-rtc", > > + .pm = &ds1374_rtc_pm, > > }, > > - .probe = ds1374_probe, > > - .remove = ds1374_remove, > > - .id_table = ds1374_id, > > + .probe = ds1374_rtc_probe, > > + .remove = ds1374_rtc_remove, > > }; > > - > > -module_i2c_driver(ds1374_driver); > > +module_platform_driver(ds1374_rtc_driver); > > MODULE_AUTHOR("Scott Wood <scottwood@xxxxxxxxxxxxx>"); > > +MODULE_AUTHOR("Moritz Fischer <mdf@xxxxxxxxxx>"); > > MODULE_DESCRIPTION("Maxim/Dallas DS1374 RTC Driver"); > > MODULE_LICENSE("GPL"); > > +MODULE_ALIAS("platform:ds1374-rtc"); > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > > index 52a70ee..1703611 100644 > > --- a/drivers/watchdog/Kconfig > > +++ b/drivers/watchdog/Kconfig > > @@ -120,6 +120,16 @@ config DA9062_WATCHDOG > > This driver can be built as a module. The module name is da9062_wdt. > > +config DS1374_WATCHDOG > > + tristate "Maxim/Dallas 1374 Watchdog" > > + depends on MFD_DS1374 > > + depends on REGMAP_I2C > > depends on I2C > select REGMAP_I2C > > but doesn't the mfd driver already depend on that ? Yeah, will fix that. > > > + select WATCHDOG_CORE > > + help > > + Support for the watchdog in the Maxim/Dallas DS1374 MFD. > > + > > + This driver can be built as a module. The module name is ds1374-wdt. > > + > > config GPIO_WATCHDOG > > tristate "Watchdog device controlled through GPIO-line" > > depends on OF_GPIO > > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > > index a2126e2..5b3b053 100644 > > --- a/drivers/watchdog/Makefile > > +++ b/drivers/watchdog/Makefile > > @@ -60,6 +60,7 @@ obj-$(CONFIG_ORION_WATCHDOG) += orion_wdt.o > > obj-$(CONFIG_SUNXI_WATCHDOG) += sunxi_wdt.o > > obj-$(CONFIG_RN5T618_WATCHDOG) += rn5t618_wdt.o > > obj-$(CONFIG_COH901327_WATCHDOG) += coh901327_wdt.o > > +obj-$(CONFIG_DS1374_WATCHDOG) += ds1374-wdt.o > > obj-$(CONFIG_STMP3XXX_RTC_WATCHDOG) += stmp3xxx_rtc_wdt.o > > obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o > > obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o > > diff --git a/drivers/watchdog/ds1374-wdt.c b/drivers/watchdog/ds1374-wdt.c > > new file mode 100644 > > index 0000000..d221560 > > --- /dev/null > > +++ b/drivers/watchdog/ds1374-wdt.c > > @@ -0,0 +1,208 @@ > > +/* > > + * Copyright (c) 2017, National Instruments Corp. > > + * > > + * Dallas/Maxim DS1374 Watchdog Driver, heavily based on the older > > + * drivers/rtc/rtc-ds1374.c implementation > > + * > > + * SPDX-License-Identifier: GPL-2.0 > > + * > > + */ > > + > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/watchdog.h> > > +#include <linux/slab.h> > > +#include <linux/regmap.h> > > +#include <linux/platform_device.h> > > +#include <linux/mfd/ds1374.h> > > + > alphabetic order, please. Ok. > > > +#define DS1374_WDT_RATE 4096 /* Hz */ > > +#define DS1374_WDT_MIN_TIMEOUT 1 /* seconds */ > > +#define DS1374_WDT_DEFAULT_TIMEOUT 30 /* seconds */ > > + > Please use tabs > Will do. > > +static bool nowayout = WATCHDOG_NOWAYOUT; > > +module_param(nowayout, bool, 0444); > > +MODULE_PARM_DESC(nowayout, > > + "Watchdog cannot be stopped once started (default=" > > + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")"); > > + > > +static unsigned int timeout; > > +module_param(timeout, int, 0444); > > +MODULE_PARM_DESC(timeout, "Watchdog timeout"); > > + > > +struct ds1374_wdt { > > + struct ds1374 *chip; > > + struct device *dev; > > + struct watchdog_device wdd; > > +}; > > + > > +static int ds1374_wdt_stop(struct watchdog_device *wdog) > > +{ > > + struct ds1374_wdt *ds1374_wdt = watchdog_get_drvdata(wdog); > > + int err; > > + > > + err = regmap_update_bits(ds1374_wdt->chip->regmap, DS1374_REG_CR, > > + DS1374_REG_CR_WACE, 0); > > + if (err) > > + return err; > > + > > + if (ds1374_wdt->chip->remapped_reset) > > + return regmap_update_bits(ds1374_wdt->chip->regmap, > > + DS1374_REG_CR, DS1374_REG_CR_WDSTR, > > + 0); > > + > > + return 0; > > +} > > + > > +static int ds1374_wdt_ping(struct watchdog_device *wdog) > > +{ > > + struct ds1374_wdt *ds1374_wdt = watchdog_get_drvdata(wdog); > > + u32 val; > > + int err; > > + > > + err = ds1374_read_bulk(ds1374_wdt->chip, &val, DS1374_REG_WDALM0, 3); > > Why not just regmap_bulk_read() ? Good catch. > > > + if (err < 0) > > + return err; > > + > > + return 0; > > +} > > + > > +static int ds1374_wdt_set_timeout(struct watchdog_device *wdog, > > + unsigned int t) > > +{ > > + struct ds1374_wdt *ds1374_wdt = watchdog_get_drvdata(wdog); > > + struct regmap *regmap = ds1374_wdt->chip->regmap; > > + unsigned int timeout = DS1374_WDT_RATE * t; > > + u8 remapped = ds1374_wdt->chip->remapped_reset > > + ? DS1374_REG_CR_WDSTR : 0; > > I personally prefer to split initialization from declaration if the initialization > requires continuation lines. Yeah, it's kludgy. Will redo it. > > > + int err; > > + > > + err = regmap_update_bits(regmap, DS1374_REG_CR, > > + DS1374_REG_CR_WACE | DS1374_REG_CR_AIE, 0); > > + > > + err = ds1374_write_bulk(ds1374_wdt->chip, timeout, > > + DS1374_REG_WDALM0, 3); > > Why not just regmap_bulk_write() ? Agreed. > > The mixed use of mfd driver functions and regmap functions is confusing. > I think it would be better to avoid it. > > > + if (err) { > > + dev_err(ds1374_wdt->dev, "couldn't set new watchdog time\n"); > > Is that noise necessary ? Same elsewhere. The error is returned to user space, > which should be sufficient. Ok, will get rid of it. > > > + return err; > > + } > > + > > + ds1374_wdt->wdd.timeout = t; > > wdog->timeout = t; > > > + > > + return regmap_update_bits(regmap, DS1374_REG_CR, > > + (DS1374_REG_CR_WACE | DS1374_REG_CR_WDALM | > > + DS1374_REG_CR_AIE | DS1374_REG_CR_WDSTR), > > + (DS1374_REG_CR_WACE | DS1374_REG_CR_WDALM | > > + DS1374_REG_CR_AIE | remapped)); > > Some unnecessary ( ) Agreed. > > > +} > > + > > +static int ds1374_wdt_start(struct watchdog_device *wdog) > > +{ > > + int err; > > + struct ds1374_wdt *ds1374_wdt = watchdog_get_drvdata(wdog); > > + > > + err = ds1374_wdt_set_timeout(wdog, wdog->timeout); > > + if (err) { > > + dev_err(ds1374_wdt->dev, "%s: failed to set timeout (%d) %u\n", > > + __func__, err, wdog->timeout); > > + return err; > > + } > > + > > + err = ds1374_wdt_ping(wdog); > > + if (err) { > > + dev_err(ds1374_wdt->dev, "%s: failed to ping (%d)\n", __func__, > > + err); I assume you'd want to get rid of that one too? > > + return err; > > + } > > + > > + return 0; > > +} > > + > > +static const struct watchdog_info ds1374_wdt_info = { > > + .identity = "DS1374 WTD", > > + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING > > + | WDIOF_MAGICCLOSE, > > +}; > > + > > +static const struct watchdog_ops ds1374_wdt_ops = { > > + .owner = THIS_MODULE, > > + .start = ds1374_wdt_start, > > + .stop = ds1374_wdt_stop, > > + .set_timeout = ds1374_wdt_set_timeout, > > + .ping = ds1374_wdt_ping, > > +}; > > + > > +static int ds1374_wdt_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct ds1374 *ds1374 = dev_get_drvdata(dev->parent); > > + struct ds1374_wdt *priv; > > + int err; > > + > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + priv->chip = ds1374; > > + platform_set_drvdata(pdev, priv); > > + > > + priv->wdd.info = &ds1374_wdt_info; > > + priv->wdd.ops = &ds1374_wdt_ops; > > + priv->wdd.min_timeout = DS1374_WDT_MIN_TIMEOUT; > > + priv->wdd.timeout = DS1374_WDT_DEFAULT_TIMEOUT; > > + priv->wdd.max_timeout = 0x1ffffff / DS1374_WDT_RATE; > > + priv->wdd.parent = dev->parent; > > + > > + watchdog_init_timeout(&priv->wdd, timeout, dev); > > + watchdog_set_nowayout(&priv->wdd, nowayout); > > + watchdog_stop_on_reboot(&priv->wdd); > > + watchdog_set_drvdata(&priv->wdd, priv); > > + > > + err = devm_watchdog_register_device(dev, &priv->wdd); > > + if (err) { > > + dev_err(dev, "Failed to register watchdog device\n"); > > + return err; > > + } > > + > > + dev_info(dev, "Registered DS1374 Watchdog\n"); > > + > > + return 0; > > +} > > + > > +static int ds1374_wdt_remove(struct platform_device *pdev) > > +{ > > + struct ds1374_wdt *priv = platform_get_drvdata(pdev); > > + > > + if (!nowayout) > > + ds1374_wdt_stop(&priv->wdd); > > + > > This may bypass MAGICCLOSE: If the watchdog daemon is killed and the module removed, > the watchdog will be stopped. Is this really what you want ? If so, why set MAGICCLOSE > in the first place ? So your suggestion would be: - if (!nowayout) - ds1374_wdt_stop(&priv->wdd) > > > + return 0; > > +} > > + > > +#ifdef CONFIG_PM_SLEEP > > +static int ds1374_wdt_suspend(struct device *dev) > > +{ > > + return 0; > > +} > > + > > +static int ds1374_wdt_resume(struct device *dev) > > +{ > > + return 0; > > +} > > +#endif > > + > Those functions are quite pointless. Agreed. > > > +static SIMPLE_DEV_PM_OPS(ds1374_wdt_pm, ds1374_wdt_suspend, ds1374_wdt_resume); > > + > > +static struct platform_driver ds1374_wdt_driver = { > > + .probe = ds1374_wdt_probe, > > + .remove = ds1374_wdt_remove, > > + .driver = { > > + .name = "ds1374-wdt", > > + .pm = &ds1374_wdt_pm, > > + }, > > +}; > > +module_platform_driver(ds1374_wdt_driver); > > + > > +MODULE_AUTHOR("Moritz Fischer <mdf@xxxxxxxxxx>"); > > +MODULE_DESCRIPTION("Maxim/Dallas DS1374 WDT Driver"); > > +MODULE_LICENSE("GPL"); > > +MODULE_ALIAS("platform:ds1374-wdt"); > > diff --git a/include/dt-bindings/mfd/ds1374.h b/include/dt-bindings/mfd/ds1374.h > > new file mode 100644 > > index 0000000..b33cd5e > > --- /dev/null > > +++ b/include/dt-bindings/mfd/ds1374.h > > @@ -0,0 +1,17 @@ > > +/* > > + * This header provides macros for Maxim/Dallas DS1374 DT bindings > > + * > > + * Copyright (C) 2017 National Instruments Corp > > + * > > + * SPDX-License-Identifier: GPL-2.0 > > + * > > + */ > > + > > +#ifndef __DT_BINDINGS_MFD_DS1374_H__ > > +#define __DT_BINDINGS_MFD_DS1374_H__ > > + > > +#define DALLAS_MODE_RTC 0 > > +#define DALLAS_MODE_ALM 1 > > +#define DALLAS_MODE_WDT 2 > > + > > +#endif /* __DT_BINDINGS_MFD_DS1374_H__ */ > > diff --git a/include/linux/mfd/ds1374.h b/include/linux/mfd/ds1374.h > > new file mode 100644 > > index 0000000..7b697f8 > > --- /dev/null > > +++ b/include/linux/mfd/ds1374.h > > @@ -0,0 +1,59 @@ > > +/* > > + * Copyright (c) 2017, National Instruments Corp. > > + * > > + * Multi Function Device for Dallas/Maxim DS1374 RTC/WDT > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; version 2 of the License. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + */ > > + > > +#ifndef MFD_DS1374_H > > +#define MFD_DS1374_H > > + > > +#include <linux/i2c.h> > > +#include <linux/regmap.h> > > + > > +enum ds1374_mode { > > + DS1374_MODE_RTC_ONLY, > > + DS1374_MODE_RTC_ALM, > > + DS1374_MODE_RTC_WDT, > > +}; > > + > > +/* Register definitions to for all subdrivers > > + */ > > +#define DS1374_REG_TOD0 0x00 /* Time of Day */ > > +#define DS1374_REG_TOD1 0x01 > > +#define DS1374_REG_TOD2 0x02 > > +#define DS1374_REG_TOD3 0x03 > > +#define DS1374_REG_WDALM0 0x04 /* Watchdog/Alarm */ > > +#define DS1374_REG_WDALM1 0x05 > > +#define DS1374_REG_WDALM2 0x06 > > +#define DS1374_REG_CR 0x07 /* Control */ > > +#define DS1374_REG_CR_AIE 0x01 /* Alarm Int. Enable */ > > +#define DS1374_REG_CR_WDSTR 0x08 /* 1=Reset on INT, 0=Rreset on RST */ > > +#define DS1374_REG_CR_WDALM 0x20 /* 1=Watchdog, 0=Alarm */ > > +#define DS1374_REG_CR_WACE 0x40 /* WD/Alarm counter enable */ > > +#define DS1374_REG_SR 0x08 /* Status */ > > +#define DS1374_REG_SR_OSF 0x80 /* Oscillator Stop Flag */ > > +#define DS1374_REG_SR_AF 0x01 /* Alarm Flag */ > > +#define DS1374_REG_TCR 0x09 /* Trickle Charge */ > > + > > +struct ds1374 { > > + struct i2c_client *client; > > + struct regmap *regmap; > > + int irq; > > + enum ds1374_mode mode; > > + bool remapped_reset; > > +}; > > + > > +int ds1374_read_bulk(struct ds1374 *ds1374, u32 *time, int reg, int nbytes); > > + > > +int ds1374_write_bulk(struct ds1374 *ds1374, u32 time, int reg, int nbytes); > > + > > +#endif /* MFD_DS1374_H */ > > > Thanks for your review! Moritz PS: I haven't forgotten about the cros-ec-hwmon, I'll get back to that at one point ;-)
Attachment:
signature.asc
Description: PGP signature